host/taint: correct the buffered-echo scoping (in-threat-model); pin ambiguous-client (Codex round 6)

Codex refuted my round-5 disposition and was right: the buffered-echo gap
is NOT limited to keyless/unbounded readers. A normal PID-bearing app —
recorder, DAW, GStreamer — can read the call, buffer it in application
memory, fully tear down its PipeWire Node *and* Client, then (still the
same live process) open a fresh Client + output and replay. `seed_sticky`
drops the PID fingerprint once every old serial is gone, so the replayed
leg is Eligible. That is in-threat-model, so my "outside the threat model"
claim was false.

- Rewrote the module-doc gap note honestly: in-threat-model, reachable by
  non-adversarial software, sitting on the design's §6.1.3 "full teardown
  ⇒ starts clean" boundary. Framed the two options — (A) accept as a
  documented v1 limitation, (B) process-generation lifetime (PID + /proc
  start-time, phase 3 supplies liveness, §6.1.3 revised). This is a
  designer's decision (it revises the security surface); NOT resolved in
  code. `a_fingerprint_does_not_outlive_its_owner` currently encodes
  Option A and flips under B.
- P2 (fixed): pinned the ambiguous-client-id branch. A mutation
  remembering only the first of two clients claiming one global id
  survived the suite; added a test scoped to the ambiguous owner (the
  global count was masked by the peerspeak owner's client). Verified the
  `.next()` mutation now fails it.

57 tests. Phase 2 is NOT converged — the buffered-echo design decision is
owed to the user before merge.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

src/host/taint/mod.rs

@@ -52,39 +52,45 @@
 //! disappeared; a key that reappears after full teardown is a new owner and
 //! starts clean.
 //!
-//! ## Documented v1 limitation — buffered audio across a full teardown of an
-//! ## *unbounded* reader (Codex phase-2 round 5)
+//! ## ⚠️ KNOWN OPEN GAP — buffered audio across a full PipeWire teardown of
+//! ## a still-live process (Codex phase-2 rounds 5–6) — DESIGN DECISION OWED
 //!
-//! The "clear on full teardown" boundary is the design's, verbatim
-//! (v3.4 §6.1.3: "a key that reappears after full teardown is a new owner
-//! and starts clean"). It leaves one theoretical echo: an **unbounded**
-//! reader — one carrying no `node.link-group`, no `pulse.module.id`, and no
-//! usable PID — reads the call into an internal buffer, tears down *all* its
-//! PipeWire objects while keeping that buffer, then reconnects with fresh
-//! objects and replays. With every member serial gone and no owner key to
-//! fingerprint, nothing links the new owner to the old, and if no reader is
-//! live in that epoch the [backstop](propagate_unresolved_owner) does not
-//! fire either — so the replayed leg is eligible.
+//! **This is an in-threat-model echo gap, not an outside-the-model one — an
+//! earlier version of this note wrongly scoped it to keyless streams.**
 //!
-//! **Left as a documented v1 limitation, not fixed, on three grounds:**
-//! 1. **Outside the threat model.** v3.4 §2 states the model is "don't echo
-//!    the user's own call back at them," *not* "defend against a hostile
-//!    local process." A stream that exposes none of PID / module id /
-//!    link-group is a malformed or deliberately identity-hiding stream; real
-//!    software (apps carry a PID, pactl modules a module id, PipeWire
-//!    filters a link-group, GStreamer a PID) does not produce one.
-//! 2. **It contradicts the design's own boundary** (§6.1.3, quoted above),
-//!    so closing it is a *design change*, not a bug fix.
-//! 3. **No proportionate fix exists.** The only closed-form fix is a
-//!    whole-share hammer — once any unbounded reader is seen, exclude every
-//!    output for the rest of the share — which makes the desktop
-//!    unshareable on the mere appearance of one keyless stream. The reachable
-//!    cases (a reader live *now*) are already covered by the backstop's
-//!    exclude-everything tier.
+//! The scenario, entirely with a real PID-bearing app (a recorder, a DAW,
+//! a GStreamer pipeline): it reads the call into an application buffer,
+//! **fully** tears down its PipeWire Node *and* Client while keeping that
+//! buffer, then — still the same live process — opens a fresh Client and a
+//! `Stream/Output/Audio` and replays. Every old serial is gone, so
+//! [`seed_sticky`] refuses to apply the remembered PID fingerprint (the
+//! fingerprint is lifetime-scoped to a live serial member, because bare keys
+//! recycle); no reader is live in the new epoch, so the backstop does not
+//! fire; the replayed leg is eligible.
 //!
-//! Owed to the design doc as a round-8 note (impl plan §11); if field
-//! testing ever surfaces a real unbounded reader, revisit with
-//! process-generation evidence rather than the hammer.
+//! It is real and reachable by non-adversarial software. It also sits
+//! exactly on the design's stated boundary (v3.4 §6.1.3: "a key that
+//! reappears after full teardown is a new owner and starts clean"), so
+//! closing it is a **design change**, not a local bug fix:
+//!
+//! - **Option A — accept as a documented v1 limitation.** Contrived in
+//!   practice (most apps hold their PipeWire connection open for their
+//!   lifetime; the round-2 fix already covers the common
+//!   idle-a-client-and-open-another case), never a *silent* correctness
+//!   regression since it is written down, and phase 5's dry run would show
+//!   it. But it is a known echo path, which sits badly against the feature's
+//!   fail-closed ethos.
+//! - **Option B — process-generation lifetime.** Key the fingerprint's
+//!   lifetime on the owning **process** being alive — PID + `/proc` start
+//!   time (or a pidfd) to defeat PID reuse — instead of on a live PipeWire
+//!   object. Phase 3 supplies process liveness; §6.1.3's node/client-only
+//!   lifetime definition is revised. Closes the PID-bearing case; the truly
+//!   keyless sub-case (no PID at all) genuinely *is* outside the threat
+//!   model and stays a documented limit.
+//!
+//! The choice is the designer's (it revises the security surface). Until it
+//! is made, `a_fingerprint_does_not_outlive_its_owner` encodes Option A's
+//! behaviour — flip it if B is chosen. Owed to the design doc as round 8.
 
 // Phase 2 lands the engine behind its own test surface and nothing else:
 // the registry observer that will feed it is phase 3, so in a non-test

src/host/taint/tests.rs

@@ -1745,3 +1745,64 @@ fn a_local_root_receiver_bridges_without_an_inbound_link() {
     );
     assert_tainted(&decisions, sink, "pixelpass-owned");
 }
+
+#[test]
+fn an_ambiguous_client_id_remembers_every_claimant_for_stickiness() {
+    // Round 6 finding 2: nothing pinned the ambiguous-Client branch, so a
+    // mutation remembering only the first claimant survived. Two live
+    // clients claim one global id; the tainted owner's node references it.
+    // If we remember only one and it is the one that later disappears, the
+    // still-live claimant that reopens an output escapes.
+    use super::snapshot::{ClientSnapshot, GlobalId};
+
+    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);
+
+    // Two clients share one global id (the observer saw an id collision).
+    let shared_id = graph.dangling_id();
+    let client_a = graph.client_with_id(shared_id, Some(PULSE_PID));
+    let _client_b = graph.client_with_id(shared_id, Some(PULSE_PID));
+    assert_eq!(client_a, shared_id);
+
+    // The tainted reader references that (ambiguous) client id, no PID/keys.
+    let reader = graph.node(
+        "reader",
+        MediaRole::StreamInput,
+        NodeProps {
+            client_id: Some(shared_id),
+            ..NodeProps::default()
+        },
+    );
+    let out = graph.node(
+        "out",
+        MediaRole::StreamOutput,
+        NodeProps {
+            client_id: Some(shared_id),
+            ..NodeProps::default()
+        },
+    );
+    graph.link(hw, reader);
+    let firefox = graph.app_node("firefox", MediaRole::StreamOutput, 11114);
+
+    let c = ctx();
+    let (first, sticky) = evaluate(&graph.build(), &c, &StickyState::default());
+    assert_eq!(
+        first.candidates[&out.serial].reason().map(Reason::code),
+        Some("tainted-owner-bridge")
+    );
+    // Both claimants must be remembered, or a mutation keeping only one
+    // could drop the surviving owner. At least both client serials appear.
+    let client_members: usize = sticky
+        .owners
+        .iter()
+        .flat_map(|o| o.members.iter())
+        .filter(|m| matches!(m, super::ObjectRef::Client(_)))
+        .count();
+    assert!(
+        client_members >= 2,
+        "both ambiguous-id clients should be remembered: {sticky:#?}"
+    );
+    let _ = (ClientSnapshot { serial: Serial(0), id: GlobalId(0), sec_pid: None }, firefox);
+}