mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-11 01:13:02 +03:00
Compare commits
7 Commits
fix/12783-
...
fix/sec-3a
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
17a1fe0bd2 | ||
|
|
dfb3d434af | ||
|
|
6aa1dad10e | ||
|
|
02813895ee | ||
|
|
df23f55c7b | ||
|
|
03ea113145 | ||
|
|
bd65c6d89e |
@@ -33,6 +33,17 @@ export interface SearchProviderConfig {
|
||||
*/
|
||||
fallbackOnly?: boolean;
|
||||
disabled?: boolean;
|
||||
/**
|
||||
* May a CALLER-supplied `provider_options.baseUrl` redirect this provider?
|
||||
*
|
||||
* Only ever true for a keyless, self-hosted provider. For anything with
|
||||
* `authType: "apikey"` the builder attaches the OPERATOR's key to whatever
|
||||
* host the base URL resolves to, so honoring a caller-chosen value hands that
|
||||
* key to the caller's server (GHSA-3f8g-pfh9-j687). The invariant
|
||||
* "never set alongside authType: apikey" is enforced by
|
||||
* tests/unit/search-baseurl-client-override-3f8g.test.ts.
|
||||
*/
|
||||
allowClientBaseUrlOverride?: boolean;
|
||||
}
|
||||
|
||||
export const SEARCH_PROVIDERS: Record<string, SearchProviderConfig> = {
|
||||
@@ -232,6 +243,11 @@ export const SEARCH_PROVIDERS: Record<string, SearchProviderConfig> = {
|
||||
timeoutMs: 10_000,
|
||||
cacheTTLMs: 3 * 60 * 1000,
|
||||
fallbackOnly: true,
|
||||
// Keyless and self-hosted by definition: the caller names their own SearXNG
|
||||
// instance and no operator credential travels with the request. This is a
|
||||
// documented flow (tests/unit/search-route.test.ts). Still block-metadata
|
||||
// guarded, so IMDS stays unreachable.
|
||||
allowClientBaseUrlOverride: true,
|
||||
},
|
||||
|
||||
"ollama-search": {
|
||||
|
||||
@@ -20,6 +20,9 @@ import { randomUUID } from "crypto";
|
||||
* }
|
||||
*/
|
||||
|
||||
export { resolveSearchBaseUrl, SearchBaseUrlOverrideError } from "./search/baseUrl.ts";
|
||||
import { resolveSearchBaseUrl } from "./search/baseUrl.ts";
|
||||
|
||||
import {
|
||||
getSearchProvider,
|
||||
isUnconfiguredLoopbackSearchProvider,
|
||||
@@ -36,7 +39,6 @@ import * as anysearchSearch from "./search/anysearchSearch.ts";
|
||||
import { freeWebSearch } from "../services/freeWebSearch.ts";
|
||||
import { saveCallLog } from "@/lib/usageDb";
|
||||
import { safeOutboundFetch } from "@/shared/network/safeOutboundFetch";
|
||||
import { parseAndValidateNonMetadataUrl } from "@/shared/network/outboundUrlGuard";
|
||||
import { Client } from "@modelcontextprotocol/sdk/client/index.js";
|
||||
import { StreamableHTTPClientTransport } from "@modelcontextprotocol/sdk/client/streamableHttp.js";
|
||||
import { z } from "zod";
|
||||
@@ -319,25 +321,6 @@ function getProviderSettingString(
|
||||
return undefined;
|
||||
}
|
||||
|
||||
export function resolveSearchBaseUrl(
|
||||
config: SearchProviderConfig,
|
||||
params: SearchRequestParams
|
||||
): string {
|
||||
const override = getProviderSettingString(params, "baseUrl");
|
||||
if (override) {
|
||||
// GHSA-j7j4-g9qc-q69c: the override is client-controlled (provider_options /
|
||||
// providerSpecificData) and flows into a plain fetch() sink — validate it
|
||||
// before any builder uses it as the server-side fetch target. Mode is
|
||||
// block-metadata (NOT public-only): the primary searxng use case is a
|
||||
// self-hosted instance on loopback/LAN, so private hosts keep working,
|
||||
// while cloud-metadata endpoints (IMDS credential theft) are rejected.
|
||||
// The catalog's own config.baseUrl is operator config and stays untouched.
|
||||
parseAndValidateNonMetadataUrl(override);
|
||||
return override.replace(/\/+$/, "");
|
||||
}
|
||||
return config.baseUrl.replace(/\/+$/, "");
|
||||
}
|
||||
|
||||
function toSearchPageNumber(offset: number | undefined, maxResults: number): number | undefined {
|
||||
if (typeof offset !== "number" || offset <= 0 || maxResults <= 0) return undefined;
|
||||
return Math.floor(offset / maxResults) + 1;
|
||||
|
||||
66
open-sse/handlers/search/baseUrl.ts
Normal file
66
open-sse/handlers/search/baseUrl.ts
Normal file
@@ -0,0 +1,66 @@
|
||||
/**
|
||||
* Base-URL resolution for /v1/search — the trust decision, on its own.
|
||||
*
|
||||
* The two override sources are NOT equally trusted, and reading them through
|
||||
* one call is what produced GHSA-3f8g-pfh9-j687:
|
||||
*
|
||||
* - `providerSpecificData` is the stored provider connection
|
||||
* (`credentials?.providerSpecificData`) — OPERATOR config. Honored under
|
||||
* block-metadata, so a self-hosted SearXNG on loopback/LAN keeps working
|
||||
* while cloud metadata (IMDS credential theft) stays rejected (GHSA-j7j4).
|
||||
* - `providerOptions` is `body.provider_options` — CALLER input. Honored only
|
||||
* by a provider that is keyless AND opted in via
|
||||
* `allowClientBaseUrlOverride`. A keyed builder attaches the OPERATOR's key
|
||||
* to whatever host this resolves to (`key=`/`api_key=` in the query for
|
||||
* google-pse/searchapi, `X-API-Key`/`Authorization` for
|
||||
* you.com/linkup/nimble/ollama), so a caller-chosen host would collect it —
|
||||
* and a block-metadata check does nothing about that, because the attacker
|
||||
* simply names their own public host.
|
||||
*
|
||||
* Full coverage: tests/unit/search-baseurl-client-override-3f8g.test.ts.
|
||||
*/
|
||||
|
||||
import { parseAndValidateNonMetadataUrl } from "@/shared/network/outboundUrlGuard";
|
||||
import type { SearchProviderConfig } from "../../config/searchRegistry.ts";
|
||||
|
||||
interface BaseUrlParams {
|
||||
providerOptions?: Record<string, unknown>;
|
||||
providerSpecificData?: Record<string, unknown>;
|
||||
}
|
||||
|
||||
/** Read one string setting from a SINGLE source, so callers can distinguish trust. */
|
||||
function readSetting(source: Record<string, unknown> | undefined, key: string): string | undefined {
|
||||
const value = source?.[key];
|
||||
return typeof value === "string" && value.trim().length > 0 ? value.trim() : undefined;
|
||||
}
|
||||
|
||||
/** Refusal for a caller-supplied `provider_options.baseUrl` (GHSA-3f8g-pfh9-j687). */
|
||||
export class SearchBaseUrlOverrideError extends Error {
|
||||
readonly code = "SEARCH_BASE_URL_OVERRIDE_REFUSED";
|
||||
constructor(providerId: string) {
|
||||
super(
|
||||
`provider_options.baseUrl is not accepted for search provider "${providerId}". ` +
|
||||
`Set the base URL on the provider connection instead.`
|
||||
);
|
||||
this.name = "SearchBaseUrlOverrideError";
|
||||
}
|
||||
}
|
||||
|
||||
export function resolveSearchBaseUrl(config: SearchProviderConfig, params: BaseUrlParams): string {
|
||||
const operatorOverride = readSetting(params.providerSpecificData, "baseUrl");
|
||||
if (operatorOverride) {
|
||||
parseAndValidateNonMetadataUrl(operatorOverride);
|
||||
return operatorOverride.replace(/\/+$/, "");
|
||||
}
|
||||
|
||||
const callerOverride = readSetting(params.providerOptions, "baseUrl");
|
||||
if (callerOverride) {
|
||||
if (!config.allowClientBaseUrlOverride || config.authType === "apikey") {
|
||||
throw new SearchBaseUrlOverrideError(config.id);
|
||||
}
|
||||
parseAndValidateNonMetadataUrl(callerOverride);
|
||||
return callerOverride.replace(/\/+$/, "");
|
||||
}
|
||||
|
||||
return config.baseUrl.replace(/\/+$/, "");
|
||||
}
|
||||
@@ -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(
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
@@ -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,9 +29,14 @@
|
||||
*/
|
||||
|
||||
import { CORS_HEADERS, handleCorsOptions } from "@/shared/utils/cors";
|
||||
import { stripSensitiveResponseHeaders } from "@omniroute/open-sse/utils/upstreamResponseHeaders";
|
||||
import { createInjectionGuard } from "@/middleware/promptInjectionGuard";
|
||||
import { getRelayTokenByHash, checkRateLimit, recordRelayUsage } from "@/lib/db/relayProxies";
|
||||
import { buildErrorBody } from "@omniroute/open-sse/utils/error";
|
||||
import {
|
||||
buildErrorBody,
|
||||
parseUpstreamError,
|
||||
sanitizeErrorMessage,
|
||||
} from "@omniroute/open-sse/utils/error";
|
||||
import { getProviderPluginManifestHeader } from "@omniroute/open-sse/config/providerPluginManifestUrl.ts";
|
||||
import { z } from "zod";
|
||||
import {
|
||||
@@ -299,7 +304,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 +330,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 + "...");
|
||||
|
||||
@@ -244,6 +244,7 @@
|
||||
"tests/unit/embeddings-auth.test.ts",
|
||||
"tests/unit/error-classification.test.ts",
|
||||
"tests/unit/error-message-sanitization.test.ts",
|
||||
"tests/unit/error-sanitizer-sk-key-qv45.test.ts",
|
||||
"tests/unit/error-sensitive-redaction.test.ts",
|
||||
"tests/unit/execute-chat-resource-pressure-breaker.test.ts",
|
||||
"tests/unit/executor-antigravity.test.ts",
|
||||
|
||||
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)");
|
||||
});
|
||||
});
|
||||
93
tests/unit/error-sanitizer-sk-key-qv45.test.ts
Normal file
93
tests/unit/error-sanitizer-sk-key-qv45.test.ts
Normal file
@@ -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}`
|
||||
);
|
||||
}
|
||||
});
|
||||
});
|
||||
159
tests/unit/search-baseurl-client-override-3f8g.test.ts
Normal file
159
tests/unit/search-baseurl-client-override-3f8g.test.ts
Normal file
@@ -0,0 +1,159 @@
|
||||
/**
|
||||
* GHSA-3f8g-pfh9-j687 — a tenant-supplied `provider_options.baseUrl` redirected
|
||||
* the SaaS search builders, so the OPERATOR's search-provider API key was sent
|
||||
* to a tenant-chosen host (query string for google-pse/searchapi, header for
|
||||
* you.com/linkup/nimble/ollama).
|
||||
*
|
||||
* This is the same override the GHSA-j7j4-g9qc-q69c fix hardened — and that fix
|
||||
* only added a block-metadata check, which does nothing here: the attacker
|
||||
* points at their own PUBLIC host and collects the key. The j7j4 regression test
|
||||
* missed it because its fixture was `searxng-search`, the one keyless provider
|
||||
* where redirecting the base URL leaks no credential.
|
||||
*
|
||||
* The two override sources have different trust:
|
||||
* - `providerSpecificData` comes from the stored provider connection
|
||||
* (`credentials?.providerSpecificData`, search.ts:1510) — OPERATOR config.
|
||||
* This is how an operator points at their self-hosted searxng, so it keeps
|
||||
* working on loopback/LAN under the block-metadata policy.
|
||||
* - `providerOptions` comes straight off the request body
|
||||
* (`body.provider_options`, v1/search/route.ts:366) — TENANT input. It is
|
||||
* refused outright: no provider needs a per-request caller-chosen fetch
|
||||
* target, and honoring one is credential exfiltration for keyed providers
|
||||
* and SSRF-with-readback for every provider.
|
||||
*
|
||||
* Run with:
|
||||
* node --import tsx/esm --test tests/unit/search-baseurl-client-override-3f8g.test.ts
|
||||
*/
|
||||
|
||||
import { describe, it } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
import { resolveSearchBaseUrl } from "../../open-sse/handlers/search.ts";
|
||||
import type { SearchProviderConfig } from "../../open-sse/config/searchRegistry.ts";
|
||||
import { SEARCH_PROVIDERS } from "../../open-sse/config/searchRegistry.ts";
|
||||
|
||||
const base = { query: "test", searchType: "web", maxResults: 5 };
|
||||
|
||||
/** Every provider whose builder attaches an operator-held key. */
|
||||
const KEYED_PROVIDER_IDS = Object.values(SEARCH_PROVIDERS)
|
||||
.filter((p) => p.authType === "apikey")
|
||||
.map((p) => p.id);
|
||||
|
||||
describe("resolveSearchBaseUrl — tenant-supplied baseUrl (GHSA-3f8g-pfh9-j687)", () => {
|
||||
it("covers every keyed provider in the registry, not one hand-picked fixture", () => {
|
||||
// The j7j4 test only exercised searxng. Assert the registry actually has
|
||||
// keyed providers so this suite cannot silently degrade to zero coverage.
|
||||
assert.ok(
|
||||
KEYED_PROVIDER_IDS.length >= 5,
|
||||
`expected keyed providers, got ${KEYED_PROVIDER_IDS.length}`
|
||||
);
|
||||
});
|
||||
|
||||
for (const id of KEYED_PROVIDER_IDS) {
|
||||
it(`refuses a tenant baseUrl for the keyed provider ${id}`, () => {
|
||||
const config = SEARCH_PROVIDERS[id];
|
||||
assert.throws(
|
||||
() =>
|
||||
resolveSearchBaseUrl(config, {
|
||||
...base,
|
||||
providerOptions: { baseUrl: "https://attacker.example" },
|
||||
}),
|
||||
`${id} honored a tenant baseUrl — the operator key would be sent there`
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
it("the opt-in flag is NEVER set on a provider that carries an operator key", () => {
|
||||
// The whole invariant in one assertion: if this ever pairs with authType
|
||||
// "apikey", a caller can redirect the operator's key again.
|
||||
for (const p of Object.values(SEARCH_PROVIDERS)) {
|
||||
if (p.allowClientBaseUrlOverride) {
|
||||
assert.equal(p.authType, "none", `${p.id} opts into a caller baseUrl while holding a key`);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it("refuses a caller baseUrl for keyless providers that did NOT opt in", () => {
|
||||
for (const id of ["context7", "duckduckgo-free"]) {
|
||||
const config = SEARCH_PROVIDERS[id];
|
||||
if (!config || config.allowClientBaseUrlOverride) continue;
|
||||
assert.throws(
|
||||
() =>
|
||||
resolveSearchBaseUrl(config, {
|
||||
...base,
|
||||
providerOptions: { baseUrl: "https://attacker.example" },
|
||||
}),
|
||||
`${id} honored a caller baseUrl without opting in`
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
it("SearXNG keeps its documented self-hosted caller override, IMDS still blocked", () => {
|
||||
// Opted in: keyless, so no operator credential travels with the request.
|
||||
// Loopback/LAN is the point of a self-hosted instance (search-route.test.ts
|
||||
// covers the route-level flow); cloud metadata stays rejected.
|
||||
const config = SEARCH_PROVIDERS["searxng-search"];
|
||||
assert.equal(config.allowClientBaseUrlOverride, true);
|
||||
assert.equal(
|
||||
resolveSearchBaseUrl(config, {
|
||||
...base,
|
||||
providerOptions: { baseUrl: "http://127.0.0.1:8888/search" },
|
||||
}),
|
||||
"http://127.0.0.1:8888/search"
|
||||
);
|
||||
assert.throws(() =>
|
||||
resolveSearchBaseUrl(config, {
|
||||
...base,
|
||||
providerOptions: { baseUrl: "http://169.254.169.254/latest/meta-data/" },
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
it("keeps the operator-configured override working, including loopback/LAN", () => {
|
||||
const config = SEARCH_PROVIDERS["searxng-search"];
|
||||
assert.equal(
|
||||
resolveSearchBaseUrl(config, {
|
||||
...base,
|
||||
providerSpecificData: { baseUrl: "http://127.0.0.1:8888/search" },
|
||||
}),
|
||||
"http://127.0.0.1:8888/search"
|
||||
);
|
||||
assert.equal(
|
||||
resolveSearchBaseUrl(config, {
|
||||
...base,
|
||||
providerSpecificData: { baseUrl: "http://10.0.0.5:8888" },
|
||||
}),
|
||||
"http://10.0.0.5:8888"
|
||||
);
|
||||
});
|
||||
|
||||
it("still blocks cloud metadata from the operator source (j7j4 must not regress)", () => {
|
||||
const config = SEARCH_PROVIDERS["searxng-search"];
|
||||
assert.throws(() =>
|
||||
resolveSearchBaseUrl(config, {
|
||||
...base,
|
||||
providerSpecificData: { baseUrl: "http://169.254.169.254/latest/meta-data/" },
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
it("the operator source wins over a tenant one instead of the tenant shadowing it", () => {
|
||||
const config = SEARCH_PROVIDERS["searxng-search"];
|
||||
assert.equal(
|
||||
resolveSearchBaseUrl(config, {
|
||||
...base,
|
||||
providerOptions: { baseUrl: "https://attacker.example" },
|
||||
providerSpecificData: { baseUrl: "http://127.0.0.1:8888" },
|
||||
}),
|
||||
"http://127.0.0.1:8888"
|
||||
);
|
||||
});
|
||||
|
||||
it("falls back to the catalog baseUrl when neither source supplies one", () => {
|
||||
const config: SearchProviderConfig = {
|
||||
...SEARCH_PROVIDERS["searxng-search"],
|
||||
baseUrl: "http://localhost:8888/search",
|
||||
};
|
||||
assert.equal(resolveSearchBaseUrl(config, base), "http://localhost:8888/search");
|
||||
});
|
||||
});
|
||||
@@ -58,18 +58,26 @@ describe("resolveSearchBaseUrl — SSRF guard on client-controlled baseUrl (GHSA
|
||||
});
|
||||
}
|
||||
|
||||
it("still allows a self-hosted loopback/LAN override (block-metadata, not public-only)", () => {
|
||||
// CORRECTED for GHSA-3f8g-pfh9-j687. This case originally asserted the
|
||||
// loopback/LAN override via `providerOptions` — i.e. via TENANT input — which
|
||||
// is the SSRF-with-readback half of that advisory. The self-hosted use case
|
||||
// this test was written to protect is operator configuration, and it arrives
|
||||
// on `providerSpecificData` (search.ts reads it from the stored connection's
|
||||
// `credentials?.providerSpecificData`), so that is where it is asserted now.
|
||||
// Tenant-supplied overrides are refused outright — see
|
||||
// search-baseurl-client-override-3f8g.test.ts.
|
||||
it("still allows a self-hosted loopback/LAN override from OPERATOR config", () => {
|
||||
assert.equal(
|
||||
resolveSearchBaseUrl(config, {
|
||||
...base,
|
||||
providerOptions: { baseUrl: "http://127.0.0.1:9999" },
|
||||
providerSpecificData: { baseUrl: "http://127.0.0.1:9999" },
|
||||
}),
|
||||
"http://127.0.0.1:9999"
|
||||
);
|
||||
assert.equal(
|
||||
resolveSearchBaseUrl(config, {
|
||||
...base,
|
||||
providerOptions: { baseUrl: "http://10.0.0.5:8080" },
|
||||
providerSpecificData: { baseUrl: "http://10.0.0.5:8080" },
|
||||
}),
|
||||
"http://10.0.0.5:8080"
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user