diff --git a/src/lib/__tests__/safe-fetch-pinned.test.ts b/src/lib/__tests__/safe-fetch-pinned.test.ts new file mode 100644 index 000000000..d94322e7c --- /dev/null +++ b/src/lib/__tests__/safe-fetch-pinned.test.ts @@ -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(); + 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((resolve) => + server.listen(0, "127.0.0.1", () => resolve()), + ); + port = (server.address() as AddressInfo).port; +}); + +afterAll(async () => { + await new Promise((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); +}); diff --git a/src/lib/__tests__/safe-fetch.test.ts b/src/lib/__tests__/safe-fetch.test.ts index cd9973c3f..c75585177 100644 --- a/src/lib/__tests__/safe-fetch.test.ts +++ b/src/lib/__tests__/safe-fetch.test.ts @@ -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")); diff --git a/src/lib/safe-fetch-dispatcher.ts b/src/lib/safe-fetch-dispatcher.ts index 1919674cb..184c242d0 100644 --- a/src/lib/safe-fetch-dispatcher.ts +++ b/src/lib/safe-fetch-dispatcher.ts @@ -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; diff --git a/src/lib/safe-fetch.ts b/src/lib/safe-fetch.ts index af06ec094..e6718c312 100644 --- a/src/lib/safe-fetch.ts +++ b/src/lib/safe-fetch.ts @@ -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[1]); + return res as unknown as Response; + } return await fetch(target, finalInit); } catch (err) { // `AbortSignal.timeout` aborts with `TimeoutError` (`DOMException`