mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-05 13:23:34 +00:00
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).
This commit is contained in:
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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)}`);
|
||||
|
||||
Reference in New Issue
Block a user