From f5b3612141a36c1ee0405278922dfcb724a2e6c9 Mon Sep 17 00:00:00 2001 From: dnukumamras Date: Thu, 9 Jul 2026 12:07:06 -0700 Subject: [PATCH 1/2] fix(eve): SSRF-guard outbound fetches through a safe-fetch wrapper Route eve's user/tenant/model-influenced outbound requests through a new #shared/safe-fetch.js wrapper. It pins https (plain http only on loopback), DNS-resolves the host and rejects private/link-local/reserved ranges (incl. cloud metadata), re-validates every redirect hop while dropping credentials cross-origin, and bounds time + body size. Migrates the OpenAPI spec and operation calls, the web_fetch tool, remote-agent dispatch, OIDC discovery (plus a jwks_uri preflight), MCP connections, and session callbacks, and adds guard rule 36 forbidding bare fetch() in the network-facing directories. Session callbacks and non-loopback OpenAPI/callback URLs now require https. Co-Authored-By: Claude Opus 4.8 Signed-off-by: dnukumamras --- .changeset/ssrf-safe-outbound-fetch.md | 5 + docs/concepts/security-model.md | 19 + docs/connections/openapi.mdx | 2 + .../execution/remote-agent-dispatch.test.ts | 16 +- .../src/execution/remote-agent-dispatch.ts | 9 +- .../execution/session-callback-step.test.ts | 24 +- .../src/execution/session-callback-step.ts | 10 +- .../eve/src/execution/web-fetch/tool.test.ts | 8 +- packages/eve/src/execution/web-fetch/tool.ts | 28 +- .../eve/src/internal/testing/unit-guard.ts | 6 + .../eve/src/runtime/connections/mcp-client.ts | 6 + .../connections/openapi-client.test.ts | 25 +- .../src/runtime/connections/openapi-client.ts | 11 +- .../eve/src/runtime/governance/auth/oidc.ts | 8 +- .../eve/src/shared/network-address.test.ts | 15 + packages/eve/src/shared/network-address.ts | 9 +- packages/eve/src/shared/safe-fetch.test.ts | 192 ++++++++ packages/eve/src/shared/safe-fetch.ts | 417 ++++++++++++++++++ scripts/guard-invariants.mjs | 48 +- 19 files changed, 787 insertions(+), 71 deletions(-) create mode 100644 .changeset/ssrf-safe-outbound-fetch.md create mode 100644 packages/eve/src/shared/safe-fetch.test.ts create mode 100644 packages/eve/src/shared/safe-fetch.ts diff --git a/.changeset/ssrf-safe-outbound-fetch.md b/.changeset/ssrf-safe-outbound-fetch.md new file mode 100644 index 000000000..a85b2f626 --- /dev/null +++ b/.changeset/ssrf-safe-outbound-fetch.md @@ -0,0 +1,5 @@ +--- +"eve": patch +--- + +Add SSRF protection to outbound requests whose host comes from author, tenant, or model input. A new safe-fetch wrapper resolves the target host and refuses to connect to private, loopback (when hardened), link-local, or cloud-metadata addresses (e.g. `169.254.169.254`), pins the transport to https (loopback http allowed for local dev), re-validates every redirect hop, and drops credentials on cross-origin redirects. It now guards the OpenAPI spec and operation calls, the `web_fetch` tool, remote-agent dispatch, OIDC discovery and JWKS, MCP connections, and session callbacks. Session callbacks and non-loopback OpenAPI/callback URLs now require https. diff --git a/docs/concepts/security-model.md b/docs/concepts/security-model.md index 0b67a79f9..691c7eb65 100644 --- a/docs/concepts/security-model.md +++ b/docs/concepts/security-model.md @@ -52,6 +52,22 @@ Credential brokering gives the model _authenticated_ network access from inside [Connection](../connections) tokens (MCP and OpenAPI) come from either `getToken()` or an interactive OAuth flow, and eve injects the resolved token into every outbound request. The token is cached per step and never serialized to durable state. +## Outbound requests are SSRF-guarded + +Several eve surfaces make outbound HTTP requests to a host that comes from author, tenant, or model input rather than being hardcoded: [OpenAPI](../connections/openapi) spec fetches and credentialed operation calls, the built-in `web_fetch` tool, [MCP](../connections/mcp) connections, remote [subagent](../subagents) dispatch, OIDC discovery, and session callbacks. Left unguarded, any of these could be steered at an internal-only service or a cloud metadata endpoint (`169.254.169.254`) — and because some carry the connection's credentials, that would make an authenticated request on an attacker's behalf. + +This protection is on by default; the author configures nothing. For every request eve issues on those surfaces, it: + +- **Pins the transport to `https`.** Plain `http` is allowed only for loopback hosts, so local development still works but a credentialed call never runs over cleartext. +- **Resolves the host and blocks private ranges.** The hostname is DNS-resolved and rejected if it is — or resolves to — a private (RFC 1918), carrier-grade-NAT, link-local (including cloud metadata), or otherwise reserved address. A public-looking hostname that resolves into a private range is caught, not just IP literals. +- **Re-validates every redirect hop.** Redirects are followed manually and each hop runs the same checks, and `Authorization`/`Cookie` are dropped when a redirect crosses origin, so a safe first hop can't bounce the request (and its credentials) to an internal host. +- **Bounds the request** with a timeout and a response-size cap. + +Two things to know: + +- **Loopback is allowed by default.** `localhost` and `127.0.0.0/8` are treated as same-host, not a network pivot, because local development depends on them. The high-value target — link-local cloud metadata — is still blocked. +- **Your own code is not automatically covered.** The guard applies to eve's built-in request paths. A custom [tool](../tools) or handler that calls `fetch()` directly reaches whatever host it is given; validate untrusted URLs yourself before fetching them. + ## Channel verification A [channel](../channels/overview) is your agent's front door, so authenticating inbound traffic is its job. The built-in platform channels follow two rules, and so must any channel you write yourself: @@ -94,6 +110,9 @@ Before exposing an agent to real traffic: shouldn't have open egress; use credential brokering for authenticated egress. - [ ] Don't surface untrusted text as markup. Model- or user-controlled strings rendered into a channel UI should be escaped for that surface. +- [ ] Validate outbound URLs in your own tools. eve's built-in request paths + are SSRF-guarded, but a custom tool that fetches an author-, tenant-, or + model-supplied URL must validate it before fetching. ## What to read next diff --git a/docs/connections/openapi.mdx b/docs/connections/openapi.mdx index 073eb0d1c..154f84888 100644 --- a/docs/connections/openapi.mdx +++ b/docs/connections/openapi.mdx @@ -25,6 +25,8 @@ Each operation becomes `__`, for example `petstore__get `spec` can be an HTTPS URL that eve fetches at runtime, or an inline parsed OpenAPI object. Prefer a URL when the provider owns the contract and updates it; prefer an inline object for private APIs, generated specs you pin in source control, or small hand-authored contracts. +The spec URL and the resolved base URL must use `https` (plain `http` is allowed only for loopback hosts during local development). Because operation calls carry the connection's credentials, eve also refuses to connect to hosts that are — or that resolve to — a private, link-local, or cloud-metadata address (for example `169.254.169.254`), and re-validates every redirect hop against the same rules. + ## Base URL and servers eve resolves operation paths against `baseUrl` when you provide one. Otherwise, it derives the base URL from the spec: diff --git a/packages/eve/src/execution/remote-agent-dispatch.test.ts b/packages/eve/src/execution/remote-agent-dispatch.test.ts index 8bee9f871..b0ab3c209 100644 --- a/packages/eve/src/execution/remote-agent-dispatch.test.ts +++ b/packages/eve/src/execution/remote-agent-dispatch.test.ts @@ -41,15 +41,13 @@ describe("startRemoteAgentSession", () => { }); expect(childSessionId).toBe("remote-session-header"); - expect(fetchMock).toHaveBeenCalledWith("https://remote.example.com/eve/v1/session", { - body: expect.any(String), - headers: { - authorization: "Bearer remote-token", - "content-type": "application/json", - "x-static": "yes", - }, - method: "POST", - }); + const [calledUrl, init] = fetchMock.mock.calls[0]!; + expect(calledUrl).toBe("https://remote.example.com/eve/v1/session"); + expect(init?.method).toBe("POST"); + const sentHeaders = new Headers(init?.headers); + expect(sentHeaders.get("authorization")).toBe("Bearer remote-token"); + expect(sentHeaders.get("content-type")).toBe("application/json"); + expect(sentHeaders.get("x-static")).toBe("yes"); expect(JSON.parse(fetchMock.mock.calls[0]?.[1]?.body as string)).toEqual({ callback: { callId: "call-remote", diff --git a/packages/eve/src/execution/remote-agent-dispatch.ts b/packages/eve/src/execution/remote-agent-dispatch.ts index a74182013..f6e9b1157 100644 --- a/packages/eve/src/execution/remote-agent-dispatch.ts +++ b/packages/eve/src/execution/remote-agent-dispatch.ts @@ -2,6 +2,7 @@ import { EVE_SESSION_ID_HEADER } from "#protocol/message.js"; import { createEveCallbackRoutePath } from "#protocol/routes.js"; import { createWorkflowCallbackUrl } from "#execution/workflow-callback-url.js"; import { formatSubagentInput } from "#execution/subagent-invocation.js"; +import { readBodyTextSafe, safeFetch } from "#shared/safe-fetch.js"; import type { HarnessSession } from "#harness/types.js"; import type { RuntimeRemoteAgentCallActionRequest } from "#runtime/actions/types.js"; import type { RuntimeSubagentRegistry } from "#runtime/subagents/registry.js"; @@ -23,7 +24,8 @@ export async function startRemoteAgentSession(input: { } const headers = await resolveRemoteAgentRequestHeaders(input.remote); - const response = await fetch(createRemoteAgentSessionUrl(input.remote), { + // Credentialed POST — safeFetch guards the remote URL against SSRF. + const response = await safeFetch(createRemoteAgentSessionUrl(input.remote), { body: JSON.stringify({ callback: { callId: input.action.callId, @@ -58,7 +60,10 @@ export async function startRemoteAgentSession(input: { } try { - const body = (await response.json()) as { readonly sessionId?: unknown }; + // Bounded read — the body only carries a small `{ sessionId }`. + const body = JSON.parse(await readBodyTextSafe(response, { maxBytes: 1024 * 1024 })) as { + readonly sessionId?: unknown; + }; if (typeof body.sessionId === "string" && body.sessionId.length > 0) { return body.sessionId; } diff --git a/packages/eve/src/execution/session-callback-step.test.ts b/packages/eve/src/execution/session-callback-step.test.ts index 7ed5eb7cd..4c5fd31c2 100644 --- a/packages/eve/src/execution/session-callback-step.test.ts +++ b/packages/eve/src/execution/session-callback-step.test.ts @@ -58,11 +58,9 @@ describe("fireSessionCallbackStep", () => { sessionId: "remote-session", subagentName: "research", }), - headers: { - "content-type": "application/json", - }, + headers: expect.any(Headers), method: "POST", - redirect: "error", + redirect: "manual", signal: expect.any(AbortSignal), }); expect(errorSpy).not.toHaveBeenCalled(); @@ -85,11 +83,9 @@ describe("fireSessionCallbackStep", () => { sessionId: "remote-session", subagentName: "research", }), - headers: { - "content-type": "application/json", - }, + headers: expect.any(Headers), method: "POST", - redirect: "error", + redirect: "manual", signal: expect.any(AbortSignal), }); }); @@ -114,11 +110,9 @@ describe("fireSessionCallbackStep", () => { subagentName: "research", usage: USAGE, }), - headers: { - "content-type": "application/json", - }, + headers: expect.any(Headers), method: "POST", - redirect: "error", + redirect: "manual", signal: expect.any(AbortSignal), }); expect(errorSpy).not.toHaveBeenCalled(); @@ -173,11 +167,9 @@ describe("fireSessionCallbackStep", () => { sessionId: "remote-session", subagentName: "research", }), - headers: { - "content-type": "application/json", - }, + headers: expect.any(Headers), method: "POST", - redirect: "error", + redirect: "manual", signal: expect.any(AbortSignal), }); }); diff --git a/packages/eve/src/execution/session-callback-step.ts b/packages/eve/src/execution/session-callback-step.ts index f74b72245..6c93fda50 100644 --- a/packages/eve/src/execution/session-callback-step.ts +++ b/packages/eve/src/execution/session-callback-step.ts @@ -3,6 +3,7 @@ import { parseSessionCallback } from "#channel/session-callback.js"; import { SessionCallbackKey } from "#context/keys.js"; import { createLogger } from "#internal/logging.js"; import { toErrorMessage } from "#shared/errors.js"; +import { safeFetch } from "#shared/safe-fetch.js"; import type { TokenUsage } from "#shared/token-usage.js"; const SESSION_CALLBACK_TIMEOUT_MS = 30_000; @@ -58,16 +59,15 @@ export async function fireSessionCallbackStep(input: { subagentName: callback.subagentName, }; - const response = await fetch(callback.url, { + const response = await safeFetch(callback.url, { body: JSON.stringify(body), headers: { "content-type": "application/json", }, method: "POST", - // Do not follow redirects: a validated callback host could otherwise - // 3xx-bounce the framework to an internal/metadata address after the - // path/token check has already passed. - redirect: "error", + // Reject redirects so a validated host can't 3xx-bounce to metadata; + // safeFetch also DNS-resolves the host (the schema guard only sees IP literals). + maxRedirects: 0, signal: AbortSignal.timeout(SESSION_CALLBACK_TIMEOUT_MS), }); diff --git a/packages/eve/src/execution/web-fetch/tool.test.ts b/packages/eve/src/execution/web-fetch/tool.test.ts index be484d262..aa566f5ef 100644 --- a/packages/eve/src/execution/web-fetch/tool.test.ts +++ b/packages/eve/src/execution/web-fetch/tool.test.ts @@ -235,9 +235,9 @@ describe("executeWebFetchTool", () => { const retryCall = fetchSpy.mock.calls[1]; expect(retryCall).toBeDefined(); - const retryHeaders = (retryCall![1] as RequestInit).headers as Record; + const retryHeaders = new Headers((retryCall![1] as RequestInit).headers); - expect(retryHeaders["User-Agent"]).toBe(EVE_PACKAGE_NAME); + expect(retryHeaders.get("User-Agent")).toBe(EVE_PACKAGE_NAME); }); it("sends format-aware Accept headers", async () => { @@ -257,9 +257,9 @@ describe("executeWebFetchTool", () => { const firstCall = fetchSpy.mock.calls[0]; expect(firstCall).toBeDefined(); - const headers = (firstCall![1] as RequestInit).headers as Record; + const headers = new Headers((firstCall![1] as RequestInit).headers); - expect(headers.Accept).toContain("text/markdown"); + expect(headers.get("Accept")).toContain("text/markdown"); }); it("truncates large bodies to the shared tool-output budget", async () => { diff --git a/packages/eve/src/execution/web-fetch/tool.ts b/packages/eve/src/execution/web-fetch/tool.ts index 0a10928ed..e53998b70 100644 --- a/packages/eve/src/execution/web-fetch/tool.ts +++ b/packages/eve/src/execution/web-fetch/tool.ts @@ -1,6 +1,7 @@ import { EVE_PACKAGE_NAME } from "#internal/package-name.js"; import { truncateHead } from "#execution/sandbox/truncate-output.js"; import { convertHtmlToMarkdown, extractTextFromHtml } from "#execution/web-fetch/html.js"; +import { readBodyTextSafe, safeFetch } from "#shared/safe-fetch.js"; const MAX_RESPONSE_SIZE = 5 * 1024 * 1024; // 5 MB const DEFAULT_TIMEOUT_MS = 30_000; // 30 seconds @@ -69,23 +70,28 @@ export async function executeWebFetchTool( MAX_TIMEOUT_MS, ); - const timeoutSignal = AbortSignal.timeout(timeoutMs); + const headers = buildHeaders(format); + // Shared deadline so the initial request and the Cloudflare retry together + // stay within timeoutMs, not one budget each. + const deadline = AbortSignal.timeout(timeoutMs); const signal = options?.abortSignal === undefined - ? timeoutSignal - : AbortSignal.any([options.abortSignal, timeoutSignal]); - const headers = buildHeaders(format); + ? deadline + : AbortSignal.any([options.abortSignal, deadline]); + // Model-supplied URL — the top SSRF target. allowHttp since web pages are + // still often http. + const fetchOptions = { allowHttp: true, headers, signal, timeoutMs }; - const initial = await fetch(url, { headers, signal }); + const initial = await safeFetch(url, fetchOptions); // Cloudflare may reject browser-like UA strings from Node.js runtimes // because the TLS fingerprint does not match a real browser. Retry with // an honest UA to bypass the challenge. const response = initial.status === 403 && initial.headers.get("cf-mitigated") === "challenge" - ? await fetch(url, { + ? await safeFetch(url, { + ...fetchOptions, headers: { ...headers, "User-Agent": EVE_PACKAGE_NAME }, - signal, }) : initial; @@ -99,15 +105,9 @@ export async function executeWebFetchTool( throw new Error("Response too large (exceeds 5 MB limit)."); } - const buffer = await response.arrayBuffer(); - - if (buffer.byteLength > MAX_RESPONSE_SIZE) { - throw new Error("Response too large (exceeds 5 MB limit)."); - } - const contentType = response.headers.get("content-type") ?? ""; const isHtml = contentType.includes("text/html"); - const body = new TextDecoder().decode(buffer); + const body = await readBodyTextSafe(response, { maxBytes: MAX_RESPONSE_SIZE }); let rawContent: string; diff --git a/packages/eve/src/internal/testing/unit-guard.ts b/packages/eve/src/internal/testing/unit-guard.ts index 41275d7fc..93be3a3f9 100644 --- a/packages/eve/src/internal/testing/unit-guard.ts +++ b/packages/eve/src/internal/testing/unit-guard.ts @@ -1,9 +1,15 @@ import { vi } from "vitest"; +import { setSafeFetchLookupOverride } from "#shared/safe-fetch.js"; + /** * Vitest setup that makes unit-tier boundary violations fail loudly. */ +// safeFetch DNS-resolves before connecting; unit tests are hermetic, so resolve +// every host to a fixed public IP. SSRF rejection is covered in safe-fetch.test.ts. +setSafeFetchLookupOverride(async () => [{ address: "93.184.216.34", family: 4 }]); + const INTEGRATION_GUIDANCE = "Move this test to `src/**/*.integration.test.ts` or `test/scenarios/*.scenario.test.ts`."; diff --git a/packages/eve/src/runtime/connections/mcp-client.ts b/packages/eve/src/runtime/connections/mcp-client.ts index a947ea979..3f9c6dc0c 100644 --- a/packages/eve/src/runtime/connections/mcp-client.ts +++ b/packages/eve/src/runtime/connections/mcp-client.ts @@ -8,6 +8,7 @@ import type { ResolvedConnectionDefinition } from "#runtime/types.js"; import { evictScopedToken, resolveScopedToken } from "#runtime/connections/scoped-authorization.js"; import { resolveConnectionAuthorization } from "#runtime/connections/resolve-authorization.js"; import { isObject } from "#shared/guards.js"; +import { assertUrlSafeToFetch } from "#shared/safe-fetch.js"; import type { AuthorizationDefinition, ConnectionClient, @@ -70,6 +71,11 @@ export class McpConnectionClient implements ConnectionClient { const headers = await resolveHeaders(this.#connection); const url = this.#connection.url; + // @ai-sdk/mcp owns the socket, so eve can only preflight the URL for SSRF. + if (typeof url === "string") { + await assertUrlSafeToFetch(url); + } + try { return await createMCPClient({ transport: { type: "http", url, headers }, diff --git a/packages/eve/src/runtime/connections/openapi-client.test.ts b/packages/eve/src/runtime/connections/openapi-client.test.ts index 9d6bf5c47..12058c512 100644 --- a/packages/eve/src/runtime/connections/openapi-client.test.ts +++ b/packages/eve/src/runtime/connections/openapi-client.test.ts @@ -268,7 +268,7 @@ describe("OpenApiConnectionClient", () => { const [calledUrl, init] = fetchMock.mock.calls[0]!; expect(calledUrl.toString()).toBe("https://api.example.com/v1/projects/prj_1?teamId=team_9"); expect(init.method).toBe("GET"); - expect((init.headers as Record).Authorization).toBe("Bearer secret"); + expect(new Headers(init.headers).get("Authorization")).toBe("Bearer secret"); }); it("resolves auth and headers from the active caller", async () => { @@ -306,10 +306,9 @@ describe("OpenApiConnectionClient", () => { await contextStorage.run(ctx, () => client.executeTool("getProject", { id: "prj_1" })); const [, init] = fetchMock.mock.calls[0]!; - expect(init.headers).toMatchObject({ - Authorization: "Bearer token-for-alice", - "X-Tenant": "acme", - }); + const callerHeaders = new Headers(init.headers); + expect(callerHeaders.get("Authorization")).toBe("Bearer token-for-alice"); + expect(callerHeaders.get("X-Tenant")).toBe("acme"); }); it("serializes the request body on execute", async () => { @@ -659,7 +658,7 @@ describe("OpenApiConnectionClient", () => { await client.executeTool("getMe", { session: "abc 123" }); const [, init] = fetchMock.mock.calls[0]!; - expect((init.headers as Record).cookie).toBe("session=abc%20123"); + expect(new Headers(init.headers).get("cookie")).toBe("session=abc%20123"); }); it("places the credential in an apiKey header security scheme", async () => { @@ -678,9 +677,9 @@ describe("OpenApiConnectionClient", () => { await client.executeTool("op", {}); const [, init] = fetchMock.mock.calls[0]!; - const headers = init.headers as Record; - expect(headers["X-API-Key"]).toBe("secret"); - expect(headers.Authorization).toBeUndefined(); + const headers = new Headers(init.headers); + expect(headers.get("X-API-Key")).toBe("secret"); + expect(headers.get("Authorization")).toBeNull(); }); it("places the credential in an apiKey query security scheme", async () => { @@ -700,7 +699,7 @@ describe("OpenApiConnectionClient", () => { const [calledUrl, init] = fetchMock.mock.calls[0]!; expect(String(calledUrl)).toContain("api_key=secret"); - expect((init.headers as Record).Authorization).toBeUndefined(); + expect(new Headers(init.headers).get("Authorization")).toBeNull(); }); it("uses HTTP basic auth when the security scheme is basic", async () => { @@ -719,7 +718,7 @@ describe("OpenApiConnectionClient", () => { await client.executeTool("op", {}); const [, init] = fetchMock.mock.calls[0]!; - expect((init.headers as Record).Authorization).toBe("Basic dXNlcjpwYXNz"); + expect(new Headers(init.headers).get("Authorization")).toBe("Basic dXNlcjpwYXNz"); }); it("places credentials from Swagger 2.0 securityDefinitions", async () => { @@ -744,7 +743,7 @@ describe("OpenApiConnectionClient", () => { const [calledUrl, init] = fetchMock.mock.calls[0]!; expect(String(calledUrl)).toBe("https://api.example.com/v1/items/itm_1?app_key=secret"); - expect((init.headers as Record).Authorization).toBeUndefined(); + expect(new Headers(init.headers).get("Authorization")).toBeNull(); }); it("keeps Bearer auth for http bearer and oauth2 schemes", async () => { @@ -763,7 +762,7 @@ describe("OpenApiConnectionClient", () => { await client.executeTool("op", {}); const [, init] = fetchMock.mock.calls[0]!; - expect((init.headers as Record).Authorization).toBe("Bearer secret"); + expect(new Headers(init.headers).get("Authorization")).toBe("Bearer secret"); }); }); diff --git a/packages/eve/src/runtime/connections/openapi-client.ts b/packages/eve/src/runtime/connections/openapi-client.ts index c35ced847..ba7779b9b 100644 --- a/packages/eve/src/runtime/connections/openapi-client.ts +++ b/packages/eve/src/runtime/connections/openapi-client.ts @@ -25,6 +25,7 @@ import type { ConnectionToolMetadata, } from "#runtime/connections/types.js"; import { isObject } from "#shared/guards.js"; +import { readBodyTextSafe, safeFetch } from "#shared/safe-fetch.js"; interface OpenApiToolCache { readonly metadata: readonly ConnectionToolMetadata[]; @@ -210,7 +211,8 @@ export class OpenApiConnectionClient implements ConnectionClient { let response: Response; try { - response = await fetch(spec, { + // safeFetch: https-pin (loopback http ok) + SSRF host/redirect guard. + response = await safeFetch(spec, { headers: { accept: "application/json, application/yaml, text/yaml, */*" }, }); } catch (error) { @@ -224,7 +226,7 @@ export class OpenApiConnectionClient implements ConnectionClient { ); } - const text = await response.text(); + const text = await readBodyTextSafe(response); let parsed: unknown; try { parsed = parseSpecDocument(text); @@ -434,7 +436,8 @@ export class OpenApiConnectionClient implements ConnectionClient { headers["content-type"] = operation.requestBody.contentType; } - const response = await fetch(url, { + // Credentialed call — safeFetch guards the resolved base URL against SSRF. + const response = await safeFetch(url, { method: operation.method.toUpperCase(), headers, body, @@ -466,7 +469,7 @@ function joinPath(baseUrl: string, path: string): string { } async function readResponseBody(response: Response): Promise { - const text = await response.text(); + const text = await readBodyTextSafe(response); if (text.length === 0) { return null; } diff --git a/packages/eve/src/runtime/governance/auth/oidc.ts b/packages/eve/src/runtime/governance/auth/oidc.ts index 1ddc6e4aa..bb4d43901 100644 --- a/packages/eve/src/runtime/governance/auth/oidc.ts +++ b/packages/eve/src/runtime/governance/auth/oidc.ts @@ -10,6 +10,7 @@ import type { ResolvedOidcAuthStrategy, RouteStrategyAuthenticationResult, } from "#runtime/governance/auth/types.js"; +import { assertUrlSafeToFetch, readBodyTextSafe, safeFetch } from "#shared/safe-fetch.js"; const oidcDiscoveryDocumentSchema = z .object({ @@ -164,6 +165,9 @@ async function getOidcRemoteJwks( return existing; } + // jwks_uri comes from the discovery doc and jose owns its fetch, so preflight + // it here against SSRF. + await assertUrlSafeToFetch(discoveryDocument.jwks_uri); const remoteJwks = createRemoteJWKSet(new URL(discoveryDocument.jwks_uri)); oidcJwksCache.set(discoveryDocument.jwks_uri, remoteJwks); @@ -179,7 +183,7 @@ async function getOidcDiscoveryDocument( return await cached; } - const discoveryPromise = fetch(discoveryUrl, { + const discoveryPromise = safeFetch(discoveryUrl, { headers: { accept: "application/json", }, @@ -189,7 +193,7 @@ async function getOidcDiscoveryDocument( throw new Error(`Discovery route returned HTTP ${response.status}.`); } - return oidcDiscoveryDocumentSchema.parse(await response.json()); + return oidcDiscoveryDocumentSchema.parse(JSON.parse(await readBodyTextSafe(response))); }) .catch((error) => { oidcDiscoveryDocumentCache.delete(discoveryUrl); diff --git a/packages/eve/src/shared/network-address.test.ts b/packages/eve/src/shared/network-address.test.ts index 5ad147eba..d404f77e3 100644 --- a/packages/eve/src/shared/network-address.test.ts +++ b/packages/eve/src/shared/network-address.test.ts @@ -4,6 +4,7 @@ import { isLoopbackHostname, isLoopbackServerUrl, isReservedIpAddress, + normalizeAddress, } from "#shared/network-address.js"; describe("isLoopbackHostname", () => { @@ -70,3 +71,17 @@ describe("isReservedIpAddress", () => { } }); }); + +describe("normalizeAddress", () => { + it("strips IPv6 brackets and zone indices and unwraps IPv4-mapped IPv6", () => { + expect(normalizeAddress("[::1]")).toBe("::1"); + expect(normalizeAddress("[fe80::1%eth0]")).toBe("fe80::1"); + expect(normalizeAddress("::ffff:169.254.169.254")).toBe("169.254.169.254"); + expect(normalizeAddress(" example.com ")).toBe("example.com"); + }); + + it("leaves a plain hostname or bare IPv4 unchanged", () => { + expect(normalizeAddress("api.example.com")).toBe("api.example.com"); + expect(normalizeAddress("10.0.0.1")).toBe("10.0.0.1"); + }); +}); diff --git a/packages/eve/src/shared/network-address.ts b/packages/eve/src/shared/network-address.ts index 59c79ac5c..d97ba98dd 100644 --- a/packages/eve/src/shared/network-address.ts +++ b/packages/eve/src/shared/network-address.ts @@ -27,7 +27,14 @@ reservedRanges.addAddress("::", "ipv6"); // unspecified reservedRanges.addSubnet("fc00::", 7, "ipv6"); // unique-local reservedRanges.addSubnet("fe80::", 10, "ipv6"); // link-local -function normalizeAddress(host: string): string { +/** + * Reduces a URL host or address literal to a bare, classifiable address: + * trims, strips IPv6 brackets and any zone index (`%eth0`), and unwraps + * IPv4-mapped IPv6 (`::ffff:a.b.c.d`) so the IPv4 ranges apply. Plain hostnames + * pass through unchanged. Shared with the SSRF fetch guard so URL hosts are + * normalized identically here and before a DNS lookup. + */ +export function normalizeAddress(host: string): string { const withoutBrackets = host.trim().replace(/^\[(.*)\]$/u, "$1"); const zoneIndex = withoutBrackets.indexOf("%"); const withoutZone = zoneIndex === -1 ? withoutBrackets : withoutBrackets.slice(0, zoneIndex); diff --git a/packages/eve/src/shared/safe-fetch.test.ts b/packages/eve/src/shared/safe-fetch.test.ts new file mode 100644 index 000000000..17ea1a850 --- /dev/null +++ b/packages/eve/src/shared/safe-fetch.test.ts @@ -0,0 +1,192 @@ +import { describe, expect, it, vi } from "vitest"; + +import { + assertUrlSafeToFetch, + readBodyTextSafe, + safeFetch, + type SafeFetchOptions, +} from "#shared/safe-fetch.js"; + +/** Resolver stub: maps a hostname to fixed addresses. */ +function lookupReturning(addresses: string[]): SafeFetchOptions["lookup"] { + return async () => + addresses.map((address) => ({ address, family: address.includes(":") ? 6 : 4 })); +} + +const PUBLIC = lookupReturning(["93.184.216.34"]); + +describe("assertUrlSafeToFetch", () => { + it("accepts a public https host", async () => { + await expect( + assertUrlSafeToFetch("https://example.com/x", { lookup: PUBLIC }), + ).resolves.toBeInstanceOf(URL); + }); + + it("rejects a reserved IP literal (cloud metadata)", async () => { + await expect(assertUrlSafeToFetch("https://169.254.169.254/latest")).rejects.toThrow( + /reserved/, + ); + }); + + it("rejects private IP literals", async () => { + for (const host of ["10.0.0.1", "192.168.1.1", "172.16.0.1", "[fd00::1]"]) { + await expect(assertUrlSafeToFetch(`https://${host}/`)).rejects.toThrow(/reserved/); + } + }); + + it("rejects a host that DNS-resolves to a private address", async () => { + await expect( + assertUrlSafeToFetch("https://rebind.example/", { lookup: lookupReturning(["10.0.0.5"]) }), + ).rejects.toThrow(/reserved/); + }); + + it("rejects when any resolved address is unsafe", async () => { + await expect( + assertUrlSafeToFetch("https://mixed.example/", { + lookup: lookupReturning(["93.184.216.34", "169.254.169.254"]), + }), + ).rejects.toThrow(/reserved/); + }); + + it("allows loopback by default and rejects it when disallowed", async () => { + await expect(assertUrlSafeToFetch("https://127.0.0.1/")).resolves.toBeInstanceOf(URL); + await expect(assertUrlSafeToFetch("https://localhost/")).resolves.toBeInstanceOf(URL); + await expect( + assertUrlSafeToFetch("https://127.0.0.1/", { allowLoopback: false }), + ).rejects.toThrow(/reserved/); + await expect( + assertUrlSafeToFetch("https://localhost/", { allowLoopback: false }), + ).rejects.toThrow(/reserved/); + }); + + it("pins protocol: rejects http unless allowHttp or loopback", async () => { + await expect(assertUrlSafeToFetch("http://example.com/", { lookup: PUBLIC })).rejects.toThrow( + /https/, + ); + await expect( + assertUrlSafeToFetch("http://example.com/", { lookup: PUBLIC, allowHttp: true }), + ).resolves.toBeInstanceOf(URL); + // Plain http on loopback is allowed for local dev. + await expect(assertUrlSafeToFetch("http://localhost:3000/")).resolves.toBeInstanceOf(URL); + }); + + it("rejects non-http(s) protocols", async () => { + await expect(assertUrlSafeToFetch("file:///etc/passwd")).rejects.toThrow(/https/); + }); + + it("enforces an optional host allowlist", async () => { + await expect( + assertUrlSafeToFetch("https://evil.com/", { lookup: PUBLIC, allowedHosts: ["example.com"] }), + ).rejects.toThrow(/allowlist/); + await expect( + assertUrlSafeToFetch("https://api.example.com/", { + lookup: PUBLIC, + allowedHosts: ["example.com"], + }), + ).resolves.toBeInstanceOf(URL); + }); + + it("prefixes errors with the label", async () => { + await expect( + assertUrlSafeToFetch("https://169.254.169.254/", { label: "my-connection" }), + ).rejects.toThrow(/^my-connection:/); + }); +}); + +describe("safeFetch", () => { + const ok = (body = "ok") => new Response(body, { status: 200 }); + + it("fetches a validated URL", async () => { + const fetchImpl = vi.fn(async () => ok("hello")); + const res = await safeFetch("https://example.com/", { lookup: PUBLIC, fetch: fetchImpl }); + expect(await res.text()).toBe("hello"); + expect(fetchImpl).toHaveBeenCalledOnce(); + expect(fetchImpl.mock.calls[0]?.[1]?.redirect).toBe("manual"); + }); + + it("follows redirects and re-validates each hop", async () => { + const fetchImpl = vi.fn(async (input) => + String(input).endsWith("/final") + ? ok("done") + : new Response(null, { status: 302, headers: { location: "https://example.com/final" } }), + ); + const res = await safeFetch("https://example.com/start", { lookup: PUBLIC, fetch: fetchImpl }); + expect(await res.text()).toBe("done"); + expect(fetchImpl).toHaveBeenCalledTimes(2); + }); + + it("rejects a redirect to a private host", async () => { + const fetchImpl = vi.fn(async (input) => + String(input).includes("start") + ? new Response(null, { status: 302, headers: { location: "https://169.254.169.254/" } }) + : ok(), + ); + await expect( + safeFetch("https://example.com/start", { lookup: PUBLIC, fetch: fetchImpl }), + ).rejects.toThrow(/reserved/); + }); + + it("drops authorization/cookie on cross-origin redirects", async () => { + const fetchImpl = vi.fn(async (input) => + String(input).includes("start") + ? new Response(null, { status: 302, headers: { location: "https://other.example/next" } }) + : ok(), + ); + await safeFetch("https://example.com/start", { + lookup: PUBLIC, + fetch: fetchImpl, + headers: { authorization: "Bearer secret", cookie: "s=1", "x-keep": "1" }, + }); + const secondHopHeaders = new Headers(fetchImpl.mock.calls[1]?.[1]?.headers); + expect(secondHopHeaders.get("authorization")).toBeNull(); + expect(secondHopHeaders.get("cookie")).toBeNull(); + expect(secondHopHeaders.get("x-keep")).toBe("1"); + }); + + it("keeps credentials on same-origin redirects", async () => { + const fetchImpl = vi.fn(async (input) => + String(input).includes("start") + ? new Response(null, { status: 307, headers: { location: "https://example.com/next" } }) + : ok(), + ); + await safeFetch("https://example.com/start", { + lookup: PUBLIC, + fetch: fetchImpl, + headers: { authorization: "Bearer secret" }, + }); + const secondHopHeaders = new Headers(fetchImpl.mock.calls[1]?.[1]?.headers); + expect(secondHopHeaders.get("authorization")).toBe("Bearer secret"); + }); + + it("throws after exceeding maxRedirects", async () => { + const fetchImpl = vi.fn( + async () => + new Response(null, { status: 302, headers: { location: "https://example.com/loop" } }), + ); + await expect( + safeFetch("https://example.com/loop", { lookup: PUBLIC, fetch: fetchImpl, maxRedirects: 2 }), + ).rejects.toThrow(/redirects/); + }); + + it("times out", async () => { + const fetchImpl: typeof fetch = (_url, init) => + new Promise((_resolve, reject) => { + init?.signal?.addEventListener("abort", () => reject(init.signal?.reason)); + }); + await expect( + safeFetch("https://example.com/", { lookup: PUBLIC, fetch: fetchImpl, timeoutMs: 10 }), + ).rejects.toThrow(/timed out/); + }); +}); + +describe("readBodyTextSafe", () => { + it("reads a small body", async () => { + expect(await readBodyTextSafe(new Response("hello"))).toBe("hello"); + }); + + it("throws when the body exceeds maxBytes", async () => { + await expect(readBodyTextSafe(new Response("x".repeat(100)), { maxBytes: 10 })).rejects.toThrow( + /exceeded/, + ); + }); +}); diff --git a/packages/eve/src/shared/safe-fetch.ts b/packages/eve/src/shared/safe-fetch.ts new file mode 100644 index 000000000..2c243150a --- /dev/null +++ b/packages/eve/src/shared/safe-fetch.ts @@ -0,0 +1,417 @@ +import { lookup as dnsLookup } from "node:dns/promises"; +import { isIP } from "node:net"; + +import { + isLoopbackHostname, + isReservedIpAddress, + normalizeAddress, +} from "#shared/network-address.js"; + +/** + * SSRF-safe outbound `fetch` for URLs whose host comes from author, tenant, or + * model input. Every hop: pins `https` (plain `http` only via `allowHttp` or on + * loopback), DNS-resolves the host and rejects any private/link-local/reserved + * address, re-validates redirects (dropping credentials cross-origin), and + * bounds time + body size. + * + * It resolves-then-connects rather than pinning the IP at connect time, so a + * sub-second DNS rebind can still slip through; closing that needs a custom + * undici dispatcher (a runtime dep eve avoids). The preflight still blocks the + * common cases (configured private host, host resolving private, redirect to + * metadata). + */ + +type FetchImpl = typeof globalThis.fetch; +type FetchInit = NonNullable[1]>; + +/** Resolver seam matching `node:dns/promises` `lookup(host, { all: true })`. */ +type LookupImpl = ( + hostname: string, + options: { all: true }, +) => Promise>; + +const dnsPromisesLookup: LookupImpl = (hostname, options) => dnsLookup(hostname, options); + +// Default resolver, swappable so hermetic unit tests can avoid real DNS without +// threading a `lookup` option through every call site. +let activeLookup: LookupImpl = dnsPromisesLookup; + +/** + * Test-only: override the default DNS resolver (or pass `undefined` to restore + * `node:dns`). Production code should never call this. + */ +export function setSafeFetchLookupOverride(lookup: LookupImpl | undefined): void { + activeLookup = lookup ?? dnsPromisesLookup; +} + +/** Options shared by {@link assertUrlSafeToFetch} and {@link safeFetch}. */ +export interface AssertUrlSafeOptions { + /** + * Allow requests to loopback hosts (`localhost`, `127.0.0.0/8`, `::1`). + * Loopback is same-host rather than a network pivot and local development + * depends on it, so it is allowed by default. Set `false` to harden a + * production surface. + * + * @default true + */ + readonly allowLoopback?: boolean; + + /** + * Allow plain `http:` to non-loopback hosts. Off by default so credentialed + * requests never run over cleartext; the `web_fetch` tool enables it because + * arbitrary web pages are still commonly `http:`. + * + * @default false + */ + readonly allowHttp?: boolean; + + /** + * Optional hostname allowlist. When set, the host must equal an entry + * (case-insensitive) or be a subdomain of one. The SSRF resolution check + * still runs on top of the allowlist. + */ + readonly allowedHosts?: readonly string[]; + + /** Prefix for error messages (e.g. an OpenAPI connection name). */ + readonly label?: string; + + /** Injectable resolver for tests. Defaults to `node:dns/promises` `lookup`. */ + readonly lookup?: LookupImpl; +} + +/** Options for {@link safeFetch}. */ +export interface SafeFetchOptions extends AssertUrlSafeOptions { + readonly method?: string; + readonly headers?: FetchInit["headers"]; + readonly body?: FetchInit["body"]; + /** External abort signal; combined with the internal timeout. */ + readonly signal?: AbortSignal; + /** + * Timeout in milliseconds across the whole redirect chain. + * + * @default 30_000 + */ + readonly timeoutMs?: number; + /** + * Maximum redirect hops to follow. + * + * @default 5 + */ + readonly maxRedirects?: number; + /** Injectable `fetch` for tests. Defaults to the global `fetch`. */ + readonly fetch?: FetchImpl; +} + +/** Options for the body readers. */ +export interface ReadBodyOptions { + /** + * Maximum bytes to read before aborting with an error. + * + * @default 10 MB + */ + readonly maxBytes?: number; +} + +const DEFAULT_TIMEOUT_MS = 30_000; +const DEFAULT_MAX_REDIRECTS = 5; +const DEFAULT_MAX_BYTES = 10 * 1024 * 1024; + +function withLabel(label: string | undefined, message: string): string { + return label === undefined ? message : `${label}: ${message}`; +} + +// isReservedIpAddress allows loopback by design, so reject it separately when +// the caller disallows loopback. +function isAddressAllowed(address: string, allowLoopback: boolean): boolean { + if (isReservedIpAddress(address)) { + return false; + } + return allowLoopback || !isLoopbackHostname(address); +} + +function assertProtocol( + url: URL, + allowHttp: boolean, + allowLoopback: boolean, + label?: string, +): void { + if (url.protocol === "https:") { + return; + } + if ( + url.protocol === "http:" && + (allowHttp || (allowLoopback && isLoopbackHostname(url.hostname))) + ) { + return; + } + throw new Error( + withLabel( + label, + `refusing to fetch "${url.protocol}//${url.host}" — only ${allowHttp ? "http(s)" : "https"} is allowed${allowHttp ? "" : " (plain http only for loopback)"}.`, + ), + ); +} + +function isHostInAllowlist(hostname: string, allowedHosts: readonly string[]): boolean { + const host = hostname.toLowerCase(); + return allowedHosts.some((entry) => { + const normalized = entry.toLowerCase(); + return normalized.length > 0 && (host === normalized || host.endsWith(`.${normalized}`)); + }); +} + +/** + * Validates a URL for SSRF safety without issuing a request: parses it, pins + * the protocol, applies any host allowlist, and rejects hosts that are — or + * that DNS-resolve to — a private, loopback (when disallowed), link-local, or + * otherwise reserved address. Returns the parsed URL. Throws otherwise. + * + * Use this to preflight a URL handed to a third-party client that owns its own + * socket (e.g. an MCP SDK or a JWKS fetcher), where {@link safeFetch} cannot + * wrap the request itself. + */ +export async function assertUrlSafeToFetch( + rawUrl: string | URL, + options: AssertUrlSafeOptions = {}, +): Promise { + const { + allowLoopback = true, + allowHttp = false, + allowedHosts, + label, + lookup = activeLookup, + } = options; + + let url: URL; + try { + url = rawUrl instanceof URL ? rawUrl : new URL(rawUrl); + } catch { + throw new Error(withLabel(label, `invalid URL "${String(rawUrl)}".`)); + } + + assertProtocol(url, allowHttp, allowLoopback, label); + + // Strip IPv6 brackets/zone so isIP classifies it and the resolver gets a bare host. + const host = normalizeAddress(url.hostname); + + if (allowedHosts !== undefined && !isHostInAllowlist(host, allowedHosts)) { + throw new Error(withLabel(label, `host "${host}" is not in the allowlist.`)); + } + + const reject = (): never => { + throw new Error( + withLabel( + label, + `refusing to connect to "${host}" — private, loopback, or reserved address.`, + ), + ); + }; + + if (isIP(host) !== 0) { + if (!isAddressAllowed(host, allowLoopback)) { + reject(); + } + return url; + } + + if (isLoopbackHostname(host)) { + if (!allowLoopback) { + reject(); + } + return url; + } + + let addresses: Array<{ address: string }>; + try { + addresses = await lookup(host, { all: true }); + } catch { + throw new Error(withLabel(label, `could not resolve host "${host}".`)); + } + if (addresses.length === 0) { + throw new Error(withLabel(label, `could not resolve host "${host}".`)); + } + for (const { address } of addresses) { + if (!isAddressAllowed(address, allowLoopback)) { + reject(); + } + } + + return url; +} + +function isRedirectStatus(status: number): boolean { + return status === 301 || status === 302 || status === 303 || status === 307 || status === 308; +} + +async function drainBody(response: Response): Promise { + try { + await response.arrayBuffer(); + } catch { + // Releasing the socket is best-effort. + } +} + +const CONTENT_HEADERS = new Set([ + "content-length", + "content-type", + "content-encoding", + "content-language", + "content-location", +]); + +/** + * SSRF-safe `fetch`. Validates the URL (see {@link assertUrlSafeToFetch}), + * follows redirects manually so every hop is re-validated, drops + * `authorization`/`cookie` on cross-origin redirects, and bounds the whole + * chain with a timeout. Returns the final `Response`; read its body with + * {@link readBodyTextSafe} to enforce a size cap. + */ +export async function safeFetch( + rawUrl: string | URL, + options: SafeFetchOptions = {}, +): Promise { + const { + method, + headers, + body, + signal: externalSignal, + timeoutMs = DEFAULT_TIMEOUT_MS, + maxRedirects = DEFAULT_MAX_REDIRECTS, + fetch: fetchImpl = globalThis.fetch, + ...assertOptions + } = options; + + let currentUrl = await assertUrlSafeToFetch(rawUrl, assertOptions); + + const controller = new AbortController(); + const timer = setTimeout(() => { + controller.abort( + new Error(withLabel(assertOptions.label, `request timed out after ${timeoutMs}ms.`)), + ); + }, timeoutMs); + + const onExternalAbort = () => controller.abort(externalSignal?.reason); + if (externalSignal !== undefined) { + if (externalSignal.aborted) { + onExternalAbort(); + } else { + externalSignal.addEventListener("abort", onExternalAbort, { once: true }); + } + } + + let currentMethod = method ?? "GET"; + let currentBody = body; + const currentHeaders = new Headers(headers); + + try { + for (let hop = 0; ; hop += 1) { + const response = await fetchImpl(currentUrl.toString(), { + method: currentMethod, + headers: currentHeaders, + body: currentBody ?? undefined, + redirect: "manual", + signal: controller.signal, + }); + + if (!isRedirectStatus(response.status)) { + return response; + } + if (hop >= maxRedirects) { + await drainBody(response); + throw new Error(withLabel(assertOptions.label, `exceeded ${maxRedirects} redirects.`)); + } + + const location = response.headers.get("location"); + if (location === null) { + await drainBody(response); + throw new Error( + withLabel(assertOptions.label, "redirect response is missing a Location header."), + ); + } + + let nextUrl: URL; + try { + nextUrl = new URL(location, currentUrl); + } catch { + await drainBody(response); + throw new Error( + withLabel(assertOptions.label, `redirect Location "${location}" is not a valid URL.`), + ); + } + + await drainBody(response); + await assertUrlSafeToFetch(nextUrl, assertOptions); + + // RFC 7231 §6.4: 301/302/303 downgrade to GET and drop the body. + if ( + (response.status === 301 || response.status === 302 || response.status === 303) && + currentMethod.toUpperCase() !== "HEAD" + ) { + currentMethod = "GET"; + currentBody = undefined; + for (const name of CONTENT_HEADERS) { + currentHeaders.delete(name); + } + } + + if (nextUrl.origin !== currentUrl.origin) { + currentHeaders.delete("authorization"); + currentHeaders.delete("cookie"); + } + + currentUrl = nextUrl; + } + } finally { + clearTimeout(timer); + if (externalSignal !== undefined) { + externalSignal.removeEventListener("abort", onExternalAbort); + } + } +} + +/** + * Reads a `Response` body as UTF-8 text, aborting the connection and throwing + * once `maxBytes` is exceeded so an oversized body cannot exhaust memory. + */ +export async function readBodyTextSafe( + response: Response, + options: ReadBodyOptions = {}, +): Promise { + const { maxBytes = DEFAULT_MAX_BYTES } = options; + const body = response.body; + if (body === null) { + return ""; + } + + const reader = body.getReader(); + const decoder = new TextDecoder("utf-8"); + let text = ""; + let received = 0; + + try { + for (;;) { + const { done, value } = await reader.read(); + if (done) { + break; + } + if (value === undefined) { + continue; + } + received += value.byteLength; + if (received > maxBytes) { + await reader.cancel().catch(() => {}); + throw new Error(`response body exceeded ${maxBytes} bytes.`); + } + text += decoder.decode(value, { stream: true }); + } + text += decoder.decode(); + } finally { + try { + reader.releaseLock(); + } catch { + // Reader may already be cancelled. + } + } + + return text; +} diff --git a/scripts/guard-invariants.mjs b/scripts/guard-invariants.mjs index 8bdd7851d..179087565 100644 --- a/scripts/guard-invariants.mjs +++ b/scripts/guard-invariants.mjs @@ -82,6 +82,14 @@ * `queue-namespace.ts`. The generated agent bootstrap installs the * agent-scoped namespace before queue-producing APIs can run. * rule 34 — `phase` stays a runtime-only dependency. No file under the Eve\n * logo renderer's GPU/runtime boundary (render/, shaders/, or the\n * offline render harness) may import the `phase` package. This keeps\n * the mechanical separation between the lifecycle layer and the GPU\n * renderer enforceable. + * rule 36 — No bare `fetch(` in the network-facing directories + * (`runtime/connections/`, `execution/`, `runtime/governance/auth/`). + * These issue outbound requests whose host is influenced by author, + * tenant, or model input, so they must go through `safeFetch` / + * `assertUrlSafeToFetch` from `#shared/safe-fetch.js`, which reject + * private/link-local/metadata targets and re-validate redirects. + * (Rule 35 is reserved for the gray-matter import guard landing + * separately; numeric gaps are normal here.) * * Baselines for rules with pre-existing violations live in * `guard-invariants-baseline.json`. Counts and allowlists in that file @@ -170,6 +178,7 @@ function isTsLike(relPath) { * rule27: Violation[]; * rule28: Violation[]; * rule33: Violation[]; + * rule36: Violation[]; * symlinks: string[]; * }} state */ @@ -197,9 +206,43 @@ async function scanRepo(state) { checkRule27(posix, lines, state.rule27); checkRule28(posix, lines, state.rule28); checkRule33(posix, lines, state.rule33); + checkRule36(posix, lines, state.rule36); } } +// ---------- Rule 36: bare fetch() in network-facing directories ---------- + +const NETWORK_FACING_PREFIXES = [ + "packages/eve/src/runtime/connections/", + "packages/eve/src/execution/", + "packages/eve/src/runtime/governance/auth/", +]; +const SAFE_FETCH_MODULE = "packages/eve/src/shared/safe-fetch.ts"; +// `safeFetch(` doesn't match (capital F); `\s*` tolerates `fetch (`. +const BARE_FETCH_RE = /\bfetch\s*\(/; + +/** + * @param {string} posix + * @param {string[]} lines + * @param {Violation[]} violations + */ +function checkRule36(posix, lines, violations) { + if (posix === SAFE_FETCH_MODULE) return; + if (!NETWORK_FACING_PREFIXES.some((prefix) => posix.startsWith(prefix))) return; + // Tests may stub fetch directly; the concern is production request paths. + if (/\.(test|integration\.test|scenario\.test)\.ts$/.test(posix)) return; + lines.forEach((line, idx) => { + if (BARE_FETCH_RE.test(line)) { + violations.push({ + rule: 36, + file: posix, + line: idx + 1, + message: `calls fetch() directly in a network-facing directory. Route outbound requests through safeFetch()/assertUrlSafeToFetch() from "#shared/safe-fetch.js" so private/link-local/metadata hosts and unsafe redirects are rejected. If this URL is fully hardcoded and non-sensitive, refactor so the fetch lives outside these directories.`, + }); + } + }); +} + // ---------- Rule 13: spread-ternary object composition ---------- /** Matches `...( ? {} : { ... })` or the mirrored form. */ @@ -846,7 +889,6 @@ async function checkRule34PhaseBoundary() { return violations; } - /** * @returns {Promise>} */ @@ -1024,6 +1066,7 @@ async function main() { rule27: /** @type {Violation[]} */ ([]), rule28: /** @type {Violation[]} */ ([]), rule33: /** @type {Violation[]} */ ([]), + rule36: /** @type {Violation[]} */ ([]), symlinks: /** @type {string[]} */ ([]), }; @@ -1112,6 +1155,9 @@ async function main() { // Rule 34 violations.push(...(await checkRule34PhaseBoundary())); + // Rule 36 + violations.push(...state.rule36); + if (violations.length === 0) { process.stdout.write("[eve:guard:invariants] ok — all mechanical lints passed.\n"); return; From 28b435b960b1c5927ee3e6c270f718df2a764677 Mon Sep 17 00:00:00 2001 From: dnukumamras Date: Thu, 9 Jul 2026 13:09:02 -0700 Subject: [PATCH 2/2] fix(eve): bound safeFetch body reads and close IPv6-tunneled SSRF gaps Address PR review: - safeFetch tied its timeout timer and abort listener to header arrival, so the caller's later readBodyTextSafe was neither bounded by timeoutMs nor cancellable by the caller signal. Hand cleanup off to the returned body's lifetime (unref'd timer for the never-read case) so both keep applying through the body read. - normalizeAddress only unwrapped dotted `::ffff:` IPv4, so private/metadata IPv4 written as IPv4-compatible (`[::169.254.169.254]`), 6to4, NAT64, or SIIT-translated IPv6 slipped past the reserved-range blocklist. Parse the IPv6 to bytes and extract the embedded IPv4 for every such form, preserving `::` and `::1`. Co-Authored-By: Claude Opus 4.8 Signed-off-by: dnukumamras --- .../eve/src/shared/network-address.test.ts | 20 ++++ packages/eve/src/shared/network-address.ts | 95 +++++++++++++++++-- packages/eve/src/shared/safe-fetch.test.ts | 22 +++++ packages/eve/src/shared/safe-fetch.ts | 63 ++++++++++-- 4 files changed, 186 insertions(+), 14 deletions(-) diff --git a/packages/eve/src/shared/network-address.test.ts b/packages/eve/src/shared/network-address.test.ts index d404f77e3..e1e98bfa2 100644 --- a/packages/eve/src/shared/network-address.test.ts +++ b/packages/eve/src/shared/network-address.test.ts @@ -59,6 +59,18 @@ describe("isReservedIpAddress", () => { } }); + it("blocks private/metadata IPv4 tunneled through alternate IPv6 encodings", () => { + for (const host of [ + "[::169.254.169.254]", // IPv4-compatible → cloud metadata + "[::10.0.0.1]", // IPv4-compatible → RFC1918 + "2002:c0a8:0101::", // 6to4 → 192.168.1.1 + "64:ff9b::169.254.169.254", // NAT64 → cloud metadata + "[::ffff:0:0a00:0001]", // SIIT-translated → 10.0.0.1 + ]) { + expect(isReservedIpAddress(host), host).toBe(true); + } + }); + it("allows public addresses, loopback, and plain hostnames", () => { for (const host of [ "8.8.8.8", @@ -80,6 +92,14 @@ describe("normalizeAddress", () => { expect(normalizeAddress(" example.com ")).toBe("example.com"); }); + it("unwraps IPv4 tunneled through alternate IPv6 forms and preserves ::/::1", () => { + expect(normalizeAddress("[::169.254.169.254]")).toBe("169.254.169.254"); // compatible + expect(normalizeAddress("2002:c0a8:0101::")).toBe("192.168.1.1"); // 6to4 + expect(normalizeAddress("[::ffff:0:7f00:1]")).toBe("127.0.0.1"); // SIIT loopback + expect(normalizeAddress("[::1]")).toBe("::1"); // loopback preserved, not 0.0.0.1 + expect(normalizeAddress("[::]")).toBe("::"); // unspecified preserved + }); + it("leaves a plain hostname or bare IPv4 unchanged", () => { expect(normalizeAddress("api.example.com")).toBe("api.example.com"); expect(normalizeAddress("10.0.0.1")).toBe("10.0.0.1"); diff --git a/packages/eve/src/shared/network-address.ts b/packages/eve/src/shared/network-address.ts index d97ba98dd..7066b1dde 100644 --- a/packages/eve/src/shared/network-address.ts +++ b/packages/eve/src/shared/network-address.ts @@ -27,11 +27,90 @@ reservedRanges.addAddress("::", "ipv6"); // unspecified reservedRanges.addSubnet("fc00::", 7, "ipv6"); // unique-local reservedRanges.addSubnet("fe80::", 10, "ipv6"); // link-local +/** + * Expands an IPv6 literal to its 16 bytes, or `undefined` if it is not a valid + * IPv6 address. Handles `::` compression and a trailing dotted-IPv4 tail + * (`::ffff:1.2.3.4`). + */ +function ipv6ToBytes(address: string): number[] | undefined { + if (isIP(address) !== 6) { + return undefined; + } + let text = address.toLowerCase(); + + // Rewrite a trailing dotted IPv4 (e.g. `::ffff:1.2.3.4`) as its two hex + // groups so the rest can be parsed as a pure-hex IPv6. + const dotted = /(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})$/u.exec(text); + if (dotted !== null) { + const o = (dotted[1] as string).split(".").map(Number); + const hi = ((o[0]! << 8) | o[1]!).toString(16); + const lo = ((o[2]! << 8) | o[3]!).toString(16); + text = `${text.slice(0, text.length - (dotted[1] as string).length)}${hi}:${lo}`; + } + + const halves = text.split("::"); + const parse = (part: string): number[] => + part === "" ? [] : part.split(":").map((g) => Number.parseInt(g, 16)); + const head = parse(halves[0] ?? ""); + const tail = halves.length > 1 ? parse(halves[1] ?? "") : []; + const groups = + halves.length > 1 + ? [...head, ...Array(8 - head.length - tail.length).fill(0), ...tail] + : head; + if (groups.length !== 8 || groups.some((g) => Number.isNaN(g) || g < 0 || g > 0xffff)) { + return undefined; + } + return groups.flatMap((g) => [(g >> 8) & 0xff, g & 0xff]); +} + +/** + * Extracts the embedded IPv4 from an IPv6 byte array for every well-known form + * that tunnels an IPv4 address (mapped `::ffff:0:0/96`, SIIT-translated, + * IPv4-compatible `::/96`, 6to4 `2002::/16`, NAT64 `64:ff9b::/96`), or + * `undefined` when none applies. Without this, a private/metadata IPv4 written + * in one of these IPv6 forms sails past the IPv4 blocklist as an opaque IPv6. + */ +function embeddedIpv4(bytes: number[]): string | undefined { + const low = `${bytes[12]}.${bytes[13]}.${bytes[14]}.${bytes[15]}`; + const zero = (start: number, end: number): boolean => + bytes.slice(start, end).every((b) => b === 0); + + // 6to4 (`2002:AABB:CCDD::/16`) tunnels A.B.C.D in bytes 2-5. + if (bytes[0] === 0x20 && bytes[1] === 0x02) { + return `${bytes[2]}.${bytes[3]}.${bytes[4]}.${bytes[5]}`; + } + // NAT64 well-known prefix `64:ff9b::/96`. + if ( + bytes[0] === 0x00 && + bytes[1] === 0x64 && + bytes[2] === 0xff && + bytes[3] === 0x9b && + zero(4, 12) + ) { + return low; + } + // IPv4-mapped `::ffff:a.b.c.d` (`ffff` at bytes 10-11). + if (zero(0, 10) && bytes[10] === 0xff && bytes[11] === 0xff) { + return low; + } + // SIIT-translated `::ffff:0:a.b.c.d` (`ffff` at bytes 8-9, bytes 10-11 zero). + if (zero(0, 8) && bytes[8] === 0xff && bytes[9] === 0xff && bytes[10] === 0 && bytes[11] === 0) { + return low; + } + // IPv4-compatible `::/96` (deprecated). Skip `::` and `::1` — a leading + // embedded octet of 0 is that reserved/loopback space, handled elsewhere. + if (zero(0, 12) && bytes[12] !== 0) { + return low; + } + return undefined; +} + /** * Reduces a URL host or address literal to a bare, classifiable address: - * trims, strips IPv6 brackets and any zone index (`%eth0`), and unwraps - * IPv4-mapped IPv6 (`::ffff:a.b.c.d`) so the IPv4 ranges apply. Plain hostnames - * pass through unchanged. Shared with the SSRF fetch guard so URL hosts are + * trims, strips IPv6 brackets and any zone index (`%eth0`), and unwraps any + * IPv6 form that tunnels an IPv4 address (mapped, SIIT, IPv4-compatible, 6to4, + * NAT64) down to that IPv4 so the IPv4 ranges apply. Plain hostnames pass + * through unchanged. Shared with the SSRF fetch guard so URL hosts are * normalized identically here and before a DNS lookup. */ export function normalizeAddress(host: string): string { @@ -39,11 +118,11 @@ export function normalizeAddress(host: string): string { const zoneIndex = withoutBrackets.indexOf("%"); const withoutZone = zoneIndex === -1 ? withoutBrackets : withoutBrackets.slice(0, zoneIndex); - // Unwrap IPv4-mapped IPv6 (`::ffff:169.254.169.254`) so the IPv4 ranges apply. - if (withoutZone.toLowerCase().startsWith("::ffff:")) { - const candidate = withoutZone.slice("::ffff:".length); - if (isIP(candidate) === 4) { - return candidate; + const bytes = ipv6ToBytes(withoutZone); + if (bytes !== undefined) { + const v4 = embeddedIpv4(bytes); + if (v4 !== undefined && isIP(v4) === 4) { + return v4; } } diff --git a/packages/eve/src/shared/safe-fetch.test.ts b/packages/eve/src/shared/safe-fetch.test.ts index 17ea1a850..f2db4f92e 100644 --- a/packages/eve/src/shared/safe-fetch.test.ts +++ b/packages/eve/src/shared/safe-fetch.test.ts @@ -177,6 +177,28 @@ describe("safeFetch", () => { safeFetch("https://example.com/", { lookup: PUBLIC, fetch: fetchImpl, timeoutMs: 10 }), ).rejects.toThrow(/timed out/); }); + + it("keeps the timeout bounding the body read after the response is returned", async () => { + // Headers arrive immediately but the body never completes; the timeout must + // still fire during the caller's readBodyTextSafe, not be cleared on return. + const fetchImpl = vi.fn( + async (_input, init) => + new Response( + new ReadableStream({ + start(controller) { + init?.signal?.addEventListener("abort", () => controller.error(init.signal?.reason)); + }, + }), + { status: 200 }, + ), + ); + const res = await safeFetch("https://example.com/", { + lookup: PUBLIC, + fetch: fetchImpl, + timeoutMs: 20, + }); + await expect(readBodyTextSafe(res)).rejects.toThrow(/timed out/); + }); }); describe("readBodyTextSafe", () => { diff --git a/packages/eve/src/shared/safe-fetch.ts b/packages/eve/src/shared/safe-fetch.ts index 2c243150a..e8f461c35 100644 --- a/packages/eve/src/shared/safe-fetch.ts +++ b/packages/eve/src/shared/safe-fetch.ts @@ -289,6 +289,9 @@ export async function safeFetch( new Error(withLabel(assertOptions.label, `request timed out after ${timeoutMs}ms.`)), ); }, timeoutMs); + // Don't let a pending timeout keep the process alive on the rare path where + // the returned body is never read (cleanup is otherwise tied to the body). + timer.unref?.(); const onExternalAbort = () => controller.abort(externalSignal?.reason); if (externalSignal !== undefined) { @@ -299,6 +302,13 @@ export async function safeFetch( } } + const cleanup = () => { + clearTimeout(timer); + if (externalSignal !== undefined) { + externalSignal.removeEventListener("abort", onExternalAbort); + } + }; + let currentMethod = method ?? "GET"; let currentBody = body; const currentHeaders = new Headers(headers); @@ -314,7 +324,10 @@ export async function safeFetch( }); if (!isRedirectStatus(response.status)) { - return response; + // The timeout and abort signal must keep bounding and cancelling the + // caller's body read, so hand cleanup off to the response body's + // lifetime rather than firing it now that headers have arrived. + return bindResponseCleanup(response, cleanup); } if (hop >= maxRedirects) { await drainBody(response); @@ -361,14 +374,52 @@ export async function safeFetch( currentUrl = nextUrl; } - } finally { - clearTimeout(timer); - if (externalSignal !== undefined) { - externalSignal.removeEventListener("abort", onExternalAbort); - } + } catch (error) { + cleanup(); + throw error; } } +/** + * Returns `response` with cleanup deferred until its body settles: reading to + * completion, erroring, or cancelling runs {@link cleanup}. This keeps the + * request timeout and abort signal live across the caller's body read (a plain + * `Response` hands the body off without them otherwise). Bodyless responses + * clean up immediately. + */ +function bindResponseCleanup(response: Response, cleanup: () => void): Response { + if (response.body === null) { + cleanup(); + return response; + } + const reader = response.body.getReader(); + const stream = new ReadableStream({ + async pull(controller) { + try { + const { done, value } = await reader.read(); + if (done) { + cleanup(); + controller.close(); + return; + } + controller.enqueue(value); + } catch (error) { + cleanup(); + controller.error(error); + } + }, + cancel(reason) { + cleanup(); + return reader.cancel(reason); + }, + }); + return new Response(stream, { + headers: response.headers, + status: response.status, + statusText: response.statusText, + }); +} + /** * Reads a `Response` body as UTF-8 text, aborting the connection and throwing * once `maxBytes` is exceeded so an oversized body cannot exhaust memory.