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

src/lib/redact.ts

@@ -2,8 +2,276 @@ 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",
+]);
+
+/**
+ * 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<string, string>([
+  // 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":"<secret>"}`) 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(
+  `(?<![A-Za-z0-9_-])["']?(?:${CREDENTIAL_HEADER_LABEL})["']?[^\\S\\r\\n]*:`,
+  "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=<value>` up to `&` or `;`.
+  [
+    new RegExp(`(?<![A-Za-z0-9_-])(?:${CREDENTIAL_HEADER_LABEL})=(?:"[^"]*"|'[^']*'|[^&;\\s]*)`, "gi"),
+    "=",
+  ],
+  // 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(
+      `(<[^>]*?\\b(?:${CREDENTIAL_HEADER_LABEL})[^\\S\\r\\n]*=[^\\S\\r\\n]*)(?:"[^"]*"|'[^']*')`,
+      "gi",
+    ),
+    "attr",
+  ],
+  // A credential-named ELEMENT carrying its value in some other attribute:
+  // `<authorization value="Basic …">`. 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 `<authorizationStatus>` and `<token-count>` 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: `<header name="authorization">`.
+  [
+    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?)+)`,
+      "gi",
+    ),
+    "multipart",
+  ],
+];
+
+function maskOtherFramings(value: string): string {
+  let out = value;
+  for (const [pattern, kind] of OTHER_FRAMED_CREDENTIALS) {
+    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;
+}
+
+/**
+ * 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[] = [];
+  // 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 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 };
+}
+
+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;
+    const lineEnd = (() => {
+      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.
+    //
+    // 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",<secret>` 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 = 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;
+    // 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 && 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 +280,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 +299,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);
   }

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