mirror of
https://github.com/uutils/coreutils.git
synced 2026-05-06 07:26:38 -04:00
cp: don't preserve xattrs with -p by default (#9704)
GNU `cp -p` preserves mode, ownership, and timestamps. xattrs are
NOT preserved unless the user asks for them via `--preserve=xattr`
or `-a`. uutils's `Attributes::DEFAULT` had xattr set to
`Preserve::Yes { required: true }`, which (1) diverges from GNU and
breaks scripts that expect the stock behavior, (2) leaks security
xattrs like file capabilities and SELinux labels into copies when
run as root, and (3) fails hard on destinations that don't support
xattrs.
Remove the xattr override in `Attributes::DEFAULT` so it inherits
`Preserve::No` from `Attributes::NONE`. `Attributes::ALL` (used by
`-a` and `--preserve=all`) still sets xattr to Yes, and
`--preserve=xattr` still works as before.
This commit is contained in:
committed by
Dorian Péron
parent
ef5d752282
commit
7ba1bf857c
+13
-3
@@ -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(())
|
||||
|
||||
@@ -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<P: AsRef<Path>>(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<P: AsRef<Path>>(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
|
||||
|
||||
@@ -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() {
|
||||
|
||||
Reference in New Issue
Block a user