Bound the libpipewire router shutdown without detaching its OS handle, contain GStreamer and pactl children in parent-bound process groups, and make ownership failures terminal through the capture supervisor.\n\nAdd focused lifecycle tests plus a serialized live router teardown gate.
Teardown used to `event_task.abort()` and then read three `Option<u32>`s.
The task's work was a synchronous `pactl` call with no await point, so the
abort could not land until the load had already returned: teardown saw
`None`, unloaded the sink, and the task then stored the new module's id into
a mutex nobody would read again. An orphan loopback, pointing at a sink that
no longer existed.
Three changes close it:
- Loads and unloads are `tokio::process::Command` with `kill_on_drop` and a
bound, so cancellation is expressible at all. They are deliberately not
`select!`ed against a cancel signal — dropping a completed load's index on
the floor is the defect, not the fix. Cancellation happens by dropping the
future, and the permit's `Drop` turns that into a question.
- `Routing::shutdown` is async and *awaits* the event task through
`&mut JoinHandle`, falling back to abort-then-await. Dropping the handle
would detach the task, which is how a load could still land after teardown
believed it had finished. It then runs two reconcile-then-unload rounds:
one round can raise exactly one new question, and a second settles it.
- `Drop` stays as the narrower synchronous backstop for the paths that never
reach `shutdown`. It cannot await or reconcile, so when the ledger is left
unexplained it says so and names `--repair`.
A load whose outcome cannot be observed is now distinguished from one the
server refused: a clean non-zero `pactl` exit abandons the permit (nothing
was created), while a signal death, a timeout, an unreadable index or
`PA_INVALID_INDEX` all leave it unsettled for reconciliation.
Two live gates, both A/B against the real module table: teardown leaves it
byte-identical with both modules carrying owner tokens, and a load cancelled
mid-flight is reconciled rather than orphaned. The second asserts the slot is
pending *before* reconciling, so it cannot pass by aborting before the load
ever began. Both mutate global state, so they need `--test-threads=1` —
running them in parallel makes each see the other's modules, which is how the
first run failed.
273 tests, clippy clean under `-D warnings`, `--doctor` all checks pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Module ids live in three `Option<u32>`s today, two of them shared with the
event task. That representation cannot express "a load is in flight", and
the gap is reachable: teardown calls `event_task.abort()` and then reads the
ids, but the task's work is a synchronous `pactl` call with no await point
inside it, so the abort cannot land until the load has already returned.
Teardown sees `None`, unloads the sink, and the still-running task stores the
new module's id into a mutex nobody will ever read again.
A slot is therefore a state machine whose transitions admit "we do not know":
Vacant / Loading / Loaded / Unloading / Ambiguous / Poisoned. The load permit
is affine — not `Clone`, consumed by value to settle — and its `Drop` marks
the slot ambiguous when it was never settled, so a cancelled task cannot
silently forget a module the server may already have created. An ambiguous
slot refuses the next load, because two sinks may share a `node.name` and
`pulsesrc` attaches to the older one: loading over unresolved debris would
silently steal the next session's capture.
Reconciliation is by owner token, whose nonce is minted per load and so names
one attempt: exactly one match adopts, zero means the load never happened,
and two or more fails closed rather than guessing. The Pulse session it lists
through is deliberately short-lived, because `repair::introspect` documents
that the binding leaks a timed-out request's callback until disconnect —
bounded for a session that ends immediately, unacceptable for one held open
for the life of a share. That is the one deviation from the round-19 design,
and it is why loads stay on `pactl`.
Pure: no I/O in the state machine, so all 17 gates run without a Pulse server.
All 8 mutants killed, each by its own named test. The stranger-at-the-same-
index gate needed strengthening first — its original fixture was a
non-canonical module, which `classify` discards regardless of how the match
was made, so an id-only comparator would have survived it.
Wiring into `host/audio.rs` follows; the dead-code warnings go with it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
nixpkgs 26.05 ships rustc 1.95.0, but this crate was developed and verified on
1.97.1 (what CachyOS had, installed 2026-07-17). Taking the compiler from
oxalica/rust-overlay decouples "which Rust the project targets" from "which
release the audio stack came from", so a nixpkgs bump can no longer move the
compiler under the lint gate as a side effect.
Chosen over rustup, which would also have worked here (nix-ld is enabled, so
its prebuilt binaries run) and would have let one rust-toolchain.toml cover the
packaging distroboxes too. The deciding factor is purity: rustup records
nothing in flake.lock, so a fresh clone or darp5 would resolve whatever it
fetched that day. rust-overlay gives the same exact-version control with the
choice pinned in the lock.
`.default` is the rustup "default" profile — rustc, cargo, rust-std, rustfmt
and clippy — so those are no longer listed individually. No windows-gnu target
here: pixelpass is Linux-only, unlike peerspeak.
Verified on 1.97.1: 256 tests pass, fmt clean, and `cargo clippy --all-targets
-- -D warnings` is clean.
The repo assumed a distro with a system-wide Rust and system-wide GStreamer,
which is exactly what NixOS does not provide. This adds a flake devShell
carrying the whole dependency surface:
- Build: rustc/cargo/clippy/rustfmt, pkg-config, and clang — pipewire-sys,
libspa-sys and libpulse-sys all generate bindings with bindgen, which needs
a real libclang via LIBCLANG_PATH rather than just clang on PATH.
- Link: pipewire, libpulseaudio, and libxcb. The libxcb one is not obvious:
x11rb is declared `default-features = false` here, but Cargo unifies
features across the graph and arboard pulls x11rb with `libxcb` on, so the
final link really does need -lxcb.
- Runtime: GStreamer is driven as a SUBPROCESS, not linked, so the tools and
their plugin search path are provided here too. NixOS keeps every plugin in
its own store path, so gst-launch-1.0 finds them only through
GST_PLUGIN_SYSTEM_PATH_1_0 — without it the `gst-inspect-1.0 --exists
pipewiresrc` preflight fails even with the plugins installed.
nixpkgs is pinned to nixos-26.05, the same channel the hosts run, so the
client libraries match the PipeWire daemon and PulseAudio server they talk to.
Verified: 256 tests pass, clippy clean, and `--doctor` reports all checks
passing (capture, encode, mux/audio, viewer, relay).
Nine commits, seven adversarial review rounds. The starting point was a real
defect — after 0c the capture sink is connection-owned, so a dead host leaves
loopbacks with no `module-null-sink` to trace its pid from, and discovery went
blind rather than getting smaller. Everything after that was the review finding
that the fix's foundations were softer than they looked.
What landed:
- Discovery derives candidate pids independently from all three module shapes,
A/B-proven on the live graph against the old binary.
- Recognition is exact-form only, and the matcher's templates are generated from
the loader's own renderer, so the two cannot drift; anything naming our sinks
that matches no known form is reported rather than silently ignored.
- Observation and unloading go through libpulse introspection over one
verified-local connection. `pactl`'s text output cannot carry this: a genuine
module whose argument contains a newline renders a first line that is
byte-exactly canonical (field-confirmed, no adversary needed), the JSON listing
carries no module index at all, and `PULSE_SERVER` is a fallback list that never
proved locality.
- A pid is not an owner. Every module carries a machine/boot/pid-namespace token,
and repair asks about a pid only when all three match — otherwise the module is
reported and its pid is never even looked up. Untagged modules from older builds
are refused by default, behind `--repair-legacy-untagged`.
- A plan is not a licence, and neither is ordering: fingerprints are re-verified
against a fresh snapshot per action, the sink unload is gated on nothing still
referencing it, and liveness runs before the snapshot so a replacement arriving
in that window is caught.
Verified beyond the unit suite: 256 tests, the phase-5 audit re-run with and
without tokens to prove the new property is inert to the taint engine, and four
live field gates covering orphan removal, the reference gate, and the token's
three cases.
Two lessons this merge is worth remembering for:
- The live field test found what unit tests structurally could not — including a
drop-order bug that made a completely successful repair exit 134, which is phase
0b's invariant one layer down.
- Every fix round in this branch contained a defect the next review caught. The
design held; the execution shell kept slipping.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex's non-blocking round-7 nit. The comment and test name still claimed liveness
is asked once per pid, which stopped being true when one pid became two questions.
No behaviour change; the wording was the last thing pointing at the old model.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Round 6, one blocking P2 — in code I wrote in the round-5 fix, not in the token
design, which the review passed again.
The verdict cache was keyed on pid alone. A pid can legitimately be claimed by a
tokened module *and* an untagged one at the same time — a host crashes, an older
build restarts and reuses the number — and those are not the same question: one is
answerable directly from the token, the other only if the degradation signals
allow it. Collapsing them let whichever module sorted last decide both, so under
`--repair-legacy-untagged` with a degraded probe an untagged winner made safely
attributable debris `Unknown`, and a tokened winner planned the untagged debris as
dead (the execution recheck happened to stop the destruction, which is luck, not
design). Verdicts are now cached per `(pid, Attribution)` and each fingerprint is
filtered by its own, never by looking its pid up in `dead_pids`. Those three pid
sets are documented as reporting-only, since a pid can now honestly appear in two
of them.
Mutation-verified: restoring the `dead_pids.contains(pid)` filter fails the new
test, which runs both module-id orders because the bug was order-dependent, and
both polarities — the second asserts that a *live* tokened owner does not lose its
module because an untagged claim on the same pid looked dead.
Also, the degraded-probe warning had become false (P3): it announced "refusing to
unload anything" while the tokened path can now legitimately unload, which in the
exact container-recovery case the token was added for would print a categorical
refusal and then destroy state. It is now scoped to what it actually means —
modules *without* a token will be left alone.
256 tests, clippy clean, fmt clean, field gates green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Round 5. Two blocking P2s, both in the execution shell rather than the token
design, plus an overstated claim of mine.
**The degradation signals were defeating the token in exactly the case it exists
for.** A host crashing inside a container leaves a token that matches this
machine, boot and pid namespace perfectly — but the probe saw a container marker
or a multi-entry `NSpid` and answered `Unknown` for everything, so token-qualified
repair did nothing precisely where it had just become safe. Those signals are
guesswork about whether a bare pid is meaningful, and for an attributed pid that
is no longer a guess. Liveness is now asked with the module's attribution in hand:
a tokened pid goes straight to `kill`, while an untagged one still has to get past
the signals, because there they are the only protection left.
**Liveness now runs before the fresh snapshot, not after it.** The natural order —
verify the module, check liveness, unload — leaves the dangerous window open:
after `kill` returns ESRCH this process can be descheduled while the planned module
vanishes, a new host inherits both the pid and the module index, and its
differently-nonced arguments occupy that index. Nothing re-read those arguments, so
the reused index would have been unloaded. Asking liveness first means the
post-liveness fingerprint check catches that replacement, leaving only the
irreducible snapshot-to-unload interval.
**The nonce claim was overstated and is now true.** A token was minted once per
`Routing` session and reused for every subsequent reload, making it a host-session
nonce rather than a per-load one. It is now minted inside `load_module`, mixing a
bumped counter with the clock, so two loads by the same pid really do render
different arguments — which, combined with the reordering above, is what lets a
fingerprint tell a module from its replacement at the same index.
⚠️ **The first version of the attribution fix had no gate, and the mutation said
so.** Swapping `of_attributed` back to `of` passed all 254 tests, because on an
ordinary desktop the probe is not degraded and the two paths agree, while the
planner tests use a fake liveness closure that never touches the probe at all. The
new test constructs a *deliberately degraded* probe, which is the only state where
the distinction is observable. Both mutants — routing a tokened pid through `of`,
and re-applying the degradation gate inside `of_attributed` — now fail it.
Codex's answers to the questions I raised, recorded because they close them:
omitting the pid from the token loses nothing, since the canonical argument already
binds the token to exactly one pid; namespace inode reuse is real but only after
the old namespace is destroyed, so its host is necessarily gone and no live owner is
endangered; and refusing foreign tokens even under `--repair-legacy-untagged` is the
right line, because the flag speaks to missing evidence rather than wrong evidence.
No finding against the two-hole template derivation.
255 tests, clippy clean, fmt clean. All four live field gates re-run green: tokened
cleaned, foreign refused with and without the flag, untagged refused then cleaned on
request, A/B orphan removal byte-identical elsewhere, reference gate still firing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Closes the last blocking finding: a pid is only a number, and the same number is
a different process in a different pid namespace. Repair running inside a
container that can reach the host's Pulse socket saw a live host's modules, asked
about that pid in its own namespace, was told nothing existed, and unloaded a
running host's audio. No negative signal closes that — `NSpid == 1` does not prove
the initial namespace, since its leftmost value is relative to whichever procfs
was mounted.
So the module now carries the answer with it. Every module a host loads gets
`pixelpass.owner=<version>-<machine>-<boot>-<pid_ns>-<nonce>`, and repair only
asks about a pid when all three identities match its own. Anything else is
reported and left alone, and its pid is never even looked up — asking is the bug,
because the answer would be meaningless.
**Untagged modules are refused by default.** Everything loaded before tokens
existed is unattributable, so `--repair` now lists those and does nothing, with
`--repair-legacy-untagged` to opt into the old pid-only heuristic after seeing the
candidates. That is a deliberate loss of reach: the failure being optimised
against is a false-positive destructive repair, and leaving an old orphan behind
is recoverable where destroying live routing is not. A foreign token is refused
even with the flag, since the flag speaks to missing evidence, not wrong evidence.
The vehicle was verified on the live server before anything was built on it: all
three shapes accept a property-list argument (`sink_properties`,
`sink_input_properties`, `source_output_properties`), the recorded argument comes
back byte-identical — so exact-form matching still holds — and the property really
lands on the resulting sink, sink-input and source-output.
**Audit gate passed, with the variable isolated.** The token rides on real graph
objects that phases 2/3 observe, so the partition had to be re-measured. Running
the same fixture with and without tokens gives an identical partition: 2 eligible
(FFXIV, Chromium), 2 excluded with the same `tainted-owner-bridge` reason, and the
same six-entry taint set. Everything that differs from the empty-graph baseline is
the fixture's own doing — a local-monitor loopback genuinely bridges our owned sink
into the real default sink — and none of it is the property's. Attributing that to
the token without the untokened control would have been the mistake.
A side benefit: the per-load nonce narrows the ABA window I previously documented
as unclosable. Two loads by the same pid no longer render byte-identical
arguments, so a fingerprint taken from one no longer matches the other.
Six new tests, three mutation-verified gates: `can_judge` always true, `pid_ns`
dropped from the comparison, and untagged treated as judgeable regardless of
policy — each killed by its own test. ⚠️ The third "survived" on first run because
my mutation script's indentation did not match and the edit silently did nothing;
the re-run asserts the file actually changed. A mutation that was never applied
proves the same amount as no mutation at all.
Field-verified live, three fixtures for one dead pid in one run: tokened with this
machine's identity is cleaned, tokened with a foreign pid namespace is left alone
and reported (and the legacy flag does not override it), and untagged is refused
then cleaned only when asked. The two older field fixtures were tokenised too —
without that the A/B test would have failed and the reference-gate test would have
passed for the wrong reason, which is a vacuous gate in the harness rather than the
code.
253 tests, clippy clean, fmt clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both P3s from the round-4 review.
The eight non-blocking mainloop iterations in `Drop` were a ritual, not a barrier:
a fixed number of polls cannot guarantee any particular event became ready. They
were also unnecessary — PulseAudio's context unlink cancels outstanding operations
and tears down the context's socket machinery synchronously, so once
`drop(context)` returns the mainloop has no obligation left to service. Removed
rather than replaced with a time-bounded drain, since there is no asynchronous
obligation for such a drain to wait on. Re-verified live: both field gates and a
clean-graph run still exit 0, with no abort.
Also recorded, at the constant it depends on: on a request timeout the `Operation`
wrapper is dropped while still running, and libpulse-binding 2.30.1 only unrefs the
C operation, so the boxed callback and its captured `Rc`s leak until the context
cancels the operation at disconnect. Harmless here — `--repair` is a one-shot
process that exits immediately after — and it cannot become a use-after-free, since
the closure owns its clones and the context clears callbacks before the mainloop is
touched. It would NOT be acceptable in the long-lived host, so the note says so
where someone would otherwise reuse this module for host-side loading.
The review's verdict on the teardown itself: disconnect, destroy the context while
the mainloop lives, then the mainloop, is the correct order, and taking the context
explicitly makes it independent of field declaration order.
247 tests, clippy clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Third review round found three more blocking P2s, and they shared one root cause:
`pactl`'s text output cannot carry the guarantees repair was claiming. All three
are closed by talking to the protocol instead. New dependency taken with the
user's explicit sign-off after vetting.
**Record boundaries were unprovable.** `pactl list short modules` prints a
module's argument raw into a tab/newline-delimited format with no escaping. A
*genuine* module whose argument contains a newline renders a first line that reads
byte-exactly like one of our canonical forms, with the remainder dropped as an
unparseable continuation — no forged index, so the duplicate-index check could not
see it. Repair would have classified and unloaded a module it never saw in full.
**Field-confirmed on the live server**, because this needed no adversary: loading a
loopback whose argument is canonical-then-newline-then-`remix=false` (a real
loopback option) produces exactly that listing. A tab in the same position is
worse: it hid a sink reference from the gate that protects a still-referenced sink.
**Index and argument could be mis-paired.** The listing carrying exact arguments
(`-f json`) carries no index at all on pactl 17; the one carrying the index cannot
carry the argument faithfully. Correlating them by position — which the previous
commit did — is unsound whenever module names repeat: another client loading one
module and unloading another between the two calls leaves counts and names aligned
while every argument has shifted by one, so a foreign module inherits a canonical
fingerprint. The name check cannot see it and the retry never fires.
**Locality was a guess.** `PULSE_SERVER` is a fallback *list*, so
`unix:/missing tcp:remote:4713` passes any "starts with unix:" test and then
connects to another machine, where local pids mean nothing and a live remote host's
modules look dead. A remote server can also be selected by client config with the
variable unset entirely.
New `repair/introspect.rs` owns one verified-local connection: `pa_module_info`
gives index, name and exact argument in a single record, `pa_context_is_local()`
answers locality about the connection actually established, and unloading goes back
through that same connection so listing and destruction cannot disagree about which
server they mean. It holds no policy beyond refusing the wrong server; every
decision stays in the pure planner.
⚠️ **The field test caught a real bug that no unit test could have.** The first
version did its work correctly and then aborted on the way out:
Assertion '!e->dead' failed at ../pulseaudio/src/pulse/mainloop.c:207,
function mainloop_io_free(). Aborting.
SIGABRT, core dumped, exit 134 — a fully successful repair reporting failure to its
caller. Cause: Rust drops fields in declaration order and the context's teardown
frees IO events living in the mainloop, which I had declared first. **This is
exactly the invariant phase 0b exists for, met again one layer down.** Fixed, and
then hardened past the fix: `Drop` explicitly takes and destroys the context before
the mainloop, so the ordering no longer depends on where the fields are written.
Liveness keeps its `NSpid`/container checks but the claim is corrected: `NSpid > 1`
means "definitely nested", while `NSpid == 1` is NOT proof of the initial namespace
— its leftmost value is relative to the procfs that was mounted, so a nested
namespace with its own `/proc` reports one entry legitimately. These are negative
signals that fail closed, not a proof of trustworthy pids. Closing that properly
needs modules to carry an owner token (machine/boot plus pid-namespace identity),
which changes what pixelpass writes into the graph and how far back `--repair` can
clean up: recorded as a design decision, not guessed at.
libpulse-binding 2.30.1 vetted before use: MIT/Apache-2.0, 5.5M downloads, 3 new
crates total, build script does nothing but probe pkg-config, no network or
subprocess use anywhere in the sources, and all three historical RustSec advisories
(2018-0020, 2018-0021, 2019-0038) were fixed by 2.6.0. The reasoning is recorded in
Cargo.toml beside the dependency.
247 tests, clippy clean, fmt clean apart from the pre-existing taint/tests.rs:2683.
The text parser's tests are gone with the parser; the liveness probe keeps its own,
and the live field gates — A/B orphan removal, the reference/unrecognised fixture,
and the newline fixture — all pass with exit 0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Second review round. Two blocking P2s and two P3s, all applied.
**The two-listing correlation was unsound, so it is gone** (P2). Pairing the short
listing's indices with the JSON listing's exact arguments by position breaks with
repeated module names: if another client loads one module and unloads another
between the two calls, the counts and names still line up while the arguments have
shifted by one — and a *foreign* module inherits a canonical fingerprint. The name
check cannot see it and the retry never fires, because correlation "succeeded".
Observations now come from a single `pactl list short modules` invocation, so every
`(index, name, argument)` comes from one server response and cannot be
mis-assembled. The two costs of that format are handled rather than hoped away:
- A tab inside an argument is invisible here, so the row is marked
`args_complete: false`. `classify` refuses such a row outright — a truncation
could otherwise coincide with a canonical form — while the reference gate can
still see that it names a sink.
- A crafted argument containing a newline can fabricate a row, but it only does
damage if it claims a *real* module's index, which makes that index appear twice.
A duplicated index now refuses the whole run.
libpulse introspection (`pa_module_info` carries index, name and argument in one
record) remains the exact route. It is a new dependency plus a mainloop in a
one-shot CLI path, so it is recorded as the upgrade rather than taken unilaterally.
**Liveness was still converting invisible-but-alive into dead** (P2). A
`/proc/self` preflight proves nothing: inside a pid namespace — a container, a
distrobox — `self` is visible while every process in the parent namespace is not,
and `hidepid` has the same shape. Repair there can reach the host's Pulse socket,
see a live host's modules, call its pid dead and unload a running host's audio.
So the probe now asks for positive confidence instead: `NSpid` in
`/proc/self/status` reports this process's pid in every namespace it appears in, so
more than one entry means our pid numbers are not the outer namespace's and every
verdict becomes `Unknown`. A kernel that does not report `NSpid`, a container
marker, and a non-local `PULSE_SERVER` all fail closed the same way. Liveness
itself is `kill(pid, 0)` via the existing `nix` dep, where `EPERM` proves
existence; pid 0 and pids past `i32::MAX` are never asked, since `kill(0, …)`
would signal our own process group.
**The renderer is the authority, not the derived template** (P3). `classify` now
re-renders the pid it extracted and demands byte equality, so the template is only
a pre-filter. `Shape` also owns the module *name* now, and `host/audio.rs` loads
through `Shape::{module_name, render_args}` — previously the "cannot drift" claim
covered only arguments while the names were still written out at both ends. The
sentinel assertion is unconditional (`assert!`), so a future shape that repeats the
pid cannot slip through a release build.
**A test I wrongly called unclosable** (P3). I argued no non-vacuous case could
prove the module name is part of the identity, because the name determines which
argument grammar can match. That was wrong: the grammars are not disjoint —
`module-echo-cancel sink_name=pixelpass_capture_9` is byte-identical to the
canonical null-sink argument, which the suite already constructs. Same index, same
arguments, different name, and `still_matches` must say no.
252 tests (+1 net; the correlation tests were replaced by parser and probe tests),
clippy clean, fmt clean apart from the pre-existing taint/tests.rs:2683. Both live
field gates re-run against the rewritten observation path: the A/B orphan test
still removes exactly the two orphans with the module table otherwise identical,
and the reference/unrecognised fixture still leaves both modules alone.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex's review of the repair planner returned changes-requested: no P1s, four
reachable P2s plus a P3, all in the observation and execution layers rather than
in the discovery fix itself. All applied.
**Only the canonical forms are ours** (P2, plan.rs). `classify` recognised any
loopback with one pixelpass-looking endpoint, so a third party's
`module-loopback source=some_mic sink=pixelpass_capture_4242` was ours to unload
once that pid died — and a `sink=` token nested inside a quoted
`sink_input_properties` value could be mistaken for a top-level argument. The
whole recorded argument string must now equal what pixelpass itself would have
written.
The matcher's templates are **generated from the loader's own renderers**, not
written out beside them: hard-coding `latency_msec=20` in a matcher means a
loader change silently blinds repair to every module the new build loads, which
is the fail-closed-and-silent failure this project has been bitten by three
times. `host/audio.rs` now loads through those same renderers, so the two cannot
drift. Blindness is also reported rather than assumed impossible —
`unrecognised_pixelpass_modules` finds modules that name our sinks but match no
canonical form, and `--repair` says so loudly.
Measured on the live server before relying on it (pactl 17.0): arguments come
back byte-for-byte as passed, joined with single spaces, with `@DEFAULT_SINK@`
NOT resolved. Both facts are load-bearing for exact matching and both have a
test.
**Ordering is not a licence either** (P2, mod.rs). The plan put loopbacks before
the sink, but an unload can fail or be skipped and a loopback can appear after
planning, so the executor could still destroy a sink that something was attached
to. The sink unload is now gated on `sink_still_referenced` against the fresh
snapshot — any other module naming that sink blocks it, ours or not, because the
question is what would break, not who owns it.
**Undecidable is not dead** (P2, mod.rs). `Path::exists()` maps permission
errors, a missing `/proc` and a foreign pid namespace all to `false`, which here
read as "dead, go ahead and unload". Liveness is now `Alive | Dead | Unknown`
via `try_exists()` behind a `/proc/self/stat` preflight, and `Unknown` is
treated exactly like alive and reported separately.
**The short listing cannot carry a fingerprint** (P2, mod.rs). Its arguments are
tab-delimited text that a module argument may itself contain, and a continuation
line beginning with a digit could fabricate a row. Observations now come from two
listings: the short one for the module index, and `pactl -f json list modules`
for the exact argument. Codex proposed JSON alone; on pactl 17 its records carry
`"index": null`, so it cannot be used on its own — verified, hence the
correlation. The pairing is positional and *checked* (same count, same name at
every position, else refuse), which is also what makes a fabricated row harmless
instead of exploitable: it has no JSON counterpart, so the sequences misalign.
Normalisation is gone (P3). Within one invocation every snapshot comes from the
same server, so re-rendering does not happen, and normalising only made
genuinely different arguments compare equal. The residual ABA window — planned
module vanishes, byte-identical one takes its index — cannot be closed through an
unload API whose only argument is an index; that is now said plainly in the
fingerprint's own doc comment rather than implied away.
Five vacuity gaps Codex found in the tests, closed: a raw-pactl-output-to-plan
test (the planner suite survived a parser that dropped every argument), the
liveness-once test now uses two pids with per-pid counters, non-canonical and
nested-quoted arguments have their own cases, and the reference gate has one.
Field-verified on the live graph, both new rules: the A/B orphan test still
removes exactly the two orphans with the module table otherwise byte-identical,
and a fixture of a dead pid's legacy sink plus a non-canonical loopback naming it
leaves both alone and reports why.
251 tests (+9), clippy clean, fmt clean apart from the pre-existing
taint/tests.rs:2683.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 0c's first landed piece. `--repair` does not get smaller when the capture
sink becomes connection-owned — it goes blind, which is the opposite of the
assumption I started from and the finding that reordered this phase.
Today repair learns a dead host's pid ONLY from `module-null-sink
sink_name=pixelpass_capture_<pid>`, and matches loopbacks only if their pid is
already in that set. After 0c the sink is a native node that removes itself with
its owning connection, so a hard-killed host leaves two Pulse loopbacks and *no
null-sink module to learn the pid from*. The set stays empty, nothing matches,
and the orphans are invisible forever. Candidate pids are now derived
independently from all three module shapes.
Split into a pure planner (`repair/plan.rs`) and an I/O shell, because every
interesting property here is a decision — which pid is dead, which module is
whose, in what order to unload — and none of them need PipeWire to exercise.
The rule that is new, and that the old code could not express: **a plan is not a
licence.** Pulse module indices are reused verbatim, so an id planned against one
module can name a different live module by the time the unload runs; a pid
recheck alone does not catch that. Every action now carries a full fingerprint
(id, module name, normalized args, derived pid, shape) which is re-verified
against a FRESH snapshot immediately before each unload, with liveness rechecked
last, closest to the destruction. Anything that does not match exactly is
skipped and said out loud — never unloaded on the strength of a stale plan.
Ordering is carried by `Shape`'s declaration order rather than by two separate
passes, so loopbacks unload before the sink they reference by construction.
Liveness is asked once per pid, not once per module: a flapping answer must not
be able to half-repair a host, which is the one outcome worse than doing nothing.
Field-tested against a real post-0c orphan, not just mocked: a connection-owned
sink created via `pw-cli create-node adapter` (module-null-sink count: 0, so the
old discovery provably could not see it), both loopback shapes loaded against it,
then SIGKILL of the owning connection. The sink vanished on its own, both
loopbacks survived, `--repair` removed exactly those two, and the module table
was otherwise byte-identical before and after — the collateral-damage half of the
two-host safety property.
Mutation-verified, five mutants, each killed by its own gate: null-sink-only
discovery (the 0c blindness itself), id-ordered unloads instead of shape-ordered,
a fingerprint that compares only the index (exactly one), no liveness filter, and
un-normalized args (exactly one).
Still owed: the live two-host gate (two real hosts, kill one, prove the other's
graph and modules are untouched) cannot run until the native sink exists, so it
is deferred to 0c's combined exit gate. This lands with test + single-host field
proof only, which Codex agreed is an acceptable phase dependency rather than an
objection to landing repair first.
242 tests (+11), clippy clean, fmt clean apart from the known pre-existing
`taint/tests.rs:2683` — pixelpass is never `cargo fmt`ed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Boundedness now requires a strong key, or a key-4 value backed by a
**resolved Client** — an unambiguous Client yielding Some(pipewire.sec.pid),
read before pipewire-pulse suppression. A node whose Client cannot be
resolved at all is unbounded whatever `application.process.id` it puts on
itself, so it can no longer spare itself from `propagate_unresolved_owner`'s
sweep with a value it made up.
Closes the round-11 review's finding 1: the key-4 union could *reduce* taint,
because the same key list feeds boundedness and the sweep is armed by an
UNbounded tainted reader. The recorded three-step path (reader bounded by its
Client's real pid, output leg on an ambiguous Client claiming a bogus pid, no
shared key so no bridge either) is now a test.
Bridging is untouched: it still uses the full union, so boundedness is stored
on OwnerKeyIndex rather than re-derived from the key set, and `bounded_by` is
the single implementation of the predicate.
Five-case Client matrix as tests (absent · ambiguous · unique-but-pid-less ·
resolved-native · resolved-to-pipewire-pulse). The pid-less row is the one
that distinguishes the correct reading of "resolved" from "a unique Client
exists", which would have left the hole open. Mutation-verified: dropping the
provenance test fails four of the six rows and passes the two that must not
regress.
Measured cost on the live graph: zero. Before- and after-binaries audited the
same graph simultaneously (tagged producer + parec on the monitor as a real
tainted reader, so the sweep was armed) — 181 records each, the same 14
distinct decision states, none exclusive to either side, no unresolved-owner
on either, eligible half non-empty throughout. O5 unmoved.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
MEASURED DEFECT, found by the §5.1 row-1 matrix run: the daemon PID was
unresolvable on this host, permanently, which switched key 4's suppression off
and fused every Pulse-emulated node into a single owner. Row 1's clean control
forwarder was excluded, Firefox was excluded across an application.process.id
bridge, and the taint reached the sunshine sinks and both sound cards -- the
same machine-wide over-exclusion cascade as phase 5's F2, from a new cause.
Stage 1 returned the one sec_pid shared by 2+ Clients, reasoning that only the
Pulse shim repeats a value. WirePlumber repeats one too: it holds Clients
'WirePlumber' and 'WirePlumber [export]', both sec_pid 1747. Two values
repeated, the rule called that ambiguous and returned None, and
owner::keys_of's documented fail-closed asymmetry did the rest.
The rule was wrong in both directions, so the prefilter is gone rather than
patched: a second process holding two Clients defeats it (permanent, not a
corner case), and a session where pipewire-pulse holds exactly one Client never
repeats anything so the candidate is missed. comm was always the authoritative
check; repetition stood in front of it and was a guess about other processes'
Client counts.
Now: candidates() lists every distinct sec_pid, resolve() picks the unique one
whose /proc comm is exactly pipewire-pulse, and several matches still fail
closed (a single Option<u32> cannot suppress two daemons -- recorded, not
approximated). The adapter probes only PIDs entering the set, and
retain_probed_comms bounds the map to live PIDs so a PID that leaves and
returns is re-probed instead of answered from a stale comm.
Row 1 now passes its exact partition, key named: tainted forwarder leg excluded
on node.link-group, clean forwarder leg and Firefox eligible, taint confined to
the tainted half. 225 tests green (WirePlumber-pair and single-Client
regressions covered), clippy clean.
Impl plan section 5.1 row 1 asserts "reason = owner bridge, naming the key",
and the record could not express that: Reason::code collapses
TaintedOwnerBridge { key } to one string, so an exclusion that arrived across a
named owner key was indistinguishable from one that arrived by an incidental
link walk reaching the same verdict. Telling those apart is the entire point of
the row.
OwnerKey::code already documented itself as ending up in the phase 5 audit
output; it was simply never wired to it. Adds owner_key to AuditRow and
TaintRow, omitted when absent, and absent is meaningful: a bridge whose tainted
member shared no key directly reached the node transitively, so there is no
single key to name and naming one would be a false diagnosis.
Read-only and diagnostic-only. New test mutation-verified (stubbing the key to
None fails it); 221 tests green, clippy clean.
pixelpass-side half of phase 1: the taint engine recognises both registry-visible
ownership carriers peerspeak now emits (peerspeak.owned and the
peerspeak_owned_ node.name prefix), giving the primary taint root a path that
survives the filtered registry global event (design v3.5 section 6.7).
Reviewed by Codex rounds 10-12. F11-1 (the owner-key union can REDUCE taint via
owner_is_bounded, switching the unresolved-owner sweep off) remains OPEN and
still blocks phase 6; it is deliberately deferred to be decided with section 5.1
matrix data, and its sharpened rule is recorded in owner_is_bounded's doc.
220 tests green, clippy clean.
Round 12 re-examined the deferral and agreed it holds while evaluate()
is audit-only, and that the recorded rule closes the path without
unbounding Pulse-emulated apps — but only under one reading of
"a node whose Client cannot be resolved at all".
The trap is worth writing down before anyone implements it: reading
"resolved" as "a unique Client object exists" passes for a unique Client
with sec_pid = None, which supplies no protected identity and leaves
exactly the self-claimed-PID hole the rule exists to close. It has to
mean an unambiguous Client yielding Some(pipewire.sec.pid), taken before
pipewire-pulse suppression.
That also means the §5.1 matrix needs five Client cases rather than two:
absent, ambiguous, unique-but-pid-less, resolved-native, and
resolved-to-pipewire-pulse. The pid-less row is the one that
distinguishes the two readings and the one a two-case matrix skips
without saying so.
Docs only. Still deferred, still to be decided with matrix data in hand.
220 tests green, clippy clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Round 11 review, finding 1. Verified correct: round 10's claim that the key
union was "strictly additive" was too strong. The same key list feeds
owner_is_bounded, and the unresolved-owner sweep is triggered by an UNbounded
tainted reader -- so adding the Client's PID can move a reader from unbounded
to bounded and switch the sweep off, letting a same-process output leg with an
ambiguous Client and a bogus self-claimed PID stay eligible.
Cannot leak today (evaluate() is audit-only); becomes live in phase 6.
Not fixed in this round, and the doc says why: the blunt repair -- only
protected keys bound an owner -- makes every Pulse-emulated app unbounded,
which re-triggers the mass over-exclusion the design exists to avoid and would
empty the eligible half of the 5.1 matrix. The targeted rule (a node whose
Client cannot be resolved at all is not bounded by its own self-claimed PID)
is written down along with what it needs structurally, to be implemented with
matrix data in hand rather than argued from a whiteboard.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Verification round on the round-10 review fixes.
Finding 6 named the fixture, taint/tests.rs and snapshot.rs, but the same
stale claim was also on PEERSPEAK_OWNED_VALUE itself — the definition site
for the very literal the finding was about, still arguing that any truthy
value counts and that this is the fail-closed direction. Corrected with the
reason the argument fails.
Also records a known imprecision the union widened: OwnerKey::ProcessId now
covers both application.process.id and the Client's pipewire.sec.pid, so a
bridge reported under the former may have resolved on the latter.
Pre-existing since R10-3; not fixed here because these codes are a stable
contract for the audit output and the phase 6 status event, so splitting one
wants its own decision.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Round 10 review, findings 1, 4 and 6.
Finding 1 (P1, phase 6) — key 4 was `node.or_else(client)`, so a node's
client-controlled application.process.id REPLACED its Client's protected
pipewire.sec.pid. One process using two Clients could therefore split its
identity: the tainted reader reports a bogus node pid, the output leg omits
the node pid and falls back to the Client's real one, the legs are bounded
by different values, and they neither bridge nor trip the unbounded sweep —
the output stays eligible while re-emitting the call. Now a union of both
values, deduplicated, with exception 1 applied to each independently so the
pipewire-pulse pid still cannot fuse unrelated Clients.
Mutation-verified: reverting to or_else fails ONLY the new split-Client test
(so the union changes nothing else), dropping exception 1 fails 32 rows, and
using the Client pid alone fails 16.
Not reachable today — evaluate() is reached only by the dry-run audit, which
creates no links. It becomes live when phase 6 consumes these decisions.
Finding 4 — R10-4's test called peerspeak_owned() directly, so reverting
node_observation_from_props to truthy() left it green; the only case it
shared with production, exact "1", passes under both. A new test builds a
real pw_properties dict and drives the production wiring, and the mutation
now fails exactly that test while the helper test still passes.
Finding 6 — the cross-repo fixture still documented carrier 1 as "any value
other than false/0", which R10-4 made exact-"1". A producer following it
could emit "true" and silently lose the carrier. Fixture updated in both
repos (byte-identical, verified), along with the stale prose in taint/tests
and snapshot.rs, and the contract is now also exercised through the
production adapter rather than only against the constants.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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>
Native PipeWire clients put no application.process.id on their nodes —
only client.id. keys_of read node properties alone, so those nodes had no
key 4, were therefore unbounded, and propagate_unresolved_owner excluded
them the moment any tainted reader existed anywhere on the machine.
Measured: an untagged mpv was eligible alone, and became unresolved-owner
the instant peerspeak played audio. Since peerspeak playing audio is the
only situation in which this feature runs, that amounted to "native
PipeWire apps are never shareable". The tainted reader that armed it was
sunshine, which is itself bounded — so this is the bounded-reader arm,
not the keyless-reader case §6.1.1 narrates.
The pid is one hop away, on the node's Client, already in the snapshot.
RISK, and the guard on it: every Pulse-emulated Client carries
pipewire-pulse's own PID as sec_pid — measured, 15 unrelated Clients
sharing 2528 on this host. An unguarded fallback would fuse all of them
into one owner. Exception 1 therefore applies to the fallback exactly as
it does to the node's own property, so the fallback strictly *adds*
correct bounding rather than trading it.
Ambiguous client ids yield no fallback pid: inventing an owner key is the
one direction that can reduce taint, so a coin toss is the wrong guess.
The client index is threaded through a new OwnerCtx rather than a sixth
positional Option<u32>, and evaluate() builds one and shares it, so the
components and the key index cannot disagree about who is bounded.
Round 10, R10-3. 6 new rows; 3 mutations verified — removing the
fallback, dropping the pulse-pid exception (11 rows die), and resolving
an ambiguous client id instead of dropping it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Neither ownership carrier is a security boundary — both are strings any
unprivileged process can put on its own node — so an unrestricted taint
root is a denial of the whole feature. An unlinked Stream/Input/Audio
named `peerspeak_owned_rogue` is a tainted *reader* (receivers includes
nodes by role, no link required) and an unbounded one, so
propagate_unresolved_owner fails every candidate on the machine closed.
Measured before this change: BASELINE eligible=1 excluded=[] became
WITH IMPOSTOR eligible=0 excluded=[firefox -> unresolved-owner].
Restricting the root to Stream/Output/Audio costs nothing real —
peerspeak only ever tags playback streams — and the AEC's virtual
sink/source is untouched, since it roots on module id, not on this tag.
A tag that is ignored is not silent: misplaced_ownership_tags feeds a
new `ignored_ownership_tags` audit field (omitted when empty), because
the fix *removes* an exclusion, and the two causes of a dropped tag —
a peerspeak tagging bug, or an impersonation attempt — both want seeing.
Codex phase-1 review F2, reproduced live. Round 10, R10-1.
5 new rows, mutation-verified: dropping the role restriction kills both
engine rows, and stubbing the diagnostic kills the third.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex phase-1 review, finding 3 (P2), concrete half. This side searched
a list and took the first match for a key; peerspeak's side collected
into a map and took the last. A byte-identical fixture containing a
duplicated key would therefore leave both suites green while the two
repos had selected *different* contracts — the precise drift the shared
file exists to prevent.
Both sides now assert the key is not already defined. Verified by
appending a duplicate `prop_value` to both fixtures: both suites fail.
The rest of finding 3 — one CI gate that feeds peerspeak's real
tag_child output through this repo's actual adapter and classifier,
rather than two per-repo literal tests — is a larger piece of work and
is not attempted here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Verification-round follow-up to 1b01847. The phase-3r live row proves
carrier 1 survives the bind, which is the property F1 destroyed — but
it tagged its fixture sink with `peerspeak.owned=true`, not the `1`
the contract pins and peerspeak actually emits. It would have passed
even if the real literal did not.
Adds PEERSPEAK_OWNED_VALUE so the fixture can name the producer's
value, and asserts it against the shared contract file alongside the
other two literals. The sink's name still deliberately avoids the
`peerspeak_owned_` prefix, so carrier 2 cannot stand in for carrier 1
in that row.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The consumer half of phase 1 (plan §5.1, impl plan §3). The engine's
tag root becomes a union: `peerspeak.owned` truthy OR `node.name`
starting with `peerspeak_owned_`. Round 8 added the second carrier
because a node property is invisible to the registry `global` event
and recoverable only by binding the node — which is exactly how the
phase-5 gate failed — while `node.name` is announced directly.
The union lives in `local_root_reason`, not in the adapter. Folding
both into the one `peerspeak_owned` bool at the observation boundary
would make each carrier untestable alone, which is the phase-3r
lesson: a gate asserting a value two sources can satisfy gates
neither. The existing `peerspeak_tagged_nodes_…` fixture now carries
both carriers, so it would keep passing if either were deleted; two
new tests pin them individually, and a third pins that the prefix
matches only at the start of a name.
Both literals are now named constants — they are a cross-repo wire
contract with peerspeak, not local naming — and asserted against
tests/fixtures/ownership-tag-contract.txt, committed byte-identical
in both repos. That test also runs the fixture's own worked example
name through the engine, so the shared file cannot document a value
this side does not actually exclude.
Five mutations verified: drop either carrier, loosen `starts_with` to
`contains`, or rename either constant, and exactly the intended test
fails.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Found by the phase-5 audit minutes after phase 3r landed — a permanent sticky
taint on a hardware sink, from one link seen during enumeration. Sticky state
is now built from an evidence-only pass; decisions still fail closed.
v3.5 §6.7, the fix for the phase-5 gate failure (F1/F2). Pure core + adapter,
each half reviewed by the other author; four-part exit gate passed, including
row 1 live on this host, plus an added live gate for the Device-side path.
Codex's adversarial review of the pure core found no *certain* P1. Two
findings taken, both mutation-verified (the fix reverted, the intended test
dies, nothing else moves):
**F2, certain, P2 — `device_props` tested the wrong kind of ambiguity.** It
required exactly one live *Device* on the claimed id rather than exactly one
live *global*. With `[Device, Port]` on one id — a missed removal, the same
precondition as every other recycled-id hazard — it kept answering from the
older Device, so a node claiming that id held a stale `session_device = true`.
That flag strips the node's owner keys and its fail-closed backstop, so a
forwarder wearing it can put its output leg back on the eligible side. Now:
one slot total, and it must be the Device.
**F3, worth checking, P3 — `device.api` was corroborating by presence.**
`device.api=v4l2` under `factory.name=api.alsa.pcm.sink` satisfied the
positive classifier. No truthful configuration produces that pair, which is
the argument for reading it as an observation gone wrong rather than as
corroboration. The API must now equal the one the factory allowlist is
written for, an empty value is not a value, and the two sides disagreeing
fails closed. Tied to the allowlist being ALSA-only via a named constant.
Two findings NOT fixed here, both pre-existing and neither introduced by
round 8 — raised to the design doc instead:
- **P1, worth checking: hardware playback-to-capture paths** (Stereo Mix,
Digital Loopback) on a card whose driver is an ordinary `snd_hda_intel`.
Both its sink and source classify `session_device`, taint cannot cross the
hardware hop, and a capture app reading that source can re-emit the call.
This is `snd_aloop` again in a form the driver name cannot detect;
distinguishing it needs ALSA control inspection, which is a design change
and a new I/O surface, not a local fix.
- **P3: the 2 s readiness budget** can in principle never see an
obligation-free instant under sustained startup churn, and `TimedOut` is
sticky by design, so the process would be silent for its lifetime.
Measured here: readiness at ~3 ms with 19 binds, so the margin is three
orders of magnitude — but it wants a calibration argument, not a guess.
197 unit + 3 live green, clippy -D warnings and fmt clean.
mollusk
