fix(mcp): take request targets verbatim, require a lone initialize and keep failed hooks retryable

Addresses the tenth automated review round: the bridge joins an origin-form
request target onto the listener's origin so a network-path target stays a
path and absolute-form targets are refused; only a lone initialize can replace
an open session; and the shutdown hook records completion only after it
returns, so a hook that threw stays eligible for the next close.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

docs/mcp-contracts.md

@@ -453,7 +453,7 @@ actually bound: any `127.0.0.0/8` address, `::1`, an IPv4-mapped loopback and
 a bind, requests whose `Host` header is outside the SDK loopback allowlist plus
 the configured name and the bound address are refused with `403`, and a browser
 `Origin` outside that allowlist is refused the same way (a missing `Origin`
-passes); a non-loopback bind relaxes both checks and logs the relaxation. A reply sent before a request body was read (a refused `Host` or `Origin`, an unknown route, the startup `503`) closes the connection after the reply, so an unread upload cannot keep a socket open. Scoped
+passes); a non-loopback bind relaxes both checks and logs the relaxation. A reply sent before a request body was read (a refused `Host` or `Origin`, an unknown route, the startup `503`) closes the connection after the reply, so an unread upload cannot keep a socket open. Request targets are taken verbatim as origin-form paths (`//host/path` is a path, not another origin); an absolute-form target is refused with `400`, and a method Fetch does not support with `405`. Only a lone `initialize` can replace an open session; a batch that carries one is left to the transport's refusal. Scoped
 IPv6 literals (`fe80::1%eth0`) are refused before binding, because no URL can
 name them. Listener errors reach the runtime's connection-loss handler only
 after a successful bind; a bind failure prints the listen-failure message alone. Request bodies are bounded by the SDK's

docs/specs/agent-environments/http-transport/verification.md

@@ -265,3 +265,13 @@ comes from the incoming message's own body declaration, so a `GET` or `HEAD`
 that carries a body also closes, and the regression uses a keep-alive agent
 and checks that the socket actually closes after the early reply while a
 bodiless `GET` keeps its socket.
+
+A tenth round (Codex, three findings): the takeover guard now requires a lone
+`initialize`, mirroring the transport (test: a batch of `initialize` plus
+`notifications/initialized` is refused while the connected client keeps
+echoing); the shutdown hook records completion only after it returns, so a
+hook that threw stays eligible for the next close (test: a hook that throws
+once runs twice across two closes and then no more); and the bridge joins the
+request target onto the listener's origin verbatim, refusing absolute-form
+targets (test: `//attacker.example/card` is a 404 for that path, a plain route
+sees the bound host only, and `http://attacker.example/card` gets `400`).

src/server/http/nodeHttpBridge.ts

@@ -5,6 +5,20 @@ import { pipeline } from "node:stream/promises";
 /** Fetch rejects a body on these methods, so their request streams are not forwarded. */
 const BODYLESS_METHODS: ReadonlySet<string> = new Set(["GET", "HEAD"]);
 
+/** The request target was not an origin-form path; the bridge serves an origin server only. */
+export class UnsupportedRequestTargetError extends Error {
+  override readonly name = "UnsupportedRequestTargetError";
+  constructor(target: string) {
+    super(`Unsupported request target ${target}`);
+  }
+}
+
+/** Join an origin-form target onto the listener's origin verbatim, so `//host/path` stays a path. */
+const requestUrl = (target: string, origin: string): URL => {
+  if (!target.startsWith("/")) throw new UnsupportedRequestTargetError(target);
+  return new URL(`${origin}${target}`);
+};
+
 const toHeaders = (incoming: IncomingMessage["headers"]): Headers => {
   const headers = new Headers();
   for (const [name, value] of Object.entries(incoming)) {
@@ -27,7 +41,7 @@ export const toWebRequest = (
     if (!response.writableFinished) controller.abort();
   });
   const method = request.method ?? "GET";
-  return new Request(new URL(request.url ?? "/", origin), {
+  return new Request(requestUrl(request.url ?? "/", origin), {
     method,
     headers: toHeaders(request.headers),
     signal: controller.signal,

src/server/http/serveStreamableHttp.test.ts

@@ -609,3 +609,100 @@ it("answers a method Fetch refuses instead of letting it escape the request guar
   ]);
   await client.close();
 });
+
+/** One request with a caller-controlled request target, which URL-based clients normalize away. */
+const rawTargetRequest = (
+  served: URL,
+  target: string,
+): Promise<{ status: number; body: string }> =>
+  new Promise((resolve, reject) => {
+    const request = httpRequest(
+      {
+        host: served.hostname,
+        port: Number(served.port),
+        path: target,
+        method: "GET",
+        agent: false,
+        headers: { host: `127.0.0.1:${served.port}` },
+      },
+      (response) => {
+        let body = "";
+        response.setEncoding("utf8");
+        response.on("data", (chunk: string) => {
+          body += chunk;
+        });
+        response.on("end", () =>
+          resolve({ status: response.statusCode ?? 0, body }),
+        );
+      },
+    );
+    request.on("error", reject);
+    request.end();
+  });
+
+it("keeps a network-path request target as a path and refuses absolute-form targets", async () => {
+  const seen: string[] = [];
+  const { served } = await start({
+    routes: [
+      {
+        method: "GET",
+        path: "/card",
+        handle: (request) => {
+          seen.push(new URL(request.url).host);
+          return Response.json({ ok: true });
+        },
+      },
+    ],
+  });
+  const url = new URL(served.url);
+  const networkPath = await rawTargetRequest(url, "//attacker.example/card");
+  expect(networkPath.status).toBe(404);
+  expect(networkPath.body).toContain(
+    "No route for GET //attacker.example/card",
+  );
+  const absolute = await rawTargetRequest(url, "http://attacker.example/card");
+  expect(absolute.status).toBe(400);
+  const plain = await rawTargetRequest(url, "/card");
+  expect(plain.status).toBe(200);
+  expect(seen).toEqual([`127.0.0.1:${url.port}`]);
+});
+
+it("lets only a lone initialize replace a session, not a batch that carries one", async () => {
+  const { served } = await start();
+  const { client } = await connect(served.url);
+  const url = new URL(served.url);
+  const single = JSON.parse(await initializeRequest().text()) as unknown;
+  const batch = await rawHttpRequest(url, {
+    method: "POST",
+    headers: {
+      host: `127.0.0.1:${url.port}`,
+      "content-type": "application/json",
+      accept: "application/json, text/event-stream",
+    },
+    body: JSON.stringify([
+      single,
+      { jsonrpc: "2.0", method: "notifications/initialized" },
+    ]),
+  });
+  expect(batch.status).toBeGreaterThanOrEqual(400);
+  expect(batch.status).toBeLessThan(500);
+  expect(await echo(client, "not evicted")).toEqual([
+    { type: "text", text: "not evicted" },
+  ]);
+  await client.close();
+});
+
+it("retries a shutdown hook that threw instead of marking it complete", async () => {
+  let attempts = 0;
+  const slot = new McpSessionSlot(
+    createEchoServer(() => {
+      attempts += 1;
+      if (attempts === 1) throw new Error("hook failed once");
+    }),
+    undefined,
+  );
+  await expect(slot.close()).rejects.toThrow("hook failed once");
+  await expect(slot.close()).resolves.toBeUndefined();
+  await slot.close();
+  expect(attempts).toBe(2);
+});

src/server/http/serveStreamableHttp.ts

@@ -19,7 +19,11 @@ import {
 import { isIPv4, isIPv6 } from "node:net";
 import type { Logger } from "pino";
 
-import { toWebRequest, writeWebResponse } from "./nodeHttpBridge.js";
+import {
+  toWebRequest,
+  UnsupportedRequestTargetError,
+  writeWebResponse,
+} from "./nodeHttpBridge.js";
 import { StreamableHttpListenError } from "./streamableHttpListenError.js";
 
 /** MCP endpoint path shared by `rea mcp --http` and agent environment servers. */
@@ -136,8 +140,9 @@ const isInitializePost = async (request: Request): Promise<boolean> => {
     const read = await readRequestBody(probe);
     if (read.tooLarge) return false;
     const body: unknown = JSON.parse(read.text);
+    // The transport accepts only a lone initialize, so only that may evict a session.
     return Array.isArray(body)
-      ? body.some((message) => isInitializeRequest(message))
+      ? body.length === 1 && isInitializeRequest(body[0])
       : isInitializeRequest(body);
   } catch {
     return false;
@@ -168,10 +173,17 @@ export class McpSessionSlot {
   ) {
     const protocol = protocolOf(instance);
     const captured = protocol.onclose ?? NO_SHUTDOWN_HOOK;
+    let running = false;
+    // Completion is recorded only after the hook returns, so a hook that threw stays eligible for a retry.
     this.#shutdownHook = () => {
-      if (this.#hookRan) return;
-      this.#hookRan = true;
-      captured();
+      if (this.#hookRan || running) return;
+      running = true;
+      try {
+        captured();
+        this.#hookRan = true;
+      } finally {
+        running = false;
+      }
     };
     protocol.onclose = this.#shutdownHook;
   }
@@ -398,10 +410,18 @@ const serveNodeRequest = async (
   try {
     try {
       webRequest = toWebRequest(request, response, origin());
-    } catch {
-      // Fetch refuses methods such as TRACE that node:http accepts; answer instead of throwing.
+    } catch (cause: unknown) {
+      // Fetch refuses methods such as TRACE that node:http accepts, and the bridge refuses
+      // targets that are not origin-form paths; both are answered instead of thrown.
+      const target = cause instanceof UnsupportedRequestTargetError;
       await writeWebResponse(
-        jsonError(405, -32600, `Unsupported method ${request.method ?? ""}`),
+        jsonError(
+          target ? 400 : 405,
+          -32600,
+          target
+            ? "Unsupported request target"
+            : `Unsupported method ${request.method ?? ""}`,
+        ),
         response,
         { closeConnection: true },
       );