Skip to content

hardening(mcp): unguarded validateProMcpToken await in runProPreChecks — a rejection escapes as raw 500 on every gated MCP call with zero Sentry #4860

Description

@koala73

What

In api/mcp/auth.ts runProPreChecks, the token-validation call is the only unguarded await on the pro-gated path:

const validation = await deps.validateProMcpToken(context.mcpTokenId);   // ← no try/catch

Its sibling right below — deps.getEntitlements(...) — IS wrapped (catch → captureSilentError → fail-closed 401 "Subscription not active"). There is no top-level catch in mcpHandler or the edge entry, so a rejection at this line escapes as a raw HTTP 500 on every gated method (tools/call incl. quota-exempt describe_tool, gated resources/read, SSE replay GET), while initialize/tools/list stay healthy (PUBLIC_MCP_METHODS skip pre-checks) — and nothing reaches Sentry because captureSilentError never runs.

Why it matters

  • Latent today: the wired helper validateProMcpTokenOrNull (server/_shared/pro-mcp-token.ts) is internally guarded and returns {userId} | null without rejecting. But the safety lives entirely inside the helper — one refactor that lets an exception escape turns into "every tools/call 500s for OAuth users, tools/list fine, zero telemetry".
  • That failure fingerprint exactly matches a paying-customer support ticket from 2026-07-05 (Pro/API subscriber: "tools/list succeeds but every tools/call — including describe_tool — returns HTTP 500", not reproducible afterward; see bug(mcp): /mcp rejects all customer-issued wm_ API keys — X-WorldMonitor-Key path only checks WORLDMONITOR_VALID_KEYS env allowlist #4859 for the ticket's other half). We can't retro-confirm the Jul 3–4 deploys, but hardening this makes any recurrence visible in Sentry instead of silent.
  • No test covers validateProMcpToken rejecting (there is one for getEntitlements throwing).

Fix

  1. Wrap the validateProMcpToken await in try/catch mirroring the getEntitlements block: captureSilentError(err, { tags: { route: 'api/mcp', step: 'pro-token-validate' } }) → fail-closed 401 (or 503 + Retry-After, matching the transient semantics the helper already encodes).
  2. Add the missing test: deps.validateProMcpToken rejecting → expect structured 401/503, not a thrown-through 500.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions