koala73 / koala73/worldmonitor

MCP plan-limit notices: attribute wm_-key-via-env_key usage (narrow #2 residual, deferred from #4648 hardening)

Open
#4,664 0 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
TypeScript
Stars
86.6k
Forks
13.1k
Avg merge
8h 4m
Merged PRs (30d)
825

Description

## Context

Surfaced during the U3-hardening review of #4648 (API/MCP plan-limit detect+notify, shadow mode). The original code review flagged a P1 "MCP notify pipeline can't attribute usage to API-key tiers" (#2). While implementing the fix, the premise turned out **narrower than stated** — so it was deferred out of the hardening branch (`fix/api-plan-limit-notify-hardening`) rather than implemented blindly on the live MCP auth hot path.

## What's actually true

- MCP `env_key` contexts are **either**:
1. operator `WORLDMONITOR_VALID_KEYS` (`api/mcp/auth.ts:170-180`) — no owner, unlimited (enterprise-infra), so no notice is warranted; **or**
2. `wm_*` API keys wrapped in an OAuth bearer (`api/_oauth-token.js:109`) — these **do** have an owner.
- Every paid tier (`api_starter` / `api_business` / `enterprise`, all `mcpAccess: true`) reaches MCP via **OAuth-`pro`**, where `mcp.toolcall` / `mcp.rate_limit_hit` already emit the Clerk `userId` (`principalIdForLog` for `pro` = `context.userId`) → the scanner join **already attributes them correctly**.
- The REST arm of #2 (`api_minute_burst` `customer_id`) was **verified a non-issue**: `server/gateway.ts:923` sets `userApiKeyCustomerRef = userKeyResult.userId`, so REST `customer_id == entitlements.userId`.

**Net real gap:** only the **`wm_*`-key-via-`env_key`** MCP sub-population (a customer who created a `wm_` API key and uses it for MCP through the OAuth bearer path) is unattributed. `mcp.rate_limit_hit` emits `user_id: null` for `env_key` and `mcp.toolcall` emits `hashKeySync(apiKey)` (a hash), neither of which joins to `entitlements.userId`.

## Why it wasn't fixed in the hardening PR

The fix is not mechanical — it touches the **live MCP auth path**, and `mcp.toolcall` fires on **every** tool call, so a naive `getKeyOwner` lookup per emit is a hot-path latency cost. The design decision (resolve the `wm_` key owner at MCP auth-resolution time and cache it on the `McpAuthContext`, then emit a dedicated `owner_user_id` field on both events; scanner groups/joins on `owner_user_id`) belongs in its own MCP-focused change, not bundled with shadow-notify hardening.

## Proposed fix (design)

- Enrich the `env_key` `McpAuthContext` with the resolved owner `userId` at key-validation time (reuse `convex/apiKeys.ts` `getKeyOwner` / `validateKeyByHash`); `null` for operator keys.
- Emit a **new** `owner_user_id` field on `mcp.toolcall` + `mcp.rate_limit_hit` (do **not** overload `principal_id`/`user_id`, which existing analytics consumers depend on). Add it to `MCP_RATE_LIMIT_HIT_TELEMETRY_KEYS` / `MCP_TOOLCALL_TELEMETRY_KEYS` and `tests/mcp-telemetry-schema.test.mjs`.
- Change the scanner `mcpDailyApl` / `mcpBurstApl` to `summarize … by owner_user_id` and join on it.
- Add a test that drives `buildProductionRows` / `normalizeAxiomRows` with an `env_key`-shaped raw Axiom row (current tests inject pre-joined synthetic rows and miss this whole class).

## Related
- PR #4648, epic #4635 (U3), hardening branch `fix/api-plan-limit-notify-hardening`.

Contributor guide

Open the contributing guide

Research direction

Start by tracing env_key authentication through api/mcp/auth.ts, api/_oauth-token.js, and convex/apiKeys.ts, then inspect the MCP telemetry schema tests. Follow the scanner paths mcpDailyApl, mcpBurstApl, buildProductionRows, and normalizeAxiomRows. Done means owner attribution is emitted for both MCP events, accepted by the schema, joined by the scanner, and covered by an env_key-shaped Axiom-row test.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
api, authentication, backend, observability
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.