diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 102edc2d3..556d41759 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -1386,6 +1386,7 @@ fn paste_special(s: &mut Session, p: &Json) -> Result { gridcraft_formula::adjust::Edit::MoveToSheet { from: src, to_sheet: dest_name, to_row: at.row, to_col: at.col } }; rewrite_all_formulas(&mut cx.wb, &src_sheet_name, &e); + cx.edits.push((src_sheet_name.clone(), e)); cx.structural = true; } let end = at.offset_clamped((tiles_r * h) as i64 - 1, (tiles_c * w) as i64 - 1); diff --git a/crates/engine/src/cmd/insert.rs b/crates/engine/src/cmd/insert.rs index 3c45031af..0078cb4fe 100644 --- a/crates/engine/src/cmd/insert.rs +++ b/crates/engine/src/cmd/insert.rs @@ -294,7 +294,7 @@ fn insert_table(s: &mut Session, p: &Json) -> Result { let e = Edit::InsertCells { axis: Axis::Rows, range: RangeRef::new(r.start, CellRef::new(r.start.row, r.end.col)) }; shift_cell_features(sh, &e); let name = sh.name.clone(); - rewrite(&mut cx.wb, &name, &e); + rewrite(cx, &name, &e); let sh = cx.sheet_mut(sheet)?; for (i, c) in (r.start.col..=r.end.col).enumerate() { sh.set_value(CellRef::new(r.start.row, c), gridcraft_core::Value::text(format!("Column{}", i + 1))); diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index 90fe4ccc8..5e2afcddb 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -340,6 +340,9 @@ pub struct Ctx<'a> { /// The command applied sheet protection itself (row/column inserts and deletes the /// protection allows), so cells it shifts aren't refused as edits of locked cells. pub protection_checked: bool, + /// The structural edits the command made, in order (the sheet's name, the edit), as + /// references were rewritten for them. + pub edits: Vec<(String, gridcraft_formula::adjust::Edit)>, } pub(crate) fn edit(s: &mut Session, f: impl FnOnce(&mut Ctx) -> Result) -> Result { @@ -357,9 +360,17 @@ pub(crate) fn commit(d: &mut DocState, f: impl FnOnce(&mut Ctx) -> Result) #[allow(clippy::disallowed_methods)] let t0 = if cfg!(target_arch = "wasm32") { None } else { std::env::var_os("GRIDCRAFT_PROFILE").map(|_| std::time::Instant::now()) }; let mut sel = d.selection.clone(); - let mut ctx = Ctx { wb: (*d.wb).clone(), changed: Vec::new(), structural: false, sel: &mut sel, fit_rows: Vec::new(), protection_checked: false }; + let mut ctx = Ctx { + wb: (*d.wb).clone(), + changed: Vec::new(), + structural: false, + sel: &mut sel, + fit_rows: Vec::new(), + protection_checked: false, + edits: Vec::new(), + }; let r = f(&mut ctx)?; - let Ctx { mut wb, changed, structural, mut fit_rows, protection_checked, .. } = ctx; + let Ctx { mut wb, changed, structural, mut fit_rows, protection_checked, edits, .. } = ctx; if !protection_checked { check_protection(&d.wb, &wb, &d.unlocked_ranges)?; } @@ -373,6 +384,9 @@ pub(crate) fn commit(d: &mut DocState, f: impl FnOnce(&mut Ctx) -> Result) } } } + if let Some(log) = d.edits.as_mut() { + log.extend(edits); + } if changed.len() <= 200_000 { fit_rows.extend(changed.iter().map(|(s, c)| (*s, c.row))); } diff --git a/crates/engine/src/cmd/sheet.rs b/crates/engine/src/cmd/sheet.rs index e6caa1dc7..dfab93736 100644 --- a/crates/engine/src/cmd/sheet.rs +++ b/crates/engine/src/cmd/sheet.rs @@ -223,7 +223,9 @@ fn shift_sheet_features(sh: &mut Sheet, axis: Axis, at: u32, count: u32, insert: } /// Rewrites every formula reference (and chart series) for a structural edit on `target`. -pub(super) fn rewrite(wb: &mut gridcraft_model::Workbook, target: &str, e: &Edit) { +pub(super) fn rewrite(cx: &mut Ctx, target: &str, e: &Edit) { + cx.edits.push((target.to_string(), e.clone())); + let wb = &mut cx.wb; super::edit::rewrite_all_formulas(wb, target, e); for si in 0..wb.sheets.len() { let host = wb.sheets.get(si).map(|s| s.name.clone()).unwrap_or_default(); @@ -307,7 +309,7 @@ fn insert_lines(s: &mut Session, p: &Json, axis: Axis) -> Result { } } } - rewrite(&mut cx.wb, &name, &Edit::Insert { axis, at, count }); + rewrite(cx, &name, &Edit::Insert { axis, at, count }); cx.structural = true; Ok(Json::Null) })?; @@ -372,7 +374,7 @@ fn delete_lines(s: &mut Session, p: &Json, axis: Axis) -> Result { } shift_sheet_features(sh, axis, at, count, false); } - rewrite(&mut cx.wb, &name, &Edit::Delete { axis, at, count }); + rewrite(cx, &name, &Edit::Delete { axis, at, count }); } cx.structural = true; Ok(Json::Null) @@ -424,7 +426,7 @@ fn move_columns(s: &mut Session, p: &Json) -> Result { sh.cells.shift_cols(before, count as i64); shift_sheet_features(sh, Axis::Cols, before, count, true); } - rewrite(&mut cx.wb, &name, &Edit::Insert { axis: Axis::Cols, at: before, count }); + rewrite(cx, &name, &Edit::Insert { axis: Axis::Cols, at: before, count }); let mut touched = Vec::new(); { @@ -462,14 +464,14 @@ fn move_columns(s: &mut Session, p: &Json) -> Result { for cell in touched { cx.touch(sheet, cell); } - rewrite(&mut cx.wb, &name, &Edit::Move { from: source, to_row: 0, to_col: destination }); + rewrite(cx, &name, &Edit::Move { from: source, to_row: 0, to_col: destination }); { let sh = cx.sheet_mut(sheet)?; sh.cells.shift_cols(delete_at + count, -(count as i64)); shift_sheet_features(sh, Axis::Cols, delete_at, count, false); } - rewrite(&mut cx.wb, &name, &Edit::Delete { axis: Axis::Cols, at: delete_at, count }); + rewrite(cx, &name, &Edit::Delete { axis: Axis::Cols, at: delete_at, count }); *cx.sel = crate::Selection { active: CellRef::new(active_row, final_start), anchor: CellRef::new(active_row, final_start), @@ -514,7 +516,7 @@ fn insert_cells(s: &mut Session, p: &Json) -> Result { } shift_cell_features(sh, &e); let name = cx.wb.sheet(sheet).map(|s| s.name.clone()).unwrap_or_default(); - rewrite(&mut cx.wb, &name, &e); + rewrite(cx, &name, &e); cx.structural = true; Ok(Json::Null) })?; @@ -549,7 +551,7 @@ fn delete_cells(s: &mut Session, p: &Json) -> Result { shift_cell_features(sh, &e); } // References into the deleted cells become #REF!; the rest move. - rewrite(&mut cx.wb, &name, &e); + rewrite(cx, &name, &e); cx.structural = true; Ok(Json::Null) })?; diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index b8ef1abe9..d47b78520 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -611,7 +611,7 @@ impl Drop for ScriptSandbox { /// thread is being torn down). fn script_sandbox_check(path: &str) -> Result<()> { if SCRIPT_SANDBOX.try_with(|c| c.get() > 0).unwrap_or(true) { - return Err(EngineError::Other(format!("{path}: scripts can't read or write files"))); + return Err(EngineError::Other(format!("{path}: scripts, and macro steps without the permission, can't read or write files"))); } Ok(()) } diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 9d4d2f93b..72dc0a152 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -209,6 +209,9 @@ pub struct DocState { /// While a macro's footprint is measured: every cell a command wrote, by sheet name, even /// where it wrote what was already there. pub touched: Option>, + /// While a macro step runs: the structural edits its command made (the sheet's name, the + /// edit), so the macro's inputs, output and Repeat ranges follow their cells as formulas do. + pub edits: Option>, /// The references `edit.goTo` with `record: true` went to in this document, newest first, /// without names, each area written with its sheet (`Sheet1!B14:C22,Sheet1!F19:G30`). pub go_to_recent: Vec, @@ -303,6 +306,7 @@ impl DocState { scripts_trusted, trust, touched: None, + edits: None, go_to_recent: vec![], } } diff --git a/crates/engine/src/macros/policy.rs b/crates/engine/src/macros/policy.rs index 8f23a5b40..a84097986 100644 --- a/crates/engine/src/macros/policy.rs +++ b/crates/engine/src/macros/policy.rs @@ -1,6 +1,8 @@ -//! What a macro step may run. Macros run commands, so this is the one place that keeps a macro -//! from saving, opening or closing files, driving the app, running other macros, or reading and -//! writing files without the person knowing. +//! What a macro step may run. Macros run commands, so this keeps a macro from saving, opening or +//! closing files, driving the app, running other macros, or reading and writing files without the +//! person knowing. It goes by command id and the `path` parameter; the runner also runs every +//! step except an approved file step inside the scripts' file sandbox, so a command that touches a +//! file under another parameter can't either. use serde_json::Value as Json; @@ -47,6 +49,11 @@ const READS_FILES: &[&str] = &["data.getData", "data.fromTextCsv", "insert.pictu /// File commands that only produce bytes unless given a `path` (then they write a file). const EXPORTS: &[&str] = &["file.exportPdf", "file.exportCsv", "file.exportHtml", "file.print", "file.printPreview", "file.saveBytes"]; +/// Whether command `id` with `params` writes a file (an export or print given a `path`). +pub fn writes_file(id: &str, params: &Json) -> bool { + EXPORTS.contains(&id) && params.get("path").is_some_and(|p| !p.is_null()) +} + /// The policy for running command `id` with `params` as a macro step in a document with `trust`. /// /// Any `path` other than `null` counts as touching a file: a token such as `{"$ref": "Input"}` diff --git a/crates/engine/src/macros/run.rs b/crates/engine/src/macros/run.rs index 9719e1e09..a6caaaafb 100644 --- a/crates/engine/src/macros/run.rs +++ b/crates/engine/src/macros/run.rs @@ -18,15 +18,16 @@ //! The steps run on a small machine with an explicit frame stack, so the debugger can stop //! between any two steps and pick up again on a later frame. -use std::collections::BTreeSet; +use std::collections::{BTreeSet, HashMap}; use std::sync::Arc; use gridcraft_core::{CellRef, RangeRef, Value}; +use gridcraft_formula::adjust::Edit; use gridcraft_model::{Cell, CompareOp, InputKind, Macro, OnConflict, RepeatOf, Sheet, Step, StepKind, Workbook}; use serde::Serialize; use serde_json::{Value as Json, json}; -use super::policy::{Policy, macro_policy, previewable}; +use super::policy::{Policy, macro_policy, previewable, writes_file}; use super::resolve::{self, Env, Place, ResolveError}; use crate::{EngineError, Result, Selection, Session, Trust, UiRequest}; @@ -37,6 +38,9 @@ pub const MAX_EXECUTED_STEPS: u64 = 100_000; pub const DEFAULT_TIME_LIMIT_MS: f64 = 60_000.0; /// The most cells an input may have for its content to be checked. pub const MAX_INPUT_CELLS: u64 = 1_000_000; +/// The most bytes of parameters one run passes to its commands, all steps together: on the web +/// build, where there is no clock, this and the step limit bound the work a run does. +pub const MAX_PARAM_BYTES: u64 = 256 * 1024 * 1024; /// The most ranges a footprint lists per sheet (the counts stay exact). const MAX_LISTED_RANGES: usize = 500; @@ -57,11 +61,22 @@ pub struct StepError { /// The step would write over data and the conflict rule is ask: overwriting answers it. #[serde(skip_serializing_if = "std::ops::Not::not")] pub confirm: bool, + /// Files the run wrote before the step failed ("step 1 (file.exportCsv)"): rolling back the + /// workbook leaves them written. + #[serde(skip_serializing_if = "Vec::is_empty")] + pub wrote_files: Vec, } impl StepError { fn new(step: &str, command: Option<&str>, message: impl Into) -> StepError { - StepError { step: step.to_string(), command: command.map(str::to_string), message: message.into(), edge: false, confirm: false } + StepError { + step: step.to_string(), + command: command.map(str::to_string), + message: message.into(), + edge: false, + confirm: false, + wrote_files: Vec::new(), + } } fn resolve(step: &str, command: Option<&str>, e: ResolveError) -> StepError { @@ -78,11 +93,30 @@ impl StepError { } pub fn into_engine(self, name: &str) -> EngineError { - let text = format!("Macro '{name}' stopped. {} Nothing was changed.", self.describe()); + let text = format!("Macro '{name}' stopped. {} {}", self.describe(), rolled_back(&self.wrote_files)); if self.confirm { EngineError::Confirm(text) } else { EngineError::Other(text) } } } +/// What a run that rolled back leaves: "Nothing was changed.", or the files it wrote before. +pub fn rolled_back(wrote_files: &[String]) -> String { + match wrote_files { + [] => "Nothing was changed.".into(), + [one] => format!("The workbook was put back; the file {one} wrote stays."), + many => format!("The workbook was put back; the files {} wrote stay.", join_and(many)), + } +} + +/// The size of `v` as JSON text, near enough to bound how much a run passes to its commands. +fn json_bytes(v: &Json) -> u64 { + match v { + Json::String(t) => t.len() as u64 + 2, + Json::Array(a) => a.iter().map(json_bytes).fold(2, u64::saturating_add), + Json::Object(o) => o.iter().map(|(k, v)| (k.len() as u64 + 3).saturating_add(json_bytes(v))).fold(2, u64::saturating_add), + _ => 8, + } +} + // ---------------------------------------------------------------- gate /// Checks every enabled step against the policy: denied commands and undeclared file access are @@ -430,6 +464,13 @@ pub struct Machine { pub partial: Option<(String, String)>, /// The last step's resolved parameters (for the debugger). pub last_params: Option, + /// Files the run wrote ("step 1 (file.exportCsv)"): rolling the run back doesn't take them back. + pub wrote_files: Vec, + /// Repeat and If bodies by the path of their frame: a body is copied the first time its step + /// runs, not on every pass. + bodies: HashMap>>, + /// Bytes of parameters passed to commands so far, against [`MAX_PARAM_BYTES`]. + param_bytes: u64, /// Set unless the macro may overwrite: each step is checked for writes over data as it runs, /// since the footprint can miss a write (it stopped early, or the run took another path). guard: Option, @@ -531,6 +572,9 @@ impl Machine { stopped: None, partial: None, last_params: None, + wrote_files: Vec::new(), + bodies: HashMap::new(), + param_bytes: 0, guard: None, }; m.settle(); @@ -587,12 +631,15 @@ impl Machine { /// Runs the current step and moves to the next one. pub fn step(&mut self, s: &mut Session) -> std::result::Result<(), StepError> { - let Some((path, st)) = self.current().map(|(p, st)| (p, st.clone())) else { return Ok(()) }; + // The frame's steps are shared, so the step (and its bodies) isn't copied to run it. + let Some((path, steps, i)) = self.frames.last().map(|f| (child_path(&f.prefix, f.next), Arc::clone(&f.steps), f.next)) else { return Ok(()) }; + let Some(st) = steps.get(i) else { return Ok(()) }; if let Some(f) = self.frames.last_mut() { f.next += 1; } - let r = self.exec(s, &path, &st); - if r.is_err() { + let mut r = self.exec(s, &path, st); + if let Err(e) = r.as_mut() { + e.wrote_files.clone_from(&self.wrote_files); self.frames.clear(); } self.settle(); @@ -631,7 +678,8 @@ impl Machine { } if items > 0 && body.iter().any(|b| b.enabled) { let repeat = Some(Loop { of: *of, over: place, at: 0, count: items }); - self.frames.push(Frame { steps: Arc::new(body.clone()), next: 0, prefix: path.to_string(), repeat }); + let steps = self.body(path.to_string(), body); + self.frames.push(Frame { steps, next: 0, prefix: path.to_string(), repeat }); } Ok(()) } @@ -647,7 +695,9 @@ impl Machine { }; let (branch, name) = if compare(&v, *op, value) { (then, "then") } else { (otherwise, "else") }; if branch.iter().any(|b| b.enabled) { - self.frames.push(Frame { steps: Arc::new(branch.clone()), next: 0, prefix: format!("{path}.{name}"), repeat: None }); + let prefix = format!("{path}.{name}"); + let steps = self.body(prefix.clone(), branch); + self.frames.push(Frame { steps, next: 0, prefix, repeat: None }); } Ok(()) } @@ -659,6 +709,39 @@ impl Machine { } } + /// After a step that inserted, deleted or moved sheets, rows, columns or cells, moves the + /// inputs, the output and the ranges of the Repeats in progress with their cells, as the + /// references in formulas move. A step that deletes one of them, or changes the size of a + /// range a Repeat is going over, stops the run. + fn follow(&mut self, before: &Workbook, after: &Workbook, edits: &[(String, Edit)]) -> std::result::Result<(), String> { + let same_sheets = before.sheets.len() == after.sheets.len() && before.sheets.iter().zip(&after.sheets).all(|(b, a)| b.name == a.name); + if same_sheets && edits.is_empty() { + return Ok(()); + } + let (pairs, _) = pair_sheets(before, after); + let follow = |what: &str, p: Place| follow_place(before, after, &pairs, edits, p).ok_or_else(|| format!("It deleted the cells of {what}.")); + for (name, place, _) in self.inputs.iter_mut() { + *place = follow(name, *place)?; + } + if let Some(o) = self.output { + self.output = Some(follow("the output anchor", o)?); + } + for f in self.frames.iter_mut() { + let Some(l) = f.repeat.as_mut() else { continue }; + let over = follow(&format!("the range Repeat {} goes over", f.prefix), l.over)?; + if (over.range.height(), over.range.width()) != (l.over.range.height(), l.over.range.width()) { + return Err(format!("It changed the size of the range Repeat {} goes over.", f.prefix)); + } + l.over = over; + } + Ok(()) + } + + /// The shared copy of the body that runs under `prefix`. + fn body(&mut self, prefix: String, body: &[Step]) -> Arc> { + Arc::clone(self.bodies.entry(prefix).or_insert_with(|| Arc::new(body.to_vec()))) + } + fn env<'a>(&'a self, wb: &'a Workbook) -> Env<'a> { Env { wb, active_sheet: wb.active_sheet, inputs: &self.inputs, output: self.output, item: self.item() } } @@ -693,10 +776,27 @@ impl Machine { self.frames.clear(); return Ok(()); } + self.param_bytes = self.param_bytes.saturating_add(json_bytes(&resolved)); + if self.param_bytes > MAX_PARAM_BYTES { + return Err(fail(format!( + "The macro passed more than {} MB of parameters to its steps and was stopped.", + MAX_PARAM_BYTES / (1024 * 1024) + ))); + } + let file_step = matches!(macro_policy(id, &resolved, self.trust), Policy::NeedsPermission(_)); + let writes = writes_file(id, &resolved); self.last_params = Some(resolved.clone()); - let prev = self.guard.as_ref().and_then(|_| s.docs.iter().find(|d| d.uid == self.doc)).map(|d| d.wb.clone()); + let prev = s.docs.iter_mut().find(|d| d.uid == self.doc).map(|d| { + d.edits = Some(Vec::new()); + d.wb.clone() + }); let queued = s.ui_requests.len(); + // Only an approved file step reaches the file system: any other step runs as a script's + // does, so a command that reads or writes a file under another parameter name can't. + let sandbox = (!file_step).then(crate::io::ScriptSandbox::enter); let r = s.execute(id, resolved); + drop(sandbox); + let edits = s.docs.iter_mut().find(|d| d.uid == self.doc).and_then(|d| d.edits.take()).unwrap_or_default(); // Macros never open dialogs or web pages, and never start editing a cell; messages for // the person stay queued. let mut blocked = None; @@ -710,11 +810,17 @@ impl Machine { } } r.map_err(|e| fail(e.to_string()))?; + if writes { + self.wrote_files.push(format!("step {path} ({id})")); + } if let Some(b) = blocked { return Err(fail(b)); } + let after = s.docs.iter().find(|d| d.uid == self.doc).map(|d| d.wb.clone()).ok_or_else(|| fail("The workbook was closed.".into()))?; + if let Some(prev) = &prev { + self.follow(prev, &after, &edits).map_err(fail)?; + } if let (Some(g), Some(prev)) = (self.guard.as_mut(), prev) { - let after = s.docs.iter().find(|d| d.uid == self.doc).map(|d| d.wb.clone()).ok_or_else(|| fail("The workbook was closed.".into()))?; let what = g.check(&prev, &after, &self.inputs); if !what.is_empty() { let msg = match &g.partial { @@ -729,6 +835,34 @@ impl Machine { } } +/// Where the cells of `p` are after a step: on the same sheet wherever it moved (`pairs`, from +/// [`pair_sheets`]), and moved by each structural edit the step made there, or `None` when the +/// step deleted them (or their sheet). +fn follow_place(before: &Workbook, after: &Workbook, pairs: &[(usize, Option)], edits: &[(String, Edit)], p: Place) -> Option { + let sheet = pairs.iter().find(|(b, _)| *b == p.sheet).and_then(|(_, a)| *a)?; + let mut name = after.sheet(sheet)?.name.clone(); + let old_name = before.sheet(p.sheet).map(|s| s.name.as_str()).unwrap_or_default(); + let mut text = format!("{}!{}", resolve::quote_sheet(&name), p.range.a1()); + for (target, e) in edits { + if target != &name && target != old_name { + continue; + } + let expr = gridcraft_formula::parse(&text).ok()?; + text = gridcraft_formula::print(&gridcraft_formula::adjust::adjust(expr, target, target, e)); + if text.contains("#REF!") { + return None; + } + // A cut and paste can take the cells to another sheet. + let env = Env { wb: after, active_sheet: usize::MAX, inputs: &[], output: None, item: None }; + name = after.sheet(resolve::resolve_ref(&text, &env).ok()?.sheet)?.name.clone(); + } + let env = Env { wb: after, active_sheet: usize::MAX, inputs: &[], output: None, item: None }; + if text.contains('$') { + text = text.replace('$', ""); + } + resolve::resolve_ref(&text, &env).ok() +} + /// If's comparison of a cell's value with a step's `value`, the way Excel's `=`, `<`… compare. fn compare(v: &Value, op: CompareOp, want: &Json) -> bool { let empty = v.is_empty() || v.as_text() == Some(""); @@ -1160,6 +1294,7 @@ pub fn begin(s: &mut Session, name: &str, p: &Json) -> Result { .and_then(|i| d.wb.macros.get(i)) .cloned() .ok_or_else(|| EngineError::Other(format!("There is no macro called '{name}'.")))?; + check_params(p, &m)?; let asks = gate(&m, trust)?; approve(s, &m, doc, &asks, p.get("approve").and_then(Json::as_bool).unwrap_or(false))?; let prepared = prepare(s, &m, p.get("inputs"))?; @@ -1238,7 +1373,7 @@ pub fn run(s: &mut Session, name: &str, p: &Json) -> Result { let before_run = machine.executed; machine.run_to_end(s).map_err(|e| e.into_engine(name))?; match machine.stopped { - Some((msg, true)) => Err(EngineError::Other(format!("Macro '{name}' stopped: {msg} Nothing was changed."))), + Some((msg, true)) => Err(EngineError::Other(format!("Macro '{name}' stopped: {msg} {}", rolled_back(&machine.wrote_files)))), stopped => { let mut out = json!({"macro": name, "ran": machine.executed.saturating_sub(before_run), "inputs": b.inputs}); if let Some((msg, _)) = stopped { @@ -1253,6 +1388,47 @@ pub fn run(s: &mut Session, name: &str, p: &Json) -> Result { }) } +/// Refuses run parameters that would otherwise be ignored: an input the macro doesn't have, an +/// input given as something other than reference text, and `onConflict`, `overwrite`, `approve` +/// or `timeLimit` of the wrong kind (`null` counts as not given). +pub fn check_params(p: &Json, m: &Macro) -> Result<()> { + let refuse = |msg: String| Err(EngineError::Other(format!("Macro '{}': {msg}", m.name))); + let names = || m.inputs.iter().map(|i| i.name.clone()).collect::>().join(", "); + match p.get("inputs") { + None | Some(Json::Null) => {} + Some(Json::String(_)) if !m.inputs.is_empty() => {} + Some(Json::String(_)) => return refuse("it takes no inputs.".into()), + Some(Json::Object(o)) => { + for (k, v) in o { + if !m.inputs.iter().any(|i| i.name.eq_ignore_ascii_case(k)) { + return refuse(if m.inputs.is_empty() { + format!("it has no input called '{k}'; it takes no inputs.") + } else { + format!("it has no input called '{k}' (its inputs: {}).", names()) + }); + } + if !v.is_string() { + return refuse(format!("give the input {k} as reference text, such as \"B2:D9\".")); + } + } + } + Some(_) => return refuse("give `inputs` as reference text, such as \"B2:D9\", or as {\"Input\": \"B2:D9\"}.".into()), + } + let given = |k: &str| p.get(k).filter(|v| !v.is_null()); + if given("onConflict").is_some_and(|v| !v.is_string()) { + return refuse("`onConflict` must be ask, fail, overwrite, shiftDown or shiftRight.".into()); + } + for k in ["overwrite", "approve"] { + if given(k).is_some_and(|v| !v.is_boolean()) { + return refuse(format!("`{k}` must be true or false.")); + } + } + if given("timeLimit").is_some_and(|v| !v.as_f64().is_some_and(|t| t > 0.0)) { + return refuse("`timeLimit` must be a number of seconds above 0.".into()); + } + Ok(()) +} + /// The run's `timeLimit` (seconds) in milliseconds, or the default. fn time_limit(p: &Json) -> f64 { p.get("timeLimit").and_then(Json::as_f64).map(|t| (t * 1000.0).clamp(1.0, 600_000.0)).unwrap_or(DEFAULT_TIME_LIMIT_MS) diff --git a/crates/engine/src/tests/macros.rs b/crates/engine/src/tests/macros.rs index 94902ee7b..a9d6c6179 100644 --- a/crates/engine/src/tests/macros.rs +++ b/crates/engine/src/tests/macros.rs @@ -3,6 +3,7 @@ use std::collections::BTreeSet; use std::sync::Arc; +use gridcraft_core::CellRef; use serde_json::{Value as Json, json}; use super::{s, v}; @@ -181,6 +182,58 @@ fn conflicts_ask_fail_or_overwrite() { assert_eq!(v(&s, "H1").display(), "400"); } +/// The value of `cell` on the sheet called `sheet`. +fn on(s: &Session, sheet: &str, cell: &str) -> String { + let wb = &s.doc().unwrap().wb; + wb.sheet(wb.sheet_index(sheet).unwrap()).unwrap().value(CellRef::parse(cell).unwrap()).display() +} + +#[test] +fn inputs_and_the_output_follow_their_cells_when_a_step_moves_them() { + // A sheet inserted in front: the input is still on Sheet1, not on whatever sheet is first now. + let mut s = s(); + let steps = json!([ + {"command": "home.insertSheet", "params": {"name": "Summary", "before": 0}}, + {"command": "cell.set", "params": {"cell": "Input", "input": "x"}} + ]); + put(&mut s, json!({"name": "Front", "inputs": [{"name": "Input", "kind": "cell"}], "output": {"onConflict": "overwrite"}, "steps": steps})); + s.execute("macro.run", json!({"name": "Front", "inputs": "Sheet1!B2"})).unwrap(); + assert_eq!((on(&s, "Sheet1", "B2"), on(&s, "Summary", "B2")), ("x".into(), "".into())); + + // Rows inserted above: the input and the output move down with their cells. + let mut s = super::s(); + set(&mut s, "B5", "in"); + let steps = json!([ + {"command": "home.insertRows", "params": {"rows": "2:3"}}, + {"command": "cell.set", "params": {"cell": "Input", "input": "x"}}, + {"command": "cell.set", "params": {"cell": "Output", "input": "y"}} + ]); + put( + &mut s, + json!({"name": "Down", "inputs": [{"name": "Input", "kind": "cell"}], "output": {"anchor": "D5", "onConflict": "overwrite"}, "steps": steps}), + ); + s.execute("macro.run", json!({"name": "Down", "inputs": "B5"})).unwrap(); + assert_eq!((v(&s, "B7").display(), v(&s, "D7").display()), ("x".into(), "y".into())); + assert_eq!((v(&s, "B5").display(), v(&s, "D5").display()), ("".into(), "".into())); + + // A step that deletes an input's cells stops the run. + let mut s = super::s(); + set(&mut s, "B5", "in"); + let steps = json!([{"command": "home.deleteRows", "params": {"rows": "5"}}, {"command": "cell.set", "params": {"cell": "Input", "input": "x"}}]); + put(&mut s, json!({"name": "Gone", "inputs": [{"name": "Input", "kind": "cell"}], "output": {"onConflict": "overwrite"}, "steps": steps})); + let start = s.doc().unwrap().wb.clone(); + let err = s.execute("macro.run", json!({"name": "Gone", "inputs": "B5"})).unwrap_err().to_string(); + assert!(err.contains("Step 1 (home.deleteRows): It deleted the cells of Input."), "{err}"); + assert!(Arc::ptr_eq(&s.doc().unwrap().wb, &start)); + + // A step inside a Repeat that changes the size of the range it goes over stops the run. + let mut s = super::s(); + let body = json!([{"command": "home.insertRows", "params": {"rows": "2"}}]); + put(&mut s, json!({"name": "Grow", "output": {"onConflict": "overwrite"}, "steps": [{"type": "repeat", "over": "A1:A3", "body": body}]})); + let err = s.execute("macro.run", json!({"name": "Grow"})).unwrap_err().to_string(); + assert!(err.contains("Step 1.1 (home.insertRows): It changed the size of the range Repeat 1 goes over."), "{err}"); +} + #[test] fn shifting_makes_room_and_is_checked_again() { for (rule, moved) in [("shiftDown", ["E4", "E5"]), ("shiftRight", ["F2", "F3"])] { @@ -382,6 +435,10 @@ fn each_step_is_checked_for_writes_over_data() { (_, r) => panic!("{first} {rule}: {r:?}"), }; assert!(e.contains("Step 2 (cell.set): It would write over data (B1 on Sheet1)"), "{e}"); + // The workbook is put back; a file the run wrote stays, and the message says so. + let left = + if first == &export { "The workbook was put back; the file step 1 (file.exportCsv) wrote stays." } else { "Nothing was changed." }; + assert!(e.ends_with(left), "{e}"); assert_eq!(v(&s, "B1").display(), "keep", "{first} {rule}"); assert!(Arc::ptr_eq(&s.doc().unwrap().wb, &start), "{first} {rule}: rolled back"); } @@ -425,6 +482,46 @@ fn a_run_that_takes_another_path_than_its_check_is_still_checked() { assert_eq!(outcomes.len(), 2, "both paths were taken: {outcomes:?}"); } +#[test] +fn run_parameters_that_would_be_ignored_are_refused() { + let mut s = s(); + data(&mut s); + put(&mut s, totals()); + for (p, says) in [ + (json!({"inputs": {"Inptu": "B2:C5"}}), "it has no input called 'Inptu' (its inputs: Input)."), + (json!({"inputs": {"Input": 5}}), "give the input Input as reference text"), + (json!({"inputs": ["B2:C5"]}), "give `inputs` as reference text"), + (json!({"inputs": "B2:C5", "onConflict": 1}), "`onConflict` must be"), + (json!({"inputs": "B2:C5", "overwrite": "yes"}), "`overwrite` must be true or false."), + (json!({"inputs": "B2:C5", "approve": 1}), "`approve` must be true or false."), + (json!({"inputs": "B2:C5", "timeLimit": "30"}), "`timeLimit` must be a number of seconds above 0."), + (json!({"inputs": "B2:C5", "timeLimit": 0}), "`timeLimit` must be a number of seconds above 0."), + ] { + let mut p = p; + p["name"] = json!("Totals"); + let err = s.execute("macro.run", p.clone()).unwrap_err().to_string(); + assert!(err.contains(&format!("Macro 'Totals': {says}")), "{p}: {err}"); + } + put(&mut s, json!({"name": "None", "steps": []})); + let err = s.execute("macro.run", json!({"name": "None", "inputs": "A1"})).unwrap_err().to_string(); + assert!(err.contains("Macro 'None': it takes no inputs."), "{err}"); + // `null` is the same as not given, and an input's name matches in any case. + let p = json!({"name": "Totals", "inputs": {"input": "B2:C5"}, "onConflict": null, "overwrite": true, "timeLimit": null}); + s.execute("macro.run", p).unwrap(); +} + +#[test] +fn a_run_passes_a_bounded_amount_of_parameters() { + // A step with a 3 MB parameter in a Repeat of 100 rows would hand its command 300 MB; the run + // stops at 256 MB, which matters on the web build, where there is no time limit. + let mut s = s(); + let big = "x".repeat(3_000_000); + let body = json!([{"command": "home.bold", "params": {"range": "A1", "note": big}}]); + put(&mut s, json!({"name": "Big", "output": {"onConflict": "overwrite"}, "steps": [{"type": "repeat", "over": "A1:A100", "body": body}]})); + let err = s.execute("macro.run", json!({"name": "Big"})).unwrap_err().to_string(); + assert!(err.contains("passed more than 256 MB of parameters to its steps"), "{err}"); +} + #[test] fn the_answer_to_the_question_counts_however_ask_was_given() { let mut s = s(); diff --git a/docs/macros.md b/docs/macros.md index fbda2f69a..994006c85 100644 --- a/docs/macros.md +++ b/docs/macros.md @@ -59,9 +59,10 @@ no macro debugger, recorder or management commands in this version, and no macro whole macro (4,194,304 characters of JSON). A macro read from a file that breaks any of these is left out with a warning; it is never changed to fit. When a macro runs, a Repeat whose range has more rows or columns than its `max` is refused, and a run executes at most 100,000 steps (each - pass through a Repeat counts) and takes at most 60 seconds, both shared by the footprint dry - runs and the run itself (`timeLimit` to change the time; the web build has no clock, so there - only the step limit applies). + pass through a Repeat counts), passes its commands at most 256 MB of parameters, all steps + together, and takes at most 60 seconds, all three shared by the footprint dry runs and the run + itself (`timeLimit` to change the time; the web build has no clock, so there only the step and + parameter limits apply). ## Tokens @@ -107,23 +108,32 @@ refused rather than read as the header. including a blank one and a token, and the policy is checked again on the parameters as they are when the step runs. A macro that touches files, or that came with a workbook opened from a file, runs once the person approves it: the call fails with `approve: …` and runs with - `approve: true`. Approval lasts for the session, until the macro changes. + `approve: true`. Approval lasts for the session, until the macro changes. Every step other + than an approved file step runs inside the file sandbox that scripts use, so no other + parameter reaches a file either. 2. **Footprint.** Unless the conflict rule is `overwrite`, the macro first runs in dry-run mode on - the real workbook, and everything is put back. Cells it would write outside its - inputs that hold data, and sheets and objects it would - delete, are conflicts, handled by `onConflict` (or the macro's `output.onConflict`): `ask` (the - call fails with `confirm: …`; `overwrite: true` answers it), `fail`, `overwrite`, `shiftDown` or - `shiftRight` (Insert Cells makes room, then the footprint is checked again). Steps that touch - files, and Solver, end the dry run early; the footprint then says it is partial. Unless the - rule is `overwrite`, the run also checks every step as it goes, since the footprint can miss a - write: it is partial, or a value such as RAND or NOW sends the run another way than the dry - run. A step that writes over cells, - sheets or objects that held data when the run started stops the run and rolls the workbook - back (a file already written stays): it asks (`confirm: …`) under `ask` and fails otherwise, - since shifting can't make room that late. + the real workbook, and everything is put back. Cells it would write outside its inputs that + hold data, and sheets and objects it would delete, are conflicts, handled by `onConflict` (or + the macro's `output.onConflict`): `ask` (the call fails with `confirm: …`; `overwrite: true` + answers it), `fail`, `overwrite`, `shiftDown` or `shiftRight` (Insert Cells makes room, then + the footprint is checked again). Steps that touch files, and Solver, end the dry run early; + the footprint then says it is partial. Unless the rule is `overwrite`, the run also checks + every step as it goes, since the footprint can miss a write: it is partial, or a value such as + RAND or NOW sends the run another way than the dry run. A step that writes over cells, sheets + or objects that held data when the run started stops the run and rolls the workbook back (a + file already written stays): it asks (`confirm: …`) under `ask` and fails otherwise, since + shifting can't make room that late. Moving cells that hold data, as inserting or deleting rows + or cells above them does, counts as writing over them, in the footprint and in this check: + both compare cells by place. 3. **Run.** One undo step, "Run Macro: Totals". Any error rolls the whole run back and names the step: `Step 3.1 (cell.set): …` (paths are 1-based; `2.1` is inside a Repeat, `4.then.1` inside - an If). + an If). The message ends "Nothing was changed.", or names the files the run already wrote, + which stay. An input the macro doesn't have, an input that isn't reference text, and + `onConflict`, `overwrite`, `approve` or `timeLimit` of the wrong kind are refused rather than + ignored. When a step inserts, deletes or moves sheets, rows, columns or cells, the inputs, the + output and the range of a Repeat in progress move with their cells, as references in formulas + do; a step that deletes one of them, or changes the size of a range a Repeat is going over, + stops the run. `macro.check {name, inputs?, timeLimit?}` reports the bound inputs, the output anchor, any approval needed and the footprint, without changing anything.