Core(fix)!: Requiring mode when setting policy to prevent all modes by default (breaking)

docs/cli/plan-mode.md

@@ -177,9 +177,9 @@ but you can customize these rules by creating your own policies in your
 #### Global vs. mode-specific rules
 
 As described in the
-[policy engine documentation](../reference/policy-engine.md#approval-modes), any
-rule that does not explicitly specify `modes` is considered "always active" and
-will apply to Plan Mode as well.
+[policy engine documentation](../reference/policy-engine.md#approval-modes),
+every rule in a TOML policy file must explicitly specify the `modes` it applies
+to.
 
 To maintain the integrity of Plan Mode as a safe research environment,
 persistent tool approvals are context-aware. Approvals granted in modes like

docs/extensions/reference.md

@@ -271,11 +271,13 @@ mcpName = "my_server"
 toolName = "dangerous_tool"
 decision = "ask_user"
 priority = 100
+modes = ["default", "autoEdit"]
 
 [[safety_checker]]
 mcpName = "my_server"
 toolName = "write_data"
 priority = 200
+modes = ["default", "autoEdit"]
 [safety_checker.checker]
 type = "in-process"
 name = "allowed-path"

docs/reference/policy-engine.md

@@ -32,6 +32,7 @@ To create your first policy:
     commandPrefix = "rm -rf"
     decision = "deny"
     priority = 100
+    modes = ["default"]
     ```
 3.  **Run a command** that triggers the policy (e.g., ask Gemini CLI to
     `rm -rf /`). The tool will now be blocked automatically.
@@ -54,14 +55,15 @@ A rule consists of the following main components:
   win.
 
 For example, this rule will ask for user confirmation before executing any `git`
-command.
+command in the default interactive mode.
 
 ```toml
 [[rule]]
 toolName = "run_shell_command"
 commandPrefix = "git"
 decision = "ask_user"
 priority = 100
+modes = ["default"]
 ```
 
 ### Conditions
@@ -158,10 +160,9 @@ For example:
 ### Approval modes
 
 Approval modes allow the policy engine to apply different sets of rules based on
-the CLI's operational mode. A rule in a TOML policy file can be associated with
-one or more modes (e.g., `yolo`, `autoEdit`, `plan`). The rule will only be
-active if the CLI is running in one of its specified modes. If a rule has no
-modes specified, it is always active.
+the CLI's operational mode. Every rule in a TOML policy file must be associated
+with one or more modes (for example, `yolo`, `autoEdit`, `plan`, `default`). The
+rule will only be active if the CLI is running in one of its specified modes.
 
 - `default`: The standard interactive mode where most write tools require
   confirmation.
@@ -321,8 +322,8 @@ priority = 10
 # useful for explaining *why* it was denied.
 denyMessage = "Deletion is permanent"
 
-# (Optional) An array of approval modes where this rule is active.
-# If omitted or empty, the rule applies to all modes.
+# An array of approval modes where this rule is active.
+# Valid values: "default", "autoEdit", "plan", "yolo".
 modes = ["default", "autoEdit", "yolo"]
 
 # (Optional) A boolean to restrict the rule to interactive (true) or
@@ -353,6 +354,7 @@ This single rule will apply to both the `write_file` and `replace` tools.
 toolName = ["write_file", "replace"]
 decision = "ask_user"
 priority = 10
+modes = ["default", "autoEdit"]
 ```
 
 ### Special syntax for `run_shell_command`
@@ -375,6 +377,7 @@ toolName = "run_shell_command"
 commandPrefix = "git"
 decision = "ask_user"
 priority = 100
+modes = ["default", "autoEdit"]
 ```
 
 ### Special syntax for MCP tools
@@ -406,6 +409,7 @@ mcpName = "my-jira-server"
 toolName = "search"
 decision = "allow"
 priority = 200
+modes = ["default", "autoEdit", "plan"]
 ```
 
 **2. Targeting all tools on a specific server**
@@ -422,6 +426,7 @@ mcpName = "untrusted-server"
 decision = "deny"
 priority = 500
 denyMessage = "This server is not trusted by the admin."
+modes = ["default", "autoEdit", "plan", "yolo"]
 ```
 
 **3. Targeting all MCP servers**
@@ -436,6 +441,7 @@ toolName = "*"
 mcpName = "*"
 decision = "ask_user"
 priority = 10
+modes = ["default", "autoEdit", "plan"]
 ```
 
 ## Default policies

packages/cli/src/config/extension.test.ts

@@ -409,11 +409,13 @@ describe('extension tests', () => {
 toolName = "deny_tool"
 decision = "deny"
 priority = 500
+modes = ["plan", "default", "autoEdit"]
 
 [[rule]]
 toolName = "ask_tool"
 decision = "ask_user"
 priority = 100
+modes = ["plan", "default", "autoEdit"]
 `;
       fs.writeFileSync(
         path.join(policiesDir, 'policies.toml'),
@@ -454,6 +456,7 @@ priority = 100
 toolName = "allow_tool"
 decision = "allow"
 priority = 100
+modes = ["plan", "default", "autoEdit"]
 
 [[rule]]
 toolName = "yolo_tool"

packages/cli/src/config/policy-engine.integration.test.ts

@@ -7,6 +7,7 @@
 import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
 import {
   ApprovalMode,
+  MODES_BY_PERMISSIVENESS,
   PolicyDecision,
   PolicyEngine,
 } from '@google/gemini-cli-core';
@@ -385,6 +386,7 @@ describe('Policy Engine Integration Tests', () => {
         toolAnnotations: { readOnlyHint: true },
         decision: PolicyDecision.ALLOW,
         priority: 10,
+        modes: MODES_BY_PERMISSIVENESS,
       });
 
       const engine = new PolicyEngine(config);

packages/cli/src/test-utils/AppRig.tsx

@@ -23,6 +23,7 @@ import {
   ExtensionLoader,
   AuthType,
   ApprovalMode,
+  MODES_BY_PERMISSIVENESS,
   createPolicyEngineConfig,
   PolicyDecision,
   ToolConfirmationOutcome,
@@ -452,6 +453,7 @@ export class AppRig {
       toolName,
       decision,
       priority,
+      modes: MODES_BY_PERMISSIVENESS,
       source: 'AppRig Override',
     });
   }

packages/core/src/agents/browser/browserAgentFactory.test.ts

@@ -12,7 +12,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';
@@ -431,6 +435,7 @@ describe('browserAgentFactory', () => {
           toolName: 'mcp_browser_agent_fill',
           decision: PolicyDecision.ASK_USER,
           priority: 999,
+          modes: MODES_BY_PERMISSIVENESS,
         }),
       );
 
@@ -439,6 +444,7 @@ describe('browserAgentFactory', () => {
           toolName: 'mcp_browser_agent_upload_file',
           decision: PolicyDecision.ASK_USER,
           priority: 999,
+          modes: MODES_BY_PERMISSIVENESS,
         }),
       );
 
@@ -447,6 +453,7 @@ describe('browserAgentFactory', () => {
           toolName: 'mcp_browser_agent_evaluate_script',
           decision: PolicyDecision.ASK_USER,
           priority: 999,
+          modes: MODES_BY_PERMISSIVENESS,
         }),
       );
     });
@@ -467,6 +474,29 @@ describe('browserAgentFactory', () => {
       );
     });
 
+    it('should not register rule if it already exists with modes in different order', async () => {
+      const existingRule = {
+        toolName: 'mcp_browser_agent_fill',
+        decision: PolicyDecision.ASK_USER,
+        priority: 999,
+        // Reverse order of MODES_BY_PERMISSIVENESS
+        modes: [...MODES_BY_PERMISSIVENESS].reverse(),
+        source: 'BrowserAgent (Sensitive Actions)',
+        mcpName: BROWSER_AGENT_NAME,
+      };
+
+      mockPolicyEngine.getRules.mockReturnValue([existingRule]);
+
+      await createBrowserAgentDefinition(mockConfig, mockMessageBus);
+
+      // Should NOT add 'fill' rule again because it's already there (even if modes order differs)
+      expect(mockPolicyEngine.addRule).not.toHaveBeenCalledWith(
+        expect.objectContaining({
+          toolName: 'mcp_browser_agent_fill',
+        }),
+      );
+    });
+
     it('should register ALLOW rules for read-only tools', async () => {
       mockBrowserManager.getDiscoveredTools.mockResolvedValue([
         {
@@ -491,6 +521,7 @@ describe('browserAgentFactory', () => {
           toolName: 'mcp_browser_agent_take_snapshot',
           decision: PolicyDecision.ALLOW,
           priority: PRIORITY_SUBAGENT_TOOL,
+          modes: MODES_BY_PERMISSIVENESS,
         }),
       );
 
@@ -499,6 +530,7 @@ describe('browserAgentFactory', () => {
           toolName: 'mcp_browser_agent_take_screenshot',
           decision: PolicyDecision.ALLOW,
           priority: PRIORITY_SUBAGENT_TOOL,
+          modes: MODES_BY_PERMISSIVENESS,
         }),
       );
 
@@ -507,6 +539,7 @@ describe('browserAgentFactory', () => {
           toolName: 'mcp_browser_agent_list_pages',
           decision: PolicyDecision.ALLOW,
           priority: PRIORITY_SUBAGENT_TOOL,
+          modes: MODES_BY_PERMISSIVENESS,
         }),
       );
     });

packages/core/src/agents/browser/browserAgentFactory.ts

@@ -41,6 +41,7 @@ import {
   PolicyDecision,
   PRIORITY_SUBAGENT_TOOL,
   type PolicyRule,
+  MODES_BY_PERMISSIVENESS,
 } from '../../policy/types.js';
 
 /**
@@ -151,6 +152,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,
     };
@@ -161,6 +163,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,
     };
@@ -172,7 +175,9 @@ export async function createBrowserAgentDefinition(
       rule1.toolName === rule2.toolName &&
       rule1.decision === rule2.decision &&
       rule1.priority === rule2.priority &&
-      rule1.mcpName === rule2.mcpName
+      rule1.mcpName === rule2.mcpName &&
+      rule1.modes.length === rule2.modes.length &&
+      rule1.modes.every((m) => rule2.modes.includes(m))
     );
   }
 

packages/core/src/agents/registry.test.ts

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

packages/core/src/agents/registry.ts

@@ -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)',
     });
   }