Merge pull request #7009 from sylvestre/xattrs

cp: when copying a read only file, make sure that the xattrs can be set properly

.github/workflows/CICD.yml

@@ -199,6 +199,11 @@ jobs:
         # * ensure '.clippy.toml' MSRV configuration setting is equal to ${{ env.RUST_MIN_SRV }}
         CLIPPY_MSRV=$(grep -P "(?i)^\s*msrv\s*=\s*" .clippy.toml | grep -oP "\d+([.]\d+)+")
         if [ "${CLIPPY_MSRV}" != "${{ env.RUST_MIN_SRV }}" ]; then { echo "::error file=.clippy.toml::Incorrect MSRV configuration for clippy (found '${CLIPPY_MSRV}'; should be '${{ env.RUST_MIN_SRV }}'); update '.clippy.toml' with 'msrv = \"${{ env.RUST_MIN_SRV }}\"'" ; exit 1 ; } ; fi
+    - name: Install/setup prerequisites
+      shell: bash
+      run: |
+        # Install a package for one of the tests
+        sudo apt-get -y update ; sudo apt-get -y install attr
     - name: Info
       shell: bash
       run: |

.vscode/cspell.dictionaries/jargon.wordlist.txt

@@ -113,6 +113,7 @@ semiprime
 semiprimes
 setcap
 setfacl
+setfattr
 shortcode
 shortcodes
 siginfo

src/uu/cp/src/cp.rs

@@ -1518,6 +1518,42 @@ fn handle_preserve<F: Fn() -> CopyResult<()>>(p: &Preserve, f: F) -> CopyResult<
     Ok(())
 }
 
+/// Copies extended attributes (xattrs) from `source` to `dest`, ensuring that `dest` is temporarily
+/// user-writable if needed and restoring its original permissions afterward. This avoids “Operation
+/// not permitted” errors on read-only files. Returns an error if permission or metadata operations fail,
+/// or if xattr copying fails.
+#[cfg(all(unix, not(target_os = "android")))]
+fn copy_extended_attrs(source: &Path, dest: &Path) -> CopyResult<()> {
+    let metadata = fs::symlink_metadata(dest)?;
+
+    // Check if the destination file is currently read-only for the user.
+    let mut perms = metadata.permissions();
+    let was_readonly = perms.readonly();
+
+    // Temporarily grant user write if it was read-only.
+    if was_readonly {
+        #[allow(clippy::permissions_set_readonly_false)]
+        perms.set_readonly(false);
+        fs::set_permissions(dest, perms)?;
+    }
+
+    // Perform the xattr copy and capture any potential error,
+    // so we can restore permissions before returning.
+    let copy_xattrs_result = copy_xattrs(source, dest);
+
+    // Restore read-only if we changed it.
+    if was_readonly {
+        let mut revert_perms = fs::symlink_metadata(dest)?.permissions();
+        revert_perms.set_readonly(true);
+        fs::set_permissions(dest, revert_perms)?;
+    }
+
+    // If copying xattrs failed, propagate that error now.
+    copy_xattrs_result?;
+
+    Ok(())
+}
+
 /// Copy the specified attributes from one path to another.
 pub(crate) fn copy_attributes(
     source: &Path,
@@ -1607,7 +1643,7 @@ pub(crate) fn copy_attributes(
     handle_preserve(&attributes.xattr, || -> CopyResult<()> {
         #[cfg(all(unix, not(target_os = "android")))]
         {
-            copy_xattrs(source, dest)?;
+            copy_extended_attrs(source, dest)?;
         }
         #[cfg(not(all(unix, not(target_os = "android"))))]
         {

tests/by-util/test_cp.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 (flags) reflink (fs) tmpfs (linux) rlimit Rlim NOFILE clob btrfs neve ROOTDIR USERDIR procfs outfile uufs xattrs
-// spell-checker:ignore bdfl hlsl IRWXO IRWXG
+// spell-checker:ignore bdfl hlsl IRWXO IRWXG getfattr
 use crate::common::util::TestScenario;
 #[cfg(not(windows))]
 use std::fs::set_permissions;
@@ -5920,3 +5920,86 @@ fn test_cp_no_file() {
         .code_is(1)
         .stderr_contains("error: the following required arguments were not provided:");
 }
+
+#[test]
+#[cfg(all(
+    unix,
+    not(any(target_os = "android", target_os = "macos", target_os = "openbsd"))
+))]
+fn test_cp_preserve_xattr_readonly_source() {
+    use crate::common::util::compare_xattrs;
+    use std::process::Command;
+
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    let source_file = "a";
+    let dest_file = "e";
+
+    at.touch(source_file);
+
+    let xattr_key = "user.test";
+    match Command::new("setfattr")
+        .args([
+            "-n",
+            xattr_key,
+            "-v",
+            "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;
+        }
+    }
+
+    let getfattr_output = Command::new("getfattr")
+        .args([&at.plus_as_string(source_file)])
+        .output()
+        .expect("Failed to run `getfattr` on the destination file");
+
+    assert!(
+        getfattr_output.status.success(),
+        "getfattr did not run successfully: {}",
+        String::from_utf8_lossy(&getfattr_output.stderr)
+    );
+
+    let stdout = String::from_utf8_lossy(&getfattr_output.stdout);
+    assert!(
+        stdout.contains(xattr_key),
+        "Expected '{}' not found in getfattr output:\n{}",
+        xattr_key,
+        stdout
+    );
+
+    at.set_readonly(source_file);
+    assert!(scene
+        .fixtures
+        .metadata(source_file)
+        .permissions()
+        .readonly());
+
+    scene
+        .ucmd()
+        .args(&[
+            "--preserve=xattr",
+            &at.plus_as_string(source_file),
+            &at.plus_as_string(dest_file),
+        ])
+        .succeeds()
+        .no_output();
+
+    assert!(scene.fixtures.metadata(dest_file).permissions().readonly());
+    assert!(
+        compare_xattrs(&at.plus(source_file), &at.plus(dest_file)),
+        "Extended attributes were not preserved"
+    );
+}