diff --git a/src/host/observer/classify.rs b/src/host/observer/classify.rs index 51c8f08..37080f6 100644 --- a/src/host/observer/classify.rs +++ b/src/host/observer/classify.rs @@ -61,9 +61,10 @@ const HARDWARE_PCM_FACTORIES: &[&str] = &[ /// would let tainted audio loop back untainted (Codex phase-3 review, /// finding 2). `factory.name` alone cannot distinguish these from a real /// card — `snd_aloop` presents as `api.alsa.pcm.{sink,source}` exactly like -/// `snd_hda_intel` — so this is a necessary denylist layered under the -/// factory allowlist, keyed on the one property that does distinguish them -/// (`alsa.driver_name`, measured present on real ALSA nodes). +/// `snd_hda_intel` — so a real ALSA terminal must present an `alsa.driver_name` +/// that is **present and not on this denylist**; a missing driver fails closed +/// (see [`classify`]). `snd_dummy` is intentionally absent: it is virtual but +/// does not couple playback to capture, so it is not a loopback hazard. const NON_TERMINAL_ALSA_DRIVERS: &[&str] = &["snd_aloop"]; /// The three node properties the classifier reads, exactly as the adapter @@ -88,8 +89,11 @@ pub struct DeviceClaim { /// `alsa.driver_name` — the kernel driver behind an ALSA node (e.g. /// `snd_hda_intel`, `snd_usb_audio`, `snd_aloop`). Needed because the /// factory allowlist cannot tell a real card from a loopback driver that - /// shares the same factory; a driver on [`NON_TERMINAL_ALSA_DRIVERS`] - /// forfeits `session_device`. Absent on non-ALSA backends. + /// shares the same factory. `session_device` requires this to be + /// **present and not** on [`NON_TERMINAL_ALSA_DRIVERS`]; a driver on the + /// denylist, or an absent value, both fail closed (see [`classify`]). + /// May be absent on non-ALSA backends or on version pairings that do not + /// copy `alsa.*` onto the node. pub alsa_driver_name: Option, } @@ -132,12 +136,21 @@ pub fn classify(claim: &DeviceClaim, device_resolved: bool) -> Classification { .factory_name .as_deref() .is_some_and(|f| HARDWARE_PCM_FACTORIES.contains(&f)); - let non_terminal_driver = claim + // A **present, non-denied** ALSA driver is required — absence fails closed + // (Codex phase-3 re-review). `alsa.driver_name` is not copied onto the + // node on every PipeWire/WirePlumber version pairing (PipeWire ≥1.2.6 + // stopped overwriting node props with card props; WirePlumber only began + // copying `alsa.*` onto nodes in 0.5.13), so a *missing* value must not be + // read as "not a loopback" — that is exactly the hole an `snd_aloop` node + // without the property would slip through. A real card whose node lacks + // the driver is instead over-excluded (keeps its owner keys — safe); + // recovering `session_device` for it needs reading the driver from the + // backing Device global, which is owed to a later round. + let driver_ok = claim .alsa_driver_name .as_deref() - .is_some_and(|d| NON_TERMINAL_ALSA_DRIVERS.contains(&d)); - let is_hardware_pcm = - claim.device_api.is_some() && on_factory_allowlist && !non_terminal_driver; + .is_some_and(|d| !NON_TERMINAL_ALSA_DRIVERS.contains(&d)); + let is_hardware_pcm = claim.device_api.is_some() && on_factory_allowlist && driver_ok; if is_hardware_pcm { Classification::SessionDevice } else { diff --git a/src/host/observer/mod.rs b/src/host/observer/mod.rs index 1f3e367..ced862f 100644 --- a/src/host/observer/mod.rs +++ b/src/host/observer/mod.rs @@ -31,10 +31,13 @@ //! The adapter drops such a global before it reaches [`RegistryModel`], so //! readiness can reach `Complete` while permanently omitting that Link — an //! invisible edge that could hide tainted ancestry. **Not reachable in -//! practice:** every real Link global carries `object.serial` (confirmed by -//! the live gate, which only counts links the strict parser admits). A full -//! fix needs a pure "required-observation-failed" token that holds readiness -//! false; deferred rather than built for a case that does not occur. +//! practice:** PipeWire's native protocol defines `object.serial` as the +//! unique identity every global carries, so a Link without one requires a +//! protocol/server failure, not ordinary churn. (The live gate is +//! consistent with this but does not *prove* it — it only counts Links the +//! strict parser already admitted.) A full fix needs a pure +//! "required-observation-failed" token that holds readiness false; deferred +//! rather than built for a case that does not occur. //! - *Removal generation ordering assumes no removal is silently lost.* On a //! recycled id with two live claimants, [`Self::on_removed`] retires the //! oldest generation first; if the *first* generation's removal was never diff --git a/src/host/observer/tests.rs b/src/host/observer/tests.rs index 014e9de..61db94d 100644 --- a/src/host/observer/tests.rs +++ b/src/host/observer/tests.rs @@ -142,6 +142,26 @@ fn classify_invented_bluez_factories_are_not_session_devices() { } } +#[test] +fn classify_alsa_without_driver_name_fails_closed() { + // Codex re-review: a missing `alsa.driver_name` must NOT grant + // session_device — an snd_aloop node whose driver prop was not copied onto + // the node would otherwise slip through. Absence fails closed. + for factory in ["api.alsa.pcm.sink", "api.alsa.pcm.source"] { + let claim = DeviceClaim { + device_id: Some(gid(7)), + device_api: Some("alsa".to_string()), + factory_name: Some(factory.to_string()), + alsa_driver_name: None, + }; + assert_eq!( + classify(&claim, true), + Classification::NotSessionDevice, + "absent driver on {factory} must fail closed" + ); + } +} + #[test] fn classify_snd_aloop_is_not_a_session_device() { // Finding 2: an ALSA loopback presents with an allowlisted factory and