From af631ab90077aae8ff927f7a771c65a045b6ff3b Mon Sep 17 00:00:00 2001 From: mattsu Date: Mon, 15 Dec 2025 19:47:41 +0900 Subject: [PATCH 1/3] 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/3] 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/3] 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() {