fix(redact): close the Bearer smuggling hole in the colon rule

src/lib/redact.ts

@@ -2,8 +2,107 @@ 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: <value>`), 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`).
+ */
+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";
+
+/**
+ * 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]*:`,
+  "gi",
+);
+
+/**
+ * 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 {
+  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 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 +111,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 <tok> 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 <tok>` 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 +130,7 @@ function isSensitiveKey(key: string): boolean {
 }
 
 export function redactSecretString(value: string): string {
-  let redacted = value;
+  let redacted = maskCredentialHeaders(value);
   for (const [pattern, replacement] of SECRET_VALUE_PATTERNS) {
     redacted = redacted.replace(pattern, replacement);
   }

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]");
   });
 

tests/redact.test.ts

@@ -79,6 +79,94 @@ 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 <secret>`.
+    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 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"))