From 791c460546810c24bcd09202496f583532d858fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Ferreira?= Date: Wed, 14 Oct 2026 17:16:40 +0100 Subject: [PATCH] Fix browser capabilities and persistent MCP session handling --- crates/obscura-js/js/bootstrap.js | 17 ++- crates/obscura-js/src/frame.rs | 4 + crates/obscura-js/src/ops.rs | 30 +++++ crates/obscura-js/src/runtime.rs | 63 +++++++++++ crates/obscura-mcp/src/lib.rs | 177 +++++++++++++++++++++++++----- 5 files changed, 264 insertions(+), 27 deletions(-) diff --git a/crates/obscura-js/js/bootstrap.js b/crates/obscura-js/js/bootstrap.js index ccfc7f4..f69012a 100644 --- a/crates/obscura-js/js/bootstrap.js +++ b/crates/obscura-js/js/bootstrap.js @@ -7083,6 +7083,9 @@ function _elementClassFor(nid) { if (svgClass) return svgClass; if (globalThis.SVGElement) return globalThis.SVGElement; } + if (tag === "OL" && globalThis.HTMLOListElement + && _domParse("namespace_uri", nid) === "http://www.w3.org/1999/xhtml" + && _domParse("local_name", nid) === "ol") return globalThis.HTMLOListElement; if (tag === "FORM" && globalThis.HTMLFormElement) return globalThis.HTMLFormElement; if (tag === "INPUT" && globalThis.HTMLInputElement && _domParse("namespace_uri", nid) === "http://www.w3.org/1999/xhtml" @@ -7116,6 +7119,7 @@ function _elementClassForKnownName(namespace, qualifiedName) { if (namespace === "http://www.w3.org/1999/xhtml") { const tag = localName.toUpperCase(); if (tag === "FORM" && globalThis.HTMLFormElement) return globalThis.HTMLFormElement; + if (localName === "ol" && globalThis.HTMLOListElement) return globalThis.HTMLOListElement; if (localName === "input" && globalThis.HTMLInputElement) return globalThis.HTMLInputElement; if (tag === "TEXTAREA" && globalThis.HTMLTextAreaElement) return globalThis.HTMLTextAreaElement; if (localName === "meta" && globalThis.HTMLMetaElement) return globalThis.HTMLMetaElement; @@ -7154,6 +7158,10 @@ globalThis._wrap = _wrap; globalThis.self = globalThis; globalThis.document = null; +Object.defineProperty(globalThis, 'isSecureContext', { + get() { return _domParse("document_is_secure_context") === true; }, + enumerable: true, configurable: true, +}); function _resolveUrl(url) { url = String(url); if (!url) return url; @@ -13241,7 +13249,13 @@ globalThis.HTMLHtmlElement = Element; globalThis.HTMLBRElement = Element; globalThis.HTMLHRElement = Element; globalThis.HTMLUListElement = Element; -globalThis.HTMLOListElement = Element; +globalThis.HTMLOListElement = class HTMLOListElement extends Element { + get reversed() { return this.hasAttribute("reversed"); } + set reversed(value) { + if (value) this.setAttribute("reversed", ""); + else this.removeAttribute("reversed"); + } +}; globalThis.HTMLLIElement = Element; globalThis.HTMLPreElement = Element; globalThis.HTMLHeadingElement = Element; @@ -15593,6 +15607,7 @@ globalThis.Worker = class Worker { console: globalThis.console, performance: globalThis.performance, location: globalThis.location, + isSecureContext: globalThis.isSecureContext, }; scope.self = scope; return scope; diff --git a/crates/obscura-js/src/frame.rs b/crates/obscura-js/src/frame.rs index cfeb283..9794e84 100644 --- a/crates/obscura-js/src/frame.rs +++ b/crates/obscura-js/src/frame.rs @@ -103,6 +103,10 @@ impl FrameRealm { state.url = url.to_string(); state.frame_id = frame_id; parent.share_resources_with(&mut state); + if parent_frame_id != 0 { + state.secure_ancestors = realms.borrow().by_frame_id(parent_frame_id) + .is_some_and(|parent| parent.borrow().is_secure_context()); + } let realms = parent.realm_states(); realms.borrow_mut().register( diff --git a/crates/obscura-js/src/ops.rs b/crates/obscura-js/src/ops.rs index 2a52399..c7dc000 100644 --- a/crates/obscura-js/src/ops.rs +++ b/crates/obscura-js/src/ops.rs @@ -322,12 +322,34 @@ struct StorageOriginCache { origin: String, } +// Secure Contexts: authenticated schemes and local development origins are +// potentially trustworthy. Private network addresses are not loopback origins. +fn potentially_trustworthy_url(value: &str) -> bool { + let Ok(parsed) = url::Url::parse(value) else { return false }; + if matches!(parsed.scheme(), "https" | "wss" | "file") { return true; } + if parsed.scheme() == "blob" { + let origin = parsed.origin().ascii_serialization(); + return origin != "null" && potentially_trustworthy_url(&origin); + } + match parsed.host() { + Some(url::Host::Ipv4(ip)) => ip.is_loopback(), + Some(url::Host::Ipv6(ip)) => ip.is_loopback(), + Some(url::Host::Domain(host)) => { + let host = host.trim_end_matches('.'); + host == "localhost" || host.ends_with(".localhost") + } + None => false, + } +} + pub struct ObscuraState { pub dom: Option, pub url: String, /// Initial about:blank documents retain their creator's base URL and origin. pub(crate) about_base_url: Option, pub(crate) inherited_origin: Option, + /// A trustworthy frame is still insecure when any embedding context is insecure. + pub(crate) secure_ancestors: bool, storage_origin_cache: RefCell, /// WHATWG canonical name of the document's character encoding (e.g. /// "UTF-8", "EUC-JP"). Backs `document.characterSet` and the URL query @@ -586,6 +608,7 @@ pub struct PendingFrameMessage { impl ObscuraState { pub(crate) fn inherit_resources(&mut self, parent: &Self) { + self.secure_ancestors = parent.is_secure_context(); self.cookie_jar = parent.cookie_jar.clone(); // A frame shares its page's Web Storage Arcs. The ops key by the frame // realm's own document URL, so a cross-origin frame still lands in its @@ -608,6 +631,11 @@ impl ObscuraState { } } + pub(crate) fn is_secure_context(&self) -> bool { + self.secure_ancestors && potentially_trustworthy_url( + self.inherited_origin.as_deref().unwrap_or(&self.url)) + } + pub fn new() -> Self { #[cfg(feature = "render")] let (render_resource_tx, render_resource_rx) = tokio::sync::mpsc::unbounded_channel(); @@ -616,6 +644,7 @@ impl ObscuraState { url: "about:blank".to_string(), about_base_url: None, inherited_origin: None, + secure_ancestors: true, storage_origin_cache: RefCell::new(StorageOriginCache { origin: "null".into(), ..Default::default() }), encoding: "UTF-8".to_string(), content_type: String::new(), @@ -2362,6 +2391,7 @@ fn op_dom_inner(shared: SharedState, cmd: String, arg1: String, arg2: String) -> .unwrap_or_default(); serde_json::to_string(&title).unwrap_or("\"\"".into()) } + "document_is_secure_context" => gs.is_secure_context().to_string(), "document_url" => serde_json::to_string(&gs.url).unwrap_or("\"\"".into()), "document_origin" => serde_json::to_string(&gs.inherited_origin.clone().unwrap_or_else(|| { url::Url::parse(&gs.url).map(|url| url.origin().ascii_serialization()) diff --git a/crates/obscura-js/src/runtime.rs b/crates/obscura-js/src/runtime.rs index 2290afa..8b02c21 100644 --- a/crates/obscura-js/src/runtime.rs +++ b/crates/obscura-js/src/runtime.rs @@ -24743,4 +24743,67 @@ mod tests { "label association must follow the HTML labelable-element rules" ); } + #[test] + fn ordered_list_reversed_reflects_boolean_attribute_on_html_lists_only() { + let mut rt = setup_runtime("
  1. One
"); + let result = rt.evaluate(r#"(() => { + const parsed = document.querySelector('ol'); + const created = document.createElement('ol'); + const namespaced = document.createElementNS('http://www.w3.org/1999/xhtml', 'ol'); + const foreign = document.createElementNS('urn:other', 'ol'); + const values = [parsed instanceof HTMLOListElement, parsed.reversed, + created instanceof HTMLOListElement, created.reversed, + namespaced instanceof HTMLOListElement, !('reversed' in foreign), + !('reversed' in document.querySelector('div'))]; + created.reversed = true; + values.push(created.getAttribute('reversed') === '', created.cloneNode().reversed); + created.reversed = false; + values.push(!created.hasAttribute('reversed')); + created.setAttribute('reversed', 'false'); + values.push(created.reversed); + created.removeAttribute('reversed'); + values.push(!created.reversed); + return values; + })()"#).unwrap(); + assert_eq!(result, serde_json::json!([true,true,true,false,true,true,true,true,true,true,true,true])); + } + + #[test] + fn secure_context_tracks_document_origin_and_embedding_context() { + let mut rt = setup_runtime(""); + for (url, expected) in [ + ("https://example.test/", true), ("http://example.test/", false), + ("http://localhost/", true), ("http://app.localhost/", true), + ("http://localhost.evil.test/", false), ("http://127.0.0.2/", true), + ("http://[::1]/", true), ("http://192.168.1.1/", false), + ("file:///tmp/page.html", true), ("data:text/html,hello", false), + ("blob:https://example.test/id", true), ("about:blank", false), + ] { + rt.set_url(url); + assert_eq!(rt.evaluate("isSecureContext").unwrap(), serde_json::json!(expected), "{url}"); + } + rt.set_url("https://example.test/"); + rt.state.borrow_mut().secure_ancestors = false; + assert_eq!(rt.evaluate("isSecureContext").unwrap(), serde_json::json!(false)); + rt.state.borrow_mut().secure_ancestors = true; + rt.set_url("about:blank"); + rt.state.borrow_mut().inherited_origin = Some("https://creator.test".into()); + assert_eq!(rt.evaluate("isSecureContext").unwrap(), serde_json::json!(true)); + rt.state.borrow_mut().inherited_origin = Some("http://creator.test".into()); + assert_eq!(rt.evaluate("isSecureContext").unwrap(), serde_json::json!(false)); + } + + #[test] + fn secure_context_respects_nested_insecure_frames() { + let mut rt = setup_runtime(""); + rt.set_url("https://secure.test/"); + let insecure = crate::frame::FrameRealm::new(&mut rt, 501, 0, + "http://insecure.test/", "").unwrap(); + let nested = crate::frame::FrameRealm::new(&mut rt, 502, 501, + "https://secure.test/", "").unwrap(); + assert_eq!(insecure.evaluate(&mut rt, "isSecureContext").unwrap(), serde_json::json!(false)); + assert_eq!(nested.evaluate(&mut rt, "isSecureContext").unwrap(), serde_json::json!(false)); + assert_eq!(rt.evaluate("isSecureContext").unwrap(), serde_json::json!(true)); + } + } diff --git a/crates/obscura-mcp/src/lib.rs b/crates/obscura-mcp/src/lib.rs index 87916c3..7058f55 100644 --- a/crates/obscura-mcp/src/lib.rs +++ b/crates/obscura-mcp/src/lib.rs @@ -200,26 +200,24 @@ impl BrowserState { /// /// [`Self::advance_active_page_tasks`] cannot cover this. It is armed /// after every dispatch, but it sits in a `biased` select behind - /// `read_line`, so a client that sends its next tool call immediately, + /// stdin reading, so a client that sends its next tool call immediately, /// which an agent does, wins that race every time. The reply is also /// written before any pump turn could run, so the tool would answer /// "Clicked" while the request has not left the process. async fn settle_synthetic_navigation(&mut self) -> Result<(), String> { - let navigated = self - .page_mut() - .process_pending_navigation() - .await - .map_err(|error| error.to_string())?; - if navigated { - // The ref table names elements in a document that has gone away. + let outcome = self.page_mut().process_pending_navigation_outcome().await + .map_err(|error|error.to_string())?; + if outcome == obscura_browser::page::PendingNavigationOutcome::Document { self.interactive_refs.clear(); - // One slice on the landed document, so a tool that reads the URL - // or the text next sees the new page rather than an empty one. - self.page_mut() - .run_autonomous_event_loop_turn() - .await - .map_err(|error| error.to_string())?; + // A bounded slice is enough to advance the landed document. A + // websocket or long timer must not hold a protocol response open. + if let Ok(result)=tokio::time::timeout(std::time::Duration::from_millis(50),self.page_mut().run_autonomous_event_loop_turn()).await { + result.map_err(|error|error.to_string())?; + } } + // pushState/replaceState keep the document. Its own task already + // delivered the route change, so waiting for another task can stall + // until an unrelated long-lived connection or interval wakes. Ok(()) } @@ -256,19 +254,28 @@ pub async fn run(proxy: Option, user_agent: Option, stealth: boo let mut reader = BufReader::new(stdin); let mut writer = stdout; + run_transport(&mut reader, &mut writer, proxy, user_agent, stealth).await +} + +async fn run_transport( + reader: &mut R, writer: &mut W, proxy: Option, user_agent: Option, stealth: bool, +) -> Result<()> { let mut state = BrowserState::new(proxy, user_agent, stealth); state.operator_network_hints = true; + // Keep partial input across page-task wakes. read_line is not cancel-safe; + // read_until appends consumed bytes before yielding, so this buffer must + // survive the select and may only be cleared after consuming a request. + let mut pending_line = Vec::new(); // Cookies and Web Storage are written on every navigation; this covers the // writes an agent makes after a page has settled, on the way out. let mut runtime_pump_armed = false; loop { // MCP stdio transport: newline-delimited JSON (one message per line) - let mut line = String::new(); let n = if runtime_pump_armed { tokio::select! { biased; - read = reader.read_line(&mut line) => Some(read?), + read = reader.read_until(b'\n', &mut pending_line) => Some(read?), pump_result = state.advance_active_page_tasks() => { match pump_result { Ok(reached_idle) => runtime_pump_armed = !reached_idle, @@ -281,16 +288,17 @@ pub async fn run(proxy: Option, user_agent: Option, stealth: boo } } } else { - Some(reader.read_line(&mut line).await?) + Some(reader.read_until(b'\n', &mut pending_line).await?) }; let Some(n) = n else { continue; }; - if n == 0 { + if n == 0 && pending_line.is_empty() { state.context.save_storage(); return Ok(()); } + let line = String::from_utf8(std::mem::take(&mut pending_line))?; let trimmed = line.trim(); if trimmed.is_empty() { continue; @@ -431,6 +439,7 @@ fn handle_tools_list(id: Value) -> RpcResponse { "type": "object", "properties": { "expression": { "type": "string", "description": "JavaScript expression to evaluate; a promise it returns is awaited" }, + "refreshRefs": {"type":"boolean","description":"Rebuild element refs immediately before evaluating a coherent DOM observation."}, "timeoutMs": { "type": "number", "description": "How long to wait for a returned promise, in milliseconds (default: 5000)" } }, "required": ["expression"] @@ -1009,7 +1018,7 @@ fn tool_snapshot(args: &Value, state: &mut BrowserState) -> Result Result Result Result Err(format!("Element not found: {selector}")), Some("error:option not found") => Err(format!("Option not found: {value}")), + Some(error) if error.starts_with("error:") => Err(error.to_string()), _ => Ok(format!("Selected '{value}' in '{selector}'")), } } @@ -1242,7 +1276,8 @@ async fn settle_expression_promise( // A promise that settles in a network callback needs the page's own // task queue to turn. Polling evaluate alone leaves it pending forever, // which is how `await fetch(...)` looked like a hang. - let _ = state.advance_active_page_tasks().await; + let remaining=deadline.saturating_duration_since(std::time::Instant::now()); + let _ = tokio::time::timeout(remaining,state.advance_active_page_tasks()).await; let _ = last; } } @@ -1251,6 +1286,7 @@ async fn tool_evaluate(args: &Value, state: &mut BrowserState) -> Result ResultReady

","waitUntil":"domcontentloaded"}}}), + json!({"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"browser_evaluate","arguments":{"expression":"(function(){setTimeout(function(){document.title='Awake';},100);return 'armed';})()"}}}), + ]; + for request in requests { + client_write.write_all(format!("{request}\n").as_bytes()).await.unwrap(); + let mut reply=String::new();client_read.read_line(&mut reply).await.unwrap(); + let reply:Value=serde_json::from_str(&reply).unwrap();assert_eq!(reply["id"],request["id"]);assert!(reply["result"]["isError"]!=true,"{reply}"); + } + let request=format!("{}\n",json!({"jsonrpc":"2.0","id":3,"method":"tools/call","params":{"name":"browser_snapshot","arguments":{"max_chars":100}}})); + client_write.write_all(&request.as_bytes()[..35]).await.unwrap(); + tokio::time::sleep(std::time::Duration::from_millis(300)).await; + client_write.write_all(&request.as_bytes()[35..]).await.unwrap(); + let mut reply=String::new();tokio::time::timeout(std::time::Duration::from_secs(2),client_read.read_line(&mut reply)).await.expect("page wake must not discard partial request").unwrap(); + let reply:Value=serde_json::from_str(&reply).unwrap();assert_eq!(reply["id"],3);assert!(reply.to_string().contains("Awake")); + client_write.shutdown().await.unwrap(); + }; + let (server_result,())=tokio::join!(server_task,client_task);server_result.unwrap(); + } + + #[tokio::test(flavor = "current_thread")] + async fn interactive_labels_use_associated_labels_and_never_password_values() { + let mut state=BrowserState::new(None,None,false); + state.page_mut().navigate("data:text/html,").await.unwrap(); + let elements=tool_interactive_elements(&json!({}),&mut state).unwrap(); + assert!(elements.contains("\"Username\""),"{elements}");assert!(elements.contains("\"Theme\""),"{elements}");assert!(elements.contains("\"Continue\""),"{elements}");assert!(!elements.contains("private-secret"),"{elements}"); + } + + #[tokio::test(flavor = "current_thread")] + async fn mcp_enter_submits_inputs_honors_cancellation_and_edits_textareas() { + let mut state=BrowserState::new(None,None,false); + state.page_mut().navigate("data:text/html,
").await.unwrap(); + state.page_mut().evaluate("(function(){globalThis.submits=0;document.getElementById('f').addEventListener('submit',function(e){e.preventDefault();submits++;});return true;})()"); + tool_press_key(&json!({"selector":"#i","key":"Enter"}),&mut state).await.unwrap();assert_eq!(state.page_mut().evaluate("submits").as_f64(),Some(1.0)); + state.page_mut().evaluate("document.getElementById('i').addEventListener('keydown',function(e){e.preventDefault();});"); + tool_press_key(&json!({"selector":"#i","key":"Enter"}),&mut state).await.unwrap();assert_eq!(state.page_mut().evaluate("submits").as_f64(),Some(1.0)); + tool_press_key(&json!({"selector":"#t","key":"Enter"}),&mut state).await.unwrap();assert_eq!(state.page_mut().evaluate("document.getElementById('t').value"),json!("\n"));assert_eq!(state.page_mut().evaluate("submits").as_f64(),Some(1.0)); + assert!(tool_press_key(&json!({"selector":"#missing","key":"Enter"}),&mut state).await.is_err()); + } + + #[tokio::test(flavor = "current_thread")] + async fn evaluation_refresh_refs_uses_the_current_document() { + let mut state=BrowserState::new(None,None,false); + state.page_mut().navigate("data:text/html,").await.unwrap();tool_snapshot(&json!({}),&mut state).unwrap(); + state.page_mut().evaluate("document.body.innerHTML='';"); + let observation=tool_evaluate(&json!({"refreshRefs":true,"expression":"({text:document.body.textContent,ref:document.querySelector('button').getAttribute('data-obscura-ref')})"}),&mut state).await.unwrap(); + let observation:Value=serde_json::from_str(&observation).unwrap();assert_eq!(observation["text"],"New");assert_eq!(observation["ref"],"e1"); + tool_click(&json!({"ref":"e1"}),&mut state).await.unwrap();assert_eq!(state.page_mut().evaluate("document.body.dataset.clicked"),json!("1")); + } + + #[tokio::test(flavor = "current_thread")] + async fn selecting_option_dispatches_trusted_input_and_change() { + let mut state=BrowserState::new(None,None,false); + state.page_mut().navigate("data:text/html,").await.unwrap(); + tool_select_option(&json!({"selector":"#s","value":"dark"}),&mut state).unwrap(); + assert_eq!(state.page_mut().evaluate("document.getElementById('s').value"),json!("dark")); + assert_eq!(state.page_mut().evaluate("document.getElementById('s').dataset.inputTrusted"),json!("true"));assert_eq!(state.page_mut().evaluate("document.getElementById('s').dataset.changeTrusted"),json!("true")); + assert!(tool_select_option(&json!({"selector":"#b","value":"dark"}),&mut state).is_err()); + } + + #[tokio::test(flavor = "current_thread")] + async fn same_document_route_does_not_wait_for_an_unrelated_long_timer() { + let mut state=BrowserState::new(None,None,false); + state.page_mut().navigate("data:text/html,

Route ready

").await.unwrap(); + state.page_mut().evaluate("setInterval(function(){},30000)"); + let result=tokio::time::timeout(std::time::Duration::from_millis(200),tool_evaluate(&json!({"expression":"(function(){history.replaceState({},'', '#route');return 'routed';})()"}),&mut state)).await.expect("same-document routing must not await a background task").unwrap(); + assert_eq!(result,"routed");assert!(state.page_mut().url_string().ends_with("#route")); + } + + #[tokio::test(flavor = "current_thread")] + async fn pending_expression_deadline_is_not_extended_by_a_background_timer() { + let mut state=BrowserState::new(None,None,false); + state.page_mut().navigate("data:text/html,

Ready

").await.unwrap();state.page_mut().evaluate("setInterval(function(){},30000)"); + let result=tokio::time::timeout(std::time::Duration::from_millis(200),tool_evaluate(&json!({"expression":"new Promise(function(){})","timeoutMs":40}),&mut state)).await.expect("expression deadline must bound task pumping").unwrap();assert!(result.contains("did not settle within 40 ms"),"{result}"); + } + // An untrusted page must not be able to pull a local file into the agent's // context by planting a file:// link for the agent to click. #[tokio::test(flavor = "current_thread")]