diff --git a/packages/core/src/agents/registry.test.ts b/packages/core/src/agents/registry.test.ts index 9eb43f357b..aa32d06bdd 100644 --- a/packages/core/src/agents/registry.test.ts +++ b/packages/core/src/agents/registry.test.ts @@ -26,6 +26,7 @@ import type { ConfigParameters } from '../config/config.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'; vi.mock('./agentLoader.js', () => ({ loadAgentsFromDirectory: vi @@ -657,6 +658,114 @@ describe('AgentRegistry', () => { await Promise.all(promises); expect(registry.getAllDefinitions()).toHaveLength(100); }); + + it('should dynamically register an ALLOW policy for local agents', async () => { + const agent: AgentDefinition = { + ...MOCK_AGENT_V1, + name: 'PolicyTestAgent', + }; + const policyEngine = mockConfig.getPolicyEngine(); + const addRuleSpy = vi.spyOn(policyEngine, 'addRule'); + + await registry.testRegisterAgent(agent); + + expect(addRuleSpy).toHaveBeenCalledWith( + expect.objectContaining({ + toolName: 'PolicyTestAgent', + decision: PolicyDecision.ALLOW, + priority: 1.05, + }), + ); + }); + + it('should dynamically register an ASK_USER policy for remote agents', async () => { + const remoteAgent: AgentDefinition = { + kind: 'remote', + name: 'RemotePolicyAgent', + description: 'A remote agent', + agentCardUrl: 'https://example.com/card', + inputConfig: { inputSchema: { type: 'object' } }, + }; + + vi.mocked(A2AClientManager.getInstance).mockReturnValue({ + loadAgent: vi.fn().mockResolvedValue({ name: 'RemotePolicyAgent' }), + } as unknown as A2AClientManager); + + const policyEngine = mockConfig.getPolicyEngine(); + const addRuleSpy = vi.spyOn(policyEngine, 'addRule'); + + await registry.testRegisterAgent(remoteAgent); + + expect(addRuleSpy).toHaveBeenCalledWith( + expect.objectContaining({ + toolName: 'RemotePolicyAgent', + decision: PolicyDecision.ASK_USER, + priority: 1.05, + }), + ); + }); + + it('should not register a policy if a USER policy already exists', async () => { + const agent: AgentDefinition = { + ...MOCK_AGENT_V1, + name: 'ExistingUserPolicyAgent', + }; + const policyEngine = mockConfig.getPolicyEngine(); + // Mock hasRuleForTool to return true when ignoreDynamic=true (simulating a user policy) + vi.spyOn(policyEngine, 'hasRuleForTool').mockImplementation( + (toolName, ignoreDynamic) => + toolName === 'ExistingUserPolicyAgent' && ignoreDynamic === true, + ); + const addRuleSpy = vi.spyOn(policyEngine, 'addRule'); + + await registry.testRegisterAgent(agent); + + expect(addRuleSpy).not.toHaveBeenCalled(); + }); + + it('should replace an existing dynamic policy when an agent is overwritten', async () => { + const localAgent: AgentDefinition = { + ...MOCK_AGENT_V1, + name: 'OverwrittenAgent', + }; + const remoteAgent: AgentDefinition = { + kind: 'remote', + name: 'OverwrittenAgent', + description: 'A remote agent', + agentCardUrl: 'https://example.com/card', + inputConfig: { inputSchema: { type: 'object' } }, + }; + + vi.mocked(A2AClientManager.getInstance).mockReturnValue({ + loadAgent: vi.fn().mockResolvedValue({ name: 'OverwrittenAgent' }), + } as unknown as A2AClientManager); + + const policyEngine = mockConfig.getPolicyEngine(); + const removeRuleSpy = vi.spyOn(policyEngine, 'removeRulesForTool'); + const addRuleSpy = vi.spyOn(policyEngine, 'addRule'); + + // 1. Register local + await registry.testRegisterAgent(localAgent); + expect(addRuleSpy).toHaveBeenLastCalledWith( + expect.objectContaining({ decision: PolicyDecision.ALLOW }), + ); + + // 2. Overwrite with remote + await registry.testRegisterAgent(remoteAgent); + + // Verify old dynamic rule was removed + expect(removeRuleSpy).toHaveBeenCalledWith( + 'OverwrittenAgent', + 'AgentRegistry (Dynamic)', + ); + // Verify new dynamic rule (remote -> ASK_USER) was added + expect(addRuleSpy).toHaveBeenLastCalledWith( + expect.objectContaining({ + toolName: 'OverwrittenAgent', + decision: PolicyDecision.ASK_USER, + }), + ); + }); }); describe('reload', () => { diff --git a/packages/core/src/agents/registry.ts b/packages/core/src/agents/registry.ts index cc91ffeeed..66a990f1db 100644 --- a/packages/core/src/agents/registry.ts +++ b/packages/core/src/agents/registry.ts @@ -21,6 +21,7 @@ import { type ModelConfig, ModelConfigService, } from '../services/modelConfigService.js'; +import { PolicyDecision } from '../policy/types.js'; /** * Returns the model config alias for a given agent definition. @@ -266,6 +267,39 @@ export class AgentRegistry { this.agents.set(mergedDefinition.name, mergedDefinition); this.registerModelConfigs(mergedDefinition); + this.addAgentPolicy(mergedDefinition); + } + + private addAgentPolicy(definition: AgentDefinition): void { + const policyEngine = this.config.getPolicyEngine(); + if (!policyEngine) { + return; + } + + // If the user has explicitly defined a policy for this tool, respect it. + // ignoreDynamic=true means we only check for rules NOT added by this registry. + if (policyEngine.hasRuleForTool(definition.name, true)) { + if (this.config.getDebugMode()) { + debugLogger.log( + `[AgentRegistry] User policy exists for '${definition.name}', skipping dynamic registration.`, + ); + } + return; + } + + // Clean up any old dynamic policy for this tool (e.g. if we are overwriting an agent) + policyEngine.removeRulesForTool(definition.name, 'AgentRegistry (Dynamic)'); + + // Add the new dynamic policy + policyEngine.addRule({ + toolName: definition.name, + decision: + definition.kind === 'local' + ? PolicyDecision.ALLOW + : PolicyDecision.ASK_USER, + priority: 1.05, + source: 'AgentRegistry (Dynamic)', + }); } private isAgentEnabled( @@ -342,6 +376,7 @@ export class AgentRegistry { ); } this.agents.set(definition.name, definition); + this.addAgentPolicy(definition); } catch (e) { debugLogger.warn( `[AgentRegistry] Error loading A2A agent "${definition.name}":`, diff --git a/packages/core/src/policy/policies/agent.toml b/packages/core/src/policy/policies/agent.toml deleted file mode 100644 index 6c9711e8c4..0000000000 --- a/packages/core/src/policy/policies/agent.toml +++ /dev/null @@ -1,27 +0,0 @@ -# Priority system for policy rules: -# - Higher priority numbers win over lower priority numbers -# - When multiple rules match, the highest priority rule is applied -# - Rules are evaluated in order of priority (highest first) -# -# Priority bands (tiers): -# - Default policies (TOML): 1 + priority/1000 (e.g., priority 100 → 1.100) -# - User policies (TOML): 2 + priority/1000 (e.g., priority 100 → 2.100) -# - Admin policies (TOML): 3 + priority/1000 (e.g., priority 100 → 3.100) -# -# This ensures Admin > User > Default hierarchy is always preserved, -# while allowing user-specified priorities to work within each tier. -# -# Settings-based and dynamic rules (all in user tier 2.x): -# 2.95: Tools that the user has selected as "Always Allow" in the interactive UI -# 2.9: MCP servers excluded list (security: persistent server blocks) -# 2.4: Command line flag --exclude-tools (explicit temporary blocks) -# 2.3: Command line flag --allowed-tools (explicit temporary allows) -# 2.2: MCP servers with trust=true (persistent trusted servers) -# 2.1: MCP servers allowed list (persistent general server allows) -# -# TOML policy priorities (before transformation): -# 10: Write tools default to ASK_USER (becomes 1.010 in default tier) -# 15: Auto-edit tool override (becomes 1.015 in default tier) -# 50: Read-only tools (becomes 1.050 in default tier) -# 999: YOLO mode allow-all (becomes 1.999 in default tier) - diff --git a/packages/core/src/policy/policies/read-only.toml b/packages/core/src/policy/policies/read-only.toml index 5a669c28c7..5af7c9b1d4 100644 --- a/packages/core/src/policy/policies/read-only.toml +++ b/packages/core/src/policy/policies/read-only.toml @@ -49,8 +49,3 @@ priority = 50 toolName = "google_web_search" decision = "allow" priority = 50 - -[[rule]] -toolName = "SubagentInvocation" -decision = "allow" -priority = 50 diff --git a/packages/core/src/policy/policy-engine.ts b/packages/core/src/policy/policy-engine.ts index be5536a9df..656a17c109 100644 --- a/packages/core/src/policy/policy-engine.ts +++ b/packages/core/src/policy/policy-engine.ts @@ -439,9 +439,14 @@ export class PolicyEngine { /** * Remove rules for a specific tool. + * If source is provided, only rules matching that source are removed. */ - removeRulesForTool(toolName: string): void { - this.rules = this.rules.filter((rule) => rule.toolName !== toolName); + removeRulesForTool(toolName: string, source?: string): void { + this.rules = this.rules.filter( + (rule) => + rule.toolName !== toolName || + (source !== undefined && rule.source !== source), + ); } /** @@ -451,6 +456,18 @@ export class PolicyEngine { return this.rules; } + /** + * Check if a rule for a specific tool already exists. + * If ignoreDynamic is true, it only returns true if a rule exists that was NOT added by AgentRegistry. + */ + hasRuleForTool(toolName: string, ignoreDynamic = false): boolean { + return this.rules.some( + (rule) => + rule.toolName === toolName && + (!ignoreDynamic || rule.source !== 'AgentRegistry (Dynamic)'), + ); + } + getCheckers(): readonly SafetyCheckerRule[] { return this.checkers; }