Skip to content

Commit 924e0af

Browse files
committed
fix(mcp): harden discovery caching + reserved-path guard (review)
- Discovery docs: when the advertised origin is derived from the request Host (no QVERIS_MCP_PUBLIC_URL), respond with Cache-Control: no-store + Vary so a shared cache can't be poisoned to serve a cross-host remote URL. With a configured public origin the response is host-independent and stays cacheable. - Warn at startup if QVERIS_MCP_HTTP_PATH collides with a reserved public path (/health or a discovery path), which would shadow the MCP transport.
1 parent 7d9473e commit 924e0af

2 files changed

Lines changed: 21 additions & 1 deletion

File tree

packages/mcp/src/http.test.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,9 @@ describe('startHttpServer (end-to-end over Streamable HTTP)', () => {
277277
expect(res.status).toBe(200);
278278
expect(res.headers.get('content-type')).toContain('application/mcp-server-card+json');
279279
expect(res.headers.get('access-control-allow-origin')).toBe('*');
280+
// Host-derived origin must not be cached cross-host (poisoning guard).
281+
expect(res.headers.get('cache-control')).toContain('no-store');
282+
expect(res.headers.get('vary')).toContain('Host');
280283
const card = (await res.json()) as Record<string, unknown>;
281284
expect(card.name).toBe('io.github.QVerisAI/mcp');
282285
expect(card.version).toBe('9.9.9');
@@ -314,6 +317,8 @@ describe('startHttpServer (end-to-end over Streamable HTTP)', () => {
314317
it('honors QVERIS_MCP_PUBLIC_URL in discovery URLs (behind a proxy)', async () => {
315318
await startServer({ QVERIS_MCP_PUBLIC_URL: 'https://mcp.example.com' }, CARD_INFO);
316319
const res = await fetch(`http://127.0.0.1:${running!.port}/mcp/server-card`);
320+
// A configured public origin is host-independent, so it's freely cacheable.
321+
expect(res.headers.get('cache-control')).toContain('public');
317322
const card = (await res.json()) as { remotes: Array<{ url: string }> };
318323
expect(card.remotes[0].url).toBe('https://mcp.example.com/mcp');
319324
});

packages/mcp/src/http.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,15 @@ export async function startHttpServer(
283283
'Ensure an external auth layer protects the endpoint.\n',
284284
);
285285
}
286+
// 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'];
289+
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+
);
294+
}
286295

287296
const transports = new Map<string, StreamableHTTPServerTransport>();
288297
const lastSeen = new Map<string, number>();
@@ -372,7 +381,13 @@ export async function startHttpServer(
372381
}
373382
if (req.method === 'GET') {
374383
const origin = requestOrigin(req, config.publicUrl);
375-
const cache = { 'Cache-Control': 'public, max-age=3600' };
384+
// With a configured public origin the response is host-independent and
385+
// freely cacheable. When the origin is derived from the request Host,
386+
// don't let a shared cache serve it cross-host (cache-poisoning): mark
387+
// it no-store and Vary on the headers it depends on.
388+
const cache: Record<string, string> = config.publicUrl
389+
? { 'Cache-Control': 'public, max-age=3600' }
390+
: { 'Cache-Control': 'no-store', Vary: 'Host, X-Forwarded-Proto' };
376391
if (url.pathname === cardPath) {
377392
const card = buildServerCard(cardInfo, `${origin}${config.path}`);
378393
writeJson(res, 200, card, { ...cors, ...cache, 'Content-Type': SERVER_CARD_MEDIA_TYPE });

0 commit comments

Comments
 (0)