diff --git a/src/uu/cp/src/copydir.rs b/src/uu/cp/src/copydir.rs index ee9de9ebdf8..d7066610602 100644 --- a/src/uu/cp/src/copydir.rs +++ b/src/uu/cp/src/copydir.rs @@ -377,6 +377,9 @@ pub(crate) fn copy_directory( // The directory we were in during the previous iteration let mut last_iter: Option = None; + // Keep track of all directories we've created that need permission fixes + let mut dirs_needing_permissions: Vec<(PathBuf, PathBuf)> = Vec::new(); + // Traverse the contents of the directory, copying each one. for direntry_result in WalkDir::new(root) .same_file_system(options.one_file_system) @@ -408,6 +411,14 @@ pub(crate) fn copy_directory( // `./a/b/c` into `./a/`, in which case we'll need to fix the // permissions of both `./a/b/c` and `./a/b`, in that order.) if direntry.file_type().is_dir() { + // Add this directory to our list for permission fixing later + let entry_for_tracking = + Entry::new(&context, direntry.path(), options.no_target_dir)?; + dirs_needing_permissions.push(( + entry_for_tracking.source_absolute, + entry_for_tracking.local_to_target, + )); + // If true, last_iter is not a parent of this iter. // The means we just exited a directory. let went_up = if let Some(last_iter) = &last_iter { @@ -452,25 +463,10 @@ pub(crate) fn copy_directory( } } - // Handle final directory permission fixes. - // This is almost the same as the permission-fixing code above, - // with minor differences (commented) - if let Some(last_iter) = last_iter { - let diff = last_iter.path().strip_prefix(root).unwrap(); - - // Do _not_ skip `.` this time, since we know we're done. - // This is where we fix the permissions of the top-level - // directory we just copied. - for p in diff.ancestors() { - let src = root.join(p); - let entry = Entry::new(&context, &src, options.no_target_dir)?; - - copy_attributes( - &entry.source_absolute, - &entry.local_to_target, - &options.attributes, - )?; - } + // Fix permissions for all directories we created + // This ensures that even sibling directories get their permissions fixed + for (source_path, dest_path) in dirs_needing_permissions { + copy_attributes(&source_path, &dest_path, &options.attributes)?; } // Also fix permissions for parent directories, diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 11a411a477c..1b3a5125b2a 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -4,7 +4,7 @@ // file that was distributed with this source code. // spell-checker:ignore (flags) reflink (fs) tmpfs (linux) rlimit Rlim NOFILE clob btrfs neve ROOTDIR USERDIR outfile uufs xattrs -// spell-checker:ignore bdfl hlsl IRWXO IRWXG nconfined matchpathcon libselinux-devel prwx doesnotexist +// spell-checker:ignore bdfl hlsl IRWXO IRWXG nconfined matchpathcon libselinux-devel prwx doesnotexist reftests subdirs use uucore::display::Quotable; #[cfg(feature = "feat_selinux")] use uucore::selinux::get_getfattr_output; @@ -6279,6 +6279,47 @@ fn test_cp_preserve_xattr_readonly_source() { ); } +#[test] +#[cfg(unix)] +fn test_cp_archive_preserves_directory_permissions() { + // Test for issue #8407 + let (at, mut ucmd) = at_and_ucmd!(); + + at.mkdir("test-images"); + + let subdirs = ["fail", "gif-test-suite", "randomly-modified", "reftests"]; + let mode = 0o755; + + for (i, subdir) in subdirs.iter().enumerate() { + let path = format!("test-images/{subdir}"); + at.mkdir(&path); + at.set_mode(&path, mode); + at.write(&format!("{path}/test{}.txt", i + 1), "test content"); + } + + ucmd.arg("-a") + .arg("test-images") + .arg("test-images-copy") + .succeeds(); + + let check_mode = |path: &str| { + let metadata = at.metadata(path); + let mode = metadata.permissions().mode(); + // Check that the permissions are 755 (only checking the last 9 bits) + assert_eq!( + mode & 0o777, + 0o755, + "Directory {} has incorrect permissions: {:o}", + path, + mode & 0o777 + ); + }; + + for subdir in subdirs { + check_mode(&format!("test-images-copy/{subdir}")); + } +} + #[test] #[cfg(unix)] fn test_cp_from_stdin() {