diff --git a/.changeset/fix-3129-validate-commit-bypass.md b/.changeset/fix-3129-validate-commit-bypass.md new file mode 100644 index 000000000..bbcbc63c2 --- /dev/null +++ b/.changeset/fix-3129-validate-commit-bypass.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3141 +--- +**`gsd-validate-commit.sh` community hook now catches all git commit forms** — the previous `[[ "$CMD" =~ ^git[[:space:]]+commit ]]` bash regex silently bypassed Conventional Commits enforcement for `git -C /path commit`, `GIT_AUTHOR_NAME=x git commit`, and `/usr/bin/git commit`. Introduces `hooks/lib/git-cmd.js` — a token-walk classifier (`isGitSubcommand(cmd, sub)`) that correctly handles env-prefix assignments, `-C path` working-directory flags, full-path executables, `--git-dir=` options, and all git global boolean flags. The hook now delegates detection to this module — the single source of truth for all hooks that gate on git subcommands. Closes #3129. diff --git a/hooks/gsd-validate-commit.sh b/hooks/gsd-validate-commit.sh index ccce5fce5..f8a655e9b 100755 --- a/hooks/gsd-validate-commit.sh +++ b/hooks/gsd-validate-commit.sh @@ -21,8 +21,15 @@ INPUT=$(cat) # Extract command from JSON using Node (handles escaping correctly, no jq needed) CMD=$(echo "$INPUT" | node -e "let d='';process.stdin.on('data',c=>d+=c);process.stdin.on('end',()=>{try{process.stdout.write(JSON.parse(d).tool_input?.command||'')}catch{}})" 2>/dev/null) -# Only check git commit commands -if [[ "$CMD" =~ ^git[[:space:]]+commit ]]; then +# Only check git commit commands. +# Delegates to hooks/lib/git-cmd.js isGitSubcommand() — the canonical token-walk +# classifier that handles env-prefix, -C path, and full-path git invocations. +# A naive `^git\s+commit` regex misses all three; this guard fixes that (#3129). +HOOK_DIR="$(cd "$(dirname "$0")" && pwd)" +if GIT_CMD_LIB="$HOOK_DIR/lib/git-cmd.js" node -e " + const {isGitSubcommand}=require(process.env.GIT_CMD_LIB); + process.exit(isGitSubcommand(process.argv[1],'commit')?0:1); +" "$CMD" 2>/dev/null; then # Extract message from -m flag MSG="" if [[ "$CMD" =~ -m[[:space:]]+\"([^\"]+)\" ]]; then diff --git a/hooks/lib/git-cmd.js b/hooks/lib/git-cmd.js new file mode 100644 index 000000000..b578ca559 --- /dev/null +++ b/hooks/lib/git-cmd.js @@ -0,0 +1,150 @@ +'use strict'; + +/** + * git-cmd.js — token-walk git command classifier. + * + * Determines whether a shell command string invokes a specific git + * subcommand. Handles the four forms that a naive `^git\s+commit` regex + * misses: + * + * bare: git commit -m "..." ✓ + * -C path: git -C /some/path commit -m "..." ✓ (missed by regex) + * env-prefix: GIT_AUTHOR_NAME=x git commit "..." ✓ (missed by regex) + * full-path: /usr/bin/git commit -m "..." ✓ (missed by regex) + * + * This module is the single source of truth for git-commit detection so all + * hooks that need to gate on git commits share one implementation. + * + * Exported by the hooks/lib/ directory — require via a path relative to the + * hook's own __dirname: + * + * const { isGitSubcommand } = require(path.join(__dirname, 'lib', 'git-cmd.js')); + */ + +const path = require('path'); + +/** + * Git global options that take a following argument. + * These must be consumed as (option, argument) pairs when walking tokens. + */ +const ARGUMENT_TAKING_FLAGS = new Set([ + '-C', // working directory + '--git-dir', // path to git repository + '--work-tree', // path to working tree + '--namespace', // git namespace + '--super-prefix', // superproject-relative prefix + '--exec-path', // path to core git programs (when given an arg) + '--html-path', + '--man-path', + '--info-path', + '--list-cmds', +]); + +/** + * Git global flags that consume no extra argument. + */ +const BOOLEAN_FLAGS = new Set([ + '-p', '--paginate', '--no-pager', + '--no-replace-objects', '--bare', + '--literal-pathspecs', '--glob-pathspecs', '--noglob-pathspecs', + '--icase-pathspecs', '--no-optional-locks', + '-P', '--no-lazy-fetch', + '--version', '--help', +]); + +/** + * Tokenize a shell command string. + * Handles single-quoted strings, double-quoted strings, and unquoted tokens. + * Does NOT perform variable expansion or brace expansion. + * + * @param {string} cmd + * @returns {string[]} + */ +function tokenize(cmd) { + const tokens = []; + let i = 0; + const len = cmd.length; + + while (i < len) { + // Skip whitespace + while (i < len && /\s/.test(cmd[i])) i++; + if (i >= len) break; + + let token = ''; + while (i < len && !/\s/.test(cmd[i])) { + if (cmd[i] === "'") { + // Single-quoted string: take everything until closing ' + i++; + while (i < len && cmd[i] !== "'") token += cmd[i++]; + if (i < len) i++; // consume closing ' + } else if (cmd[i] === '"') { + // Double-quoted string: take everything until closing " (no escape handling) + i++; + while (i < len && cmd[i] !== '"') token += cmd[i++]; + if (i < len) i++; // consume closing " + } else { + token += cmd[i++]; + } + } + if (token) tokens.push(token); + } + + return tokens; +} + +/** + * Return true if `cmd` invokes the git subcommand `sub`. + * + * @param {string} cmd - Full shell command string (may include env vars, full paths) + * @param {string} sub - Subcommand to test for, e.g. 'commit' + * @returns {boolean} + */ +function isGitSubcommand(cmd, sub) { + if (!cmd || !sub) return false; + + const tokens = tokenize(cmd); + let i = 0; + + // Phase 1: skip leading VAR=VALUE environment assignments + while (i < tokens.length && /^[A-Za-z_][A-Za-z0-9_]*=/.test(tokens[i])) { + i++; + } + + // Phase 2: the next token must be the git executable + if (i >= tokens.length) return false; + const gitToken = tokens[i++]; + if (path.basename(gitToken) !== 'git') return false; + + // Phase 3: consume git global options + while (i < tokens.length) { + const t = tokens[i]; + + // --flag=value form for argument-taking flags + const eqIdx = t.indexOf('='); + const flagName = eqIdx !== -1 ? t.slice(0, eqIdx) : t; + if (ARGUMENT_TAKING_FLAGS.has(flagName)) { + if (eqIdx !== -1) { + // consumed as one token: --git-dir=.git + i++; + } else { + // consumed as two tokens: -C /path + i += 2; + } + continue; + } + + if (BOOLEAN_FLAGS.has(t)) { + i++; + continue; + } + + // Not a global option — this is the subcommand + break; + } + + // Phase 4: check the subcommand + if (i >= tokens.length) return false; + return tokens[i] === sub; +} + +module.exports = { isGitSubcommand, tokenize }; diff --git a/tests/bug-3129-validate-commit-git-bypass.test.cjs b/tests/bug-3129-validate-commit-git-bypass.test.cjs new file mode 100644 index 000000000..0d386d98c --- /dev/null +++ b/tests/bug-3129-validate-commit-git-bypass.test.cjs @@ -0,0 +1,119 @@ +'use strict'; +// allow-test-rule: reads hook shell script to verify delegation pattern — structural contract test, not source-grep + +// Regression tests for bug #3129. +// +// gsd-validate-commit.sh used `[[ "$CMD" =~ ^git[[:space:]]+commit ]]` to +// detect git commit invocations. This regex silently bypasses Conventional +// Commits enforcement for three real git commit forms: +// 1. git -C /some/path commit -m "..." (working-directory prefix) +// 2. GIT_AUTHOR_NAME=x git commit "..." (env-var prefix) +// 3. /usr/bin/git commit -m "..." (full path) +// +// Fix: the hook delegates detection to hooks/lib/git-cmd.js isGitSubcommand(), +// a token-walk classifier that correctly handles all four forms. The module +// is the canonical single source of truth for all hooks that gate on git commits. + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const path = require('node:path'); +const fs = require('node:fs'); + +const ROOT = path.join(__dirname, '..'); +const { isGitSubcommand, tokenize } = require(path.join(ROOT, 'hooks', 'lib', 'git-cmd.js')); + +// ── tokenize ───────────────────────────────────────────────────────────────── + +describe('git-cmd.js tokenize', () => { + test('splits bare command', () => { + assert.deepEqual(tokenize('git commit -m "msg"'), ['git', 'commit', '-m', 'msg']); + }); + test('handles single-quoted args', () => { + assert.deepEqual(tokenize("git commit -m 'my message'"), ['git', 'commit', '-m', 'my message']); + }); + test('handles env-prefix assignment', () => { + assert.deepEqual( + tokenize('GIT_AUTHOR_NAME=Alice git commit -m "fix"'), + ['GIT_AUTHOR_NAME=Alice', 'git', 'commit', '-m', 'fix'], + ); + }); + test('handles -C path', () => { + assert.deepEqual( + tokenize('git -C /some/path commit -m "x"'), + ['git', '-C', '/some/path', 'commit', '-m', 'x'], + ); + }); +}); + +// ── isGitSubcommand: must-match cases ──────────────────────────────────────── + +describe('git-cmd.js isGitSubcommand: should match commit', () => { + const cases = [ + ['bare form', 'git commit -m "feat: add thing"'], + ['single-quoted message', "git commit -m 'fix: typo'"], + ['with --no-verify', 'git commit --no-verify -m "wip"'], + ['-C path form (bug #3129)', 'git -C /some/path commit -m "fix: x"'], + ['env-prefix form (bug #3129)', 'GIT_AUTHOR_NAME=Alice git commit -m "fix"'], + ['full-path form (bug #3129)', '/usr/bin/git commit -m "feat: y"'], + ['multiple env vars', 'GIT_AUTHOR_NAME=A GIT_AUTHOR_EMAIL=b@c git commit -m "x"'], + ['--git-dir= flag', 'git --git-dir=.git commit -m "x"'], + ['--git-dir two-token', 'git --git-dir .git commit -m "x"'], + ['--no-pager before subcommand', 'git --no-pager commit -m "x"'], + ['-C + full path', '/usr/bin/git -C /proj commit -m "x"'], + ['-p paginate flag', 'git -p commit -m "x"'], + ]; + for (const [desc, cmd] of cases) { + test(desc, () => { + assert.ok(isGitSubcommand(cmd, 'commit'), `Expected match for: ${cmd}`); + }); + } +}); + +// ── isGitSubcommand: must-not-match cases ──────────────────────────────────── + +describe('git-cmd.js isGitSubcommand: should NOT match commit', () => { + const cases = [ + ['git push', 'git push origin main'], + ['git status', 'git status'], + ['git add', 'git add .'], + ['git log', 'git log --oneline'], + ['not git at all', 'npm install'], + ['empty string', ''], + ['git checkout (not commit)', 'git checkout main'], + ['git -C path push', 'git -C /path push'], + ]; + for (const [desc, cmd] of cases) { + test(desc, () => { + assert.ok(!isGitSubcommand(cmd, 'commit'), `Expected NO match for: ${cmd}`); + }); + } +}); + +// ── gsd-validate-commit.sh source check ────────────────────────────────────── + +describe('gsd-validate-commit.sh delegates to git-cmd.js', () => { + const hookSrc = fs.readFileSync( + path.join(ROOT, 'hooks', 'gsd-validate-commit.sh'), 'utf8', + ); + + test('hook no longer uses the stale ^git\\s+commit bash regex', () => { + assert.ok( + !hookSrc.includes('^git[[:space:]]+commit'), + 'gsd-validate-commit.sh still uses the bypassed regex — fix not applied', + ); + }); + + test('hook delegates to git-cmd.js isGitSubcommand', () => { + assert.ok( + hookSrc.includes('git-cmd.js') && hookSrc.includes('isGitSubcommand'), + 'gsd-validate-commit.sh does not reference git-cmd.js or isGitSubcommand', + ); + }); + + test('hooks/lib/git-cmd.js exists at the expected install path', () => { + assert.ok( + fs.existsSync(path.join(ROOT, 'hooks', 'lib', 'git-cmd.js')), + 'hooks/lib/git-cmd.js does not exist — library file missing', + ); + }); +});