From 280fa274a31cc7c722907543a6ed2742d52e419f Mon Sep 17 00:00:00 2001 From: Makisuo Date: Sat, 29 Aug 2026 02:14:31 +0200 Subject: [PATCH 1/3] feat(mcp): rate-limit the authenticated MCP tool-call surface per credential POST /mcp had no rate limiting while its tool calls fan out into warehouse queries and LLM calls, making it the most expensive unlimited endpoint. - Extract the v2 limiter's fail-open check (allowed/limited/failed_open with maple.rate_limit.outcome telemetry) into makeRateLimitCheck and build the new McpToolRateLimiter on it, on a dedicated 120/60s binding: the budget has to differ from the v2 API's 600/60s, and a limit is fixed per binding. - Key per credential, never IP: key: for OAuth tokens and manual MCP keys (rolling a key starts a fresh budget; the secret never reaches the counter), user: for session auth. The internal service token is exempt - one shared token would put every internal caller in a single bucket. - Over-budget requests get 429 + Retry-After: 60 with V2RateLimited's wording in the MCP surface's { error, message } envelope; span records outcome, limit, and period. - Bind MCP_TOOLS_RATE_LIMITER in alchemy.run.ts and mirror it in wrangler.jsonc; document the limit in docs/mcp-oauth.md (api-v2.md covers only the /v2 surface). --- apps/api/alchemy.run.ts | 6 + apps/api/src/mcp/app.test.ts | 76 ++++++++++++ apps/api/src/mcp/app.ts | 42 ++++++- apps/api/src/mcp/lib/resolve-tenant.ts | 16 ++- apps/api/src/routes/v2/api-keys.http.test.ts | 4 +- apps/api/src/runtime/http-graph.ts | 2 + .../api/src/services/auth/ApiV2RateLimiter.ts | 114 +++++++++++------- .../services/auth/McpToolRateLimiter.test.ts | 43 +++++++ .../src/services/auth/McpToolRateLimiter.ts | 33 +++++ apps/api/wrangler.jsonc | 8 ++ docs/mcp-oauth.md | 10 ++ 11 files changed, 301 insertions(+), 53 deletions(-) create mode 100644 apps/api/src/services/auth/McpToolRateLimiter.test.ts create mode 100644 apps/api/src/services/auth/McpToolRateLimiter.ts diff --git a/apps/api/alchemy.run.ts b/apps/api/alchemy.run.ts index d0f692a33..833b2ce4e 100644 --- a/apps/api/alchemy.run.ts +++ b/apps/api/alchemy.run.ts @@ -383,6 +383,12 @@ export const createMapleApi = ({ stage, domains, replayBlobs }: CreateMapleApiOp namespaceId: 2026072102, simple: { limit: 60, period: 60 }, }), + // Authenticated POST /mcp, per credential. Tighter than the v2 API's + // because tool calls fan out into warehouse queries and LLM calls. + MCP_TOOLS_RATE_LIMITER: Cloudflare.RateLimit("MCP_TOOLS_RATE_LIMITER", { + namespaceId: 2026082901, + simple: { limit: 120, period: 60 }, + }), API_V2_RATE_LIMIT_PARTITION: formatMapleStage(stage), // Production only: preview/stg workers run the same email crons against // their own DB branches, so a binding here means every live stage sends diff --git a/apps/api/src/mcp/app.test.ts b/apps/api/src/mcp/app.test.ts index dc4e2ee21..ba0e5374c 100644 --- a/apps/api/src/mcp/app.test.ts +++ b/apps/api/src/mcp/app.test.ts @@ -6,6 +6,8 @@ import { Env } from "@/platform/Env" import { cleanupTestDbs, createTestDb, type TestDb } from "@/platform/test-pglite" import { ApiKeysService } from "@/services/org/ApiKeysService" import { AuthService } from "@/services/auth/AuthService" +import type { RateLimiterApi } from "@/services/auth/ApiV2RateLimiter" +import { McpToolRateLimiter } from "@/services/auth/McpToolRateLimiter" import { McpToolExecutor, type McpToolExecutorApi } from "./dispatcher" import { type SessionPayload, sessionStore } from "./lib/session-store" import { McpLive } from "./app" @@ -18,6 +20,10 @@ const makeMcpToolExecutorStubLayer = ( Effect.succeed({ content: [{ type: "text" as const, text: "ok" }] }), ) => Layer.succeed(McpToolExecutor, { execute }) +const makeRateLimiterStubLayer = ( + check: RateLimiterApi["check"] = () => Effect.succeed("allowed" as const), +) => Layer.succeed(McpToolRateLimiter, { check }) + const testConfig = () => ConfigProvider.layer( ConfigProvider.fromUnknown({ @@ -40,6 +46,7 @@ describe("MCP HTTP authorization", () => { ApiKeysService.layer, AuthService.layer, makeMcpToolExecutorStubLayer(), + makeRateLimiterStubLayer(), ).pipe(Layer.provideMerge(base)) const routes = McpLive.pipe(Layer.provideMerge(services)) const { handler, dispose } = HttpRouter.toWebHandler(routes, { disableLogger: true }) @@ -86,6 +93,7 @@ describe("MCP HTTP authorization", () => { executedOrgId = tenant.orgId return Effect.succeed({ content: [{ type: "text" as const, text: "ok" }] }) }), + makeRateLimiterStubLayer(), ).pipe(Layer.provideMerge(base)) const orgId = Schema.decodeUnknownSync(OrgId)("org_test") const userId = Schema.decodeUnknownSync(UserId)("user_test") @@ -202,6 +210,7 @@ describe("MCP HTTP authorization", () => { ApiKeysService.layer, AuthService.layer, makeMcpToolExecutorStubLayer(), + makeRateLimiterStubLayer(), ).pipe(Layer.provideMerge(base)) const orgId = Schema.decodeUnknownSync(OrgId)("org_test") const userId = Schema.decodeUnknownSync(UserId)("user_test") @@ -286,6 +295,7 @@ describe("MCP HTTP authorization", () => { ApiKeysService.layer, AuthService.layer, makeMcpToolExecutorStubLayer(), + makeRateLimiterStubLayer(), ).pipe(Layer.provideMerge(base)) const orgId = Schema.decodeUnknownSync(OrgId)("org_test") const userId = Schema.decodeUnknownSync(UserId)("user_test") @@ -366,4 +376,70 @@ describe("MCP HTTP authorization", () => { await second.dispose() } }) + + it("refuses an over-budget credential with 429 and Retry-After", async () => { + const db = createTestDb(createdDbs) + const base = Layer.mergeAll(db.layer, Env.layer.pipe(Layer.provide(testConfig()))) + const limitedKeys: string[] = [] + let executed = false + const services = Layer.mergeAll( + ApiKeysService.layer, + AuthService.layer, + makeMcpToolExecutorStubLayer(() => { + executed = true + return Effect.succeed({ content: [{ type: "text" as const, text: "ok" }] }) + }), + makeRateLimiterStubLayer((key) => { + limitedKeys.push(key) + return Effect.succeed("limited" as const) + }), + ).pipe(Layer.provideMerge(base)) + const orgId = Schema.decodeUnknownSync(OrgId)("org_test") + const userId = Schema.decodeUnknownSync(UserId)("user_test") + const key = await Effect.runPromise( + Effect.gen(function* () { + const apiKeys = yield* ApiKeysService + return yield* apiKeys.create(orgId, userId, { name: "Rate limit test", kind: "mcp" }) + }).pipe(Effect.provide(services)), + ) + const routes = McpLive.pipe(Layer.provideMerge(services)) + const { handler, dispose } = HttpRouter.toWebHandler(routes, { disableLogger: true }) + try { + const response = await handler( + new Request("https://api.example.com/mcp", { + method: "POST", + headers: { + authorization: `Bearer ${key.secret}`, + accept: "application/json, text/event-stream", + "content-type": "application/json", + host: "api.example.com", + "x-forwarded-proto": "https", + }, + body: JSON.stringify({ + jsonrpc: "2.0", + id: 1, + method: "initialize", + params: { + protocolVersion: "2025-06-18", + capabilities: {}, + clientInfo: { name: "test", version: "1.0.0" }, + }, + }), + }), + Context.empty() as never, + ) + const body = await response.clone().json() + expect({ + status: response.status, + retryAfter: response.headers.get("retry-after"), + error: body.error, + executed, + }).toEqual({ status: 429, retryAfter: "60", error: "rate_limited", executed: false }) + // Buckets are per internal key id, so a rolled key gets a fresh budget + // and the raw secret never reaches the counter key. + expect(limitedKeys).toEqual([`key:${key.id}`]) + } finally { + await dispose() + } + }) }) diff --git a/apps/api/src/mcp/app.ts b/apps/api/src/mcp/app.ts index 867536a2d..874cd0652 100644 --- a/apps/api/src/mcp/app.ts +++ b/apps/api/src/mcp/app.ts @@ -12,6 +12,11 @@ import type { McpToolExecutor } from "./dispatcher" import { CurrentMcpRequestTenant, CurrentMcpTenant, resolveHttpMcpTenant } from "./lib/query-warehouse" import { ApiKeysService } from "@/services/org/ApiKeysService" import { AuthService } from "@/services/auth/AuthService" +import { + MCP_TOOLS_RATE_LIMIT_PERIOD_SECONDS, + MCP_TOOLS_RATE_LIMIT_REQUESTS, + McpToolRateLimiter, +} from "@/services/auth/McpToolRateLimiter" import { Env } from "@/platform/Env" const MCP_PROTOCOL_VERSION_HEADER = "mcp-protocol-version" @@ -93,11 +98,29 @@ const mcpUnavailable = () => ), ) +// Wording mirrors the v2 envelope's `V2RateLimited`; the body stays in this +// surface's `{ error, message }` shape like the 401/503 responses above. +const mcpRateLimited = () => + HttpServerResponse.jsonUnsafe( + { + error: "rate_limited", + message: "Too many requests. Retry after the interval in the Retry-After header.", + }, + { + status: 429, + headers: { + "retry-after": String(MCP_TOOLS_RATE_LIMIT_PERIOD_SECONDS), + "cache-control": "no-store", + }, + }, + ) + const McpAuthorizationMiddleware = HttpRouter.middleware<{ provides: CurrentMcpTenant }>()( Effect.gen(function* () { const apiKeys = yield* ApiKeysService const auth = yield* AuthService const env = yield* Env + const rateLimiter = yield* McpToolRateLimiter return (httpEffect) => Effect.gen(function* () { const request = yield* HttpServerRequest.HttpServerRequest @@ -106,9 +129,20 @@ const McpAuthorizationMiddleware = HttpRouter.middleware<{ provides: CurrentMcpT Effect.provideService(AuthService, auth), Effect.provideService(Env, env), Effect.flatMap((tenant) => - Effect.provideService(httpEffect, CurrentMcpTenant, tenant).pipe( - Effect.provideService(CurrentMcpRequestTenant, tenant), - ), + Effect.gen(function* () { + if (tenant.rateLimitCredentialId !== undefined) { + const outcome = yield* rateLimiter.check(tenant.rateLimitCredentialId) + yield* Effect.annotateCurrentSpan({ + "maple.rate_limit.outcome": outcome, + "maple.rate_limit.limit": MCP_TOOLS_RATE_LIMIT_REQUESTS, + "maple.rate_limit.period_seconds": MCP_TOOLS_RATE_LIMIT_PERIOD_SECONDS, + }) + if (outcome === "limited") return mcpRateLimited() + } + return yield* Effect.provideService(httpEffect, CurrentMcpTenant, tenant).pipe( + Effect.provideService(CurrentMcpRequestTenant, tenant), + ) + }), ), Effect.catchTags({ "@maple/mcp/errors/McpAuthMissingError": () => mcpChallenge(false), @@ -137,7 +171,7 @@ const McpHttpLive = McpServer.layerHttp({ export const McpLive: Layer.Layer< never, Cause.IllegalArgumentError, - HttpRouter.HttpRouter | ApiKeysService | AuthService | Env | McpToolExecutor + HttpRouter.HttpRouter | ApiKeysService | AuthService | Env | McpToolExecutor | McpToolRateLimiter > = Layer.mergeAll( McpToolsLive, DebugErrorsPrompt, diff --git a/apps/api/src/mcp/lib/resolve-tenant.ts b/apps/api/src/mcp/lib/resolve-tenant.ts index 3e7168602..f72ecca4b 100644 --- a/apps/api/src/mcp/lib/resolve-tenant.ts +++ b/apps/api/src/mcp/lib/resolve-tenant.ts @@ -15,6 +15,16 @@ import { recordExpectedMcpFailure } from "@/mcp/expected-failures" import { sessionStore } from "@/mcp/lib/session-store" const INTERNAL_SERVICE_PREFIX = "maple_svc_" + +/** + * The tenant plus the rate-limit identity of the credential that produced it, + * consumed by the MCP authorization middleware. Absent only for internal + * service auth: that is Maple's own traffic behind one shared token, so a + * single bucket would throttle every internal caller together. + */ +export interface McpAuthenticatedTenant extends McpTenantContext { + readonly rateLimitCredentialId?: string +} const decodeOrgId = Schema.decodeUnknownEffect(OrgId) const decodeUserId = Schema.decodeUnknownEffect(UserId) const decodeActorIdOption = Schema.decodeUnknownOption(ActorId) @@ -136,7 +146,7 @@ export const resolveMcpTenantContext = Effect.fn("resolveMcpTenantContext")( roles: [], authMode: "self_hosted", ...(mcpClientName ? { mcpClientName } : undefined), - } as McpTenantContext + } as McpAuthenticatedTenant } return yield* new McpAuthInvalidError({ @@ -216,9 +226,10 @@ export const resolveMcpTenantContext = Effect.fn("resolveMcpTenantContext")( userId: validUserId, roles: resolved.roles ?? apiKeyDefaultRoles, authMode: "self_hosted", + rateLimitCredentialId: `key:${resolved.keyId}`, ...(actorId ? { actorId } : undefined), ...(mcpClientName ? { mcpClientName } : undefined), - } as McpTenantContext + } as McpAuthenticatedTenant } // Fall back to existing Clerk / self-hosted session auth @@ -238,6 +249,7 @@ export const resolveMcpTenantContext = Effect.fn("resolveMcpTenantContext")( userId: tenant.userId, roles: [...tenant.roles], authMode: tenant.authMode, + rateLimitCredentialId: `user:${tenant.userId}`, ...(mcpClientName ? { mcpClientName } : undefined), } }, diff --git a/apps/api/src/routes/v2/api-keys.http.test.ts b/apps/api/src/routes/v2/api-keys.http.test.ts index b73d59b7c..c4329b32f 100644 --- a/apps/api/src/routes/v2/api-keys.http.test.ts +++ b/apps/api/src/routes/v2/api-keys.http.test.ts @@ -12,7 +12,7 @@ import { AuthService } from "@/services/auth/AuthService" import { DashboardPersistenceService } from "@/services/dashboards/DashboardPersistenceService" import { SharedDashboardService } from "@/services/dashboards/SharedDashboardService" import { ApiAuthorizationV2Layer } from "@/services/auth/ApiAuthorizationV2Layer" -import { ApiV2RateLimiter, type ApiV2RateLimiterApi } from "@/services/auth/ApiV2RateLimiter" +import { ApiV2RateLimiter, type RateLimiterApi } from "@/services/auth/ApiV2RateLimiter" import { V2TransportErrorBoundaryLive } from "./error-envelope" import { AlertsServiceStubLayer, @@ -49,7 +49,7 @@ const testConfig = () => ) const makeHarness = ( - checkRateLimit: ApiV2RateLimiterApi["check"] = () => Effect.succeed("allowed" as const), + checkRateLimit: RateLimiterApi["check"] = () => Effect.succeed("allowed" as const), ) => { const testDb = createTestDb(createdDbs) const envLive = Env.layer.pipe(Layer.provide(testConfig())) diff --git a/apps/api/src/runtime/http-graph.ts b/apps/api/src/runtime/http-graph.ts index 40adb9d09..da89189f2 100644 --- a/apps/api/src/runtime/http-graph.ts +++ b/apps/api/src/runtime/http-graph.ts @@ -66,6 +66,7 @@ import { ApiAuthorizationLayer } from "@/services/auth/ApiAuthorizationLayer" import { ApiAuthorizationV2Layer } from "@/services/auth/ApiAuthorizationV2Layer" import { SessionAuthorizationLayer } from "@/services/auth/SessionAuthorizationLayer" import { ApiV2RateLimiter } from "@/services/auth/ApiV2RateLimiter" +import { McpToolRateLimiter } from "@/services/auth/McpToolRateLimiter" import { EdgeCacheService } from "@maple/cache" import { CacheBackendLive } from "@/platform/CacheBackendLive" import { OrgMembershipService } from "@/services/auth/OrgMembershipService" @@ -183,6 +184,7 @@ export const ApiAuthLive = Layer.mergeAll( SessionAuthorizationLayer, ).pipe( Layer.provideMerge(ApiV2RateLimiter.layer), + Layer.provideMerge(McpToolRateLimiter.layer), Layer.provideMerge(ApiKeysService.layer), // Membership verification for `x-maple-org-id`. Only the v2 layer asks for // it; without it that layer cannot build, which is deliberate — the header diff --git a/apps/api/src/services/auth/ApiV2RateLimiter.ts b/apps/api/src/services/auth/ApiV2RateLimiter.ts index 850542ee6..ebace43e2 100644 --- a/apps/api/src/services/auth/ApiV2RateLimiter.ts +++ b/apps/api/src/services/auth/ApiV2RateLimiter.ts @@ -7,13 +7,13 @@ export const API_V2_RATE_LIMIT_PARTITION = "API_V2_RATE_LIMIT_PARTITION" export const API_V2_RATE_LIMIT_REQUESTS = 600 export const API_V2_RATE_LIMIT_PERIOD_SECONDS = 60 -export type ApiV2RateLimitOutcome = "allowed" | "limited" | "failed_open" +export type RateLimitOutcome = "allowed" | "limited" | "failed_open" interface RateLimitBinding { readonly limit: (options: { readonly key: string }) => Promise<{ readonly success: boolean }> } -export interface ApiV2RateLimiterApi { +export interface RateLimiterApi { /** * Rate-limit one caller-chosen key. * @@ -22,11 +22,11 @@ export interface ApiV2RateLimiterApi { * per client IP, neither of which is an API key. The `v2:` / `share:` scoping * prefix therefore belongs to the caller — see `makeApiV2RateLimitKey`. */ - readonly check: (key: string) => Effect.Effect + readonly check: (key: string) => Effect.Effect } -class ApiV2RateLimiterBindingError extends Schema.TaggedError()( - "@maple/api/services/ApiV2RateLimiterBindingError", +class RateLimitBindingError extends Schema.TaggedError()( + "@maple/api/services/RateLimitBindingError", { message: Schema.String, cause: Schema.Defect(), @@ -63,52 +63,76 @@ export const shareIpRateLimitKey = (ip: string): string => `shareip:${ip}` */ export const shareOgRateLimitKey = (shareKeyPrefix: string): string => `shareog:${shareKeyPrefix}` -const warnFailedOpen = (reason: "binding_missing" | "partition_missing" | "binding_error", cause?: unknown) => - Effect.logWarning("API v2 rate limiter unavailable; allowing request").pipe( - Effect.annotateLogs({ - "maple.rate_limit.outcome": "failed_open", - "maple.rate_limit.reason": reason, - ...(cause instanceof Error ? { "error.type": cause.name } : undefined), - }), - ) +export interface RateLimitCheckConfig { + /** `WorkerEnvironment` name of the Cloudflare rate-limit binding to call. */ + readonly bindingName: string + /** Span name for the check, e.g. `"ApiV2RateLimiter.check"`. */ + readonly spanName: string + /** Warn log emitted when the limiter fails open. */ + readonly failOpenMessage: string +} + +/** + * The one fail-open check implementation behind every limiter service: allow / + * limited from the binding, `failed_open` (with `maple.rate_limit.outcome` + * telemetry, never a silent pass) when the binding or partition is unavailable. + */ +export const makeRateLimitCheck = ( + environment: Record, + config: RateLimitCheckConfig, +): RateLimiterApi["check"] => { + const warnFailedOpen = ( + reason: "binding_missing" | "partition_missing" | "binding_error", + cause?: unknown, + ) => + Effect.logWarning(config.failOpenMessage).pipe( + Effect.annotateLogs({ + "maple.rate_limit.outcome": "failed_open", + "maple.rate_limit.reason": reason, + ...(cause instanceof Error ? { "error.type": cause.name } : undefined), + }), + ) + + return Effect.fn(config.spanName)(function* (key: string) { + const binding = environment[config.bindingName] + if (!isRateLimitBinding(binding)) { + yield* warnFailedOpen("binding_missing") + return "failed_open" as const + } + + const partition = readPartition(environment) + if (partition === undefined) { + yield* warnFailedOpen("partition_missing") + return "failed_open" as const + } + + return yield* Effect.tryPromise({ + try: () => binding.limit({ key: makeApiV2RateLimitKey(partition, key) }), + catch: (cause) => + new RateLimitBindingError({ + message: "Cloudflare rate-limit binding call failed", + cause, + }), + }).pipe( + Effect.map(({ success }) => (success ? ("allowed" as const) : ("limited" as const))), + Effect.catchTag("@maple/api/services/RateLimitBindingError", (error) => + warnFailedOpen("binding_error", error.cause).pipe(Effect.as("failed_open")), + ), + ) + }) +} -export class ApiV2RateLimiter extends Context.Service()( +export class ApiV2RateLimiter extends Context.Service()( "@maple/api/services/ApiV2RateLimiter", { make: Effect.gen(function* () { const environment = yield* WorkerEnvironment - - const check = Effect.fn("ApiV2RateLimiter.check")(function* (key: string) { - const binding = environment[API_V2_RATE_LIMIT_BINDING] - if (!isRateLimitBinding(binding)) { - yield* warnFailedOpen("binding_missing") - return "failed_open" as const - } - - const partition = readPartition(environment) - if (partition === undefined) { - yield* warnFailedOpen("partition_missing") - return "failed_open" as const - } - - return yield* Effect.tryPromise({ - try: () => binding.limit({ key: makeApiV2RateLimitKey(partition, key) }), - catch: (cause) => - new ApiV2RateLimiterBindingError({ - message: "Cloudflare rate-limit binding call failed", - cause, - }), - }).pipe( - Effect.map(({ success }) => (success ? ("allowed" as const) : ("limited" as const))), - Effect.catchTag("@maple/api/services/ApiV2RateLimiterBindingError", (error) => - warnFailedOpen("binding_error", error.cause).pipe( - Effect.as("failed_open"), - ), - ), - ) + const check = makeRateLimitCheck(environment, { + bindingName: API_V2_RATE_LIMIT_BINDING, + spanName: "ApiV2RateLimiter.check", + failOpenMessage: "API v2 rate limiter unavailable; allowing request", }) - - return { check } satisfies ApiV2RateLimiterApi + return { check } satisfies RateLimiterApi }), }, ) { diff --git a/apps/api/src/services/auth/McpToolRateLimiter.test.ts b/apps/api/src/services/auth/McpToolRateLimiter.test.ts new file mode 100644 index 000000000..646700759 --- /dev/null +++ b/apps/api/src/services/auth/McpToolRateLimiter.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, it } from "@effect/vitest" +import { Effect, Layer } from "effect" +import { WorkerEnvironment } from "@maple/effect-cloudflare/worker-environment" +import { API_V2_RATE_LIMIT_PARTITION, makeApiV2RateLimitKey } from "./ApiV2RateLimiter" +import { MCP_TOOLS_RATE_LIMIT_BINDING, McpToolRateLimiter } from "./McpToolRateLimiter" + +const limiterLayer = (environment: Record) => + McpToolRateLimiter.layer.pipe(Layer.provide(Layer.succeed(WorkerEnvironment, environment))) + +describe("McpToolRateLimiter", () => { + it.effect("counts against its own binding under the stage partition", () => { + const keys: string[] = [] + const environment = { + [API_V2_RATE_LIMIT_PARTITION]: "stg", + [MCP_TOOLS_RATE_LIMIT_BINDING]: { + limit: ({ key }: { key: string }) => { + keys.push(key) + return Promise.resolve({ success: false }) + }, + }, + } + + return Effect.gen(function* () { + const limiter = yield* McpToolRateLimiter + expect(yield* limiter.check("key:abc")).toBe("limited") + expect(keys).toEqual([makeApiV2RateLimitKey("stg", "key:abc")]) + }).pipe(Effect.provide(limiterLayer(environment))) + }) + + it.effect("fails open when only the v2 binding is present", () => + Effect.gen(function* () { + const limiter = yield* McpToolRateLimiter + expect(yield* limiter.check("key:abc")).toBe("failed_open") + }).pipe( + Effect.provide( + limiterLayer({ + [API_V2_RATE_LIMIT_PARTITION]: "prd", + API_V2_RATE_LIMITER: { limit: () => Promise.resolve({ success: true }) }, + }), + ), + ), + ) +}) diff --git a/apps/api/src/services/auth/McpToolRateLimiter.ts b/apps/api/src/services/auth/McpToolRateLimiter.ts new file mode 100644 index 000000000..2e98d63ed --- /dev/null +++ b/apps/api/src/services/auth/McpToolRateLimiter.ts @@ -0,0 +1,33 @@ +import { Context, Effect, Layer } from "effect" +import { WorkerEnvironment } from "@maple/effect-cloudflare/worker-environment" +import { makeRateLimitCheck, type RateLimiterApi } from "./ApiV2RateLimiter" + +export const MCP_TOOLS_RATE_LIMIT_BINDING = "MCP_TOOLS_RATE_LIMITER" +export const MCP_TOOLS_RATE_LIMIT_REQUESTS = 120 +export const MCP_TOOLS_RATE_LIMIT_PERIOD_SECONDS = 60 + +/** + * Per-credential limiter for the authenticated MCP surface (`POST /mcp`). + * + * A dedicated binding rather than `API_V2_RATE_LIMITER` because the budget must + * differ: MCP tool calls fan out into warehouse queries and LLM calls, so + * 600/60s per credential is a much larger cost ceiling than it is on `/v2`. + * Keys arrive pre-scoped by the resolver (`key:` / `user:`) and + * share the stage partition with the other limiters. + */ +export class McpToolRateLimiter extends Context.Service()( + "@maple/api/services/McpToolRateLimiter", + { + make: Effect.gen(function* () { + const environment = yield* WorkerEnvironment + const check = makeRateLimitCheck(environment, { + bindingName: MCP_TOOLS_RATE_LIMIT_BINDING, + spanName: "McpToolRateLimiter.check", + failOpenMessage: "MCP tool rate limiter unavailable; allowing request", + }) + return { check } satisfies RateLimiterApi + }), + }, +) { + static readonly layer = Layer.effect(this, this.make) +} diff --git a/apps/api/wrangler.jsonc b/apps/api/wrangler.jsonc index cb25adbaf..05d714abc 100644 --- a/apps/api/wrangler.jsonc +++ b/apps/api/wrangler.jsonc @@ -26,6 +26,14 @@ "period": 60, }, }, + { + "name": "MCP_TOOLS_RATE_LIMITER", + "namespace_id": "2026082901", + "simple": { + "limit": 120, + "period": 60, + }, + }, ], // Cron schedules — handler: worker.ts `scheduled`, dispatched on `event.cron`. // "0 */12 * * *" = VCS sync backstop; "0 * * * *" = scrape_target_checks diff --git a/docs/mcp-oauth.md b/docs/mcp-oauth.md index 00a770598..5ebe65924 100644 --- a/docs/mcp-oauth.md +++ b/docs/mcp-oauth.md @@ -30,6 +30,16 @@ rotate on every use. Reusing a rotated refresh token revokes the whole grant fam Manual MCP keys remain supported for clients without OAuth. They continue to use the existing `Authorization: Bearer ...` configuration and are isolated to the MCP server by `kind: "mcp"`. +## Rate limiting + +Authenticated `POST /mcp` requests share one budget per credential — the internal key ID for +OAuth tokens and manual MCP keys, the user for dashboard sessions — of **120 requests per 60 +seconds**, partitioned by deployment stage. Exceeding it returns `429` with +`{ "error": "rate_limited" }` and `Retry-After: 60`. The limiter fails open with +`maple.rate_limit.outcome=failed_open` telemetry, like the `/v2` limiter documented in +[api-v2.md](api-v2.md#rate-limiting); the OAuth handshake endpoints above have their own +separate 60/60s budget. + ## Browser approval The API redirects valid authorization requests to `/mcp-authorize` on `MAPLE_APP_BASE_URL`. The From e7058fa71c63f2930eb2bced6cf1c8902165a151 Mon Sep 17 00:00:00 2001 From: Makisuo Date: Mon, 31 Aug 2026 00:00:58 +0200 Subject: [PATCH 2/3] fix(mcp): raise the MCP tool budget to 600/60s 120/60s was set as a cost ceiling, but it lands as a usage ceiling: an agent session bursts tool calls in a way a hand-rolled /v2 client never does, and two per second per credential is inside normal single-user traffic. Match the v2 API's 600/60s and treat the limiter as the runaway-loop backstop it actually is. Cost control belongs in the per-org spend limits, not in a per-credential request counter. --- apps/api/alchemy.run.ts | 6 +++--- apps/api/src/services/auth/McpToolRateLimiter.ts | 9 +++++---- apps/api/wrangler.jsonc | 2 +- docs/mcp-oauth.md | 2 +- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/apps/api/alchemy.run.ts b/apps/api/alchemy.run.ts index 833b2ce4e..8d3503448 100644 --- a/apps/api/alchemy.run.ts +++ b/apps/api/alchemy.run.ts @@ -383,11 +383,11 @@ export const createMapleApi = ({ stage, domains, replayBlobs }: CreateMapleApiOp namespaceId: 2026072102, simple: { limit: 60, period: 60 }, }), - // Authenticated POST /mcp, per credential. Tighter than the v2 API's - // because tool calls fan out into warehouse queries and LLM calls. + // Authenticated POST /mcp, per credential. Matches the v2 API's budget: + // agents burst tool calls, so this is a runaway-loop backstop. MCP_TOOLS_RATE_LIMITER: Cloudflare.RateLimit("MCP_TOOLS_RATE_LIMITER", { namespaceId: 2026082901, - simple: { limit: 120, period: 60 }, + simple: { limit: 600, period: 60 }, }), API_V2_RATE_LIMIT_PARTITION: formatMapleStage(stage), // Production only: preview/stg workers run the same email crons against diff --git a/apps/api/src/services/auth/McpToolRateLimiter.ts b/apps/api/src/services/auth/McpToolRateLimiter.ts index 2e98d63ed..ba64e9a39 100644 --- a/apps/api/src/services/auth/McpToolRateLimiter.ts +++ b/apps/api/src/services/auth/McpToolRateLimiter.ts @@ -3,15 +3,16 @@ import { WorkerEnvironment } from "@maple/effect-cloudflare/worker-environment" import { makeRateLimitCheck, type RateLimiterApi } from "./ApiV2RateLimiter" export const MCP_TOOLS_RATE_LIMIT_BINDING = "MCP_TOOLS_RATE_LIMITER" -export const MCP_TOOLS_RATE_LIMIT_REQUESTS = 120 +export const MCP_TOOLS_RATE_LIMIT_REQUESTS = 600 export const MCP_TOOLS_RATE_LIMIT_PERIOD_SECONDS = 60 /** * Per-credential limiter for the authenticated MCP surface (`POST /mcp`). * - * A dedicated binding rather than `API_V2_RATE_LIMITER` because the budget must - * differ: MCP tool calls fan out into warehouse queries and LLM calls, so - * 600/60s per credential is a much larger cost ceiling than it is on `/v2`. + * A dedicated binding rather than `API_V2_RATE_LIMITER` so the budget can move + * independently — an agent driving MCP bursts tool calls far harder than a + * client hand-rolling `/v2` requests. It currently sits at the same 600/60s, + * which is a runaway-loop backstop rather than a cost ceiling. * Keys arrive pre-scoped by the resolver (`key:` / `user:`) and * share the stage partition with the other limiters. */ diff --git a/apps/api/wrangler.jsonc b/apps/api/wrangler.jsonc index 05d714abc..e595187e9 100644 --- a/apps/api/wrangler.jsonc +++ b/apps/api/wrangler.jsonc @@ -30,7 +30,7 @@ "name": "MCP_TOOLS_RATE_LIMITER", "namespace_id": "2026082901", "simple": { - "limit": 120, + "limit": 600, "period": 60, }, }, diff --git a/docs/mcp-oauth.md b/docs/mcp-oauth.md index 5ebe65924..b9ff100e2 100644 --- a/docs/mcp-oauth.md +++ b/docs/mcp-oauth.md @@ -33,7 +33,7 @@ Manual MCP keys remain supported for clients without OAuth. They continue to use ## Rate limiting Authenticated `POST /mcp` requests share one budget per credential — the internal key ID for -OAuth tokens and manual MCP keys, the user for dashboard sessions — of **120 requests per 60 +OAuth tokens and manual MCP keys, the user for dashboard sessions — of **600 requests per 60 seconds**, partitioned by deployment stage. Exceeding it returns `429` with `{ "error": "rate_limited" }` and `Retry-After: 60`. The limiter fails open with `maple.rate_limit.outcome=failed_open` telemetry, like the `/v2` limiter documented in From 95bc492d65a646f22dbf85cf4ba0b65f669c67b9 Mon Sep 17 00:00:00 2001 From: Makisuo Date: Mon, 31 Aug 2026 00:05:05 +0200 Subject: [PATCH 3/3] fix(mcp): move the MCP tool budget to 120/10s A minute-long window is the wrong shape for an agent surface: it either starves a legitimate burst or lets a runaway loop fan out for a full minute before the counter catches it. 120 per 10 seconds is twice the v2 API's sustained throughput with a window short enough that a stuck client backs off in seconds. Retry-After follows the period constant, so it now advertises 10. --- apps/api/alchemy.run.ts | 6 +++--- apps/api/src/mcp/app.test.ts | 2 +- apps/api/src/services/auth/McpToolRateLimiter.ts | 9 +++++---- apps/api/wrangler.jsonc | 4 ++-- docs/mcp-oauth.md | 4 ++-- 5 files changed, 13 insertions(+), 12 deletions(-) diff --git a/apps/api/alchemy.run.ts b/apps/api/alchemy.run.ts index 8d3503448..e9990aa69 100644 --- a/apps/api/alchemy.run.ts +++ b/apps/api/alchemy.run.ts @@ -383,11 +383,11 @@ export const createMapleApi = ({ stage, domains, replayBlobs }: CreateMapleApiOp namespaceId: 2026072102, simple: { limit: 60, period: 60 }, }), - // Authenticated POST /mcp, per credential. Matches the v2 API's budget: - // agents burst tool calls, so this is a runaway-loop backstop. + // Authenticated POST /mcp, per credential. A short window so a runaway + // agent loop is cut off in seconds, at twice the v2 API's throughput. MCP_TOOLS_RATE_LIMITER: Cloudflare.RateLimit("MCP_TOOLS_RATE_LIMITER", { namespaceId: 2026082901, - simple: { limit: 600, period: 60 }, + simple: { limit: 120, period: 10 }, }), API_V2_RATE_LIMIT_PARTITION: formatMapleStage(stage), // Production only: preview/stg workers run the same email crons against diff --git a/apps/api/src/mcp/app.test.ts b/apps/api/src/mcp/app.test.ts index ba0e5374c..48638f659 100644 --- a/apps/api/src/mcp/app.test.ts +++ b/apps/api/src/mcp/app.test.ts @@ -434,7 +434,7 @@ describe("MCP HTTP authorization", () => { retryAfter: response.headers.get("retry-after"), error: body.error, executed, - }).toEqual({ status: 429, retryAfter: "60", error: "rate_limited", executed: false }) + }).toEqual({ status: 429, retryAfter: "10", error: "rate_limited", executed: false }) // Buckets are per internal key id, so a rolled key gets a fresh budget // and the raw secret never reaches the counter key. expect(limitedKeys).toEqual([`key:${key.id}`]) diff --git a/apps/api/src/services/auth/McpToolRateLimiter.ts b/apps/api/src/services/auth/McpToolRateLimiter.ts index ba64e9a39..e13316bbe 100644 --- a/apps/api/src/services/auth/McpToolRateLimiter.ts +++ b/apps/api/src/services/auth/McpToolRateLimiter.ts @@ -3,16 +3,17 @@ import { WorkerEnvironment } from "@maple/effect-cloudflare/worker-environment" import { makeRateLimitCheck, type RateLimiterApi } from "./ApiV2RateLimiter" export const MCP_TOOLS_RATE_LIMIT_BINDING = "MCP_TOOLS_RATE_LIMITER" -export const MCP_TOOLS_RATE_LIMIT_REQUESTS = 600 -export const MCP_TOOLS_RATE_LIMIT_PERIOD_SECONDS = 60 +export const MCP_TOOLS_RATE_LIMIT_REQUESTS = 120 +export const MCP_TOOLS_RATE_LIMIT_PERIOD_SECONDS = 10 /** * Per-credential limiter for the authenticated MCP surface (`POST /mcp`). * * A dedicated binding rather than `API_V2_RATE_LIMITER` so the budget can move * independently — an agent driving MCP bursts tool calls far harder than a - * client hand-rolling `/v2` requests. It currently sits at the same 600/60s, - * which is a runaway-loop backstop rather than a cost ceiling. + * client hand-rolling `/v2` requests. 120/10s allows twice the v2 throughput + * while keeping the window short, so a runaway loop is cut off in seconds + * rather than after a minute of fan-out. * Keys arrive pre-scoped by the resolver (`key:` / `user:`) and * share the stage partition with the other limiters. */ diff --git a/apps/api/wrangler.jsonc b/apps/api/wrangler.jsonc index e595187e9..82b65cd13 100644 --- a/apps/api/wrangler.jsonc +++ b/apps/api/wrangler.jsonc @@ -30,8 +30,8 @@ "name": "MCP_TOOLS_RATE_LIMITER", "namespace_id": "2026082901", "simple": { - "limit": 600, - "period": 60, + "limit": 120, + "period": 10, }, }, ], diff --git a/docs/mcp-oauth.md b/docs/mcp-oauth.md index b9ff100e2..e11c87502 100644 --- a/docs/mcp-oauth.md +++ b/docs/mcp-oauth.md @@ -33,9 +33,9 @@ Manual MCP keys remain supported for clients without OAuth. They continue to use ## Rate limiting Authenticated `POST /mcp` requests share one budget per credential — the internal key ID for -OAuth tokens and manual MCP keys, the user for dashboard sessions — of **600 requests per 60 +OAuth tokens and manual MCP keys, the user for dashboard sessions — of **120 requests per 10 seconds**, partitioned by deployment stage. Exceeding it returns `429` with -`{ "error": "rate_limited" }` and `Retry-After: 60`. The limiter fails open with +`{ "error": "rate_limited" }` and `Retry-After: 10`. The limiter fails open with `maple.rate_limit.outcome=failed_open` telemetry, like the `/v2` limiter documented in [api-v2.md](api-v2.md#rate-limiting); the OAuth handshake endpoints above have their own separate 60/60s budget.