From 6a1fe28da7729beda6fc5f3603314364bb8558ed Mon Sep 17 00:00:00 2001 From: Felix dingfeli Date: Thu, 3 Jul 2025 12:07:43 -0700 Subject: [PATCH] Fix: error from merge for fs read --- crates/chat-cli/src/cli/agent.rs | 2 +- crates/chat-cli/src/cli/chat/tools/fs_read.rs | 215 ++++++++++-------- 2 files changed, 116 insertions(+), 101 deletions(-) diff --git a/crates/chat-cli/src/cli/agent.rs b/crates/chat-cli/src/cli/agent.rs index 9dc3c18200..1042886d23 100644 --- a/crates/chat-cli/src/cli/agent.rs +++ b/crates/chat-cli/src/cli/agent.rs @@ -186,7 +186,7 @@ impl Agent { } } -#[derive(Debug)] +#[derive(Debug, PartialEq)] pub enum PermissionEvalResult { Allow, Ask, diff --git a/crates/chat-cli/src/cli/chat/tools/fs_read.rs b/crates/chat-cli/src/cli/chat/tools/fs_read.rs index 4fd5871037..b6cd9faaf3 100644 --- a/crates/chat-cli/src/cli/chat/tools/fs_read.rs +++ b/crates/chat-cli/src/cli/chat/tools/fs_read.rs @@ -99,6 +99,121 @@ impl FsRead { } } + pub fn eval_perm(&self, agent: &Agent) -> PermissionEvalResult { + #[derive(Debug, Deserialize)] + #[serde(rename_all = "camelCase")] + struct Settings { + #[serde(default)] + allowed_paths: Vec, + #[serde(default)] + denied_paths: Vec, + #[serde(default = "default_allow_read_only")] + allow_read_only: bool, + } + + fn default_allow_read_only() -> bool { + true + } + + let is_in_allowlist = agent.allowed_tools.contains("fs_read"); + match agent.tools_settings.get("fs_read") { + Some(settings) if is_in_allowlist => { + let Settings { + allowed_paths, + denied_paths, + allow_read_only, + } = match serde_json::from_value::(settings.clone()) { + Ok(settings) => settings, + Err(e) => { + error!("Failed to deserialize tool settings for fs_read: {:?}", e); + return PermissionEvalResult::Ask; + }, + }; + let allow_set = { + let mut builder = GlobSetBuilder::new(); + for path in &allowed_paths { + if let Ok(glob) = Glob::new(path) { + builder.add(glob); + } else { + warn!("Failed to create glob from path given: {path}. Ignoring."); + } + } + builder.build() + }; + + let deny_set = { + let mut builder = GlobSetBuilder::new(); + for path in &denied_paths { + if let Ok(glob) = Glob::new(path) { + builder.add(glob); + } else { + warn!("Failed to create glob from path given: {path}. Ignoring."); + } + } + builder.build() + }; + + match (allow_set, deny_set) { + (Ok(allow_set), Ok(deny_set)) => { + let eval_res = self + .operations + .iter() + .map(|op| { + match op { + FsReadOperation::Line(FsLine { path, .. }) + | FsReadOperation::Directory(FsDirectory { path, .. }) + | FsReadOperation::Search(FsSearch { path, .. }) => { + if deny_set.is_match(path) { + return PermissionEvalResult::Deny; + } + if allow_set.is_match(path) { + return PermissionEvalResult::Allow; + } + }, + FsReadOperation::Image(fs_image) => { + let paths = &fs_image.image_paths; + if paths.iter().any(|path| deny_set.is_match(path)) { + return PermissionEvalResult::Deny; + } + if paths.iter().all(|path| allow_set.is_match(path)) { + return PermissionEvalResult::Allow; + } + }, + } + + if allow_read_only { + PermissionEvalResult::Allow + } else { + PermissionEvalResult::Ask + } + }) + .collect::>(); + + if eval_res.contains(&PermissionEvalResult::Deny) { + PermissionEvalResult::Deny + } else if eval_res.contains(&PermissionEvalResult::Ask) { + PermissionEvalResult::Ask + } else { + PermissionEvalResult::Allow + } + }, + (allow_res, deny_res) => { + if let Err(e) = allow_res { + warn!("fs_read failed to build allow set: {:?}", e); + } + if let Err(e) = deny_res { + warn!("fs_read failed to build deny set: {:?}", e); + } + warn!("One or more detailed args failed to parse, falling back to ask"); + PermissionEvalResult::Ask + }, + } + }, + None if is_in_allowlist => PermissionEvalResult::Allow, + _ => PermissionEvalResult::Ask, + } + } + pub async fn invoke(&self, os: &Os, updates: &mut impl Write) -> Result { if self.operations.len() == 1 { // Single operation - return result directly @@ -218,106 +333,6 @@ impl FsReadOperation { FsReadOperation::Image(fs_image) => fs_image.invoke(updates).await, } } - - pub fn eval_perm(&self, agent: &Agent) -> PermissionEvalResult { - #[derive(Debug, Deserialize)] - #[serde(rename_all = "camelCase")] - struct Settings { - #[serde(default)] - allowed_paths: Vec, - #[serde(default)] - denied_paths: Vec, - #[serde(default = "default_allow_read_only")] - allow_read_only: bool, - } - - fn default_allow_read_only() -> bool { - true - } - - let is_in_allowlist = agent.allowed_tools.contains("fs_read"); - match agent.tools_settings.get("fs_read") { - Some(settings) if is_in_allowlist => { - let Settings { - allowed_paths, - denied_paths, - allow_read_only, - } = match serde_json::from_value::(settings.clone()) { - Ok(settings) => settings, - Err(e) => { - error!("Failed to deserialize tool settings for fs_read: {:?}", e); - return PermissionEvalResult::Ask; - }, - }; - let allow_set = { - let mut builder = GlobSetBuilder::new(); - for path in &allowed_paths { - if let Ok(glob) = Glob::new(path) { - builder.add(glob); - } else { - warn!("Failed to create glob from path given: {path}. Ignoring."); - } - } - builder.build() - }; - - let deny_set = { - let mut builder = GlobSetBuilder::new(); - for path in &denied_paths { - if let Ok(glob) = Glob::new(path) { - builder.add(glob); - } else { - warn!("Failed to create glob from path given: {path}. Ignoring."); - } - } - builder.build() - }; - - match (allow_set, deny_set) { - (Ok(allow_set), Ok(deny_set)) => { - match self { - Self::Line(FsLine { path, .. }) - | Self::Directory(FsDirectory { path, .. }) - | Self::Search(FsSearch { path, .. }) => { - if deny_set.is_match(path) { - return PermissionEvalResult::Deny; - } - if allow_set.is_match(path) { - return PermissionEvalResult::Allow; - } - }, - Self::Image(fs_image) => { - let paths = &fs_image.image_paths; - if paths.iter().any(|path| deny_set.is_match(path)) { - return PermissionEvalResult::Deny; - } - if paths.iter().all(|path| allow_set.is_match(path)) { - return PermissionEvalResult::Allow; - } - }, - } - if allow_read_only { - PermissionEvalResult::Allow - } else { - PermissionEvalResult::Ask - } - }, - (allow_res, deny_res) => { - if let Err(e) = allow_res { - warn!("fs_read failed to build allow set: {:?}", e); - } - if let Err(e) = deny_res { - warn!("fs_read failed to build deny set: {:?}", e); - } - warn!("One or more detailed args failed to parse, falling back to ask"); - PermissionEvalResult::Ask - }, - } - }, - None if is_in_allowlist => PermissionEvalResult::Allow, - _ => PermissionEvalResult::Ask, - } - } } /// Read images from given paths.