Skip to content

Commit 30fe246

Browse files
committed
Changed: Optimize bare ** pattern to use AllowedPathResolver
Pattern `**` is functionally equivalent to `Action(Allow)` (workspace-only access). Previously it fell through to AllowedGlobResolver; now it uses the cheaper AllowedPathResolver (prefix check instead of glob matching). - Added is_bare_globstar() to detect single `**` pattern - Updated optimization tables in module and function docs - Updated test to expect Allowed resolver for bare `**`
1 parent 02e8f2a commit 30fe246

1 file changed

Lines changed: 34 additions & 10 deletions

File tree

src/llm-coding-tools-agents/src/path/resolver.rs

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
//! |------------------------------------|-------------------------------|------------------|
1111
//! | No config for tool | `AllowedPathResolver([root])` | prefix check |
1212
//! | `Action(Allow)` | `AllowedPathResolver([root])` | prefix check |
13+
//! | Pattern `**` with Allow | `AllowedPathResolver([root])` | prefix check |
1314
//! | `/**` with Allow | `AbsolutePathResolver` | zero |
1415
//! | Otherwise | `AllowedGlobResolver` | glob matching |
1516
@@ -70,6 +71,7 @@ impl PathResolver for FileToolResolver {
7071
///
7172
/// - No config for tool -> `AllowedPathResolver([workspace_root])` (workspace only)
7273
/// - `Action(Allow)` -> `AllowedPathResolver([workspace_root])`
74+
/// - Pattern `"**"` with Allow -> `AllowedPathResolver([workspace_root])` (workspace only)
7375
/// - `"/**"` -> `AbsolutePathResolver` (any absolute path)
7476
/// - Otherwise -> `AllowedGlobResolver` with `GlobPolicy`
7577
///
@@ -117,11 +119,17 @@ pub fn build_resolver_for_tool(
117119
Ok(FileToolResolver::Allowed(resolver))
118120
}
119121
PermissionRule::Pattern(patterns) => {
120-
// Optimisation: all-allow + "/**" -> unrestricted access
122+
// Optimisation: all-allow patterns
121123
if patterns.values().all(|a| *a == PermissionAction::Allow) {
124+
// "/**" -> unrestricted access to any absolute path
122125
if let Some(resolver) = try_globstar_optimisation(patterns)? {
123126
return Ok(FileToolResolver::Absolute(resolver));
124127
}
128+
// "**" -> workspace only (equivalent to Action(Allow))
129+
if is_bare_globstar(patterns) {
130+
let resolver = AllowedPathResolver::from_canonical(vec![workspace_root]);
131+
return Ok(FileToolResolver::Allowed(resolver));
132+
}
125133
}
126134
// Fall through to full glob policy
127135
let resolver = AllowedGlobResolver::from_canonical(&workspace_root)
@@ -146,6 +154,21 @@ fn try_globstar_optimisation(
146154
Ok(None)
147155
}
148156

157+
/// Checks if the pattern map contains exactly one pattern "**" (bare globstar).
158+
///
159+
/// Returns `true` if there's a single pattern and it expands to "**",
160+
/// indicating workspace-only access (equivalent to `Action(Allow)`).
161+
fn is_bare_globstar(patterns: &IndexMap<String, PermissionAction>) -> bool {
162+
if patterns.len() != 1 {
163+
return false;
164+
}
165+
let pattern = patterns.keys().next().expect("len == 1");
166+
if let Ok(expanded) = expand_shell(pattern) {
167+
return expanded.to_string_lossy() == "**";
168+
}
169+
false
170+
}
171+
149172
/// Builds a `GlobPolicy` from a pattern map.
150173
fn build_glob_policy(
151174
patterns: &IndexMap<String, PermissionAction>,
@@ -245,11 +268,11 @@ mod tests {
245268

246269
// ---------------------------------------------------------------
247270
// build_resolver_for_tool: pattern "**" (bare globstar)
248-
// Bare ** pattern falls through to AllowedGlobResolver.
271+
// Equivalent to Action(Allow): workspace only via AllowedPathResolver.
249272
// ---------------------------------------------------------------
250273

251274
#[test]
252-
fn bare_globstar_returns_glob_workspace_root() -> TestResult {
275+
fn bare_globstar_returns_allowed_workspace_root() -> TestResult {
253276
let temp = tempfile::TempDir::new().unwrap();
254277

255278
let mut patterns = IndexMap::new();
@@ -259,20 +282,21 @@ mod tests {
259282

260283
let resolver = build_resolver_for_tool(&config, "read", temp.path())?;
261284

262-
assert!(
263-
matches!(resolver, FileToolResolver::Glob(_)),
264-
"bare ** should use AllowedGlobResolver"
265-
);
285+
let FileToolResolver::Allowed(inner) = &resolver else {
286+
panic!("expected Allowed, got {resolver:?}");
287+
};
266288

267-
let root = soft_canonicalize(temp.path())?;
289+
// Bare "**" -> workspace root (same as Action(Allow)).
290+
let expected = soft_canonicalize(temp.path())?;
291+
assert_eq!(inner.allowed_paths(), &[expected.clone()]);
268292

269293
// Any path within workspace should be allowed.
270294
assert!(
271-
resolver.is_path_allowed(&root.join("src/lib.rs")),
295+
resolver.is_path_allowed(&expected.join("src/lib.rs")),
272296
"** should permit src/lib.rs"
273297
);
274298
assert!(
275-
resolver.is_path_allowed(&root.join("any/path/file.txt")),
299+
resolver.is_path_allowed(&expected.join("any/path/file.txt")),
276300
"** should permit any path"
277301
);
278302

0 commit comments

Comments
 (0)