fix: stop rejecting valid Anthropic keys — verify against /v1/messages, not /v1/models

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<AnthropicKeyCheckResult> {
   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.

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<string, string>)["x-api-key"]).toBe("sk-ant-real-key");
       expect((init?.headers as Record<string, string>)["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);