From b96a26b4efb1e3f6473dc9d652c434a39ceaeb05 Mon Sep 17 00:00:00 2001 From: Keith Schaab Date: Mon, 6 Apr 2026 15:44:51 +0000 Subject: [PATCH] Requiring mode when injecting policy to prevent allowing all modes by default --- .../browser/browserAgentFactory.test.ts | 12 +- .../src/agents/browser/browserAgentFactory.ts | 6 +- packages/core/src/agents/registry.test.ts | 5 +- packages/core/src/agents/registry.ts | 7 +- packages/core/src/policy/config.ts | 12 +- .../core/src/policy/policies/conseca.toml | 1 + .../core/src/policy/policies/discovered.toml | 2 + .../src/policy/policies/memory-manager.toml | 2 + .../src/policy/policies/non-interactive.toml | 1 + packages/core/src/policy/policies/plan.toml | 4 + .../core/src/policy/policies/read-only.toml | 10 +- .../core/src/policy/policies/tracker.toml | 1 + packages/core/src/policy/policies/write.toml | 13 +- .../core/src/policy/policy-engine.test.ts | 391 +++++++++++++++--- packages/core/src/policy/policy-engine.ts | 7 +- packages/core/src/policy/shell-safety.test.ts | 19 +- packages/core/src/policy/toml-loader.test.ts | 105 ++++- packages/core/src/policy/toml-loader.ts | 6 +- packages/core/src/policy/types.ts | 6 +- packages/core/src/scheduler/policy.test.ts | 7 +- packages/core/src/scheduler/scheduler.test.ts | 14 +- packages/core/src/services/memoryService.ts | 3 +- .../core/src/utils/extensionLoader.test.ts | 4 +- packages/core/src/utils/googleErrors.ts | 2 +- 24 files changed, 558 insertions(+), 82 deletions(-) diff --git a/packages/core/src/agents/browser/browserAgentFactory.test.ts b/packages/core/src/agents/browser/browserAgentFactory.test.ts index 79e38c5361..c43301025d 100644 --- a/packages/core/src/agents/browser/browserAgentFactory.test.ts +++ b/packages/core/src/agents/browser/browserAgentFactory.test.ts @@ -11,7 +11,11 @@ import { } from './browserAgentFactory.js'; import { injectAutomationOverlay } from './automationOverlay.js'; import { makeFakeConfig } from '../../test-utils/config.js'; -import { PolicyDecision, PRIORITY_SUBAGENT_TOOL } from '../../policy/types.js'; +import { + PolicyDecision, + PRIORITY_SUBAGENT_TOOL, + MODES_BY_PERMISSIVENESS, +} from '../../policy/types.js'; import type { Config } from '../../config/config.js'; import type { MessageBus } from '../../confirmation-bus/message-bus.js'; import type { PolicyEngine } from '../../policy/policy-engine.js'; @@ -372,6 +376,7 @@ describe('browserAgentFactory', () => { toolName: 'mcp_browser_agent_fill', decision: PolicyDecision.ASK_USER, priority: 999, + modes: MODES_BY_PERMISSIVENESS, }), ); @@ -380,6 +385,7 @@ describe('browserAgentFactory', () => { toolName: 'mcp_browser_agent_upload_file', decision: PolicyDecision.ASK_USER, priority: 999, + modes: MODES_BY_PERMISSIVENESS, }), ); @@ -388,6 +394,7 @@ describe('browserAgentFactory', () => { toolName: 'mcp_browser_agent_evaluate_script', decision: PolicyDecision.ASK_USER, priority: 999, + modes: MODES_BY_PERMISSIVENESS, }), ); }); @@ -432,6 +439,7 @@ describe('browserAgentFactory', () => { toolName: 'mcp_browser_agent_take_snapshot', decision: PolicyDecision.ALLOW, priority: PRIORITY_SUBAGENT_TOOL, + modes: MODES_BY_PERMISSIVENESS, }), ); @@ -440,6 +448,7 @@ describe('browserAgentFactory', () => { toolName: 'mcp_browser_agent_take_screenshot', decision: PolicyDecision.ALLOW, priority: PRIORITY_SUBAGENT_TOOL, + modes: MODES_BY_PERMISSIVENESS, }), ); @@ -448,6 +457,7 @@ describe('browserAgentFactory', () => { toolName: 'mcp_browser_agent_list_pages', decision: PolicyDecision.ALLOW, priority: PRIORITY_SUBAGENT_TOOL, + modes: MODES_BY_PERMISSIVENESS, }), ); }); diff --git a/packages/core/src/agents/browser/browserAgentFactory.ts b/packages/core/src/agents/browser/browserAgentFactory.ts index a1f34a127d..15886be6bb 100644 --- a/packages/core/src/agents/browser/browserAgentFactory.ts +++ b/packages/core/src/agents/browser/browserAgentFactory.ts @@ -36,6 +36,7 @@ import { PolicyDecision, PRIORITY_SUBAGENT_TOOL, type PolicyRule, + MODES_BY_PERMISSIVENESS, } from '../../policy/types.js'; /** @@ -134,6 +135,7 @@ export async function createBrowserAgentDefinition( toolName: `${MCP_TOOL_PREFIX}${BROWSER_AGENT_NAME}_${toolName}`, decision: PolicyDecision.ASK_USER, priority: 999, + modes: MODES_BY_PERMISSIVENESS, source: 'BrowserAgent (Sensitive Actions)', mcpName: BROWSER_AGENT_NAME, }; @@ -144,6 +146,7 @@ export async function createBrowserAgentDefinition( toolName: `${MCP_TOOL_PREFIX}${BROWSER_AGENT_NAME}_${toolName}`, decision: PolicyDecision.ALLOW, priority: PRIORITY_SUBAGENT_TOOL, + modes: MODES_BY_PERMISSIVENESS, source: 'BrowserAgent (Read-Only)', mcpName: BROWSER_AGENT_NAME, }; @@ -155,7 +158,8 @@ export async function createBrowserAgentDefinition( rule1.toolName === rule2.toolName && rule1.decision === rule2.decision && rule1.priority === rule2.priority && - rule1.mcpName === rule2.mcpName + rule1.mcpName === rule2.mcpName && + JSON.stringify(rule1.modes) === JSON.stringify(rule2.modes) ); } diff --git a/packages/core/src/agents/registry.test.ts b/packages/core/src/agents/registry.test.ts index 55517a20d5..0f7d080cab 100644 --- a/packages/core/src/agents/registry.test.ts +++ b/packages/core/src/agents/registry.test.ts @@ -29,7 +29,7 @@ import { SimpleExtensionLoader } from '../utils/extensionLoader.js'; import type { ToolRegistry } from '../tools/tool-registry.js'; import { ThinkingLevel } from '@google/genai'; import type { AcknowledgedAgentsService } from './acknowledgedAgents.js'; -import { PolicyDecision } from '../policy/types.js'; +import { PolicyDecision, MODES_BY_PERMISSIVENESS } from '../policy/types.js'; import { A2AAuthProviderFactory } from './auth-provider/factory.js'; import type { A2AAuthProvider } from './auth-provider/types.js'; @@ -1076,6 +1076,7 @@ describe('AgentRegistry', () => { toolName: 'PolicyTestAgent', decision: PolicyDecision.ALLOW, priority: 1.05, + modes: MODES_BY_PERMISSIVENESS, }), ); }); @@ -1103,6 +1104,7 @@ describe('AgentRegistry', () => { toolName: 'RemotePolicyAgent', decision: PolicyDecision.ASK_USER, priority: 1.05, + modes: MODES_BY_PERMISSIVENESS, }), ); }); @@ -1165,6 +1167,7 @@ describe('AgentRegistry', () => { expect.objectContaining({ toolName: 'OverwrittenAgent', decision: PolicyDecision.ASK_USER, + modes: MODES_BY_PERMISSIVENESS, }), ); }); diff --git a/packages/core/src/agents/registry.ts b/packages/core/src/agents/registry.ts index 7ff547fba9..fd3443af95 100644 --- a/packages/core/src/agents/registry.ts +++ b/packages/core/src/agents/registry.ts @@ -25,7 +25,11 @@ import { type ModelConfig, ModelConfigService, } from '../services/modelConfigService.js'; -import { PolicyDecision, PRIORITY_SUBAGENT_TOOL } from '../policy/types.js'; +import { + PolicyDecision, + PRIORITY_SUBAGENT_TOOL, + MODES_BY_PERMISSIVENESS, +} from '../policy/types.js'; import { A2AAgentError, AgentAuthConfigMissingError } from './a2a-errors.js'; /** @@ -388,6 +392,7 @@ export class AgentRegistry { ? PolicyDecision.ALLOW : PolicyDecision.ASK_USER, priority: PRIORITY_SUBAGENT_TOOL, + modes: MODES_BY_PERMISSIVENESS, source: 'AgentRegistry (Dynamic)', }); } diff --git a/packages/core/src/policy/config.ts b/packages/core/src/policy/config.ts index 9147a66a9d..b1ec622d9f 100644 --- a/packages/core/src/policy/config.ts +++ b/packages/core/src/policy/config.ts @@ -11,6 +11,7 @@ import { fileURLToPath } from 'node:url'; import { Storage } from '../config/storage.js'; import { ApprovalMode, + MODES_BY_PERMISSIVENESS, type PolicyEngineConfig, PolicyDecision, type PolicyRule, @@ -415,6 +416,7 @@ export async function createPolicyEngineConfig( mcpName: serverName, decision: PolicyDecision.DENY, priority: MCP_EXCLUDED_PRIORITY, + modes: MODES_BY_PERMISSIVENESS, source: 'Settings (MCP Excluded)', }); } @@ -428,6 +430,7 @@ export async function createPolicyEngineConfig( toolName: tool, decision: PolicyDecision.DENY, priority: EXCLUDE_TOOLS_FLAG_PRIORITY, + modes: MODES_BY_PERMISSIVENESS, source: 'Settings (Tools Excluded)', }); } @@ -456,6 +459,7 @@ export async function createPolicyEngineConfig( decision: PolicyDecision.ALLOW, priority: ALLOWED_TOOLS_FLAG_PRIORITY, argsPattern: new RegExp(pattern), + modes: MODES_BY_PERMISSIVENESS, source: 'Settings (Tools Allowed)', }); } @@ -467,6 +471,7 @@ export async function createPolicyEngineConfig( toolName, decision: PolicyDecision.ALLOW, priority: ALLOWED_TOOLS_FLAG_PRIORITY, + modes: MODES_BY_PERMISSIVENESS, source: 'Settings (Tools Allowed)', }); } @@ -479,6 +484,7 @@ export async function createPolicyEngineConfig( toolName, decision: PolicyDecision.ALLOW, priority: ALLOWED_TOOLS_FLAG_PRIORITY, + modes: MODES_BY_PERMISSIVENESS, source: 'Settings (Tools Allowed)', }); } @@ -499,6 +505,7 @@ export async function createPolicyEngineConfig( mcpName: serverName, decision: PolicyDecision.ALLOW, priority: TRUSTED_MCP_SERVER_PRIORITY, + modes: MODES_BY_PERMISSIVENESS, source: 'Settings (MCP Trusted)', }); } @@ -517,6 +524,7 @@ export async function createPolicyEngineConfig( mcpName: serverName, decision: PolicyDecision.ALLOW, priority: ALLOWED_MCP_SERVER_PRIORITY, + modes: MODES_BY_PERMISSIVENESS, source: 'Settings (MCP Allowed)', }); } @@ -638,7 +646,7 @@ export function createPolicyUpdater( priority, argsPattern: new RegExp(pattern), mcpName: message.mcpName, - modes: message.modes, + modes: message.modes ?? MODES_BY_PERMISSIVENESS, source: 'Dynamic (Confirmed)', allowRedirection: message.allowRedirection, }); @@ -676,7 +684,7 @@ export function createPolicyUpdater( priority, argsPattern, mcpName: message.mcpName, - modes: message.modes, + modes: message.modes ?? MODES_BY_PERMISSIVENESS, source: 'Dynamic (Confirmed)', allowRedirection: message.allowRedirection, }); diff --git a/packages/core/src/policy/policies/conseca.toml b/packages/core/src/policy/policies/conseca.toml index 48c7e1b1c3..ad0044703a 100644 --- a/packages/core/src/policy/policies/conseca.toml +++ b/packages/core/src/policy/policies/conseca.toml @@ -1,6 +1,7 @@ [[safety_checker]] toolName = "*" priority = 100 +modes = ["plan", "default", "autoEdit", "yolo"] [safety_checker.checker] type = "in-process" name = "conseca" diff --git a/packages/core/src/policy/policies/discovered.toml b/packages/core/src/policy/policies/discovered.toml index 41ebe8124e..2448ac00c8 100644 --- a/packages/core/src/policy/policies/discovered.toml +++ b/packages/core/src/policy/policies/discovered.toml @@ -6,10 +6,12 @@ toolName = "discovered_tool_*" decision = "ask_user" priority = 10 +modes = ["plan", "default", "autoEdit", "yolo"] interactive = true [[rule]] toolName = "discovered_tool_*" decision = "deny" priority = 10 +modes = ["plan", "default", "autoEdit", "yolo"] interactive = false diff --git a/packages/core/src/policy/policies/memory-manager.toml b/packages/core/src/policy/policies/memory-manager.toml index 3794871be3..fd57c95608 100644 --- a/packages/core/src/policy/policies/memory-manager.toml +++ b/packages/core/src/policy/policies/memory-manager.toml @@ -7,6 +7,7 @@ subagent = "save_memory" toolName = ["read_file", "list_directory", "glob", "grep_search"] decision = "allow" priority = 100 +modes = ["plan", "default", "autoEdit", "yolo"] argsPattern = "(^|.*/)\\.gemini/.*" denyMessage = "Memory Manager is only allowed to access the .gemini folder." @@ -16,5 +17,6 @@ subagent = "save_memory" toolName = ["write_file", "replace"] decision = "allow" priority = 100 +modes = ["plan", "default", "autoEdit", "yolo"] argsPattern = "(^|.*/)\\.gemini/.*\\.md\"" denyMessage = "Memory Manager is only allowed to write .md files in the .gemini folder." diff --git a/packages/core/src/policy/policies/non-interactive.toml b/packages/core/src/policy/policies/non-interactive.toml index 04c41f6eb1..92d61bc3f4 100644 --- a/packages/core/src/policy/policies/non-interactive.toml +++ b/packages/core/src/policy/policies/non-interactive.toml @@ -4,4 +4,5 @@ toolName = "ask_user" decision = "deny" priority = 999 +modes = ["plan", "default", "autoEdit", "yolo"] interactive = false diff --git a/packages/core/src/policy/policies/plan.toml b/packages/core/src/policy/policies/plan.toml index e7a64e0748..d77bf7c7b4 100644 --- a/packages/core/src/policy/policies/plan.toml +++ b/packages/core/src/policy/policies/plan.toml @@ -33,12 +33,14 @@ toolName = "enter_plan_mode" decision = "ask_user" priority = 50 +modes = ["plan"] interactive = true [[rule]] toolName = "enter_plan_mode" decision = "allow" priority = 50 +modes = ["plan"] interactive = false [[rule]] @@ -59,12 +61,14 @@ interactive = true toolName = "exit_plan_mode" decision = "allow" priority = 70 +modes = ["plan"] interactive = false [[rule]] toolName = "exit_plan_mode" decision = "deny" priority = 50 +modes = ["plan"] denyMessage = "You are not currently in Plan Mode. Use enter_plan_mode first to design a plan." diff --git a/packages/core/src/policy/policies/read-only.toml b/packages/core/src/policy/policies/read-only.toml index c56984b522..1f77eb2d7f 100644 --- a/packages/core/src/policy/policies/read-only.toml +++ b/packages/core/src/policy/policies/read-only.toml @@ -31,40 +31,48 @@ toolName = "glob" decision = "allow" priority = 50 +modes = ["plan", "default", "autoEdit", "yolo"] [[rule]] toolName = "grep_search" decision = "allow" priority = 50 +modes = ["plan", "default", "autoEdit", "yolo"] [[rule]] toolName = "list_directory" decision = "allow" priority = 50 +modes = ["plan", "default", "autoEdit", "yolo"] [[rule]] toolName = "read_file" decision = "allow" priority = 50 +modes = ["plan", "default", "autoEdit", "yolo"] [[rule]] toolName = "google_web_search" decision = "allow" priority = 50 +modes = ["plan", "default", "autoEdit", "yolo"] [[rule]] toolName = ["codebase_investigator", "cli_help", "get_internal_docs"] decision = "allow" priority = 50 +modes = ["plan", "default", "autoEdit", "yolo"] # Topic grouping tool is innocuous and used for UI organization. [[rule]] toolName = "update_topic" decision = "allow" priority = 50 +modes = ["plan", "default", "autoEdit", "yolo"] # Core agent lifecycle tool [[rule]] toolName = "complete_task" decision = "allow" -priority = 50 \ No newline at end of file +priority = 50 +modes = ["plan", "default", "autoEdit", "yolo"] diff --git a/packages/core/src/policy/policies/tracker.toml b/packages/core/src/policy/policies/tracker.toml index e17c4fc387..aaf8f71ff5 100644 --- a/packages/core/src/policy/policies/tracker.toml +++ b/packages/core/src/policy/policies/tracker.toml @@ -32,3 +32,4 @@ toolName = [ ] decision = "allow" priority = 50 +modes = ["plan", "default", "autoEdit", "yolo"] diff --git a/packages/core/src/policy/policies/write.toml b/packages/core/src/policy/policies/write.toml index 55ffd8c54f..8ebc04749a 100644 --- a/packages/core/src/policy/policies/write.toml +++ b/packages/core/src/policy/policies/write.toml @@ -31,13 +31,14 @@ toolName = "replace" decision = "ask_user" priority = 10 +modes = ["plan", "default", "autoEdit", "yolo"] interactive = true [[rule]] toolName = "replace" decision = "allow" priority = 15 -modes = ["autoEdit"] +modes = ["plan", "default", "autoEdit", "yolo"] [rule.safety_checker] type = "in-process" @@ -48,31 +49,35 @@ required_context = ["environment"] toolName = "save_memory" decision = "ask_user" priority = 10 +modes = ["plan", "default", "autoEdit", "yolo"] interactive = true [[rule]] toolName = "run_shell_command" decision = "ask_user" priority = 10 +modes = ["plan", "default", "autoEdit", "yolo"] interactive = true [[rule]] toolName = "write_file" decision = "ask_user" priority = 10 +modes = ["plan", "default", "autoEdit", "yolo"] interactive = true [[rule]] toolName = "activate_skill" decision = "ask_user" priority = 10 +modes = ["plan", "default", "autoEdit", "yolo"] interactive = true [[rule]] toolName = "write_file" decision = "allow" priority = 15 -modes = ["autoEdit"] +modes = ["plan", "default", "autoEdit", "yolo"] [rule.safety_checker] type = "in-process" @@ -83,12 +88,13 @@ required_context = ["environment"] toolName = "web_fetch" decision = "allow" priority = 15 -modes = ["autoEdit"] +modes = ["plan", "default", "autoEdit", "yolo"] [[rule]] toolName = "web_fetch" decision = "ask_user" priority = 10 +modes = ["plan", "default", "autoEdit", "yolo"] interactive = true # Headless Denial Rule (Priority 10) @@ -104,4 +110,5 @@ toolName = [ ] decision = "deny" priority = 10 +modes = ["plan", "default", "autoEdit", "yolo"] interactive = false diff --git a/packages/core/src/policy/policy-engine.test.ts b/packages/core/src/policy/policy-engine.test.ts index 0299000f73..33101a61ee 100644 --- a/packages/core/src/policy/policy-engine.test.ts +++ b/packages/core/src/policy/policy-engine.test.ts @@ -16,6 +16,7 @@ import { PRIORITY_SUBAGENT_TOOL, ALWAYS_ALLOW_PRIORITY_FRACTION, PRIORITY_YOLO_ALLOW_ALL, + MODES_BY_PERMISSIVENESS, } from './types.js'; import type { FunctionCall } from '@google/genai'; import { SafetyCheckDecision } from '../safety/protocol.js'; @@ -123,9 +124,24 @@ describe('PolicyEngine', () => { it('should sort rules by priority', () => { const rules: PolicyRule[] = [ - { toolName: 'tool1', decision: PolicyDecision.DENY, priority: 1 }, - { toolName: 'tool2', decision: PolicyDecision.ALLOW, priority: 10 }, - { toolName: 'tool3', decision: PolicyDecision.ASK_USER, priority: 5 }, + { + toolName: 'tool1', + decision: PolicyDecision.DENY, + priority: 1, + modes: MODES_BY_PERMISSIVENESS, + }, + { + toolName: 'tool2', + decision: PolicyDecision.ALLOW, + priority: 10, + modes: MODES_BY_PERMISSIVENESS, + }, + { + toolName: 'tool3', + decision: PolicyDecision.ASK_USER, + priority: 5, + modes: MODES_BY_PERMISSIVENESS, + }, ]; engine = new PolicyEngine({ rules }); @@ -140,8 +156,16 @@ describe('PolicyEngine', () => { describe('check', () => { it('should match tool by name', async () => { const rules: PolicyRule[] = [ - { toolName: 'shell', decision: PolicyDecision.ALLOW }, - { toolName: 'edit', decision: PolicyDecision.DENY }, + { + toolName: 'shell', + decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, + }, + { + toolName: 'edit', + decision: PolicyDecision.DENY, + modes: MODES_BY_PERMISSIVENESS, + }, ]; engine = new PolicyEngine({ rules }); @@ -163,6 +187,7 @@ describe('PolicyEngine', () => { toolName: 'mcp_my-server_tool', mcpName: 'my-server', decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -186,10 +211,12 @@ describe('PolicyEngine', () => { toolName: 'shell', argsPattern: /rm -rf/, decision: PolicyDecision.DENY, + modes: MODES_BY_PERMISSIVENESS, }, { toolName: 'shell', decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -215,8 +242,18 @@ describe('PolicyEngine', () => { it('should apply rules by priority', async () => { const rules: PolicyRule[] = [ - { toolName: 'shell', decision: PolicyDecision.DENY, priority: 1 }, - { toolName: 'shell', decision: PolicyDecision.ALLOW, priority: 10 }, + { + toolName: 'shell', + decision: PolicyDecision.DENY, + priority: 1, + modes: MODES_BY_PERMISSIVENESS, + }, + { + toolName: 'shell', + decision: PolicyDecision.ALLOW, + priority: 10, + modes: MODES_BY_PERMISSIVENESS, + }, ]; engine = new PolicyEngine({ rules }); @@ -232,7 +269,11 @@ describe('PolicyEngine', () => { const currentName = 'current_test_tool'; const rules: PolicyRule[] = [ - { toolName: legacyName, decision: PolicyDecision.DENY }, + { + toolName: legacyName, + decision: PolicyDecision.DENY, + modes: MODES_BY_PERMISSIVENESS, + }, ]; engine = new PolicyEngine({ rules }); @@ -247,7 +288,11 @@ describe('PolicyEngine', () => { const currentName = 'current_test_tool'; const rules: PolicyRule[] = [ - { toolName: currentName, decision: PolicyDecision.ALLOW }, + { + toolName: currentName, + decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, + }, ]; engine = new PolicyEngine({ rules }); @@ -262,7 +307,11 @@ describe('PolicyEngine', () => { const legacyName2 = 'another_legacy_test_tool'; const rules: PolicyRule[] = [ - { toolName: legacyName2, decision: PolicyDecision.DENY }, + { + toolName: legacyName2, + decision: PolicyDecision.DENY, + modes: MODES_BY_PERMISSIVENESS, + }, ]; engine = new PolicyEngine({ rules }); @@ -275,8 +324,17 @@ describe('PolicyEngine', () => { it('should apply wildcard rules (no toolName)', async () => { const rules: PolicyRule[] = [ - { toolName: '*', decision: PolicyDecision.DENY }, // Applies to all tools - { toolName: 'safe-tool', decision: PolicyDecision.ALLOW, priority: 10 }, + { + toolName: '*', + decision: PolicyDecision.DENY, + modes: MODES_BY_PERMISSIVENESS, + }, // Applies to all tools + { + toolName: 'safe-tool', + decision: PolicyDecision.ALLOW, + priority: 10, + modes: MODES_BY_PERMISSIVENESS, + }, ]; engine = new PolicyEngine({ rules }); @@ -297,17 +355,24 @@ describe('PolicyEngine', () => { toolName: 'interactive-tool', decision: PolicyDecision.ASK_USER, interactive: true, + modes: MODES_BY_PERMISSIVENESS, }, { toolName: 'interactive-tool', decision: PolicyDecision.DENY, interactive: false, + modes: MODES_BY_PERMISSIVENESS, + }, + { + toolName: 'allowed-tool', + decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, }, - { toolName: 'allowed-tool', decision: PolicyDecision.ALLOW }, { toolName: 'ask_user', decision: PolicyDecision.DENY, interactive: false, + modes: MODES_BY_PERMISSIVENESS, }, ], }; @@ -334,6 +399,7 @@ describe('PolicyEngine', () => { toolName: 'edit', decision: PolicyDecision.ASK_USER, priority: 10, + modes: MODES_BY_PERMISSIVENESS, }, { toolName: 'edit', @@ -401,7 +467,11 @@ describe('PolicyEngine', () => { it('should NOT override explicit DENY rules in YOLO mode', async () => { const rules: PolicyRule[] = [ - { toolName: 'dangerous-tool', decision: PolicyDecision.DENY }, + { + toolName: 'dangerous-tool', + decision: PolicyDecision.DENY, + modes: [ApprovalMode.YOLO], + }, ]; engine = new PolicyEngine({ rules, approvalMode: ApprovalMode.YOLO }); @@ -423,8 +493,14 @@ describe('PolicyEngine', () => { toolName: 'test-tool', decision: PolicyDecision.ASK_USER, priority: 10, + modes: [ApprovalMode.YOLO], + }, + { + toolName: 'test-tool', + decision: PolicyDecision.DENY, + priority: 20, + modes: [ApprovalMode.YOLO], }, - { toolName: 'test-tool', decision: PolicyDecision.DENY, priority: 20 }, ]; engine = new PolicyEngine({ rules, approvalMode: ApprovalMode.YOLO }); @@ -440,16 +516,19 @@ describe('PolicyEngine', () => { toolName: 'tool1', decision: PolicyDecision.ALLOW, priority: 5, + modes: MODES_BY_PERMISSIVENESS, }); engine.addRule({ toolName: 'tool2', decision: PolicyDecision.DENY, priority: 10, + modes: MODES_BY_PERMISSIVENESS, }); engine.addRule({ toolName: 'tool3', decision: PolicyDecision.ASK_USER, priority: 1, + modes: MODES_BY_PERMISSIVENESS, }); const rules = engine.getRules(); @@ -464,7 +543,11 @@ describe('PolicyEngine', () => { (await engine.check({ name: 'new-tool' }, undefined)).decision, ).toBe(PolicyDecision.ASK_USER); - engine.addRule({ toolName: 'new-tool', decision: PolicyDecision.ALLOW }); + engine.addRule({ + toolName: 'new-tool', + decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, + }); expect( (await engine.check({ name: 'new-tool' }, undefined)).decision, @@ -474,12 +557,21 @@ describe('PolicyEngine', () => { describe('removeRulesForTool', () => { it('should remove rules for specific tool', () => { - engine.addRule({ toolName: 'tool1', decision: PolicyDecision.ALLOW }); - engine.addRule({ toolName: 'tool2', decision: PolicyDecision.DENY }); + engine.addRule({ + toolName: 'tool1', + decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, + }); + engine.addRule({ + toolName: 'tool2', + decision: PolicyDecision.DENY, + modes: MODES_BY_PERMISSIVENESS, + }); engine.addRule({ toolName: 'tool1', decision: PolicyDecision.ASK_USER, priority: 10, + modes: MODES_BY_PERMISSIVENESS, }); expect(engine.getRules()).toHaveLength(3); @@ -497,16 +589,19 @@ describe('PolicyEngine', () => { toolName: 'tool1', decision: PolicyDecision.ALLOW, source: 'source1', + modes: MODES_BY_PERMISSIVENESS, }); engine.addRule({ toolName: 'tool1', decision: PolicyDecision.DENY, source: 'source2', + modes: MODES_BY_PERMISSIVENESS, }); engine.addRule({ toolName: 'tool2', decision: PolicyDecision.ALLOW, source: 'source1', + modes: MODES_BY_PERMISSIVENESS, }); expect(engine.getRules()).toHaveLength(3); @@ -527,7 +622,11 @@ describe('PolicyEngine', () => { }); it('should handle removing non-existent tool', () => { - engine.addRule({ toolName: 'existing', decision: PolicyDecision.ALLOW }); + engine.addRule({ + toolName: 'existing', + decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, + }); expect(() => engine.removeRulesForTool('non-existent')).not.toThrow(); expect(engine.getRules()).toHaveLength(1); @@ -537,8 +636,16 @@ describe('PolicyEngine', () => { describe('getRules', () => { it('should return readonly array of rules', () => { const rules: PolicyRule[] = [ - { toolName: 'tool1', decision: PolicyDecision.ALLOW }, - { toolName: 'tool2', decision: PolicyDecision.DENY }, + { + toolName: 'tool1', + decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, + }, + { + toolName: 'tool2', + decision: PolicyDecision.DENY, + modes: MODES_BY_PERMISSIVENESS, + }, ]; engine = new PolicyEngine({ rules }); @@ -554,7 +661,12 @@ describe('PolicyEngine', () => { it('should match global wildcard (*)', async () => { engine = new PolicyEngine({ rules: [ - { toolName: '*', decision: PolicyDecision.ALLOW, priority: 10 }, + { + toolName: '*', + decision: PolicyDecision.ALLOW, + priority: 10, + modes: MODES_BY_PERMISSIVENESS, + }, ], }); @@ -570,7 +682,12 @@ describe('PolicyEngine', () => { it('should match any MCP tool when toolName is mcp_*', async () => { engine = new PolicyEngine({ rules: [ - { toolName: 'mcp_*', decision: PolicyDecision.ALLOW, priority: 10 }, + { + toolName: 'mcp_*', + decision: PolicyDecision.ALLOW, + priority: 10, + modes: MODES_BY_PERMISSIVENESS, + }, ], defaultDecision: PolicyDecision.DENY, }); @@ -593,12 +710,14 @@ describe('PolicyEngine', () => { mcpName: 'my-server', decision: PolicyDecision.ALLOW, priority: 10, + modes: MODES_BY_PERMISSIVENESS, }, { toolName: 'mcp_blocked-server_*', mcpName: 'blocked-server', decision: PolicyDecision.DENY, priority: 20, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -656,12 +775,14 @@ describe('PolicyEngine', () => { mcpName: 'my-server', decision: PolicyDecision.ALLOW, priority: 10, + modes: MODES_BY_PERMISSIVENESS, }, { toolName: 'mcp_my-server_dangerous-tool', mcpName: 'my-server', decision: PolicyDecision.DENY, priority: 20, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -690,6 +811,7 @@ describe('PolicyEngine', () => { toolName: 'mcp_safe_server_*', mcpName: 'safe_server', decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, }, ]; engine = new PolicyEngine({ rules }); @@ -711,6 +833,7 @@ describe('PolicyEngine', () => { toolName: 'mcp_safe_server_*', mcpName: 'safe_server', decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, }, ]; engine = new PolicyEngine({ rules }); @@ -728,6 +851,7 @@ describe('PolicyEngine', () => { toolName: 'mcp_safe_server_*', mcpName: 'safe_server', decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, }, ]; engine = new PolicyEngine({ rules }); @@ -742,13 +866,24 @@ describe('PolicyEngine', () => { describe('complex scenarios', () => { it('should handle multiple matching rules with different priorities', async () => { const rules: PolicyRule[] = [ - { toolName: '*', decision: PolicyDecision.DENY, priority: 0 }, // Default deny all - { toolName: 'shell', decision: PolicyDecision.ASK_USER, priority: 5 }, + { + toolName: '*', + decision: PolicyDecision.DENY, + priority: 0, + modes: MODES_BY_PERMISSIVENESS, + }, // Default deny all + { + toolName: 'shell', + decision: PolicyDecision.ASK_USER, + priority: 5, + modes: MODES_BY_PERMISSIVENESS, + }, { toolName: 'shell', argsPattern: /"command":"ls/, decision: PolicyDecision.ALLOW, priority: 10, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -788,6 +923,7 @@ describe('PolicyEngine', () => { toolName: 'run_shell_command', argsPattern: new RegExp(patterns[0]!), decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, }, ]; engine = new PolicyEngine({ rules }); @@ -809,6 +945,7 @@ describe('PolicyEngine', () => { toolName: 'read', argsPattern: /secret/, decision: PolicyDecision.DENY, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -847,6 +984,7 @@ describe('PolicyEngine', () => { // Pattern matches the stable stringified format argsPattern: /"command":"rm[^"]*-rf/, decision: PolicyDecision.DENY, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -879,6 +1017,7 @@ describe('PolicyEngine', () => { toolName: 'api', argsPattern: /"sensitive":true/, decision: PolicyDecision.DENY, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -908,6 +1047,7 @@ describe('PolicyEngine', () => { toolName: 'test', argsPattern: /\[Circular\]/, decision: PolicyDecision.DENY, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -950,6 +1090,7 @@ describe('PolicyEngine', () => { toolName: 'deep', argsPattern: /\[Circular\]/, decision: PolicyDecision.DENY, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -992,12 +1133,14 @@ describe('PolicyEngine', () => { toolName: 'test', argsPattern: /\[Circular\]/, decision: PolicyDecision.DENY, + modes: MODES_BY_PERMISSIVENESS, }, { toolName: 'test', argsPattern: /"value":"shared"/, decision: PolicyDecision.ALLOW, priority: 10, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -1023,6 +1166,7 @@ describe('PolicyEngine', () => { toolName: 'test', argsPattern: /"definedValue":"test"/, decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -1046,6 +1190,7 @@ describe('PolicyEngine', () => { toolName: 'test', argsPattern: /undefinedValue/, decision: PolicyDecision.DENY, + modes: MODES_BY_PERMISSIVENESS, }, ]; engine = new PolicyEngine({ rules: rulesWithUndefined }); @@ -1059,6 +1204,7 @@ describe('PolicyEngine', () => { toolName: 'test', argsPattern: /functionValue/, decision: PolicyDecision.DENY, + modes: MODES_BY_PERMISSIVENESS, }, ]; engine = new PolicyEngine({ rules: rulesWithFunction }); @@ -1073,6 +1219,7 @@ describe('PolicyEngine', () => { toolName: 'test', argsPattern: /\["value",null,null,null\]/, decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -1114,6 +1261,7 @@ describe('PolicyEngine', () => { toolName: 'test', argsPattern: /.*/, decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, }, ]; engine = new PolicyEngine({ rules }); @@ -1137,11 +1285,13 @@ describe('PolicyEngine', () => { toolName: 'test', argsPattern: /"sanitized":"safe"/, decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, }, { toolName: 'test', argsPattern: /"dangerous":"data"/, decision: PolicyDecision.DENY, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -1167,6 +1317,7 @@ describe('PolicyEngine', () => { toolName: 'test', argsPattern: /"value":"string-value"/, decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -1191,6 +1342,7 @@ describe('PolicyEngine', () => { toolName: 'test', argsPattern: /"fallback":"value"/, decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -1217,6 +1369,7 @@ describe('PolicyEngine', () => { // Matches "echo" prefix argsPattern: /"command":"echo/, decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -1254,6 +1407,7 @@ describe('PolicyEngine', () => { argsPattern: /"command":"echo/, decision: PolicyDecision.ALLOW, allowRedirection: true, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -1282,6 +1436,7 @@ describe('PolicyEngine', () => { toolName, decision: PolicyDecision.ALLOW, allowRedirection: true, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -1302,11 +1457,13 @@ describe('PolicyEngine', () => { toolName, decision: PolicyDecision.ASK_USER, interactive: true, + modes: MODES_BY_PERMISSIVENESS, }, { toolName, decision: PolicyDecision.DENY, interactive: false, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -1324,6 +1481,7 @@ describe('PolicyEngine', () => { toolName: 'run_shell_command', argsPattern: /"command":"echo/, decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -1351,12 +1509,14 @@ describe('PolicyEngine', () => { // Note: stableStringify sorts keys alphabetically and has no spaces: {"command":"echo hello","dir_path":"/safe/path"} argsPattern: /"command":"echo hello".*"dir_path":"\/safe\/path"/, decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, }, { // Catch-all ALLOW for shell but with low priority toolName: 'run_shell_command', decision: PolicyDecision.ALLOW, priority: -100, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -1382,18 +1542,21 @@ describe('PolicyEngine', () => { argsPattern: /"command":"git status/, decision: PolicyDecision.ALLOW, priority: 20, + modes: MODES_BY_PERMISSIVENESS, }, { toolName: 'run_shell_command', argsPattern: /"command":"ls/, decision: PolicyDecision.ALLOW, priority: 20, + modes: MODES_BY_PERMISSIVENESS, }, { // Catch-all ASK_USER for shell toolName: 'run_shell_command', decision: PolicyDecision.ASK_USER, priority: 10, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -1420,18 +1583,21 @@ describe('PolicyEngine', () => { argsPattern: /"command":"git status && ls"/, decision: PolicyDecision.DENY, priority: 30, + modes: MODES_BY_PERMISSIVENESS, }, { toolName: 'run_shell_command', argsPattern: /"command":"git status/, decision: PolicyDecision.ALLOW, priority: 20, + modes: MODES_BY_PERMISSIVENESS, }, { toolName: 'run_shell_command', argsPattern: /"command":"ls/, decision: PolicyDecision.ALLOW, priority: 20, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -1455,17 +1621,20 @@ describe('PolicyEngine', () => { argsPattern: /"command":"rm/, decision: PolicyDecision.DENY, priority: 20, + modes: MODES_BY_PERMISSIVENESS, }, { toolName: 'run_shell_command', argsPattern: /"command":"echo/, decision: PolicyDecision.ALLOW, priority: 20, + modes: MODES_BY_PERMISSIVENESS, }, { toolName: 'run_shell_command', decision: PolicyDecision.ASK_USER, priority: 10, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -1491,11 +1660,13 @@ describe('PolicyEngine', () => { toolName: 'run_shell_command', decision: PolicyDecision.ALLOW, interactive: true, + modes: MODES_BY_PERMISSIVENESS, }, { toolName: 'run_shell_command', decision: PolicyDecision.DENY, interactive: false, + modes: MODES_BY_PERMISSIVENESS, }, ], }; @@ -1521,6 +1692,7 @@ describe('PolicyEngine', () => { { toolName: 'run_shell_command', decision: PolicyDecision.ASK_USER, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -1549,6 +1721,7 @@ describe('PolicyEngine', () => { toolName: 'run_shell_command', decision: PolicyDecision.ALLOW, allowRedirection: true, + modes: MODES_BY_PERMISSIVENESS, }, ], }; @@ -1574,6 +1747,7 @@ describe('PolicyEngine', () => { { toolName: 'run_shell_command', decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -1599,12 +1773,14 @@ describe('PolicyEngine', () => { argsPattern: /"command":"mkdir\b/, decision: PolicyDecision.ALLOW, priority: 20, + modes: MODES_BY_PERMISSIVENESS, }, { toolName: 'run_shell_command', argsPattern: /"command":"echo\b/, decision: PolicyDecision.ALLOW, priority: 20, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -1631,12 +1807,14 @@ describe('PolicyEngine', () => { argsPattern: /"command":"mkdir\b/, decision: PolicyDecision.ALLOW, priority: 20, + modes: MODES_BY_PERMISSIVENESS, }, { toolName: 'run_shell_command', argsPattern: /"command":"echo\b/, decision: PolicyDecision.ALLOW, priority: 20, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -1668,6 +1846,7 @@ describe('PolicyEngine', () => { argsPattern: /"command":"echo\b/, decision: PolicyDecision.ALLOW, priority: 20, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -1695,6 +1874,7 @@ describe('PolicyEngine', () => { argsPattern: /"command":"echo\b/, decision: PolicyDecision.ALLOW, priority: 20, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -1728,6 +1908,7 @@ describe('PolicyEngine', () => { toolName: 'unknown_subagent', decision: PolicyDecision.ALLOW, priority: PRIORITY_SUBAGENT_TOOL, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -1759,6 +1940,7 @@ describe('PolicyEngine', () => { toolName: 'run_shell_command', decision: PolicyDecision.ASK_USER, priority: 10, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -1786,7 +1968,8 @@ describe('PolicyEngine', () => { { toolName: 'run_shell_command', decision: PolicyDecision.DENY, - priority: 2000, // Very high priority DENY (e.g. Admin) + priority: 2000, // Very high priority DENY (e.g. Admin), + modes: MODES_BY_PERMISSIVENESS, }, { toolName: '*', @@ -1819,6 +2002,7 @@ describe('PolicyEngine', () => { toolName: 'run_shell_command', decision: PolicyDecision.ALLOW, priority: 20, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -1847,6 +2031,7 @@ describe('PolicyEngine', () => { { toolName: 'test-tool', decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, }, ]; const checkers: SafetyCheckerRule[] = [ @@ -1857,6 +2042,7 @@ describe('PolicyEngine', () => { name: 'test-checker', config: { content: 'test-content' }, }, + modes: MODES_BY_PERMISSIVENESS, }, ]; engine = new PolicyEngine({ rules, checkers }, mockCheckerRunner); @@ -1885,6 +2071,7 @@ describe('PolicyEngine', () => { { toolName: 'test', decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, }, ]; const checkers: SafetyCheckerRule[] = [ @@ -1894,6 +2081,7 @@ describe('PolicyEngine', () => { type: 'in-process', name: InProcessCheckerType.ALLOWED_PATH, }, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -1912,6 +2100,7 @@ describe('PolicyEngine', () => { { toolName: 'test-tool', decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, }, ]; const checkers: SafetyCheckerRule[] = [ @@ -1922,6 +2111,7 @@ describe('PolicyEngine', () => { name: 'test-checker', config: { content: 'test-content' }, }, + modes: MODES_BY_PERMISSIVENESS, }, ]; engine = new PolicyEngine({ rules, checkers }, mockCheckerRunner); @@ -1944,6 +2134,7 @@ describe('PolicyEngine', () => { { toolName: 'test-tool', decision: PolicyDecision.ASK_USER, + modes: MODES_BY_PERMISSIVENESS, }, ]; const checkers: SafetyCheckerRule[] = [ @@ -1954,6 +2145,7 @@ describe('PolicyEngine', () => { name: 'test-checker', config: { content: 'test-content' }, }, + modes: MODES_BY_PERMISSIVENESS, }, ]; engine = new PolicyEngine({ rules, checkers }, mockCheckerRunner); @@ -1976,6 +2168,7 @@ describe('PolicyEngine', () => { { toolName: 'test', decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, }, ]; const checkers: SafetyCheckerRule[] = [ @@ -1985,6 +2178,7 @@ describe('PolicyEngine', () => { type: 'in-process', name: InProcessCheckerType.ALLOWED_PATH, }, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -2004,6 +2198,7 @@ describe('PolicyEngine', () => { { toolName: 'test-tool', decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, }, ]; engine = new PolicyEngine({ rules }, mockCheckerRunner); @@ -2037,6 +2232,7 @@ describe('PolicyEngine', () => { { toolName: 'test', decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, }, ]; const checkers: SafetyCheckerRule[] = [ @@ -2044,11 +2240,13 @@ describe('PolicyEngine', () => { toolName: 'test', priority: 10, checker: { type: 'external', name: 'checker1' }, + modes: MODES_BY_PERMISSIVENESS, }, { toolName: 'test', priority: 20, // Should run first checker: { type: 'external', name: 'checker2' }, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -2086,11 +2284,13 @@ describe('PolicyEngine', () => { toolName: '*', checker: { type: 'external', name: 'checker1' }, priority: 5, + modes: MODES_BY_PERMISSIVENESS, }; const checker2: SafetyCheckerRule = { toolName: '*', checker: { type: 'external', name: 'checker2' }, priority: 10, + modes: MODES_BY_PERMISSIVENESS, }; engine.addChecker(checker1); @@ -2108,16 +2308,22 @@ describe('PolicyEngine', () => { describe('checker matching logic', () => { it('should match checkers using toolName and argsPattern', async () => { const rules: PolicyRule[] = [ - { toolName: 'tool', decision: PolicyDecision.ALLOW }, + { + toolName: 'tool', + decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, + }, ]; const matchingChecker: SafetyCheckerRule = { checker: { type: 'external', name: 'matching' }, toolName: 'tool', argsPattern: /"safe":true/, + modes: MODES_BY_PERMISSIVENESS, }; const nonMatchingChecker: SafetyCheckerRule = { checker: { type: 'external', name: 'non-matching' }, toolName: 'other', + modes: MODES_BY_PERMISSIVENESS, }; engine = new PolicyEngine( @@ -2143,11 +2349,16 @@ describe('PolicyEngine', () => { it('should match global wildcard (*) for checkers', async () => { const rules: PolicyRule[] = [ - { toolName: '*', decision: PolicyDecision.ALLOW }, + { + toolName: '*', + decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, + }, ]; const globalChecker: SafetyCheckerRule = { checker: { type: 'external', name: 'global' }, toolName: '*', + modes: MODES_BY_PERMISSIVENESS, }; engine = new PolicyEngine( @@ -2180,12 +2391,14 @@ describe('PolicyEngine', () => { toolName: 'mcp_server_tool', mcpName: 'server', decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, }, ]; const wildcardChecker: SafetyCheckerRule = { checker: { type: 'external', name: 'wildcard' }, toolName: 'mcp_server_*', mcpName: 'server', + modes: MODES_BY_PERMISSIVENESS, }; engine = new PolicyEngine( @@ -2206,7 +2419,11 @@ describe('PolicyEngine', () => { }); it('should run safety checkers when decision is ASK_USER and downgrade to DENY on failure', async () => { const rules: PolicyRule[] = [ - { toolName: 'tool', decision: PolicyDecision.ASK_USER }, + { + toolName: 'tool', + decision: PolicyDecision.ASK_USER, + modes: MODES_BY_PERMISSIVENESS, + }, ]; const checkers: SafetyCheckerRule[] = [ { @@ -2215,6 +2432,7 @@ describe('PolicyEngine', () => { type: 'in-process', name: InProcessCheckerType.ALLOWED_PATH, }, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -2232,7 +2450,11 @@ describe('PolicyEngine', () => { it('should run safety checkers when decision is ASK_USER and keep ASK_USER on success', async () => { const rules: PolicyRule[] = [ - { toolName: 'tool', decision: PolicyDecision.ASK_USER }, + { + toolName: 'tool', + decision: PolicyDecision.ASK_USER, + modes: MODES_BY_PERMISSIVENESS, + }, ]; const checkers: SafetyCheckerRule[] = [ { @@ -2241,6 +2463,7 @@ describe('PolicyEngine', () => { type: 'in-process', name: InProcessCheckerType.ALLOWED_PATH, }, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -2257,7 +2480,11 @@ describe('PolicyEngine', () => { it('should downgrade ALLOW to ASK_USER if checker returns ASK_USER', async () => { const rules: PolicyRule[] = [ - { toolName: 'tool', decision: PolicyDecision.ALLOW }, + { + toolName: 'tool', + decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, + }, ]; const checkers: SafetyCheckerRule[] = [ { @@ -2266,6 +2493,7 @@ describe('PolicyEngine', () => { type: 'in-process', name: InProcessCheckerType.ALLOWED_PATH, }, + modes: MODES_BY_PERMISSIVENESS, }, ]; @@ -2301,7 +2529,13 @@ describe('PolicyEngine', () => { }, { name: 'should apply rules without explicit modes to all modes', - rules: [{ toolName: 'tool1', decision: PolicyDecision.DENY }], + rules: [ + { + toolName: 'tool1', + decision: PolicyDecision.DENY, + modes: MODES_BY_PERMISSIVENESS, + }, + ], allToolNames: ['tool1', 'tool2'], expected: ['tool1'], }, @@ -2313,13 +2547,13 @@ describe('PolicyEngine', () => { decision: PolicyDecision.ALLOW, argsPattern: /safe/, priority: 100, - modes: [ApprovalMode.DEFAULT], + modes: MODES_BY_PERMISSIVENESS, }, { toolName: 'tool1', decision: PolicyDecision.DENY, priority: 10, - modes: [ApprovalMode.DEFAULT], + modes: MODES_BY_PERMISSIVENESS, }, ], allToolNames: ['tool1'], @@ -2331,12 +2565,12 @@ describe('PolicyEngine', () => { { toolName: 'tool1', decision: PolicyDecision.DENY, - modes: [ApprovalMode.DEFAULT], + modes: MODES_BY_PERMISSIVENESS, }, { toolName: 'tool2', decision: PolicyDecision.ALLOW, - modes: [ApprovalMode.DEFAULT], + modes: MODES_BY_PERMISSIVENESS, }, ], allToolNames: ['tool1', 'tool2', 'tool3'], @@ -2349,13 +2583,13 @@ describe('PolicyEngine', () => { toolName: 'tool1', decision: PolicyDecision.DENY, priority: 100, - modes: [ApprovalMode.DEFAULT], + modes: MODES_BY_PERMISSIVENESS, }, { toolName: 'tool1', decision: PolicyDecision.ALLOW, priority: 10, - modes: [ApprovalMode.DEFAULT], + modes: MODES_BY_PERMISSIVENESS, }, ], allToolNames: ['tool1'], @@ -2368,13 +2602,13 @@ describe('PolicyEngine', () => { toolName: 'tool1', decision: PolicyDecision.ALLOW, priority: 100, - modes: [ApprovalMode.DEFAULT], + modes: MODES_BY_PERMISSIVENESS, }, { toolName: 'tool1', decision: PolicyDecision.DENY, priority: 10, - modes: [ApprovalMode.DEFAULT], + modes: MODES_BY_PERMISSIVENESS, }, ], allToolNames: ['tool1'], @@ -2386,13 +2620,13 @@ describe('PolicyEngine', () => { { toolName: 'tool1', decision: PolicyDecision.ASK_USER, - modes: [ApprovalMode.DEFAULT], + modes: MODES_BY_PERMISSIVENESS, interactive: true, }, { toolName: 'tool1', decision: PolicyDecision.DENY, - modes: [ApprovalMode.DEFAULT], + modes: MODES_BY_PERMISSIVENESS, interactive: false, }, ], @@ -2407,10 +2641,12 @@ describe('PolicyEngine', () => { toolName: 'ask_user', decision: PolicyDecision.DENY, interactive: false, + modes: MODES_BY_PERMISSIVENESS, }, { toolName: 'read_file', decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, }, ], nonInteractive: true, @@ -2424,7 +2660,7 @@ describe('PolicyEngine', () => { toolName: 'tool1', decision: PolicyDecision.DENY, argsPattern: /something/, - modes: [ApprovalMode.DEFAULT], + modes: MODES_BY_PERMISSIVENESS, }, ], allToolNames: ['tool1'], @@ -2483,7 +2719,7 @@ describe('PolicyEngine', () => { toolName: 'mcp_server_*', mcpName: 'server', decision: PolicyDecision.DENY, - modes: [ApprovalMode.DEFAULT], + modes: MODES_BY_PERMISSIVENESS, }, ], allToolNames: [ @@ -2506,14 +2742,14 @@ describe('PolicyEngine', () => { mcpName: 'server', decision: PolicyDecision.DENY, priority: 100, - modes: [ApprovalMode.DEFAULT], + modes: MODES_BY_PERMISSIVENESS, }, { toolName: 'mcp_server_tool1', mcpName: 'server', decision: PolicyDecision.DENY, // redundant but tests ordering priority: 10, - modes: [ApprovalMode.DEFAULT], + modes: MODES_BY_PERMISSIVENESS, }, ], allToolNames: ['mcp_server_tool1', 'mcp_server_tool2'], @@ -2547,6 +2783,7 @@ describe('PolicyEngine', () => { toolName: 'run_shell_command', decision: PolicyDecision.ASK_USER, priority: 10, + modes: MODES_BY_PERMISSIVENESS, }, ], allToolNames: ['write_file', 'run_shell_command', 'read_file'], @@ -2560,14 +2797,14 @@ describe('PolicyEngine', () => { mcpName: 'server', decision: PolicyDecision.ALLOW, priority: 100, - modes: [ApprovalMode.DEFAULT], + modes: MODES_BY_PERMISSIVENESS, }, { toolName: 'mcp_server_tool1', mcpName: 'server', decision: PolicyDecision.DENY, priority: 10, - modes: [ApprovalMode.DEFAULT], + modes: MODES_BY_PERMISSIVENESS, }, ], allToolNames: ['mcp_server_tool1'], @@ -2581,6 +2818,7 @@ describe('PolicyEngine', () => { toolName: '*', decision: PolicyDecision.DENY, priority: 10, + modes: MODES_BY_PERMISSIVENESS, }, ], allToolNames: ['toolA', 'toolB', 'mcp_server_toolC'], @@ -2593,6 +2831,7 @@ describe('PolicyEngine', () => { toolName: 'mcp_*', decision: PolicyDecision.DENY, priority: 10, + modes: MODES_BY_PERMISSIVENESS, }, ], allToolNames: ['localTool', 'mcp_myserver_mytool'], @@ -2608,6 +2847,7 @@ describe('PolicyEngine', () => { toolName: 'mcp_server_*', decision: PolicyDecision.DENY, priority: 10, + modes: MODES_BY_PERMISSIVENESS, }, ], allToolNames: [ @@ -2652,6 +2892,7 @@ describe('PolicyEngine', () => { toolAnnotations: { destructiveHint: true }, decision: PolicyDecision.DENY, priority: 10, + modes: MODES_BY_PERMISSIVENESS, }, ], }); @@ -2670,6 +2911,7 @@ describe('PolicyEngine', () => { toolAnnotations: { destructiveHint: true }, decision: PolicyDecision.DENY, priority: 10, + modes: MODES_BY_PERMISSIVENESS, }, ], }); @@ -2692,6 +2934,7 @@ describe('PolicyEngine', () => { toolAnnotations: { destructiveHint: true }, decision: PolicyDecision.DENY, priority: 10, + modes: MODES_BY_PERMISSIVENESS, }, ], }); @@ -2714,6 +2957,7 @@ describe('PolicyEngine', () => { toolAnnotations: { destructiveHint: true }, decision: PolicyDecision.DENY, priority: 10, + modes: MODES_BY_PERMISSIVENESS, }, ], }); @@ -2746,11 +2990,13 @@ describe('PolicyEngine', () => { toolName: 'glob', decision: PolicyDecision.ALLOW, priority: 70, + modes: MODES_BY_PERMISSIVENESS, }, { toolName: 'read_file', decision: PolicyDecision.ALLOW, priority: 70, + modes: MODES_BY_PERMISSIVENESS, }, { // Simulates plan.toml: mcpName="*" → toolName="mcp_*" @@ -2758,11 +3004,13 @@ describe('PolicyEngine', () => { toolAnnotations: { readOnlyHint: true }, decision: PolicyDecision.ASK_USER, priority: 70, + modes: MODES_BY_PERMISSIVENESS, }, { toolName: '*', decision: PolicyDecision.DENY, priority: 60, + modes: MODES_BY_PERMISSIVENESS, }, ], }); @@ -2805,11 +3053,13 @@ describe('PolicyEngine', () => { toolAnnotations: { readOnlyHint: true }, decision: PolicyDecision.ASK_USER, priority: 70, + modes: MODES_BY_PERMISSIVENESS, }, { toolName: '*', decision: PolicyDecision.DENY, priority: 60, + modes: MODES_BY_PERMISSIVENESS, }, ], }); @@ -2841,16 +3091,19 @@ describe('PolicyEngine', () => { toolName: 'glob', decision: PolicyDecision.ALLOW, priority: 70, + modes: MODES_BY_PERMISSIVENESS, }, { toolName: 'read_file', decision: PolicyDecision.ALLOW, priority: 70, + modes: MODES_BY_PERMISSIVENESS, }, { toolName: '*', decision: PolicyDecision.DENY, priority: 60, + modes: MODES_BY_PERMISSIVENESS, }, ], }); @@ -3127,7 +3380,7 @@ describe('PolicyEngine', () => { toolName: 'exit_plan_mode', decision: PolicyDecision.DENY, priority: 10, - modes: [ApprovalMode.DEFAULT], + modes: MODES_BY_PERMISSIVENESS, denyMessage: 'You are not in Plan Mode.', }, ]; @@ -3183,23 +3436,31 @@ describe('PolicyEngine', () => { toolName: 'rule1', decision: PolicyDecision.ALLOW, priority: 1.1, + modes: MODES_BY_PERMISSIVENESS, }); engine.addRule({ toolName: 'rule2', decision: PolicyDecision.ALLOW, priority: 1.5, + modes: MODES_BY_PERMISSIVENESS, }); engine.addRule({ toolName: 'rule3', decision: PolicyDecision.ALLOW, priority: 2.1, + modes: MODES_BY_PERMISSIVENESS, }); engine.addRule({ toolName: 'rule4', decision: PolicyDecision.ALLOW, priority: 0.5, + modes: MODES_BY_PERMISSIVENESS, }); - engine.addRule({ toolName: 'rule5', decision: PolicyDecision.ALLOW }); // priority undefined -> 0 + engine.addRule({ + toolName: 'rule5', + decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, + }); // priority undefined -> 0 expect(engine.getRules()).toHaveLength(5); @@ -3219,12 +3480,18 @@ describe('PolicyEngine', () => { toolName: 'rule1', decision: PolicyDecision.ALLOW, priority: 0.5, + modes: MODES_BY_PERMISSIVENESS, }); - engine.addRule({ toolName: 'rule2', decision: PolicyDecision.ALLOW }); // defaults to 0 + engine.addRule({ + toolName: 'rule2', + decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, + }); // defaults to 0 engine.addRule({ toolName: 'rule3', decision: PolicyDecision.ALLOW, priority: 1.5, + modes: MODES_BY_PERMISSIVENESS, }); expect(engine.getRules()).toHaveLength(3); @@ -3243,16 +3510,19 @@ describe('PolicyEngine', () => { toolName: 'rule1', decision: PolicyDecision.ALLOW, source: 'source1', + modes: MODES_BY_PERMISSIVENESS, }); engine.addRule({ toolName: 'rule2', decision: PolicyDecision.ALLOW, source: 'source2', + modes: MODES_BY_PERMISSIVENESS, }); engine.addRule({ toolName: 'rule3', decision: PolicyDecision.ALLOW, source: 'source1', + modes: MODES_BY_PERMISSIVENESS, }); expect(engine.getRules()).toHaveLength(3); @@ -3271,16 +3541,19 @@ describe('PolicyEngine', () => { toolName: '*', checker: { type: 'external', name: 'c1' }, priority: 1.1, + modes: MODES_BY_PERMISSIVENESS, }); engine.addChecker({ toolName: '*', checker: { type: 'external', name: 'c2' }, priority: 1.9, + modes: MODES_BY_PERMISSIVENESS, }); engine.addChecker({ toolName: '*', checker: { type: 'external', name: 'c3' }, priority: 2.5, + modes: MODES_BY_PERMISSIVENESS, }); expect(engine.getCheckers()).toHaveLength(3); @@ -3299,16 +3572,19 @@ describe('PolicyEngine', () => { toolName: '*', checker: { type: 'external', name: 'c1' }, source: 'sourceA', + modes: MODES_BY_PERMISSIVENESS, }); engine.addChecker({ toolName: '*', checker: { type: 'external', name: 'c2' }, source: 'sourceB', + modes: MODES_BY_PERMISSIVENESS, }); engine.addChecker({ toolName: '*', checker: { type: 'external', name: 'c3' }, source: 'sourceA', + modes: MODES_BY_PERMISSIVENESS, }); expect(engine.getCheckers()).toHaveLength(3); @@ -3329,6 +3605,7 @@ describe('PolicyEngine', () => { toolAnnotations: { readOnlyHint: true }, decision: PolicyDecision.ALLOW, priority: 10, + modes: MODES_BY_PERMISSIVENESS, }, ], defaultDecision: PolicyDecision.DENY, @@ -3359,11 +3636,13 @@ describe('PolicyEngine', () => { toolAnnotations: { experimental: true }, decision: PolicyDecision.DENY, priority: 20, + modes: MODES_BY_PERMISSIVENESS, }, { toolName: 'mcp_*', decision: PolicyDecision.ALLOW, priority: 10, + modes: MODES_BY_PERMISSIVENESS, }, ], }); @@ -3409,6 +3688,7 @@ describe('PolicyEngine', () => { decision: PolicyDecision.ALLOW, priority: 3 + ALWAYS_ALLOW_PRIORITY_FRACTION / 1000, // 3.95 source: 'Dynamic (Confirmed)', + modes: MODES_BY_PERMISSIVENESS, }; const engine = new PolicyEngine({ @@ -3430,6 +3710,7 @@ describe('PolicyEngine', () => { decision: PolicyDecision.ALLOW, priority: 3 + ALWAYS_ALLOW_PRIORITY_FRACTION / 1000, // 3.95 source: 'Dynamic (Confirmed)', + modes: MODES_BY_PERMISSIVENESS, }; const engine = new PolicyEngine({ @@ -3451,6 +3732,7 @@ describe('PolicyEngine', () => { decision: PolicyDecision.ALLOW, priority: 1.5, // Not a .950 fraction source: 'Normal Rule', + modes: MODES_BY_PERMISSIVENESS, }; const engine = new PolicyEngine({ @@ -3478,6 +3760,7 @@ describe('PolicyEngine', () => { toolName: 'test-tool', decision: PolicyDecision.ALLOW, priority: 3 + ALWAYS_ALLOW_PRIORITY_FRACTION / 1000, + modes: MODES_BY_PERMISSIVENESS, }; const engine = new PolicyEngine({ @@ -3498,6 +3781,7 @@ describe('PolicyEngine', () => { toolName: 'test-tool', decision: PolicyDecision.ALLOW, priority: 3 + ALWAYS_ALLOW_PRIORITY_FRACTION / 1000, + modes: MODES_BY_PERMISSIVENESS, }; const engine = new PolicyEngine({ @@ -3522,6 +3806,7 @@ describe('PolicyEngine', () => { toolName: 'my_tool', decision: PolicyDecision.ALLOW, interactive: true, + modes: MODES_BY_PERMISSIVENESS, }, ], nonInteractive: true, @@ -3542,6 +3827,7 @@ describe('PolicyEngine', () => { toolName: 'my_tool', decision: PolicyDecision.ALLOW, interactive: true, + modes: MODES_BY_PERMISSIVENESS, }, ], nonInteractive: false, @@ -3562,6 +3848,7 @@ describe('PolicyEngine', () => { toolName: 'my_tool', decision: PolicyDecision.ALLOW, interactive: false, + modes: MODES_BY_PERMISSIVENESS, }, ], nonInteractive: false, @@ -3582,6 +3869,7 @@ describe('PolicyEngine', () => { toolName: 'my_tool', decision: PolicyDecision.ALLOW, interactive: false, + modes: MODES_BY_PERMISSIVENESS, }, ], nonInteractive: true, @@ -3599,6 +3887,7 @@ describe('PolicyEngine', () => { const rule: PolicyRule = { toolName: 'my_tool', decision: PolicyDecision.ALLOW, + modes: MODES_BY_PERMISSIVENESS, }; const engineInteractive = new PolicyEngine({ diff --git a/packages/core/src/policy/policy-engine.ts b/packages/core/src/policy/policy-engine.ts index f2376df914..99c61edd2e 100644 --- a/packages/core/src/policy/policy-engine.ts +++ b/packages/core/src/policy/policy-engine.ts @@ -90,12 +90,9 @@ function ruleMatches( subagent?: string, ): boolean { // Check if rule applies to current approval mode - if (rule.modes && rule.modes.length > 0) { - if (!rule.modes.includes(currentApprovalMode)) { - return false; - } + if (!rule.modes.includes(currentApprovalMode)) { + return false; } - // Check subagent if specified (only for PolicyRule, SafetyCheckerRule doesn't have it) if ('subagent' in rule && rule.subagent !== undefined) { if (rule.subagent !== subagent) { diff --git a/packages/core/src/policy/shell-safety.test.ts b/packages/core/src/policy/shell-safety.test.ts index 340264485e..4852ed6b94 100644 --- a/packages/core/src/policy/shell-safety.test.ts +++ b/packages/core/src/policy/shell-safety.test.ts @@ -75,7 +75,11 @@ vi.mock('../utils/shell-utils.js', async (importOriginal) => { }); import { PolicyEngine } from './policy-engine.js'; -import { PolicyDecision, ApprovalMode } from './types.js'; +import { + PolicyDecision, + ApprovalMode, + MODES_BY_PERMISSIVENESS, +} from './types.js'; import type { FunctionCall } from '@google/genai'; import { buildArgsPatterns } from './utils.js'; @@ -96,6 +100,7 @@ describe('Shell Safety Policy', () => { argsPattern, decision: PolicyDecision.ALLOW, priority: 1.01, + modes: MODES_BY_PERMISSIVENESS, }, ], defaultDecision: PolicyDecision.ASK_USER, @@ -204,12 +209,14 @@ describe('Shell Safety Policy', () => { argsPattern: new RegExp(argsPatternsEcho[0]!), decision: PolicyDecision.ALLOW, priority: 2, + modes: MODES_BY_PERMISSIVENESS, }, { toolName: 'run_shell_command', argsPattern: new RegExp(argsPatternsGit[0]!), decision: PolicyDecision.ALLOW, priority: 2, + modes: MODES_BY_PERMISSIVENESS, }, ], defaultDecision: PolicyDecision.ASK_USER, @@ -290,6 +297,7 @@ describe('Shell Safety Policy', () => { argsPattern: new RegExp(argsPatternsEcho[0]!), decision: PolicyDecision.ALLOW, priority: 2, + modes: MODES_BY_PERMISSIVENESS, }, { toolName: 'run_shell_command', @@ -297,6 +305,7 @@ describe('Shell Safety Policy', () => { argsPattern: new RegExp(argsPatternsGit[0]!), decision: PolicyDecision.ALLOW, priority: 2, + modes: MODES_BY_PERMISSIVENESS, }, ], defaultDecision: PolicyDecision.ASK_USER, @@ -336,6 +345,7 @@ describe('Shell Safety Policy', () => { decision: PolicyDecision.ALLOW, priority: 2, allowRedirection: true, + modes: MODES_BY_PERMISSIVENESS, }, ], defaultDecision: PolicyDecision.ASK_USER, @@ -378,6 +388,7 @@ describe('Shell Safety Policy', () => { argsPattern: new RegExp(argsPatternsPush[0]!), decision: PolicyDecision.DENY, priority: 2, + modes: MODES_BY_PERMISSIVENESS, }, ], defaultDecision: PolicyDecision.ASK_USER, @@ -409,7 +420,8 @@ describe('Shell Safety Policy', () => { argsPattern: new RegExp(argsPatternsGitStatus[0]!), decision: PolicyDecision.ALLOW, priority: 2, - name: 'allow_git_status_rule', // Give a name to easily identify + name: 'allow_git_status_rule', // Give a name to easily identify, + modes: MODES_BY_PERMISSIVENESS, }, ], defaultDecision: PolicyDecision.ASK_USER, @@ -447,6 +459,7 @@ describe('Shell Safety Policy', () => { decision: PolicyDecision.ASK_USER, priority: 2, name: 'ask_another_unknown_command_rule', + modes: MODES_BY_PERMISSIVENESS, }, ], defaultDecision: PolicyDecision.ASK_USER, @@ -490,6 +503,7 @@ describe('Shell Safety Policy', () => { decision: PolicyDecision.ASK_USER, priority: 2, name: 'ask_rule_1', + modes: MODES_BY_PERMISSIVENESS, }, { toolName: 'run_shell_command', @@ -497,6 +511,7 @@ describe('Shell Safety Policy', () => { decision: PolicyDecision.ASK_USER, priority: 2, name: 'ask_rule_2', + modes: MODES_BY_PERMISSIVENESS, }, ], defaultDecision: PolicyDecision.ALLOW, // Set default to ALLOW to ensure rules are hit diff --git a/packages/core/src/policy/toml-loader.test.ts b/packages/core/src/policy/toml-loader.test.ts index 6835e200b4..fc4e92c8f2 100644 --- a/packages/core/src/policy/toml-loader.test.ts +++ b/packages/core/src/policy/toml-loader.test.ts @@ -9,6 +9,7 @@ import { PolicyDecision, ApprovalMode, PRIORITY_SUBAGENT_TOOL, + MODES_BY_PERMISSIVENESS, } from './types.js'; import * as fs from 'node:fs/promises'; import * as path from 'node:path'; @@ -68,6 +69,7 @@ describe('policy-toml-loader', () => { toolName = "glob" decision = "allow" priority = 100 +modes = ["plan", "default", "autoEdit", "yolo"] `); expect(result.rules).toHaveLength(1); @@ -76,6 +78,7 @@ priority = 100 decision: PolicyDecision.ALLOW, priority: 1.1, // tier 1 + 100/1000 source: 'Default: test.toml', + modes: MODES_BY_PERMISSIVENESS, }); expect(result.checkers).toHaveLength(0); expect(result.errors).toHaveLength(0); @@ -88,6 +91,7 @@ toolName = "run_shell_command" commandPrefix = ["git status", "git log"] decision = "allow" priority = 100 +modes = ["plan", "default", "autoEdit", "yolo"] `); expect(result.rules).toHaveLength(2); @@ -109,6 +113,7 @@ toolName = "annotated-tool" toolAnnotations = { readOnlyHint = true, custom = "value" } decision = "allow" priority = 70 +modes = ["plan", "default", "autoEdit", "yolo"] `); expect(result.rules).toHaveLength(1); @@ -127,6 +132,7 @@ toolName = "*" mcpName = "*" decision = "ask_user" priority = 10 +modes = ["plan", "default", "autoEdit", "yolo"] `); expect(result.rules).toHaveLength(1); @@ -142,6 +148,7 @@ mcpName = "*" toolName = "search" decision = "allow" priority = 10 +modes = ["plan", "default", "autoEdit", "yolo"] `); expect(result.rules).toHaveLength(1); @@ -156,6 +163,7 @@ toolName = "run_shell_command" commandRegex = "git (status|log).*" decision = "allow" priority = 100 +modes = ["plan", "default", "autoEdit", "yolo"] `); expect(result.rules).toHaveLength(1); @@ -178,6 +186,7 @@ toolName = "run_shell_command" commandRegex = "^git status" decision = "allow" priority = 100 +modes = ["plan", "default", "autoEdit", "yolo"] `); expect(result.rules).toHaveLength(1); @@ -195,6 +204,7 @@ priority = 100 toolName = ["glob", "grep", "read"] decision = "allow" priority = 100 +modes = ["plan", "default", "autoEdit", "yolo"] `); expect(result.rules).toHaveLength(3); @@ -213,6 +223,7 @@ mcpName = "google-workspace" toolName = ["calendar.list", "calendar.get"] decision = "allow" priority = 100 +modes = ["plan", "default", "autoEdit", "yolo"] `); expect(result.rules).toHaveLength(2); @@ -257,6 +268,7 @@ commandPrefix = "echo" decision = "allow" priority = 100 allow_redirection = true +modes = ["plan", "default", "autoEdit", "yolo"] `); expect(result.rules).toHaveLength(1); @@ -272,6 +284,7 @@ commandPrefix = "echo" decision = "allow" priority = 100 allowRedirection = true +modes = ["plan", "default", "autoEdit", "yolo"] `); expect(result.rules).toHaveLength(1); @@ -285,6 +298,7 @@ toolName = "rm" decision = "deny" priority = 100 deny_message = "Deletion is permanent" +modes = ["plan", "default", "autoEdit", "yolo"] `); expect(result.rules).toHaveLength(1); @@ -300,6 +314,7 @@ toolName = "rm" decision = "deny" priority = 100 denyMessage = "Deletion is permanent" +modes = ["plan", "default", "autoEdit", "yolo"] `); expect(result.rules).toHaveLength(1); @@ -357,6 +372,7 @@ priority = 100 [[rule]] toolName = "glob" priority = 100 +modes = ["default"] `); expect(result.rules).toHaveLength(0); @@ -372,6 +388,7 @@ toolName = "glob" commandPrefix = "git status" decision = "allow" priority = 100 +modes = ["plan", "default", "autoEdit", "yolo"] `); expect(result.errors).toHaveLength(1); @@ -387,6 +404,7 @@ commandPrefix = "git status" argsPattern = "test" decision = "allow" priority = 100 +modes = ["plan", "default", "autoEdit", "yolo"] `); expect(result.errors).toHaveLength(1); @@ -401,6 +419,7 @@ toolName = "run_shell_command" commandRegex = "git (status|branch" decision = "allow" priority = 100 +modes = ["plan", "default", "autoEdit", "yolo"] `); expect(result.rules).toHaveLength(0); @@ -416,6 +435,7 @@ toolName = "run_shell_command" commandPrefix = "git log *.txt" decision = "allow" priority = 100 +modes = ["plan", "default", "autoEdit", "yolo"] `); expect(result.rules).toHaveLength(1); @@ -437,6 +457,7 @@ priority = 100 toolName = "glob" decision = "allow" priority = 100 +modes = ["plan", "default", "autoEdit", "yolo"] `, ); @@ -447,6 +468,7 @@ priority = 100 toolName = "grep" decision = "allow" priority = -1 +modes = ["plan", "default", "autoEdit", "yolo"] `, ); @@ -465,6 +487,7 @@ priority = -1 [[safety_checker]] toolName = "write_file" priority = 100 +modes = ["plan", "default", "autoEdit", "yolo"] [safety_checker.checker] type = "in-process" name = "allowed-path" @@ -477,10 +500,62 @@ name = "allowed-path" }); describe('Negative Tests', () => { + it('should fail validation if modes field is missing in a rule', async () => { + const result = await runLoadPoliciesFromToml(` +[[rule]] +toolName = "glob" +decision = "allow" +priority = 100 +`); + + const errors = result.errors.filter( + (e) => e.errorType === 'schema_validation', + ); + expect(errors).toHaveLength(1); + expect(errors[0].message).toBe('Schema validation failed'); + expect(errors[0].details).toContain('modes'); + expect(errors[0].details).toContain('Required'); + }); + + it('should fail validation if modes field is missing in a safety checker', async () => { + const result = await runLoadPoliciesFromToml(` +[[safety_checker]] +toolName = "run_shell_command" +priority = 100 +checker = { type = "in-process", name = "allowed-path" } +`); + + const errors = result.errors.filter( + (e) => e.errorType === 'schema_validation', + ); + expect(errors).toHaveLength(1); + expect(errors[0].message).toBe('Schema validation failed'); + expect(errors[0].details).toContain('modes'); + expect(errors[0].details).toContain('Required'); + }); + + it('should not contain hardcoded required fields in the suggestion when validation fails', async () => { + const result = await runLoadPoliciesFromToml(` +[[rule]] +toolName = "glob" +# missing decision and priority and modes +`); + + const errors = result.errors.filter( + (e) => e.errorType === 'schema_validation', + ); + expect(errors).toHaveLength(1); + expect(errors[0].suggestion).not.toContain('(decision, priority)'); + expect(errors[0].suggestion).toBe( + 'Ensure all required fields are present with correct types', + ); + }); + it('should return a schema_validation error if toolName is missing in safety_checker', async () => { const result = await runLoadPoliciesFromToml(` [[safety_checker]] priority = 100 +modes = ["default"] [safety_checker.checker] type = "in-process" name = "allowed-path" @@ -497,6 +572,7 @@ name = "allowed-path" [[rule]] toolName = "test" decision = "allow" +modes = ["default"] `); expect(result.errors).toHaveLength(1); const error = result.errors[0]; @@ -510,6 +586,7 @@ decision = "allow" toolName = "test" decision = "allow" priority = 1.5 +modes = ["default"] `); expect(result.errors).toHaveLength(1); const error = result.errors[0]; @@ -524,6 +601,7 @@ priority = 1.5 toolName = "test" decision = "allow" priority = -1 +modes = ["default"] `); expect(result.errors).toHaveLength(1); const error = result.errors[0]; @@ -538,6 +616,7 @@ priority = -1 toolName = "test" decision = "allow" priority = -9999 +modes = ["default"] `); expect(result.errors).toHaveLength(1); const error = result.errors[0]; @@ -552,6 +631,7 @@ priority = -9999 toolName = "test" decision = "allow" priority = 1000 +modes = ["default"] `); expect(result.errors).toHaveLength(1); const error = result.errors[0]; @@ -566,6 +646,7 @@ priority = 1000 toolName = "test" decision = "allow" priority = 9999 +modes = ["default"] `); expect(result.errors).toHaveLength(1); const error = result.errors[0]; @@ -580,6 +661,7 @@ priority = 9999 toolName = "test" decision = "maybe" priority = 100 +modes = ["default"] `); expect(result.errors).toHaveLength(1); const error = result.errors[0]; @@ -592,6 +674,7 @@ priority = 100 [[rule]] decision = "allow" priority = 100 +modes = ["default"] `); expect(result.errors).toHaveLength(1); const error = result.errors[0]; @@ -606,6 +689,7 @@ priority = 100 toolName = 123 decision = "allow" priority = 100 +modes = ["default"] `); expect(result.errors).toHaveLength(1); const error = result.errors[0]; @@ -620,6 +704,7 @@ toolName = "not_shell" commandRegex = ".*" decision = "allow" priority = 100 +modes = ["default"] `); expect(getErrors(result)).toHaveLength(1); const error = getErrors(result)[0]; @@ -635,6 +720,7 @@ commandPrefix = "git" commandRegex = ".*" decision = "allow" priority = 100 +modes = ["default"] `); expect(result.errors).toHaveLength(1); const error = result.errors[0]; @@ -649,6 +735,7 @@ toolName = "test" argsPattern = "([a-z)" decision = "allow" priority = 100 +modes = ["default"] `); expect(getErrors(result)).toHaveLength(1); const error = getErrors(result)[0]; @@ -660,7 +747,7 @@ priority = 100 const filePath = path.join(tempDir, 'single-rule.toml'); await fs.writeFile( filePath, - '[[rule]]\ntoolName = "test-tool"\ndecision = "allow"\npriority = 500\n', + '[[rule]]\ntoolName = "test-tool"\ndecision = "allow"\npriority = 500\nmodes = ["default"]\n', ); const getPolicyTier = (_dir: string) => 1; @@ -693,6 +780,7 @@ priority = 100 toolName = "grob" decision = "allow" priority = 100 +modes = ["plan", "default", "autoEdit", "yolo"] `); const warnings = getWarnings(result); @@ -712,11 +800,13 @@ priority = 100 toolName = "glob" decision = "allow" priority = 100 +modes = ["plan", "default", "autoEdit", "yolo"] [[rule]] toolName = "read_file" decision = "allow" priority = 100 +modes = ["plan", "default", "autoEdit", "yolo"] `); expect(getWarnings(result)).toHaveLength(0); @@ -730,6 +820,7 @@ priority = 100 toolName = "*" decision = "allow" priority = 100 +modes = ["plan", "default", "autoEdit", "yolo"] `); expect(getWarnings(result)).toHaveLength(0); @@ -742,11 +833,13 @@ priority = 100 toolName = "mcp_my-server_my-tool" decision = "allow" priority = 100 +modes = ["plan", "default", "autoEdit", "yolo"] [[rule]] toolName = "mcp_my-server_*" decision = "allow" priority = 100 +modes = ["plan", "default", "autoEdit", "yolo"] `); expect(getWarnings(result)).toHaveLength(0); @@ -760,6 +853,7 @@ mcpName = "my-server" toolName = "nonexistent" decision = "allow" priority = 100 +modes = ["plan", "default", "autoEdit", "yolo"] `); expect(getWarnings(result)).toHaveLength(0); @@ -772,6 +866,7 @@ priority = 100 toolName = "search_file_content" decision = "allow" priority = 100 +modes = ["plan", "default", "autoEdit", "yolo"] `); expect(getWarnings(result)).toHaveLength(0); @@ -784,6 +879,7 @@ priority = 100 toolName = "discovered_tool_my_custom_tool" decision = "allow" priority = 100 +modes = ["plan", "default", "autoEdit", "yolo"] `); expect(getWarnings(result)).toHaveLength(0); @@ -796,6 +892,7 @@ priority = 100 toolName = ["grob", "glob", "replce"] decision = "allow" priority = 100 +modes = ["plan", "default", "autoEdit", "yolo"] `); const warnings = getWarnings(result); @@ -812,11 +909,13 @@ priority = 100 toolName = "delegate_to_agent" decision = "allow" priority = 100 +modes = ["plan", "default", "autoEdit", "yolo"] [[rule]] toolName = "my_custom_tool" decision = "allow" priority = 100 +modes = ["plan", "default", "autoEdit", "yolo"] `); expect(getWarnings(result)).toHaveLength(0); @@ -830,6 +929,7 @@ priority = 100 toolName = "*" decision = "deny" priority = 100 +modes = ["plan", "default", "autoEdit", "yolo"] `); expect(getWarnings(result)).toHaveLength(0); @@ -843,11 +943,13 @@ priority = 100 toolName = "wrte_file" decision = "deny" priority = 50 +modes = ["plan", "default", "autoEdit", "yolo"] [[rule]] toolName = "glob" decision = "allow" priority = 100 +modes = ["plan", "default", "autoEdit", "yolo"] `); expect(getWarnings(result)).toHaveLength(1); @@ -1000,6 +1102,7 @@ priority = 100 decision: PolicyDecision.ALLOW, priority: PRIORITY_SUBAGENT_TOOL, source: 'AgentRegistry (Dynamic)', + modes: MODES_BY_PERMISSIVENESS, }); // 4. Verify Behavior: diff --git a/packages/core/src/policy/toml-loader.ts b/packages/core/src/policy/toml-loader.ts index 977e8a399a..9a6976c667 100644 --- a/packages/core/src/policy/toml-loader.ts +++ b/packages/core/src/policy/toml-loader.ts @@ -60,7 +60,7 @@ const PolicyRuleSchema = z.object({ message: 'priority must be <= 999 to prevent tier overflow. Priorities >= 1000 would jump to the next tier.', }), - modes: z.array(z.nativeEnum(ApprovalMode)).optional(), + modes: z.array(z.nativeEnum(ApprovalMode)), interactive: z.boolean().optional(), toolAnnotations: z.record(z.any()).optional(), allowRedirection: z.boolean().optional(), @@ -79,7 +79,7 @@ const SafetyCheckerRuleSchema = z.object({ commandPrefix: z.union([z.string(), z.array(z.string())]).optional(), commandRegex: z.string().optional(), priority: z.number().int().default(0), - modes: z.array(z.nativeEnum(ApprovalMode)).optional(), + modes: z.array(z.nativeEnum(ApprovalMode)), toolAnnotations: z.record(z.any()).optional(), checker: z.discriminatedUnion('type', [ z.object({ @@ -383,7 +383,7 @@ export async function loadPoliciesFromToml( message: 'Schema validation failed', details: formatSchemaError(validationResult.error, 0), suggestion: - 'Ensure all required fields (decision, priority) are present with correct types', + 'Ensure all required fields are present with correct types', }); continue; } diff --git a/packages/core/src/policy/types.ts b/packages/core/src/policy/types.ts index 622cde0abd..8d44dfbf12 100644 --- a/packages/core/src/policy/types.ts +++ b/packages/core/src/policy/types.ts @@ -161,9 +161,8 @@ export interface PolicyRule { /** * Approval modes this rule applies to. - * If undefined or empty, it applies to all modes. */ - modes?: ApprovalMode[]; + modes: ApprovalMode[]; /** * If true, this rule only applies to interactive environments. @@ -230,9 +229,8 @@ export interface SafetyCheckerRule { /** * Approval modes this rule applies to. - * If undefined or empty, it applies to all modes. */ - modes?: ApprovalMode[]; + modes: ApprovalMode[]; /** * Source of the rule. diff --git a/packages/core/src/scheduler/policy.test.ts b/packages/core/src/scheduler/policy.test.ts index acea3d3ab6..ae42032376 100644 --- a/packages/core/src/scheduler/policy.test.ts +++ b/packages/core/src/scheduler/policy.test.ts @@ -21,7 +21,11 @@ import { MessageBusType, type SerializableConfirmationDetails, } from '../confirmation-bus/types.js'; -import { ApprovalMode, PolicyDecision } from '../policy/types.js'; +import { + ApprovalMode, + PolicyDecision, + MODES_BY_PERMISSIVENESS, +} from '../policy/types.js'; import { escapeRegex } from '../policy/utils.js'; import { ToolConfirmationOutcome, @@ -786,6 +790,7 @@ describe('policy.ts', () => { toolName: '*', decision: PolicyDecision.DENY, denyMessage: 'Custom Deny', + modes: MODES_BY_PERMISSIVENESS, }; const { errorMessage, errorType } = getPolicyDenialError( diff --git a/packages/core/src/scheduler/scheduler.test.ts b/packages/core/src/scheduler/scheduler.test.ts index 54562933a8..1fd66a1263 100644 --- a/packages/core/src/scheduler/scheduler.test.ts +++ b/packages/core/src/scheduler/scheduler.test.ts @@ -68,7 +68,11 @@ import type { Config } from '../config/config.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; import type { PolicyEngine } from '../policy/policy-engine.js'; import type { ToolRegistry } from '../tools/tool-registry.js'; -import { PolicyDecision, ApprovalMode } from '../policy/types.js'; +import { + PolicyDecision, + ApprovalMode, + MODES_BY_PERMISSIVENESS, +} from '../policy/types.js'; import { ToolConfirmationOutcome, type AnyDeclarativeTool, @@ -684,6 +688,7 @@ describe('Scheduler (Orchestrator)', () => { toolName: '*', decision: PolicyDecision.DENY, denyMessage: 'Custom denial reason', + modes: MODES_BY_PERMISSIVENESS, }, }); @@ -757,7 +762,11 @@ describe('Scheduler (Orchestrator)', () => { it('should return POLICY_VIOLATION error type when denied in Plan Mode', async () => { vi.mocked(checkPolicy).mockResolvedValue({ decision: PolicyDecision.DENY, - rule: { toolName: '*', decision: PolicyDecision.DENY }, + rule: { + toolName: '*', + decision: PolicyDecision.DENY, + modes: MODES_BY_PERMISSIVENESS, + }, }); mockConfig.getApprovalMode.mockReturnValue(ApprovalMode.PLAN); @@ -790,6 +799,7 @@ describe('Scheduler (Orchestrator)', () => { toolName: '*', decision: PolicyDecision.DENY, denyMessage: customMessage, + modes: MODES_BY_PERMISSIVENESS, }, }); diff --git a/packages/core/src/services/memoryService.ts b/packages/core/src/services/memoryService.ts index 495cbdc5ef..0398615620 100644 --- a/packages/core/src/services/memoryService.ts +++ b/packages/core/src/services/memoryService.ts @@ -23,7 +23,7 @@ import { ExecutionLifecycleService } from './executionLifecycleService.js'; import { PromptRegistry } from '../prompts/prompt-registry.js'; import { ResourceRegistry } from '../resources/resource-registry.js'; import { PolicyEngine } from '../policy/policy-engine.js'; -import { PolicyDecision } from '../policy/types.js'; +import { PolicyDecision, ApprovalMode } from '../policy/types.js'; import { MessageBus } from '../confirmation-bus/message-bus.js'; import { Storage } from '../config/storage.js'; import type { AgentLoopContext } from '../config/agent-loop-context.js'; @@ -475,6 +475,7 @@ function buildAgentLoopContext(config: Config): AgentLoopContext { toolName: '*', decision: PolicyDecision.ALLOW, priority: 100, + modes: [ApprovalMode.DEFAULT], }, ], }); diff --git a/packages/core/src/utils/extensionLoader.test.ts b/packages/core/src/utils/extensionLoader.test.ts index 415cec1543..c674b2b82f 100644 --- a/packages/core/src/utils/extensionLoader.test.ts +++ b/packages/core/src/utils/extensionLoader.test.ts @@ -14,7 +14,7 @@ import { type MockInstance, } from 'vitest'; import { SimpleExtensionLoader } from './extensionLoader.js'; -import { PolicyDecision } from '../policy/types.js'; +import { PolicyDecision, MODES_BY_PERMISSIVENESS } from '../policy/types.js'; import type { Config, GeminiCLIExtension } from '../config/config.js'; import { type McpClientManager } from '../tools/mcp-client-manager.js'; import type { GeminiClient } from '../core/client.js'; @@ -59,12 +59,14 @@ describe('SimpleExtensionLoader', () => { toolName: 'test-tool', decision: PolicyDecision.ALLOW, source: 'Extension (test-extension): policies.toml', + modes: MODES_BY_PERMISSIVENESS, }, ], checkers: [ { toolName: 'test-tool', checker: { type: 'external', name: 'test-checker' }, + modes: MODES_BY_PERMISSIVENESS, source: 'Extension (test-extension): policies.toml', }, ], diff --git a/packages/core/src/utils/googleErrors.ts b/packages/core/src/utils/googleErrors.ts index bcb57425b3..c4ddd0a879 100644 --- a/packages/core/src/utils/googleErrors.ts +++ b/packages/core/src/utils/googleErrors.ts @@ -20,7 +20,7 @@ function sanitizeJsonString(jsonStr: string): string { // Match a comma, optional whitespace/newlines, then another comma. // Replace with just a comma + the captured whitespace. - // Loop to handle cases like `,,,` which would otherwise become `,,` on a single pass. + // Loop to handle cases like `,,` which would otherwise become `,` on a single pass. let prev: string; do { prev = jsonStr;