fix(engine security): close witness domain-separation collision in governed-trust cycle + prove privacy monotonicity (#1074)

* fix(engine): length-prefix witness fields to close domain-separation collision

The BLAKE3 trust witness concatenated model_version, calibration_version,
and privacy_decision boundary-to-boundary, with the variable-length evidence
list lacking an explicit count. A string straddling a field boundary (e.g. a
per-room adapter id absorbing the leading bytes of the calibration epoch, or a
model_version absorbing a trailing evidence ref) collided with a different
trust decision — silently un-distinguishing two distinct privacy-relevant
inputs and defeating the ADR-137 tamper/drift audit guarantee. model_version
is operator-influenceable via the adapter id (ADR-150 §3.4), so the ambiguity
was reachable.

Fix: domain-tag the hash and length-prefix every field (8-byte LE length),
plus an explicit evidence count. Pinned by two fails-on-old tests:
witness_distinguishes_model_calibration_boundary and
witness_distinguishes_evidence_model_boundary.

Co-Authored-By: claude-flow <ruv@ruv.net>

* test(engine): pin privacy monotonicity, fail-closed boundaries; de-magic constants

Review hardening for the governed-trust cycle (no behavior change):

- forced_contradiction_never_relaxes_class: property test over all 5 privacy
  modes proving a forced contradiction only ever raises the emitted class byte
  (more restrictive) and a clean cycle emits exactly the base class — the
  ADR-141/120 information-only-removed invariant.
- empty_cycle_fails_closed: a zero-frame cycle errors (fusion NoFrames),
  emits no SemanticState, and does not advance the cycle counter.
- single_node_cycle_is_well_formed: characterizes the n=1 boundary (no mesh,
  no directional, base class, witness still emitted) — documents single-node
  sensing as a valid non-demoting mode, not a bypass.
- De-magicked the engine-construction literals (coherence accept gate, ADR-143
  SLAM discovery + static-anchor thresholds) into named documented consts,
  value-identical, pinned by engine_constants_match_prior_values.

Co-Authored-By: claude-flow <ruv@ruv.net>

* docs(engine-review): record witness domain-separation fix + monotonicity clean bill

CHANGELOG [Unreleased] Security entry and review notes appended to ADR-137
(witness domain-separation fix) and ADR-141 (privacy monotonicity confirmed
clean over all 5 modes, fail-closed boundaries pinned).

Co-Authored-By: claude-flow <ruv@ruv.net>

v2/crates/wifi-densepose-engine/src/lib.rs

@@ -205,7 +205,7 @@ impl StreamingEngine {
     pub fn new(mode: PrivacyMode, model_version: u16, registration: GeoRegistration) -> Self {
         Self {
             fuser: MultistaticFuser::with_config(MultistaticConfig::default()),
-            coherence_accept: 0.85,
+            coherence_accept: Self::DEFAULT_COHERENCE_ACCEPT,
             privacy: PrivacyModeRegistry::new(mode),
             world: WorldGraph::new(registration),
             model_version,
@@ -213,7 +213,11 @@ impl StreamingEngine {
             array: ArrayCoordinator::new(ArrayCoordinatorConfig::default()),
             node_geom: BTreeMap::new(),
             evolution: None,
-            slam: RfSlam::with_discovery(0.5, 5, 0.6),
+            slam: RfSlam::with_discovery(
+                Self::SLAM_ASSOC_RADIUS_M,
+                Self::SLAM_MIN_SIGHTINGS,
+                Self::SLAM_MIN_COHERENCE,
+            ),
             person_tracks: BTreeMap::new(),
             semantic_retention: Self::DEFAULT_SEMANTIC_RETENTION,
             adapter: None,
@@ -257,6 +261,31 @@ impl StreamingEngine {
     /// durable history belongs to the recorder).
     pub const DEFAULT_SEMANTIC_RETENTION: usize = 7_200;
 
+    /// Cross-node coherence at or above which fusion records a positive
+    /// `CoherenceGateThreshold` evidence ref (ADR-137). Below it the cycle still
+    /// emits, but without that corroborating evidence — so this gate shapes the
+    /// trust record, not the privacy class. (== prior inline 0.85.)
+    pub const DEFAULT_COHERENCE_ACCEPT: f32 = 0.85;
+
+    /// ADR-143 reflector-discovery parameters used to build the persistent
+    /// `RfSlam`: association radius (m) within which two sightings are the same
+    /// reflector, the minimum number of sightings before a reflector is
+    /// considered stable, and the minimum per-sighting coherence to admit it.
+    /// (== prior inline `with_discovery(0.5, 5, 0.6)`.)
+    pub const SLAM_ASSOC_RADIUS_M: f64 = 0.5;
+    /// Minimum sightings before a discovered reflector is treated as stable.
+    pub const SLAM_MIN_SIGHTINGS: u64 = 5;
+    /// Minimum per-sighting coherence to admit a reflector sighting.
+    pub const SLAM_MIN_COHERENCE: f32 = 0.6;
+
+    /// ADR-143 static-anchor classification thresholds passed to
+    /// `RfSlam::static_anchors`: the wall/ceiling stationarity ceiling and the
+    /// mobile-reflector floor (anchors more mobile than this are dropped, not
+    /// persisted). (== prior inline `static_anchors(0.05, 1.0)`.)
+    pub const ANCHOR_WALL_CEILING: f64 = 0.05;
+    /// Mobility floor above which a reflector is treated as mobile (skipped).
+    pub const ANCHOR_MOBILE_FLOOR: f64 = 1.0;
+
     /// Override the `SemanticState` retention cap (minimum 1).
     pub fn set_semantic_retention(&mut self, max_states: usize) {
         self.semantic_retention = max_states.max(1);
@@ -331,7 +360,9 @@ impl StreamingEngine {
             self.slam.observe(obs);
         }
         let mut written = Vec::new();
-        for (pos, class) in self.slam.static_anchors(0.05, 1.0) {
+        for (pos, class) in
+            self.slam.static_anchors(Self::ANCHOR_WALL_CEILING, Self::ANCHOR_MOBILE_FLOOR)
+        {
             let kind = match class {
                 wifi_densepose_signal::ruvsense::ReflectorClass::Wall => AnchorKind::Reflector,
                 wifi_densepose_signal::ruvsense::ReflectorClass::Furniture => AnchorKind::Furniture,
@@ -595,19 +626,46 @@ impl StreamingEngine {
     }
 }
 
+/// Domain-separation tag for the witness hash. Bumping this string
+/// intentionally invalidates every previously-recorded witness (a schema break).
+const WITNESS_DOMAIN: &[u8] = b"ruview.engine.witness.v1";
+
+/// Length-prefix a variable-length field into the witness hash so adjacent
+/// fields can never be confused for one another. The 8-byte little-endian
+/// length makes the field framing unambiguous regardless of the bytes inside
+/// it (a field can contain the separator, the domain tag, anything).
+fn witness_field(h: &mut blake3::Hasher, bytes: &[u8]) {
+    h.update(&(bytes.len() as u64).to_le_bytes());
+    h.update(bytes);
+}
+
 /// Deterministic BLAKE3 witness over a trust decision: the provenance tuple
 /// (evidence ‖ model ‖ calibration ‖ privacy decision) plus the effective
 /// privacy-class byte. Stable across runs for identical decisions — the
 /// "signed operational belief" fingerprint (ADR-137 §2.7 / ADR-028).
+///
+/// # Witness integrity (review finding: domain separation)
+/// Every privacy-relevant field is **length-prefixed** before hashing, and the
+/// (variable-length) evidence list is preceded by an explicit count. Without
+/// this framing the fields were concatenated boundary-to-boundary, so a string
+/// straddling a field boundary (e.g. an adapter id absorbing the leading bytes
+/// of the calibration epoch, or a model_version absorbing a trailing evidence
+/// ref) collided with a *different* trust decision — silently un-distinguishing
+/// two distinct privacy-relevant inputs and defeating the tamper/drift audit.
+/// `model_version` is operator-influenceable (per-room adapter id, ADR-150
+/// §3.4), so the ambiguity was reachable, not merely theoretical.
 fn witness_of(p: &SemanticProvenance, class: PrivacyClass) -> [u8; 32] {
     let mut h = blake3::Hasher::new();
+    h.update(WITNESS_DOMAIN);
+    // Explicit evidence count, then each ref length-prefixed: the number of
+    // evidence refs is itself privacy-relevant and must be unambiguous.
+    h.update(&(p.evidence.len() as u64).to_le_bytes());
     for e in &p.evidence {
-        h.update(e.as_bytes());
-        h.update(b"\x1f");
+        witness_field(&mut h, e.as_bytes());
     }
-    h.update(p.model_version.as_bytes());
-    h.update(p.calibration_version.as_bytes());
-    h.update(p.privacy_decision.as_bytes());
+    witness_field(&mut h, p.model_version.as_bytes());
+    witness_field(&mut h, p.calibration_version.as_bytes());
+    witness_field(&mut h, p.privacy_decision.as_bytes());
     h.update(&[class.as_u8()]);
     *h.finalize().as_bytes()
 }
@@ -1113,4 +1171,179 @@ mod tests {
         // StrictNoIdentity base = Restricted, even with no contradiction.
         assert_eq!(out.effective_class, PrivacyClass::Restricted);
     }
+
+    /// De-magic pin (review finding): the named engine constants must keep
+    /// their prior inline values exactly, so the de-magic is a pure rename with
+    /// no behavior change.
+    #[test]
+    fn engine_constants_match_prior_values() {
+        assert_eq!(StreamingEngine::DEFAULT_COHERENCE_ACCEPT, 0.85);
+        assert_eq!(StreamingEngine::SLAM_ASSOC_RADIUS_M, 0.5);
+        assert_eq!(StreamingEngine::SLAM_MIN_SIGHTINGS, 5);
+        assert_eq!(StreamingEngine::SLAM_MIN_COHERENCE, 0.6);
+        assert_eq!(StreamingEngine::ANCHOR_WALL_CEILING, 0.05);
+        assert_eq!(StreamingEngine::ANCHOR_MOBILE_FLOOR, 1.0);
+    }
+
+    /// Privacy monotonicity (the crux): across EVERY base mode, a forced
+    /// contradiction may only ever make the emitted class *more* restrictive
+    /// (higher byte) and never less. Demotion is single-step and clamps at
+    /// Restricted; a clean cycle emits exactly the base class. This is the
+    /// information-only-removed invariant of ADR-141/120 stated as a property
+    /// over the whole mode set.
+    #[test]
+    fn forced_contradiction_never_relaxes_class() {
+        let cal_mismatch = [Some(CalibrationId(1)), Some(CalibrationId(2))]; // disagree → contradiction
+        let cal_match = [Some(CalibrationId(5)), Some(CalibrationId(5))];
+        let frames = [node_frame(0, 1000, 56), node_frame(1, 1001, 56)];
+        for mode in [
+            PrivacyMode::RawResearch,
+            PrivacyMode::PrivateHome,
+            PrivacyMode::EnterpriseAnonymous,
+            PrivacyMode::CareWithConsent,
+            PrivacyMode::StrictNoIdentity,
+        ] {
+            let base_class = mode.target_class();
+
+            // Clean cycle: emits exactly the base class (no relaxation upward).
+            let mut clean = StreamingEngine::new(mode, 1, GeoRegistration::default());
+            let room_c = clean.add_room("r", "R");
+            let oc = clean
+                .process_cycle_calibrated(&frames, &cal_match, room_c, 1)
+                .unwrap();
+            assert_eq!(oc.effective_class, base_class, "clean cycle == base class");
+            assert!(!oc.demoted);
+
+            // Forced contradiction: class byte only ever increases (more
+            // restrictive), never decreases below the base.
+            let mut dirty = StreamingEngine::new(mode, 1, GeoRegistration::default());
+            let room_d = dirty.add_room("r", "R");
+            let od = dirty
+                .process_cycle_calibrated(&frames, &cal_mismatch, room_d, 1)
+                .unwrap();
+            assert!(od.demoted, "calibration mismatch must demote in {mode:?}");
+            assert!(
+                od.effective_class.as_u8() >= base_class.as_u8(),
+                "demotion must never relax: {mode:?} base={:?} got={:?}",
+                base_class,
+                od.effective_class
+            );
+            // And it must be strictly more restrictive unless already clamped
+            // at the most-restrictive class.
+            if base_class != PrivacyClass::Restricted {
+                assert!(
+                    od.effective_class.as_u8() > base_class.as_u8(),
+                    "unclamped demotion must increase restriction in {mode:?}"
+                );
+            } else {
+                assert_eq!(od.effective_class, PrivacyClass::Restricted);
+            }
+        }
+    }
+
+    /// Fail-closed boundary: an empty cycle (zero frames) must NOT emit a
+    /// trusted output at all — fusion rejects it and the engine surfaces a
+    /// hard error. There is no degenerate output that could carry a stale or
+    /// over-permissive class.
+    #[test]
+    fn empty_cycle_fails_closed() {
+        let (mut e, room) = engine();
+        let err = e.process_cycle(&[], CalibrationId(1), room, 1);
+        assert!(matches!(err, Err(EngineError::Fusion(_))), "empty cycle must error, got {err:?}");
+        // No SemanticState was appended (room + sensor only).
+        assert_eq!(e.world().node_count(), 2);
+        assert_eq!(e.cycle_count(), 0, "a failed cycle must not advance the counter");
+    }
+
+    /// Single-node boundary characterization: a one-node cycle fuses (no
+    /// multistatic cross-check is possible), reports no mesh (n<2), and emits a
+    /// well-formed witness at the base class. Documents that single-node sensing
+    /// is a valid, non-demoting mode — not a silent bypass.
+    #[test]
+    fn single_node_cycle_is_well_formed() {
+        let (mut e, room) = engine();
+        let out = e
+            .process_cycle(&[node_frame(0, 1000, 56)], CalibrationId(1), room, 1)
+            .unwrap();
+        assert!(out.mesh.is_none(), "one node has no mesh cut");
+        assert!(out.directional.is_none(), "no geometry registered");
+        assert_eq!(out.effective_class, PrivacyClass::Anonymous); // PrivateHome base
+        assert_ne!(out.witness, [0u8; 32], "witness still emitted");
+    }
+
+    /// Witness domain-separation (review finding): the witness must change
+    /// whenever ANY privacy-relevant field changes. The model_version,
+    /// calibration_version, and privacy_decision fields are concatenated into
+    /// the hash; without an unambiguous delimiter between them, a string that
+    /// straddles the model/calibration boundary collides with a different
+    /// (model, calibration) tuple.
+    ///
+    /// `model_version` is operator-influenceable through the per-room adapter id
+    /// (ADR-150 §3.4), and `calibration_version` is `cal:<hex>` — so the two
+    /// provenances below are *both reachable* and represent genuinely different
+    /// trust decisions (different model identity, different calibration epoch),
+    /// yet the field-boundary ambiguity makes them hash-collide. A colliding
+    /// witness silently un-distinguishes two distinct privacy-relevant inputs,
+    /// defeating the tamper/drift audit guarantee.
+    #[test]
+    fn witness_distinguishes_model_calibration_boundary() {
+        let class = PrivacyClass::Anonymous;
+        // A: model "rfenc-v1+adapter:X", calibration epoch "cal:00ab".
+        let a = SemanticProvenance {
+            evidence: vec!["ev".into()],
+            model_version: "rfenc-v1+adapter:X".into(),
+            calibration_version: "cal:00ab".into(),
+            privacy_decision: "PrivateHome/Anonymous".into(),
+        };
+        // B: adapter id absorbs the leading "cal:00a" of A's calibration; B's
+        // own calibration is the remaining "b". A.model‖A.cal == B.model‖B.cal,
+        // so the unseparated concatenation hashes identically — yet these are
+        // distinct (model identity, calibration epoch) tuples.
+        let b = SemanticProvenance {
+            evidence: vec!["ev".into()],
+            model_version: "rfenc-v1+adapter:Xcal:00a".into(),
+            calibration_version: "b".into(),
+            privacy_decision: "PrivateHome/Anonymous".into(),
+        };
+        assert_ne!(a.model_version, b.model_version);
+        assert_ne!(a.calibration_version, b.calibration_version);
+        // Sanity: the two collide under naive concatenation.
+        assert_eq!(
+            format!("{}{}", a.model_version, a.calibration_version),
+            format!("{}{}", b.model_version, b.calibration_version),
+        );
+        assert_ne!(
+            witness_of(&a, class),
+            witness_of(&b, class),
+            "distinct (model, calibration) tuples must not share a witness"
+        );
+    }
+
+    /// Witness domain-separation across the evidence/model boundary: a witness
+    /// must distinguish an extra evidence ref from a model_version that absorbs
+    /// the same bytes. The evidence loop terminates each ref with one separator;
+    /// the model field must itself be unambiguously delimited from the (variable
+    /// number of) evidence refs that precede it.
+    #[test]
+    fn witness_distinguishes_evidence_model_boundary() {
+        let class = PrivacyClass::Anonymous;
+        let a = SemanticProvenance {
+            evidence: vec!["e1".into(), "e2".into()],
+            model_version: "m".into(),
+            calibration_version: "cal:1".into(),
+            privacy_decision: "PrivateHome/Anonymous".into(),
+        };
+        let b = SemanticProvenance {
+            evidence: vec!["e1".into()],
+            // absorbs "e2" + its 0x1f separator into the model field.
+            model_version: "e2\u{1f}m".into(),
+            calibration_version: "cal:1".into(),
+            privacy_decision: "PrivateHome/Anonymous".into(),
+        };
+        assert_ne!(
+            witness_of(&a, class),
+            witness_of(&b, class),
+            "an extra evidence ref must not collide with a model_version that absorbs it"
+        );
+    }
 }