From 0dcde13384eb556a238d6b01a7d715a2ca8cff80 Mon Sep 17 00:00:00 2001 From: Affaan Mustafa Date: Wed, 29 Apr 2026 21:50:05 -0400 Subject: [PATCH] fix: parse block-no-verify flags by shell words --- scripts/hooks/block-no-verify.js | 311 +++++++++++++++++++++++++--- tests/hooks/block-no-verify.test.js | 53 +++++ 2 files changed, 340 insertions(+), 24 deletions(-) diff --git a/scripts/hooks/block-no-verify.js b/scripts/hooks/block-no-verify.js index 864b6289..84979b96 100644 --- a/scripts/hooks/block-no-verify.js +++ b/scripts/hooks/block-no-verify.js @@ -35,6 +35,212 @@ const GIT_COMMANDS_WITH_NO_VERIFY = [ */ const VALID_BEFORE_GIT = ' \t\n\r;&|$`(<{!"\']/.~\\'; +const GIT_CONFIG_KEY_PREFIX = 'core.hooksPath='; + +const COMMIT_OPTIONS_WITH_VALUE = new Set([ + '-m', + '--message', + '-F', + '--file', + '-C', + '--reuse-message', + '-c', + '--reedit-message', + '--author', + '--date', + '--template', + '--fixup', + '--squash', + '--pathspec-from-file', +]); + +const COMMIT_OPTIONS_WITH_INLINE_VALUE = [ + '--message=', + '--file=', + '--reuse-message=', + '--reedit-message=', + '--author=', + '--date=', + '--template=', + '--fixup=', + '--squash=', + '--pathspec-from-file=', +]; + +const COMMIT_SHORT_OPTIONS_WITH_VALUE = new Set(['m', 'F', 'C', 'c']); + +function tokenizeShellWords(input, start = 0, end = input.length) { + const tokens = []; + let value = ''; + let tokenStart = null; + let quote = null; + let escaped = false; + + function beginToken(index) { + if (tokenStart === null) { + tokenStart = index; + } + } + + function pushToken(index) { + if (tokenStart === null) { + return; + } + + tokens.push({ + value, + start: tokenStart, + end: index, + }); + value = ''; + tokenStart = null; + } + + for (let i = start; i < end; i++) { + const char = input.charAt(i); + + if (escaped) { + beginToken(i - 1); + value += char; + escaped = false; + continue; + } + + if (quote) { + if (char === quote) { + quote = null; + continue; + } + + if (quote === '"' && char === '\\') { + beginToken(i); + escaped = true; + continue; + } + + beginToken(i); + value += char; + continue; + } + + if (char === '"' || char === "'") { + beginToken(i); + quote = char; + continue; + } + + if (char === '\\') { + beginToken(i); + escaped = true; + continue; + } + + if (/\s/.test(char)) { + pushToken(i); + continue; + } + + beginToken(i); + value += char; + } + + if (escaped) { + value += '\\'; + } + pushToken(end); + + return tokens; +} + +function findCommandSegmentEnd(input, start) { + let quote = null; + let escaped = false; + + for (let i = start; i < input.length; i++) { + const char = input.charAt(i); + + if (escaped) { + escaped = false; + continue; + } + + if (quote) { + if (quote === '"' && char === '\\') { + escaped = true; + continue; + } + if (char === quote) { + quote = null; + } + continue; + } + + if (char === '"' || char === "'") { + quote = char; + continue; + } + + if (char === '\\') { + escaped = true; + continue; + } + + if (char === ';' || char === '|' || char === '&' || char === '\n') { + return i; + } + } + + return input.length; +} + +function commitOptionConsumesNextValue(value) { + if (isCommitNoVerifyShortFlag(value)) { + return false; + } + + if (COMMIT_OPTIONS_WITH_VALUE.has(value)) { + return true; + } + + const shortValueOption = getCommitShortValueOption(value); + return Boolean(shortValueOption && shortValueOption.consumesNextValue); +} + +function commitOptionContainsInlineValue(value) { + if (isCommitNoVerifyShortFlag(value)) { + return false; + } + + if (COMMIT_OPTIONS_WITH_INLINE_VALUE.some(prefix => value.startsWith(prefix))) { + return true; + } + + const shortValueOption = getCommitShortValueOption(value); + return Boolean(shortValueOption && shortValueOption.containsInlineValue); +} + +function getCommitShortValueOption(value) { + if (!value.startsWith('-') || value.startsWith('--') || value === '-') { + return null; + } + + const options = value.slice(1); + for (let i = 0; i < options.length; i++) { + if (COMMIT_SHORT_OPTIONS_WITH_VALUE.has(options.charAt(i))) { + return { + consumesNextValue: i === options.length - 1, + containsInlineValue: i < options.length - 1, + }; + } + } + + return null; +} + +function isCommitNoVerifyShortFlag(value) { + return value === '-n' || /^-n[a-zA-Z]/.test(value); +} + /** * Check if a position in the input is inside a shell comment. */ @@ -79,8 +285,7 @@ function findGit(input, start) { * 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; +function detectGitCommand(input, start = 0) { while (start < input.length) { const git = findGit(input, start); if (!git) return null; @@ -141,7 +346,13 @@ function detectGitCommand(input) { } if (bestCmd) { - return { command: bestCmd, offset: bestIdx + bestCmd.length }; + return { + command: bestCmd, + offset: bestIdx + bestCmd.length, + gitStart: git.idx, + gitEnd: git.idx + git.len, + commandStart: bestIdx, + }; } start = git.idx + git.len; @@ -156,12 +367,39 @@ function detectGitCommand(input) { * earlier commands in a chain are not falsely matched. */ function hasNoVerifyFlag(input, command, offset) { - const region = input.slice(offset); - if (/--no-verify\b/.test(region)) return true; + const segmentEnd = findCommandSegmentEnd(input, offset); + const tokens = tokenizeShellWords(input, offset, segmentEnd); + let skipNext = false; - // For commit, -n is shorthand for --no-verify - if (command === 'commit') { - if (/\s-n(?:\s|$)/.test(region) || /\s-n[a-zA-Z]/.test(region)) return true; + for (const token of tokens) { + const value = token.value; + + if (skipNext) { + skipNext = false; + continue; + } + + if (value === '--') { + break; + } + + if (command === 'commit') { + if (commitOptionConsumesNextValue(value)) { + skipNext = true; + continue; + } + + if (commitOptionContainsInlineValue(value)) { + continue; + } + } + + if (value === '--no-verify') return true; + + // For commit, -n is shorthand for --no-verify. + if (command === 'commit' && isCommitNoVerifyShortFlag(value)) { + return true; + } } return false; @@ -170,31 +408,56 @@ function hasNoVerifyFlag(input, command, offset) { /** * Check if the input contains a -c core.hooksPath= override. */ -function hasHooksPathOverride(input) { - return /-c\s+["']?core\.hooksPath\s*=/.test(input); +function hasHooksPathOverride(input, detected) { + const tokens = tokenizeShellWords(input, detected.gitEnd, detected.commandStart); + + for (let i = 0; i < tokens.length; i++) { + const value = tokens[i].value; + + if (value === '-c') { + const next = tokens[i + 1] && tokens[i + 1].value; + if (typeof next === 'string' && next.startsWith(GIT_CONFIG_KEY_PREFIX)) { + return true; + } + i++; + continue; + } + + if (value.startsWith(`-c${GIT_CONFIG_KEY_PREFIX}`)) { + return true; + } + } + + return false; } /** * Check a command string for git hook bypass attempts. */ function checkCommand(input) { - const detected = detectGitCommand(input); - if (!detected) return { blocked: false }; + let start = 0; - const { command: gitCommand, offset } = detected; + while (start < input.length) { + const detected = detectGitCommand(input, start); + if (!detected) return { blocked: false }; - 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.`, - }; - } + const { command: gitCommand, offset } = detected; - if (hasHooksPathOverride(input)) { - return { - blocked: true, - reason: `BLOCKED: Overriding core.hooksPath is not allowed with git ${gitCommand}. Git hooks must not be bypassed.`, - }; + if (hasHooksPathOverride(input, detected)) { + return { + blocked: true, + reason: `BLOCKED: Overriding core.hooksPath is not allowed with git ${gitCommand}. Git hooks must not be bypassed.`, + }; + } + + 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.`, + }; + } + + start = findCommandSegmentEnd(input, offset) + 1; } return { blocked: false }; diff --git a/tests/hooks/block-no-verify.test.js b/tests/hooks/block-no-verify.test.js index de6e4aca..45dbc292 100644 --- a/tests/hooks/block-no-verify.test.js +++ b/tests/hooks/block-no-verify.test.js @@ -72,6 +72,12 @@ if (test('blocks core.hooksPath override', () => { assert.ok(r.stderr.includes('core.hooksPath'), `stderr should mention core.hooksPath: ${r.stderr}`); })) passed++; else failed++; +if (test('blocks quoted core.hooksPath override argument', () => { + 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', () => { @@ -84,11 +90,58 @@ if (test('does not false-positive on --no-verify in a prior non-git command', () assert.strictEqual(r.code, 0, `expected exit 0, got ${r.code}: ${r.stderr}`); })) passed++; else failed++; +if (test('allows --no-verify discussed in a double-quoted commit message', () => { + const r = runHook({ tool_input: { command: 'git commit -m "fix: --no-verify edge case"' } }); + assert.strictEqual(r.code, 0, `expected exit 0, got ${r.code}: ${r.stderr}`); +})) passed++; else failed++; + +if (test('allows --no-verify discussed in a single-quoted commit message', () => { + const r = runHook({ tool_input: { command: "git commit -m 'fix: --no-verify edge case'" } }); + assert.strictEqual(r.code, 0, `expected exit 0, got ${r.code}: ${r.stderr}`); +})) passed++; else failed++; + +if (test('allows -n discussed in a quoted commit message', () => { + const r = runHook({ tool_input: { command: 'git commit -m "Fixed -n bug in module"' } }); + assert.strictEqual(r.code, 0, `expected exit 0, got ${r.code}: ${r.stderr}`); +})) passed++; else failed++; + +if (test('allows --no-verify after combined -am message option', () => { + const r = runHook({ tool_input: { command: 'git commit -am "--no-verify"' } }); + assert.strictEqual(r.code, 0, `expected exit 0, got ${r.code}: ${r.stderr}`); +})) passed++; else failed++; + +if (test('allows -n after combined -am message option', () => { + const r = runHook({ tool_input: { command: 'git commit -am "-n"' } }); + assert.strictEqual(r.code, 0, `expected exit 0, got ${r.code}: ${r.stderr}`); +})) passed++; else failed++; + +if (test('allows core.hooksPath discussed in a quoted commit message', () => { + const r = runHook({ tool_input: { command: 'git commit -m "doc: explain core.hooksPath= setting"' } }); + assert.strictEqual(r.code, 0, `expected exit 0, got ${r.code}: ${r.stderr}`); +})) passed++; else failed++; + +if (test('allows git bypass phrase discussed in a quoted commit message', () => { + const r = runHook({ tool_input: { command: 'git commit -m "doc: explain git push --no-verify risk"' } }); + 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++; +if (test('still blocks a real quoted --no-verify flag', () => { + 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('still blocks bypass flags in later chained git commands', () => { + const r = runHook({ tool_input: { command: 'git commit -m "msg" && 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++; + // --- Subcommand detection (Comment 4) --- if (test('does not misclassify "commit" as subcommand when it is an argument to push', () => {