diff --git a/CLAUDE.md b/CLAUDE.md index 9848449020..05941c7055 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -466,8 +466,24 @@ determined leaker (a CHANGELOG line that does would fail a hostile screenshotter / ` ```greptile ` fences so example credentials those tools quote WARN-degrade instead of blocking. A live-format credential inside the fence still blocks. - **Config keys:** `redact_repo_visibility` (public|private|unknown, local-only - override for repos gh/glab can't read), `redact_prepush_hook` (true|false). + override for repos gh/glab can't read), `redact_prepush_hook` (true|false), + `redact_prepush_ignore_globs` (comma-separated globs, local-only supplement). There is intentionally NO key to disable HIGH blocking. +- **Pre-push path-ignore (opt-in, auditable):** a repo can exempt generated data + files (large `*.csv`/`*.parquet` exports that oversize the 1 MiB scan cap and + trip a false-positive HIGH `engine.input_too_large`) from the pre-push scan + without weakening credential scanning on code. Ignore globs merge two sources: + the committed, reviewable `.gstack/redact-prepush-ignore` file at the repo root + (one glob per line, `#` comments — the recommended mechanism, shared across + contributors and visible in git history) and the machine-local + `redact_prepush_ignore_globs` config key (comma-separated, no spaces — for + several globs prefer the committed file). Globs match repo-root-relative paths + (`**` spans directories, e.g. `prospecting/exports/**/*.csv`); matched paths are + dropped from the scanned diff via a git `:(exclude)` pathspec. Every exemption + prints a one-line stderr notice (how many paths / bytes were skipped) — an + ignore is never silent. This preserves the #1946 fail-closed invariant: an + ignore glob is an explicit, auditable opt-in, and any file NOT matched still + fails closed (a bare oversize non-ignored file still blocks). - **Audit:** the /spec semantic pass appends a content-free record (categories + body sha256, no spec text) to `~/.gstack/security/semantic-reviews.jsonl` (0600). diff --git a/bin/gstack-config b/bin/gstack-config index 15c7300c56..3a6fc46108 100755 --- a/bin/gstack-config +++ b/bin/gstack-config @@ -99,6 +99,23 @@ CONFIG_HEADER='# gstack configuration — edit freely, changes take effect on ne # gstack_contributor: false # true = file field reports when gstack misbehaves # skip_eng_review: false # true = skip eng review gate in /ship (not recommended) # +# ─── Redaction guard ───────────────────────────────────────────────── +# redact_prepush_ignore_globs: # Comma-separated globs whose matching paths are +# # EXEMPTED from the pre-push credential scan (e.g. +# # large generated data files that oversize the 1 MiB +# # scan cap and trip a false-positive block). Machine- +# # local supplement to the committed, reviewable +# # .gstack/redact-prepush-ignore file. Empty by default. +# # An exemption is always reported on stderr at push +# # time; non-ignored files still fail closed. `**` +# # spans directories, e.g. +# # prospecting/exports/**/*.parquet,data/**/*.csv +# # NOTE: comma-separated, NO spaces (config values are +# # whitespace-delimited on read — a space would drop +# # the trailing globs). For several globs, prefer the +# # committed .gstack/redact-prepush-ignore file +# # (one glob per line). +# # ─── Workspace-aware ship ──────────────────────────────────────────── # workspace_root: $HOME/conductor/workspaces # Where /ship looks for sibling # # Conductor worktrees when picking a VERSION slot. @@ -133,6 +150,7 @@ lookup_default() { redact_repo_visibility) echo "" ;; # empty → fall through to gh/glab detection redact_prepush_hook) echo "false" ;; + redact_prepush_ignore_globs) echo "" ;; # empty → no paths exempted from the pre-push scan # Brain-aware planning (v1.48 / T5+T10+T16). Defaults documented inline: # brain_trust_policy@ — unset on fresh install; setup-gbrain # writes 'personal' for local engines, diff --git a/bin/gstack-redact-prepush b/bin/gstack-redact-prepush index fd1b05fb93..cb9b3614c9 100755 --- a/bin/gstack-redact-prepush +++ b/bin/gstack-redact-prepush @@ -22,6 +22,22 @@ * - MEDIUM → warn (non-blocking). LOW/WARN → silent. * - GSTACK_REDACT_PREPUSH=skip → log + exit 0 (escape valve). * + * Path-ignore (opt-in, auditable): a repo can exempt generated data files (big + * CSV/JSON exports that oversize the 1 MiB scan cap and would otherwise trip a + * false-positive HIGH `engine.input_too_large`) from the scan. Ignore globs come + * from two merged sources: + * - committed `.gstack/redact-prepush-ignore` at the repo root (one glob per + * line, `#` comments, blank lines skipped) — the recommended mechanism, since + * it is versioned and reviewable in git history and shared across contributors. + * - `gstack-config get redact_prepush_ignore_globs` (comma-separated) — a + * machine-local supplement in ~/.gstack (never committed). + * Globs are matched against repo-root-relative paths (Bun.Glob, so `**` spans + * directories). Matched paths are dropped from the scanned diff via a git + * `:(exclude)` pathspec, and a one-line stderr notice always reports how many + * paths / bytes were skipped — an exemption is never silent. This does NOT weaken + * the #1946 fail-closed posture: anything NOT matched by an explicit ignore glob + * still goes through scan() and a huge non-ignored file still blocks fail-closed. + * * Installed/uninstalled via `gstack-redact install-prepush-hook` (see the * gstack-redact CLI), which chains any pre-existing hook. */ @@ -80,8 +96,84 @@ function defaultRemoteBranch(): string { return "origin/main"; } -/** Return the added-line text for a ref update being pushed. */ -function addedLinesFor(localSha: string, remoteSha: string): string { +/** + * Read the merged, deduped ignore-glob list from the committed + * `.gstack/redact-prepush-ignore` file plus the machine-local + * `redact_prepush_ignore_globs` config key. Empty by default (no behavior + * change for existing repos). Best-effort: an unreadable source contributes + * nothing rather than blocking the push on a config error. + */ +function readIgnoreGlobs(): string[] { + const globs: string[] = []; + + // 1) Committed, reviewable file at the repo root. + const repoRoot = git(["rev-parse", "--show-toplevel"]).trim(); + if (repoRoot) { + try { + const raw = fs.readFileSync(path.join(repoRoot, ".gstack", "redact-prepush-ignore"), "utf8"); + for (const line of raw.split("\n")) { + const g = line.trim(); + if (g && !g.startsWith("#")) globs.push(g); + } + } catch { + // no file (the common case) or unreadable → contribute nothing + } + } + + // 2) Machine-local config supplement (comma-separated; never committed). + try { + const configBin = path.join(import.meta.dir, "gstack-config"); + const r = spawnSync(configBin, ["get", "redact_prepush_ignore_globs"], { encoding: "utf8" }); + if (r.status === 0) { + for (const g of (r.stdout ?? "").trim().split(",")) { + const t = g.trim(); + if (t) globs.push(t); + } + } + } catch { + // gstack-config missing/unreadable → contribute nothing + } + + return [...new Set(globs)]; +} + +/** Sum the byte length of added lines in a unified diff (skip the +++ header). */ +function addedByteLen(diff: string): number { + let bytes = 0; + for (const line of diff.split("\n")) { + if (line.startsWith("+") && !line.startsWith("+++")) { + bytes += Buffer.byteLength(line.slice(1), "utf8"); + } + } + return bytes; +} + +/** Extract the added-line text (drop the +++ header) from a unified diff. */ +function extractAdded(diff: string): string { + const added: string[] = []; + for (const line of diff.split("\n")) { + if (line.startsWith("+") && !line.startsWith("+++")) { + added.push(line.slice(1)); + } + } + return added.join("\n"); +} + +export interface SkipSummary { + paths: number; + bytes: number; + samples: string[]; +} + +/** + * Return the added-line text for a ref update being pushed, plus a summary of + * any paths excluded by ignore globs. Excluded paths never reach scan(). + */ +function addedLinesFor( + localSha: string, + remoteSha: string, + ignoreGlobs: string[], +): { added: string; skip: SkipSummary } { let range: string; if (ZERO.test(remoteSha)) { // New branch: prefer what's unique to localSha vs the remote default branch. @@ -100,28 +192,84 @@ function addedLinesFor(localSha: string, remoteSha: string): string { // Existing branch (incl. force-push): net new content remote..local. range = `${remoteSha}..${localSha}`; } + + const emptySkip: SkipSummary = { paths: 0, bytes: 0, samples: [] }; + + // Fast path: no ignore rules → original behavior, one diff call. // -U0: only changed lines; we keep lines starting with '+' (added), drop the - // +++ file header. Unified diff added lines start with a single '+'. - // Strict (#1946): a failed diff used to return "" and the push sailed - // through unscanned — fail open on the exact path the guard exists for. - const diff = gitStrict(["diff", "--unified=0", "--no-color", range]); - const added: string[] = []; - for (const line of diff.split("\n")) { - if (line.startsWith("+") && !line.startsWith("+++")) { - added.push(line.slice(1)); - } + // +++ file header. Strict (#1946): a failed diff used to return "" and the + // push sailed through unscanned — fail open on the exact path the guard + // exists for. + if (ignoreGlobs.length === 0) { + const diff = gitStrict(["diff", "--unified=0", "--no-color", range]); + return { added: extractAdded(diff), skip: emptySkip }; } - return added.join("\n"); + + // Partition the changed paths against the ignore globs in-process (Bun.Glob, + // so `**` spans directories). We match paths ourselves rather than lean on + // git pathspec glob magic, which keeps the ignore semantics predictable and + // lets us report exactly what was skipped. core.quotePath=false so non-ASCII + // filenames arrive as raw UTF-8 — otherwise git C-quotes them (`"caf\303\251"`) + // and the glob silently fails to match, so the user's ignore rule wouldn't apply. + const names = gitStrict(["-c", "core.quotePath=false", "diff", "--name-only", range]) + .split("\n") + .map((n) => n.trim()) + .filter(Boolean); + const matchers = ignoreGlobs.map((g) => new Bun.Glob(g)); + const ignored = names.filter((n) => matchers.some((m) => m.match(n))); + + if (ignored.length === 0) { + const diff = gitStrict(["diff", "--unified=0", "--no-color", range]); + return { added: extractAdded(diff), skip: emptySkip }; + } + + // Scan diff: everything EXCEPT the ignored paths. `:/` anchors inclusion at + // the repo root regardless of cwd (matches the fast-path's whole-repo + // semantics). Each exclude uses `:(top,exclude,literal)`: `literal` is + // load-bearing — a plain `:(exclude)` is glob-ENABLED, so a real + // filename holding a `?`/`[`/`]` metacharacter (legal on disk) would be read + // as a PATTERN and could drop OTHER, non-ignored files from the scan — a + // fail-open of the #1946 invariant. `top` anchors each exclude at the repo + // root so it lines up with the root-relative names from `--name-only`. + const excludeSpecs = ignored.map((n) => `:(top,exclude,literal)${n}`); + const scanDiff = gitStrict([ + "diff", + "--unified=0", + "--no-color", + range, + "--", + ":/", + ...excludeSpecs, + ]); + + // Skip accounting: diff ONLY the ignored paths to report added bytes. Same + // `:(top,literal)` anchoring/literalness as the exclude specs so the notice + // reports exactly the paths the scan dropped (no glob-overreach divergence). + // The bytes never reach scan(). + const skippedSpecs = ignored.map((n) => `:(top,literal)${n}`); + const skippedDiff = gitStrict(["diff", "--unified=0", "--no-color", range, "--", ...skippedSpecs]); + + return { + added: extractAdded(scanDiff), + skip: { paths: ignored.length, bytes: addedByteLen(skippedDiff), samples: ignored.slice(0, 3) }, + }; } -function logSkip(reason: string): void { +/** Human-readable byte size for the skip notice. */ +function humanBytes(n: number): string { + if (n < 1024) return `${n} B`; + if (n < 1024 * 1024) return `${(n / 1024).toFixed(1)} KB`; + return `${(n / (1024 * 1024)).toFixed(1)} MB`; +} + +function logSkip(reason: string, extra: Record = {}): void { try { const home = process.env.GSTACK_HOME || path.join(os.homedir(), ".gstack"); const dir = path.join(home, "security"); fs.mkdirSync(dir, { recursive: true }); fs.appendFileSync( path.join(dir, "prepush-skip.jsonl"), - JSON.stringify({ ts: new Date().toISOString(), reason }) + "\n", + JSON.stringify({ ts: new Date().toISOString(), reason, ...extra }) + "\n", ); } catch { // best-effort; never block a push because logging failed @@ -142,14 +290,22 @@ function main() { .filter(Boolean) .map((l) => l.split(/\s+/)); + const ignoreGlobs = readIgnoreGlobs(); const allHigh: Finding[] = []; let mediumCount = 0; + const totalSkip: SkipSummary = { paths: 0, bytes: 0, samples: [] }; for (const [, localSha, , remoteSha] of refs) { if (!localSha || ZERO.test(localSha)) continue; // branch delete → nothing pushed let added: string; try { - added = addedLinesFor(localSha, remoteSha || "0"); + const r = addedLinesFor(localSha, remoteSha || "0", ignoreGlobs); + added = r.added; + totalSkip.paths += r.skip.paths; + totalSkip.bytes += r.skip.bytes; + for (const s of r.skip.samples) { + if (totalSkip.samples.length < 3 && !totalSkip.samples.includes(s)) totalSkip.samples.push(s); + } } catch (err) { // Fail CLOSED (#1946): if we can't compute the pushed diff we can't // scan it, and unscanned-but-allowed is the failure mode this hook @@ -172,6 +328,23 @@ function main() { } } + // Always surface an exemption — a skipped path must never be silent. + if (totalSkip.paths > 0) { + const eg = totalSkip.samples.length + ? ` e.g. ${totalSkip.samples.join(", ")}${totalSkip.paths > totalSkip.samples.length ? ", ..." : ""}` + : ""; + process.stderr.write( + `gstack-redact-prepush: skipped ${totalSkip.paths} path(s) / ${humanBytes(totalSkip.bytes)} ` + + `from the credential scan via ignore rules (.gstack/redact-prepush-ignore / ` + + `redact_prepush_ignore_globs).${eg}\n`, + ); + logSkip("ignore-globs", { + paths: totalSkip.paths, + bytes: totalSkip.bytes, + samples: totalSkip.samples, + }); + } + if (mediumCount > 0) { process.stderr.write( `gstack-redact-prepush: ${mediumCount} MEDIUM finding(s) in pushed diff (PII/internal). ` + diff --git a/test/redact-prepush-hook.test.ts b/test/redact-prepush-hook.test.ts index cf85985237..167886ea91 100644 --- a/test/redact-prepush-hook.test.ts +++ b/test/redact-prepush-hook.test.ts @@ -201,6 +201,150 @@ describe("escape valve", () => { }); }); +describe("path-ignore for generated data files (#1946 follow-up)", () => { + // A blob larger than the engine's 1 MiB scan cap. Without an ignore rule this + // trips a false-positive HIGH engine.input_too_large and blocks the push. + const BIG = "x,y\n".repeat(400_000); // ~1.5 MiB + + function writeIgnoreFile(globs: string): void { + fs.mkdirSync(path.join(repo, ".gstack"), { recursive: true }); + fs.writeFileSync(path.join(repo, ".gstack", "redact-prepush-ignore"), globs); + } + + test("(a) an ignored large file passes, and the skip is reported on stderr", () => { + writeIgnoreFile("# generated exports\nprospecting/exports/**/*.csv\n"); + const base = git(["rev-parse", "HEAD"]); + fs.mkdirSync(path.join(repo, "prospecting", "exports", "CA"), { recursive: true }); + fs.writeFileSync(path.join(repo, "prospecting", "exports", "CA", "suspects-1.csv"), BIG); + git(["add", "-A"]); + git(["commit", "-q", "-m", "big export + ignore rule"]); + const head = git(["rev-parse", "HEAD"]); + const { code, stderr } = runHook(`refs/heads/main ${head} refs/heads/main ${base}\n`); + expect(code).toBe(0); + expect(stderr).toContain("skipped 1 path(s)"); + expect(stderr).toContain("suspects-1.csv"); + expect(stderr).toContain(".gstack/redact-prepush-ignore"); + }); + + test("(b) a non-ignored large file still blocks fail-closed", () => { + writeIgnoreFile("prospecting/exports/**/*.csv\n"); + const base = git(["rev-parse", "HEAD"]); + // Matches no ignore glob → must still oversize-block. + fs.writeFileSync(path.join(repo, "bigdata.txt"), BIG); + git(["add", "-A"]); + git(["commit", "-q", "-m", "big non-ignored file"]); + const head = git(["rev-parse", "HEAD"]); + const { code, stderr } = runHook(`refs/heads/main ${head} refs/heads/main ${base}\n`); + expect(code).toBe(1); + expect(stderr).toContain("BLOCKED"); + expect(stderr).toContain("engine.input_too_large"); + }); + + test("(c) a real secret in a non-ignored file still blocks even with ignore rules present", () => { + writeIgnoreFile("prospecting/exports/**/*.csv\n"); + const base = git(["rev-parse", "HEAD"]); + fs.writeFileSync(path.join(repo, "config.env"), "key AKIA1234567890ABCDEF\n"); + git(["add", "-A"]); + git(["commit", "-q", "-m", "secret in code"]); + const head = git(["rev-parse", "HEAD"]); + const { code, stderr } = runHook(`refs/heads/main ${head} refs/heads/main ${base}\n`); + expect(code).toBe(1); + expect(stderr).toContain("BLOCKED"); + expect(stderr).toContain("aws.access_key"); + }); + + test("(d) an explicit ignore glob exempts its file even when it holds a secret (auditable opt-in)", () => { + // The tradeoff is intentional and loud: an ignore glob is a versioned, + // reviewable opt-in, and the skip is always reported. Anything NOT matched + // stays fail-closed (see tests b + c). + writeIgnoreFile("prospecting/exports/**/*.csv\n"); + const base = git(["rev-parse", "HEAD"]); + fs.mkdirSync(path.join(repo, "prospecting", "exports", "CA"), { recursive: true }); + fs.writeFileSync( + path.join(repo, "prospecting", "exports", "CA", "x.csv"), + "AKIA1234567890ABCDEF\n", + ); + git(["add", "-A"]); + git(["commit", "-q", "-m", "secret inside ignored export"]); + const head = git(["rev-parse", "HEAD"]); + const { code, stderr } = runHook(`refs/heads/main ${head} refs/heads/main ${base}\n`); + expect(code).toBe(0); + expect(stderr).toContain("skipped 1 path(s)"); + expect(stderr).not.toContain("aws.access_key"); + }); + + test("(e) the machine-local config key is an additional ignore source", () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "ghome-cfg-")); + const cfg = path.resolve(import.meta.dir, "..", "bin", "gstack-config"); + spawnSync(cfg, ["set", "redact_prepush_ignore_globs", "data/**/*.parquet"], { + env: { ...process.env, GSTACK_HOME: home }, + }); + const base = git(["rev-parse", "HEAD"]); + fs.mkdirSync(path.join(repo, "data"), { recursive: true }); + fs.writeFileSync(path.join(repo, "data", "big.parquet"), BIG); + git(["add", "-A"]); + git(["commit", "-q", "-m", "big parquet, no committed ignore file"]); + const head = git(["rev-parse", "HEAD"]); + const { code, stderr } = runHook(`refs/heads/main ${head} refs/heads/main ${base}\n`, { + GSTACK_HOME: home, + }); + expect(code).toBe(0); + expect(stderr).toContain("skipped 1 path(s)"); + expect(stderr).toContain("big.parquet"); + fs.rmSync(home, { recursive: true, force: true }); + }); + + test("(f) no ignore rules → default behavior unchanged (large file blocks)", () => { + const base = git(["rev-parse", "HEAD"]); + fs.writeFileSync(path.join(repo, "export.csv"), BIG); + git(["add", "-A"]); + git(["commit", "-q", "-m", "big csv, no ignore rules"]); + const head = git(["rev-parse", "HEAD"]); + const { code, stderr } = runHook(`refs/heads/main ${head} refs/heads/main ${base}\n`); + expect(code).toBe(1); + expect(stderr).toContain("engine.input_too_large"); + expect(stderr).not.toContain("skipped"); + }); + + test("(g) an ignore glob applies to a non-ASCII filename (core.quotePath=false)", () => { + // Regression: git C-quotes non-ASCII paths by default (`"caf\303\251.csv"`), + // which the glob can't match — the ignore rule would silently fail to apply + // and the large export would still oversize-block. The hook forces + // core.quotePath=false so the raw UTF-8 path matches. + writeIgnoreFile("prospecting/exports/**/*.csv\n"); + const base = git(["rev-parse", "HEAD"]); + fs.mkdirSync(path.join(repo, "prospecting", "exports"), { recursive: true }); + fs.writeFileSync(path.join(repo, "prospecting", "exports", "café.csv"), BIG); + git(["add", "-A"]); + git(["commit", "-q", "-m", "big export with non-ascii name"]); + const head = git(["rev-parse", "HEAD"]); + const { code, stderr } = runHook(`refs/heads/main ${head} refs/heads/main ${base}\n`); + expect(code).toBe(0); + expect(stderr).toContain("skipped 1 path(s)"); + expect(stderr).toContain("café.csv"); + }); + + test("(h) a glob metacharacter in an ignored filename does NOT exclude a sibling (fail-open guard)", () => { + // Fail-open regression (adversarial review): a real filename holding a `?` + // is a legal path. A plain `:(exclude)export?.csv` pathspec is glob-ENABLED, + // so git would ALSO drop `exportX.csv` (a sibling holding a real secret) from + // the scan. `:(top,exclude,literal)` treats the name literally, so only the + // exact ignored file is exempted and the sibling secret still blocks. + // The glob is `export\?.csv` (escaped) so Bun.Glob matches ONLY the literal + // `export?.csv` — isolating the git pathspec overreach, not Bun.Glob's own. + writeIgnoreFile("export\\?.csv\n"); + const base = git(["rev-parse", "HEAD"]); + fs.writeFileSync(path.join(repo, "export?.csv"), "benign generated data\n"); + fs.writeFileSync(path.join(repo, "exportX.csv"), "key AKIA1234567890ABCDEF\n"); + git(["add", "-A"]); + git(["commit", "-q", "-m", "ignored ? file + sibling with a secret"]); + const head = git(["rev-parse", "HEAD"]); + const { code, stderr } = runHook(`refs/heads/main ${head} refs/heads/main ${base}\n`); + expect(code).toBe(1); // the sibling's credential must still block + expect(stderr).toContain("aws.access_key"); + }); +}); + describe("install / chaining", () => { test("install creates a managed hook; existing hook preserved + chained", () => { const hookDir = path.join(repo, ".git", "hooks");