diff --git a/src/lib/redact.ts b/src/lib/redact.ts index 185acc3f8..45a468b41 100644 --- a/src/lib/redact.ts +++ b/src/lib/redact.ts @@ -2,8 +2,413 @@ 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 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. + * + * `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. + * + * 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`). + */ +// 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(/(?([ + ["colon", ":"], ["semi", ";"], ["equals", "="], ["quot", '"'], ["apos", "'"], + ["lt", "<"], ["gt", ">"], ["amp", "&"], ["sol", "/"], ["lowbar", "_"], + ["hyphen", "-"], ["dash", "-"], ["ndash", "-"], ["mdash", "-"], ["minus", "-"], + ["period", "."], ["comma", ","], ["num", "#"], ["nbsp", " "], +]); + +/** + * 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([ + // 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"], + ["\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 +// `_`, 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( + `(? = [ + // 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, the mask keeps only the tag name and runs to END OF + // 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/>]))[\\s\\S]*`, + "gi", + ), + "element", + ], + [ + new RegExp( + `(<[^\\S\\r\\n]*(?:[A-Za-z_][\\w.-]*:)?[A-Za-z_][\\w:.-]*)(?=[^>]*?(?]))[\\s\\S]*`, + "gi", + ), + "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 + // 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( + `(name=["']?(?:${CREDENTIAL_HEADER_LABEL})["']?[^\\r\\n]*\\r?\\n(?:\\r?\\n)?)([\\s\\S]+)`, + "gi", + ), + "multipart", + ], +]; + +/** + * 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 { + // 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, decodeEscapes); + 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; + } + current = out + current.slice(cursor); + } + return current; +} + +/** + * 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, decodeEscapes = true): { 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 => { + // 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) { + 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]!; + 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 }; + } + } + // 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 separator = SEPARATOR_ENTITIES.get(named[1]!.toLowerCase()); + return { ch: separator ?? NAMED_ENTITY_PLACEHOLDER, width: named[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 + // 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 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)) { + i += width; + continue; + } + 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; + // 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 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; + 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; + const lineEnd = (() => { + const nl = value.slice(afterLabel).search(/[\r\n]/); + return nl === -1 ? value.length : afterLabel + nl; + })(); + // THE VALUE ALWAYS RUNS TO END-OF-LINE. There is no early termination and + // no attempt to preserve sibling fields. + // + // 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"}`). + // + // 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 = ""; + // `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 && 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); +} + 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,31 +417,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 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}`], [/((?:"(?: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`], @@ -56,7 +436,7 @@ function isSensitiveKey(key: string): boolean { } export function redactSecretString(value: string): string { - let redacted = value; + let redacted = maskOtherFramings(maskCredentialHeaders(value)); for (const [pattern, replacement] of SECRET_VALUE_PATTERNS) { redacted = redacted.replace(pattern, replacement); } 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 1a299be8c..e3facd7d2 100644 --- a/tests/redact.test.ts +++ b/tests/redact.test.ts @@ -65,6 +65,8 @@ 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. + // 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}`); expect(redactSecretString("Authorization: Basic dXNlcjpwYXNz")) @@ -79,6 +81,365 @@ describe("redactSecretString", () => { .toBe(`Authorization: Bearer ${REDACTED_SECRET}`); }); + 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. + // 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: ${REDACTED_SECRET}`); + expect(redactSecretString("Authorization: Bearer custom:credential123456")) + .toBe(`Authorization: Bearer ${REDACTED_SECRET}`); + expect(redactSecretString("x-api-key: Bearer short")) + .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", () => { + // `[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: ${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("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}`); + 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. + // 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${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 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("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 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. + 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}`); + }); + + test("a decoy quoted value does not end the mask early", () => { + // 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("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", () => { + // 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(` { + // 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', + 'public-status', + 'public', + '
public
', + 'Basic dXNlcjpwYXNz', + // Self-closing, namespace-qualified, and same-name nesting. + '', + '
', + "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); + expect(redacted).not.toContain("dXNlcjpwYXNz"); + expect(redacted).not.toContain("secret123456"); + expect(redacted).not.toContain("credential-suffix-123456"); + } + }); + + 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"); + // A prefixed attribute does not identify a credential either. + expect(redactSecretString('public-status')) + .toBe('public-status'); + }); + + 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, 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("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("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("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"); + }); + + 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("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}`); + }); + + 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", () => { // 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")) diff --git a/tests/usage-debug.test.ts b/tests/usage-debug.test.ts index fba52cd36..e915d324c 100644 --- a/tests/usage-debug.test.ts +++ b/tests/usage-debug.test.ts @@ -112,8 +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"); - expect(parsed.bodySample).toContain("Bearer [REDACTED]"); - expect(parsed.bodySample).toContain("refreshToken"); + // 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("[REDACTED]"); }); test("preserves estimated extracted usage while redacting surrounding secrets", () => {