fix: prevent DNS-rebinding TOCTOU in safeProxyFetch by pinning resolved IPs

Previously assertSafeProxyTarget resolved the target hostname and validated
the IPs, but safeProxyFetch then passed the raw URL to node-fetch, which
performed its own DNS lookup. In the window between the two resolutions an
attacker who controls the domain's TTL could flip the record to
169.254.169.254 (cloud-metadata), causing fetch() to connect to the instance-
metadata service even though the block-list check passed.

Fix: assertSafeProxyTarget now returns the validated IP addresses. On each
hop of safeProxyFetch we call createPinnedAgent() to build an http/https.Agent
whose lookup hook unconditionally returns the pre-validated IP. node-fetch
uses that hook instead of the OS resolver, closing the TOCTOU window entirely.

Refactoring: isBlockedProxyAddress, assertSafeProxyTarget, and the new
createPinnedAgent are extracted to server/src/proxy-security.ts so they can
be unit-tested in isolation. A vitest suite is added to the server package
(the first unit tests for this package) with 26 cases covering the block-list,
DNS validation, IP-pinning, and the TOCTOU guarantee itself.

server/package.json

@@ -24,15 +24,19 @@
     "build": "tsc && shx cp -R static build",
     "start": "node build/index.js",
     "dev": "tsx watch --clear-screen=false src/index.ts",
-    "dev:windows": "tsx watch --clear-screen=false src/index.ts < NUL"
+    "dev:windows": "tsx watch --clear-screen=false src/index.ts < NUL",
+    "test": "vitest run",
+    "test:watch": "vitest"
   },
   "devDependencies": {
     "@types/cors": "^2.8.19",
     "@types/express": "^5.0.0",
+    "@types/node": "^22.0.0",
     "@types/shell-quote": "^1.7.5",
     "@types/ws": "^8.5.12",
     "tsx": "^4.19.0",
-    "typescript": "^5.6.2"
+    "typescript": "^5.6.2",
+    "vitest": "^4.1.0"
   },
   "dependencies": {
     "@modelcontextprotocol/sdk": "^1.25.2",

server/src/__tests__/proxy-security.test.ts

@@ -0,0 +1,265 @@
+/**
+ * Unit tests for proxy-security.ts
+ *
+ * Tests cover:
+ *  - isBlockedProxyAddress: IPv4, IPv6, IPv4-mapped IPv6 (hex + dotted), edge cases
+ *  - assertSafeProxyTarget: safe IPs, blocked IPs, literal-IP hosts, DNS errors
+ *  - createPinnedAgent: correct agent type, lookup always returns pinned IP
+ *  - TOCTOU guarantee: the pinned agent never invokes the OS resolver
+ */
+
+import http from "node:http";
+import https from "node:https";
+import { vi, describe, it, expect, afterEach } from "vitest";
+
+// Mock node:dns/promises before importing the module under test so that
+// assertSafeProxyTarget's dnsLookup is replaceable in each test.
+vi.mock("node:dns/promises", () => ({
+  lookup: vi.fn(),
+}));
+
+import * as dns from "node:dns/promises";
+import {
+  isBlockedProxyAddress,
+  assertSafeProxyTarget,
+  createPinnedAgent,
+  ProxyTargetError,
+} from "../proxy-security.js";
+
+// Convenience cast — vitest doesn't know the mock shape yet.
+const mockLookup = dns.lookup as ReturnType<typeof vi.fn>;
+
+afterEach(() => {
+  vi.clearAllMocks();
+});
+
+// ---------------------------------------------------------------------------
+// isBlockedProxyAddress
+// ---------------------------------------------------------------------------
+
+describe("isBlockedProxyAddress", () => {
+  describe("IPv4 link-local (169.254.0.0/16)", () => {
+    it("blocks 169.254.169.254 (AWS metadata)", () => {
+      expect(isBlockedProxyAddress("169.254.169.254")).toBe(true);
+    });
+
+    it("blocks 169.254.0.1 (first address in range)", () => {
+      expect(isBlockedProxyAddress("169.254.0.1")).toBe(true);
+    });
+
+    it("blocks 169.254.255.255 (last address in range)", () => {
+      expect(isBlockedProxyAddress("169.254.255.255")).toBe(true);
+    });
+
+    it("allows 169.253.0.1 (just outside the range)", () => {
+      expect(isBlockedProxyAddress("169.253.0.1")).toBe(false);
+    });
+
+    it("allows 170.254.0.1 (just outside the range)", () => {
+      expect(isBlockedProxyAddress("170.254.0.1")).toBe(false);
+    });
+
+    it("allows loopback 127.0.0.1", () => {
+      expect(isBlockedProxyAddress("127.0.0.1")).toBe(false);
+    });
+
+    it("allows a public IP", () => {
+      expect(isBlockedProxyAddress("93.184.216.34")).toBe(false);
+    });
+  });
+
+  describe("IPv6 link-local (fe80::/10)", () => {
+    it("blocks fe80::1", () => {
+      expect(isBlockedProxyAddress("fe80::1")).toBe(true);
+    });
+
+    it("blocks fe80::aabb:ccdd (arbitrary link-local)", () => {
+      expect(isBlockedProxyAddress("fe80::aabb:ccdd")).toBe(true);
+    });
+
+    it("allows ::1 (loopback)", () => {
+      expect(isBlockedProxyAddress("::1")).toBe(false);
+    });
+
+    it("allows 2001:db8::1 (documentation range)", () => {
+      expect(isBlockedProxyAddress("2001:db8::1")).toBe(false);
+    });
+  });
+
+  describe("AWS IPv6 IMDS (fd00:ec2::254)", () => {
+    it("blocks fd00:ec2::254 exactly", () => {
+      expect(isBlockedProxyAddress("fd00:ec2::254")).toBe(true);
+    });
+
+    it("allows fd00:ec2::255 (adjacent address)", () => {
+      expect(isBlockedProxyAddress("fd00:ec2::255")).toBe(false);
+    });
+  });
+
+  describe("IPv4-mapped IPv6 variants of 169.254.169.254", () => {
+    it("blocks dotted form ::ffff:169.254.169.254", () => {
+      expect(isBlockedProxyAddress("::ffff:169.254.169.254")).toBe(true);
+    });
+
+    it("blocks hex form ::ffff:a9fe:a9fe (WHATWG URL serialization)", () => {
+      expect(isBlockedProxyAddress("::ffff:a9fe:a9fe")).toBe(true);
+    });
+
+    it("allows IPv4-mapped loopback ::ffff:127.0.0.1", () => {
+      expect(isBlockedProxyAddress("::ffff:127.0.0.1")).toBe(false);
+    });
+  });
+
+  describe("non-IP strings", () => {
+    it("allows empty string (not an IP)", () => {
+      expect(isBlockedProxyAddress("")).toBe(false);
+    });
+
+    it("allows hostname string (not an IP)", () => {
+      expect(isBlockedProxyAddress("example.com")).toBe(false);
+    });
+  });
+});
+
+// ---------------------------------------------------------------------------
+// assertSafeProxyTarget
+// ---------------------------------------------------------------------------
+
+describe("assertSafeProxyTarget", () => {
+  it("resolves and allows a safe hostname", async () => {
+    mockLookup.mockResolvedValueOnce([{ address: "93.184.216.34", family: 4 }]);
+
+    const addrs = await assertSafeProxyTarget(new URL("http://example.com/"));
+    expect(addrs).toEqual(["93.184.216.34"]);
+    expect(mockLookup).toHaveBeenCalledWith("example.com", { all: true });
+  });
+
+  it("throws ProxyTargetError when host resolves to blocked IP", async () => {
+    mockLookup.mockResolvedValueOnce([
+      { address: "169.254.169.254", family: 4 },
+    ]);
+
+    await expect(
+      assertSafeProxyTarget(new URL("http://evil.example.com/")),
+    ).rejects.toThrow(ProxyTargetError);
+  });
+
+  it("throws ProxyTargetError when any resolved IP is blocked (mixed results)", async () => {
+    mockLookup.mockResolvedValueOnce([
+      { address: "93.184.216.34", family: 4 },
+      { address: "169.254.169.254", family: 4 },
+    ]);
+
+    await expect(
+      assertSafeProxyTarget(new URL("http://dual.example.com/")),
+    ).rejects.toThrow(ProxyTargetError);
+  });
+
+  it("throws ProxyTargetError when DNS lookup fails", async () => {
+    mockLookup.mockRejectedValueOnce(new Error("ENOTFOUND"));
+
+    await expect(
+      assertSafeProxyTarget(new URL("http://nonexistent.invalid/")),
+    ).rejects.toThrow(ProxyTargetError);
+  });
+
+  it("skips DNS lookup for literal IPv4 hosts", async () => {
+    const addrs = await assertSafeProxyTarget(
+      new URL("http://127.0.0.1/path"),
+    );
+    expect(addrs).toEqual(["127.0.0.1"]);
+    expect(mockLookup).not.toHaveBeenCalled();
+  });
+
+  it("throws ProxyTargetError for literal blocked IPv4", async () => {
+    await expect(
+      assertSafeProxyTarget(new URL("http://169.254.169.254/")),
+    ).rejects.toThrow(ProxyTargetError);
+    expect(mockLookup).not.toHaveBeenCalled();
+  });
+
+  it("skips DNS lookup for literal IPv6 hosts", async () => {
+    const addrs = await assertSafeProxyTarget(new URL("http://[::1]/"));
+    expect(addrs).toEqual(["::1"]);
+    expect(mockLookup).not.toHaveBeenCalled();
+  });
+
+  it("returns all validated addresses so caller can pick one for pinning", async () => {
+    mockLookup.mockResolvedValueOnce([
+      { address: "192.0.2.1", family: 4 },
+      { address: "192.0.2.2", family: 4 },
+    ]);
+
+    const addrs = await assertSafeProxyTarget(new URL("http://multi.example/"));
+    expect(addrs).toHaveLength(2);
+    expect(addrs).toContain("192.0.2.1");
+    expect(addrs).toContain("192.0.2.2");
+  });
+});
+
+// ---------------------------------------------------------------------------
+// createPinnedAgent
+// ---------------------------------------------------------------------------
+
+describe("createPinnedAgent", () => {
+  it("returns an http.Agent for http: protocol", () => {
+    const agent = createPinnedAgent("http:", "127.0.0.1");
+    expect(agent).toBeInstanceOf(http.Agent);
+    expect(agent).not.toBeInstanceOf(https.Agent);
+  });
+
+  it("returns an https.Agent for https: protocol", () => {
+    const agent = createPinnedAgent("https:", "127.0.0.1");
+    expect(agent).toBeInstanceOf(https.Agent);
+  });
+
+  it("pinned lookup always returns the IPv4 address regardless of queried hostname", () =>
+    new Promise<void>((resolve) => {
+      const agent = createPinnedAgent("http:", "192.0.2.99");
+      const lookup = (agent as http.Agent & { options: { lookup: Function } })
+        .options.lookup;
+
+      lookup(
+        "example.com",
+        {},
+        (err: Error | null, address: string, family: number) => {
+          expect(err).toBeNull();
+          expect(address).toBe("192.0.2.99");
+          expect(family).toBe(4);
+          resolve();
+        },
+      );
+    }));
+
+  it("pinned lookup always returns the IPv6 address and family 6", () =>
+    new Promise<void>((resolve) => {
+      const agent = createPinnedAgent("http:", "2001:db8::1");
+      const lookup = (agent as http.Agent & { options: { lookup: Function } })
+        .options.lookup;
+
+      lookup(
+        "example.com",
+        {},
+        (err: Error | null, address: string, family: number) => {
+          expect(err).toBeNull();
+          expect(address).toBe("2001:db8::1");
+          expect(family).toBe(6);
+          resolve();
+        },
+      );
+    }));
+
+  it("TOCTOU guarantee: lookup never invokes the OS resolver", () => {
+    const agent = createPinnedAgent("http:", "10.0.0.1");
+    const lookup = (agent as http.Agent & { options: { lookup: Function } })
+      .options.lookup;
+
+    const osDnsLookup = vi.fn();
+    lookup("any-hostname.example", {}, osDnsLookup);
+
+    // osDnsLookup was called as the callback, not as a resolver —
+    // the pinned implementation calls it synchronously with the fixed IP.
+    expect(osDnsLookup).toHaveBeenCalledTimes(1);
+    expect(osDnsLookup).toHaveBeenCalledWith(null, "10.0.0.1", 4);
+  });
+});

server/src/index.ts

@@ -4,6 +4,11 @@ import cors from "cors";
 import { parseArgs } from "node:util";
 import { parse as shellParseArgs } from "shell-quote";
 import nodeFetch, { Headers as NodeHeaders } from "node-fetch";
+import {
+  assertSafeProxyTarget,
+  createPinnedAgent,
+  ProxyTargetError,
+} from "./proxy-security.js";
 
 // Type-compatible wrappers for node-fetch to work with browser-style types
 const fetch = nodeFetch;
@@ -29,8 +34,6 @@ import rateLimit from "express-rate-limit";
 import { findActualExecutable } from "spawn-rx";
 import mcpProxy, { type ProxyHeaderHolder } from "./mcpProxy.js";
 import { randomUUID, randomBytes, timingSafeEqual } from "node:crypto";
-import { lookup as dnsLookup } from "node:dns/promises";
-import { isIP } from "node:net";
 import { fileURLToPath } from "url";
 import { dirname, join } from "path";
 import { readFileSync } from "fs";
@@ -863,91 +866,17 @@ app.get("/health", (req, res) => {
   });
 });
 
-// Raised when the /fetch proxy is asked to reach a disallowed target, so the
-// route can distinguish an SSRF rejection (403) from an upstream failure (500).
-class ProxyTargetError extends Error {}
-
 const MAX_PROXY_REDIRECTS = 5;
 
-// The /fetch proxy is used by the client to reach OAuth discovery/token
-// endpoints on the connected MCP server (bypassing browser CORS). Connecting
-// to loopback and RFC1918/private LAN hosts is a core use case (testing local
-// or self-hosted MCP servers), so those stay reachable. We only block
-// link-local addresses — 169.254.0.0/16 (the AWS/GCP/Azure/etc. cloud metadata
-// endpoint 169.254.169.254 lives here) and IPv6 link-local / the AWS IPv6 IMDS
-// address — which are never legitimate proxy targets and are the real SSRF
-// exfiltration vector.
-function isBlockedProxyAddress(ip: string): boolean {
-  const kind = isIP(ip);
-  if (kind === 4) {
-    const parts = ip.split(".").map(Number);
-    // 169.254.0.0/16 — IPv4 link-local (incl. cloud metadata 169.254.169.254)
-    return parts[0] === 169 && parts[1] === 254;
-  }
-  if (kind === 6) {
-    const addr = ip.toLowerCase();
-    // IPv4-mapped IPv6 (::ffff:x). The WHATWG URL parser serializes the embedded
-    // IPv4 in hex (::ffff:a9fe:a9fe for 169.254.169.254), while DNS and other
-    // sources may use the dotted form (::ffff:169.254.169.254) — handle both by
-    // extracting the IPv4 and re-checking it.
-    const mapped = addr.match(/^::ffff:(.+)$/);
-    if (mapped) {
-      const tail = mapped[1];
-      if (isIP(tail) === 4) {
-        return isBlockedProxyAddress(tail);
-      }
-      const hex = tail.match(/^([0-9a-f]{1,4}):([0-9a-f]{1,4})$/);
-      if (hex) {
-        const high = parseInt(hex[1], 16);
-        const low = parseInt(hex[2], 16);
-        return isBlockedProxyAddress(
-          `${high >> 8}.${high & 0xff}.${low >> 8}.${low & 0xff}`,
-        );
-      }
-    }
-    // fe80::/10 — IPv6 link-local
-    if (/^fe[89ab]/.test(addr)) {
-      return true;
-    }
-    // fd00:ec2::254 — AWS IPv6 instance metadata endpoint
-    return addr === "fd00:ec2::254";
-  }
-  return false;
-}
-
-// Resolves the target host and rejects it if any resolved address is a blocked
-// (link-local/metadata) address, mitigating SSRF including the DNS case where a
-// hostname resolves to the cloud metadata IP.
-async function assertSafeProxyTarget(parsedUrl: URL): Promise<void> {
-  const host = parsedUrl.hostname.replace(/^\[|\]$/g, ""); // strip IPv6 brackets
-
-  let addresses: string[];
-  if (isIP(host)) {
-    addresses = [host];
-  } else {
-    try {
-      addresses = (await dnsLookup(host, { all: true })).map((r) => r.address);
-    } catch {
-      throw new ProxyTargetError(`Could not resolve host: ${host}`);
-    }
-  }
-
-  if (addresses.length === 0) {
-    throw new ProxyTargetError(`Could not resolve host: ${host}`);
-  }
-
-  for (const address of addresses) {
-    if (isBlockedProxyAddress(address)) {
-      throw new ProxyTargetError(
-        `Refusing to proxy request to blocked address (${address})`,
-      );
-    }
-  }
-}
-
 // Fetch that follows redirects manually so every hop's target is re-validated
 // against assertSafeProxyTarget — otherwise an allowed host could redirect the
 // proxy into a blocked internal/metadata address.
+//
+// On each hop we call assertSafeProxyTarget (which resolves DNS and validates
+// every returned IP) and then pass a createPinnedAgent to node-fetch so it
+// connects to the IP we just validated instead of re-resolving the hostname.
+// This eliminates the TOCTOU window between the block-list check and the
+// actual TCP connection that a DNS-rebinding attack would exploit.
 async function safeProxyFetch(initialUrl: string, init?: RequestInit) {
   let currentUrl = new URL(initialUrl);
   let method = init?.method ?? "GET";
@@ -958,13 +887,19 @@ async function safeProxyFetch(initialUrl: string, init?: RequestInit) {
     if (!["http:", "https:"].includes(currentUrl.protocol)) {
       throw new ProxyTargetError("Only http/https URLs are allowed");
     }
-    await assertSafeProxyTarget(currentUrl);
+
+    // Resolve DNS once, validate every address, and pin the connection to the
+    // first validated IP so node-fetch never does a second DNS lookup.
+    const validatedAddresses = await assertSafeProxyTarget(currentUrl);
+    const agent = createPinnedAgent(currentUrl.protocol, validatedAddresses[0]);
 
     const response = await fetch(currentUrl.toString(), {
       method,
       headers,
       body,
       redirect: "manual",
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      agent: agent as any,
     });
 
     if (response.status >= 300 && response.status < 400) {

server/src/proxy-security.ts

@@ -0,0 +1,157 @@
+/**
+ * SSRF-prevention helpers for the /fetch reverse proxy.
+ *
+ * Extracted from index.ts so they can be unit-tested independently.
+ *
+ * ## DNS-rebinding / TOCTOU threat model
+ *
+ * `assertSafeProxyTarget` resolves the target hostname and validates every
+ * returned IP against the block-list.  The resolved IPs are returned to the
+ * caller so they can be passed to `createPinnedAgent`, which builds an
+ * http/https.Agent whose `lookup` callback unconditionally returns the
+ * pre-validated IP.  The agent is then passed to `node-fetch`, which calls the
+ * lookup hook instead of doing its own DNS query.
+ *
+ * Without this pinning there is a TOCTOU race: an attacker who controls the
+ * target domain can flip its DNS record to 169.254.169.254 (AWS/GCP/Azure
+ * instance-metadata) in the window between `assertSafeProxyTarget` returning
+ * and `fetch` performing its own resolution.  Pinning eliminates that window.
+ */
+
+import http from "node:http";
+import https from "node:https";
+import { isIP } from "node:net";
+import { lookup as dnsLookup } from "node:dns/promises";
+
+// ---------------------------------------------------------------------------
+// Block-list
+// ---------------------------------------------------------------------------
+
+/**
+ * Returns true if `ip` is a link-local or cloud-metadata address that must
+ * never be reachable through the proxy.
+ *
+ * Blocked ranges:
+ *  - 169.254.0.0/16  (IPv4 link-local; includes 169.254.169.254 IMDS)
+ *  - ::ffff:169.254.x.x  (IPv4-mapped IPv6 variants of the above)
+ *  - fe80::/10  (IPv6 link-local)
+ *  - fd00:ec2::254  (AWS IPv6 instance-metadata)
+ */
+export function isBlockedProxyAddress(ip: string): boolean {
+  const kind = isIP(ip);
+  if (kind === 4) {
+    const parts = ip.split(".").map(Number);
+    return parts[0] === 169 && parts[1] === 254;
+  }
+  if (kind === 6) {
+    const addr = ip.toLowerCase();
+
+    // IPv4-mapped IPv6 — two serialization forms:
+    //   dotted   ::ffff:169.254.169.254
+    //   hex      ::ffff:a9fe:a9fe
+    const mapped = addr.match(/^::ffff:(.+)$/);
+    if (mapped) {
+      const tail = mapped[1];
+      if (isIP(tail) === 4) {
+        return isBlockedProxyAddress(tail);
+      }
+      const hex = tail.match(/^([0-9a-f]{1,4}):([0-9a-f]{1,4})$/);
+      if (hex) {
+        const high = parseInt(hex[1], 16);
+        const low = parseInt(hex[2], 16);
+        return isBlockedProxyAddress(
+          `${high >> 8}.${high & 0xff}.${low >> 8}.${low & 0xff}`,
+        );
+      }
+    }
+
+    if (/^fe[89ab]/.test(addr)) return true; // fe80::/10
+    return addr === "fd00:ec2::254"; // AWS IPv6 IMDS
+  }
+  return false;
+}
+
+// ---------------------------------------------------------------------------
+// DNS validation
+// ---------------------------------------------------------------------------
+
+/** Raised when the /fetch proxy is asked to reach a disallowed target. */
+export class ProxyTargetError extends Error {}
+
+/**
+ * Resolves `parsedUrl`'s hostname and asserts that every returned IP is
+ * outside the block-list.
+ *
+ * Returns the list of validated IP strings so the caller can pin one of them
+ * into an HTTP agent (see `createPinnedAgent`), eliminating the TOCTOU window
+ * between this check and the actual `fetch()` call.
+ *
+ * @throws {ProxyTargetError} if the host cannot be resolved or any resolved
+ *   address falls inside the block-list.
+ */
+export async function assertSafeProxyTarget(
+  parsedUrl: URL,
+): Promise<string[]> {
+  const host = parsedUrl.hostname.replace(/^\[|\]$/g, ""); // strip IPv6 brackets
+
+  let addresses: string[];
+  if (isIP(host)) {
+    addresses = [host];
+  } else {
+    try {
+      addresses = (await dnsLookup(host, { all: true })).map((r) => r.address);
+    } catch {
+      throw new ProxyTargetError(`Could not resolve host: ${host}`);
+    }
+  }
+
+  if (addresses.length === 0) {
+    throw new ProxyTargetError(`Could not resolve host: ${host}`);
+  }
+
+  for (const address of addresses) {
+    if (isBlockedProxyAddress(address)) {
+      throw new ProxyTargetError(
+        `Refusing to proxy request to blocked address (${address})`,
+      );
+    }
+  }
+
+  return addresses;
+}
+
+// ---------------------------------------------------------------------------
+// IP-pinned agent factory
+// ---------------------------------------------------------------------------
+
+/**
+ * Creates an `http.Agent` or `https.Agent` whose internal `lookup` hook always
+ * returns `pinnedIp`, bypassing the OS resolver entirely.
+ *
+ * Pass this agent to `node-fetch` on every hop of `safeProxyFetch` so the TCP
+ * connection always goes to the IP that was validated by `assertSafeProxyTarget`
+ * — even if the domain's DNS record changes mid-flight.
+ *
+ * @param protocol - `"http:"` or `"https:"` (from `URL.protocol`)
+ * @param pinnedIp - A validated IPv4 or IPv6 address string
+ */
+export function createPinnedAgent(
+  protocol: string,
+  pinnedIp: string,
+): http.Agent | https.Agent {
+  const family = isIP(pinnedIp) === 6 ? 6 : 4;
+
+  // Node.js lookup signature: (hostname, options, callback)
+  const lookup = (
+    _hostname: string,
+    _opts: unknown,
+    callback: (err: Error | null, address: string, family: number) => void,
+  ): void => {
+    callback(null, pinnedIp, family);
+  };
+
+  if (protocol === "https:") {
+    return new https.Agent({ lookup } as https.AgentOptions);
+  }
+  return new http.Agent({ lookup } as http.AgentOptions);
+}

server/vitest.config.ts

@@ -0,0 +1,9 @@
+import { defineConfig } from "vitest/config";
+
+export default defineConfig({
+  test: {
+    globals: true,
+    environment: "node",
+    include: ["src/__tests__/**/*.test.ts"],
+  },
+});