From 150fcbffc12518c00e04c2e4dafb914a81b4eb31 Mon Sep 17 00:00:00 2001 From: Abhijit Balaji Date: Wed, 11 Feb 2026 22:48:12 -0800 Subject: [PATCH 1/2] feat(core): require explicit modes for policy tool exclusion Policy rules must now explicitly specify 'modes' to be considered by the PolicyEngine for excluding tools from the model. This prevents accidental exclusions from rules intended for different approval modes. Also explicitly denies 'run_shell_command' in Plan mode for enhanced security. --- packages/core/src/policy/policies/plan.toml | 9 +++ .../core/src/policy/policy-engine.test.ts | 67 ++++++++++++++++--- packages/core/src/policy/policy-engine.ts | 3 + 3 files changed, 71 insertions(+), 8 deletions(-) diff --git a/packages/core/src/policy/policies/plan.toml b/packages/core/src/policy/policies/plan.toml index 656c1008456..2be609f209e 100644 --- a/packages/core/src/policy/policies/plan.toml +++ b/packages/core/src/policy/policies/plan.toml @@ -26,6 +26,7 @@ # 999: YOLO mode allow-all (becomes 1.999 in default tier) # Catch-All: Deny everything by default in Plan mode. +# Catch-All Deny works only for tool names that are set with low priority approve and are explicitly part of "plan" mode [[rule]] decision = "deny" @@ -33,6 +34,13 @@ priority = 60 modes = ["plan"] deny_message = "You are in Plan Mode with access to read-only tools. Execution of scripts (including those from skills) is blocked." +# Deny run_shell_command in Plan mode. +[[rule]] +toolName = ["run_shell_command"] +decision = "deny" +priority = 100 +modes = ["plan"] + # Explicitly Allow Read-Only Tools in Plan mode. [[rule]] @@ -54,3 +62,4 @@ decision = "allow" priority = 70 modes = ["plan"] argsPattern = "\"file_path\":\"[^\"]+/\\.gemini/tmp/[a-zA-Z0-9_-]+/plans/[a-zA-Z0-9_-]+\\.md\"" + diff --git a/packages/core/src/policy/policy-engine.test.ts b/packages/core/src/policy/policy-engine.test.ts index 26aecaa1ebf..ec57b75aaca 100644 --- a/packages/core/src/policy/policy-engine.test.ts +++ b/packages/core/src/policy/policy-engine.test.ts @@ -2046,33 +2046,72 @@ describe('PolicyEngine', () => { rules: [], expected: [], }, + { + name: 'should ignore rules without explicit modes', + rules: [{ toolName: 'tool1', decision: PolicyDecision.DENY }], + expected: [], + }, { name: 'should include tools with DENY decision', rules: [ - { toolName: 'tool1', decision: PolicyDecision.DENY }, - { toolName: 'tool2', decision: PolicyDecision.ALLOW }, + { + toolName: 'tool1', + decision: PolicyDecision.DENY, + modes: [ApprovalMode.DEFAULT], + }, + { + toolName: 'tool2', + decision: PolicyDecision.ALLOW, + modes: [ApprovalMode.DEFAULT], + }, ], expected: ['tool1'], }, { name: 'should respect priority and ignore lower priority rules (DENY wins)', rules: [ - { toolName: 'tool1', decision: PolicyDecision.DENY, priority: 100 }, - { toolName: 'tool1', decision: PolicyDecision.ALLOW, priority: 10 }, + { + toolName: 'tool1', + decision: PolicyDecision.DENY, + priority: 100, + modes: [ApprovalMode.DEFAULT], + }, + { + toolName: 'tool1', + decision: PolicyDecision.ALLOW, + priority: 10, + modes: [ApprovalMode.DEFAULT], + }, ], expected: ['tool1'], }, { name: 'should respect priority and ignore lower priority rules (ALLOW wins)', rules: [ - { toolName: 'tool1', decision: PolicyDecision.ALLOW, priority: 100 }, - { toolName: 'tool1', decision: PolicyDecision.DENY, priority: 10 }, + { + toolName: 'tool1', + decision: PolicyDecision.ALLOW, + priority: 100, + modes: [ApprovalMode.DEFAULT], + }, + { + toolName: 'tool1', + decision: PolicyDecision.DENY, + priority: 10, + modes: [ApprovalMode.DEFAULT], + }, ], expected: [], }, { name: 'should NOT include ASK_USER tools even in non-interactive mode', - rules: [{ toolName: 'tool1', decision: PolicyDecision.ASK_USER }], + rules: [ + { + toolName: 'tool1', + decision: PolicyDecision.ASK_USER, + modes: [ApprovalMode.DEFAULT], + }, + ], nonInteractive: true, expected: [], }, @@ -2083,6 +2122,7 @@ describe('PolicyEngine', () => { toolName: 'tool1', decision: PolicyDecision.DENY, argsPattern: /something/, + modes: [ApprovalMode.DEFAULT], }, ], expected: [], @@ -2123,6 +2163,7 @@ describe('PolicyEngine', () => { toolName: 'dangerous-tool', decision: PolicyDecision.DENY, priority: 10, + modes: [ApprovalMode.YOLO], }, ], approvalMode: ApprovalMode.YOLO, @@ -2130,7 +2171,13 @@ describe('PolicyEngine', () => { }, { name: 'should respect server wildcard DENY', - rules: [{ toolName: 'server__*', decision: PolicyDecision.DENY }], + rules: [ + { + toolName: 'server__*', + decision: PolicyDecision.DENY, + modes: [ApprovalMode.DEFAULT], + }, + ], expected: ['server__*'], }, { @@ -2140,11 +2187,13 @@ describe('PolicyEngine', () => { toolName: 'server__*', decision: PolicyDecision.DENY, priority: 100, + modes: [ApprovalMode.DEFAULT], }, { toolName: 'server__tool1', decision: PolicyDecision.DENY, priority: 10, + modes: [ApprovalMode.DEFAULT], }, ], expected: ['server__*', 'server__tool1'], @@ -2156,11 +2205,13 @@ describe('PolicyEngine', () => { toolName: 'server__*', decision: PolicyDecision.ALLOW, priority: 100, + modes: [ApprovalMode.DEFAULT], }, { toolName: 'server__tool1', decision: PolicyDecision.DENY, priority: 10, + modes: [ApprovalMode.DEFAULT], }, ], expected: [], diff --git a/packages/core/src/policy/policy-engine.ts b/packages/core/src/policy/policy-engine.ts index 1fc5e7cde52..4d8aa9959ba 100644 --- a/packages/core/src/policy/policy-engine.ts +++ b/packages/core/src/policy/policy-engine.ts @@ -548,6 +548,9 @@ export class PolicyEngine { if (!rule.modes.includes(this.approvalMode)) { continue; } + } else { + // If no mode is given we shouldn't exclude the tool from the model + continue; } // Handle Global Rules From 2b50bfc922bbd99cc90692c6de4d9440611cc8a3 Mon Sep 17 00:00:00 2001 From: Abhijit Balaji Date: Thu, 12 Feb 2026 08:31:35 -0800 Subject: [PATCH 2/2] fix(core): prioritize conditional policy rules over global exclusions Ensure tools with argsPattern (conditional rules) are marked as processed so they aren't hidden by lower-priority global DENY rules. Fixes an issue in Plan Mode where conditional allows for write_file were being blocked by the global catch-all deny. Treat policy rules without explicit modes as applying to ALL modes (implicit default) for consistency and safety. Remove redundant explicit deny for run_shell_command in Plan Mode as the catch-all now correctly handles it. --- packages/core/src/policy/policies/plan.toml | 9 ---- .../core/src/policy/policy-engine.test.ts | 48 ++++++++++++++++++- packages/core/src/policy/policy-engine.ts | 7 ++- 3 files changed, 50 insertions(+), 14 deletions(-) diff --git a/packages/core/src/policy/policies/plan.toml b/packages/core/src/policy/policies/plan.toml index 2be609f209e..656c1008456 100644 --- a/packages/core/src/policy/policies/plan.toml +++ b/packages/core/src/policy/policies/plan.toml @@ -26,7 +26,6 @@ # 999: YOLO mode allow-all (becomes 1.999 in default tier) # Catch-All: Deny everything by default in Plan mode. -# Catch-All Deny works only for tool names that are set with low priority approve and are explicitly part of "plan" mode [[rule]] decision = "deny" @@ -34,13 +33,6 @@ priority = 60 modes = ["plan"] deny_message = "You are in Plan Mode with access to read-only tools. Execution of scripts (including those from skills) is blocked." -# Deny run_shell_command in Plan mode. -[[rule]] -toolName = ["run_shell_command"] -decision = "deny" -priority = 100 -modes = ["plan"] - # Explicitly Allow Read-Only Tools in Plan mode. [[rule]] @@ -62,4 +54,3 @@ decision = "allow" priority = 70 modes = ["plan"] argsPattern = "\"file_path\":\"[^\"]+/\\.gemini/tmp/[a-zA-Z0-9_-]+/plans/[a-zA-Z0-9_-]+\\.md\"" - diff --git a/packages/core/src/policy/policy-engine.test.ts b/packages/core/src/policy/policy-engine.test.ts index ec57b75aaca..693ae3a4b2a 100644 --- a/packages/core/src/policy/policy-engine.test.ts +++ b/packages/core/src/policy/policy-engine.test.ts @@ -2047,8 +2047,27 @@ describe('PolicyEngine', () => { expected: [], }, { - name: 'should ignore rules without explicit modes', + name: 'should apply rules without explicit modes to all modes', rules: [{ toolName: 'tool1', decision: PolicyDecision.DENY }], + expected: ['tool1'], + }, + { + name: 'should NOT exclude tool if higher priority argsPattern rule exists', + rules: [ + { + toolName: 'tool1', + decision: PolicyDecision.ALLOW, + argsPattern: /safe/, + priority: 100, + modes: [ApprovalMode.DEFAULT], + }, + { + toolName: 'tool1', + decision: PolicyDecision.DENY, + priority: 10, + modes: [ApprovalMode.DEFAULT], + }, + ], expected: [], }, { @@ -2198,6 +2217,33 @@ describe('PolicyEngine', () => { ], expected: ['server__*', 'server__tool1'], }, + { + name: 'should exclude run_shell_command but NOT write_file in simulated Plan Mode', + approvalMode: ApprovalMode.PLAN, + rules: [ + { + // Simulates the high-priority allow for plans directory + toolName: 'write_file', + decision: PolicyDecision.ALLOW, + priority: 70, + argsPattern: /plans/, + modes: [ApprovalMode.PLAN], + }, + { + // Simulates the global deny in Plan Mode + decision: PolicyDecision.DENY, + priority: 60, + modes: [ApprovalMode.PLAN], + }, + { + // Simulates a tool from another policy (e.g. write.toml) + toolName: 'run_shell_command', + decision: PolicyDecision.ASK_USER, + priority: 10, + }, + ], + expected: ['run_shell_command'], + }, { name: 'should NOT exclude tool if covered by a higher priority wildcard ALLOW', rules: [ diff --git a/packages/core/src/policy/policy-engine.ts b/packages/core/src/policy/policy-engine.ts index 4d8aa9959ba..3f386edd8f2 100644 --- a/packages/core/src/policy/policy-engine.ts +++ b/packages/core/src/policy/policy-engine.ts @@ -538,8 +538,10 @@ export class PolicyEngine { let globalVerdict: PolicyDecision | undefined; for (const rule of this.rules) { - // We only care about rules without args pattern for exclusion from the model if (rule.argsPattern) { + if (rule.toolName && rule.decision !== PolicyDecision.DENY) { + processedTools.add(rule.toolName); + } continue; } @@ -548,9 +550,6 @@ export class PolicyEngine { if (!rule.modes.includes(this.approvalMode)) { continue; } - } else { - // If no mode is given we shouldn't exclude the tool from the model - continue; } // Handle Global Rules