From 43d82e3541915ab1f287b392aa186a75d8e44bc4 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 31 Oct 2026 18:33:38 +0000 Subject: [PATCH] fix: pre-landing review fixes (combined-diff marker columns) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- lib/redact-patterns.ts | 11 ++++++----- test/redact-diff-prefix.test.ts | 6 +++--- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/redact-patterns.ts b/lib/redact-patterns.ts index 73765a351..bdc76a8ff 100644 --- a/lib/redact-patterns.ts +++ b/lib/redact-patterns.ts @@ -121,7 +121,7 @@ export function shannonEntropy(s: string): number { // (apiKey, authToken, clientSecret, stripeApiKey) — cacheKey/sortKey/ // monkey have no credential prefix and are rejected. const ENV_KV_NAME = - /^(?:\+|-(?! [^ \t]))?[ \t]*(?:export[ \t]+)?["']?([A-Za-z0-9_.-]*?(?:KEY|TOKEN|SECRET|PASSWORD|PASSWD|CREDENTIALS?|DSN|AUTH|COOKIE|SESSION|PRIVATE))["']?[ \t]*[:=]/i; + /^(?:[ +-]?\+|[ +-]?-(?! [^ \t]))?[ \t]*(?:export[ \t]+)?["']?([A-Za-z0-9_.-]*?(?:KEY|TOKEN|SECRET|PASSWORD|PASSWD|CREDENTIALS?|DSN|AUTH|COOKIE|SESSION|PRIVATE))["']?[ \t]*[:=]/i; const ENV_KV_SUFFIX = /(KEY|TOKEN|SECRET|PASSWORD|PASSWD|CREDENTIALS?|DSN|AUTH|COOKIE|SESSION|PRIVATE)$/i; const ENV_KV_CRED_PREFIX = @@ -656,7 +656,7 @@ const CALL_SHAPED_VALUE = /^[A-Za-z_$][\w$]*(?:\.[A-Za-z_$][\w$]*)*\(/; * A random key is mixed-case or one unbroken run, so it never takes this shape. */ const SCHEMA_FIELD_NAME = - /^(?:\+|-(?! [^ \t]))?[ \t]*["']?(?:input|output|field|column|lookup|cache|sort|partition|range|index|row|group|join|property|metric|catalog|schema|translation|idempotency|dedup)[_.-]key["']?[ \t]*[:=]/; + /^(?:[ +-]?\+|[ +-]?-(?! [^ \t]))?[ \t]*["']?(?:input|output|field|column|lookup|cache|sort|partition|range|index|row|group|join|property|metric|catalog|schema|translation|idempotency|dedup)[_.-]key["']?[ \t]*[:=]/; const SCHEMA_FIELD_VALUE = /^[a-z][a-z0-9]*(?:[_.-][a-z][a-z0-9]*)+$/; function isSchemaFieldIdentifier(span: string, match: RegExpExecArray): boolean { return SCHEMA_FIELD_NAME.test(match[0]) && SCHEMA_FIELD_VALUE.test(span); @@ -963,14 +963,15 @@ export const PATTERNS: RedactPattern[] = [ // values. The value must stay capture group 1 (the engine masks group 1), // so name-shape checking lives in validate, not in a second group. // GST-543: an optional leading `+` or `-` is a unified-diff line marker - // (`git diff | gstack-redact`, a diff fence in a PR body); without it the - // line anchor stopped at the marker, so an added or removed credential + // (`git diff | gstack-redact`, a diff fence in a PR body), and one more + // column before it (`++`, ` +`, `+-`) is a combined (merge) diff. Without + // them the line anchor stopped at the marker, so an added or removed credential // went unreported (`-` alone matched only as a name character, never // before indentation or `export`). A `-` followed by exactly one space // and text is a Markdown bullet or YAML list item, not a marker, so those // read as they did before. ENV_KV_NAME and SCHEMA_FIELD_NAME take the same // marker so the name-shape and schema exemptions hold on a diff line. - regex: /^(?:\+|-(?! [^ \t]))?[ \t]*(?:export[ \t]+)?["']?[A-Za-z0-9_.-]*(?:KEY|TOKEN|SECRET|PASSWORD|PASSWD|CREDENTIALS?|DSN|AUTH|COOKIE|SESSION|PRIVATE)["']?[ \t]*[:=][ \t]*["']?([^\s'"]{8,})["']?/i, + regex: /^(?:[ +-]?\+|[ +-]?-(?! [^ \t]))?[ \t]*(?:export[ \t]+)?["']?[A-Za-z0-9_.-]*(?:KEY|TOKEN|SECRET|PASSWORD|PASSWD|CREDENTIALS?|DSN|AUTH|COOKIE|SESSION|PRIVATE)["']?[ \t]*[:=][ \t]*["']?([^\s'"]{8,})["']?/i, // Only fire on credential-shaped names with high-entropy values — kills // `FOO_KEY=changeme` and `cacheKey: ` FPs. #2912: a value // that is exactly an environment read (`os.environ["X"]`, diff --git a/test/redact-diff-prefix.test.ts b/test/redact-diff-prefix.test.ts index f608cd9d7..5e4c25212 100644 --- a/test/redact-diff-prefix.test.ts +++ b/test/redact-diff-prefix.test.ts @@ -73,7 +73,7 @@ describe("GST-543: a diff line prefix never hides a finding", () => { const missed: string[] = []; for (const p of PATTERNS) { const s = SAMPLES[p.id]!; - for (const [label, text] of [["+", "+" + s], ["-", "-" + s], ["space", " " + s], ["hunk", hunk("+" + s)], ["indented hunk", hunk("+ " + s)], ["indented removed", hunk("- " + s)], ["tab removed", hunk("-\t" + s)]] as const) + for (const [label, text] of [["+", "+" + s], ["-", "-" + s], ["space", " " + s], ["hunk", hunk("+" + s)], ["indented hunk", hunk("+ " + s)], ["indented removed", hunk("- " + s)], ["tab removed", hunk("-\t" + s)], ["combined ++", hunk("++" + s)], ["combined space+", hunk(" + " + s)], ["combined +-", hunk("+-" + s)]] as const) if (!ids(text).includes(p.id)) missed.push(`${p.id} (${label})`); } expect(missed).toEqual([]); @@ -83,7 +83,7 @@ describe("GST-543: a diff line prefix never hides a finding", () => { const v = SAMPLES["env.kv"]!.slice("API_TOKEN=".length); for (const line of [`-export API_TOKEN=${v}`, `+export API_TOKEN=${v}`, `- api_token: ${v}`]) expect({ line, kv: ids(hunk(line)).includes("env.kv") }).toEqual({ line, kv: true }); - for (const bullet of ["- Credentials: `.auth.json`, `auth-token.json`", "- Session: `GSTACK_SESSION_KIND=unattended`"]) + for (const bullet of ["- Credentials: `.auth.json`, `auth-token.json`", "- Session: `GSTACK_SESSION_KIND=unattended`", " - Session: `GSTACK_SESSION_KIND=unattended`", "-- Session: `GSTACK_SESSION_KIND=unattended`"]) expect({ bullet, kv: ids(bullet).includes("env.kv") }).toEqual({ bullet, kv: false }); }); @@ -97,7 +97,7 @@ describe("GST-543: a diff line prefix never hides a finding", () => { test("the prefix changes nothing else: env.kv's exemptions hold on an added line", () => { for (const line of ["API_TOKEN=changeme", "cacheKey: 8Fk2pQ9vXz4wL7mN3rT6yB1cD5eG0hJ", "input_key: expected_media_rate_bits_per_second", 'api_key=os.environ["API_KEY"]', "TOKEN=${API_TOKEN}"]) { expect({ line, bare: ids(line).includes("env.kv") }).toEqual({ line, bare: false }); - for (const marker of ["+", "-", "+ ", "- "]) expect({ line: marker + line, kv: ids(hunk(marker + line)).includes("env.kv") }).toEqual({ line: marker + line, kv: false }); + for (const marker of ["+", "-", "+ ", "- ", "++", " +", "+-"]) expect({ line: marker + line, kv: ids(hunk(marker + line)).includes("env.kv") }).toEqual({ line: marker + line, kv: false }); } }); });