mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-10 15:09:00 +02:00
fix(redact): env.kv stops flagging cacheKey-style names; prepush exclusion scoped to the push remote
Two calibration/coverage fixes in the redaction guard: - env.kv's zero-or-more-prefix regex fired on ANY identifier ending in a credential suffix, so ordinary code (cacheKey:, sortKey:, partitionKey:, hotkey:, even monkey:) with an 8+-char entropic value hit a MEDIUM confirm prompt — a gate that cries wolf gets ignored. A name now only counts when its shape is credential-semantic: suffix separated by _/-/. (api_key, x-access-key, AUTH.TOKEN), a bare suffix (key:, token:), ALL-CAPS env style (APIKEY=, MY_APIKEY=), or a camel compound with a credential prefix (apiKey, authToken, clientSecret). The value stays capture group 1, so the shape check lives in validate (isCredentialShapedEnvName), not the regex. - gstack-redact-prepush's narrowing excluded commits reachable from ANY remote (`--not --remotes`), so a secret that had only ever reached a private/local-path remote was never scanned when later pushed to a PUBLIC remote. The exclusion is now scoped to the push target (`--remotes=<name>/*`) via the remote name git hands pre-push as $1 (the installed wrapper already forwards "$@"); stdin/CLI invocations and URL pushes without a configured name fall back to the historical all-remotes behavior. #2592's catch-up-merge fix is unaffected: upstream commits come from the same remote being pushed to. New coverage: env.kv negative controls (cacheKey/sortKey/partitionKey/ hotkey/monkey/idempotencyKey) + positive controls for all four name shapes; end-to-end hook tests proving a second-remote secret blocks a push to origin while origin-published catch-up content still doesn't, plus both fallbacks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
4a95ce61a0
commit
412ad5c1f9
@@ -183,6 +183,29 @@ describe("MEDIUM demoted credential-shaped patterns (TENSION-1)", () => {
|
||||
expect(ids("apiKey: YOUR_API_KEY_HERE")).not.toContain("env.kv");
|
||||
expect(ids("api_key=${MY_VAR}")).not.toContain("env.kv");
|
||||
});
|
||||
// T1 calibration: the zero-or-more-prefix net matched ANY identifier ending
|
||||
// in a suffix, so ordinary code (`cacheKey: <entropic id>`) hit a MEDIUM
|
||||
// confirm prompt. Name shape must be credential-semantic to count.
|
||||
test("env.kv ignores non-credential names ending in a suffix (entropic values)", () => {
|
||||
const v = "8Fk2pQ9vXz4wL7mN3rT6yB1cD5eG0hJ";
|
||||
expect(ids(`cacheKey: ${v}`)).not.toContain("env.kv");
|
||||
expect(ids(`sortKey: ${v}`)).not.toContain("env.kv");
|
||||
expect(ids(`partitionKey: ${v}`)).not.toContain("env.kv");
|
||||
expect(ids(`hotkey: ${v}`)).not.toContain("env.kv");
|
||||
expect(ids(`monkey: ${v}`)).not.toContain("env.kv");
|
||||
expect(ids(`idempotencyKey: ${v}`)).not.toContain("env.kv");
|
||||
});
|
||||
test("env.kv still fires on every credential-shaped name form", () => {
|
||||
const v = "8Fk2pQ9vXz4wL7mN3rT6yB1cD5eG0hJ";
|
||||
expect(ids(`api_key=${v}`)).toContain("env.kv"); // (i) separator
|
||||
expect(ids(`API_KEY=${v}`)).toContain("env.kv"); // (i) + ALL-CAPS
|
||||
expect(ids(`x-access-key: ${v}`)).toContain("env.kv"); // (i) dash separator
|
||||
expect(ids(`key: ${v}`)).toContain("env.kv"); // (ii) bare suffix
|
||||
expect(ids(`APIKEY=${v}`)).toContain("env.kv"); // (iii) ALL-CAPS compound
|
||||
expect(ids(`apiKey: ${v}`)).toContain("env.kv"); // (iv) credential camel
|
||||
expect(ids(`authToken: ${v}`)).toContain("env.kv"); // (iv) credential camel
|
||||
expect(ids(`clientSecret: ${v}`)).toContain("env.kv"); // (iv) credential camel
|
||||
});
|
||||
test("env.kv stays MEDIUM (calibration: generic net, not a blocker)", () => {
|
||||
const f = scan("api_key=8Fk2pQ9vXz4wL7mN3rT6yB1cD5eG0hJ", { repoVisibility: "private" })
|
||||
.findings.find((x) => x.id === "env.kv");
|
||||
|
||||
@@ -159,3 +159,118 @@ describe("narrowing the range does not narrow coverage", () => {
|
||||
expect(addedOnly(addedLinesFromNewCommits(dir))).toContain(FAKE_AWS_NOREMOT);
|
||||
});
|
||||
});
|
||||
|
||||
// ── S1: the exclusion is scoped to the PUSH TARGET's remote ─────────────────
|
||||
//
|
||||
// A bare `--remotes` excludes commits reachable from ANY remote-tracking ref,
|
||||
// so a secret that had only ever reached a private/local-path remote was never
|
||||
// scanned when pushed to a PUBLIC remote. Git hands pre-push the push remote's
|
||||
// name as $1; the hook now scopes the exclusion to `--remotes=<name>/*`.
|
||||
// These run END-TO-END through the hook binary with the real argv + stdin
|
||||
// protocol, because the behavior under test is the argv threading itself.
|
||||
describe("S1: exclusion scoped to the push-target remote", () => {
|
||||
const PREPUSH = join(import.meta.dir, "..", "bin", "gstack-redact-prepush");
|
||||
const FAKE_AWS_OTHERREM = ["AKIA", "IOSFODNN7OTHERRM"].join("");
|
||||
|
||||
function runHook(stdinLines: string, argv: string[]): { code: number; stderr: string } {
|
||||
const r = spawnSync("bun", [PREPUSH, ...argv], {
|
||||
cwd: dir,
|
||||
input: Buffer.from(stdinLines),
|
||||
encoding: "utf8",
|
||||
env: { ...process.env },
|
||||
});
|
||||
return { code: r.status ?? 0, stderr: r.stderr ?? "" };
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the S1 shape. Returns the feature branch's last-pushed origin tip
|
||||
* (what git hands the hook as remoteSha):
|
||||
* 1. main + feature pushed to origin (T0 = feature's origin tip)
|
||||
* 2. a HIGH-shaped secret commit reaches a SECOND remote only
|
||||
* (pushed there, fetched back → other/leaky tracking ref)
|
||||
* 3. feature merges the secret commit — the next push to origin is
|
||||
* the first time this content heads anywhere public
|
||||
*/
|
||||
function buildSecretOnSecondRemote(): { originTip: string } {
|
||||
const origin = mkdtempSync(join(tmpdir(), "gstack-prepush-origin-"));
|
||||
run(["init", "-q", "--bare", "-b", "main"], origin);
|
||||
run(["remote", "add", "origin", origin]);
|
||||
run(["push", "-q", "origin", "main"]);
|
||||
run(["checkout", "-q", "-b", "feature"]);
|
||||
commit("mine.ts", "export const mine = 1;\n", "my work");
|
||||
run(["push", "-q", "-u", "origin", "feature"]);
|
||||
const originTip = run(["rev-parse", "HEAD"]).trim();
|
||||
|
||||
const other = mkdtempSync(join(tmpdir(), "gstack-prepush-other-"));
|
||||
run(["init", "-q", "--bare", "-b", "main"], other);
|
||||
run(["remote", "add", "other", other]);
|
||||
run(["checkout", "-q", "-b", "leaky"]);
|
||||
commit("leak.ts", `const k = "${FAKE_AWS_OTHERREM}";\n`, "secret to private remote only");
|
||||
run(["push", "-q", "other", "leaky"]);
|
||||
run(["fetch", "-q", "other"]);
|
||||
|
||||
run(["checkout", "-q", "feature"]);
|
||||
// --no-ff: a fast-forward would make the secret commit the branch TIP,
|
||||
// where the remoteSha two-dot fallback catches it regardless of the
|
||||
// --remotes exclusion. The hole shape needs a real merge commit, so the
|
||||
// narrowed path (per-commit --cc diffs) is what decides coverage.
|
||||
run(["merge", "-q", "--no-ff", "--no-edit", "leaky"]);
|
||||
return { originTip };
|
||||
}
|
||||
|
||||
test("a commit known only to a SECOND remote IS scanned when pushing to origin", () => {
|
||||
const { originTip } = buildSecretOnSecondRemote();
|
||||
const head = run(["rev-parse", "HEAD"]).trim();
|
||||
const { code, stderr } = runHook(
|
||||
`refs/heads/feature ${head} refs/heads/feature ${originTip}\n`,
|
||||
["origin", "file:///ignored"],
|
||||
);
|
||||
expect(code).toBe(1);
|
||||
expect(stderr).toContain("BLOCKED");
|
||||
expect(stderr).toContain("aws.access_key");
|
||||
});
|
||||
|
||||
test("origin-published commits still are NOT re-scanned (catch-up merge, #2592 kept)", () => {
|
||||
setUpRemoteWithForeignFixture();
|
||||
run(["checkout", "-q", "-b", "feature", "HEAD~1"]);
|
||||
commit("mine.ts", "export const mine = 1;\n", "my work");
|
||||
run(["push", "-q", "-u", "origin", "feature"]);
|
||||
const originTip = run(["rev-parse", "HEAD"]).trim();
|
||||
run(["merge", "-q", "--no-edit", "main"]); // catch-up merge brings the foreign fixture
|
||||
|
||||
const head = run(["rev-parse", "HEAD"]).trim();
|
||||
const { code, stderr } = runHook(
|
||||
`refs/heads/feature ${head} refs/heads/feature ${originTip}\n`,
|
||||
["origin", "file:///ignored"],
|
||||
);
|
||||
expect(stderr).not.toContain("BLOCKED");
|
||||
expect(code).toBe(0);
|
||||
});
|
||||
|
||||
test("no argv (stdin/CLI invocation) falls back to the historical all-remotes exclusion", () => {
|
||||
// Documented contract, not a gap being celebrated: without the remote
|
||||
// name there is nothing to scope to, and the fallback scans exactly what
|
||||
// the hook always scanned. The installed hook wrapper forwards "$@", so
|
||||
// real pushes always carry the name.
|
||||
const { originTip } = buildSecretOnSecondRemote();
|
||||
const head = run(["rev-parse", "HEAD"]).trim();
|
||||
const { code, stderr } = runHook(
|
||||
`refs/heads/feature ${head} refs/heads/feature ${originTip}\n`,
|
||||
[],
|
||||
);
|
||||
expect(stderr).not.toContain("BLOCKED");
|
||||
expect(code).toBe(0);
|
||||
});
|
||||
|
||||
test("an unconfigured name (URL push) also falls back rather than erroring", () => {
|
||||
const { originTip } = buildSecretOnSecondRemote();
|
||||
const head = run(["rev-parse", "HEAD"]).trim();
|
||||
const url = "file:///not-a-configured-remote";
|
||||
const { code, stderr } = runHook(
|
||||
`refs/heads/feature ${head} refs/heads/feature ${originTip}\n`,
|
||||
[url, url],
|
||||
);
|
||||
expect(stderr).not.toContain("could not");
|
||||
expect(code).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user