From 224ea76d29d0c235f8caf4e1cdb8c43332ffc38d Mon Sep 17 00:00:00 2001 From: Cameron Beeley Date: Sat, 6 Jun 2026 20:20:17 +0100 Subject: [PATCH] fix(gateway): keep MCP loopback stateless; add DELETE no-op + transport tests Addresses review feedback on the Streamable HTTP transport: - Keep the loopback server stateless: drop the advertised Mcp-Session-Id header (the server owns no session lifecycle, so advertising a session id clients would echo back was misleading). Resolves the stateless-vs-sessionful concern. - Add DELETE /mcp as an auth-gated 200 no-op (Streamable HTTP teardown), so clients that send DELETE on close get a clean 200 instead of 405; Allow now advertises GET, POST, DELETE. - Keep the GET/SSE notification channel (the actual fix for the 'still connecting' hang) auth-gated and browser-origin-rejected. - Add focused gateway tests: GET 200 + text/event-stream, GET 401 (no auth), GET 403 (browser origin), DELETE 200, DELETE 401, unsupported 405 with the correct Allow, and POST stays stateless (no Mcp-Session-Id). --- src/gateway/mcp-http.request.ts | 25 +++++++++++- src/gateway/mcp-http.test.ts | 72 +++++++++++++++++++++++++++++++++ src/gateway/mcp-http.ts | 6 +-- 3 files changed, 97 insertions(+), 6 deletions(-) diff --git a/src/gateway/mcp-http.request.ts b/src/gateway/mcp-http.request.ts index 470a7d0f70fa..ccd71e07fe37 100644 --- a/src/gateway/mcp-http.request.ts +++ b/src/gateway/mcp-http.request.ts @@ -169,13 +169,36 @@ export function validateMcpLoopbackRequest(params: { return null; } + if (params.req.method === "DELETE") { + // Streamable HTTP session teardown. The loopback server is stateless — it owns no + // session lifecycle — so this is an auth-gated no-op acknowledgement: clients that + // send DELETE when closing the transport get a clean 200 rather than a 405. + const authHeader = getHeader(params.req, "authorization") ?? ""; + const ownerTokenMatched = safeEqualSecret(authHeader, `Bearer ${params.ownerToken}`); + const nonOwnerTokenMatched = safeEqualSecret(authHeader, `Bearer ${params.nonOwnerToken}`); + if (!ownerTokenMatched && !nonOwnerTokenMatched) { + params.res.writeHead(401, { "Content-Type": "application/json" }); + params.res.end(JSON.stringify({ error: "unauthorized" })); + return null; + } + if (rejectsBrowserLoopbackRequest(params.req)) { + params.res.writeHead(403, { "Content-Type": "application/json" }); + params.res.end(JSON.stringify({ error: "forbidden" })); + return null; + } + logMcpLoopbackHttp("session-delete", { method: "DELETE", path: url.pathname }); + params.res.writeHead(200, { "Content-Type": "application/json" }); + params.res.end(JSON.stringify({ ok: true })); + return null; + } + if (params.req.method !== "POST") { logMcpLoopbackHttp("reject", { reason: "method_not_allowed", method: params.req.method ?? "", path: url.pathname, }); - params.res.writeHead(405, { Allow: "GET, POST" }); + params.res.writeHead(405, { Allow: "GET, POST, DELETE" }); params.res.end(); return null; } diff --git a/src/gateway/mcp-http.test.ts b/src/gateway/mcp-http.test.ts index 903a4d75f78a..ea0f0f29315b 100644 --- a/src/gateway/mcp-http.test.ts +++ b/src/gateway/mcp-http.test.ts @@ -1148,4 +1148,76 @@ describe("createMcpLoopbackServerConfig", () => { ); expect(config.mcpServers?.openclaw?.headers).not.toHaveProperty("x-openclaw-sender-is-owner"); }); + + it("opens an auth-gated SSE stream on GET (Streamable HTTP notification channel)", async () => { + server = await startMcpLoopbackServer(0); + const runtime = getActiveMcpLoopbackRuntime(); + const token = runtime ? resolveMcpLoopbackBearerToken(runtime, false) : undefined; + const res = await fetch(`http://127.0.0.1:${server.port}/mcp`, { + method: "GET", + headers: token ? { authorization: `Bearer ${token}` } : {}, + }); + expect(res.status).toBe(200); + expect(res.headers.get("content-type")).toContain("text/event-stream"); + await res.body?.cancel(); + }); + + it("rejects a GET notification channel without a bearer token (401)", async () => { + server = await startMcpLoopbackServer(0); + const res = await fetch(`http://127.0.0.1:${server.port}/mcp`, { method: "GET" }); + expect(res.status).toBe(401); + await res.body?.cancel(); + }); + + it("rejects a GET notification channel from a browser Origin (403)", async () => { + server = await startMcpLoopbackServer(0); + const runtime = getActiveMcpLoopbackRuntime(); + const token = runtime ? resolveMcpLoopbackBearerToken(runtime, false) : undefined; + const res = await fetch(`http://127.0.0.1:${server.port}/mcp`, { + method: "GET", + headers: { + ...(token ? { authorization: `Bearer ${token}` } : {}), + origin: "https://evil.example", + }, + }); + expect(res.status).toBe(403); + await res.body?.cancel(); + }); + + it("acknowledges DELETE session teardown with 200 (stateless no-op)", async () => { + server = await startMcpLoopbackServer(0); + const runtime = getActiveMcpLoopbackRuntime(); + const token = runtime ? resolveMcpLoopbackBearerToken(runtime, false) : undefined; + const res = await fetch(`http://127.0.0.1:${server.port}/mcp`, { + method: "DELETE", + headers: token ? { authorization: `Bearer ${token}` } : {}, + }); + expect(res.status).toBe(200); + }); + + it("rejects DELETE without a bearer token (401)", async () => { + server = await startMcpLoopbackServer(0); + const res = await fetch(`http://127.0.0.1:${server.port}/mcp`, { method: "DELETE" }); + expect(res.status).toBe(401); + }); + + it("rejects unsupported methods with 405 advertising GET, POST, DELETE", async () => { + server = await startMcpLoopbackServer(0); + const res = await fetch(`http://127.0.0.1:${server.port}/mcp`, { method: "PUT" }); + expect(res.status).toBe(405); + expect(res.headers.get("allow")).toBe("GET, POST, DELETE"); + }); + + it("stays stateless: POST responses advertise no Mcp-Session-Id", async () => { + server = await startMcpLoopbackServer(0); + const runtime = getActiveMcpLoopbackRuntime(); + const res = await sendRaw({ + port: server.port, + token: runtime ? resolveMcpLoopbackBearerToken(runtime, false) : undefined, + headers: { "content-type": "application/json", "x-session-key": "agent:main:main" }, + body: JSON.stringify({ jsonrpc: "2.0", id: 1, method: "tools/list" }), + }); + expect(res.status).toBe(200); + expect(res.headers.get("mcp-session-id")).toBeNull(); + }); }); diff --git a/src/gateway/mcp-http.ts b/src/gateway/mcp-http.ts index 84a0d62d1f76..3f25bd0a9506 100644 --- a/src/gateway/mcp-http.ts +++ b/src/gateway/mcp-http.ts @@ -142,7 +142,6 @@ export async function startMcpLoopbackServer(port = 0): Promise<{ }> { const ownerToken = crypto.randomBytes(32).toString("hex"); const nonOwnerToken = crypto.randomBytes(32).toString("hex"); - const mcpSessionId = crypto.randomUUID(); const toolCache = new McpLoopbackToolCache(); const httpServer = createHttpServer((req, res) => { @@ -227,10 +226,7 @@ export async function startMcpLoopbackServer(port = 0): Promise<{ const payload = Array.isArray(parsed) ? JSON.stringify(responses) : JSON.stringify(responses[0]); - res.writeHead(200, { - "Content-Type": "application/json", - "Mcp-Session-Id": mcpSessionId, - }); + res.writeHead(200, { "Content-Type": "application/json" }); res.end(payload); } catch (error) { logWarn(`mcp loopback: request handling failed: ${formatErrorMessage(error)}`);