fix mcp streamable-http method handling (#19496)
## Summary Fix MCP `/mcp` transport handling for clients using `streamable-http`. ## Changes - add explicit `GET /mcp` and `DELETE /mcp` handlers - return `405 Method Not Allowed` with `Allow: POST` - keep `POST /mcp` protected by MCP auth guards - mark `GET` and `DELETE` as intentionally public with `PublicEndpointGuard` + `NoPermissionGuard` - update the advertised MCP protocol version to `2025-03-26` - add unit and integration coverage for the new behavior ## Why The frontend advertises the MCP server as `streamable-http`, but the backend only effectively handled `POST /mcp`. Some MCP clients probe `GET /mcp` during connection setup, so unsupported methods need explicit method-level responses instead of falling through or being blocked before the handler. ## Validation - verified locally: - `GET /mcp` -> `405` - `DELETE /mcp` -> `405` - unauthenticated `POST /mcp` -> `401` - passed controller unit tests - added integration assertions for `GET` and `DELETE` --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Félix Malfait <felix.malfait@gmail.com>
This commit is contained in:
committed by
GitHub
parent
fb950cf312
commit
0fa7beba44
@@ -17,6 +17,7 @@ import { CoreGraphQLApiModule } from 'src/engine/api/graphql/core-graphql-api.mo
|
||||
import { GraphQLConfigModule } from 'src/engine/api/graphql/graphql-config/graphql-config.module';
|
||||
import { GraphQLConfigService } from 'src/engine/api/graphql/graphql-config/graphql-config.service';
|
||||
import { MetadataGraphQLApiModule } from 'src/engine/api/graphql/metadata-graphql-api.module';
|
||||
import { McpMethodGuardMiddleware } from 'src/engine/api/mcp/middlewares/mcp-method-guard.middleware';
|
||||
import { McpModule } from 'src/engine/api/mcp/mcp.module';
|
||||
import { RestApiModule } from 'src/engine/api/rest/rest-api.module';
|
||||
import { WorkspaceAuthContextMiddleware } from 'src/engine/core-modules/auth/middlewares/workspace-auth-context.middleware';
|
||||
@@ -125,6 +126,10 @@ export class AppModule {
|
||||
)
|
||||
.forRoutes({ path: 'metadata', method: RequestMethod.ALL });
|
||||
|
||||
consumer
|
||||
.apply(McpMethodGuardMiddleware)
|
||||
.forRoutes({ path: 'mcp', method: RequestMethod.ALL });
|
||||
|
||||
for (const method of MIGRATED_REST_METHODS) {
|
||||
consumer
|
||||
.apply(RestCoreMiddleware, WorkspaceAuthContextMiddleware)
|
||||
|
||||
+76
@@ -0,0 +1,76 @@
|
||||
import { type Request, type Response } from 'express';
|
||||
|
||||
import { JSON_RPC_ERROR_CODE } from 'src/engine/api/mcp/constants/json-rpc-error-code.const';
|
||||
import { McpMethodGuardMiddleware } from 'src/engine/api/mcp/middlewares/mcp-method-guard.middleware';
|
||||
|
||||
describe('McpMethodGuardMiddleware', () => {
|
||||
let middleware: McpMethodGuardMiddleware;
|
||||
let mockRes: Partial<Response>;
|
||||
let next: jest.Mock;
|
||||
|
||||
beforeEach(() => {
|
||||
middleware = new McpMethodGuardMiddleware();
|
||||
next = jest.fn();
|
||||
mockRes = {
|
||||
setHeader: jest.fn().mockReturnThis(),
|
||||
status: jest.fn().mockReturnThis(),
|
||||
json: jest.fn().mockReturnThis(),
|
||||
};
|
||||
});
|
||||
|
||||
it('should call next() for POST requests', () => {
|
||||
const req = { method: 'POST' } as Request;
|
||||
|
||||
middleware.use(req, mockRes as Response, next);
|
||||
|
||||
expect(next).toHaveBeenCalled();
|
||||
expect(mockRes.status).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should return 405 with Allow header for GET requests', () => {
|
||||
const req = { method: 'GET' } as Request;
|
||||
|
||||
middleware.use(req, mockRes as Response, next);
|
||||
|
||||
expect(next).not.toHaveBeenCalled();
|
||||
expect(mockRes.setHeader).toHaveBeenCalledWith('Allow', 'POST');
|
||||
expect(mockRes.status).toHaveBeenCalledWith(405);
|
||||
expect(mockRes.json).toHaveBeenCalledWith({
|
||||
jsonrpc: '2.0',
|
||||
error: {
|
||||
code: JSON_RPC_ERROR_CODE.INVALID_REQUEST,
|
||||
message:
|
||||
'HTTP method GET is not allowed. This MCP endpoint only accepts POST requests.',
|
||||
},
|
||||
id: null,
|
||||
});
|
||||
});
|
||||
|
||||
it('should return 405 with Allow header for DELETE requests', () => {
|
||||
const req = { method: 'DELETE' } as Request;
|
||||
|
||||
middleware.use(req, mockRes as Response, next);
|
||||
|
||||
expect(next).not.toHaveBeenCalled();
|
||||
expect(mockRes.setHeader).toHaveBeenCalledWith('Allow', 'POST');
|
||||
expect(mockRes.status).toHaveBeenCalledWith(405);
|
||||
expect(mockRes.json).toHaveBeenCalledWith({
|
||||
jsonrpc: '2.0',
|
||||
error: {
|
||||
code: JSON_RPC_ERROR_CODE.INVALID_REQUEST,
|
||||
message:
|
||||
'HTTP method DELETE is not allowed. This MCP endpoint only accepts POST requests.',
|
||||
},
|
||||
id: null,
|
||||
});
|
||||
});
|
||||
|
||||
it('should return 405 for PUT requests', () => {
|
||||
const req = { method: 'PUT' } as Request;
|
||||
|
||||
middleware.use(req, mockRes as Response, next);
|
||||
|
||||
expect(next).not.toHaveBeenCalled();
|
||||
expect(mockRes.status).toHaveBeenCalledWith(405);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,30 @@
|
||||
import { Injectable, type NestMiddleware } from '@nestjs/common';
|
||||
|
||||
import { type NextFunction, type Request, type Response } from 'express';
|
||||
|
||||
import { JSON_RPC_ERROR_CODE } from 'src/engine/api/mcp/constants/json-rpc-error-code.const';
|
||||
|
||||
// MCP streamable-http spec (2025-03-26) requires that servers respond with
|
||||
// 405 Method Not Allowed (plus an Allow header) for HTTP methods they do not
|
||||
// support. This middleware runs before guards and controllers so non-POST
|
||||
// requests are rejected without any authentication overhead.
|
||||
@Injectable()
|
||||
export class McpMethodGuardMiddleware implements NestMiddleware {
|
||||
use(req: Request, res: Response, next: NextFunction) {
|
||||
if (req.method === 'POST') {
|
||||
next();
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
res.setHeader('Allow', 'POST');
|
||||
res.status(405).json({
|
||||
jsonrpc: '2.0',
|
||||
error: {
|
||||
code: JSON_RPC_ERROR_CODE.INVALID_REQUEST,
|
||||
message: `HTTP method ${req.method} is not allowed. This MCP endpoint only accepts POST requests.`,
|
||||
},
|
||||
id: null,
|
||||
});
|
||||
}
|
||||
}
|
||||
@@ -19,6 +19,32 @@ describe('MCP Controller (integration)', () => {
|
||||
.send(JSON.stringify(body));
|
||||
};
|
||||
|
||||
it('should return 405 with JSON-RPC error for GET /mcp', async () => {
|
||||
await request(baseUrl)
|
||||
.get(endpoint)
|
||||
.expect(405)
|
||||
.expect((res) => {
|
||||
expect(res.headers.allow).toBe('POST');
|
||||
expect(res.body.jsonrpc).toBe('2.0');
|
||||
expect(res.body.error).toBeDefined();
|
||||
expect(res.body.error.code).toBe(-32600);
|
||||
expect(res.body.id).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
it('should return 405 with JSON-RPC error for DELETE /mcp', async () => {
|
||||
await request(baseUrl)
|
||||
.delete(endpoint)
|
||||
.expect(405)
|
||||
.expect((res) => {
|
||||
expect(res.headers.allow).toBe('POST');
|
||||
expect(res.body.jsonrpc).toBe('2.0');
|
||||
expect(res.body.error).toBeDefined();
|
||||
expect(res.body.error.code).toBe(-32600);
|
||||
expect(res.body.id).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
it('should respond to ping with a JSON-RPC result envelope', async () => {
|
||||
await postMcp({ jsonrpc: '2.0', method: 'ping', id: '1' })
|
||||
.expect(200)
|
||||
|
||||
Reference in New Issue
Block a user