From 8610ab2eb66edbe6e474da3de3a9a16d98a08b11 Mon Sep 17 00:00:00 2001 From: Mollusk Date: Tue, 21 Jul 2026 03:49:09 -0400 Subject: [PATCH] =?UTF-8?q?docs:=20v3.2=20audio-exclusion=20=E2=80=94=20si?= =?UTF-8?q?gnal-graph=20taint,=20measured?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 5. Codex and I converged independently on the same conclusion — a Link-only ancestry walk does not catch the leak — it from the crate/header/ WirePlumber sources, me from the live graph. Its sandbox could not reach the daemon (pw-dump: Operation not permitted), so the measurements are mine. Reproduced the EasyEffects topology with module-null-sink + module-loopback (same shape, no EasyEffects needed). Result: there is NO Link object between a forwarder's input leg and its output leg. Walking upstream from the leaking node over Links alone finds no inbound links at all — a dead end that reads as "clean". The legs are related only by shared node.link-group / client.id / pulse.module.id. So the signal graph needs three edge types: 1. Link edges — measured: registry Links carry all four endpoint props. 2. Sink-monitor — measured FREE at node granularity: the monitor connection IS a real Link whose output node is the sink itself. Codex held that this must be modelled explicitly; that is true only for a port-granular walk. Taint walks at node granularity, links are created per port. 3. Owner bridge — node.link-group when present, else client.id (measured shared across the forwarder's legs, distinct per app). Only modules set link-group, so client.id is what covers ordinary apps. New §6.1.1: bridge taint must be CONDITIONAL on the input leg being tainted. "Client has both legs ⇒ exclude" would exclude every app using a microphone. Firefox in a Meet call stays shareable; Firefox sharing desktop audio does not. Also: §6.5 rejects the cheap "hardware-sink-only" predicate with a measurement — the forwarder's output leg links directly to alsa_output, so the shortcut passes the leak and excludes the innocent app, backwards on both halves. §6.3 barrier corrected: core sync/done is a previous-work roundtrip, not graph quiescence. §6.4 adds crate version, endpoint fast path + bind fallback, and full-recompute cost. §5.2 correction 5 rewritten: application.process.id lives on the Node and is the app's own PID; pipewire.sec.pid lives on the Client and is pipewire-pulse's for every Pulse client. That resolves four rounds of contradictory PID claims. Co-Authored-By: Claude Opus 4.8 --- docs/screenshare-audio-exclusion-plan.md | 152 ++++++++++++++++++++--- 1 file changed, 135 insertions(+), 17 deletions(-) diff --git a/docs/screenshare-audio-exclusion-plan.md b/docs/screenshare-audio-exclusion-plan.md index 12c9a93..223c74a 100644 --- a/docs/screenshare-audio-exclusion-plan.md +++ b/docs/screenshare-audio-exclusion-plan.md @@ -1,7 +1,7 @@ # Design v3: whole-desktop screen-share audio without self-echo -**Status:** v3.1 — round-4 findings applied; **still BLOCKED pending round-5 re-review** (§14). -**Date:** 2026-07-21 (v1: 07-19 · v2: 07-20 · Option C adopted 07-20 · v3.1 round-4 revision) +**Status:** v3.2 — round-5 measurements applied; **awaiting round-6 re-review** (§14). +**Date:** 2026-07-21 (v1: 07-19 · v2: 07-20 · Option C 07-20 · v3.1 round 4 · v3.2 round 5) **Origin:** Joe's suggestion — "whitelist all audio except audio coming from peerspeak." **Scope:** a new capture mode in pixelpass (`src/host/pipeline.rs`, `src/host/audio.rs`), playback tagging + AEC-identity export + teardown-ordering invariants in peerspeak. @@ -214,11 +214,22 @@ exclude every node where pulse.module.id == A node is eligible only if **no** path through Node/Port/Link globals reaches a -> peerspeak-owned node, the live AEC identity, any pixelpass-owned object (ours *or* -> another host's `pixelpass_capture_*`), or a monitor/capture source that cannot be -> proven clean. +> A node is eligible only if **no** signal path reaches it from a peerspeak-owned node, +> the live AEC identity, or any pixelpass-owned object (ours *or* another host's +> `pixelpass_capture_*`). Unresolvable ancestry is **not** eligible. + +⚠️ **A Link-only walk does NOT catch the leak.** Measured on the live graph 2026-07-21 +by reproducing the forwarder topology with `module-null-sink` + `module-loopback` +(same shape as EasyEffects, no EasyEffects required): + +``` + fabletest_sink:monitor_FL ──Link 87──► input.loopback-2541-13 (Stream/Input/Audio, id 109) + ⋮ ← NO LINK OBJECT EXISTS HERE + output.loopback-2541-13 ──Link 105─► alsa_output…analog-stereo (id 56) + (Stream/Output/Audio, id 86) +``` + +**There is no `Link` between the forwarder's input leg (109) and its output leg (86).** +Walking upstream from the leaking node 86 over Links alone finds *no inbound links at +all* — a dead end that reads as "clean," and the node gets fanned out. The two legs are +related only by shared properties: + +| key | input leg (109) | output leg (86) | +| --- | --- | --- | +| `node.link-group` | `loopback-2541-13` | `loopback-2541-13` | +| `client.id` | `107` | `107` | +| `pulse.module.id` | `536870917` | `536870917` | + +So the signal graph needs **three** edge types: + +1. **Link edges** — `link.output.node → link.input.node`. ✅ Measured: registry Link + objects carry all four endpoint props (`link.output.node`, `link.output.port`, + `link.input.node`, `link.input.port`). +2. **Sink-monitor edges** — ✅ **free at node granularity.** Measured: the monitor + connection *is* a real `Link` whose output node is the sink node itself + (`out-node=104 (fabletest_sink) → in-node=109`). So a node-level walk crosses + `app → sink → monitor-reader` with no synthetic edge. A **port**-granular walk *does* + need an explicit rule (inputs feed `port.monitor=true` outputs); a node-granular one + does not. **Use node granularity for taint; use ports only for link creation.** +3. **Owner-bridge edges** — the intra-process hop the graph cannot see. Bridge on + `node.link-group` when present, else `client.id`. ✅ Measured: `client.id` is shared + by the forwarder's two legs (`107`) and is distinct per application (Firefox `96`, + Steam `82`, loopback `107`), so it is a usable fallback where `node.link-group` is + absent — which is the case for ordinary apps (OBS, browsers, recorders) that read a + monitor and re-emit, since only PipeWire *modules* set `link-group`. + +### 6.1.1 ⚠️ Bridge taint must be conditional, or it over-excludes badly + +The owner bridge must propagate taint **only when the input leg is itself tainted.** +The naive rule "this client has both an input and an output leg ⇒ exclude the output" is +catastrophic: it excludes any app using a microphone. Firefox in a video call has both +legs, and its playback is perfectly shareable. + +The correct rule: + +1. Taint roots: peerspeak-owned nodes, live AEC identity, pixelpass-owned objects. +2. Taint flows downstream along Link edges (crossing sinks and monitors for free, §6.1 + edge type 2). A sink carrying tainted audio has a **tainted monitor**. +3. A `Stream/Input/Audio` reading a tainted monitor becomes tainted. +4. **Only then** does the owner bridge carry taint to that client's `Stream/Output/Audio` + legs. + +Result, both cases correct: + +| scenario | outcome | +| --- | --- | +| Firefox playing music (output leg only) | **eligible** — shared | +| Firefox in a Meet call (mic input leg, untainted source) | **eligible** — shared | +| Firefox screen-sharing *with desktop audio* (input leg on a tainted monitor) | **excluded** | +| EasyEffects / loopback / combine-sink forwarding a mix containing peerspeak | **excluded** | + +Where a client has a tainted input leg and an output leg with **no** resolvable bridge, +fail closed and exclude the output leg. Note this only ever engages for clients actually +reading a tainted monitor, so the blast radius is small. This means pixelpass must **track Node, Port and Link globals** — not just node global-add, which is all the existing router does (`pixelpass/src/host/audio.rs:523-584` @@ -327,9 +407,9 @@ E2 makes mis-selection cheap *for the user's audio graph*, but it is not cheap f | --- | --- | | `peerspeak.owned` present | §5.1 env tag | | `pulse.module.id` == the live AEC index | §5.2, exact equality only | -| **any upstream ancestry reaching an excluded node** | §6.1 — the load-bearing one | +| **tainted via the signal graph** (Link + monitor + conditional owner bridge) | §6.1 — the load-bearing one | | pixelpass-owned objects, incl. **other hosts'** `pixelpass_capture_*` ancestry | cycle prevention | -| unresolvable / unknown ancestry | fail closed | +| tainted input leg + output leg with no resolvable bridge | §6.1.1, fail closed | | `port.exclusive` ports, encoded/passthrough streams | fan-out will refuse or corrupt | | links we already own for that node | idempotence | @@ -342,14 +422,52 @@ A node can appear with incomplete ancestry, pass the predicate, get fanned out, *then* receive an inbound link from a filter input or `pixelpass_capture_.monitor`. An add-only listener never sees it. Required: -1. An **initial registry sync barrier** — enumerate to a `core sync`/`done` before - deciding anything; a partially-populated registry is not a graph. +1. An **initial registry sync barrier** — register the listener, call `Core::sync()`, wait + for `done`, and only then consider the initial pass complete. + ⚠️ **Know what this barrier is and is not.** PipeWire's `core.h:261` defines sync as a + *previous-work roundtrip*: because methods and events are ordered, `done` means + "everything emitted so far has been handled." It is **not** graph quiescence and does + not promise the graph has stopped changing. Readiness therefore = `done` **plus** "no + unresolved required Link/Port observations outstanding," with a fail-closed timeout. 2. Candidates stay **pending** until their ports *and* current inbound links are known. 3. **Revalidate immediately before creating each Link**, not just at selection time. 4. On any later link add/remove that makes ancestry unsafe, **drop the owned link proxies** for the affected node — retention (§4.2) is what makes revocation possible. -## 6.5 Foreign/hazard nodes +### 6.4 Implementation shape and cost + +`pipewire` crate **0.9.2** (`pixelpass/Cargo.toml:48`, `Cargo.lock:4121`). + +- **Endpoint props fast path.** Registry `Link` globals carry all four endpoint keys + (`/usr/include/pipewire-0.3/pipewire/keys.h:256`), and they were observed present on + every live Link. **Residual doubt:** the observation was via `pw-dump`, which binds + objects — so "present on the object" is measured, "always present in the *global-add* + props" is not strictly proven. Cheap resolution: log the raw global-add props once in + the implementation spike. +- **Fallback if a prop is missing:** bind that Link, install an info listener, read the + endpoint ids off `LinkInfoRef` (`pipewire-0.9.2/src/link.rs:153` exposes all four + without params), then **drop the observer proxy**. Retained proxies are needed only for + *our own* fan-out links, where dropping is the ownership/lifecycle mechanism (§4.2) — + not for external links we merely observe. +- **Full recompute per graph event is fine for v1.** Rebuild taint by BFS/DFS from the + hazard roots, O(V+E). A typical desktop graph is ~40 nodes / ~120 links; this is + sub-millisecond-class in Rust. No incremental dirty-set in v1; coalesce bursts only if + logs show churn. + +### 6.5 Rejected: the "hardware-sink-only" shortcut + +Considered and **rejected**: "fan out only nodes whose output links to a real hardware +sink, excluding anything feeding a virtual/null sink." It is cheaper and it is **unsound +for exactly the case that matters** — measured: `output.loopback-2541-13` links *directly* +to `alsa_output.pci-0000_10_00.6.analog-stereo`. The forwarder's output leg **is** a +hardware-linked node, so the shortcut passes it and leaks the mix. The original app stream +would meanwhile be excluded for feeding the virtual sink — i.e. the shortcut gets both +halves backwards. + +It survives only as an optional degraded fallback mode whose stated behaviour is "no +EasyEffects/filter/loopback-routed audio is captured at all." + +### 6.6 Foreign/hazard nodes Treat an unvalidated `echo-cancel-*` output node as an excluded hazard rather than an eligible stream (§5.4).