fix(redact): reject malformed --max-bytes instead of silently disabling the size guard (#1824)

The oversize check is designed to fail CLOSED, but a malformed --max-bytes turned
it fail-OPEN. bin/gstack-redact did parseInt(maxBytes,10) and passed it straight
through; parseInt("foo") is NaN. The engine guarded with `opts.maxBytes ?? DEFAULT`,
and ?? does not catch NaN, so `byteLen > NaN` was always false and the fail-closed
block never fired. A negative value made `byteLen > -5` always true, blocking
everything.

Two layers:
- bin/gstack-redact validates the RAW string (parseInt accepts "123abc"->123,
  "1.5"->1): require /^\d+$/ and > 0, else exit 1 with a clear message.
- lib/redact-engine.ts hardens the fallback to Number.isFinite && > 0 else the
  default cap — a guardrail so the engine never silently runs uncapped even if a
  bad value reaches it directly.

Tests: NaN and negative both fall back to the default cap (oversize still blocks);
CLI rejects garbage/negative with exit 1.

Reported by @jbetala7.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Garry Tan
2026-06-07 22:48:55 -07:00
parent 7f9a9a9dff
commit be3d4c7171
3 changed files with 45 additions and 2 deletions
+14 -1
View File
@@ -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 } : {}),
};
}