host/taint: close the partial fixes found in Codex round 2
The verification round earned its place: five of the six round-1 fixes were partial, and two of the gaps were worse than the bugs they replaced. 1. ⚠️ The round-1 sticky fix smuggled the suppressed key back in. `client_serials_of` recorded the shared `WirePlumber [export]` client as a member of a tainted hardware sink's owner, so the *second* recompute expanded that client to every sound card on the box, tainted the microphone, and excluded every app holding one — the §6.1.1 catastrophe arriving one epoch late instead of never. `client.id` may now only be recorded, or expanded, for nodes where it is a usable owner key. The regression test evaluates an unchanged snapshot three times: a correct engine's answer must not drift when nothing has. 2. Sticky followed a surviving *connection*, not a surviving *owner*. A process can leave one client idle and open a second — GStreamer opens one per stream as a matter of course — and the new leg escaped. `StickyOwner` now carries owner **fingerprints** (strong keys and a usable PID, never `client.id`), applied only while some serial member is still live, so a recyclable key cannot resurrect a dead owner. 3. An **ambiguous** link input endpoint tainted every claimant but made none of them a receiver, so their sibling output legs stayed eligible. Taint without receiver status cannot start an owner bridge. 4. `device.id` is a raw observation, not the classification the coarse-key exception needs — PipeWire defines it only as "the Device this node belongs to", so a forwarding node carrying one would have lost both its owner keys and its ability to trip the backstop. Replaced by `session_device`, a phase-3 obligation (`device.id` AND `device.api`) documented to fail closed when it cannot classify. 5. Readiness now gates sticky **retirement only**. Round 1 stopped a not-ready epoch erasing history; it also stopped it recording any, so a reader could consume and buffer the call during that epoch, vanish before readiness, and leave its output eligible. 6. Added the unresolved-output-plus-unknown-role fixture: deleting one `receivers.insert` survived all 42 previous tests. Mutation-verified: 7/7 reverts killed by their intended test. Two attempts did not land first time and both were my error, not the engine's — the client-key guard is applied at two sites so removing one is not a revert (removing the pair is, and that is killed), and the fingerprint-lifetime test put the recycled node in a snapshot *after* the entry had already been retired, so the guard was never consulted. Rewritten to place it in the same snapshot that first sees the owner gone. Cost comment corrected again, to O(D·(V+E+Σ|sources|·|targets|)). 49 tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+67
-8
@@ -114,9 +114,9 @@ fn keys_of(node: &NodeSnapshot, pipewire_pulse_pid: Option<u32>) -> Vec<(OwnerKe
|
||||
if let Some(module) = node.props.pulse_module_id {
|
||||
out.push((OwnerKey::PulseModuleId, KeyValue::Num(module)));
|
||||
}
|
||||
// Exception 2: coarse keys never bridge nodes exported from a real
|
||||
// Device — they all share the session manager's client.
|
||||
if node.props.device_id.is_some() {
|
||||
// Exception 2: coarse keys never bridge passive session-manager device
|
||||
// nodes — they all share the session manager's client.
|
||||
if node.props.session_device {
|
||||
return out;
|
||||
}
|
||||
if let Some(client) = node.props.client_id {
|
||||
@@ -191,6 +191,53 @@ impl OwnerKeyIndex {
|
||||
})
|
||||
}
|
||||
|
||||
/// Is `client.id` a usable owner key for this node?
|
||||
///
|
||||
/// ⚠️ Load-bearing for sticky state. A device node's `client.id` is
|
||||
/// suppressed by exception 2, so recording the session manager's Client
|
||||
/// as a *member* of a tainted device's sticky owner would smuggle the
|
||||
/// suppressed key back in: the next recompute would expand that Client
|
||||
/// to every hardware node on the box — the microphone included — and
|
||||
/// the §6.1.1 catastrophe would arrive one epoch late instead of never.
|
||||
/// (Codex round 2, finding 1.)
|
||||
pub fn uses_client_key(&self, serial: Serial) -> bool {
|
||||
self.keys
|
||||
.get(&serial)
|
||||
.is_some_and(|keys| keys.iter().any(|(key, _)| *key == OwnerKey::ClientId))
|
||||
}
|
||||
|
||||
/// The owner keys that are safe to remember *across* connections, for
|
||||
/// sticky taint: the strong keys plus a usable process id.
|
||||
///
|
||||
/// `client.id` is deliberately excluded — it identifies a *connection*,
|
||||
/// and the whole point of a fingerprint is to survive one process
|
||||
/// closing a connection and opening another. A live Client member is
|
||||
/// what covers the same-connection case, precisely.
|
||||
///
|
||||
/// These are recyclable strings and numbers, so they are only ever
|
||||
/// applied while some **serial** member of the owner is still live
|
||||
/// (v3.4 §6.1.3): while the process is alive, its PID cannot have been
|
||||
/// handed to anyone else.
|
||||
pub fn fingerprints(&self, serial: Serial) -> Vec<Fingerprint> {
|
||||
self.keys
|
||||
.get(&serial)
|
||||
.map(|keys| {
|
||||
keys.iter()
|
||||
.filter(|(key, _)| *key != OwnerKey::ClientId)
|
||||
.map(|(key, value)| Fingerprint(*key, value.clone()))
|
||||
.collect()
|
||||
})
|
||||
.unwrap_or_default()
|
||||
}
|
||||
|
||||
/// Does this node currently present `fingerprint`?
|
||||
pub fn has_fingerprint(&self, serial: Serial, fingerprint: &Fingerprint) -> bool {
|
||||
self.keys.get(&serial).is_some_and(|keys| {
|
||||
keys.iter()
|
||||
.any(|(key, value)| *key == fingerprint.0 && *value == fingerprint.1)
|
||||
})
|
||||
}
|
||||
|
||||
/// See [`owner_is_bounded`].
|
||||
pub fn is_bounded(&self, serial: Serial) -> bool {
|
||||
self.keys
|
||||
@@ -218,6 +265,10 @@ pub fn strongest_shared_key(
|
||||
})
|
||||
}
|
||||
|
||||
/// A remembered owner key — see [`OwnerKeyIndex::fingerprints`].
|
||||
#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Debug)]
|
||||
pub struct Fingerprint(OwnerKey, KeyValue);
|
||||
|
||||
/// Nodes partitioned into owner components.
|
||||
#[derive(Clone, Debug, Default)]
|
||||
pub struct OwnerComponents {
|
||||
@@ -315,15 +366,23 @@ impl UnionFind {
|
||||
/// Client objects belonging to an owner component, so sticky taint can be
|
||||
/// keyed on every object that constitutes the owner (v3.4 §6.1.3: clear the
|
||||
/// entry only once **all** member objects are gone).
|
||||
pub fn client_serials_of(snapshot: &GraphSnapshot, nodes: &[Serial]) -> Vec<Serial> {
|
||||
pub fn client_serials_of(
|
||||
snapshot: &GraphSnapshot,
|
||||
keys: &OwnerKeyIndex,
|
||||
nodes: &[Serial],
|
||||
) -> Vec<Serial> {
|
||||
let mut out: Vec<Serial> = nodes
|
||||
.iter()
|
||||
// Only nodes for which `client.id` is a *usable* owner key. See
|
||||
// `uses_client_key`: recording a device node's shared session-manager
|
||||
// Client here would defeat exception 2 on the next recompute.
|
||||
.filter(|serial| keys.uses_client_key(**serial))
|
||||
.filter_map(|serial| snapshot.node(*serial))
|
||||
.filter_map(|node| node.props.client_id)
|
||||
.filter_map(|id: GlobalId| match snapshot.client_by_id(id) {
|
||||
Some(super::snapshot::IdLookup::Unique(serial)) => Some(serial),
|
||||
_ => None,
|
||||
})
|
||||
// An ambiguous client id means two Clients claim it and we cannot
|
||||
// say which one is ours, so remember both: an entry that recorded
|
||||
// neither could be retired while its owner was still live.
|
||||
.flat_map(|id: GlobalId| snapshot.clients_with_id(id).map(|client| client.serial))
|
||||
.collect();
|
||||
out.sort_unstable();
|
||||
out.dedup();
|
||||
|
||||
Reference in New Issue
Block a user