fix(server): read HTTP request bodies with a size limit and bound JSON-RPC batch length (#2698)
Co-authored-by: Felix Weinberger <fweinberger@anthropic.com>
This commit is contained in:
parent
3924de99df
commit
7b781ed4e2
2 changed files with 651 additions and 53 deletions
|
|
@ -76,7 +76,6 @@ export function createMcpExpressApp(options: CreateMcpExpressAppOptions = {}): E
|
|||
const { host = '127.0.0.1', allowedHosts, allowedOrigins, jsonLimit } = options;
|
||||
|
||||
const app = express();
|
||||
app.use(express.json(jsonLimit ? { limit: jsonLimit } : undefined));
|
||||
|
||||
// If allowedHosts is explicitly provided, use that for validation
|
||||
if (allowedHosts) {
|
||||
|
|
@ -106,5 +105,9 @@ export function createMcpExpressApp(options: CreateMcpExpressAppOptions = {}): E
|
|||
app.use(localhostOriginValidation());
|
||||
}
|
||||
|
||||
// The JSON body parser runs after the Host/Origin validation, so a request
|
||||
// from a disallowed origin is answered 403 without its body being read.
|
||||
app.use(express.json(jsonLimit ? { limit: jsonLimit } : undefined));
|
||||
|
||||
return app;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import type { NextFunction, Request, Response } from 'express';
|
||||
import supertest from 'supertest';
|
||||
import { vi } from 'vitest';
|
||||
|
||||
import { createMcpExpressApp } from '../src/express';
|
||||
|
|
@ -107,6 +108,43 @@ describe('@modelcontextprotocol/express', () => {
|
|||
});
|
||||
|
||||
describe('createMcpExpressApp', () => {
|
||||
test('validates Host/Origin before the JSON body parser runs', async () => {
|
||||
const app = createMcpExpressApp();
|
||||
app.post('/mcp', (req: Request, res: Response) => {
|
||||
res.json({ parsed: req.body });
|
||||
});
|
||||
|
||||
const disallowedHost = await supertest(app)
|
||||
.post('/mcp')
|
||||
.set('Host', 'evil.example.com')
|
||||
.set('Content-Type', 'application/json')
|
||||
.send('{not json');
|
||||
expect(disallowedHost.status).toBe(403);
|
||||
|
||||
const disallowedOrigin = await supertest(app)
|
||||
.post('/mcp')
|
||||
.set('Host', 'localhost:3000')
|
||||
.set('Origin', 'http://evil.example.com')
|
||||
.set('Content-Type', 'application/json')
|
||||
.send('{not json');
|
||||
expect(disallowedOrigin.status).toBe(403);
|
||||
|
||||
const invalidJson = await supertest(app)
|
||||
.post('/mcp')
|
||||
.set('Host', 'localhost:3000')
|
||||
.set('Content-Type', 'application/json')
|
||||
.send('{not json');
|
||||
expect(invalidJson.status).toBe(400);
|
||||
|
||||
const allowed = await supertest(app)
|
||||
.post('/mcp')
|
||||
.set('Host', 'localhost:3000')
|
||||
.set('Content-Type', 'application/json')
|
||||
.send({ ok: true });
|
||||
expect(allowed.status).toBe(200);
|
||||
expect(allowed.body).toEqual({ parsed: { ok: true } });
|
||||
});
|
||||
|
||||
test('should enable localhost DNS rebinding protection by default', () => {
|
||||
const app = createMcpExpressApp();
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue