host/taint: close five leaks found in Codex round 1
All five were reachable, all five now have a regression test, and each test was verified by injecting the mutation that reverts its fix. 1. Sticky taint ignored surviving Client members. An app can close every stream while keeping its PipeWire connection open and then open a new one — Firefox does this constantly — and the new leg came back Eligible while the owner's buffers still held the call. Sticky seeding now resolves live Client serials to their current nodes. 2. "Receives audio" was inferred from `media.class` alone, so a node with an absent or unexpected class sitting on a real inbound link could not start an owner bridge and its sibling re-emitted the call. A node is now a receiver if it appears as a resolved `link.input.node` OR has a receiving role. 3. The device-node coarse-key exception was keyed on `media.class` being `Audio/Sink|Source|Duplex`, which also stripped the only correlation a *native virtual sink* has (own client, no link-group, no module id). Now keyed on `device.id`, measured on the live graph as the exact discriminator: the 5 ALSA nodes carry device.id 43/45/46 and share `client.id` 42 (`WirePlumber [export]`); the 3 `support.null-audio-sink` nodes carry no device.id and hold their own clients. 4. The unbounded-owner backstop required the tainted *reader* to be unbounded. Properties can be asymmetric — a reader with a link-group whose re-emitting leg has none is bounded while its sibling is not findable — so that condition is dropped; targets stay restricted to unbounded output legs, which keeps the blast radius small. 5. A not-ready snapshot could retire sticky owners, erasing taint history on the strength of a graph already declared untrustworthy. `evaluate` now returns the prior state unchanged while `!graph_ready`. Test-quality findings, also fixed: - a single pass of each rule survived all 32 tests (every fixture needed at most one owner hop) → two-chained-forwarder test with a clean control, plus a 60-layer chain to catch an accidental blow-up - first-write-wins `raise()` survived → a node reached by bridge on one pass and by a direct link on the next must report the stronger reason - `drop_clients` left the fixture's client caches stale, so "a fresh client after teardown" was really a dangling id; the recycling row now reuses node id, client id AND `pulse.module.id` verbatim Also corrected the cost claim: this is O((V+E)·D) for owner-bridge depth D, not O(V+E) as v3.4 §6.4 states. Owner keys are now computed once per snapshot instead of per candidate pair. 42 tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Claude Opus 4.8
@@ -924,3 +924,354 @@ fn recycled_ids_module_indices_and_link_groups_do_not_inherit_taint() {
|
||||
&[("call", call, "peerspeak-owned")],
|
||||
);
|
||||
}
|
||||
|
||||
// ──────────────────────────────────────────────────────────────────────
|
||||
// Regressions from Codex round 1 — each of these was a leak or a lost
|
||||
// taint history, and each mutation-tests the fix that closed it.
|
||||
// ──────────────────────────────────────────────────────────────────────
|
||||
|
||||
#[test]
|
||||
fn sticky_taint_survives_when_only_the_client_object_remains() {
|
||||
// Finding 1. An app can close every stream it holds while keeping its
|
||||
// PipeWire connection open, then open a fresh one — Firefox does this
|
||||
// constantly. Seeding sticky taint only from live *nodes* let the new
|
||||
// leg come back Eligible while the owner's buffers were still full of
|
||||
// the call.
|
||||
let (mut graph, call, rec_in, rec_out, firefox) = sticky_scene();
|
||||
let c = ctx();
|
||||
let (_, sticky) = evaluate(&graph.build(), &c, &StickyState::default());
|
||||
|
||||
// Both legs vanish; the client object does not.
|
||||
let late = graph.app_node("rec-out-late", MediaRole::StreamOutput, 8080);
|
||||
let (next, _) = evaluate(&graph.build_without(&[rec_in, rec_out]), &c, &sticky);
|
||||
assert_partition(
|
||||
&next,
|
||||
&[("firefox", firefox)],
|
||||
&[
|
||||
("call", call, "peerspeak-owned"),
|
||||
("late", late, "tainted-owner-bridge"),
|
||||
],
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_inbound_linked_node_of_unknown_role_still_bridges() {
|
||||
// Finding 2. "Receives audio" cannot be inferred from `media.class`
|
||||
// alone: a node with an absent or unexpected class sits on a real
|
||||
// inbound link carrying the call, and if it cannot start an owner
|
||||
// bridge its sibling output leg re-emits the call as Eligible.
|
||||
let mut graph = Graph::new();
|
||||
let sink = graph.device_node("hw-sink", MediaRole::Sink);
|
||||
let call = graph.peerspeak_node("peerspeak", 7);
|
||||
graph.link(call, sink);
|
||||
|
||||
let client = graph.client_of_app(8080);
|
||||
let unknown_in = graph.node("unknown-in", MediaRole::Other, app(client, 8080));
|
||||
let leaked_out = graph.node("leaked-out", MediaRole::StreamOutput, app(client, 8080));
|
||||
graph.link(sink, unknown_in);
|
||||
|
||||
let decisions = run(&graph, &ctx());
|
||||
assert_partition(
|
||||
&decisions,
|
||||
&[],
|
||||
&[
|
||||
("call", call, "peerspeak-owned"),
|
||||
("leaked-out", leaked_out, "tainted-owner-bridge"),
|
||||
],
|
||||
);
|
||||
assert_tainted(&decisions, unknown_in, "tainted-upstream");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_candidate_that_is_itself_a_receiver_still_bridges_to_its_siblings() {
|
||||
// The other half of finding 2: a `Stream/Output/Audio` on an inbound
|
||||
// link is excluded by the link walk, but it must also be able to carry
|
||||
// taint across the owner bridge to its siblings.
|
||||
let mut graph = Graph::new();
|
||||
let sink = graph.device_node("hw-sink", MediaRole::Sink);
|
||||
let call = graph.peerspeak_node("peerspeak", 7);
|
||||
graph.link(call, sink);
|
||||
|
||||
let odd_in = graph.app_node("odd-in", MediaRole::StreamOutput, 9100);
|
||||
let sibling = graph.app_node("sibling-out", MediaRole::StreamOutput, 9100);
|
||||
graph.link(sink, odd_in);
|
||||
|
||||
assert_partition(
|
||||
&run(&graph, &ctx()),
|
||||
&[],
|
||||
&[
|
||||
("call", call, "peerspeak-owned"),
|
||||
("odd-in", odd_in, "tainted-upstream"),
|
||||
("sibling", sibling, "tainted-owner-bridge"),
|
||||
],
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_native_virtual_sink_bridges_on_its_client() {
|
||||
// Finding 3. The device exception is keyed on `device.id`, not on the
|
||||
// node's role: an app that creates its own `Audio/Sink` plus a
|
||||
// re-emitting stream on one client has no `device.id`, no link-group
|
||||
// and no `pulse.module.id`, so `client.id` is the *only* thing relating
|
||||
// the two. Suppressing coarse keys by role stripped it and leaked the
|
||||
// whole call.
|
||||
let mut graph = Graph::new();
|
||||
let hw = graph.device_node("hw", MediaRole::Sink);
|
||||
let call = graph.peerspeak_node("peerspeak", 7);
|
||||
let virtual_sink = graph.native_virtual_node("native-virtual-sink", MediaRole::Sink, 4321);
|
||||
let reemitter = graph.native_virtual_node("native-output", MediaRole::StreamOutput, 4321);
|
||||
graph.link(call, virtual_sink);
|
||||
graph.link(reemitter, hw);
|
||||
|
||||
let decisions = run(&graph, &ctx());
|
||||
assert_partition(
|
||||
&decisions,
|
||||
&[],
|
||||
&[
|
||||
("call", call, "peerspeak-owned"),
|
||||
("reemitter", reemitter, "tainted-owner-bridge"),
|
||||
],
|
||||
);
|
||||
assert_eq!(
|
||||
decisions.taint[&reemitter.serial].reason,
|
||||
Reason::TaintedOwnerBridge {
|
||||
key: Some(OwnerKey::ClientId)
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_asymmetric_forwarder_fails_closed() {
|
||||
// Finding 4. The reader carries a `node.link-group` and is therefore
|
||||
// "bounded", but its re-emitting leg carries none, so the strong key
|
||||
// finds no sibling. Requiring the *source* to be unbounded let this
|
||||
// through as Eligible.
|
||||
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 in_client = graph.client(Some(PULSE_PID));
|
||||
let out_client = graph.client(Some(PULSE_PID));
|
||||
let fwd_in = graph.node(
|
||||
"fwd-in",
|
||||
MediaRole::StreamInput,
|
||||
NodeProps {
|
||||
link_group: Some("asymmetric-forwarder".into()),
|
||||
client_id: Some(in_client),
|
||||
process_id: Some(PULSE_PID),
|
||||
..NodeProps::default()
|
||||
},
|
||||
);
|
||||
let fwd_out = graph.node(
|
||||
"fwd-out",
|
||||
MediaRole::StreamOutput,
|
||||
app(out_client, PULSE_PID),
|
||||
);
|
||||
graph.link(hw, fwd_in);
|
||||
|
||||
// Blast radius control: a real app is bounded by its own PID.
|
||||
let firefox = graph.app_node("firefox", MediaRole::StreamOutput, 11114);
|
||||
graph.link(firefox, hw);
|
||||
|
||||
assert_partition(
|
||||
&run(&graph, &ctx()),
|
||||
&[("firefox", firefox)],
|
||||
&[
|
||||
("call", call, "peerspeak-owned"),
|
||||
("fwd-out", fwd_out, "unresolved-owner"),
|
||||
],
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_not_ready_snapshot_never_retires_sticky_owners() {
|
||||
// Finding 5. `graph_ready == false` says "this view of the graph is not
|
||||
// trustworthy". Retiring sticky owners because their members are absent
|
||||
// from an untrustworthy snapshot erases the taint history, and the next
|
||||
// ready recompute hands back a clean bill of health for an owner that
|
||||
// never went away.
|
||||
let (graph, call, rec_in, rec_out, firefox) = sticky_scene();
|
||||
let c = ctx();
|
||||
let (_, sticky) = evaluate(&graph.build(), &c, &StickyState::default());
|
||||
|
||||
let not_ready = ExclusionCtx {
|
||||
graph_ready: false,
|
||||
..ctx()
|
||||
};
|
||||
let (_, carried) = evaluate(
|
||||
&super::snapshot::GraphSnapshot::default(),
|
||||
¬_ready,
|
||||
&sticky,
|
||||
);
|
||||
assert_eq!(
|
||||
carried, sticky,
|
||||
"a not-ready recompute must not edit history"
|
||||
);
|
||||
|
||||
let (after, _) = evaluate(&graph.build_without(&[rec_in]), &c, &carried);
|
||||
assert_partition(
|
||||
&after,
|
||||
&[("firefox", firefox)],
|
||||
&[
|
||||
("call", call, "peerspeak-owned"),
|
||||
("rec-out", rec_out, "tainted-owner-bridge"),
|
||||
],
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn taint_crosses_two_chained_forwarders() {
|
||||
// Finding 6. Every earlier fixture needed at most one owner hop, so
|
||||
// replacing the fixpoint with a single pass of each rule survived the
|
||||
// whole suite. Two chained forwarders need two.
|
||||
let mut graph = Graph::new();
|
||||
let hw = graph.device_node("hw-sink", MediaRole::Sink);
|
||||
let call = graph.peerspeak_node("peerspeak", 7);
|
||||
let sink1 = graph.module_node("sink1", MediaRole::Sink, 100);
|
||||
graph.link(call, sink1);
|
||||
|
||||
let fwd1_in = graph.module_node("fwd1-in", MediaRole::StreamInput, 101);
|
||||
let fwd1_out = graph.module_node("fwd1-out", MediaRole::StreamOutput, 101);
|
||||
graph.link(sink1, fwd1_in);
|
||||
|
||||
let sink2 = graph.module_node("sink2", MediaRole::Sink, 102);
|
||||
graph.link(fwd1_out, sink2);
|
||||
let fwd2_in = graph.module_node("fwd2-in", MediaRole::StreamInput, 103);
|
||||
let fwd2_out = graph.module_node("fwd2-out", MediaRole::StreamOutput, 103);
|
||||
graph.link(sink2, fwd2_in);
|
||||
graph.link(fwd2_out, hw);
|
||||
|
||||
// Negative control: an unrelated clean forwarder chain of the same shape.
|
||||
let clean_sink = graph.device_node("clean-sink", MediaRole::Sink);
|
||||
let clean_in = graph.module_node("clean-in", MediaRole::StreamInput, 200);
|
||||
let clean_out = graph.module_node("clean-out", MediaRole::StreamOutput, 200);
|
||||
graph.link(clean_sink, clean_in);
|
||||
graph.link(clean_out, hw);
|
||||
|
||||
assert_partition(
|
||||
&run(&graph, &ctx()),
|
||||
&[("clean-out", clean_out)],
|
||||
&[
|
||||
("call", call, "peerspeak-owned"),
|
||||
("fwd1-out", fwd1_out, "tainted-owner-bridge"),
|
||||
("fwd2-out", fwd2_out, "tainted-owner-bridge"),
|
||||
],
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_stronger_reason_replaces_a_weaker_one_regardless_of_arrival_order() {
|
||||
// Finding 7. The old determinism test reversed two links that never
|
||||
// competed, so a first-write-wins `raise()` survived it. Here a node is
|
||||
// reached by the owner bridge on the first fixpoint pass and by a
|
||||
// direct tainted link on the second; the direct link is the stronger
|
||||
// reason and must win.
|
||||
let build = |reverse: bool| {
|
||||
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 rec_in = graph.app_node("rec-in", MediaRole::StreamInput, 5000);
|
||||
let mid = graph.app_node("mid-out", MediaRole::StreamOutput, 5000);
|
||||
let owned_sink = graph.native_virtual_node("owned-sink", MediaRole::Sink, 5000);
|
||||
if reverse {
|
||||
graph.link(mid, owned_sink);
|
||||
graph.link(hw, rec_in);
|
||||
} else {
|
||||
graph.link(hw, rec_in);
|
||||
graph.link(mid, owned_sink);
|
||||
}
|
||||
let decisions = run(&graph, &ctx());
|
||||
(
|
||||
decisions.taint[&owned_sink.serial].reason.code(),
|
||||
decisions.excluded(),
|
||||
)
|
||||
};
|
||||
let forward = build(false);
|
||||
assert_eq!(
|
||||
forward.0, "tainted-upstream",
|
||||
"bridged first, then reached directly — the direct link is stronger"
|
||||
);
|
||||
assert_eq!(forward, build(true), "and it must not depend on link order");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_recycled_client_id_and_module_index_do_not_inherit_taint() {
|
||||
// Finding 8. The original recycling test never reused a
|
||||
// `pulse.module.id`, and its "fresh" client silently reused the dead
|
||||
// client's global id because the fixture cache was stale — so the node
|
||||
// pointed at a client object that no longer existed.
|
||||
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 old_in = graph.module_node("old-in", MediaRole::StreamInput, 536_870_919);
|
||||
let old_out = graph.module_node("old-out", MediaRole::StreamOutput, 536_870_919);
|
||||
graph.link(hw, old_in);
|
||||
|
||||
let c = ctx();
|
||||
let (first, sticky) = evaluate(&graph.build(), &c, &StickyState::default());
|
||||
assert_eq!(
|
||||
first.candidates[&old_out.serial].reason().map(Reason::code),
|
||||
Some("tainted-owner-bridge")
|
||||
);
|
||||
|
||||
// Full teardown, then a module that reuses the node id, the client id
|
||||
// and the module index verbatim. Only the serials are new — which is
|
||||
// the entire reason stickiness is keyed on them.
|
||||
let old_client = graph.client_of_module(536_870_919);
|
||||
graph.drop_clients(&[old_client]);
|
||||
let reborn_client = graph.client_with_id(old_client, Some(PULSE_PID));
|
||||
let reborn = graph.node_with_id(
|
||||
"reborn-out",
|
||||
MediaRole::StreamOutput,
|
||||
old_out.id,
|
||||
super::fixture::pulse_module(reborn_client, 536_870_919, PULSE_PID),
|
||||
);
|
||||
let (second, _) = evaluate(&graph.build_without(&[old_in, old_out]), &c, &sticky);
|
||||
assert_partition(
|
||||
&second,
|
||||
&[("reborn", reborn)],
|
||||
&[("call", call, "peerspeak-owned")],
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_deep_forwarder_chain_converges() {
|
||||
// Finding 9. The fixpoint is monotone so it terminates, but each pass
|
||||
// re-runs a full link BFS and a full owner scan, so a chain of owner
|
||||
// hops costs one pass per layer. This is the shape that would expose an
|
||||
// accidental blow-up; phase 5 measures the real distribution.
|
||||
const LAYERS: u64 = 60;
|
||||
let mut graph = Graph::new();
|
||||
let hw = graph.device_node("hw-sink", MediaRole::Sink);
|
||||
let call = graph.peerspeak_node("peerspeak", 7);
|
||||
let mut upstream = graph.module_node("sink-0", MediaRole::Sink, 1);
|
||||
graph.link(call, upstream);
|
||||
let mut last_out = None;
|
||||
for layer in 1..=LAYERS {
|
||||
let module = 1000 + layer;
|
||||
let leg_in = graph.module_node(&format!("fwd-{layer}-in"), MediaRole::StreamInput, module);
|
||||
let leg_out =
|
||||
graph.module_node(&format!("fwd-{layer}-out"), MediaRole::StreamOutput, module);
|
||||
graph.link(upstream, leg_in);
|
||||
let next = graph.module_node(&format!("sink-{layer}"), MediaRole::Sink, 2000 + layer);
|
||||
graph.link(leg_out, next);
|
||||
upstream = next;
|
||||
last_out = Some(leg_out);
|
||||
}
|
||||
graph.link(last_out.expect("at least one layer"), hw);
|
||||
|
||||
let decisions = run(&graph, &ctx());
|
||||
assert_eq!(
|
||||
decisions.eligible(),
|
||||
Vec::new(),
|
||||
"every leg of the chain carries the call"
|
||||
);
|
||||
assert_eq!(
|
||||
decisions.candidates.len(),
|
||||
LAYERS as usize + 1,
|
||||
"the whole chain plus peerspeak's own playback"
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user