cp: fix -p to not preserve xattrs by default by AnarchistHoneybun · Pull Request #9708 · uutils/coreutils

AnarchistHoneybun · 2025-12-19T04:11:33Z

The -p flag should only preserve mode, ownership, and timestamps, matching GNU cp behavior. Extended attributes require explicit --preserve=xattr or -a (archive mode).

Changed Attributes::DEFAULT to set xattr preservation to No, preventing unintended security risks and filesystem compatibility issues.

Adds regression test.

github-actions · 2025-12-19T04:28:14Z

GNU testsuite comparison:

GNU test failed: tests/cp/acl. tests/cp/acl is passing on 'main'. Maybe you have to rebase?

codspeed-hq · 2025-12-19T06:49:27Z

Merging this PR will not alter performance

✅ 309 untouched benchmarks
⏩ 46 skipped benchmarks¹

_{Comparing AnarchistHoneybun:bug/cp-preserve-xattr-9704 (13877ac) with main (0a9fc3e)²}

46 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
No successful run was found on main (1f45676) during the generation of this report, so 0a9fc3e was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

github-actions · 2025-12-19T06:51:40Z

GNU testsuite comparison:

GNU test failed: tests/cp/acl. tests/cp/acl is passing on 'main'. Maybe you have to rebase?

github-actions · 2025-12-19T10:25:48Z

GNU testsuite comparison:

GNU test failed: tests/cp/acl. tests/cp/acl is passing on 'main'. Maybe you have to rebase?
Congrats! The gnu test tests/tail/inotify-dir-recreate is now passing!

github-actions · 2025-12-19T14:09:14Z

GNU testsuite comparison:

GNU test failed: tests/cp/acl. tests/cp/acl is passing on 'main'. Maybe you have to rebase?

ChrisDryden · 2025-12-19T17:20:55Z

+    );
+
+    // mode, ownership, and timestamps should be preserved
+    #[cfg(not(any(target_os = "freebsd", target_os = "macos")))]


I think these are redundant because its at the top of the test already

ChrisDryden · 2025-12-19T17:21:29Z

+    use std::process::Command;
+    use uutests::util::compare_xattrs;
+
+    let scene = TestScenario::new(util_name!());


Could you use the at_and_ucmd macro here?

ChrisDryden · 2025-12-19T17:28:55Z

I think the issue that was provided was an oversimplification and luckily the GNU test catches it.

The issue #9704 is correct that cp -p shouldn't preserve general xattrs, the implementation in PR #9708 is too expansive - it also removes ACL preservation, which GNU cp -p does support.

We need to make a fix with granular xattr handling that preserves ACLs but not other xattrs.

ChrisDryden · 2025-12-19T17:34:36Z

Maybe combining your approach with changing of the preserve code to also add ACL xattrs?

  /// Copies only ACL-related xattrs (system.posix_acl_*) from source to dest.
  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(())
  }



  handle_preserve(&attributes.mode, || -> CopyResult<()> {
      // existing mode code...

      // then
      #[cfg(all(unix, not(target_os = "android")))]
      copy_acl_xattrs(source, dest)?;

      Ok(())
  })?;

AnarchistHoneybun · 2025-12-20T03:59:07Z

i will look into this today

github-actions · 2025-12-20T04:12:26Z

GNU testsuite comparison:

GNU test failed: tests/cp/acl. tests/cp/acl is passing on 'main'. Maybe you have to rebase?
Congrats! The gnu test tests/tail/inotify-dir-recreate is now passing!

github-actions · 2025-12-20T11:52:33Z

GNU testsuite comparison:

GNU test failed: tests/cp/acl. tests/cp/acl is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

github-actions · 2025-12-21T05:26:29Z

GNU testsuite comparison:

Congrats! The gnu test tests/basenc/bounded-memory is now passing!

github-actions · 2025-12-21T08:53:10Z

GNU testsuite comparison:

Congrats! The gnu test tests/basenc/bounded-memory is now passing!

github-actions · 2025-12-21T09:59:51Z

GNU testsuite comparison:

Congrats! The gnu test tests/basenc/bounded-memory is now passing!

sylvestre · 2025-12-22T12:32:04Z

@@ -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};


clippy warning here :)

for some reason this doesn't come up for me when I locally run it, should I be adding the openbsd flag here too, that we have on :1722?

github-actions · 2025-12-23T03:36:22Z

GNU testsuite comparison:

Congrats! The gnu test tests/printf/printf-surprise is now passing!
Note: The gnu test tests/dd/no-allocate was skipped on 'main' but is now failing.
Note: The gnu test tests/misc/write-errors was skipped on 'main' but is now failing.

github-actions · 2026-01-05T05:35:47Z

GNU testsuite comparison:

Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

github-actions · 2026-02-11T12:16:10Z

GNU testsuite comparison:

GNU test failed: tests/pr/bounded-memory. tests/pr/bounded-memory is passing on 'main'. Maybe you have to rebase?
Note: The gnu test tests/expand/bounded-memory is now being skipped but was previously passing.

github-actions · 2026-04-14T20:29:15Z

GNU testsuite comparison:

GNU test failed: tests/cp/acl. tests/cp/acl is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/cut/bounded-memory (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/tail/retry is no longer failing!
Congrats! The gnu test tests/cut/cut-huge-range is now passing!

Fixes uutils#9704 The -p flag should only preserve mode, ownership, and timestamps, matching GNU cp behavior. Extended attributes require explicit --preserve=xattr or -a (archive mode). Changed Attributes::DEFAULT to set xattr preservation to No, preventing unintended security risks and filesystem compatibility issues. Adds regression test.

ChrisDryden reviewed Dec 19, 2025

View reviewed changes

ChrisDryden mentioned this pull request Dec 19, 2025

GNU coreutils 9.9: Detailed Test report 12/19 #9729

Closed

sylvestre reviewed Dec 21, 2025

View reviewed changes

Comment thread src/uu/cp/src/cp.rs Outdated

sylvestre reviewed Dec 22, 2025

View reviewed changes

sylvestre force-pushed the bug/cp-preserve-xattr-9704 branch from 12ddfdd to c00b554 Compare April 14, 2026 20:19

sylvestre force-pushed the bug/cp-preserve-xattr-9704 branch 2 times, most recently from c00b554 to 13877ac Compare April 14, 2026 20:59

Uh oh!

Conversation

AnarchistHoneybun commented Dec 19, 2025

Uh oh!

github-actions Bot commented Dec 19, 2025

Uh oh!

codspeed-hq Bot commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Footnotes

Uh oh!

github-actions Bot commented Dec 19, 2025

Uh oh!

github-actions Bot commented Dec 19, 2025

Uh oh!

github-actions Bot commented Dec 19, 2025

Uh oh!

ChrisDryden Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

ChrisDryden Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

ChrisDryden commented Dec 19, 2025

Uh oh!

ChrisDryden commented Dec 19, 2025

Uh oh!

AnarchistHoneybun commented Dec 20, 2025

Uh oh!

github-actions Bot commented Dec 20, 2025

Uh oh!

github-actions Bot commented Dec 20, 2025

Uh oh!

github-actions Bot commented Dec 21, 2025

Uh oh!

Uh oh!

github-actions Bot commented Dec 21, 2025

Uh oh!

github-actions Bot commented Dec 21, 2025

Uh oh!

sylvestre Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

AnarchistHoneybun Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Dec 23, 2025

Uh oh!

github-actions Bot commented Jan 5, 2026

Uh oh!

github-actions Bot commented Feb 11, 2026

Uh oh!

github-actions Bot commented Apr 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codspeed-hq Bot commented Dec 19, 2025 •

edited

Loading