fix: saving follows symbolic links and keeps the file's permissions (#845)

A save through a symbolic link replaces the file it points to and keeps the link; the saved file and its backup keep the original permissions.

crates/io/src/safe_write.rs

@@ -47,12 +47,40 @@ enum Step {
     Replaced,
 }
 
+/// Most links followed before a path is called a loop.
+const MAX_LINK_HOPS: usize = 40;
+
+/// The file a save has to replace: `path`, or where a symbolic link at `path` (or a chain of
+/// them) leads, so a design linked into a project from a synced folder is saved there and the
+/// link survives. A link whose target is missing resolves to the place the target would be.
+fn resolve_links(path: &Path) -> std::io::Result<PathBuf> {
+    let mut at = path.to_path_buf();
+    for _ in 0..MAX_LINK_HOPS {
+        if !fs::symlink_metadata(&at).is_ok_and(|m| m.file_type().is_symlink()) {
+            return Ok(at);
+        }
+        let to = fs::read_link(&at)?;
+        at = if to.is_absolute() { to } else { at.parent().map(|d| d.join(&to)).unwrap_or(to) };
+    }
+    Err(std::io::Error::new(std::io::ErrorKind::InvalidInput, format!("{}: too many levels of symbolic links", path.display())))
+}
+
+/// Give the file that replaces `old` the permissions `old` had (a private `0600` design must not
+/// come back readable by everyone, a group's `0664` must not lose the group's write access).
+fn keep_permissions(old: &Option<fs::Permissions>, new: &Path) {
+    if let Some(p) = old {
+        let _ = fs::set_permissions(new, p.clone());
+    }
+}
+
 fn write_steps(path: &Path, bytes: &[u8], backup: bool, stop: Option<Step>) -> std::io::Result<()> {
+    let permissions = fs::metadata(path).ok().filter(|m| m.is_file()).map(|m| m.permissions());
     let tmp = temp_beside(path, "new");
     if let Err(e) = write_synced(&tmp, bytes) {
         let _ = fs::remove_file(&tmp);
         return Err(e);
     }
+    keep_permissions(&permissions, &tmp);
     if stop == Some(Step::TempWritten) {
         return Ok(());
     }
@@ -60,7 +88,10 @@ fn write_steps(path: &Path, bytes: &[u8], backup: bool, stop: Option<Step>) -> s
         // Copy (not move) the old file, so the target exists at every moment.
         let old = fs::read(path);
         let bak_tmp = temp_beside(path, "bak");
-        let r = old.and_then(|old| write_synced(&bak_tmp, &old)).and_then(|_| fs::rename(&bak_tmp, backup_path(path)));
+        let r = old
+            .and_then(|old| write_synced(&bak_tmp, &old))
+            .map(|_| keep_permissions(&permissions, &bak_tmp))
+            .and_then(|_| fs::rename(&bak_tmp, backup_path(path)));
         if let Err(e) = r {
             let _ = fs::remove_file(&bak_tmp);
             let _ = fs::remove_file(&tmp);
@@ -85,18 +116,22 @@ fn write_steps(path: &Path, bytes: &[u8], backup: bool, stop: Option<Step>) -> s
 }
 
 /// Write `bytes` to `path` atomically; with `backup`, the file being replaced is kept as
-/// `<path>.bak` first.
+/// `<path>.bak` first. A symbolic link at `path` is followed (the file it points to is replaced,
+/// the link stays), and the file keeps the permissions it had.
 pub fn write_atomic(path: &Path, bytes: &[u8], backup: bool) -> std::io::Result<()> {
     // In the browser the host stores files (one write is already atomic there).
     if cfg!(target_arch = "wasm32") {
         let _ = backup;
         return crate::vfs::write(&path.to_string_lossy(), bytes);
     }
-    write_steps(path, bytes, backup, None)
+    write_steps(&resolve_links(path)?, bytes, backup, None)
 }
 
-/// Leftover temporary files of saves that never finished (a crash part way), next to `path`.
+/// Leftover temporary files of saves that never finished (a crash part way), next to `path`
+/// (next to the file a link at `path` points to, where a save puts them).
 pub fn stale_temps(path: &Path) -> Vec<PathBuf> {
+    let resolved = resolve_links(path).unwrap_or_else(|_| path.to_path_buf());
+    let path = resolved.as_path();
     let Some(name) = path.file_name().map(|n| n.to_string_lossy().to_string()) else { return Vec::new() };
     let dir = path.parent().filter(|d| !d.as_os_str().is_empty()).unwrap_or(Path::new("."));
     let prefix = format!(".{name}.");
@@ -165,4 +200,98 @@ mod tests {
         assert!(!p.exists());
         let _ = fs::remove_dir_all(&d);
     }
+
+    #[cfg(unix)]
+    mod unix {
+        use super::*;
+        use std::os::unix::fs::{PermissionsExt, symlink};
+
+        fn mode(p: &Path) -> u32 {
+            fs::metadata(p).unwrap().permissions().mode() & 0o777
+        }
+
+        /// saving through a link replaces the file it points to and leaves the link.
+        #[test]
+        fn a_save_through_a_link_updates_the_target() {
+            let d = dir("link");
+            fs::create_dir_all(d.join("cloud")).unwrap();
+            fs::create_dir_all(d.join("work")).unwrap();
+            let target = d.join("cloud").join("a.solidcraft");
+            let link = d.join("work").join("a.solidcraft");
+            write_atomic(&target, b"one", false).unwrap();
+            symlink(&target, &link).unwrap();
+            write_atomic(&link, b"two", true).unwrap();
+            assert!(fs::symlink_metadata(&link).unwrap().file_type().is_symlink(), "the link stays a link");
+            assert_eq!(fs::read(&target).unwrap(), b"two");
+            assert_eq!(fs::read(&link).unwrap(), b"two");
+            assert_eq!(fs::read(backup_path(&target)).unwrap(), b"one", "the backup sits beside the real file");
+            assert!(!backup_path(&link).exists());
+            let _ = fs::remove_dir_all(&d);
+        }
+
+        #[test]
+        fn relative_and_chained_links_are_followed() {
+            let d = dir("chain");
+            fs::create_dir_all(d.join("real")).unwrap();
+            fs::create_dir_all(d.join("mid")).unwrap();
+            let target = d.join("real").join("a.solidcraft");
+            write_atomic(&target, b"one", false).unwrap();
+            symlink("../real/a.solidcraft", d.join("mid").join("b")).unwrap();
+            symlink("mid/b", d.join("c")).unwrap();
+            write_atomic(&d.join("c"), b"two", false).unwrap();
+            assert_eq!(fs::read(&target).unwrap(), b"two");
+            assert!(fs::symlink_metadata(d.join("c")).unwrap().file_type().is_symlink());
+            assert!(fs::symlink_metadata(d.join("mid").join("b")).unwrap().file_type().is_symlink());
+            let _ = fs::remove_dir_all(&d);
+        }
+
+        #[test]
+        fn a_dangling_link_is_filled_in_and_a_loop_is_an_error() {
+            let d = dir("dangling");
+            let link = d.join("a.solidcraft");
+            let target = d.join("later.solidcraft");
+            symlink(&target, &link).unwrap();
+            write_atomic(&link, b"new", true).unwrap();
+            assert_eq!(fs::read(&target).unwrap(), b"new");
+            assert!(fs::symlink_metadata(&link).unwrap().file_type().is_symlink());
+
+            let (x, y) = (d.join("x"), d.join("y"));
+            symlink(&y, &x).unwrap();
+            symlink(&x, &y).unwrap();
+            let e = write_atomic(&x, b"loop", true).unwrap_err();
+            assert!(e.to_string().contains("symbolic links"), "{e}");
+            let _ = fs::remove_dir_all(&d);
+        }
+
+        /// a private design stays private, a group's file keeps the group's access, and
+        /// the backup carries the same permissions.
+        #[test]
+        fn a_save_keeps_the_permissions() {
+            let d = dir("mode");
+            for want in [0o600, 0o640, 0o664, 0o444] {
+                let p = d.join(format!("a{want:o}.solidcraft"));
+                write_atomic(&p, b"one", false).unwrap();
+                fs::set_permissions(&p, fs::Permissions::from_mode(want)).unwrap();
+                write_atomic(&p, b"two", true).unwrap();
+                assert_eq!(fs::read(&p).unwrap(), b"two");
+                assert_eq!(mode(&p), want, "{want:o}");
+                assert_eq!(mode(&backup_path(&p)), want, "backup {want:o}");
+                let _ = fs::set_permissions(&p, fs::Permissions::from_mode(0o644));
+            }
+            let _ = fs::remove_dir_all(&d);
+        }
+
+        #[test]
+        fn a_save_through_a_link_keeps_the_targets_permissions() {
+            let d = dir("linkmode");
+            let target = d.join("a.solidcraft");
+            write_atomic(&target, b"one", false).unwrap();
+            fs::set_permissions(&target, fs::Permissions::from_mode(0o600)).unwrap();
+            symlink(&target, d.join("l")).unwrap();
+            write_atomic(&d.join("l"), b"two", true).unwrap();
+            assert_eq!(mode(&target), 0o600);
+            assert_eq!(mode(&backup_path(&target)), 0o600);
+            let _ = fs::remove_dir_all(&d);
+        }
+    }
 }

docs/book/src/guide/files.md

@@ -36,7 +36,9 @@ keep a model's faces (for loads, boundary conditions or feature edges); other 3M
 them. The format is specified in [`docs/formats/3mf-face-tags.md`](https://github.com/bherbruck/solidcraft/blob/main/docs/formats/3mf-face-tags.md).
 
 Saves are atomic: a crash at any moment leaves either the old file or the new one, and the
-replaced version is kept as `<file>.bak`. Unsaved changes are autosaved to a recovery folder,
+replaced version is kept as `<file>.bak`. A save keeps the file's permissions, and a design
+opened through a symbolic link (a synced folder linked into a project) is saved into the file the
+link points to, so the link stays. Unsaved changes are autosaved to a recovery folder,
 and the next launch offers to recover them. Older design files are upgraded on open, and files
 from a newer version are refused with a message.