MediaMetz/gemini-cli
1
0
Fork 0
mirror of https://github.com/google-gemini/gemini-cli.git synced 2026-04-22 11:04:42 -07:00
Code Issues Packages Projects Releases Wiki Activity

Address comments and remove not needed security changes.

Browse Source
This commit is contained in:
Alisa Novikova
2026-03-11 06:22:04 -07:00
parent 670bdf2674
commit 75e6d0cf6c
18 changed files with 22 additions and 804 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/core/src/agents/a2a-client-manager.test.ts
-95
View File
@@ -8,8 +8,6 @@ import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest';
import { A2AClientManager } from './a2a-client-manager.js';
import type { AgentCard } from '@a2a-js/sdk';
import * as sdkClient from '@a2a-js/sdk/client';
import * as dnsPromises from 'node:dns/promises';
import type { LookupOptions } from 'node:dns';
import { debugLogger } from '../utils/debugLogger.js';
interface MockClient {
@@ -38,10 +36,6 @@ vi.mock('../utils/debugLogger.js', () => ({
},
}));
vi.mock('node:dns/promises', () => ({
lookup: vi.fn(),
}));
describe('A2AClientManager', () => {
let manager: A2AClientManager;
const mockAgentCard: AgentCard = {
@@ -69,17 +63,6 @@ describe('A2AClientManager', () => {
manager = A2AClientManager.getInstance();
manager.clearCache();
// Default DNS mock: resolve to public IP.
// Use any cast only for return value due to complex multi-signature overloads.
vi.mocked(dnsPromises.lookup).mockImplementation(
async (_h: string, options?: LookupOptions | number) => {
const addr = { address: '93.184.216.34', family: 4 };
const isAll = typeof options === 'object' && options?.all;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
return (isAll ? [addr] : addr) as any;
},
);
// Re-create the instances as plain objects that can be spied on
const factoryInstance = {
createFromUrl: vi.fn(),
@@ -460,82 +443,4 @@ describe('A2AClientManager', () => {
).rejects.toThrow("Agent 'NonExistentAgent' not found.");
});
});
describe('Protocol Routing & URL Logic', () => {
it('should correctly split URLs to prevent .well-known doubling', async () => {
const fullUrl = 'http://localhost:9001/.well-known/agent-card.json';
const resolverInstance = {
resolve: vi.fn().mockResolvedValue({ name: 'test' } as AgentCard),
};
vi.mocked(sdkClient.DefaultAgentCardResolver).mockReturnValue(
resolverInstance as unknown as sdkClient.DefaultAgentCardResolver,
);
await manager.loadAgent('test-doubling', fullUrl);
expect(resolverInstance.resolve).toHaveBeenCalledWith(
'http://localhost:9001/',
undefined,
);
});
it('should throw if a remote agent uses a private IP (SSRF protection)', async () => {
const privateUrl = 'http://169.254.169.254/.well-known/agent-card.json';
await expect(manager.loadAgent('ssrf-agent', privateUrl)).rejects.toThrow(
/Refusing to load agent 'ssrf-agent' from private IP range/,
);
});
it('should throw if a domain resolves to a private IP (DNS SSRF protection)', async () => {
const maliciousDomainUrl =
'http://malicious.com/.well-known/agent-card.json';
vi.mocked(dnsPromises.lookup).mockImplementationOnce(
async (_h: string, options?: LookupOptions | number) => {
const addr = { address: '10.0.0.1', family: 4 };
const isAll = typeof options === 'object' && options?.all;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
return (isAll ? [addr] : addr) as any;
},
);
await expect(
manager.loadAgent('dns-ssrf-agent', maliciousDomainUrl),
).rejects.toThrow(/private IP range/);
});
it('should throw if a public agent card contains a private transport URL (Deep SSRF protection)', async () => {
const publicUrl = 'https://public.agent.com/card.json';
const resolverInstance = {
resolve: vi.fn().mockResolvedValue({
...mockAgentCard,
url: 'http://192.168.1.1/api', // Malicious private transport in public card
} as AgentCard),
};
vi.mocked(sdkClient.DefaultAgentCardResolver).mockReturnValue(
resolverInstance as unknown as sdkClient.DefaultAgentCardResolver,
);
// DNS for public.agent.com is public
vi.mocked(dnsPromises.lookup).mockImplementation(
async (hostname: string, options?: LookupOptions | number) => {
const isAll = typeof options === 'object' && options?.all;
if (hostname === 'public.agent.com') {
const addr = { address: '1.1.1.1', family: 4 };
// eslint-disable-next-line @typescript-eslint/no-explicit-any
return (isAll ? [addr] : addr) as any;
}
const addr = { address: '192.168.1.1', family: 4 };
// eslint-disable-next-line @typescript-eslint/no-explicit-any
return (isAll ? [addr] : addr) as any;
},
);
await expect(
manager.loadAgent('malicious-agent', publicUrl),
).rejects.toThrow(
/contains transport URL pointing to private IP range: http:\/\/192.168.1.1\/api/,
);
});
});
});
packages/core/src/agents/a2a-client-manager.ts
+8 -112
View File
@@ -24,19 +24,7 @@ import {
import { GrpcTransportFactory } from '@a2a-js/sdk/client/grpc';
import { v4 as uuidv4 } from 'uuid';
import { Agent as UndiciAgent } from 'undici';
import {
getGrpcChannelOptions,
getGrpcCredentials,
normalizeAgentCard,
pinUrlToIp,
splitAgentCardUrl,
} from './a2aUtils.js';
import {
isPrivateIpAsync,
safeLookup,
isLoopbackHost,
safeFetch,
} from '../utils/fetch.js';
import { getGrpcCredentials, normalizeAgentCard } from './a2aUtils.js';
import { debugLogger } from '../utils/debugLogger.js';
import { classifyAgentError } from './a2a-errors.js';
@@ -50,29 +38,16 @@ export type SendMessageResult =
| TaskStatusUpdateEvent
| TaskArtifactUpdateEvent;
/**
* Internal interface representing properties we inject into the SDK
* to enable DNS rebinding protection for gRPC connections.
* TODO: Replace with official SDK pinning API once available.
*/
interface InternalGrpcExtensions {
target: string;
grpcChannelOptions: Record<string, unknown>;
}
// Remote agents can take 10+ minutes (e.g. Deep Research).
// Use a dedicated dispatcher so the global 5-min timeout isn't affected.
const A2A_TIMEOUT = 1800000; // 30 minutes
const a2aDispatcher = new UndiciAgent({
headersTimeout: A2A_TIMEOUT,
bodyTimeout: A2A_TIMEOUT,
connect: {
// SSRF protection at the connection level (mitigates DNS rebinding)
lookup: safeLookup,
},
});
const a2aFetch: typeof fetch = (input, init) =>
safeFetch(input, { ...init, dispatcher: a2aDispatcher });
// @ts-expect-error The `dispatcher` property is a Node.js extension to fetch not present in standard types.
fetch(input, { ...init, dispatcher: a2aDispatcher });
/**
* Orchestrates communication with remote A2A agents.
@@ -139,30 +114,6 @@ export class A2AClientManager {
const resolver = new DefaultAgentCardResolver({ fetchImpl: cardFetch });
const agentCard = await this.resolveAgentCard(name, agentCardUrl, resolver);
// Pin URL to IP to prevent DNS rebinding for gRPC (connection-level SSRF protection)
const grpcInterface = agentCard.additionalInterfaces?.find(
(i) => i.transport === 'GRPC',
);
const urlToPin = grpcInterface?.url ?? agentCard.url;
const { pinnedUrl, hostname } = await pinUrlToIp(urlToPin, name);
// Prepare base gRPC options
const baseGrpcOptions: ConstructorParameters<
typeof GrpcTransportFactory
>[0] = {
grpcChannelCredentials: getGrpcCredentials(urlToPin),
};
// We inject additional properties into the transport options to force
// the use of a pinned IP address and matching SSL authority. This is
// required for robust DNS Rebinding protection.
const transportOptions = {
...baseGrpcOptions,
target: pinnedUrl,
grpcChannelOptions: getGrpcChannelOptions(hostname),
} as ConstructorParameters<typeof GrpcTransportFactory>[0] &
InternalGrpcExtensions;
// Configure standard SDK client for tool registration and discovery
const clientOptions = ClientFactoryOptions.createFrom(
ClientFactoryOptions.default,
@@ -170,11 +121,9 @@ export class A2AClientManager {
transports: [
new RestTransportFactory({ fetchImpl: authFetch }),
new JsonRpcTransportFactory({ fetchImpl: authFetch }),
new GrpcTransportFactory(
transportOptions as ConstructorParameters<
typeof GrpcTransportFactory
>[0],
),
new GrpcTransportFactory({
grpcChannelCredentials: getGrpcCredentials(agentCard.url),
}),
],
cardResolver: resolver,
},
@@ -182,8 +131,7 @@ export class A2AClientManager {
try {
const factory = new ClientFactory(clientOptions);
const client = await factory.createFromUrl(agentCardUrl, '');
const agentCard = await client.getAgentCard();
const client = await factory.createFromAgentCard(agentCard);
this.clients.set(name, client);
this.agentCards.set(name, agentCard);
@@ -307,65 +255,13 @@ export class A2AClientManager {
}
}
/**
* Resolves and normalizes an agent card from a given URL.
* Handles splitting the URL if it already contains the standard .well-known path.
* Also performs basic SSRF validation to prevent internal IP access.
*/
private async resolveAgentCard(
agentName: string,
url: string,
resolver: DefaultAgentCardResolver,
): Promise<AgentCard> {
// Validate URL to prevent SSRF (with DNS resolution)
if (await isPrivateIpAsync(url)) {
// Local/private IPs are allowed ONLY for localhost for testing.
const parsed = new URL(url);
if (!isLoopbackHost(parsed.hostname)) {
throw new Error(
`Refusing to load agent '${agentName}' from private IP range: ${url}. Remote agents must use public URLs.`,
);
}
}
const { baseUrl, path } = splitAgentCardUrl(url);
const rawCard = await resolver.resolve(baseUrl, path);
const rawCard = await resolver.resolve(url);
const agentCard = normalizeAgentCard(rawCard);
// Deep validation of all transport URLs within the card to prevent SSRF
await this.validateAgentCardUrls(agentName, agentCard);
return agentCard;
}
/**
* Validates all URLs (top-level and interfaces) within an AgentCard for SSRF.
*/
private async validateAgentCardUrls(
agentName: string,
card: AgentCard,
): Promise<void> {
const urlsToValidate = [card.url];
if (card.additionalInterfaces) {
for (const intf of card.additionalInterfaces) {
if (intf.url) urlsToValidate.push(intf.url);
}
}
for (const url of urlsToValidate) {
if (!url) continue;
// Ensure URL has a scheme for the parser (gRPC often provides raw IP:port)
const validationUrl = url.includes('://') ? url : `http://${url}`;
if (await isPrivateIpAsync(validationUrl)) {
const parsed = new URL(validationUrl);
if (!isLoopbackHost(parsed.hostname)) {
throw new Error(
`Refusing to load agent '${agentName}': contains transport URL pointing to private IP range: ${url}.`,
);
}
}
}
}
}
packages/core/src/agents/a2aUtils.test.ts
-122
View File
@@ -13,8 +13,6 @@ import {
AUTH_REQUIRED_MSG,
normalizeAgentCard,
getGrpcCredentials,
pinUrlToIp,
splitAgentCardUrl,
} from './a2aUtils.js';
import type { SendMessageResult } from './a2a-client-manager.js';
import type {
@@ -26,12 +24,6 @@ import type {
TaskStatusUpdateEvent,
TaskArtifactUpdateEvent,
} from '@a2a-js/sdk';
import * as dnsPromises from 'node:dns/promises';
import type { LookupAddress } from 'node:dns';
vi.mock('node:dns/promises', () => ({
lookup: vi.fn(),
}));
describe('a2aUtils', () => {
beforeEach(() => {
@@ -54,77 +46,6 @@ describe('a2aUtils', () => {
});
});
describe('pinUrlToIp', () => {
it('should resolve and pin hostname to IP', async () => {
vi.mocked(
dnsPromises.lookup as unknown as (
hostname: string,
options: { all: true },
) => Promise<LookupAddress[]>,
).mockResolvedValue([{ address: '93.184.216.34', family: 4 }]);
const { pinnedUrl, hostname } = await pinUrlToIp(
'http://example.com:9000',
'test-agent',
);
expect(hostname).toBe('example.com');
expect(pinnedUrl).toBe('http://93.184.216.34:9000/');
});
it('should handle raw host:port strings (standard for gRPC)', async () => {
vi.mocked(
dnsPromises.lookup as unknown as (
hostname: string,
options: { all: true },
) => Promise<LookupAddress[]>,
).mockResolvedValue([{ address: '93.184.216.34', family: 4 }]);
const { pinnedUrl, hostname } = await pinUrlToIp(
'example.com:9000',
'test-agent',
);
expect(hostname).toBe('example.com');
expect(pinnedUrl).toBe('93.184.216.34:9000');
});
it('should throw error if resolution fails (fail closed)', async () => {
vi.mocked(dnsPromises.lookup).mockRejectedValue(new Error('DNS Error'));
await expect(
pinUrlToIp('http://unreachable.com', 'test-agent'),
).rejects.toThrow("Failed to resolve host for agent 'test-agent'");
});
it('should throw error if resolved to private IP', async () => {
vi.mocked(
dnsPromises.lookup as unknown as (
hostname: string,
options: { all: true },
) => Promise<LookupAddress[]>,
).mockResolvedValue([{ address: '10.0.0.1', family: 4 }]);
await expect(
pinUrlToIp('http://malicious.com', 'test-agent'),
).rejects.toThrow('resolves to private IP range');
});
it('should allow localhost/127.0.0.1/::1 exceptions', async () => {
vi.mocked(
dnsPromises.lookup as unknown as (
hostname: string,
options: { all: true },
) => Promise<LookupAddress[]>,
).mockResolvedValue([{ address: '127.0.0.1', family: 4 }]);
const { pinnedUrl, hostname } = await pinUrlToIp(
'http://localhost:9000',
'test-agent',
);
expect(hostname).toBe('localhost');
expect(pinnedUrl).toBe('http://127.0.0.1:9000/');
});
});
describe('isTerminalState', () => {
it('should return true for completed, failed, canceled, and rejected', () => {
expect(isTerminalState('completed')).toBe(true);
@@ -450,49 +371,6 @@ describe('a2aUtils', () => {
});
});
describe('splitAgentCardUrl', () => {
const standard = '.well-known/agent-card.json';
it('should return baseUrl as-is if it does not end with standard path', () => {
const url = 'http://localhost:9001/custom/path';
expect(splitAgentCardUrl(url)).toEqual({ baseUrl: url });
});
it('should split correctly if URL ends with standard path', () => {
const url = `http://localhost:9001/${standard}`;
expect(splitAgentCardUrl(url)).toEqual({
baseUrl: 'http://localhost:9001/',
path: undefined,
});
});
it('should handle trailing slash in baseUrl when splitting', () => {
const url = `http://example.com/api/${standard}`;
expect(splitAgentCardUrl(url)).toEqual({
baseUrl: 'http://example.com/api/',
path: undefined,
});
});
it('should ignore hashes and query params when splitting', () => {
const url = `http://localhost:9001/${standard}?foo=bar#baz`;
expect(splitAgentCardUrl(url)).toEqual({
baseUrl: 'http://localhost:9001/',
path: undefined,
});
});
it('should return original URL if parsing fails', () => {
const url = 'not-a-url';
expect(splitAgentCardUrl(url)).toEqual({ baseUrl: url });
});
it('should handle standard path appearing earlier in the path', () => {
const url = `http://localhost:9001/${standard}/something-else`;
expect(splitAgentCardUrl(url)).toEqual({ baseUrl: url });
});
});
describe('A2AResultReassembler', () => {
it('should reassemble sequential messages and incremental artifacts', () => {
const reassembler = new A2AResultReassembler();
packages/core/src/agents/a2aUtils.ts
-114
View File
@@ -5,7 +5,6 @@
*/
import * as grpc from '@grpc/grpc-js';
import { lookup } from 'node:dns/promises';
import { z } from 'zod';
import type {
Message,
@@ -18,7 +17,6 @@ import type {
AgentCard,
AgentInterface,
} from '@a2a-js/sdk';
import { isAddressPrivate } from '../utils/fetch.js';
import type { SendMessageResult } from './a2a-client-manager.js';
export const AUTH_REQUIRED_MSG = `[Authorization Required] The agent has indicated it requires authorization to proceed. Please follow the agent's instructions.`;
@@ -289,118 +287,6 @@ export function getGrpcCredentials(url: string): grpc.ChannelCredentials {
: grpc.credentials.createInsecure();
}
/**
* Returns gRPC channel options to ensure SSL/authority matches the original hostname
* when connecting via a pinned IP address.
*/
export function getGrpcChannelOptions(
hostname: string,
): Record<string, unknown> {
return {
'grpc.default_authority': hostname,
'grpc.ssl_target_name_override': hostname,
};
}
/**
* Resolves a hostname to its IP address and validates it against SSRF.
* Returns the pinned IP-based URL and the original hostname.
*/
export async function pinUrlToIp(
url: string,
agentName: string,
): Promise<{ pinnedUrl: string; hostname: string }> {
if (!url) return { pinnedUrl: url, hostname: '' };
// gRPC URLs in A2A can be 'host:port' or 'dns:///host:port' or have schemes.
// We normalize to host:port for resolution.
const hasScheme = url.includes('://');
const normalizedUrl = hasScheme ? url : `http://${url}`;
try {
const parsed = new URL(normalizedUrl);
const hostname = parsed.hostname;
const sanitizedHost =
hostname.startsWith('[') && hostname.endsWith(']')
? hostname.slice(1, -1)
: hostname;
// Resolve DNS to check the actual target IP and pin it
const addresses = await lookup(hostname, { all: true });
const publicAddresses = addresses.filter(
(addr) =>
!isAddressPrivate(addr.address) ||
sanitizedHost === 'localhost' ||
sanitizedHost === '127.0.0.1' ||
sanitizedHost === '::1',
);
if (publicAddresses.length === 0) {
if (addresses.length > 0) {
throw new Error(
`Refusing to load agent '${agentName}': transport URL '${url}' resolves to private IP range.`,
);
}
throw new Error(
`Failed to resolve any public IP addresses for host: ${hostname}`,
);
}
const pinnedIp = publicAddresses[0].address;
const pinnedHostname = pinnedIp.includes(':') ? `[${pinnedIp}]` : pinnedIp;
// Reconstruct URL with IP
parsed.hostname = pinnedHostname;
let pinnedUrl = parsed.toString();
// If original didn't have scheme, remove it (standard for gRPC targets)
if (!hasScheme) {
pinnedUrl = pinnedUrl.replace(/^http:\/\//, '');
// URL.toString() might append a trailing slash
if (pinnedUrl.endsWith('/') && !url.endsWith('/')) {
pinnedUrl = pinnedUrl.slice(0, -1);
}
}
return { pinnedUrl, hostname };
} catch (e) {
if (e instanceof Error && e.message.includes('Refusing')) throw e;
throw new Error(`Failed to resolve host for agent '${agentName}': ${url}`, {
cause: e,
});
}
}
/**
* Splts an agent card URL into a baseUrl and a standard path if it already
* contains '.well-known/agent-card.json'.
*/
export function splitAgentCardUrl(url: string): {
baseUrl: string;
path?: string;
} {
const standardPath = '.well-known/agent-card.json';
try {
const parsedUrl = new URL(url);
if (parsedUrl.pathname.endsWith(standardPath)) {
// Reconstruct baseUrl from parsed components to avoid issues with hashes or query params.
parsedUrl.pathname = parsedUrl.pathname.substring(
0,
parsedUrl.pathname.lastIndexOf(standardPath),
);
parsedUrl.search = '';
parsedUrl.hash = '';
// We return undefined for path if it's the standard one,
// because the SDK's DefaultAgentCardResolver appends it automatically.
return { baseUrl: parsedUrl.toString(), path: undefined };
}
} catch (_e) {
// Ignore URL parsing errors here, let the resolver handle them.
}
return { baseUrl: url };
}
/**
* Extracts contextId and taskId from a Message, Task, or Update response.
* Follows the pattern from the A2A CLI sample to maintain conversational continuity.