From 8a00705e451cb52a145e899cdf8a917f7d553a46 Mon Sep 17 00:00:00 2001 From: herdiyana256 Date: Mon, 8 Jun 2026 22:11:45 +0700 Subject: [PATCH 1/4] fix(web-fetch): resolve DNS before SSRF guard to block hostname-to-private-IP bypass --- packages/core/src/tools/web-fetch.test.ts | 121 +++++++++++++++++++--- packages/core/src/tools/web-fetch.ts | 34 ++++-- 2 files changed, 131 insertions(+), 24 deletions(-) diff --git a/packages/core/src/tools/web-fetch.test.ts b/packages/core/src/tools/web-fetch.test.ts index 6266644a7ff..be4bec1bf3b 100644 --- a/packages/core/src/tools/web-fetch.test.ts +++ b/packages/core/src/tools/web-fetch.test.ts @@ -52,7 +52,7 @@ vi.mock('../utils/fetch.js', async (importOriginal) => { return { ...actual, fetchWithTimeout: vi.fn(), - isPrivateIp: vi.fn(), + isPrivateIpAsync: vi.fn().mockResolvedValue(false), }; }); @@ -376,7 +376,7 @@ describe('WebFetchTool', () => { describe('execute', () => { it('should return WEB_FETCH_PROCESSING_ERROR on rate limit exceeded', async () => { - vi.spyOn(fetchUtils, 'isPrivateIp').mockReturnValue(false); + vi.spyOn(fetchUtils, 'isPrivateIpAsync').mockResolvedValue(false); mockGenerateContent.mockResolvedValue({ candidates: [{ content: { parts: [{ text: 'response' }] } }], }); @@ -400,7 +400,7 @@ describe('WebFetchTool', () => { }); it('should skip rate-limited URLs but fetch others', async () => { - vi.spyOn(fetchUtils, 'isPrivateIp').mockReturnValue(false); + vi.spyOn(fetchUtils, 'isPrivateIpAsync').mockResolvedValue(false); const tool = new WebFetchTool(mockConfig, bus); const params = { @@ -439,8 +439,8 @@ describe('WebFetchTool', () => { }); it('should skip private or local URLs but fetch others and log telemetry', async () => { - vi.mocked(fetchUtils.isPrivateIp).mockImplementation( - (url) => url === 'https://private.com/', + vi.mocked(fetchUtils.isPrivateIpAsync).mockImplementation( + async (url) => url === 'https://private.com/', ); const tool = new WebFetchTool(mockConfig, bus); @@ -475,7 +475,7 @@ describe('WebFetchTool', () => { }); it('should fallback to all public URLs if primary fails', async () => { - vi.spyOn(fetchUtils, 'isPrivateIp').mockReturnValue(false); + vi.spyOn(fetchUtils, 'isPrivateIpAsync').mockResolvedValue(false); // Primary fetch fails mockGenerateContent.mockRejectedValueOnce(new Error('primary fail')); @@ -513,8 +513,8 @@ describe('WebFetchTool', () => { }); it('should NOT include private URLs in fallback', async () => { - vi.mocked(fetchUtils.isPrivateIp).mockImplementation( - (url) => url === 'https://private.com/', + vi.mocked(fetchUtils.isPrivateIpAsync).mockImplementation( + async (url) => url === 'https://private.com/', ); // Primary fetch fails @@ -546,7 +546,7 @@ describe('WebFetchTool', () => { }); it('should return WEB_FETCH_FALLBACK_FAILED on total failure', async () => { - vi.spyOn(fetchUtils, 'isPrivateIp').mockReturnValue(false); + vi.spyOn(fetchUtils, 'isPrivateIpAsync').mockResolvedValue(false); mockGenerateContent.mockRejectedValue(new Error('primary fail')); mockFetch('https://public.ip/', new Error('fallback fetch failed')); const tool = new WebFetchTool(mockConfig, bus); @@ -559,7 +559,7 @@ describe('WebFetchTool', () => { }); it('should log telemetry when falling back due to primary fetch failure', async () => { - vi.spyOn(fetchUtils, 'isPrivateIp').mockReturnValue(false); + vi.spyOn(fetchUtils, 'isPrivateIpAsync').mockResolvedValue(false); // Mock primary fetch to return empty response, triggering fallback mockGenerateContent.mockResolvedValueOnce({ candidates: [], @@ -591,7 +591,7 @@ describe('WebFetchTool', () => { describe('execute (fallback)', () => { beforeEach(() => { // Force fallback by mocking primary fetch to fail - vi.spyOn(fetchUtils, 'isPrivateIp').mockReturnValue(false); + vi.spyOn(fetchUtils, 'isPrivateIpAsync').mockResolvedValue(false); mockGenerateContent.mockResolvedValueOnce({ candidates: [], }); @@ -929,7 +929,7 @@ describe('WebFetchTool', () => { }); it('should execute normally after confirmation approval', async () => { - vi.spyOn(fetchUtils, 'isPrivateIp').mockReturnValue(false); + vi.spyOn(fetchUtils, 'isPrivateIpAsync').mockResolvedValue(false); mockGenerateContent.mockResolvedValue({ candidates: [ { @@ -963,7 +963,7 @@ describe('WebFetchTool', () => { describe('execute (experimental)', () => { beforeEach(() => { vi.spyOn(mockConfig, 'getDirectWebFetch').mockReturnValue(true); - vi.spyOn(fetchUtils, 'isPrivateIp').mockReturnValue(false); + vi.spyOn(fetchUtils, 'isPrivateIpAsync').mockResolvedValue(false); }); it('should perform direct fetch and return text for plain text content', async () => { @@ -1142,7 +1142,7 @@ describe('WebFetchTool', () => { }); it('should block private IP (experimental)', async () => { - vi.spyOn(fetchUtils, 'isPrivateIp').mockReturnValue(true); + vi.spyOn(fetchUtils, 'isPrivateIpAsync').mockResolvedValue(true); const tool = new WebFetchTool(mockConfig, bus); const invocation = tool['createInvocation']( { url: 'http://localhost' }, @@ -1158,6 +1158,99 @@ describe('WebFetchTool', () => { expect(result.error?.type).toBe(ToolErrorType.WEB_FETCH_PROCESSING_ERROR); }); + describe('SSRF guard — DNS hostname-to-private-IP bypass (CVE-class)', () => { + it.each([ + { + label: 'localhost (literal name)', + url: 'http://localhost', + // caught by explicit loopback check, no DNS mock needed + dnsPrivate: false, + }, + { + label: '127.0.0.1 (IPv4 loopback literal)', + url: 'http://127.0.0.1', + dnsPrivate: false, + }, + { + label: '::1 (IPv6 loopback literal)', + url: 'http://[::1]', + dnsPrivate: false, + }, + { + label: '0.0.0.0 (unspecified literal)', + url: 'http://0.0.0.0', + dnsPrivate: false, + }, + { + label: '127.0.0.1.nip.io (nip.io loopback bypass)', + url: 'http://127.0.0.1.nip.io', + // DNS resolves to 127.0.0.1 — mocked via isPrivateIpAsync + dnsPrivate: true, + }, + { + label: '169.254.169.254.nip.io (cloud metadata bypass)', + url: 'http://169.254.169.254.nip.io', + dnsPrivate: true, + }, + ])('should block $label', async ({ url, dnsPrivate }) => { + if (dnsPrivate) { + vi.spyOn(fetchUtils, 'isPrivateIpAsync').mockResolvedValue(true); + } + const tool = new WebFetchTool(mockConfig, bus); + const invocation = tool['createInvocation']({ url }, bus); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + + expect(result.error?.type).toBe( + ToolErrorType.WEB_FETCH_PROCESSING_ERROR, + ); + expect(result.llmContent).toContain( + 'Access to blocked or private host', + ); + }); + + it('should allow a public URL (https://google.com)', async () => { + vi.spyOn(fetchUtils, 'isPrivateIpAsync').mockResolvedValue(false); + const content = 'Google homepage'; + mockFetch('https://google.com/', { + status: 200, + headers: new Headers({ 'content-type': 'text/plain' }), + text: () => Promise.resolve(content), + }); + + const tool = new WebFetchTool(mockConfig, bus); + const invocation = tool.build({ url: 'https://google.com' }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + + expect(result.error).toBeUndefined(); + expect(result.llmContent).toBe(content); + }); + + it('should block nip.io bypass in standard (non-experimental) mode', async () => { + // Override the outer beforeEach that enables experimental mode + vi.spyOn(mockConfig, 'getDirectWebFetch').mockReturnValue(false); + // Simulate DNS resolving the hostname to a private IP + vi.mocked(fetchUtils.isPrivateIpAsync).mockImplementation(async (url) => + url.includes('127.0.0.1.nip.io'), + ); + + const tool = new WebFetchTool(mockConfig, bus); + const params = { prompt: 'fetch http://127.0.0.1.nip.io/secret' }; + const invocation = tool.build(params); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + + expect(result.error?.type).toBe( + ToolErrorType.WEB_FETCH_PROCESSING_ERROR, + ); + expect(result.llmContent).toContain('[Blocked Host]'); + }); + }); + it('should bypass truncation if isContextManagementEnabled is true', async () => { vi.spyOn(mockConfig, 'isContextManagementEnabled').mockReturnValue(true); const largeContent = 'a'.repeat(300000); // Larger than MAX_CONTENT_LENGTH (250000) diff --git a/packages/core/src/tools/web-fetch.ts b/packages/core/src/tools/web-fetch.ts index d468064f237..b339d3a378b 100644 --- a/packages/core/src/tools/web-fetch.ts +++ b/packages/core/src/tools/web-fetch.ts @@ -19,7 +19,11 @@ import type { MessageBus } from '../confirmation-bus/message-bus.js'; import { ToolErrorType } from './tool-error.js'; import { getErrorMessage } from '../utils/errors.js'; import { getResponseText } from '../utils/partUtils.js'; -import { fetchWithTimeout, isPrivateIp } from '../utils/fetch.js'; +import { + fetchWithTimeout, + isLoopbackHost, + isPrivateIpAsync, +} from '../utils/fetch.js'; import { truncateString, wrapUntrusted } from '../utils/textUtils.js'; import { convert } from 'html-to-text'; import { @@ -267,14 +271,24 @@ class WebFetchToolInvocation extends BaseToolInvocation< ); } - private isBlockedHost(urlStr: string): boolean { + private async isBlockedHost(urlStr: string): Promise { try { const url = new URL(urlStr); const hostname = url.hostname.toLowerCase(); - if (hostname === 'localhost' || hostname === '127.0.0.1') { + // Block loopback and unspecified addresses by literal name/IP without DNS + // (covers localhost, 127.0.0.1, ::1, [::1], 0.0.0.0) + if (isLoopbackHost(hostname) || hostname === '0.0.0.0') { + return true; + } + // Resolve DNS to catch hostname-to-private-IP bypasses such as + // 127.0.0.1.nip.io, 169.254.169.254.nip.io, or attacker-controlled DNS + try { + return await isPrivateIpAsync(urlStr); + } catch { + // DNS resolution failed — deny by default to avoid SSRF via + // unresolvable or ambiguous hostnames return true; } - return isPrivateIp(urlStr); } catch { return true; } @@ -285,7 +299,7 @@ class WebFetchToolInvocation extends BaseToolInvocation< signal: AbortSignal, ): Promise { const url = convertGithubUrlToRaw(urlStr); - if (this.isBlockedHost(url)) { + if (await this.isBlockedHost(url)) { debugLogger.warn(`[WebFetchTool] Blocked access to host: ${url}`); throw new Error( `Access to blocked or private host ${url} is not allowed.`, @@ -350,16 +364,16 @@ class WebFetchToolInvocation extends BaseToolInvocation< return textContent; } - private filterAndValidateUrls(urls: string[]): { + private async filterAndValidateUrls(urls: string[]): Promise<{ toFetch: string[]; skipped: string[]; - } { + }> { const uniqueUrls = [...new Set(urls.map(normalizeUrl))]; const toFetch: string[] = []; const skipped: string[] = []; for (const url of uniqueUrls) { - if (this.isBlockedHost(url)) { + if (await this.isBlockedHost(url)) { debugLogger.warn( `[WebFetchTool] Skipped private or local host: ${url}`, ); @@ -615,7 +629,7 @@ ${aggregatedContent} // Convert GitHub blob URL to raw URL url = convertGithubUrlToRaw(url); - if (this.isBlockedHost(url)) { + if (await this.isBlockedHost(url)) { const errorMessage = `Access to blocked or private host ${url} is not allowed.`; debugLogger.warn( `[WebFetchTool] Blocked experimental fetch to host: ${url}`, @@ -769,7 +783,7 @@ Response: ${rawResponseText}`; const userPrompt = this.params.prompt!; const { validUrls } = parsePrompt(userPrompt); - const { toFetch, skipped } = this.filterAndValidateUrls(validUrls); + const { toFetch, skipped } = await this.filterAndValidateUrls(validUrls); // If everything was skipped, fail early if (toFetch.length === 0 && skipped.length > 0) { From 17a70c7eb75f1471fc97ee4f95d2cde4681de112 Mon Sep 17 00:00:00 2001 From: Herdiyan Date: Mon, 8 Jun 2026 22:49:02 +0700 Subject: [PATCH 2/4] fix(web-fetch): implement IP pinning, block [::], add tests for DNS rebinding --- packages/core/src/tools/web-fetch.test.ts | 76 ++++++++++++++++------- packages/core/src/tools/web-fetch.ts | 74 +++++++++++++++------- packages/core/src/utils/fetch.test.ts | 22 ++++--- packages/core/src/utils/fetch.ts | 18 +++--- 4 files changed, 126 insertions(+), 64 deletions(-) diff --git a/packages/core/src/tools/web-fetch.test.ts b/packages/core/src/tools/web-fetch.test.ts index be4bec1bf3b..3af56d0b6ae 100644 --- a/packages/core/src/tools/web-fetch.test.ts +++ b/packages/core/src/tools/web-fetch.test.ts @@ -27,6 +27,7 @@ import { type ToolConfirmationResponse, } from '../confirmation-bus/types.js'; import { randomUUID } from 'node:crypto'; +import { lookup } from 'node:dns/promises'; import { logWebFetchFallbackAttempt, WebFetchFallbackAttemptEvent, @@ -52,7 +53,7 @@ vi.mock('../utils/fetch.js', async (importOriginal) => { return { ...actual, fetchWithTimeout: vi.fn(), - isPrivateIpAsync: vi.fn().mockResolvedValue(false), + resolveAndValidateDns: vi.fn().mockResolvedValue(['8.8.8.8']), }; }); @@ -60,6 +61,8 @@ vi.mock('node:crypto', () => ({ randomUUID: vi.fn(), })); +vi.mock('node:dns/promises'); + /** * Helper to mock fetchWithTimeout with URL matching. */ @@ -67,7 +70,16 @@ const mockFetch = (url: string, response: Partial | Error) => vi .spyOn(fetchUtils, 'fetchWithTimeout') .mockImplementation(async (actualUrl) => { - if (actualUrl !== url) { + let pinnedUrlStr = url; + try { + const u = new URL(url); + u.hostname = '8.8.8.8'; + pinnedUrlStr = u.toString(); + } catch { + // ignore error + } + + if (actualUrl !== url && actualUrl !== pinnedUrlStr) { throw new Error( `Unexpected fetch URL: expected "${url}", got "${actualUrl}"`, ); @@ -270,6 +282,10 @@ describe('WebFetchTool', () => { beforeEach(() => { vi.resetAllMocks(); + vi.mocked(lookup).mockResolvedValue([ + { address: '8.8.8.8', family: 4 }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ] as any); bus = createMockMessageBus(); getMockMessageBusInstance(bus).defaultToolDecision = 'ask_user'; mockConfig = { @@ -376,7 +392,9 @@ describe('WebFetchTool', () => { describe('execute', () => { it('should return WEB_FETCH_PROCESSING_ERROR on rate limit exceeded', async () => { - vi.spyOn(fetchUtils, 'isPrivateIpAsync').mockResolvedValue(false); + vi.spyOn(fetchUtils, 'resolveAndValidateDns').mockResolvedValue([ + '8.8.8.8', + ]); mockGenerateContent.mockResolvedValue({ candidates: [{ content: { parts: [{ text: 'response' }] } }], }); @@ -400,7 +418,9 @@ describe('WebFetchTool', () => { }); it('should skip rate-limited URLs but fetch others', async () => { - vi.spyOn(fetchUtils, 'isPrivateIpAsync').mockResolvedValue(false); + vi.spyOn(fetchUtils, 'resolveAndValidateDns').mockResolvedValue([ + '8.8.8.8', + ]); const tool = new WebFetchTool(mockConfig, bus); const params = { @@ -439,8 +459,8 @@ describe('WebFetchTool', () => { }); it('should skip private or local URLs but fetch others and log telemetry', async () => { - vi.mocked(fetchUtils.isPrivateIpAsync).mockImplementation( - async (url) => url === 'https://private.com/', + vi.mocked(fetchUtils.resolveAndValidateDns).mockImplementation( + async (url) => (url === 'https://private.com/' ? [] : ['8.8.8.8']), ); const tool = new WebFetchTool(mockConfig, bus); @@ -475,7 +495,9 @@ describe('WebFetchTool', () => { }); it('should fallback to all public URLs if primary fails', async () => { - vi.spyOn(fetchUtils, 'isPrivateIpAsync').mockResolvedValue(false); + vi.spyOn(fetchUtils, 'resolveAndValidateDns').mockResolvedValue([ + '8.8.8.8', + ]); // Primary fetch fails mockGenerateContent.mockRejectedValueOnce(new Error('primary fail')); @@ -513,8 +535,8 @@ describe('WebFetchTool', () => { }); it('should NOT include private URLs in fallback', async () => { - vi.mocked(fetchUtils.isPrivateIpAsync).mockImplementation( - async (url) => url === 'https://private.com/', + vi.mocked(fetchUtils.resolveAndValidateDns).mockImplementation( + async (url) => (url === 'https://private.com/' ? [] : ['8.8.8.8']), ); // Primary fetch fails @@ -546,7 +568,9 @@ describe('WebFetchTool', () => { }); it('should return WEB_FETCH_FALLBACK_FAILED on total failure', async () => { - vi.spyOn(fetchUtils, 'isPrivateIpAsync').mockResolvedValue(false); + vi.spyOn(fetchUtils, 'resolveAndValidateDns').mockResolvedValue([ + '8.8.8.8', + ]); mockGenerateContent.mockRejectedValue(new Error('primary fail')); mockFetch('https://public.ip/', new Error('fallback fetch failed')); const tool = new WebFetchTool(mockConfig, bus); @@ -559,7 +583,9 @@ describe('WebFetchTool', () => { }); it('should log telemetry when falling back due to primary fetch failure', async () => { - vi.spyOn(fetchUtils, 'isPrivateIpAsync').mockResolvedValue(false); + vi.spyOn(fetchUtils, 'resolveAndValidateDns').mockResolvedValue([ + '8.8.8.8', + ]); // Mock primary fetch to return empty response, triggering fallback mockGenerateContent.mockResolvedValueOnce({ candidates: [], @@ -591,7 +617,9 @@ describe('WebFetchTool', () => { describe('execute (fallback)', () => { beforeEach(() => { // Force fallback by mocking primary fetch to fail - vi.spyOn(fetchUtils, 'isPrivateIpAsync').mockResolvedValue(false); + vi.spyOn(fetchUtils, 'resolveAndValidateDns').mockResolvedValue([ + '8.8.8.8', + ]); mockGenerateContent.mockResolvedValueOnce({ candidates: [], }); @@ -929,7 +957,9 @@ describe('WebFetchTool', () => { }); it('should execute normally after confirmation approval', async () => { - vi.spyOn(fetchUtils, 'isPrivateIpAsync').mockResolvedValue(false); + vi.spyOn(fetchUtils, 'resolveAndValidateDns').mockResolvedValue([ + '8.8.8.8', + ]); mockGenerateContent.mockResolvedValue({ candidates: [ { @@ -963,7 +993,9 @@ describe('WebFetchTool', () => { describe('execute (experimental)', () => { beforeEach(() => { vi.spyOn(mockConfig, 'getDirectWebFetch').mockReturnValue(true); - vi.spyOn(fetchUtils, 'isPrivateIpAsync').mockResolvedValue(false); + vi.spyOn(fetchUtils, 'resolveAndValidateDns').mockResolvedValue([ + '8.8.8.8', + ]); }); it('should perform direct fetch and return text for plain text content', async () => { @@ -986,7 +1018,7 @@ describe('WebFetchTool', () => { ); expect(result.returnDisplay).toContain('Fetched text/plain content'); expect(fetchUtils.fetchWithTimeout).toHaveBeenCalledWith( - 'https://example.com/', + 'https://8.8.8.8/', expect.any(Number), expect.objectContaining({ headers: expect.objectContaining({ @@ -1142,7 +1174,7 @@ describe('WebFetchTool', () => { }); it('should block private IP (experimental)', async () => { - vi.spyOn(fetchUtils, 'isPrivateIpAsync').mockResolvedValue(true); + vi.spyOn(fetchUtils, 'resolveAndValidateDns').mockResolvedValue([]); const tool = new WebFetchTool(mockConfig, bus); const invocation = tool['createInvocation']( { url: 'http://localhost' }, @@ -1184,7 +1216,7 @@ describe('WebFetchTool', () => { { label: '127.0.0.1.nip.io (nip.io loopback bypass)', url: 'http://127.0.0.1.nip.io', - // DNS resolves to 127.0.0.1 — mocked via isPrivateIpAsync + // DNS resolves to 127.0.0.1 — mocked via resolveAndValidateDns dnsPrivate: true, }, { @@ -1194,7 +1226,7 @@ describe('WebFetchTool', () => { }, ])('should block $label', async ({ url, dnsPrivate }) => { if (dnsPrivate) { - vi.spyOn(fetchUtils, 'isPrivateIpAsync').mockResolvedValue(true); + vi.spyOn(fetchUtils, 'resolveAndValidateDns').mockResolvedValue([]); } const tool = new WebFetchTool(mockConfig, bus); const invocation = tool['createInvocation']({ url }, bus); @@ -1211,7 +1243,9 @@ describe('WebFetchTool', () => { }); it('should allow a public URL (https://google.com)', async () => { - vi.spyOn(fetchUtils, 'isPrivateIpAsync').mockResolvedValue(false); + vi.spyOn(fetchUtils, 'resolveAndValidateDns').mockResolvedValue([ + '8.8.8.8', + ]); const content = 'Google homepage'; mockFetch('https://google.com/', { status: 200, @@ -1233,8 +1267,8 @@ describe('WebFetchTool', () => { // Override the outer beforeEach that enables experimental mode vi.spyOn(mockConfig, 'getDirectWebFetch').mockReturnValue(false); // Simulate DNS resolving the hostname to a private IP - vi.mocked(fetchUtils.isPrivateIpAsync).mockImplementation(async (url) => - url.includes('127.0.0.1.nip.io'), + vi.mocked(fetchUtils.resolveAndValidateDns).mockImplementation( + async (url) => (url.includes('127.0.0.1.nip.io') ? [] : ['8.8.8.8']), ); const tool = new WebFetchTool(mockConfig, bus); diff --git a/packages/core/src/tools/web-fetch.ts b/packages/core/src/tools/web-fetch.ts index b339d3a378b..5fe74796b7c 100644 --- a/packages/core/src/tools/web-fetch.ts +++ b/packages/core/src/tools/web-fetch.ts @@ -22,7 +22,7 @@ import { getResponseText } from '../utils/partUtils.js'; import { fetchWithTimeout, isLoopbackHost, - isPrivateIpAsync, + resolveAndValidateDns, } from '../utils/fetch.js'; import { truncateString, wrapUntrusted } from '../utils/textUtils.js'; import { convert } from 'html-to-text'; @@ -271,26 +271,34 @@ class WebFetchToolInvocation extends BaseToolInvocation< ); } - private async isBlockedHost(urlStr: string): Promise { + private async isBlockedHost(urlStr: string): Promise { try { const url = new URL(urlStr); const hostname = url.hostname.toLowerCase(); // Block loopback and unspecified addresses by literal name/IP without DNS - // (covers localhost, 127.0.0.1, ::1, [::1], 0.0.0.0) - if (isLoopbackHost(hostname) || hostname === '0.0.0.0') { - return true; + // (covers localhost, 127.0.0.1, ::1, [::1], 0.0.0.0, [::]) + if ( + isLoopbackHost(hostname) || + hostname === '0.0.0.0' || + hostname === '[::]' + ) { + return null; } // Resolve DNS to catch hostname-to-private-IP bypasses such as // 127.0.0.1.nip.io, 169.254.169.254.nip.io, or attacker-controlled DNS try { - return await isPrivateIpAsync(urlStr); + const safeIps = await resolveAndValidateDns(urlStr); + if (safeIps.length === 0) { + return null; + } + return safeIps[0]; } catch { // DNS resolution failed — deny by default to avoid SSRF via // unresolvable or ambiguous hostnames - return true; + return null; } } catch { - return true; + return null; } } @@ -299,21 +307,31 @@ class WebFetchToolInvocation extends BaseToolInvocation< signal: AbortSignal, ): Promise { const url = convertGithubUrlToRaw(urlStr); - if (await this.isBlockedHost(url)) { + const pinnedIp = await this.isBlockedHost(url); + if (!pinnedIp) { debugLogger.warn(`[WebFetchTool] Blocked access to host: ${url}`); throw new Error( `Access to blocked or private host ${url} is not allowed.`, ); } + const urlWithIp = new URL(url); + const originalHostname = urlWithIp.hostname; + urlWithIp.hostname = pinnedIp; + const response = await retryWithBackoff( async () => { - const res = await fetchWithTimeout(url, URL_FETCH_TIMEOUT_MS, { - signal, - headers: { - 'User-Agent': USER_AGENT, + const res = await fetchWithTimeout( + urlWithIp.toString(), + URL_FETCH_TIMEOUT_MS, + { + signal, + headers: { + 'User-Agent': USER_AGENT, + Host: originalHostname, + }, }, - }); + ); if (!res.ok) { const error = new Error( `Request failed with status code ${res.status} ${res.statusText}`, @@ -373,7 +391,7 @@ class WebFetchToolInvocation extends BaseToolInvocation< const skipped: string[] = []; for (const url of uniqueUrls) { - if (await this.isBlockedHost(url)) { + if (!(await this.isBlockedHost(url))) { debugLogger.warn( `[WebFetchTool] Skipped private or local host: ${url}`, ); @@ -629,7 +647,8 @@ ${aggregatedContent} // Convert GitHub blob URL to raw URL url = convertGithubUrlToRaw(url); - if (await this.isBlockedHost(url)) { + const pinnedIp = await this.isBlockedHost(url); + if (!pinnedIp) { const errorMessage = `Access to blocked or private host ${url} is not allowed.`; debugLogger.warn( `[WebFetchTool] Blocked experimental fetch to host: ${url}`, @@ -645,16 +664,25 @@ ${aggregatedContent} } try { + const urlWithIp = new URL(url); + const originalHostname = urlWithIp.hostname; + urlWithIp.hostname = pinnedIp; + const response = await retryWithBackoff( async () => { - const res = await fetchWithTimeout(url, URL_FETCH_TIMEOUT_MS, { - signal, - headers: { - Accept: - 'text/markdown, text/plain;q=0.9, application/json;q=0.9, text/html;q=0.8, application/pdf;q=0.7, video/*;q=0.7, */*;q=0.5', - 'User-Agent': USER_AGENT, + const res = await fetchWithTimeout( + urlWithIp.toString(), + URL_FETCH_TIMEOUT_MS, + { + signal, + headers: { + Accept: + 'text/markdown, text/plain;q=0.9, application/json;q=0.9, text/html;q=0.8, application/pdf;q=0.7, video/*;q=0.7, */*;q=0.5', + 'User-Agent': USER_AGENT, + Host: originalHostname, + }, }, - }); + ); return res; }, { diff --git a/packages/core/src/utils/fetch.test.ts b/packages/core/src/utils/fetch.test.ts index ff438910c6b..0ec419a15fb 100644 --- a/packages/core/src/utils/fetch.test.ts +++ b/packages/core/src/utils/fetch.test.ts @@ -29,7 +29,7 @@ vi.mock('node:dns/promises', () => ({ // Import after mocks are established const { isPrivateIp, - isPrivateIpAsync, + resolveAndValidateDns, isAddressPrivate, fetchWithTimeout, setGlobalProxy, @@ -138,9 +138,9 @@ describe('fetch utils', () => { }); }); - describe('isPrivateIpAsync', () => { + describe('resolveAndValidateDns', () => { it('should identify private IPs directly', async () => { - expect(await isPrivateIpAsync('http://10.0.0.1/')).toBe(true); + expect(await resolveAndValidateDns('http://10.0.0.1/')).toEqual([]); }); it('should identify domains resolving to private IPs', async () => { @@ -150,7 +150,7 @@ describe('fetch utils', () => { options: LookupAllOptions, ) => Promise, ).mockImplementation(async () => [{ address: '10.0.0.1', family: 4 }]); - expect(await isPrivateIpAsync('http://malicious.com/')).toBe(true); + expect(await resolveAndValidateDns('http://malicious.com/')).toEqual([]); }); it('should identify domains resolving to public IPs as non-private', async () => { @@ -160,18 +160,20 @@ describe('fetch utils', () => { options: LookupAllOptions, ) => Promise, ).mockImplementation(async () => [{ address: '8.8.8.8', family: 4 }]); - expect(await isPrivateIpAsync('http://google.com/')).toBe(false); + expect(await resolveAndValidateDns('http://google.com/')).toEqual([ + '8.8.8.8', + ]); }); - it('should throw error if DNS resolution fails (fail closed)', async () => { + it('should return empty array if DNS resolution fails (fail closed)', async () => { vi.mocked(dnsPromises.lookup).mockRejectedValue(new Error('DNS Error')); - await expect(isPrivateIpAsync('http://unreachable.com/')).rejects.toThrow( - 'Failed to verify if URL resolves to private IP', + expect(await resolveAndValidateDns('http://unreachable.com/')).toEqual( + [], ); }); - it('should return false for invalid URLs instead of throwing verification error', async () => { - expect(await isPrivateIpAsync('not-a-url')).toBe(false); + it('should return empty array for invalid URLs', async () => { + expect(await resolveAndValidateDns('not-a-url')).toEqual([]); }); }); diff --git a/packages/core/src/utils/fetch.ts b/packages/core/src/utils/fetch.ts index e0c34cf4f69..fe068b312c2 100644 --- a/packages/core/src/utils/fetch.ts +++ b/packages/core/src/utils/fetch.ts @@ -144,27 +144,25 @@ export function isAddressPrivate(address: string): boolean { } } -/** - * Checks if a URL resolves to a private IP address. - */ -export async function isPrivateIpAsync(url: string): Promise { +export async function resolveAndValidateDns(url: string): Promise { try { const parsedUrl = new URL(url); const hostname = parsedUrl.hostname; if (isLoopbackHost(hostname)) { - return false; + return []; } const addresses = await lookup(hostname, { all: true }); - return addresses.some((addr) => isAddressPrivate(addr.address)); + if (addresses.some((addr) => isAddressPrivate(addr.address))) { + return []; + } + return addresses.map((addr) => addr.address); } catch (error) { if (error instanceof TypeError) { - return false; + return []; } - throw new Error('Failed to verify if URL resolves to private IP', { - cause: error, - }); + return []; // Fail closed on DNS errors } } From 04ef846b66729c3cb68357828810ad8dbb3dd23c Mon Sep 17 00:00:00 2001 From: herdiyana256 Date: Tue, 23 Jun 2026 22:11:46 +0700 Subject: [PATCH 3/4] fix(mcp): add SSRF protection to OAuth metadata discovery The OAuth discovery flow in oauth-utils.ts and oauth-provider.ts fetches URLs received directly from external MCP server responses without any SSRF validation, unlike web-fetch.ts which already uses isLoopbackHost() and resolveAndValidateDns() from utils/fetch.ts. A malicious MCP server can return private/internal IPs in: - WWW-Authenticate: Bearer resource_metadata="http://169.254.169.254/..." - /.well-known/oauth-protected-resource: {authorization_servers: ["http://169.254.169.254"]} - /.well-known/oauth-authorization-server: {registration_endpoint: "http://10.x.x.x/admin"} This causes gemini-cli to make GET and POST requests to cloud IMDS endpoints (AWS 169.254.169.254, GCP metadata.google.internal, Azure 169.254.169.254) or internal services on the developer's network. Fix: apply the same isLoopbackHost() + resolveAndValidateDns() guards already used in web-fetch.ts to: - OAuthUtils.fetchProtectedResourceMetadata() - OAuthUtils.fetchAuthorizationServerMetadata() - MCPOAuthProvider.registerClient() --- packages/core/src/mcp/oauth-provider.ts | 35 ++++++++++- packages/core/src/mcp/oauth-utils.ts | 72 +++++++++++++++++++++- packages/core/src/tools/web-fetch.test.ts | 16 ++--- packages/core/src/tools/web-fetch.ts | 75 ++++++++++++++--------- packages/core/src/utils/fetch.ts | 10 ++- 5 files changed, 164 insertions(+), 44 deletions(-) diff --git a/packages/core/src/mcp/oauth-provider.ts b/packages/core/src/mcp/oauth-provider.ts index 8fd44183af7..5a801ba433f 100644 --- a/packages/core/src/mcp/oauth-provider.ts +++ b/packages/core/src/mcp/oauth-provider.ts @@ -14,6 +14,8 @@ import { OAuthUtils, ResourceMismatchError } from './oauth-utils.js'; import { coreEvents } from '../utils/events.js'; import { debugLogger } from '../utils/debugLogger.js'; import { getConsentForOauth } from '../utils/authConsent.js'; +import { isLoopbackHost, resolveAndValidateDns } from '../utils/fetch.js'; +import { Agent, type Dispatcher } from 'undici'; import { generatePKCEParams, startCallbackServer, @@ -111,13 +113,44 @@ export class MCPOAuthProvider { scope: config.scopes?.join(' ') || '', }; + const parsedRegUrl = new URL(registrationUrl); + if (isLoopbackHost(parsedRegUrl.hostname)) { + throw new Error( + `Client registration blocked: loopback address not allowed: ${registrationUrl}`, + ); + } + const resolvedAddrs = await resolveAndValidateDns(registrationUrl); + if (resolvedAddrs.length === 0) { + throw new Error( + `Client registration blocked: private/reserved IP not allowed: ${registrationUrl}`, + ); + } + const pinnedIp = resolvedAddrs[0]; + const dispatcher = new Agent({ + connect: { + lookup: ( + _hostname: string, + _options: unknown, + callback: ( + err: NodeJS.ErrnoException | null, + addresses: Array<{ address: string; family: number }>, + ) => void, + ) => { + callback(null, [ + { address: pinnedIp, family: pinnedIp.includes(':') ? 6 : 4 }, + ]); + }, + }, + }); + const response = await fetch(registrationUrl, { method: 'POST', headers: { 'Content-Type': 'application/json', }, body: JSON.stringify(registrationRequest), - }); + dispatcher: dispatcher as Dispatcher, + } as RequestInit & { dispatcher: Dispatcher }); if (!response.ok) { const errorText = await response.text(); diff --git a/packages/core/src/mcp/oauth-utils.ts b/packages/core/src/mcp/oauth-utils.ts index 12ab2bd9ff6..ca62c3fe759 100644 --- a/packages/core/src/mcp/oauth-utils.ts +++ b/packages/core/src/mcp/oauth-utils.ts @@ -7,6 +7,8 @@ import type { MCPOAuthConfig } from './oauth-provider.js'; import { getErrorMessage } from '../utils/errors.js'; import { debugLogger } from '../utils/debugLogger.js'; +import { isLoopbackHost, resolveAndValidateDns } from '../utils/fetch.js'; +import { Agent, type Dispatcher } from 'undici'; /** * Error thrown when the discovered resource metadata does not match the expected resource. @@ -97,7 +99,40 @@ export class OAuthUtils { resourceMetadataUrl: string, ): Promise { try { - const response = await fetch(resourceMetadataUrl); + const parsedUrl = new URL(resourceMetadataUrl); + if (isLoopbackHost(parsedUrl.hostname)) { + debugLogger.debug( + `Blocked OAuth metadata fetch to loopback address: ${resourceMetadataUrl}`, + ); + return null; + } + const resolvedAddrs = await resolveAndValidateDns(resourceMetadataUrl); + if (resolvedAddrs.length === 0) { + debugLogger.debug( + `Blocked OAuth metadata fetch to private/reserved IP: ${resourceMetadataUrl}`, + ); + return null; + } + const pinnedIp = resolvedAddrs[0]; + const dispatcher = new Agent({ + connect: { + lookup: ( + _hostname: string, + _options: unknown, + callback: ( + err: NodeJS.ErrnoException | null, + addresses: Array<{ address: string; family: number }>, + ) => void, + ) => { + callback(null, [ + { address: pinnedIp, family: pinnedIp.includes(':') ? 6 : 4 }, + ]); + }, + }, + }); + const response = await fetch(resourceMetadataUrl, { + dispatcher: dispatcher as Dispatcher, + } as RequestInit & { dispatcher: Dispatcher }); if (!response.ok) { return null; } @@ -121,7 +156,40 @@ export class OAuthUtils { authServerMetadataUrl: string, ): Promise { try { - const response = await fetch(authServerMetadataUrl); + const parsedUrl = new URL(authServerMetadataUrl); + if (isLoopbackHost(parsedUrl.hostname)) { + debugLogger.debug( + `Blocked OAuth metadata fetch to loopback address: ${authServerMetadataUrl}`, + ); + return null; + } + const resolvedAddrs = await resolveAndValidateDns(authServerMetadataUrl); + if (resolvedAddrs.length === 0) { + debugLogger.debug( + `Blocked OAuth metadata fetch to private/reserved IP: ${authServerMetadataUrl}`, + ); + return null; + } + const pinnedIp = resolvedAddrs[0]; + const dispatcher = new Agent({ + connect: { + lookup: ( + _hostname: string, + _options: unknown, + callback: ( + err: NodeJS.ErrnoException | null, + addresses: Array<{ address: string; family: number }>, + ) => void, + ) => { + callback(null, [ + { address: pinnedIp, family: pinnedIp.includes(':') ? 6 : 4 }, + ]); + }, + }, + }); + const response = await fetch(authServerMetadataUrl, { + dispatcher: dispatcher as Dispatcher, + } as RequestInit & { dispatcher: Dispatcher }); if (!response.ok) { return null; } diff --git a/packages/core/src/tools/web-fetch.test.ts b/packages/core/src/tools/web-fetch.test.ts index 3af56d0b6ae..aca89bef891 100644 --- a/packages/core/src/tools/web-fetch.test.ts +++ b/packages/core/src/tools/web-fetch.test.ts @@ -70,16 +70,9 @@ const mockFetch = (url: string, response: Partial | Error) => vi .spyOn(fetchUtils, 'fetchWithTimeout') .mockImplementation(async (actualUrl) => { - let pinnedUrlStr = url; - try { - const u = new URL(url); - u.hostname = '8.8.8.8'; - pinnedUrlStr = u.toString(); - } catch { - // ignore error - } - - if (actualUrl !== url && actualUrl !== pinnedUrlStr) { + // With the undici Agent dispatcher pattern, the original URL is + // always passed unchanged; DNS pinning happens inside the Agent. + if (actualUrl !== url) { throw new Error( `Unexpected fetch URL: expected "${url}", got "${actualUrl}"`, ); @@ -1018,9 +1011,10 @@ describe('WebFetchTool', () => { ); expect(result.returnDisplay).toContain('Fetched text/plain content'); expect(fetchUtils.fetchWithTimeout).toHaveBeenCalledWith( - 'https://8.8.8.8/', + 'https://example.com/', expect.any(Number), expect.objectContaining({ + dispatcher: expect.any(Object), headers: expect.objectContaining({ Accept: expect.stringContaining('text/plain'), }), diff --git a/packages/core/src/tools/web-fetch.ts b/packages/core/src/tools/web-fetch.ts index 5fe74796b7c..8101a721fa9 100644 --- a/packages/core/src/tools/web-fetch.ts +++ b/packages/core/src/tools/web-fetch.ts @@ -24,6 +24,7 @@ import { isLoopbackHost, resolveAndValidateDns, } from '../utils/fetch.js'; +import { Agent, type Dispatcher } from 'undici'; import { truncateString, wrapUntrusted } from '../utils/textUtils.js'; import { convert } from 'html-to-text'; import { @@ -315,23 +316,32 @@ class WebFetchToolInvocation extends BaseToolInvocation< ); } - const urlWithIp = new URL(url); - const originalHostname = urlWithIp.hostname; - urlWithIp.hostname = pinnedIp; + const dispatcher = new Agent({ + connect: { + lookup: ( + _hostname: string, + _options: unknown, + callback: ( + err: NodeJS.ErrnoException | null, + addresses: Array<{ address: string; family: number }>, + ) => void, + ) => { + callback(null, [ + { address: pinnedIp, family: pinnedIp.includes(':') ? 6 : 4 }, + ]); + }, + }, + }) as Dispatcher; const response = await retryWithBackoff( async () => { - const res = await fetchWithTimeout( - urlWithIp.toString(), - URL_FETCH_TIMEOUT_MS, - { - signal, - headers: { - 'User-Agent': USER_AGENT, - Host: originalHostname, - }, + const res = await fetchWithTimeout(url, URL_FETCH_TIMEOUT_MS, { + signal, + dispatcher, + headers: { + 'User-Agent': USER_AGENT, }, - ); + }); if (!res.ok) { const error = new Error( `Request failed with status code ${res.status} ${res.statusText}`, @@ -664,25 +674,34 @@ ${aggregatedContent} } try { - const urlWithIp = new URL(url); - const originalHostname = urlWithIp.hostname; - urlWithIp.hostname = pinnedIp; + const dispatcher = new Agent({ + connect: { + lookup: ( + _hostname: string, + _options: unknown, + callback: ( + err: NodeJS.ErrnoException | null, + addresses: Array<{ address: string; family: number }>, + ) => void, + ) => { + callback(null, [ + { address: pinnedIp, family: pinnedIp.includes(':') ? 6 : 4 }, + ]); + }, + }, + }) as Dispatcher; const response = await retryWithBackoff( async () => { - const res = await fetchWithTimeout( - urlWithIp.toString(), - URL_FETCH_TIMEOUT_MS, - { - signal, - headers: { - Accept: - 'text/markdown, text/plain;q=0.9, application/json;q=0.9, text/html;q=0.8, application/pdf;q=0.7, video/*;q=0.7, */*;q=0.5', - 'User-Agent': USER_AGENT, - Host: originalHostname, - }, + const res = await fetchWithTimeout(url, URL_FETCH_TIMEOUT_MS, { + signal, + dispatcher, + headers: { + Accept: + 'text/markdown, text/plain;q=0.9, application/json;q=0.9, text/html;q=0.8, application/pdf;q=0.7, video/*;q=0.7, */*;q=0.5', + 'User-Agent': USER_AGENT, }, - ); + }); return res; }, { diff --git a/packages/core/src/utils/fetch.ts b/packages/core/src/utils/fetch.ts index fe068b312c2..de0e7da0087 100644 --- a/packages/core/src/utils/fetch.ts +++ b/packages/core/src/utils/fetch.ts @@ -6,7 +6,13 @@ import { getErrorMessage, isAbortError } from './errors.js'; import { URL } from 'node:url'; -import { Agent, EnvHttpProxyAgent, setGlobalDispatcher } from 'undici'; +import type { + Dispatcher} from 'undici'; +import { + Agent, + EnvHttpProxyAgent, + setGlobalDispatcher, +} from 'undici'; import ipaddr from 'ipaddr.js'; import { lookup } from 'node:dns/promises'; @@ -188,7 +194,7 @@ export function createSafeProxyAgent(proxyUrl: string): EnvHttpProxyAgent { export async function fetchWithTimeout( url: string, timeout: number, - options?: RequestInit, + options?: RequestInit & { dispatcher?: Dispatcher }, ): Promise { const controller = new AbortController(); const timeoutId = setTimeout(() => controller.abort(), timeout); From f844b2f6ebeab8de11f3d143aeecde49d37f275a Mon Sep 17 00:00:00 2001 From: herdiyana256 Date: Sat, 4 Jul 2026 16:55:12 +0700 Subject: [PATCH 4/4] refactor(security): extract shared pinned-DNS dispatcher helper The SSRF fix for OAuth discovery and web-fetch duplicated the same undici Agent + custom lookup dispatcher construction in five places across oauth-utils.ts, oauth-provider.ts, and web-fetch.ts. Consolidate it into createPinnedDispatcher() in utils/fetch.ts. --- packages/core/src/mcp/oauth-provider.ts | 29 +++++---------- packages/core/src/mcp/oauth-utils.ts | 48 ++++++------------------- packages/core/src/tools/web-fetch.ts | 36 ++----------------- packages/core/src/utils/fetch.ts | 37 +++++++++++++++---- 4 files changed, 51 insertions(+), 99 deletions(-) diff --git a/packages/core/src/mcp/oauth-provider.ts b/packages/core/src/mcp/oauth-provider.ts index 5a801ba433f..470b9cb028c 100644 --- a/packages/core/src/mcp/oauth-provider.ts +++ b/packages/core/src/mcp/oauth-provider.ts @@ -14,8 +14,12 @@ import { OAuthUtils, ResourceMismatchError } from './oauth-utils.js'; import { coreEvents } from '../utils/events.js'; import { debugLogger } from '../utils/debugLogger.js'; import { getConsentForOauth } from '../utils/authConsent.js'; -import { isLoopbackHost, resolveAndValidateDns } from '../utils/fetch.js'; -import { Agent, type Dispatcher } from 'undici'; +import { + createPinnedDispatcher, + isLoopbackHost, + resolveAndValidateDns, +} from '../utils/fetch.js'; +import type { Dispatcher } from 'undici'; import { generatePKCEParams, startCallbackServer, @@ -125,31 +129,14 @@ export class MCPOAuthProvider { `Client registration blocked: private/reserved IP not allowed: ${registrationUrl}`, ); } - const pinnedIp = resolvedAddrs[0]; - const dispatcher = new Agent({ - connect: { - lookup: ( - _hostname: string, - _options: unknown, - callback: ( - err: NodeJS.ErrnoException | null, - addresses: Array<{ address: string; family: number }>, - ) => void, - ) => { - callback(null, [ - { address: pinnedIp, family: pinnedIp.includes(':') ? 6 : 4 }, - ]); - }, - }, - }); - + const dispatcher = createPinnedDispatcher(resolvedAddrs[0]); const response = await fetch(registrationUrl, { method: 'POST', headers: { 'Content-Type': 'application/json', }, body: JSON.stringify(registrationRequest), - dispatcher: dispatcher as Dispatcher, + dispatcher, } as RequestInit & { dispatcher: Dispatcher }); if (!response.ok) { diff --git a/packages/core/src/mcp/oauth-utils.ts b/packages/core/src/mcp/oauth-utils.ts index ca62c3fe759..f1918a73b13 100644 --- a/packages/core/src/mcp/oauth-utils.ts +++ b/packages/core/src/mcp/oauth-utils.ts @@ -7,8 +7,12 @@ import type { MCPOAuthConfig } from './oauth-provider.js'; import { getErrorMessage } from '../utils/errors.js'; import { debugLogger } from '../utils/debugLogger.js'; -import { isLoopbackHost, resolveAndValidateDns } from '../utils/fetch.js'; -import { Agent, type Dispatcher } from 'undici'; +import { + createPinnedDispatcher, + isLoopbackHost, + resolveAndValidateDns, +} from '../utils/fetch.js'; +import type { Dispatcher } from 'undici'; /** * Error thrown when the discovered resource metadata does not match the expected resource. @@ -113,25 +117,9 @@ export class OAuthUtils { ); return null; } - const pinnedIp = resolvedAddrs[0]; - const dispatcher = new Agent({ - connect: { - lookup: ( - _hostname: string, - _options: unknown, - callback: ( - err: NodeJS.ErrnoException | null, - addresses: Array<{ address: string; family: number }>, - ) => void, - ) => { - callback(null, [ - { address: pinnedIp, family: pinnedIp.includes(':') ? 6 : 4 }, - ]); - }, - }, - }); + const dispatcher = createPinnedDispatcher(resolvedAddrs[0]); const response = await fetch(resourceMetadataUrl, { - dispatcher: dispatcher as Dispatcher, + dispatcher, } as RequestInit & { dispatcher: Dispatcher }); if (!response.ok) { return null; @@ -170,25 +158,9 @@ export class OAuthUtils { ); return null; } - const pinnedIp = resolvedAddrs[0]; - const dispatcher = new Agent({ - connect: { - lookup: ( - _hostname: string, - _options: unknown, - callback: ( - err: NodeJS.ErrnoException | null, - addresses: Array<{ address: string; family: number }>, - ) => void, - ) => { - callback(null, [ - { address: pinnedIp, family: pinnedIp.includes(':') ? 6 : 4 }, - ]); - }, - }, - }); + const dispatcher = createPinnedDispatcher(resolvedAddrs[0]); const response = await fetch(authServerMetadataUrl, { - dispatcher: dispatcher as Dispatcher, + dispatcher, } as RequestInit & { dispatcher: Dispatcher }); if (!response.ok) { return null; diff --git a/packages/core/src/tools/web-fetch.ts b/packages/core/src/tools/web-fetch.ts index 8101a721fa9..c89a37b23a9 100644 --- a/packages/core/src/tools/web-fetch.ts +++ b/packages/core/src/tools/web-fetch.ts @@ -20,11 +20,11 @@ import { ToolErrorType } from './tool-error.js'; import { getErrorMessage } from '../utils/errors.js'; import { getResponseText } from '../utils/partUtils.js'; import { + createPinnedDispatcher, fetchWithTimeout, isLoopbackHost, resolveAndValidateDns, } from '../utils/fetch.js'; -import { Agent, type Dispatcher } from 'undici'; import { truncateString, wrapUntrusted } from '../utils/textUtils.js'; import { convert } from 'html-to-text'; import { @@ -316,22 +316,7 @@ class WebFetchToolInvocation extends BaseToolInvocation< ); } - const dispatcher = new Agent({ - connect: { - lookup: ( - _hostname: string, - _options: unknown, - callback: ( - err: NodeJS.ErrnoException | null, - addresses: Array<{ address: string; family: number }>, - ) => void, - ) => { - callback(null, [ - { address: pinnedIp, family: pinnedIp.includes(':') ? 6 : 4 }, - ]); - }, - }, - }) as Dispatcher; + const dispatcher = createPinnedDispatcher(pinnedIp); const response = await retryWithBackoff( async () => { @@ -674,22 +659,7 @@ ${aggregatedContent} } try { - const dispatcher = new Agent({ - connect: { - lookup: ( - _hostname: string, - _options: unknown, - callback: ( - err: NodeJS.ErrnoException | null, - addresses: Array<{ address: string; family: number }>, - ) => void, - ) => { - callback(null, [ - { address: pinnedIp, family: pinnedIp.includes(':') ? 6 : 4 }, - ]); - }, - }, - }) as Dispatcher; + const dispatcher = createPinnedDispatcher(pinnedIp); const response = await retryWithBackoff( async () => { diff --git a/packages/core/src/utils/fetch.ts b/packages/core/src/utils/fetch.ts index de0e7da0087..c8f4a6d58cd 100644 --- a/packages/core/src/utils/fetch.ts +++ b/packages/core/src/utils/fetch.ts @@ -6,13 +6,8 @@ import { getErrorMessage, isAbortError } from './errors.js'; import { URL } from 'node:url'; -import type { - Dispatcher} from 'undici'; -import { - Agent, - EnvHttpProxyAgent, - setGlobalDispatcher, -} from 'undici'; +import type { Dispatcher } from 'undici'; +import { Agent, EnvHttpProxyAgent, setGlobalDispatcher } from 'undici'; import ipaddr from 'ipaddr.js'; import { lookup } from 'node:dns/promises'; @@ -150,6 +145,34 @@ export function isAddressPrivate(address: string): boolean { } } +/** + * Creates an undici dispatcher that pins DNS resolution to a specific IP + * address while leaving the request URL and TLS SNI/hostname untouched. + * + * This is used to prevent DNS rebinding: the hostname is resolved and + * validated once by the caller (see resolveAndValidateDns), and the + * connection is then forced to that already-validated address instead of + * re-resolving the hostname at connect time. + */ +export function createPinnedDispatcher(pinnedIp: string): Dispatcher { + return new Agent({ + connect: { + lookup: ( + _hostname: string, + _options: unknown, + callback: ( + err: NodeJS.ErrnoException | null, + addresses: Array<{ address: string; family: number }>, + ) => void, + ) => { + callback(null, [ + { address: pinnedIp, family: pinnedIp.includes(':') ? 6 : 4 }, + ]); + }, + }, + }) as Dispatcher; +} + export async function resolveAndValidateDns(url: string): Promise { try { const parsedUrl = new URL(url);