Catalog and downloads: Don't write through a link at a temp file's name (#1502)

crates/catalog/src/store.rs

@@ -73,6 +73,16 @@ impl FsStore {
     }
 }
 
+/// A new staging file at `tmp`: whatever is already there (a leftover from a crashed save, or a link) is removed first
+/// (a link itself, never what it points to), then the file is created exclusively, so a write never follows a link
+/// planted at that name into another file (issue #1502).
+fn create_staging(tmp: &std::path::Path) -> io::Result<std::fs::File> {
+    if std::fs::symlink_metadata(tmp).is_ok() {
+        std::fs::remove_file(tmp)?;
+    }
+    std::fs::OpenOptions::new().write(true).create_new(true).open(tmp)
+}
+
 impl Store for FsStore {
     fn read(&mut self, name: &str) -> io::Result<Option<Vec<u8>>> {
         match std::fs::read(self.path(name)) {
@@ -89,7 +99,7 @@ impl Store for FsStore {
         }
         let tmp = self.path(&format!("{name}.tmp"));
         {
-            let mut f = std::fs::File::create(&tmp)?;
+            let mut f = create_staging(&tmp)?;
             f.write_all(data)?;
             f.sync_all()?;
         }
@@ -105,7 +115,7 @@ impl Store for FsStore {
         }
         let tmp = self.path(&format!("{name}.tmp"));
         let written = (|| {
-            let mut w = Counter { inner: io::BufWriter::with_capacity(1 << 20, std::fs::File::create(&tmp)?), n: 0 };
+            let mut w = Counter { inner: io::BufWriter::with_capacity(1 << 20, create_staging(&tmp)?), n: 0 };
             fill(&mut w)?;
             w.flush()?;
             let f = w.inner.into_inner().map_err(|e| e.into_error())?;
@@ -231,3 +241,29 @@ impl Store for MemStore {
         self.background.then(|| Box::new(self.clone()) as Box<dyn Store>)
     }
 }
+
+#[cfg(test)]
+mod tests_staging {
+    use super::*;
+
+    /// A link planted at a staging name (`<name>.tmp`) is removed, never followed: the file it points to keeps its
+    /// bytes and the save still lands (issue #1502).
+    #[cfg(unix)]
+    #[test]
+    fn a_link_at_the_staging_name_is_never_written_through() {
+        let dir = std::env::temp_dir().join(format!("lc-staging-link-{}", std::process::id()));
+        let _ = std::fs::remove_dir_all(&dir);
+        let mut store = FsStore::open(&dir).unwrap();
+        let victim = dir.join("victim.txt");
+        std::fs::write(&victim, b"keep").unwrap();
+        std::os::unix::fs::symlink(&victim, dir.join("catalog.snap.tmp")).unwrap();
+        store.write_atomic("catalog.snap", b"{}").unwrap();
+        assert_eq!(std::fs::read(&victim).unwrap(), b"keep");
+        assert_eq!(std::fs::read(dir.join("catalog.snap")).unwrap(), b"{}");
+        std::os::unix::fs::symlink(&victim, dir.join("catalog.log.tmp")).unwrap();
+        store.write_atomic_with("catalog.log", &mut |w| w.write_all(b"[1]")).unwrap();
+        assert_eq!(std::fs::read(&victim).unwrap(), b"keep");
+        assert_eq!(std::fs::read(dir.join("catalog.log")).unwrap(), b"[1]");
+        let _ = std::fs::remove_dir_all(&dir);
+    }
+}

crates/fetch/src/lib.rs

@@ -314,6 +314,15 @@ fn sha256_file(path: &Path) -> Result<String, String> {
     Ok(h.finalize().iter().map(|b| format!("{b:02x}")).collect())
 }
 
+/// A new, empty partial download at `part`, created exclusively after removing whatever was there (a link itself,
+/// never its target).
+fn new_part(part: &Path) -> std::io::Result<File> {
+    if std::fs::symlink_metadata(part).is_ok() {
+        std::fs::remove_file(part)?;
+    }
+    OpenOptions::new().write(true).create_new(true).open(part)
+}
+
 fn part_path(dir: &Path, name: &str) -> PathBuf {
     dir.join(format!("{name}.part"))
 }
@@ -324,9 +333,10 @@ fn fetch_file(f: &FileSpec<'_>, url: &str, dir: &Path, opts: &Options, cancel: &
     let part = part_path(dir, f.name);
     let limit = f.size.unwrap_or(f.max);
     let disk = |e: std::io::Error| Fail::Disk(format!("{}: {e}", part.display()));
-    // resume only a pinned file (an unpinned one can't be checked after resuming)
-    let mut have = match std::fs::metadata(&part) {
-        Ok(m) if f.size.is_some() && m.len() <= limit => m.len(),
+    // resume only a pinned file (an unpinned one can't be checked after resuming), and only a plain file: anything
+    // else at that name (a link, which `metadata` would follow) is removed, never written through (issue #1502)
+    let mut have = match std::fs::symlink_metadata(&part) {
+        Ok(m) if m.is_file() && f.size.is_some() && m.len() <= limit => m.len(),
         Ok(_) => {
             std::fs::remove_file(&part).map_err(disk)?;
             0
@@ -368,7 +378,7 @@ fn fetch_file(f: &FileSpec<'_>, url: &str, dir: &Path, opts: &Options, cancel: &
                     return Err(Fail::NextMirror(format!("the server's file has the wrong size ({:?} bytes)", resp.content_length())));
                 }
                 have = 0;
-                File::create(&part).map_err(disk)?
+                new_part(&part).map_err(disk)?
             }
             206 if have > 0 => {
                 let range = resp.header("content-range").unwrap_or_default();
@@ -376,6 +386,10 @@ fn fetch_file(f: &FileSpec<'_>, url: &str, dir: &Path, opts: &Options, cancel: &
                 if start != have || total.is_some_and(|t| Some(t) != f.size) {
                     return Err(Fail::NextMirror(format!("the server resumed at the wrong place ({range})")));
                 }
+                // (still the plain file checked above: a link put there meanwhile is not appended to)
+                if !std::fs::symlink_metadata(&part).is_ok_and(|m| m.is_file()) {
+                    return Err(Fail::Retry("the partial download changed; starting over".into()));
+                }
                 OpenOptions::new().append(true).open(&part).map_err(disk)?
             }
             416 => {

crates/fetch/src/tests.rs

@@ -210,6 +210,28 @@ fn resumes_partial_files_and_restarts_bad_ones() {
     let _ = std::fs::remove_dir_all(&dir);
 }
 
+/// A link planted at the partial download's name is removed, never written or appended through: the file it points
+/// to keeps its bytes and the download completes (issue #1502).
+#[cfg(unix)]
+#[test]
+fn a_link_at_the_partial_name_is_never_written_through() {
+    let srv = server();
+    let dir = tmp("part-link");
+    std::fs::create_dir_all(&dir).unwrap();
+    let victim = dir.join("victim.txt");
+    for target_len in [10usize, 0] {
+        std::fs::write(&victim, vec![b'k'; target_len.max(1)]).unwrap();
+        let before = std::fs::read(&victim).unwrap();
+        let _ = std::fs::remove_file(dir.join("model.bin"));
+        let _ = std::fs::remove_file(part_path(&dir, "model.bin"));
+        std::os::unix::fs::symlink(&victim, part_path(&dir, "model.bin")).unwrap();
+        run(&[pinned()], &[format!("{}/good", srv.base)], &dir, &AtomicBool::new(false)).0.unwrap();
+        assert_eq!(std::fs::read(&victim).unwrap(), before, "the linked file is untouched");
+        assert_eq!(std::fs::read(dir.join("model.bin")).unwrap(), payload());
+    }
+    let _ = std::fs::remove_dir_all(&dir);
+}
+
 #[test]
 fn a_416_or_a_bad_complete_part_starts_over() {
     let srv = server();

docs/whats-new.md

@@ -2,6 +2,11 @@
 
 ## 2026-10-14
 
+### Reliability
+- Saving the catalog and resuming a model download no longer write through a link left at the temporary file's
+  name (`catalog.snap.tmp`, `model.onnx.part`): the link is removed and a new file made in its place, so the file it
+  pointed to is never changed (issue #1502).
+
 ### Hard-edged Heal and Clone spots
 - A spot with Feather at 0 no longer draws black pixels exactly on its radius. This also fixes spots at 0% Opacity
   changing pixels along their edge. The hard edge now leaves pixels on the radius unchanged.