fix(engine): append a restored snapshot to its own source (#3276)

From review. When undo restores security from before the last save,
save_bytes now appends the snapshot to its own source instead of
writing it in full. That source has the snapshot's encryption and none
of the revisions being undone, so the save keeps its revisions, object
numbers and signatures; saves made after it are dropped, since undo
went back past them. A snapshot with no edits of its own is saved as
its source's bytes unchanged. Deleting a signed field lets security
change before an undo back to the signed state; that state now saves
as the signed file itself instead of being refused.

The output-security comparison drops file_key(): the dictionary and,
below revision 5, the first /ID determine the key.

crates/cos/src/document.rs

@@ -802,17 +802,17 @@ impl Document {
     }
 
     /// True when `self` and `other` are written with the same encryption: the same security
-    /// dictionary and file key, and below revision 5, where the first file identifier is part of
-    /// the key, the same identifier. Passwords and authentication level are not an encryption
-    /// identity: the same passwords can protect a different key, algorithm or permissions, and
-    /// user and owner openings share one. Where the dictionary is stored (its object number, or
-    /// inline) is not compared: [`Document::overlay_changes_from`] takes that from the document
-    /// it copies. A different file key always changes the dictionary too; comparing it as well
-    /// costs nothing.
+    /// dictionary, and below revision 5, where the first file identifier is part of the key, the
+    /// same identifier. Together they determine the file key: revisions 2–4 derive it from the
+    /// password, `/O`, `/P` and `/ID`, and revisions 5–6 wrap it in `/UE` and `/OE`. Passwords
+    /// and authentication level are not an encryption identity: the same passwords can protect
+    /// a different key, algorithm or permissions, and user and owner openings share one. Where
+    /// the dictionary is stored (its object number, or inline) is not compared:
+    /// [`Document::overlay_changes_from`] takes that from the document it copies.
     pub fn has_same_output_security(&self, other: &Document) -> bool {
         match (self.output_handler(), other.output_handler()) {
             (None, None) => true,
-            (Some(a), Some(b)) => a.dict() == b.dict() && a.file_key() == b.file_key() && (a.dict().r >= 5 || self.first_id() == other.first_id()),
+            (Some(a), Some(b)) => a.dict() == b.dict() && (a.dict().r >= 5 || self.first_id() == other.first_id()),
             _ => false,
         }
     }
@@ -1356,7 +1356,7 @@ impl Document {
     ///
     /// Returns `false`, changing nothing, when output security differs
     /// ([`Document::has_same_output_security`]): an update appended to this file would be read
-    /// under other security than the objects before it, so `current` must be written in full. Otherwise the `/Encrypt` dictionary
+    /// under other security than the objects before it, so append `current` to its own source. Otherwise the `/Encrypt` dictionary
     /// comes with `current`'s objects and trailer, and so does the number the writer leaves
     /// unencrypted, even where a full save had moved the dictionary.
     pub fn overlay_changes_from(&mut self, current: &Document) -> Result<bool, CosError> {

crates/cos/tests/it/encryption.rs

@@ -513,7 +513,8 @@ fn set_title(doc: &mut Document, title: &str) {
     }
 }
 
-/// Protect again with the same passwords and no restrictions; `seed` picks the key and `/ID`.
+/// Protect again with the same passwords and no restrictions. `seed` picks the AES-256 key, and
+/// the `/ID` when the document has none.
 fn rekey(doc: &mut Document, alg: Algorithm, seed: u8) {
     doc.set_encryption(&NewEncryption {
         algorithm: alg,

crates/engine/src/session.rs

@@ -665,10 +665,11 @@ impl Session {
     /// break its signatures, so with no edits it keeps its bytes, damage and all.
     ///
     /// Undo past a save that changed security restores a snapshot whose encryption differs from
-    /// the last saved file. Like any change of security, that snapshot is written in full: an
-    /// update appended to the saved file would be read with the wrong key, and would keep the
-    /// revision it was meant to replace (#3276). In a signed document that fails with
-    /// [`EditError::SignedRewrite`] rather than invalidate the signatures.
+    /// the last saved file. An update appended to that file would be read with the wrong key and
+    /// keep the revision it was meant to replace, so the snapshot is appended to its own source
+    /// instead, whose encryption it shares (#3276). That keeps the source's revisions, object
+    /// numbers and signatures. Revisions saved after that source are dropped: undo went back
+    /// past all of them. An unedited snapshot is its source's bytes as they are.
     pub fn save_bytes(&self, id: DocId) -> Result<Arc<Vec<u8>>, EditError> {
         let doc = self.get(id).ok_or(EditError::NoDocument)?;
         let Some(editor) = doc.editor.as_ref() else { return Ok(doc.bytes.clone()) };
@@ -679,29 +680,29 @@ impl Session {
         // What the user removed again before saving (a cleared redaction mark) stays out of the file.
         let opts = SaveOptions { mod_date: self.now().map(pdfcraft_cos::pdf_date), drop_unlinked: true, ..SaveOptions::default() };
         // A restored snapshot can be based on an earlier revision. Append to the last saved
-        // file so that revision stays in the chain. A full rewrite (redaction, new encryption,
-        // a reconstructed file) still writes the current graph on its own.
+        // file when it has the snapshot's security, so that revision stays in the chain. A full
+        // rewrite (redaction, new encryption, a reconstructed file) still writes the current
+        // graph on its own.
         let rewrite = editor.cos.full_save_required() || editor.cos.encryption_changed() || editor.cos.revisions().is_empty();
         let cos = writable(&editor.cos)?;
-        let signed = doc.is_signed();
-        let written = guard(|| {
+        guard(|| {
             if on_saved || rewrite {
-                return write_incremental(&cos, &opts).map(Some);
+                return write_incremental(&cos, &opts);
             }
             let mut base = pdfcraft_cos::Document::open_with_password(editor.saved.clone(), editor.saved_keys.reopen.as_deref())?;
-            // Undo may restore security from before the last save even though it has not changed
-            // relative to the snapshot's own source. That can't be appended to the saved file.
             if base.overlay_changes_from(&cos)? {
-                write_incremental(&base, &opts).map(Some)
-            } else if signed {
-                Ok(None)
+                write_incremental(&base, &opts)
+            } else if !cos.is_modified() {
+                Ok(cos.bytes().to_vec())
             } else {
-                write_full(&cos, &opts).map(Some)
+                // Undo restored security from before the last save: append the snapshot to its
+                // own source, which it hasn't changed security against.
+                write_incremental(&cos, &opts)
             }
         })
         .map_err(EditError::Write)?
-        .map_err(|e| EditError::Write(e.to_string()))?;
-        written.map(Arc::new).ok_or_else(|| EditError::SignedRewrite("Saving after undoing a security change".into()))
+        .map(Arc::new)
+        .map_err(|e| EditError::Write(e.to_string()))
     }
 
     /// A compact, garbage-collected rewrite (Save As ▸ "Optimized" / Reduce File Size groundwork).

crates/engine/src/tests/security_save.rs

@@ -69,7 +69,8 @@ fn undo_saved_security_removal_restores_encrypted_content() {
         for inline in [false, true] {
             for password in ["owner", "user"] {
                 let mut s = Session::new();
-                let id = s.open("encrypted.pdf", None, encrypted_fixture(algorithm, inline), Some(password)).unwrap();
+                let original = encrypted_fixture(algorithm, inline);
+                let id = s.open("encrypted.pdf", None, original.clone(), Some(password)).unwrap();
                 s.apply(id, Edit::RemoveProtection).unwrap();
                 let plain = s.save_bytes(id).unwrap();
                 assert_saved_content(&s, id, plain.clone(), None, false).unwrap();
@@ -81,6 +82,7 @@ fn undo_saved_security_removal_restores_encrypted_content() {
                     failures.push(format!("{algorithm:?}, inline={inline}, {password}: {error}"));
                     continue;
                 }
+                assert!(restored.starts_with(original.as_slice()), "appended to the encrypted original, not rewritten");
                 // Both original passwords still work; saving also retains this session's rights.
                 assert_saved_content(&s, id, restored.clone(), Some("user"), true).unwrap();
                 assert_saved_content(&s, id, restored.clone(), Some("owner"), true).unwrap();
@@ -103,6 +105,7 @@ fn undo_saved_protection_restores_plain_content() {
     let mut failures = Vec::new();
     for algorithm in [Algorithm::Rc4_128, Algorithm::Aes128, Algorithm::Aes256] {
         let (mut s, id) = session_with(2);
+        let original = s.get(id).unwrap().bytes.clone();
         s.apply(id, Edit::Protect(Protection { algorithm, ..protection(Some("user"), Some("owner")) })).unwrap();
         let protected = s.save_bytes(id).unwrap();
         assert_saved_content(&s, id, protected.clone(), Some("owner"), true).unwrap();
@@ -114,6 +117,7 @@ fn undo_saved_protection_restores_plain_content() {
             failures.push(format!("{algorithm:?}: {error}"));
             continue;
         }
+        assert!(restored.starts_with(original.as_slice()), "{algorithm:?}: appended to the plain original");
         s.mark_saved(id, restored.clone(), None).unwrap();
         assert!(!s.get(id).unwrap().dirty);
         assert_eq!(*s.save_bytes(id).unwrap(), *restored);
@@ -264,30 +268,59 @@ fn undo_saved_password_change_restores_original_passwords() {
     assert!(CosDocument::open_with_password(restored, Some("new-owner")).is_err());
 }
 
-/// A signed document is never rewritten in full, even by this fallback. Signing clears the undo
-/// history and security can't be changed once signed, so no edit reaches this state; the test
-/// marks the document signed directly.
+/// Undo past a security save back to a signed state, then save: the signature stays valid.
+/// Deleting a signed field unsigns the document, so security can change before the undo.
 #[test]
-fn undo_past_a_security_change_never_rewrites_a_signed_document() {
+fn undo_past_a_security_save_keeps_a_restored_signature_valid() {
     let p12 = std::fs::read(concat!(env!("CARGO_MANIFEST_DIR"), "/../sign/tests/data/ec-p256.p12")).unwrap();
     let digital_id = sign::pkcs12::open(&p12, "test").unwrap();
     // After the test certificate's start date (2026-10-02).
-    let mut signer = Session::new().with_clock(|| 1_800_000_000);
-    let signed_id = signer.open("signed.pdf", None, Arc::new(fixture(1)), None).unwrap();
-    let signed =
-        signer.sign(signed_id, &digital_id, SignOptions { page: 0, rect: Some([20.0, 20.0, 180.0, 60.0]), ..SignOptions::default() }).unwrap();
-    signer.mark_signed(signed_id, signed, None).unwrap();
-    let signatures = signer.get(signed_id).unwrap().signatures.clone();
-
-    let mut s = Session::new();
-    let id = s.open("encrypted.pdf", None, encrypted_fixture(Algorithm::Aes256, false), Some("owner")).unwrap();
-    s.apply(id, Edit::RemoveProtection).unwrap();
-    let plain = s.save_bytes(id).unwrap();
-    s.mark_saved(id, plain, None).unwrap();
+    let mut s = Session::new().with_clock(|| 1_800_000_000);
+    s.set_trusted_certificates(vec![digital_id.certificate.clone()]);
+    let id = s.open("fixture.pdf", None, Arc::new(fixture(1)), None).unwrap();
+    let signed = s.sign(id, &digital_id, SignOptions { page: 0, rect: Some([20.0, 20.0, 180.0, 60.0]), ..SignOptions::default() }).unwrap();
+    s.mark_signed(id, signed.clone(), None).unwrap();
+    let field = s.get(id).unwrap().signatures[0].field.clone();
+    s.apply(id, Edit::DeleteField { name: field }).unwrap();
+    assert!(!s.get(id).unwrap().is_signed());
+    s.apply(id, Edit::Protect(protection(Some("user"), Some("owner")))).unwrap();
+    let protected = s.save_bytes(id).unwrap();
+    s.mark_saved(id, protected, None).unwrap();
     s.undo(id).unwrap();
-    s.doc_mut(id).unwrap().signatures = signatures;
-    assert!(s.get(id).unwrap().is_signed());
-    assert!(matches!(s.save_bytes(id), Err(EditError::SignedRewrite(_))));
+    s.undo(id).unwrap();
+    assert!(s.get(id).unwrap().is_signed(), "undo restores the signed snapshot");
+    let restored = s.save_bytes(id).unwrap();
+    assert_eq!(*restored, *signed, "the signed file as it was, with no revision after the signature");
+    let mut u = Session::new().with_clock(|| 1_800_000_000);
+    u.set_trusted_certificates(vec![digital_id.certificate.clone()]);
+    let uid = u.open("restored.pdf", None, restored, None).unwrap();
+    assert_eq!(u.get(uid).unwrap().signatures[0].status, SignatureStatus::Valid);
+}
+
+/// The restored state can have unsaved edits of its own: they are appended to its source, in
+/// that source's security, and the undone revision is still left out.
+#[test]
+fn undo_past_a_security_save_keeps_the_snapshot_edits() {
+    for encrypted in [false, true] {
+        let mut s = Session::new();
+        let (original, password) =
+            if encrypted { (encrypted_fixture(Algorithm::Aes256, false), Some("owner")) } else { (Arc::new(fixture(2)), None) };
+        let id = s.open("a.pdf", None, original.clone(), password).unwrap();
+        s.apply(id, Edit::RotatePages { pages: vec![0], degrees: 90 }).unwrap();
+        let change = if encrypted { Edit::RemoveProtection } else { Edit::Protect(protection(Some("user"), Some("owner"))) };
+        s.apply(id, change).unwrap();
+        let changed = s.save_bytes(id).unwrap();
+        s.mark_saved(id, changed, None).unwrap();
+        s.undo(id).unwrap();
+        let restored = s.save_bytes(id).unwrap();
+        assert!(restored.starts_with(original.as_slice()) && restored.len() > original.len(), "{encrypted}: appended to the source");
+        let mut reopened = Session::new();
+        let rid = reopened.open("restored.pdf", None, restored.clone(), password).unwrap();
+        let doc = reopened.get(rid).unwrap();
+        assert_eq!((doc.info.encrypted, doc.info.pages[0].rotation), (encrypted, 90), "{encrypted}");
+        assert_eq!(page_texts(&reopened, rid), ["Page 1", "Page 2"], "{encrypted}");
+        assert_eq!(restored.windows(b"(Page 1)".len()).any(|w| w == b"(Page 1)"), !encrypted, "{encrypted}: plaintext only in a plain file");
+    }
 }
 
 /// An optimized save packs objects into object streams under new numbers. Undo past it restores