diff --git a/bin/gstack-redact b/bin/gstack-redact index ccb6e48c5..f4ee0583f 100755 --- a/bin/gstack-redact +++ b/bin/gstack-redact @@ -160,13 +160,27 @@ function readLines(path: string | undefined): string[] | undefined { function buildOpts(): ScanOptions { const vis = (arg("--repo-visibility") as RepoVisibility) || "unknown"; - const maxBytes = arg("--max-bytes"); + const maxBytesRaw = arg("--max-bytes"); + let maxBytes: number | undefined; + if (maxBytesRaw !== undefined) { + // Reject a malformed cap loudly. Silently passing NaN/0/negative would + // weaken the engine's fail-closed oversize guard (exit 1 = usage error, + // distinct from the 0/2/3 finding-tier codes). + const parsed = Number(maxBytesRaw); + if (!Number.isInteger(parsed) || parsed <= 0) { + process.stderr.write( + `gstack-redact: --max-bytes must be a positive integer, got: ${maxBytesRaw}\n`, + ); + process.exit(1); + } + maxBytes = parsed; + } return { repoVisibility: ["public", "private", "unknown"].includes(vis) ? vis : "unknown", allowlist: readLines(arg("--allowlist")), selfEmail: arg("--self-email"), repoPublicEmails: readLines(arg("--repo-public-emails")), - ...(maxBytes ? { maxBytes: parseInt(maxBytes, 10) } : {}), + ...(maxBytes !== undefined ? { maxBytes } : {}), }; } diff --git a/lib/redact-engine.ts b/lib/redact-engine.ts index 88149f5d9..c4c19882c 100644 --- a/lib/redact-engine.ts +++ b/lib/redact-engine.ts @@ -253,7 +253,15 @@ function emailAllowed(email: string, opts: ScanOptions): boolean { export function scan(input: string, opts: ScanOptions = {}): ScanResult { const repoVisibility: RepoVisibility = opts.repoVisibility ?? "unknown"; - const maxBytes = opts.maxBytes ?? DEFAULT_MAX_BYTES; + // `??` only catches null/undefined, so a NaN (e.g. from a malformed + // `--max-bytes` flag) or a non-positive value would slip through and make + // `byteLen > maxBytes` always false — silently turning this fail-CLOSED guard + // into fail-OPEN. Treat any invalid cap as "use the known-good default". + const requestedMax = opts.maxBytes; + const maxBytes = + typeof requestedMax === "number" && Number.isFinite(requestedMax) && requestedMax > 0 + ? requestedMax + : DEFAULT_MAX_BYTES; // Fail CLOSED on oversize input. Check byte length BEFORE heavy work. const byteLen = Buffer.byteLength(input, "utf8"); diff --git a/test/gstack-redact-cli.test.ts b/test/gstack-redact-cli.test.ts index 4808ba53b..d37b06051 100644 --- a/test/gstack-redact-cli.test.ts +++ b/test/gstack-redact-cli.test.ts @@ -94,4 +94,15 @@ describe("gstack-redact oversize fails closed", () => { expect(code).toBe(3); expect(stdout).toContain("too large"); }); + + // Regression: a malformed --max-bytes must error loudly (usage exit 1), not + // silently pass NaN into the engine where it would disable the fail-closed + // guard. Exit 1 is distinct from the 0/2/3 finding-tier codes. + for (const bad of ["notanumber", "-5", "0", "10.5"]) { + test(`malformed --max-bytes (${bad}) exits 1 with a clear error`, () => { + const { code, stderr } = run(["--max-bytes", bad], "a".repeat(500)); + expect(code).toBe(1); + expect(stderr).toContain("--max-bytes must be a positive integer"); + }); + } }); diff --git a/test/redact-engine.test.ts b/test/redact-engine.test.ts index 52c119a19..64f3b5a39 100644 --- a/test/redact-engine.test.ts +++ b/test/redact-engine.test.ts @@ -239,6 +239,20 @@ describe("oversize fails CLOSED", () => { expect(r.findings[0].id).toBe("engine.input_too_large"); expect(exitCodeFor(r)).toBe(3); }); + + // Regression: an invalid cap (NaN/0/negative) must NOT disable the guard. + // `?? DEFAULT` did not catch NaN, so `byteLen > NaN` was always false and the + // fail-CLOSED guard silently failed OPEN. Invalid caps fall back to the + // 1 MiB default, so a >1 MiB input still blocks. + for (const bad of [NaN, 0, -1]) { + test(`invalid maxBytes (${bad}) falls back to the default cap and still blocks >1 MiB`, () => { + const big = "a".repeat(1024 * 1024 + 1); + const r = scan(big, { maxBytes: bad }); + expect(r.oversize).toBe(true); + expect(r.findings[0].id).toBe("engine.input_too_large"); + expect(exitCodeFor(r)).toBe(3); + }); + } }); describe("validators", () => {