diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 85498448..4ef69c40 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -21,6 +21,11 @@ jobs: # This job executes repo code (cargo build/test); don't persist the # token in git config. persist-credentials: false + # `vendor/tinytools` is a path dependency of this crate, so cargo + # cannot even resolve the manifest without it. Without this the build + # fails at `Updating crates.io index` with "failed to read + # vendor/tinytools/crates/tinytools/Cargo.toml". + submodules: recursive - uses: dtolnay/rust-toolchain@stable with: diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 5ee1bfcd..902917f4 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -31,6 +31,8 @@ jobs: - uses: actions/checkout@v7 with: fetch-depth: 0 + # Required to resolve the `vendor/tinytools` path dependency. + submodules: recursive - uses: dtolnay/rust-toolchain@stable with: diff --git a/.gitmodules b/.gitmodules index a01a23c2..fc0c4cc4 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,3 +1,6 @@ [submodule "wiki"] path = wiki url = https://github.com/tinyhumansai/tinyagents.wiki.git +[submodule "vendor/tinytools"] + path = vendor/tinytools + url = https://github.com/tinyhumansai/tinytools.git diff --git a/Cargo.lock b/Cargo.lock index 7bbd5dbe..20e7d686 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -26,6 +26,12 @@ dependencies = [ "libc", ] +[[package]] +name = "anyhow" +version = "1.0.104" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "330a5ed07fa54e4702c9d6c4174f74427fc0ef6e214bbd677ae50a5099946470" + [[package]] name = "async-trait" version = "0.1.92" @@ -1341,6 +1347,7 @@ dependencies = [ "sha2", "tempfile", "thiserror", + "tinytools", "tokio", "tracing", ] @@ -1355,6 +1362,16 @@ dependencies = [ "zerovec", ] +[[package]] +name = "tinytools" +version = "0.1.0" +dependencies = [ + "anyhow", + "async-trait", + "serde", + "serde_json", +] + [[package]] name = "tinyvec" version = "1.11.0" diff --git a/Cargo.toml b/Cargo.toml index 088c121a..da79f4f2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,6 +11,24 @@ categories = ["asynchronous", "api-bindings"] [dependencies] async-trait = "0.1" +# The tool vocabulary. This crate re-exports the host-facing naming/context +# helpers (`humanize_tool_name`, `context_detail_from_args`, `WorkspaceDescriptor`, +# `SandboxMode`, …) rather than declaring its own copies, so a host and this +# harness read a tool's workspace root and display name identically. +# +# `harness::tool::Tool` itself stays a distinct, harness-owned trait — +# it is generic over application `State` and its signature carries this +# crate's own model-facing dialect types (`ToolCall` / `ToolResult` / +# `ToolPolicy` / `ToolTimeout`), which are a deliberately separate concern from +# `tinytools::Tool`'s host-facing shape. The interop point is +# `ToolExecutionContext` implementing `tinytools::ToolRunContext` (below in +# `harness::tool::types`), which lets a tool read its workspace descriptor +# without this crate naming `ToolExecutionContext` inside `tinytools` (the +# edge points one way: `tinytools` must never depend on this crate). +# +# Path-only for now, which is what stops this crate being published. Publishing +# `tinytools` is the prerequisite; see that repository's AGENTS.md. +tinytools = { path = "vendor/tinytools/crates/tinytools" } # Cheap, reference-counted byte buffers. Used only on the *internal* SSE # byte-stream seam (`harness::providers::openai::sse::SseState`) so each # network chunk from `reqwest::Response::bytes_stream` is forwarded without a diff --git a/docs/modules/harness/workspace.md b/docs/modules/harness/workspace.md index 655f72a1..d5b65e10 100644 --- a/docs/modules/harness/workspace.md +++ b/docs/modules/harness/workspace.md @@ -112,17 +112,22 @@ assert_eq!(kinds, vec!["workspace.prepared", "workspace.cleanup"]); ## Fail-closed path enforcement -Before a tool touches a path, call `WorkspaceDescriptor::enforce(path, &events)`. +Before a tool touches a path, call `enforce_workspace_path(&ws, path, &events)`. It is a fail-closed gate: an allowed path returns `Ok(())` silently; a path outside every allowed root emits `AgentEvent::WorkspaceViolation { path }` and returns `TinyAgentsError::Validation`, so the caller blocks the operation. +`WorkspaceDescriptor` is now `tinytools`' type — its lexical `allows()` check +moved there with it — so the event-emitting half of the old `enforce()` method +is a free function here instead of an inherent method on a foreign type. + ```rust +use tinyagents::harness::workspace::{WorkspaceDescriptor, enforce_workspace_path}; + let ws = WorkspaceDescriptor::new("/work/agent-a"); -ws.enforce(std::path::Path::new("/work/agent-a/out.txt"), &events)?; // allowed, no event +enforce_workspace_path(&ws, std::path::Path::new("/work/agent-a/out.txt"), &events)?; // allowed, no event -let err = ws - .enforce(std::path::Path::new("/etc/passwd"), &events) +let err = enforce_workspace_path(&ws, std::path::Path::new("/etc/passwd"), &events) .expect_err("path outside root must be blocked"); assert!(err.to_string().contains("outside the allowed workspace")); // A `workspace.violation` event was emitted for audit. diff --git a/src/harness/tool/mod.rs b/src/harness/tool/mod.rs index 6f69aa3d..7e60ef7e 100644 --- a/src/harness/tool/mod.rs +++ b/src/harness/tool/mod.rs @@ -25,11 +25,19 @@ use serde_json::Value; use crate::error::{Result, TinyAgentsError}; pub use error_policy::{ToolErrorPolicy, is_control_flow_error}; +// Rendering a tool call for a human is not harness-specific, and two copies of +// the prefix list is how one of them silently stops stripping a prefix the +// other does. The definitions live in `tinytools` so a host that never links +// this crate still renders a tool name the same way. pub use injected::{project_injected_arguments, strip_injected_arguments}; pub use prompt::*; pub use schema::*; pub use schema_prepare::*; pub use timeout::*; +pub use tinytools::{ + ContextDetailOptions, context_detail_from_args, context_detail_from_args_with, + humanize_tool_name, +}; pub use types::*; impl ToolSchema { @@ -272,149 +280,6 @@ impl ToolPolicy { } } -/// Derives a title-cased human-readable label from a raw tool name. -/// -/// Common machine prefixes are stripped, and `snake_case` / `kebab-case` names -/// become spaced labels. Degenerate names fall back to the original input so -/// callers never receive an empty label unless the input itself was empty. -pub fn humanize_tool_name(name: &str) -> String { - let trimmed = name - .strip_prefix("composio_") - .or_else(|| name.strip_prefix("mcp_")) - .unwrap_or(name); - - let mut out = String::with_capacity(trimmed.len()); - let mut capitalize = true; - for ch in trimmed.chars() { - if ch == '_' || ch == '-' { - if !out.is_empty() && !out.ends_with(' ') { - out.push(' '); - } - capitalize = true; - } else if capitalize { - out.extend(ch.to_uppercase()); - capitalize = false; - } else { - out.push(ch); - } - } - - let label = out.trim(); - if label.is_empty() { - name.to_string() - } else { - label.to_string() - } -} - -/// How a context detail is trimmed for display. -/// -/// Exists because the cap and the ellipsis are **presentation**, and a host -/// that renders tool activity in its own timeline has already picked both. The -/// key-scanning rule underneath is what is actually shared; forcing a host to -/// re-implement the whole function to change one character is how two copies of -/// it end up in a codebase. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub struct ContextDetailOptions { - /// Maximum rendered length, in characters, including the ellipsis. - pub max_chars: usize, - /// Appended when the value is trimmed. - pub ellipsis: &'static str, -} - -impl Default for ContextDetailOptions { - fn default() -> Self { - Self { - max_chars: 80, - ellipsis: "...", - } - } -} - -/// Extracts a compact human-facing detail from common tool argument keys. -/// -/// The first recognized scalar value wins, using keys that usually identify the -/// resource being acted on (`path`, `query`, `to`, `url`, and similar). Returns -/// `None` for non-object args, empty values, and complex values. -/// -/// Uses [`ContextDetailOptions::default`]; see -/// [`context_detail_from_args_with`] to choose the cap and ellipsis. -pub fn context_detail_from_args(args: &Value) -> Option { - context_detail_from_args_with(args, ContextDetailOptions::default()) -} - -/// [`context_detail_from_args`] with explicit trimming. -pub fn context_detail_from_args_with( - args: &Value, - options: ContextDetailOptions, -) -> Option { - const CONTEXT_KEYS: &[&str] = &[ - "to", - "recipient", - "recipient_email", - "to_email", - "email", - "query", - "q", - "search", - "search_query", - "url", - "file_path", - "path", - "command", - "cmd", - "subject", - "title", - "channel", - "channel_id", - "repo", - "repository", - "name", - "id", - ]; - - let obj = args.as_object()?; - for key in CONTEXT_KEYS { - let Some(value) = obj.get(*key) else { - continue; - }; - if let Some(rendered) = render_context_value(value, options) { - return Some(rendered); - } - } - None -} - -fn render_context_value(value: &Value, options: ContextDetailOptions) -> Option { - let raw = match value { - Value::String(s) => s.trim().to_string(), - Value::Number(n) => n.to_string(), - Value::Bool(b) => b.to_string(), - Value::Array(items) => items - .iter() - .filter_map(Value::as_str) - .collect::>() - .join(", "), - _ => String::new(), - }; - let raw = raw.split_whitespace().collect::>().join(" "); - if raw.is_empty() { - return None; - } - if raw.chars().count() > options.max_chars { - // Clamp the ellipsis itself to max_chars first: an ellipsis longer than - // the cap (a misconfigured caller) would otherwise survive - // `saturating_sub`'s zero and still get appended in full, pushing the - // rendered value past `max_chars`. - let ellipsis: String = options.ellipsis.chars().take(options.max_chars).collect(); - let keep = options.max_chars.saturating_sub(ellipsis.chars().count()); - let truncated: String = raw.chars().take(keep).collect(); - Some(format!("{truncated}{ellipsis}")) - } else { - Some(raw) - } -} - impl ToolRegistry { /// Creates an empty registry. pub fn new() -> Self { diff --git a/src/harness/tool/types.rs b/src/harness/tool/types.rs index 34f82db3..41fdbaa7 100644 --- a/src/harness/tool/types.rs +++ b/src/harness/tool/types.rs @@ -229,20 +229,38 @@ impl ToolExecutionContext { } } -/// How strictly a tool must be sandboxed when it executes. -#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] -#[serde(rename_all = "snake_case")] -pub enum SandboxMode { - /// Inherit whatever the run's execution environment provides (the default). - #[default] - Inherit, - /// The tool is safe to run without any sandbox. - Disabled, - /// The tool must run inside an isolated execution environment; policy - /// enforcement fails closed if no sandbox is available. - Required, +/// Lets a tool read this context without depending on the harness. +/// +/// A tool is written against `tinytools`, which cannot name this type: this +/// crate depends on `tinytools`, so an edge back would be a cycle. The +/// vocabulary therefore declares a narrow trait and this crate implements it, +/// which is what lets a host hand a live [`ToolExecutionContext`] to a tool +/// that has never heard of the harness. +/// +/// Only the facts a tool actually reads are exposed. The run id, event sink, +/// cancellation token and streaming flag stay harness-internal — a tool that +/// wanted them would be reaching into the run rather than doing its job. +/// +/// `workspace` needs no conversion: [`WorkspaceDescriptor`] is `tinytools`' +/// type, re-exported by this crate, so the field is already the right one. +impl tinytools::ToolRunContext for ToolExecutionContext { + fn workspace(&self) -> Option<&tinytools::WorkspaceDescriptor> { + self.workspace.as_ref() + } + + fn thread_id(&self) -> Option<&str> { + self.thread_id.as_ref().map(ThreadId::as_str) + } + + fn max_turn_output_tokens(&self) -> Option { + self.max_turn_output_tokens + } } +// `SandboxMode` rides on `WorkspaceDescriptor`, which is `tinytools`' type, so +// the mode has to be the same type on both sides of that field. +pub use tinytools::SandboxMode; + /// How a tool is allowed to reach the caller's workspace / filesystem root. #[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] diff --git a/src/harness/workspace/mod.rs b/src/harness/workspace/mod.rs index 51c11668..10090aa3 100644 --- a/src/harness/workspace/mod.rs +++ b/src/harness/workspace/mod.rs @@ -14,6 +14,7 @@ mod policy; mod types; pub use git::*; +pub use policy::enforce_workspace_path; pub use types::*; use std::path::PathBuf; diff --git a/src/harness/workspace/policy.rs b/src/harness/workspace/policy.rs index 1822732c..76747027 100644 --- a/src/harness/workspace/policy.rs +++ b/src/harness/workspace/policy.rs @@ -1,121 +1,42 @@ -//! Path-gating policy for [`WorkspaceDescriptor`]: the `allows`/`enforce` -//! checks and the lexical path-normalization helpers they rely on. +//! The fail-closed path gate for a [`WorkspaceDescriptor`]. //! -//! Split out of `workspace/types.rs`; kept separate from the plain type -//! definitions because this is where the fail-closed security guarantee -//! actually lives. +//! The descriptor and its lexical `allows` check live in `tinytools`, which +//! owns the tool vocabulary. What stays here is the half that needs this +//! crate: emitting a [`WorkspaceViolation`][crate::harness::events::AgentEvent::WorkspaceViolation] +//! and returning this crate's error type. It is a free function rather than an +//! inherent method because the descriptor is now a foreign type. -use std::path::{Path, PathBuf}; +use std::path::Path; -use crate::Result; -use crate::harness::tool::SandboxMode; -use crate::harness::workspace::types::WorkspaceDescriptor; - -impl WorkspaceDescriptor { - /// Creates a descriptor rooted at `root` with no extra trusted roots. - pub fn new(root: impl Into) -> Self { - Self { - root: root.into(), - trusted_roots: Vec::new(), - policy_id: String::new(), - sandbox: SandboxMode::Inherit, - } - } - - /// Adds a trusted root the tool may also touch. - pub fn with_trusted_root(mut self, root: impl Into) -> Self { - self.trusted_roots.push(root.into()); - self - } - - /// Sets the audit policy identity. - pub fn with_policy_id(mut self, id: impl Into) -> Self { - self.policy_id = id.into(); - self - } - - /// Sets the sandbox mode. - pub fn with_sandbox(mut self, sandbox: SandboxMode) -> Self { - self.sandbox = sandbox; - self - } +use tinytools::WorkspaceDescriptor; - /// Returns `true` when `path` is contained within the root or any trusted - /// root. - /// - /// Comparison is lexical (after normalizing `.`/`..` components) so it does - /// not require the path to exist; it is a policy gate, not a canonicalizing - /// filesystem call. Relative candidates and roots are first anchored to the - /// current working directory so a relative path cannot use leading `..` - /// components to spoof re-entry into a same-named sibling of the root. If - /// the current directory cannot be read, the gate fails closed (`false`). - pub fn allows(&self, path: &Path) -> bool { - let Some(candidate) = anchored_normalize(path) else { - return false; - }; - std::iter::once(&self.root) - .chain(self.trusted_roots.iter()) - .filter_map(|root| anchored_normalize(root)) - .any(|root| candidate.starts_with(&root)) - } - - /// Fail-closed path gate to call *before* a tool touches `path`: when the - /// path is outside every allowed root, emits an - /// [`AgentEvent::WorkspaceViolation`][crate::harness::events::AgentEvent::WorkspaceViolation] - /// on `events` and returns a [`TinyAgentsError::Validation`] so the caller - /// blocks the operation. Returns `Ok(())` when the path is allowed. - pub fn enforce(&self, path: &Path, events: &crate::harness::events::EventSink) -> Result<()> { - if self.allows(path) { - return Ok(()); - } - let rendered = path.display().to_string(); - events.emit(crate::harness::events::AgentEvent::WorkspaceViolation { - path: rendered.clone(), - }); - Err(crate::error::TinyAgentsError::Validation(format!( - "path `{rendered}` is outside the allowed workspace roots" - ))) - } -} - -/// Anchors `path` to an absolute base (the current working directory when -/// relative) and lexically normalizes it. Returns `None` when a relative path -/// cannot be anchored because the current directory is unavailable, so callers -/// fail closed. -fn anchored_normalize(path: &Path) -> Option { - let absolute = if path.is_absolute() { - path.to_path_buf() - } else { - std::env::current_dir().ok()?.join(path) - }; - Some(normalize(&absolute)) -} +use crate::Result; +use crate::harness::events::{AgentEvent, EventSink}; -/// Lexically normalizes a path by resolving `.` and `..` components without -/// touching the filesystem. +/// Fail-closed path gate to call *before* a tool touches `path`. +/// +/// When the path is outside every allowed root, emits a +/// [`AgentEvent::WorkspaceViolation`] on `events` and returns a validation +/// error so the caller blocks the operation. Returns `Ok(())` when the path is +/// allowed. +/// +/// # Errors /// -/// A `..` only pops a preceding *named* segment; a `..` that would escape the -/// accumulated prefix (leading or after another `..`) is preserved rather than -/// discarded. Dropping such components would let a relative path like -/// `ws/../../ws/secret` collapse back onto `ws` and spoof re-entry into a -/// same-named sibling directory outside the workspace. -fn normalize(path: &Path) -> PathBuf { - use std::path::Component; - let mut out = PathBuf::new(); - for component in path.components() { - match component { - Component::ParentDir => match out.components().next_back() { - Some(Component::Normal(_)) => { - out.pop(); - } - Some(Component::RootDir | Component::Prefix(_)) => { - // At a filesystem root; `..` cannot go higher. - } - _ => out.push(Component::ParentDir), - }, - Component::CurDir => {} - other => out.push(other.as_os_str()), - } +/// Returns [`TinyAgentsError::Validation`][crate::error::TinyAgentsError::Validation] +/// when `path` lies outside the descriptor's root and trusted roots. +pub fn enforce_workspace_path( + workspace: &WorkspaceDescriptor, + path: &Path, + events: &EventSink, +) -> Result<()> { + if workspace.allows(path) { + return Ok(()); } - out + let rendered = path.display().to_string(); + events.emit(AgentEvent::WorkspaceViolation { + path: rendered.clone(), + }); + Err(crate::error::TinyAgentsError::Validation(format!( + "path `{rendered}` is outside the allowed workspace roots" + ))) } diff --git a/src/harness/workspace/test.rs b/src/harness/workspace/test.rs index 95709b8c..8782c8a8 100644 --- a/src/harness/workspace/test.rs +++ b/src/harness/workspace/test.rs @@ -90,13 +90,11 @@ fn enforce_blocks_unsafe_paths_and_emits_violation() { let ws = WorkspaceDescriptor::new("/work/agent-a"); // Allowed path passes silently with no event. - ws.enforce(Path::new("/work/agent-a/out.txt"), &events) - .unwrap(); + enforce_workspace_path(&ws, Path::new("/work/agent-a/out.txt"), &events).unwrap(); assert!(recorder.is_empty()); // Unsafe path fails closed and emits a violation. - let err = ws - .enforce(Path::new("/etc/passwd"), &events) + let err = enforce_workspace_path(&ws, Path::new("/etc/passwd"), &events) .expect_err("path outside root must be blocked"); assert!(err.to_string().contains("outside the allowed workspace")); assert_eq!(recorder.events()[0].event.kind(), "workspace.violation"); diff --git a/src/harness/workspace/types.rs b/src/harness/workspace/types.rs index e6a1f1d2..59397bea 100644 --- a/src/harness/workspace/types.rs +++ b/src/harness/workspace/types.rs @@ -7,34 +7,16 @@ //! worktrees/sandboxes. TinyAgents does not own any concrete policy; it owns the //! interface so parallel agents can be isolated consistently. -use std::path::PathBuf; - use async_trait::async_trait; -use serde::{Deserialize, Serialize}; use crate::Result; -use crate::harness::tool::SandboxMode; -/// Describes the isolated execution environment a tool is allowed to operate in. -/// -/// A tool discovers its allowed root from this descriptor (via -/// [`ToolExecutionContext::workspace`][crate::harness::tool::ToolExecutionContext::workspace]) -/// instead of reaching for an application global, and a policy engine can call -/// [`allows`](Self::allows) to block unsafe paths before execution. -#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] -pub struct WorkspaceDescriptor { - /// The primary root the agent/tool may read and write under. - pub root: PathBuf, - /// Additional roots the tool is explicitly trusted to touch. - #[serde(default)] - pub trusted_roots: Vec, - /// Identity of the policy that produced this descriptor (for audit). - #[serde(default)] - pub policy_id: String, - /// How strictly the environment is sandboxed. - #[serde(default)] - pub sandbox: SandboxMode, -} +// The descriptor is tool vocabulary — it tells a tool which filesystem root it +// may touch — so it is defined in `tinytools` alongside the trait that reads +// it, and re-exported here at its historical path. `WorkspaceIsolation` stays: +// preparing and tearing down a worktree is harness work, and it returns this +// crate's `Result`. +pub use tinytools::WorkspaceDescriptor; /// Prepares and tears down per-agent execution environments. /// diff --git a/tests/e2e_workspace_and_registry.rs b/tests/e2e_workspace_and_registry.rs index 5ee51309..fbdbfe57 100644 --- a/tests/e2e_workspace_and_registry.rs +++ b/tests/e2e_workspace_and_registry.rs @@ -25,7 +25,9 @@ use tinyagents::harness::tool::{ SandboxMode, Tool, ToolCall, ToolExecutionContext, ToolResult, ToolSchema, }; use tinyagents::harness::usage::Usage; -use tinyagents::harness::workspace::{cleanup_workspace, prepare_workspace}; +use tinyagents::harness::workspace::{ + cleanup_workspace, enforce_workspace_path, prepare_workspace, +}; use tinyagents::language::Blueprint; use tinyagents::{ CapabilityRegistry, ComponentKind, DiagnosticSeverity, RegistrySnapshot, SharedRootWorkspace, @@ -202,9 +204,8 @@ impl Tool<()> for WorkspaceEnforcingTool { .expect("the run was configured with a workspace"); let root = ws.root.display().to_string(); // Enforcing an out-of-root path fails closed and emits a violation event. - let blocked = ws - .enforce(Path::new("/etc/shadow"), &context.events) - .is_err(); + let blocked = + enforce_workspace_path(&ws, Path::new("/etc/shadow"), &context.events).is_err(); Ok(ToolResult::text( call.id, call.name, diff --git a/vendor/tinytools b/vendor/tinytools new file mode 160000 index 00000000..3fe45114 --- /dev/null +++ b/vendor/tinytools @@ -0,0 +1 @@ +Subproject commit 3fe451147a5aaadd45dc15f6a771b83c1c4fccd7