diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index b1f81ea56..e1a1dc122 100644 --- a/src/uu/cp/src/cp.rs +++ b/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, copy_xattrs_skip_selinux}; +use uucore::fsxattr::{copy_acls, copy_xattrs, copy_xattrs_skip_selinux}; use uucore::translate; use clap::{Arg, ArgAction, ArgMatches, Command, builder::ValueParser, value_parser}; @@ -921,13 +921,15 @@ impl Attributes { xattr: Preserve::No { explicit: false }, }; - // TODO: ownership is required if the user is root, for non-root users it's not required. + // xattr is intentionally NOT in DEFAULT — GNU `cp -p` only preserves + // mode, ownership, and timestamps. Default xattr preservation leaks + // capability / SELinux labels into copies and fails hard on filesystems + // without xattr support. See issue #9704. pub const DEFAULT: Self = Self { #[cfg(unix)] ownership: Preserve::Yes { required: true }, mode: Preserve::Yes { required: true }, timestamps: Preserve::Yes { required: true }, - xattr: Preserve::Yes { required: true }, ..Self::NONE }; @@ -1829,6 +1831,14 @@ 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()))?; + // GNU `cp -p` preserves POSIX ACLs as part of mode. On Linux the + // ACLs are stored as `system.posix_acl_*` xattrs; copy just those + // so we keep ACL parity with GNU without preserving user xattrs + // (which are intentionally excluded from the default -p set per + // issue #9704). Best-effort: ignore failures on filesystems that + // do not support ACL xattrs. + #[cfg(all(unix, not(target_os = "android")))] + copy_acls(source, dest); } Ok(()) diff --git a/src/uucore/src/lib/features/fsxattr.rs b/src/uucore/src/lib/features/fsxattr.rs index d96832646..2b42edaa3 100644 --- a/src/uucore/src/lib/features/fsxattr.rs +++ b/src/uucore/src/lib/features/fsxattr.rs @@ -3,7 +3,7 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore getxattr posix_acl_default +// spell-checker:ignore getxattr posix_acl_default posix_acl_access //! Set of functions to manage xattr on files and dirs use itertools::Itertools; @@ -45,6 +45,28 @@ pub fn copy_xattrs_skip_selinux>(source: P, dest: P) -> std::io:: Ok(()) } +/// Copies only the POSIX ACL xattrs (`system.posix_acl_access` and +/// `system.posix_acl_default`) from `source` to `dest`. +/// +/// GNU `cp -p` preserves ACLs as part of mode preservation but does not +/// preserve other (user/security) xattrs unless `--preserve=xattr` or +/// `-a` is requested. On Linux, POSIX ACLs are stored as the two `system.*` +/// xattrs above; copying them here without copying the rest gives the +/// GNU-compatible "preserve mode (incl. ACLs) but not user xattrs" behavior. +/// +/// Errors from the underlying xattr calls are silently ignored: filesystems +/// without ACL/xattr support are common, and GNU cp itself does not surface +/// failures here when `mode` is the only thing being preserved. +#[cfg(unix)] +pub fn copy_acls>(source: P, dest: P) { + for name in ["system.posix_acl_access", "system.posix_acl_default"] { + if let Ok(Some(value)) = xattr::get(&source, name) { + // Best-effort: silently skip if dest doesn't support ACL xattrs. + let _ = xattr::set(&dest, name, &value); + } + } +} + /// Retrieves the extended attributes (xattrs) of a given file or directory. /// /// # Arguments diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 8ca8d1889..83eefd211 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -1768,6 +1768,47 @@ fn test_cp_preserve_all() { } } +// GNU `cp -p` preserves mode, ownership, and timestamps but NOT xattrs. +// xattr preservation requires explicit `--preserve=xattr` or `-a`. See #9704. +#[test] +#[cfg(all( + unix, + not(any(target_os = "android", target_os = "openbsd", target_os = "macos")) +))] +fn test_cp_p_does_not_preserve_xattr_by_default() { + use std::process::Command; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.touch("src"); + + let xattr_key = "user.test_preserve_p"; + let setfattr = Command::new("setfattr") + .args(["-n", xattr_key, "-v", "v", &at.plus_as_string("src")]) + .status(); + match setfattr { + Ok(s) if s.success() => {} + _ => { + println!("test skipped: setfattr not available / filesystem rejects xattrs"); + return; + } + } + + scene + .ucmd() + .args(&["-p", &at.plus_as_string("src"), &at.plus_as_string("dst")]) + .succeeds(); + + let out = Command::new("getfattr") + .args(["--only-values", "-n", xattr_key, &at.plus_as_string("dst")]) + .output() + .expect("getfattr failed"); + assert!( + !out.status.success(), + "cp -p should not preserve xattrs by default, but '{xattr_key}' was copied" + ); +} + #[test] #[cfg(all(unix, not(any(target_os = "android", target_os = "openbsd"))))] fn test_cp_preserve_xattr() {