Greptile flagged a bypass in PR #1889: `$(echo ")"; (npm run dev))`
threaded the depth-counting loops in `extractCommandSubstitutions`
and `extractSubshellGroups` to terminate early, because a literal `)`
inside double quotes was treated as a real closing paren. The
truncated body then ended in a dangling `"` that toggled `inDouble`
in the outer scan, masking the subsequent `(npm run dev)` group from
extraction.
Reproduced (before this commit) by piping the synthetic PreToolUse
payload `{"tool_input":{"command":"$(echo \")\"; (npm run dev))"}}`
into `scripts/hooks/pre-bash-dev-server-block.js` and observing
exit 0 (allow) where the dev pattern is clearly present.
Fix: each `$(...)` and `(...)` body loop now tracks its own
single/double quote state and only treats `(` / `)` as depth
delimiters when outside quotes. The quoted `)` no longer closes
the group early, the body now extends to the real closing paren,
and the outer scan's quote state remains untouched.
After this commit:
$ echo '{"tool_input":{"command":"$(echo \")\"; (npm run dev))"}}' \
| node scripts/hooks/pre-bash-dev-server-block.js; echo $?
2
The symmetric form `$(echo "(npm run dev)")` correctly remains
allowed (bash does not honor `(...)` inside double quotes).
`extractCommandSubstitutions` only walks `$(...)` and backticks — the two
shell constructs whose bodies are captured as strings. Bash also has
plain `(...)` subshells (e.g. `(npm run dev)`), where the body executes
in a child shell but is not value-captured. Our PreToolUse hooks need
to peer inside those too, because a `(...)` group bypasses the
top-level segment splitter just like `$(...)` does.
This commit adds a sibling extractor with the same conventions as
`extractCommandSubstitutions`:
- single quotes literal — `'(npm run dev)'` is a string, ignored
- double quotes literal for parens — `"(npm run dev)"` is a string
(bash only honors `$(...)`, not bare `(...)`, inside double quotes)
- skips `$(...)` and backtick spans so we don't double-extract
bodies the other helper already handles
- recurses into its own bodies for nested groups
No consumer yet; the next commit wires both extractors into
`scripts/hooks/pre-bash-dev-server-block.js` to close the subshell
bypass surface.
Extract the `extractCommandSubstitutions` function originally
introduced in scripts/hooks/gateguard-fact-force.js (PR #1853
round 2) into scripts/lib/shell-substitution.js so other PreToolUse
hooks can reuse the same single-quote-aware, double-quote-aware,
nested-subshell-aware parser without duplicating it.
No behavior change in this commit — the function body is copied
verbatim and exposed via `module.exports`. The next commit wires it
into scripts/hooks/pre-bash-dev-server-block.js to close that hook's
own subshell-bypass holes.
gateguard-fact-force.js still defines its own private copy of the
function; consolidating both call sites onto this shared lib is a
follow-up worth doing once this PR lands, but is intentionally out
of scope here to keep the diff focused on the dev-server-block fix.
Jamkris ·