From af631ab90077aae8ff927f7a771c65a045b6ff3b Mon Sep 17 00:00:00 2001 From: mattsu Date: Mon, 15 Dec 2025 19:47:41 +0900 Subject: [PATCH 1/6] feat: preserve ownership and permissions in mv fallback operations on Unix Add functions to preserve file ownership (UID/GID) and permissions (mode) during mv operations when fs::rename fails and falls back to copy+remove. This ensures consistency with GNU mv behavior across filesystems, applying preservation in rename fallbacks for files, directories, symlinks, FIFOs, and recursive copies. Changes are Unix-specific, using libc::chown/lchown and fs::set_permissions. --- src/uu/mv/src/mv.rs | 78 ++++++++++++++++++++++++++++++++++++++-- tests/by-util/test_mv.rs | 55 ++++++++++++++++++++++++++++ 2 files changed, 131 insertions(+), 2 deletions(-) diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index 723875f615f..b038c084069 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -811,6 +811,44 @@ fn is_fifo(_filetype: fs::FileType) -> bool { false } +#[cfg(unix)] +fn try_preserve_ownership(from_meta: &fs::Metadata, to: &Path, follow_symlinks: bool) { + use std::ffi::CString; + use std::os::unix::ffi::OsStrExt as _; + use std::os::unix::fs::MetadataExt as _; + + let uid = from_meta.uid() as libc::uid_t; + let gid = from_meta.gid() as libc::gid_t; + + let Ok(to_cstr) = CString::new(to.as_os_str().as_bytes()) else { + return; + }; + + unsafe { + if follow_symlinks { + let _ = libc::chown(to_cstr.as_ptr(), uid, gid); + } else { + let _ = libc::lchown(to_cstr.as_ptr(), uid, gid); + } + } +} + +#[cfg(unix)] +fn try_preserve_permissions(from_meta: &fs::Metadata, to: &Path) { + use std::os::unix::fs::{MetadataExt as _, PermissionsExt as _}; + + // Keep mode bits only (file type bits are not allowed in chmod). + let mode = from_meta.mode() & 0o7777; + let _ = fs::set_permissions(to, fs::Permissions::from_mode(mode)); +} + +#[cfg(unix)] +fn try_preserve_ownership_and_permissions(from_meta: &fs::Metadata, to: &Path) { + // `chown` can clear setuid/setgid bits, so restore the mode afterwards. + try_preserve_ownership(from_meta, to, true); + try_preserve_permissions(from_meta, to); +} + /// A wrapper around `fs::rename`, so that if it fails, we try falling back on /// copying and removing. fn rename_with_fallback( @@ -887,10 +925,14 @@ fn rename_with_fallback( /// Replace the destination with a new pipe with the same name as the source. #[cfg(unix)] fn rename_fifo_fallback(from: &Path, to: &Path) -> io::Result<()> { + let from_meta = from.symlink_metadata()?; if to.try_exists()? { fs::remove_file(to)?; } - make_fifo(to).and_then(|_| fs::remove_file(from)) + make_fifo(to).and_then(|_| { + try_preserve_ownership_and_permissions(&from_meta, to); + fs::remove_file(from) + }) } #[cfg(not(unix))] @@ -902,8 +944,12 @@ fn rename_fifo_fallback(_from: &Path, _to: &Path) -> io::Result<()> { /// symlinks return an error. #[cfg(unix)] fn rename_symlink_fallback(from: &Path, to: &Path) -> io::Result<()> { + let from_meta = from.symlink_metadata()?; let path_symlink_points_to = fs::read_link(from)?; - unix::fs::symlink(path_symlink_points_to, to).and_then(|_| fs::remove_file(from)) + unix::fs::symlink(path_symlink_points_to, to).and_then(|_| { + try_preserve_ownership(&from_meta, to, false); + fs::remove_file(from) + }) } #[cfg(windows)] @@ -941,6 +987,9 @@ fn rename_dir_fallback( #[cfg(unix)] hardlink_tracker: Option<&mut HardlinkTracker>, #[cfg(unix)] hardlink_scanner: Option<&HardlinkGroupScanner>, ) -> io::Result<()> { + #[cfg(unix)] + let from_meta = from.symlink_metadata()?; + // We remove the destination directory if it exists to match the // behavior of `fs::rename`. As far as I can tell, `fs_extra`'s // `move_dir` would otherwise behave differently. @@ -986,6 +1035,9 @@ fn rename_dir_fallback( result?; + #[cfg(unix)] + try_preserve_ownership_and_permissions(&from_meta, to); + // Remove the source directory after successful copy fs::remove_dir_all(from)?; @@ -1081,6 +1133,11 @@ fn copy_dir_contents_recursive( progress_bar, display_manager, )?; + + #[cfg(unix)] + if let Ok(from_meta) = fs::symlink_metadata(&from_path) { + try_preserve_ownership_and_permissions(&from_meta, &to_path); + } } else { // Copy file with or without hardlink support based on platform #[cfg(unix)] @@ -1126,6 +1183,8 @@ fn copy_file_with_hardlinks_helper( hardlink_tracker: &mut HardlinkTracker, hardlink_scanner: &HardlinkGroupScanner, ) -> io::Result<()> { + let from_meta = from.symlink_metadata()?; + // Check if this file should be a hardlink to an already-copied file use crate::hardlink::HardlinkOptions; let hardlink_options = HardlinkOptions::default(); @@ -1147,6 +1206,8 @@ fn copy_file_with_hardlinks_helper( fs::copy(from, to)?; } + try_preserve_ownership_and_permissions(&from_meta, to); + Ok(()) } @@ -1156,6 +1217,9 @@ fn rename_file_fallback( #[cfg(unix)] hardlink_tracker: Option<&mut HardlinkTracker>, #[cfg(unix)] hardlink_scanner: Option<&HardlinkGroupScanner>, ) -> io::Result<()> { + #[cfg(unix)] + let from_meta = from.symlink_metadata()?; + // Remove existing target file if it exists if to.is_symlink() { fs::remove_file(to).map_err(|err| { @@ -1188,10 +1252,20 @@ fn rename_file_fallback( #[cfg(all(unix, not(any(target_os = "macos", target_os = "redox"))))] fs::copy(from, to) .and_then(|_| fsxattr::copy_xattrs(&from, &to)) + .and_then(|_| { + #[cfg(unix)] + try_preserve_ownership_and_permissions(&from_meta, to); + Ok(()) + }) .and_then(|_| fs::remove_file(from)) .map_err(|err| io::Error::new(err.kind(), translate!("mv-error-permission-denied")))?; #[cfg(any(target_os = "macos", target_os = "redox", not(unix)))] fs::copy(from, to) + .and_then(|_| { + #[cfg(unix)] + try_preserve_ownership_and_permissions(&from_meta, to); + Ok(()) + }) .and_then(|_| fs::remove_file(from)) .map_err(|err| io::Error::new(err.kind(), translate!("mv-error-permission-denied")))?; Ok(()) diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index f28fc8c28a6..7deb6c60e46 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -2036,6 +2036,61 @@ mod inter_partition_copying { ); } + // Test that ownership is preserved when moving files across partitions as root. + // + // This specifically guards the EXDEV (copy+delete) fallback path, which must not + // change uid/gid compared to a same-filesystem rename. + #[test] + #[cfg(target_os = "linux")] + pub(crate) fn test_mv_preserves_ownership_across_partitions_when_root() { + use std::ffi::CString; + use std::fs::metadata; + use std::os::unix::ffi::OsStrExt as _; + use std::os::unix::fs::MetadataExt as _; + use tempfile::TempDir; + use uutests::util::TestScenario; + + // Requires root to set an arbitrary uid/gid. + if unsafe { libc::geteuid() } != 0 { + return; + } + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + at.write("file", "test content"); + + let src_path = at.plus("file"); + let src_path_c = CString::new(src_path.as_os_str().as_bytes()).unwrap(); + + // Pick a non-root uid/gid. If chown isn't possible in this environment, skip. + let target_uid: libc::uid_t = 1; + let target_gid: libc::gid_t = 1; + let chown_result = unsafe { libc::chown(src_path_c.as_ptr(), target_uid, target_gid) }; + if chown_result != 0 { + return; + } + + let src_meta = metadata(&src_path).expect("Failed to get metadata for source file"); + assert_eq!(src_meta.uid(), target_uid); + assert_eq!(src_meta.gid(), target_gid); + + // Force cross-filesystem move using /dev/shm (tmpfs) + let other_fs_tempdir = TempDir::new_in("/dev/shm/") + .expect("Unable to create temp directory in /dev/shm - test requires tmpfs"); + + scene + .ucmd() + .arg("file") + .arg(other_fs_tempdir.path().to_str().unwrap()) + .succeeds(); + + let moved_file = other_fs_tempdir.path().join("file"); + let moved_meta = metadata(&moved_file).expect("Failed to get metadata for moved file"); + assert_eq!(moved_meta.uid(), target_uid); + assert_eq!(moved_meta.gid(), target_gid); + } + // Test that hardlinks are preserved even with multiple sets of hardlinked files #[test] #[cfg(unix)] From 61d884da05a9bc4208f88a8f41031ae8f38402c7 Mon Sep 17 00:00:00 2001 From: mattsu Date: Mon, 15 Dec 2025 20:41:49 +0900 Subject: [PATCH 2/6] refactor(mv): simplify fallback file copy chain using map over and_then Replace and_then with map in rename_file_fallback's copy operation for Unix systems, as the inner closure performs side effects and returns unit, making map more idiomatic than chaining with Ok(()). This improves code readability without altering behavior. --- src/uu/mv/src/mv.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index b038c084069..dca66fe7726 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -1252,19 +1252,17 @@ fn rename_file_fallback( #[cfg(all(unix, not(any(target_os = "macos", target_os = "redox"))))] fs::copy(from, to) .and_then(|_| fsxattr::copy_xattrs(&from, &to)) - .and_then(|_| { + .map(|_| { #[cfg(unix)] try_preserve_ownership_and_permissions(&from_meta, to); - Ok(()) }) .and_then(|_| fs::remove_file(from)) .map_err(|err| io::Error::new(err.kind(), translate!("mv-error-permission-denied")))?; #[cfg(any(target_os = "macos", target_os = "redox", not(unix)))] fs::copy(from, to) - .and_then(|_| { + .map(|_| { #[cfg(unix)] try_preserve_ownership_and_permissions(&from_meta, to); - Ok(()) }) .and_then(|_| fs::remove_file(from)) .map_err(|err| io::Error::new(err.kind(), translate!("mv-error-permission-denied")))?; From c8e07e86dc5426b8eb20f2374841bb2d8b9aa719 Mon Sep 17 00:00:00 2001 From: mattsu Date: Wed, 24 Dec 2025 20:28:44 +0900 Subject: [PATCH 3/6] test: add rootless unshare tmpfs test for mv with dangling symlink Add a new Linux-only test to verify cross-device move behavior using user namespaces and tmpfs mounts, avoiding sudo. This mirrors GNU's part-fail scenario for directories containing dangling symlinks, ensuring the mv command preserves symlinks correctly in rootless environments. --- tests/by-util/test_mv.rs | 68 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index 7deb6c60e46..7c3ce4628af 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -2636,6 +2636,74 @@ fn test_mv_cross_device_permission_denied() { .expect("Unable to restore directory permissions"); } +/// Rootless cross-device move using unshare + tmpfs mounts. +/// This mirrors the GNU part-fail scenario but avoids sudo by using user namespaces. +#[test] +#[cfg(target_os = "linux")] +fn test_mv_rootless_unshare_tmpfs_dir_with_dangling_symlink() { + use std::fs; + use std::process::Command; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let base = at.plus("unshare-rootless"); + fs::create_dir_all(&base).unwrap(); + + // Preflight: ensure unshare works in this environment (user namespaces enabled). + let preflight = match Command::new("unshare") + .args(["-rm", "sh", "-c", "true"]) + .output() + { + Ok(out) => out, + Err(e) => { + println!("test skipped: unshare not available: {e}"); + return; + } + }; + if !preflight.status.success() { + let stderr = String::from_utf8_lossy(&preflight.stderr); + println!("test skipped: unshare not permitted: {stderr}"); + return; + } + + let script = r#"set -eu +cleanup() { + umount -l "$BASE/a" 2>/dev/null || true + umount -l "$BASE/b" 2>/dev/null || true + rmdir "$BASE/a" "$BASE/b" 2>/dev/null || true +} +trap cleanup EXIT +mkdir -p "$BASE/a" "$BASE/b" +mount -t tmpfs tmpfs "$BASE/a" +mount -t tmpfs tmpfs "$BASE/b" +mkdir -p "$BASE/a/d" +ln -s miss "$BASE/a/d/dang" +"$UUTILS" mv -v "$BASE/a/d" "$BASE/b" +test -L "$BASE/b/d/dang" +test ! -e "$BASE/a/d" +"#; + + let output = Command::new("unshare") + .args(["-rm", "sh", "-c", script]) + .env("BASE", &base) + .env("UUTILS", &scene.bin_path) + .output() + .expect("failed to run unshare"); + + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + if stderr.contains("Operation not permitted") + || stderr.contains("permission denied") + || stderr.contains("not permitted") + { + println!("test skipped: unshare/mount not permitted: {stderr}"); + return; + } + panic!("unshare rootless mv test failed: {stderr}"); + } +} + #[test] #[cfg(feature = "selinux")] fn test_mv_selinux_context() { From b20063f1b1b8f41b50773de9fd6ceb180cd4b611 Mon Sep 17 00:00:00 2001 From: mattsu Date: Sat, 27 Dec 2025 16:57:16 +0900 Subject: [PATCH 4/6] feat(mv): add symlink handling in mv operations - Modified HardlinkGroupScanner to skip symlinks using symlink_metadata, as hardlink preservation does not apply to them - Added copy_symlink functions for Unix, Windows, and other platforms to copy symlinks without dereferencing - Updated copy_dir_contents_recursive to detect and copy symlinks, including verbose output, preventing dereferencing during directory moves --- src/uu/mv/src/hardlink.rs | 44 +++++++++++++++++----------- src/uu/mv/src/mv.rs | 60 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 86 insertions(+), 18 deletions(-) diff --git a/src/uu/mv/src/hardlink.rs b/src/uu/mv/src/hardlink.rs index 63bb152fd47..ecaa12b1b0e 100644 --- a/src/uu/mv/src/hardlink.rs +++ b/src/uu/mv/src/hardlink.rs @@ -217,18 +217,23 @@ impl HardlinkGroupScanner { fn scan_single_path(&mut self, path: &Path) -> io::Result<()> { use std::os::unix::fs::MetadataExt; - if path.is_dir() { + let metadata = path.symlink_metadata()?; + let file_type = metadata.file_type(); + + if file_type.is_symlink() { + // Hardlink preservation does not apply to symlinks. + return Ok(()); + } + + if file_type.is_dir() { // Recursively scan directory contents self.scan_directory_recursive(path)?; - } else { - let metadata = path.metadata()?; - if metadata.nlink() > 1 { - let key = (metadata.dev(), metadata.ino()); - self.hardlink_groups - .entry(key) - .or_default() - .push(path.to_path_buf()); - } + } else if metadata.nlink() > 1 { + let key = (metadata.dev(), metadata.ino()); + self.hardlink_groups + .entry(key) + .or_default() + .push(path.to_path_buf()); } Ok(()) } @@ -242,14 +247,19 @@ impl HardlinkGroupScanner { let entry = entry?; let path = entry.path(); - if path.is_dir() { + let metadata = path.symlink_metadata()?; + let file_type = metadata.file_type(); + + if file_type.is_symlink() { + // Skip symlinks to avoid following targets (including dangling links). + continue; + } + + if file_type.is_dir() { self.scan_directory_recursive(&path)?; - } else { - let metadata = path.metadata()?; - if metadata.nlink() > 1 { - let key = (metadata.dev(), metadata.ino()); - self.hardlink_groups.entry(key).or_default().push(path); - } + } else if metadata.nlink() > 1 { + let key = (metadata.dev(), metadata.ino()); + self.hardlink_groups.entry(key).or_default().push(path); } } Ok(()) diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index 00a0f4e7a3b..c95840eeb53 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -984,6 +984,45 @@ fn rename_symlink_fallback(from: &Path, to: &Path) -> io::Result<()> { )) } +/// Copy the given symlink to the given destination without dereferencing. +/// On Windows, dangling symlinks return an error. +#[cfg(unix)] +fn copy_symlink(from: &Path, to: &Path) -> io::Result<()> { + let from_meta = from.symlink_metadata()?; + let path_symlink_points_to = fs::read_link(from)?; + unix::fs::symlink(path_symlink_points_to, to).and_then(|_| { + try_preserve_ownership(&from_meta, to, false); + Ok(()) + }) +} + +#[cfg(windows)] +fn copy_symlink(from: &Path, to: &Path) -> io::Result<()> { + let path_symlink_points_to = fs::read_link(from)?; + if path_symlink_points_to.exists() { + if path_symlink_points_to.is_dir() { + windows::fs::symlink_dir(&path_symlink_points_to, to)?; + } else { + windows::fs::symlink_file(&path_symlink_points_to, to)?; + } + Ok(()) + } else { + Err(io::Error::new( + io::ErrorKind::NotFound, + translate!("mv-error-dangling-symlink"), + )) + } +} + +#[cfg(not(any(windows, unix)))] +fn copy_symlink(from: &Path, to: &Path) -> io::Result<()> { + let _ = (from, to); + Err(io::Error::new( + io::ErrorKind::Other, + translate!("mv-error-no-symlink-support"), + )) +} + fn rename_dir_fallback( from: &Path, to: &Path, @@ -1108,7 +1147,26 @@ fn copy_dir_contents_recursive( pb.set_message(from_path.to_string_lossy().to_string()); } - if from_path.is_dir() { + let entry_type = entry.file_type()?; + + if entry_type.is_symlink() { + copy_symlink(&from_path, &to_path)?; + + // Print verbose message for symlink + if verbose { + let message = translate!( + "mv-verbose-renamed", + "from" => from_path.quote(), + "to" => to_path.quote() + ); + match display_manager { + Some(pb) => pb.suspend(|| { + println!("{message}"); + }), + None => println!("{message}"), + } + } + } else if entry_type.is_dir() { // Recursively copy subdirectory fs::create_dir_all(&to_path)?; From 5d25c3532f77daf1a94dccfad89d9d874e461b30 Mon Sep 17 00:00:00 2001 From: mattsu Date: Sat, 27 Dec 2025 17:06:39 +0900 Subject: [PATCH 5/6] refactor(mv): use map instead of and_then in copy_symlink function Updated the copy_symlink function to use .map() combinator instead of .and_then() for clarity, as the closure performs a side effect (preserving ownership) and returns a unit value without chaining Results. This improves code readability and appropriateness of the combinator used. --- src/uu/mv/src/mv.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index c95840eeb53..164af32c0f2 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -990,9 +990,9 @@ fn rename_symlink_fallback(from: &Path, to: &Path) -> io::Result<()> { fn copy_symlink(from: &Path, to: &Path) -> io::Result<()> { let from_meta = from.symlink_metadata()?; let path_symlink_points_to = fs::read_link(from)?; - unix::fs::symlink(path_symlink_points_to, to).and_then(|_| { + unix::fs::symlink(path_symlink_points_to, to).map(|_| { try_preserve_ownership(&from_meta, to, false); - Ok(()) + () }) } From 6e8d7474b974f8846d84dc0e20ab7f9199c42545 Mon Sep 17 00:00:00 2001 From: mattsu Date: Sat, 27 Dec 2025 17:12:08 +0900 Subject: [PATCH 6/6] refactor(mv): remove unnecessary unit return in copy_symlink Simplify the map closure by removing the explicit return of an empty tuple, as it is implicit in Rust when no value is needed. This improves code clarity without affecting functionality. --- src/uu/mv/src/mv.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index 164af32c0f2..0a6006ebf65 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -992,7 +992,6 @@ fn copy_symlink(from: &Path, to: &Path) -> io::Result<()> { let path_symlink_points_to = fs::read_link(from)?; unix::fs::symlink(path_symlink_points_to, to).map(|_| { try_preserve_ownership(&from_meta, to, false); - () }) }