From 4d8c80a86d311b7be6539cbbe738c11fd78b9512 Mon Sep 17 00:00:00 2001 From: Lasse Lian Date: Tue, 13 Oct 2026 22:32:55 +0200 Subject: [PATCH] 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 | 137 ++++++++++++++++++++++++++++++++++- docs/book/src/guide/files.md | 4 +- 2 files changed, 136 insertions(+), 5 deletions(-) diff --git a/crates/io/src/safe_write.rs b/crates/io/src/safe_write.rs index a2567580..9260b4c0 100644 --- a/crates/io/src/safe_write.rs +++ b/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 { + 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, 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) -> 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) -> 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) -> s } /// Write `bytes` to `path` atomically; with `backup`, the file being replaced is kept as -/// `.bak` first. +/// `.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 { + 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); + } + } } diff --git a/docs/book/src/guide/files.md b/docs/book/src/guide/files.md index 87800ba9..0f4dc6b5 100644 --- a/docs/book/src/guide/files.md +++ b/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 `.bak`. Unsaved changes are autosaved to a recovery folder, +replaced version is kept as `.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.