diff --git a/Cargo.lock b/Cargo.lock index 4449d23e..c3b76e27 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -979,6 +979,22 @@ version = "2.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "88904434abc2901f197fe8cc55f0445e7ded921dba5911dad2e2b39b48e663c4" +[[package]] +name = "mime" +version = "0.3.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6877bb514081ee2a7ff5ef9de3281f14a4dd4bceac4c09388074a6b5df8a139a" + +[[package]] +name = "mime_guess" +version = "2.0.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f7c44f8e672c00fe5308fa235f821cb4198414e1c77935c1ab6948d3fd78550e" +dependencies = [ + "mime", + "unicase", +] + [[package]] name = "miniz_oxide" version = "0.8.9" @@ -1312,6 +1328,7 @@ dependencies = [ "hyper-util", "js-sys", "log", + "mime_guess", "percent-encoding", "pin-project-lite", "quinn", @@ -1449,6 +1466,15 @@ version = "1.0.23" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9774ba4a74de5f7b1c1451ed6cd5285a32eddb5cccb8cc655a4e50009e06477f" +[[package]] +name = "same-file" +version = "1.0.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "93fc1dc3aaa9bfed95e02e6eadabb4baf7e3078b0bd1b4d7b6b0b68378900502" +dependencies = [ + "winapi-util", +] + [[package]] name = "scopeguard" version = "1.2.0" @@ -1762,6 +1788,7 @@ dependencies = [ "tinystoragedrivers-core", "tinytools", "tinytools-agent", + "tinytools-std", "tokio", "tracing", "uuid", @@ -2059,6 +2086,30 @@ dependencies = [ "tracing", ] +[[package]] +name = "tinytools-std" +version = "0.5.0" +dependencies = [ + "anyhow", + "async-trait", + "base64 0.23.1", + "fs2", + "futures-util", + "glob", + "libc", + "log", + "parking_lot", + "regex", + "reqwest", + "rustix", + "serde_json", + "sha2", + "tinytools", + "tokio", + "tracing", + "walkdir", +] + [[package]] name = "tinyvec" version = "1.11.0" @@ -2246,6 +2297,12 @@ version = "1.20.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b6f5e870be6c3b371b77fe0ee0bafb859fa4964b4404c27de1d380043c4dda20" +[[package]] +name = "unicase" +version = "2.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "357cc3acc6a036009fd6c973ed009037c732d60d0b4f6c673e9041497482a28f" + [[package]] name = "unicode-ident" version = "1.0.24" @@ -2308,6 +2365,16 @@ dependencies = [ "libc", ] +[[package]] +name = "walkdir" +version = "2.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "29790946404f91d9c5d06f9874efddea1dc06c5efe94541a7d6863108e3a5e4b" +dependencies = [ + "same-file", + "winapi-util", +] + [[package]] name = "want" version = "0.3.1" @@ -2454,6 +2521,15 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" +[[package]] +name = "winapi-util" +version = "0.1.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" +dependencies = [ + "windows-sys 0.61.2", +] + [[package]] name = "winapi-x86_64-pc-windows-gnu" version = "0.4.0" diff --git a/crates/tinyagents-harness/Cargo.toml b/crates/tinyagents-harness/Cargo.toml index 4f90f51e..8b47096e 100644 --- a/crates/tinyagents-harness/Cargo.toml +++ b/crates/tinyagents-harness/Cargo.toml @@ -39,6 +39,8 @@ tinyinference-embeddings = { path = "../../vendor/tinyinference/crates/tinyinfer tinyinference-image = { path = "../../vendor/tinyinference/crates/tinyinference-image", version = "0.3.0", optional = true } tinyinference-video = { path = "../../vendor/tinyinference/crates/tinyinference-video", version = "0.3.0", optional = true } tinytools = { path = "../../vendor/tinytools/crates/tinytools", version = "0.5.0" } +# Shared URL admission for opt-in remote multimodal attachments. +tinytools-std = { path = "../../vendor/tinytools/crates/tinytools-std", version = "0.5.0", optional = true } tokio = { workspace = true, features = ["sync", "time", "macros", "rt", "rt-multi-thread", "fs", "io-util", "process"] } # Storage ports (no database client): the `storage-drivers` adapters put # the harness `Store` / `AppendStore` on any tinystoragedrivers backend. @@ -58,7 +60,7 @@ storage-drivers = ["dep:tinystoragedrivers-core"] # still spell out the old name keep compiling. tools = ["builtin-tools"] builtin-tools = ["dep:chrono-tz"] -multimodal = ["dep:flate2", "dep:reqwest", "dep:tar", "dep:zip"] +multimodal = ["dep:flate2", "dep:reqwest", "dep:tar", "dep:tinytools-std", "dep:zip"] # Lossless PNG re-compression (`multimodal::optimize_png_lossless`) through # `oxipng`. Off by default: without it the helper returns `None` and callers # keep the original bytes, and `oxipng`/`libdeflater` (a C build) are not linked. diff --git a/crates/tinyagents-harness/src/multimodal/README.md b/crates/tinyagents-harness/src/multimodal/README.md index ef405463..5e82a4cc 100644 --- a/crates/tinyagents-harness/src/multimodal/README.md +++ b/crates/tinyagents-harness/src/multimodal/README.md @@ -77,13 +77,18 @@ format contributes a header naming it plus a content hash. - **Check the sentinel before the clamp.** `max_files == 0` means *none*; `FileLimits::effective` clamps it up to `1`. Consulting only the clamped value would admit one attachment from a source that asked for zero. -- What stays with the host, deliberately: the `reqwest::Client` (proxy/timeout - policy), the `TextExtractor` implementation (which parser, if any, and its +- What stays with the host, deliberately: the `TextExtractor` implementation (which parser, if any, and its timeout), the attachment stash (where bytes live between ingress and dispatch), and message-level marker counting (only the host knows its message type). This module never decides which local paths may be read — that is `FileLimits::files_disabled`'s lever, not a filesystem allowlist here. +- Opt-in remote image and file URLs pass TinyTools' URL and DNS guard. The + resolver pins the vetted addresses in its own direct HTTP client and refuses + redirects. It uses 30-second request and 10-second connection timeouts. + The legacy `reqwest::Client` argument remains for source compatibility but is + ignored for remote fetches: its proxy, custom DNS, redirect and timeout + settings cannot safely be inherited from a built client. - Text extraction failures degrade to a `FilePayload::Reference`. Resolution errors (read/fetch/MIME/size) remain typed errors for the host to present or skip according to its own policy. diff --git a/crates/tinyagents-harness/src/multimodal/mod.rs b/crates/tinyagents-harness/src/multimodal/mod.rs index ac1afdc3..0665bca6 100644 --- a/crates/tinyagents-harness/src/multimodal/mod.rs +++ b/crates/tinyagents-harness/src/multimodal/mod.rs @@ -42,7 +42,7 @@ //! //! | Host owns | Why | //! | --- | --- | -//! | The `reqwest::Client` | proxy configuration and timeouts are host policy; this module borrows one | +//! | The `reqwest::Client` | retained in resolver signatures for compatibility; remote fetches use a DNS-pinned direct client with redirects disabled | //! | [`TextExtractor`] | which document parser (if any) a host carries, and how long it may run | //! | The stash policy | which directory holds bytes between ingress and dispatch, how large it may grow, how long files live ([`stash::AttachmentStash`] is the mechanism) | //! | Message-level counting | only the host knows what its message type is | diff --git a/crates/tinyagents-harness/src/multimodal/resolve.rs b/crates/tinyagents-harness/src/multimodal/resolve.rs index 8a1c441a..8cbfe487 100644 --- a/crates/tinyagents-harness/src/multimodal/resolve.rs +++ b/crates/tinyagents-harness/src/multimodal/resolve.rs @@ -31,6 +31,7 @@ //! text and not the turn. use std::path::{Path, PathBuf}; +use std::time::Duration; use async_trait::async_trait; use reqwest::Client; @@ -89,6 +90,33 @@ impl TextExtractor for NoTextExtractor { } } +/// Fetch only from addresses vetted by TinyTools. The caller's `Client` cannot +/// be used here: reqwest does not let a request override its DNS or redirect +/// policy, so that client could reach a different address after validation. +async fn guarded_remote_get(source: &str) -> std::result::Result { + let validated = tinytools_std::url_guard::validate_url_with_dns_check(source, &[]) + .await + .map_err(|error| error.to_string())?; + let client = guarded_remote_client(&validated).map_err(|error| error.to_string())?; + client + .get(validated.url()) + .send() + .await + .map_err(|error| error.to_string()) +} + +fn guarded_remote_client( + validated: &tinytools_std::url_guard::ValidatedUrl, +) -> reqwest::Result { + Client::builder() + .no_proxy() + .redirect(reqwest::redirect::Policy::none()) + .timeout(Duration::from_secs(30)) + .connect_timeout(Duration::from_secs(10)) + .resolve_to_addrs(&validated.host, validated.addresses()) + .build() +} + // ── Images ─────────────────────────────────────────────────────────────── /// Resolve one `[IMAGE:…]` reference into a canonical `data:` URI. @@ -156,14 +184,15 @@ fn resolve_image_data_uri(source: &str, max_bytes: usize) -> Result { async fn resolve_remote_image( source: &str, max_bytes: usize, - remote_client: &Client, + _remote_client: &Client, ) -> Result { - let response = remote_client.get(source).send().await.map_err(|error| { - MultimodalError::RemoteFetchFailed { - input: source.to_string(), - reason: error.to_string(), - } - })?; + let response = + guarded_remote_get(source) + .await + .map_err(|error| MultimodalError::RemoteFetchFailed { + input: source.to_string(), + reason: error, + })?; let status = response.status(); if !status.is_success() { @@ -565,12 +594,12 @@ async fn read_local_file(source: &str, max_bytes: usize) -> Result<(Vec, Pat async fn fetch_remote_file( source: &str, max_bytes: usize, - remote_client: &Client, + _remote_client: &Client, ) -> Result<(Vec, String, Option)> { - let response = remote_client.get(source).send().await.map_err(|error| { + let response = guarded_remote_get(source).await.map_err(|error| { MultimodalError::RemoteFileFetchFailed { input: source.to_string(), - reason: error.to_string(), + reason: error, } })?; diff --git a/crates/tinyagents-harness/src/multimodal/resolve_tests.rs b/crates/tinyagents-harness/src/multimodal/resolve_tests.rs index f638503a..8cb45873 100644 --- a/crates/tinyagents-harness/src/multimodal/resolve_tests.rs +++ b/crates/tinyagents-harness/src/multimodal/resolve_tests.rs @@ -2,6 +2,136 @@ use super::*; use base64::{Engine as _, engine::general_purpose::STANDARD}; use std::io::Write; +#[tokio::test] +async fn remote_image_rejects_private_destination_before_fetch() { + let limits = ImageLimits { + allow_remote_fetch: true, + ..ImageLimits::default() + }; + let client = Client::builder() + .timeout(std::time::Duration::from_millis(100)) + .build() + .unwrap(); + let error = resolve_image("http://127.0.0.1:9/image.png", &limits, 1024, &client) + .await + .unwrap_err(); + assert!( + matches!(error, MultimodalError::RemoteFetchFailed { reason, .. } if reason.contains("Blocked local/private host")) + ); +} + +#[tokio::test] +async fn remote_file_rejects_mapped_private_destination_before_fetch() { + let limits = FileLimits { + allow_remote_fetch: true, + ..FileLimits::default() + }; + let client = Client::builder() + .timeout(std::time::Duration::from_millis(100)) + .build() + .unwrap(); + let error = resolve_attachment( + "http://[::ffff:127.0.0.1]:9/file.bin", + &limits, + 1024, + &client, + UnknownMimePolicy::Accept, + ) + .await + .unwrap_err(); + assert!( + matches!(error, MultimodalError::RemoteFileFetchFailed { reason, .. } if reason.contains("IPv6")) + ); +} + +#[tokio::test] +async fn remote_file_ignores_caller_dns_override_to_loopback() { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let client = Client::builder() + .no_proxy() + .resolve("attachment.invalid", listener.local_addr().unwrap()) + .build() + .unwrap(); + let limits = FileLimits { + allow_remote_fetch: true, + ..FileLimits::default() + }; + let error = resolve_attachment( + "http://attachment.invalid/file.txt", + &limits, + 1024, + &client, + UnknownMimePolicy::Accept, + ) + .await + .unwrap_err(); + assert!(matches!( + error, + MultimodalError::RemoteFileFetchFailed { .. } + )); + assert!( + tokio::time::timeout(std::time::Duration::from_millis(50), listener.accept()) + .await + .is_err() + ); +} + +#[tokio::test] +async fn remote_image_ignores_caller_dns_override_to_loopback() { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let client = Client::builder() + .no_proxy() + .resolve("image.invalid", listener.local_addr().unwrap()) + .build() + .unwrap(); + let limits = ImageLimits { + allow_remote_fetch: true, + ..ImageLimits::default() + }; + let error = resolve_image("http://image.invalid/a.png", &limits, 1024, &client) + .await + .unwrap_err(); + assert!(matches!(error, MultimodalError::RemoteFetchFailed { .. })); + assert!( + tokio::time::timeout(std::time::Duration::from_millis(50), listener.accept()) + .await + .is_err() + ); +} + +#[tokio::test] +async fn guarded_client_does_not_follow_redirects() { + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let address = listener.local_addr().unwrap(); + // The synthetic validated address tests the transport policy in isolation. + // Production obtains this struct only from the DNS guard, which rejects it. + let validated = tinytools_std::url_guard::ValidatedUrl { + url: format!("http://example.com:{}/image.png", address.port()), + host: "example.com".to_string(), + addrs: vec![address], + }; + let server = tokio::spawn(async move { + let (mut stream, _) = listener.accept().await.unwrap(); + let mut request = [0; 512]; + assert!(stream.read(&mut request).await.unwrap() > 0); + stream + .write_all(b"HTTP/1.1 302 Found\r\nLocation: http://127.0.0.1:9/secret\r\nContent-Length: 0\r\nConnection: close\r\n\r\n") + .await + .unwrap(); + }); + let response = guarded_remote_client(&validated) + .unwrap() + .get(validated.url()) + .send() + .await + .unwrap(); + assert_eq!(response.status(), reqwest::StatusCode::FOUND); + assert_eq!(response.url().as_str(), validated.url()); + server.await.unwrap(); +} + #[tokio::test] async fn generic_resolution_retains_binary_bytes_and_decodes_only_transport_gzip() { let bytes = [0, 255, 7, 0]; @@ -224,57 +354,18 @@ async fn malformed_and_oversized_data_uri_payloads_fail_before_extraction() { )); } -#[tokio::test] -async fn generic_http_mime_precedes_utf8_sniff_and_legacy_stays_narrow() { - use tokio::io::{AsyncReadExt, AsyncWriteExt}; - let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); - let url = format!("http://{}/media", listener.local_addr().unwrap()); - let server = tokio::spawn(async move { - for _ in 0..2 { - let (mut stream, _) = listener.accept().await.unwrap(); - let mut request = [0; 2048]; - let mut received = 0; - loop { - let amount = stream.read(&mut request[received..]).await.unwrap(); - assert!(amount > 0); - received += amount; - if request[..received] - .windows(4) - .any(|part| part == b"\r\n\r\n") - { - break; - } - assert!(received < request.len()); - } - stream.write_all(b"HTTP/1.1 200 OK\r\nContent-Type: audio/wav; charset=binary\r\nContent-Length: 12\r\nConnection: close\r\n\r\nRIFF\0\0\0\0WAVE").await.unwrap(); - } - }); - let limits = FileLimits { - allow_remote_fetch: true, - ..FileLimits::default() - }; - let resolved = resolve_attachment( - &url, - &limits, - 1024, - &Client::new(), - UnknownMimePolicy::Accept, - ) - .await - .unwrap(); - assert_eq!(resolved.mime, "audio/wav"); - assert_eq!(resolved.bytes, b"RIFF\0\0\0\0WAVE"); - let legacy = resolve_attachment( - &url, - &limits, - 1024, - &Client::new(), - UnknownMimePolicy::Reject, - ) - .await - .unwrap(); - assert_eq!(legacy.mime, "text/plain"); - server.await.unwrap(); +#[test] +fn generic_http_mime_precedes_utf8_sniff_and_legacy_stays_narrow() { + let bytes = b"RIFF\0\0\0\0WAVE"; + let path = std::path::Path::new("media"); + assert_eq!( + super::super::mime::detect_attachment_mime(path, bytes, Some("audio/wav; charset=binary")), + Some("audio/wav".to_string()) + ); + assert_eq!( + detect_file_mime(Some(path), bytes, Some("audio/wav; charset=binary")), + Some("text/plain".to_string()) + ); } #[tokio::test] diff --git a/vendor/tinytools b/vendor/tinytools index c3d99e07..8a87a262 160000 --- a/vendor/tinytools +++ b/vendor/tinytools @@ -1 +1 @@ -Subproject commit c3d99e079702fb3723b58881f48a97892073143b +Subproject commit 8a87a26293341c51afa11bccfdbe920def7ca9d6