From 4b2b192601569233419433f4a7e059ffacde9b9e Mon Sep 17 00:00:00 2001 From: Mollusk Date: Sun, 26 Jul 2026 00:00:38 -0400 Subject: [PATCH] host/taint: correct the constant's own doc, and record the ProcessId ambiguity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/host/taint/mod.rs | 18 ++++++++++++------ src/host/taint/owner.rs | 10 ++++++++++ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/host/taint/mod.rs b/src/host/taint/mod.rs index 76a8b45..a9e1054 100644 --- a/src/host/taint/mod.rs +++ b/src/host/taint/mod.rs @@ -136,13 +136,19 @@ pub const ECHO_CANCEL_GROUP_PREFIX: &str = "echo-cancel-"; /// change that invalidates the phase 5 matrix. pub const PEERSPEAK_OWNED_PROP: &str = "peerspeak.owned"; -/// The value peerspeak emits for [`PEERSPEAK_OWNED_PROP`]. +/// The value peerspeak emits for [`PEERSPEAK_OWNED_PROP`], and the **only** +/// value this consumer reads as owned. /// -/// The consumer deliberately accepts **any** truthy value, not just this one -/// (see the observer's `truthy`) — treating an unexpected value as "owned" is -/// the fail-closed direction. This constant exists so live tests can tag a -/// node with exactly what the producer sends, rather than something merely -/// truthy that would pass even if the real literal did not. +/// ⚠️ This doc used to say the opposite — that any truthy value counted, on +/// the theory that treating an unexpected value as "owned" is the fail-closed +/// direction. R10-4 removed that leniency and the round-10 review caught the +/// prose surviving it here and in the shared fixture. The theory is wrong: +/// leniency buys false-positive *exclusion*, not safety, and it let any +/// process suppress a rival application's audio from the share with a +/// property it did not have to spell right. Fail-closed on this feature is +/// about **ancestry** — an unresolvable graph is not eligible — not about +/// parsing. The matching lives in the observer's `peerspeak_owned`, which is +/// deliberately *not* the lenient `truthy` used for PipeWire's own booleans. pub const PEERSPEAK_OWNED_VALUE: &str = "1"; /// Ownership carrier 2: a `node.name` prefix (v3.5 §5.1, round 8). diff --git a/src/host/taint/owner.rs b/src/host/taint/owner.rs index 4727dd5..7e71fd4 100644 --- a/src/host/taint/owner.rs +++ b/src/host/taint/owner.rs @@ -145,6 +145,16 @@ pub enum OwnerKey { impl OwnerKey { /// Stable, machine-readable — this ends up in the phase 5 audit output /// and the phase 6 status event. + /// + /// ⚠️ **Known imprecision, deliberately not fixed here.** `ProcessId` now + /// covers two sources — the node's `application.process.id` and its + /// Client's `pipewire.sec.pid` (see [`keys_of`]) — so a bridge reported as + /// `application.process.id` may in fact have resolved on the Client's + /// protected pid. Pre-existing since R10-3 made the Client a fallback, and + /// widened by the review's finding 1 making it a union. Splitting it would + /// add a code to a set that is explicitly a stable contract for the audit + /// output and the "why isn't this app being shared?" answer, so it wants + /// its own decision rather than a drive-by. pub fn code(self) -> &'static str { match self { Self::LinkGroup => "node.link-group",