mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-24 08:02:14 +03:00
fix(security): refuse proxy-authorization and proxy-authenticate upstream (#11328)
Validated on a 17-PR combined board: upstream-headers-proxy-auth within the board's 287/287, typecheck:core clean, gates within baseline. proxy-authorization and proxy-authenticate join the FORBIDDEN denylist — forwarding proxy-authorization to a model provider would hand that provider the operator's own proxy credential. Thank you @ntdat812!
This commit is contained in:
1
changelog.d/fixes/11328-upstream-headers-proxy-auth.md
Normal file
1
changelog.d/fixes/11328-upstream-headers-proxy-auth.md
Normal file
@@ -0,0 +1 @@
|
||||
- **fix(security):** `proxy-authorization` and `proxy-authenticate` are refused as upstream/custom headers, so a proxy credential is no longer forwarded to the model provider — the canonical denylist now matches the RFC 7230 §6.1 set the rest of the codebase already strips ([#11328](https://github.com/diegosouzapw/OmniRoute/pull/11328))
|
||||
@@ -10,6 +10,16 @@ const FORBIDDEN = new Set(
|
||||
"content-length",
|
||||
"keep-alive",
|
||||
"proxy-connection",
|
||||
// The two RFC 7230 §6.1 hop-by-hop names this list was missing. They belong
|
||||
// to the connection between the client and OmniRoute (or its upstream
|
||||
// proxy), never to the request OmniRoute makes to the model provider —
|
||||
// forwarding `proxy-authorization` hands that proxy credential to the
|
||||
// provider. `src/lib/services/reverseProxy.ts` (HOP_BY_HOP),
|
||||
// `src/mitm/sanitizeHeaders.ts`, `src/mitm/inspector/httpProxyServer.ts`,
|
||||
// `src/mitm/tproxy/tlsCapture.ts` and `src/app/api/openapi/try/route.ts`
|
||||
// all already strip them; this list, the canonical one, did not.
|
||||
"proxy-authenticate",
|
||||
"proxy-authorization",
|
||||
"transfer-encoding",
|
||||
"te",
|
||||
"trailer",
|
||||
|
||||
66
tests/unit/upstream-headers-proxy-auth.test.ts
Normal file
66
tests/unit/upstream-headers-proxy-auth.test.ts
Normal file
@@ -0,0 +1,66 @@
|
||||
// `FORBIDDEN` in src/shared/constants/upstreamHeaders.ts is documented as the
|
||||
// hop-by-hop / Host / framing denylist, and it was missing two of the RFC 7230
|
||||
// §6.1 names. Measured before the fix:
|
||||
//
|
||||
// proxy-authorization upstream=allow custom=allow
|
||||
// proxy-authenticate upstream=allow custom=allow
|
||||
// proxy-connection upstream=BLOCK custom=BLOCK
|
||||
//
|
||||
// `proxy-authorization` is the one that costs something: it authenticates the
|
||||
// hop to the operator's own proxy, so forwarding it hands that credential to
|
||||
// the model provider. Five other modules in this repo already strip it
|
||||
// (reverseProxy HOP_BY_HOP, mitm/sanitizeHeaders, inspector/httpProxyServer,
|
||||
// tproxy/tlsCapture, openapi/try) — the canonical list did not.
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
import {
|
||||
isForbiddenUpstreamHeaderName,
|
||||
isForbiddenCustomHeaderName,
|
||||
} from "../../src/shared/constants/upstreamHeaders.ts";
|
||||
import { HOP_BY_HOP } from "../../src/lib/services/reverseProxy.ts";
|
||||
import { sanitizeUpstreamHeadersMap } from "../../src/lib/db/models.ts";
|
||||
|
||||
test("proxy-authorization and proxy-authenticate are refused", () => {
|
||||
for (const name of ["proxy-authorization", "proxy-authenticate"]) {
|
||||
assert.equal(isForbiddenUpstreamHeaderName(name), true, name);
|
||||
assert.equal(isForbiddenCustomHeaderName(name), true, name);
|
||||
}
|
||||
});
|
||||
|
||||
test("the refusal is case-insensitive, like every other name in the list", () => {
|
||||
for (const name of ["Proxy-Authorization", "PROXY-AUTHENTICATE", " Proxy-Authorization "]) {
|
||||
assert.equal(isForbiddenUpstreamHeaderName(name), true, name);
|
||||
}
|
||||
});
|
||||
|
||||
test("sanitizeUpstreamHeadersMap drops them and keeps the rest", () => {
|
||||
const out = sanitizeUpstreamHeadersMap({
|
||||
"Proxy-Authorization": "Basic c2VjcmV0",
|
||||
"Proxy-Authenticate": "Basic realm=x",
|
||||
"X-Custom": "ok",
|
||||
});
|
||||
|
||||
assert.deepEqual(out, { "X-Custom": "ok" });
|
||||
});
|
||||
|
||||
test("the canonical list now covers every hop-by-hop name reverseProxy strips", () => {
|
||||
// `reverseProxy.HOP_BY_HOP` is the repo's own RFC 7230 §6.1 list. The two
|
||||
// lists drifting apart is what this fix repairs, so compare them directly —
|
||||
// `trailers` is the TE token, spelled `trailer` as a header name.
|
||||
const missing = [...HOP_BY_HOP]
|
||||
.map((name) => (name === "trailers" ? "trailer" : name))
|
||||
.filter((name) => !isForbiddenUpstreamHeaderName(name));
|
||||
|
||||
assert.deepEqual(missing, []);
|
||||
});
|
||||
|
||||
test("ordinary headers are still allowed", () => {
|
||||
for (const name of ["x-custom", "x-forwarded-for", "user-agent", "accept"]) {
|
||||
assert.equal(isForbiddenUpstreamHeaderName(name), false, name);
|
||||
}
|
||||
// Auth headers stay allowed as *upstream* headers (the credential layer owns
|
||||
// them) while remaining forbidden as operator-supplied custom headers.
|
||||
assert.equal(isForbiddenUpstreamHeaderName("authorization"), false);
|
||||
assert.equal(isForbiddenCustomHeaderName("authorization"), true);
|
||||
});
|
||||
Reference in New Issue
Block a user