diff --git a/crates/engine/Cargo.toml b/crates/engine/Cargo.toml index b559b83..e8ea91c 100644 --- a/crates/engine/Cargo.toml +++ b/crates/engine/Cargo.toml @@ -25,4 +25,5 @@ gridcraft-pdf = { workspace = true } image = { workspace = true } [dev-dependencies] +gridcraft-xlsx = { workspace = true, features = ["test-support"] } zip = { workspace = true } diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index d7765d2..3c4a101 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -8,7 +8,7 @@ use crate::DocState; pub fn specs() -> Vec { vec![ cmd!(noundo "file.new", "New Workbook", ["File"], Some("Cmd+N"), "{sample?: \"budget\"|\"sales\"|\"grades\"}", always, new_workbook), - cmd!(noundo "file.open", "Open…", ["File"], Some("Cmd+O"), "{path} | {name, base64} (xlsx, xlsm, xls import, xlsb and ods data import, csv, tsv, txt, json)", always, open), + cmd!(noundo "file.open", "Open…", ["File"], Some("Cmd+O"), "{path} | {name, base64}, plus {password} for a password-protected workbook (xlsx, xlsm, xls import, xlsb and ods data import, csv, tsv, txt, json)", always, open), cmd!(noundo "file.save", "Save", ["File"], Some("Cmd+S"), "{path?} (xlsx by default; .csv/.tsv/.json/.html/.pdf by extension)", has_doc, save), cmd!(noundo "file.saveAs", "Save As…", ["File"], Some("Cmd+Shift+S"), "{path} (.xlsx, .csv, .tsv, .json, .html, or .pdf: a PDF of the active sheet)", has_doc, save_as), cmd!(query "file.saveBytes", "Encode Workbook", [], None, "{format?: xlsx|csv|tsv|json|html} → {base64}", has_doc, save_bytes), @@ -55,7 +55,8 @@ fn open(s: &mut Session, p: &Json) -> Result { s.ui_requests.push(crate::UiRequest::Dialog("open".into(), json!({}))); return ok(); }; - let (wb, warnings) = crate::io::open_bytes(&name, &bytes)?; + let encrypted = gridcraft_xlsx::sniff(&bytes) == gridcraft_xlsx::Format::Encrypted; + let (wb, warnings) = crate::io::open_bytes_with_password(&name, &bytes, str_param(p, "password"))?; let imported = matches!(crate::io::FileKind::from_path(&name), Some(crate::io::FileKind::Ods | crate::io::FileKind::Xlsb | crate::io::FileKind::Xls)) || matches!(gridcraft_xlsx::sniff(&bytes), gridcraft_xlsx::Format::Ods | gridcraft_xlsx::Format::Xlsb | gridcraft_xlsx::Format::Xls); @@ -64,8 +65,10 @@ fn open(s: &mut Session, p: &Json) -> Result { } else { std::path::Path::new(&name).file_name().and_then(|n| n.to_str()).unwrap_or("Book").to_string() }; - // An import has no save target: ordinary Save must ask for a new destination. - let path = str_param(p, "path").filter(|_| !imported).map(str::to_string); + // An import has no save target: ordinary Save must ask for a new destination. Neither does a + // workbook decrypted with its password, so Save never replaces the encrypted original with + // an unprotected copy. + let path = str_param(p, "path").filter(|_| !imported && !encrypted).map(str::to_string); // Replace an untouched blank Book1 like Excel does. if s.documents().len() == 1 && s.active().is_some_and(|d| d.path.is_none() && !d.is_dirty() && d.undo.is_empty() && d.wb.sheets.iter().all(|sh| sh.cells.is_empty())) @@ -73,10 +76,12 @@ fn open(s: &mut Session, p: &Json) -> Result { s.close_document(0); } let i = s.add_document(DocState::new(wb, path, title)); - if !warnings.is_empty() { - let mut message = warnings.iter().take(5).cloned().collect::>().join("\n\n"); - if warnings.len() > 5 { - message.push_str(&format!("\n\n{} additional import warnings.", warnings.len() - 5)); + // The decrypted-workbook notice is for the info bar, not a message box. + let shown: Vec<&str> = warnings.iter().map(String::as_str).filter(|w| *w != crate::io::DECRYPTED_NOTICE).collect(); + if !shown.is_empty() { + let mut message = shown.iter().take(5).copied().collect::>().join("\n\n"); + if shown.len() > 5 { + message.push_str(&format!("\n\n{} additional import warnings.", shown.len() - 5)); } s.ui_requests.push(crate::UiRequest::Message(message)); } diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index e388f87..7d35029 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -38,16 +38,50 @@ impl FileKind { } } +/// The error for an encrypted workbook opened without a password contains this. The interface +/// asks for the password when it sees it; programmatic callers pass `password` instead. +pub const PASSWORD_REQUIRED: &str = "is password-protected. Enter its password to open it."; +/// The error for a wrong password contains this. +pub const WRONG_PASSWORD: &str = "The password is incorrect."; +/// The first open warning of a workbook decrypted with its password. +pub const DECRYPTED_NOTICE: &str = "This workbook was password-protected. GridCraft saves it without the password."; + /// Parses a file's bytes into a workbook. `name` picks the format by extension when the bytes -/// don't say. +/// don't say. An encrypted workbook needs [`open_bytes_with_password`]. pub fn open_bytes(name: &str, bytes: &[u8]) -> Result<(Workbook, Vec)> { + open_bytes_with_password(name, bytes, None) +} + +/// [`open_bytes`], first decrypting a password-protected workbook with `password`. Errors never +/// quote the password. +pub fn open_bytes_with_password(name: &str, bytes: &[u8], password: Option<&str>) -> Result<(Workbook, Vec)> { + if gridcraft_xlsx::sniff(bytes) != gridcraft_xlsx::Format::Encrypted { + return open_plain(name, bytes); + } + let Some(password) = password else { + return Err(EngineError::Other(format!("'{name}' {PASSWORD_REQUIRED}"))); + }; + let plain = gridcraft_xlsx::decrypt(bytes, password).map_err(|e| { + EngineError::Other(match e { + gridcraft_xlsx::DecryptError::WrongPassword => format!("{WRONG_PASSWORD} Check that Caps Lock is off and try again."), + gridcraft_xlsx::DecryptError::Unsupported(scheme) => { + format!("'{name}' uses {scheme} encryption, which GridCraft can't open yet. Remove the password in Excel and try again.") + } + gridcraft_xlsx::DecryptError::Malformed(detail) => format!("We can't open '{name}': the encrypted file is damaged ({detail})."), + }) + })?; + if gridcraft_xlsx::sniff(&plain) == gridcraft_xlsx::Format::Encrypted { + return Err(EngineError::Other(format!("We can't open '{name}': the encrypted file is damaged (it holds another encrypted file)."))); + } + let (wb, mut warnings) = open_plain(name, &plain)?; + warnings.insert(0, DECRYPTED_NOTICE.into()); + Ok((wb, warnings)) +} + +/// Parses bytes that aren't encrypted. +fn open_plain(name: &str, bytes: &[u8]) -> Result<(Workbook, Vec)> { let kind = FileKind::from_path(name); let sniffed = gridcraft_xlsx::sniff(bytes); - if sniffed == gridcraft_xlsx::Format::Encrypted { - return Err(EngineError::Other(format!( - "'{name}' is password-protected. GridCraft can't open encrypted workbooks yet; remove the password in Excel and try again." - ))); - } if sniffed == gridcraft_xlsx::Format::Xls { let (wb, report) = gridcraft_xlsx::read_xls(bytes).map_err(|e| EngineError::Other(format!("We can't open '{name}': {e}")))?; return Ok((wb, report.warnings)); @@ -412,10 +446,14 @@ mod tests { #[test] fn encrypted_workbook_is_reported_not_misread() { - let err = open_bytes("secret.xlsx", &cfb(&["EncryptionInfo", "EncryptedPackage"])).unwrap_err(); + let file = cfb(&["EncryptionInfo", "EncryptedPackage"]); + let err = open_bytes("secret.xlsx", &file).unwrap_err(); let msg = err.to_string(); - assert!(msg.contains("password-protected"), "got: {msg}"); + assert!(msg.contains(PASSWORD_REQUIRED), "got: {msg}"); assert!(!msg.contains("zip"), "must not surface the zip error: {msg}"); + // These streams are empty: with a password it is damage, not a wrong password. + let msg = open_bytes_with_password("secret.xlsx", &file, Some("pw")).unwrap_err().to_string(); + assert!(msg.contains("damaged"), "got: {msg}"); } #[test] diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index c9f4d0a..c45a476 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2360,3 +2360,74 @@ fn editing_a_date_cell_shows_a_date_not_the_serial() { let d = s2.doc().unwrap(); assert_eq!(crate::display::edit_text(&d.wb, d.wb.active().unwrap(), at), "42"); } + +/// A workbook with 42 in A1, saved as `.xlsx` and encrypted with `password`. +fn encrypted_book(password: &str) -> Vec { + let mut src = s(); + src.execute("cell.set", json!({"cell": "A1", "input": "42"})).unwrap(); + let xlsx = crate::io::save_bytes(&src.doc().unwrap().wb, "book.xlsx").unwrap(); + gridcraft_xlsx::testing::encrypt_workbook(&xlsx, password) +} + +#[test] +fn encrypted_workbooks_open_with_their_password() { + const PW: &str = "synthetic-pw-5521"; + let b64 = crate::io::base64_encode(&encrypted_book(PW)); + let open = |s: &mut Session, password: Option<&str>| { + let mut p = json!({"name": "Secret.xlsx", "base64": b64}); + if let Some(pw) = password { + p["password"] = json!(pw); + } + s.execute("file.open", p) + }; + let mut s = Session::new(); + + // Programmatic open without a password: an error carrying the marker, never a dialog. + let err = open(&mut s, None).unwrap_err().to_string(); + assert!(err.contains(crate::io::PASSWORD_REQUIRED), "{err}"); + assert!(err.contains("Secret.xlsx"), "{err}"); + assert!(s.take_ui_requests().is_empty(), "programmatic calls never open dialogs"); + assert!(s.documents().is_empty()); + + // Wrong and empty passwords are "incorrect", and the password is never quoted. + for wrong in ["not-it", ""] { + let err = open(&mut s, Some(wrong)).unwrap_err().to_string(); + assert!(err.contains(crate::io::WRONG_PASSWORD), "{err}"); + assert!(wrong.is_empty() || !err.contains(wrong), "{err}"); + } + assert!(s.documents().is_empty()); + + // The right password opens it: no save path, the file name as title, the notice first. + let r = open(&mut s, Some(PW)).unwrap(); + assert_eq!(r["warnings"][0], crate::io::DECRYPTED_NOTICE); + assert_eq!(r["title"], "Secret.xlsx"); + assert_eq!(v(&s, "A1"), Value::Number(42.0)); + assert_eq!(s.doc().unwrap().path, None); + // The notice belongs in the info bar, not a message box. + assert!(!s.take_ui_requests().iter().any(|r| matches!(r, crate::UiRequest::Message(_)))); + let journal = serde_json::to_string(&s.journal).unwrap(); + assert!(!journal.contains(PW), "the password never reaches the journal"); +} + +#[test] +#[cfg(not(target_arch = "wasm32"))] +fn a_decrypted_workbook_never_overwrites_its_encrypted_original() { + const PW: &str = "synthetic-pw-5522"; + #[allow(clippy::disallowed_methods)] // native-only test: a unique temp dir name + let nonce = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).unwrap().as_nanos(); + let dir = std::env::temp_dir().join(format!("gridcraft-encrypted-{}-{nonce}", std::process::id())); + std::fs::create_dir(&dir).unwrap(); + let path = dir.join("Secret.xlsx"); + let bytes = encrypted_book(PW); + std::fs::write(&path, &bytes).unwrap(); + + let mut s = s(); + s.execute("file.open", json!({"path": path, "password": PW})).unwrap(); + assert_eq!(s.doc().unwrap().path, None); + s.take_ui_requests(); + s.execute("cell.set", json!({"cell": "A1", "input": "43"})).unwrap(); + s.execute("file.save", json!({})).unwrap(); + assert!(s.take_ui_requests().iter().any(|r| matches!(r, crate::UiRequest::Dialog(name, _) if name == "saveAs"))); + assert_eq!(std::fs::read(&path).unwrap(), bytes, "the encrypted original is untouched"); + std::fs::remove_dir_all(dir).unwrap(); +}