Issue #813: a cancelled recovery password no longer attaches to the next unlocked document (#858)

Recovering an encrypted snapshot parked its metadata in `pending_recovered`
until the password prompt was answered. Cancel left it there, and the next
encrypted document unlocked in the foreground took the snapshot's original
path and recovery key: Save would overwrite the other file and delete its
snapshot.

The association now travels with the prompt (`PasswordPrompt::recovered`).
Cancel puts the entry back on offer and leaves the store alone; a wrong
password keeps it on the re-raised prompt; the correct one finishes the
recovery of the tab that was just opened. `pending_recovered` is gone.

Test: crash with an encrypted snapshot of A, Recover, Cancel, open encrypted
B with its password. B keeps its own path and is clean, A's snapshot stays
in the store, also after closing B. Fails on main at "B keeps its own path".

Co-authored-by: Tristan von Kries <[email protected]>
Co-authored-by: Claude Fable 5.1 <[email protected]>

crates/ui-egui/src/lib.rs

@@ -349,6 +349,9 @@ pub struct PasswordPrompt {
     pub error: Option<String>,
     /// A late startup file must stay in the background even after it is unlocked.
     activate: bool,
+    /// The bytes are a recovery snapshot: once the document is open it takes the snapshot's
+    /// path and recovery entry. Cancelling the prompt drops the association with it (#813).
+    recovered: Option<RecoveryMeta>,
 }
 
 #[derive(serde::Serialize, serde::Deserialize)]
@@ -525,7 +528,6 @@ pub struct PdfCraftApp {
     pub recoverable: Vec<RecoveryMeta>,
     recovery_keys: std::collections::HashMap<DocId, String>,
     last_autosave: f64,
-    pending_recovered: Option<RecoveryMeta>,
     allow_quit: bool,
     /// Shortcuts pressed while a text field had the keyboard, run on the next frame (see
     /// `registry_shortcuts`).
@@ -751,7 +753,6 @@ impl PdfCraftApp {
             recoverable: Vec::new(),
             recovery_keys: Default::default(),
             last_autosave: 0.0,
-            pending_recovered: None,
             allow_quit: false,
             deferred_commands: Vec::new(),
             dialog_seen: None,
@@ -869,7 +870,8 @@ impl PdfCraftApp {
             Ok(id) => id,
             Err(e @ (OpenError::NeedsPassword | OpenError::WrongPassword)) => {
                 let error = matches!(e, OpenError::WrongPassword).then(|| "Incorrect password. Try again.".to_string());
-                self.password_prompt = Some(PasswordPrompt { name: name.to_string(), path, bytes, input: String::new(), error, activate });
+                self.password_prompt =
+                    Some(PasswordPrompt { name: name.to_string(), path, bytes, input: String::new(), error, activate, recovered: None });
                 return Ok(());
             }
             Err(e) => return Err(e.to_string()),
@@ -1050,17 +1052,28 @@ impl PdfCraftApp {
     /// Answer the password prompt (`None` cancels).
     pub fn submit_password(&mut self, password: Option<String>) {
         let Some(p) = self.password_prompt.take() else { return };
-        let Some(pw) = password else { return };
+        let Some(pw) = password else {
+            // A cancelled recovery goes back on offer; its snapshot stays in the store and must
+            // not attach itself to whatever document is unlocked next (#813).
+            if let Some(meta) = p.recovered {
+                self.recoverable.push(meta);
+            }
+            return;
+        };
         match self.try_open(&p.name, p.path, p.bytes, Some(&pw), p.activate) {
             Err(e) => self.notify_fmt("Couldn't open {name}: {e}", &[("name", &p.name), ("e", &e.to_string())]),
-            // A recovered encrypted document is open once its foreground prompt is gone.
-            // Unlocking a background startup file must not finish another file's recovery.
-            Ok(()) if p.activate && self.password_prompt.is_none() => {
-                if let Some(meta) = self.pending_recovered.clone() {
-                    self.finish_recovery(&meta);
+            // A recovered encrypted document is open once its prompt is gone.
+            Ok(()) if self.password_prompt.is_none() => {
+                if let Some(meta) = &p.recovered {
+                    self.finish_recovery(meta);
+                }
+            }
+            // Wrong password: the new prompt is for the same bytes, so it keeps the snapshot.
+            Ok(()) => {
+                if let Some(prompt) = self.password_prompt.as_mut() {
+                    prompt.recovered = p.recovered;
                 }
             }
-            Ok(()) => {}
         }
     }
 

crates/ui-egui/src/recovery.rs

@@ -170,13 +170,16 @@ impl PdfCraftApp {
                     continue;
                 }
             };
-            if meta.encrypted {
-                // The password prompt opens it; mark it recovered once it is open.
-                self.pending_recovered = Some(meta.clone());
-            }
             match self.open_bytes(&meta.name, None, bytes) {
                 Ok(()) if self.password_prompt.is_none() => self.finish_recovery(&meta),
-                Ok(()) => {}
+                // Encrypted: the password prompt opens it, and finishes its recovery once it
+                // does. The association travels with the prompt, so cancelling it cannot hand
+                // this snapshot's path to the next document that is unlocked (#813).
+                Ok(()) => {
+                    if let Some(prompt) = self.password_prompt.as_mut() {
+                        prompt.recovered = Some(meta.clone());
+                    }
+                }
                 Err(e) => self.notify_fmt("Couldn't recover {name}: {e}", &[("name", &meta.name), ("e", &e.to_string())]),
             }
         }
@@ -185,10 +188,9 @@ impl PdfCraftApp {
 
     /// The most recently opened tab came from `meta`: keep its recovery entry and path.
     pub(crate) fn finish_recovery(&mut self, meta: &RecoveryMeta) {
-        let Some((_, id)) = self.active_ids() else { return };
+        let Some(id) = self.views.last().map(|v| v.id) else { return };
         self.session.mark_recovered(id, meta.path.clone());
         self.recovery_keys.insert(id, meta.key.clone());
-        self.pending_recovered = None;
     }
 
     /// Delete recovery entries the user chose not to recover.

crates/ui-egui/tests/recovery.rs

@@ -153,19 +153,24 @@ fn quitting_cleanly_leaves_nothing_behind() {
     assert_eq!(files_in(&s), 0);
 }
 
-#[test]
-fn encrypted_documents_are_autosaved_encrypted_and_recovered_with_the_password() {
-    let mut doc = pdfcraft_cos::Document::open(std::sync::Arc::new(fixture(2))).unwrap();
+/// `bytes` protected with an open password (AES-256).
+fn encrypted(bytes: Vec<u8>, password: &str) -> Vec<u8> {
+    let mut doc = pdfcraft_cos::Document::open(std::sync::Arc::new(bytes)).unwrap();
     doc.set_encryption(&pdfcraft_cos::NewEncryption {
         algorithm: pdfcraft_cos::Algorithm::Aes256,
-        user_password: "pw",
+        user_password: password,
         owner_password: "owner",
         permissions: -1,
         encrypt_metadata: true,
         seed: [2; 32],
     })
     .unwrap();
-    let bytes = pdfcraft_cos::write_full(&doc, &Default::default()).unwrap();
+    pdfcraft_cos::write_full(&doc, &Default::default()).unwrap()
+}
+
+#[test]
+fn encrypted_documents_are_autosaved_encrypted_and_recovered_with_the_password() {
+    let bytes = encrypted(fixture(2), "pw");
     let s = store("encrypted");
     crashed_session(&s, bytes, Some("pw"), None);
     let meta = s.list().remove(0);
@@ -190,6 +195,42 @@ fn encrypted_documents_are_autosaved_encrypted_and_recovered_with_the_password()
     let _ = Modifiers::NONE;
 }
 
+/// #813: cancelling the password prompt of a recovered snapshot must not hand its path and
+/// recovery entry to the next encrypted document that is unlocked (Save would then overwrite
+/// the snapshot's original file and delete the snapshot).
+#[test]
+fn a_cancelled_recovery_password_does_not_attach_to_the_next_unlocked_document() {
+    let s = store("cancelled");
+    crashed_session(&s, encrypted(fixture(2), "pw-a"), Some("pw-a"), Some("/docs/A.pdf"));
+    assert_eq!(s.list().len(), 1);
+    let mut h = harness(s.clone());
+    h.get_by_label("Recover").click();
+    h.run_steps(3);
+    assert!(h.state().password_prompt.is_some(), "the snapshot asks for its password");
+    h.get_by_label("Cancel").click();
+    h.run_steps(3);
+    assert!(h.state().password_prompt.is_none());
+    assert!(h.state().views.is_empty());
+
+    let app = h.state_mut();
+    app.open_bytes("B.pdf", Some("/docs/B.pdf".into()), encrypted(fixture(1), "pw-b")).unwrap();
+    app.submit_password(Some("pw-b".into()));
+    h.run_steps(3);
+    let app = h.state();
+    assert_eq!(app.views.len(), 1);
+    let b = app.session.get(app.views[0].id).unwrap();
+    assert_eq!(b.path.as_deref(), Some("/docs/B.pdf"), "B keeps its own path");
+    assert!(!b.dirty, "B is clean: it is not the recovered document");
+    assert_eq!(s.list().len(), 1, "A's snapshot is still in the store");
+    assert_eq!(app.recoverable.len(), 1, "and on offer again");
+
+    // Closing B (clean, so no prompt) must not remove A's snapshot either.
+    h.state_mut().execute("file.close");
+    h.run_steps(3);
+    assert!(h.state().views.is_empty());
+    assert_eq!(s.list().len(), 1, "A's snapshot survives closing B");
+}
+
 #[test]
 fn incomplete_entries_are_ignored_and_cleaned_up() {
     let s = store("partial");