feat(security): implement robust IP validation and safeFetch foundation (#21401)

This commit is contained in:
Alisa
2026-03-09 12:02:07 -07:00
committed by GitHub
parent b68d7bc0f9
commit e92ccec6c8
16 changed files with 612 additions and 27 deletions

View File

@@ -21,11 +21,13 @@
"dist"
],
"dependencies": {
"@a2a-js/sdk": "^0.3.8",
"@a2a-js/sdk": "^0.3.10",
"@bufbuild/protobuf": "^2.11.0",
"@google-cloud/logging": "^11.2.1",
"@google-cloud/opentelemetry-cloud-monitoring-exporter": "^0.21.0",
"@google-cloud/opentelemetry-cloud-trace-exporter": "^3.0.0",
"@google/genai": "1.41.0",
"@grpc/grpc-js": "^1.14.3",
"@iarna/toml": "^2.2.5",
"@joshua.litt/get-ripgrep": "^0.0.3",
"@modelcontextprotocol/sdk": "^1.23.0",
@@ -63,6 +65,7 @@
"html-to-text": "^9.0.5",
"https-proxy-agent": "^7.0.6",
"ignore": "^7.0.0",
"ipaddr.js": "^1.9.1",
"js-yaml": "^4.1.1",
"marked": "^15.0.12",
"mime": "4.0.7",

View File

@@ -25,6 +25,7 @@ import {
import { v4 as uuidv4 } from 'uuid';
import { Agent as UndiciAgent } from 'undici';
import { debugLogger } from '../utils/debugLogger.js';
import { safeLookup } from '../utils/fetch.js';
// Remote agents can take 10+ minutes (e.g. Deep Research).
// Use a dedicated dispatcher so the global 5-min timeout isn't affected.
@@ -32,10 +33,13 @@ const A2A_TIMEOUT = 1800000; // 30 minutes
const a2aDispatcher = new UndiciAgent({
headersTimeout: A2A_TIMEOUT,
bodyTimeout: A2A_TIMEOUT,
connect: {
lookup: safeLookup, // SSRF protection at connection level
},
});
const a2aFetch: typeof fetch = (input, init) =>
// @ts-expect-error The `dispatcher` property is a Node.js extension to fetch not present in standard types.
fetch(input, { ...init, dispatcher: a2aDispatcher });
// eslint-disable-next-line no-restricted-syntax -- TODO: Migrate to safeFetch for SSRF protection
fetch(input, { ...init, dispatcher: a2aDispatcher } as RequestInit);
export type SendMessageResult =
| Message

View File

@@ -700,6 +700,7 @@ async function fetchAndCacheUserInfo(client: OAuth2Client): Promise<void> {
return;
}
// eslint-disable-next-line no-restricted-syntax -- TODO: Migrate to safeFetch for SSRF protection
const response = await fetch(
'https://www.googleapis.com/oauth2/v2/userinfo',
{

View File

@@ -111,6 +111,7 @@ export class MCPOAuthProvider {
scope: config.scopes?.join(' ') || '',
};
// eslint-disable-next-line no-restricted-syntax -- TODO: Migrate to safeFetch for SSRF protection
const response = await fetch(registrationUrl, {
method: 'POST',
headers: {
@@ -300,6 +301,7 @@ export class MCPOAuthProvider {
? { Accept: 'text/event-stream' }
: { Accept: 'application/json' };
// eslint-disable-next-line no-restricted-syntax -- TODO: Migrate to safeFetch for SSRF protection
const response = await fetch(mcpServerUrl, {
method: 'HEAD',
headers,

View File

@@ -97,6 +97,7 @@ export class OAuthUtils {
resourceMetadataUrl: string,
): Promise<OAuthProtectedResourceMetadata | null> {
try {
// eslint-disable-next-line no-restricted-syntax -- TODO: Migrate to safeFetch for SSRF protection
const response = await fetch(resourceMetadataUrl);
if (!response.ok) {
return null;
@@ -121,6 +122,7 @@ export class OAuthUtils {
authServerMetadataUrl: string,
): Promise<OAuthAuthorizationServerMetadata | null> {
try {
// eslint-disable-next-line no-restricted-syntax -- TODO: Migrate to safeFetch for SSRF protection
const response = await fetch(authServerMetadataUrl);
if (!response.ok) {
return null;

View File

@@ -474,6 +474,7 @@ export class ClearcutLogger {
let result: LogResponse = {};
try {
// eslint-disable-next-line no-restricted-syntax -- TODO: Migrate to safeFetch for SSRF protection
const response = await fetch(CLEARCUT_URL, {
method: 'POST',
body: safeJsonStringify(request),

View File

@@ -1903,6 +1903,7 @@ export async function connectToMcpServer(
acceptHeader = 'application/json';
}
// eslint-disable-next-line no-restricted-syntax -- TODO: Migrate to safeFetch for SSRF protection
const response = await fetch(urlToFetch, {
method: 'HEAD',
headers: {

View File

@@ -0,0 +1,291 @@
/**
* @license
* Copyright 2025 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import { describe, it, expect, vi, beforeEach, afterAll } from 'vitest';
import {
isPrivateIp,
isPrivateIpAsync,
isAddressPrivate,
safeLookup,
safeFetch,
fetchWithTimeout,
PrivateIpError,
} from './fetch.js';
import * as dnsPromises from 'node:dns/promises';
import * as dns from 'node:dns';
vi.mock('node:dns/promises', () => ({
lookup: vi.fn(),
}));
// We need to mock node:dns for safeLookup since it uses the callback API
vi.mock('node:dns', () => ({
lookup: vi.fn(),
}));
// Mock global fetch
const originalFetch = global.fetch;
global.fetch = vi.fn();
describe('fetch utils', () => {
beforeEach(() => {
vi.clearAllMocks();
});
afterAll(() => {
global.fetch = originalFetch;
});
describe('isAddressPrivate', () => {
it('should identify private IPv4 addresses', () => {
expect(isAddressPrivate('10.0.0.1')).toBe(true);
expect(isAddressPrivate('127.0.0.1')).toBe(true);
expect(isAddressPrivate('172.16.0.1')).toBe(true);
expect(isAddressPrivate('192.168.1.1')).toBe(true);
});
it('should identify non-routable and reserved IPv4 addresses (RFC 6890)', () => {
expect(isAddressPrivate('0.0.0.0')).toBe(true);
expect(isAddressPrivate('100.64.0.1')).toBe(true);
expect(isAddressPrivate('192.0.0.1')).toBe(true);
expect(isAddressPrivate('192.0.2.1')).toBe(true);
expect(isAddressPrivate('192.88.99.1')).toBe(true);
// Benchmark range (198.18.0.0/15)
expect(isAddressPrivate('198.18.0.0')).toBe(true);
expect(isAddressPrivate('198.18.0.1')).toBe(true);
expect(isAddressPrivate('198.19.255.255')).toBe(true);
expect(isAddressPrivate('198.51.100.1')).toBe(true);
expect(isAddressPrivate('203.0.113.1')).toBe(true);
expect(isAddressPrivate('224.0.0.1')).toBe(true);
expect(isAddressPrivate('240.0.0.1')).toBe(true);
});
it('should identify private IPv6 addresses', () => {
expect(isAddressPrivate('::1')).toBe(true);
expect(isAddressPrivate('fc00::')).toBe(true);
expect(isAddressPrivate('fd00::')).toBe(true);
expect(isAddressPrivate('fe80::')).toBe(true);
expect(isAddressPrivate('febf::')).toBe(true);
});
it('should identify special local addresses', () => {
expect(isAddressPrivate('0.0.0.0')).toBe(true);
expect(isAddressPrivate('::')).toBe(true);
expect(isAddressPrivate('localhost')).toBe(true);
});
it('should identify link-local addresses', () => {
expect(isAddressPrivate('169.254.169.254')).toBe(true);
});
it('should identify IPv4-mapped IPv6 private addresses', () => {
expect(isAddressPrivate('::ffff:127.0.0.1')).toBe(true);
expect(isAddressPrivate('::ffff:10.0.0.1')).toBe(true);
expect(isAddressPrivate('::ffff:169.254.169.254')).toBe(true);
expect(isAddressPrivate('::ffff:192.168.1.1')).toBe(true);
expect(isAddressPrivate('::ffff:172.16.0.1')).toBe(true);
expect(isAddressPrivate('::ffff:0.0.0.0')).toBe(true);
expect(isAddressPrivate('::ffff:100.64.0.1')).toBe(true);
expect(isAddressPrivate('::ffff:a9fe:101')).toBe(true); // 169.254.1.1
});
it('should identify public addresses as non-private', () => {
expect(isAddressPrivate('8.8.8.8')).toBe(false);
expect(isAddressPrivate('93.184.216.34')).toBe(false);
expect(isAddressPrivate('2001:4860:4860::8888')).toBe(false);
expect(isAddressPrivate('::ffff:8.8.8.8')).toBe(false);
});
});
describe('isPrivateIp', () => {
it('should identify private IPs in URLs', () => {
expect(isPrivateIp('http://10.0.0.1/')).toBe(true);
expect(isPrivateIp('https://127.0.0.1:8080/')).toBe(true);
expect(isPrivateIp('http://localhost/')).toBe(true);
expect(isPrivateIp('http://[::1]/')).toBe(true);
});
it('should identify public IPs in URLs as non-private', () => {
expect(isPrivateIp('http://8.8.8.8/')).toBe(false);
expect(isPrivateIp('https://google.com/')).toBe(false);
});
});
describe('isPrivateIpAsync', () => {
it('should identify private IPs directly', async () => {
expect(await isPrivateIpAsync('http://10.0.0.1/')).toBe(true);
});
it('should identify domains resolving to private IPs', async () => {
vi.mocked(dnsPromises.lookup).mockImplementation(
async () =>
// eslint-disable-next-line @typescript-eslint/no-explicit-any
[{ address: '10.0.0.1', family: 4 }] as any,
);
expect(await isPrivateIpAsync('http://malicious.com/')).toBe(true);
});
it('should identify domains resolving to public IPs as non-private', async () => {
vi.mocked(dnsPromises.lookup).mockImplementation(
async () =>
// eslint-disable-next-line @typescript-eslint/no-explicit-any
[{ address: '8.8.8.8', family: 4 }] as any,
);
expect(await isPrivateIpAsync('http://google.com/')).toBe(false);
});
it('should throw error if DNS resolution fails (fail closed)', async () => {
vi.mocked(dnsPromises.lookup).mockRejectedValue(new Error('DNS Error'));
await expect(isPrivateIpAsync('http://unreachable.com/')).rejects.toThrow(
'Failed to verify if URL resolves to private IP',
);
});
it('should return false for invalid URLs instead of throwing verification error', async () => {
expect(await isPrivateIpAsync('not-a-url')).toBe(false);
});
});
describe('safeLookup', () => {
it('should filter out private IPs', async () => {
const addresses = [
{ address: '8.8.8.8', family: 4 },
{ address: '10.0.0.1', family: 4 },
];
vi.mocked(dns.lookup).mockImplementation(((
_h: string,
_o: dns.LookupOptions,
cb: (
err: Error | null,
addr: Array<{ address: string; family: number }>,
) => void,
) => {
cb(null, addresses);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
}) as any);
const result = await new Promise<
Array<{ address: string; family: number }>
>((resolve, reject) => {
safeLookup('example.com', { all: true }, (err, filtered) => {
if (err) reject(err);
else resolve(filtered);
});
});
expect(result).toHaveLength(1);
expect(result[0].address).toBe('8.8.8.8');
});
it('should allow explicit localhost', async () => {
const addresses = [{ address: '127.0.0.1', family: 4 }];
vi.mocked(dns.lookup).mockImplementation(((
_h: string,
_o: dns.LookupOptions,
cb: (
err: Error | null,
addr: Array<{ address: string; family: number }>,
) => void,
) => {
cb(null, addresses);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
}) as any);
const result = await new Promise<
Array<{ address: string; family: number }>
>((resolve, reject) => {
safeLookup('localhost', { all: true }, (err, filtered) => {
if (err) reject(err);
else resolve(filtered);
});
});
expect(result).toHaveLength(1);
expect(result[0].address).toBe('127.0.0.1');
});
it('should error if all resolved IPs are private', async () => {
const addresses = [{ address: '10.0.0.1', family: 4 }];
vi.mocked(dns.lookup).mockImplementation(((
_h: string,
_o: dns.LookupOptions,
cb: (
err: Error | null,
addr: Array<{ address: string; family: number }>,
) => void,
) => {
cb(null, addresses);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
}) as any);
await expect(
new Promise((resolve, reject) => {
safeLookup('malicious.com', { all: true }, (err, filtered) => {
if (err) reject(err);
else resolve(filtered);
});
}),
).rejects.toThrow(PrivateIpError);
});
});
describe('safeFetch', () => {
it('should forward to fetch with dispatcher', async () => {
vi.mocked(global.fetch).mockResolvedValue(new Response('ok'));
const response = await safeFetch('https://example.com');
expect(response.status).toBe(200);
expect(global.fetch).toHaveBeenCalledWith(
'https://example.com',
expect.objectContaining({
dispatcher: expect.any(Object),
}),
);
});
it('should handle Refusing to connect errors', async () => {
vi.mocked(global.fetch).mockRejectedValue(new PrivateIpError());
await expect(safeFetch('http://10.0.0.1')).rejects.toThrow(
'Access to private network is blocked',
);
});
});
describe('fetchWithTimeout', () => {
it('should handle timeouts', async () => {
vi.mocked(global.fetch).mockImplementation(
(_input, init) =>
new Promise((_resolve, reject) => {
if (init?.signal) {
init.signal.addEventListener('abort', () => {
const error = new Error('The operation was aborted');
error.name = 'AbortError';
// @ts-expect-error - for mocking purposes
error.code = 'ABORT_ERR';
reject(error);
});
}
}),
);
await expect(fetchWithTimeout('http://example.com', 50)).rejects.toThrow(
'Request timed out after 50ms',
);
});
it('should handle private IP errors via handleFetchError', async () => {
vi.mocked(global.fetch).mockRejectedValue(new PrivateIpError());
await expect(fetchWithTimeout('http://10.0.0.1', 1000)).rejects.toThrow(
'Access to private network is blocked: http://10.0.0.1',
);
});
});
});

View File

@@ -6,7 +6,10 @@
import { getErrorMessage, isNodeError } from './errors.js';
import { URL } from 'node:url';
import * as dns from 'node:dns';
import { lookup } from 'node:dns/promises';
import { Agent, ProxyAgent, setGlobalDispatcher } from 'undici';
import ipaddr from 'ipaddr.js';
const DEFAULT_HEADERS_TIMEOUT = 300000; // 5 minutes
const DEFAULT_BODY_TIMEOUT = 300000; // 5 minutes
@@ -19,15 +22,20 @@ setGlobalDispatcher(
}),
);
const PRIVATE_IP_RANGES = [
/^10\./,
/^127\./,
/^172\.(1[6-9]|2[0-9]|3[0-1])\./,
/^192\.168\./,
/^::1$/,
/^fc00:/,
/^fe80:/,
];
// Local extension of RequestInit to support Node.js/undici dispatcher
interface NodeFetchInit extends RequestInit {
dispatcher?: Agent | ProxyAgent;
}
/**
* Error thrown when a connection to a private IP address is blocked for security reasons.
*/
export class PrivateIpError extends Error {
constructor(message = 'Refusing to connect to private IP address') {
super(message);
this.name = 'PrivateIpError';
}
}
export class FetchError extends Error {
constructor(
@@ -40,15 +48,234 @@ export class FetchError extends Error {
}
}
/**
* Sanitizes a hostname by stripping IPv6 brackets if present.
*/
export function sanitizeHostname(hostname: string): string {
return hostname.startsWith('[') && hostname.endsWith(']')
? hostname.slice(1, -1)
: hostname;
}
/**
* Checks if a hostname is a local loopback address allowed for development/testing.
*/
export function isLoopbackHost(hostname: string): boolean {
const sanitized = sanitizeHostname(hostname);
return (
sanitized === 'localhost' ||
sanitized === '127.0.0.1' ||
sanitized === '::1'
);
}
/**
* A custom DNS lookup implementation for undici agents that prevents
* connection to private IP ranges (SSRF protection).
*/
export function safeLookup(
hostname: string,
options: dns.LookupOptions | number | null | undefined,
callback: (
err: Error | null,
addresses: Array<{ address: string; family: number }>,
) => void,
): void {
// Use the callback-based dns.lookup to match undici's expected signature.
// We explicitly handle the 'all' option to ensure we get an array of addresses.
const lookupOptions =
typeof options === 'number' ? { family: options } : { ...options };
const finalOptions = { ...lookupOptions, all: true };
dns.lookup(hostname, finalOptions, (err, addresses) => {
if (err) {
callback(err, []);
return;
}
const addressArray = Array.isArray(addresses) ? addresses : [];
const filtered = addressArray.filter(
(addr) => !isAddressPrivate(addr.address) || isLoopbackHost(hostname),
);
if (filtered.length === 0 && addressArray.length > 0) {
callback(new PrivateIpError(), []);
return;
}
callback(null, filtered);
});
}
// Dedicated dispatcher with connection-level SSRF protection (safeLookup)
const safeDispatcher = new Agent({
headersTimeout: DEFAULT_HEADERS_TIMEOUT,
bodyTimeout: DEFAULT_BODY_TIMEOUT,
connect: {
lookup: safeLookup,
},
});
export function isPrivateIp(url: string): boolean {
try {
const hostname = new URL(url).hostname;
return PRIVATE_IP_RANGES.some((range) => range.test(hostname));
} catch (_e) {
return isAddressPrivate(hostname);
} catch {
return false;
}
}
/**
* Checks if a URL resolves to a private IP address.
* Performs DNS resolution to prevent DNS rebinding/SSRF bypasses.
*/
export async function isPrivateIpAsync(url: string): Promise<boolean> {
try {
const parsed = new URL(url);
const hostname = parsed.hostname;
// Fast check for literal IPs or localhost
if (isAddressPrivate(hostname)) {
return true;
}
// Resolve DNS to check the actual target IP
const addresses = await lookup(hostname, { all: true });
return addresses.some((addr) => isAddressPrivate(addr.address));
} catch (e) {
if (
e instanceof Error &&
e.name === 'TypeError' &&
e.message.includes('Invalid URL')
) {
return false;
}
throw new Error(`Failed to verify if URL resolves to private IP: ${url}`, {
cause: e,
});
}
}
/**
* IANA Benchmark Testing Range (198.18.0.0/15).
* Classified as 'unicast' by ipaddr.js but is reserved and should not be
* accessible as public internet.
*/
const IANA_BENCHMARK_RANGE = ipaddr.parseCIDR('198.18.0.0/15');
/**
* Checks if an address falls within the IANA benchmark testing range.
*/
function isBenchmarkAddress(addr: ipaddr.IPv4 | ipaddr.IPv6): boolean {
const [rangeAddr, rangeMask] = IANA_BENCHMARK_RANGE;
return (
addr instanceof ipaddr.IPv4 &&
rangeAddr instanceof ipaddr.IPv4 &&
addr.match(rangeAddr, rangeMask)
);
}
/**
* Internal helper to check if an IP address string is in a private or reserved range.
*/
export function isAddressPrivate(address: string): boolean {
const sanitized = sanitizeHostname(address);
if (sanitized === 'localhost') {
return true;
}
try {
if (!ipaddr.isValid(sanitized)) {
return false;
}
const addr = ipaddr.parse(sanitized);
// Special handling for IPv4-mapped IPv6 (::ffff:x.x.x.x)
// We unmap it and check the underlying IPv4 address.
if (addr instanceof ipaddr.IPv6 && addr.isIPv4MappedAddress()) {
return isAddressPrivate(addr.toIPv4Address().toString());
}
// Explicitly block IANA benchmark testing range.
if (isBenchmarkAddress(addr)) {
return true;
}
return addr.range() !== 'unicast';
} catch {
// If parsing fails despite isValid(), we treat it as potentially unsafe.
return true;
}
}
/**
* Internal helper to map varied fetch errors to a standardized FetchError.
* Centralizes security-related error mapping (e.g. PrivateIpError).
*/
function handleFetchError(error: unknown, url: string): never {
if (error instanceof PrivateIpError) {
throw new FetchError(
`Access to private network is blocked: ${url}`,
'ERR_PRIVATE_NETWORK',
{ cause: error },
);
}
if (error instanceof FetchError) {
throw error;
}
throw new FetchError(
getErrorMessage(error),
isNodeError(error) ? error.code : undefined,
{ cause: error },
);
}
/**
* Enhanced fetch with SSRF protection.
* Prevents access to private/internal networks at the connection level.
*/
export async function safeFetch(
input: RequestInfo | URL,
init?: RequestInit,
): Promise<Response> {
const nodeInit: NodeFetchInit = {
...init,
dispatcher: safeDispatcher,
};
try {
// eslint-disable-next-line no-restricted-syntax
return await fetch(input, nodeInit);
} catch (error) {
const url =
input instanceof Request
? input.url
: typeof input === 'string'
? input
: input.toString();
handleFetchError(error, url);
}
}
/**
* Creates an undici ProxyAgent that incorporates safe DNS lookup.
*/
export function createSafeProxyAgent(proxyUrl: string): ProxyAgent {
return new ProxyAgent({
uri: proxyUrl,
connect: {
lookup: safeLookup,
},
});
}
/**
* Performs a fetch with a specified timeout and connection-level SSRF protection.
*/
export async function fetchWithTimeout(
url: string,
timeout: number,
@@ -67,17 +294,21 @@ export async function fetchWithTimeout(
}
}
const nodeInit: NodeFetchInit = {
...options,
signal: controller.signal,
dispatcher: safeDispatcher,
};
try {
const response = await fetch(url, {
...options,
signal: controller.signal,
});
// eslint-disable-next-line no-restricted-syntax
const response = await fetch(url, nodeInit);
return response;
} catch (error) {
if (isNodeError(error) && error.code === 'ABORT_ERR') {
throw new FetchError(`Request timed out after ${timeout}ms`, 'ETIMEDOUT');
}
throw new FetchError(getErrorMessage(error), undefined, { cause: error });
handleFetchError(error, url.toString());
} finally {
clearTimeout(timeoutId);
}

View File

@@ -454,6 +454,7 @@ export async function exchangeCodeForToken(
params.append('resource', resource);
}
// eslint-disable-next-line no-restricted-syntax -- TODO: Migrate to safeFetch for SSRF protection
const response = await fetch(config.tokenUrl, {
method: 'POST',
headers: {
@@ -507,6 +508,7 @@ export async function refreshAccessToken(
params.append('resource', resource);
}
// eslint-disable-next-line no-restricted-syntax -- TODO: Migrate to safeFetch for SSRF protection
const response = await fetch(tokenUrl, {
method: 'POST',
headers: {