mirror of https://github.com/garrytan/gstack.git
fix(redact-prepush): resolve the real push base instead of EMPTY_TREE whole-repo scans
When the remote default branch is not main/master (or origin/HEAD is unset), the merge-base guess failed and the hook fell back to scanning the ENTIRE repository as added lines — re-attributing long-pushed secrets to the current push and, on any real repo, tripping the engine byte cap so the push blocked having scanned nothing. Derive the base from commits reachable from no remote-tracking branch, keep the empty-tree path only for genuinely fresh repos, and split the block message so an unscannable diff is reported as "could not scan (fail closed)" rather than "credential found — rotate it". Contributed by @stormeoio (PR #2398). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
69cc39c625
commit
d77a2c8e58
|
|
@ -80,21 +80,57 @@ function defaultRemoteBranch(): string {
|
|||
return "origin/main";
|
||||
}
|
||||
|
||||
/**
|
||||
* Base commit for a push whose remote tip we cannot use directly, ordered from
|
||||
* most precise to most conservative. Returns null when nothing can anchor the
|
||||
* range, i.e. the whole history really is new content.
|
||||
*/
|
||||
function unknownRemoteTipBase(localSha: string): string | null {
|
||||
// 1. The common case: a merge-base with the remote's default branch.
|
||||
const base = git(["merge-base", localSha, defaultRemoteBranch()]).trim();
|
||||
if (base) return base;
|
||||
|
||||
// 2. No merge-base. defaultRemoteBranch() guessed a ref that does not exist
|
||||
// (default branch named trunk/develop, origin/HEAD unset), or history is
|
||||
// disjoint. Anything reachable from localSha but from NO remote-tracking
|
||||
// branch is what this push actually adds; the parent of its oldest commit
|
||||
// is the real base.
|
||||
//
|
||||
// Without this we drop straight to EMPTY_TREE and re-scan content that is
|
||||
// already on the remote. That is not merely wasteful, it is wrong in two
|
||||
// ways: a secret pushed long ago gets re-reported as if THIS push
|
||||
// introduced it (telling the operator to rotate a key over someone else's
|
||||
// old commit), and on any real repository the input overshoots the
|
||||
// engine's byte cap, so `engine.input_too_large` blocks having scanned
|
||||
// NOTHING — "scans more, never less" inverted into "scans nothing".
|
||||
//
|
||||
// `--remotes` covers every remote, not just the push target: content
|
||||
// already published anywhere has already left this machine, so treating it
|
||||
// as pre-existing is deliberate. Git hands the remote name to pre-push in
|
||||
// argv, which this hook does not read; narrowing to it would only matter
|
||||
// for a repo that pushes secrets to one remote but not another.
|
||||
const newCommits = git(["rev-list", "--reverse", localSha, "--not", "--remotes"]).trim();
|
||||
if (newCommits) {
|
||||
const oldest = newCommits.split("\n")[0];
|
||||
const parent = git(["rev-parse", "--verify", `${oldest}^`]).trim();
|
||||
if (parent) return parent;
|
||||
// Oldest new commit is a root commit: there is no parent to anchor on.
|
||||
}
|
||||
|
||||
// 3. Nothing to anchor on — a genuinely fresh repository with no remote refs.
|
||||
// Every commit IS new content, so scanning it all is the correct answer.
|
||||
return null;
|
||||
}
|
||||
|
||||
/** Return the added-line text for a ref update being pushed. */
|
||||
function addedLinesFor(localSha: string, remoteSha: string): string {
|
||||
let range: string;
|
||||
if (ZERO.test(remoteSha)) {
|
||||
// New branch: prefer what's unique to localSha vs the remote default branch.
|
||||
// With no merge-base (e.g. no remote yet), diff against the empty tree so ALL
|
||||
// branch content is scanned as added — fail-safe (scans more, never less).
|
||||
const base = git(["merge-base", localSha, defaultRemoteBranch()]).trim();
|
||||
range = base ? `${base}..${localSha}` : `${EMPTY_TREE}..${localSha}`;
|
||||
} else if (!objectExists(remoteSha)) {
|
||||
// Remote tip object absent locally (shallow clone, force-push without a
|
||||
// prior fetch, CI checkout): remote..local can't resolve. Fall back to
|
||||
// the merge-base/empty-tree path — scans MORE, never less — instead of
|
||||
// hard-blocking a legitimate push (adversarial review finding 8).
|
||||
const base = git(["merge-base", localSha, defaultRemoteBranch()]).trim();
|
||||
if (ZERO.test(remoteSha) || !objectExists(remoteSha)) {
|
||||
// Either a new branch (zero remote sha), or the remote tip object is absent
|
||||
// locally (shallow clone, force-push without a prior fetch, CI checkout) so
|
||||
// remote..local cannot resolve. Both need a base derived locally; scan MORE
|
||||
// rather than hard-blocking a legitimate push (adversarial review finding 8).
|
||||
const base = unknownRemoteTipBase(localSha);
|
||||
range = base ? `${base}..${localSha}` : `${EMPTY_TREE}..${localSha}`;
|
||||
} else {
|
||||
// Existing branch (incl. force-push): net new content remote..local.
|
||||
|
|
@ -239,15 +275,50 @@ function main() {
|
|||
}
|
||||
|
||||
if (allHigh.length > 0) {
|
||||
process.stderr.write(
|
||||
"\n⛔ gstack-redact-prepush BLOCKED the push — credential(s) in the pushed diff:\n\n",
|
||||
);
|
||||
for (const f of allHigh) {
|
||||
process.stderr.write(` HIGH ${f.id} ${f.preview}\n`);
|
||||
// A scan that could not RUN is not a scan that FOUND something. Reporting
|
||||
// "credential(s) in the pushed diff — rotate the credential" for an
|
||||
// `engine.*` finding tells the operator to rotate a secret that was never
|
||||
// detected, on a diff that was never read. Blocking is still right (fail
|
||||
// closed), but the reason must be the true one: a guardrail that cries wolf
|
||||
// is a guardrail that gets bypassed by reflex, which is worse than none.
|
||||
// Seen live 2026-07-30: a diff of a few hundred bytes reported HIGH
|
||||
// engine.input_too_large, because an unresolvable base branch made the hook
|
||||
// fall back to EMPTY_TREE..local — i.e. the WHOLE repo (~7 MiB) as "added
|
||||
// lines". The size the operator sees and the size the hook measures can
|
||||
// therefore differ by four orders of magnitude.
|
||||
const unscanned = allHigh.filter((f) => f.id.startsWith("engine."));
|
||||
const secrets = allHigh.filter((f) => !f.id.startsWith("engine."));
|
||||
|
||||
if (secrets.length > 0) {
|
||||
process.stderr.write(
|
||||
"\n⛔ gstack-redact-prepush BLOCKED the push — credential(s) in the pushed diff:\n\n",
|
||||
);
|
||||
for (const f of secrets) {
|
||||
process.stderr.write(` HIGH ${f.id} ${f.preview}\n`);
|
||||
}
|
||||
process.stderr.write(
|
||||
"\nRotate the credential (a pushed secret is compromised) and remove it from the diff.\n",
|
||||
);
|
||||
}
|
||||
|
||||
if (unscanned.length > 0) {
|
||||
process.stderr.write(
|
||||
"\n⛔ gstack-redact-prepush BLOCKED the push — the diff could NOT be scanned.\n" +
|
||||
" No credential was found; none was looked for. Blocking fail-closed.\n\n",
|
||||
);
|
||||
for (const f of unscanned) {
|
||||
process.stderr.write(` ${f.id}: ${f.description}\n`);
|
||||
}
|
||||
process.stderr.write(
|
||||
"\nLikely cause: the base branch could not be resolved, so the whole repo was\n" +
|
||||
"treated as added lines. Check `git rev-parse --abbrev-ref origin/HEAD` and\n" +
|
||||
"`git merge-base HEAD origin/main`, then push again. Scan the diff yourself\n" +
|
||||
"before bypassing: `git diff <base>..HEAD | grep -inE \'password|secret|token|api.?key\'`.\n",
|
||||
);
|
||||
}
|
||||
|
||||
process.stderr.write(
|
||||
"\nRotate the credential (a pushed secret is compromised) and remove it from the diff.\n" +
|
||||
"This is a guardrail: `git push --no-verify` or `GSTACK_REDACT_PREPUSH=skip git push` bypass it.\n",
|
||||
"This is a guardrail: `git push --no-verify` or `GSTACK_REDACT_PREPUSH=skip git push` bypass it.\n",
|
||||
);
|
||||
process.exit(1);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -229,3 +229,50 @@ describe("install / chaining", () => {
|
|||
expect(restored).not.toContain("managed");
|
||||
});
|
||||
});
|
||||
|
||||
describe("base resolution when the default branch is neither main nor master", () => {
|
||||
test("a new branch scans its own commits, not the whole repository", () => {
|
||||
// The remote's default branch is `trunk` and origin/HEAD is unset, so
|
||||
// defaultRemoteBranch() falls through to `origin/main` — a ref that does
|
||||
// not exist — and merge-base fails. The EMPTY_TREE fallback then treats the
|
||||
// WHOLE repository as added lines, re-scanning history that is already on
|
||||
// the remote. Two consequences, both bad: a secret long since pushed gets
|
||||
// re-reported as if this push introduced it, and on any real repository the
|
||||
// input blows past the engine's byte cap, so `engine.input_too_large`
|
||||
// blocks the push having scanned NOTHING — the "scans more, never less"
|
||||
// fallback inverting into "scans nothing".
|
||||
const bare = fs.mkdtempSync(path.join(os.tmpdir(), "prepush-remote-"));
|
||||
spawnSync("git", ["init", "-q", "--bare", "-b", "trunk", bare]);
|
||||
|
||||
git(["branch", "-M", "trunk"]);
|
||||
const old = commit("legacy.txt", "AKIA1234567890ABCDEF\n", "secret already on the remote");
|
||||
git(["remote", "add", "origin", bare]);
|
||||
git(["push", "-q", "origin", "trunk"]);
|
||||
|
||||
// The remote HAS the old commit, and the default-branch guess is unresolvable.
|
||||
expect(git(["rev-parse", "origin/trunk"])).toBe(old);
|
||||
expect(git(["rev-parse", "--verify", "origin/main"])).toBe("");
|
||||
expect(git(["symbolic-ref", "refs/remotes/origin/HEAD"])).toBe("");
|
||||
|
||||
git(["checkout", "-q", "-b", "feat"]);
|
||||
const head = commit("feature.txt", "totally clean\n", "clean feature commit");
|
||||
|
||||
const { code, stderr } = runHook(`refs/heads/feat ${head} refs/heads/feat ${ZERO}\n`);
|
||||
fs.rmSync(bare, { recursive: true, force: true });
|
||||
|
||||
// The only NEW content is a clean file. The already-pushed secret must not
|
||||
// be attributed to this push.
|
||||
expect(stderr).not.toContain("aws.access_key");
|
||||
expect(code).toBe(0);
|
||||
});
|
||||
|
||||
test("a genuinely new repository with no remote refs still scans everything", () => {
|
||||
// Nothing is on any remote, so every commit IS new content: scanning the
|
||||
// full history is correct here. The narrowing must not open a hole in the
|
||||
// case the EMPTY_TREE fallback exists for.
|
||||
const head = commit("secrets.txt", "AKIA1234567890ABCDEF\n", "secret in a fresh repo");
|
||||
const { code, stderr } = runHook(`refs/heads/feat ${head} refs/heads/feat ${ZERO}\n`);
|
||||
expect(code).toBe(1);
|
||||
expect(stderr).toContain("aws.access_key");
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue