diff --git a/alchemy.run.ts b/alchemy.run.ts index 4e16731..4c6e02e 100644 --- a/alchemy.run.ts +++ b/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 diff --git a/src/lib/auth-api-key.ts b/src/lib/auth-api-key.ts index 8755ec8..8154e5f 100644 --- a/src/lib/auth-api-key.ts +++ b/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 }, }); } diff --git a/src/lib/auth.ts b/src/lib/auth.ts index 2561e31..67eec5f 100644 --- a/src/lib/auth.ts +++ b/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, diff --git a/src/server/mcp/api-key-auth.test.ts b/src/server/mcp/api-key-auth.test.ts index 1950dae..872772e 100644 --- a/src/server/mcp/api-key-auth.test.ts +++ b/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", }); diff --git a/src/server/mcp/api-key-auth.ts b/src/server/mcp/api-key-auth.ts index 2d3d485..0de319d 100644 --- a/src/server/mcp/api-key-auth.ts +++ b/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); diff --git a/worker-configuration.d.ts b/worker-configuration.d.ts index 9ce19fd..1fb726a 100644 --- a/worker-configuration.d.ts +++ b/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[0]['payload']>;