mirror of
https://github.com/storytold/gridcraft.git
synced 2026-11-07 02:51:33 +00:00
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.
This commit is contained in:
1 parent
975d5407a1
commit
36a44a35cb
3 files changed
+80
-4
No files matched your search
@@ -369,6 +369,38 @@ fn format(s: &mut Session, p: &Json) -> Result<Json> {
|
||||
})
|
||||
}
|
||||
|
||||
/// 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<String> {
|
||||
use crate::UiRequest;
|
||||
let mut blocked = None;
|
||||
let asked: Vec<UiRequest> = 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<Option<ControlAction>> {
|
||||
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<Option<ControlAction
|
||||
return Err(bad(cmd, "`params` must be an object"));
|
||||
}
|
||||
let trust = s.doc()?.trust;
|
||||
if let Policy::Deny(why) = macro_policy(id, ¶ms, trust) {
|
||||
if let Policy::Deny(why) = button_policy(id, ¶ms, trust) {
|
||||
return Err(EngineError::Other(format!("A button can't run {id}: {why}.")));
|
||||
}
|
||||
return Ok(Some(ControlAction::Command { id: id.to_string(), params }));
|
||||
@@ -489,7 +521,7 @@ fn press(s: &mut Session, p: &Json) -> Result<Json> {
|
||||
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<Json> {
|
||||
// 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),
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
+4
-1
@@ -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
|
||||
|
||||
Reference in new issue
Block a user