From a7e51e804635a3bc1df72483124a05fc0623130e Mon Sep 17 00:00:00 2001 From: Jamkris Date: Thu, 14 May 2026 11:21:41 +0900 Subject: [PATCH] fix(hooks): close subshell bypass in pre-bash-dev-server-block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- scripts/hooks/pre-bash-dev-server-block.js | 58 ++++++++++++++++++---- 1 file changed, 47 insertions(+), 11 deletions(-) diff --git a/scripts/hooks/pre-bash-dev-server-block.js b/scripts/hooks/pre-bash-dev-server-block.js index 9c0861b8..d3eea48b 100755 --- a/scripts/hooks/pre-bash-dev-server-block.js +++ b/scripts/hooks/pre-bash-dev-server-block.js @@ -4,6 +4,10 @@ const MAX_STDIN = 1024 * 1024; const path = require('path'); const { splitShellSegments } = require('../lib/shell-split'); +const { + extractCommandSubstitutions, + extractSubshellGroups +} = require('../lib/shell-substitution'); const DEV_COMMAND_WORDS = new Set([ 'npm', @@ -154,23 +158,55 @@ process.stdin.on('data', chunk => { } }); +const TMUX_LAUNCHER = /^\s*tmux\s+(new|new-session|new-window|split-window)\b/; +const DEV_PATTERN = /\b(npm\s+run\s+dev|pnpm(?:\s+run)?\s+dev|yarn\s+dev|bun\s+run\s+dev)\b/; + +/** + * Collect every command-line segment we should evaluate. Returns the top-level + * segments first, then segments harvested from `$(...)` / backtick command + * substitutions and plain `(...)` subshell groups, recursively. + * + * Without this expansion the leading-command and dev-pattern check below only + * sees the outermost command, so wrappers like `$(npm run dev)` and + * `(npm run dev)` (which still spawn a dev server) sneak past. + */ +function collectCheckSegments(cmd) { + const segments = [...splitShellSegments(cmd)]; + const queue = [cmd]; + const seen = new Set(); + + while (queue.length) { + const current = queue.shift(); + if (seen.has(current)) continue; + seen.add(current); + + for (const body of extractCommandSubstitutions(current)) { + for (const seg of splitShellSegments(body)) segments.push(seg); + queue.push(body); + } + for (const body of extractSubshellGroups(current)) { + for (const seg of splitShellSegments(body)) segments.push(seg); + queue.push(body); + } + } + + return segments; +} + +function isBlockedDevSegment(segment) { + const commandWord = getLeadingCommandWord(segment); + if (!commandWord || !DEV_COMMAND_WORDS.has(commandWord)) return false; + return DEV_PATTERN.test(segment) && !TMUX_LAUNCHER.test(segment); +} + process.stdin.on('end', () => { try { const input = JSON.parse(raw); const cmd = String(input.tool_input?.command || ''); if (process.platform !== 'win32') { - const segments = splitShellSegments(cmd); - const tmuxLauncher = /^\s*tmux\s+(new|new-session|new-window|split-window)\b/; - const devPattern = /\b(npm\s+run\s+dev|pnpm(?:\s+run)?\s+dev|yarn\s+dev|bun\s+run\s+dev)\b/; - - const hasBlockedDev = segments.some(segment => { - const commandWord = getLeadingCommandWord(segment); - if (!commandWord || !DEV_COMMAND_WORDS.has(commandWord)) { - return false; - } - return devPattern.test(segment) && !tmuxLauncher.test(segment); - }); + const segments = collectCheckSegments(cmd); + const hasBlockedDev = segments.some(isBlockedDevSegment); if (hasBlockedDev) { console.error('[Hook] BLOCKED: Dev server must run in tmux for log access');