mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 12:32:43 +00:00
Merge branch 'fix/v125-mcp-egress' into release/v1.25.0
This commit is contained in:
commit
3a84a07423
16 files changed
+608
-47
No files matched your search
@@ -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<typeof import("undici")>();
|
||||
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: <T extends (...args: unknown[]) => unknown>(fn: T) => fn,
|
||||
requireAuth: vi.fn(async () => ({
|
||||
|
||||
@@ -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<string, string | undefined>,
|
||||
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<Response> {
|
||||
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<Response> {
|
||||
|
||||
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;
|
||||
|
||||
@@ -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<typeof import("undici")>();
|
||||
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: <T extends (...args: unknown[]) => unknown>(fn: T) => fn,
|
||||
}));
|
||||
|
||||
@@ -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<typeof import("undici")>();
|
||||
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.
|
||||
*
|
||||
|
||||
@@ -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<typeof import("undici")>();
|
||||
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
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
@@ -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"));
|
||||
|
||||
@@ -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<typeof import("undici")>();
|
||||
return {
|
||||
...actual,
|
||||
fetch: (input: unknown, init?: unknown) =>
|
||||
(globalThis.fetch as unknown as (i: unknown, n?: unknown) => unknown)(
|
||||
input,
|
||||
init,
|
||||
),
|
||||
};
|
||||
});
|
||||
|
||||
describe("AnthropicClient", () => {
|
||||
beforeEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
|
||||
@@ -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<typeof import("undici")>();
|
||||
return {
|
||||
...actual,
|
||||
fetch: (input: unknown, init?: unknown) =>
|
||||
(globalThis.fetch as unknown as (i: unknown, n?: unknown) => unknown)(
|
||||
input,
|
||||
init,
|
||||
),
|
||||
};
|
||||
});
|
||||
|
||||
describe("OpenAIClient", () => {
|
||||
beforeEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
|
||||
@@ -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<typeof import("undici")>();
|
||||
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
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
+111
-15
@@ -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<ClientResolution> {
|
||||
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<ClientResolution> {
|
||||
{ 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<ClientResolution> {
|
||||
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 };
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -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<typeof import("undici")>();
|
||||
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() } },
|
||||
}));
|
||||
|
||||
@@ -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<typeof import("undici")>();
|
||||
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.
|
||||
|
||||
@@ -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
@@ -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`
|
||||
|
||||
Reference in new issue
Block a user