From bcc1b3ee5781d8e7a75ce4fc53462f12d2ec4096 Mon Sep 17 00:00:00 2001 From: JUN Date: Mon, 14 Sep 2026 05:14:03 +0900 Subject: [PATCH 1/2] fix(web-search): assess the bridge search endpoint as a destination (#4519) providers..webSearchBridge.endpoint names the URL that receives that provider's own API key as a Bearer token when the ollama bridge backend runs a search. Two checks stood in front of it and neither was a destination assessment: providerWebSearchBridgeConfigError did new URL plus an http/https protocol test, and resolveOllamaWebSearchEndpoint returned the configured value whenever originOf parsed it. Provider baseUrl has had the real assessment for a long time; the endpoint had none, so endpoint: "http://169.254.169.254/latest/meta-data" was accepted and the key was sent there. Both boundaries now run the existing providerDestinationConfigError. Metadata destinations are refused unconditionally. Loopback, localhost and private space are refused unless the provider sets allowPrivateNetwork or its name is a registry entry that is local by definition, which is what keeps a self-hosted Ollama working. The plan-time check is the load-bearing one, not a second opinion. A hand-edited config file, ocx config set and ocx config import all reach configSchema only and never call providerWebSearchBridgeConfigError, and resolveOllamaWebSearchEndpoint is the only reader of this field in the tree, so a value that survives file load still cannot be spent. It refuses silently by design; config-time is where the operator is told why. Both checks are synchronous and literal-only and resolve no DNS, so a hostname that resolves into metadata or private space still passes. No new classifier was written and no DNS was added at this boundary. --- src/config.ts | 29 ++- src/server/auth-cors.ts | 2 +- src/server/responses/core.ts | 1 + src/web-search/passthrough-bridge.ts | 26 +- structure/runtime.md | 19 ++ .../web-search-passthrough-bridge.test.ts | 223 ++++++++++++++++-- 6 files changed, 276 insertions(+), 24 deletions(-) diff --git a/src/config.ts b/src/config.ts index 19642652c8..601a70bbc2 100644 --- a/src/config.ts +++ b/src/config.ts @@ -507,9 +507,15 @@ export function requestPacingConfigError(value: unknown): string | null { /** * Bounds for the opt-in passthrough web-search bridge (`providers..webSearchBridge`, * #3761). Strict for the same reason `retryOn429` is: a misspelled key here would silently - * leave the bridge disarmed while the operator believes they enabled it. `endpoint` is only - * shape-checked here; `planPassthroughWebSearchBridge` re-validates the origin before any key - * is sent to it, because config validation is not an authorization boundary. + * leave the bridge disarmed while the operator believes they enabled it. + * + * `endpoint` names the destination that receives this provider's API key, so it gets the same + * literal destination assessment `baseUrl` gets (#4519) — see `providerWebSearchBridgeConfigError` + * below. This schema itself still only shape-checks: it is `.catch(undefined)` at the provider + * row, and a hand-edited config file never reaches the error function at all. The authorization + * boundary is therefore `resolveOllamaWebSearchEndpoint`, which runs the same assessment and is + * the only reader of this field in the tree; config validation is where an operator is told why, + * not what makes the value safe. */ const providerWebSearchBridgeSchema = z.object({ enabled: z.boolean().optional(), @@ -519,7 +525,11 @@ const providerWebSearchBridgeSchema = z.object({ endpoint: z.string().min(1).optional(), }).strict(); -export function providerWebSearchBridgeConfigError(value: unknown): string | null { +export function providerWebSearchBridgeConfigError( + value: unknown, + providerName: string, + provider: Pick, +): string | null { if (value === undefined) return null; if (!value || typeof value !== "object" || Array.isArray(value)) { return "webSearchBridge must be a plain object"; @@ -541,6 +551,17 @@ export function providerWebSearchBridgeConfigError(value: unknown): string | nul if (url.protocol !== "https:" && url.protocol !== "http:") { return "webSearchBridge.endpoint must be an absolute http(s) URL"; } + // Same classifier baseUrl uses, so a metadata address is refused outright and loopback or + // private space needs the provider's allowPrivateNetwork opt-in (or a registry entry that is + // local by definition, which is what keeps a self-hosted Ollama working). Literal-only and + // synchronous, exactly as at the baseUrl boundary: no DNS is resolved here. + const destinationError = providerDestinationConfigError(providerName, { + baseUrl: endpoint, + allowPrivateNetwork: provider.allowPrivateNetwork, + }); + if (destinationError) { + return destinationError.replace(/^baseUrl/, "webSearchBridge.endpoint"); + } } return null; } diff --git a/src/server/auth-cors.ts b/src/server/auth-cors.ts index d72d0ff0aa..be6bfd3fca 100644 --- a/src/server/auth-cors.ts +++ b/src/server/auth-cors.ts @@ -766,7 +766,7 @@ export function providerManagementConfigError( if (requestPacingError) { return `provider ${JSON.stringify(redactSecretString(name))} ${requestPacingError}`; } - const webSearchBridgeError = providerWebSearchBridgeConfigError(raw.webSearchBridge); + const webSearchBridgeError = providerWebSearchBridgeConfigError(raw.webSearchBridge, name, typed); if (webSearchBridgeError) { return `provider ${JSON.stringify(redactSecretString(name))} ${webSearchBridgeError}`; } diff --git a/src/server/responses/core.ts b/src/server/responses/core.ts index 3fc8917c5d..1dc58f277a 100644 --- a/src/server/responses/core.ts +++ b/src/server/responses/core.ts @@ -6251,6 +6251,7 @@ async function handleResponsesInner( openAiSidecar, ); const webSearchBridgePlan = planPassthroughWebSearchBridge(parsed, route.provider, { + providerName: route.providerName, isPassthrough: true, stream: parsed.stream === true, auth: webSearchBridgeAuth, diff --git a/src/web-search/passthrough-bridge.ts b/src/web-search/passthrough-bridge.ts index c4e9804e46..ade8624bf9 100644 --- a/src/web-search/passthrough-bridge.ts +++ b/src/web-search/passthrough-bridge.ts @@ -64,6 +64,7 @@ import { resolveSidecarBackend, xaiSearchOptionsFromConfig, } from "./sidecar-providers"; +import { providerDestinationConfigError } from "../lib/destination-policy"; /** Canonical Ollama Cloud origin. The only origin the "ollama" backend derives on its own. */ export const OLLAMA_CLOUD_ORIGIN = "https://ollama.com"; @@ -123,13 +124,28 @@ function originOf(value: string | undefined): string | undefined { * that receives this provider's API key. Without one, the origin must be canonical Ollama Cloud * -- a renamed row pointing at an arbitrary host must not silently receive the key just because * its adapter happens to be openai-responses. + * + * Naming a destination is not the same as it being an allowed one. The endpoint therefore gets the + * same literal destination assessment "baseUrl" already gets (#4519): metadata addresses are + * refused outright, and loopback/private need the provider's "allowPrivateNetwork" opt-in or a + * registry entry that is local by definition, so a local Ollama on 127.0.0.1 keeps working. This + * is the ONLY reader of "webSearchBridge.endpoint" in the tree, which is what lets it act as the + * authorization boundary for a config file the operator edited by hand -- that path never reaches + * "providerWebSearchBridgeConfigError". The refusal is deliberately silent: config-time is where + * an operator is told why, and a value that survives file load simply cannot be spent. */ export function resolveOllamaWebSearchEndpoint( + providerName: string, provider: OcxProviderConfig, ): string | undefined { const configured = provider.webSearchBridge?.endpoint; if (configured !== undefined) { - return originOf(configured) === undefined ? undefined : configured; + if (originOf(configured) === undefined) return undefined; + if (providerDestinationConfigError(providerName, { + baseUrl: configured, + allowPrivateNetwork: provider.allowPrivateNetwork, + })) return undefined; + return configured; } return originOf(provider.baseUrl) === OLLAMA_CLOUD_ORIGIN ? OLLAMA_CLOUD_ORIGIN + OLLAMA_WEB_SEARCH_PATH @@ -211,6 +227,12 @@ export function planPassthroughWebSearchBridge( parsed: OcxParsedRequest, provider: OcxProviderConfig, options: { + /** + * Registry key for this provider. Required rather than optional: the destination assessment + * consults the registry's local-by-default entries, and an absent name would silently pick a + * different answer than the operator configured. + */ + providerName: string; isPassthrough: boolean; stream: boolean; auth?: PassthroughWebSearchBridgeAuth; @@ -239,7 +261,7 @@ export function planPassthroughWebSearchBridge( ? bridge.timeoutMs! : DEFAULT_BRIDGE_TIMEOUT_MS; if (backend === "ollama") { - const endpoint = resolveOllamaWebSearchEndpoint(provider); + const endpoint = resolveOllamaWebSearchEndpoint(options.providerName, provider); if (!endpoint) return undefined; return { backend, endpoint, maxSearches, timeoutMs }; } diff --git a/structure/runtime.md b/structure/runtime.md index 1dd4e456e8..ea80d0a46e 100644 --- a/structure/runtime.md +++ b/structure/runtime.md @@ -261,6 +261,25 @@ value import of the barrel; the barrel re-exports it. `tests/web-search/web-search-passthrough-bridge.test.ts` covers the mismatch and matching cases for anthropic, xai, and gemini, plus the unset-backend default. +`providers..webSearchBridge.endpoint` names the destination that receives that provider's own +API key, so it carries the same literal destination assessment as `baseUrl`: +`providerDestinationConfigError` runs both at management write time, inside +`providerWebSearchBridgeConfigError`, and at plan time inside `resolveOllamaWebSearchEndpoint`. +Metadata destinations are refused unconditionally; loopback, localhost, and private space need the +provider's `allowPrivateNetwork` opt-in or a registry entry that is local by default, which is what +keeps a self-hosted Ollama on `127.0.0.1` working. Both checks are synchronous and literal-only and +resolve no DNS, so a hostname that resolves into metadata or private space is a disclosed residual +rather than a blocked case. That residual is strictly larger than `baseUrl`'s: `baseUrl` also runs +the async `providerDestinationResolvedError` at management write, which the endpoint does not, and +parity there would still leave the hand-edited-file path uncovered because the plan-time boundary is +synchronous. The plan-time check is the +authorization boundary rather than a second opinion: a hand-edited config file, `ocx config set`, +and `ocx config import` all reach `configSchema` only and never call +`providerWebSearchBridgeConfigError`, and `resolveOllamaWebSearchEndpoint` is the only reader of +this field in the tree, so a value that survives file load still cannot be spent. It refuses +silently by design; config-time is where the operator is told why. The planner requires the +provider name for that assessment, so `planPassthroughWebSearchBridge` takes it explicitly. + ## Remote Hub hardening ownership `src/remote/protocol.ts` owns pure interval/feature negotiation. `src/remote/hub-state.ts` owns the `GET|HEAD /v1/hub-state` contract, its caps, and the parser both sides share. `src/client/hub-client.ts` owns bounded, schema-validated remote catalog consumption, hub-state reads, and key-id probes; `src/client/hub-state.ts` owns the resolution and the owner-stamped 0600 cache, and a failed read reports "unavailable" rather than degrading to the client's own local provider and login state. `src/client/hub-relay.ts` is a fixed-authority management relay with URL, header, body, redirect, and stream bounds. The public data listener remains the direct client→hub path; the loopback management ingress never serves data-plane routes. diff --git a/tests/web-search/web-search-passthrough-bridge.test.ts b/tests/web-search/web-search-passthrough-bridge.test.ts index c7d0b8b98f..c6c4606cc3 100644 --- a/tests/web-search/web-search-passthrough-bridge.test.ts +++ b/tests/web-search/web-search-passthrough-bridge.test.ts @@ -21,6 +21,7 @@ import { WEB_SEARCH_BRIDGE_MIXED_TOOLS_ERROR_CODE, type PassthroughWebSearchBridgePlan, } from "../../src/web-search/passthrough-bridge"; +import { providerWebSearchBridgeConfigError } from "../../src/config"; import { mapOllamaSearchResponse } from "../../src/web-search/ollama-executor"; import { UNDECLARED_TOOL_CALL_ERROR_CODE } from "../../src/server/responses-undeclared-tool-guard"; import { handleResponses } from "../../src/server/responses"; @@ -94,6 +95,7 @@ const armed: ProviderWebSearchBridgeConfig = { enabled: true, backend: "ollama" describe("planPassthroughWebSearchBridge arming", () => { test("arms for an enabled ollama-backed key provider on the canonical origin", () => { const plan = planPassthroughWebSearchBridge(parsedFixture(), providerFixture(armed), { + providerName: "gateway", isPassthrough: true, stream: true, }); @@ -113,6 +115,7 @@ describe("planPassthroughWebSearchBridge arming", () => { ]; for (const bridge of off) { expect(planPassthroughWebSearchBridge(parsedFixture(), providerFixture(bridge), { + providerName: "gateway", isPassthrough: true, stream: true, })).toBeUndefined(); @@ -124,28 +127,35 @@ describe("planPassthroughWebSearchBridge arming", () => { expect(planPassthroughWebSearchBridge( parsedFixture(), providerFixture(armed, { authMode }), - { isPassthrough: true, stream: true }, + { providerName: "gateway", isPassthrough: true, stream: true }, )).toBeUndefined(); } }); test("stays disarmed off the passthrough, without hosted web_search, and for non-streaming turns", () => { const provider = providerFixture(armed); - expect(planPassthroughWebSearchBridge(parsedFixture(), provider, { isPassthrough: false, stream: true })) - .toBeUndefined(); + expect(planPassthroughWebSearchBridge(parsedFixture(), provider, { + providerName: "gateway", + isPassthrough: false, + stream: true, + })).toBeUndefined(); expect(planPassthroughWebSearchBridge(parsedFixture({ _webSearch: undefined }), provider, { + providerName: "gateway", isPassthrough: true, stream: true, })).toBeUndefined(); - expect(planPassthroughWebSearchBridge(parsedFixture(), provider, { isPassthrough: true, stream: false })) - .toBeUndefined(); + expect(planPassthroughWebSearchBridge(parsedFixture(), provider, { + providerName: "gateway", + isPassthrough: true, + stream: false, + })).toBeUndefined(); }); test("a tool_choice that excludes search excludes the bridge", () => { expect(planPassthroughWebSearchBridge( parsedFixture({ options: { toolChoice: { type: "function", name: "exec" } } }), providerFixture(armed), - { isPassthrough: true, stream: true }, + { providerName: "gateway", isPassthrough: true, stream: true }, )).toBeUndefined(); }); @@ -154,22 +164,26 @@ describe("planPassthroughWebSearchBridge arming", () => { expect(planPassthroughWebSearchBridge( parsedFixture(), providerFixture({ enabled: true, backend }), - { isPassthrough: true, stream: true }, + { providerName: "gateway", isPassthrough: true, stream: true }, )).toBeUndefined(); } }); test("the ollama backend refuses a non-canonical origin unless the operator names the endpoint", () => { const renamed = providerFixture(armed, { baseUrl: "https://gateway.example/v1" }); - expect(resolveOllamaWebSearchEndpoint(renamed)).toBeUndefined(); - expect(planPassthroughWebSearchBridge(parsedFixture(), renamed, { isPassthrough: true, stream: true })) - .toBeUndefined(); + expect(resolveOllamaWebSearchEndpoint("gateway", renamed)).toBeUndefined(); + expect(planPassthroughWebSearchBridge(parsedFixture(), renamed, { + providerName: "gateway", + isPassthrough: true, + stream: true, + })).toBeUndefined(); const operatorSet = providerFixture( { enabled: true, backend: "ollama", endpoint: "https://search.internal/api/web_search" }, { baseUrl: "https://gateway.example/v1" }, ); const plan = planPassthroughWebSearchBridge(parsedFixture(), operatorSet, { + providerName: "gateway", isPassthrough: true, stream: true, }); @@ -180,7 +194,7 @@ describe("planPassthroughWebSearchBridge arming", () => { const plan = planPassthroughWebSearchBridge( parsedFixture(), providerFixture({ enabled: true, backend: "ollama", maxSearches: 99, timeoutMs: 1 }), - { isPassthrough: true, stream: true }, + { providerName: "gateway", isPassthrough: true, stream: true }, ); expect(plan?.maxSearches).toBe(3); expect(plan?.timeoutMs).toBe(60_000); @@ -189,6 +203,7 @@ describe("planPassthroughWebSearchBridge arming", () => { test("an openai backend arms only when the ChatGPT sidecar is present", () => { const provider = providerFixture({ enabled: true, backend: "openai" }, { baseUrl: "https://gateway.example/v1" }); expect(planPassthroughWebSearchBridge(parsedFixture(), provider, { + providerName: "gateway", isPassthrough: true, stream: true, })).toBeUndefined(); @@ -200,6 +215,7 @@ describe("planPassthroughWebSearchBridge arming", () => { headers: new Headers({ authorization: "Bearer chatgpt" }), }; const planned = planPassthroughWebSearchBridge(parsedFixture(), provider, { + providerName: "gateway", isPassthrough: true, stream: true, auth: { openAiSidecar }, @@ -217,33 +233,33 @@ describe("planPassthroughWebSearchBridge arming", () => { expect(planPassthroughWebSearchBridge( parsedFixture(), providerFixture({ enabled: true, backend: "anthropic" }, gateway), - { isPassthrough: true, stream: true, auth: { anthropic } }, + { providerName: "gateway", isPassthrough: true, stream: true, auth: { anthropic } }, )?.backend).toBe("anthropic"); expect(planPassthroughWebSearchBridge( parsedFixture(), providerFixture({ enabled: true, backend: "xai" }, gateway), - { isPassthrough: true, stream: true, auth: { xai } }, + { providerName: "gateway", isPassthrough: true, stream: true, auth: { xai } }, )?.backend).toBe("xai"); expect(planPassthroughWebSearchBridge( parsedFixture(), providerFixture({ enabled: true, backend: "gemini" }, gateway), - { isPassthrough: true, stream: true, auth: { gemini } }, + { providerName: "gateway", isPassthrough: true, stream: true, auth: { gemini } }, )?.backend).toBe("gemini"); expect(planPassthroughWebSearchBridge( parsedFixture(), providerFixture({ enabled: true, backend: "exa" }, gateway), - { isPassthrough: true, stream: true, auth: { exaApiKey: "exa-canary" } }, + { providerName: "gateway", isPassthrough: true, stream: true, auth: { exaApiKey: "exa-canary" } }, )?.backend).toBe("exa"); // A named backend does not borrow a different credential. expect(planPassthroughWebSearchBridge( parsedFixture(), providerFixture({ enabled: true, backend: "exa" }, gateway), - { isPassthrough: true, stream: true, auth: { anthropic, xai, gemini } }, + { providerName: "gateway", isPassthrough: true, stream: true, auth: { anthropic, xai, gemini } }, )).toBeUndefined(); expect(planPassthroughWebSearchBridge( parsedFixture(), providerFixture({ enabled: true, backend: "openai" }, gateway), - { isPassthrough: true, stream: true, auth: { exaApiKey: "exa-canary" } }, + { providerName: "gateway", isPassthrough: true, stream: true, auth: { exaApiKey: "exa-canary" } }, )).toBeUndefined(); }); @@ -263,6 +279,179 @@ describe("planPassthroughWebSearchBridge arming", () => { }); }); +// webSearchBridge.endpoint names the destination that receives this provider's API key, so it +// gets the same literal destination assessment baseUrl already gets: metadata is refused +// outright, and loopback or private space needs the provider's allowPrivateNetwork opt-in or a +// registry entry that is local by default. Every provider here sits on a non-canonical baseUrl +// so the configured endpoint, not the Ollama Cloud fallback, decides the outcome. +describe("webSearchBridge.endpoint destination policy", () => { + const gateway = { baseUrl: "https://gateway.example/v1" }; + + test("a configured metadata endpoint disarms the bridge", () => { + const provider = providerFixture( + { enabled: true, backend: "ollama", endpoint: "http://169.254.169.254/latest/meta-data" }, + gateway, + ); + expect(resolveOllamaWebSearchEndpoint("gateway", provider)).toBeUndefined(); + expect(planPassthroughWebSearchBridge(parsedFixture(), provider, { + providerName: "gateway", + isPassthrough: true, + stream: true, + })).toBeUndefined(); + }); + + test("allowPrivateNetwork does not waive a metadata endpoint", () => { + const provider = providerFixture( + { enabled: true, backend: "ollama", endpoint: "http://169.254.169.254/latest/meta-data" }, + { ...gateway, allowPrivateNetwork: true }, + ); + expect(resolveOllamaWebSearchEndpoint("gateway", provider)).toBeUndefined(); + expect(planPassthroughWebSearchBridge(parsedFixture(), provider, { + providerName: "gateway", + isPassthrough: true, + stream: true, + })).toBeUndefined(); + }); + + test("the Aliyun metadata address stays refused under the opt-in", () => { + const provider = providerFixture( + { enabled: true, backend: "ollama", endpoint: "http://100.100.100.200/api/web_search" }, + { ...gateway, allowPrivateNetwork: true }, + ); + expect(resolveOllamaWebSearchEndpoint("gateway", provider)).toBeUndefined(); + expect(planPassthroughWebSearchBridge(parsedFixture(), provider, { + providerName: "gateway", + isPassthrough: true, + stream: true, + })).toBeUndefined(); + }); + + test("a private-network endpoint stays disarmed without the opt-in", () => { + const provider = providerFixture( + { enabled: true, backend: "ollama", endpoint: "http://10.0.0.5/api/web_search" }, + gateway, + ); + expect(planPassthroughWebSearchBridge(parsedFixture(), provider, { + providerName: "gateway", + isPassthrough: true, + stream: true, + })).toBeUndefined(); + }); + + test("allowPrivateNetwork arms a private-network endpoint", () => { + const provider = providerFixture( + { enabled: true, backend: "ollama", endpoint: "http://10.0.0.5/api/web_search" }, + { ...gateway, allowPrivateNetwork: true }, + ); + expect(resolveOllamaWebSearchEndpoint("gateway", provider)).toBe("http://10.0.0.5/api/web_search"); + expect(planPassthroughWebSearchBridge(parsedFixture(), provider, { + providerName: "gateway", + isPassthrough: true, + stream: true, + })?.endpoint).toBe("http://10.0.0.5/api/web_search"); + }); + + test("a self-hosted ollama keeps its loopback endpoint because the registry entry is local by default", () => { + const provider = providerFixture( + { enabled: true, backend: "ollama", endpoint: "http://127.0.0.1:11434/api/web_search" }, + gateway, + ); + expect(resolveOllamaWebSearchEndpoint("ollama", provider)).toBe("http://127.0.0.1:11434/api/web_search"); + expect(planPassthroughWebSearchBridge(parsedFixture(), provider, { + providerName: "ollama", + isPassthrough: true, + stream: true, + })?.endpoint).toBe("http://127.0.0.1:11434/api/web_search"); + }); + + test("the same loopback endpoint is refused under a name with no registry default", () => { + const provider = providerFixture( + { enabled: true, backend: "ollama", endpoint: "http://127.0.0.1:11434/api/web_search" }, + gateway, + ); + expect(planPassthroughWebSearchBridge(parsedFixture(), provider, { + providerName: "gateway", + isPassthrough: true, + stream: true, + })).toBeUndefined(); + }); + + test("a local-by-default registry name also covers private space, not just loopback", () => { + // allowPrivateNetworkByDefault is not loopback-only; it is the same waiver baseUrl gets, so a + // LAN Ollama arms too. Pinned because the rule is broader than the 127.0.0.1 case suggests. + const provider = providerFixture( + { enabled: true, backend: "ollama", endpoint: "http://10.0.0.5:11434/api/web_search" }, + gateway, + ); + expect(resolveOllamaWebSearchEndpoint("ollama", provider)).toBe("http://10.0.0.5:11434/api/web_search"); + expect(planPassthroughWebSearchBridge(parsedFixture(), provider, { + providerName: "ollama", + isPassthrough: true, + stream: true, + })?.endpoint).toBe("http://10.0.0.5:11434/api/web_search"); + }); + + test("a public endpoint still arms", () => { + const provider = providerFixture( + { enabled: true, backend: "ollama", endpoint: "https://ollama.com/api/web_search" }, + gateway, + ); + expect(planPassthroughWebSearchBridge(parsedFixture(), provider, { + providerName: "gateway", + isPassthrough: true, + stream: true, + })?.endpoint).toBe("https://ollama.com/api/web_search"); + }); + + test("a hostname that merely resembles a metadata address still arms", () => { + // The synchronous classifier is literal-only and resolves no DNS, exactly as at the baseUrl + // boundary, so a lookalike hostname is just a hostname here. + const provider = providerFixture( + { enabled: true, backend: "ollama", endpoint: "https://imds.example.test/latest/meta-data" }, + gateway, + ); + expect(planPassthroughWebSearchBridge(parsedFixture(), provider, { + providerName: "gateway", + isPassthrough: true, + stream: true, + })?.endpoint).toBe("https://imds.example.test/latest/meta-data"); + }); +}); + +describe("providerWebSearchBridgeConfigError endpoint destination policy", () => { + test("names webSearchBridge.endpoint rather than baseUrl in a metadata refusal", () => { + const value = { enabled: true, backend: "ollama", endpoint: "http://169.254.169.254/latest/meta-data" }; + const error = providerWebSearchBridgeConfigError(value, "gateway", {}); + expect(error).toContain("webSearchBridge.endpoint"); + expect(error).toContain("metadata"); + expect(error).not.toStartWith("baseUrl"); + expect(providerWebSearchBridgeConfigError(value, "gateway", { allowPrivateNetwork: true })).not.toBeNull(); + }); + + test("a private-network endpoint errors without the opt-in and passes with it", () => { + const value = { enabled: true, backend: "ollama", endpoint: "http://10.0.0.5/api/web_search" }; + expect(providerWebSearchBridgeConfigError(value, "gateway", {})).toContain("allowPrivateNetwork"); + expect(providerWebSearchBridgeConfigError(value, "gateway", { allowPrivateNetwork: true })).toBeNull(); + }); + + test("a public endpoint and an absent endpoint both pass", () => { + expect(providerWebSearchBridgeConfigError( + { enabled: true, backend: "ollama", endpoint: "https://ollama.com/api/web_search" }, + "gateway", + {}, + )).toBeNull(); + expect(providerWebSearchBridgeConfigError({ enabled: true, backend: "ollama" }, "gateway", {})).toBeNull(); + }); + + test("the shape check still runs before the destination check", () => { + expect(providerWebSearchBridgeConfigError( + { enabled: true, backend: "ollama", endpoint: "not-a-url" }, + "gateway", + {}, + )).toBe("webSearchBridge.endpoint must be an absolute http(s) URL"); + }); +}); + const plan: PassthroughWebSearchBridgePlan = { backend: "ollama", endpoint: "https://ollama.com/api/web_search", From e8b36b0e202025780e84759542a78cb1488b2333 Mon Sep 17 00:00:00 2001 From: JUN Date: Mon, 14 Sep 2026 05:31:20 +0900 Subject: [PATCH 2/2] fix(web-search): tell the operator when a bridge endpoint is refused A provider keyed under a custom name, say "my-ollama", pointing at http://127.0.0.1:11434/api/web_search armed before the destination check and disarms after it, because only the registry ids (ollama, vllm, lm-studio, litellm) are local by default. Two properties combined to make that invisible: config load never runs providerWebSearchBridgeConfigError, so the block loads cleanly, and the plan-time refusal returns undefined by design so the key stays unspent. The operator's web search stopped working with no signal at all. resolveOllamaWebSearchEndpoint now warns once per provider and endpoint when the refusal is a destination decision, naming the two remedies: set allowPrivateNetwork, or key the provider under its registry id. The planner runs per request, so the warning is deduplicated and the dedupe set is bounded. The destination URL is never logged and the provider key goes through redactSecretString, since a provider key is caller-controlled. Adds the coverage the review named as missing: a metadata endpoint survives validateConfigCandidate intact and is then refused at plan time, which pins the load-path behavior the whole argument rests on rather than simulating it. --- src/web-search/passthrough-bridge.ts | 52 ++++++++- .../web-search-passthrough-bridge.test.ts | 102 +++++++++++++++++- 2 files changed, 150 insertions(+), 4 deletions(-) diff --git a/src/web-search/passthrough-bridge.ts b/src/web-search/passthrough-bridge.ts index ade8624bf9..8850b7e2ac 100644 --- a/src/web-search/passthrough-bridge.ts +++ b/src/web-search/passthrough-bridge.ts @@ -65,11 +65,52 @@ import { xaiSearchOptionsFromConfig, } from "./sidecar-providers"; import { providerDestinationConfigError } from "../lib/destination-policy"; +import { redactSecretString } from "../lib/redact"; /** Canonical Ollama Cloud origin. The only origin the "ollama" backend derives on its own. */ export const OLLAMA_CLOUD_ORIGIN = "https://ollama.com"; const OLLAMA_WEB_SEARCH_PATH = "/api/web_search"; +/** + * Providers already warned about a destination-refused bridge endpoint. The planner runs per + * request, so without this a refused endpoint would warn on every turn. Keyed on provider plus + * endpoint so that editing the config warns again; the key itself is never logged. + */ +const warnedRefusedBridgeEndpoints = new Set(); +/** Bound the dedupe set so a pathological config cannot grow it without limit. */ +const MAX_WARNED_REFUSED_ENDPOINTS = 64; + +/** + * A refused endpoint disarms the bridge, and the refusal itself has to stay silent at the point of + * use -- returning undefined is what keeps the key unspent. But silence alone made a real + * configuration fail invisibly: a provider keyed under a CUSTOM name (say "my-ollama") pointing at + * a loopback endpoint used to arm, and the destination policy now refuses it because only the + * registry ids are local by default. The config file never reaches + * "providerWebSearchBridgeConfigError", so nothing else would tell the operator. One warning per + * provider and endpoint gives them the remedy without leaking the destination: the URL is + * deliberately omitted and the provider name is redacted, because a provider key is + * caller-controlled and can be token-shaped. + */ +function warnRefusedBridgeEndpointOnce(providerName: string, endpoint: string): void { + const key = providerName + "\u0000" + endpoint; + if (warnedRefusedBridgeEndpoints.has(key)) return; + if (warnedRefusedBridgeEndpoints.size >= MAX_WARNED_REFUSED_ENDPOINTS) { + warnedRefusedBridgeEndpoints.clear(); + } + warnedRefusedBridgeEndpoints.add(key); + console.warn( + "[web-search] provider " + JSON.stringify(redactSecretString(providerName)) + + " webSearchBridge.endpoint was refused by destination policy, so the bridge stays disarmed." + + " Set allowPrivateNetwork:true for an intentionally local endpoint, or key the provider under" + + " its registry id (ollama, vllm, lm-studio, litellm).", + ); +} + +/** Test seam: the dedupe is process-wide, so a test that asserts the warning must reset it. */ +export function resetRefusedBridgeEndpointWarningsForTests(): void { + warnedRefusedBridgeEndpoints.clear(); +} + const DEFAULT_BRIDGE_MAX_SEARCHES = 3; const DEFAULT_BRIDGE_TIMEOUT_MS = 60_000; /** Queries honored from one call's "queries" array; the rest are ignored rather than billed. */ @@ -131,8 +172,10 @@ function originOf(value: string | undefined): string | undefined { * registry entry that is local by definition, so a local Ollama on 127.0.0.1 keeps working. This * is the ONLY reader of "webSearchBridge.endpoint" in the tree, which is what lets it act as the * authorization boundary for a config file the operator edited by hand -- that path never reaches - * "providerWebSearchBridgeConfigError". The refusal is deliberately silent: config-time is where - * an operator is told why, and a value that survives file load simply cannot be spent. + * "providerWebSearchBridgeConfigError", so a value that survives file load simply cannot be spent. + * The refusal returns undefined rather than an error, because disarming is what keeps the key + * unspent -- but it is not silent: see warnRefusedBridgeEndpointOnce for why a custom-named local + * provider has to be told, once, that its endpoint was refused and how to re-authorize it. */ export function resolveOllamaWebSearchEndpoint( providerName: string, @@ -144,7 +187,10 @@ export function resolveOllamaWebSearchEndpoint( if (providerDestinationConfigError(providerName, { baseUrl: configured, allowPrivateNetwork: provider.allowPrivateNetwork, - })) return undefined; + })) { + warnRefusedBridgeEndpointOnce(providerName, configured); + return undefined; + } return configured; } return originOf(provider.baseUrl) === OLLAMA_CLOUD_ORIGIN diff --git a/tests/web-search/web-search-passthrough-bridge.test.ts b/tests/web-search/web-search-passthrough-bridge.test.ts index c6c4606cc3..1af8a45955 100644 --- a/tests/web-search/web-search-passthrough-bridge.test.ts +++ b/tests/web-search/web-search-passthrough-bridge.test.ts @@ -15,13 +15,14 @@ import { planPassthroughWebSearchBridge, resolveOllamaWebSearchEndpoint, resolvePassthroughWebSearchBridgeAuth, + resetRefusedBridgeEndpointWarningsForTests, shouldResolveOpenAiPassthroughWebSearchBridge, sidecarSettingsForBridge, WEB_SEARCH_BRIDGE_ERROR_CODE, WEB_SEARCH_BRIDGE_MIXED_TOOLS_ERROR_CODE, type PassthroughWebSearchBridgePlan, } from "../../src/web-search/passthrough-bridge"; -import { providerWebSearchBridgeConfigError } from "../../src/config"; +import { providerWebSearchBridgeConfigError, validateConfigCandidate } from "../../src/config"; import { mapOllamaSearchResponse } from "../../src/web-search/ollama-executor"; import { UNDECLARED_TOOL_CALL_ERROR_CODE } from "../../src/server/responses-undeclared-tool-guard"; import { handleResponses } from "../../src/server/responses"; @@ -418,6 +419,105 @@ describe("webSearchBridge.endpoint destination policy", () => { }); }); +// The refusal disarms the bridge without an error, which is what keeps the key unspent. That +// silence broke a real configuration: a provider keyed under a CUSTOM name pointing at loopback +// used to arm, and only the registry ids are local by default. The operator has to be told once. +describe("a refused endpoint tells the operator once", () => { + const gateway = { baseUrl: "https://gateway.example/v1" }; + + function captureWarnings(run: () => void): string[] { + const lines: string[] = []; + const saved = console.warn; + console.warn = (...args: unknown[]) => { lines.push(args.map(String).join(" ")); }; + try { + run(); + } finally { + console.warn = saved; + } + return lines; + } + + test("a custom-named local provider is warned, with the remedy and without the endpoint", () => { + resetRefusedBridgeEndpointWarningsForTests(); + const provider = providerFixture( + { enabled: true, backend: "ollama", endpoint: "http://127.0.0.1:11434/api/web_search" }, + gateway, + ); + const warnings = captureWarnings(() => { + expect(resolveOllamaWebSearchEndpoint("my-ollama", provider)).toBeUndefined(); + }); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain("my-ollama"); + expect(warnings[0]).toContain("allowPrivateNetwork"); + // The destination itself never reaches the log. + expect(warnings[0]).not.toContain("127.0.0.1"); + expect(warnings[0]).not.toContain("/api/web_search"); + }); + + test("the same refusal does not warn again on every later request", () => { + resetRefusedBridgeEndpointWarningsForTests(); + const provider = providerFixture( + { enabled: true, backend: "ollama", endpoint: "http://10.0.0.5/api/web_search" }, + gateway, + ); + const warnings = captureWarnings(() => { + for (let i = 0; i < 5; i += 1) { + expect(planPassthroughWebSearchBridge(parsedFixture(), provider, { + providerName: "local-llm", + isPassthrough: true, + stream: true, + })).toBeUndefined(); + } + }); + expect(warnings).toHaveLength(1); + }); + + test("an accepted endpoint is not warned about", () => { + resetRefusedBridgeEndpointWarningsForTests(); + const provider = providerFixture( + { enabled: true, backend: "ollama", endpoint: "http://127.0.0.1:11434/api/web_search" }, + gateway, + ); + const warnings = captureWarnings(() => { + expect(resolveOllamaWebSearchEndpoint("ollama", provider)).toBe("http://127.0.0.1:11434/api/web_search"); + }); + expect(warnings).toEqual([]); + }); +}); + +// The blocker this policy exists for: config load does NOT run providerWebSearchBridgeConfigError, +// so a metadata endpoint reaches running config intact. Plan time is what refuses to spend it. +describe("a metadata endpoint survives config load and is refused at plan time", () => { + test("configSchema accepts the block and the planner still disarms", () => { + const result = validateConfigCandidate({ + port: 0, + defaultProvider: "gateway", + providers: { + gateway: { + adapter: "openai-responses", + baseUrl: "https://gateway.example/v1", + authMode: "key", + apiKey: "fixture-key", + webSearchBridge: { + enabled: true, + backend: "ollama", + endpoint: "http://169.254.169.254/latest/meta-data", + }, + }, + }, + }); + expect(result.ok).toBe(true); + const loaded = (result as { ok: true; config: OcxConfig }).config.providers.gateway!; + // It really did survive validation, untouched. + expect(loaded.webSearchBridge?.endpoint).toBe("http://169.254.169.254/latest/meta-data"); + expect(planPassthroughWebSearchBridge(parsedFixture(), loaded, { + providerName: "gateway", + isPassthrough: true, + stream: true, + })).toBeUndefined(); + }); +}); + describe("providerWebSearchBridgeConfigError endpoint destination policy", () => { test("names webSearchBridge.endpoint rather than baseUrl in a metadata refusal", () => { const value = { enabled: true, backend: "ollama", endpoint: "http://169.254.169.254/latest/meta-data" };