diff --git a/src/lib/destination-policy.ts b/src/lib/destination-policy.ts index 75818af311..46b2aa91ba 100644 --- a/src/lib/destination-policy.ts +++ b/src/lib/destination-policy.ts @@ -130,6 +130,50 @@ function registryAllowsPrivateNetwork(name: string): boolean { return getProviderRegistryEntry(name)?.allowPrivateNetworkByDefault === true; } +/** + * OAuth registry entries that opt into `allowBaseUrlOverride` send bearer credentials to a + * user-configured endpoint (review findings, PR #2109 / PR #2110): a cleartext `http:` + * override would expose the OAuth token on the wire. `https:` is therefore required for + * every non-local destination. Loopback/localhost/private relays keep working over + * `http:` because they already sit behind the explicit `allowPrivateNetwork` opt-in + * enforced by {@link providerDestinationConfigError}. Keyed/local providers (Ollama, + * vLLM, LM Studio, LiteLLM, Moonshot, Qwen, Alibaba) are untouched: they are not + * `authKind: "oauth"`, so this check never fires for them. + */ +function registrySendsOAuthToOverriddenBaseUrl(name: string): boolean { + const entry = getProviderRegistryEntry(name); + return entry?.authKind === "oauth" && entry.allowBaseUrlOverride === true; +} + +export function providerSecureTransportConfigError( + name: string, + provider: Pick, +): string | null { + if (!registrySendsOAuthToOverriddenBaseUrl(name)) return null; + let parsed: URL; + try { + parsed = new URL(provider.baseUrl.trim()); + } catch { + return null; // invalid URLs are providerBaseUrlConfigError's concern + } + if (parsed.protocol !== "http:") return null; + const assessment = assessDestination(provider.baseUrl); + // Classify FIRST, then consult the opt-in. `allowPrivateNetwork` says "this destination is + // intentionally local", which is a statement about the address, not a waiver of transport + // security — reading it before classification let `http://attacker.example` with the opt-in + // set carry an OAuth bearer in cleartext to a public host. + if (!assessment) return null; + const local = assessment.kind === "localhost" + || assessment.kind === "loopback" + || assessment.kind === "private"; + if (local && providerAllowsPrivateNetwork(name, provider)) { + // A genuinely local relay over http stays reachable through the explicit opt-in; the + // private-network gate still governs whether it may be reached at all. + return null; + } + return "baseUrl must use https: this provider sends OAuth credentials to its endpoint, and http is allowed only for loopback/private relays"; +} + /** * Whether a provider may reach loopback/private addresses. * @@ -150,6 +194,8 @@ export function providerAllowsPrivateNetwork( } export function providerDestinationConfigError(name: string, provider: Pick): string | null { + const secureTransportError = providerSecureTransportConfigError(name, provider); + if (secureTransportError) return secureTransportError; const assessment = assessDestination(provider.baseUrl); if (!assessment) return null; if (assessment.kind === "public" || assessment.kind === "hostname") return null; @@ -331,3 +377,4 @@ export async function resolvePublicAddresses( export async function assertUrlResolvesPublic(url: string): Promise { await resolvePublicAddresses(url); } + diff --git a/src/providers/registry.ts b/src/providers/registry.ts index 222767459d..bd25a5ea3f 100644 --- a/src/providers/registry.ts +++ b/src/providers/registry.ts @@ -1107,6 +1107,7 @@ export const PROVIDER_REGISTRY: readonly ProviderRegistryEntry[] = [ adapter: "anthropic", baseUrl: "https://api.anthropic.com", authKind: "oauth", + allowBaseUrlOverride: true, featured: true, oauthId: "anthropic", jawcodeBundle: "anthropic", @@ -1520,7 +1521,7 @@ export const PROVIDER_REGISTRY: readonly ProviderRegistryEntry[] = [ // 2026-07-10: defaultModel is frozen pending Vertex-specific Tier-2 evidence; Gemini API // evidence from ai.google.dev does not establish Vertex publisher availability. { id: "google-vertex", label: "Google Vertex AI", adapter: "google", baseUrl: "https://aiplatform.googleapis.com", authKind: "key", dashboardUrl: "https://console.cloud.google.com/vertex-ai", defaultModel: "gemini-3-pro", googleMode: "vertex", jawcodeBundle: "google", extraMetadataAliases: ["gemini-vertex"] }, - { id: "google-antigravity", label: "Google Antigravity", adapter: "google", baseUrl: "https://daily-cloudcode-pa.googleapis.com", authKind: "oauth", dashboardUrl: "https://antigravity.google", models: ANTIGRAVITY_MODELS, liveModels: true, defaultModel: "gemini-3.7-flash", modelContextWindows: ANTIGRAVITY_MODEL_CONTEXT_WINDOWS, modelInputModalities: ANTIGRAVITY_MODEL_INPUT_MODALITIES, modelReasoningEfforts: ANTIGRAVITY_MODEL_EFFORTS, googleMode: "cloud-code-assist", jawcodeBundle: "google", extraMetadataAliases: ["antigravity", "gemini-antigravity"] }, + { id: "google-antigravity", label: "Google Antigravity", adapter: "google", baseUrl: "https://daily-cloudcode-pa.googleapis.com", authKind: "oauth", allowBaseUrlOverride: true, dashboardUrl: "https://antigravity.google", models: ANTIGRAVITY_MODELS, liveModels: true, defaultModel: "gemini-3.7-flash", modelContextWindows: ANTIGRAVITY_MODEL_CONTEXT_WINDOWS, modelInputModalities: ANTIGRAVITY_MODEL_INPUT_MODALITIES, modelReasoningEfforts: ANTIGRAVITY_MODEL_EFFORTS, googleMode: "cloud-code-assist", jawcodeBundle: "google", extraMetadataAliases: ["antigravity", "gemini-antigravity"] }, { id: "azure-openai", label: "Azure OpenAI", adapter: "azure-openai", baseUrl: "https://{resource}.openai.azure.com/openai", authKind: "key", featured: true, dashboardUrl: "https://portal.azure.com" }, { id: "ollama", label: "Ollama (local)", adapter: "openai-chat", baseUrl: "http://localhost:11434/v1", authKind: "local", allowPrivateNetworkByDefault: true, allowBaseUrlOverride: true, featured: true, note: "Local — key usually blank" }, { id: "vllm", label: "vLLM (local)", adapter: "openai-chat", baseUrl: "http://localhost:8000/v1", authKind: "local", allowPrivateNetworkByDefault: true, allowBaseUrlOverride: true, featured: true, note: "Local — key usually blank" }, diff --git a/tests/anthropic-baseurl-override.test.ts b/tests/anthropic-baseurl-override.test.ts new file mode 100644 index 0000000000..b64e0feaab --- /dev/null +++ b/tests/anthropic-baseurl-override.test.ts @@ -0,0 +1,144 @@ +import { expect, test } from "bun:test"; +import { routeModel } from "../src/router"; +import { providerDestinationConfigError } from "../src/lib/destination-policy"; +import type { OcxConfig, OcxProviderConfig } from "../src/types"; + +/** + * Regression coverage for the allowBaseUrlOverride opt-in on the anthropic + * registry entry. + * + * Before the opt-in, the pinned registry endpoint silently outranked a saved + * baseUrl and the router emitted the discarded-baseUrl diagnostic (see + * tests/router-discarded-baseurl-warning.test.ts, which now pins google as + * its fixture). Users routing Claude traffic through a local relay or an + * enterprise gateway therefore could not redirect the provider at all. These + * tests pin the new contract: a resolved user baseUrl wins, no warning fires, + * and the registry endpoint remains the default seeded value. + */ +const PROVIDER = "anthropic"; +const REGISTRY_BASE_URL = "https://api.anthropic.com"; +const MODEL = PROVIDER + "/claude-sonnet-5"; + +function configFor(provider: OcxProviderConfig): OcxConfig { + return { + port: 10100, + defaultProvider: PROVIDER, + providers: { [PROVIDER]: provider }, + }; +} + +function routeCapturingWarnings(config: OcxConfig): { baseUrl: string; warnings: string[] } { + const warnings: string[] = []; + const originalWarn = console.warn; + console.warn = (...args: unknown[]) => { warnings.push(args.map(String).join(" ")); }; + try { + const route = routeModel(config, MODEL); + return { baseUrl: route.provider.baseUrl, warnings }; + } finally { + console.warn = originalWarn; + } +} + +test("anthropic honors a configured baseUrl override", () => { + const { baseUrl, warnings } = routeCapturingWarnings(configFor({ + adapter: "anthropic", + baseUrl: "https://claude-relay.example.test", + } as OcxProviderConfig)); + + expect(baseUrl).toBe("https://claude-relay.example.test"); + // The override is applied, so the discarded-baseUrl diagnostic must not fire. + expect(warnings).toHaveLength(0); +}); + +test("anthropic keeps the registry endpoint when the seeded baseUrl is unchanged", () => { + // providerConfigSeed copies the registry baseUrl into every saved config, so the + // no-override case reaches the router as a config whose baseUrl equals the registry URL. + const { baseUrl, warnings } = routeCapturingWarnings(configFor({ + adapter: "anthropic", + baseUrl: REGISTRY_BASE_URL, + } as OcxProviderConfig)); + + expect(baseUrl).toBe(REGISTRY_BASE_URL); + expect(warnings).toHaveLength(0); +}); + +test("anthropic requires a resolved baseUrl once override is enabled", () => { + // allowBaseUrlOverride providers fail closed on a missing baseUrl instead of silently + // re-pinning the registry endpoint; the seed guarantees real configs always carry one. + expect(() => routeModel(configFor({ + adapter: "anthropic", + } as OcxProviderConfig), MODEL)).toThrow(/Invalid baseUrl/); +}); + +test("anthropic rejects an unresolved template baseUrl override", () => { + expect(() => routeModel(configFor({ + adapter: "anthropic", + baseUrl: "https://{region}.example.test", + } as OcxProviderConfig), MODEL)).toThrow(/Invalid baseUrl/); +}); + +/** + * Security regression (CodeRabbit, PR #2109): anthropic is an OAuth provider, so an + * allowBaseUrlOverride endpoint receives bearer credentials. A cleartext http override to a + * non-local destination must be rejected on BOTH enforcement paths: routing (normal requests, + * via assertProviderDestinationAllowed) and providerDestinationConfigError, the shared gate + * that config validation and the model-discovery outbound layer (providerGet/providerPost in + * src/lib/provider-outbound.ts) consult before any fetch. + */ +test("anthropic rejects a cleartext http override on the routing path", () => { + expect(() => routeModel(configFor({ + adapter: "anthropic", + baseUrl: "http://claude-relay.example.test", + } as OcxProviderConfig), MODEL)).toThrow(/https/); +}); + +test("anthropic rejects a cleartext http override on the discovery/config gate", () => { + expect(providerDestinationConfigError(PROVIDER, { + baseUrl: "http://claude-relay.example.test", + } as OcxProviderConfig)).toMatch(/https/); + // The https form of the same destination stays accepted. + expect(providerDestinationConfigError(PROVIDER, { + baseUrl: "https://claude-relay.example.test", + } as OcxProviderConfig)).toBeNull(); +}); + +test("anthropic keeps http for an explicitly local relay", () => { + // Loopback and allowPrivateNetwork opt-ins are the documented local-transport escape + // hatch; the https requirement must not break a localhost proxy. + const { baseUrl, warnings } = routeCapturingWarnings(configFor({ + adapter: "anthropic", + baseUrl: "http://127.0.0.1:8787", + allowPrivateNetwork: true, + } as OcxProviderConfig)); + + expect(baseUrl).toBe("http://127.0.0.1:8787"); + expect(warnings).toHaveLength(0); +}); + + +test("a public http override cannot buy transport security with allowPrivateNetwork", () => { + // allowPrivateNetwork states that a destination is intentionally LOCAL. It is not a waiver of + // transport security. Reading it before classifying the address let http://attacker.example + // carry this provider's OAuth bearer in cleartext to a public host. + // + // Routing REFUSES rather than downgrading: a request must not reach an endpoint that would + // receive the token in the clear, so this fails closed at the route boundary. + expect(() => routeCapturingWarnings(configFor({ + adapter: "anthropic", + baseUrl: "http://attacker.example/v1", + allowPrivateNetwork: true, + } as OcxProviderConfig))).toThrow(/must use https/); +}); + +test("the seeded https endpoint is still reachable with the opt-in set", () => { + // Guard against over-correcting: the fix must refuse cleartext to a public host without + // refusing an ordinary https override that happens to carry the flag. + const { baseUrl, warnings } = routeCapturingWarnings(configFor({ + adapter: "anthropic", + baseUrl: "https://gateway.example/v1", + allowPrivateNetwork: true, + } as OcxProviderConfig)); + + expect(baseUrl).toBe("https://gateway.example/v1"); + expect(warnings).toHaveLength(0); +}); diff --git a/tests/antigravity-baseurl-override.test.ts b/tests/antigravity-baseurl-override.test.ts new file mode 100644 index 0000000000..5bc190c715 --- /dev/null +++ b/tests/antigravity-baseurl-override.test.ts @@ -0,0 +1,117 @@ +import { expect, test } from "bun:test"; +import { routeModel } from "../src/router"; +import { providerDestinationConfigError } from "../src/lib/destination-policy"; +import type { OcxConfig, OcxProviderConfig } from "../src/types"; + +/** + * Regression coverage for the allowBaseUrlOverride opt-in on the + * google-antigravity registry entry. + * + * Before the opt-in, the pinned registry endpoint silently outranked a saved + * baseUrl and the router emitted the discarded-baseUrl diagnostic (see + * tests/router-discarded-baseurl-warning.test.ts). Users routing Antigravity + * traffic through a local relay or region-specific proxy therefore could not + * redirect the provider at all. These tests pin the new contract: a resolved + * user baseUrl wins, no warning fires, and the registry endpoint remains the + * default when nothing is configured. + */ +const PROVIDER = "google-antigravity"; +const REGISTRY_BASE_URL = "https://daily-cloudcode-pa.googleapis.com"; +const MODEL = PROVIDER + "/gemini-3.7-flash"; + +function configFor(provider: OcxProviderConfig): OcxConfig { + return { + port: 10100, + defaultProvider: PROVIDER, + providers: { [PROVIDER]: provider }, + }; +} + +function routeCapturingWarnings(config: OcxConfig): { baseUrl: string; warnings: string[] } { + const warnings: string[] = []; + const originalWarn = console.warn; + console.warn = (...args: unknown[]) => { warnings.push(args.map(String).join(" ")); }; + try { + const route = routeModel(config, MODEL); + return { baseUrl: route.provider.baseUrl, warnings }; + } finally { + console.warn = originalWarn; + } +} + +test("google-antigravity honors a configured baseUrl override", () => { + const { baseUrl, warnings } = routeCapturingWarnings(configFor({ + adapter: "google", + baseUrl: "https://antigravity-relay.example.test", + } as OcxProviderConfig)); + + expect(baseUrl).toBe("https://antigravity-relay.example.test"); + // The override is applied, so the discarded-baseUrl diagnostic must not fire. + expect(warnings).toHaveLength(0); +}); + +test("google-antigravity keeps the registry endpoint when the seeded baseUrl is unchanged", () => { + // providerConfigSeed copies the registry baseUrl into every saved config, so the + // no-override case reaches the router as a config whose baseUrl equals the registry URL. + const { baseUrl, warnings } = routeCapturingWarnings(configFor({ + adapter: "google", + baseUrl: REGISTRY_BASE_URL, + } as OcxProviderConfig)); + + expect(baseUrl).toBe(REGISTRY_BASE_URL); + expect(warnings).toHaveLength(0); +}); + +test("google-antigravity requires a resolved baseUrl once override is enabled", () => { + // allowBaseUrlOverride providers fail closed on a missing baseUrl instead of silently + // re-pinning the registry endpoint; the seed guarantees real configs always carry one. + expect(() => routeModel(configFor({ + adapter: "google", + } as OcxProviderConfig), MODEL)).toThrow(/Invalid baseUrl/); +}); + +test("google-antigravity rejects an unresolved template baseUrl override", () => { + expect(() => routeModel(configFor({ + adapter: "google", + baseUrl: "https://{region}.example.test", + } as OcxProviderConfig), MODEL)).toThrow(/Invalid baseUrl/); +}); + +/** + * Security regression (CodeRabbit, PR #2110): google-antigravity is an OAuth provider, so an + * allowBaseUrlOverride endpoint receives bearer credentials. A cleartext http override to a + * non-local destination must be rejected on BOTH enforcement paths: routing (normal requests, + * via assertProviderDestinationAllowed) and providerDestinationConfigError, the shared gate + * that config validation and the outbound layer (providerGet/providerPost in + * src/lib/provider-outbound.ts) consult before any fetch. + */ +test("google-antigravity rejects a cleartext http override on the routing path", () => { + expect(() => routeModel(configFor({ + adapter: "google", + baseUrl: "http://antigravity-relay.example.test", + } as OcxProviderConfig), MODEL)).toThrow(/https/); +}); + +test("google-antigravity rejects a cleartext http override on the discovery/config gate", () => { + expect(providerDestinationConfigError(PROVIDER, { + baseUrl: "http://antigravity-relay.example.test", + } as OcxProviderConfig)).toMatch(/https/); + // The https form of the same destination stays accepted. + expect(providerDestinationConfigError(PROVIDER, { + baseUrl: "https://antigravity-relay.example.test", + } as OcxProviderConfig)).toBeNull(); +}); + +test("google-antigravity keeps http for an explicitly local relay", () => { + // The local proxy (127.0.0.1) is the motivating use case for this override; the https + // requirement must not break it. allowPrivateNetwork is the documented local opt-in. + const { baseUrl, warnings } = routeCapturingWarnings(configFor({ + adapter: "google", + baseUrl: "http://127.0.0.1:47821", + allowPrivateNetwork: true, + } as OcxProviderConfig)); + + expect(baseUrl).toBe("http://127.0.0.1:47821"); + expect(warnings).toHaveLength(0); +}); + diff --git a/tests/provider-model-discovery-contract.test.ts b/tests/provider-model-discovery-contract.test.ts index 07c8c0e6ac..c49d4062dd 100644 --- a/tests/provider-model-discovery-contract.test.ts +++ b/tests/provider-model-discovery-contract.test.ts @@ -172,15 +172,15 @@ describe("registry-owned provider model discovery", () => { authMode: "oauth", }; - await withRegistryDiscovery("anthropic", { path: "catalog" }, () => { - const relative = buildModelsRequest(staleConfig, "oauth-token", "anthropic"); - expect(relative.url).toBe("https://api.anthropic.com/catalog"); + await withRegistryDiscovery("kimi", { path: "catalog" }, () => { + const relative = buildModelsRequest(staleConfig, "oauth-token", "kimi"); + expect(relative.url).toBe("https://api.kimi.com/coding/v1/catalog"); expect(relative.headers.Authorization).toBe("Bearer oauth-token"); }); - await withRegistryDiscovery("anthropic", { maxModels: 25 }, () => { - const defaultEndpoint = buildModelsRequest(staleConfig, "oauth-token", "anthropic"); - expect(defaultEndpoint.url).toBe("https://api.anthropic.com/v1/models?limit=1000"); + await withRegistryDiscovery("kimi", { maxModels: 25 }, () => { + const defaultEndpoint = buildModelsRequest(staleConfig, "oauth-token", "kimi"); + expect(defaultEndpoint.url).toBe("https://api.kimi.com/coding/v1/models"); }); }); @@ -604,3 +604,4 @@ describe("same-named custom provider preservation", () => { }); }); }); + diff --git a/tests/provider-registry-parity.test.ts b/tests/provider-registry-parity.test.ts index f06a8953c3..d2858f6f44 100644 --- a/tests/provider-registry-parity.test.ts +++ b/tests/provider-registry-parity.test.ts @@ -563,7 +563,10 @@ describe("provider registry parity", () => { test("base URL override permission is registry-only and limited to opted-in providers", () => { const optedIn = PROVIDER_REGISTRY.filter(entry => entry.allowBaseUrlOverride); - expect(optedIn.map(entry => entry.id)).toEqual(["ollama", "vllm", "lm-studio", "moonshot", "qwen-cloud", "alibaba", "alibaba-token-plan-intl", "litellm"]); + // Registry order. Both OAuth entries (anthropic, google-antigravity) are gated by + // providerSecureTransportConfigError; the rest are key/local providers that never send a + // subscription bearer to the override. + expect(optedIn.map(entry => entry.id)).toEqual(["anthropic", "google-antigravity", "ollama", "vllm", "lm-studio", "moonshot", "qwen-cloud", "alibaba", "alibaba-token-plan-intl", "litellm"]); for (const entry of optedIn) { expect(providerConfigSeed(entry)).not.toHaveProperty("allowBaseUrlOverride"); } diff --git a/tests/router-discarded-baseurl-warning.test.ts b/tests/router-discarded-baseurl-warning.test.ts index 05ea18e250..dc31220a3a 100644 --- a/tests/router-discarded-baseurl-warning.test.ts +++ b/tests/router-discarded-baseurl-warning.test.ts @@ -7,12 +7,12 @@ import type { OcxConfig, OcxProviderConfig } from "../src/types"; * asserted in tests/router-template-baseurl.test.ts; these tests cover the diagnostic that * tells the user it happened, so a wrong-region URL stops surfacing as a bare 401. * - * `anthropic` is the pinned fixture: a fixed remote registry endpoint, no `allowBaseUrlOverride`. + * `google` is the pinned fixture: a fixed remote registry endpoint, no `allowBaseUrlOverride`. * Warnings dedupe per (provider, discarded URL, effective URL), so each test uses a distinct * discarded URL and the suite stays order-independent. */ -const PINNED_PROVIDER = "anthropic"; -const PINNED_REGISTRY_BASE_URL = "https://api.anthropic.com"; +const PINNED_PROVIDER = "google"; +const PINNED_REGISTRY_BASE_URL = "https://generativelanguage.googleapis.com"; function configFor(providerName: string, provider: OcxProviderConfig): OcxConfig { return { @@ -37,8 +37,8 @@ function routeCapturingWarnings(config: OcxConfig, model: string, times = 1): st function routePinned(baseUrl: unknown, times = 1): string[] { return routeCapturingWarnings( - configFor(PINNED_PROVIDER, { adapter: "anthropic", baseUrl } as OcxProviderConfig), - `${PINNED_PROVIDER}/claude-sonnet-5`, + configFor(PINNED_PROVIDER, { adapter: "google", baseUrl } as OcxProviderConfig), + `${PINNED_PROVIDER}/gemini-3-pro`, times, ); } @@ -57,13 +57,13 @@ test("warns when a pinned provider discards a configured baseUrl", () => { test("routing is unchanged by the warning", () => { const config = configFor(PINNED_PROVIDER, { - adapter: "anthropic", + adapter: "google", baseUrl: "https://routing-unchanged.example.test/v1", }); const originalWarn = console.warn; console.warn = () => {}; try { - expect(routeModel(config, `${PINNED_PROVIDER}/claude-sonnet-5`).provider.baseUrl) + expect(routeModel(config, `${PINNED_PROVIDER}/gemini-3-pro`).provider.baseUrl) .toBe(PINNED_REGISTRY_BASE_URL); } finally { console.warn = originalWarn; @@ -221,3 +221,4 @@ for (const { label, id, adapter, baseUrl } of [ expect(routeModel(config, `${id}/model`).provider.baseUrl).toBe(baseUrl); }); } + diff --git a/tests/router-template-baseurl.test.ts b/tests/router-template-baseurl.test.ts index e3880a45d4..053f2ff7b7 100644 --- a/tests/router-template-baseurl.test.ts +++ b/tests/router-template-baseurl.test.ts @@ -56,7 +56,7 @@ for (const { id, registryBaseUrl } of OVERRIDE_PROVIDERS) { for (const { id, registryBaseUrl, adapter } of [ { id: "ollama-cloud", registryBaseUrl: "https://ollama.com/v1", adapter: "openai-chat" }, - { id: "anthropic", registryBaseUrl: "https://api.anthropic.com", adapter: "anthropic" }, + { id: "google", registryBaseUrl: "https://generativelanguage.googleapis.com", adapter: "google" }, ] as const) { test(`${id} keeps its fixed remote registry endpoint authoritative`, () => { const config = configFor(id, { @@ -100,3 +100,4 @@ for (const { id, adapter, registryTemplate, resolvedBaseUrl } of [ expect(routeModel(config, `${id}/model`).provider.baseUrl).toBe(registryTemplate); }); } +