host/observer: match the ownership carrier exactly, not leniently
The lenient `truthy` spelling was wrong for this one property. Under it, `peerspeak.owned=""` and `peerspeak.owned="false "` both read as owned, so any process could suppress a rival application's audio from the share with a property it did not have to spell correctly. The justification for leniency was that treating an unexpected value as "owned" over-excludes and is therefore safe. That does not hold: leniency here buys false-positive exclusion, not safety. 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, and a garbled property still leaves carrier 2's node.name prefix, which is a union with this one. `truthy` stays as it is for port.exclusive, port.monitor and node.passthrough: those are PipeWire's own, their spelling varies by producer, and each causes exclusion when true, so leniency really is the safe direction there. Both halves now have a row saying so. Codex phase-1 review F6. Round 10, R10-4. Mutation-verified. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -9,10 +9,10 @@ use super::{
|
|||||||
EventKind, LinkEndpoints, NodeObservation, Outcome, Projection, RegEvent, RegistryModel,
|
EventKind, LinkEndpoints, NodeObservation, Outcome, Projection, RegEvent, RegistryModel,
|
||||||
};
|
};
|
||||||
use crate::host::audio::parse_object_serial;
|
use crate::host::audio::parse_object_serial;
|
||||||
use crate::host::taint::PEERSPEAK_OWNED_PROP;
|
|
||||||
use crate::host::taint::snapshot::{
|
use crate::host::taint::snapshot::{
|
||||||
ClientSnapshot, GlobalId, MediaRole, NodeProps, PortDirection, PortSnapshot, Serial,
|
ClientSnapshot, GlobalId, MediaRole, NodeProps, PortDirection, PortSnapshot, Serial,
|
||||||
};
|
};
|
||||||
|
use crate::host::taint::{PEERSPEAK_OWNED_PROP, PEERSPEAK_OWNED_VALUE};
|
||||||
use anyhow::{Context, Result};
|
use anyhow::{Context, Result};
|
||||||
use pipewire::{self as pw, types::ObjectType};
|
use pipewire::{self as pw, types::ObjectType};
|
||||||
use std::cell::{Cell, RefCell};
|
use std::cell::{Cell, RefCell};
|
||||||
@@ -649,10 +649,34 @@ fn parse_serial(id: u32, kind: &str, raw: Option<&str>) -> Option<Serial> {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// 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 {
|
fn truthy(value: Option<&str>) -> bool {
|
||||||
value.is_some_and(|value| value != "false" && value != "0")
|
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 {
|
fn node_observation_from_props(props: &pw::spa::utils::dict::DictRef) -> NodeObservation {
|
||||||
NodeObservation {
|
NodeObservation {
|
||||||
name: props.get("node.name").map(str::to_string),
|
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
|
// in the engine off `NodeObservation::name` above, so each
|
||||||
// carrier stays independently testable — see
|
// carrier stays independently testable — see
|
||||||
// [`crate::host::taint::PEERSPEAK_OWNED_NODE_PREFIX`].
|
// [`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
|
pulse_module_id: props
|
||||||
.get("pulse.module.id")
|
.get("pulse.module.id")
|
||||||
.and_then(|value| value.parse::<u64>().ok()),
|
.and_then(|value| value.parse::<u64>().ok()),
|
||||||
@@ -722,6 +746,53 @@ mod tests {
|
|||||||
use super::*;
|
use super::*;
|
||||||
use std::process::Command;
|
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 {
|
struct PactlModule {
|
||||||
id: Option<u32>,
|
id: Option<u32>,
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user