diff --git a/packages/cli/src/modules/instance-ai/__tests__/instance-ai.adapter.service.security.test.ts b/packages/cli/src/modules/instance-ai/__tests__/instance-ai.adapter.service.security.test.ts index b3fb239c6a2..4abd428edcb 100644 --- a/packages/cli/src/modules/instance-ai/__tests__/instance-ai.adapter.service.security.test.ts +++ b/packages/cli/src/modules/instance-ai/__tests__/instance-ai.adapter.service.security.test.ts @@ -46,6 +46,7 @@ import type { EnterpriseWorkflowService } from '@/workflows/workflow.service.ee' import type { ExecutionPersistence } from '@/executions/execution-persistence'; import type { EventService } from '@/events/event.service'; import type { RoleService } from '@/services/role.service'; +import type { SsrfProtectionService } from '@/services/ssrf/ssrf-protection.service'; import type { Telemetry } from '@/telemetry'; jest.mock('@/permissions.ee/check-access'); @@ -127,6 +128,7 @@ const service = new InstanceAiAdapterService( roleService, telemetry, aiBuilderTemporaryWorkflowRepository, + mock(), ); const user = mock({ diff --git a/packages/cli/src/modules/instance-ai/__tests__/instance-ai.adapter.service.test.ts b/packages/cli/src/modules/instance-ai/__tests__/instance-ai.adapter.service.test.ts index 598a3214e9a..18bdcf4947c 100644 --- a/packages/cli/src/modules/instance-ai/__tests__/instance-ai.adapter.service.test.ts +++ b/packages/cli/src/modules/instance-ai/__tests__/instance-ai.adapter.service.test.ts @@ -996,6 +996,7 @@ function createNodeAdapterForTests(nodes: Array>) { {} as unknown as ConstructorParameters[28], {} as unknown as ConstructorParameters[29], {} as unknown as ConstructorParameters[30], + {} as unknown as ConstructorParameters[31], ); ( @@ -1126,6 +1127,7 @@ function createDataTableAdapterForTests(overrides?: { {} as unknown as ConstructorParameters[28], {} as unknown as ConstructorParameters[29], {} as unknown as ConstructorParameters[30], + {} as unknown as ConstructorParameters[31], ); const adapter = service.createContext(mockUser).dataTableService; @@ -1403,6 +1405,7 @@ function createWorkflowAdapterForTests(overrides?: { {} as unknown as ConstructorParameters[28], { track: jest.fn() } as unknown as ConstructorParameters[29], mockAiBuilderTemporaryWorkflowRepository as unknown as AiBuilderTemporaryWorkflowRepository, + {} as unknown as ConstructorParameters[31], ); const context = service.createContext(mockUser, { threadId: 'thread-1' }); @@ -1731,6 +1734,7 @@ function createExecutionAdapterForTests(overrides?: { sharingEnabled?: boolean } mockRoleService as unknown as RoleService, {} as unknown as ConstructorParameters[29], {} as unknown as ConstructorParameters[30], + {} as unknown as ConstructorParameters[31], ); const adapter = service.createContext(mockUser).executionService; @@ -1985,6 +1989,7 @@ function createRunAdapterForTests(workflow: Record) { {} as unknown as ConstructorParameters[28], {} as unknown as ConstructorParameters[29], {} as unknown as ConstructorParameters[30], + {} as unknown as ConstructorParameters[31], ); const adapter = service.createContext(mockUser).executionService; diff --git a/packages/cli/src/modules/instance-ai/instance-ai.adapter.service.ts b/packages/cli/src/modules/instance-ai/instance-ai.adapter.service.ts index f99ab713d17..05fe6ebd75f 100644 --- a/packages/cli/src/modules/instance-ai/instance-ai.adapter.service.ts +++ b/packages/cli/src/modules/instance-ai/instance-ai.adapter.service.ts @@ -115,6 +115,7 @@ import { DynamicNodeParametersService } from '@/services/dynamic-node-parameters import { FolderService } from '@/services/folder.service'; import { ProjectService } from '@/services/project.service.ee'; import { RoleService } from '@/services/role.service'; +import { SsrfProtectionService } from '@/services/ssrf/ssrf-protection.service'; import { TagService } from '@/services/tag.service'; import { WorkflowFinderService } from '@/workflows/workflow-finder.service'; import { WorkflowHistoryService } from '@/workflows/workflow-history/workflow-history.service'; @@ -190,6 +191,7 @@ export class InstanceAiAdapterService { private readonly roleService: RoleService, private readonly telemetry: Telemetry, private readonly aiBuilderTemporaryWorkflowRepository: AiBuilderTemporaryWorkflowRepository, + private readonly ssrfProtectionService: SsrfProtectionService, ) { this.logger = logger.scoped('instance-ai'); this.allowSendingParameterValues = globalConfig.ai.allowSendingParameterValues; @@ -1451,6 +1453,7 @@ export class InstanceAiAdapterService { const fetchCache = this.webResearchCache; const searchCacheRef = this.searchCache; const settingsService = this.settingsService; + const ssrf = this.ssrfProtectionService; const userId = user.id; // Lazy search method that resolves credentials on first call @@ -1509,6 +1512,7 @@ export class InstanceAiAdapterService { maxResponseBytes: options?.maxResponseBytes, timeoutMs: options?.timeoutMs, authorizeUrl: options?.authorizeUrl, + ssrf, }); // Attempt summarization (truncation fallback — no model injection yet) diff --git a/packages/cli/src/modules/instance-ai/web-research/__tests__/fetch-and-extract.test.ts b/packages/cli/src/modules/instance-ai/web-research/__tests__/fetch-and-extract.test.ts index e1206c83db6..f2302df3666 100644 --- a/packages/cli/src/modules/instance-ai/web-research/__tests__/fetch-and-extract.test.ts +++ b/packages/cli/src/modules/instance-ai/web-research/__tests__/fetch-and-extract.test.ts @@ -1,18 +1,38 @@ +import type { Logger } from '@n8n/backend-common'; +import { SsrfProtectionConfig } from '@n8n/config'; +import { mock } from 'jest-mock-extended'; +import type { SsrfBridge } from 'n8n-core'; +import { createResultOk } from 'n8n-workflow'; +import type { LookupFunction } from 'node:net'; + +import type { DnsResolver } from '@/services/ssrf/dns-resolver'; +import { SsrfProtectionService } from '@/services/ssrf/ssrf-protection.service'; + import { fetchAndExtract } from '../fetch-and-extract'; -import * as ssrfGuard from '../ssrf-guard'; -jest.mock('../ssrf-guard'); - -const mockAssertPublicUrl = ssrfGuard.assertPublicUrl as jest.MockedFunction< - typeof ssrfGuard.assertPublicUrl ->; +function createSsrfMock(): jest.Mocked { + const ssrf = mock(); + ssrf.validateUrl.mockResolvedValue(createResultOk(undefined)); + ssrf.validateRedirectSync.mockReturnValue(undefined); + // Lookup is invoked by undici when fetching real hosts. Our tests stub fetch + // itself so the lookup is never called — return a noop. + ssrf.createSecureLookup.mockReturnValue((_h, _o, cb) => + cb(null, '127.0.0.1', 4), + ) as unknown as jest.Mocked['createSecureLookup']; + return ssrf; +} // Helper to create a mock Response function createMockResponse( body: string, - options: { contentType?: string; status?: number; url?: string } = {}, + options: { + contentType?: string; + status?: number; + url?: string; + location?: string; + } = {}, ): Response { - const { contentType = 'text/html', status = 200, url: responseUrl } = options; + const { contentType = 'text/html', status = 200, url: responseUrl, location } = options; const encoder = new TextEncoder(); const stream = new ReadableStream({ start(controller) { @@ -21,22 +41,26 @@ function createMockResponse( }, }); + const headers = new Headers({ 'content-type': contentType }); + if (location) headers.set('location', location); + return { ok: status >= 200 && status < 300, status, - statusText: status === 200 ? 'OK' : 'Not Found', + statusText: status === 200 ? 'OK' : status === 301 ? 'Moved' : 'Not Found', url: responseUrl ?? 'https://example.com', - headers: new Headers({ 'content-type': contentType }), + headers, body: stream, } as unknown as Response; } describe('fetchAndExtract', () => { const originalFetch = globalThis.fetch; + let ssrf: jest.Mocked; beforeEach(() => { jest.clearAllMocks(); - mockAssertPublicUrl.mockResolvedValue(undefined); + ssrf = createSsrfMock(); }); afterAll(() => { @@ -58,7 +82,7 @@ describe('fetchAndExtract', () => { globalThis.fetch = jest.fn().mockResolvedValue(createMockResponse(html)); - const result = await fetchAndExtract('https://example.com/docs'); + const result = await fetchAndExtract('https://example.com/docs', { ssrf }); expect(result.title).toBeTruthy(); expect(result.content).toContain('API Reference'); @@ -82,7 +106,7 @@ describe('fetchAndExtract', () => { globalThis.fetch = jest.fn().mockResolvedValue(createMockResponse(html)); - const result = await fetchAndExtract('https://example.com/table'); + const result = await fetchAndExtract('https://example.com/table', { ssrf }); expect(result.content).toContain('Code'); expect(result.content).toContain('200'); @@ -94,7 +118,7 @@ describe('fetchAndExtract', () => { .fn() .mockResolvedValue(createMockResponse(text, { contentType: 'text/plain' })); - const result = await fetchAndExtract('https://example.com/file.txt'); + const result = await fetchAndExtract('https://example.com/file.txt', { ssrf }); expect(result.content).toBe(text); expect(result.truncated).toBe(false); @@ -107,6 +131,7 @@ describe('fetchAndExtract', () => { .mockResolvedValue(createMockResponse(longText, { contentType: 'text/plain' })); const result = await fetchAndExtract('https://example.com/long', { + ssrf, maxContentLength: 1000, }); @@ -128,7 +153,7 @@ describe('fetchAndExtract', () => { globalThis.fetch = jest.fn().mockResolvedValue(createMockResponse(html)); - const result = await fetchAndExtract('https://example.com/spa'); + const result = await fetchAndExtract('https://example.com/spa', { ssrf }); expect(result.safetyFlags?.jsRenderingSuspected).toBe(true); }); @@ -147,45 +172,134 @@ describe('fetchAndExtract', () => { globalThis.fetch = jest.fn().mockResolvedValue(createMockResponse(html)); - const result = await fetchAndExtract('https://example.com/login'); + const result = await fetchAndExtract('https://example.com/login', { ssrf }); expect(result.safetyFlags?.loginRequired).toBe(true); }); - it('performs post-redirect SSRF check', async () => { - const html = '

Hello

'; - globalThis.fetch = jest - .fn() - .mockResolvedValue(createMockResponse(html, { url: 'https://redirected.example.com' })); - - await fetchAndExtract('https://example.com'); - - // Should be called twice: once for original URL, once for redirect URL - expect(mockAssertPublicUrl).toHaveBeenCalledTimes(2); - expect(mockAssertPublicUrl).toHaveBeenCalledWith('https://example.com'); - expect(mockAssertPublicUrl).toHaveBeenCalledWith('https://redirected.example.com'); - }); - it('handles HTTP errors gracefully', async () => { globalThis.fetch = jest .fn() .mockResolvedValue(createMockResponse('Not Found', { status: 404 })); - const result = await fetchAndExtract('https://example.com/missing'); + const result = await fetchAndExtract('https://example.com/missing', { ssrf }); expect(result.content).toContain('HTTP 404'); expect(result.contentLength).toBe(0); }); - it('skips post-redirect SSRF check when URL did not change', async () => { - const html = '

Hello

'; - globalThis.fetch = jest - .fn() - .mockResolvedValue(createMockResponse(html, { url: 'https://example.com' })); + describe('SSRF integration', () => { + it('validates the initial URL via SsrfBridge', async () => { + const html = '

Hi

'; + globalThis.fetch = jest.fn().mockResolvedValue(createMockResponse(html)); - await fetchAndExtract('https://example.com'); + await fetchAndExtract('https://example.com', { ssrf }); - // Only called once for the original URL - expect(mockAssertPublicUrl).toHaveBeenCalledTimes(1); + expect(ssrf.validateUrl).toHaveBeenCalledWith('https://example.com'); + }); + + it('aborts the fetch if validateUrl rejects', async () => { + const blockedError = new Error('Blocked: 10.0.0.1'); + ssrf.validateUrl.mockResolvedValueOnce({ ok: false, error: blockedError }); + globalThis.fetch = jest.fn(); + + await expect(fetchAndExtract('https://blocked.example', { ssrf })).rejects.toThrow( + 'Blocked: 10.0.0.1', + ); + expect(globalThis.fetch).not.toHaveBeenCalled(); + }); + + it('passes a dispatcher with the secure lookup to fetch', async () => { + const html = '

Hi

'; + const fetchSpy = jest.fn().mockResolvedValue(createMockResponse(html)); + globalThis.fetch = fetchSpy; + + await fetchAndExtract('https://example.com', { ssrf }); + + expect(ssrf.createSecureLookup).toHaveBeenCalled(); + const fetchOptions = fetchSpy.mock.calls[0][1] as { dispatcher?: unknown }; + expect(fetchOptions.dispatcher).toBeDefined(); + }); + + it('validates each redirect target via validateRedirectSync and validateUrl', async () => { + const html = '

Hi

'; + globalThis.fetch = jest + .fn() + .mockResolvedValueOnce( + createMockResponse('', { + status: 301, + location: 'https://final.example.com/page', + }), + ) + .mockResolvedValueOnce(createMockResponse(html)); + + await fetchAndExtract('https://example.com', { ssrf }); + + expect(ssrf.validateUrl).toHaveBeenCalledWith('https://example.com'); + expect(ssrf.validateUrl).toHaveBeenCalledWith('https://final.example.com/page'); + expect(ssrf.validateRedirectSync).toHaveBeenCalledWith('https://final.example.com/page'); + }); + + it('blocks redirects whose direct-IP target is private', async () => { + globalThis.fetch = jest.fn().mockResolvedValueOnce( + createMockResponse('', { + status: 301, + location: 'http://10.0.0.1/secret', + }), + ); + ssrf.validateRedirectSync.mockImplementationOnce(() => { + throw new Error('Blocked: 10.0.0.1'); + }); + + await expect(fetchAndExtract('https://example.com', { ssrf })).rejects.toThrow( + 'Blocked: 10.0.0.1', + ); + }); + + it('defeats DNS rebinding: secure lookup rejects a private IP returned at connect time', async () => { + // End-to-end TOCTOU regression guard. Wires fetchAndExtract to a real + // SsrfProtectionService backed by a mocked DnsResolver that returns a + // public IP on the validation lookup and a private IP on the connect + // lookup. The secure lookup must reject the private IP rather than + // hand it to undici, even though validation passed. + const dnsResolver = mock(); + dnsResolver.lookup + .mockResolvedValueOnce([{ address: '93.184.216.34', family: 4 }]) + .mockResolvedValueOnce([{ address: '10.0.0.1', family: 4 }]); + + const realSsrf = new SsrfProtectionService( + new SsrfProtectionConfig(), + dnsResolver, + mock({ scoped: jest.fn().mockReturnThis() }), + ); + + // Capture the lookup function that fetchAndExtract installs on the dispatcher. + let dispatcherLookup: LookupFunction | undefined; + const realCreate = realSsrf.createSecureLookup.bind(realSsrf); + jest.spyOn(realSsrf, 'createSecureLookup').mockImplementation(() => { + dispatcherLookup = realCreate(); + return dispatcherLookup; + }); + + globalThis.fetch = jest + .fn() + .mockResolvedValue(createMockResponse('x')); + + // Validation lookup returns the public IP, so fetchAndExtract proceeds. + await fetchAndExtract('https://evil.example/', { ssrf: realSsrf }); + expect(dispatcherLookup).toBeDefined(); + + // Simulate undici invoking the dispatcher's lookup at connect time. + // DnsResolver now returns the private IP — the secure lookup must error + // out instead of returning the rebound address. + await expect( + new Promise((resolve, reject) => { + dispatcherLookup!('evil.example', { family: 0 }, (err) => { + if (err) reject(err); + else resolve(); + }); + }), + ).rejects.toThrow(/restricted IP/i); + }); }); }); diff --git a/packages/cli/src/modules/instance-ai/web-research/__tests__/ssrf-guard.test.ts b/packages/cli/src/modules/instance-ai/web-research/__tests__/ssrf-guard.test.ts deleted file mode 100644 index b6f15dfde4e..00000000000 --- a/packages/cli/src/modules/instance-ai/web-research/__tests__/ssrf-guard.test.ts +++ /dev/null @@ -1,162 +0,0 @@ -import * as dns from 'node:dns/promises'; - -import { assertPublicUrl } from '../ssrf-guard'; - -jest.mock('node:dns/promises'); - -const mockLookup = dns.lookup as jest.MockedFunction; - -describe('assertPublicUrl', () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - - it('allows public IP addresses', async () => { - mockLookup.mockResolvedValue([{ address: '93.184.216.34', family: 4 }] as never); - await expect(assertPublicUrl('https://example.com')).resolves.toBeUndefined(); - }); - - it('blocks non-HTTP(S) schemes', async () => { - await expect(assertPublicUrl('ftp://example.com')).rejects.toThrow( - 'scheme "ftp:" is not allowed', - ); - }); - - it('blocks file:// scheme', async () => { - await expect(assertPublicUrl('file:///etc/passwd')).rejects.toThrow( - 'scheme "file:" is not allowed', - ); - }); - - it('blocks 127.x.x.x loopback', async () => { - mockLookup.mockResolvedValue([{ address: '127.0.0.1', family: 4 }] as never); - await expect(assertPublicUrl('https://localhost')).rejects.toThrow('private IP'); - }); - - it('blocks 10.x.x.x private range', async () => { - mockLookup.mockResolvedValue([{ address: '10.0.0.1', family: 4 }] as never); - await expect(assertPublicUrl('https://internal.corp')).rejects.toThrow('private IP'); - }); - - it('blocks 172.16.x.x private range', async () => { - mockLookup.mockResolvedValue([{ address: '172.16.0.1', family: 4 }] as never); - await expect(assertPublicUrl('https://internal.corp')).rejects.toThrow('private IP'); - }); - - it('blocks 192.168.x.x private range', async () => { - mockLookup.mockResolvedValue([{ address: '192.168.1.1', family: 4 }] as never); - await expect(assertPublicUrl('https://home.local')).rejects.toThrow('private IP'); - }); - - it('blocks 169.254.x.x link-local', async () => { - mockLookup.mockResolvedValue([{ address: '169.254.169.254', family: 4 }] as never); - await expect(assertPublicUrl('https://metadata.cloud')).rejects.toThrow('private IP'); - }); - - it('blocks IPv6 loopback ::1', async () => { - mockLookup.mockResolvedValue([{ address: '::1', family: 6 }] as never); - await expect(assertPublicUrl('https://ipv6-local')).rejects.toThrow('private IPv6'); - }); - - it('blocks IPv4 literal private IP', async () => { - await expect(assertPublicUrl('https://192.168.1.1/path')).rejects.toThrow('private IP'); - }); - - it('allows IPv4 literal public IP', async () => { - await expect(assertPublicUrl('https://8.8.8.8/path')).resolves.toBeUndefined(); - }); - - it('handles DNS resolution failure', async () => { - mockLookup.mockRejectedValue(new Error('ENOTFOUND')); - await expect(assertPublicUrl('https://nonexistent.invalid')).rejects.toThrow( - 'DNS resolution failed', - ); - }); - - it('blocks when any resolved address is private', async () => { - mockLookup.mockResolvedValue([ - { address: '93.184.216.34', family: 4 }, - { address: '10.0.0.1', family: 4 }, - ] as never); - await expect(assertPublicUrl('https://dual-stack.example')).rejects.toThrow('private IP'); - }); - - it('blocks 100.64.x.x carrier-grade NAT (RFC 6598)', async () => { - mockLookup.mockResolvedValue([{ address: '100.64.0.1', family: 4 }] as never); - await expect(assertPublicUrl('https://cgnat.internal')).rejects.toThrow('private IP'); - }); - - it('blocks 198.18.x.x benchmarking range', async () => { - mockLookup.mockResolvedValue([{ address: '198.18.0.1', family: 4 }] as never); - await expect(assertPublicUrl('https://bench.internal')).rejects.toThrow('private IP'); - }); - - it('blocks 240.x.x.x reserved range', async () => { - mockLookup.mockResolvedValue([{ address: '240.0.0.1', family: 4 }] as never); - await expect(assertPublicUrl('https://reserved.internal')).rejects.toThrow('private IP'); - }); - - describe('IPv4-mapped IPv6 addresses', () => { - it('blocks ::ffff:127.0.0.1 (loopback)', async () => { - await expect(assertPublicUrl('http://[::ffff:127.0.0.1]:8080/x')).rejects.toThrow('private'); - }); - - it('blocks ::ffff:10.0.0.1 (RFC-1918)', async () => { - await expect(assertPublicUrl('http://[::ffff:10.0.0.1]/x')).rejects.toThrow('private'); - }); - - it('blocks ::ffff:192.168.1.1 (RFC-1918)', async () => { - await expect(assertPublicUrl('http://[::ffff:192.168.1.1]/x')).rejects.toThrow('private'); - }); - - it('blocks ::ffff:169.254.169.254 (link-local / cloud metadata)', async () => { - await expect(assertPublicUrl('http://[::ffff:169.254.169.254]/x')).rejects.toThrow('private'); - }); - - it('blocks DNS-resolved IPv4-mapped IPv6 loopback', async () => { - mockLookup.mockResolvedValue([{ address: '::ffff:127.0.0.1', family: 6 }] as never); - await expect(assertPublicUrl('https://sneaky.example')).rejects.toThrow('private'); - }); - - it('blocks DNS-resolved IPv4-mapped IPv6 private range', async () => { - mockLookup.mockResolvedValue([{ address: '::ffff:10.0.0.1', family: 6 }] as never); - await expect(assertPublicUrl('https://sneaky.example')).rejects.toThrow('private'); - }); - - it('allows ::ffff: mapped public IP', async () => { - await expect(assertPublicUrl('http://[::ffff:8.8.8.8]/x')).resolves.toBeUndefined(); - }); - - it('blocks hex-pair form loopback (::ffff:7f00:1)', async () => { - await expect(assertPublicUrl('http://[::ffff:7f00:1]/')).rejects.toThrow('private'); - }); - - it('blocks hex-pair form private address (::ffff:a00:1)', async () => { - await expect(assertPublicUrl('http://[::ffff:a00:1]/')).rejects.toThrow('private'); - }); - - it('blocks hex-pair form link-local (::ffff:a9fe:a9fe)', async () => { - await expect(assertPublicUrl('http://[::ffff:a9fe:a9fe]/')).rejects.toThrow('private'); - }); - - it('allows hex-pair form public address (::ffff:808:808)', async () => { - await expect(assertPublicUrl('http://[::ffff:808:808]/')).resolves.toBeUndefined(); - }); - }); - - it('blocks decimal IP for loopback (2130706433 = 127.0.0.1)', async () => { - await expect(assertPublicUrl('http://2130706433/')).rejects.toThrow('private IP'); - }); - - it('blocks hex IP for loopback (0x7f000001 = 127.0.0.1)', async () => { - await expect(assertPublicUrl('http://0x7f000001/')).rejects.toThrow('private IP'); - }); - - it('blocks decimal IP for private range (167772161 = 10.0.0.1)', async () => { - await expect(assertPublicUrl('http://167772161/')).rejects.toThrow('private IP'); - }); - - it('blocks hex IP for metadata endpoint (0xa9fea9fe = 169.254.169.254)', async () => { - await expect(assertPublicUrl('http://0xa9fea9fe/')).rejects.toThrow('private IP'); - }); -}); diff --git a/packages/cli/src/modules/instance-ai/web-research/fetch-and-extract.ts b/packages/cli/src/modules/instance-ai/web-research/fetch-and-extract.ts index 2586fbe6085..3dcd3e0ea32 100644 --- a/packages/cli/src/modules/instance-ai/web-research/fetch-and-extract.ts +++ b/packages/cli/src/modules/instance-ai/web-research/fetch-and-extract.ts @@ -1,10 +1,10 @@ -import { Readability } from '@mozilla/readability'; -import { parseHTML } from 'linkedom'; -import TurndownService from 'turndown'; import { gfm } from '@joplin/turndown-plugin-gfm'; - +import { Readability } from '@mozilla/readability'; import type { FetchedPage } from '@n8n/instance-ai'; -import { assertPublicUrl } from './ssrf-guard'; +import { parseHTML } from 'linkedom'; +import type { SsrfBridge } from 'n8n-core'; +import TurndownService from 'turndown'; +import { Agent } from 'undici'; const DEFAULT_TIMEOUT_MS = 30_000; const MAX_TIMEOUT_MS = 120_000; @@ -21,6 +21,11 @@ export interface FetchAndExtractOptions { * Throw to abort the fetch (e.g. for HITL domain approval). */ authorizeUrl?: (url: string) => Promise; + /** + * SSRF guard. The same secure lookup function pins DNS for the actual + * connect, so the IP that passes validation is the one fetch connects to. + */ + ssrf: SsrfBridge; } /** @@ -32,13 +37,13 @@ export interface FetchAndExtractOptions { */ export async function fetchAndExtract( url: string, - options?: FetchAndExtractOptions, + options: FetchAndExtractOptions, ): Promise { - const maxContentLength = options?.maxContentLength ?? DEFAULT_MAX_CONTENT_LENGTH; - const maxResponseBytes = options?.maxResponseBytes ?? MAX_RESPONSE_BYTES; - const timeoutMs = Math.min(options?.timeoutMs ?? DEFAULT_TIMEOUT_MS, MAX_TIMEOUT_MS); + const maxContentLength = options.maxContentLength ?? DEFAULT_MAX_CONTENT_LENGTH; + const maxResponseBytes = options.maxResponseBytes ?? MAX_RESPONSE_BYTES; + const timeoutMs = Math.min(options.timeoutMs ?? DEFAULT_TIMEOUT_MS, MAX_TIMEOUT_MS); - const authorizeUrl = options?.authorizeUrl; + const { authorizeUrl, ssrf } = options; // Manual redirect handling — validate every hop against SSRF guard let currentUrl = url; @@ -46,8 +51,10 @@ export async function fetchAndExtract( let redirectCount = 0; while (redirectCount <= MAX_REDIRECTS) { - await assertPublicUrl(currentUrl); + const validation = await ssrf.validateUrl(currentUrl); + if (!validation.ok) throw validation.error; + const dispatcher = new Agent({ connect: { lookup: ssrf.createSecureLookup() } }); const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), timeoutMs); @@ -60,9 +67,12 @@ export async function fetchAndExtract( 'text/html,application/xhtml+xml,application/xml;q=0.9,text/plain;q=0.8,application/pdf;q=0.7,*/*;q=0.5', }, redirect: 'manual', + // @ts-expect-error dispatcher is a valid undici option for Node.js fetch + dispatcher, }); } finally { clearTimeout(timeout); + await dispatcher.close(); } // Follow redirects manually so each hop is SSRF-checked @@ -78,6 +88,10 @@ export async function fetchAndExtract( // Resolve relative redirect URLs against the current URL currentUrl = new URL(location, currentUrl).href; + // Direct-IP redirect targets are caught here; hostnames are caught by + // validateUrl on the next loop iteration before the dispatcher connects. + ssrf.validateRedirectSync(currentUrl); + // Domain-access authorization for the redirect target if (authorizeUrl) { await authorizeUrl(currentUrl); @@ -86,13 +100,6 @@ export async function fetchAndExtract( continue; } - // Defense-in-depth: if the runtime followed a redirect despite manual mode, - // validate the actual response URL against the SSRF guard. - if (response.url && response.url !== currentUrl) { - await assertPublicUrl(response.url); - currentUrl = response.url; - } - break; } diff --git a/packages/cli/src/modules/instance-ai/web-research/index.ts b/packages/cli/src/modules/instance-ai/web-research/index.ts index 9e0e2f13f42..0cde7c14777 100644 --- a/packages/cli/src/modules/instance-ai/web-research/index.ts +++ b/packages/cli/src/modules/instance-ai/web-research/index.ts @@ -1,4 +1,3 @@ -export { assertPublicUrl } from './ssrf-guard'; export { fetchAndExtract } from './fetch-and-extract'; export type { FetchAndExtractOptions } from './fetch-and-extract'; export { maybeSummarize } from './summarize-content'; diff --git a/packages/cli/src/modules/instance-ai/web-research/ssrf-guard.ts b/packages/cli/src/modules/instance-ai/web-research/ssrf-guard.ts deleted file mode 100644 index 0ad1044fc89..00000000000 --- a/packages/cli/src/modules/instance-ai/web-research/ssrf-guard.ts +++ /dev/null @@ -1,131 +0,0 @@ -import { lookup } from 'node:dns/promises'; - -/** RFC-1918 / loopback / link-local / IETF reserved ranges. */ -const PRIVATE_RANGES = [ - // 10.0.0.0/8 - { start: ip4ToNum('10.0.0.0'), end: ip4ToNum('10.255.255.255') }, - // 172.16.0.0/12 - { start: ip4ToNum('172.16.0.0'), end: ip4ToNum('172.31.255.255') }, - // 192.168.0.0/16 - { start: ip4ToNum('192.168.0.0'), end: ip4ToNum('192.168.255.255') }, - // 127.0.0.0/8 (loopback) - { start: ip4ToNum('127.0.0.0'), end: ip4ToNum('127.255.255.255') }, - // 169.254.0.0/16 (link-local) - { start: ip4ToNum('169.254.0.0'), end: ip4ToNum('169.254.255.255') }, - // 0.0.0.0/8 - { start: ip4ToNum('0.0.0.0'), end: ip4ToNum('0.255.255.255') }, - // 100.64.0.0/10 (Carrier-grade NAT, RFC 6598 — common in cloud VPCs) - { start: ip4ToNum('100.64.0.0'), end: ip4ToNum('100.127.255.255') }, - // 192.0.0.0/24 (IETF protocol assignments, RFC 6890) - { start: ip4ToNum('192.0.0.0'), end: ip4ToNum('192.0.0.255') }, - // 198.18.0.0/15 (Benchmarking, RFC 2544) - { start: ip4ToNum('198.18.0.0'), end: ip4ToNum('198.19.255.255') }, - // 240.0.0.0/4 (Reserved, class E) - { start: ip4ToNum('240.0.0.0'), end: ip4ToNum('255.255.255.255') }, -]; - -/** IPv6 loopback and link-local prefixes. */ -const PRIVATE_IPV6_PREFIXES = ['::1', 'fe80:', 'fd', 'fc']; - -/** Regex for IPv4-mapped IPv6 addresses like ::ffff:127.0.0.1 or ::ffff:7f00:1 */ -const IPV4_MAPPED_IPV6_DOTTED = /^::ffff:(\d+\.\d+\.\d+\.\d+)$/i; -const IPV4_MAPPED_IPV6_HEX = /^::ffff:([0-9a-f]{1,4}):([0-9a-f]{1,4})$/i; - -function ip4ToNum(ip: string): number { - const parts = ip.split('.').map(Number); - // eslint-disable-next-line no-bitwise - return ((parts[0] << 24) | (parts[1] << 16) | (parts[2] << 8) | parts[3]) >>> 0; -} - -function isPrivateIPv4(ip: string): boolean { - const num = ip4ToNum(ip); - return PRIVATE_RANGES.some((r) => num >= r.start && num <= r.end); -} - -/** - * Extract the embedded IPv4 address from an IPv4-mapped IPv6 address. - * Handles both dotted notation (::ffff:127.0.0.1) and hex notation (::ffff:7f00:1). - * Returns null if the address is not an IPv4-mapped IPv6 address. - */ -function extractMappedIPv4(ip: string): string | null { - const dottedMatch = IPV4_MAPPED_IPV6_DOTTED.exec(ip); - if (dottedMatch) return dottedMatch[1]; - - const hexMatch = IPV4_MAPPED_IPV6_HEX.exec(ip); - if (hexMatch) { - const high = parseInt(hexMatch[1], 16); - const low = parseInt(hexMatch[2], 16); - // eslint-disable-next-line no-bitwise - return `${(high >> 8) & 0xff}.${high & 0xff}.${(low >> 8) & 0xff}.${low & 0xff}`; - } - - return null; -} - -function isPrivateIPv6(ip: string): boolean { - const lower = ip.toLowerCase(); - - // Check IPv4-mapped IPv6 addresses (e.g. ::ffff:127.0.0.1, ::ffff:7f00:1) - const mappedIPv4 = extractMappedIPv4(lower); - if (mappedIPv4 !== null) { - return isPrivateIPv4(mappedIPv4); - } - - return PRIVATE_IPV6_PREFIXES.some((prefix) => lower.startsWith(prefix)); -} - -/** - * Validates that a URL points to a public internet host. - * Blocks non-HTTP(S) schemes, private/loopback IPs, and IPv6 link-local. - * - * @throws Error if the URL fails any SSRF check - */ -export async function assertPublicUrl(url: string): Promise { - const parsed = new URL(url); - - // Only allow http and https - if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') { - throw new Error( - `Blocked: scheme "${parsed.protocol}" is not allowed. Only HTTP(S) is supported.`, - ); - } - - // Resolve hostname to IP and check against private ranges - const hostname = parsed.hostname; - - // Check if hostname is already an IP literal - if (/^\d+\.\d+\.\d+\.\d+$/.test(hostname)) { - if (isPrivateIPv4(hostname)) { - throw new Error(`Blocked: ${hostname} resolves to a private IP address.`); - } - return; - } - - if (hostname.startsWith('[') || hostname.includes(':')) { - const bare = hostname.replace(/^\[|\]$/g, ''); - if (isPrivateIPv6(bare)) { - throw new Error(`Blocked: ${hostname} resolves to a private IPv6 address.`); - } - return; - } - - // DNS resolve and check all returned addresses - try { - const result = await lookup(hostname, { all: true }); - for (const { address, family } of result) { - if (family === 4 && isPrivateIPv4(address)) { - throw new Error(`Blocked: ${hostname} resolves to a private IP address (${address}).`); - } - if (family === 6 && isPrivateIPv6(address)) { - throw new Error(`Blocked: ${hostname} resolves to a private IPv6 address (${address}).`); - } - } - } catch (error) { - if (error instanceof Error && error.message.startsWith('Blocked:')) { - throw error; - } - throw new Error( - `DNS resolution failed for ${hostname}: ${error instanceof Error ? error.message : String(error)}`, - ); - } -} diff --git a/packages/cli/src/services/ssrf/__tests__/ssrf-protection.service.test.ts b/packages/cli/src/services/ssrf/__tests__/ssrf-protection.service.test.ts index 6e5b5306559..b5bbfc86661 100644 --- a/packages/cli/src/services/ssrf/__tests__/ssrf-protection.service.test.ts +++ b/packages/cli/src/services/ssrf/__tests__/ssrf-protection.service.test.ts @@ -467,33 +467,72 @@ describe('SsrfProtectionService', () => { }); describe('decimal/octal/hex IP representations', () => { - it('should block decimal IP representation via DNS resolution', async () => { - // 2130706433 = 127.0.0.1 in decimal — URL constructor treats it as hostname - const dnsResolver = createMockDnsResolver(); - dnsResolver.lookup.mockResolvedValue([{ address: '127.0.0.1', family: 4 }]); + it('should block decimal IP representation for loopback', async () => { + // new URL('http://2130706433/').hostname === '127.0.0.1' — normalized before lookup + const { service } = createService(); + expectBlocked(await service.validateUrl('http://2130706433/')); + }); - const { service } = createService({}, dnsResolver); - const result = await service.validateUrl('http://2130706433/'); + it('should block decimal IP representation for RFC-1918', async () => { + // 167772161 === 10.0.0.1 + const { service } = createService(); + expectBlocked(await service.validateUrl('http://167772161/')); + }); - expectBlocked(result); + it('should block hex IP representation for loopback', async () => { + // 0x7f000001 === 127.0.0.1 + const { service } = createService(); + expectBlocked(await service.validateUrl('http://0x7f000001/')); + }); + + it('should block hex IP representation for cloud metadata', async () => { + // 0xa9fea9fe === 169.254.169.254 + const { service } = createService(); + expectBlocked(await service.validateUrl('http://0xa9fea9fe/')); }); }); describe('IPv6-mapped IPv4 addresses', () => { - it('should block ::ffff:127.0.0.1', () => { + it('should block ::ffff:127.0.0.1 (dotted form)', () => { const { service } = createService(); expectBlocked(service.validateIp('::ffff:127.0.0.1')); }); - it('should block ::ffff:10.0.0.1', () => { + it('should block ::ffff:10.0.0.1 (dotted form)', () => { const { service } = createService(); expectBlocked(service.validateIp('::ffff:10.0.0.1')); }); + it('should block ::ffff:7f00:1 (hex-compressed form for 127.0.0.1)', () => { + // new URL auto-compresses ::ffff:127.0.0.1 to this form + const { service } = createService(); + expectBlocked(service.validateIp('::ffff:7f00:1')); + }); + + it('should block ::ffff:a9fe:a9fe (hex-compressed form for 169.254.169.254)', () => { + const { service } = createService(); + expectBlocked(service.validateIp('::ffff:a9fe:a9fe')); + }); + + it('should block validateUrl for bracketed IPv4-mapped IPv6 loopback', async () => { + const { service } = createService(); + expectBlocked(await service.validateUrl('http://[::ffff:127.0.0.1]/')); + }); + + it('should block validateUrl for bracketed IPv4-mapped IPv6 metadata', async () => { + const { service } = createService(); + expectBlocked(await service.validateUrl('http://[::ffff:169.254.169.254]/')); + }); + it('should allow ::ffff: with public IP', () => { const { service } = createService(); expectAllowed(service.validateIp('::ffff:8.8.8.8')); }); + + it('should allow ::ffff:808:808 (hex-compressed public IP)', () => { + const { service } = createService(); + expectAllowed(service.validateIp('::ffff:808:808')); + }); }); describe('DNS rebinding prevention (TOCTOU)', () => {