fix(egress): vet all addresses and version-lock the pinned fetch

The connect-time SSRF pin returned only the first vetted address and
attached an undici@8 Agent to Node's built-in fetch. Two failures:

- A host whose first DNS record is an unreachable IPv6 (no IPv6 route on
  the container / LAN / Tailscale) failed the connect instantly, because
  the single-address pin discarded the working IPv4 record behind it and
  Happy Eyeballs never ran.
- Node's global fetch is backed by its own pinned undici copy; handing it
  an Agent built from the project's undici throws UND_ERR_INVALID_ARG on
  a dispatch-handler interface mismatch.

Hand undici the full vetted survivor set and enable autoSelectFamily so
it runs RFC 8305 Happy Eyeballs across the already-vetted IPs (rebinding
stays closed: every dialable IP passed isPublicIp in the same lookup).
Drive the pinned path through undici's own fetch so the dispatcher and
the fetch engine share one undici version.

Add an end-to-end regression that drives a real localhost-bound server
through the pinned path (not a mocked fetch, which hid the coupling),
covering the IPv6-to-IPv4 fallback, streaming, timeout, and abort.
This commit is contained in:
Marc-André Bombeck committed 2026-06-28 15:18:03 +02:00
1 parent c0a91dbbcd
commit 324df70071
4 files changed
+245 -23

No files matched your search

+181
View File
@@ -0,0 +1,181 @@
/**
* End-to-end regression for the pinned (`requirePublicHost: true`) egress path.
*
* This is deliberately NOT a mocked-fetch test. The production bug it guards
* against is a coupling between the fetch ENGINE and the undici dispatcher:
*
* 1. Version skew — handing an `undici@8` `Agent` to Node's built-in global
* `fetch` (backed by a different, internal undici copy) throws
* `UND_ERR_INVALID_ARG`. A mocked `fetch` would never surface this; only a
* real dispatch through the real fetch engine does. The fix routes the
* pinned path through undici's OWN `fetch`.
* 2. Single-address pin — returning only the first resolved address made a
* host whose first record is an unreachable IPv6 fail instantly. The fix
* hands undici the full vetted set + enables Happy Eyeballs.
*
* To exercise the real transport against a localhost-bound server we stub the
* two IP-classification predicates (so loopback is treated as dialable for the
* harness) and `dns.lookup` (so the test hostname maps to the test server).
* The dispatcher, the undici fetch engine, the connect, and the server are all
* REAL — which is exactly what makes this catch the coupling bug.
*/
import {
afterAll,
afterEach,
beforeAll,
describe,
expect,
it,
vi,
} from "vitest";
import http from "node:http";
import dns from "node:dns";
import type { AddressInfo } from "node:net";
// Treat every address/URL as public so the harness can reach a loopback server
// through the requirePublicHost guard. The transport coupling under test is
// orthogonal to IP classification (covered by safe-fetch-dispatcher.test.ts).
vi.mock("@/lib/validations/notifications", async (importOriginal) => {
const actual =
await importOriginal<typeof import("@/lib/validations/notifications")>();
return {
...actual,
isPublicIp: () => true,
isPublicUrl: () => true,
};
});
import { safeFetch, SafeFetchError } from "../safe-fetch";
import { _resetPinnedDispatcherForTests } from "../safe-fetch-dispatcher";
let server: http.Server;
let port: number;
function mockLookupTo(addresses: dns.LookupAddress[]): void {
vi.spyOn(dns, "lookup").mockImplementation(((
_hostname: string,
_opts: dns.LookupAllOptions,
callback: (
err: NodeJS.ErrnoException | null,
addrs: dns.LookupAddress[],
) => void,
) => {
callback(null, addresses);
}) as unknown as typeof dns.lookup);
}
beforeAll(async () => {
server = http.createServer((req, res) => {
if (req.url === "/stream") {
res.writeHead(200, { "content-type": "text/plain" });
res.write("chunk-1;");
setTimeout(() => {
res.write("chunk-2;");
res.end("chunk-3");
}, 10);
return;
}
if (req.url === "/hang") {
// Never respond — drives the timeout path.
return;
}
res.writeHead(200, { "content-type": "application/json" });
res.end(JSON.stringify({ ok: true, path: req.url }));
});
await new Promise<void>((resolve) =>
server.listen(0, "127.0.0.1", () => resolve()),
);
port = (server.address() as AddressInfo).port;
});
afterAll(async () => {
await new Promise<void>((resolve) => server.close(() => resolve()));
});
afterEach(() => {
vi.restoreAllMocks();
_resetPinnedDispatcherForTests();
});
describe("safeFetch pinned path — real transport", () => {
it("returns 200 end-to-end through the pinned dispatcher + undici fetch", async () => {
mockLookupTo([{ address: "127.0.0.1", family: 4 }]);
const res = await safeFetch(
`http://pinned.test:${port}/api`,
{},
{ requirePublicHost: true, timeoutMs: 5000 },
);
expect(res.status).toBe(200);
const body = (await res.json()) as { ok: boolean };
expect(body.ok).toBe(true);
}, 20_000);
it("falls back across families when the first address is a black-holed IPv6", async () => {
// First record is an unreachable IPv6 (RFC 6666 discard prefix); the
// working IPv4 record is behind it. Pre-fix this failed instantly; with
// Happy Eyeballs the connect falls back to v4 and succeeds.
mockLookupTo([
{ address: "0100::1", family: 6 },
{ address: "127.0.0.1", family: 4 },
]);
const res = await safeFetch(
`http://dualstack.test:${port}/api`,
{},
{ requirePublicHost: true, timeoutMs: 5000 },
);
expect(res.status).toBe(200);
}, 20_000);
it("streams a chunked response body via getReader() on the pinned path", async () => {
mockLookupTo([{ address: "127.0.0.1", family: 4 }]);
const res = await safeFetch(
`http://pinned.test:${port}/stream`,
{},
{ requirePublicHost: true, timeoutMs: 5000 },
);
expect(res.status).toBe(200);
expect(res.body).not.toBeNull();
const reader = res.body!.getReader();
const decoder = new TextDecoder();
let out = "";
for (;;) {
const { done, value } = await reader.read();
if (done) break;
if (value) out += decoder.decode(value, { stream: true });
}
expect(out).toBe("chunk-1;chunk-2;chunk-3");
}, 20_000);
it("times out on the pinned path and maps to SafeFetchError kind 'timeout'", async () => {
mockLookupTo([{ address: "127.0.0.1", family: 4 }]);
await expect(
safeFetch(
`http://pinned.test:${port}/hang`,
{},
{ requirePublicHost: true, timeoutMs: 200 },
),
).rejects.toMatchObject({ kind: "timeout" });
}, 20_000);
it("honours a caller-supplied abort signal on the pinned path", async () => {
mockLookupTo([{ address: "127.0.0.1", family: 4 }]);
const controller = new AbortController();
const pending = safeFetch(
`http://pinned.test:${port}/hang`,
{ signal: controller.signal },
{ requirePublicHost: true, timeoutMs: 5000 },
);
controller.abort();
await expect(pending).rejects.toBeInstanceOf(SafeFetchError);
await expect(pending).rejects.toMatchObject({ kind: "timeout" });
}, 20_000);
});
+16 -12
View File
@@ -102,18 +102,22 @@ describe("safeFetch", () => {
expect(fetchSpy).not.toHaveBeenCalled();
});
it("passes a public host through with requirePublicHost", async () => {
fetchSpy.mockResolvedValueOnce(new Response("ok", { status: 200 }));
const res = await safeFetch(
"https://example.com/api",
{},
{ requirePublicHost: true },
);
expect(res.status).toBe(200);
expect(fetchSpy).toHaveBeenCalledOnce();
});
it("does NOT use global fetch on the pinned (requirePublicHost) path", async () => {
// The pinned path must go through undici's own `fetch` so the dispatcher
// and the fetch engine share one undici version (the version-skew fix).
// Global `fetch` must therefore be bypassed entirely. The end-to-end 200
// against a real server is asserted in `safe-fetch-pinned.test.ts`.
await expect(
safeFetch(
// 192.0.2.0/24 is TEST-NET-1 (public-classified, unroutable) so the
// connect fails fast without touching global fetch.
"https://192.0.2.1/api",
{ signal: AbortSignal.timeout(300) },
{ requirePublicHost: true },
),
).rejects.toBeInstanceOf(SafeFetchError);
expect(fetchSpy).not.toHaveBeenCalled();
}, 20_000);
it("wraps a native network error as kind: 'network'", async () => {
fetchSpy.mockRejectedValueOnce(new TypeError("network down"));
+30 -8
View File
@@ -5,7 +5,7 @@ import { isPublicIp } from "@/lib/validations/notifications";
/**
* Custom `undici.Agent` that resolves the request hostname literally,
* vets every returned address against `isPublicIp`, and pins the
* connection to the first allowed address.
* connection to the vetted survivor set.
*
* This closes the DNS-rebinding gap that `isPublicUrl` cannot reach
* (issue #217). The input-time check accepts a hostname that resolves
@@ -22,16 +22,23 @@ import { isPublicIp } from "@/lib/validations/notifications";
* 2. Filter that list through `isPublicIp` — same allowlist
* `isPublicUrl` enforces, applied to a literal IP address rather
* than the input string.
* 3. If at least one address passes, hand the single first survivor
* to undici. Undici then pins the TCP connect to that exact IP,
* side-stepping any further resolver hop.
* 3. Hand undici the FULL vetted survivor set, not just the first
* address. The `Agent` runs with `autoSelectFamily`, so undici's
* connector calls this lookup with `all: true` and then performs
* RFC 8305 Happy Eyeballs across the survivors. Returning only the
* first address — historically `allowed[0]` — defeated that: a host
* whose first record is an unreachable IPv6 (common on a no-IPv6
* container / LAN / Tailscale path, e.g. `claude.ai` from a v4-only
* box) failed the connect instantly even though a working IPv4
* record was right behind it. Every IP undici may dial was vetted in
* THIS lookup, so DNS rebinding stays closed while v4/v6 fallback
* works.
* 4. If no address passes, fail closed with `dns.NOTFOUND` so the
* dispatch surfaces as a normal connect error rather than a silent
* bypass.
*
* Wired into `safeFetch` only when `opts.requirePublicHost` is true.
* The five outbound paths that accept a user-supplied host all use
* that flag.
* The outbound paths that accept a user-supplied host all use that flag.
*/
function pinnedLookup(
hostname: string,
@@ -67,12 +74,17 @@ function pinnedLookup(
return;
}
const picked = allowed[0];
if (options.all) {
// Caller asked for the array shape — pass through the survivors.
// Caller asked for the array shape — pass through EVERY survivor so
// undici's Happy-Eyeballs loop can fall back across families. With
// `autoSelectFamily` enabled (below) the connector always takes
// this branch.
callback(null, allowed);
return;
}
// Single-address contract (no `autoSelectFamily`, or a caller that
// explicitly asked for one address): hand back the first survivor.
const picked = allowed[0];
callback(null, picked.address, picked.family);
});
}
@@ -90,6 +102,16 @@ export function getPinnedPublicDispatcher(): Agent {
cached = new Agent({
connect: {
lookup: pinnedLookup,
// RFC 8305 Happy Eyeballs across the vetted survivor set. Without
// this undici pins the single first address and dies when that is
// an unreachable IPv6 record on a v4-only host. With it, undici
// calls `pinnedLookup` with `all: true` and races the vetted v4/v6
// candidates, taking the first that connects.
autoSelectFamily: true,
// How long to wait for one family before trying the next. 250 ms is
// the documented sane default — long enough not to thrash on a slow
// RTT, short enough that a black-holed family falls back quickly.
autoSelectFamilyAttemptTimeout: 250,
},
});
return cached;
+18 -3
View File
@@ -1,3 +1,4 @@
import { fetch as undiciFetch } from "undici";
import { isPublicUrl } from "@/lib/validations/notifications";
import { getPinnedPublicDispatcher } from "@/lib/safe-fetch-dispatcher";
@@ -119,19 +120,33 @@ export async function safeFetch(
// When the caller asks for the public-host guard, also route through
// the pinned-IP dispatcher so a DNS rebinding cannot flip the host
// between the input-time `isPublicUrl` accept and the connect call.
// Node's `fetch` accepts an undici `dispatcher` on RequestInit; the
// DOM types don't reflect that, so we cast at the boundary.
const dispatcher = opts.requirePublicHost
? getPinnedPublicDispatcher()
: undefined;
const finalInit = {
...init,
redirect,
signal,
...(dispatcher ? { dispatcher } : {}),
} as RequestInit;
try {
if (dispatcher) {
// Drive the pinned dispatcher through undici's OWN `fetch`, not the
// global one. Node's built-in `fetch` is backed by an internal,
// version-pinned copy of undici; handing it an `Agent` built from
// the project's `undici@8` throws `UND_ERR_INVALID_ARG` on a
// dispatch-handler interface mismatch (Node 25 locally; the skew is
// by construction, not environment luck). Using undici's own fetch
// locks the dispatcher and the fetch engine to one undici version.
// The cast bridges undici's RequestInit (which carries `dispatcher`)
// and its Response back to the DOM types this module exposes.
const res = await undiciFetch(target, {
...finalInit,
dispatcher,
} as Parameters<typeof undiciFetch>[1]);
return res as unknown as Response;
}
return await fetch(target, finalInit);
} catch (err) {
// `AbortSignal.timeout` aborts with `TimeoutError` (`DOMException`