From dae663d85664887e438747e1af9db245080da4d1 Mon Sep 17 00:00:00 2001 From: Ke Wang Date: Sun, 12 Apr 2026 19:53:15 -0500 Subject: [PATCH 1/2] fix: route block-no-verify hook through run-with-flags.js MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace inline `npx block-no-verify@1.1.2` with a standalone Node.js script routed through `run-with-flags.js`, matching every other hook. Fixes two bugs: 1. npx inherits the project cwd and triggers EBADDEVENGINES in pnpm-only projects that set devEngines.packageManager.onFail=error. 2. The hook bypassed run-with-flags.js so ECC_DISABLED_HOOKS had no effect — the isHookEnabled() check never ran. The new script replicates the full block-no-verify@1.1.2 detection logic (--no-verify, -n shorthand for commit, core.hooksPath override) with zero external dependencies. Closes #1378 --- hooks/hooks.json | 2 +- scripts/hooks/block-no-verify.js | 219 +++++++++++++++++++++++++++++++ 2 files changed, 220 insertions(+), 1 deletion(-) create mode 100644 scripts/hooks/block-no-verify.js diff --git a/hooks/hooks.json b/hooks/hooks.json index 528b03f8..8cba1b8f 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -7,7 +7,7 @@ "hooks": [ { "type": "command", - "command": "npx block-no-verify@1.1.2" + "command": "node \"${CLAUDE_PLUGIN_ROOT}/scripts/hooks/run-with-flags.js\" \"pre:bash:block-no-verify\" \"scripts/hooks/block-no-verify.js\" \"standard,strict\"" } ], "description": "Block git hook-bypass flag to protect pre-commit, commit-msg, and pre-push hooks from being skipped", diff --git a/scripts/hooks/block-no-verify.js b/scripts/hooks/block-no-verify.js new file mode 100644 index 00000000..1ac0bc42 --- /dev/null +++ b/scripts/hooks/block-no-verify.js @@ -0,0 +1,219 @@ +#!/usr/bin/env node +/** + * PreToolUse Hook: Block --no-verify flag + * + * Blocks git hook-bypass flags (--no-verify, -c core.hooksPath=) to protect + * pre-commit, commit-msg, and pre-push hooks from being skipped by AI agents. + * + * Replaces the previous npx-based invocation that failed in pnpm-only projects + * (EBADDEVENGINES) and could not be disabled via ECC_DISABLED_HOOKS. + * + * Exit codes: + * 0 = allow (not a git command or no bypass flags) + * 2 = block (bypass flag detected) + */ + +'use strict'; + +const MAX_STDIN = 1024 * 1024; +let raw = ''; + +/** + * Git commands that support the --no-verify flag. + */ +const GIT_COMMANDS_WITH_NO_VERIFY = [ + 'commit', + 'push', + 'merge', + 'cherry-pick', + 'rebase', + 'am', +]; + +/** + * Characters that can appear immediately before 'git' in a command string. + */ +const VALID_BEFORE_GIT = ' \t\n\r;&|$`(<{!"\']/.~\\'; + +/** + * Check if a position in the input is inside a shell comment. + */ +function isInComment(input, idx) { + const lineStart = input.lastIndexOf('\n', idx - 1) + 1; + const before = input.slice(lineStart, idx); + for (let i = 0; i < before.length; i++) { + if (before.charAt(i) === '#') { + const prev = i > 0 ? before.charAt(i - 1) : ''; + if (prev !== '$' && prev !== '\\') return true; + } + } + return false; +} + +/** + * Find the next 'git' token in the input starting from a position. + */ +function findGit(input, start) { + let pos = start; + while (pos < input.length) { + const idx = input.indexOf('git', pos); + if (idx === -1) return null; + + const isExe = input.slice(idx + 3, idx + 7).toLowerCase() === '.exe'; + const len = isExe ? 7 : 3; + const after = input[idx + len] || ' '; + if (!/[\s"']/.test(after)) { + pos = idx + 1; + continue; + } + + const before = idx > 0 ? input[idx - 1] : ' '; + if (VALID_BEFORE_GIT.includes(before)) return { idx, len }; + pos = idx + 1; + } + return null; +} + +/** + * Detect which git subcommand (commit, push, etc.) is being invoked. + */ +function detectGitCommand(input) { + let start = 0; + while (start < input.length) { + const git = findGit(input, start); + if (!git) return null; + + if (isInComment(input, git.idx)) { + start = git.idx + git.len; + continue; + } + + for (const cmd of GIT_COMMANDS_WITH_NO_VERIFY) { + const cmdIdx = input.indexOf(cmd, git.idx + git.len); + if (cmdIdx === -1) continue; + + const before = cmdIdx > 0 ? input[cmdIdx - 1] : ' '; + const after = input[cmdIdx + cmd.length] || ' '; + if (!/\s/.test(before)) continue; + if (!/[\s;&#|>)\]}"']/.test(after) && after !== '') continue; + if (/[;|]/.test(input.slice(git.idx + git.len, cmdIdx))) continue; + if (isInComment(input, cmdIdx)) continue; + + return cmd; + } + + start = git.idx + git.len; + } + return null; +} + +/** + * Check if the input contains a --no-verify flag for a specific git command. + */ +function hasNoVerifyFlag(input, command) { + if (/--no-verify\b/.test(input)) return true; + + // For commit, -n is shorthand for --no-verify + if (command === 'commit') { + if (/\s-n(?:\s|$)/.test(input) || /\s-n[a-zA-Z]/.test(input)) return true; + } + + return false; +} + +/** + * Check if the input contains a -c core.hooksPath= override. + */ +function hasHooksPathOverride(input) { + return /-c\s+["']?core\.hooksPath\s*=/.test(input); +} + +/** + * Check a command string for git hook bypass attempts. + */ +function checkCommand(input) { + const gitCommand = detectGitCommand(input); + if (!gitCommand) return { blocked: false }; + + if (hasNoVerifyFlag(input, gitCommand)) { + return { + blocked: true, + reason: `BLOCKED: --no-verify flag is not allowed with git ${gitCommand}. Git hooks must not be bypassed.`, + }; + } + + if (hasHooksPathOverride(input)) { + return { + blocked: true, + reason: `BLOCKED: Overriding core.hooksPath is not allowed with git ${gitCommand}. Git hooks must not be bypassed.`, + }; + } + + return { blocked: false }; +} + +/** + * Extract the command string from hook input (JSON or plain text). + */ +function extractCommand(rawInput) { + const trimmed = rawInput.trim(); + if (!trimmed.startsWith('{')) return trimmed; + + try { + const parsed = JSON.parse(trimmed); + if (typeof parsed !== 'object' || parsed === null) return trimmed; + + // Claude Code format: { tool_input: { command: "..." } } + const cmd = parsed.tool_input?.command; + if (typeof cmd === 'string') return cmd; + + // Generic JSON formats + for (const key of ['command', 'cmd', 'input', 'shell', 'script']) { + if (typeof parsed[key] === 'string') return parsed[key]; + } + + return trimmed; + } catch { + return trimmed; + } +} + +/** + * Exportable run() for in-process execution via run-with-flags.js. + */ +function run(rawInput) { + const command = extractCommand(rawInput); + const result = checkCommand(command); + + if (result.blocked) { + return { + exitCode: 2, + stderr: result.reason, + }; + } + + return { exitCode: 0 }; +} + +module.exports = { run }; + +// Stdin fallback for spawnSync execution +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); + } +}); + +process.stdin.on('end', () => { + const command = extractCommand(raw); + const result = checkCommand(command); + + if (result.blocked) { + process.stderr.write(result.reason + '\n'); + process.exit(2); + } + + process.stdout.write(raw); +}); From 809e0fa0a91879f06ec66d383cfe327a2d03705e Mon Sep 17 00:00:00 2001 From: Ke Wang Date: Sun, 12 Apr 2026 20:29:01 -0500 Subject: [PATCH 2/2] fix: address PR review comments on block-no-verify hook - Add `minimal` profile so the security hook runs in all profiles - Scope -n/--no-verify flag check to the detected subcommand region, preventing false positives on chained commands (e.g. `git log -n 10`) - Guard stdin listeners with `require.main === module` so require() from run-with-flags.js does not register unnecessary listeners - Verify subcommand token is preceded only by flags/flag-args after "git", preventing misclassification of argument values as subcommands - Add integration tests for block-no-verify hook Co-Authored-By: Claude Opus 4.6 (1M context) --- hooks/hooks.json | 2 +- scripts/hooks/block-no-verify.js | 114 +++++++++++++++++------- tests/hooks/block-no-verify.test.js | 132 ++++++++++++++++++++++++++++ 3 files changed, 215 insertions(+), 33 deletions(-) create mode 100644 tests/hooks/block-no-verify.test.js diff --git a/hooks/hooks.json b/hooks/hooks.json index 8cba1b8f..cdd2584d 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -7,7 +7,7 @@ "hooks": [ { "type": "command", - "command": "node \"${CLAUDE_PLUGIN_ROOT}/scripts/hooks/run-with-flags.js\" \"pre:bash:block-no-verify\" \"scripts/hooks/block-no-verify.js\" \"standard,strict\"" + "command": "node \"${CLAUDE_PLUGIN_ROOT}/scripts/hooks/run-with-flags.js\" \"pre:bash:block-no-verify\" \"scripts/hooks/block-no-verify.js\" \"minimal,standard,strict\"" } ], "description": "Block git hook-bypass flag to protect pre-commit, commit-msg, and pre-push hooks from being skipped", diff --git a/scripts/hooks/block-no-verify.js b/scripts/hooks/block-no-verify.js index 1ac0bc42..864b6289 100644 --- a/scripts/hooks/block-no-verify.js +++ b/scripts/hooks/block-no-verify.js @@ -76,6 +76,8 @@ function findGit(input, start) { /** * Detect which git subcommand (commit, push, etc.) is being invoked. + * Returns { command, offset } where offset is the position right after the + * subcommand keyword, so callers can scope flag checks to only that portion. */ function detectGitCommand(input) { let start = 0; @@ -88,18 +90,58 @@ function detectGitCommand(input) { continue; } + // Find the first matching subcommand token after "git". + // We pick the one closest to "git" so that argument values like + // "git push origin commit" don't misclassify "commit" as the subcommand. + let bestCmd = null; + let bestIdx = Infinity; + for (const cmd of GIT_COMMANDS_WITH_NO_VERIFY) { - const cmdIdx = input.indexOf(cmd, git.idx + git.len); - if (cmdIdx === -1) continue; + let searchPos = git.idx + git.len; + while (searchPos < input.length) { + const cmdIdx = input.indexOf(cmd, searchPos); + if (cmdIdx === -1) break; - const before = cmdIdx > 0 ? input[cmdIdx - 1] : ' '; - const after = input[cmdIdx + cmd.length] || ' '; - if (!/\s/.test(before)) continue; - if (!/[\s;&#|>)\]}"']/.test(after) && after !== '') continue; - if (/[;|]/.test(input.slice(git.idx + git.len, cmdIdx))) continue; - if (isInComment(input, cmdIdx)) continue; + const before = cmdIdx > 0 ? input[cmdIdx - 1] : ' '; + const after = input[cmdIdx + cmd.length] || ' '; + if (!/\s/.test(before)) { searchPos = cmdIdx + 1; continue; } + if (!/[\s;&#|>)\]}"']/.test(after) && after !== '') { searchPos = cmdIdx + 1; continue; } + if (/[;|]/.test(input.slice(git.idx + git.len, cmdIdx))) break; + if (isInComment(input, cmdIdx)) { searchPos = cmdIdx + 1; continue; } - return cmd; + // Verify this token is the first non-flag word after "git" — i.e. the + // actual subcommand, not an argument value to a different subcommand. + const gap = input.slice(git.idx + git.len, cmdIdx); + const tokens = gap.trim().split(/\s+/).filter(Boolean); + // Every token before the candidate must be a flag or a flag argument. + // Git global flags like -c take a value argument (e.g. -c key=value). + let onlyFlagsAndArgs = true; + let expectFlagArg = false; + for (const t of tokens) { + if (expectFlagArg) { expectFlagArg = false; continue; } + if (t.startsWith('-')) { + // -c is a git global flag that takes the next token as its argument + if (t === '-c' || t === '-C' || t === '--work-tree' || t === '--git-dir' || + t === '--namespace' || t === '--super-prefix') { + expectFlagArg = true; + } + continue; + } + onlyFlagsAndArgs = false; + break; + } + if (!onlyFlagsAndArgs) { searchPos = cmdIdx + 1; continue; } + + if (cmdIdx < bestIdx) { + bestIdx = cmdIdx; + bestCmd = cmd; + } + break; + } + } + + if (bestCmd) { + return { command: bestCmd, offset: bestIdx + bestCmd.length }; } start = git.idx + git.len; @@ -109,13 +151,17 @@ function detectGitCommand(input) { /** * Check if the input contains a --no-verify flag for a specific git command. + * Only inspects the portion of the input starting at `offset` (the position + * right after the detected subcommand keyword) so that flags belonging to + * earlier commands in a chain are not falsely matched. */ -function hasNoVerifyFlag(input, command) { - if (/--no-verify\b/.test(input)) return true; +function hasNoVerifyFlag(input, command, offset) { + const region = input.slice(offset); + if (/--no-verify\b/.test(region)) return true; // For commit, -n is shorthand for --no-verify if (command === 'commit') { - if (/\s-n(?:\s|$)/.test(input) || /\s-n[a-zA-Z]/.test(input)) return true; + if (/\s-n(?:\s|$)/.test(region) || /\s-n[a-zA-Z]/.test(region)) return true; } return false; @@ -132,10 +178,12 @@ function hasHooksPathOverride(input) { * Check a command string for git hook bypass attempts. */ function checkCommand(input) { - const gitCommand = detectGitCommand(input); - if (!gitCommand) return { blocked: false }; + const detected = detectGitCommand(input); + if (!detected) return { blocked: false }; - if (hasNoVerifyFlag(input, gitCommand)) { + const { command: gitCommand, offset } = detected; + + if (hasNoVerifyFlag(input, gitCommand, offset)) { return { blocked: true, reason: `BLOCKED: --no-verify flag is not allowed with git ${gitCommand}. Git hooks must not be bypassed.`, @@ -197,23 +245,25 @@ function run(rawInput) { module.exports = { run }; -// Stdin fallback for spawnSync execution -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); - } -}); +// Stdin fallback for spawnSync execution — only when invoked directly, not via require() +if (require.main === module) { + 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); + } + }); -process.stdin.on('end', () => { - const command = extractCommand(raw); - const result = checkCommand(command); + process.stdin.on('end', () => { + const command = extractCommand(raw); + const result = checkCommand(command); - if (result.blocked) { - process.stderr.write(result.reason + '\n'); - process.exit(2); - } + if (result.blocked) { + process.stderr.write(result.reason + '\n'); + process.exit(2); + } - process.stdout.write(raw); -}); + process.stdout.write(raw); + }); +} diff --git a/tests/hooks/block-no-verify.test.js b/tests/hooks/block-no-verify.test.js new file mode 100644 index 00000000..de6e4aca --- /dev/null +++ b/tests/hooks/block-no-verify.test.js @@ -0,0 +1,132 @@ +/** + * Tests for scripts/hooks/block-no-verify.js via run-with-flags.js + */ + +const assert = require('assert'); +const path = require('path'); +const { spawnSync } = require('child_process'); + +const runner = path.join(__dirname, '..', '..', 'scripts', 'hooks', 'run-with-flags.js'); + +function test(name, fn) { + try { + fn(); + console.log(` \u2713 ${name}`); + return true; + } catch (error) { + console.log(` \u2717 ${name}`); + console.log(` Error: ${error.message}`); + return false; + } +} + +function runHook(input, env = {}) { + const rawInput = typeof input === 'string' ? input : JSON.stringify(input); + const result = spawnSync('node', [runner, 'pre:bash:block-no-verify', 'scripts/hooks/block-no-verify.js', 'minimal,standard,strict'], { + input: rawInput, + encoding: 'utf8', + env: { + ...process.env, + ECC_HOOK_PROFILE: 'standard', + ...env + }, + timeout: 15000, + stdio: ['pipe', 'pipe', 'pipe'] + }); + + return { + code: Number.isInteger(result.status) ? result.status : 1, + stdout: result.stdout || '', + stderr: result.stderr || '' + }; +} + +let passed = 0; +let failed = 0; + +console.log('\nblock-no-verify hook tests'); +console.log('─'.repeat(50)); + +// --- Basic allow/block --- + +if (test('allows plain git commit', () => { + const r = runHook({ tool_input: { command: 'git commit -m "hello"' } }); + assert.strictEqual(r.code, 0, `expected exit 0, got ${r.code}: ${r.stderr}`); +})) passed++; else failed++; + +if (test('blocks --no-verify on git commit', () => { + const r = runHook({ tool_input: { command: 'git commit --no-verify -m "msg"' } }); + assert.strictEqual(r.code, 2, `expected exit 2, got ${r.code}`); + assert.ok(r.stderr.includes('BLOCKED'), `stderr should contain BLOCKED: ${r.stderr}`); +})) passed++; else failed++; + +if (test('blocks -n shorthand on git commit', () => { + const r = runHook({ tool_input: { command: 'git commit -n -m "msg"' } }); + assert.strictEqual(r.code, 2, `expected exit 2, got ${r.code}`); + assert.ok(r.stderr.includes('BLOCKED'), `stderr should contain BLOCKED: ${r.stderr}`); +})) passed++; else failed++; + +if (test('blocks core.hooksPath override', () => { + const r = runHook({ tool_input: { command: 'git -c core.hooksPath=/dev/null commit -m "msg"' } }); + assert.strictEqual(r.code, 2, `expected exit 2, got ${r.code}`); + assert.ok(r.stderr.includes('core.hooksPath'), `stderr should mention core.hooksPath: ${r.stderr}`); +})) passed++; else failed++; + +// --- Chained command false positive prevention (Comment 2) --- + +if (test('does not false-positive on -n belonging to git log in a chain', () => { + const r = runHook({ tool_input: { command: 'git log -n 10 && git commit -m "msg"' } }); + assert.strictEqual(r.code, 0, `expected exit 0, got ${r.code}: ${r.stderr}`); +})) passed++; else failed++; + +if (test('does not false-positive on --no-verify in a prior non-git command', () => { + const r = runHook({ tool_input: { command: 'echo --no-verify && git commit -m "msg"' } }); + assert.strictEqual(r.code, 0, `expected exit 0, got ${r.code}: ${r.stderr}`); +})) passed++; else failed++; + +if (test('still blocks --no-verify on the git commit part of a chain', () => { + const r = runHook({ tool_input: { command: 'git log -n 5 && git commit --no-verify -m "msg"' } }); + assert.strictEqual(r.code, 2, `expected exit 2, got ${r.code}`); +})) passed++; else failed++; + +// --- Subcommand detection (Comment 4) --- + +if (test('does not misclassify "commit" as subcommand when it is an argument to push', () => { + // "git push origin commit" — "commit" is a refspec arg, not the subcommand + const r = runHook({ tool_input: { command: 'git push origin commit' } }); + // This should detect "push" as the subcommand, not "commit" + // Either way it should not block since there's no --no-verify + assert.strictEqual(r.code, 0, `expected exit 0, got ${r.code}: ${r.stderr}`); +})) passed++; else failed++; + +// --- Blocks on push --no-verify --- + +if (test('blocks --no-verify on git push', () => { + const r = runHook({ tool_input: { command: 'git push --no-verify' } }); + assert.strictEqual(r.code, 2, `expected exit 2, got ${r.code}`); + assert.ok(r.stderr.includes('git push'), `stderr should mention git push: ${r.stderr}`); +})) passed++; else failed++; + +// --- Non-git commands pass through --- + +if (test('allows non-git commands', () => { + const r = runHook({ tool_input: { command: 'npm test' } }); + assert.strictEqual(r.code, 0, `expected exit 0, got ${r.code}: ${r.stderr}`); +})) passed++; else failed++; + +// --- Plain text input (not JSON) --- + +if (test('handles plain text input', () => { + const r = runHook('git commit -m "hello"'); + assert.strictEqual(r.code, 0, `expected exit 0, got ${r.code}: ${r.stderr}`); +})) passed++; else failed++; + +if (test('blocks plain text input with --no-verify', () => { + const r = runHook('git commit --no-verify -m "msg"'); + assert.strictEqual(r.code, 2, `expected exit 2, got ${r.code}`); +})) passed++; else failed++; + +console.log('─'.repeat(50)); +console.log(`Passed: ${passed} Failed: ${failed}`); + +process.exit(failed > 0 ? 1 : 0);