fix(engine): undo and redo purge stale grants; explicit-text paste is not a clipboard read

crates/engine/src/cmd.rs

@@ -467,11 +467,19 @@ pub(crate) fn commit<R>(d: &mut DocState, f: impl FnOnce(&mut Ctx) -> Result<R>)
     if let Some(t0) = t0 {
         eprintln!("commit: + rows {:?}", since(t0));
     }
+    // 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.
+    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

crates/engine/src/cmd/edit.rs

@@ -209,6 +209,7 @@ fn undo(s: &mut Session, p: &Json) -> Result<Json> {
         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<Json> {
         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}))

crates/engine/src/cmd/extra/scripts.rs

@@ -75,6 +75,9 @@ fn denied_script_step(id: &str, params: &Json) -> Option<String> {
     None
 }
 
+/// Commands that read the session's clipboard, which a script may use only for a copy made in its own workbook.
+const CLIPBOARD_READERS: [&str; 3] = ["edit.paste", "edit.pasteSpecial", "home.insertCopiedCells"];
+
 pub(super) fn trust_scripts(s: &mut Session, p: &Json) -> Result<Json> {
     // Only a missing `trusted` means "trust"; `"false"`, `0` or `null` are mistakes, not consent.
     let trusted = match p.get("trusted") {
@@ -86,6 +89,23 @@ pub(super) fn trust_scripts(s: &mut Session, p: &Json) -> Result<Json> {
     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<u64> {
+    Ok(s.doc()?.uid)
+}
+
 /// 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.
@@ -108,6 +128,7 @@ pub(super) fn run_script(s: &mut Session, p: &Json) -> Result<Json> {
         }
     }
     let _sandbox = crate::io::ScriptSandbox::enter();
+    let home = d_uid(s)?;
     let mut n = 0;
     // What ran, for a recorder in progress: the recording keeps these steps with the run, so
     // editing or deleting the script before the recording stops does not change what it saved.
@@ -117,9 +138,25 @@ pub(super) fn run_script(s: &mut Session, p: &Json) -> Result<Json> {
         if id.starts_with("automate.") {
             continue;
         }
+        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();
         let wb_before = s.doc().ok().map(|d| (d.uid, d.wb.clone()));
-        let ran = s.execute(id, st["params"].clone());
+        // A stored step is not the person: an `approve` it carries is not consent to run a macro.
+        let mut params = st["params"].clone();
+        if let Some(o) = params.as_object_mut() {
+            o.remove("approve");
+        }
+        let ran = s.execute(id, params);
+        // A step (a nested macro, an alias of a window command) that switched windows would point the
+        // rest of the script at another workbook: put the script's own back and stop.
+        if s.doc().map(|d| d.uid).ok() != Some(home) {
+            if let Some(i) = s.docs.iter().position(|d| d.uid == home) {
+                s.set_active(i);
+            }
+            return Err(bad("automate.runScript", format!("`{id}` switched to another workbook; scripts stay in their own")));
+        }
         // 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;

crates/engine/src/io.rs

@@ -924,56 +924,17 @@ fn script_sandbox_check(path: &str) -> Result<()> {
 #[cfg(not(target_arch = "wasm32"))]
 const MAX_READ_BYTES: u64 = 2 * 1024 * 1024 * 1024;
 
-/// `O_NONBLOCK` as the platform defines it (std offers no constant and the crate takes no libc
-/// dependency); `None` on a Unix not listed, where the pathname check alone applies.
-#[cfg(all(unix, not(target_arch = "wasm32")))]
-const O_NONBLOCK: Option<i32> =
-    if cfg!(any(target_os = "macos", target_os = "ios", target_os = "freebsd", target_os = "netbsd", target_os = "openbsd", target_os = "dragonfly"))
-    {
-        Some(0x4)
-    } else if cfg!(all(
-        any(target_os = "linux", target_os = "android"),
-        any(target_arch = "mips", target_arch = "mips64", target_arch = "mips32r6", target_arch = "mips64r6")
-    )) {
-        Some(0x80)
-    } else if cfg!(all(any(target_os = "linux", target_os = "android"), any(target_arch = "sparc", target_arch = "sparc64"))) {
-        Some(0x4000)
-    } else if cfg!(any(target_os = "linux", target_os = "android")) {
-        Some(0o4000)
-    } else {
-        None
-    };
-
-/// Opens `path` for reading without blocking on a FIFO or device that has no writer.
-#[cfg(not(target_arch = "wasm32"))]
-fn open_nonblocking(path: &std::path::Path) -> std::io::Result<std::fs::File> {
-    let mut opts = std::fs::OpenOptions::new();
-    opts.read(true);
-    #[cfg(unix)]
-    if let Some(flag) = O_NONBLOCK {
-        use std::os::unix::fs::OpenOptionsExt;
-        opts.custom_flags(flag);
-    }
-    opts.open(path)
-}
-
 #[cfg(not(target_arch = "wasm32"))]
 pub fn read_file(path: &str) -> Result<Vec<u8>> {
     use std::io::Read;
     script_sandbox_check(path)?;
-    // Checked before opening: opening a FIFO nobody writes to blocks the calling (GUI) thread. The
-    // open itself is non-blocking where the platform's flag is known, so a FIFO swapped in after
-    // this check cannot block it either; the opened handle is checked again below.
+    // Checked before opening: opening a FIFO nobody writes to blocks the calling (GUI) thread.
+    // shortcut: a FIFO swapped in between this check and the open still blocks; close that with
+    // O_NONBLOCK when the crate takes a libc dependency.
     if !std::fs::metadata(path).map_err(|e| EngineError::Other(format!("{path}: {e}")))?.is_file() {
         return Err(EngineError::Other(format!("{path}: only regular files can be opened")));
     }
-    let f = match open_nonblocking(std::path::Path::new(path)) {
-        Ok(f) => f,
-        // A conflicting lease (Samba, NFS) makes a non-blocking open of a regular file wait to be retried; the
-        // path was just checked to be a regular file, so open it the plain way, as before.
-        Err(e) if e.kind() == std::io::ErrorKind::WouldBlock => std::fs::File::open(path).map_err(|e| EngineError::Other(format!("{path}: {e}")))?,
-        Err(e) => return Err(EngineError::Other(format!("{path}: {e}"))),
-    };
+    let f = std::fs::File::open(path).map_err(|e| EngineError::Other(format!("{path}: {e}")))?;
     // A symlink to an endless device reports length 0 but streams forever, so metadata
     // alone never bounds the read: require a real file and cap the bytes actually taken.
     if !f.metadata().map_err(|e| EngineError::Other(format!("{path}: {e}")))?.is_file() {

crates/engine/src/io/tests.rs

@@ -810,44 +810,3 @@ fn reading_a_fifo_is_refused_without_blocking() {
     assert!(err.contains("only regular files"), "{err}");
     let _ = std::fs::remove_dir_all(&dir);
 }
-
-/// Opening a FIFO nobody writes to returns at once (it is not a regular file), where a plain open
-/// blocks the calling thread for good.
-#[cfg(unix)]
-#[test]
-fn a_fifo_without_a_writer_does_not_block_the_open() {
-    if O_NONBLOCK.is_none() {
-        return;
-    }
-    /// Removes the folder when the test ends, however it ends.
-    struct Cleanup(std::path::PathBuf);
-    impl Drop for Cleanup {
-        fn drop(&mut self) {
-            let _ = std::fs::remove_dir_all(&self.0);
-        }
-    }
-    // Created exclusively (a fresh name per try), so no other process shares it.
-    let dir = (0..100)
-        .map(|n| {
-            std::env::temp_dir().join(format!(
-                "gridcraft-fifo-{}-{n}-{}",
-                std::process::id(),
-                std::time::UNIX_EPOCH.elapsed().map_or(0, |t| t.as_nanos())
-            ))
-        })
-        .find(|d| std::fs::create_dir(d).is_ok())
-        .expect("a fresh temporary folder");
-    let _cleanup = Cleanup(dir.clone());
-    let fifo = dir.join("pipe");
-    if !std::process::Command::new("mkfifo").arg(&fifo).status().is_ok_and(|s| s.success()) {
-        return; // no mkfifo here
-    }
-    let (tx, rx) = std::sync::mpsc::channel();
-    let path = fifo.clone();
-    std::thread::spawn(move || {
-        let _ = tx.send(open_nonblocking(&path).and_then(|f| f.metadata()).map(|m| m.is_file()));
-    });
-    let opened = rx.recv_timeout(std::time::Duration::from_secs(10)).expect("the open blocked");
-    assert!(!opened.unwrap());
-    assert!(read_file(&fifo.to_string_lossy()).is_err());
-}

crates/engine/src/tests.rs

@@ -6480,6 +6480,34 @@ fn clearing_links_and_notes_keeps_allow_edit_ranges() {
     assert!(sh.hyperlinks.is_empty() && sh.comments.is_empty());
 }
 
+#[test]
+fn an_edit_range_grant_does_not_outlive_its_sheet_name() {
+    let mut s = s();
+    s.execute("review.allowEditRanges", json!({"action": "add", "title": "Kept", "range": "C5", "password": "example"})).unwrap();
+    s.execute("review.protectSheet", json!({})).unwrap();
+    s.execute("review.allowEditRanges", json!({"action": "unlock", "title": "Kept", "password": "example"})).unwrap();
+    assert!(!s.doc().unwrap().unlocked_ranges.is_empty());
+    // The sheet is renamed and another takes its name, with a range of the same title and password.
+    s.execute("review.unprotectSheet", json!({})).unwrap();
+    let old = s.doc().unwrap().wb.active().unwrap().name.clone();
+    s.execute("sheet.rename", json!({"name": "Renamed"})).unwrap();
+    assert!(s.doc().unwrap().unlocked_ranges.is_empty(), "the grant went with the old name");
+    s.execute("home.insertSheet", json!({"name": old})).unwrap();
+    s.execute("review.allowEditRanges", json!({"action": "add", "title": "Kept", "range": "C5", "password": "other"})).unwrap();
+    s.execute("review.protectSheet", json!({})).unwrap();
+    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();

crates/engine/src/tests/macros.rs

@@ -2514,3 +2514,40 @@ fn a_step_that_deletes_a_form_control_after_a_partial_footprint_is_caught() {
     s.execute("macro.run", json!({"name": "Drop", "inputs": "A1", "overwrite": true})).unwrap();
     assert!(s.doc().unwrap().wb.active().unwrap().controls.is_empty());
 }
+
+#[test]
+fn a_script_cannot_paste_what_another_workbook_copied() {
+    let mut s = s();
+    let steps = json!([{"command": "edit.paste", "params": {"at": "B2"}}]);
+    s.execute("automate.newScript", json!({"name": "Paste", "commands": steps})).unwrap();
+    // A copy made in this workbook may be pasted by its script.
+    s.execute("cell.set", json!({"cell": "A1", "input": "mine"})).unwrap();
+    s.execute("edit.copy", json!({"range": "A1"})).unwrap();
+    s.execute("automate.runScript", json!({"name": "Paste"})).unwrap();
+    // A copy from another workbook is not read.
+    s.execute("file.new", json!({})).unwrap();
+    s.execute("cell.set", json!({"cell": "A1", "input": "secret"})).unwrap();
+    s.execute("edit.copy", json!({"range": "A1"})).unwrap();
+    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]
+fn a_script_step_cannot_approve_a_macro_for_the_person() {
+    let mut s = s();
+    let mut wb = gridcraft_model::Workbook::new();
+    wb.macros.push(crate::macros::parse_macro(&json!({"name": "Theirs", "output": {"onConflict": "overwrite"}, "steps": [{"command": "cell.set", "params": {"cell": "A1", "input": "ran"}}]})).unwrap());
+    let bytes = serde_json::to_vec(&wb).unwrap();
+    s.execute("file.open", json!({"name": "theirs.json", "base64": crate::io::base64_encode(&bytes)})).unwrap();
+    let steps = json!([{"command": "macro.run", "params": {"name": "Theirs", "approve": true}}]);
+    s.execute("automate.newScript", json!({"name": "Run", "commands": steps})).unwrap();
+    let r = s.execute("automate.runScript", json!({"name": "Run", "trusted": true}));
+    assert!(matches!(r, Err(EngineError::Approve(_))), "{r:?}");
+    assert!(s.macro_approvals.is_empty(), "the step's flag approved nothing");
+    assert!(v(&s, "A1").is_empty());
+}