perf(classify): skip NFKC + zero-width fold on pure-ASCII commands (~1.8x)#71
Open
askalf wants to merge 1 commit into
Open
perf(classify): skip NFKC + zero-width fold on pure-ASCII commands (~1.8x)#71askalf wants to merge 1 commit into
askalf wants to merge 1 commit into
Conversation
classify() ran cmd.normalize('NFKC'), allocated a fresh Set, and rebuilt the
whole command via Array.from(...).join('') on every call - but all three only
affect non-ASCII input (NFKC is identity on ASCII; every stripped formatting
char is >= U+00AD). Gate the fold behind a cheap non-ASCII test and hoist the
constants to module scope. Pure-ASCII commands (the common case) now skip it
entirely; non-ASCII input is folded exactly as before, so verdicts are
byte-identical.
~1.8x faster classify() on a realistic ASCII command mix (~15.7us -> ~8.7us per
call, same-process A/B). classify() dominates decide(), which runs on every
hook/daemon tool call, so this roughly halves per-call CPU for normal commands.
Adds the first unit coverage for the homoglyph/zero-width fold (fullwidth RM,
ZWSP/soft-hyphen/ZWNJ/RLO keyword splits, ASCII substring-safety) - previously
only the red-team bench exercised it. Detection corpus unchanged (same recall
and precision); 203/203 tests pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
classify()folds fullwidth/homoglyph forms (NFKC) and strips zero-width / bidi formatting characters before matching, so an attacker can't disguise or split a keyword past the ruleset. That fold ran on every call:cmd.normalize('NFKC')(up to 16KB)Setallocated inlineArray.from(cmd, ...).join('')— a full code-point rebuild of the commandAll three only ever change non-ASCII input: NFKC is the identity on ASCII, and every stripped character is
>= U+00AD. This PR hoists the constants to module scope and guards the whole fold behind a cheapNON_ASCII_RE.test(cmd). Pure-ASCII commands — essentially all real traffic — skip it entirely; non-ASCII input is folded exactly as before.Why it's safe
The result is byte-identical to always running the fold — the guard only elides work that is provably a no-op on ASCII.
test/homoglyph-normalize.test.mjsadds the first unit coverage for the fold: fullwidthRM, ZWSP / soft-hyphen / ZWNJ / RLO keyword splits, and ASCII substring-safety (stormmust not trip thermrule). Evasion chars are built from numeric code points so the test file stays pure-ASCII on disk.Perf
Same-process A/B (baseline = current
master), realistic ASCII command mix, best-of-3 min:classify()dominatesdecide(), which runs on every hook/daemon tool call, so this roughly halves the firewall's per-call CPU for normal commands. No new dependencies; still zero-runtime-dep.