From ca671d6f658cf21f4d92392ef4b824f79af86eac Mon Sep 17 00:00:00 2001 From: Ricky Date: Mon, 17 Aug 2026 18:32:40 +0800 Subject: [PATCH] fix(redact): a typo'd subcommand no longer exits 0 having done nothing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit main() recognised exactly two subcommands and let everything else fall through to the stdin scan. On empty stdin that prints "(no findings)" and exits 0, so: $ gstack-redact install-prepush-hooks # plural typo gstack-redact scan — repo UNKNOWN (no findings) $ echo $? 0 No hook was installed, and the operator has every reason to believe the credential guard is armed. A guard that silently no-ops must never exit 0. Two smaller faults in the same dispatch, both of which lead people here: - There was no --help handler, so `gstack-redact --help` fell through to the scanner. Piping a credential to it scanned the secret and exited 3. - With no piped input and no --from-file, readInput() blocks on readSync(fd 0) until an EOF that an interactive terminal never sends. That prints nothing at all, so it reads as a hang rather than as "this is a filter, feed it". Now: --help/-h/help prints usage and exits 0; an unrecognised positional prints the offender and exits 1; a TTY with nothing piped in prints usage instead of blocking. "scan" stays accepted, because the human output header reads "gstack-redact scan — repo …" and that is what people type. Usage errors exit 1, deliberately not 2 or 3. Those mean MEDIUM and HIGH findings and callers gate dispatch on them, so a usage error exiting 2 would be read as "medium findings — prompt the user". A test pins that. Tests: 4 written failing first, then fixed. Full suite 7,722 pass / 0 fail. Co-Authored-By: Claude Opus 5 --- bin/gstack-redact | 54 ++++++++++++++++++++++++++++++++++ test/gstack-redact-cli.test.ts | 44 +++++++++++++++++++++++++++ 2 files changed, 98 insertions(+) diff --git a/bin/gstack-redact b/bin/gstack-redact index edcea8ef4..740af3f08 100755 --- a/bin/gstack-redact +++ b/bin/gstack-redact @@ -199,13 +199,67 @@ function humanTable(findings: Finding[]): string { return rows.join("\n"); } +/** + * Usage. Exits 0 when asked for (--help), 1 when the invocation was wrong. + * + * Deliberately NOT 2 or 3: those mean MEDIUM and HIGH findings, and callers + * gate dispatch on them (see the exit-code table at the top). A usage error + * that exited 2 would be read as "medium findings — prompt the user". + */ +function printUsage(code: number): never { + const out = code === 0 ? process.stdout : process.stderr; + out.write( + "gstack-redact — scan text for secrets/PII/legal content.\n" + + "\n" + + "Reads the text to scan from STDIN, or from --from-file PATH. It is a\n" + + "filter: with nothing piped in it has nothing to scan.\n" + + "\n" + + " git diff | gstack-redact --repo-visibility private\n" + + " gstack-redact --from-file notes.md --json\n" + + "\n" + + "Subcommands:\n" + + " install-prepush-hook install the managed git pre-push credential guard\n" + + " uninstall-prepush-hook remove it\n" + + "\n" + + "Flags: --json --repo-visibility V --from-file PATH --allowlist PATH\n" + + " --self-email EMAIL --repo-public-emails PATH --auto-redact IDS\n" + + " --max-bytes N\n" + + "\n" + + "Exit: 0 clean · 1 usage error · 2 MEDIUM present · 3 HIGH present\n", + ); + process.exit(code); +} + function main() { // Subcommands (positional, not flags). const sub = process.argv[2]; if (sub === "install-prepush-hook") return installPrepushHook(); if (sub === "uninstall-prepush-hook") return uninstallPrepushHook(); + if (sub === "--help" || sub === "-h" || sub === "help") return printUsage(0); + + // An unrecognized POSITIONAL is a typo, not input. This used to fall through + // to the stdin scan, which on empty stdin prints "(no findings)" and exits 0 + // — so `install-prepush-hooks` (plural) installed nothing and still looked + // like success, leaving the credential guard absent while the operator + // believed it was armed. A guard that no-ops must never exit 0. + // + // "scan" is exempt: the human output header reads "gstack-redact scan — + // repo …", so people reasonably type it. It stays an alias for the default. + // Flags start with "-" and are parsed further down, so only bare words land + // here. + if (sub !== undefined && sub !== "scan" && !sub.startsWith("-")) { + process.stderr.write(`gstack-redact: unknown subcommand "${sub}"\n\n`); + return printUsage(1); + } const opts = buildOpts(); + + // Nothing piped in and no --from-file: readInput() below blocks on + // readSync(fd 0) until EOF, which on an interactive terminal never comes. + // That prints nothing at all and is indistinguishable from a crash or a + // slow scan. Show usage instead of hanging silently. + if (!arg("--from-file") && process.stdin.isTTY) return printUsage(1); + const input = readInput(); // Auto-redact mode: print redacted body to stdout, diff to stderr, exit 0. diff --git a/test/gstack-redact-cli.test.ts b/test/gstack-redact-cli.test.ts index 4808ba53b..4de4ba00d 100644 --- a/test/gstack-redact-cli.test.ts +++ b/test/gstack-redact-cli.test.ts @@ -95,3 +95,47 @@ describe("gstack-redact oversize fails closed", () => { expect(stdout).toContain("too large"); }); }); + +describe("gstack-redact argv dispatch", () => { + // The bug: main() recognised exactly two subcommands and let everything else + // fall through to the stdin scan, which reports "no findings" and exits 0. + // So `install-prepush-hooks` (plural typo) installed no hook and still looked + // like success — the credential guard silently absent while the operator + // believes it is armed. A guard that no-ops must never exit 0. + test("a typo'd install subcommand fails loudly instead of exiting 0", () => { + const { code, stderr } = run(["install-prepush-hooks"], ""); + expect(code).not.toBe(0); + expect(stderr).toContain("unknown subcommand"); + }); + + test("an unknown positional never reports a clean scan", () => { + const { code, stdout } = run(["totally-bogus"], ""); + expect(code).not.toBe(0); + expect(stdout).not.toContain("HIGH=0"); + }); + + // Usage errors must not collide with the findings codes (2 = MEDIUM, + // 3 = HIGH); a caller gating on those would read a typo as "findings". + test("usage errors exit 1, not a findings code", () => { + expect(run(["totally-bogus"], "").code).toBe(1); + }); + + test("--help prints usage and exits 0 without scanning", () => { + const { code, stdout } = run(["--help"], "key AKIA1234567890ABCDEF"); + expect(code).toBe(0); + expect(stdout).toContain("STDIN"); + expect(stdout).not.toContain("HIGH=1"); + }); + + // "scan" is what the human output header ("gstack-redact scan — repo …") + // invites people to type, so it stays an accepted alias for the default + // filter mode. Rejecting it would break that muscle memory for no gain. + test("the 'scan' alias still scans normally", () => { + expect(run(["scan"], "key AKIA1234567890ABCDEF").code).toBe(3); + expect(run(["scan"], "just prose").code).toBe(0); + }); + + test("flags are still parsed, not mistaken for subcommands", () => { + expect(run(["--json"], "just prose").code).toBe(0); + }); +});