fix: parse block-no-verify flags by shell words

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) {
+function detectGitCommand(input, start = 0) {
-  let 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);
+  const segmentEnd = findCommandSegmentEnd(input, offset);
-  if (/--no-verify\b/.test(region)) return true;
+  const tokens = tokenizeShellWords(input, offset, segmentEnd);
+  let skipNext = false;
+
+  for (const token of tokens) {
+    const value = token.value;
+
+    if (skipNext) {
+      skipNext = false;
+      continue;
+    }
+
+    if (value === '--') {
+      break;
+    }
 
-  // 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;
+      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,19 +408,48 @@ function hasNoVerifyFlag(input, command, offset) {
 /**
  * Check if the input contains a -c core.hooksPath= override.
  */
-function hasHooksPathOverride(input) {
+function hasHooksPathOverride(input, detected) {
-  return /-c\s+["']?core\.hooksPath\s*=/.test(input);
+  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);
+  let start = 0;
+
+  while (start < input.length) {
+    const detected = detectGitCommand(input, start);
     if (!detected) return { blocked: false };
 
     const { command: gitCommand, offset } = detected;
 
+    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,
@@ -190,11 +457,7 @@ function checkCommand(input) {
       };
     }
 
-  if (hasHooksPathOverride(input)) {
+    start = findCommandSegmentEnd(input, offset) + 1;
-    return {
-      blocked: true,
-      reason: `BLOCKED: Overriding core.hooksPath is not allowed with git ${gitCommand}. Git hooks must not be bypassed.`,
-    };
   }
 
   return { blocked: false };

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', () => {