Review: mask the Protect / Unprotect Sheet password (#347)

The password fields in Protect Sheet and Unprotect Sheet were plain
text fields, so the password was readable on screen and in the
accessibility tree. A new Password form field masks it as typed and adds
a "Show password" check box that reveals it on purpose (off each time
the dialog opens).

These are the only password fields in the UI: Protect Workbook runs
without a dialog and encrypted files are refused on open.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

crates/ui-egui/src/dialogs.rs

@@ -13,6 +13,7 @@ use crate::theme;
 #[derive(Clone, Debug, PartialEq)]
 pub enum Field {
     Text { key: &'static str, label: &'static str },
+    Password { key: &'static str, label: &'static str },
     Number { key: &'static str, label: &'static str },
     Choice { key: &'static str, label: &'static str, options: Vec<(&'static str, &'static str)> },
     Check { key: &'static str, label: &'static str },
@@ -31,6 +32,8 @@ pub struct Dialog {
     pub search: String,
     pub list_index: usize,
     pub result: Option<Json>,
+    /// "Show password" is ticked: password fields show their text.
+    pub show_password: bool,
     picture_inbox: Option<crate::Inbox>,
     picture_target: Option<(u64, usize)>,
 }
@@ -51,6 +54,7 @@ impl Dialog {
             search: String::new(),
             list_index: 0,
             result: None,
+            show_password: false,
             picture_inbox: None,
             picture_target: None,
         }
@@ -67,6 +71,7 @@ impl Dialog {
             search: String::new(),
             list_index: 0,
             result: None,
+            show_password: false,
             picture_inbox: None,
             picture_target: None,
         }
@@ -151,7 +156,7 @@ impl Dialog {
                 "Protect Sheet",
                 "review.protectSheet",
                 vec![
-                    Text { key: "password", label: "Password (optional):" },
+                    Password { key: "password", label: "Password (optional):" },
                     Note("Allow all users of this sheet to:".into()),
                     Check { key: "formatCells", label: "Format cells" },
                     Check { key: "formatColumns", label: "Format columns" },
@@ -167,7 +172,7 @@ impl Dialog {
                 "unprotectSheet",
                 "Unprotect Sheet",
                 "review.unprotectSheet",
-                vec![Text { key: "password", label: "Password:" }],
+                vec![Password { key: "password", label: "Password:" }],
                 json!({}),
             ),
             "unhideSheet" => {
@@ -989,6 +994,17 @@ fn form(ui: &mut egui::Ui, d: &mut Dialog, confirm: &mut bool, open: &mut bool)
                         }
                     });
                 }
+                Field::Password { key, label } => {
+                    // Masked as typed (Excel shows dots); "Show password" reveals it on purpose.
+                    ui.vertical(|ui| {
+                        ui.label(label);
+                        let mut s = d.values.get(key).and_then(Json::as_str).unwrap_or_default().to_string();
+                        if ui.add(egui::TextEdit::singleline(&mut s).password(!d.show_password).desired_width(320.0)).changed() {
+                            d.values.insert(key.into(), json!(s));
+                        }
+                        ui.checkbox(&mut d.show_password, "Show password");
+                    });
+                }
                 Field::Number { key, label } => {
                     ui.horizontal(|ui| {
                         ui.label(label);

crates/ui-egui/tests/protect_password.rs

@@ -0,0 +1,70 @@
+//! Protect Sheet / Unprotect Sheet mask the password as it's typed (issue #347).
+
+use egui::accesskit::Role;
+use egui_kittest::kittest::Queryable as _;
+use gridcraft_engine::Session;
+use gridcraft_ui_egui::SheetApp;
+use serde_json::json;
+
+const SECRET: &str = "s3cret";
+
+fn harness() -> egui_kittest::Harness<'static, SheetApp> {
+    let mut session = Session::new();
+    session.new_workbook();
+    let mut h = egui_kittest::Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_ui_state(
+        |ui, app: &mut SheetApp| {
+            let ctx = ui.ctx().clone();
+            app.logic(&ctx);
+            app.ui(ui);
+        },
+        SheetApp::new(session, Default::default()),
+    );
+    h.run_steps(4);
+    h
+}
+
+fn type_password(h: &mut egui_kittest::Harness<'static, SheetApp>) {
+    h.get_by_role(Role::PasswordInput).click();
+    h.run_steps(2);
+    h.input_mut().events.push(egui::Event::Text(SECRET.into()));
+    h.run_steps(2);
+}
+
+fn secret_on_screen(h: &egui_kittest::Harness<'static, SheetApp>) -> bool {
+    h.query_all_by(|n| n.value().is_some_and(|v| v.contains(SECRET)) || n.label().is_some_and(|l| l.contains(SECRET))).next().is_some()
+}
+
+#[test]
+fn protect_and_unprotect_sheet_mask_the_password() {
+    let mut h = harness();
+    h.state_mut().open_dialog("protectSheet", json!({}));
+    h.run_steps(3);
+    type_password(&mut h);
+    assert_eq!(h.state().dialog.as_ref().and_then(|d| d.values.get("password")), Some(&json!(SECRET)));
+    assert!(!secret_on_screen(&h), "the typed password must not be readable");
+    assert!(h.get_by_role(Role::PasswordInput).value().is_some_and(|v| !v.is_empty() && !v.contains(SECRET)));
+
+    // Show password reveals it on purpose; unticking masks it again.
+    h.get_by_label("Show password").click();
+    h.run_steps(3);
+    assert!(h.query_by_role(Role::PasswordInput).is_none());
+    assert!(secret_on_screen(&h));
+    h.get_by_label("Show password").click();
+    h.run_steps(3);
+    assert!(h.query_by_role(Role::PasswordInput).is_some());
+    assert!(!secret_on_screen(&h));
+
+    h.get_by_label("   OK   ").click();
+    h.run_steps(3);
+    assert!(h.state().dialog.is_none());
+    assert!(h.state().session.active().unwrap().wb.active().unwrap().protection.as_ref().is_some_and(|p| p.password_hash.is_some()));
+
+    // Unprotect asks for the password with a masked field too, and it starts hidden again.
+    h.state_mut().open_dialog("unprotectSheet", json!({}));
+    h.run_steps(3);
+    type_password(&mut h);
+    assert!(!secret_on_screen(&h), "the typed password must not be readable");
+    h.get_by_label("   OK   ").click();
+    h.run_steps(3);
+    assert!(h.state().session.active().unwrap().wb.active().unwrap().protection.is_none());
+}