From 6eaa4bc928a90eed66fcd86e03c7a8f1005930cd Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 22:46:39 +0000 Subject: [PATCH] fix(engine): undo and redo purge stale grants; explicit-text paste is not a clipboard read --- crates/engine/src/cmd.rs | 7 ++++++- crates/engine/src/cmd/edit.rs | 2 ++ crates/engine/src/cmd/extra/scripts.rs | 15 ++++++++++++++- crates/engine/src/tests.rs | 10 ++++++++++ crates/engine/src/tests/macros.rs | 4 ++++ 5 files changed, 36 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd.rs b/crates/engine/src/cmd.rs index fa335c439..2eac363f0 100644 --- a/crates/engine/src/cmd.rs +++ b/crates/engine/src/cmd.rs @@ -469,12 +469,17 @@ pub(crate) fn commit(d: &mut DocState, f: impl FnOnce(&mut Ctx) -> Result) } // A grant to edit a protected range belongs to the sheet and range it was given for: a sheet that // was renamed, deleted or replaced by another of the same name no longer carries it. - d.unlocked_ranges.retain(|(sheet, title)| wb.sheets.iter().any(|sh| sh.name == *sheet && sh.edit_ranges.iter().any(|r| r.title == *title))); + purge_unlock_grants(&mut d.unlocked_ranges, &wb); d.wb = std::sync::Arc::new(wb); d.selection = sel; Ok(r) } +/// Drops the grants to edit protected ranges whose sheet name or range title `wb` no longer has. +pub(crate) fn purge_unlock_grants(grants: &mut Vec<(String, String)>, wb: &Workbook) { + grants.retain(|(sheet, title)| wb.sheets.iter().any(|sh| sh.name == *sheet && sh.edit_ranges.iter().any(|r| r.title == *title))); +} + /// "Set precision as displayed": a number typed, pasted or reformatted is stored as its format /// shows it (Excel rounds a constant when it is entered and when its cell's format changes, /// observed through COM). Only cells the edit touched are visited, so this costs nothing for diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 8c7072f52..b3edc1481 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -209,6 +209,7 @@ fn undo(s: &mut Session, p: &Json) -> Result { crate::push_history(&mut d.redo, label.clone(), cur_wb, &d.wb, cur_selection); } super::extra::keep_watch_list(&now, &mut d.wb); + crate::cmd::purge_unlock_grants(&mut d.unlocked_ranges, &d.wb); d.calc.rebuild(&d.wb); d.revision += 1; Ok(json!({"undone": label})) @@ -230,6 +231,7 @@ fn redo(s: &mut Session, p: &Json) -> Result { crate::push_history(&mut d.undo, label.clone(), cur_wb, &d.wb, cur_selection); } super::extra::keep_watch_list(&now, &mut d.wb); + crate::cmd::purge_unlock_grants(&mut d.unlocked_ranges, &d.wb); d.calc.rebuild(&d.wb); d.revision += 1; Ok(json!({"redone": label})) diff --git a/crates/engine/src/cmd/extra/scripts.rs b/crates/engine/src/cmd/extra/scripts.rs index ab570da31..68c1784a7 100644 --- a/crates/engine/src/cmd/extra/scripts.rs +++ b/crates/engine/src/cmd/extra/scripts.rs @@ -89,6 +89,19 @@ pub(super) fn trust_scripts(s: &mut Session, p: &Json) -> Result { Ok(json!({"trusted": trusted})) } +/// Whether step `id` with `params` takes cells from the session clipboard: not `edit.paste` of explicit +/// text that differs from it (that pastes the text, see `edit::clipboard::paste`). +fn reads_clipboard(s: &Session, id: &str, params: &Json) -> bool { + if !CLIPBOARD_READERS.contains(&id) { + return false; + } + let trim = |t: &str| t.trim_end_matches(['\r', '\n']).to_string(); + match (id, str_param(params, "text")) { + ("edit.paste", Some(text)) => s.clipboard.as_ref().is_some_and(|c| trim(&c.text) == trim(text)), + _ => true, + } +} + fn d_uid(s: &Session) -> Result { Ok(s.doc()?.uid) } @@ -125,7 +138,7 @@ pub(super) fn run_script(s: &mut Session, p: &Json) -> Result { if id.starts_with("automate.") { continue; } - if CLIPBOARD_READERS.contains(&id) && s.clipboard.as_ref().is_some_and(|c| c.doc_uid != home) { + if reads_clipboard(s, id, &st["params"]) && s.clipboard.as_ref().is_some_and(|c| c.doc_uid != home) { return Err(bad("automate.runScript", format!("scripts can't run `{id}` on cells copied from another workbook"))); } let before = s.ui_requests.len(); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 70c85bf30..34227cd92 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -6498,6 +6498,16 @@ fn an_edit_range_grant_does_not_outlive_its_sheet_name() { assert!(s.execute("cell.set", json!({"cell": "C5", "input": "1"})).is_err(), "the new sheet's range is locked"); } +#[test] +fn undo_drops_the_grant_of_a_range_it_removes() { + let mut s = s(); + s.execute("review.allowEditRanges", json!({"action": "add", "title": "Kept", "range": "C5", "password": "example"})).unwrap(); + s.execute("review.allowEditRanges", json!({"action": "unlock", "title": "Kept", "password": "example"})).unwrap(); + assert!(!s.doc().unwrap().unlocked_ranges.is_empty()); + s.execute("edit.undo", json!({})).unwrap(); + assert!(s.doc().unwrap().unlocked_ranges.is_empty(), "the range is gone, so is its grant"); +} + #[test] fn formulatext_does_not_reveal_a_hidden_formula() { let mut s = s(); diff --git a/crates/engine/src/tests/macros.rs b/crates/engine/src/tests/macros.rs index b80a9343f..f9a6e2eed 100644 --- a/crates/engine/src/tests/macros.rs +++ b/crates/engine/src/tests/macros.rs @@ -2531,6 +2531,10 @@ fn a_script_cannot_paste_what_another_workbook_copied() { s.set_active(0); let err = s.execute("automate.runScript", json!({"name": "Paste"})).unwrap_err().to_string(); assert!(err.contains("another workbook"), "{err}"); + // Explicit text that is not the clipboard's does not read it. + let own = json!([{"command": "edit.paste", "params": {"at": "C3", "text": "typed"}}]); + s.execute("automate.newScript", json!({"name": "Typed", "commands": own})).unwrap(); + s.execute("automate.runScript", json!({"name": "Typed"})).unwrap(); } #[test]