cp: apply the finalize chmod without following a symlink

src/uu/cp/src/cp.rs

@@ -1881,7 +1881,7 @@
             #[cfg(not(unix))]
             let source_perms = source_metadata.permissions();
 
-            fs::set_permissions(dest, source_perms)
+            chmod_nofollow(dest, &source_perms)
                 .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?;
             // FIXME: Implement this for windows as well
             #[cfg(feature = "feat_acl")]
@@ -2511,6 +2511,42 @@
 ///
 /// The original permissions of `source` will be copied to `dest`
 /// after a successful copy.
+/// Apply `permissions` to `dest` without following a symlink at the final component.
+///
+/// Both callers test `dest` for being a symlink well before the chmod, so a symlink
+/// swapped in inside that gap would redirect the mode onto the link's target.
+/// `chmod(2)` cannot change a symlink's permissions, so `EOPNOTSUPP` here means
+/// exactly the "do nothing" case the callers' guards describe.
+#[cfg(unix)]
+fn chmod_nofollow(dest: &Path, permissions: &Permissions) -> io::Result<()> {
+    use uucore::safe_traversal::{DirFd, SymlinkBehavior};
+
+    let (Some(parent), Some(name)) = (dest.parent(), dest.file_name()) else {
+        // `.`, `..` and `/` have no final component to anchor against. They are
+        // always directories and never a symlink themselves, so there is nothing
+        // to be redirected: chmod the directory through its own fd.
+        let dir_fd = DirFd::open(dest, SymlinkBehavior::NoFollow)?;
+        return dir_fd.fchmod(permissions.mode());
+    };
+    // `Path::parent()` yields "" for a bare filename.
+    let parent = if parent.as_os_str().is_empty() {
+        Path::new(".")
+    } else {
+        parent
+    };
+
+    let dir_fd = DirFd::open(parent, SymlinkBehavior::Follow)?;
+    match dir_fd.chmod_at(name, permissions.mode(), SymlinkBehavior::NoFollow) {
+        Err(e) if matches!(e.raw_os_error(), Some(libc::EOPNOTSUPP) | Some(libc::ELOOP)) => Ok(()),
+        other => other,
+    }
+}
+
+#[cfg(not(unix))]
+fn chmod_nofollow(dest: &Path, permissions: &Permissions) -> io::Result<()> {
+    fs::set_permissions(dest, permissions.clone())
+}
+
 #[allow(clippy::cognitive_complexity, clippy::too_many_arguments)]
 fn copy_file(
     progress_bar: Option<&ProgressBar>,
@@ -2703,7 +2739,7 @@
         //
         // FWIW, the OS will throw an error later, on the write op, if
         // the user does not have permission to write to the file.
-        fs::set_permissions(dest, dest_permissions).ok();
+        chmod_nofollow(dest, &dest_permissions).ok();
     }
 
     let copy_attributes_result = if options.dereference(source_in_command_line) {

tests/by-util/test_cp.rs

@@ -6695,6 +6695,47 @@ fn test_cp_parents_symlink_permissions_file() {
     );
 }
 
+/// The finalize chmod goes through a parent-fd + `NoFollow` chmod rather than a
+/// path-based `set_permissions`. Pin the modes it is responsible for, including
+/// the setuid and read-only cases, so the no-follow path cannot silently stop
+/// applying them.
+#[test]
+#[cfg(unix)]
+fn test_cp_preserve_mode_via_nofollow_chmod() {
+    for mode in [0o644, 0o600, 0o755, 0o444, 0o4755] {
+        let scene = TestScenario::new(util_name!());
+        let at = &scene.fixtures;
+        at.write("src", "hi");
+        at.set_mode("src", mode);
+
+        scene.ucmd().args(&["-p", "src", "dst"]).succeeds();
+
+        assert_eq!(
+            at.metadata("dst").permissions().mode() & 0o7777,
+            mode,
+            "cp -p did not reproduce mode {mode:o}"
+        );
+    }
+}
+
+/// Overwriting an existing destination keeps the destination's mode, not the
+/// source's — the other branch feeding the same chmod.
+#[test]
+#[cfg(unix)]
+fn test_cp_existing_dest_keeps_its_mode() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+    at.write("src", "new");
+    at.set_mode("src", 0o600);
+    at.write("dst", "old");
+    at.set_mode("dst", 0o640);
+
+    scene.ucmd().args(&["src", "dst"]).succeeds();
+
+    assert_eq!(at.metadata("dst").permissions().mode() & 0o7777, 0o640);
+    assert_eq!(at.read("dst"), "new");
+}
+
 /// A destination subdirectory that is really a symlink must not be descended
 /// into: doing so writes the source subtree through the link and out of the
 /// destination tree. GNU refuses with "cannot overwrite non-directory ... with