From 8587c2ba14433b10d3fbd6b9dc30b4c5c26a98dc Mon Sep 17 00:00:00 2001
From: vangel76 <vangel1976@googlemail.com>
Date: Sun, 11 Oct 2026 21:36:05 +0200
Subject: [PATCH] Review: Protect Sheet can't replace a sheet's password
 without unprotecting it

Re-protecting a password-protected sheet would drop its hash and let a
plain Unprotect succeed; it is now refused. The sheet tab menu offers
Unprotect Sheet on a protected sheet, like the ribbon.

Refs #346

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 crates/engine/src/cmd/review.rs | 9 +++++++++
 crates/ui-egui/src/tabs.rs      | 5 +++--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/crates/engine/src/cmd/review.rs b/crates/engine/src/cmd/review.rs
index 0dc5962..c55aac8 100644
--- a/crates/engine/src/cmd/review.rs
+++ b/crates/engine/src/cmd/review.rs
@@ -132,6 +132,12 @@ fn convert_notes(s: &mut Session, _: &Json) -> Result<Json> {
 
 fn protect_sheet(s: &mut Session, p: &Json) -> Result<Json> {
     let sheet = target_sheet(s, p)?;
+    // Re-protecting a password-protected sheet would replace the password without knowing it:
+    // unprotect it first, as in Excel. One without a password can take new options directly.
+    let has_password = |pr: &SheetProtection| pr.password.is_some() || pr.legacy_password_hash.is_some();
+    if s.doc()?.wb.sheet(sheet).and_then(|sh| sh.protection.as_ref()).is_some_and(has_password) {
+        return Err(EngineError::Other("The sheet is protected with a password. Unprotect it first (Review › Unprotect Sheet).".into()));
+    }
     let b = |k: &str| bool_param(p, k).unwrap_or(false);
     let prot = SheetProtection {
         password: None,
@@ -401,6 +407,9 @@ mod tests {
         assert!(matches!(&prot.password, Some(PasswordHash::Iterated { algorithm, .. }) if algorithm == "SHA-512"));
         assert!(s.execute("review.unprotectSheet", json!({})).is_err());
         assert!(s.execute("review.unprotectSheet", json!({"password": "pass"})).is_err());
+        // Protecting again can't swap the password out from under it.
+        assert!(s.execute("review.protectSheet", json!({})).is_err());
+        assert_eq!(s.doc().unwrap().wb.active().unwrap().protection.as_ref(), Some(&prot));
         s.execute("review.unprotectSheet", json!({"password": "pa55"})).unwrap();
         assert!(!s.doc().unwrap().wb.active().unwrap().is_protected());
         // A GridCraft JSON workbook saved before carried the internal hash under `password_hash`.
diff --git a/crates/ui-egui/src/tabs.rs b/crates/ui-egui/src/tabs.rs
index 6e68ab5..62a9940 100644
--- a/crates/ui-egui/src/tabs.rs
+++ b/crates/ui-egui/src/tabs.rs
@@ -164,8 +164,9 @@ pub fn sheet_tabs(app: &mut SheetApp, ui: &mut Ui) {
                                         app.run_or_alert("sheet.unhide", json!({}));
                                     }
                                     ui.separator();
-                                    if ui.button("Protect Sheet…").clicked() {
-                                        app.open_dialog("protectSheet", json!({}));
+                                    let protected = app.session.active().and_then(|d| d.wb.active().map(|s| s.is_protected())).unwrap_or(false);
+                                    if ui.button(if protected { "Unprotect Sheet…" } else { "Protect Sheet…" }).clicked() {
+                                        app.open_dialog(if protected { "unprotectSheet" } else { "protectSheet" }, json!({}));
                                     }
                                     if ui.button("Select All Sheets").clicked() {
                                         ui.close();