From 258d9223e41bb096b254b4baa3c134919ff2e197 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Fri, 16 Jan 2026 18:18:39 -0500 Subject: [PATCH] Make always_allow_tool_actions override always_confirm and default_mode (#47012) Previously, `always_confirm` patterns would force confirmation even when `always_allow_tool_actions` was set to true. This was counterintuitive since the global setting should provide a way to skip all confirmations. The new precedence order is: 1. **`always_deny`** - still blocks for security 2. **`always_allow_tool_actions`** - when true, allows all non-denied actions 3. **`always_confirm`** - prompts if `always_allow_tool_actions` is false 4. **`always_allow`** - allows without prompting 5. **`default_mode`** - fallback behavior This means setting `always_allow_tool_actions=true` will now skip confirmation prompts from `always_confirm` patterns and override `default_mode: Deny` settings, while still respecting `always_deny` patterns for security. (No release notes because granular tool permissions are still feature-flagged.) Release Notes: - N/A --- assets/settings/default.json | 4 ++- crates/agent/src/tests/mod.rs | 30 ++++++---------- crates/agent/src/tool_permissions.rs | 52 ++++++++++++++++++---------- 3 files changed, 47 insertions(+), 39 deletions(-) diff --git a/assets/settings/default.json b/assets/settings/default.json index 7fb99b0af88f8672ae7dc4626c2351422b607f74..9a8a3da63308e02e7fe34aca0f0527291bec07c1 100644 --- a/assets/settings/default.json +++ b/assets/settings/default.json @@ -954,7 +954,9 @@ // "temperature": 1.0 // } ], - // When enabled, the agent can run potentially destructive actions without asking for your confirmation. + // When enabled, the agent can run tool actions without asking for your confirmation. + // This setting takes precedence over `always_confirm` patterns and `default_mode` settings, + // but `always_deny` patterns still block actions for security. // // Note: This setting has no effect on external agents that support permission modes, such as Claude Code. // You can set `agent_servers.claude.default_mode` to `bypassPermissions` to skip all permission requests. diff --git a/crates/agent/src/tests/mod.rs b/crates/agent/src/tests/mod.rs index e31953fc15170ae1fdc4651448058be192c83c60..bd26c10cae39432c5d989fef9567a26681921750 100644 --- a/crates/agent/src/tests/mod.rs +++ b/crates/agent/src/tests/mod.rs @@ -3643,7 +3643,7 @@ async fn test_terminal_tool_permission_rules(cx: &mut TestAppContext) { ); } - // Test 3: Confirm rule forces confirmation even with always_allow_tool_actions=true + // Test 3: always_allow_tool_actions=true overrides always_confirm patterns { let handle = Rc::new(cx.update(|cx| FakeTerminalHandle::new_with_immediate_exit(cx, 0))); let environment = Rc::new(FakeThreadEnvironment { @@ -3670,9 +3670,9 @@ async fn test_terminal_tool_permission_rules(cx: &mut TestAppContext) { #[allow(clippy::arc_with_non_send_sync)] let tool = Arc::new(crate::TerminalTool::new(project.clone(), environment)); - let (event_stream, mut rx) = crate::ToolCallEventStream::test(); + let (event_stream, _rx) = crate::ToolCallEventStream::test(); - let _task = cx.update(|cx| { + let task = cx.update(|cx| { tool.run( crate::TerminalToolInput { command: "sudo rm file".to_string(), @@ -3684,16 +3684,14 @@ async fn test_terminal_tool_permission_rules(cx: &mut TestAppContext) { ) }); - let auth = rx.expect_authorization().await; - assert!( - auth.tool_call.fields.title.is_some(), - "expected authorization request for sudo command despite always_allow_tool_actions=true" - ); + // With always_allow_tool_actions=true, confirm patterns are overridden + task.await + .expect("command should be allowed with always_allow_tool_actions=true"); } - // Test 4: default_mode: Deny blocks commands when no pattern matches + // Test 4: always_allow_tool_actions=true overrides default_mode: Deny { - let handle = Rc::new(cx.update(|cx| FakeTerminalHandle::new_never_exits(cx))); + let handle = Rc::new(cx.update(|cx| FakeTerminalHandle::new_with_immediate_exit(cx, 0))); let environment = Rc::new(FakeThreadEnvironment { handle: handle.clone(), }); @@ -3730,15 +3728,9 @@ async fn test_terminal_tool_permission_rules(cx: &mut TestAppContext) { ) }); - let result = task.await; - assert!( - result.is_err(), - "expected command to be blocked by default_mode: Deny" - ); - assert!( - result.unwrap_err().to_string().contains("disabled"), - "error should mention the tool is disabled" - ); + // With always_allow_tool_actions=true, even default_mode: Deny is overridden + task.await + .expect("command should be allowed with always_allow_tool_actions=true"); } } diff --git a/crates/agent/src/tool_permissions.rs b/crates/agent/src/tool_permissions.rs index 85d4aec0fe0083f1fb6231e6de6aff3707ee83ff..6ae4d14721f73c19ea08135f8990935c6589c7b3 100644 --- a/crates/agent/src/tool_permissions.rs +++ b/crates/agent/src/tool_permissions.rs @@ -12,15 +12,16 @@ pub enum ToolPermissionDecision { /// /// # Precedence Order (highest to lowest) /// -/// 1. **`always_deny`** - If any deny pattern matches, the tool call is blocked immediately. -/// This takes precedence over all other rules for security. -/// 2. **`always_confirm`** - If any confirm pattern matches (and no deny matched), -/// the user is prompted for confirmation regardless of other settings. -/// 3. **`always_allow`** - If any allow pattern matches (and no deny/confirm matched), +/// 1. **`always_allow_tool_actions`** - When enabled, allows all tool actions except those +/// blocked by `always_deny` patterns. This global setting takes precedence over +/// `always_confirm` patterns and `default_mode`. +/// 2. **`always_deny`** - If any deny pattern matches, the tool call is blocked immediately. +/// This takes precedence over all other rules for security (including `always_allow_tool_actions`). +/// 3. **`always_confirm`** - If any confirm pattern matches (and no deny matched), +/// the user is prompted for confirmation (unless `always_allow_tool_actions` is enabled). +/// 4. **`always_allow`** - If any allow pattern matches (and no deny/confirm matched), /// the tool call proceeds without prompting. -/// 4. **`default_mode`** - If no patterns match, falls back to the tool's default mode. -/// 5. **`always_allow_tool_actions`** - Global setting used as fallback when no tool-specific -/// rules are configured, or when `default_mode` is `Confirm`. +/// 5. **`default_mode`** - If no patterns match, falls back to the tool's default mode. /// /// # Pattern Matching Tips /// @@ -67,25 +68,25 @@ pub fn decide_permission( } if rules.always_confirm.iter().any(|r| r.is_match(input)) { - return ToolPermissionDecision::Confirm; + if !always_allow_tool_actions { + return ToolPermissionDecision::Confirm; + } } if rules.always_allow.iter().any(|r| r.is_match(input)) { return ToolPermissionDecision::Allow; } + if always_allow_tool_actions { + return ToolPermissionDecision::Allow; + } + match rules.default_mode { ToolPermissionMode::Deny => { ToolPermissionDecision::Deny(format!("{} tool is disabled", tool_name)) } ToolPermissionMode::Allow => ToolPermissionDecision::Allow, - ToolPermissionMode::Confirm => { - if always_allow_tool_actions { - ToolPermissionDecision::Allow - } else { - ToolPermissionDecision::Confirm - } - } + ToolPermissionMode::Confirm => ToolPermissionDecision::Confirm, } } @@ -316,11 +317,11 @@ mod tests { t("sudo apt install").confirm(&["sudo\\s"]).is_confirm(); } #[test] - fn confirm_overrides_global() { + fn global_overrides_confirm() { t("sudo reboot") .confirm(&["sudo\\s"]) .global(true) - .is_confirm(); + .is_allow(); } #[test] fn confirm_overrides_mode_allow() { @@ -400,6 +401,13 @@ mod tests { fn default_deny() { t("python x.py").mode(ToolPermissionMode::Deny).is_deny(); } + #[test] + fn default_deny_global_true() { + t("python x.py") + .mode(ToolPermissionMode::Deny) + .global(true) + .is_allow(); + } // default_mode confirm + global #[test] @@ -487,8 +495,14 @@ mod tests { }, ); let p = ToolPermissions { tools }; - assert!(matches!( + // With always_allow_tool_actions=true, even default_mode: Deny is overridden + assert_eq!( decide_permission("terminal", "x", &p, true), + ToolPermissionDecision::Allow + ); + // With always_allow_tool_actions=false, default_mode: Deny is respected + assert!(matches!( + decide_permission("terminal", "x", &p, false), ToolPermissionDecision::Deny(_) )); assert_eq!(