Detailed changes
@@ -1115,6 +1115,27 @@ fn test_permission_options_without_pattern() {
assert!(!labels.iter().any(|label| label.contains("commands")));
}
+#[test]
+fn test_permission_options_symlink_target_are_flat_once_only() {
+ let permission_options =
+ ToolPermissionContext::symlink_target(EditFileTool::NAME, vec!["/outside/file.txt".into()])
+ .build_permission_options();
+
+ let PermissionOptions::Flat(options) = permission_options else {
+ panic!("Expected flat permission options for symlink target authorization");
+ };
+
+ assert_eq!(options.len(), 2);
+ assert!(options.iter().any(|option| {
+ option.option_id.0.as_ref() == "allow"
+ && option.kind == acp::PermissionOptionKind::AllowOnce
+ }));
+ assert!(options.iter().any(|option| {
+ option.option_id.0.as_ref() == "deny"
+ && option.kind == acp::PermissionOptionKind::RejectOnce
+ }));
+}
+
#[test]
fn test_permission_option_ids_for_terminal() {
let permission_options = ToolPermissionContext::new(
@@ -128,10 +128,10 @@ impl AgentTool for ToolRequiringPermission {
return Task::ready(Err(anyhow::anyhow!("{}", reason)));
}
ToolPermissionDecision::Confirm => {
- let context = crate::ToolPermissionContext {
- tool_name: "tool_requiring_permission".to_string(),
- input_values: vec![String::new()],
- };
+ let context = crate::ToolPermissionContext::new(
+ "tool_requiring_permission",
+ vec![String::new()],
+ );
Some(event_stream.authorize("Authorize?", context, cx))
}
};
@@ -629,6 +629,13 @@ pub struct NewTerminal {
pub struct ToolPermissionContext {
pub tool_name: String,
pub input_values: Vec<String>,
+ pub scope: ToolPermissionScope,
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum ToolPermissionScope {
+ ToolInput,
+ SymlinkTarget,
}
impl ToolPermissionContext {
@@ -636,6 +643,15 @@ impl ToolPermissionContext {
Self {
tool_name: tool_name.into(),
input_values,
+ scope: ToolPermissionScope::ToolInput,
+ }
+ }
+
+ pub fn symlink_target(tool_name: impl Into<String>, target_paths: Vec<String>) -> Self {
+ Self {
+ tool_name: tool_name.into(),
+ input_values: target_paths,
+ scope: ToolPermissionScope::SymlinkTarget,
}
}
@@ -669,6 +685,20 @@ impl ToolPermissionContext {
let tool_name = &self.tool_name;
let input_values = &self.input_values;
+ if self.scope == ToolPermissionScope::SymlinkTarget {
+ return acp_thread::PermissionOptions::Flat(vec![
+ acp::PermissionOption::new(
+ acp::PermissionOptionId::new("allow"),
+ "Yes",
+ acp::PermissionOptionKind::AllowOnce,
+ ),
+ acp::PermissionOption::new(
+ acp::PermissionOptionId::new("deny"),
+ "No",
+ acp::PermissionOptionKind::RejectOnce,
+ ),
+ ]);
+ }
// 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.
@@ -17,6 +17,7 @@ mod save_file_tool;
mod streaming_edit_file_tool;
mod subagent_tool;
mod terminal_tool;
+mod tool_permissions;
mod web_search_tool;
use crate::AgentTool;
@@ -41,6 +42,7 @@ pub use save_file_tool::*;
pub use streaming_edit_file_tool::*;
pub use subagent_tool::*;
pub use terminal_tool::*;
+pub use tool_permissions::*;
pub use web_search_tool::*;
macro_rules! tools {
@@ -1,5 +1,6 @@
-use super::edit_file_tool::{
- SensitiveSettingsKind, is_sensitive_settings_path, sensitive_settings_kind,
+use super::tool_permissions::{
+ SensitiveSettingsKind, authorize_symlink_escapes, canonicalize_worktree_roots,
+ collect_symlink_escapes, sensitive_settings_kind,
};
use crate::{AgentTool, ToolCallEventStream, ToolPermissionDecision, decide_permission_for_paths};
use agent_client_protocol::ToolKind;
@@ -84,40 +85,67 @@ impl AgentTool for CopyPathTool {
cx: &mut App,
) -> Task<Result<Self::Output>> {
let settings = AgentSettings::get_global(cx);
-
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 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 project = self.project.clone();
+ cx.spawn(async move |cx| {
+ let fs = project.read_with(cx, |project, _cx| project.fs().clone());
+ let canonical_roots = canonicalize_worktree_roots(&project, &fs, cx).await;
- 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_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))
- .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,
+ let symlink_escapes: Vec<(&str, std::path::PathBuf)> =
+ project.read_with(cx, |project, cx| {
+ collect_symlink_escapes(
+ project,
+ &input.source_path,
+ &input.destination_path,
+ &canonical_roots,
+ cx,
+ )
+ });
+
+ let sensitive_kind =
+ sensitive_settings_kind(Path::new(&input.source_path), fs.as_ref())
+ .await
+ .or(
+ sensitive_settings_kind(Path::new(&input.destination_path), fs.as_ref())
+ .await,
+ );
+
+ let needs_confirmation = matches!(decision, ToolPermissionDecision::Confirm)
+ || (matches!(decision, ToolPermissionDecision::Allow) && sensitive_kind.is_some());
+
+ let authorize = if !symlink_escapes.is_empty() {
+ // Symlink escape authorization replaces (rather than supplements)
+ // the normal tool-permission prompt. The symlink prompt already
+ // requires explicit user approval with the canonical target shown,
+ // which is strictly more security-relevant than a generic confirm.
+ Some(cx.update(|cx| {
+ authorize_symlink_escapes(Self::NAME, &symlink_escapes, &event_stream, cx)
+ }))
+ } else if needs_confirmation {
+ Some(cx.update(|cx| {
+ let src = MarkdownInlineCode(&input.source_path);
+ let dest = MarkdownInlineCode(&input.destination_path);
+ let context = crate::ToolPermissionContext::new(
+ Self::NAME,
+ vec![input.source_path.clone(), input.destination_path.clone()],
+ );
+ let title = format!("Copy {src} to {dest}");
+ let title = match sensitive_kind {
+ Some(SensitiveSettingsKind::Local) => format!("{title} (local settings)"),
+ Some(SensitiveSettingsKind::Global) => format!("{title} (settings)"),
+ None => title,
+ };
+ event_stream.authorize(title, context, cx)
+ }))
+ } else {
+ None
};
- Some(event_stream.authorize(title, context, cx))
- } else {
- None
- };
- let project = self.project.clone();
- cx.spawn(async move |cx| {
if let Some(authorize) = authorize {
authorize.await?;
}
@@ -160,3 +188,258 @@ impl AgentTool for CopyPathTool {
})
}
}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+ use agent_client_protocol as acp;
+ use fs::Fs as _;
+ use gpui::TestAppContext;
+ use project::{FakeFs, Project};
+ use serde_json::json;
+ use settings::SettingsStore;
+ use std::path::PathBuf;
+ use util::path;
+
+ fn init_test(cx: &mut TestAppContext) {
+ cx.update(|cx| {
+ 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]
+ async fn test_copy_path_symlink_escape_source_requests_authorization(cx: &mut TestAppContext) {
+ init_test(cx);
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "project": {
+ "src": { "file.txt": "content" }
+ },
+ "external": {
+ "secret.txt": "SECRET"
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(
+ path!("/root/project/link_to_external").as_ref(),
+ PathBuf::from("../external"),
+ )
+ .await
+ .unwrap();
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let tool = Arc::new(CopyPathTool::new(project));
+
+ let input = CopyPathToolInput {
+ source_path: "project/link_to_external".into(),
+ destination_path: "project/external_copy".into(),
+ };
+
+ let (event_stream, mut event_rx) = ToolCallEventStream::test();
+ let task = cx.update(|cx| tool.run(input, event_stream, cx));
+
+ let auth = event_rx.expect_authorization().await;
+ let title = auth.tool_call.fields.title.as_deref().unwrap_or("");
+ assert!(
+ title.contains("points outside the project")
+ || title.contains("symlinks outside project"),
+ "Authorization title should mention symlink escape, got: {title}",
+ );
+
+ auth.response
+ .send(acp::PermissionOptionId::new("allow"))
+ .unwrap();
+
+ let result = task.await;
+ assert!(result.is_ok(), "should succeed after approval: {result:?}");
+ }
+
+ #[gpui::test]
+ async fn test_copy_path_symlink_escape_denied(cx: &mut TestAppContext) {
+ init_test(cx);
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "project": {
+ "src": { "file.txt": "content" }
+ },
+ "external": {
+ "secret.txt": "SECRET"
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(
+ path!("/root/project/link_to_external").as_ref(),
+ PathBuf::from("../external"),
+ )
+ .await
+ .unwrap();
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let tool = Arc::new(CopyPathTool::new(project));
+
+ let input = CopyPathToolInput {
+ source_path: "project/link_to_external".into(),
+ destination_path: "project/external_copy".into(),
+ };
+
+ let (event_stream, mut event_rx) = ToolCallEventStream::test();
+ let task = cx.update(|cx| tool.run(input, event_stream, cx));
+
+ let auth = event_rx.expect_authorization().await;
+ drop(auth);
+
+ let result = task.await;
+ assert!(result.is_err(), "should fail when denied");
+ }
+
+ #[gpui::test]
+ async fn test_copy_path_symlink_escape_confirm_requires_single_approval(
+ cx: &mut TestAppContext,
+ ) {
+ init_test(cx);
+ cx.update(|cx| {
+ let mut settings = AgentSettings::get_global(cx).clone();
+ settings.tool_permissions.default = settings::ToolPermissionMode::Confirm;
+ AgentSettings::override_global(settings, cx);
+ });
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "project": {
+ "src": { "file.txt": "content" }
+ },
+ "external": {
+ "secret.txt": "SECRET"
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(
+ path!("/root/project/link_to_external").as_ref(),
+ PathBuf::from("../external"),
+ )
+ .await
+ .unwrap();
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let tool = Arc::new(CopyPathTool::new(project));
+
+ let input = CopyPathToolInput {
+ source_path: "project/link_to_external".into(),
+ destination_path: "project/external_copy".into(),
+ };
+
+ let (event_stream, mut event_rx) = ToolCallEventStream::test();
+ let task = cx.update(|cx| tool.run(input, event_stream, cx));
+
+ let auth = event_rx.expect_authorization().await;
+ let title = auth.tool_call.fields.title.as_deref().unwrap_or("");
+ assert!(
+ title.contains("points outside the project")
+ || title.contains("symlinks outside project"),
+ "Authorization title should mention symlink escape, got: {title}",
+ );
+
+ auth.response
+ .send(acp::PermissionOptionId::new("allow"))
+ .unwrap();
+
+ assert!(
+ !matches!(
+ event_rx.try_next(),
+ Ok(Some(Ok(crate::ThreadEvent::ToolCallAuthorization(_))))
+ ),
+ "Expected a single authorization prompt",
+ );
+
+ let result = task.await;
+ assert!(
+ result.is_ok(),
+ "Tool should succeed after one authorization: {result:?}"
+ );
+ }
+
+ #[gpui::test]
+ async fn test_copy_path_symlink_escape_honors_deny_policy(cx: &mut TestAppContext) {
+ init_test(cx);
+ cx.update(|cx| {
+ let mut settings = AgentSettings::get_global(cx).clone();
+ settings.tool_permissions.tools.insert(
+ "copy_path".into(),
+ agent_settings::ToolRules {
+ default: Some(settings::ToolPermissionMode::Deny),
+ ..Default::default()
+ },
+ );
+ AgentSettings::override_global(settings, cx);
+ });
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "project": {
+ "src": { "file.txt": "content" }
+ },
+ "external": {
+ "secret.txt": "SECRET"
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(
+ path!("/root/project/link_to_external").as_ref(),
+ PathBuf::from("../external"),
+ )
+ .await
+ .unwrap();
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let tool = Arc::new(CopyPathTool::new(project));
+
+ let input = CopyPathToolInput {
+ source_path: "project/link_to_external".into(),
+ destination_path: "project/external_copy".into(),
+ };
+
+ let (event_stream, mut event_rx) = ToolCallEventStream::test();
+ let result = cx.update(|cx| tool.run(input, event_stream, cx)).await;
+
+ assert!(result.is_err(), "Tool should fail when policy denies");
+ assert!(
+ !matches!(
+ event_rx.try_next(),
+ Ok(Some(Ok(crate::ThreadEvent::ToolCallAuthorization(_))))
+ ),
+ "Deny policy should not emit symlink authorization prompt",
+ );
+ }
+}
@@ -1,4 +1,7 @@
-use super::edit_file_tool::{SensitiveSettingsKind, sensitive_settings_kind};
+use super::tool_permissions::{
+ SensitiveSettingsKind, authorize_symlink_access, canonicalize_worktree_roots,
+ detect_symlink_escape, sensitive_settings_kind,
+};
use agent_client_protocol::ToolKind;
use agent_settings::AgentSettings;
use anyhow::{Context as _, Result, anyhow};
@@ -8,11 +11,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_for_path};
+use std::path::Path;
/// Creates a new directory at the specified path within the project. Returns confirmation that the directory was created.
///
@@ -71,37 +74,67 @@ impl AgentTool for CreateDirectoryTool {
cx: &mut App,
) -> Task<Result<Self::Output>> {
let settings = AgentSettings::get_global(cx);
- let mut decision = decide_permission_for_path(Self::NAME, &input.path, settings);
- let sensitive_kind = sensitive_settings_kind(Path::new(&input.path));
+ let decision = decide_permission_for_path(Self::NAME, &input.path, settings);
- if matches!(decision, ToolPermissionDecision::Allow) && sensitive_kind.is_some() {
- decision = ToolPermissionDecision::Confirm;
+ if let ToolPermissionDecision::Deny(reason) = decision {
+ return Task::ready(Err(anyhow!("{}", reason)));
}
- let authorize = match decision {
- ToolPermissionDecision::Allow => None,
- ToolPermissionDecision::Deny(reason) => {
- 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_values: vec![input.path.clone()],
- };
- Some(event_stream.authorize(title, context, cx))
- }
- };
-
let destination_path: Arc<str> = input.path.as_str().into();
let project = self.project.clone();
cx.spawn(async move |cx| {
+ let fs = project.read_with(cx, |project, _cx| project.fs().clone());
+ let canonical_roots = canonicalize_worktree_roots(&project, &fs, cx).await;
+
+ let symlink_escape_target = project.read_with(cx, |project, cx| {
+ detect_symlink_escape(project, &input.path, &canonical_roots, cx)
+ .map(|(_, target)| target)
+ });
+
+ let sensitive_kind = sensitive_settings_kind(Path::new(&input.path), fs.as_ref()).await;
+
+ let decision =
+ if matches!(decision, ToolPermissionDecision::Allow) && sensitive_kind.is_some() {
+ ToolPermissionDecision::Confirm
+ } else {
+ decision
+ };
+
+ let authorize = if let Some(canonical_target) = symlink_escape_target {
+ // Symlink escape authorization replaces (rather than supplements)
+ // the normal tool-permission prompt. The symlink prompt already
+ // requires explicit user approval with the canonical target shown,
+ // which is strictly more security-relevant than a generic confirm.
+ Some(cx.update(|cx| {
+ authorize_symlink_access(
+ Self::NAME,
+ &input.path,
+ &canonical_target,
+ &event_stream,
+ cx,
+ )
+ }))
+ } else {
+ match decision {
+ ToolPermissionDecision::Allow => None,
+ ToolPermissionDecision::Confirm => Some(cx.update(|cx| {
+ 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::new(Self::NAME, vec![input.path.clone()]);
+ event_stream.authorize(title, context, cx)
+ })),
+ ToolPermissionDecision::Deny(_) => None,
+ }
+ };
+
if let Some(authorize) = authorize {
authorize.await?;
}
@@ -126,3 +159,279 @@ impl AgentTool for CreateDirectoryTool {
})
}
}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+ use agent_client_protocol as acp;
+ use fs::Fs as _;
+ use gpui::TestAppContext;
+ use project::{FakeFs, Project};
+ use serde_json::json;
+ use settings::SettingsStore;
+ use std::path::PathBuf;
+ use util::path;
+
+ use crate::ToolCallEventStream;
+
+ fn init_test(cx: &mut TestAppContext) {
+ cx.update(|cx| {
+ 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]
+ async fn test_create_directory_symlink_escape_requests_authorization(cx: &mut TestAppContext) {
+ init_test(cx);
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "project": {
+ "src": { "main.rs": "fn main() {}" }
+ },
+ "external": {
+ "data": { "file.txt": "content" }
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(
+ path!("/root/project/link_to_external").as_ref(),
+ PathBuf::from("../external"),
+ )
+ .await
+ .unwrap();
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let tool = Arc::new(CreateDirectoryTool::new(project));
+
+ let (event_stream, mut event_rx) = ToolCallEventStream::test();
+ let task = cx.update(|cx| {
+ tool.run(
+ CreateDirectoryToolInput {
+ path: "project/link_to_external".into(),
+ },
+ event_stream,
+ cx,
+ )
+ });
+
+ let auth = event_rx.expect_authorization().await;
+ let title = auth.tool_call.fields.title.as_deref().unwrap_or("");
+ assert!(
+ title.contains("points outside the project") || title.contains("symlink"),
+ "Authorization title should mention symlink escape, got: {title}",
+ );
+
+ auth.response
+ .send(acp::PermissionOptionId::new("allow"))
+ .unwrap();
+
+ let result = task.await;
+ assert!(
+ result.is_ok(),
+ "Tool should succeed after authorization: {result:?}"
+ );
+ }
+
+ #[gpui::test]
+ async fn test_create_directory_symlink_escape_denied(cx: &mut TestAppContext) {
+ init_test(cx);
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "project": {
+ "src": { "main.rs": "fn main() {}" }
+ },
+ "external": {
+ "data": { "file.txt": "content" }
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(
+ path!("/root/project/link_to_external").as_ref(),
+ PathBuf::from("../external"),
+ )
+ .await
+ .unwrap();
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let tool = Arc::new(CreateDirectoryTool::new(project));
+
+ let (event_stream, mut event_rx) = ToolCallEventStream::test();
+ let task = cx.update(|cx| {
+ tool.run(
+ CreateDirectoryToolInput {
+ path: "project/link_to_external".into(),
+ },
+ event_stream,
+ cx,
+ )
+ });
+
+ let auth = event_rx.expect_authorization().await;
+
+ drop(auth);
+
+ let result = task.await;
+ assert!(
+ result.is_err(),
+ "Tool should fail when authorization is denied"
+ );
+ }
+
+ #[gpui::test]
+ async fn test_create_directory_symlink_escape_confirm_requires_single_approval(
+ cx: &mut TestAppContext,
+ ) {
+ init_test(cx);
+ cx.update(|cx| {
+ let mut settings = AgentSettings::get_global(cx).clone();
+ settings.tool_permissions.default = settings::ToolPermissionMode::Confirm;
+ AgentSettings::override_global(settings, cx);
+ });
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "project": {
+ "src": { "main.rs": "fn main() {}" }
+ },
+ "external": {
+ "data": { "file.txt": "content" }
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(
+ path!("/root/project/link_to_external").as_ref(),
+ PathBuf::from("../external"),
+ )
+ .await
+ .unwrap();
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let tool = Arc::new(CreateDirectoryTool::new(project));
+
+ let (event_stream, mut event_rx) = ToolCallEventStream::test();
+ let task = cx.update(|cx| {
+ tool.run(
+ CreateDirectoryToolInput {
+ path: "project/link_to_external".into(),
+ },
+ event_stream,
+ cx,
+ )
+ });
+
+ let auth = event_rx.expect_authorization().await;
+ let title = auth.tool_call.fields.title.as_deref().unwrap_or("");
+ assert!(
+ title.contains("points outside the project") || title.contains("symlink"),
+ "Authorization title should mention symlink escape, got: {title}",
+ );
+
+ auth.response
+ .send(acp::PermissionOptionId::new("allow"))
+ .unwrap();
+
+ assert!(
+ !matches!(
+ event_rx.try_next(),
+ Ok(Some(Ok(crate::ThreadEvent::ToolCallAuthorization(_))))
+ ),
+ "Expected a single authorization prompt",
+ );
+
+ let result = task.await;
+ assert!(
+ result.is_ok(),
+ "Tool should succeed after one authorization: {result:?}"
+ );
+ }
+
+ #[gpui::test]
+ async fn test_create_directory_symlink_escape_honors_deny_policy(cx: &mut TestAppContext) {
+ init_test(cx);
+ cx.update(|cx| {
+ let mut settings = AgentSettings::get_global(cx).clone();
+ settings.tool_permissions.tools.insert(
+ "create_directory".into(),
+ agent_settings::ToolRules {
+ default: Some(settings::ToolPermissionMode::Deny),
+ ..Default::default()
+ },
+ );
+ AgentSettings::override_global(settings, cx);
+ });
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "project": {
+ "src": { "main.rs": "fn main() {}" }
+ },
+ "external": {
+ "data": { "file.txt": "content" }
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(
+ path!("/root/project/link_to_external").as_ref(),
+ PathBuf::from("../external"),
+ )
+ .await
+ .unwrap();
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let tool = Arc::new(CreateDirectoryTool::new(project));
+
+ let (event_stream, mut event_rx) = ToolCallEventStream::test();
+ let result = cx
+ .update(|cx| {
+ tool.run(
+ CreateDirectoryToolInput {
+ path: "project/link_to_external".into(),
+ },
+ event_stream,
+ cx,
+ )
+ })
+ .await;
+
+ assert!(result.is_err(), "Tool should fail when policy denies");
+ assert!(
+ !matches!(
+ event_rx.try_next(),
+ Ok(Some(Ok(crate::ThreadEvent::ToolCallAuthorization(_))))
+ ),
+ "Deny policy should not emit symlink authorization prompt",
+ );
+ }
+}
@@ -1,5 +1,6 @@
-use super::edit_file_tool::{
- SensitiveSettingsKind, is_sensitive_settings_path, sensitive_settings_kind,
+use super::tool_permissions::{
+ SensitiveSettingsKind, authorize_symlink_access, canonicalize_worktree_roots,
+ detect_symlink_escape, sensitive_settings_kind,
};
use crate::{AgentTool, ToolCallEventStream, ToolPermissionDecision, decide_permission_for_path};
use action_log::ActionLog;
@@ -78,37 +79,66 @@ impl AgentTool for DeletePathTool {
let path = input.path;
let settings = AgentSettings::get_global(cx);
- let mut decision = decide_permission_for_path(Self::NAME, &path, settings);
+ let decision = decide_permission_for_path(Self::NAME, &path, settings);
- if matches!(decision, ToolPermissionDecision::Allow)
- && is_sensitive_settings_path(Path::new(&path))
- {
- decision = ToolPermissionDecision::Confirm;
+ if let ToolPermissionDecision::Deny(reason) = decision {
+ return Task::ready(Err(anyhow!("{}", reason)));
}
- let authorize = match decision {
- ToolPermissionDecision::Allow => None,
- ToolPermissionDecision::Deny(reason) => {
- return Task::ready(Err(anyhow!("{}", reason)));
- }
- ToolPermissionDecision::Confirm => {
- let context = crate::ToolPermissionContext {
- tool_name: Self::NAME.to_string(),
- input_values: vec![path.clone()],
- };
- 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 project = self.project.clone();
let action_log = self.action_log.clone();
cx.spawn(async move |cx| {
+ let fs = project.read_with(cx, |project, _cx| project.fs().clone());
+ let canonical_roots = canonicalize_worktree_roots(&project, &fs, cx).await;
+
+ let symlink_escape_target = project.read_with(cx, |project, cx| {
+ detect_symlink_escape(project, &path, &canonical_roots, cx)
+ .map(|(_, target)| target)
+ });
+
+ let settings_kind = sensitive_settings_kind(Path::new(&path), fs.as_ref()).await;
+
+ let decision =
+ if matches!(decision, ToolPermissionDecision::Allow) && settings_kind.is_some() {
+ ToolPermissionDecision::Confirm
+ } else {
+ decision
+ };
+
+ let authorize = if let Some(canonical_target) = symlink_escape_target {
+ // Symlink escape authorization replaces (rather than supplements)
+ // the normal tool-permission prompt. The symlink prompt already
+ // requires explicit user approval with the canonical target shown,
+ // which is strictly more security-relevant than a generic confirm.
+ Some(cx.update(|cx| {
+ authorize_symlink_access(
+ Self::NAME,
+ &path,
+ &canonical_target,
+ &event_stream,
+ cx,
+ )
+ }))
+ } else {
+ match decision {
+ ToolPermissionDecision::Allow => None,
+ ToolPermissionDecision::Confirm => Some(cx.update(|cx| {
+ let context =
+ crate::ToolPermissionContext::new(Self::NAME, vec![path.clone()]);
+ let title = format!("Delete {}", MarkdownInlineCode(&path));
+ let title = match settings_kind {
+ Some(SensitiveSettingsKind::Local) => {
+ format!("{title} (local settings)")
+ }
+ Some(SensitiveSettingsKind::Global) => format!("{title} (settings)"),
+ None => title,
+ };
+ event_stream.authorize(title, context, cx)
+ })),
+ ToolPermissionDecision::Deny(_) => None,
+ }
+ };
+
if let Some(authorize) = authorize {
authorize.await?;
}
@@ -188,3 +218,293 @@ impl AgentTool for DeletePathTool {
})
}
}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+ use agent_client_protocol as acp;
+ use fs::Fs as _;
+ use gpui::TestAppContext;
+ use project::{FakeFs, Project};
+ use serde_json::json;
+ use settings::SettingsStore;
+ use std::path::PathBuf;
+ use util::path;
+
+ use crate::ToolCallEventStream;
+
+ fn init_test(cx: &mut TestAppContext) {
+ cx.update(|cx| {
+ 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]
+ async fn test_delete_path_symlink_escape_requests_authorization(cx: &mut TestAppContext) {
+ init_test(cx);
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "project": {
+ "src": { "main.rs": "fn main() {}" }
+ },
+ "external": {
+ "data": { "file.txt": "content" }
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(
+ path!("/root/project/link_to_external").as_ref(),
+ PathBuf::from("../external"),
+ )
+ .await
+ .unwrap();
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let action_log = cx.new(|_| ActionLog::new(project.clone()));
+ let tool = Arc::new(DeletePathTool::new(project, action_log));
+
+ let (event_stream, mut event_rx) = ToolCallEventStream::test();
+ let task = cx.update(|cx| {
+ tool.run(
+ DeletePathToolInput {
+ path: "project/link_to_external".into(),
+ },
+ event_stream,
+ cx,
+ )
+ });
+
+ let auth = event_rx.expect_authorization().await;
+ let title = auth.tool_call.fields.title.as_deref().unwrap_or("");
+ assert!(
+ title.contains("points outside the project") || title.contains("symlink"),
+ "Authorization title should mention symlink escape, got: {title}",
+ );
+
+ auth.response
+ .send(acp::PermissionOptionId::new("allow"))
+ .unwrap();
+
+ let result = task.await;
+ // FakeFs cannot delete symlink entries (they are neither Dir nor File
+ // internally), so the deletion itself may fail. The important thing is
+ // that the authorization was requested and accepted — any error must
+ // come from the fs layer, not from a permission denial.
+ if let Err(err) = &result {
+ let msg = format!("{err:#}");
+ assert!(
+ !msg.contains("denied") && !msg.contains("authorization"),
+ "Error should not be a permission denial, got: {msg}",
+ );
+ }
+ }
+
+ #[gpui::test]
+ async fn test_delete_path_symlink_escape_denied(cx: &mut TestAppContext) {
+ init_test(cx);
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "project": {
+ "src": { "main.rs": "fn main() {}" }
+ },
+ "external": {
+ "data": { "file.txt": "content" }
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(
+ path!("/root/project/link_to_external").as_ref(),
+ PathBuf::from("../external"),
+ )
+ .await
+ .unwrap();
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let action_log = cx.new(|_| ActionLog::new(project.clone()));
+ let tool = Arc::new(DeletePathTool::new(project, action_log));
+
+ let (event_stream, mut event_rx) = ToolCallEventStream::test();
+ let task = cx.update(|cx| {
+ tool.run(
+ DeletePathToolInput {
+ path: "project/link_to_external".into(),
+ },
+ event_stream,
+ cx,
+ )
+ });
+
+ let auth = event_rx.expect_authorization().await;
+
+ drop(auth);
+
+ let result = task.await;
+ assert!(
+ result.is_err(),
+ "Tool should fail when authorization is denied"
+ );
+ }
+
+ #[gpui::test]
+ async fn test_delete_path_symlink_escape_confirm_requires_single_approval(
+ cx: &mut TestAppContext,
+ ) {
+ init_test(cx);
+ cx.update(|cx| {
+ let mut settings = AgentSettings::get_global(cx).clone();
+ settings.tool_permissions.default = settings::ToolPermissionMode::Confirm;
+ AgentSettings::override_global(settings, cx);
+ });
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "project": {
+ "src": { "main.rs": "fn main() {}" }
+ },
+ "external": {
+ "data": { "file.txt": "content" }
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(
+ path!("/root/project/link_to_external").as_ref(),
+ PathBuf::from("../external"),
+ )
+ .await
+ .unwrap();
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let action_log = cx.new(|_| ActionLog::new(project.clone()));
+ let tool = Arc::new(DeletePathTool::new(project, action_log));
+
+ let (event_stream, mut event_rx) = ToolCallEventStream::test();
+ let task = cx.update(|cx| {
+ tool.run(
+ DeletePathToolInput {
+ path: "project/link_to_external".into(),
+ },
+ event_stream,
+ cx,
+ )
+ });
+
+ let auth = event_rx.expect_authorization().await;
+ let title = auth.tool_call.fields.title.as_deref().unwrap_or("");
+ assert!(
+ title.contains("points outside the project") || title.contains("symlink"),
+ "Authorization title should mention symlink escape, got: {title}",
+ );
+
+ auth.response
+ .send(acp::PermissionOptionId::new("allow"))
+ .unwrap();
+
+ assert!(
+ !matches!(
+ event_rx.try_next(),
+ Ok(Some(Ok(crate::ThreadEvent::ToolCallAuthorization(_))))
+ ),
+ "Expected a single authorization prompt",
+ );
+
+ let result = task.await;
+ if let Err(err) = &result {
+ let message = format!("{err:#}");
+ assert!(
+ !message.contains("denied") && !message.contains("authorization"),
+ "Error should not be a permission denial, got: {message}",
+ );
+ }
+ }
+
+ #[gpui::test]
+ async fn test_delete_path_symlink_escape_honors_deny_policy(cx: &mut TestAppContext) {
+ init_test(cx);
+ cx.update(|cx| {
+ let mut settings = AgentSettings::get_global(cx).clone();
+ settings.tool_permissions.tools.insert(
+ "delete_path".into(),
+ agent_settings::ToolRules {
+ default: Some(settings::ToolPermissionMode::Deny),
+ ..Default::default()
+ },
+ );
+ AgentSettings::override_global(settings, cx);
+ });
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "project": {
+ "src": { "main.rs": "fn main() {}" }
+ },
+ "external": {
+ "data": { "file.txt": "content" }
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(
+ path!("/root/project/link_to_external").as_ref(),
+ PathBuf::from("../external"),
+ )
+ .await
+ .unwrap();
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let action_log = cx.new(|_| ActionLog::new(project.clone()));
+ let tool = Arc::new(DeletePathTool::new(project, action_log));
+
+ let (event_stream, mut event_rx) = ToolCallEventStream::test();
+ let result = cx
+ .update(|cx| {
+ tool.run(
+ DeletePathToolInput {
+ path: "project/link_to_external".into(),
+ },
+ event_stream,
+ cx,
+ )
+ })
+ .await;
+
+ assert!(result.is_err(), "Tool should fail when policy denies");
+ assert!(
+ !matches!(
+ event_rx.try_next(),
+ Ok(Some(Ok(crate::ThreadEvent::ToolCallAuthorization(_))))
+ ),
+ "Deny policy should not emit symlink authorization prompt",
+ );
+ }
+}
@@ -1,8 +1,8 @@
use super::restore_file_from_disk_tool::RestoreFileFromDiskTool;
use super::save_file_tool::SaveFileTool;
+use super::tool_permissions::authorize_file_edit;
use crate::{
- AgentTool, Templates, Thread, ToolCallEventStream, ToolPermissionDecision,
- decide_permission_for_path,
+ AgentTool, Templates, Thread, ToolCallEventStream,
edit_agent::{EditAgent, EditAgentOutput, EditAgentOutputEvent, EditFormat},
};
use acp_thread::Diff;
@@ -16,14 +16,11 @@ use indoc::formatdoc;
use language::language_settings::{self, FormatOnSave};
use language::{LanguageRegistry, ToPoint};
use language_model::LanguageModelToolResultContent;
-use paths;
use project::lsp_store::{FormatTrigger, LspFormatTarget};
use project::{Project, ProjectPath};
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
-use settings::Settings;
-use std::ffi::OsStr;
-use std::path::{Path, PathBuf};
+use std::path::PathBuf;
use std::sync::Arc;
use ui::SharedString;
use util::ResultExt;
@@ -163,144 +160,6 @@ impl EditFileTool {
}
}
-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,
- }
- }
- };
- 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)));
- }
-
- let explicitly_allowed = matches!(decision, ToolPermissionDecision::Allow);
-
- if explicitly_allowed && !is_sensitive_settings_path(path) {
- return Task::ready(Ok(()));
- }
-
- match sensitive_settings_kind(path) {
- Some(SensitiveSettingsKind::Local) => {
- let context = crate::ToolPermissionContext {
- tool_name: tool_name.to_string(),
- input_values: vec![path_str.to_string()],
- };
- return event_stream.authorize(
- format!("{} (local settings)", display_description),
- context,
- cx,
- );
- }
- Some(SensitiveSettingsKind::Global) => {
- let context = crate::ToolPermissionContext {
- tool_name: tool_name.to_string(),
- input_values: vec![path_str.to_string()],
- };
- return event_stream.authorize(
- format!("{} (settings)", display_description),
- context,
- cx,
- );
- }
- None => {}
- }
-
- 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_values: vec![path_str.to_string()],
- };
- event_stream.authorize(display_description, context, cx)
- }
-}
-
impl AgentTool for EditFileTool {
type Input = EditFileToolInput;
type Output = EditFileToolOutput;
@@ -725,12 +584,14 @@ fn resolve_path(
#[cfg(test)]
mod tests {
use super::*;
+ use crate::tools::tool_permissions::{SensitiveSettingsKind, sensitive_settings_kind};
use crate::{ContextServerRegistry, Templates};
- use fs::Fs;
+ use fs::Fs as _;
use gpui::{TestAppContext, UpdateGlobal};
use language_model::fake_provider::FakeLanguageModel;
use prompt_store::ProjectContext;
use serde_json::json;
+ use settings::Settings;
use settings::SettingsStore;
use util::{path, rel_path::rel_path};
@@ -1391,6 +1252,302 @@ mod tests {
assert_eq!(event.tool_call.fields.title, Some("test 5.4".into()));
}
+ #[gpui::test]
+ async fn test_authorize_create_under_symlink_with_allow(cx: &mut TestAppContext) {
+ init_test(cx);
+
+ let fs = project::FakeFs::new(cx.executor());
+ fs.insert_tree("/root", json!({})).await;
+ fs.insert_tree("/outside", json!({})).await;
+ fs.insert_symlink("/root/link", PathBuf::from("/outside"))
+ .await;
+
+ let project = Project::test(fs.clone(), [path!("/root").as_ref()], cx).await;
+ let context_server_registry =
+ cx.new(|cx| ContextServerRegistry::new(project.read(cx).context_server_store(), cx));
+ let language_registry = project.read_with(cx, |project, _cx| project.languages().clone());
+ let model = Arc::new(FakeLanguageModel::default());
+ let thread = cx.new(|cx| {
+ Thread::new(
+ project.clone(),
+ cx.new(|_cx| ProjectContext::default()),
+ context_server_registry,
+ Templates::new(),
+ Some(model),
+ cx,
+ )
+ });
+ let tool = Arc::new(EditFileTool::new(
+ project,
+ thread.downgrade(),
+ language_registry,
+ Templates::new(),
+ ));
+
+ 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 (stream_tx, mut stream_rx) = ToolCallEventStream::test();
+ let authorize_task = cx.update(|cx| {
+ tool.authorize(
+ &EditFileToolInput {
+ display_description: "create through symlink".into(),
+ path: "link/new.txt".into(),
+ mode: EditFileMode::Create,
+ },
+ &stream_tx,
+ cx,
+ )
+ });
+
+ let event = stream_rx.expect_authorization().await;
+ assert!(
+ event
+ .tool_call
+ .fields
+ .title
+ .as_deref()
+ .is_some_and(|title| title.contains("points outside the project")),
+ "Expected symlink escape authorization for create under external symlink"
+ );
+
+ event
+ .response
+ .send(acp::PermissionOptionId::new("allow"))
+ .unwrap();
+ authorize_task.await.unwrap();
+ }
+
+ #[gpui::test]
+ async fn test_edit_file_symlink_escape_requests_authorization(cx: &mut TestAppContext) {
+ init_test(cx);
+
+ let fs = project::FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "src": { "main.rs": "fn main() {}" }
+ }),
+ )
+ .await;
+ fs.insert_tree(
+ path!("/outside"),
+ json!({
+ "config.txt": "old content"
+ }),
+ )
+ .await;
+ fs.create_symlink(
+ path!("/root/link_to_external").as_ref(),
+ PathBuf::from("/outside"),
+ )
+ .await
+ .unwrap();
+
+ let project = Project::test(fs.clone(), [path!("/root").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let language_registry = project.read_with(cx, |project, _| project.languages().clone());
+ let context_server_registry =
+ cx.new(|cx| ContextServerRegistry::new(project.read(cx).context_server_store(), cx));
+ let model = Arc::new(FakeLanguageModel::default());
+ let thread = cx.new(|cx| {
+ Thread::new(
+ project.clone(),
+ cx.new(|_cx| ProjectContext::default()),
+ context_server_registry,
+ Templates::new(),
+ Some(model),
+ cx,
+ )
+ });
+ let tool = Arc::new(EditFileTool::new(
+ project.clone(),
+ thread.downgrade(),
+ language_registry,
+ Templates::new(),
+ ));
+
+ let (stream_tx, mut stream_rx) = ToolCallEventStream::test();
+ let _authorize_task = cx.update(|cx| {
+ tool.authorize(
+ &EditFileToolInput {
+ display_description: "edit through symlink".into(),
+ path: PathBuf::from("link_to_external/config.txt"),
+ mode: EditFileMode::Edit,
+ },
+ &stream_tx,
+ cx,
+ )
+ });
+
+ let auth = stream_rx.expect_authorization().await;
+ let title = auth.tool_call.fields.title.as_deref().unwrap_or("");
+ assert!(
+ title.contains("points outside the project"),
+ "title should mention symlink escape, got: {title}"
+ );
+ }
+
+ #[gpui::test]
+ async fn test_edit_file_symlink_escape_denied(cx: &mut TestAppContext) {
+ init_test(cx);
+
+ let fs = project::FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "src": { "main.rs": "fn main() {}" }
+ }),
+ )
+ .await;
+ fs.insert_tree(
+ path!("/outside"),
+ json!({
+ "config.txt": "old content"
+ }),
+ )
+ .await;
+ fs.create_symlink(
+ path!("/root/link_to_external").as_ref(),
+ PathBuf::from("/outside"),
+ )
+ .await
+ .unwrap();
+
+ let project = Project::test(fs.clone(), [path!("/root").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let language_registry = project.read_with(cx, |project, _| project.languages().clone());
+ let context_server_registry =
+ cx.new(|cx| ContextServerRegistry::new(project.read(cx).context_server_store(), cx));
+ let model = Arc::new(FakeLanguageModel::default());
+ let thread = cx.new(|cx| {
+ Thread::new(
+ project.clone(),
+ cx.new(|_cx| ProjectContext::default()),
+ context_server_registry,
+ Templates::new(),
+ Some(model),
+ cx,
+ )
+ });
+ let tool = Arc::new(EditFileTool::new(
+ project.clone(),
+ thread.downgrade(),
+ language_registry,
+ Templates::new(),
+ ));
+
+ let (stream_tx, mut stream_rx) = ToolCallEventStream::test();
+ let authorize_task = cx.update(|cx| {
+ tool.authorize(
+ &EditFileToolInput {
+ display_description: "edit through symlink".into(),
+ path: PathBuf::from("link_to_external/config.txt"),
+ mode: EditFileMode::Edit,
+ },
+ &stream_tx,
+ cx,
+ )
+ });
+
+ let auth = stream_rx.expect_authorization().await;
+ drop(auth); // deny by dropping
+
+ let result = authorize_task.await;
+ assert!(result.is_err(), "should fail when denied");
+ }
+
+ #[gpui::test]
+ async fn test_edit_file_symlink_escape_honors_deny_policy(cx: &mut TestAppContext) {
+ init_test(cx);
+ 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: Some(settings::ToolPermissionMode::Deny),
+ ..Default::default()
+ },
+ );
+ agent_settings::AgentSettings::override_global(settings, cx);
+ });
+
+ let fs = project::FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "src": { "main.rs": "fn main() {}" }
+ }),
+ )
+ .await;
+ fs.insert_tree(
+ path!("/outside"),
+ json!({
+ "config.txt": "old content"
+ }),
+ )
+ .await;
+ fs.create_symlink(
+ path!("/root/link_to_external").as_ref(),
+ PathBuf::from("/outside"),
+ )
+ .await
+ .unwrap();
+
+ let project = Project::test(fs.clone(), [path!("/root").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let language_registry = project.read_with(cx, |project, _| project.languages().clone());
+ let context_server_registry =
+ cx.new(|cx| ContextServerRegistry::new(project.read(cx).context_server_store(), cx));
+ let model = Arc::new(FakeLanguageModel::default());
+ let thread = cx.new(|cx| {
+ Thread::new(
+ project.clone(),
+ cx.new(|_cx| ProjectContext::default()),
+ context_server_registry,
+ Templates::new(),
+ Some(model),
+ cx,
+ )
+ });
+ let tool = Arc::new(EditFileTool::new(
+ project.clone(),
+ thread.downgrade(),
+ language_registry,
+ Templates::new(),
+ ));
+
+ let (stream_tx, mut stream_rx) = ToolCallEventStream::test();
+ let result = cx
+ .update(|cx| {
+ tool.authorize(
+ &EditFileToolInput {
+ display_description: "edit through symlink".into(),
+ path: PathBuf::from("link_to_external/config.txt"),
+ mode: EditFileMode::Edit,
+ },
+ &stream_tx,
+ cx,
+ )
+ })
+ .await;
+
+ assert!(result.is_err(), "Tool should fail when policy denies");
+ assert!(
+ !matches!(
+ stream_rx.try_next(),
+ Ok(Some(Ok(crate::ThreadEvent::ToolCallAuthorization(_))))
+ ),
+ "Deny policy should not emit symlink authorization prompt",
+ );
+ }
+
#[gpui::test]
async fn test_authorize_global_config(cx: &mut TestAppContext) {
init_test(cx);
@@ -2392,13 +2549,18 @@ mod tests {
);
}
- #[test]
- fn test_sensitive_settings_kind_detects_nonexistent_subdirectory() {
+ #[gpui::test]
+ async fn test_sensitive_settings_kind_detects_nonexistent_subdirectory(
+ cx: &mut TestAppContext,
+ ) {
+ let fs = project::FakeFs::new(cx.executor());
let config_dir = paths::config_dir();
+ fs.insert_tree(&*config_dir.to_string_lossy(), json!({}))
+ .await;
let path = config_dir.join("nonexistent_subdir_xyz").join("evil.json");
assert!(
matches!(
- sensitive_settings_kind(&path),
+ sensitive_settings_kind(&path, fs.as_ref()).await,
Some(SensitiveSettingsKind::Global)
),
"Path in non-existent subdirectory of config dir should be detected as sensitive: {:?}",
@@ -2406,13 +2568,18 @@ mod tests {
);
}
- #[test]
- fn test_sensitive_settings_kind_detects_deeply_nested_nonexistent_subdirectory() {
+ #[gpui::test]
+ async fn test_sensitive_settings_kind_detects_deeply_nested_nonexistent_subdirectory(
+ cx: &mut TestAppContext,
+ ) {
+ let fs = project::FakeFs::new(cx.executor());
let config_dir = paths::config_dir();
+ fs.insert_tree(&*config_dir.to_string_lossy(), json!({}))
+ .await;
let path = config_dir.join("a").join("b").join("c").join("evil.json");
assert!(
matches!(
- sensitive_settings_kind(&path),
+ sensitive_settings_kind(&path, fs.as_ref()).await,
Some(SensitiveSettingsKind::Global)
),
"Path in deeply nested non-existent subdirectory of config dir should be detected as sensitive: {:?}",
@@ -2420,11 +2587,14 @@ mod tests {
);
}
- #[test]
- fn test_sensitive_settings_kind_returns_none_for_non_config_path() {
+ #[gpui::test]
+ async fn test_sensitive_settings_kind_returns_none_for_non_config_path(
+ cx: &mut TestAppContext,
+ ) {
+ let fs = project::FakeFs::new(cx.executor());
let path = PathBuf::from("/tmp/not_a_config_dir/some_file.json");
assert!(
- sensitive_settings_kind(&path).is_none(),
+ sensitive_settings_kind(&path, fs.as_ref()).await.is_none(),
"Path outside config dir should not be detected as sensitive: {:?}",
path
);
@@ -155,10 +155,8 @@ impl AgentTool for FetchTool {
return Task::ready(Err(anyhow::anyhow!("{}", reason)));
}
ToolPermissionDecision::Confirm => {
- let context = crate::ToolPermissionContext {
- tool_name: Self::NAME.to_string(),
- input_values: vec![input.url.clone()],
- };
+ let context =
+ crate::ToolPermissionContext::new(Self::NAME, vec![input.url.clone()]);
Some(event_stream.authorize(
format!("Fetch {}", MarkdownInlineCode(&input.url)),
context,
@@ -1,6 +1,10 @@
+use super::tool_permissions::{
+ ResolvedProjectPath, authorize_symlink_access, canonicalize_worktree_roots,
+ resolve_project_path,
+};
use crate::{AgentTool, ToolCallEventStream};
use agent_client_protocol::ToolKind;
-use anyhow::{Result, anyhow};
+use anyhow::{Context as _, Result, anyhow};
use gpui::{App, Entity, SharedString, Task};
use project::{Project, ProjectPath, WorktreeSettings};
use schemars::JsonSchema;
@@ -45,6 +49,76 @@ impl ListDirectoryTool {
pub fn new(project: Entity<Project>) -> Self {
Self { project }
}
+
+ fn build_directory_output(
+ project: &Entity<Project>,
+ project_path: &ProjectPath,
+ input_path: &str,
+ cx: &App,
+ ) -> Result<String> {
+ let worktree = project
+ .read(cx)
+ .worktree_for_id(project_path.worktree_id, cx)
+ .with_context(|| format!("{input_path} is not in a known worktree"))?;
+
+ let global_settings = WorktreeSettings::get_global(cx);
+ let worktree_settings = WorktreeSettings::get(Some(project_path.into()), cx);
+ let worktree_snapshot = worktree.read(cx).snapshot();
+ let worktree_root_name = worktree.read(cx).root_name();
+
+ let Some(entry) = worktree_snapshot.entry_for_path(&project_path.path) else {
+ return Err(anyhow!("Path not found: {}", input_path));
+ };
+
+ if !entry.is_dir() {
+ return Err(anyhow!("{input_path} is not a directory."));
+ }
+
+ let mut folders = Vec::new();
+ let mut files = Vec::new();
+
+ for entry in worktree_snapshot.child_entries(&project_path.path) {
+ // Skip private and excluded files and directories
+ if global_settings.is_path_private(&entry.path)
+ || global_settings.is_path_excluded(&entry.path)
+ {
+ continue;
+ }
+
+ let project_path: ProjectPath = (worktree_snapshot.id(), entry.path.clone()).into();
+ if worktree_settings.is_path_excluded(&project_path.path)
+ || worktree_settings.is_path_private(&project_path.path)
+ {
+ continue;
+ }
+
+ let full_path = worktree_root_name
+ .join(&entry.path)
+ .display(worktree_snapshot.path_style())
+ .into_owned();
+ if entry.is_dir() {
+ folders.push(full_path);
+ } else {
+ files.push(full_path);
+ }
+ }
+
+ let mut output = String::new();
+
+ if !folders.is_empty() {
+ writeln!(output, "# Folders:\n{}", folders.join("\n")).unwrap();
+ }
+
+ if !files.is_empty() {
+ writeln!(output, "\n# Files:\n{}", files.join("\n")).unwrap();
+ }
+
+ if output.is_empty() {
+ writeln!(output, "{input_path} is empty.").unwrap();
+ }
+
+ Ok(output)
+ }
}
impl AgentTool for ListDirectoryTool {
@@ -73,7 +147,7 @@ impl AgentTool for ListDirectoryTool {
fn run(
self: Arc<Self>,
input: Self::Input,
- _event_stream: ToolCallEventStream,
+ event_stream: ToolCallEventStream,
cx: &mut App,
) -> Task<Result<Self::Output>> {
// Sometimes models will return these even though we tell it to give a path and not a glob.
@@ -98,115 +172,104 @@ impl AgentTool for ListDirectoryTool {
return Task::ready(Ok(output));
}
- let Some(project_path) = self.project.read(cx).find_project_path(&input.path, cx) else {
- return Task::ready(Err(anyhow!("Path {} not found in project", input.path)));
- };
- let Some(worktree) = self
- .project
- .read(cx)
- .worktree_for_id(project_path.worktree_id, cx)
- else {
- return Task::ready(Err(anyhow!("Worktree not found")));
- };
-
- // Check if the directory whose contents we're listing is itself excluded or private
- let global_settings = WorktreeSettings::get_global(cx);
- if global_settings.is_path_excluded(&project_path.path) {
- return Task::ready(Err(anyhow!(
- "Cannot list directory because its path matches the user's global `file_scan_exclusions` setting: {}",
- &input.path
- )));
- }
-
- if global_settings.is_path_private(&project_path.path) {
- return Task::ready(Err(anyhow!(
- "Cannot list directory because its path matches the user's global `private_files` setting: {}",
- &input.path
- )));
- }
-
- let worktree_settings = WorktreeSettings::get(Some((&project_path).into()), cx);
- if worktree_settings.is_path_excluded(&project_path.path) {
- return Task::ready(Err(anyhow!(
- "Cannot list directory because its path matches the user's worktree`file_scan_exclusions` setting: {}",
- &input.path
- )));
- }
-
- if worktree_settings.is_path_private(&project_path.path) {
- return Task::ready(Err(anyhow!(
- "Cannot list directory because its path matches the user's worktree `private_paths` setting: {}",
- &input.path
- )));
- }
-
- let worktree_snapshot = worktree.read(cx).snapshot();
- let worktree_root_name = worktree.read(cx).root_name();
-
- let Some(entry) = worktree_snapshot.entry_for_path(&project_path.path) else {
- return Task::ready(Err(anyhow!("Path not found: {}", input.path)));
- };
+ let project = self.project.clone();
+ cx.spawn(async move |cx| {
+ let fs = project.read_with(cx, |project, _cx| project.fs().clone());
+ let canonical_roots = canonicalize_worktree_roots(&project, &fs, cx).await;
+
+ let (project_path, symlink_canonical_target) =
+ project.read_with(cx, |project, cx| -> anyhow::Result<_> {
+ let resolved = resolve_project_path(project, &input.path, &canonical_roots, cx)?;
+ Ok(match resolved {
+ ResolvedProjectPath::Safe(path) => (path, None),
+ ResolvedProjectPath::SymlinkEscape {
+ project_path,
+ canonical_target,
+ } => (project_path, Some(canonical_target)),
+ })
+ })?;
+
+ // Check settings exclusions synchronously
+ project.read_with(cx, |project, cx| {
+ let worktree = project
+ .worktree_for_id(project_path.worktree_id, cx)
+ .with_context(|| {
+ format!("{} is not in a known worktree", &input.path)
+ })?;
+
+ let global_settings = WorktreeSettings::get_global(cx);
+ if global_settings.is_path_excluded(&project_path.path) {
+ anyhow::bail!(
+ "Cannot list directory because its path matches the user's global `file_scan_exclusions` setting: {}",
+ &input.path
+ );
+ }
- if !entry.is_dir() {
- return Task::ready(Err(anyhow!("{} is not a directory.", input.path)));
- }
- let worktree_snapshot = worktree.read(cx).snapshot();
+ if global_settings.is_path_private(&project_path.path) {
+ anyhow::bail!(
+ "Cannot list directory because its path matches the user's global `private_files` setting: {}",
+ &input.path
+ );
+ }
- let mut folders = Vec::new();
- let mut files = Vec::new();
+ let worktree_settings = WorktreeSettings::get(Some((&project_path).into()), cx);
+ if worktree_settings.is_path_excluded(&project_path.path) {
+ anyhow::bail!(
+ "Cannot list directory because its path matches the user's worktree `file_scan_exclusions` setting: {}",
+ &input.path
+ );
+ }
- for entry in worktree_snapshot.child_entries(&project_path.path) {
- // Skip private and excluded files and directories
- if global_settings.is_path_private(&entry.path)
- || global_settings.is_path_excluded(&entry.path)
- {
- continue;
- }
+ if worktree_settings.is_path_private(&project_path.path) {
+ anyhow::bail!(
+ "Cannot list directory because its path matches the user's worktree `private_paths` setting: {}",
+ &input.path
+ );
+ }
- let project_path: ProjectPath = (worktree_snapshot.id(), entry.path.clone()).into();
- if worktree_settings.is_path_excluded(&project_path.path)
- || worktree_settings.is_path_private(&project_path.path)
- {
- continue;
- }
+ let worktree_snapshot = worktree.read(cx).snapshot();
+ let Some(entry) = worktree_snapshot.entry_for_path(&project_path.path) else {
+ anyhow::bail!("Path not found: {}", input.path);
+ };
+ if !entry.is_dir() {
+ anyhow::bail!("{} is not a directory.", input.path);
+ }
- let full_path = worktree_root_name
- .join(&entry.path)
- .display(worktree_snapshot.path_style())
- .into_owned();
- if entry.is_dir() {
- folders.push(full_path);
- } else {
- files.push(full_path);
+ anyhow::Ok(())
+ })?;
+
+ if let Some(canonical_target) = &symlink_canonical_target {
+ let authorize = cx.update(|cx| {
+ authorize_symlink_access(
+ Self::NAME,
+ &input.path,
+ canonical_target,
+ &event_stream,
+ cx,
+ )
+ });
+ authorize.await?;
}
- }
-
- let mut output = String::new();
-
- if !folders.is_empty() {
- writeln!(output, "# Folders:\n{}", folders.join("\n")).unwrap();
- }
-
- if !files.is_empty() {
- writeln!(output, "\n# Files:\n{}", files.join("\n")).unwrap();
- }
- if output.is_empty() {
- writeln!(output, "{} is empty.", input.path).unwrap();
- }
-
- Task::ready(Ok(output))
+ let list_path = input.path;
+ cx.update(|cx| {
+ Self::build_directory_output(&project, &project_path, &list_path, cx)
+ })
+ })
}
}
#[cfg(test)]
mod tests {
use super::*;
+ use agent_client_protocol as acp;
+ use fs::Fs as _;
use gpui::{TestAppContext, UpdateGlobal};
use indoc::indoc;
use project::{FakeFs, Project};
use serde_json::json;
use settings::SettingsStore;
+ use std::path::PathBuf;
use util::path;
fn platform_paths(path_str: &str) -> String {
@@ -655,4 +718,305 @@ mod tests {
.contains("Cannot list directory"),
);
}
+
+ #[gpui::test]
+ async fn test_list_directory_symlink_escape_requests_authorization(cx: &mut TestAppContext) {
+ init_test(cx);
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "project": {
+ "src": {
+ "main.rs": "fn main() {}"
+ }
+ },
+ "external": {
+ "secrets": {
+ "key.txt": "SECRET_KEY=abc123"
+ }
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(
+ path!("/root/project/link_to_external").as_ref(),
+ PathBuf::from("../external"),
+ )
+ .await
+ .unwrap();
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let tool = Arc::new(ListDirectoryTool::new(project));
+
+ let (event_stream, mut event_rx) = ToolCallEventStream::test();
+ let task = cx.update(|cx| {
+ tool.clone().run(
+ ListDirectoryToolInput {
+ path: "project/link_to_external".into(),
+ },
+ event_stream,
+ cx,
+ )
+ });
+
+ let auth = event_rx.expect_authorization().await;
+ let title = auth.tool_call.fields.title.as_deref().unwrap_or("");
+ assert!(
+ title.contains("points outside the project"),
+ "Authorization title should mention symlink escape, got: {title}",
+ );
+
+ auth.response
+ .send(acp::PermissionOptionId::new("allow"))
+ .unwrap();
+
+ let result = task.await;
+ assert!(
+ result.is_ok(),
+ "Tool should succeed after authorization: {result:?}"
+ );
+ }
+
+ #[gpui::test]
+ async fn test_list_directory_symlink_escape_denied(cx: &mut TestAppContext) {
+ init_test(cx);
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "project": {
+ "src": {
+ "main.rs": "fn main() {}"
+ }
+ },
+ "external": {
+ "secrets": {}
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(
+ path!("/root/project/link_to_external").as_ref(),
+ PathBuf::from("../external"),
+ )
+ .await
+ .unwrap();
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let tool = Arc::new(ListDirectoryTool::new(project));
+
+ let (event_stream, mut event_rx) = ToolCallEventStream::test();
+ let task = cx.update(|cx| {
+ tool.clone().run(
+ ListDirectoryToolInput {
+ path: "project/link_to_external".into(),
+ },
+ event_stream,
+ cx,
+ )
+ });
+
+ let auth = event_rx.expect_authorization().await;
+
+ // Deny by dropping the response sender without sending
+ drop(auth);
+
+ let result = task.await;
+ assert!(
+ result.is_err(),
+ "Tool should fail when authorization is denied"
+ );
+ }
+
+ #[gpui::test]
+ async fn test_list_directory_symlink_escape_private_path_no_authorization(
+ cx: &mut TestAppContext,
+ ) {
+ init_test(cx);
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "project": {
+ "src": {
+ "main.rs": "fn main() {}"
+ }
+ },
+ "external": {
+ "secrets": {}
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(
+ path!("/root/project/link_to_external").as_ref(),
+ PathBuf::from("../external"),
+ )
+ .await
+ .unwrap();
+
+ cx.update(|cx| {
+ SettingsStore::update_global(cx, |store, cx| {
+ store.update_user_settings(cx, |settings| {
+ settings.project.worktree.private_files =
+ Some(vec!["**/link_to_external".to_string()].into());
+ });
+ });
+ });
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let tool = Arc::new(ListDirectoryTool::new(project));
+
+ let (event_stream, mut event_rx) = ToolCallEventStream::test();
+ let result = cx
+ .update(|cx| {
+ tool.clone().run(
+ ListDirectoryToolInput {
+ path: "project/link_to_external".into(),
+ },
+ event_stream,
+ cx,
+ )
+ })
+ .await;
+
+ assert!(
+ result.is_err(),
+ "Expected list_directory to fail on private path"
+ );
+ let error = result.unwrap_err().to_string();
+ assert!(
+ error.contains("private"),
+ "Expected private path validation error, got: {error}"
+ );
+
+ let event = event_rx.try_next();
+ assert!(
+ !matches!(
+ event,
+ Ok(Some(Ok(crate::thread::ThreadEvent::ToolCallAuthorization(
+ _
+ ))))
+ ),
+ "No authorization should be requested when validation fails before listing",
+ );
+ }
+
+ #[gpui::test]
+ async fn test_list_directory_no_authorization_for_normal_paths(cx: &mut TestAppContext) {
+ init_test(cx);
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/project"),
+ json!({
+ "src": {
+ "main.rs": "fn main() {}"
+ }
+ }),
+ )
+ .await;
+
+ let project = Project::test(fs.clone(), [path!("/project").as_ref()], cx).await;
+ let tool = Arc::new(ListDirectoryTool::new(project));
+
+ let (event_stream, mut event_rx) = ToolCallEventStream::test();
+ let result = cx
+ .update(|cx| {
+ tool.clone().run(
+ ListDirectoryToolInput {
+ path: "project/src".into(),
+ },
+ event_stream,
+ cx,
+ )
+ })
+ .await;
+
+ assert!(
+ result.is_ok(),
+ "Normal path should succeed without authorization"
+ );
+
+ let event = event_rx.try_next();
+ assert!(
+ !matches!(
+ event,
+ Ok(Some(Ok(crate::thread::ThreadEvent::ToolCallAuthorization(
+ _
+ ))))
+ ),
+ "No authorization should be requested for normal paths",
+ );
+ }
+
+ #[gpui::test]
+ async fn test_list_directory_intra_project_symlink_no_authorization(cx: &mut TestAppContext) {
+ init_test(cx);
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/project"),
+ json!({
+ "real_dir": {
+ "file.txt": "content"
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(
+ path!("/project/link_dir").as_ref(),
+ PathBuf::from("real_dir"),
+ )
+ .await
+ .unwrap();
+
+ let project = Project::test(fs.clone(), [path!("/project").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let tool = Arc::new(ListDirectoryTool::new(project));
+
+ let (event_stream, mut event_rx) = ToolCallEventStream::test();
+ let result = cx
+ .update(|cx| {
+ tool.clone().run(
+ ListDirectoryToolInput {
+ path: "project/link_dir".into(),
+ },
+ event_stream,
+ cx,
+ )
+ })
+ .await;
+
+ assert!(
+ result.is_ok(),
+ "Intra-project symlink should succeed without authorization: {result:?}",
+ );
+
+ let event = event_rx.try_next();
+ assert!(
+ !matches!(
+ event,
+ Ok(Some(Ok(crate::thread::ThreadEvent::ToolCallAuthorization(
+ _
+ ))))
+ ),
+ "No authorization should be requested for intra-project symlinks",
+ );
+ }
}
@@ -1,5 +1,6 @@
-use super::edit_file_tool::{
- SensitiveSettingsKind, is_sensitive_settings_path, sensitive_settings_kind,
+use super::tool_permissions::{
+ SensitiveSettingsKind, authorize_symlink_escapes, canonicalize_worktree_roots,
+ collect_symlink_escapes, sensitive_settings_kind,
};
use crate::{AgentTool, ToolCallEventStream, ToolPermissionDecision, decide_permission_for_paths};
use agent_client_protocol::ToolKind;
@@ -97,40 +98,67 @@ impl AgentTool for MovePathTool {
cx: &mut App,
) -> Task<Result<Self::Output>> {
let settings = AgentSettings::get_global(cx);
-
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 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 project = self.project.clone();
+ cx.spawn(async move |cx| {
+ let fs = project.read_with(cx, |project, _cx| project.fs().clone());
+ let canonical_roots = canonicalize_worktree_roots(&project, &fs, cx).await;
- 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_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))
- .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,
+ let symlink_escapes: Vec<(&str, std::path::PathBuf)> =
+ project.read_with(cx, |project, cx| {
+ collect_symlink_escapes(
+ project,
+ &input.source_path,
+ &input.destination_path,
+ &canonical_roots,
+ cx,
+ )
+ });
+
+ let sensitive_kind =
+ sensitive_settings_kind(Path::new(&input.source_path), fs.as_ref())
+ .await
+ .or(
+ sensitive_settings_kind(Path::new(&input.destination_path), fs.as_ref())
+ .await,
+ );
+
+ let needs_confirmation = matches!(decision, ToolPermissionDecision::Confirm)
+ || (matches!(decision, ToolPermissionDecision::Allow) && sensitive_kind.is_some());
+
+ let authorize = if !symlink_escapes.is_empty() {
+ // Symlink escape authorization replaces (rather than supplements)
+ // the normal tool-permission prompt. The symlink prompt already
+ // requires explicit user approval with the canonical target shown,
+ // which is strictly more security-relevant than a generic confirm.
+ Some(cx.update(|cx| {
+ authorize_symlink_escapes(Self::NAME, &symlink_escapes, &event_stream, cx)
+ }))
+ } else if needs_confirmation {
+ Some(cx.update(|cx| {
+ let src = MarkdownInlineCode(&input.source_path);
+ let dest = MarkdownInlineCode(&input.destination_path);
+ let context = crate::ToolPermissionContext::new(
+ Self::NAME,
+ vec![input.source_path.clone(), input.destination_path.clone()],
+ );
+ let title = format!("Move {src} to {dest}");
+ let title = match sensitive_kind {
+ Some(SensitiveSettingsKind::Local) => format!("{title} (local settings)"),
+ Some(SensitiveSettingsKind::Global) => format!("{title} (settings)"),
+ None => title,
+ };
+ event_stream.authorize(title, context, cx)
+ }))
+ } else {
+ None
};
- Some(event_stream.authorize(title, context, cx))
- } else {
- None
- };
- let project = self.project.clone();
- cx.spawn(async move |cx| {
if let Some(authorize) = authorize {
authorize.await?;
}
@@ -170,3 +198,258 @@ impl AgentTool for MovePathTool {
})
}
}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+ use agent_client_protocol as acp;
+ use fs::Fs as _;
+ use gpui::TestAppContext;
+ use project::{FakeFs, Project};
+ use serde_json::json;
+ use settings::SettingsStore;
+ use std::path::PathBuf;
+ use util::path;
+
+ fn init_test(cx: &mut TestAppContext) {
+ cx.update(|cx| {
+ 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]
+ async fn test_move_path_symlink_escape_source_requests_authorization(cx: &mut TestAppContext) {
+ init_test(cx);
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "project": {
+ "src": { "file.txt": "content" }
+ },
+ "external": {
+ "secret.txt": "SECRET"
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(
+ path!("/root/project/link_to_external").as_ref(),
+ PathBuf::from("../external"),
+ )
+ .await
+ .unwrap();
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let tool = Arc::new(MovePathTool::new(project));
+
+ let input = MovePathToolInput {
+ source_path: "project/link_to_external".into(),
+ destination_path: "project/external_moved".into(),
+ };
+
+ let (event_stream, mut event_rx) = ToolCallEventStream::test();
+ let task = cx.update(|cx| tool.run(input, event_stream, cx));
+
+ let auth = event_rx.expect_authorization().await;
+ let title = auth.tool_call.fields.title.as_deref().unwrap_or("");
+ assert!(
+ title.contains("points outside the project")
+ || title.contains("symlinks outside project"),
+ "Authorization title should mention symlink escape, got: {title}",
+ );
+
+ auth.response
+ .send(acp::PermissionOptionId::new("allow"))
+ .unwrap();
+
+ let result = task.await;
+ assert!(result.is_ok(), "should succeed after approval: {result:?}");
+ }
+
+ #[gpui::test]
+ async fn test_move_path_symlink_escape_denied(cx: &mut TestAppContext) {
+ init_test(cx);
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "project": {
+ "src": { "file.txt": "content" }
+ },
+ "external": {
+ "secret.txt": "SECRET"
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(
+ path!("/root/project/link_to_external").as_ref(),
+ PathBuf::from("../external"),
+ )
+ .await
+ .unwrap();
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let tool = Arc::new(MovePathTool::new(project));
+
+ let input = MovePathToolInput {
+ source_path: "project/link_to_external".into(),
+ destination_path: "project/external_moved".into(),
+ };
+
+ let (event_stream, mut event_rx) = ToolCallEventStream::test();
+ let task = cx.update(|cx| tool.run(input, event_stream, cx));
+
+ let auth = event_rx.expect_authorization().await;
+ drop(auth);
+
+ let result = task.await;
+ assert!(result.is_err(), "should fail when denied");
+ }
+
+ #[gpui::test]
+ async fn test_move_path_symlink_escape_confirm_requires_single_approval(
+ cx: &mut TestAppContext,
+ ) {
+ init_test(cx);
+ cx.update(|cx| {
+ let mut settings = AgentSettings::get_global(cx).clone();
+ settings.tool_permissions.default = settings::ToolPermissionMode::Confirm;
+ AgentSettings::override_global(settings, cx);
+ });
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "project": {
+ "src": { "file.txt": "content" }
+ },
+ "external": {
+ "secret.txt": "SECRET"
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(
+ path!("/root/project/link_to_external").as_ref(),
+ PathBuf::from("../external"),
+ )
+ .await
+ .unwrap();
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let tool = Arc::new(MovePathTool::new(project));
+
+ let input = MovePathToolInput {
+ source_path: "project/link_to_external".into(),
+ destination_path: "project/external_moved".into(),
+ };
+
+ let (event_stream, mut event_rx) = ToolCallEventStream::test();
+ let task = cx.update(|cx| tool.run(input, event_stream, cx));
+
+ let auth = event_rx.expect_authorization().await;
+ let title = auth.tool_call.fields.title.as_deref().unwrap_or("");
+ assert!(
+ title.contains("points outside the project")
+ || title.contains("symlinks outside project"),
+ "Authorization title should mention symlink escape, got: {title}",
+ );
+
+ auth.response
+ .send(acp::PermissionOptionId::new("allow"))
+ .unwrap();
+
+ assert!(
+ !matches!(
+ event_rx.try_next(),
+ Ok(Some(Ok(crate::ThreadEvent::ToolCallAuthorization(_))))
+ ),
+ "Expected a single authorization prompt",
+ );
+
+ let result = task.await;
+ assert!(
+ result.is_ok(),
+ "Tool should succeed after one authorization: {result:?}"
+ );
+ }
+
+ #[gpui::test]
+ async fn test_move_path_symlink_escape_honors_deny_policy(cx: &mut TestAppContext) {
+ init_test(cx);
+ cx.update(|cx| {
+ let mut settings = AgentSettings::get_global(cx).clone();
+ settings.tool_permissions.tools.insert(
+ "move_path".into(),
+ agent_settings::ToolRules {
+ default: Some(settings::ToolPermissionMode::Deny),
+ ..Default::default()
+ },
+ );
+ AgentSettings::override_global(settings, cx);
+ });
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "project": {
+ "src": { "file.txt": "content" }
+ },
+ "external": {
+ "secret.txt": "SECRET"
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(
+ path!("/root/project/link_to_external").as_ref(),
+ PathBuf::from("../external"),
+ )
+ .await
+ .unwrap();
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let tool = Arc::new(MovePathTool::new(project));
+
+ let input = MovePathToolInput {
+ source_path: "project/link_to_external".into(),
+ destination_path: "project/external_moved".into(),
+ };
+
+ let (event_stream, mut event_rx) = ToolCallEventStream::test();
+ let result = cx.update(|cx| tool.run(input, event_stream, cx)).await;
+
+ assert!(result.is_err(), "Tool should fail when policy denies");
+ assert!(
+ !matches!(
+ event_rx.try_next(),
+ Ok(Some(Ok(crate::ThreadEvent::ToolCallAuthorization(_))))
+ ),
+ "Deny policy should not emit symlink authorization prompt",
+ );
+ }
+}
@@ -1,8 +1,12 @@
+use super::tool_permissions::{
+ ResolvedProjectPath, authorize_symlink_access, canonicalize_worktree_roots,
+ resolve_project_path,
+};
use crate::AgentTool;
use agent_client_protocol::ToolKind;
use anyhow::{Context as _, Result};
use futures::FutureExt as _;
-use gpui::{App, AppContext, Entity, SharedString, Task};
+use gpui::{App, AppContext as _, Entity, SharedString, Task};
use project::Project;
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
@@ -64,13 +68,51 @@ impl AgentTool for OpenTool {
) -> Task<Result<Self::Output>> {
// If path_or_url turns out to be a path in the project, make it absolute.
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_values: vec![input.path_or_url.clone()],
- };
- let authorize =
- event_stream.authorize(self.initial_title(Ok(input.clone()), cx), context, cx);
- cx.background_spawn(async move {
+ let initial_title = self.initial_title(Ok(input.clone()), cx);
+
+ let project = self.project.clone();
+ cx.spawn(async move |cx| {
+ let fs = project.read_with(cx, |project, _cx| project.fs().clone());
+ let canonical_roots = canonicalize_worktree_roots(&project, &fs, cx).await;
+
+ // Symlink escape authorization replaces (rather than supplements)
+ // the normal tool-permission prompt. The symlink prompt already
+ // requires explicit user approval with the canonical target shown,
+ // which is strictly more security-relevant than a generic confirm.
+ let symlink_escape = project.read_with(cx, |project, cx| {
+ match resolve_project_path(
+ project,
+ PathBuf::from(&input.path_or_url),
+ &canonical_roots,
+ cx,
+ ) {
+ Ok(ResolvedProjectPath::SymlinkEscape {
+ canonical_target, ..
+ }) => Some(canonical_target),
+ _ => None,
+ }
+ });
+
+ let authorize = if let Some(canonical_target) = symlink_escape {
+ cx.update(|cx| {
+ authorize_symlink_access(
+ Self::NAME,
+ &input.path_or_url,
+ &canonical_target,
+ &event_stream,
+ cx,
+ )
+ })
+ } else {
+ cx.update(|cx| {
+ let context = crate::ToolPermissionContext::new(
+ Self::NAME,
+ vec![input.path_or_url.clone()],
+ );
+ event_stream.authorize(initial_title, context, cx)
+ })
+ };
+
futures::select! {
result = authorize.fuse() => result?,
_ = event_stream.cancelled_by_user().fuse() => {
@@ -78,11 +120,15 @@ impl AgentTool for OpenTool {
}
}
- match abs_path {
- Some(path) => open::that(path),
- None => open::that(&input.path_or_url),
- }
- .context("Failed to open URL or file path")?;
+ let path_or_url = input.path_or_url.clone();
+ cx.background_spawn(async move {
+ match abs_path {
+ Some(path) => open::that(path),
+ None => open::that(path_or_url),
+ }
+ .context("Failed to open URL or file path")
+ })
+ .await?;
Ok(format!("Successfully opened {}", input.path_or_url))
})
@@ -13,6 +13,10 @@ use settings::Settings;
use std::sync::Arc;
use util::markdown::MarkdownCodeBlock;
+use super::tool_permissions::{
+ ResolvedProjectPath, authorize_symlink_access, canonicalize_worktree_roots,
+ resolve_project_path,
+};
use crate::{AgentTool, Thread, ToolCallEventStream, outline};
/// Reads the content of the given file in the project.
@@ -110,57 +114,97 @@ impl AgentTool for ReadFileTool {
event_stream: ToolCallEventStream,
cx: &mut App,
) -> Task<Result<LanguageModelToolResultContent>> {
- let Some(project_path) = self.project.read(cx).find_project_path(&input.path, cx) else {
- return Task::ready(Err(anyhow!("Path {} not found in project", &input.path)));
- };
- let Some(abs_path) = self.project.read(cx).absolute_path(&project_path, cx) else {
- return Task::ready(Err(anyhow!(
- "Failed to convert {} to absolute path",
- &input.path
- )));
- };
-
- // Error out if this path is either excluded or private in global settings
- let global_settings = WorktreeSettings::get_global(cx);
- if global_settings.is_path_excluded(&project_path.path) {
- return Task::ready(Err(anyhow!(
- "Cannot read file because its path matches the global `file_scan_exclusions` setting: {}",
- &input.path
- )));
- }
+ let project = self.project.clone();
+ let thread = self.thread.clone();
+ let action_log = self.action_log.clone();
+ cx.spawn(async move |cx| {
+ let fs = project.read_with(cx, |project, _cx| project.fs().clone());
+ let canonical_roots = canonicalize_worktree_roots(&project, &fs, cx).await;
+
+ let (project_path, symlink_canonical_target) =
+ project.read_with(cx, |project, cx| {
+ let resolved =
+ resolve_project_path(project, &input.path, &canonical_roots, cx)?;
+ anyhow::Ok(match resolved {
+ ResolvedProjectPath::Safe(path) => (path, None),
+ ResolvedProjectPath::SymlinkEscape {
+ project_path,
+ canonical_target,
+ } => (project_path, Some(canonical_target)),
+ })
+ })?;
- if global_settings.is_path_private(&project_path.path) {
- return Task::ready(Err(anyhow!(
- "Cannot read file because its path matches the global `private_files` setting: {}",
- &input.path
- )));
- }
+ let abs_path = project
+ .read_with(cx, |project, cx| {
+ project.absolute_path(&project_path, cx)
+ })
+ .ok_or_else(|| {
+ anyhow!("Failed to convert {} to absolute path", &input.path)
+ })?;
+
+ // Check settings exclusions synchronously
+ project.read_with(cx, |_project, cx| {
+ let global_settings = WorktreeSettings::get_global(cx);
+ if global_settings.is_path_excluded(&project_path.path) {
+ anyhow::bail!(
+ "Cannot read file because its path matches the global `file_scan_exclusions` setting: {}",
+ &input.path
+ );
+ }
- // Error out if this path is either excluded or private in worktree settings
- let worktree_settings = WorktreeSettings::get(Some((&project_path).into()), cx);
- if worktree_settings.is_path_excluded(&project_path.path) {
- return Task::ready(Err(anyhow!(
- "Cannot read file because its path matches the worktree `file_scan_exclusions` setting: {}",
- &input.path
- )));
- }
+ if global_settings.is_path_private(&project_path.path) {
+ anyhow::bail!(
+ "Cannot read file because its path matches the global `private_files` setting: {}",
+ &input.path
+ );
+ }
- if worktree_settings.is_path_private(&project_path.path) {
- return Task::ready(Err(anyhow!(
- "Cannot read file because its path matches the worktree `private_files` setting: {}",
- &input.path
- )));
- }
+ let worktree_settings = WorktreeSettings::get(Some((&project_path).into()), cx);
+ if worktree_settings.is_path_excluded(&project_path.path) {
+ anyhow::bail!(
+ "Cannot read file because its path matches the worktree `file_scan_exclusions` setting: {}",
+ &input.path
+ );
+ }
- let file_path = input.path.clone();
+ if worktree_settings.is_path_private(&project_path.path) {
+ anyhow::bail!(
+ "Cannot read file because its path matches the worktree `private_files` setting: {}",
+ &input.path
+ );
+ }
+
+ anyhow::Ok(())
+ })?;
+
+ if let Some(canonical_target) = &symlink_canonical_target {
+ let authorize = cx.update(|cx| {
+ authorize_symlink_access(
+ Self::NAME,
+ &input.path,
+ canonical_target,
+ &event_stream,
+ cx,
+ )
+ });
+ authorize.await?;
+ }
+
+ let file_path = input.path.clone();
- event_stream.update_fields(ToolCallUpdateFields::new().locations(vec![
- acp::ToolCallLocation::new(&abs_path)
- .line(input.start_line.map(|line| line.saturating_sub(1))),
- ]));
+ cx.update(|_cx| {
+ event_stream.update_fields(ToolCallUpdateFields::new().locations(vec![
+ acp::ToolCallLocation::new(&abs_path)
+ .line(input.start_line.map(|line| line.saturating_sub(1))),
+ ]));
+ });
+
+ let is_image = project.read_with(cx, |_project, cx| {
+ image_store::is_image_file(&project, &project_path, cx)
+ });
+
+ if is_image {
- if image_store::is_image_file(&self.project, &project_path, cx) {
- return cx.spawn(async move |cx| {
let image_entity: Entity<ImageItem> = cx
.update(|cx| {
self.project.update(cx, |project, cx| {
@@ -183,18 +227,11 @@ impl AgentTool for ReadFileTool {
))),
]));
- Ok(language_model_image.into())
- });
- }
-
- let project = self.project.clone();
- let action_log = self.action_log.clone();
+ return Ok(language_model_image.into());
+ }
- cx.spawn(async move |cx| {
- let open_buffer_task = cx.update(|cx| {
- project.update(cx, |project, cx| {
- project.open_buffer(project_path.clone(), cx)
- })
+ let open_buffer_task = project.update(cx, |project, cx| {
+ project.open_buffer(project_path.clone(), cx)
});
let buffer = futures::select! {
@@ -216,7 +253,7 @@ impl AgentTool for ReadFileTool {
if let Some(mtime) = buffer.read_with(cx, |buffer, _| {
buffer.file().and_then(|file| file.disk_state().mtime())
}) {
- self.thread
+ thread
.update(cx, |thread, _| {
thread.file_read_times.insert(abs_path.to_path_buf(), mtime);
})
@@ -292,6 +329,7 @@ impl AgentTool for ReadFileTool {
cx,
);
if let Ok(LanguageModelToolResultContent::Text(text)) = &result {
+ let text: &str = text;
let markdown = MarkdownCodeBlock {
tag: &input.path,
text,
@@ -312,12 +350,15 @@ impl AgentTool for ReadFileTool {
mod test {
use super::*;
use crate::{ContextServerRegistry, Templates, Thread};
+ use agent_client_protocol as acp;
+ use fs::Fs as _;
use gpui::{AppContext, TestAppContext, UpdateGlobal as _};
use language_model::fake_provider::FakeLanguageModel;
use project::{FakeFs, Project};
use prompt_store::ProjectContext;
use serde_json::json;
use settings::SettingsStore;
+ use std::path::PathBuf;
use std::sync::Arc;
use util::path;
@@ -606,6 +647,16 @@ mod test {
});
}
+ fn single_pixel_png() -> Vec<u8> {
+ vec![
+ 0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A, 0x00, 0x00, 0x00, 0x0D, 0x49, 0x48,
+ 0x44, 0x52, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x08, 0x06, 0x00, 0x00,
+ 0x00, 0x1F, 0x15, 0xC4, 0x89, 0x00, 0x00, 0x00, 0x0A, 0x49, 0x44, 0x41, 0x54, 0x78,
+ 0x9C, 0x63, 0x00, 0x01, 0x00, 0x00, 0x05, 0x00, 0x01, 0x0D, 0x0A, 0x2D, 0xB4, 0x00,
+ 0x00, 0x00, 0x00, 0x49, 0x45, 0x4E, 0x44, 0xAE, 0x42, 0x60, 0x82,
+ ]
+ }
+
#[gpui::test]
async fn test_read_file_security(cx: &mut TestAppContext) {
init_test(cx);
@@ -813,6 +864,70 @@ mod test {
);
}
+ #[gpui::test]
+ async fn test_read_image_symlink_requires_authorization(cx: &mut TestAppContext) {
+ init_test(cx);
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(path!("/root"), json!({})).await;
+ fs.insert_tree(path!("/outside"), json!({})).await;
+ fs.insert_file(path!("/outside/secret.png"), single_pixel_png())
+ .await;
+ fs.insert_symlink(
+ path!("/root/secret.png"),
+ PathBuf::from("/outside/secret.png"),
+ )
+ .await;
+
+ let project = Project::test(fs.clone(), [path!("/root").as_ref()], cx).await;
+ let action_log = cx.new(|_| ActionLog::new(project.clone()));
+ let context_server_registry =
+ cx.new(|cx| ContextServerRegistry::new(project.read(cx).context_server_store(), cx));
+ let model = Arc::new(FakeLanguageModel::default());
+ let thread = cx.new(|cx| {
+ Thread::new(
+ project.clone(),
+ cx.new(|_cx| ProjectContext::default()),
+ context_server_registry,
+ Templates::new(),
+ Some(model),
+ cx,
+ )
+ });
+ let tool = Arc::new(ReadFileTool::new(thread.downgrade(), project, action_log));
+
+ let (event_stream, mut event_rx) = ToolCallEventStream::test();
+ let read_task = cx.update(|cx| {
+ tool.run(
+ ReadFileToolInput {
+ path: "root/secret.png".to_string(),
+ start_line: None,
+ end_line: None,
+ },
+ event_stream,
+ cx,
+ )
+ });
+
+ let authorization = event_rx.expect_authorization().await;
+ assert!(
+ authorization
+ .tool_call
+ .fields
+ .title
+ .as_deref()
+ .is_some_and(|title| title.contains("points outside the project")),
+ "Expected symlink escape authorization before reading the image"
+ );
+ authorization
+ .response
+ .send(acp::PermissionOptionId::new("allow"))
+ .unwrap();
+
+ let result = read_task.await;
+ assert!(result.is_ok());
+ }
+
#[gpui::test]
async fn test_read_file_with_multiple_worktree_settings(cx: &mut TestAppContext) {
init_test(cx);
@@ -1046,4 +1161,245 @@ mod test {
"Config.toml should be blocked by worktree1's private_files setting"
);
}
+
+ #[gpui::test]
+ async fn test_read_file_symlink_escape_requests_authorization(cx: &mut TestAppContext) {
+ init_test(cx);
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "project": {
+ "src": { "main.rs": "fn main() {}" }
+ },
+ "external": {
+ "secret.txt": "SECRET_KEY=abc123"
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(
+ path!("/root/project/secret_link.txt").as_ref(),
+ PathBuf::from("../external/secret.txt"),
+ )
+ .await
+ .unwrap();
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let action_log = cx.new(|_| ActionLog::new(project.clone()));
+ let context_server_registry =
+ cx.new(|cx| ContextServerRegistry::new(project.read(cx).context_server_store(), cx));
+ let model = Arc::new(FakeLanguageModel::default());
+ let thread = cx.new(|cx| {
+ Thread::new(
+ project.clone(),
+ cx.new(|_cx| ProjectContext::default()),
+ context_server_registry,
+ Templates::new(),
+ Some(model),
+ cx,
+ )
+ });
+ let tool = Arc::new(ReadFileTool::new(
+ thread.downgrade(),
+ project.clone(),
+ action_log,
+ ));
+
+ let (event_stream, mut event_rx) = ToolCallEventStream::test();
+ let task = cx.update(|cx| {
+ tool.clone().run(
+ ReadFileToolInput {
+ path: "project/secret_link.txt".to_string(),
+ start_line: None,
+ end_line: None,
+ },
+ event_stream,
+ cx,
+ )
+ });
+
+ let auth = event_rx.expect_authorization().await;
+ let title = auth.tool_call.fields.title.as_deref().unwrap_or("");
+ assert!(
+ title.contains("points outside the project"),
+ "title: {title}"
+ );
+
+ auth.response
+ .send(acp::PermissionOptionId::new("allow"))
+ .unwrap();
+
+ let result = task.await;
+ assert!(result.is_ok(), "should succeed after approval: {result:?}");
+ }
+
+ #[gpui::test]
+ async fn test_read_file_symlink_escape_denied(cx: &mut TestAppContext) {
+ init_test(cx);
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "project": {
+ "src": { "main.rs": "fn main() {}" }
+ },
+ "external": {
+ "secret.txt": "SECRET_KEY=abc123"
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(
+ path!("/root/project/secret_link.txt").as_ref(),
+ PathBuf::from("../external/secret.txt"),
+ )
+ .await
+ .unwrap();
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let action_log = cx.new(|_| ActionLog::new(project.clone()));
+ let context_server_registry =
+ cx.new(|cx| ContextServerRegistry::new(project.read(cx).context_server_store(), cx));
+ let model = Arc::new(FakeLanguageModel::default());
+ let thread = cx.new(|cx| {
+ Thread::new(
+ project.clone(),
+ cx.new(|_cx| ProjectContext::default()),
+ context_server_registry,
+ Templates::new(),
+ Some(model),
+ cx,
+ )
+ });
+ let tool = Arc::new(ReadFileTool::new(
+ thread.downgrade(),
+ project.clone(),
+ action_log,
+ ));
+
+ let (event_stream, mut event_rx) = ToolCallEventStream::test();
+ let task = cx.update(|cx| {
+ tool.clone().run(
+ ReadFileToolInput {
+ path: "project/secret_link.txt".to_string(),
+ start_line: None,
+ end_line: None,
+ },
+ event_stream,
+ cx,
+ )
+ });
+
+ let auth = event_rx.expect_authorization().await;
+ drop(auth);
+
+ let result = task.await;
+ assert!(
+ result.is_err(),
+ "Tool should fail when authorization is denied"
+ );
+ }
+
+ #[gpui::test]
+ async fn test_read_file_symlink_escape_private_path_no_authorization(cx: &mut TestAppContext) {
+ init_test(cx);
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "project": {
+ "src": { "main.rs": "fn main() {}" }
+ },
+ "external": {
+ "secret.txt": "SECRET_KEY=abc123"
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(
+ path!("/root/project/secret_link.txt").as_ref(),
+ PathBuf::from("../external/secret.txt"),
+ )
+ .await
+ .unwrap();
+
+ cx.update(|cx| {
+ settings::SettingsStore::update_global(cx, |store, cx| {
+ store.update_user_settings(cx, |settings| {
+ settings.project.worktree.private_files =
+ Some(vec!["**/secret_link.txt".to_string()].into());
+ });
+ });
+ });
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let action_log = cx.new(|_| ActionLog::new(project.clone()));
+ let context_server_registry =
+ cx.new(|cx| ContextServerRegistry::new(project.read(cx).context_server_store(), cx));
+ let model = Arc::new(FakeLanguageModel::default());
+ let thread = cx.new(|cx| {
+ Thread::new(
+ project.clone(),
+ cx.new(|_cx| ProjectContext::default()),
+ context_server_registry,
+ Templates::new(),
+ Some(model),
+ cx,
+ )
+ });
+ let tool = Arc::new(ReadFileTool::new(
+ thread.downgrade(),
+ project.clone(),
+ action_log,
+ ));
+
+ let (event_stream, mut event_rx) = ToolCallEventStream::test();
+ let result = cx
+ .update(|cx| {
+ tool.clone().run(
+ ReadFileToolInput {
+ path: "project/secret_link.txt".to_string(),
+ start_line: None,
+ end_line: None,
+ },
+ event_stream,
+ cx,
+ )
+ })
+ .await;
+
+ assert!(
+ result.is_err(),
+ "Expected read_file to fail on private path"
+ );
+ let error = result.unwrap_err().to_string();
+ assert!(
+ error.contains("private_files"),
+ "Expected private-files validation error, got: {error}"
+ );
+
+ let event = event_rx.try_next();
+ assert!(
+ !matches!(
+ event,
+ Ok(Some(Ok(crate::thread::ThreadEvent::ToolCallAuthorization(
+ _
+ ))))
+ ),
+ "No authorization should be requested when validation fails before read",
+ );
+ }
}
@@ -1,5 +1,7 @@
-use super::edit_file_tool::{
- SensitiveSettingsKind, is_sensitive_settings_path, sensitive_settings_kind,
+use super::tool_permissions::{
+ ResolvedProjectPath, SensitiveSettingsKind, authorize_symlink_access,
+ canonicalize_worktree_roots, path_has_symlink_escape, resolve_project_path,
+ sensitive_settings_kind,
};
use agent_client_protocol as acp;
use agent_settings::AgentSettings;
@@ -69,71 +71,94 @@ impl AgentTool for RestoreFileFromDiskTool {
event_stream: ToolCallEventStream,
cx: &mut App,
) -> Task<Result<String>> {
- let settings = AgentSettings::get_global(cx);
- let mut confirmation_paths: Vec<String> = Vec::new();
+ let settings = AgentSettings::get_global(cx).clone();
+ // Check for any immediate deny before spawning async work.
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());
+ let decision = decide_permission_for_path(Self::NAME, &path_str, &settings);
+ if let ToolPermissionDecision::Deny(reason) = decision {
+ return Task::ready(Err(anyhow::anyhow!("{}", reason)));
+ }
+ }
+
+ let project = self.project.clone();
+ let input_paths = input.paths;
+
+ cx.spawn(async move |cx| {
+ let fs = project.read_with(cx, |project, _cx| project.fs().clone());
+ let canonical_roots = canonicalize_worktree_roots(&project, &fs, cx).await;
+
+ let mut confirmation_paths: Vec<String> = 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);
+ let symlink_escape = project.read_with(cx, |project, cx| {
+ path_has_symlink_escape(project, path, &canonical_roots, cx)
+ });
+
+ match decision {
+ ToolPermissionDecision::Allow => {
+ if !symlink_escape {
+ let is_sensitive = super::tool_permissions::is_sensitive_settings_path(
+ Path::new(&*path_str),
+ fs.as_ref(),
+ )
+ .await;
+ if is_sensitive {
+ confirmation_paths.push(path_str.to_string());
+ }
+ }
+ }
+ ToolPermissionDecision::Deny(reason) => {
+ return Err(anyhow::anyhow!("{}", reason));
+ }
+ ToolPermissionDecision::Confirm => {
+ if !symlink_escape {
+ 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 {
+ if !confirmation_paths.is_empty() {
+ let title = if confirmation_paths.len() == 1 {
format!(
- "Restore {}, and {} more from disk",
- paths.join(", "),
- confirmation_paths.len() - 3
+ "Restore {} from disk",
+ MarkdownInlineCode(&confirmation_paths[0])
)
} 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;
+ 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(", "))
+ }
+ };
- cx.spawn(async move |cx| {
- if let Some(authorize) = authorize {
+ let mut settings_kind = None;
+ for p in &confirmation_paths {
+ if let Some(kind) = sensitive_settings_kind(Path::new(p), fs.as_ref()).await {
+ settings_kind = Some(kind);
+ break;
+ }
+ }
+ let title = match settings_kind {
+ Some(SensitiveSettingsKind::Local) => format!("{title} (local settings)"),
+ Some(SensitiveSettingsKind::Global) => format!("{title} (settings)"),
+ None => title,
+ };
+ let context = crate::ToolPermissionContext::new(Self::NAME, confirmation_paths);
+ let authorize = cx.update(|cx| event_stream.authorize(title, context, cx));
authorize.await?;
}
let mut buffers_to_reload: FxHashSet<Entity<Buffer>> = FxHashSet::default();
@@ -146,11 +171,40 @@ impl AgentTool for RestoreFileFromDiskTool {
let mut reload_errors: Vec<String> = Vec::new();
for path in input_paths {
- let Some(project_path) =
- project.read_with(cx, |project, cx| project.find_project_path(&path, cx))
- else {
- not_found_paths.push(path);
- continue;
+ let project_path = match project.read_with(cx, |project, cx| {
+ resolve_project_path(project, &path, &canonical_roots, cx)
+ }) {
+ Ok(resolved) => {
+ let (project_path, symlink_canonical_target) = match resolved {
+ ResolvedProjectPath::Safe(path) => (path, None),
+ ResolvedProjectPath::SymlinkEscape {
+ project_path,
+ canonical_target,
+ } => (project_path, Some(canonical_target)),
+ };
+ if let Some(canonical_target) = &symlink_canonical_target {
+ let path_str = path.to_string_lossy();
+ let authorize_task = cx.update(|cx| {
+ authorize_symlink_access(
+ Self::NAME,
+ &path_str,
+ canonical_target,
+ &event_stream,
+ cx,
+ )
+ });
+ let result = authorize_task.await;
+ if let Err(err) = result {
+ reload_errors.push(format!("{}: {}", path.to_string_lossy(), err));
+ continue;
+ }
+ }
+ project_path
+ }
+ Err(_) => {
+ not_found_paths.push(path);
+ continue;
+ }
};
let open_buffer_task =
@@ -246,7 +300,7 @@ impl AgentTool for RestoreFileFromDiskTool {
#[cfg(test)]
mod tests {
use super::*;
- use fs::Fs;
+ use fs::Fs as _;
use gpui::TestAppContext;
use language::LineEnding;
use project::FakeFs;
@@ -408,4 +462,197 @@ mod tests {
let _ = LineEnding::Unix; // keep import used if the buffer edit API changes
}
+
+ #[gpui::test]
+ async fn test_restore_file_symlink_escape_requests_authorization(cx: &mut TestAppContext) {
+ init_test(cx);
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "project": {
+ "src": {}
+ },
+ "external": {
+ "secret.txt": "secret content"
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(
+ path!("/root/project/link.txt").as_ref(),
+ PathBuf::from("../external/secret.txt"),
+ )
+ .await
+ .unwrap();
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let tool = Arc::new(RestoreFileFromDiskTool::new(project));
+
+ let (event_stream, mut event_rx) = ToolCallEventStream::test();
+ let task = cx.update(|cx| {
+ tool.clone().run(
+ RestoreFileFromDiskToolInput {
+ paths: vec![PathBuf::from("project/link.txt")],
+ },
+ event_stream,
+ cx,
+ )
+ });
+
+ cx.run_until_parked();
+
+ let auth = event_rx.expect_authorization().await;
+ let title = auth.tool_call.fields.title.as_deref().unwrap_or("");
+ assert!(
+ title.contains("points outside the project"),
+ "Expected symlink escape authorization, got: {title}",
+ );
+
+ auth.response
+ .send(acp::PermissionOptionId::new("allow"))
+ .unwrap();
+
+ let _result = task.await;
+ }
+
+ #[gpui::test]
+ async fn test_restore_file_symlink_escape_honors_deny_policy(cx: &mut TestAppContext) {
+ init_test(cx);
+ cx.update(|cx| {
+ let mut settings = AgentSettings::get_global(cx).clone();
+ settings.tool_permissions.tools.insert(
+ "restore_file_from_disk".into(),
+ agent_settings::ToolRules {
+ default: Some(settings::ToolPermissionMode::Deny),
+ ..Default::default()
+ },
+ );
+ AgentSettings::override_global(settings, cx);
+ });
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "project": {
+ "src": {}
+ },
+ "external": {
+ "secret.txt": "secret content"
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(
+ path!("/root/project/link.txt").as_ref(),
+ PathBuf::from("../external/secret.txt"),
+ )
+ .await
+ .unwrap();
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let tool = Arc::new(RestoreFileFromDiskTool::new(project));
+
+ let (event_stream, mut event_rx) = ToolCallEventStream::test();
+ let result = cx
+ .update(|cx| {
+ tool.clone().run(
+ RestoreFileFromDiskToolInput {
+ paths: vec![PathBuf::from("project/link.txt")],
+ },
+ event_stream,
+ cx,
+ )
+ })
+ .await;
+
+ assert!(result.is_err(), "Tool should fail when policy denies");
+ assert!(
+ !matches!(
+ event_rx.try_next(),
+ Ok(Some(Ok(crate::ThreadEvent::ToolCallAuthorization(_))))
+ ),
+ "Deny policy should not emit symlink authorization prompt",
+ );
+ }
+
+ #[gpui::test]
+ async fn test_restore_file_symlink_escape_confirm_requires_single_approval(
+ cx: &mut TestAppContext,
+ ) {
+ init_test(cx);
+ cx.update(|cx| {
+ let mut settings = AgentSettings::get_global(cx).clone();
+ settings.tool_permissions.default = settings::ToolPermissionMode::Confirm;
+ AgentSettings::override_global(settings, cx);
+ });
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "project": {
+ "src": {}
+ },
+ "external": {
+ "secret.txt": "secret content"
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(
+ path!("/root/project/link.txt").as_ref(),
+ PathBuf::from("../external/secret.txt"),
+ )
+ .await
+ .unwrap();
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let tool = Arc::new(RestoreFileFromDiskTool::new(project));
+
+ let (event_stream, mut event_rx) = ToolCallEventStream::test();
+ let task = cx.update(|cx| {
+ tool.clone().run(
+ RestoreFileFromDiskToolInput {
+ paths: vec![PathBuf::from("project/link.txt")],
+ },
+ event_stream,
+ cx,
+ )
+ });
+
+ cx.run_until_parked();
+
+ let auth = event_rx.expect_authorization().await;
+ let title = auth.tool_call.fields.title.as_deref().unwrap_or("");
+ assert!(
+ title.contains("points outside the project"),
+ "Expected symlink escape authorization, got: {title}",
+ );
+
+ auth.response
+ .send(acp::PermissionOptionId::new("allow"))
+ .unwrap();
+
+ assert!(
+ !matches!(
+ event_rx.try_next(),
+ Ok(Some(Ok(crate::ThreadEvent::ToolCallAuthorization(_))))
+ ),
+ "Expected a single authorization prompt",
+ );
+
+ let _result = task.await;
+ }
}
@@ -13,8 +13,10 @@ use std::path::{Path, PathBuf};
use std::sync::Arc;
use util::markdown::MarkdownInlineCode;
-use super::edit_file_tool::{
- SensitiveSettingsKind, is_sensitive_settings_path, sensitive_settings_kind,
+use super::tool_permissions::{
+ ResolvedProjectPath, SensitiveSettingsKind, authorize_symlink_access,
+ canonicalize_worktree_roots, path_has_symlink_escape, resolve_project_path,
+ sensitive_settings_kind,
};
use crate::{AgentTool, ToolCallEventStream, ToolPermissionDecision, decide_permission_for_path};
@@ -66,68 +68,92 @@ impl AgentTool for SaveFileTool {
event_stream: ToolCallEventStream,
cx: &mut App,
) -> Task<Result<String>> {
- let settings = AgentSettings::get_global(cx);
- let mut confirmation_paths: Vec<String> = Vec::new();
+ let settings = AgentSettings::get_global(cx).clone();
+ // Check for any immediate deny before spawning async work.
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 decision = decide_permission_for_path(Self::NAME, &path_str, &settings);
+ if let ToolPermissionDecision::Deny(reason) = decision {
+ return Task::ready(Err(anyhow::anyhow!("{}", reason)));
}
}
- let authorize = if !confirmation_paths.is_empty() {
- let title = if confirmation_paths.len() == 1 {
- format!("Save {}", 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!(
- "Save {}, and {} more",
- paths.join(", "),
- confirmation_paths.len() - 3
- )
- } else {
- format!("Save {}", 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.clone(),
- };
- 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 {
+ let fs = project.read_with(cx, |project, _cx| project.fs().clone());
+ let canonical_roots = canonicalize_worktree_roots(&project, &fs, cx).await;
+
+ let mut confirmation_paths: Vec<String> = 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);
+ let symlink_escape = project.read_with(cx, |project, cx| {
+ path_has_symlink_escape(project, path, &canonical_roots, cx)
+ });
+
+ match decision {
+ ToolPermissionDecision::Allow => {
+ if !symlink_escape {
+ let is_sensitive = super::tool_permissions::is_sensitive_settings_path(
+ Path::new(&*path_str),
+ fs.as_ref(),
+ )
+ .await;
+ if is_sensitive {
+ confirmation_paths.push(path_str.to_string());
+ }
+ }
+ }
+ ToolPermissionDecision::Deny(reason) => {
+ return Err(anyhow::anyhow!("{}", reason));
+ }
+ ToolPermissionDecision::Confirm => {
+ if !symlink_escape {
+ confirmation_paths.push(path_str.to_string());
+ }
+ }
+ }
+ }
+
+ if !confirmation_paths.is_empty() {
+ let title = if confirmation_paths.len() == 1 {
+ format!("Save {}", 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!(
+ "Save {}, and {} more",
+ paths.join(", "),
+ confirmation_paths.len() - 3
+ )
+ } else {
+ format!("Save {}", paths.join(", "))
+ }
+ };
+
+ let mut settings_kind = None;
+ for p in &confirmation_paths {
+ if let Some(kind) = sensitive_settings_kind(Path::new(p), fs.as_ref()).await {
+ settings_kind = Some(kind);
+ break;
+ }
+ }
+ let title = match settings_kind {
+ Some(SensitiveSettingsKind::Local) => format!("{title} (local settings)"),
+ Some(SensitiveSettingsKind::Global) => format!("{title} (settings)"),
+ None => title,
+ };
+ let context =
+ crate::ToolPermissionContext::new(Self::NAME, confirmation_paths.clone());
+ let authorize = cx.update(|cx| event_stream.authorize(title, context, cx));
authorize.await?;
}
@@ -137,14 +163,44 @@ impl AgentTool for SaveFileTool {
let mut clean_paths: Vec<PathBuf> = Vec::new();
let mut not_found_paths: Vec<PathBuf> = Vec::new();
let mut open_errors: Vec<(PathBuf, String)> = Vec::new();
+ let mut authorization_errors: Vec<(PathBuf, String)> = Vec::new();
let mut save_errors: Vec<(String, String)> = Vec::new();
for path in input_paths {
- let Some(project_path) =
- project.read_with(cx, |project, cx| project.find_project_path(&path, cx))
- else {
- not_found_paths.push(path);
- continue;
+ let project_path = match project.read_with(cx, |project, cx| {
+ resolve_project_path(project, &path, &canonical_roots, cx)
+ }) {
+ Ok(resolved) => {
+ let (project_path, symlink_canonical_target) = match resolved {
+ ResolvedProjectPath::Safe(path) => (path, None),
+ ResolvedProjectPath::SymlinkEscape {
+ project_path,
+ canonical_target,
+ } => (project_path, Some(canonical_target)),
+ };
+ if let Some(canonical_target) = &symlink_canonical_target {
+ let path_str = path.to_string_lossy();
+ let authorize_task = cx.update(|cx| {
+ authorize_symlink_access(
+ Self::NAME,
+ &path_str,
+ canonical_target,
+ &event_stream,
+ cx,
+ )
+ });
+ let result = authorize_task.await;
+ if let Err(err) = result {
+ authorization_errors.push((path.clone(), err.to_string()));
+ continue;
+ }
+ }
+ project_path
+ }
+ Err(_) => {
+ not_found_paths.push(path);
+ continue;
+ }
};
let open_buffer_task =
@@ -221,6 +277,15 @@ impl AgentTool for SaveFileTool {
lines.push(format!("- {}: {}", path.display(), error));
}
}
+ if !authorization_errors.is_empty() {
+ lines.push(format!(
+ "Authorization failed ({}):",
+ authorization_errors.len()
+ ));
+ for (path, error) in &authorization_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 {
@@ -240,7 +305,7 @@ impl AgentTool for SaveFileTool {
#[cfg(test)]
mod tests {
use super::*;
- use fs::Fs;
+ use fs::Fs as _;
use gpui::TestAppContext;
use project::FakeFs;
use serde_json::json;
@@ -392,4 +457,282 @@ mod tests {
"expected not-found path bullet, got:\n{output}"
);
}
+
+ #[gpui::test]
+ async fn test_save_file_symlink_escape_requests_authorization(cx: &mut TestAppContext) {
+ init_test(cx);
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "project": {
+ "src": {}
+ },
+ "external": {
+ "secret.txt": "secret content"
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(
+ path!("/root/project/link.txt").as_ref(),
+ PathBuf::from("../external/secret.txt"),
+ )
+ .await
+ .unwrap();
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let tool = Arc::new(SaveFileTool::new(project));
+
+ let (event_stream, mut event_rx) = ToolCallEventStream::test();
+ let task = cx.update(|cx| {
+ tool.clone().run(
+ SaveFileToolInput {
+ paths: vec![PathBuf::from("project/link.txt")],
+ },
+ event_stream,
+ cx,
+ )
+ });
+
+ cx.run_until_parked();
+
+ let auth = event_rx.expect_authorization().await;
+ let title = auth.tool_call.fields.title.as_deref().unwrap_or("");
+ assert!(
+ title.contains("points outside the project"),
+ "Expected symlink escape authorization, got: {title}",
+ );
+
+ auth.response
+ .send(acp::PermissionOptionId::new("allow"))
+ .unwrap();
+
+ let _result = task.await;
+ }
+
+ #[gpui::test]
+ async fn test_save_file_symlink_escape_honors_deny_policy(cx: &mut TestAppContext) {
+ init_test(cx);
+ cx.update(|cx| {
+ let mut settings = AgentSettings::get_global(cx).clone();
+ settings.tool_permissions.tools.insert(
+ "save_file".into(),
+ agent_settings::ToolRules {
+ default: Some(settings::ToolPermissionMode::Deny),
+ ..Default::default()
+ },
+ );
+ AgentSettings::override_global(settings, cx);
+ });
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "project": {
+ "src": {}
+ },
+ "external": {
+ "secret.txt": "secret content"
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(
+ path!("/root/project/link.txt").as_ref(),
+ PathBuf::from("../external/secret.txt"),
+ )
+ .await
+ .unwrap();
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let tool = Arc::new(SaveFileTool::new(project));
+
+ let (event_stream, mut event_rx) = ToolCallEventStream::test();
+ let result = cx
+ .update(|cx| {
+ tool.clone().run(
+ SaveFileToolInput {
+ paths: vec![PathBuf::from("project/link.txt")],
+ },
+ event_stream,
+ cx,
+ )
+ })
+ .await;
+
+ assert!(result.is_err(), "Tool should fail when policy denies");
+ assert!(
+ !matches!(
+ event_rx.try_next(),
+ Ok(Some(Ok(crate::ThreadEvent::ToolCallAuthorization(_))))
+ ),
+ "Deny policy should not emit symlink authorization prompt",
+ );
+ }
+
+ #[gpui::test]
+ async fn test_save_file_symlink_escape_confirm_requires_single_approval(
+ cx: &mut TestAppContext,
+ ) {
+ init_test(cx);
+ cx.update(|cx| {
+ let mut settings = AgentSettings::get_global(cx).clone();
+ settings.tool_permissions.default = settings::ToolPermissionMode::Confirm;
+ AgentSettings::override_global(settings, cx);
+ });
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "project": {
+ "src": {}
+ },
+ "external": {
+ "secret.txt": "secret content"
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(
+ path!("/root/project/link.txt").as_ref(),
+ PathBuf::from("../external/secret.txt"),
+ )
+ .await
+ .unwrap();
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let tool = Arc::new(SaveFileTool::new(project));
+
+ let (event_stream, mut event_rx) = ToolCallEventStream::test();
+ let task = cx.update(|cx| {
+ tool.clone().run(
+ SaveFileToolInput {
+ paths: vec![PathBuf::from("project/link.txt")],
+ },
+ event_stream,
+ cx,
+ )
+ });
+
+ cx.run_until_parked();
+
+ let auth = event_rx.expect_authorization().await;
+ let title = auth.tool_call.fields.title.as_deref().unwrap_or("");
+ assert!(
+ title.contains("points outside the project"),
+ "Expected symlink escape authorization, got: {title}",
+ );
+
+ auth.response
+ .send(acp::PermissionOptionId::new("allow"))
+ .unwrap();
+
+ assert!(
+ !matches!(
+ event_rx.try_next(),
+ Ok(Some(Ok(crate::ThreadEvent::ToolCallAuthorization(_))))
+ ),
+ "Expected a single authorization prompt",
+ );
+
+ let _result = task.await;
+ }
+
+ #[gpui::test]
+ async fn test_save_file_symlink_denial_does_not_reduce_success_count(cx: &mut TestAppContext) {
+ init_test(cx);
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "project": {
+ "dirty.txt": "on disk value\n",
+ },
+ "external": {
+ "secret.txt": "secret content"
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(
+ path!("/root/project/link.txt").as_ref(),
+ PathBuf::from("../external/secret.txt"),
+ )
+ .await
+ .unwrap();
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.executor().run_until_parked();
+
+ let dirty_project_path = project.read_with(cx, |project, cx| {
+ project
+ .find_project_path("project/dirty.txt", cx)
+ .expect("dirty.txt should exist in project")
+ });
+ let dirty_buffer = project
+ .update(cx, |project, cx| {
+ project.open_buffer(dirty_project_path, cx)
+ })
+ .await
+ .unwrap();
+ dirty_buffer.update(cx, |buffer, cx| {
+ buffer.edit([(0..buffer.len(), "in memory value\n")], None, cx);
+ });
+ assert!(
+ dirty_buffer.read_with(cx, |buffer, _| buffer.is_dirty()),
+ "dirty.txt should be dirty before save"
+ );
+
+ let tool = Arc::new(SaveFileTool::new(project));
+
+ let (event_stream, mut event_rx) = ToolCallEventStream::test();
+ let task = cx.update(|cx| {
+ tool.clone().run(
+ SaveFileToolInput {
+ paths: vec![
+ PathBuf::from("project/dirty.txt"),
+ PathBuf::from("project/link.txt"),
+ ],
+ },
+ event_stream,
+ cx,
+ )
+ });
+
+ cx.run_until_parked();
+
+ let auth = event_rx.expect_authorization().await;
+ auth.response
+ .send(acp::PermissionOptionId::new("deny"))
+ .unwrap();
+
+ let output = task.await.unwrap();
+ assert!(
+ output.contains("Saved 1 file(s)."),
+ "Expected successful save count to remain accurate, got:\n{output}",
+ );
+ assert!(
+ output.contains("Authorization failed (1):"),
+ "Expected authorization failure section, got:\n{output}",
+ );
+ assert!(
+ !output.contains("Save failed"),
+ "Authorization denials should not be counted as save failures, got:\n{output}",
+ );
+ }
}
@@ -180,7 +180,7 @@ impl StreamingEditFileTool {
event_stream: &ToolCallEventStream,
cx: &mut App,
) -> Task<Result<()>> {
- super::edit_file_tool::authorize_file_edit(
+ super::tool_permissions::authorize_file_edit(
EditFileTool::NAME,
&input.path,
&input.display_description,
@@ -107,10 +107,8 @@ impl AgentTool for TerminalTool {
return Task::ready(Err(anyhow::anyhow!("{}", reason)));
}
ToolPermissionDecision::Confirm => {
- let context = crate::ToolPermissionContext {
- tool_name: Self::NAME.to_string(),
- input_values: vec![input.command.clone()],
- };
+ let context =
+ crate::ToolPermissionContext::new(Self::NAME, vec![input.command.clone()]);
Some(event_stream.authorize(self.initial_title(Ok(input.clone()), cx), context, cx))
}
};
@@ -0,0 +1,837 @@
+use crate::{
+ Thread, ToolCallEventStream, ToolPermissionContext, ToolPermissionDecision,
+ decide_permission_for_path,
+};
+use anyhow::{Result, anyhow};
+use fs::Fs;
+use gpui::{App, Entity, Task, WeakEntity};
+use project::{Project, ProjectPath};
+use settings::Settings;
+use std::ffi::OsStr;
+use std::path::{Path, PathBuf};
+use std::sync::Arc;
+
+pub enum SensitiveSettingsKind {
+ Local,
+ Global,
+}
+
+/// Result of resolving a path within the project with symlink safety checks.
+///
+/// See [`resolve_project_path`].
+#[derive(Debug, Clone)]
+pub enum ResolvedProjectPath {
+ /// The path resolves to a location safely within the project boundaries.
+ Safe(ProjectPath),
+ /// The path resolves through a symlink to a location outside the project.
+ /// Agent tools should prompt the user before proceeding with access.
+ SymlinkEscape {
+ /// The project-relative path (before symlink resolution).
+ project_path: ProjectPath,
+ /// The canonical (real) filesystem path the symlink points to.
+ canonical_target: PathBuf,
+ },
+}
+
+/// Asynchronously canonicalizes the absolute paths of all worktrees in a
+/// project using the provided `Fs`. The returned paths can be passed to
+/// [`resolve_project_path`] and related helpers so that they don't need to
+/// perform blocking filesystem I/O themselves.
+pub async fn canonicalize_worktree_roots<C: gpui::AppContext>(
+ project: &Entity<Project>,
+ fs: &Arc<dyn Fs>,
+ cx: &C,
+) -> Vec<PathBuf> {
+ let abs_paths: Vec<Arc<Path>> = project.read_with(cx, |project, cx| {
+ project
+ .worktrees(cx)
+ .map(|worktree| worktree.read(cx).abs_path())
+ .collect()
+ });
+
+ let mut canonical_roots = Vec::with_capacity(abs_paths.len());
+ for abs_path in &abs_paths {
+ match fs.canonicalize(abs_path).await {
+ Ok(canonical) => canonical_roots.push(canonical),
+ Err(_) => canonical_roots.push(abs_path.to_path_buf()),
+ }
+ }
+ canonical_roots
+}
+
+/// Walks up ancestors of `path` to find the deepest one that exists on disk and
+/// can be canonicalized, then reattaches the remaining suffix components.
+///
+/// This is needed for paths where the leaf (or intermediate directories) don't
+/// exist yet but an ancestor may be a symlink. For example, when creating
+/// `.zed/settings.json` where `.zed` is a symlink to an external directory.
+///
+/// Note: intermediate directories *can* be symlinks (not just leaf entries),
+/// so we must walk the full ancestor chain. For example:
+/// `ln -s /external/config /project/.zed`
+/// makes `.zed` an intermediate symlink directory.
+async fn canonicalize_with_ancestors(path: &Path, fs: &dyn Fs) -> Option<PathBuf> {
+ let mut current: Option<&Path> = Some(path);
+ let mut suffix_components = Vec::new();
+ loop {
+ match current {
+ Some(ancestor) => match fs.canonicalize(ancestor).await {
+ Ok(canonical) => {
+ let mut result = canonical;
+ for component in suffix_components.into_iter().rev() {
+ result.push(component);
+ }
+ return Some(result);
+ }
+ Err(_) => {
+ if let Some(file_name) = ancestor.file_name() {
+ suffix_components.push(file_name.to_os_string());
+ }
+ current = ancestor.parent();
+ }
+ },
+ None => return None,
+ }
+ }
+}
+
+fn is_within_any_worktree(canonical_path: &Path, canonical_worktree_roots: &[PathBuf]) -> bool {
+ canonical_worktree_roots
+ .iter()
+ .any(|root| canonical_path.starts_with(root))
+}
+
+/// 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 async fn sensitive_settings_kind(path: &Path, fs: &dyn Fs) -> 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);
+ }
+
+ if let Some(canonical_path) = canonicalize_with_ancestors(path, fs).await {
+ let config_dir = fs
+ .canonicalize(paths::config_dir())
+ .await
+ .unwrap_or_else(|_| paths::config_dir().to_path_buf());
+ if canonical_path.starts_with(&config_dir) {
+ return Some(SensitiveSettingsKind::Global);
+ }
+ }
+
+ None
+}
+
+pub async fn is_sensitive_settings_path(path: &Path, fs: &dyn Fs) -> bool {
+ sensitive_settings_kind(path, fs).await.is_some()
+}
+
+/// Resolves a path within the project, checking for symlink escapes.
+///
+/// This is the primary entry point for agent tools that need to resolve a
+/// user-provided path string into a validated `ProjectPath`. It combines
+/// path lookup (`find_project_path`) with symlink safety verification.
+///
+/// `canonical_worktree_roots` should be obtained from
+/// [`canonicalize_worktree_roots`] before calling this function so that no
+/// blocking I/O is needed here.
+///
+/// # Returns
+///
+/// - `Ok(ResolvedProjectPath::Safe(project_path))` — the path resolves to a
+/// location within the project boundaries.
+/// - `Ok(ResolvedProjectPath::SymlinkEscape { .. })` — the path resolves
+/// through a symlink to a location outside the project. Agent tools should
+/// prompt the user before proceeding.
+/// - `Err(..)` — the path could not be found in the project or could not be
+/// verified. The error message is suitable for returning to the model.
+pub fn resolve_project_path(
+ project: &Project,
+ path: impl AsRef<Path>,
+ canonical_worktree_roots: &[PathBuf],
+ cx: &App,
+) -> Result<ResolvedProjectPath> {
+ let path = path.as_ref();
+ let project_path = project
+ .find_project_path(path, cx)
+ .ok_or_else(|| anyhow!("Path {} is not in the project", path.display()))?;
+
+ let worktree = project
+ .worktree_for_id(project_path.worktree_id, cx)
+ .ok_or_else(|| anyhow!("Could not resolve path {}", path.display()))?;
+ let snapshot = worktree.read(cx);
+
+ // Fast path: if the entry exists in the snapshot and is not marked
+ // external, we know it's safe (the background scanner already verified).
+ if let Some(entry) = snapshot.entry_for_path(&project_path.path) {
+ if !entry.is_external {
+ return Ok(ResolvedProjectPath::Safe(project_path));
+ }
+
+ // Entry is external (set by the worktree scanner when a symlink's
+ // canonical target is outside the worktree root). Return the
+ // canonical path if the entry has one, otherwise fall through to
+ // filesystem-level canonicalization.
+ if let Some(canonical) = &entry.canonical_path {
+ if is_within_any_worktree(canonical.as_ref(), canonical_worktree_roots) {
+ return Ok(ResolvedProjectPath::Safe(project_path));
+ }
+
+ return Ok(ResolvedProjectPath::SymlinkEscape {
+ project_path,
+ canonical_target: canonical.to_path_buf(),
+ });
+ }
+ }
+
+ // For missing/create-mode paths (or external descendants without their own
+ // canonical_path), resolve symlink safety through snapshot metadata rather
+ // than std::fs canonicalization. This keeps behavior correct for non-local
+ // worktrees and in-memory fs backends.
+ for ancestor in project_path.path.ancestors() {
+ let Some(ancestor_entry) = snapshot.entry_for_path(ancestor) else {
+ continue;
+ };
+
+ if !ancestor_entry.is_external {
+ return Ok(ResolvedProjectPath::Safe(project_path));
+ }
+
+ let Some(canonical_ancestor) = ancestor_entry.canonical_path.as_ref() else {
+ continue;
+ };
+
+ let suffix = project_path.path.strip_prefix(ancestor).map_err(|_| {
+ anyhow!(
+ "Path {} could not be resolved in the project",
+ path.display()
+ )
+ })?;
+
+ let canonical_target = if suffix.is_empty() {
+ canonical_ancestor.to_path_buf()
+ } else {
+ canonical_ancestor.join(suffix.as_std_path())
+ };
+
+ if is_within_any_worktree(&canonical_target, canonical_worktree_roots) {
+ return Ok(ResolvedProjectPath::Safe(project_path));
+ }
+
+ return Ok(ResolvedProjectPath::SymlinkEscape {
+ project_path,
+ canonical_target,
+ });
+ }
+
+ Ok(ResolvedProjectPath::Safe(project_path))
+}
+
+/// Prompts the user for permission when a path resolves through a symlink to a
+/// location outside the project. This check is an additional gate after
+/// settings-based deny decisions: even if a tool is configured as "always allow,"
+/// a symlink escape still requires explicit user approval.
+pub fn authorize_symlink_access(
+ tool_name: &str,
+ display_path: &str,
+ canonical_target: &Path,
+ event_stream: &ToolCallEventStream,
+ cx: &mut App,
+) -> Task<Result<()>> {
+ let title = format!(
+ "`{}` points outside the project (symlink to `{}`)",
+ display_path,
+ canonical_target.display(),
+ );
+
+ let context = ToolPermissionContext::symlink_target(
+ tool_name,
+ vec![canonical_target.display().to_string()],
+ );
+
+ event_stream.authorize(title, context, cx)
+}
+
+/// Creates a single authorization prompt for multiple symlink escapes.
+/// Each escape is a `(display_path, canonical_target)` pair.
+///
+/// Accepts `&[(&str, PathBuf)]` to match the natural return type of
+/// [`detect_symlink_escape`], avoiding intermediate owned-to-borrowed
+/// conversions at call sites.
+pub fn authorize_symlink_escapes(
+ tool_name: &str,
+ escapes: &[(&str, PathBuf)],
+ event_stream: &ToolCallEventStream,
+ cx: &mut App,
+) -> Task<Result<()>> {
+ debug_assert!(!escapes.is_empty());
+
+ if escapes.len() == 1 {
+ return authorize_symlink_access(tool_name, escapes[0].0, &escapes[0].1, event_stream, cx);
+ }
+
+ let targets = escapes
+ .iter()
+ .map(|(path, target)| format!("`{}` → `{}`", path, target.display()))
+ .collect::<Vec<_>>()
+ .join(" and ");
+ let title = format!("{} (symlinks outside project)", targets);
+
+ let context = ToolPermissionContext::symlink_target(
+ tool_name,
+ escapes
+ .iter()
+ .map(|(_, target)| target.display().to_string())
+ .collect(),
+ );
+
+ event_stream.authorize(title, context, cx)
+}
+
+/// Checks whether a path escapes the project via symlink, without creating
+/// an authorization task. Useful for pre-filtering paths before settings checks.
+pub fn path_has_symlink_escape(
+ project: &Project,
+ path: impl AsRef<Path>,
+ canonical_worktree_roots: &[PathBuf],
+ cx: &App,
+) -> bool {
+ matches!(
+ resolve_project_path(project, path, canonical_worktree_roots, cx),
+ Ok(ResolvedProjectPath::SymlinkEscape { .. })
+ )
+}
+
+/// Collects symlink escape info for a path without creating an authorization task.
+/// Returns `Some((display_path, canonical_target))` if the path escapes via symlink.
+pub fn detect_symlink_escape<'a>(
+ project: &Project,
+ display_path: &'a str,
+ canonical_worktree_roots: &[PathBuf],
+ cx: &App,
+) -> Option<(&'a str, PathBuf)> {
+ match resolve_project_path(project, display_path, canonical_worktree_roots, cx).ok()? {
+ ResolvedProjectPath::Safe(_) => None,
+ ResolvedProjectPath::SymlinkEscape {
+ canonical_target, ..
+ } => Some((display_path, canonical_target)),
+ }
+}
+
+/// Collects symlink escape info for two paths (source and destination) and
+/// returns any escapes found. This deduplicates the common pattern used by
+/// tools that operate on two paths (copy, move).
+///
+/// Returns a `Vec` of `(display_path, canonical_target)` pairs for paths
+/// that escape the project via symlink. The returned vec borrows the display
+/// paths from the input strings.
+pub fn collect_symlink_escapes<'a>(
+ project: &Project,
+ source_path: &'a str,
+ destination_path: &'a str,
+ canonical_worktree_roots: &[PathBuf],
+ cx: &App,
+) -> Vec<(&'a str, PathBuf)> {
+ let mut escapes = Vec::new();
+ if let Some(escape) = detect_symlink_escape(project, source_path, canonical_worktree_roots, cx)
+ {
+ escapes.push(escape);
+ }
+ if let Some(escape) =
+ detect_symlink_escape(project, destination_path, canonical_worktree_roots, cx)
+ {
+ escapes.push(escape);
+ }
+ escapes
+}
+
+/// Checks authorization for file edits, handling symlink escapes and
+/// sensitive settings paths.
+///
+/// # Authorization precedence
+///
+/// When a symlink escape is detected, the symlink authorization prompt
+/// *replaces* (rather than supplements) the normal tool-permission prompt.
+/// This is intentional: the symlink prompt already requires explicit user
+/// approval and displays the canonical target, which provides strictly more
+/// security-relevant information than the generic tool confirmation. Requiring
+/// two sequential prompts for the same operation would degrade UX without
+/// meaningfully improving security, since the user must already approve the
+/// more specific symlink-escape prompt.
+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)));
+ }
+
+ let path_owned = path.to_path_buf();
+ let display_description = display_description.to_string();
+ let tool_name = tool_name.to_string();
+ let thread = thread.clone();
+ let event_stream = event_stream.clone();
+
+ // The local settings folder check is synchronous (pure path inspection),
+ // so we can handle this common case without spawning.
+ let local_settings_folder = paths::local_settings_folder_name();
+ let is_local_settings = path.components().any(|component| {
+ component.as_os_str() == <_ as AsRef<OsStr>>::as_ref(&local_settings_folder)
+ });
+
+ cx.spawn(async move |cx| {
+ // Resolve the path and check for symlink escapes.
+ let (project_entity, fs) = thread.read_with(cx, |thread, cx| {
+ let project = thread.project().clone();
+ let fs = project.read(cx).fs().clone();
+ (project, fs)
+ })?;
+
+ let canonical_roots = canonicalize_worktree_roots(&project_entity, &fs, cx).await;
+
+ let resolved = project_entity.read_with(cx, |project, cx| {
+ resolve_project_path(project, &path_owned, &canonical_roots, cx)
+ });
+
+ if let Ok(ResolvedProjectPath::SymlinkEscape {
+ canonical_target, ..
+ }) = &resolved
+ {
+ let authorize = cx.update(|cx| {
+ authorize_symlink_access(
+ &tool_name,
+ &path_owned.to_string_lossy(),
+ canonical_target,
+ &event_stream,
+ cx,
+ )
+ });
+ return authorize.await;
+ }
+
+ // Create-mode paths may not resolve yet, so also inspect the parent path
+ // for symlink escapes before applying settings-based allow decisions.
+ if resolved.is_err() {
+ if let Some(parent_path) = path_owned.parent() {
+ let parent_resolved = project_entity.read_with(cx, |project, cx| {
+ resolve_project_path(project, parent_path, &canonical_roots, cx)
+ });
+
+ if let Ok(ResolvedProjectPath::SymlinkEscape {
+ canonical_target, ..
+ }) = &parent_resolved
+ {
+ let authorize = cx.update(|cx| {
+ authorize_symlink_access(
+ &tool_name,
+ &path_owned.to_string_lossy(),
+ canonical_target,
+ &event_stream,
+ cx,
+ )
+ });
+ return authorize.await;
+ }
+ }
+ }
+
+ let explicitly_allowed = matches!(decision, ToolPermissionDecision::Allow);
+
+ // Check sensitive settings asynchronously.
+ let settings_kind = if is_local_settings {
+ Some(SensitiveSettingsKind::Local)
+ } else {
+ sensitive_settings_kind(&path_owned, fs.as_ref()).await
+ };
+
+ let is_sensitive = settings_kind.is_some();
+ if explicitly_allowed && !is_sensitive {
+ return Ok(());
+ }
+
+ match settings_kind {
+ Some(SensitiveSettingsKind::Local) => {
+ let authorize = cx.update(|cx| {
+ let context = ToolPermissionContext::new(
+ &tool_name,
+ vec![path_owned.to_string_lossy().to_string()],
+ );
+ event_stream.authorize(
+ format!("{} (local settings)", display_description),
+ context,
+ cx,
+ )
+ });
+ return authorize.await;
+ }
+ Some(SensitiveSettingsKind::Global) => {
+ let authorize = cx.update(|cx| {
+ let context = ToolPermissionContext::new(
+ &tool_name,
+ vec![path_owned.to_string_lossy().to_string()],
+ );
+ event_stream.authorize(
+ format!("{} (settings)", display_description),
+ context,
+ cx,
+ )
+ });
+ return authorize.await;
+ }
+ None => {}
+ }
+
+ match resolved {
+ Ok(_) => Ok(()),
+ Err(_) => {
+ let authorize = cx.update(|cx| {
+ let context = ToolPermissionContext::new(
+ &tool_name,
+ vec![path_owned.to_string_lossy().to_string()],
+ );
+ event_stream.authorize(&display_description, context, cx)
+ });
+ authorize.await
+ }
+ }
+ })
+}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+ use fs::Fs;
+ use gpui::TestAppContext;
+ use project::{FakeFs, Project};
+ use serde_json::json;
+ use settings::SettingsStore;
+ use util::path;
+
+ fn init_test(cx: &mut TestAppContext) {
+ cx.update(|cx| {
+ let settings_store = SettingsStore::test(cx);
+ cx.set_global(settings_store);
+ });
+ }
+
+ async fn worktree_roots(
+ project: &Entity<Project>,
+ fs: &Arc<dyn Fs>,
+ cx: &TestAppContext,
+ ) -> Vec<PathBuf> {
+ let abs_paths: Vec<Arc<Path>> = project.read_with(cx, |project, cx| {
+ project
+ .worktrees(cx)
+ .map(|wt| wt.read(cx).abs_path())
+ .collect()
+ });
+
+ let mut roots = Vec::with_capacity(abs_paths.len());
+ for p in &abs_paths {
+ match fs.canonicalize(p).await {
+ Ok(c) => roots.push(c),
+ Err(_) => roots.push(p.to_path_buf()),
+ }
+ }
+ roots
+ }
+
+ #[gpui::test]
+ async fn test_resolve_project_path_safe_for_normal_files(cx: &mut TestAppContext) {
+ init_test(cx);
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root/project"),
+ json!({
+ "src": {
+ "main.rs": "fn main() {}",
+ "lib.rs": "pub fn hello() {}"
+ },
+ "README.md": "# Project"
+ }),
+ )
+ .await;
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.run_until_parked();
+ let fs_arc: Arc<dyn Fs> = fs;
+ let roots = worktree_roots(&project, &fs_arc, cx).await;
+
+ cx.read(|cx| {
+ let project = project.read(cx);
+
+ let resolved = resolve_project_path(project, "project/src/main.rs", &roots, cx)
+ .expect("should resolve normal file");
+ assert!(
+ matches!(resolved, ResolvedProjectPath::Safe(_)),
+ "normal file should be Safe, got: {:?}",
+ resolved
+ );
+
+ let resolved = resolve_project_path(project, "project/README.md", &roots, cx)
+ .expect("should resolve readme");
+ assert!(
+ matches!(resolved, ResolvedProjectPath::Safe(_)),
+ "readme should be Safe, got: {:?}",
+ resolved
+ );
+ });
+ }
+
+ #[gpui::test]
+ async fn test_resolve_project_path_detects_symlink_escape(cx: &mut TestAppContext) {
+ init_test(cx);
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "project": {
+ "src": {
+ "main.rs": "fn main() {}"
+ }
+ },
+ "external": {
+ "secret.txt": "top secret"
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(path!("/root/project/link").as_ref(), "../external".into())
+ .await
+ .expect("should create symlink");
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.run_until_parked();
+ let fs_arc: Arc<dyn Fs> = fs;
+ let roots = worktree_roots(&project, &fs_arc, cx).await;
+
+ cx.read(|cx| {
+ let project = project.read(cx);
+
+ let resolved = resolve_project_path(project, "project/link", &roots, cx)
+ .expect("should resolve symlink path");
+ match &resolved {
+ ResolvedProjectPath::SymlinkEscape {
+ canonical_target, ..
+ } => {
+ assert_eq!(
+ canonical_target,
+ Path::new(path!("/root/external")),
+ "canonical target should point to external directory"
+ );
+ }
+ ResolvedProjectPath::Safe(_) => {
+ panic!("symlink escaping project should be detected as SymlinkEscape");
+ }
+ }
+ });
+ }
+
+ #[gpui::test]
+ async fn test_resolve_project_path_allows_intra_project_symlinks(cx: &mut TestAppContext) {
+ init_test(cx);
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root/project"),
+ json!({
+ "real_dir": {
+ "file.txt": "hello"
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(path!("/root/project/link_dir").as_ref(), "real_dir".into())
+ .await
+ .expect("should create symlink");
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.run_until_parked();
+ let fs_arc: Arc<dyn Fs> = fs;
+ let roots = worktree_roots(&project, &fs_arc, cx).await;
+
+ cx.read(|cx| {
+ let project = project.read(cx);
+
+ let resolved = resolve_project_path(project, "project/link_dir", &roots, cx)
+ .expect("should resolve intra-project symlink");
+ assert!(
+ matches!(resolved, ResolvedProjectPath::Safe(_)),
+ "intra-project symlink should be Safe, got: {:?}",
+ resolved
+ );
+ });
+ }
+
+ #[gpui::test]
+ async fn test_resolve_project_path_missing_child_under_external_symlink(
+ cx: &mut TestAppContext,
+ ) {
+ init_test(cx);
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "project": {},
+ "external": {
+ "existing.txt": "hello"
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(path!("/root/project/link").as_ref(), "../external".into())
+ .await
+ .expect("should create symlink");
+
+ let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
+ cx.run_until_parked();
+ let fs_arc: Arc<dyn Fs> = fs;
+ let roots = worktree_roots(&project, &fs_arc, cx).await;
+
+ cx.read(|cx| {
+ let project = project.read(cx);
+
+ let resolved = resolve_project_path(project, "project/link/new_dir", &roots, cx)
+ .expect("should resolve missing child path under symlink");
+ match resolved {
+ ResolvedProjectPath::SymlinkEscape {
+ canonical_target, ..
+ } => {
+ assert_eq!(
+ canonical_target,
+ Path::new(path!("/root/external/new_dir")),
+ "missing child path should resolve to escaped canonical target",
+ );
+ }
+ ResolvedProjectPath::Safe(_) => {
+ panic!("missing child under external symlink should be SymlinkEscape");
+ }
+ }
+ });
+ }
+
+ #[gpui::test]
+ async fn test_resolve_project_path_allows_cross_worktree_symlinks(cx: &mut TestAppContext) {
+ init_test(cx);
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "worktree_one": {},
+ "worktree_two": {
+ "shared_dir": {
+ "file.txt": "hello"
+ }
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(
+ path!("/root/worktree_one/link_to_worktree_two").as_ref(),
+ PathBuf::from("../worktree_two/shared_dir"),
+ )
+ .await
+ .expect("should create symlink");
+
+ let project = Project::test(
+ fs.clone(),
+ [
+ path!("/root/worktree_one").as_ref(),
+ path!("/root/worktree_two").as_ref(),
+ ],
+ cx,
+ )
+ .await;
+ cx.run_until_parked();
+ let fs_arc: Arc<dyn Fs> = fs;
+ let roots = worktree_roots(&project, &fs_arc, cx).await;
+
+ cx.read(|cx| {
+ let project = project.read(cx);
+
+ let resolved =
+ resolve_project_path(project, "worktree_one/link_to_worktree_two", &roots, cx)
+ .expect("should resolve cross-worktree symlink");
+ assert!(
+ matches!(resolved, ResolvedProjectPath::Safe(_)),
+ "cross-worktree symlink should be Safe, got: {:?}",
+ resolved
+ );
+ });
+ }
+
+ #[gpui::test]
+ async fn test_resolve_project_path_missing_child_under_cross_worktree_symlink(
+ cx: &mut TestAppContext,
+ ) {
+ init_test(cx);
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree(
+ path!("/root"),
+ json!({
+ "worktree_one": {},
+ "worktree_two": {
+ "shared_dir": {}
+ }
+ }),
+ )
+ .await;
+
+ fs.create_symlink(
+ path!("/root/worktree_one/link_to_worktree_two").as_ref(),
+ PathBuf::from("../worktree_two/shared_dir"),
+ )
+ .await
+ .expect("should create symlink");
+
+ let project = Project::test(
+ fs.clone(),
+ [
+ path!("/root/worktree_one").as_ref(),
+ path!("/root/worktree_two").as_ref(),
+ ],
+ cx,
+ )
+ .await;
+ cx.run_until_parked();
+ let fs_arc: Arc<dyn Fs> = fs;
+ let roots = worktree_roots(&project, &fs_arc, cx).await;
+
+ cx.read(|cx| {
+ let project = project.read(cx);
+
+ let resolved = resolve_project_path(
+ project,
+ "worktree_one/link_to_worktree_two/new_dir",
+ &roots,
+ cx,
+ )
+ .expect("should resolve missing child under cross-worktree symlink");
+ assert!(
+ matches!(resolved, ResolvedProjectPath::Safe(_)),
+ "missing child under cross-worktree symlink should be Safe, got: {:?}",
+ resolved
+ );
+ });
+ }
+}
@@ -84,10 +84,8 @@ impl AgentTool for WebSearchTool {
return Task::ready(Err(anyhow!("{}", reason)));
}
ToolPermissionDecision::Confirm => {
- let context = crate::ToolPermissionContext {
- tool_name: Self::NAME.to_string(),
- input_values: vec![input.query.clone()],
- };
+ let context =
+ crate::ToolPermissionContext::new(Self::NAME, vec![input.query.clone()]);
Some(event_stream.authorize(
format!("Search the web for {}", MarkdownInlineCode(&input.query)),
context,