diff --git a/Cargo.lock b/Cargo.lock index 6c1052e..333b656 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -50,17 +50,6 @@ dependencies = [ "syn", ] -[[package]] -name = "async-trait" -version = "0.1.89" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9035ad2d096bed7955a320ee7e2230574d28fd3c3a0f186cbea1ff3c7eed5dbb" -dependencies = [ - "proc-macro2", - "quote", - "syn", -] - [[package]] name = "autocfg" version = "1.5.1" @@ -307,12 +296,6 @@ dependencies = [ "windows-sys 0.61.2", ] -[[package]] -name = "fastrand" -version = "2.4.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9f1f227452a390804cdb637b74a86990f2a7d7ba4b7d5693aac9b4dd6defd8d6" - [[package]] name = "find-msvc-tools" version = "0.1.9" @@ -582,12 +565,6 @@ version = "0.2.186" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "68ab91017fe16c622486840e4c83c9a37afeff978bd239b5293d61ece587de66" -[[package]] -name = "linux-raw-sys" -version = "0.12.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "32a66949e030da00e8c7d4434b251670a91556f4144941d37452769c25d58a53" - [[package]] name = "litemap" version = "0.8.2" @@ -1014,19 +991,6 @@ dependencies = [ "nom", ] -[[package]] -name = "rustix" -version = "1.1.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b6fe4565b9518b83ef4f91bb47ce29620ca828bd32cb7e408f0062e9930ba190" -dependencies = [ - "bitflags", - "errno", - "libc", - "linux-raw-sys", - "windows-sys 0.61.2", -] - [[package]] name = "rustls" version = "0.23.41" @@ -1204,12 +1168,10 @@ checksum = "f8fadd59c855ef2080decdef8ff161eb6661b86933c9d82e5ba29dc602a55aba" name = "shoal-types" version = "0.1.0" dependencies = [ - "async-trait", "chrono", "mtp", "serde", "serde_json", - "tempfile", "thiserror", "tokio", "uuid", @@ -1281,19 +1243,6 @@ dependencies = [ "syn", ] -[[package]] -name = "tempfile" -version = "3.27.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd" -dependencies = [ - "fastrand", - "getrandom 0.4.3", - "once_cell", - "rustix", - "windows-sys 0.61.2", -] - [[package]] name = "thiserror" version = "2.0.18" diff --git a/Cargo.toml b/Cargo.toml index c99c10c..f95f263 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,8 +12,6 @@ serde = { version = "1", features = ["derive"] } serde_json = "1" mtp = { git = "https://git@git.methanium.net/methanium/mtp.git", features = [] } tokio = "1" -async-trait = "0.1" [dev-dependencies] tokio = { version = "1", features = ["full"] } -tempfile = "3" diff --git a/src/lib.rs b/src/lib.rs index 39e809d..f479187 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -12,7 +12,6 @@ pub mod errors; pub mod krill; pub mod mutations; pub mod project; -pub mod sandbox; pub mod sync; pub mod task; pub mod todo; @@ -35,8 +34,7 @@ pub use tools::{ format_tool_error, format_tools_json, format_tool_result, parse_tool_call_blocks, parse_tool_call_stream, tool_definitions, tool_ids_for_agent_type, ApprovalRequirement, CompositionStep, ContextVisibility, ExecutionContext, MarketplaceSearchResults, ParsedToolCall, - REEF_PROXY_TOOLS, GLOBAL_TOOLS, is_reef_proxy_tool, is_global_tool, TestAction, ToolComposition, ToolDefinition, ToolDependency, ToolDocumentation, ToolExample, - ToolListing, ToolParser, ToolPlugin, ToolExecutor, ToolRegistry, ToolTest, ToolTestResult, ToolTestSuite, + ToolListing, ToolParser, ToolPlugin, ToolTest, ToolTestResult, ToolTestSuite, ToolTestSuiteResult, ToolVersion, }; diff --git a/src/sandbox.rs b/src/sandbox.rs deleted file mode 100644 index 459e951..0000000 --- a/src/sandbox.rs +++ /dev/null @@ -1,210 +0,0 @@ -use std::path::{Path, PathBuf, Component}; - -/* Maximum allowed path depth as a safety measure */ -const MAX_PATH_DEPTH: usize = 64; - -/* Validate a file path to prevent directory traversal, null bytes, shell -metacharacters, and other path-based attacks. Returns the normalized path -if valid, or an error message. - -This is the canonical path validation shared by all runtimes (connector-krill, -Reef API, etc.). */ -pub fn validate_path(path: &str) -> Result { - if path.is_empty() { - return Err("Path cannot be empty".to_string()); - } - - if path.len() > 4096 { - return Err("Path exceeds maximum length (4096 characters)".to_string()); - } - - let path = Path::new(path); - - if has_directory_traversal(path) { - return Err("Path contains directory traversal ('..') which is not allowed".to_string()); - } - - if contains_null_bytes(path) { - return Err("Path contains null bytes which is not allowed".to_string()); - } - - if contains_shell_metacharacters(path) { - return Err("Path contains shell metacharacters which is not allowed".to_string()); - } - - let depth = path.components().count(); - if depth > MAX_PATH_DEPTH { - return Err(format!( - "Path depth ({}) exceeds maximum allowed ({})", - depth, MAX_PATH_DEPTH - )); - } - - Ok(path.to_path_buf()) -} - -/* Given a validated path and a sandbox root, resolve the full path and -verify it does not escape the sandbox. Handles both existing and new paths. -The `validated` path should already have passed `validate_path`. */ -pub fn resolve_sandboxed_path(validated: &Path, sandbox_root: &Path) -> Result { - let full_path = if validated.is_absolute() { - validated.to_path_buf() - } else { - sandbox_root.join(validated) - }; - - let canonical = if full_path.exists() { - full_path.canonicalize().map_err(|e| { - format!("Failed to canonicalize path: {}", e) - })? - } else { - if let Some(parent) = full_path.parent() { - if parent.exists() { - let canonical_parent = parent.canonicalize().map_err(|e| { - format!("Failed to canonicalize parent path: {}", e) - })?; - if !canonical_parent.starts_with(sandbox_root) { - return Err("Path escapes sandbox root boundaries".to_string()); - } - } else { - return Err("Parent directory does not exist".to_string()); - } - } - full_path - }; - - if !canonical.starts_with(sandbox_root) { - return Err("Path escapes sandbox root boundaries".to_string()); - } - - Ok(canonical) -} - -fn has_directory_traversal(path: &Path) -> bool { - path.components().any(|comp| matches!(comp, Component::ParentDir)) -} - -fn contains_null_bytes(path: &Path) -> bool { - path.to_str().map_or(true, |s| s.contains('\0')) -} - -fn contains_shell_metacharacters(path: &Path) -> bool { - const SHELL_METACHARACTERS: &[char] = &[ - '|', ';', '&', '$', '`', '>', '<', '(', ')', '{', '}', - '!', '#', '*', '?', '[', ']', '~', '\n', '\r', - ]; - path.to_str().map_or(true, |s| s.contains(SHELL_METACHARACTERS)) -} - -#[cfg(test)] -mod tests { - use super::*; - use std::fs; - use tempfile::tempdir; - - #[test] - fn test_rejects_empty_path() { - let result = validate_path(""); - assert!(result.is_err()); - } - - #[test] - fn test_rejects_directory_traversal() { - let attacks = vec![ - "../etc/passwd", - "../../etc/passwd", - "file/../../../etc/passwd", - "a/../../b/../../../c", - "..", - ]; - for path in attacks { - let result = validate_path(path); - assert!(result.is_err(), "Expected '{}' to be rejected", path); - } - } - - #[test] - fn test_accepts_valid_paths() { - let valid = vec![ - "file.txt", - "subdir/file.txt", - "a/b/c/d/file.rs", - "src/main.rs", - "docs/README.md", - "project_name/src/lib.rs", - ]; - for path in valid { - let result = validate_path(path); - assert!( - result.is_ok(), - "Expected '{}' to be accepted: {:?}", - path, - result.err() - ); - } - } - - #[test] - fn test_rejects_path_with_null_bytes() { - assert!(validate_path("file\0.txt").is_err()); - } - - #[test] - fn test_rejects_path_with_shell_metacharacters() { - let attacks = vec![ - "file|echo", "file;rm", "file$(id)", "file`id`", "file>out", "file>().join("/"); - assert!(validate_path(&deep).is_err()); - } - - #[test] - fn test_sandboxed_path_within_root() { - let dir = tempdir().unwrap(); - let file_path = dir.path().join("test.txt"); - fs::write(&file_path, "content").unwrap(); - - let validated = validate_path("test.txt").unwrap(); - let result = resolve_sandboxed_path(&validated, dir.path()); - assert!(result.is_ok()); - assert_eq!(result.unwrap(), file_path); - } - - #[test] - fn test_sandboxed_path_escape_via_absolute() { - let dir = tempdir().unwrap(); - let outside_file = std::env::temp_dir().join("escape_test.txt"); - fs::write(&outside_file, "content").unwrap(); - - let validated = validate_path(outside_file.to_str().unwrap()).unwrap(); - let result = resolve_sandboxed_path(&validated, dir.path()); - assert!(result.is_err()); - let _ = fs::remove_file(&outside_file); - } - - #[test] - fn test_sandboxed_path_new_file() { - let dir = tempdir().unwrap(); - - let validated = validate_path("new_file.txt").unwrap(); - let result = resolve_sandboxed_path(&validated, dir.path()); - assert!(result.is_ok()); - } - - #[test] - fn test_sandboxed_path_nonexistent_parent() { - let dir = tempdir().unwrap(); - - let validated = validate_path("nonexistent_dir/file.txt").unwrap(); - let result = resolve_sandboxed_path(&validated, dir.path()); - assert!(result.is_err()); - } -} diff --git a/src/tools.rs b/src/tools.rs index 069ddaf..8f7beb0 100644 --- a/src/tools.rs +++ b/src/tools.rs @@ -1,4 +1,3 @@ -use async_trait::async_trait; use serde::{Deserialize, Serialize}; /* Context in which a tool should be available */ @@ -228,20 +227,6 @@ impl ToolDefinition { pub fn is_compatible_with(&self, other: &Self) -> bool { self.name == other.name && !self.deprecated && !other.deprecated } - - /* Execution tier classification based on the tool's properties. - Tier 1 (Transient): global tools with no project state — always local - Tier 2 (Project Read): read-only project-scoped tools — local or Reef proxy - Tier 3 (Persistent): mutation tools + Reef-only tools — Reef-backed */ - pub fn tier(&self) -> u8 { - if is_global_tool(&self.name) { - 1 - } else if self.mutates() || self.execution_context == ExecutionContext::Reef { - 3 - } else { - 2 - } - } } /* Simple semver comparison (major.minor.patch) */ @@ -510,6 +495,22 @@ impl Default for ToolVersion { } } +/* Execution context for tool filtering. Determines which tools are +available in which execution environment (Reef vs. Krill vs. both). */ +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum ToolContext { + Reef, + Krill, + Both, +} + +impl Default for ToolContext { + fn default() -> Self { + Self::Both + } +} + /* Tool category classification for UI grouping and filtering. */ #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] @@ -667,29 +668,6 @@ pub trait ToolPlugin: Send + Sync { } } -/// Shared trait for executing a tool. -/// -/// Every Krill binary (connector-krill, example-krill, custom) implements -/// this trait for each tool it supports. The trait is deliberately simple — -/// it takes parsed JSON arguments and returns either a string result or an -/// error. Wrapping in a sandbox/project context is handled by the caller. -#[async_trait] -pub trait ToolExecutor: Send + Sync { - fn definition(&self) -> ToolDefinition; - - async fn execute(&self, arguments: &serde_json::Value) -> Result; -} - -/// A registry of tools that can be looked up by name and executed. -#[async_trait] -pub trait ToolRegistry: Send + Sync { - async fn register(&self, tool: ToolDefinition, executor: Box); - - async fn list_tools(&self) -> Vec; - - async fn execute(&self, name: &str, arguments: &serde_json::Value) -> Result; -} - /* A marketplace listing for a published tool. */ #[derive(Debug, Clone, Serialize, Deserialize)] pub struct ToolListing { @@ -842,41 +820,6 @@ NOT here: they stay Everywhere so direct chat can call them under approval routi (Phase 4). Only genuinely agent-internal collaboration tools are agent-only. */ pub const AGENT_ONLY_TOOLS: &[&str] = &["submit_batch_plan", "report_completion"]; -/* Tools that proxy to the Reef API when a `REEF_URL` / `REEF_API_URL` is -configured. These tools have both a local implementation and a Reef-backed -implementation (via Reservoir). When Reef is available the Reef path is -preferred so that file operations go through the Reservoir-backed sandbox; -when Reef is unreachable the local fallback executes instead. - -This is the Krill → Reef proxy set, not the full set of Reef-only tools. -Tools like kanban_*, document_*, and strategic items are always executed on -Reef via MTP and are NOT in this list — they have no local implementation. */ -pub const REEF_PROXY_TOOLS: &[&str] = &[ - "read_file", "file_read", - "write_file", "file_write", - "edit_file", - "list_directory", - "search_files", - "grep", - "bash", "execute", - "delete_workspace", - "list_files", -]; - -/* Tools that operate without a project scope (global). These are always -executed locally on the Krill and never need project_id injection. */ -pub const GLOBAL_TOOLS: &[&str] = &["web_search", "web_fetch", "web_api"]; - -/* Returns true if the tool name is in the global (non-project-scoped) set. */ -pub fn is_global_tool(name: &str) -> bool { - GLOBAL_TOOLS.contains(&name) -} - -/* Returns true if the tool name should proxy to Reef when Reef is available. */ -pub fn is_reef_proxy_tool(name: &str) -> bool { - REEF_PROXY_TOOLS.contains(&name) -} - /* Returns all built-in tool definitions across all categories. */ pub fn tool_definitions() -> Vec { let mut tools = vec![