diff --git a/app/src/lib/anthropicKeyCheck.ts b/app/src/lib/anthropicKeyCheck.ts index 28d32a1..e9a7f22 100644 --- a/app/src/lib/anthropicKeyCheck.ts +++ b/app/src/lib/anthropicKeyCheck.ts @@ -5,11 +5,24 @@ // a revoked key, or another provider's key entirely all "saved // successfully" and only failed once a real chat session hit Anthropic). // -// Uses GET /v1/models — the same base URL and `anthropic-version` header -// the official `@anthropic-ai/sdk` client itself sends (see -// `node_modules/@anthropic-ai/sdk/client.js`) — a free, side-effect-free -// metadata endpoint, so verifying costs nothing and can't consume a -// message/token budget. +// IMPORTANT: verifies against POST /v1/messages -- the exact endpoint the +// real chat feature uses -- NOT /v1/models. An earlier version of this +// file used /v1/models (a metadata/listing endpoint) and that was a real, +// shipped bug: Anthropic supports scoped/restricted API keys that can work +// perfectly for chat completions while lacking permission to list models, +// so a fully valid, working key could get a false 403 on /v1/models and be +// silently rejected at save time -- users could no longer save a real key +// at all. Verifying against /v1/messages instead eliminates that entire +// class of false positives by construction: whatever this call proves +// works IS the exact capability the app actually needs. +// +// Zero-cost: sends a deliberately empty/invalid body (missing the required +// `model`/`messages`/`max_tokens` fields). Confirmed live against the real +// API (not assumed) that Anthropic validates the `x-api-key` header BEFORE +// it ever validates the request body -- an invalid key gets a 401 +// regardless of body content, so this never reaches the model and never +// consumes a token, while still exercising the identical auth path a real +// chat request takes. const ANTHROPIC_BASE_URL = "https://api.anthropic.com"; const ANTHROPIC_VERSION = "2023-06-01"; @@ -18,28 +31,27 @@ export type AnthropicKeyCheckResult = | { ok: false; reason: "invalid" | "network"; message: string }; /** Calls Anthropic's own API with `key` and reports whether Anthropic itself - * accepts it. Distinguishes a confirmed-bad key (401/403 — reject the - * save) from a transient network/outage failure (don't block a save on - * our own connectivity issue; the key may well be fine). */ + * rejects it outright. Deliberately conservative: only a confirmed, + * unambiguous 401 ("invalid x-api-key") blocks a save. Every other + * response -- including the expected 400 invalid_request_error from the + * intentionally-empty body once auth succeeds, 403, 429, 5xx, or a + * network failure reaching Anthropic at all -- is treated as "not + * evidence the key itself is bad" and does not block saving it. */ export async function verifyAnthropicApiKey(key: string, signal?: AbortSignal): Promise { try { - const res = await fetch(`${ANTHROPIC_BASE_URL}/v1/models?limit=1`, { - method: "GET", - headers: { "x-api-key": key, "anthropic-version": ANTHROPIC_VERSION }, + const res = await fetch(`${ANTHROPIC_BASE_URL}/v1/messages`, { + method: "POST", + headers: { "x-api-key": key, "anthropic-version": ANTHROPIC_VERSION, "content-type": "application/json" }, + body: "{}", // deliberately missing required fields -- never reaches the model signal, }); - if (res.ok) return { ok: true }; - if (res.status === 401 || res.status === 403) { - const body = await res.json().catch(() => null) as { error?: { message?: string } } | null; - return { - ok: false, - reason: "invalid", - message: body?.error?.message || "Anthropic rejected this API key (invalid or revoked).", - }; - } - // Any other status (429 rate-limited, 5xx) isn't evidence the KEY is - // wrong -- don't block the save on it. - return { ok: true }; + if (res.status !== 401) return { ok: true }; + const body = await res.json().catch(() => null) as { error?: { message?: string } } | null; + return { + ok: false, + reason: "invalid", + message: body?.error?.message || "Anthropic rejected this API key (invalid or revoked).", + }; } catch { // Network failure reaching Anthropic (offline, DNS, timeout, egress // blocked) -- not evidence the key itself is bad, so don't block the save. diff --git a/app/tests/anthropicKeyCheck.test.ts b/app/tests/anthropicKeyCheck.test.ts index 4bfd4df..00ca982 100644 --- a/app/tests/anthropicKeyCheck.test.ts +++ b/app/tests/anthropicKeyCheck.test.ts @@ -1,8 +1,17 @@ // Regression tests for verifyAnthropicApiKey (closes the "Invalid API key" // gap: previously any string was accepted and persisted with zero // validation, only failing deep inside a real chat turn against Anthropic). -// Also proves the /api/settings/assistant route actually rejects a -// confirmed-bad key BEFORE ever writing it to the settings table. +// +// Also locks in the fix for a real, shipped regression: the first version +// of this check verified against GET /v1/models (a metadata/listing +// endpoint). Anthropic supports scoped/restricted API keys that can work +// perfectly for chat completions while lacking permission to list models -- +// so a fully valid, working key could get a false 403 there and be +// silently rejected at save time, meaning a real key could no longer be +// saved at all. This file now verifies the check calls POST /v1/messages +// (the exact endpoint the real chat feature uses) and only ever blocks a +// save on a confirmed 401 -- everything else, including 403, is treated as +// "not evidence the key is bad". import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; @@ -27,19 +36,27 @@ describe("verifyAnthropicApiKey", () => { global.fetch = originalFetch; }); - it("reports ok:true when Anthropic accepts the key (200)", async () => { + it("calls POST /v1/messages (NOT /v1/models) with the exact same auth headers the real chat feature sends", async () => { global.fetch = vi.fn(async (url: string, init?: RequestInit) => { - expect(url).toBe("https://api.anthropic.com/v1/models?limit=1"); + expect(url).toBe("https://api.anthropic.com/v1/messages"); + expect(init?.method).toBe("POST"); expect((init?.headers as Record)["x-api-key"]).toBe("sk-ant-real-key"); expect((init?.headers as Record)["anthropic-version"]).toBe("2023-06-01"); - return new Response(JSON.stringify({ data: [] }), { status: 200 }); + return new Response(JSON.stringify({ type: "error", error: { type: "invalid_request_error", message: "model: Field required" } }), { status: 400 }); }) as unknown as typeof fetch; const result = await verifyAnthropicApiKey("sk-ant-real-key"); expect(result.ok).toBe(true); }); - it("reports ok:false reason:invalid when Anthropic returns 401 (this IS the 'Invalid API key' case)", async () => { + it("reports ok:true when Anthropic returns the expected 400 for the deliberately-empty body (proves auth passed)", async () => { + global.fetch = vi.fn(async () => + new Response(JSON.stringify({ error: { message: "model: Field required" } }), { status: 400 }) + ) as unknown as typeof fetch; + expect((await verifyAnthropicApiKey("sk-ant-real-working-key")).ok).toBe(true); + }); + + it("reports ok:false reason:invalid ONLY on a confirmed 401 (this IS the 'Invalid API key' case)", async () => { global.fetch = vi.fn(async () => new Response(JSON.stringify({ error: { message: "invalid x-api-key" } }), { status: 401 }) ) as unknown as typeof fetch; @@ -52,11 +69,10 @@ describe("verifyAnthropicApiKey", () => { } }); - it("reports ok:false reason:invalid on 403 too", async () => { + it("does NOT reject on 403 -- a scoped/restricted key can legitimately lack unrelated permissions without being broken for chat (this is the exact false-positive the /v1/models version had)", async () => { global.fetch = vi.fn(async () => new Response(JSON.stringify({}), { status: 403 })) as unknown as typeof fetch; - const result = await verifyAnthropicApiKey("sk-ant-forbidden"); - expect(result.ok).toBe(false); - if (!result.ok) expect(result.reason).toBe("invalid"); + const result = await verifyAnthropicApiKey("sk-ant-scoped-but-working-key"); + expect(result.ok).toBe(true); }); it("does NOT treat a rate-limit (429) or server error (500) as an invalid key", async () => { @@ -77,7 +93,6 @@ describe("verifyAnthropicApiKey", () => { describe("POST /api/settings/assistant rejects a confirmed-invalid key before persisting it", () => { const originalFetch = global.fetch; - const USER_ID = 9001; afterEach(() => { global.fetch = originalFetch; }); @@ -90,13 +105,6 @@ describe("POST /api/settings/assistant rejects a confirmed-invalid key before pe }); } - beforeEach(() => { - // GitHub OAuth isn't configured in this test env, so unauthorized() is a - // no-op and every request lands in the ANONYMOUS_USER_ID bucket via - // userIdFrom(req) -- fine, this suite tests the verification gate, not - // the account-scoping gate (already covered by settings.test.ts). - }); - it("rejects a key Anthropic returns 401 for, and never writes it to the DB", async () => { global.fetch = vi.fn(async () => new Response(JSON.stringify({ error: { message: "invalid x-api-key" } }), { status: 401 }) @@ -107,12 +115,19 @@ describe("POST /api/settings/assistant rejects a confirmed-invalid key before pe const data = await res.json(); expect(data.error).toContain("invalid x-api-key"); - // The anonymous bucket must not have picked up the rejected key. expect(getAssistantSettings(0).anthropicApiKey).toBeNull(); }); - it("accepts and persists a key Anthropic returns 200 for", async () => { - global.fetch = vi.fn(async () => new Response(JSON.stringify({ data: [] }), { status: 200 })) as unknown as typeof fetch; + it("accepts and persists a real key even when Anthropic returns 403 on this check (the exact regression that used to silently break saving)", async () => { + global.fetch = vi.fn(async () => new Response(JSON.stringify({}), { status: 403 })) as unknown as typeof fetch; + + const res = await settingsPost(postRequest({ anthropicApiKey: "sk-ant-SCOPED-BUT-VALID-KEY" })); + expect(res.status).toBe(200); + expect(getAssistantSettings(0).anthropicApiKey).toBe("sk-ant-SCOPED-BUT-VALID-KEY"); + }); + + it("accepts and persists a key Anthropic's expected 400 (empty-body validation error) confirms is authenticated", async () => { + global.fetch = vi.fn(async () => new Response(JSON.stringify({ error: { message: "model: Field required" } }), { status: 400 })) as unknown as typeof fetch; const res = await settingsPost(postRequest({ anthropicApiKey: "sk-ant-GOOD-KEY" })); expect(res.status).toBe(200);