From 1bf8f3409359a433d8f42efe200eb3b003ecf01e Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Wed, 5 Aug 2026 10:57:02 +0900 Subject: [PATCH 01/15] fix(redact): close the Bearer smuggling hole in the colon rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-review of #1038. Exempting the bare scheme word from the colon rule meant a credential only had to be prefixed with `Bearer` to pass through untouched, because the dedicated Bearer rule matches a single opaque `[A-Za-z0-9._~+/=-]{8,}` token and nothing else. Anything it could not parse survived: - `x-api-key: Bearer "smuggledcredential123456"` (quoted) - `Authorization: Bearer custom:credential123456` (punctuation) - `x-api-key: Bearer short` (under the length floor) All three were reachable through formatErrorBody on the sidecar bridge. The exemption now matches only the SANITIZED result — `Bearer [REDACTED]` — so a value the Bearer rule could not sanitize is masked whole by the colon rule. The readable case is unchanged: `Authorization: Bearer ` still renders as `Bearer [REDACTED]` with trailing diagnostics intact. --- src/lib/redact.ts | 13 ++++++++----- tests/redact.test.ts | 13 +++++++++++++ 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/lib/redact.ts b/src/lib/redact.ts index 185acc3f8..1baf76738 100644 --- a/src/lib/redact.ts +++ b/src/lib/redact.ts @@ -32,11 +32,14 @@ const SECRET_VALUE_PATTERNS: Array<[RegExp, string]> = [ // scheme (Basic, Digest, …) carries its credential as the payload, so those // are masked whole by this rule. // - // The rules run in order, so by the time this one fires the Bearer rule has - // already replaced `Bearer ` with `Bearer [REDACTED]`. Skipping a value - // that is already redacted keeps this rule from eating that result — and from - // eating the trailing diagnostics after it. - [/\b((?:x-api-key|x-goog-api-key|x-amz-security-token|api[_-]?key|apiKey|access[_-]?token|accessToken|refresh[_-]?token|refreshToken|id[_-]?token|client[_-]?secret|clientSecret|authorization|proxy-authorization|cookie|set-cookie|password|secret|token)\s*:)(?![^\S\r\n]*(?:Bearer\b|\[REDACTED\]|\r?\n|$))([^\S\r\n]*)[^\r\n]+/gi, `$1$2${REDACTED_SECRET}`], + // The exemption is for the SANITIZED result only — `Bearer [REDACTED]` — + // never a raw `Bearer …` value. Exempting the bare scheme word let a + // credential be smuggled past this rule simply by prefixing it: the Bearer + // rule above only matches an opaque `[A-Za-z0-9._~+/=-]{8,}` token, so + // `x-api-key: Bearer "quoted…"`, `Authorization: Bearer custom:cred…`, and + // a short token all slipped through untouched. Anything the Bearer rule + // could not sanitize is therefore masked whole here. + [/\b((?:x-api-key|x-goog-api-key|x-amz-security-token|api[_-]?key|apiKey|access[_-]?token|accessToken|refresh[_-]?token|refreshToken|id[_-]?token|client[_-]?secret|clientSecret|authorization|proxy-authorization|cookie|set-cookie|password|secret|token)\s*:)(?![^\S\r\n]*(?:Bearer[^\S\r\n]+\[REDACTED\]|\[REDACTED\])(?![^\s.,;)\]]))(?![^\S\r\n]*(?:\r?\n|$))([^\S\r\n]*)[^\r\n]+/gi, `$1$2${REDACTED_SECRET}`], [/((?:"(?:api[_-]?key|access[_-]?token|refresh[_-]?token|id[_-]?token|client[_-]?secret|refreshToken|accessToken|clientSecret|apiKey)"\s*:\s*"))([^"]+)(")/gi, `$1${REDACTED_SECRET}$3`], // Raw JSON "token" field values (Copilot token exchange bodies echo the credential here). [/(("token"\s*:\s*"))([^"]+)(")/gi, `$1${REDACTED_SECRET}$4`], diff --git a/tests/redact.test.ts b/tests/redact.test.ts index 1a299be8c..2d10134b0 100644 --- a/tests/redact.test.ts +++ b/tests/redact.test.ts @@ -79,6 +79,19 @@ describe("redactSecretString", () => { .toBe(`Authorization: Bearer ${REDACTED_SECRET}`); }); + test("the Bearer carve-out cannot be used to smuggle a credential", () => { + // Re-review: exempting the bare scheme word meant anything the Bearer rule + // could not parse (quoted, punctuation-bearing, or under 8 chars) passed + // through untouched — a credential just had to be prefixed with "Bearer". + // Only the SANITIZED result is exempt now. + expect(redactSecretString('x-api-key: Bearer "smuggledcredential123456"')) + .toBe(`x-api-key: ${REDACTED_SECRET}`); + expect(redactSecretString("Authorization: Bearer custom:credential123456")) + .toBe(`Authorization: ${REDACTED_SECRET}`); + expect(redactSecretString("x-api-key: Bearer short")) + .toBe(`x-api-key: ${REDACTED_SECRET}`); + }); + test("masks each credential line independently without eating the next", () => { // End-of-line, not end-of-string: a multi-line error body must not collapse. expect(redactSecretString("x-api-key: one-secret\nmodel: gpt-5.5\ncookie: two=secret")) From b3289a295e5370c761afce146b513092cc5ea888 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Wed, 5 Aug 2026 11:04:22 +0900 Subject: [PATCH 02/15] fix(redact): replace the layered regexes with one explicit decision MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third re-review round on the same rule. Each previous attempt failed at the same seam — a pattern reasoning about what an earlier pattern had already done: 1. Exempting the bare word `Bearer` let anything the Bearer rule could not parse escape both rules (quoted, punctuation-bearing, or short values). 2. Exempting the sanitized marker `Bearer [REDACTED]` trusted a PUBLIC string that an upstream can emit too, so a suffix appended after it rode along. 3. Splitting into two ordered patterns had the second one eat the first one's output. So the header case is now a single pass with a replacement callback, and the boundary is stated in code rather than assembled from lookaheads: the value after the colon is a credential and is masked whole; `Bearer` keeps its scheme word and exactly one token is consumed, so trailing prose such as `… at /path/file.json` stays readable; nothing in the value grants trust. Also fixes the standalone Bearer rule crossing line boundaries — `\\s+` included newlines, so a header quoted with a trailing break masked the first word of the next line. --- src/lib/redact.ts | 67 ++++++++++++++++++++++++-------------------- tests/redact.test.ts | 41 ++++++++++++++++++++++----- 2 files changed, 71 insertions(+), 37 deletions(-) diff --git a/src/lib/redact.ts b/src/lib/redact.ts index 1baf76738..83cb70e82 100644 --- a/src/lib/redact.ts +++ b/src/lib/redact.ts @@ -2,8 +2,43 @@ export const REDACTED_SECRET = "[REDACTED]"; const SENSITIVE_KEY_PATTERN = /^(?:authorization|proxy-authorization|cookie|set-cookie|set-cookie2|api[-_]?key|x-api-key|x-goog-api-key|x-amz-security-token|access[-_]?token|refresh[-_]?token|id[-_]?token|token|secret|client[-_]?secret|password|profile[-_]?arn)$/i; +/** + * Colon-labelled credential headers echoed back inside an error body + * (`x-api-key: `), which the `key=value` rules never match. + * + * This is one pass with an explicit decision rather than a stack of regexes + * that have to reason about each other's output. Three earlier attempts failed + * exactly there: exempting `Bearer` let anything the Bearer rule could not + * parse escape both rules; trusting the public `[REDACTED]` marker let a + * suffix ride along behind it; and splitting into two ordered patterns had the + * second eat the first one's result. + * + * The rule: the value after the colon is a credential and gets masked whole. + * `Bearer` is the single exception — an auth scheme is diagnostically useful, + * and its token is one opaque word — so the scheme is kept and exactly that + * word is consumed, leaving trailing prose (`… at /path/file.json`) readable. + * `[REDACTED]` is a PUBLIC string an upstream can emit too, so its presence + * never grants trust. + */ +const CREDENTIAL_HEADER_LABEL = "x-api-key|x-goog-api-key|x-amz-security-token|api[_-]?key|apiKey|access[_-]?token|accessToken|refresh[_-]?token|refreshToken|id[_-]?token|client[_-]?secret|clientSecret|authorization|proxy-authorization|cookie|set-cookie|password|secret|token"; + +const COLON_LABELLED_CREDENTIAL = new RegExp( + `\\b((?:${CREDENTIAL_HEADER_LABEL})[^\\S\\r\\n]*:[^\\S\\r\\n]*)([^\\r\\n]+)`, + "gi", +); + +function maskColonLabelledCredential(_match: string, label: string, value: string): string { + const bearer = /^(Bearer[^\S\r\n]+)(\S+)([^\r\n]*)$/i.exec(value); + // Keep the scheme word and mask only its token; the remainder is prose. + if (bearer) return `${label}${bearer[1]}${REDACTED_SECRET}${bearer[3]}`; + return `${label}${REDACTED_SECRET}`; +} + const SECRET_VALUE_PATTERNS: Array<[RegExp, string]> = [ - [/\b(Bearer)(\s+)[A-Za-z0-9._~+/=-]{8,}\b/gi, `$1$2${REDACTED_SECRET}`], + // A Bearer token outside a labelled header (prose, JSON fragments, logs). + // Horizontal whitespace only: `\s+` crossed line boundaries and masked the + // first word of the NEXT line when a header was quoted with a trailing break. + [/\b(Bearer)([^\S\r\n]+)[A-Za-z0-9._~+/=-]{8,}\b/gi, `$1$2${REDACTED_SECRET}`], [/\b(sk-[A-Za-z0-9][A-Za-z0-9._-]{6,})\b/g, REDACTED_SECRET], // GitHub tokens (classic + fine-grained + OAuth/refresh): ghp_/gho_/ghu_/ghs_/ghr_/github_pat_. [/\b(gh[pousr]_[A-Za-z0-9_]{8,}|github_pat_[A-Za-z0-9_]{20,})\b/g, REDACTED_SECRET], @@ -12,34 +47,6 @@ const SECRET_VALUE_PATTERNS: Array<[RegExp, string]> = [ // a Bearer-prefix rule alone leaves the suffix intact. [/\btid=[A-Za-z0-9-]+(?:;[A-Za-z0-9_.-]+=[^;\s"']*)+(?::[A-Za-z0-9+/=_-]+)?/g, REDACTED_SECRET], [/\b((?:api[_-]?key|access[_-]?token|refresh[_-]?token|id[_-]?token|client[_-]?secret|refreshToken|accessToken|clientSecret|apiKey)=)([^&\s"',;]+)/gi, `$1${REDACTED_SECRET}`], - // Colon-labelled credentials. Upstream error bodies quote the offending header - // or field back at us ("x-api-key: abc…"), and the `=` rules never fire for - // that shape, so the credential survived into client-visible error text. - // - // The value class deliberately runs to end-of-line rather than stopping at a - // quote, space, or semicolon. A first attempt tokenized on those characters - // and leaked every delimiter-bearing variant: `x-api-key: "quoted…"` kept the - // whole quoted secret, `Authorization: Basic dXNlcjpwYXNz` kept the payload - // after the scheme, and `Cookie: a=1; b=2` kept everything after the first - // `;`. A credential header's value IS the rest of the line, so that is what - // gets masked. - // - // `Bearer` is the one readable exception, and it is handled by the dedicated - // Bearer rule ABOVE rather than here: an auth scheme is diagnostically useful, - // and its token is a single opaque word, so consuming the rest of the line - // there would swallow trailing diagnostics that follow a quoted header in - // prose (`… Authorization: Bearer at /path/file.json`). Every other - // scheme (Basic, Digest, …) carries its credential as the payload, so those - // are masked whole by this rule. - // - // The exemption is for the SANITIZED result only — `Bearer [REDACTED]` — - // never a raw `Bearer …` value. Exempting the bare scheme word let a - // credential be smuggled past this rule simply by prefixing it: the Bearer - // rule above only matches an opaque `[A-Za-z0-9._~+/=-]{8,}` token, so - // `x-api-key: Bearer "quoted…"`, `Authorization: Bearer custom:cred…`, and - // a short token all slipped through untouched. Anything the Bearer rule - // could not sanitize is therefore masked whole here. - [/\b((?:x-api-key|x-goog-api-key|x-amz-security-token|api[_-]?key|apiKey|access[_-]?token|accessToken|refresh[_-]?token|refreshToken|id[_-]?token|client[_-]?secret|clientSecret|authorization|proxy-authorization|cookie|set-cookie|password|secret|token)\s*:)(?![^\S\r\n]*(?:Bearer[^\S\r\n]+\[REDACTED\]|\[REDACTED\])(?![^\s.,;)\]]))(?![^\S\r\n]*(?:\r?\n|$))([^\S\r\n]*)[^\r\n]+/gi, `$1$2${REDACTED_SECRET}`], [/((?:"(?:api[_-]?key|access[_-]?token|refresh[_-]?token|id[_-]?token|client[_-]?secret|refreshToken|accessToken|clientSecret|apiKey)"\s*:\s*"))([^"]+)(")/gi, `$1${REDACTED_SECRET}$3`], // Raw JSON "token" field values (Copilot token exchange bodies echo the credential here). [/(("token"\s*:\s*"))([^"]+)(")/gi, `$1${REDACTED_SECRET}$4`], @@ -59,7 +66,7 @@ function isSensitiveKey(key: string): boolean { } export function redactSecretString(value: string): string { - let redacted = value; + let redacted = value.replace(COLON_LABELLED_CREDENTIAL, maskColonLabelledCredential); for (const [pattern, replacement] of SECRET_VALUE_PATTERNS) { redacted = redacted.replace(pattern, replacement); } diff --git a/tests/redact.test.ts b/tests/redact.test.ts index 2d10134b0..bacd5537b 100644 --- a/tests/redact.test.ts +++ b/tests/redact.test.ts @@ -79,17 +79,44 @@ describe("redactSecretString", () => { .toBe(`Authorization: Bearer ${REDACTED_SECRET}`); }); - test("the Bearer carve-out cannot be used to smuggle a credential", () => { - // Re-review: exempting the bare scheme word meant anything the Bearer rule - // could not parse (quoted, punctuation-bearing, or under 8 chars) passed - // through untouched — a credential just had to be prefixed with "Bearer". - // Only the SANITIZED result is exempt now. + test("a Bearer-prefixed value cannot smuggle a credential past the header rule", () => { + // Re-review history: the colon rule first EXEMPTED `Bearer` and left it to + // a separate rule, so anything that rule could not parse escaped both — a + // quoted value, one containing punctuation, or one under the length floor. + // The scheme is now handled in the same pass, so the token after it is + // always consumed whatever its shape. expect(redactSecretString('x-api-key: Bearer "smuggledcredential123456"')) - .toBe(`x-api-key: ${REDACTED_SECRET}`); + .toBe(`x-api-key: Bearer ${REDACTED_SECRET}`); expect(redactSecretString("Authorization: Bearer custom:credential123456")) - .toBe(`Authorization: ${REDACTED_SECRET}`); + .toBe(`Authorization: Bearer ${REDACTED_SECRET}`); expect(redactSecretString("x-api-key: Bearer short")) + .toBe(`x-api-key: Bearer ${REDACTED_SECRET}`); + }); + + test("a suffix appended after the public marker is not trusted", () => { + // `[REDACTED]` is a PUBLIC string: an upstream can emit it too. Treating it + // as proof that a prefix was already sanitized let a credential ride along + // behind it. Nothing in the value grants trust now. + expect(redactSecretString("x-api-key: Bearer [REDACTED].smuggledcredential123456")) + .toBe(`x-api-key: Bearer ${REDACTED_SECRET}`); + expect(redactSecretString("x-api-key: [REDACTED],smuggledcredential123456")) .toBe(`x-api-key: ${REDACTED_SECRET}`); + expect(redactSecretString("Authorization: Bearer abcdefgh12345678,smuggledcredential123456")) + .toBe(`Authorization: Bearer ${REDACTED_SECRET}`); + }); + + test("trailing prose after a quoted Bearer header stays readable", () => { + // Error text quotes a header inside a sentence; eating the rest of the line + // would take the diagnostic with it (this broke a Vertex path marker once). + expect(redactSecretString("failed with Authorization: Bearer secret-abc123 at /Users/example/secret.json")) + .toBe(`failed with Authorization: Bearer ${REDACTED_SECRET} at /Users/example/secret.json`); + }); + + test("a Bearer token never masks across a line break", () => { + // `\s+` included newlines, so a header quoted with a trailing break masked + // the first word of the NEXT line as if it were the token. + expect(redactSecretString("Authorization: Bearer\nrequestidentifier123456 diagnostic")) + .toBe(`Authorization: ${REDACTED_SECRET}\nrequestidentifier123456 diagnostic`); }); test("masks each credential line independently without eating the next", () => { From c67bb1330577eb791558432ad48283cafc79d36b Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Wed, 5 Aug 2026 11:10:26 +0900 Subject: [PATCH 03/15] fix(redact): rescan preserved remainders and normalize colon confusables MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fourth re-review round. Two more bypasses, both in what the Bearer carve-out chose to keep: - The preserved remainder was assumed to be prose, but the INPUT controls it. `Authorization: Bearer x-api-key: ` handed the second credential back untouched, and `Authorization: Bearer Bearer ` masked the literal word and returned the real token. The remainder is now rescanned, and a repeated scheme word consumes the following token instead. - Only ASCII `:` was recognized, so `x-api-key：` and the small and vertical colon forms were never seen as headers at all. Confusables are normalized before matching. The carve-out is also scoped to `authorization` / `proxy-authorization`, where a scheme is meaningful. On `x-api-key` the word bought nothing and only gave an attacker a way to keep part of the line. The rescan is iterative with a bounded fixpoint, not recursive: a per-match recursion overflowed the stack on a line of 3000 repeated headers, which is now a regression test. --- src/lib/redact.ts | 62 ++++++++++++++++++++++++++++++++++++++++---- tests/redact.test.ts | 37 +++++++++++++++++++++++--- 2 files changed, 91 insertions(+), 8 deletions(-) diff --git a/src/lib/redact.ts b/src/lib/redact.ts index 83cb70e82..9bd2c660e 100644 --- a/src/lib/redact.ts +++ b/src/lib/redact.ts @@ -19,19 +19,71 @@ const SENSITIVE_KEY_PATTERN = /^(?:authorization|proxy-authorization|cookie|set- * word is consumed, leaving trailing prose (`… at /path/file.json`) readable. * `[REDACTED]` is a PUBLIC string an upstream can emit too, so its presence * never grants trust. + * + * The preserved remainder is RE-SCANNED, because "the rest is prose" is an + * assumption the input controls: `Authorization: Bearer x-api-key: ` + * and `Authorization: Bearer Bearer ` both hid a second credential in what + * the first pass treated as trailing text. + * + * Colon confusables are normalized for MATCHING only. A full-width `：` or a + * small/vertical form reads as a colon to a human and to whatever produced the + * error body, so accepting only ASCII `:` was a bypass, not a strictness. */ const CREDENTIAL_HEADER_LABEL = "x-api-key|x-goog-api-key|x-amz-security-token|api[_-]?key|apiKey|access[_-]?token|accessToken|refresh[_-]?token|refreshToken|id[_-]?token|client[_-]?secret|clientSecret|authorization|proxy-authorization|cookie|set-cookie|password|secret|token"; +/** Colon confusables that render as a separator: full-width, small, vertical, modifier. */ +const COLON_CONFUSABLES = /[\uFF1A\uFE55\uFE13\uA789\u02D0\u2236]/g; + const COLON_LABELLED_CREDENTIAL = new RegExp( `\\b((?:${CREDENTIAL_HEADER_LABEL})[^\\S\\r\\n]*:[^\\S\\r\\n]*)([^\\r\\n]+)`, "gi", ); function maskColonLabelledCredential(_match: string, label: string, value: string): string { - const bearer = /^(Bearer[^\S\r\n]+)(\S+)([^\r\n]*)$/i.exec(value); - // Keep the scheme word and mask only its token; the remainder is prose. - if (bearer) return `${label}${bearer[1]}${REDACTED_SECRET}${bearer[3]}`; - return `${label}${REDACTED_SECRET}`; + // The Bearer carve-out exists for `Authorization`-style headers, where the + // scheme is real and worth reading. On `x-api-key` or `cookie` the word + // carries no meaning, so honoring it there just hands an attacker a way to + // keep part of the line: `x-api-key: Bearer first ` used to survive. + const bearer = /^authorization$/i.test(label.replace(/[^\S\r\n]*:[^\S\r\n]*$/, "").trim()) + || /^proxy-authorization$/i.test(label.replace(/[^\S\r\n]*:[^\S\r\n]*$/, "").trim()) + ? /^(Bearer[^\S\r\n]+)(\S+)([^\r\n]*)$/i.exec(value) + : null; + if (!bearer) return `${label}${REDACTED_SECRET}`; + // A repeated scheme word means the NEXT token is the credential, not this + // one: `Bearer Bearer ` would otherwise mask the literal word "Bearer" + // and hand the real token back as prose. + if (/^Bearer$/i.test(bearer[2] ?? "")) return `${label}${bearer[1]}${REDACTED_SECRET}`; + // Keep the scheme word and mask its token, then scan the remainder ONE level + // for further labels. The outer match consumed the whole line, so nothing + // else will revisit this text. Depth is capped rather than recursive: a + // per-match recursion blew the stack on a line with thousands of repeated + // headers. + return `${label}${bearer[1]}${REDACTED_SECRET}${maskRemainder(bearer[3] ?? "")}`; +} + +/** Single-level rescan of text a Bearer match preserved as "prose". */ +function maskRemainder(value: string): string { + if (!value) return value; + return value.replace(COLON_LABELLED_CREDENTIAL, (_m, label: string, rest: string) => { + const nested = /^(Bearer[^\S\r\n]+)(\S+)([^\r\n]*)$/i.exec(rest); + if (nested && /^authorization$/i.test(label.replace(/[^\S\r\n]*:[^\S\r\n]*$/, "").trim())) { + return `${label}${nested[1]}${REDACTED_SECRET}`; + } + return `${label}${REDACTED_SECRET}`; + }); +} + +function maskCredentialHeaders(value: string): string { + let current = value.replace(COLON_CONFUSABLES, ":"); + // Bounded fixpoint over the whole string. Each pass masks at least one more + // credential; the bound keeps a pathological input from spinning, and the + // value patterns that run afterwards still cover anything left. + for (let pass = 0; pass < 8; pass += 1) { + const next = current.replace(COLON_LABELLED_CREDENTIAL, maskColonLabelledCredential); + if (next === current) return current; + current = next; + } + return current; } const SECRET_VALUE_PATTERNS: Array<[RegExp, string]> = [ @@ -66,7 +118,7 @@ function isSensitiveKey(key: string): boolean { } export function redactSecretString(value: string): string { - let redacted = value.replace(COLON_LABELLED_CREDENTIAL, maskColonLabelledCredential); + let redacted = maskCredentialHeaders(value); for (const [pattern, replacement] of SECRET_VALUE_PATTERNS) { redacted = redacted.replace(pattern, replacement); } diff --git a/tests/redact.test.ts b/tests/redact.test.ts index bacd5537b..c00691571 100644 --- a/tests/redact.test.ts +++ b/tests/redact.test.ts @@ -85,12 +85,17 @@ describe("redactSecretString", () => { // quoted value, one containing punctuation, or one under the length floor. // The scheme is now handled in the same pass, so the token after it is // always consumed whatever its shape. + // The Bearer carve-out is also scoped to headers where a scheme is + // meaningful; on x-api-key the word buys nothing and the value is masked + // whole, which closed `x-api-key: Bearer first `. expect(redactSecretString('x-api-key: Bearer "smuggledcredential123456"')) - .toBe(`x-api-key: Bearer ${REDACTED_SECRET}`); + .toBe(`x-api-key: ${REDACTED_SECRET}`); expect(redactSecretString("Authorization: Bearer custom:credential123456")) .toBe(`Authorization: Bearer ${REDACTED_SECRET}`); expect(redactSecretString("x-api-key: Bearer short")) - .toBe(`x-api-key: Bearer ${REDACTED_SECRET}`); + .toBe(`x-api-key: ${REDACTED_SECRET}`); + expect(redactSecretString("x-api-key: Bearer first secondsecret123456")) + .toBe(`x-api-key: ${REDACTED_SECRET}`); }); test("a suffix appended after the public marker is not trusted", () => { @@ -98,13 +103,39 @@ describe("redactSecretString", () => { // as proof that a prefix was already sanitized let a credential ride along // behind it. Nothing in the value grants trust now. expect(redactSecretString("x-api-key: Bearer [REDACTED].smuggledcredential123456")) - .toBe(`x-api-key: Bearer ${REDACTED_SECRET}`); + .toBe(`x-api-key: ${REDACTED_SECRET}`); expect(redactSecretString("x-api-key: [REDACTED],smuggledcredential123456")) .toBe(`x-api-key: ${REDACTED_SECRET}`); expect(redactSecretString("Authorization: Bearer abcdefgh12345678,smuggledcredential123456")) .toBe(`Authorization: Bearer ${REDACTED_SECRET}`); }); + test("the preserved Bearer remainder is re-scanned for further credentials", () => { + // "the rest is prose" is an assumption the INPUT controls. A second label + // or a repeated scheme word after the token is a credential, not text. + expect(redactSecretString("Authorization: Bearer firstsecret123456 x-api-key: secondsecret123456")) + .toBe(`Authorization: Bearer ${REDACTED_SECRET} x-api-key: ${REDACTED_SECRET}`); + expect(redactSecretString("Authorization: Bearer Bearer nestedcredential123456")) + .toBe(`Authorization: Bearer ${REDACTED_SECRET}`); + }); + + test("colon look-alikes do not bypass credential-label recognition", () => { + // A full-width or small-form colon reads as a separator to a human and to + // whatever produced the error body, so matching only ASCII ":" was a + // bypass rather than strictness. + for (const colon of ["\uFF1A", "\uFE55", "\uFE13"]) { + expect(redactSecretString(`x-api-key${colon}unicodesecret123456`)) + .toBe(`x-api-key:${REDACTED_SECRET}`); + } + }); + + test("a pathological repeated-header line neither overflows nor leaks", () => { + // The first rescan attempt recursed per match and blew the stack here. + const line = "Authorization: Bearer tok ".repeat(3000); + const redacted = redactSecretString(line); + expect(redacted).not.toContain("Bearer tok"); + }); + test("trailing prose after a quoted Bearer header stays readable", () => { // Error text quotes a header inside a sentence; eating the rest of the line // would take the diagnostic with it (this broke a Vertex path marker once). From 7031ce7e9b2a2cf1c707577653eb282eb0d9edcf Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Wed, 5 Aug 2026 11:17:53 +0900 Subject: [PATCH 04/15] fix(redact): drop the preserved-remainder exception entirely MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fifth review round on this rule, and the last one that should be needed — because the thing that kept breaking was the design, not the regex. Every round preserved some readable part of a credential line, and every round the reviewer found a credential hidden inside exactly that part: a quoted value, a value with punctuation, a suffix after the public [REDACTED] marker, a second label after a Bearer token, a repeated scheme word, then a third token two levels deep. Preserving attacker-controlled text next to a credential IS the bug. So the value after a credential label now runs to end-of-line, unconditionally. The only thing kept is the literal word `Bearer` on authorization-style headers — emitted by the code, never copied from the input — so a diagnostic still says which auth scheme failed. The Vertex test that relied on a trailing path marker is updated to reflect that, with a separate case proving path redaction still works where no credential precedes it. Colon confusables and invisible format characters are folded for MATCHING only, with offsets mapped back to the original string. Folding the string itself rewrote innocent text (`ratio∶1` became `ratio:1`). Eight more separator forms are covered, along with zero-width and word-joiner characters placed before the colon. --- src/lib/redact.ts | 126 +++++++++++++++++-------------- tests/google-vertex-http.test.ts | 10 +++ tests/redact.test.ts | 37 ++++++--- 3 files changed, 106 insertions(+), 67 deletions(-) diff --git a/src/lib/redact.ts b/src/lib/redact.ts index 9bd2c660e..ad5b539a1 100644 --- a/src/lib/redact.ts +++ b/src/lib/redact.ts @@ -13,77 +13,89 @@ const SENSITIVE_KEY_PATTERN = /^(?:authorization|proxy-authorization|cookie|set- * suffix ride along behind it; and splitting into two ordered patterns had the * second eat the first one's result. * - * The rule: the value after the colon is a credential and gets masked whole. - * `Bearer` is the single exception — an auth scheme is diagnostically useful, - * and its token is one opaque word — so the scheme is kept and exactly that - * word is consumed, leaving trailing prose (`… at /path/file.json`) readable. - * `[REDACTED]` is a PUBLIC string an upstream can emit too, so its presence - * never grants trust. + * The rule: the value after the label is a credential and gets masked to + * end-of-line. There is no "keep the readable part" exception, because every + * round of review found another way to hide a credential inside whatever the + * previous round chose to preserve — a second label, a repeated `Bearer` + * scheme, a third token two levels deep. Preserving attacker-controlled text + * next to a credential is the bug; the scheme word is not worth it. * - * The preserved remainder is RE-SCANNED, because "the rest is prose" is an - * assumption the input controls: `Authorization: Bearer x-api-key: ` - * and `Authorization: Bearer Bearer ` both hid a second credential in what - * the first pass treated as trailing text. + * `Bearer` survives only as a fixed prefix on `authorization` / + * `proxy-authorization`, where it says which scheme failed and carries nothing + * from the input. `[REDACTED]` is a PUBLIC string an upstream can emit too, so + * its presence never grants trust. * - * Colon confusables are normalized for MATCHING only. A full-width `：` or a - * small/vertical form reads as a colon to a human and to whatever produced the - * error body, so accepting only ASCII `:` was a bypass, not a strictness. + * The label boundary is matched over a NORMALIZED VIEW: colon confusables and + * invisible format characters are folded for matching only, with offsets mapped + * back so unrelated text keeps its original bytes. Folding the string itself + * rewrote innocent diagnostics (`ratio∶1` became `ratio:1`). */ const CREDENTIAL_HEADER_LABEL = "x-api-key|x-goog-api-key|x-amz-security-token|api[_-]?key|apiKey|access[_-]?token|accessToken|refresh[_-]?token|refreshToken|id[_-]?token|client[_-]?secret|clientSecret|authorization|proxy-authorization|cookie|set-cookie|password|secret|token"; -/** Colon confusables that render as a separator: full-width, small, vertical, modifier. */ -const COLON_CONFUSABLES = /[\uFF1A\uFE55\uFE13\uA789\u02D0\u2236]/g; +/** + * Characters that render as a colon separator. Folded to `:` in the matching + * view so a look-alike cannot hide a header from the label pattern. + */ +const COLON_CONFUSABLES = new Set([ + "\uFF1A", "\uFE55", "\uFE13", "\uA789", "\u02D0", "\u2236", + "\u205A", "\u0589", "\u1361", "\u16EC", "\u1803", "\u2982", "\u2AF6", "\uFE30", +]); + +/** Zero-width and other invisible format characters, dropped from the matching view. */ +const INVISIBLE_FORMAT = /[\u200B-\u200F\u2060-\u2064\uFEFF\u00AD]/; const COLON_LABELLED_CREDENTIAL = new RegExp( - `\\b((?:${CREDENTIAL_HEADER_LABEL})[^\\S\\r\\n]*:[^\\S\\r\\n]*)([^\\r\\n]+)`, + `\\b(?:${CREDENTIAL_HEADER_LABEL})[^\\S\\r\\n]*:`, "gi", ); -function maskColonLabelledCredential(_match: string, label: string, value: string): string { - // The Bearer carve-out exists for `Authorization`-style headers, where the - // scheme is real and worth reading. On `x-api-key` or `cookie` the word - // carries no meaning, so honoring it there just hands an attacker a way to - // keep part of the line: `x-api-key: Bearer first ` used to survive. - const bearer = /^authorization$/i.test(label.replace(/[^\S\r\n]*:[^\S\r\n]*$/, "").trim()) - || /^proxy-authorization$/i.test(label.replace(/[^\S\r\n]*:[^\S\r\n]*$/, "").trim()) - ? /^(Bearer[^\S\r\n]+)(\S+)([^\r\n]*)$/i.exec(value) - : null; - if (!bearer) return `${label}${REDACTED_SECRET}`; - // A repeated scheme word means the NEXT token is the credential, not this - // one: `Bearer Bearer ` would otherwise mask the literal word "Bearer" - // and hand the real token back as prose. - if (/^Bearer$/i.test(bearer[2] ?? "")) return `${label}${bearer[1]}${REDACTED_SECRET}`; - // Keep the scheme word and mask its token, then scan the remainder ONE level - // for further labels. The outer match consumed the whole line, so nothing - // else will revisit this text. Depth is capped rather than recursive: a - // per-match recursion blew the stack on a line with thousands of repeated - // headers. - return `${label}${bearer[1]}${REDACTED_SECRET}${maskRemainder(bearer[3] ?? "")}`; -} - -/** Single-level rescan of text a Bearer match preserved as "prose". */ -function maskRemainder(value: string): string { - if (!value) return value; - return value.replace(COLON_LABELLED_CREDENTIAL, (_m, label: string, rest: string) => { - const nested = /^(Bearer[^\S\r\n]+)(\S+)([^\r\n]*)$/i.exec(rest); - if (nested && /^authorization$/i.test(label.replace(/[^\S\r\n]*:[^\S\r\n]*$/, "").trim())) { - return `${label}${nested[1]}${REDACTED_SECRET}`; - } - return `${label}${REDACTED_SECRET}`; - }); +/** + * Build a folded copy plus an index map back to the original string, so the + * match runs on normalized text while the output keeps every byte the match did + * not cover. + */ +function foldForMatching(value: string): { folded: string; map: number[] } { + let folded = ""; + const map: number[] = []; + for (let i = 0; i < value.length; i += 1) { + const ch = value[i]!; + if (INVISIBLE_FORMAT.test(ch)) continue; + folded += COLON_CONFUSABLES.has(ch) ? ":" : ch; + map.push(i); + } + map.push(value.length); + return { folded, map }; } function maskCredentialHeaders(value: string): string { - let current = value.replace(COLON_CONFUSABLES, ":"); - // Bounded fixpoint over the whole string. Each pass masks at least one more - // credential; the bound keeps a pathological input from spinning, and the - // value patterns that run afterwards still cover anything left. - for (let pass = 0; pass < 8; pass += 1) { - const next = current.replace(COLON_LABELLED_CREDENTIAL, maskColonLabelledCredential); - if (next === current) return current; - current = next; + const { folded, map } = foldForMatching(value); + COLON_LABELLED_CREDENTIAL.lastIndex = 0; + let out = ""; + let cursor = 0; + let match: RegExpExecArray | null; + while ((match = COLON_LABELLED_CREDENTIAL.exec(folded)) !== null) { + const start = map[match.index] ?? value.length; + const afterLabel = map[match.index + match[0].length] ?? value.length; + if (start < cursor) continue; + // Everything from the separator to end-of-line is the credential. + const lineEnd = (() => { + const nl = value.slice(afterLabel).search(/[\r\n]/); + return nl === -1 ? value.length : afterLabel + nl; + })(); + const rawValue = value.slice(afterLabel, lineEnd); + if (!rawValue.trim()) continue; + // Keep the original separator spacing so a diagnostic still reads as + // `header: [REDACTED]` rather than `header:[REDACTED]`. + const gap = /^[^\S\r\n]*/.exec(rawValue)?.[0] ?? ""; + // `Bearer` is a fixed prefix, reproduced from a literal — never copied from + // the input — and only where an auth scheme is meaningful. + const label = match[0].replace(/[^\S\r\n]*:$/, "").trim(); + const isAuthHeader = /^(?:proxy-)?authorization$/i.test(label); + const prefix = isAuthHeader && /^[^\S\r\n]*Bearer[^\S\r\n]/i.test(rawValue) ? "Bearer " : ""; + out += value.slice(cursor, afterLabel) + gap + prefix + REDACTED_SECRET; + cursor = lineEnd; } - return current; + return out + value.slice(cursor); } const SECRET_VALUE_PATTERNS: Array<[RegExp, string]> = [ diff --git a/tests/google-vertex-http.test.ts b/tests/google-vertex-http.test.ts index 12926c5e4..e640f6fab 100644 --- a/tests/google-vertex-http.test.ts +++ b/tests/google-vertex-http.test.ts @@ -226,9 +226,19 @@ describe("safeVertexHttpErrorMessage classification + redaction", () => { }); test("redacts a bearer token and an absolute path in the detail", () => { + // A credential header quoted mid-sentence takes the remainder of the line + // with it: review of the credential-header rule established that anything + // after the credential is attacker-controlled and cannot be preserved. + // The scheme word still names which auth failed. const msg = safeVertexHttpErrorMessage(400, vertexError(400, "INVALID_ARGUMENT", "failed with Authorization: Bearer secret-abc123 at /Users/example/secret.json")); expect(msg).not.toContain("secret-abc123"); expect(msg).not.toContain("/Users/example/secret.json"); + expect(msg).toContain("Authorization: Bearer [REDACTED]"); + }); + + test("redacts an absolute path that is not trailing a credential", () => { + const msg = safeVertexHttpErrorMessage(400, vertexError(400, "INVALID_ARGUMENT", "failed reading /Users/example/secret.json")); + expect(msg).not.toContain("/Users/example/secret.json"); expect(msg).toContain("[REDACTED_PATH]"); }); diff --git a/tests/redact.test.ts b/tests/redact.test.ts index c00691571..b463a588f 100644 --- a/tests/redact.test.ts +++ b/tests/redact.test.ts @@ -110,23 +110,39 @@ describe("redactSecretString", () => { .toBe(`Authorization: Bearer ${REDACTED_SECRET}`); }); - test("the preserved Bearer remainder is re-scanned for further credentials", () => { - // "the rest is prose" is an assumption the INPUT controls. A second label - // or a repeated scheme word after the token is a credential, not text. + test("nothing after a credential label survives, at any nesting depth", () => { + // Four review rounds each found a new way to hide a credential inside + // whatever the previous round chose to preserve: a second label, a + // repeated scheme word, then a third token two levels deep. Preserving + // attacker-controlled text next to a credential was the bug itself. expect(redactSecretString("Authorization: Bearer firstsecret123456 x-api-key: secondsecret123456")) - .toBe(`Authorization: Bearer ${REDACTED_SECRET} x-api-key: ${REDACTED_SECRET}`); + .toBe(`Authorization: Bearer ${REDACTED_SECRET}`); expect(redactSecretString("Authorization: Bearer Bearer nestedcredential123456")) .toBe(`Authorization: Bearer ${REDACTED_SECRET}`); + expect(redactSecretString("Authorization: Bearer a123456 Bearer b123456 c123456")) + .toBe(`Authorization: Bearer ${REDACTED_SECRET}`); }); test("colon look-alikes do not bypass credential-label recognition", () => { // A full-width or small-form colon reads as a separator to a human and to // whatever produced the error body, so matching only ASCII ":" was a // bypass rather than strictness. - for (const colon of ["\uFF1A", "\uFE55", "\uFE13"]) { + // The fold is a MATCHING view: the original separator byte is preserved. + for (const colon of ["\uFF1A", "\uFE55", "\uFE13", "\u205A", "\u0589", "\u1361", "\u16EC", "\u1803"]) { expect(redactSecretString(`x-api-key${colon}unicodesecret123456`)) - .toBe(`x-api-key:${REDACTED_SECRET}`); + .toBe(`x-api-key${colon}${REDACTED_SECRET}`); } + expect(redactSecretString("x-api-key\u200B: secretcredential123456")) + .toBe(`x-api-key\u200B: ${REDACTED_SECRET}`); + expect(redactSecretString("Authorization\u2060: Basic dXNlcjpwYXNz")) + .toBe(`Authorization\u2060: ${REDACTED_SECRET}`); + }); + + test("folding never rewrites an unrelated diagnostic", () => { + // Normalizing the string itself turned `ratio∶1` into `ratio:1`. Offsets + // map back to the original bytes so untouched text is byte-identical. + const diagnostic = "model\u2236gpt-5.5 status\u205A429 ratio\u2236 1"; + expect(redactSecretString(diagnostic)).toBe(diagnostic); }); test("a pathological repeated-header line neither overflows nor leaks", () => { @@ -136,11 +152,12 @@ describe("redactSecretString", () => { expect(redacted).not.toContain("Bearer tok"); }); - test("trailing prose after a quoted Bearer header stays readable", () => { - // Error text quotes a header inside a sentence; eating the rest of the line - // would take the diagnostic with it (this broke a Vertex path marker once). + test("text before a quoted header is kept; everything after it is not", () => { + // The scheme word still says which auth failed. The trailing path is lost + // deliberately — keeping it meant keeping an attacker-controlled suffix, + // which is exactly what the earlier rounds kept getting wrong. expect(redactSecretString("failed with Authorization: Bearer secret-abc123 at /Users/example/secret.json")) - .toBe(`failed with Authorization: Bearer ${REDACTED_SECRET} at /Users/example/secret.json`); + .toBe(`failed with Authorization: Bearer ${REDACTED_SECRET}`); }); test("a Bearer token never masks across a line break", () => { From 9d0b5a9525ed469e616d72fe39a1abcf84ef56ac Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Wed, 5 Aug 2026 11:23:16 +0900 Subject: [PATCH 05/15] fix(redact): canonicalize label candidates instead of listing characters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sixth round. Two findings, both about the label rather than the value. Homoglyph and invisible-character spoofing: `x-api-kеy` with a Cyrillic e, `x-аpi-key` with a Cyrillic a, and five different default-ignorable code points inserted mid-word all hid the label from the pattern. Growing the character list again would just invite a seventh round, so the matching view now folds systematically: `\p{Default_Ignorable_Code_Point}`, `\p{Cf}`, and combining marks are dropped, NFKD collapses width and font variants, and a homoglyph table covers the cross-script look-alikes NFKD deliberately leaves alone. The offset map stays one-to-one, so output bytes are unchanged. Over-redaction: `\b` matches after `-` and `_`, so `not-authorization:` and `internal_token:` were redacted as the credential headers they merely end with. The left boundary now excludes identifier characters. Perf on the folded path: 2 MB value 98 ms, 20k repeated headers 28 ms, 5k random UTF-16 strings with no throw. --- src/lib/redact.ts | 50 ++++++++++++++++++++++++++++++++++++++++---- tests/redact.test.ts | 28 +++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 4 deletions(-) diff --git a/src/lib/redact.ts b/src/lib/redact.ts index ad5b539a1..b7b2a5376 100644 --- a/src/lib/redact.ts +++ b/src/lib/redact.ts @@ -41,11 +41,37 @@ const COLON_CONFUSABLES = new Set([ "\u205A", "\u0589", "\u1361", "\u16EC", "\u1803", "\u2982", "\u2AF6", "\uFE30", ]); -/** Zero-width and other invisible format characters, dropped from the matching view. */ -const INVISIBLE_FORMAT = /[\u200B-\u200F\u2060-\u2064\uFEFF\u00AD]/; +/** + * Characters dropped from the matching view: anything with no visible width + * that could split a label into pieces the pattern no longer recognizes. + * `\p{Default_Ignorable_Code_Point}` is the systematic answer — it covers the + * zero-width set, the bidi isolates and marks, the Mongolian vowel separator, + * and the variation selectors in one property instead of a list that review + * keeps finding another member of. `\p{Cf}` and combining marks are folded too. + */ +const INVISIBLE_FORMAT = /[\p{Default_Ignorable_Code_Point}\p{Cf}\p{Mn}\p{Me}]/u; + +/** + * Latin look-alikes for the ASCII letters that appear in credential labels. + * Cyrillic `а`/`е`, Greek `ο`, fullwidth forms and the mathematical alphabets + * all render as the label to a human, so the matching view folds them back. + * NFKD handles the width/font variants; this table covers the cross-script + * homoglyphs NFKD deliberately leaves alone. + */ +const LETTER_CONFUSABLES = new Map([ + ["\u0430", "a"], ["\u0435", "e"], ["\u043E", "o"], ["\u0440", "p"], ["\u0441", "c"], + ["\u0445", "x"], ["\u0443", "y"], ["\u04BB", "h"], ["\u0455", "s"], ["\u0456", "i"], + ["\u0458", "j"], ["\u043A", "k"], ["\u0442", "t"], ["\u0432", "b"], ["\u043C", "m"], + ["\u03B1", "a"], ["\u03BF", "o"], ["\u03C1", "p"], ["\u03BD", "v"], ["\u03BA", "k"], + ["\u0261", "g"], ["\u0131", "i"], ["\u2044", "/"], +]); +// `\b` is the wrong left boundary for a header name: it matches after a `-` or +// `_`, so `not-authorization:` and `internal_token:` were treated as the +// credential labels they merely end with. Requiring a non-identifier character +// (or start of input) keeps the match to whole field names. const COLON_LABELLED_CREDENTIAL = new RegExp( - `\\b(?:${CREDENTIAL_HEADER_LABEL})[^\\S\\r\\n]*:`, + `(? { expect(redactSecretString(diagnostic)).toBe(diagnostic); }); + test("a label disguised with homoglyphs or invisible characters is still recognized", () => { + // Review kept finding another character that splits or spoofs the label. + // The matching view now folds cross-script homoglyphs and drops every + // default-ignorable code point, rather than growing another finite list. + const disguised = [ + "x-api-k\u0435y", // Cyrillic e + "x-\u0430pi-key", // Cyrillic a + "x-api-ke\u034Fy", // combining grapheme joiner + "x-api-ke\u2066y", // bidi isolate + "x-api-ke\u2069y", // pop directional isolate + "x-api-ke\u061Cy", // arabic letter mark + "x-api-ke\u180Ey", // mongolian vowel separator + ]; + for (const label of disguised) { + expect(redactSecretString(`${label}: secretcredential123456`)) + .toBe(`${label}: ${REDACTED_SECRET}`); + } + }); + + test("a longer field name that merely ends with a credential label is untouched", () => { + // `\b` matched after `-` and `_`, so these were redacted as if they were + // the credential headers they only end with. + expect(redactSecretString("not-authorization: public-diagnostic-value")) + .toBe("not-authorization: public-diagnostic-value"); + expect(redactSecretString("internal_token: public-diagnostic-value")) + .toBe("internal_token: public-diagnostic-value"); + }); + test("a pathological repeated-header line neither overflows nor leaks", () => { // The first rescan attempt recursed per match and blew the stack here. const line = "Authorization: Bearer tok ".repeat(3000); From 0082c4b92b3c0027002929dcfe84dce83c84f60a Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Wed, 5 Aug 2026 11:30:08 +0900 Subject: [PATCH 06/15] fix(redact): fold by code point and widen the confusable table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Seventh round, two findings, both real. The fold iterated UTF-16 code UNITS, so every supplementary character arrived as two halves and neither half matched a Unicode property or normalized. That is a plain bug: `𝕩-api-key` (mathematical letter, NFKD-normalizes to x) and a U+E0100 variation selector inside a label both walked past the fold. It now iterates code points and keeps the offset map aligned per source code point. The homoglyph table missed Cyrillic ԁ and Greek ε / τ, which NFKD deliberately leaves alone. Extended to cover the credential-label alphabet across Cyrillic, Greek, Latin-extended, and Armenian. Perf after the per-code-point work: 2 MB value 149 ms, 20k repeated headers 42 ms, 5k random UTF-16 strings with no throw. Ordinary supplementary text (emoji) is byte-identical on output. --- src/lib/redact.ts | 51 +++++++++++++++++++++++++++----------------- tests/redact.test.ts | 26 ++++++++++++++++++++++ 2 files changed, 57 insertions(+), 20 deletions(-) diff --git a/src/lib/redact.ts b/src/lib/redact.ts index b7b2a5376..05b81a33a 100644 --- a/src/lib/redact.ts +++ b/src/lib/redact.ts @@ -59,11 +59,19 @@ const INVISIBLE_FORMAT = /[\p{Default_Ignorable_Code_Point}\p{Cf}\p{Mn}\p{Me}]/u * homoglyphs NFKD deliberately leaves alone. */ const LETTER_CONFUSABLES = new Map([ + // Cyrillic ["\u0430", "a"], ["\u0435", "e"], ["\u043E", "o"], ["\u0440", "p"], ["\u0441", "c"], ["\u0445", "x"], ["\u0443", "y"], ["\u04BB", "h"], ["\u0455", "s"], ["\u0456", "i"], ["\u0458", "j"], ["\u043A", "k"], ["\u0442", "t"], ["\u0432", "b"], ["\u043C", "m"], + ["\u043D", "h"], ["\u0501", "d"], ["\u0503", "g"], ["\u051B", "q"], ["\u051D", "w"], + ["\u04CF", "l"], ["\u0261", "g"], ["\u04AB", "c"], ["\u04BD", "e"], ["\u0459", "k"], + // Greek ["\u03B1", "a"], ["\u03BF", "o"], ["\u03C1", "p"], ["\u03BD", "v"], ["\u03BA", "k"], - ["\u0261", "g"], ["\u0131", "i"], ["\u2044", "/"], + ["\u03B5", "e"], ["\u03C4", "t"], ["\u03B9", "i"], ["\u03C5", "u"], ["\u03C7", "x"], + ["\u03B7", "n"], ["\u03BC", "u"], ["\u03C3", "o"], ["\u03B2", "b"], ["\u03B3", "y"], + // Latin extended / other + ["\u0131", "i"], ["\u0269", "i"], ["\u1D0F", "o"], ["\u0280", "r"], ["\u01BF", "p"], + ["\u0578", "n"], ["\u057D", "u"], ["\u0585", "o"], ["\u0581", "g"], ["\u2044", "/"], ]); // `\b` is the wrong left boundary for a header name: it matches after a `-` or @@ -83,27 +91,30 @@ const COLON_LABELLED_CREDENTIAL = new RegExp( function foldForMatching(value: string): { folded: string; map: number[] } { let folded = ""; const map: number[] = []; - for (let i = 0; i < value.length; i += 1) { - const ch = value[i]!; - if (INVISIBLE_FORMAT.test(ch)) continue; - if (COLON_CONFUSABLES.has(ch)) { - folded += ":"; - map.push(i); + // Iterate by CODE POINT, not UTF-16 code unit: a supplementary character + // (mathematical letters, variation selectors above the BMP) is two units, so + // a per-unit loop hands each half to the property tests separately and + // neither half matches anything. `𝕩-api-key` and a U+E0100 inside a label + // both walked straight past the fold that way. + let i = 0; + while (i < value.length) { + const ch = String.fromCodePoint(value.codePointAt(i)!); + const width = ch.length; + if (INVISIBLE_FORMAT.test(ch)) { + i += width; continue; } - const lower = ch.toLowerCase(); - const homoglyph = LETTER_CONFUSABLES.get(lower); - if (homoglyph) { - folded += homoglyph; - map.push(i); - continue; - } - // NFKD collapses fullwidth, circled, and mathematical letter variants onto - // their ASCII base. Only single-unit results are used so the offset map - // stays one-to-one. - const compat = ch.normalize("NFKD"); - folded += compat.length === 1 ? compat : ch; - map.push(i); + const mapped = COLON_CONFUSABLES.has(ch) + ? ":" + : LETTER_CONFUSABLES.get(ch.toLowerCase()) + // NFKD collapses fullwidth, circled, and mathematical letter variants + // onto their ASCII base. + ?? (ch.normalize("NFKD").length === 1 ? ch.normalize("NFKD") : ch); + // One folded unit per source code point keeps the offset map aligned; a + // multi-unit fold would desynchronize it, so those keep the original. + folded += mapped.length === 1 ? mapped : ch; + for (let k = 0; k < (mapped.length === 1 ? 1 : width); k += 1) map.push(i); + i += width; } map.push(value.length); return { folded, map }; diff --git a/tests/redact.test.ts b/tests/redact.test.ts index fb6aea2bb..0a6cd11f0 100644 --- a/tests/redact.test.ts +++ b/tests/redact.test.ts @@ -173,6 +173,32 @@ describe("redactSecretString", () => { .toBe("internal_token: public-diagnostic-value"); }); + test("supplementary-plane characters are canonicalized, not split", () => { + // The fold iterated UTF-16 code UNITS, so a supplementary character arrived + // as two halves and neither half matched any property test. Mathematical + // letters and high variation selectors walked straight past. + expect(redactSecretString("\u{1D569}-api-key: credentialvalue123456")) + .toBe(`\u{1D569}-api-key: ${REDACTED_SECRET}`); + expect(redactSecretString("\u{1D431}-api-key: credentialvalue123456")) + .toBe(`\u{1D431}-api-key: ${REDACTED_SECRET}`); + expect(redactSecretString("x-api-ke\u{E0100}y: credentialvalue123456")) + .toBe(`x-api-ke\u{E0100}y: ${REDACTED_SECRET}`); + }); + + test("cross-script homoglyphs NFKD leaves alone are still folded", () => { + expect(redactSecretString("passwor\u0501: credentialvalue123456")) + .toBe(`passwor\u0501: ${REDACTED_SECRET}`); + expect(redactSecretString("s\u03B5cret: credentialvalue123456")) + .toBe(`s\u03B5cret: ${REDACTED_SECRET}`); + expect(redactSecretString("\u03C4oken: credentialvalue123456")) + .toBe(`\u03C4oken: ${REDACTED_SECRET}`); + }); + + test("ordinary supplementary text survives the fold unchanged", () => { + expect(redactSecretString("emoji \u{1F600} and text stay intact")) + .toBe("emoji \u{1F600} and text stay intact"); + }); + test("a pathological repeated-header line neither overflows nor leaks", () => { // The first rescan attempt recursed per match and blew the stack here. const line = "Authorization: Bearer tok ".repeat(3000); From 0e29a1b3f64a011120318b9927cd04db34957846 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Wed, 5 Aug 2026 11:52:20 +0900 Subject: [PATCH 07/15] fix(redact): recognize quoted credential keys in serialized objects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Structural gap, and the last one of that class: a serialized headers object puts a closing quote between the field name and the colon, so the label pattern never matched it, and the pre-existing JSON rules listed only a few field names without sharing the credential-label grammar. Ordinary JSON serialization — no homoglyphs, no attacker-chosen alphabet — walked a credential straight through: request headers: {"x-api-key":""} headers={"authorization":"Basic "} headers={"cookie":"session="} The label now accepts optional surrounding quotes, so both spellings share one grammar. A quoted value is masked to its CLOSING QUOTE rather than end-of-line: running to the line end inside an object would swallow the closing brace and the sibling fields, which are not the credential and which a reader needs. Escaped quotes inside the value are handled, and multiple objects on one line each get their own mask. This predates the branch — it is a gap in the base patterns, not a regression introduced here. --- src/lib/redact.ts | 29 +++++++++++++++++++++++------ tests/redact.test.ts | 28 +++++++++++++++++++++++++++- 2 files changed, 50 insertions(+), 7 deletions(-) diff --git a/src/lib/redact.ts b/src/lib/redact.ts index 05b81a33a..918b296cb 100644 --- a/src/lib/redact.ts +++ b/src/lib/redact.ts @@ -78,8 +78,15 @@ const LETTER_CONFUSABLES = new Map([ // `_`, so `not-authorization:` and `internal_token:` were treated as the // credential labels they merely end with. Requiring a non-identifier character // (or start of input) keeps the match to whole field names. +// +// The optional quotes around the label matter: a serialized headers object +// (`{"x-api-key":""}`) puts a closing quote between the name and the +// colon, so a bare `label:` pattern never saw it. The pre-existing JSON rules +// below only listed a few field names and did not share this label grammar, +// which is how ordinary JSON serialization — no homoglyphs, no attacker +// alphabet — walked a credential straight through. const COLON_LABELLED_CREDENTIAL = new RegExp( - `(? { const nl = value.slice(afterLabel).search(/[\r\n]/); return nl === -1 ? value.length : afterLabel + nl; })(); - const rawValue = value.slice(afterLabel, lineEnd); + // A QUOTED value ends at its closing quote; everything else runs to + // end-of-line. Consuming the rest of the line inside a serialized object + // would swallow the closing brace and the sibling fields, turning a + // diagnostic into unparseable soup — and those siblings are not the + // credential. + const rest = value.slice(afterLabel, lineEnd); + const quoted = /^([^\S\r\n]*)(["'])(?:\\.|[^\\])*?\2/.exec(rest); + const valueEnd = quoted ? afterLabel + quoted[0].length : lineEnd; + const rawValue = value.slice(afterLabel, valueEnd); if (!rawValue.trim()) continue; // Keep the original separator spacing so a diagnostic still reads as // `header: [REDACTED]` rather than `header:[REDACTED]`. const gap = /^[^\S\r\n]*/.exec(rawValue)?.[0] ?? ""; + const quote = quoted ? quoted[2]! : ""; // `Bearer` is a fixed prefix, reproduced from a literal — never copied from // the input — and only where an auth scheme is meaningful. const label = match[0].replace(/[^\S\r\n]*:$/, "").trim(); const isAuthHeader = /^(?:proxy-)?authorization$/i.test(label); - const prefix = isAuthHeader && /^[^\S\r\n]*Bearer[^\S\r\n]/i.test(rawValue) ? "Bearer " : ""; - out += value.slice(cursor, afterLabel) + gap + prefix + REDACTED_SECRET; - cursor = lineEnd; + const prefix = isAuthHeader && new RegExp(`^[^\\S\\r\\n]*${quote}?Bearer[^\\S\\r\\n]`, "i").test(rawValue) + ? "Bearer " + : ""; + out += value.slice(cursor, afterLabel) + gap + quote + prefix + REDACTED_SECRET + quote; + cursor = valueEnd; } return out + value.slice(cursor); } diff --git a/tests/redact.test.ts b/tests/redact.test.ts index 0a6cd11f0..4903919c5 100644 --- a/tests/redact.test.ts +++ b/tests/redact.test.ts @@ -65,8 +65,10 @@ describe("redactSecretString", () => { // Re-review of the first fix: tokenizing the value on quotes, spaces, and // semicolons leaked every variant that contains one. A credential header's // value is the rest of the line, so that is what must be masked. + // A quoted value keeps its quotes: the mask replaces the contents, so a + // serialized field stays syntactically intact. expect(redactSecretString('x-api-key: "quotedcredential123456"')) - .toBe(`x-api-key: ${REDACTED_SECRET}`); + .toBe(`x-api-key: "${REDACTED_SECRET}"`); expect(redactSecretString("Authorization: Basic dXNlcjpwYXNz")) .toBe(`Authorization: ${REDACTED_SECRET}`); expect(redactSecretString("Cookie: session=secret-one; csrf=secret-two")) @@ -199,6 +201,30 @@ describe("redactSecretString", () => { .toBe("emoji \u{1F600} and text stay intact"); }); + test("a serialized headers object does not hide the credential", () => { + // Structural, not a confusable gap: ordinary JSON serialization puts a + // closing quote between the field name and the colon, so a bare `label:` + // pattern never saw it, and the older JSON rules listed only a few field + // names without sharing the credential-label grammar. + expect(redactSecretString('request headers: {"x-api-key":"credentialvalue123456"}')) + .toBe(`request headers: {"x-api-key":"${REDACTED_SECRET}"}`); + expect(redactSecretString('headers={"authorization":"Basic dXNlcjpwYXNz"}')) + .toBe(`headers={"authorization":"${REDACTED_SECRET}"}`); + expect(redactSecretString('headers={"cookie":"session=credentialvalue123456"}')) + .toBe(`headers={"cookie":"${REDACTED_SECRET}"}`); + expect(redactSecretString("{'x-api-key': 'credentialvalue123456'}")) + .toBe(`{'x-api-key': '${REDACTED_SECRET}'}`); + expect(redactSecretString('"x-goog-api-key" : "credentialvalue123456"')) + .toBe(`"x-goog-api-key" : "${REDACTED_SECRET}"`); + }); + + test("a quoted value stops at its closing quote, keeping the object parseable", () => { + // Running to end-of-line inside an object would swallow the closing brace + // and every sibling field — and those siblings are not the credential. + expect(redactSecretString('{"x-api-key":"secret123456","model":"gpt-5.5"}')) + .toBe(`{"x-api-key":"${REDACTED_SECRET}","model":"gpt-5.5"}`); + }); + test("a pathological repeated-header line neither overflows nor leaks", () => { // The first rescan attempt recursed per match and blew the stack here. const line = "Authorization: Bearer tok ".repeat(3000); From ea4bc7f078a688b25b40e26b0f8dfdd60a524c66 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Wed, 5 Aug 2026 12:01:43 +0900 Subject: [PATCH 08/15] fix(redact): require a structural terminator, and cover non-colon framings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two findings from the round-8 review, the first a regression I introduced. REGRESSION: masking a quoted value to its closing quote ended the mask at the first quote regardless of what followed, so `x-api-key: "decoy"` handed the credential back as a suffix. That is the smuggling shape earlier rounds closed, walking back in through a different door, and it made the branch WEAKER than dev for a case dev already handled. A closing quote now terminates the value only when a structural terminator follows (comma, closing brace or bracket, semicolon, end of line); anything else falls back to masking the whole line. FRAMINGS: an upstream error body is not always a header dump. Form-encoded `authorization=`, an XML element `…`, and a multipart part named after a credential all carried the same names past a colon-only matcher. Each now masks with the terminator its own grammar defines, so `&`-separated siblings, the closing tag, and the multipart boundary all survive. The usage-debug expectation is updated rather than worked around: a quoted credential field is now masked as a whole value, scheme word included, because inside a serialized object the scheme is part of what the upstream echoed back. The field name still survives, which is what keeps the sample readable. --- src/lib/redact.ts | 49 +++++++++++++++++++++++++++++++++++++-- tests/redact.test.ts | 30 ++++++++++++++++++++++++ tests/usage-debug.test.ts | 7 +++++- 3 files changed, 83 insertions(+), 3 deletions(-) diff --git a/src/lib/redact.ts b/src/lib/redact.ts index 918b296cb..1882479a5 100644 --- a/src/lib/redact.ts +++ b/src/lib/redact.ts @@ -90,6 +90,45 @@ const COLON_LABELLED_CREDENTIAL = new RegExp( "gi", ); +/** + * Framings other than `label: value` that carry the same credential names. + * + * An upstream error body is not always a header dump. It can echo the request + * as a form-encoded string, an XML element, or a multipart part header, and a + * colon-only matcher sees none of those. Each entry masks the value with the + * terminator its own grammar defines, so the surrounding structure survives. + */ +const OTHER_FRAMED_CREDENTIALS: Array<[RegExp, string]> = [ + // URL query / form-encoded: `authorization=` up to `&` or `;`. + [ + new RegExp(`(?value`. + [ + new RegExp(`(<[^\\S\\r\\n]*(?:${CREDENTIAL_HEADER_LABEL})[^>]*>)([^<]+)`, "gi"), + "xml", + ], + // Multipart part: `name="authorization"` followed by the blank line and body. + [ + new RegExp( + `(name=["'](?:${CREDENTIAL_HEADER_LABEL})["'][^\\r\\n]*\\r?\\n\\r?\\n)([^\\r\\n]+)`, + "gi", + ), + "multipart", + ], +]; + +function maskOtherFramings(value: string): string { + let out = value; + for (const [pattern, kind] of OTHER_FRAMED_CREDENTIALS) { + out = kind === "=" + ? out.replace(pattern, match => `${match.slice(0, match.indexOf("=") + 1)}${REDACTED_SECRET}`) + : out.replace(pattern, (_m, head: string) => `${head}${REDACTED_SECRET}`); + } + return out; +} + /** * Build a folded copy plus an index map back to the original string, so the * match runs on normalized text while the output keeps every byte the match did @@ -146,8 +185,14 @@ function maskCredentialHeaders(value: string): string { // would swallow the closing brace and the sibling fields, turning a // diagnostic into unparseable soup — and those siblings are not the // credential. + // + // The quote only ends the value when a STRUCTURAL TERMINATOR follows it. + // Otherwise `x-api-key: "decoy"` would end the mask at the decoy's + // closing quote and hand the real credential back as a suffix — the same + // smuggling shape earlier rounds closed, reintroduced through a different + // door. Anything else falls back to masking the whole line. const rest = value.slice(afterLabel, lineEnd); - const quoted = /^([^\S\r\n]*)(["'])(?:\\.|[^\\])*?\2/.exec(rest); + const quoted = /^([^\S\r\n]*)(["'])(?:\\.|[^\\])*?\2(?=[^\S\r\n]*(?:[,;)\]}]|$))/.exec(rest); const valueEnd = quoted ? afterLabel + quoted[0].length : lineEnd; const rawValue = value.slice(afterLabel, valueEnd); if (!rawValue.trim()) continue; @@ -200,7 +245,7 @@ function isSensitiveKey(key: string): boolean { } export function redactSecretString(value: string): string { - let redacted = maskCredentialHeaders(value); + let redacted = maskOtherFramings(maskCredentialHeaders(value)); for (const [pattern, replacement] of SECRET_VALUE_PATTERNS) { redacted = redacted.replace(pattern, replacement); } diff --git a/tests/redact.test.ts b/tests/redact.test.ts index 4903919c5..e27326af8 100644 --- a/tests/redact.test.ts +++ b/tests/redact.test.ts @@ -225,6 +225,36 @@ describe("redactSecretString", () => { .toBe(`{"x-api-key":"${REDACTED_SECRET}","model":"gpt-5.5"}`); }); + test("a decoy quoted value does not end the mask early", () => { + // The closing quote only terminates the value when a structural terminator + // follows. Otherwise `label: "decoy"` ended the mask at the decoy + // and handed the real credential back as a suffix — the smuggling shape + // earlier rounds closed, walking back in through a different door. + expect(redactSecretString('x-api-key: "decoy"credential-suffix-123456')) + .toBe(`x-api-key: ${REDACTED_SECRET}`); + expect(redactSecretString("x-api-key: 'decoy'credential-suffix-123456")) + .toBe(`x-api-key: ${REDACTED_SECRET}`); + expect(redactSecretString('Authorization: "decoy"Bearer realsecret123456')) + .toBe(`Authorization: ${REDACTED_SECRET}`); + }); + + test("credential names are recognized in non-colon framings", () => { + // An upstream error body is not always a header dump: it can echo the + // request form-encoded, as XML, or as a multipart part. A colon-only + // matcher sees none of those. + expect(redactSecretString("authorization=Basic%20dXNlcjpwYXNz&model=gpt-5.5")) + .toBe(`authorization=${REDACTED_SECRET}&model=gpt-5.5`); + expect(redactSecretString("secret123456")) + .toBe(`${REDACTED_SECRET}`); + expect(redactSecretString('Content-Disposition: form-data; name="authorization"\r\n\r\nBasic dXNlcjpwYXNz\r\n--boundary')) + .toBe(`Content-Disposition: form-data; name="authorization"\r\n\r\n${REDACTED_SECRET}\r\n--boundary`); + }); + + test("non-credential fields in those framings are untouched", () => { + expect(redactSecretString("model=gpt-5.5&status=429")).toBe("model=gpt-5.5&status=429"); + expect(redactSecretString("gpt-5.5")).toBe("gpt-5.5"); + }); + test("a pathological repeated-header line neither overflows nor leaks", () => { // The first rescan attempt recursed per match and blew the stack here. const line = "Authorization: Bearer tok ".repeat(3000); diff --git a/tests/usage-debug.test.ts b/tests/usage-debug.test.ts index fba52cd36..454af2e4d 100644 --- a/tests/usage-debug.test.ts +++ b/tests/usage-debug.test.ts @@ -112,7 +112,12 @@ describe("appendUsageDebug", () => { const parsed = JSON.parse(lines[0]) as { bodySample: string }; expect(parsed.bodySample).not.toContain("usage-debug-token"); expect(parsed.bodySample).not.toContain("refresh-debug-token"); - expect(parsed.bodySample).toContain("Bearer [REDACTED]"); + // A quoted credential field is masked as a whole value now, scheme word + // included: inside a serialized object the scheme is part of what the + // upstream echoed back, not a diagnostic the redactor should reconstruct. + // The field NAME still survives, which is what makes the sample readable. + expect(parsed.bodySample).not.toContain("Bearer usage-debug-token"); + expect(parsed.bodySample).toContain("authorization"); expect(parsed.bodySample).toContain("refreshToken"); }); From e56bcbc3356381f0dc48d020dd1e4eccd4ccfdd4 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Wed, 5 Aug 2026 12:08:29 +0900 Subject: [PATCH 09/15] fix(redact): decide early termination by the label, and bound the framings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 9. The regression was mine twice over, and both times for the same reason: I let a property of attacker-controlled TEXT decide when to stop redacting. First 'stop at the closing quote', then 'stop at a closing quote followed by punctuation' — and `x-api-key: "decoy",` walked through both, masking less than the rule did before quoted-key support existed. Early termination is now decided by the LABEL. A quoted label (`"x-api-key":`) is the input proving it is a serialized field, so its value is one quoted token and the siblings after it are structure worth keeping. An unquoted label is a header line, and there the value is the rest of the line — the baseline behavior, restored unconditionally. The framings are bounded properly too: - XML tag names match exactly, so `` and `` keep their values. - A credential can be identified by the tag name, by a `name`/`key`/`id` attribute, or carried in an attribute value; all three are covered, and the attribute rules run first because the element rules consume the opening tag. - Multipart is part-based rather than line-based: the mask runs from the part header through the next boundary, covering a multi-line body, a missing blank line, and an unquoted `name=`. - Quoted form values (`authorization="…"&model=…`) are covered. Monotonicity is now asserted directly: an unquoted header label masks to end of line for every decoy terminator. --- src/lib/redact.ts | 82 ++++++++++++++++++++++++++++++++++++-------- tests/redact.test.ts | 63 ++++++++++++++++++++++++++++++---- 2 files changed, 124 insertions(+), 21 deletions(-) diff --git a/src/lib/redact.ts b/src/lib/redact.ts index 1882479a5..1ca42f5ab 100644 --- a/src/lib/redact.ts +++ b/src/lib/redact.ts @@ -101,18 +101,50 @@ const COLON_LABELLED_CREDENTIAL = new RegExp( const OTHER_FRAMED_CREDENTIALS: Array<[RegExp, string]> = [ // URL query / form-encoded: `authorization=` up to `&` or `;`. [ - new RegExp(`(?value`. + // A credential carried in an XML/HTML ATTRIBUTE value rather than a body. + // Runs BEFORE the element rules: those consume the whole opening tag, so a + // credential-bearing attribute inside it would never be reached. [ - new RegExp(`(<[^\\S\\r\\n]*(?:${CREDENTIAL_HEADER_LABEL})[^>]*>)([^<]+)`, "gi"), + new RegExp( + `(<[^>]*?\\b(?:${CREDENTIAL_HEADER_LABEL})[^\\S\\r\\n]*=[^\\S\\r\\n]*)(?:"[^"]*"|'[^']*')`, + "gi", + ), + "attr", + ], + // A credential-named ELEMENT carrying its value in some other attribute: + // ``. The tag name identifies the credential, + // so every quoted attribute value on that tag is masked. + [ + new RegExp( + `(<[^\\S\\r\\n]*(?:${CREDENTIAL_HEADER_LABEL})(?=[\\s/>])[^>]*?[A-Za-z_:][\\w:.-]*[^\\S\\r\\n]*=[^\\S\\r\\n]*)(?:"[^"]*"|'[^']*')`, + "gi", + ), + "attr", + ], + // XML/HTML element whose TAG NAME is the credential. The tag name is bounded + // exactly, or `` and `` lose their values + // for merely starting with a credential word. + [ + new RegExp(`(<[^\\S\\r\\n]*(?:${CREDENTIAL_HEADER_LABEL})(?=[\\s/>])[^>]*>)([^<]*)`, "gi"), "xml", ], - // Multipart part: `name="authorization"` followed by the blank line and body. + // XML/HTML element IDENTIFIED BY an attribute: `
`. [ new RegExp( - `(name=["'](?:${CREDENTIAL_HEADER_LABEL})["'][^\\r\\n]*\\r?\\n\\r?\\n)([^\\r\\n]+)`, + `(<[^>]*\\b(?:name|key|id)=["']?(?:${CREDENTIAL_HEADER_LABEL})["']?(?=[\\s/>])[^>]*>)([^<]*)`, + "gi", + ), + "xml", + ], + // Multipart part: everything from a credential-named part header through the + // next boundary. Part-based, not line-based — a body can span lines, and the + // blank line is often missing in a malformed echo. The name may be unquoted. + [ + new RegExp( + `(name=["']?(?:${CREDENTIAL_HEADER_LABEL})["']?[^\\r\\n]*\\r?\\n(?:\\r?\\n)?)((?:(?!--)[^\\r\\n]*\\r?\\n?)+)`, "gi", ), "multipart", @@ -122,9 +154,19 @@ const OTHER_FRAMED_CREDENTIALS: Array<[RegExp, string]> = [ function maskOtherFramings(value: string): string { let out = value; for (const [pattern, kind] of OTHER_FRAMED_CREDENTIALS) { - out = kind === "=" - ? out.replace(pattern, match => `${match.slice(0, match.indexOf("=") + 1)}${REDACTED_SECRET}`) - : out.replace(pattern, (_m, head: string) => `${head}${REDACTED_SECRET}`); + if (kind === "=" || kind === "attr") { + out = out.replace(pattern, match => { + const eq = match.lastIndexOf("="); + return `${match.slice(0, eq + 1)}${REDACTED_SECRET}`; + }); + continue; + } + out = out.replace(pattern, (_m, head: string, body: string) => { + if (!body.trim()) return `${head}${body}`; + // Preserve the trailing newline so the boundary line stays on its own. + const tail = /\r?\n$/.exec(body)?.[0] ?? ""; + return `${head}${REDACTED_SECRET}${tail}`; + }); } return out; } @@ -186,13 +228,25 @@ function maskCredentialHeaders(value: string): string { // diagnostic into unparseable soup — and those siblings are not the // credential. // - // The quote only ends the value when a STRUCTURAL TERMINATOR follows it. - // Otherwise `x-api-key: "decoy"` would end the mask at the decoy's - // closing quote and hand the real credential back as a suffix — the same - // smuggling shape earlier rounds closed, reintroduced through a different - // door. Anything else falls back to masking the whole line. + // Early termination is decided by the LABEL, not by the value. + // + // Two attempts got this wrong by inspecting the value: ending at the first + // closing quote, then ending at a closing quote followed by punctuation. + // Both let `x-api-key: "decoy",` end the mask at the decoy and hand + // the real credential back as a suffix — masking LESS than the rule did + // before quoted-key support existed. A property of attacker-controlled + // text can never be the thing that stops a redaction. + // + // A QUOTED LABEL (`"x-api-key":`) is different in kind: the input has + // already proven it is a serialized field, so its value is one quoted token + // and the siblings after it are structure worth keeping. An UNQUOTED label + // is a header line, and there the value has always been the rest of the + // line — that is the baseline behavior and it stays. + const labelWasQuoted = /^["']/.test(match[0]); const rest = value.slice(afterLabel, lineEnd); - const quoted = /^([^\S\r\n]*)(["'])(?:\\.|[^\\])*?\2(?=[^\S\r\n]*(?:[,;)\]}]|$))/.exec(rest); + const quoted = labelWasQuoted + ? /^([^\S\r\n]*)(["'])(?:\\.|[^\\])*?\2/.exec(rest) + : null; const valueEnd = quoted ? afterLabel + quoted[0].length : lineEnd; const rawValue = value.slice(afterLabel, valueEnd); if (!rawValue.trim()) continue; diff --git a/tests/redact.test.ts b/tests/redact.test.ts index e27326af8..8bc0e8968 100644 --- a/tests/redact.test.ts +++ b/tests/redact.test.ts @@ -65,10 +65,10 @@ describe("redactSecretString", () => { // Re-review of the first fix: tokenizing the value on quotes, spaces, and // semicolons leaked every variant that contains one. A credential header's // value is the rest of the line, so that is what must be masked. - // A quoted value keeps its quotes: the mask replaces the contents, so a - // serialized field stays syntactically intact. + // An unquoted header LABEL always masks to end of line, quotes and all — + // only a quoted label (a proven serialized field) terminates early. expect(redactSecretString('x-api-key: "quotedcredential123456"')) - .toBe(`x-api-key: "${REDACTED_SECRET}"`); + .toBe(`x-api-key: ${REDACTED_SECRET}`); expect(redactSecretString("Authorization: Basic dXNlcjpwYXNz")) .toBe(`Authorization: ${REDACTED_SECRET}`); expect(redactSecretString("Cookie: session=secret-one; csrf=secret-two")) @@ -226,16 +226,31 @@ describe("redactSecretString", () => { }); test("a decoy quoted value does not end the mask early", () => { - // The closing quote only terminates the value when a structural terminator - // follows. Otherwise `label: "decoy"` ended the mask at the decoy - // and handed the real credential back as a suffix — the smuggling shape - // earlier rounds closed, walking back in through a different door. + // Early termination is decided by the LABEL, not the value. Two attempts + // inspected the value instead — first "stop at the closing quote", then + // "stop at a closing quote followed by punctuation" — and both let a decoy + // end the mask and hand the real credential back as a suffix, masking LESS + // than the rule did before quoted-key support existed. expect(redactSecretString('x-api-key: "decoy"credential-suffix-123456')) .toBe(`x-api-key: ${REDACTED_SECRET}`); expect(redactSecretString("x-api-key: 'decoy'credential-suffix-123456")) .toBe(`x-api-key: ${REDACTED_SECRET}`); expect(redactSecretString('Authorization: "decoy"Bearer realsecret123456')) .toBe(`Authorization: ${REDACTED_SECRET}`); + // A terminator after the decoy does not help either. + for (const terminator of [",", ";", ")", "]", "}"]) { + expect(redactSecretString(`x-api-key: "decoy"${terminator}credential-suffix-123456`)) + .toBe(`x-api-key: ${REDACTED_SECRET}`); + } + }); + + test("an unquoted header label always masks to end of line", () => { + // This is the baseline behavior and the monotonicity guarantee: quoted-key + // support must never make a plain header line mask less than it used to. + expect(redactSecretString('x-api-key: "quotedcredential123456"')) + .toBe(`x-api-key: ${REDACTED_SECRET}`); + expect(redactSecretString("x-api-key: plain secret with spaces 123456")) + .toBe(`x-api-key: ${REDACTED_SECRET}`); }); test("credential names are recognized in non-colon framings", () => { @@ -250,6 +265,40 @@ describe("redactSecretString", () => { .toBe(`Content-Disposition: form-data; name="authorization"\r\n\r\n${REDACTED_SECRET}\r\n--boundary`); }); + test("XML credentials are covered by tag name, identifying attribute, and attribute value", () => { + expect(redactSecretString('
Basic dXNlcjpwYXNz
')) + .toBe(`
${REDACTED_SECRET}
`); + expect(redactSecretString('secret123456')) + .toBe(`${REDACTED_SECRET}`); + // The credential can also ride in an attribute of a credential-named tag. + expect(redactSecretString('public-status')) + .not.toContain("dXNlcjpwYXNz"); + }); + + test("a tag that merely starts with a credential word keeps its value", () => { + // Without an exact tag-name boundary these lost their values. + expect(redactSecretString("denied")) + .toBe("denied"); + expect(redactSecretString("42")) + .toBe("42"); + }); + + test("a multipart credential part is masked through its boundary", () => { + // Line-based masking left a multi-line body and the no-blank-line shape + // partly intact; the name may also be unquoted. + expect(redactSecretString('name="authorization"\r\nBasic dXNlcjpwYXNz\r\n--boundary')) + .toBe(`name="authorization"\r\n${REDACTED_SECRET}\r\n--boundary`); + expect(redactSecretString('name="authorization"\r\n\r\ndecoy\r\ncredential-suffix-123456\r\n--boundary')) + .toBe(`name="authorization"\r\n\r\n${REDACTED_SECRET}\r\n--boundary`); + expect(redactSecretString("name=authorization\r\n\r\nBasic dXNlcjpwYXNz\r\n--boundary")) + .toBe(`name=authorization\r\n\r\n${REDACTED_SECRET}\r\n--boundary`); + }); + + test("a quoted form value is masked too", () => { + expect(redactSecretString('authorization="Basic%20dXNlcjpwYXNz"&model=gpt-5.5')) + .toBe(`authorization=${REDACTED_SECRET}&model=gpt-5.5`); + }); + test("non-credential fields in those framings are untouched", () => { expect(redactSecretString("model=gpt-5.5&status=429")).toBe("model=gpt-5.5&status=429"); expect(redactSecretString("gpt-5.5")).toBe("gpt-5.5"); From 4860f588e208a7db03a2cc358e3800003cf40092 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Wed, 5 Aug 2026 12:15:32 +0900 Subject: [PATCH 10/15] fix(redact): stop trying to stop early MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 10, and the fourth time the same mistake produced a leak — so this removes the idea rather than the instance. Every early-termination rule was a way to read attacker-controlled text and let it decide where a secret ends: stop at the first closing quote; stop at a quote followed by punctuation; stop only when the LABEL was quoted. The third still leaked on an unmatched opening quote (`"x-api-key: "decoy",`) and on a correctly quoted key whose value quote was a decoy (`{"x-api-key":"decoy"}`). A credential value now runs to end-of-line, unconditionally, in every framing: - Form values run to `&` or `;`, with no quoted-value shortcut. - Multipart consumes the remainder of the body rather than stopping at the first `--`, because the boundary token is attacker-controlled too — a body line reading `--not-the-boundary` used to end the mask. - A qualifying XML tag masks EVERY quoted attribute and its entire element content: masking one attribute left `type="Basic" value=""` leaking, and masking direct text only left a nested `` untouched. `name`/`key`/`id` must be the whole attribute name, so `data-name="authorization"` no longer eats an innocent status. The cost is real and accepted: serialized siblings after a credential are lost, so a debug body sample keeps its first field name and little else. That is the right trade — an uglier diagnostic against a redactor that cannot be talked out of redacting. --- src/lib/redact.ts | 112 ++++++++++++++++++-------------------- tests/redact.test.ts | 104 +++++++++++++++++++++++------------ tests/usage-debug.test.ts | 11 ++-- 3 files changed, 126 insertions(+), 101 deletions(-) diff --git a/src/lib/redact.ts b/src/lib/redact.ts index 1ca42f5ab..a75ef2bae 100644 --- a/src/lib/redact.ts +++ b/src/lib/redact.ts @@ -100,51 +100,44 @@ const COLON_LABELLED_CREDENTIAL = new RegExp( */ const OTHER_FRAMED_CREDENTIALS: Array<[RegExp, string]> = [ // URL query / form-encoded: `authorization=` up to `&` or `;`. + // Unconditionally to the separator — a quoted value is NOT allowed to end it + // early, or `authorization="decoy"&model=…` leaks the suffix. [ - new RegExp(`(?` loses + // harmless status text. Once a tag qualifies, EVERY quoted attribute value on + // it and its ENTIRE element content are masked: masking only the first + // attribute left `` leaking, and + // masking only direct text left `` untouched. [ new RegExp( - `(<[^>]*?\\b(?:${CREDENTIAL_HEADER_LABEL})[^\\S\\r\\n]*=[^\\S\\r\\n]*)(?:"[^"]*"|'[^']*')`, + `(<[^\\S\\r\\n]*(?:${CREDENTIAL_HEADER_LABEL})(?=[\\s/>])[^>]*>)([\\s\\S]*?)(]*>)`, "gi", ), - "attr", + "element", ], - // A credential-named ELEMENT carrying its value in some other attribute: - // ``. The tag name identifies the credential, - // so every quoted attribute value on that tag is masked. [ new RegExp( - `(<[^\\S\\r\\n]*(?:${CREDENTIAL_HEADER_LABEL})(?=[\\s/>])[^>]*?[A-Za-z_:][\\w:.-]*[^\\S\\r\\n]*=[^\\S\\r\\n]*)(?:"[^"]*"|'[^']*')`, + `(<([A-Za-z_:][\\w:.-]*)[^>]*?(?])[^>]*>)([\\s\\S]*?)(]*>)`, "gi", ), - "attr", + "named-element", ], - // XML/HTML element whose TAG NAME is the credential. The tag name is bounded - // exactly, or `` and `` lose their values - // for merely starting with a credential word. - [ - new RegExp(`(<[^\\S\\r\\n]*(?:${CREDENTIAL_HEADER_LABEL})(?=[\\s/>])[^>]*>)([^<]*)`, "gi"), - "xml", - ], - // XML/HTML element IDENTIFIED BY an attribute: `
`. + // Multipart part: everything from a credential-named part header to the end + // of the input. Part-based, not line-based — a body can span lines and the + // blank line is often missing in a malformed echo. + // + // It deliberately does NOT stop at the first `--`: the boundary token is + // attacker-controlled text, so a body line starting `--not-the-boundary` + // ended the mask and exposed everything after it. Consuming the remainder + // costs trailing context in one framing and closes the bypass. [ new RegExp( - `(<[^>]*\\b(?:name|key|id)=["']?(?:${CREDENTIAL_HEADER_LABEL})["']?(?=[\\s/>])[^>]*>)([^<]*)`, - "gi", - ), - "xml", - ], - // Multipart part: everything from a credential-named part header through the - // next boundary. Part-based, not line-based — a body can span lines, and the - // blank line is often missing in a malformed echo. The name may be unquoted. - [ - new RegExp( - `(name=["']?(?:${CREDENTIAL_HEADER_LABEL})["']?[^\\r\\n]*\\r?\\n(?:\\r?\\n)?)((?:(?!--)[^\\r\\n]*\\r?\\n?)+)`, + `(name=["']?(?:${CREDENTIAL_HEADER_LABEL})["']?[^\\r\\n]*\\r?\\n(?:\\r?\\n)?)([\\s\\S]+)`, "gi", ), "multipart", @@ -154,18 +147,27 @@ const OTHER_FRAMED_CREDENTIALS: Array<[RegExp, string]> = [ function maskOtherFramings(value: string): string { let out = value; for (const [pattern, kind] of OTHER_FRAMED_CREDENTIALS) { - if (kind === "=" || kind === "attr") { + if (kind === "=") { out = out.replace(pattern, match => { - const eq = match.lastIndexOf("="); + const eq = match.indexOf("="); return `${match.slice(0, eq + 1)}${REDACTED_SECRET}`; }); continue; } + if (kind === "element" || kind === "named-element") { + out = out.replace(pattern, (whole: string) => { + const open = /^<[^>]*>/.exec(whole)?.[0] ?? ""; + const close = /<\/[^>]*>$/.exec(whole)?.[0] ?? ""; + // Every quoted attribute on a qualifying tag is masked, not just the + // first, and the whole element content goes with it. + const safeOpen = open.replace(/=[^\S\r\n]*(?:"[^"]*"|'[^']*')/g, `=${REDACTED_SECRET}`); + return `${safeOpen}${REDACTED_SECRET}${close}`; + }); + continue; + } out = out.replace(pattern, (_m, head: string, body: string) => { if (!body.trim()) return `${head}${body}`; - // Preserve the trailing newline so the boundary line stays on its own. - const tail = /\r?\n$/.exec(body)?.[0] ?? ""; - return `${head}${REDACTED_SECRET}${tail}`; + return `${head}${REDACTED_SECRET}`; }); } return out; @@ -222,38 +224,28 @@ function maskCredentialHeaders(value: string): string { const nl = value.slice(afterLabel).search(/[\r\n]/); return nl === -1 ? value.length : afterLabel + nl; })(); - // A QUOTED value ends at its closing quote; everything else runs to - // end-of-line. Consuming the rest of the line inside a serialized object - // would swallow the closing brace and the sibling fields, turning a - // diagnostic into unparseable soup — and those siblings are not the - // credential. - // - // Early termination is decided by the LABEL, not by the value. + // THE VALUE ALWAYS RUNS TO END-OF-LINE. There is no early termination and + // no attempt to preserve sibling fields. // - // Two attempts got this wrong by inspecting the value: ending at the first - // closing quote, then ending at a closing quote followed by punctuation. - // Both let `x-api-key: "decoy",` end the mask at the decoy and hand - // the real credential back as a suffix — masking LESS than the rule did - // before quoted-key support existed. A property of attacker-controlled - // text can never be the thing that stops a redaction. + // Three attempts tried to be smarter, and each one leaked: stop at the + // first closing quote; stop at a closing quote followed by punctuation; + // stop only when the LABEL was quoted. The third still leaked on an + // unmatched opening quote (`"x-api-key: "decoy",`) and on a + // correctly quoted key whose value quote was a decoy + // (`{"x-api-key":"decoy"}`). // - // A QUOTED LABEL (`"x-api-key":`) is different in kind: the input has - // already proven it is a serialized field, so its value is one quoted token - // and the siblings after it are structure worth keeping. An UNQUOTED label - // is a header line, and there the value has always been the rest of the - // line — that is the baseline behavior and it stays. - const labelWasQuoted = /^["']/.test(match[0]); - const rest = value.slice(afterLabel, lineEnd); - const quoted = labelWasQuoted - ? /^([^\S\r\n]*)(["'])(?:\\.|[^\\])*?\2/.exec(rest) - : null; - const valueEnd = quoted ? afterLabel + quoted[0].length : lineEnd; + // The pattern is the lesson: any rule that stops early is reading + // attacker-controlled text to decide where a secret ends, and the attacker + // gets to write that text. Losing the siblings in a serialized object + // makes a diagnostic less pretty; stopping early makes it leak. Monotonic + // and blunt wins. + const valueEnd = lineEnd; const rawValue = value.slice(afterLabel, valueEnd); if (!rawValue.trim()) continue; // Keep the original separator spacing so a diagnostic still reads as // `header: [REDACTED]` rather than `header:[REDACTED]`. const gap = /^[^\S\r\n]*/.exec(rawValue)?.[0] ?? ""; - const quote = quoted ? quoted[2]! : ""; + const quote = ""; // `Bearer` is a fixed prefix, reproduced from a literal — never copied from // the input — and only where an auth scheme is meaningful. const label = match[0].replace(/[^\S\r\n]*:$/, "").trim(); diff --git a/tests/redact.test.ts b/tests/redact.test.ts index 8bc0e8968..87d269e59 100644 --- a/tests/redact.test.ts +++ b/tests/redact.test.ts @@ -206,23 +206,28 @@ describe("redactSecretString", () => { // closing quote between the field name and the colon, so a bare `label:` // pattern never saw it, and the older JSON rules listed only a few field // names without sharing the credential-label grammar. - expect(redactSecretString('request headers: {"x-api-key":"credentialvalue123456"}')) - .toBe(`request headers: {"x-api-key":"${REDACTED_SECRET}"}`); - expect(redactSecretString('headers={"authorization":"Basic dXNlcjpwYXNz"}')) - .toBe(`headers={"authorization":"${REDACTED_SECRET}"}`); - expect(redactSecretString('headers={"cookie":"session=credentialvalue123456"}')) - .toBe(`headers={"cookie":"${REDACTED_SECRET}"}`); - expect(redactSecretString("{'x-api-key': 'credentialvalue123456'}")) - .toBe(`{'x-api-key': '${REDACTED_SECRET}'}`); - expect(redactSecretString('"x-goog-api-key" : "credentialvalue123456"')) - .toBe(`"x-goog-api-key" : "${REDACTED_SECRET}"`); - }); - - test("a quoted value stops at its closing quote, keeping the object parseable", () => { - // Running to end-of-line inside an object would swallow the closing brace - // and every sibling field — and those siblings are not the credential. + for (const input of [ + 'request headers: {"x-api-key":"credentialvalue123456"}', + 'headers={"authorization":"Basic dXNlcjpwYXNz"}', + 'headers={"cookie":"session=credentialvalue123456"}', + "{'x-api-key': 'credentialvalue123456'}", + '"x-goog-api-key" : "credentialvalue123456"', + ]) { + const redacted = redactSecretString(input); + expect(redacted).toContain(REDACTED_SECRET); + expect(redacted).not.toContain("credentialvalue123456"); + expect(redacted).not.toContain("dXNlcjpwYXNz"); + } + }); + + test("the value always runs to end of line, siblings included", () => { + // Three attempts tried to stop early and keep the siblings readable — at + // the first closing quote, at a quote followed by punctuation, and only + // when the LABEL was quoted. Each leaked, because every early stop reads + // attacker-controlled text to decide where a secret ends. Losing the + // siblings makes a diagnostic uglier; stopping early makes it leak. expect(redactSecretString('{"x-api-key":"secret123456","model":"gpt-5.5"}')) - .toBe(`{"x-api-key":"${REDACTED_SECRET}","model":"gpt-5.5"}`); + .toBe(`{"x-api-key":${REDACTED_SECRET}`); }); test("a decoy quoted value does not end the mask early", () => { @@ -244,13 +249,18 @@ describe("redactSecretString", () => { } }); - test("an unquoted header label always masks to end of line", () => { - // This is the baseline behavior and the monotonicity guarantee: quoted-key - // support must never make a plain header line mask less than it used to. + test("no framing or quoting makes the rule mask less", () => { + // The monotonicity guarantee, asserted directly. expect(redactSecretString('x-api-key: "quotedcredential123456"')) .toBe(`x-api-key: ${REDACTED_SECRET}`); expect(redactSecretString("x-api-key: plain secret with spaces 123456")) .toBe(`x-api-key: ${REDACTED_SECRET}`); + // An UNMATCHED opening quote before the label is not a serialized field. + expect(redactSecretString('"x-api-key: "decoy",credential-suffix-123456')) + .toBe(`"x-api-key: ${REDACTED_SECRET}`); + // Nor is a correctly quoted key whose value quote is a decoy. + expect(redactSecretString('{"x-api-key":"decoy"credential-suffix-123456}')) + .toBe(`{"x-api-key":${REDACTED_SECRET}`); }); test("credential names are recognized in non-colon framings", () => { @@ -261,18 +271,28 @@ describe("redactSecretString", () => { .toBe(`authorization=${REDACTED_SECRET}&model=gpt-5.5`); expect(redactSecretString("secret123456")) .toBe(`${REDACTED_SECRET}`); - expect(redactSecretString('Content-Disposition: form-data; name="authorization"\r\n\r\nBasic dXNlcjpwYXNz\r\n--boundary')) - .toBe(`Content-Disposition: form-data; name="authorization"\r\n\r\n${REDACTED_SECRET}\r\n--boundary`); + const multipart = redactSecretString('Content-Disposition: form-data; name="authorization"\r\n\r\nBasic dXNlcjpwYXNz\r\n--boundary'); + expect(multipart).toContain(REDACTED_SECRET); + expect(multipart).not.toContain("dXNlcjpwYXNz"); }); test("XML credentials are covered by tag name, identifying attribute, and attribute value", () => { - expect(redactSecretString('
Basic dXNlcjpwYXNz
')) - .toBe(`
${REDACTED_SECRET}
`); - expect(redactSecretString('secret123456')) - .toBe(`${REDACTED_SECRET}`); - // The credential can also ride in an attribute of a credential-named tag. - expect(redactSecretString('public-status')) - .not.toContain("dXNlcjpwYXNz"); + // A qualifying tag masks EVERY quoted attribute and its whole content: + // masking only the first attribute left `type="Basic" value=""` + // leaking, and masking only direct text left a nested `` untouched. + for (const input of [ + '
Basic dXNlcjpwYXNz
', + 'secret123456', + 'public-status', + 'public', + '
public
', + 'Basic dXNlcjpwYXNz', + ]) { + const redacted = redactSecretString(input); + expect(redacted).toContain(REDACTED_SECRET); + expect(redacted).not.toContain("dXNlcjpwYXNz"); + expect(redacted).not.toContain("secret123456"); + } }); test("a tag that merely starts with a credential word keeps its value", () => { @@ -281,22 +301,34 @@ describe("redactSecretString", () => { .toBe("denied"); expect(redactSecretString("42")) .toBe("42"); + // A prefixed attribute does not identify a credential either. + expect(redactSecretString('public-status')) + .toBe('public-status'); }); - test("a multipart credential part is masked through its boundary", () => { + test("a multipart credential part is masked through the rest of the body", () => { // Line-based masking left a multi-line body and the no-blank-line shape - // partly intact; the name may also be unquoted. - expect(redactSecretString('name="authorization"\r\nBasic dXNlcjpwYXNz\r\n--boundary')) - .toBe(`name="authorization"\r\n${REDACTED_SECRET}\r\n--boundary`); - expect(redactSecretString('name="authorization"\r\n\r\ndecoy\r\ncredential-suffix-123456\r\n--boundary')) - .toBe(`name="authorization"\r\n\r\n${REDACTED_SECRET}\r\n--boundary`); - expect(redactSecretString("name=authorization\r\n\r\nBasic dXNlcjpwYXNz\r\n--boundary")) - .toBe(`name=authorization\r\n\r\n${REDACTED_SECRET}\r\n--boundary`); + // partly intact, and stopping at the first `--` trusted an attacker-chosen + // boundary token: a body line reading `--not-the-boundary` ended the mask. + for (const input of [ + 'name="authorization"\r\nBasic dXNlcjpwYXNz\r\n--boundary', + 'name="authorization"\r\n\r\ndecoy\r\ncredential-suffix-123456\r\n--boundary', + "name=authorization\r\n\r\nBasic dXNlcjpwYXNz\r\n--boundary", + 'name="authorization"\r\n\r\n--not-the-boundary\r\ncredential-suffix-123456\r\n--boundary', + ]) { + const redacted = redactSecretString(input); + expect(redacted).toContain(REDACTED_SECRET); + expect(redacted).not.toContain("dXNlcjpwYXNz"); + expect(redacted).not.toContain("credential-suffix-123456"); + } }); test("a quoted form value is masked too", () => { expect(redactSecretString('authorization="Basic%20dXNlcjpwYXNz"&model=gpt-5.5')) .toBe(`authorization=${REDACTED_SECRET}&model=gpt-5.5`); + // A decoy quoted value does not end it early either. + expect(redactSecretString('authorization="decoy"credential-suffix-123456&model=gpt-5.5')) + .toBe(`authorization=${REDACTED_SECRET}&model=gpt-5.5`); }); test("non-credential fields in those framings are untouched", () => { diff --git a/tests/usage-debug.test.ts b/tests/usage-debug.test.ts index 454af2e4d..e915d324c 100644 --- a/tests/usage-debug.test.ts +++ b/tests/usage-debug.test.ts @@ -112,13 +112,14 @@ describe("appendUsageDebug", () => { const parsed = JSON.parse(lines[0]) as { bodySample: string }; expect(parsed.bodySample).not.toContain("usage-debug-token"); expect(parsed.bodySample).not.toContain("refresh-debug-token"); - // A quoted credential field is masked as a whole value now, scheme word - // included: inside a serialized object the scheme is part of what the - // upstream echoed back, not a diagnostic the redactor should reconstruct. - // The field NAME still survives, which is what makes the sample readable. + // A credential value now masks to end of line, so the trailing fields of a + // serialized body go with it. That is deliberate: every attempt to stop + // early and keep the siblings readable turned out to be a way to smuggle a + // credential past the redactor. The first field name still identifies what + // the sample was, which is what a debug line actually needs. expect(parsed.bodySample).not.toContain("Bearer usage-debug-token"); expect(parsed.bodySample).toContain("authorization"); - expect(parsed.bodySample).toContain("refreshToken"); + expect(parsed.bodySample).toContain("[REDACTED]"); }); test("preserves estimated extracted usage while redacting surrounding secrets", () => { From 1ff2783b783962772e264e3db5b56816f0b970be Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Wed, 5 Aug 2026 12:19:46 +0900 Subject: [PATCH 11/15] fix(redact): apply the same blunt policy to XML MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit XML was the last framing still using a stopping point, and it failed the same way everything else did. A closing tag is attacker-controlled text: same-name nesting ended the mask at the INNER `` and exposed the outer element's remaining content, and a self-closing tag had no closing tag to find at all. Namespace-qualified names were not recognized either. A qualifying tag — credential name, optionally namespace-qualified, or a whole `name`/`key`/`id` attribute naming one — now keeps only its tag name and masks to end of line, like every other framing. `data-name` still does not qualify, and `` / `` still keep their values. The differential matrix the reviewer ran over 972 cases reports zero regression versus both baselines with early termination gone; this closes the last structural leak it found. --- src/lib/redact.ts | 37 ++++++++++++++++++------------------- tests/redact.test.ts | 16 ++++++++++++---- 2 files changed, 30 insertions(+), 23 deletions(-) diff --git a/src/lib/redact.ts b/src/lib/redact.ts index a75ef2bae..d93814490 100644 --- a/src/lib/redact.ts +++ b/src/lib/redact.ts @@ -106,26 +106,30 @@ const OTHER_FRAMED_CREDENTIALS: Array<[RegExp, string]> = [ new RegExp(`(?` loses - // harmless status text. Once a tag qualifies, EVERY quoted attribute value on - // it and its ENTIRE element content are masked: masking only the first - // attribute left `` leaking, and - // masking only direct text left `` untouched. + // XML/HTML. A tag qualifies when its NAME is a credential (optionally + // namespace-qualified), or when a whole `name`/`key`/`id` attribute names one + // — `data-name` does not count, or `` loses + // harmless status text. + // + // Once a tag qualifies, the mask keeps only the tag name and runs to END OF + // LINE. Using the CLOSING TAG as the stopping point was the same mistake as + // every other early termination: same-name nesting ended the mask at the + // inner `` and exposed the outer element's remaining content, + // and a self-closing tag had no closing tag to find at all. The closing tag + // is attacker-controlled text like everything else. [ new RegExp( - `(<[^\\S\\r\\n]*(?:${CREDENTIAL_HEADER_LABEL})(?=[\\s/>])[^>]*>)([\\s\\S]*?)(]*>)`, + `(<[^\\S\\r\\n]*(?:[A-Za-z_][\\w.-]*:)?(?:${CREDENTIAL_HEADER_LABEL})(?=[\\s/>]))[^\\r\\n]*`, "gi", ), "element", ], [ new RegExp( - `(<([A-Za-z_:][\\w:.-]*)[^>]*?(?])[^>]*>)([\\s\\S]*?)(]*>)`, + `(<[^\\S\\r\\n]*(?:[A-Za-z_][\\w.-]*:)?[A-Za-z_][\\w:.-]*)(?=[^>]*?(?]))[^\\r\\n]*`, "gi", ), - "named-element", + "element", ], // Multipart part: everything from a credential-named part header to the end // of the input. Part-based, not line-based — a body can span lines and the @@ -154,15 +158,10 @@ function maskOtherFramings(value: string): string { }); continue; } - if (kind === "element" || kind === "named-element") { - out = out.replace(pattern, (whole: string) => { - const open = /^<[^>]*>/.exec(whole)?.[0] ?? ""; - const close = /<\/[^>]*>$/.exec(whole)?.[0] ?? ""; - // Every quoted attribute on a qualifying tag is masked, not just the - // first, and the whole element content goes with it. - const safeOpen = open.replace(/=[^\S\r\n]*(?:"[^"]*"|'[^']*')/g, `=${REDACTED_SECRET}`); - return `${safeOpen}${REDACTED_SECRET}${close}`; - }); + if (kind === "element") { + // Keep the tag name so the diagnostic still says WHICH element carried a + // credential, and mask everything after it. + out = out.replace(pattern, (_m, head: string) => `${head}${REDACTED_SECRET}`); continue; } out = out.replace(pattern, (_m, head: string, body: string) => { diff --git a/tests/redact.test.ts b/tests/redact.test.ts index 87d269e59..a7b9f1cae 100644 --- a/tests/redact.test.ts +++ b/tests/redact.test.ts @@ -270,16 +270,18 @@ describe("redactSecretString", () => { expect(redactSecretString("authorization=Basic%20dXNlcjpwYXNz&model=gpt-5.5")) .toBe(`authorization=${REDACTED_SECRET}&model=gpt-5.5`); expect(redactSecretString("secret123456")) - .toBe(`${REDACTED_SECRET}`); + .toBe(` { - // A qualifying tag masks EVERY quoted attribute and its whole content: - // masking only the first attribute left `type="Basic" value=""` - // leaking, and masking only direct text left a nested `` untouched. + // A qualifying tag keeps only its NAME and masks to end of line. Using the + // closing tag as the stopping point was the same early-termination mistake + // as everywhere else: same-name nesting ended the mask at the INNER + // `` and exposed the outer element's remaining content, and + // a self-closing tag had no closing tag to find at all. for (const input of [ '
Basic dXNlcjpwYXNz
', 'secret123456', @@ -287,11 +289,17 @@ describe("redactSecretString", () => { 'public', '
public
', 'Basic dXNlcjpwYXNz', + // Self-closing, namespace-qualified, and same-name nesting. + '', + '
', + "Basic dXNlcjpwYXNz", + "decoycredential-suffix-123456", ]) { const redacted = redactSecretString(input); expect(redacted).toContain(REDACTED_SECRET); expect(redacted).not.toContain("dXNlcjpwYXNz"); expect(redacted).not.toContain("secret123456"); + expect(redacted).not.toContain("credential-suffix-123456"); } }); From e74c137f5182ec9f036a75d1954c4974652dc6f3 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Wed, 5 Aug 2026 12:23:15 +0900 Subject: [PATCH 12/15] fix(redact): XML has no line discipline to borrow either MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second stopping point removed from the XML rule. After the closing tag came end-of-line, and an opening tag may legally span lines, so `` left the credential sitting on the next line. A qualifying tag now masks through end of input, like the multipart rule. Also allows whitespace around an attribute `=` (`name = "authorization"`, `key\t=\t"x-api-key"`), which XML permits and an upstream echo may well reproduce. --- src/lib/redact.ts | 18 +++++++++++------- tests/redact.test.ts | 6 ++++++ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/lib/redact.ts b/src/lib/redact.ts index d93814490..aa0e6f00a 100644 --- a/src/lib/redact.ts +++ b/src/lib/redact.ts @@ -112,21 +112,25 @@ const OTHER_FRAMED_CREDENTIALS: Array<[RegExp, string]> = [ // harmless status text. // // Once a tag qualifies, the mask keeps only the tag name and runs to END OF - // LINE. Using the CLOSING TAG as the stopping point was the same mistake as - // every other early termination: same-name nesting ended the mask at the - // inner `` and exposed the outer element's remaining content, - // and a self-closing tag had no closing tag to find at all. The closing tag - // is attacker-controlled text like everything else. + // INPUT. Two stopping points were tried and both leaked: the CLOSING TAG + // (same-name nesting ended the mask at the inner ``, and a + // self-closing tag had none), then END OF LINE (an opening tag may legally + // span lines, so `` left the credential on the + // next line). XML has no line discipline to borrow, so there is no boundary + // left worth trusting. + // + // Whitespace is allowed around an attribute `=`, which XML permits and an + // echo may well reproduce. [ new RegExp( - `(<[^\\S\\r\\n]*(?:[A-Za-z_][\\w.-]*:)?(?:${CREDENTIAL_HEADER_LABEL})(?=[\\s/>]))[^\\r\\n]*`, + `(<[^\\S\\r\\n]*(?:[A-Za-z_][\\w.-]*:)?(?:${CREDENTIAL_HEADER_LABEL})(?=[\\s/>]))[\\s\\S]*`, "gi", ), "element", ], [ new RegExp( - `(<[^\\S\\r\\n]*(?:[A-Za-z_][\\w.-]*:)?[A-Za-z_][\\w:.-]*)(?=[^>]*?(?]))[^\\r\\n]*`, + `(<[^\\S\\r\\n]*(?:[A-Za-z_][\\w.-]*:)?[A-Za-z_][\\w:.-]*)(?=[^>]*?(?]))[\\s\\S]*`, "gi", ), "element", diff --git a/tests/redact.test.ts b/tests/redact.test.ts index a7b9f1cae..7e6e038ca 100644 --- a/tests/redact.test.ts +++ b/tests/redact.test.ts @@ -294,6 +294,12 @@ describe("redactSecretString", () => { '
', "Basic dXNlcjpwYXNz", "decoycredential-suffix-123456", + // An opening tag may legally span lines, and XML allows whitespace + // around an attribute `=`. End-of-line was the second stopping point + // that leaked here, after the closing tag. + '\npublic-status\n', + '
Basic dXNlcjpwYXNz
', + 'secret123456', ]) { const redacted = redactSecretString(input); expect(redacted).toContain(REDACTED_SECRET); From 5a44743cf321bb3650ff2c12a26696e2a0748737 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Wed, 5 Aug 2026 12:27:44 +0900 Subject: [PATCH 13/15] fix(redact): decode serialization escapes into the matching view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A JSON `\u0069`, a percent-encoded `%69`, and an XML `i` all spell the credential field name to whatever parses the body, and spell something else to a literal matcher. `{"author\u0069zation":""}`, `author%69zation=`, and `
` were therefore invisible — structural aliases, not confusable-table coverage. The fold now decodes all three, one folded character per escape with the whole escape mapped back to its start, so the offset map still writes the mask at the right place in the original bytes. The form and XML rules run over that folded view too, rather than the raw text, which is what let the percent and character reference forms through. Cost: the fold runs once per framing pass, so a 2 MB value goes from ~150 ms to ~340 ms. Still linear, still stack-safe, and this path only runs on error bodies. --- src/lib/redact.ts | 86 +++++++++++++++++++++++++++++++++----------- tests/redact.test.ts | 18 ++++++++++ 2 files changed, 83 insertions(+), 21 deletions(-) diff --git a/src/lib/redact.ts b/src/lib/redact.ts index aa0e6f00a..bbdce7290 100644 --- a/src/lib/redact.ts +++ b/src/lib/redact.ts @@ -152,28 +152,46 @@ const OTHER_FRAMED_CREDENTIALS: Array<[RegExp, string]> = [ ], ]; +/** + * These run over the FOLDED view too, then map back to the original string. + * + * Matching the raw text meant a percent-encoded form key (`author%69zation=`) + * and an XML character reference (`name="authorization"`) were invisible, + * even though both spell the credential name to anything that parses the body. + * The fold already decodes those, so the grammars are applied there and the + * mask is written back at the corresponding original offsets. + */ function maskOtherFramings(value: string): string { - let out = value; + let current = value; for (const [pattern, kind] of OTHER_FRAMED_CREDENTIALS) { - if (kind === "=") { - out = out.replace(pattern, match => { - const eq = match.indexOf("="); - return `${match.slice(0, eq + 1)}${REDACTED_SECRET}`; - }); - continue; + const { folded, map } = foldForMatching(current); + pattern.lastIndex = 0; + let out = ""; + let cursor = 0; + let match: RegExpExecArray | null; + while ((match = pattern.exec(folded)) !== null) { + const start = map[match.index] ?? current.length; + const end = map[match.index + match[0].length] ?? current.length; + if (start < cursor) continue; + const head = (() => { + if (kind === "=") { + const eq = match[0].indexOf("="); + const headEnd = map[match.index + eq + 1] ?? end; + return current.slice(start, headEnd); + } + const captured = match[1] ?? ""; + const headEnd = map[match.index + captured.length] ?? end; + return current.slice(start, headEnd); + })(); + const body = current.slice(start + head.length, end); + if (kind === "multipart" && !body.trim()) continue; + out += current.slice(cursor, start) + head + REDACTED_SECRET; + cursor = end; + if (pattern.lastIndex === match.index) pattern.lastIndex += 1; } - if (kind === "element") { - // Keep the tag name so the diagnostic still says WHICH element carried a - // credential, and mask everything after it. - out = out.replace(pattern, (_m, head: string) => `${head}${REDACTED_SECRET}`); - continue; - } - out = out.replace(pattern, (_m, head: string, body: string) => { - if (!body.trim()) return `${head}${body}`; - return `${head}${REDACTED_SECRET}`; - }); + current = out + current.slice(cursor); } - return out; + return current; } /** @@ -184,6 +202,30 @@ function maskOtherFramings(value: string): string { function foldForMatching(value: string): { folded: string; map: number[] } { let folded = ""; const map: number[] = []; + // Serialization escapes are ALIASES for the label, not decoration: a JSON + // `\u0069`, a percent-encoded `%69`, and an XML `i` all spell the same + // field name to whatever parses the body, while spelling something else to a + // literal matcher. Decode them into the matching view (one folded character + // per escape, with the whole escape mapped back to its start) so + // `author\u0069zation`, `author%69zation`, and `authorization` are the + // label they claim to be. + const decodeEscape = (at: number): { ch: string; width: number } | null => { + const json = /^\\u([0-9a-fA-F]{4})/.exec(value.slice(at, at + 6)); + if (json) return { ch: String.fromCharCode(parseInt(json[1]!, 16)), width: 6 }; + const pct = /^%([0-9a-fA-F]{2})/.exec(value.slice(at, at + 3)); + if (pct) return { ch: String.fromCharCode(parseInt(pct[1]!, 16)), width: 3 }; + const xml = /^&#(x[0-9a-fA-F]{1,6}|[0-9]{1,7});/.exec(value.slice(at, at + 11)); + if (xml) { + const raw = xml[1]!; + const code = raw[0] === "x" || raw[0] === "X" + ? parseInt(raw.slice(1), 16) + : parseInt(raw, 10); + if (Number.isFinite(code) && code > 0 && code <= 0x10ffff) { + return { ch: String.fromCodePoint(code), width: xml[0].length }; + } + } + return null; + }; // Iterate by CODE POINT, not UTF-16 code unit: a supplementary character // (mathematical letters, variation selectors above the BMP) is two units, so // a per-unit loop hands each half to the property tests separately and @@ -191,8 +233,9 @@ function foldForMatching(value: string): { folded: string; map: number[] } { // both walked straight past the fold that way. let i = 0; while (i < value.length) { - const ch = String.fromCodePoint(value.codePointAt(i)!); - const width = ch.length; + const escaped = decodeEscape(i); + const ch = escaped ? escaped.ch : String.fromCodePoint(value.codePointAt(i)!); + const width = escaped ? escaped.width : ch.length; if (INVISIBLE_FORMAT.test(ch)) { i += width; continue; @@ -206,7 +249,8 @@ function foldForMatching(value: string): { folded: string; map: number[] } { // One folded unit per source code point keeps the offset map aligned; a // multi-unit fold would desynchronize it, so those keep the original. folded += mapped.length === 1 ? mapped : ch; - for (let k = 0; k < (mapped.length === 1 ? 1 : width); k += 1) map.push(i); + const emitted = mapped.length === 1 ? 1 : (escaped ? 1 : width); + for (let k = 0; k < emitted; k += 1) map.push(i); i += width; } map.push(value.length); diff --git a/tests/redact.test.ts b/tests/redact.test.ts index 7e6e038ca..44dd531a4 100644 --- a/tests/redact.test.ts +++ b/tests/redact.test.ts @@ -345,6 +345,24 @@ describe("redactSecretString", () => { .toBe(`authorization=${REDACTED_SECRET}&model=gpt-5.5`); }); + test("serialization escapes are aliases for the label, not a disguise", () => { + // A JSON `\u0069`, a percent-encoded `%69`, and an XML `i` all spell + // the credential name to whatever parses the body, while spelling + // something else to a literal matcher. The matching view decodes them. + for (const input of [ + '{"author\\u0069zation":"opaquecredential123456"}', + "author%69zation=opaquecredential123456&model=gpt-5.5", + '
opaquecredential123456
', + '{"x-api-\\u006bey":"opaquecredential123456"}', + "x%2Dapi%2Dkey=opaquecredential123456&model=gpt-5.5", + '
opaquecredential123456
', + ]) { + const redacted = redactSecretString(input); + expect(redacted).toContain(REDACTED_SECRET); + expect(redacted).not.toContain("opaquecredential123456"); + } + }); + test("non-credential fields in those framings are untouched", () => { expect(redactSecretString("model=gpt-5.5&status=429")).toBe("model=gpt-5.5&status=429"); expect(redactSecretString("gpt-5.5")).toBe("gpt-5.5"); From a1bdb6539c1ec2b6a4a67237dab621be1cf6e289 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Wed, 5 Aug 2026 12:34:24 +0900 Subject: [PATCH 14/15] fix(redact): make decoding one-way, and fix the supplementary offset map MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Decoding introduced a regression, which is exactly the failure mode this rule keeps hitting: a change that masks MORE in one shape and LESS in another. `𝕩x-api-key: ` decoded to a mathematical letter that folds to `x`, which moved the following label's left boundary and suppressed a match both baselines made. So decoding is now one-way by construction: the header and framing passes run over BOTH matching views — decoded and plain — and mask whatever either finds. Decoding can add coverage; it cannot take any away. The offset map also allocated one entry per source code point rather than per EMITTED UTF-16 unit, so an escaped supplementary character desynchronized every later offset and the mask landed mid-token (`😀authorization=o[REDACTED]model=…`). Entries are per emitted unit now. HTML named entities are decoded as well: `:` is the separator itself, and the Greek names decode to characters the homoglyph fold already handles, so `authorιzation` resolves to the label. Perf on this path is fine — the reviewer confirmed error bodies are capped at 64 KiB, where a full pass is ~35-39 ms. --- src/lib/redact.ts | 59 +++++++++++++++++++++++++++++++++++++++----- tests/redact.test.ts | 23 +++++++++++++++++ 2 files changed, 76 insertions(+), 6 deletions(-) diff --git a/src/lib/redact.ts b/src/lib/redact.ts index bbdce7290..325738c23 100644 --- a/src/lib/redact.ts +++ b/src/lib/redact.ts @@ -51,6 +51,21 @@ const COLON_CONFUSABLES = new Set([ */ const INVISIBLE_FORMAT = /[\p{Default_Ignorable_Code_Point}\p{Cf}\p{Mn}\p{Me}]/u; +/** + * HTML named entities that can spell a credential label or its separator. + * The Greek names decode to characters the homoglyph fold already handles, so + * decoding them here is what connects the two: `ι` is `ι` is `i`. + */ +const HTML_NAMED_ENTITIES = new Map([ + ["colon", ":"], ["semi", ";"], ["equals", "="], ["quot", '"'], ["apos", "'"], + ["lt", "<"], ["gt", ">"], ["amp", "&"], ["sol", "/"], ["lowbar", "_"], + ["hyphen", "-"], ["dash", "-"], ["ndash", "-"], ["period", "."], ["comma", ","], + ["iota", "\u03B9"], ["alpha", "\u03B1"], ["omicron", "\u03BF"], ["rho", "\u03C1"], + ["epsilon", "\u03B5"], ["tau", "\u03C4"], ["kappa", "\u03BA"], ["nu", "\u03BD"], + ["upsilon", "\u03C5"], ["chi", "\u03C7"], ["eta", "\u03B7"], ["mu", "\u03BC"], + ["beta", "\u03B2"], ["gamma", "\u03B3"], ["sigma", "\u03C3"], +]); + /** * Latin look-alikes for the ASCII letters that appear in credential labels. * Cyrillic `а`/`е`, Greek `ο`, fullwidth forms and the mathematical alphabets @@ -162,9 +177,15 @@ const OTHER_FRAMED_CREDENTIALS: Array<[RegExp, string]> = [ * mask is written back at the corresponding original offsets. */ function maskOtherFramings(value: string): string { + // Same union rule as the header pass: decoding may add coverage, never + // remove it. + return maskOtherFramingsOnce(maskOtherFramingsOnce(value, true), false); +} + +function maskOtherFramingsOnce(value: string, decodeEscapes: boolean): string { let current = value; for (const [pattern, kind] of OTHER_FRAMED_CREDENTIALS) { - const { folded, map } = foldForMatching(current); + const { folded, map } = foldForMatching(current, decodeEscapes); pattern.lastIndex = 0; let out = ""; let cursor = 0; @@ -199,7 +220,7 @@ function maskOtherFramings(value: string): string { * match runs on normalized text while the output keeps every byte the match did * not cover. */ -function foldForMatching(value: string): { folded: string; map: number[] } { +function foldForMatching(value: string, decodeEscapes = true): { folded: string; map: number[] } { let folded = ""; const map: number[] = []; // Serialization escapes are ALIASES for the label, not decoration: a JSON @@ -224,6 +245,14 @@ function foldForMatching(value: string): { folded: string; map: number[] } { return { ch: String.fromCodePoint(code), width: xml[0].length }; } } + // HTML named entities. Only the ones that can spell a credential label or + // its separator matter here — `:` is the separator itself, and the + // Greek names decode to characters the homoglyph fold already covers. + const named = /^&([A-Za-z][A-Za-z0-9]{1,31});/.exec(value.slice(at, at + 34)); + if (named) { + const decoded = HTML_NAMED_ENTITIES.get(named[1]!.toLowerCase()); + if (decoded) return { ch: decoded, width: named[0].length }; + } return null; }; // Iterate by CODE POINT, not UTF-16 code unit: a supplementary character @@ -233,7 +262,7 @@ function foldForMatching(value: string): { folded: string; map: number[] } { // both walked straight past the fold that way. let i = 0; while (i < value.length) { - const escaped = decodeEscape(i); + const escaped = decodeEscapes ? decodeEscape(i) : null; const ch = escaped ? escaped.ch : String.fromCodePoint(value.codePointAt(i)!); const width = escaped ? escaped.width : ch.length; if (INVISIBLE_FORMAT.test(ch)) { @@ -249,16 +278,34 @@ function foldForMatching(value: string): { folded: string; map: number[] } { // One folded unit per source code point keeps the offset map aligned; a // multi-unit fold would desynchronize it, so those keep the original. folded += mapped.length === 1 ? mapped : ch; - const emitted = mapped.length === 1 ? 1 : (escaped ? 1 : width); - for (let k = 0; k < emitted; k += 1) map.push(i); + // One map entry per EMITTED UTF-16 unit. An escaped supplementary + // character emits two units, and giving it one entry desynchronized every + // later offset — the mask then landed mid-token and left part of the + // credential behind. + const emittedText = mapped.length === 1 ? mapped : ch; + for (let k = 0; k < emittedText.length; k += 1) map.push(i); i += width; } map.push(value.length); return { folded, map }; } +/** + * Run the header rule over BOTH matching views and take the union. + * + * Decoding may only ADD coverage. Applying it unconditionally removed some: + * `𝕩x-api-key: ` decoded to `𝕩x-api-key:`, which folds to + * `xx-api-key:` and no longer matches the label boundary — so a decode-only + * view masked LESS than the plain view did. Running both and masking whatever + * either one finds makes the direction of the change one-way. + */ function maskCredentialHeaders(value: string): string { - const { folded, map } = foldForMatching(value); + const decoded = maskCredentialHeadersOnce(value, true); + return maskCredentialHeadersOnce(decoded, false); +} + +function maskCredentialHeadersOnce(value: string, decodeEscapes: boolean): string { + const { folded, map } = foldForMatching(value, decodeEscapes); COLON_LABELLED_CREDENTIAL.lastIndex = 0; let out = ""; let cursor = 0; diff --git a/tests/redact.test.ts b/tests/redact.test.ts index 44dd531a4..3bab75ecc 100644 --- a/tests/redact.test.ts +++ b/tests/redact.test.ts @@ -363,6 +363,29 @@ describe("redactSecretString", () => { } }); + test("decoding may add coverage but never remove it", () => { + // Decoding `𝕩` yields a mathematical letter that folds to `x`, + // which changed the FOLLOWING label's left boundary and suppressed a match + // the plain view made. The rule now runs over both views and masks what + // either one finds, so the change is one-way by construction. + expect(redactSecretString("𝕩x-api-key: opaquecredential123456")) + .toBe(`𝕩x-api-key: ${REDACTED_SECRET}`); + // An escaped supplementary character emits two UTF-16 units; giving it one + // offset entry desynchronized the map and left part of the credential. + expect(redactSecretString("😀authorization=opaquecredential123456&model=gpt-5.5")) + .toBe(`😀authorization=${REDACTED_SECRET}&model=gpt-5.5`); + }); + + test("HTML named entities are decoded too", () => { + // `:` IS the separator, and `ι` decodes to a character the + // homoglyph fold already covers — decoding is what connects the two. + expect(redactSecretString("x-api-key: opaquecredential123456")) + .toBe(`x-api-key: ${REDACTED_SECRET}`); + const xml = redactSecretString('
opaquecredential123456
'); + expect(xml).toContain(REDACTED_SECRET); + expect(xml).not.toContain("opaquecredential123456"); + }); + test("non-credential fields in those framings are untouched", () => { expect(redactSecretString("model=gpt-5.5&status=429")).toBe("model=gpt-5.5&status=429"); expect(redactSecretString("gpt-5.5")).toBe("gpt-5.5"); From 7336b54e24b2cda633eeff3e8c09863f1fbde468 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Wed, 5 Aug 2026 12:41:21 +0900 Subject: [PATCH 15/15] fix(redact): decode multi-unit escapes, and stop promising an entity table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two more alias classes, both structural. Multi-unit escapes were decoded a unit at a time. A JSON surrogate PAIR is one code point, so decoding the halves separately left two lone surrogates that normalize to nothing; percent encoding is UTF-8, so `%D0%B5` is one Cyrillic character, not two Latin-1 ones. Both now decode as single characters. HTML named references are handled by giving up on naming them. A hand-picked list is a coverage promise nobody can keep — review found `ⅈ`, `ⅇ`, and `ⅆ` decoding to compatibility letters NFKD already maps, and the WHATWG table holds ~2200 entries that neither Bun nor Node exposes. An unresolved name now folds to a placeholder that the label grammar accepts wherever a letter may appear, so every named entity is covered without pretending to know what any of them mean. Only the separator names (`:` and friends) resolve exactly, since a separator is structure rather than part of the name. --- src/lib/redact.ts | 87 +++++++++++++++++++++++++++++++++++--------- tests/redact.test.ts | 27 ++++++++++++++ 2 files changed, 96 insertions(+), 18 deletions(-) diff --git a/src/lib/redact.ts b/src/lib/redact.ts index 325738c23..45a468b41 100644 --- a/src/lib/redact.ts +++ b/src/lib/redact.ts @@ -30,7 +30,13 @@ const SENSITIVE_KEY_PATTERN = /^(?:authorization|proxy-authorization|cookie|set- * back so unrelated text keeps its original bytes. Folding the string itself * rewrote innocent diagnostics (`ratio∶1` became `ratio:1`). */ -const CREDENTIAL_HEADER_LABEL = "x-api-key|x-goog-api-key|x-amz-security-token|api[_-]?key|apiKey|access[_-]?token|accessToken|refresh[_-]?token|refreshToken|id[_-]?token|client[_-]?secret|clientSecret|authorization|proxy-authorization|cookie|set-cookie|password|secret|token"; +// Every letter position also accepts \u0001, the placeholder the fold emits for +// an unresolved HTML named reference: `authorⅈzation` is the label with one +// character we cannot name, and that is still the label. +const CREDENTIAL_HEADER_LABEL_RAW = "x-api-key|x-goog-api-key|x-amz-security-token|api[_-]?key|apiKey|access[_-]?token|accessToken|refresh[_-]?token|refreshToken|id[_-]?token|client[_-]?secret|clientSecret|authorization|proxy-authorization|cookie|set-cookie|password|secret|token"; + +const CREDENTIAL_HEADER_LABEL = CREDENTIAL_HEADER_LABEL_RAW + .replace(/(?([ +const SEPARATOR_ENTITIES = new Map([ ["colon", ":"], ["semi", ";"], ["equals", "="], ["quot", '"'], ["apos", "'"], ["lt", "<"], ["gt", ">"], ["amp", "&"], ["sol", "/"], ["lowbar", "_"], - ["hyphen", "-"], ["dash", "-"], ["ndash", "-"], ["period", "."], ["comma", ","], - ["iota", "\u03B9"], ["alpha", "\u03B1"], ["omicron", "\u03BF"], ["rho", "\u03C1"], - ["epsilon", "\u03B5"], ["tau", "\u03C4"], ["kappa", "\u03BA"], ["nu", "\u03BD"], - ["upsilon", "\u03C5"], ["chi", "\u03C7"], ["eta", "\u03B7"], ["mu", "\u03BC"], - ["beta", "\u03B2"], ["gamma", "\u03B3"], ["sigma", "\u03C3"], + ["hyphen", "-"], ["dash", "-"], ["ndash", "-"], ["mdash", "-"], ["minus", "-"], + ["period", "."], ["comma", ","], ["num", "#"], ["nbsp", " "], ]); /** @@ -231,10 +251,41 @@ function foldForMatching(value: string, decodeEscapes = true): { folded: string; // `author\u0069zation`, `author%69zation`, and `authorization` are the // label they claim to be. const decodeEscape = (at: number): { ch: string; width: number } | null => { + // JSON `\uXXXX`, INCLUDING a surrogate pair. Decoding the halves + // independently left `\uD835\uDD69` as two lone surrogates, so the + // mathematical letter they spell was never normalized as one code point. const json = /^\\u([0-9a-fA-F]{4})/.exec(value.slice(at, at + 6)); - if (json) return { ch: String.fromCharCode(parseInt(json[1]!, 16)), width: 6 }; - const pct = /^%([0-9a-fA-F]{2})/.exec(value.slice(at, at + 3)); - if (pct) return { ch: String.fromCharCode(parseInt(pct[1]!, 16)), width: 3 }; + if (json) { + const high = parseInt(json[1]!, 16); + if (high >= 0xd800 && high <= 0xdbff) { + const low = /^\\u([0-9a-fA-F]{4})/.exec(value.slice(at + 6, at + 12)); + const lowCode = low ? parseInt(low[1]!, 16) : NaN; + if (lowCode >= 0xdc00 && lowCode <= 0xdfff) { + return { ch: String.fromCharCode(high, lowCode), width: 12 }; + } + } + return { ch: String.fromCharCode(high), width: 6 }; + } + // Percent encoding is UTF-8: consecutive `%XX` bytes form ONE character. + // Decoding each byte on its own turned `%D0%B5` into two unrelated + // Latin-1 characters instead of the Cyrillic `е` the fold would have + // recognized. + const pct = /^(?:%[0-9a-fA-F]{2})+/.exec(value.slice(at, at + 24)); + if (pct) { + try { + const decoded = decodeURIComponent(pct[0]); + if (decoded.length >= 1) { + // Consume only the bytes that produced the FIRST character, so the + // rest of the sequence is decoded on the next iteration. + const first = String.fromCodePoint(decoded.codePointAt(0)!); + const bytes = new TextEncoder().encode(first).length; + return { ch: first, width: bytes * 3 }; + } + } catch { + const single = parseInt(pct[0].slice(1, 3), 16); + return { ch: String.fromCharCode(single), width: 3 }; + } + } const xml = /^&#(x[0-9a-fA-F]{1,6}|[0-9]{1,7});/.exec(value.slice(at, at + 11)); if (xml) { const raw = xml[1]!; @@ -245,13 +296,13 @@ function foldForMatching(value: string, decodeEscapes = true): { folded: string; return { ch: String.fromCodePoint(code), width: xml[0].length }; } } - // HTML named entities. Only the ones that can spell a credential label or - // its separator matter here — `:` is the separator itself, and the - // Greek names decode to characters the homoglyph fold already covers. + // HTML named references. `:` and the other separator names are + // resolved exactly; anything else folds to the opaque placeholder so the + // label still matches without pretending to know the character. const named = /^&([A-Za-z][A-Za-z0-9]{1,31});/.exec(value.slice(at, at + 34)); if (named) { - const decoded = HTML_NAMED_ENTITIES.get(named[1]!.toLowerCase()); - if (decoded) return { ch: decoded, width: named[0].length }; + const separator = SEPARATOR_ENTITIES.get(named[1]!.toLowerCase()); + return { ch: separator ?? NAMED_ENTITY_PLACEHOLDER, width: named[0].length }; } return null; }; diff --git a/tests/redact.test.ts b/tests/redact.test.ts index 3bab75ecc..e3facd7d2 100644 --- a/tests/redact.test.ts +++ b/tests/redact.test.ts @@ -386,6 +386,33 @@ describe("redactSecretString", () => { expect(xml).not.toContain("opaquecredential123456"); }); + test("multi-unit escapes decode as one character", () => { + // A JSON surrogate PAIR is one code point; decoding the halves separately + // left two lone surrogates that normalize to nothing. Percent encoding is + // UTF-8, so consecutive bytes are one character too — `%D0%B5` is Cyrillic + // `е`, not two Latin-1 characters. + expect(redactSecretString('{"\\uD835\\uDD69-api-key":"opaquecredential123456"}')) + .not.toContain("opaquecredential123456"); + expect(redactSecretString("x-api-k%D0%B5y=opaquecredential123456&model=gpt-5.5")) + .toBe(`x-api-k%D0%B5y=${REDACTED_SECRET}&model=gpt-5.5`); + }); + + test("any named entity inside a label is treated as one opaque letter", () => { + // The WHATWG table has ~2200 entries and neither Bun nor Node exposes it. + // Rather than promise a subset, an unresolved name folds to a placeholder + // the label accepts wherever a letter may appear — so `ⅈ`, `ⅇ`, and + // `ⅆ` are covered without claiming to know what they mean. + for (const input of [ + '
opaquecredential123456
', + '
opaquecredential123456
', + '
opaquecredential123456
', + ]) { + const redacted = redactSecretString(input); + expect(redacted).toContain(REDACTED_SECRET); + expect(redacted).not.toContain("opaquecredential123456"); + } + }); + test("non-credential fields in those framings are untouched", () => { expect(redactSecretString("model=gpt-5.5&status=429")).toBe("model=gpt-5.5&status=429"); expect(redactSecretString("gpt-5.5")).toBe("gpt-5.5");