From 713fc3d38122338e8e84d7715d1e5835419dbfae Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Wed, 14 Oct 2026 07:08:39 +0300 Subject: [PATCH 01/12] refactor(artifacts): split artifact paths and types into modules Moved the artifact path helpers and type definitions into separate modules to keep the artifacts code organised as it grows. No behaviour changes. Auto-committed-on: dragonfly --- .../tinyagents-harness/src/artifacts/paths.rs | 138 +++++++----------- .../tinyagents-harness/src/artifacts/types.rs | 31 +--- 2 files changed, 56 insertions(+), 113 deletions(-) diff --git a/crates/tinyagents-harness/src/artifacts/paths.rs b/crates/tinyagents-harness/src/artifacts/paths.rs index 9c55d8815..3e879f94c 100644 --- a/crates/tinyagents-harness/src/artifacts/paths.rs +++ b/crates/tinyagents-harness/src/artifacts/paths.rs @@ -1,13 +1,13 @@ -//! Path hardening for the artifact-offload convention. +//! Key hardening for the artifact-offload convention. //! -//! Everything an agent offloads resolves under -//! `/`. The resolver is deliberately -//! fail-closed: it rejects absolute paths, `..` traversal, anything landing -//! outside its convention root after lexical normalization, and — when the host -//! supplies an [`ArtifactPathPolicy`] — anything reaching the host's internal -//! state. +//! Everything an agent offloads is a blob keyed `/`. +//! The resolver is deliberately fail-closed: it rejects absolute names, `..` +//! traversal and control characters, and — when the host supplies an +//! [`ArtifactPathPolicy`] — anything reaching the host's internal state. +//! There is no filesystem underneath, so there are no symlinks to escape +//! through; the key is the whole address. -use std::path::{Component, Path, PathBuf}; +use std::path::Path; use super::policy::ArtifactPathPolicy; use super::types::{ArtifactKind, OffloadError}; @@ -38,120 +38,80 @@ pub fn sanitize_component(value: &str) -> String { } } -/// Lexically normalize `path` by dropping `.` components and popping a -/// directory for each `..`. +/// Resolve `relative` into the blob key `/`, or explain +/// why it is refused. /// -/// Purely lexical on purpose: the target usually does not exist yet, so -/// `canonicalize` is unavailable. [`resolve_artifact_path`] rejects `..` -/// outright before calling this; the popping here is defence in depth for any -/// future caller. -fn normalize_lexically(path: &Path) -> PathBuf { - let mut out = PathBuf::new(); - for component in path.components() { - match component { - Component::CurDir => {} - Component::ParentDir => { - out.pop(); - } - other => out.push(other.as_os_str()), - } - } - out -} - -/// Render `absolute` as a `root`-relative, `/`-separated path, so the string can -/// be pasted straight into a file-read call on any platform. -/// -/// Falls back to the lossy display form when `absolute` is somehow not under -/// `root` — unreachable via [`resolve_artifact_path`], which validates -/// containment first, but reachable when a caller renders against a *different* -/// root than it wrote to (see `ArtifactOffload::with_render_root`). Falling back -/// to an absolute path is correct there: a wrong relative path would silently -/// resolve against the reader's own root and miss the file. -pub fn relative_to_root(root: &Path, absolute: &Path) -> String { - match absolute.strip_prefix(root) { - Ok(rel) => rel - .components() - .map(|c| c.as_os_str().to_string_lossy().to_string()) - .collect::>() - .join("/"), - Err(_) => absolute.to_string_lossy().to_string(), - } -} - -/// Resolve `relative` into an absolute offload target under -/// `root/`, or explain why it is refused. -/// -/// When `policy` is supplied the resolved path is additionally checked against -/// the host's internal state: +/// `.` and empty segments are dropped. When `policy` is supplied the key is +/// additionally checked against the host's internal state: /// /// * anything [`ArtifactPathPolicy::is_internal_state`] flags is refused with /// [`OffloadError::InternalState`]; /// * anything under [`ArtifactPathPolicy::internal_root`] is refused with /// [`OffloadError::InternalRoot`]. /// -/// The specific check runs **first** so its more precise error survives when a -/// host has configured the artifact root inside its internal root — reporting -/// the blanket containment failure there would lose the useful detail. +/// The specific check runs **first** so its more precise error survives. /// /// Passing `policy: None` skips only those two checks; traversal and /// containment always run. -pub fn resolve_artifact_path( - root: &Path, +pub fn resolve_artifact_key( policy: Option<&dyn ArtifactPathPolicy>, kind: ArtifactKind, relative: &str, -) -> Result { +) -> Result { let trimmed = relative.trim(); if trimmed.is_empty() { return Err(OffloadError::EmptyName); } - - let requested = Path::new(trimmed); - let convention_root = root.join(kind.subdir()); - for component in requested.components() { - match component { - Component::Normal(_) | Component::CurDir => {} - Component::ParentDir => { + if trimmed.starts_with(['/', '\\']) || has_drive_prefix(trimmed) { + return Err(OffloadError::AbsolutePath { + path: trimmed.to_string(), + }); + } + let mut segments: Vec<&str> = Vec::new(); + for segment in trimmed.split(['/', '\\']) { + match segment { + "" | "." => {} + ".." => { return Err(OffloadError::PathEscape { - root: convention_root.display().to_string(), + root: kind.subdir().to_string(), path: trimmed.to_string(), }); } - Component::RootDir | Component::Prefix(_) => { - return Err(OffloadError::AbsolutePath { - path: trimmed.to_string(), + other if other.chars().any(char::is_control) => { + return Err(OffloadError::PathEscape { + root: kind.subdir().to_string(), + path: trimmed.escape_debug().to_string(), }); } + other => segments.push(other), } } - - let candidate = normalize_lexically(&convention_root.join(requested)); - // `normalize_lexically` cannot climb above the root given the `..` - // rejection above, but assert containment anyway so a future relaxation of - // that rejection cannot silently widen the write surface. - if !candidate.starts_with(&convention_root) { - return Err(OffloadError::PathEscape { - root: convention_root.display().to_string(), - path: candidate.display().to_string(), - }); + if segments.is_empty() { + return Err(OffloadError::EmptyName); } + let key = format!("{}/{}", kind.subdir(), segments.join("/")); if let Some(policy) = policy { - if policy.is_internal_state(&candidate) { - return Err(OffloadError::InternalState { - path: candidate.display().to_string(), - }); + let as_path = Path::new(&key); + if policy.is_internal_state(as_path) { + return Err(OffloadError::InternalState { path: key }); } if policy .internal_root() - .is_some_and(|internal_root| candidate.starts_with(internal_root)) + .is_some_and(|internal_root| as_path.starts_with(internal_root)) { - return Err(OffloadError::InternalRoot { - path: candidate.display().to_string(), - }); + return Err(OffloadError::InternalRoot { path: key }); } } - Ok(candidate) + Ok(key) +} + +/// `C:` style prefix, refused so a Windows-absolute name is never a key. +fn has_drive_prefix(value: &str) -> bool { + let mut chars = value.chars(); + matches!( + (chars.next(), chars.next()), + (Some(first), Some(':')) if first.is_ascii_alphabetic() + ) } diff --git a/crates/tinyagents-harness/src/artifacts/types.rs b/crates/tinyagents-harness/src/artifacts/types.rs index a13c2debb..87c1c8056 100644 --- a/crates/tinyagents-harness/src/artifacts/types.rs +++ b/crates/tinyagents-harness/src/artifacts/types.rs @@ -3,8 +3,6 @@ //! Dependency-free by design (std + `thiserror` only) so a host can build //! against these without pulling the engine — RFC §2 rule 2. -use std::path::PathBuf; - /// Deliverables directory under the artifact root. Artifacts here are meant to /// outlive the step that produced them and to be handed to a parent agent or a /// later step **by path**, not by value. @@ -61,16 +59,15 @@ impl ArtifactKind { } } -/// A worker artifact that was successfully written to disk. +/// A worker artifact that was successfully stored as a blob. #[derive(Debug, Clone, PartialEq, Eq)] pub struct OffloadedArtifact { /// Which convention directory it landed in. pub kind: ArtifactKind, - /// Path relative to the render root, always `/`-separated so it can be - /// pasted straight into a file-read call on any platform. + /// The blob key, `/`-separated (`outputs/`). It is what a pointer + /// hands the reader and what [`ArtifactOffload::read`](super::ArtifactOffload::read) + /// takes back. pub relative_path: String, - /// Absolute path on disk. - pub absolute_path: PathBuf, /// Bytes actually stored (post-redaction). pub stored_bytes: usize, /// Bytes of the caller's original payload (pre-redaction). @@ -126,21 +123,7 @@ pub enum OffloadError { path: String, }, - /// The parent directory that actually materialised on disk resolves, - /// through symlinks, to somewhere outside the convention root. - /// - /// The lexical checks in - /// [`resolve_artifact_path`](super::resolve_artifact_path) cannot see this, - /// because the target does not exist yet when they run. - #[error("artifact parent escapes its root through a symlink: {path} resolves to {resolved}")] - SymlinkEscape { - /// The parent directory as requested. - path: String, - /// Where it actually resolves to. - resolved: String, - }, - - /// Creating the parent directory or writing the file failed. - #[error("artifact write failed: {0}")] - Io(#[from] std::io::Error), + /// The blob store refused or failed the write. + #[error("artifact storage failed: {0}")] + Storage(#[from] tinystoragedrivers_core::StorageError), } From 8de8d688528d152d371d8799112e48e32b741bee Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Wed, 14 Oct 2026 07:09:00 +0300 Subject: [PATCH 02/12] feat(artifacts): add artifact operations module Introduce a new artifacts module with types and operations for managing harness artifacts. This provides the foundation for storing and retrieving artifacts produced during agent runs. Auto-committed-on: dragonfly --- .../tinyagents-harness/src/artifacts/mod.rs | 2 +- .../tinyagents-harness/src/artifacts/ops.rs | 181 ++++++++---------- .../tinyagents-harness/src/artifacts/types.rs | 4 +- 3 files changed, 84 insertions(+), 103 deletions(-) diff --git a/crates/tinyagents-harness/src/artifacts/mod.rs b/crates/tinyagents-harness/src/artifacts/mod.rs index 453ada9d1..f9779645b 100644 --- a/crates/tinyagents-harness/src/artifacts/mod.rs +++ b/crates/tinyagents-harness/src/artifacts/mod.rs @@ -75,7 +75,7 @@ pub use ops::{ effective_offload_threshold, extract_artifact_paths, note_artifact_handoff, offload_oversized_result, render_artifact_pointer, should_offload, }; -pub use paths::{relative_to_root, resolve_artifact_path, sanitize_component}; +pub use paths::{resolve_artifact_key, sanitize_component}; pub use policy::{ArtifactPathPolicy, ArtifactRedactor, NoRedaction, OpenPathPolicy, Redacted}; pub use request_truncation::{ RequestTruncation, reducible_tool_result_bytes, truncate_older_tool_results, diff --git a/crates/tinyagents-harness/src/artifacts/ops.rs b/crates/tinyagents-harness/src/artifacts/ops.rs index 8b73bb3f2..41b49c981 100644 --- a/crates/tinyagents-harness/src/artifacts/ops.rs +++ b/crates/tinyagents-harness/src/artifacts/ops.rs @@ -1,9 +1,10 @@ //! Write, pointer-render, and handoff plumbing for the artifact-offload //! convention. //! -//! [`ArtifactOffload`] is the per-run writer: it resolves a target path via -//! [`super::paths::resolve_artifact_path`], applies the host redactor from -//! [`super::policy`], and writes the redacted body to disk. [`should_offload`] +//! [`ArtifactOffload`] is the per-run writer: it resolves a target key via +//! [`super::paths::resolve_artifact_key`], applies the host redactor from +//! [`super::policy`], and stores the redacted body on a +//! [`BlobStore`] port (memory by default). [`should_offload`] //! and [`effective_offload_threshold`] decide *whether* a result should be //! written; [`offload_oversized_result`] ties threshold decision, write, and //! pointer rendering into the single entry point most callers use. @@ -11,13 +12,15 @@ //! side: parsing pointers back out of a handoff payload and logging both ends //! of the exchange. See [`super`] for the convention this module implements. -use std::path::{Path, PathBuf}; use std::sync::Arc; -use super::paths::{relative_to_root, resolve_artifact_path, sanitize_component}; +use tinystoragedrivers_core::{BlobStore, MemoryStorage, Scope, StorageBackend}; + +use super::paths::{resolve_artifact_key, sanitize_component}; use super::policy::{ArtifactPathPolicy, ArtifactRedactor}; use super::types::{ - ABSTRACT_BUDGET_CHARS, ARTIFACT_POINTER_PREFIX, ArtifactKind, OffloadError, OffloadedArtifact, + ABSTRACT_BUDGET_CHARS, ARTIFACT_POINTER_PREFIX, ArtifactKind, OUTPUTS_DIR, OffloadError, + OffloadedArtifact, SCRATCH_DIR, }; /// Whether a payload of `bytes` should be offloaded at `threshold_bytes`. @@ -74,8 +77,8 @@ pub fn render_artifact_pointer( format!( "{ARTIFACT_POINTER_PREFIX} kind={kind} path={path} bytes={bytes}\n\ read_with: {read_tool} {{\"path\":\"{path}\"}}\n\ - note: The full result was written to the action workspace instead of being inlined. \ - Read the file for complete fidelity; the abstract below is not exhaustive.{redaction_note}\n\n\ + note: The full result was stored as an artifact instead of being inlined. \ + Read the artifact for complete fidelity; the abstract below is not exhaustive.{redaction_note}\n\n\ [abstract]\n{abstract_text}", kind = artifact.kind.as_str(), path = artifact.relative_path, @@ -162,15 +165,12 @@ pub fn effective_offload_threshold( /// Per-run writer for the offload convention. /// -/// Holds the resolved artifact root, the host policies used for the fail-closed -/// path checks and redaction, and the identifiers that name generated artifacts -/// and tag the `[artifact]` logs. +/// Holds the blob store the artifacts land on, the host policies used for the +/// fail-closed key checks and redaction, and the identifiers that name +/// generated artifacts and tag the `[artifact]` logs. #[derive(Clone)] pub struct ArtifactOffload { - root: PathBuf, - /// Root the handed-back path is rendered against. Normally identical to - /// `root`; see [`Self::with_render_root`]. - render_root: PathBuf, + blobs: Arc, policy: Option>, redactor: Option>, agent_id: String, @@ -180,8 +180,7 @@ pub struct ArtifactOffload { impl std::fmt::Debug for ArtifactOffload { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("ArtifactOffload") - .field("root", &self.root) - .field("render_root", &self.render_root) + .field("blobs", &self.blobs) .field("policy", &self.policy.as_ref().map(|_| "set")) .field("redactor", &self.redactor.as_ref().map(|_| "set")) .field("agent_id", &self.agent_id) @@ -191,16 +190,20 @@ impl std::fmt::Debug for ArtifactOffload { } impl ArtifactOffload { - /// A writer rooted at `root` for the given agent and task. + /// A writer over `blobs` (a scoped tinystoragedrivers blob handle) for the + /// given agent and task. /// /// Starts with no path policy and no redactor: add them with /// [`with_path_policy`](Self::with_path_policy) and /// [`with_redactor`](Self::with_redactor). A writer with no redactor stores /// bytes verbatim — see [`ArtifactRedactor`]. - pub fn new(root: PathBuf, agent_id: impl Into, task_id: impl Into) -> Self { + pub fn new( + blobs: Arc, + agent_id: impl Into, + task_id: impl Into, + ) -> Self { Self { - render_root: root.clone(), - root, + blobs, policy: None, redactor: None, agent_id: agent_id.into(), @@ -208,32 +211,27 @@ impl ArtifactOffload { } } - /// Consults `policy` for host-internal path refusals. + /// A writer over a private in-memory blob store: the default for library + /// users with no storage backend. Nothing outlives the process. + pub fn in_memory(agent_id: impl Into, task_id: impl Into) -> Self { + let scoped = MemoryStorage::new() + .for_scope(&Scope::new("local").expect("static scope")) + .expect("memory scope"); + Self::new(scoped.blobs().clone(), agent_id, task_id) + } + + /// Consults `policy` for host-internal key refusals. pub fn with_path_policy(mut self, policy: Arc) -> Self { self.policy = Some(policy); self } - /// Scrubs bodies through `redactor` before they touch disk. + /// Scrubs bodies through `redactor` before they are stored. pub fn with_redactor(mut self, redactor: Arc) -> Self { self.redactor = Some(redactor); self } - /// Render handed-back paths relative to `render_root` instead of the write - /// root. - /// - /// An isolated worker writes inside its own checkout, but the parent that - /// receives the pointer does not hold that worker's root — a bare - /// `outputs/…` would resolve against the *parent's* root and miss the file - /// entirely. Rendering against the parent's root yields a relative path when - /// the worker's root is nested inside it, and an absolute path when it is - /// not, so the pointer is never silently wrong. - pub fn with_render_root(mut self, render_root: PathBuf) -> Self { - self.render_root = render_root; - self - } - /// Convention-stable name for a worker's offloaded final result: /// `/-result.md`, both components sanitized. pub fn default_result_name(&self) -> String { @@ -244,16 +242,16 @@ impl ArtifactOffload { ) } - /// Resolve `relative` under this run's root without writing. + /// Resolve `relative` to its blob key without writing. /// - /// Exposed so callers can validate a model-supplied path before acting on it. - pub fn resolve(&self, kind: ArtifactKind, relative: &str) -> Result { - resolve_artifact_path(&self.root, self.policy.as_deref(), kind, relative) + /// Exposed so callers can validate a model-supplied name before acting on it. + pub fn resolve(&self, kind: ArtifactKind, relative: &str) -> Result { + resolve_artifact_key(self.policy.as_deref(), kind, relative) } - /// Write `content` to `relative` under the convention directory for `kind`. + /// Store `content` under `relative` in the convention namespace for `kind`. /// - /// The body passes through the host redactor before it touches disk. + /// The body passes through the host redactor before it is stored. pub async fn write( &self, kind: ArtifactKind, @@ -271,34 +269,30 @@ impl ArtifactOffload { /// Callers surfacing any part of the artifact back into a model's context /// must render it from this value, never from their own input: building a /// preview from the raw text would re-expose exactly the credentials the - /// redactor just scrubbed out of the file. + /// redactor just scrubbed out of the artifact. pub async fn write_returning_stored( &self, kind: ArtifactKind, relative: &str, content: &str, ) -> Result<(OffloadedArtifact, String), OffloadError> { - let absolute = self.resolve(kind, relative)?; - if let Some(parent) = absolute.parent() { - tokio::fs::create_dir_all(parent).await?; - // The checks in `resolve` are lexical, so a pre-existing symlink - // (`outputs -> /elsewhere`, or `outputs/x -> `) would - // still be followed by the write below. Re-validate the parent that - // actually materialised on disk before touching it. - self.assert_real_parent_inside_root(kind, parent).await?; - } + let key = self.resolve(kind, relative)?; let stored = match self.redactor.as_deref() { Some(redactor) => redactor.redact(content), None => super::policy::Redacted::unchanged(content), }; - tokio::fs::write(&absolute, stored.text.as_bytes()).await?; + self.blobs + .put( + &key, + stored.text.clone().into_bytes(), + Some("text/markdown; charset=utf-8"), + ) + .await?; - let relative_path = relative_to_root(&self.render_root, &absolute); let artifact = OffloadedArtifact { kind, - relative_path, - absolute_path: absolute, + relative_path: key, stored_bytes: stored.text.len(), original_bytes: content.len(), redacted: stored.changed, @@ -312,60 +306,47 @@ impl ArtifactOffload { original_bytes = artifact.original_bytes, stored_bytes = artifact.stored_bytes, redacted = artifact.redacted, - "[artifact] wrote worker artifact under the artifact root" + "[artifact] stored worker artifact on the blob store" ); Ok((artifact, stored.text)) } - /// Resolve `parent` through symlinks and confirm it still sits inside this - /// kind's convention root, and outside the host's internal state. + /// Read an artifact back by the key a pointer carried + /// ([`OffloadedArtifact::relative_path`]). /// - /// [`resolve_artifact_path`] cannot do this: its target usually does not - /// exist yet, so it is lexical by necessity. Once `create_dir_all` has run - /// the parent *does* exist, which is the first moment the real, - /// link-resolved location can be checked. - async fn assert_real_parent_inside_root( - &self, - kind: ArtifactKind, - parent: &Path, - ) -> Result<(), OffloadError> { - let convention_root = self.root.join(kind.subdir()); - let real_parent = tokio::fs::canonicalize(parent).await?; - // The root itself may be reached through a symlink (macOS `/tmp` -> - // `/private/tmp` is the everyday case), so compare like with like - // rather than against the lexical root. - let real_root = tokio::fs::canonicalize(&convention_root).await?; - if !real_parent.starts_with(&real_root) { - return Err(OffloadError::SymlinkEscape { - path: parent.display().to_string(), - resolved: real_parent.display().to_string(), - }); - } - if let Some(policy) = self.policy.as_deref() { - let inside_internal_root = match policy.internal_root() { - Some(root) => tokio::fs::canonicalize(root) - .await - .map(|resolved| real_parent.starts_with(&resolved)) - .unwrap_or(false), - None => false, - }; - if policy.is_internal_state(&real_parent) || inside_internal_root { - return Err(OffloadError::SymlinkEscape { - path: parent.display().to_string(), - resolved: real_parent.display().to_string(), - }); - } - } - Ok(()) + /// The key passes the same fail-closed checks as a write, so a pointer a + /// model rewrote by hand cannot address anything outside the two + /// convention namespaces. `Ok(None)` means no such artifact. + pub async fn read(&self, key: &str) -> Result, OffloadError> { + let (kind, relative) = split_key(key).ok_or_else(|| OffloadError::PathEscape { + root: format!("{OUTPUTS_DIR}|{SCRATCH_DIR}"), + path: key.to_string(), + })?; + let key = self.resolve(kind, relative)?; + let blob = self.blobs.get(&key).await?; + Ok(blob.map(|blob| String::from_utf8_lossy(&blob.bytes).into_owned())) } - /// Artifact root this writer is anchored at. - pub fn root(&self) -> &Path { - &self.root + /// The blob store artifacts are written to, for hosts that materialize or + /// list them. + pub fn blobs(&self) -> &Arc { + &self.blobs } } +/// Split `outputs/x` / `workspace/x` into its kind and the name below it. +fn split_key(key: &str) -> Option<(ArtifactKind, &str)> { + let key = key.trim(); + [ArtifactKind::Output, ArtifactKind::Scratch] + .into_iter() + .find_map(|kind| { + key.strip_prefix(kind.subdir()) + .and_then(|rest| rest.strip_prefix('/')) + .map(|rest| (kind, rest)) + }) +} + /// Offload `output` when it exceeds `threshold_bytes`, returning the text the /// parent should receive plus the artifact when one was written. /// diff --git a/crates/tinyagents-harness/src/artifacts/types.rs b/crates/tinyagents-harness/src/artifacts/types.rs index 87c1c8056..ae425398b 100644 --- a/crates/tinyagents-harness/src/artifacts/types.rs +++ b/crates/tinyagents-harness/src/artifacts/types.rs @@ -1,7 +1,7 @@ //! Inert value types for the artifact-offload convention. //! -//! Dependency-free by design (std + `thiserror` only) so a host can build -//! against these without pulling the engine — RFC §2 rule 2. +//! Light by design (std, `thiserror` and the storage error type) so a host can +//! build against these without pulling the engine — RFC §2 rule 2. /// Deliverables directory under the artifact root. Artifacts here are meant to /// outlive the step that produced them and to be handed to a parent agent or a From 08fc6a2477c19d4899e4af7881c7c1338ad6d8a7 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Wed, 14 Oct 2026 07:09:20 +0300 Subject: [PATCH 03/12] feat(media): add image tool for loading and inspecting images Introduce an image tool that lets agents load image files and inspect their metadata, giving them a way to work with image inputs. Supporting types were added to represent the loaded image data. Auto-committed-on: dragonfly --- .../src/media/image_tool.rs | 17 +- crates/tinyagents-harness/src/media/types.rs | 209 ++++-------------- 2 files changed, 60 insertions(+), 166 deletions(-) diff --git a/crates/tinyagents-harness/src/media/image_tool.rs b/crates/tinyagents-harness/src/media/image_tool.rs index e86783438..9670d65b3 100644 --- a/crates/tinyagents-harness/src/media/image_tool.rs +++ b/crates/tinyagents-harness/src/media/image_tool.rs @@ -153,8 +153,8 @@ impl GenerateImageTool { Err(message) => return ToolResult::error(message), } } - let dir = match self.output.dir(workspace) { - Ok(dir) => dir, + let prefix = match self.output.prefix() { + Ok(prefix) => prefix, Err(message) => return ToolResult::error(message), }; @@ -211,13 +211,20 @@ impl GenerateImageTool { }; match self .output - .persist(&dir, &format!("{stem}-{index}"), ext, &image.data) + .persist( + &prefix, + &format!("{stem}-{index}"), + ext, + &image.data, + image.media_type.split(';').next().map_or("", str::trim), + ) + .await { Ok(path) => { - lines.push(format!("- {}", path.display())); + lines.push(format!("- {path}")); artifacts.push(json!({ "type": "image", - "path": path.display().to_string(), + "path": path, "media_type": image.media_type, "bytes": image.data.len(), })); diff --git a/crates/tinyagents-harness/src/media/types.rs b/crates/tinyagents-harness/src/media/types.rs index 55da204e5..6ca2f20d0 100644 --- a/crates/tinyagents-harness/src/media/types.rs +++ b/crates/tinyagents-harness/src/media/types.rs @@ -1,11 +1,10 @@ //! Shared configuration for the media generation tools. -use std::fs::File; -use std::io::Write; use std::path::{Component, Path, PathBuf}; use std::sync::Arc; use serde_json::Value; +use tinystoragedrivers_core::{BlobStore, MemoryStorage, Scope, StorageBackend}; use tinyinference_image::MediaReference; /// Sub-directory of the output root that generated media is written into. @@ -26,27 +25,36 @@ pub struct MediaOutput { /// Local-reference admission policy; `None` confines references to the /// workspace (or fallback) root. pub reference_policy: Option, -} - -/// A verified artifact directory whose open handle prevents a later rename -/// from redirecting writes through a swapped ancestor. -pub(crate) struct ArtifactDirectory { - path: PathBuf, - #[cfg(unix)] - handle: File, + /// Where generated artifacts are stored. Defaults to a private in-memory + /// blob store; hosts pass their scoped blob handle with + /// [`with_blobs`](Self::with_blobs). + pub blobs: Arc, } impl MediaOutput { - /// Writes under `fallback_root/generated-media` when no workspace is known. + /// Resolves local references against `fallback_root` when no workspace is + /// known. Generated artifacts go to the blob store under + /// `generated-media/`, in memory unless [`with_blobs`](Self::with_blobs). #[must_use] pub fn new(fallback_root: impl Into) -> Self { + let scoped = MemoryStorage::new() + .for_scope(&Scope::new("local").expect("static scope")) + .expect("memory scope"); Self { fallback_root: fallback_root.into(), subdir: DEFAULT_MEDIA_SUBDIR.to_owned(), reference_policy: None, + blobs: scoped.blobs().clone(), } } + /// Stores generated artifacts on `blobs` instead of the in-memory default. + #[must_use] + pub fn with_blobs(mut self, blobs: Arc) -> Self { + self.blobs = blobs; + self + } + /// Changes the artifact sub-directory. #[must_use] pub fn with_subdir(mut self, subdir: impl Into) -> Self { @@ -66,180 +74,58 @@ impl MediaOutput { workspace.unwrap_or(&self.fallback_root) } - /// Creates and returns the artifact directory for this call. + /// Validates the artifact sub-directory and returns the blob key prefix + /// (`/`) generated artifacts are stored under. /// - /// Every pre-existing component is inspected without following symlinks, - /// then the resolved directory is checked against the resolved root. This - /// makes a scoped workspace reject an output directory redirected outside - /// the workspace before a generation request can be billed. - pub(crate) fn dir(&self, workspace: Option<&Path>) -> Result { + /// Checked before a generation request is made, so a bad host + /// configuration is refused before the call can be billed. + pub(crate) fn prefix(&self) -> Result { let subdir = Path::new(&self.subdir); if self.subdir.trim().is_empty() || subdir .components() .any(|component| !matches!(component, Component::Normal(_))) + || self.subdir.contains('\\') { return Err(format!( "artifact subdirectory `{}` must be a relative path below the output root", self.subdir )); } - let root = self.root(workspace); - std::fs::create_dir_all(root).map_err(|error| { - format!( - "output root {} could not be created: {error}", - root.display() - ) - })?; - let canonical_root = root.canonicalize().map_err(|error| { - format!( - "output root {} could not be resolved: {error}", - root.display() - ) - })?; - - let mut dir = canonical_root.clone(); - for component in subdir.components() { - let Component::Normal(component) = component else { - unreachable!("subdirectory components were validated above"); - }; - dir.push(component); - match std::fs::symlink_metadata(&dir) { - Ok(metadata) if metadata.file_type().is_symlink() => { - return Err(format!( - "artifact directory {} must not contain symlinks", - dir.display() - )); - } - Ok(metadata) if !metadata.is_dir() => { - return Err(format!( - "artifact path {} is not a directory", - dir.display() - )); - } - Ok(_) => {} - Err(error) if error.kind() == std::io::ErrorKind::NotFound => { - std::fs::create_dir(&dir).map_err(|error| { - format!( - "artifact directory {} could not be created: {error}", - dir.display() - ) - })?; - let metadata = std::fs::symlink_metadata(&dir).map_err(|error| { - format!( - "artifact directory {} could not be inspected: {error}", - dir.display() - ) - })?; - if metadata.file_type().is_symlink() || !metadata.is_dir() { - return Err(format!( - "artifact directory {} changed while being created", - dir.display() - )); - } - } - Err(error) => { - return Err(format!( - "artifact directory {} could not be inspected: {error}", - dir.display() - )); - } - } - } - let canonical_dir = dir.canonicalize().map_err(|error| { - format!( - "artifact directory {} could not be resolved: {error}", - dir.display() - ) - })?; - if !canonical_dir.starts_with(&canonical_root) { - return Err(format!( - "artifact directory {} is outside the output root", - canonical_dir.display() - )); - } - #[cfg(unix)] - let handle = { - use rustix::fs::{Mode, OFlags, open}; - - let handle = open( - &canonical_dir, - OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC, - Mode::empty(), - ) - .map_err(|error| { - format!( - "artifact directory {} could not be opened safely: {error}", - canonical_dir.display() - ) - })?; - File::from(handle) - }; - Ok(ArtifactDirectory { - path: canonical_dir, - #[cfg(unix)] - handle, - }) + let joined = subdir + .components() + .map(|component| component.as_os_str().to_string_lossy().into_owned()) + .collect::>() + .join("/"); + Ok(format!("{joined}/")) } - /// Persists one artifact without following a final-path symlink or - /// replacing an existing file. - pub(crate) fn persist( + /// Stores one artifact without replacing an existing blob, and returns its + /// key (the path the tool reports). + pub(crate) async fn persist( &self, - dir: &ArtifactDirectory, + prefix: &str, stem: &str, extension: &str, bytes: &[u8], - ) -> Result { + content_type: &str, + ) -> Result { if !is_filename_component(stem) || !is_filename_component(extension) { return Err( "artifact name components must be non-empty filename components".to_owned(), ); } - let filename = format!("{stem}.{extension}"); - let path = dir.path.join(&filename); - #[cfg(unix)] - { - use rustix::fs::{Mode, OFlags, openat}; - - // `openat` writes relative to the verified directory handle, so an - // attacker cannot redirect this write by replacing an ancestor. - let file = openat( - &dir.handle, - &filename, - OFlags::WRONLY | OFlags::CREATE | OFlags::EXCL | OFlags::NOFOLLOW | OFlags::CLOEXEC, - Mode::RUSR | Mode::WUSR | Mode::RGRP | Mode::WGRP | Mode::ROTH | Mode::WOTH, - ) - .map(File::from) - .map_err(|error| { - format!( - "artifact {} could not be created safely: {error}", - path.display() - ) - })?; - let mut file = file; - file.write_all(bytes).map_err(|error| { - format!("artifact {} could not be written: {error}", path.display()) - })?; - Ok(path) - } - #[cfg(not(unix))] - { - let mut file = File::options() - .write(true) - .create_new(true) - .open(&path) - .map_err(|error| { - format!( - "artifact {} could not be created safely: {error}", - path.display() - ) - })?; - file.write_all(bytes).map_err(|error| { - format!("artifact {} could not be written: {error}", path.display()) - })?; - Ok(path) + let key = format!("{prefix}{stem}.{extension}"); + match self.blobs.head(&key).await { + Ok(None) => {} + Ok(Some(_)) => return Err(format!("artifact {key} already exists")), + Err(error) => return Err(format!("artifact {key} could not be checked: {error}")), } + self.blobs + .put(&key, bytes.to_vec(), Some(content_type)) + .await + .map_err(|error| format!("artifact {key} could not be stored: {error}"))?; + Ok(key) } /// Converts a model-supplied reference string into a [`MediaReference`], @@ -298,6 +184,7 @@ impl std::fmt::Debug for MediaOutput { .field("fallback_root", &self.fallback_root) .field("subdir", &self.subdir) .field("reference_policy", &self.reference_policy.is_some()) + .field("blobs", &self.blobs) .finish() } } From 296b192521df2a75f97f7c123b2fd4198ceebcb0 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Wed, 14 Oct 2026 07:10:09 +0300 Subject: [PATCH 04/12] feat(media): add image and video tool modules Adds harness tool implementations for image and video media handling. Auto-committed-on: dragonfly --- .../src/media/image_tool.rs | 4 ++-- .../src/media/video_tool.rs | 21 ++++++++++++------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/crates/tinyagents-harness/src/media/image_tool.rs b/crates/tinyagents-harness/src/media/image_tool.rs index 9670d65b3..2a099ab01 100644 --- a/crates/tinyagents-harness/src/media/image_tool.rs +++ b/crates/tinyagents-harness/src/media/image_tool.rs @@ -23,7 +23,7 @@ pub const GENERATE_IMAGE_TOOL_NAME: &str = "generate_image"; const IMAGE_TIMEOUT_MS: u64 = 300_000; /// Generates images with an [`ImageGenerator`] and saves them to the run's -/// workspace. +/// output blob store. /// /// The result lists the saved file paths. A billed call that produced nothing /// is reported as an error that tells the model not to retry, because a retry @@ -47,7 +47,7 @@ impl GenerateImageTool { name: GENERATE_IMAGE_TOOL_NAME.to_owned(), description: "Generate or edit images from a text prompt. Pass reference images \ (URLs or workspace file paths) to edit, restyle, or keep a subject \ - consistent. Saves each image into the workspace and returns its path. \ + consistent. Stores each image as an artifact and returns its key. \ Billed per call: do not call again after an error that says it was billed." .to_owned(), permission: PermissionLevel::Write, diff --git a/crates/tinyagents-harness/src/media/video_tool.rs b/crates/tinyagents-harness/src/media/video_tool.rs index 62698d9cc..e7ec15643 100644 --- a/crates/tinyagents-harness/src/media/video_tool.rs +++ b/crates/tinyagents-harness/src/media/video_tool.rs @@ -20,7 +20,7 @@ use super::{artifact_stem, media_policy}; pub const GENERATE_VIDEO_TOOL_NAME: &str = "generate_video"; /// Generates videos with a [`VideoGenerator`], waits for the job, and saves -/// the clips to the run's workspace. +/// the clips as artifacts on the output blob store. /// /// A job that is still running when the wait budget runs out is reported with /// its id; calling the tool again with `resume_job_id` and its original model collects it without @@ -45,7 +45,7 @@ impl GenerateVideoTool { wait: WaitPolicy::default(), name: GENERATE_VIDEO_TOOL_NAME.to_owned(), description: "Generate a short video clip from a text prompt, optionally starting \ - from (or ending on) an image, and save it into the workspace. Takes \ + from (or ending on) an image, and store it as an artifact. Takes \ minutes. Billed per call: if a job times out, call again with \ `resume_job_id` and the original `model` instead of submitting a new one." .to_owned(), @@ -162,8 +162,8 @@ impl GenerateVideoTool { if let Err(message) = check_string_options(args, &["resume_job_id", "model"]) { return ToolResult::error(message); } - let dir = match self.output.dir(workspace) { - Ok(dir) => dir, + let prefix = match self.output.prefix() { + Ok(prefix) => prefix, Err(message) => return ToolResult::error(message), }; let outcome = if let Some(job_id) = arg_str(args, &["resume_job_id"]) { @@ -236,13 +236,20 @@ impl GenerateVideoTool { }; match self .output - .persist(&dir, &format!("{stem}-{index}"), extension, &video.data) + .persist( + &prefix, + &format!("{stem}-{index}"), + extension, + &video.data, + video.media_type.split(';').next().map_or("", str::trim), + ) + .await { Ok(path) => { - lines.push(format!("- {}", path.display())); + lines.push(format!("- {path}")); artifacts.push(json!({ "type": "video", - "path": path.display().to_string(), + "path": path, "media_type": video.media_type, "bytes": video.data.len(), })); From 6dbe1f32533228757209241ade80c203660d6b15 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Wed, 14 Oct 2026 07:10:26 +0300 Subject: [PATCH 05/12] test(media): add tests for media module Auto-committed-on: dragonfly --- .../tinyagents-harness/src/media/mod_tests.rs | 120 +++++++++++------- 1 file changed, 71 insertions(+), 49 deletions(-) diff --git a/crates/tinyagents-harness/src/media/mod_tests.rs b/crates/tinyagents-harness/src/media/mod_tests.rs index b99ebc99b..7ec54238e 100644 --- a/crates/tinyagents-harness/src/media/mod_tests.rs +++ b/crates/tinyagents-harness/src/media/mod_tests.rs @@ -136,10 +136,11 @@ impl VideoGenerator for WebmVideoGenerator { } #[tokio::test] -async fn image_tool_saves_into_the_workspace_and_reports_paths() { +async fn image_tool_stores_blobs_and_reports_their_keys() { let dir = tempfile::tempdir().unwrap(); let generator = Arc::new(MockImageGenerator::new()); - let tool = GenerateImageTool::new(generator.clone(), MediaOutput::new("/nonexistent")) + let output = MediaOutput::new("/nonexistent"); + let tool = GenerateImageTool::new(generator.clone(), output.clone()) .with_name("media_generate_image"); let result = tool .execute_with_context( @@ -150,10 +151,15 @@ async fn image_tool_saves_into_the_workspace_and_reports_paths() { .await .unwrap(); assert!(!result.is_error, "{}", text(&result)); - let saved: Vec<_> = std::fs::read_dir(dir.path().join("generated-media")) - .unwrap() - .collect(); + let saved = stored(&output).await; assert_eq!(saved.len(), 2); + assert!( + !dir.path().join("generated-media").exists(), + "nothing is written to the workspace" + ); + for key in &saved { + assert!(text(&result).contains(key.as_str()), "{key} is reported"); + } let request = generator.requests().pop().unwrap(); assert_eq!(request.n, Some(2), "numeric strings are accepted"); @@ -335,6 +341,18 @@ async fn a_host_reference_policy_replaces_the_default_confinement() { assert!(!result.is_error, "{}", text(&result)); } +/// Keys of every artifact the output has stored. +async fn stored(output: &MediaOutput) -> Vec { + output + .blobs + .list(&output.prefix().unwrap()) + .await + .unwrap() + .into_iter() + .map(|meta| meta.key) + .collect() +} + fn fast() -> WaitPolicy { WaitPolicy::new(Duration::from_millis(1), Duration::from_secs(5)) } @@ -350,8 +368,8 @@ async fn video_tool_waits_for_delivery_and_saves_the_clip() { ], error: None, })); - let tool = GenerateVideoTool::new(generator.clone(), MediaOutput::new(dir.path())) - .with_wait_policy(fast()); + let output = MediaOutput::new(dir.path()); + let tool = GenerateVideoTool::new(generator.clone(), output.clone()).with_wait_policy(fast()); let result = tool .execute(json!({ "prompt": "a lighthouse", "durationSeconds": 5, "aspect_ratio": "16:9", @@ -361,10 +379,7 @@ async fn video_tool_waits_for_delivery_and_saves_the_clip() { .unwrap(); assert!(!result.is_error, "{}", text(&result)); assert!(text(&result).contains("mock-job")); - let saved: Vec<_> = std::fs::read_dir(dir.path().join("generated-media")) - .unwrap() - .collect(); - assert_eq!(saved.len(), 1); + assert_eq!(stored(&output).await.len(), 1); let request = generator.requests().pop().unwrap(); assert_eq!(request.duration_s, Some(5), "legacy durationSeconds alias"); @@ -385,18 +400,14 @@ fn text_response(text: &str) -> ModelResponse { #[tokio::test] async fn video_tool_preserves_delivered_media_extension() { let dir = tempfile::tempdir().unwrap(); - let tool = GenerateVideoTool::new(Arc::new(WebmVideoGenerator), MediaOutput::new(dir.path())); + let output = MediaOutput::new(dir.path()); + let tool = GenerateVideoTool::new(Arc::new(WebmVideoGenerator), output.clone()); let result = tool.execute(json!({ "prompt": "x" })).await.unwrap(); assert!(!result.is_error, "{}", text(&result)); - let saved: Vec<_> = std::fs::read_dir(dir.path().join("generated-media")) - .unwrap() - .collect(); + let saved = stored(&output).await; assert_eq!(saved.len(), 1); - assert_eq!( - saved[0].as_ref().unwrap().path().extension().unwrap(), - "webm" - ); + assert!(saved[0].ends_with(".webm"), "{}", saved[0]); } /// Regression (R2): a timed-out job names its id and points at resume instead @@ -636,41 +647,52 @@ async fn artifact_subdirectory_must_stay_below_the_output_root() { ); } -/// A pre-existing output symlink must be rejected before the provider is -/// called, otherwise a scoped workspace can write artifacts outside itself. -#[cfg(unix)] #[tokio::test] -async fn symlinked_artifact_directory_cannot_escape_the_workspace() { - let workspace_dir = tempfile::tempdir().unwrap(); - let outside_dir = tempfile::tempdir().unwrap(); - std::os::unix::fs::symlink( - outside_dir.path(), - workspace_dir.path().join("generated-media"), - ) - .unwrap(); +async fn a_host_blob_store_receives_the_artifacts_with_their_content_type() { + use tinystoragedrivers_core::{MemoryStorage, Scope, StorageBackend}; - let generator = Arc::new(MockImageGenerator::new()); - let tool = GenerateImageTool::new(generator.clone(), MediaOutput::new("/nonexistent")); - let result = tool - .execute_with_context( - json!({ "prompt": "x" }), - ToolCallOptions::default(), - Some(&workspace(workspace_dir.path())), - ) + let scoped = MemoryStorage::new() + .for_scope(&Scope::new("host").unwrap()) + .unwrap(); + let blobs = scoped.blobs().clone(); + let output = MediaOutput::new("/nonexistent") + .with_subdir("media/out") + .with_blobs(blobs.clone()); + let tool = GenerateImageTool::new(Arc::new(MockImageGenerator::new()), output); + let result = tool.execute(json!({ "prompt": "x" })).await.unwrap(); + assert!(!result.is_error, "{}", text(&result)); + + let listed = blobs.list("media/out/").await.unwrap(); + assert_eq!(listed.len(), 1); + assert_eq!(listed[0].content_type.as_deref(), Some("image/png")); +} + +#[tokio::test] +async fn persist_refuses_to_replace_an_existing_artifact() { + let output = MediaOutput::new("/nonexistent"); + let prefix = output.prefix().unwrap(); + output + .persist(&prefix, "clip", "png", b"one", "image/png") .await .unwrap(); + let error = output + .persist(&prefix, "clip", "png", b"two", "image/png") + .await + .unwrap_err(); + assert!(error.contains("already exists"), "{error}"); + let kept = output.blobs.get("generated-media/clip.png").await.unwrap(); + assert_eq!(kept.unwrap().bytes, b"one"); +} - assert!(result.is_error, "{}", text(&result)); - assert!( - generator.requests().is_empty(), - "must reject before billing" - ); - assert!( - std::fs::read_dir(outside_dir.path()) - .unwrap() - .next() - .is_none() - ); +#[tokio::test] +async fn persist_refuses_path_like_names() { + let output = MediaOutput::new("/nonexistent"); + let prefix = output.prefix().unwrap(); + let error = output + .persist(&prefix, "../escape", "png", b"x", "image/png") + .await + .unwrap_err(); + assert!(error.contains("filename components"), "{error}"); } #[test] From abad47cd3c1434e0d3c72ab63a49c9da7277099f Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Wed, 14 Oct 2026 07:10:31 +0300 Subject: [PATCH 06/12] chore(tinyagents-harness): drop unused rustix dependency The rustix crate with the fs feature was no longer referenced by the harness, so the dependency entry was removed to keep the manifest lean. Auto-committed-on: dragonfly --- crates/tinyagents-harness/Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/tinyagents-harness/Cargo.toml b/crates/tinyagents-harness/Cargo.toml index 275300062..41f2b39c1 100644 --- a/crates/tinyagents-harness/Cargo.toml +++ b/crates/tinyagents-harness/Cargo.toml @@ -23,7 +23,6 @@ flate2 = { version = "1", optional = true } oxipng = { version = "9", default-features = false, optional = true } futures = { workspace = true } regex = "1" -rustix = { version = "1", features = ["fs"] } reqwest = { workspace = true, features = ["stream", "http2"], optional = true } serde = { workspace = true } serde_json = { workspace = true } From 611b6372530f48b94f9b35227b5d15ac0a91fd6f Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Wed, 14 Oct 2026 07:12:10 +0300 Subject: [PATCH 07/12] test(artifacts): cover the blob-store offload path Rework the artifact tests around the in-memory store and key-based resolution, replacing the filesystem-root fixtures. Adds coverage for namespace separation, read refusals, host-supplied blob stores, and storage errors, and drops the now-unused rustix dependency from the harness lockfile. Auto-committed-on: dragonfly --- Cargo.lock | 1 - .../src/artifacts/mod_tests.rs | 372 ++++++++---------- 2 files changed, 166 insertions(+), 207 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 68b4f827b..85ae733d3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1973,7 +1973,6 @@ dependencies = [ "png", "regex", "reqwest", - "rustix 1.1.4", "serde", "serde_json", "sha2 0.11.0", diff --git a/crates/tinyagents-harness/src/artifacts/mod_tests.rs b/crates/tinyagents-harness/src/artifacts/mod_tests.rs index 651ffbfbd..e985388ac 100644 --- a/crates/tinyagents-harness/src/artifacts/mod_tests.rs +++ b/crates/tinyagents-harness/src/artifacts/mod_tests.rs @@ -3,7 +3,7 @@ //! Covers the happy path (oversized result lands in `outputs/`, the parent gets //! a path + abstract), the fallback path (offload refused, inline payload //! survives for the host's backstop), and the fail-closed path hardening that -//! keeps offload inside the artifact root and out of host-internal state. +//! keeps offload inside its two namespaces and out of host-internal state. use std::path::{Path, PathBuf}; use std::sync::Arc; @@ -71,12 +71,27 @@ impl ArtifactRedactor for TestRedactor { const READ_TOOL: &str = "file_read"; -fn offload_for(root: &Path, internal_root: &Path) -> ArtifactOffload { - ArtifactOffload::new(root.to_path_buf(), "worker-agent", "task-1") - .with_path_policy(Arc::new(TestPolicy::rooted_at(internal_root.to_path_buf()))) +fn offload() -> ArtifactOffload { + ArtifactOffload::in_memory("worker-agent", "task-1") + .with_path_policy(Arc::new(TestPolicy::rooted_at(PathBuf::from("internal")))) .with_redactor(Arc::new(TestRedactor)) } +/// Everything stored, as `(key, body)` pairs. +async fn stored_bodies(offload: &ArtifactOffload) -> Vec<(String, String)> { + let mut out = Vec::new(); + for meta in offload.blobs().list("").await.expect("list") { + let blob = offload + .blobs() + .get(&meta.key) + .await + .expect("get") + .expect("listed blob exists"); + out.push((meta.key, String::from_utf8(blob.bytes).expect("utf8"))); + } + out +} + // ── Kinds and directories ───────────────────────────────────────────────────── #[test] @@ -90,87 +105,74 @@ fn kinds_map_to_the_documented_directories() { // ── Path resolution ─────────────────────────────────────────────────────────── #[test] -fn resolves_under_the_convention_directory() { - let root = PathBuf::from("/action"); - let resolved = - resolve_artifact_path(&root, None, ArtifactKind::Output, "report.md").expect("resolve"); - assert_eq!(resolved, root.join("outputs").join("report.md")); +fn resolves_under_the_convention_namespace() { + let key = resolve_artifact_key(None, ArtifactKind::Output, "report.md").expect("resolve"); + assert_eq!(key, "outputs/report.md"); } #[test] -fn scratch_resolves_under_the_artifact_root_not_the_internal_root() { - // The `workspace` subdir of the artifact root and the host's internal root - // are different places that happen to share a word. Conflating them would - // route every scratch write into host state. - let root = PathBuf::from("/action"); - let internal = PathBuf::from("/internal/workspace"); - let policy = TestPolicy::rooted_at(internal.clone()); - let resolved = resolve_artifact_path(&root, Some(&policy), ArtifactKind::Scratch, "notes.md") +fn scratch_resolves_under_its_own_namespace_not_the_internal_root() { + // The `workspace` namespace of the artifact store and the host's internal + // root are different places that happen to share a word. Conflating them + // would route every scratch write into host state. + let policy = TestPolicy::rooted_at(PathBuf::from("internal/workspace")); + let key = resolve_artifact_key(Some(&policy), ArtifactKind::Scratch, "notes.md") .expect("resolve"); - assert_eq!(resolved, root.join("workspace").join("notes.md")); - assert!(!resolved.starts_with(&internal)); + assert_eq!(key, "workspace/notes.md"); } #[test] fn rejects_parent_traversal() { - let err = resolve_artifact_path( - Path::new("/action"), - None, - ArtifactKind::Output, - "../../etc/passwd", - ) - .expect_err("traversal must be refused"); - assert!( - matches!(err, OffloadError::PathEscape { .. }), - "got {err:?}" - ); + for name in ["../../etc/passwd", "a/../../b", "a\\..\\b"] { + let err = resolve_artifact_key(None, ArtifactKind::Output, name) + .expect_err("traversal must be refused"); + assert!( + matches!(err, OffloadError::PathEscape { .. }), + "{name}: got {err:?}" + ); + } } #[test] -fn rejects_absolute_paths() { - let err = resolve_artifact_path( - Path::new("/action"), - None, - ArtifactKind::Output, - "/etc/passwd", - ) - .expect_err("absolute path must be refused"); - assert!( - matches!(err, OffloadError::AbsolutePath { .. }), - "got {err:?}" - ); +fn rejects_absolute_names() { + for name in ["/etc/passwd", "\\share\\x", "C:\\Windows\\x", "c:/x"] { + let err = resolve_artifact_key(None, ArtifactKind::Output, name) + .expect_err("absolute name must be refused"); + assert!( + matches!(err, OffloadError::AbsolutePath { .. }), + "{name}: got {err:?}" + ); + } +} + +#[test] +fn rejects_control_characters() { + let err = resolve_artifact_key(None, ArtifactKind::Output, "a\0b") + .expect_err("NUL must be refused"); + assert!(matches!(err, OffloadError::PathEscape { .. }), "got {err:?}"); } #[test] fn rejects_empty_and_whitespace_names() { - for name in ["", " ", "\t\n"] { - let err = resolve_artifact_path(Path::new("/action"), None, ArtifactKind::Output, name) + for name in ["", " ", "\t\n", "./", "//"] { + let err = resolve_artifact_key(None, ArtifactKind::Output, name) .expect_err("blank name must be refused"); - assert!(matches!(err, OffloadError::EmptyName), "got {err:?}"); + assert!(matches!(err, OffloadError::EmptyName), "{name:?}: {err:?}"); } } #[test] -fn accepts_leading_current_dir_segments() { +fn accepts_leading_current_dir_segments_and_collapses_empty_ones() { // `./report.md` is how a model commonly writes a relative path; refusing it // would reject a correct request on a formatting technicality. - let resolved = resolve_artifact_path( - Path::new("/action"), - None, - ArtifactKind::Output, - "./report.md", - ) - .expect("resolve"); - assert_eq!(resolved, Path::new("/action/outputs/report.md")); + let key = resolve_artifact_key(None, ArtifactKind::Output, "./a//report.md").expect("resolve"); + assert_eq!(key, "outputs/a/report.md"); } #[test] -fn rejects_targets_inside_the_internal_root_fail_closed() { - // An artifact root configured inside the host's internal root: containment - // passes, so only the policy check stands between an agent and host state. - let root = PathBuf::from("/internal/action"); - let policy = TestPolicy::rooted_at(PathBuf::from("/internal")); - let err = resolve_artifact_path(&root, Some(&policy), ArtifactKind::Output, "leak.md") +fn rejects_keys_inside_the_internal_root_fail_closed() { + let policy = TestPolicy::rooted_at(PathBuf::from("outputs/internal")); + let err = resolve_artifact_key(Some(&policy), ArtifactKind::Output, "internal/leak.md") .expect_err("internal root must be refused"); assert!( matches!(err, OffloadError::InternalRoot { .. }), @@ -179,12 +181,11 @@ fn rejects_targets_inside_the_internal_root_fail_closed() { } #[test] -fn rejects_host_internal_state_paths_with_the_specific_error() { +fn rejects_host_internal_state_keys_with_the_specific_error() { // The specific check runs first so its more useful message survives when a - // path trips both rules. - let root = PathBuf::from("/internal/action"); - let policy = TestPolicy::rooted_at(PathBuf::from("/internal")).with_internal_suffix("outputs"); - let err = resolve_artifact_path(&root, Some(&policy), ArtifactKind::Output, "leak.md") + // key trips both rules. + let policy = TestPolicy::rooted_at(PathBuf::from("outputs")).with_internal_suffix("outputs"); + let err = resolve_artifact_key(Some(&policy), ArtifactKind::Output, "leak.md") .expect_err("internal state must be refused"); assert!( matches!(err, OffloadError::InternalState { .. }), @@ -192,23 +193,6 @@ fn rejects_host_internal_state_paths_with_the_specific_error() { ); } -#[test] -fn a_policy_free_resolve_still_enforces_containment() { - // `policy: None` relaxes only the host checks. If it ever relaxed traversal - // too, every host without a policy would gain an escape. - let err = resolve_artifact_path( - Path::new("/action"), - None, - ArtifactKind::Output, - "../out.md", - ) - .expect_err("traversal must still be refused without a policy"); - assert!( - matches!(err, OffloadError::PathEscape { .. }), - "got {err:?}" - ); -} - #[test] fn sanitize_component_strips_separators_and_never_returns_empty() { assert_eq!(sanitize_component("sub-1a/2b"), "sub-1a_2b"); @@ -221,23 +205,6 @@ fn sanitize_component_strips_separators_and_never_returns_empty() { assert!(sanitize_component(&"x".repeat(500)).len() <= 80); } -#[test] -fn relative_to_root_falls_back_to_display_for_outside_paths() { - // An absolute fallback is correct here: a bogus relative path would resolve - // against the reader's own root and silently miss the file. - let rendered = relative_to_root(Path::new("/action"), Path::new("/elsewhere/outputs/x.md")); - assert_eq!(rendered, "/elsewhere/outputs/x.md"); -} - -#[test] -fn relative_to_root_renders_slash_separated() { - let rendered = relative_to_root( - Path::new("/action"), - &PathBuf::from("/action").join("outputs").join("x.md"), - ); - assert_eq!(rendered, "outputs/x.md"); -} - // ── Thresholds ──────────────────────────────────────────────────────────────── #[test] @@ -307,7 +274,6 @@ fn sample_artifact(redacted: bool) -> OffloadedArtifact { OffloadedArtifact { kind: ArtifactKind::Output, relative_path: "outputs/agent/task-result.md".to_string(), - absolute_path: PathBuf::from("/action/outputs/agent/task-result.md"), stored_bytes: 1234, original_bytes: 1300, redacted, @@ -394,19 +360,9 @@ fn handoff_stages_are_distinct() { // ── Writing ─────────────────────────────────────────────────────────────────── -fn temp_roots() -> (tempfile::TempDir, PathBuf, PathBuf) { - let dir = tempfile::tempdir().expect("tempdir"); - let root = dir.path().join("action"); - let internal = dir.path().join("internal"); - std::fs::create_dir_all(&root).expect("action dir"); - std::fs::create_dir_all(&internal).expect("internal dir"); - (dir, root, internal) -} - #[tokio::test] -async fn write_persists_under_outputs_and_reports_the_relative_path() { - let (_dir, root, internal) = temp_roots(); - let offload = offload_for(&root, &internal); +async fn write_stores_under_outputs_and_reports_the_key() { + let offload = offload(); let artifact = offload .write(ArtifactKind::Output, "report.md", "hello") @@ -417,16 +373,15 @@ async fn write_persists_under_outputs_and_reports_the_relative_path() { assert_eq!(artifact.stored_bytes, 5); assert_eq!(artifact.original_bytes, 5); assert!(!artifact.redacted); - let on_disk = tokio::fs::read_to_string(&artifact.absolute_path) - .await - .expect("read back"); - assert_eq!(on_disk, "hello"); + assert_eq!( + offload.read(&artifact.relative_path).await.expect("read"), + Some("hello".to_string()) + ); } #[tokio::test] -async fn write_redacts_credentials_before_they_reach_disk() { - let (_dir, root, internal) = temp_roots(); - let offload = offload_for(&root, &internal); +async fn write_redacts_credentials_before_they_are_stored() { + let offload = offload(); let body = format!("token: {SECRET}\n"); let artifact = offload @@ -435,40 +390,38 @@ async fn write_redacts_credentials_before_they_reach_disk() { .expect("write"); assert!(artifact.redacted, "the redactor rewrote the body"); - let on_disk = tokio::fs::read_to_string(&artifact.absolute_path) - .await - .expect("read back"); + let stored = stored_bodies(&offload).await; + assert_eq!(stored.len(), 1); assert!( - !on_disk.contains(SECRET), - "the secret reached disk: {on_disk}" + !stored[0].1.contains(SECRET), + "the secret reached the store: {}", + stored[0].1 ); - assert!(on_disk.contains(REDACTION)); - // Original bytes describe the caller's payload, stored bytes the file. + assert!(stored[0].1.contains(REDACTION)); + // Original bytes describe the caller's payload, stored bytes the artifact. assert_eq!(artifact.original_bytes, body.len()); - assert_eq!(artifact.stored_bytes, on_disk.len()); + assert_eq!(artifact.stored_bytes, stored[0].1.len()); } #[tokio::test] async fn write_without_a_redactor_stores_bytes_verbatim() { // The documented consequence of omitting a redactor. Pinned so the default // cannot quietly become "scrub something" and give false assurance. - let (_dir, root, _internal) = temp_roots(); - let offload = ArtifactOffload::new(root.clone(), "agent", "task"); + let offload = ArtifactOffload::in_memory("agent", "task"); let artifact = offload .write(ArtifactKind::Output, "raw.md", SECRET) .await .expect("write"); assert!(!artifact.redacted); - let on_disk = tokio::fs::read_to_string(&artifact.absolute_path) - .await - .expect("read back"); - assert_eq!(on_disk, SECRET); + assert_eq!( + offload.read(&artifact.relative_path).await.expect("read"), + Some(SECRET.to_string()) + ); } #[tokio::test] -async fn write_refuses_a_traversal_target_without_touching_disk() { - let (_dir, root, internal) = temp_roots(); - let offload = offload_for(&root, &internal); +async fn write_refuses_a_traversal_target_without_storing_anything() { + let offload = offload(); let err = offload .write(ArtifactKind::Output, "../escape.md", "x") @@ -480,17 +433,16 @@ async fn write_refuses_a_traversal_target_without_touching_disk() { "got {err:?}" ); assert!( - !root.join("escape.md").exists(), - "nothing may be written on a refused path" + stored_bodies(&offload).await.is_empty(), + "nothing may be stored on a refused key" ); } #[tokio::test] async fn default_result_name_sanitizes_both_identifiers() { - let (_dir, root, _internal) = temp_roots(); - let offload = ArtifactOffload::new(root, "agent/../x", "task id/1"); + let offload = ArtifactOffload::in_memory("agent/../x", "task id/1"); // Each of `/`, `.`, `.`, `/` becomes its own underscore — the separators are - // replaced, never collapsed, so no identifier can smuggle in a path level. + // replaced, never collapsed, so no identifier can smuggle in a key level. assert_eq!( offload.default_result_name(), "agent____x/task_id_1-result.md" @@ -498,77 +450,85 @@ async fn default_result_name_sanitizes_both_identifiers() { } #[tokio::test] -async fn write_refuses_a_parent_that_symlinks_out_of_the_convention_root() { - let (dir, root, internal) = temp_roots(); - let outside = dir.path().join("outside"); - std::fs::create_dir_all(&outside).expect("outside dir"); - std::fs::create_dir_all(root.join("outputs")).expect("outputs dir"); - - // `outputs/escape -> ../../outside`. The lexical checks cannot see this: - // the target does not exist when they run. - #[cfg(unix)] - std::os::unix::fs::symlink(&outside, root.join("outputs").join("escape")).expect("symlink"); - #[cfg(not(unix))] - return; - - let offload = offload_for(&root, &internal); - let err = offload - .write(ArtifactKind::Output, "escape/leak.md", "x") +async fn scratch_and_outputs_are_separate_namespaces() { + let offload = offload(); + offload + .write(ArtifactKind::Output, "a.md", "out") .await - .expect_err("symlink escape must be refused"); + .expect("write output"); + offload + .write(ArtifactKind::Scratch, "a.md", "scratch") + .await + .expect("write scratch"); - assert!( - matches!(err, OffloadError::SymlinkEscape { .. }), - "got {err:?}" + assert_eq!( + offload.read("outputs/a.md").await.expect("read"), + Some("out".to_string()) ); - assert!( - !outside.join("leak.md").exists(), - "the write must not have followed the link" + assert_eq!( + offload.read("workspace/a.md").await.expect("read"), + Some("scratch".to_string()) ); } #[tokio::test] -async fn worktree_artifact_renders_a_path_the_parent_can_resolve() { - // An isolated worker writes in its own checkout; the parent that receives - // the pointer holds a different root. Rendering against the parent's root is - // what keeps the path resolvable on the receiving side. - let (dir, root, internal) = temp_roots(); - let parent_root = dir.path().to_path_buf(); - let offload = offload_for(&root, &internal).with_render_root(parent_root); - - let artifact = offload - .write(ArtifactKind::Output, "report.md", "x") - .await - .expect("write"); +async fn read_returns_none_for_a_missing_artifact() { + assert_eq!(offload().read("outputs/nope.md").await.expect("read"), None); +} - assert_eq!(artifact.relative_path, "action/outputs/report.md"); +#[tokio::test] +async fn read_refuses_keys_outside_the_two_namespaces() { + let offload = offload(); + for key in ["internal/state", "etc/passwd", "outputs", "outputs/../workspace/x"] { + let err = offload.read(key).await.expect_err("must be refused"); + assert!(matches!(err, OffloadError::PathEscape { .. }), "{key}: {err:?}"); + } } #[tokio::test] -async fn a_render_root_outside_the_write_root_falls_back_to_absolute() { - // Better an absolute path than a relative one that resolves against the - // wrong root and silently misses the file. - let (_dir, root, internal) = temp_roots(); - let offload = offload_for(&root, &internal).with_render_root(PathBuf::from("/unrelated")); +async fn artifacts_land_on_a_host_supplied_blob_store() { + use tinystoragedrivers_core::{MemoryStorage, Scope, StorageBackend}; - let artifact = offload - .write(ArtifactKind::Output, "report.md", "x") + let scoped = MemoryStorage::new() + .for_scope(&Scope::new("host").expect("scope")) + .expect("scoped"); + let blobs = scoped.blobs().clone(); + let offload = ArtifactOffload::new(blobs.clone(), "agent", "task"); + + offload + .write(ArtifactKind::Output, "report.md", "body") .await .expect("write"); - assert!( - Path::new(&artifact.relative_path).is_absolute(), - "expected an absolute fallback, got {}", - artifact.relative_path + let blob = blobs + .get("outputs/report.md") + .await + .expect("get") + .expect("present on the host store"); + assert_eq!(blob.bytes, b"body"); + assert_eq!( + blob.meta.content_type.as_deref(), + Some("text/markdown; charset=utf-8") ); } +#[tokio::test] +async fn a_failing_store_surfaces_a_storage_error() { + let offload = offload(); + // A key longer than any driver's limit makes the port refuse the write. + let name = "x".repeat(4096); + let err = offload + .write(ArtifactKind::Output, &name, "x") + .await + .expect_err("the store must refuse an oversized key"); + assert!(matches!(err, OffloadError::Storage(_)), "got {err:?}"); +} + // ── The offload entry point ─────────────────────────────────────────────────── #[tokio::test] -async fn oversized_result_is_offloaded_and_the_parent_gets_a_path_plus_abstract() { - let (_dir, root, internal) = temp_roots(); - let offload = offload_for(&root, &internal); +async fn oversized_result_is_offloaded_and_the_parent_gets_a_key_plus_abstract() { + let offload = offload(); let big = "x".repeat(10_000); let (text, artifact) = offload_oversized_result(big.clone(), &offload, 8_192, READ_TOOL).await; @@ -579,16 +539,20 @@ async fn oversized_result_is_offloaded_and_the_parent_gets_a_path_plus_abstract( text.len() < big.len(), "the pointer must be smaller than the payload it replaced" ); - let on_disk = tokio::fs::read_to_string(&artifact.absolute_path) - .await - .expect("read back"); - assert_eq!(on_disk, big, "full fidelity is preserved on disk"); + assert_eq!( + extract_artifact_paths(&text), + vec![artifact.relative_path.clone()] + ); + assert_eq!( + offload.read(&artifact.relative_path).await.expect("read"), + Some(big), + "full fidelity is preserved in the store" + ); } #[tokio::test] async fn small_result_stays_inline() { - let (_dir, root, internal) = temp_roots(); - let offload = offload_for(&root, &internal); + let offload = offload(); let (text, artifact) = offload_oversized_result("small".to_string(), &offload, 8_192, READ_TOOL).await; @@ -599,8 +563,7 @@ async fn small_result_stays_inline() { #[tokio::test] async fn offload_is_disabled_by_a_zero_threshold() { - let (_dir, root, internal) = temp_roots(); - let offload = offload_for(&root, &internal); + let offload = offload(); let big = "x".repeat(10_000); let (text, artifact) = offload_oversized_result(big.clone(), &offload, 0, READ_TOOL).await; @@ -612,10 +575,8 @@ async fn offload_is_disabled_by_a_zero_threshold() { #[tokio::test] async fn offload_failure_keeps_the_inline_payload_for_the_host_backstop() { // The load-bearing soft-failure contract: a refused offload must never cost - // the caller its content, or a disk problem turns into data loss. - let (_dir, _root, internal) = temp_roots(); - let unwritable = PathBuf::from("/proc/nonexistent-artifact-root"); - let offload = offload_for(&unwritable, &internal); + // the caller its content, or a storage problem turns into data loss. + let offload = ArtifactOffload::in_memory(&"a".repeat(4096), "task"); let big = "x".repeat(10_000); let (text, artifact) = offload_oversized_result(big.clone(), &offload, 8_192, READ_TOOL).await; @@ -631,9 +592,8 @@ async fn offload_failure_keeps_the_inline_payload_for_the_host_backstop() { async fn abstract_is_built_from_the_redacted_body_not_the_raw_output() { // The pointer goes straight into the parent's context. Building the // abstract from the raw text would re-expose the credential that was just - // scrubbed out of the file — and the file would still look correct. - let (_dir, root, internal) = temp_roots(); - let offload = offload_for(&root, &internal); + // scrubbed out of the artifact — and the artifact would still look correct. + let offload = offload(); let body = format!("{SECRET} {}", "x".repeat(10_000)); let (text, artifact) = offload_oversized_result(body, &offload, 8_192, READ_TOOL).await; From 4f68749d0d1ba63319ade897ae43db6efe720785 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Wed, 14 Oct 2026 07:12:43 +0300 Subject: [PATCH 08/12] test(artifacts): cover path policy in offload failure test The blank-name rejection test now exercises a dotted double-slash name instead of a plain double slash, and the offload soft-failure test builds its offload with a path policy that marks every key under outputs as host-internal, so the refusal comes from policy rather than an unrelated cause. Auto-committed-on: dragonfly --- crates/tinyagents-harness/src/artifacts/mod_tests.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/tinyagents-harness/src/artifacts/mod_tests.rs b/crates/tinyagents-harness/src/artifacts/mod_tests.rs index e985388ac..823beb6b3 100644 --- a/crates/tinyagents-harness/src/artifacts/mod_tests.rs +++ b/crates/tinyagents-harness/src/artifacts/mod_tests.rs @@ -154,7 +154,7 @@ fn rejects_control_characters() { #[test] fn rejects_empty_and_whitespace_names() { - for name in ["", " ", "\t\n", "./", "//"] { + for name in ["", " ", "\t\n", "./", ".//."] { let err = resolve_artifact_key(None, ArtifactKind::Output, name) .expect_err("blank name must be refused"); assert!(matches!(err, OffloadError::EmptyName), "{name:?}: {err:?}"); @@ -576,7 +576,11 @@ async fn offload_is_disabled_by_a_zero_threshold() { async fn offload_failure_keeps_the_inline_payload_for_the_host_backstop() { // The load-bearing soft-failure contract: a refused offload must never cost // the caller its content, or a storage problem turns into data loss. - let offload = ArtifactOffload::in_memory(&"a".repeat(4096), "task"); + // Every key under `outputs` is host-internal state here, so the write is + // refused by policy. + let offload = ArtifactOffload::in_memory("agent", "task").with_path_policy(Arc::new( + TestPolicy::rooted_at(PathBuf::from("internal")).with_internal_suffix("outputs"), + )); let big = "x".repeat(10_000); let (text, artifact) = offload_oversized_result(big.clone(), &offload, 8_192, READ_TOOL).await; From 1efcd1b4f56655e5ac6b67532a9300e55044e72f Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Wed, 14 Oct 2026 07:13:04 +0300 Subject: [PATCH 09/12] docs(artifacts): describe blob-store offload instead of filesystem paths The artifacts module documentation now reflects that oversized worker results are stored as blobs on a tinystoragedrivers BlobStore rather than written to the filesystem, so pointers carry a blob key instead of a path and the symlink-escape caveats no longer apply. Auto-committed-on: dragonfly --- .../src/artifacts/README.md | 24 +++++++++---------- .../tinyagents-harness/src/artifacts/mod.rs | 17 ++++++------- 2 files changed, 18 insertions(+), 23 deletions(-) diff --git a/crates/tinyagents-harness/src/artifacts/README.md b/crates/tinyagents-harness/src/artifacts/README.md index 37c2267b5..3fa45a607 100644 --- a/crates/tinyagents-harness/src/artifacts/README.md +++ b/crates/tinyagents-harness/src/artifacts/README.md @@ -1,7 +1,7 @@ # harness::artifacts -Filesystem offload for oversized worker results on long-horizon runs, plus the -fail-closed path resolution and host policy hooks it depends on. +Blob-store offload for oversized worker results on long-horizon runs, plus the +fail-closed key resolution and host policy hooks it depends on. ## Why this exists @@ -64,20 +64,22 @@ already had. | ------------ | --------------------------------------------------------------------- | | `mod.rs` | Module overview, re-exports. | | `types.rs` | `ArtifactKind`, `OffloadedArtifact`, `OffloadError`, constants. | -| `paths.rs` | Fail-closed path resolution (`resolve_artifact_path` and helpers). | +| `paths.rs` | Fail-closed key resolution (`resolve_artifact_key`, `sanitize_component`). | | `policy.rs` | `ArtifactPathPolicy`, `ArtifactRedactor` traits and their defaults. | | `tool_results.rs` | Per-tool-result persistence: `ToolResultArtifactStore` (`new`: under `/artifacts/tool-results/`, relative pointers; `detached`: under `/tool-results/` outside the working tree, absolute pointers), `apply_per_result_persistence`, `spill_aggregate_tool_results`, paged artifact reads (`read_target`). Host supplies redactor, read/wrapper tool names, read limit, and for a detached store the read grant on its artifact namespace. | | `tool_results_tests.rs` | Tests for the above, incl. a byte-stable envelope fixture. | -| `ops.rs` | `ArtifactOffload` writer, `offload_oversized_result`, pointer/handoff plumbing. | +| `ops.rs` | `ArtifactOffload` writer over a `BlobStore`, `offload_oversized_result`, pointer/handoff plumbing. | | `mod_tests.rs`, `contract_tests.rs`, `policy_tests.rs`, `request_truncation_tests.rs` | Unit tests: happy path, fallback path, fail-closed hardening. | ## Operational constraints -- `resolve_artifact_path` is lexical only — it cannot see symlinks, because - the write target usually does not exist yet. `ArtifactOffload::write` - re-validates the real, symlink-resolved parent directory after - `create_dir_all` and before the write; that is the first moment a - pre-existing symlink escape can be detected. +- Artifacts are blobs on a tinystoragedrivers `BlobStore` (in memory unless + the host passes its scoped handle to `ArtifactOffload::new`). A pointer + carries the blob key (`outputs/`), not a filesystem path: read it back + with `ArtifactOffload::read`, or have the host expose the store through its + own read tool or materialise it. `resolve_artifact_key` is fail-closed on + `..`, absolute names, drive prefixes and control characters; there are no + symlinks to escape through. - A `None` redactor stores bytes verbatim. That is a legitimate choice for a host with nothing to scrub, but it is a security decision the host must make explicitly — this crate cannot supply credential/PII patterns for it. @@ -85,10 +87,6 @@ already had. **stored** (post-redaction) body, never from the caller's original input. `write_returning_stored` and `offload_oversized_result` exist specifically so callers have that stored value at hand. -- `ArtifactOffload::with_render_root` matters whenever a worker writes inside - its own checkout: the parent receiving the pointer does not hold the - worker's root, so paths must be rendered relative to the *parent's* root - (or absolute, when the worker's root is not nested inside it). - Every write and every handoff emits a structured `[artifact]` log line, so a run journal shows both ends of a pointer (see `mod.rs` for the exact messages). diff --git a/crates/tinyagents-harness/src/artifacts/mod.rs b/crates/tinyagents-harness/src/artifacts/mod.rs index f9779645b..a28669471 100644 --- a/crates/tinyagents-harness/src/artifacts/mod.rs +++ b/crates/tinyagents-harness/src/artifacts/mod.rs @@ -1,4 +1,4 @@ -//! Filesystem offload for oversized worker artifacts on long-horizon runs. +//! Blob-store offload for oversized worker artifacts on long-horizon runs. //! //! ## The problem //! @@ -37,21 +37,18 @@ //! //! ## Hardening //! -//! [`resolve_artifact_path`] is fail-closed. Absolute paths, `..` traversal, and -//! anything escaping the convention root are refused; when an +//! [`resolve_artifact_key`] is fail-closed. Absolute names, drive prefixes, `..` +//! traversal and control characters are refused; when an //! [`ArtifactPathPolicy`] is supplied, so is anything reaching the host's -//! internal state. -//! -//! The lexical checks cannot see symlinks, because the target does not exist -//! when they run. So the real parent directory is re-validated after -//! `create_dir_all` and before the write — that is the first moment the -//! link-resolved location can be checked at all. +//! internal state. The artifacts live on a tinystoragedrivers blob store, not +//! the filesystem, so the key is the whole address and there is no symlink to +//! follow out of the namespace. //! //! ## What the host supplies //! //! Two policies, both of which a redistributed crate cannot decide for itself: //! [`ArtifactPathPolicy`] (which paths are off limits) and [`ArtifactRedactor`] -//! (what is scrubbed before bytes hit disk). See [`policy`]. +//! (what is scrubbed before bytes are stored). See [`policy`]. //! //! ## Logging //! From fc7317078f1a7c76b50b30364e8ec553633a9ae6 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Wed, 14 Oct 2026 07:13:11 +0300 Subject: [PATCH 10/12] docs(artifacts): update stale doc references Correct the doc comments to match the current API and log output, renaming the resolve_artifact_path reference to resolve_artifact_key and updating the write log line to describe storing on the blob store. Auto-committed-on: dragonfly --- crates/tinyagents-harness/src/artifacts/contract.rs | 2 +- crates/tinyagents-harness/src/artifacts/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/tinyagents-harness/src/artifacts/contract.rs b/crates/tinyagents-harness/src/artifacts/contract.rs index b24970cea..c80cd252a 100644 --- a/crates/tinyagents-harness/src/artifacts/contract.rs +++ b/crates/tinyagents-harness/src/artifacts/contract.rs @@ -1,7 +1,7 @@ //! The prompt-side half of the artifact-offload convention (#3883). //! //! Single source of truth for the directory names the model is told about, so -//! the prompt and [`super::resolve_artifact_path`] can never drift. The write +//! the prompt and [`super::resolve_artifact_key`] can never drift. The write //! tool the contract may name is a parameter: tool names are host vocabulary. use std::collections::HashSet; diff --git a/crates/tinyagents-harness/src/artifacts/mod.rs b/crates/tinyagents-harness/src/artifacts/mod.rs index a28669471..f29ba0085 100644 --- a/crates/tinyagents-harness/src/artifacts/mod.rs +++ b/crates/tinyagents-harness/src/artifacts/mod.rs @@ -52,7 +52,7 @@ //! //! ## Logging //! -//! Every write emits `[artifact] wrote worker artifact under the artifact root`, +//! Every write emits `[artifact] stored worker artifact on the blob store`, //! and every path a handoff carries emits `[artifact] handoff carried an //! artifact path`, so a run journal shows both ends of a pointer. From 2dac230422d5a96c3d304d971f34e7302668e3e2 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Wed, 14 Oct 2026 07:13:24 +0300 Subject: [PATCH 11/12] docs(artifacts): document blob store offload for artifacts and media The changelog now records that ArtifactOffload and MediaOutput write to a tinystoragedrivers BlobStore instead of the filesystem, with pointers carrying blob keys and the old path-based APIs removed. The artifacts README was updated to match the new public surface, and the not-yet-moved list now points at the remaining path-bound transcript and tool-result stores. Auto-committed-on: dragonfly --- CHANGELOG.md | 27 ++++++++++++++++--- .../src/artifacts/README.md | 15 +++++------ 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9afdcca6c..580675b6d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,23 @@ drivers. `ConversationError::Legacy` and `::Worker` are removed. - Classic `memory/conversations/` files are no longer read by this crate. +### Artifact offload and generated media: blob store + +- `artifacts::ArtifactOffload` writes to a tinystoragedrivers `BlobStore` + (`ArtifactOffload::new(blobs, agent, task)`; `ArtifactOffload::in_memory` + is the default for library hosts) instead of the filesystem. Pointers carry + the blob key (`outputs/`); `ArtifactOffload::read` and `::blobs` + fetch it back. Removed: `ArtifactOffload::root` and `::with_render_root`, + `OffloadedArtifact::absolute_path`, `OffloadError::Io` and `::SymlinkEscape` + (new `OffloadError::Storage`), `relative_to_root`, and + `resolve_artifact_path` (now `resolve_artifact_key`, with no root argument). +- `media::MediaOutput` stores generated images and clips on a blob store + (`MediaOutput::with_blobs`, in-memory by default) under `generated-media/`; + tool results report the blob key as `path`. Local reference inputs still + resolve against the workspace. `rustix` is no longer a dependency. +- Hosts that let the model open these artifacts by file path must expose the + store through their read tool or materialise it themselves. + ### Removed, with the replacement | Removed | Use instead | @@ -100,9 +117,11 @@ let report = import_workspace(&provider, "agent-id", workspace_dir).await?; ### Not yet moved These still use files and are migrated in follow-ups (tracked in -tinyhumansai/openhuman#7688 and #7768): +tinyhumansai/openhuman#7688, #7768 and #7783): - the JSONL transcript writer, reader and `FileTranscriptLocator` - (path-bound; `session_raw/` adoption, spend and view helpers read them); -- the artifact offload (`artifacts::ArtifactOffload`) and media outputs, which - write files the model later opens by path. + (path-bound; `session_raw/` adoption, spend and view helpers read them, + about 12k lines across `session::transcript`); +- `artifacts::tool_results::ToolResultArtifactStore`, whose pointers are + action-relative or absolute file paths the host's `file_read` opens, and + whose pruning relies on file mtimes (a blob has no modification time). diff --git a/crates/tinyagents-harness/src/artifacts/README.md b/crates/tinyagents-harness/src/artifacts/README.md index 3fa45a607..712af1d0c 100644 --- a/crates/tinyagents-harness/src/artifacts/README.md +++ b/crates/tinyagents-harness/src/artifacts/README.md @@ -31,10 +31,10 @@ already had. ## Public surface -- [`ArtifactOffload`] (`ops.rs`) — per-run writer holding the resolved - artifact root, host policies, and the ids used to name artifacts and tag - logs. `write` / `write_returning_stored` perform a redacted write; - `resolve` validates a path without writing. +- [`ArtifactOffload`] (`ops.rs`) — per-run writer holding a + `BlobStore`, host policies, and the ids used to name artifacts and tag + logs. `write` / `write_returning_stored` perform a redacted write; `read` + fetches one back by key; `resolve` validates a name without writing. - [`offload_oversized_result`] (`ops.rs`) — the deterministic entry point: offloads `output` when it exceeds a threshold, returning the text the parent should receive plus the artifact when one was written. @@ -43,12 +43,11 @@ already had. [`note_artifact_handoff`] (`ops.rs`) — the supporting free functions: threshold decision, cap-aware threshold, abstract truncation, pointer rendering, pointer parsing, and handoff logging. -- [`resolve_artifact_path`], [`relative_to_root`], [`sanitize_component`] - (`paths.rs`) — fail-closed path resolution under the convention root and - the helpers it builds on. +- [`resolve_artifact_key`], [`sanitize_component`] (`paths.rs`) — fail-closed + key resolution under the two convention namespaces and the helper it builds on. - [`policy`] module (`policy.rs`) — the two host-supplied gates: [`ArtifactPathPolicy`] (which paths are off limits) and - [`ArtifactRedactor`] (what is scrubbed before bytes hit disk), plus the + [`ArtifactRedactor`] (what is scrubbed before bytes are stored), plus the permissive [`OpenPathPolicy`] / [`NoRedaction`] defaults used in tests and by hosts with nothing to restrict. - [`ArtifactKind`], [`OffloadedArtifact`], [`OffloadError`] and the From 026dd502778c06118728d60999d9fec765f48c0c Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Wed, 14 Oct 2026 07:14:42 +0300 Subject: [PATCH 12/12] style: apply rustfmt formatting to harness tests and imports Auto-committed-on: dragonfly --- .../src/artifacts/mod_tests.rs | 25 +++++++++++++------ .../tinyagents-harness/src/media/mod_tests.rs | 4 +-- crates/tinyagents-harness/src/media/types.rs | 2 +- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/crates/tinyagents-harness/src/artifacts/mod_tests.rs b/crates/tinyagents-harness/src/artifacts/mod_tests.rs index 823beb6b3..8bc2d7b20 100644 --- a/crates/tinyagents-harness/src/artifacts/mod_tests.rs +++ b/crates/tinyagents-harness/src/artifacts/mod_tests.rs @@ -116,8 +116,8 @@ fn scratch_resolves_under_its_own_namespace_not_the_internal_root() { // root are different places that happen to share a word. Conflating them // would route every scratch write into host state. let policy = TestPolicy::rooted_at(PathBuf::from("internal/workspace")); - let key = resolve_artifact_key(Some(&policy), ArtifactKind::Scratch, "notes.md") - .expect("resolve"); + let key = + resolve_artifact_key(Some(&policy), ArtifactKind::Scratch, "notes.md").expect("resolve"); assert_eq!(key, "workspace/notes.md"); } @@ -147,9 +147,12 @@ fn rejects_absolute_names() { #[test] fn rejects_control_characters() { - let err = resolve_artifact_key(None, ArtifactKind::Output, "a\0b") - .expect_err("NUL must be refused"); - assert!(matches!(err, OffloadError::PathEscape { .. }), "got {err:?}"); + let err = + resolve_artifact_key(None, ArtifactKind::Output, "a\0b").expect_err("NUL must be refused"); + assert!( + matches!(err, OffloadError::PathEscape { .. }), + "got {err:?}" + ); } #[test] @@ -479,9 +482,17 @@ async fn read_returns_none_for_a_missing_artifact() { #[tokio::test] async fn read_refuses_keys_outside_the_two_namespaces() { let offload = offload(); - for key in ["internal/state", "etc/passwd", "outputs", "outputs/../workspace/x"] { + for key in [ + "internal/state", + "etc/passwd", + "outputs", + "outputs/../workspace/x", + ] { let err = offload.read(key).await.expect_err("must be refused"); - assert!(matches!(err, OffloadError::PathEscape { .. }), "{key}: {err:?}"); + assert!( + matches!(err, OffloadError::PathEscape { .. }), + "{key}: {err:?}" + ); } } diff --git a/crates/tinyagents-harness/src/media/mod_tests.rs b/crates/tinyagents-harness/src/media/mod_tests.rs index 7ec54238e..6477e86e7 100644 --- a/crates/tinyagents-harness/src/media/mod_tests.rs +++ b/crates/tinyagents-harness/src/media/mod_tests.rs @@ -140,8 +140,8 @@ async fn image_tool_stores_blobs_and_reports_their_keys() { let dir = tempfile::tempdir().unwrap(); let generator = Arc::new(MockImageGenerator::new()); let output = MediaOutput::new("/nonexistent"); - let tool = GenerateImageTool::new(generator.clone(), output.clone()) - .with_name("media_generate_image"); + let tool = + GenerateImageTool::new(generator.clone(), output.clone()).with_name("media_generate_image"); let result = tool .execute_with_context( json!({ "prompt": "a comic", "n": "2", "aspectRatio": "landscape", "seed": 5 }), diff --git a/crates/tinyagents-harness/src/media/types.rs b/crates/tinyagents-harness/src/media/types.rs index 6ca2f20d0..7dbf05d98 100644 --- a/crates/tinyagents-harness/src/media/types.rs +++ b/crates/tinyagents-harness/src/media/types.rs @@ -4,8 +4,8 @@ use std::path::{Component, Path, PathBuf}; use std::sync::Arc; use serde_json::Value; -use tinystoragedrivers_core::{BlobStore, MemoryStorage, Scope, StorageBackend}; use tinyinference_image::MediaReference; +use tinystoragedrivers_core::{BlobStore, MemoryStorage, Scope, StorageBackend}; /// Sub-directory of the output root that generated media is written into. pub const DEFAULT_MEDIA_SUBDIR: &str = "generated-media";