fix(core): prioritize conditional policy rules and harden Plan Mode (#18882)

This commit is contained in:
Abhijit Balaji
2026-02-12 09:04:39 -08:00
committed by GitHub
parent f603f4a12b
commit ddcfe5b1f2
2 changed files with 108 additions and 9 deletions

View File

@@ -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: [],

View File

@@ -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;
}