diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index f7103f78b07..d8d7834f701 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -966,6 +966,8 @@ fn rename_dir_fallback( (_, _) => None, }; + // Retrieve xattrs before copying (directories use path-based operations + // since they cannot be opened in write mode for xattr operations) #[cfg(all(unix, not(any(target_os = "macos", target_os = "redox"))))] let xattrs = fsxattr::retrieve_xattrs(from).unwrap_or_else(|_| HashMap::new()); @@ -982,8 +984,14 @@ fn rename_dir_fallback( display_manager, ); + // Apply xattrs after directory contents are copied, ignoring ENOTSUP errors + // (filesystem doesn't support xattrs, which is acceptable for cross-device moves) #[cfg(all(unix, not(any(target_os = "macos", target_os = "redox"))))] - fsxattr::apply_xattrs(to, xattrs)?; + if let Err(e) = fsxattr::apply_xattrs(to, xattrs) { + if e.raw_os_error() != Some(libc::EOPNOTSUPP) { + return Err(e); + } + } result?; @@ -1050,8 +1058,35 @@ fn copy_dir_contents_recursive( pb.set_message(from_path.to_string_lossy().to_string()); } - if from_path.is_dir() { - // Recursively copy subdirectory + if from_path.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)] + { + copy_file_with_hardlinks_helper( + &from_path, + &to_path, + hardlink_tracker, + hardlink_scanner, + )?; + } + #[cfg(not(unix))] + { + rename_symlink_fallback(&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 from_path.is_dir() { + // Recursively copy subdirectory (only real directories, not symlinks) fs::create_dir_all(&to_path)?; // Print verbose message for directory @@ -1203,12 +1238,19 @@ fn rename_file_fallback( Ok(()) } -/// Copy xattrs from source to destination, ignoring ENOTSUP/EOPNOTSUPP errors. +/// Copy xattrs from source to destination using file descriptors, ignoring ENOTSUP/EOPNOTSUPP errors. +/// This version avoids TOCTOU races by operating on file descriptors rather than paths. /// These errors indicate the filesystem doesn't support extended attributes, /// which is acceptable when moving files across filesystems. #[cfg(all(unix, not(any(target_os = "macos", target_os = "redox"))))] fn copy_xattrs_if_supported(from: &Path, to: &Path) -> io::Result<()> { - match fsxattr::copy_xattrs(from, to) { + use std::fs::{File, OpenOptions}; + + // Open both files to pin their inodes, avoiding TOCTOU races during xattr operations + let source_file = File::open(from)?; + let dest_file = OpenOptions::new().write(true).open(to)?; + + match fsxattr::copy_xattrs_fd(&source_file, &dest_file) { Ok(()) => Ok(()), Err(e) if e.raw_os_error() == Some(libc::EOPNOTSUPP) => Ok(()), Err(e) => Err(e), diff --git a/src/uucore/src/lib/features/fsxattr.rs b/src/uucore/src/lib/features/fsxattr.rs index e9ae84c5f68..94e74d807e2 100644 --- a/src/uucore/src/lib/features/fsxattr.rs +++ b/src/uucore/src/lib/features/fsxattr.rs @@ -9,9 +9,11 @@ use itertools::Itertools; use std::collections::HashMap; use std::ffi::{OsStr, OsString}; +use std::fs::File; #[cfg(unix)] use std::os::unix::ffi::OsStrExt; use std::path::Path; +use xattr::FileExt; /// Copies extended attributes (xattrs) from one file or directory to another. /// @@ -32,6 +34,29 @@ pub fn copy_xattrs>(source: P, dest: P) -> std::io::Result<()> { Ok(()) } +/// Copies extended attributes (xattrs) from one file to another using file descriptors. +/// +/// This version avoids TOCTOU (time-of-check to time-of-use) races by operating +/// on open file descriptors rather than paths, ensuring all operations target +/// the same inodes throughout. +/// +/// # Arguments +/// +/// * `source` - A reference to the source file (open file descriptor). +/// * `dest` - A reference to the destination file (open file descriptor). +/// +/// # Returns +/// +/// A result indicating success or failure. +pub fn copy_xattrs_fd(source: &File, dest: &File) -> std::io::Result<()> { + for attr_name in source.list_xattr()? { + if let Some(value) = source.get_xattr(&attr_name)? { + dest.set_xattr(&attr_name, &value)?; + } + } + Ok(()) +} + /// Retrieves the extended attributes (xattrs) of a given file or directory. /// /// # Arguments @@ -51,6 +76,28 @@ pub fn retrieve_xattrs>(source: P) -> std::io::Result std::io::Result>> { + let mut attrs = HashMap::new(); + for attr_name in source.list_xattr()? { + if let Some(value) = source.get_xattr(&attr_name)? { + attrs.insert(attr_name, value); + } + } + Ok(attrs) +} + /// Applies extended attributes (xattrs) to a given file or directory. /// /// # Arguments @@ -71,6 +118,26 @@ pub fn apply_xattrs>( Ok(()) } +/// Applies extended attributes (xattrs) to a given file using a file descriptor. +/// +/// This version avoids TOCTOU races by operating on an open file descriptor +/// rather than a path, ensuring all operations target the same inode. +/// +/// # Arguments +/// +/// * `dest` - A reference to the file (open file descriptor). +/// * `xattrs` - A HashMap containing attribute names and their corresponding values. +/// +/// # Returns +/// +/// A result indicating success or failure. +pub fn apply_xattrs_fd(dest: &File, xattrs: HashMap>) -> std::io::Result<()> { + for (attr, value) in xattrs { + dest.set_xattr(&attr, &value)?; + } + Ok(()) +} + /// Checks if a file has an Access Control List (ACL) based on its extended attributes. /// /// # Arguments @@ -286,4 +353,59 @@ mod tests { assert!(has_security_cap_acl(&file_path)); } } + + #[test] + fn test_copy_xattrs_fd() { + use std::fs::OpenOptions; + + let temp_dir = tempdir().unwrap(); + let source_path = temp_dir.path().join("source.txt"); + let dest_path = temp_dir.path().join("dest.txt"); + + File::create(&source_path).unwrap(); + File::create(&dest_path).unwrap(); + + let test_attr = "user.test_fd"; + let test_value = b"test value fd"; + xattr::set(&source_path, test_attr, test_value).unwrap(); + + let source_file = File::open(&source_path).unwrap(); + let dest_file = OpenOptions::new().write(true).open(&dest_path).unwrap(); + + copy_xattrs_fd(&source_file, &dest_file).unwrap(); + + let copied_value = xattr::get(&dest_path, test_attr).unwrap().unwrap(); + assert_eq!(copied_value, test_value); + } + + #[test] + fn test_apply_and_retrieve_xattrs_fd() { + use std::fs::OpenOptions; + + let temp_dir = tempdir().unwrap(); + let file_path = temp_dir.path().join("test_file.txt"); + + File::create(&file_path).unwrap(); + + let mut test_xattrs = HashMap::new(); + let test_attr = "user.test_attr_fd"; + let test_value = b"test value fd"; + test_xattrs.insert(OsString::from(test_attr), test_value.to_vec()); + + // Apply using file descriptor + let file = OpenOptions::new().write(true).open(&file_path).unwrap(); + apply_xattrs_fd(&file, test_xattrs).unwrap(); + drop(file); + + // Retrieve using file descriptor + let file = File::open(&file_path).unwrap(); + let retrieved_xattrs = retrieve_xattrs_fd(&file).unwrap(); + assert!(retrieved_xattrs.contains_key(OsString::from(test_attr).as_os_str())); + assert_eq!( + retrieved_xattrs + .get(OsString::from(test_attr).as_os_str()) + .unwrap(), + test_value + ); + } } diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index 761f62b5a05..3d9cdbb6407 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -2836,3 +2836,54 @@ fn test_mv_xattr_enotsup_silent() { std::fs::remove_file("/dev/shm/mv_test").ok(); } } + +/// Test that symlinks inside directories are preserved during cross-device moves +/// (not expanded into full copies of their targets) +#[test] +#[cfg(target_os = "linux")] +fn test_mv_cross_device_symlink_preserved() { + use std::fs; + use std::os::unix::fs::symlink; + use tempfile::TempDir; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + // Create a directory with a symlink to /etc inside + at.mkdir("src_dir"); + at.write("src_dir/local.txt", "local content"); + symlink("/etc", at.plus("src_dir/etc_link")).expect("Failed to create symlink"); + + // Verify initial state + assert!(at.is_symlink("src_dir/etc_link")); + + // Force cross-filesystem move using /dev/shm (tmpfs) + let target_dir = + TempDir::new_in("/dev/shm/").expect("Unable to create temp directory in /dev/shm"); + let target_path = target_dir.path().join("dst_dir"); + + scene + .ucmd() + .arg("src_dir") + .arg(target_path.to_str().unwrap()) + .succeeds() + .no_stderr(); + + // Verify source was removed + assert!(!at.dir_exists("src_dir")); + + // Verify the symlink was preserved (not expanded) + let moved_symlink = target_path.join("etc_link"); + assert!( + moved_symlink.is_symlink(), + "etc_link should still be a symlink after cross-device move" + ); + assert_eq!( + fs::read_link(&moved_symlink).expect("Failed to read symlink"), + std::path::Path::new("/etc"), + "symlink should still point to /etc" + ); + + // Verify the regular file was also moved + assert!(target_path.join("local.txt").exists()); +}