fix(server): harden the pinned remote MCP transport's failure paths

Pinning to `approved[0]` alone dropped the failover `fetch` gave a
multi-homed host, so one dead A record took the whole connection down.
Walk the approved addresses in resolution order instead — but only for
addresses that are unreachable. A peer that answers from an unapproved
address is the rebinding defence firing, not a reachability problem, so
it still fails closed on the first try rather than being retried past.

Also close the gaps the response deadline left behind:

- a connect timeout used to leak the half-open socket, since nothing
  above that point owned it yet;
- both deadlines threw bare `Error`s, so callers that switch on the
  guard's error codes saw a timeout as an unclassified failure — they
  now carry `remote_http_response_timeout`;
- a finished response left an armed socket timer behind; and
- the default deadline drops from undici's 300 s to 30 s, which is
  already generous for metadata discovery, a token exchange or a
  JSON-RPC round trip. `tools/call` owns a caller-set budget an operator
  can raise to 60 s, so it passes `responseTimeoutMs` explicitly rather
  than being truncated by the tighter default.

Covers each with a regression test, including a peer that dribbles
header bytes forever — the case that rules out `req.setTimeout`, whose
idle clock such a peer can reset indefinitely.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

server/src/__tests__/remote-http-rebinding.test.ts

@@ -1,5 +1,5 @@
 import { createServer, type IncomingMessage, type Server, type ServerResponse } from "node:http";
-import { connect as netConnect, type AddressInfo, type Socket } from "node:net";
+import { connect as netConnect, createServer as netCreateServer, type AddressInfo, type Socket } from "node:net";
 import { gzipSync } from "node:zlib";
 import { afterEach, describe, expect, it } from "vitest";
 
@@ -79,18 +79,26 @@ function guardError(message: string, code: string) {
 function routingSocketFactory(routes: Record<string, number>): {
   factory: RemoteHttpSocketFactory;
   dialled: string[];
+  sockets: Socket[];
 } {
   const dialled: string[] = [];
+  const sockets: Socket[] = [];
   const factory: RemoteHttpSocketFactory = (target) => {
     dialled.push(target.address);
     const port = routes[target.address];
     if (port === undefined) throw new Error(`test network has no route to ${target.address}`);
     const socket = netConnect({ host: "127.0.0.1", port });
     openSockets.push(socket);
+    sockets.push(socket);
     Object.defineProperty(socket, "remoteAddress", { get: () => target.address, configurable: true });
     return socket;
   };
-  return { factory, dialled };
+  return { factory, dialled, sockets };
+}
+
+/** Waits for the socket bookkeeping the transport does after it rejects. */
+async function flush(): Promise<void> {
+  await new Promise((resolve) => setTimeout(resolve, 50));
 }
 
 /** A name server that answers public first and loopback afterwards. */
@@ -309,9 +317,45 @@ describe("guarded remote HTTP fetch (PAP-17098 DNS rebinding)", () => {
       socketFactory: network.factory,
       responseTimeoutMs: 150,
       error: guardError,
-    })).rejects.toThrow(/did not send response headers/);
+    })).rejects.toMatchObject({ code: "remote_http_response_timeout" });
 
+    // The deadline has to hand back the socket, not just the request handler:
+    // a bounded call that still leaks a descriptor per silent peer is the same
+    // exhaustion bug wearing a timer.
     expect(upstream.requests).toHaveLength(1);
+    await flush();
+    expect(network.sockets.map((socket) => socket.destroyed)).toEqual([true]);
+  });
+
+  it("gives up on a peer that dribbles response headers forever", async () => {
+    // This is why the headers deadline is a hard timer rather than
+    // `req.setTimeout`: an idle timeout never fires against this peer, because
+    // every trickled header line resets it.
+    const raw = netCreateServer((socket) => {
+      socket.write("HTTP/1.1 200 OK\r\n");
+      const beat = setInterval(() => socket.write("x-pad: keepalive\r\n"), 25);
+      const stop = () => clearInterval(beat);
+      socket.on("close", stop);
+      socket.on("error", stop);
+    });
+    await new Promise<void>((resolve) => raw.listen(0, "127.0.0.1", resolve));
+    const port = (raw.address() as AddressInfo).port;
+    const network = routingSocketFactory({ [PUBLIC_ADDRESS]: port });
+
+    try {
+      await expect(guardedRemoteHttpFetch(`http://${REBIND_HOST}/mcp`, {}, {
+        allowPrivateNetwork: false,
+        lookup: async () => [{ address: PUBLIC_ADDRESS, family: 4 }],
+        socketFactory: network.factory,
+        responseTimeoutMs: 150,
+        error: guardError,
+      })).rejects.toMatchObject({ code: "remote_http_response_timeout" });
+
+      await flush();
+      expect(network.sockets.map((socket) => socket.destroyed)).toEqual([true]);
+    } finally {
+      await new Promise<void>((resolve) => raw.close(() => resolve()));
+    }
   });
 
   it("gives up on a response body that stalls midway", async () => {
@@ -331,7 +375,64 @@ describe("guarded remote HTTP fetch (PAP-17098 DNS rebinding)", () => {
     });
 
     expect(response.status).toBe(200);
-    await expect(response.text()).rejects.toThrow();
+    await expect(response.text()).rejects.toMatchObject({ code: "remote_http_response_timeout" });
+    await flush();
+    expect(network.sockets.map((socket) => socket.destroyed)).toEqual([true]);
+  });
+
+  it("falls over to the next approved address when the first is unreachable", async () => {
+    // Pinning must not cost the failover `fetch` gave a multi-homed host: one
+    // dead A record should not take the connection down with it.
+    const upstream = await startServer();
+    const dead = await startServer();
+    // Close the listener so the address is routable but refuses connections.
+    await new Promise<void>((resolve) => dead.server.close(() => resolve()));
+    const network = routingSocketFactory({
+      "93.184.216.35": dead.port,
+      [PUBLIC_ADDRESS]: upstream.port,
+    });
+
+    const response = await guardedRemoteHttpFetch(`http://${REBIND_HOST}/mcp`, {}, {
+      allowPrivateNetwork: false,
+      lookup: async () => [
+        { address: "93.184.216.35", family: 4 },
+        { address: PUBLIC_ADDRESS, family: 4 },
+      ],
+      socketFactory: network.factory,
+      error: guardError,
+    });
+
+    await expect(response.json()).resolves.toEqual({ ok: true });
+    expect(network.dialled).toEqual(["93.184.216.35", PUBLIC_ADDRESS]);
+  });
+
+  it("fails closed instead of falling over when a peer fails the address check", async () => {
+    // Failover covers unreachable addresses only. A peer that answers from an
+    // unapproved address is the rebinding defence firing, and must not be
+    // retried past.
+    const internal = await startServer();
+    const upstream = await startServer();
+    const dialled: string[] = [];
+    const factory: RemoteHttpSocketFactory = (target) => {
+      dialled.push(target.address);
+      const socket = netConnect({ host: "127.0.0.1", port: internal.port });
+      openSockets.push(socket);
+      return socket;
+    };
+
+    await expect(guardedRemoteHttpFetch(`http://${REBIND_HOST}/mcp`, {}, {
+      allowPrivateNetwork: false,
+      lookup: async () => [
+        { address: "93.184.216.35", family: 4 },
+        { address: PUBLIC_ADDRESS, family: 4 },
+      ],
+      socketFactory: factory,
+      error: guardError,
+    })).rejects.toMatchObject({ code: "remote_http_private_endpoint" });
+
+    expect(dialled).toEqual(["93.184.216.35"]);
+    expect(internal.requests).toHaveLength(0);
+    expect(upstream.requests).toHaveLength(0);
   });
 
   it("still refuses a private IP literal before any transport runs", async () => {

server/src/services/remote-http-fetch.ts

@@ -19,12 +19,19 @@ const NULL_BODY_STATUSES = new Set([101, 103, 204, 205, 304]);
 const DEFAULT_CONNECT_TIMEOUT_MS = 10_000;
 
 /**
- * Matches undici's `headersTimeout`/`bodyTimeout`, which the platform `fetch`
- * applied for free. Without it a remote server that accepts the connection and
- * then stays silent would hold the request open indefinitely — the OAuth callers
- * pass no `AbortSignal`, so nothing else would ever cut it loose.
+ * Stands in for undici's `headersTimeout`/`bodyTimeout`, which the platform
+ * `fetch` applied for free. Without it a remote server that accepts the
+ * connection and then stays silent would hold the request open indefinitely —
+ * the OAuth callers pass no `AbortSignal`, so nothing else would ever cut it
+ * loose (PAP-17110).
+ *
+ * Deliberately far tighter than undici's 300 s: everything that reaches this
+ * transport is metadata discovery, a token exchange, a DCR call or an MCP
+ * JSON-RPC round trip, none of which has any business taking minutes. Callers
+ * that own a longer budget — `tools/call`, which an operator can raise to 60 s —
+ * pass `responseTimeoutMs` so this default never truncates it.
  */
-const DEFAULT_RESPONSE_TIMEOUT_MS = 300_000;
+const DEFAULT_RESPONSE_TIMEOUT_MS = 30_000;
 
 /** How a verified socket is opened. Overridable so tests can simulate a rebind. */
 export type RemoteHttpSocketFactory = (target: {
@@ -115,15 +122,7 @@ async function pinnedRequest(
 
   signal?.throwIfAborted?.();
 
-  const socket = await openVerifiedSocket({
-    address: approved[0]!,
-    approvedSet,
-    port,
-    hostname,
-    useTls,
-    signal,
-    options,
-  });
+  const socket = await dialApprovedAddress({ approved, approvedSet, port, hostname, useTls, signal, options });
 
   try {
     return await sendRequest({
@@ -135,6 +134,7 @@ async function pinnedRequest(
       init,
       signal,
       responseTimeoutMs: options.responseTimeoutMs ?? DEFAULT_RESPONSE_TIMEOUT_MS,
+      error: options.error,
     });
   } catch (error) {
     socket.destroy();
@@ -142,6 +142,45 @@ async function pinnedRequest(
   }
 }
 
+/** An approved address that could not be reached, so the next one may be tried. */
+class UnreachableAddressError extends Error {
+  constructor(readonly reason: unknown) {
+    super("Remote MCP approved address was unreachable");
+  }
+}
+
+/**
+ * Try the approved addresses in resolution order.
+ *
+ * `fetch` walks every A/AAAA record before giving up, so pinning to `approved[0]`
+ * alone would break a multi-homed host whose first record happens to be dead. A
+ * peer that fails the address check is a different matter — that is the rebinding
+ * defence firing, not a reachability problem — so it fails closed immediately
+ * rather than moving down the list.
+ */
+async function dialApprovedAddress(input: {
+  approved: string[];
+  approvedSet: Set<string>;
+  port: number;
+  hostname: string;
+  useTls: boolean;
+  signal: AbortSignal | null;
+  options: GuardedRemoteHttpFetchOptions;
+}): Promise<Socket | TLSSocket> {
+  let lastReason: unknown;
+  for (const address of input.approved) {
+    input.signal?.throwIfAborted?.();
+    try {
+      return await openVerifiedSocket({ ...input, address });
+    } catch (error) {
+      if (!(error instanceof UnreachableAddressError)) throw error;
+      lastReason = error.reason;
+    }
+  }
+  throw lastReason
+    ?? input.options.error("Remote MCP endpoint could not be reached", "remote_http_connect_failed");
+}
+
 /**
  * Connect to `address`, confirm the peer really is that approved address, and
  * only then complete the TLS handshake. Verifying before the handshake keeps
@@ -163,7 +202,15 @@ async function openVerifiedSocket(input: {
 
   const raw = factory({ address, port, hostname, useTls });
 
-  await once(raw, "connect", { signal, timeoutMs: connectTimeoutMs, what: "connect to" });
+  try {
+    await once(raw, "connect", { signal, timeoutMs: connectTimeoutMs, what: "connect to" });
+  } catch (error) {
+    // A connect timeout used to leave the half-open socket behind, because
+    // nothing above this point owns it yet.
+    raw.destroy();
+    if (signal?.aborted) throw error;
+    throw new UnreachableAddressError(error);
+  }
 
   const peer = raw.remoteAddress ? normalizeIpAddress(raw.remoteAddress) : null;
   if (!peer || isPrivateOrReservedIp(peer) || !approvedSet.has(peer)) {
@@ -188,7 +235,8 @@ async function openVerifiedSocket(input: {
   } catch (error) {
     secure.destroy();
     raw.destroy();
-    throw error;
+    if (signal?.aborted) throw error;
+    throw new UnreachableAddressError(error);
   }
   return secure;
 }
@@ -202,8 +250,9 @@ async function sendRequest(input: {
   init: RequestInit;
   signal: AbortSignal | null;
   responseTimeoutMs: number;
+  error: RemoteHttpEndpointErrorFactory;
 }): Promise<Response> {
-  const { endpoint, hostname, port, useTls, socket, init, signal, responseTimeoutMs } = input;
+  const { endpoint, hostname, port, useTls, socket, init, signal, responseTimeoutMs, error } = input;
   const headers = new Headers(init.headers);
   const body = readRequestBody(init.body);
   const method = (init.method ?? "GET").toUpperCase();
@@ -237,7 +286,9 @@ async function sendRequest(input: {
     // Headers deadline. `req.setTimeout` is socket-idle based, which a server
     // that dribbles bytes could reset forever, so hold a hard timer instead.
     const headersTimer = setTimeout(() => {
-      req.destroy(new Error("Remote MCP endpoint did not send response headers in time"));
+      // Destroying the request tears the socket down too, so a silent peer costs
+      // neither a pending handler nor a leaked descriptor.
+      req.destroy(error("Remote MCP endpoint did not respond in time", "remote_http_response_timeout"));
     }, responseTimeoutMs);
     headersTimer.unref?.();
     req.on("response", () => clearTimeout(headersTimer));
@@ -249,8 +300,13 @@ async function sendRequest(input: {
   // Body idle deadline, mirroring undici's `bodyTimeout`: a stalled stream is
   // destroyed so `response.text()` rejects instead of hanging the caller.
   message.setTimeout(responseTimeoutMs, () => {
-    message.destroy(new Error("Remote MCP endpoint stalled while sending its response body"));
+    message.destroy(error("Remote MCP endpoint stalled mid-response", "remote_http_response_timeout"));
   });
+  // A finished response must not leave an armed socket timer behind, or a later
+  // reader of the same socket inherits a deadline it never asked for.
+  const disarm = () => message.setTimeout(0);
+  message.once("end", disarm);
+  message.once("close", disarm);
 
   const responseHeaders = new Headers();
   for (const [key, value] of Object.entries(message.headers)) {

server/src/services/tool-gateway.ts

@@ -3107,7 +3107,13 @@ export function createToolGatewayService(
             arguments: parameters ?? {},
           },
         }),
-      }, remoteHttpFetchOptions());
+      }, {
+        ...remoteHttpFetchOptions(),
+        // This call site owns a caller-set budget that can exceed the
+        // transport's default response deadline, so hand it down rather than
+        // letting the tighter default cut a legitimately slow tool short.
+        responseTimeoutMs: ms,
+      });
       const body = await readBoundedRemoteResponse(response);
       execution.response = {
         httpStatus: response.status,