fix(tools): enforce allowed_dirs allowlist in ReadFileTool

Adds allowed_dirs config field to BuiltinToolConfig. ReadFileTool now
rejects reads outside the configured base directories after canonicalize(),
blocking both path traversal and symlink attacks. Logs a warning when
allowed_dirs is empty. Threads config through register_all so constructors
receive per-tool settings.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

crates/proxy/src/config/simple.rs

@@ -118,6 +118,10 @@ pub struct BuiltinToolConfig {
     pub policy: Option<String>,
     #[serde(default)]
     pub timeout_secs: Option<u64>,
+    /// For read_file: restrict reads to files under these absolute directory paths.
+    /// If empty or absent, all paths are permitted (dangerous; set this in production).
+    #[serde(default)]
+    pub allowed_dirs: Vec<String>,
 }
 
 fn default_true() -> bool {

crates/proxy/src/main.rs

@@ -175,7 +175,10 @@ async fn main() {
 
             let mut registry = tools::ToolRegistry::new();
             // Register built-in tools (gated behind the dangerous-builtin-tools feature).
-            anyllm_proxy::tools::builtin::register_all(&mut registry);
+            anyllm_proxy::tools::builtin::register_all(
+                &mut registry,
+                simple_config_shell.builtin_tools.as_ref(),
+            );
 
             // Build MCP manager and discover tools from configured servers.
             let mcp_manager = if let Some(ref servers) = simple_config_shell.mcp_servers {

crates/proxy/src/tools/builtin/mod.rs

@@ -1,10 +1,60 @@
+// SAFETY: The tools in this module execute arbitrary shell commands (BashTool)
+// and read arbitrary files (ReadFileTool) as the proxy process user. They must
+// NEVER be registered in a server-side tool registry without explicit operator
+// opt-in and appropriate sandboxing. Gated behind the `dangerous-builtin-tools`
+// feature flag, which is OFF by default.
+
+#[cfg(feature = "dangerous-builtin-tools")]
 pub mod bash;
+#[cfg(feature = "dangerous-builtin-tools")]
 pub mod read_file;
 
 use crate::tools::registry::ToolRegistry;
 
-/// Convenience function to populate a registry with standard tools.
-pub fn register_all(registry: &mut ToolRegistry) {
-    registry.register(Box::new(bash::BashTool));
-    registry.register(Box::new(read_file::ReadFileTool));
+/// Populate a registry with standard built-in tools.
+///
+/// `builtin_configs`: optional map of tool name -> config from PROXY_CONFIG; used to
+/// pass per-tool settings (e.g., `allowed_dirs` for `read_file`) to tool constructors.
+///
+/// # Safety
+///
+/// When `dangerous-builtin-tools` is enabled, this registers `BashTool` (arbitrary
+/// shell execution) and `ReadFileTool` (arbitrary file reads). Only call this if
+/// the tool execution engine is sandboxed or if the operator has explicitly opted in.
+///
+/// When the feature is disabled (the default), this is a no-op.
+pub fn register_all(
+    _registry: &mut ToolRegistry,
+    _builtin_configs: Option<&std::collections::HashMap<String, crate::config::simple::BuiltinToolConfig>>,
+) {
+    #[cfg(feature = "dangerous-builtin-tools")]
+    {
+        _registry.register(Box::new(bash::BashTool));
+
+        // Build ReadFileTool with allowed_dirs from config, canonicalized at registration time.
+        let allowed_dirs = _builtin_configs
+            .and_then(|m| m.get("read_file"))
+            .map(|cfg| {
+                cfg.allowed_dirs
+                    .iter()
+                    .filter_map(|d| {
+                        let p = std::path::PathBuf::from(d);
+                        match p.canonicalize() {
+                            Ok(canon) => Some(canon),
+                            Err(e) => {
+                                tracing::warn!(
+                                    dir = %d,
+                                    error = %e,
+                                    "read_file allowed_dirs entry could not be canonicalized; skipping"
+                                );
+                                None
+                            }
+                        }
+                    })
+                    .collect()
+            })
+            .unwrap_or_default();
+
+        _registry.register(Box::new(read_file::ReadFileTool { allowed_dirs }));
+    }
 }

crates/proxy/src/tools/builtin/read_file.rs

@@ -1,12 +1,16 @@
 use crate::tools::registry::Tool;
 use serde_json::Value;
-use std::path::Path;
+use std::path::{Path, PathBuf};
 
 /// Maximum file size to read (1 MB). Prevents OOM from huge files.
 const MAX_FILE_SIZE: u64 = 1024 * 1024;
 
 /// Tool for reading file contents safely.
-pub struct ReadFileTool;
+pub struct ReadFileTool {
+    /// If non-empty, restrict reads to files under these directories.
+    /// All entries must be canonical absolute paths (no symlinks, no ..).
+    pub allowed_dirs: Vec<PathBuf>,
+}
 
 impl Tool for ReadFileTool {
     fn name(&self) -> &str {
@@ -35,6 +39,7 @@ impl Tool for ReadFileTool {
         input: Value,
     ) -> std::pin::Pin<Box<dyn std::future::Future<Output = Result<Value, String>> + Send + 'a>>
     {
+        let allowed_dirs = self.allowed_dirs.clone();
         Box::pin(async move {
             let raw_path = input
                 .get("path")
@@ -43,18 +48,37 @@ impl Tool for ReadFileTool {
 
             let path = Path::new(raw_path);
 
-            // Require absolute paths to prevent path traversal
+            // Require absolute paths to prevent relative path traversal.
             if !path.is_absolute() {
                 return Err("Only absolute paths are allowed".to_string());
             }
 
-            // Resolve symlinks and .. components, then verify the canonical path
-            // still starts with the original parent to block traversal via symlinks
+            // Resolve symlinks and .. components.
             let canonical = path
                 .canonicalize()
                 .map_err(|e| format!("Cannot resolve path '{}': {}", raw_path, e))?;
 
-            // Check file size before reading
+            // Enforce allowed_dirs allowlist. After canonicalize(), the resolved
+            // path must start with at least one of the configured base directories.
+            // This blocks both path traversal (../../../etc) and symlink attacks.
+            if !allowed_dirs.is_empty() {
+                let permitted = allowed_dirs.iter().any(|base| canonical.starts_with(base));
+                if !permitted {
+                    return Err(format!(
+                        "Path '{}' is outside the configured allowed directories",
+                        raw_path
+                    ));
+                }
+            } else {
+                // No allowed_dirs configured: warn the operator.
+                tracing::warn!(
+                    path = %raw_path,
+                    "read_file executed with no allowed_dirs restriction; \
+                     set allowed_dirs in builtin_tools config to restrict file access"
+                );
+            }
+
+            // Check file size before reading.
             let metadata = std::fs::metadata(&canonical)
                 .map_err(|e| format!("Cannot stat '{}': {}", raw_path, e))?;
             if metadata.len() > MAX_FILE_SIZE {