From bc4fd98b57d970d7c90a3e849aea1fe4ca35d335 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Tue, 4 Aug 2026 11:50:31 -0700 Subject: [PATCH 1/2] Revert "Cache DNS guard resolutions and harden the outbound URL guard (#1524)" This reverts commit 6a924dd98de916d6ff8cea2329bf672f149b64f4. --- .../core/api/src/server/scoped-executor.ts | 17 +- packages/core/sdk/src/host-internal.ts | 1 - .../core/sdk/src/hosted-http-client.test.ts | 1548 +---------------- packages/core/sdk/src/hosted-http-client.ts | 774 ++------- 4 files changed, 173 insertions(+), 2167 deletions(-) diff --git a/packages/core/api/src/server/scoped-executor.ts b/packages/core/api/src/server/scoped-executor.ts index 7e195c25d..ea0e33ce6 100644 --- a/packages/core/api/src/server/scoped-executor.ts +++ b/packages/core/api/src/server/scoped-executor.ts @@ -43,7 +43,11 @@ import { type ExecutorConfig, type StorageFailure, } from "@executor-js/sdk"; -import { makeHostedHttp, touchSubject } from "@executor-js/sdk/host-internal"; +import { + makeHostedFetch, + makeHostedHttpClientLayer, + touchSubject, +} from "@executor-js/sdk/host-internal"; import { DbProvider } from "./executor-fuma-db"; @@ -265,10 +269,8 @@ export const makeScopedExecutor = < const hostedHttpOptions = { allowLocalNetwork: config.allowLocalNetwork, }; - // One resolution cache behind both adapters: the plugins take the raw - // fetch and the SDK takes the layer, so building them separately would - // resolve every hostname twice per session. - const { fetch: hostedFetch, httpClientLayer } = makeHostedHttp(hostedHttpOptions); + const httpClientLayer = makeHostedHttpClientLayer(hostedHttpOptions); + const hostedFetch = makeHostedFetch(hostedHttpOptions); // The org id is the tenant (catalog partition); the account id is the acting // subject (drives `owner: "user"` rows). `organizationName` is no longer part @@ -361,15 +363,14 @@ export const makePlatformExecutor = ( Effect.withSpan("executor.platform.plugins.init"), ); const hostedHttpOptions = { allowLocalNetwork: config.allowLocalNetwork }; - const platformHttp = makeHostedHttp(hostedHttpOptions); return yield* createExecutor({ tenant: Tenant.make(organizationId), db, blobs, plugins, - httpClientLayer: platformHttp.httpClientLayer, - fetch: platformHttp.fetch, + httpClientLayer: makeHostedHttpClientLayer(hostedHttpOptions), + fetch: makeHostedFetch(hostedHttpOptions), onElicitation: "accept-all", platformView: true, }).pipe(Effect.withSpan("executor.platform.create_executor")); diff --git a/packages/core/sdk/src/host-internal.ts b/packages/core/sdk/src/host-internal.ts index 658843258..e62cb15bb 100644 --- a/packages/core/sdk/src/host-internal.ts +++ b/packages/core/sdk/src/host-internal.ts @@ -32,7 +32,6 @@ export { HostedOutboundRequestBlocked, makeHostedFetch, - makeHostedHttp, makeHostedHttpClientLayer, type HostedHttpClientOptions, } from "./hosted-http-client"; diff --git a/packages/core/sdk/src/hosted-http-client.test.ts b/packages/core/sdk/src/hosted-http-client.test.ts index 20fc5295f..61f4590e0 100644 --- a/packages/core/sdk/src/hosted-http-client.test.ts +++ b/packages/core/sdk/src/hosted-http-client.test.ts @@ -1,575 +1,42 @@ import { describe, expect, it } from "@effect/vitest"; -import { Effect, Layer, Result } from "effect"; -import { FetchHttpClient, HttpClient, HttpClientRequest } from "effect/unstable/http"; -import { createServer, type Server } from "node:http"; +import { Effect, Predicate, Result } from "effect"; +import { HttpClient, HttpClientRequest } from "effect/unstable/http"; import { type HostedHostnameResolver, makeHostedFetch, - makeHostedHttp, makeHostedHttpClientLayer, validateHostedOutboundUrl, } from "./hosted-http-client"; -const publicResolver: HostedHostnameResolver = async () => [{ address: "93.184.216.34" }]; - -// A stub `fetch` accepts any init, so the shape of what the adapter hands the -// transport is exactly what a stub cannot check — undici rejects a streamed -// body whose init omits `duplex`, and no fake will ever say so. This drives a -// real server so the transport's own requirements are the assertion. -const withServer = ( - f: (input: { - readonly baseUrl: string; - readonly received: Array<{ readonly method: string; readonly body: string }>; - }) => Promise, -) => - new Promise((resolve, reject) => { - const received: Array<{ method: string; body: string }> = []; - const server: Server = createServer((request, response) => { - const chunks: Array = []; - request.on("data", (chunk: Buffer) => chunks.push(chunk)); - request.on("end", () => { - received.push({ - method: request.method ?? "", - body: Buffer.concat(chunks).toString("utf8"), - }); - response.statusCode = 200; - response.end("ok"); - }); - }); - - server.listen(0, "127.0.0.1", () => { - const address = server.address(); - if (!address || typeof address === "string") { - server.close(); - // oxlint-disable-next-line executor/no-promise-reject, executor/no-error-constructor -- boundary: Node listen callback is adapted into the test Promise failure path - reject(new Error("Server did not bind to a TCP port")); - return; - } - f({ baseUrl: `http://127.0.0.1:${address.port}`, received }) - .then(resolve, reject) - .finally(() => server.close()); - }); - }); - -// Reproduces the raw Promise rejection node:dns/promises produces at the -// adapter boundary under test. The rejection value is deliberately opaque: -// the guard discards it and maps every failure to the same tagged error, so -// carrying a getaddrinfo code here would imply a distinction that is not made. -const nodeDnsRejection = (): Promise => - // oxlint-disable-next-line executor/no-promise-reject -- boundary: fixture mirrors the raw Promise rejection node:dns produces - Promise.reject("getaddrinfo failure"); - -// The guard error is thrown inside the fetch adapter, so the HttpClient -// surfaces it wrapped in a RequestError whose cause is the blocked error. -const expectBlocked = (result: Result.Result, reason: string) => { - expect(result).toMatchObject({ - _tag: "Failure", - failure: { cause: { _tag: "HostedOutboundRequestBlocked", reason } }, - }); -}; +const publicResolver: HostedHostnameResolver = async () => [ + { address: "93.184.216.34", family: 4 }, +]; describe("hosted outbound HTTP client", () => { it.effect("allows public HTTP and HTTPS URLs", () => Effect.gen(function* () { - yield* validateHostedOutboundUrl("https://example.com/openapi.json", { - resolveHostname: publicResolver, - }); - yield* validateHostedOutboundUrl("http://example.com/graphql", { - resolveHostname: publicResolver, - }); - }), - ); - - it.effect("rejects non-HTTP protocols", () => - Effect.gen(function* () { - for (const url of ["file:///etc/passwd", "data:text/plain,x", "gopher://example.com/"]) { - const error = yield* validateHostedOutboundUrl(url).pipe(Effect.flip); - expect(error).toMatchObject({ - _tag: "HostedOutboundRequestBlocked", - reason: "Only HTTP and HTTPS outbound requests are allowed", - }); - } + yield* validateHostedOutboundUrl("https://example.com/openapi.json"); + yield* validateHostedOutboundUrl("http://example.com/graphql"); }), ); - // One representative per arm of the IPv4 blocklist, so deleting any range - // turns a test red: this-network, loopback, RFC1918 x3, CGNAT, link-local, - // 192.0.0.0/24, benchmark, multicast. it.effect("rejects local and private network URLs", () => Effect.gen(function* () { for (const url of [ "http://localhost:3000", - // The subdomain form is what a dev server or local reverse proxy - // actually hands out, and it never reaches the resolved-address check. - "http://app.localhost/", - // WHATWG keeps every trailing dot, so a hostname stripped of only one - // stays unequal to "localhost" and skips the whole name check while - // still resolving to the same host. - "http://localhost../", - "http://0.0.0.0/", "http://127.0.0.1:3000", "http://10.0.0.1/openapi.json", - "http://100.64.0.1/", - "http://169.254.1.1/", "http://172.16.0.1/graphql", - "http://192.0.0.1/", "http://192.168.1.10/mcp", - "http://198.18.0.1/", - "http://224.0.0.1/", - // The far edge of every range spanning more than one second octet. - // Without these, narrowing a range to the single value tested above - // leaves the suite green while a live private destination becomes - // reachable — the guard would still look covered. - "http://100.127.255.255/", - "http://172.31.255.255/", - "http://198.19.255.255/", - // 224.0.0.0-255.255.255.255 is one arm spanning four /4s: with only - // 224.0.0.1 above, narrowing it to `a === 224` — or to multicast plus - // one value — unblocks reserved 240.0.0.0/4 and the broadcast address - // with the suite still green. - "http://240.0.0.1/", - "http://255.255.255.255/", - ]) { - // The public resolver pins WHICH rule blocked: were these to slip - // past the pre-resolution check, resolution would succeed with a - // public address and the URL would be allowed, failing the test. - const error = yield* validateHostedOutboundUrl(url, { - resolveHostname: publicResolver, - }).pipe(Effect.flip); - expect(error).toMatchObject({ - _tag: "HostedOutboundRequestBlocked", - reason: "Local and private network addresses are not allowed", - }); - } - // The second entry is the same name with a second root dot appended: - // the resolver treats it identically, so stripping only one dot would - // let the metadata endpoint through by adding one character. - for (const url of [ "http://169.254.169.254/latest/meta-data/", - "http://metadata.google.internal../computeMetadata/v1/", - // The metadata block runs ahead of the allowLocalNetwork gate, so its - // invariant is "never reachable" — and a comparison against the one - // dotted-decimal spelling cannot hold that. Every IPv6 form carrying - // the same destination has to take the same block, or the local and - // desktop hosts reach the endpoint by writing the address differently. - // Each of these normalizes to a distinct hostname — the WHATWG parser - // rewrites a dotted quad inside a literal, so ::ffff:169.254.169.254 - // and ::ffff:a9fe:a9fe are one input, not two. - "http://[::ffff:169.254.169.254]/latest/meta-data/", - "http://[::169.254.169.254]/latest/meta-data/", - "http://[::ffff:0:a9fe:a9fe]/latest/meta-data/", - "http://[2002:a9fe:a9fe::]/latest/meta-data/", - "http://[64:ff9b::169.254.169.254]/latest/meta-data/", - ]) { - const metadataError = yield* validateHostedOutboundUrl(url, { - // allowLocalNetwork isolates the metadata arm: without it the - // local/private check would block the name first and the test would - // pass whether or not the trailing dots were handled. - allowLocalNetwork: true, - resolveHostname: publicResolver, - }).pipe(Effect.flip); - expect(metadataError).toMatchObject({ - _tag: "HostedOutboundRequestBlocked", - reason: "Metadata service addresses are not allowed", - }); - } - }), - ); - - it.effect("rejects native IPv6 loopback, link-local, ULA and multicast URLs", () => - Effect.gen(function* () { - for (const url of [ - "http://[::1]/", - "http://[::]/", - "http://[fe80::1]/", - "http://[fd00::1]/", - "http://[ff02::1]/", - // Both classifiers match a masked range, so testing only its most - // canonical member leaves the mask width free: fc00::/7 tightened to - // fd00::/8, or fe80::/10 to the exact word, would unblock the other - // half of each range with the suite still green. These pin the width. - "http://[fc00::1]/", - "http://[febf::1]/", - // Site-local (fec0::/10) is deprecated but still routed by older - // stacks and still handed out by some equipment, and it sits in the - // half of fe80::/9 that a link-local-only mask leaves public. - "http://[fec0::1]/", - "http://[feff::1]/", ]) { const error = yield* validateHostedOutboundUrl(url).pipe(Effect.flip); - expect(error).toMatchObject({ - _tag: "HostedOutboundRequestBlocked", - reason: "Local and private network addresses are not allowed", - }); - } - }), - ); - - // Every prefix that carries an IPv4 destination in its low 32 bits must be - // classified by that destination. Reading the first non-empty group instead - // would let "::169.254.1.1" — which starts with six zero words, not with - // 0xa9fe — straight through to the link-local range. - // - // These deliberately avoid 169.254.169.254: the metadata check runs ahead of - // this one and would answer for them, leaving the local/private classifier - // free to break with the suite green. - it.effect("rejects IPv6 literals that embed a private IPv4 address", () => - Effect.gen(function* () { - for (const url of [ - "http://[::169.254.1.1]/", - "http://[::127.0.0.1]/", - "http://[::10.0.0.1]/", - "http://[64:ff9b::169.254.1.1]/", - "http://[64:ff9b::127.0.0.1]/", - // RFC 6052 IPv4-translatable (::ffff:0:0:0/96), 6to4 (2002::/16), and - // RFC 8215 local-use NAT64 (64:ff9b:1::/48) each reach an embedded or - // locally translated IPv4 destination too. Being address literals they - // skip the resolved-address check entirely, so classifying them by - // their own prefix is the whole guard, not one layer of it. - "http://[::ffff:0:7f00:1]/", - "http://[::ffff:0:a9fe:101]/", - "http://[2002:a9fe:101::]/", - "http://[2002:7f00:1::]/", - "http://[64:ff9b:1::a9fe:a9fe]/", - ]) { - const error = yield* validateHostedOutboundUrl(url, { - resolveHostname: publicResolver, - }).pipe(Effect.flip); - expect(error).toMatchObject({ - _tag: "HostedOutboundRequestBlocked", - reason: "Local and private network addresses are not allowed", - }); - } - }), - ); - - // The mirror of the case above: classification reads the leading word, so a - // public address keeps being allowed even when a *later* word spells a - // blocked prefix — "2001:db8::fe80" is public, not link-local — and a - // v4-compatible address to a public destination is allowed like its - // v4-mapped equivalent. - it.effect("allows IPv6 literals whose leading word is public", () => - Effect.gen(function* () { - yield* validateHostedOutboundUrl("http://[2001:db8::fe80]/", { - resolveHostname: publicResolver, - }); - yield* validateHostedOutboundUrl("http://[::93.184.216.34]/", { - resolveHostname: publicResolver, - }); - // The added prefixes are classified by their embedded destination, not - // blocked wholesale — 93.184.216.34 is 5db8:d822. - yield* validateHostedOutboundUrl("http://[::ffff:0:5db8:d822]/", { - resolveHostname: publicResolver, - }); - yield* validateHostedOutboundUrl("http://[2002:5db8:d822::]/", { - resolveHostname: publicResolver, - }); - }), - ); - - // A resolved address the classifiers cannot decode is not evidence that the - // destination is public — it is the guard admitting it cannot tell. Treating - // it as "not private" would route every address these parsers reject - // straight past the one check that exists to stop it. - it.effect("rejects resolved addresses it cannot classify", () => - Effect.gen(function* () { - for (const address of ["not-an-address", "10.0.0.1.5", "1:2:3:4:5:6:7:8:9", "::gggg"]) { - const error = yield* validateHostedOutboundUrl("https://api.example/x", { - resolveHostname: async () => [{ address }], - }).pipe(Effect.flip); - - expect(error).toMatchObject({ - _tag: "HostedOutboundRequestBlocked", - reason: "Resolved address is local or private", - }); - } - }), - ); - - // Where the parsers disagree with the platform resolver about what an - // address means, the guard reads one destination and the connection dials - // another. Both forms below are decoded by the parsers, not rejected by - // them, so they reach the classifiers wearing a public-looking leading word - // — the exact failure the "cannot classify" test above cannot catch. - // - // A dotted quad is legal only at the end of the whole address; permitting - // one at the end of the head half of a compressed literal puts 0x7f00 in - // the leading word, where no prefix or mask matches it. And a leading zero - // means octal to inet_aton, so "0177.0.0.1" is 127.0.0.1 to the resolver - // and 177.0.0.1 to a decimal-only parser. - it.effect("rejects resolved addresses whose syntax the platform reads differently", () => - Effect.gen(function* () { - for (const address of [ - "127.0.0.1::1", - "192.168.1.1::", - "10.0.0.1::0", - "169.254.169.254::1", - "0177.0.0.1", - ]) { - const error = yield* validateHostedOutboundUrl("https://api.example/x", { - resolveHostname: async () => [{ address }], - }).pipe(Effect.flip); - - expect(error).toMatchObject({ - _tag: "HostedOutboundRequestBlocked", - reason: "Resolved address is local or private", - }); - } - - // Guards against over-blocking: the trailing quad is still legal where - // it belongs, so tightening the rule must not reject the v4-compatible - // and v4-mapped forms of an ordinary public address. - for (const address of ["::93.184.216.34", "::ffff:93.184.216.34"]) { - yield* validateHostedOutboundUrl("https://api.example/x", { - resolveHostname: async () => [{ address }], - }); - } - }), - ); - - // node:dns can hand back a link-local address carrying its interface scope. - // The zone identifier is not part of the address and must not defeat the - // classification of the address it is attached to. - it.effect("rejects resolved IPv6 addresses that carry a zone identifier", () => - Effect.gen(function* () { - const error = yield* validateHostedOutboundUrl("https://api.example/x", { - resolveHostname: async () => [{ address: "fe80::1%eth0" }], - }).pipe(Effect.flip); - - expect(error).toMatchObject({ - _tag: "HostedOutboundRequestBlocked", - reason: "Resolved address is local or private", - }); - - // The blocked case alone cannot tell "zone stripped, then classified - // link-local" from "parse failed, unclassifiable" — both reach the same - // reason. A public zoned address is only allowed when the zone is - // genuinely stripped before classification. - yield* validateHostedOutboundUrl("https://api.example/x", { - resolveHostname: async () => [{ address: "2001:db8::1%eth0" }], - }); - }), - ); - - // The tagged error is the guard's contract: a malformed URL has to arrive as - // a typed block, not as a raw TypeError escaping as an untyped defect. - it.effect("rejects input that is not a URL", () => - Effect.gen(function* () { - const error = yield* validateHostedOutboundUrl("not a url").pipe(Effect.flip); - // The url field is asserted, not just the reason: it is the only part of - // the error telling an operator which destination was refused. - expect(error).toMatchObject({ - _tag: "HostedOutboundRequestBlocked", - reason: "URL is invalid", - url: "not a url", - }); - }), - ); - - // The root dot is the same name to a resolver but a different string, so an - // exact-match blocklist that skipped canonicalization would be defeated by - // appending one character. This check sits above the allowLocalNetwork gate - // deliberately — it is the one rule that holds even in self-host mode, which - // is exactly where the bypass would be reachable. - it.effect("blocks metadata hostnames written with a trailing root dot", () => - Effect.gen(function* () { - for (const hostname of ["metadata.google.internal.", "metadata.", "instance-data."]) { - const error = yield* validateHostedOutboundUrl(`http://${hostname}/latest/meta-data/`, { - allowLocalNetwork: true, - resolveHostname: async () => [{ address: "169.254.169.254" }], - }).pipe(Effect.flip); - - expect(error).toMatchObject({ - _tag: "HostedOutboundRequestBlocked", - reason: "Metadata service addresses are not allowed", - }); - } - }), - ); - - // The name blocklist covers the published metadata hostnames, which is not - // the same as covering the endpoint: any name at all can carry an A record - // for it. Since the metadata rule holds regardless of allowLocalNetwork, the - // resolved-address check has to run in that mode too — gating the lookup on - // the flag left the endpoint one DNS record away on exactly the hosts that - // set it, with the name never resolved and so never classified. - it.effect("blocks a public name that resolves to the metadata endpoint", () => - Effect.gen(function* () { - for (const allowLocalNetwork of [false, true]) { - const error = yield* validateHostedOutboundUrl( - "http://harmless.example/latest/meta-data/", - { - allowLocalNetwork, - resolveHostname: async () => [{ address: "169.254.169.254" }], - }, - ).pipe(Effect.flip); - - expect(error).toMatchObject({ - _tag: "HostedOutboundRequestBlocked", - reason: "Metadata service addresses are not allowed", - }); - } - }), - ); - - // The permissive mode allows local destinations by name; only the metadata - // rule applies to what comes back. Were the resolved-address check to reject - // local answers here, the flag would stop doing the one thing it exists for. - it.effect("allows a name resolving to a local address when local network is allowed", () => - Effect.gen(function* () { - yield* validateHostedOutboundUrl("http://nas.local/openapi.json", { - allowLocalNetwork: true, - resolveHostname: async () => [{ address: "192.168.1.10" }], - }); - }), - ); - - // A name this resolver cannot see is fatal in the default mode and not in - // the permissive one: a self-host behind a tunnel resolves names remotely, - // so the transport reaches destinations the local resolver cannot, and that - // deployment is what the flag is for. Nothing resolved means nothing can be - // the metadata endpoint, and a name that truly does not exist fails at the - // transport with its own error. - it.effect("does not block on an unresolvable name when local network is allowed", () => - Effect.gen(function* () { - const options = { - // oxlint-disable-next-line executor/no-promise-reject, executor/no-error-constructor -- boundary: a resolver is a Promise-returning platform seam, and a rejected lookup is what node:dns actually does - resolveHostname: async () => Promise.reject(new Error("EAI_AGAIN")), - }; - yield* validateHostedOutboundUrl("http://tunnel.internal/mcp", { - ...options, - allowLocalNetwork: true, - }); - - const error = yield* validateHostedOutboundUrl("http://tunnel.internal/mcp", options).pipe( - Effect.flip, - ); - expect(error).toMatchObject({ - _tag: "HostedOutboundRequestBlocked", - reason: "Hostname could not be resolved", - }); - }), - ); - - // The named metadata hostnames are the classic cloud SSRF vector; they are - // blocked before resolution, so no resolver is consulted. - it.effect("rejects metadata service hostnames without resolving them", () => - Effect.gen(function* () { - for (const url of [ - "http://metadata.google.internal/computeMetadata/v1/", - "http://metadata/", - "http://instance-data/", - ]) { - const resolved: string[] = []; - const error = yield* validateHostedOutboundUrl(url, { - resolveHostname: async (hostname) => { - resolved.push(hostname); - return [{ address: "93.184.216.34" }]; - }, - }).pipe(Effect.flip); - expect(resolved).toEqual([]); - expect(error).toMatchObject({ - _tag: "HostedOutboundRequestBlocked", - reason: "Metadata service addresses are not allowed", - }); + expect(Predicate.isTagged(error, "HostedOutboundRequestBlocked")).toBe(true); } }), ); - it.effect("rejects hostnames that resolve to no addresses", () => - Effect.gen(function* () { - const error = yield* validateHostedOutboundUrl("https://api.example/openapi.json", { - resolveHostname: async () => [], - }).pipe(Effect.flip); - - expect(error).toMatchObject({ - _tag: "HostedOutboundRequestBlocked", - reason: "Hostname did not resolve to an address", - }); - }), - ); - - // An empty answer is a failure, not a successful "no addresses". Only the - // error channel takes the zero failure TTL, so caching it as a success would - // stamp the full 60s window on one transient empty answer and blackhole the - // hostname for its duration — something the uncached code could never do. - it("re-resolves after a resolver returns no addresses", async () => { - let attempts = 0; - const hostedFetch = makeHostedFetch({ - fetch: async () => new Response("ok"), - resolveHostname: async () => { - attempts++; - return attempts === 1 ? [] : [{ address: "93.184.216.34" }]; - }, - }); - - await expect(hostedFetch("https://api.example/x")).rejects.toMatchObject({ - _tag: "HostedOutboundRequestBlocked", - reason: "Hostname did not resolve to an address", - }); - - // The second request must reach the resolver again rather than replaying - // the empty answer out of the cache. - const recovered = await hostedFetch("https://api.example/x"); - expect(recovered.status).toBe(200); - expect(attempts).toBe(2); - }); - - // Every host needs both adapters — plugins take the raw fetch, the SDK takes - // the layer — so building them separately would resolve each hostname twice - // per session and run two TTL windows side by side. - it.effect("shares one resolution cache between the fetch and the client layer", () => - Effect.gen(function* () { - let resolutions = 0; - const fakeFetch: typeof globalThis.fetch = async () => new Response("ok", { status: 200 }); - const hosted = makeHostedHttp({ - fetch: fakeFetch, - resolveHostname: async () => { - resolutions++; - return [{ address: "93.184.216.34" }]; - }, - }); - - yield* Effect.promise(() => hosted.fetch("https://api.example/one")); - yield* Effect.gen(function* () { - const client = yield* HttpClient.HttpClient; - return yield* client.execute(HttpClientRequest.get("https://api.example/two")); - }).pipe(Effect.provide(hosted.httpClientLayer)); - - // The layer's request reuses what the fetch already resolved. - expect(resolutions).toBe(1); - }), - ); - - // A failed lookup is stale under the zero TTL but still occupies a slot, so - // a run of dead hostnames would otherwise push the working ones out and put - // the resolver back on the hot path — the stall this cache exists to remove. - it("does not let failed lookups evict cached ones", async () => { - const resolved: Array = []; - const hostedFetch = makeHostedFetch({ - fetch: async () => new Response("ok"), - dnsCacheCapacity: 4, - resolveHostname: async (hostname) => { - resolved.push(hostname); - if (hostname.startsWith("dead")) return []; - return [{ address: "93.184.216.34" }]; - }, - }); - - await hostedFetch("https://live.example/x"); - expect(resolved).toEqual(["live.example"]); - - for (let index = 0; index < 20; index++) { - await expect(hostedFetch(`https://dead${index}.example/x`)).rejects.toMatchObject({ - _tag: "HostedOutboundRequestBlocked", - }); - } - - await hostedFetch("https://live.example/x"); - expect(resolved.filter((hostname) => hostname === "live.example")).toHaveLength(1); - }); - it.effect("rejects IPv4-mapped IPv6 URLs for local and private networks", () => Effect.gen(function* () { for (const url of [ @@ -577,110 +44,39 @@ describe("hosted outbound HTTP client", () => { "http://[::ffff:10.0.0.1]/openapi.json", "http://[::ffff:172.16.0.1]/graphql", "http://[::ffff:192.168.1.10]/mcp", - "http://[::ffff:169.254.1.1]/", + "http://[::ffff:169.254.169.254]/latest/meta-data/", ]) { - const error = yield* validateHostedOutboundUrl(url, { - resolveHostname: publicResolver, - }).pipe(Effect.flip); - expect(error).toMatchObject({ - _tag: "HostedOutboundRequestBlocked", - reason: "Local and private network addresses are not allowed", - }); + const error = yield* validateHostedOutboundUrl(url).pipe(Effect.flip); + expect(Predicate.isTagged(error, "HostedOutboundRequestBlocked")).toBe(true); } }), ); - // Guards against over-blocking, not under-blocking: an embedded-IPv4 prefix - // is only a container, so it must be classified by the address it carries - // rather than treated as suspicious in itself. Deleting embeddedIpv4 does - // NOT turn this red — these prefixes start with 0x0000/0x0064 and match none - // of the first-word masks — so this pins the allow behavior against a future - // change that blanket-blocks the prefix. The block path embeddedIpv4 does - // carry is covered by "rejects IPv6 literals that embed a private IPv4". - it.effect("allows IPv4-mapped IPv6 URLs for public addresses", () => - validateHostedOutboundUrl("http://[::ffff:93.184.216.34]/openapi.json"), - ); - it.effect("can allow local network URLs explicitly", () => - validateHostedOutboundUrl("http://127.0.0.1:3000", { allowLocalNetwork: true }), - ); - - it.effect("rejects hostnames that resolve to local or private addresses", () => - Effect.gen(function* () { - const error = yield* validateHostedOutboundUrl("https://api.example/openapi.json", { - resolveHostname: async () => [{ address: "10.0.0.10" }], - }).pipe(Effect.flip); - - expect(error).toMatchObject({ - _tag: "HostedOutboundRequestBlocked", - reason: "Resolved address is local or private", - }); - }), - ); - - // A resolver returns whatever string the platform gives it, uncompressed - // forms included, and nothing normalizes it on the way in — unlike a URL - // literal, which the WHATWG parser always compresses. So this is the only - // path that reaches the eight-group branch of parseIpv6; a URL can never - // exercise it. - it.effect("classifies an uncompressed IPv6 address returned by the resolver", () => Effect.gen(function* () { - const error = yield* validateHostedOutboundUrl("https://api.example/openapi.json", { - resolveHostname: async () => [{ address: "0:0:0:0:0:0:0:1" }], - }).pipe(Effect.flip); - - expect(error).toMatchObject({ - _tag: "HostedOutboundRequestBlocked", - reason: "Resolved address is local or private", + yield* validateHostedOutboundUrl("http://127.0.0.1:3000", { + allowLocalNetwork: true, }); }), ); - // A hostname may hold several A records, and an attacker only needs one of - // them to point inside. Every address the name resolves to has to clear the - // check, not just the first one the resolver happened to order first — - // checking only `addresses[0]` leaves the rest of this file green. - it.effect("rejects a hostname whose records mix a public address with a private one", () => + it.effect("rejects hostnames that resolve to local or private addresses", () => Effect.gen(function* () { const error = yield* validateHostedOutboundUrl("https://api.example/openapi.json", { - resolveHostname: async () => [{ address: "93.184.216.34" }, { address: "169.254.1.1" }], + resolveHostname: async () => [{ address: "10.0.0.10", family: 4 }], }).pipe(Effect.flip); - expect(error).toMatchObject({ - _tag: "HostedOutboundRequestBlocked", - reason: "Resolved address is local or private", - }); - }), - ); - - // "beef.cafe" is a registrable DNS name spelled entirely in hex digits and - // dots. It must go through resolution like any other hostname; treating it - // as an address literal would skip the resolved-address check entirely. - it.effect("resolves hex-digit hostnames instead of treating them as IP literals", () => - Effect.gen(function* () { - const resolved: string[] = []; - const error = yield* validateHostedOutboundUrl("https://beef.cafe/x", { - resolveHostname: async (hostname) => { - resolved.push(hostname); - return [{ address: "169.254.1.1" }]; - }, - }).pipe(Effect.flip); - - expect(resolved).toEqual(["beef.cafe"]); - expect(error).toMatchObject({ - _tag: "HostedOutboundRequestBlocked", - reason: "Resolved address is local or private", - }); + expect(Predicate.isTagged(error, "HostedOutboundRequestBlocked")).toBe(true); }), ); it.effect("checks DNS before the first fetch call", () => Effect.gen(function* () { let calls = 0; - const fakeFetch: typeof globalThis.fetch = async () => { + const fakeFetch: typeof globalThis.fetch = (async () => { calls++; return new Response("unexpected", { status: 200 }); - }; + }) as typeof globalThis.fetch; const result = yield* Effect.gen(function* () { const client = yield* HttpClient.HttpClient; @@ -689,870 +85,39 @@ describe("hosted outbound HTTP client", () => { Effect.provide( makeHostedHttpClientLayer({ fetch: fakeFetch, - resolveHostname: async () => [{ address: "169.254.1.1" }], + resolveHostname: async () => [{ address: "169.254.169.254", family: 4 }], }), ), Effect.result, ); - expectBlocked(result, "Resolved address is local or private"); + expect(Result.isFailure(result)).toBe(true); expect(calls).toBe(0); }), ); it("applies the DNS guard to fetch callers", async () => { let calls = 0; - const underlying: typeof globalThis.fetch = async () => { - calls++; - return new Response("unexpected", { status: 200 }); - }; const hostedFetch = makeHostedFetch({ - fetch: underlying, - resolveHostname: async () => [{ address: "10.0.0.20" }], - }); - - await expect(hostedFetch("https://api.example/token")).rejects.toMatchObject({ - _tag: "HostedOutboundRequestBlocked", - reason: "Resolved address is local or private", - }); - expect(calls).toBe(0); - }); - - it("fails the request when the hostname cannot be resolved", async () => { - const hostedFetch = makeHostedFetch({ - fetch: async () => new Response("unexpected"), - resolveHostname: () => nodeDnsRejection(), - }); - - await expect(hostedFetch("https://api.example/token")).rejects.toMatchObject({ - _tag: "HostedOutboundRequestBlocked", - reason: "Hostname could not be resolved", - }); - }); - - // Omitting resolveHostname must not silently drop the resolved-address - // check: the default adapter is the one that ships. The label is longer - // than the 63 octets RFC 1035 allows, so getaddrinfo rejects it structurally - // without emitting a query — the assertion cannot be flipped by a resolver - // that wildcards NXDOMAIN, and it touches no network. - it("uses the node:dns resolver when none is supplied", async () => { - let calls = 0; - const hostedFetch = makeHostedFetch({ - fetch: async () => { + fetch: (async () => { calls++; - return new Response("unexpected"); - }, + return new Response("unexpected", { status: 200 }); + }) as typeof globalThis.fetch, + resolveHostname: async () => [{ address: "10.0.0.20", family: 4 }], }); - const overlongLabel = `${"a".repeat(300)}.example.com`; - await expect(hostedFetch(`https://${overlongLabel}/x`)).rejects.toMatchObject({ + await expect(hostedFetch("https://api.example/token")).rejects.toMatchObject({ _tag: "HostedOutboundRequestBlocked", - reason: "Hostname could not be resolved", }); expect(calls).toBe(0); }); - it("resolves each hostname once per adapter within the cache window", async () => { - const resolved: string[] = []; - const hostedFetch = makeHostedFetch({ - fetch: async () => new Response("ok"), - resolveHostname: async (hostname) => { - resolved.push(hostname); - return [{ address: "93.184.216.34" }]; - }, - }); - - await hostedFetch("https://api.example/one"); - await hostedFetch("https://api.example/two"); - await hostedFetch("https://cdn.example/blob"); - - expect(resolved).toEqual(["api.example", "cdn.example"]); - }); - - // The cache is per adapter on purpose (see withResolutionCache): the MCP host - // builds one per session, so a verdict reached in one session must not decide - // requests in another. Hoisting the Cache to module scope — the shape that - // makes this shared — keeps every other cache test green. - it("keeps one adapter's cached resolutions out of another's", async () => { - const makeCounting = () => { - const resolved: string[] = []; - const hostedFetch = makeHostedFetch({ - fetch: async () => new Response("ok"), - resolveHostname: async (hostname) => { - resolved.push(hostname); - return [{ address: "93.184.216.34" }]; - }, - }); - return { hostedFetch, resolved }; - }; - - const first = makeCounting(); - const second = makeCounting(); - - await first.hostedFetch("https://api.example/one"); - await second.hostedFetch("https://api.example/two"); - - expect(first.resolved).toEqual(["api.example"]); - expect(second.resolved).toEqual(["api.example"]); - }); - - it("re-resolves a hostname after the cache window elapses", async () => { - const resolved: string[] = []; - const hostedFetch = makeHostedFetch({ - fetch: async () => new Response("ok"), - resolveHostname: async (hostname) => { - resolved.push(hostname); - return [{ address: "93.184.216.34" }]; - }, - dnsCacheTtlMillis: 20, - }); - - await hostedFetch("https://api.example/one"); - await new Promise((resolve) => setTimeout(resolve, 40)); - await hostedFetch("https://api.example/two"); - - expect(resolved).toEqual(["api.example", "api.example"]); - }); - - it("evicts the oldest hostname when the cache capacity is exceeded", async () => { - const resolved: string[] = []; - const hostedFetch = makeHostedFetch({ - fetch: async () => new Response("ok"), - resolveHostname: async (hostname) => { - resolved.push(hostname); - return [{ address: "93.184.216.34" }]; - }, - dnsCacheCapacity: 1, - }); - - await hostedFetch("https://api.example/one"); - await hostedFetch("https://cdn.example/blob"); - await hostedFetch("https://api.example/two"); - - expect(resolved).toEqual(["api.example", "cdn.example", "api.example"]); - }); - - it("does not cache failed resolutions", async () => { - let attempts = 0; - const hostedFetch = makeHostedFetch({ - fetch: async () => new Response("ok"), - resolveHostname: async () => { - attempts++; - if (attempts === 1) return nodeDnsRejection(); - return [{ address: "93.184.216.34" }]; - }, - }); - - await expect(hostedFetch("https://api.example/one")).rejects.toMatchObject({ - _tag: "HostedOutboundRequestBlocked", - }); - const response = await hostedFetch("https://api.example/two"); - - expect(response.status).toBe(200); - expect(attempts).toBe(2); - }); - - it("shares one in-flight resolution across concurrent requests", async () => { - let resolutions = 0; - const gate = Promise.withResolvers(); - const entered = Promise.withResolvers(); - const hostedFetch = makeHostedFetch({ - fetch: async () => new Response("ok"), - resolveHostname: async () => { - resolutions++; - entered.resolve(); - await gate.promise; - return [{ address: "93.184.216.34" }]; - }, - // A zero TTL is what makes the count mean what this test says it means. - // Under the default 60s window a settled entry is reusable, so - // `resolutions === 1` is also satisfied by the second request simply - // arriving after the first finished and reading the cache — the test - // would keep passing while no longer testing in-flight sharing at all. - // With no settled entry to find, one resolution can only be a join. - dnsCacheTtlMillis: 0, - }); - - // The second request is issued only once the first is provably inside the - // resolver, then given a turn of the event loop to reach the cache before - // the gate opens: issuance order alone does not put it there. - const first = hostedFetch("https://api.example/one"); - await entered.promise; - const second = hostedFetch("https://api.example/two"); - await new Promise((resolve) => setTimeout(resolve, 0)); - gate.resolve(); - await Promise.all([first, second]); - - expect(resolutions).toBe(1); - }); - - // Concurrent callers share one resolution, so aborting one must not cancel - // the lookup the others are waiting on. The survivor's outcome is the whole - // point: before the shared lookup was detached from the requesting fiber it - // failed with an untyped "All fibers interrupted without error". - it("keeps concurrent requests alive when one caller aborts", async () => { - const gate = Promise.withResolvers(); - const entered = Promise.withResolvers(); - const hostedFetch = makeHostedFetch({ - fetch: async () => new Response("ok"), - resolveHostname: async () => { - entered.resolve(); - await gate.promise; - return [{ address: "93.184.216.34" }]; - }, - }); - - const aborter = new AbortController(); - const aborted = hostedFetch("https://api.example/one", { signal: aborter.signal }); - const survivor = hostedFetch("https://api.example/two"); - // Gate on the resolver itself having been entered, not on a microtask tick: - // the lookup sits behind runPromise plus forkDetach, so a bare `await` does - // not reach it and the abort would land before the work it must not kill. - await entered.promise; - aborter.abort(); - gate.resolve(); - - // Asserting the shape, not merely that it threw: the regression this test - // exists to exclude also throws, so a bare toThrow() would accept it. - await expect(aborted).rejects.toMatchObject({ name: "AbortError" }); - // The survivor completing is the assertion. Its 200 does not prove it was - // still waiting on the shared lookup — under the default TTL a caller that - // arrived after the first resolution settled would read the cache and get - // the same 200 — so no resolution count is asserted here. That the lookup - // survives an abort at all is what fails without forkDetach: the survivor - // rejects with "All fibers interrupted without error". In-flight sharing - // itself is pinned by the zero-TTL test above. - expect((await survivor).status).toBe(200); - }); - - // The guard's verdict is decided by the failure's cause, not by whether the - // caller's signal happens to be aborted. Branching on `signal.aborted` - // instead reclassifies a real SSRF block as an AbortError — which callers - // read as a transport failure and retry — and keeps every test above green. - it("reports a blocked URL as a guard failure even when the caller has aborted", async () => { - const hostedFetch = makeHostedFetch({ - fetch: async () => new Response("ok"), - resolveHostname: async () => [{ address: "93.184.216.34" }], - }); - - const aborter = new AbortController(); - const blocked = hostedFetch("http://169.254.169.254/latest/meta-data", { - signal: aborter.signal, - }); - aborter.abort(); - - await expect(blocked).rejects.toMatchObject({ - _tag: "HostedOutboundRequestBlocked", - reason: "Metadata service addresses are not allowed", - }); - }); - - // A pending cache entry carries no expiry, so without a bound on the lookup - // itself one stuck resolution would leave every later request for that - // hostname joining the same dead Deferred for the adapter's lifetime. - it("recovers from a resolution that never settles", async () => { - let attempts = 0; - const hostedFetch = makeHostedFetch({ - fetch: async () => new Response("ok"), - resolveHostname: async () => { - attempts++; - if (attempts === 1) return new Promise(() => {}); - return [{ address: "93.184.216.34" }]; - }, - dnsResolutionTimeoutMillis: 20, - }); - - await expect(hostedFetch("https://api.example/one")).rejects.toMatchObject({ - _tag: "HostedOutboundRequestBlocked", - reason: "Hostname could not be resolved", - }); - const response = await hostedFetch("https://api.example/two"); - - expect(response.status).toBe(200); - expect(attempts).toBe(2); - }); - - it("stops following redirects at the maxRedirects bound", async () => { - let calls = 0; - const alwaysRedirect: typeof globalThis.fetch = async () => { - calls++; - return new Response(null, { status: 302, headers: { location: "/next" } }); - }; - const hostedFetch = makeHostedFetch({ - fetch: alwaysRedirect, - resolveHostname: publicResolver, - maxRedirects: 3, - }); - - // The exhausted budget is a guard decision, so it arrives as one. Handing - // back the raw 302 would be indistinguishable from a successful final - // response to a caller that asked to follow redirects, and it may then - // follow the Location itself with no guard in front of it. - await expect(hostedFetch("https://api.example/start")).rejects.toMatchObject({ - _tag: "HostedOutboundRequestBlocked", - reason: "Redirect limit of 3 exceeded", - }); - expect(calls).toBe(4); - }); - - it("rejects redirects whose Location header is not a valid URL", async () => { - const hostedFetch = makeHostedFetch({ - fetch: async () => new Response(null, { status: 302, headers: { location: "http://[bad" } }), - resolveHostname: publicResolver, - }); - - await expect(hostedFetch("https://api.example/start")).rejects.toMatchObject({ - _tag: "HostedOutboundRequestBlocked", - reason: "Redirect target is not a valid URL", - }); - }); - - // Every redirect rule below is downstream of one instruction: the adapter - // must ask the transport not to follow 3xx itself. A real fetch left on its - // default follows internally, so hops 2..n are never re-validated and none of - // the stripping runs — the guard sees one request and an already-followed - // response. The fakes here cannot show that, so the mode is asserted directly. - it("tells the transport not to follow redirects on any hop", async () => { - const modes: Array = []; - const underlying: typeof globalThis.fetch = async (input, init) => { - modes.push(init?.redirect); - if (String(input) === "https://api.example/start") { - return new Response(null, { status: 302, headers: { location: "/moved" } }); - } - return new Response("ok", { status: 200 }); - }; - const hostedFetch = makeHostedFetch({ fetch: underlying, resolveHostname: publicResolver }); - - await hostedFetch("https://api.example/start"); - - expect(modes).toEqual(["manual", "manual"]); - }); - - it("demotes POST to GET and drops the body on a 302 redirect", async () => { - const seen: Array<{ - url: string; - method: string; - hasBody: boolean; - contentType: string | null; - contentLength: string | null; - contentEncoding: string | null; - contentLanguage: string | null; - }> = []; - const underlying: typeof globalThis.fetch = async (input, init) => { - const url = String(input); - const headers = new Headers(init?.headers); - seen.push({ - url, - method: init?.method ?? "GET", - hasBody: init?.body !== undefined && init?.body !== null, - contentType: headers.get("content-type"), - contentLength: headers.get("content-length"), - contentEncoding: headers.get("content-encoding"), - contentLanguage: headers.get("content-language"), - }); - if (url === "https://api.example/start") { - return new Response(null, { status: 302, headers: { location: "/see-here" } }); - } - return new Response("ok", { status: 200 }); - }; - const hostedFetch = makeHostedFetch({ fetch: underlying, resolveHostname: publicResolver }); - - await hostedFetch("https://api.example/start", { - method: "POST", - headers: { - "content-type": "application/x-www-form-urlencoded", - "content-length": "25", - "content-encoding": "gzip", - "content-language": "en", - }, - body: "client_secret=supersecret", - }); - - expect(seen).toHaveLength(2); - // One representative per content header the demotion drops: a bodyless GET - // still advertising content-length: 25 reads as a truncated request, and - // the upstream 4xx gets blamed on the caller. - expect(seen[1]).toMatchObject({ - url: "https://api.example/see-here", - method: "GET", - hasBody: false, - contentType: null, - contentLength: null, - contentEncoding: null, - contentLanguage: null, - }); - }); - - // Integrations render credentials into arbitrary header names, so a - // cross-origin hop keeps only a known-safe set. A 302 demotes the POST - // first, so by the time the hop is taken there is no body left to leak. - it("strips non-safelisted headers and the body on cross-origin redirects", async () => { - const seen: Array<{ - url: string; - apiKey: string | null; - accept: string | null; - acceptLanguage: string | null; - userAgent: string | null; - hasBody: boolean; - }> = []; - const underlying: typeof globalThis.fetch = async (input, init) => { - const url = String(input); - const headers = new Headers(init?.headers); - seen.push({ - url, - apiKey: headers.get("x-api-key"), - accept: headers.get("accept"), - acceptLanguage: headers.get("accept-language"), - userAgent: headers.get("user-agent"), - hasBody: init?.body !== undefined && init?.body !== null, - }); - if (url === "https://api.example/start") { - return new Response(null, { - status: 302, - headers: { location: "https://evil.example/collect" }, - }); - } - return new Response("ok", { status: 200 }); - }; - const hostedFetch = makeHostedFetch({ fetch: underlying, resolveHostname: publicResolver }); - - await hostedFetch("https://api.example/start", { - method: "POST", - headers: { - "x-api-key": "rendered-credential", - accept: "application/json", - "accept-language": "en-GB", - "user-agent": "executor-test", - }, - body: "client_secret=supersecret", - }); - - expect(seen).toHaveLength(2); - expect(seen[0]).toMatchObject({ apiKey: "rendered-credential", hasBody: true }); - // One representative per safelisted header: dropping user-agent on a hop - // changes what the upstream serves — bot rules, content negotiation — so - // the safelist is a contract, not an implementation detail. - expect(seen[1]).toMatchObject({ - url: "https://evil.example/collect", - apiKey: null, - accept: "application/json", - acceptLanguage: "en-GB", - userAgent: "executor-test", - hasBody: false, - }); - }); - - // The scrub is not only about headers. `referrer` and `credentials` are - // credential-carrying init fields, and the platform renders `referrer` into - // a real Referer header — the full URL, query included, under - // referrerPolicy "unsafe-url". An OAuth callback whose query holds a token - // would hand that token to the redirect target, past a header safelist that - // looks like it covered this. - it("drops credential-carrying init fields on cross-origin redirects", async () => { - const seen: Array = []; - const underlying: typeof globalThis.fetch = async (input, init) => { - seen.push(init); - return String(input).startsWith("https://api.example/start") - ? new Response(null, { - status: 302, - headers: { location: "https://evil.example/collect" }, - }) - : new Response("ok", { status: 200 }); - }; - const hostedFetch = makeHostedFetch({ fetch: underlying, resolveHostname: publicResolver }); - - await hostedFetch( - new Request("https://api.example/start?code=authorization-code", { - method: "GET", - referrer: "https://api.example/start?code=authorization-code", - referrerPolicy: "unsafe-url", - credentials: "include", - }), - ); - - expect(seen).toHaveLength(2); - expect(seen[0]).toMatchObject({ - referrer: "https://api.example/start?code=authorization-code", - }); - expect(seen[1]?.referrer).toBeUndefined(); - expect(seen[1]?.referrerPolicy).toBeUndefined(); - expect(seen[1]?.credentials).toBeUndefined(); - }); - - // The guard pins the transport to `redirect: "manual"` for its own reasons, - // which must not be mistaken for the caller's intent. A caller inspecting a - // 3xx — the standard OAuth authorize probe — needs the unfollowed response - // with its Location header intact. - it("returns the unfollowed 3xx when the caller asks for manual redirects", async () => { - const seen: Array = []; - const underlying: typeof globalThis.fetch = async (input) => { - seen.push(String(input)); - return String(input) === "https://api.example/start" - ? new Response(null, { status: 302, headers: { location: "https://api.example/moved" } }) - : new Response("followed", { status: 200 }); - }; - const hostedFetch = makeHostedFetch({ fetch: underlying, resolveHostname: publicResolver }); - - const response = await hostedFetch("https://api.example/start", { redirect: "manual" }); - - expect(response.status).toBe(302); - expect(response.headers.get("location")).toBe("https://api.example/moved"); - // The hop was never taken, so the guard never saw the second URL. - expect(seen).toEqual(["https://api.example/start"]); - }); - - it("rejects a redirect when the caller asks for redirect: error", async () => { - const underlying: typeof globalThis.fetch = async (input) => - String(input) === "https://api.example/start" - ? new Response(null, { status: 302, headers: { location: "https://api.example/moved" } }) - : new Response("followed", { status: 200 }); - const hostedFetch = makeHostedFetch({ fetch: underlying, resolveHostname: publicResolver }); - - await expect( - hostedFetch("https://api.example/start", { redirect: "error" }), - ).rejects.toMatchObject({ - _tag: "HostedOutboundRequestBlocked", - reason: "Redirect received while the caller requested redirect: error", - }); - }); - - // The mode travels on a Request the same way it travels on an init. - it("honors a Request input's redirect mode", async () => { - const underlying: typeof globalThis.fetch = async (input) => - String(input) === "https://api.example/start" - ? new Response(null, { status: 302, headers: { location: "https://api.example/moved" } }) - : new Response("followed", { status: 200 }); - const hostedFetch = makeHostedFetch({ fetch: underlying, resolveHostname: publicResolver }); - - const response = await hostedFetch( - new Request("https://api.example/start", { redirect: "manual" }), - ); - - expect(response.status).toBe(302); - }); - - // `fetch(request, {method: undefined})` keeps the Request's method — the - // undefined member is absent, not an instruction to clear the field. Reading - // the Request into an init and spreading over it would invert that, sending - // a credential-bearing POST as an unauthenticated GET. - it("ignores explicitly undefined init members over a Request input", async () => { - const seen: Array<{ method: string; authorization: string | null }> = []; - const underlying: typeof globalThis.fetch = async (_input, init) => { - seen.push({ - method: init?.method ?? "GET", - authorization: new Headers(init?.headers).get("authorization"), - }); - return new Response("ok", { status: 200 }); - }; - const hostedFetch = makeHostedFetch({ fetch: underlying, resolveHostname: publicResolver }); - - await hostedFetch( - new Request("https://api.example/x", { - method: "POST", - headers: { authorization: "Bearer token" }, - }), - { method: undefined, headers: undefined }, - ); - - expect(seen).toEqual([{ method: "POST", authorization: "Bearer token" }]); - }); - - // `integrity` is a check on what comes back, not a secret handed to the - // target, so the scrub that drops credentials must not drop it too: the hop - // that lands on a different origin is precisely the one whose bytes the - // caller has least reason to trust unverified. - it("keeps subresource integrity across a cross-origin redirect", async () => { - const seen: Array = []; - const underlying: typeof globalThis.fetch = async (input, init) => { - seen.push(init?.integrity); - return String(input) === "https://api.example/start" - ? new Response(null, { - status: 302, - headers: { location: "https://cdn.example/asset.js" }, - }) - : new Response("ok", { status: 200 }); - }; - const hostedFetch = makeHostedFetch({ fetch: underlying, resolveHostname: publicResolver }); - - await hostedFetch(new Request("https://api.example/start", { integrity: "sha256-abc" })); - - expect(seen).toEqual(["sha256-abc", "sha256-abc"]); - }); - - // Origin is scheme plus host, and the scheme half is the half that matters - // here: comparing hosts alone would call an https -> http hop to the same - // name same-origin and replay the credential headers over plaintext. - it("treats a scheme downgrade to the same host as cross-origin", async () => { - const seen: Array<{ url: string; apiKey: string | null }> = []; - const underlying: typeof globalThis.fetch = async (input, init) => { - const url = String(input); - seen.push({ url, apiKey: new Headers(init?.headers).get("x-api-key") }); - if (url === "https://api.example/start") { - return new Response(null, { - status: 302, - headers: { location: "http://api.example/moved" }, - }); - } - return new Response("ok", { status: 200 }); - }; - const hostedFetch = makeHostedFetch({ fetch: underlying, resolveHostname: publicResolver }); - - await hostedFetch("https://api.example/start", { - headers: { "x-api-key": "rendered-credential" }, - }); - - expect(seen).toHaveLength(2); - expect(seen[1]).toMatchObject({ url: "http://api.example/moved", apiKey: null }); - }); - - // A 307/308 keeps the method, so stripping the body would send a bodyless - // POST the caller never wrote and blame the upstream for the resulting 4xx. - // The body cannot cross origins either, so the hop is refused outright. - it("refuses a cross-origin redirect that would replay a body", async () => { - let calls = 0; - const underlying: typeof globalThis.fetch = async (input) => { - calls++; - if (String(input) === "https://api.example/start") { - return new Response(null, { - status: 307, - headers: { location: "https://evil.example/collect" }, - }); - } - return new Response("unexpected", { status: 200 }); - }; - const hostedFetch = makeHostedFetch({ fetch: underlying, resolveHostname: publicResolver }); - - await expect( - hostedFetch("https://api.example/start", { - method: "POST", - headers: { "x-api-key": "rendered-credential" }, - body: "client_secret=supersecret", - }), - ).rejects.toMatchObject({ - _tag: "HostedOutboundRequestBlocked", - reason: "Cross-origin redirect cannot replay a request body", - }); - expect(calls).toBe(1); - }); - - // A stream reads once, so replaying the same object on the next hop sends an - // empty body and still returns 200 — the caller's upload silently truncated. - // The hop is refused rather than buffering every upload up front to make it - // replayable: FetchHttpClient passes a ReadableStream for every streamed - // body, so that buffer would land on every request, redirect or not. - it("refuses to replay a streamed body across a redirect", async () => { - const seen: Array = []; - const underlying: typeof globalThis.fetch = async (input, init) => { - seen.push(init?.body instanceof ReadableStream); - if (String(input) === "https://api.example/start") { - return new Response(null, { status: 308, headers: { location: "/moved" } }); - } - return new Response("ok"); - }; - const hostedFetch = makeHostedFetch({ fetch: underlying, resolveHostname: publicResolver }); - const stream = new ReadableStream({ - start(controller) { - controller.enqueue(new TextEncoder().encode("hello-streamed-body")); - controller.close(); - }, - }); - - await expect( - hostedFetch("https://api.example/start", { method: "POST", body: stream }), - ).rejects.toMatchObject({ - _tag: "HostedOutboundRequestBlocked", - reason: "Redirect cannot replay a streamed request body", - }); - // The first hop got the stream itself, unbuffered — the whole point of - // refusing the replay rather than materializing it. - expect(seen).toEqual([true]); - }); - - // The refusal is scoped to a replay. A 303 or 301 demotes to GET and drops - // the body first, so there is no stream left to replay and the redirect is - // followed normally — blocking those too would break every streamed upload - // whose upstream answers with a see-other. - it("follows a demoting redirect after a streamed body, since the body is dropped", async () => { - const seen: Array<{ url: string; method: string; hasBody: boolean }> = []; - const underlying: typeof globalThis.fetch = async (input, init) => { - const url = String(input); - seen.push({ - url, - method: init?.method ?? "GET", - hasBody: init?.body !== undefined && init?.body !== null, - }); - if (url === "https://api.example/start") { - return new Response(null, { status: 303, headers: { location: "/moved" } }); - } - return new Response("final-body"); - }; - const hostedFetch = makeHostedFetch({ fetch: underlying, resolveHostname: publicResolver }); - const stream = new ReadableStream({ - start(controller) { - controller.enqueue(new TextEncoder().encode("hello-streamed-body")); - controller.close(); - }, - }); - - const response = await hostedFetch("https://api.example/start", { - method: "POST", - body: stream, - }); - - expect(response.status).toBe(200); - // The body the last hop produced reaches the caller — the redirect loop - // returns that response rather than a synthesized or drained stand-in. - expect(await response.text()).toBe("final-body"); - expect(seen[1]).toMatchObject({ - url: "https://api.example/moved", - method: "GET", - hasBody: false, - }); - }); - - // One representative per arm of redirectDemotesToGet, mirroring the - // arm-coverage rule this file states for the IPv4 blocklist: 303 demotes any - // method, 301 demotes a non-GET, and 308 replays method and body untouched. - it("applies the right method semantics per redirect status", async () => { - const run = async (status: number, method: string) => { - const seen: Array<{ method: string; hasBody: boolean }> = []; - const underlying: typeof globalThis.fetch = async (input, init) => { - seen.push({ - method: init?.method ?? "GET", - hasBody: init?.body !== undefined && init?.body !== null, - }); - if (String(input) === "https://api.example/start") { - return new Response(null, { status, headers: { location: "/moved" } }); - } - return new Response("ok", { status: 200 }); - }; - const hostedFetch = makeHostedFetch({ fetch: underlying, resolveHostname: publicResolver }); - await hostedFetch( - "https://api.example/start", - method === "HEAD" ? { method } : { method, body: "secret=1" }, - ); - return seen[1]; - }; - - expect(await run(303, "POST")).toMatchObject({ method: "GET", hasBody: false }); - expect(await run(301, "POST")).toMatchObject({ method: "GET", hasBody: false }); - expect(await run(308, "POST")).toMatchObject({ method: "POST", hasBody: true }); - // The demotion exempts GET and HEAD: turning a HEAD probe into a GET makes - // the caller download a body it deliberately asked not to receive. - expect(await run(301, "HEAD")).toMatchObject({ method: "HEAD" }); - // 301/302 demote POST alone, which is where a "any non-GET" reading - // diverges from platform fetch and from every caller's expectation: a - // rewritten DELETE never deletes and a rewritten PUT never writes, and - // both hand back a 200 for a request that was never made. Verified against - // Node's own fetch against a real server, which keeps the method here. - expect(await run(301, "DELETE")).toMatchObject({ method: "DELETE", hasBody: true }); - expect(await run(302, "PUT")).toMatchObject({ method: "PUT", hasBody: true }); - // 303 is the mirror: it demotes any method except GET and HEAD. - expect(await run(303, "DELETE")).toMatchObject({ method: "GET", hasBody: false }); - expect(await run(303, "HEAD")).toMatchObject({ method: "HEAD" }); - }); - - it("preserves method and headers from a Request input across redirects", async () => { - const seen: Array<{ url: string; method: string; accept: string | null }> = []; - const underlying: typeof globalThis.fetch = async (input, init) => { - const url = String(input); - seen.push({ - url, - method: init?.method ?? "GET", - accept: new Headers(init?.headers).get("accept"), - }); - if (url === "https://api.example/start") { - return new Response(null, { status: 307, headers: { location: "/moved" } }); - } - return new Response("ok", { status: 200 }); - }; - const hostedFetch = makeHostedFetch({ fetch: underlying, resolveHostname: publicResolver }); - - const response = await hostedFetch( - new Request("https://api.example/start", { - method: "DELETE", - headers: { accept: "application/json" }, - }), - ); - - expect(response.status).toBe(200); - expect(seen).toHaveLength(2); - expect(seen[1]).toMatchObject({ - url: "https://api.example/moved", - method: "DELETE", - accept: "application/json", - }); - }); - - // A Request always exposes its body as a ReadableStream, whatever it was - // constructed from, so a 307/308 after one hits the same one-shot problem as - // an explicitly streamed body and is refused for the same reason. The - // alternative is not "replay it" — the bytes are gone once the first hop - // reads them — it is sending a bodyless POST under a 200 and calling that - // success. - // normalizeFetchInput turns every Request input into its ReadableStream - // body, and undici refuses a streamed body whose init omits `duplex`. Both - // of these reach the real transport as streams, so without it every - // body-bearing Request and every streamed upload throws before a byte - // leaves — a failure the whole rest of this file is structurally blind to. - it("sends body-bearing requests through a real transport", async () => { - await withServer(async ({ baseUrl, received }) => { - const hostedFetch = makeHostedFetch({ - allowLocalNetwork: true, - resolveHostname: publicResolver, - }); - - const fromRequest = await hostedFetch( - new Request(`${baseUrl}/from-request`, { method: "POST", body: "payload" }), - ); - expect(fromRequest.status).toBe(200); - // The adapter's primary contract, and the one a status-only assertion - // cannot see: the caller gets the upstream body back unread. Cancelling - // or draining it anywhere in the guard would leave the status intact and - // hand the caller nothing. - expect(await fromRequest.text()).toBe("ok"); - - const streamed = await hostedFetch(`${baseUrl}/streamed`, { - method: "PUT", - body: new ReadableStream({ - start(controller) { - controller.enqueue(new TextEncoder().encode("streamed")); - controller.close(); - }, - }), - }); - expect(streamed.status).toBe(200); - expect(await streamed.text()).toBe("ok"); - - expect(received).toEqual([ - { method: "POST", body: "payload" }, - { method: "PUT", body: "streamed" }, - ]); - }); - }); - - it("refuses to replay a Request input's body across a redirect", async () => { - const underlying: typeof globalThis.fetch = async (input) => - String(input) === "https://api.example/start" - ? new Response(null, { status: 307, headers: { location: "/moved" } }) - : new Response("ok", { status: 200 }); - const hostedFetch = makeHostedFetch({ fetch: underlying, resolveHostname: publicResolver }); - - await expect( - hostedFetch(new Request("https://api.example/start", { method: "POST", body: "payload" })), - ).rejects.toMatchObject({ - _tag: "HostedOutboundRequestBlocked", - url: "https://api.example/moved", - reason: "Redirect cannot replay a streamed request body", - }); - }); - it.effect("checks redirected URLs before following them", () => Effect.gen(function* () { let calls = 0; - const fakeFetch: typeof globalThis.fetch = async (input) => { + const fakeFetch: typeof globalThis.fetch = (async (input) => { calls++; - const url = String(input); + const url = input instanceof Request ? input.url : String(input); if (url === "https://public.example/start") { return new Response(null, { status: 302, @@ -1560,7 +125,7 @@ describe("hosted outbound HTTP client", () => { }); } return new Response("unexpected", { status: 200 }); - }; + }) as typeof globalThis.fetch; const result = yield* Effect.gen(function* () { const client = yield* HttpClient.HttpClient; return yield* client.execute(HttpClientRequest.get("https://public.example/start")); @@ -1571,7 +136,7 @@ describe("hosted outbound HTTP client", () => { Effect.result, ); - expectBlocked(result, "Local and private network addresses are not allowed"); + expect(Result.isFailure(result)).toBe(true); expect(calls).toBe(1); }), ); @@ -1583,8 +148,8 @@ describe("hosted outbound HTTP client", () => { authorization: string | null; cookie: string | null; }> = []; - const fakeFetch: typeof globalThis.fetch = async (input, init) => { - const url = String(input); + const fakeFetch: typeof globalThis.fetch = (async (input, init) => { + const url = input instanceof Request ? input.url : String(input); const headers = new Headers(init?.headers); seen.push({ url, @@ -1598,7 +163,7 @@ describe("hosted outbound HTTP client", () => { }); } return new Response("bytes", { status: 200 }); - }; + }) as typeof globalThis.fetch; const response = yield* Effect.gen(function* () { const client = yield* HttpClient.HttpClient; @@ -1635,8 +200,8 @@ describe("hosted outbound HTTP client", () => { it.effect("keeps credential headers on same-origin redirects", () => Effect.gen(function* () { const seen: Array<{ url: string; authorization: string | null }> = []; - const fakeFetch: typeof globalThis.fetch = async (input, init) => { - const url = String(input); + const fakeFetch: typeof globalThis.fetch = (async (input, init) => { + const url = input instanceof Request ? input.url : String(input); seen.push({ url, authorization: new Headers(init?.headers).get("authorization"), @@ -1648,7 +213,7 @@ describe("hosted outbound HTTP client", () => { }); } return new Response("ok", { status: 200 }); - }; + }) as typeof globalThis.fetch; const response = yield* Effect.gen(function* () { const client = yield* HttpClient.HttpClient; @@ -1672,40 +237,12 @@ describe("hosted outbound HTTP client", () => { }), ); - // The layer has two branches and the ambient one is what ships when a - // composition root provides FetchHttpClient.Fetch itself; leaving it - // unguarded would be a complete SSRF bypass on the live path. - it.effect("guards an ambient FetchHttpClient.Fetch when options.fetch is omitted", () => - Effect.gen(function* () { - let calls = 0; - const ambient: typeof globalThis.fetch = async () => { - calls++; - return new Response("unexpected", { status: 200 }); - }; - - const result = yield* Effect.gen(function* () { - const client = yield* HttpClient.HttpClient; - return yield* client.execute(HttpClientRequest.get("https://api.example/start")); - }).pipe( - Effect.provide( - makeHostedHttpClientLayer({ - resolveHostname: async () => [{ address: "169.254.1.1" }], - }).pipe(Layer.provide(Layer.succeed(FetchHttpClient.Fetch)(ambient))), - ), - Effect.result, - ); - - expectBlocked(result, "Resolved address is local or private"); - expect(calls).toBe(0); - }), - ); - it.effect("rejects cross-origin redirects to private addresses", () => Effect.gen(function* () { let calls = 0; - const fakeFetch: typeof globalThis.fetch = async (input) => { + const fakeFetch: typeof globalThis.fetch = (async (input) => { calls++; - const url = String(input); + const url = input instanceof Request ? input.url : String(input); if (url === "https://api.example/start") { return new Response(null, { status: 302, @@ -1713,7 +250,7 @@ describe("hosted outbound HTTP client", () => { }); } return new Response("unexpected", { status: 200 }); - }; + }) as typeof globalThis.fetch; const result = yield* Effect.gen(function* () { const client = yield* HttpClient.HttpClient; @@ -1725,14 +262,7 @@ describe("hosted outbound HTTP client", () => { Effect.result, ); - expectBlocked(result, "Metadata service addresses are not allowed"); - // On a redirect block the reported url is genuinely ambiguous — the - // request the caller made, or the hop that was actually refused. It is - // the hop: naming the original would point an operator at a URL that is - // fine and hide the destination the guard stopped. - expect(result).toMatchObject({ - failure: { cause: { url: "http://169.254.169.254/latest/meta-data/" } }, - }); + expect(Result.isFailure(result)).toBe(true); expect(calls).toBe(1); }), ); diff --git a/packages/core/sdk/src/hosted-http-client.ts b/packages/core/sdk/src/hosted-http-client.ts index 0673f242f..37f1ea427 100644 --- a/packages/core/sdk/src/hosted-http-client.ts +++ b/packages/core/sdk/src/hosted-http-client.ts @@ -1,4 +1,4 @@ -import { Cache, Cause, Duration, Effect, Exit, Fiber, Layer, Schema } from "effect"; +import { Effect, Layer, Schema } from "effect"; import { FetchHttpClient, HttpClient } from "effect/unstable/http"; export class HostedOutboundRequestBlocked extends Schema.TaggedErrorClass()( @@ -9,16 +9,13 @@ export class HostedOutboundRequestBlocked extends Schema.TaggedErrorClass Promise>; export interface HostedHttpClientOptions { @@ -26,26 +23,14 @@ export interface HostedHttpClientOptions { readonly maxRedirects?: number; readonly fetch?: typeof globalThis.fetch; readonly resolveHostname?: HostedHostnameResolver; - /** How long a successful guard resolution is reused, default 60s. */ - readonly dnsCacheTtlMillis?: number; - /** How many hostnames the guard resolution cache holds, default 256. */ - readonly dnsCacheCapacity?: number; - /** How long a single guard resolution may run before failing, default 60s. */ - readonly dnsResolutionTimeoutMillis?: number; } -// Octets are decimal-only and leading zeros are rejected. inet_aton reads a -// leading zero as octal, so accepting "0177.0.0.1" here would classify it as -// 177.0.0.1 (public) while the platform resolver dials 127.0.0.1 — the guard -// and the connection would disagree about the destination. WHATWG normalizes -// these forms, so the URL path never produces one, but a resolver answer -// reaches isAllowedResolvedAddress unnormalized. const parseIpv4 = (hostname: string): readonly [number, number, number, number] | null => { const parts = hostname.split("."); if (parts.length !== 4) return null; const parsed: number[] = []; for (const part of parts) { - if (!/^(?:0|[1-9]\d*)$/.test(part)) return null; + if (!/^\d+$/.test(part)) return null; const value = Number(part); if (!Number.isInteger(value) || value < 0 || value > 255) return null; parsed.push(value); @@ -53,53 +38,34 @@ const parseIpv4 = (hostname: string): readonly [number, number, number, number] return parsed as [number, number, number, number]; }; -// `allowTrailingQuad` is the caller's statement that this segment ends the -// whole address, not merely that it ends its own half. Deciding it from the -// segment alone would legalize a quad at the end of the head half of a -// compressed literal: "127.0.0.1::1" would parse, land 0x7f00 in the leading -// word where no prefix or mask matches it, and be classified public. -const parseIpv6Groups = (text: string, allowTrailingQuad: boolean): number[] | null => { - if (text === "") return []; - const parts = text.split(":"); - const groups: number[] = []; - for (const [index, part] of parts.entries()) { - // A trailing dotted quad is the only place IPv4 syntax is legal. - if (index === parts.length - 1 && part.includes(".")) { - if (!allowTrailingQuad) return null; - const dotted = parseIpv4(part); - if (!dotted) return null; - groups.push((dotted[0] << 8) | dotted[1], (dotted[2] << 8) | dotted[3]); - continue; - } - if (!/^[0-9a-f]{1,4}$/.test(part)) return null; - groups.push(Number.parseInt(part, 16)); +const parseIpv4MappedIpv6 = ( + hostname: string, +): readonly [number, number, number, number] | null => { + const prefix = "::ffff:"; + if (!hostname.startsWith(prefix)) return null; + const embedded = hostname.slice(prefix.length); + const dotted = parseIpv4(embedded); + if (dotted) return dotted; + + const parts = embedded.split(":"); + if (parts.length !== 2) return null; + + const words = parts.map((part) => Number.parseInt(part, 16)); + if ( + words.some( + (word, index) => + parts[index] === "" || + !/^[0-9a-f]+$/i.test(parts[index]) || + !Number.isInteger(word) || + word < 0 || + word > 0xffff, + ) + ) { + return null; } - return groups; -}; - -// Expands an IPv6 literal into its eight 16-bit words, resolving the "::" -// run to its true position. Classification has to work from the real words: -// the leading group of a compressed literal is not the leading word — -// "::127.0.0.1" starts with six zero words, not with 0x7f00 — so reading the -// first non-empty group instead lets loopback and metadata addresses past a -// guard whose whole purpose is blocking them. -const parseIpv6 = (hostname: string): ReadonlyArray | null => { - if (!hostname.includes(":")) return null; - // node:dns returns link-local addresses with their interface scope - // ("fe80::1%eth0"); the zone is not part of the address being classified. - const [head, tail, ...extra] = hostname.replace(/%.*$/, "").split("::"); - if (extra.length > 0) return null; - - // The head half ends the address only when there is no "::" after it. - const high = parseIpv6Groups(head, tail === undefined); - if (high === null) return null; - if (tail === undefined) return high.length === 8 ? high : null; - const low = parseIpv6Groups(tail, true); - if (low === null) return null; - const elided = 8 - high.length - low.length; - if (elided < 1) return null; - return [...high, ...Array(elided).fill(0), ...low]; + const [high, low] = words; + return [high >> 8, high & 0xff, low >> 8, low & 0xff]; }; const isBlockedIpv4 = ([a, b]: readonly [number, number, number, number]): boolean => @@ -114,247 +80,63 @@ const isBlockedIpv4 = ([a, b]: readonly [number, number, number, number]): boole (a === 198 && (b === 18 || b === 19)) || a >= 224; -const dotted = (high: number, low: number): readonly [number, number, number, number] => [ - high >> 8, - high & 0xff, - low >> 8, - low & 0xff, -]; - -// Prefixes that carry an IPv4 address at a fixed position, so the destination -// they actually reach is that address and not their own prefix: v4-mapped -// (::ffff:0:0/96), the deprecated v4-compatible form (::/96), RFC 6052's -// IPv4-translatable form (::ffff:0:0:0/96), the well-known NAT64 prefix -// (64:ff9b::/96), and 6to4 (2002::/16), which carries its address one word -// higher. Classifying by the prefix instead would let ::ffff:0:7f00:1 and -// 2002:a9fe:a9fe:: reach loopback and the metadata endpoint untouched. -const embeddedIpv4 = ( - words: ReadonlyArray, -): readonly [number, number, number, number] | null => { - const [w0, w1, w2, w3, w4, w5, w6, w7] = words; - const zeroThrough = (...parts: ReadonlyArray) => parts.every((word) => word === 0); - if (w0 === 0x2002) return dotted(w1, w2); - const isV4Mapped = zeroThrough(w0, w1, w2, w3, w4) && w5 === 0xffff; - const isV4Compatible = zeroThrough(w0, w1, w2, w3, w4, w5); - const isV4Translatable = zeroThrough(w0, w1, w2, w3, w5) && w4 === 0xffff; - const isNat64 = w0 === 0x0064 && w1 === 0xff9b && zeroThrough(w2, w3, w4, w5); - if (!isV4Mapped && !isV4Compatible && !isV4Translatable && !isNat64) return null; - return dotted(w6, w7); -}; - -// RFC 8215 reserves 64:ff9b:1::/48 for local-use NAT64. Where the IPv4 address -// sits inside one of these depends on the prefix length the local translator -// was configured with, which the address alone does not carry — so there is no -// embedded address to classify, and a translator that exists at all is by -// definition on the local network. -const isLocalUseNat64 = (words: ReadonlyArray): boolean => - words[0] === 0x0064 && words[1] === 0xff9b && words[2] === 0x0001; - -const isBlockedIpv6 = (words: ReadonlyArray): boolean => { - if (isLocalUseNat64(words)) return true; - const embedded = embeddedIpv4(words); - if (embedded) return isBlockedIpv4(embedded); - const leading = words[0]; - // fe80::/9 rather than fe80::/10: the upper half of it is site-local - // (fec0::/10), deprecated by RFC 3879 but still routed by stacks that - // predate the deprecation and still assigned by some equipment, so fec0::1 - // is a local address that the /10 mask let through as public. +const isBlockedIpv6 = (hostname: string): boolean => { + const normalized = hostname.toLowerCase(); + if ( + normalized === "::" || + normalized === "::1" || + normalized === "0:0:0:0:0:0:0:0" || + normalized === "0:0:0:0:0:0:0:1" + ) { + return true; + } + const firstWordText = normalized.split(":").find((part) => part.length > 0); + if (!firstWordText || !/^[0-9a-f]{1,4}$/.test(firstWordText)) return false; + const firstWord = Number.parseInt(firstWordText, 16); return ( - (leading & 0xff80) === 0xfe80 || (leading & 0xfe00) === 0xfc00 || (leading & 0xff00) === 0xff00 + (firstWord & 0xffc0) === 0xfe80 || + (firstWord & 0xfe00) === 0xfc00 || + (firstWord & 0xff00) === 0xff00 ); }; -// Every hostname check runs against this form, so each one sees the same name. -// The trailing root dot matters: "metadata.google.internal." is the same name -// to the resolver but a different string, and an exact-match blocklist that -// skipped this would be defeated by appending one character. -const canonicalHostname = (hostname: string): string => - hostname - .toLowerCase() - .replace(/^\[|\]$/g, "") - .replace(/\.+$/, ""); - -const isMetadataAddress = ([a, b, c, d]: readonly [number, number, number, number]): boolean => - a === 169 && b === 254 && c === 169 && d === 254; - -// The metadata endpoint is blocked unconditionally, ahead of the -// allowLocalNetwork gate, so the invariant is "never reachable" — which a -// string comparison cannot hold. It only ever matched the one dotted-decimal -// spelling, so under allowLocalNetwork (the local and desktop hosts) every -// IPv6 form carrying the same destination — ::ffff:169.254.169.254, the 6to4 -// 2002:a9fe:a9fe::, the NAT64 64:ff9b::169.254.169.254 — reached it untouched. -// The address decides now, by the same parsers the rest of the guard uses, so -// every spelling of it takes the same block. -const isBlockedMetadataAddress = (normalized: string): boolean => { - const ipv4 = parseIpv4(normalized); - if (ipv4) return isMetadataAddress(ipv4); - const ipv6 = parseIpv6(normalized); - if (!ipv6) return false; - const embedded = embeddedIpv4(ipv6); - return embedded !== null && isMetadataAddress(embedded); +const isBlockedMetadataHostname = (hostname: string): boolean => { + const normalized = hostname.toLowerCase(); + return ( + normalized === "metadata.google.internal" || + normalized === "metadata" || + normalized === "instance-data" || + normalized === "169.254.169.254" + ); }; -const isBlockedMetadataHostname = (normalized: string): boolean => - normalized === "metadata.google.internal" || - normalized === "metadata" || - normalized === "instance-data" || - isBlockedMetadataAddress(normalized); - -const isLocalOrPrivateHostname = (normalized: string): boolean => { +const isLocalOrPrivateHostname = (hostname: string): boolean => { + const normalized = hostname.toLowerCase().replace(/^\[|\]$/g, ""); if (normalized === "localhost" || normalized.endsWith(".localhost")) return true; const ipv4 = parseIpv4(normalized); if (ipv4) return isBlockedIpv4(ipv4); - const ipv6 = parseIpv6(normalized); - if (ipv6) return isBlockedIpv6(ipv6); - return false; + const mappedIpv4 = parseIpv4MappedIpv6(normalized); + if (mappedIpv4) return isBlockedIpv4(mappedIpv4); + return isBlockedIpv6(normalized); }; -// A resolved address is known to be an IP, so failing to decode it is not -// "this is a public name" — it is "the guard cannot classify this". Reusing -// isLocalOrPrivateHostname there would collapse both into `false` and let an -// address these parsers reject through the one check that exists to stop it. -const isAllowedResolvedAddress = (address: string): boolean => { - const normalized = address.toLowerCase().replace(/^\[|\]$/g, ""); - const ipv4 = parseIpv4(normalized); - if (ipv4) return !isBlockedIpv4(ipv4); - const ipv6 = parseIpv6(normalized); - if (ipv6) return !isBlockedIpv6(ipv6); - return false; +const isAddressLiteral = (hostname: string): boolean => { + const normalized = hostname.toLowerCase().replace(/^\[|\]$/g, ""); + return parseIpv4(normalized) !== null || /^[0-9a-f:.]+$/i.test(normalized); }; -// A URL hostname is an IP literal only in canonical shapes: the WHATWG parser -// rewrites every IPv4 form (hex, octal, integer) to dotted decimal, and an -// IPv6 literal always contains a colon. A character-class test is not enough -// here: registrable names spelled entirely in hex digits ("beef.cafe") must -// count as DNS names, or they would skip the resolved-address check below. -const isAddressLiteral = (normalized: string): boolean => - parseIpv4(normalized) !== null || normalized.includes(":"); - -// `empty` separates "the resolver answered, with nothing" from "the resolver -// did not answer". Both fail closed and both must stay out of the success -// cache, but they are different operator diagnoses, so the block reason keeps -// them apart rather than collapsing into one message. -class HostedHostnameResolutionFailed extends Schema.TaggedErrorClass()( - "HostedHostnameResolutionFailed", - { - hostname: Schema.String, - empty: Schema.Boolean, - }, -) {} - const resolveHostnameWithNodeDns: HostedHostnameResolver = async (hostname) => { const { lookup } = await import("node:dns/promises"); const addresses = await lookup(hostname, { all: true, verbatim: true }); - return addresses.map(({ address }) => ({ address })); -}; - -type GuardResolver = ( - hostname: string, -) => Effect.Effect, HostedHostnameResolutionFailed>; - -// An empty answer becomes a failure at the resolver boundary, not at the call -// site, so it reaches every caller the same way: the cached path needs it in -// the error channel for the zero failure TTL to drop it, and putting it here -// rather than inside the cache keeps the uncached path — the exported -// validateHostedOutboundUrl — from having a second copy of the rule. -const toGuardResolver = - (resolve: HostedHostnameResolver): GuardResolver => - (hostname) => - Effect.tryPromise({ - try: (signal) => resolve(hostname, signal), - catch: () => new HostedHostnameResolutionFailed({ hostname, empty: false }), - }).pipe( - Effect.flatMap((addresses) => - addresses.length === 0 - ? Effect.fail(new HostedHostnameResolutionFailed({ hostname, empty: true })) - : Effect.succeed(addresses), - ), - ); - -const DNS_CACHE_TTL_MILLIS = 60_000; -const DNS_CACHE_CAPACITY = 256; -const DNS_RESOLUTION_TIMEOUT_MILLIS = 60_000; - -// The cache is what keeps the guard off the request hot path: without it, -// every outbound request pays a full system-resolver round trip before the -// fetch, and on networks whose resolver drops AAAA queries (common behind -// IPv4-only home routers) that round trip stalls for the resolver timeout, -// several seconds per request. Only the first request per hostname per TTL -// window pays it now; concurrent requests share one in-flight resolution and -// failed resolutions are not retained. -// -// The TTL does widen an existing hole rather than opening one. The guard hands -// the underlying fetch a hostname, never the addresses it checked, so the fetch -// resolves again on its own and the verdict is already advisory: an attacker -// serving a TTL-0 record defeats it in the unresolved gap with or without this -// cache. Caching lengthens that gap from milliseconds to the TTL, and only -// closing it — pinning the connection to the checked address — makes either -// window matter. Until then a zero TTL would buy no real protection while -// restoring the per-request stall this exists to remove. -// -// Scope is the guarded-fetch adapter, deliberately: the MCP host builds one -// adapter per session, so the window spans a session there; the per-request -// HTTP API path only dedupes hops within one request. A process-wide cache -// would need a layer seam through every composition root. -const withResolutionCache = ( - resolve: GuardResolver, - options: HostedHttpClientOptions, -): GuardResolver => { - const ttl = Duration.millis(options.dnsCacheTtlMillis ?? DNS_CACHE_TTL_MILLIS); - // The cached effect is bounded because a pending entry is not subject to the - // TTL: Cache stamps expiry only when the lookup settles, so a resolution - // that never settles would leave every later request for that hostname - // joining the same dead Deferred for the adapter's lifetime — the hostname - // would be permanently unreachable. Timing out turns that into an ordinary - // typed failure, which the zero failure TTL then drops so the next request - // resolves again. - // - // It is a liveness backstop, not a latency policy, so it sits far above any - // real resolver: a resolver that answers slowly still answers, and blocking - // the request instead is a worse outcome than the wait. Callers that will - // not wait that long abort their own fetch, which the detached lookup - // survives on behalf of everyone else waiting on it. - // - // An empty answer is already a failure by the time it reaches the cache — - // toGuardResolver raises it — so the zero failure TTL drops it. Were it a - // success, one transient empty answer would stamp the full TTL on "no - // addresses" and blackhole the hostname for the rest of the window; the - // uncached code re-resolved every request and never could. - const bounded: GuardResolver = (hostname) => - Effect.timeoutOrElse(resolve(hostname), { - duration: Duration.millis( - options.dnsResolutionTimeoutMillis ?? DNS_RESOLUTION_TIMEOUT_MILLIS, - ), - orElse: () => Effect.fail(new HostedHostnameResolutionFailed({ hostname, empty: false })), - }); - const cache = Effect.runSync( - Cache.makeWith(bounded, { - capacity: options.dnsCacheCapacity ?? DNS_CACHE_CAPACITY, - timeToLive: (exit) => (Exit.isSuccess(exit) ? ttl : Duration.zero), - }), - ); - // A zero TTL makes a failed entry stale, not absent: it still holds a slot - // until something evicts it. Failures are the unbounded input here — one - // agent walking a list of dead hostnames mints a fresh key per name — so at - // capacity they evict the live entries this cache exists to keep, and the - // per-request resolver stall comes back for every hostname that still works. - // Dropping a failed entry on the way out keeps capacity for answers. - const forget = (hostname: string) => - Effect.onExit(Cache.get(cache, hostname), (exit) => - Exit.isSuccess(exit) ? Effect.void : Cache.invalidate(cache, hostname), - ); - // The lookup runs on a detached fiber that no caller owns, and each caller - // only joins it. A cache entry is shared, so running it on the requesting - // fiber would make one caller's abort interrupt the resolution every other - // caller is waiting on — they would fail with an untyped interrupt instead - // of their own answer. - return (hostname) => Effect.flatMap(Effect.forkDetach(forget(hostname)), Fiber.join); + return addresses.map(({ address, family }) => ({ + address, + family: family === 6 ? 6 : 4, + })); }; -const validateOutboundUrl = ( +export const validateHostedOutboundUrl = ( value: string, - options: HostedHttpClientOptions, - resolve: GuardResolver, + options: HostedHttpClientOptions = {}, ): Effect.Effect => Effect.gen(function* () { const url = yield* Effect.try({ @@ -373,419 +155,113 @@ const validateOutboundUrl = ( }); } - const normalizedHostname = canonicalHostname(url.hostname); - - if (isBlockedMetadataHostname(normalizedHostname)) { + if (isBlockedMetadataHostname(url.hostname)) { return yield* new HostedOutboundRequestBlocked({ url: value, reason: "Metadata service addresses are not allowed", }); } - if (!options.allowLocalNetwork && isLocalOrPrivateHostname(normalizedHostname)) { + if (!options.allowLocalNetwork && isLocalOrPrivateHostname(url.hostname)) { return yield* new HostedOutboundRequestBlocked({ url: value, reason: "Local and private network addresses are not allowed", }); } - // The resolved-address check runs whether or not the local network is - // allowed, because the metadata block above is only as good as the names it - // sees. Gating the whole lookup on `!allowLocalNetwork` left the endpoint - // one DNS record away on exactly the hosts that set the flag: a name with - // an A record for 169.254.169.254 is not an address literal, so nothing - // resolved it and nothing classified it. What the flag changes is the - // verdict, not whether the lookup happens — with it set, only the metadata - // rule applies to the answers, so a local address stays reachable by name - // as intended. The lookup this adds is the cached one, so a repeat name - // costs nothing after the first. - if (isAddressLiteral(normalizedHostname)) return; - - // The empty answer is already a failure by the time it arrives here — - // the cached lookup raises it, so it takes the zero failure TTL instead - // of being retained as a successful "no addresses" for the full window. - // - // A lookup that fails is fatal in the default mode and not in the - // permissive one. Blocking on it there would newly reject names this - // resolver cannot see but the transport can — a self-host behind a tunnel - // that resolves remotely is the case, and it is the deployment the flag - // exists for. No address came back, so none of them can be the metadata - // endpoint; a name that truly does not resolve fails at the transport a - // moment later, with its own error rather than a guard verdict. - const addresses = yield* resolve(normalizedHostname).pipe( - Effect.catchTag("HostedHostnameResolutionFailed", (error) => - options.allowLocalNetwork - ? Effect.succeed([]) - : Effect.fail( - new HostedOutboundRequestBlocked({ - url: value, - reason: error.empty - ? "Hostname did not resolve to an address" - : "Hostname could not be resolved", - }), - ), - ), - ); + const normalizedHostname = url.hostname.toLowerCase().replace(/^\[|\]$/g, ""); + if (!options.allowLocalNetwork && options.resolveHostname && !isAddressLiteral(url.hostname)) { + const addresses = yield* Effect.tryPromise({ + try: () => options.resolveHostname!(normalizedHostname), + catch: () => + new HostedOutboundRequestBlocked({ + url: value, + reason: "Hostname could not be resolved", + }), + }); - for (const { address } of addresses) { - if (isBlockedMetadataAddress(address.toLowerCase().replace(/^\[|\]$/g, ""))) { + if (addresses.length === 0) { return yield* new HostedOutboundRequestBlocked({ url: value, - reason: "Metadata service addresses are not allowed", + reason: "Hostname did not resolve to an address", }); } - if (!options.allowLocalNetwork && !isAllowedResolvedAddress(address)) { - return yield* new HostedOutboundRequestBlocked({ - url: value, - reason: "Resolved address is local or private", - }); + + for (const { address } of addresses) { + if (isLocalOrPrivateHostname(address)) { + return yield* new HostedOutboundRequestBlocked({ + url: value, + reason: "Resolved address is local or private", + }); + } } } }); -// Defaults to the same node:dns resolver guardFetch uses so both public -// entry points enforce the same guard; omitting the resolver must not -// silently drop the resolved-address check. -export const validateHostedOutboundUrl = ( - value: string, - options: HostedHttpClientOptions = {}, -): Effect.Effect => - validateOutboundUrl( - value, - options, - toGuardResolver(options.resolveHostname ?? resolveHostnameWithNodeDns), - ); - -// Cross-origin redirects keep only headers that cannot carry credentials. -// A blocklist of well-known credential names is unsound here: integrations -// render credentials into arbitrary header names (X-Api-Key and friends), -// so everything not on this list is dropped on a cross-origin hop. The body -// is dropped with them — a credential-bearing POST body must not replay to -// a redirect target whose headers were just scrubbed. -const SAFE_REDIRECT_HEADERS = ["accept", "accept-language", "user-agent"] as const; +const CREDENTIAL_HEADERS = ["authorization", "proxy-authorization", "cookie"] as const; -const retainSafeRedirectHeaders = (init: RequestInit | undefined): RequestInit => { +const stripCredentialHeaders = (init: RequestInit | undefined): RequestInit => { const headers = new Headers(init?.headers); - const kept = new Headers(); - for (const name of SAFE_REDIRECT_HEADERS) { - const value = headers.get(name); - if (value !== null) kept.set(name, value); - } - // Rebuilt field by field rather than spread from the original: `referrer` - // and `credentials` are credential-carrying too, and spreading would carry - // them past the header scrub. The platform turns `referrer` into a real - // Referer header — the whole URL, query string included, once the caller - // asked for `referrerPolicy: "unsafe-url"` — so an OAuth callback whose - // query holds a token would hand that token to the redirect target. Only - // the transport-shaped fields survive. - // - // `integrity` is kept because it is a check on the response, not a secret - // sent to the target, and the cross-origin hop is exactly where it earns its - // keep: dropping it would leave the caller believing the bytes were verified - // while whatever the redirect target served went unchecked. Platform fetch - // carries it across redirects for the same reason. - return { - headers: kept, - body: undefined, - method: init?.method ?? "GET", - signal: init?.signal, - cache: init?.cache, - integrity: init?.integrity, - keepalive: init?.keepalive, - mode: init?.mode, - redirect: init?.redirect, - }; -}; - -const CONTENT_HEADERS = [ - "content-type", - "content-length", - "content-encoding", - "content-language", -] as const; - -// Platform redirect semantics (fetch, curl): 303 always demotes to GET, and -// 301/302 demote any non-GET/HEAD method to GET; the body and its content -// headers go with it. Only 307/308 replay the method and body. -const demoteToGet = (init: RequestInit | undefined): RequestInit => { - const headers = new Headers(init?.headers); - for (const name of CONTENT_HEADERS) headers.delete(name); - return { ...init, method: "GET", body: undefined, headers }; -}; - -// The WHATWG rule, which is narrower than "3xx that is not 307/308": 303 -// demotes everything except GET and HEAD, and 301/302 demote POST alone. -// Demoting on 301/302 for any non-GET rewrote a DELETE or a PUT into a GET — -// the delete never happened, the write never happened, and the caller got a -// 200 for a request it did not make. A HEAD meeting a 303 was turned into a -// GET the same way, downloading a body the caller asked not to receive. -const redirectDemotesToGet = (status: number, method: string): boolean => - (status === 303 && method !== "GET" && method !== "HEAD") || - ((status === 301 || status === 302) && method === "POST"); - -// A stream can only be read once, so a hop that would replay it sends an empty -// body and still reports success — the caller's upload silently truncated. The -// stream is passed through untouched and the replay is refused instead: -// buffering it up front to make the rare redirect replayable would materialize -// every streamed upload in memory, and FetchHttpClient passes a ReadableStream -// for every streamed body, so that cost lands on the common path. -const isStreamedBody = (init: RequestInit | undefined): boolean => - init?.body instanceof ReadableStream; - -// Every hop goes out through here, so this is the one place the transport's -// requirements are met. `redirect: "manual"` is what makes the guard see each -// hop at all — without it the platform follows 3xx internally and hops 2..n -// are never re-validated. `duplex: "half"` is required by undici whenever the -// body is a stream, and rejects the request outright when it is missing; since -// normalizeFetchInput turns every Request input into its ReadableStream body, -// omitting it would kill every body-bearing Request and every streamed upload. -const withStreamingDuplex = (init: RequestInit | undefined): RequestInit => ({ - ...init, - redirect: "manual", - // @ts-expect-error — undici/Bun extension, absent from the DOM RequestInit - duplex: isStreamedBody(init) ? "half" : undefined, -}); - -// `fetch(request, init)` treats a member the caller left undefined as absent, -// so the Request's own value stands. A plain spread does not: an explicit -// `{ method: undefined }` would overwrite the Request's POST with undefined, -// which the transport then reads as GET, and `{ headers: undefined }` would -// erase the Authorization header — a credential-bearing call would go out as a -// bare unauthenticated GET. Dropping the undefined members restores the -// platform's own merge. -const definedMembers = (init: RequestInit | undefined): RequestInit => - Object.fromEntries(Object.entries(init ?? {}).filter(([, value]) => value !== undefined)); - -// A Request input is read into url + init up front so redirect hops keep the -// request shape instead of silently degrading to a bare GET. -const normalizeFetchInput = ( - input: Parameters[0], - init: RequestInit | undefined, -): { readonly url: string; readonly init: RequestInit | undefined } => { - if (!(input instanceof Request)) { - return { url: String(input), init }; - } - const normalized: RequestInit = { - method: input.method, - headers: new Headers(input.headers), - signal: input.signal, - credentials: input.credentials, - cache: input.cache, - integrity: input.integrity, - keepalive: input.keepalive, - mode: input.mode, - redirect: input.redirect, - referrer: input.referrer, - referrerPolicy: input.referrerPolicy, - ...definedMembers(init), - }; - if (input.body !== null && normalized.body === undefined) { - normalized.body = input.body; - } - return { url: input.url, init: normalized }; + for (const name of CREDENTIAL_HEADERS) headers.delete(name); + return { ...init, headers }; }; -// An abort during the guard would otherwise surface as the interrupt Effect -// raises when its fiber is cancelled ("All fibers interrupted without error"), -// which callers branching on `error.name === "AbortError"` read as a transport -// failure and retry. This is a Fetch adapter, so it rejects the way fetch does. -const rejectAsAbort = (signal: AbortSignal): never => { - // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: Fetch-compatible adapter must reject with the caller's own abort reason - if (signal.reason !== undefined) throw signal.reason; - // oxlint-disable-next-line executor/no-error-constructor -- boundary: Fetch-compatible adapter must reject with an AbortError-shaped value - const error = new Error("The operation was aborted"); - error.name = "AbortError"; - // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: Fetch-compatible adapter must reject with an AbortError-shaped value - throw error; -}; - -// One place the guard's verdict becomes a rejected Promise. Each call site had -// its own `Effect.runPromise(Effect.fail(...))`, which is a fresh fiber per -// verdict carrying no context and no signal — six runtime entries for what is -// one decision repeated. The adapter must return `Promise`, so the -// rejection itself is the contract; this keeps the tagged error the single -// currency and the entry in one reviewable spot. -const rejectBlocked = (url: string, reason: string): Promise => - Effect.runPromise(Effect.fail(new HostedOutboundRequestBlocked({ url, reason }))); - -// The cached resolver for one composition. Built here rather than inside -// guardFetch so several adapters over the same options can share a single -// cache instead of one each. -const makeSharedResolver = (options: HostedHttpClientOptions): GuardResolver => - withResolutionCache( - toGuardResolver(options.resolveHostname ?? resolveHostnameWithNodeDns), - options, - ); - const guardFetch = ( underlying: typeof globalThis.fetch, options: HostedHttpClientOptions, - sharedResolve?: GuardResolver, -): typeof globalThis.fetch => { - const resolve = sharedResolve ?? makeSharedResolver(options); - const maxRedirects = Math.max(0, options.maxRedirects ?? 10); - // SAFETY: narrowed to the one thing a closure cannot express — Bun's fetch - // type carries a static `preconnect` property, which packages built against - // @types/bun require. The call signature is checked by the annotations, and - // the return type is `Promise` on every path. - return (async (input, init) => { - const normalized = normalizeFetchInput(input, init); - let currentUrl = normalized.url; - let currentInit = normalized.init; - const signal = currentInit?.signal ?? undefined; - // Read once, before the loop rewrites `currentInit` on each hop. - const redirectMode = currentInit?.redirect ?? "follow"; - for (let redirects = 0; ; redirects++) { - const guarded = await Effect.runPromiseExit( - validateOutboundUrl(currentUrl, options, resolve), - { signal }, - ); - if (Exit.isFailure(guarded)) { - // Both halves are required. The cause decides that this was an - // interrupt rather than a guard verdict — an aborted signal alone - // would bury a real SSRF block under a retryable AbortError. The - // signal's own state decides that the caller asked for it: an - // interrupt from anywhere else must not be reported as the caller's - // abort, which would fabricate a reason that never happened. - if (signal?.aborted && Cause.hasInterruptsOnly(guarded.cause)) rejectAsAbort(signal); - return await Effect.runPromise(Effect.failCause(guarded.cause)); - } - const response = await underlying(currentUrl, withStreamingDuplex(currentInit)); - const isRedirect = - response.status >= 300 && response.status < 400 && response.headers.has("location"); - // The guard pins the transport to `redirect: "manual"` so it sees every - // hop, which is a separate question from what the caller asked for. - // Following anyway would answer a caller who asked to inspect a 3xx with - // the followed response and no Location header — the OAuth authorize - // probe reads exactly that header — and would turn a requested rejection - // into a 200. Neither mode is a security concession: not following is - // strictly safer than following, so the caller's intent stands. - if (isRedirect && redirectMode === "error") { - return await rejectBlocked( - currentUrl, - "Redirect received while the caller requested redirect: error", - ); - } - if (isRedirect && redirectMode === "manual") return response; - if (isRedirect && redirects < maxRedirects) { - // The 3xx is abandoned here. Undici keeps a connection out of the - // pool until an unread body is consumed or cancelled, so a - // redirect-heavy integration would leak one connection per hop. A - // cancel that itself fails — a 3xx declaring Content-Length whose - // connection dies mid-body rejects with "terminated" — is connection - // hygiene failing, not the request: the redirect still proceeds. - const body = response.body; - if (body) { - await Effect.runPromise( - Effect.tryPromise({ - try: () => body.cancel(), - catch: (error) => error, - }).pipe(Effect.ignore), - ); - } - const location = response.headers.get("location")!; - // A malformed Location header is a rejected redirect target, not a - // client defect, so it surfaces as the tagged guard error. - const parsed = URL.parse(location, currentUrl); - if (parsed === null) { - return await rejectBlocked(location, "Redirect target is not a valid URL"); - } - const next = parsed; - if (redirectDemotesToGet(response.status, currentInit?.method ?? "GET")) { - currentInit = demoteToGet(currentInit); - } - // A demotion has already dropped the body, so what is left here is a - // 307/308 that would replay it. A stream cannot be replayed — the - // first hop drained it — and sending the drained object would deliver - // an empty body under a 200, so the hop is refused instead. - if (isStreamedBody(currentInit)) { - return await rejectBlocked( - next.toString(), - "Redirect cannot replay a streamed request body", - ); - } +): typeof globalThis.fetch => + (async (input, init) => { + const guardOptions = { + ...options, + resolveHostname: options.resolveHostname ?? resolveHostnameWithNodeDns, + }; + const maxRedirects = options.maxRedirects ?? 10; + let current: Parameters[0] | URL = input; + let currentInit = init; + for (let redirects = 0; redirects <= maxRedirects; redirects++) { + const url = current instanceof Request ? current.url : String(current); + await Effect.runPromise(validateHostedOutboundUrl(url, guardOptions)); + const response = await underlying(current, { + ...currentInit, + redirect: "manual", + }); + if ( + response.status >= 300 && + response.status < 400 && + response.headers.has("location") && + redirects < maxRedirects + ) { + const next = new URL(response.headers.get("location")!, url); // Cross-origin redirects are followed (the loop re-validates every - // hop), but credentials minted for the original origin — in any - // header or in the body — must not leak to the redirect target. - if (next.origin !== new URL(currentUrl).origin) { - // A 307/308 replays the method, so a request still carrying a body - // here would go out as a bodyless POST/PUT once the body is - // stripped — a different request than the caller made, whose 4xx - // would misattribute the cause. The hop is refused instead, so the - // caller sees the guard decision rather than an upstream symptom. - if (currentInit?.body !== undefined && currentInit?.body !== null) { - return await rejectBlocked( - next.toString(), - "Cross-origin redirect cannot replay a request body", - ); - } - currentInit = retainSafeRedirectHeaders(currentInit); + // hop), but credentials minted for the original origin must not leak + // to the redirect target — same as fetch/curl behavior. + if (next.origin !== new URL(url).origin) { + currentInit = stripCredentialHeaders(currentInit); } - currentUrl = next.toString(); + current = next.toString(); continue; } - // A 3xx still here means the budget ran out: the manual and error modes - // returned above, so a "follow" caller would otherwise receive the raw - // 3xx as if it were the final response and might follow it itself, - // unguarded. Every sibling anomaly in this loop is a typed rejection, so - // this one is too. - if (isRedirect) { - return await rejectBlocked( - currentUrl, - `Redirect limit of ${String(maxRedirects)} exceeded`, - ); - } return response; } + return await underlying(current, { ...currentInit, redirect: "manual" }); }) as typeof globalThis.fetch; -}; export const makeHostedFetch = (options: HostedHttpClientOptions = {}): typeof globalThis.fetch => // oxlint-disable-next-line executor/no-raw-fetch -- boundary: exposes a guarded Fetch API adapter for libraries that require fetch guardFetch(options.fetch ?? globalThis.fetch, options); -/** - * The guarded fetch and the guarded HttpClient layer over ONE resolution - * cache. - * - * Calling `makeHostedFetch` and `makeHostedHttpClientLayer` separately builds - * a cache each, so a composition that needs both — every host does, since - * plugins take the raw fetch and the SDK takes the layer — resolves each - * hostname twice and runs two TTL windows that drift apart. That halves the - * benefit the cache exists for, on the path where it matters most. Prefer this - * over constructing the two independently. - */ -export const makeHostedHttp = ( +export const makeHostedHttpClientLayer = ( options: HostedHttpClientOptions = {}, -): { - readonly fetch: typeof globalThis.fetch; - readonly httpClientLayer: Layer.Layer; -} => { - const resolve = makeSharedResolver(options); - return { - // oxlint-disable-next-line executor/no-raw-fetch -- boundary: exposes a guarded Fetch API adapter for libraries that require fetch - fetch: guardFetch(options.fetch ?? globalThis.fetch, options, resolve), - httpClientLayer: hostedHttpClientLayer(options, resolve), - }; -}; - -const hostedHttpClientLayer = ( - options: HostedHttpClientOptions, - resolve?: GuardResolver, ): Layer.Layer => FetchHttpClient.layer.pipe( Layer.provide( options.fetch - ? Layer.succeed(FetchHttpClient.Fetch)(guardFetch(options.fetch, options, resolve)) + ? Layer.succeed(FetchHttpClient.Fetch)(guardFetch(options.fetch, options)) : Layer.effect( FetchHttpClient.Fetch, Effect.map(Effect.service(FetchHttpClient.Fetch), (underlying) => - guardFetch(underlying, options, resolve), + guardFetch(underlying, options), ), ), ), ); - -export const makeHostedHttpClientLayer = ( - options: HostedHttpClientOptions = {}, -): Layer.Layer => hostedHttpClientLayer(options); From c8e46e734eb4207b293dad6d8575bccb80af9deb Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Tue, 4 Aug 2026 11:51:49 -0700 Subject: [PATCH 2/2] Add changeset for the DNS guard revert --- .changeset/revert-hosted-dns-guard.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/revert-hosted-dns-guard.md diff --git a/.changeset/revert-hosted-dns-guard.md b/.changeset/revert-hosted-dns-guard.md new file mode 100644 index 000000000..10d78a3f1 --- /dev/null +++ b/.changeset/revert-hosted-dns-guard.md @@ -0,0 +1,5 @@ +--- +"@executor-js/sdk": patch +--- + +Revert the hosted outbound DNS guard resolution cache and the accompanying outbound guard changes released in 1.5.38. The guard returns to its previous behavior: no resolution cache, the caller's `redirect` mode is not honored, and `makeHostedHttp` is no longer exported — use `makeHostedFetch` and `makeHostedHttpClientLayer` as before.