From 36a44a35cb8c6478fea61fa2bef7a2c3b661a78b Mon Sep 17 00:00:00 2001 From: prxnvt <53838599+prxnvt@users.noreply.github.com> Date: Mon, 2 Nov 2026 02:20:48 -0500 Subject: [PATCH] Form controls: a button never opens a web page or a dialog A button's command ran through the macro policy and the file sandbox, but not through the macro runner's request filter, so a file-borne button assigned `link.open {url}` opened the page once the generic trust question was answered. Now `link.open` is refused for buttons up front, when it is assigned and when a file brings it in, with the wording of the other denied commands; and after a press runs its command, the UI requests it queued are checked as the macro runner checks a step's: a dialog or a web page makes the press fail with the reason and rolls the group back, a cell edit is dropped, and messages for the person stay queued. Tests: link.open joins a_denied_command_is_refused_when_assigned_and_when_a_file_brings_it_in (assign, insert and a file-borne press refuse it; no OpenUrl request is left), and a_button_command_that_would_open_a_dialog_or_a_page_is_refused_after_it_runs (insert.link with no parameters is refused with the dialog wording and its request dropped; the filter refuses a page, drops a cell edit and keeps messages). docs/macros.md says what a button's command may not do. --- crates/engine/src/cmd/controls.rs | 49 +++++++++++++++++++++++++++-- crates/engine/src/tests/controls.rs | 30 ++++++++++++++++++ docs/macros.md | 5 ++- 3 files changed, 80 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/controls.rs b/crates/engine/src/cmd/controls.rs index 0e03f8f9f..ac1c9d90f 100644 --- a/crates/engine/src/cmd/controls.rs +++ b/crates/engine/src/cmd/controls.rs @@ -369,6 +369,38 @@ fn format(s: &mut Session, p: &Json) -> Result { }) } +/// Commands a button never runs, beyond the macro policy's lists: a button has no way to show +/// what its command would open, so a web page is refused when the action is assigned or comes in +/// from a file. A command that turns out to open a page or a dialog when it runs is refused +/// then, by [`press`]. +const BUTTON_DENIED: &[(&str, &str)] = &[("link.open", "buttons don't open web pages")]; + +/// The macro policy for a button's command, with the button's own refusals first. +fn button_policy(id: &str, params: &Json, trust: crate::Trust) -> Policy { + match BUTTON_DENIED.iter().find(|(d, _)| *d == id) { + Some((_, why)) => Policy::Deny((*why).into()), + None => macro_policy(id, params, trust), + } +} + +/// Drops the UI requests a button's command queued from `queued` on, keeping messages for the +/// person, and says why the press is refused when one would open a dialog or a web page, which +/// buttons don't do (`name` is the button's). A cell edit the command asked for is dropped too. +pub(crate) fn take_requests(s: &mut Session, queued: usize, name: &str) -> Option { + use crate::UiRequest; + let mut blocked = None; + let asked: Vec = s.ui_requests.drain(queued.min(s.ui_requests.len())..).collect(); + for q in asked { + match q { + UiRequest::Dialog(d, _) => blocked = Some(format!("'{name}' would open the {d} dialog; give its command the parameters it needs.")), + UiRequest::OpenUrl(_) => blocked = Some(format!("'{name}' would open a web page, which buttons don't do.")), + UiRequest::EditCell(_) | UiRequest::InsertName(_) => {} + m @ UiRequest::Message(_) => s.ui_requests.push(m), + } + } + blocked +} + /// A button action from `macro` (with `input` and `output`) or `command` (with `params`). fn action_param(s: &Session, cmd: &str, p: &Json) -> Result> { if let Some(name) = str_param(p, "macro").map(str::trim).filter(|n| !n.is_empty()) { @@ -403,7 +435,7 @@ fn action_param(s: &Session, cmd: &str, p: &Json) -> Result Result { Some(ControlAction::Command { id, params }) => { let d = s.doc()?; let (doc, trust) = (d.uid, d.trust); - let policy = macro_policy(&id, ¶ms, trust); + let policy = button_policy(&id, ¶ms, trust); let file_command = matches!(policy, Policy::NeedsPermission(_)); let why = match policy { Policy::Allow => None, @@ -508,7 +540,18 @@ fn press(s: &mut Session, p: &Json) -> Result { // As a macro step: only an approved file command reaches the file system, so a // command that reads or writes a file under another parameter name can't. let _sandbox = (!file_command).then(crate::io::ScriptSandbox::enter); - s.grouped(&name, |s| s.execute(&id, params.clone())) + s.grouped(&name, |s| { + let queued = s.ui_requests.len(); + let r = s.execute(&id, params.clone()); + // As a macro step: a command that would open a dialog or a web page is refused + // and its group rolled back; messages for the person stay queued. + let blocked = take_requests(s, queued, &name); + let r = r?; + match blocked { + Some(why) => Err(EngineError::Other(why)), + None => Ok(r), + } + }) } } } diff --git a/crates/engine/src/tests/controls.rs b/crates/engine/src/tests/controls.rs index e567fb913..5442aabde 100644 --- a/crates/engine/src/tests/controls.rs +++ b/crates/engine/src/tests/controls.rs @@ -625,6 +625,7 @@ fn a_denied_command_is_refused_when_assigned_and_when_a_file_brings_it_in() { let dir = std::env::temp_dir().join(format!("gridcraft-control-deny-{}", std::process::id())); let leak = dir.join("leak.xlsx").to_string_lossy().to_string(); let denied = [ + ("link.open", json!({"url": "https://example.com/?d=secret"})), ("file.saveAs", json!({"path": leak})), ("file.save", json!({})), ("file.open", json!({"path": leak})), @@ -665,6 +666,35 @@ fn a_denied_command_is_refused_when_assigned_and_when_a_file_brings_it_in() { } assert!(!std::path::Path::new(&leak).exists()); assert_eq!(v(&s, "A1"), Value::Empty, "the macro never ran"); + assert!(!s.ui_requests.iter().any(|r| matches!(r, crate::UiRequest::OpenUrl(_))), "no page was opened: {:?}", s.ui_requests); +} + +/// A command the policy allows may still turn out to open a dialog or a web page when it runs +/// (a command given no parameters opens its dialog): the press is refused with the reason, as a +/// macro step is, and the request is dropped, so nothing opens. Messages for the person pass. +#[test] +fn a_button_command_that_would_open_a_dialog_or_a_page_is_refused_after_it_runs() { + use crate::UiRequest; + use crate::cmd::controls::take_requests; + let mut s = s(); + let b = id(&s.execute("insert.button", json!({"at": "B2", "text": "Link", "command": "insert.link", "params": {}})).unwrap()); + let before = undo_len(&s); + let name = control(&s, b).display_name(); + assert_eq!( + err(s.execute("control.press", json!({"id": b}))), + format!("'{name}' would open the insertLink dialog; give its command the parameters it needs.") + ); + assert!(!s.ui_requests.iter().any(|r| matches!(r, UiRequest::Dialog(..))), "{:?}", s.ui_requests); + assert_eq!(undo_len(&s), before, "nothing stays done"); + // The filter itself, with every kind of request a command can queue: the page is refused, + // the cell edit is dropped, the message stays, and requests queued before are left alone. + s.ui_requests.push(UiRequest::Message("earlier".into())); + let queued = s.ui_requests.len(); + s.ui_requests.extend([UiRequest::EditCell(None), UiRequest::Message("saved".into()), UiRequest::OpenUrl("https://example.com/".into())]); + assert_eq!(take_requests(&mut s, queued, "Go").as_deref(), Some("'Go' would open a web page, which buttons don't do.")); + assert_eq!(s.ui_requests, vec![UiRequest::Message("earlier".into()), UiRequest::Message("saved".into())]); + assert_eq!(take_requests(&mut s, 5, "Go"), None, "a mark past the queue's end drops nothing"); + assert_eq!(s.ui_requests.len(), 2); } /// Using a slider or spin button writes its linked cell through `cell.set`, so the cell's diff --git a/docs/macros.md b/docs/macros.md index 8073b2fcd..b4937fcbe 100644 --- a/docs/macros.md +++ b/docs/macros.md @@ -263,7 +263,10 @@ default) or always on one range (`input: "B2:D9"`), with an optional output cell {id, command, params}` makes it run a command with fixed parameters. A macro runs through `macro.run` with its checks; a command goes through the same policy as a macro step and runs, as a step does, inside the scripts' file sandbox unless it is an approved file command, so a button -in a workbook opened from a file asks before it runs anything. +in a workbook opened from a file asks before it runs anything. Like a step, a button's command +never opens a web page or a dialog: `link.open` is refused when it is assigned or comes in from +a file, and a command that would open a dialog or a page once it runs (a command given no +parameters opens its dialog) is refused then, with the request dropped and nothing done. In xlsx a button carries its macro as Excel writes it (`macro="[0]!Name"`, a macro run on the selection). A command, a fixed input range or an output cell has no place there and is kept in a