Skip to content

fix(mcp): validate bearer during initialize #104

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Baijack-star wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(mcp): validate bearer during initialize #104

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 51 additions & 17 deletions mcp/src/http-server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -171,10 +182,7 @@ async function handleClientRequest(
): Promise<void> {
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;
}
Expand All @@ -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({
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down
59 changes: 56 additions & 3 deletions mcp/tests/http.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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<void>;
Expand Down Expand Up @@ -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-
Expand Down Expand Up @@ -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();
Expand All @@ -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();
});

Expand Down Expand Up @@ -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();
Expand Down
Loading