Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/tinytools-std/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ walkdir = "2"

[dev-dependencies]
tempfile = "3"
tokio = { workspace = true }
tokio = { workspace = true, features = ["test-util"] }

[lints]
workspace = true
3 changes: 2 additions & 1 deletion crates/tinytools-std/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ Host-independent building blocks for agent tools, extracted from OpenHuman.
| `file_state` | Process-wide read/write stamps so parallel agents detect stale or partial reads before overwriting a file. The host decides whether the guard is on (`init_global(enabled)`); read tools pass `record_read` an `Instant` captured before their I/O. |
| `url_guard` | URL validation with SSRF checks for outbound network tools. `validate_url_with_dns_check` returns a `ValidatedUrl` whose vetted `addrs` the caller must pin its HTTP client to (e.g. `reqwest`'s `resolve_to_addrs`); re-resolving the hostname reopens DNS rebinding. |
| `filesystem` | The file and repository tools: `file_read`, `file_write`, `edit_file`, `apply_patch`, `grep`, `glob`, `list_files`, `csv_export`, `read_diff`, `git_operations`, `run_linter`, `run_tests`, `update_memory_md`, `image_info`, `read_workspace_state`. Every tool that can touch a path takes an `Arc<dyn FsGate>`: the tool does the I/O, the host's gate (autonomy, workspace boundary, approvals, action budget) decides whether it may. |
| `network` | The network tools: `http_request`, `web_fetch`, `curl`, `pushover`. Every tool takes an `Arc<dyn NetGate>`: the tool does the I/O, the host's gate (autonomy, action budget, approval, privacy mode, proxy) decides whether it may and how. Two smaller seams keep host behavior out: `PaymentHook` answers a `402 Payment Required` for `http_request`, and `HtmlExtractor` converts pages to Markdown for `web_fetch`. Names, descriptions and schemas are pinned by `src/network/fixtures/`. |
| `network` | The network tools: `http_request`, `web_fetch`, `curl`, `pushover`. Every tool takes an `Arc<dyn NetGate>`: the tool does the I/O, the host's gate (autonomy, action budget, approval, privacy mode, proxy) decides whether it may and how. Two smaller seams keep host behavior out: `PaymentHook` answers a `402 Payment Required` for `http_request`, and `HtmlExtractor` converts pages to Markdown for `web_fetch`. `WebFetchTool::new_async` accepts a fallible `AsyncHtmlExtractor` for transforms supplied by a remote service or loadable module; detection and extraction are awaited with the configured request timeout applied to each operation, and failures propagate without a local retry. Both constructors extract before applying the output cap, and skip extraction for raw or explicitly non-HTML responses. Names, descriptions and schemas are pinned by `src/network/fixtures/`. |
Comment thread
senamakel marked this conversation as resolved.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

priority medium critique likely

Document the extractor input ceiling

This documentation now explicitly says extraction happens before the output cap, so max_bytes does not bound the response text supplied to either extractor. A large remote response can therefore be fully buffered and passed to a local or asynchronous extractor before the result limit is applied. Add an enforced input ceiling (preferably while reading the response) and document that separate limit; otherwise callers have no protection against oversized extractor input.

[RULE] unbounded-extractor-input ·

| `sanitize` | Lexical sanitization helpers for untrusted tool, skill and capability metadata, including control and instruction-fence removal plus UTF-8-safe byte truncation. |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

priority medium critique confident

Integrate sanitization into metadata production paths

The sanitizer is still only defined and tested; the repository search shows no production caller of sanitize_for_llm. Consequently, tool, skill, and capability metadata can still reach the LLM context without the control-character, instruction-fence, and byte-limit protections this entry describes. Wire the sanitizer into each metadata construction/forwarding path, or narrow this documentation to a helper that callers must explicitly invoke.

[RULE] missing-sanitization-integration ·

| `detect_tools` | `find_on_path` and the read-only `detect_tools` tool. |

No enforcement of host policy lives here; the crate only supplies mechanisms. The `filesystem` tools' name, description and JSON Schema are pinned by the fixtures in `src/filesystem/fixtures/`.
2 changes: 2 additions & 0 deletions crates/tinytools-std/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
//! - [`url_guard`] — URL validation with SSRF checks, plus DNS resolution
//! that returns the vetted addresses for the caller to pin its connection to.
//! - [`detect_tools`] — `PATH` probing and the read-only `detect_tools` tool.
//! - [`sanitize`] — lexical sanitization and truncation for untrusted metadata.
//!
//! # Example
//!
Expand All @@ -35,4 +36,5 @@ pub mod detect_tools;
pub mod file_state;
pub mod filesystem;
pub mod network;
pub mod sanitize;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

priority medium security confident

Integrate sanitization into metadata production paths

Exporting the sanitizer does not apply it anywhere: the repository has no call sites for sanitize_for_llm, while tool, skill, and capability metadata can still flow directly into the LLM context. Wire sanitize_for_llm into the metadata construction or forwarding paths so callers cannot accidentally bypass the lexical defence.


Additional critique observation

priority medium confident

Route metadata through the sanitization pipeline

[RULE] missing-sanitization-integration

This only exposes the sanitizer; the repository-wide search shows no production caller of sanitize_for_llm or strip_instruction_fences. Consequently, remote tool/skill/capability descriptions can still flow directly into the LLM context despite the module documentation claiming that the pipeline runs before metadata is stored or forwarded. Integrate the pipeline at each metadata construction or forwarding boundary, with the owning byte caps.

[RULE] untrusted-input-sanitization ·

pub mod url_guard;
2 changes: 1 addition & 1 deletion crates/tinytools-std/src/network/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,4 @@ pub use curl::CurlTool;
pub use gate::{HttpLimits, NetGate};
pub use http_request::{HttpRequestTool, PaymentAttempt, PaymentHook, PaymentOutcome};
pub use pushover::PushoverTool;
pub use web_fetch::{HtmlExtractor, WebFetchTool};
pub use web_fetch::{AsyncHtmlExtractor, HtmlExtractor, WebFetchTool};
178 changes: 150 additions & 28 deletions crates/tinytools-std/src/network/web_fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,66 @@ pub trait HtmlExtractor: std::fmt::Debug + Send + Sync {
fn to_markdown(&self, html: &str) -> String;
}

/// An asynchronous, fallible HTML transform supplied by the host.
///
/// Use this interface when extraction runs in a separate service or module.
/// Failures propagate to the fetch caller; the tool does not retry locally.
#[async_trait]
Comment thread
senamakel marked this conversation as resolved.
pub trait AsyncHtmlExtractor: std::fmt::Debug + Send + Sync {
/// Whether a response without a usable content type contains HTML.
///
/// # Errors
/// Returns the host's detection failure.
async fn looks_like_html(&self, body: &str) -> anyhow::Result<bool>;

/// Convert HTML to Markdown, preserving links and dropping scripts.
///
/// # Errors
/// Returns the host's extraction failure.
async fn to_markdown(&self, html: &str) -> anyhow::Result<String>;
}

#[derive(Debug)]
enum HtmlProvider {
Local(Arc<dyn HtmlExtractor>),
Async {
extractor: Arc<dyn AsyncHtmlExtractor>,
timeout: Duration,
},
}

impl HtmlProvider {
async fn looks_like_html(&self, body: &str) -> anyhow::Result<bool> {
match self {
Self::Local(html) => Ok(html.looks_like_html(body)),
Self::Async { extractor, timeout } => {
tokio::time::timeout(*timeout, extractor.looks_like_html(body))
.await
.map_err(|_| anyhow::anyhow!("HTML detection timed out"))?
}
}
}

async fn to_markdown(&self, body: &str) -> anyhow::Result<String> {
match self {
Self::Local(html) => Ok(html.to_markdown(body)),
Self::Async { extractor, timeout } => {
tokio::time::timeout(*timeout, extractor.to_markdown(body))
.await
.map_err(|_| anyhow::anyhow!("HTML extraction timed out"))?
}
}
}
}

/// Fetches a URL and returns its text body.
#[derive(Debug)]
pub struct WebFetchTool {
gate: Arc<dyn NetGate>,
allowed_domains: Vec<String>,
max_bytes: usize,
timeout_secs: u64,
html: Arc<dyn HtmlExtractor>,
html: HtmlProvider,
extra_properties: Vec<(String, serde_json::Value)>,
}

Expand All @@ -53,6 +105,63 @@ impl WebFetchTool {
timeout_secs: Option<u64>,
defaults: HttpLimits,
html: Arc<dyn HtmlExtractor>,
) -> Self {
Self::with_provider(
gate,
allowed_domains,
max_bytes,
timeout_secs,
defaults,
HtmlProvider::Local(html),
)
}

/// A `web_fetch` tool with asynchronous HTML detection and extraction.
///
/// Limits and raw-response handling match [`Self::new`]. Extraction errors
/// propagate without invoking another provider. Both asynchronous detection
/// and conversion are bounded by the configured request timeout.
#[must_use]
pub fn new_async(
gate: Arc<dyn NetGate>,
allowed_domains: Vec<String>,
max_bytes: Option<usize>,
timeout_secs: Option<u64>,
defaults: HttpLimits,
html: Arc<dyn AsyncHtmlExtractor>,
) -> Self {
let resolved_timeout = match timeout_secs {
Some(0) => {
log::warn!(
"[tool.web_fetch] coercing invalid limit field=timeout_secs from=0 to={} \
(stale/invalid config — see migration 5→6)",
defaults.timeout_secs
);
defaults.timeout_secs
}
Some(seconds) => seconds,
None => defaults.timeout_secs,
};
Self::with_provider(
gate,
allowed_domains,
max_bytes,
Some(resolved_timeout),
defaults,
HtmlProvider::Async {
extractor: html,
timeout: Duration::from_secs(resolved_timeout),
},
)
}

fn with_provider(
gate: Arc<dyn NetGate>,
allowed_domains: Vec<String>,
max_bytes: Option<usize>,
timeout_secs: Option<u64>,
defaults: HttpLimits,
html: HtmlProvider,
) -> Self {
// Treat both `None` and `Some(0)` as "use default": callers wire these
// from `[http_request]`, and a 0-byte cap truncates every body to
Expand Down Expand Up @@ -295,12 +404,9 @@ impl WebFetchTool {
status.as_u16(),
retry_after.is_some()
);
let excerpt = error_body_excerpt(
self.html.as_ref(),
&body,
content_type.as_deref(),
raw_requested,
);
let excerpt =
error_body_excerpt(&self.html, &body, content_type.as_deref(), raw_requested)
.await?;
return Ok(ToolResult::error(http_error_message(
status,
&host,
Expand All @@ -317,8 +423,8 @@ impl WebFetchTool {
// 1,083,069 input tokens. The host's `HtmlExtractor` owns
// content transforms.
let converted =
!raw_requested && is_html(self.html.as_ref(), &body, content_type.as_deref());
let rendered = render_body(self.html.as_ref(), body, converted, max_bytes);
!raw_requested && is_html(&self.html, &body, content_type.as_deref()).await?;
let rendered = render_body(&self.html, body, converted, max_bytes).await?;
Comment thread
senamakel marked this conversation as resolved.
Comment thread
senamakel marked this conversation as resolved.

let extracted = rendered.extracted;
let mut header = format!("status={} url={final_url}", status.as_u16());
Expand Down Expand Up @@ -395,22 +501,23 @@ fn http_error_message(
///
/// HTML goes through the host extractor (unless the caller asked for `raw`)
/// so the model reads the page's words rather than its markup.
fn error_body_excerpt(
extractor: &dyn HtmlExtractor,
async fn error_body_excerpt(
extractor: &HtmlProvider,
body: &str,
content_type: Option<&str>,
raw_requested: bool,
) -> String {
let text = if !raw_requested && is_html(extractor, body, content_type) {
extractor.to_markdown(body)
) -> anyhow::Result<String> {
let (bounded_body, _) = cap_extractor_input(body);
let text = if !raw_requested && is_html(extractor, bounded_body, content_type).await? {
extractor.to_markdown(bounded_body).await?
} else {
body.to_string()
};
let collapsed = text.split_whitespace().collect::<Vec<_>>().join(" ");
match collapsed.char_indices().nth(ERROR_EXCERPT_CHARS) {
Ok(match collapsed.char_indices().nth(ERROR_EXCERPT_CHARS) {
Some((cut, _)) => format!("{}...", &collapsed[..cut]),
None => collapsed,
}
})
}

/// Raw markup handed to the HTML extractor, at most.
Expand Down Expand Up @@ -462,20 +569,20 @@ fn append_markup_truncation_header(header: &mut String, rendered: &RenderedBody)
/// Bounding the output is also what the schema promises, and it costs no
/// memory: the caller has already materialised the whole body. Only the
/// extractor's input needs a ceiling, and that is for CPU.
fn render_body(
html: &dyn HtmlExtractor,
async fn render_body(
html: &HtmlProvider,
body: String,
converted: bool,
max_bytes: usize,
) -> RenderedBody {
let (markup_truncated, body) = if converted && body.len() > EXTRACTOR_INPUT_CEILING {
let cut = floor_char_boundary(&body, EXTRACTOR_INPUT_CEILING);
(true, body[..cut].to_string())
) -> anyhow::Result<RenderedBody> {
let (markup_truncated, body) = if converted {
let (body, truncated) = cap_extractor_input(&body);
(truncated, body.to_string())
} else {
(false, body)
};
let full = if converted {
html.to_markdown(&body)
html.to_markdown(&body).await?
Comment thread
senamakel marked this conversation as resolved.
} else {
body
};
Expand All @@ -486,11 +593,21 @@ fn render_body(
} else {
(full, false)
};
RenderedBody {
Ok(RenderedBody {
content,
extracted,
output_capped,
markup_truncated,
})
}

/// Bound every body before it crosses into a host supplied extractor.
fn cap_extractor_input(body: &str) -> (&str, bool) {
if body.len() > EXTRACTOR_INPUT_CEILING {
let cut = floor_char_boundary(body, EXTRACTOR_INPUT_CEILING);
(&body[..cut], true)
} else {
(body, false)
}
}

Expand All @@ -509,20 +626,25 @@ fn floor_char_boundary(s: &str, index: usize) -> usize {
/// Is this HTML? The server's own `Content-Type` is authoritative when it
/// says so; otherwise fall back to the host's content detection, which
/// already distinguishes HTML from JSON, diffs and code.
fn is_html(extractor: &dyn HtmlExtractor, body: &str, content_type: Option<&str>) -> bool {
async fn is_html(
extractor: &HtmlProvider,
body: &str,
content_type: Option<&str>,
) -> anyhow::Result<bool> {
if let Some(ct) = content_type {
let ct = ct.to_ascii_lowercase();
let mime = ct.split(';').next().unwrap_or("").trim().to_string();
// An explicit non-HTML type is a statement, not a guess: a JSON API
// that happens to embed markup must come back verbatim.
if !mime.is_empty() && mime != "text/html" && mime != "application/xhtml+xml" {
return false;
return Ok(false);
}
if !mime.is_empty() {
return true;
return Ok(true);
}
}
extractor.looks_like_html(body)
let (body, _) = cap_extractor_input(body);
extractor.looks_like_html(body).await
}

#[cfg(test)]
Expand Down
Loading
Loading