Move secure HTTP client IP validation to connection level (#18006)
## Summary - Refactors SSRF protection from a request-level adapter to connection-level agents, validating resolved IPs in `createConnection` + socket `lookup` events - Sets both `httpAgent` and `httpsAgent` so validation applies regardless of protocol switches during redirects - Caps `maxRedirects` to 10 as defense in depth ## Test plan - [x] All 59 existing + new unit tests pass (agent util, isPrivateIp, service) - [x] No linter errors - [ ] Verify webhook delivery still works with URLs that redirect - [ ] Verify image upload from external URLs still works (relies on redirect following) Made with [Cursor](https://cursor.com) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes core outbound HTTP security behavior and redirect handling, which could impact webhook/image-fetch flows and connection semantics despite improved SSRF coverage. > > **Overview** > Refactors outbound SSRF protection from a custom axios `adapter` to connection-level `httpAgent`/`httpsAgent` created by new `createSsrfSafeAgent`, which blocks private IP literals up front and validates DNS-resolved IPs via the socket `lookup` event. > > When safe mode is enabled, `SecureHttpClientService.getHttpClient` now always installs both agents and enforces a capped `maxRedirects` (default `5`), and the old `getSecureAxiosAdapter` implementation/tests/types are removed. `isPrivateIp` is tightened/expanded to treat `0.0.0.0/8` as private and avoid misclassifying bare IPv4 decimals as IPv6. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 8261da4ff05ba3bca3318ad647c04faf6603d91a. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Félix Malfait
+40
-2
@@ -1,3 +1,6 @@
|
||||
import * as http from 'http';
|
||||
import * as https from 'https';
|
||||
|
||||
import { SecureHttpClientService } from 'src/engine/core-modules/secure-http-client/secure-http-client.service';
|
||||
import { type TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';
|
||||
|
||||
@@ -25,14 +28,49 @@ describe('SecureHttpClientService', () => {
|
||||
expect(client.defaults.httpsAgent).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should return an axios instance with secure adapter when safe mode is on', () => {
|
||||
it('should return an axios instance with SSRF-safe agents when safe mode is on', () => {
|
||||
const service = new SecureHttpClientService(
|
||||
createMockConfigService({ OUTBOUND_HTTP_SAFE_MODE_ENABLED: true }),
|
||||
);
|
||||
const client = service.getHttpClient();
|
||||
|
||||
expect(client).toBeDefined();
|
||||
expect(typeof client.defaults.adapter).toBe('function');
|
||||
expect(client.defaults.httpAgent).toBeInstanceOf(http.Agent);
|
||||
expect(client.defaults.httpsAgent).toBeInstanceOf(https.Agent);
|
||||
});
|
||||
|
||||
it('should default maxRedirects to 5 when safe mode is on', () => {
|
||||
const service = new SecureHttpClientService(
|
||||
createMockConfigService({ OUTBOUND_HTTP_SAFE_MODE_ENABLED: true }),
|
||||
);
|
||||
const client = service.getHttpClient();
|
||||
|
||||
expect(client.defaults.maxRedirects).toBe(5);
|
||||
});
|
||||
|
||||
it('should cap maxRedirects when caller requests more', () => {
|
||||
const service = new SecureHttpClientService(
|
||||
createMockConfigService({ OUTBOUND_HTTP_SAFE_MODE_ENABLED: true }),
|
||||
);
|
||||
const client = service.getHttpClient({ maxRedirects: 100 });
|
||||
|
||||
expect(client.defaults.maxRedirects).toBe(5);
|
||||
});
|
||||
|
||||
it('should respect caller maxRedirects when lower than cap', () => {
|
||||
const service = new SecureHttpClientService(
|
||||
createMockConfigService({ OUTBOUND_HTTP_SAFE_MODE_ENABLED: true }),
|
||||
);
|
||||
const client = service.getHttpClient({ maxRedirects: 2 });
|
||||
|
||||
expect(client.defaults.maxRedirects).toBe(2);
|
||||
});
|
||||
|
||||
it('should not set maxRedirects when safe mode is off', () => {
|
||||
const service = new SecureHttpClientService(createMockConfigService());
|
||||
const client = service.getHttpClient();
|
||||
|
||||
expect(client.defaults.maxRedirects).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should pass through axios config like baseURL', () => {
|
||||
|
||||
+14
-2
@@ -2,11 +2,13 @@ import { Injectable, Logger } from '@nestjs/common';
|
||||
|
||||
import axios, { type AxiosInstance, type CreateAxiosDefaults } from 'axios';
|
||||
|
||||
import { getSecureAxiosAdapter } from 'src/engine/core-modules/secure-http-client/utils/get-secure-axios-adapter.util';
|
||||
import { createSsrfSafeAgent } from 'src/engine/core-modules/secure-http-client/utils/create-ssrf-safe-agent.util';
|
||||
import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';
|
||||
|
||||
import { type OutboundRequestContext } from './outbound-request-context.type';
|
||||
|
||||
const MAX_REDIRECTS = 5;
|
||||
|
||||
@Injectable()
|
||||
export class SecureHttpClientService {
|
||||
private readonly logger = new Logger(SecureHttpClientService.name);
|
||||
@@ -14,6 +16,8 @@ export class SecureHttpClientService {
|
||||
constructor(private readonly twentyConfigService: TwentyConfigService) {}
|
||||
|
||||
// Returns an SSRF-protected HTTP client for external requests.
|
||||
// Protection is enforced at the connection level via custom agents
|
||||
// that validate resolved IPs, which covers redirects automatically.
|
||||
// When context is provided, outbound requests are logged with
|
||||
// workspace/user info for GuardDuty correlation.
|
||||
getHttpClient(
|
||||
@@ -25,7 +29,15 @@ export class SecureHttpClientService {
|
||||
);
|
||||
|
||||
const client = isSafeModeEnabled
|
||||
? axios.create({ ...config, adapter: getSecureAxiosAdapter() })
|
||||
? axios.create({
|
||||
...config,
|
||||
httpAgent: createSsrfSafeAgent('http'),
|
||||
httpsAgent: createSsrfSafeAgent('https'),
|
||||
maxRedirects: Math.min(
|
||||
config?.maxRedirects ?? MAX_REDIRECTS,
|
||||
MAX_REDIRECTS,
|
||||
),
|
||||
})
|
||||
: axios.create(config);
|
||||
|
||||
if (context) {
|
||||
|
||||
+275
@@ -0,0 +1,275 @@
|
||||
import { EventEmitter } from 'events';
|
||||
import * as http from 'http';
|
||||
import * as https from 'https';
|
||||
import { type Socket } from 'net';
|
||||
|
||||
import { createSsrfSafeAgent } from 'src/engine/core-modules/secure-http-client/utils/create-ssrf-safe-agent.util';
|
||||
|
||||
const createMockSocket = (): Socket & { destroy: jest.Mock } => {
|
||||
const emitter = new EventEmitter();
|
||||
|
||||
return Object.assign(emitter, {
|
||||
destroy: jest.fn(),
|
||||
// Minimal Socket stubs to avoid type errors
|
||||
connecting: false,
|
||||
writable: true,
|
||||
}) as unknown as Socket & { destroy: jest.Mock };
|
||||
};
|
||||
|
||||
describe('createSsrfSafeAgent', () => {
|
||||
let mockSocket: ReturnType<typeof createMockSocket>;
|
||||
let createConnectionSpy: jest.SpyInstance;
|
||||
|
||||
beforeEach(() => {
|
||||
mockSocket = createMockSocket();
|
||||
|
||||
createConnectionSpy = jest
|
||||
.spyOn(http.Agent.prototype, 'createConnection')
|
||||
.mockReturnValue(mockSocket as any);
|
||||
|
||||
jest
|
||||
.spyOn(https.Agent.prototype, 'createConnection')
|
||||
.mockReturnValue(mockSocket as any);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
jest.restoreAllMocks();
|
||||
});
|
||||
|
||||
describe('agent creation', () => {
|
||||
it('should return an http.Agent for http protocol', () => {
|
||||
const agent = createSsrfSafeAgent('http');
|
||||
|
||||
expect(agent).toBeInstanceOf(http.Agent);
|
||||
});
|
||||
|
||||
it('should return an https.Agent for https protocol', () => {
|
||||
const agent = createSsrfSafeAgent('https');
|
||||
|
||||
expect(agent).toBeInstanceOf(https.Agent);
|
||||
});
|
||||
});
|
||||
|
||||
describe('IP literal blocking in createConnection', () => {
|
||||
it('should throw for loopback IP 127.0.0.1', () => {
|
||||
const agent = createSsrfSafeAgent('http');
|
||||
|
||||
expect(() => {
|
||||
agent.createConnection({ host: '127.0.0.1' } as any, jest.fn() as any);
|
||||
}).toThrow('Request to internal IP address 127.0.0.1 is not allowed.');
|
||||
});
|
||||
|
||||
it('should throw for private IP 10.0.0.1', () => {
|
||||
const agent = createSsrfSafeAgent('http');
|
||||
|
||||
expect(() => {
|
||||
agent.createConnection({ host: '10.0.0.1' } as any, jest.fn() as any);
|
||||
}).toThrow('Request to internal IP address 10.0.0.1 is not allowed.');
|
||||
});
|
||||
|
||||
it('should throw for private IP 192.168.1.1', () => {
|
||||
const agent = createSsrfSafeAgent('http');
|
||||
|
||||
expect(() => {
|
||||
agent.createConnection(
|
||||
{ host: '192.168.1.1' } as any,
|
||||
jest.fn() as any,
|
||||
);
|
||||
}).toThrow('Request to internal IP address 192.168.1.1 is not allowed.');
|
||||
});
|
||||
|
||||
it('should throw for private IP 172.16.0.1', () => {
|
||||
const agent = createSsrfSafeAgent('http');
|
||||
|
||||
expect(() => {
|
||||
agent.createConnection({ host: '172.16.0.1' } as any, jest.fn() as any);
|
||||
}).toThrow('Request to internal IP address 172.16.0.1 is not allowed.');
|
||||
});
|
||||
|
||||
it('should throw for link-local IP 169.254.169.254', () => {
|
||||
const agent = createSsrfSafeAgent('http');
|
||||
|
||||
expect(() => {
|
||||
agent.createConnection(
|
||||
{ host: '169.254.169.254' } as any,
|
||||
jest.fn() as any,
|
||||
);
|
||||
}).toThrow(
|
||||
'Request to internal IP address 169.254.169.254 is not allowed.',
|
||||
);
|
||||
});
|
||||
|
||||
it('should allow public IP 93.184.216.34', () => {
|
||||
const agent = createSsrfSafeAgent('http');
|
||||
|
||||
agent.createConnection(
|
||||
{ host: '93.184.216.34' } as any,
|
||||
jest.fn() as any,
|
||||
);
|
||||
|
||||
expect(createConnectionSpy).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should allow hostnames (validated later via DNS lookup)', () => {
|
||||
const agent = createSsrfSafeAgent('http');
|
||||
|
||||
agent.createConnection({ host: 'example.com' } as any, jest.fn() as any);
|
||||
|
||||
expect(createConnectionSpy).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('DNS lookup validation via socket event', () => {
|
||||
it('should destroy socket when DNS resolves to loopback IP', () => {
|
||||
const agent = createSsrfSafeAgent('http');
|
||||
|
||||
agent.createConnection({ host: 'evil.com' } as any, jest.fn() as any);
|
||||
|
||||
mockSocket.emit('lookup', null, '127.0.0.1', 4, 'evil.com');
|
||||
|
||||
expect(mockSocket.destroy).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
message: expect.stringContaining('127.0.0.1'),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should destroy socket when DNS resolves to 10.x.x.x', () => {
|
||||
const agent = createSsrfSafeAgent('http');
|
||||
|
||||
agent.createConnection({ host: 'evil.com' } as any, jest.fn() as any);
|
||||
|
||||
mockSocket.emit('lookup', null, '10.0.0.1', 4, 'evil.com');
|
||||
|
||||
expect(mockSocket.destroy).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
message: expect.stringContaining('10.0.0.1'),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should destroy socket when DNS resolves to 192.168.x.x', () => {
|
||||
const agent = createSsrfSafeAgent('http');
|
||||
|
||||
agent.createConnection({ host: 'evil.com' } as any, jest.fn() as any);
|
||||
|
||||
mockSocket.emit('lookup', null, '192.168.1.1', 4, 'evil.com');
|
||||
|
||||
expect(mockSocket.destroy).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
message: expect.stringContaining('192.168.1.1'),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should destroy socket when DNS resolves to cloud metadata IP', () => {
|
||||
const agent = createSsrfSafeAgent('http');
|
||||
|
||||
agent.createConnection(
|
||||
{ host: 'metadata.internal' } as any,
|
||||
jest.fn() as any,
|
||||
);
|
||||
|
||||
mockSocket.emit(
|
||||
'lookup',
|
||||
null,
|
||||
'169.254.169.254',
|
||||
4,
|
||||
'metadata.internal',
|
||||
);
|
||||
|
||||
expect(mockSocket.destroy).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
message: expect.stringContaining('169.254.169.254'),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should not destroy socket when DNS resolves to public IP', () => {
|
||||
const agent = createSsrfSafeAgent('http');
|
||||
|
||||
agent.createConnection({ host: 'example.com' } as any, jest.fn() as any);
|
||||
|
||||
mockSocket.emit('lookup', null, '93.184.216.34', 4, 'example.com');
|
||||
|
||||
expect(mockSocket.destroy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should not destroy socket on DNS lookup error', () => {
|
||||
const agent = createSsrfSafeAgent('http');
|
||||
|
||||
agent.createConnection(
|
||||
{ host: 'nonexistent.example' } as any,
|
||||
jest.fn() as any,
|
||||
);
|
||||
|
||||
mockSocket.emit(
|
||||
'lookup',
|
||||
new Error('ENOTFOUND'),
|
||||
'',
|
||||
4,
|
||||
'nonexistent.example',
|
||||
);
|
||||
|
||||
expect(mockSocket.destroy).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('HTTPS agent', () => {
|
||||
it('should block private IPs for HTTPS connections', () => {
|
||||
const agent = createSsrfSafeAgent('https');
|
||||
|
||||
expect(() => {
|
||||
agent.createConnection({ host: '127.0.0.1' } as any, jest.fn() as any);
|
||||
}).toThrow('Request to internal IP address 127.0.0.1 is not allowed.');
|
||||
});
|
||||
|
||||
it('should validate DNS lookups for HTTPS connections', () => {
|
||||
const agent = createSsrfSafeAgent('https');
|
||||
|
||||
agent.createConnection({ host: 'evil.com' } as any, jest.fn() as any);
|
||||
|
||||
mockSocket.emit('lookup', null, '10.0.0.1', 4, 'evil.com');
|
||||
|
||||
expect(mockSocket.destroy).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
message: expect.stringContaining('10.0.0.1'),
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('IPv6 handling', () => {
|
||||
it('should block IPv6 loopback ::1 as IP literal', () => {
|
||||
const agent = createSsrfSafeAgent('http');
|
||||
|
||||
expect(() => {
|
||||
agent.createConnection({ host: '::1' } as any, jest.fn() as any);
|
||||
}).toThrow('Request to internal IP address ::1 is not allowed.');
|
||||
});
|
||||
|
||||
it('should destroy socket when DNS resolves to IPv6 private address', () => {
|
||||
const agent = createSsrfSafeAgent('http');
|
||||
|
||||
agent.createConnection({ host: 'evil.com' } as any, jest.fn() as any);
|
||||
|
||||
mockSocket.emit('lookup', null, 'fe80::1', 6, 'evil.com');
|
||||
|
||||
expect(mockSocket.destroy).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
message: expect.stringContaining('fe80::1'),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should allow public IPv6 addresses', () => {
|
||||
const agent = createSsrfSafeAgent('http');
|
||||
|
||||
agent.createConnection({ host: 'example.com' } as any, jest.fn() as any);
|
||||
|
||||
mockSocket.emit('lookup', null, '2001:4860:4860::8888', 6, 'example.com');
|
||||
|
||||
expect(mockSocket.destroy).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
-443
@@ -1,443 +0,0 @@
|
||||
import * as http from 'http';
|
||||
import * as https from 'https';
|
||||
|
||||
import { AxiosHeaders, type InternalAxiosRequestConfig } from 'axios';
|
||||
|
||||
import { type SecureAdapterDependencies } from 'src/engine/core-modules/secure-http-client/utils/secure-adapter-dependencies.type';
|
||||
import { getSecureAxiosAdapter } from 'src/engine/core-modules/secure-http-client/utils/get-secure-axios-adapter.util';
|
||||
|
||||
describe('getSecureAxiosAdapter', () => {
|
||||
let mockDnsLookup: jest.Mock;
|
||||
let mockHttpAdapter: jest.Mock;
|
||||
let dependencies: SecureAdapterDependencies;
|
||||
|
||||
beforeEach(() => {
|
||||
mockDnsLookup = jest.fn();
|
||||
mockHttpAdapter = jest.fn().mockResolvedValue({ data: 'success' });
|
||||
dependencies = {
|
||||
dnsLookup: mockDnsLookup,
|
||||
httpAdapter: mockHttpAdapter,
|
||||
};
|
||||
});
|
||||
|
||||
describe('URL validation', () => {
|
||||
it('should throw if URL is not provided', async () => {
|
||||
const adapter = getSecureAxiosAdapter(dependencies);
|
||||
const config = { url: undefined } as InternalAxiosRequestConfig;
|
||||
|
||||
await expect(adapter(config)).rejects.toThrow('URL is required');
|
||||
});
|
||||
|
||||
it('should throw for non-http/https protocols', async () => {
|
||||
const adapter = getSecureAxiosAdapter(dependencies);
|
||||
const config = {
|
||||
url: 'file:///etc/passwd',
|
||||
} as InternalAxiosRequestConfig;
|
||||
|
||||
await expect(adapter(config)).rejects.toThrow(
|
||||
'URL should use http/https protocol',
|
||||
);
|
||||
});
|
||||
|
||||
it('should throw for ftp protocol', async () => {
|
||||
const adapter = getSecureAxiosAdapter(dependencies);
|
||||
const config = {
|
||||
url: 'ftp://example.com/file',
|
||||
} as InternalAxiosRequestConfig;
|
||||
|
||||
await expect(adapter(config)).rejects.toThrow(
|
||||
'URL should use http/https protocol',
|
||||
);
|
||||
});
|
||||
|
||||
it('should allow http protocol', async () => {
|
||||
mockDnsLookup.mockResolvedValue({
|
||||
address: '93.184.216.34',
|
||||
family: 4,
|
||||
});
|
||||
|
||||
const adapter = getSecureAxiosAdapter(dependencies);
|
||||
const config = {
|
||||
url: 'http://example.com',
|
||||
headers: new AxiosHeaders(),
|
||||
} as InternalAxiosRequestConfig;
|
||||
|
||||
await adapter(config);
|
||||
|
||||
expect(mockHttpAdapter).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should allow https protocol', async () => {
|
||||
mockDnsLookup.mockResolvedValue({
|
||||
address: '93.184.216.34',
|
||||
family: 4,
|
||||
});
|
||||
|
||||
const adapter = getSecureAxiosAdapter(dependencies);
|
||||
const config = {
|
||||
url: 'https://example.com',
|
||||
headers: new AxiosHeaders(),
|
||||
} as InternalAxiosRequestConfig;
|
||||
|
||||
await adapter(config);
|
||||
|
||||
expect(mockHttpAdapter).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('private IP blocking', () => {
|
||||
it('should block requests to 127.0.0.1', async () => {
|
||||
mockDnsLookup.mockResolvedValue({ address: '127.0.0.1', family: 4 });
|
||||
|
||||
const adapter = getSecureAxiosAdapter(dependencies);
|
||||
const config = {
|
||||
url: 'http://localhost',
|
||||
headers: new AxiosHeaders(),
|
||||
} as InternalAxiosRequestConfig;
|
||||
|
||||
await expect(adapter(config)).rejects.toThrow(
|
||||
'Request to internal IP address 127.0.0.1 is not allowed.',
|
||||
);
|
||||
});
|
||||
|
||||
it('should block requests to 10.x.x.x range', async () => {
|
||||
mockDnsLookup.mockResolvedValue({ address: '10.0.0.1', family: 4 });
|
||||
|
||||
const adapter = getSecureAxiosAdapter(dependencies);
|
||||
const config = {
|
||||
url: 'http://internal.example.com',
|
||||
headers: new AxiosHeaders(),
|
||||
} as InternalAxiosRequestConfig;
|
||||
|
||||
await expect(adapter(config)).rejects.toThrow(
|
||||
'Request to internal IP address 10.0.0.1 is not allowed.',
|
||||
);
|
||||
});
|
||||
|
||||
it('should block requests to 192.168.x.x range', async () => {
|
||||
mockDnsLookup.mockResolvedValue({ address: '192.168.1.1', family: 4 });
|
||||
|
||||
const adapter = getSecureAxiosAdapter(dependencies);
|
||||
const config = {
|
||||
url: 'http://router.local',
|
||||
headers: new AxiosHeaders(),
|
||||
} as InternalAxiosRequestConfig;
|
||||
|
||||
await expect(adapter(config)).rejects.toThrow(
|
||||
'Request to internal IP address 192.168.1.1 is not allowed.',
|
||||
);
|
||||
});
|
||||
|
||||
it('should block requests to 172.16-31.x.x range', async () => {
|
||||
mockDnsLookup.mockResolvedValue({ address: '172.16.0.1', family: 4 });
|
||||
|
||||
const adapter = getSecureAxiosAdapter(dependencies);
|
||||
const config = {
|
||||
url: 'http://internal.corp',
|
||||
headers: new AxiosHeaders(),
|
||||
} as InternalAxiosRequestConfig;
|
||||
|
||||
await expect(adapter(config)).rejects.toThrow(
|
||||
'Request to internal IP address 172.16.0.1 is not allowed.',
|
||||
);
|
||||
});
|
||||
|
||||
it('should block requests to cloud metadata endpoint (169.254.169.254)', async () => {
|
||||
mockDnsLookup.mockResolvedValue({
|
||||
address: '169.254.169.254',
|
||||
family: 4,
|
||||
});
|
||||
|
||||
const adapter = getSecureAxiosAdapter(dependencies);
|
||||
const config = {
|
||||
url: 'http://metadata.google.internal',
|
||||
headers: new AxiosHeaders(),
|
||||
} as InternalAxiosRequestConfig;
|
||||
|
||||
await expect(adapter(config)).rejects.toThrow(
|
||||
'Request to internal IP address 169.254.169.254 is not allowed.',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('DNS rebinding protection', () => {
|
||||
it('should preserve original hostname in URL for TLS validation', async () => {
|
||||
mockDnsLookup.mockResolvedValue({
|
||||
address: '93.184.216.34',
|
||||
family: 4,
|
||||
});
|
||||
|
||||
const adapter = getSecureAxiosAdapter(dependencies);
|
||||
const config = {
|
||||
url: 'https://example.com/api/data',
|
||||
headers: new AxiosHeaders(),
|
||||
} as InternalAxiosRequestConfig;
|
||||
|
||||
await adapter(config);
|
||||
|
||||
expect(mockHttpAdapter).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
url: 'https://example.com/api/data',
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should set httpsAgent with custom lookup for HTTPS requests', async () => {
|
||||
mockDnsLookup.mockResolvedValue({
|
||||
address: '93.184.216.34',
|
||||
family: 4,
|
||||
});
|
||||
|
||||
const adapter = getSecureAxiosAdapter(dependencies);
|
||||
const config = {
|
||||
url: 'https://example.com/api/data',
|
||||
headers: new AxiosHeaders(),
|
||||
} as InternalAxiosRequestConfig;
|
||||
|
||||
await adapter(config);
|
||||
|
||||
expect(config.httpsAgent).toBeInstanceOf(https.Agent);
|
||||
expect(config.httpAgent).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should set httpAgent with custom lookup for HTTP requests', async () => {
|
||||
mockDnsLookup.mockResolvedValue({
|
||||
address: '93.184.216.34',
|
||||
family: 4,
|
||||
});
|
||||
|
||||
const adapter = getSecureAxiosAdapter(dependencies);
|
||||
const config = {
|
||||
url: 'http://example.com/api/data',
|
||||
headers: new AxiosHeaders(),
|
||||
} as InternalAxiosRequestConfig;
|
||||
|
||||
await adapter(config);
|
||||
|
||||
expect(config.httpAgent).toBeInstanceOf(http.Agent);
|
||||
expect(config.httpsAgent).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should use custom lookup that returns validated IP', async () => {
|
||||
mockDnsLookup.mockResolvedValue({
|
||||
address: '93.184.216.34',
|
||||
family: 4,
|
||||
});
|
||||
|
||||
const adapter = getSecureAxiosAdapter(dependencies);
|
||||
const config = {
|
||||
url: 'https://example.com/api/data',
|
||||
headers: new AxiosHeaders(),
|
||||
} as InternalAxiosRequestConfig;
|
||||
|
||||
await adapter(config);
|
||||
|
||||
// Verify the agent's lookup returns our pre-validated IP
|
||||
const agent = config.httpsAgent as https.Agent;
|
||||
const lookupFn = (agent.options as { lookup?: Function }).lookup;
|
||||
|
||||
expect(lookupFn).toBeDefined();
|
||||
|
||||
const lookupResult = await new Promise<{
|
||||
address: string;
|
||||
family: number;
|
||||
}>((resolve) => {
|
||||
lookupFn!(
|
||||
'any-hostname',
|
||||
{},
|
||||
(err: unknown, address: string, family: number) => {
|
||||
resolve({ address, family });
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
expect(lookupResult.address).toBe('93.184.216.34');
|
||||
expect(lookupResult.family).toBe(4);
|
||||
});
|
||||
|
||||
it('should handle IPv6 addresses correctly', async () => {
|
||||
mockDnsLookup.mockResolvedValue({
|
||||
address: '2001:4860:4860::8888',
|
||||
family: 6,
|
||||
});
|
||||
|
||||
const adapter = getSecureAxiosAdapter(dependencies);
|
||||
const config = {
|
||||
url: 'https://example.com/api/data',
|
||||
headers: new AxiosHeaders(),
|
||||
} as InternalAxiosRequestConfig;
|
||||
|
||||
await adapter(config);
|
||||
|
||||
const agent = config.httpsAgent as https.Agent;
|
||||
const lookupFn = (agent.options as { lookup?: Function }).lookup;
|
||||
|
||||
const lookupResult = await new Promise<{
|
||||
address: string;
|
||||
family: number;
|
||||
}>((resolve) => {
|
||||
lookupFn!(
|
||||
'any-hostname',
|
||||
{},
|
||||
(err: unknown, address: string, family: number) => {
|
||||
resolve({ address, family });
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
expect(lookupResult.address).toBe('2001:4860:4860::8888');
|
||||
expect(lookupResult.family).toBe(6);
|
||||
});
|
||||
|
||||
it('should preserve query parameters in URL', async () => {
|
||||
mockDnsLookup.mockResolvedValue({
|
||||
address: '93.184.216.34',
|
||||
family: 4,
|
||||
});
|
||||
|
||||
const adapter = getSecureAxiosAdapter(dependencies);
|
||||
const config = {
|
||||
url: 'https://example.com/api?foo=bar&baz=qux',
|
||||
headers: new AxiosHeaders(),
|
||||
} as InternalAxiosRequestConfig;
|
||||
|
||||
await adapter(config);
|
||||
|
||||
expect(mockHttpAdapter).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
url: 'https://example.com/api?foo=bar&baz=qux',
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should preserve port in URL', async () => {
|
||||
mockDnsLookup.mockResolvedValue({
|
||||
address: '93.184.216.34',
|
||||
family: 4,
|
||||
});
|
||||
|
||||
const adapter = getSecureAxiosAdapter(dependencies);
|
||||
const config = {
|
||||
url: 'https://example.com:8443/api',
|
||||
headers: new AxiosHeaders(),
|
||||
} as InternalAxiosRequestConfig;
|
||||
|
||||
await adapter(config);
|
||||
|
||||
expect(mockHttpAdapter).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
url: 'https://example.com:8443/api',
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('public IP allowlist', () => {
|
||||
it('should allow requests to public IP addresses', async () => {
|
||||
mockDnsLookup.mockResolvedValue({ address: '8.8.8.8', family: 4 });
|
||||
|
||||
const adapter = getSecureAxiosAdapter(dependencies);
|
||||
const config = {
|
||||
url: 'https://dns.google',
|
||||
headers: new AxiosHeaders(),
|
||||
} as InternalAxiosRequestConfig;
|
||||
|
||||
await adapter(config);
|
||||
|
||||
expect(mockHttpAdapter).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should allow requests to standard web servers', async () => {
|
||||
mockDnsLookup.mockResolvedValue({
|
||||
address: '93.184.216.34',
|
||||
family: 4,
|
||||
});
|
||||
|
||||
const adapter = getSecureAxiosAdapter(dependencies);
|
||||
const config = {
|
||||
url: 'https://example.com',
|
||||
headers: new AxiosHeaders(),
|
||||
} as InternalAxiosRequestConfig;
|
||||
|
||||
await adapter(config);
|
||||
|
||||
expect(mockHttpAdapter).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('edge cases', () => {
|
||||
it('should handle URLs with authentication', async () => {
|
||||
mockDnsLookup.mockResolvedValue({
|
||||
address: '93.184.216.34',
|
||||
family: 4,
|
||||
});
|
||||
|
||||
const adapter = getSecureAxiosAdapter(dependencies);
|
||||
const config = {
|
||||
url: 'https://user:pass@example.com/api',
|
||||
headers: new AxiosHeaders(),
|
||||
} as InternalAxiosRequestConfig;
|
||||
|
||||
await adapter(config);
|
||||
|
||||
expect(mockHttpAdapter).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
url: 'https://user:pass@example.com/api',
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should handle URLs with fragments', async () => {
|
||||
mockDnsLookup.mockResolvedValue({
|
||||
address: '93.184.216.34',
|
||||
family: 4,
|
||||
});
|
||||
|
||||
const adapter = getSecureAxiosAdapter(dependencies);
|
||||
const config = {
|
||||
url: 'https://example.com/page#section',
|
||||
headers: new AxiosHeaders(),
|
||||
} as InternalAxiosRequestConfig;
|
||||
|
||||
await adapter(config);
|
||||
|
||||
expect(mockHttpAdapter).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should work without pre-existing headers', async () => {
|
||||
mockDnsLookup.mockResolvedValue({
|
||||
address: '93.184.216.34',
|
||||
family: 4,
|
||||
});
|
||||
|
||||
const adapter = getSecureAxiosAdapter(dependencies);
|
||||
const config = {
|
||||
url: 'https://example.com',
|
||||
headers: undefined,
|
||||
} as unknown as InternalAxiosRequestConfig;
|
||||
|
||||
await adapter(config);
|
||||
|
||||
expect(mockHttpAdapter).toHaveBeenCalled();
|
||||
expect(config.httpsAgent).toBeInstanceOf(https.Agent);
|
||||
});
|
||||
|
||||
it('should work with plain object headers', async () => {
|
||||
mockDnsLookup.mockResolvedValue({
|
||||
address: '93.184.216.34',
|
||||
family: 4,
|
||||
});
|
||||
|
||||
const adapter = getSecureAxiosAdapter(dependencies);
|
||||
const config = {
|
||||
url: 'https://example.com',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
} as unknown as InternalAxiosRequestConfig;
|
||||
|
||||
await adapter(config);
|
||||
|
||||
expect(mockHttpAdapter).toHaveBeenCalled();
|
||||
expect(config.httpsAgent).toBeInstanceOf(https.Agent);
|
||||
});
|
||||
});
|
||||
});
|
||||
+19
@@ -32,6 +32,25 @@ describe('isPrivateIp', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('this-host addresses (0.0.0.0/8)', () => {
|
||||
it('should detect 0.0.0.0 as private', () => {
|
||||
expect(isPrivateIp('0.0.0.0')).toBe(true);
|
||||
});
|
||||
|
||||
it('should detect 0.x.x.x range as private', () => {
|
||||
expect(isPrivateIp('0.0.0.1')).toBe(true);
|
||||
expect(isPrivateIp('0.255.255.255')).toBe(true);
|
||||
});
|
||||
|
||||
it('should detect decimal 0 (shorthand for 0.0.0.0) as private', () => {
|
||||
expect(isPrivateIp('0')).toBe(true);
|
||||
});
|
||||
|
||||
it('should detect IPv4-mapped 0.0.0.0 as private', () => {
|
||||
expect(isPrivateIp('::ffff:0.0.0.0')).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('private IPv4 ranges', () => {
|
||||
it('should detect 10.x.x.x range as private', () => {
|
||||
expect(isPrivateIp('10.0.0.1')).toBe(true);
|
||||
|
||||
+87
@@ -0,0 +1,87 @@
|
||||
import * as http from 'http';
|
||||
import * as https from 'https';
|
||||
import { type Socket } from 'net';
|
||||
import { type Duplex } from 'stream';
|
||||
|
||||
import { isPrivateIp } from 'src/engine/core-modules/secure-http-client/utils/is-private-ip.util';
|
||||
|
||||
// Checks whether a hostname is a private IP literal.
|
||||
// Returns false for domain names — those are validated after DNS
|
||||
// resolution in the socket 'lookup' event handler.
|
||||
const isHostnamePrivateIp = (hostname: string): boolean => {
|
||||
try {
|
||||
return isPrivateIp(hostname);
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
};
|
||||
|
||||
const validateHost = (host?: string) => {
|
||||
if (host && isHostnamePrivateIp(host)) {
|
||||
throw new Error(`Request to internal IP address ${host} is not allowed.`);
|
||||
}
|
||||
};
|
||||
|
||||
// Validates a resolved IP and destroys the socket if it's private.
|
||||
// Fails closed: if the IP cannot be parsed, the socket is destroyed.
|
||||
const attachLookupValidation = (duplex: Duplex): Socket => {
|
||||
// createConnection returns a net.Socket at runtime; the Duplex
|
||||
// return type in @types/node is overly broad.
|
||||
const socket = duplex as Socket;
|
||||
|
||||
socket.on('lookup', (error: Error | null, address: string) => {
|
||||
if (error) {
|
||||
return;
|
||||
}
|
||||
|
||||
try {
|
||||
if (isPrivateIp(address)) {
|
||||
socket.destroy(
|
||||
new Error(
|
||||
`Request to internal IP address ${address} is not allowed.`,
|
||||
),
|
||||
);
|
||||
}
|
||||
} catch {
|
||||
socket.destroy(
|
||||
new Error(
|
||||
`Request to unvalidatable IP address ${address} is not allowed.`,
|
||||
),
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
return socket;
|
||||
};
|
||||
|
||||
// Agents that block connections to private IPs. Validation happens at
|
||||
// the connection level (createConnection + socket 'lookup' event),
|
||||
// which means every connection is checked — including those created
|
||||
// by automatic redirect following.
|
||||
class SsrfSafeHttpAgent extends http.Agent {
|
||||
createConnection(
|
||||
options: http.ClientRequestArgs,
|
||||
callback?: (err: Error, stream: Duplex) => void,
|
||||
): Duplex {
|
||||
validateHost(options.host ?? undefined);
|
||||
|
||||
return attachLookupValidation(super.createConnection(options, callback));
|
||||
}
|
||||
}
|
||||
|
||||
class SsrfSafeHttpsAgent extends https.Agent {
|
||||
createConnection(
|
||||
options: http.ClientRequestArgs,
|
||||
callback?: (err: Error, stream: Duplex) => void,
|
||||
): Duplex {
|
||||
validateHost(options.host ?? undefined);
|
||||
|
||||
return attachLookupValidation(super.createConnection(options, callback));
|
||||
}
|
||||
}
|
||||
|
||||
export const createSsrfSafeAgent = (protocol: 'http' | 'https'): http.Agent => {
|
||||
return protocol === 'https'
|
||||
? new SsrfSafeHttpsAgent()
|
||||
: new SsrfSafeHttpAgent();
|
||||
};
|
||||
-89
@@ -1,89 +0,0 @@
|
||||
import * as dns from 'dns/promises';
|
||||
import * as http from 'http';
|
||||
import * as https from 'https';
|
||||
import { type LookupFunction } from 'net';
|
||||
|
||||
import axios, {
|
||||
type AxiosAdapter,
|
||||
type InternalAxiosRequestConfig,
|
||||
} from 'axios';
|
||||
|
||||
import { isPrivateIp } from 'src/engine/core-modules/secure-http-client/utils/is-private-ip.util';
|
||||
|
||||
import { type SecureAdapterDependencies } from './secure-adapter-dependencies.type';
|
||||
|
||||
const defaultDependencies: SecureAdapterDependencies = {
|
||||
dnsLookup: dns.lookup,
|
||||
httpAdapter: axios.getAdapter('http'),
|
||||
};
|
||||
|
||||
export const getSecureAxiosAdapter = (
|
||||
dependencies: SecureAdapterDependencies = defaultDependencies,
|
||||
): AxiosAdapter => {
|
||||
const { dnsLookup, httpAdapter } = dependencies;
|
||||
|
||||
return async (config: InternalAxiosRequestConfig) => {
|
||||
// Resolve full URL by combining baseURL and url, matching what the
|
||||
// default axios HTTP adapter does internally. Without this, requests
|
||||
// that rely on baseURL (e.g. captcha drivers) would fail the check
|
||||
// below because config.url can be an empty string.
|
||||
const resolvedUrl = config.url
|
||||
? config.baseURL
|
||||
? new URL(config.url, config.baseURL).toString()
|
||||
: config.url
|
||||
: config.baseURL;
|
||||
|
||||
if (!resolvedUrl) {
|
||||
throw new Error('URL is required');
|
||||
}
|
||||
|
||||
const url = new URL(resolvedUrl);
|
||||
|
||||
if (!['http:', 'https:'].includes(url.protocol)) {
|
||||
throw new Error('URL should use http/https protocol');
|
||||
}
|
||||
|
||||
const { address: resolvedIp, family } = await dnsLookup(url.hostname);
|
||||
|
||||
if (isPrivateIp(resolvedIp)) {
|
||||
throw new Error(
|
||||
`Request to internal IP address ${resolvedIp} is not allowed.`,
|
||||
);
|
||||
}
|
||||
|
||||
// Use a custom lookup function that returns our pre-validated IP.
|
||||
// This prevents DNS rebinding attacks while preserving the original
|
||||
// hostname in the URL for proper TLS certificate validation and SNI.
|
||||
// Note: LookupFunction can be called with 2 or 3 arguments, and when
|
||||
// options.all is true, it expects an array of addresses.
|
||||
const ipFamily = family === 6 ? 6 : 4;
|
||||
const secureLookup: LookupFunction = (
|
||||
_hostname,
|
||||
optionsOrCallback,
|
||||
maybeCallback,
|
||||
) => {
|
||||
const lookupOptions =
|
||||
typeof optionsOrCallback === 'object' ? optionsOrCallback : {};
|
||||
const callback =
|
||||
typeof optionsOrCallback === 'function'
|
||||
? optionsOrCallback
|
||||
: maybeCallback;
|
||||
|
||||
if (lookupOptions.all) {
|
||||
(callback as Function)(null, [
|
||||
{ address: resolvedIp, family: ipFamily },
|
||||
]);
|
||||
} else {
|
||||
(callback as Function)(null, resolvedIp, ipFamily);
|
||||
}
|
||||
};
|
||||
|
||||
if (url.protocol === 'https:') {
|
||||
config.httpsAgent = new https.Agent({ lookup: secureLookup });
|
||||
} else {
|
||||
config.httpAgent = new http.Agent({ lookup: secureLookup });
|
||||
}
|
||||
|
||||
return httpAdapter(config);
|
||||
};
|
||||
};
|
||||
+6
-1
@@ -65,7 +65,10 @@ const normalizeToLong = (addr: string) => {
|
||||
return val >>> 0;
|
||||
};
|
||||
|
||||
const isIpV6 = (hostname: string) => ipv6Regex.test(hostname);
|
||||
// IPv6 addresses always contain colons; the colon check prevents the
|
||||
// loose regex from false-positiving on bare decimal/hex IPv4 like '0'.
|
||||
const isIpV6 = (hostname: string) =>
|
||||
hostname.includes(':') && ipv6Regex.test(hostname);
|
||||
|
||||
export const isPrivateIp = (addr: string) => {
|
||||
if (isLoopback(addr)) {
|
||||
@@ -82,6 +85,8 @@ export const isPrivateIp = (addr: string) => {
|
||||
}
|
||||
|
||||
return (
|
||||
// 0.0.0.0/8 — "this host on this network" (RFC 1122), reaches localhost
|
||||
/^(::f{4}:)?0\.([0-9]{1,3})\.([0-9]{1,3})\.([0-9]{1,3})$/i.test(addr) ||
|
||||
/^(::f{4}:)?10\.([0-9]{1,3})\.([0-9]{1,3})\.([0-9]{1,3})$/i.test(addr) ||
|
||||
/^(::f{4}:)?192\.168\.([0-9]{1,3})\.([0-9]{1,3})$/i.test(addr) ||
|
||||
/^(::f{4}:)?172\.(1[6-9]|2\d|30|31)\.([0-9]{1,3})\.([0-9]{1,3})$/i.test(
|
||||
|
||||
-8
@@ -1,8 +0,0 @@
|
||||
import { type AxiosAdapter } from 'axios';
|
||||
|
||||
import type * as dns from 'dns/promises';
|
||||
|
||||
export type SecureAdapterDependencies = {
|
||||
dnsLookup: typeof dns.lookup;
|
||||
httpAdapter: AxiosAdapter;
|
||||
};
|
||||
Reference in New Issue
Block a user