diff --git a/bin/gstack-redact-prepush b/bin/gstack-redact-prepush index fd1b05fb9..e43e2af49 100755 --- a/bin/gstack-redact-prepush +++ b/bin/gstack-redact-prepush @@ -114,6 +114,64 @@ function addedLinesFor(localSha: string, remoteSha: string): string { return added.join("\n"); } +/** + * Byte budget per scan() call. Kept comfortably under redact-engine's + * DEFAULT_MAX_BYTES (1 MiB) so a slice never trips its oversize guard. + */ +const SCAN_CHUNK_BYTES = 768 * 1024; + +/** + * Scan added lines in line-aligned slices, unioning the findings. + * + * Why: the engine refuses input over its byte cap and fails closed, which is + * right for one scan() call but wrong as a push policy — a feature branch + * catching up to a busy main legitimately produces more added lines than the + * cap (1,146,782 bytes against the 1 MiB default in the push that prompted + * this, and only ~7% of that was the lockfile). The push then blocked on + * `engine.input_too_large` — a size error naming no credential — which trains + * people to reach for --no-verify, defeating the guardrail far more thoroughly + * than a large diff does. + * + * Slicing loses NO detection coverage, because every pattern is single-line: + * none in redact-patterns.ts carries the `m` or `s` flag, the + * BEGIN-PRIVATE-KEY patterns capture only the header line rather than the key + * body, and the engine itself iterates line by line. A line boundary therefore + * cannot bisect a detectable secret, so no inter-slice overlap is needed. + * + * Fail-closed is preserved: a SINGLE line over the budget is still passed to + * the engine intact, so a genuinely unscannable blob (minified bundle, + * embedded base64) trips input_too_large and blocks exactly as before. + * + * Findings' line/col are slice-relative, which is fine here — this hook only + * reads severity, id and preview. Do not lift this into the engine, where + * callers rely on absolute line numbers. + */ +function scanAddedLines(added: string, opts: Parameters[1]): Finding[] { + const findings: Finding[] = []; + let slice: string[] = []; + let sliceBytes = 0; + + const flush = () => { + if (slice.length === 0) return; + findings.push(...scan(slice.join("\n"), opts).findings); + slice = []; + sliceBytes = 0; + }; + + for (const line of added.split("\n")) { + // +1 for the newline that rejoins it. + const lineBytes = Buffer.byteLength(line, "utf8") + 1; + // Close the current slice BEFORE overflowing it. A single oversized line + // lands in a slice of its own and is handed to the engine as-is. + if (sliceBytes > 0 && sliceBytes + lineBytes > SCAN_CHUNK_BYTES) flush(); + slice.push(line); + sliceBytes += lineBytes; + } + flush(); + + return findings; +} + function logSkip(reason: string): void { try { const home = process.env.GSTACK_HOME || path.join(os.homedir(), ".gstack"); @@ -165,8 +223,9 @@ function main() { if (!added.trim()) continue; // Visibility doesn't change HIGH behavior; pass private so nothing is treated // as public-strict (HIGH blocks regardless either way). - const result = scan(added, { repoVisibility: "private" }); - for (const f of result.findings) { + // Sliced (see scanAddedLines) so a large-but-legitimate diff is actually + // scanned rather than blocked unscanned on the engine's size cap. + for (const f of scanAddedLines(added, { repoVisibility: "private" })) { if (f.severity === "HIGH") allHigh.push(f); else if (f.severity === "MEDIUM") mediumCount++; } diff --git a/lib/redact-patterns.ts b/lib/redact-patterns.ts index 76b81f3d2..5c68e2b7a 100644 --- a/lib/redact-patterns.ts +++ b/lib/redact-patterns.ts @@ -174,6 +174,53 @@ export function isPlaceholderSpan(span: string): boolean { return false; } +/** Canonical 8-4-4-4-12 hex UUID. Global: a line may hold several. */ +const UUID_RE = /[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}/g; + +/** How far either side of a span to look for an enclosing UUID. A UUID is 36 + * chars, so 40 covers one that starts immediately before the span. Bounded so + * this stays cheap on a multi-megabyte buffer. */ +const UUID_CONTEXT_CHARS = 40; + +/** + * True when the matched span sits ENTIRELY inside a UUID. + * + * Digit-only UUIDs — `00000000-0000-0000-0000-000000000000`, + * `11111111-1111-…` — are the standard fixture shape in test suites, and their + * digit runs collide with both the credit-card and phone patterns: a 16-digit + * slice of one is Luhn-valid often enough to matter, and the hyphen groups read + * as national phone formatting. Observed live: 14 of 21 MEDIUM findings on one + * ordinary branch were this, all from test files. That volume is what stops + * people reading MEDIUM output at all, so it costs real detection elsewhere. + * + * Containment must be TOTAL, deliberately. A span merely adjacent to or + * overlapping a UUID still reports — suppression is the exception, so it may + * only fire when the whole match is demonstrably UUID interior. + * + * Takes the match (not just the span) because the decision needs surrounding + * context; span offset is derived exactly as redact-engine.ts derives it, so + * the two cannot disagree about where the span begins. + */ +export function insideUuid(match: RegExpExecArray): boolean { + const input = match.input ?? ""; + // Mirror the engine: capture group 1 when present, else the whole match. + const spanStartInMatch = match[1] !== undefined ? match[0].indexOf(match[1]) : 0; + const spanStart = match.index + Math.max(0, spanStartInMatch); + const spanEnd = spanStart + (match[1] ?? match[0]).length; + + const from = Math.max(0, spanStart - UUID_CONTEXT_CHARS); + const window = input.slice(from, spanEnd + UUID_CONTEXT_CHARS); + + UUID_RE.lastIndex = 0; + let u: RegExpExecArray | null; + while ((u = UUID_RE.exec(window)) !== null) { + const uuidStart = from + u.index; + const uuidEnd = uuidStart + u[0].length; + if (spanStart >= uuidStart && spanEnd <= uuidEnd) return true; + } + return false; +} + // ── The taxonomy ───────────────────────────────────────────────────────────── export const PATTERNS: RedactPattern[] = [ @@ -431,7 +478,8 @@ export const PATTERNS: RedactPattern[] = [ regex: /(?", - validate: (span) => span.replace(/\D/g, "").length >= 10, + // A digit-only UUID's hyphen groups read as national phone formatting. + validate: (span, match) => !insideUuid(match) && span.replace(/\D/g, "").length >= 10, }, { id: "pii.ssn", @@ -455,7 +503,9 @@ export const PATTERNS: RedactPattern[] = [ regex: /\b((?:\d[ \-]?){13,19})\b/, autoRedactable: true, redactToken: "", - validate: (span) => luhnValid(span), + // A 13-19 digit slice of a digit-only UUID passes Luhn often enough to + // matter; the enclosing-UUID check runs first so it never reaches Luhn. + validate: (span, match) => !insideUuid(match) && luhnValid(span), }, { id: "pii.ip_public", diff --git a/test/redact-engine.test.ts b/test/redact-engine.test.ts index b52c630d5..eebf584ce 100644 --- a/test/redact-engine.test.ts +++ b/test/redact-engine.test.ts @@ -201,6 +201,34 @@ describe("PII patterns", () => { expect(ids("local 192.168.1.5")).not.toContain("pii.ip_public"); expect(ids("local 10.0.0.1")).not.toContain("pii.ip_public"); }); + + // Digit-only UUIDs are the standard test-fixture shape, and their digit runs + // collide with both the card pattern (a 13-19 digit slice passes Luhn often + // enough to matter) and the phone pattern (hyphen groups read as national + // formatting). Observed live: 14 of 21 MEDIUM findings on one ordinary branch + // were exactly this, all from test files — the volume that makes people stop + // reading MEDIUM output at all. + test("digit-only UUID fixtures are not cards or phones", () => { + expect(ids("owner_user_id: '00000000-0000-0000-0000-000000000000'")).not.toContain("pii.cc"); + expect(ids("const OWNER = '11111111-1111-1111-1111-111111111111'")).not.toContain( + "pii.phone.e164", + ); + expect(ids("const TEAM = '22222222-2222-2222-2222-222222222222'")).not.toContain( + "pii.phone.e164", + ); + // Hex UUIDs never matched these digit patterns; pinned so the suppression + // is not silently widened to something that swallows real numbers. + expect(ids("id 'a1b2c3d4-1111-2222-3333-444455556666'")).not.toContain("pii.cc"); + }); + + test("UUID suppression requires TOTAL containment", () => { + // Real card sitting next to a UUID still reports — suppression is the + // exception and may only fire when the whole match is UUID interior. + expect(ids("00000000-0000-0000-0000-000000000000 4111111111111111")).toContain("pii.cc"); + // And the plain cases are untouched. + expect(ids("card 4111-1111-1111-1111")).toContain("pii.cc"); + expect(ids("reach me on +1 415 555 2671")).toContain("pii.phone.e164"); + }); }); describe("internal + legal patterns", () => {