mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 20:42:43 +00:00
fix(ai): block aiBaseUrl from leaking across providers
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) <noreply@anthropic.com>
This commit is contained in:
1 parent
a75fbc6aaf
commit
eba898f4c1
3 files changed
+71
-5
No files matched your search
@@ -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 });
|
||||
|
||||
@@ -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",
|
||||
|
||||
+15
-5
@@ -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();
|
||||
|
||||
Reference in new issue
Block a user