mirror of
https://github.com/storytold/gridcraft.git
synced 2026-10-28 04:36:51 -06:00
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
912b817abe
commit
5392d95bf8
@@ -193,7 +193,24 @@ pub fn specs() -> Vec<CommandSpec> {
|
||||
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<Json> {
|
||||
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<Json> {
|
||||
}
|
||||
|
||||
fn all_scripts(s: &mut Session, _: &Json) -> Result<Json> {
|
||||
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<Json> =
|
||||
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<Json> = 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<Json> {
|
||||
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<String> {
|
||||
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<Json> {
|
||||
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<Json> {
|
||||
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<Json> = 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;
|
||||
|
||||
@@ -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<io::FileKind>,
|
||||
/// 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<String>, 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 {
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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::<Vec<_>>())))
|
||||
@@ -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<Json> = 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)");
|
||||
|
||||
Reference in New Issue
Block a user