Skip to content

Commit 5dcfd78

Browse files
committed
fix(mcp): fail-closed on reserved-path collision; drop dead reserved entry (review)
- Reserved-path collision now throws (refuses to start) instead of warning, so a QVERIS_MCP_HTTP_PATH set to /health or the catalog path can't silently start a server whose MCP endpoint is shadowed (every POST -> 405). Consistent with the fail-closed non-loopback-without-auth check. - Drop the dead '${config.path}/server-card' reserved entry — it's always longer than config.path and could never match. Added a collision test.
1 parent 924e0af commit 5dcfd78

2 files changed

Lines changed: 18 additions & 5 deletions

File tree

packages/mcp/src/http.test.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,16 @@ describe('startHttpServer (end-to-end over Streamable HTTP)', () => {
305305
await res.text();
306306
});
307307

308+
it('refuses to start when the HTTP path collides with a reserved endpoint', async () => {
309+
const config = resolveTransportConfig(
310+
{ QVERIS_MCP_TRANSPORT: 'http', QVERIS_MCP_HTTP_PORT: '0', QVERIS_MCP_HTTP_PATH: '/health' },
311+
[],
312+
);
313+
await expect(
314+
startHttpServer(config, (sessionId) => createQverisServer(undefined, sessionId), CARD_INFO),
315+
).rejects.toThrow(/reserved endpoint/);
316+
});
317+
308318
it('does not serve discovery endpoints when no card info is provided', async () => {
309319
await startServer(); // no cardInfo
310320
const res = await fetch(`http://127.0.0.1:${running!.port}/mcp/server-card`, {

packages/mcp/src/http.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -284,12 +284,15 @@ export async function startHttpServer(
284284
);
285285
}
286286
// The reserved public paths are matched before the transport path; a custom
287-
// QVERIS_MCP_HTTP_PATH set to one of them would shadow the MCP endpoint.
288-
const reservedPaths = cardInfo ? ['/health', CATALOG_PATH, `${config.path}/server-card`] : ['/health'];
287+
// QVERIS_MCP_HTTP_PATH set to one of them would shadow the MCP endpoint, so
288+
// fail fast rather than start a silently-broken server (consistent with the
289+
// fail-closed auth check above). The card path (config.path + '/server-card')
290+
// can't equal config.path, so it isn't a collision candidate.
291+
const reservedPaths = cardInfo ? ['/health', CATALOG_PATH] : ['/health'];
289292
if (reservedPaths.includes(config.path)) {
290-
logger(
291-
`[qveris] WARNING: QVERIS_MCP_HTTP_PATH="${config.path}" collides with a reserved ` +
292-
'endpoint (health/discovery) and would shadow the MCP transport; choose a different path.\n',
293+
throw new Error(
294+
`Refusing to start: QVERIS_MCP_HTTP_PATH="${config.path}" collides with a reserved ` +
295+
`endpoint (${reservedPaths.join(', ')}) and would shadow the MCP transport; choose a different path.`,
293296
);
294297
}
295298

0 commit comments

Comments
 (0)