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/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/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/__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/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/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 }; } /** 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. 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`