Skip to content

Rework path resolvers to use workspace-rooted semantics #98

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Sewer56 merged 4 commits into from
Apr 10, 2026
Merged

Rework path resolvers to use workspace-rooted semantics #98

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
227a56d
Changed: Rework path resolvers to use workspace-rooted semantics (#98)
Sewer56 Apr 10, 2026
a6054fd
Changed: Extract validate_resolved helper to eliminate code duplication
Sewer56 Apr 10, 2026
4a5915c
Added: Debug assertion in AllowedGlob::from_canonical
Sewer56 Apr 10, 2026
1e9e0c6
Fixed: Use platform-aware paths in glob policy tests on Windows
Sewer56 Apr 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/llm-coding-tools-agents/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,9 @@ permission:
task: allow # Required to delegate to subagents
```

**Glob patterns in permissions:** Permission values use globs (`*` = one component, `**` = any depth).
Bare `allow` equals `**`. Patterns are workspace-relative.

**Note:** `task` is special - when omitted, it allows delegation to all callable
subagents for OpenCode compatibility. To disable delegation, explicitly set
`task: deny`.
Expand Down
21 changes: 16 additions & 5 deletions src/llm-coding-tools-core/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,26 +69,27 @@ Path-based tools are generic over [`PathResolver`], so wrappers can choose unres

- [`AbsolutePathResolver`] enforces absolute-path inputs (unrestricted mode).
- [`AllowedPathResolver`] constrains operations to configured directories (sandbox mode).
- [`AllowedGlobResolver`] constrains to directories with glob pattern filtering (fine-grained sandbox mode).
- [`AllowedGlobResolver`] constrains to a workspace root with glob pattern filtering (fine-grained sandbox mode).
- Failed resolution rejects traversal and out-of-sandbox paths before tool execution.

```rust,no_run
use llm_coding_tools_core::{
path::{AllowedGlobResolver, GlobPolicy, RuleAction},
AbsolutePathResolver, AllowedPathResolver, PathResolver, ToolResult,
resolve_workspace_root, AbsolutePathResolver, AllowedPathResolver, PathResolver, ToolResult,
};

fn demo() -> ToolResult<()> {
// Unrestricted mode: any absolute path is allowed.
let any_path = AbsolutePathResolver;
let _hosts = any_path.resolve("/etc/hosts")?;

// Sandboxed mode: only configured directories are allowed.
// Sandboxed mode: multiple allowed directories.
let sandbox = AllowedPathResolver::new(["/workspace/project", "/tmp"])?;
let _lib = sandbox.resolve("src/lib.rs")?;

// Fine-grained sandbox (last-match-wins).
let glob = AllowedGlobResolver::new(["/workspace/project"])?
// Fine-grained sandbox with glob policy (workspace-relative patterns).
let root = resolve_workspace_root()?;
let glob = AllowedGlobResolver::new(&root)?
.with_policy(
GlobPolicy::builder()
.add("src/**", RuleAction::Allow)? // Matches src/lib.rs
Expand All @@ -103,6 +104,16 @@ fn demo() -> ToolResult<()> {
}
```

#### Permission glob semantics

- `*` matches any characters within a single path component (e.g., `*.rs` matches `lib.rs` but not `src/lib.rs`).
- `**` matches any number of path components (e.g., `src/**/*.rs` matches `src/deep/nested/mod.rs`).
- Bare `allow` maps to `**` (all files under the workspace root).
- Relative patterns are implicitly joined with the workspace root at construction time.
- Absolute patterns (leading `/` or drive-root like `C:/`) are treated as-is.

Last-match-wins: both deny-then-allow and allow-then-deny orders work depending on whether you want a default-deny or default-allow posture.

#### Linux shell sandboxing

Enable the `linux-bubblewrap` feature flag to sandbox [`bash`] ([`execute_command`])
Expand Down
134 changes: 6 additions & 128 deletions src/llm-coding-tools-core/benches/path_resolvers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
//! - `resolvers`: Compares [`AllowedPathResolver`] and [`AllowedGlobResolver`] on the same paths
//! - `multiple_bases`: Tests [`AllowedPathResolver`] with multiple base directories
//! - `canonicalize`: Isolates `canonicalize` vs `soft_canonicalize` performance
//! - `external_directory`: Tests external directory permission fallback for absolute paths
//!
//! # Test Cases (resolvers)
//!
Expand Down Expand Up @@ -55,18 +54,6 @@
//! canonicalize/existing_file_soft_canonicalize ~5.3 µs (2.7x slower than canonicalize)
//! canonicalize/new_file_shallow_soft_canonicalize ~7.2 µs
//! canonicalize/new_file_deep_soft_canonicalize ~8.4 µs
//!
//! external_directory/AllowedPathResolver/external_existing_file ~548 ns (canonicalize + permission)
//! external_directory/AllowedPathResolver/external_new_file ~3.3 µs (soft_canonicalize + permission)
//! external_directory/AllowedPathResolver/external_rejected ~2.4 µs (canonicalize + deny)
//! external_directory/AllowedPathResolver/external_no_ruleset ~2.3 µs (canonicalize, no permission)
//! external_directory/AllowedPathResolver/relative_still_fails ~9.8 µs (soft_canonicalize, not external)
//!
//! external_directory/AllowedGlobResolver/external_existing_file ~535 ns (canonicalize + permission)
//! external_directory/AllowedGlobResolver/external_new_file ~3.3 µs (soft_canonicalize + permission)
//! external_directory/AllowedGlobResolver/external_rejected ~2.3 µs (canonicalize + deny)
//! external_directory/AllowedGlobResolver/external_no_ruleset ~2.3 µs (canonicalize, no permission)
//! external_directory/AllowedGlobResolver/relative_still_fails ~9.8 µs (soft_canonicalize, not external)
//! ```
//!
//! # Platform Differences
Expand All @@ -93,10 +80,8 @@ use criterion::{criterion_group, criterion_main, BenchmarkId, Criterion, Through
use llm_coding_tools_core::path::{
AllowedGlobResolver, AllowedPathResolver, GlobPolicy, GlobPolicyBuilder, PathResolver,
};
use llm_coding_tools_core::permissions::{PermissionAction, Rule, Ruleset};
use soft_canonicalize::soft_canonicalize;
use std::fs;
use std::sync::Arc;
use tempfile::TempDir;

const EXISTING_FILE: &str = "src/lib.rs";
Expand All @@ -112,7 +97,8 @@ fn build_policy<F>(f: F) -> llm_coding_tools_core::error::ToolResult<GlobPolicy>
where
F: FnOnce(GlobPolicyBuilder) -> llm_coding_tools_core::error::ToolResult<GlobPolicyBuilder>,
{
f(GlobPolicy::builder()).and_then(|b| b.build())
let base = soft_canonicalize(std::env::current_dir().unwrap()).unwrap();
f(GlobPolicy::builder_with_base(&base)?).and_then(|b| b.build())
}

/// Benchmarks [`AllowedPathResolver`] and [`AllowedGlobResolver`] on the same paths.
Expand Down Expand Up @@ -149,13 +135,11 @@ fn bench_resolvers_same_paths(c: &mut Criterion) {
let allowed = AllowedPathResolver::new(vec![current_dir.clone()]).unwrap();

// Simple policy: single glob pattern (src/**/*.rs)
// This tests minimal glob matching overhead.
let simple_policy = build_policy(|b| b.allow("src/**/*.rs")).unwrap();

// Complex policy: 10 rules simulating a realistic project configuration.
// Tests last-match-wins semantics and rule iteration overhead.
let complex_policy = build_policy(|b| {
b.allow("src/**/*.rs")?
b.allow("src/**")?
.deny("target/**")?
.allow("*.toml")?
.deny("*.log")?
Expand All @@ -167,10 +151,10 @@ fn bench_resolvers_same_paths(c: &mut Criterion) {
})
.unwrap();

let glob_simple = AllowedGlobResolver::new(vec![current_dir.clone()])
let glob_simple = AllowedGlobResolver::new(&current_dir)
.unwrap()
.with_policy(simple_policy);
let glob_complex = AllowedGlobResolver::new(vec![current_dir.clone()])
let glob_complex = AllowedGlobResolver::new(&current_dir)
.unwrap()
.with_policy(complex_policy);

Expand Down Expand Up @@ -306,117 +290,11 @@ fn bench_canonicalize_vs_soft(c: &mut Criterion) {
group.finish();
}

/// Benchmarks the external directory permission fallback path.
///
/// Tests the performance of resolving paths that are outside all base
/// directories but may be allowed via an `"external_directory"` permission rule.
///
/// # Test Cases
///
/// ```text
/// | Case | Description |
/// |-------------------------|----------------------------------------------------------|
/// | external_existing_file | Absolute path to file in allowed temp dir (fast path) |
/// | external_new_file | Absolute path to new file in allowed temp dir (soft_can) |
/// | external_rejected | Absolute path denied by permission ruleset (early exit) |
/// | external_no_ruleset | Absolute path with no external_permission configured |
/// | relative_still_fails | Relative path even when external_permission is set |
/// ```
fn bench_external_directory(c: &mut Criterion) {
let mut group = c.benchmark_group("external_directory");

let current_dir = std::env::current_dir().unwrap();
let external_dir = TempDir::new().unwrap();
let existing_file = external_dir.path().join("existing.txt");
fs::write(&existing_file, "content").unwrap();
let new_file = external_dir.path().join("subdir/new_file.txt");

let canon_external = soft_canonicalize(external_dir.path()).unwrap();
let allow_pattern = canon_external.join("*").to_str().unwrap().to_owned();

let mut allow_ruleset = Ruleset::new();
allow_ruleset.push(
Rule::new(
"external_directory",
allow_pattern.as_str(),
PermissionAction::Allow,
)
.unwrap(),
);

let mut deny_ruleset = Ruleset::new();
deny_ruleset.push(Rule::new("external_directory", "*", PermissionAction::Deny).unwrap());

let existing_file_str = existing_file.to_str().unwrap().to_owned();
let new_file_str = new_file.to_str().unwrap().to_owned();
let rejected_path = std::env::temp_dir()
.join("rejected_external")
.join("path.txt")
.to_str()
.unwrap()
.to_owned();

let allowed_path_resolver = AllowedPathResolver::new(vec![current_dir.clone()])
.unwrap()
.with_external_permission(Arc::new(allow_ruleset.clone()));

let denied_path_resolver = AllowedPathResolver::new(vec![current_dir.clone()])
.unwrap()
.with_external_permission(Arc::new(deny_ruleset.clone()));

let no_permission_resolver = AllowedPathResolver::new(vec![current_dir.clone()]).unwrap();

let allowed_glob_resolver = AllowedGlobResolver::new(vec![current_dir.clone()])
.unwrap()
.with_external_permission(Arc::new(allow_ruleset));

let denied_glob_resolver = AllowedGlobResolver::new(vec![current_dir.clone()])
.unwrap()
.with_external_permission(Arc::new(deny_ruleset));

let no_permission_glob_resolver = AllowedGlobResolver::new(vec![current_dir.clone()]).unwrap();

group.throughput(Throughput::Elements(1));

for (case_name, path_input) in [
("external_existing_file", existing_file_str.as_str()),
("external_new_file", new_file_str.as_str()),
("external_rejected", rejected_path.as_str()),
("external_no_ruleset", rejected_path.as_str()),
("relative_still_fails", "relative/path.txt"),
] {
let (path_resolver, glob_resolver) = match case_name {
"external_existing_file" | "external_new_file" => {
(&allowed_path_resolver, &allowed_glob_resolver)
}
"external_rejected" => (&denied_path_resolver, &denied_glob_resolver),
"external_no_ruleset" => (&no_permission_resolver, &no_permission_glob_resolver),
"relative_still_fails" => (&allowed_path_resolver, &allowed_glob_resolver),
_ => unreachable!(),
};

group.bench_with_input(
BenchmarkId::new("AllowedPathResolver", case_name),
path_resolver,
|b, resolver| b.iter(|| resolver.resolve(black_box(path_input))),
);

group.bench_with_input(
BenchmarkId::new("AllowedGlobResolver", case_name),
glob_resolver,
|b, resolver| b.iter(|| resolver.resolve(black_box(path_input))),
);
}

group.finish();
}

criterion_group!(
benches,
bench_resolvers_same_paths,
bench_multiple_bases,
bench_canonicalize_vs_soft,
bench_external_directory
bench_canonicalize_vs_soft
);

criterion_main!(benches);
2 changes: 2 additions & 0 deletions src/llm-coding-tools-core/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ pub mod system_prompt;
pub mod tool_metadata;
pub mod tools;
pub mod util;
pub mod workspace;

mod internal;

Expand All @@ -29,6 +30,7 @@ pub use error::{ToolError, ToolResult};
pub use output::ToolOutput;
pub use path::{AbsolutePathResolver, AllowedGlobResolver, AllowedPathResolver, PathResolver};
pub use system_prompt::SystemPromptBuilder;
pub use workspace::resolve_workspace_root;

// Re-export tools (always available, sync or async based on runtime feature)
pub use tools::{
Expand Down
Loading