diff --git a/src/audio/ownership.rs b/src/audio/ownership.rs index 740132e..ffe5c4a 100644 --- a/src/audio/ownership.rs +++ b/src/audio/ownership.rs @@ -44,8 +44,14 @@ use std::process::Command; /// after binding the node — see the module docs. pub const OWNED_PROP_KEY: &str = "peerspeak.owned"; -/// Carrier 1, value. pixelpass treats anything other than `false`/`0` as -/// truthy (fail-closed), but we always emit exactly this. +/// Carrier 1, value. +/// +/// ⚠️ **pixelpass matches this EXACTLY** (its round-10 R10-4). An earlier +/// version of this comment said it accepted anything other than `false`/`0`; +/// that leniency was removed, because it let any process suppress a rival +/// app's audio from the share with a property it did not have to spell right. +/// Emitting any other value here now means the node is not owned on carrier 1 +/// — only carrier 2's `node.name` prefix would still catch it. pub const OWNED_PROP_VALUE: &str = "1"; /// Carrier 2. The `node.name` prefix pixelpass matches; the remainder of the @@ -161,18 +167,112 @@ fn pipewire_props_value(node_name: &str) -> String { /// Tag a child process that will play audio, so its resulting node carries /// both ownership carriers. /// -/// Both environment variables are set unconditionally, because we cannot know -/// from here which audio API the child will pick (mpv chooses at runtime, and -/// VLC and `paplay` differ from `pw-play`). Setting the one the child ignores -/// costs nothing. +/// All three environment variables are set unconditionally, because we cannot +/// know from here which audio API the child will pick: mpv chooses at runtime, +/// VLC and `paplay` differ from `pw-play`, and either may be configured for +/// ALSA output. Setting the ones the child ignores costs nothing. /// -/// Any inherited value of either variable is **replaced**, not appended: the -/// tag is a correctness mechanism, and a half-merged property string that -/// fails to parse would drop it silently. +/// An inherited value is **merged** rather than replaced (round 10, R10-5). +/// Both variables can legitimately carry a user's own routing policy — +/// `media.role`, a target sink — and clobbering it changes where the user's +/// audio goes as a side effect of a tagging mechanism that is supposed to be +/// behaviourally invisible. Our pairs go **last**, so they win any duplicate +/// key. An inherited value that does not match the expected shape is logged +/// and overwritten: a half-merged string that fails to parse would drop the +/// tag silently, which is the one outcome worse than losing the user's +/// routing preference. +/// +/// ⚠️ Reachability, measured 2026-07-25: neither variable is set anywhere in +/// this user's environment or configuration, so this is a correctness +/// property with no live consumer today. pub fn tag_child(command: &mut Command, role: &str) { let node_name = owned_node_name(role); - command.env(PULSE_PROP_ENV, pulse_prop_value(&node_name)); - command.env(PIPEWIRE_PROPS_ENV, pipewire_props_value(&node_name)); + command.env( + PULSE_PROP_ENV, + merge_pulse_prop(inherited(PULSE_PROP_ENV).as_deref(), &node_name), + ); + let pipewire_props = merge_pipewire_props(inherited(PIPEWIRE_PROPS_ENV).as_deref(), &node_name); + // The ALSA carrier takes the same SPA-JSON grammar and is set here too, so + // a child configured for ALSA output — which neither of the other two + // variables reaches — is tagged with *its own* role rather than inheriting + // this process's `clip` tag from `tag_this_process_alsa_audio`. + command.env(PIPEWIRE_ALSA_ENV, &pipewire_props); + command.env(PIPEWIRE_PROPS_ENV, pipewire_props); +} + +/// This process's value for `key`, which is what a child would inherit. +/// Non-UTF-8 reads as absent — we cannot merge into bytes we cannot parse, and +/// the overwrite path is where that belongs. +fn inherited(key: &str) -> Option { + match std::env::var(key) { + Ok(value) if !value.trim().is_empty() => Some(value), + Ok(_) => None, + Err(std::env::VarError::NotPresent) => None, + Err(std::env::VarError::NotUnicode(_)) => { + crate::log_msg(&format!( + "ownership: inherited {key} is not valid UTF-8; overwriting it with the ownership tag" + )); + None + } + } +} + +/// `PULSE_PROP` is whitespace-separated `key=value`, so merging is appending. +/// +/// The shape check is every token containing `=`. A token without one is not +/// something we can append to safely — PulseAudio's parser would take the +/// whole string as malformed and we would lose the tag with it. +fn merge_pulse_prop(inherited: Option<&str>, node_name: &str) -> String { + let ours = pulse_prop_value(node_name); + let Some(existing) = inherited else { + return ours; + }; + if existing.split_whitespace().all(|pair| pair.contains('=')) { + format!("{} {ours}", existing.trim()) + } else { + crate::log_msg(&format!( + "ownership: inherited {PULSE_PROP_ENV} is not space-separated key=value \ + ({existing:?}); overwriting it with the ownership tag" + )); + ours + } +} + +/// `PIPEWIRE_PROPS` is a SPA-JSON object, so merging is inserting our pairs +/// before the closing brace. +/// +/// The shape check is the outer braces. Anything else — a bare `key = value` +/// list, a truncated object — is left alone and overwritten rather than +/// guessed at; a full SPA-JSON parser is not worth writing for a case with no +/// live consumer. +fn merge_pipewire_props(inherited: Option<&str>, node_name: &str) -> String { + let ours = pipewire_props_value(node_name); + let Some(existing) = inherited else { + return ours; + }; + let trimmed = existing.trim(); + let inner = trimmed + .strip_prefix('{') + .and_then(|rest| rest.strip_suffix('}')) + .map(str::trim); + match inner { + // An empty object carries nothing to preserve. + Some("") => ours, + Some(inner) => { + let separator = if inner.ends_with(',') { "" } else { "," }; + format!( + "{{ {inner}{separator} \"{OWNED_PROP_KEY}\" = \"{OWNED_PROP_VALUE}\", \ + \"node.name\" = \"{node_name}\" }}" + ) + } + None => { + crate::log_msg(&format!( + "ownership: inherited {PIPEWIRE_PROPS_ENV} is not a SPA-JSON object \ + ({existing:?}); overwriting it with the ownership tag" + )); + ours + } + } } /// Tag the audio this process plays **through ALSA** — in practice everything @@ -499,6 +599,130 @@ mod tests { ); } + /// R10-5: a user's inherited `PULSE_PROP` survives, and our pairs go last + /// so they win any duplicate key. + /// + /// Tested on the merge functions rather than through `tag_child`, because + /// `tag_child` reads the *process* environment and a test that set it + /// would race every other test in the binary. + #[test] + fn pulse_prop_merges_rather_than_clobbering() { + let name = owned_node_name("mpv"); + let merged = merge_pulse_prop(Some("media.role=music target.object=my-sink"), &name); + + assert!( + merged.contains("media.role=music") && merged.contains("target.object=my-sink"), + "the user's routing policy survives: {merged}" + ); + assert!(merged.contains(&format!("{OWNED_PROP_KEY}={OWNED_PROP_VALUE}"))); + assert!(merged.contains(&format!("node.name={name}"))); + + // Still parses as whitespace-separated key=value, with ours last. + let pairs: Vec<(&str, &str)> = merged + .split_whitespace() + .map(|pair| pair.split_once('=').expect("key=value")) + .collect(); + assert_eq!(pairs.len(), 4, "{merged}"); + assert_eq!(pairs[pairs.len() - 1].0, "node.name"); + } + + /// Ours must win a key the inherited value already sets, or a user with + /// `node.name` in their environment would silently untag us. + #[test] + fn our_pulse_prop_pairs_win_a_duplicate_key() { + let name = owned_node_name("mpv"); + let merged = merge_pulse_prop(Some("node.name=theirs"), &name); + let last_name = merged + .split_whitespace() + .filter_map(|pair| pair.split_once('=')) + .rfind(|(key, _)| *key == "node.name") + .expect("a node.name pair"); + assert_eq!(last_name.1, name, "ours must come last: {merged}"); + } + + #[test] + fn a_malformed_pulse_prop_is_overwritten_not_half_merged() { + let name = owned_node_name("mpv"); + // No `=` in the token: not something we can safely append to. + let merged = merge_pulse_prop(Some("garbage-without-equals"), &name); + assert_eq!(merged, pulse_prop_value(&name)); + assert!(!merged.contains("garbage")); + } + + #[test] + fn pipewire_props_merges_inside_the_spa_json_object() { + let name = owned_node_name("vlc"); + let merged = merge_pipewire_props(Some("{ \"media.role\" = \"music\" }"), &name); + + assert!(merged.starts_with('{') && merged.ends_with('}'), "{merged}"); + assert!(merged.contains("\"media.role\" = \"music\""), "{merged}"); + assert!(merged.contains(&format!("\"{OWNED_PROP_KEY}\" = \"{OWNED_PROP_VALUE}\""))); + assert!(merged.contains(&format!("\"node.name\" = \"{name}\""))); + // Exactly one object, not two concatenated. + assert_eq!(merged.matches('{').count(), 1, "{merged}"); + assert_eq!(merged.matches('}').count(), 1, "{merged}"); + } + + /// A trailing comma in the inherited object must not become a double + /// comma, which SPA-JSON would reject — taking our tag down with it. + #[test] + fn pipewire_props_merge_does_not_double_a_trailing_comma() { + let name = owned_node_name("vlc"); + let merged = merge_pipewire_props(Some("{ \"media.role\" = \"music\", }"), &name); + assert!(!merged.contains(",,"), "{merged}"); + assert!(!merged.contains(", ,"), "{merged}"); + assert!(merged.contains("\"media.role\" = \"music\""), "{merged}"); + } + + #[test] + fn an_empty_or_malformed_pipewire_props_yields_our_value_alone() { + let name = owned_node_name("vlc"); + assert_eq!( + merge_pipewire_props(Some("{ }"), &name), + pipewire_props_value(&name) + ); + // Not an object: overwritten rather than guessed at. + assert_eq!( + merge_pipewire_props(Some("media.role = music"), &name), + pipewire_props_value(&name) + ); + assert_eq!( + merge_pipewire_props(None, &name), + pipewire_props_value(&name) + ); + } + + /// A spawned player configured for ALSA output is reached by neither + /// `PULSE_PROP` nor `PIPEWIRE_PROPS`. `tag_child` therefore sets the ALSA + /// carrier too — with the *child's* role, so it does not inherit this + /// process's `clip` tag from `tag_this_process_alsa_audio`. + #[test] + fn tagged_child_command_also_carries_the_alsa_carrier_with_its_own_role() { + let mut command = Command::new("mpv"); + tag_child(&mut command, "mpv"); + + let env: BTreeMap = command + .get_envs() + .map(|(key, value)| { + ( + key.to_string_lossy().into_owned(), + value + .expect("tagging sets values, never removals") + .to_string_lossy() + .into_owned(), + ) + }) + .collect(); + + let alsa = env.get(PIPEWIRE_ALSA_ENV).expect("PIPEWIRE_ALSA set"); + assert!( + alsa.contains(&format!("\"node.name\" = \"{}\"", owned_node_name("mpv"))), + "the child's own role, not this process's clip role: {alsa}" + ); + assert!(!alsa.contains(&format!("_{CLIP_ROLE}_")), "{alsa}"); + assert!(alsa.contains(&format!("\"{OWNED_PROP_KEY}\" = \"{OWNED_PROP_VALUE}\""))); + } + #[test] fn every_generated_name_matches_the_prefix_pixelpass_looks_for() { for role in ["call", "mpv", "vlc", "notify"] {