Commit74202f0
fix(push-policy): pre-receive secret scan only inspects pushed changes, not whole tree
fix(push-policy): pre-receive secret scan only inspects pushed changes, not whole tree For a NEW branch (oldSha = zero-SHA) the pre-receive hook set LOG_RANGE="$NEW" and diffed against the empty-tree SHA, so `git diff <empty-tree> $NEW` reported EVERY file in the repo as changed. The secret scan then ran over the entire tree — so pushing a clean feature branch was rejected because unrelated, pre-existing committed fixtures (crafted to look like real secrets, to test the scanner itself) matched a pattern. This broke the documented "push a feature branch → open a PR" workflow for every new branch. Fix: on a new ref, inspect only commits/paths not already reachable from an existing ref (`git log ... "$NEW" --not --all`). The existing-branch path ($OLD..$NEW + git diff $OLD $NEW) is unchanged, so there is zero behavior change for normal updates. On a genuinely empty repo (no existing refs) --not --all excludes nothing, so the first push is still fully scanned — correct. Adds an integration test that drives the real generated hook via an actual `git push` into a bare repo: a clean new branch is accepted (would fail before this fix), while a branch that itself introduces a real secret is still blocked and does not falsely flag untouched files. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2 files changed+118−1074202f09fc3612858f981a0f83c573621d613bae
2 changed files+118−10
Addedsrc/__tests__/push-policy-newbranch-scan.test.ts+103−0View fileUnifiedSplit
@@ -0,0 +1,103 @@
1/**
2 * Regression: the pre-receive secret scan must inspect ONLY what a push
3 * actually introduces — not the entire repo tree.
4 *
5 * The bug: for a NEW branch (oldSha = zero-SHA), buildPreReceiveScript() used
6 * LOG_RANGE="$NEW" and a `git diff <empty-tree> $NEW`, which reports every
7 * file in the whole repo as "changed". So pushing a clean feature branch was
8 * rejected because a PRE-EXISTING committed test fixture (crafted to look like
9 * a real secret, to test the scanner itself) matched a pattern — even though
10 * the branch never touched it. This broke the documented
11 * "push a feature branch → open a PR" workflow for every new branch.
12 *
13 * These tests drive the ACTUAL generated hook via a real `git push` into a
14 * bare repo (same as production), rather than re-implementing the range logic.
15 */
16import { describe, expect, it, beforeAll } from "bun:test";
17import { mkdtempSync, writeFileSync } from "fs";
18import { tmpdir } from "os";
19import { join } from "path";
20import { installPackInspectionHook } from "../lib/push-policy";
21
22// A fake-but-realistic AWS key that is NOT caught by the placeholder filter —
23// the kind of value a scanner-test fixture legitimately commits.
24const FIXTURE_SECRET = "AKIAABCDEFGH1234IJKL";
25
26function git(cwd: string, args: string[], allowFail = false) {
27 const proc = Bun.spawnSync(["git", ...args], {
28 cwd,
29 env: { ...process.env, GIT_TERMINAL_PROMPT: "0" },
30 });
31 const exitCode = proc.exitCode;
32 const stderr = new TextDecoder().decode(proc.stderr);
33 if (!allowFail && exitCode !== 0) {
34 throw new Error(`git ${args.join(" ")} failed (${exitCode}):\n${stderr}`);
35 }
36 return { exitCode, stderr, stdout: new TextDecoder().decode(proc.stdout) };
37}
38
39const GIT_OK = (() => {
40 try {
41 return Bun.spawnSync(["git", "--version"]).exitCode === 0;
42 } catch {
43 return false;
44 }
45})();
46
47describe.skipIf(!GIT_OK)("pre-receive scan only inspects pushed changes", () => {
48 let bare: string;
49 let work: string;
50
51 beforeAll(async () => {
52 const root = mkdtempSync(join(tmpdir(), "gc-newbranch-"));
53 bare = join(root, "bare.git");
54 work = join(root, "work");
55
56 git(root, ["init", "--bare", "-b", "main", bare]);
57 git(root, ["init", "-b", "main", work]);
58 git(work, ["config", "user.email", "t@example.com"]);
59 git(work, ["config", "user.name", "Test"]);
60 git(work, ["config", "commit.gpgsign", "false"]);
61
62 // Seed main with a pre-existing fixture that CONTAINS a secret pattern,
63 // pushed BEFORE the hook is installed (so it lands, exactly like the real
64 // fixtures that predate the scanner).
65 writeFileSync(join(work, "fixture.ts"), `const K = '${FIXTURE_SECRET}';\n`);
66 git(work, ["add", "."]);
67 git(work, ["commit", "-m", "seed fixture (has a secret pattern)"]);
68 git(work, ["remote", "add", "origin", bare]);
69 git(work, ["push", "-u", "origin", "main"]);
70
71 // Now arm the pre-receive hook on the bare repo.
72 const hook = await installPackInspectionHook([], { secretScan: true });
73 if (!hook) throw new Error("hook install returned null");
74 git(bare, ["config", "core.hooksPath", hook.env.GIT_CONFIG_VALUE_0]);
75 });
76
77 it("accepts a new branch that only adds a clean file (does NOT rescan the pre-existing fixture)", () => {
78 git(work, ["checkout", "-b", "clean-feature", "main"]);
79 writeFileSync(join(work, "clean.ts"), "export const ok = 1;\n");
80 git(work, ["add", "clean.ts"]);
81 git(work, ["commit", "-m", "add a clean file"]);
82
83 const res = git(work, ["push", "origin", "clean-feature"], true);
84 // Before the fix this failed with "Secret scan: possible AWS Access Key
85 // in fixture.ts" — a file the branch never touched.
86 expect(res.stderr).not.toContain("Secret scan");
87 expect(res.exitCode).toBe(0);
88 });
89
90 it("still blocks a new branch that itself introduces a real secret", () => {
91 git(work, ["checkout", "-b", "leaky-feature", "main"]);
92 writeFileSync(join(work, "leak.ts"), `const K = '${FIXTURE_SECRET}';\n`);
93 git(work, ["add", "leak.ts"]);
94 git(work, ["commit", "-m", "oops committed a secret"]);
95
96 const res = git(work, ["push", "origin", "leaky-feature"], true);
97 expect(res.exitCode).not.toBe(0);
98 expect(res.stderr).toContain("Secret scan");
99 expect(res.stderr).toContain("leak.ts");
100 // And it must NOT also (falsely) flag the untouched fixture.
101 expect(res.stderr).not.toContain("fixture.ts");
102 });
103});
Modifiedsrc/lib/push-policy.ts+15−10View fileUnifiedSplit
@@ -223,21 +223,26 @@ function buildPreReceiveScript(evalScriptPath: string, rulesJsonPath: string): s
223223 "",
224224 `while IFS=' ' read -r OLD NEW REF; do`,
225225 ` [[ "${D}NEW" =~ ^0+${D} ]] && continue`,
226 ` if [[ "${D}OLD" =~ ^0+${D} ]]; then`,
227 ` LOG_RANGE="${D}NEW"`,
228 // Empty-tree SHA — safe diff base for new branches with no parent.
229 ` DIFF_BASE=4b825dc642cb6eb9a060e54bf8d69288fbee4904`,
230 ` else`,
231 ` LOG_RANGE="${D}OLD..${D}NEW"`,
232 ` DIFF_BASE="${D}OLD"`,
233 ` fi`,
234226 "",
235227 ` COMMITS_TMP=$(mktemp)`,
236228 ` SIZES_TMP=$(mktemp)`,
237229 ` PATHS_TMP=$(mktemp)`,
238230 ` CONTENTS_TMP=$(mktemp)`,
239 ` git log --format="%H %s" "${D}LOG_RANGE" 2>/dev/null > "${D}COMMITS_TMP" || true`,
240 ` git diff --name-only "${D}DIFF_BASE" "${D}NEW" 2>/dev/null > "${D}PATHS_TMP" || true`,
231 ` if [[ "${D}OLD" =~ ^0+${D} ]]; then`,
232 // New branch/ref: inspect ONLY the commits and paths that are not already
233 // reachable from an existing ref (--not --all). The previous empty-tree
234 // diff base ("git diff <empty-tree> NEW") reported EVERY file in the whole
235 // repo as changed, so pushing any new branch re-scanned pre-existing files
236 // the push never touched — a clean feature branch was rejected because an
237 // unrelated committed fixture matched a secret pattern. On a genuinely
238 // empty repo (no existing refs) --not --all excludes nothing, so the very
239 // first push is still fully scanned, which is correct.
240 ` git log --format="%H %s" "${D}NEW" --not --all 2>/dev/null > "${D}COMMITS_TMP" || true`,
241 ` git log --name-only --format="" "${D}NEW" --not --all 2>/dev/null | sort -u > "${D}PATHS_TMP" || true`,
242 ` else`,
243 ` git log --format="%H %s" "${D}OLD..${D}NEW" 2>/dev/null > "${D}COMMITS_TMP" || true`,
244 ` git diff --name-only "${D}OLD" "${D}NEW" 2>/dev/null > "${D}PATHS_TMP" || true`,
245 ` fi`,
241246 "",
242247 // Build sizes file: "<bytes> <path>" per changed file. Also dump
243248 // base64 content (capped at 256KB/file) for the secret scan below —
244249