diff --git a/changelog.d/gst-543-redact-diff-prefix.md b/changelog.d/gst-543-redact-diff-prefix.md index 1efd77cb7..d22afeeaa 100644 --- a/changelog.d/gst-543-redact-diff-prefix.md +++ b/changelog.d/gst-543-redact-diff-prefix.md @@ -1,5 +1,5 @@ **`gstack-redact` now catches a credential on a diff line (GST-543).** The env.kv rule matched from the start of the line, so on a unified diff the leading `+` or `-` stopped the match. `git diff | gstack-redact` returned no finding and exit 0 for `+API_TOKEN=`, and for a removed line that was indented or used `export`. A `diff` code block in a PR body had the same blind spot. Pregate's `secrets` check and the pre-push hook were not affected, because both strip the `+` before scanning. ### Fixed -- **`lib/redact-patterns.ts`:** env.kv accepts one leading `+` or `-`, so the line in the example above now reports MEDIUM env.kv and exits 2. The name-shape check and the schema-field exemption accept the same marker. As a result, `API_TOKEN=changeme`, `cacheKey: …` and `input_key: …` stay clean on a diff line, as they are without the marker. An unindented YAML list item (`- api_token: `) now reports too, because it has the same shape as a removed line. +- **`lib/redact-patterns.ts`:** env.kv accepts one leading `+` or `-`, so the line in the example above now reports MEDIUM env.kv and exits 2. The name-shape check and the schema-field exemption accept the same marker. As a result, `API_TOKEN=changeme`, `cacheKey: …` and `input_key: …` stay clean on a diff line, as they are without the marker. A `-` followed by one space and text reads as a Markdown bullet or YAML list item, as before, not as a diff marker. - **`test/redact-diff-prefix.test.ts`:** each pattern in `PATTERNS` has a sample. The test requires it to report bare, behind `+`, `-` and a space, and on an added or removed line inside a real hunk, indented or not. A new pattern without a sample fails the test. The audit found env.kv was the only rule with this blind spot. Every other rule starts at a word boundary or has no anchor. diff --git a/lib/redact-patterns.ts b/lib/redact-patterns.ts index 216669967..73765a351 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]*(?: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]*["']?(?: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); @@ -966,10 +966,11 @@ export const PATTERNS: RedactPattern[] = [ // (`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 // went unreported (`-` alone matched only as a name character, never - // before indentation or `export`). 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]*(?:export[ \t]+)?["']?[A-Za-z0-9_.-]*(?:KEY|TOKEN|SECRET|PASSWORD|PASSWD|CREDENTIALS?|DSN|AUTH|COOKIE|SESSION|PRIVATE)["']?[ \t]*[:=][ \t]*["']?([^\s'"]{8,})["']?/i, + // 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, // 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 f2619ba9f..f608cd9d7 100644 --- a/test/redact-diff-prefix.test.ts +++ b/test/redact-diff-prefix.test.ts @@ -81,8 +81,10 @@ describe("GST-543: a diff line prefix never hides a finding", () => { test("env.kv reports an exported or indented credential on an added or removed line", () => { const v = SAMPLES["env.kv"]!.slice("API_TOKEN=".length); - for (const line of [`-export API_TOKEN=${v}`, `+export API_TOKEN=${v}`, `- api_token: ${v}`, `- api_token: ${v}`]) + 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`"]) + expect({ bullet, kv: ids(bullet).includes("env.kv") }).toEqual({ bullet, kv: false }); }); test("git diff | gstack-redact reports an added env.kv credential and exits 2; a placeholder on the same line stays clean", () => {