feat: add a control anomaly kind, closing seven CVE rows nothing reported (#612) - #627
Conversation
…ported (#612) A non-whitespace control — NUL, ESC, BEL, DEL, the C1 block — is never legitimate in text, and no detector reported one. strip_control_chars has removed them since #433, so the transform existed and the detector did not. Why they were invisible is the interesting part: the introducers are plain ASCII, so the ASCII fast path in classify() — which exists because the invisible, bidi, zalgo and mixed-script branches can only fire above U+007F — skipped them entirely. The new branch runs before that gate. PRESENCE, NOT POSITION. #612 framed this as an "edge" question because it started from whitespace trimming, but a control hides things wherever it sits: the last character of "malicious\x1b\\" is a backslash, so an edge-only rule would call that token clean while the escape introducer sits one place in. There is a test for exactly that case. The whitespace-class controls are excluded by reusing is_fold_whitespace rather than restating the set, so the two cannot drift. TAB, LF, VT, FF, CR, U+001C-U+001F and NEL are real separators that collapse_whitespace folds to a space; flagging them would fire on every multi-line string. has_anomalies goes from 11 CVE rows to 18. Seven rows that docs/security/cve-validation.md listed as reported by nothing are now reported: CVE-2023-24329 (leading NUL) and the whole terminal-control class (CVE-2008-2383, CVE-2019-9535, CVE-2025-55754, CVE-2024-52005, CVE-2023-43620, CVE-2023-37275). Four pinned tests failed by design and are updated, including one whose assertion inverts — it is kept rather than deleted so a regression fails loudly. The registry's per-CVE detector lists and dispositions are derived from behaviour, so those and the rendered docs matrix moved with it. The three rows still undetected are a different shape, and the page now says so: each needs a comparison (a fold collision, a length budget, a table lookup) rather than the presence of a character, so no further character class will close them. Deliberately NOT added: leading/trailing whitespace detection, which #612 also asked for. inspect_anomalies documents itself as flagging characters "disguising a real word". Padding disguises nothing, and a kind for it would fire on ordinary text. Also fixes the Node AnomalyKind union, which shipped without bidi_mixed from #412. A TypeScript caller matching on it got a type error for a kind the library really returns, and nothing caught it because the value crosses napi as a bare String and index.ts casts. Node is the only binding that restates the set, so a drift gate now reads the as_str arms out of src/anomalies.rs and compares them, plus a second test asserting every kind is reachable from some input. Signed-off-by: Richard Quinn <quinn.richard@gmail.com> Assisted-by: Claude Code:claude-opus-5
|
📄 Docs preview: https://2946ccf6.disarm-docs.pages.dev |
There was a problem hiding this comment.
🟡 Changes recommended
The new control-character detection introduces an avoidable extra per-token scan that weakens the existing ASCII fast-path’s performance benefits, and the docs need a small wording adjustment to match evaluation semantics.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a new control anomaly kind to the core anomaly classifier so non-whitespace control characters (e.g., NUL/ESC/DEL/C1) are detected (not just stripped by presets), and updates the CVE validation suite + docs to reflect the newly detected vectors. Also fixes Node’s AnomalyKind TypeScript union drift and adds a drift gate to prevent future mismatches.
Changes:
- Add
AnomalyKind::Controland detect non-whitespace control characters inclassify()before the ASCII fast path. - Update CVE registry expectations/tests and documentation to mark seven previously-undetected rows as detected by
has_anomalies. - Fix Node
AnomalyKindunion to include missing kinds and add a drift/reachability gate in Python tests.
File summaries
| File | Description |
|---|---|
src/anomalies.rs |
Adds Control kind and implements early control-character detection in the token classifier. |
tests/test_anomalies.py |
Adds control-character behavior tests plus a Rust↔Node kind drift/reachability gate. |
tests/test_cve_vectors.py |
Updates CVE rows/dispositions/detector expectations and inverts a pinned test to lock in the new behavior. |
docs/user-guide/anomaly-detection.md |
Documents the new control kind in the anomaly detection guide. |
docs/security/cve-validation.md |
Updates the rendered CVE matrix and narrative for newly detected vectors. |
CHANGELOG.md |
Records the new detection capability and the Node union drift fix. |
bindings/node/index.ts |
Updates the TS AnomalyKind union to include bidi_mixed and control. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
#621 added ten encoding CVEs to the same registry this branch re-pins, so the conflict was real rather than textual. Resolving it mechanically would have been wrong: git put #621's nine new UNDETECTED_IN_SCOPE rows inside CLOSED_BY_THE_CONTROL_KIND, which claims the opposite of what they are. Resolved by structure, then verified by measurement rather than by reasoning — and the measurement found one more row than expected. CVE-2009-4142's probe is '�\x00<script>', a NUL-byte injection, so the control branch reports it: eight rows closed, not seven, and has_anomalies covers 19 rather than 18. Its detectors, disposition and rendered docs row moved with it. The other two byte-level rows carry no control and stay silent, which keeps the section's point intact: everything still undetected needs a comparison — a fold collision, a length budget, a decode result — rather than the presence of a character. Signed-off-by: Richard Quinn <quinn.richard@gmail.com> Assisted-by: Claude Code:claude-opus-5
The intro sentence had drifted from the code in two ways, and adding two branches made both visible. "in order" no longer described evaluation order. `control` is checked first, ahead of the ASCII fast-path, but the table lists it eighth. The mismatch also predates this change: `bidi_mixed` evaluates before `mixed_script` and the table has them the other way round. "the first six need no lexicon" was a mechanical update of the previous "first four", and the count moved past `leet`, which does need one. Six branches need no lexicon, but they are not the first six in table order. "script-agnostic" was never true of `mixed_script`, which is anchored on Latin -- the row two lines below says so. The sentence now states what is true: six branches need no lexicon; the table is grouped by kind rather than evaluation order; `control` runs first and why; the rest split on `!tok.is_ascii()`; and `mixed_script` is the Latin-anchored exception to script-agnosticism. Signed-off-by: Richard Quinn <quinn.richard@gmail.com> Assisted-by: Claude Code:claude-opus-5
# Conflicts: # CHANGELOG.md
Closes #612.
The gap
A non-whitespace control —
NUL,ESC,BEL,DEL, the C1 block — is never legitimate in text, and no detector reported one.strip_control_charshas removed them since #433, so the transform existed and the detector did not.Why they were invisible is the interesting part. The introducers are plain ASCII, so the ASCII fast path in
classify()— which exists because the invisible, bidi, zalgo and mixed-script branches can only fire aboveU+007F— skipped them entirely. The new branch runs before that gate.Presence, not position
#612 framed this as an edge question because it started from whitespace trimming. A control hides things wherever it sits:
The last character there is a backslash, so an edge-only rule would call that token clean while the escape introducer sits one place in. There is a test for exactly that case.
The whitespace-class controls are excluded by reusing
is_fold_whitespacerather than restating the set, so the two cannot drift. TAB, LF, VT, FF, CR,U+001C–U+001Fand NEL are real separators thatcollapse_whitespacefolds to a space; flagging them would fire on every multi-line string.has_anomaliesgoes from 11 CVE rows to 18Seven rows that
docs/security/cve-validation.mdlisted as reported by nothing are now reported:Four pinned tests failed by design and are updated. One has its assertion inverted rather than deleted — it used to assert that no detector reports any terminal-control row, and now asserts that all of them do, so a regression fails loudly instead of silently.
The registry derives each row's detector list and disposition from behaviour, so those moved too, and with them the rendered docs matrix. That chain working end to end is the CVE suite doing its job.
What is left undetected is a different shape
Three rows remain, and the page now says why they are not a "one more character class" problem:
Each needs a comparison — a fold collision against another string, a length budget, a table lookup — not the presence of a character.
Deliberately not added
Leading/trailing whitespace detection, which #612 also asked for.
inspect_anomaliesdocuments itself as flagging characters "disguising a real word", and every existing kind is a disguise. Padding disguises nothing, and a kind for it would fire on ordinary text. That half of the issue is a category error rather than a missing feature.Drive-by: the Node
AnomalyKindunion shipped withoutbidi_mixedAdded to the Rust enum in #412 and never mirrored, so a TypeScript caller matching on it got a type error for a kind the library really returns. Nothing caught it, because the value crosses napi as a bare
Stringandindex.tscasts.Node is the only binding that restates the set — every other surface passes it through as a string, which is why none of them can drift. So a gate now reads the
as_strarms out ofsrc/anomalies.rsand compares them to the union, plus a second test asserting every kind is reachable from some input, since a kind nothing can produce is worse than a missing one.Verification
Full local gate: 4,714 pytest, 24 Rust test targets,
cargo fmt --check, clippy clean on both feature sets,scripts/perf_lint.shclean, mypy clean, ruff clean, language-consistency audit clean.docs/fences execute under Sybil, so the updated matrix and prose are checked rather than asserted.