cp: fix -p to not preserve xattrs by default

src/uu/cp/src/cp.rs

@@ -16,7 +16,7 @@ use std::os::unix::net::UnixListener;
 use std::path::{Path, PathBuf, StripPrefixError};
 use std::{fmt, io};
 #[cfg(all(unix, not(target_os = "android")))]
-use uucore::fsxattr::copy_xattrs;
+use uucore::fsxattr::{copy_acl_xattrs, copy_xattrs};
 use uucore::translate;
 
 use clap::{Arg, ArgAction, ArgMatches, Command, builder::ValueParser, value_parser};
@@ -897,7 +897,7 @@ impl Attributes {
         ownership: Preserve::Yes { required: true },
         mode: Preserve::Yes { required: true },
         timestamps: Preserve::Yes { required: true },
-        xattr: Preserve::Yes { required: true },
+        xattr: Preserve::No { explicit: false },
         ..Self::NONE
     };
 
@@ -1715,6 +1715,12 @@ pub(crate) fn copy_attributes(
             exacl::getfacl(source, None)
                 .and_then(|acl| exacl::setfacl(&[dest], &acl, None))
                 .map_err(|err| CpError::Error(err.to_string()))?;
+
+            // When preserving mode with -p, also preserve ACL xattrs
+            // ACLs are stored as xattrs (system.posix_acl_*) and should be
+            // preserved even when general xattrs are not
+            #[cfg(all(unix, not(target_os = "android")))]
+            copy_acl_xattrs(source, dest)?;
         }
 
         Ok(())

src/uucore/src/lib/features/fsxattr.rs

@@ -29,6 +29,32 @@ pub fn copy_xattrs<P: AsRef<Path>>(source: P, dest: P) -> std::io::Result<()> {
     Ok(())
 }
 
+/// Copies only ACL-related xattrs (system.posix_acl_*) from source to dest.
+///
+/// This is used for preserving ACLs with `-p` flag, which should preserve
+/// ACLs but not other extended attributes.
+///
+/// # Arguments
+///
+/// * `source` - A reference to the source path.
+/// * `dest` - A reference to the destination path.
+///
+/// # Returns
+///
+/// A result indicating success or failure.
+pub fn copy_acl_xattrs<P: AsRef<Path>>(source: P, dest: P) -> std::io::Result<()> {
+    for attr_name in xattr::list(&source)? {
+        if let Some(name_str) = attr_name.to_str() {
+            if name_str.starts_with("system.posix_acl_") {
+                if let Some(value) = xattr::get(&source, &attr_name)? {
+                    xattr::set(&dest, &attr_name, &value)?;
+                }
+            }
+        }
+    }
+    Ok(())
+}
+
 /// Retrieves the extended attributes (xattrs) of a given file or directory.
 ///
 /// # Arguments

tests/by-util/test_cp.rs

@@ -1773,6 +1773,112 @@ fn test_cp_preserve_xattr() {
     }
 }
 
+// -p preserves mode, ownership, and timestamps, but not xattrs
+// xattrs require explicit --preserve=xattr or -a
+#[test]
+#[cfg(all(
+    unix,
+    not(any(target_os = "android", target_os = "macos", target_os = "openbsd"))
+))]
+fn test_cp_preserve_default_no_xattr() {
+    use std::process::Command;
+    use uutests::util::compare_xattrs;
+
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    let source_file = "source_with_xattr";
+    let dest_file_p = "dest_with_p_flag";
+    let dest_file_explicit = "dest_with_explicit_xattr";
+
+    at.touch(source_file);
+
+    // set xattr on source
+    let xattr_key = "user.test_preserve";
+    let xattr_value = "test_value";
+    match Command::new("setfattr")
+        .args([
+            "-n",
+            xattr_key,
+            "-v",
+            xattr_value,
+            &at.plus_as_string(source_file),
+        ])
+        .status()
+        .map(|status| status.code())
+    {
+        Ok(Some(0)) => {}
+        Ok(_) => {
+            println!("test skipped: setfattr failed");
+            return;
+        }
+        Err(e) => {
+            println!("test skipped: setfattr failed with {e}");
+            return;
+        }
+    }
+
+    // verify xattr was set
+    let getfattr_output = Command::new("getfattr")
+        .args([&at.plus_as_string(source_file)])
+        .output()
+        .expect("failed to run getfattr");
+
+    assert!(
+        getfattr_output.status.success(),
+        "getfattr failed: {}",
+        String::from_utf8_lossy(&getfattr_output.stderr)
+    );
+
+    let stdout = String::from_utf8_lossy(&getfattr_output.stdout);
+    assert!(
+        stdout.contains(xattr_key),
+        "xattr not found on source: {xattr_key}"
+    );
+
+    // -p should not preserve xattrs
+    scene
+        .ucmd()
+        .args(&[
+            "-p",
+            &at.plus_as_string(source_file),
+            &at.plus_as_string(dest_file_p),
+        ])
+        .succeeds()
+        .no_output();
+
+    // xattrs should not be copied
+    assert!(
+        !compare_xattrs(&at.plus(source_file), &at.plus(dest_file_p)),
+        "cp -p incorrectly preserved xattrs"
+    );
+
+    // mode, ownership, and timestamps should be preserved
+    #[cfg(not(any(target_os = "freebsd", target_os = "macos")))]
+    {
+        let metadata_src = at.metadata(source_file);
+        let metadata_dst = at.metadata(dest_file_p);
+        assert_metadata_eq!(metadata_src, metadata_dst);
+    }
+
+    // --preserve=xattr should explicitly preserve xattrs
+    scene
+        .ucmd()
+        .args(&[
+            "--preserve=xattr",
+            &at.plus_as_string(source_file),
+            &at.plus_as_string(dest_file_explicit),
+        ])
+        .succeeds()
+        .no_output();
+
+    // xattrs should be copied
+    assert!(
+        compare_xattrs(&at.plus(source_file), &at.plus(dest_file_explicit)),
+        "cp --preserve=xattr did not preserve xattrs"
+    );
+}
+
 #[test]
 #[cfg(all(target_os = "linux", not(feature = "feat_selinux")))]
 fn test_cp_preserve_all_context_fails_on_non_selinux() {