diff --git a/sdk/src/__tests__/read-url.test.ts b/sdk/src/__tests__/read-url.test.ts index 902d47c45a..5ae4c56299 100644 --- a/sdk/src/__tests__/read-url.test.ts +++ b/sdk/src/__tests__/read-url.test.ts @@ -369,4 +369,40 @@ describe('readUrl SSRF protection', () => { errorMessage: 'Invalid redirect location: http://', }) }) + + it('refuses to fetch through a proxy instead of dropping DNS pinning', async () => { + const proxyVars = [ + 'HTTPS_PROXY', + 'https_proxy', + 'HTTP_PROXY', + 'http_proxy', + 'ALL_PROXY', + 'all_proxy', + ] as const + const saved = new Map( + proxyVars.map((name) => [name, process.env[name]]), + ) + for (const name of proxyVars) delete process.env[name] + process.env.HTTPS_PROXY = 'http://proxy.example.com:8080' + try { + // No custom fetch: the default transport is under test. The hostname + // resolves public, so only the proxy refusal can stop the fetch. + const result = await readUrl({ + url: 'https://example.com/page', + lookupHost: async () => ['93.184.216.34'], + }) + + expect(result[0].value).toEqual({ + url: 'https://example.com/page', + errorMessage: + 'read_url is unavailable while a proxy is configured (HTTP_PROXY, HTTPS_PROXY or ALL_PROXY is set): the proxy resolves hostnames itself, so the destination cannot be validated against the SSRF policy. Unset the proxy variables to use read_url.', + }) + } finally { + for (const name of proxyVars) { + const value = saved.get(name) + if (value === undefined) delete process.env[name] + else process.env[name] = value + } + } + }) }) diff --git a/sdk/src/tools/read-url.ts b/sdk/src/tools/read-url.ts index cd09cd99b7..d34f6ab878 100644 --- a/sdk/src/tools/read-url.ts +++ b/sdk/src/tools/read-url.ts @@ -330,9 +330,12 @@ function truncateText( * The transport used when the caller supplies no `fetch`. * * Normally {@link pinnedFetch}: the connection goes only to addresses that - * were validated in the same resolution. Under Bun with a proxy configured, - * Bun's own `fetch` instead, because it honours the proxy variables and - * `node:http` there does not; the proxy, not this process, resolves the name. + * were validated in the same resolution. Under Bun with a proxy configured + * that guarantee cannot be kept: the proxy, not this process, resolves the + * name, so the address {@link assertUrlAllowed} validated is not the + * address the connection would use. Rather than fetch unvalidated through + * `globalThis.fetch`, the transport refuses until the pinned transport can + * speak proxy. A clear error beats a silent downgrade of the SSRF check. */ function defaultTransport(opts: { lookupHost?: HostLookup @@ -340,7 +343,11 @@ function defaultTransport(opts: { }): FetchLike { const isBun = typeof process !== 'undefined' && !!process.versions?.bun if (isBun && isHttpProxyConfiguredFromEnv()) { - return globalThis.fetch + return async () => { + throw new Error( + 'read_url is unavailable while a proxy is configured (HTTP_PROXY, HTTPS_PROXY or ALL_PROXY is set): the proxy resolves hostnames itself, so the destination cannot be validated against the SSRF policy. Unset the proxy variables to use read_url.', + ) + } } return async (input, init) => pinnedFetch(String(input), {