diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index bbc8b1681e..65cca798b0 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -818,9 +818,10 @@ export async function loadCliConfig( model: resolvedModel, maxSessionTurns: settings.model?.maxSessionTurns, experimentalZedIntegration: argv.experimentalAcp || false, - listExtensions: argv.listExtensions || false, listSessions: argv.listSessions || false, deleteSession: argv.deleteSession, + autoAddPolicy: + settings.security?.autoAddPolicy && !settings.admin?.secureModeEnabled, enabledExtensions: argv.extensions, extensionLoader: extensionManager, enableExtensionReloading: settings.experimental?.extensionReloading, @@ -843,7 +844,6 @@ export async function loadCliConfig( interactive, trustedFolder, useBackgroundColor: settings.ui?.useBackgroundColor, - useAlternateBuffer: settings.ui?.useAlternateBuffer, useRipgrep: settings.tools?.useRipgrep, enableInteractiveShell: settings.tools?.shell?.enableInteractiveShell, shellToolInactivityTimeout: settings.tools?.shell?.inactivityTimeout, diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index 48a7641766..a69b40cc2e 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -1436,6 +1436,16 @@ const SETTINGS_SCHEMA = { 'Enable the "Allow for all future sessions" option in tool confirmation dialogs.', showInDialog: true, }, + autoAddPolicy: { + type: 'boolean', + label: 'Auto-add to Policy', + category: 'Security', + requiresRestart: false, + default: true, + description: + 'Automatically add "Proceed always" approvals to your persistent policy.', + showInDialog: true, + }, blockGitExtensions: { type: 'boolean', label: 'Blocks extensions from Git', diff --git a/packages/cli/src/gemini.tsx b/packages/cli/src/gemini.tsx index 2e238765e8..1037a6ae96 100644 --- a/packages/cli/src/gemini.tsx +++ b/packages/cli/src/gemini.tsx @@ -594,6 +594,13 @@ export async function main() { const messageBus = config.getMessageBus(); createPolicyUpdater(policyEngine, messageBus, config.storage); + // Listen for settings changes to update reactive config properties + coreEvents.on(CoreEvent.SettingsChanged, () => { + if (settings.merged.security.autoAddPolicy !== undefined) { + config.setAutoAddPolicy(settings.merged.security.autoAddPolicy); + } + }); + // Register SessionEnd hook to fire on graceful exit // This runs before telemetry shutdown in runExitCleanup() registerCleanup(async () => { diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 256e079fde..39a8c53711 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -428,6 +428,7 @@ export enum AuthProviderType { export interface SandboxConfig { command: 'docker' | 'podman' | 'sandbox-exec'; image: string; + flags?: string; } /** @@ -498,6 +499,7 @@ export interface ConfigParameters { experimentalZedIntegration?: boolean; listSessions?: boolean; deleteSession?: string; + autoAddPolicy?: boolean; listExtensions?: boolean; extensionLoader?: ExtensionLoader; enabledExtensions?: string[]; @@ -519,7 +521,6 @@ export interface ConfigParameters { interactive?: boolean; trustedFolder?: boolean; useBackgroundColor?: boolean; - useAlternateBuffer?: boolean; useRipgrep?: boolean; enableInteractiveShell?: boolean; skipNextSpeakerCheck?: boolean; @@ -651,6 +652,7 @@ export class Config { private _activeModel: string; private readonly maxSessionTurns: number; + private readonly autoAddPolicy: boolean; private readonly listSessions: boolean; private readonly deleteSession: string | undefined; private readonly listExtensions: boolean; @@ -703,7 +705,6 @@ export class Config { private readonly enableInteractiveShell: boolean; private readonly skipNextSpeakerCheck: boolean; private readonly useBackgroundColor: boolean; - private readonly useAlternateBuffer: boolean; private shellExecutionConfig: ShellExecutionConfig; private readonly extensionManagement: boolean = true; private readonly truncateToolOutputThreshold: number; @@ -882,6 +883,7 @@ export class Config { params.experimentalZedIntegration ?? false; this.listSessions = params.listSessions ?? false; this.deleteSession = params.deleteSession; + this.autoAddPolicy = params.autoAddPolicy ?? false; this.listExtensions = params.listExtensions ?? false; this._extensionLoader = params.extensionLoader ?? new SimpleExtensionLoader([]); @@ -902,7 +904,6 @@ export class Config { this.directWebFetch = params.directWebFetch ?? false; this.useRipgrep = params.useRipgrep ?? true; this.useBackgroundColor = params.useBackgroundColor ?? true; - this.useAlternateBuffer = params.useAlternateBuffer ?? false; this.enableInteractiveShell = params.enableInteractiveShell ?? false; this.skipNextSpeakerCheck = params.skipNextSpeakerCheck ?? true; this.shellExecutionConfig = { @@ -2144,6 +2145,18 @@ export class Config { return this.bugCommand; } + getAutoAddPolicy(): boolean { + if (this.disableYoloMode) { + return false; + } + return this.autoAddPolicy; + } + + setAutoAddPolicy(value: boolean): void { + // @ts-expect-error - readonly property + this.autoAddPolicy = value; + } + getFileService(): FileDiscoveryService { if (!this.fileDiscoveryService) { this.fileDiscoveryService = new FileDiscoveryService(this.targetDir, { @@ -2524,10 +2537,6 @@ export class Config { return this.useBackgroundColor; } - getUseAlternateBuffer(): boolean { - return this.useAlternateBuffer; - } - getEnableInteractiveShell(): boolean { return this.enableInteractiveShell; } diff --git a/packages/core/src/policy/auto-add.test.ts b/packages/core/src/policy/auto-add.test.ts new file mode 100644 index 0000000000..98f1c32f8d --- /dev/null +++ b/packages/core/src/policy/auto-add.test.ts @@ -0,0 +1,206 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + describe, + it, + expect, + vi, + beforeEach, + afterEach, + type Mocked, +} from 'vitest'; +import * as fs from 'node:fs/promises'; +import { createPolicyUpdater } from './config.js'; +import { + MessageBusType, + type UpdatePolicy, +} from '../confirmation-bus/types.js'; +import { coreEvents } from '../utils/events.js'; +import type { PolicyEngine } from './policy-engine.js'; +import type { MessageBus } from '../confirmation-bus/message-bus.js'; +import type { Storage } from '../config/storage.js'; + +vi.mock('node:fs/promises'); +vi.mock('../utils/events.js', () => ({ + coreEvents: { + emitFeedback: vi.fn(), + }, +})); + +describe('Policy Auto-add Safeguards', () => { + let policyEngine: Mocked; + let messageBus: Mocked; + let storage: Mocked; + let updateCallback: (msg: UpdatePolicy) => Promise; + + beforeEach(() => { + policyEngine = { + addRule: vi.fn(), + } as unknown as Mocked; + messageBus = { + subscribe: vi.fn((type, cb) => { + if (type === MessageBusType.UPDATE_POLICY) { + updateCallback = cb; + } + }), + publish: vi.fn(), + } as unknown as Mocked; + storage = { + getWorkspacePoliciesDir: vi.fn().mockReturnValue('/tmp/policies'), + getAutoSavedPolicyPath: vi + .fn() + .mockReturnValue('/tmp/policies/autosaved.toml'), + } as unknown as Mocked; + + const enoent = new Error('ENOENT'); + (enoent as NodeJS.ErrnoException).code = 'ENOENT'; + + vi.mocked(fs.mkdir).mockResolvedValue(undefined); + vi.mocked(fs.readFile).mockRejectedValue(enoent); + vi.mocked(fs.open).mockResolvedValue({ + writeFile: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), + } as unknown as fs.FileHandle); + vi.mocked(fs.rename).mockResolvedValue(undefined); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('should skip persistence for wildcard toolName', async () => { + createPolicyUpdater(policyEngine, messageBus, storage); + expect(updateCallback).toBeDefined(); + + await updateCallback({ + type: MessageBusType.UPDATE_POLICY, + toolName: '*', + persist: true, + }); + + expect(fs.open).not.toHaveBeenCalled(); + expect(coreEvents.emitFeedback).toHaveBeenCalledWith( + 'warning', + expect.stringContaining('Policy for all tools was not auto-saved'), + ); + }); + + it('should skip persistence for broad argsPattern (.*)', async () => { + createPolicyUpdater(policyEngine, messageBus, storage); + + await updateCallback({ + type: MessageBusType.UPDATE_POLICY, + toolName: 'test-tool', + argsPattern: '.*', + persist: true, + }); + + expect(fs.open).not.toHaveBeenCalled(); + expect(coreEvents.emitFeedback).toHaveBeenCalledWith( + 'warning', + expect.stringContaining('was not auto-saved for safety reasons'), + ); + }); + + it('should allow persistence for specific argsPattern', async () => { + createPolicyUpdater(policyEngine, messageBus, storage); + + await updateCallback({ + type: MessageBusType.UPDATE_POLICY, + toolName: 'test-tool', + argsPattern: '.*"file_path":"test.txt".*', + persist: true, + }); + + await vi.waitFor(() => { + expect(fs.open).toHaveBeenCalled(); + }); + expect(fs.rename).toHaveBeenCalledWith( + expect.stringContaining('autosaved.toml'), + '/tmp/policies/autosaved.toml', + ); + }); + + it('should skip persistence for sensitive tool with no pattern', async () => { + createPolicyUpdater(policyEngine, messageBus, storage); + + await updateCallback({ + type: MessageBusType.UPDATE_POLICY, + toolName: 'sensitive-tool', + isSensitive: true, + persist: true, + }); + + await vi.waitFor(() => { + expect(coreEvents.emitFeedback).toHaveBeenCalledWith( + 'warning', + expect.stringContaining( + 'Broad approval for "sensitive-tool" was not auto-saved', + ), + ); + }); + + expect(fs.open).not.toHaveBeenCalled(); + }); + + it('should skip persistence for MCP tool with no pattern', async () => { + createPolicyUpdater(policyEngine, messageBus, storage); + + const mcpToolName = 'mcp-server__sensitive-tool'; + await updateCallback({ + type: MessageBusType.UPDATE_POLICY, + toolName: mcpToolName, + mcpName: 'mcp-server', + persist: true, + }); + + await vi.waitFor(() => { + expect(coreEvents.emitFeedback).toHaveBeenCalledWith( + 'warning', + expect.stringContaining( + `Broad approval for "${mcpToolName}" was not auto-saved`, + ), + ); + }); + + expect(fs.open).not.toHaveBeenCalled(); + }); + + it('should de-duplicate identical rules when auto-saving', async () => { + createPolicyUpdater(policyEngine, messageBus, storage); + + // First call: file doesn't exist (ENOENT already mocked in beforeEach) + await updateCallback({ + type: MessageBusType.UPDATE_POLICY, + toolName: 'read_file', + argsPattern: '.*"file_path":"test.txt".*', + persist: true, + isSensitive: true, + }); + + await vi.waitFor(() => { + expect(fs.open).toHaveBeenCalledTimes(1); + }); + + // Mock file existing with the rule for the second call + vi.mocked(fs.readFile).mockResolvedValue( + '[[rule]]\ntoolName = "read_file"\ndecision = "allow"\npriority = 100\nargsPattern = \'.*"file_path":"test.txt".*\'\n', + ); + + // Second call: should skip persistence because it's a duplicate + await updateCallback({ + type: MessageBusType.UPDATE_POLICY, + toolName: 'read_file', + argsPattern: '.*"file_path":"test.txt".*', + persist: true, + isSensitive: true, + }); + + // Still only 1 call to fs.open + expect(fs.open).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/core/src/policy/config.ts b/packages/core/src/policy/config.ts index 800006e27e..eaa104f82c 100644 --- a/packages/core/src/policy/config.ts +++ b/packages/core/src/policy/config.ts @@ -514,6 +514,40 @@ export function createPolicyUpdater( } if (message.persist) { + // Validation safeguards for auto-adding to persistent policy + if (!toolName || toolName === '*') { + coreEvents.emitFeedback( + 'warning', + 'Policy for all tools was not auto-saved for safety reasons. You can add it manually to your policy file if desired.', + ); + return; + } + + const broadPatternRegex = /^\s*(\^)?\.\*(\$)?\s*$/; + if ( + message.argsPattern && + broadPatternRegex.test(message.argsPattern) + ) { + coreEvents.emitFeedback( + 'warning', + `Policy for "${toolName}" with all arguments was not auto-saved for safety reasons. You can add it manually to your policy file if desired.`, + ); + return; + } + + const isMcpTool = !!message.mcpName; + if ( + (message.isSensitive || isMcpTool) && + !message.argsPattern && + !message.commandPrefix + ) { + coreEvents.emitFeedback( + 'warning', + `Broad approval for "${toolName}" was not auto-saved for safety reasons. Approvals for sensitive tools must be specific to be auto-saved.`, + ); + return; + } + persistenceQueue = persistenceQueue.then(async () => { try { const workspacePoliciesDir = storage.getWorkspacePoliciesDir(); @@ -571,6 +605,36 @@ export function createPolicyUpdater( newRule.argsPattern = message.argsPattern; } + // De-duplicate: check if an identical rule already exists + const isDuplicate = existingData.rule.some((r) => { + const matchesTool = r.toolName === newRule.toolName; + const matchesMcp = r.mcpName === newRule.mcpName; + const matchesPattern = r.argsPattern === newRule.argsPattern; + + const rPrefix = r.commandPrefix; + const newPrefix = newRule.commandPrefix; + + let matchesPrefix = false; + if (Array.isArray(rPrefix) && Array.isArray(newPrefix)) { + matchesPrefix = + rPrefix.length === newPrefix.length && + rPrefix.every((v, i) => v === newPrefix[i]); + } else { + matchesPrefix = rPrefix === newPrefix; + } + + return ( + matchesTool && matchesMcp && matchesPattern && matchesPrefix + ); + }); + + if (isDuplicate) { + debugLogger.debug( + `Policy rule for ${toolName} already exists in persistent policy, skipping.`, + ); + return; + } + // Add to rules existingData.rule.push(newRule); diff --git a/packages/core/src/policy/persistence.test.ts b/packages/core/src/policy/persistence.test.ts index 43f52a956d..a5d1d6160d 100644 --- a/packages/core/src/policy/persistence.test.ts +++ b/packages/core/src/policy/persistence.test.ts @@ -198,6 +198,7 @@ describe('createPolicyUpdater', () => { toolName, persist: true, mcpName, + argsPattern: '{"issue": ".*"}', }); await new Promise((resolve) => setTimeout(resolve, 0)); @@ -208,6 +209,7 @@ describe('createPolicyUpdater', () => { const writtenContent = writeCall[0] as string; expect(writtenContent).toContain(`mcpName = "${mcpName}"`); expect(writtenContent).toContain(`toolName = "${simpleToolName}"`); + expect(writtenContent).toContain(`argsPattern = '{"issue": ".*"}'`); expect(writtenContent).toContain('priority = 200'); }); @@ -243,6 +245,7 @@ describe('createPolicyUpdater', () => { toolName, persist: true, mcpName, + argsPattern: '{"query": ".*"}', }); await new Promise((resolve) => setTimeout(resolve, 0)); @@ -265,5 +268,7 @@ describe('createPolicyUpdater', () => { } catch { expect(writtenContent).toContain(`toolName = 'search"tool"'`); } + + expect(writtenContent).toContain(`argsPattern = '{"query": ".*"}'`); }); }); diff --git a/packages/core/src/scheduler/auto-add-scheduler.test.ts b/packages/core/src/scheduler/auto-add-scheduler.test.ts new file mode 100644 index 0000000000..446cc9091b --- /dev/null +++ b/packages/core/src/scheduler/auto-add-scheduler.test.ts @@ -0,0 +1,241 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, afterEach, type Mocked } from 'vitest'; +import { updatePolicy } from './policy.js'; +import type { Config } from '../config/config.js'; +import type { MessageBus } from '../confirmation-bus/message-bus.js'; +import { MessageBusType } from '../confirmation-bus/types.js'; +import { + ToolConfirmationOutcome, + type AnyDeclarativeTool, + type ToolEditConfirmationDetails, +} from '../tools/tools.js'; +import { + READ_FILE_TOOL_NAME, + LS_TOOL_NAME, + WRITE_FILE_TOOL_NAME, + GLOB_TOOL_NAME, + GREP_TOOL_NAME, + READ_MANY_FILES_TOOL_NAME, + WEB_FETCH_TOOL_NAME, + WEB_SEARCH_TOOL_NAME, + WRITE_TODOS_TOOL_NAME, +} from '../tools/tool-names.js'; + +describe('Scheduler Auto-add Policy Logic', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('should set persist: true for ProceedAlways if autoAddPolicy is enabled', async () => { + const mockConfig = { + getAutoAddPolicy: vi.fn().mockReturnValue(true), + setApprovalMode: vi.fn(), + } as unknown as Mocked; + const mockMessageBus = { + publish: vi.fn(), + } as unknown as Mocked; + const tool = { + name: 'test-tool', + isSensitive: false, + } as AnyDeclarativeTool; + + await updatePolicy(tool, ToolConfirmationOutcome.ProceedAlways, undefined, { + config: mockConfig, + messageBus: mockMessageBus, + }); + + expect(mockMessageBus.publish).toHaveBeenCalledWith( + expect.objectContaining({ + type: MessageBusType.UPDATE_POLICY, + toolName: 'test-tool', + persist: true, + }), + ); + }); + + it('should set persist: false for ProceedAlways if autoAddPolicy is disabled', async () => { + const mockConfig = { + getAutoAddPolicy: vi.fn().mockReturnValue(false), + setApprovalMode: vi.fn(), + } as unknown as Mocked; + const mockMessageBus = { + publish: vi.fn(), + } as unknown as Mocked; + const tool = { + name: 'test-tool', + isSensitive: false, + } as AnyDeclarativeTool; + + await updatePolicy(tool, ToolConfirmationOutcome.ProceedAlways, undefined, { + config: mockConfig, + messageBus: mockMessageBus, + }); + + expect(mockMessageBus.publish).toHaveBeenCalledWith( + expect.objectContaining({ + type: MessageBusType.UPDATE_POLICY, + toolName: 'test-tool', + persist: false, + }), + ); + }); + + it('should generate specific argsPattern for edit tools', async () => { + const mockConfig = { + getAutoAddPolicy: vi.fn().mockReturnValue(true), + setApprovalMode: vi.fn(), + } as unknown as Mocked; + const mockMessageBus = { + publish: vi.fn(), + } as unknown as Mocked; + const tool = { + name: WRITE_FILE_TOOL_NAME, + isSensitive: true, + } as AnyDeclarativeTool; + const details: ToolEditConfirmationDetails = { + type: 'edit', + title: 'Confirm Write', + fileName: 'test.txt', + filePath: 'test.txt', + fileDiff: '', + originalContent: '', + newContent: '', + onConfirm: vi.fn(), + }; + + await updatePolicy(tool, ToolConfirmationOutcome.ProceedAlways, details, { + config: mockConfig, + messageBus: mockMessageBus, + }); + + expect(mockMessageBus.publish).toHaveBeenCalledWith( + expect.objectContaining({ + type: MessageBusType.UPDATE_POLICY, + argsPattern: expect.stringMatching(/test.*txt/), + isSensitive: true, + }), + ); + }); + + it.each([ + { + name: 'read_file', + tool: { + name: READ_FILE_TOOL_NAME, + params: { file_path: 'read.me' }, + }, + pattern: /read\\.me/, + }, + { + name: 'list_directory', + tool: { + name: LS_TOOL_NAME, + params: { dir_path: './src' }, + }, + pattern: /\.\/src/, + }, + { + name: 'glob', + tool: { + name: GLOB_TOOL_NAME, + params: { dir_path: './packages' }, + }, + pattern: /\.\/packages/, + }, + { + name: 'grep_search', + tool: { + name: GREP_TOOL_NAME, + params: { dir_path: './src' }, + }, + pattern: /\.\/src/, + }, + { + name: 'read_many_files', + tool: { + name: READ_MANY_FILES_TOOL_NAME, + params: { include: ['src/**/*.ts', 'test/'] }, + }, + pattern: /include.*src.*ts.*test/, + }, + { + name: 'web_fetch', + tool: { + name: WEB_FETCH_TOOL_NAME, + params: { prompt: 'Summarize https://example.com/page' }, + }, + pattern: /example\\.com/, + }, + { + name: 'web_fetch (direct url)', + tool: { + name: WEB_FETCH_TOOL_NAME, + params: { url: 'https://google.com' }, + }, + pattern: /"url":"https:\/\/google\\.com"/, + }, + { + name: 'web_search', + tool: { + name: WEB_SEARCH_TOOL_NAME, + params: { query: 'how to bake bread' }, + }, + pattern: /how\\ to\\ bake\\ bread/, + }, + { + name: 'write_todos', + tool: { + name: WRITE_TODOS_TOOL_NAME, + params: { todos: [{ description: 'fix the bug', status: 'pending' }] }, + }, + pattern: /fix\\ the\\ bug/, + }, + { + name: 'read_file (Windows path)', + tool: { + name: READ_FILE_TOOL_NAME, + params: { file_path: 'C:\\foo\\bar.txt' }, + }, + pattern: /C:\\\\\\\\foo\\\\\\\\bar\\.txt/, + }, + ])( + 'should generate specific argsPattern for $name', + async ({ tool, pattern }) => { + const toolWithSensitive = { + ...tool, + isSensitive: true, + } as unknown as AnyDeclarativeTool; + const mockConfig = { + getAutoAddPolicy: vi.fn().mockReturnValue(true), + setApprovalMode: vi.fn(), + } as unknown as Mocked; + const mockMessageBus = { + publish: vi.fn(), + } as unknown as Mocked; + + await updatePolicy( + toolWithSensitive, + ToolConfirmationOutcome.ProceedAlways, + undefined, + { + config: mockConfig, + messageBus: mockMessageBus, + }, + ); + + expect(mockMessageBus.publish).toHaveBeenCalledWith( + expect.objectContaining({ + type: MessageBusType.UPDATE_POLICY, + toolName: tool.name, + argsPattern: expect.stringMatching(pattern), + isSensitive: true, + }), + ); + }, + ); +}); diff --git a/packages/core/src/scheduler/policy.test.ts b/packages/core/src/scheduler/policy.test.ts index be79b7c62d..0a6f9ef2c1 100644 --- a/packages/core/src/scheduler/policy.test.ts +++ b/packages/core/src/scheduler/policy.test.ts @@ -150,13 +150,17 @@ describe('policy.ts', () => { describe('updatePolicy', () => { it('should set AUTO_EDIT mode for auto-edit transition tools', async () => { const mockConfig = { + getAutoAddPolicy: vi.fn().mockReturnValue(false), setApprovalMode: vi.fn(), } as unknown as Mocked; const mockMessageBus = { publish: vi.fn(), } as unknown as Mocked; - const tool = { name: 'replace' } as AnyDeclarativeTool; // 'replace' is in EDIT_TOOL_NAMES + const tool = { + name: 'replace', + isSensitive: false, + } as AnyDeclarativeTool; // 'replace' is in EDIT_TOOL_NAMES await updatePolicy( tool, @@ -168,17 +172,27 @@ describe('policy.ts', () => { expect(mockConfig.setApprovalMode).toHaveBeenCalledWith( ApprovalMode.AUTO_EDIT, ); - expect(mockMessageBus.publish).not.toHaveBeenCalled(); + expect(mockMessageBus.publish).toHaveBeenCalledWith( + expect.objectContaining({ + type: MessageBusType.UPDATE_POLICY, + toolName: 'replace', + persist: false, + }), + ); }); it('should handle standard policy updates (persist=false)', async () => { const mockConfig = { + getAutoAddPolicy: vi.fn().mockReturnValue(false), setApprovalMode: vi.fn(), } as unknown as Mocked; const mockMessageBus = { publish: vi.fn(), } as unknown as Mocked; - const tool = { name: 'test-tool' } as AnyDeclarativeTool; + const tool = { + name: 'test-tool', + isSensitive: false, + } as AnyDeclarativeTool; await updatePolicy( tool, @@ -198,12 +212,16 @@ describe('policy.ts', () => { it('should handle standard policy updates with persistence', async () => { const mockConfig = { + getAutoAddPolicy: vi.fn().mockReturnValue(false), setApprovalMode: vi.fn(), } as unknown as Mocked; const mockMessageBus = { publish: vi.fn(), } as unknown as Mocked; - const tool = { name: 'test-tool' } as AnyDeclarativeTool; + const tool = { + name: 'test-tool', + isSensitive: false, + } as AnyDeclarativeTool; await updatePolicy( tool, @@ -223,12 +241,16 @@ describe('policy.ts', () => { it('should handle shell command prefixes', async () => { const mockConfig = { + getAutoAddPolicy: vi.fn().mockReturnValue(false), setApprovalMode: vi.fn(), } as unknown as Mocked; const mockMessageBus = { publish: vi.fn(), } as unknown as Mocked; - const tool = { name: 'run_shell_command' } as AnyDeclarativeTool; + const tool = { + name: 'run_shell_command', + isSensitive: true, + } as AnyDeclarativeTool; const details: ToolExecuteConfirmationDetails = { type: 'exec', command: 'ls -la', @@ -254,12 +276,16 @@ describe('policy.ts', () => { it('should handle MCP policy updates (server scope)', async () => { const mockConfig = { + getAutoAddPolicy: vi.fn().mockReturnValue(false), setApprovalMode: vi.fn(), } as unknown as Mocked; const mockMessageBus = { publish: vi.fn(), } as unknown as Mocked; - const tool = { name: 'mcp-tool' } as AnyDeclarativeTool; + const tool = { + name: 'mcp-tool', + isSensitive: false, + } as AnyDeclarativeTool; const details: ToolMcpConfirmationDetails = { type: 'mcp', serverName: 'my-server', @@ -288,12 +314,16 @@ describe('policy.ts', () => { it('should NOT publish update for ProceedOnce', async () => { const mockConfig = { + getAutoAddPolicy: vi.fn().mockReturnValue(false), setApprovalMode: vi.fn(), } as unknown as Mocked; const mockMessageBus = { publish: vi.fn(), } as unknown as Mocked; - const tool = { name: 'test-tool' } as AnyDeclarativeTool; + const tool = { + name: 'test-tool', + isSensitive: false, + } as AnyDeclarativeTool; await updatePolicy(tool, ToolConfirmationOutcome.ProceedOnce, undefined, { config: mockConfig, @@ -306,12 +336,16 @@ describe('policy.ts', () => { it('should NOT publish update for Cancel', async () => { const mockConfig = { + getAutoAddPolicy: vi.fn().mockReturnValue(false), setApprovalMode: vi.fn(), } as unknown as Mocked; const mockMessageBus = { publish: vi.fn(), } as unknown as Mocked; - const tool = { name: 'test-tool' } as AnyDeclarativeTool; + const tool = { + name: 'test-tool', + isSensitive: false, + } as AnyDeclarativeTool; await updatePolicy(tool, ToolConfirmationOutcome.Cancel, undefined, { config: mockConfig, @@ -323,12 +357,16 @@ describe('policy.ts', () => { it('should NOT publish update for ModifyWithEditor', async () => { const mockConfig = { + getAutoAddPolicy: vi.fn().mockReturnValue(false), setApprovalMode: vi.fn(), } as unknown as Mocked; const mockMessageBus = { publish: vi.fn(), } as unknown as Mocked; - const tool = { name: 'test-tool' } as AnyDeclarativeTool; + const tool = { + name: 'test-tool', + isSensitive: false, + } as AnyDeclarativeTool; await updatePolicy( tool, @@ -342,12 +380,16 @@ describe('policy.ts', () => { it('should handle MCP ProceedAlwaysTool (specific tool name)', async () => { const mockConfig = { + getAutoAddPolicy: vi.fn().mockReturnValue(false), setApprovalMode: vi.fn(), } as unknown as Mocked; const mockMessageBus = { publish: vi.fn(), } as unknown as Mocked; - const tool = { name: 'mcp-tool' } as AnyDeclarativeTool; + const tool = { + name: 'mcp-tool', + isSensitive: false, + } as AnyDeclarativeTool; const details: ToolMcpConfirmationDetails = { type: 'mcp', serverName: 'my-server', @@ -376,12 +418,16 @@ describe('policy.ts', () => { it('should handle MCP ProceedAlways (persist: false)', async () => { const mockConfig = { + getAutoAddPolicy: vi.fn().mockReturnValue(false), setApprovalMode: vi.fn(), } as unknown as Mocked; const mockMessageBus = { publish: vi.fn(), } as unknown as Mocked; - const tool = { name: 'mcp-tool' } as AnyDeclarativeTool; + const tool = { + name: 'mcp-tool', + isSensitive: false, + } as AnyDeclarativeTool; const details: ToolMcpConfirmationDetails = { type: 'mcp', serverName: 'my-server', @@ -408,12 +454,16 @@ describe('policy.ts', () => { it('should handle MCP ProceedAlwaysAndSave (persist: true)', async () => { const mockConfig = { + getAutoAddPolicy: vi.fn().mockReturnValue(false), setApprovalMode: vi.fn(), } as unknown as Mocked; const mockMessageBus = { publish: vi.fn(), } as unknown as Mocked; - const tool = { name: 'mcp-tool' } as AnyDeclarativeTool; + const tool = { + name: 'mcp-tool', + isSensitive: false, + } as AnyDeclarativeTool; const details: ToolMcpConfirmationDetails = { type: 'mcp', serverName: 'my-server', @@ -436,6 +486,80 @@ describe('policy.ts', () => { toolName: 'mcp-tool', mcpName: 'my-server', persist: true, + argsPattern: '\\{.*\\}', // Fix verified: specific pattern provided for auto-add + }), + ); + }); + + it('should NOT provide argsPattern for server-wide MCP approvals', async () => { + const mockConfig = { + getAutoAddPolicy: vi.fn().mockReturnValue(true), + setApprovalMode: vi.fn(), + } as unknown as Mocked; + const mockMessageBus = { + publish: vi.fn(), + } as unknown as Mocked; + const tool = { + name: 'mcp-tool', + isSensitive: false, + } as AnyDeclarativeTool; + const details: ToolMcpConfirmationDetails = { + type: 'mcp', + serverName: 'my-server', + toolName: 'mcp-tool', + toolDisplayName: 'My Tool', + title: 'MCP', + onConfirm: vi.fn(), + }; + + await updatePolicy( + tool, + ToolConfirmationOutcome.ProceedAlwaysServer, + details, + { config: mockConfig, messageBus: mockMessageBus }, + ); + + expect(mockMessageBus.publish).toHaveBeenCalledWith( + expect.objectContaining({ + type: MessageBusType.UPDATE_POLICY, + toolName: 'my-server__*', + mcpName: 'my-server', + persist: false, // Server-wide approvals currently do not auto-persist for safety + argsPattern: undefined, // Server-wide approvals are intentionally not specific + }), + ); + }); + + it('should handle specificity for read_many_files', async () => { + const mockConfig = { + getAutoAddPolicy: vi.fn().mockReturnValue(true), + setApprovalMode: vi.fn(), + } as unknown as Mocked; + const mockMessageBus = { + publish: vi.fn(), + } as unknown as Mocked; + const tool = { + name: 'read_many_files', + isSensitive: true, + params: { include: ['file1.ts', 'file2.ts'] }, + } as unknown as AnyDeclarativeTool; + + await updatePolicy( + tool, + ToolConfirmationOutcome.ProceedAlways, + undefined, + { + config: mockConfig, + messageBus: mockMessageBus, + }, + ); + + expect(mockMessageBus.publish).toHaveBeenCalledWith( + expect.objectContaining({ + type: MessageBusType.UPDATE_POLICY, + toolName: 'read_many_files', + persist: true, + argsPattern: expect.stringContaining('file1\\.ts'), }), ); }); diff --git a/packages/core/src/scheduler/policy.ts b/packages/core/src/scheduler/policy.ts index ad4aa745bb..8234b302d6 100644 --- a/packages/core/src/scheduler/policy.ts +++ b/packages/core/src/scheduler/policy.ts @@ -4,7 +4,12 @@ * SPDX-License-Identifier: Apache-2.0 */ +import { escapeJsonRegex } from '../policy/utils.js'; import { ToolErrorType } from '../tools/tool-error.js'; +import { + MCP_QUALIFIED_NAME_SEPARATOR, + DiscoveredMCPTool, +} from '../tools/mcp-tool.js'; import { ApprovalMode, PolicyDecision, @@ -22,10 +27,37 @@ import { type AnyDeclarativeTool, type PolicyUpdateOptions, } from '../tools/tools.js'; -import { DiscoveredMCPTool } from '../tools/mcp-tool.js'; -import { EDIT_TOOL_NAMES } from '../tools/tool-names.js'; +import { + EDIT_TOOL_NAMES, + READ_FILE_TOOL_NAME, + LS_TOOL_NAME, + GLOB_TOOL_NAME, + GREP_TOOL_NAME, + READ_MANY_FILES_TOOL_NAME, + WEB_FETCH_TOOL_NAME, + WEB_SEARCH_TOOL_NAME, + WRITE_TODOS_TOOL_NAME, + GET_INTERNAL_DOCS_TOOL_NAME, +} from '../tools/tool-names.js'; import type { ValidatingToolCall } from './types.js'; +interface ToolWithParams { + params: Record; +} + +function hasParams( + tool: AnyDeclarativeTool, +): tool is AnyDeclarativeTool & ToolWithParams { + const t = tool as unknown; + return ( + typeof t === 'object' && + t !== null && + 'params' in t && + typeof (t as { params: unknown }).params === 'object' && + (t as { params: unknown }).params !== null + ); +} + /** * Helper to format the policy denial error. */ @@ -99,7 +131,6 @@ export async function updatePolicy( // Mode Transitions (AUTO_EDIT) if (isAutoEditTransition(tool, outcome)) { deps.config.setApprovalMode(ApprovalMode.AUTO_EDIT); - return; } // Specialized Tools (MCP) @@ -108,6 +139,7 @@ export async function updatePolicy( tool, outcome, confirmationDetails, + deps.config, deps.messageBus, ); return; @@ -118,6 +150,7 @@ export async function updatePolicy( tool, outcome, confirmationDetails, + deps.config, deps.messageBus, ); } @@ -139,6 +172,99 @@ function isAutoEditTransition( ); } +type SpecificityGenerator = ( + tool: AnyDeclarativeTool, + confirmationDetails?: SerializableConfirmationDetails, +) => string | undefined; + +const specificityGenerators: Record = { + [READ_FILE_TOOL_NAME]: (tool) => { + if (!hasParams(tool)) return undefined; + const filePath = tool.params['file_path']; + if (typeof filePath !== 'string') return undefined; + const escapedPath = escapeJsonRegex(filePath); + return `.*"file_path":"${escapedPath}".*`; + }, + [LS_TOOL_NAME]: (tool) => { + if (!hasParams(tool)) return undefined; + const dirPath = tool.params['dir_path']; + if (typeof dirPath !== 'string') return undefined; + const escapedPath = escapeJsonRegex(dirPath); + return `.*"dir_path":"${escapedPath}".*`; + }, + [GLOB_TOOL_NAME]: (tool) => specificityGenerators[LS_TOOL_NAME](tool), + [GREP_TOOL_NAME]: (tool) => specificityGenerators[LS_TOOL_NAME](tool), + [READ_MANY_FILES_TOOL_NAME]: (tool) => { + if (!hasParams(tool)) return undefined; + const include = tool.params['include']; + if (!Array.isArray(include) || include.length === 0) return undefined; + const lookaheads = include + .map((p) => escapeJsonRegex(String(p))) + .map((p) => `(?=.*"${p}")`) + .join(''); + const pattern = `.*"include":\\[${lookaheads}.*\\].*`; + + // Limit regex length for safety + if (pattern.length > 2048) { + return '.*"include":\\[.*\\].*'; + } + + return pattern; + }, + [WEB_FETCH_TOOL_NAME]: (tool) => { + if (!hasParams(tool)) return undefined; + const url = tool.params['url']; + if (typeof url === 'string') { + const escaped = escapeJsonRegex(url); + return `.*"url":"${escaped}".*`; + } + + const prompt = tool.params['prompt']; + if (typeof prompt !== 'string') return undefined; + const urlMatches = prompt.matchAll(/https?:\/\/[^\s"']+/g); + const urls = Array.from(urlMatches) + .map((m) => m[0]) + .slice(0, 3); + if (urls.length === 0) return undefined; + const lookaheads = urls + .map((u) => escapeJsonRegex(u)) + .map((u) => `(?=.*${u})`) + .join(''); + return `.*${lookaheads}.*`; + }, + [WEB_SEARCH_TOOL_NAME]: (tool) => { + if (!hasParams(tool)) return undefined; + const query = tool.params['query']; + if (typeof query === 'string') { + const escaped = escapeJsonRegex(query); + return `.*"query":"${escaped}".*`; + } + // Fallback to a pattern that matches any arguments + // but isn't just ".*" to satisfy the auto-add safeguard. + return '\\{.*\\}'; + }, + [WRITE_TODOS_TOOL_NAME]: (tool) => { + if (!hasParams(tool)) return undefined; + const todos = tool.params['todos']; + if (!Array.isArray(todos)) return undefined; + const escaped = todos + .filter( + (v): v is { description: string } => typeof v?.description === 'string', + ) + .map((v) => escapeJsonRegex(v.description)) + .join('|'); + if (!escaped) return undefined; + return `.*"todos":\\[.*(?:${escaped}).*\\].*`; + }, + [GET_INTERNAL_DOCS_TOOL_NAME]: (tool) => { + if (!hasParams(tool)) return undefined; + const filePath = tool.params['file_path']; + if (typeof filePath !== 'string') return undefined; + const escaped = escapeJsonRegex(filePath); + return `.*"file_path":"${escaped}".*`; + }, +}; + /** * Handles policy updates for standard tools (Shell, Info, etc.), including * session-level and persistent approvals. @@ -147,6 +273,7 @@ async function handleStandardPolicyUpdate( tool: AnyDeclarativeTool, outcome: ToolConfirmationOutcome, confirmationDetails: SerializableConfirmationDetails | undefined, + config: Config, messageBus: MessageBus, ): Promise { if ( @@ -159,10 +286,27 @@ async function handleStandardPolicyUpdate( options.commandPrefix = confirmationDetails.rootCommands; } + if (confirmationDetails?.type === 'edit') { + // Generate a specific argsPattern for file edits to prevent broad approvals + const escapedPath = escapeJsonRegex(confirmationDetails.filePath); + options.argsPattern = `.*"file_path":"${escapedPath}".*`; + } else { + const generator = specificityGenerators[tool.name]; + if (generator) { + options.argsPattern = generator(tool, confirmationDetails); + } + } + + const persist = + outcome === ToolConfirmationOutcome.ProceedAlwaysAndSave || + (outcome === ToolConfirmationOutcome.ProceedAlways && + config.getAutoAddPolicy()); + await messageBus.publish({ type: MessageBusType.UPDATE_POLICY, toolName: tool.name, - persist: outcome === ToolConfirmationOutcome.ProceedAlwaysAndSave, + persist, + isSensitive: tool.isSensitive, ...options, }); } @@ -179,6 +323,7 @@ async function handleMcpPolicyUpdate( SerializableConfirmationDetails, { type: 'mcp' } >, + config: Config, messageBus: MessageBus, ): Promise { const isMcpAlways = @@ -192,17 +337,31 @@ async function handleMcpPolicyUpdate( } let toolName = tool.name; - const persist = outcome === ToolConfirmationOutcome.ProceedAlwaysAndSave; + const persist = + outcome === ToolConfirmationOutcome.ProceedAlwaysAndSave || + (outcome === ToolConfirmationOutcome.ProceedAlways && + config.getAutoAddPolicy()); // If "Always allow all tools from this server", use the wildcard pattern if (outcome === ToolConfirmationOutcome.ProceedAlwaysServer) { - toolName = `${confirmationDetails.serverName}__*`; + toolName = `${confirmationDetails.serverName}${MCP_QUALIFIED_NAME_SEPARATOR}*`; } + // MCP tools are treated as sensitive, so we MUST provide a specific argsPattern + // or commandPrefix to satisfy the auto-add safeguard in createPolicyUpdater. + // For single-tool approvals, we default to a pattern that matches the JSON structure + // of the arguments string (e.g. \{.*\}). + const argsPattern = + outcome !== ToolConfirmationOutcome.ProceedAlwaysServer + ? '\\{.*\\}' + : undefined; + await messageBus.publish({ type: MessageBusType.UPDATE_POLICY, toolName, mcpName: confirmationDetails.serverName, persist, + isSensitive: tool.isSensitive, + argsPattern, }); } diff --git a/schemas/settings.schema.json b/schemas/settings.schema.json index 51bf9c84e2..426c52c88e 100644 --- a/schemas/settings.schema.json +++ b/schemas/settings.schema.json @@ -1255,6 +1255,13 @@ "markdownDescription": "Sandbox execution environment. Set to a boolean to enable or disable the sandbox, or provide a string path to a sandbox profile.\n\n- Category: `Tools`\n- Requires restart: `yes`", "$ref": "#/$defs/BooleanOrString" }, + "sandboxFlags": { + "title": "Sandbox Flags", + "description": "Additional flags to pass to the sandbox container engine (Docker or Podman). Environment variables can be used and will be expanded.", + "markdownDescription": "Additional flags to pass to the sandbox container engine (Docker or Podman). Environment variables can be used and will be expanded.\n\n- Category: `Tools`\n- Requires restart: `yes`\n- Default: ``", + "default": "", + "type": "string" + }, "shell": { "title": "Shell", "description": "Settings for shell execution.", @@ -1425,6 +1432,13 @@ "default": false, "type": "boolean" }, + "autoAddPolicy": { + "title": "Auto-add to Policy", + "description": "Automatically add \"Proceed always\" approvals to your persistent policy.", + "markdownDescription": "Automatically add \"Proceed always\" approvals to your persistent policy.\n\n- Category: `Security`\n- Requires restart: `no`\n- Default: `true`", + "default": true, + "type": "boolean" + }, "blockGitExtensions": { "title": "Blocks extensions from Git", "description": "Blocks installing and loading extensions from Git.",