Requiring mode when injecting policy to prevent allowing all modes by default

This commit is contained in:
Keith Schaab
2026-04-06 15:44:51 +00:00
parent 6fb58bd31f
commit b96a26b4ef
24 changed files with 558 additions and 82 deletions
@@ -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,
}),
);
});
@@ -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)
);
}
+4 -1
View File
@@ -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,
}),
);
});
+6 -1
View File
@@ -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)',
});
}
+10 -2
View File
@@ -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,
});
@@ -1,6 +1,7 @@
[[safety_checker]]
toolName = "*"
priority = 100
modes = ["plan", "default", "autoEdit", "yolo"]
[safety_checker.checker]
type = "in-process"
name = "conseca"
@@ -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
@@ -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."
@@ -4,4 +4,5 @@
toolName = "ask_user"
decision = "deny"
priority = 999
modes = ["plan", "default", "autoEdit", "yolo"]
interactive = false
@@ -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."
@@ -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
priority = 50
modes = ["plan", "default", "autoEdit", "yolo"]
@@ -32,3 +32,4 @@ toolName = [
]
decision = "allow"
priority = 50
modes = ["plan", "default", "autoEdit", "yolo"]
+10 -3
View File
@@ -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
File diff suppressed because it is too large Load Diff
+2 -5
View File
@@ -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) {
+17 -2
View File
@@ -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
+104 -1
View File
@@ -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:
+3 -3
View File
@@ -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;
}
+2 -4
View File
@@ -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.
+6 -1
View File
@@ -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(
+12 -2
View File
@@ -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,
},
});
+2 -1
View File
@@ -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],
},
],
});
@@ -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',
},
],
+1 -1
View File
@@ -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;