From a8c6da485d144f44d31b74c4a8d4e78b8f04d572 Mon Sep 17 00:00:00 2001 From: Peopleoftech Date: Tue, 4 Aug 2026 23:08:56 +0200 Subject: [PATCH] fix(hooks): catch the bypass short flag anywhere in a cluster (#2668) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit isCommitNoVerifyShortFlag anchored on the first character, so it only recognised the flag when it led the cluster. Git clusters short options, which means git commit -an is -a plus the bypass flag and skips the hooks. -sn and -vn slip through the same way, while -na and -nm are caught — the difference is position, not intent. Scanning now walks the cluster and stops at a value-taking option, since that option swallows the rest as its inline value. The n in -mn stays message text, and the existing -tn case keeps working. Adds 4 tests: the three clustered forms that were escaping, plus -mn to pin the inline-value boundary. Verified the three fail against current main. Suite 25 to 29. Co-authored-by: haelyra <49814733+haelyra@users.noreply.github.com> --- scripts/hooks/block-no-verify.js | 20 +++++++++++++++++++- tests/hooks/block-no-verify.test.js | 22 ++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/scripts/hooks/block-no-verify.js b/scripts/hooks/block-no-verify.js index 138075484..ecd29100c 100644 --- a/scripts/hooks/block-no-verify.js +++ b/scripts/hooks/block-no-verify.js @@ -248,7 +248,25 @@ function getCommitShortValueOption(value) { } function isCommitNoVerifyShortFlag(value) { - return value === '-n' || /^-n[a-zA-Z]/.test(value); + if (!value.startsWith('-') || value.startsWith('--') || value === '-') { + return false; + } + + // Short options cluster, so -n need not lead: `git commit -an` is -a plus -n + // and bypasses the hooks just as `-n` does. Anchoring on the first character + // let -an, -sn and -vn through. + // + // Scanning stops at a value-taking option because that option swallows the + // rest of the cluster as its inline value — the n in `-mn` is message text, + // not a flag. + const options = value.slice(1); + for (let i = 0; i < options.length; i++) { + const option = options.charAt(i); + if (option === 'n') return true; + if (COMMIT_SHORT_OPTIONS_WITH_VALUE.has(option)) return false; + } + + return false; } /** diff --git a/tests/hooks/block-no-verify.test.js b/tests/hooks/block-no-verify.test.js index f610030c6..db38dbb19 100644 --- a/tests/hooks/block-no-verify.test.js +++ b/tests/hooks/block-no-verify.test.js @@ -115,6 +115,28 @@ if (test('allows -n after combined -am message option', () => { assert.strictEqual(r.code, 0, `expected exit 0, got ${r.code}: ${r.stderr}`); })) passed++; else failed++; +// --- Short options cluster, so -n need not lead --- + +if (test('blocks -n clustered after -a', () => { + const r = runHook({ tool_input: { command: 'git commit -an -m "msg"' } }); + assert.strictEqual(r.code, 2, `expected exit 2, got ${r.code}`); +})) passed++; else failed++; + +if (test('blocks -n clustered after -s', () => { + const r = runHook({ tool_input: { command: 'git commit -sn -m "msg"' } }); + assert.strictEqual(r.code, 2, `expected exit 2, got ${r.code}`); +})) passed++; else failed++; + +if (test('blocks -n clustered after -v', () => { + const r = runHook({ tool_input: { command: 'git commit -vn -m "msg"' } }); + assert.strictEqual(r.code, 2, `expected exit 2, got ${r.code}`); +})) passed++; else failed++; + +if (test('allows -mn, where n is the inline message and not a flag', () => { + const r = runHook({ tool_input: { command: 'git commit -mn' } }); + 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}`);