Repository navigation
Regex character class / - creates unintended range admitting injection chars (", $, &, ') #150
Description
Non-Blocking Review Concern: Regex character class / - creates unintended range admitting injection chars (", $, &, ')
Source: pre-existing whole-codebase review
Location: .github/workflows/claude-assistant.yml:183
Date: 2026-08-17
What was flagged
The allowed-charset regex ^[a-zA-Z0-9_,:*()./ -]+$ contains the sequence ./ -] at the end of the character class. In POSIX ERE (and GNU grep -E), a hyphen between two characters is a range. Here (0x20) through - (0x2D) is the range, which includes ! (0x21), " (0x22), # (0x23), $ (0x24), % (0x25), & (0x26), ' (0x27), ( (0x28), ) (0x29), * (0x2A), + (0x2B), , (0x2C). The error message explicitly claims ", $, & are rejected as injection vectors, but the regex admits them. This also applies to line 202 (the new DISALLOWED_TOOLS validation), which copies the same pattern. Defense-in-depth mitigation: the values are consumed via env: (not direct interpolation), so shell injection is already blocked at that layer — but the charset validation is providing false assurance. Fix: move the - to the end of the character class (or escape it as \-), yielding ^[a-zA-Z0-9_,:*()./ -]+$ → ^[a-zA-Z0-9_,:*()./ -]+$ corrected as ^[a-zA-Z0-9_,:*(). /-]+$ or ^[a-zA-Z0-9_,:*()./\- ]+$.
Context
This issue was automatically created from a non-blocking concern identified
during pre-existing whole-codebase review. It was flagged for tracking.
Created by lib-review-issues.sh
Activity
chore: close four tracked low-effort review issues (#153)
twistedmelonman commentedon Aug 19, 2026
Closing as invalid — the finding is incorrect, and no code change was made.
The claim
That
^[a-zA-Z0-9_,:*()./ -]+$contains a range from-(0x2D), admitting!"#$%&'()*+,— and that the error message therefore gives false assurance when it says",$, and&are rejected.Why it's wrong
A hyphen in the final position of a bracket expression is a literal, not a range operator. In
[a-zA-Z0-9_,:*()./ -]the trailing-sits immediately before], so there is nospace-to-hyphenrange; the space and the hyphen are both ordinary literal members. This is standard POSIX bracket-expression behavior and is how GNU grep -E parses it.The suggested "fix" (
^[a-zA-Z0-9_,:*(). /-]+$) is therefore a no-op reshuffle of an already-correct character class.Verified, not just reasoned
Fired known-bad inputs at the actual regex rather than reading it:
Every character the issue says is admitted is in fact rejected. The error message's claim is accurate as written, for both the
allowed_toolscheck and theDISALLOWED_TOOLScheck at line 202 that shares the pattern.Acting on this report would have churned a correct security check on the strength of a misreading. Flagging that explicitly since these findings are auto-filed and this one reads plausibly.
Context: reviewed alongside #124/#127/#139/#142 in #153.