feat(hook-pack): warn-default push-to-main + commit-size PreToolUse gates#248
Conversation
…se gates New Bash-matched hook-pack PreToolUse hook with two gates: a direct git push to a protected branch (main/master/release-*), and a git commit staging more than 15 files. Pure logic in lib/hook-pack-gate (unit-tested); the hook is an I/O shell. Mode CLAUDE_CI_HOOKPACK_GATE=warn (default) | block | off — warn never blocks, so zero disruption until opt-in. Registered second in PreToolUse with a Bash matcher so gateguard stays first and non-Bash tools keep the two-subprocess hot path. Closes the WILD pilot from the 2026-06-17 report-coverage map. Plan: docs/plans/2026-06-17-enforcing-hook-pack.md
There was a problem hiding this comment.
Code Review
This pull request introduces a new PreToolUse hook pack (hook-pack) that gates direct pushes to protected branches and oversized commits exceeding 15 staged files. The feedback highlights several critical edge cases in the git command parsing logic: parsePushTarget and classifyCommand fail to account for global git options, pushing explicitly to HEAD or @ bypasses branch protection fallback, and fully qualified ref names (e.g., refs/heads/main) are not recognized as protected. The reviewer provided robust code suggestions to address these parsing vulnerabilities and recommended expanding the test suite to cover these scenarios.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| function parsePushTarget(command: string): string | null { | ||
| const tokens = command.split(/\s+/).filter(Boolean); | ||
| const pushIdx = tokens.findIndex((t, i) => t === "push" && tokens[i - 1] === "git"); | ||
| if (pushIdx === -1) return null; | ||
| const operands = tokens.slice(pushIdx + 1).filter((t) => !t.startsWith("-")); | ||
| // operands[0] is the remote; operands[1] is the refspec. Fewer than two means | ||
| // no explicit branch (e.g. `git push` or `git push origin`). | ||
| if (operands.length < 2) return null; | ||
| const refspec = operands[1]!; | ||
| return refspec.includes(":") ? (refspec.split(":").pop() ?? null) : refspec; | ||
| } |
There was a problem hiding this comment.
There are two critical issues in parsePushTarget:
- It assumes
pushis always adjacent togit(tokens[pushIdx - 1] === "git"). If global options are used (e.g.,git -c foo=bar push),pushIdxbecomes-1, returningnulland falling back to the current branch. If the user is on a feature branch, this completely bypasses the gate and allows pushing directly tomain! - If the explicit target is
HEADor@(e.g.,git push origin HEAD), it resolves the target branch to"HEAD", which is not marked as protected, bypassing the gate even if the current branch ismain. Pushing toHEADshould fall back to the current branch.
We can fix both issues by finding push anywhere after git and returning null (to trigger the current branch fallback) if the target is HEAD or @.
function parsePushTarget(command: string): string | null {
const tokens = command.split(/\s+/).filter(Boolean);
const gitIdx = tokens.indexOf("git");
if (gitIdx === -1) return null;
const pushIdx = tokens.indexOf("push", gitIdx + 1);
if (pushIdx === -1) return null;
const operands = tokens.slice(pushIdx + 1).filter((t) => !t.startsWith("-"));
// operands[0] is the remote; operands[1] is the refspec. Fewer than two means
// no explicit branch (e.g. `git push` or `git push origin`).
if (operands.length < 2) return null;
const refspec = operands[1]!;
const target = refspec.includes(":") ? (refspec.split(":").pop() ?? null) : refspec;
return target === "HEAD" || target === "@" ? null : target;
}
| export function classifyCommand(command: string): "push" | "commit" | "other" { | ||
| if (/(?:^|[\n;|&])\s*git\s+push\b/.test(command)) return "push"; | ||
| if (/(?:^|[\n;|&])\s*git\s+commit\b/.test(command)) return "commit"; | ||
| return "other"; | ||
| } |
There was a problem hiding this comment.
The current command classification regexes do not account for global git options (such as -c core.quotePath=false or -C /path/to/repo) that can be specified between git and the push/commit subcommands. This allows commands using these options to completely bypass the hook. We should update the regexes to robustly match optional global options.
export function classifyCommand(command: string): "push" | "commit" | "other" {
if (/(?:^|[\n;|&])\s*git\s+(?:-[^\s|&;\n]+(?:\s+[^\s|&;\n]+)?\s+)*push\b/.test(command)) return "push";
if (/(?:^|[\n;|&])\s*git\s+(?:-[^\s|&;\n]+(?:\s+[^\s|&;\n]+)?\s+)*commit\b/.test(command)) return "commit";
return "other";
}
| export function isProtectedBranch(branch: string): boolean { | ||
| return branch === "main" || branch === "master" || /^release\//.test(branch); | ||
| } |
There was a problem hiding this comment.
If a user or agent pushes using a fully qualified ref name (e.g., refs/heads/main), isProtectedBranch will fail to match it because it strictly compares the string with "main". We should strip the refs/heads/ prefix before performing the protection checks.
export function isProtectedBranch(branch: string): boolean {
const clean = branch.replace(/^refs\/heads\//, "");
return clean === "main" || clean === "master" || /^release\//.test(clean);
}
| it("gates an explicit push to a protected branch regardless of current branch", () => { | ||
| assert.equal(evaluatePush("git push origin main", "feature/x").gated, true); | ||
| assert.equal(evaluatePush("git push -u origin main", "feature/x").gated, true); | ||
| assert.equal(evaluatePush("git push origin HEAD:main", "x").gated, true); | ||
| assert.equal(evaluatePush("git push origin release/1.0", "x").gated, true); | ||
| }); |
There was a problem hiding this comment.
Let's add test cases to verify that pushes with global options and fully qualified ref names (e.g., refs/heads/main) are correctly gated.
it("gates an explicit push to a protected branch regardless of current branch", () => {
assert.equal(evaluatePush("git push origin main", "feature/x").gated, true);
assert.equal(evaluatePush("git push -u origin main", "feature/x").gated, true);
assert.equal(evaluatePush("git push origin HEAD:main", "x").gated, true);
assert.equal(evaluatePush("git push origin release/1.0", "x").gated, true);
assert.equal(evaluatePush("git -c core.quotePath=false push origin main", "feature/x").gated, true);
assert.equal(evaluatePush("git push origin refs/heads/main", "feature/x").gated, true);
});
| it("falls back to the current branch when no explicit target is given", () => { | ||
| assert.equal(evaluatePush("git push", "main").gated, true); | ||
| assert.equal(evaluatePush("git push", "feature/x").gated, false); | ||
| }); |
There was a problem hiding this comment.
Let's add test cases to verify that pushing to HEAD or @ correctly falls back to the current branch and is gated when on a protected branch.
it("falls back to the current branch when no explicit target is given", () => {
assert.equal(evaluatePush("git push", "main").gated, true);
assert.equal(evaluatePush("git push", "feature/x").gated, false);
assert.equal(evaluatePush("git push origin HEAD", "main").gated, true);
assert.equal(evaluatePush("git push origin @", "main").gated, true);
});
Two CI-only defects from the prior commit: (1) the bundled hooks/hook-pack.mjs imports ../lib/hook-pack-gate.mjs, but the generator's copyFileTo list omitted it, so the installed plugin's hook would crash at runtime (repo-root tests passed because they exercise the flat copy) — add it to the bundle and pin its presence with a regression test. (2) The new .mjs files were committed from Windows as mode 644; the Linux build chmods hooks/lib/plugins .mjs to 755, so verify:generated's git diff failed — set the exec bit so the index matches. Fixes CI on PR #248.
Bundles #244-#248: /plan-pack, the how-to-best-use guide, /ship, /production-readiness-review, and the enforcing hook-pack. Marketplace consumers get these on merge (the version bump refreshes their plugin cache). npm publish stays gated on the OIDC trusted-publisher config on npmjs.com (v3.13.0 E404'd for exactly that), so the v3.15.0 tag is held pending operator confirmation.
Summary
Adds
hook-pack, a warn-default PreToolUse hook that converts two prose-only repo rules into enforced gates:git pushto a protected branch (main/master/release/*)git commitstaging more than 15 files (the one-concern limit)This is the WILD pilot from the 2026-06-17 report-coverage map — the first piece that turns advised discipline into tool-boundary enforcement (the map's core finding was that this discipline existed only as prose, which is why the source report's failures happened despite the guidance).
Design
hooks/hook-pack.mjs), registered second in PreToolUse with aBashmatcher so it only spawns on Bash tool calls — the non-Bash hot path stays at two subprocesses, and gateguard stays first (existing test pins this).lib/hook-pack-gate.mjs(parseMode,classifyCommand,evaluatePush,evaluateCommitSize,decide); the hook is an I/O shell wiring stdin +gitsubprocess (rev-parse/diff --cached) around it. Split mirrors gateguard (lib/gateguard-state) and goal-drift (lib/goal-drift-gate), and keeps the logic importable by tests (thetest-imports-onlyinvariant forbids importing fromhooks/).CLAUDE_CI_HOOKPACK_GATE = warn (default) | block | off.warnprints a stderr notice and never blocks;blockemits the PreToolUse deny shape (empty stdout = allow, per the schema we already burned on);offis a no-op. Ships warn-default, so nothing changes in any project until the operator opts intoblock.git push/git commitspawn a subprocess.Scope
src/hooks/hook-pack.mts,src/lib/hook-pack-gate.mts, registration insrc/lib/plugin-metadata.mts,src/test/hook-pack.test.mts, + generated mirrors (12 files total: 5 source + 7 generated).=offescape hatch).Verification (TDD)
verify:all12/12 + typecheck clean;verify:generatedclean (deterministic build);gateguard-hooktest 19/19 (gateguard still first).echo git pushfalse-positive guard; push-target parsing incl.HEAD:branch,--dry-run,-u; commit-size incl.--amend; the decide matrix). Integration tests spawn the built hook for block/warn/off/allow + a temp-repo commit-size gate.Test plan
CLAUDE_CI_HOOKPACK_GATE=block,git push origin mainis denied; on a feature branch it's allowed.Part of the 2026-06-17 report-coverage build train (PRs #246
/ship, #247/production-readiness-review). The Stop-hook full-ladder extension is the remaining train item. No new dependencies.