From de213ad4e74193ab19af460434466729d728d14f Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Tue, 10 Feb 2026 18:57:31 -0500 Subject: [PATCH] Replace `always_allow_tool_actions` with `tool_permissions.default` (#48553) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Screenshot 2026-02-10 at 6 55 03 PM Screenshot 2026-02-10 at 6 55 15 PM Replaces the boolean `always_allow_tool_actions` setting with a three-valued `tool_permissions.default` field (`"allow"` / `"confirm"` / `"deny"`). Release Notes: - Introduced per-tool permission settings, including regexes for controlling when tools may be auto-allowed, auto-denied, or always require confirmation. - Replaced the `always_allow_tool_actions` setting with `tool_permissions.default`. --------- Co-authored-by: Zed Zippy <234243425+zed-zippy[bot]@users.noreply.github.com> --- Cargo.lock | 1 - assets/settings/default.json | 35 +- crates/acp_thread/Cargo.toml | 1 - crates/acp_thread/src/acp_thread.rs | 19 +- crates/acp_thread/src/connection.rs | 6 +- crates/agent/src/agent.rs | 5 +- crates/agent/src/tests/mod.rs | 172 ++-- crates/agent/src/tests/test_tools.rs | 4 +- crates/agent/src/thread.rs | 95 ++- crates/agent/src/tool_permissions.rs | 685 ++++++++++++---- crates/agent/src/tools/copy_path_tool.rs | 29 +- .../agent/src/tools/create_directory_tool.rs | 7 +- crates/agent/src/tools/delete_path_tool.rs | 3 +- crates/agent/src/tools/edit_file_tool.rs | 70 +- crates/agent/src/tools/fetch_tool.rs | 5 +- crates/agent/src/tools/move_path_tool.rs | 29 +- crates/agent/src/tools/open_tool.rs | 2 +- .../src/tools/restore_file_from_disk_tool.rs | 78 +- crates/agent/src/tools/save_file_tool.rs | 54 +- .../src/tools/streaming_edit_file_tool.rs | 9 + crates/agent/src/tools/terminal_tool.rs | 8 +- crates/agent/src/tools/web_search_tool.rs | 8 +- crates/agent_servers/src/acp.rs | 3 - crates/agent_settings/src/agent_settings.rs | 364 ++++++++- crates/agent_ui/src/acp/thread_view.rs | 60 +- crates/agent_ui/src/agent_ui.rs | 2 +- crates/eval/runner_settings.json | 4 +- crates/migrator/src/migrations.rs | 6 + .../src/migrations/m_2026_02_02/settings.rs | 2 +- .../src/migrations/m_2026_02_04/settings.rs | 124 +++ crates/migrator/src/migrator.rs | 759 ++++++++++++++++++ crates/settings_content/src/agent.rs | 94 ++- crates/settings_ui/Cargo.toml | 2 +- crates/settings_ui/src/page_data.rs | 24 +- crates/settings_ui/src/pages.rs | 5 +- .../src/pages/tool_permissions_setup.rs | 400 ++++++--- crates/zed/src/visual_test_runner.rs | 125 +-- docs/src/SUMMARY.md | 1 + docs/src/ai/agent-panel.md | 16 +- docs/src/ai/agent-settings.md | 158 +++- docs/src/ai/mcp.md | 18 +- docs/src/ai/overview.md | 2 + docs/src/ai/privacy-and-security.md | 2 + docs/src/ai/tool-permissions.md | 282 +++++++ docs/src/ai/tools.md | 20 +- 45 files changed, 3080 insertions(+), 718 deletions(-) create mode 100644 crates/migrator/src/migrations/m_2026_02_04/settings.rs create mode 100644 docs/src/ai/tool-permissions.md diff --git a/Cargo.lock b/Cargo.lock index 485554e57f8f41ac6ecb417c713408be8e71b819..ff21d537bdd92b8395682ad75b4de2be4fe0808b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8,7 +8,6 @@ version = "0.1.0" dependencies = [ "action_log", "agent-client-protocol", - "agent_settings", "anyhow", "base64 0.22.1", "buffer_diff", diff --git a/assets/settings/default.json b/assets/settings/default.json index 7824959df6e4e198d5401476096f19ac76f81a7e..b9be729d425ebd542c47a9c4815a789fd56e8080 100644 --- a/assets/settings/default.json +++ b/assets/settings/default.json @@ -958,20 +958,29 @@ // "temperature": 1.0 // } ], - // 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. + // Permission rules for tool actions. // - // 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. - "always_allow_tool_actions": false, - // Per-tool permission rules for granular control over tool actions. - // This setting only applies to the native Zed agent. + // The "default" setting applies when no tool-specific rules match. + // For external agents that define their own permission modes, + // "deny" and "confirm" still take precedence — the external agent's + // permission system is only used when Zed would allow the action. + // + // Per-tool regex patterns ("tools" below) match against tool input text + // (commands, paths, URLs, etc.). For `copy_path` and `move_path`, + // patterns are matched independently against each path (source and + // destination). "tool_permissions": { - // Here are some examples of tool-specific permissions. + // Global default permission when no tool-specific rules match. + // "allow" - Auto-approve without prompting + // "deny" - Auto-reject + // "confirm" - Always prompt (default) + "default": "confirm", + // Per-tool permission rules. Regex patterns match against tool input text. + // The per-tool "default" also applies to MCP tools. + // Each tool can have its own default and regex patterns. "tools": { // "terminal": { - // "default_mode": "confirm", + // "default": "confirm", // "always_confirm": [ // // Destructive git operations // { "pattern": "git\\s+(reset|clean)\\s+--hard" }, @@ -979,7 +988,7 @@ // ], // }, // "edit_file": { - // "default_mode": "confirm", + // "default": "confirm", // "always_deny": [ // // Secrets and credentials // { "pattern": "\\.env($|\\.)" }, @@ -2235,9 +2244,9 @@ // Whether to show the LSP servers button in the status bar. "button": true, // The maximum amount of time to wait for responses from language servers, in seconds. + // A value of 0 will result in no timeout being applied. // - // A value of `0` will result in no timeout being applied (causing all LSP responses to wait - // indefinitely until completed). + // Default: 120 "request_timeout": 120, "notifications": { // Timeout in milliseconds for automatically dismissing language server notifications. diff --git a/crates/acp_thread/Cargo.toml b/crates/acp_thread/Cargo.toml index c51b97c067e3bb801cb94f558c90c9eb43d9de39..0dc8d7d447bbf3be370da7a3b69d43eb13702ba2 100644 --- a/crates/acp_thread/Cargo.toml +++ b/crates/acp_thread/Cargo.toml @@ -19,7 +19,6 @@ test-support = ["gpui/test-support", "project/test-support", "dep:parking_lot", action_log.workspace = true agent-client-protocol.workspace = true base64.workspace = true -agent_settings.workspace = true anyhow.workspace = true buffer_diff.workspace = true chrono.workspace = true diff --git a/crates/acp_thread/src/acp_thread.rs b/crates/acp_thread/src/acp_thread.rs index aad35f6999a7ec024c9fcdff317deaf4b3ca539d..ea99df204145d563ce41401de64a7598f63d59d8 100644 --- a/crates/acp_thread/src/acp_thread.rs +++ b/crates/acp_thread/src/acp_thread.rs @@ -3,8 +3,6 @@ mod diff; mod mention; mod terminal; -use agent_settings::AgentSettings; - /// Key used in ACP ToolCall meta to store the tool's programmatic name. /// This is a workaround since ACP's ToolCall doesn't have a dedicated name field. pub const TOOL_NAME_META_KEY: &str = "tool_name"; @@ -40,7 +38,7 @@ pub use mention::*; use project::lsp_store::{FormatTrigger, LspFormatTarget}; use serde::{Deserialize, Serialize}; use serde_json::to_string_pretty; -use settings::Settings as _; + use task::{Shell, ShellBuilder}; pub use terminal::*; @@ -1733,25 +1731,10 @@ impl AcpThread { &mut self, tool_call: acp::ToolCallUpdate, options: PermissionOptions, - respect_always_allow_setting: bool, cx: &mut Context, ) -> Result> { let (tx, rx) = oneshot::channel(); - if respect_always_allow_setting && AgentSettings::get_global(cx).always_allow_tool_actions { - // Don't use AllowAlways, because then if you were to turn off always_allow_tool_actions, - // some tools would (incorrectly) continue to auto-accept. - if let Some(allow_once_option) = options.allow_once_option_id() { - self.upsert_tool_call_inner(tool_call, ToolCallStatus::Pending, cx)?; - return Ok(async { - acp::RequestPermissionOutcome::Selected(acp::SelectedPermissionOutcome::new( - allow_once_option, - )) - } - .boxed()); - } - } - let status = ToolCallStatus::WaitingForConfirmation { options, respond_tx: tx, diff --git a/crates/acp_thread/src/connection.rs b/crates/acp_thread/src/connection.rs index 102b5baa5a9fc5cbb445371e5138ebe7b31d83c4..3bba53847b7bf9910ef5fb286cc41694ec9aef07 100644 --- a/crates/acp_thread/src/connection.rs +++ b/crates/acp_thread/src/connection.rs @@ -477,6 +477,11 @@ impl PermissionOptions { self.first_option_of_kind(acp::PermissionOptionKind::AllowOnce) .map(|option| option.option_id.clone()) } + + pub fn deny_once_option_id(&self) -> Option { + self.first_option_of_kind(acp::PermissionOptionKind::RejectOnce) + .map(|option| option.option_id.clone()) + } } #[cfg(feature = "test-support")] @@ -689,7 +694,6 @@ mod test_support { thread.request_tool_call_authorization( tool_call.clone().into(), options.clone(), - false, cx, ) })?? diff --git a/crates/agent/src/agent.rs b/crates/agent/src/agent.rs index c3e0c28df822eed664c666a0a8ca7a4f4d4a178c..3c4428034950fe1f9c4db17127fbb7be37622bf6 100644 --- a/crates/agent/src/agent.rs +++ b/crates/agent/src/agent.rs @@ -16,6 +16,7 @@ use context_server::ContextServerId; pub use db::*; pub use native_agent_server::NativeAgentServer; pub use pattern_extraction::*; +pub use shell_command_parser::extract_commands; pub use templates::*; pub use thread::*; pub use thread_store::*; @@ -1038,9 +1039,7 @@ impl NativeAgentConnection { context: _, }) => { let outcome_task = acp_thread.update(cx, |thread, cx| { - thread.request_tool_call_authorization( - tool_call, options, true, cx, - ) + thread.request_tool_call_authorization(tool_call, options, cx) })??; cx.background_spawn(async move { if let acp::RequestPermissionOutcome::Selected( diff --git a/crates/agent/src/tests/mod.rs b/crates/agent/src/tests/mod.rs index d165b888ccb3f7350e14d95d3eed2daa269d004e..1dae5c8206f49ba0d3d08912e6f5639800bbe0e5 100644 --- a/crates/agent/src/tests/mod.rs +++ b/crates/agent/src/tests/mod.rs @@ -280,7 +280,7 @@ impl crate::ThreadEnvironment for MultiTerminalEnvironment { fn always_allow_tools(cx: &mut TestAppContext) { cx.update(|cx| { let mut settings = agent_settings::AgentSettings::get_global(cx).clone(); - settings.always_allow_tool_actions = true; + settings.tool_permissions.default = settings::ToolPermissionMode::Allow; agent_settings::AgentSettings::override_global(settings, cx); }); } @@ -1037,9 +1037,11 @@ async fn next_tool_call_authorization( #[test] fn test_permission_options_terminal_with_pattern() { - let permission_options = - ToolPermissionContext::new(TerminalTool::NAME, "cargo build --release") - .build_permission_options(); + let permission_options = ToolPermissionContext::new( + TerminalTool::NAME, + vec!["cargo build --release".to_string()], + ) + .build_permission_options(); let PermissionOptions::Dropdown(choices) = permission_options else { panic!("Expected dropdown permission options"); @@ -1058,7 +1060,8 @@ fn test_permission_options_terminal_with_pattern() { #[test] fn test_permission_options_edit_file_with_path_pattern() { let permission_options = - ToolPermissionContext::new(EditFileTool::NAME, "src/main.rs").build_permission_options(); + ToolPermissionContext::new(EditFileTool::NAME, vec!["src/main.rs".to_string()]) + .build_permission_options(); let PermissionOptions::Dropdown(choices) = permission_options else { panic!("Expected dropdown permission options"); @@ -1074,8 +1077,9 @@ fn test_permission_options_edit_file_with_path_pattern() { #[test] fn test_permission_options_fetch_with_domain_pattern() { - let permission_options = ToolPermissionContext::new(FetchTool::NAME, "https://docs.rs/gpui") - .build_permission_options(); + let permission_options = + ToolPermissionContext::new(FetchTool::NAME, vec!["https://docs.rs/gpui".to_string()]) + .build_permission_options(); let PermissionOptions::Dropdown(choices) = permission_options else { panic!("Expected dropdown permission options"); @@ -1091,9 +1095,11 @@ fn test_permission_options_fetch_with_domain_pattern() { #[test] fn test_permission_options_without_pattern() { - let permission_options = - ToolPermissionContext::new(TerminalTool::NAME, "./deploy.sh --production") - .build_permission_options(); + let permission_options = ToolPermissionContext::new( + TerminalTool::NAME, + vec!["./deploy.sh --production".to_string()], + ) + .build_permission_options(); let PermissionOptions::Dropdown(choices) = permission_options else { panic!("Expected dropdown permission options"); @@ -1111,9 +1117,11 @@ fn test_permission_options_without_pattern() { #[test] fn test_permission_option_ids_for_terminal() { - let permission_options = - ToolPermissionContext::new(TerminalTool::NAME, "cargo build --release") - .build_permission_options(); + let permission_options = ToolPermissionContext::new( + TerminalTool::NAME, + vec!["cargo build --release".to_string()], + ) + .build_permission_options(); let PermissionOptions::Dropdown(choices) = permission_options else { panic!("Expected dropdown permission options"); @@ -1288,7 +1296,7 @@ async fn test_mcp_tools(cx: &mut TestAppContext) { paths::settings_file(), json!({ "agent": { - "always_allow_tool_actions": true, + "tool_permissions": { "default": "allow" }, "profiles": { "test": { "name": "Test Profile", @@ -3731,7 +3739,7 @@ async fn test_terminal_tool_permission_rules(cx: &mut TestAppContext) { settings.tool_permissions.tools.insert( TerminalTool::NAME.into(), agent_settings::ToolRules { - default_mode: settings::ToolPermissionMode::Confirm, + default: Some(settings::ToolPermissionMode::Confirm), always_allow: vec![], always_deny: vec![ agent_settings::CompiledRegex::new(r"rm\s+-rf", false).unwrap(), @@ -3771,7 +3779,7 @@ async fn test_terminal_tool_permission_rules(cx: &mut TestAppContext) { ); } - // Test 2: Allow rule skips confirmation (and overrides default_mode: Deny) + // Test 2: Allow rule skips confirmation (and overrides default: Deny) { let environment = Rc::new(cx.update(|cx| { FakeThreadEnvironment::default() @@ -3780,11 +3788,10 @@ async fn test_terminal_tool_permission_rules(cx: &mut TestAppContext) { cx.update(|cx| { let mut settings = agent_settings::AgentSettings::get_global(cx).clone(); - settings.always_allow_tool_actions = false; settings.tool_permissions.tools.insert( TerminalTool::NAME.into(), agent_settings::ToolRules { - default_mode: settings::ToolPermissionMode::Deny, + default: Some(settings::ToolPermissionMode::Deny), always_allow: vec![ agent_settings::CompiledRegex::new(r"^echo\s", false).unwrap(), ], @@ -3829,7 +3836,7 @@ async fn test_terminal_tool_permission_rules(cx: &mut TestAppContext) { ); } - // Test 3: always_allow_tool_actions=true overrides always_confirm patterns + // Test 3: global default: allow does NOT override always_confirm patterns { let environment = Rc::new(cx.update(|cx| { FakeThreadEnvironment::default() @@ -3838,11 +3845,11 @@ async fn test_terminal_tool_permission_rules(cx: &mut TestAppContext) { cx.update(|cx| { let mut settings = agent_settings::AgentSettings::get_global(cx).clone(); - settings.always_allow_tool_actions = true; + settings.tool_permissions.default = settings::ToolPermissionMode::Allow; settings.tool_permissions.tools.insert( TerminalTool::NAME.into(), agent_settings::ToolRules { - default_mode: settings::ToolPermissionMode::Allow, + default: Some(settings::ToolPermissionMode::Allow), always_allow: vec![], always_deny: vec![], always_confirm: vec![ @@ -3856,9 +3863,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, _rx) = crate::ToolCallEventStream::test(); + let (event_stream, mut rx) = crate::ToolCallEventStream::test(); - let task = cx.update(|cx| { + let _task = cx.update(|cx| { tool.run( crate::TerminalToolInput { command: "sudo rm file".to_string(), @@ -3870,12 +3877,15 @@ async fn test_terminal_tool_permission_rules(cx: &mut TestAppContext) { ) }); - // With always_allow_tool_actions=true, confirm patterns are overridden - task.await - .expect("command should be allowed with always_allow_tool_actions=true"); + // With global default: allow, confirm patterns are still respected + // The expect_authorization() call will panic if no authorization is requested, + // which validates that the confirm pattern still triggers confirmation + let _auth = rx.expect_authorization().await; + + drop(_task); } - // Test 4: always_allow_tool_actions=true overrides default_mode: Deny + // Test 4: tool-specific default: deny is respected even with global default: allow { let environment = Rc::new(cx.update(|cx| { FakeThreadEnvironment::default() @@ -3884,11 +3894,11 @@ async fn test_terminal_tool_permission_rules(cx: &mut TestAppContext) { cx.update(|cx| { let mut settings = agent_settings::AgentSettings::get_global(cx).clone(); - settings.always_allow_tool_actions = true; + settings.tool_permissions.default = settings::ToolPermissionMode::Allow; settings.tool_permissions.tools.insert( TerminalTool::NAME.into(), agent_settings::ToolRules { - default_mode: settings::ToolPermissionMode::Deny, + default: Some(settings::ToolPermissionMode::Deny), always_allow: vec![], always_deny: vec![], always_confirm: vec![], @@ -3914,9 +3924,17 @@ async fn test_terminal_tool_permission_rules(cx: &mut TestAppContext) { ) }); - // 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"); + // tool-specific default: deny is respected even with global default: allow + let result = task.await; + assert!( + result.is_err(), + "expected command to be blocked by tool-specific deny default" + ); + let err_msg = result.unwrap_err().to_string().to_lowercase(); + assert!( + err_msg.contains("disabled"), + "error should mention the tool is disabled, got: {err_msg}" + ); } } @@ -4625,7 +4643,7 @@ async fn test_edit_file_tool_deny_rule_blocks_edit(cx: &mut TestAppContext) { settings.tool_permissions.tools.insert( EditFileTool::NAME.into(), agent_settings::ToolRules { - default_mode: settings::ToolPermissionMode::Allow, + default: Some(settings::ToolPermissionMode::Allow), always_allow: vec![], always_deny: vec![agent_settings::CompiledRegex::new(r"sensitive", false).unwrap()], always_confirm: vec![], @@ -4693,7 +4711,7 @@ async fn test_delete_path_tool_deny_rule_blocks_deletion(cx: &mut TestAppContext settings.tool_permissions.tools.insert( DeletePathTool::NAME.into(), agent_settings::ToolRules { - default_mode: settings::ToolPermissionMode::Allow, + default: Some(settings::ToolPermissionMode::Allow), always_allow: vec![], always_deny: vec![agent_settings::CompiledRegex::new(r"important", false).unwrap()], always_confirm: vec![], @@ -4747,7 +4765,7 @@ async fn test_move_path_tool_denies_if_destination_denied(cx: &mut TestAppContex settings.tool_permissions.tools.insert( MovePathTool::NAME.into(), agent_settings::ToolRules { - default_mode: settings::ToolPermissionMode::Allow, + default: Some(settings::ToolPermissionMode::Allow), always_allow: vec![], always_deny: vec![agent_settings::CompiledRegex::new(r"protected", false).unwrap()], always_confirm: vec![], @@ -4803,7 +4821,7 @@ async fn test_move_path_tool_denies_if_source_denied(cx: &mut TestAppContext) { settings.tool_permissions.tools.insert( MovePathTool::NAME.into(), agent_settings::ToolRules { - default_mode: settings::ToolPermissionMode::Allow, + default: Some(settings::ToolPermissionMode::Allow), always_allow: vec![], always_deny: vec![agent_settings::CompiledRegex::new(r"secret", false).unwrap()], always_confirm: vec![], @@ -4859,7 +4877,7 @@ async fn test_copy_path_tool_deny_rule_blocks_copy(cx: &mut TestAppContext) { settings.tool_permissions.tools.insert( CopyPathTool::NAME.into(), agent_settings::ToolRules { - default_mode: settings::ToolPermissionMode::Allow, + default: Some(settings::ToolPermissionMode::Allow), always_allow: vec![], always_deny: vec![ agent_settings::CompiledRegex::new(r"confidential", false).unwrap(), @@ -4916,7 +4934,7 @@ async fn test_save_file_tool_denies_if_any_path_denied(cx: &mut TestAppContext) settings.tool_permissions.tools.insert( SaveFileTool::NAME.into(), agent_settings::ToolRules { - default_mode: settings::ToolPermissionMode::Allow, + default: Some(settings::ToolPermissionMode::Allow), always_allow: vec![], always_deny: vec![agent_settings::CompiledRegex::new(r"readonly", false).unwrap()], always_confirm: vec![], @@ -4965,11 +4983,10 @@ async fn test_save_file_tool_respects_deny_rules(cx: &mut TestAppContext) { cx.update(|cx| { let mut settings = agent_settings::AgentSettings::get_global(cx).clone(); - settings.always_allow_tool_actions = false; settings.tool_permissions.tools.insert( SaveFileTool::NAME.into(), agent_settings::ToolRules { - default_mode: settings::ToolPermissionMode::Allow, + default: Some(settings::ToolPermissionMode::Allow), always_allow: vec![], always_deny: vec![agent_settings::CompiledRegex::new(r"\.secret$", false).unwrap()], always_confirm: vec![], @@ -5010,7 +5027,7 @@ async fn test_web_search_tool_deny_rule_blocks_search(cx: &mut TestAppContext) { settings.tool_permissions.tools.insert( WebSearchTool::NAME.into(), agent_settings::ToolRules { - default_mode: settings::ToolPermissionMode::Allow, + default: Some(settings::ToolPermissionMode::Allow), always_allow: vec![], always_deny: vec![ agent_settings::CompiledRegex::new(r"internal\.company", false).unwrap(), @@ -5050,11 +5067,10 @@ async fn test_edit_file_tool_allow_rule_skips_confirmation(cx: &mut TestAppConte cx.update(|cx| { let mut settings = agent_settings::AgentSettings::get_global(cx).clone(); - settings.always_allow_tool_actions = false; settings.tool_permissions.tools.insert( EditFileTool::NAME.into(), agent_settings::ToolRules { - default_mode: settings::ToolPermissionMode::Confirm, + default: Some(settings::ToolPermissionMode::Confirm), always_allow: vec![agent_settings::CompiledRegex::new(r"\.md$", false).unwrap()], always_deny: vec![], always_confirm: vec![], @@ -5109,6 +5125,71 @@ async fn test_edit_file_tool_allow_rule_skips_confirmation(cx: &mut TestAppConte ); } +#[gpui::test] +async fn test_edit_file_tool_allow_still_prompts_for_local_settings(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + "/root", + json!({ + ".zed": { + "settings.json": "{}" + }, + "README.md": "# Hello" + }), + ) + .await; + let project = Project::test(fs.clone(), ["/root".as_ref()], cx).await; + + cx.update(|cx| { + let mut settings = agent_settings::AgentSettings::get_global(cx).clone(); + settings.tool_permissions.default = settings::ToolPermissionMode::Allow; + agent_settings::AgentSettings::override_global(settings, cx); + }); + + let context_server_registry = + cx.new(|cx| crate::ContextServerRegistry::new(project.read(cx).context_server_store(), cx)); + let language_registry = project.read_with(cx, |project, _cx| project.languages().clone()); + let templates = crate::Templates::new(); + let thread = cx.new(|cx| { + crate::Thread::new( + project.clone(), + cx.new(|_cx| prompt_store::ProjectContext::default()), + context_server_registry, + templates.clone(), + None, + cx, + ) + }); + + #[allow(clippy::arc_with_non_send_sync)] + let tool = Arc::new(crate::EditFileTool::new( + project, + thread.downgrade(), + language_registry, + templates, + )); + + // Editing a file inside .zed/ should still prompt even with global default: allow, + // because local settings paths are sensitive and require confirmation regardless. + let (event_stream, mut rx) = crate::ToolCallEventStream::test(); + let _task = cx.update(|cx| { + tool.run( + crate::EditFileToolInput { + display_description: "Edit local settings".to_string(), + path: "root/.zed/settings.json".into(), + mode: crate::EditFileMode::Edit, + }, + event_stream, + cx, + ) + }); + + let _update = rx.expect_update_fields().await; + let _auth = rx.expect_authorization().await; +} + #[gpui::test] async fn test_fetch_tool_deny_rule_blocks_url(cx: &mut TestAppContext) { init_test(cx); @@ -5118,7 +5199,7 @@ async fn test_fetch_tool_deny_rule_blocks_url(cx: &mut TestAppContext) { settings.tool_permissions.tools.insert( FetchTool::NAME.into(), agent_settings::ToolRules { - default_mode: settings::ToolPermissionMode::Allow, + default: Some(settings::ToolPermissionMode::Allow), always_allow: vec![], always_deny: vec![ agent_settings::CompiledRegex::new(r"internal\.company\.com", false).unwrap(), @@ -5155,11 +5236,10 @@ async fn test_fetch_tool_allow_rule_skips_confirmation(cx: &mut TestAppContext) cx.update(|cx| { let mut settings = agent_settings::AgentSettings::get_global(cx).clone(); - settings.always_allow_tool_actions = false; settings.tool_permissions.tools.insert( FetchTool::NAME.into(), agent_settings::ToolRules { - default_mode: settings::ToolPermissionMode::Confirm, + default: Some(settings::ToolPermissionMode::Confirm), always_allow: vec![agent_settings::CompiledRegex::new(r"docs\.rs", false).unwrap()], always_deny: vec![], always_confirm: vec![], diff --git a/crates/agent/src/tests/test_tools.rs b/crates/agent/src/tests/test_tools.rs index eb9018960180a167951cbba5458aa110377f2b16..ed304a3bb6c4ea9b9eea75fdcfd245e265714080 100644 --- a/crates/agent/src/tests/test_tools.rs +++ b/crates/agent/src/tests/test_tools.rs @@ -120,7 +120,7 @@ impl AgentTool for ToolRequiringPermission { cx: &mut App, ) -> Task> { let settings = AgentSettings::get_global(cx); - let decision = decide_permission_from_settings(Self::NAME, "", settings); + let decision = decide_permission_from_settings(Self::NAME, &[String::new()], settings); let authorize = match decision { ToolPermissionDecision::Allow => None, @@ -130,7 +130,7 @@ impl AgentTool for ToolRequiringPermission { ToolPermissionDecision::Confirm => { let context = crate::ToolPermissionContext { tool_name: "tool_requiring_permission".to_string(), - input_value: String::new(), + input_values: vec![String::new()], }; Some(event_stream.authorize("Authorize?", context, cx)) } diff --git a/crates/agent/src/thread.rs b/crates/agent/src/thread.rs index 6acc71b3dcb7ec582512b71557c6bd7d077c3f62..b062cfda357b809a074e65dd49f989a243af478c 100644 --- a/crates/agent/src/thread.rs +++ b/crates/agent/src/thread.rs @@ -627,14 +627,14 @@ pub struct NewTerminal { #[derive(Debug, Clone)] pub struct ToolPermissionContext { pub tool_name: String, - pub input_value: String, + pub input_values: Vec, } impl ToolPermissionContext { - pub fn new(tool_name: impl Into, input_value: impl Into) -> Self { + pub fn new(tool_name: impl Into, input_values: Vec) -> Self { Self { tool_name: tool_name.into(), - input_value: input_value.into(), + input_values, } } @@ -667,7 +667,7 @@ impl ToolPermissionContext { use util::shell::ShellKind; let tool_name = &self.tool_name; - let input_value = &self.input_value; + let input_values = &self.input_values; // Check if the user's shell supports POSIX-like command chaining. // See the doc comment above for the full explanation of why this is needed. @@ -677,35 +677,50 @@ impl ToolPermissionContext { true }; - let (pattern, pattern_display) = if tool_name == TerminalTool::NAME { - ( - extract_terminal_pattern(input_value), - extract_terminal_pattern_display(input_value), - ) - } else if tool_name == CopyPathTool::NAME || tool_name == MovePathTool::NAME { - // input_value is "source\ndestination"; extract a pattern from the - // common parent directory of both paths so that "always allow" covers - // future checks against both the source and the destination. - ( - extract_copy_move_pattern(input_value), - extract_copy_move_pattern_display(input_value), - ) - } else if tool_name == EditFileTool::NAME - || tool_name == DeletePathTool::NAME - || tool_name == CreateDirectoryTool::NAME - || tool_name == SaveFileTool::NAME - { - ( - extract_path_pattern(input_value), - extract_path_pattern_display(input_value), - ) - } else if tool_name == FetchTool::NAME { - ( - extract_url_pattern(input_value), - extract_url_pattern_display(input_value), - ) - } else { - (None, None) + let extract_for_value = |value: &str| -> (Option, Option) { + if tool_name == TerminalTool::NAME { + ( + extract_terminal_pattern(value), + extract_terminal_pattern_display(value), + ) + } else if tool_name == CopyPathTool::NAME + || tool_name == MovePathTool::NAME + || tool_name == EditFileTool::NAME + || tool_name == DeletePathTool::NAME + || tool_name == CreateDirectoryTool::NAME + || tool_name == SaveFileTool::NAME + { + ( + extract_path_pattern(value), + extract_path_pattern_display(value), + ) + } else if tool_name == FetchTool::NAME { + ( + extract_url_pattern(value), + extract_url_pattern_display(value), + ) + } else { + (None, None) + } + }; + + // Extract patterns from all input values. Only offer a pattern-specific + // "always allow/deny" button when every value produces the same pattern. + let (pattern, pattern_display) = match input_values.as_slice() { + [single] => extract_for_value(single), + _ => { + let mut iter = input_values.iter().map(|v| extract_for_value(v)); + match iter.next() { + Some(first) => { + if iter.all(|pair| pair.0 == first.0) { + first + } else { + (None, None) + } + } + None => (None, None), + } + } }; let mut choices = Vec::new(); @@ -3090,10 +3105,10 @@ impl ToolCallEventStream { /// Authorize a third-party tool (e.g., MCP tool from a context server). /// /// Unlike built-in tools, third-party tools don't support pattern-based permissions. - /// They only support `default_mode` (allow/deny/confirm) per tool. + /// They only support `default` (allow/deny/confirm) per tool. /// /// Uses the dropdown authorization flow with two granularities: - /// - "Always for MCP tool" → sets `tools..default_mode = "allow"` or "deny" + /// - "Always for MCP tool" → sets `tools..default = "allow"` or "deny" /// - "Only this time" → allow/deny once pub fn authorize_third_party_tool( &self, @@ -3104,7 +3119,7 @@ impl ToolCallEventStream { ) -> Task> { let settings = agent_settings::AgentSettings::get_global(cx); - let decision = decide_permission_from_settings(&tool_id, "", &settings); + let decision = decide_permission_from_settings(&tool_id, &[String::new()], &settings); match decision { ToolPermissionDecision::Allow => return Task::ready(Ok(())), @@ -3176,7 +3191,7 @@ impl ToolCallEventStream { settings .agent .get_or_insert_default() - .set_tool_default_mode(&tool_id, ToolPermissionMode::Allow); + .set_tool_default_permission(&tool_id, ToolPermissionMode::Allow); }); }); } @@ -3189,7 +3204,7 @@ impl ToolCallEventStream { settings .agent .get_or_insert_default() - .set_tool_default_mode(&tool_id, ToolPermissionMode::Deny); + .set_tool_default_permission(&tool_id, ToolPermissionMode::Deny); }); }); } @@ -3249,7 +3264,7 @@ impl ToolCallEventStream { settings .agent .get_or_insert_default() - .set_tool_default_mode(&tool, ToolPermissionMode::Allow); + .set_tool_default_permission(&tool, ToolPermissionMode::Allow); }); }); } @@ -3265,7 +3280,7 @@ impl ToolCallEventStream { settings .agent .get_or_insert_default() - .set_tool_default_mode(&tool, ToolPermissionMode::Deny); + .set_tool_default_permission(&tool, ToolPermissionMode::Deny); }); }); } diff --git a/crates/agent/src/tool_permissions.rs b/crates/agent/src/tool_permissions.rs index 220122db75d61caa912a784f9b12668fe058fbae..efafe917d8cca94fd78b4f3cbdd9fb505ab6ab8f 100644 --- a/crates/agent/src/tool_permissions.rs +++ b/crates/agent/src/tool_permissions.rs @@ -63,7 +63,7 @@ pub static HARDCODED_SECURITY_RULES: LazyLock = LazyLock /// Returns a Deny decision if blocked, None otherwise. fn check_hardcoded_security_rules( tool_name: &str, - input: &str, + inputs: &[String], shell_kind: ShellKind, ) -> Option { // Currently only terminal tool has hardcoded rules @@ -74,21 +74,23 @@ fn check_hardcoded_security_rules( let rules = &*HARDCODED_SECURITY_RULES; let terminal_patterns = &rules.terminal_deny; - // First: check the original input as-is (and its path-normalized form) - if matches_hardcoded_patterns(input, terminal_patterns) { - return Some(ToolPermissionDecision::Deny( - HARDCODED_SECURITY_DENIAL_MESSAGE.into(), - )); - } + for input in inputs { + // First: check the original input as-is (and its path-normalized form) + if matches_hardcoded_patterns(input, terminal_patterns) { + return Some(ToolPermissionDecision::Deny( + HARDCODED_SECURITY_DENIAL_MESSAGE.into(), + )); + } - // Second: parse and check individual sub-commands (for chained commands) - if shell_kind.supports_posix_chaining() { - if let Some(commands) = extract_commands(input) { - for command in &commands { - if matches_hardcoded_patterns(command, terminal_patterns) { - return Some(ToolPermissionDecision::Deny( - HARDCODED_SECURITY_DENIAL_MESSAGE.into(), - )); + // Second: parse and check individual sub-commands (for chained commands) + if shell_kind.supports_posix_chaining() { + if let Some(commands) = extract_commands(input) { + for command in &commands { + if matches_hardcoded_patterns(command, terminal_patterns) { + return Some(ToolPermissionDecision::Deny( + HARDCODED_SECURITY_DENIAL_MESSAGE.into(), + )); + } } } } @@ -209,17 +211,17 @@ impl ToolPermissionDecision { /// # Precedence Order (highest to lowest) /// /// 1. **Hardcoded security rules** - Critical safety checks (e.g., blocking `rm -rf /`) - /// that cannot be bypassed by any user settings, including `always_allow_tool_actions`. - /// 2. **`always_allow_tool_actions`** - When enabled, allows all tool actions without - /// prompting. This global setting bypasses user-configured deny/confirm/allow patterns, - /// but does **not** bypass hardcoded security rules. - /// 3. **`always_deny`** - If any deny pattern matches, the tool call is blocked immediately. + /// that cannot be bypassed by any user settings. + /// 2. **`always_deny`** - If any deny pattern matches, the tool call is blocked immediately. /// This takes precedence over `always_confirm` and `always_allow` patterns. - /// 4. **`always_confirm`** - If any confirm pattern matches (and no deny matched), + /// 3. **`always_confirm`** - If any confirm pattern matches (and no deny matched), /// the user is prompted for confirmation. - /// 5. **`always_allow`** - If any allow pattern matches (and no deny/confirm matched), + /// 4. **`always_allow`** - If any allow pattern matches (and no deny/confirm matched), /// the tool call proceeds without prompting. - /// 6. **`default_mode`** - If no patterns match, falls back to the tool's default mode. + /// 5. **Tool-specific `default`** - If no patterns match and the tool has an explicit + /// `default` configured, that mode is used. + /// 6. **Global `default`** - Falls back to `tool_permissions.default` when no + /// tool-specific default is set, or when the tool has no entry at all. /// /// # Shell Compatibility (Terminal Tool Only) /// @@ -244,27 +246,27 @@ impl ToolPermissionDecision { /// - Use `^` and `$` anchors to match the start/end of the input. pub fn from_input( tool_name: &str, - input: &str, + inputs: &[String], permissions: &ToolPermissions, - always_allow_tool_actions: bool, shell_kind: ShellKind, ) -> ToolPermissionDecision { // First, check hardcoded security rules, such as banning `rm -rf /` in terminal tool. // These cannot be bypassed by any user settings. - if let Some(denial) = check_hardcoded_security_rules(tool_name, input, shell_kind) { + if let Some(denial) = check_hardcoded_security_rules(tool_name, inputs, shell_kind) { return denial; } - // If always_allow_tool_actions is enabled, bypass user-configured permission checks. - // Note: This no longer bypasses hardcoded security rules (checked above). - if always_allow_tool_actions { - return ToolPermissionDecision::Allow; - } - let rules = match permissions.tools.get(tool_name) { Some(rules) => rules, None => { - return ToolPermissionDecision::Confirm; + // No tool-specific rules, use the global default + return match permissions.default { + ToolPermissionMode::Allow => ToolPermissionDecision::Allow, + ToolPermissionMode::Deny => { + ToolPermissionDecision::Deny("Blocked by global default: deny".into()) + } + ToolPermissionMode::Confirm => ToolPermissionDecision::Confirm, + }; } }; @@ -274,7 +276,7 @@ impl ToolPermissionDecision { return ToolPermissionDecision::Deny(error); } - // For the terminal tool, parse the command to extract all sub-commands. + // For the terminal tool, parse each input command to extract all sub-commands. // This prevents shell injection attacks where a user configures an allow // pattern like "^ls" and an attacker crafts "ls && rm -rf /". // @@ -299,21 +301,43 @@ impl ToolPermissionDecision { )); } // No always_allow rules, so we can still check deny/confirm patterns. - return check_commands(std::iter::once(input.to_string()), rules, tool_name, false); + return check_commands( + inputs.iter().map(|s| s.to_string()), + rules, + tool_name, + false, + permissions.default, + ); } - match extract_commands(input) { - Some(commands) => check_commands(commands, rules, tool_name, true), - None => { - // The command failed to parse, so we check to see if we should auto-deny - // or auto-confirm; if neither auto-deny nor auto-confirm applies here, - // fall back on the default (based on the user's settings, which is Confirm - // if not specified otherwise). Ignore "always allow" when it failed to parse. - check_commands(std::iter::once(input.to_string()), rules, tool_name, false) + // Expand each input into its sub-commands and check them all together. + let mut all_commands = Vec::new(); + let mut any_parse_failed = false; + for input in inputs { + match extract_commands(input) { + Some(commands) => all_commands.extend(commands), + None => { + any_parse_failed = true; + all_commands.push(input.to_string()); + } } } + // If any command failed to parse, disable allow patterns for safety. + check_commands( + all_commands, + rules, + tool_name, + !any_parse_failed, + permissions.default, + ) } else { - check_commands(std::iter::once(input.to_string()), rules, tool_name, true) + check_commands( + inputs.iter().map(|s| s.to_string()), + rules, + tool_name, + true, + permissions.default, + ) } } } @@ -333,6 +357,7 @@ fn check_commands( rules: &ToolRules, tool_name: &str, allow_enabled: bool, + global_default: ToolPermissionMode, ) -> ToolPermissionDecision { // Single pass through all commands: // - DENY: If ANY command matches a deny pattern, deny immediately (short-circuit) @@ -373,7 +398,7 @@ fn check_commands( return ToolPermissionDecision::Allow; } - match rules.default_mode { + match rules.default.unwrap_or(global_default) { ToolPermissionMode::Deny => { ToolPermissionDecision::Deny(format!("{} tool is disabled", tool_name)) } @@ -402,24 +427,23 @@ fn check_invalid_patterns(tool_name: &str, rules: &ToolRules) -> Option /// Convenience wrapper that extracts permission settings from `AgentSettings`. /// /// This is the primary entry point for tools to check permissions. It extracts -/// `tool_permissions` and `always_allow_tool_actions` from the settings and +/// `tool_permissions` from the settings and /// delegates to [`ToolPermissionDecision::from_input`], using the system shell. pub fn decide_permission_from_settings( tool_name: &str, - input: &str, + inputs: &[String], settings: &AgentSettings, ) -> ToolPermissionDecision { ToolPermissionDecision::from_input( tool_name, - input, + inputs, &settings.tool_permissions, - settings.always_allow_tool_actions, ShellKind::system(), ) } /// Normalizes a path by collapsing `.` and `..` segments without touching the filesystem. -fn normalize_path(raw: &str) -> String { +pub fn normalize_path(raw: &str) -> String { let is_absolute = Path::new(raw).has_root(); let mut components: Vec<&str> = Vec::new(); for component in Path::new(raw).components() { @@ -452,24 +476,37 @@ fn normalize_path(raw: &str) -> String { /// Decides permission by checking both the raw input path and a simplified/canonicalized /// version. Returns the most restrictive decision (Deny > Confirm > Allow). -pub fn decide_permission_for_path( +pub fn decide_permission_for_paths( tool_name: &str, - raw_path: &str, + raw_paths: &[String], settings: &AgentSettings, ) -> ToolPermissionDecision { - let raw_decision = decide_permission_from_settings(tool_name, raw_path, settings); - - let simplified = normalize_path(raw_path); - if simplified == raw_path { + let raw_inputs: Vec = raw_paths.to_vec(); + let raw_decision = decide_permission_from_settings(tool_name, &raw_inputs, settings); + + let normalized: Vec = raw_paths.iter().map(|p| normalize_path(p)).collect(); + let any_changed = raw_paths + .iter() + .zip(&normalized) + .any(|(raw, norm)| raw != norm); + if !any_changed { return raw_decision; } - let simplified_decision = decide_permission_from_settings(tool_name, &simplified, settings); + let normalized_decision = decide_permission_from_settings(tool_name, &normalized, settings); - most_restrictive(raw_decision, simplified_decision) + most_restrictive(raw_decision, normalized_decision) } -fn most_restrictive( +pub fn decide_permission_for_path( + tool_name: &str, + raw_path: &str, + settings: &AgentSettings, +) -> ToolPermissionDecision { + decide_permission_for_paths(tool_name, &[raw_path.to_string()], settings) +} + +pub fn most_restrictive( a: ToolPermissionDecision, b: ToolPermissionDecision, ) -> ToolPermissionDecision { @@ -488,16 +525,13 @@ mod tests { use super::*; use crate::AgentTool; use crate::pattern_extraction::extract_terminal_pattern; - use crate::tools::{EditFileTool, TerminalTool}; + use crate::tools::{DeletePathTool, EditFileTool, FetchTool, TerminalTool}; use agent_settings::{AgentProfileId, CompiledRegex, InvalidRegexPattern, ToolRules}; use gpui::px; use settings::{DefaultAgentView, DockPosition, DockSide, NotifyWhenAgentWaiting}; use std::sync::Arc; - fn test_agent_settings( - tool_permissions: ToolPermissions, - always_allow_tool_actions: bool, - ) -> AgentSettings { + fn test_agent_settings(tool_permissions: ToolPermissions) -> AgentSettings { AgentSettings { enabled: true, button: true, @@ -515,7 +549,6 @@ mod tests { default_profile: AgentProfileId::default(), default_view: DefaultAgentView::Thread, profiles: Default::default(), - always_allow_tool_actions, notify_when_agent_waiting: NotifyWhenAgentWaiting::default(), play_sound_when_agent_done: false, single_file_review: false, @@ -542,11 +575,11 @@ mod tests { struct PermTest { tool: &'static str, input: &'static str, - mode: ToolPermissionMode, + mode: Option, allow: Vec<(&'static str, bool)>, deny: Vec<(&'static str, bool)>, confirm: Vec<(&'static str, bool)>, - global: bool, + global_default: ToolPermissionMode, shell: ShellKind, } @@ -555,11 +588,11 @@ mod tests { Self { tool: TerminalTool::NAME, input, - mode: ToolPermissionMode::Confirm, + mode: None, allow: vec![], deny: vec![], confirm: vec![], - global: false, + global_default: ToolPermissionMode::Confirm, shell: ShellKind::Posix, } } @@ -569,7 +602,7 @@ mod tests { self } fn mode(mut self, m: ToolPermissionMode) -> Self { - self.mode = m; + self.mode = Some(m); self } fn allow(mut self, p: &[&'static str]) -> Self { @@ -592,8 +625,8 @@ mod tests { self.confirm = p.iter().map(|s| (*s, false)).collect(); self } - fn global(mut self, g: bool) -> Self { - self.global = g; + fn global_default(mut self, m: ToolPermissionMode) -> Self { + self.global_default = m; self } fn shell(mut self, s: ShellKind) -> Self { @@ -630,30 +663,41 @@ mod tests { tools.insert( Arc::from(self.tool), ToolRules { - default_mode: self.mode, + default: self.mode, always_allow: self .allow .iter() - .filter_map(|(p, cs)| CompiledRegex::new(p, *cs)) + .map(|(p, cs)| { + CompiledRegex::new(p, *cs) + .unwrap_or_else(|| panic!("invalid regex in test: {p:?}")) + }) .collect(), always_deny: self .deny .iter() - .filter_map(|(p, cs)| CompiledRegex::new(p, *cs)) + .map(|(p, cs)| { + CompiledRegex::new(p, *cs) + .unwrap_or_else(|| panic!("invalid regex in test: {p:?}")) + }) .collect(), always_confirm: self .confirm .iter() - .filter_map(|(p, cs)| CompiledRegex::new(p, *cs)) + .map(|(p, cs)| { + CompiledRegex::new(p, *cs) + .unwrap_or_else(|| panic!("invalid regex in test: {p:?}")) + }) .collect(), invalid_patterns: vec![], }, ); ToolPermissionDecision::from_input( self.tool, - self.input, - &ToolPermissions { tools }, - self.global, + &[self.input.to_string()], + &ToolPermissions { + default: self.global_default, + tools, + }, self.shell, ) } @@ -663,14 +707,14 @@ mod tests { PermTest::new(input) } - fn no_rules(input: &str, global: bool) -> ToolPermissionDecision { + fn no_rules(input: &str, global_default: ToolPermissionMode) -> ToolPermissionDecision { ToolPermissionDecision::from_input( TerminalTool::NAME, - input, + &[input.to_string()], &ToolPermissions { + default: global_default, tools: collections::HashMap::default(), }, - global, ShellKind::Posix, ) } @@ -707,7 +751,23 @@ mod tests { fn allow_no_match_global_allows() { t("python x.py") .allow(&[pattern("cargo")]) - .global(true) + .global_default(ToolPermissionMode::Allow) + .is_allow(); + } + #[test] + fn allow_no_match_tool_confirm_overrides_global_allow() { + t("python x.py") + .allow(&[pattern("cargo")]) + .mode(ToolPermissionMode::Confirm) + .global_default(ToolPermissionMode::Allow) + .is_confirm(); + } + #[test] + fn allow_no_match_tool_allow_overrides_global_confirm() { + t("python x.py") + .allow(&[pattern("cargo")]) + .mode(ToolPermissionMode::Allow) + .global_default(ToolPermissionMode::Confirm) .is_allow(); } @@ -716,13 +776,13 @@ mod tests { fn deny_blocks() { t("rm -rf ./temp").deny(&["rm\\s+-rf"]).is_deny(); } + // global default: allow does NOT bypass user-configured deny rules #[test] - fn global_bypasses_user_deny() { - // always_allow_tool_actions bypasses user-configured deny rules + fn deny_not_bypassed_by_global_default_allow() { t("rm -rf ./temp") .deny(&["rm\\s+-rf"]) - .global(true) - .is_allow(); + .global_default(ToolPermissionMode::Allow) + .is_deny(); } #[test] fn deny_blocks_with_mode_allow() { @@ -750,12 +810,13 @@ mod tests { .confirm(&[pattern("sudo")]) .is_confirm(); } + // global default: allow does NOT bypass user-configured confirm rules #[test] - fn global_overrides_confirm() { + fn global_default_allow_does_not_override_confirm_pattern() { t("sudo reboot") .confirm(&[pattern("sudo")]) - .global(true) - .is_allow(); + .global_default(ToolPermissionMode::Allow) + .is_confirm(); } #[test] fn confirm_overrides_mode_allow() { @@ -815,7 +876,7 @@ mod tests { .is_deny(); } - // no patterns -> default_mode + // no patterns -> default #[test] fn default_confirm() { t("python x.py") @@ -830,25 +891,38 @@ mod tests { fn default_deny() { t("python x.py").mode(ToolPermissionMode::Deny).is_deny(); } + // Tool-specific default takes precedence over global default #[test] - fn default_deny_global_true() { + fn tool_default_deny_overrides_global_allow() { t("python x.py") .mode(ToolPermissionMode::Deny) - .global(true) - .is_allow(); + .global_default(ToolPermissionMode::Allow) + .is_deny(); } + // Tool-specific default takes precedence over global default #[test] - fn default_confirm_global_true() { + fn tool_default_confirm_overrides_global_allow() { t("x") .mode(ToolPermissionMode::Confirm) - .global(true) - .is_allow(); + .global_default(ToolPermissionMode::Allow) + .is_confirm(); } #[test] - fn no_rules_confirms_by_default() { - assert_eq!(no_rules("x", false), ToolPermissionDecision::Confirm); + fn no_rules_uses_global_default() { + assert_eq!( + no_rules("x", ToolPermissionMode::Confirm), + ToolPermissionDecision::Confirm + ); + assert_eq!( + no_rules("x", ToolPermissionMode::Allow), + ToolPermissionDecision::Allow + ); + assert!(matches!( + no_rules("x", ToolPermissionMode::Deny), + ToolPermissionDecision::Deny(_) + )); } #[test] @@ -889,7 +963,7 @@ mod tests { tools.insert( Arc::from(TerminalTool::NAME), ToolRules { - default_mode: ToolPermissionMode::Deny, + default: Some(ToolPermissionMode::Deny), always_allow: vec![], always_deny: vec![], always_confirm: vec![], @@ -899,26 +973,22 @@ mod tests { tools.insert( Arc::from(EditFileTool::NAME), ToolRules { - default_mode: ToolPermissionMode::Allow, + default: Some(ToolPermissionMode::Allow), always_allow: vec![], always_deny: vec![], always_confirm: vec![], invalid_patterns: vec![], }, ); - let p = ToolPermissions { tools }; - // With always_allow_tool_actions=true, even default_mode: Deny is overridden - assert_eq!( - ToolPermissionDecision::from_input(TerminalTool::NAME, "x", &p, true, ShellKind::Posix), - ToolPermissionDecision::Allow - ); - // With always_allow_tool_actions=false, default_mode: Deny is respected + let p = ToolPermissions { + default: ToolPermissionMode::Confirm, + tools, + }; assert!(matches!( ToolPermissionDecision::from_input( TerminalTool::NAME, - "x", + &["x".to_string()], &p, - false, ShellKind::Posix ), ToolPermissionDecision::Deny(_) @@ -926,9 +996,8 @@ mod tests { assert_eq!( ToolPermissionDecision::from_input( EditFileTool::NAME, - "x", + &["x".to_string()], &p, - false, ShellKind::Posix ), ToolPermissionDecision::Allow @@ -941,35 +1010,37 @@ mod tests { tools.insert( Arc::from("term"), ToolRules { - default_mode: ToolPermissionMode::Deny, + default: Some(ToolPermissionMode::Deny), always_allow: vec![], always_deny: vec![], always_confirm: vec![], invalid_patterns: vec![], }, ); - let p = ToolPermissions { tools }; + let p = ToolPermissions { + default: ToolPermissionMode::Confirm, + tools, + }; // "terminal" should not match "term" rules, so falls back to Confirm (no rules) assert_eq!( ToolPermissionDecision::from_input( TerminalTool::NAME, - "x", + &["x".to_string()], &p, - false, ShellKind::Posix ), ToolPermissionDecision::Confirm ); } - // invalid patterns block the tool (but global bypasses all checks) + // invalid patterns block the tool #[test] fn invalid_pattern_blocks() { let mut tools = collections::HashMap::default(); tools.insert( Arc::from(TerminalTool::NAME), ToolRules { - default_mode: ToolPermissionMode::Allow, + default: Some(ToolPermissionMode::Allow), always_allow: vec![CompiledRegex::new("echo", false).unwrap()], always_deny: vec![], always_confirm: vec![], @@ -981,26 +1052,15 @@ mod tests { }, ); let p = ToolPermissions { - tools: tools.clone(), + default: ToolPermissionMode::Confirm, + tools, }; - // With global=true, all checks are bypassed including invalid pattern check - assert!(matches!( - ToolPermissionDecision::from_input( - TerminalTool::NAME, - "echo hi", - &p, - true, - ShellKind::Posix - ), - ToolPermissionDecision::Allow - )); - // With global=false, invalid patterns block the tool + // Invalid patterns block the tool regardless of other settings assert!(matches!( ToolPermissionDecision::from_input( TerminalTool::NAME, - "echo hi", + &["echo hi".to_string()], &p, - false, ShellKind::Posix ), ToolPermissionDecision::Deny(_) @@ -1171,8 +1231,8 @@ mod tests { t("") .tool("mcp:gh:issue") .mode(ToolPermissionMode::Confirm) - .global(true) - .is_allow(); + .global_default(ToolPermissionMode::Allow) + .is_confirm(); } #[test] @@ -1181,7 +1241,7 @@ mod tests { tools.insert( Arc::from(TerminalTool::NAME), ToolRules { - default_mode: ToolPermissionMode::Deny, + default: Some(ToolPermissionMode::Deny), always_allow: vec![], always_deny: vec![], always_confirm: vec![], @@ -1191,20 +1251,22 @@ mod tests { tools.insert( Arc::from("mcp:srv:terminal"), ToolRules { - default_mode: ToolPermissionMode::Allow, + default: Some(ToolPermissionMode::Allow), always_allow: vec![], always_deny: vec![], always_confirm: vec![], invalid_patterns: vec![], }, ); - let p = ToolPermissions { tools }; + let p = ToolPermissions { + default: ToolPermissionMode::Confirm, + tools, + }; assert!(matches!( ToolPermissionDecision::from_input( TerminalTool::NAME, - "x", + &["x".to_string()], &p, - false, ShellKind::Posix ), ToolPermissionDecision::Deny(_) @@ -1212,9 +1274,8 @@ mod tests { assert_eq!( ToolPermissionDecision::from_input( "mcp:srv:terminal", - "x", + &["x".to_string()], &p, - false, ShellKind::Posix ), ToolPermissionDecision::Allow @@ -1294,7 +1355,7 @@ mod tests { tools.insert( Arc::from(TerminalTool::NAME), ToolRules { - default_mode: ToolPermissionMode::Allow, + default: Some(ToolPermissionMode::Allow), always_allow: vec![], always_deny: vec![], always_confirm: vec![], @@ -1312,13 +1373,15 @@ mod tests { ], }, ); - let p = ToolPermissions { tools }; + let p = ToolPermissions { + default: ToolPermissionMode::Confirm, + tools, + }; let result = ToolPermissionDecision::from_input( TerminalTool::NAME, - "echo hi", + &["echo hi".to_string()], &p, - false, ShellKind::Posix, ); match result { @@ -1333,6 +1396,64 @@ mod tests { } } + // always_confirm patterns on non-terminal tools + #[test] + fn always_confirm_works_for_file_tools() { + t("sensitive.env") + .tool(EditFileTool::NAME) + .confirm(&["sensitive"]) + .is_confirm(); + + t("normal.txt") + .tool(EditFileTool::NAME) + .confirm(&["sensitive"]) + .mode(ToolPermissionMode::Allow) + .is_allow(); + + t("/etc/config") + .tool(DeletePathTool::NAME) + .confirm(&["/etc/"]) + .is_confirm(); + + t("/home/user/safe.txt") + .tool(DeletePathTool::NAME) + .confirm(&["/etc/"]) + .mode(ToolPermissionMode::Allow) + .is_allow(); + + t("https://secret.internal.com/api") + .tool(FetchTool::NAME) + .confirm(&["secret\\.internal"]) + .is_confirm(); + + t("https://public.example.com/api") + .tool(FetchTool::NAME) + .confirm(&["secret\\.internal"]) + .mode(ToolPermissionMode::Allow) + .is_allow(); + + // confirm on non-terminal tools still beats allow + t("sensitive.env") + .tool(EditFileTool::NAME) + .allow(&["sensitive"]) + .confirm(&["\\.env$"]) + .is_confirm(); + + // confirm on non-terminal tools is still beaten by deny + t("sensitive.env") + .tool(EditFileTool::NAME) + .confirm(&["sensitive"]) + .deny(&["\\.env$"]) + .is_deny(); + + // global default allow does not bypass confirm on non-terminal tools + t("/etc/passwd") + .tool(EditFileTool::NAME) + .confirm(&["/etc/"]) + .global_default(ToolPermissionMode::Allow) + .is_confirm(); + } + // Hardcoded security rules tests - these rules CANNOT be bypassed #[test] @@ -1344,6 +1465,7 @@ mod tests { t("rm -r -f /").is_deny(); t("rm -f -r /").is_deny(); t("RM -RF /").is_deny(); + t("rm /").is_deny(); // Long flags t("rm --recursive --force /").is_deny(); t("rm --force --recursive /").is_deny(); @@ -1441,12 +1563,22 @@ mod tests { #[test] fn hardcoded_cannot_be_bypassed_by_global() { - // Even with always_allow_tool_actions=true, hardcoded rules block - t("rm -rf /").global(true).is_deny(); - t("rm -rf ~").global(true).is_deny(); - t("rm -rf $HOME").global(true).is_deny(); - t("rm -rf .").global(true).is_deny(); - t("rm -rf ..").global(true).is_deny(); + // Even with global default Allow, hardcoded rules block + t("rm -rf /") + .global_default(ToolPermissionMode::Allow) + .is_deny(); + t("rm -rf ~") + .global_default(ToolPermissionMode::Allow) + .is_deny(); + t("rm -rf $HOME") + .global_default(ToolPermissionMode::Allow) + .is_deny(); + t("rm -rf .") + .global_default(ToolPermissionMode::Allow) + .is_deny(); + t("rm -rf ..") + .global_default(ToolPermissionMode::Allow) + .is_deny(); } #[test] @@ -1479,6 +1611,12 @@ mod tests { t("rm -rf .hidden_dir") .mode(ToolPermissionMode::Allow) .is_allow(); + t("rm -rfv ./build") + .mode(ToolPermissionMode::Allow) + .is_allow(); + t("rm --recursive --force ./build") + .mode(ToolPermissionMode::Allow) + .is_allow(); } #[test] @@ -1486,12 +1624,73 @@ mod tests { // Hardcoded rules should catch dangerous commands in chains t("ls && rm -rf /").is_deny(); t("echo hello; rm -rf ~").is_deny(); - t("cargo build && rm -rf /").global(true).is_deny(); + t("cargo build && rm -rf /") + .global_default(ToolPermissionMode::Allow) + .is_deny(); t("echo hello; rm -rf $HOME").is_deny(); t("echo hello; rm -rf .").is_deny(); t("echo hello; rm -rf ..").is_deny(); } + #[test] + fn hardcoded_blocks_rm_with_extra_flags() { + // Extra flags like -v, -i should not bypass the security rules + t("rm -rfv /").is_deny(); + t("rm -v -rf /").is_deny(); + t("rm -rfi /").is_deny(); + t("rm -rfv ~").is_deny(); + t("rm -rfv ~/").is_deny(); + t("rm -rfv $HOME").is_deny(); + t("rm -rfv .").is_deny(); + t("rm -rfv ./").is_deny(); + t("rm -rfv ..").is_deny(); + t("rm -rfv ../").is_deny(); + } + + #[test] + fn hardcoded_blocks_rm_with_long_flags() { + t("rm --recursive --force /").is_deny(); + t("rm --force --recursive /").is_deny(); + t("rm --recursive --force ~").is_deny(); + t("rm --recursive --force ~/").is_deny(); + t("rm --recursive --force $HOME").is_deny(); + t("rm --recursive --force .").is_deny(); + t("rm --recursive --force ..").is_deny(); + } + + #[test] + fn hardcoded_blocks_rm_with_glob_star() { + // rm -rf /* is equally catastrophic to rm -rf / + t("rm -rf /*").is_deny(); + t("rm -rf ~/*").is_deny(); + t("rm -rf $HOME/*").is_deny(); + t("rm -rf ${HOME}/*").is_deny(); + t("rm -rf ./*").is_deny(); + t("rm -rf ../*").is_deny(); + } + + #[test] + fn hardcoded_extra_flags_allow_safe_rm() { + // Extra flags on specific paths should NOT be blocked + t("rm -rfv ~/somedir") + .mode(ToolPermissionMode::Allow) + .is_allow(); + t("rm -rfv /tmp/test") + .mode(ToolPermissionMode::Allow) + .is_allow(); + t("rm --recursive --force ./build") + .mode(ToolPermissionMode::Allow) + .is_allow(); + } + + #[test] + fn hardcoded_does_not_block_words_containing_rm() { + // Words like "storm", "inform" contain "rm" but should not be blocked + t("storm -rf /").mode(ToolPermissionMode::Allow).is_allow(); + t("inform -rf /").mode(ToolPermissionMode::Allow).is_allow(); + t("gorm -rf ~").mode(ToolPermissionMode::Allow).is_allow(); + } + #[test] fn hardcoded_blocks_rm_with_trailing_flags() { // GNU rm accepts flags after operands by default @@ -1593,7 +1792,9 @@ mod tests { t("ls && rm -rf /tmp/../../").is_deny(); t("echo hello; rm -rf /./").is_deny(); // Traversal cannot be bypassed by global or allow patterns - t("rm -rf /tmp/../../").global(true).is_deny(); + t("rm -rf /tmp/../../") + .global_default(ToolPermissionMode::Allow) + .is_deny(); t("rm -rf /./").allow(&[".*"]).is_deny(); // Safe paths with traversal should still be allowed t("rm -rf /tmp/../tmp/foo") @@ -1654,9 +1855,13 @@ mod tests { t("sudo rm -rf /").is_deny(); t("sudo rm -rf --no-preserve-root /").is_deny(); // Traversal cannot be bypassed even with global allow or allow patterns - t("rm -rf /etc/../").global(true).is_deny(); + t("rm -rf /etc/../") + .global_default(ToolPermissionMode::Allow) + .is_deny(); t("rm -rf /etc/../").allow(&[".*"]).is_deny(); - t("rm -rf --no-preserve-root /").global(true).is_deny(); + t("rm -rf --no-preserve-root /") + .global_default(ToolPermissionMode::Allow) + .is_deny(); t("rm -rf --no-preserve-root /").allow(&[".*"]).is_deny(); } @@ -1809,12 +2014,10 @@ mod tests { fn decide_permission_for_path_no_dots_early_return() { // When the path has no `.` or `..`, normalize_path returns the same string, // so decide_permission_for_path returns the raw decision directly. - let settings = test_agent_settings( - ToolPermissions { - tools: Default::default(), - }, - false, - ); + let settings = test_agent_settings(ToolPermissions { + default: ToolPermissionMode::Confirm, + tools: Default::default(), + }); let decision = decide_permission_for_path(EditFileTool::NAME, "src/main.rs", &settings); assert_eq!(decision, ToolPermissionDecision::Confirm); } @@ -1826,14 +2029,17 @@ mod tests { tools.insert( Arc::from(EditFileTool::NAME), ToolRules { - default_mode: ToolPermissionMode::Allow, + default: Some(ToolPermissionMode::Allow), always_allow: vec![], always_deny: vec![deny_regex], always_confirm: vec![], invalid_patterns: vec![], }, ); - let settings = test_agent_settings(ToolPermissions { tools }, false); + let settings = test_agent_settings(ToolPermissions { + default: ToolPermissionMode::Confirm, + tools, + }); let decision = decide_permission_for_path(EditFileTool::NAME, "/tmp/../etc/passwd", &settings); @@ -1843,4 +2049,155 @@ mod tests { decision ); } + + #[test] + fn normalize_path_collapses_dot_segments() { + assert_eq!( + normalize_path("src/../.zed/settings.json"), + ".zed/settings.json" + ); + assert_eq!(normalize_path("a/b/../c"), "a/c"); + assert_eq!(normalize_path("a/./b/c"), "a/b/c"); + assert_eq!(normalize_path("a/b/./c/../d"), "a/b/d"); + assert_eq!(normalize_path(".zed/settings.json"), ".zed/settings.json"); + assert_eq!(normalize_path("a/b/c"), "a/b/c"); + } + + #[test] + fn normalize_path_handles_multiple_parent_dirs() { + assert_eq!(normalize_path("a/b/c/../../d"), "a/d"); + assert_eq!(normalize_path("a/b/c/../../../d"), "d"); + } + + fn path_perm( + tool: &str, + input: &str, + deny: &[&str], + allow: &[&str], + confirm: &[&str], + ) -> ToolPermissionDecision { + let mut tools = collections::HashMap::default(); + tools.insert( + Arc::from(tool), + ToolRules { + default: None, + always_allow: allow + .iter() + .map(|p| { + CompiledRegex::new(p, false) + .unwrap_or_else(|| panic!("invalid regex: {p:?}")) + }) + .collect(), + always_deny: deny + .iter() + .map(|p| { + CompiledRegex::new(p, false) + .unwrap_or_else(|| panic!("invalid regex: {p:?}")) + }) + .collect(), + always_confirm: confirm + .iter() + .map(|p| { + CompiledRegex::new(p, false) + .unwrap_or_else(|| panic!("invalid regex: {p:?}")) + }) + .collect(), + invalid_patterns: vec![], + }, + ); + let permissions = ToolPermissions { + default: ToolPermissionMode::Confirm, + tools, + }; + let raw_decision = ToolPermissionDecision::from_input( + tool, + &[input.to_string()], + &permissions, + ShellKind::Posix, + ); + + let simplified = normalize_path(input); + if simplified == input { + return raw_decision; + } + + let simplified_decision = + ToolPermissionDecision::from_input(tool, &[simplified], &permissions, ShellKind::Posix); + + most_restrictive(raw_decision, simplified_decision) + } + + #[test] + fn decide_permission_for_path_denies_traversal_to_denied_dir() { + let decision = path_perm( + "copy_path", + "src/../.zed/settings.json", + &["^\\.zed/"], + &[], + &[], + ); + assert!(matches!(decision, ToolPermissionDecision::Deny(_))); + } + + #[test] + fn decide_permission_for_path_confirms_traversal_to_confirmed_dir() { + let decision = path_perm( + "copy_path", + "src/../.zed/settings.json", + &[], + &[], + &["^\\.zed/"], + ); + assert!(matches!(decision, ToolPermissionDecision::Confirm)); + } + + #[test] + fn decide_permission_for_path_allows_when_no_traversal_issue() { + let decision = path_perm("copy_path", "src/main.rs", &[], &["^src/"], &[]); + assert!(matches!(decision, ToolPermissionDecision::Allow)); + } + + #[test] + fn decide_permission_for_path_most_restrictive_wins() { + let decision = path_perm( + "copy_path", + "allowed/../.zed/settings.json", + &["^\\.zed/"], + &["^allowed/"], + &[], + ); + assert!(matches!(decision, ToolPermissionDecision::Deny(_))); + } + + #[test] + fn decide_permission_for_path_dot_segment_only() { + let decision = path_perm( + "delete_path", + "./.zed/settings.json", + &["^\\.zed/"], + &[], + &[], + ); + assert!(matches!(decision, ToolPermissionDecision::Deny(_))); + } + + #[test] + fn decide_permission_for_path_no_change_when_already_simple() { + // When path has no `.` or `..` segments, behavior matches decide_permission_from_settings + let decision = path_perm("copy_path", ".zed/settings.json", &["^\\.zed/"], &[], &[]); + assert!(matches!(decision, ToolPermissionDecision::Deny(_))); + } + + #[test] + fn decide_permission_for_path_raw_deny_still_works() { + // Even without traversal, if the raw path itself matches deny, it's denied + let decision = path_perm("copy_path", "secret/file.txt", &["^secret/"], &[], &[]); + assert!(matches!(decision, ToolPermissionDecision::Deny(_))); + } + + #[test] + fn decide_permission_for_path_denies_edit_file_traversal_to_dotenv() { + let decision = path_perm(EditFileTool::NAME, "src/../.env", &["^\\.env"], &[], &[]); + assert!(matches!(decision, ToolPermissionDecision::Deny(_))); + } } diff --git a/crates/agent/src/tools/copy_path_tool.rs b/crates/agent/src/tools/copy_path_tool.rs index 0d8e5b3cfe960dac806c0d9b3b570961ff747d55..a8af9515fd28dcd121d04900cef1f800442ff53f 100644 --- a/crates/agent/src/tools/copy_path_tool.rs +++ b/crates/agent/src/tools/copy_path_tool.rs @@ -1,7 +1,7 @@ use super::edit_file_tool::{ SensitiveSettingsKind, is_sensitive_settings_path, sensitive_settings_kind, }; -use crate::{AgentTool, ToolCallEventStream, ToolPermissionDecision, decide_permission_for_path}; +use crate::{AgentTool, ToolCallEventStream, ToolPermissionDecision, decide_permission_for_paths}; use agent_client_protocol::ToolKind; use agent_settings::AgentSettings; use anyhow::{Context as _, Result, anyhow}; @@ -85,32 +85,23 @@ impl AgentTool for CopyPathTool { ) -> Task> { let settings = AgentSettings::get_global(cx); - let source_decision = decide_permission_for_path(Self::NAME, &input.source_path, settings); - if let ToolPermissionDecision::Deny(reason) = source_decision { + let paths = vec![input.source_path.clone(), input.destination_path.clone()]; + let decision = decide_permission_for_paths(Self::NAME, &paths, settings); + if let ToolPermissionDecision::Deny(reason) = decision { return Task::ready(Err(anyhow!("{}", reason))); } - let dest_decision = - decide_permission_for_path(Self::NAME, &input.destination_path, settings); - if let ToolPermissionDecision::Deny(reason) = dest_decision { - return Task::ready(Err(anyhow!("{}", reason))); - } - - let needs_confirmation = matches!(source_decision, ToolPermissionDecision::Confirm) - || matches!(dest_decision, ToolPermissionDecision::Confirm) - || (!settings.always_allow_tool_actions - && matches!(source_decision, ToolPermissionDecision::Allow) - && is_sensitive_settings_path(Path::new(&input.source_path))) - || (!settings.always_allow_tool_actions - && matches!(dest_decision, ToolPermissionDecision::Allow) - && is_sensitive_settings_path(Path::new(&input.destination_path))); + let needs_confirmation = matches!(decision, ToolPermissionDecision::Confirm) + || (matches!(decision, ToolPermissionDecision::Allow) + && (is_sensitive_settings_path(Path::new(&input.source_path)) + || is_sensitive_settings_path(Path::new(&input.destination_path)))); let authorize = if needs_confirmation { let src = MarkdownInlineCode(&input.source_path); let dest = MarkdownInlineCode(&input.destination_path); let context = crate::ToolPermissionContext { tool_name: Self::NAME.to_string(), - input_value: format!("{}\n{}", input.source_path, input.destination_path), + input_values: vec![input.source_path.clone(), input.destination_path.clone()], }; let title = format!("Copy {src} to {dest}"); let sensitive_kind = sensitive_settings_kind(Path::new(&input.source_path)) @@ -156,7 +147,7 @@ impl AgentTool for CopyPathTool { anyhow::bail!("Copy cancelled by user"); } }; - let _ = result.with_context(|| { + result.with_context(|| { format!( "Copying {} to {}", input.source_path, input.destination_path diff --git a/crates/agent/src/tools/create_directory_tool.rs b/crates/agent/src/tools/create_directory_tool.rs index 2683f6237c27c9dcc0bc599143cabfa3bdeb7dff..43c3829bdd835cc08ec2ad2314c94c562b149505 100644 --- a/crates/agent/src/tools/create_directory_tool.rs +++ b/crates/agent/src/tools/create_directory_tool.rs @@ -74,10 +74,7 @@ impl AgentTool for CreateDirectoryTool { let mut decision = decide_permission_for_path(Self::NAME, &input.path, settings); let sensitive_kind = sensitive_settings_kind(Path::new(&input.path)); - if matches!(decision, ToolPermissionDecision::Allow) - && !settings.always_allow_tool_actions - && sensitive_kind.is_some() - { + if matches!(decision, ToolPermissionDecision::Allow) && sensitive_kind.is_some() { decision = ToolPermissionDecision::Confirm; } @@ -95,7 +92,7 @@ impl AgentTool for CreateDirectoryTool { }; let context = crate::ToolPermissionContext { tool_name: Self::NAME.to_string(), - input_value: input.path.clone(), + input_values: vec![input.path.clone()], }; Some(event_stream.authorize(title, context, cx)) } diff --git a/crates/agent/src/tools/delete_path_tool.rs b/crates/agent/src/tools/delete_path_tool.rs index 8f0c721b289e495ccb7e12e331a12dfcf841a14a..41e060933fd4aa3ad57b719736b5ea9097663313 100644 --- a/crates/agent/src/tools/delete_path_tool.rs +++ b/crates/agent/src/tools/delete_path_tool.rs @@ -81,7 +81,6 @@ impl AgentTool for DeletePathTool { let mut decision = decide_permission_for_path(Self::NAME, &path, settings); if matches!(decision, ToolPermissionDecision::Allow) - && !settings.always_allow_tool_actions && is_sensitive_settings_path(Path::new(&path)) { decision = ToolPermissionDecision::Confirm; @@ -95,7 +94,7 @@ impl AgentTool for DeletePathTool { ToolPermissionDecision::Confirm => { let context = crate::ToolPermissionContext { tool_name: Self::NAME.to_string(), - input_value: path.clone(), + input_values: vec![path.clone()], }; let title = format!("Delete {}", MarkdownInlineCode(&path)); let title = match sensitive_settings_kind(Path::new(&path)) { diff --git a/crates/agent/src/tools/edit_file_tool.rs b/crates/agent/src/tools/edit_file_tool.rs index 9fdfd8c726bd2073cdf26db01da5d15b0768ba48..80df7b219fb265ea5f6172c9c21113dcfe95d689 100644 --- a/crates/agent/src/tools/edit_file_tool.rs +++ b/crates/agent/src/tools/edit_file_tool.rs @@ -254,9 +254,7 @@ pub fn authorize_file_edit( let explicitly_allowed = matches!(decision, ToolPermissionDecision::Allow); - if explicitly_allowed - && (settings.always_allow_tool_actions || !is_sensitive_settings_path(path)) - { + if explicitly_allowed && !is_sensitive_settings_path(path) { return Task::ready(Ok(())); } @@ -264,7 +262,7 @@ pub fn authorize_file_edit( Some(SensitiveSettingsKind::Local) => { let context = crate::ToolPermissionContext { tool_name: tool_name.to_string(), - input_value: path_str.to_string(), + input_values: vec![path_str.to_string()], }; return event_stream.authorize( format!("{} (local settings)", display_description), @@ -275,7 +273,7 @@ pub fn authorize_file_edit( Some(SensitiveSettingsKind::Global) => { let context = crate::ToolPermissionContext { tool_name: tool_name.to_string(), - input_value: path_str.to_string(), + input_values: vec![path_str.to_string()], }; return event_stream.authorize( format!("{} (settings)", display_description), @@ -297,7 +295,7 @@ pub fn authorize_file_edit( } else { let context = crate::ToolPermissionContext { tool_name: tool_name.to_string(), - input_value: path_str.to_string(), + input_values: vec![path_str.to_string()], }; event_stream.authorize(display_description, context, cx) } @@ -540,7 +538,8 @@ impl AgentTool for EditFileTool { } } - // If format_on_save is enabled, format the buffer + let edit_agent_output = output.await?; + let format_on_save_enabled = buffer.read_with(cx, |buffer, cx| { let settings = language_settings::language_settings( buffer.language().map(|l| l.name()), @@ -550,8 +549,6 @@ impl AgentTool for EditFileTool { settings.format_on_save != FormatOnSave::Off }); - let edit_agent_output = output.await?; - if format_on_save_enabled { action_log.update(cx, |log, cx| { log.buffer_edited(buffer.clone(), cx); @@ -1309,15 +1306,17 @@ mod tests { Some("test 4 (local settings)".into()) ); - // Test 5: When always_allow_tool_actions is enabled, no confirmation needed + // Test 5: When global default is allow, sensitive and outside-project + // paths still require confirmation cx.update(|cx| { let mut settings = agent_settings::AgentSettings::get_global(cx).clone(); - settings.always_allow_tool_actions = true; + settings.tool_permissions.default = settings::ToolPermissionMode::Allow; agent_settings::AgentSettings::override_global(settings, cx); }); + // 5.1: .zed/settings.json is a sensitive path — still prompts let (stream_tx, mut stream_rx) = ToolCallEventStream::test(); - cx.update(|cx| { + let _auth = cx.update(|cx| { tool.authorize( &EditFileToolInput { display_description: "test 5.1".into(), @@ -1327,17 +1326,37 @@ mod tests { &stream_tx, cx, ) + }); + let event = stream_rx.expect_authorization().await; + assert_eq!( + event.tool_call.fields.title, + Some("test 5.1 (local settings)".into()) + ); + + // 5.2: /etc/hosts is outside the project, but Allow auto-approves + let (stream_tx, mut stream_rx) = ToolCallEventStream::test(); + cx.update(|cx| { + tool.authorize( + &EditFileToolInput { + display_description: "test 5.2".into(), + path: "/etc/hosts".into(), + mode: EditFileMode::Edit, + }, + &stream_tx, + cx, + ) }) .await .unwrap(); assert!(stream_rx.try_next().is_err()); + // 5.3: Normal in-project path with allow — no confirmation needed let (stream_tx, mut stream_rx) = ToolCallEventStream::test(); cx.update(|cx| { tool.authorize( &EditFileToolInput { - display_description: "test 5.2".into(), - path: "/etc/hosts".into(), + display_description: "test 5.3".into(), + path: "root/src/main.rs".into(), mode: EditFileMode::Edit, }, &stream_tx, @@ -1347,6 +1366,29 @@ mod tests { .await .unwrap(); assert!(stream_rx.try_next().is_err()); + + // 5.4: With Confirm default, non-project paths still prompt + cx.update(|cx| { + let mut settings = agent_settings::AgentSettings::get_global(cx).clone(); + settings.tool_permissions.default = settings::ToolPermissionMode::Confirm; + agent_settings::AgentSettings::override_global(settings, cx); + }); + + let (stream_tx, mut stream_rx) = ToolCallEventStream::test(); + let _auth = cx.update(|cx| { + tool.authorize( + &EditFileToolInput { + display_description: "test 5.4".into(), + path: "/etc/hosts".into(), + mode: EditFileMode::Edit, + }, + &stream_tx, + cx, + ) + }); + + let event = stream_rx.expect_authorization().await; + assert_eq!(event.tool_call.fields.title, Some("test 5.4".into())); } #[gpui::test] diff --git a/crates/agent/src/tools/fetch_tool.rs b/crates/agent/src/tools/fetch_tool.rs index 65fd3a9e5311392b92dd2dc594eccae0768c6bd2..69263e3b765f75b2dfd1af94ff015afd36c4e279 100644 --- a/crates/agent/src/tools/fetch_tool.rs +++ b/crates/agent/src/tools/fetch_tool.rs @@ -146,7 +146,8 @@ impl AgentTool for FetchTool { cx: &mut App, ) -> Task> { let settings = AgentSettings::get_global(cx); - let decision = decide_permission_from_settings(Self::NAME, &input.url, settings); + let decision = + decide_permission_from_settings(Self::NAME, std::slice::from_ref(&input.url), settings); let authorize = match decision { ToolPermissionDecision::Allow => None, @@ -156,7 +157,7 @@ impl AgentTool for FetchTool { ToolPermissionDecision::Confirm => { let context = crate::ToolPermissionContext { tool_name: Self::NAME.to_string(), - input_value: input.url.clone(), + input_values: vec![input.url.clone()], }; Some(event_stream.authorize( format!("Fetch {}", MarkdownInlineCode(&input.url)), diff --git a/crates/agent/src/tools/move_path_tool.rs b/crates/agent/src/tools/move_path_tool.rs index 572f01c04e76269d37ebb4d0ee271fe534725560..25cbac9507eda1d23312ac220a43bd70f600e415 100644 --- a/crates/agent/src/tools/move_path_tool.rs +++ b/crates/agent/src/tools/move_path_tool.rs @@ -1,7 +1,7 @@ use super::edit_file_tool::{ SensitiveSettingsKind, is_sensitive_settings_path, sensitive_settings_kind, }; -use crate::{AgentTool, ToolCallEventStream, ToolPermissionDecision, decide_permission_for_path}; +use crate::{AgentTool, ToolCallEventStream, ToolPermissionDecision, decide_permission_for_paths}; use agent_client_protocol::ToolKind; use agent_settings::AgentSettings; use anyhow::{Context as _, Result, anyhow}; @@ -98,32 +98,23 @@ impl AgentTool for MovePathTool { ) -> Task> { let settings = AgentSettings::get_global(cx); - let source_decision = decide_permission_for_path(Self::NAME, &input.source_path, settings); - if let ToolPermissionDecision::Deny(reason) = source_decision { + let paths = vec![input.source_path.clone(), input.destination_path.clone()]; + let decision = decide_permission_for_paths(Self::NAME, &paths, settings); + if let ToolPermissionDecision::Deny(reason) = decision { return Task::ready(Err(anyhow!("{}", reason))); } - let dest_decision = - decide_permission_for_path(Self::NAME, &input.destination_path, settings); - if let ToolPermissionDecision::Deny(reason) = dest_decision { - return Task::ready(Err(anyhow!("{}", reason))); - } - - let needs_confirmation = matches!(source_decision, ToolPermissionDecision::Confirm) - || matches!(dest_decision, ToolPermissionDecision::Confirm) - || (!settings.always_allow_tool_actions - && matches!(source_decision, ToolPermissionDecision::Allow) - && is_sensitive_settings_path(Path::new(&input.source_path))) - || (!settings.always_allow_tool_actions - && matches!(dest_decision, ToolPermissionDecision::Allow) - && is_sensitive_settings_path(Path::new(&input.destination_path))); + let needs_confirmation = matches!(decision, ToolPermissionDecision::Confirm) + || (matches!(decision, ToolPermissionDecision::Allow) + && (is_sensitive_settings_path(Path::new(&input.source_path)) + || is_sensitive_settings_path(Path::new(&input.destination_path)))); let authorize = if needs_confirmation { let src = MarkdownInlineCode(&input.source_path); let dest = MarkdownInlineCode(&input.destination_path); let context = crate::ToolPermissionContext { tool_name: Self::NAME.to_string(), - input_value: format!("{}\n{}", input.source_path, input.destination_path), + input_values: vec![input.source_path.clone(), input.destination_path.clone()], }; let title = format!("Move {src} to {dest}"); let settings_kind = sensitive_settings_kind(Path::new(&input.source_path)) @@ -169,7 +160,7 @@ impl AgentTool for MovePathTool { anyhow::bail!("Move cancelled by user"); } }; - let _ = result.with_context(|| { + result.with_context(|| { format!("Moving {} to {}", input.source_path, input.destination_path) })?; Ok(format!( diff --git a/crates/agent/src/tools/open_tool.rs b/crates/agent/src/tools/open_tool.rs index dac797769fa692a32c659f67cefdd92655b1a567..f401e56908d4f2a4702d71f6baeb766bcef1b60b 100644 --- a/crates/agent/src/tools/open_tool.rs +++ b/crates/agent/src/tools/open_tool.rs @@ -66,7 +66,7 @@ impl AgentTool for OpenTool { let abs_path = to_absolute_path(&input.path_or_url, self.project.clone(), cx); let context = crate::ToolPermissionContext { tool_name: Self::NAME.to_string(), - input_value: input.path_or_url.clone(), + input_values: vec![input.path_or_url.clone()], }; let authorize = event_stream.authorize(self.initial_title(Ok(input.clone()), cx), context, cx); diff --git a/crates/agent/src/tools/restore_file_from_disk_tool.rs b/crates/agent/src/tools/restore_file_from_disk_tool.rs index bf329a02bd0c0e8a0dcadf61e9b3b269d7906c76..e16930046316a2aa9187587afc67493f1bd6908c 100644 --- a/crates/agent/src/tools/restore_file_from_disk_tool.rs +++ b/crates/agent/src/tools/restore_file_from_disk_tool.rs @@ -1,4 +1,8 @@ +use super::edit_file_tool::{ + SensitiveSettingsKind, is_sensitive_settings_path, sensitive_settings_kind, +}; use agent_client_protocol as acp; +use agent_settings::AgentSettings; use anyhow::Result; use collections::FxHashSet; use futures::FutureExt as _; @@ -7,10 +11,12 @@ use language::Buffer; use project::Project; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use std::path::PathBuf; +use settings::Settings; +use std::path::{Path, PathBuf}; use std::sync::Arc; +use util::markdown::MarkdownInlineCode; -use crate::{AgentTool, ToolCallEventStream}; +use crate::{AgentTool, ToolCallEventStream, ToolPermissionDecision, decide_permission_for_path}; /// Discards unsaved changes in open buffers by reloading file contents from disk. /// @@ -63,10 +69,73 @@ impl AgentTool for RestoreFileFromDiskTool { event_stream: ToolCallEventStream, cx: &mut App, ) -> Task> { + let settings = AgentSettings::get_global(cx); + let mut confirmation_paths: Vec = Vec::new(); + + for path in &input.paths { + let path_str = path.to_string_lossy(); + let decision = decide_permission_for_path(Self::NAME, &path_str, settings); + match decision { + ToolPermissionDecision::Allow => { + if is_sensitive_settings_path(Path::new(&*path_str)) { + confirmation_paths.push(path_str.to_string()); + } + } + ToolPermissionDecision::Deny(reason) => { + return Task::ready(Err(anyhow::anyhow!("{}", reason))); + } + ToolPermissionDecision::Confirm => { + confirmation_paths.push(path_str.to_string()); + } + } + } + + let authorize = if !confirmation_paths.is_empty() { + let title = if confirmation_paths.len() == 1 { + format!( + "Restore {} from disk", + MarkdownInlineCode(&confirmation_paths[0]) + ) + } else { + let paths: Vec<_> = confirmation_paths + .iter() + .take(3) + .map(|p| p.as_str()) + .collect(); + if confirmation_paths.len() > 3 { + format!( + "Restore {}, and {} more from disk", + paths.join(", "), + confirmation_paths.len() - 3 + ) + } else { + format!("Restore {} from disk", paths.join(", ")) + } + }; + let sensitive_kind = confirmation_paths + .iter() + .find_map(|p| sensitive_settings_kind(Path::new(p))); + let title = match sensitive_kind { + Some(SensitiveSettingsKind::Local) => format!("{title} (local settings)"), + Some(SensitiveSettingsKind::Global) => format!("{title} (settings)"), + None => title, + }; + let context = crate::ToolPermissionContext { + tool_name: Self::NAME.to_string(), + input_values: confirmation_paths, + }; + Some(event_stream.authorize(title, context, cx)) + } else { + None + }; + let project = self.project.clone(); let input_paths = input.paths; cx.spawn(async move |cx| { + if let Some(authorize) = authorize { + authorize.await?; + } let mut buffers_to_reload: FxHashSet> = FxHashSet::default(); let mut restored_paths: Vec = Vec::new(); @@ -190,6 +259,11 @@ mod tests { let settings_store = SettingsStore::test(cx); cx.set_global(settings_store); }); + cx.update(|cx| { + let mut settings = AgentSettings::get_global(cx).clone(); + settings.tool_permissions.default = settings::ToolPermissionMode::Allow; + AgentSettings::override_global(settings, cx); + }); } #[gpui::test] diff --git a/crates/agent/src/tools/save_file_tool.rs b/crates/agent/src/tools/save_file_tool.rs index 70994c55db115264dbff9b2b0909bf6ed07cdf18..842afa609bbfd946e4fb256d7ae11d4b6c383212 100644 --- a/crates/agent/src/tools/save_file_tool.rs +++ b/crates/agent/src/tools/save_file_tool.rs @@ -9,7 +9,7 @@ use project::Project; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use settings::Settings; -use std::path::{Component, Path, PathBuf}; +use std::path::{Path, PathBuf}; use std::sync::Arc; use util::markdown::MarkdownInlineCode; @@ -18,24 +18,6 @@ use super::edit_file_tool::{ }; use crate::{AgentTool, ToolCallEventStream, ToolPermissionDecision, decide_permission_for_path}; -fn common_parent_for_paths(paths: &[String]) -> Option { - let first = paths.first()?; - let mut common: Vec> = Path::new(first).parent()?.components().collect(); - for path in &paths[1..] { - let parent: Vec> = Path::new(path).parent()?.components().collect(); - let prefix_len = common - .iter() - .zip(parent.iter()) - .take_while(|(a, b)| a == b) - .count(); - common.truncate(prefix_len); - } - if common.is_empty() { - return None; - } - Some(common.iter().collect()) -} - /// Saves files that have unsaved changes. /// /// Use this tool when you need to edit files but they have unsaved changes that must be saved first. @@ -92,9 +74,7 @@ impl AgentTool for SaveFileTool { let decision = decide_permission_for_path(Self::NAME, &path_str, settings); match decision { ToolPermissionDecision::Allow => { - if !settings.always_allow_tool_actions - && is_sensitive_settings_path(Path::new(&*path_str)) - { + if is_sensitive_settings_path(Path::new(&*path_str)) { confirmation_paths.push(path_str.to_string()); } } @@ -134,16 +114,9 @@ impl AgentTool for SaveFileTool { Some(SensitiveSettingsKind::Global) => format!("{title} (settings)"), None => title, }; - let input_value = if confirmation_paths.len() == 1 { - confirmation_paths[0].clone() - } else { - common_parent_for_paths(&confirmation_paths) - .map(|parent| format!("{}/_", parent.display())) - .unwrap_or_else(|| confirmation_paths[0].clone()) - }; let context = crate::ToolPermissionContext { tool_name: Self::NAME.to_string(), - input_value, + input_values: confirmation_paths.clone(), }; Some(event_stream.authorize(title, context, cx)) } else { @@ -160,11 +133,10 @@ impl AgentTool for SaveFileTool { let mut buffers_to_save: FxHashSet> = FxHashSet::default(); - let mut saved_paths: Vec = Vec::new(); + let mut dirty_count: usize = 0; let mut clean_paths: Vec = Vec::new(); let mut not_found_paths: Vec = Vec::new(); let mut open_errors: Vec<(PathBuf, String)> = Vec::new(); - let dirty_check_errors: Vec<(PathBuf, String)> = Vec::new(); let mut save_errors: Vec<(String, String)> = Vec::new(); for path in input_paths { @@ -197,7 +169,7 @@ impl AgentTool for SaveFileTool { if is_dirty { buffers_to_save.insert(buffer); - saved_paths.push(path); + dirty_count += 1; } else { clean_paths.push(path); } @@ -229,8 +201,9 @@ impl AgentTool for SaveFileTool { let mut lines: Vec = Vec::new(); - if !saved_paths.is_empty() { - lines.push(format!("Saved {} file(s).", saved_paths.len())); + let successful_saves = dirty_count.saturating_sub(save_errors.len()); + if successful_saves > 0 { + lines.push(format!("Saved {} file(s).", successful_saves)); } if !clean_paths.is_empty() { lines.push(format!("{} clean.", clean_paths.len())); @@ -248,15 +221,6 @@ impl AgentTool for SaveFileTool { lines.push(format!("- {}: {}", path.display(), error)); } } - if !dirty_check_errors.is_empty() { - lines.push(format!( - "Dirty check failed ({}):", - dirty_check_errors.len() - )); - for (path, error) in &dirty_check_errors { - lines.push(format!("- {}: {}", path.display(), error)); - } - } if !save_errors.is_empty() { lines.push(format!("Save failed ({}):", save_errors.len())); for (path, error) in &save_errors { @@ -290,7 +254,7 @@ mod tests { }); cx.update(|cx| { let mut settings = AgentSettings::get_global(cx).clone(); - settings.always_allow_tool_actions = true; + settings.tool_permissions.default = settings::ToolPermissionMode::Allow; AgentSettings::override_global(settings, cx); }); } diff --git a/crates/agent/src/tools/streaming_edit_file_tool.rs b/crates/agent/src/tools/streaming_edit_file_tool.rs index 717761038a37f2b2089b5547d5daab411fb38a5e..c2c577b58250413de0044a40f9de11c9c9623d96 100644 --- a/crates/agent/src/tools/streaming_edit_file_tool.rs +++ b/crates/agent/src/tools/streaming_edit_file_tool.rs @@ -165,6 +165,15 @@ impl StreamingEditFileTool { } } + pub fn with_thread(&self, new_thread: WeakEntity) -> Self { + Self { + project: self.project.clone(), + thread: new_thread, + language_registry: self.language_registry.clone(), + templates: self.templates.clone(), + } + } + fn authorize( &self, input: &StreamingEditFileToolInput, diff --git a/crates/agent/src/tools/terminal_tool.rs b/crates/agent/src/tools/terminal_tool.rs index 37537642bc047e1a1dbdb4ee8600ef576feea5d9..252545eaed10287119845142acfa0bc80165121f 100644 --- a/crates/agent/src/tools/terminal_tool.rs +++ b/crates/agent/src/tools/terminal_tool.rs @@ -95,7 +95,11 @@ impl AgentTool for TerminalTool { }; let settings = AgentSettings::get_global(cx); - let decision = decide_permission_from_settings(Self::NAME, &input.command, settings); + let decision = decide_permission_from_settings( + Self::NAME, + std::slice::from_ref(&input.command), + settings, + ); let authorize = match decision { ToolPermissionDecision::Allow => None, @@ -105,7 +109,7 @@ impl AgentTool for TerminalTool { ToolPermissionDecision::Confirm => { let context = crate::ToolPermissionContext { tool_name: Self::NAME.to_string(), - input_value: input.command.clone(), + input_values: vec![input.command.clone()], }; Some(event_stream.authorize(self.initial_title(Ok(input.clone()), cx), context, cx)) } diff --git a/crates/agent/src/tools/web_search_tool.rs b/crates/agent/src/tools/web_search_tool.rs index 65a51b6cb3befb0c47f32e769cd3fd0332f50589..c01486c1816137fa0ccc53478a60825a08d469cb 100644 --- a/crates/agent/src/tools/web_search_tool.rs +++ b/crates/agent/src/tools/web_search_tool.rs @@ -72,7 +72,11 @@ impl AgentTool for WebSearchTool { cx: &mut App, ) -> Task> { let settings = AgentSettings::get_global(cx); - let decision = decide_permission_from_settings(Self::NAME, &input.query, settings); + let decision = decide_permission_from_settings( + Self::NAME, + std::slice::from_ref(&input.query), + settings, + ); let authorize = match decision { ToolPermissionDecision::Allow => None, @@ -82,7 +86,7 @@ impl AgentTool for WebSearchTool { ToolPermissionDecision::Confirm => { let context = crate::ToolPermissionContext { tool_name: Self::NAME.to_string(), - input_value: input.query.clone(), + input_values: vec![input.query.clone()], }; Some(event_stream.authorize( format!("Search the web for {}", MarkdownInlineCode(&input.query)), diff --git a/crates/agent_servers/src/acp.rs b/crates/agent_servers/src/acp.rs index 873a80f84d26d7cfd6f60defdf8be387b64db0c3..7617646d038e9bb7da07de07f62f560b44ec7571 100644 --- a/crates/agent_servers/src/acp.rs +++ b/crates/agent_servers/src/acp.rs @@ -1147,14 +1147,12 @@ impl acp::Client for ClientDelegate { &self, arguments: acp::RequestPermissionRequest, ) -> Result { - let respect_always_allow_setting; let thread; { let sessions_ref = self.sessions.borrow(); let session = sessions_ref .get(&arguments.session_id) .context("Failed to get session")?; - respect_always_allow_setting = session.session_modes.is_none(); thread = session.thread.clone(); } @@ -1164,7 +1162,6 @@ impl acp::Client for ClientDelegate { thread.request_tool_call_authorization( arguments.tool_call, acp_thread::PermissionOptions::Flat(arguments.options), - respect_always_allow_setting, cx, ) })??; diff --git a/crates/agent_settings/src/agent_settings.rs b/crates/agent_settings/src/agent_settings.rs index f60452078ebca6675ddc77b487e6448c6e0e65e2..fffb55c34fa1c38a4366052ebc0383e7e3d5a2ea 100644 --- a/crates/agent_settings/src/agent_settings.rs +++ b/crates/agent_settings/src/agent_settings.rs @@ -1,6 +1,7 @@ mod agent_profile; -use std::sync::Arc; +use std::path::{Component, Path}; +use std::sync::{Arc, LazyLock}; use agent_client_protocol::ModelId; use collections::{HashSet, IndexMap}; @@ -38,7 +39,7 @@ pub struct AgentSettings { pub default_profile: AgentProfileId, pub default_view: DefaultAgentView, pub profiles: IndexMap, - pub always_allow_tool_actions: bool, + pub notify_when_agent_waiting: NotifyWhenAgentWaiting, pub play_sound_when_agent_done: bool, pub single_file_review: bool, @@ -111,6 +112,8 @@ impl Default for AgentProfileId { #[derive(Clone, Debug, Default)] pub struct ToolPermissions { + /// Global default permission when no tool-specific rules or patterns match. + pub default: ToolPermissionMode, pub tools: collections::HashMap, ToolRules>, } @@ -142,9 +145,9 @@ pub struct InvalidRegexPattern { pub error: String, } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Default)] pub struct ToolRules { - pub default_mode: ToolPermissionMode, + pub default: Option, pub always_allow: Vec, pub always_deny: Vec, pub always_confirm: Vec, @@ -152,18 +155,6 @@ pub struct ToolRules { pub invalid_patterns: Vec, } -impl Default for ToolRules { - fn default() -> Self { - Self { - default_mode: ToolPermissionMode::Confirm, - always_allow: Vec::new(), - always_deny: Vec::new(), - always_confirm: Vec::new(), - invalid_patterns: Vec::new(), - } - } -} - #[derive(Clone)] pub struct CompiledRegex { pub pattern: String, @@ -201,6 +192,214 @@ impl CompiledRegex { } } +pub const HARDCODED_SECURITY_DENIAL_MESSAGE: &str = "Blocked by built-in security rule. This operation is considered too \ + harmful to be allowed, and cannot be overridden by settings."; + +/// Security rules that are always enforced and cannot be overridden by any setting. +/// These protect against catastrophic operations like wiping filesystems. +pub struct HardcodedSecurityRules { + pub terminal_deny: Vec, +} + +pub static HARDCODED_SECURITY_RULES: LazyLock = LazyLock::new(|| { + const FLAGS: &str = r"(--[a-zA-Z0-9][-a-zA-Z0-9_]*(=[^\s]*)?\s+|-[a-zA-Z]+\s+)*"; + const TRAILING_FLAGS: &str = r"(\s+--[a-zA-Z0-9][-a-zA-Z0-9_]*(=[^\s]*)?|\s+-[a-zA-Z]+)*\s*"; + + HardcodedSecurityRules { + terminal_deny: vec![ + // Recursive deletion of root - "rm -rf /", "rm -rf /*" + CompiledRegex::new( + &format!(r"\brm\s+{FLAGS}(--\s+)?/\*?{TRAILING_FLAGS}$"), + false, + ) + .expect("hardcoded regex should compile"), + // Recursive deletion of home via tilde - "rm -rf ~", "rm -rf ~/" + CompiledRegex::new( + &format!(r"\brm\s+{FLAGS}(--\s+)?~/?\*?{TRAILING_FLAGS}$"), + false, + ) + .expect("hardcoded regex should compile"), + // Recursive deletion of home via env var - "rm -rf $HOME", "rm -rf ${HOME}" + CompiledRegex::new( + &format!(r"\brm\s+{FLAGS}(--\s+)?(\$HOME|\$\{{HOME\}})/?(\*)?{TRAILING_FLAGS}$"), + false, + ) + .expect("hardcoded regex should compile"), + // Recursive deletion of current directory - "rm -rf .", "rm -rf ./" + CompiledRegex::new( + &format!(r"\brm\s+{FLAGS}(--\s+)?\./?\*?{TRAILING_FLAGS}$"), + false, + ) + .expect("hardcoded regex should compile"), + // Recursive deletion of parent directory - "rm -rf ..", "rm -rf ../" + CompiledRegex::new( + &format!(r"\brm\s+{FLAGS}(--\s+)?\.\./?\*?{TRAILING_FLAGS}$"), + false, + ) + .expect("hardcoded regex should compile"), + ], + } +}); + +/// Checks if input matches any hardcoded security rules that cannot be bypassed. +/// Returns the denial reason string if blocked, None otherwise. +/// +/// `terminal_tool_name` should be the tool name used for the terminal tool +/// (e.g. `"terminal"`). `extracted_commands` can optionally provide parsed +/// sub-commands for chained command checking; callers with access to a shell +/// parser should extract sub-commands and pass them here. +pub fn check_hardcoded_security_rules( + tool_name: &str, + terminal_tool_name: &str, + input: &str, + extracted_commands: Option<&[String]>, +) -> Option { + if tool_name != terminal_tool_name { + return None; + } + + let rules = &*HARDCODED_SECURITY_RULES; + let terminal_patterns = &rules.terminal_deny; + + if matches_hardcoded_patterns(input, terminal_patterns) { + return Some(HARDCODED_SECURITY_DENIAL_MESSAGE.into()); + } + + if let Some(commands) = extracted_commands { + for command in commands { + if matches_hardcoded_patterns(command, terminal_patterns) { + return Some(HARDCODED_SECURITY_DENIAL_MESSAGE.into()); + } + } + } + + None +} + +fn matches_hardcoded_patterns(command: &str, patterns: &[CompiledRegex]) -> bool { + for pattern in patterns { + if pattern.is_match(command) { + return true; + } + } + + for expanded in expand_rm_to_single_path_commands(command) { + for pattern in patterns { + if pattern.is_match(&expanded) { + return true; + } + } + } + + false +} + +fn expand_rm_to_single_path_commands(command: &str) -> Vec { + let trimmed = command.trim(); + + let first_token = trimmed.split_whitespace().next(); + if !first_token.is_some_and(|t| t.eq_ignore_ascii_case("rm")) { + return vec![]; + } + + let parts: Vec<&str> = trimmed.split_whitespace().collect(); + let mut flags = Vec::new(); + let mut paths = Vec::new(); + let mut past_double_dash = false; + + for part in parts.iter().skip(1) { + if !past_double_dash && *part == "--" { + past_double_dash = true; + flags.push(*part); + continue; + } + if !past_double_dash && part.starts_with('-') { + flags.push(*part); + } else { + paths.push(*part); + } + } + + let flags_str = if flags.is_empty() { + String::new() + } else { + format!("{} ", flags.join(" ")) + }; + + let mut results = Vec::new(); + for path in &paths { + if path.starts_with('$') { + let home_prefix = if path.starts_with("${HOME}") { + Some("${HOME}") + } else if path.starts_with("$HOME") { + Some("$HOME") + } else { + None + }; + + if let Some(prefix) = home_prefix { + let suffix = &path[prefix.len()..]; + if suffix.is_empty() { + results.push(format!("rm {flags_str}{path}")); + } else if suffix.starts_with('/') { + let normalized_suffix = normalize_path(suffix); + let reconstructed = if normalized_suffix == "/" { + prefix.to_string() + } else { + format!("{prefix}{normalized_suffix}") + }; + results.push(format!("rm {flags_str}{reconstructed}")); + } else { + results.push(format!("rm {flags_str}{path}")); + } + } else { + results.push(format!("rm {flags_str}{path}")); + } + continue; + } + + let mut normalized = normalize_path(path); + if normalized.is_empty() && !Path::new(path).has_root() { + normalized = ".".to_string(); + } + + results.push(format!("rm {flags_str}{normalized}")); + } + + results +} + +pub fn normalize_path(raw: &str) -> String { + let is_absolute = Path::new(raw).has_root(); + let mut components: Vec<&str> = Vec::new(); + for component in Path::new(raw).components() { + match component { + Component::CurDir => {} + Component::ParentDir => { + if components.last() == Some(&"..") { + components.push(".."); + } else if !components.is_empty() { + components.pop(); + } else if !is_absolute { + components.push(".."); + } + } + Component::Normal(segment) => { + if let Some(s) = segment.to_str() { + components.push(s); + } + } + Component::RootDir | Component::Prefix(_) => {} + } + } + let joined = components.join("/"); + if is_absolute { + format!("/{joined}") + } else { + joined + } +} + impl Settings for AgentSettings { fn from_settings(content: &settings::SettingsContent) -> Self { let agent = content.agent.clone().unwrap(); @@ -228,7 +427,7 @@ impl Settings for AgentSettings { .into_iter() .map(|(key, val)| (AgentProfileId(key), val.into())) .collect(), - always_allow_tool_actions: agent.always_allow_tool_actions.unwrap(), + notify_when_agent_waiting: agent.notify_when_agent_waiting.unwrap(), play_sound_when_agent_done: agent.play_sound_when_agent_done.unwrap(), single_file_review: agent.single_file_review.unwrap(), @@ -290,7 +489,8 @@ fn compile_tool_permissions(content: Option) - } let rules = ToolRules { - default_mode: rules_content.default_mode.unwrap_or_default(), + // Preserve tool-specific default; None means fall back to global default at decision time + default: rules_content.default, always_allow, always_deny, always_confirm, @@ -300,7 +500,10 @@ fn compile_tool_permissions(content: Option) - }) .collect(); - ToolPermissions { tools } + ToolPermissions { + default: content.default.unwrap_or_default(), + tools, + } } fn compile_regex_rules( @@ -311,6 +514,14 @@ fn compile_regex_rules( let mut errors = Vec::new(); for rule in rules { + if rule.pattern.is_empty() { + errors.push(InvalidRegexPattern { + pattern: rule.pattern, + rule_type: rule_type.to_string(), + error: "empty regex patterns are not allowed".to_string(), + }); + continue; + } let case_sensitive = rule.case_sensitive.unwrap_or(false); match CompiledRegex::try_new(&rule.pattern, case_sensitive) { Ok(regex) => compiled.push(regex), @@ -331,6 +542,7 @@ fn compile_regex_rules( mod tests { use super::*; use serde_json::json; + use settings::ToolPermissionMode; use settings::ToolPermissionsContent; #[test] @@ -359,7 +571,7 @@ mod tests { let json = json!({ "tools": { "terminal": { - "default_mode": "allow", + "default": "allow", "always_deny": [ { "pattern": "rm\\s+-rf" } ], @@ -374,7 +586,7 @@ mod tests { let permissions = compile_tool_permissions(Some(content)); let terminal_rules = permissions.tools.get("terminal").unwrap(); - assert_eq!(terminal_rules.default_mode, ToolPermissionMode::Allow); + assert_eq!(terminal_rules.default, Some(ToolPermissionMode::Allow)); assert_eq!(terminal_rules.always_deny.len(), 1); assert_eq!(terminal_rules.always_allow.len(), 1); assert!(terminal_rules.always_deny[0].is_match("rm -rf /")); @@ -382,11 +594,11 @@ mod tests { } #[test] - fn test_tool_rules_default_mode() { + fn test_tool_rules_default() { let json = json!({ "tools": { "edit_file": { - "default_mode": "deny" + "default": "deny" } } }); @@ -395,19 +607,20 @@ mod tests { let permissions = compile_tool_permissions(Some(content)); let rules = permissions.tools.get("edit_file").unwrap(); - assert_eq!(rules.default_mode, ToolPermissionMode::Deny); + assert_eq!(rules.default, Some(ToolPermissionMode::Deny)); } #[test] fn test_tool_permissions_empty() { let permissions = compile_tool_permissions(None); assert!(permissions.tools.is_empty()); + assert_eq!(permissions.default, ToolPermissionMode::Confirm); } #[test] fn test_tool_rules_default_returns_confirm() { let default_rules = ToolRules::default(); - assert_eq!(default_rules.default_mode, ToolPermissionMode::Confirm); + assert_eq!(default_rules.default, None); assert!(default_rules.always_allow.is_empty()); assert!(default_rules.always_deny.is_empty()); assert!(default_rules.always_confirm.is_empty()); @@ -418,15 +631,15 @@ mod tests { let json = json!({ "tools": { "terminal": { - "default_mode": "allow", + "default": "allow", "always_deny": [{ "pattern": "rm\\s+-rf" }] }, "edit_file": { - "default_mode": "confirm", + "default": "confirm", "always_deny": [{ "pattern": "\\.env$" }] }, "delete_path": { - "default_mode": "deny" + "default": "deny" } } }); @@ -437,15 +650,15 @@ mod tests { assert_eq!(permissions.tools.len(), 3); let terminal = permissions.tools.get("terminal").unwrap(); - assert_eq!(terminal.default_mode, ToolPermissionMode::Allow); + assert_eq!(terminal.default, Some(ToolPermissionMode::Allow)); assert_eq!(terminal.always_deny.len(), 1); let edit_file = permissions.tools.get("edit_file").unwrap(); - assert_eq!(edit_file.default_mode, ToolPermissionMode::Confirm); + assert_eq!(edit_file.default, Some(ToolPermissionMode::Confirm)); assert!(edit_file.always_deny[0].is_match("secrets.env")); let delete_path = permissions.tools.get("delete_path").unwrap(); - assert_eq!(delete_path.default_mode, ToolPermissionMode::Deny); + assert_eq!(delete_path.default, Some(ToolPermissionMode::Deny)); } #[test] @@ -526,7 +739,7 @@ mod tests { let json = json!({ "tools": { "terminal": { - "default_mode": "allow", + "default": "allow", "always_deny": [{ "pattern": "dangerous" }], "always_confirm": [{ "pattern": "dangerous" }], "always_allow": [{ "pattern": "dangerous" }] @@ -557,7 +770,7 @@ mod tests { let json = json!({ "tools": { "terminal": { - "default_mode": "allow", + "default": "allow", "always_confirm": [{ "pattern": "risky" }], "always_allow": [{ "pattern": "risky" }] } @@ -661,7 +874,7 @@ mod tests { let json = json!({ "tools": { "terminal": { - "default_mode": "allow" + "default": "allow" } } }); @@ -722,11 +935,86 @@ mod tests { "Pattern ^echo\\s should NOT match 'echoHello' (requires whitespace)" ); - // Verify default_mode is Confirm (the default) assert_eq!( - terminal.default_mode, - settings::ToolPermissionMode::Confirm, - "default_mode should be Confirm when not specified" + terminal.default, None, + "default should be None when not specified" ); } + + #[test] + fn test_empty_regex_pattern_is_invalid() { + let json = json!({ + "tools": { + "terminal": { + "always_allow": [ + { "pattern": "" } + ], + "always_deny": [ + { "case_sensitive": true } + ], + "always_confirm": [ + { "pattern": "" }, + { "pattern": "valid_pattern" } + ] + } + } + }); + + let content: ToolPermissionsContent = serde_json::from_value(json).unwrap(); + let permissions = compile_tool_permissions(Some(content)); + + let terminal = permissions.tools.get("terminal").unwrap(); + + assert_eq!(terminal.always_allow.len(), 0); + assert_eq!(terminal.always_deny.len(), 0); + assert_eq!(terminal.always_confirm.len(), 1); + assert!(terminal.always_confirm[0].is_match("valid_pattern")); + + assert_eq!(terminal.invalid_patterns.len(), 3); + for invalid in &terminal.invalid_patterns { + assert_eq!(invalid.pattern, ""); + assert!(invalid.error.contains("empty")); + } + } + + #[test] + fn test_default_json_tool_permissions_parse() { + let default_json = include_str!("../../../assets/settings/default.json"); + let value: serde_json_lenient::Value = serde_json_lenient::from_str(default_json).unwrap(); + let agent = value + .get("agent") + .expect("default.json should have 'agent' key"); + let tool_permissions_value = agent + .get("tool_permissions") + .expect("agent should have 'tool_permissions' key"); + + let content: ToolPermissionsContent = + serde_json_lenient::from_value(tool_permissions_value.clone()).unwrap(); + let permissions = compile_tool_permissions(Some(content)); + + assert_eq!(permissions.default, ToolPermissionMode::Confirm); + + assert!( + permissions.tools.is_empty(), + "default.json should not have any active tool-specific rules, found: {:?}", + permissions.tools.keys().collect::>() + ); + } + + #[test] + fn test_tool_permissions_explicit_global_default() { + let json_allow = json!({ + "default": "allow" + }); + let content: ToolPermissionsContent = serde_json::from_value(json_allow).unwrap(); + let permissions = compile_tool_permissions(Some(content)); + assert_eq!(permissions.default, ToolPermissionMode::Allow); + + let json_deny = json!({ + "default": "deny" + }); + let content: ToolPermissionsContent = serde_json::from_value(json_deny).unwrap(); + let permissions = compile_tool_permissions(Some(content)); + assert_eq!(permissions.default, ToolPermissionMode::Deny); + } } diff --git a/crates/agent_ui/src/acp/thread_view.rs b/crates/agent_ui/src/acp/thread_view.rs index b5b93ab63875766adfa1f855012dfc2a01564233..e2933ac46cb5aa1d23dfea9bd3e242f39056f75e 100644 --- a/crates/agent_ui/src/acp/thread_view.rs +++ b/crates/agent_ui/src/acp/thread_view.rs @@ -4576,9 +4576,11 @@ pub(crate) mod tests { let tool_call = acp::ToolCall::new(tool_call_id.clone(), "Run `cargo build --release`") .kind(acp::ToolKind::Edit); - let permission_options = - ToolPermissionContext::new(TerminalTool::NAME, "cargo build --release") - .build_permission_options(); + let permission_options = ToolPermissionContext::new( + TerminalTool::NAME, + vec!["cargo build --release".to_string()], + ) + .build_permission_options(); let connection = StubAgentConnection::new().with_permission_requests(HashMap::from_iter([( @@ -4684,8 +4686,9 @@ pub(crate) mod tests { let tool_call = acp::ToolCall::new(tool_call_id.clone(), "Edit `src/main.rs`") .kind(acp::ToolKind::Edit); - let permission_options = ToolPermissionContext::new(EditFileTool::NAME, "src/main.rs") - .build_permission_options(); + let permission_options = + ToolPermissionContext::new(EditFileTool::NAME, vec!["src/main.rs".to_string()]) + .build_permission_options(); let connection = StubAgentConnection::new().with_permission_requests(HashMap::from_iter([( @@ -4772,7 +4775,7 @@ pub(crate) mod tests { .kind(acp::ToolKind::Fetch); let permission_options = - ToolPermissionContext::new(FetchTool::NAME, "https://docs.rs/gpui") + ToolPermissionContext::new(FetchTool::NAME, vec!["https://docs.rs/gpui".to_string()]) .build_permission_options(); let connection = @@ -4860,9 +4863,11 @@ pub(crate) mod tests { .kind(acp::ToolKind::Edit); // No pattern button since ./deploy.sh doesn't match the alphanumeric pattern - let permission_options = - ToolPermissionContext::new(TerminalTool::NAME, "./deploy.sh --production") - .build_permission_options(); + let permission_options = ToolPermissionContext::new( + TerminalTool::NAME, + vec!["./deploy.sh --production".to_string()], + ) + .build_permission_options(); let connection = StubAgentConnection::new().with_permission_requests(HashMap::from_iter([( @@ -4960,7 +4965,8 @@ pub(crate) mod tests { acp::ToolCall::new(tool_call_id.clone(), "Run `cargo test`").kind(acp::ToolKind::Edit); let permission_options = - ToolPermissionContext::new(TerminalTool::NAME, "cargo test").build_permission_options(); + ToolPermissionContext::new(TerminalTool::NAME, vec!["cargo test".to_string()]) + .build_permission_options(); let connection = StubAgentConnection::new().with_permission_requests(HashMap::from_iter([( @@ -5043,8 +5049,9 @@ pub(crate) mod tests { let tool_call = acp::ToolCall::new(tool_call_id.clone(), "Run `npm install`").kind(acp::ToolKind::Edit); - let permission_options = ToolPermissionContext::new(TerminalTool::NAME, "npm install") - .build_permission_options(); + let permission_options = + ToolPermissionContext::new(TerminalTool::NAME, vec!["npm install".to_string()]) + .build_permission_options(); let connection = StubAgentConnection::new().with_permission_requests(HashMap::from_iter([( @@ -5132,8 +5139,9 @@ pub(crate) mod tests { let tool_call = acp::ToolCall::new(tool_call_id.clone(), "Run `cargo build`").kind(acp::ToolKind::Edit); - let permission_options = ToolPermissionContext::new(TerminalTool::NAME, "cargo build") - .build_permission_options(); + let permission_options = + ToolPermissionContext::new(TerminalTool::NAME, vec!["cargo build".to_string()]) + .build_permission_options(); let connection = StubAgentConnection::new().with_permission_requests(HashMap::from_iter([( @@ -5211,8 +5219,9 @@ pub(crate) mod tests { let tool_call = acp::ToolCall::new(tool_call_id.clone(), "Run `npm install`").kind(acp::ToolKind::Edit); - let permission_options = ToolPermissionContext::new(TerminalTool::NAME, "npm install") - .build_permission_options(); + let permission_options = + ToolPermissionContext::new(TerminalTool::NAME, vec!["npm install".to_string()]) + .build_permission_options(); // Verify we have the expected options let PermissionOptions::Dropdown(choices) = &permission_options else { @@ -5314,7 +5323,8 @@ pub(crate) mod tests { acp::ToolCall::new(tool_call_id.clone(), "Run `git push`").kind(acp::ToolKind::Edit); let permission_options = - ToolPermissionContext::new(TerminalTool::NAME, "git push").build_permission_options(); + ToolPermissionContext::new(TerminalTool::NAME, vec!["git push".to_string()]) + .build_permission_options(); let connection = StubAgentConnection::new().with_permission_requests(HashMap::from_iter([( @@ -5373,9 +5383,11 @@ pub(crate) mod tests { #[gpui::test] async fn test_option_id_transformation_for_allow() { - let permission_options = - ToolPermissionContext::new(TerminalTool::NAME, "cargo build --release") - .build_permission_options(); + let permission_options = ToolPermissionContext::new( + TerminalTool::NAME, + vec!["cargo build --release".to_string()], + ) + .build_permission_options(); let PermissionOptions::Dropdown(choices) = permission_options else { panic!("Expected dropdown permission options"); @@ -5398,9 +5410,11 @@ pub(crate) mod tests { #[gpui::test] async fn test_option_id_transformation_for_deny() { - let permission_options = - ToolPermissionContext::new(TerminalTool::NAME, "cargo build --release") - .build_permission_options(); + let permission_options = ToolPermissionContext::new( + TerminalTool::NAME, + vec!["cargo build --release".to_string()], + ) + .build_permission_options(); let PermissionOptions::Dropdown(choices) = permission_options else { panic!("Expected dropdown permission options"); diff --git a/crates/agent_ui/src/agent_ui.rs b/crates/agent_ui/src/agent_ui.rs index ee035eb29188a7360c6d33df5334467f397532a4..aca99810b259107fd3be5bcfc05064ff6158a3c3 100644 --- a/crates/agent_ui/src/agent_ui.rs +++ b/crates/agent_ui/src/agent_ui.rs @@ -552,7 +552,7 @@ mod tests { default_profile: AgentProfileId::default(), default_view: DefaultAgentView::Thread, profiles: Default::default(), - always_allow_tool_actions: false, + notify_when_agent_waiting: NotifyWhenAgentWaiting::default(), play_sound_when_agent_done: false, single_file_review: false, diff --git a/crates/eval/runner_settings.json b/crates/eval/runner_settings.json index ea2ccb051164c4a6c40aed9d6607db0a8911c5d6..44a9eb6fc60d5c7e44d945114ab4b71fbb0208c3 100644 --- a/crates/eval/runner_settings.json +++ b/crates/eval/runner_settings.json @@ -1,5 +1,7 @@ { "agent": { - "always_allow_tool_actions": true + "tool_permissions": { + "default": "allow" + } } } diff --git a/crates/migrator/src/migrations.rs b/crates/migrator/src/migrations.rs index 66b7131a06637036c71149957dead64e3ef4e399..5868a2f89ec2ffcebacf97bd712499b70737aa0d 100644 --- a/crates/migrator/src/migrations.rs +++ b/crates/migrator/src/migrations.rs @@ -286,3 +286,9 @@ pub(crate) mod m_2026_02_03 { pub(crate) use settings::migrate_experimental_sweep_mercury; } + +pub(crate) mod m_2026_02_04 { + mod settings; + + pub(crate) use settings::migrate_tool_permission_defaults; +} diff --git a/crates/migrator/src/migrations/m_2026_02_02/settings.rs b/crates/migrator/src/migrations/m_2026_02_02/settings.rs index 53ef04aa5bc3eb54b4c3cfeb2905532d2a80c1d1..a3c7656de373e375878cb960c3dbd75b45e68f5e 100644 --- a/crates/migrator/src/migrations/m_2026_02_02/settings.rs +++ b/crates/migrator/src/migrations/m_2026_02_02/settings.rs @@ -31,7 +31,7 @@ fn migrate_one(obj: &mut serde_json::Map) -> Result<()> { .or_insert_with(|| Value::Object(Default::default())); let Some(edit_predictions_obj) = edit_predictions.as_object_mut() else { - anyhow::bail!("Expected edit_predictions to be an object"); + return Ok(()); }; if !edit_predictions_obj.contains_key("provider") { diff --git a/crates/migrator/src/migrations/m_2026_02_04/settings.rs b/crates/migrator/src/migrations/m_2026_02_04/settings.rs new file mode 100644 index 0000000000000000000000000000000000000000..1fcb50d31f3252ac89e68b5da0d4b8cdac6210fe --- /dev/null +++ b/crates/migrator/src/migrations/m_2026_02_04/settings.rs @@ -0,0 +1,124 @@ +use anyhow::{Result, bail}; +use serde_json::Value; + +use crate::migrations::migrate_settings; + +const AGENT_KEY: &str = "agent"; +const ALWAYS_ALLOW_TOOL_ACTIONS: &str = "always_allow_tool_actions"; +const DEFAULT_KEY: &str = "default"; +const DEFAULT_MODE_KEY: &str = "default_mode"; +const PROFILES_KEY: &str = "profiles"; +const TOOL_PERMISSIONS_KEY: &str = "tool_permissions"; +const TOOLS_KEY: &str = "tools"; + +pub fn migrate_tool_permission_defaults(value: &mut Value) -> Result<()> { + migrate_settings(value, migrate_one) +} + +fn migrate_one(obj: &mut serde_json::Map) -> Result<()> { + if let Some(agent) = obj.get_mut(AGENT_KEY) { + migrate_agent_with_profiles(agent)?; + } + + Ok(()) +} + +fn migrate_agent_with_profiles(agent: &mut Value) -> Result<()> { + migrate_agent_tool_permissions(agent)?; + + if let Some(agent_object) = agent.as_object_mut() { + if let Some(profiles) = agent_object.get_mut(PROFILES_KEY) { + if let Some(profiles_object) = profiles.as_object_mut() { + for (_profile_name, profile) in profiles_object.iter_mut() { + migrate_agent_tool_permissions(profile)?; + } + } + } + } + + Ok(()) +} + +fn migrate_agent_tool_permissions(agent: &mut Value) -> Result<()> { + let Some(agent_object) = agent.as_object_mut() else { + return Ok(()); + }; + + let should_migrate_always_allow = match agent_object.get(ALWAYS_ALLOW_TOOL_ACTIONS) { + Some(Value::Bool(true)) => { + agent_object.remove(ALWAYS_ALLOW_TOOL_ACTIONS); + true + } + Some(Value::Bool(false)) | Some(Value::Null) | None => { + agent_object.remove(ALWAYS_ALLOW_TOOL_ACTIONS); + false + } + Some(_) => { + // Non-boolean value — leave it in place so the schema validator + // can report it, rather than silently dropping user data. + false + } + }; + + if should_migrate_always_allow { + if matches!( + agent_object.get(TOOL_PERMISSIONS_KEY), + None | Some(Value::Null) + ) { + agent_object.insert( + TOOL_PERMISSIONS_KEY.to_string(), + Value::Object(Default::default()), + ); + } + + let Some(Value::Object(tool_permissions_object)) = + agent_object.get_mut(TOOL_PERMISSIONS_KEY) + else { + bail!( + "agent.tool_permissions should be an object or null when migrating \ + always_allow_tool_actions" + ); + }; + + if !tool_permissions_object.contains_key(DEFAULT_KEY) + && !tool_permissions_object.contains_key(DEFAULT_MODE_KEY) + { + tool_permissions_object + .insert(DEFAULT_KEY.to_string(), Value::String("allow".to_string())); + } + } + + if let Some(tool_permissions) = agent_object.get_mut(TOOL_PERMISSIONS_KEY) { + migrate_default_mode_to_default(tool_permissions)?; + } + + Ok(()) +} + +fn migrate_default_mode_to_default(tool_permissions: &mut Value) -> Result<()> { + let Some(tool_permissions_object) = tool_permissions.as_object_mut() else { + return Ok(()); + }; + + if let Some(default_mode) = tool_permissions_object.remove(DEFAULT_MODE_KEY) { + if !tool_permissions_object.contains_key(DEFAULT_KEY) { + tool_permissions_object.insert(DEFAULT_KEY.to_string(), default_mode); + } + } + + if let Some(tools) = tool_permissions_object.get_mut(TOOLS_KEY) { + if let Some(tools_object) = tools.as_object_mut() { + for (_tool_name, tool_rules) in tools_object.iter_mut() { + if let Some(tool_rules_object) = tool_rules.as_object_mut() { + if let Some(default_mode) = tool_rules_object.remove(DEFAULT_MODE_KEY) { + if !tool_rules_object.contains_key(DEFAULT_KEY) { + tool_rules_object.insert(DEFAULT_KEY.to_string(), default_mode); + } + } + } + } + } + } + + Ok(()) +} diff --git a/crates/migrator/src/migrator.rs b/crates/migrator/src/migrator.rs index e9892e28c05768fa4e0384810d9ad891b8723515..5372fc126cc7ecd88db0eb751354bdc81e67495a 100644 --- a/crates/migrator/src/migrator.rs +++ b/crates/migrator/src/migrator.rs @@ -236,6 +236,7 @@ pub fn migrate_settings(text: &str) -> Result> { migrations::m_2026_02_02::move_edit_prediction_provider_to_edit_predictions, ), MigrationType::Json(migrations::m_2026_02_03::migrate_experimental_sweep_mercury), + MigrationType::Json(migrations::m_2026_02_04::migrate_tool_permission_defaults), ]; run_migrations(text, migrations) } @@ -2708,6 +2709,31 @@ mod tests { None, ); + // Non-object edit_predictions (e.g. true) should gracefully skip + // instead of bail!-ing and aborting the entire migration chain. + assert_migrate_settings_with_migrations( + &[MigrationType::Json( + migrations::m_2026_02_02::move_edit_prediction_provider_to_edit_predictions, + )], + &r#" + { + "features": { + "edit_prediction_provider": "copilot" + }, + "edit_predictions": true + } + "# + .unindent(), + Some( + &r#" + { + "edit_predictions": true + } + "# + .unindent(), + ), + ); + // Platform key: settings nested inside "macos" should be migrated assert_migrate_settings_with_migrations( &[MigrationType::Json( @@ -3061,4 +3087,737 @@ mod tests { ), ); } + + #[test] + fn test_migrate_always_allow_tool_actions_to_default() { + // No agent settings - no change + assert_migrate_settings_with_migrations( + &[MigrationType::Json( + migrations::m_2026_02_04::migrate_tool_permission_defaults, + )], + &r#"{ }"#.unindent(), + None, + ); + + // always_allow_tool_actions: true -> tool_permissions.default: "allow" + assert_migrate_settings_with_migrations( + &[MigrationType::Json( + migrations::m_2026_02_04::migrate_tool_permission_defaults, + )], + &r#" + { + "agent": { + "always_allow_tool_actions": true + } + } + "# + .unindent(), + Some( + &r#" + { + "agent": { + "tool_permissions": { + "default": "allow" + } + } + } + "# + .unindent(), + ), + ); + + // always_allow_tool_actions: false -> just remove it + assert_migrate_settings_with_migrations( + &[MigrationType::Json( + migrations::m_2026_02_04::migrate_tool_permission_defaults, + )], + &r#" + { + "agent": { + "always_allow_tool_actions": false + } + } + "# + .unindent(), + Some( + // The blank line has spaces because the migration preserves the original indentation + "{\n \"agent\": {\n \n }\n}\n", + ), + ); + + // Preserve existing tool_permissions.tools when migrating + assert_migrate_settings_with_migrations( + &[MigrationType::Json( + migrations::m_2026_02_04::migrate_tool_permission_defaults, + )], + &r#" + { + "agent": { + "always_allow_tool_actions": true, + "tool_permissions": { + "tools": { + "terminal": { + "always_deny": [{ "pattern": "rm\\s+-rf" }] + } + } + } + } + } + "# + .unindent(), + Some( + &r#" + { + "agent": { + "tool_permissions": { + "default": "allow", + "tools": { + "terminal": { + "always_deny": [{ "pattern": "rm\\s+-rf" }] + } + } + } + } + } + "# + .unindent(), + ), + ); + + // Don't override existing default (and migrate default_mode to default) + assert_migrate_settings_with_migrations( + &[MigrationType::Json( + migrations::m_2026_02_04::migrate_tool_permission_defaults, + )], + &r#" + { + "agent": { + "always_allow_tool_actions": true, + "tool_permissions": { + "default_mode": "confirm" + } + } + } + "# + .unindent(), + Some( + &r#" + { + "agent": { + "tool_permissions": { + "default": "confirm" + } + } + } + "# + .unindent(), + ), + ); + + // Migrate existing default_mode to default (no always_allow_tool_actions) + assert_migrate_settings_with_migrations( + &[MigrationType::Json( + migrations::m_2026_02_04::migrate_tool_permission_defaults, + )], + &r#" + { + "agent": { + "tool_permissions": { + "default_mode": "allow" + } + } + } + "# + .unindent(), + Some( + &r#" + { + "agent": { + "tool_permissions": { + "default": "allow" + } + } + } + "# + .unindent(), + ), + ); + + // No migration needed if already using new format with "default" + assert_migrate_settings_with_migrations( + &[MigrationType::Json( + migrations::m_2026_02_04::migrate_tool_permission_defaults, + )], + &r#" + { + "agent": { + "tool_permissions": { + "default": "allow" + } + } + } + "# + .unindent(), + None, + ); + + // Migrate default_mode to default in tool-specific rules + assert_migrate_settings_with_migrations( + &[MigrationType::Json( + migrations::m_2026_02_04::migrate_tool_permission_defaults, + )], + &r#" + { + "agent": { + "tool_permissions": { + "default_mode": "confirm", + "tools": { + "terminal": { + "default_mode": "allow" + } + } + } + } + } + "# + .unindent(), + Some( + &r#" + { + "agent": { + "tool_permissions": { + "default": "confirm", + "tools": { + "terminal": { + "default": "allow" + } + } + } + } + } + "# + .unindent(), + ), + ); + + // When tool_permissions is null, replace it so always_allow is preserved + assert_migrate_settings_with_migrations( + &[MigrationType::Json( + migrations::m_2026_02_04::migrate_tool_permission_defaults, + )], + &r#" + { + "agent": { + "always_allow_tool_actions": true, + "tool_permissions": null + } + } + "# + .unindent(), + Some( + &r#" + { + "agent": { + "tool_permissions": { + "default": "allow" + } + } + } + "# + .unindent(), + ), + ); + + // Platform-specific agent migration + assert_migrate_settings_with_migrations( + &[MigrationType::Json( + migrations::m_2026_02_04::migrate_tool_permission_defaults, + )], + &r#" + { + "linux": { + "agent": { + "always_allow_tool_actions": true + } + } + } + "# + .unindent(), + Some( + &r#" + { + "linux": { + "agent": { + "tool_permissions": { + "default": "allow" + } + } + } + } + "# + .unindent(), + ), + ); + + // Channel-specific agent migration + assert_migrate_settings_with_migrations( + &[MigrationType::Json( + migrations::m_2026_02_04::migrate_tool_permission_defaults, + )], + &r#" + { + "agent": { + "always_allow_tool_actions": true + }, + "nightly": { + "agent": { + "tool_permissions": { + "default_mode": "confirm" + } + } + } + } + "# + .unindent(), + Some( + &r#" + { + "agent": { + "tool_permissions": { + "default": "allow" + } + }, + "nightly": { + "agent": { + "tool_permissions": { + "default": "confirm" + } + } + } + } + "# + .unindent(), + ), + ); + + // Profile-level migration + assert_migrate_settings_with_migrations( + &[MigrationType::Json( + migrations::m_2026_02_04::migrate_tool_permission_defaults, + )], + &r#" + { + "agent": { + "profiles": { + "custom": { + "always_allow_tool_actions": true, + "tool_permissions": { + "default_mode": "allow" + } + } + } + } + } + "# + .unindent(), + Some( + &r#" + { + "agent": { + "profiles": { + "custom": { + "tool_permissions": { + "default": "allow" + } + } + } + } + } + "# + .unindent(), + ), + ); + + // Platform-specific agent with profiles + assert_migrate_settings_with_migrations( + &[MigrationType::Json( + migrations::m_2026_02_04::migrate_tool_permission_defaults, + )], + &r#" + { + "macos": { + "agent": { + "always_allow_tool_actions": true, + "profiles": { + "strict": { + "tool_permissions": { + "default_mode": "deny" + } + } + } + } + } + } + "# + .unindent(), + Some( + &r#" + { + "macos": { + "agent": { + "tool_permissions": { + "default": "allow" + }, + "profiles": { + "strict": { + "tool_permissions": { + "default": "deny" + } + } + } + } + } + } + "# + .unindent(), + ), + ); + + // Root-level profile with always_allow_tool_actions + assert_migrate_settings_with_migrations( + &[MigrationType::Json( + migrations::m_2026_02_04::migrate_tool_permission_defaults, + )], + &r#" + { + "profiles": { + "work": { + "agent": { + "always_allow_tool_actions": true + } + } + } + } + "# + .unindent(), + Some( + &r#" + { + "profiles": { + "work": { + "agent": { + "tool_permissions": { + "default": "allow" + } + } + } + } + } + "# + .unindent(), + ), + ); + + // Root-level profile with default_mode + assert_migrate_settings_with_migrations( + &[MigrationType::Json( + migrations::m_2026_02_04::migrate_tool_permission_defaults, + )], + &r#" + { + "profiles": { + "work": { + "agent": { + "tool_permissions": { + "default_mode": "allow" + } + } + } + } + } + "# + .unindent(), + Some( + &r#" + { + "profiles": { + "work": { + "agent": { + "tool_permissions": { + "default": "allow" + } + } + } + } + } + "# + .unindent(), + ), + ); + + // Root-level profile + root-level agent both migrated + assert_migrate_settings_with_migrations( + &[MigrationType::Json( + migrations::m_2026_02_04::migrate_tool_permission_defaults, + )], + &r#" + { + "agent": { + "always_allow_tool_actions": true + }, + "profiles": { + "strict": { + "agent": { + "tool_permissions": { + "default_mode": "deny" + } + } + } + } + } + "# + .unindent(), + Some( + &r#" + { + "agent": { + "tool_permissions": { + "default": "allow" + } + }, + "profiles": { + "strict": { + "agent": { + "tool_permissions": { + "default": "deny" + } + } + } + } + } + "# + .unindent(), + ), + ); + + // Non-boolean always_allow_tool_actions (string "true") is left in place + // so the schema validator can report it, rather than silently dropping user data. + assert_migrate_settings_with_migrations( + &[MigrationType::Json( + migrations::m_2026_02_04::migrate_tool_permission_defaults, + )], + &r#" + { + "agent": { + "always_allow_tool_actions": "true" + } + } + "# + .unindent(), + None, + ); + + // null always_allow_tool_actions is removed (treated as false) + assert_migrate_settings_with_migrations( + &[MigrationType::Json( + migrations::m_2026_02_04::migrate_tool_permission_defaults, + )], + &r#" + { + "agent": { + "always_allow_tool_actions": null + } + } + "# + .unindent(), + Some(&"{\n \"agent\": {\n \n }\n}\n"), + ); + + // Project-local settings (.zed/settings.json) with always_allow_tool_actions + // These files have no platform/channel overrides or root-level profiles. + assert_migrate_settings_with_migrations( + &[MigrationType::Json( + migrations::m_2026_02_04::migrate_tool_permission_defaults, + )], + &r#" + { + "agent": { + "always_allow_tool_actions": true, + "tool_permissions": { + "tools": { + "terminal": { + "default_mode": "confirm", + "always_deny": [{ "pattern": "rm\\s+-rf" }] + } + } + } + } + } + "# + .unindent(), + Some( + &r#" + { + "agent": { + "tool_permissions": { + "default": "allow", + "tools": { + "terminal": { + "default": "confirm", + "always_deny": [{ "pattern": "rm\\s+-rf" }] + } + } + } + } + } + "# + .unindent(), + ), + ); + + // Project-local settings with only default_mode (no always_allow_tool_actions) + assert_migrate_settings_with_migrations( + &[MigrationType::Json( + migrations::m_2026_02_04::migrate_tool_permission_defaults, + )], + &r#" + { + "agent": { + "tool_permissions": { + "default_mode": "deny" + } + } + } + "# + .unindent(), + Some( + &r#" + { + "agent": { + "tool_permissions": { + "default": "deny" + } + } + } + "# + .unindent(), + ), + ); + + // Project-local settings with no agent section at all - no change + assert_migrate_settings_with_migrations( + &[MigrationType::Json( + migrations::m_2026_02_04::migrate_tool_permission_defaults, + )], + &r#" + { + "tab_size": 4, + "format_on_save": "on" + } + "# + .unindent(), + None, + ); + + // Existing agent_servers are left untouched + assert_migrate_settings_with_migrations( + &[MigrationType::Json( + migrations::m_2026_02_04::migrate_tool_permission_defaults, + )], + &r#" + { + "agent": { + "always_allow_tool_actions": true + }, + "agent_servers": { + "claude": { + "default_mode": "plan" + }, + "codex": { + "default_mode": "read-only" + } + } + } + "# + .unindent(), + Some( + &r#" + { + "agent": { + "tool_permissions": { + "default": "allow" + } + }, + "agent_servers": { + "claude": { + "default_mode": "plan" + }, + "codex": { + "default_mode": "read-only" + } + } + } + "# + .unindent(), + ), + ); + + // Existing agent_servers are left untouched even with partial entries + assert_migrate_settings_with_migrations( + &[MigrationType::Json( + migrations::m_2026_02_04::migrate_tool_permission_defaults, + )], + &r#" + { + "agent": { + "always_allow_tool_actions": true + }, + "agent_servers": { + "claude": { + "default_mode": "plan" + } + } + } + "# + .unindent(), + Some( + &r#" + { + "agent": { + "tool_permissions": { + "default": "allow" + } + }, + "agent_servers": { + "claude": { + "default_mode": "plan" + } + } + } + "# + .unindent(), + ), + ); + + // always_allow_tool_actions: false leaves agent_servers untouched + assert_migrate_settings_with_migrations( + &[MigrationType::Json( + migrations::m_2026_02_04::migrate_tool_permission_defaults, + )], + &r#" + { + "agent": { + "always_allow_tool_actions": false + }, + "agent_servers": { + "claude": {} + } + } + "# + .unindent(), + Some( + "{\n \"agent\": {\n \n },\n \"agent_servers\": {\n \"claude\": {}\n }\n}\n", + ), + ); + } } diff --git a/crates/settings_content/src/agent.rs b/crates/settings_content/src/agent.rs index 4bd64441863bf0aede39ad3bc4e3fcfa25ac84df..c592c13c133fe264b254db44250b37dcf520a504 100644 --- a/crates/settings_content/src/agent.rs +++ b/crates/settings_content/src/agent.rs @@ -49,9 +49,7 @@ pub struct AgentSettingsContent { /// /// Default: true pub inline_assistant_use_streaming_tools: Option, - /// Model to use for generating git commit messages. - /// - /// Default: true + /// Model to use for generating git commit messages. Defaults to default_model when not specified. pub commit_message_model: Option, /// Model to use for generating thread summaries. Defaults to default_model when not specified. pub thread_summary_model: Option, @@ -67,20 +65,6 @@ pub struct AgentSettingsContent { pub default_view: Option, /// The available agent profiles. pub profiles: Option, AgentProfileContent>>, - /// Whenever a tool action would normally wait for your confirmation - /// that you allow it, always choose to allow it. - /// - /// **Security note**: Even with this enabled, Zed's built-in security rules - /// still block some tool actions, such as the terminal tool running `rm -rf /`, `rm -rf ~`, - /// `rm -rf $HOME`, `rm -rf .`, or `rm -rf ..`, to prevent certain classes of failures - /// from happening. - /// - /// This setting has no effect on external agents that support permission modes, such as Claude Code. - /// - /// Set `agent_servers.claude.default_mode` to `bypassPermissions`, to disable all permission requests when using Claude Code. - /// - /// Default: false - pub always_allow_tool_actions: Option, /// Where to show a popup notification when the agent is waiting for user input. /// /// Default: "primary_screen" @@ -131,10 +115,16 @@ pub struct AgentSettingsContent { /// /// Default: false pub show_turn_stats: Option, - /// Per-tool permission rules for granular control over which tool actions require confirmation. - /// - /// This setting only applies to the native Zed agent. External agent servers (Claude Code, Gemini CLI, etc.) - /// have their own permission systems and are not affected by these settings. + /// Per-tool permission rules for granular control over which tool actions + /// require confirmation. + /// + /// The global `default` applies when no tool-specific rules match. + /// For external agent servers (e.g. Claude Code) that define their own + /// permission modes, "deny" and "confirm" still take precedence — the + /// external agent's permission system is only used when Zed would allow + /// the action. Per-tool regex patterns (`always_allow`, `always_deny`, + /// `always_confirm`) match against the tool's text input (command, path, + /// URL, etc.). pub tool_permissions: Option, } @@ -170,13 +160,13 @@ impl AgentSettingsContent { self.favorite_models.retain(|m| m != model); } - pub fn set_tool_default_mode(&mut self, tool_id: &str, mode: ToolPermissionMode) { + pub fn set_tool_default_permission(&mut self, tool_id: &str, mode: ToolPermissionMode) { let tool_permissions = self.tool_permissions.get_or_insert_default(); let tool_rules = tool_permissions .tools .entry(Arc::from(tool_id)) .or_default(); - tool_rules.default_mode = Some(mode); + tool_rules.default = Some(mode); } pub fn add_tool_allow_pattern(&mut self, tool_name: &str, pattern: String) { @@ -530,9 +520,16 @@ pub enum CustomAgentServerSettings { #[with_fallible_options] #[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize, JsonSchema, MergeFrom)] pub struct ToolPermissionsContent { + /// Global default permission when no tool-specific rules match. + /// Individual tools can override this with their own default. + /// Default: confirm + #[serde(alias = "default_mode")] + pub default: Option, + /// Per-tool permission rules. - /// Keys: terminal, edit_file, delete_path, move_path, create_directory, - /// save_file, fetch, web_search + /// Keys are tool names (e.g. terminal, edit_file, fetch) including MCP + /// tools (e.g. mcp:server_name:tool_name). Any tool name is accepted; + /// even tools without meaningful text input can have a `default` set. #[serde(default)] pub tools: HashMap, ToolRulesContent>, } @@ -541,21 +538,34 @@ pub struct ToolPermissionsContent { #[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize, JsonSchema, MergeFrom)] pub struct ToolRulesContent { /// Default mode when no regex rules match. - /// Default: confirm - pub default_mode: Option, + /// When unset, inherits from the global `tool_permissions.default`. + #[serde(alias = "default_mode")] + pub default: Option, /// Regexes for inputs to auto-approve. /// For terminal: matches command. For file tools: matches path. For fetch: matches URL. + /// For `copy_path` and `move_path`, patterns are matched independently against each + /// path (source and destination). + /// Patterns accumulate across settings layers (user, project, profile) and cannot be + /// removed by a higher-priority layer—only new patterns can be added. /// Default: [] pub always_allow: Option>, /// Regexes for inputs to auto-reject. /// **SECURITY**: These take precedence over ALL other rules, across ALL settings layers. + /// For `copy_path` and `move_path`, patterns are matched independently against each + /// path (source and destination). + /// Patterns accumulate across settings layers (user, project, profile) and cannot be + /// removed by a higher-priority layer—only new patterns can be added. /// Default: [] pub always_deny: Option>, /// Regexes for inputs that must always prompt. /// Takes precedence over always_allow but not always_deny. + /// For `copy_path` and `move_path`, patterns are matched independently against each + /// path (source and destination). + /// Patterns accumulate across settings layers (user, project, profile) and cannot be + /// removed by a higher-priority layer—only new patterns can be added. /// Default: [] pub always_confirm: Option>, } @@ -586,46 +596,56 @@ pub enum ToolPermissionMode { Confirm, } +impl std::fmt::Display for ToolPermissionMode { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + ToolPermissionMode::Allow => write!(f, "Allow"), + ToolPermissionMode::Deny => write!(f, "Deny"), + ToolPermissionMode::Confirm => write!(f, "Confirm"), + } + } +} + #[cfg(test)] mod tests { use super::*; #[test] - fn test_set_tool_default_mode_creates_structure() { + fn test_set_tool_default_permission_creates_structure() { let mut settings = AgentSettingsContent::default(); assert!(settings.tool_permissions.is_none()); - settings.set_tool_default_mode("terminal", ToolPermissionMode::Allow); + settings.set_tool_default_permission("terminal", ToolPermissionMode::Allow); let tool_permissions = settings.tool_permissions.as_ref().unwrap(); let terminal_rules = tool_permissions.tools.get("terminal").unwrap(); - assert_eq!(terminal_rules.default_mode, Some(ToolPermissionMode::Allow)); + assert_eq!(terminal_rules.default, Some(ToolPermissionMode::Allow)); } #[test] - fn test_set_tool_default_mode_updates_existing() { + fn test_set_tool_default_permission_updates_existing() { let mut settings = AgentSettingsContent::default(); - settings.set_tool_default_mode("terminal", ToolPermissionMode::Confirm); - settings.set_tool_default_mode("terminal", ToolPermissionMode::Allow); + settings.set_tool_default_permission("terminal", ToolPermissionMode::Confirm); + settings.set_tool_default_permission("terminal", ToolPermissionMode::Allow); let tool_permissions = settings.tool_permissions.as_ref().unwrap(); let terminal_rules = tool_permissions.tools.get("terminal").unwrap(); - assert_eq!(terminal_rules.default_mode, Some(ToolPermissionMode::Allow)); + assert_eq!(terminal_rules.default, Some(ToolPermissionMode::Allow)); } #[test] - fn test_set_tool_default_mode_for_mcp_tool() { + fn test_set_tool_default_permission_for_mcp_tool() { let mut settings = AgentSettingsContent::default(); - settings.set_tool_default_mode("mcp:github:create_issue", ToolPermissionMode::Allow); + settings.set_tool_default_permission("mcp:github:create_issue", ToolPermissionMode::Allow); let tool_permissions = settings.tool_permissions.as_ref().unwrap(); let mcp_rules = tool_permissions .tools .get("mcp:github:create_issue") .unwrap(); - assert_eq!(mcp_rules.default_mode, Some(ToolPermissionMode::Allow)); + assert_eq!(mcp_rules.default, Some(ToolPermissionMode::Allow)); } #[test] diff --git a/crates/settings_ui/Cargo.toml b/crates/settings_ui/Cargo.toml index e1bc70f1f448497bc22a6bcb8e3caf8ceff05690..4e3962b9aa207dc7314201a5de538225f228541a 100644 --- a/crates/settings_ui/Cargo.toml +++ b/crates/settings_ui/Cargo.toml @@ -38,8 +38,8 @@ log.workspace = true menu.workspace = true paths.workspace = true picker.workspace = true -platform_title_bar.workspace = true regex.workspace = true +platform_title_bar.workspace = true project.workspace = true release_channel.workspace = true schemars.workspace = true diff --git a/crates/settings_ui/src/page_data.rs b/crates/settings_ui/src/page_data.rs index 7168121563c887d215ab5fa1ecd53940e0c4255c..19b76040611c463e87b39ee09b004ca03826d0ff 100644 --- a/crates/settings_ui/src/page_data.rs +++ b/crates/settings_ui/src/page_data.rs @@ -6870,31 +6870,9 @@ fn ai_page() -> SettingsPage { ] } - fn agent_configuration_section() -> [SettingsPageItem; 13] { + fn agent_configuration_section() -> [SettingsPageItem; 12] { [ SettingsPageItem::SectionHeader("Agent Configuration"), - SettingsPageItem::SettingItem(SettingItem { - title: "Always Allow Tool Actions", - description: "When enabled, the agent can run potentially destructive actions without asking for your confirmation. This setting has no effect on external agents.", - field: Box::new(SettingField { - json_path: Some("agent.always_allow_tool_actions"), - pick: |settings_content| { - settings_content - .agent - .as_ref()? - .always_allow_tool_actions - .as_ref() - }, - write: |settings_content, value| { - settings_content - .agent - .get_or_insert_default() - .always_allow_tool_actions = value; - }, - }), - metadata: None, - files: USER, - }), SettingsPageItem::SubPageLink(SubPageLink { title: "Tool Permissions".into(), r#type: Default::default(), diff --git a/crates/settings_ui/src/pages.rs b/crates/settings_ui/src/pages.rs index 3ea489abe0801fad6ce5666cc5543230b09add83..b68af5725e9396033a5a3e74fc635d64add0e779 100644 --- a/crates/settings_ui/src/pages.rs +++ b/crates/settings_ui/src/pages.rs @@ -5,7 +5,8 @@ pub(crate) use edit_prediction_provider_setup::render_edit_prediction_setup_page pub(crate) use tool_permissions_setup::render_tool_permissions_setup_page; pub use tool_permissions_setup::{ - render_create_directory_tool_config, render_delete_path_tool_config, - render_edit_file_tool_config, render_fetch_tool_config, render_move_path_tool_config, + render_copy_path_tool_config, render_create_directory_tool_config, + render_delete_path_tool_config, render_edit_file_tool_config, render_fetch_tool_config, + render_move_path_tool_config, render_restore_file_from_disk_tool_config, render_save_file_tool_config, render_terminal_tool_config, render_web_search_tool_config, }; diff --git a/crates/settings_ui/src/pages/tool_permissions_setup.rs b/crates/settings_ui/src/pages/tool_permissions_setup.rs index b2058665f98c115a34f4fb00deb3dece016def1d..e3ec03516cab9d165b3b996105f1de0899f7b239 100644 --- a/crates/settings_ui/src/pages/tool_permissions_setup.rs +++ b/crates/settings_ui/src/pages/tool_permissions_setup.rs @@ -9,6 +9,7 @@ use shell_command_parser::extract_commands; use std::sync::Arc; use theme::ThemeSettings; use ui::{Banner, ContextMenu, Divider, PopoverMenu, Severity, Tooltip, prelude::*}; +use util::ResultExt as _; use util::shell::ShellKind; use crate::{SettingsWindow, components::SettingsInputField}; @@ -73,6 +74,12 @@ const TOOLS: &[ToolInfo] = &[ description: "Web search queries", regex_explanation: "Patterns are matched against the search query.", }, + ToolInfo { + id: "restore_file_from_disk", + name: "Restore File from Disk", + description: "Discards unsaved changes by reloading from disk", + regex_explanation: "Patterns are matched against the file path being restored.", + }, ]; pub(crate) struct ToolInfo { @@ -156,6 +163,9 @@ pub(crate) fn render_tool_permissions_setup_page( .map(|(i, tool)| render_tool_list_item(settings_window, tool, i, window, cx)) .collect(); + let settings = AgentSettings::get_global(cx); + let global_default = settings.tool_permissions.default; + let scroll_step = px(40.); v_flex() @@ -192,13 +202,16 @@ pub(crate) fn render_tool_permissions_setup_page( ), ) .child( - v_flex().children(tool_items.into_iter().enumerate().flat_map(|(i, item)| { - let mut elements: Vec = vec![item]; - if i + 1 < TOOLS.len() { - elements.push(Divider::horizontal().into_any_element()); - } - elements - })), + v_flex() + .child(render_global_default_mode_section(global_default)) + .child(Divider::horizontal()) + .children(tool_items.into_iter().enumerate().flat_map(|(i, item)| { + let mut elements: Vec = vec![item]; + if i + 1 < TOOLS.len() { + elements.push(Divider::horizontal().into_any_element()); + } + elements + })), ) .into_any_element() } @@ -293,6 +306,7 @@ fn get_tool_render_fn( "save_file" => render_save_file_tool_config, "fetch" => render_fetch_tool_config, "web_search" => render_web_search_tool_config, + "restore_file_from_disk" => render_restore_file_from_disk_tool_config, _ => render_terminal_tool_config, // fallback } } @@ -372,7 +386,7 @@ pub(crate) fn render_tool_config_page( .min_w_0() .w_full() .gap_5() - .child(render_default_mode_section(tool.id, rules.default_mode, cx)) + .child(render_default_mode_section(tool.id, rules.default, cx)) .child(Divider::horizontal().color(ui::DividerColor::BorderFaded)) .child(render_rule_section( tool.id, @@ -440,9 +454,6 @@ fn render_verification_section( ) -> AnyElement { let input_id = format!("{}-verification-input", tool_id); - let settings = AgentSettings::get_global(cx); - let always_allow_enabled = settings.always_allow_tool_actions; - let editor = window.use_keyed_state(input_id, cx, |window, cx| { let mut editor = editor::Editor::single_line(window, cx); editor.set_placeholder_text("Enter a tool input to test your rules…", window, cx); @@ -470,7 +481,7 @@ fn render_verification_section( (Some(decision), matches) }; - let default_mode = get_tool_rules(tool_id, cx).default_mode; + let default_mode = get_tool_rules(tool_id, cx).default; let is_hardcoded_denial = matches!( &decision, Some(ToolPermissionDecision::Deny(reason)) @@ -504,37 +515,12 @@ fn render_verification_section( None => (None, true), }; - let always_allow_description = "The Always Allow Tool Actions setting is enabled: all tools will be allowed regardless of these rules."; let color = cx.theme().colors(); v_flex() .mt_3() .min_w_0() .gap_2() - .when(always_allow_enabled, |this| { - this.child( - Banner::new() - .severity(Severity::Warning) - .wrap_content(false) - .child( - Label::new(always_allow_description) - .size(LabelSize::Small) - .mt(px(3.)) - .mr_8(), - ) - .action_slot( - Button::new("configure_setting", "Configure Setting") - .label_size(LabelSize::Small) - .on_click(cx.listener(|this, _, window, cx| { - this.navigate_to_setting( - "agent.always_allow_tool_actions", - window, - cx, - ); - })), - ), - ) - }) .child( v_flex() .p_2p5() @@ -561,11 +547,11 @@ fn render_verification_section( .track_focus(&focus_handle) .child(editor), ) - .when_some(authoritative_mode, |this, _| { + .when_some(authoritative_mode, |this, mode| { this.when(patterns_agree, |this| { if matched_patterns.is_empty() { this.child( - Label::new("No regex matches, using the default action (confirm).") + Label::new("No regex matches, using the default action.") .size(LabelSize::Small) .color(Color::Muted), ) @@ -578,24 +564,32 @@ fn render_verification_section( this.child(render_hardcoded_rules(true, cx)) } else if let Some(reason) = &denial_reason { this.child( - Label::new(reason).size(LabelSize::XSmall), + Label::new(format!("Denied: {}", reason)) + .size(LabelSize::XSmall) + .color(Color::Warning), ) } else { this.child( Label::new( "Pattern preview differs from engine — showing authoritative result.", ) - .size(LabelSize::XSmall), + .size(LabelSize::XSmall) + .color(Color::Warning), ) } }) .when(is_hardcoded_denial && patterns_agree, |this| { this.child(render_hardcoded_rules(true, cx)) }) + .child(render_verdict_label(mode)) .when_some( denial_reason.filter(|_| patterns_agree && !is_hardcoded_denial), |this, reason| { - this.child(Label::new(reason).size(LabelSize::XSmall)) + this.child( + Label::new(format!("Reason: {}", reason)) + .size(LabelSize::XSmall) + .color(Color::Error), + ) }, ) }), @@ -726,15 +720,12 @@ fn render_matched_patterns(patterns: &[MatchedPattern], cx: &App) -> AnyElement fn evaluate_test_input(tool_id: &str, input: &str, cx: &App) -> ToolPermissionDecision { let settings = AgentSettings::get_global(cx); - // Always pass false for always_allow_tool_actions so we test the actual rules, - // not the global override that bypasses all checks. // ShellKind is only used for terminal tool's hardcoded security rules; // for other tools, the check returns None immediately. ToolPermissionDecision::from_input( tool_id, - input, + &[input.to_string()], &settings.tool_permissions, - false, ShellKind::system(), ) } @@ -780,6 +771,132 @@ fn mode_display_label(mode: ToolPermissionMode) -> &'static str { } } +fn verdict_color(mode: ToolPermissionMode) -> Color { + match mode { + ToolPermissionMode::Allow => Color::Success, + ToolPermissionMode::Deny => Color::Error, + ToolPermissionMode::Confirm => Color::Warning, + } +} + +fn render_verdict_label(mode: ToolPermissionMode) -> AnyElement { + h_flex() + .gap_1() + .child( + Label::new("Result:") + .size(LabelSize::Small) + .color(Color::Muted), + ) + .child( + Label::new(mode_display_label(mode)) + .size(LabelSize::Small) + .color(verdict_color(mode)), + ) + .into_any_element() +} + +fn render_invalid_patterns_section( + tool_id: &'static str, + invalid_patterns: &[InvalidPatternView], + cx: &mut Context, +) -> AnyElement { + let section_id = format!("{}-invalid-patterns-section", tool_id); + let theme_colors = cx.theme().colors(); + + v_flex() + .id(section_id) + .child( + h_flex() + .gap_1() + .child( + Icon::new(IconName::Warning) + .size(IconSize::Small) + .color(Color::Error), + ) + .child(Label::new("Invalid Patterns").color(Color::Error)), + ) + .child( + Label::new( + "These patterns failed to compile as regular expressions. \ + The tool will be blocked until they are fixed or removed.", + ) + .size(LabelSize::Small) + .color(Color::Muted), + ) + .child( + v_flex() + .mt_2() + .w_full() + .gap_1p5() + .children(invalid_patterns.iter().map(|invalid| { + let rule_type_label = match invalid.rule_type.as_str() { + "always_allow" => "Always Allow", + "always_deny" => "Always Deny", + "always_confirm" => "Always Confirm", + other => other, + }; + + let pattern_for_delete = invalid.pattern.clone(); + let rule_type = match invalid.rule_type.as_str() { + "always_allow" => ToolPermissionMode::Allow, + "always_deny" => ToolPermissionMode::Deny, + _ => ToolPermissionMode::Confirm, + }; + let tool_id_for_delete = tool_id.to_string(); + let delete_id = + format!("{}-invalid-delete-{}", tool_id, invalid.pattern.clone()); + + v_flex() + .p_2() + .rounded_md() + .border_1() + .border_color(theme_colors.border_variant) + .bg(theme_colors.surface_background.opacity(0.15)) + .gap_1() + .child( + h_flex() + .justify_between() + .child( + h_flex() + .gap_1p5() + .min_w_0() + .child( + Label::new(invalid.pattern.clone()) + .size(LabelSize::Small) + .color(Color::Error) + .buffer_font(cx), + ) + .child( + Label::new(format!("({})", rule_type_label)) + .size(LabelSize::XSmall) + .color(Color::Muted), + ), + ) + .child( + IconButton::new(delete_id, IconName::Trash) + .icon_size(IconSize::Small) + .icon_color(Color::Muted) + .tooltip(Tooltip::text("Delete Invalid Pattern")) + .on_click(cx.listener(move |_, _, _, cx| { + delete_pattern( + &tool_id_for_delete, + rule_type, + &pattern_for_delete, + cx, + ); + })), + ), + ) + .child( + Label::new(format!("Error: {}", invalid.error)) + .size(LabelSize::XSmall) + .color(Color::Muted), + ) + })), + ) + .into_any_element() +} + fn render_rule_section( tool_id: &'static str, title: &'static str, @@ -826,35 +943,6 @@ fn render_rule_section( .into_any_element() } -fn render_invalid_patterns_section( - tool_id: &'static str, - invalid_patterns: &[String], - _cx: &mut Context, -) -> AnyElement { - let section_id = format!("{}-invalid-section", tool_id); - - v_flex() - .id(section_id) - .child(Label::new("Invalid Patterns").color(Color::Error)) - .child( - Label::new("These patterns failed to compile as valid regexes. They will block the tool from running until fixed or removed.") - .size(LabelSize::Small) - .color(Color::Muted), - ) - .child( - v_flex() - .mt_2() - .gap_1() - .children(invalid_patterns.iter().map(|description| { - Label::new(description.clone()) - .size(LabelSize::Small) - .color(Color::Error) - .into_any_element() - })), - ) - .into_any_element() -} - fn render_pattern_empty_state(cx: &mut Context) -> AnyElement { h_flex() .p_2() @@ -883,6 +971,7 @@ fn render_user_pattern_row( let tool_id_for_update = tool_id.to_string(); let input_id = format!("{}-{:?}-pattern-{}", tool_id, rule_type, index); let delete_id = format!("{}-{:?}-delete-{}", tool_id, rule_type, index); + let settings_window = cx.entity().downgrade(); SettingsInputField::new() .with_id(input_id) @@ -903,13 +992,33 @@ fn render_user_pattern_row( if let Some(new_pattern) = new_pattern { let new_pattern = new_pattern.trim().to_string(); if !new_pattern.is_empty() && new_pattern != pattern_for_update { - update_pattern( + let updated = update_pattern( &tool_id_for_update, rule_type, &pattern_for_update, - new_pattern, + new_pattern.clone(), cx, ); + + let validation_error = if !updated { + Some( + "A pattern with that name already exists in this rule list." + .to_string(), + ) + } else { + match regex::Regex::new(&new_pattern) { + Err(err) => Some(format!( + "Invalid regex: {err}. Pattern saved but will block this tool until fixed or removed." + )), + Ok(_) => None, + } + }; + settings_window + .update(cx, |this, cx| { + this.regex_validation_error = validation_error; + cx.notify(); + }) + .log_err(); } } }) @@ -919,10 +1028,11 @@ fn render_user_pattern_row( fn render_add_pattern_input( tool_id: &'static str, rule_type: ToolPermissionMode, - _cx: &mut Context, + cx: &mut Context, ) -> AnyElement { let tool_id_owned = tool_id.to_string(); let input_id = format!("{}-{:?}-new-pattern", tool_id, rule_type); + let settings_window = cx.entity().downgrade(); SettingsInputField::new() .with_id(input_id) @@ -936,16 +1046,72 @@ fn render_add_pattern_input( if let Some(pattern) = pattern { let trimmed = pattern.trim().to_string(); if !trimmed.is_empty() { - if let Err(err) = regex::Regex::new(&trimmed) { - log::warn!("Invalid regex pattern '{}': {}", trimmed, err); - } - save_pattern(&tool_id_owned, rule_type, trimmed, cx); + save_pattern(&tool_id_owned, rule_type, trimmed.clone(), cx); + + let validation_error = match regex::Regex::new(&trimmed) { + Err(err) => Some(format!( + "Invalid regex: {err}. Pattern saved but will block this tool until fixed or removed." + )), + Ok(_) => None, + }; + settings_window + .update(cx, |this, cx| { + this.regex_validation_error = validation_error; + cx.notify(); + }) + .log_err(); } } }) .into_any_element() } +fn render_global_default_mode_section(current_mode: ToolPermissionMode) -> AnyElement { + let mode_label = current_mode.to_string(); + + h_flex() + .mt_4() + .justify_between() + .child( + v_flex() + .child(Label::new("Default Permission")) + .child( + Label::new( + "Controls the default behavior for all tool actions. Per-tool rules and patterns can override this.", + ) + .size(LabelSize::Small) + .color(Color::Muted), + ), + ) + .child( + PopoverMenu::new("global-default-mode") + .trigger( + Button::new("global-mode-trigger", mode_label) + .tab_index(0_isize) + .style(ButtonStyle::Outlined) + .size(ButtonSize::Medium) + .icon(IconName::ChevronDown) + .icon_position(IconPosition::End) + .icon_size(IconSize::Small), + ) + .menu(move |window, cx| { + Some(ContextMenu::build(window, cx, move |menu, _, _| { + menu.entry("Confirm", None, move |_, cx| { + set_global_default_permission(ToolPermissionMode::Confirm, cx); + }) + .entry("Allow", None, move |_, cx| { + set_global_default_permission(ToolPermissionMode::Allow, cx); + }) + .entry("Deny", None, move |_, cx| { + set_global_default_permission(ToolPermissionMode::Deny, cx); + }) + })) + }) + .anchor(gpui::Corner::TopRight), + ) + .into_any_element() +} + fn render_default_mode_section( tool_id: &'static str, current_mode: ToolPermissionMode, @@ -1002,12 +1168,18 @@ fn render_default_mode_section( .into_any_element() } +struct InvalidPatternView { + pattern: String, + rule_type: String, + error: String, +} + struct ToolRulesView { - default_mode: ToolPermissionMode, + default: ToolPermissionMode, always_allow: Vec, always_deny: Vec, always_confirm: Vec, - invalid_patterns: Vec, + invalid_patterns: Vec, } fn get_tool_rules(tool_name: &str, cx: &App) -> ToolRulesView { @@ -1017,7 +1189,7 @@ fn get_tool_rules(tool_name: &str, cx: &App) -> ToolRulesView { match tool_rules { Some(rules) => ToolRulesView { - default_mode: rules.default_mode, + default: rules.default.unwrap_or(settings.tool_permissions.default), always_allow: rules .always_allow .iter() @@ -1036,11 +1208,15 @@ fn get_tool_rules(tool_name: &str, cx: &App) -> ToolRulesView { invalid_patterns: rules .invalid_patterns .iter() - .map(|p| format!("{} ({}): {}", p.pattern, p.rule_type, p.error)) + .map(|p| InvalidPatternView { + pattern: p.pattern.clone(), + rule_type: p.rule_type.clone(), + error: p.error.clone(), + }) .collect(), }, None => ToolRulesView { - default_mode: ToolPermissionMode::Confirm, + default: settings.tool_permissions.default, always_allow: Vec::new(), always_deny: Vec::new(), always_confirm: Vec::new(), @@ -1086,7 +1262,19 @@ fn update_pattern( old_pattern: &str, new_pattern: String, cx: &mut App, -) { +) -> bool { + let settings = AgentSettings::get_global(cx); + if let Some(tool_rules) = settings.tool_permissions.tools.get(tool_name) { + let patterns = match rule_type { + ToolPermissionMode::Allow => &tool_rules.always_allow, + ToolPermissionMode::Deny => &tool_rules.always_deny, + ToolPermissionMode::Confirm => &tool_rules.always_confirm, + }; + if patterns.iter().any(|r| r.pattern == new_pattern) { + return false; + } + } + let tool_name = tool_name.to_string(); let old_pattern = old_pattern.to_string(); @@ -1105,12 +1293,17 @@ fn update_pattern( }; if let Some(list) = rules_list { - if let Some(rule) = list.0.iter_mut().find(|r| r.pattern == old_pattern) { - rule.pattern = new_pattern; + let already_exists = list.0.iter().any(|r| r.pattern == new_pattern); + if !already_exists { + if let Some(rule) = list.0.iter_mut().find(|r| r.pattern == old_pattern) { + rule.pattern = new_pattern; + } } } } }); + + true } fn delete_pattern(tool_name: &str, rule_type: ToolPermissionMode, pattern: &str, cx: &mut App) { @@ -1138,6 +1331,17 @@ fn delete_pattern(tool_name: &str, rule_type: ToolPermissionMode, pattern: &str, }); } +fn set_global_default_permission(mode: ToolPermissionMode, cx: &mut App) { + SettingsStore::global(cx).update_settings_file(::global(cx), move |settings, _| { + settings + .agent + .get_or_insert_default() + .tool_permissions + .get_or_insert_default() + .default = Some(mode); + }); +} + fn set_default_mode(tool_name: &str, mode: ToolPermissionMode, cx: &mut App) { let tool_name = tool_name.to_string(); @@ -1151,7 +1355,7 @@ fn set_default_mode(tool_name: &str, mode: ToolPermissionMode, cx: &mut App) { .tools .entry(Arc::from(tool_name.as_str())) .or_default(); - tool_rules.default_mode = Some(mode); + tool_rules.default = Some(mode); }); } @@ -1178,6 +1382,10 @@ tool_config_page_fn!(render_create_directory_tool_config, "create_directory"); tool_config_page_fn!(render_save_file_tool_config, "save_file"); tool_config_page_fn!(render_fetch_tool_config, "fetch"); tool_config_page_fn!(render_web_search_tool_config, "web_search"); +tool_config_page_fn!( + render_restore_file_from_disk_tool_config, + "restore_file_from_disk" +); #[cfg(test)] mod tests { @@ -1185,7 +1393,12 @@ mod tests { #[test] fn test_all_tools_are_in_tool_info_or_excluded() { + // Tools that intentionally don't appear in the permissions UI. + // If you add a new tool and this test fails, either: + // 1. Add a ToolInfo entry to TOOLS (if the tool has permission checks), or + // 2. Add it to this list with a comment explaining why it's excluded. const EXCLUDED_TOOLS: &[&str] = &[ + // Read-only / low-risk tools that don't call decide_permission_from_settings "diagnostics", "find_path", "grep", @@ -1193,9 +1406,12 @@ mod tests { "now", "open", "read_file", - "restore_file_from_disk", "thinking", + // streaming_edit_file uses "edit_file" for permission lookups, + // so its rules are configured under the edit_file entry. "streaming_edit_file", + // Subagent permission checks happen at the level of individual + // tool calls within the subagent, not at the spawning level. "subagent", ]; diff --git a/crates/zed/src/visual_test_runner.rs b/crates/zed/src/visual_test_runner.rs index f167454e9ab4e2b6e329925c0f2ee4c9c29951eb..9b9e8dab38a5738f9b7d51c14ad5e7c66f0e9c0e 100644 --- a/crates/zed/src/visual_test_runner.rs +++ b/crates/zed/src/visual_test_runner.rs @@ -354,7 +354,7 @@ fn run_visual_tests(project_path: PathBuf, update_baseline: bool) -> Result<()> None } }) - .ok() + .log_err() .flatten(); if let Some(task) = open_file_task { @@ -1018,7 +1018,7 @@ fn run_breakpoint_hover_visual_tests( None } }) - .ok() + .log_err() .flatten(); if let Some(task) = open_file_task { @@ -1588,7 +1588,7 @@ import { AiPaneTabContext } from 'context'; None } }) - .ok() + .log_err() .flatten(); if let Some(task) = open_file_task { @@ -2232,17 +2232,39 @@ fn run_agent_thread_view_test( /// Visual test for the Tool Permissions Settings UI page /// -/// Takes two screenshots: -/// 1. The settings page showing the "Configure Tool Rules" item -/// 2. The tool permissions sub-page after clicking Configure +/// Takes a screenshot showing the tool config page with matched patterns and verdict. #[cfg(target_os = "macos")] fn run_tool_permissions_visual_tests( app_state: Arc, cx: &mut VisualTestAppContext, _update_baseline: bool, ) -> Result { + use agent_settings::{AgentSettings, CompiledRegex, ToolPermissions, ToolRules}; + use collections::HashMap; + use settings::ToolPermissionMode; use zed_actions::OpenSettingsAt; + // Set up tool permissions with "hi" as both always_deny and always_allow for terminal + cx.update(|cx| { + let mut tools = HashMap::default(); + tools.insert( + Arc::from("terminal"), + ToolRules { + default: None, + always_allow: vec![CompiledRegex::new("hi", false).unwrap()], + always_deny: vec![CompiledRegex::new("hi", false).unwrap()], + always_confirm: vec![], + invalid_patterns: vec![], + }, + ); + let mut settings = AgentSettings::get_global(cx).clone(); + settings.tool_permissions = ToolPermissions { + default: ToolPermissionMode::Confirm, + tools, + }; + AgentSettings::override_global(settings, cx); + }); + // Create a minimal workspace to dispatch the settings action from let window_size = size(px(900.0), px(700.0)); let bounds = Bounds { @@ -2310,22 +2332,9 @@ fn run_tool_permissions_visual_tests( let all_windows = cx.update(|cx| cx.windows()); let settings_window = all_windows.last().copied().context("No windows found")?; - // Save screenshot 1: Settings page showing "Configure Tool Rules" item let output_dir = std::env::var("VISUAL_TEST_OUTPUT_DIR") .unwrap_or_else(|_| "target/visual_tests".to_string()); - std::fs::create_dir_all(&output_dir).ok(); - - cx.update_window(settings_window, |_, window, _cx| { - window.refresh(); - }) - .ok(); - cx.run_until_parked(); - - let output_path = PathBuf::from(&output_dir).join("tool_permissions_settings.png"); - if let Ok(screenshot) = cx.capture_screenshot(settings_window) { - let _: Result<(), _> = screenshot.save(&output_path); - println!("Screenshot 1 saved to: {}", output_path.display()); - } + std::fs::create_dir_all(&output_dir).log_err(); // Navigate to the tool permissions sub-page using the public API let settings_window_handle = settings_window @@ -2346,32 +2355,7 @@ fn run_tool_permissions_visual_tests( cx.run_until_parked(); } - // Refresh and redraw - cx.update_window(settings_window, |_, window, cx| { - window.draw(cx).clear(); - }) - .ok(); - cx.run_until_parked(); - - cx.update_window(settings_window, |_, window, _cx| { - window.refresh(); - }) - .ok(); - cx.run_until_parked(); - - // Save screenshot 2: The tool permissions sub-page (list of tools) - let subpage_output_path = PathBuf::from(&output_dir).join("tool_permissions_subpage.png"); - - if let Ok(screenshot) = cx.capture_screenshot(settings_window) { - let _: Result<(), _> = screenshot.save(&subpage_output_path); - println!( - "Screenshot 2 (tool list) saved to: {}", - subpage_output_path.display() - ); - } - // Now navigate into a specific tool (Terminal) to show the tool config page - // We need to use push_dynamic_sub_page since the tool pages are nested settings_window_handle .update(cx, |settings_window, window, cx| { settings_window.push_dynamic_sub_page( @@ -2393,40 +2377,71 @@ fn run_tool_permissions_visual_tests( cx.run_until_parked(); } + // Refresh and redraw so the "Test Your Rules" input is present + cx.update_window(settings_window, |_, window, cx| { + window.draw(cx).clear(); + }) + .log_err(); + cx.run_until_parked(); + + cx.update_window(settings_window, |_, window, _cx| { + window.refresh(); + }) + .log_err(); + cx.run_until_parked(); + + // Focus the first tab stop in the window (the "Test Your Rules" editor + // has tab_index(0) and tab_stop(true)) and type "hi" into it. + cx.update_window(settings_window, |_, window, cx| { + window.focus_next(cx); + }) + .log_err(); + cx.run_until_parked(); + + cx.simulate_input(settings_window, "hi"); + + // Let the UI update with the matched patterns + for _ in 0..5 { + cx.advance_clock(Duration::from_millis(50)); + cx.run_until_parked(); + } + // Refresh and redraw cx.update_window(settings_window, |_, window, cx| { window.draw(cx).clear(); }) - .ok(); + .log_err(); cx.run_until_parked(); cx.update_window(settings_window, |_, window, _cx| { window.refresh(); }) - .ok(); + .log_err(); cx.run_until_parked(); - // Save screenshot 3: Individual tool config page + // Save screenshot: Tool config page with "hi" typed and matched patterns visible let tool_config_output_path = - PathBuf::from(&output_dir).join("tool_permissions_tool_config.png"); + PathBuf::from(&output_dir).join("tool_permissions_test_rules.png"); if let Ok(screenshot) = cx.capture_screenshot(settings_window) { - let _: Result<(), _> = screenshot.save(&tool_config_output_path); + screenshot.save(&tool_config_output_path).log_err(); println!( - "Screenshot 3 (tool config) saved to: {}", + "Screenshot (test rules) saved to: {}", tool_config_output_path.display() ); } // Clean up - close the settings window - let _ = cx.update_window(settings_window, |_, window, _cx| { + cx.update_window(settings_window, |_, window, _cx| { window.remove_window(); - }); + }) + .log_err(); // Close the workspace window - let _ = cx.update_window(workspace_window.into(), |_, window, _cx| { + cx.update_window(workspace_window.into(), |_, window, _cx| { window.remove_window(); - }); + }) + .log_err(); cx.run_until_parked(); diff --git a/docs/src/SUMMARY.md b/docs/src/SUMMARY.md index 04e77eba7be126446d50b87521337bc830465e45..2b45c581685e9ecb63888edd256ec14b0da94a30 100644 --- a/docs/src/SUMMARY.md +++ b/docs/src/SUMMARY.md @@ -13,6 +13,7 @@ - [Overview](./ai/overview.md) - [Agent Panel](./ai/agent-panel.md) - [Tools](./ai/tools.md) + - [Tool Permissions](./ai/tool-permissions.md) - [External Agents](./ai/external-agents.md) - [Inline Assistant](./ai/inline-assistant.md) - [Edit Prediction](./ai/edit-prediction.md) diff --git a/docs/src/ai/agent-panel.md b/docs/src/ai/agent-panel.md index f6aeee100885bd18c2102c9b110a0d14a2de21f5..1f0e2b34f2c86f4fe4d269c42c77dca32cb94a8a 100644 --- a/docs/src/ai/agent-panel.md +++ b/docs/src/ai/agent-panel.md @@ -171,11 +171,21 @@ To delete a custom profile, open the Agent Profile modal, select the profile you ### Tool Approval -Zed's Agent Panel surfaces the `agent.always_allow_tool_actions` setting that, if turned to `false`, will require you to give permission to any editing attempt as well as tool calls coming from MCP servers. +> **Note:** In Zed v0.224.0 and above, tool approval is controlled by `agent.tool_permissions.default`. -You can change that by setting this key to `true` in either your `settings.json` or via the Agent Panel's settings view. +Zed's Agent Panel provides the `agent.tool_permissions.default` setting to control tool approval behavior: -You can also give more granular permissions through the dropdown that appears in the UI whenever the agent requests authorization to run a tool call. +- `"confirm"` (default) — Prompts for approval before running any tool action +- `"allow"` — Auto-approves tool actions without prompting +- `"deny"` — Blocks all tool actions + +You can change this in either your `settings.json` or via the Agent Panel's settings view. + +Even with `"default": "allow"`, per-tool `always_deny` and `always_confirm` patterns are still respected — so you can auto-approve most actions while blocking or gating specific ones. For the `copy_path` and `move_path` tools, patterns are matched independently against both the source and destination paths. See [Per-tool Permission Rules](./agent-settings.md#per-tool-permission-rules) for details. + +When the agent requests permission for an action, the confirmation dialog includes options to allow or deny once, plus "Always for " choices that set a tool-level default. When Zed can extract a safe pattern from the input, it also offers pattern-based "Always for ..." choices that add `always_allow`/`always_deny` rules. MCP tools only support tool-level defaults. + +> **Note:** Before Zed v0.224.0, tool approval was controlled by the `agent.always_allow_tool_actions` boolean (default `false`). Set it to `true` to auto-approve tool actions, or leave it `false` to require confirmation for edits and tool calls (including MCP tools). ### Model Support {#model-support} diff --git a/docs/src/ai/agent-settings.md b/docs/src/ai/agent-settings.md index 531742a0b8fe29f2714b8137f567563c185e41b9..db3195f2c7b0360ee4819091c2e27665399abace 100644 --- a/docs/src/ai/agent-settings.md +++ b/docs/src/ai/agent-settings.md @@ -68,7 +68,7 @@ Here's how you can customize your `settings.json` to add this functionality: "inline_alternatives": [ { "provider": "zed.dev", - "model": "gpt-4-mini" + "model": "gpt-5-mini" } ] } @@ -92,7 +92,7 @@ One with Claude Sonnet 4 (the default model), another with GPT-5-mini, and anoth "inline_alternatives": [ { "provider": "zed.dev", - "model": "gpt-4-mini" + "model": "gpt-5-mini" }, { "provider": "zed.dev", @@ -108,23 +108,27 @@ One with Claude Sonnet 4 (the default model), another with GPT-5-mini, and anoth Specify a custom temperature for a provider and/or model: ```json [settings] -"model_parameters": [ - // To set parameters for all requests to OpenAI models: - { - "provider": "openai", - "temperature": 0.5 - }, - // To set parameters for all requests in general: - { - "temperature": 0 - }, - // To set parameters for a specific provider and model: - { - "provider": "zed.dev", - "model": "claude-sonnet-4", - "temperature": 1.0 +{ + "agent": { + "model_parameters": [ + // To set parameters for all requests to OpenAI models: + { + "provider": "openai", + "temperature": 0.5 + }, + // To set parameters for all requests in general: + { + "temperature": 0 + }, + // To set parameters for a specific provider and model: + { + "provider": "zed.dev", + "model": "claude-sonnet-4", + "temperature": 1.0 + } + ] } -], +} ``` ## Agent Panel Settings {#agent-panel-settings} @@ -146,45 +150,143 @@ You can choose between `thread` (the default) and `text_thread`: ### Font Size -Use the `agent_font_size` setting to change the font size of rendered agent responses in the panel. +Use the `agent_ui_font_size` setting to change the font size of rendered agent responses in the panel. + +```json [settings] +{ + "agent_ui_font_size": 18 +} +``` + +> Editors in the Agent Panel—such as the main message textarea—use monospace fonts and are controlled by `agent_buffer_font_size` (which defaults to `buffer_font_size` when unset). + +### Default Tool Permissions + +> **Note:** In Zed v0.224.0 and above, tool approval uses the `agent.tool_permissions` settings described below. + +The `agent.tool_permissions.default` setting controls the baseline tool approval behavior for Zed's native agent: + +- `"confirm"` (default) — Prompts for approval before running any tool action +- `"allow"` — Auto-approves tool actions without prompting +- `"deny"` — Blocks all tool actions ```json [settings] { "agent": { - "agent_font_size": 18 + "tool_permissions": { + "default": "confirm" + } } } ``` -> Editors in the Agent Panel—whether that is the main message textarea or previous messages—use monospace fonts and therefore, are controlled by the `buffer_font_size` setting, which is defined globally in your `settings.json`. +Even with `"default": "allow"`, per-tool `always_deny` and `always_confirm` patterns are still respected, so you can auto-approve most actions while keeping guardrails on dangerous or sensitive ones. -### Auto-run Commands +### Per-tool Permission Rules {#per-tool-permission-rules} -Control whether to allow the agent to run commands without asking you for permission. -The default value is `false`. +For granular control over individual tool actions, use the `tools` key inside `tool_permissions` to configure regex-based rules that auto-approve, auto-deny, or always require confirmation for specific inputs. + +Each tool entry supports the following keys: + +- `default` — Fallback when no patterns match: `"confirm"`, `"allow"`, or `"deny"` +- `always_allow` — Array of patterns that auto-approve matching actions +- `always_deny` — Array of patterns that block matching actions immediately +- `always_confirm` — Array of patterns that always prompt for confirmation ```json [settings] { "agent": { - "always_allow_tool_actions": true + "tool_permissions": { + "default": "allow", + "tools": { + "terminal": { + "default": "confirm", + "always_allow": [ + { "pattern": "^cargo\\s+(build|test|check)" }, + { "pattern": "^git\\s+(status|log|diff)" } + ], + "always_deny": [{ "pattern": "rm\\s+-rf\\s+(/|~)" }], + "always_confirm": [{ "pattern": "sudo\\s" }] + }, + "edit_file": { + "always_deny": [ + { "pattern": "\\.env" }, + { "pattern": "\\.(pem|key)$" } + ] + } + } + } } } ``` +#### Pattern Precedence + +When evaluating a tool action, rules are checked in the following order (highest priority first): + +1. **Built-in security rules** — Hardcoded protections (e.g., `rm -rf /`) that cannot be overridden +2. **`always_deny`** — Blocks matching actions immediately +3. **`always_confirm`** — Requires confirmation for matching actions +4. **`always_allow`** — Auto-approves matching actions. For the terminal tool with chained commands (e.g., `echo hello && rm file`), **all** sub-commands must match an `always_allow` pattern +5. **Tool-specific `default`** — Per-tool fallback when no patterns match (e.g., `tools.terminal.default`) +6. **Global `default`** — Falls back to `tool_permissions.default` + +#### Case Sensitivity + +Patterns are **case-insensitive** by default. To make a pattern case-sensitive, set `case_sensitive` to `true`: + +```json [settings] +{ + "pattern": "^Makefile$", + "case_sensitive": true +} +``` + +#### `copy_path` and `move_path` Patterns + +For the `copy_path` and `move_path` tools, patterns are matched independently against both the source and destination paths. A `deny` or `confirm` match on **either** path takes effect. For `always_allow`, **both** paths must match for auto-approval. + +#### MCP Tool Permissions + +MCP tools use the key format `mcp::` in the `tools` configuration. For example: + +```json [settings] +{ + "agent": { + "tool_permissions": { + "tools": { + "mcp:github:create_issue": { + "default": "confirm" + }, + "mcp:github:create_pull_request": { + "default": "deny" + } + } + } + } +} +``` + +The `default` key on each MCP tool entry is the primary mechanism for controlling MCP tool permissions. Pattern-based rules (`always_allow`, `always_deny`, `always_confirm`) match against an empty string for MCP tools, so most patterns won't match — use the tool-level `default` instead. + +See the [Tool Permissions](./tool-permissions.md) documentation for more examples and complete details. + +> **Note:** Before Zed v0.224.0, tool approval was controlled by the `agent.always_allow_tool_actions` boolean (default `false`). Set it to `true` to auto-approve tool actions, or leave it `false` to require confirmation for edits and tool calls. + ### Single-file Review Control whether to display review actions (accept & reject) in single buffers after the agent is done performing edits. -The default value is `false`. +The default value is `true`. ```json [settings] { "agent": { - "single_file_review": true + "single_file_review": false } } ``` -When set to false, these controls are only available in the multibuffer review tab. +When set to `false`, these controls are only available in the multibuffer review tab. ### Sound Notification diff --git a/docs/src/ai/mcp.md b/docs/src/ai/mcp.md index e149225b81f567cf64a84c1c930d881bd6cbf290..fa5bc37b08fcf25adee426106685aa28b032f15d 100644 --- a/docs/src/ai/mcp.md +++ b/docs/src/ai/mcp.md @@ -130,9 +130,23 @@ As an example, [the Dagger team suggests](https://container-use.com/agent-integr ### Tool Approval -Zed's Agent Panel includes the `agent.always_allow_tool_actions` setting that, if set to `false`, will require you to give permission for any editing attempt as well as tool calls coming from MCP servers. +> **Note:** In Zed v0.224.0 and above, tool approval for the native Zed agent is controlled by `agent.tool_permissions.default`. -You can change this by setting this key to `true` in either your `settings.json` or through the Agent Panel's settings view. +Zed's Agent Panel provides the `agent.tool_permissions.default` setting to control tool approval behavior for the native Zed agent: + +- `"confirm"` (default) — Prompts for approval before running any tool action, including MCP tool calls +- `"allow"` — Auto-approves tool actions without prompting +- `"deny"` — Blocks all tool actions + +You can change this in either your `settings.json` or through the Agent Panel settings. + +Even with `"default": "allow"`, per-tool `always_deny` and `always_confirm` patterns are still respected, so you can auto-approve most actions while still blocking or gating sensitive ones. + +For granular control over specific MCP tools, you can configure per-tool permission rules. MCP tools use the key format `mcp::` — for example, `mcp:github:create_issue`. The `default` key on a per-tool entry is the primary mechanism for MCP tools, since pattern-based rules match against an empty string for MCP tools and most patterns won't match. + +See [Per-tool Permission Rules](./agent-settings.md#per-tool-permission-rules) and [Tool Permissions](./tool-permissions.md) for complete details. + +> **Note:** Before Zed v0.224.0, tool approval was controlled by the `agent.always_allow_tool_actions` boolean (default `false`). Set it to `true` to auto-approve tool actions, or leave it `false` to require confirmation for edits and MCP tool calls. ### External Agents diff --git a/docs/src/ai/overview.md b/docs/src/ai/overview.md index 05f954092df367e197629d667aa4b59c1418ce00..77906fc49c5962080db05fc3281652d2054b5e89 100644 --- a/docs/src/ai/overview.md +++ b/docs/src/ai/overview.md @@ -20,6 +20,8 @@ Zed integrates AI throughout the editor: agentic coding, inline transformations, - [Tools](./tools.md): The built-in capabilities agents use: file operations, terminal commands, web search. +- [Tool Permissions](./tool-permissions.md): Configure granular permission rules for agent tool actions. + - [Model Context Protocol](./mcp.md): Extend agents with custom tools via MCP servers. - [Inline Assistant](./inline-assistant.md): Transform selected code or terminal output with `ctrl-enter`. diff --git a/docs/src/ai/privacy-and-security.md b/docs/src/ai/privacy-and-security.md index c6bbdf4b8001cd0baf36063db0184b0fd8b41fbb..f8b72e230e2106c4eb3604b53f3c045b54e6b991 100644 --- a/docs/src/ai/privacy-and-security.md +++ b/docs/src/ai/privacy-and-security.md @@ -12,6 +12,8 @@ Zed, including AI features, works without sharing data with us and without authe ## Documentation +- [Tool Permissions](./tool-permissions.md): Configure granular rules to control which agent actions are auto-approved, blocked, or require confirmation. + - [Worktree trust](../worktree-trust.md): How Zed opens files and directories in restricted mode. - [Telemetry](../telemetry.md): How Zed collects general telemetry data. diff --git a/docs/src/ai/tool-permissions.md b/docs/src/ai/tool-permissions.md new file mode 100644 index 0000000000000000000000000000000000000000..34f3268233a12b8a5d959f1ec1566bf26aa88f74 --- /dev/null +++ b/docs/src/ai/tool-permissions.md @@ -0,0 +1,282 @@ +# Tool Permissions + +Configure which agent actions run automatically and which require your approval. + +> **Note:** In Zed v0.224.0 and above, this page documents the granular `agent.tool_permissions` system. +> +> **Note:** Before Zed v0.224.0, tool approval was controlled by the `agent.always_allow_tool_actions` boolean (default `false`). Set it to `true` to auto-approve tool actions, or leave it `false` to require confirmation. + +## Quick Start + +You can use Zed's Settings UI to configure tool permissions, or add rules directly to your `settings.json`: + +```json [settings] +{ + "agent": { + "tool_permissions": { + "default": "allow", + "tools": { + "terminal": { + "default": "confirm", + "always_allow": [ + { "pattern": "^cargo\\s+(build|test|check)" }, + { "pattern": "^npm\\s+(install|test|run)" } + ], + "always_confirm": [{ "pattern": "sudo\\s+/" }] + } + } + } + } +} +``` + +This example auto-approves `cargo` and `npm` commands in the terminal tool, while requiring manual confirmation on a case-by-case basis for `sudo` commands. Non-terminal commands follow the global `"default": "allow"` setting, but tool-specific defaults and `always_confirm` rules can still prompt. + +## How It Works + +The `tool_permissions` setting lets you customize tool permissions by specifying regex patterns that: + +- **Auto-approve** actions you trust +- **Auto-deny** dangerous actions (blocked even when `tool_permissions.default` is set to `"allow"`) +- **Always confirm** sensitive actions regardless of other settings + +## Supported Tools + +| Tool | Input Matched Against | +| ------------------------ | ---------------------------- | +| `terminal` | The shell command string | +| `edit_file` | The file path | +| `delete_path` | The path being deleted | +| `move_path` | Source and destination paths | +| `copy_path` | Source and destination paths | +| `create_directory` | The directory path | +| `restore_file_from_disk` | The file paths | +| `save_file` | The file paths | +| `fetch` | The URL | +| `web_search` | The search query | + +For MCP tools, use the format `mcp::`. For example, a tool called `create_issue` on a server called `github` would be `mcp:github:create_issue`. + +## Configuration + +```json [settings] +{ + "agent": { + "tool_permissions": { + "default": "confirm", + "tools": { + "": { + "default": "confirm", + "always_allow": [{ "pattern": "...", "case_sensitive": false }], + "always_deny": [{ "pattern": "...", "case_sensitive": false }], + "always_confirm": [{ "pattern": "...", "case_sensitive": false }] + } + } + } + } +} +``` + +### Options + +| Option | Description | +| ---------------- | ------------------------------------------------------------------------------ | +| `default` | Fallback when no patterns match: `"confirm"` (default), `"allow"`, or `"deny"` | +| `always_allow` | Patterns that auto-approve (unless deny or confirm also matches) | +| `always_deny` | Patterns that block immediately—highest priority, cannot be overridden | +| `always_confirm` | Patterns that always prompt, even when `tool_permissions.default` is `"allow"` | + +### Pattern Syntax + +```json [settings] +{ + "pattern": "your-regex-here", + "case_sensitive": false +} +``` + +Patterns use Rust regex syntax. Matching is case-insensitive by default. + +## Rule Precedence + +From highest to lowest priority: + +1. **Built-in security rules** — Hardcoded protections (e.g., `rm -rf /`). Cannot be overridden. +2. **`always_deny`** — Blocks matching actions +3. **`always_confirm`** — Requires confirmation for matching actions +4. **`always_allow`** — Auto-approves matching actions +5. **Tool-specific `default`** — Per-tool fallback when no patterns match (e.g., `tools.terminal.default`) +6. **Global `default`** — Falls back to `tool_permissions.default` when no tool-specific default is set + +## Examples + +### Terminal: Auto-Approve Build Commands + +```json [settings] +{ + "agent": { + "tool_permissions": { + "tools": { + "terminal": { + "default": "confirm", + "always_allow": [ + { "pattern": "^cargo\\s+(build|test|check|clippy|fmt)" }, + { "pattern": "^npm\\s+(install|test|run|build)" }, + { "pattern": "^git\\s+(status|log|diff|branch)" }, + { "pattern": "^ls\\b" }, + { "pattern": "^cat\\s" } + ], + "always_deny": [ + { "pattern": "rm\\s+-rf\\s+(/|~)" }, + { "pattern": "sudo\\s+rm" } + ], + "always_confirm": [ + { "pattern": "sudo\\s" }, + { "pattern": "git\\s+push" } + ] + } + } + } + } +} +``` + +### File Editing: Protect Sensitive Files + +```json [settings] +{ + "agent": { + "tool_permissions": { + "tools": { + "edit_file": { + "default": "confirm", + "always_allow": [ + { "pattern": "\\.(md|txt|json)$" }, + { "pattern": "^src/" } + ], + "always_deny": [ + { "pattern": "\\.env" }, + { "pattern": "secrets?/" }, + { "pattern": "\\.(pem|key)$" } + ] + } + } + } + } +} +``` + +### Path Deletion: Block Critical Directories + +```json [settings] +{ + "agent": { + "tool_permissions": { + "tools": { + "delete_path": { + "default": "confirm", + "always_deny": [ + { "pattern": "^/etc" }, + { "pattern": "^/usr" }, + { "pattern": "\\.git/?$" }, + { "pattern": "node_modules/?$" } + ] + } + } + } + } +} +``` + +### URL Fetching: Control External Access + +```json [settings] +{ + "agent": { + "tool_permissions": { + "tools": { + "fetch": { + "default": "confirm", + "always_allow": [ + { "pattern": "docs\\.rs" }, + { "pattern": "github\\.com" } + ], + "always_deny": [{ "pattern": "internal\\.company\\.com" }] + } + } + } + } +} +``` + +### MCP Tools + +```json [settings] +{ + "agent": { + "tool_permissions": { + "tools": { + "mcp:github:create_issue": { + "default": "confirm" + }, + "mcp:github:create_pull_request": { + "default": "confirm" + } + } + } + } +} +``` + +## Global Auto-Approve + +To auto-approve all tool actions: + +```json [settings] +{ + "agent": { + "tool_permissions": { + "default": "allow" + } + } +} +``` + +This bypasses confirmation prompts for most actions, but `always_deny`, `always_confirm`, built-in security rules, and paths inside Zed settings directories still prompt or block. + +## Shell Compatibility + +For the `terminal` tool, Zed parses chained commands (e.g., `echo hello && rm file`) to check each sub-command against your patterns. + +All supported shells work with tool permission patterns, including sh, bash, zsh, dash, fish, PowerShell 7+, pwsh, cmd, xonsh, csh, tcsh, Nushell, Elvish, and rc (Plan 9). + +## Writing Patterns + +- Use `\b` for word boundaries: `\brm\b` matches "rm" but not "storm" +- Use `^` and `$` to anchor patterns to start/end of input +- Escape special characters: `\.` for literal dot, `\\` for backslash +- Test carefully—a typo in a deny pattern blocks legitimate actions + +## Built-in Security Rules + +Zed includes a small set of hardcoded security rules that **cannot be overridden** by any setting. These only apply to the **terminal** tool and block recursive deletion of critical directories: + +- `rm -rf /` and `rm -rf /*` — filesystem root +- `rm -rf ~` and `rm -rf ~/*` — home directory +- `rm -rf $HOME` / `rm -rf ${HOME}` (and `$HOME/*`) — home directory via environment variable +- `rm -rf .` and `rm -rf ./*` — current directory +- `rm -rf ..` and `rm -rf ../*` — parent directory + +These patterns catch any flag combination (e.g., `-fr`, `-rfv`, `-r -f`, `--recursive --force`) and are case-insensitive. They are checked against both the raw command and each parsed sub-command in chained commands (e.g., `ls && rm -rf /`). + +There are no other built-in rules. The default settings file ({#action zed::OpenDefaultSettings}) includes commented-out examples for protecting `.env` files, secrets directories, and private keys — you can uncomment or adapt these to suit your needs. + +## UI Options + +When the agent requests permission, the dialog includes: + +- **Allow once** / **Deny once** — One-time decision +- **Always for ** — Sets a tool-level default to allow or deny +- **Always for ** — Adds an `always_allow` or `always_deny` pattern (when a safe pattern can be extracted) + +Selecting "Always for " sets `tools..default` to allow or deny. When a pattern can be safely extracted, selecting "Always for " adds an `always_allow` or `always_deny` rule for that input. MCP tools only support the tool-level option. diff --git a/docs/src/ai/tools.md b/docs/src/ai/tools.md index eb386b901d423c21f4af65c7cd1c8be4a5ba8153..5897cdc5b370b1d9a31b1513bc1f2e0298fd4bca 100644 --- a/docs/src/ai/tools.md +++ b/docs/src/ai/tools.md @@ -2,6 +2,8 @@ Zed's built-in agent has access to these tools for reading, searching, and editing your codebase. +You can configure permissions for tool actions, including situations where they are automatically approved, automatically denied, or require your confirmation on a case-by-case basis. See [Tool Permissions](./tool-permissions.md) for the list of permission-gated tools and details. + ## Read & Search Tools ### `diagnostics` @@ -64,18 +66,24 @@ Deletes a file or directory (including contents recursively) at the specified pa Edits files by replacing specific text with new content. -### `restore_file_from_disk` +### `move_path` -Reloads a file from disk, discarding any unsaved changes in the buffer. +Moves or renames a file or directory in the project, performing a rename if only the filename differs. -### `save_file` +### `restore_file_from_disk` -Saves a buffer's current contents to disk, preserving unsaved changes before the agent modifies the file. +Discards unsaved changes in open buffers by reloading file contents from disk. Useful for resetting files to their on-disk state before retrying an edit. -### `move_path` +### `save_file` -Moves or renames a file or directory in the project, performing a rename if only the filename differs. +Saves files that have unsaved changes. Used when files need to be saved before further edits can be made. ### `terminal` Executes shell commands and returns the combined output, creating a new shell process for each invocation. + +## Other Tools + +### `subagent` + +Spawns a subagent with its own context window to perform a delegated task. Useful for running parallel investigations, completing self-contained tasks, or performing research where only the outcome matters. Each subagent has access to the same tools as the parent agent.