diff --git a/package-lock.json b/package-lock.json index 09e5180..6efd141 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8483,6 +8483,7 @@ "ajv": "8.20.0", "ajv-formats": "3.0.1", "express": "5.2.1", + "express-rate-limit": "^8.2.1", "jose": "^6.2.3" }, "devDependencies": { diff --git a/packages/shared/package.json b/packages/shared/package.json index 0775226..460ceb3 100644 --- a/packages/shared/package.json +++ b/packages/shared/package.json @@ -34,6 +34,7 @@ "ajv": "8.20.0", "ajv-formats": "3.0.1", "express": "5.2.1", + "express-rate-limit": "^8.2.1", "jose": "^6.2.3" } } diff --git a/packages/shared/src/dispatch.ts b/packages/shared/src/dispatch.ts index 583810f..f1a2da5 100644 --- a/packages/shared/src/dispatch.ts +++ b/packages/shared/src/dispatch.ts @@ -91,7 +91,15 @@ export interface DispatchDeps { // One Ajv instance for the process. Schemas are JSON Schema (draft-07 / the // OpenAPI 3.1 subset our tools use). `strict: false` because tool authors use // vocabulary (e.g. `description`) liberally; we only need structural validation. -const ajv = new Ajv({ allErrors: true, strict: false, coerceTypes: false }); +// +// `allErrors: false` (the default) is deliberate and security-relevant: the +// input is UNTRUSTED, and `allErrors: true` makes ajv enumerate every schema +// violation, which an attacker can weaponize into CPU/memory exhaustion by +// sending a large/deeply-nested payload that fails many constraints at once +// (CodeQL js/resource-exhaustion). Short-circuiting on the first error caps the +// work per request; the 400 still names the first failing path, which is enough +// for a caller to fix their input. +const ajv = new Ajv({ allErrors: false, strict: false, coerceTypes: false }); addFormats(ajv); const validatorCache = new WeakMap(); diff --git a/packages/shared/src/http.test.ts b/packages/shared/src/http.test.ts index 5c7dfc4..65c0e97 100644 --- a/packages/shared/src/http.test.ts +++ b/packages/shared/src/http.test.ts @@ -16,7 +16,7 @@ import { NoopAuditLogger } from './audit.js'; import { InMemoryRateLimiter } from './rate-limit.js'; import type { DispatchDeps } from './dispatch.js'; -function build() { +function build(opts: { edgeRateLimit?: { windowMs: number; limit: number } } = {}) { const registry = new ToolRegistry(); registry.register( defineTool<{ id: string }, { id: string; ok: boolean }>({ @@ -71,6 +71,7 @@ function build() { deps, mcpInfo: { name: 'sh-mcp-test', version: '0.0.1' }, openApi: { info: { title: 'Test', version: '0.0.1' }, servers: [{ url: 'http://x' }] }, + edgeRateLimit: opts.edgeRateLimit, }); } @@ -145,6 +146,25 @@ describe('createApp routes', () => { expect(res.status).toBe(429); }); + it('edge rate limiter — throttles by IP BEFORE auth (429 on an unauthenticated flood)', async () => { + // limit 2/window. Unauthenticated requests would normally 401, but the edge + // limiter sits in front of `authenticate`, so the 3rd request is throttled + // (429), not 401 — proving floods are capped before any JWT verification. + const app = build({ edgeRateLimit: { windowMs: 60_000, limit: 2 } }); + expect((await request(app).post('/tools/lookup_thing').send({ id: '1' })).status).toBe(401); + expect((await request(app).post('/tools/lookup_thing').send({ id: '2' })).status).toBe(401); + const res = await request(app).post('/tools/lookup_thing').send({ id: '3' }); + expect(res.status).toBe(429); + expect(res.body).toEqual({ error: 'rate_limited' }); + }); + + it('edge rate limiter — also fronts /mcp', async () => { + const app = build({ edgeRateLimit: { windowMs: 60_000, limit: 1 } }); + expect((await request(app).post('/mcp').send({})).status).toBe(401); + const res = await request(app).post('/mcp').send({}); + expect(res.status).toBe(429); + }); + it('POST /mcp — 401 without a token', async () => { const res = await request(build()) .post('/mcp') diff --git a/packages/shared/src/http.ts b/packages/shared/src/http.ts index 9a4d40b..883f677 100644 --- a/packages/shared/src/http.ts +++ b/packages/shared/src/http.ts @@ -18,6 +18,7 @@ */ import express, { type Express, type Request, type Response, type NextFunction } from 'express'; +import { rateLimit, type RateLimitRequestHandler } from 'express-rate-limit'; import { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp.js'; import { AuthError } from './cognito-auth.js'; @@ -43,6 +44,16 @@ export interface CreateAppOptions { /** MCP handshake identity + OpenAPI `info`/`servers`. */ mcpInfo: McpServerInfo; openApi: BuildOpenApiOptions; + /** + * Per-IP edge rate limit for the authenticated routes (`/mcp`, `/tools/:name`). + * This is a coarse abuse/DoS guard that runs BEFORE `authProvider.authenticate` + * — so it throttles unauthenticated and invalid-token floods before the (more + * expensive) JWT verification and dispatch. It complements, and does not + * replace, the per-`sub` + per-tool limiter inside `executeTool` (design.md + * §7.3). Defaults: 120 requests / 60s / IP. In production the API Gateway/WAF + * is the first edge; this is defense-in-depth. + */ + edgeRateLimit?: { windowMs: number; limit: number }; } /** Express `Request` augmented with the authenticated context. */ @@ -58,6 +69,21 @@ export function createApp(options: CreateAppOptions): Express { const app = express(); app.use(express.json()); + // --- Per-IP edge rate limiter for the authenticated routes --- + // Runs ahead of auth so request floods are throttled before JWT verification + // and dispatch. Keyed by client IP (express-rate-limit default). Returns the + // same 429 shape adapters use elsewhere; standard RateLimit headers, no legacy. + const { windowMs = 60_000, limit = 120 } = options.edgeRateLimit ?? {}; + const edgeRateLimit: RateLimitRequestHandler = rateLimit({ + windowMs, + limit, + standardHeaders: 'draft-7', + legacyHeaders: false, + handler: (_req: Request, res: Response) => { + res.status(429).json({ error: 'rate_limited' }); + }, + }); + // --- Unauthenticated liveness --- app.get('/healthz', (_req: Request, res: Response) => { res.status(200).json({ status: 'ok' }); @@ -103,12 +129,12 @@ export function createApp(options: CreateAppOptions): Express { await server.connect(transport); await transport.handleRequest(req, res, req.body); }; - app.post('/mcp', authenticate, (req, res) => void handleMcp(req, res)); - app.get('/mcp', authenticate, (req, res) => void handleMcp(req, res)); - app.delete('/mcp', authenticate, (req, res) => void handleMcp(req, res)); + app.post('/mcp', edgeRateLimit, authenticate, (req, res) => void handleMcp(req, res)); + app.get('/mcp', edgeRateLimit, authenticate, (req, res) => void handleMcp(req, res)); + app.delete('/mcp', edgeRateLimit, authenticate, (req, res) => void handleMcp(req, res)); // --- One-shot OpenAPI tool call --- - app.post('/tools/:name', authenticate, (req: AuthedRequest, res: Response) => { + app.post('/tools/:name', edgeRateLimit, authenticate, (req: AuthedRequest, res: Response) => { const ctx = req.authContext!; const rawName = req.params['name']; const name = Array.isArray(rawName) ? (rawName[0] ?? '') : (rawName ?? '');