diff --git a/open-sse/utils/upstreamResponseHeaders.ts b/open-sse/utils/upstreamResponseHeaders.ts index 834d354f18..9f6c16d1e6 100644 --- a/open-sse/utils/upstreamResponseHeaders.ts +++ b/open-sse/utils/upstreamResponseHeaders.ts @@ -45,3 +45,36 @@ export function filterUpstreamResponseHeaderEntries( } export const STRIP_UPSTREAM_HEADER_NAMES: ReadonlySet = STRIP_HEADER_NAMES; + +/** + * Response headers that must never be relayed back to a client. + * + * A relay sends its own credential upstream (the bifrost route sends + * `Authorization: Bearer ${BIFROST_API_KEY}` to the sidecar). If that upstream + * echoes the header back — or sets its own session cookie — copying the response + * headers wholesale hands it to whoever holds the relay token + * (GHSA-9m72-44hg-w32g). `set-cookie` matters as much as `authorization`: it is + * a session, and the browser would store it against OUR origin. + */ +const SENSITIVE_RESPONSE_HEADER_NAMES: ReadonlyArray = [ + "authorization", + "proxy-authorization", + "x-api-key", + "x-goog-api-key", + "api-key", + "cookie", + "set-cookie", +]; + +/** + * New Headers with the stale framing set AND any echoed credential/session + * header removed. Use this instead of `new Headers(upstream.headers)` on every + * path that relays an upstream response to a client. Does not mutate the input. + */ +export function stripSensitiveResponseHeaders(input: Headers): Headers { + return new Headers( + filterUpstreamResponseHeaderEntries(input.entries(), SENSITIVE_RESPONSE_HEADER_NAMES) + ); +} + +export { SENSITIVE_RESPONSE_HEADER_NAMES }; diff --git a/src/app/api/v1/relay/chat/completions/bifrost/route.ts b/src/app/api/v1/relay/chat/completions/bifrost/route.ts index 31b007959f..9f0576bf19 100644 --- a/src/app/api/v1/relay/chat/completions/bifrost/route.ts +++ b/src/app/api/v1/relay/chat/completions/bifrost/route.ts @@ -29,6 +29,9 @@ */ import { CORS_HEADERS, handleCorsOptions } from "@/shared/utils/cors"; +import { stripSensitiveResponseHeaders } from "@omniroute/open-sse/utils/upstreamResponseHeaders"; +import { buildErrorBody, sanitizeErrorMessage } from "@omniroute/open-sse/utils/error"; +import { parseUpstreamError } from "@omniroute/open-sse/handlers/usageExtractor"; import { createInjectionGuard } from "@/middleware/promptInjectionGuard"; import { getRelayTokenByHash, checkRateLimit, recordRelayUsage } from "@/lib/db/relayProxies"; import { buildErrorBody } from "@omniroute/open-sse/utils/error"; @@ -299,7 +302,11 @@ export async function POST(request: Request) { }); }; - const newHeaders = new Headers(upstream.headers); + // Never copy the upstream headers wholesale: the sidecar receives our + // `Authorization: Bearer ${BIFROST_API_KEY}` and anything it echoes back — + // that header, its own set-cookie — would reach the relay-token holder + // (GHSA-9m72-44hg-w32g). + const newHeaders = stripSensitiveResponseHeaders(upstream.headers); newHeaders.set("X-Routed-By", "bifrost"); newHeaders.set("X-Relay-Token", token.tokenPrefix + "..."); if (!wantsStream) { @@ -321,6 +328,28 @@ export async function POST(request: Request) { } clearTimeout(tid); + + // Normalize non-2xx through parseUpstreamError + buildErrorBody instead of + // relaying the sidecar body verbatim — parity with the TS sibling route and + // Hard Rule #12 (GHSA-9m72-44hg-w32g). + if (!upstream.ok) { + const parsed = await parseUpstreamError(upstream, null); + const errorBody = buildErrorBody( + parsed.statusCode, + sanitizeErrorMessage(parsed.message), + parsed.responseBody + ); + newHeaders.set("Content-Type", "application/json"); + if (parsed.retryAfterMs && parsed.retryAfterMs > 0) { + newHeaders.set("Retry-After", String(Math.ceil(parsed.retryAfterMs / 1000))); + } + recordUsage("error", parsed.statusCode); + return new Response(JSON.stringify(errorBody), { + status: parsed.statusCode, + headers: newHeaders, + }); + } + recordUsage(upstream.status < 500 ? "success" : "error", upstream.status); return new Response(upstream.body, { diff --git a/src/app/api/v1/relay/chat/completions/route.ts b/src/app/api/v1/relay/chat/completions/route.ts index fa40d4a009..4e43bb7a13 100644 --- a/src/app/api/v1/relay/chat/completions/route.ts +++ b/src/app/api/v1/relay/chat/completions/route.ts @@ -7,6 +7,7 @@ */ import { CORS_HEADERS, handleCorsOptions } from "@/shared/utils/cors"; +import { stripSensitiveResponseHeaders } from "@omniroute/open-sse/utils/upstreamResponseHeaders"; import { handleChat } from "@/sse/handlers/chat"; import { withChatAdmission } from "@/shared/middleware/withChatAdmission"; import { createInjectionGuard } from "@/middleware/promptInjectionGuard"; @@ -109,7 +110,9 @@ async function forwardToBifrost( signal: ac.signal, }); - const headers = new Headers(upstream.headers); + // Same strip as the bifrost sibling: an echoed upstream credential or + // set-cookie must not reach the relay-token holder (GHSA-9m72-44hg-w32g). + const headers = stripSensitiveResponseHeaders(upstream.headers); headers.set("X-Routed-By", "bifrost"); headers.set("X-Routing-Backend", "bifrost"); headers.set("X-Relay-Token", token.tokenPrefix + "..."); diff --git a/tests/unit/bifrost-relay-response-leak-9m72.test.ts b/tests/unit/bifrost-relay-response-leak-9m72.test.ts new file mode 100644 index 0000000000..c5affaf655 --- /dev/null +++ b/tests/unit/bifrost-relay-response-leak-9m72.test.ts @@ -0,0 +1,103 @@ +/** + * GHSA-9m72-44hg-w32g — the standalone bifrost relay route copied ALL upstream + * response headers (`new Headers(upstream.headers)`) and returned non-2xx bodies + * verbatim, while its TypeScript sibling routed non-2xx through + * parseUpstreamError + buildErrorBody + stripStaleEncodingHeaders. + * + * The relay sends `Authorization: Bearer ${BIFROST_API_KEY}` to the sidecar, so + * anything the sidecar (or a further upstream) echoes back — that header, its own + * `set-cookie`, an `x-api-key` — reached the relay-token holder untouched. + * + * Both relay routes copy upstream headers, so the strip is a shared helper used + * by both rather than a fix in one and a second copy waiting to drift (the + * failure mode of GHSA-v7g9 and GHSA-qv45). + * + * Run with: + * node --import tsx/esm --test tests/unit/bifrost-relay-response-leak-9m72.test.ts + */ + +import { describe, it } from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; + +import { stripSensitiveResponseHeaders } from "../../open-sse/utils/upstreamResponseHeaders.ts"; + +const read = (rel: string) => + readFileSync(fileURLToPath(new URL(`../../${rel}`, import.meta.url)), "utf8"); + +const BIFROST_ROUTE = "src/app/api/v1/relay/chat/completions/bifrost/route.ts"; +const TS_ROUTE = "src/app/api/v1/relay/chat/completions/route.ts"; + +describe("stripSensitiveResponseHeaders", () => { + it("drops credentials and cookies the upstream echoed back", () => { + const upstream = new Headers([ + ["authorization", "Bearer sidecar-secret"], + ["x-api-key", "op-key"], + ["x-goog-api-key", "goog-key"], + ["api-key", "azure-key"], + ["cookie", "session=abc"], + ["set-cookie", "session=abc; HttpOnly"], + ["proxy-authorization", "Basic zzz"], + ["content-type", "application/json"], + ["x-request-id", "keep-me"], + ]); + const out = stripSensitiveResponseHeaders(upstream); + for (const gone of [ + "authorization", + "x-api-key", + "x-goog-api-key", + "api-key", + "cookie", + "set-cookie", + "proxy-authorization", + ]) { + assert.equal(out.get(gone), null, `${gone} survived`); + } + assert.equal(out.get("content-type"), "application/json"); + assert.equal(out.get("x-request-id"), "keep-me"); + }); + + it("also drops the stale framing headers", () => { + const out = stripSensitiveResponseHeaders( + new Headers([ + ["content-encoding", "gzip"], + ["content-length", "123"], + ["transfer-encoding", "chunked"], + ["x-keep", "yes"], + ]) + ); + assert.equal(out.get("content-encoding"), null); + assert.equal(out.get("content-length"), null); + assert.equal(out.get("transfer-encoding"), null); + assert.equal(out.get("x-keep"), "yes"); + }); + + it("does not mutate the input Headers", () => { + const input = new Headers([["authorization", "Bearer x"]]); + stripSensitiveResponseHeaders(input); + assert.equal(input.get("authorization"), "Bearer x"); + }); +}); + +describe("both relay routes use the shared strip (GHSA-9m72-44hg-w32g)", () => { + for (const route of [BIFROST_ROUTE, TS_ROUTE]) { + it(`${route} strips sensitive upstream response headers`, () => { + const src = read(route); + assert.ok( + src.includes("stripSensitiveResponseHeaders"), + `${route} relays upstream headers verbatim — a sidecar-echoed credential reaches the caller` + ); + assert.ok( + !/new Headers\(upstream\.headers\)/.test(src), + `${route} still copies upstream headers wholesale` + ); + }); + } + + it("the bifrost route normalizes non-2xx through the error sanitizer", () => { + const src = read(BIFROST_ROUTE); + assert.ok(src.includes("buildErrorBody"), "bifrost non-2xx must not be relayed verbatim"); + assert.ok(src.includes("sanitizeErrorMessage"), "bifrost non-2xx must be sanitized (HR#12)"); + }); +});