diff --git a/Cargo.lock b/Cargo.lock index 7a8da0062a..b5867bcf8d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4624,6 +4624,7 @@ version = "0.18.1" dependencies = [ "async-channel", "futures-lite", + "log", "rustix 1.1.5", "zbus", ] diff --git a/crates/platform/Cargo.toml b/crates/platform/Cargo.toml index a552a43f81..1808c4128f 100644 --- a/crates/platform/Cargo.toml +++ b/crates/platform/Cargo.toml @@ -9,6 +9,7 @@ license.workspace = true workspace = true [dependencies] +log = { workspace = true } [target.'cfg(target_os = "macos")'.dependencies] # `write_atomically`: a volume that refuses F_FULLFSYNC gets a plain fsync. diff --git a/crates/platform/src/staging.rs b/crates/platform/src/staging.rs index 4316051352..b9b2a80443 100644 --- a/crates/platform/src/staging.rs +++ b/crates/platform/src/staging.rs @@ -132,7 +132,8 @@ pub fn staging_suffixes() -> impl Iterator { /// (`sync_all`); parent-directory durability after a power loss is not promised. On Unix a file /// that is replaced keeps its permissions and group, so a private file stays private; a new file gets the /// usual mode for a new file. On Windows a replaced file stays hidden or system if it was, and -/// plain if it was plain. On Windows and macOS a replaced file keeps its creation date, where +/// plain if it was plain, even where a server hides the dot-named staging file (#5001, #4593, #6029, #6119). +/// On Windows and macOS a replaced file keeps its creation date, where /// the volume stores one. /// On macOS, accessible extended attributes of an existing regular destination are copied /// before flushing; a copy failure refuses the save rather than losing metadata (#2658). @@ -185,7 +186,28 @@ pub fn write_atomically_with( keep_created(&file, created); sync_staged(&file, sync_all)?; } - std::fs::rename(&tmp, path) + let rename_result = std::fs::rename(&tmp, path); + #[cfg(windows)] + if rename_result.is_ok() + && let Some(keep) = &keep + { + // Restore attributes only if the saved file's hidden or system bits differ from the + // replaced file's (#5001, #4593, #6029, #6119). Samba/NAS servers mark dot-named + // staging files hidden and persist that flag through the rename; on local disks, the + // staging file already has the right KEPT_ATTRIBUTES. Restoring unconditionally would + // overwrite the newly written file's entire attribute word with the old file's, + // incorrectly wiping the archive bit (which backup tools need) or retaining offline/index + // flags. On servers where attributes differ, restoring permissions is a safe trade to keep + // the file visible. When saving a new file (`keep` is None), no baseline exists. + use std::os::windows::fs::MetadataExt as _; + let saved = std::fs::symlink_metadata(path).map(|m| m.file_attributes() & KEPT_ATTRIBUTES); + if saved.is_ok_and(|a| a != (keep.file_attributes() & KEPT_ATTRIBUTES)) + && let Err(e) = std::fs::set_permissions(path, keep.permissions()) + { + log::warn!("could not restore replaced file attributes on {}: {e}", path.display()); + } + } + rename_result })(); if result.is_err() { let _ = std::fs::remove_file(&tmp); diff --git a/crates/platform/src/staging/tests.rs b/crates/platform/src/staging/tests.rs index b5552a2c7b..da1e015e15 100644 --- a/crates/platform/src/staging/tests.rs +++ b/crates/platform/src/staging/tests.rs @@ -448,3 +448,76 @@ fn a_replaced_link_keeps_its_own_attributes_not_its_targets() { assert_ne!(std::fs::metadata(&referent).unwrap().file_attributes() & HIDDEN, 0, "and still hidden"); let _ = std::fs::remove_dir_all(&dir); } + +/// #5001, #4593: a save overwriting an existing file preserves its original attributes even if +/// the server (such as Samba/QNAP NAS hiding dot-files) marks the temporary staging file hidden. +#[cfg(windows)] +#[test] +fn a_replaced_file_restores_attributes_if_server_hides_staging_file() { + use std::os::windows::fs::{MetadataExt as _, OpenOptionsExt as _}; + const HIDDEN: u32 = 0x2; + let dir = test_dir("server-hides-staging"); + let target = dir.join("plain.pdf"); + std::fs::write(&target, b"OLD").unwrap(); + assert_eq!(std::fs::metadata(&target).unwrap().file_attributes() & HIDDEN, 0, "target starts visible"); + + // Create a temporary hidden reference file to obtain permissions with the HIDDEN attribute. + let hidden_ref = dir.join("hidden_ref"); + let _ = std::fs::OpenOptions::new().write(true).create_new(true).attributes(HIDDEN).open(&hidden_ref).unwrap(); + let hidden_perm = std::fs::metadata(&hidden_ref).unwrap().permissions(); + let _ = std::fs::remove_file(&hidden_ref); + + for form in [StagingName::TagThenSuffix, StagingName::SuffixThenTag] { + write_atomically_with(&target, b"NEW", form, [77], |_file| { + // Find the staging file in dir (the file that is not target): + for entry in std::fs::read_dir(&dir)? { + let path = entry?.path(); + if path != target { + // Mark the staging file as HIDDEN on disk (simulating Samba hiding dot-files). + std::fs::set_permissions(&path, hidden_perm.clone())?; + assert_ne!(std::fs::metadata(&path)?.file_attributes() & HIDDEN, 0, "staging file was marked hidden"); + } + } + Ok(()) + }) + .unwrap(); + + assert_eq!( + std::fs::metadata(&target).unwrap().file_attributes() & HIDDEN, + 0, + "the saved file remains visible despite server hiding the staging file" + ); + assert_eq!(std::fs::read(&target).unwrap(), b"NEW"); + } + + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +#[cfg(windows)] +fn a_replaced_local_file_keeps_archive_bit_intact() { + use std::os::windows::fs::MetadataExt as _; + let mut dir = std::env::temp_dir(); + dir.push(format!("pdfcraft-platform-test-archive-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + + let target = dir.join("plain.pdf"); + std::fs::write(&target, b"OLD").unwrap(); + + // Clear the archive bit on the replaced file (simulating backup tools like robocopy /M or attrib -A). + const ARCHIVE: u32 = 0x20; + let status = std::process::Command::new("attrib").arg("-A").arg(&target).status().unwrap(); + assert!(status.success(), "attrib -A succeeded"); + let old_attrs = std::fs::metadata(&target).unwrap().file_attributes(); + assert_eq!(old_attrs & ARCHIVE, 0, "archive bit was cleared on the old file"); + + write_atomically(&target, b"NEW", StagingName::TagThenSuffix).unwrap(); + + // The newly written file must have the archive bit set by the system (#5001, #4593, #6029, #6119). + let new_attrs = std::fs::metadata(&target).unwrap().file_attributes(); + assert_ne!(new_attrs & ARCHIVE, 0, "the newly saved file has the archive bit set by the file system"); + assert_eq!(std::fs::read(&target).unwrap(), b"NEW"); + + let _ = std::fs::remove_dir_all(&dir); +}