diff --git a/bin/gstack-redact-prepush b/bin/gstack-redact-prepush index 6d3f2a5c9..695516bd2 100755 --- a/bin/gstack-redact-prepush +++ b/bin/gstack-redact-prepush @@ -21,6 +21,8 @@ * - HIGH finding in added lines → print + exit 1 (block), for public AND private. * - MEDIUM → warn (non-blocking), one line per finding naming its rule, * file line and fix, never the matched value. LOW/WARN → silent. + * - `pii.email` skips the pusher's own address and addresses already in + * the destination's commit metadata (see emailScanOptions). * - GSTACK_REDACT_PREPUSH=skip → log + exit 0 (escape valve). * * Installed/uninstalled via `gstack-redact install-prepush-hook` (see the @@ -29,7 +31,7 @@ import { spawnSync } from "child_process"; import * as fs from "fs"; import * as path from "path"; -import { normalizeWithMap, scan, type Finding } from "../lib/redact-engine"; +import { normalizeWithMap, scan, type Finding, type ScanOptions } from "../lib/redact-engine"; import { mkdirpSync } from "../lib/fs-utils"; import { resolveStateRoot } from "../lib/state-root"; @@ -500,6 +502,50 @@ function logSkip(reason: string): void { } } +const HISTORY_MAX_COMMITS = 50_000; +const HISTORY_TIMEOUT_MS = 5_000; + +function configValues(args: string[]): string[] { + return git(["config", ...args]).split("\n").map((v) => v.trim()).filter(Boolean); +} + +/** + * Addresses `pii.email` treats as not a new leak (#3060): the pusher's own + * (`git config user.email`) and the author and committer addresses (raw and + * mailmapped) the destination will hold after this push. That last set is the history of every pushed tip + * (pushed commits become public with the push, third-party authors included), + * of each remote tip being replaced, and, for a ref whose remote tip is new or + * unknown, the push remote's own tracking refs. A URL or unknown destination + * never borrows a remote's tracking refs; another remote's history never counts. + * + * Each input fails on its own and a failure omits only that input, which is + * the behavior before these exemptions existed. The history read is bounded: + * past the commit cap it uses the newest authors it read; an error or the + * timeout passes no history, and the reason is returned so the report can say + * suppression was limited. + */ +function emailScanOptions(historyRevs: string[], trackingRemote: string | null): { + options: Pick; + limited?: string; +} { + const selfEmail = configValues(["--get", "user.email"])[0]; + const r = spawnSync("git", [ + "log", "--no-show-signature", "--no-color", `--max-count=${HISTORY_MAX_COMMITS}`, "--format=%ae%n%aE%n%ce%n%cE", + ...historyRevs, ...(trackingRemote ? [`--remotes=${trackingRemote}/*`] : []), "--", + ], { encoding: "utf8", maxBuffer: 64 * 1024 * 1024, timeout: HISTORY_TIMEOUT_MS, killSignal: "SIGKILL" }); + const timedOut = (r.error as NodeJS.ErrnoException | undefined)?.code === "ETIMEDOUT"; + const history = r.status === 0 && !r.error + ? (r.stdout ?? "").split("\n").map((e) => e.trim()).filter((e) => e.includes("@")) + : null; + return { + options: { + ...(selfEmail ? { selfEmail } : {}), + ...(history ? { repoPublicEmails: [...new Set(history)] } : {}), + }, + ...(history ? {} : { limited: timedOut ? `reading commit history took over ${HISTORY_TIMEOUT_MS / 1000} s` : "reading commit history failed" }), + }; +} + /** One MEDIUM report line: rule id, file line (and commit), and the fix. Never the matched value. */ function mediumLine(f: Finding, part: AddedFile): string { const line = part.lines[f.line - 1]; @@ -523,6 +569,8 @@ function main() { .map((l) => l.split(/\s+/)); const parts: AddedFile[] = []; + const historyRevs = new Set(); + let trackingRemote: string | null = null; for (const fields of refs) { // Fail CLOSED on a ref line we cannot parse (#2498): git hands pre-push @@ -557,20 +605,31 @@ function main() { process.exit(1); } parts.push(...added.filter((part) => part.text.trim())); + historyRevs.add(localSha); + if (!ZERO.test(remoteSha) && objectExists(remoteSha)) historyRevs.add(remoteSha); + else { + const target = pushTarget(); + if (target.kind === "remote") trackingRemote = target.name; + } } const allHigh: Finding[] = []; const mediums: string[] = []; + let emailMedium = false; + const emails = parts.length ? emailScanOptions([...historyRevs], trackingRemote) : { options: {} }; // Visibility doesn't change HIGH behavior; pass private so nothing is treated // as public-strict (HIGH blocks regardless either way). // Sliced (see scanAddedLines) so a large-but-legitimate diff is actually // scanned rather than blocked unscanned on the engine's size cap. // Scanned per file so a pattern can use the file's type (#3048). for (const part of parts) { - const opts = { repoVisibility: "private" as const, ...(part.file ? { sourcePath: part.file } : {}) }; + const opts = { repoVisibility: "private" as const, ...emails.options, ...(part.file ? { sourcePath: part.file } : {}) }; for (const f of scanAddedLines(part.text, opts)) { if (f.severity === "HIGH") allHigh.push(f); - else if (f.severity === "MEDIUM") mediums.push(mediumLine(f, part)); + else if (f.severity === "MEDIUM") { + mediums.push(mediumLine(f, part)); + if (f.id === "pii.email") emailMedium = true; + } } } @@ -580,6 +639,12 @@ function main() { "Not blocking. Review before this becomes public.\n", ); for (const line of mediums) process.stderr.write(line); + if (emailMedium && "limited" in emails && emails.limited) { + process.stderr.write( + `gstack-redact-prepush: existing-email suppression was limited for this push (${emails.limited}), ` + + "so an address already in the history may be listed above.\n", + ); + } } if (allHigh.length > 0) { diff --git a/test/redact-prepush-emails.test.ts b/test/redact-prepush-emails.test.ts index 519d6cb4d..9f851925c 100644 --- a/test/redact-prepush-emails.test.ts +++ b/test/redact-prepush-emails.test.ts @@ -116,7 +116,43 @@ function publishForeignBranch(fx: Fixture, remote: string, branch: string, email git(fx, producer, ["push", "-q", "origin", `HEAD:refs/heads/${branch}`]); } -describe("MEDIUM findings are listed one per line", () => { +describe("own and already-public addresses are not reported (#3060)", () => { + test("the pusher's own address (git config user.email) is not reported", () => { + const fx = fixture({ selfEmail: "me@corp.io" }); + commitFile(fx, fx.repo, "AUTHORS.md", "Maintainer: me@corp.io\n", "colleague@corp.io"); + const { code, stderr } = push(fx, ["origin", "main"]); + expect(code).toBe(0); + expect(stderr).not.toContain("MEDIUM"); + }); + + test("an address that authored a commit the remote already has is not reported", () => { + const fx = fixture(); + commitFile(fx, fx.repo, "lib.txt", "code\n", "colleague@corp.io"); + expect(push(fx, ["origin", "main"]).code).toBe(0); + commitFile(fx, fx.repo, "CODEOWNERS", "* colleague@corp.io\n"); + const { code, stderr } = push(fx, ["origin", "main"]); + expect(code).toBe(0); + expect(stderr).not.toContain("MEDIUM"); + }); + + test("an address that authors one of the pushed commits is not reported", () => { + const fx = fixture(); + commitFile(fx, fx.repo, "package.json", '{ "author": "newcomer@corp.io" }\n', "newcomer@corp.io"); + const { code, stderr } = push(fx, ["origin", "main"]); + expect(code).toBe(0); + expect(stderr).not.toContain("MEDIUM"); + }); + + test("mailmapped author addresses count as already public", () => { + const fx = fixture(); + commitFile(fx, fx.repo, "lib.txt", "code\n", "old-name@corp.io"); + expect(push(fx, ["origin", "main"]).code).toBe(0); + fs.writeFileSync(path.join(fx.repo, ".mailmap"), "Someone \n"); + commitFile(fx, fx.repo, "docs/contact.md", "Ask current@corp.io\n"); + const { stderr } = push(fx, ["origin", "main"]); + expect(stderr).not.toContain("MEDIUM"); + }); + test("control: a stranger's address is reported with rule, file, line and fix, never the address", () => { const fx = fixture(); commitFile(fx, fx.repo, "notes.md", "line one\nline two\ncontact stranger@corp.io\n"); @@ -141,6 +177,131 @@ describe("MEDIUM findings are listed one per line", () => { }); }); +describe("history scope follows the push destination (CEO-7, ENG-8)", () => { + test("a new branch to a configured remote counts authors on that remote's tracking refs", () => { + const fx = fixture(); + publishForeignBranch(fx, fx.origin, "other", "colleague@corp.io"); + git(fx, fx.repo, ["fetch", "-q", "origin"]); + git(fx, fx.repo, ["checkout", "-q", "-b", "feature"]); + commitFile(fx, fx.repo, "CODEOWNERS", "* colleague@corp.io\n"); + const { code, stderr } = push(fx, ["origin", "feature"]); + expect(code).toBe(0); + expect(stderr).not.toContain("MEDIUM"); + }); + + test("control: a URL push does not count the tracking refs of a remote that happens to match", () => { + const fx = fixture(); + publishForeignBranch(fx, fx.origin, "other", "colleague@corp.io"); + git(fx, fx.repo, ["fetch", "-q", "origin"]); + git(fx, fx.repo, ["checkout", "-q", "-b", "feature"]); + commitFile(fx, fx.repo, "CODEOWNERS", "* colleague@corp.io\n"); + const { code, stderr } = push(fx, [fx.origin, "feature"]); + expect(code).toBe(0); + expect(mediumLines(stderr)).toHaveLength(1); + }); + + test("control: an address only another remote's history holds is still reported", () => { + const fx = fixture(); + const upstream = path.join(fx.root, "upstream.git"); + git(fx, fx.root, ["init", "--bare", "-q", "-b", "main", upstream]); + git(fx, fx.repo, ["push", "-q", upstream, "main"]); + publishForeignBranch(fx, upstream, "other", "private-colleague@corp.io"); + git(fx, fx.repo, ["remote", "add", "upstream", upstream]); + git(fx, fx.repo, ["fetch", "-q", "upstream"]); + git(fx, fx.repo, ["checkout", "-q", "-b", "feature"]); + commitFile(fx, fx.repo, "CODEOWNERS", "* private-colleague@corp.io\n"); + const { code, stderr } = push(fx, ["origin", "feature"]); + expect(code).toBe(0); + expect(mediumLines(stderr)).toHaveLength(1); + }); + + test("a force-push counts authors in the history of the remote tip it replaces", () => { + const fx = fixture(); + commitFile(fx, fx.repo, "draft.txt", "draft\n", "reviewer@corp.io"); + expect(push(fx, ["origin", "main"]).code).toBe(0); + git(fx, fx.repo, ["reset", "-q", "--hard", "HEAD~1"]); + commitFile(fx, fx.repo, "CODEOWNERS", "* reviewer@corp.io\n"); + const { code, stderr } = push(fx, ["--force", "origin", "main"]); + expect(code).toBe(0); + expect(stderr).not.toContain("MEDIUM"); + }); + + test("hitting the 50,000-commit cap uses the newest authors and stays quiet about it", () => { + const fx = fixture({ seed: false }); + const count = 50_001; + const lines: string[] = []; + for (let i = 1; i <= count; i++) { + const email = i === 1 ? "oldest@corp.io" : "filler@example.com"; + lines.push("commit refs/heads/main", `mark :${i}`, `committer Someone <${email}> ${1_700_000_000 + i} +0000`, "data 1", "c"); + if (i > 1) lines.push(`from :${i - 1}`); + lines.push(""); + } + const imported = spawnSync("git", ["fast-import", "--quiet"], { + cwd: fx.repo, input: lines.join("\n"), encoding: "utf8", env: fx.env, timeout: 120_000, + }); + expect(imported.status).toBe(0); + git(fx, fx.repo, ["reset", "-q", "--hard", "main"]); + const head = commitFile(fx, fx.repo, "CODEOWNERS", "* oldest@corp.io\n* newest@corp.io\n", "newest@corp.io"); + const { code, stderr } = runHook(fx, `refs/heads/main ${head} refs/heads/main ${ZERO}\n`, []); + expect(code).toBe(0); + const found = mediumLines(stderr); + expect(found).toHaveLength(1); + expect(found[0]).toContain("CODEOWNERS:1"); + expect(stderr).not.toMatch(limitedLine); + }, 120_000); +}); + +describe("each input fails on its own (CEO-23, DX-9)", () => { + /** A `git` that fails or hangs only for `git log`, so every other call is real. */ + function stubGitLog(fx: Fixture, behavior: "fail" | "hang"): Record { + const realGit = Bun.which("git"); + if (!realGit) throw new Error("git not found"); + const bin = path.join(fx.root, "stub-bin"); + fs.mkdirSync(bin, { recursive: true }); + const action = behavior === "fail" ? 'echo "fatal: stub" >&2; exit 128' : "exec sleep 30"; + fs.writeFileSync(path.join(bin, "git"), `#!/bin/sh\nif [ "$1" = "log" ]; then ${action}; fi\nexec "${realGit}" "$@"\n`, { mode: 0o755 }); + const pathKey = Object.keys(fx.env).find((k) => k.toLowerCase() === "path") || "PATH"; + return { [pathKey]: `${bin}${path.delimiter}${fx.env[pathKey] ?? ""}` }; + } + + function pushedRange(fx: Fixture): string { + const base = git(fx, fx.repo, ["rev-parse", "origin/main"]); + const head = git(fx, fx.repo, ["rev-parse", "HEAD"]); + return `refs/heads/main ${head} refs/heads/main ${base}\n`; + } + + test("without user.email, already-public authors are still not reported", () => { + const fx = fixture({ selfEmail: null }); + commitFile(fx, fx.repo, "lib.txt", "code\n", "colleague@corp.io"); + expect(push(fx, ["origin", "main"]).code).toBe(0); + commitFile(fx, fx.repo, "CODEOWNERS", "* colleague@corp.io\n* stranger@corp.io\n", "colleague@corp.io"); + const { code, stderr } = push(fx, ["origin", "main"]); + expect(code).toBe(0); + const found = mediumLines(stderr); + expect(found).toHaveLength(1); + expect(found[0]).toContain("CODEOWNERS:2"); + }); + + for (const behavior of ["fail", "hang"] as const) { + test(`a history read that ${behavior === "fail" ? "errors" : "times out"} still honors the pusher's own address and says suppression was limited`, () => { + const fx = fixture({ selfEmail: "me@corp.io" }); + commitFile(fx, fx.repo, "lib.txt", "code\n", "colleague@corp.io"); + expect(push(fx, ["origin", "main"]).code).toBe(0); + git(fx, fx.repo, ["fetch", "-q", "origin"]); + commitFile(fx, fx.repo, "CODEOWNERS", "* me@corp.io\n* colleague@corp.io\n", "colleague@corp.io"); + const started = Date.now(); + const { code, stderr } = runHook(fx, pushedRange(fx), ["origin", fx.origin], stubGitLog(fx, behavior)); + expect(Date.now() - started).toBeLessThan(25_000); + expect(code).toBe(0); + const found = mediumLines(stderr); + expect(found).toHaveLength(1); + expect(found[0]).toContain("CODEOWNERS:2"); + expect(stderr.split("\n").filter((l) => limitedLine.test(l))).toHaveLength(1); + expect(stderr).toContain(behavior === "fail" ? "reading commit history failed" : "reading commit history took over 5 s"); + }, 40_000); + } +}); + describe("MEDIUM lines point at the real file line (ENG-8)", () => { test("separated hunks in one file report each hunk's own line", () => { const fx = fixture();