From 03ea11314524ea0cf3b34a5d33cbd53e351b0866 Mon Sep 17 00:00:00 2001
From: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
Date: Thu, 3 Sep 2026 12:53:15 -0300
Subject: [PATCH] fix(security): redact raw sk-/AIza/JWT credentials in the
 error sanitizer
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

GHSA-qv45-56jc-4wmj: two copies of one redaction rule, and only one of them
learned about `sk-`.

`upstreamErrorPassthrough.ts` recognized `\bsk-[…]{8,}` and REFUSED verbatim
passthrough for any upstream 4xx carrying it — correctly treating it as a leak.
The body then fell through to `buildErrorBody` → `sanitizeErrorMessage` →
`redactSensitiveErrorText`, which had no such pattern. So the layer that
identified the credential handed it to a layer that could not see it, and
OpenAI-style `Incorrect API key provided: sk-proj-…` bodies came back to the
caller intact. For a gateway pooling operator keys that is cross-tenant
disclosure of the operator's upstream credential.

The passthrough file's comment said it "mirrors the vocabulary of
redactSensitiveErrorText". The mirror had drifted, which is why this is fixed at
the source rather than by adding a pattern on each side: the raw-token shapes now
live once, in `error.ts` as `RAW_CREDENTIAL_PATTERNS`, and the passthrough layer
imports them. Both sides move together or not at all.

Covers `sk-`/`sk_`, Google `AIza…`, and three-segment JWTs. Quantifiers are
upper-bounded per AGENTS.md → PII learnings §1 — these run over untrusted
upstream error bodies.

tests/unit/error-sanitizer-sk-key-qv45.test.ts — 9 tests, 7 red before the fix.
The last one is the anti-drift assertion: for every body the passthrough layer
refuses as leaky, the fallback sanitizer must not return it unchanged. It also
pins that ordinary prose containing "risk"/"task sk" is left alone.

Reported by @skeletonsec.

Closes GHSA-qv45-56jc-4wmj
---
 open-sse/utils/error.ts                       | 30 +++++-
 open-sse/utils/upstreamErrorPassthrough.ts    | 22 ++++-
 .../unit/error-sanitizer-sk-key-qv45.test.ts  | 93 +++++++++++++++++++
 3 files changed, 141 insertions(+), 4 deletions(-)
 create mode 100644 tests/unit/error-sanitizer-sk-key-qv45.test.ts

diff --git a/open-sse/utils/error.ts b/open-sse/utils/error.ts
index 4fafd05b1a..66563c618f 100644
--- a/open-sse/utils/error.ts
+++ b/open-sse/utils/error.ts
@@ -40,8 +40,36 @@ function looksLikeAbsolutePath(tok: string): boolean {
   return (SOURCE_EXT as readonly string[]).includes(ext);
 }
 
+/**
+ * Raw credential shapes that carry no `key=` label to key off — the token IS the
+ * whole match, so the only way to redact them is to recognize the shape.
+ *
+ * GHSA-qv45-56jc-4wmj: `upstreamErrorPassthrough.ts` already recognized `sk-`
+ * and refused verbatim passthrough for bodies containing it, then handed those
+ * bodies to THIS sanitizer — which had no such pattern, so the key came back to
+ * the caller anyway. The passthrough file's comment claimed to "mirror the
+ * vocabulary of redactSensitiveErrorText"; the mirror had drifted. It now
+ * imports this array instead of keeping a second copy, so the two cannot drift
+ * again.
+ *
+ * Quantifiers are upper-bounded (AGENTS.md → PII learnings §1, ReDoS): these run
+ * over untrusted upstream error bodies.
+ */
+export const RAW_CREDENTIAL_PATTERNS: ReadonlyArray<RegExp> = [
+  // OpenAI/Anthropic/Stripe-style secret keys: sk-…, sk-ant-…, sk_live_…
+  /\bsk[-_][A-Za-z0-9._-]{8,200}/g,
+  // Google API keys
+  /\bAIza[A-Za-z0-9_-]{20,200}/g,
+  // JWTs (three base64url segments)
+  /\beyJ[A-Za-z0-9_-]{8,400}\.[A-Za-z0-9_-]{8,800}\.[A-Za-z0-9_-]{8,800}/g,
+];
+
 export function redactSensitiveErrorText(value: string): string {
-  return value
+  let out = value;
+  for (const pattern of RAW_CREDENTIAL_PATTERNS) {
+    out = out.replace(pattern, "[REDACTED_CREDENTIAL]");
+  }
+  return out
     .replace(/data:[^,\s]+;base64,[A-Za-z0-9+/=_-]+/gi, "[REDACTED_DATA_URL]")
     .replace(/\b(Bearer|Basic)\s+[A-Za-z0-9._~+/=-]+/gi, "$1 [REDACTED]")
     .replace(
diff --git a/open-sse/utils/upstreamErrorPassthrough.ts b/open-sse/utils/upstreamErrorPassthrough.ts
index 21d0c6c964..b62fff2adf 100644
--- a/open-sse/utils/upstreamErrorPassthrough.ts
+++ b/open-sse/utils/upstreamErrorPassthrough.ts
@@ -1,3 +1,4 @@
+import { RAW_CREDENTIAL_PATTERNS } from "./error.ts";
 /**
  * Selective upstream 4xx error passthrough (Claude Code auto-recover contract).
  *
@@ -24,8 +25,23 @@ const INTERNAL_LEAK_RE = /\sat\s\/|node_modules|omniroute\//i;
 // caller fall back to the sanitized buildErrorBody path. Bodies without a
 // secret (the overwhelming majority, carrying capability/quota wording) still
 // relay verbatim. Mirrors the vocabulary of redactSensitiveErrorText in error.ts.
-const CREDENTIAL_LEAK_RE =
-  /\b(?:Bearer|Basic)\s+[A-Za-z0-9._~+/=-]{8,}|\bsk-[A-Za-z0-9._-]{8,}|(?:api[_-]?key|access[_-]?token|refresh[_-]?token|authorization|cookie|secret)\\?["']?\s*[:=]\s*\\?["']?[^"'\\,\s}]{6,}/i;
+const LABELLED_CREDENTIAL_RE =
+  /\b(?:Bearer|Basic)\s+[A-Za-z0-9._~+/=-]{8,}|(?:api[_-]?key|access[_-]?token|refresh[_-]?token|authorization|cookie|secret)\\?["']?\s*[:=]\s*\\?["']?[^"'\\,\s}]{6,}/i;
+
+/**
+ * The raw-token shapes (sk-…, AIza…, JWT) come from error.ts's
+ * RAW_CREDENTIAL_PATTERNS rather than a second local copy. The previous local
+ * copy carried `sk-` while the sanitizer this file falls back to did NOT, so a
+ * body recognized as leaky here was returned unredacted there
+ * (GHSA-qv45-56jc-4wmj). One source, no drift.
+ */
+function containsCredential(text: string): boolean {
+  if (LABELLED_CREDENTIAL_RE.test(text)) return true;
+  return RAW_CREDENTIAL_PATTERNS.some((pattern) => {
+    pattern.lastIndex = 0; // the shared patterns are /g — reset before .test()
+    return pattern.test(text);
+  });
+}
 
 export function shouldPassthroughUpstreamError(statusCode: number, upstreamBody: unknown): boolean {
   if (statusCode < PASSTHROUGH_MIN || statusCode > PASSTHROUGH_MAX) return false;
@@ -34,7 +50,7 @@ export function shouldPassthroughUpstreamError(statusCode: number, upstreamBody:
   const text = JSON.stringify(upstreamBody);
   if (INTERNAL_LEAK_RE.test(text)) return false;
   // Refuse passthrough when the provider echoed a credential back to us.
-  if (CREDENTIAL_LEAK_RE.test(text)) return false;
+  if (containsCredential(text)) return false;
   return true;
 }
 
diff --git a/tests/unit/error-sanitizer-sk-key-qv45.test.ts b/tests/unit/error-sanitizer-sk-key-qv45.test.ts
new file mode 100644
index 0000000000..b92d5ebbb1
--- /dev/null
+++ b/tests/unit/error-sanitizer-sk-key-qv45.test.ts
@@ -0,0 +1,93 @@
+/**
+ * GHSA-qv45-56jc-4wmj — two copies of one redaction rule, and only one got the
+ * `sk-` pattern.
+ *
+ * `upstreamErrorPassthrough.ts`'s CREDENTIAL_LEAK_RE matches `\bsk-[…]{8,}` and
+ * REFUSES verbatim passthrough when an upstream 4xx echoes a key — correctly
+ * treating it as a leak. The body then falls through to `buildErrorBody` →
+ * `sanitizeErrorMessage` → `redactSensitiveErrorText`, which had no `sk-`
+ * pattern at all. So the layer that recognized the credential handed it to a
+ * layer that did not, and OpenAI-style `Incorrect API key provided: sk-proj-…`
+ * bodies were returned to the caller verbatim.
+ *
+ * The passthrough file's own comment says it "mirrors the vocabulary of
+ * redactSensitiveErrorText" — the mirror had diverged. This suite pins both
+ * directions so it cannot diverge again.
+ *
+ * Run with:
+ *   node --import tsx/esm --test tests/unit/error-sanitizer-sk-key-qv45.test.ts
+ */
+
+import { describe, it } from "node:test";
+import assert from "node:assert/strict";
+
+import { redactSensitiveErrorText, sanitizeErrorMessage } from "../../open-sse/utils/error.ts";
+
+const LEAKY_BODIES = [
+  "Incorrect API key provided: sk-proj-AbCdEfGhIjKlMnOpQrStUv. You can find your API key at …",
+  "401 Unauthorized: sk-ant-api03-abcdefghijklmnopqrstuvwxyz-1234567890",
+  "invalid key sk_live_51H8xKzAbCdEfGhIjKlMn",
+  "Bad credentials for AIzaSyA1B2C3D4E5F6G7H8I9J0KaLbMcNdOeP",
+  "token rejected: eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0.dozjgNryP4J3jVmNHl0w5N_XgL0n3I9PlFUP0THsR8U",
+];
+
+describe("redactSensitiveErrorText — raw credential patterns (GHSA-qv45-56jc-4wmj)", () => {
+  for (const body of LEAKY_BODIES) {
+    it(`redacts the raw credential in: ${body.slice(0, 42)}…`, () => {
+      const out = redactSensitiveErrorText(body);
+      assert.ok(!/\bsk[-_][A-Za-z0-9._-]{8,}/.test(out), `sk- survived: ${out}`);
+      assert.ok(!/\bAIza[A-Za-z0-9_-]{20,}/.test(out), `Google key survived: ${out}`);
+      assert.ok(!/\beyJ[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,}\./.test(out), `JWT survived: ${out}`);
+      assert.ok(out.includes("[REDACTED"), `nothing was redacted: ${out}`);
+    });
+  }
+
+  it("redacts through sanitizeErrorMessage, the path buildErrorBody actually uses", () => {
+    const out = sanitizeErrorMessage("Incorrect API key provided: sk-proj-AbCdEfGhIjKlMnOpQr.");
+    assert.ok(!out.includes("sk-proj-AbCdEfGhIjKlMnOpQr"), out);
+  });
+
+  it("keeps the pre-existing redactions working", () => {
+    assert.match(redactSensitiveErrorText("401: Bearer abc123def456"), /Bearer \[REDACTED\]/);
+    assert.match(
+      redactSensitiveErrorText('{"api_key":"secret-value","detail":"bad"}'),
+      /\[REDACTED\]/
+    );
+    assert.match(
+      redactSensitiveErrorText("data:image/png;base64,AAAABBBBCCCC"),
+      /\[REDACTED_DATA_URL\]/
+    );
+  });
+
+  it("does not maul ordinary error prose that merely contains 'sk'", () => {
+    for (const benign of [
+      "Model gpt-5 is not available on this plan",
+      "risk score too high",
+      "task sk failed", // short, no credential shape
+      "Rate limit reached for requests",
+    ]) {
+      assert.equal(redactSensitiveErrorText(benign), benign);
+    }
+  });
+});
+
+describe("the two redaction layers stay in step", () => {
+  it("every credential shape the passthrough layer refuses is also redacted here", async () => {
+    // If passthrough REFUSES a body as leaky, the fallback sanitizer is the only
+    // thing standing between that body and the caller. Anything the first layer
+    // calls a credential, the second must scrub.
+    const { shouldPassthroughUpstreamError } =
+      await import("../../open-sse/utils/upstreamErrorPassthrough.ts");
+    for (const body of LEAKY_BODIES) {
+      const payload = { error: { message: body } };
+      const relayedVerbatim = shouldPassthroughUpstreamError(401, payload);
+      if (relayedVerbatim) continue; // not classified as a leak — nothing to assert
+      const scrubbed = redactSensitiveErrorText(body);
+      assert.notEqual(
+        scrubbed,
+        body,
+        `passthrough refused this body as leaky but the sanitizer left it untouched: ${body}`
+      );
+    }
+  });
+});