From 45b1b97dd8ba02346b9ee4bebd7eb1d543c934b7 Mon Sep 17 00:00:00 2001
From: Mollusk <silvernode@gmail.com>
Date: Sun, 26 Jul 2026 00:00:23 -0400
Subject: [PATCH] audio/ownership: pin the no-lost-carrier invariant, and
 harden the byte scan
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Verification round on the round-10 review fixes.

Adds the property the whole of finding 3 is about, stated directly: over
20,000 deterministic inputs built from the exact characters that break
SPA-JSON (braces, brackets, quotes, separators, comment marks, escapes,
newlines, multi-byte characters), the merge always emits both carriers in an
object it can read back. Either outcome — parse and rebuild, or overwrite —
has to end that way, and now nothing can quietly change which.

Also replaces two byte-index steps with character-boundary steps. Both were
correct on the ASCII input they actually see, but `index + 1` after a
reverse find would have split a multi-byte character and panicked the slice.
scan_token gains multi-byte cases for the same reason.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 src/audio/ownership.rs | 86 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 83 insertions(+), 3 deletions(-)

diff --git a/src/audio/ownership.rs b/src/audio/ownership.rs
index fbb1b0d..44f12f5 100644
--- a/src/audio/ownership.rs
+++ b/src/audio/ownership.rs
@@ -872,10 +872,13 @@ mod tests {
             .expect("main has a body");
         // Back up over the module path the call is written with, so the
         // statement's own `peerspeak::audio::ownership::` is not mistaken for
-        // code preceding it.
+        // code preceding it. Stepping by `len_utf8` rather than by one byte
+        // keeps the slice below on a character boundary whatever precedes it.
         let call_start = code[..call]
-            .rfind(|c: char| !(c.is_alphanumeric() || c == '_' || c == ':'))
-            .map_or(0, |index| index + 1);
+            .char_indices()
+            .rev()
+            .find(|(_, c)| !(c.is_alphanumeric() || *c == '_' || *c == ':'))
+            .map_or(0, |(index, c)| index + c.len_utf8());
         assert!(
             call_start > body,
             "the call must be inside main, not above it"
@@ -1173,6 +1176,13 @@ mod tests {
     fn scan_token_returns_only_complete_tokens() {
         let complete = [
             ("\"music\"", "\"music\""),
+            // Multi-byte characters: the scan steps by bytes, so a token
+            // boundary landing inside one would panic the slice. It cannot —
+            // UTF-8 continuation bytes are never ASCII — and this pins it.
+            ("\"café ☕\" rest", "\"café ☕\""),
+            ("café", "café"),
+            ("\"\\é\" rest", "\"\\é\""),
+            ("[ \"ünïcode\" ] rest", "[ \"ünïcode\" ]"),
             // An escaped quote does not end the string.
             ("\"we\\\"ird\" rest", "\"we\\\"ird\""),
             // Brackets inside a string are not structure.
@@ -1216,6 +1226,76 @@ mod tests {
         }
     }
 
+    /// **The invariant the whole finding is about: no inherited value, however
+    /// hostile, may cost us a carrier.**
+    ///
+    /// The merge has two outcomes — parse and rebuild, or overwrite — and both
+    /// are supposed to end with our two pairs present in a well-formed object.
+    /// A deterministic walk over an alphabet of the exact characters that
+    /// break SPA-JSON (braces, brackets, quotes, separators, comment marks,
+    /// escapes, newlines, multi-byte characters) checks that directly, and
+    /// checks the result parses — an output our own parser rejects would be an
+    /// output the daemon may well truncate, which is the live failure this
+    /// finding began with.
+    #[test]
+    fn no_inherited_value_can_cost_us_a_carrier() {
+        let name = owned_node_name(CLIP_ROLE);
+        let alphabet = [
+            "{",
+            "}",
+            "[",
+            "]",
+            "\"",
+            "=",
+            ":",
+            ",",
+            "#",
+            "\\",
+            " ",
+            "\n",
+            "\t",
+            "a",
+            "1",
+            "é",
+            "☕",
+            "peerspeak.owned",
+            "node.name",
+            "media.role",
+            "true",
+        ];
+        // A fixed seed: a failure here must be reproducible, not a flake.
+        let mut state: u64 = 0x2545_f491_4f6c_dd1d;
+        let mut next = move || {
+            state = state
+                .wrapping_mul(6_364_136_223_846_793_005)
+                .wrapping_add(1_442_695_040_888_963_407);
+            (state >> 33) as usize
+        };
+
+        for _ in 0..20_000 {
+            let length = next() % 14;
+            let mut inherited = String::new();
+            for _ in 0..length {
+                inherited.push_str(alphabet[next() % alphabet.len()]);
+            }
+
+            let merged = merge_pipewire_props(Some(&inherited), &name, PIPEWIRE_ALSA_ENV);
+            assert!(
+                merged.contains(&format!("\"{OWNED_PROP_KEY}\" = \"{OWNED_PROP_VALUE}\"")),
+                "carrier 1 lost for inherited {inherited:?}: {merged}"
+            );
+            assert!(
+                merged.contains(&format!("\"node.name\" = \"{name}\"")),
+                "carrier 2 lost for inherited {inherited:?}: {merged}"
+            );
+            // And whatever we emit must be an object we can read back.
+            assert!(
+                parse_spa_object(&merged).is_some(),
+                "emitted an object we cannot parse for inherited {inherited:?}: {merged}"
+            );
+        }
+    }
+
     /// The warning names the variable that was actually malformed. It used to
     /// always say `PIPEWIRE_PROPS`, including when `tag_child` merged
     /// `PIPEWIRE_ALSA` (round 10 review, finding 3's second half).