From eba898f4c1d0a8064c8268540e78faaef68ed301 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sat, 9 May 2026 01:26:48 +0200 Subject: [PATCH] fix(ai): block aiBaseUrl from leaking across providers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The user row stores a single `aiBaseUrl` column that is meaningful only for the LOCAL provider, but `buildUserProvider` and `resolveProviderForTest` were forwarding that value to OpenAI and Anthropic clients too. A user who had configured LOCAL pointing at e.g. `http://192.168.x.x/v1` and later switched to OPENAI would have their OpenAI API key sent to that LAN host on the next request. Defence in three layers: - PATCH /api/user/ai-provider — when the provider switches to anything other than LOCAL and the body does not include an explicit `aiBaseUrl` write, set the column back to NULL. - `buildUserProvider` — for OPENAI hardcode `https://api.openai.com/v1`, for ANTHROPIC do not forward a base URL at all (the SDK default is correct and the UI never exposed an Anthropic base-URL input). - `resolveProviderForTest` — same belt-and-braces: ignore the merged base URL for OPENAI/ANTHROPIC so a stored LOCAL value cannot bleed into a connection-test request. Adds two regression tests covering the LOCAL → OPENAI and LOCAL → ANTHROPIC paths with a stale `aiBaseUrl`. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/app/api/user/ai-provider/route.ts | 14 +++++++++ src/lib/ai/__tests__/provider.test.ts | 42 +++++++++++++++++++++++++++ src/lib/ai/provider.ts | 20 +++++++++---- 3 files changed, 71 insertions(+), 5 deletions(-) diff --git a/src/app/api/user/ai-provider/route.ts b/src/app/api/user/ai-provider/route.ts index d62ae99b8..fd5a31444 100644 --- a/src/app/api/user/ai-provider/route.ts +++ b/src/app/api/user/ai-provider/route.ts @@ -117,6 +117,20 @@ export const PATCH = apiHandler(async (request: NextRequest) => { throw new HttpError(422, "No valid fields"); } + // When the provider switches away from LOCAL, drop any stored + // `aiBaseUrl`. The column is shared across providers, so without + // this a user who once configured LOCAL → http://192.168.x.x and + // then switched to OPENAI/ANTHROPIC would have their cloud key + // sent to that URL on the next request. Only LOCAL legitimately + // uses a custom base URL. + if ( + typeof updates.aiProvider === "string" && + updates.aiProvider !== "LOCAL" && + !("aiBaseUrl" in updates) + ) { + updates.aiBaseUrl = null; + } + await prisma.user.update({ where: { id: user.id }, data: updates }); return apiSuccess({ updated: true }); diff --git a/src/lib/ai/__tests__/provider.test.ts b/src/lib/ai/__tests__/provider.test.ts index cafaad855..c10116d3a 100644 --- a/src/lib/ai/__tests__/provider.test.ts +++ b/src/lib/ai/__tests__/provider.test.ts @@ -224,6 +224,48 @@ describe("resolveProvider", () => { expect(provider.type).toBe("admin-key"); }); + it("OPENAI with user key ignores stored aiBaseUrl (provider-leak guard)", async () => { + // Regression for v1.4.6 T4: a user who had configured LOCAL with a + // private LAN URL and later switched to OPENAI must NOT have their + // OpenAI key forwarded to that URL. The persisted `aiBaseUrl` is + // shared across providers, so `buildUserProvider` hardcodes + // api.openai.com for OPENAI regardless of what the column carries. + vi.mocked(prisma.user.findUnique).mockResolvedValueOnce({ + aiProvider: "OPENAI", + aiModel: null, + aiBaseUrl: "http://192.168.0.42/v1", // stale LOCAL value + aiAnthropicKeyEncrypted: null, + aiLocalKeyEncrypted: null, + aiOpenaiKeyEncrypted: "enc-user-openai-key", + } as never); + + const provider = await resolveProvider("user-123"); + expect(provider.type).toBe("admin-key"); + const inspect = provider as unknown as { + config: { baseUrl: string }; + }; + expect(inspect.config.baseUrl).toBe("https://api.openai.com/v1"); + }); + + it("ANTHROPIC ignores stored aiBaseUrl (provider-leak guard)", async () => { + vi.mocked(prisma.user.findUnique).mockResolvedValueOnce({ + aiProvider: "ANTHROPIC", + aiModel: "claude-3-5-sonnet-latest", + aiBaseUrl: "http://192.168.0.42/v1", // stale LOCAL value + aiAnthropicKeyEncrypted: "enc-user-anthropic-key", + aiLocalKeyEncrypted: null, + aiOpenaiKeyEncrypted: null, + } as never); + + const provider = await resolveProvider("user-123"); + expect(provider.type).toBe("anthropic"); + // AnthropicClient should default to its SDK baseUrl, never the LAN URL. + const inspect = provider as unknown as { + config: { baseUrl?: string }; + }; + expect(inspect.config.baseUrl).toBeUndefined(); + }); + it("CHATGPT_OAUTH selection routes through Codex when connected", async () => { vi.mocked(decryptTokens).mockReturnValue({ accessToken: "decrypted-access", diff --git a/src/lib/ai/provider.ts b/src/lib/ai/provider.ts index e1dffc269..4b988c6f9 100644 --- a/src/lib/ai/provider.ts +++ b/src/lib/ai/provider.ts @@ -46,10 +46,13 @@ function buildUserProvider(row: UserAIRow): AIProvider | null { switch (choice) { case "ANTHROPIC": { if (!row.aiAnthropicKeyEncrypted) return null; + // Belt-and-braces: even if a stale `aiBaseUrl` from a prior LOCAL + // configuration survived in the row, refuse to forward an Anthropic + // key to it. Anthropic has no per-tenant base URL the UI exposes; + // the SDK default is correct. return new AnthropicClient({ apiKey: decrypt(row.aiAnthropicKeyEncrypted), model: row.aiModel ?? "claude-3-5-sonnet-latest", - baseUrl: row.aiBaseUrl ?? undefined, }); } case "LOCAL": { @@ -68,11 +71,14 @@ function buildUserProvider(row: UserAIRow): AIProvider | null { // model-default mirrors the admin path for consistency so a saved // user "OPENAI" without an explicit model still produces an // OpenAIClient with `gpt-4o-mini`. + // Belt-and-braces: ignore any persisted `aiBaseUrl`. The column is + // shared with LOCAL, so a stale LAN URL there would otherwise + // redirect the user's OpenAI key to a private host. if (!row.aiOpenaiKeyEncrypted) return null; return new OpenAIClient({ apiKey: decrypt(row.aiOpenaiKeyEncrypted), model: row.aiModel ?? "gpt-4o-mini", - baseUrl: row.aiBaseUrl ?? "https://api.openai.com/v1", + baseUrl: "https://api.openai.com/v1", }); } case "CHATGPT_OAUTH": @@ -306,10 +312,12 @@ export async function resolveProviderForTest( if (!apiKey) { throw new AITestConfigError(422, "Anthropic API key not configured"); } + // Anthropic has no UI base-URL input. Ignore the merged value to + // avoid leaking the key to a stale LOCAL URL still parked in the + // shared column. return new AnthropicClient({ apiKey, model: model || "claude-3-5-sonnet-latest", - baseUrl: baseUrl || undefined, }); } case "LOCAL": { @@ -343,7 +351,9 @@ export async function resolveProviderForTest( // Test path mirrors the persistent resolution: user key first, // admin fallback if absent. We accept an `openaiKey` override // from the test endpoint so a user can verify a not-yet-saved - // key dropdown change without persisting anything. + // key dropdown change without persisting anything. Always use + // the canonical OpenAI base URL — the merged `baseUrl` may carry + // a stale LOCAL URL through `stored.aiBaseUrl`. const userKey = override.openaiKey?.trim() || (stored?.aiOpenaiKeyEncrypted @@ -353,7 +363,7 @@ export async function resolveProviderForTest( return new OpenAIClient({ apiKey: userKey, model: model || "gpt-4o-mini", - baseUrl: baseUrl || "https://api.openai.com/v1", + baseUrl: "https://api.openai.com/v1", }); } const admin = await resolveAdminProvider();