From abaf5d9c1056897c3a4d33e35a0e0bce4f6e1376 Mon Sep 17 00:00:00 2001 From: Mollusk Date: Sat, 25 Jul 2026 21:07:42 -0400 Subject: [PATCH] host/taint: a pid-less Client still makes its id ambiguous MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Verification round on R10-3's own fix. The ambiguity guard detected a duplicate client id by looking it up in the pid map — which is only populated for Clients that carry a sec_pid at all. A pid-less first claimant therefore left no trace, so the next Client claiming the same id looked unique and its pid was used, resolving an ambiguous id: exactly the guess the guard exists to refuse. Reachable, not theoretical — pid-less Clients are ordinary here (the session manager's is one). Reproduced: the bystander app went eligible off a coin-toss owner attribution. Claimed ids are now tracked separately from resolved pids. Co-Authored-By: Claude Opus 5 --- src/host/taint/owner.rs | 12 +++++++++--- src/host/taint/tests.rs | 42 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/src/host/taint/owner.rs b/src/host/taint/owner.rs index d0cbd7b..579d27f 100644 --- a/src/host/taint/owner.rs +++ b/src/host/taint/owner.rs @@ -100,13 +100,19 @@ pub struct OwnerCtx { impl OwnerCtx { pub fn new(snapshot: &GraphSnapshot, pipewire_pulse_pid: Option) -> Self { let mut client_pids: BTreeMap = BTreeMap::new(); - let mut ambiguous: BTreeSet = BTreeSet::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 client_pids.contains_key(&client.id) || ambiguous.contains(&client.id) { + 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); - ambiguous.insert(client.id); continue; } if let Some(pid) = client.sec_pid { diff --git a/src/host/taint/tests.rs b/src/host/taint/tests.rs index c8e57f5..ef883de 100644 --- a/src/host/taint/tests.rs +++ b/src/host/taint/tests.rs @@ -949,6 +949,48 @@ fn an_ambiguous_client_id_yields_no_fallback_pid() { ); } +/// 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"), + ], + ); +} + /// The node's own `application.process.id` wins when both are available. It is /// a direct statement about the node; the Client's is a one-hop inference, and /// they can legitimately differ (a Pulse-emulated node's pid is the app's while