fix(mcp): accept customer wm_ API keys on /mcp + guard pro-token validate (#4859, #4860)#4863
Conversation
…date (#4859, #4860) Two P0s from a paying-customer ticket (2026-07-05): 1. #4859 — /mcp validated X-WorldMonitor-Key ONLY against the WORLDMONITOR_VALID_KEYS env allowlist, so every dashboard-issued key (Convex userApiKeys) got 401 -32001 "Invalid API key" while the same keys worked on REST — and the endpoint's own 401 hint told users to send exactly that header. New `user_key` context kind: env-miss falls back to the gateway-shared validateUserApiKey hash lookup; gated methods enforce the OWNER's mcpAccess entitlement via the same fail-closed gate as the pro path (never mapped onto env_key, which skips pre-checks); per-user 60/min limiter + the pro daily quota (cache tools bypass downstream metering, so an unquota'd user_key would be an unmetered loophole); _execute fetches authenticate downstream with the raw key so REST metering attributes to the owner. 2. #4860 — `await deps.validateProMcpToken(...)` in runProPreChecks was the only unguarded await on the gated path with no top-level catch behind it: a rejection escaped as a raw 500 on every tools/call (tools/list unaffected, zero Sentry) — the exact fingerprint of the customer's second symptom. Now caught -> captureSilentError (step: pro-token-validate) -> structured 503 + Retry-After. Makes the documented contract (docs/mcp-overview.mdx auth method 2, mcp-quickstart curl examples) actually true for user-issued keys. Tests: tests/mcp-user-key-auth.test.mjs (10 cases, failing-first); mcp/quota/conformance/transport/telemetry/gateway/oauth suites green (360 tests); typecheck:api + biome clean. Claude-Session: https://claude.ai/code/session_01Ex9zkdxdbxYMogHmEK2YXZ
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR fixes two production auth failures on the
Confidence Score: 4/5The production auth paths are structurally correct: user_key contexts are properly gated through entitlement, quota, and rate-limit checks, and the #4860 raw-500 escape is cleanly closed. The one concern worth resolving before shipping is that runContextPreChecks uses an if-chain rather than an exhaustive switch, so a future McpAuthContext variant would silently skip all gate checks — directly contradicting the design's stated guarantee. Fixing it is a small, safe change that makes the invariant a compile-time error instead of a convention. The two production bugs are correctly fixed and the 10-case test suite gives good coverage of the new user_key path. The auth logic in resolveAuthContext (env allowlist → wm_ Convex fallback → 401) and the shared checkMcpEntitlementGate are sound. The gap in runContextPreChecks — where TypeScript would not catch an unhandled McpAuthContext kind — is a real discrepancy between the stated design and the implementation that could produce a silent entitlement bypass in a future change. api/mcp/auth.ts — specifically runContextPreChecks (exhaustiveness) and the buildAuthHeaders JSDoc (missing user_key bullet). All other files are clean. Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| export async function runContextPreChecks( | ||
| context: McpAuthContext, | ||
| deps: McpHandlerDeps, | ||
| resourceMetadataUrl: string, | ||
| corsHeaders: Record<string, string>, | ||
| ctx?: { waitUntil: (p: Promise<unknown>) => void }, | ||
| ): Promise<Response | null> { | ||
| if (context.kind === 'pro') { | ||
| return runProPreChecks(context, deps, resourceMetadataUrl, corsHeaders, ctx); | ||
| } | ||
| if (context.kind === 'user_key') { | ||
| return runUserKeyPreChecks(context, deps, resourceMetadataUrl, corsHeaders, ctx); | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
runContextPreChecks fall-through is not compiler-enforced
The PR description explicitly names this function as "the single kind-dispatched entry point so a future context kind can't silently ship without deciding its gate." However, the if/if/return null chain provides no TypeScript-level enforcement of this invariant. Adding a new variant to McpAuthContext (e.g. { kind: 'webhook_key' }) would compile without error and silently return null here, skipping all pre-checks — the exact entitlement-bypass class the design was meant to prevent. A switch with a never default turns the stated guarantee into a compile-time error.
| export async function runContextPreChecks( | |
| context: McpAuthContext, | |
| deps: McpHandlerDeps, | |
| resourceMetadataUrl: string, | |
| corsHeaders: Record<string, string>, | |
| ctx?: { waitUntil: (p: Promise<unknown>) => void }, | |
| ): Promise<Response | null> { | |
| if (context.kind === 'pro') { | |
| return runProPreChecks(context, deps, resourceMetadataUrl, corsHeaders, ctx); | |
| } | |
| if (context.kind === 'user_key') { | |
| return runUserKeyPreChecks(context, deps, resourceMetadataUrl, corsHeaders, ctx); | |
| } | |
| return null; | |
| } | |
| export async function runContextPreChecks( | |
| context: McpAuthContext, | |
| deps: McpHandlerDeps, | |
| resourceMetadataUrl: string, | |
| corsHeaders: Record<string, string>, | |
| ctx?: { waitUntil: (p: Promise<unknown>) => void }, | |
| ): Promise<Response | null> { | |
| switch (context.kind) { | |
| case 'pro': | |
| return runProPreChecks(context, deps, resourceMetadataUrl, corsHeaders, ctx); | |
| case 'user_key': | |
| return runUserKeyPreChecks(context, deps, resourceMetadataUrl, corsHeaders, ctx); | |
| case 'env_key': | |
| return null; | |
| default: { | |
| // TypeScript will error here if a new McpAuthContext kind is added | |
| // without a corresponding case — enforcing the design invariant that | |
| // every future credential class must explicitly decide its gate. | |
| const _exhaustiveCheck: never = context; | |
| captureSilentError(new Error(`Unhandled McpAuthContext kind`), { | |
| tags: { route: 'api/mcp', step: 'pre-check-dispatch', kind: (_exhaustiveCheck as { kind: string }).kind }, | |
| ctx, | |
| }); | |
| // Fail closed: unknown credential class must never bypass the gate. | |
| return new Response( | |
| JSON.stringify({ jsonrpc: '2.0', id: null, error: { code: -32603, message: 'Service configuration error.' } }), | |
| { status: 503, headers: withMcpNoStore({ 'Content-Type': 'application/json', 'Retry-After': '5', ...corsHeaders }) }, | |
| ); | |
| } | |
| } | |
| } |
…ns + raw 500s become visible (#4866) (#4870) * feat(mcp): emit wm_api_usage request events from /mcp (#4866) /mcp rewrites straight to the handler and never passes server/gateway.ts, so the MCP surface had ZERO rows in Axiom — auth rejections, quota 429s, and successes were all invisible (the #4859 paying-customer diagnosis had to be reconstructed from REST-side rows), and no Vercel log drain exists to catch the console telemetry lines. One RequestEvent per POST / SSE-replay GET, registered on ctx.waitUntil via the gateway's own builders so the envelope is byte-compatible and joinable on customer_id: - auth_kind: anon / enterprise_api_key (env keys) / user_api_key (#4863 user keys, owner userId attributed) / new `mcp_oauth` (pro bearers, Clerk userId attributed — distinct from clerk_jwt so connector traffic never conflates with dashboard traffic). - reason derived from (funnel phase x status), no body parsing: auth_401, tier_403, rate_limit_429, rate_limit_degraded, method_not_allowed, malformed_request, ok, and new `auth_unavailable` (Convex/Redis down during credential resolution — never reads as an invalid-credential wave). - Uncaught inner-handler throws (the #4860 raw-500 class) emit a status-500 event before re-throwing, closing the last blind spot. - OPTIONS/HEAD and the static manifest GET are skipped; emission is fail-silent and gated on USAGE_TELEMETRY, matching the gateway. Tests: tests/mcp-usage-telemetry.test.mjs (8 cases, failing-first) — anon ok, invalid-key auth_401, lapsed-pro tier_403 w/ customer_id, user-key attribution, quota-cap 429, OPTIONS/telemetry-off silence, Axiom-down never breaks the response. MCP family green (251 tests); typecheck:api + biome clean. Claude-Session: https://claude.ai/code/session_01Ex9zkdxdbxYMogHmEK2YXZ * chore(api): list api/mcp/usage.ts in route exceptions (ops plumbing, not a data API) Claude-Session: https://claude.ai/code/session_01Ex9zkdxdbxYMogHmEK2YXZ
1 participant
Summary
Two P0s surfaced by a paying-customer support ticket (2026-07-05). Closes #4859, closes #4860.
#4859 —
/mcprejected every customer-issuedwm_API key. TheX-WorldMonitor-Keypath validated only against the staticWORLDMONITOR_VALID_KEYSenv allowlist (legacy operator keys) and never consulted ConvexuserApiKeys— structural since inception. Every dashboard key got401 -32001 "Invalid API key"on/mcpwhile the same keys worked on REST (verified for the affected customer: 153 × 200 key-authed REST requests, MCP always rejected). The endpoint's own no-credentials 401 hint — and the published docs (docs/mcp-overview.mdxauth method 2,mcp-quickstartcurl examples) — told users to send exactly that header.#4860 — a
validateProMcpTokenrejection escaped as a raw 500. The await inrunProPreCheckswas the only unguarded step on the gated path, with no top-level catch inmcpHandlerbehind it. A rejection produced the exact fingerprint of the customer's second symptom: everytools/call(incl.describe_tool) → raw HTTP 500,tools/listunaffected, zero Sentry. Latent today (the wired helper fail-softs internally) — protection now lives at the call site, not by convention.Design (per the #4859 fix-design constraints)
user_keycontext kind — not mapped ontoenv_key, which skips pre-checks entirely; a lapsed key owner must never bypass the entitlement gate.validateUserApiKey(same Convex hash lookup + Redis cache the REST gateway uses). Identity only; a dep throw → 503, mirroring bearer-resolve.runUserKeyPreChecks→ shared fail-closedmcpAccess/tier/validUntilgate (factored out ofrunProPreChecks, same "Subscription not active." 401).tools/liststays available to a lapsed owner, symmetric with pro.user_keywould be an unmetered data loophole. Raising API-plan MCP allowances above the Pro cap is a deliberate follow-up._executefetches send the raw user key downstream — the REST gateway re-validates and meters the owner (auth_kind=user_api_key), including the fix(security): cancelled/downgraded users keep full API access via un-revoked wm_ keys (apiAccess only checked on premium paths) #4611 apiAccess gate.runContextPreChecksis now the single kind-dispatched entry point so a future context kind can't ship without deciding its gate.captureSilentError(step: pro-token-validate) → structured 503 +Retry-After, so any recurrence is visible in Sentry instead of a silent platform 500.Tests
tests/mcp-user-key-auth.test.mjs(10 cases, written failing-first: 7 failed on the old code): happy path, quota reserve + cap + describe_tool exemption, entitlement-lapsed 401 with tools/list still 200, entitlement-throw fail-closed, unknown-key 401 shape preserved, dep-throw 503, env-key short-circuit (no Convex touch), downstream header contract (user key present, internal-HMAC absent), and the hardening(mcp): unguarded validateProMcpToken await in runProPreChecks — a rejection escapes as raw 500 on every gated MCP call with zero Sentry #4860 rejection → 503.typecheck:api+ biome clean.Verification against the live incident
Live probes during diagnosis confirmed the failure shape this PR fixes: bogus and valid user keys both got
-32001 "Invalid API key"on/mcpwhile the pro OAuth path (verified end-to-end with a minted pro bearer incl. SSE framing and_executeHMAC hop) and REST user-key auth were healthy. Post-deploy plan: re-probe/mcpwith an unknown key (same 401 shape), env key (200), and a fresh pro bearer (200).https://claude.ai/code/session_01Ex9zkdxdbxYMogHmEK2YXZ