diff --git a/bin/gstack-redact b/bin/gstack-redact index ccb6e48c5..41bd54c65 100755 --- a/bin/gstack-redact +++ b/bin/gstack-redact @@ -161,12 +161,25 @@ function readLines(path: string | undefined): string[] | undefined { function buildOpts(): ScanOptions { const vis = (arg("--repo-visibility") as RepoVisibility) || "unknown"; const maxBytes = arg("--max-bytes"); + // #1824: validate the RAW string, not the parse result. parseInt("123abc") + // is 123 and parseInt("foo") is NaN — both silently corrupt the fail-closed + // oversize guard. Require a clean positive integer or reject before scanning. + let maxBytesOpt: number | undefined; + if (maxBytes !== undefined) { + if (!/^\d+$/.test(maxBytes) || Number(maxBytes) <= 0) { + process.stderr.write( + `gstack-redact: --max-bytes must be a positive integer (got "${maxBytes}")\n`, + ); + process.exit(1); + } + maxBytesOpt = Number(maxBytes); + } 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) } : {}), + ...(maxBytesOpt !== undefined ? { maxBytes: maxBytesOpt } : {}), }; } diff --git a/lib/redact-engine.ts b/lib/redact-engine.ts index 88149f5d9..02cf66829 100644 --- a/lib/redact-engine.ts +++ b/lib/redact-engine.ts @@ -253,7 +253,16 @@ 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; + // #1824: ?? only catches null/undefined, not NaN or <= 0. A bad value + // (NaN from a malformed --max-bytes, or a negative) would make `byteLen > + // maxBytes` always false and silently disable the fail-closed oversize guard. + // Guardrail: any non-finite or non-positive value falls back to the default + // cap. The CLI is the layer that rejects bad args; this is belt-and-suspenders + // so the engine never silently runs uncapped. + const maxBytes = + Number.isFinite(opts.maxBytes) && (opts.maxBytes as number) > 0 + ? (opts.maxBytes as number) + : DEFAULT_MAX_BYTES; // Fail CLOSED on oversize input. Check byte length BEFORE heavy work. const byteLen = Buffer.byteLength(input, "utf8"); diff --git a/test/redact-engine.test.ts b/test/redact-engine.test.ts index dbbfd8a3a..1300e94cb 100644 --- a/test/redact-engine.test.ts +++ b/test/redact-engine.test.ts @@ -269,6 +269,27 @@ describe("oversize fails CLOSED", () => { expect(r.findings[0].id).toBe("engine.input_too_large"); expect(exitCodeFor(r)).toBe(3); }); + + // #1824: a malformed --max-bytes used to reach the engine as NaN. `byteLen > + // NaN` is always false, silently disabling the fail-closed guard. The engine + // guardrail must fall back to the default cap for any non-finite / <= 0 value. + test("NaN maxBytes falls back to the default cap (does NOT disable the guard)", () => { + const big = "a".repeat(2 * 1024 * 1024); // > 1 MiB default cap + const r = scan(big, { maxBytes: NaN }); + expect(r.oversize).toBe(true); + expect(r.findings[0].id).toBe("engine.input_too_large"); + expect(exitCodeFor(r)).toBe(3); + }); + + test("negative / zero maxBytes falls back to the default cap", () => { + // negative would make `byteLen > -5` always true (block everything); + // the guardrail normalizes it to the default instead. + const small = "ok"; + expect(scan(small, { maxBytes: -5 }).oversize).toBeFalsy(); + expect(scan(small, { maxBytes: 0 }).oversize).toBeFalsy(); + const big = "a".repeat(2 * 1024 * 1024); + expect(scan(big, { maxBytes: -5 }).oversize).toBe(true); + }); }); describe("validators", () => {