From 4d8483fc5bac4783468af94aebaaae8136ef1eb3 Mon Sep 17 00:00:00 2001 From: Abhijit Balaji Date: Fri, 6 Feb 2026 19:25:06 -0800 Subject: [PATCH 1/6] feat(core): deprecate tools.exclude and unify aggregation in Config - Implement PolicyEngine.getExcludedTools() to expose effectively denied tools. - Update Config.getExcludeTools() to centrally aggregate exclusions from system defaults, extensions, and the Policy Engine. - Deprecate tools.exclude in settings.json with a single startup warning in the CLI. - Mark excludeTools as deprecated in Core Config parameters and class. - Ensure backward compatibility by maintaining legacy exclude processing in Core's createPolicyEngineConfig while bypassing it in the CLI to avoid redundancy. - Add comprehensive tests for unified tool exclusion logic. --- packages/cli/src/config/config.test.ts | 4 +- packages/cli/src/config/config.ts | 5 +- packages/cli/src/gemini.tsx | 10 +++ packages/core/src/config/config.ts | 11 ++- .../core/src/policy/policy-engine.test.ts | 67 +++++++++++++++++++ packages/core/src/policy/policy-engine.ts | 37 ++++++++++ 6 files changed, 131 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/config/config.test.ts b/packages/cli/src/config/config.test.ts index 6614fe2af01..aaac4cb59bc 100644 --- a/packages/cli/src/config/config.test.ts +++ b/packages/cli/src/config/config.test.ts @@ -3183,10 +3183,12 @@ describe('Policy Engine Integration in loadCliConfig', () => { await loadCliConfig(settings, 'test-session', argv); // In non-interactive mode, ShellTool, etc. are excluded + // But we don't pass them to createPolicyEngineConfig anymore, + // as they are aggregated in Config.getExcludeTools() expect(ServerConfig.createPolicyEngineConfig).toHaveBeenCalledWith( expect.objectContaining({ tools: expect.objectContaining({ - exclude: expect.arrayContaining([SHELL_TOOL_NAME]), + exclude: [], }), }), expect.anything(), diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 87eb1e8fa7d..3faa28a8627 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -660,7 +660,10 @@ export async function loadCliConfig( tools: { ...settings.tools, allowed: allowedTools, - exclude: excludeTools, + // Exclude is intentionally empty for PolicyEngine configuration to + // avoid generating redundant DENY rules. The full set of tool + // exclusions is aggregated centrally in Config.getExcludeTools(). + exclude: [], }, mcp: { ...settings.mcp, diff --git a/packages/cli/src/gemini.tsx b/packages/cli/src/gemini.tsx index a18f3ace378..945aa7ac1fe 100644 --- a/packages/cli/src/gemini.tsx +++ b/packages/cli/src/gemini.tsx @@ -361,6 +361,16 @@ export async function main() { const argv = await parseArguments(settings.merged); parseArgsHandle?.end(); + if ( + settings.merged.tools?.exclude && + settings.merged.tools.exclude.length > 0 + ) { + coreEvents.emitFeedback( + 'warning', + 'Warning: tools.exclude in settings.json is deprecated and will be removed in 1.0. Migrate to Policy Engine: https://geminicli.com/docs/core/policy-engine/', + ); + } + if (argv.startupMessages) { argv.startupMessages.forEach((msg) => { coreEvents.emitFeedback('info', msg); diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 6d811799bc6..f1a8a4f0b6d 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -384,6 +384,7 @@ export interface ConfigParameters { coreTools?: string[]; allowedTools?: string[]; + /** @deprecated Use Policy Engine instead */ excludeTools?: string[]; toolDiscoveryCommand?: string; toolCallCommand?: string; @@ -517,6 +518,7 @@ export class Config { private readonly coreTools: string[] | undefined; private readonly allowedTools: string[] | undefined; + /** @deprecated Use Policy Engine instead */ private readonly excludeTools: string[] | undefined; private readonly toolDiscoveryCommand: string | undefined; private readonly toolCallCommand: string | undefined; @@ -1487,11 +1489,12 @@ export class Config { /** * All the excluded tools from static configuration, loaded extensions, or - * other sources. + * other sources (like the Policy Engine). * * May change over time. */ getExcludeTools(): Set | undefined { + // Right now this is present for backward compatibility with settings.json exclude const excludeToolsSet = new Set([...(this.excludeTools ?? [])]); for (const extension of this.getExtensionLoader().getExtensions()) { if (!extension.isActive) { @@ -1501,6 +1504,12 @@ export class Config { excludeToolsSet.add(tool); } } + + const policyExclusions = this.policyEngine.getExcludedTools(); + for (const tool of policyExclusions) { + excludeToolsSet.add(tool); + } + return excludeToolsSet; } diff --git a/packages/core/src/policy/policy-engine.test.ts b/packages/core/src/policy/policy-engine.test.ts index 59b0fd8106b..a21277f663c 100644 --- a/packages/core/src/policy/policy-engine.test.ts +++ b/packages/core/src/policy/policy-engine.test.ts @@ -2031,6 +2031,73 @@ describe('PolicyEngine', () => { }); }); + describe('getExcludedTools', () => { + it('should return empty set when no rules provided', () => { + engine = new PolicyEngine({}); + const excluded = engine.getExcludedTools(); + expect(excluded.size).toBe(0); + }); + + it('should include tools with DENY decision', () => { + const rules: PolicyRule[] = [ + { toolName: 'tool1', decision: PolicyDecision.DENY }, + { toolName: 'tool2', decision: PolicyDecision.ALLOW }, + ]; + engine = new PolicyEngine({ rules }); + const excluded = engine.getExcludedTools(); + expect(excluded.has('tool1')).toBe(true); + expect(excluded.has('tool2')).toBe(false); + }); + + it('should respect priority and ignore lower priority rules', () => { + // Case 1: Higher priority DENY wins + const rules: PolicyRule[] = [ + { toolName: 'tool1', decision: PolicyDecision.DENY, priority: 100 }, + { toolName: 'tool1', decision: PolicyDecision.ALLOW, priority: 10 }, + ]; + engine = new PolicyEngine({ rules }); + let excluded = engine.getExcludedTools(); + expect(excluded.has('tool1')).toBe(true); + + // Case 2: Higher priority ALLOW wins + const rules2: PolicyRule[] = [ + { toolName: 'tool1', decision: PolicyDecision.ALLOW, priority: 100 }, + { toolName: 'tool1', decision: PolicyDecision.DENY, priority: 10 }, + ]; + engine = new PolicyEngine({ rules: rules2 }); + excluded = engine.getExcludedTools(); + expect(excluded.has('tool1')).toBe(false); + }); + + it('should include ASK_USER tools in non-interactive mode', () => { + const rules: PolicyRule[] = [ + { toolName: 'tool1', decision: PolicyDecision.ASK_USER }, + ]; + // Default (interactive) mode + engine = new PolicyEngine({ rules }); + let excluded = engine.getExcludedTools(); + expect(excluded.has('tool1')).toBe(false); + + // Non-interactive mode + engine = new PolicyEngine({ rules, nonInteractive: true }); + excluded = engine.getExcludedTools(); + expect(excluded.has('tool1')).toBe(true); + }); + + it('should ignore rules with argsPattern', () => { + const rules: PolicyRule[] = [ + { + toolName: 'tool1', + decision: PolicyDecision.DENY, + argsPattern: /something/, + }, + ]; + engine = new PolicyEngine({ rules }); + const excluded = engine.getExcludedTools(); + expect(excluded.has('tool1')).toBe(false); + }); + }); + describe('YOLO mode with ask_user tool', () => { it('should return ASK_USER for ask_user tool even in YOLO mode', async () => { const rules: PolicyRule[] = [ diff --git a/packages/core/src/policy/policy-engine.ts b/packages/core/src/policy/policy-engine.ts index 8a643c89304..4a99654ae30 100644 --- a/packages/core/src/policy/policy-engine.ts +++ b/packages/core/src/policy/policy-engine.ts @@ -509,6 +509,43 @@ export class PolicyEngine { return this.hookCheckers; } + /** + * Get tools that are effectively denied by the current rules. + * This takes into account: + * 1. Global rules (no argsPattern) + * 2. Priority order (higher priority wins) + * 3. Non-interactive mode (ASK_USER becomes DENY) + */ + getExcludedTools(): Set { + const excludedTools = new Set(); + const processedTools = new Set(); + + for (const rule of this.rules) { + // We only care about global rules for exclusions + if (rule.argsPattern) { + continue; + } + + if (!rule.toolName) { + continue; + } + + // If we've already processed this tool (found a higher priority rule), skip + if (processedTools.has(rule.toolName)) { + continue; + } + + processedTools.add(rule.toolName); + + const effectiveDecision = this.applyNonInteractiveMode(rule.decision); + if (effectiveDecision === PolicyDecision.DENY) { + excludedTools.add(rule.toolName); + } + } + + return excludedTools; + } + private applyNonInteractiveMode(decision: PolicyDecision): PolicyDecision { // In non-interactive mode, ASK_USER becomes DENY if (this.nonInteractive && decision === PolicyDecision.ASK_USER) { From 247197b57c3d895864d0b1e42e19df993e8734d5 Mon Sep 17 00:00:00 2001 From: Abhijit Balaji Date: Fri, 6 Feb 2026 20:36:56 -0800 Subject: [PATCH 2/6] feat(cli): deprecate --allowed-tools in favor of policy engine Deprecates the --allowed-tools CLI flag and tools.allowed configuration option. Emits a runtime warning when used and guides users to the Policy Engine for tool permission management. --- packages/cli/src/config/config.ts | 3 ++- packages/cli/src/gemini.tsx | 10 ++++++++++ packages/core/src/config/config.ts | 2 ++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 3faa28a8627..cd2eb34370d 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -177,7 +177,8 @@ export async function parseArguments( type: 'array', string: true, nargs: 1, - description: 'Tools that are allowed to run without confirmation', + description: + '[DEPRECATED: Use Policy Engine instead See https://geminicli.com/docs/core/policy-engine] Tools that are allowed to run without confirmation', coerce: (tools: string[]) => // Handle comma-separated values tools.flatMap((tool) => tool.split(',').map((t) => t.trim())), diff --git a/packages/cli/src/gemini.tsx b/packages/cli/src/gemini.tsx index 945aa7ac1fe..e138cfe03a0 100644 --- a/packages/cli/src/gemini.tsx +++ b/packages/cli/src/gemini.tsx @@ -361,6 +361,16 @@ export async function main() { const argv = await parseArguments(settings.merged); parseArgsHandle?.end(); + if ( + (argv.allowedTools && argv.allowedTools.length > 0) || + (settings.merged.tools?.allowed && settings.merged.tools.allowed.length > 0) + ) { + coreEvents.emitFeedback( + 'warning', + 'Warning: --allowed-tools cli argument and tools.allowed in settings.json are deprecated and will be removed in 1.0: Migrate to Policy Engine: https://geminicli.com/docs/core/policy-engine/', + ); + } + if ( settings.merged.tools?.exclude && settings.merged.tools.exclude.length > 0 diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index f1a8a4f0b6d..db4085c1fa4 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -383,6 +383,7 @@ export interface ConfigParameters { question?: string; coreTools?: string[]; + /** @deprecated Use Policy Engine instead */ allowedTools?: string[]; /** @deprecated Use Policy Engine instead */ excludeTools?: string[]; @@ -517,6 +518,7 @@ export class Config { private readonly question: string | undefined; private readonly coreTools: string[] | undefined; + /** @deprecated Use Policy Engine instead */ private readonly allowedTools: string[] | undefined; /** @deprecated Use Policy Engine instead */ private readonly excludeTools: string[] | undefined; From 1f17fa114a465deef15425f353fe6ce7beaf7ad1 Mon Sep 17 00:00:00 2001 From: Abhijit Balaji Date: Fri, 6 Feb 2026 21:19:20 -0800 Subject: [PATCH 3/6] fix: ensure policy engine respects mode-specific tool exclusions - Update PolicyEngine.getExcludedTools() to filter rules based on the current approval mode. - Pass merged excluded tools to createPolicyEngineConfig in the CLI to ensure execution-level blocking via DENY rules. - Update tests to reflect that excluded tools are now passed to the Policy Engine configuration. --- packages/cli/src/config/config.test.ts | 4 +-- packages/cli/src/config/config.ts | 5 +-- .../core/src/policy/policy-engine.test.ts | 31 +++++++++++++++++++ packages/core/src/policy/policy-engine.ts | 7 +++++ 4 files changed, 40 insertions(+), 7 deletions(-) diff --git a/packages/cli/src/config/config.test.ts b/packages/cli/src/config/config.test.ts index aaac4cb59bc..6614fe2af01 100644 --- a/packages/cli/src/config/config.test.ts +++ b/packages/cli/src/config/config.test.ts @@ -3183,12 +3183,10 @@ describe('Policy Engine Integration in loadCliConfig', () => { await loadCliConfig(settings, 'test-session', argv); // In non-interactive mode, ShellTool, etc. are excluded - // But we don't pass them to createPolicyEngineConfig anymore, - // as they are aggregated in Config.getExcludeTools() expect(ServerConfig.createPolicyEngineConfig).toHaveBeenCalledWith( expect.objectContaining({ tools: expect.objectContaining({ - exclude: [], + exclude: expect.arrayContaining([SHELL_TOOL_NAME]), }), }), expect.anything(), diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index cd2eb34370d..4dea16df0e8 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -661,10 +661,7 @@ export async function loadCliConfig( tools: { ...settings.tools, allowed: allowedTools, - // Exclude is intentionally empty for PolicyEngine configuration to - // avoid generating redundant DENY rules. The full set of tool - // exclusions is aggregated centrally in Config.getExcludeTools(). - exclude: [], + exclude: excludeTools, }, mcp: { ...settings.mcp, diff --git a/packages/core/src/policy/policy-engine.test.ts b/packages/core/src/policy/policy-engine.test.ts index a21277f663c..965268097de 100644 --- a/packages/core/src/policy/policy-engine.test.ts +++ b/packages/core/src/policy/policy-engine.test.ts @@ -2096,6 +2096,37 @@ describe('PolicyEngine', () => { const excluded = engine.getExcludedTools(); expect(excluded.has('tool1')).toBe(false); }); + + it('should respect approval mode', () => { + const rules: PolicyRule[] = [ + { + toolName: 'tool1', + decision: PolicyDecision.DENY, + modes: [ApprovalMode.PLAN], + }, + ]; + + // Default mode (not PLAN) + engine = new PolicyEngine({ + rules, + approvalMode: ApprovalMode.DEFAULT, + }); + let excluded = engine.getExcludedTools(); + expect(excluded.has('tool1')).toBe(false); + + // PLAN mode + engine = new PolicyEngine({ + rules, + approvalMode: ApprovalMode.PLAN, + }); + excluded = engine.getExcludedTools(); + expect(excluded.has('tool1')).toBe(true); + + // Switch mode dynamically + engine.setApprovalMode(ApprovalMode.DEFAULT); + excluded = engine.getExcludedTools(); + expect(excluded.has('tool1')).toBe(false); + }); }); describe('YOLO mode with ask_user tool', () => { diff --git a/packages/core/src/policy/policy-engine.ts b/packages/core/src/policy/policy-engine.ts index 4a99654ae30..8be011b540a 100644 --- a/packages/core/src/policy/policy-engine.ts +++ b/packages/core/src/policy/policy-engine.ts @@ -530,6 +530,13 @@ export class PolicyEngine { continue; } + // Check if rule applies to current approval mode + if (rule.modes && rule.modes.length > 0) { + if (!rule.modes.includes(this.approvalMode)) { + continue; + } + } + // If we've already processed this tool (found a higher priority rule), skip if (processedTools.has(rule.toolName)) { continue; From c4988f14bf1a31ce39a90f7b10559904f08051b3 Mon Sep 17 00:00:00 2001 From: Abhijit Balaji Date: Fri, 6 Feb 2026 21:32:47 -0800 Subject: [PATCH 4/6] docs: deprecate tools.allowed and tools.exclude in favor of policy engine --- docs/cli/cli-reference.md | 46 ++++++++++++++-------------- docs/cli/enterprise.md | 11 ++++--- docs/get-started/configuration-v1.md | 8 +++-- docs/tools/shell.md | 9 +++--- 4 files changed, 40 insertions(+), 34 deletions(-) diff --git a/docs/cli/cli-reference.md b/docs/cli/cli-reference.md index d1094a15e20..8199445625d 100644 --- a/docs/cli/cli-reference.md +++ b/docs/cli/cli-reference.md @@ -27,29 +27,29 @@ and parameters. ## CLI Options -| Option | Alias | Type | Default | Description | -| -------------------------------- | ----- | ------- | --------- | ---------------------------------------------------------------------------------------------------------- | -| `--debug` | `-d` | boolean | `false` | Run in debug mode with verbose logging | -| `--version` | `-v` | - | - | Show CLI version number and exit | -| `--help` | `-h` | - | - | Show help information | -| `--model` | `-m` | string | `auto` | Model to use. See [Model Selection](#model-selection) for available values. | -| `--prompt` | `-p` | string | - | Prompt text. Appended to stdin input if provided. **Deprecated:** Use positional arguments instead. | -| `--prompt-interactive` | `-i` | string | - | Execute prompt and continue in interactive mode | -| `--sandbox` | `-s` | boolean | `false` | Run in a sandboxed environment for safer execution | -| `--approval-mode` | - | string | `default` | Approval mode for tool execution. Choices: `default`, `auto_edit`, `yolo` | -| `--yolo` | `-y` | boolean | `false` | **Deprecated.** Auto-approve all actions. Use `--approval-mode=yolo` instead. | -| `--experimental-acp` | - | boolean | - | Start in ACP (Agent Code Pilot) mode. **Experimental feature.** | -| `--experimental-zed-integration` | - | boolean | - | Run in Zed editor integration mode. **Experimental feature.** | -| `--allowed-mcp-server-names` | - | array | - | Allowed MCP server names (comma-separated or multiple flags) | -| `--allowed-tools` | - | array | - | Tools that are allowed to run without confirmation (comma-separated or multiple flags) | -| `--extensions` | `-e` | array | - | List of extensions to use. If not provided, all extensions are enabled (comma-separated or multiple flags) | -| `--list-extensions` | `-l` | boolean | - | List all available extensions and exit | -| `--resume` | `-r` | string | - | Resume a previous session. Use `"latest"` for most recent or index number (e.g. `--resume 5`) | -| `--list-sessions` | - | boolean | - | List available sessions for the current project and exit | -| `--delete-session` | - | string | - | Delete a session by index number (use `--list-sessions` to see available sessions) | -| `--include-directories` | - | array | - | Additional directories to include in the workspace (comma-separated or multiple flags) | -| `--screen-reader` | - | boolean | - | Enable screen reader mode for accessibility | -| `--output-format` | `-o` | string | `text` | The format of the CLI output. Choices: `text`, `json`, `stream-json` | +| Option | Alias | Type | Default | Description | +| -------------------------------- | ----- | ------- | --------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `--debug` | `-d` | boolean | `false` | Run in debug mode with verbose logging | +| `--version` | `-v` | - | - | Show CLI version number and exit | +| `--help` | `-h` | - | - | Show help information | +| `--model` | `-m` | string | `auto` | Model to use. See [Model Selection](#model-selection) for available values. | +| `--prompt` | `-p` | string | - | Prompt text. Appended to stdin input if provided. **Deprecated:** Use positional arguments instead. | +| `--prompt-interactive` | `-i` | string | - | Execute prompt and continue in interactive mode | +| `--sandbox` | `-s` | boolean | `false` | Run in a sandboxed environment for safer execution | +| `--approval-mode` | - | string | `default` | Approval mode for tool execution. Choices: `default`, `auto_edit`, `yolo` | +| `--yolo` | `-y` | boolean | `false` | **Deprecated.** Auto-approve all actions. Use `--approval-mode=yolo` instead. | +| `--experimental-acp` | - | boolean | - | Start in ACP (Agent Code Pilot) mode. **Experimental feature.** | +| `--experimental-zed-integration` | - | boolean | - | Run in Zed editor integration mode. **Experimental feature.** | +| `--allowed-mcp-server-names` | - | array | - | Allowed MCP server names (comma-separated or multiple flags) | +| `--allowed-tools` | - | array | - | **Deprecated.** Use the [Policy Engine](../core/policy-engine.md) instead. Tools that are allowed to run without confirmation (comma-separated or multiple flags) | +| `--extensions` | `-e` | array | - | List of extensions to use. If not provided, all extensions are enabled (comma-separated or multiple flags) | +| `--list-extensions` | `-l` | boolean | - | List all available extensions and exit | +| `--resume` | `-r` | string | - | Resume a previous session. Use `"latest"` for most recent or index number (e.g. `--resume 5`) | +| `--list-sessions` | - | boolean | - | List available sessions for the current project and exit | +| `--delete-session` | - | string | - | Delete a session by index number (use `--list-sessions` to see available sessions) | +| `--include-directories` | - | array | - | Additional directories to include in the workspace (comma-separated or multiple flags) | +| `--screen-reader` | - | boolean | - | Enable screen reader mode for accessibility | +| `--output-format` | `-o` | string | `text` | The format of the CLI output. Choices: `text`, `json`, `stream-json` | ## Model selection diff --git a/docs/cli/enterprise.md b/docs/cli/enterprise.md index f22ec81c37c..861fc68c715 100644 --- a/docs/cli/enterprise.md +++ b/docs/cli/enterprise.md @@ -223,9 +223,9 @@ gemini ## Restricting tool access You can significantly enhance security by controlling which tools the Gemini -model can use. This is achieved through the `tools.core` and `tools.exclude` -settings. For a list of available tools, see the -[Tools documentation](../tools/index.md). +model can use. This is achieved through the `tools.core` setting and the +[Policy Engine](../core/policy-engine.md). For a list of available tools, see +the [Tools documentation](../tools/index.md). ### Allowlisting with `coreTools` @@ -243,7 +243,10 @@ on the approved list. } ``` -### Blocklisting with `excludeTools` +### Blocklisting with `excludeTools` (Deprecated) + +> **Deprecated:** Use the [Policy Engine](../core/policy-engine.md) for more +> robust control. Alternatively, you can add specific tools that are considered dangerous in your environment to a blocklist. diff --git a/docs/get-started/configuration-v1.md b/docs/get-started/configuration-v1.md index 050dce32b6c..cd1325b977f 100644 --- a/docs/get-started/configuration-v1.md +++ b/docs/get-started/configuration-v1.md @@ -166,19 +166,21 @@ a few things you can try in order of recommendation: - **Default:** All tools available for use by the Gemini model. - **Example:** `"coreTools": ["ReadFileTool", "GlobTool", "ShellTool(ls)"]`. -- **`allowedTools`** (array of strings): +- **`allowedTools`** (array of strings) [DEPRECATED]: - **Default:** `undefined` - **Description:** A list of tool names that will bypass the confirmation dialog. This is useful for tools that you trust and use frequently. The - match semantics are the same as `coreTools`. + match semantics are the same as `coreTools`. **Deprecated**: Use the + [Policy Engine](../core/policy-engine.md) instead. - **Example:** `"allowedTools": ["ShellTool(git status)"]`. -- **`excludeTools`** (array of strings): +- **`excludeTools`** (array of strings) [DEPRECATED]: - **Description:** Allows you to specify a list of core tool names that should be excluded from the model. A tool listed in both `excludeTools` and `coreTools` is excluded. You can also specify command-specific restrictions for tools that support it, like the `ShellTool`. For example, `"excludeTools": ["ShellTool(rm -rf)"]` will block the `rm -rf` command. + **Deprecated**: Use the [Policy Engine](../core/policy-engine.md) instead. - **Default**: No tools excluded. - **Example:** `"excludeTools": ["run_shell_command", "findFiles"]`. - **Security Note:** Command-specific restrictions in `excludeTools` for diff --git a/docs/tools/shell.md b/docs/tools/shell.md index 0bb4b682442..48854e82f1e 100644 --- a/docs/tools/shell.md +++ b/docs/tools/shell.md @@ -167,10 +167,11 @@ configuration file. `"tools": {"core": ["run_shell_command(git)"]}` will only allow `git` commands. Including the generic `run_shell_command` acts as a wildcard, allowing any command not explicitly blocked. -- `tools.exclude`: To block specific commands, add entries to the `exclude` list - under the `tools` category in the format `run_shell_command()`. For - example, `"tools": {"exclude": ["run_shell_command(rm)"]}` will block `rm` - commands. +- `tools.exclude` [DEPRECATED]: To block specific commands, use the + [Policy Engine](../core/policy-engine.md). Historically, this setting allowed + adding entries to the `exclude` list under the `tools` category in the format + `run_shell_command()`. For example, + `"tools": {"exclude": ["run_shell_command(rm)"]}` will block `rm` commands. The validation logic is designed to be secure and flexible: From 87d220b840732f09abd2d4651a28a8a477460823 Mon Sep 17 00:00:00 2001 From: Abhijit Balaji Date: Tue, 10 Feb 2026 15:07:17 -0800 Subject: [PATCH 5/6] fix(core): refactor tool exclusion logic to support global and wildcard rules Updates the PolicyEngine to correctly handle global rules (rules without a specific tool name) and ensure wildcard rules cover matching tools. Also corrects the handling of ASK_USER tools in non-interactive mode. --- .../core/src/policy/policy-engine.test.ts | 29 ++++++++- packages/core/src/policy/policy-engine.ts | 64 +++++++++++++++---- 2 files changed, 79 insertions(+), 14 deletions(-) diff --git a/packages/core/src/policy/policy-engine.test.ts b/packages/core/src/policy/policy-engine.test.ts index 965268097de..8fa097e4676 100644 --- a/packages/core/src/policy/policy-engine.test.ts +++ b/packages/core/src/policy/policy-engine.test.ts @@ -2069,7 +2069,7 @@ describe('PolicyEngine', () => { expect(excluded.has('tool1')).toBe(false); }); - it('should include ASK_USER tools in non-interactive mode', () => { + it('should NOT include ASK_USER tools in non-interactive mode', () => { const rules: PolicyRule[] = [ { toolName: 'tool1', decision: PolicyDecision.ASK_USER }, ]; @@ -2081,7 +2081,7 @@ describe('PolicyEngine', () => { // Non-interactive mode engine = new PolicyEngine({ rules, nonInteractive: true }); excluded = engine.getExcludedTools(); - expect(excluded.has('tool1')).toBe(true); + expect(excluded.has('tool1')).toBe(false); }); it('should ignore rules with argsPattern', () => { @@ -2127,6 +2127,31 @@ describe('PolicyEngine', () => { excluded = engine.getExcludedTools(); expect(excluded.has('tool1')).toBe(false); }); + + it('should respect wildcard ALLOW rules (e.g. YOLO mode)', () => { + const rules: PolicyRule[] = [ + { + // Wildcard ALLOW (YOLO) - High Priority + decision: PolicyDecision.ALLOW, + priority: 999, + modes: [ApprovalMode.YOLO], + }, + { + // Specific DENY - Low Priority + toolName: 'dangerous-tool', + decision: PolicyDecision.DENY, + priority: 10, + }, + ]; + + // In DEFAULT mode, wildcard shouldn't apply, so tool is excluded + engine = new PolicyEngine({ rules, approvalMode: ApprovalMode.DEFAULT }); + expect(engine.getExcludedTools().has('dangerous-tool')).toBe(true); + + // In YOLO mode, wildcard SHOULD apply and override the specific deny + engine = new PolicyEngine({ rules, approvalMode: ApprovalMode.YOLO }); + expect(engine.getExcludedTools().has('dangerous-tool')).toBe(false); + }); }); describe('YOLO mode with ask_user tool', () => { diff --git a/packages/core/src/policy/policy-engine.ts b/packages/core/src/policy/policy-engine.ts index 8be011b540a..ed4e65bea33 100644 --- a/packages/core/src/policy/policy-engine.ts +++ b/packages/core/src/policy/policy-engine.ts @@ -519,17 +519,14 @@ export class PolicyEngine { getExcludedTools(): Set { const excludedTools = new Set(); const processedTools = new Set(); + let globalVerdict: PolicyDecision | undefined; for (const rule of this.rules) { - // We only care about global rules for exclusions + // We only care about rules without args pattern for exclusion from the model if (rule.argsPattern) { continue; } - if (!rule.toolName) { - continue; - } - // Check if rule applies to current approval mode if (rule.modes && rule.modes.length > 0) { if (!rule.modes.includes(this.approvalMode)) { @@ -537,19 +534,62 @@ export class PolicyEngine { } } - // If we've already processed this tool (found a higher priority rule), skip - if (processedTools.has(rule.toolName)) { + // Handle Global Rules + if (!rule.toolName) { + if (globalVerdict === undefined) { + globalVerdict = rule.decision; + if (globalVerdict !== PolicyDecision.DENY) { + // Global ALLOW/ASK found. + // Since rules are sorted by priority, this overrides any lower-priority rules. + // We can stop processing because nothing else will be excluded. + break; + } + // If Global DENY, we continue to find specific tools to add to excluded set + } continue; } - processedTools.add(rule.toolName); + const toolName = rule.toolName; - const effectiveDecision = this.applyNonInteractiveMode(rule.decision); - if (effectiveDecision === PolicyDecision.DENY) { - excludedTools.add(rule.toolName); + // Check if already processed (exact match) + if (processedTools.has(toolName)) { + continue; } - } + // Check if covered by a processed wildcard + let coveredByWildcard = false; + for (const processed of processedTools) { + if ( + processed.endsWith('__*') && + toolName.startsWith(processed.slice(0, -3)) + ) { + // It's covered by a higher-priority wildcard rule. + // If that wildcard rule resulted in exclusion, this tool should also be excluded. + if (excludedTools.has(processed)) { + excludedTools.add(toolName); + } + coveredByWildcard = true; + break; + } + } + if (coveredByWildcard) { + continue; + } + + processedTools.add(toolName); + + // Determine decision + let decision: PolicyDecision; + if (globalVerdict !== undefined) { + decision = globalVerdict; + } else { + decision = rule.decision; + } + + if (decision === PolicyDecision.DENY) { + excludedTools.add(toolName); + } + } return excludedTools; } From 88cb82b6e639c6efeb23f31988b8ba2fcba4a446 Mon Sep 17 00:00:00 2001 From: Abhijit Balaji Date: Wed, 11 Feb 2026 14:56:37 -0800 Subject: [PATCH 6/6] refactor(core): cleanup wildcard matching and use parameterized tests in PolicyEngine --- .../core/src/policy/policy-engine.test.ts | 265 ++++++++++-------- packages/core/src/policy/policy-engine.ts | 26 +- 2 files changed, 167 insertions(+), 124 deletions(-) diff --git a/packages/core/src/policy/policy-engine.test.ts b/packages/core/src/policy/policy-engine.test.ts index 8fa097e4676..26aecaa1ebf 100644 --- a/packages/core/src/policy/policy-engine.test.ts +++ b/packages/core/src/policy/policy-engine.test.ts @@ -2032,126 +2032,153 @@ describe('PolicyEngine', () => { }); describe('getExcludedTools', () => { - it('should return empty set when no rules provided', () => { - engine = new PolicyEngine({}); - const excluded = engine.getExcludedTools(); - expect(excluded.size).toBe(0); - }); - - it('should include tools with DENY decision', () => { - const rules: PolicyRule[] = [ - { toolName: 'tool1', decision: PolicyDecision.DENY }, - { toolName: 'tool2', decision: PolicyDecision.ALLOW }, - ]; - engine = new PolicyEngine({ rules }); - const excluded = engine.getExcludedTools(); - expect(excluded.has('tool1')).toBe(true); - expect(excluded.has('tool2')).toBe(false); - }); - - it('should respect priority and ignore lower priority rules', () => { - // Case 1: Higher priority DENY wins - const rules: PolicyRule[] = [ - { toolName: 'tool1', decision: PolicyDecision.DENY, priority: 100 }, - { toolName: 'tool1', decision: PolicyDecision.ALLOW, priority: 10 }, - ]; - engine = new PolicyEngine({ rules }); - let excluded = engine.getExcludedTools(); - expect(excluded.has('tool1')).toBe(true); - - // Case 2: Higher priority ALLOW wins - const rules2: PolicyRule[] = [ - { toolName: 'tool1', decision: PolicyDecision.ALLOW, priority: 100 }, - { toolName: 'tool1', decision: PolicyDecision.DENY, priority: 10 }, - ]; - engine = new PolicyEngine({ rules: rules2 }); - excluded = engine.getExcludedTools(); - expect(excluded.has('tool1')).toBe(false); - }); - - it('should NOT include ASK_USER tools in non-interactive mode', () => { - const rules: PolicyRule[] = [ - { toolName: 'tool1', decision: PolicyDecision.ASK_USER }, - ]; - // Default (interactive) mode - engine = new PolicyEngine({ rules }); - let excluded = engine.getExcludedTools(); - expect(excluded.has('tool1')).toBe(false); - - // Non-interactive mode - engine = new PolicyEngine({ rules, nonInteractive: true }); - excluded = engine.getExcludedTools(); - expect(excluded.has('tool1')).toBe(false); - }); - - it('should ignore rules with argsPattern', () => { - const rules: PolicyRule[] = [ - { - toolName: 'tool1', - decision: PolicyDecision.DENY, - argsPattern: /something/, - }, - ]; - engine = new PolicyEngine({ rules }); - const excluded = engine.getExcludedTools(); - expect(excluded.has('tool1')).toBe(false); - }); - - it('should respect approval mode', () => { - const rules: PolicyRule[] = [ - { - toolName: 'tool1', - decision: PolicyDecision.DENY, - modes: [ApprovalMode.PLAN], - }, - ]; - - // Default mode (not PLAN) - engine = new PolicyEngine({ - rules, - approvalMode: ApprovalMode.DEFAULT, - }); - let excluded = engine.getExcludedTools(); - expect(excluded.has('tool1')).toBe(false); - - // PLAN mode - engine = new PolicyEngine({ - rules, + interface TestCase { + name: string; + rules: PolicyRule[]; + approvalMode?: ApprovalMode; + nonInteractive?: boolean; + expected: string[]; + } + + const testCases: TestCase[] = [ + { + name: 'should return empty set when no rules provided', + rules: [], + expected: [], + }, + { + name: 'should include tools with DENY decision', + rules: [ + { toolName: 'tool1', decision: PolicyDecision.DENY }, + { toolName: 'tool2', decision: PolicyDecision.ALLOW }, + ], + 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 }, + ], + 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 }, + ], + expected: [], + }, + { + name: 'should NOT include ASK_USER tools even in non-interactive mode', + rules: [{ toolName: 'tool1', decision: PolicyDecision.ASK_USER }], + nonInteractive: true, + expected: [], + }, + { + name: 'should ignore rules with argsPattern', + rules: [ + { + toolName: 'tool1', + decision: PolicyDecision.DENY, + argsPattern: /something/, + }, + ], + expected: [], + }, + { + name: 'should respect approval mode (PLAN mode)', + rules: [ + { + toolName: 'tool1', + decision: PolicyDecision.DENY, + modes: [ApprovalMode.PLAN], + }, + ], approvalMode: ApprovalMode.PLAN, - }); - excluded = engine.getExcludedTools(); - expect(excluded.has('tool1')).toBe(true); - - // Switch mode dynamically - engine.setApprovalMode(ApprovalMode.DEFAULT); - excluded = engine.getExcludedTools(); - expect(excluded.has('tool1')).toBe(false); - }); - - it('should respect wildcard ALLOW rules (e.g. YOLO mode)', () => { - const rules: PolicyRule[] = [ - { - // Wildcard ALLOW (YOLO) - High Priority - decision: PolicyDecision.ALLOW, - priority: 999, - modes: [ApprovalMode.YOLO], - }, - { - // Specific DENY - Low Priority - toolName: 'dangerous-tool', - decision: PolicyDecision.DENY, - priority: 10, - }, - ]; - - // In DEFAULT mode, wildcard shouldn't apply, so tool is excluded - engine = new PolicyEngine({ rules, approvalMode: ApprovalMode.DEFAULT }); - expect(engine.getExcludedTools().has('dangerous-tool')).toBe(true); - - // In YOLO mode, wildcard SHOULD apply and override the specific deny - engine = new PolicyEngine({ rules, approvalMode: ApprovalMode.YOLO }); - expect(engine.getExcludedTools().has('dangerous-tool')).toBe(false); - }); + expected: ['tool1'], + }, + { + name: 'should respect approval mode (DEFAULT mode)', + rules: [ + { + toolName: 'tool1', + decision: PolicyDecision.DENY, + modes: [ApprovalMode.PLAN], + }, + ], + approvalMode: ApprovalMode.DEFAULT, + expected: [], + }, + { + name: 'should respect wildcard ALLOW rules (e.g. YOLO mode)', + rules: [ + { + decision: PolicyDecision.ALLOW, + priority: 999, + modes: [ApprovalMode.YOLO], + }, + { + toolName: 'dangerous-tool', + decision: PolicyDecision.DENY, + priority: 10, + }, + ], + approvalMode: ApprovalMode.YOLO, + expected: [], + }, + { + name: 'should respect server wildcard DENY', + rules: [{ toolName: 'server__*', decision: PolicyDecision.DENY }], + expected: ['server__*'], + }, + { + name: 'should expand server wildcard for specific tools if already processed', + rules: [ + { + toolName: 'server__*', + decision: PolicyDecision.DENY, + priority: 100, + }, + { + toolName: 'server__tool1', + decision: PolicyDecision.DENY, + priority: 10, + }, + ], + expected: ['server__*', 'server__tool1'], + }, + { + name: 'should NOT exclude tool if covered by a higher priority wildcard ALLOW', + rules: [ + { + toolName: 'server__*', + decision: PolicyDecision.ALLOW, + priority: 100, + }, + { + toolName: 'server__tool1', + decision: PolicyDecision.DENY, + priority: 10, + }, + ], + expected: [], + }, + ]; + + it.each(testCases)( + '$name', + ({ rules, approvalMode, nonInteractive, expected }) => { + engine = new PolicyEngine({ + rules, + approvalMode: approvalMode ?? ApprovalMode.DEFAULT, + nonInteractive: nonInteractive ?? false, + }); + const excluded = engine.getExcludedTools(); + expect(Array.from(excluded).sort()).toEqual(expected.sort()); + }, + ); }); describe('YOLO mode with ask_user tool', () => { diff --git a/packages/core/src/policy/policy-engine.ts b/packages/core/src/policy/policy-engine.ts index ed4e65bea33..1fc5e7cde52 100644 --- a/packages/core/src/policy/policy-engine.ts +++ b/packages/core/src/policy/policy-engine.ts @@ -26,6 +26,22 @@ import { } from '../utils/shell-utils.js'; import { getToolAliases } from '../tools/tool-names.js'; +function isWildcardPattern(name: string): boolean { + return name.endsWith('__*'); +} + +function getWildcardPrefix(pattern: string): string { + return pattern.slice(0, -3); +} + +function matchesWildcard(pattern: string, toolName: string): boolean { + if (!isWildcardPattern(pattern)) { + return false; + } + const prefix = getWildcardPrefix(pattern); + return toolName.startsWith(prefix + '__'); +} + function ruleMatches( rule: PolicyRule | SafetyCheckerRule, toolCall: FunctionCall, @@ -43,8 +59,8 @@ function ruleMatches( // Check tool name if specified if (rule.toolName) { // Support wildcard patterns: "serverName__*" matches "serverName__anyTool" - if (rule.toolName.endsWith('__*')) { - const prefix = rule.toolName.slice(0, -3); // Remove "__*" + if (isWildcardPattern(rule.toolName)) { + const prefix = getWildcardPrefix(rule.toolName); if (serverName !== undefined) { // Robust check: if serverName is provided, it MUST match the prefix exactly. // This prevents "malicious-server" from spoofing "trusted-server" by naming itself "trusted-server__malicious". @@ -53,7 +69,7 @@ function ruleMatches( } } // Always verify the prefix, even if serverName matched - if (!toolCall.name || !toolCall.name.startsWith(prefix + '__')) { + if (!toolCall.name || !matchesWildcard(rule.toolName, toolCall.name)) { return false; } } else if (toolCall.name !== rule.toolName) { @@ -560,8 +576,8 @@ export class PolicyEngine { let coveredByWildcard = false; for (const processed of processedTools) { if ( - processed.endsWith('__*') && - toolName.startsWith(processed.slice(0, -3)) + isWildcardPattern(processed) && + matchesWildcard(processed, toolName) ) { // It's covered by a higher-priority wildcard rule. // If that wildcard rule resulted in exclusion, this tool should also be excluded.