From eafa1a1d06c4ee8f0605839db96842181ffad0f2 Mon Sep 17 00:00:00 2001
From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com>
Date: Thu, 22 Oct 2026 22:20:49 +0200
Subject: [PATCH] Never overwrite a file's log when it can't be read

Fixes #1090

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 apps/figcraft-server/src/store.rs       | 19 ++++++++-
 apps/figcraft-server/src/store_tests.rs | 51 +++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/apps/figcraft-server/src/store.rs b/apps/figcraft-server/src/store.rs
index ea9bc626..c6d62462 100644
--- a/apps/figcraft-server/src/store.rs
+++ b/apps/figcraft-server/src/store.rs
@@ -1110,7 +1110,24 @@ impl Store {
         let mut issues = Vec::new();
         let mut fresh_log = true;
         let mut kept = 0;
-        match disk::read_capped(&dir.join(LOG), self.cfg.max_file_bytes).and_then(|b| disk::read_log(&b)) {
+        let log_path = dir.join(LOG);
+        // A log that's there but can't be read (an IO error, or over a lowered cap) may hold
+        // confirmed edits: the file isn't opened, and the log is left as it is for another try.
+        let read = match disk::read_capped(&log_path, self.cfg.max_file_bytes) {
+            Err(why) if log_path.exists() => return Err(format!("its log can't be read ({why}); it was left as it is")),
+            r => r,
+        };
+        // One that isn't a log is moved aside before a new one replaces it (or the file isn't opened).
+        let read = match read.map(|b| disk::read_log(&b)) {
+            Ok(Err(why)) => {
+                let aside = dir.join(format!("{LOG}.unreadable-{generation}-{}", now()));
+                fs::rename(&log_path, &aside).map_err(|e| format!("{why}, and it couldn't be moved aside ({e})"))?;
+                Err(format!("{why}; it was moved aside to {}", aside.display()))
+            }
+            Ok(Ok(log)) => Ok(log),
+            Err(why) => Err(why),
+        };
+        match read {
             Ok(log) if log.generation == generation => {
                 fresh_log = false;
                 for entry in log.entries {
diff --git a/apps/figcraft-server/src/store_tests.rs b/apps/figcraft-server/src/store_tests.rs
index f380568c..e7434b45 100644
--- a/apps/figcraft-server/src/store_tests.rs
+++ b/apps/figcraft-server/src/store_tests.rs
@@ -313,3 +313,54 @@ fn edits_confirmed_after_a_compaction_cut_short_survive_a_restart() {
     let name = page_name(&restored);
     assert!(at(&name) >= at(&confirmed), "restored {name:?}, confirmed {confirmed:?}");
 }
+
+#[test]
+fn a_log_that_cant_be_read_is_never_overwritten() {
+    let dir = TempDir::new("unreadlog");
+    let c = StoreConfig { compact_after: 1000, ..cfg(dir.path()) };
+    let log_path = dir.path().join(FILES);
+    let last = "Confirmed page name number 39 with some padding";
+    let id = {
+        let mut store = Store::open(dir.path(), c.clone()).unwrap();
+        let (id, _, _) = publish(&mut store, "Busy");
+        let mut server = store.open(&id).unwrap();
+        let mut a = join(&mut server, Access::Edit);
+        store.changed(&id, &mut server);
+        for i in 0..40 {
+            rename(&mut server, &mut a, &format!("Confirmed page name number {i} with some padding"));
+            store.changed(&id, &mut server);
+        }
+        assert!(store.flush().is_empty());
+        id
+    };
+    let log_path = log_path.join(&id).join(LOG);
+    let before = fs::read(&log_path).unwrap();
+    // The cap lowered just under the log: it can't be read now, but it must still be there after.
+    let lowered = StoreConfig { max_file_bytes: before.len() as u64 - 1, ..c.clone() };
+    {
+        let mut store = Store::open(dir.path(), lowered).unwrap();
+        let opened = store.open(&id);
+        assert!(opened.is_err(), "the file isn't opened without its log");
+        assert!(!store.problems().is_empty(), "the problem is reported");
+        assert_eq!(fs::read(&log_path).unwrap(), before, "the log is left as it was");
+    }
+    let mut store = Store::open(dir.path(), c).unwrap();
+    let server = store.open(&id).unwrap();
+    assert_eq!(page_name(&server), last, "every confirmed edit comes back");
+}
+
+#[test]
+fn a_log_that_isnt_one_is_moved_aside() {
+    let dir = TempDir::new("badlog");
+    let mut store = Store::open(dir.path(), cfg(dir.path())).unwrap();
+    let (id, _, _) = publish(&mut store, "Busy");
+    let folder = dir.path().join(FILES).join(&id);
+    let junk = b"not a log at all, but maybe worth keeping".to_vec();
+    fs::write(folder.join(LOG), &junk).unwrap();
+    drop(store);
+    let mut store = Store::open(dir.path(), cfg(dir.path())).unwrap();
+    store.open(&id).unwrap();
+    assert!(store.problems().iter().any(|p| p.contains("moved aside")), "{:?}", store.problems());
+    let kept = fs::read_dir(&folder).unwrap().filter_map(|e| e.ok()).any(|e| fs::read(e.path()).is_ok_and(|b| b == junk));
+    assert!(kept, "the old log's bytes are kept beside the file");
+}