diff --git a/src/host/audit/mod.rs b/src/host/audit/mod.rs index 996805a..b76dfc9 100644 --- a/src/host/audit/mod.rs +++ b/src/host/audit/mod.rs @@ -198,6 +198,17 @@ pub struct TaintRow { pub sticky: bool, } +/// A node carrying a peerspeak ownership carrier on a role the engine does not +/// honour it on (round 10, R10-1). `role` is the point of the row: it says +/// which non-producer role the tag turned up on, which is what distinguishes a +/// producer-side bug from an impersonation attempt. +#[derive(Clone, Debug, PartialEq, Eq, Serialize)] +pub struct IgnoredTagRow { + pub serial: u64, + pub name: Option, + pub role: &'static str, +} + /// The decision content of one recompute — everything except which recompute it /// was. Split out from [`AuditRecord`] so "did anything actually change?" is a /// derived `==` rather than a hand-maintained field comparison that a later @@ -227,6 +238,16 @@ pub struct AuditBody { pub excluded_count: usize, /// Taint across all node roles, ascending by serial. pub taint: Vec, + /// Nodes carrying a peerspeak ownership carrier that the engine + /// **ignored** because they are not `Stream/Output/Audio` (round 10, + /// R10-1). Normally empty; a non-empty list means either peerspeak is + /// tagging something it should not, or a process is impersonating the + /// tag. Neither is an exclusion, and neither should be silent. + /// + /// Omitted from the JSONL when empty, so it costs nothing on the common + /// path and is impossible to miss when it is not. + #[serde(skip_serializing_if = "Vec::is_empty")] + pub ignored_ownership_tags: Vec, } impl AuditBody { @@ -433,6 +454,16 @@ fn build_body( }) .collect(); + let ignored_ownership_tags: Vec = + crate::host::taint::misplaced_ownership_tags(&projection.snapshot) + .into_iter() + .map(|node| IgnoredTagRow { + serial: node.serial.0, + name: node.name.clone(), + role: node.role.code(), + }) + .collect(); + AuditBody { graph_ready: projection.graph_ready, epoch: readiness_code(projection.readiness), @@ -444,6 +475,7 @@ fn build_body( eligible_count, candidates, taint, + ignored_ownership_tags, } } diff --git a/src/host/audit/tests.rs b/src/host/audit/tests.rs index ce254df..80fc890 100644 --- a/src/host/audit/tests.rs +++ b/src/host/audit/tests.rs @@ -22,9 +22,17 @@ use super::metrics::{BUCKET_LABELS, Metrics, QUEUE_THRESHOLD_US, Sample}; use super::*; use crate::host::aec::AecConfig; use crate::host::observer::{EventKind, Readiness}; +use crate::host::taint::PEERSPEAK_OWNED_NODE_PREFIX; use crate::host::taint::fixture::{self, Graph, NodeRef}; use crate::host::taint::snapshot::{GraphSnapshot, MediaRole}; +/// The `node.name` a [`Graph::peerspeak_node`] fixture produces. Built from +/// the same constant the engine matches on, so these audit rows report the +/// name shape a live peerspeak node actually has (v3.5 §5.1, carrier 2). +fn owned_name(role: &str, pid: u32) -> String { + format!("{PEERSPEAK_OWNED_NODE_PREFIX}{role}_{pid}") +} + const AEC_MODULE: u64 = 7; const TIMEOUT: Millis = 5_000; @@ -117,6 +125,58 @@ fn the_record_carries_the_complete_candidate_universe() { assert_eq!(outcome.record.body.excluded_count, 0); } +/// **R10-1's diagnostic reaches the record.** The engine deliberately ignores +/// an ownership carrier on a non-producer, which means the fix removes an +/// exclusion — so the only way an operator learns a tag was seen and dropped is +/// this field. A matrix row that silently grew an impostor would otherwise read +/// as a clean pass. +#[test] +fn an_ignored_ownership_tag_is_reported_without_excluding_anything() { + let mut graph = Graph::new(); + graph.app_node("music", MediaRole::StreamOutput, 100); + let impostor = graph.peerspeak_tagged_node("rogue", MediaRole::StreamInput, 4_242); + let projection = ready(graph.build()); + + let body = observe(&mut auditor_off(), &projection, 0).record.body; + + // The bystander is untouched — the point of the fix. + let (eligible, excluded) = partition(&body); + assert_eq!(eligible, vec!["music"]); + assert!(excluded.is_empty(), "unexpected exclusions: {excluded:?}"); + assert!(body.taint.is_empty(), "unexpected taint: {:?}", body.taint); + + // ...but the tag is not silent, and the row names the role it appeared on. + assert_eq!(body.ignored_ownership_tags.len(), 1); + let row = &body.ignored_ownership_tags[0]; + assert_eq!(row.serial, impostor.serial.0); + assert_eq!(row.role, "stream-input"); + assert_eq!( + row.name.as_deref(), + Some(owned_name("rogue", 4_242).as_str()) + ); +} + +/// The common path stays quiet: a correctly tagged peerspeak producer is +/// honoured as a taint root and is *not* reported as a misplaced tag. Without +/// this, a diagnostic that fired on every normal run would be worthless. +#[test] +fn a_correctly_tagged_producer_is_not_reported_as_misplaced() { + let mut graph = Graph::new(); + let sink = graph.device_node("speakers", MediaRole::Sink); + let call = graph.peerspeak_node("call", 200); + graph.link(call, sink); + let projection = ready(graph.build()); + + let body = observe(&mut auditor_off(), &projection, 0).record.body; + + assert_eq!(body.excluded_count, 1); + assert!( + body.ignored_ownership_tags.is_empty(), + "honoured tag reported as misplaced: {:?}", + body.ignored_ownership_tags + ); +} + /// The fail-closed default asserted at the boundary (impl plan §4, phase 2's /// "one addition"): nothing in, nothing eligible — and, just as importantly, no /// panic and no invented row. @@ -284,13 +344,14 @@ fn a_shut_gate_preserves_the_engines_own_reasons() { .body; let (_, excluded) = partition(&body); + let playback = owned_name("peerspeak-playback", 200); assert!(!body.fan_out_permitted); assert_eq!( excluded, vec![ ("music", "aec-validating"), // Tagged, so it keeps the reason that actually applies to it. - ("peerspeak-playback", "peerspeak-owned"), + (playback.as_str(), "peerspeak-owned"), ] ); } @@ -457,11 +518,12 @@ fn row_1_owner_bridge_forwarder_with_an_untainted_control() { .body; let (eligible, excluded) = partition(&body); + let call_name = owned_name("peerspeak-call", 200); assert_eq!(eligible, vec!["clean-loopback-playback"]); assert_eq!( excluded, vec![ - ("peerspeak-call", "peerspeak-owned"), + (call_name.as_str(), "peerspeak-owned"), ("tainted-loopback-playback", "tainted-owner-bridge"), ] ); @@ -494,11 +556,12 @@ fn row_3_one_tainted_module_does_not_taint_the_other() { .body; let (eligible, excluded) = partition(&body); + let call_name = owned_name("peerspeak-call", 200); assert_eq!(eligible, vec!["module-b-playback"]); assert_eq!( excluded, vec![ - ("peerspeak-call", "peerspeak-owned"), + (call_name.as_str(), "peerspeak-owned"), ("module-a-playback", "tainted-owner-bridge"), ] ); @@ -701,7 +764,8 @@ fn the_taint_view_covers_non_candidate_roles() { tainted.contains(&("null-sink", "tainted-upstream")), "the sink is not a candidate but its taint is what explains the row: {tainted:?}" ); - assert!(tainted.contains(&("peerspeak-call", "peerspeak-owned"))); + let call_name = owned_name("peerspeak-call", 200); + assert!(tainted.contains(&(call_name.as_str(), "peerspeak-owned"))); } /// A record must serialise to a single line. Newlines inside a JSON Lines diff --git a/src/host/observer/adapter.rs b/src/host/observer/adapter.rs index f0e31ee..85a41c7 100644 --- a/src/host/observer/adapter.rs +++ b/src/host/observer/adapter.rs @@ -12,6 +12,7 @@ use crate::host::audio::parse_object_serial; use crate::host::taint::snapshot::{ ClientSnapshot, GlobalId, MediaRole, NodeProps, PortDirection, PortSnapshot, Serial, }; +use crate::host::taint::{PEERSPEAK_OWNED_PROP, PEERSPEAK_OWNED_VALUE}; use anyhow::{Context, Result}; use pipewire::{self as pw, types::ObjectType}; use std::cell::{Cell, RefCell}; @@ -648,16 +649,44 @@ fn parse_serial(id: u32, kind: &str, raw: Option<&str>) -> Option { } } +/// Lenient boolean for PipeWire's own `bool`-ish properties +/// (`port.exclusive`, `port.monitor`, `node.passthrough`), whose spelling +/// varies by producer. Leniency is the fail-closed direction *for these*: +/// each one, when true, causes exclusion. fn truthy(value: Option<&str>) -> bool { value.is_some_and(|value| value != "false" && value != "0") } +/// The ownership carrier is matched **exactly**, not leniently (round 10, +/// R10-4). +/// +/// It is tempting to reuse [`truthy`] here on the grounds that treating an +/// unexpected value as "owned" over-excludes and is therefore safe. That +/// argument does not hold: leniency buys false-positive *exclusion*, not +/// safety. Under `truthy`, `peerspeak.owned=""` and `peerspeak.owned=false ` +/// (trailing space) both mean owned, so any process can suppress a rival's +/// audio from the share with a property it does not even have to spell right. +/// +/// Fail-closed on this feature is about **ancestry** — an unresolvable graph +/// is not eligible — not about parsing. The producer emits exactly +/// [`PEERSPEAK_OWNED_VALUE`] at all three of its sites and is pinned to it by +/// the shared cross-repo fixture, so there is no real value to be lenient +/// about. And a missed tag is not silent: carrier 2 is a union with this one, +/// so a garbled property still leaves the `node.name` prefix. +fn peerspeak_owned(value: Option<&str>) -> bool { + value == Some(PEERSPEAK_OWNED_VALUE) +} + fn node_observation_from_props(props: &pw::spa::utils::dict::DictRef) -> NodeObservation { NodeObservation { name: props.get("node.name").map(str::to_string), role: MediaRole::parse(props.get("media.class")), props: NodeProps { - peerspeak_owned: truthy(props.get("peerspeak.owned")), + // Carrier 1 only. Carrier 2 (the `node.name` prefix) is matched + // in the engine off `NodeObservation::name` above, so each + // carrier stays independently testable — see + // [`crate::host::taint::PEERSPEAK_OWNED_NODE_PREFIX`]. + peerspeak_owned: peerspeak_owned(props.get(PEERSPEAK_OWNED_PROP)), pulse_module_id: props .get("pulse.module.id") .and_then(|value| value.parse::().ok()), @@ -717,6 +746,165 @@ mod tests { use super::*; use std::process::Command; + /// **R10-4.** The ownership carrier is matched exactly; the lenient + /// [`truthy`] spelling is wrong for it. + /// + /// Under `truthy`, every value in `denied` below meant "peerspeak owns + /// this" — including the empty string and a `false` with a trailing space + /// — so any process could suppress a rival application's audio from the + /// share with a property it did not have to spell correctly. Leniency here + /// buys false-positive exclusion, not safety. + #[test] + fn the_ownership_carrier_is_matched_exactly_not_leniently() { + assert!(peerspeak_owned(Some(PEERSPEAK_OWNED_VALUE))); + + let denied = [ + None, + Some(""), + Some("false"), + Some("0"), + Some("false "), + Some("true"), + Some("yes"), + Some("1 "), + Some(" 1"), + Some("01"), + Some("2"), + ]; + for value in denied { + assert!( + !peerspeak_owned(value), + "{value:?} must not read as peerspeak-owned" + ); + } + } + + /// The same property, asserted through the **production wiring** rather + /// than the helper. + /// + /// ⚠️ **This is the gate; the one above is a unit test of a private + /// function** (round 10 review, finding 4). Mutating + /// [`node_observation_from_props`] back to `truthy(props.get(…))` left the + /// helper test green, because it calls [`peerspeak_owned`] directly and + /// the only live case it shares with production — exact `"1"` — passes + /// under both implementations. That is precisely the "a gate satisfiable + /// by two sources gates neither" failure that bit the `main.rs` wiring + /// guard and phase 3r row 1. + /// + /// So: build a real `pw_properties` dictionary, push it through the same + /// function the registry callback calls, and assert the resulting + /// [`NodeProps::peerspeak_owned`] for every spelling. + #[test] + fn the_production_wiring_reads_the_ownership_carrier_exactly() { + pw::init(); + + // (property value, must be read as peerspeak-owned) + let spellings = [ + (Some(PEERSPEAK_OWNED_VALUE), true), + (None, false), + (Some(""), false), + (Some("false"), false), + (Some("0"), false), + (Some("false "), false), + (Some("true"), false), + (Some("yes"), false), + (Some("1 "), false), + (Some(" 1"), false), + (Some("01"), false), + (Some("2"), false), + ]; + + for (value, expected) in spellings { + let mut props = pw::properties::PropertiesBox::new(); + // A realistic node, so the rest of the parse runs too: this is the + // shape peerspeak's own tagged playback arrives in. + props.insert("media.class", "Stream/Output/Audio"); + props.insert("node.name", "probe"); + props.insert("client.id", "42"); + if let Some(value) = value { + props.insert(PEERSPEAK_OWNED_PROP, value); + } + + let observation = node_observation_from_props(props.dict()); + assert_eq!( + observation.props.peerspeak_owned, expected, + "{PEERSPEAK_OWNED_PROP}={value:?} through the real adapter" + ); + // The surrounding parse must still work, or a green result above + // could just mean the whole dictionary was dropped. + assert_eq!(observation.role, MediaRole::StreamOutput); + assert_eq!(observation.name.as_deref(), Some("probe")); + assert_eq!(observation.props.client_id, Some(GlobalId(42))); + } + } + + /// The cross-repo fixture's `prop_value` is the only spelling this + /// consumer treats as owned — asserted through the production wiring. + /// + /// The taint module's `ownership_carriers_match_the_cross_repo_fixture` + /// proves the two repos agree on the *literal*. That is not the same as + /// proving the shipping observer reads it, which is the half the round-10 + /// review's finding 6 was about: a future producer following the fixture + /// needs the file to describe what the code does, and only a test that + /// runs the code can keep those two honest. + #[test] + fn the_fixture_value_is_the_only_owned_spelling() { + const FIXTURE: &str = include_str!("../../../tests/fixtures/ownership-tag-contract.txt"); + pw::init(); + + let pinned = FIXTURE + .lines() + .map(str::trim) + .filter(|line| !line.is_empty() && !line.starts_with('#')) + .map(|line| line.split_once('=').expect("fixture line is key=value")); + let mut prop_key = None; + let mut prop_value = None; + for (key, value) in pinned { + match key { + "prop_key" => prop_key = Some(value), + "prop_value" => prop_value = Some(value), + _ => {} + } + } + let prop_key = prop_key.expect("fixture defines prop_key"); + let prop_value = prop_value.expect("fixture defines prop_value"); + + let observe = |value: &str| { + let mut props = pw::properties::PropertiesBox::new(); + props.insert("media.class", "Stream/Output/Audio"); + props.insert(prop_key, value); + node_observation_from_props(props.dict()) + .props + .peerspeak_owned + }; + + assert!( + observe(prop_value), + "the fixture's own {prop_key}={prop_value} must read as owned" + ); + // The spellings the fixture explicitly says are NOT owned. + for denied in ["true", "yes", ""] { + assert!( + !observe(denied), + "{prop_key}={denied:?} must not read as owned; the fixture says so" + ); + } + } + + /// The other three boolean properties keep the lenient spelling, and that + /// is deliberate rather than an oversight: each is PipeWire's own, each + /// varies by producer, and each causes *exclusion* when true — so reading + /// an unrecognised value as true is genuinely the safe direction for them. + #[test] + fn pipewires_own_boolean_props_stay_lenient() { + assert!(truthy(Some("true"))); + assert!(truthy(Some("1"))); + assert!(truthy(Some(""))); + assert!(!truthy(Some("false"))); + assert!(!truthy(Some("0"))); + assert!(!truthy(None)); + } + struct PactlModule { id: Option, } @@ -893,7 +1081,15 @@ mod tests { "module-null-sink", &[ format!("sink_name={unique}"), - "sink_properties=peerspeak.owned=true node.passthrough=true".to_string(), + // The value peerspeak actually emits, not merely a truthy one: + // this row is the live proof that carrier 1 survives the bind, + // and the sink's name deliberately does *not* carry the + // `peerspeak_owned_` prefix, so carrier 2 cannot stand in for + // it here. + format!( + "sink_properties={PEERSPEAK_OWNED_PROP}={} node.passthrough=true", + crate::host::taint::PEERSPEAK_OWNED_VALUE + ), ], ); let null_sink_id = null_sink.id.expect("null-sink module must have an id"); diff --git a/src/host/taint/fixture.rs b/src/host/taint/fixture.rs index ec3f2bd..d0c2647 100644 --- a/src/host/taint/fixture.rs +++ b/src/host/taint/fixture.rs @@ -36,6 +36,10 @@ pub struct Graph { /// (GStreamer opens one per stream) pass clients explicitly instead. client_by_app: BTreeMap, client_by_module: BTreeMap, + /// Native (non-Pulse-emulated) clients, whose `pipewire.sec.pid` is the + /// app's **own** pid rather than pipewire-pulse's. See + /// [`Graph::native_client_node`]. + native_client_by_app: BTreeMap, session_client: Option, } @@ -84,6 +88,35 @@ impl Graph { id } + /// A **native PipeWire** client's stream: `client.id` on the node, **no + /// `application.process.id`**, and the app's real pid only on the Client + /// as `pipewire.sec.pid`. + /// + /// ⚠️ This is what an ordinary app actually looks like when it does not go + /// through pipewire-pulse — measured for mpv on its default ao and for + /// peerspeak's own playback stream. [`Graph::app_node`] models the + /// Pulse-emulated shape, where the pid is on the node and the Client's + /// `sec_pid` is the *daemon's*; both shapes are live on this host, and + /// only this one exercises key 4's Client fallback (round 10, R10-3). + pub fn native_client_node(&mut self, name: &str, role: MediaRole, pid: u32) -> NodeRef { + let client = match self.native_client_by_app.get(&pid) { + Some(id) => *id, + None => { + let id = self.client(Some(pid)); + self.native_client_by_app.insert(pid, id); + id + } + }; + self.node( + name, + role, + NodeProps { + client_id: Some(client), + ..NodeProps::default() + }, + ) + } + /// An ordinary application stream: its own client, its own PID. pub fn app_node(&mut self, name: &str, role: MediaRole, pid: u32) -> NodeRef { let client = self.client_of_app(pid); @@ -152,11 +185,43 @@ impl Graph { self.node(name, role, app(client, pid)) } + /// A peerspeak-owned node carrying **both** ownership carriers, as a + /// live one does. `name` gets the real `node.name` prefix so the fixture + /// cannot pass on the property alone. pub fn peerspeak_node(&mut self, name: &str, pid: u32) -> NodeRef { + let client = self.client_of_app(pid); + let name = format!("{}{name}_{pid}", super::PEERSPEAK_OWNED_NODE_PREFIX); + self.node(&name, MediaRole::StreamOutput, peerspeak_owned(client, pid)) + } + + /// Both ownership carriers on a node of **any** role — an impostor, or a + /// producer-side tagging bug. Only [`MediaRole::StreamOutput`] makes it a + /// taint root (round 10, R10-1); every other role must be ignored, and + /// these are the fixtures that prove it. + pub fn peerspeak_tagged_node(&mut self, name: &str, role: MediaRole, pid: u32) -> NodeRef { + let client = self.client_of_app(pid); + let name = format!("{}{name}_{pid}", super::PEERSPEAK_OWNED_NODE_PREFIX); + self.node(&name, role, peerspeak_owned(client, pid)) + } + + /// Carrier 1 alone: the `peerspeak.owned` property present, the + /// `node.name` prefix absent. What the engine sees for a node it had to + /// bind to observe (v3.5 §6.7). + pub fn peerspeak_node_prop_only(&mut self, name: &str, pid: u32) -> NodeRef { let client = self.client_of_app(pid); self.node(name, MediaRole::StreamOutput, peerspeak_owned(client, pid)) } + /// Carrier 2 alone: the `node.name` prefix present, the property absent + /// — indistinguishable from an ordinary app in every other respect. + /// This is the case that survives the F1 observation defect, and the + /// reason round 8 added a second carrier at all. + pub fn peerspeak_node_name_only(&mut self, role: &str, pid: u32) -> NodeRef { + let client = self.client_of_app(pid); + let name = format!("{}{role}_{pid}", super::PEERSPEAK_OWNED_NODE_PREFIX); + self.node(&name, MediaRole::StreamOutput, app(client, pid)) + } + pub fn node(&mut self, name: &str, role: MediaRole, props: NodeProps) -> NodeRef { let id = self.id(); self.node_with_id(name, role, id, props) diff --git a/src/host/taint/mod.rs b/src/host/taint/mod.rs index 9ee8b9c..a9e1054 100644 --- a/src/host/taint/mod.rs +++ b/src/host/taint/mod.rs @@ -123,6 +123,44 @@ pub const CAPTURE_SINK_PREFIX: &str = "pixelpass_capture_"; /// what `pulse.module.id` is for (v3.4 §5.2 correction 4). pub const ECHO_CANCEL_GROUP_PREFIX: &str = "echo-cancel-"; +/// Ownership carrier 1: the node property peerspeak sets on everything it +/// plays (v3.5 §5.1). Read at the observer boundary, which is the only place +/// that touches raw property names — see [`super::observer`]. +/// +/// ⚠️ **Cross-repo wire contract.** peerspeak emits this; it does not depend +/// on this crate, nor this crate on it. The values are pinned in +/// `tests/fixtures/ownership-tag-contract.txt`, committed byte-identical in +/// both repos, and asserted by [`tests::ownership_carriers_match_the_cross_repo_fixture`]. +/// The producer's matching constants live in peerspeak +/// `src/audio/ownership.rs`. Changing either is a both-repos-same-session +/// change that invalidates the phase 5 matrix. +pub const PEERSPEAK_OWNED_PROP: &str = "peerspeak.owned"; + +/// The value peerspeak emits for [`PEERSPEAK_OWNED_PROP`], and the **only** +/// value this consumer reads as owned. +/// +/// ⚠️ This doc used to say the opposite — that any truthy value counted, on +/// the theory that treating an unexpected value as "owned" is the fail-closed +/// direction. R10-4 removed that leniency and the round-10 review caught the +/// prose surviving it here and in the shared fixture. The theory is wrong: +/// leniency buys false-positive *exclusion*, not safety, and it let any +/// process suppress a rival application's audio from the share with a +/// property it did not have to spell right. Fail-closed on this feature is +/// about **ancestry** — an unresolvable graph is not eligible — not about +/// parsing. The matching lives in the observer's `peerspeak_owned`, which is +/// deliberately *not* the lenient `truthy` used for PipeWire's own booleans. +pub const PEERSPEAK_OWNED_VALUE: &str = "1"; + +/// Ownership carrier 2: a `node.name` prefix (v3.5 §5.1, round 8). +/// +/// Matched as a **union** with [`PEERSPEAK_OWNED_PROP`] — either one makes a +/// node peerspeak-owned. Two carriers because a property is invisible to the +/// registry `global` event and recoverable only by binding the node (v3.5 +/// §6.7), which is precisely how the phase-5 gate failed; this one is +/// announced directly. A union is also the fail-closed direction: a missed +/// tag leaks call audio into the share, a spurious one only over-excludes. +pub const PEERSPEAK_OWNED_NODE_PREFIX: &str = "peerspeak_owned_"; + /// Why a node is tainted or excluded. Stable machine-readable codes: this /// value is the phase 5 audit output, the phase 6 status event, and the /// eventual answer to "why isn't this app being shared?". @@ -378,8 +416,13 @@ pub fn evaluate( ctx: &ExclusionCtx, prior: &StickyState, ) -> (Decisions, StickyState) { - let components = OwnerComponents::build(snapshot, ctx.pipewire_pulse_pid); - let keys = owner::OwnerKeyIndex::build(snapshot, ctx.pipewire_pulse_pid); + // Built once and shared: it carries the Client → `pipewire.sec.pid` index + // that key 4 falls back to (round 10, R10-3), so the components and the + // key index must be derived from the *same* one or they would disagree + // about which nodes are bounded. + let owner_ctx = owner::OwnerCtx::new(snapshot, ctx.pipewire_pulse_pid); + let components = OwnerComponents::build(snapshot, &owner_ctx); + let keys = owner::OwnerKeyIndex::build(snapshot, &owner_ctx); // Pass 1 — the fail-closed view. Every decision is made from this one, so // "we could not see" counts as taint. @@ -534,8 +577,59 @@ fn ambiguous_id_nodes(snapshot: &GraphSnapshot) -> BTreeSet { .collect() } +/// Does this node carry either ownership carrier? **Tag presence only** — it +/// deliberately says nothing about whether the tag is honoured, which is +/// `local_root_reason`'s business (round 10 restricts that to producers). +/// Split out so the "is it tagged?" and "does the tag count?" questions can +/// be tested, and reported, independently. +pub fn is_peerspeak_tagged(node: &NodeSnapshot) -> bool { + node.props.peerspeak_owned + || node + .name + .as_deref() + .is_some_and(|name| name.starts_with(PEERSPEAK_OWNED_NODE_PREFIX)) +} + +/// Nodes carrying an ownership carrier that `local_root_reason` **ignored** +/// because the node is not a producer (round 10, R10-1). Ascending by serial. +/// +/// Purely diagnostic — nothing in the engine consumes it. It exists because +/// R10-1 turns a formerly load-bearing tag into a no-op, and a silently +/// ignored tag has exactly two causes, both of which someone wants to know +/// about: peerspeak tagging a node it should not (a producer-side bug this +/// would otherwise hide), or another process impersonating the tag (the F2 +/// attack, now defanged but still worth seeing). +pub fn misplaced_ownership_tags(snapshot: &GraphSnapshot) -> Vec<&NodeSnapshot> { + let mut tagged: Vec<&NodeSnapshot> = snapshot + .nodes() + .filter(|node| node.role != MediaRole::StreamOutput && is_peerspeak_tagged(node)) + .collect(); + tagged.sort_by_key(|node| node.serial); + tagged +} + fn local_root_reason(node: &NodeSnapshot, ctx: &ExclusionCtx) -> Option { - if node.props.peerspeak_owned { + // The two ownership carriers, as a union (v3.5 §5.1). Kept here rather + // than folded together at the observer boundary so that the union is a + // pure, directly-testable rule: an adapter that collapsed both into the + // one `peerspeak_owned` bool would make each carrier untestable alone, + // which is exactly how phase 3r's row 1 nearly gated nothing. + // + // ⚠️ **Producer roles only** (round 10, R10-1). Neither carrier is a + // security boundary — both are strings any unprivileged process can put + // on its own node — so an unrestricted root is a denial of the whole + // feature: an unlinked `Stream/Input/Audio` named `peerspeak_owned_x` + // is a tainted *reader* with no owner bound to it, which fails every + // candidate closed machine-wide (Codex phase-1 F2, reproduced live). + // Restricting the root to `Stream/Output/Audio` costs nothing real — + // peerspeak only ever tags playback streams — and the attack needs the + // impostor to be a plausible playback node instead, which taints only + // its own descendants. The AEC's virtual sink/source is unaffected: it + // roots on [`Reason::AecIdentity`] below, by module id, not by this tag. + // A tag on a non-producer falls through: ignored for taint, but not + // nothing — it is either a peerspeak bug or an impostor, and + // [`misplaced_ownership_tags`] surfaces it so neither is silent. + if is_peerspeak_tagged(node) && node.role == MediaRole::StreamOutput { return Some(Reason::PeerspeakOwned); } if let (Some(module), Some(aec)) = (node.props.pulse_module_id, ctx.aec_module_id) diff --git a/src/host/taint/owner.rs b/src/host/taint/owner.rs index 94de20e..464d4d0 100644 --- a/src/host/taint/owner.rs +++ b/src/host/taint/owner.rs @@ -67,10 +67,71 @@ //! Grouping is **transitive** (union-find). That is the fail-closed //! direction: bigger owner components mean more taint, never less. -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; use super::snapshot::{GlobalId, GraphSnapshot, NodeSnapshot, Serial}; +/// Everything owner-key derivation needs from outside a single node. +/// +/// Introduced by round 10 (R10-3). Before it, `keys_of` read only node +/// properties, and key 4 was therefore available **only** to nodes carrying +/// `application.process.id` — which native PipeWire clients do not. mpv on its +/// default ao, and peerspeak's own playback stream, expose nothing but +/// `client.id`, so both were *unbounded*, and the moment any tainted reader +/// existed anywhere, `propagate_unresolved_owner` excluded every one of them. +/// Measured: an untagged mpv went from eligible (alone) to `unresolved-owner` +/// the instant peerspeak played audio. That is "native-PipeWire apps are never +/// shareable", which is not a feature. +/// +/// The missing pid is not missing at all — it is one hop away, on the node's +/// **Client**, as `pipewire.sec.pid`, and already in the snapshot. +pub struct OwnerCtx { + pub pipewire_pulse_pid: Option, + /// `client.id` → that Client's `pipewire.sec.pid`. + /// + /// Clients whose global id is **ambiguous** (two live objects claiming it, + /// i.e. the observer missed a removal) are deliberately absent: resolving + /// an ambiguous id to a pid would attribute a node to whichever Client won + /// a coin toss, and inventing an owner key is the one direction that can + /// *reduce* taint. Absent ⇒ unbounded ⇒ fails closed, as before. + client_pids: BTreeMap, +} + +impl OwnerCtx { + pub fn new(snapshot: &GraphSnapshot, pipewire_pulse_pid: Option) -> Self { + let mut client_pids: BTreeMap = BTreeMap::new(); + // ⚠️ Tracked separately from `client_pids`, and that is the point: a + // Client with no `sec_pid` still *claims* its id. Detecting duplicates + // by looking in the pid map would let a pid-less first claimant leave + // no trace, so the next Client claiming the same id would look unique + // and its pid would be used — resolving an ambiguous id, which is the + // one guess this guard exists to refuse. Pid-less Clients are ordinary + // (the session manager's is one). + let mut seen: BTreeSet = BTreeSet::new(); + for client in snapshot.clients() { + if !seen.insert(client.id) { + // Two Clients claiming one id: drop it entirely rather than + // pick. See the field docs. + client_pids.remove(&client.id); + continue; + } + if let Some(pid) = client.sec_pid { + client_pids.insert(client.id, pid); + } + } + Self { + pipewire_pulse_pid, + client_pids, + } + } + + /// The `pipewire.sec.pid` of this node's Client, if it has one and that + /// Client's id is unambiguous. + fn client_pid(&self, node: &NodeSnapshot) -> Option { + self.client_pids.get(&node.props.client_id?).copied() + } +} + /// Which key bridged two legs. Ordered strongest first; the `Ord` derive is /// load-bearing for "report the strongest shared key". #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)] @@ -84,6 +145,16 @@ pub enum OwnerKey { impl OwnerKey { /// Stable, machine-readable — this ends up in the phase 5 audit output /// and the phase 6 status event. + /// + /// ⚠️ **Known imprecision, deliberately not fixed here.** `ProcessId` now + /// covers two sources — the node's `application.process.id` and its + /// Client's `pipewire.sec.pid` (see [`keys_of`]) — so a bridge reported as + /// `application.process.id` may in fact have resolved on the Client's + /// protected pid. Pre-existing since R10-3 made the Client a fallback, and + /// widened by the review's finding 1 making it a union. Splitting it would + /// add a code to a set that is explicitly a stable contract for the audit + /// output and the "why isn't this app being shared?" answer, so it wants + /// its own decision rather than a drive-by. pub fn code(self) -> &'static str { match self { Self::LinkGroup => "node.link-group", @@ -106,7 +177,7 @@ enum KeyValue { /// A key that is present but unusable (the pipewire-pulse PID; a coarse key /// on a device node) is **absent** here — that is the whole mechanism of the /// two exceptions. -fn keys_of(node: &NodeSnapshot, pipewire_pulse_pid: Option) -> Vec<(OwnerKey, KeyValue)> { +fn keys_of(node: &NodeSnapshot, ctx: &OwnerCtx) -> Vec<(OwnerKey, KeyValue)> { let mut out = Vec::new(); if let Some(group) = &node.props.link_group { out.push((OwnerKey::LinkGroup, KeyValue::Text(group.clone()))); @@ -122,14 +193,48 @@ fn keys_of(node: &NodeSnapshot, pipewire_pulse_pid: Option) -> Vec<(OwnerKe if let Some(client) = node.props.client_id { out.push((OwnerKey::ClientId, KeyValue::Num(u64::from(client.0)))); } - if let Some(pid) = node.props.process_id { - // Exception 1. Note the fail-closed asymmetry when the daemon PID is - // unknown (`None`): the exception does *not* fire, key 4 applies to - // everything, and Pulse modules fuse into one owner. That is broad - // over-exclusion — annoying and safe — which is the direction v3.4 - // §6.1.2's failure-mode paragraph asks for. - if Some(pid) != pipewire_pulse_pid { - out.push((OwnerKey::ProcessId, KeyValue::Num(u64::from(pid)))); + // Key 4, from the node **and** from its Client (round 10, R10-3; made a + // union rather than a fallback by the round-10 review, finding 1). + // + // ⚠️ **A union, not `node.or_else(client)`, and the difference is a leak.** + // The node's `application.process.id` is client-controlled and optional; + // the Client's `pipewire.sec.pid` is `pipewire.*`, protected, and the only + // one that can carry a soundness argument (the same reason + // `propagate_unresolved_owner` sweeps everything for an unbounded reader). + // Letting the node's value *replace* the Client's meant one process using + // two Clients could escape the bridge entirely: its tainted reader reports + // a bogus node pid, its output leg omits the node pid and falls back to + // the Client's real one, the two legs are bounded by different values, so + // they neither bridge nor trip the unbounded sweep — and the output stays + // eligible while re-emitting the call. Carrying both values costs nothing + // and closes it: a leg that presents *either* value bridges. + // + // ⚠️ **Exception 1 applies to each value independently, and that is the + // whole risk here.** Measured on this host: 15 unrelated Clients share + // `sec_pid` 2528, which is pipewire-pulse's own — every Pulse-emulated app + // has one. Suppressing it per value is what keeps the union from fusing + // all fifteen into a single owner while still keeping each app's real + // per-app pid. For the common Pulse shape (node pid = the app's, Client + // `sec_pid` = the daemon's) the union therefore reduces to exactly the + // node's pid, as before. + // + // Note the fail-closed asymmetry when the daemon PID is unknown (`None`): + // the exception does *not* fire, key 4 applies to everything, and Pulse + // modules fuse into one owner. That is broad over-exclusion — annoying and + // safe — which is the direction v3.4 §6.1.2's failure-mode paragraph asks + // for. + for pid in [node.props.process_id, ctx.client_pid(node)] + .into_iter() + .flatten() + { + if Some(pid) == ctx.pipewire_pulse_pid { + continue; + } + let key = (OwnerKey::ProcessId, KeyValue::Num(u64::from(pid))); + // The two agree far more often than not; a duplicate entry would be + // harmless but would make the audit's key list read oddly. + if !out.contains(&key) { + out.push(key); } } out @@ -151,8 +256,74 @@ fn keys_of(node: &NodeSnapshot, pipewire_pulse_pid: Option) -> Vec<(OwnerKe /// nothing else relates them. Its sibling output leg cannot be found, so /// the engine must fail closed rather than declare it clean /// (v3.4 §6.1.1, final paragraph). -pub fn owner_is_bounded(node: &NodeSnapshot, pipewire_pulse_pid: Option) -> bool { - keys_of(node, pipewire_pulse_pid) +/// +/// # 🔴 OPEN, phase-6 blocking — the key union can *reduce* taint here +/// +/// **Round 11 review, finding 1. Verified correct; deliberately not fixed in +/// that round.** Round 10 made key 4 a union of the node's +/// `application.process.id` and its Client's `pipewire.sec.pid`, and the claim +/// that this was "strictly additive" was too strong: the same key list also +/// feeds *this* predicate, so adding a value can move a node from unbounded to +/// bounded, and `propagate_unresolved_owner`'s global sweep is triggered by an +/// **un**bounded tainted reader. Concretely: +/// +/// 1. A tainted reader's node claims the pipewire-pulse PID while its Client +/// holds a real protected PID `A`. Under `or_else` the node's value won and +/// exception 1 suppressed it, leaving the reader unbounded; under the union +/// it is bounded by `A`. +/// 2. Its process's output leg uses a second Client whose id is **ambiguous** +/// (the observer missed a removal), so no protected PID is available — but +/// the leg claims a bogus `application.process.id` `B`, which bounds it. +/// 3. Neither the bridge nor the sweep fires, and the output stays eligible +/// while re-emitting the call. +/// +/// It cannot leak today: `evaluate()` is reached only by the dry-run audit, +/// which creates no links. It becomes live when phase 6 consumes eligibility. +/// +/// **Why it is not fixed yet.** The principled repair is provenance: a +/// self-claimed `application.process.id` is not a *sound* bound, only the +/// protected keys are. But applying that bluntly makes every Pulse-emulated +/// app unbounded — their Client's `sec_pid` is the daemon's and suppressed, so +/// the node's own claim is their only per-app identity — which re-triggers the +/// §6.1.1 mass over-exclusion the whole design is built to avoid, and would +/// make the eligible half of the §5.1 matrix empty. +/// +/// The targeted rule that closes the path above without that cost: **a node +/// whose Client cannot be resolved at all must not be bounded by its own +/// self-claimed PID.** An ambiguous Client already means "we do not know who +/// owns this", and a self-claim must not paper over it; a Pulse app's Client +/// *is* resolved (to the daemon's PID, then suppressed), so it keeps its +/// bound. Implementing it needs `OwnerCtx` to distinguish "resolved" from +/// "absent", and `OwnerKeyIndex` to carry boundedness separately from the key +/// set, since bridging must keep using the full union. +/// +/// ⚠️ Do this **with the §5.1 matrix data in hand**, not before: the whole +/// question is how much over-exclusion the rule actually causes on a real +/// graph, and that is measurable rather than arguable. +/// +/// ## Round 12 — the deferral holds, and "resolved" has a trap in it +/// +/// Codex re-examined this and agreed the deferral is defensible while +/// `evaluate()` is audit-only, and that the rule above closes the recorded path +/// without unbounding normal Pulse-emulated apps — **but only under one +/// reading of "resolves"**, and the wrong reading reintroduces the hole: +/// +/// - ✅ "Resolved" must mean **an unambiguous Client that yields +/// `Some(pipewire.sec.pid)`**, taken *before* the pipewire-pulse suppression +/// step. A Pulse app then still has the daemon's protected PID as +/// provenance, even though that value is omitted from the bridge keys, so it +/// stays bounded and the eligible half survives. +/// - ❌ **Do not** implement it as "a unique Client object exists". A unique +/// Client with `sec_pid = None` would satisfy that test while providing no +/// protected identity at all, leaving exactly the self-claimed-PID hole this +/// rule is meant to close. +/// +/// So the matrix needs five Client cases, not two: **absent**, **ambiguous**, +/// **unique but pid-less**, **resolved-native**, and +/// **resolved-to-pipewire-pulse**. The third is the one that distinguishes the +/// two readings, and it is the row a two-case matrix would silently skip. +pub fn owner_is_bounded(node: &NodeSnapshot, ctx: &OwnerCtx) -> bool { + keys_of(node, ctx) .iter() .any(|(key, _)| *key != OwnerKey::ClientId) } @@ -168,11 +339,11 @@ pub struct OwnerKeyIndex { } impl OwnerKeyIndex { - pub fn build(snapshot: &GraphSnapshot, pipewire_pulse_pid: Option) -> Self { + pub fn build(snapshot: &GraphSnapshot, ctx: &OwnerCtx) -> Self { Self { keys: snapshot .nodes() - .map(|node| (node.serial, keys_of(node, pipewire_pulse_pid))) + .map(|node| (node.serial, keys_of(node, ctx))) .collect(), } } @@ -252,10 +423,10 @@ impl OwnerKeyIndex { pub fn strongest_shared_key( a: &NodeSnapshot, b: &NodeSnapshot, - pipewire_pulse_pid: Option, + ctx: &OwnerCtx, ) -> Option { - let a_keys = keys_of(a, pipewire_pulse_pid); - let b_keys = keys_of(b, pipewire_pulse_pid); + let a_keys = keys_of(a, ctx); + let b_keys = keys_of(b, ctx); // `keys_of` yields strongest-first, so the first match is the strongest. a_keys.iter().find_map(|(key, value)| { b_keys @@ -279,7 +450,7 @@ pub struct OwnerComponents { } impl OwnerComponents { - pub fn build(snapshot: &GraphSnapshot, pipewire_pulse_pid: Option) -> Self { + pub fn build(snapshot: &GraphSnapshot, ctx: &OwnerCtx) -> Self { let serials: Vec = snapshot.nodes().map(|n| n.serial).collect(); let index: BTreeMap = serials.iter().enumerate().map(|(i, s)| (*s, i)).collect(); @@ -290,7 +461,7 @@ impl OwnerComponents { let mut buckets: BTreeMap<(OwnerKey, KeyValue), Vec> = BTreeMap::new(); for node in snapshot.nodes() { let slot = index[&node.serial]; - for (key, value) in keys_of(node, pipewire_pulse_pid) { + for (key, value) in keys_of(node, ctx) { buckets.entry((key, value)).or_default().push(slot); } } diff --git a/src/host/taint/snapshot.rs b/src/host/taint/snapshot.rs index 88c3417..61d75ad 100644 --- a/src/host/taint/snapshot.rs +++ b/src/host/taint/snapshot.rs @@ -87,6 +87,20 @@ impl MediaRole { pub fn is_candidate(self) -> bool { matches!(self, Self::StreamOutput) } + + /// Stable machine-readable code for the audit output. Not the raw + /// `media.class`: `Other` has no single one, and the audit's codes are a + /// contract with the matrix, not with PipeWire. + pub fn code(self) -> &'static str { + match self { + Self::StreamOutput => "stream-output", + Self::StreamInput => "stream-input", + Self::Sink => "sink", + Self::Source => "source", + Self::Duplex => "duplex", + Self::Other => "other", + } + } } /// The subset of node properties the engine actually reasons about. @@ -97,8 +111,17 @@ impl MediaRole { /// on this feature means "not tainted". #[derive(Clone, Debug, Default, PartialEq, Eq)] pub struct NodeProps { - /// `peerspeak.owned` is present and truthy (v3.4 §5.1). A correctness - /// mechanism, explicitly *not* a security boundary. + /// `peerspeak.owned` is present and **exactly** + /// [`super::PEERSPEAK_OWNED_VALUE`] (v3.4 §5.1, tightened by round 10's + /// R10-4 — it is not "present and truthy", and the round-10 review found + /// this doc still saying so). A correctness mechanism, explicitly *not* a + /// security boundary. + /// + /// ⚠️ **Ownership carrier 1 of 2, so this being `false` does not mean + /// "not peerspeak's".** Carrier 2 is the [`NodeSnapshot::name`] prefix + /// [`super::PEERSPEAK_OWNED_NODE_PREFIX`], matched as a union in + /// `local_root_reason`. Read that function, not this field, to answer + /// "is this node owned?". pub peerspeak_owned: bool, /// `pulse.module.id`, parsed as `u64` — never `u32`, per v3.4 §5.2's /// parse-defensively note and the phase 0a truncation bug. diff --git a/src/host/taint/tests.rs b/src/host/taint/tests.rs index d6bde50..ac0a958 100644 --- a/src/host/taint/tests.rs +++ b/src/host/taint/tests.rs @@ -15,7 +15,7 @@ use std::collections::BTreeSet; use super::fixture::{Graph, NodeRef, PULSE_PID, app}; -use super::owner::{OwnerKey, strongest_shared_key}; +use super::owner::{OwnerCtx, OwnerKey, strongest_shared_key}; use super::snapshot::{MediaRole, NodeProps, PortDirection, Serial}; use super::{Decisions, Eligibility, ExclusionCtx, ObjectRef, Reason, StickyState, evaluate}; @@ -176,6 +176,217 @@ fn peerspeak_tagged_nodes_are_excluded_and_plain_apps_are_not() { assert_tainted(&decisions, sink, "tainted-upstream"); } +/// Each ownership carrier must work **alone** (v3.5 §5.1). +/// +/// ⚠️ The phase-3r lesson, applied deliberately: a gate that asserts a value +/// two sources can satisfy gates neither. `peerspeak_tagged_nodes_…` above +/// uses nodes carrying both carriers, so it would keep passing if either +/// were deleted. These are the rows that actually pin them. +#[test] +fn either_ownership_carrier_alone_taints_the_node() { + let mut graph = Graph::new(); + let sink = graph.device_node("hw-sink", MediaRole::Sink); + // Carrier 1: the property, on a node whose name says nothing. + let prop_only = graph.peerspeak_node_prop_only("some-playback-stream", 7); + // Carrier 2: the name prefix, property absent — the F1 case. + let name_only = graph.peerspeak_node_name_only("mpv", 31_284); + let firefox = graph.app_node("firefox", MediaRole::StreamOutput, 11_114); + for node in [prop_only, name_only, firefox] { + graph.link(node, sink); + } + + assert_partition( + &run(&graph, &ctx()), + &[("firefox", firefox)], + &[ + ("prop_only", prop_only, "peerspeak-owned"), + ("name_only", name_only, "peerspeak-owned"), + ], + ); +} + +/// **R10-1, the F2 fix.** Neither carrier is a security boundary — both are +/// strings any unprivileged process can set on its own node — so the tag is +/// honoured only on `Stream/Output/Audio`, the one role peerspeak ever tags. +/// +/// Without the restriction, a tagged `Stream/Input/Audio` **with no links at +/// all** is a tainted *reader* (`receivers` includes nodes by role, no link +/// required), and an unbounded one, so `propagate_unresolved_owner` fails +/// every candidate on the machine closed. That is a whole-feature denial from +/// an unprivileged process, reproduced live during the phase-1 review. +#[test] +fn an_ownership_tag_on_a_non_producer_is_not_a_taint_root() { + for role in [ + MediaRole::StreamInput, + MediaRole::Sink, + MediaRole::Source, + MediaRole::Duplex, + MediaRole::Other, + ] { + let mut graph = Graph::new(); + let sink = graph.device_node("hw-sink", MediaRole::Sink); + let firefox = graph.app_node("firefox", MediaRole::StreamOutput, 11_114); + graph.link(firefox, sink); + // Deliberately unlinked: the F2 shape needs no edges whatsoever. + let impostor = graph.peerspeak_tagged_node("rogue", role, 4_242); + + let decisions = run(&graph, &ctx()); + assert_untainted(&decisions, impostor); + assert!( + decisions.taint.is_empty(), + "{role:?} impostor tainted something: {:?}", + decisions.taint.keys().collect::>() + ); + // The whole point: the eligible half stays non-empty. + assert_partition(&decisions, &[("firefox", firefox)], &[]); + } +} + +/// **The live F2 reproduction, verbatim.** The measured impostor was an +/// *unbounded* reader — `client.id` present, `application.process.id` absent +/// — which is what turns "one bogus tainted node" into "nothing on this +/// machine is shareable": `propagate_unresolved_owner` cannot prove any +/// candidate independent of a reader it cannot attribute to an owner. +/// +/// Measured before the fix: `BASELINE eligible=1 excluded=[]` → +/// `WITH IMPOSTOR eligible=0 excluded=[firefox → unresolved-owner]`. +/// +/// Distinct from the row above, which uses a *bounded* impostor and so would +/// still pass if only the cheap half of the fix were present. +#[test] +fn an_unbounded_tagged_impostor_cannot_exclude_a_bystander_app() { + let mut graph = Graph::new(); + let sink = graph.device_node("hw-sink", MediaRole::Sink); + let firefox = graph.app_node("firefox", MediaRole::StreamOutput, 11_114); + let mpv = graph.app_node("mpv", MediaRole::StreamOutput, 31_284); + for node in [firefox, mpv] { + graph.link(node, sink); + } + + let baseline = run(&graph, &ctx()); + assert_partition(&baseline, &[("firefox", firefox), ("mpv", mpv)], &[]); + + // Both carriers, no pid, no links — everything an unprivileged process + // can arrange for itself in one `pw-cli` invocation. + let rogue_client = graph.client(Some(PULSE_PID)); + let impostor = graph.node( + &format!("{}rogue_4242", super::PEERSPEAK_OWNED_NODE_PREFIX), + MediaRole::StreamInput, + NodeProps { + peerspeak_owned: true, + client_id: Some(rogue_client), + ..NodeProps::default() + }, + ); + + let decisions = run(&graph, &ctx()); + assert_untainted(&decisions, impostor); + assert_partition(&decisions, &[("firefox", firefox), ("mpv", mpv)], &[]); +} + +/// A tag that R10-1 ignores is still reported, so that neither a peerspeak +/// tagging bug nor an impersonation attempt is silent. +#[test] +fn ignored_ownership_tags_are_surfaced_for_diagnostics() { + let mut graph = Graph::new(); + let sink = graph.device_node("hw-sink", MediaRole::Sink); + let call = graph.peerspeak_node("call", 7); + graph.link(call, sink); + let impostor = graph.peerspeak_tagged_node("rogue", MediaRole::StreamInput, 4_242); + + let snapshot = graph.build(); + let misplaced: Vec = super::misplaced_ownership_tags(&snapshot) + .iter() + .map(|node| node.serial) + .collect(); + + // Exactly the ignored one: the honoured producer is not "misplaced". + assert_eq!(misplaced, vec![impostor.serial]); + assert_ne!(impostor.serial, call.serial); +} + +/// The prefix is a **prefix**, not a substring: an unrelated app must not be +/// excluded because the literal appears somewhere in its name. Over-exclusion +/// is the safe direction, but it is still wrong, and the phase-5 gate now +/// asserts exact partitions in both halves. +#[test] +fn the_owned_prefix_matches_only_at_the_start_of_node_name() { + let mut graph = Graph::new(); + let sink = graph.device_node("hw-sink", MediaRole::Sink); + let impostor = graph.app_node( + &format!("recorder-of-{}stuff", super::PEERSPEAK_OWNED_NODE_PREFIX), + MediaRole::StreamOutput, + 11_114, + ); + graph.link(impostor, sink); + + assert_partition(&run(&graph, &ctx()), &[("impostor", impostor)], &[]); +} + +/// The consumer half of the cross-repo contract test (impl plan §3 +/// requirement 2). peerspeak runs the mirror of this against a byte-identical +/// copy of the same file, and asserts the environment a real child `Command` +/// would carry produces exactly these literals. +/// +/// This proves the two repos agree on the *literals*. That pixelpass actually +/// *listens* is proven by the two carrier tests above, and against the live +/// graph by the phase 5 dry-run. +#[test] +fn ownership_carriers_match_the_cross_repo_fixture() { + const FIXTURE: &str = include_str!("../../../tests/fixtures/ownership-tag-contract.txt"); + + let pinned: Vec<(&str, &str)> = FIXTURE + .lines() + .map(str::trim) + .filter(|line| !line.is_empty() && !line.starts_with('#')) + .map(|line| line.split_once('=').expect("fixture line is key=value")) + .collect(); + + // ⚠️ Refuse a duplicated key rather than resolving it (Codex phase-1 + // review, finding 3). This side takes the first match and peerspeak's + // took the last, so a duplicate in a byte-identical file could leave both + // repos green having selected *different* contracts. + for (index, (key, _)) in pinned.iter().enumerate() { + assert!( + !pinned[..index].iter().any(|(seen, _)| seen == key), + "fixture defines {key:?} twice; the two repos would disagree on which wins" + ); + } + let get = |key: &str| -> &str { + pinned + .iter() + .find(|(k, _)| *k == key) + .unwrap_or_else(|| panic!("fixture has no key {key:?}")) + .1 + }; + + assert_eq!(super::PEERSPEAK_OWNED_PROP, get("prop_key")); + assert_eq!(super::PEERSPEAK_OWNED_NODE_PREFIX, get("node_name_prefix")); + // ⚠️ **Equality, and that is now the whole rule**: carrier 1 is matched + // exactly, not as "anything but false/0" (round 10, R10-4). This assert + // used to be followed by a weaker `value != "false" && value != "0"` + // check, which described a leniency that no longer exists — the round-10 + // review's finding 6, and a real trap: a future producer reading the old + // fixture prose could emit "true" and silently lose this carrier. + // + // That this consumer actually *listens* to the fixture's value, through + // the production observer wiring rather than a helper, is asserted by + // `observer::adapter::tests::the_fixture_value_is_the_only_owned_spelling`. + assert_eq!(super::PEERSPEAK_OWNED_VALUE, get("prop_value")); + + // And the fixture's own worked example must be one this engine excludes, + // through carrier 2, exactly as written in the shared file. + let mut graph = Graph::new(); + let sink = graph.device_node("hw-sink", MediaRole::Sink); + let example = graph.app_node(get("node_name_example"), MediaRole::StreamOutput, 31_284); + graph.link(example, sink); + assert_partition( + &run(&graph, &ctx()), + &[], + &[("example", example, "peerspeak-owned")], + ); +} + #[test] fn aec_identity_is_exact_equality_and_other_modules_stay_eligible() { let mut graph = Graph::new(); @@ -507,8 +718,9 @@ fn owner_key_union_falls_through_a_present_but_unequal_key() { snapshot.node(b.serial).unwrap(), ); assert_ne!(a.props.client_id, b.props.client_id); + let owner_ctx = OwnerCtx::new(&snapshot, Some(PULSE_PID)); assert_eq!( - strongest_shared_key(a, b, Some(PULSE_PID)), + strongest_shared_key(a, b, &owner_ctx), Some(OwnerKey::ProcessId) ); } @@ -519,11 +731,12 @@ fn the_strongest_shared_key_wins_when_several_match() { let a = graph.group_node("a", MediaRole::StreamInput, "g", 500); let b = graph.group_node("b", MediaRole::StreamOutput, "g", 500); let snapshot = graph.build(); + let owner_ctx = OwnerCtx::new(&snapshot, Some(PULSE_PID)); assert_eq!( strongest_shared_key( snapshot.node(a.serial).unwrap(), snapshot.node(b.serial).unwrap(), - Some(PULSE_PID) + &owner_ctx ), Some(OwnerKey::LinkGroup) ); @@ -560,6 +773,321 @@ fn the_pipewire_pulse_pid_does_not_fuse_unrelated_modules() { assert_untainted(&decisions, b_in); } +/// **R10-3, the fix.** A native PipeWire client puts no +/// `application.process.id` on its node — only `client.id` — so before the +/// Client fallback it had no key 4, was therefore *unbounded*, and +/// `propagate_unresolved_owner` excluded it the moment any tainted reader +/// existed anywhere on the machine. +/// +/// Measured live: an untagged mpv was eligible alone, and became +/// `unresolved-owner` the instant peerspeak played audio. Since peerspeak +/// playing audio is the only situation in which this feature runs at all, that +/// amounted to "native-PipeWire apps are never shareable". +#[test] +fn a_native_client_is_bounded_by_its_clients_sec_pid() { + let mut graph = Graph::new(); + let hw = graph.device_node("hw-sink", MediaRole::Sink); + let call = graph.peerspeak_node("peerspeak", 7); + graph.link(call, hw); + + // The tainted reader that arms the unresolved-owner arm. Bounded itself + // (a real pid), exactly as the live `sunshine` was — so this is the + // bounded-reader arm, not the keyless-reader one. + let sunshine = graph.app_node("sunshine", MediaRole::StreamInput, 3_838); + graph.link(hw, sunshine); + + // mpv on its default ao: client.id only, pid on the Client. + let mpv = graph.native_client_node("mpv", MediaRole::StreamOutput, 31_284); + graph.link(mpv, hw); + + assert_partition( + &run(&graph, &ctx()), + &[("mpv", mpv)], + &[("call", call, "peerspeak-owned")], + ); +} + +/// The fallback must bridge a native app's *own* legs, or it has bought +/// boundedness without buying correctness: an app that reads the call and +/// re-emits it on a second native node would be declared clean. +#[test] +fn the_sec_pid_fallback_still_bridges_a_native_apps_own_legs() { + let mut graph = Graph::new(); + let hw = graph.device_node("hw-sink", MediaRole::Sink); + let call = graph.peerspeak_node("peerspeak", 7); + graph.link(call, hw); + + // One native process, two nodes, no link between them — the forwarder + // shape, in the native flavour. + let leg_in = graph.native_client_node("forwarder-in", MediaRole::StreamInput, 50_000); + let leg_out = graph.native_client_node("forwarder-out", MediaRole::StreamOutput, 50_000); + graph.link(hw, leg_in); + + let decisions = run(&graph, &ctx()); + assert_tainted(&decisions, leg_out, "tainted-owner-bridge"); + assert_partition( + &decisions, + &[], + &[ + ("call", call, "peerspeak-owned"), + ("forwarder-out", leg_out, "tainted-owner-bridge"), + ], + ); +} + +/// **The risk the fallback creates, and the guard on it.** Every +/// Pulse-emulated Client carries pipewire-pulse's own PID as `sec_pid` — +/// measured, 15 unrelated Clients sharing 2528 on this host. An unguarded +/// fallback would give all of them key 4 with the *same* value and fuse them +/// into one owner, so a single tainted Pulse app would exclude every other +/// Pulse app on the machine. +/// +/// Exception 1 therefore applies to the fallback exactly as it does to the +/// node's own property. Without that, this row goes red. +#[test] +fn the_sec_pid_fallback_does_not_fuse_every_pulse_client() { + let mut graph = Graph::new(); + let hw = graph.device_node("hw-sink", MediaRole::Sink); + let call = graph.peerspeak_node("peerspeak", 7); + graph.link(call, hw); + + // Three unrelated Pulse-emulated apps, each on its own Client, none + // exposing a node-level pid — so each can only reach key 4 through its + // Client, whose sec_pid is the daemon's. + let pulse_app = |graph: &mut Graph, name: &str, role| { + let client = graph.client(Some(PULSE_PID)); + graph.node( + name, + role, + NodeProps { + client_id: Some(client), + ..NodeProps::default() + }, + ) + }; + // One of them reads the tainted sink; the other two must not care. + let reader = pulse_app(&mut graph, "recorder", MediaRole::StreamInput); + graph.link(hw, reader); + let other_a = pulse_app(&mut graph, "player-a", MediaRole::StreamOutput); + let other_b = pulse_app(&mut graph, "player-b", MediaRole::StreamOutput); + + let decisions = run(&graph, &ctx()); + // They are unbounded (`client.id` alone never bounds an owner), so the + // fail-closed arm still excludes them — but as `unresolved-owner`, NOT as + // `tainted-owner-bridge`. That distinction is the whole assertion: a + // bridge reason here would mean the daemon pid had fused three unrelated + // applications into one owner, and unlike fail-closed exclusion, fusion + // does not go away when the apps are given real pids + // (`distinct_sec_pids_bound_each_native_app_separately` is that half). + assert_tainted(&decisions, other_a, "unresolved-owner"); + assert_tainted(&decisions, other_b, "unresolved-owner"); + for node in [other_a, other_b] { + assert_ne!( + decisions.taint.get(&node.serial).map(|e| e.reason.code()), + Some("tainted-owner-bridge"), + "the daemon pid must not bridge unrelated Pulse clients" + ); + } +} + +/// The same three apps, given **real per-app** `sec_pid`s: now the fallback +/// fires, all three are bounded, and only the one actually reading the call is +/// affected. This is the row that proves the guard above suppresses the daemon +/// pid *specifically* rather than disabling the fallback outright. +#[test] +fn distinct_sec_pids_bound_each_native_app_separately() { + let mut graph = Graph::new(); + let hw = graph.device_node("hw-sink", MediaRole::Sink); + let call = graph.peerspeak_node("peerspeak", 7); + graph.link(call, hw); + + let reader = graph.native_client_node("recorder", MediaRole::StreamInput, 6_001); + graph.link(hw, reader); + let other_a = graph.native_client_node("player-a", MediaRole::StreamOutput, 6_002); + let other_b = graph.native_client_node("player-b", MediaRole::StreamOutput, 6_003); + + assert_partition( + &run(&graph, &ctx()), + &[("player-a", other_a), ("player-b", other_b)], + &[("call", call, "peerspeak-owned")], + ); +} + +/// An **ambiguous** `client.id` — two live Clients claiming it, meaning the +/// observer missed a removal — must not yield a fallback pid. Inventing an +/// owner key is the one direction that can *reduce* taint, so resolving the +/// ambiguity by coin toss is the wrong kind of guess. +#[test] +fn an_ambiguous_client_id_yields_no_fallback_pid() { + let mut graph = Graph::new(); + let hw = graph.device_node("hw-sink", MediaRole::Sink); + let call = graph.peerspeak_node("peerspeak", 7); + graph.link(call, hw); + let sunshine = graph.app_node("sunshine", MediaRole::StreamInput, 3_838); + graph.link(hw, sunshine); + + // Two Clients, one id, distinct real pids. + let shared_id = graph.client(Some(6_010)); + graph.client_with_id(shared_id, Some(6_011)); + let app = graph.node( + "native-app", + MediaRole::StreamOutput, + NodeProps { + client_id: Some(shared_id), + ..NodeProps::default() + }, + ); + graph.link(app, hw); + + // Unbounded ⇒ fails closed, exactly as before R10-3. + assert_partition( + &run(&graph, &ctx()), + &[], + &[ + ("call", call, "peerspeak-owned"), + ("native-app", app, "unresolved-owner"), + ], + ); +} + +/// The ambiguity guard must not depend on the *first* Client claiming an id +/// having a `sec_pid`. +/// +/// Found by auditing R10-3 rather than by a failing case: the first cut +/// detected a duplicate id by looking it up in the pid map, which is only +/// populated for Clients that carry a pid at all. A pid-less Client therefore +/// left no trace, and the next Client claiming the same id was treated as +/// unique — resolving an ambiguous id, which is exactly the guess the guard +/// exists to refuse. Pid-less Clients are ordinary here (`device_node`'s +/// session client is one), so this is reachable, not theoretical. +#[test] +fn a_pidless_first_client_still_makes_its_id_ambiguous() { + let mut graph = Graph::new(); + let hw = graph.device_node("hw-sink", MediaRole::Sink); + let call = graph.peerspeak_node("peerspeak", 7); + graph.link(call, hw); + let sunshine = graph.app_node("sunshine", MediaRole::StreamInput, 3_838); + graph.link(hw, sunshine); + + // First claimant has NO sec_pid; second has one. + let shared_id = graph.client(None); + graph.client_with_id(shared_id, Some(6_011)); + let app = graph.node( + "native-app", + MediaRole::StreamOutput, + NodeProps { + client_id: Some(shared_id), + ..NodeProps::default() + }, + ); + graph.link(app, hw); + + assert_partition( + &run(&graph, &ctx()), + &[], + &[ + ("call", call, "peerspeak-owned"), + ("native-app", app, "unresolved-owner"), + ], + ); +} + +/// A process using **two** Clients cannot escape the bridge by presenting a +/// bogus pid on one leg and none on the other. +/// +/// ⚠️ **This is the round-10 review's finding 1, and it was a real leak while +/// key 4 was `node.or_else(client)`.** The node's `application.process.id` is +/// client-controlled; the Client's `pipewire.sec.pid` is protected. Letting +/// the node's value *replace* the Client's meant the reader was bounded by +/// `12_345` and the output leg by `50_000`, so they shared no key, did not +/// bridge, and — both being bounded — neither tripped the unbounded sweep. +/// The output stayed eligible while re-emitting the call. +/// +/// Carrying both values fixes it: the two legs share the Client pid. +/// +/// Reachability, stated honestly: `evaluate()` today is reached only by the +/// dry-run audit, which creates no links, so this could not echo on this +/// branch. It becomes live the moment phase 6 consumes these decisions. +#[test] +fn one_process_with_two_clients_cannot_split_its_pid_to_escape_the_bridge() { + let mut graph = Graph::new(); + let hw = graph.device_node("hw-sink", MediaRole::Sink); + let call = graph.peerspeak_node("peerspeak", 7); + graph.link(call, hw); + + // One native process, two Clients, one protected pid. + let reader_client = graph.client(Some(50_000)); + let output_client = graph.client(Some(50_000)); + + // Its reading leg claims a pid that is not its own. + let reader = graph.node( + "two-client-reader", + MediaRole::StreamInput, + NodeProps { + client_id: Some(reader_client), + process_id: Some(12_345), + ..NodeProps::default() + }, + ); + graph.link(hw, reader); + + // Its re-emitting leg claims no pid at all. + let output = graph.node( + "two-client-output", + MediaRole::StreamOutput, + NodeProps { + client_id: Some(output_client), + process_id: None, + ..NodeProps::default() + }, + ); + graph.link(output, hw); + + // A genuinely unrelated app must survive, or "exclude everything" would + // pass this test — the §5.1 eligible-half rule. + let bystander = graph.app_node("mpv", MediaRole::StreamOutput, 9_001); + graph.link(bystander, hw); + + assert_partition( + &run(&graph, &ctx()), + &[("mpv", bystander)], + &[ + ("call", call, "peerspeak-owned"), + ("two-client-output", output, "tainted-owner-bridge"), + ], + ); +} + +/// The node's own `application.process.id` is used even when its Client's +/// `sec_pid` is the daemon's — the single most common shape here, since a +/// Pulse-emulated node's pid is the app's while its Client's is +/// pipewire-pulse's. +/// +/// ⚠️ Both values are now carried (round-10 review, finding 1), so this is no +/// longer "the node's wins" but "exception 1 is applied per value": the +/// daemon's `sec_pid` is dropped and the node's real pid is kept, leaving the +/// same single key as before. +#[test] +fn the_nodes_own_process_id_wins_over_its_clients() { + let mut graph = Graph::new(); + // `app_node` is exactly that shape: node pid 11_114, Client sec_pid + // PULSE_PID. If the Client's won, exception 1 would suppress key 4 and + // this node would be unbounded. + let hw = graph.device_node("hw-sink", MediaRole::Sink); + let call = graph.peerspeak_node("peerspeak", 7); + graph.link(call, hw); + let sunshine = graph.app_node("sunshine", MediaRole::StreamInput, 3_838); + graph.link(hw, sunshine); + let firefox = graph.app_node("firefox", MediaRole::StreamOutput, 11_114); + graph.link(firefox, hw); + + assert_partition( + &run(&graph, &ctx()), + &[("firefox", firefox)], + &[("call", call, "peerspeak-owned")], + ); +} + #[test] fn an_unknown_pipewire_pulse_pid_over_excludes_rather_than_leaks() { // v3.4 §6.1.2's failure-mode paragraph: if pixelpass cannot identify 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