fix: replace the broken per-key API rate limit with a real 5000/min per-user /mcp limit (#545)

alchemy.run.ts

@@ -468,6 +468,15 @@ export default Alchemy.Stack(
         // name).
         AUDIT_ENGINE: auditWorker,
 
+        // Per-user throttle for /mcp API-key auth (see
+        // src/server/mcp/api-key-auth.ts). Only this stack declares the
+        // binding — the wrangler.jsonc surfaces (local dev, Docker
+        // self-host) skip limiting when it's absent.
+        MCP_RATE_LIMIT: Cloudflare.RateLimit("MCP_RATE_LIMIT", {
+          namespaceId: 1001,
+          simple: { limit: 5000, period: 60 },
+        }),
+
         // Durable Objects (the chat agents; the audit scratchpad lives
         // privately in the open-seo-audit worker). Alchemy backs new DO
         // classes with SQLite storage; wrangler.jsonc's `migrations` only

src/lib/auth-api-key.ts

@@ -10,10 +10,20 @@ export function createApiKeyPlugin() {
     // Stored display prefix ("oseo_" + 4 key chars) shown in Settings so keys
     // are tellable apart; the plugin default of 6 barely clears the prefix.
     startingCharactersConfig: { shouldStore: true, charactersLength: 9 },
-    rateLimit: {
-      enabled: true,
-      timeWindow: 60 * 1000,
-      maxRequests: 500,
-    },
+    // The plugin's own rate limit is off; the /mcp handler enforces a real
+    // 5000-per-minute per-user limit via Cloudflare's rate-limit binding
+    // instead (see server/mcp/api-key-auth.ts; hosted prod only). This
+    // limiter is disabled because it is broken for steady traffic: its
+    // window resets only after a full timeWindow of complete idle (it
+    // compares against the previous request, not a window start), so
+    // "500 / 60s" really meant "500 requests without a 60s pause". An active
+    // MCP session never pauses that long, so keys hit the cap and were
+    // hard-blocked for a minute at a time. It also never protected against
+    // invalid keys — it ran only AFTER the hashed key matched a row.
+    //
+    // Raising maxRequests instead would not have worked: the limits are
+    // snapshotted onto each apikey row at creation, whereas `enabled: false`
+    // is read from this live config first — it frees existing keys too.
+    rateLimit: { enabled: false },
   });
 }

src/lib/auth.ts

@@ -68,6 +68,16 @@ function createAuth() {
   const auth = betterAuth({
     baseURL: baseUrl,
     secret: getHostedSecret(),
+    // The api-key plugin logs every verification failure at error level — a
+    // stale key or a throttled caller included. The /mcp handler already logs
+    // the response it returns at the right level (debug for 401, warn for 429),
+    // so drop the duplicate.
+    logger: {
+      log: (level, message, ...args) => {
+        if (message.startsWith("Failed to validate API key")) return;
+        console[level](`[better-auth] ${message}`, ...args);
+      },
+    },
     ...baseAuthConfig,
     emailAndPassword: {
       ...baseAuthConfig.emailAndPassword,

src/server/mcp/api-key-auth.test.ts

@@ -146,25 +146,23 @@ describe("handleMcpApiKeyRequest", () => {
     expect(mocks.handleAuthenticatedOpenSeoMcpRequest).not.toHaveBeenCalled();
   });
 
-  it("returns 429 with Retry-After when Better Auth rate-limits the key", async () => {
+  it("returns 429 with Retry-After when the MCP_RATE_LIMIT binding denies", async () => {
     mocks.verifyApiKey.mockResolvedValue({
-      valid: false,
-      error: {
-        code: "RATE_LIMITED",
-        message: "Rate limit exceeded",
-        details: { tryAgainIn: 30500 },
-      },
-      key: null,
+      valid: true,
+      error: null,
+      key: { referenceId: "user-1" },
     });
+    const limit = vi.fn().mockResolvedValue({ success: false });
 
     const response = await handleMcpApiKeyRequest(
       request({ "x-api-key": "oseo_limited" }),
-      env,
+      { MCP_RATE_LIMIT: { limit } },
       ctx,
     );
 
+    expect(limit).toHaveBeenCalledWith({ key: "user-1" });
     expect(response?.status).toBe(429);
-    expect(response?.headers.get("Retry-After")).toBe("31");
+    expect(response?.headers.get("Retry-After")).toBe("60");
     await expect(response?.json()).resolves.toMatchObject({
       error: "rate_limited",
     });

src/server/mcp/api-key-auth.ts

@@ -95,6 +95,26 @@ export async function handleMcpApiKeyRequest(
     }
 
     const userId = result.key.referenceId;
+
+    // Per-user request throttle. The binding is declared in alchemy.run.ts
+    // (hosted prod only); local dev and self-host run without it and skip
+    // limiting, which is fine single-user. This replaces the better-auth
+    // plugin limiter, whose broken idle-gap window hard-blocked active MCP
+    // clients (see lib/auth-api-key.ts). Cloudflare's counter is per-colo
+    // best-effort, which is all this needs to be: credits bound spend, this
+    // bounds runaway request volume.
+    const rateLimit = (env as { MCP_RATE_LIMIT?: RateLimit }).MCP_RATE_LIMIT;
+    if (rateLimit) {
+      const { success } = await rateLimit.limit({ key: userId });
+      if (!success) {
+        return apiKeyErrorResponse({
+          code: "RATE_LIMITED",
+          message: "Rate limit exceeded: 5000 requests per minute",
+          details: { tryAgainIn: 60_000 },
+        });
+      }
+    }
+
     const user = await AuthRepository.getHostedUser(userId);
     if (!user?.email) return apiKeyErrorResponse(null);
 

worker-configuration.d.ts

@@ -20,6 +20,9 @@ declare namespace Cloudflare {
 		LOOPS_TRANSACTIONAL_RESET_PASSWORD_ID: string;
 		POSTHOG_HOST: string;
 		POSTHOG_PUBLIC_KEY: string;
+		// Hand-patched: hosted-prod-only binding declared in alchemy.run.ts,
+		// absent in the wrangler.jsonc surfaces (local dev, Docker self-host).
+		MCP_RATE_LIMIT?: RateLimit;
 		// Hand-patched: the class moved to src/audit-worker.ts and a full regen pulls
 		// unrelated runtime-type drift (see cf-typegen); keep this in sync until then.
 		SITE_AUDIT_WORKFLOW: Workflow<Parameters<import("./src/audit-worker").SiteAuditWorkflow['run']>[0]['payload']>;