feat(mcp): ready for a remote server: quiet logs and allowed domains
- Logs default to warnings and errors (MCP_LOG_LEVEL): the library logged every tool on every request (~48 lines per call). - DNS rebinding protection on /mcp, as the MCP spec asks of HTTP servers: the Host header must be in MCP_ALLOWED_HOSTS (default: localhost; compose default mcp.taskstream.pireon.dev,localhost) and a browser Origin, when present, too; otherwise 403 with a JSON-RPC error. Uses the official validators from @modelcontextprotocol/server. - Test for allowed/foreign Host and Origin; .env.example, compose and docs/09-mcp.md document the variables; Word/PDF regenerated. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,8 +1,9 @@
|
||||
import { Module } from '@nestjs/common';
|
||||
import { MiddlewareConsumer, Module, NestModule } from '@nestjs/common';
|
||||
import { ConfigModule, ConfigService } from '@nestjs/config';
|
||||
import { McpModule } from '@nestjs-mcp/server';
|
||||
import { TASKSTREAM_API_URL, TaskStreamApi } from './api/taskstream-api';
|
||||
import { HealthController } from './health.controller';
|
||||
import { McpHostsMiddleware } from './mcp-hosts.middleware';
|
||||
import { IssuesResolver } from './tools/issues.resolver';
|
||||
import { ProjectsResolver } from './tools/projects.resolver';
|
||||
import { ReleasesResolver } from './tools/releases.resolver';
|
||||
@@ -23,7 +24,9 @@ token the client sends; without one, only public projects can be read.`;
|
||||
name: 'taskstream',
|
||||
version: '1.0.0',
|
||||
instructions: INSTRUCTIONS,
|
||||
logging: { level: config.get('MCP_LOG_LEVEL', 'log') },
|
||||
// The library logs every tool on every request (it builds a server per call):
|
||||
// only warnings and errors unless asked for more.
|
||||
logging: { level: config.get('MCP_LOG_LEVEL', 'warn') },
|
||||
}),
|
||||
}),
|
||||
],
|
||||
@@ -45,4 +48,8 @@ token the client sends; without one, only public projects can be read.`;
|
||||
UsersResolver,
|
||||
],
|
||||
})
|
||||
export class AppModule {}
|
||||
export class AppModule implements NestModule {
|
||||
configure(consumer: MiddlewareConsumer): void {
|
||||
consumer.apply(McpHostsMiddleware).forRoutes('mcp');
|
||||
}
|
||||
}
|
||||
|
||||
46
apps/mcp/src/app/mcp-hosts.middleware.ts
Normal file
46
apps/mcp/src/app/mcp-hosts.middleware.ts
Normal file
@@ -0,0 +1,46 @@
|
||||
import { Injectable, NestMiddleware } from '@nestjs/common';
|
||||
import { ConfigService } from '@nestjs/config';
|
||||
import {
|
||||
localhostAllowedHostnames,
|
||||
validateHostHeader,
|
||||
validateOriginHeader,
|
||||
} from '@modelcontextprotocol/server';
|
||||
import type { NextFunction, Request, Response } from 'express';
|
||||
|
||||
/**
|
||||
* Protection against DNS rebinding on /mcp (what the MCP spec asks of HTTP
|
||||
* servers): the Host header must be one of MCP_ALLOWED_HOSTS (default: only
|
||||
* localhost), and a browser's Origin, when sent, too. AI clients calling from
|
||||
* a server send no Origin, so they only need the right domain.
|
||||
*/
|
||||
@Injectable()
|
||||
export class McpHostsMiddleware implements NestMiddleware {
|
||||
private readonly hosts: string[];
|
||||
|
||||
constructor(config: ConfigService) {
|
||||
const configured = (config.get<string>('MCP_ALLOWED_HOSTS') ?? '')
|
||||
.split(',')
|
||||
.map((host) => host.trim().toLowerCase())
|
||||
.filter(Boolean);
|
||||
this.hosts = configured.length ? configured : localhostAllowedHostnames();
|
||||
}
|
||||
|
||||
use(req: Request, res: Response, next: NextFunction): void {
|
||||
const error = this.rejection(req);
|
||||
if (!error) return next();
|
||||
res.status(403).json({
|
||||
jsonrpc: '2.0',
|
||||
id: null,
|
||||
error: { code: -32000, message: error },
|
||||
});
|
||||
}
|
||||
|
||||
/** Why the request is refused, or null when its Host (and Origin, if any) are allowed. */
|
||||
private rejection(req: Request): string | null {
|
||||
const host = validateHostHeader(req.headers.host, this.hosts);
|
||||
if ('message' in host) return host.message;
|
||||
if (!req.headers.origin) return null;
|
||||
const origin = validateOriginHeader(req.headers.origin, this.hosts);
|
||||
return 'message' in origin ? origin.message : null;
|
||||
}
|
||||
}
|
||||
@@ -1,6 +1,11 @@
|
||||
import { INestApplication } from '@nestjs/common';
|
||||
import { Test } from '@nestjs/testing';
|
||||
import { createServer, IncomingMessage, Server } from 'node:http';
|
||||
import {
|
||||
createServer,
|
||||
IncomingMessage,
|
||||
request as httpRequest,
|
||||
Server,
|
||||
} from 'node:http';
|
||||
import { AddressInfo } from 'node:net';
|
||||
import { TASKSTREAM_API_URL } from './api/taskstream-api';
|
||||
import { AppModule } from './app.module';
|
||||
@@ -211,4 +216,50 @@ describe('TaskStream MCP', () => {
|
||||
expect(result.isError).toBe(true);
|
||||
expect(received).toHaveLength(0);
|
||||
});
|
||||
|
||||
/** A raw POST /mcp with chosen Host / Origin headers (fetch would not let us set Host). */
|
||||
function rawPost(headers: Record<string, string>): Promise<number> {
|
||||
const { port } = new URL(url);
|
||||
return new Promise((resolve, reject) => {
|
||||
const req = httpRequest(
|
||||
{
|
||||
host: '127.0.0.1',
|
||||
port,
|
||||
path: '/mcp',
|
||||
method: 'POST',
|
||||
headers: {
|
||||
'content-type': 'application/json',
|
||||
accept: 'application/json, text/event-stream',
|
||||
'mcp-protocol-version': '2025-06-18',
|
||||
...headers,
|
||||
},
|
||||
},
|
||||
(res) => {
|
||||
res.resume();
|
||||
resolve(res.statusCode ?? 0);
|
||||
},
|
||||
);
|
||||
req.on('error', reject);
|
||||
req.end(JSON.stringify({ jsonrpc: '2.0', id: 1, method: 'tools/list' }));
|
||||
});
|
||||
}
|
||||
|
||||
it('only answers on its own domain (DNS rebinding protection)', async () => {
|
||||
// Default allowlist: localhost only (MCP_ALLOWED_HOSTS sets the real domain).
|
||||
expect(await rawPost({ host: `localhost:${new URL(url).port}` })).toBe(200);
|
||||
expect(await rawPost({ host: 'evil.example' })).toBe(403);
|
||||
expect(
|
||||
await rawPost({
|
||||
host: `localhost:${new URL(url).port}`,
|
||||
origin: 'https://evil.example',
|
||||
}),
|
||||
).toBe(403);
|
||||
expect(
|
||||
await rawPost({
|
||||
host: `localhost:${new URL(url).port}`,
|
||||
origin: 'http://localhost:6274',
|
||||
}),
|
||||
).toBe(200);
|
||||
expect((await fetch(`${url}/health`)).status).toBe(200);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user