From 2b6e935b09e4367557157f09c659b87089d70b57 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Mon, 12 Jan 2026 11:08:06 -0500 Subject: [PATCH] Add remaining tool permissions (#46164) Add permission checking to the remaining 7 tools that require granular permissions: - `edit_file`: Checks path against permission rules - `delete_path`: Checks path against permission rules - `move_path`: Checks both source and destination paths - `create_directory`: Checks path against permission rules - `save_file`: Checks all paths, denies if any are blocked - `fetch`: Checks URL against permission rules - `web_search`: Checks query against permission rules Each tool follows the pattern established in PR #46155 (terminal tool): - `Allow` = proceed without prompting - `Deny` = return error immediately - `Confirm` = prompt user for confirmation The deny > confirm > allow precedence is enforced by the `decide_permission_from_settings()` function. Release Notes: - N/A --------- Co-authored-by: Amp --- crates/agent/src/tests/mod.rs | 579 ++++++++++++++++++ crates/agent/src/tools/copy_path_tool.rs | 35 +- .../agent/src/tools/create_directory_tool.rs | 24 +- crates/agent/src/tools/delete_path_tool.rs | 25 +- crates/agent/src/tools/edit_file_tool.rs | 15 +- crates/agent/src/tools/fetch_tool.rs | 25 +- crates/agent/src/tools/move_path_tool.rs | 35 +- crates/agent/src/tools/save_file_tool.rs | 61 +- crates/agent/src/tools/terminal_tool.rs | 2 +- crates/agent/src/tools/web_search_tool.rs | 25 +- 10 files changed, 812 insertions(+), 14 deletions(-) diff --git a/crates/agent/src/tests/mod.rs b/crates/agent/src/tests/mod.rs index 31d6c194b4325c5d8e68bc55c17ac8b1b4c7f2b5..3ed5afb38c3e23ad38a4f0bf84cb6b1b4e63ac6b 100644 --- a/crates/agent/src/tests/mod.rs +++ b/crates/agent/src/tests/mod.rs @@ -21,6 +21,7 @@ use gpui::{ http_client::FakeHttpClient, }; use indoc::indoc; + use language_model::{ LanguageModel, LanguageModelCompletionError, LanguageModelCompletionEvent, LanguageModelId, LanguageModelProviderName, LanguageModelRegistry, LanguageModelRequest, @@ -3925,3 +3926,581 @@ async fn test_terminal_tool_permission_rules(cx: &mut TestAppContext) { ); } } + +#[gpui::test] +async fn test_edit_file_tool_deny_rule_blocks_edit(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree("/root", json!({"sensitive_config.txt": "secret data"})) + .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.tools.insert( + "edit_file".into(), + agent_settings::ToolRules { + default_mode: settings::ToolPermissionMode::Allow, + always_allow: vec![], + always_deny: vec![agent_settings::CompiledRegex::new(r"sensitive", false).unwrap()], + always_confirm: vec![], + invalid_patterns: vec![], + }, + ); + 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.clone(), + thread.downgrade(), + language_registry, + templates, + )); + let (event_stream, _rx) = crate::ToolCallEventStream::test(); + + let task = cx.update(|cx| { + tool.run( + crate::EditFileToolInput { + display_description: "Edit sensitive file".to_string(), + path: "root/sensitive_config.txt".into(), + mode: crate::EditFileMode::Edit, + }, + event_stream, + cx, + ) + }); + + let result = task.await; + assert!(result.is_err(), "expected edit to be blocked"); + assert!( + result.unwrap_err().to_string().contains("blocked"), + "error should mention the edit was blocked" + ); +} + +#[gpui::test] +async fn test_delete_path_tool_deny_rule_blocks_deletion(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree("/root", json!({"important_data.txt": "critical info"})) + .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.tools.insert( + "delete_path".into(), + agent_settings::ToolRules { + default_mode: settings::ToolPermissionMode::Allow, + always_allow: vec![], + always_deny: vec![agent_settings::CompiledRegex::new(r"important", false).unwrap()], + always_confirm: vec![], + invalid_patterns: vec![], + }, + ); + agent_settings::AgentSettings::override_global(settings, cx); + }); + + let action_log = cx.new(|_cx| action_log::ActionLog::new(project.clone())); + + #[allow(clippy::arc_with_non_send_sync)] + let tool = Arc::new(crate::DeletePathTool::new(project, action_log)); + let (event_stream, _rx) = crate::ToolCallEventStream::test(); + + let task = cx.update(|cx| { + tool.run( + crate::DeletePathToolInput { + path: "root/important_data.txt".to_string(), + }, + event_stream, + cx, + ) + }); + + let result = task.await; + assert!(result.is_err(), "expected deletion to be blocked"); + assert!( + result.unwrap_err().to_string().contains("blocked"), + "error should mention the deletion was blocked" + ); +} + +#[gpui::test] +async fn test_move_path_tool_denies_if_destination_denied(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + "/root", + json!({ + "safe.txt": "content", + "protected": {} + }), + ) + .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.tools.insert( + "move_path".into(), + agent_settings::ToolRules { + default_mode: settings::ToolPermissionMode::Allow, + always_allow: vec![], + always_deny: vec![agent_settings::CompiledRegex::new(r"protected", false).unwrap()], + always_confirm: vec![], + invalid_patterns: vec![], + }, + ); + agent_settings::AgentSettings::override_global(settings, cx); + }); + + #[allow(clippy::arc_with_non_send_sync)] + let tool = Arc::new(crate::MovePathTool::new(project)); + let (event_stream, _rx) = crate::ToolCallEventStream::test(); + + let task = cx.update(|cx| { + tool.run( + crate::MovePathToolInput { + source_path: "root/safe.txt".to_string(), + destination_path: "root/protected/safe.txt".to_string(), + }, + event_stream, + cx, + ) + }); + + let result = task.await; + assert!( + result.is_err(), + "expected move to be blocked due to destination path" + ); + assert!( + result.unwrap_err().to_string().contains("blocked"), + "error should mention the move was blocked" + ); +} + +#[gpui::test] +async fn test_move_path_tool_denies_if_source_denied(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + "/root", + json!({ + "secret.txt": "secret content", + "public": {} + }), + ) + .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.tools.insert( + "move_path".into(), + agent_settings::ToolRules { + default_mode: settings::ToolPermissionMode::Allow, + always_allow: vec![], + always_deny: vec![agent_settings::CompiledRegex::new(r"secret", false).unwrap()], + always_confirm: vec![], + invalid_patterns: vec![], + }, + ); + agent_settings::AgentSettings::override_global(settings, cx); + }); + + #[allow(clippy::arc_with_non_send_sync)] + let tool = Arc::new(crate::MovePathTool::new(project)); + let (event_stream, _rx) = crate::ToolCallEventStream::test(); + + let task = cx.update(|cx| { + tool.run( + crate::MovePathToolInput { + source_path: "root/secret.txt".to_string(), + destination_path: "root/public/not_secret.txt".to_string(), + }, + event_stream, + cx, + ) + }); + + let result = task.await; + assert!( + result.is_err(), + "expected move to be blocked due to source path" + ); + assert!( + result.unwrap_err().to_string().contains("blocked"), + "error should mention the move was blocked" + ); +} + +#[gpui::test] +async fn test_copy_path_tool_deny_rule_blocks_copy(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + "/root", + json!({ + "confidential.txt": "confidential data", + "dest": {} + }), + ) + .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.tools.insert( + "copy_path".into(), + agent_settings::ToolRules { + default_mode: settings::ToolPermissionMode::Allow, + always_allow: vec![], + always_deny: vec![ + agent_settings::CompiledRegex::new(r"confidential", false).unwrap(), + ], + always_confirm: vec![], + invalid_patterns: vec![], + }, + ); + agent_settings::AgentSettings::override_global(settings, cx); + }); + + #[allow(clippy::arc_with_non_send_sync)] + let tool = Arc::new(crate::CopyPathTool::new(project)); + let (event_stream, _rx) = crate::ToolCallEventStream::test(); + + let task = cx.update(|cx| { + tool.run( + crate::CopyPathToolInput { + source_path: "root/confidential.txt".to_string(), + destination_path: "root/dest/copy.txt".to_string(), + }, + event_stream, + cx, + ) + }); + + let result = task.await; + assert!(result.is_err(), "expected copy to be blocked"); + assert!( + result.unwrap_err().to_string().contains("blocked"), + "error should mention the copy was blocked" + ); +} + +#[gpui::test] +async fn test_save_file_tool_denies_if_any_path_denied(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + "/root", + json!({ + "normal.txt": "normal content", + "readonly": { + "config.txt": "readonly content" + } + }), + ) + .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.tools.insert( + "save_file".into(), + agent_settings::ToolRules { + default_mode: settings::ToolPermissionMode::Allow, + always_allow: vec![], + always_deny: vec![agent_settings::CompiledRegex::new(r"readonly", false).unwrap()], + always_confirm: vec![], + invalid_patterns: vec![], + }, + ); + agent_settings::AgentSettings::override_global(settings, cx); + }); + + #[allow(clippy::arc_with_non_send_sync)] + let tool = Arc::new(crate::SaveFileTool::new(project)); + let (event_stream, _rx) = crate::ToolCallEventStream::test(); + + let task = cx.update(|cx| { + tool.run( + crate::SaveFileToolInput { + paths: vec![ + std::path::PathBuf::from("root/normal.txt"), + std::path::PathBuf::from("root/readonly/config.txt"), + ], + }, + event_stream, + cx, + ) + }); + + let result = task.await; + assert!( + result.is_err(), + "expected save to be blocked due to denied path" + ); + assert!( + result.unwrap_err().to_string().contains("blocked"), + "error should mention the save was blocked" + ); +} + +#[gpui::test] +async fn test_save_file_tool_respects_deny_rules(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree("/root", json!({"config.secret": "secret config"})) + .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.always_allow_tool_actions = false; + settings.tool_permissions.tools.insert( + "save_file".into(), + agent_settings::ToolRules { + default_mode: settings::ToolPermissionMode::Allow, + always_allow: vec![], + always_deny: vec![agent_settings::CompiledRegex::new(r"\.secret$", false).unwrap()], + always_confirm: vec![], + invalid_patterns: vec![], + }, + ); + agent_settings::AgentSettings::override_global(settings, cx); + }); + + #[allow(clippy::arc_with_non_send_sync)] + let tool = Arc::new(crate::SaveFileTool::new(project)); + let (event_stream, _rx) = crate::ToolCallEventStream::test(); + + let task = cx.update(|cx| { + tool.run( + crate::SaveFileToolInput { + paths: vec![std::path::PathBuf::from("root/config.secret")], + }, + event_stream, + cx, + ) + }); + + let result = task.await; + assert!(result.is_err(), "expected save to be blocked"); + assert!( + result.unwrap_err().to_string().contains("blocked"), + "error should mention the save was blocked" + ); +} + +#[gpui::test] +async fn test_web_search_tool_deny_rule_blocks_search(cx: &mut TestAppContext) { + init_test(cx); + + cx.update(|cx| { + let mut settings = agent_settings::AgentSettings::get_global(cx).clone(); + settings.tool_permissions.tools.insert( + "web_search".into(), + agent_settings::ToolRules { + default_mode: settings::ToolPermissionMode::Allow, + always_allow: vec![], + always_deny: vec![ + agent_settings::CompiledRegex::new(r"internal\.company", false).unwrap(), + ], + always_confirm: vec![], + invalid_patterns: vec![], + }, + ); + agent_settings::AgentSettings::override_global(settings, cx); + }); + + #[allow(clippy::arc_with_non_send_sync)] + let tool = Arc::new(crate::WebSearchTool); + let (event_stream, _rx) = crate::ToolCallEventStream::test(); + + let input: crate::WebSearchToolInput = + serde_json::from_value(json!({"query": "internal.company.com secrets"})).unwrap(); + + let task = cx.update(|cx| tool.run(input, event_stream, cx)); + + let result = task.await; + assert!(result.is_err(), "expected search to be blocked"); + assert!( + result.unwrap_err().to_string().contains("blocked"), + "error should mention the search was blocked" + ); +} + +#[gpui::test] +async fn test_edit_file_tool_allow_rule_skips_confirmation(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree("/root", 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.always_allow_tool_actions = false; + settings.tool_permissions.tools.insert( + "edit_file".into(), + agent_settings::ToolRules { + default_mode: settings::ToolPermissionMode::Confirm, + always_allow: vec![agent_settings::CompiledRegex::new(r"\.md$", false).unwrap()], + always_deny: vec![], + always_confirm: vec![], + invalid_patterns: vec![], + }, + ); + 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, + )); + let (event_stream, mut rx) = crate::ToolCallEventStream::test(); + + let _task = cx.update(|cx| { + tool.run( + crate::EditFileToolInput { + display_description: "Edit README".to_string(), + path: "root/README.md".into(), + mode: crate::EditFileMode::Edit, + }, + event_stream, + cx, + ) + }); + + cx.run_until_parked(); + + let event = rx.try_next(); + assert!( + !matches!(event, Ok(Some(Ok(ThreadEvent::ToolCallAuthorization(_))))), + "expected no authorization request for allowed .md file" + ); +} + +#[gpui::test] +async fn test_fetch_tool_deny_rule_blocks_url(cx: &mut TestAppContext) { + init_test(cx); + + cx.update(|cx| { + let mut settings = agent_settings::AgentSettings::get_global(cx).clone(); + settings.tool_permissions.tools.insert( + "fetch".into(), + agent_settings::ToolRules { + default_mode: settings::ToolPermissionMode::Allow, + always_allow: vec![], + always_deny: vec![ + agent_settings::CompiledRegex::new(r"internal\.company\.com", false).unwrap(), + ], + always_confirm: vec![], + invalid_patterns: vec![], + }, + ); + agent_settings::AgentSettings::override_global(settings, cx); + }); + + let http_client = gpui::http_client::FakeHttpClient::with_200_response(); + + #[allow(clippy::arc_with_non_send_sync)] + let tool = Arc::new(crate::FetchTool::new(http_client)); + let (event_stream, _rx) = crate::ToolCallEventStream::test(); + + let input: crate::FetchToolInput = + serde_json::from_value(json!({"url": "https://internal.company.com/api"})).unwrap(); + + let task = cx.update(|cx| tool.run(input, event_stream, cx)); + + let result = task.await; + assert!(result.is_err(), "expected fetch to be blocked"); + assert!( + result.unwrap_err().to_string().contains("blocked"), + "error should mention the fetch was blocked" + ); +} + +#[gpui::test] +async fn test_fetch_tool_allow_rule_skips_confirmation(cx: &mut TestAppContext) { + init_test(cx); + + cx.update(|cx| { + let mut settings = agent_settings::AgentSettings::get_global(cx).clone(); + settings.always_allow_tool_actions = false; + settings.tool_permissions.tools.insert( + "fetch".into(), + agent_settings::ToolRules { + default_mode: settings::ToolPermissionMode::Confirm, + always_allow: vec![agent_settings::CompiledRegex::new(r"docs\.rs", false).unwrap()], + always_deny: vec![], + always_confirm: vec![], + invalid_patterns: vec![], + }, + ); + agent_settings::AgentSettings::override_global(settings, cx); + }); + + let http_client = gpui::http_client::FakeHttpClient::with_200_response(); + + #[allow(clippy::arc_with_non_send_sync)] + let tool = Arc::new(crate::FetchTool::new(http_client)); + let (event_stream, mut rx) = crate::ToolCallEventStream::test(); + + let input: crate::FetchToolInput = + serde_json::from_value(json!({"url": "https://docs.rs/some-crate"})).unwrap(); + + let _task = cx.update(|cx| tool.run(input, event_stream, cx)); + + cx.run_until_parked(); + + let event = rx.try_next(); + assert!( + !matches!(event, Ok(Some(Ok(ThreadEvent::ToolCallAuthorization(_))))), + "expected no authorization request for allowed docs.rs URL" + ); +} diff --git a/crates/agent/src/tools/copy_path_tool.rs b/crates/agent/src/tools/copy_path_tool.rs index e1fdded7125ec847bbed77e898838a97b3dc2271..694d5d0582b9e6828286cf6207d6232a48bb0b29 100644 --- a/crates/agent/src/tools/copy_path_tool.rs +++ b/crates/agent/src/tools/copy_path_tool.rs @@ -1,11 +1,15 @@ -use crate::{AgentTool, ToolCallEventStream}; +use crate::{ + AgentTool, ToolCallEventStream, ToolPermissionDecision, decide_permission_from_settings, +}; use agent_client_protocol::ToolKind; +use agent_settings::AgentSettings; use anyhow::{Context as _, Result, anyhow}; use futures::FutureExt as _; use gpui::{App, AppContext, Entity, Task}; use project::Project; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; +use settings::Settings; use std::sync::Arc; use util::markdown::MarkdownInlineCode; @@ -79,6 +83,31 @@ impl AgentTool for CopyPathTool { event_stream: ToolCallEventStream, cx: &mut App, ) -> Task> { + let settings = AgentSettings::get_global(cx); + + let source_decision = + decide_permission_from_settings(Self::name(), &input.source_path, settings); + if let ToolPermissionDecision::Deny(reason) = source_decision { + return Task::ready(Err(anyhow!("{}", reason))); + } + + let dest_decision = + decide_permission_from_settings(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); + + let authorize = if needs_confirmation { + let src = MarkdownInlineCode(&input.source_path); + let dest = MarkdownInlineCode(&input.destination_path); + Some(event_stream.authorize(format!("Copy {src} to {dest}"), cx)) + } else { + None + }; + let copy_task = self.project.update(cx, |project, cx| { match project .find_project_path(&input.source_path, cx) @@ -99,6 +128,10 @@ impl AgentTool for CopyPathTool { }); cx.background_spawn(async move { + if let Some(authorize) = authorize { + authorize.await?; + } + let result = futures::select! { result = copy_task.fuse() => result, _ = event_stream.cancelled_by_user().fuse() => { diff --git a/crates/agent/src/tools/create_directory_tool.rs b/crates/agent/src/tools/create_directory_tool.rs index 6ef6566679f0e5dd2f2efb702054d03284115893..bc31bed42196ea4788c7f6d29154d4f546083bce 100644 --- a/crates/agent/src/tools/create_directory_tool.rs +++ b/crates/agent/src/tools/create_directory_tool.rs @@ -1,14 +1,18 @@ use agent_client_protocol::ToolKind; +use agent_settings::AgentSettings; use anyhow::{Context as _, Result, anyhow}; use futures::FutureExt as _; use gpui::{App, Entity, SharedString, Task}; use project::Project; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; +use settings::Settings; use std::sync::Arc; use util::markdown::MarkdownInlineCode; -use crate::{AgentTool, ToolCallEventStream}; +use crate::{ + AgentTool, ToolCallEventStream, ToolPermissionDecision, decide_permission_from_settings, +}; /// Creates a new directory at the specified path within the project. Returns confirmation that the directory was created. /// @@ -68,6 +72,20 @@ impl AgentTool for CreateDirectoryTool { event_stream: ToolCallEventStream, cx: &mut App, ) -> Task> { + let settings = AgentSettings::get_global(cx); + let decision = decide_permission_from_settings(Self::name(), &input.path, settings); + + let authorize = match decision { + ToolPermissionDecision::Allow => None, + ToolPermissionDecision::Deny(reason) => { + return Task::ready(Err(anyhow!("{}", reason))); + } + ToolPermissionDecision::Confirm => Some(event_stream.authorize( + format!("Create directory {}", MarkdownInlineCode(&input.path)), + cx, + )), + }; + let project_path = match self.project.read(cx).find_project_path(&input.path, cx) { Some(project_path) => project_path, None => { @@ -81,6 +99,10 @@ impl AgentTool for CreateDirectoryTool { }); cx.spawn(async move |_cx| { + if let Some(authorize) = authorize { + authorize.await?; + } + futures::select! { result = create_entry.fuse() => { result.with_context(|| format!("Creating directory {destination_path}"))?; diff --git a/crates/agent/src/tools/delete_path_tool.rs b/crates/agent/src/tools/delete_path_tool.rs index 3ec7ad7a7fc3604e6ef1bfc53d5f00f624dc98df..c3fc1cc72d8eae22969ae72dc1bd39f791cf0362 100644 --- a/crates/agent/src/tools/delete_path_tool.rs +++ b/crates/agent/src/tools/delete_path_tool.rs @@ -1,13 +1,18 @@ -use crate::{AgentTool, ToolCallEventStream}; +use crate::{ + AgentTool, ToolCallEventStream, ToolPermissionDecision, decide_permission_from_settings, +}; use action_log::ActionLog; use agent_client_protocol::ToolKind; +use agent_settings::AgentSettings; use anyhow::{Context as _, Result, anyhow}; use futures::{FutureExt as _, SinkExt, StreamExt, channel::mpsc}; use gpui::{App, AppContext, Entity, SharedString, Task}; use project::{Project, ProjectPath}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; +use settings::Settings; use std::sync::Arc; +use util::markdown::MarkdownInlineCode; /// Deletes the file or directory (and the directory's contents, recursively) at the specified path in the project, and returns confirmation of the deletion. #[derive(Debug, Serialize, Deserialize, JsonSchema)] @@ -71,6 +76,20 @@ impl AgentTool for DeletePathTool { cx: &mut App, ) -> Task> { let path = input.path; + + let settings = AgentSettings::get_global(cx); + let decision = decide_permission_from_settings(Self::name(), &path, settings); + + let authorize = match decision { + ToolPermissionDecision::Allow => None, + ToolPermissionDecision::Deny(reason) => { + return Task::ready(Err(anyhow!("{}", reason))); + } + ToolPermissionDecision::Confirm => { + Some(event_stream.authorize(format!("Delete {}", MarkdownInlineCode(&path)), cx)) + } + }; + let Some(project_path) = self.project.read(cx).find_project_path(&path, cx) else { return Task::ready(Err(anyhow!( "Couldn't delete {path} because that path isn't in this project." @@ -113,6 +132,10 @@ impl AgentTool for DeletePathTool { let project = self.project.clone(); let action_log = self.action_log.clone(); cx.spawn(async move |cx| { + if let Some(authorize) = authorize { + authorize.await?; + } + loop { let path_result = futures::select! { path = paths_rx.next().fuse() => path, diff --git a/crates/agent/src/tools/edit_file_tool.rs b/crates/agent/src/tools/edit_file_tool.rs index 20e624efa1cf5f56365f36ef1d152773f7e9cbe7..68d5c0b6e121277e25f8ccfa36c42b30f8926296 100644 --- a/crates/agent/src/tools/edit_file_tool.rs +++ b/crates/agent/src/tools/edit_file_tool.rs @@ -1,5 +1,6 @@ use crate::{ - AgentTool, Templates, Thread, ToolCallEventStream, + AgentTool, Templates, Thread, ToolCallEventStream, ToolPermissionDecision, + decide_permission_from_settings, edit_agent::{EditAgent, EditAgentOutput, EditAgentOutputEvent, EditFormat}, }; use acp_thread::Diff; @@ -149,8 +150,16 @@ impl EditFileTool { event_stream: &ToolCallEventStream, cx: &mut App, ) -> Task> { - if agent_settings::AgentSettings::get_global(cx).always_allow_tool_actions { - return Task::ready(Ok(())); + let path_str = input.path.to_string_lossy(); + let settings = agent_settings::AgentSettings::get_global(cx); + let decision = decide_permission_from_settings(Self::name(), &path_str, settings); + + match decision { + ToolPermissionDecision::Allow => return Task::ready(Ok(())), + ToolPermissionDecision::Deny(reason) => { + return Task::ready(Err(anyhow!("{}", reason))); + } + ToolPermissionDecision::Confirm => {} } // If any path component matches the local settings folder, then this could affect diff --git a/crates/agent/src/tools/fetch_tool.rs b/crates/agent/src/tools/fetch_tool.rs index a62d24863563d77f4a4140eceb19a31c9da7c8a2..e52ae4bd241df3d8c252c9fbed1872505ac2c0e3 100644 --- a/crates/agent/src/tools/fetch_tool.rs +++ b/crates/agent/src/tools/fetch_tool.rs @@ -3,6 +3,7 @@ use std::sync::Arc; use std::{borrow::Cow, cell::RefCell}; use agent_client_protocol as acp; +use agent_settings::AgentSettings; use anyhow::{Context as _, Result, bail}; use futures::{AsyncReadExt as _, FutureExt as _}; use gpui::{App, AppContext as _, Task}; @@ -10,10 +11,13 @@ use html_to_markdown::{TagHandler, convert_html_to_markdown, markdown}; use http_client::{AsyncBody, HttpClientWithUrl}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; +use settings::Settings; use ui::SharedString; -use util::markdown::MarkdownEscaped; +use util::markdown::{MarkdownEscaped, MarkdownInlineCode}; -use crate::{AgentTool, ToolCallEventStream}; +use crate::{ + AgentTool, ToolCallEventStream, ToolPermissionDecision, decide_permission_from_settings, +}; #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Copy)] enum ContentType { @@ -143,12 +147,25 @@ impl AgentTool for FetchTool { event_stream: ToolCallEventStream, cx: &mut App, ) -> Task> { - let authorize = event_stream.authorize(input.url.clone(), cx); + let settings = AgentSettings::get_global(cx); + let decision = decide_permission_from_settings(Self::name(), &input.url, settings); + + let authorize = match decision { + ToolPermissionDecision::Allow => None, + ToolPermissionDecision::Deny(reason) => { + return Task::ready(Err(anyhow::anyhow!("{}", reason))); + } + ToolPermissionDecision::Confirm => Some( + event_stream.authorize(format!("Fetch {}", MarkdownInlineCode(&input.url)), cx), + ), + }; let fetch_task = cx.background_spawn({ let http_client = self.http_client.clone(); async move { - authorize.await?; + if let Some(authorize) = authorize { + authorize.await?; + } Self::build_message(http_client, &input.url).await } }); diff --git a/crates/agent/src/tools/move_path_tool.rs b/crates/agent/src/tools/move_path_tool.rs index 8b340089e8b2941c45c7da376b181eaf7eaa2b66..9ec91a5350dbe550596c5c6ec5f5e27408cb5b6b 100644 --- a/crates/agent/src/tools/move_path_tool.rs +++ b/crates/agent/src/tools/move_path_tool.rs @@ -1,11 +1,15 @@ -use crate::{AgentTool, ToolCallEventStream}; +use crate::{ + AgentTool, ToolCallEventStream, ToolPermissionDecision, decide_permission_from_settings, +}; use agent_client_protocol::ToolKind; +use agent_settings::AgentSettings; use anyhow::{Context as _, Result, anyhow}; use futures::FutureExt as _; use gpui::{App, AppContext, Entity, SharedString, Task}; use project::Project; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; +use settings::Settings; use std::{path::Path, sync::Arc}; use util::markdown::MarkdownInlineCode; @@ -93,6 +97,31 @@ impl AgentTool for MovePathTool { event_stream: ToolCallEventStream, cx: &mut App, ) -> Task> { + let settings = AgentSettings::get_global(cx); + + let source_decision = + decide_permission_from_settings(Self::name(), &input.source_path, settings); + if let ToolPermissionDecision::Deny(reason) = source_decision { + return Task::ready(Err(anyhow!("{}", reason))); + } + + let dest_decision = + decide_permission_from_settings(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); + + let authorize = if needs_confirmation { + let src = MarkdownInlineCode(&input.source_path); + let dest = MarkdownInlineCode(&input.destination_path); + Some(event_stream.authorize(format!("Move {src} to {dest}"), cx)) + } else { + None + }; + let rename_task = self.project.update(cx, |project, cx| { match project .find_project_path(&input.source_path, cx) @@ -113,6 +142,10 @@ impl AgentTool for MovePathTool { }); cx.background_spawn(async move { + if let Some(authorize) = authorize { + authorize.await?; + } + let result = futures::select! { result = rename_task.fuse() => result, _ = event_stream.cancelled_by_user().fuse() => { diff --git a/crates/agent/src/tools/save_file_tool.rs b/crates/agent/src/tools/save_file_tool.rs index 384675bd32728f22effcc7a97ae61ec19bb49d37..1265b2f5c28b007574cbe495ad5884a21faf6670 100644 --- a/crates/agent/src/tools/save_file_tool.rs +++ b/crates/agent/src/tools/save_file_tool.rs @@ -1,4 +1,5 @@ use agent_client_protocol as acp; +use agent_settings::AgentSettings; use anyhow::Result; use collections::FxHashSet; use futures::FutureExt as _; @@ -7,10 +8,14 @@ use language::Buffer; use project::Project; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; +use settings::Settings; use std::path::PathBuf; use std::sync::Arc; +use util::markdown::MarkdownInlineCode; -use crate::{AgentTool, ToolCallEventStream}; +use crate::{ + AgentTool, ToolCallEventStream, ToolPermissionDecision, decide_permission_from_settings, +}; /// Saves files that have unsaved changes. /// @@ -62,10 +67,59 @@ impl AgentTool for SaveFileTool { event_stream: ToolCallEventStream, cx: &mut App, ) -> Task> { + let settings = AgentSettings::get_global(cx); + let mut needs_confirmation = false; + + for path in &input.paths { + let path_str = path.to_string_lossy(); + let decision = decide_permission_from_settings(Self::name(), &path_str, settings); + match decision { + ToolPermissionDecision::Allow => {} + ToolPermissionDecision::Deny(reason) => { + return Task::ready(Err(anyhow::anyhow!("{}", reason))); + } + ToolPermissionDecision::Confirm => { + needs_confirmation = true; + } + } + } + + let authorize = if needs_confirmation { + let title = if input.paths.len() == 1 { + format!( + "Save {}", + MarkdownInlineCode(&input.paths[0].to_string_lossy()) + ) + } else { + let paths: Vec<_> = input + .paths + .iter() + .take(3) + .map(|p| p.to_string_lossy().to_string()) + .collect(); + if input.paths.len() > 3 { + format!( + "Save {}, and {} more", + paths.join(", "), + input.paths.len() - 3 + ) + } else { + format!("Save {}", paths.join(", ")) + } + }; + Some(event_stream.authorize(title, 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_save: FxHashSet> = FxHashSet::default(); let mut saved_paths: Vec = Vec::new(); @@ -196,6 +250,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.always_allow_tool_actions = true; + AgentSettings::override_global(settings, cx); + }); } #[gpui::test] diff --git a/crates/agent/src/tools/terminal_tool.rs b/crates/agent/src/tools/terminal_tool.rs index 09d92452abc165c60d2e4dece88fe6167b8615e0..2cc6e96dd9c034a014cbe140cc457902aa32d8fd 100644 --- a/crates/agent/src/tools/terminal_tool.rs +++ b/crates/agent/src/tools/terminal_tool.rs @@ -112,7 +112,7 @@ impl AgentTool for TerminalTool { }; let settings = AgentSettings::get_global(cx); - let decision = decide_permission_from_settings("terminal", &input.command, settings); + let decision = decide_permission_from_settings(Self::name(), &input.command, settings); let authorize = match decision { ToolPermissionDecision::Allow => None, diff --git a/crates/agent/src/tools/web_search_tool.rs b/crates/agent/src/tools/web_search_tool.rs index 3d3998cf000783f5fe347432873cc9579dc14f2f..26d073415d77dc396c8bd2a24b203a7a46099a79 100644 --- a/crates/agent/src/tools/web_search_tool.rs +++ b/crates/agent/src/tools/web_search_tool.rs @@ -1,7 +1,10 @@ use std::sync::Arc; -use crate::{AgentTool, ToolCallEventStream}; +use crate::{ + AgentTool, ToolCallEventStream, ToolPermissionDecision, decide_permission_from_settings, +}; use agent_client_protocol as acp; +use agent_settings::AgentSettings; use anyhow::{Result, anyhow}; use cloud_llm_client::WebSearchResponse; use futures::FutureExt as _; @@ -11,7 +14,9 @@ use language_model::{ }; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; +use settings::Settings; use ui::prelude::*; +use util::markdown::MarkdownInlineCode; use web_search::WebSearchRegistry; /// Search the web for information using your query. @@ -68,12 +73,30 @@ impl AgentTool for WebSearchTool { event_stream: ToolCallEventStream, cx: &mut App, ) -> Task> { + let settings = AgentSettings::get_global(cx); + let decision = decide_permission_from_settings(Self::name(), &input.query, settings); + + let authorize = match decision { + ToolPermissionDecision::Allow => None, + ToolPermissionDecision::Deny(reason) => { + return Task::ready(Err(anyhow!("{}", reason))); + } + ToolPermissionDecision::Confirm => Some(event_stream.authorize( + format!("Search the web for {}", MarkdownInlineCode(&input.query)), + cx, + )), + }; + let Some(provider) = WebSearchRegistry::read_global(cx).active_provider() else { return Task::ready(Err(anyhow!("Web search is not available."))); }; let search_task = provider.search(input.query, cx); cx.background_spawn(async move { + if let Some(authorize) = authorize { + authorize.await?; + } + let response = futures::select! { result = search_task.fuse() => { match result {