From 6be241a463910fa32b8db17425b87d751dad39e1 Mon Sep 17 00:00:00 2001 From: SeungHyun Date: Wed, 13 May 2026 11:28:12 +0900 Subject: [PATCH] fix: close block-no-verify bypass holes Backport Jamkris's fix for case-insensitive core.hooksPath overrides and the git commit -tn template-path false positive. Verified locally on current main with 25/25 block-no-verify tests and node tests/run-all.js passing 2369/2369. --- scripts/hooks/block-no-verify.js | 22 ++++++++++++++++++---- tests/hooks/block-no-verify.test.js | 18 ++++++++++++++++++ 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/scripts/hooks/block-no-verify.js b/scripts/hooks/block-no-verify.js index 84979b96..13807548 100644 --- a/scripts/hooks/block-no-verify.js +++ b/scripts/hooks/block-no-verify.js @@ -35,7 +35,12 @@ const GIT_COMMANDS_WITH_NO_VERIFY = [ */ const VALID_BEFORE_GIT = ' \t\n\r;&|$`(<{!"\']/.~\\'; -const GIT_CONFIG_KEY_PREFIX = 'core.hooksPath='; +// Git config section and variable names are case-insensitive +// (subsection names are case-sensitive but core.hooksPath has none), +// so we normalize the candidate token to lowercase before matching. +// See https://git-scm.com/docs/git-config — "The variable names are +// case-insensitive." +const GIT_CONFIG_KEY_PREFIX = 'core.hookspath='; const COMMIT_OPTIONS_WITH_VALUE = new Set([ '-m', @@ -67,7 +72,12 @@ const COMMIT_OPTIONS_WITH_INLINE_VALUE = [ '--pathspec-from-file=', ]; -const COMMIT_SHORT_OPTIONS_WITH_VALUE = new Set(['m', 'F', 'C', 'c']); +// Short options that take a value. When seen as part of a combined +// short-option token (e.g. -tn), git's parser treats the rest of the +// token as the option's value (template path 'n' here), so the scanner +// must stop at this character — anything after it is the inline value, +// not another flag. +const COMMIT_SHORT_OPTIONS_WITH_VALUE = new Set(['m', 'F', 'C', 'c', 't']); function tokenizeShellWords(input, start = 0, end = input.length) { const tokens = []; @@ -413,17 +423,21 @@ function hasHooksPathOverride(input, detected) { for (let i = 0; i < tokens.length; i++) { const value = tokens[i].value; + // Git config section + variable names are case-insensitive, so a + // bypass attempt like `core.HOOKSPATH=...` or `core.hookspath=...` + // must compare against the lowercased token. + const lowered = value.toLowerCase(); if (value === '-c') { const next = tokens[i + 1] && tokens[i + 1].value; - if (typeof next === 'string' && next.startsWith(GIT_CONFIG_KEY_PREFIX)) { + if (typeof next === 'string' && next.toLowerCase().startsWith(GIT_CONFIG_KEY_PREFIX)) { return true; } i++; continue; } - if (value.startsWith(`-c${GIT_CONFIG_KEY_PREFIX}`)) { + if (lowered.startsWith(`-c${GIT_CONFIG_KEY_PREFIX}`)) { return true; } } diff --git a/tests/hooks/block-no-verify.test.js b/tests/hooks/block-no-verify.test.js index 45dbc292..f610030c 100644 --- a/tests/hooks/block-no-verify.test.js +++ b/tests/hooks/block-no-verify.test.js @@ -179,6 +179,24 @@ if (test('blocks plain text input with --no-verify', () => { assert.strictEqual(r.code, 2, `expected exit 2, got ${r.code}`); })) passed++; else failed++; +// --- Case-insensitivity of git config keys + -t template short option --- + +if (test('blocks case-variant core.hooksPath (lowercase)', () => { + 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(/core\.hookspath/i.test(r.stderr), `stderr should mention core.hooksPath: ${r.stderr}`); +})) passed++; else failed++; + +if (test('blocks case-variant core.hooksPath (uppercase)', () => { + 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}`); +})) passed++; else failed++; + +if (test('still allows -tn (n is the -t template path, not a flag)', () => { + const r = runHook({ tool_input: { command: 'git commit -tn -m "msg"' } }); + assert.strictEqual(r.code, 0, `expected exit 0, got ${r.code}: ${r.stderr}`); +})) passed++; else failed++; + console.log('─'.repeat(50)); console.log(`Passed: ${passed} Failed: ${failed}`);