From 5392d95bf85d53eab9abedf78a8757e7d292d9ad Mon Sep 17 00:00:00 2001 From: Higgs32584 Date: Wed, 21 Oct 2026 21:13:13 -0400 Subject: [PATCH 1/2] scripts: require workbook trust before running embedded scripts (#2178) A workbook opened with GridCraft scripts (`_sc_scripts`) starts untrusted: automate.runScript refuses them until the user trusts the workbook for the session (Macros dialog > Trust Scripts, i.e. automate.trustScripts) or the caller passes `trusted: true` for that one run. Workbooks without scripts, and scripts written in the session, are unaffected. Whatever the trust, a script whose steps reach outside the workbook (file.*, link.*, or any `path` param: Get Data, From Text/CSV, Insert Picture) is refused before any step runs. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/extra.rs | 75 ++++++++++++++++++++++++++--- crates/engine/src/lib.rs | 7 +++ crates/engine/src/tests.rs | 87 ++++++++++++++++++++++++++++++++++ crates/ui-egui/src/dialogs.rs | 15 +++++- 4 files changed, 176 insertions(+), 8 deletions(-) diff --git a/crates/engine/src/cmd/extra.rs b/crates/engine/src/cmd/extra.rs index 01e2ac6e6..b88ce5592 100644 --- a/crates/engine/src/cmd/extra.rs +++ b/crates/engine/src/cmd/extra.rs @@ -193,7 +193,24 @@ pub fn specs() -> Vec { new_script ), cmd!(query "automate.allScripts", "All Scripts", ["Automate", "Scripting Tools"], None, "{} → saved scripts", has_doc, all_scripts), - cmd!("automate.runScript", "Run Script", [], None, "{name}", has_doc, run_script), + cmd!( + "automate.runScript", + "Run Script", + [], + None, + "{name, trusted?: bool (runs a script from an untrusted workbook this once)} — refuses scripts in a workbook opened from a file until it is trusted, and steps that reach outside the workbook (file.*, link.*, any `path` param)", + has_doc, + run_script + ), + cmd!( + noundo "automate.trustScripts", + "Trust Scripts", + ["Automate", "Scripting Tools"], + None, + "{trusted?: bool (default true)} → {trusted}: lets this workbook's scripts run for the rest of the session", + has_doc, + trust_scripts + ), cmd!("automate.deleteScript", "Delete Script", [], None, "{name}", has_doc, delete_script), // Cell controls, themes, borders, styles cmd!( @@ -782,6 +799,7 @@ fn new_window(s: &mut Session, _: &Json) -> Result { let d = s.doc()?.clone(); let mut copy = crate::DocState::new((*d.wb).clone(), d.path.clone(), format!("{}:2", d.display_title())); copy.selection = d.selection.clone(); + copy.scripts_trusted = d.scripts_trusted; let i = s.add_document(copy); Ok(json!({"index": i})) } @@ -970,10 +988,13 @@ fn new_script(s: &mut Session, p: &Json) -> Result { } fn all_scripts(s: &mut Session, _: &Json) -> Result { - let wb = &s.doc()?.wb; + let d = s.doc()?; + let (wb, trusted) = (&d.wb, d.scripts_trusted); let all = stash_get(wb, SCRIPTS_NAME); - let mut list: Vec = - all.as_object().map(|o| o.iter().map(|(k, v)| json!({"name": k, "steps": v.as_array().map_or(0, Vec::len)})).collect()).unwrap_or_default(); + let mut list: Vec = all + .as_object() + .map(|o| o.iter().map(|(k, v)| json!({"name": k, "steps": v.as_array().map_or(0, Vec::len), "trusted": trusted})).collect()) + .unwrap_or_default(); // A preserved VBA project is reported alongside GridCraft's own scripts; it is carried on // save but never executed here, so it has no step count. if let Some(bytes) = &wb.vba_project { @@ -982,12 +1003,54 @@ fn all_scripts(s: &mut Session, _: &Json) -> Result { Ok(Json::Array(list)) } +/// Whether `wb` carries GridCraft scripts (a workbook opened with them starts untrusted). +pub(crate) fn has_scripts(wb: &Workbook) -> bool { + stash_map(wb, SCRIPTS_NAME).as_object().is_some_and(|o| !o.is_empty()) +} + +/// Why a script step is refused, whatever the workbook's trust: scripts stay inside the +/// workbook. Anything that reads or writes files (`file.*`, or any `path` parameter: Get Data, +/// From Text/CSV, Insert Picture) or leaves the app (`link.*` opens URLs) is the caller's to run +/// directly, not a stored script's. (Nested `automate.*` steps are skipped.) +fn denied_script_step(id: &str, params: &Json) -> Option { + if id.starts_with("file.") || id.starts_with("link.") { + return Some(format!("scripts can't run `{id}`: it reaches outside the workbook")); + } + if params.get("path").is_some() { + return Some(format!("scripts can't pass a file `path` to `{id}`")); + } + None +} + +fn trust_scripts(s: &mut Session, p: &Json) -> Result { + let trusted = bool_param(p, "trusted").unwrap_or(true); + s.doc_mut()?.scripts_trusted = trusted; + Ok(json!({"trusted": trusted})) +} + +/// Runs a stored script. Scripts in a workbook opened from a file run only once the workbook is +/// trusted for this session (`automate.trustScripts`, or `trusted: true` on this call); steps that +/// reach outside the workbook ([`denied_script_step`]) are refused before any step runs. fn run_script(s: &mut Session, p: &Json) -> Result { let name = str_param(p, "name").ok_or_else(|| bad("automate.runScript", "missing `name`"))?; - let all = stash_get(&s.doc()?.wb, SCRIPTS_NAME); + let d = s.doc()?; + let all = stash_get(&d.wb, SCRIPTS_NAME); let steps = all.get(name).and_then(Json::as_array).cloned().ok_or_else(|| bad("automate.runScript", "no such script"))?; + if !d.scripts_trusted && !bool_param(p, "trusted").unwrap_or(false) { + return Err(bad( + "automate.runScript", + "scripts in this workbook came from a file and are disabled; trust the workbook first (Automate › Macros › Trust Scripts, or `trusted: true`)", + )); + } + let steps: Vec = steps.into_iter().take(100_000).collect(); + for st in &steps { + let id = st["command"].as_str().unwrap_or(""); + if let Some(why) = denied_script_step(id, &st["params"]) { + return Err(bad("automate.runScript", why)); + } + } let mut n = 0; - for st in steps.iter().take(100_000) { + for st in &steps { let id = st["command"].as_str().unwrap_or(""); if id.starts_with("automate.") { continue; diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 258a1cb72..bf7dd633d 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -112,6 +112,11 @@ pub struct DocState { /// The format `path`'s content was recognised as when its extension doesn't name one (an /// XLSX called `budget.dat`): Save writes it back in that format. pub content_kind: Option, + /// Whether the scripts stored in this workbook may run (`automate.runScript`). A workbook + /// that arrives already carrying scripts (opened from a file) starts untrusted until the + /// user trusts it in this session (`automate.trustScripts`); one with no scripts starts + /// trusted, since any script it gains later was written here. + pub scripts_trusted: bool, } static NEXT_UID: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(1); @@ -121,6 +126,7 @@ impl DocState { pub fn new(mut wb: Workbook, path: Option, title: String) -> DocState { let mut calc = Calc::new(); calc.recalc_all(&mut wb); + let scripts_trusted = !cmd::extra::has_scripts(&wb); let wb = Arc::new(wb); let sel = Selection::at(wb.active().map(|s| s.view_active).unwrap_or_default()); DocState { @@ -137,6 +143,7 @@ impl DocState { sheet_selections: vec![], unlocked_ranges: vec![], content_kind: None, + scripts_trusted, } } pub fn is_dirty(&self) -> bool { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 2ac290654..357b925c1 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -940,6 +940,93 @@ fn protection_passwords_stay_out_of_the_journal_and_recorded_scripts() { s.execute("automate.runScript", json!({"name": "Lock"})).unwrap(); } +/// A session holding a workbook opened from file bytes whose `Fill` script sets B1 (#2178). +fn opened_with_script() -> Session { + let mut author = s(); + author + .execute("automate.newScript", json!({"name": "Fill", "commands": [{"command": "cell.set", "params": {"cell": "B1", "input": "ran"}}]})) + .unwrap(); + let bytes = crate::io::save_bytes(&author.doc().unwrap().wb, "book.xlsx").unwrap(); + let mut s = Session::new(); + s.execute("file.open", json!({"name": "downloaded.xlsx", "base64": crate::io::base64_encode(&bytes)})).unwrap(); + s +} + +#[test] +fn scripts_written_in_this_session_run_without_a_trust_step() { + let mut s = s(); + assert!(s.doc().unwrap().scripts_trusted); + s.execute("automate.newScript", json!({"name": "Fill", "commands": [{"command": "cell.set", "params": {"cell": "B1", "input": "ran"}}]})) + .unwrap(); + assert_eq!(s.execute("automate.runScript", json!({"name": "Fill"})).unwrap()["ran"], 1); + assert_eq!(v(&s, "B1"), Value::Text("ran".into())); +} + +#[test] +fn untrusted_workbook_scripts_are_refused_until_trusted() { + let mut s = opened_with_script(); + s.ui_requests.clear(); + assert!(!s.doc().unwrap().scripts_trusted, "a workbook opened with scripts starts untrusted"); + let listed = s.execute("automate.allScripts", json!({})).unwrap(); + assert_eq!(listed[0]["trusted"], false, "{listed}"); + let err = s.execute("automate.runScript", json!({"name": "Fill"})).unwrap_err().to_string(); + assert!(err.contains("trust"), "{err}"); + assert_eq!(v(&s, "B1"), Value::Empty, "no step of an untrusted script runs"); + // Trust can't be smuggled in as a string, and nothing opens a dialog. + assert!(s.execute("automate.runScript", json!({"name": "Fill", "trusted": "yes"})).is_err()); + assert!(s.ui_requests.is_empty(), "{:?}", s.ui_requests); + + // An explicit `trusted: true` runs it this once without trusting the workbook. + s.execute("automate.runScript", json!({"name": "Fill", "trusted": true})).unwrap(); + assert_eq!(v(&s, "B1"), Value::Text("ran".into())); + assert!(!s.doc().unwrap().scripts_trusted); + s.execute("cell.set", json!({"cell": "B1", "input": ""})).unwrap(); + assert!(s.execute("automate.runScript", json!({"name": "Fill"})).is_err()); + + // Trusting the workbook (the UI's Trust Scripts action) lets its scripts run this session. + assert_eq!(s.execute("automate.trustScripts", json!({})).unwrap()["trusted"], true); + s.execute("automate.runScript", json!({"name": "Fill"})).unwrap(); + assert_eq!(v(&s, "B1"), Value::Text("ran".into())); + // A second window on the same workbook keeps the decision; withdrawing it disables them again. + s.execute("view.newWindow", json!({})).unwrap(); + assert!(s.doc().unwrap().scripts_trusted); + s.execute("automate.trustScripts", json!({"trusted": false})).unwrap(); + assert!(s.execute("automate.runScript", json!({"name": "Fill"})).is_err()); +} + +#[test] +fn scripts_cannot_reach_outside_the_workbook_even_when_trusted() { + let dir = std::env::temp_dir().join(format!("gridcraft-script-deny-{}", std::process::id())); + let leak = dir.join("leak.xlsx"); + let leak = leak.to_string_lossy().to_string(); + let mut s = opened_with_script(); + s.ui_requests.clear(); + s.execute("automate.trustScripts", json!({})).unwrap(); + let denied = [ + json!({"command": "link.open", "params": {"url": "https://example.com/?d=secret"}}), + json!({"command": "file.saveAs", "params": {"path": leak}}), + json!({"command": "file.save", "params": {}}), + json!({"command": "file.exportPdf", "params": {"path": leak}}), + json!({"command": "file.open", "params": {"path": leak}}), + json!({"command": "data.getData", "params": {"path": leak}}), + json!({"command": "data.fromTextCsv", "params": {"path": leak}}), + json!({"command": "insert.picture", "params": {"path": leak}}), + ]; + for step in denied { + // The harmless first step proves the whole script is refused before anything runs. + let first = json!({"command": "cell.set", "params": {"cell": "C1", "input": "partial"}}); + s.execute("automate.newScript", json!({"name": "Leak", "commands": [first, step]})).unwrap(); + let r = s.execute("automate.runScript", json!({"name": "Leak", "trusted": true})); + assert!(r.is_err(), "{step} must be refused"); + assert_eq!(v(&s, "C1"), Value::Empty, "{step}: no step runs"); + } + assert!(!s.ui_requests.iter().any(|r| matches!(r, crate::UiRequest::OpenUrl(_))), "{:?}", s.ui_requests); + assert!(!std::path::Path::new(&leak).exists()); + // Data a script supplies itself is still fine. + s.execute("automate.newScript", json!({"name": "Csv", "commands": [{"command": "data.fromTextCsv", "params": {"text": "a,b\n1,2"}}]})).unwrap(); + s.execute("automate.runScript", json!({"name": "Csv"})).unwrap(); +} + /// A sheet with locked data in A1:B3, unlocked cells in D1:D3 and row 10, then protected. fn protected(extra: serde_json::Value) -> Session { let mut s = s(); diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index fb899c702..10320476c 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -1077,7 +1077,7 @@ pub fn show(app: &mut SheetApp, ctx: &egui::Context) { } }); } - "macros" => macros(ui, &mut d), + "macros" => macros(app, ui, &mut d), "statistics" | "accessibility" | "errorChecking" | "evaluateFormula" | "comments" => { let r = if d.name == "comments" { app.session.active().and_then(|doc| doc.wb.active().map(|s| json!(s.comments.iter().map(|(c, m)| json!({"cell": c.a1(), "author": m.author, "text": m.text})).collect::>()))) @@ -1496,7 +1496,7 @@ fn about(ui: &mut egui::Ui, d: &mut Dialog) { /// The Macros dialog: lists GridCraft's own recorded scripts and (if the file carries one) the /// preserved VBA project. Mirrors Excel's Developer ▸ Macros list, but is honest that VBA is /// preserved and not executed. -fn macros(ui: &mut egui::Ui, d: &mut Dialog) { +fn macros(app: &mut SheetApp, ui: &mut egui::Ui, d: &mut Dialog) { ui.label("Automation in this workbook. GridCraft runs its own scripts; a file's VBA project is preserved but not executed."); ui.add_space(6.0); let items: Vec = d.result.as_ref().and_then(Json::as_array).cloned().unwrap_or_default(); @@ -1504,6 +1504,17 @@ fn macros(ui: &mut egui::Ui, d: &mut Dialog) { ui.weak("This workbook has no scripts or macros."); return; } + // Scripts that came with a file stay disabled until the user trusts the workbook (#2178). + if items.iter().any(|it| it.get("trusted").and_then(Json::as_bool) == Some(false)) { + ui.horizontal(|ui| { + ui.weak("Scripts in this workbook came from a file and are disabled."); + if ui.button("Trust Scripts").on_hover_text("Let this workbook's scripts run for the rest of this session").clicked() { + let _ = app.run("automate.trustScripts", json!({"trusted": true})); + d.result = app.session.run("view.macros", json!({})).ok(); + } + }); + ui.add_space(6.0); + } egui::ScrollArea::vertical().max_height(300.0).show(ui, |ui| { for it in &items { let name = it.get("name").and_then(Json::as_str).unwrap_or("(unnamed)"); From b5b3969377b125cd782768ace8cfbaee06f57bc9 Mon Sep 17 00:00:00 2001 From: Higgs32584 Date: Thu, 22 Oct 2026 12:43:48 -0400 Subject: [PATCH 2/2] scripts: trust gate review fixes (#2178) - automate.trustScripts: only a missing `trusted` defaults to true; a non-boolean value is an error and leaves the workbook untrusted. - The refusal message points at Trust Scripts in View > Macros (Alt+F8) and at automate.trustScripts. - Catalog row for automate.trustScripts; parity checklist regenerated. - Script steps run inside an io::ScriptSandbox (thread-local) so read_file/write_file refuse whatever the parameter is called, and a step that raises OpenUrl is dropped and stops the script. The id/param deny list stays as the up-front refusal. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/catalog.rs | 1 + crates/engine/src/cmd/extra.rs | 28 ++++++++++++--- crates/engine/src/io.rs | 62 ++++++++++++++++++++++++++++++++++ crates/engine/src/tests.rs | 38 +++++++++++++++++++++ docs/parity-checklist.md | 7 ++-- 5 files changed, 129 insertions(+), 7 deletions(-) diff --git a/crates/engine/src/catalog.rs b/crates/engine/src/catalog.rs index d6b67e1b4..007cbf4bc 100644 --- a/crates/engine/src/catalog.rs +++ b/crates/engine/src/catalog.rs @@ -273,6 +273,7 @@ pub const CATALOG: &[(&str, &str, &str, &str)] = &[ ("automate.newScript", "Automate", "Scripting Tools", "New Script"), ("automate.allScripts", "Automate", "Scripting Tools", "All Scripts"), ("automate.recordActions", "Automate", "Scripting Tools", "Record Actions"), + ("automate.trustScripts", "Automate", "Scripting Tools", "Trust Scripts"), // Table Design ("table.rename", "Table Design", "Properties", "Table Name"), ("table.resize", "Table Design", "Properties", "Resize Table"), diff --git a/crates/engine/src/cmd/extra.rs b/crates/engine/src/cmd/extra.rs index b2d8c352d..f248c59f2 100644 --- a/crates/engine/src/cmd/extra.rs +++ b/crates/engine/src/cmd/extra.rs @@ -1017,7 +1017,10 @@ pub(crate) fn has_scripts(wb: &Workbook) -> bool { /// Why a script step is refused, whatever the workbook's trust: scripts stay inside the /// workbook. Anything that reads or writes files (`file.*`, or any `path` parameter: Get Data, /// From Text/CSV, Insert Picture) or leaves the app (`link.*` opens URLs) is the caller's to run -/// directly, not a stored script's. (Nested `automate.*` steps are skipped.) +/// directly, not a stored script's. (Nested `automate.*` steps are skipped.) This check goes by +/// command id and parameter name, so it is only the up-front refusal: the steps also run inside +/// an [`crate::io::ScriptSandbox`] (file access refused whatever the parameter is called) and +/// [`run_script`] drops any `OpenUrl` request a step raises. fn denied_script_step(id: &str, params: &Json) -> Option { if id.starts_with("file.") || id.starts_with("link.") { return Some(format!("scripts can't run `{id}`: it reaches outside the workbook")); @@ -1029,7 +1032,12 @@ fn denied_script_step(id: &str, params: &Json) -> Option { } fn trust_scripts(s: &mut Session, p: &Json) -> Result { - let trusted = bool_param(p, "trusted").unwrap_or(true); + // Only a missing `trusted` means "trust"; `"false"`, `0` or `null` are mistakes, not consent. + let trusted = match p.get("trusted") { + None => true, + Some(Json::Bool(b)) => *b, + Some(_) => return Err(bad("automate.trustScripts", "`trusted` must be true or false")), + }; s.doc_mut()?.scripts_trusted = trusted; Ok(json!({"trusted": trusted})) } @@ -1045,7 +1053,7 @@ fn run_script(s: &mut Session, p: &Json) -> Result { if !d.scripts_trusted && !bool_param(p, "trusted").unwrap_or(false) { return Err(bad( "automate.runScript", - "scripts in this workbook came from a file and are disabled; trust the workbook first (Automate › Macros › Trust Scripts, or `trusted: true`)", + "scripts in this workbook came from a file and are disabled; trust the workbook first (Trust Scripts in View › Macros, Alt+F8; `automate.trustScripts`; or `trusted: true`)", )); } let steps: Vec = steps.into_iter().take(100_000).collect(); @@ -1055,13 +1063,25 @@ fn run_script(s: &mut Session, p: &Json) -> Result { return Err(bad("automate.runScript", why)); } } + let _sandbox = crate::io::ScriptSandbox::enter(); let mut n = 0; for st in &steps { let id = st["command"].as_str().unwrap_or(""); if id.starts_with("automate.") { continue; } - s.execute(id, st["params"].clone())?; + let before = s.ui_requests.len(); + let ran = s.execute(id, st["params"].clone()); + // A step that asks the host to open a URL leaves the app: drop the request and stop. + if s.ui_requests.get(before..).is_some_and(|new| new.iter().any(|r| matches!(r, crate::UiRequest::OpenUrl(_)))) { + let mut i = 0; + s.ui_requests.retain(|r| { + i += 1; + i <= before || !matches!(r, crate::UiRequest::OpenUrl(_)) + }); + return Err(bad("automate.runScript", format!("scripts can't open links (`{id}`)"))); + } + ran?; n += 1; } Ok(json!({"ran": n})) diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 5c0bf800b..b18e8577b 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -556,8 +556,41 @@ pub fn to_html(wb: &Workbook, sheet: usize) -> Result { Ok(out) } +thread_local! { + /// How many [`ScriptSandbox`]es are alive on this thread. + static SCRIPT_SANDBOX: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +/// While one is alive, [`read_file`] and [`write_file`] on this thread refuse every path: a +/// stored script's steps run inside one (#2178), so no command reaches the file system from a +/// script whatever its parameters are called. Dropping it (also on unwind) lifts the refusal. +pub(crate) struct ScriptSandbox(()); + +impl ScriptSandbox { + pub(crate) fn enter() -> Self { + let _ = SCRIPT_SANDBOX.try_with(|c| c.set(c.get().saturating_add(1))); + ScriptSandbox(()) + } +} + +impl Drop for ScriptSandbox { + fn drop(&mut self) { + let _ = SCRIPT_SANDBOX.try_with(|c| c.set(c.get().saturating_sub(1))); + } +} + +/// Refuses file access while a [`ScriptSandbox`] is alive (and, failing closed, while the +/// 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"))); + } + Ok(()) +} + #[cfg(not(target_arch = "wasm32"))] pub fn read_file(path: &str) -> Result> { + script_sandbox_check(path)?; let meta = std::fs::metadata(path).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; if meta.len() > 2 * 1024 * 1024 * 1024 { return Err(EngineError::Other("files larger than 2 GB aren't supported".into())); @@ -567,11 +600,13 @@ pub fn read_file(path: &str) -> Result> { #[cfg(target_arch = "wasm32")] pub fn read_file(path: &str) -> Result> { + script_sandbox_check(path)?; Err(EngineError::Other(format!("{path}: no file system in the browser"))) } #[cfg(not(target_arch = "wasm32"))] pub fn write_file(path: &str, bytes: &[u8]) -> Result<()> { + script_sandbox_check(path)?; // Write to a temporary file, then rename: a crash never leaves a half-written workbook. let tmp = format!("{path}.sctmp"); std::fs::write(&tmp, bytes).and_then(|()| std::fs::rename(&tmp, path)).map_err(|e| { @@ -584,6 +619,7 @@ pub fn write_file(path: &str, bytes: &[u8]) -> Result<()> { #[cfg(target_arch = "wasm32")] pub fn write_file(path: &str, _bytes: &[u8]) -> Result<()> { + script_sandbox_check(path)?; Err(EngineError::Other(format!("{path}: no file system in the browser"))) } @@ -902,6 +938,32 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + /// While a script runs (#2178), file reads and writes are refused whatever command or + /// parameter asked for them; nesting and dropping the sandbox restore access. + #[cfg(not(target_arch = "wasm32"))] + #[test] + fn script_sandbox_refuses_file_access_until_dropped() { + let dir = std::env::temp_dir().join(format!("gridcraft-sandbox-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let existing = dir.join("in.csv"); + std::fs::write(&existing, b"a,b\n").unwrap(); + let (existing, out) = (existing.to_string_lossy().to_string(), dir.join("out.xlsx").to_string_lossy().to_string()); + { + let _outer = ScriptSandbox::enter(); + { + let _inner = ScriptSandbox::enter(); + assert!(read_file(&existing).is_err()); + } + assert!(read_file(&existing).is_err(), "the outer sandbox still holds"); + assert!(write_file(&out, b"leak").is_err()); + assert!(!std::path::Path::new(&out).exists()); + } + assert_eq!(read_file(&existing).unwrap(), b"a,b\n"); + write_file(&out, b"ok").unwrap(); + let _ = std::fs::remove_dir_all(&dir); + } + /// A minimal compound (CFB/OLE2) file — FAT in sector 0, directory in sector 1 — holding a /// root entry and the given stream names. fn cfb(streams: &[&str]) -> Vec { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 567148d41..dcab1d204 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1147,6 +1147,44 @@ fn untrusted_workbook_scripts_are_refused_until_trusted() { assert!(s.execute("automate.runScript", json!({"name": "Fill"})).is_err()); } +#[test] +fn trust_scripts_only_defaults_to_trusted_when_the_flag_is_missing() { + let mut s = opened_with_script(); + // A `trusted` that isn't a boolean is an error and leaves the workbook untrusted. + for wrong in [json!("false"), json!("true"), json!(0), json!(1), json!(null), json!([true])] { + let err = s.execute("automate.trustScripts", json!({"trusted": wrong})).unwrap_err().to_string(); + assert!(err.contains("trusted"), "{wrong}: {err}"); + assert!(!s.doc().unwrap().scripts_trusted, "{wrong} must not trust the workbook"); + assert!(s.execute("automate.runScript", json!({"name": "Fill"})).is_err()); + } + // The refusal names where the Trust Scripts button actually is. + let err = s.execute("automate.runScript", json!({"name": "Fill"})).unwrap_err().to_string(); + assert!(err.contains("View › Macros") && err.contains("automate.trustScripts"), "{err}"); + assert_eq!(s.execute("automate.trustScripts", json!({"trusted": false})).unwrap()["trusted"], false); + assert!(!s.doc().unwrap().scripts_trusted); + assert_eq!(s.execute("automate.trustScripts", json!({})).unwrap()["trusted"], true); + assert!(s.doc().unwrap().scripts_trusted); +} + +#[test] +fn scripts_leave_file_access_working_after_they_run() { + // The sandbox a script runs in is lifted afterwards, also when a step fails. + let dir = std::env::temp_dir().join(format!("gridcraft-script-sandbox-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let csv = dir.join("in.csv"); + std::fs::write(&csv, b"a,b\n1,2\n").unwrap(); + let csv = csv.to_string_lossy().to_string(); + let mut s = s(); + let bad_step = json!({"command": "cell.set", "params": {"cell": "not a cell", "input": "x"}}); + s.execute("automate.newScript", json!({"name": "Bad", "commands": [bad_step]})).unwrap(); + assert!(s.execute("automate.runScript", json!({"name": "Bad"})).is_err()); + s.execute("automate.newScript", json!({"name": "Ok", "commands": [{"command": "cell.set", "params": {"cell": "A1", "input": "1"}}]})).unwrap(); + s.execute("automate.runScript", json!({"name": "Ok"})).unwrap(); + s.execute("data.fromTextCsv", json!({"path": csv})).unwrap(); + let _ = std::fs::remove_dir_all(&dir); +} + #[test] fn scripts_cannot_reach_outside_the_workbook_even_when_trusted() { let dir = std::env::temp_dir().join(format!("gridcraft-script-deny-{}", std::process::id())); diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index d218c8169..ee8bcfb75 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -4,7 +4,7 @@ Each entry is an Excel ribbon or menu command (feature names only). It counts as implemented when GridCraft registers a command with the same id. This measures breadth only; the overall assessment (depth, fidelity, effort) is in [target-app-parity.md](target-app-parity.md). -**Overall: 274 / 290 commands implemented (94.5%).** +**Overall: 275 / 291 commands implemented (94.5%).** | Tab | Implemented | Total | Parity | |---|---|---|---| @@ -17,7 +17,7 @@ Each entry is an Excel ribbon or menu command (feature names only). It counts as | Data | 23 | 28 | 82% | | Review | 15 | 18 | 83% | | View | 20 | 20 | 100% | -| Automate | 3 | 3 | 100% | +| Automate | 4 | 4 | 100% | | Table Design | 13 | 13 | 100% | | Chart Design | 8 | 8 | 100% | | Format | 3 | 3 | 100% | @@ -324,13 +324,14 @@ Implemented: - `view.switchWindows` Switch Windows (Window) - `view.macros` Macros (Macros) -## Automate (3 / 3, 100%) +## Automate (4 / 4, 100%) Implemented: - `automate.newScript` New Script (Scripting Tools) - `automate.allScripts` All Scripts (Scripting Tools) - `automate.recordActions` Record Actions (Scripting Tools) +- `automate.trustScripts` Trust Scripts (Scripting Tools) ## Table Design (13 / 13, 100%)