refactor(policy): add isSensitive plumbing and tool Kind.Write

2026-03-18 01:51:20 -07:00 · 2026-02-27 20:32:20 +00:00
parent d7320f5425
commit c6ff82944f
21 changed files with 367 additions and 215 deletions
--- 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 {
--- 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.
--- 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<ActivateSkillToolParams, ToolResult> {
    return new ActivateSkillToolInvocation(
      this.config,
      params,
      messageBus,
      _toolName,
-      _toolDisplayName ?? 'Activate Skill',
+      _toolDisplayName,
    );
  }

--- 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<EditToolParams, ToolResult> {
    return new EditToolInvocation(
      this.config,
@@ -1008,6 +1023,7 @@ export class EditTool
      messageBus,
      this.name,
      this.displayName,
+      isSensitive,
    );
  }

--- 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<GetInternalDocsParams, ToolResult> {
    return new GetInternalDocsInvocation(
      params,
      messageBus,
      _toolName ?? GetInternalDocsTool.Name,
      _toolDisplayName,
+      isSensitive,
    );
  }

--- 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<GlobToolParams, ToolResult> {
      messageBus,
      true,
      false,
+      undefined,
+      undefined,
+      true,
    );
  }

@@ -328,6 +340,7 @@ export class GlobTool extends BaseDeclarativeTool<GlobToolParams, ToolResult> {
    messageBus: MessageBus,
    _toolName?: string,
    _toolDisplayName?: string,
+    isSensitive?: boolean,
  ): ToolInvocation<GlobToolParams, ToolResult> {
    return new GlobToolInvocation(
      this.config,
@@ -335,6 +348,7 @@ export class GlobTool extends BaseDeclarativeTool<GlobToolParams, ToolResult> {
      messageBus,
      _toolName,
      _toolDisplayName,
+      isSensitive,
    );
  }

--- 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<GrepToolParams, ToolResult> {
      messageBus,
      true,
      false,
+      undefined,
+      undefined,
+      true,
    );
  }

@@ -676,6 +688,7 @@ export class GrepTool extends BaseDeclarativeTool<GrepToolParams, ToolResult> {
    messageBus: MessageBus,
    _toolName?: string,
    _toolDisplayName?: string,
+    isSensitive?: boolean,
  ): ToolInvocation<GrepToolParams, ToolResult> {
    return new GrepToolInvocation(
      this.config,
@@ -683,6 +696,7 @@ export class GrepTool extends BaseDeclarativeTool<GrepToolParams, ToolResult> {
      messageBus,
      _toolName,
      _toolDisplayName,
+      isSensitive,
    );
  }

--- a/packages/core/src/tools/ls.ts
+++ b/packages/core/src/tools/ls.ts
@@ -78,8 +78,17 @@ class LSToolInvocation extends BaseToolInvocation<LSToolParams, ToolResult> {
    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<LSToolParams, ToolResult> {
      messageBus,
      true,
      false,
+      undefined,
+      undefined,
+      true,
    );
  }

@@ -316,13 +328,15 @@ export class LSTool extends BaseDeclarativeTool<LSToolParams, ToolResult> {
    messageBus: MessageBus,
    _toolName?: string,
    _toolDisplayName?: string,
+    isSensitive?: boolean,
  ): ToolInvocation<LSToolParams, ToolResult> {
    return new LSToolInvocation(
      this.config,
      params,
-      messageBus ?? this.messageBus,
+      messageBus,
      _toolName,
      _toolDisplayName,
+      isSensitive,
    );
  }

--- 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')
--- 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<string, string>,
-  sanitizationConfig: EnvironmentSanitizationConfig,
 ): RequestInit {
-  const extensionEnv = getExtensionEnvironment(mcpServerConfig.extension);
-  const expansionEnv = { ...process.env, ...extensionEnv };
-
-  const sanitizedEnv = sanitizeEnvironment(expansionEnv, {
-    ...sanitizationConfig,
-    enableEnvironmentVariableRedaction: true,
-  });
-
  const expandedHeaders: Record<string, string> = {};
  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<StreamableHTTPClientTransport | SSEClientTransport | null> {
  try {
    const headers: Record<string, string> = {
@@ -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<never> {
 * @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<void> {
  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<string, string> = {
      [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<string, string> {
-  const env: Record<string, string> = {};
-  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,
--- 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<string, unknown>,
+    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<ToolParams, ToolResult> {
    return new DiscoveredMCPToolInvocation(
      this.mcpTool,
@@ -343,6 +347,7 @@ export class DiscoveredMCPTool extends BaseDeclarativeTool<
      this.description,
      this.parameterSchema,
      this._toolAnnotations,
+      isSensitive,
    );
  }
 }
--- 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,
    );
  }

--- 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<ReadFileToolParams, ToolResult> {
    return new ReadFileToolInvocation(
      this.config,
@@ -249,6 +276,7 @@ export class ReadFileTool extends BaseDeclarativeTool<
      messageBus,
      _toolName,
      _toolDisplayName,
+      isSensitive,
    );
  }

--- 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<ReadManyFilesParams, ToolResult> {
    return new ReadManyFilesToolInvocation(
      this.config,
@@ -489,6 +502,7 @@ export class ReadManyFilesTool extends BaseDeclarativeTool<
      messageBus,
      _toolName,
      _toolDisplayName,
+      isSensitive,
    );
  }

--- 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<ToolResult> {
@@ -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<RipGrepToolParams, ToolResult> {
    return new GrepToolInvocation(
      this.config,
      this.fileDiscoveryService,
      params,
-      messageBus ?? this.messageBus,
+      messageBus,
      _toolName,
      _toolDisplayName,
+      isSensitive,
    );
  }

--- 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<ShellToolParams, ToolResult> {
    return new ShellToolInvocation(
      this.config,
@@ -511,6 +524,7 @@ export class ShellTool extends BaseDeclarativeTool<
      messageBus,
      _toolName,
      _toolDisplayName,
+      isSensitive,
    );
  }

--- 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<string, unknown>,
+    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<TParams, TResult> {
+  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<TParams, TResult> {
    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<TParams, TResult>;
 }

@@ -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,
--- 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<ToolResult> {
@@ -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<WebFetchToolParams, ToolResult> {
    return new WebFetchToolInvocation(
      this.config,
@@ -736,6 +749,7 @@ export class WebFetchTool extends BaseDeclarativeTool<
      messageBus,
      _toolName,
      _toolDisplayName,
+      isSensitive,
    );
  }

--- 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<WebSearchToolParams, WebSearchToolResult> {
    return new WebSearchToolInvocation(
      this.config,
@@ -237,6 +250,7 @@ export class WebSearchTool extends BaseDeclarativeTool<
      messageBus ?? this.messageBus,
      _toolName,
      _toolDisplayName,
+      isSensitive,
    );
  }

--- 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<WriteFileToolParams, ToolResult> {
    return new WriteFileToolInvocation(
      this.config,
@@ -484,6 +499,7 @@ export class WriteFileTool
      messageBus ?? this.messageBus,
      this.name,
      this.displayName,
+      isSensitive,
    );
  }

--- 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<WriteTodosToolParams, ToolResult> {
    return new WriteTodosToolInvocation(
      params,
      messageBus,
      _toolName,
      _displayName,
+      isSensitive,
    );
  }
 }