feat(mcp): rate-limit client_credentials issuance and CIMD fetch attempts (#163)

[heskew]

Closes #163 (part 4/4 of #159 — the defense-in-depth rate limiter that gates closing the umbrella). Depends on #170 (merged).

Summary

• New src/lib/mcp/rateLimit.ts — a minimal per-node in-memory token bucket: continuous elapsed-time refill, injected clock (tests never sleep), LRU-bounded key space (keys are attacker-chosen client_ids / URLs, so the map is capped and evicts LRU), retryAfterSeconds on block.
• Issuance limiter (token.ts) — at the top of handleClientCredentialsGrant, before resolveClient, so an over-limit client triggers no resolution or crypto work. mcp.clientCredentials.rateLimit requests/min per client_id (default 30; false/0 disables; non-finite → default). Over-limit → 429 { error: "slow_down" } + Retry-After.
• CIMD fetch limiter (cimd.ts) — fixed 10 attempts/min per client_id URL, placed after the cache-hit and in-flight-dedup returns so only genuine fetch attempts consume. Closes the two #163-deferred amplification comments from feat: Client ID Metadata Documents with SSRF-guarded resolution + consent interstitial (#166) #167. No config knob (YAGNI — legit clients are cache-served after first success; only failing documents repeat).
• Discovery/config/docs unchanged in shape; the grant remains client_credentials-only.

Scope decisions (settled before implementation): grant coverage is client_credentials-only (module is generic, extending to other grants is one line each later); CIMD limit is fixed-policy, no knob; buckets are per-node in-memory — a replicated counter is a Harper hot-write anti-pattern, and the assertion replay guard + ≤60s exp bound cross-node abuse.

Where to look

• rateLimit.ts bucket math — refill, capacity clamp, retry-after, and backward-clock safety (elapsedMs > 0 guard means a backward wall-clock jump only ever makes the limiter stricter). Unit-tested with an injected clock.
• Limiter ordering in cimd.ts — the concurrency cap (MAX_CONCURRENT_RESOLUTIONS) is checked before the fetch-limiter take, so a capacity reject doesn't spend a token without fetching.

Deliberate design bounds (adjudicated, no change)

Cross-model review (Gemini leg + Harper domain pass; Codex leg stalled — second consecutive PR, no code-trace-depth second model): zero blockers, zero significant concerns. Two Gemini findings were adjudicated and rejected as by-design, surfaced here so the human reviewer needn't rediscover them:

1. Per-key limiting doesn't stop an identity-rotating flood. True and intended — per-client_id bucketing bounds per-identity issuance; aggregate work is bounded by the pre-existing MAX_CONCURRENT_RESOLUTIONS=16 / MAX_CONCURRENT_DNS=2 caps + SSRF gate + allowedHosts, not by this limiter. This is a defense-in-depth layer, not the primary control.
2. LRU eviction can reset a key's bucket. Real mechanism, but it only ever relaxes a limit (an evicted key returns at full capacity), never starves a victim — and reclaiming one key's tokens costs an attacker maxKeys (10k) requests, each doing more work than it saves. Documented as the accepted tradeoff in the module header.

Also documented (domain-pass note): the "per node" buckets are in fact per worker thread, so N threads = N× effective ceiling — inherited from the existing per-thread CIMD cache/concurrency design, and consistent with the defense-in-depth framing. Docs now say so rather than implying a hard node-wide cap.

Suite: 1033 tests / 1031 pass / 2 pre-existing skips (+14 new: bucket unit suite, grant 429/disable/no-fetch-on-limit, CIMD per-URL isolation). Generated by an LLM (Claude Fable 5).

🤖 Generated with Claude Code

https://claude.ai/code/session_01SerGP6Am3xz2CKyPgbRc73

[github-actions]

Reviewed; no blockers found.

The PR introduces a robust rate-limiting layer for the MCP authorization flow, addressing both client-credentials issuance and CIMD metadata fetch attempts. The implementation is well-aligned with Harper's per-node/per-worker architectural model, effectively mitigating DoS risks and fetch-amplification attacks without introducing hot-write replication bottlenecks.

Key observations:

• Security-First Limiting: The issuance rate limit is correctly applied after proof-of-possession (signature verification), preventing unauthenticated attackers from draining the quota of legitimate agents.
• Resource Protection: The fixed-size fingerprinting (SHA-256) of rate-limiter keys and the 2048-character cap on client_id length provide strong defense-in-depth against memory exhaustion from attacker-chosen strings.
• Continuous Refill Logic: The token-bucket implementation uses elapsed-time refills rather than interval timers, ensuring a smooth and accurate rate limit.
• Clear Documentation: The updates to configuration.md and mcp-oauth.md accurately reflect the new settings and their technical implications (e.g., per-worker-thread behavior).

All 144 unit tests passed, including the new edge-case tests for bucket capacity, LRU eviction, and Retry-After header generation.

[gemini-code-assist]

Code Review

This pull request implements a per-node in-memory token-bucket rate limiter to protect against fetch amplification and restrict token issuance rates. It integrates rate limiting into the client credentials grant flow and CIMD metadata fetches, accompanied by documentation updates and comprehensive unit tests. The review feedback highlights a potential bug where a configured capacity of less than 1 would permanently block all requests, and suggests ensuring the capacity is always at least 1.

[claude]

Reviewed; no blockers found.

[heskew]

Review: request changes

1. Unauthenticated requests can exhaust an agent's issuance quota — medium

src/lib/mcp/token.ts:517-529 debits a bucket keyed only by the submitted client_id, before resolution or assertion verification. CIMD client IDs are public URLs, so any caller can send 30 requests/minute with a known agent URL and a non-empty bogus assertion; the real agent is then rejected with 429 before its valid assertion is evaluated.

Apply the per-client issuance limit only after successful proof-of-possession verification. If pre-auth work also needs a guard, make that a separate limiter keyed by a transport identity (or an aggregate concurrency/cost control), not the claimed client_id. Add a regression test showing malformed assertions cannot drain another client's quota.

2. The LRU entry count does not bound limiter memory — medium

src/lib/mcp/rateLimit.ts:45-73 retains the raw key in its Map; src/lib/mcp/token.ts:492-524 supplies an unvalidated, unauthenticated client_id. maxKeys: 10_000 therefore caps only the number of entries, not their bytes: a flood of long, distinct IDs leaves 10,000 attacker-sized strings reachable. The CIMD limiter has the same raw-key behavior.

Store a fixed-size cryptographic fingerprint as the bucket key and impose a short client-ID length limit before bucket allocation. Add a long, unique-key flood test. This preserves the LRU design while making its memory claim true.

3. Positive fractional limits permanently deny issuance — low

resolveRateLimit() accepts every finite positive number, while getGrantLimiter() uses that number as both capacity and refill rate (src/lib/mcp/token.ts:86-103). rateLimit: 0.5, for example, yields a bucket that can never reach the one token tryTake() requires, and extremely small positive values can produce a non-finite Retry-After.

Require an integer rate of at least one, or use capacity: Math.max(1, rate) while keeping a fractional refill rate. Add fractional-rate tests.

---

AI-assisted review by Codex (GPT-5).

[heskew]

Codex review addressed on 523ce12 (+ ad30fb2):

1. Unauthenticated quota exhaustion (medium) — fixed. Real catch — our pre-PR pipeline framed the per-key limiter as benign because it reasoned about an attacker limiting itself, and missed that a public client_id debited pre-auth lets anyone 429 a specific agent. The issuance limiter now runs after verifyClientAssertion succeeds, keyed by the verified client_id, so only a caller that proved key possession can spend that identity's bucket. Regression test: five forged-assertion requests for the victim's client_id all 401 and leave its quota intact (two valid mints still succeed).

Pre-auth work is left bounded by the controls already in place rather than a new limiter: CIMD fetches by the per-URL fetch limiter (10/min) + MAX_CONCURRENT_RESOLUTIONS=16 / MAX_CONCURRENT_DNS=2 + the SSRF gate; signature verification is CPU-only (no I/O, no jti burn) and bounded by Node's request concurrency. I did not add the optional transport-identity (IP) pre-auth limiter you floated — generic unauthenticated request flooding is a transport/edge concern, IP-keying misbehaves behind proxies/NAT, and it'd be speculative for v1. Flagging it so you can push back if you'd rather have it.

2. Raw keys don't bound memory (medium) — fixed. createRateLimiter now stores a SHA-256 fingerprint as the map key, so maxKeys bounds bytes (× a constant), not just entry count — for both the grant and CIMD limiters. Combined with the 2048-char client_id cap (ad30fb2) bounding pre-hash input. Added a long-unique-key flood test (4 KB keys, maxKeys: 3) asserting the map stays bounded and LRU eviction still works.

3. Fractional limits permanently deny (low) — already fixed in ad30fb2 (you reviewed the pre-clamp head). createRateLimiter clamps capacity = Math.max(1, rate) while keeping the fractional refill — exactly the second option you gave. Retry-After stays finite because resolveRateLimit treats 0/non-finite as disable/default, so refillPerMinute is always finite-positive when the limiter is active. Fractional-rate test (0.5/min) added.

Suite: 1036 tests / 1034 pass / 2 pre-existing skips; tsc, lint, prettier clean by exit code.

🤖 Response by Claude (Fable 5) on Nathan's behalf