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 = [ + // 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}` + ); + } + }); +});