From 809ea48d08c160a87ff72eb7131abb137ddfe8c5 Mon Sep 17 00:00:00 2001 From: Tomi Turtiainen <10324676+tomi@users.noreply.github.com> Date: Mon, 9 Mar 2026 09:59:03 +0200 Subject: [PATCH] feat(core): Integrate SSRF protection into request helpers (#26581) --- .../__tests__/ssrf-protection.service.test.ts | 83 +++--- .../services/ssrf/ssrf-protection.service.ts | 32 ++- .../src/workflow-execute-additional-data.ts | 8 +- .../core/nodes-testing/node-test-harness.ts | 1 + packages/core/src/execution-engine/index.ts | 15 +- .../request-helper-functions.test.ts | 263 +++++++++++++++++- .../utils/request-helper-functions.ts | 101 +++++-- .../nodes-base/test/nodes/TriggerHelpers.ts | 8 +- 8 files changed, 430 insertions(+), 81 deletions(-) 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 527dfc69614..f4cad17e51b 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 @@ -48,7 +48,7 @@ describe('SsrfProtectionService', () => { ['192.168.255.255', '192.168.0.0/16'], ])('should block %s (in %s)', (ip) => { const { service } = createService(); - const result = service.validateAddress(ip); + const result = service.validateIp(ip); expect(result).toEqual({ allowed: false, reason: 'IP address is blocked', ip }); }); }); @@ -58,7 +58,7 @@ describe('SsrfProtectionService', () => { 'should block IPv4 loopback %s', (ip) => { const { service } = createService(); - expect(service.validateAddress(ip)).toEqual({ + expect(service.validateIp(ip)).toEqual({ allowed: false, reason: 'IP address is blocked', ip, @@ -68,7 +68,7 @@ describe('SsrfProtectionService', () => { it('should block IPv6 loopback ::1', () => { const { service } = createService(); - expect(service.validateAddress('::1')).toEqual({ + expect(service.validateIp('::1')).toEqual({ allowed: false, reason: 'IP address is blocked', ip: '::1', @@ -79,7 +79,7 @@ describe('SsrfProtectionService', () => { describe('blocked link-local addresses', () => { it('should block IPv4 link-local 169.254.1.1', () => { const { service } = createService(); - expect(service.validateAddress('169.254.1.1')).toEqual({ + expect(service.validateIp('169.254.1.1')).toEqual({ allowed: false, reason: 'IP address is blocked', ip: '169.254.1.1', @@ -88,7 +88,7 @@ describe('SsrfProtectionService', () => { it('should block IPv6 link-local fe80::1', () => { const { service } = createService(); - expect(service.validateAddress('fe80::1')).toEqual({ + expect(service.validateIp('fe80::1')).toEqual({ allowed: false, reason: 'IP address is blocked', ip: 'fe80::1', @@ -101,7 +101,7 @@ describe('SsrfProtectionService', () => { 'should block special address %s', (ip) => { const { service } = createService(); - expect(service.validateAddress(ip)).toEqual({ + expect(service.validateIp(ip)).toEqual({ allowed: false, reason: 'IP address is blocked', ip, @@ -115,7 +115,7 @@ describe('SsrfProtectionService', () => { 'should allow public IP %s', (ip) => { const { service } = createService(); - expect(service.validateAddress(ip)).toEqual({ allowed: true }); + expect(service.validateIp(ip)).toEqual({ allowed: true }); }, ); }); @@ -126,7 +126,7 @@ describe('SsrfProtectionService', () => { allowedIpRanges: ['10.0.0.0/8'] as unknown as SsrfProtectionConfig['allowedIpRanges'], }); - expect(service.validateAddress('10.0.0.1')).toEqual({ allowed: true }); + expect(service.validateIp('10.0.0.1')).toEqual({ allowed: true }); }); it('should allow a specific blocked IP in the allowlist', () => { @@ -134,9 +134,9 @@ describe('SsrfProtectionService', () => { allowedIpRanges: ['127.0.0.1/32'] as unknown as SsrfProtectionConfig['allowedIpRanges'], }); - expect(service.validateAddress('127.0.0.1')).toEqual({ allowed: true }); + expect(service.validateIp('127.0.0.1')).toEqual({ allowed: true }); // Other loopback IPs should still be blocked - expect(service.validateAddress('127.0.0.2')).toEqual({ + expect(service.validateIp('127.0.0.2')).toEqual({ allowed: false, reason: 'IP address is blocked', ip: '127.0.0.2', @@ -146,7 +146,7 @@ describe('SsrfProtectionService', () => { it('should return invalid for non-IP strings', () => { const { service } = createService(); - expect(service.validateAddress('not-an-ip')).toEqual({ + expect(service.validateIp('not-an-ip')).toEqual({ allowed: false, reason: 'Invalid IP address', ip: 'not-an-ip', @@ -318,17 +318,13 @@ describe('SsrfProtectionService', () => { }); }); - describe('validateRedirect', () => { - it('should validate redirect targets through the same flow', async () => { + describe('validateRedirectSync', () => { + it('should block direct-IP redirect targets', () => { const { service } = createService(); - const result = await service.validateRedirect('http://127.0.0.1/admin'); - - expect(result).toEqual({ - allowed: false, - reason: 'IP address is blocked', - ip: '127.0.0.1', - }); + expect(() => service.validateRedirectSync('http://127.0.0.1/admin')).toThrow( + 'IP address is blocked', + ); }); it('should block redirect chains from public to private', async () => { @@ -342,20 +338,15 @@ describe('SsrfProtectionService', () => { expect(initial).toEqual({ allowed: true }); // Redirect target is private (blocked) - const redirect = await service.validateRedirect('http://192.168.1.1/admin'); - expect(redirect).toEqual({ - allowed: false, - reason: 'IP address is blocked', - ip: '192.168.1.1', - }); + expect(() => service.validateRedirectSync('http://192.168.1.1/admin')).toThrow( + 'IP address is blocked', + ); }); - it('should reject invalid redirect URLs', async () => { + it('should ignore invalid redirect URLs', () => { const { service } = createService(); - const result = await service.validateRedirect('not-a-url'); - - expect(result).toEqual({ allowed: false, reason: 'Invalid URL', url: 'not-a-url' }); + expect(() => service.validateRedirectSync('not-a-url')).not.toThrow(); }); }); @@ -547,7 +538,7 @@ describe('SsrfProtectionService', () => { it('should block ::ffff:127.0.0.1', () => { const { service } = createService(); - const result = service.validateAddress('::ffff:127.0.0.1'); + const result = service.validateIp('::ffff:127.0.0.1'); expect(result).toEqual({ allowed: false, @@ -558,7 +549,7 @@ describe('SsrfProtectionService', () => { it('should block ::ffff:10.0.0.1', () => { const { service } = createService(); - expect(service.validateAddress('::ffff:10.0.0.1')).toEqual({ + expect(service.validateIp('::ffff:10.0.0.1')).toEqual({ allowed: false, reason: 'IP address is blocked', ip: '::ffff:10.0.0.1', @@ -567,7 +558,7 @@ describe('SsrfProtectionService', () => { it('should allow ::ffff: with public IP', () => { const { service } = createService(); - expect(service.validateAddress('::ffff:8.8.8.8')).toEqual({ allowed: true }); + expect(service.validateIp('::ffff:8.8.8.8')).toEqual({ allowed: true }); }); }); @@ -588,33 +579,27 @@ describe('SsrfProtectionService', () => { }); describe('redirect chains', () => { - it('should block redirect from public to private IP', async () => { + it('should block redirect from public to private IP', () => { const { service } = createService(); - const redirect = await service.validateRedirect('http://10.0.0.1/internal'); - expect(redirect).toEqual({ - allowed: false, - reason: 'IP address is blocked', - ip: '10.0.0.1', - }); + expect(() => service.validateRedirectSync('http://10.0.0.1/internal')).toThrow( + 'IP address is blocked', + ); }); - it('should block redirect to loopback', async () => { + it('should block redirect to loopback', () => { const { service } = createService(); - const redirect = await service.validateRedirect('http://[::1]/admin'); - expect(redirect).toEqual({ - allowed: false, - reason: 'IP address is blocked', - ip: '::1', - }); + expect(() => service.validateRedirectSync('http://[::1]/admin')).toThrow( + 'IP address is blocked', + ); }); }); describe('IPv6 unique local addresses', () => { it('should block fc00:: addresses', () => { const { service } = createService(); - expect(service.validateAddress('fc00::1')).toEqual({ + expect(service.validateIp('fc00::1')).toEqual({ allowed: false, reason: 'IP address is blocked', ip: 'fc00::1', @@ -623,7 +608,7 @@ describe('SsrfProtectionService', () => { it('should block fd00:: addresses', () => { const { service } = createService(); - expect(service.validateAddress('fd00::1')).toEqual({ + expect(service.validateIp('fd00::1')).toEqual({ allowed: false, reason: 'IP address is blocked', ip: 'fd00::1', diff --git a/packages/cli/src/services/ssrf/ssrf-protection.service.ts b/packages/cli/src/services/ssrf/ssrf-protection.service.ts index 4fcd7e817da..5ad92278b79 100644 --- a/packages/cli/src/services/ssrf/ssrf-protection.service.ts +++ b/packages/cli/src/services/ssrf/ssrf-protection.service.ts @@ -82,7 +82,7 @@ export class SsrfProtectionService { const cleanIp = this.normalizeIpInHostname(hostname); if (isIP(cleanIp)) { - return this.validateAddress(cleanIp); + return this.validateIp(cleanIp); } // Resolve hostname via DNS and validate all IPs @@ -92,7 +92,7 @@ export class SsrfProtectionService { } for (const ip of ips) { - const result = this.validateAddress(ip.address); + const result = this.validateIp(ip.address); if (!result.allowed) { return result; } @@ -104,7 +104,7 @@ export class SsrfProtectionService { /** * Validate a single IP address against the allowlist and blocklist. */ - validateAddress(ip: string): SsrfCheckResult { + validateIp(ip: string): SsrfCheckResult { const family = this.getIpFamily(ip); if (family === null) { return { allowed: false, reason: 'Invalid IP address', ip }; @@ -151,15 +151,31 @@ export class SsrfProtectionService { } /** - * Validate a redirect target URL through the same validation flow. + * Synchronous redirect validation for use in axios beforeRedirect callback. + * Validates direct-IP redirect targets immediately. Hostname-based redirect + * targets are covered by the secureLookup on the redirect agent. + * Throws SsrfBlockedIpError if the redirect target is blocked. */ - async validateRedirect(redirectUrl: string): Promise { - return await this.validateUrl(redirectUrl); + validateRedirectSync(url: string): void { + const parsed = this.tryParseUrl(url); + if (!parsed) return; + + const { hostname } = parsed; + + if (this.allowedHostnameMatcher.matches(hostname)) return; + + const cleanIp = this.normalizeIpInHostname(hostname); + if (isIP(cleanIp)) { + const result = this.validateIp(cleanIp); + if (!result.allowed) { + throw new SsrfBlockedIpError(cleanIp, hostname); + } + } } /** * Normalize IPv6 bracket notation from a URL hostname. - * E.g. `[::1]` → `::1`, `127.0.0.1` → `127.0.0.1`. + * E.g. `[::1]` -> `::1`, `127.0.0.1` -> `127.0.0.1`. */ private normalizeIpInHostname(hostname: string): string { return hostname.startsWith('[') && hostname.endsWith(']') ? hostname.slice(1, -1) : hostname; @@ -176,7 +192,7 @@ export class SsrfProtectionService { } for (const ip of resolved) { - const result = this.validateAddress(ip.address); + const result = this.validateIp(ip.address); if (!result.allowed) { throw new SsrfBlockedIpError(ip.address, hostname); } diff --git a/packages/cli/src/workflow-execute-additional-data.ts b/packages/cli/src/workflow-execute-additional-data.ts index 0c6d77588aa..db929416986 100644 --- a/packages/cli/src/workflow-execute-additional-data.ts +++ b/packages/cli/src/workflow-execute-additional-data.ts @@ -4,7 +4,7 @@ /* eslint-disable @typescript-eslint/no-unsafe-assignment */ import type { PushMessage, PushType } from '@n8n/api-types'; import { Logger, ModuleRegistry } from '@n8n/backend-common'; -import { GlobalConfig } from '@n8n/config'; +import { GlobalConfig, SsrfProtectionConfig } from '@n8n/config'; import { ExecutionRepository, WorkflowRepository } from '@n8n/db'; import { Container } from '@n8n/di'; import { ExternalSecretsProxy, WorkflowExecute } from 'n8n-core'; @@ -47,6 +47,7 @@ import type { UpdateExecutionPayload } from '@/interfaces'; import { NodeTypes } from '@/node-types'; import { Push } from '@/push'; import { UrlService } from '@/services/url.service'; +import { SsrfProtectionService } from '@/services/ssrf/ssrf-protection.service'; import { TaskRequester } from '@/task-runners/task-managers/task-requester'; import { findSubworkflowStart } from '@/utils'; import { objectToError } from '@/utils/object-to-error'; @@ -540,6 +541,11 @@ export async function getBase({ getRunnerStatus: (taskType: string) => Container.get(TaskRequester).getRunnerStatus(taskType), }; + const ssrfConfig = Container.get(SsrfProtectionConfig); + if (ssrfConfig.enabled) { + additionalData.ssrfBridge = Container.get(SsrfProtectionService); + } + for (const [moduleName, moduleContext] of Container.get(ModuleRegistry).context.entries()) { // @ts-expect-error Adding an index signature `[key: string]: unknown` // to `IWorkflowExecuteAdditionalData` triggers complex type errors for derived types. diff --git a/packages/core/nodes-testing/node-test-harness.ts b/packages/core/nodes-testing/node-test-harness.ts index 1c3e3c3b9f8..bd0219966ac 100644 --- a/packages/core/nodes-testing/node-test-harness.ts +++ b/packages/core/nodes-testing/node-test-harness.ts @@ -232,6 +232,7 @@ export class NodeTestHarness { // Get from node.parameters currentNodeParameters: undefined, parentCallbackManager: undefined, + ssrfBridge: undefined, }); additionalData.credentialsHelper = credentialsHelper; diff --git a/packages/core/src/execution-engine/index.ts b/packages/core/src/execution-engine/index.ts index 8268635688d..9de8886bc80 100644 --- a/packages/core/src/execution-engine/index.ts +++ b/packages/core/src/execution-engine/index.ts @@ -1,8 +1,20 @@ import type { DataTableProxyProvider, IExecutionContext, IWorkflowSettings } from 'n8n-workflow'; +import type { LookupFunction } from 'node:net'; import type { ExecutionLifecycleHooks } from './execution-lifecycle-hooks'; import type { ExternalSecretsProxy } from './external-secrets-proxy'; +/** + * Narrow interface for SSRF protection, satisfied structurally by SsrfProtectionService. + * Defined here so packages/core can use it without importing from packages/cli. + */ +export interface SsrfBridge { + validateIp(ip: string): { allowed: boolean; reason?: string }; + validateUrl(url: string | URL): Promise<{ allowed: boolean; reason?: string }>; + validateRedirectSync(url: string): void; + createSecureLookup(): LookupFunction; +} + declare module 'n8n-workflow' { interface IWorkflowExecuteAdditionalData { hooks?: ExecutionLifecycleHooks; @@ -13,7 +25,8 @@ declare module 'n8n-workflow' { * that owns the credential to decrypt. */ externalSecretProviderKeysAccessibleByCredential?: Set; - + /** SSRF protection bridge — present only when N8N_SSRF_PROTECTION_ENABLED=true */ + ssrfBridge?: SsrfBridge; 'data-table'?: { dataTableProxyProvider: DataTableProxyProvider }; // Project ID is currently only added on the additionalData if the user // has data table listing permission for that project. We should consider diff --git a/packages/core/src/execution-engine/node-execution-context/utils/__tests__/request-helper-functions.test.ts b/packages/core/src/execution-engine/node-execution-context/utils/__tests__/request-helper-functions.test.ts index 2fedb18f5e5..ba72e8b70b7 100644 --- a/packages/core/src/execution-engine/node-execution-context/utils/__tests__/request-helper-functions.test.ts +++ b/packages/core/src/execution-engine/node-execution-context/utils/__tests__/request-helper-functions.test.ts @@ -13,9 +13,11 @@ import type { PaginationOptions, Workflow, } from 'n8n-workflow'; +import { UserError } from 'n8n-workflow'; import nock from 'nock'; import type { SecureContextOptions } from 'tls'; +import type { SsrfBridge } from '@/execution-engine'; import type { ExecutionLifecycleHooks } from '@/execution-engine/execution-lifecycle-hooks'; import { @@ -36,7 +38,10 @@ describe('Request Helper Functions', () => { const baseUrl = 'https://example.de'; const workflow = mock(); const hooks = mock(); - const additionalData = mock({ hooks }); + const additionalData = mock({ + hooks, + ssrfBridge: undefined, + }); const node = mock(); beforeEach(() => { @@ -813,6 +818,21 @@ describe('Request Helper Functions', () => { scope.done(); }); + test('should ignore invalid baseURL when url is absolute', async () => { + const scope = nock(baseUrl) + .get('/users') + .reply(200, { users: ['John', 'Jane'] }); + + const response = await httpRequest({ + method: 'GET', + url: `${baseUrl}/users`, + baseURL: 'not-a-valid-url', + }); + + expect(response).toEqual({ users: ['John', 'Jane'] }); + scope.done(); + }); + test('should make a POST request with JSON body', async () => { const requestBody = { name: 'John', age: 30 }; const scope = nock(baseUrl) @@ -1416,4 +1436,245 @@ describe('Request Helper Functions', () => { expect(mockThis.helpers.httpRequest).toHaveBeenCalledTimes(2); }); }); + + describe('SSRF protection integration', () => { + const baseUrl = 'https://example.com'; + const workflow = mock(); + const hooks = mock(); + const node = mock(); + + const createSsrfBridge = (overrides?: Partial): SsrfBridge => ({ + validateIp: jest.fn().mockReturnValue({ allowed: true }), + validateUrl: jest.fn().mockResolvedValue({ allowed: true }), + validateRedirectSync: jest.fn(), + createSecureLookup: jest.fn().mockReturnValue(jest.fn()), + ...overrides, + }); + + beforeEach(() => { + nock.cleanAll(); + hooks.runHook.mockClear(); + }); + + describe('httpRequest (modern path)', () => { + test('should work normally when ssrfBridge is absent', async () => { + nock(baseUrl).get('/test').reply(200, { ok: true }); + + const response = await httpRequest({ + method: 'GET', + url: `${baseUrl}/test`, + }); + + expect(response).toEqual({ ok: true }); + }); + + test('should throw UserError when validateUrl blocks a direct IP request', async () => { + const ssrfBridge = createSsrfBridge({ + validateUrl: jest + .fn() + .mockResolvedValue({ allowed: false, reason: 'IP address is blocked' }), + }); + const additionalData = mock({ + hooks, + ssrfBridge, + }); + + const { getRequestHelperFunctions } = await import('../request-helper-functions'); + const helpers = getRequestHelperFunctions(workflow, node, additionalData, null, []); + + await expect( + helpers.httpRequest({ + method: 'GET', + url: 'http://127.0.0.1/secret', + }), + ).rejects.toThrow(UserError); + + expect(ssrfBridge.validateUrl).toHaveBeenCalledWith(new URL('http://127.0.0.1/secret')); + }); + + test('should validate hostname URLs with validateUrl', async () => { + const ssrfBridge = createSsrfBridge(); + const additionalData = mock({ + hooks, + ssrfBridge, + }); + + nock(baseUrl).get('/test').reply(200, { ok: true }); + + const { getRequestHelperFunctions } = await import('../request-helper-functions'); + const helpers = getRequestHelperFunctions(workflow, node, additionalData, null, []); + + const response = await helpers.httpRequest({ + method: 'GET', + url: `${baseUrl}/test`, + }); + + expect(response).toEqual({ ok: true }); + expect(ssrfBridge.validateUrl).toHaveBeenCalledWith(new URL(`${baseUrl}/test`)); + }); + }); + + describe('convertN8nRequestToAxios with ssrfBridge', () => { + test('should inject secureLookup into agent options when no proxy', () => { + const lookupFn = jest.fn(); + const ssrfBridge = createSsrfBridge({ + createSecureLookup: jest.fn().mockReturnValue(lookupFn), + }); + + const axiosConfig = convertN8nRequestToAxios( + { method: 'GET', url: 'https://example.com/test' }, + ssrfBridge, + ); + + expect(ssrfBridge.createSecureLookup).toHaveBeenCalled(); + expect((axiosConfig.httpsAgent as HttpsAgent).options.lookup).toBe(lookupFn); + }); + + test('should NOT inject secureLookup when proxy is configured', () => { + const lookupFn = jest.fn(); + const ssrfBridge = createSsrfBridge({ + createSecureLookup: jest.fn().mockReturnValue(lookupFn), + }); + + const axiosConfig = convertN8nRequestToAxios( + { + method: 'GET', + url: 'https://example.com/test', + proxy: { host: 'my-proxy', port: 8080 }, + }, + ssrfBridge, + ); + + expect((axiosConfig.httpsAgent as HttpsAgent).options.lookup).toBeUndefined(); + }); + + test('should not inject secureLookup when ssrfBridge is absent', () => { + const axiosConfig = convertN8nRequestToAxios({ + method: 'GET', + url: 'https://example.com/test', + }); + + expect((axiosConfig.httpsAgent as HttpsAgent).options.lookup).toBeUndefined(); + }); + }); + + describe('proxyRequestToAxios (legacy path)', () => { + test('should throw UserError when validateUrl blocks a direct IP request', async () => { + const ssrfBridge = createSsrfBridge({ + validateUrl: jest + .fn() + .mockResolvedValue({ allowed: false, reason: 'IP address is blocked' }), + }); + const additionalData = mock({ + hooks, + ssrfBridge, + }); + + await expect( + proxyRequestToAxios(workflow, additionalData, node, 'http://10.0.0.1/internal'), + ).rejects.toThrow(UserError); + + expect(ssrfBridge.validateUrl).toHaveBeenCalledWith(new URL('http://10.0.0.1/internal')); + }); + + test('should validate hostname URLs with validateUrl', async () => { + const ssrfBridge = createSsrfBridge(); + const additionalData = mock({ + hooks, + ssrfBridge, + }); + + nock(baseUrl).get('/test').reply(200, 'ok'); + + const response = await proxyRequestToAxios( + workflow, + additionalData, + node, + `${baseUrl}/test`, + ); + + expect(response).toEqual('ok'); + expect(ssrfBridge.validateUrl).toHaveBeenCalledWith(new URL(`${baseUrl}/test`)); + }); + + test('should validate hostname URLs with baseURL via validateUrl', async () => { + const ssrfBridge = createSsrfBridge(); + const additionalData = mock({ + hooks, + ssrfBridge, + }); + + nock(baseUrl).get('/test').reply(200, 'ok'); + + const response = await proxyRequestToAxios(workflow, additionalData, node, { + baseURL: baseUrl, + url: '/test', + }); + + expect(response).toEqual('ok'); + expect(ssrfBridge.validateUrl).toHaveBeenCalledWith(new URL(`${baseUrl}/test`)); + }); + + test('should work normally when ssrfBridge is absent', async () => { + const additionalData = mock({ + hooks, + ssrfBridge: undefined, + }); + + nock(baseUrl).get('/test').reply(200, 'ok'); + + const response = await proxyRequestToAxios( + workflow, + additionalData, + node, + `${baseUrl}/test`, + ); + + expect(response).toEqual('ok'); + }); + }); + + describe('redirect validation', () => { + test('should call validateRedirectSync on redirect', async () => { + const ssrfBridge = createSsrfBridge(); + const additionalData = mock({ + hooks, + ssrfBridge, + }); + + nock(baseUrl) + .get('/redirect') + .reply(301, '', { Location: `${baseUrl}/target` }); + nock(baseUrl).get('/target').reply(200, 'redirected'); + + const response = await proxyRequestToAxios( + workflow, + additionalData, + node, + `${baseUrl}/redirect`, + ); + + expect(response).toEqual('redirected'); + expect(ssrfBridge.validateRedirectSync).toHaveBeenCalledWith(`${baseUrl}/target`); + }); + + test('should block redirect when validateRedirectSync throws', async () => { + const ssrfBridge = createSsrfBridge({ + validateRedirectSync: jest.fn().mockImplementation(() => { + throw new UserError('SSRF: blocked redirect to internal IP'); + }), + }); + const additionalData = mock({ + hooks, + ssrfBridge, + }); + + nock(baseUrl).get('/redirect').reply(301, '', { Location: 'http://127.0.0.1/evil' }); + + await expect( + proxyRequestToAxios(workflow, additionalData, node, `${baseUrl}/redirect`), + ).rejects.toThrow('SSRF: blocked redirect to internal IP'); + }); + }); + }); }); diff --git a/packages/core/src/execution-engine/node-execution-context/utils/request-helper-functions.ts b/packages/core/src/execution-engine/node-execution-context/utils/request-helper-functions.ts index 19314809878..87735b747a0 100644 --- a/packages/core/src/execution-engine/node-execution-context/utils/request-helper-functions.ts +++ b/packages/core/src/execution-engine/node-execution-context/utils/request-helper-functions.ts @@ -31,6 +31,7 @@ import { NodeApiError, NodeOperationError, NodeSslError, + UserError, isObjectEmpty, ExecutionBaseError, jsonParse, @@ -68,6 +69,7 @@ import clientOAuth1 from 'oauth-1.0a'; import { stringify } from 'qs'; import { Readable } from 'stream'; +import type { SsrfBridge } from '@/execution-engine'; import { createHttpProxyAgent, createHttpsProxyAgent } from '@/http-proxy'; import type { IResponseError } from '@/interfaces'; @@ -91,12 +93,8 @@ axios.defaults.proxy = false; function validateUrl(url?: string): boolean { if (!url) return false; - try { - new URL(url); - return true; - } catch { - return false; - } + + return tryParseUrl(url) !== null; } function isIgnoreStatusErrorConfig( @@ -147,6 +145,7 @@ function setAxiosAgents( config: AxiosRequestConfig, agentOptions?: AgentOptions, proxyConfig?: IHttpRequestOptions['proxy'] | string, + secureLookup?: ReturnType, ): void { if (config.httpAgent || config.httpsAgent) return; @@ -156,8 +155,14 @@ function setAxiosAgents( if (!targetUrl) return; - config.httpAgent = createHttpProxyAgent(customProxyUrl, targetUrl, agentOptions); - config.httpsAgent = createHttpsProxyAgent(customProxyUrl, targetUrl, agentOptions); + // Inject secureLookup only for non-proxy agents. When a proxy is used, + // the lookup option applies to resolving the proxy server hostname, not + // the target. Pre-request validateUrl covers the proxy path. + const effectiveOptions = + secureLookup && !customProxyUrl ? { ...agentOptions, lookup: secureLookup } : agentOptions; + + config.httpAgent = createHttpProxyAgent(customProxyUrl, targetUrl, effectiveOptions); + config.httpsAgent = createHttpsProxyAgent(customProxyUrl, targetUrl, effectiveOptions); } function applyVendorHeaders(config: AxiosRequestConfig) { @@ -212,18 +217,31 @@ const getBeforeRedirectFn = axiosConfig: AxiosRequestConfig, proxyConfig: IHttpRequestOptions['proxy'] | string | undefined, sendCredentialsOnCrossOriginRedirect: boolean, + ssrfBridge?: SsrfBridge, ) => (redirectedRequest: Record) => { - const redirectAgentOptions = { + // SSRF: validate redirect target synchronously for direct-IP URIs. + // Hostname-based redirect targets are caught by secureLookup on the agent. + if (ssrfBridge) { + ssrfBridge.validateRedirectSync(redirectedRequest.href); + } + + const redirectAgentOptions: AgentOptions = { ...agentOptions, servername: redirectedRequest.hostname, }; const customProxyUrl = getUrlFromProxyConfig(proxyConfig); + // Inject secureLookup into redirect agents for non-proxy paths + const effectiveRedirectOptions = + ssrfBridge && !customProxyUrl + ? { ...redirectAgentOptions, lookup: ssrfBridge.createSecureLookup() } + : redirectAgentOptions; + // Create both agents and set them const targetUrl = redirectedRequest.href; - const httpAgent = createHttpProxyAgent(customProxyUrl, targetUrl, redirectAgentOptions); - const httpsAgent = createHttpsProxyAgent(customProxyUrl, targetUrl, redirectAgentOptions); + const httpAgent = createHttpProxyAgent(customProxyUrl, targetUrl, effectiveRedirectOptions); + const httpsAgent = createHttpsProxyAgent(customProxyUrl, targetUrl, effectiveRedirectOptions); redirectedRequest.agent = redirectedRequest.href.startsWith('https://') ? httpsAgent @@ -370,7 +388,7 @@ async function generateContentLengthHeader(config: AxiosRequestConfig) { * @deprecated This is only used by legacy request helpers, that are also deprecated */ // eslint-disable-next-line complexity -export async function parseRequestObject(requestObject: IRequestOptions) { +export async function parseRequestObject(requestObject: IRequestOptions, ssrfBridge?: SsrfBridge) { const axiosConfig: AxiosRequestConfig = {}; if (requestObject.headers !== undefined) { @@ -605,13 +623,15 @@ export async function parseRequestObject(requestObject: IRequestOptions) { axiosConfig.timeout = requestObject.timeout; } - setAxiosAgents(axiosConfig, agentOptions, requestObject.proxy); + const secureLookup = ssrfBridge?.createSecureLookup(); + setAxiosAgents(axiosConfig, agentOptions, requestObject.proxy, secureLookup); axiosConfig.beforeRedirect = getBeforeRedirectFn( agentOptions, axiosConfig, requestObject.proxy, requestObject.sendCredentialsOnCrossOriginRedirect ?? true, + ssrfBridge, ); if (requestObject.useStream) { @@ -680,7 +700,11 @@ export async function proxyRequestToAxios( configObject = uriOrObject ?? {}; } - axiosConfig = Object.assign(axiosConfig, await parseRequestObject(configObject)); + const ssrfBridge = additionalData?.ssrfBridge; + const url = resolveLegacyRequestUrl(configObject); + await validateUrlSsrf(url, ssrfBridge); + + axiosConfig = Object.assign(axiosConfig, await parseRequestObject(configObject, ssrfBridge)); try { const response = await invokeAxios(axiosConfig, configObject.auth); @@ -750,7 +774,10 @@ export async function proxyRequestToAxios( } } -export function convertN8nRequestToAxios(n8nRequest: IHttpRequestOptions): AxiosRequestConfig { +export function convertN8nRequestToAxios( + n8nRequest: IHttpRequestOptions, + ssrfBridge?: SsrfBridge, +): AxiosRequestConfig { // Destructure properties with the same name first. const { headers, method, timeout, auth, proxy, url } = n8nRequest; @@ -790,13 +817,15 @@ export function convertN8nRequestToAxios(n8nRequest: IHttpRequestOptions): Axios if (n8nRequest.skipSslCertificateValidation === true) { agentOptions.rejectUnauthorized = false; } - setAxiosAgents(axiosRequest, agentOptions, proxy); + const secureLookup = ssrfBridge?.createSecureLookup(); + setAxiosAgents(axiosRequest, agentOptions, proxy, secureLookup); axiosRequest.beforeRedirect = getBeforeRedirectFn( agentOptions, axiosRequest, n8nRequest.proxy, n8nRequest.sendCredentialsOnCrossOriginRedirect ?? true, + ssrfBridge, ); if (n8nRequest.arrayFormat !== undefined) { @@ -870,6 +899,33 @@ export function convertN8nRequestToAxios(n8nRequest: IHttpRequestOptions): Axios return axiosRequest; } +function tryParseUrl(url: string): URL | null { + try { + return new URL(url); + } catch { + return null; + } +} + +/** Validates a URL against SSRF protection rules. Throws UserError if blocked. */ +async function validateUrlSsrf(url: string | undefined, ssrfBridge?: SsrfBridge): Promise { + if (!ssrfBridge || !url) return; + + const parsed = tryParseUrl(url); + if (!parsed) return; + + const result = await ssrfBridge.validateUrl(parsed); + if (!result.allowed) { + throw new UserError(`SSRF protection blocked request to ${url}: ${result.reason}`); + } +} + +function resolveLegacyRequestUrl(requestObject: IRequestOptions): string | undefined { + const rawUrl = requestObject.uri?.toString() ?? requestObject.url?.toString(); + const baseURL = requestObject.baseURL?.toString(); + return buildTargetUrl(rawUrl, baseURL) ?? rawUrl; +} + const NoBodyHttpMethods = ['GET', 'HEAD', 'OPTIONS']; /** Remove empty request body on GET, HEAD, and OPTIONS requests */ @@ -882,10 +938,14 @@ export const removeEmptyBody = (requestOptions: IHttpRequestOptions | IRequestOp export async function httpRequest( requestOptions: IHttpRequestOptions, + ssrfBridge?: SsrfBridge, ): Promise { removeEmptyBody(requestOptions); - const axiosRequest = convertN8nRequestToAxios(requestOptions); + const url = buildTargetUrl(requestOptions.url, requestOptions.baseURL) ?? requestOptions.url; + await validateUrlSsrf(url, ssrfBridge); + + const axiosRequest = convertN8nRequestToAxios(requestOptions, ssrfBridge); if ( axiosRequest.data === undefined || (axiosRequest.method !== undefined && axiosRequest.method.toUpperCase() === 'GET') @@ -1331,7 +1391,7 @@ export async function httpRequestWithAuthentication( workflow, node, ); - return await httpRequest(requestOptions); + return await httpRequest(requestOptions, additionalData.ssrfBridge); } catch (error) { // if there is a pre authorization method defined and // the method failed due to unauthorized request @@ -1365,7 +1425,7 @@ export async function httpRequestWithAuthentication( ); } // retry the request - return await httpRequest(requestOptions); + return await httpRequest(requestOptions, additionalData.ssrfBridge); } catch (error) { throw new NodeApiError(this.getNode(), error); } @@ -1735,7 +1795,8 @@ export const getRequestHelperFunctions = ( } return { - httpRequest, + httpRequest: async (requestOptions: IHttpRequestOptions) => + await httpRequest(requestOptions, additionalData.ssrfBridge), requestWithAuthenticationPaginated, async httpRequestWithAuthentication( this, diff --git a/packages/nodes-base/test/nodes/TriggerHelpers.ts b/packages/nodes-base/test/nodes/TriggerHelpers.ts index 14e162f1592..e2266a068f4 100644 --- a/packages/nodes-base/test/nodes/TriggerHelpers.ts +++ b/packages/nodes-base/test/nodes/TriggerHelpers.ts @@ -5,7 +5,7 @@ import get from 'lodash/get'; import merge from 'lodash/merge'; import set from 'lodash/set'; import { PollContext, returnJsonArray } from 'n8n-core'; -import type { InstanceSettings, ExecutionLifecycleHooks } from 'n8n-core'; +import type { InstanceSettings, ExecutionLifecycleHooks, SsrfBridge } from 'n8n-core'; import { ScheduledTaskManager } from 'n8n-core/dist/execution-engine/scheduled-task-manager'; import { createDeferredPromise, @@ -263,6 +263,12 @@ export async function testPollingTriggerNode( }, }), hooks: mock(), + ssrfBridge: { + validateIp: jest.fn().mockReturnValue({ allowed: true }), + validateUrl: jest.fn().mockResolvedValue({ allowed: true }), + validateRedirectSync: jest.fn(), + createSecureLookup: jest.fn().mockReturnValue(jest.fn()), + } as SsrfBridge, }), mode, 'init',