mv:fix file ownership changes when a file is mv'ed by root to a different file system

src/uu/mv/src/hardlink.rs

@@ -204,18 +204,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(())
     }
@@ -229,14 +234,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(())

src/uu/mv/src/mv.rs

@@ -804,6 +804,72 @@ fn is_fifo(_filetype: fs::FileType) -> bool {
     false
 }
 
+#[cfg(unix)]
+/// Best-effort ownership preservation for `to` using `from_meta`.
+///
+/// On Unix, this tries to set `uid`/`gid` on `to`. If `follow_symlinks` is
+/// true it uses `chown`, otherwise it uses `lchown` so the link itself (not its
+/// target) is updated. `chown`/`lchown` failures are non-fatal; permission
+/// errors are ignored, and other failures emit a warning because ownership
+/// preservation is optional.
+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;
+    };
+
+    let result = unsafe {
+        if follow_symlinks {
+            libc::chown(to_cstr.as_ptr(), uid, gid)
+        } else {
+            libc::lchown(to_cstr.as_ptr(), uid, gid)
+        }
+    };
+    if result != 0 {
+        let err = io::Error::last_os_error();
+        if err.kind() != io::ErrorKind::PermissionDenied {
+            eprintln!(
+                "mv: warning: failed to preserve ownership for {}: {err}",
+                to.quote()
+            );
+        }
+    }
+}
+
+#[cfg(unix)]
+/// Best-effort permission preservation for `to` using `from_meta`.
+///
+/// Only the mode bits are applied (`chmod` does not accept file type bits).
+/// Failures are non-fatal; permission errors are ignored, and other failures
+/// emit a warning because this is optional.
+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;
+    if let Err(err) = fs::set_permissions(to, fs::Permissions::from_mode(mode)) {
+        if err.kind() != io::ErrorKind::PermissionDenied {
+            eprintln!(
+                "mv: warning: failed to preserve permissions for {}: {err}",
+                to.quote()
+            );
+        }
+    }
+}
+
+#[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(
@@ -880,10 +946,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))]
@@ -897,27 +967,36 @@ fn rename_fifo_fallback(_from: &Path, _to: &Path) -> io::Result<()> {
 
 /// Move the given symlink to the given destination. On Windows, dangling
 /// symlinks return an error.
-#[cfg(unix)]
 fn rename_symlink_fallback(from: &Path, to: &Path) -> io::Result<()> {
+    copy_symlink(from, to)?;
+    fs::remove_file(from)
+}
+
+/// 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)?;
     #[cfg(not(any(target_os = "macos", target_os = "redox")))]
     {
         let _ = copy_xattrs_if_supported(from, to);
     }
-    fs::remove_file(from)
+    try_preserve_ownership(&from_meta, to, false);
+    Ok(())
 }
 
 #[cfg(windows)]
-fn rename_symlink_fallback(from: &Path, to: &Path) -> io::Result<()> {
+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)?;
         }
-        fs::remove_file(from)
+        Ok(())
     } else {
         Err(io::Error::new(
             io::ErrorKind::NotFound,
@@ -927,8 +1006,8 @@ fn rename_symlink_fallback(from: &Path, to: &Path) -> io::Result<()> {
 }
 
 #[cfg(not(any(windows, unix)))]
-fn rename_symlink_fallback(from: &Path, to: &Path) -> io::Result<()> {
-    let path_symlink_points_to = fs::read_link(from)?;
+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"),
@@ -943,6 +1022,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.
@@ -988,6 +1070,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)?;
 
@@ -1065,7 +1150,9 @@ fn copy_dir_contents_recursive(
             pb.set_message(from_path.to_string_lossy().to_string());
         }
 
-        if from_path.is_symlink() {
+        let entry_type = entry.file_type()?;
+
+        if entry_type.is_symlink() {
             // Handle symlinks first, before checking is_dir() which follows symlinks.
             // This prevents symlinks to directories from being expanded into full copies.
             #[cfg(unix)]
@@ -1083,7 +1170,7 @@ fn copy_dir_contents_recursive(
             }
 
             print_verbose(&from_path, &to_path);
-        } else if from_path.is_dir() {
+        } else if entry_type.is_dir() {
             // Recursively copy subdirectory (only real directories, not symlinks)
             fs::create_dir_all(&to_path)?;
 
@@ -1100,6 +1187,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)]
@@ -1137,6 +1229,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();
@@ -1148,10 +1242,10 @@ fn copy_file_with_hardlinks_helper(
         return Ok(());
     }
 
-    if from.is_symlink() {
+    if from_meta.file_type().is_symlink() {
         // Copy a symlink file (no-follow).
-        rename_symlink_fallback(from, to)?;
-    } else if is_fifo(from.symlink_metadata()?.file_type()) {
+        copy_symlink(from, to)?;
+    } else if is_fifo(from_meta.file_type()) {
         make_fifo(to)?;
     } else {
         // Copy a regular file.
@@ -1163,6 +1257,8 @@ fn copy_file_with_hardlinks_helper(
         }
     }
 
+    try_preserve_ownership_and_permissions(&from_meta, to);
+
     Ok(())
 }
 
@@ -1172,6 +1268,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| {
@@ -1210,6 +1309,11 @@ fn rename_file_fallback(
         let _ = copy_xattrs_if_supported(from, to);
     }
 
+    #[cfg(unix)]
+    {
+        try_preserve_ownership_and_permissions(&from_meta, to);
+    }
+
     fs::remove_file(from)
         .map_err(|err| io::Error::new(err.kind(), translate!("mv-error-permission-denied")))?;
     Ok(())

tests/by-util/test_df.rs

@@ -16,7 +16,7 @@ use std::collections::HashSet;
 use uutests::at_and_ucmd;
 use uutests::new_ucmd;
 #[cfg(target_os = "linux")]
-use uutests::util::TestScenario;
+use uutests::util::{TestScenario, run_in_rootless_unshare};
 
 #[test]
 fn test_invalid_arg() {
@@ -1098,26 +1098,12 @@ fn test_df_hides_binfmt_misc_by_default() {
 /// Returns (success, stdout, stderr).
 #[cfg(target_os = "linux")]
 fn run_df_with_masked_proc(args: &str) -> Option<(bool, String, String)> {
-    use std::process::Command;
-
-    // Check if user namespaces are available
-    if !Command::new("unshare")
-        .args(["-rm", "true"])
-        .status()
-        .is_ok_and(|s| s.success())
-    {
-        return None;
-    }
-
     let df_path = TestScenario::new("df").bin_path.clone();
-    let output = Command::new("unshare")
-        .args(["-rm", "sh", "-c"])
-        .arg(format!(
-            "mount -t tmpfs tmpfs /proc && {} df {args}",
-            df_path.display()
-        ))
-        .output()
-        .ok()?;
+    let script = format!(
+        "mount -t tmpfs tmpfs /proc && {} df {args}",
+        df_path.display()
+    );
+    let output = run_in_rootless_unshare(&script)?;
 
     Some((
         output.status.success(),