* fix(#3129): replace bypassed bash regex with token-walk git-cmd.js classifier Root cause: gsd-validate-commit.sh used: if [[ "$CMD" =~ ^git[[:space:]]+commit ]] This regex silently bypasses Conventional Commits enforcement for: git -C /path commit -m ... (working-directory prefix) GIT_AUTHOR_NAME=x git commit (env-var prefix) /usr/bin/git commit -m ... (full-path executable) Fix: introduces hooks/lib/git-cmd.js with isGitSubcommand(cmd, sub) — a token-walk classifier that handles all four forms by: 1. Skipping leading VAR=VALUE env assignments 2. Validating the git executable (basename check for full-path support) 3. Consuming git global options (-C <path>, --git-dir=, -p, etc.) 4. Checking the subcommand token The hook delegates to this classifier via node shell-out. node is already called twice in this hook (config check + JSON parse), so no new runtime dependency. This becomes the single source of truth for all hooks that gate on git subcommands (pre-commit-review-gate, post-push-verify, etc.). Regression test: 27 assertions — tokenize correctness, 12 must-match cases (including all 3 bypass forms), 8 must-not-match cases, 3 source checks. All are real behavioral tests, not string comparisons. Suite: 7035/7035. Closes #3129. * fix(lint+hook+changeset): allow-test-rule, fix HOOK_DIR quote injection, fix changeset pr+typo
This commit is contained in:
5
.changeset/fix-3129-validate-commit-bypass.md
Normal file
5
.changeset/fix-3129-validate-commit-bypass.md
Normal file
@@ -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.
|
||||
@@ -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
|
||||
|
||||
150
hooks/lib/git-cmd.js
Normal file
150
hooks/lib/git-cmd.js
Normal file
@@ -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 };
|
||||
119
tests/bug-3129-validate-commit-git-bypass.test.cjs
Normal file
119
tests/bug-3129-validate-commit-git-bypass.test.cjs
Normal file
@@ -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',
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user