From fbabac15d4df14ffa752d2589b15e6eeca160f34 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sun, 11 Oct 2026 21:12:24 +0200 Subject: [PATCH] Review: Protect Workbook dialog with an Excel-compatible password (#345) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ribbon button toggled structure protection silently. It now opens a Protect Structure and Windows dialog with an optional password; when a password was set, unprotecting asks for it and really verifies it (SHA-512, spin count 100000, salted). The password hash is read from and written to XLSX workbookProtection (workbookAlgorithmName/HashValue/ SaltValue/SpinCount or the legacy workbookPassword); an unreadable one stays locked. File › Info uses the same flow. Co-Authored-By: Claude Opus 5.5 --- ROADMAP.md | 1 + crates/engine/src/cmd/review.rs | 65 ++++++++++++++++++++++++++---- crates/model/src/workbook.rs | 4 ++ crates/ui-egui/src/backstage.rs | 16 +++++++- crates/ui-egui/src/dialogs.rs | 17 ++++++++ crates/ui-egui/src/ribbon.rs | 2 +- crates/xlsx/src/read.rs | 1 + crates/xlsx/src/sheet_read.rs | 44 +++++++++++++------- crates/xlsx/src/sheet_write.rs | 35 +++++++++------- crates/xlsx/src/tests/fixtures.rs | 5 ++- crates/xlsx/src/tests/roundtrip.rs | 3 ++ crates/xlsx/src/write.rs | 3 +- 12 files changed, 156 insertions(+), 40 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index a62418e..b927fc4 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -132,6 +132,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | What landed | |---|---| +| 2026-10-11 | Review › Protect Workbook opens a dialog with an optional password (Excel-compatible hash, kept in XLSX `workbookProtection`); unprotecting verifies it (#345) | | 2026-10-11 | Legacy Excel 97-2003 `.xls` (BIFF8) import with formulas, number formats, merges, sizes and names (#121) | | 2026-10-11 | XLSX pass-through: custom XML, external links, connections, query tables, printer settings, form/ActiveX controls and OLE objects are kept on save (#330, #324) | | 2026-10-11 | Data › Forecast › Forecast Sheet: a new sheet with the history, FORECAST.ETS forecast, confidence bounds, statistics and a chart (#322) | diff --git a/crates/engine/src/cmd/review.rs b/crates/engine/src/cmd/review.rs index 70823cd..8931bf5 100644 --- a/crates/engine/src/cmd/review.rs +++ b/crates/engine/src/cmd/review.rs @@ -19,7 +19,15 @@ pub fn specs() -> Vec { protect_sheet ), cmd!("review.unprotectSheet", "Unprotect Sheet", ["Review", "Protect"], None, "{password?}", has_doc, unprotect_sheet), - cmd!("review.protectWorkbook", "Protect Workbook", ["Review", "Protect"], None, "{on?: bool}", has_doc, protect_workbook), + cmd!( + "review.protectWorkbook", + "Protect Workbook", + ["Review", "Protect"], + None, + "{on?: bool (default: toggle), password?} (structure protection; unprotecting needs the password when one was set)", + has_doc, + protect_workbook + ), cmd!(query "review.workbookStatistics", "Workbook Statistics", ["Review", "Proofing"], None, "{}", has_doc, stats), // Not journaled: the params can hold a password. CommandSpec { @@ -159,6 +167,15 @@ fn unprotect_sheet(s: &mut Session, p: &Json) -> Result { }) } +/// A new Excel-compatible (SHA-512, salted) hash of `pw`. The salt mixes `what` is protected +/// with the document, a counter and the clock, so equal passwords get different hashes. +fn new_password_hash(d: &DocState, what: &str, pw: &str) -> PasswordHash { + static SALT_COUNTER: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); + let n = SALT_COUNTER.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + let seed = format!("{what}\u{0}{n}\u{0}{}\u{0}{}", d.uid, gridcraft_calc::now_serial().to_bits()); + PasswordHash::new_sha512(pw, &gridcraft_model::password::salt_from(seed.as_bytes())) +} + fn range_text(r: &EditRange) -> String { r.ranges.iter().map(|x| x.a1()).collect::>().join(",") } @@ -166,7 +183,6 @@ fn range_text(r: &EditRange) -> String { /// Review › Allow Edit Ranges: list, add, modify or delete the ranges that stay editable on a /// protected sheet, or unlock a password range for this session. fn allow_edit_ranges(s: &mut Session, p: &Json) -> Result { - static SALT_COUNTER: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); let sheet = target_sheet(s, p)?; let action = str_param(p, "action").unwrap_or("list"); let title = str_param(p, "title").map(str::trim).filter(|t| !t.is_empty()); @@ -180,11 +196,7 @@ fn allow_edit_ranges(s: &mut Session, p: &Json) -> Result { .position(|r| r.title.eq_ignore_ascii_case(t)) .ok_or_else(|| bad("review.allowEditRanges", format!("no range titled `{t}`"))) }; - let hash = |pw: &str, title: &str| { - let n = SALT_COUNTER.fetch_add(1, std::sync::atomic::Ordering::Relaxed); - let seed = format!("{sheet_name}\u{0}{title}\u{0}{n}\u{0}{}\u{0}{}", d.uid, gridcraft_calc::now_serial().to_bits()); - PasswordHash::new_sha512(pw, &gridcraft_model::password::salt_from(seed.as_bytes())) - }; + let hash = |pw: &str, title: &str| new_password_hash(d, &format!("{sheet_name}\u{0}{title}"), pw); if matches!(action, "add" | "modify" | "delete") && sh.is_protected() { return Err(EngineError::Other("Unprotect the sheet to change its edit ranges.".into())); } @@ -270,11 +282,28 @@ fn allow_edit_ranges(s: &mut Session, p: &Json) -> Result { )) } +/// Review › Protect Workbook: lock the workbook structure (sheets can't be added, moved, +/// renamed, hidden or deleted), optionally with a password that unprotecting must match. fn protect_workbook(s: &mut Session, p: &Json) -> Result { - let cur = s.doc()?.wb.protected_structure; + let d = s.doc()?; + let cur = d.wb.protected_structure; let on = bool_param(p, "on").unwrap_or(!cur); + if on == cur { + return Ok(json!({"on": on})); + } + let password = if on { + str_param(p, "password").filter(|pw| !pw.is_empty()).map(|pw| new_password_hash(d, "workbook", pw)) + } else { + if let Some(h) = &d.wb.structure_password + && !h.verify(str_param(p, "password").unwrap_or("")).map_err(EngineError::Other)? + { + return Err(EngineError::Other("The password you supplied is not correct.".into())); + } + None + }; edit(s, |cx| { cx.wb.protected_structure = on; + cx.wb.structure_password = password.clone(); Ok(json!({"on": on})) }) } @@ -355,6 +384,26 @@ mod tests { assert_eq!(at(&s), (0, "A1".to_string())); } + #[test] + fn protect_workbook_with_a_password_really_verifies() { + let mut s = book(); + s.execute("review.protectWorkbook", json!({"password": "pa55"})).unwrap(); + assert!(s.doc().unwrap().wb.protected_structure); + assert!(s.execute("home.insertSheet", json!({"name": "Three"})).is_err(), "the structure is locked"); + // A second Protect keeps it protected; unprotecting needs the right password. + assert_eq!(s.execute("review.protectWorkbook", json!({"on": true})).unwrap()["on"], true); + assert!(s.execute("review.protectWorkbook", json!({})).is_err()); + assert!(s.execute("review.protectWorkbook", json!({"on": false, "password": "pass"})).is_err()); + assert!(s.doc().unwrap().wb.protected_structure); + s.execute("review.protectWorkbook", json!({"on": false, "password": "pa55"})).unwrap(); + let wb = &s.doc().unwrap().wb; + assert!(!wb.protected_structure && wb.structure_password.is_none()); + // Without a password it toggles freely. + s.execute("review.protectWorkbook", json!({})).unwrap(); + s.execute("review.protectWorkbook", json!({})).unwrap(); + assert!(!s.doc().unwrap().wb.protected_structure); + } + #[test] fn convert_to_comments_turns_every_note_into_a_comment_and_undoes() { let mut s = book(); diff --git a/crates/model/src/workbook.rs b/crates/model/src/workbook.rs index 7e03cb3..6ea5f99 100644 --- a/crates/model/src/workbook.rs +++ b/crates/model/src/workbook.rs @@ -67,6 +67,9 @@ pub struct Workbook { pub props: DocProps, /// Workbook structure protection. pub protected_structure: bool, + /// Password of the structure protection (XLSX `workbookProtection`), never the password. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub structure_password: Option, /// Custom fill lists (Options › Custom Lists) beyond the built-in ones. pub custom_lists: Vec>, /// Named cell styles (Cell Styles gallery): name → style. @@ -95,6 +98,7 @@ impl Workbook { active_sheet: 0, props: DocProps::default(), protected_structure: false, + structure_password: None, custom_lists: vec![], cell_styles: vec![], preserved: None, diff --git a/crates/ui-egui/src/backstage.rs b/crates/ui-egui/src/backstage.rs index 32000f7..07e5705 100644 --- a/crates/ui-egui/src/backstage.rs +++ b/crates/ui-egui/src/backstage.rs @@ -422,6 +422,20 @@ fn open_page(app: &mut SheetApp, ui: &mut Ui, out: &mut Outcome) { recent_list(app, ui, out, usize::MAX); } +/// Review › Protect Workbook and File › Info › Protect Workbook: the protect dialog (with an +/// optional password) when the structure is open; when it's locked, the password prompt if one +/// was set, else unprotect right away. +pub fn protect_workbook(app: &mut SheetApp) { + let (on, password) = app.session.active().map(|d| (d.wb.protected_structure, d.wb.structure_password.is_some())).unwrap_or((false, false)); + if !on { + app.open_dialog("protectWorkbook", json!({})); + } else if password { + app.open_dialog("unprotectWorkbook", json!({})); + } else { + app.run_or_alert("review.protectWorkbook", json!({"on": false})); + } +} + fn info(app: &mut SheetApp, ui: &mut Ui, bs: &mut Backstage) { let t = Tokens::get(ui.ctx()); let lang = app.ui.language; @@ -451,7 +465,7 @@ fn info(app: &mut SheetApp, ui: &mut Ui, bs: &mut Backstage) { ui.add_space(18.0); ui.horizontal(|ui| { if square_button(ui, Icon::Lock, lang.tr("Protect Workbook")).clicked() { - app.run_or_alert("review.protectWorkbook", json!({"on": !protected})); + protect_workbook(app); } ui.add_space(12.0); ui.vertical(|ui| { diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 196d47a..0291ebb 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -219,6 +219,23 @@ impl Dialog { json!({"action": "add", "range": sel}), ) } + "protectWorkbook" => Dialog::form( + "protectWorkbook", + "Protect Structure and Windows", + "review.protectWorkbook", + vec![ + Password { key: "password", label: "Password (optional):" }, + Note("Protect workbook for: Structure (sheets can't be added, moved, renamed, hidden or deleted).".into()), + ], + json!({"on": true}), + ), + "unprotectWorkbook" => Dialog::form( + "unprotectWorkbook", + "Unprotect Workbook", + "review.protectWorkbook", + vec![Password { key: "password", label: "Password:" }], + json!({"on": false}), + ), "unprotectSheet" => Dialog::form( "unprotectSheet", "Unprotect Sheet", diff --git a/crates/ui-egui/src/ribbon.rs b/crates/ui-egui/src/ribbon.rs index ccc51eb..af602b2 100644 --- a/crates/ui-egui/src/ribbon.rs +++ b/crates/ui-egui/src/ribbon.rs @@ -1714,7 +1714,7 @@ fn review(app: &mut SheetApp, ui: &mut Ui) { } } if big_button(ui, Icon::Book, "Protect\nWorkbook", "Protect Workbook structure", false).clicked() { - act(app, "review.protectWorkbook", json!({})); + crate::backstage::protect_workbook(app); } if big_button(ui, Icon::Validation, "Allow Edit\nRanges", "Allow Edit Ranges", false).clicked() { app.open_dialog("allowEditRanges", json!({})); diff --git a/crates/xlsx/src/read.rs b/crates/xlsx/src/read.rs index 7c00d01..d1da00a 100644 --- a/crates/xlsx/src/read.rs +++ b/crates/xlsx/src/read.rs @@ -185,6 +185,7 @@ pub fn read_xlsx(bytes: &[u8]) -> Result<(Workbook, ReadReport), IoError> { } if let Some(p) = wb_xml.child("workbookProtection") { wb.protected_structure = p.flag("lockStructure", false); + wb.structure_password = if wb.protected_structure { crate::sheet_read::read_password(p, "workbook") } else { None }; } let active_tab = wb_xml.path(&["bookViews", "workbookView"]).and_then(|v| v.attr_u32("activeTab")).unwrap_or(0) as usize; diff --git a/crates/xlsx/src/sheet_read.rs b/crates/xlsx/src/sheet_read.rs index 07ee340..2c33d1f 100644 --- a/crates/xlsx/src/sheet_read.rs +++ b/crates/xlsx/src/sheet_read.rs @@ -208,6 +208,35 @@ pub fn read_sheet(cx: &mut Ctx<'_>, part: &str, name: &str) -> Result Option { + let attr = |base: &str| e.attr(&password_attr(prefix, base)); + if attr("algorithmName").is_some() || attr("hashValue").is_some() { + return Some(PasswordHash::Iterated { + algorithm: attr("algorithmName").unwrap_or("").to_string(), + hash: attr("hashValue").unwrap_or("").to_string(), + salt: attr("saltValue").unwrap_or("").to_string(), + spin_count: e.attr_u32(&password_attr(prefix, "spinCount")).unwrap_or(0), + }); + } + attr("password").map(|h| match u16::from_str_radix(h.trim(), 16) { + Ok(h) => PasswordHash::Legacy(h), + Err(_) => PasswordHash::Iterated { algorithm: String::new(), hash: String::new(), salt: String::new(), spin_count: 0 }, + }) +} + +/// `base` preceded by `prefix` in camel case: ("workbook", "hashValue") → `workbookHashValue`. +pub(crate) fn password_attr(prefix: &str, base: &str) -> String { + let mut chars = base.chars(); + match chars.next() { + Some(c) if !prefix.is_empty() => format!("{prefix}{}{}", c.to_ascii_uppercase(), chars.as_str()), + _ => base.to_string(), + } +} + /// Allow Edit Ranges. A range whose password can't be read keeps a hash that never matches /// (or an unsupported algorithm), so it stays locked rather than open. fn read_protected_ranges(e: &El, sheet: &mut Sheet) { @@ -216,20 +245,7 @@ fn read_protected_ranges(e: &El, sheet: &mut Sheet) { if ranges.is_empty() { continue; } - let password = if p.attr("algorithmName").is_some() || p.attr("hashValue").is_some() { - Some(PasswordHash::Iterated { - algorithm: p.attr("algorithmName").unwrap_or("").to_string(), - hash: p.attr("hashValue").unwrap_or("").to_string(), - salt: p.attr("saltValue").unwrap_or("").to_string(), - spin_count: p.attr_u32("spinCount").unwrap_or(0), - }) - } else { - p.attr("password").map(|h| match u16::from_str_radix(h.trim(), 16) { - Ok(h) => PasswordHash::Legacy(h), - // Unreadable: keep it locked. - Err(_) => PasswordHash::Iterated { algorithm: String::new(), hash: String::new(), salt: String::new(), spin_count: 0 }, - }) - }; + let password = read_password(p, ""); let title = p.attr("name").unwrap_or("").to_string(); sheet.edit_ranges.push(EditRange { title, ranges, password, security_descriptor: p.attr("securityDescriptor").map(str::to_string) }); } diff --git a/crates/xlsx/src/sheet_write.rs b/crates/xlsx/src/sheet_write.rs index fee804c..acc98f6 100644 --- a/crates/xlsx/src/sheet_write.rs +++ b/crates/xlsx/src/sheet_write.rs @@ -1118,26 +1118,33 @@ fn threaded_xml(sheet: &Sheet, ids: &BTreeMap>, persons: &m s } +/// A protection password hash as attributes, each name preceded by `prefix` (see +/// [`crate::sheet_read::read_password`]). +pub(crate) fn password_attrs(h: &gridcraft_model::PasswordHash, prefix: &str) -> String { + let name = |base: &str| crate::sheet_read::password_attr(prefix, base); + match h { + gridcraft_model::PasswordHash::Legacy(h) => format!(" {}=\"{h:04X}\"", name("password")), + gridcraft_model::PasswordHash::Iterated { algorithm, hash, salt, spin_count } => format!( + " {}=\"{}\" {}=\"{}\" {}=\"{}\" {}=\"{spin_count}\"", + name("algorithmName"), + esc_attr(algorithm), + name("hashValue"), + esc_attr(hash), + name("saltValue"), + esc_attr(salt), + name("spinCount") + ), + } +} + /// Allow Edit Ranges: `` (after `sheetProtection`). fn protected_ranges_xml(sheet: &Sheet) -> String { let mut s = String::new(); for r in sheet.edit_ranges.iter().filter(|r| !r.ranges.is_empty()) { let sqref: Vec = r.ranges.iter().map(|x| x.a1()).collect(); let _ = write!(s, " { - let _ = write!(s, " password=\"{h:04X}\""); - } - Some(gridcraft_model::PasswordHash::Iterated { algorithm, hash, salt, spin_count }) => { - let _ = write!( - s, - " algorithmName=\"{}\" hashValue=\"{}\" saltValue=\"{}\" spinCount=\"{spin_count}\"", - esc_attr(algorithm), - esc_attr(hash), - esc_attr(salt) - ); - } - None => {} + if let Some(h) = &r.password { + s.push_str(&password_attrs(h, "")); } if let Some(sd) = &r.security_descriptor { let _ = write!(s, " securityDescriptor=\"{}\"", esc_attr(sd)); diff --git a/crates/xlsx/src/tests/fixtures.rs b/crates/xlsx/src/tests/fixtures.rs index 5533e01..42adcb5 100644 --- a/crates/xlsx/src/tests/fixtures.rs +++ b/crates/xlsx/src/tests/fixtures.rs @@ -48,7 +48,7 @@ fn shared_formulas_and_value_types() { #[test] fn workbook_settings_names_and_states() { - let wb_extra = r#""#; + let wb_extra = r#""#; // Defined names must come after ; build the workbook by hand. let wbx = format!( r#"{wb_extra} @@ -69,6 +69,9 @@ fn workbook_settings_names_and_states() { assert_eq!(wb.sheets.len(), 3); assert_eq!(wb.date_system, DateSystem::D1904); assert!(wb.protected_structure); + // The legacy structure password ("password" hashes to 83AF) is kept and really verifies. + let pw = wb.structure_password.as_ref().unwrap(); + assert!(pw.verify("password").unwrap() && !pw.verify("Password").unwrap()); assert_eq!(wb.calc.mode, CalcMode::Manual); assert!(wb.calc.iterative); assert_eq!(wb.calc.max_iterations, 50); diff --git a/crates/xlsx/src/tests/roundtrip.rs b/crates/xlsx/src/tests/roundtrip.rs index 2385b8e..4038908 100644 --- a/crates/xlsx/src/tests/roundtrip.rs +++ b/crates/xlsx/src/tests/roundtrip.rs @@ -22,6 +22,7 @@ fn sample() -> Workbook { wb.date_system = DateSystem::D1904; wb.calc = CalcSettings { mode: CalcMode::Manual, iterative: true, max_iterations: 42, max_change: 0.5, precision_as_displayed: true }; wb.protected_structure = true; + wb.structure_password = Some(gridcraft_model::PasswordHash::new_sha512("s3cret", b"0123456789abcdef")); wb.props = DocProps { title: "T & ".into(), subject: "S".into(), @@ -412,6 +413,8 @@ fn full_roundtrip() { assert_eq!(back.date_system, wb.date_system); assert_eq!(back.calc, wb.calc); assert!(back.protected_structure); + assert_eq!(back.structure_password, wb.structure_password); + assert!(back.structure_password.as_ref().unwrap().verify("s3cret").unwrap()); assert_eq!(back.props, wb.props); assert_eq!(back.theme, wb.theme); assert_eq!(back.names, wb.names); diff --git a/crates/xlsx/src/write.rs b/crates/xlsx/src/write.rs index dc2ddc4..6aae60e 100644 --- a/crates/xlsx/src/write.rs +++ b/crates/xlsx/src/write.rs @@ -268,7 +268,8 @@ pub fn write_xlsx(wb: &Workbook) -> Result, IoError> { let mut w = format!("{XML_DECL}"); w.push_str(if wb.date_system == gridcraft_core::DateSystem::D1904 { "" } else { "" }); if wb.protected_structure { - w.push_str(""); + let pw = wb.structure_password.as_ref().map(|h| crate::sheet_write::password_attrs(h, "workbook")).unwrap_or_default(); + let _ = write!(w, ""); } let _ = write!(w, "{sheets_xml}"); w.push_str(&crate::passthrough::write_workbook(wb, &mut wb_rels, &mut out));