diff --git a/src/audio/clip_player.rs b/src/audio/clip_player.rs index 6ee3ba4..fcd0492 100644 --- a/src/audio/clip_player.rs +++ b/src/audio/clip_player.rs @@ -328,4 +328,54 @@ mod tests { assert_eq!(seek_target(-1.0, total), Duration::ZERO); assert_eq!(seek_target(2.0, total), total); } + + /// **The fourth playback path's exit gate (round 10, R10-2).** Drives a + /// real [`ClipPlayer`] — the same object the app uses for chat clips, peer + /// music and the local playlist — and asserts the node it puts on the + /// graph carries both ownership carriers. + /// + /// This path was untagged through all of phase 1, which is a real echo: + /// B broadcasts music, A tunes in, A shares their desktop, B hears their + /// own track. It was missed because phase 1 worked from the impl plan's + /// list of three playback sites and that list was incomplete — so this + /// gate drives the *player*, not the tagging helper. + /// + /// ⚠️ **Run alone**: it sets a process-wide environment variable, which is + /// only sound single-threaded. In production `main` does this before + /// anything is spawned; a test binary has no such guarantee, hence + /// `--test-threads=1`. + /// + /// `cargo test --lib -- --ignored --test-threads=1 clip_player_node` + #[test] + #[ignore = "live: requires a running PipeWire daemon and pw-dump; run with --test-threads=1"] + fn clip_player_node_carries_both_ownership_carriers() { + use crate::audio::ownership::{self, live_test}; + + // SAFETY: `--test-threads=1` is documented above and in the ignore + // reason; this is the same call `main` makes, exercised for real + // rather than reimplemented, so the gate cannot pass against a + // formatter that production never uses. + unsafe { ownership::tag_this_process_alsa_audio() }; + + let (player, _status) = ClipPlayer::new(1.0); + // Six seconds of silence: long enough for the poll, inaudible. + player.play([0u8; 32], live_test::silent_wav(6)); + + let prefix = live_test::expected_prefix(ownership::CLIP_ROLE); + let found = live_test::poll_for_owned_node(&prefix, Duration::from_secs(5)); + player.stop(); + + let (name, owned) = found.unwrap_or_else(|| { + panic!("no live clip-player node named {prefix:?} appeared within 5s") + }); + assert!( + name.starts_with(ownership::OWNED_NODE_NAME_PREFIX), + "{name}" + ); + assert_eq!( + owned.as_deref(), + Some(ownership::OWNED_PROP_VALUE), + "carrier 1 must be on the live node, not just carrier 2" + ); + } } diff --git a/src/audio/mod.rs b/src/audio/mod.rs index 6bca441..9a1e9ac 100644 --- a/src/audio/mod.rs +++ b/src/audio/mod.rs @@ -65,6 +65,10 @@ pub mod eq; pub mod gate; pub mod limiter; pub mod multitrack; +// The cross-repo ownership tag (plan §5.1). Platform-neutral on purpose: the +// carriers only matter on PipeWire, but the literals are a wire contract and +// their test must run on every platform so a rename can't pass CI elsewhere. +pub mod ownership; pub mod pan; // Linear resamplers used by the Windows/cpal backend (W4). Platform-neutral and // pure, so it builds (and its tests run) everywhere even though only the cpal diff --git a/src/audio/ownership.rs b/src/audio/ownership.rs new file mode 100644 index 0000000..481f0db --- /dev/null +++ b/src/audio/ownership.rs @@ -0,0 +1,1572 @@ +//! Ownership tagging for every audio node peerspeak puts on the graph. +//! +//! Zero behaviour change on its own. Its whole purpose is to make peerspeak's +//! own playback *identifiable from outside the process*, so that pixelpass's +//! screenshare audio exclusion engine can refuse to fan our own call audio +//! back into the stream the far end is already hearing (the echo it exists to +//! prevent). See `docs/screenshare-audio-exclusion-plan.md` §5.1 and +//! `docs/screenshare-audio-exclusion-impl-plan.md` §3. +//! +//! # The wire contract +//! +//! The literals below are a **cross-repo contract**, not local naming. +//! pixelpass matches on them and does not depend on this crate (nor this +//! crate on it) — the coupling is the PipeWire graph itself. The values are +//! committed in both repos as `tests/fixtures/ownership-tag-contract.txt`, +//! and [`contract_matches_fixture`](tests::contract_matches_fixture) asserts +//! these constants against that file. pixelpass's matching constants are in +//! `src/host/taint/mod.rs` (`PEERSPEAK_OWNED_PROP`, +//! `PEERSPEAK_OWNED_NODE_PREFIX`), asserted against its own copy of the same +//! fixture. +//! +//! # Why two carriers +//! +//! A node is owned if **either** carrier matches — a union, which is the +//! fail-closed direction (a missed tag means our audio gets shared; a +//! spurious tag only over-excludes). Round 8 of the design added the second +//! carrier after the phase-5 audit found that a node *property* is invisible +//! to the PipeWire registry's `global` event and recoverable only by binding +//! the node, while `node.name` is announced directly. The primary taint root +//! should not rest on a single observation mechanism. +//! +//! `node.description` is deliberately left alone, so mixers still show "mpv" +//! rather than an internal identifier. +//! +//! # Known limitation (accepted for v1) +//! +//! The child environment is inherited by *grandchildren*, so a player that +//! spawns its own audio helper tags it too. That over-excludes rather than +//! leaks, and is the accepted v1 limitation recorded in plan §5.1. + +use std::process::Command; + +/// Carrier 1, key. Set on every peerspeak-owned node. Read by pixelpass only +/// after binding the node — see the module docs. +pub const OWNED_PROP_KEY: &str = "peerspeak.owned"; + +/// 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 +/// name is diagnostic only. Format: `peerspeak_owned__`. +pub const OWNED_NODE_NAME_PREFIX: &str = "peerspeak_owned_"; + +/// The environment variable through which a PulseAudio-API client (mpv/VLC on +/// `--ao=pulse`, `paplay`) receives extra stream properties. +pub const PULSE_PROP_ENV: &str = "PULSE_PROP"; + +/// The environment variable through which a PipeWire-native client +/// (`pw-play`, mpv on `--ao=pipewire`) receives extra stream properties. +pub const PIPEWIRE_PROPS_ENV: &str = "PIPEWIRE_PROPS"; + +/// The environment variable read **only by PipeWire's ALSA plugin**, and so +/// the one that reaches audio played through ALSA — which on Linux is how +/// `rodio`/`cpal` reach the graph (round 10, R10-2). +/// +/// This is the *in-process* carrier: unlike [`PULSE_PROP_ENV`] and +/// [`PIPEWIRE_PROPS_ENV`], which are set per-child on a [`Command`], this one +/// is set process-wide in `main` because the audio it tags is played by a +/// library inside this process, with no fork to hang properties on. +/// +/// **Measured 2026-07-25**, with `PIPEWIRE_PROPS` and `PULSE_PROP` explicitly +/// unset, to establish that it is surgical enough to set process-wide: +/// +/// - `aplay` (ALSA plugin) ⇒ node carries **both** carriers. ✅ +/// - `pw-play` (PipeWire-native) ⇒ `node.name=pw-play`, `peerspeak.owned` +/// absent. ✅ So our own native call-playback and capture streams, which go +/// through `pipewire_impl`, are **not** affected by this variable; they keep +/// being tagged explicitly at their own site. +/// - `arecord` (ALSA plugin, capture) ⇒ **is** tagged, on a +/// `Stream/Input/Audio`. ⚠️ Not surgical in the *role* dimension. Harmless +/// only because pixelpass honours the carriers on `Stream/Output/Audio` +/// alone (R10-1) — which is why R10-1 lands first. Nothing in peerspeak +/// captures via ALSA on Linux today (the Linux backend is native PipeWire; +/// `cpal` is the Windows backend), so this is a guard against a future +/// caller, not a live condition. +pub const PIPEWIRE_ALSA_ENV: &str = "PIPEWIRE_ALSA"; + +/// Role of in-process audio played through `rodio`: received chat clips, peer +/// music, and locally chosen playlist tracks (round 10, R10-2). +/// +/// # Policy: peerspeak's own playlist audio is never screen-shareable +/// +/// **All three are tagged, and for the local playlist that is a deliberate +/// policy rather than a consequence** (round 10 review, finding 2). +/// +/// ⚠️ An earlier version of this comment justified it by claiming a local +/// track is "already being broadcast to peers on the same keypress", so +/// sharing it again would send the far end two copies. **That claim is false +/// in the default configuration** and Codex was right to call it: shared +/// listening is opt-in, `Config::music_broadcast` defaults to `false` (pinned +/// by `config::tests::test_input_output_volume_fields`), +/// `app::play_music_index` starts local playback unconditionally, and +/// `app::broadcast_track` returns immediately when +/// `can_broadcast_music` is false. With broadcasting off there is no second +/// copy and no defect — only a track the user chose that they cannot put in +/// the share. +/// +/// The policy stands anyway, on cost rather than on that argument: +/// +/// - The two carriers reach `rodio` through [`PIPEWIRE_ALSA_ENV`], which is +/// **process-wide**. `clip_player` and `music_player` are two `ClipPlayer` +/// instances in one process (`app::AppState`), so no value of that variable +/// can tag one and not the other, and `set_var` cannot be flipped at +/// runtime once threads exist. +/// - Exempting the playlist therefore means giving it a separately taggable +/// stream — a helper process, or rebuilding `ClipPlayer` on peerspeak's own +/// PipeWire backend — for a case with a one-step workaround. +/// - Tagging is **not** optional for the other two roles: received clips and +/// peer music are the far end's own audio, and re-sharing them is the exact +/// echo this feature exists to prevent. +/// +/// So: a user who wants music inside the screen share plays it in any other +/// application, which peerspeak never tags. When shared listening *is* on, +/// the original two-copies argument does apply, and the policy is simply +/// right for that case too. +pub const CLIP_ROLE: &str = "clip"; + +/// Role of the native call-playback stream — the node carrying the far end's +/// voice. +pub const NATIVE_PLAYBACK_ROLE: &str = "call"; + +/// Role of a notification-sound player (`pw-play`/`paplay`/`aplay`). +pub const NOTIFICATION_ROLE: &str = "notify"; + +/// Build the `node.name` for a node this process owns. +/// +/// The pid is **peerspeak's own**, not the child's: for a spawned player we +/// set the name before the fork, so the child's pid does not exist yet — and +/// the owning process is the more useful thing to record anyway. +pub fn owned_node_name(role: &str) -> String { + format!( + "{OWNED_NODE_NAME_PREFIX}{}_{}", + sanitize_role(role), + std::process::id() + ) +} + +/// Keep a role to `[a-z0-9-]`, lowercased, so the generated `node.name` is a +/// single shell-safe, whitespace-free token. +/// +/// Not defensive theatre: [`PULSE_PROP_ENV`] is a space-separated +/// `key=value` encoding, so a role containing a space would silently split +/// into a second bogus property. Every current caller passes a literal, and +/// this keeps that true no matter who calls next. +fn sanitize_role(role: &str) -> String { + let cleaned: String = role + .chars() + .map(|c| { + if c.is_ascii_alphanumeric() { + c.to_ascii_lowercase() + } else { + '-' + } + }) + .collect(); + let trimmed = cleaned.trim_matches('-'); + if trimmed.is_empty() { + "child".to_string() + } else { + trimmed.to_string() + } +} + +/// The `PULSE_PROP` value carrying both carriers. +fn pulse_prop_value(node_name: &str) -> String { + format!("{OWNED_PROP_KEY}={OWNED_PROP_VALUE} node.name={node_name}") +} + +/// The `PIPEWIRE_PROPS` value carrying both carriers. +/// +/// SPA-JSON; keys are quoted because `peerspeak.owned` contains a dot. +fn pipewire_props_value(node_name: &str) -> String { + format!("{{ \"{OWNED_PROP_KEY}\" = \"{OWNED_PROP_VALUE}\", \"node.name\" = \"{node_name}\" }}") +} + +/// Tag a child process that will play audio, so its resulting node carries +/// both ownership carriers. +/// +/// 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. +/// +/// An inherited value is **merged** rather than replaced (round 10, R10-5). +/// All three 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. 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. +/// +/// ✅ **Our pairs go last, and last wins — measured 2026-07-25**, not assumed. +/// `PIPEWIRE_PROPS='{ "node.name" = "theirs_first", "media.role" = "music", +/// "node.name" = "ours_last" }'` on `pw-play` produced `node.name=ours_last` +/// with `media.role` preserved, and the `PULSE_PROP` equivalent on `paplay` +/// did the same. So a user who already sets `node.name` cannot silently +/// untag us, and their other keys survive. +/// +/// ⚠️ Reachability, measured 2026-07-25: none of the three 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, + merge_pulse_prop(inherited(PULSE_PROP_ENV).as_deref(), &node_name), + ); + command.env( + PIPEWIRE_PROPS_ENV, + merge_pipewire_props( + inherited(PIPEWIRE_PROPS_ENV).as_deref(), + &node_name, + PIPEWIRE_PROPS_ENV, + ), + ); + // 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 at all, and with *its own* role. + // + // Merged from the inherited `PIPEWIRE_ALSA`, which by now holds this + // process's own `clip` tag from [`tag_this_process_alsa_audio`] plus + // whatever the user set. The duplicate `node.name` that produces is + // resolved in our favour by the measured last-wins rule above, so the + // child gets its own role rather than inheriting `clip`. + command.env( + PIPEWIRE_ALSA_ENV, + merge_pipewire_props( + inherited(PIPEWIRE_ALSA_ENV).as_deref(), + &node_name, + PIPEWIRE_ALSA_ENV, + ), + ); +} + +/// 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 + } +} + +/// The diagnostic for an inherited value we refuse to merge into. +/// +/// A pure function so a test can assert the variable name is the one that was +/// actually malformed; as an inline `format!` inside the logging call it was +/// ungated, and it was wrong — it always said `PIPEWIRE_PROPS`, including for +/// `PIPEWIRE_ALSA`. +fn malformed_object_warning(var: &str, existing: &str) -> String { + format!( + "ownership: inherited {var} is not a well-formed SPA-JSON object \ + ({existing:?}); overwriting it with the ownership tag" + ) +} + +/// A SPA-JSON object that **libspa itself** has validated. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct SpaObject { + /// How many tokens the object holds. Only ever compared against zero, to + /// tell an empty object from one with something worth preserving, so + /// tokens rather than pairs is fine. + tokens: usize, + /// Byte offset of the object's **own** closing `}` within the validated + /// text. + /// + /// ⚠️ **This exists because searching for the brace ourselves was a + /// measured fail-open** (round 12 review, finding 1). The caller used + /// `rfind('}')`, which is a byte scan, not a parse — the last brace in the + /// text is not necessarily the one that closes the object. libspa reports + /// the true position instead: closing a container at depth 0 writes the + /// brace's own position back to the parent iterator + /// (`iter->parent->cur = iter->cur`, `spa/utils/json-core.h`). + closer: usize, +} + +/// Validate a SPA-JSON **object** with **libspa's own parser** — the same code +/// the daemon runs — and report what it found. +/// +/// ⚠️ **This was a hand-written scanner for one round, and that is worth +/// recording** (round 11 review, findings 2 and 3). Checking the outer braces +/// was not a validity check; neither is checking that brackets balance. +/// Measured against `spa-json-dump -s` on this host, the hand-written version +/// disagreed with the real grammar in **both** directions: +/// +/// - It **accepted** `{ "foo" = { garbage } }`, `{ "a" = "\é" }` (invalid +/// escape), `{ "a" = é }` (non-ASCII bare token) and `{ "a" = foo\bar }`. +/// Merging into those put an invalid pair *before* our carriers, so the +/// daemon would stop at it and drop both — recreating the exact fail-open +/// the fix existed to close. +/// - It **rejected** `{ target.object, "my-sink" }` (a comma between key and +/// value is ignored by SPA-JSON), `{ key == "value" }`, and CR-terminated +/// comments, all of which are valid — so a user with one of those in their +/// environment silently lost their routing policy to an overwrite. +/// +/// A second implementation of someone else's grammar is a liability in both +/// directions at once. Using theirs makes the disagreement impossible. +/// +/// `None` for anything that is not one complete, well-formed object, including +/// trailing text after it. A trailing **comment** is not trailing text — +/// libspa ends the document at one (`case __COMMENT: return 0`), so +/// `{ "a" = "b" } # note` is a valid object whose closer is not the last brace +/// in the string. That is exactly why [`SpaObject::closer`] is reported here +/// rather than searched for by the caller. +#[cfg(target_os = "linux")] +fn spa_object(text: &str) -> Option { + use pipewire::spa::sys as spa; + + // SAFETY: the iterators borrow `text` and never outlive this call, and + // every pointer libspa hands back points into `text`. No slice is taken — + // only counts and libspa's own predicates — so there is no span + // arithmetic to get wrong. + unsafe { + let mut outer = std::mem::zeroed::(); + spa::spa_json_init( + &mut outer, + text.as_ptr().cast::(), + text.len(), + ); + + // The whole input must be exactly one object. + let mut token: *const std::os::raw::c_char = std::ptr::null(); + let length = spa::spa_json_next(&mut outer, &mut token); + if length <= 0 || spa::spa_json_is_object(token, length) == 0 { + return None; + } + + // ⚠️ **Nothing here re-implements the grammar, and that is the whole + // point.** An earlier cut walked the object itself, checking key/value + // pairing and recursing into nested containers. Mutation testing then + // showed every one of those checks was redundant: `spa_json_next` + // already returns an error for `{ garbage }`, for + // `{ "foo" = { garbage } }`, and for an unterminated string, and it + // *skips* a nested container rather than descending into it — so a + // flat token walk is both correct and complete. Deleting the + // hand-written half is what makes "use their grammar, not ours" + // actually true. + let mut object = std::mem::zeroed::(); + spa::spa_json_enter(&mut outer, &mut object); + + let mut tokens = 0; + loop { + let mut token: *const std::os::raw::c_char = std::ptr::null(); + let length = spa::spa_json_next(&mut object, &mut token); + if length == 0 { + break; + } + // ⚠️ **Load-bearing for termination, not just correctness.** + // Mutation-tested: with this removed, libspa keeps reporting the + // same error without advancing the cursor and the loop spins + // forever — the test run hung rather than failed, which is a much + // worse way to find out. + if length < 0 { + return None; + } + tokens += 1; + // Belt and braces on the same hazard: every token consumes at + // least one byte, so a token count past the input length means the + // iterator is not advancing and we must not trust it. + if tokens > text.len() { + return None; + } + + // **Step over a container the way the real consumer does.** + // + // Both variables are parsed by `pw_properties_update_string`, which + // walks top-level pairs and calls `spa_json_container_len` on a + // container value. That helper enters one more sub-iterator before + // its flat walk, and that single extra level is the whole reason + // our ceiling used to sit one below the consumer's: measured, we + // accepted depth 513 and rejected 514, while the ALSA plugin + // accepts 514 and rejects 515. Doing what it does puts the two + // boundaries on the same number instead of one apart. + // + // ⚠️ **Matching libspa's *grammar* here would be a bug, not a + // fix.** `spa-json-dump` accepts arbitrary nesting, so a recursive + // walk would make us accept depth 515+, which every real consumer + // rejects outright — we would splice carriers into a value the + // plugin discards wholesale and lose both. Measured live: at depth + // 515 the node came back as `alsa_playback.aplay` with no + // properties at all. The dump tool is the grammar oracle; it is + // not the acceptance oracle. + if spa::spa_json_is_container(token, length) != 0 + && spa::spa_json_container_len(&mut object, token, length) <= 0 + { + return None; + } + } + + // Where the object actually closes, straight from the parser. + // + // ⚠️ **Read it here, before the trailing check below**, which advances + // `outer` past the brace. Closing a container at depth 0 executes + // `iter->parent->cur = iter->cur` with `cur` still *at* the `}` + // (`spa/utils/json-core.h`), and `spa_json_enter` set `outer` as this + // sub-iterator's parent, so `outer.cur` now points at the brace. + let closer = (outer.cur as usize).checked_sub(text.as_ptr() as usize)?; + if closer >= text.len() || text.as_bytes()[closer] != b'}' { + // The writeback did not land where the contract says it should. + // Refusing is the safe direction: the caller overwrites. + return None; + } + + // Nothing may follow the object. + let mut trailing: *const std::os::raw::c_char = std::ptr::null(); + if spa::spa_json_next(&mut outer, &mut trailing) != 0 { + return None; + } + Some(SpaObject { tokens, closer }) + } +} + +/// Off Linux there is nothing to merge into: `PIPEWIRE_PROPS` and +/// `PIPEWIRE_ALSA` are PipeWire's own and libspa is a Linux-only dependency, +/// so refusing to validate makes [`merge_pipewire_props`] overwrite — the +/// right answer on a platform where the variable has no reader. +/// +/// The module stays platform-neutral on purpose (see `audio::mod`): the +/// literals are a wire contract and their test must run everywhere. +#[cfg(not(target_os = "linux"))] +fn spa_object(_text: &str) -> Option { + None +} + +/// `PIPEWIRE_PROPS` and `PIPEWIRE_ALSA` are SPA-JSON objects, so merging is +/// inserting our two pairs before the closing brace of a **validated** object. +/// +/// Anything that is not a complete, well-formed object — a bare `key = value` +/// list, a truncated object, an unterminated string, a key with no value, an +/// invalid escape — is logged and overwritten rather than guessed at. +/// +/// Three properties make the splice safe, and all three depend on the object +/// having been validated first: +/// +/// 1. **libspa says where the object closes**; we never look for the brace +/// ourselves. ⚠️ **This point used to read "the last `}` is the object's +/// own", and that was wrong** (round 12 review, finding 1). A trailing +/// comment is not trailing *text* — libspa ends the document at one — so +/// `{ "a" = "b" } # trailing }` is valid and its last brace belongs to the +/// comment. `rfind('}')` picked that one, spliced both carriers into the +/// comment, and the re-validation below still passed because the result +/// parsed fine as `{ a = "b" }`. Measured against `spa-json-dump -s`: +/// semantically no carriers at all, which is the round-10 fail-open reached +/// through a third door. [`SpaObject::closer`] is the fix. +/// 2. **Our pairs cannot land inside a comment.** A `#` comment runs to the +/// end of its line, so the object's own closing brace is never inside one, +/// and the text immediately before it is never mid-comment. Only spaces and +/// tabs are trimmed before appending — never a newline — so a value ending +/// in `# note\n ` keeps its line break. +/// 3. **A leading comma is always legal.** Measured: SPA-JSON treats commas +/// as pure separators, accepting `{ "a" = "b",, "c" = "d" }` and even +/// `{ target.object, "my-sink" }`. +/// +/// Anything that followed the object is **preserved**, so a user's trailing +/// comment survives the merge rather than being silently deleted. +/// +/// And then the result is checked before it is returned — **not merely that it +/// parses, but that it closes where we put our brace**. That asks "are our +/// carriers really members of this object?" rather than "is this string +/// parseable", and a parse-only check is precisely what round 12 defeated. +/// +/// ⚠️ Honest note, and it applies to **two** guards here: that closer +/// comparison, and `spa_object`'s "must be an object" test. **Mutation testing +/// distinguishes neither.** Replacing the comparison with "any parse succeeds" +/// leaves the suite green, because once the closer comes from libspa the +/// splice lands correctly for every input we can construct — only comments and +/// whitespace may follow a validated object, so the tail can never re-open +/// one. Removing the object test changes nothing because a non-object has no +/// `}` to splice before. Both are deliberate belt-and-braces on the one code +/// path where being wrong is a silent echo, not load-bearing logic — and +/// saying so is better than leaving a future reader to assume a test is +/// watching them. +/// +/// `var` names the variable being merged, and is a parameter rather than a +/// constant because both variables take this grammar: the warning used to say +/// `PIPEWIRE_PROPS` even when `PIPEWIRE_ALSA` was the malformed one. +fn merge_pipewire_props(inherited: Option<&str>, node_name: &str, var: &str) -> String { + let ours = pipewire_props_value(node_name); + let Some(existing) = inherited else { + return ours; + }; + + let overwrite = || { + crate::log_msg(&malformed_object_warning(var, existing)); + ours.clone() + }; + + // An empty object carries nothing worth preserving. + let Some(object) = spa_object(existing) else { + return overwrite(); + }; + if object.tokens == 0 { + return ours; + } + let closer = object.closer; + // A trailing separator of the user's would make ours a double comma. That + // is legal SPA-JSON (measured) but it reads like a bug, so drop it. It + // cannot be a comma inside a comment: a comment runs to end of line, so an + // object whose closing brace followed one on the same line would not have + // validated. + let kept = existing[..closer] + .trim_end_matches([' ', '\t']) + .trim_end_matches(','); + // Whatever followed the object is the user's — in practice a comment — and + // goes back untouched. The previous cut silently deleted it, having already + // mistaken part of it for the closing brace. + let tail = &existing[closer + 1..]; + + let head = format!( + "{kept}, \"{OWNED_PROP_KEY}\" = \"{OWNED_PROP_VALUE}\", \ + \"node.name\" = \"{node_name}\" " + ); + let our_closer = head.len(); + let merged = format!("{head}}}{tail}"); + + // **Does the object we just built actually contain our carriers?** + // + // Parsing the result back is not enough on its own, and that is precisely + // how round 12's fail-open survived a re-validation: the emitted text + // parsed cleanly while both carriers sat inside a comment, semantically + // absent. Asking *where libspa thinks the object closes* answers the real + // question. If anything swallowed our pairs, the object closes at some + // earlier brace and this comparison fails, so we overwrite instead of + // shipping a value whose tag is decorative. + match spa_object(&merged) { + Some(check) if check.closer == our_closer => merged, + _ => overwrite(), + } +} + +/// Tag the audio this process plays **through ALSA** — in practice everything +/// `rodio` plays, which is [`crate::audio::clip_player::ClipPlayer`]: received +/// chat clips, peer music, and local playlist tracks. +/// +/// `ClipPlayer` opens a `rodio` default sink, which on Linux reaches the graph +/// through PipeWire's ALSA plugin. It is a genuine fourth playback path and it +/// was untagged until round 10, which is a real echo: B broadcasts music, A +/// tunes in, A shares their desktop, and B hears their own track played back +/// at them. Found by Codex's phase-1 review; confirmed live as +/// `alsa_playback.peerspeak-…` with no ownership properties at all. +/// +/// # Why an environment variable and not a property on the stream +/// +/// `rodio` exposes no way to set PipeWire node properties — it is an ALSA +/// consumer several layers down. The alternative fix is to rebuild `ClipPlayer` +/// on peerspeak's own PipeWire backend, which is a large change for an +/// identical outcome. [`PIPEWIRE_ALSA_ENV`] reaches exactly the ALSA path and +/// nothing else (measured — see the constant). +/// +/// # Must be called before any thread starts +/// +/// `std::env::set_var` is `unsafe` in edition 2024 because a concurrent +/// `getenv` in another thread is a data race. The only safe moment is the top +/// of `main`, before anything is spawned; that is also correct on the merits, +/// since the plugin reads the variable when a stream is opened. +/// +/// An inherited value is **merged**, on the same reasoning as [`tag_child`]'s +/// (R10-5): a user's `PIPEWIRE_ALSA` can carry routing policy, and this +/// function is not entitled to move their audio as a side effect of tagging +/// ours. Ours goes last and last wins (measured — see [`tag_child`]). +/// +/// # Safety +/// +/// The caller must guarantee no other thread exists in this process. +pub unsafe fn tag_this_process_alsa_audio() { + let node_name = owned_node_name(CLIP_ROLE); + let value = merge_pipewire_props( + inherited(PIPEWIRE_ALSA_ENV).as_deref(), + &node_name, + PIPEWIRE_ALSA_ENV, + ); + // SAFETY: the caller's obligation, discharged by calling this at the top + // of `main` before any thread is spawned. + unsafe { std::env::set_var(PIPEWIRE_ALSA_ENV, value) }; +} + +/// Live-test support for the phase-1 exit gate, shared by the two modules +/// that spawn tagged players (`notify`, `screenshare`). +/// +/// Lives here rather than in either test module because the gate is one +/// property — "the carriers reach a live node" — asked of two call sites, and +/// two copies of the poll would drift. +#[cfg(all(test, not(windows)))] +pub mod live_test { + use std::process::Command; + use std::time::{Duration, Instant}; + + /// Poll `pw-dump` for a `Stream/Output/Audio` node whose `node.name` + /// starts with `prefix`; returns its name and `peerspeak.owned` value. + /// + /// Polls rather than sleeping once because node creation races the + /// player process starting up: a fixed sleep either flakes or is slow. + pub fn poll_for_owned_node( + prefix: &str, + timeout: Duration, + ) -> Option<(String, Option)> { + let deadline = Instant::now() + timeout; + while Instant::now() < deadline { + if let Some(found) = scan_once(prefix) { + return Some(found); + } + std::thread::sleep(Duration::from_millis(100)); + } + None + } + + fn scan_once(prefix: &str) -> Option<(String, Option)> { + let out = Command::new("pw-dump").output().ok()?; + let dump: serde_json::Value = serde_json::from_slice(&out.stdout).ok()?; + for object in dump.as_array()? { + let props = &object["info"]["props"]; + // `continue`, never `?`: most objects in a dump are not nodes and + // have no `node.name` at all, and propagating that as `None` + // would abandon the scan at the first one. + let Some(name) = props["node.name"].as_str() else { + continue; + }; + if !name.starts_with(prefix) { + continue; + } + // PipeWire may hand the property back as a JSON string or a + // number depending on how the client set it; both are the same + // property, and reading only one would make the assertion + // accidentally depend on the producer's encoding. + let owned = props[super::OWNED_PROP_KEY] + .as_str() + .map(str::to_string) + .or_else(|| props[super::OWNED_PROP_KEY].as_i64().map(|n| n.to_string())); + return Some((name.to_string(), owned)); + } + None + } + + /// A zero-filled 48 kHz stereo s16 WAV of `seconds` length — long enough + /// for the poll to catch the node, silent so the gate never makes noise. + pub fn silent_wav(seconds: u32) -> Vec { + let data_len = 48_000 * seconds * 4; + let mut wav = Vec::with_capacity(44 + data_len as usize); + wav.extend_from_slice(b"RIFF"); + wav.extend_from_slice(&(36 + data_len).to_le_bytes()); + wav.extend_from_slice(b"WAVEfmt "); + wav.extend_from_slice(&16u32.to_le_bytes()); + wav.extend_from_slice(&1u16.to_le_bytes()); // PCM + wav.extend_from_slice(&2u16.to_le_bytes()); // stereo + wav.extend_from_slice(&48_000u32.to_le_bytes()); + wav.extend_from_slice(&(48_000u32 * 4).to_le_bytes()); // byte rate + wav.extend_from_slice(&4u16.to_le_bytes()); // block align + wav.extend_from_slice(&16u16.to_le_bytes()); // bits per sample + wav.extend_from_slice(b"data"); + wav.extend_from_slice(&data_len.to_le_bytes()); + wav.resize(44 + data_len as usize, 0); + wav + } + + /// The `node.name` prefix a node tagged by *this* process with `role` + /// must carry. Narrower than [`super::OWNED_NODE_NAME_PREFIX`] on + /// purpose: it pins the match to this test run, so a stray tagged node + /// left by anything else cannot make a gate pass. + pub fn expected_prefix(role: &str) -> String { + format!( + "{}{role}_{}", + super::OWNED_NODE_NAME_PREFIX, + std::process::id() + ) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::collections::BTreeMap; + + /// The contract file, byte-identical to pixelpass's copy. + const FIXTURE: &str = include_str!("../../tests/fixtures/ownership-tag-contract.txt"); + + /// ⚠️ **Rejects duplicate keys, and that is the point** (Codex phase-1 + /// review, finding 3). This side collected into a map, so a duplicate + /// silently took the *last* value; pixelpass's half searches a list and + /// takes the *first*. A byte-identical fixture with a duplicated key + /// could therefore leave both suites green while the two repos had + /// selected different contracts — the exact drift this file exists to + /// prevent. Both sides now refuse the ambiguity instead of resolving it. + fn fixture() -> BTreeMap { + let mut seen: Vec<&str> = Vec::new(); + FIXTURE + .lines() + .map(str::trim) + .filter(|line| !line.is_empty() && !line.starts_with('#')) + .map(|line| { + let (key, value) = line + .split_once('=') + .unwrap_or_else(|| panic!("fixture line is not key=value: {line:?}")); + assert!( + !seen.contains(&key), + "fixture defines {key:?} twice; the two repos would disagree on which wins" + ); + seen.push(key); + (key.to_string(), value.to_string()) + }) + .collect() + } + + fn fixture_get(key: &str) -> String { + fixture() + .get(key) + .unwrap_or_else(|| panic!("fixture has no key {key:?}")) + .clone() + } + + /// Half of the cross-repo contract test: our constants are exactly what + /// the shared fixture pins. pixelpass runs the mirror of this against its + /// own byte-identical copy. + #[test] + fn contract_matches_fixture() { + assert_eq!(OWNED_PROP_KEY, fixture_get("prop_key")); + assert_eq!(OWNED_PROP_VALUE, fixture_get("prop_value")); + assert_eq!(OWNED_NODE_NAME_PREFIX, fixture_get("node_name_prefix")); + } + + /// The fixture's documented format and example must agree with the + /// generator, so a future reader can trust the example. + #[test] + fn fixture_example_is_well_formed() { + let example = fixture_get("node_name_example"); + assert!(example.starts_with(&fixture_get("node_name_prefix"))); + let format = fixture_get("node_name_format"); + assert_eq!(format, "peerspeak_owned__"); + // Same shape our generator emits: prefix, role, pid. + let generated = owned_node_name("mpv"); + let suffix = generated + .strip_prefix(OWNED_NODE_NAME_PREFIX) + .expect("generated name carries the prefix"); + let (role, pid) = suffix.split_once('_').expect("role_pid suffix"); + assert_eq!(role, "mpv"); + assert_eq!(pid.parse::().unwrap(), std::process::id()); + } + + /// The black-box half: build a real `Command` exactly as a call site + /// does, read back the environment it *would* carry across the fork, and + /// assert the property string pixelpass matches on is present in it. + /// + /// Deliberately inspects the `Command`, not our own formatter — a helper + /// that returns the right string but is wired into the wrong variable + /// would pass a formatter test and fail in the field. + #[test] + fn tagged_child_command_carries_both_carriers() { + 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 expected_name = owned_node_name("mpv"); + let prop = format!("{}={}", fixture_get("prop_key"), fixture_get("prop_value")); + + let pulse = env.get(PULSE_PROP_ENV).expect("PULSE_PROP set"); + assert!( + pulse.contains(&prop), + "PULSE_PROP carries carrier 1: {pulse}" + ); + assert!( + pulse.contains(&format!("node.name={expected_name}")), + "PULSE_PROP carries carrier 2: {pulse}" + ); + + let pw = env.get(PIPEWIRE_PROPS_ENV).expect("PIPEWIRE_PROPS set"); + assert!( + pw.contains(&format!( + "\"{}\" = \"{}\"", + fixture_get("prop_key"), + fixture_get("prop_value") + )), + "PIPEWIRE_PROPS carries carrier 1: {pw}" + ); + assert!( + pw.contains(&format!("\"node.name\" = \"{expected_name}\"")), + "PIPEWIRE_PROPS carries carrier 2: {pw}" + ); + assert!( + pw.starts_with('{') && pw.ends_with('}'), + "PIPEWIRE_PROPS is a SPA-JSON object: {pw}" + ); + } + + /// R10-2's in-process carrier, checked as a *value*, not by setting the + /// variable: mutating the process environment from a test races every + /// other test in the binary. That the value reaches `PIPEWIRE_ALSA` at all + /// is what the live gate below proves, and only a live gate can. + #[test] + fn the_alsa_carrier_value_carries_both_carriers() { + let name = owned_node_name(CLIP_ROLE); + let value = pipewire_props_value(&name); + + assert_eq!(PIPEWIRE_ALSA_ENV, "PIPEWIRE_ALSA"); + assert!( + value.contains(&format!( + "\"{}\" = \"{}\"", + fixture_get("prop_key"), + fixture_get("prop_value") + )), + "carrier 1: {value}" + ); + assert!( + value.contains(&format!("\"node.name\" = \"{name}\"")), + "carrier 2: {value}" + ); + assert!(name.starts_with(&fixture_get("node_name_prefix"))); + // The role is a distinct token, so an audit can tell clip audio from + // the call stream and from a spawned player. + assert!(name.contains("_clip_"), "role token present: {name}"); + } + + /// The in-process carrier only works if `main` actually sets it, and the + /// live gate cannot prove that half: a test binary has no `main`, so it + /// must call the tagger itself. That leaves exactly one way to regress — + /// delete the line from `main` — and this is the cheapest thing that + /// catches it. + /// + /// A source-text assertion is crude. It is also the *only* check available + /// short of driving the real GUI binary, and phase 1's miss was precisely + /// a call site nobody verified existed. + /// + /// ⚠️ **The check is "first statement in `main`", not "before + /// `run_gui`"** (round 10 review, finding 5). Ordering against `run_gui` + /// proved far less than the safety obligation needs: inserting a + /// long-lived `std::thread::spawn` above the tag still satisfied it, while + /// making the `set_var` a data race — the exact thing the `unsafe` block's + /// SAFETY comment claims cannot happen. `set_var` is sound only while the + /// process is single-threaded, so the property to gate is that **nothing + /// executable precedes the call**, which is checkable and is what `main` + /// actually does today. + #[test] + fn main_tags_this_process_before_anything_starts() { + const MAIN: &str = include_str!("../main.rs"); + + // ⚠️ Comments are stripped first, and that is not tidiness. The first + // version of this test searched the raw source and **passed against a + // `main` with the call deleted**, because the explanatory comment + // above it still named the function. A gate satisfiable by two sources + // gates neither (the phase-3r lesson); here the prose was the second + // source. Caught by mutation, which is the only reason it is not still + // green and worthless. + let code: String = MAIN + .lines() + .map(|line| line.split("//").next().unwrap_or("")) + .collect::>() + .join("\n"); + + let call = code + .find("tag_this_process_alsa_audio()") + .expect("main must call tag_this_process_alsa_audio (round 10, R10-2)"); + + let body = code + .find("fn main()") + .and_then(|start| code[start..].find('{').map(|open| start + open + 1)) + .expect("main has a body"); + // Back up over the module path the call is written with, so the + // statement's own `peerspeak::audio::ownership::` is not mistaken for + // code preceding it. Stepping by `len_utf8` rather than by one byte + // keeps the slice below on a character boundary whatever precedes it. + let call_start = code[..call] + .char_indices() + .rev() + .find(|(_, c)| !(c.is_alphanumeric() || *c == '_' || *c == ':')) + .map_or(0, |(index, c)| index + c.len_utf8()); + assert!( + call_start > body, + "the call must be inside main, not above it" + ); + + // Everything between `main`'s opening brace and the call, with the + // syntax that cannot *run* removed: attributes, the `unsafe` keyword, + // block punctuation, whitespace. Anything left is a statement that + // executes before the tag — which is what must not exist. + let mut before = code[body..call_start].to_string(); + while let Some(start) = before.find("#[") { + let end = before[start..] + .find(']') + .map(|offset| start + offset + 1) + .expect("an attribute closes"); + before.replace_range(start..end, ""); + } + let residue: String = before + .replace("unsafe", "") + .chars() + .filter(|c| !c.is_whitespace() && !matches!(c, '{' | '}')) + .collect(); + assert!( + residue.is_empty(), + "nothing may execute before the tag; found {residue:?} in main" + ); + + // And it must still be the real `main` of a program that starts the + // GUI, or the check above is asserting about the wrong function. + let gui = code + .find("run_gui") + .expect("main runs the GUI; this test's ordering check assumes it"); + assert!( + call < gui, + "the tag must be set before the GUI starts any thread" + ); + + // ⚠️ **Every search above takes the FIRST textual match, so the whole + // check is only as good as there being one candidate** (round 11 + // review, finding 4). A raw-string decoy above the real function — + // `const DECOY: &str = r#" fn main() { unsafe { …tag…(); run_gui(); } "#;` + // — would satisfy all three searches while the real `main` spawned a + // thread before the tag. Requiring uniqueness kills that class without + // pulling in a parser: if a second `fn main` or a second call ever + // appears, this fails and asks a human to look. + assert_eq!( + code.matches("fn main").count(), + 1, + "exactly one `fn main` in main.rs; the ordering check reads the first match" + ); + assert_eq!( + code.matches("tag_this_process_alsa_audio").count(), + 1, + "exactly one tag call; a second would make the ordering check ambiguous" + ); + assert_eq!( + code.matches("run_gui").count(), + 1, + "exactly one run_gui; a second would make the ordering check ambiguous" + ); + } + + /// 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, + PIPEWIRE_PROPS_ENV, + ); + + 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, + PIPEWIRE_PROPS_ENV, + ); + assert!(!merged.contains(",,"), "{merged}"); + assert!(!merged.contains(", ,"), "{merged}"); + assert!(merged.contains("\"media.role\" = \"music\""), "{merged}"); + } + + /// **The ceiling we match is the consumer's, not the grammar's** (round 12 + /// review, finding 2 — filed as P2, retracted to P3 by its author once the + /// operational boundary was measured, and the remedy it originally + /// suggested would have been a fail-open). + /// + /// `spa-json-dump -s` accepts arbitrarily deep nesting, so "agree with the + /// dump tool" would mean accepting depth 515+. Measured on this host + /// against the live ALSA plugin — the actual reader of `PIPEWIRE_ALSA` — + /// depth 515 is rejected *wholesale*: the node came back as + /// `alsa_playback.aplay` carrying no properties at all, so a merge into + /// such a value loses both carriers and echoes. Codex reached the same + /// three numbers independently by calling + /// `pw_properties_update_string_checked(NULL, …)` directly. + /// + /// | depth | `spa-json-dump` | property updater | here | + /// | --- | --- | --- | --- | + /// | 513 | accept | accept | accept | + /// | 514 | accept | accept | accept | + /// | 515 | accept | **reject** | **reject** | + /// + /// Over-rejecting is the safe direction — it costs a routing preference, + /// not a carrier — but there is no reason to sit a level below the + /// consumer when `spa_json_container_len` puts us exactly on it. + #[cfg(target_os = "linux")] + #[test] + fn the_depth_ceiling_matches_the_property_updater_not_the_grammar() { + let nested = |n: usize| format!("{{ \"x\" = {}{} }}", "[".repeat(n), "]".repeat(n)); + assert!(spa_object(&nested(1)).is_some(), "a shallow container"); + assert!( + spa_object(&nested(513)).is_some(), + "513 is accepted by both" + ); + assert!( + spa_object(&nested(514)).is_some(), + "514 is accepted by the property updater, so it must be accepted here" + ); + assert!( + spa_object(&nested(515)).is_none(), + "515 is rejected by the property updater, so accepting it would be a fail-open" + ); + assert!(spa_object(&nested(1000)).is_none(), "far past the ceiling"); + } + + /// **Round 12 review, finding 1 — a measured fail-open.** + /// + /// A trailing comment is valid SPA-JSON and ends the document, so an + /// object may close *before* the last `}` in the string. The old + /// `rfind('}')` picked the comment's brace and spliced both carriers into + /// the comment; the result still parsed, so the re-validation passed, and + /// `spa-json-dump -s` reported an object with neither carrier in it. + /// + /// The assertion is deliberately **semantic, not textual**: a carrier that + /// appears in the string but after the object's closing brace is exactly + /// the bug, so `contains` would pass on the broken output. Comparing the + /// carrier's offset against the closer libspa reports is what actually + /// distinguishes them. + #[test] + fn a_valid_trailing_comment_does_not_swallow_the_carriers() { + let name = owned_node_name("vlc"); + for existing in [ + // The brace in the comment is the one `rfind` used to find. + "{ \"target.object\" = \"my-sink\" } # trailing }", + // The same thing written the way a person actually would. + "{ \"target.object\" = \"my-sink\" }\n# trailing }", + // No brace in the comment: the carriers were fine here, but the + // comment itself was silently deleted. + "{ \"target.object\" = \"my-sink\" } # plain note", + ] { + let merged = merge_pipewire_props(Some(existing), &name, PIPEWIRE_ALSA_ENV); + let object = spa_object(&merged).expect("merged value is a valid object"); + + let owned_at = merged + .find(OWNED_PROP_KEY) + .unwrap_or_else(|| panic!("no ownership carrier at all in {merged:?}")); + assert!( + owned_at < object.closer, + "carrier sits outside the object it is supposed to tag: {merged:?}" + ); + let name_at = merged.find(&name).expect("node.name carrier present"); + assert!( + name_at < object.closer, + "node.name outside the object: {merged:?}" + ); + + // The user keeps both their routing property and their comment. + assert!( + merged.contains("\"target.object\" = \"my-sink\""), + "routing property lost: {merged:?}" + ); + assert!( + merged.contains('#'), + "the user's comment was deleted: {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_ENV), + pipewire_props_value(&name) + ); + // Not an object: overwritten rather than guessed at. + assert_eq!( + merge_pipewire_props(Some("media.role = music"), &name, PIPEWIRE_PROPS_ENV), + pipewire_props_value(&name) + ); + assert_eq!( + merge_pipewire_props(None, &name, PIPEWIRE_PROPS_ENV), + pipewire_props_value(&name) + ); + } + + /// **Round 10 review, finding 3 — the brace check was not a validity + /// check, and the gap is a measured fail-open.** + /// + /// Every value below is brace-wrapped, so the old rule spliced our pairs + /// into it and produced an object the daemon does not accept. ⚠️ The live + /// failure is worse than a rejection: measured 2026-07-25, `PIPEWIRE_ALSA` + /// set to the old merge of `{ garbage }` gave a real `aplay` node + /// `node.name=alsa_playback.aplay`, **no** `peerspeak.owned`, and a junk + /// property `garbage = "peerspeak.owned"` — the lenient parser ate our key + /// as their value and stopped, losing *both* carriers on a live + /// `Stream/Output/Audio` node. Each of these must overwrite instead. + #[test] + fn a_brace_wrapped_but_malformed_pipewire_props_is_overwritten() { + let name = owned_node_name("vlc"); + let malformed = [ + // The measured case: a key with no value. + "{ garbage }", + // A key with no value, followed by real pairs. + "{ garbage, \"media.role\" = \"music\" }", + // Unterminated string. + "{ \"media.role\" = \"music }", + // Broken nested container. + "{ \"channels\" = [ 1, 2 }", + // Mismatched closer. + "{ \"channels\" = [ 1, 2 } ]", + // Truncated object that still ends in a brace, via nesting. + "{ \"a\" = { \"b\" = 1 }", + // A trailing comment would swallow anything appended after it. + "{ \"media.role\" = \"music\" # note }", + // Two objects concatenated: splicing would have tagged only the + // second one. + "{ \"a\" = 1 } { \"b\" = 2 }", + // A dangling assignment. + "{ \"media.role\" = }", + // ⚠️ Round 11 finding 2's decisive case: garbage *inside* a nested + // container. The hand-written scanner only balanced brackets, so + // it accepted these and spliced our carriers in after an invalid + // pair — which the daemon stops at, dropping both. + "{ \"foo\" = { garbage } }", + "{ \"foo\" = [ { garbage } ] }", + "{ \"foo\" = { \"bar\" = { garbage } } }", + // Invalid escapes and non-ASCII bare tokens, also accepted before. + "{ \"a\" = \"\\é\" }", + "{ \"a\" = é }", + "{ \"a\" = foo\\bar }", + ]; + for value in malformed { + assert_eq!( + merge_pipewire_props(Some(value), &name, PIPEWIRE_PROPS_ENV), + pipewire_props_value(&name), + "{value:?} must be overwritten, not merged into" + ); + } + } + + /// The other half of finding 3: values that *are* well-formed must survive, + /// or the fix would be "overwrite everything" wearing a parser as a + /// disguise. + /// + /// ⚠️ **The user's text is preserved byte-for-byte, not re-encoded.** An + /// earlier version parsed into pairs and re-emitted them, which re-quoted + /// bare keys — and round 11 pointed out that re-quoting a token the parser + /// wrongly accepted can invent a *different* key (`foo\bar` becoming a + /// string with a `\b` escape). Splicing into validated text cannot change + /// the meaning of anything it does not touch. + /// + /// Every input here is one `spa-json-dump -s` accepts; the last three were + /// specifically rejected by the hand-written scanner this replaced. + #[test] + fn a_well_formed_pipewire_props_survives_the_merge() { + let name = owned_node_name("vlc"); + let cases = [ + // Bare key, bare value — kept bare, exactly as written. + ("{ media.role = music }", "media.role = music"), + // Colon separator. + ("{ \"media.role\": \"music\" }", "\"media.role\": \"music\""), + // No separator at all — legal SPA-JSON. + ("{ \"media.role\" \"music\" }", "\"media.role\" \"music\""), + // Nested array and object. + ( + "{ \"audio.position\" = [ FL, FR ] }", + "\"audio.position\" = [ FL, FR ]", + ), + ("{ \"props\" = { \"a\" = 1 } }", "\"props\" = { \"a\" = 1 }"), + // ⚠️ An ODD number of array elements. This is the case that + // distinguishes recursive validation from a flat token walk: a + // flat walk pairs FL with FR, then finds RC with no partner and + // wrongly rejects a valid object. Found by mutation. + ( + "{ \"audio.position\" = [ FL, FR, RC ] }", + "\"audio.position\" = [ FL, FR, RC ]", + ), + // A brace inside a string is not structure. + ( + "{ \"node.name\" = \"weird}name\" }", + "\"node.name\" = \"weird}name\"", + ), + // A leading comment line. + ( + "# mine\n{ \"media.role\" = \"music\" }", + "\"media.role\" = \"music\"", + ), + // ⚠️ Round 11 finding 3 — the hand-written scanner REJECTED these + // three valid objects, overwriting the user's routing policy. + ( + "{ target.object, \"my-sink\" }", + "target.object, \"my-sink\"", + ), + ("{ key == \"value\" }", "key == \"value\""), + ("{ \"a\" = \"b\" # note\r }", "\"a\" = \"b\""), + ]; + for (input, preserved) in cases { + let merged = merge_pipewire_props(Some(input), &name, PIPEWIRE_PROPS_ENV); + assert!( + merged.contains(preserved), + "{input:?} must keep {preserved:?}: {merged}" + ); + assert!( + merged.contains(&format!("\"{OWNED_PROP_KEY}\" = \"{OWNED_PROP_VALUE}\"")), + "{input:?} must still carry the tag: {merged}" + ); + assert!( + merged.contains(&format!("\"node.name\" = \"{name}\"")), + "{input:?} must still carry the name: {merged}" + ); + // Ours goes last, so it wins any duplicate key. + let ours = merged + .rfind("\"node.name\" = ") + .expect("a node.name pair in the merged value"); + assert!(merged[ours..].contains(&name), "ours last: {merged}"); + } + } + + /// ✅ **The exact bytes below were verified against the live daemon** + /// (2026-07-25, and the third re-verified 2026-07-26 after round 11 + /// changed what it emits): each was set as `PIPEWIRE_ALSA` for a real + /// `aplay`, and `pw-dump` showed a `Stream/Output/Audio` node carrying + /// `peerspeak.owned = 1`, the expected `node.name`, **and** the user's own + /// properties (`media.role = music`, `audio.position = [ FL, FR ]`). + /// `spa-json-dump -s` accepts all three. A comma-between-key-and-value and + /// a nested-object case were confirmed live in the same run. + /// + /// Pinned byte-for-byte rather than asserted with `contains`, because + /// what the live run establishes is that *this string* works — a + /// reformatted equivalent is a different experiment, and the failure mode + /// this whole finding is about was a value that looked fine and parsed + /// wrong. + #[test] + fn the_merged_value_is_byte_for_byte_what_was_verified_live() { + assert_eq!( + merge_pipewire_props( + Some("{ \"media.role\" = \"music\" }"), + "f3_rebuilt_probe", + PIPEWIRE_ALSA_ENV + ), + "{ \"media.role\" = \"music\", \"peerspeak.owned\" = \"1\", \ + \"node.name\" = \"f3_rebuilt_probe\" }" + ); + // The malformed case collapses to our value alone. + assert_eq!( + merge_pipewire_props( + Some("{ garbage }"), + "f3_overwritten_probe", + PIPEWIRE_ALSA_ENV + ), + "{ \"peerspeak.owned\" = \"1\", \"node.name\" = \"f3_overwritten_probe\" }" + ); + // A bare key, a bare value and a nested array all survive verbatim — + // the bare key stays bare, which is the round-11 correction. + assert_eq!( + merge_pipewire_props( + Some("{ media.role = music, \"audio.position\" = [ FL, FR ] }"), + "f3_exotic_probe", + PIPEWIRE_ALSA_ENV + ), + "{ media.role = music, \"audio.position\" = [ FL, FR ], \ + \"peerspeak.owned\" = \"1\", \"node.name\" = \"f3_exotic_probe\" }" + ); + } + + /// **Differential against `spa-json-dump -s` — a *grammar* oracle, and + /// only that.** + /// + /// This is the check whose absence caused round 11. The previous validator + /// was a hand-written scanner, and nothing compared it to the real grammar + /// — so it drifted in both directions at once and neither the unit tests + /// nor the 20,000-input property test noticed, because both used *it* as + /// their oracle. + /// + /// Now that the validator is libspa itself this should be true by + /// construction, which is exactly why it is worth asserting: if it ever + /// fails, the construction is not what we think it is. + /// + /// ⚠️ **Do not add deeply nested values to this corpus.** The dump tool + /// accepts nesting that the property updater — the code that actually + /// reads these variables — rejects, and we deliberately follow the + /// updater. A depth-515 entry here would fail this test *by design*; the + /// boundary is owned by + /// [`the_depth_ceiling_matches_the_property_updater_not_the_grammar`], + /// which is the acceptance oracle. Keeping the two apart is the point: + /// this one asks "do we read the grammar correctly", that one asks "do we + /// accept what the consumer accepts", and round 12 turned on the gap + /// between those questions. + /// + /// `#[ignore]` because it shells out to a PipeWire tool that CI need not + /// have; it is part of the phase 5 matrix procedure. + #[test] + #[ignore = "needs spa-json-dump"] + fn our_validator_agrees_with_spa_json_dump() { + let corpus = [ + // Valid. + "{ \"a\" = \"b\" }", + "{ }", + "{ media.role = music }", + "{ \"media.role\": \"music\" }", + "{ \"media.role\" \"music\" }", + "{ target.object, \"my-sink\" }", + "{ key == \"value\" }", + "{ \"a\" = \"b\",, \"c\" = \"d\" }", + "{ \"a\" = \"b\", }", + "{ \"audio.position\" = [ FL, FR ] }", + "{ \"props\" = { \"a\" = 1 } }", + "{ \"node.name\" = \"weird}name\" }", + "# mine\n{ \"media.role\" = \"music\" }", + "{ \"a\" = \"b\" # note\r }", + // Invalid. + "{ garbage }", + "{ garbage, \"media.role\" = \"music\" }", + "{ \"media.role\" = \"music }", + "{ \"channels\" = [ 1, 2 }", + "{ \"channels\" = [ 1, 2 } ]", + "{ \"a\" = { \"b\" = 1 }", + "{ \"media.role\" = \"music\" # note }", + "{ \"a\" = 1 } { \"b\" = 2 }", + "{ \"media.role\" = }", + "{ \"foo\" = { garbage } }", + "{ \"a\" = \"\\é\" }", + "{ \"a\" = é }", + "{ \"a\" = foo\\bar }", + ]; + // ⚠️ The corpus is deliberately all *objects*. `spa-json-dump` accepts + // any SPA-JSON document, so a bare `media.role = music` passes there + // and is rejected here — not a disagreement about the grammar but our + // own additional requirement, since there is no object to splice into. + // The first run of this test flagged exactly that, which is the point + // of having it. `an_empty_or_malformed_pipewire_props_yields_our_value_alone` + // pins the bare-list case. + + for value in corpus { + let mut child = std::process::Command::new("spa-json-dump") + .arg("-s") + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .spawn() + .expect("spa-json-dump must be installed for this test"); + { + use std::io::Write; + child + .stdin + .as_mut() + .expect("piped stdin") + .write_all(value.as_bytes()) + .expect("write to spa-json-dump"); + } + let theirs = child.wait().expect("spa-json-dump exits").success(); + let ours = spa_object(value).is_some(); + assert_eq!( + ours, theirs, + "disagreement on {value:?}: ours={ours}, spa-json-dump={theirs}" + ); + } + } + + /// The warning names the variable that was actually malformed. It used to + /// always say `PIPEWIRE_PROPS`, including when `tag_child` merged + /// `PIPEWIRE_ALSA` (round 10 review, finding 3's second half). + #[test] + fn the_malformed_warning_names_the_variable_it_was_given() { + // Both variables take the same grammar and go through the same merge, + // so the only thing distinguishing their diagnostics is this name + // being threaded through rather than hard-coded — which it was. + assert_ne!(PIPEWIRE_PROPS_ENV, PIPEWIRE_ALSA_ENV); + + let alsa = malformed_object_warning(PIPEWIRE_ALSA_ENV, "{ garbage }"); + assert!(alsa.contains(PIPEWIRE_ALSA_ENV), "{alsa}"); + assert!(!alsa.contains(PIPEWIRE_PROPS_ENV), "{alsa}"); + + let props = malformed_object_warning(PIPEWIRE_PROPS_ENV, "{ garbage }"); + assert!(props.contains(PIPEWIRE_PROPS_ENV), "{props}"); + // The offending value is quoted into the message, so the user can see + // what was dropped rather than only that something was. + assert!(props.contains("garbage"), "{props}"); + } + + /// 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: {alsa}" + ); + assert!(alsa.contains(&format!("\"{OWNED_PROP_KEY}\" = \"{OWNED_PROP_VALUE}\""))); + } + + /// ⚠️ In production `main` has already set `PIPEWIRE_ALSA` to this + /// process's `clip` tag by the time a player is spawned, so `tag_child` + /// merges into *that*, not into nothing. The test binary has no such + /// value, so the row above never exercises the real shape — this one does, + /// on the merge function directly. + /// + /// The child's `node.name` must come last, because last wins (measured): + /// otherwise a spawned mpv would report itself as `clip` and the audit + /// could not tell the incoming screenshare's audio from a chat clip. + #[test] + fn a_childs_role_overrides_the_inherited_clip_tag_in_the_alsa_carrier() { + let clip = pipewire_props_value(&owned_node_name(CLIP_ROLE)); + let child = owned_node_name("mpv"); + let merged = merge_pipewire_props(Some(&clip), &child, PIPEWIRE_ALSA_ENV); + + let last_name = merged + .rfind("\"node.name\" = ") + .expect("a node.name pair in the merged value"); + assert!( + merged[last_name..].contains(&child), + "the child's role must be the last node.name: {merged}" + ); + // Both are still peerspeak-owned, so the duplicate is harmless. + assert!(merged.contains(&format!("\"{OWNED_PROP_KEY}\" = \"{OWNED_PROP_VALUE}\""))); + assert!(merged.starts_with('{') && merged.ends_with('}'), "{merged}"); + assert_eq!(merged.matches('{').count(), 1, "one object only: {merged}"); + } + + #[test] + fn every_generated_name_matches_the_prefix_pixelpass_looks_for() { + for role in ["call", "mpv", "vlc", "notify"] { + let name = owned_node_name(role); + assert!( + name.starts_with(OWNED_NODE_NAME_PREFIX), + "{name} must match the prefix" + ); + } + } + + #[test] + fn role_is_reduced_to_a_single_safe_token() { + // A space would split PULSE_PROP into a bogus second property. + assert_eq!(sanitize_role("my player"), "my-player"); + assert_eq!(sanitize_role("MPV"), "mpv"); + assert_eq!(sanitize_role("vlc/../.."), "vlc"); + assert_eq!(sanitize_role(""), "child"); + assert_eq!(sanitize_role("---"), "child"); + assert!(!owned_node_name("my player").contains(' ')); + } + + #[test] + fn pulse_prop_pairs_are_whitespace_separated_and_parse_back() { + let value = pulse_prop_value(&owned_node_name("notify")); + let pairs: BTreeMap<&str, &str> = value + .split_whitespace() + .map(|pair| pair.split_once('=').expect("key=value")) + .collect(); + assert_eq!(pairs.len(), 2, "exactly two properties: {value}"); + assert_eq!(pairs.get(OWNED_PROP_KEY), Some(&OWNED_PROP_VALUE)); + assert!( + pairs + .get("node.name") + .is_some_and(|name| name.starts_with(OWNED_NODE_NAME_PREFIX)) + ); + } +} diff --git a/src/audio/pipewire_impl.rs b/src/audio/pipewire_impl.rs index c3cd319..d5b04c0 100644 --- a/src/audio/pipewire_impl.rs +++ b/src/audio/pipewire_impl.rs @@ -1,3 +1,4 @@ +use crate::audio::ownership; use crate::audio::{AudioBackend, AudioError}; use pipewire as pw; use pw::{properties::properties, spa}; @@ -371,6 +372,11 @@ fn run_playback( mainloop_clone.quit(); }); + // Ownership tag, both carriers (`crate::audio::ownership`, plan §5.1). + // This is the node that carries the far end's voice, so it is the single + // most important thing for pixelpass to refuse to fan out: sharing it + // would send the call back to the person already speaking on it. + let owned_node_name = ownership::owned_node_name(ownership::NATIVE_PLAYBACK_ROLE); let mut props = properties! { *pw::keys::MEDIA_TYPE => "Audio", *pw::keys::MEDIA_CATEGORY => "Playback", @@ -379,6 +385,19 @@ fn run_playback( // buffer — the real fix is the explicit Buffers param below — but it // expresses the intended quantum for any node that honours it. *pw::keys::NODE_LATENCY => "1024/48000", + ownership::OWNED_PROP_KEY => ownership::OWNED_PROP_VALUE, + // Set explicitly rather than relying on the stream name passed to + // `StreamBox::new` below: props win over that name, and this one has + // to be exact. + *pw::keys::NODE_NAME => owned_node_name.as_str(), + // Measured: this stream sets neither `application.name` nor a + // description, so a mixer falls back to `node.name` — which the line + // above just turned into an internal identifier. The plan's rule is + // that the ownership prefix must not reach `node.description`; a + // human label there is what keeps that rule's *intent* (mixers stay + // readable) true for our own stream, exactly as mpv's own + // description does for the spawned players. + *pw::keys::NODE_DESCRIPTION => "PeerSpeak", }; if let Some(target) = target_node { props.insert("node.target", target); @@ -637,6 +656,62 @@ mod tests { use std::time::Duration; use std::{sync::mpsc, thread}; + /// Phase-1 exit gate, native-playback half (impl plan §3): the stream + /// that carries the far end's voice appears on the graph with **both** + /// ownership carriers, and still with the `Communication` media role. + /// + /// The third and most important of the three tagged paths — this is the + /// node whose audio, if fanned out, would send the call back to whoever + /// is speaking on it. + /// + /// Feeds silence, so the gate is inaudible. Live: needs PipeWire and + /// `pw-dump`. `cargo test --lib -- --ignored native_playback` + #[test] + #[ignore = "live: requires a running PipeWire daemon and pw-dump"] + fn native_playback_node_carries_both_ownership_carriers() { + use crate::audio::ownership::{self, live_test}; + use crate::audio::{AudioBackend, PLAYBACK_TARGET_SAMPLES}; + + let backend = super::PipeWireBackend::new(); + let (tx, rx) = mpsc::channel::>(); + let ring_fill = Arc::new(AtomicUsize::new(0)); + backend + .start_playback(rx, None, ring_fill.clone()) + .expect("playback starts"); + + // Keep the ring fed so the node stays live for the whole poll; the + // stream is created on connect, but a starved one is not a fair test + // of what a real call looks like on the graph. + let feeder = thread::spawn(move || { + let silence = vec![0i16; 960 * 2]; + for _ in 0..300 { + if ring_fill.load(Ordering::Relaxed) < PLAYBACK_TARGET_SAMPLES + && tx.send(silence.clone()).is_err() + { + return; + } + thread::sleep(Duration::from_millis(20)); + } + }); + + let prefix = live_test::expected_prefix(ownership::NATIVE_PLAYBACK_ROLE); + let found = live_test::poll_for_owned_node(&prefix, Duration::from_secs(5)); + let _ = backend.stop(); + let _ = feeder.join(); + + let (name, owned) = + found.unwrap_or_else(|| panic!("no live node named {prefix:?} appeared within 5s")); + assert!( + name.starts_with(ownership::OWNED_NODE_NAME_PREFIX), + "{name}" + ); + assert_eq!( + owned.as_deref(), + Some(ownership::OWNED_PROP_VALUE), + "carrier 1 must be on the live node, not just carrier 2" + ); + } + #[test] fn requested_in_range_is_honored() { // The graph's requested quantum is produced verbatim when it fits. diff --git a/src/main.rs b/src/main.rs index 9a42670..270523c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -3,6 +3,22 @@ #![cfg_attr(not(debug_assertions), windows_subsystem = "windows")] fn main() { + // Tag the audio we play through ALSA (rodio's `ClipPlayer`: chat clips, + // peer music, local playlist tracks) so the screen-share exclusion engine + // can recognise it as ours and refuse to fan it back to the far end. + // + // First statement in the program, and that is load-bearing: this sets an + // environment variable, which is only sound while the process is still + // single-threaded, and PipeWire's ALSA plugin reads it when a stream is + // opened. See `audio::ownership::tag_this_process_alsa_audio`. + // + // SAFETY: nothing has been spawned yet, so no thread can be reading the + // environment concurrently. + #[cfg(target_os = "linux")] + unsafe { + peerspeak::audio::ownership::tag_this_process_alsa_audio() + }; + if let Err(e) = peerspeak::app::run_gui() { eprintln!("Error running GUI: {:?}", e); } diff --git a/src/notify.rs b/src/notify.rs index 1d7adcd..f968cfb 100644 --- a/src/notify.rs +++ b/src/notify.rs @@ -10,6 +10,8 @@ //! leaves a zombie. Any failure (no player, no audio) is silent by design — a //! missing chime should never disrupt a call. +#[cfg(not(windows))] +use crate::audio::ownership; use std::collections::HashMap; use std::fs::OpenOptions; use std::io::Write; @@ -264,12 +266,20 @@ fn escape_powershell_single_quoted(s: &str) -> String { #[cfg(not(windows))] fn spawn_player(path: &Path) { for player in ["pw-play", "paplay", "aplay"] { - let started = Command::new(player) + let mut command = Command::new(player); + command .arg(path) .stdin(Stdio::null()) .stdout(Stdio::null()) - .stderr(Stdio::null()) - .status(); + .stderr(Stdio::null()); + // Ownership tag (plan §5.1). A chime is short, but it is still our + // audio on the default sink, and an untagged one is an unowned root + // the exclusion engine would have to reason about from scratch. + // Measured on this host: all three fallbacks tag correctly, `aplay` + // included — it reaches the graph through PipeWire's ALSA plugin, + // which honours `PIPEWIRE_PROPS` like any other client. + ownership::tag_child(&mut command, ownership::NOTIFICATION_ROLE); + let started = command.status(); // `status()` errors only if the player binary isn't present; on a real // playback error it still returns (non-zero), so a started player ends // the loop either way — we don't want to double-play through fallbacks. @@ -313,6 +323,49 @@ mod tests { dir } + /// Phase-1 exit gate, notification half (impl plan §3): a chime peerspeak + /// actually plays produces a live PipeWire node carrying **both** + /// ownership carriers. + /// + /// ⚠️ Deliberately drives `play()`, not `tag_child()`. The unit test in + /// `audio::ownership` proves the environment is built correctly; only a + /// live run proves this module *uses* it and that the audio stack honours + /// it end to end. The chime is silent (a zero-filled WAV), so running it + /// never makes noise. + /// + /// Live: needs a running PipeWire daemon, `pw-play`/`paplay` and + /// `pw-dump`. `cargo test --lib -- --ignored notification_chime` + #[test] + #[ignore = "live: requires a running PipeWire daemon and pw-dump"] + #[cfg(not(windows))] + fn notification_chime_node_carries_both_ownership_carriers() { + use crate::audio::ownership::{self, live_test}; + + let dir = temp_wav_dir("ownership"); + let path = dir.join("silence.wav"); + std::fs::write(&path, live_test::silent_wav(6)).unwrap(); + + set_enabled(true); + set_sound_enabled(Sound::PeerJoin, true); + play(Sound::PeerJoin, Some(path.to_str().unwrap())); + + let prefix = live_test::expected_prefix(ownership::NOTIFICATION_ROLE); + let found = live_test::poll_for_owned_node(&prefix, std::time::Duration::from_secs(5)); + std::fs::remove_dir_all(&dir).ok(); + + let (name, owned) = + found.unwrap_or_else(|| panic!("no live node named {prefix:?} appeared within 5s")); + assert!( + name.starts_with(ownership::OWNED_NODE_NAME_PREFIX), + "{name}" + ); + assert_eq!( + owned.as_deref(), + Some(ownership::OWNED_PROP_VALUE), + "carrier 1 must be on the live node too, not just carrier 2" + ); + } + #[test] fn test_should_play_truth_table() { // Plays only when BOTH the master and the per-sound flag are on. diff --git a/src/screenshare/mod.rs b/src/screenshare/mod.rs index 2b7a247..839a6fe 100644 --- a/src/screenshare/mod.rs +++ b/src/screenshare/mod.rs @@ -21,6 +21,8 @@ use std::time::Duration; use tokio::io::{AsyncBufReadExt, BufReader}; use tokio::process::{Child, Command}; +use crate::audio::ownership; + pub mod livesync; use crate::config::{ScreenShareSettings, ShareBuffering, SharePlayer, ShareQuality}; @@ -765,20 +767,69 @@ fn spawn_player(bin: &str, args: &[String], url: &str) -> std::io::Result // and is not needed to verify the flags. Logged on each attempt, so a // fallback from the preferred player to the other one is visible too. crate::log_msg(&format!("player spawn: {bin} {}", args.join(" "))); - Command::new(bin) + let mut command = Command::new(bin); + command .args(args) .arg(url) .stdin(Stdio::null()) .stdout(Stdio::null()) .stderr(Stdio::null()) - .kill_on_drop(false) - .spawn() + .kill_on_drop(false); + // Ownership tag (plan §5.1): this player is playing the *incoming* + // screenshare's audio, so it is exactly what must not be fanned back out + // if this machine also starts sharing. The role is the player binary, so + // a `pw-dump` during a field test names which one produced the node. + ownership::tag_child(command.as_std_mut(), bin); + command.spawn() } #[cfg(test)] mod tests { use super::*; + /// Phase-1 exit gate, player half (impl plan §3): the mpv peerspeak + /// actually spawns produces a live node carrying **both** ownership + /// carriers, tagged with the player's own name as the role. + /// + /// ⚠️ Drives the real [`spawn_player`], for the same reason the notify + /// gate does: the plan requires the tag to be shown "landing on a live + /// mpv node, not just in the env". Plays a silent WAV, so it is quiet. + /// + /// Live: needs PipeWire, `mpv` and `pw-dump`. + /// `cargo test --lib -- --ignored spawned_player` + #[tokio::test] + #[ignore = "live: requires a running PipeWire daemon, mpv and pw-dump"] + async fn spawned_player_node_carries_both_ownership_carriers() { + use crate::audio::ownership::live_test; + + let dir = std::env::temp_dir().join(format!("peerspeak-playertest-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("silence.wav"); + std::fs::write(&path, live_test::silent_wav(6)).unwrap(); + + let mut child = spawn_player( + "mpv", + &["--no-video".to_string(), "--really-quiet".to_string()], + path.to_str().unwrap(), + ) + .expect("mpv spawns"); + + // The role is the player binary, so this also pins that the call site + // passes `bin` and not a fixed literal. + let prefix = live_test::expected_prefix("mpv"); + let found = live_test::poll_for_owned_node(&prefix, std::time::Duration::from_secs(5)); + let _ = child.kill().await; + std::fs::remove_dir_all(&dir).ok(); + + let (name, owned) = + found.unwrap_or_else(|| panic!("no live node named {prefix:?} appeared within 5s")); + assert!( + name.starts_with(ownership::OWNED_NODE_NAME_PREFIX), + "{name}" + ); + assert_eq!(owned.as_deref(), Some(ownership::OWNED_PROP_VALUE)); + } + #[test] fn viewer_args_guard_neutralizes_flag_like_ticket() { // A malicious "ticket" that looks like a flag must end up positional, diff --git a/tests/fixtures/ownership-tag-contract.txt b/tests/fixtures/ownership-tag-contract.txt new file mode 100644 index 0000000..fa1b3ef --- /dev/null +++ b/tests/fixtures/ownership-tag-contract.txt @@ -0,0 +1,42 @@ +# Screenshare audio exclusion — ownership tagging wire contract. +# +# peerspeak PRODUCES these carriers on every audio node it owns; pixelpass +# CONSUMES them as the primary taint root of the exclusion engine. Neither +# repo depends on the other, so this file is the contract: it is committed +# byte-identical in both, and each repo has a test that asserts its own named +# constants (and, on the producer side, the environment a real child Command +# would carry) match these values exactly. +# +# peerspeak/tests/fixtures/ownership-tag-contract.txt +# pixelpass/tests/fixtures/ownership-tag-contract.txt +# +# Pinned by peerspeak docs/screenshare-audio-exclusion-impl-plan.md §3 and +# docs/screenshare-audio-exclusion-plan.md §5.1 (v3.5). Changing a value here +# is a cross-repo breaking change: both repos must land in the same session, +# and the phase 5 matrix must be re-run. +# +# Two carriers, matched as a UNION — a node is peerspeak-owned if EITHER +# matches. Round 8 added the second because a property is invisible to the +# PipeWire registry `global` event and readable only via a node bind, so the +# primary taint root must not rest on one observation mechanism alone. + +# Carrier 1 — a node property, matched EXACTLY: `prop_value` below is the +# ONLY spelling the consumer reads as owned. A producer emitting "true", "yes" +# or "" is NOT owned on this carrier, and only carrier 2 would still catch it. +# +# ⚠️ This wording is load-bearing and it CHANGED in round 10. The consumer +# used to accept any value other than "false"/"0", on the theory that leniency +# over-excludes and is therefore safe. It is not: leniency buys false-positive +# exclusion, and it let any process suppress a rival application's audio from +# the share with a property it did not even have to spell right. Fail-closed +# on this feature is about ANCESTRY — an unresolvable graph is not eligible — +# not about parsing. +prop_key=peerspeak.owned +prop_value=1 + +# Carrier 2 — a `node.name` prefix, announced by the registry without a bind. +# `node.description` is deliberately NOT touched, so mixers still show "mpv". +# Only the prefix is matched; the rest of the name is for diagnostics. +node_name_prefix=peerspeak_owned_ +node_name_format=peerspeak_owned__ +node_name_example=peerspeak_owned_mpv_31284