diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index c4a290e..a37c1a8 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -69,6 +69,21 @@ impl CommandSpec { } } +/// Params that hold a secret: `password`, or a name ending in `Password`. +pub fn is_secret_param(key: &str) -> bool { + key == "password" || key.ends_with("Password") +} + +/// The params as the journal records them: secret params are left out, so a password never +/// reaches the journal dialog, `engine.journal` or a recorded script (saved in the workbook). +/// Replaying such an entry runs the command without that param. +pub fn journal_params(params: &Json) -> Json { + match params { + Json::Object(o) => Json::Object(o.iter().filter(|(k, _)| !is_secret_param(k)).map(|(k, v)| (k.clone(), v.clone())).collect()), + other => other.clone(), + } +} + pub fn always(_: &Session) -> std::result::Result<(), String> { Ok(()) } diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index e034345..451404d 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -188,6 +188,7 @@ pub struct Session { pub clipboard: Option, pub prefs: Prefs, pub mode: Mode, + /// Commands run so far, without their passwords ([`cmd::journal_params`]). pub journal: Vec<(String, Json)>, pub ui_requests: Vec, /// Format painter source style (sticky when double-clicked). @@ -288,7 +289,7 @@ impl Session { } }; if spec.journal && result.is_ok() { - self.journal.push((id.to_string(), params.clone())); + self.journal.push((id.to_string(), cmd::journal_params(¶ms))); if self.journal.len() > 10_000 { self.journal.drain(..5_000); } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 2aa3c9f..3e2f66f 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -611,6 +611,43 @@ fn filter_on_a_selected_header_row_covers_the_data_below() { assert_eq!(r["range"], "A1:B1"); } +#[test] +fn protection_passwords_stay_out_of_the_journal_and_recorded_scripts() { + const PW: &str = "synthetic-pw-7319"; + let mut s = s(); + s.execute("automate.recordActions", json!({})).unwrap(); + s.execute("review.protectSheet", json!({"password": PW, "formatCells": true})).unwrap(); + // The password still protects the sheet; the refused attempt isn't journaled. + assert!(s.execute("review.unprotectSheet", json!({})).is_err()); + s.execute("review.unprotectSheet", json!({"password": PW})).unwrap(); + s.execute("automate.recordActions", json!({"stop": true, "name": "Lock"})).unwrap(); + + let journal = serde_json::to_string(&s.journal).unwrap(); + assert!(!journal.contains(PW), "{journal}"); + let params = |s: &Session, id: &str| s.journal.iter().find(|(c, _)| c == id).map(|(_, p)| p.clone()); + assert_eq!(params(&s, "review.protectSheet"), Some(json!({"formatCells": true}))); + assert_eq!(params(&s, "review.unprotectSheet"), Some(json!({}))); + + // The recorded script is stored in the workbook, so it is saved with the file. + let bytes = crate::io::save_bytes(&s.doc().unwrap().wb, "book.xlsx").unwrap(); + let (reopened, _) = crate::io::open_bytes("book.xlsx", &bytes).unwrap(); + let script = reopened.names.iter().find(|n| n.name == "_sc_scripts").map(|n| n.formula.clone()).unwrap(); + assert!(script.contains("review.protectSheet"), "{script}"); + assert!(!script.contains(PW), "{script}"); + + // The journal still replays: protection without a password, then unprotection. + let mut r = self::s(); + for (id, p) in s.journal.iter().filter(|(id, _)| !id.starts_with("automate.")) { + r.execute(id, p.clone()).unwrap_or_else(|e| panic!("{id}: {e}")); + let prot = r.doc().unwrap().wb.active().unwrap().protection.clone(); + if id == "review.protectSheet" { + assert!(prot.as_ref().is_some_and(|p| p.format_cells && p.password_hash.is_none()), "{prot:?}"); + } + } + assert!(r.doc().unwrap().wb.active().unwrap().protection.is_none()); + s.execute("automate.runScript", json!({"name": "Lock"})).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/docs/control-protocol.md b/docs/control-protocol.md index 2b1beaa..1a9ce74 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -21,7 +21,7 @@ Errors come back as `{"id": …, "ok": false, "error": "…"}`. |---|---|---| | `engine.execute` | `{command, params}` | Runs any engine command (see `engine.commands` or `gridcraft-cli commands`) | | `engine.commands` | `{search?}` | Every command: id, label, ribbon path, shortcut, params doc, enabled; `search` keeps those whose id, label or ribbon path contains it (case-insensitive) | -| `engine.journal` | | Commands run so far (replayable) | +| `engine.journal` | | Commands run so far (replayable). Passwords (`password`, `…Password` params) are left out, so a replayed Protect Sheet has no password | | `document.inspect` | | Workbook summary: sheets, used ranges, tables, charts, names, selection, undo labels | | `sheet.read` | `{range?, sheet?, formulas?, formatted?}` | Values (or formulas / displayed text) of a range | | `cell.get` | `{cell?}` | One cell: value, formula, displayed text, style, merge, comment, link |