diff --git a/packages/core/src/policy/policy-engine.test.ts b/packages/core/src/policy/policy-engine.test.ts index 26aecaa1eb..693ae3a4b2 100644 --- a/packages/core/src/policy/policy-engine.test.ts +++ b/packages/core/src/policy/policy-engine.test.ts @@ -2046,33 +2046,91 @@ describe('PolicyEngine', () => { rules: [], expected: [], }, + { + 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: [], + }, { 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 +2141,7 @@ describe('PolicyEngine', () => { toolName: 'tool1', decision: PolicyDecision.DENY, argsPattern: /something/, + modes: [ApprovalMode.DEFAULT], }, ], expected: [], @@ -2123,6 +2182,7 @@ describe('PolicyEngine', () => { toolName: 'dangerous-tool', decision: PolicyDecision.DENY, priority: 10, + modes: [ApprovalMode.YOLO], }, ], approvalMode: ApprovalMode.YOLO, @@ -2130,7 +2190,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,15 +2206,44 @@ 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'], }, + { + 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: [ @@ -2156,11 +2251,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 1fc5e7cde5..3f386edd8f 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; }