From 5c3557bb0e895810cde9f4249f7cc1e1636ae035 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=AF=86=E7=A0=810000?= Date: Fri, 22 May 2026 09:58:00 +0800 Subject: [PATCH] fix(mcp): validate bearer during initialize --- mcp/src/http-server.ts | 68 +++++++++++++++++++++++++++++++----------- mcp/tests/http.test.ts | 59 ++++++++++++++++++++++++++++++++++-- 2 files changed, 107 insertions(+), 20 deletions(-) diff --git a/mcp/src/http-server.ts b/mcp/src/http-server.ts index f05bb30..bb7c114 100644 --- a/mcp/src/http-server.ts +++ b/mcp/src/http-server.ts @@ -163,6 +163,17 @@ function resourceMetadataURL(req: Request, opts: HttpServerOptions): string { return `${base}/.well-known/oauth-protected-resource`; } +function bearerChallenge(req: Request, opts: HttpServerOptions): string { + return `Bearer realm="e2a", resource_metadata="${resourceMetadataURL(req, opts)}"`; +} + +class InvalidBearerError extends Error { + constructor() { + super("invalid bearer token"); + this.name = "InvalidBearerError"; + } +} + async function handleClientRequest( req: Request, res: Response, @@ -171,10 +182,7 @@ async function handleClientRequest( ): Promise { const bearer = extractBearer(req); if (!bearer) { - res.setHeader( - "WWW-Authenticate", - `Bearer realm="e2a", resource_metadata="${resourceMetadataURL(req, opts)}"`, - ); + res.setHeader("WWW-Authenticate", bearerChallenge(req, opts)); res.status(401).json(jsonRpcError(req.body, -32001, "missing bearer token")); return; } @@ -200,7 +208,20 @@ async function handleClientRequest( } if (!entry && isInitializeRequest(req.body)) { - const client = await buildSessionClient(opts, bearer); + let client: E2AClient; + try { + client = await buildSessionClient(opts, bearer); + } catch (err) { + if (err instanceof InvalidBearerError) { + res.setHeader( + "WWW-Authenticate", + `${bearerChallenge(req, opts)}, error="invalid_token"`, + ); + res.status(401).json(jsonRpcError(req.body, -32001, "invalid bearer token")); + return; + } + throw err; + } const server = buildServer({ client }); const bearerFp = fingerprintBearer(bearer); const transport = new StreamableHTTPServerTransport({ @@ -259,10 +280,7 @@ async function handleStreamingOrDelete( // notification stream or terminate it. const bearer = extractBearer(req); if (!bearer) { - res.setHeader( - "WWW-Authenticate", - `Bearer realm="e2a", resource_metadata="${resourceMetadataURL(req, opts)}"`, - ); + res.setHeader("WWW-Authenticate", bearerChallenge(req, opts)); res.status(401).end(); return; } @@ -337,23 +355,39 @@ export async function buildSessionClient( }; const client = make(); - // Env-var path already populated agentEmail — operator opted in - // explicitly, don't second-guess them with an API call. - if (client.agentEmail) return client; - let resolved: string | undefined; try { const { agents } = await client.listAgents(); - if (Array.isArray(agents) && agents.length === 1) { + // Env-var path already populated agentEmail — operator opted in + // explicitly, don't second-guess it with auto-resolution. + if (!client.agentEmail && Array.isArray(agents) && agents.length === 1) { resolved = agents[0]?.email; } - } catch { - // Best-effort. Multi-agent / zero-agent / network-failed all land - // here and leave agentEmail empty. + } catch (err) { + if (isUnauthorizedError(err)) { + throw new InvalidBearerError(); + } + // Non-auth failures remain best-effort. A transient backend hiccup + // shouldn't break MCP initialize; worst case, the user sees the + // same "agentEmail is required" error they'd see today. } return resolved ? make(resolved) : client; } +function isUnauthorizedError(err: unknown): boolean { + if (!err || typeof err !== "object") return false; + const maybe = err as { + status?: unknown; + statusCode?: unknown; + response?: { status?: unknown }; + }; + return ( + maybe.status === 401 + || maybe.statusCode === 401 + || maybe.response?.status === 401 + ); +} + function extractBearer(req: Request): string | null { const auth = req.headers.authorization; if (!auth || typeof auth !== "string") return null; diff --git a/mcp/tests/http.test.ts b/mcp/tests/http.test.ts index 3a6ab9a..60102be 100644 --- a/mcp/tests/http.test.ts +++ b/mcp/tests/http.test.ts @@ -3,6 +3,7 @@ import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { StreamableHTTPClientTransport } from "@modelcontextprotocol/sdk/client/streamableHttp.js"; import type { E2AClient } from "@e2a/sdk/v1"; import { startHttpServer } from "../src/http-server.js"; +import { Sessions } from "../src/session.js"; // Reuse the same stub shape from tools.test.ts. Only the methods the // tools actually call need to be present. @@ -26,6 +27,12 @@ function makeStubClient(): E2AClient { return stub as unknown as E2AClient; } +function makeHttpError(statusCode: number): Error & { statusCode: number } { + const err = new Error(`HTTP ${statusCode}`) as Error & { statusCode: number }; + err.statusCode = statusCode; + return err; +} + describe("HTTP MCP server", () => { let stub: E2AClient; let close: () => Promise; @@ -82,6 +89,50 @@ describe("HTTP MCP server", () => { expect(body.error.message).toMatch(/missing bearer/); }); + it("invalid bearer returns 401 during initialize before session allocation", async () => { + await close(); + const sessions = new Sessions({ idleTimeoutMs: 60_000, maxSessions: 10 }); + const invalidStub = makeStubClient(); + invalidStub.listAgents = vi.fn(async () => { + throw makeHttpError(401); + }) as E2AClient["listAgents"]; + const { close: c, port } = await startHttpServer(0, { + baseUrl: "http://e2a.local", + allowedHosts: ["127.0.0.1", "localhost"], + clientFactory: () => invalidStub, + sessions, + }); + close = c; + url = `http://127.0.0.1:${port}/mcp`; + + const res = await fetch(url, { + method: "POST", + headers: { + "Content-Type": "application/json", + Accept: "application/json, text/event-stream", + Authorization: "Bearer bogus_token", + }, + body: JSON.stringify({ + jsonrpc: "2.0", + id: 1, + method: "initialize", + params: { + protocolVersion: "2024-11-05", + capabilities: {}, + clientInfo: { name: "x", version: "0" }, + }, + }), + }); + + expect(res.status).toBe(401); + expect(res.headers.get("www-authenticate")).toMatch(/Bearer realm="e2a"/); + expect(res.headers.get("mcp-session-id")).toBeNull(); + expect(sessions.size()).toBe(0); + expect(invalidStub.listAgents).toHaveBeenCalledOnce(); + const body = await res.json(); + expect(body.error.message).toMatch(/invalid bearer/); + }); + it("rejects requests that present a known session-id with a different bearer", async () => { // Regression for the session-id-hijack class: the per-session // E2AClient holds the original bearer baked in. Without session- @@ -280,7 +331,7 @@ describe("HTTP MCP server", () => { await transport.close(); }); - it("skips the listAgents probe when agentEmail is already set", async () => { + it("validates the bearer but skips reconstruction when agentEmail is already set", async () => { // Stub from beforeEach already has agentEmail "bot@example.com". const factory = vi.fn(() => stub); await close(); @@ -297,8 +348,9 @@ describe("HTTP MCP server", () => { // since the env-var path (constructor) already populated it. expect(factory).toHaveBeenCalledTimes(1); expect(factory.mock.calls[0]).toEqual(["e2a_test"]); - // And listAgents was NOT invoked as a probe at session init. - expect(stub.listAgents).not.toHaveBeenCalled(); + // listAgents still validates the bearer once at session init, + // but does not trigger a second construction for auto-resolution. + expect(stub.listAgents).toHaveBeenCalledOnce(); await transport.close(); }); @@ -370,6 +422,7 @@ describe("HTTP MCP server", () => { it("tool call dispatches to the per-session client", async () => { const { client, transport } = await connect(); + vi.mocked(stub.listAgents).mockClear(); await client.callTool({ name: "list_agents", arguments: {} }); expect(stub.listAgents).toHaveBeenCalledOnce(); await transport.close();