Merge PR #2149: Refuse catalog snapshots with an out-of-range seq

crates/catalog/src/journal.rs

@@ -271,7 +271,7 @@ fn resync(line: &[u8], last: u64) -> Option<(usize, (u64, Op))> {
     const START: &[u8] = b"{\"seq\":";
     (1..line.len().saturating_sub(START.len() - 1)).filter(|&i| line[i..].starts_with(START)).find_map(|i| {
         let rec = std::str::from_utf8(&line[i..]).ok().map(|l| l.trim_end_matches('\r')).and_then(decode_record)?;
-        (rec.0 <= last + 1).then_some((i, rec))
+        (rec.0 <= last.saturating_add(1)).then_some((i, rec))
     })
 }
 
@@ -298,6 +298,9 @@ fn write_snapshot(store: &mut dyn Store, seq: u64, catalog: &Catalog) -> std::io
     Ok(SnapshotTiming { serialize_ms, write_sync_ms: (ms_since(t0) - serialize_ms).max(0.0), bytes, ..Default::default() })
 }
 
+/// The highest `seq` a snapshot may hold; beyond it the counter is treated as corrupt, far from overflow.
+const MAX_SEQ: u64 = u64::MAX / 2;
+
 /// Just the identification of `catalog.snap` (read before the catalog itself).
 #[derive(serde::Deserialize)]
 struct SnapHeader {
@@ -331,6 +334,10 @@ impl Journal {
                     return Err(newer(format!("{SNAPSHOT} is catalog format v{}", h.version)));
                 }
                 let s: SnapFile = serde_json::from_slice(&bytes).map_err(|e| CatalogError::Corrupt(format!("{SNAPSHOT}: {e}")))?;
+                // a seq this high can't come from real use; the next record's seq would wrap
+                if s.seq > MAX_SEQ {
+                    return Err(CatalogError::Corrupt(format!("{SNAPSHOT}: seq {} is out of range", s.seq)));
+                }
                 let mut catalog = s.catalog;
                 report.repaired = catalog.repair();
                 for r in &report.repaired {
@@ -392,7 +399,7 @@ impl Journal {
                     let _ = op;
                     report.stale += 1;
                 }
-                Some((seq, op)) if seq == j.seq + 1 => {
+                Some((seq, op)) if j.seq.checked_add(1) == Some(seq) => {
                     if catalog.apply(op).is_err() {
                         report.failed += 1;
                     } else {
@@ -476,7 +483,7 @@ impl Journal {
         let mut buf = String::new();
         let mut seq = self.seq;
         for op in ops {
-            seq += 1;
+            seq = seq.checked_add(1).ok_or_else(|| CatalogError::Corrupt(format!("{LOG}: op counter overflow")))?;
             buf.push_str(&encode_record(seq, op));
             buf.push('\n');
         }
@@ -527,7 +534,7 @@ impl Journal {
             log::warn!("catalog: background snapshot failed ({e}); writing one now");
         }
         let t0 = web_time::Instant::now();
-        let seq = self.seq + unlogged;
+        let seq = self.seq.checked_add(unlogged).ok_or_else(|| CatalogError::Corrupt(format!("{SNAPSHOT}: op counter overflow")))?;
         let mut timing = match write_snapshot(self.store.as_mut(), seq, catalog) {
             Ok(t) => t,
             Err(e) => {

crates/catalog/src/tests_journal.rs

@@ -376,3 +376,18 @@ fn a_damaged_process_value_loads_as_v1_with_the_edits() {
     let d = &loaded.photo(id).unwrap().develop;
     assert_eq!((d.process, d.light.exposure), (ProcessVersion::V1, 0.8));
 }
+
+/// A snapshot whose `seq` is near `u64::MAX` is refused as corrupt: the next record's seq would
+/// wrap (and every later edit would be skipped as stale on reopen) or overflow-panic in debug.
+#[test]
+fn snapshot_with_seq_near_u64_max_is_corrupt() {
+    for seq in [u64::MAX, u64::MAX - 1, u64::MAX / 2 + 1] {
+        let m = MemStore::new();
+        m.set(
+            SNAPSHOT,
+            format!("{{\"format\":\"lightcraft-catalog\",\"version\":1,\"seq\":{seq},\"catalog\":{}}}\n", Catalog::new().to_snapshot()).into_bytes(),
+        );
+        let e = Journal::open(Box::new(m)).err();
+        assert!(matches!(e, Some(CatalogError::Corrupt(_))), "seq {seq}: {e:?}");
+    }
+}