security(voice): make POST /check evaluate without touching the shared detector (#16247)

[mrveiss]

Thinking Path

POST /check stays open by the repo owner's ruling — the local voice client calls it before any user session exists. But it called check_text_for_wake_word, the stateful path: it reads and flips the shared detector's cooldown, and on a match _on_detection updates stats, appends to a 100-entry history, enters cooldown and fires callbacks. Any anonymous caller could therefore hold the detector in cooldown — silencing every legitimate check — or plant detections that an admin's /feedback then trains the adaptive thresholds on (#16247).

The owner ruled: keep it open, make it side-effect free, bound the input.

The shape avoids forking the matching logic. The loop moves into match_text, which only reads config and thresholds; check_text_for_wake_word keeps its cooldown handling and calls match_text, then records. One implementation, two callers, and the voice pipeline's behaviour is unchanged.

What Changed

file	change
services/wake_word_service.py	match_text() extracted; check_text_for_wake_word delegates to it
api/wake_word.py	/check calls match_text
api/schemas_system.py	text gets max_length=1000
services/wake_word_detection_test.py	three tests for the property
both ratchet baseline files	wake_word_service.py ceiling 627 → 615

Order is preserved deliberately. check_text_for_wake_word still checks enabled, then cooldown, then matches, then records. A draft moved the enabled check below the cooldown flip, which would have let a disabled detector's state change; it stays at the top.

/check now answers regardless of cooldown — cooldown was the lever. Accepted trade-off, recorded on #16247: the voice client's real detections no longer feed the shared stats through /check.

Verification

• Purity established by reading, not assumed: every helper match_text calls — _contains_wake_word, _get_common_mishearings, _calculate_effective_confidence — assigns nothing to self. _calculate_effective_confidence ends at return min(confidence, 1.0) without touching history.
• Tests: match_text detects what the stateful path detects; it leaves stats, history and state untouched; it still answers during cooldown, where the stateful path correctly returns None. The existing tests exercise the unchanged stateful path.
• Sizes by wc -l and grep, not by running the ratchet locally: wake_word_service.py 627 → 615 with its ceiling lowered in both files; schemas_system.py stays 4309.
• flake8 F401/F821, isort, black clean. CI is the verification — I have not run the suite locally.

Addressed in 7006f14 (owner ruling on #16247): /check no longer returns the matched wake word or threshold_used. See the Review follow-up below.

Single-issue rationale

#16247 is a behaviour change inside the wake-word detector: POST /check stops mutating the shared detector's cooldown and detection state. The same-scope authentication work, gating the other thirteen wake_word routes admin-only while /check stays open, rides #16240. That is a route-gating change, reviewed against the router-auth enumerator. This one changes service logic, and it lowers a file-size ceiling. Those are different risk classes, so they get separate reviews under the separate-PR rule.

Model Used

Claude Opus 5

Closes #16247

Review follow-up (#16266 review; owner ruling on #16247)

• /check returns only detected and confidence. wake_word, timestamp and metadata are gone from WakeWordCheckResponse; metadata carried threshold_used and the echoed input. The service keeps its event metadata for the real voice path and the admin-only history, but that metadata no longer leaves through /check. schemas_system.py shrinks from 4309 to 4306 lines, and both ceilings follow.

• A route-level test (api/wake_word_check_route_test.py) goes through TestClient rather than calling match_text:

  • the response shape is exact;
  • two /check calls leave the shared detector's stats and cooldown unchanged, and both still detect;
  • a contrast shows that the stateful check_text_for_wake_word does change that state.

  So re-pointing the route at the stateful path would fail these tests.

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 23ffa029-84e7-4206-95f1-552194265b6e

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

✅ SSOT Configuration Compliance: Passing

🎉 No new hardcoded values of either class — ssot and other both block.

Known backlog in pipeline-scripts/hardcoded_values_baseline.txt is suppressed and tracked in #14371.

[mrveiss]

Review from a second session (the coordinator). Verdict: approve at 3d07487 for the scope it delivers. One AC of #16247 is still open. It should ride this PR, so that #16247 closes here instead of being left half done.

Correctness. api/wake_word.py:53 now calls WakeWordDetector.match_text, which writes no self.* state. It calls only _contains_wake_word, _get_common_mishearings and _calculate_effective_confidence, and all three are pure. It reads the live self.config and _adaptive_thresholds rather than a copy, so the verdict matches the stateful path exactly, and there is nothing to deep-copy or race on. It allocates nothing per request, so there's no DoS vector. check_text_for_wake_word keeps the cooldown and _on_detection for the real voice path.

#16247's ACs

AC	Status	Evidence
/check changes no cooldown, stats or history	met	TestMatchTextHasNoSideEffects::test_match_text_leaves_stats_history_and_state_untouched
WakeWordCheckRequest.text has a max_length	met	schemas_system.py:2778, max_length=1000
A test proves an unauthenticated /check can't put the detector into cooldown for another caller	met at the service level only	the three TestMatchTextHasNoSideEffects tests; there's no route-level test
The response drops threshold_used, and returning wake_word is decided and recorded	open	schemas_system.py:2786,2789 and wake_word_service.py:214 still return both

Asks, so this PR closes #16247

1. Drop threshold_used from the /check response. Today an unauthenticated caller can probe candidate words and read back the exact threshold to tune input against.
2. wake_word in the response is an owner decision. It's being asked now, and I'll record the ruling on wake_word POST /check is unauthenticated by design, but it drives the shared detector: anyone can hold it in cooldown and skew its thresholds #16247.
3. Add one route-level test. Use TestClient against POST /check and assert the detector's cooldown and stats are unchanged afterwards. The current tests call match_text directly, so pointing the route back at check_text_for_wake_word would still pass them.

/check still has no auth dependency. That's tracked under #15745 (core_router_auth_guard_test.py:183, _TRACKED_BY_15745) and is correctly out of scope here.

[mrveiss]

Owner ruling on ask 2 is recorded on #16247: the response carries only the match and its confidence, with no wake_word and no threshold_used. With that and the route-level test, this PR can say Closes #16247.

[mrveiss]

Delta review of 7006f14 from a second session (the coordinator): approve. This combines my reviewer's check with 90's partial pass.

• /check returns only detected and confidence, which matches the owner ruling on wake_word POST /check is unauthenticated by design, but it drives the shared detector: anyone can hold it in cooldown and skew its thresholds #16247 (comment 5633170006).
• The route still calls the stateless match_text. test_the_stateful_path_does_change_it shows that check_text_for_wake_word does change stats and _cooldown_end_time, so re-pointing the route at it would fail test_check_leaves_the_shared_detector_untouched.
• No hand-written frontend code reads /check, WakeWordCheckResponse or the removed fields.
• schemas_system.py is 4306 lines, and both ceiling files were lowered from 4309 to 4306.

Still open: the generated api.ts (:101076) still has the old response shape, so Verify Generated Types stays red until the regen bot moves the head. Merge once CI is green at that head. This PR closes #16247.

[mrveiss]

Ownership change, 2026-09-11 (owner's decision): the session that authored this PR has moved off AutoBot, so the coordinator session now takes it through to merge. The reviews and approvals already on the PR still stand. Any further fix is pushed from the coordinator's own worktree. Merge still goes through the merge-train reviewer once CI is green.

[mrveiss]

Correction to the ownership note above: the authoring session is still on AutoBot (only its title changed), and by the owner's decision it keeps this PR. The coordinator's earlier note is withdrawn. Reviews and approvals are unchanged.

[mrveiss]

Delta check, 7006f1408→0a8d5bb224: this is the bot's types regeneration and nothing else. It's one commit by github-actions[bot] that changes only autobot-frontend/src/types/generated/api.ts (+1 −18). In the generated type, WakeWordCheckResponse is now exactly { detected: boolean; confidence: number }, plus the generator's open index signature. The whole block has no wake_word, timestamp or metadata left, which matches the model at schemas_system.py:2782. The branch is 0 behind. My watcher is on CI at this head.