Merge branch 'main' into restart-resume

This commit is contained in:
Jack Wotherspoon
2026-03-10 09:21:29 +01:00
committed by GitHub
208 changed files with 4823 additions and 1292 deletions
+4 -1
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",
@@ -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
@@ -19,22 +19,24 @@ vi.mock('../scheduler/scheduler.js', () => ({
}));
describe('agent-scheduler', () => {
let mockConfig: Mocked<Config>;
let mockToolRegistry: Mocked<ToolRegistry>;
let mockMessageBus: Mocked<MessageBus>;
beforeEach(() => {
vi.mocked(Scheduler).mockClear();
mockMessageBus = {} as Mocked<MessageBus>;
mockToolRegistry = {
getTool: vi.fn(),
} as unknown as Mocked<ToolRegistry>;
mockConfig = {
getMessageBus: vi.fn().mockReturnValue(mockMessageBus),
toolRegistry: mockToolRegistry,
} as unknown as Mocked<Config>;
} as unknown as Mocked<ToolRegistry>;
});
it('should create a scheduler with agent-specific config', async () => {
const mockConfig = {
getMessageBus: vi.fn().mockReturnValue(mockMessageBus),
toolRegistry: mockToolRegistry,
} as unknown as Mocked<Config>;
const requests: ToolCallRequestInfo[] = [
{
callId: 'call-1',
@@ -67,8 +69,46 @@ describe('agent-scheduler', () => {
}),
);
// Verify that the scheduler's config has the overridden tool registry
const schedulerConfig = vi.mocked(Scheduler).mock.calls[0][0].config;
expect(schedulerConfig.toolRegistry).toBe(mockToolRegistry);
});
it('should override toolRegistry getter from prototype chain', async () => {
const mainRegistry = { _id: 'main' } as unknown as Mocked<ToolRegistry>;
const agentRegistry = {
_id: 'agent',
getMessageBus: vi.fn().mockReturnValue(mockMessageBus),
} as unknown as Mocked<ToolRegistry>;
const config = {
getMessageBus: vi.fn().mockReturnValue(mockMessageBus),
} as unknown as Mocked<Config>;
Object.defineProperty(config, 'toolRegistry', {
get: () => mainRegistry,
configurable: true,
});
await scheduleAgentTools(
config as unknown as Config,
[
{
callId: 'c1',
name: 'new_page',
args: {},
isClientInitiated: false,
prompt_id: 'p1',
},
],
{
schedulerId: 'browser-1',
toolRegistry: agentRegistry as unknown as ToolRegistry,
signal: new AbortController().signal,
},
);
const schedulerConfig = vi.mocked(Scheduler).mock.calls[0][0].config;
expect(schedulerConfig.toolRegistry).toBe(agentRegistry);
expect(schedulerConfig.toolRegistry).not.toBe(mainRegistry);
expect(schedulerConfig.getToolRegistry()).toBe(agentRegistry);
});
});
+7 -1
View File
@@ -57,10 +57,16 @@ export async function scheduleAgentTools(
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
const agentConfig: Config = Object.create(config);
agentConfig.getToolRegistry = () => toolRegistry;
agentConfig.getMessageBus = () => toolRegistry.getMessageBus();
// Override toolRegistry property so AgentLoopContext reads the agent-specific registry.
Object.defineProperty(agentConfig, 'toolRegistry', {
get: () => toolRegistry,
configurable: true,
});
const scheduler = new Scheduler({
config: agentConfig,
messageBus: config.getMessageBus(),
messageBus: toolRegistry.getMessageBus(),
getPreferredEditor: getPreferredEditor ?? (() => undefined),
schedulerId,
parentCallId,
@@ -209,6 +209,45 @@ describe('browserAgentFactory', () => {
.map((t) => t.name) ?? [];
expect(toolNames).toContain('analyze_screenshot');
});
it('should include all MCP navigation tools (new_page, navigate_page) in definition', async () => {
mockBrowserManager.getDiscoveredTools.mockResolvedValue([
{ name: 'take_snapshot', description: 'Take snapshot' },
{ name: 'click', description: 'Click element' },
{ name: 'fill', description: 'Fill form field' },
{ name: 'navigate_page', description: 'Navigate to URL' },
{ name: 'new_page', description: 'Open a new page/tab' },
{ name: 'close_page', description: 'Close page' },
{ name: 'select_page', description: 'Select page' },
{ name: 'press_key', description: 'Press key' },
{ name: 'hover', description: 'Hover element' },
]);
const { definition } = await createBrowserAgentDefinition(
mockConfig,
mockMessageBus,
);
const toolNames =
definition.toolConfig?.tools
?.filter(
(t): t is { name: string } => typeof t === 'object' && 'name' in t,
)
.map((t) => t.name) ?? [];
// All MCP tools must be present
expect(toolNames).toContain('new_page');
expect(toolNames).toContain('navigate_page');
expect(toolNames).toContain('close_page');
expect(toolNames).toContain('select_page');
expect(toolNames).toContain('click');
expect(toolNames).toContain('take_snapshot');
expect(toolNames).toContain('press_key');
// Custom composite tool must also be present
expect(toolNames).toContain('type_text');
// Total: 9 MCP + 1 type_text (no analyze_screenshot without visualModel)
expect(definition.toolConfig?.tools).toHaveLength(10);
});
});
describe('cleanupBrowserAgent', () => {
@@ -17,6 +17,7 @@
import { randomUUID } from 'node:crypto';
import type { Config } from '../../config/config.js';
import { LocalAgentExecutor } from '../local-executor.js';
import { safeJsonToMarkdown } from '../../utils/markdownUtils.js';
import {
BaseToolInvocation,
type ToolResult,
@@ -414,6 +415,8 @@ export class BrowserAgentInvocation extends BaseToolInvocation<
const output = await executor.run(this.params, signal);
const displayResult = safeJsonToMarkdown(output.result);
const resultContent = `Browser agent finished.
Termination Reason: ${output.terminate_reason}
Result:
@@ -425,7 +428,7 @@ Browser Agent Finished
Termination Reason: ${output.terminate_reason}
Result:
${output.result}
${displayResult}
`;
if (updateOutput) {
+19 -1
View File
@@ -19,6 +19,7 @@ import { ToolRegistry } from '../tools/tool-registry.js';
import { DiscoveredMCPTool } from '../tools/mcp-tool.js';
import { CompressionStatus } from '../core/turn.js';
import { type ToolCallRequestInfo } from '../scheduler/types.js';
import { type Message } from '../confirmation-bus/types.js';
import { ChatCompressionService } from '../services/chatCompressionService.js';
import { getDirectoryContextString } from '../utils/environmentContext.js';
import { promptIdContext } from '../utils/promptIdContext.js';
@@ -113,10 +114,27 @@ export class LocalAgentExecutor<TOutput extends z.ZodTypeAny> {
runtimeContext: Config,
onActivity?: ActivityCallback,
): Promise<LocalAgentExecutor<TOutput>> {
const parentMessageBus = runtimeContext.getMessageBus();
// Create an override object to inject the subagent name into tool confirmation requests
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
const subagentMessageBus = Object.create(
parentMessageBus,
) as typeof parentMessageBus;
subagentMessageBus.publish = async (message: Message) => {
if (message.type === 'tool-confirmation-request') {
return parentMessageBus.publish({
...message,
subagent: definition.name,
});
}
return parentMessageBus.publish(message);
};
// Create an isolated tool registry for this agent instance.
const agentToolRegistry = new ToolRegistry(
runtimeContext,
runtimeContext.getMessageBus(),
subagentMessageBus,
);
const parentToolRegistry = runtimeContext.getToolRegistry();
const allAgentNames = new Set(
+4 -1
View File
@@ -6,6 +6,7 @@
import type { Config } from '../config/config.js';
import { LocalAgentExecutor } from './local-executor.js';
import { safeJsonToMarkdown } from '../utils/markdownUtils.js';
import {
BaseToolInvocation,
type ToolResult,
@@ -245,6 +246,8 @@ export class LocalSubagentInvocation extends BaseToolInvocation<
throw cancelError;
}
const displayResult = safeJsonToMarkdown(output.result);
const resultContent = `Subagent '${this.definition.name}' finished.
Termination Reason: ${output.terminate_reason}
Result:
@@ -256,7 +259,7 @@ Subagent ${this.definition.name} Finished
Termination Reason:\n ${output.terminate_reason}
Result:
${output.result}
${displayResult}
`;
return {
@@ -25,6 +25,7 @@ import { extractIdsFromResponse, A2AResultReassembler } from './a2aUtils.js';
import { GoogleAuth } from 'google-auth-library';
import type { AuthenticationHandler } from '@a2a-js/sdk/client';
import { debugLogger } from '../utils/debugLogger.js';
import { safeJsonToMarkdown } from '../utils/markdownUtils.js';
import type { AnsiOutput } from '../utils/terminalSerializer.js';
import { A2AAuthProviderFactory } from './auth-provider/factory.js';
@@ -222,7 +223,7 @@ export class RemoteAgentInvocation extends BaseToolInvocation<
return {
llmContent: [{ text: finalOutput }],
returnDisplay: finalOutput,
returnDisplay: safeJsonToMarkdown(finalOutput),
};
} catch (error: unknown) {
const partialOutput = reassembler.toString();
+1
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',
{
+1 -1
View File
@@ -31,7 +31,7 @@ export interface MessageActionReturn {
export interface LoadHistoryActionReturn<HistoryType = unknown> {
type: 'load_history';
history: HistoryType;
clientHistory: Content[]; // The history for the generative client
clientHistory: readonly Content[]; // The history for the generative client
}
/**
+4
View File
@@ -361,6 +361,10 @@ export interface GeminiCLIExtension {
*/
directory?: string;
};
/**
* Used to migrate an extension to a new repository source.
*/
migratedTo?: string;
}
export interface ExtensionInstallMetadata {
@@ -160,6 +160,7 @@ describe('MessageBus', () => {
{ name: 'test-tool', args: {} },
'test-server',
annotations,
undefined,
);
});
@@ -56,6 +56,7 @@ export class MessageBus extends EventEmitter {
message.toolCall,
message.serverName,
message.toolAnnotations,
message.subagent,
);
switch (decision) {
@@ -38,6 +38,10 @@ export interface ToolConfirmationRequest {
* Optional tool annotations (e.g., readOnlyHint, destructiveHint) from MCP.
*/
toolAnnotations?: Record<string, unknown>;
/**
* Optional subagent name, if this tool call was initiated by a subagent.
*/
subagent?: string;
/**
* Optional rich details for the confirmation UI (diffs, counts, etc.)
*/
+3 -3
View File
@@ -255,7 +255,7 @@ export class GeminiClient {
return this.chat !== undefined;
}
getHistory(): Content[] {
getHistory(): readonly Content[] {
return this.getChat().getHistory();
}
@@ -263,7 +263,7 @@ export class GeminiClient {
this.getChat().stripThoughtsFromHistory();
}
setHistory(history: Content[]) {
setHistory(history: readonly Content[]) {
this.getChat().setHistory(history);
this.updateTelemetryTokenCount();
this.forceFullIdeContext = true;
@@ -1171,7 +1171,7 @@ export class GeminiClient {
/**
* Masks bulky tool outputs to save context window space.
*/
private async tryMaskToolOutputs(history: Content[]): Promise<void> {
private async tryMaskToolOutputs(history: readonly Content[]): Promise<void> {
if (!this.config.getToolOutputMaskingEnabled()) {
return;
}
+11 -15
View File
@@ -470,7 +470,7 @@ export class GeminiChat {
private async makeApiCallAndProcessStream(
modelConfigKey: ModelConfigKey,
requestContents: Content[],
requestContents: readonly Content[],
prompt_id: string,
abortSignal: AbortSignal,
role: LlmRole,
@@ -489,7 +489,7 @@ export class GeminiChat {
let currentGenerateContentConfig: GenerateContentConfig =
newAvailabilityConfig;
let lastConfig: GenerateContentConfig = currentGenerateContentConfig;
let lastContentsToUse: Content[] = requestContents;
let lastContentsToUse: Content[] = [...requestContents];
const getAvailabilityContext = createAvailabilityContextProvider(
this.config,
@@ -528,9 +528,9 @@ export class GeminiChat {
abortSignal,
};
let contentsToUse = supportsModernFeatures(modelToUse)
? contentsForPreviewModel
: requestContents;
let contentsToUse: Content[] = supportsModernFeatures(modelToUse)
? [...contentsForPreviewModel]
: [...requestContents];
const hookSystem = this.config.getHookSystem();
if (hookSystem) {
@@ -687,16 +687,10 @@ export class GeminiChat {
* @return History contents alternating between user and model for the entire
* chat session.
*/
getHistory(curated: boolean = false): Content[] {
getHistory(curated: boolean = false): readonly Content[] {
const history = curated
? extractCuratedHistory(this.history)
: this.history;
// Return a shallow copy of the array to prevent callers from mutating
// the internal history array (push/pop/splice). Content objects are
// shared references — callers MUST NOT mutate them in place.
// This replaces a prior structuredClone() which deep-copied the entire
// conversation on every call, causing O(n) memory pressure per turn
// that compounded into OOM crashes in long-running sessions.
return [...history];
}
@@ -714,8 +708,8 @@ export class GeminiChat {
this.history.push(content);
}
setHistory(history: Content[]): void {
this.history = history;
setHistory(history: readonly Content[]): void {
this.history = [...history];
this.lastPromptTokenCount = estimateTokenCountSync(
this.history.flatMap((c) => c.parts || []),
);
@@ -742,7 +736,9 @@ export class GeminiChat {
// To ensure our requests validate, the first function call in every model
// turn within the active loop must have a `thoughtSignature` property.
// If we do not do this, we will get back 400 errors from the API.
ensureActiveLoopHasThoughtSignatures(requestContents: Content[]): Content[] {
ensureActiveLoopHasThoughtSignatures(
requestContents: readonly Content[],
): readonly Content[] {
// First, find the start of the active loop by finding the last user turn
// with a text message, i.e. that is not a function response.
let activeLoopStartIndex = -1;
+1 -1
View File
@@ -27,7 +27,7 @@ export interface LogEntry {
}
export interface Checkpoint {
history: Content[];
history: readonly Content[];
authType?: AuthType;
}
+2
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,
+2
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;
+16 -1
View File
@@ -74,6 +74,7 @@ function ruleMatches(
serverName: string | undefined,
currentApprovalMode: ApprovalMode,
toolAnnotations?: Record<string, unknown>,
subagent?: string,
): boolean {
// Check if rule applies to current approval mode
if (rule.modes && rule.modes.length > 0) {
@@ -82,6 +83,13 @@ function ruleMatches(
}
}
// Check subagent if specified (only for PolicyRule, SafetyCheckerRule doesn't have it)
if ('subagent' in rule && rule.subagent) {
if (rule.subagent !== subagent) {
return false;
}
}
// Strictly enforce mcpName identity if the rule dictates it
if (rule.mcpName) {
if (rule.mcpName === '*') {
@@ -203,6 +211,7 @@ export class PolicyEngine {
allowRedirection?: boolean,
rule?: PolicyRule,
toolAnnotations?: Record<string, unknown>,
subagent?: string,
): Promise<CheckResult> {
if (!command) {
return {
@@ -294,6 +303,7 @@ export class PolicyEngine {
{ name: toolName, args: { command: subCmd, dir_path } },
serverName,
toolAnnotations,
subagent,
);
// subResult.decision is already filtered through applyNonInteractiveMode by this.check()
@@ -352,6 +362,7 @@ export class PolicyEngine {
toolCall: FunctionCall,
serverName: string | undefined,
toolAnnotations?: Record<string, unknown>,
subagent?: string,
): Promise<CheckResult> {
// Case 1: Metadata injection is the primary and safest way to identify an MCP server.
// If we have explicit `_serverName` metadata (usually injected by tool-registry for active tools), use it.
@@ -419,6 +430,7 @@ export class PolicyEngine {
serverName,
this.approvalMode,
toolAnnotations,
subagent,
),
);
@@ -437,6 +449,7 @@ export class PolicyEngine {
rule.allowRedirection,
rule,
toolAnnotations,
subagent,
);
decision = shellResult.decision;
if (shellResult.rule) {
@@ -463,9 +476,10 @@ export class PolicyEngine {
this.defaultDecision,
serverName,
shellDirPath,
undefined,
false,
undefined,
toolAnnotations,
subagent,
);
decision = shellResult.decision;
matchedRule = shellResult.rule;
@@ -485,6 +499,7 @@ export class PolicyEngine {
serverName,
this.approvalMode,
toolAnnotations,
subagent,
)
) {
debugLogger.debug(
+2
View File
@@ -38,6 +38,7 @@ const MAX_TYPO_DISTANCE = 3;
*/
const PolicyRuleSchema = z.object({
toolName: z.union([z.string(), z.array(z.string())]).optional(),
subagent: z.string().optional(),
mcpName: z.string().optional(),
argsPattern: z.string().optional(),
commandPrefix: z.union([z.string(), z.array(z.string())]).optional(),
@@ -464,6 +465,7 @@ export async function loadPoliciesFromToml(
const policyRule: PolicyRule = {
toolName: effectiveToolName,
subagent: rule.subagent,
mcpName: rule.mcpName,
decision: rule.decision,
priority: transformPriority(rule.priority, tier),
+6
View File
@@ -110,6 +110,12 @@ export interface PolicyRule {
*/
toolName?: string;
/**
* The name of the subagent this rule applies to.
* If undefined, the rule applies regardless of whether it's the main agent or a subagent.
*/
subagent?: string;
/**
* Identifies the MCP server this rule applies to.
* Enables precise rule matching against `serverName` metadata instead
+1 -1
View File
@@ -31,7 +31,7 @@ export interface RoutingDecision {
*/
export interface RoutingContext {
/** The full history of the conversation. */
history: Content[];
history: readonly Content[];
/** The immediate request parts to be processed. */
request: PartListUnion;
/** An abort signal to cancel an LLM call during routing. */
+3 -1
View File
@@ -74,7 +74,9 @@ export class ContextBuilder {
}
// Helper to convert Google GenAI Content[] to Safety Protocol ConversationTurn[]
private convertHistoryToTurns(history: Content[]): ConversationTurn[] {
private convertHistoryToTurns(
history: readonly Content[],
): ConversationTurn[] {
const turns: ConversationTurn[] = [];
let currentUserRequest: { text: string } | undefined;
@@ -130,7 +130,7 @@ export function modelStringToModelConfigAlias(model: string): string {
* contain massive tool outputs (like large grep results or logs).
*/
async function truncateHistoryToBudget(
history: Content[],
history: readonly Content[],
config: Config,
): Promise<Content[]> {
let functionResponseTokenCounter = 0;
@@ -664,7 +664,7 @@ export class ChatRecordingService {
* Updates the conversation history based on the provided API Content array.
* This is used to persist changes made to the history (like masking) back to disk.
*/
updateMessagesFromHistory(history: Content[]): void {
updateMessagesFromHistory(history: readonly Content[]): void {
if (!this.conversationFile) return;
try {
@@ -13,6 +13,7 @@ import {
KeychainSchema,
KEYCHAIN_TEST_PREFIX,
} from './keychainTypes.js';
import { isRecord } from '../utils/markdownUtils.js';
/**
* Service for interacting with OS-level secure storage (e.g. keytar).
@@ -111,7 +112,7 @@ export class KeychainService {
private async loadKeychainModule(): Promise<Keychain | null> {
const moduleName = 'keytar';
const module: unknown = await import(moduleName);
const potential = (this.isRecord(module) && module['default']) || module;
const potential = (isRecord(module) && module['default']) || module;
const result = KeychainSchema.safeParse(potential);
if (result.success) {
@@ -126,10 +127,6 @@ export class KeychainService {
return null;
}
private isRecord(obj: unknown): obj is Record<string, unknown> {
return typeof obj === 'object' && obj !== null;
}
// Performs a set-get-delete cycle to verify keychain functionality.
private async isKeychainFunctional(keychain: Keychain): Promise<boolean> {
const testAccount = `${KEYCHAIN_TEST_PREFIX}${crypto.randomBytes(8).toString('hex')}`;
@@ -870,6 +870,77 @@ describe('ShellExecutionService', () => {
expect(ShellExecutionService['activePtys'].size).toBe(0);
});
it('should destroy the PTY when kill() is called', async () => {
// Execute a command to populate activePtys
const abortController = new AbortController();
await ShellExecutionService.execute(
'long-running',
'/test/dir',
onOutputEventMock,
abortController.signal,
true,
shellExecutionConfig,
);
await new Promise((resolve) => process.nextTick(resolve));
const pid = mockPtyProcess.pid;
const activePty = ShellExecutionService['activePtys'].get(pid);
expect(activePty).toBeTruthy();
// Spy on the actual stored object's destroy
const storedDestroySpy = vi.spyOn(
activePty!.ptyProcess as never as { destroy: () => void },
'destroy',
);
ShellExecutionService.kill(pid);
expect(storedDestroySpy).toHaveBeenCalled();
expect(ShellExecutionService['activePtys'].has(pid)).toBe(false);
});
it('should destroy the PTY when an exception occurs after spawn in executeWithPty', async () => {
// Simulate: spawn succeeds, Promise executor runs fine (pid accesses 1-2),
// but the return statement `{ pid: ptyProcess.pid }` (access 3) throws.
// The catch block should call spawnedPty.destroy() to release the fd.
const destroySpy = vi.fn();
let pidAccessCount = 0;
const faultyPty = {
onData: vi.fn(),
onExit: vi.fn(),
write: vi.fn(),
kill: vi.fn(),
resize: vi.fn(),
destroy: destroySpy,
get pid() {
pidAccessCount++;
// Accesses 1-2 are inside the Promise executor (setup).
// Access 3 is at `return { pid: ptyProcess.pid, result }`,
// outside the Promise — caught by the outer try/catch.
if (pidAccessCount > 2) {
throw new Error('Simulated post-spawn failure on pid access');
}
return 77777;
},
};
mockPtySpawn.mockReturnValueOnce(faultyPty);
const handle = await ShellExecutionService.execute(
'will-fail-after-spawn',
'/test/dir',
onOutputEventMock,
new AbortController().signal,
true,
shellExecutionConfig,
);
const result = await handle.result;
expect(result.exitCode).toBe(1);
expect(result.error).toBeTruthy();
// The catch block must call destroy() on spawnedPty to prevent fd leak
expect(destroySpy).toHaveBeenCalled();
});
});
});
@@ -552,6 +552,8 @@ export class ShellExecutionService {
// This should not happen, but as a safeguard...
throw new Error('PTY implementation not found');
}
let spawnedPty: IPty | undefined;
try {
const cols = shellExecutionConfig.terminalWidth ?? 80;
const rows = shellExecutionConfig.terminalHeight ?? 30;
@@ -585,6 +587,8 @@ export class ShellExecutionService {
},
handleFlowControl: true,
});
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
spawnedPty = ptyProcess as IPty;
const result = new Promise<ShellExecutionResult>((resolve) => {
this.activeResolvers.set(ptyProcess.pid, resolve);
@@ -882,6 +886,15 @@ export class ShellExecutionService {
} catch (e) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
const error = e as Error;
if (spawnedPty) {
try {
(spawnedPty as IPty & { destroy?: () => void }).destroy?.();
} catch {
// Ignore errors during cleanup
}
}
if (error.message.includes('posix_spawnp failed')) {
onOutputEvent({
type: 'data',
@@ -1008,6 +1021,11 @@ export class ShellExecutionService {
this.activeChildProcesses.delete(pid);
} else if (activePty) {
killProcessGroup({ pid, pty: activePty.ptyProcess }).catch(() => {});
try {
(activePty.ptyProcess as IPty & { destroy?: () => void }).destroy?.();
} catch {
// Ignore errors during cleanup
}
this.activePtys.delete(pid);
}
@@ -43,7 +43,7 @@ const EXEMPT_TOOLS = new Set([
]);
export interface MaskingResult {
newHistory: Content[];
newHistory: readonly Content[];
maskedCount: number;
tokensSaved: number;
}
@@ -67,7 +67,10 @@ export interface MaskingResult {
* are preserved until they collectively reach the threshold.
*/
export class ToolOutputMaskingService {
async mask(history: Content[], config: Config): Promise<MaskingResult> {
async mask(
history: readonly Content[],
config: Config,
): Promise<MaskingResult> {
const maskingConfig = await config.getToolOutputMaskingConfig();
if (!maskingConfig.enabled || history.length === 0) {
return { newHistory: history, maskedCount: 0, tokensSaved: 0 };
@@ -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),
+5
View File
@@ -104,6 +104,10 @@ export enum MCPServerStatus {
CONNECTING = 'connecting',
/** Server is connected and ready to use */
CONNECTED = 'connected',
/** Server is blocked via configuration and cannot be used */
BLOCKED = 'blocked',
/** Server is disabled and cannot be used */
DISABLED = 'disabled',
}
/**
@@ -1899,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: {
@@ -776,7 +776,7 @@ Content of file[1]
// Mock to track concurrent vs sequential execution
detectFileTypeSpy.mockImplementation(async (filePath: string) => {
const fileName = filePath.split('/').pop() || '';
const fileName = path.basename(filePath);
executionOrder.push(`start:${fileName}`);
// Add delay to make timing differences visible
+1 -1
View File
@@ -14,7 +14,7 @@ import type { ToolCallRequestInfo } from '../scheduler/types.js';
export interface ToolCallData<HistoryType = unknown, ArgsType = unknown> {
history?: HistoryType;
clientHistory?: Content[];
clientHistory?: readonly Content[];
commitHash?: string;
toolCall: {
name: string;
+69 -2
View File
@@ -392,7 +392,10 @@ describe('editor utils', () => {
);
});
it(`should reject if ${editor} exits with non-zero code`, async () => {
it(`should resolve and log warning if ${editor} exits with non-zero code`, async () => {
const warnSpy = vi
.spyOn(debugLogger, 'warn')
.mockImplementation(() => {});
const mockSpawnOn = vi.fn((event, cb) => {
if (event === 'close') {
cb(1);
@@ -400,9 +403,73 @@ describe('editor utils', () => {
});
(spawn as Mock).mockReturnValue({ on: mockSpawnOn });
await openDiff('old.txt', 'new.txt', editor);
expect(warnSpy).toHaveBeenCalledWith(`${editor} exited with code 1`);
});
it(`should emit ExternalEditorClosed when ${editor} exits successfully`, async () => {
const emitSpy = vi.spyOn(coreEvents, 'emit');
const mockSpawnOn = vi.fn((event, cb) => {
if (event === 'close') {
cb(0);
}
});
(spawn as Mock).mockReturnValue({ on: mockSpawnOn });
await openDiff('old.txt', 'new.txt', editor);
expect(emitSpy).toHaveBeenCalledWith(CoreEvent.ExternalEditorClosed);
});
it(`should emit ExternalEditorClosed when ${editor} exits with non-zero code`, async () => {
vi.spyOn(debugLogger, 'warn').mockImplementation(() => {});
const emitSpy = vi.spyOn(coreEvents, 'emit');
const mockSpawnOn = vi.fn((event, cb) => {
if (event === 'close') {
cb(1);
}
});
(spawn as Mock).mockReturnValue({ on: mockSpawnOn });
await openDiff('old.txt', 'new.txt', editor);
expect(emitSpy).toHaveBeenCalledWith(CoreEvent.ExternalEditorClosed);
});
it(`should emit ExternalEditorClosed when ${editor} spawn errors`, async () => {
const emitSpy = vi.spyOn(coreEvents, 'emit');
const mockError = new Error('spawn error');
const mockSpawnOn = vi.fn((event, cb) => {
if (event === 'error') {
cb(mockError);
}
});
(spawn as Mock).mockReturnValue({ on: mockSpawnOn });
await expect(openDiff('old.txt', 'new.txt', editor)).rejects.toThrow(
`${editor} exited with code 1`,
'spawn error',
);
expect(emitSpy).toHaveBeenCalledWith(CoreEvent.ExternalEditorClosed);
});
it(`should only emit ExternalEditorClosed once when ${editor} fires both error and close`, async () => {
const emitSpy = vi.spyOn(coreEvents, 'emit');
const callbacks: Record<string, (arg: unknown) => void> = {};
const mockSpawnOn = vi.fn(
(event: string, cb: (arg: unknown) => void) => {
callbacks[event] = cb;
},
);
(spawn as Mock).mockReturnValue({ on: mockSpawnOn });
const promise = openDiff('old.txt', 'new.txt', editor);
// Simulate Node.js behavior: error fires first, then close.
callbacks['error'](new Error('spawn error'));
callbacks['close'](1);
await expect(promise).rejects.toThrow('spawn error');
const editorClosedEmissions = emitSpy.mock.calls.filter(
(call) => call[0] === CoreEvent.ExternalEditorClosed,
);
expect(editorClosedEmissions).toHaveLength(1);
});
}
+19 -4
View File
@@ -323,15 +323,30 @@ export async function openDiff(
shell: process.platform === 'win32',
});
// Guard against both 'error' and 'close' firing for a single failure,
// which would emit ExternalEditorClosed twice and attempt to settle
// the promise twice.
let isSettled = false;
childProcess.on('close', (code) => {
if (code === 0) {
resolve();
} else {
reject(new Error(`${editor} exited with code ${code}`));
if (isSettled) return;
isSettled = true;
if (code !== 0) {
// GUI editors (VS Code, Zed, etc.) can exit with non-zero codes
// under normal circumstances (e.g., window closed while loading).
// Log a warning instead of crashing the CLI process.
debugLogger.warn(`${editor} exited with code ${code}`);
}
coreEvents.emit(CoreEvent.ExternalEditorClosed);
resolve();
});
childProcess.on('error', (error) => {
if (isSettled) return;
isSettled = true;
coreEvents.emit(CoreEvent.ExternalEditorClosed);
reject(error);
});
});
+291
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',
);
});
});
});
+247 -16
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);
}
+1 -2
View File
@@ -8,7 +8,6 @@ import fs from 'node:fs';
import fsPromises from 'node:fs/promises';
import path from 'node:path';
import type { PartUnion } from '@google/genai';
import mime from 'mime/lite';
import type { FileSystemService } from '../services/fileSystemService.js';
import { ToolErrorType } from '../tools/tool-error.js';
@@ -473,7 +472,7 @@ export async function processSingleFileContent(
case 'text': {
// Use BOM-aware reader to avoid leaving a BOM character in content and to support UTF-16/32 transparently
const content = await readFileWithEncoding(filePath);
const lines = content.split('\n');
const lines = content.split(/\r?\n/);
const originalLineCount = lines.length;
let sliceStart = 0;
@@ -0,0 +1,128 @@
/**
* @license
* Copyright 2026 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import { describe, expect, it } from 'vitest';
import { jsonToMarkdown, safeJsonToMarkdown } from './markdownUtils.js';
describe('markdownUtils', () => {
describe('jsonToMarkdown', () => {
it('should handle primitives', () => {
expect(jsonToMarkdown('hello')).toBe('hello');
expect(jsonToMarkdown(123)).toBe('123');
expect(jsonToMarkdown(true)).toBe('true');
expect(jsonToMarkdown(null)).toBe('null');
expect(jsonToMarkdown(undefined)).toBe('undefined');
});
it('should handle simple arrays', () => {
const data = ['a', 'b', 'c'];
expect(jsonToMarkdown(data)).toBe('- a\n- b\n- c');
});
it('should handle simple objects and convert camelCase to Space Case', () => {
const data = { userName: 'Alice', userAge: 30 };
expect(jsonToMarkdown(data)).toBe(
'- **User Name**: Alice\n- **User Age**: 30',
);
});
it('should handle empty structures', () => {
expect(jsonToMarkdown([])).toBe('[]');
expect(jsonToMarkdown({})).toBe('{}');
});
it('should handle nested structures with proper indentation', () => {
const data = {
userInfo: {
fullName: 'Bob Smith',
userRoles: ['admin', 'user'],
},
isActive: true,
};
const result = jsonToMarkdown(data);
expect(result).toBe(
'- **User Info**:\n' +
' - **Full Name**: Bob Smith\n' +
' - **User Roles**:\n' +
' - admin\n' +
' - user\n' +
'- **Is Active**: true',
);
});
it('should render tables for arrays of similar objects with Space Case keys', () => {
const data = [
{ userId: 1, userName: 'Item 1' },
{ userId: 2, userName: 'Item 2' },
];
const result = jsonToMarkdown(data);
expect(result).toBe(
'| User Id | User Name |\n| --- | --- |\n| 1 | Item 1 |\n| 2 | Item 2 |',
);
});
it('should handle pipe characters, backslashes, and newlines in table data', () => {
const data = [
{ colInfo: 'val|ue', otherInfo: 'line\nbreak', pathInfo: 'C:\\test' },
];
const result = jsonToMarkdown(data);
expect(result).toBe(
'| Col Info | Other Info | Path Info |\n| --- | --- | --- |\n| val\\|ue | line break | C:\\\\test |',
);
});
it('should fallback to lists for arrays with mixed objects', () => {
const data = [
{ userId: 1, userName: 'Item 1' },
{ userId: 2, somethingElse: 'Item 2' },
];
const result = jsonToMarkdown(data);
expect(result).toContain('- **User Id**: 1');
expect(result).toContain('- **Something Else**: Item 2');
});
it('should properly indent nested tables', () => {
const data = {
items: [
{ id: 1, name: 'A' },
{ id: 2, name: 'B' },
],
};
const result = jsonToMarkdown(data);
const lines = result.split('\n');
expect(lines[0]).toBe('- **Items**:');
expect(lines[1]).toBe(' | Id | Name |');
expect(lines[2]).toBe(' | --- | --- |');
expect(lines[3]).toBe(' | 1 | A |');
expect(lines[4]).toBe(' | 2 | B |');
});
it('should indent subsequent lines of multiline strings', () => {
const data = {
description: 'Line 1\nLine 2\nLine 3',
};
const result = jsonToMarkdown(data);
expect(result).toBe('- **Description**: Line 1\n Line 2\n Line 3');
});
});
describe('safeJsonToMarkdown', () => {
it('should convert valid JSON', () => {
const json = JSON.stringify({ keyName: 'value' });
expect(safeJsonToMarkdown(json)).toBe('- **Key Name**: value');
});
it('should return original string for invalid JSON', () => {
const notJson = 'Not a JSON string';
expect(safeJsonToMarkdown(notJson)).toBe(notJson);
});
it('should handle plain strings that look like numbers or booleans but are valid JSON', () => {
expect(safeJsonToMarkdown('123')).toBe('123');
expect(safeJsonToMarkdown('true')).toBe('true');
});
});
});
+147
View File
@@ -0,0 +1,147 @@
/**
* @license
* Copyright 2026 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
/**
* Converts a camelCase string to a Space Case string.
* e.g., "camelCaseString" -> "Camel Case String"
*/
function camelToSpace(text: string): string {
const result = text.replace(/([A-Z])/g, ' $1');
return result.charAt(0).toUpperCase() + result.slice(1).trim();
}
/**
* Converts a JSON-compatible value into a readable Markdown representation.
*
* @param data The data to convert.
* @param indent The current indentation level (for internal recursion).
* @returns A Markdown string representing the data.
*/
export function jsonToMarkdown(data: unknown, indent = 0): string {
const spacing = ' '.repeat(indent);
if (data === null) {
return 'null';
}
if (data === undefined) {
return 'undefined';
}
if (Array.isArray(data)) {
if (data.length === 0) {
return '[]';
}
if (isArrayOfSimilarObjects(data)) {
return renderTable(data, indent);
}
return data
.map((item) => {
if (
typeof item === 'object' &&
item !== null &&
Object.keys(item).length > 0
) {
const rendered = jsonToMarkdown(item, indent + 1);
return `${spacing}-\n${rendered}`;
}
const rendered = jsonToMarkdown(item, indent + 1).trimStart();
return `${spacing}- ${rendered}`;
})
.join('\n');
}
if (typeof data === 'object') {
const entries = Object.entries(data);
if (entries.length === 0) {
return '{}';
}
return entries
.map(([key, value]) => {
const displayKey = camelToSpace(key);
if (
typeof value === 'object' &&
value !== null &&
Object.keys(value).length > 0
) {
const renderedValue = jsonToMarkdown(value, indent + 1);
return `${spacing}- **${displayKey}**:\n${renderedValue}`;
}
const renderedValue = jsonToMarkdown(value, indent + 1).trimStart();
return `${spacing}- **${displayKey}**: ${renderedValue}`;
})
.join('\n');
}
if (typeof data === 'string') {
return data
.split('\n')
.map((line, i) => (i === 0 ? line : spacing + line))
.join('\n');
}
return String(data);
}
/**
* Safely attempts to parse a string as JSON and convert it to Markdown.
* If parsing fails, returns the original string.
*
* @param text The text to potentially convert.
* @returns The Markdown representation or the original text.
*/
export function safeJsonToMarkdown(text: string): string {
try {
const parsed: unknown = JSON.parse(text);
return jsonToMarkdown(parsed);
} catch {
return text;
}
}
export function isRecord(value: unknown): value is Record<string, unknown> {
return typeof value === 'object' && value !== null && !Array.isArray(value);
}
function isArrayOfSimilarObjects(
data: unknown[],
): data is Array<Record<string, unknown>> {
if (data.length === 0) {
return false;
}
if (!data.every(isRecord)) return false;
const firstKeys = Object.keys(data[0]).sort().join(',');
return data.every((item) => Object.keys(item).sort().join(',') === firstKeys);
}
function renderTable(data: Array<Record<string, unknown>>, indent = 0): string {
const spacing = ' '.repeat(indent);
const keys = Object.keys(data[0]);
const displayKeys = keys.map(camelToSpace);
const header = `${spacing}| ${displayKeys.join(' | ')} |`;
const separator = `${spacing}| ${keys.map(() => '---').join(' | ')} |`;
const rows = data.map(
(item) =>
`${spacing}| ${keys
.map((key) => {
const val = item[key];
if (typeof val === 'object' && val !== null) {
return JSON.stringify(val)
.replace(/\\/g, '\\\\')
.replace(/\|/g, '\\|');
}
return String(val)
.replace(/\\/g, '\\\\')
.replace(/\|/g, '\\|')
.replace(/\n/g, ' ');
})
.join(' | ')} |`,
);
return [header, separator, ...rows].join('\n');
}
+2
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: {
+26
View File
@@ -484,6 +484,10 @@ describe('shortenPath', () => {
});
describe('resolveToRealPath', () => {
afterEach(() => {
vi.restoreAllMocks();
});
it.each([
{
description:
@@ -542,6 +546,28 @@ describe('resolveToRealPath', () => {
expect(resolveToRealPath(childPath)).toBe(expectedPath);
});
it('should prevent infinite recursion on malicious symlink structures', () => {
const maliciousPath = path.resolve('malicious', 'symlink');
vi.spyOn(fs, 'realpathSync').mockImplementation(() => {
const err = new Error('ENOENT') as NodeJS.ErrnoException;
err.code = 'ENOENT';
throw err;
});
vi.spyOn(fs, 'lstatSync').mockImplementation(
() => ({ isSymbolicLink: () => true }) as fs.Stats,
);
vi.spyOn(fs, 'readlinkSync').mockImplementation(() =>
['..', 'malicious', 'symlink'].join(path.sep),
);
expect(() => resolveToRealPath(maliciousPath)).toThrow(
/Infinite recursion detected/,
);
});
});
describe('normalizePath', () => {
+21 -5
View File
@@ -375,7 +375,12 @@ export function resolveToRealPath(pathStr: string): string {
return robustRealpath(path.resolve(resolvedPath));
}
function robustRealpath(p: string): string {
function robustRealpath(p: string, visited = new Set<string>()): string {
const key = process.platform === 'win32' ? p.toLowerCase() : p;
if (visited.has(key)) {
throw new Error(`Infinite recursion detected in robustRealpath: ${p}`);
}
visited.add(key);
try {
return fs.realpathSync(p);
} catch (e: unknown) {
@@ -385,14 +390,25 @@ function robustRealpath(p: string): string {
if (stat.isSymbolicLink()) {
const target = fs.readlinkSync(p);
const resolvedTarget = path.resolve(path.dirname(p), target);
return robustRealpath(resolvedTarget);
return robustRealpath(resolvedTarget, visited);
}
} catch (lstatError: unknown) {
// Not a symlink, or lstat failed. Re-throw if it's not an expected
// ENOENT (e.g., a permissions error), otherwise resolve parent.
if (
!(
lstatError &&
typeof lstatError === 'object' &&
'code' in lstatError &&
lstatError.code === 'ENOENT'
)
) {
throw lstatError;
}
} catch {
// Not a symlink, or lstat failed. Just resolve parent.
}
const parent = path.dirname(p);
if (parent === p) return p;
return path.join(robustRealpath(parent), path.basename(p));
return path.join(robustRealpath(parent, visited), path.basename(p));
}
throw e;
}