diff --git a/bin/gstack-redact-prepush b/bin/gstack-redact-prepush index 25fc8c1d4..57e04ce9e 100755 --- a/bin/gstack-redact-prepush +++ b/bin/gstack-redact-prepush @@ -35,11 +35,33 @@ const ZERO = /^0+$/; // The canonical empty-tree object; diffing against it yields all content as added. const EMPTY_TREE = "4b825dc642cb6eb9a060e54bf8d69288fbee4904"; +/** + * Permissive git for legitimately-fallible PROBES (symbolic-ref, rev-parse, + * merge-base) where a non-zero exit is normal control flow. The DIFF call + * must NOT use this — see gitStrict (#1946 fail-closed). + */ function git(args: string[]): string { const r = spawnSync("git", args, { encoding: "utf8", maxBuffer: 64 * 1024 * 1024 }); return r.status === 0 ? (r.stdout ?? "") : ""; } +/** + * Fail-closed git for the diff that decides whether the push is scanned + * (#1946). status !== 0 covers repo errors; status === null covers a killed + * process AND maxBuffer overflow — the oversized-diff case is exactly where + * a large secret-bearing blob is most likely, so "couldn't read the diff" + * must block, not silently allow. + */ +function gitStrict(args: string[]): string { + const r = spawnSync("git", args, { encoding: "utf8", maxBuffer: 64 * 1024 * 1024 }); + if (r.status !== 0 || r.status === null) { + throw new Error( + `git ${args[0]} failed (status=${r.status ?? "killed/overflow"}): ${(r.stderr ?? "").slice(0, 300)}`, + ); + } + return r.stdout ?? ""; +} + function defaultRemoteBranch(): string { // origin/HEAD → origin/main, fall back to main/master. const sym = git(["symbolic-ref", "refs/remotes/origin/HEAD"]).trim(); @@ -65,7 +87,9 @@ function addedLinesFor(localSha: string, remoteSha: string): string { } // -U0: only changed lines; we keep lines starting with '+' (added), drop the // +++ file header. Unified diff added lines start with a single '+'. - const diff = git(["diff", "--unified=0", "--no-color", range]); + // 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("+++")) { @@ -108,7 +132,21 @@ function main() { for (const [, localSha, , remoteSha] of refs) { if (!localSha || ZERO.test(localSha)) continue; // branch delete → nothing pushed - const added = addedLinesFor(localSha, remoteSha || "0"); + let added: string; + try { + added = addedLinesFor(localSha, remoteSha || "0"); + } 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 + // exists to prevent. + process.stderr.write( + "\n⛔ gstack-redact-prepush BLOCKED the push — could not compute the pushed diff, " + + "so it cannot be scanned for credentials.\n" + + ` (${err instanceof Error ? err.message.split("\n")[0] : String(err)})\n` + + "Bypass if you're sure: GSTACK_REDACT_PREPUSH=skip git push (or git push --no-verify)\n", + ); + process.exit(1); + } if (!added.trim()) continue; // Visibility doesn't change HIGH behavior; pass private so nothing is treated // as public-strict (HIGH blocks regardless either way). diff --git a/setup b/setup index ec1db22b7..bd4161c3c 100755 --- a/setup +++ b/setup @@ -1487,3 +1487,15 @@ fi if [ "$NO_TEAM_MODE" -eq 1 ] && [ -x "$SETTINGS_HOOK" ]; then "$SETTINGS_HOOK" remove-source --source plan-tune-cathedral 2>/dev/null || true fi + +# ─── Redact pre-push guard hint (#1946) ────────────────────────────────────── +# The credential pre-push hook is per-REPO state — setup runs in the gstack +# checkout, the wrong repo to install it into. /ship offers the install once +# at the moment of relevance (first push) and silently installs in any repo +# where redact_prepush_hook=true. This hint is setup's whole involvement. +if [ "$("$GSTACK_CONFIG" get redact_prepush_hook 2>/dev/null || echo false)" != "true" ]; then + log "" + log "Tip: gstack can block pushes containing credentials (per-repo git hook)." + log " Enable once: gstack-config set redact_prepush_hook true — /ship" + log " installs the hook automatically in every repo you ship from." +fi diff --git a/ship/SKILL.md b/ship/SKILL.md index f78dcd5a0..b7ce93c03 100644 --- a/ship/SKILL.md +++ b/ship/SKILL.md @@ -1218,6 +1218,49 @@ Claiming work is complete without verification is dishonesty, not efficiency. ## Step 17: Push +**Credential pre-push guard (#1946) — run before the push:** + +```bash +_REDACT_PREPUSH=$(~/.claude/skills/gstack/bin/gstack-config get redact_prepush_hook 2>/dev/null || echo "false") +_HOOK_PATH=$(git rev-parse --git-path hooks/pre-push 2>/dev/null || echo "") +_HOOK_INSTALLED="no" +[ -n "$_HOOK_PATH" ] && [ -f "$_HOOK_PATH" ] && grep -q "gstack-redact" "$_HOOK_PATH" 2>/dev/null && _HOOK_INSTALLED="yes" +_PREPUSH_PROMPTED=$([ -f ~/.gstack/.redact-prepush-prompted ] && echo "yes" || echo "no") +echo "REDACT_PREPUSH: $_REDACT_PREPUSH" +echo "HOOK_INSTALLED: $_HOOK_INSTALLED" +echo "PREPUSH_PROMPTED: $_PREPUSH_PROMPTED" +``` + +Branch on the echoed values: + +1. **`REDACT_PREPUSH: true` and `HOOK_INSTALLED: no`** — consent already + given; install silently (no question) and continue: + ```bash + ~/.claude/skills/gstack/bin/gstack-redact install-prepush-hook + ``` +2. **`REDACT_PREPUSH` not true AND `PREPUSH_PROMPTED: no`** — one-time + offer (fires once EVER, machine-wide). AskUserQuestion: + + > gstack can install a per-repo git pre-push hook that blocks pushes + > containing credentials (API keys, tokens, private keys). It's a + > guardrail, not enforcement — `GSTACK_REDACT_PREPUSH=skip` bypasses it. + > Install it for repos you ship from? + + Options: + - A) Yes — install the credential guard (recommended) + - B) No — never ask again + + If A: run `~/.claude/skills/gstack/bin/gstack-config set redact_prepush_hook true` + then `~/.claude/skills/gstack/bin/gstack-redact install-prepush-hook`. + If B: run `~/.claude/skills/gstack/bin/gstack-config set redact_prepush_hook false`. + ALWAYS (after either answer, but NOT if the question itself failed to + render — a failed AskUserQuestion must re-offer next time): + ```bash + touch ~/.gstack/.redact-prepush-prompted + ``` +3. **Anything else** (declined earlier, or already installed) — continue + without comment. + **Idempotency check:** Check if the branch is already pushed and up to date. ```bash diff --git a/ship/SKILL.md.tmpl b/ship/SKILL.md.tmpl index fb39e73b6..538f154a4 100644 --- a/ship/SKILL.md.tmpl +++ b/ship/SKILL.md.tmpl @@ -381,6 +381,49 @@ Claiming work is complete without verification is dishonesty, not efficiency. ## Step 17: Push +**Credential pre-push guard (#1946) — run before the push:** + +```bash +_REDACT_PREPUSH=$(~/.claude/skills/gstack/bin/gstack-config get redact_prepush_hook 2>/dev/null || echo "false") +_HOOK_PATH=$(git rev-parse --git-path hooks/pre-push 2>/dev/null || echo "") +_HOOK_INSTALLED="no" +[ -n "$_HOOK_PATH" ] && [ -f "$_HOOK_PATH" ] && grep -q "gstack-redact" "$_HOOK_PATH" 2>/dev/null && _HOOK_INSTALLED="yes" +_PREPUSH_PROMPTED=$([ -f ~/.gstack/.redact-prepush-prompted ] && echo "yes" || echo "no") +echo "REDACT_PREPUSH: $_REDACT_PREPUSH" +echo "HOOK_INSTALLED: $_HOOK_INSTALLED" +echo "PREPUSH_PROMPTED: $_PREPUSH_PROMPTED" +``` + +Branch on the echoed values: + +1. **`REDACT_PREPUSH: true` and `HOOK_INSTALLED: no`** — consent already + given; install silently (no question) and continue: + ```bash + ~/.claude/skills/gstack/bin/gstack-redact install-prepush-hook + ``` +2. **`REDACT_PREPUSH` not true AND `PREPUSH_PROMPTED: no`** — one-time + offer (fires once EVER, machine-wide). AskUserQuestion: + + > gstack can install a per-repo git pre-push hook that blocks pushes + > containing credentials (API keys, tokens, private keys). It's a + > guardrail, not enforcement — `GSTACK_REDACT_PREPUSH=skip` bypasses it. + > Install it for repos you ship from? + + Options: + - A) Yes — install the credential guard (recommended) + - B) No — never ask again + + If A: run `~/.claude/skills/gstack/bin/gstack-config set redact_prepush_hook true` + then `~/.claude/skills/gstack/bin/gstack-redact install-prepush-hook`. + If B: run `~/.claude/skills/gstack/bin/gstack-config set redact_prepush_hook false`. + ALWAYS (after either answer, but NOT if the question itself failed to + render — a failed AskUserQuestion must re-offer next time): + ```bash + touch ~/.gstack/.redact-prepush-prompted + ``` +3. **Anything else** (declined earlier, or already installed) — continue + without comment. + **Idempotency check:** Check if the branch is already pushed and up to date. ```bash diff --git a/test/fixtures/golden/claude-ship-SKILL.md b/test/fixtures/golden/claude-ship-SKILL.md index f78dcd5a0..b7ce93c03 100644 --- a/test/fixtures/golden/claude-ship-SKILL.md +++ b/test/fixtures/golden/claude-ship-SKILL.md @@ -1218,6 +1218,49 @@ Claiming work is complete without verification is dishonesty, not efficiency. ## Step 17: Push +**Credential pre-push guard (#1946) — run before the push:** + +```bash +_REDACT_PREPUSH=$(~/.claude/skills/gstack/bin/gstack-config get redact_prepush_hook 2>/dev/null || echo "false") +_HOOK_PATH=$(git rev-parse --git-path hooks/pre-push 2>/dev/null || echo "") +_HOOK_INSTALLED="no" +[ -n "$_HOOK_PATH" ] && [ -f "$_HOOK_PATH" ] && grep -q "gstack-redact" "$_HOOK_PATH" 2>/dev/null && _HOOK_INSTALLED="yes" +_PREPUSH_PROMPTED=$([ -f ~/.gstack/.redact-prepush-prompted ] && echo "yes" || echo "no") +echo "REDACT_PREPUSH: $_REDACT_PREPUSH" +echo "HOOK_INSTALLED: $_HOOK_INSTALLED" +echo "PREPUSH_PROMPTED: $_PREPUSH_PROMPTED" +``` + +Branch on the echoed values: + +1. **`REDACT_PREPUSH: true` and `HOOK_INSTALLED: no`** — consent already + given; install silently (no question) and continue: + ```bash + ~/.claude/skills/gstack/bin/gstack-redact install-prepush-hook + ``` +2. **`REDACT_PREPUSH` not true AND `PREPUSH_PROMPTED: no`** — one-time + offer (fires once EVER, machine-wide). AskUserQuestion: + + > gstack can install a per-repo git pre-push hook that blocks pushes + > containing credentials (API keys, tokens, private keys). It's a + > guardrail, not enforcement — `GSTACK_REDACT_PREPUSH=skip` bypasses it. + > Install it for repos you ship from? + + Options: + - A) Yes — install the credential guard (recommended) + - B) No — never ask again + + If A: run `~/.claude/skills/gstack/bin/gstack-config set redact_prepush_hook true` + then `~/.claude/skills/gstack/bin/gstack-redact install-prepush-hook`. + If B: run `~/.claude/skills/gstack/bin/gstack-config set redact_prepush_hook false`. + ALWAYS (after either answer, but NOT if the question itself failed to + render — a failed AskUserQuestion must re-offer next time): + ```bash + touch ~/.gstack/.redact-prepush-prompted + ``` +3. **Anything else** (declined earlier, or already installed) — continue + without comment. + **Idempotency check:** Check if the branch is already pushed and up to date. ```bash diff --git a/test/fixtures/golden/codex-ship-SKILL.md b/test/fixtures/golden/codex-ship-SKILL.md index 95de61439..3883df2c3 100644 --- a/test/fixtures/golden/codex-ship-SKILL.md +++ b/test/fixtures/golden/codex-ship-SKILL.md @@ -2385,6 +2385,49 @@ Claiming work is complete without verification is dishonesty, not efficiency. ## Step 17: Push +**Credential pre-push guard (#1946) — run before the push:** + +```bash +_REDACT_PREPUSH=$($GSTACK_ROOT/bin/gstack-config get redact_prepush_hook 2>/dev/null || echo "false") +_HOOK_PATH=$(git rev-parse --git-path hooks/pre-push 2>/dev/null || echo "") +_HOOK_INSTALLED="no" +[ -n "$_HOOK_PATH" ] && [ -f "$_HOOK_PATH" ] && grep -q "gstack-redact" "$_HOOK_PATH" 2>/dev/null && _HOOK_INSTALLED="yes" +_PREPUSH_PROMPTED=$([ -f ~/.gstack/.redact-prepush-prompted ] && echo "yes" || echo "no") +echo "REDACT_PREPUSH: $_REDACT_PREPUSH" +echo "HOOK_INSTALLED: $_HOOK_INSTALLED" +echo "PREPUSH_PROMPTED: $_PREPUSH_PROMPTED" +``` + +Branch on the echoed values: + +1. **`REDACT_PREPUSH: true` and `HOOK_INSTALLED: no`** — consent already + given; install silently (no question) and continue: + ```bash + $GSTACK_ROOT/bin/gstack-redact install-prepush-hook + ``` +2. **`REDACT_PREPUSH` not true AND `PREPUSH_PROMPTED: no`** — one-time + offer (fires once EVER, machine-wide). AskUserQuestion: + + > gstack can install a per-repo git pre-push hook that blocks pushes + > containing credentials (API keys, tokens, private keys). It's a + > guardrail, not enforcement — `GSTACK_REDACT_PREPUSH=skip` bypasses it. + > Install it for repos you ship from? + + Options: + - A) Yes — install the credential guard (recommended) + - B) No — never ask again + + If A: run `$GSTACK_ROOT/bin/gstack-config set redact_prepush_hook true` + then `$GSTACK_ROOT/bin/gstack-redact install-prepush-hook`. + If B: run `$GSTACK_ROOT/bin/gstack-config set redact_prepush_hook false`. + ALWAYS (after either answer, but NOT if the question itself failed to + render — a failed AskUserQuestion must re-offer next time): + ```bash + touch ~/.gstack/.redact-prepush-prompted + ``` +3. **Anything else** (declined earlier, or already installed) — continue + without comment. + **Idempotency check:** Check if the branch is already pushed and up to date. ```bash diff --git a/test/fixtures/golden/factory-ship-SKILL.md b/test/fixtures/golden/factory-ship-SKILL.md index dd8d0e01d..7606687f5 100644 --- a/test/fixtures/golden/factory-ship-SKILL.md +++ b/test/fixtures/golden/factory-ship-SKILL.md @@ -2791,6 +2791,49 @@ Claiming work is complete without verification is dishonesty, not efficiency. ## Step 17: Push +**Credential pre-push guard (#1946) — run before the push:** + +```bash +_REDACT_PREPUSH=$($GSTACK_ROOT/bin/gstack-config get redact_prepush_hook 2>/dev/null || echo "false") +_HOOK_PATH=$(git rev-parse --git-path hooks/pre-push 2>/dev/null || echo "") +_HOOK_INSTALLED="no" +[ -n "$_HOOK_PATH" ] && [ -f "$_HOOK_PATH" ] && grep -q "gstack-redact" "$_HOOK_PATH" 2>/dev/null && _HOOK_INSTALLED="yes" +_PREPUSH_PROMPTED=$([ -f ~/.gstack/.redact-prepush-prompted ] && echo "yes" || echo "no") +echo "REDACT_PREPUSH: $_REDACT_PREPUSH" +echo "HOOK_INSTALLED: $_HOOK_INSTALLED" +echo "PREPUSH_PROMPTED: $_PREPUSH_PROMPTED" +``` + +Branch on the echoed values: + +1. **`REDACT_PREPUSH: true` and `HOOK_INSTALLED: no`** — consent already + given; install silently (no question) and continue: + ```bash + $GSTACK_ROOT/bin/gstack-redact install-prepush-hook + ``` +2. **`REDACT_PREPUSH` not true AND `PREPUSH_PROMPTED: no`** — one-time + offer (fires once EVER, machine-wide). AskUserQuestion: + + > gstack can install a per-repo git pre-push hook that blocks pushes + > containing credentials (API keys, tokens, private keys). It's a + > guardrail, not enforcement — `GSTACK_REDACT_PREPUSH=skip` bypasses it. + > Install it for repos you ship from? + + Options: + - A) Yes — install the credential guard (recommended) + - B) No — never ask again + + If A: run `$GSTACK_ROOT/bin/gstack-config set redact_prepush_hook true` + then `$GSTACK_ROOT/bin/gstack-redact install-prepush-hook`. + If B: run `$GSTACK_ROOT/bin/gstack-config set redact_prepush_hook false`. + ALWAYS (after either answer, but NOT if the question itself failed to + render — a failed AskUserQuestion must re-offer next time): + ```bash + touch ~/.gstack/.redact-prepush-prompted + ``` +3. **Anything else** (declined earlier, or already installed) — continue + without comment. + **Idempotency check:** Check if the branch is already pushed and up to date. ```bash diff --git a/test/redact-prepush-hook.test.ts b/test/redact-prepush-hook.test.ts index 8447cf6d5..589f679c6 100644 --- a/test/redact-prepush-hook.test.ts +++ b/test/redact-prepush-hook.test.ts @@ -107,6 +107,46 @@ describe("diff direction + special refs", () => { }); }); +describe("fail closed on unscannable diffs (#1946)", () => { + test("a diff git cannot compute BLOCKS the push and names the escape valve", () => { + // Bogus-but-well-formed SHAs: git diff exits non-zero, the old git() + // helper returned "" and the push sailed through unscanned. + const bogusLocal = "a".repeat(40); + const bogusRemote = "b".repeat(40); + const { code, stderr } = runHook( + `refs/heads/main ${bogusLocal} refs/heads/main ${bogusRemote}\n`, + ); + expect(code).toBe(1); + expect(stderr).toContain("could not compute the pushed diff"); + expect(stderr).toContain("GSTACK_REDACT_PREPUSH=skip"); + }); + + test("an empty-but-successful diff still passes (no-op push)", () => { + const head = git(["rev-parse", "HEAD"]); + // remote == local: diff succeeds and is empty — must NOT block. + const { code } = runHook(`refs/heads/main ${head} refs/heads/main ${head}\n`); + expect(code).toBe(0); + }); +}); + +describe("install UX surfaces (#1946 / eng review D3+D10)", () => { + const ROOT = path.resolve(import.meta.dir, ".."); + + test("setup carries the hint only — never a per-repo install (it runs in the wrong repo)", () => { + const setup = fs.readFileSync(path.join(ROOT, "setup"), "utf8"); + expect(setup).toContain("redact_prepush_hook"); + // The hint must not invoke the installer from setup. + expect(setup).not.toContain("install-prepush-hook"); + }); + + test("ship template owns per-repo install: silent-install path + one-time offer marker", () => { + const tmpl = fs.readFileSync(path.join(ROOT, "ship", "SKILL.md.tmpl"), "utf8"); + expect(tmpl).toContain("install-prepush-hook"); + expect(tmpl).toContain(".redact-prepush-prompted"); + expect(tmpl).toContain("redact_prepush_hook"); + }); +}); + describe("escape valve", () => { test("GSTACK_REDACT_PREPUSH=skip bypasses + logs", () => { const base = git(["rev-parse", "HEAD"]);