diff --git a/packages/core/src/confirmation-bus/types.ts b/packages/core/src/confirmation-bus/types.ts index aefafe0fa0..65a3b30fb9 100644 --- a/packages/core/src/confirmation-bus/types.ts +++ b/packages/core/src/confirmation-bus/types.ts @@ -121,6 +121,7 @@ export interface UpdatePolicy { argsPattern?: string; commandPrefix?: string | string[]; mcpName?: string; + isSensitive?: boolean; } export interface ToolPolicyRejection { diff --git a/packages/core/src/policy/utils.ts b/packages/core/src/policy/utils.ts index 3742ba3ed6..3485b50727 100644 --- a/packages/core/src/policy/utils.ts +++ b/packages/core/src/policy/utils.ts @@ -11,6 +11,20 @@ export function escapeRegex(text: string): string { return text.replace(/[-[\]{}()*+?.,\\^$|#\s"]/g, '\\$&'); } +/** + * Escapes a string for use in a regular expression that matches a JSON-stringified value. + * + * This is necessary because some characters (like backslashes and quotes) are + * escaped twice in the final JSON string representation used for policy matching. + */ +export function escapeJsonRegex(text: string): string { + // 1. Get the JSON-escaped version of the string (e.g. C:\foo -> C:\\foo) + // 2. Remove the surrounding quotes + const jsonEscaped = JSON.stringify(text).slice(1, -1); + // 3. Regex-escape the result (e.g. C:\\foo -> C:\\\\foo) + return escapeRegex(jsonEscaped); +} + /** * Basic validation for regular expressions to prevent common ReDoS patterns. * This is a heuristic check and not a substitute for a full ReDoS scanner. diff --git a/packages/core/src/tools/activate-skill.ts b/packages/core/src/tools/activate-skill.ts index cf6a33f3e6..c87f796508 100644 --- a/packages/core/src/tools/activate-skill.ts +++ b/packages/core/src/tools/activate-skill.ts @@ -43,7 +43,15 @@ class ActivateSkillToolInvocation extends BaseToolInvocation< _toolName?: string, _toolDisplayName?: string, ) { - super(params, messageBus, _toolName, _toolDisplayName); + super( + params, + messageBus, + _toolName, + _toolDisplayName, + undefined, + undefined, + true, // ActivateSkill is always sensitive + ); } getDescription(): string { @@ -185,13 +193,14 @@ export class ActivateSkillTool extends BaseDeclarativeTool< messageBus: MessageBus, _toolName?: string, _toolDisplayName?: string, + _isSensitive?: boolean, ): ToolInvocation { return new ActivateSkillToolInvocation( this.config, params, messageBus, _toolName, - _toolDisplayName ?? 'Activate Skill', + _toolDisplayName, ); } diff --git a/packages/core/src/tools/edit.ts b/packages/core/src/tools/edit.ts index a7169e99f2..9ed75c55c4 100644 --- a/packages/core/src/tools/edit.ts +++ b/packages/core/src/tools/edit.ts @@ -434,8 +434,17 @@ class EditToolInvocation messageBus: MessageBus, toolName?: string, displayName?: string, + isSensitive?: boolean, ) { - super(params, messageBus, toolName, displayName); + super( + params, + messageBus, + toolName, + displayName, + undefined, + undefined, + isSensitive, + ); } override toolLocations(): ToolLocation[] { @@ -956,6 +965,9 @@ export class EditTool messageBus, true, // isOutputMarkdown false, // canUpdateOutput + undefined, + undefined, + true, ); } @@ -1001,6 +1013,9 @@ export class EditTool protected createInvocation( params: EditToolParams, messageBus: MessageBus, + _toolName?: string, + _toolDisplayName?: string, + isSensitive?: boolean, ): ToolInvocation { return new EditToolInvocation( this.config, @@ -1008,6 +1023,7 @@ export class EditTool messageBus, this.name, this.displayName, + isSensitive, ); } diff --git a/packages/core/src/tools/get-internal-docs.ts b/packages/core/src/tools/get-internal-docs.ts index 23bda8f4dd..2abf2b0ad2 100644 --- a/packages/core/src/tools/get-internal-docs.ts +++ b/packages/core/src/tools/get-internal-docs.ts @@ -79,8 +79,17 @@ class GetInternalDocsInvocation extends BaseToolInvocation< messageBus: MessageBus, _toolName?: string, _toolDisplayName?: string, + isSensitive?: boolean, ) { - super(params, messageBus, _toolName, _toolDisplayName); + super( + params, + messageBus, + _toolName, + _toolDisplayName, + undefined, + undefined, + isSensitive, + ); } override async shouldConfirmExecute( @@ -165,6 +174,9 @@ export class GetInternalDocsTool extends BaseDeclarativeTool< messageBus, /* isOutputMarkdown */ true, /* canUpdateOutput */ false, + undefined, + undefined, + true, ); } @@ -173,12 +185,14 @@ export class GetInternalDocsTool extends BaseDeclarativeTool< messageBus: MessageBus, _toolName?: string, _toolDisplayName?: string, + isSensitive?: boolean, ): ToolInvocation { return new GetInternalDocsInvocation( params, messageBus, _toolName ?? GetInternalDocsTool.Name, _toolDisplayName, + isSensitive, ); } diff --git a/packages/core/src/tools/glob.ts b/packages/core/src/tools/glob.ts index 78b445e762..308b0113fd 100644 --- a/packages/core/src/tools/glob.ts +++ b/packages/core/src/tools/glob.ts @@ -96,8 +96,17 @@ class GlobToolInvocation extends BaseToolInvocation< messageBus: MessageBus, _toolName?: string, _toolDisplayName?: string, + isSensitive?: boolean, ) { - super(params, messageBus, _toolName, _toolDisplayName); + super( + params, + messageBus, + _toolName, + _toolDisplayName, + undefined, + undefined, + isSensitive, + ); } getDescription(): string { @@ -278,6 +287,9 @@ export class GlobTool extends BaseDeclarativeTool { messageBus, true, false, + undefined, + undefined, + true, ); } @@ -328,6 +340,7 @@ export class GlobTool extends BaseDeclarativeTool { messageBus: MessageBus, _toolName?: string, _toolDisplayName?: string, + isSensitive?: boolean, ): ToolInvocation { return new GlobToolInvocation( this.config, @@ -335,6 +348,7 @@ export class GlobTool extends BaseDeclarativeTool { messageBus, _toolName, _toolDisplayName, + isSensitive, ); } diff --git a/packages/core/src/tools/grep.ts b/packages/core/src/tools/grep.ts index 3d74521513..1fdffa25b4 100644 --- a/packages/core/src/tools/grep.ts +++ b/packages/core/src/tools/grep.ts @@ -83,8 +83,17 @@ class GrepToolInvocation extends BaseToolInvocation< messageBus: MessageBus, _toolName?: string, _toolDisplayName?: string, + isSensitive?: boolean, ) { - super(params, messageBus, _toolName, _toolDisplayName); + super( + params, + messageBus, + _toolName, + _toolDisplayName, + undefined, + undefined, + isSensitive, + ); this.fileExclusions = config.getFileExclusions(); } @@ -601,6 +610,9 @@ export class GrepTool extends BaseDeclarativeTool { messageBus, true, false, + undefined, + undefined, + true, ); } @@ -676,6 +688,7 @@ export class GrepTool extends BaseDeclarativeTool { messageBus: MessageBus, _toolName?: string, _toolDisplayName?: string, + isSensitive?: boolean, ): ToolInvocation { return new GrepToolInvocation( this.config, @@ -683,6 +696,7 @@ export class GrepTool extends BaseDeclarativeTool { messageBus, _toolName, _toolDisplayName, + isSensitive, ); } diff --git a/packages/core/src/tools/ls.ts b/packages/core/src/tools/ls.ts index b98dfb9e38..fc46408b84 100644 --- a/packages/core/src/tools/ls.ts +++ b/packages/core/src/tools/ls.ts @@ -78,8 +78,17 @@ class LSToolInvocation extends BaseToolInvocation { messageBus: MessageBus, _toolName?: string, _toolDisplayName?: string, + isSensitive?: boolean, ) { - super(params, messageBus, _toolName, _toolDisplayName); + super( + params, + messageBus, + _toolName, + _toolDisplayName, + undefined, + undefined, + isSensitive, + ); } /** @@ -293,6 +302,9 @@ export class LSTool extends BaseDeclarativeTool { messageBus, true, false, + undefined, + undefined, + true, ); } @@ -316,13 +328,15 @@ export class LSTool extends BaseDeclarativeTool { messageBus: MessageBus, _toolName?: string, _toolDisplayName?: string, + isSensitive?: boolean, ): ToolInvocation { return new LSToolInvocation( this.config, params, - messageBus ?? this.messageBus, + messageBus, _toolName, _toolDisplayName, + isSensitive, ); } diff --git a/packages/core/src/tools/mcp-client.test.ts b/packages/core/src/tools/mcp-client.test.ts index df4c869c34..68e1ba20f3 100644 --- a/packages/core/src/tools/mcp-client.test.ts +++ b/packages/core/src/tools/mcp-client.test.ts @@ -1792,80 +1792,6 @@ describe('mcp-client', () => { expect(callArgs.env!['GEMINI_CLI_EXT_VAR']).toBeUndefined(); }); - it('should include extension settings with defined values in environment', async () => { - const mockedTransport = vi - .spyOn(SdkClientStdioLib, 'StdioClientTransport') - .mockReturnValue({} as SdkClientStdioLib.StdioClientTransport); - - await createTransport( - 'test-server', - { - command: 'test-command', - extension: { - name: 'test-ext', - resolvedSettings: [ - { - envVar: 'GEMINI_CLI_EXT_VAR', - value: 'defined-value', - sensitive: false, - name: 'ext-setting', - }, - ], - version: '', - isActive: false, - path: '', - contextFiles: [], - id: '', - }, - }, - false, - EMPTY_CONFIG, - ); - - const callArgs = mockedTransport.mock.calls[0][0]; - expect(callArgs.env).toBeDefined(); - expect(callArgs.env!['GEMINI_CLI_EXT_VAR']).toBe('defined-value'); - }); - - it('should resolve environment variables in mcpServerConfig.env using extension settings', async () => { - const mockedTransport = vi - .spyOn(SdkClientStdioLib, 'StdioClientTransport') - .mockReturnValue({} as SdkClientStdioLib.StdioClientTransport); - - await createTransport( - 'test-server', - { - command: 'test-command', - env: { - RESOLVED_VAR: '$GEMINI_CLI_EXT_VAR', - }, - extension: { - name: 'test-ext', - resolvedSettings: [ - { - envVar: 'GEMINI_CLI_EXT_VAR', - value: 'ext-value', - sensitive: false, - name: 'ext-setting', - }, - ], - version: '', - isActive: false, - path: '', - contextFiles: [], - id: '', - }, - }, - false, - EMPTY_CONFIG, - ); - - const callArgs = mockedTransport.mock.calls[0][0]; - expect(callArgs.env).toBeDefined(); - expect(callArgs.env!['GEMINI_CLI_EXT_VAR']).toBe('ext-value'); - expect(callArgs.env!['RESOLVED_VAR']).toBe('ext-value'); - }); - it('should expand environment variables in mcpServerConfig.env and not redact them', async () => { const mockedTransport = vi .spyOn(SdkClientStdioLib, 'StdioClientTransport') diff --git a/packages/core/src/tools/mcp-client.ts b/packages/core/src/tools/mcp-client.ts index c393273dbf..f0a9a6be8c 100644 --- a/packages/core/src/tools/mcp-client.ts +++ b/packages/core/src/tools/mcp-client.ts @@ -34,11 +34,7 @@ import { ProgressNotificationSchema, } from '@modelcontextprotocol/sdk/types.js'; import { parse } from 'shell-quote'; -import type { - Config, - MCPServerConfig, - GeminiCLIExtension, -} from '../config/config.js'; +import type { Config, MCPServerConfig } from '../config/config.js'; import { AuthProviderType } from '../config/config.js'; import { GoogleCredentialProvider } from '../mcp/google-auth-provider.js'; import { ServiceAccountImpersonationProvider } from '../mcp/sa-impersonation-provider.js'; @@ -782,25 +778,15 @@ async function handleAutomaticOAuth( * * @param mcpServerConfig The MCP server configuration * @param headers Additional headers - * @param sanitizationConfig Configuration for environment sanitization */ function createTransportRequestInit( mcpServerConfig: MCPServerConfig, headers: Record, - sanitizationConfig: EnvironmentSanitizationConfig, ): RequestInit { - const extensionEnv = getExtensionEnvironment(mcpServerConfig.extension); - const expansionEnv = { ...process.env, ...extensionEnv }; - - const sanitizedEnv = sanitizeEnvironment(expansionEnv, { - ...sanitizationConfig, - enableEnvironmentVariableRedaction: true, - }); - const expandedHeaders: Record = {}; if (mcpServerConfig.headers) { for (const [key, value] of Object.entries(mcpServerConfig.headers)) { - expandedHeaders[key] = expandEnvVars(value, sanitizedEnv); + expandedHeaders[key] = expandEnvVars(value, process.env); } } @@ -840,14 +826,12 @@ function createAuthProvider( * @param mcpServerName The name of the MCP server * @param mcpServerConfig The MCP server configuration * @param accessToken The OAuth access token - * @param sanitizationConfig Configuration for environment sanitization * @returns The transport with OAuth token, or null if creation fails */ async function createTransportWithOAuth( mcpServerName: string, mcpServerConfig: MCPServerConfig, accessToken: string, - sanitizationConfig: EnvironmentSanitizationConfig, ): Promise { try { const headers: Record = { @@ -856,11 +840,7 @@ async function createTransportWithOAuth( const transportOptions: | StreamableHTTPClientTransportOptions | SSEClientTransportOptions = { - requestInit: createTransportRequestInit( - mcpServerConfig, - headers, - sanitizationConfig, - ), + requestInit: createTransportRequestInit(mcpServerConfig, headers), }; return createUrlTransport(mcpServerName, mcpServerConfig, transportOptions); @@ -1455,7 +1435,6 @@ async function showAuthRequiredMessage(serverName: string): Promise { * @param config The MCP server configuration * @param accessToken The OAuth access token to use * @param httpReturned404 Whether the HTTP transport returned 404 (indicating SSE-only server) - * @param sanitizationConfig Configuration for environment sanitization */ async function retryWithOAuth( client: Client, @@ -1463,7 +1442,6 @@ async function retryWithOAuth( config: MCPServerConfig, accessToken: string, httpReturned404: boolean, - sanitizationConfig: EnvironmentSanitizationConfig, ): Promise { if (httpReturned404) { // HTTP returned 404, only try SSE @@ -1484,7 +1462,6 @@ async function retryWithOAuth( serverName, config, accessToken, - sanitizationConfig, ); if (!httpTransport) { throw new Error( @@ -1764,7 +1741,6 @@ export async function connectToMcpServer( mcpServerConfig, accessToken, httpReturned404, - sanitizationConfig, ); return mcpClient; } else { @@ -1837,7 +1813,6 @@ export async function connectToMcpServer( mcpServerName, mcpServerConfig, accessToken, - sanitizationConfig, ); if (!oauthTransport) { throw new Error( @@ -1985,11 +1960,7 @@ export async function createTransport( const transportOptions: | StreamableHTTPClientTransportOptions | SSEClientTransportOptions = { - requestInit: createTransportRequestInit( - mcpServerConfig, - headers, - sanitizationConfig, - ), + requestInit: createTransportRequestInit(mcpServerConfig, headers), authProvider, }; @@ -1997,11 +1968,8 @@ export async function createTransport( } if (mcpServerConfig.command) { - const extensionEnv = getExtensionEnvironment(mcpServerConfig.extension); - const expansionEnv = { ...process.env, ...extensionEnv }; - // 1. Sanitize the base process environment to prevent unintended leaks of system-wide secrets. - const sanitizedEnv = sanitizeEnvironment(expansionEnv, { + const sanitizedEnv = sanitizeEnvironment(process.env, { ...sanitizationConfig, enableEnvironmentVariableRedaction: true, }); @@ -2009,7 +1977,6 @@ export async function createTransport( const finalEnv: Record = { [GEMINI_CLI_IDENTIFICATION_ENV_VAR]: GEMINI_CLI_IDENTIFICATION_ENV_VAR_VALUE, - ...extensionEnv, }; for (const [key, value] of Object.entries(sanitizedEnv)) { if (value !== undefined) { @@ -2020,7 +1987,7 @@ export async function createTransport( // Expand and merge explicit environment variables from the MCP configuration. if (mcpServerConfig.env) { for (const [key, value] of Object.entries(mcpServerConfig.env)) { - finalEnv[key] = expandEnvVars(value, expansionEnv); + finalEnv[key] = expandEnvVars(value, process.env); } } @@ -2078,20 +2045,6 @@ interface NamedTool { name?: string; } -function getExtensionEnvironment( - extension?: GeminiCLIExtension, -): Record { - const env: Record = {}; - if (extension?.resolvedSettings) { - for (const setting of extension.resolvedSettings) { - if (setting.value !== undefined) { - env[setting.envVar] = setting.value; - } - } - } - return env; -} - /** Visible for testing */ export function isEnabled( funcDecl: NamedTool, diff --git a/packages/core/src/tools/mcp-tool.ts b/packages/core/src/tools/mcp-tool.ts index 18c1638c95..ea516e4d99 100644 --- a/packages/core/src/tools/mcp-tool.ts +++ b/packages/core/src/tools/mcp-tool.ts @@ -93,6 +93,7 @@ export class DiscoveredMCPToolInvocation extends BaseToolInvocation< private readonly toolDescription?: string, private readonly toolParameterSchema?: unknown, toolAnnotationsData?: Record, + isSensitive: boolean = false, ) { // Use composite format for policy checks: serverName__toolName // This enables server wildcards (e.g., "google-workspace__*") @@ -105,6 +106,7 @@ export class DiscoveredMCPToolInvocation extends BaseToolInvocation< displayName, serverName, toolAnnotationsData, + isSensitive, ); } @@ -282,6 +284,7 @@ export class DiscoveredMCPTool extends BaseDeclarativeTool< false, // canUpdateOutput, extensionName, extensionId, + true, // isSensitive ); this._isReadOnly = isReadOnly; } @@ -330,6 +333,7 @@ export class DiscoveredMCPTool extends BaseDeclarativeTool< messageBus: MessageBus, _toolName?: string, _displayName?: string, + isSensitive?: boolean, ): ToolInvocation { return new DiscoveredMCPToolInvocation( this.mcpTool, @@ -343,6 +347,7 @@ export class DiscoveredMCPTool extends BaseDeclarativeTool< this.description, this.parameterSchema, this._toolAnnotations, + isSensitive, ); } } diff --git a/packages/core/src/tools/memoryTool.ts b/packages/core/src/tools/memoryTool.ts index 33cb9483e1..68b1b0c6c8 100644 --- a/packages/core/src/tools/memoryTool.ts +++ b/packages/core/src/tools/memoryTool.ts @@ -285,6 +285,9 @@ export class MemoryTool messageBus, true, false, + undefined, + undefined, + true, ); } diff --git a/packages/core/src/tools/read-file.ts b/packages/core/src/tools/read-file.ts index 170cccf905..276ae29ddc 100644 --- a/packages/core/src/tools/read-file.ts +++ b/packages/core/src/tools/read-file.ts @@ -11,7 +11,6 @@ import type { ToolInvocation, ToolLocation, ToolResult } from './tools.js'; import { BaseDeclarativeTool, BaseToolInvocation, Kind } from './tools.js'; import { ToolErrorType } from './tool-error.js'; -import type { PartUnion } from '@google/genai'; import { processSingleFileContent, getSpecificMimeType, @@ -45,20 +44,29 @@ export interface ReadFileToolParams { */ end_line?: number; } - class ReadFileToolInvocation extends BaseToolInvocation< ReadFileToolParams, ToolResult > { private readonly resolvedPath: string; + constructor( - private config: Config, + private readonly config: Config, params: ReadFileToolParams, messageBus: MessageBus, _toolName?: string, _toolDisplayName?: string, + isSensitive?: boolean, ) { - super(params, messageBus, _toolName, _toolDisplayName); + super( + params, + messageBus, + _toolName, + _toolDisplayName, + undefined, + undefined, + isSensitive, + ); this.resolvedPath = path.resolve( this.config.getTargetDir(), this.params.file_path, @@ -97,66 +105,80 @@ class ReadFileToolInvocation extends BaseToolInvocation< }, }; } + try { + const result = await processSingleFileContent( + this.resolvedPath, + this.config.getTargetDir(), + this.config.getFileSystemService(), + this.params.start_line, + this.params.end_line, + ); - const result = await processSingleFileContent( - this.resolvedPath, - this.config.getTargetDir(), - this.config.getFileSystemService(), - this.params.start_line, - this.params.end_line, - ); + if (result.error) { + return { + llmContent: result.llmContent, + returnDisplay: result.returnDisplay || 'Error reading file', + error: { + message: result.error, + type: result.errorType, + }, + }; + } - if (result.error) { - return { - llmContent: result.llmContent, - returnDisplay: result.returnDisplay || 'Error reading file', - error: { - message: result.error, - type: result.errorType, - }, - }; - } + let llmContent = result.llmContent; - let llmContent: PartUnion; - if (result.isTruncated) { - const [start, end] = result.linesShown!; - const total = result.originalLineCount!; - - llmContent = ` + if (result.isTruncated && typeof llmContent === 'string') { + const [startLine, endLine] = result.linesShown || [1, 0]; + llmContent = ` IMPORTANT: The file content has been truncated. -Status: Showing lines ${start}-${end} of ${total} total lines. -Action: To read more of the file, you can use the 'start_line' and 'end_line' parameters in a subsequent 'read_file' call. For example, to read the next section of the file, use start_line: ${end + 1}. +Status: Showing lines ${startLine}-${endLine} of ${result.originalLineCount} total lines. +Action: To read more of the file, you can use the 'start_line' and 'end_line' parameters in a subsequent 'read_file' call. For example, to read the next section of the file, use start_line: ${ + endLine + 1 + }. --- FILE CONTENT (truncated) --- -${result.llmContent}`; - } else { - llmContent = result.llmContent || ''; +${llmContent} +`; + } + + const programming_language = getProgrammingLanguage({ + file_path: this.resolvedPath, + }); + + logFileOperation( + this.config, + new FileOperationEvent( + this._toolName || READ_FILE_TOOL_NAME, + FileOperation.READ, + result.originalLineCount, + getSpecificMimeType(this.resolvedPath), + path.extname(this.resolvedPath), + programming_language, + ), + ); + + const finalResult: ToolResult = { + llmContent, + returnDisplay: result.returnDisplay || '', + }; + return finalResult; + } catch (err: unknown) { + const error = err instanceof Error ? err : new Error(String(err)); + const errorMessage = String(error.message); + const toolResult: ToolResult = { + llmContent: [ + { + text: `Error reading file: ${errorMessage}`, + }, + ], + returnDisplay: `Error: ${errorMessage}`, + error: { + message: errorMessage, + type: ToolErrorType.EXECUTION_FAILED, + }, + }; + return toolResult; } - - const lines = - typeof result.llmContent === 'string' - ? result.llmContent.split('\n').length - : undefined; - const mimetype = getSpecificMimeType(this.resolvedPath); - const programming_language = getProgrammingLanguage({ - file_path: this.resolvedPath, - }); - logFileOperation( - this.config, - new FileOperationEvent( - READ_FILE_TOOL_NAME, - FileOperation.READ, - lines, - mimetype, - path.extname(this.resolvedPath), - programming_language, - ), - ); - - return { - llmContent, - returnDisplay: result.returnDisplay || '', - }; } } @@ -183,6 +205,9 @@ export class ReadFileTool extends BaseDeclarativeTool< messageBus, true, false, + undefined, + undefined, + true, ); this.fileDiscoveryService = new FileDiscoveryService( config.getTargetDir(), @@ -193,29 +218,18 @@ export class ReadFileTool extends BaseDeclarativeTool< protected override validateToolParamValues( params: ReadFileToolParams, ): string | null { - if (params.file_path.trim() === '') { + if (!params.file_path) { return "The 'file_path' parameter must be non-empty."; } - const resolvedPath = path.resolve( - this.config.getTargetDir(), - params.file_path, - ); - - const validationError = this.config.validatePathAccess( - resolvedPath, - 'read', - ); - if (validationError) { - return validationError; - } - if (params.start_line !== undefined && params.start_line < 1) { return 'start_line must be at least 1'; } + if (params.end_line !== undefined && params.end_line < 1) { return 'end_line must be at least 1'; } + if ( params.start_line !== undefined && params.end_line !== undefined && @@ -224,6 +238,18 @@ export class ReadFileTool extends BaseDeclarativeTool< return 'start_line cannot be greater than end_line'; } + const resolvedPath = path.resolve( + this.config.getTargetDir(), + params.file_path, + ); + const validationError = this.config.validatePathAccess( + resolvedPath, + 'read', + ); + if (validationError) { + return validationError; + } + const fileFilteringOptions = this.config.getFileFilteringOptions(); if ( this.fileDiscoveryService.shouldIgnoreFile( @@ -242,6 +268,7 @@ export class ReadFileTool extends BaseDeclarativeTool< messageBus: MessageBus, _toolName?: string, _toolDisplayName?: string, + isSensitive?: boolean, ): ToolInvocation { return new ReadFileToolInvocation( this.config, @@ -249,6 +276,7 @@ export class ReadFileTool extends BaseDeclarativeTool< messageBus, _toolName, _toolDisplayName, + isSensitive, ); } diff --git a/packages/core/src/tools/read-many-files.ts b/packages/core/src/tools/read-many-files.ts index 0a5d68a6ba..78c23bacb3 100644 --- a/packages/core/src/tools/read-many-files.ts +++ b/packages/core/src/tools/read-many-files.ts @@ -114,8 +114,17 @@ class ReadManyFilesToolInvocation extends BaseToolInvocation< messageBus: MessageBus, _toolName?: string, _toolDisplayName?: string, + isSensitive?: boolean, ) { - super(params, messageBus, _toolName, _toolDisplayName); + super( + params, + messageBus, + _toolName, + _toolDisplayName, + undefined, + undefined, + isSensitive, + ); } getDescription(): string { @@ -474,6 +483,9 @@ export class ReadManyFilesTool extends BaseDeclarativeTool< messageBus, true, // isOutputMarkdown false, // canUpdateOutput + undefined, + undefined, + true, ); } @@ -482,6 +494,7 @@ export class ReadManyFilesTool extends BaseDeclarativeTool< messageBus: MessageBus, _toolName?: string, _toolDisplayName?: string, + isSensitive?: boolean, ): ToolInvocation { return new ReadManyFilesToolInvocation( this.config, @@ -489,6 +502,7 @@ export class ReadManyFilesTool extends BaseDeclarativeTool< messageBus, _toolName, _toolDisplayName, + isSensitive, ); } diff --git a/packages/core/src/tools/ripGrep.ts b/packages/core/src/tools/ripGrep.ts index ac65cf6362..9e04011373 100644 --- a/packages/core/src/tools/ripGrep.ts +++ b/packages/core/src/tools/ripGrep.ts @@ -167,8 +167,17 @@ class GrepToolInvocation extends BaseToolInvocation< messageBus: MessageBus, _toolName?: string, _toolDisplayName?: string, + isSensitive?: boolean, ) { - super(params, messageBus, _toolName, _toolDisplayName); + super( + params, + messageBus, + _toolName, + _toolDisplayName, + undefined, + undefined, + isSensitive, + ); } async execute(signal: AbortSignal): Promise { @@ -584,6 +593,9 @@ export class RipGrepTool extends BaseDeclarativeTool< messageBus, true, // isOutputMarkdown false, // canUpdateOutput + undefined, + undefined, + true, ); this.fileDiscoveryService = new FileDiscoveryService( config.getTargetDir(), @@ -665,14 +677,16 @@ export class RipGrepTool extends BaseDeclarativeTool< messageBus: MessageBus, _toolName?: string, _toolDisplayName?: string, + isSensitive?: boolean, ): ToolInvocation { return new GrepToolInvocation( this.config, this.fileDiscoveryService, params, - messageBus ?? this.messageBus, + messageBus, _toolName, _toolDisplayName, + isSensitive, ); } diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index 741272f555..0a9be05986 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -68,8 +68,17 @@ export class ShellToolInvocation extends BaseToolInvocation< messageBus: MessageBus, _toolName?: string, _toolDisplayName?: string, + isSensitive?: boolean, ) { - super(params, messageBus, _toolName, _toolDisplayName); + super( + params, + messageBus, + _toolName, + _toolDisplayName, + undefined, + undefined, + isSensitive, + ); } getDescription(): string { @@ -479,6 +488,9 @@ export class ShellTool extends BaseDeclarativeTool< messageBus, false, // output is not markdown true, // output can be updated + undefined, + undefined, + true, ); } @@ -504,6 +516,7 @@ export class ShellTool extends BaseDeclarativeTool< messageBus: MessageBus, _toolName?: string, _toolDisplayName?: string, + isSensitive?: boolean, ): ToolInvocation { return new ShellToolInvocation( this.config, @@ -511,6 +524,7 @@ export class ShellTool extends BaseDeclarativeTool< messageBus, _toolName, _toolDisplayName, + isSensitive, ); } diff --git a/packages/core/src/tools/tools.ts b/packages/core/src/tools/tools.ts index 3c024168d4..07fc843dca 100644 --- a/packages/core/src/tools/tools.ts +++ b/packages/core/src/tools/tools.ts @@ -75,6 +75,7 @@ export interface ToolInvocation< export interface PolicyUpdateOptions { commandPrefix?: string | string[]; mcpName?: string; + argsPattern?: string; } /** @@ -92,6 +93,7 @@ export abstract class BaseToolInvocation< readonly _toolDisplayName?: string, readonly _serverName?: string, readonly _toolAnnotations?: Record, + readonly isSensitive: boolean = false, ) {} abstract getDescription(): string; @@ -152,6 +154,7 @@ export abstract class BaseToolInvocation< type: MessageBusType.UPDATE_POLICY, toolName: this._toolName, persist: outcome === ToolConfirmationOutcome.ProceedAlwaysAndSave, + isSensitive: this.isSensitive, ...options, }); } @@ -340,6 +343,11 @@ export interface ToolBuilder< */ isReadOnly: boolean; + /** + * Whether the tool is sensitive and requires specific policy approvals. + */ + isSensitive: boolean; + /** * Validates raw parameters and builds a ready-to-execute invocation. * @param params The raw, untrusted parameters from the model. @@ -368,6 +376,7 @@ export abstract class DeclarativeTool< readonly canUpdateOutput: boolean = false, readonly extensionName?: string, readonly extensionId?: string, + readonly isSensitive: boolean = false, ) {} get isReadOnly(): boolean { @@ -498,6 +507,34 @@ export abstract class BaseDeclarativeTool< TParams extends object, TResult extends ToolResult, > extends DeclarativeTool { + constructor( + name: string, + displayName: string, + description: string, + kind: Kind, + parameterSchema: unknown, + messageBus: MessageBus, + isOutputMarkdown: boolean = true, + canUpdateOutput: boolean = false, + extensionName?: string, + extensionId?: string, + isSensitive: boolean = false, + ) { + super( + name, + displayName, + description, + kind, + parameterSchema, + messageBus, + isOutputMarkdown, + canUpdateOutput, + extensionName, + extensionId, + isSensitive, + ); + } + build(params: TParams): ToolInvocation { const validationError = this.validateToolParams(params); if (validationError) { @@ -508,6 +545,7 @@ export abstract class BaseDeclarativeTool< this.messageBus, this.name, this.displayName, + this.isSensitive, ); } @@ -533,6 +571,7 @@ export abstract class BaseDeclarativeTool< messageBus: MessageBus, _toolName?: string, _toolDisplayName?: string, + isSensitive?: boolean, ): ToolInvocation; } @@ -826,6 +865,7 @@ export enum ToolConfirmationOutcome { export enum Kind { Read = 'read', + Write = 'write', Edit = 'edit', Delete = 'delete', Move = 'move', @@ -842,6 +882,7 @@ export enum Kind { // Function kinds that have side effects export const MUTATOR_KINDS: Kind[] = [ + Kind.Write, Kind.Edit, Kind.Delete, Kind.Move, diff --git a/packages/core/src/tools/web-fetch.ts b/packages/core/src/tools/web-fetch.ts index 55d2474c1c..2bedb6a52b 100644 --- a/packages/core/src/tools/web-fetch.ts +++ b/packages/core/src/tools/web-fetch.ts @@ -178,8 +178,17 @@ class WebFetchToolInvocation extends BaseToolInvocation< messageBus: MessageBus, _toolName?: string, _toolDisplayName?: string, + isSensitive?: boolean, ) { - super(params, messageBus, _toolName, _toolDisplayName); + super( + params, + messageBus, + _toolName, + _toolDisplayName, + undefined, + undefined, + isSensitive, + ); } private async executeFallback(signal: AbortSignal): Promise { @@ -689,6 +698,9 @@ export class WebFetchTool extends BaseDeclarativeTool< messageBus, true, // isOutputMarkdown false, // canUpdateOutput + undefined, + undefined, + true, ); } @@ -729,6 +741,7 @@ export class WebFetchTool extends BaseDeclarativeTool< messageBus: MessageBus, _toolName?: string, _toolDisplayName?: string, + isSensitive?: boolean, ): ToolInvocation { return new WebFetchToolInvocation( this.config, @@ -736,6 +749,7 @@ export class WebFetchTool extends BaseDeclarativeTool< messageBus, _toolName, _toolDisplayName, + isSensitive, ); } diff --git a/packages/core/src/tools/web-search.ts b/packages/core/src/tools/web-search.ts index a5ac9937b8..d16e116313 100644 --- a/packages/core/src/tools/web-search.ts +++ b/packages/core/src/tools/web-search.ts @@ -71,8 +71,17 @@ class WebSearchToolInvocation extends BaseToolInvocation< messageBus: MessageBus, _toolName?: string, _toolDisplayName?: string, + isSensitive?: boolean, ) { - super(params, messageBus, _toolName, _toolDisplayName); + super( + params, + messageBus, + _toolName, + _toolDisplayName, + undefined, + undefined, + isSensitive, + ); } override getDescription(): string { @@ -208,6 +217,9 @@ export class WebSearchTool extends BaseDeclarativeTool< messageBus, true, // isOutputMarkdown false, // canUpdateOutput + undefined, + undefined, + true, ); } @@ -230,6 +242,7 @@ export class WebSearchTool extends BaseDeclarativeTool< messageBus: MessageBus, _toolName?: string, _toolDisplayName?: string, + isSensitive?: boolean, ): ToolInvocation { return new WebSearchToolInvocation( this.config, @@ -237,6 +250,7 @@ export class WebSearchTool extends BaseDeclarativeTool< messageBus ?? this.messageBus, _toolName, _toolDisplayName, + isSensitive, ); } diff --git a/packages/core/src/tools/write-file.ts b/packages/core/src/tools/write-file.ts index 1c8a230001..ef403a57f0 100644 --- a/packages/core/src/tools/write-file.ts +++ b/packages/core/src/tools/write-file.ts @@ -136,8 +136,17 @@ class WriteFileToolInvocation extends BaseToolInvocation< messageBus: MessageBus, toolName?: string, displayName?: string, + isSensitive?: boolean, ) { - super(params, messageBus, toolName, displayName); + super( + params, + messageBus, + toolName, + displayName, + undefined, + undefined, + isSensitive, + ); this.resolvedPath = path.resolve( this.config.getTargetDir(), this.params.file_path, @@ -429,11 +438,14 @@ export class WriteFileTool WriteFileTool.Name, WRITE_FILE_DISPLAY_NAME, WRITE_FILE_DEFINITION.base.description!, - Kind.Edit, + Kind.Write, WRITE_FILE_DEFINITION.base.parametersJsonSchema, messageBus, true, false, + undefined, + undefined, + true, ); } @@ -477,6 +489,9 @@ export class WriteFileTool protected createInvocation( params: WriteFileToolParams, messageBus: MessageBus, + _toolName?: string, + _toolDisplayName?: string, + isSensitive?: boolean, ): ToolInvocation { return new WriteFileToolInvocation( this.config, @@ -484,6 +499,7 @@ export class WriteFileTool messageBus ?? this.messageBus, this.name, this.displayName, + isSensitive, ); } diff --git a/packages/core/src/tools/write-todos.ts b/packages/core/src/tools/write-todos.ts index 5eb42c73f4..f94174c3bd 100644 --- a/packages/core/src/tools/write-todos.ts +++ b/packages/core/src/tools/write-todos.ts @@ -34,8 +34,17 @@ class WriteTodosToolInvocation extends BaseToolInvocation< messageBus: MessageBus, _toolName?: string, _toolDisplayName?: string, + isSensitive?: boolean, ) { - super(params, messageBus, _toolName, _toolDisplayName); + super( + params, + messageBus, + _toolName, + _toolDisplayName, + undefined, + undefined, + isSensitive, + ); } getDescription(): string { @@ -85,6 +94,9 @@ export class WriteTodosTool extends BaseDeclarativeTool< messageBus, true, // isOutputMarkdown false, // canUpdateOutput + undefined, + undefined, + true, ); } @@ -128,12 +140,14 @@ export class WriteTodosTool extends BaseDeclarativeTool< messageBus: MessageBus, _toolName?: string, _displayName?: string, + isSensitive?: boolean, ): ToolInvocation { return new WriteTodosToolInvocation( params, messageBus, _toolName, _displayName, + isSensitive, ); } }