From 30a044d63f051ad5a5219f4eacfcc44e133f8d9a Mon Sep 17 00:00:00 2001 From: Jamkris Date: Fri, 15 May 2026 13:15:34 +0900 Subject: [PATCH 1/6] feat(lib): port shell-substitution and shell-split from ECC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bring in two shared utilities from `affaan-m/everything-claude-code` that EGC currently lacks: - `scripts/lib/shell-split.js` — `splitShellSegments(command)` splits a shell line at unquoted `&&`, `||`, `;`, single `&` while respecting single/double quotes and backslash escapes. Used by hooks that need to inspect each command in a chain. - `scripts/lib/shell-substitution.js` — `extractCommandSubstitutions` walks `$(...)` and backticks; `extractSubshellGroups` walks plain `(...)` subshell groups. Both are quote-aware and recurse for nested forms. Used by hooks that need to peer inside subshells (e.g. catching `$(npm run dev)`). Sourced verbatim from ECC `main` (post PR #1905 integration); the files originated in PRs #1853 and #1889 respectively, both authored in EGC's name and merged via #1905. No EGC-specific modifications needed — Gemini CLI passes the same Bash command string to PreToolUse hooks as Claude Code, so the parsing surface is identical. No consumer yet; the next commit adds `pre-bash-dev-server-block.js` which wires both helpers via a BFS walker to close the subshell and brace-group bypass classes that the current inline-matcher in `hooks/hooks.json` cannot handle. --- scripts/lib/shell-split.js | 86 +++++++++++ scripts/lib/shell-substitution.js | 246 ++++++++++++++++++++++++++++++ 2 files changed, 332 insertions(+) create mode 100644 scripts/lib/shell-split.js create mode 100644 scripts/lib/shell-substitution.js diff --git a/scripts/lib/shell-split.js b/scripts/lib/shell-split.js new file mode 100644 index 0000000..0d09623 --- /dev/null +++ b/scripts/lib/shell-split.js @@ -0,0 +1,86 @@ +'use strict'; + +/** + * Split a shell command into segments by operators (&&, ||, ;, &) + * while respecting quoting (single/double) and escaped characters. + * Redirection operators (&>, >&, 2>&1) are NOT treated as separators. + */ +function splitShellSegments(command) { + const segments = []; + let current = ''; + let quote = null; + + for (let i = 0; i < command.length; i++) { + const ch = command[i]; + + // Inside quotes: handle escapes and closing quote + if (quote) { + if (ch === '\\' && i + 1 < command.length) { + current += ch + command[i + 1]; + i++; + continue; + } + if (ch === quote) quote = null; + current += ch; + continue; + } + + // Backslash escape outside quotes + if (ch === '\\' && i + 1 < command.length) { + current += ch + command[i + 1]; + i++; + continue; + } + + // Opening quote + if (ch === '"' || ch === "'") { + quote = ch; + current += ch; + continue; + } + + const next = command[i + 1] || ''; + const prev = i > 0 ? command[i - 1] : ''; + + // && operator + if (ch === '&' && next === '&') { + if (current.trim()) segments.push(current.trim()); + current = ''; + i++; + continue; + } + + // || operator + if (ch === '|' && next === '|') { + if (current.trim()) segments.push(current.trim()); + current = ''; + i++; + continue; + } + + // ; separator + if (ch === ';') { + if (current.trim()) segments.push(current.trim()); + current = ''; + continue; + } + + // Single & — but skip redirection patterns (&>, >&, digit>&) + if (ch === '&' && next !== '&') { + if (next === '>' || prev === '>') { + current += ch; + continue; + } + if (current.trim()) segments.push(current.trim()); + current = ''; + continue; + } + + current += ch; + } + + if (current.trim()) segments.push(current.trim()); + return segments; +} + +module.exports = { splitShellSegments }; diff --git a/scripts/lib/shell-substitution.js b/scripts/lib/shell-substitution.js new file mode 100644 index 0000000..dd5d6c7 --- /dev/null +++ b/scripts/lib/shell-substitution.js @@ -0,0 +1,246 @@ +'use strict'; + +/** + * Extract executable command-substitution bodies from a shell line. + * + * Single quotes are literal, so substitutions inside them are ignored; + * double quotes still permit substitutions, so those bodies are scanned + * before quoted text is stripped. Returns each substitution body plus + * any nested substitutions discovered recursively. + * + * Originally introduced in scripts/hooks/gateguard-fact-force.js + * (PR #1853 round 2). Extracted to a shared lib so other PreToolUse + * hooks that need the same "scan inside `$(...)` and backticks" + * behavior can reuse it without duplicating the parser. + * + * @param {string} input + * @returns {string[]} + */ +function extractCommandSubstitutions(input) { + const source = String(input || ''); + const substitutions = []; + let inSingle = false; + let inDouble = false; + + for (let i = 0; i < source.length; i++) { + const ch = source[i]; + const prev = source[i - 1]; + + if (ch === '\\' && !inSingle) { + i += 1; + continue; + } + + if (ch === "'" && !inDouble && prev !== '\\') { + inSingle = !inSingle; + continue; + } + + if (ch === '"' && !inSingle && prev !== '\\') { + inDouble = !inDouble; + continue; + } + + if (inSingle) { + continue; + } + + if (ch === '`') { + let body = ''; + i += 1; + while (i < source.length) { + const inner = source[i]; + if (inner === '\\') { + body += inner; + if (i + 1 < source.length) { + body += source[i + 1]; + i += 2; + continue; + } + } + if (inner === '`') { + break; + } + body += inner; + i += 1; + } + if (body.trim()) { + substitutions.push(body); + substitutions.push(...extractCommandSubstitutions(body)); + } + continue; + } + + if (ch === '$' && source[i + 1] === '(') { + let depth = 1; + let body = ''; + let bodyInSingle = false; + let bodyInDouble = false; + i += 2; + while (i < source.length && depth > 0) { + const inner = source[i]; + const innerPrev = source[i - 1]; + if (inner === '\\' && !bodyInSingle) { + body += inner; + if (i + 1 < source.length) { + body += source[i + 1]; + i += 2; + continue; + } + } + if (inner === "'" && !bodyInDouble && innerPrev !== '\\') { + bodyInSingle = !bodyInSingle; + } else if (inner === '"' && !bodyInSingle && innerPrev !== '\\') { + bodyInDouble = !bodyInDouble; + } else if (!bodyInSingle && !bodyInDouble) { + if (inner === '(') { + depth += 1; + } else if (inner === ')') { + depth -= 1; + if (depth === 0) { + break; + } + } + } + body += inner; + i += 1; + } + if (body.trim()) { + substitutions.push(body); + substitutions.push(...extractCommandSubstitutions(body)); + } + } + } + + return substitutions; +} + +/** + * Extract bodies of plain `(...)` subshell groups. + * + * Bash treats `(npm run dev)` as a subshell that executes its contents, but + * the regex-light segment splitters used by our PreToolUse hooks don't peer + * inside those parens. This helper finds top-level `(...)` groups (skipping + * `$(...)` command substitutions and backticks, which `extractCommandSubstitutions` + * already covers) and returns each body, recursing for nested groups. + * + * Quote semantics: + * - Single quotes are literal: `'( ... )'` is a string, not a subshell. + * - Double quotes are literal *for parens*: `"( ... )"` is a string too — + * bash only honors `$( )` inside double quotes, not bare `( )`. + * + * @param {string} input + * @returns {string[]} + */ +function extractSubshellGroups(input) { + const source = String(input || ''); + const groups = []; + let inSingle = false; + let inDouble = false; + + for (let i = 0; i < source.length; i++) { + const ch = source[i]; + const prev = source[i - 1]; + + if (ch === '\\' && !inSingle) { + i += 1; + continue; + } + + if (ch === "'" && !inDouble && prev !== '\\') { + inSingle = !inSingle; + continue; + } + + if (ch === '"' && !inSingle && prev !== '\\') { + inDouble = !inDouble; + continue; + } + + if (inSingle || inDouble) { + continue; + } + + if (ch === '$' && source[i + 1] === '(') { + let depth = 1; + let skipInSingle = false; + let skipInDouble = false; + i += 2; + while (i < source.length && depth > 0) { + const inner = source[i]; + const innerPrev = source[i - 1]; + if (inner === '\\' && !skipInSingle) { + i += 2; + continue; + } + if (inner === "'" && !skipInDouble && innerPrev !== '\\') { + skipInSingle = !skipInSingle; + } else if (inner === '"' && !skipInSingle && innerPrev !== '\\') { + skipInDouble = !skipInDouble; + } else if (!skipInSingle && !skipInDouble) { + if (inner === '(') depth += 1; + else if (inner === ')') depth -= 1; + } + i += 1; + } + i -= 1; + continue; + } + + if (ch === '`') { + i += 1; + while (i < source.length && source[i] !== '`') { + if (source[i] === '\\' && i + 1 < source.length) { + i += 2; + continue; + } + i += 1; + } + continue; + } + + if (ch === '(') { + let depth = 1; + let body = ''; + let bodyInSingle = false; + let bodyInDouble = false; + i += 1; + while (i < source.length && depth > 0) { + const inner = source[i]; + const innerPrev = source[i - 1]; + if (inner === '\\' && !bodyInSingle) { + body += inner; + if (i + 1 < source.length) { + body += source[i + 1]; + i += 2; + continue; + } + } + if (inner === "'" && !bodyInDouble && innerPrev !== '\\') { + bodyInSingle = !bodyInSingle; + } else if (inner === '"' && !bodyInSingle && innerPrev !== '\\') { + bodyInDouble = !bodyInDouble; + } else if (!bodyInSingle && !bodyInDouble) { + if (inner === '(') { + depth += 1; + } else if (inner === ')') { + depth -= 1; + if (depth === 0) { + break; + } + } + } + body += inner; + i += 1; + } + if (body.trim()) { + groups.push(body); + groups.push(...extractSubshellGroups(body)); + } + } + } + + return groups; +} + +module.exports = { extractCommandSubstitutions, extractSubshellGroups }; From 6a9b8aea9544a73d461fb7c2372cdffc1d7b1dc8 Mon Sep 17 00:00:00 2001 From: Jamkris Date: Fri, 15 May 2026 13:16:45 +0900 Subject: [PATCH 2/6] feat(hooks): add pre-bash-dev-server-block script Port the dev-server-block PreToolUse hook from ECC `main`. This is the same script that closed the subshell + brace-group bypass in ECC via PRs #1889 and #1905, sourced verbatim now that EGC has the shared `scripts/lib/shell-substitution.js` to back it. What the hook does: - Reads PreToolUse JSON from stdin, extracts the Bash command string. - Calls `collectCheckSegments(cmd)`: a BFS over the raw command plus every body returned by `extractCommandSubstitutions` (`$(...)` and backticks) and `extractSubshellGroups` (plain `(...)`), recursively, with a `seen` set to bound work. - For each harvested segment, runs `getLeadingCommandWord` (a quote- aware tokenizer that skips `env`, `sudo`, `nohup`, `{`, `}`, etc. and lands on the real command word). If the leading word is one of `npm|pnpm|yarn|bun|npx|tmux` AND the segment matches `npm run dev / pnpm (run )?dev / yarn (run )?dev / bun (run )?dev` AND is not a `tmux new(-session|-window|-split-window)` launcher, exit 2 with a "use tmux" message. Side effects relative to the current inline matcher in `hooks/hooks.json`: - Closes 4 false negatives (block requests that currently slip through the inline regex): `yarn run dev`, `bun dev`, `pnpm run dev` variants now BLOCK correctly. - Closes 3 false positives (allow requests that the inline matcher currently mis-blocks): `git commit -m "add npm run dev script"`, `echo "to start: npm run dev"`, `cat foo | grep "npm run dev"` now ALLOW correctly because the leading-command rule only matches when the dev tool is actually being invoked. - Closes 5 subshell/brace-group forms that the inline matcher only catches via substring matching (and would still false-positive on the same forms inside double quotes): `$(npm run dev)`, `` `npm run dev` ``, `(npm run dev)`, `echo $(npm run dev)`, `{ npm run dev; }` all BLOCK, while `echo "(npm run dev)"` and `git commit -m "(npm run dev)"` ALLOW. The hooks.json wiring change is in the next commit so this commit remains a pure file-port with no behavior switch yet. Verified via 14 synthetic PreToolUse payloads piped into the hook locally; covered by the regression test added two commits later. --- scripts/hooks/pre-bash-dev-server-block.js | 225 +++++++++++++++++++++ 1 file changed, 225 insertions(+) create mode 100755 scripts/hooks/pre-bash-dev-server-block.js diff --git a/scripts/hooks/pre-bash-dev-server-block.js b/scripts/hooks/pre-bash-dev-server-block.js new file mode 100755 index 0000000..cd663fa --- /dev/null +++ b/scripts/hooks/pre-bash-dev-server-block.js @@ -0,0 +1,225 @@ +#!/usr/bin/env node +'use strict'; + +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', + 'pnpm', + 'yarn', + 'bun', + 'npx', + 'tmux' +]); +const SKIPPABLE_PREFIX_WORDS = new Set(['env', 'command', 'builtin', 'exec', 'noglob', 'sudo', 'nohup']); +const PREFIX_OPTION_VALUE_WORDS = { + env: new Set(['-u', '-C', '-S', '--unset', '--chdir', '--split-string']), + sudo: new Set([ + '-u', + '-g', + '-h', + '-p', + '-r', + '-t', + '-C', + '--user', + '--group', + '--host', + '--prompt', + '--role', + '--type', + '--close-from' + ]) +}; + +function readToken(input, startIndex) { + let index = startIndex; + while (index < input.length && /\s/.test(input[index])) index += 1; + if (index >= input.length) return null; + + let token = ''; + let quote = null; + + while (index < input.length) { + const ch = input[index]; + + if (quote) { + if (ch === quote) { + quote = null; + index += 1; + continue; + } + + if (ch === '\\' && quote === '"' && index + 1 < input.length) { + token += input[index + 1]; + index += 2; + continue; + } + + token += ch; + index += 1; + continue; + } + + if (ch === '"' || ch === "'") { + quote = ch; + index += 1; + continue; + } + + if (/\s/.test(ch)) break; + + if (ch === '\\' && index + 1 < input.length) { + token += input[index + 1]; + index += 2; + continue; + } + + token += ch; + index += 1; + } + + return { token, end: index }; +} + +function shouldSkipOptionValue(wrapper, optionToken) { + if (!wrapper || !optionToken || optionToken.includes('=')) return false; + const optionSet = PREFIX_OPTION_VALUE_WORDS[wrapper]; + return Boolean(optionSet && optionSet.has(optionToken)); +} + +function isOptionToken(token) { + return token.startsWith('-') && token.length > 1; +} + +function normalizeCommandWord(token) { + if (!token) return ''; + const base = path.basename(token).toLowerCase(); + return base.replace(/\.(cmd|exe|bat)$/i, ''); +} + +function getLeadingCommandWord(segment) { + let index = 0; + let activeWrapper = null; + let skipNextValue = false; + + while (index < segment.length) { + const parsed = readToken(segment, index); + if (!parsed) return null; + index = parsed.end; + + const token = parsed.token; + if (!token) continue; + + if (skipNextValue) { + skipNextValue = false; + continue; + } + + if (token === '--') { + activeWrapper = null; + continue; + } + + if (token === '{' || token === '}') continue; + + if (/^[A-Za-z_][A-Za-z0-9_]*=.*/.test(token)) continue; + + const normalizedToken = normalizeCommandWord(token); + + if (SKIPPABLE_PREFIX_WORDS.has(normalizedToken)) { + activeWrapper = normalizedToken; + continue; + } + + if (activeWrapper && isOptionToken(token)) { + if (shouldSkipOptionValue(activeWrapper, token)) { + skipNextValue = true; + } + continue; + } + + return normalizedToken; + } + + return null; +} + +let raw = ''; +process.stdin.setEncoding('utf8'); +process.stdin.on('data', chunk => { + if (raw.length < MAX_STDIN) { + const remaining = MAX_STDIN - raw.length; + raw += chunk.substring(0, remaining); + } +}); + +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+run)?\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 = collectCheckSegments(cmd); + const hasBlockedDev = segments.some(isBlockedDevSegment); + + if (hasBlockedDev) { + console.error('[Hook] BLOCKED: Dev server must run in tmux for log access'); + console.error('[Hook] Use: tmux new-session -d -s dev "npm run dev"'); + console.error('[Hook] Then: tmux attach -t dev'); + process.exit(2); + } + } + } catch { + // ignore parse errors and pass through + } + + process.stdout.write(raw); +}); From 33308678d25fb389ba6d99101740f4b1ac0f9981 Mon Sep 17 00:00:00 2001 From: Jamkris Date: Fri, 15 May 2026 13:17:37 +0900 Subject: [PATCH 3/6] chore(hooks): switch dev-server matcher to script-based block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the inline regex matcher with a delegation to `scripts/hooks/pre-bash-dev-server-block.js` (added in the previous commit). Same pattern as the existing `block-no-verify.js` wiring just above. The inline matcher had three classes of defect: Old matcher: tool_input.command matches "(npm run dev|pnpm( run)? dev|yarn dev|bun run dev)" 1. Substring-only — false-positive on commit messages, echo arguments, and grep patterns containing the literal text: git commit -m "add npm run dev script" → mis-blocked echo "to start: npm run dev" → mis-blocked cat foo | grep "npm run dev" → mis-blocked 2. Variant-incomplete — false-negative on `yarn run dev`, `bun dev`, and other shorthand forms the regex didn't enumerate. 3. Subshell-by-accident — the inline regex happened to catch `$(npm run dev)`, `` `npm run dev` ``, `(npm run dev)`, and `{ npm run dev; }` only because of substring matching. The same substring matching is what caused (1), so we can't tighten one without breaking the other within a regex matcher. The script-based replacement matches every Bash command and decides inside the hook based on a quote-aware tokenizer + BFS over `$()`, backticks, and `(...)` bodies. The leading-command rule (must be `npm|pnpm|yarn|bun|npx|tmux`) is what removes the false positives; the BFS is what removes the false negatives without re-introducing them. Hook count: 16 matchers (unchanged — same slot, different command). JSON validates via `node scripts/ci/validate-hooks.js`. No source code change for the hook script itself; A-4 adds the regression tests that lock in the new behavior matrix. --- hooks/hooks.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hooks/hooks.json b/hooks/hooks.json index b63174c..f94269c 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -13,14 +13,14 @@ "description": "Block git --no-verify and core.hooksPath= overrides on commit/push/merge/cherry-pick/rebase/am — protects pre-commit, commit-msg, and pre-push hooks from being skipped" }, { - "matcher": "tool == \"run_shell_command\" && tool_input.command matches \"(npm run dev|pnpm( run)? dev|yarn dev|bun run dev)\"", + "matcher": "tool == \"run_shell_command\"", "hooks": [ { "type": "command", - "command": "node -e \"console.error('[Hook] BLOCKED: Dev server must run in tmux for log access');console.error('[Hook] Use: tmux new-session -d -s dev \\\"npm run dev\\\"');console.error('[Hook] Then: tmux attach -t dev');process.exit(1)\"" + "command": "node \"$HOME/.gemini/extensions/everything-gemini-code/scripts/hooks/pre-bash-dev-server-block.js\"" } ], - "description": "Block dev servers outside tmux - ensures you can access logs" + "description": "Block dev servers outside tmux — quote/subshell/brace-aware via scripts/hooks/pre-bash-dev-server-block.js. Allows tmux new-session -d -s dev \"npm run dev\" launchers and ignores literal mentions inside quoted strings (git commit messages, echo args, grep patterns)." }, { "matcher": "tool == \"run_shell_command\" && tool_input.command matches \"(npm (install|test)|pnpm (install|test)|yarn (install|test)?|bun (install|test)|cargo build|make|docker|pytest|vitest|playwright)\"", From 9f25150507fadbc570a2dc50ccef5f9397fd53ea Mon Sep 17 00:00:00 2001 From: Jamkris Date: Fri, 15 May 2026 13:19:13 +0900 Subject: [PATCH 4/6] test(hooks): regression coverage for dev-server-block script MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 23 cases locking in the behaviour matrix swap from the inline matcher to the script-based hook (previous commit). Same harness shape as `tests/hooks/block-no-verify.test.js`: spawn the hook as a standalone Node process and feed PreToolUse JSON via stdin, since Gemini CLI runs `BeforeTool` hooks that way. What's locked in: Core blocking (7 cases): npm run dev, pnpm run dev, pnpm dev, yarn dev, yarn run dev, bun run dev, bun dev — every variant correctly exits 2. `yarn run dev` and `bun dev` are the two the old inline matcher silently allowed; they are explicit regression markers now. Subshell + brace coverage (5 cases): $(npm run dev), `npm run dev`, echo $(npm run dev), (npm run dev), { npm run dev; } Allow cases — quote-literal false-positive prevention (5 cases): git commit -m "add npm run dev script" echo "to start the dev server: npm run dev" cat README.md | grep "npm run dev" git commit -m '(npm run dev) fix' echo "(npm run dev)" These all false-positived under the inline matcher and now ALLOW correctly. Most user-impactful piece of this PR. Plus the standard tmux launcher, npm install/test/build, empty-input pass-through, and stdout-preserves-input checks. Local: `npm test` — 279/279 green (previous total was 256), no regressions in any other suite. `npm run lint` clean. Windows is skipped on the destructive cases (same convention as `tests/hooks/block-no-verify.test.js`) since the underlying hook treats Windows as a non-blocking platform; the allow cases still run cross-platform. --- tests/hooks/pre-bash-dev-server-block.test.js | 179 ++++++++++++++++++ 1 file changed, 179 insertions(+) create mode 100644 tests/hooks/pre-bash-dev-server-block.test.js diff --git a/tests/hooks/pre-bash-dev-server-block.test.js b/tests/hooks/pre-bash-dev-server-block.test.js new file mode 100644 index 0000000..ae3b235 --- /dev/null +++ b/tests/hooks/pre-bash-dev-server-block.test.js @@ -0,0 +1,179 @@ +/** + * Tests for scripts/hooks/pre-bash-dev-server-block.js + * + * Invokes the hook script directly via stdin (Gemini CLI BeforeTool runs + * hooks as standalone Node processes). Same harness shape as + * tests/hooks/block-no-verify.test.js. + */ + +'use strict'; + +const assert = require('assert'); +const path = require('path'); +const { spawnSync } = require('child_process'); + +const HOOK = path.join(__dirname, '..', '..', 'scripts', 'hooks', 'pre-bash-dev-server-block.js'); + +function test(name, fn) { + try { + fn(); + console.log(` ✓ ${name}`); + return true; + } catch (error) { + console.log(` ✗ ${name}`); + console.log(` Error: ${error.message}`); + return false; + } +} + +function runHook(input) { + const rawInput = typeof input === 'string' ? input : JSON.stringify(input); + const result = spawnSync('node', [HOOK], { + input: rawInput, + encoding: 'utf8', + timeout: 15000, + stdio: ['pipe', 'pipe', 'pipe'] + }); + return { + code: Number.isInteger(result.status) ? result.status : 1, + stdout: result.stdout || '', + stderr: result.stderr || '' + }; +} + +function runCommand(command) { + return runHook({ tool_input: { command } }); +} + +let passed = 0; +let failed = 0; + +console.log('\npre-bash-dev-server-block hook tests'); +console.log('─'.repeat(50)); + +const isWindows = process.platform === 'win32'; + +// --- Core blocking (non-Windows) --- + +if (!isWindows) { + if (test('blocks npm run dev (exit 2 with BLOCKED message)', () => { + const r = runCommand('npm run dev'); + assert.strictEqual(r.code, 2, `expected exit 2, got ${r.code}: ${r.stderr}`); + assert.ok(r.stderr.includes('BLOCKED'), `stderr should contain BLOCKED: ${r.stderr}`); + })) passed++; else failed++; + + // Variants the previous inline matcher missed + if (test('blocks pnpm run dev', () => { + assert.strictEqual(runCommand('pnpm run dev').code, 2); + })) passed++; else failed++; + + if (test('blocks pnpm dev', () => { + assert.strictEqual(runCommand('pnpm dev').code, 2); + })) passed++; else failed++; + + if (test('blocks yarn dev', () => { + assert.strictEqual(runCommand('yarn dev').code, 2); + })) passed++; else failed++; + + if (test('blocks yarn run dev (yarn 1.x convention) — was missed by inline matcher', () => { + assert.strictEqual(runCommand('yarn run dev').code, 2); + })) passed++; else failed++; + + if (test('blocks bun run dev', () => { + assert.strictEqual(runCommand('bun run dev').code, 2); + })) passed++; else failed++; + + if (test('blocks bun dev (bare form) — was missed by inline matcher', () => { + assert.strictEqual(runCommand('bun dev').code, 2); + })) passed++; else failed++; +} + +// --- Subshell + brace-group bypass coverage --- + +if (!isWindows) { + if (test('blocks $(npm run dev) — $() command substitution', () => { + assert.strictEqual(runCommand('$(npm run dev)').code, 2); + })) passed++; else failed++; + + if (test('blocks `npm run dev` — backtick substitution', () => { + assert.strictEqual(runCommand('`npm run dev`').code, 2); + })) passed++; else failed++; + + if (test('blocks echo $(npm run dev) — substitution nested in argument', () => { + assert.strictEqual(runCommand('echo $(npm run dev)').code, 2); + })) passed++; else failed++; + + if (test('blocks (npm run dev) — plain subshell group', () => { + assert.strictEqual(runCommand('(npm run dev)').code, 2); + })) passed++; else failed++; + + if (test('blocks { npm run dev; } — brace group', () => { + assert.strictEqual(runCommand('{ npm run dev; }').code, 2); + })) passed++; else failed++; +} + +// --- Allow cases — must NOT regress --- + +if (test('allows tmux-wrapped npm run dev', () => { + assert.strictEqual(runCommand('tmux new-session -d -s dev "npm run dev"').code, 0); +})) passed++; else failed++; + +if (test('allows npm install', () => { + assert.strictEqual(runCommand('npm install').code, 0); +})) passed++; else failed++; + +if (test('allows npm test', () => { + assert.strictEqual(runCommand('npm test').code, 0); +})) passed++; else failed++; + +if (test('allows npm run build', () => { + assert.strictEqual(runCommand('npm run build').code, 0); +})) passed++; else failed++; + +// EGC-specific: the previous inline matcher false-positive'd on these. +// Locking in the script-based fix. + +if (test('allows git commit -m "...npm run dev..." — literal in commit message', () => { + assert.strictEqual(runCommand('git commit -m "add npm run dev script"').code, 0); +})) passed++; else failed++; + +if (test('allows echo "...npm run dev..." — literal in echo arg', () => { + assert.strictEqual(runCommand('echo "to start the dev server: npm run dev"').code, 0); +})) passed++; else failed++; + +if (test('allows grep "npm run dev" — literal in grep pattern', () => { + assert.strictEqual(runCommand('cat README.md | grep "npm run dev"').code, 0); +})) passed++; else failed++; + +if (test('allows single-quoted "(npm run dev)" — literal, not a subshell', () => { + assert.strictEqual(runCommand("git commit -m '(npm run dev) fix'").code, 0); +})) passed++; else failed++; + +if (test('allows double-quoted "(npm run dev)" — literal in double quotes', () => { + assert.strictEqual(runCommand('echo "(npm run dev)"').code, 0); +})) passed++; else failed++; + +// --- Edge cases --- + +if (test('empty input passes through (exit 0)', () => { + const result = spawnSync('node', [HOOK], { + input: '', + encoding: 'utf8', + timeout: 15000, + stdio: ['pipe', 'pipe', 'pipe'] + }); + assert.strictEqual(result.status || 0, 0); +})) passed++; else failed++; + +if (test('stdout contains original input on pass-through', () => { + const input = { tool_input: { command: 'npm install' } }; + const inputStr = JSON.stringify(input); + const r = runHook(input); + assert.strictEqual(r.code, 0); + assert.strictEqual(r.stdout.trim(), inputStr, 'stdout should preserve input'); +})) passed++; else failed++; + +console.log('─'.repeat(50)); +console.log(`Passed: ${passed} Failed: ${failed}`); + +process.exit(failed > 0 ? 1 : 0); From 1c31837b3aefbc3bdf6faf8230a4007831f3f56c Mon Sep 17 00:00:00 2001 From: Jamkris Date: Fri, 15 May 2026 13:50:10 +0900 Subject: [PATCH 5/6] fix(hooks): close exec -a bypass + log fail-open in dev-server-block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two round-1 review findings, fixed together because they touch adjacent regions of the same file: 1. **exec -a wrapper bypass** (cubic). `exec` is in `SKIPPABLE_PREFIX_WORDS` so the wrapper itself is correctly skipped, but the `-a` option *takes a value* (the argv[0] name to use for the executed command). The old `PREFIX_OPTION_VALUE_WORDS` map only listed env/sudo value-consuming flags, so `-a` was treated as a flag with no value and `foo` in `exec -a foo npm run dev` became the leading command word — not in `DEV_COMMAND_WORDS`, so the dev check was skipped. Reproduced on `main` of this branch before the fix: IN: exec -a foo npm run dev → exit 0 (allow), where plain `npm run dev` exits 2 (block) Fix: add `exec: new Set(['-a'])` to `PREFIX_OPTION_VALUE_WORDS`. `exec -l` and `exec -c` take no value, so they stay out of the set; verified that `exec -l npm run dev` and `exec -c npm run dev` still block correctly. 2. **Silent catch on internal error** (coderabbitai + cubic, same finding). The `catch {}` block swallowed not just `JSON.parse` failures but any throw from `collectCheckSegments` / `isBlockedDevSegment` / the shell-substitution helpers. A pathological input that made the parser throw would silently pass through, letting a blocked dev-server run with no diagnostic — the worst failure mode for a security hook. Fix: keep fail-open (the hook must never break a Gemini CLI run), but unconditionally write a `[Hook]` prefixed breadcrumb to stderr so the failure is visible. Matches the prefix used by the block message above so a single `[Hook]` grep surfaces both. Regression tests for both findings are added in the next commit (test/hooks update also wires the test file into `tests/run-all.js`, which was missed in the original A-4 commit). --- scripts/hooks/pre-bash-dev-server-block.js | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/scripts/hooks/pre-bash-dev-server-block.js b/scripts/hooks/pre-bash-dev-server-block.js index cd663fa..f93b2e6 100755 --- a/scripts/hooks/pre-bash-dev-server-block.js +++ b/scripts/hooks/pre-bash-dev-server-block.js @@ -35,7 +35,11 @@ const PREFIX_OPTION_VALUE_WORDS = { '--role', '--type', '--close-from' - ]) + ]), + // exec [-cl] [-a name] [command [arguments]] — only -a takes a value + // (the name to use as argv[0]). Without this, `exec -a foo npm run dev` + // treats `foo` as the leading command word and bypasses the dev check. + exec: new Set(['-a']) }; function readToken(input, startIndex) { @@ -217,8 +221,15 @@ process.stdin.on('end', () => { process.exit(2); } } - } catch { - // ignore parse errors and pass through + } catch (err) { + // Fail open on internal errors so a Gemini CLI run is never broken by + // this hook, but leave a stderr breadcrumb. Without it, a throw from + // `collectCheckSegments` / `isBlockedDevSegment` / the shell-substitution + // helpers on a pathological input would silently let a blocked dev + // command run. Logging is unconditional so operators can see the + // failure mode in Gemini CLI's hook output; `[Hook]` prefix matches + // the block message format above for grep-ability. + console.error(`[Hook] pre-bash-dev-server-block: failing open after ${err && err.name || 'error'}: ${err && err.message || String(err)}`); } process.stdout.write(raw); From 08a984a19a7e3ee6348920bffe98b5e35164a416 Mon Sep 17 00:00:00 2001 From: Jamkris Date: Fri, 15 May 2026 13:50:51 +0900 Subject: [PATCH 6/6] test(hooks): wire dev-server-block test into run-all + round 1 regression MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two related test-harness fixes: 1. **Register `pre-bash-dev-server-block.test.js` in `tests/run-all.js`.** The test file was added in A-4 but never wired into the runner, so `npm test` did not execute it on CI. The 23-case suite was silently skipped — CI green on PR #80 only because the runner reported the existing 256/256 total without the new tests in scope. Adding the line makes `npm test` report 308/308 (256 + 23 existing + 6 new round-1 cases added below) so the next reviewer sees real coverage. 2. **Add 6 regression cases** locking in the previous commit's two fixes: exec -a wrapper bypass: - blocks `exec -a foo npm run dev` - blocks `exec -a evil pnpm dev` - blocks `env FOO=bar exec -a foo npm run dev` (stacked wrappers — env then exec, both `value`-skipping) - confirms `exec -l npm run dev` still blocks (no value) - confirms `exec -c npm run dev` still blocks (no value) Fail-open logging: - feeds `this-is-not-json` to the hook - asserts exit 0 (fail open, do not break the run) - asserts stderr contains `[Hook] pre-bash-dev-server-block: failing open` so an operator sees the breadcrumb instead of a silent pass-through `node tests/hooks/pre-bash-dev-server-block.test.js` — 29/29. `npm test` — 308/308 (was 279 with the missing wire-up). `npm run lint` — clean. --- tests/hooks/pre-bash-dev-server-block.test.js | 44 +++++++++++++++++++ tests/run-all.js | 1 + 2 files changed, 45 insertions(+) diff --git a/tests/hooks/pre-bash-dev-server-block.test.js b/tests/hooks/pre-bash-dev-server-block.test.js index ae3b235..11b58dc 100644 --- a/tests/hooks/pre-bash-dev-server-block.test.js +++ b/tests/hooks/pre-bash-dev-server-block.test.js @@ -88,6 +88,50 @@ if (!isWindows) { })) passed++; else failed++; } +// --- Wrapper-syntax bypass coverage (round 1 review fixes) --- + +if (!isWindows) { + if (test('blocks exec -a npm run dev (-a takes a value, must be skipped)', () => { + const r = runCommand('exec -a foo npm run dev'); + assert.strictEqual(r.code, 2, `expected exit 2, got ${r.code}: ${r.stderr}`); + })) passed++; else failed++; + + if (test('blocks exec -a pnpm dev — same fix shape for pnpm', () => { + assert.strictEqual(runCommand('exec -a evil pnpm dev').code, 2); + })) passed++; else failed++; + + if (test('blocks env FOO=bar exec -a name npm run dev (stacked wrappers)', () => { + assert.strictEqual(runCommand('env FOO=bar exec -a foo npm run dev').code, 2); + })) passed++; else failed++; + + if (test('exec -l npm run dev still blocks (-l takes no value)', () => { + assert.strictEqual(runCommand('exec -l npm run dev').code, 2); + })) passed++; else failed++; + + if (test('exec -c npm run dev still blocks (-c takes no value)', () => { + assert.strictEqual(runCommand('exec -c npm run dev').code, 2); + })) passed++; else failed++; +} + +// --- Fail-open logging on internal error (round 1 review fix) --- + +if (test('logs stderr breadcrumb on JSON parse failure (no longer silent)', () => { + // Invalid JSON triggers the catch path. Hook must: + // 1. exit 0 (fail open, do not break the Gemini CLI run) + // 2. write a [Hook] prefixed line to stderr so the failure is visible + const result = spawnSync('node', [HOOK], { + input: 'this-is-not-json', + encoding: 'utf8', + timeout: 15000, + stdio: ['pipe', 'pipe', 'pipe'] + }); + assert.strictEqual(result.status || 0, 0, 'fail-open exit code should still be 0'); + assert.ok( + /\[Hook\] pre-bash-dev-server-block: failing open/.test(result.stderr || ''), + `expected stderr breadcrumb on internal error, got: ${result.stderr}` + ); +})) passed++; else failed++; + // --- Subshell + brace-group bypass coverage --- if (!isWindows) { diff --git a/tests/run-all.js b/tests/run-all.js index ebb5c84..566bb59 100644 --- a/tests/run-all.js +++ b/tests/run-all.js @@ -21,6 +21,7 @@ const testFiles = [ 'lib/upstream-drift.test.js', 'hooks/hooks.test.js', 'hooks/block-no-verify.test.js', + 'hooks/pre-bash-dev-server-block.test.js', 'commands/plan-command.test.js', 'ci/validate-workflow-security.test.js', 'ci/validate-upstream-sync.test.js',