mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-20 03:42:24 +02:00
fix(redact): scan large diffs in line-aligned slices; stop digit-UUIDs matching as cards/phones
The prepush guard blocked any push whose added lines exceeded the engine's 1 MiB cap with engine.input_too_large — a size error naming no credential — which trains people onto GSTACK_REDACT_PREPUSH=skip. Scan in 768 KiB line-aligned slices instead (no pattern is multi-line, so a boundary cannot bisect a secret); a single oversized line still goes to the engine intact and fails closed. Also suppress card/phone matches whose span sits ENTIRELY inside a UUID — digit-only UUID fixtures were 14 of 21 MEDIUM findings on an ordinary branch, the noise level that stops people reading MEDIUM at all. Fixes #2304. Contributed by @luckywenapere (PR #2543). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
3fa7739689
commit
d410142c2f
@@ -114,6 +114,64 @@ function addedLinesFor(localSha: string, remoteSha: string): string {
|
|||||||
return added.join("\n");
|
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<typeof scan>[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 {
|
function logSkip(reason: string): void {
|
||||||
try {
|
try {
|
||||||
const home = process.env.GSTACK_HOME || path.join(os.homedir(), ".gstack");
|
const home = process.env.GSTACK_HOME || path.join(os.homedir(), ".gstack");
|
||||||
@@ -165,8 +223,9 @@ function main() {
|
|||||||
if (!added.trim()) continue;
|
if (!added.trim()) continue;
|
||||||
// Visibility doesn't change HIGH behavior; pass private so nothing is treated
|
// Visibility doesn't change HIGH behavior; pass private so nothing is treated
|
||||||
// as public-strict (HIGH blocks regardless either way).
|
// as public-strict (HIGH blocks regardless either way).
|
||||||
const result = scan(added, { repoVisibility: "private" });
|
// Sliced (see scanAddedLines) so a large-but-legitimate diff is actually
|
||||||
for (const f of result.findings) {
|
// 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);
|
if (f.severity === "HIGH") allHigh.push(f);
|
||||||
else if (f.severity === "MEDIUM") mediumCount++;
|
else if (f.severity === "MEDIUM") mediumCount++;
|
||||||
}
|
}
|
||||||
|
|||||||
+52
-2
@@ -174,6 +174,53 @@ export function isPlaceholderSpan(span: string): boolean {
|
|||||||
return false;
|
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 ─────────────────────────────────────────────────────────────
|
// ── The taxonomy ─────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
export const PATTERNS: RedactPattern[] = [
|
export const PATTERNS: RedactPattern[] = [
|
||||||
@@ -431,7 +478,8 @@ export const PATTERNS: RedactPattern[] = [
|
|||||||
regex: /(?<![\w.])(\+?[1-9]\d{0,2}[ \-.]?\(?\d{2,4}\)?[ \-.]?\d{3,4}[ \-.]?\d{3,4})(?![\w.])/,
|
regex: /(?<![\w.])(\+?[1-9]\d{0,2}[ \-.]?\(?\d{2,4}\)?[ \-.]?\d{3,4}[ \-.]?\d{3,4})(?![\w.])/,
|
||||||
autoRedactable: true,
|
autoRedactable: true,
|
||||||
redactToken: "<REDACTED-PHONE>",
|
redactToken: "<REDACTED-PHONE>",
|
||||||
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",
|
id: "pii.ssn",
|
||||||
@@ -455,7 +503,9 @@ export const PATTERNS: RedactPattern[] = [
|
|||||||
regex: /\b((?:\d[ \-]?){13,19})\b/,
|
regex: /\b((?:\d[ \-]?){13,19})\b/,
|
||||||
autoRedactable: true,
|
autoRedactable: true,
|
||||||
redactToken: "<REDACTED-CC>",
|
redactToken: "<REDACTED-CC>",
|
||||||
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",
|
id: "pii.ip_public",
|
||||||
|
|||||||
@@ -201,6 +201,34 @@ describe("PII patterns", () => {
|
|||||||
expect(ids("local 192.168.1.5")).not.toContain("pii.ip_public");
|
expect(ids("local 192.168.1.5")).not.toContain("pii.ip_public");
|
||||||
expect(ids("local 10.0.0.1")).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", () => {
|
describe("internal + legal patterns", () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user