Detailed changes
@@ -1,6 +1,12 @@
use crate::shell_parser::extract_commands;
+use std::path::{Path, PathBuf};
use url::Url;
+/// Normalize path separators to forward slashes for consistent cross-platform patterns.
+fn normalize_separators(path_str: &str) -> String {
+ path_str.replace('\\', "/")
+}
+
/// Extracts the command name from a shell command using the shell parser.
///
/// This parses the command properly to extract just the command name (first word),
@@ -41,23 +47,64 @@ pub fn extract_terminal_pattern_display(command: &str) -> Option<String> {
}
pub fn extract_path_pattern(path: &str) -> Option<String> {
- let parent = std::path::Path::new(path).parent()?;
- let parent_str = parent.to_str()?;
+ let parent = Path::new(path).parent()?;
+ let parent_str = normalize_separators(parent.to_str()?);
if parent_str.is_empty() || parent_str == "/" {
return None;
}
- Some(format!("^{}/", regex::escape(parent_str)))
+ Some(format!("^{}/", regex::escape(&parent_str)))
}
pub fn extract_path_pattern_display(path: &str) -> Option<String> {
- let parent = std::path::Path::new(path).parent()?;
- let parent_str = parent.to_str()?;
+ let parent = Path::new(path).parent()?;
+ let parent_str = normalize_separators(parent.to_str()?);
if parent_str.is_empty() || parent_str == "/" {
return None;
}
Some(format!("{}/", parent_str))
}
+fn common_parent_dir(path_a: &str, path_b: &str) -> Option<PathBuf> {
+ let parent_a = Path::new(path_a).parent()?;
+ let parent_b = Path::new(path_b).parent()?;
+
+ let components_a: Vec<_> = parent_a.components().collect();
+ let components_b: Vec<_> = parent_b.components().collect();
+
+ let common_count = components_a
+ .iter()
+ .zip(components_b.iter())
+ .take_while(|(a, b)| a == b)
+ .count();
+
+ if common_count == 0 {
+ return None;
+ }
+
+ let common: PathBuf = components_a[..common_count].iter().collect();
+ Some(common)
+}
+
+pub fn extract_copy_move_pattern(input: &str) -> Option<String> {
+ let (source, dest) = input.split_once('\n')?;
+ let common = common_parent_dir(source, dest)?;
+ let common_str = normalize_separators(common.to_str()?);
+ if common_str.is_empty() || common_str == "/" {
+ return None;
+ }
+ Some(format!("^{}/", regex::escape(&common_str)))
+}
+
+pub fn extract_copy_move_pattern_display(input: &str) -> Option<String> {
+ let (source, dest) = input.split_once('\n')?;
+ let common = common_parent_dir(source, dest)?;
+ let common_str = normalize_separators(common.to_str()?);
+ if common_str.is_empty() || common_str == "/" {
+ return None;
+ }
+ Some(format!("{}/", common_str))
+}
+
pub fn extract_url_pattern(url: &str) -> Option<String> {
let parsed = Url::parse(url).ok()?;
let domain = parsed.host_str()?;
@@ -170,4 +217,56 @@ mod tests {
Some("^https?://test\\.example\\.com".to_string())
);
}
+
+ #[test]
+ fn test_extract_copy_move_pattern_same_directory() {
+ assert_eq!(
+ extract_copy_move_pattern(
+ "/Users/alice/project/src/old.rs\n/Users/alice/project/src/new.rs"
+ ),
+ Some("^/Users/alice/project/src/".to_string())
+ );
+ }
+
+ #[test]
+ fn test_extract_copy_move_pattern_sibling_directories() {
+ assert_eq!(
+ extract_copy_move_pattern(
+ "/Users/alice/project/src/old.rs\n/Users/alice/project/dst/new.rs"
+ ),
+ Some("^/Users/alice/project/".to_string())
+ );
+ }
+
+ #[test]
+ fn test_extract_copy_move_pattern_no_common_prefix() {
+ assert_eq!(
+ extract_copy_move_pattern("/home/file.txt\n/tmp/file.txt"),
+ None
+ );
+ }
+
+ #[test]
+ fn test_extract_copy_move_pattern_relative_paths() {
+ assert_eq!(
+ extract_copy_move_pattern("src/old.rs\nsrc/new.rs"),
+ Some("^src/".to_string())
+ );
+ }
+
+ #[test]
+ fn test_extract_copy_move_pattern_display() {
+ assert_eq!(
+ extract_copy_move_pattern_display(
+ "/Users/alice/project/src/old.rs\n/Users/alice/project/dst/new.rs"
+ ),
+ Some("/Users/alice/project/".to_string())
+ );
+ }
+
+ #[test]
+ fn test_extract_copy_move_pattern_no_arrow() {
+ assert_eq!(extract_copy_move_pattern("just/a/path.rs"), None);
+ assert_eq!(extract_copy_move_pattern_display("just/a/path.rs"), None);
+ }
}
@@ -675,9 +675,16 @@ impl ToolPermissionContext {
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 == MovePathTool::NAME
|| tool_name == CreateDirectoryTool::NAME
|| tool_name == SaveFileTool::NAME
{
@@ -1,15 +1,17 @@
-use crate::{
- AgentTool, ToolCallEventStream, ToolPermissionDecision, decide_permission_from_settings,
+use super::edit_file_tool::{
+ SensitiveSettingsKind, is_sensitive_settings_path, sensitive_settings_kind,
};
+use crate::{AgentTool, ToolCallEventStream, ToolPermissionDecision, decide_permission_for_path};
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 gpui::{App, Entity, Task};
use project::Project;
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
use settings::Settings;
+use std::path::Path;
use std::sync::Arc;
use util::markdown::MarkdownInlineCode;
@@ -83,57 +85,71 @@ impl AgentTool for CopyPathTool {
) -> Task<Result<Self::Output>> {
let settings = AgentSettings::get_global(cx);
- let source_decision =
- decide_permission_from_settings(Self::NAME, &input.source_path, settings);
+ let source_decision = decide_permission_for_path(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);
+ 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);
+ || 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 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: input.source_path.clone(),
+ input_value: format!("{}\n{}", input.source_path, input.destination_path),
};
- Some(event_stream.authorize(format!("Copy {src} to {dest}"), context, cx))
+ let title = format!("Copy {src} to {dest}");
+ let sensitive_kind = sensitive_settings_kind(Path::new(&input.source_path))
+ .or_else(|| sensitive_settings_kind(Path::new(&input.destination_path)));
+ let title = match sensitive_kind {
+ Some(SensitiveSettingsKind::Local) => format!("{title} (local settings)"),
+ Some(SensitiveSettingsKind::Global) => format!("{title} (settings)"),
+ None => title,
+ };
+ Some(event_stream.authorize(title, context, cx))
} else {
None
};
- let copy_task = self.project.update(cx, |project, cx| {
- match project
- .find_project_path(&input.source_path, cx)
- .and_then(|project_path| project.entry_for_path(&project_path, cx))
- {
- Some(entity) => match project.find_project_path(&input.destination_path, cx) {
- Some(project_path) => project.copy_entry(entity.id, project_path, cx),
- None => Task::ready(Err(anyhow!(
- "Destination path {} was outside the project.",
- input.destination_path
- ))),
- },
- None => Task::ready(Err(anyhow!(
- "Source path {} was not found in the project.",
- input.source_path
- ))),
- }
- });
-
- cx.background_spawn(async move {
+ let project = self.project.clone();
+ cx.spawn(async move |cx| {
if let Some(authorize) = authorize {
authorize.await?;
}
+ let copy_task = project.update(cx, |project, cx| {
+ match project
+ .find_project_path(&input.source_path, cx)
+ .and_then(|project_path| project.entry_for_path(&project_path, cx))
+ {
+ Some(entity) => match project.find_project_path(&input.destination_path, cx) {
+ Some(project_path) => Ok(project.copy_entry(entity.id, project_path, cx)),
+ None => Err(anyhow!(
+ "Destination path {} was outside the project.",
+ input.destination_path
+ )),
+ },
+ None => Err(anyhow!(
+ "Source path {} was not found in the project.",
+ input.source_path
+ )),
+ }
+ })?;
+
let result = futures::select! {
result = copy_task.fuse() => result,
_ = event_stream.cancelled_by_user().fuse() => {
@@ -1,3 +1,4 @@
+use super::edit_file_tool::{SensitiveSettingsKind, sensitive_settings_kind};
use agent_client_protocol::ToolKind;
use agent_settings::AgentSettings;
use anyhow::{Context as _, Result, anyhow};
@@ -7,12 +8,11 @@ use project::Project;
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
use settings::Settings;
+use std::path::Path;
use std::sync::Arc;
use util::markdown::MarkdownInlineCode;
-use crate::{
- AgentTool, ToolCallEventStream, ToolPermissionDecision, decide_permission_from_settings,
-};
+use crate::{AgentTool, ToolCallEventStream, ToolPermissionDecision, decide_permission_for_path};
/// Creates a new directory at the specified path within the project. Returns confirmation that the directory was created.
///
@@ -71,7 +71,15 @@ impl AgentTool for CreateDirectoryTool {
cx: &mut App,
) -> Task<Result<Self::Output>> {
let settings = AgentSettings::get_global(cx);
- let decision = decide_permission_from_settings(Self::NAME, &input.path, settings);
+ 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()
+ {
+ decision = ToolPermissionDecision::Confirm;
+ }
let authorize = match decision {
ToolPermissionDecision::Allow => None,
@@ -79,35 +87,35 @@ impl AgentTool for CreateDirectoryTool {
return Task::ready(Err(anyhow!("{}", reason)));
}
ToolPermissionDecision::Confirm => {
+ let title = format!("Create directory {}", MarkdownInlineCode(&input.path));
+ 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_value: input.path.clone(),
};
- Some(event_stream.authorize(
- format!("Create directory {}", MarkdownInlineCode(&input.path)),
- context,
- cx,
- ))
+ Some(event_stream.authorize(title, context, cx))
}
};
- let project_path = match self.project.read(cx).find_project_path(&input.path, cx) {
- Some(project_path) => project_path,
- None => {
- return Task::ready(Err(anyhow!("Path to create was outside the project")));
- }
- };
let destination_path: Arc<str> = input.path.as_str().into();
- let create_entry = self.project.update(cx, |project, cx| {
- project.create_entry(project_path.clone(), true, cx)
- });
-
- cx.spawn(async move |_cx| {
+ let project = self.project.clone();
+ cx.spawn(async move |cx| {
if let Some(authorize) = authorize {
authorize.await?;
}
+ let create_entry = project.update(cx, |project, cx| {
+ match project.find_project_path(&input.path, cx) {
+ Some(project_path) => Ok(project.create_entry(project_path, true, cx)),
+ None => Err(anyhow!("Path to create was outside the project")),
+ }
+ })?;
+
futures::select! {
result = create_entry.fuse() => {
result.with_context(|| format!("Creating directory {destination_path}"))?;
@@ -1,6 +1,7 @@
-use crate::{
- AgentTool, ToolCallEventStream, ToolPermissionDecision, decide_permission_from_settings,
+use super::edit_file_tool::{
+ SensitiveSettingsKind, is_sensitive_settings_path, sensitive_settings_kind,
};
+use crate::{AgentTool, ToolCallEventStream, ToolPermissionDecision, decide_permission_for_path};
use action_log::ActionLog;
use agent_client_protocol::ToolKind;
use agent_settings::AgentSettings;
@@ -11,6 +12,7 @@ use project::{Project, ProjectPath};
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
use settings::Settings;
+use std::path::Path;
use std::sync::Arc;
use util::markdown::MarkdownInlineCode;
@@ -76,7 +78,14 @@ impl AgentTool for DeletePathTool {
let path = input.path;
let settings = AgentSettings::get_global(cx);
- let decision = decide_permission_from_settings(Self::NAME, &path, settings);
+ 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;
+ }
let authorize = match decision {
ToolPermissionDecision::Allow => None,
@@ -88,53 +97,16 @@ impl AgentTool for DeletePathTool {
tool_name: Self::NAME.to_string(),
input_value: path.clone(),
};
- Some(event_stream.authorize(
- format!("Delete {}", MarkdownInlineCode(&path)),
- context,
- cx,
- ))
+ let title = format!("Delete {}", MarkdownInlineCode(&path));
+ let title = match sensitive_settings_kind(Path::new(&path)) {
+ Some(SensitiveSettingsKind::Local) => format!("{title} (local settings)"),
+ Some(SensitiveSettingsKind::Global) => format!("{title} (settings)"),
+ None => title,
+ };
+ Some(event_stream.authorize(title, context, 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."
- )));
- };
-
- let Some(worktree) = self
- .project
- .read(cx)
- .worktree_for_id(project_path.worktree_id, cx)
- else {
- return Task::ready(Err(anyhow!(
- "Couldn't delete {path} because that path isn't in this project."
- )));
- };
-
- let worktree_snapshot = worktree.read(cx).snapshot();
- let (mut paths_tx, mut paths_rx) = mpsc::channel(256);
- cx.background_spawn({
- let project_path = project_path.clone();
- async move {
- for entry in
- worktree_snapshot.traverse_from_path(true, false, false, &project_path.path)
- {
- if !entry.path.starts_with(&project_path.path) {
- break;
- }
- paths_tx
- .send(ProjectPath {
- worktree_id: project_path.worktree_id,
- path: entry.path.clone(),
- })
- .await?;
- }
- anyhow::Ok(())
- }
- })
- .detach();
-
let project = self.project.clone();
let action_log = self.action_log.clone();
cx.spawn(async move |cx| {
@@ -142,6 +114,41 @@ impl AgentTool for DeletePathTool {
authorize.await?;
}
+ let (project_path, worktree_snapshot) = project.read_with(cx, |project, cx| {
+ let project_path = project.find_project_path(&path, cx).ok_or_else(|| {
+ anyhow!("Couldn't delete {path} because that path isn't in this project.")
+ })?;
+ let worktree = project
+ .worktree_for_id(project_path.worktree_id, cx)
+ .ok_or_else(|| {
+ anyhow!("Couldn't delete {path} because that path isn't in this project.")
+ })?;
+ let worktree_snapshot = worktree.read(cx).snapshot();
+ anyhow::Ok((project_path, worktree_snapshot))
+ })?;
+
+ let (mut paths_tx, mut paths_rx) = mpsc::channel(256);
+ cx.background_spawn({
+ let project_path = project_path.clone();
+ async move {
+ for entry in
+ worktree_snapshot.traverse_from_path(true, false, false, &project_path.path)
+ {
+ if !entry.path.starts_with(&project_path.path) {
+ break;
+ }
+ paths_tx
+ .send(ProjectPath {
+ worktree_id: project_path.worktree_id,
+ path: entry.path.clone(),
+ })
+ .await?;
+ }
+ anyhow::Ok(())
+ }
+ })
+ .detach();
+
loop {
let path_result = futures::select! {
path = paths_rx.next().fuse() => path,
@@ -2,7 +2,7 @@ use super::restore_file_from_disk_tool::RestoreFileFromDiskTool;
use super::save_file_tool::SaveFileTool;
use crate::{
AgentTool, Templates, Thread, ToolCallEventStream, ToolPermissionDecision,
- decide_permission_from_settings,
+ decide_permission_for_path,
edit_agent::{EditAgent, EditAgentOutput, EditAgentOutputEvent, EditFormat},
};
use acp_thread::Diff;
@@ -161,72 +161,154 @@ impl EditFileTool {
event_stream: &ToolCallEventStream,
cx: &mut App,
) -> Task<Result<()>> {
- 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)));
+ authorize_file_edit(
+ Self::NAME,
+ &input.path,
+ &input.display_description,
+ &self.thread,
+ event_stream,
+ cx,
+ )
+ }
+}
+
+pub enum SensitiveSettingsKind {
+ Local,
+ Global,
+}
+
+/// Canonicalize a path, stripping the Windows extended-length path prefix (`\\?\`)
+/// that `std::fs::canonicalize` adds on Windows. This ensures that canonicalized
+/// paths can be compared with non-canonicalized paths via `starts_with`.
+fn safe_canonicalize(path: &Path) -> std::io::Result<PathBuf> {
+ let canonical = std::fs::canonicalize(path)?;
+ #[cfg(target_os = "windows")]
+ {
+ let s = canonical.to_string_lossy();
+ if let Some(stripped) = s.strip_prefix("\\\\?\\") {
+ return Ok(PathBuf::from(stripped));
+ }
+ }
+ Ok(canonical)
+}
+
+/// Returns the kind of sensitive settings location this path targets, if any:
+/// either inside a `.zed/` local-settings directory or inside the global config dir.
+pub fn sensitive_settings_kind(path: &Path) -> Option<SensitiveSettingsKind> {
+ let local_settings_folder = paths::local_settings_folder_name();
+ if path.components().any(|component| {
+ component.as_os_str() == <_ as AsRef<OsStr>>::as_ref(&local_settings_folder)
+ }) {
+ return Some(SensitiveSettingsKind::Local);
+ }
+
+ // Walk up the path hierarchy until we find an ancestor that exists and can
+ // be canonicalized, then reconstruct the path from there. This handles
+ // cases where multiple levels of subdirectories don't exist yet (e.g.
+ // ~/.config/zed/new_subdir/evil.json).
+ let canonical_path = {
+ let mut current: Option<&Path> = Some(path);
+ let mut suffix_components = Vec::new();
+ loop {
+ match current {
+ Some(ancestor) => match safe_canonicalize(ancestor) {
+ Ok(canonical) => {
+ let mut result = canonical;
+ for component in suffix_components.into_iter().rev() {
+ result.push(component);
+ }
+ break Some(result);
+ }
+ Err(_) => {
+ if let Some(file_name) = ancestor.file_name() {
+ suffix_components.push(file_name.to_os_string());
+ }
+ current = ancestor.parent();
+ }
+ },
+ None => break None,
}
- ToolPermissionDecision::Confirm => {}
}
+ };
+ if let Some(canonical_path) = canonical_path {
+ let config_dir = safe_canonicalize(paths::config_dir())
+ .unwrap_or_else(|_| paths::config_dir().to_path_buf());
+ if canonical_path.starts_with(&config_dir) {
+ return Some(SensitiveSettingsKind::Global);
+ }
+ }
+
+ None
+}
+
+pub fn is_sensitive_settings_path(path: &Path) -> bool {
+ sensitive_settings_kind(path).is_some()
+}
+
+pub fn authorize_file_edit(
+ tool_name: &str,
+ path: &Path,
+ display_description: &str,
+ thread: &WeakEntity<Thread>,
+ event_stream: &ToolCallEventStream,
+ cx: &mut App,
+) -> Task<Result<()>> {
+ let path_str = path.to_string_lossy();
+ let settings = agent_settings::AgentSettings::get_global(cx);
+ let decision = decide_permission_for_path(tool_name, &path_str, settings);
+
+ if let ToolPermissionDecision::Deny(reason) = decision {
+ return Task::ready(Err(anyhow!("{}", reason)));
+ }
- // If any path component matches the local settings folder, then this could affect
- // the editor in ways beyond the project source, so prompt.
- let local_settings_folder = paths::local_settings_folder_name();
- let path = Path::new(&input.path);
- if path.components().any(|component| {
- component.as_os_str() == <_ as AsRef<OsStr>>::as_ref(&local_settings_folder)
- }) {
+ let explicitly_allowed = matches!(decision, ToolPermissionDecision::Allow);
+
+ if explicitly_allowed
+ && (settings.always_allow_tool_actions || !is_sensitive_settings_path(path))
+ {
+ return Task::ready(Ok(()));
+ }
+
+ match sensitive_settings_kind(path) {
+ Some(SensitiveSettingsKind::Local) => {
let context = crate::ToolPermissionContext {
- tool_name: Self::NAME.to_string(),
+ tool_name: tool_name.to_string(),
input_value: path_str.to_string(),
};
return event_stream.authorize(
- format!("{} (local settings)", input.display_description),
+ format!("{} (local settings)", display_description),
context,
cx,
);
}
-
- // It's also possible that the global config dir is configured to be inside the project,
- // so check for that edge case too.
- // TODO this is broken when remoting
- if let Ok(canonical_path) = std::fs::canonicalize(&input.path)
- && canonical_path.starts_with(paths::config_dir())
- {
+ Some(SensitiveSettingsKind::Global) => {
let context = crate::ToolPermissionContext {
- tool_name: Self::NAME.to_string(),
+ tool_name: tool_name.to_string(),
input_value: path_str.to_string(),
};
return event_stream.authorize(
- format!("{} (global settings)", input.display_description),
+ format!("{} (settings)", display_description),
context,
cx,
);
}
+ None => {}
+ }
- // Check if path is inside the global config directory
- // First check if it's already inside project - if not, try to canonicalize
- let Ok(project_path) = self.thread.read_with(cx, |thread, cx| {
- thread.project().read(cx).find_project_path(&input.path, cx)
- }) else {
- return Task::ready(Err(anyhow!("thread was dropped")));
+ let Ok(project_path) = thread.read_with(cx, |thread, cx| {
+ thread.project().read(cx).find_project_path(path, cx)
+ }) else {
+ return Task::ready(Err(anyhow!("thread was dropped")));
+ };
+
+ if project_path.is_some() {
+ Task::ready(Ok(()))
+ } else {
+ let context = crate::ToolPermissionContext {
+ tool_name: tool_name.to_string(),
+ input_value: path_str.to_string(),
};
-
- // If the path is inside the project, and it's not one of the above edge cases,
- // then no confirmation is necessary. Otherwise, confirmation is necessary.
- if project_path.is_some() {
- Task::ready(Ok(()))
- } else {
- let context = crate::ToolPermissionContext {
- tool_name: Self::NAME.to_string(),
- input_value: path_str.to_string(),
- };
- event_stream.authorize(&input.display_description, context, cx)
- }
+ event_stream.authorize(display_description, context, cx)
}
}
@@ -2283,4 +2365,42 @@ mod tests {
error_msg
);
}
+
+ #[test]
+ fn test_sensitive_settings_kind_detects_nonexistent_subdirectory() {
+ let config_dir = paths::config_dir();
+ let path = config_dir.join("nonexistent_subdir_xyz").join("evil.json");
+ assert!(
+ matches!(
+ sensitive_settings_kind(&path),
+ Some(SensitiveSettingsKind::Global)
+ ),
+ "Path in non-existent subdirectory of config dir should be detected as sensitive: {:?}",
+ path
+ );
+ }
+
+ #[test]
+ fn test_sensitive_settings_kind_detects_deeply_nested_nonexistent_subdirectory() {
+ let config_dir = paths::config_dir();
+ let path = config_dir.join("a").join("b").join("c").join("evil.json");
+ assert!(
+ matches!(
+ sensitive_settings_kind(&path),
+ Some(SensitiveSettingsKind::Global)
+ ),
+ "Path in deeply nested non-existent subdirectory of config dir should be detected as sensitive: {:?}",
+ path
+ );
+ }
+
+ #[test]
+ fn test_sensitive_settings_kind_returns_none_for_non_config_path() {
+ let path = PathBuf::from("/tmp/not_a_config_dir/some_file.json");
+ assert!(
+ sensitive_settings_kind(&path).is_none(),
+ "Path outside config dir should not be detected as sensitive: {:?}",
+ path
+ );
+ }
}
@@ -1,11 +1,12 @@
-use crate::{
- AgentTool, ToolCallEventStream, ToolPermissionDecision, decide_permission_from_settings,
+use super::edit_file_tool::{
+ SensitiveSettingsKind, is_sensitive_settings_path, sensitive_settings_kind,
};
+use crate::{AgentTool, ToolCallEventStream, ToolPermissionDecision, decide_permission_for_path};
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 gpui::{App, Entity, SharedString, Task};
use project::Project;
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
@@ -97,57 +98,71 @@ impl AgentTool for MovePathTool {
) -> Task<Result<Self::Output>> {
let settings = AgentSettings::get_global(cx);
- let source_decision =
- decide_permission_from_settings(Self::NAME, &input.source_path, settings);
+ let source_decision = decide_permission_for_path(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);
+ 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);
+ || 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 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: input.source_path.clone(),
+ input_value: format!("{}\n{}", input.source_path, input.destination_path),
};
- Some(event_stream.authorize(format!("Move {src} to {dest}"), context, cx))
+ let title = format!("Move {src} to {dest}");
+ let settings_kind = sensitive_settings_kind(Path::new(&input.source_path))
+ .or_else(|| sensitive_settings_kind(Path::new(&input.destination_path)));
+ let title = match settings_kind {
+ Some(SensitiveSettingsKind::Local) => format!("{title} (local settings)"),
+ Some(SensitiveSettingsKind::Global) => format!("{title} (settings)"),
+ None => title,
+ };
+ Some(event_stream.authorize(title, context, cx))
} else {
None
};
- let rename_task = self.project.update(cx, |project, cx| {
- match project
- .find_project_path(&input.source_path, cx)
- .and_then(|project_path| project.entry_for_path(&project_path, cx))
- {
- Some(entity) => match project.find_project_path(&input.destination_path, cx) {
- Some(project_path) => project.rename_entry(entity.id, project_path, cx),
- None => Task::ready(Err(anyhow!(
- "Destination path {} was outside the project.",
- input.destination_path
- ))),
- },
- None => Task::ready(Err(anyhow!(
- "Source path {} was not found in the project.",
- input.source_path
- ))),
- }
- });
-
- cx.background_spawn(async move {
+ let project = self.project.clone();
+ cx.spawn(async move |cx| {
if let Some(authorize) = authorize {
authorize.await?;
}
+ let rename_task = project.update(cx, |project, cx| {
+ match project
+ .find_project_path(&input.source_path, cx)
+ .and_then(|project_path| project.entry_for_path(&project_path, cx))
+ {
+ Some(entity) => match project.find_project_path(&input.destination_path, cx) {
+ Some(project_path) => Ok(project.rename_entry(entity.id, project_path, cx)),
+ None => Err(anyhow!(
+ "Destination path {} was outside the project.",
+ input.destination_path
+ )),
+ },
+ None => Err(anyhow!(
+ "Source path {} was not found in the project.",
+ input.source_path
+ )),
+ }
+ })?;
+
let result = futures::select! {
result = rename_task.fuse() => result,
_ = event_stream.cancelled_by_user().fuse() => {
@@ -9,13 +9,32 @@ use project::Project;
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
use settings::Settings;
-use std::path::PathBuf;
+use std::path::{Component, Path, PathBuf};
use std::sync::Arc;
use util::markdown::MarkdownInlineCode;
-use crate::{
- AgentTool, ToolCallEventStream, ToolPermissionDecision, decide_permission_from_settings,
+use super::edit_file_tool::{
+ SensitiveSettingsKind, is_sensitive_settings_path, sensitive_settings_kind,
};
+use crate::{AgentTool, ToolCallEventStream, ToolPermissionDecision, decide_permission_for_path};
+
+fn common_parent_for_paths(paths: &[String]) -> Option<PathBuf> {
+ let first = paths.first()?;
+ let mut common: Vec<Component<'_>> = Path::new(first).parent()?.components().collect();
+ for path in &paths[1..] {
+ let parent: Vec<Component<'_>> = 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.
///
@@ -66,53 +85,65 @@ impl AgentTool for SaveFileTool {
cx: &mut App,
) -> Task<Result<String>> {
let settings = AgentSettings::get_global(cx);
- let mut needs_confirmation = false;
+ let mut confirmation_paths: Vec<String> = Vec::new();
for path in &input.paths {
let path_str = path.to_string_lossy();
- let decision = decide_permission_from_settings(Self::NAME, &path_str, settings);
+ let decision = decide_permission_for_path(Self::NAME, &path_str, settings);
match decision {
- ToolPermissionDecision::Allow => {}
+ ToolPermissionDecision::Allow => {
+ if !settings.always_allow_tool_actions
+ && 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 => {
- needs_confirmation = true;
+ confirmation_paths.push(path_str.to_string());
}
}
}
- let authorize = if needs_confirmation {
- let title = if input.paths.len() == 1 {
- format!(
- "Save {}",
- MarkdownInlineCode(&input.paths[0].to_string_lossy())
- )
+ let authorize = if !confirmation_paths.is_empty() {
+ let title = if confirmation_paths.len() == 1 {
+ format!("Save {}", MarkdownInlineCode(&confirmation_paths[0]))
} else {
- let paths: Vec<_> = input
- .paths
+ let paths: Vec<_> = confirmation_paths
.iter()
.take(3)
- .map(|p| p.to_string_lossy().to_string())
+ .map(|p| p.as_str())
.collect();
- if input.paths.len() > 3 {
+ if confirmation_paths.len() > 3 {
format!(
"Save {}, and {} more",
paths.join(", "),
- input.paths.len() - 3
+ confirmation_paths.len() - 3
)
} else {
format!("Save {}", paths.join(", "))
}
};
- let first_path = input
- .paths
- .first()
- .map(|p| p.to_string_lossy().to_string())
- .unwrap_or_default();
+ 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 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: first_path,
+ input_value,
};
Some(event_stream.authorize(title, context, cx))
} else {
@@ -2,8 +2,8 @@ use super::edit_file_tool::EditFileTool;
use super::restore_file_from_disk_tool::RestoreFileFromDiskTool;
use super::save_file_tool::SaveFileTool;
use crate::{
- AgentTool, Templates, Thread, ToolCallEventStream, ToolPermissionDecision,
- decide_permission_from_settings, edit_agent::streaming_fuzzy_matcher::StreamingFuzzyMatcher,
+ AgentTool, Templates, Thread, ToolCallEventStream,
+ edit_agent::streaming_fuzzy_matcher::StreamingFuzzyMatcher,
};
use acp_thread::Diff;
use agent_client_protocol::{self as acp, ToolCallLocation, ToolCallUpdateFields};
@@ -11,14 +11,11 @@ use anyhow::{Context as _, Result, anyhow};
use gpui::{App, AppContext, AsyncApp, Entity, Task, WeakEntity};
use language::LanguageRegistry;
use language_model::LanguageModelToolResultContent;
-use paths;
use project::{Project, ProjectPath};
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
-use settings::Settings;
-use std::ffi::OsStr;
use std::ops::Range;
-use std::path::{Path, PathBuf};
+use std::path::PathBuf;
use std::sync::Arc;
use text::BufferSnapshot;
use ui::SharedString;
@@ -169,63 +166,14 @@ impl StreamingEditFileTool {
event_stream: &ToolCallEventStream,
cx: &mut App,
) -> Task<Result<()>> {
- 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 => {}
- }
-
- let local_settings_folder = paths::local_settings_folder_name();
- let path = Path::new(&input.path);
- if path.components().any(|component| {
- component.as_os_str() == <_ as AsRef<OsStr>>::as_ref(&local_settings_folder)
- }) {
- let context = crate::ToolPermissionContext {
- tool_name: EditFileTool::NAME.to_string(),
- input_value: path_str.to_string(),
- };
- return event_stream.authorize(
- format!("{} (local settings)", input.display_description),
- context,
- cx,
- );
- }
-
- if let Ok(canonical_path) = std::fs::canonicalize(&input.path)
- && canonical_path.starts_with(paths::config_dir())
- {
- let context = crate::ToolPermissionContext {
- tool_name: EditFileTool::NAME.to_string(),
- input_value: path_str.to_string(),
- };
- return event_stream.authorize(
- format!("{} (global settings)", input.display_description),
- context,
- cx,
- );
- }
-
- let Ok(project_path) = self.thread.read_with(cx, |thread, cx| {
- thread.project().read(cx).find_project_path(&input.path, cx)
- }) else {
- return Task::ready(Err(anyhow!("thread was dropped")));
- };
-
- if project_path.is_some() {
- Task::ready(Ok(()))
- } else {
- let context = crate::ToolPermissionContext {
- tool_name: EditFileTool::NAME.to_string(),
- input_value: path_str.to_string(),
- };
- event_stream.authorize(&input.display_description, context, cx)
- }
+ super::edit_file_tool::authorize_file_edit(
+ EditFileTool::NAME,
+ &input.path,
+ &input.display_description,
+ &self.thread,
+ event_stream,
+ cx,
+ )
}
}