Engine: file.open decrypts password-protected workbooks (#11)
Without a password, file.open returns an error carrying io::PASSWORD_REQUIRED (never a dialog). A decrypted workbook gets no save path, so Save never replaces the encrypted original. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
c9f03cc36a
commit
aad58d4c75
|
|
@ -25,4 +25,5 @@ gridcraft-pdf = { workspace = true }
|
|||
image = { workspace = true }
|
||||
|
||||
[dev-dependencies]
|
||||
gridcraft-xlsx = { workspace = true, features = ["test-support"] }
|
||||
zip = { workspace = true }
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@ use crate::DocState;
|
|||
pub fn specs() -> Vec<CommandSpec> {
|
||||
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<Json> {
|
|||
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<Json> {
|
|||
} 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<Json> {
|
|||
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::<Vec<_>>().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::<Vec<_>>().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));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<String>)> {
|
||||
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<String>)> {
|
||||
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<String>)> {
|
||||
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]
|
||||
|
|
|
|||
|
|
@ -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<u8> {
|
||||
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();
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue