From 026d084f855130dc40e76c7bcca96d0620f8219a Mon Sep 17 00:00:00 2001 From: Sewer56 Date: Thu, 15 Jan 2026 21:53:27 +0000 Subject: [PATCH] Refactor: use Arc<[PathBuf]> for AllowedPathResolver Move path-to-Vec conversion into AllowedPathResolver::new(), eliminating duplicated boilerplate in all 5 allowed tool constructors. Using Arc<[PathBuf]> instead of Vec enables cheap Clone and signals immutability. --- src/llm-coding-tools-core/src/path/allowed.rs | 25 ++++++++++++------- src/llm-coding-tools-rig/src/allowed/edit.rs | 8 ++---- src/llm-coding-tools-rig/src/allowed/glob.rs | 8 ++---- src/llm-coding-tools-rig/src/allowed/grep.rs | 8 ++---- src/llm-coding-tools-rig/src/allowed/read.rs | 8 ++---- src/llm-coding-tools-rig/src/allowed/write.rs | 8 ++---- 6 files changed, 26 insertions(+), 39 deletions(-) diff --git a/src/llm-coding-tools-core/src/path/allowed.rs b/src/llm-coding-tools-core/src/path/allowed.rs index 8b81898e..8e9b828e 100644 --- a/src/llm-coding-tools-core/src/path/allowed.rs +++ b/src/llm-coding-tools-core/src/path/allowed.rs @@ -2,7 +2,8 @@ use super::PathResolver; use crate::error::{ToolError, ToolResult}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; +use std::sync::Arc; /// Path resolver that restricts access to allowed directories. /// @@ -32,7 +33,7 @@ use std::path::PathBuf; #[derive(Debug, Clone)] pub struct AllowedPathResolver { /// Canonicalized allowed base directories. - allowed_paths: Vec, + allowed_paths: Arc<[PathBuf]>, } impl AllowedPathResolver { @@ -41,14 +42,15 @@ impl AllowedPathResolver { /// 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. - pub fn new(allowed_paths: Vec) -> ToolResult { - let canonicalized: Result, _> = allowed_paths + pub fn new(allowed_paths: impl IntoIterator>) -> ToolResult { + let canonicalized: Result, _> = allowed_paths .into_iter() .map(|p| { - p.canonicalize().map_err(|e| { + let path = p.as_ref(); + path.canonicalize().map_err(|e| { ToolError::InvalidPath(format!( "failed to canonicalize allowed path '{}': {}", - p.display(), + path.display(), e )) }) @@ -69,8 +71,13 @@ impl AllowedPathResolver { /// /// Caller must ensure paths are actually canonical. Using non-canonical /// paths may allow path traversal attacks. - pub fn from_canonical(allowed_paths: Vec) -> Self { - Self { allowed_paths } + pub fn from_canonical(allowed_paths: impl IntoIterator>) -> Self { + Self { + allowed_paths: allowed_paths + .into_iter() + .map(|p| p.as_ref().to_path_buf()) + .collect(), + } } /// Returns the allowed base directories. @@ -84,7 +91,7 @@ impl PathResolver for AllowedPathResolver { let input_path = PathBuf::from(path); // Try each allowed base directory in order - for base in &self.allowed_paths { + for base in self.allowed_paths.iter() { let candidate = base.join(&input_path); // Try to canonicalize for existing paths diff --git a/src/llm-coding-tools-rig/src/allowed/edit.rs b/src/llm-coding-tools-rig/src/allowed/edit.rs index 0e7933e7..82ba6902 100644 --- a/src/llm-coding-tools-rig/src/allowed/edit.rs +++ b/src/llm-coding-tools-rig/src/allowed/edit.rs @@ -8,7 +8,7 @@ use rig::completion::ToolDefinition; use rig::tool::Tool; use schemars::{schema_for, JsonSchema}; use serde::Deserialize; -use std::path::{Path, PathBuf}; +use std::path::Path; /// Arguments for file editing. #[derive(Debug, Clone, Deserialize, JsonSchema)] @@ -33,12 +33,8 @@ pub struct EditTool { impl EditTool { /// Creates a new edit tool restricted to the given directories. pub fn new(allowed_paths: impl IntoIterator>) -> ToolResult { - let paths: Vec = allowed_paths - .into_iter() - .map(|p| p.as_ref().to_path_buf()) - .collect(); Ok(Self { - resolver: AllowedPathResolver::new(paths)?, + resolver: AllowedPathResolver::new(allowed_paths)?, }) } diff --git a/src/llm-coding-tools-rig/src/allowed/glob.rs b/src/llm-coding-tools-rig/src/allowed/glob.rs index bea6019d..5dfdad8d 100644 --- a/src/llm-coding-tools-rig/src/allowed/glob.rs +++ b/src/llm-coding-tools-rig/src/allowed/glob.rs @@ -7,7 +7,7 @@ use rig::completion::ToolDefinition; use rig::tool::Tool; use schemars::{schema_for, JsonSchema}; use serde::Deserialize; -use std::path::{Path, PathBuf}; +use std::path::Path; /// Arguments for the glob tool. #[derive(Debug, Deserialize, JsonSchema)] @@ -27,12 +27,8 @@ pub struct GlobTool { impl GlobTool { /// Creates a new glob tool restricted to the given directories. pub fn new(allowed_paths: impl IntoIterator>) -> ToolResult { - let paths: Vec = allowed_paths - .into_iter() - .map(|p| p.as_ref().to_path_buf()) - .collect(); Ok(Self { - resolver: AllowedPathResolver::new(paths)?, + resolver: AllowedPathResolver::new(allowed_paths)?, }) } diff --git a/src/llm-coding-tools-rig/src/allowed/grep.rs b/src/llm-coding-tools-rig/src/allowed/grep.rs index 0952b67b..decda513 100644 --- a/src/llm-coding-tools-rig/src/allowed/grep.rs +++ b/src/llm-coding-tools-rig/src/allowed/grep.rs @@ -8,7 +8,7 @@ use rig::tool::Tool; use schemars::{schema_for, JsonSchema}; use serde::Deserialize; use std::fmt::Write; -use std::path::{Path, PathBuf}; +use std::path::Path; const DEFAULT_LIMIT: usize = 100; const MAX_LIMIT: usize = 2000; @@ -42,12 +42,8 @@ pub struct GrepTool { impl GrepTool { /// Creates a new grep tool restricted to the given directories. pub fn new(allowed_paths: impl IntoIterator>) -> ToolResult { - let paths: Vec = allowed_paths - .into_iter() - .map(|p| p.as_ref().to_path_buf()) - .collect(); Ok(Self { - resolver: AllowedPathResolver::new(paths)?, + resolver: AllowedPathResolver::new(allowed_paths)?, }) } diff --git a/src/llm-coding-tools-rig/src/allowed/read.rs b/src/llm-coding-tools-rig/src/allowed/read.rs index 6ae3bf16..63134b63 100644 --- a/src/llm-coding-tools-rig/src/allowed/read.rs +++ b/src/llm-coding-tools-rig/src/allowed/read.rs @@ -7,7 +7,7 @@ use rig::completion::ToolDefinition; use rig::tool::Tool; use schemars::{schema_for, JsonSchema}; use serde::Deserialize; -use std::path::{Path, PathBuf}; +use std::path::Path; const DEFAULT_OFFSET: usize = 1; const DEFAULT_LIMIT: usize = 2000; @@ -44,12 +44,8 @@ pub struct ReadTool { impl ReadTool { /// Creates a new read tool restricted to the given directories. pub fn new(allowed_paths: impl IntoIterator>) -> ToolResult { - let paths: Vec = allowed_paths - .into_iter() - .map(|p| p.as_ref().to_path_buf()) - .collect(); Ok(Self { - resolver: AllowedPathResolver::new(paths)?, + resolver: AllowedPathResolver::new(allowed_paths)?, }) } diff --git a/src/llm-coding-tools-rig/src/allowed/write.rs b/src/llm-coding-tools-rig/src/allowed/write.rs index a3f3fd8f..47cf7228 100644 --- a/src/llm-coding-tools-rig/src/allowed/write.rs +++ b/src/llm-coding-tools-rig/src/allowed/write.rs @@ -7,7 +7,7 @@ use rig::completion::ToolDefinition; use rig::tool::Tool; use schemars::{schema_for, JsonSchema}; use serde::Deserialize; -use std::path::{Path, PathBuf}; +use std::path::Path; /// Arguments for the write tool. #[derive(Debug, Clone, Deserialize, JsonSchema)] @@ -27,12 +27,8 @@ pub struct WriteTool { impl WriteTool { /// Creates a new write tool restricted to the given directories. pub fn new(allowed_paths: impl IntoIterator>) -> ToolResult { - let paths: Vec = allowed_paths - .into_iter() - .map(|p| p.as_ref().to_path_buf()) - .collect(); Ok(Self { - resolver: AllowedPathResolver::new(paths)?, + resolver: AllowedPathResolver::new(allowed_paths)?, }) }