mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-13 18:32:12 +03:00
fix(relay): strip echoed credentials from relayed upstream responses
GHSA-9m72-44hg-w32g: the standalone bifrost relay copied ALL upstream response
headers via `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 whoever holds the relay token. `set-cookie`
matters as much as the credential: it is a session, and a browser would store it
against our origin.
Both fixes:
- New shared `stripSensitiveResponseHeaders()` in upstreamResponseHeaders.ts,
built on the `filterUpstreamResponseHeaderEntries` helper that already
existed there unused. Drops authorization / proxy-authorization / x-api-key /
x-goog-api-key / api-key / cookie / set-cookie on top of the stale framing
set. Applied to BOTH relay routes — the TS sibling copied headers wholesale
too, so fixing only the reported one would have left the same hole next door
and a second copy waiting to drift, which is the exact failure mode of
GHSA-v7g9 and GHSA-qv45.
- The bifrost route normalizes non-2xx through parseUpstreamError +
buildErrorBody + sanitizeErrorMessage, reaching parity with the sibling and
Hard Rule #12.
tests/unit/bifrost-relay-response-leak-9m72.test.ts — 6 tests, red before the
fix: the helper's behaviour (including that it does not mutate its input), plus
source guards pinning BOTH routes to the shared strip with a negative assertion
that the wholesale copy has not come back.
Reported by @skeletonsec.
Closes GHSA-9m72-44hg-w32g
This commit is contained in:
@@ -45,3 +45,36 @@ export function filterUpstreamResponseHeaderEntries(
|
||||
}
|
||||
|
||||
export const STRIP_UPSTREAM_HEADER_NAMES: ReadonlySet<string> = 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<string> = [
|
||||
"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 };
|
||||
|
||||
@@ -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, {
|
||||
|
||||
@@ -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 + "...");
|
||||
|
||||
103
tests/unit/bifrost-relay-response-leak-9m72.test.ts
Normal file
103
tests/unit/bifrost-relay-response-leak-9m72.test.ts
Normal file
@@ -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)");
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user