@@ -114,156 +114,156 @@ impl FsRead {
114114 }
115115
116116 let is_in_allowlist = matches_any_pattern ( & agent. allowed_tools , "fs_read" ) ;
117- let settings = agent. tools_settings . get ( "fs_read" ) . cloned ( )
117+ let settings = agent
118+ . tools_settings
119+ . get ( "fs_read" )
120+ . cloned ( )
118121 . unwrap_or_else ( || serde_json:: json!( { } ) ) ;
119-
122+
120123 {
121- let Settings {
122- mut allowed_paths,
123- denied_paths,
124- allow_read_only,
125- } = match serde_json:: from_value :: < Settings > ( settings) {
126- Ok ( settings) => settings,
127- Err ( e) => {
128- error ! ( "Failed to deserialize tool settings for fs_read: {:?}" , e) ;
129- return PermissionEvalResult :: Ask ;
130- } ,
131- } ;
132-
133- // Always add current working directory to allowed paths
134- if let Ok ( cwd) = os. env . current_dir ( ) {
135- allowed_paths. push ( cwd. to_string_lossy ( ) . to_string ( ) ) ;
124+ let Settings {
125+ mut allowed_paths,
126+ denied_paths,
127+ allow_read_only,
128+ } = match serde_json:: from_value :: < Settings > ( settings) {
129+ Ok ( settings) => settings,
130+ Err ( e) => {
131+ error ! ( "Failed to deserialize tool settings for fs_read: {:?}" , e) ;
132+ return PermissionEvalResult :: Ask ;
133+ } ,
134+ } ;
135+
136+ // Always add current working directory to allowed paths
137+ if let Ok ( cwd) = os. env . current_dir ( ) {
138+ allowed_paths. push ( cwd. to_string_lossy ( ) . to_string ( ) ) ;
139+ }
140+
141+ let allow_set = {
142+ let mut builder = GlobSetBuilder :: new ( ) ;
143+ for path in & allowed_paths {
144+ let Ok ( path) = directories:: canonicalizes_path ( os, path) else {
145+ continue ;
146+ } ;
147+ if let Err ( e) = directories:: add_gitignore_globs ( & mut builder, path. as_str ( ) ) {
148+ warn ! ( "Failed to create glob from path given: {path}: {e}. Ignoring." ) ;
149+ }
136150 }
137-
138- let allow_set = {
139- let mut builder = GlobSetBuilder :: new ( ) ;
140- for path in & allowed_paths {
141- let Ok ( path) = directories:: canonicalizes_path ( os, path) else {
142- continue ;
143- } ;
144- if let Err ( e) = directories:: add_gitignore_globs ( & mut builder, path. as_str ( ) ) {
145- warn ! ( "Failed to create glob from path given: {path}: {e}. Ignoring." ) ;
146- }
151+ builder. build ( )
152+ } ;
153+
154+ let mut sanitized_deny_list = Vec :: < & String > :: new ( ) ;
155+ let deny_set = {
156+ let mut builder = GlobSetBuilder :: new ( ) ;
157+ for path in & denied_paths {
158+ let Ok ( processed_path) = directories:: canonicalizes_path ( os, path) else {
159+ continue ;
160+ } ;
161+ match directories:: add_gitignore_globs ( & mut builder, processed_path. as_str ( ) ) {
162+ Ok ( _) => {
163+ // Note that we need to push twice here because for each rule we
164+ // are creating two globs (one for file and one for directory)
165+ sanitized_deny_list. push ( path) ;
166+ sanitized_deny_list. push ( path) ;
167+ } ,
168+ Err ( e) => warn ! ( "Failed to create glob from path given: {path}: {e}. Ignoring." ) ,
147169 }
148- builder. build ( )
149- } ;
170+ }
171+ builder. build ( )
172+ } ;
150173
151- let mut sanitized_deny_list = Vec :: < & String > :: new ( ) ;
152- let deny_set = {
153- let mut builder = GlobSetBuilder :: new ( ) ;
154- for path in & denied_paths {
155- let Ok ( processed_path) = directories:: canonicalizes_path ( os, path) else {
156- continue ;
157- } ;
158- match directories:: add_gitignore_globs ( & mut builder, processed_path. as_str ( ) ) {
159- Ok ( _) => {
160- // Note that we need to push twice here because for each rule we
161- // are creating two globs (one for file and one for directory)
162- sanitized_deny_list. push ( path) ;
163- sanitized_deny_list. push ( path) ;
174+ match ( allow_set, deny_set) {
175+ ( Ok ( allow_set) , Ok ( deny_set) ) => {
176+ let mut deny_list = Vec :: < PermissionEvalResult > :: new ( ) ;
177+ let mut ask = false ;
178+
179+ for op in & self . operations {
180+ match op {
181+ FsReadOperation :: Line ( FsLine { path, .. } )
182+ | FsReadOperation :: Directory ( FsDirectory { path, .. } )
183+ | FsReadOperation :: Search ( FsSearch { path, .. } ) => {
184+ let Ok ( path) = directories:: canonicalizes_path ( os, path) else {
185+ ask = true ;
186+ continue ;
187+ } ;
188+ let denied_match_set = deny_set. matches ( path. as_ref ( ) as & str ) ;
189+ if !denied_match_set. is_empty ( ) {
190+ let deny_res = PermissionEvalResult :: Deny ( {
191+ denied_match_set
192+ . iter ( )
193+ . filter_map ( |i| sanitized_deny_list. get ( * i) . map ( |s| ( * s) . clone ( ) ) )
194+ . collect :: < Vec < _ > > ( )
195+ } ) ;
196+ deny_list. push ( deny_res) ;
197+ continue ;
198+ }
199+
200+ // We only want to ask if we are not allowing read only
201+ // operation
202+ if !is_in_allowlist && !allow_read_only && !allow_set. is_match ( path. as_ref ( ) as & str ) {
203+ ask = true ;
204+ }
205+ } ,
206+ FsReadOperation :: Image ( fs_image) => {
207+ let paths = & fs_image. image_paths ;
208+ let denied_match_set = paths
209+ . iter ( )
210+ . flat_map ( |path| {
211+ let Ok ( path) = directories:: canonicalizes_path ( os, path) else {
212+ return vec ! [ ] ;
213+ } ;
214+ deny_set. matches ( path. as_ref ( ) as & str )
215+ } )
216+ . collect :: < Vec < _ > > ( ) ;
217+ if !denied_match_set. is_empty ( ) {
218+ let deny_res = PermissionEvalResult :: Deny ( {
219+ denied_match_set
220+ . iter ( )
221+ . filter_map ( |i| sanitized_deny_list. get ( * i) . map ( |s| ( * s) . clone ( ) ) )
222+ . collect :: < Vec < _ > > ( )
223+ } ) ;
224+ deny_list. push ( deny_res) ;
225+ continue ;
226+ }
227+
228+ // We only want to ask if we are not allowing read only
229+ // operation
230+ if !is_in_allowlist
231+ && !allow_read_only
232+ && !paths. iter ( ) . any ( |path| allow_set. is_match ( path) )
233+ {
234+ ask = true ;
235+ }
164236 } ,
165- Err ( e) => warn ! ( "Failed to create glob from path given: {path}: {e}. Ignoring." ) ,
166237 }
167238 }
168- builder. build ( )
169- } ;
170-
171- match ( allow_set, deny_set) {
172- ( Ok ( allow_set) , Ok ( deny_set) ) => {
173- let mut deny_list = Vec :: < PermissionEvalResult > :: new ( ) ;
174- let mut ask = false ;
175-
176- for op in & self . operations {
177- match op {
178- FsReadOperation :: Line ( FsLine { path, .. } )
179- | FsReadOperation :: Directory ( FsDirectory { path, .. } )
180- | FsReadOperation :: Search ( FsSearch { path, .. } ) => {
181- let Ok ( path) = directories:: canonicalizes_path ( os, path) else {
182- ask = true ;
183- continue ;
184- } ;
185- let denied_match_set = deny_set. matches ( path. as_ref ( ) as & str ) ;
186- if !denied_match_set. is_empty ( ) {
187- let deny_res = PermissionEvalResult :: Deny ( {
188- denied_match_set
189- . iter ( )
190- . filter_map ( |i| sanitized_deny_list. get ( * i) . map ( |s| ( * s) . clone ( ) ) )
191- . collect :: < Vec < _ > > ( )
192- } ) ;
193- deny_list. push ( deny_res) ;
194- continue ;
195- }
196-
197- // We only want to ask if we are not allowing read only
198- // operation
199- if !is_in_allowlist
200- && !allow_read_only
201- && !allow_set. is_match ( path. as_ref ( ) as & str )
202- {
203- ask = true ;
204- }
205- } ,
206- FsReadOperation :: Image ( fs_image) => {
207- let paths = & fs_image. image_paths ;
208- let denied_match_set = paths
209- . iter ( )
210- . flat_map ( |path| {
211- let Ok ( path) = directories:: canonicalizes_path ( os, path) else {
212- return vec ! [ ] ;
213- } ;
214- deny_set. matches ( path. as_ref ( ) as & str )
215- } )
216- . collect :: < Vec < _ > > ( ) ;
217- if !denied_match_set. is_empty ( ) {
218- let deny_res = PermissionEvalResult :: Deny ( {
219- denied_match_set
220- . iter ( )
221- . filter_map ( |i| sanitized_deny_list. get ( * i) . map ( |s| ( * s) . clone ( ) ) )
222- . collect :: < Vec < _ > > ( )
223- } ) ;
224- deny_list. push ( deny_res) ;
225- continue ;
226- }
227-
228- // We only want to ask if we are not allowing read only
229- // operation
230- if !is_in_allowlist
231- && !allow_read_only
232- && !paths. iter ( ) . any ( |path| allow_set. is_match ( path) )
233- {
234- ask = true ;
235- }
236- } ,
237- }
238- }
239239
240- if !deny_list. is_empty ( ) {
241- PermissionEvalResult :: Deny ( {
242- deny_list. into_iter ( ) . fold ( Vec :: < String > :: new ( ) , |mut acc, res| {
243- if let PermissionEvalResult :: Deny ( mut rules) = res {
244- acc. append ( & mut rules) ;
245- }
246- acc
247- } )
240+ if !deny_list. is_empty ( ) {
241+ PermissionEvalResult :: Deny ( {
242+ deny_list. into_iter ( ) . fold ( Vec :: < String > :: new ( ) , |mut acc, res| {
243+ if let PermissionEvalResult :: Deny ( mut rules) = res {
244+ acc. append ( & mut rules) ;
245+ }
246+ acc
248247 } )
249- } else if ask {
250- PermissionEvalResult :: Ask
251- } else {
252- PermissionEvalResult :: Allow
253- }
254- } ,
255- ( allow_res, deny_res) => {
256- if let Err ( e) = allow_res {
257- warn ! ( "fs_read failed to build allow set: {:?}" , e) ;
258- }
259- if let Err ( e) = deny_res {
260- warn ! ( "fs_read failed to build deny set: {:?}" , e) ;
261- }
262- warn ! ( "One or more detailed args failed to parse, falling back to ask" ) ;
248+ } )
249+ } else if ask {
263250 PermissionEvalResult :: Ask
264- } ,
265- }
251+ } else {
252+ PermissionEvalResult :: Allow
253+ }
254+ } ,
255+ ( allow_res, deny_res) => {
256+ if let Err ( e) = allow_res {
257+ warn ! ( "fs_read failed to build allow set: {:?}" , e) ;
258+ }
259+ if let Err ( e) = deny_res {
260+ warn ! ( "fs_read failed to build deny set: {:?}" , e) ;
261+ }
262+ warn ! ( "One or more detailed args failed to parse, falling back to ask" ) ;
263+ PermissionEvalResult :: Ask
264+ } ,
266265 }
266+ }
267267 }
268268
269269 pub async fn invoke ( & self , os : & Os , updates : & mut impl Write ) -> Result < InvokeOutput > {
@@ -1452,12 +1452,11 @@ mod tests {
14521452
14531453 #[ tokio:: test]
14541454 async fn test_eval_perm_allowed_path_and_cwd ( ) {
1455-
14561455 // by default the fake env uses "/" as the CWD.
14571456 // change it to a sub folder so we can test fs_read reading files outside CWD
14581457 let os = Os :: new ( ) . await . unwrap ( ) ;
14591458 os. env . set_current_dir_for_test ( PathBuf :: from ( "/home/user" ) ) ;
1460-
1459+
14611460 let agent = Agent {
14621461 name : "test_agent" . to_string ( ) ,
14631462 tools_settings : {
@@ -1479,7 +1478,8 @@ mod tests {
14791478 { "path" : "/explicitly/allowed/path" , "mode" : "Directory" } ,
14801479 { "path" : "/explicitly/allowed/path/file.txt" , "mode" : "Line" } ,
14811480 ]
1482- } ) ) . unwrap ( ) ;
1481+ } ) )
1482+ . unwrap ( ) ;
14831483 let res = allowed_tool. eval_perm ( & os, & agent) ;
14841484 assert ! ( matches!( res, PermissionEvalResult :: Allow ) ) ;
14851485
@@ -1489,7 +1489,8 @@ mod tests {
14891489 { "path" : "/home/user/" , "mode" : "Directory" } ,
14901490 { "path" : "/home/user/file.txt" , "mode" : "Line" } ,
14911491 ]
1492- } ) ) . unwrap ( ) ;
1492+ } ) )
1493+ . unwrap ( ) ;
14931494 let res = cwd_tool. eval_perm ( & os, & agent) ;
14941495 assert ! ( matches!( res, PermissionEvalResult :: Allow ) ) ;
14951496
@@ -1498,7 +1499,8 @@ mod tests {
14981499 "operations" : [
14991500 { "path" : "/tmp/not/allowed/file.txt" , "mode" : "Line" }
15001501 ]
1501- } ) ) . unwrap ( ) ;
1502+ } ) )
1503+ . unwrap ( ) ;
15021504 let res = outside_tool. eval_perm ( & os, & agent) ;
15031505 assert ! ( matches!( res, PermissionEvalResult :: Ask ) ) ;
15041506 }
@@ -1507,7 +1509,7 @@ mod tests {
15071509 async fn test_eval_perm_no_settings_cwd_behavior ( ) {
15081510 let os = Os :: new ( ) . await . unwrap ( ) ;
15091511 os. env . set_current_dir_for_test ( PathBuf :: from ( "/home/user" ) ) ;
1510-
1512+
15111513 let agent = Agent {
15121514 name : "test_agent" . to_string ( ) ,
15131515 tools_settings : HashMap :: new ( ) , // No fs_read settings
@@ -1520,7 +1522,8 @@ mod tests {
15201522 { "path" : "/home/user/" , "mode" : "Directory" } ,
15211523 { "path" : "/home/user/file.txt" , "mode" : "Line" } ,
15221524 ]
1523- } ) ) . unwrap ( ) ;
1525+ } ) )
1526+ . unwrap ( ) ;
15241527 let res = cwd_tool. eval_perm ( & os, & agent) ;
15251528 assert ! ( matches!( res, PermissionEvalResult :: Allow ) ) ;
15261529
@@ -1529,7 +1532,8 @@ mod tests {
15291532 "operations" : [
15301533 { "path" : "/tmp/not/allowed/file.txt" , "mode" : "Line" }
15311534 ]
1532- } ) ) . unwrap ( ) ;
1535+ } ) )
1536+ . unwrap ( ) ;
15331537 let res = outside_tool. eval_perm ( & os, & agent) ;
15341538 assert ! ( matches!( res, PermissionEvalResult :: Ask ) ) ;
15351539 }
0 commit comments