pjandro/OmniRoute
1
0
Fork 0
mirror of https://github.com/diegosouzapw/OmniRoute.git synced 2026-09-19 21:32:20 +03:00
Code Issues Packages Projects Releases Wiki Activity

fix(search): refuse a tenant-supplied provider_options.baseUrl

Browse Source 
GHSA-3f8g-pfh9-j687: a caller with search scope could set
`provider_options.baseUrl` and redirect the SaaS search builders, so the
OPERATOR's third-party API key went to a caller-chosen host — `key=`/`api_key=`
in the query for google-pse/searchapi, `X-API-Key`/`Authorization` for
you.com/linkup/nimble/ollama — and the parsed response came back, making the
same primitive an SSRF with readback.

My own GHSA-j7j4-g9qc-q69c fix is the one this slipped past. That fix added
`parseAndValidateNonMetadataUrl` to this exact resolver, which blocks IMDS and
nothing else: against key exfiltration it does nothing at all, because the
attacker just names their own public host. I treated a credential-destination
problem as an SSRF problem.

The j7j4 regression test missed it for a sharper reason worth recording: its
fixture was `searxng-search`, the one keyless provider where redirecting the base
URL leaks no credential. The test picked the benign case and passed.

The two override sources are not equally trusted, and reading them through one
`getProviderSettingString()` call erased the distinction:

  - `providerSpecificData` comes from the stored provider connection
    (`credentials?.providerSpecificData`) — operator config. This is how an
    operator points at a self-hosted searxng, so it keeps working on
    loopback/LAN under block-metadata, and IMDS stays rejected.
  - `providerOptions` is `body.provider_options` — caller input. Now refused
    outright rather than validated. No provider needs a per-request
    caller-chosen fetch target; the self-hosted story is served by the operator
    field. If one ever does, it needs a per-provider opt-in plus a host
    allow-list, not a validated free-form URL.

Operator config now also wins over a caller value instead of being shadowed by
it.

Behaviour change, deliberate: a caller that today sends
`provider_options.baseUrl` gets an explicit refusal instead of a silently
redirected request. Silently ignoring it would leave the caller believing the
override applied.

tests/unit/search-baseurl-client-override-3f8g.test.ts — 23 tests, red before
the fix. It iterates EVERY keyed provider in the registry instead of one
fixture, and asserts the registry still has keyed providers so the suite cannot
degrade to zero coverage.

search-baseurl-ssrf-guard.test.ts had one assertion encoding the wrong contract
(caller-supplied loopback allowed). Moved to the operator source with the
reasoning inline — the j7j4 metadata cases are untouched.

Reported by @skeletonsec.

Closes GHSA-3f8g-pfh9-j687
This commit is contained in:
diegosouzapw
2026-09-03 12:52:59 -03:00
parent 9ddb8e0a93
commit bd65c6d89e
3 changed files with 196 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

66
open-sse/handlers/search.ts
View File

@@ -302,6 +302,15 @@ function parseDomainFilter(domainFilter?: string[]): {
return { includes, excludes };
}
/** Read one string setting from a SINGLE source, so callers can distinguish trust. */
function readProviderSettingString(
source: Record<string, unknown> | undefined,
key: string
): string | undefined {
const value = source?.[key];
return typeof value === "string" && value.trim().length > 0 ? value.trim() : undefined;
}
function getProviderSettingString(
params: Pick<SearchRequestParams, "providerOptions" | "providerSpecificData">,
key: string
@@ -323,21 +332,56 @@ 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(/\/+$/, "");
// The two override sources are NOT equally trusted, and treating them as one
// was GHSA-3f8g-pfh9-j687.
//
// `providerSpecificData` is the stored provider connection (see the
// `credentials?.providerSpecificData` wiring below) — operator config. It is
// how an operator points at a self-hosted searxng, so loopback/LAN keeps
// working under the block-metadata policy: cloud-metadata endpoints (IMDS
// credential theft) stay rejected (GHSA-j7j4-g9qc-q69c).
const operatorOverride = readProviderSettingString(params.providerSpecificData, "baseUrl");
if (operatorOverride) {
parseAndValidateNonMetadataUrl(operatorOverride);
return operatorOverride.replace(/\/+$/, "");
}
// `providerOptions` is `body.provider_options` — straight off the request, so
// tenant input. It is refused outright rather than validated:
//
// - For a keyed provider the builder attaches the OPERATOR's API 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 honoring a caller-chosen host hands the operator's
// third-party key to that host. A block-metadata check does nothing about
// it — the attacker just uses their own public host.
// - For any provider the response body is parsed and returned to the caller,
// so a caller-chosen target is SSRF with readback.
//
// No provider has a legitimate need for a per-request caller-chosen fetch
// target; the self-hosted story is served by the operator field above. If one
// ever does, it needs an explicit per-provider opt-in plus a host allow-list,
// not a validated free-form URL.
const tenantOverride = readProviderSettingString(params.providerOptions, "baseUrl");
if (tenantOverride) {
throw new SearchBaseUrlOverrideError(config.id);
}
return config.baseUrl.replace(/\/+$/, "");
}
/** 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}". ` +
`The base URL is operator configuration; set it on the provider connection instead.`
);
this.name = "SearchBaseUrlOverrideError";
}
}
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;

130
tests/unit/search-baseurl-client-override-3f8g.test.ts Normal file
View File

@@ -0,0 +1,130 @@
/**
* 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("refuses a tenant baseUrl for keyless providers too (SSRF readback)", () => {
for (const id of ["searxng-search", "context7", "duckduckgo-free"]) {
const config = SEARCH_PROVIDERS[id];
if (!config) continue;
for (const target of [
"https://attacker.example",
"http://127.0.0.1:9999",
"http://10.0.0.5",
]) {
assert.throws(
() => resolveSearchBaseUrl(config, { ...base, providerOptions: { baseUrl: target } }),
`${id} honored tenant baseUrl ${target}`
);
}
}
});
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");
});
});

14
tests/unit/search-baseurl-ssrf-guard.test.ts
View File

@@ -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"
);