diff --git a/packages/cli/src/config/extension.test.ts b/packages/cli/src/config/extension.test.ts index ef7e61cf25..b03b83e0ff 100644 --- a/packages/cli/src/config/extension.test.ts +++ b/packages/cli/src/config/extension.test.ts @@ -409,11 +409,13 @@ describe('extension tests', () => { toolName = "deny_tool" decision = "deny" priority = 500 +modes = ["plan", "default", "autoEdit"] [[rule]] toolName = "ask_tool" decision = "ask_user" priority = 100 +modes = ["plan", "default", "autoEdit"] `; fs.writeFileSync( path.join(policiesDir, 'policies.toml'), @@ -454,6 +456,7 @@ priority = 100 toolName = "allow_tool" decision = "allow" priority = 100 +modes = ["plan", "default", "autoEdit"] [[rule]] toolName = "yolo_tool" diff --git a/packages/cli/src/config/policy-engine.integration.test.ts b/packages/cli/src/config/policy-engine.integration.test.ts index edc06bfbf0..dcf681047a 100644 --- a/packages/cli/src/config/policy-engine.integration.test.ts +++ b/packages/cli/src/config/policy-engine.integration.test.ts @@ -7,6 +7,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { ApprovalMode, + MODES_BY_PERMISSIVENESS, PolicyDecision, PolicyEngine, } from '@google/gemini-cli-core'; @@ -385,6 +386,7 @@ describe('Policy Engine Integration Tests', () => { toolAnnotations: { readOnlyHint: true }, decision: PolicyDecision.ALLOW, priority: 10, + modes: MODES_BY_PERMISSIVENESS, }); const engine = new PolicyEngine(config); diff --git a/packages/cli/src/test-utils/AppRig.tsx b/packages/cli/src/test-utils/AppRig.tsx index 548372a139..96899722e9 100644 --- a/packages/cli/src/test-utils/AppRig.tsx +++ b/packages/cli/src/test-utils/AppRig.tsx @@ -23,6 +23,7 @@ import { ExtensionLoader, AuthType, ApprovalMode, + MODES_BY_PERMISSIVENESS, createPolicyEngineConfig, PolicyDecision, ToolConfirmationOutcome, @@ -452,6 +453,7 @@ export class AppRig { toolName, decision, priority, + modes: MODES_BY_PERMISSIVENESS, source: 'AppRig Override', }); } diff --git a/packages/core/src/policy/config.test.ts b/packages/core/src/policy/config.test.ts index 7e39fe41dd..01524e2954 100644 --- a/packages/core/src/policy/config.test.ts +++ b/packages/core/src/policy/config.test.ts @@ -326,7 +326,8 @@ describe('createPolicyEngineConfig', () => { (r) => r.toolName === 'replace' && r.decision === PolicyDecision.ALLOW && - r.modes?.includes(ApprovalMode.AUTO_EDIT), + r.modes?.includes(ApprovalMode.AUTO_EDIT) && + !r.argsPattern, ); expect(rule).toBeDefined(); expect(rule?.priority).toBeCloseTo(1.015, 5); @@ -421,7 +422,7 @@ describe('createPolicyEngineConfig', () => { it('should handle complex priority scenarios correctly', async () => { mockPolicyFile( nodePath.join(MOCK_DEFAULT_DIR, 'default.toml'), - '[[rule]]\ntoolName = "glob"\ndecision = "allow"\npriority = 50\n', + '[[rule]]\ntoolName = "glob"\ndecision = "allow"\npriority = 50\nmodes = ["default", "plan", "autoEdit", "yolo"]\n', ); const settings: PolicySettings = { @@ -543,6 +544,7 @@ describe('createPolicyEngineConfig', () => { argsPattern = "\\"command\\":\\"git (status|diff|log)\\"" decision = "allow" priority = 150 + modes = ["default", "plan", "autoEdit", "yolo"] `, ); @@ -573,10 +575,12 @@ describe('createPolicyEngineConfig', () => { toolName = "write_file" decision = "allow" priority = 10 +modes = ["default", "plan", "autoEdit", "yolo"] [[safety_checker]] toolName = "write_file" priority = 10 +modes = ["default", "plan", "autoEdit", "yolo"] [safety_checker.checker] type = "in-process" name = "allowed-path" @@ -610,10 +614,12 @@ required_context = ["environment"] toolName = "write_file" decision = "allow" priority = 10 +modes = ["default", "plan", "autoEdit", "yolo"] [[safety_checker]] toolName = "write_file" priority = 10 +modes = ["default", "plan", "autoEdit", "yolo"] [safety_checker.checker] type = "in-process" name = "invalid-name" @@ -639,6 +645,7 @@ name = "invalid-name" mcpName = "my-server" decision = "allow" priority = 150 + modes = ["default", "plan", "autoEdit", "yolo"] `, ); diff --git a/packages/core/src/policy/config.ts b/packages/core/src/policy/config.ts index b1ec622d9f..48ef8015e9 100644 --- a/packages/core/src/policy/config.ts +++ b/packages/core/src/policy/config.ts @@ -584,6 +584,7 @@ function createTomlRule(toolName: string, message: UpdatePolicy): TomlRule { decision: 'allow', priority: getAlwaysAllowPriorityFraction(), toolName, + modes: message.modes ?? MODES_BY_PERMISSIVENESS, }; if (message.mcpName) { @@ -600,10 +601,6 @@ function createTomlRule(toolName: string, message: UpdatePolicy): TomlRule { rule.allowRedirection = message.allowRedirection; } - if (message.modes) { - rule.modes = message.modes; - } - return rule; } diff --git a/packages/core/src/policy/policies/write.toml b/packages/core/src/policy/policies/write.toml index 8ebc04749a..5fefb85c10 100644 --- a/packages/core/src/policy/policies/write.toml +++ b/packages/core/src/policy/policies/write.toml @@ -38,7 +38,7 @@ interactive = true toolName = "replace" decision = "allow" priority = 15 -modes = ["plan", "default", "autoEdit", "yolo"] +modes = ["autoEdit", "yolo"] [rule.safety_checker] type = "in-process" @@ -63,7 +63,7 @@ interactive = true toolName = "write_file" decision = "ask_user" priority = 10 -modes = ["plan", "default", "autoEdit", "yolo"] +modes = ["plan", "default"] interactive = true [[rule]] @@ -77,7 +77,7 @@ interactive = true toolName = "write_file" decision = "allow" priority = 15 -modes = ["plan", "default", "autoEdit", "yolo"] +modes = ["autoEdit", "yolo"] [rule.safety_checker] type = "in-process" @@ -88,13 +88,13 @@ required_context = ["environment"] toolName = "web_fetch" decision = "allow" priority = 15 -modes = ["plan", "default", "autoEdit", "yolo"] +modes = ["autoEdit", "yolo"] [[rule]] toolName = "web_fetch" decision = "ask_user" priority = 10 -modes = ["plan", "default", "autoEdit", "yolo"] +modes = ["plan", "default"] interactive = true # Headless Denial Rule (Priority 10) @@ -110,5 +110,5 @@ toolName = [ ] decision = "deny" priority = 10 -modes = ["plan", "default", "autoEdit", "yolo"] +modes = ["plan", "default"] interactive = false diff --git a/packages/core/src/policy/policy-engine.ts b/packages/core/src/policy/policy-engine.ts index 99c61edd2e..90df1996aa 100644 --- a/packages/core/src/policy/policy-engine.ts +++ b/packages/core/src/policy/policy-engine.ts @@ -89,8 +89,10 @@ function ruleMatches( toolAnnotations?: Record, subagent?: string, ): boolean { - // Check if rule applies to current approval mode - if (!rule.modes.includes(currentApprovalMode)) { + // Check if rule applies to current approval mode. + // Legacy rules or those injected manually in tests might not have the modes field. + // If so, we default to matching all modes for backward compatibility. + if (rule.modes && !rule.modes.includes(currentApprovalMode)) { return false; } // Check subagent if specified (only for PolicyRule, SafetyCheckerRule doesn't have it) @@ -568,7 +570,7 @@ export class PolicyEngine { if (match) { debugLogger.debug( - `[PolicyEngine.check] MATCHED rule: toolName=${rule.toolName}, decision=${rule.decision}, priority=${rule.priority}, argsPattern=${rule.argsPattern?.source || 'none'}`, + `[PolicyEngine.check] MATCHED rule: toolName=${rule.toolName}, decision=${rule.decision}, priority=${rule.priority}, source=${rule.source}, argsPattern=${rule.argsPattern?.source || 'none'}`, ); let ruleDecision = rule.decision; diff --git a/packages/core/src/policy/workspace-policy.test.ts b/packages/core/src/policy/workspace-policy.test.ts index 0a277bc072..211ed15173 100644 --- a/packages/core/src/policy/workspace-policy.test.ts +++ b/packages/core/src/policy/workspace-policy.test.ts @@ -91,6 +91,7 @@ describe('Workspace-Level Policies', () => { toolName = "test_tool" decision = "allow" priority = 10 +modes = ["plan", "default", "autoEdit", "yolo"] `; // Tier 1 -> 1.010 } if (path.includes('user.toml')) { @@ -98,6 +99,7 @@ priority = 10 toolName = "test_tool" decision = "deny" priority = 10 +modes = ["plan", "default", "autoEdit", "yolo"] `; // Tier 4 -> 4.010 } if (path.includes('workspace.toml')) { @@ -105,6 +107,7 @@ priority = 10 toolName = "test_tool" decision = "allow" priority = 10 +modes = ["plan", "default", "autoEdit", "yolo"] `; // Tier 3 -> 3.010 } if (path.includes('admin.toml')) { @@ -112,6 +115,7 @@ priority = 10 toolName = "test_tool" decision = "deny" priority = 10 +modes = ["plan", "default", "autoEdit", "yolo"] `; // Tier 5 -> 5.010 } return ''; @@ -194,7 +198,8 @@ priority = 10 async () => `[[rule]] toolName="t" decision="allow" -priority=10`, +priority=10 +modes = ["plan", "default", "autoEdit", "yolo"]`, ); vi.doMock('node:fs/promises', () => ({ @@ -259,7 +264,8 @@ priority=10`, async () => `[[rule]] toolName="p_tool" decision="allow" -priority=500`, +priority=500 +modes = ["plan", "default", "autoEdit", "yolo"]`, ); vi.doMock('node:fs/promises', () => ({