diff --git a/src/host/observer/adapter.rs b/src/host/observer/adapter.rs index 387bb26..501e5b6 100644 --- a/src/host/observer/adapter.rs +++ b/src/host/observer/adapter.rs @@ -9,10 +9,10 @@ use super::{ EventKind, LinkEndpoints, NodeObservation, Outcome, Projection, RegEvent, RegistryModel, }; use crate::host::audio::parse_object_serial; -use crate::host::taint::PEERSPEAK_OWNED_PROP; 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}; @@ -649,10 +649,34 @@ 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), @@ -662,7 +686,7 @@ fn node_observation_from_props(props: &pw::spa::utils::dict::DictRef) -> NodeObs // in the engine off `NodeObservation::name` above, so each // carrier stays independently testable — see // [`crate::host::taint::PEERSPEAK_OWNED_NODE_PREFIX`]. - peerspeak_owned: truthy(props.get(PEERSPEAK_OWNED_PROP)), + peerspeak_owned: peerspeak_owned(props.get(PEERSPEAK_OWNED_PROP)), pulse_module_id: props .get("pulse.module.id") .and_then(|value| value.parse::().ok()), @@ -722,6 +746,53 @@ 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 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, }