Two false-negatives surfaced in PR #1889 review:
1. Brace-group bypass (Greptile).
`{ npm run dev; }` evaluates the dev command in the *current*
shell — semantically distinct from `( ... )` but with the same
effect for this hook. `splitShellSegments` correctly cleaves the
group at `;` into `["{ npm run dev", "}"]`, but the first segment's
leading token under `readToken` is the bare `{`, which was not in
`DEV_COMMAND_WORDS`, so the dev-pattern check was skipped.
Fix: treat `{` and `}` as no-op tokens in `getLeadingCommandWord`
so we keep walking to the real command word. Matches how shell
itself parses brace groups (the braces are reserved words, not
commands). Bash requires a space after `{` and a terminator before
`}` for an actual group, so `{npm run dev}` correctly remains
allowed (single token `{npm`, not in `DEV_COMMAND_WORDS`).
2. Missing yarn-run / bun-bare variants (CodeRabbit).
Both `yarn dev` *and* `yarn run dev` are valid (the latter is what
`package.json` actually wires `dev` to under yarn 1.x). The same
`(run )?` symmetry applies to bun. The previous `DEV_PATTERN` only
matched `yarn\s+dev` and `bun\s+run\s+dev`, allowing the cross
forms to pass through silently.
Fix: `yarn(?:\s+run)?\s+dev` and `bun(?:\s+run)?\s+dev` — same
shape `pnpm(?:\s+run)?\s+dev` was already using.
Verified after this commit (every form now exits 2):
{ npm run dev; }
{ npm run dev ; }
echo hi && { npm run dev; }
({ npm run dev; })
$( { npm run dev; } )
yarn run dev
bun dev
Verified still allowed (no regression):
echo "{ npm run dev; }" # literal inside double quotes
{npm run dev} # not a brace group per bash syntax
Before this commit the dev-server-block hook ran the leading-command
and dev-pattern check only against the top-level segments returned by
`splitShellSegments`, which doesn't split on `$(...)`, backticks, or
plain `(...)`. That left the policy bypassable by wrapping a dev
command in any of those constructs:
$(npm run dev)
`npm run dev`
echo $(npm run dev)
(npm run dev)
Each verified by piping a synthetic PreToolUse payload into the hook
on this branch: every form above returned exit 0 (allow) where a plain
`npm run dev` correctly returned exit 2 (block).
Fix: expand the check space before running the leading-command rule.
A small BFS walks the raw command, harvesting bodies from
`extractCommandSubstitutions` (`$(...)` and backticks) and from
`extractSubshellGroups` (plain `(...)`), then splits each harvested
body through `splitShellSegments` and feeds the result into the
existing `isBlockedDevSegment` check.
This preserves every existing allow case (`tmux new-session -d -s dev
"npm run dev"`, quoted-string mentions like `git commit -m "npm run
dev fix"`, `echo hi`) because the leading-command rule is unchanged —
only the set of segments it runs against grew.
Known limitation, not fixed here: `eval "$(echo npm run dev)"` still
slips through because the substitution body's leading command is
`echo`, and statically modeling echo's output to recover the executed
command is out of scope. The same class affects `gateguard-fact-force`
(via `eval "$(echo rm -rf /)"` etc.) and is best addressed in both
hooks together as a follow-up rather than as a one-off here.
Jamkris ·
Affaan Mustafa ·
zzzhizhi ·