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

docs/mcp-oauth.md

@@ -636,6 +636,7 @@ mcp:
   clientCredentials:
     enabled: true
     accessTokenTtl: 300 # default; agents re-mint on 401
+    rateLimit: 30 # default; issuance requests/min per client_id, false disables
 ```
 
 Agents don't register. Each agent's `client_id` is an HTTPS URL to a CIMD
@@ -707,6 +708,25 @@ is the document-declared `scope`; a `scope` parameter on the token request is
 not honored (a client can never escalate past its registered scope, and
 RFC 6749 §3.3 downscoping-on-request is future work).
 
+Issuance is **rate-limited per `client_id`** (`mcp.clientCredentials.rateLimit`,
+default 30 requests/min, `false` disables): over-limit requests receive `429`
+with `error: "slow_down"` and a `Retry-After` header (seconds until a retry can
+succeed). The limit is debited **after** the client assertion is verified, so it
+counts only authenticated issuance — a caller cannot drain a real agent's quota
+by replaying the agent's public `client_id` URL with a bogus assertion (those
+fail verification with `401` and never touch the bucket). Pre-auth work is
+bounded separately: CIMD metadata fetches are limited at a fixed 10 attempts/min
+per `client_id` URL (cache hits don't consume, so only failing documents
+repeat), and resolution/DNS concurrency is capped globally.
+Both limits are per-node token buckets (a replicated counter would be a
+hot-write anti-pattern; the assertion replay guard and ≤60s window bound
+cross-node abuse). The bucket state is per worker thread: if the plugin runs
+across N HTTP worker threads on a node, the effective ceiling is N × the
+configured limit per `client_id` (as with the CIMD fetch cache and concurrency
+caps, which are likewise per-thread). This is intentional for a defense-in-depth
+control — treat the configured value as a per-thread floor, not a hard node-wide
+cap.
+
 Key rotation / revocation semantics: the fleet rotates a key by updating the
 agent's metadata document. The change takes effect within the CIMD cache TTL
 (up to 24 h, typically 1 h — bound it with `Cache-Control: max-age` on the

src/lib/mcp/authorize.ts

@@ -48,8 +48,9 @@ import {
 import { resolveIssuer, resolveResource } from './wellKnown.ts';
 
 type ErrorJSON = {
-	status: 400 | 500;
+	status: 400 | 429 | 500;
 	body: { error: string; error_description?: string };
+	headers?: Record<string, string>;
 };
 
 type Redirect = {
@@ -384,10 +385,12 @@ export async function handleAuthorize(
 		client = await resolveClient(query.client_id, mcpConfig, logger);
 	} catch (error) {
 		if (error instanceof CimdClientError) {
-			return {
-				status: 400,
+			const json: ErrorJSON = {
+				status: error.statusCode === 429 ? 429 : 400,
 				body: { error: error.oauthError, error_description: error.message },
 			};
+			if (error.retryAfterSeconds !== undefined) json.headers = { 'Retry-After': String(error.retryAfterSeconds) };
+			return json;
 		}
 		logger?.error?.('MCP authorize: client lookup failed:', error instanceof Error ? error.message : String(error));
 		return {

src/lib/mcp/cimd.ts

@@ -54,8 +54,10 @@
  *   protection (an attacker's document must not be able to force a fetch
  *   per authorize request).
  * - Failures are NOT cached (the CIMD draft forbids caching error responses
- *   and invalid documents); repeated fetches of bad documents are left to
- *   rate limiting (#163).
+ *   and invalid documents); repeated fetch attempts are instead rate-limited
+ *   per client_id URL (fixed 10/min token bucket, #163) — legit clients hit
+ *   the cache after their first success, so only failures repeat. Issuance
+ *   itself is separately rate-limited at the client_credentials grant.
  * - Cached records are revalidated against the live redirect-host policy on
  *   every hit, so tightening `allowedRedirectUriHosts` takes effect
  *   immediately instead of after cache expiry (up to 24 h).
@@ -76,6 +78,7 @@ import { request as httpsRequest } from 'node:https';
 import { Readable } from 'node:stream';
 import type { Logger, MCPClientIdMetadataDocumentsConfig, MCPClientRecord, MCPConfig } from '../../types.ts';
 import { MCPClientStore } from './clientStore.ts';
+import { createRateLimiter } from './rateLimit.ts';
 import {
 	validateGrantTypes,
 	validateRedirectUri,
@@ -117,19 +120,46 @@ const cimdCache = new Map<string, CacheEntry>();
 // requests for one uncached URL trigger a single fetch (thundering-herd guard).
 const inFlightResolutions = new Map<string, Promise<MCPClientRecord | null>>();
 
+// Per-URL fetch-attempt rate limit (#163) — fixed policy, no config knob:
+// legit clients hit the cache after their first success (only failures
+// repeat, and failures are never cached per the CIMD draft). Per-node
+// semantics: see rateLimit.ts module header.
+const CIMD_FETCH_ATTEMPTS_PER_MINUTE = 10;
+const cimdFetchLimiter = createRateLimiter({
+	capacity: CIMD_FETCH_ATTEMPTS_PER_MINUTE,
+	refillPerMinute: CIMD_FETCH_ATTEMPTS_PER_MINUTE,
+});
+
+/**
+ * Upper bound on a client_id length, shared by every entry point that uses a
+ * caller-supplied client_id as a lookup or rate-limiter map key. Same
+ * defense-in-depth family as the repo's 2048-char request-path cap.
+ */
+export const MAX_CLIENT_ID_LENGTH = 2048;
+
 /**
  * Thrown when CIMD resolution fails due to a client-side issue (bad URL,
  * invalid document, unsupported auth method, allowedHosts policy).
  * Callers should surface this as the given `oauthError` with the `message`
- * as the `error_description`.
+ * as the `error_description`. Throttle rejections additionally carry a
+ * `statusCode` (429) and `retryAfterSeconds` so callers can emit a proper
+ * `Retry-After` instead of a misleading auth error.
  */
 export class CimdClientError extends Error {
 	readonly oauthError: string;
-
-	constructor(oauthError: string, message: string, options?: { cause?: unknown }) {
+	readonly statusCode?: number;
+	readonly retryAfterSeconds?: number;
+
+	constructor(
+		oauthError: string,
+		message: string,
+		options?: { cause?: unknown; statusCode?: number; retryAfterSeconds?: number }
+	) {
 		super(message, options);
 		this.name = 'CimdClientError';
 		this.oauthError = oauthError;
+		this.statusCode = options?.statusCode;
+		this.retryAfterSeconds = options?.retryAfterSeconds;
 	}
 }
 
@@ -210,9 +240,12 @@ export function _setFetch(fn: CimdFetch | null): void {
 	_fetch = fn ?? pinnedHttpsFetch;
 }
 
-/** Clear the CIMD cache (for testing). @internal */
+/** Clear the CIMD cache AND the per-URL fetch limiter (for testing) — tests
+ * that loop many resolution attempts against one URL rely on the limiter
+ * resetting alongside the cache. @internal */
 export function _clearCimdCache(): void {
 	cimdCache.clear();
+	cimdFetchLimiter._reset();
 }
 
 // --- SSRF guard helpers ---
@@ -846,8 +879,25 @@ export async function resolveCimdClient(
 	if (existing) return existing;
 	// Total-concurrency bound: the in-flight map IS the counter (no await sits
 	// between this check and the set below, so the cap cannot be raced past).
+	// Checked BEFORE the rate-limit take so a capacity reject doesn't spend a
+	// token without a fetch (keeps "only actual fetch attempts consume" true).
 	if (inFlightResolutions.size >= MAX_CONCURRENT_RESOLUTIONS) {
-		throw new CimdClientError('temporarily_unavailable', 'CIMD resolution capacity reached; retry shortly');
+		throw new CimdClientError('temporarily_unavailable', 'CIMD resolution capacity reached; retry shortly', {
+			statusCode: 429,
+		});
+	}
+	// Per-URL fetch rate limit (#163): only actual fetch attempts consume —
+	// cache hits returned above, dedup'd joiners, and capacity rejects never
+	// reach this. Fixed policy (no knob): legit clients are served from the
+	// cache after their first success; repeated attempts are the bad-document
+	// amplification vector this closes.
+	const rateLimit = cimdFetchLimiter.tryTake(clientId);
+	if (!rateLimit.allowed) {
+		logger?.warn?.(`CIMD: fetch rate limit reached for ${clientId}`);
+		throw new CimdClientError('slow_down', 'CIMD resolution rate limit reached; retry shortly', {
+			statusCode: 429,
+			retryAfterSeconds: rateLimit.retryAfterSeconds,
+		});
 	}
 	const pending = fetchAndValidateCimd(clientId, cimdConfig, allowedRedirectUriHosts, logger).finally(() =>
 		inFlightResolutions.delete(clientId)
@@ -977,8 +1027,9 @@ async function fetchAndValidateCimd(
 		return record;
 	} catch (err) {
 		// Failures are never cached — the CIMD draft forbids caching error
-		// responses and invalid documents. Fetch amplification from repeated
-		// bad requests is rate limiting's job (#163).
+		// responses and invalid documents. Amplification from repeated bad
+		// requests is bounded by the per-URL fetch rate limit in
+		// `resolveCimdClient` (#163) instead.
 		if (err instanceof CimdClientError) {
 			logger?.warn?.(`CIMD: rejected client ${clientId}: ${err.message}`);
 			throw err;
@@ -1002,6 +1053,9 @@ export async function resolveClient(
 	mcpConfig: MCPConfig | undefined,
 	logger?: Logger
 ): Promise<MCPClientRecord | null> {
+	// An over-length client_id is never a real client — reject before it becomes
+	// a CIMD fetch-limiter key or a store lookup (unknown-client null, no leak).
+	if (clientId.length > MAX_CLIENT_ID_LENGTH) return null;
 	const cimdConfig = mcpConfig?.clientIdMetadataDocuments;
 	// CIMD is enabled by default when mcp.enabled; disabled only by explicit `enabled: false`.
 	const cimdEnabled = cimdConfig?.enabled !== false;

src/lib/mcp/rateLimit.ts

@@ -0,0 +1,103 @@
+/**
+ * Per-node in-memory token-bucket rate limiter (#163).
+ *
+ * Deliberately PER-NODE, not replicated: Harper replication makes a shared
+ * counter table a hot-write anti-pattern, and the surfaces this limits are
+ * already bounded cross-node — the client_credentials grant by the assertion
+ * replay guard and its ≤60s `exp` window, CIMD fetches by the concurrency
+ * caps. A node-local bucket is the right defense-in-depth without turning
+ * every token request into a replicated write.
+ *
+ * Buckets refill continuously (elapsed-time based — no interval timers), so a
+ * limit of N/min admits a burst of up to N and then one request per 60/N
+ * seconds. Keys may be ATTACKER-CHOSEN strings (client_ids, URLs), so: (1) the
+ * key space is LRU-bounded — at `maxKeys` the least-recently-used bucket is
+ * evicted (an evicted bucket forgets that key's spend history, acceptable
+ * because eviction requires `maxKeys` distinct keys inside one window and the
+ * global concurrency caps still bound aggregate work); and (2) the key is
+ * stored as a fixed-size SHA-256 fingerprint, so a flood of long distinct keys
+ * bounds map memory to `maxKeys` × a constant, not × the raw key length.
+ */
+
+import { createHash } from 'node:crypto';
+
+type BucketState = { tokens: number; lastRefill: number };
+
+export type RateLimitResult = { allowed: true } | { allowed: false; retryAfterSeconds: number };
+
+export type RateLimiter = {
+	tryTake(key: string): RateLimitResult;
+	/** Drop all bucket state (for testing). @internal */
+	_reset(): void;
+};
+
+const DEFAULT_MAX_KEYS = 10_000;
+
+// Cap the advertised Retry-After. A tiny configured rate (e.g. 0.001/min)
+// would otherwise yield an absurd multi-year value; 2,147,483 s (≈24.8 days,
+// int32-max milliseconds expressed as whole seconds) is a sane ceiling for any
+// client parsing the header.
+const MAX_RETRY_AFTER_SECONDS = 2_147_483;
+
+export function createRateLimiter(options: {
+	/** Bucket capacity (maximum burst). */
+	capacity: number;
+	/** Continuous refill rate, tokens per minute. */
+	refillPerMinute: number;
+	/** LRU bound on distinct keys tracked. Default 10 000. */
+	maxKeys?: number;
+	/** Clock override (for testing). Default Date.now. */
+	now?: () => number;
+}): RateLimiter {
+	// A token bucket takes 1 token per request, so a burst ceiling below 1 could
+	// never admit anyone — clamp it up. The refill rate is left untouched (a
+	// sub-1/min limit still means "one request, then one per 60/rate seconds"),
+	// except that a non-finite or non-positive rate — which would make the
+	// bucket never refill and the retry-after math divide by zero/negative —
+	// falls back to the (already ≥1) capacity so the limiter stays well-defined.
+	const capacity = Math.max(1, options.capacity);
+	const refillPerMinute =
+		Number.isFinite(options.refillPerMinute) && options.refillPerMinute > 0 ? options.refillPerMinute : capacity;
+	const maxKeys = options.maxKeys ?? DEFAULT_MAX_KEYS;
+	const now = options.now ?? Date.now;
+	const buckets = new Map<string, BucketState>();
+
+	return {
+		tryTake(key: string): RateLimitResult {
+			// Fixed-size fingerprint: the raw key may be an attacker-chosen URL of
+			// arbitrary length, and it is retained in the map until evicted.
+			const mapKey = createHash('sha256').update(key).digest('base64url');
+			const at = now();
+			let bucket = buckets.get(mapKey);
+			if (bucket) {
+				const elapsedMs = at - bucket.lastRefill;
+				if (elapsedMs > 0) {
+					bucket.tokens = Math.min(capacity, bucket.tokens + (elapsedMs / 60_000) * refillPerMinute);
+					bucket.lastRefill = at;
+				}
+				// Delete now; the set below re-inserts so Map order tracks recency (LRU).
+				buckets.delete(mapKey);
+			} else {
+				bucket = { tokens: capacity, lastRefill: at };
+				if (buckets.size >= maxKeys) {
+					const oldest = buckets.keys().next().value;
+					if (oldest !== undefined) buckets.delete(oldest);
+				}
+			}
+			let result: RateLimitResult;
+			if (bucket.tokens >= 1) {
+				bucket.tokens -= 1;
+				result = { allowed: true };
+			} else {
+				const deficit = 1 - bucket.tokens;
+				const retryAfterSeconds = Math.min(MAX_RETRY_AFTER_SECONDS, Math.ceil((deficit * 60) / refillPerMinute));
+				result = { allowed: false, retryAfterSeconds };
+			}
+			buckets.set(mapKey, bucket);
+			return result;
+		},
+		_reset(): void {
+			buckets.clear();
+		},
+	};
+}