Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
33 changes: 30 additions & 3 deletions src/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions src/llm-coding-tools-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ serde_json = "1.0"
# Zero overhead compile time bitflag generation
bitflags = "2.11.0"

# Shell-like expansion for home directory paths (~/ and $HOME/)
shellexpand = "3.1.2"

# Cross-platform canonicalization for non-existent paths
soft-canonicalize = "0.5.5"

# Fast binary serialization for catalog cache types
bitcode = "0.6.9"

Expand Down Expand Up @@ -102,6 +108,7 @@ llm-coding-tools-bubblewrap = { version = "0.1.0", path = "../llm-coding-tools-b
[dev-dependencies]
serial_test = "3"
tempfile = "3.27"
dirs = "6"
# For tests (async and blocking with wiremock)
tokio = { version = "1.51", features = ["rt-multi-thread", "macros"] }
wiremock = "0.6"
Expand Down
20 changes: 19 additions & 1 deletion src/llm-coding-tools-core/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,14 @@ 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).
- Failed resolution rejects traversal and out-of-sandbox paths before tool execution.

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

fn demo() -> ToolResult<()> {
// Unrestricted mode: any absolute path is allowed.
Expand All @@ -82,6 +86,19 @@ fn demo() -> ToolResult<()> {
// Sandboxed mode: only configured directories are allowed.
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"])?
.with_policy(
GlobPolicy::builder()
.add("src/**", RuleAction::Allow)? // Matches src/lib.rs
.add("*.rs", RuleAction::Allow)? // Also matches src/lib.rs
.add("target/**", RuleAction::Deny)? // Blocks target/ even if *.rs matches
.build()?
);
let _lib = glob.resolve("src/lib.rs")?;
let _main = glob.resolve("main.rs")?;
// glob.resolve("target/debug/app")?; // Denied
Ok(())
}
```
Expand Down Expand Up @@ -313,6 +330,7 @@ let key = resolver.resolve("OPENAI_API_KEY");
[`ToolContext`]: crate::context::ToolContext
[`PathResolver`]: crate::PathResolver
[`AbsolutePathResolver`]: crate::AbsolutePathResolver
[`AllowedGlobResolver`]: crate::path::AllowedGlobResolver
[`AllowedPathResolver`]: crate::AllowedPathResolver
[`permissions`]: crate::permissions
[`Rule`]: crate::permissions::Rule
Expand Down
2 changes: 1 addition & 1 deletion src/llm-coding-tools-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ pub use context::ToolContext;
pub use credentials::{CredentialLookup, CredentialResolver};
pub use error::{ToolError, ToolResult};
pub use output::ToolOutput;
pub use path::{AbsolutePathResolver, AllowedPathResolver, PathResolver};
pub use path::{AbsolutePathResolver, AllowedGlobResolver, AllowedPathResolver, PathResolver};
pub use system_prompt::SystemPromptBuilder;

// Re-export tools (always available, sync or async based on runtime feature)
Expand Down
56 changes: 38 additions & 18 deletions src/llm-coding-tools-core/src/path/allowed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use super::PathResolver;
use crate::context::PathMode;
use crate::error::{ToolError, ToolResult};
use soft_canonicalize::soft_canonicalize;
use std::path::{Path, PathBuf};
use std::sync::Arc;

Expand Down Expand Up @@ -37,17 +38,24 @@ pub struct AllowedPathResolver {
impl AllowedPathResolver {
/// Creates a new resolver with the given allowed directories.
///
/// Each directory is canonicalized during construction to ensure
/// consistent path comparison. Returns an error if any directory
/// doesn't exist or can't be canonicalized.
/// Each directory is resolved during construction to ensure consistent path
/// comparison. Returns an error if any directory doesn't exist or can't be
/// resolved.
pub fn new(allowed_paths: impl IntoIterator<Item = impl AsRef<Path>>) -> ToolResult<Self> {
let canonicalized: Result<Arc<[PathBuf]>, _> = allowed_paths
.into_iter()
.map(|p| {
let path = p.as_ref();
path.canonicalize().map_err(|e| {
if !path.is_dir() {
return Err(ToolError::InvalidPath(format!(
"failed to resolve allowed path '{}': path is not an existing directory",
path.display()
)));
}

soft_canonicalize(path).map_err(|e| {
ToolError::InvalidPath(format!(
"failed to canonicalize allowed path '{}': {}",
"failed to resolve allowed path '{}': {}",
path.display(),
e
))
Expand All @@ -60,10 +68,15 @@ impl AllowedPathResolver {
})
}

/// Creates a resolver from already-canonicalized paths.
/// Creates a resolver from already-canonicalized paths, skipping
/// filesystem validation.
///
/// A canonical path is absolute, with all symlinks resolved and all
/// `.` and `..` components normalized. Use [`std::fs::canonicalize`] or
/// [`std::path::Path::canonicalize`] to canonicalize paths.
///
/// Use this when paths are known to be valid and canonicalized,
/// skipping the filesystem check.
/// Use this when paths are known to be valid and canonicalized, skipping
/// the filesystem check.
///
/// # Safety
///
Expand Down Expand Up @@ -104,16 +117,11 @@ impl PathResolver for AllowedPathResolver {
continue;
}

// For non-existent paths (write operations), validate parent
if let Some(parent) = candidate.parent() {
if let Ok(canonical_parent) = parent.canonicalize() {
if canonical_parent.starts_with(base) {
// Parent is valid, construct the final path
let file_name = candidate.file_name().ok_or_else(|| {
ToolError::InvalidPath("path has no file name".into())
})?;
return Ok(canonical_parent.join(file_name));
}
// Non-existent paths still need a resolved absolute target so we can
// validate containment consistently across platforms.
if let Ok(resolved) = soft_canonicalize(&candidate) {
if resolved.starts_with(base) {
return Ok(resolved);
}
}
}
Expand Down Expand Up @@ -147,6 +155,7 @@ mod tests {
#[case::nested_existing_file("subdir/nested.txt", "nested.txt")] // exists: created by setup_test_dir()
#[case::new_file_in_root("new_file.txt", "new_file.txt")] // does NOT exist: tests write path resolution
#[case::new_file_in_subdir("subdir/new_file.txt", "new_file.txt")] // does NOT exist: tests write path resolution
#[case::new_file_in_missing_directories("new_dir/nested/new_file.txt", "new_file.txt")]
fn resolves_valid_paths_successfully(
#[case] input_path: &str,
#[case] expected_filename: &str,
Expand All @@ -167,6 +176,7 @@ mod tests {
#[rstest]
#[case::parent_traversal("../../../etc/passwd")]
#[case::nested_parent_traversal("subdir/../../../new_file.txt")]
#[case::missing_dir_parent_traversal("new_dir/../../new_file.txt")]
fn rejects_paths_that_escape_allowed_directory(#[case] input_path: &str) {
let dir = setup_test_dir();
let resolver = AllowedPathResolver::new(vec![dir.path().to_path_buf()]).unwrap();
Expand All @@ -179,6 +189,16 @@ mod tests {
);
}

#[test]
fn resolves_existing_file_through_missing_directory_parent_traversal() {
let dir = setup_test_dir();
let resolver = AllowedPathResolver::new(vec![dir.path().to_path_buf()]).unwrap();

let result = resolver.resolve("subdir/new_dir/../../file.txt");
assert!(result.is_ok());
assert!(result.unwrap().ends_with("file.txt"));
}

#[test]
fn tries_multiple_allowed_paths() {
let dir1 = setup_test_dir();
Expand Down
Loading