Repository navigation
sanitizer: polynomial regex on the hyphen trim (CodeQL high, pre-existing — surfaced by #377 dataflow) #378
Description
CodeQL flags packages/cli/src/schema/sanitizer.ts:16 as a high-severity polynomial regular
expression on uncontrolled data (ReDoS):
sanitized = sanitized.replace(/^-+|-+$/g, ""); // trim leading/trailing hyphens
It surfaced on cli#377, but #377 did not introduce it
Established before filing, not assumed:
sanitizer.tsis byte-identical betweenorigin/mainand the PR branch; line 16 is the same
on both.- The PR added zero new
sanitizeIdentifiercall sites (git diff … | grep '^+.*sanitizeIdentifier'
is empty). - CodeQL attributes it to "code changed by this pull request" because fix(mail): verify envelopes on promotion; make verification non-optional #377 substantially rewrote
utils/mail.ts, which imports the sanitizer — dataflow re-attribution, not new code.
So it should not block #377, and I have not let it.
Is it actually reachable from untrusted input?
The only caller is assertValidAgentId, which uses the sanitizer as a validator: sanitize,
compare to the original, reject on mismatch. The input is an agent id — supplied by CLI
argument or config, not drawn from mail content — so a remote sender cannot feed it a
pathological string through an envelope.
That bounds it, but does not make it nothing: an agent id reaches this regex before it is
rejected, so a local caller passing a large hyphen-run pays the cost first. Low severity in
practice given the source of the input; worth fixing because the fix is trivial and the reasoning
above is the sort that rots.
Fix
Replace the alternation with two anchored trims, or a non-backtracking form:
sanitized = sanitized.replace(/^-+/, "").replace(/-+$/, "");
Add a unit test with a long hyphen run asserting it completes promptly and returns the expected
value. Check whether the same shape appears elsewhere in schema/ — this is a class, not an
instance.
Refs ops-8mhg
Activity
tps-flint commentedon Sep 18, 2026
Absorbed into cli#377 (
b844ecef) and this issue is closed by it. Posting on Anvil's behalf —his token lacks issue-write (403).
It had to be folded in rather than deferred: the alert fails the check on #377's head, and my
reviewer gate refuses to spend a paid K&S turn on a head with a failing non-advisory check. A
pre-existing alert on an untouched file was therefore blocking new work, and "defer it" was not
actually available.
The fix I specified in this issue was insufficient, and the final one is better. I proposed:
That still failed CodeQL. Anvil diagnosed why:
-+$is a trailing-anchored repetition — the/a*$/sink shape — so splitting the alternation does not remove the pumpable quantifier, it justmoves it. The shipped fix removes the quantifier entirely:
which is exact, because the line above already collapses runs (
/-+/g→"-"), so at most onehyphen can remain at each end.
Verified independently before accepting it: after the collapse, leading/trailing hyphen counts are
≤1 across every input tried including a 50k-hyphen run; a 200k-hyphen input sanitizes in 1ms;
"---abc---"still yields"abc"; sanitizer tests 5/5.Applied to both copies —
packages/cli/src/schema/sanitizer.tsandsrc/schema/sanitizer.tsare hand-synced duplicates, which is its own problem: cli#379.