From 324df7007173b9059b3d558e8ef68df398eeee69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 28 Jun 2026 15:18:03 +0200 Subject: [PATCH 1/3] 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. --- src/lib/__tests__/safe-fetch-pinned.test.ts | 181 ++++++++++++++++++++ src/lib/__tests__/safe-fetch.test.ts | 28 +-- src/lib/safe-fetch-dispatcher.ts | 38 +++- src/lib/safe-fetch.ts | 21 ++- 4 files changed, 245 insertions(+), 23 deletions(-) create mode 100644 src/lib/__tests__/safe-fetch-pinned.test.ts 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` From 3c6cb8b08df2fa941b903ee1ded9046ef4c82603 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 28 Jun 2026 15:18:12 +0200 Subject: [PATCH 2/3] fix(mcp): record why a client fails to resolve at /authorize CIMD client resolution swallowed every fetch failure as ssrf_blocked and /authorize logged a bare invalid event, so an unreachable metadata host was indistinguishable from a genuine SSRF block and the consent flow returned invalid_client with no diagnosable cause. Split the resolution failure into ssrf_blocked (a real private-host refusal) and fetch_failed (timeout / connect error), carrying the concrete SafeFetchError kind and cause as a short non-secret detail. Thread the resolution reason and detail into the authorize.invalid annotation behind a pinned meta shape, and cache a resolved CIMD document for up to 24h so the consent flow does not re-fetch on every hit. --- src/app/api/mcp/oauth/authorize/route.ts | 47 ++++++- .../mcp/oauth/__tests__/oauth-bridge.test.ts | 79 ++++++++++- src/lib/mcp/oauth/clients.ts | 126 +++++++++++++++--- 3 files changed, 228 insertions(+), 24 deletions(-) diff --git a/src/app/api/mcp/oauth/authorize/route.ts b/src/app/api/mcp/oauth/authorize/route.ts index 906d4677a..1abc2c847 100644 --- a/src/app/api/mcp/oauth/authorize/route.ts +++ b/src/app/api/mcp/oauth/authorize/route.ts @@ -118,16 +118,42 @@ interface ValidatedRequest { resource: string; } +/** + * The gate a request failed, pinned to a stable enum so the + * `mcp.oauth.authorize.invalid` annotation stays dashboard-friendly. The + * swallowed-reason gap (a bare `invalid` event) is exactly why the prod CIMD + * failure was undiagnosable. + */ +type ValidateFailure = + | "invalid_params" + | "client_unresolved" + | "redirect_mismatch" + | "bad_response_type" + | "pkce_required" + | "audience_mismatch"; + +type ValidateMeta = { + reason: ValidateFailure; + /** For `client_unresolved`: the `resolveClient` reason. */ + clientReason?: string; + /** For `client_unresolved`: a short, non-secret transport diagnostic. */ + detail?: string; +}; + /** Shared validation for GET + POST. Returns a ready error Response on failure. */ async function validate( params: Record, requestUrl: string, -): Promise<{ ok: true; v: ValidatedRequest } | { ok: false; res: Response }> { +): Promise< + | { ok: true; v: ValidatedRequest } + | { ok: false; res: Response; meta: ValidateMeta } +> { const parsed = z.object(baseParams).safeParse(params); if (!parsed.success) { return { ok: false, res: badRequest("invalid_request", "Missing or malformed parameters"), + meta: { reason: "invalid_params" }, }; } const p = parsed.data; @@ -139,6 +165,11 @@ async function validate( return { ok: false, res: badRequest("invalid_client", "Unknown or unresolvable client"), + meta: { + reason: "client_unresolved", + clientReason: resolved.reason, + ...(resolved.detail ? { detail: resolved.detail } : {}), + }, }; } @@ -150,6 +181,7 @@ async function validate( "invalid_request", "redirect_uri does not match the registered client", ), + meta: { reason: "redirect_mismatch" }, }; } @@ -161,6 +193,7 @@ async function validate( "unsupported_response_type", "Only response_type=code is supported", ), + meta: { reason: "bad_response_type" }, }; } @@ -175,6 +208,7 @@ async function validate( "invalid_request", "PKCE with code_challenge_method=S256 is required", ), + meta: { reason: "pkce_required" }, }; } @@ -186,6 +220,7 @@ async function validate( "invalid_target", "resource must equal this server's MCP endpoint", ), + meta: { reason: "audience_mismatch" }, }; } @@ -280,7 +315,10 @@ export async function GET(request: NextRequest): Promise { const params = Object.fromEntries(url.searchParams.entries()); const result = await validate(params, request.url); if (!result.ok) { - annotate({ action: { name: "mcp.oauth.authorize.invalid" } }); + annotate({ + action: { name: "mcp.oauth.authorize.invalid" }, + meta: result.meta, + }); return result.res; } const { v } = result; @@ -409,7 +447,10 @@ export async function POST(request: NextRequest): Promise { const result = await validate(params, request.url); if (!result.ok) { - annotate({ action: { name: "mcp.oauth.authorize.invalid" } }); + annotate({ + action: { name: "mcp.oauth.authorize.invalid" }, + meta: result.meta, + }); return result.res; } const { v } = result; diff --git a/src/lib/mcp/oauth/__tests__/oauth-bridge.test.ts b/src/lib/mcp/oauth/__tests__/oauth-bridge.test.ts index 29b6a6a99..4e952589b 100644 --- a/src/lib/mcp/oauth/__tests__/oauth-bridge.test.ts +++ b/src/lib/mcp/oauth/__tests__/oauth-bridge.test.ts @@ -8,18 +8,31 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; process.env.API_TOKEN_HMAC_KEY = "x".repeat(48); process.env.APP_URL = "https://health.example"; +// Mirror the real SafeFetchError shape (message + kind + cause) so the +// `instanceof` branch in `resolveCimdClient` exercises the kind→reason mapping. +type SafeFetchErrorKind = "private_host" | "timeout" | "network"; vi.mock("@/lib/safe-fetch", () => ({ safeFetch: vi.fn(), - SafeFetchError: class SafeFetchError extends Error {}, + SafeFetchError: class SafeFetchError extends Error { + readonly kind: SafeFetchErrorKind; + readonly cause?: unknown; + constructor(message: string, kind: SafeFetchErrorKind, cause?: unknown) { + super(message); + this.name = "SafeFetchError"; + this.kind = kind; + if (cause !== undefined) this.cause = cause; + } + }, })); -import { safeFetch } from "@/lib/safe-fetch"; +import { safeFetch, SafeFetchError } from "@/lib/safe-fetch"; import { signArtifact, verifyArtifact } from "../artifacts"; import { s256Challenge, verifyPkceS256, isValidVerifier } from "../pkce"; import { registerDcrClient, resolveClient, redirectUriAllowed, + _resetCimdCacheForTests, } from "../clients"; import { protectedResourceMetadata, @@ -105,7 +118,10 @@ describe("clients — DCR (stateless) round-trip", () => { }); describe("clients — CIMD via safeFetch (SSRF-safe)", () => { - beforeEach(() => vi.clearAllMocks()); + beforeEach(() => { + vi.clearAllMocks(); + _resetCimdCacheForTests(); + }); it("fetches a CIMD document through safeFetch with requirePublicHost", async () => { const clientId = "https://app.example/mcp-client.json"; @@ -144,10 +160,61 @@ describe("clients — CIMD via safeFetch (SSRF-safe)", () => { expect(res).toEqual({ ok: false, reason: "invalid_metadata" }); }); - it("surfaces an SSRF block as ssrf_blocked", async () => { - vi.mocked(safeFetch).mockRejectedValue(new Error("private host")); + it("surfaces a private-host SSRF block as ssrf_blocked", async () => { + vi.mocked(safeFetch).mockRejectedValue( + new SafeFetchError("private host", "private_host"), + ); const res = await resolveClient("https://169.254.169.254/meta.json"); - expect(res).toEqual({ ok: false, reason: "ssrf_blocked" }); + expect(res).toMatchObject({ ok: false, reason: "ssrf_blocked" }); + }); + + it("surfaces an unreachable host as fetch_failed (not ssrf_blocked)", async () => { + // The prod CIMD outage: a transport failure (no IPv6 route) that was + // mislabelled `ssrf_blocked`. It must now resolve to `fetch_failed` and + // carry the concrete kind + cause for the `/authorize` annotation. + vi.mocked(safeFetch).mockRejectedValue( + new SafeFetchError( + "safeFetch network error: connect ENETUNREACH", + "network", + new Error("connect ENETUNREACH 2607:6bc0::1:443"), + ), + ); + const res = await resolveClient("https://claude.ai/.well-known/mcp.json"); + expect(res.ok).toBe(false); + if (!res.ok) { + expect(res.reason).toBe("fetch_failed"); + expect(res.detail).toContain("network"); + expect(res.detail).toContain("ENETUNREACH"); + } + }); + + it("surfaces a timeout as fetch_failed", async () => { + vi.mocked(safeFetch).mockRejectedValue( + new SafeFetchError("safeFetch aborted (TimeoutError)", "timeout"), + ); + const res = await resolveClient("https://slow.example/mcp.json"); + expect(res).toMatchObject({ ok: false, reason: "fetch_failed" }); + }); + + it("caches a resolved CIMD document and does not re-fetch within the TTL", async () => { + const clientId = "https://app.example/mcp-client.json"; + vi.mocked(safeFetch).mockResolvedValue( + new Response( + JSON.stringify({ + client_id: clientId, + client_name: "Cached Client", + redirect_uris: ["https://app.example/cb"], + }), + { status: 200 }, + ), + ); + + const first = await resolveClient(clientId); + const second = await resolveClient(clientId); + expect(first.ok).toBe(true); + expect(second.ok).toBe(true); + // Only the first call hits the network; the second is served from cache. + expect(vi.mocked(safeFetch)).toHaveBeenCalledTimes(1); }); it("rejects a CIMD doc with a non-https / non-loopback redirect (L2)", async () => { diff --git a/src/lib/mcp/oauth/clients.ts b/src/lib/mcp/oauth/clients.ts index 01717c1c1..e6060ddfb 100644 --- a/src/lib/mcp/oauth/clients.ts +++ b/src/lib/mcp/oauth/clients.ts @@ -22,7 +22,7 @@ * `https://claude.ai/api/mcp/auth_callback` hosted callback is matched as an * ordinary registered entry. */ -import { safeFetch } from "@/lib/safe-fetch"; +import { safeFetch, SafeFetchError } from "@/lib/safe-fetch"; import { readBodyCapped } from "@/lib/http/read-capped"; import { ARTIFACT_KINDS, signArtifact, verifyArtifact } from "./artifacts"; @@ -35,16 +35,82 @@ export interface ResolvedClient { source: "cimd" | "dcr"; } +/** + * Why a `client_id` failed to resolve. + * + * - `unknown_client` — not a CIMD URL / DCR id, or the document 404'd. + * - `invalid_metadata` — fetched, but the document is malformed / oversized / + * fails a SEP-991 constraint. + * - `ssrf_blocked` — the SSRF floor refused the host (private / metadata + * range): a genuine policy block. + * - `fetch_failed` — the metadata host could not be reached (timeout, + * connect error, version skew). This is NOT an SSRF + * block; conflating the two is exactly what made the + * prod CIMD outage undiagnosable — every transport + * failure surfaced as `ssrf_blocked / invalid_client`. + * + * `detail` carries a short, non-secret diagnostic string (the + * `SafeFetchError.kind` + message + cause) for the `/authorize` annotation. + */ +export type ClientResolutionReason = + | "unknown_client" + | "invalid_metadata" + | "ssrf_blocked" + | "fetch_failed"; + export type ClientResolution = | { ok: true; client: ResolvedClient } | { ok: false; - reason: "unknown_client" | "invalid_metadata" | "ssrf_blocked"; + reason: ClientResolutionReason; + detail?: string; }; const MAX_REDIRECT_URIS = 12; const CIMD_MAX_BYTES = 16 * 1024; +/** + * SEP-991 recommends caching a resolved CIMD document (≤24h). Re-resolving on + * every `/authorize` hit would re-pay the outbound fetch and widen the SSRF + * surface needlessly; a short cache keeps the consent flow snappy without + * letting a stale document linger. Only SUCCESSFUL resolutions are cached — + * failures must re-try so a transient outage self-heals. + */ +const CIMD_CACHE_TTL_MS = 24 * 60 * 60 * 1000; +const CIMD_CACHE_MAX_ENTRIES = 256; +const cimdCache = new Map< + string, + { client: ResolvedClient; expiresAt: number } +>(); + +function getCachedCimd(clientId: string): ResolvedClient | null { + const hit = cimdCache.get(clientId); + if (!hit) return null; + if (hit.expiresAt <= Date.now()) { + cimdCache.delete(clientId); + return null; + } + return hit.client; +} + +function putCachedCimd(clientId: string, client: ResolvedClient): void { + // Cheap bound: a flood of distinct attacker URLs cannot grow the map without + // limit. When full, drop the oldest insertion (Map preserves insertion order). + if (cimdCache.size >= CIMD_CACHE_MAX_ENTRIES) { + const oldest = cimdCache.keys().next().value; + if (oldest !== undefined) cimdCache.delete(oldest); + } + cimdCache.set(clientId, { + client, + expiresAt: Date.now() + CIMD_CACHE_TTL_MS, + }); +} + +/** Test helper — clears the resolved-CIMD cache between cases. */ +export function _resetCimdCacheForTests(): void { + cimdCache.clear(); +} + function asStringArray(value: unknown): string[] | null { if (!Array.isArray(value)) return null; const out: string[] = []; @@ -142,6 +208,9 @@ function resolveDcrClient(clientId: string): ClientResolution { } async function resolveCimdClient(clientId: string): Promise { + const cached = getCachedCimd(clientId); + if (cached) return { ok: true, client: cached }; + let res: Response; try { // SSRF floor: the metadata host is fully user-controlled, so pin the @@ -151,10 +220,38 @@ async function resolveCimdClient(clientId: string): Promise { { method: "GET", headers: { accept: "application/json" } }, { requirePublicHost: true, timeoutMs: 8000 }, ); - } catch { - return { ok: false, reason: "ssrf_blocked" }; + } catch (err) { + // Distinguish a genuine SSRF policy block from an unreachable host. The + // prod CIMD outage was a transport failure (no IPv6 route to claude.ai) + // mislabelled `ssrf_blocked`; carry the concrete kind + cause so the + // `/authorize` annotation records WHY, instead of a blanket label. + if (err instanceof SafeFetchError) { + const causeMsg = + err.cause instanceof Error + ? err.cause.message + : err.cause !== undefined + ? String(err.cause) + : undefined; + const detail = causeMsg ? `${err.kind}: ${causeMsg}` : err.kind; + return { + ok: false, + reason: err.kind === "private_host" ? "ssrf_blocked" : "fetch_failed", + detail, + }; + } + return { + ok: false, + reason: "fetch_failed", + detail: err instanceof Error ? err.message : String(err), + }; + } + if (!res.ok) { + return { + ok: false, + reason: "unknown_client", + detail: `http_${res.status}`, + }; } - if (!res.ok) return { ok: false, reason: "unknown_client" }; // M3 — enforce the byte cap WHILE reading: reject up front on an oversized // Content-Length and abort the stream the moment it overflows, so a hostile @@ -181,17 +278,16 @@ async function resolveCimdClient(clientId: string): Promise { return { ok: false, reason: "invalid_metadata" }; } - return { - ok: true, - client: { - clientId, - clientName: - typeof doc.client_name === "string" ? doc.client_name : "MCP client", - redirectUris, - isPublic: true, - source: "cimd", - }, + const client: ResolvedClient = { + clientId, + clientName: + typeof doc.client_name === "string" ? doc.client_name : "MCP client", + redirectUris, + isPublic: true, + source: "cimd", }; + putCachedCimd(clientId, client); + return { ok: true, client }; } /** From 6c27e053b7c318673d6f718428b683b3d59fbd04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 28 Jun 2026 15:18:20 +0200 Subject: [PATCH 3/3] test: route the global fetch stub onto the undici pinned engine The requirePublicHost egress paths now dispatch through undici's own fetch, so the existing global fetch stubs in the AI, GeoIP, moodlog, send, and ntfy suites no longer intercept them. Delegate undici's fetch back to the installed global stub so each suite keeps asserting its request shape and canned responses without exercising the network. --- .../moodlog/test/__tests__/route.test.ts | 15 +++++++++++++++ src/app/api/send/__tests__/route.test.ts | 15 +++++++++++++++ src/lib/__tests__/geo-asn.test.ts | 15 +++++++++++++++ src/lib/__tests__/geo.test.ts | 15 +++++++++++++++ src/lib/ai/__tests__/anthropic-client.test.ts | 15 +++++++++++++++ src/lib/ai/__tests__/openai-client.test.ts | 15 +++++++++++++++ src/lib/ai/__tests__/provider-contract.test.ts | 15 +++++++++++++++ src/lib/moodlog/__tests__/push.test.ts | 15 +++++++++++++++ .../senders/__tests__/push-attempt-record.test.ts | 15 +++++++++++++++ 9 files changed, 135 insertions(+) diff --git a/src/app/api/integrations/moodlog/test/__tests__/route.test.ts b/src/app/api/integrations/moodlog/test/__tests__/route.test.ts index b2a77a689..33ac0c7e5 100644 --- a/src/app/api/integrations/moodlog/test/__tests__/route.test.ts +++ b/src/app/api/integrations/moodlog/test/__tests__/route.test.ts @@ -1,5 +1,20 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +// safeFetch's requirePublicHost path runs through undici's own `fetch` +// (version-locked with its dispatcher). Delegate it to the global `fetch` +// stub these tests install so the existing interception still applies. +vi.mock("undici", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + fetch: (input: unknown, init?: unknown) => + (globalThis.fetch as unknown as (i: unknown, n?: unknown) => unknown)( + input, + init, + ), + }; +}); + vi.mock("@/lib/api-handler", () => ({ apiHandler: unknown>(fn: T) => fn, requireAuth: vi.fn(async () => ({ diff --git a/src/app/api/send/__tests__/route.test.ts b/src/app/api/send/__tests__/route.test.ts index 8ed6df78a..874cca183 100644 --- a/src/app/api/send/__tests__/route.test.ts +++ b/src/app/api/send/__tests__/route.test.ts @@ -1,6 +1,21 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { NextRequest } from "next/server"; +// safeFetch's requirePublicHost path runs through undici's own `fetch` +// (version-locked with its dispatcher). Delegate it to the global `fetch` +// stub these tests install so the existing interception still applies. +vi.mock("undici", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + fetch: (input: unknown, init?: unknown) => + (globalThis.fetch as unknown as (i: unknown, n?: unknown) => unknown)( + input, + init, + ), + }; +}); + vi.mock("@/lib/api-handler", () => ({ apiHandler: unknown>(fn: T) => fn, })); diff --git a/src/lib/__tests__/geo-asn.test.ts b/src/lib/__tests__/geo-asn.test.ts index 6a59dfc7b..f10903253 100644 --- a/src/lib/__tests__/geo-asn.test.ts +++ b/src/lib/__tests__/geo-asn.test.ts @@ -1,5 +1,20 @@ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +// safeFetch's requirePublicHost path runs through undici's own `fetch` +// (version-locked with its dispatcher). Delegate it to the global `fetch` +// stub these tests install so the existing interception still applies. +vi.mock("undici", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + fetch: (input: unknown, init?: unknown) => + (globalThis.fetch as unknown as (i: unknown, n?: unknown) => unknown)( + input, + init, + ), + }; +}); + /** * v1.4.27 B3 — ASN + carrier lookup tests. * diff --git a/src/lib/__tests__/geo.test.ts b/src/lib/__tests__/geo.test.ts index dd231e4fb..63af5bb27 100644 --- a/src/lib/__tests__/geo.test.ts +++ b/src/lib/__tests__/geo.test.ts @@ -1,5 +1,20 @@ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +// safeFetch's requirePublicHost path runs through undici's own `fetch` +// (version-locked with its dispatcher). Delegate it to the global `fetch` +// stub these tests install so the existing interception still applies. +vi.mock("undici", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + fetch: (input: unknown, init?: unknown) => + (globalThis.fetch as unknown as (i: unknown, n?: unknown) => unknown)( + input, + init, + ), + }; +}); + // v1.18.11 (W3): capture wide-event warnings so the non-ok-status path can // be asserted. `getEvent()` returns null outside a request context, so // without this mock the `getEvent()?.addWarning(...)` call is a silent diff --git a/src/lib/ai/__tests__/anthropic-client.test.ts b/src/lib/ai/__tests__/anthropic-client.test.ts index fe637eabe..63bfa5e73 100644 --- a/src/lib/ai/__tests__/anthropic-client.test.ts +++ b/src/lib/ai/__tests__/anthropic-client.test.ts @@ -2,6 +2,21 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { AnthropicClient } from "../anthropic-client"; import { singleUserTurn } from "../types"; +// safeFetch's requirePublicHost path runs through undici's own `fetch` +// (version-locked with its dispatcher). Delegate it to the global `fetch` +// stub these tests install so the existing interception still applies. +vi.mock("undici", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + fetch: (input: unknown, init?: unknown) => + (globalThis.fetch as unknown as (i: unknown, n?: unknown) => unknown)( + input, + init, + ), + }; +}); + describe("AnthropicClient", () => { beforeEach(() => { vi.restoreAllMocks(); diff --git a/src/lib/ai/__tests__/openai-client.test.ts b/src/lib/ai/__tests__/openai-client.test.ts index 2a94eed33..70bbcf647 100644 --- a/src/lib/ai/__tests__/openai-client.test.ts +++ b/src/lib/ai/__tests__/openai-client.test.ts @@ -2,6 +2,21 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { OpenAIClient } from "../openai-client"; import { singleUserTurn } from "../types"; +// safeFetch's requirePublicHost path runs through undici's own `fetch` +// (version-locked with its dispatcher). Delegate it to the global `fetch` +// stub these tests install so the existing interception still applies. +vi.mock("undici", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + fetch: (input: unknown, init?: unknown) => + (globalThis.fetch as unknown as (i: unknown, n?: unknown) => unknown)( + input, + init, + ), + }; +}); + describe("OpenAIClient", () => { beforeEach(() => { vi.restoreAllMocks(); diff --git a/src/lib/ai/__tests__/provider-contract.test.ts b/src/lib/ai/__tests__/provider-contract.test.ts index 7b9190b4b..39ed56e34 100644 --- a/src/lib/ai/__tests__/provider-contract.test.ts +++ b/src/lib/ai/__tests__/provider-contract.test.ts @@ -10,6 +10,21 @@ import { type CompletionResult, } from "../types"; +// safeFetch's requirePublicHost path runs through undici's own `fetch` +// (version-locked with its dispatcher). Delegate it to the global `fetch` +// stub these tests install so the existing interception still applies. +vi.mock("undici", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + fetch: (input: unknown, init?: unknown) => + (globalThis.fetch as unknown as (i: unknown, n?: unknown) => unknown)( + input, + init, + ), + }; +}); + /** * Provider-contract tests: every concrete AIProvider implementation must * satisfy the same shape — return a `CompletionResult` with the four diff --git a/src/lib/moodlog/__tests__/push.test.ts b/src/lib/moodlog/__tests__/push.test.ts index 28e22446a..3c150e981 100644 --- a/src/lib/moodlog/__tests__/push.test.ts +++ b/src/lib/moodlog/__tests__/push.test.ts @@ -9,6 +9,21 @@ */ import { describe, it, expect, beforeEach, vi } from "vitest"; +// safeFetch's requirePublicHost path runs through undici's own `fetch` +// (version-locked with its dispatcher). Delegate it to the global `fetch` +// stub these tests install so the existing interception still applies. +vi.mock("undici", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + fetch: (input: unknown, init?: unknown) => + (globalThis.fetch as unknown as (i: unknown, n?: unknown) => unknown)( + input, + init, + ), + }; +}); + vi.mock("@/lib/db", () => ({ prisma: { user: { findUnique: vi.fn() } }, })); diff --git a/src/lib/notifications/senders/__tests__/push-attempt-record.test.ts b/src/lib/notifications/senders/__tests__/push-attempt-record.test.ts index 0c9a339e3..b2e5b7cb8 100644 --- a/src/lib/notifications/senders/__tests__/push-attempt-record.test.ts +++ b/src/lib/notifications/senders/__tests__/push-attempt-record.test.ts @@ -16,6 +16,21 @@ */ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +// safeFetch's requirePublicHost path runs through undici's own `fetch` +// (version-locked with its dispatcher). Delegate it to the global `fetch` +// stub these tests install so the existing interception still applies. +vi.mock("undici", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + fetch: (input: unknown, init?: unknown) => + (globalThis.fetch as unknown as (i: unknown, n?: unknown) => unknown)( + input, + init, + ), + }; +}); + // vi.mock factories run before module top-level executes, so anything // referenced inside a factory must be created via vi.hoisted to share // the same hoisted scope.