Skip to content

Commit 8f9d2b3

Browse files
gxklclaude
andcommitted
fix(service-worker): rework MCP request pipeline + add DNS-rebinding guard
Auth is now the outermost gate: the request is rejected before any business middleware or transport dispatch runs. The JSON-RPC body is read once (via a clone) and handed to the SDK as `parsedBody`, so auth or a middleware reading the one-shot body no longer breaks the SDK's own read. Middlewares are selected per target — a `tools/call` runs only the owning controller's middlewares plus that tool's method middlewares, so one controller's middleware no longer runs for another's tool, nor for `initialize` / `tools/list`; method-level `@Middleware` (previously dropped) now runs. Adds `ServiceWorkerAppOptions.mcp` (allowedHosts / allowedOrigins) forwarded to the SDK transport's DNS-rebinding protection, off by default for compatibility. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 60ed905 commit 8f9d2b3

6 files changed

Lines changed: 259 additions & 31 deletions

File tree

tegg/standalone/service-worker-controller/src/mcp/ServiceWorkerMcpRouter.ts

Lines changed: 132 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,46 @@
1-
import { CONTROLLER_META_DATA, type MCPControllerMeta } from '@eggjs/controller-decorator';
1+
import { CONTROLLER_META_DATA, type MCPControllerMeta, type MCPToolMeta } from '@eggjs/controller-decorator';
22
import {
33
MCPServerHelper,
44
MCP_ROUTER_NAME,
55
type McpRouter,
66
type McpServerRegistration,
7+
type ServerRegisterRecord,
78
} from '@eggjs/controller-runtime';
89
import { Inject, InnerObjectProto } from '@eggjs/tegg';
910
import { EggContainerFactory } from '@eggjs/tegg-runtime';
1011
import { CONTROLLER_AOP_MIDDLEWARES } from '@eggjs/tegg-types';
1112
import type { EggProtoImplClass, EggPrototype } from '@eggjs/tegg-types';
12-
import { WebStandardStreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/webStandardStreamableHttp.js';
13+
import {
14+
type HandleRequestOptions,
15+
WebStandardStreamableHTTPServerTransport,
16+
} from '@modelcontextprotocol/sdk/server/webStandardStreamableHttp.js';
1317

1418
import type { FetchRouter } from '../http/FetchRouter.ts';
1519
import type { ServiceWorkerFetchContext } from '../http/ServiceWorkerFetchContext.ts';
16-
import type { MCPAuthHandler } from '../types.ts';
20+
import type { MCPAuthHandler, MCPTransportOptions } from '../types.ts';
1721
import type { AbstractControllerAdvice } from './AbstractControllerAdvice.ts';
1822

1923
type MCPMiddleware = (ctx: ServiceWorkerFetchContext, next: () => Promise<void>) => Promise<void>;
2024

25+
/** Minimal koa-style onion compose so selected middlewares wrap the dispatch. */
26+
function composeMiddlewares(middlewares: MCPMiddleware[]): (ctx: ServiceWorkerFetchContext) => Promise<void> {
27+
return (ctx) => {
28+
let index = -1;
29+
const dispatch = (i: number): Promise<void> => {
30+
if (i <= index) {
31+
return Promise.reject(new Error('next() called multiple times'));
32+
}
33+
index = i;
34+
const fn = middlewares[i];
35+
if (!fn) {
36+
return Promise.resolve();
37+
}
38+
return Promise.resolve(fn(ctx, () => dispatch(i + 1)));
39+
};
40+
return dispatch(0);
41+
};
42+
}
43+
2144
/**
2245
* The fetch host's MCP transport boundary: stateless streamable HTTP under
2346
* `/mcp[/name]/stream` (POST only). The service worker speaks Fetch natively,
@@ -39,6 +62,9 @@ export class ServiceWorkerMcpRouter implements McpRouter {
3962
@Inject()
4063
private readonly mcpAuthHandler: MCPAuthHandler;
4164

65+
@Inject()
66+
private readonly mcpTransportOptions: MCPTransportOptions;
67+
4268
#registrations: McpServerRegistration[] = [];
4369
#mounted = false;
4470

@@ -79,38 +105,107 @@ export class ServiceWorkerMcpRouter implements McpRouter {
79105
}
80106
const transport = new WebStandardStreamableHTTPServerTransport({
81107
sessionIdGenerator: undefined,
108+
...this.#dnsRebindingOptions(),
82109
});
83110
await mcpServerHelper.server.connect(transport);
84111
return transport;
85112
}
86113

87114
/**
88-
* Resolve the per-server middlewares from every controller proto that
89-
* contributed to this server: function-type middlewares from the controller
90-
* metadata plus AOP advice classes declared on the proto.
115+
* Forward Host/Origin allow-lists to the SDK transport. Protection turns on
116+
* once either list is configured (default off for backward compatibility) —
117+
* the host should set it before exposing the endpoint beyond loopback.
91118
*/
92-
private buildMiddlewares(reg: McpServerRegistration): MCPMiddleware[] {
119+
#dnsRebindingOptions(): {
120+
allowedHosts?: string[];
121+
allowedOrigins?: string[];
122+
enableDnsRebindingProtection?: boolean;
123+
} {
124+
const { allowedHosts, allowedOrigins, enableDnsRebindingProtection } = this.mcpTransportOptions ?? {};
125+
if (!allowedHosts?.length && !allowedOrigins?.length) {
126+
return {};
127+
}
128+
return {
129+
allowedHosts,
130+
allowedOrigins,
131+
enableDnsRebindingProtection: enableDnsRebindingProtection ?? true,
132+
};
133+
}
134+
135+
/** Read the JSON-RPC body once via a clone; the original stream stays intact
136+
* for the SDK / a middleware, and the parsed value is handed to the SDK so it
137+
* never re-reads the one-shot body after auth or a middleware touched it. */
138+
async #readJsonRpcBody(request: Request): Promise<unknown> {
139+
try {
140+
return await request.clone().json();
141+
} catch {
142+
// Empty / non-JSON: let the SDK read the original and raise its own error.
143+
return undefined;
144+
}
145+
}
146+
147+
/** Find the tool/prompt/resource a JSON-RPC message targets, if any. */
148+
#findTargetRecord(
149+
reg: McpServerRegistration,
150+
message: unknown,
151+
): ServerRegisterRecord<{ name?: string; mcpName?: string; uri?: string }> | undefined {
152+
const method = (message as { method?: string })?.method;
153+
const params = ((message as { params?: Record<string, unknown> })?.params ?? {}) as {
154+
name?: string;
155+
uri?: string;
156+
};
157+
switch (method) {
158+
case 'tools/call':
159+
return reg.tools.find((t) => (t.meta.mcpName ?? t.meta.name) === params.name);
160+
case 'prompts/get':
161+
return reg.prompts.find((p) => (p.meta.mcpName ?? p.meta.name) === params.name);
162+
case 'resources/read':
163+
return reg.resources.find((r) => r.meta.uri !== undefined && r.meta.uri === params.uri);
164+
default:
165+
// initialize / tools|prompts|resources/list / ping etc. target no record.
166+
return undefined;
167+
}
168+
}
169+
170+
/** Controller-level (function + AOP advice) middlewares declared on a proto. */
171+
#controllerMiddlewares(proto: EggPrototype): MCPMiddleware[] {
172+
const out: MCPMiddleware[] = [];
173+
const metadata = proto.getMetaData(CONTROLLER_META_DATA) as MCPControllerMeta;
174+
for (const mw of metadata.middlewares ?? []) {
175+
out.push(mw as unknown as MCPMiddleware);
176+
}
177+
const aopMiddlewareClasses = (proto.getMetaData(CONTROLLER_AOP_MIDDLEWARES) ??
178+
[]) as EggProtoImplClass<AbstractControllerAdvice>[];
179+
for (const clazz of aopMiddlewareClasses) {
180+
out.push(async (ctx, next) => {
181+
const eggObj = await EggContainerFactory.getOrCreateEggObjectFromClazz(clazz);
182+
await (eggObj.obj as AbstractControllerAdvice).middleware(ctx, next);
183+
});
184+
}
185+
return out;
186+
}
187+
188+
/**
189+
* Select only the middlewares of the targeted tool/prompt/resource:
190+
* controller-level (once per owning proto) plus that method's own
191+
* middlewares. So one controller's middleware never runs for another
192+
* controller's tool, nor for `initialize` / `tools/list`.
193+
*/
194+
#selectMiddlewares(reg: McpServerRegistration, parsedBody: unknown): MCPMiddleware[] {
195+
const messages = Array.isArray(parsedBody) ? parsedBody : [parsedBody];
93196
const middlewares: MCPMiddleware[] = [];
94-
const seen = new Set<EggPrototype>();
95-
for (const record of [...reg.tools, ...reg.resources, ...reg.prompts]) {
96-
if (seen.has(record.proto)) {
197+
const seenProtos = new Set<EggPrototype>();
198+
for (const message of messages) {
199+
const record = this.#findTargetRecord(reg, message);
200+
if (!record) {
97201
continue;
98202
}
99-
seen.add(record.proto);
100-
const metadata = record.proto.getMetaData(CONTROLLER_META_DATA) as MCPControllerMeta;
101-
// Function-type middlewares from MCPControllerMeta
102-
const classMiddlewares = metadata.middlewares ?? [];
103-
for (const mw of classMiddlewares) {
104-
middlewares.push(mw as unknown as MCPMiddleware);
203+
if (!seenProtos.has(record.proto)) {
204+
seenProtos.add(record.proto);
205+
middlewares.push(...this.#controllerMiddlewares(record.proto));
105206
}
106-
// AOP-type middlewares from class metadata
107-
const aopMiddlewareClasses = (record.proto.getMetaData(CONTROLLER_AOP_MIDDLEWARES) ??
108-
[]) as EggProtoImplClass<AbstractControllerAdvice>[];
109-
for (const clazz of aopMiddlewareClasses) {
110-
middlewares.push(async (ctx, next) => {
111-
const eggObj = await EggContainerFactory.getOrCreateEggObjectFromClazz(clazz);
112-
await (eggObj.obj as AbstractControllerAdvice).middleware(ctx, next);
113-
});
207+
for (const mw of (record.meta as MCPToolMeta).middlewares ?? []) {
208+
middlewares.push(mw as unknown as MCPMiddleware);
114209
}
115210
}
116211
return middlewares;
@@ -123,24 +218,32 @@ export class ServiceWorkerMcpRouter implements McpRouter {
123218
name: reg.controllerMeta.name ?? `mcp-${reg.serverName}-server`,
124219
version: reg.controllerMeta.version ?? '1.0.0',
125220
});
126-
const middlewares = this.buildMiddlewares(reg);
127221

128222
const postRouterFunc = router.post;
129223
const initHandler = async (ctx: ServiceWorkerFetchContext) => {
130-
const denied = await this.mcpAuthHandler.authenticate(ctx.event.request);
224+
const request = ctx.event.request;
225+
// Parse the body first (via a clone, no side effect) so target selection
226+
// and the SDK share one read; authentication remains the outermost gate
227+
// before any middleware or transport dispatch runs.
228+
const parsedBody = await this.#readJsonRpcBody(request);
229+
const denied = await this.mcpAuthHandler.authenticate(request);
131230
if (denied) {
132231
ctx.response = denied;
133232
return;
134233
}
135-
const transport = await this.createServerTransport(reg, helperFactory);
136-
ctx.response = await transport.handleRequest(ctx.event.request);
234+
const options: HandleRequestOptions | undefined = parsedBody === undefined ? undefined : { parsedBody };
235+
const dispatch: MCPMiddleware = async () => {
236+
const transport = await this.createServerTransport(reg, helperFactory);
237+
ctx.response = await transport.handleRequest(request, options);
238+
};
239+
await composeMiddlewares([...this.#selectMiddlewares(reg, parsedBody), dispatch])(ctx);
137240
};
138241

139242
const streamPath = `/mcp${name ? `/${name}` : ''}/stream`;
140243
const basePath = `/mcp${name ? `/${name}` : ''}`;
141244
const paths = [streamPath, basePath];
142245
for (const path of paths) {
143-
Reflect.apply(postRouterFunc, router, ['mcpStatelessStreamInit', path, ...middlewares, initHandler]);
246+
Reflect.apply(postRouterFunc, router, ['mcpStatelessStreamInit', path, initHandler]);
144247
}
145248

146249
// Only POST is allowed for stateless streamable HTTP

tegg/standalone/service-worker-controller/src/types.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,19 @@ export interface MCPAuthHandler {
1414
authenticate(request: Request): Promise<Response | undefined>;
1515
}
1616

17+
/**
18+
* DNS-rebinding protection for the MCP transport, forwarded to the SDK's
19+
* web-standard transport. When `allowedHosts`/`allowedOrigins` are configured
20+
* the SDK validates the request Host/Origin; enable-protection defaults on once
21+
* either list is set. Left empty (the default) there is no host/origin gate —
22+
* set it before exposing the MCP endpoint beyond loopback.
23+
*/
24+
export interface MCPTransportOptions {
25+
allowedHosts?: string[];
26+
allowedOrigins?: string[];
27+
enableDnsRebindingProtection?: boolean;
28+
}
29+
1730
export interface ServiceWorkerContextInit<T> {
1831
event: T;
1932
}

tegg/standalone/service-worker/src/ServiceWorkerApp.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { Readable } from 'node:stream';
44
import { pipeline } from 'node:stream/promises';
55
import { fileURLToPath } from 'node:url';
66

7-
import { FetchEventImpl, type MCPAuthHandler } from '@eggjs/service-worker-controller';
7+
import { FetchEventImpl, type MCPAuthHandler, type MCPTransportOptions } from '@eggjs/service-worker-controller';
88
import { ContextProtoProperty } from '@eggjs/service-worker-runtime';
99
import {
1010
StandaloneApp,
@@ -18,6 +18,8 @@ export interface ServiceWorkerAppOptions extends StandaloneAppOptions {
1818
config?: Record<string, any>;
1919
/** Auth hook for MCP routes; the default lets every request through. */
2020
mcpAuthHandler?: MCPAuthHandler;
21+
/** DNS-rebinding protection for the MCP transport (Host/Origin allow-lists). */
22+
mcp?: MCPTransportOptions;
2123
}
2224

2325
const PASS_THROUGH_MCP_AUTH_HANDLER: MCPAuthHandler = {
@@ -43,7 +45,7 @@ export class ServiceWorkerApp {
4345
#initialized = false;
4446

4547
constructor(cwd: string, options?: ServiceWorkerAppOptions) {
46-
const { config, mcpAuthHandler, ...standaloneOptions } = options ?? {};
48+
const { config, mcpAuthHandler, mcp, ...standaloneOptions } = options ?? {};
4749
// Scan this package's own root so its framework-module deps
4850
// (service-worker-runtime + -controller) are auto-discovered via the
4951
// node_modules eggModule convention. `!test/**` keeps their test fixtures out.
@@ -64,6 +66,7 @@ export class ServiceWorkerApp {
6466
innerObjects: {
6567
config: [{ obj: config ?? {} }],
6668
mcpAuthHandler: [{ obj: mcpAuthHandler ?? PASS_THROUGH_MCP_AUTH_HANDLER }],
69+
mcpTransportOptions: [{ obj: mcp ?? {} }],
6770
...standaloneOptions.innerObjectHandlers,
6871
},
6972
});

tegg/standalone/service-worker/test/MCP.test.ts

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/
88
import { afterAll, beforeAll, describe, it } from 'vitest';
99

1010
import { ServiceWorkerApp } from '../src/index.ts';
11+
import { MCP_MW_CALLS } from './fixtures/hello-app/mcpMiddlewareRecorder.ts';
1112

1213
const MCP_HEADERS = {
1314
accept: 'application/json, text/event-stream',
@@ -79,6 +80,34 @@ describe('standalone/service-worker/test/MCP.test.ts', () => {
7980
assert.equal(body.error.code, -32000);
8081
});
8182

83+
it('should run controller + method middlewares only for the targeted tool', async () => {
84+
MCP_MW_CALLS.length = 0;
85+
// tools/list targets no tool → no business middleware runs
86+
await fetch(`${base}/mcp/mwcalc/stream`, {
87+
method: 'POST',
88+
headers: MCP_HEADERS,
89+
body: JSON.stringify({ jsonrpc: '2.0', id: 1, method: 'tools/list', params: {} }),
90+
});
91+
assert.deepEqual(MCP_MW_CALLS, []);
92+
93+
// tools/call targets `echo` → controller middleware (outer) then method
94+
// middleware (inner) run
95+
const res = await fetch(`${base}/mcp/mwcalc/stream`, {
96+
method: 'POST',
97+
headers: MCP_HEADERS,
98+
body: JSON.stringify({
99+
jsonrpc: '2.0',
100+
id: 2,
101+
method: 'tools/call',
102+
params: { name: 'echo', arguments: { v: 'hi' } },
103+
}),
104+
});
105+
assert.equal(res.status, 200);
106+
const message = parseSSEMessage(await res.text());
107+
assert.deepEqual(message.result.content, [{ type: 'text', text: 'hi' }]);
108+
assert.deepEqual(MCP_MW_CALLS, ['controller-mw', 'tool-mw']);
109+
});
110+
82111
it('should serve a full MCP SDK client round-trip', async () => {
83112
const client = new Client({ name: 'test-client', version: '1.0.0' });
84113
const transport = new StreamableHTTPClientTransport(new URL(`${base}/mcp/calc`));
@@ -146,3 +175,58 @@ describe('standalone/service-worker/test/MCP.test.ts mcpAuthHandler', () => {
146175
assert.equal(message.result.tools.length, 1);
147176
});
148177
});
178+
179+
describe('standalone/service-worker/test/MCP.test.ts hardening', () => {
180+
it('should reject a request whose Host is not in allowedHosts', async () => {
181+
const app = new ServiceWorkerApp(path.join(__dirname, 'fixtures/hello-app'), {
182+
mcp: { allowedHosts: ['allowed.example'] },
183+
});
184+
const server = await app.serve();
185+
const { address, port } = server.address() as AddressInfo;
186+
try {
187+
// The real Host (127.0.0.1:port) is not in the allow-list → SDK rejects.
188+
const res = await fetch(`http://${address}:${port}/mcp/calc/stream`, {
189+
method: 'POST',
190+
headers: MCP_HEADERS,
191+
body: JSON.stringify({ jsonrpc: '2.0', id: 1, method: 'tools/list', params: {} }),
192+
});
193+
assert.equal(res.status, 403);
194+
} finally {
195+
await app.destroy();
196+
}
197+
});
198+
199+
it('should still serve when the auth hook consumes the request body', async () => {
200+
const seen: unknown[] = [];
201+
const app = new ServiceWorkerApp(path.join(__dirname, 'fixtures/hello-app'), {
202+
mcpAuthHandler: {
203+
async authenticate(request: Request) {
204+
// Auth reads the one-shot body; the SDK must still parse via the
205+
// pre-read parsedBody rather than the now-consumed stream.
206+
seen.push(await request.json());
207+
return undefined;
208+
},
209+
},
210+
});
211+
const server = await app.serve();
212+
const { address, port } = server.address() as AddressInfo;
213+
try {
214+
const res = await fetch(`http://${address}:${port}/mcp/calc/stream`, {
215+
method: 'POST',
216+
headers: MCP_HEADERS,
217+
body: JSON.stringify({
218+
jsonrpc: '2.0',
219+
id: 1,
220+
method: 'tools/call',
221+
params: { name: 'add', arguments: { a: 2, b: 3 } },
222+
}),
223+
});
224+
assert.equal(res.status, 200);
225+
const message = parseSSEMessage(await res.text());
226+
assert.deepEqual(message.result.content, [{ type: 'text', text: 'hello, mcp: 5' }]);
227+
assert.equal(seen.length, 1);
228+
} finally {
229+
await app.destroy();
230+
}
231+
});
232+
});

0 commit comments

Comments
 (0)