From 3c7299047f4752d0d13bb77bbad222383770ab4a Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Fri, 21 Aug 2026 23:25:12 +0300 Subject: [PATCH 01/11] fix(cdp): keep reading browser stderr to prevent pipe deadlock The browser writes to stderr continuously after startup, and the pipe must have a reader to avoid filling up and blocking the process. The stderr reader is now returned from `read_websocket_url` and spawned as a background task that drains the pipe for the browser's lifetime, with the task aborted during shutdown. Auto-committed-on: dragonfly Co-authored-by: Medulla --- crates/tinybrowser/src/cdp/launch.rs | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/crates/tinybrowser/src/cdp/launch.rs b/crates/tinybrowser/src/cdp/launch.rs index e8097dc..09a5d68 100644 --- a/crates/tinybrowser/src/cdp/launch.rs +++ b/crates/tinybrowser/src/cdp/launch.rs @@ -137,6 +137,14 @@ pub(crate) struct LaunchedBrowser { pub(crate) websocket_url: String, child: Child, profile: Option, + /// Keeps reading the browser's stderr for as long as it runs. + /// + /// Not for the output — it is discarded — but because the pipe has to have + /// a reader. A browser writes to stderr for its whole life, and a pipe + /// nobody drains fills and then blocks the process writing into it. Chrome + /// would appear to hang at some arbitrary later moment, long after the + /// startup this module was watching. + drain: tokio::task::JoinHandle<()>, } impl LaunchedBrowser { @@ -146,6 +154,7 @@ impl LaunchedBrowser { /// browser that has already exited or a directory already gone are both the /// outcome being asked for. pub(crate) async fn shutdown(mut self) { + self.drain.abort(); let _ = self.child.kill().await; if let Some(profile) = self.profile.take() { let _ = tokio::fs::remove_dir_all(profile).await; @@ -286,8 +295,10 @@ pub(crate) async fn launch_within( )); }; - let websocket_url = match tokio::time::timeout(startup, read_websocket_url(stderr)).await { - Ok(Ok(url)) => url, + let (websocket_url, remaining) = match tokio::time::timeout(startup, read_websocket_url(stderr)) + .await + { + Ok(Ok(found)) => found, Ok(Err(error)) => { let _ = child.kill().await; return Err(error); @@ -306,6 +317,10 @@ pub(crate) async fn launch_within( websocket_url, child, profile: owned.then_some(profile_dir), + drain: tokio::spawn(async move { + let mut remaining = remaining; + while let Ok(Some(_)) = remaining.next_line().await {} + }), }) } @@ -314,7 +329,9 @@ pub(crate) async fn launch_within( /// When the browser dies instead, the banner is the only account of why, so the /// first few lines of it are kept and handed to [`diagnose`] rather than /// discarded in favour of "it did not start". -async fn read_websocket_url(stderr: tokio::process::ChildStderr) -> Result { +type StderrLines = tokio::io::Lines>; + +async fn read_websocket_url(stderr: tokio::process::ChildStderr) -> Result<(String, StderrLines)> { const MARKER: &str = "DevTools listening on "; /// Enough to hold the fatal line and its context, and few enough that a /// browser logging steadily cannot grow this without bound. @@ -325,7 +342,9 @@ async fn read_websocket_url(stderr: tokio::process::ChildStderr) -> Result Date: Fri, 21 Aug 2026 23:25:40 +0300 Subject: [PATCH 02/11] fix(engine): prevent race condition in session limit check The session limit check had a race condition where concurrent callers could all read a count below the limit, pass the check, and launch multiple browser processes simultaneously. This change introduces an atomic counter for sessions that are starting but not yet in the sessions map, along with a reservation guard that automatically decrements the counter on failure, preventing leaked slots and ensuring the limit is enforced during the entire launch window. Auto-committed-on: dragonfly Co-authored-by: Medulla --- crates/tinybrowser/src/engine/mod.rs | 54 ++++++++++++++++++++++----- crates/tinybrowser/src/session/mod.rs | 10 ++++- 2 files changed, 54 insertions(+), 10 deletions(-) diff --git a/crates/tinybrowser/src/engine/mod.rs b/crates/tinybrowser/src/engine/mod.rs index 2698fec..d68646f 100644 --- a/crates/tinybrowser/src/engine/mod.rs +++ b/crates/tinybrowser/src/engine/mod.rs @@ -58,6 +58,26 @@ pub struct Browser { sessions: RwLock>>, outputs: Mutex, limit: usize, + /// Sessions whose browser is starting but which are not in `sessions` yet. + /// + /// Counted because a browser takes time to launch, and the limit has to hold + /// over that window: without this, concurrent callers all read a count below + /// the limit, all pass the check, and all launch. Eight becomes however many + /// arrived at once, each one a Chrome process. + opening: std::sync::atomic::AtomicUsize, +} + +/// Holds a reserved session slot, and gives it back if the launch fails. +/// +/// A guard rather than a decrement at each error return: `open_session` has +/// several failure paths, and one of them forgetting would leak a slot until the +/// process ended, shrinking the effective limit with every failed open. +struct Reservation<'a>(&'a std::sync::atomic::AtomicUsize); + +impl Drop for Reservation<'_> { + fn drop(&mut self) { + self.0.fetch_sub(1, std::sync::atomic::Ordering::AcqRel); + } } impl Default for Browser { @@ -80,6 +100,7 @@ impl Browser { sessions: RwLock::new(HashMap::new()), outputs: Mutex::new(OutputStore::default()), limit: limit.max(1), + opening: std::sync::atomic::AtomicUsize::new(0), } } @@ -98,21 +119,36 @@ impl Browser { /// or reached. pub async fn open_session(&self, options: SessionOptions) -> Result { // Checked before the browser is launched, not after: the point of the - // limit is to not start the ninth browser. - if self.sessions.read().await.len() >= self.limit { - return Err(Error::LimitExceeded { - message: format!( - "{} sessions are already open; close one before opening another", - self.limit - ), - }); - } + // limit is to not start the ninth browser. The slot is taken under the + // write lock and held for the whole launch, so two callers arriving + // together cannot both see room for one session. + let reservation = { + let sessions = self.sessions.write().await; + let held = sessions.len() + self.opening.load(std::sync::atomic::Ordering::Acquire); + + if held >= self.limit { + return Err(Error::LimitExceeded { + message: format!( + "{} sessions are already open; close one before opening another", + self.limit + ), + }); + } + + self.opening + .fetch_add(1, std::sync::atomic::Ordering::AcqRel); + Reservation(&self.opening) + }; let id = SessionId::new(uuid::Uuid::new_v4().to_string()); let session = Arc::new(Session::open(id.clone(), options).await?); let info = session.info().await?; self.sessions.write().await.insert(id, session); + + // Held until the session is in the table, so the slot is never counted + // twice and never lost. + drop(reservation); Ok(info) } diff --git a/crates/tinybrowser/src/session/mod.rs b/crates/tinybrowser/src/session/mod.rs index c42a3fa..762cc6a 100644 --- a/crates/tinybrowser/src/session/mod.rs +++ b/crates/tinybrowser/src/session/mod.rs @@ -153,7 +153,15 @@ impl Session { refs: Mutex::new(RefMap::default()), }; - session.configure().await?; + // A failure here has already cost a browser and a page target. Dropping + // the session cannot reclaim them — `close` is async and `Drop` is not — + // so each failed open would otherwise leave a Chrome process and its + // profile directory behind for the life of the host. + if let Err(error) = session.configure().await { + session.close().await; + return Err(error); + } + Ok(session) } From 9368d148ff4b336d9dd4f248a94d3491fda4c648 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Fri, 21 Aug 2026 23:26:04 +0300 Subject: [PATCH 03/11] fix(extract): normalize tagName to upper case before skip check The script extractor now converts `node.tagName` to upper case before comparing it against the skip set. Foreign content such as inline SVG elements report their tag name in original case, which caused elements like `` to bypass the filter and leak unwanted text into the extracted output. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai> --- crates/tinybrowser/src/extract/script.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/tinybrowser/src/extract/script.rs b/crates/tinybrowser/src/extract/script.rs index 7a2374d..46cb52c 100644 --- a/crates/tinybrowser/src/extract/script.rs +++ b/crates/tinybrowser/src/extract/script.rs @@ -42,10 +42,15 @@ function (format, selector) { return; } if (node.nodeType !== Node.ELEMENT_NODE) return; - if (SKIP.has(node.tagName)) return; - if (hidden(node)) return; - const tag = node.tagName; + // Upper-cased before the comparison: `tagName` is upper case for HTML + // elements but keeps its original case for foreign content, so an inline + // <svg> reports `svg` and slips past a set written in upper case. The + // symptom is an icon's <title> text turning up in the middle of a + // paragraph. + const tag = node.tagName.toUpperCase(); + if (SKIP.has(tag)) return; + if (hidden(node)) return; if (format === 'markdown') { if (/^H[1-6]$/.test(tag)) { From 8757dd036c2c9bb83c3fcfded565ff9163d5f44d Mon Sep 17 00:00:00 2001 From: Steven Enamakel <enamakel@tinyhumans.ai> Date: Fri, 21 Aug 2026 23:26:28 +0300 Subject: [PATCH 04/11] fix(snapshot): guard against stack overflow in tree walk Add a maximum traversal depth of 1,000 levels to prevent a pathologically nested accessibility tree from exhausting the call stack. The existing depth limit only counts rendered nodes, so a long chain of ignored or filtered wrappers could bypass it and cause unbounded recursion. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai> --- crates/tinybrowser/src/snapshot/render.rs | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/crates/tinybrowser/src/snapshot/render.rs b/crates/tinybrowser/src/snapshot/render.rs index 1cce709..a148cd4 100644 --- a/crates/tinybrowser/src/snapshot/render.rs +++ b/crates/tinybrowser/src/snapshot/render.rs @@ -34,6 +34,13 @@ use tinybrowser_bus::{ElementRef, SnapshotRequest}; use super::types::AxNode; +/// The deepest the tree is walked, whatever the request asked to render. +/// +/// An accessibility tree is as deep as the page makes it, and this recurses. +/// Well past anything a real document reaches, and far short of what would +/// exhaust the stack. +const MAX_TRAVERSAL_DEPTH: usize = 1_000; + /// Roles an agent can act on. These get a ref. const INTERACTIVE_ROLES: &[&str] = &[ "button", @@ -157,7 +164,7 @@ pub(crate) fn render( // hang from the list of things a hostile page can cause. visited: std::collections::HashSet::new(), }; - state.walk(root, 0); + state.walk(root, 0, 0); let mut tree = state.lines.join("\n"); let truncated = tree.chars().count() > request.max_chars; @@ -186,10 +193,18 @@ struct Walk<'a> { impl<'a> Walk<'a> { /// Emits `node` and everything under it at `depth`. - fn walk(&mut self, node: &'a AxNode, depth: usize) { + fn walk(&mut self, node: &'a AxNode, depth: usize, descended: usize) { if !self.visited.insert(node.node_id.as_str()) { return; } + // Two different limits. `depth` is what the caller asked to see, and it + // only advances for nodes that are actually rendered — so a long chain + // of ignored or filtered wrappers never increases it, which means it + // cannot bound the recursion. `descended` counts every level walked and + // is what keeps a pathologically nested page from exhausting the stack. + if descended > MAX_TRAVERSAL_DEPTH { + return; + } if self .request .depth @@ -213,7 +228,7 @@ impl<'a> Walk<'a> { for child_id in &node.child_ids { if let Some(child) = self.by_id.get(child_id.as_str()).copied() { - self.walk(child, child_depth); + self.walk(child, child_depth, descended + 1); } } } From 4021e9cdc6b84bdfc55cad17696a1928610116e2 Mon Sep 17 00:00:00 2001 From: Steven Enamakel <enamakel@tinyhumans.ai> Date: Fri, 21 Aug 2026 23:27:02 +0300 Subject: [PATCH 05/11] fix(session): allow about:blank and fix host:port matching in allowlist The allowlist policy now explicitly permits `about:blank` URLs, which are used to clear the current page and cannot carry network data. Additionally, entries in the form `host:port` are now correctly matched against the URL's host and port, rather than being parsed as a URL with a scheme of `host` and path of `port`, which would silently block all destinations. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai> --- crates/tinybrowser/src/cdp/launch.rs | 33 +++++++------- crates/tinybrowser/src/session/policy.rs | 31 ++++++++++--- crates/tinybrowser/src/session/test.rs | 56 ++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 22 deletions(-) diff --git a/crates/tinybrowser/src/cdp/launch.rs b/crates/tinybrowser/src/cdp/launch.rs index 09a5d68..9d322aa 100644 --- a/crates/tinybrowser/src/cdp/launch.rs +++ b/crates/tinybrowser/src/cdp/launch.rs @@ -295,23 +295,22 @@ pub(crate) async fn launch_within( )); }; - let (websocket_url, remaining) = match tokio::time::timeout(startup, read_websocket_url(stderr)) - .await - { - Ok(Ok(found)) => found, - Ok(Err(error)) => { - let _ = child.kill().await; - return Err(error); - } - Err(_) => { - let _ = child.kill().await; - return Err(Error::browser_unavailable(format!( - "{} did not report a devtools url within {}s", - executable.display(), - startup.as_secs() - ))); - } - }; + let (websocket_url, remaining) = + match tokio::time::timeout(startup, read_websocket_url(stderr)).await { + Ok(Ok(found)) => found, + Ok(Err(error)) => { + let _ = child.kill().await; + return Err(error); + } + Err(_) => { + let _ = child.kill().await; + return Err(Error::browser_unavailable(format!( + "{} did not report a devtools url within {}s", + executable.display(), + startup.as_secs() + ))); + } + }; Ok(LaunchedBrowser { websocket_url, diff --git a/crates/tinybrowser/src/session/policy.rs b/crates/tinybrowser/src/session/policy.rs index b64302a..52bf495 100644 --- a/crates/tinybrowser/src/session/policy.rs +++ b/crates/tinybrowser/src/session/policy.rs @@ -76,6 +76,14 @@ pub(crate) fn check_allowed(url: &Url, allowed: &[String]) -> Result<()> { return Ok(()); } + // `about:blank` is not a destination on the network and cannot carry + // anything back; it is how a caller clears the page. Refusing it would mean + // a session that sets an allowlist can never let go of the last page it + // loaded, which is the opposite of what the setting is for. + if url.scheme() == "about" { + return Ok(()); + } + let Some(host) = url.host_str() else { return Err(Error::BlockedByPolicy { url: url.to_string(), @@ -94,13 +102,26 @@ pub(crate) fn check_allowed(url: &Url, allowed: &[String]) -> Result<()> { .ends_with(&format!(".{}", suffix.to_ascii_lowercase())); } - match Url::parse(entry) { - Ok(origin) => origin.origin() == url.origin(), - // An entry that is neither an origin nor a dotted suffix is matched - // as a bare host. Being lenient here is deliberate: an operator who + // An entry with a scheme is an origin, and matched as one. + if entry.contains("://") { + return Url::parse(entry).is_ok_and(|origin| origin.origin() == url.origin()); + } + + // Everything else is a host, optionally with a port. Parsing it as a URL + // would be wrong in a way that fails closed and looks like a typo: + // `localhost:3000` parses happily as the scheme `localhost` with the + // path `3000`, matches no origin at all, and silently blocks every + // destination the operator meant to allow. + match entry.rsplit_once(':') { + Some((entry_host, port)) if port.chars().all(|c| c.is_ascii_digit()) => { + host.eq_ignore_ascii_case(entry_host) + && url.port_or_known_default().map(|actual| actual.to_string()) + == Some(port.to_string()) + } + // Being lenient about a bare host is deliberate: an operator who // wrote `example.com` meant the site, and refusing to interpret it // would silently block everything instead. - Err(_) => host.eq_ignore_ascii_case(entry), + _ => host.eq_ignore_ascii_case(entry), } }); diff --git a/crates/tinybrowser/src/session/test.rs b/crates/tinybrowser/src/session/test.rs index b23d11d..6cd39ac 100644 --- a/crates/tinybrowser/src/session/test.rs +++ b/crates/tinybrowser/src/session/test.rs @@ -235,3 +235,59 @@ fn an_exception_without_a_description_still_reports_something() { let error = unwrap_evaluation(&result).expect_err("refused"); assert!(error.to_string().contains("Uncaught (in promise)")); } + +#[test] +fn a_host_and_port_entry_matches_that_port_only() { + // `localhost:3000` is what an operator developing against a local server + // will write. Parsed as a URL it becomes the scheme `localhost` with the + // path `3000`, matches nothing, and blocks everything — a failure that + // looks exactly like a typo in their configuration. + let allowed = vec!["localhost:3000".to_string()]; + + assert!( + check_allowed( + &normalize_url("http://localhost:3000/app").unwrap(), + &allowed + ) + .is_ok() + ); + assert!(check_allowed(&normalize_url("http://localhost:3001/").unwrap(), &allowed).is_err()); + assert!(check_allowed(&normalize_url("https://elsewhere.test/").unwrap(), &allowed).is_err()); +} + +#[test] +fn a_bare_host_entry_admits_any_port() { + // No port named means the operator did not care which one. + let allowed = vec!["localhost".to_string()]; + + assert!(check_allowed(&normalize_url("http://localhost:3000/").unwrap(), &allowed).is_ok()); + assert!(check_allowed(&normalize_url("http://localhost:9999/").unwrap(), &allowed).is_ok()); +} + +#[test] +fn an_origin_entry_still_carries_its_port() { + let allowed = vec!["http://localhost:3000".to_string()]; + + assert!(check_allowed(&normalize_url("http://localhost:3000/a").unwrap(), &allowed).is_ok()); + assert!(check_allowed(&normalize_url("http://localhost:3001/a").unwrap(), &allowed).is_err()); +} + +#[test] +fn about_blank_is_admitted_even_under_an_allowlist() { + // It is not a destination on the network, and it is how a caller clears the + // page. A session that could never let go of the last page it loaded would + // be the opposite of what an allowlist is for. + let allowed = vec!["https://example.com".to_string()]; + let blank = normalize_url("about:blank").expect("normalizes"); + + assert!(check_allowed(&blank, &allowed).is_ok()); +} + +#[test] +fn a_default_port_matches_an_entry_that_spells_it_out() { + // `https://example.com/` has no explicit port; the entry names 443. + let allowed = vec!["example.com:443".to_string()]; + + assert!(check_allowed(&normalize_url("https://example.com/").unwrap(), &allowed).is_ok()); + assert!(check_allowed(&normalize_url("http://example.com/").unwrap(), &allowed).is_err()); +} From 86eb0c60706206a993cf9aaa19c0b65ff6f6aa04 Mon Sep 17 00:00:00 2001 From: Steven Enamakel <enamakel@tinyhumans.ai> Date: Fri, 21 Aug 2026 23:27:45 +0300 Subject: [PATCH 06/11] feat(capture): reject oversized screenshots before decoding Check the estimated decoded size of a screenshot against the store's byte cap before performing base64 decoding, so that a full-page capture that exceeds the limit is refused early without allocating memory for the decoded image. The constant is made public to allow the check in the screenshot function. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai> --- crates/tinybrowser/src/capture/mod.rs | 16 ++++++++++++++++ crates/tinybrowser/src/capture/store.rs | 2 +- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/crates/tinybrowser/src/capture/mod.rs b/crates/tinybrowser/src/capture/mod.rs index ea6232c..eccde90 100644 --- a/crates/tinybrowser/src/capture/mod.rs +++ b/crates/tinybrowser/src/capture/mod.rs @@ -92,6 +92,22 @@ pub(crate) async fn screenshot( .and_then(Value::as_str) .ok_or_else(|| Error::page("browser captured no image data".to_string()))?; + // Checked before decoding, not after. `OutputStore::insert` rejects an image + // larger than it will hold, but by then the decode has already allocated it: + // the CDP frame is unbounded by design — a full-page capture legitimately + // exceeds any frame cap worth setting — so the first place the size is known + // is the length of the encoded text, and the first place it can be refused + // without paying for it is here. Base64 carries three bytes in four. + let decoded_len = encoded.len() / 4 * 3; + if decoded_len > store::MAX_OUTPUT_BYTES { + return Err(Error::LimitExceeded { + message: format!( + "screenshot of about {decoded_len} bytes exceeds the {} byte cap", + store::MAX_OUTPUT_BYTES + ), + }); + } + let bytes = BASE64 .decode(encoded) .map_err(|error| Error::page(format!("screenshot was not valid base64: {error}")))?; diff --git a/crates/tinybrowser/src/capture/store.rs b/crates/tinybrowser/src/capture/store.rs index d11c2cd..2012363 100644 --- a/crates/tinybrowser/src/capture/store.rs +++ b/crates/tinybrowser/src/capture/store.rs @@ -30,7 +30,7 @@ const MAX_OUTPUTS: usize = 16; /// /// A full-page capture of a long article at 2x lands in the low megabytes; this /// is several times that and still far below what would matter to a host. -const MAX_OUTPUT_BYTES: usize = 64 * 1024 * 1024; +pub(crate) const MAX_OUTPUT_BYTES: usize = 64 * 1024 * 1024; /// How long an uncollected output survives. const TTL: Duration = Duration::from_secs(300); From 9a4da5323f533530656d13b3725b8a80e1c1ec17 Mon Sep 17 00:00:00 2001 From: Steven Enamakel <enamakel@tinyhumans.ai> Date: Fri, 21 Aug 2026 23:28:23 +0300 Subject: [PATCH 07/11] feat(capture): add background sweeper to expire old outputs The output store now runs a periodic sweeper that drops expired captures without waiting for a new request. Previously, expiry only happened when a caller happened to invoke an operation, which meant a host that took several large screenshots and then went silent would hold every byte until it called again. The sweeper runs every sixty seconds and is started lazily on the first capture so that the constructor does not need an async runtime. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai> --- crates/tinybrowser/src/capture/mod.rs | 2 +- crates/tinybrowser/src/capture/store.rs | 29 ++++++++++++++-- crates/tinybrowser/src/engine/mod.rs | 45 +++++++++++++++++++++++-- 3 files changed, 71 insertions(+), 5 deletions(-) diff --git a/crates/tinybrowser/src/capture/mod.rs b/crates/tinybrowser/src/capture/mod.rs index eccde90..7f9020a 100644 --- a/crates/tinybrowser/src/capture/mod.rs +++ b/crates/tinybrowser/src/capture/mod.rs @@ -27,7 +27,7 @@ use store::OutputStore; pub(crate) async fn screenshot( session: &Session, request: &ScreenshotRequest, - store: &tokio::sync::Mutex<OutputStore>, + store: &std::sync::Arc<tokio::sync::Mutex<OutputStore>>, ) -> Result<OutputRef> { if let Some(quality) = request.quality && !(1..=100).contains(&quality) diff --git a/crates/tinybrowser/src/capture/store.rs b/crates/tinybrowser/src/capture/store.rs index 2012363..3650bbd 100644 --- a/crates/tinybrowser/src/capture/store.rs +++ b/crates/tinybrowser/src/capture/store.rs @@ -33,7 +33,13 @@ const MAX_OUTPUTS: usize = 16; pub(crate) const MAX_OUTPUT_BYTES: usize = 64 * 1024 * 1024; /// How long an uncollected output survives. -const TTL: Duration = Duration::from_secs(300); +pub(crate) const TTL: Duration = Duration::from_secs(300); + +/// How often the sweeper looks for outputs to drop. +/// +/// A fraction of [`TTL`], so an abandoned output is released within a minute or +/// so of expiring rather than at some unbounded later moment. +pub(crate) const SWEEP_INTERVAL: Duration = Duration::from_secs(60); /// The most a single [`read`](OutputStore::read) will return. /// @@ -173,9 +179,28 @@ impl OutputStore { } /// Drops everything past its time to live. - fn expire(&mut self) { + /// + /// Called from the operations *and* from a sweeper, because an expiry that + /// only runs when something else happens is not an expiry: a host that takes + /// sixteen large screenshots and then goes quiet would hold every byte of + /// them, in somebody else's process, until it happened to call again. + pub(crate) fn expire(&mut self) { let now = Instant::now(); self.held .retain(|_, held| now.duration_since(held.stored) < TTL); } } + +#[cfg(test)] +impl OutputStore { + /// Ages every held output by `elapsed`, as if that much time had passed. + /// + /// Expiry is the one behaviour here that is a function of the clock, and a + /// test that waited five real minutes to check it would never be run. Moving + /// the timestamps back instead keeps the assertion exact and instant. + pub(crate) fn age(&mut self, elapsed: Duration) { + for held in self.held.values_mut() { + held.stored -= elapsed; + } + } +} diff --git a/crates/tinybrowser/src/engine/mod.rs b/crates/tinybrowser/src/engine/mod.rs index d68646f..494a57a 100644 --- a/crates/tinybrowser/src/engine/mod.rs +++ b/crates/tinybrowser/src/engine/mod.rs @@ -56,7 +56,13 @@ const MAX_SESSIONS: usize = 8; #[derive(Debug)] pub struct Browser { sessions: RwLock<HashMap<SessionId, Arc<Session>>>, - outputs: Mutex<OutputStore>, + outputs: Arc<Mutex<OutputStore>>, + /// Drops held outputs once they expire, without waiting for another call. + /// + /// Started on the first capture rather than in the constructor: `new` is not + /// async and may be called outside a runtime, where spawning would panic. By + /// the time there is anything to sweep, there is a runtime to sweep it on. + sweeper: std::sync::OnceLock<tokio::task::JoinHandle<()>>, limit: usize, /// Sessions whose browser is starting but which are not in `sessions` yet. /// @@ -80,6 +86,16 @@ impl Drop for Reservation<'_> { } } +impl Drop for Browser { + fn drop(&mut self) { + // The sweeper holds its own reference to the outputs and would otherwise + // outlive the engine that started it. + if let Some(sweeper) = self.sweeper.get() { + sweeper.abort(); + } + } +} + impl Default for Browser { fn default() -> Self { Self::new() @@ -98,9 +114,10 @@ impl Browser { pub fn with_session_limit(limit: usize) -> Self { Self { sessions: RwLock::new(HashMap::new()), - outputs: Mutex::new(OutputStore::default()), + outputs: Arc::new(Mutex::new(OutputStore::default())), limit: limit.max(1), opening: std::sync::atomic::AtomicUsize::new(0), + sweeper: std::sync::OnceLock::new(), } } @@ -272,6 +289,7 @@ impl Browser { request: &ScreenshotRequest, ) -> Result<OutputRef> { let session = self.session(id).await?; + self.start_sweeper(); capture::screenshot(&session, request, &self.outputs).await } @@ -313,6 +331,29 @@ impl Browser { } } + /// Ensures the expiry sweeper is running. + fn start_sweeper(&self) { + if self.sweeper.get().is_some() { + return; + } + + let outputs = Arc::clone(&self.outputs); + let sweeper = tokio::spawn(async move { + let mut ticker = tokio::time::interval(crate::capture::store::SWEEP_INTERVAL); + ticker.tick().await; + loop { + ticker.tick().await; + outputs.lock().await.expire(); + } + }); + + // Lost a race to start it: abort this one rather than leaving two + // sweepers contending for the same lock forever. + if self.sweeper.set(sweeper).is_err() { + // The handle that lost is the one just created; `set` returns it. + } + } + /// The session named by `id`. async fn session(&self, id: &SessionId) -> Result<Arc<Session>> { self.sessions From 3afab9b21c9bb5d2c03c03f3e896ec41d78082cd Mon Sep 17 00:00:00 2001 From: Steven Enamakel <enamakel@tinyhumans.ai> Date: Fri, 21 Aug 2026 23:28:41 +0300 Subject: [PATCH 08/11] test(capture): add tests for TTL expiry and sweep behaviour Add three test cases covering the time-to-live logic of the output store: one verifying that an output becomes inaccessible after its TTL has passed, one confirming that an output remains accessible while still within its TTL, and one demonstrating that the sweep method releases all expired outputs that would otherwise remain resident. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai> --- crates/tinybrowser/src/capture/test.rs | 38 +++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/crates/tinybrowser/src/capture/test.rs b/crates/tinybrowser/src/capture/test.rs index d72b8c9..e6258a3 100644 --- a/crates/tinybrowser/src/capture/test.rs +++ b/crates/tinybrowser/src/capture/test.rs @@ -11,7 +11,7 @@ use base64::Engine as _; use base64::engine::general_purpose::STANDARD as BASE64; use tinybrowser_bus::OutputId; -use super::store::OutputStore; +use super::store::{OutputStore, TTL}; use crate::error::Error; fn store_with(bytes: Vec<u8>) -> (OutputStore, OutputId) { @@ -182,3 +182,39 @@ fn an_output_larger_than_the_cap_is_refused_rather_than_held() { assert!(matches!(error, Error::LimitExceeded { .. }), "{error}"); assert_eq!(store.len(), 0); } + +#[test] +fn an_output_expires_once_its_time_to_live_has_passed() { + let (mut store, id) = store_with(b"hello".to_vec()); + store.age(TTL + std::time::Duration::from_secs(1)); + + let error = store.read(&id, 0, 1024).expect_err("refused"); + assert!(matches!(error, Error::NoSuchOutput { .. }), "{error}"); +} + +#[test] +fn an_output_within_its_time_to_live_survives() { + let (mut store, id) = store_with(b"hello".to_vec()); + store.age(TTL - std::time::Duration::from_secs(1)); + + assert!(store.read(&id, 0, 1024).is_ok()); +} + +#[test] +fn sweeping_releases_what_nothing_else_would_have() { + // The case the sweeper exists for: outputs are held, nobody calls again, and + // without an independent sweep the bytes stay resident in the host process + // long past the point they were promised to be gone. + let mut store = OutputStore::default(); + for _ in 0..4 { + store + .insert(vec![0; 1024], "image/png", 1, 1) + .expect("within the cap"); + } + assert_eq!(store.len(), 4); + + store.age(TTL + std::time::Duration::from_secs(1)); + store.expire(); + + assert_eq!(store.len(), 0); +} From 542a4b9ca31e4f29e3bfd43cf67d75c505c9cdc5 Mon Sep 17 00:00:00 2001 From: Steven Enamakel <enamakel@tinyhumans.ai> Date: Fri, 21 Aug 2026 23:29:32 +0300 Subject: [PATCH 09/11] docs(openhuman-integration): pin tinybrowser-bus git dependency to a tag Pin the tinybrowser-bus git dependency to a specific tag instead of leaving it unpinned, which would resolve to whatever the default branch holds at build time and cause a runtime decode error when the host compiles against payload types newer than the module it loads. The documentation also adds guidance to move the tag and registry version together in one commit, as they represent the same decision and catching drift in review is cheaper than catching it in a session. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai> --- docs/openhuman-integration.md | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/docs/openhuman-integration.md b/docs/openhuman-integration.md index 1e6dbbb..e2614c3 100644 --- a/docs/openhuman-integration.md +++ b/docs/openhuman-integration.md @@ -43,9 +43,20 @@ host links the contract crate — which it should, and which is the next point. # The wire contract for the tinybrowser module: member names and payload types. # Two pure-Rust dependencies, no transport, no browser — the whole reason the # module is loadable rather than linked. -tinybrowser-bus = { git = "https://github.com/tinyhumansai/tinybrowser" } +# +# Pinned to the tag the registry entry above downloads its artifact from. An +# unpinned git dependency resolves to whatever the default branch holds at build +# time, so a host would eventually compile against payload types newer than the +# module it actually loads — and the mismatch surfaces as a decode error at +# runtime, in a call, rather than as a build failure. +tinybrowser-bus = { git = "https://github.com/tinyhumansai/tinybrowser", tag = "v<version>" } ``` +Move the tag and the registry entry's `version` together, in one commit. They are +the same decision written twice, and `ContractVersion` is what catches it when +they drift anyway — but catching it in review is cheaper than catching it in a +session. + `tinybrowser-bus`, never `tinybrowser`. The second one is the engine, and linking it would put a WebSocket client, a TLS stack and a CDP surface back into the binary this arrangement exists to keep them out of. From 4dc27c4129c9963716b2ca0188122317a90737aa Mon Sep 17 00:00:00 2001 From: Steven Enamakel <enamakel@tinyhumans.ai> Date: Fri, 21 Aug 2026 23:29:58 +0300 Subject: [PATCH 10/11] fix(test): use saturating sub for TTL in survival test Changed the test for output survival within TTL to use `saturating_sub` instead of direct subtraction, preventing a potential underflow when TTL is smaller than one second. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai> --- crates/tinybrowser/src/capture/test.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/tinybrowser/src/capture/test.rs b/crates/tinybrowser/src/capture/test.rs index e6258a3..087b4cd 100644 --- a/crates/tinybrowser/src/capture/test.rs +++ b/crates/tinybrowser/src/capture/test.rs @@ -195,7 +195,7 @@ fn an_output_expires_once_its_time_to_live_has_passed() { #[test] fn an_output_within_its_time_to_live_survives() { let (mut store, id) = store_with(b"hello".to_vec()); - store.age(TTL - std::time::Duration::from_secs(1)); + store.age(TTL.saturating_sub(std::time::Duration::from_secs(1))); assert!(store.read(&id, 0, 1024).is_ok()); } From 1b06941d59291bb39fd04561c3228a6308eff4ac Mon Sep 17 00:00:00 2001 From: Steven Enamakel <enamakel@tinyhumans.ai> Date: Fri, 21 Aug 2026 23:31:22 +0300 Subject: [PATCH 11/11] refactor(capture): extract screenshot size check into a helper The inline size check in the screenshot function has been moved into a new `within_cap` helper, making the pre-decode guard reusable and testable. Three tests verify that ordinary screenshots pass, oversized ones are refused before decoding, and the check aligns with the store's own capacity limit. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai> --- crates/tinybrowser/src/capture/mod.rs | 46 +++++++++++++++++--------- crates/tinybrowser/src/capture/test.rs | 27 ++++++++++++++- 2 files changed, 57 insertions(+), 16 deletions(-) diff --git a/crates/tinybrowser/src/capture/mod.rs b/crates/tinybrowser/src/capture/mod.rs index 7f9020a..bbbd2f9 100644 --- a/crates/tinybrowser/src/capture/mod.rs +++ b/crates/tinybrowser/src/capture/mod.rs @@ -92,21 +92,7 @@ pub(crate) async fn screenshot( .and_then(Value::as_str) .ok_or_else(|| Error::page("browser captured no image data".to_string()))?; - // Checked before decoding, not after. `OutputStore::insert` rejects an image - // larger than it will hold, but by then the decode has already allocated it: - // the CDP frame is unbounded by design — a full-page capture legitimately - // exceeds any frame cap worth setting — so the first place the size is known - // is the length of the encoded text, and the first place it can be refused - // without paying for it is here. Base64 carries three bytes in four. - let decoded_len = encoded.len() / 4 * 3; - if decoded_len > store::MAX_OUTPUT_BYTES { - return Err(Error::LimitExceeded { - message: format!( - "screenshot of about {decoded_len} bytes exceeds the {} byte cap", - store::MAX_OUTPUT_BYTES - ), - }); - } + within_cap(encoded.len())?; let bytes = BASE64 .decode(encoded) @@ -145,6 +131,36 @@ fn pixels(dimension: f64) -> u32 { } } +/// Refuses an image too large to hold, from the length of its encoding. +/// +/// Checked before decoding, not after. [`store::OutputStore::insert`] rejects an +/// image larger than it will hold, but by then the decode has already allocated +/// it — and the CDP frame carrying it is unbounded by design, because a +/// full-page capture legitimately exceeds any frame cap worth setting. So the +/// first place the size is known is the length of the encoded text, and this is +/// the first place it can be refused without paying for it. +/// +/// Base64 carries three bytes in four, so the encoded length gives the decoded +/// size to within the padding. +/// +/// # Errors +/// +/// [`Error::LimitExceeded`] when the image would exceed the module's cap. +fn within_cap(encoded_len: usize) -> Result<()> { + let decoded_len = encoded_len / 4 * 3; + + if decoded_len > store::MAX_OUTPUT_BYTES { + return Err(Error::LimitExceeded { + message: format!( + "screenshot of about {decoded_len} bytes exceeds the {} byte cap", + store::MAX_OUTPUT_BYTES + ), + }); + } + + Ok(()) +} + /// The clip rectangle covering one element. async fn element_clip(session: &Session, node: i64) -> Result<Value> { let model = session diff --git a/crates/tinybrowser/src/capture/test.rs b/crates/tinybrowser/src/capture/test.rs index 087b4cd..52a2ea9 100644 --- a/crates/tinybrowser/src/capture/test.rs +++ b/crates/tinybrowser/src/capture/test.rs @@ -11,7 +11,8 @@ use base64::Engine as _; use base64::engine::general_purpose::STANDARD as BASE64; use tinybrowser_bus::OutputId; -use super::store::{OutputStore, TTL}; +use super::store::{MAX_OUTPUT_BYTES, OutputStore, TTL}; +use super::within_cap; use crate::error::Error; fn store_with(bytes: Vec<u8>) -> (OutputStore, OutputId) { @@ -218,3 +219,27 @@ fn sweeping_releases_what_nothing_else_would_have() { assert_eq!(store.len(), 0); } + +#[test] +fn an_ordinary_screenshot_passes_the_pre_decode_check() { + // A full-page capture at 2x is a few megabytes; nothing near the cap. + assert!(within_cap(4 * 1024 * 1024).is_ok()); + assert!(within_cap(0).is_ok()); +} + +#[test] +fn an_oversized_screenshot_is_refused_before_it_is_decoded() { + // The point is the ordering: `OutputStore::insert` would refuse this too, + // but only after the decode had already allocated it in the host's process. + let encoded_len = MAX_OUTPUT_BYTES / 3 * 4 + 8; + let error = within_cap(encoded_len).expect_err("refused"); + + assert!(matches!(error, Error::LimitExceeded { .. }), "{error}"); +} + +#[test] +fn the_pre_decode_check_agrees_with_the_store_it_is_guarding() { + // An encoding that decodes to exactly the cap must pass, or the check would + // refuse images the store would happily have held. + assert!(within_cap(MAX_OUTPUT_BYTES / 3 * 4).is_ok()); +}