chown, chgrp: say "cannot read directory" for a directory that does not open

src/uucore/locales/en-US.ftl

@@ -99,6 +99,7 @@ mode-diag-help-syntax = a mode is either octal, as in 644, or clauses such as u+
 
 # Shared recursive chown/chgrp/chmod diagnostics (uucore::perms)
 perms-cannot-access = cannot access { $file }: { $error }
+perms-cannot-read-directory = cannot read directory { $file }: { $error }
 perms-cannot-access-replaced = cannot access { $file }: replaced while it was being processed
 
 # Encoding/decoding error messages (base32, base64, basenc)

src/uucore/src/lib/features/perms.rs

@@ -541,9 +541,8 @@ impl ChownExecutor {
                 *ret = 1;
                 if self.verbosity.level != VerbosityLevel::Silent {
                     show_error!(
-                        "cannot read directory {}: {}",
-                        dir_path.quote(),
-                        strip_errno(&e)
+                        "{}",
+                        translate!("perms-cannot-read-directory", "file" => dir_path.quote(), "error" => strip_errno(&e))
                     );
                 }
                 return;
@@ -632,7 +631,7 @@ impl ChownExecutor {
                         if self.verbosity.level != VerbosityLevel::Silent {
                             show_error!(
                                 "{}",
-                                translate!("perms-cannot-access", "file" => entry_path.quote(), "error" => strip_errno(&e))
+                                translate!("perms-cannot-read-directory", "file" => entry_path.quote(), "error" => strip_errno(&e))
                             );
                         }
                     }
@@ -662,16 +661,28 @@ impl ChownExecutor {
             .follow_links(self.traverse_symlinks == TraverseSymlinks::All)
             .min_depth(1)
             .into_iter();
+        // The directory whose entry came last: walkdir reports a directory it
+        // cannot read right after that entry, under the same path. The root is
+        // below min_depth, so it never comes out as an entry: start from it.
+        let mut last_dir = Some(root.to_path_buf());
         // We can't use a for loop because we need to manipulate the iterator inside the loop.
         while let Some(entry) = iterator.next() {
             let entry = match entry {
                 Err(e) => {
                     ret = 1;
+                    if self.verbosity.level == VerbosityLevel::Silent {
+                        continue;
+                    }
                     if let Some(path) = e.path() {
+                        let message = if last_dir.as_deref() == Some(path) {
+                            "perms-cannot-read-directory"
+                        } else {
+                            "perms-cannot-access"
+                        };
                         show_error!(
                             "{}",
                             translate!(
-                                "perms-cannot-access",
+                                message,
                                 "file" => path.quote(),
                                 "error" => if let Some(error) = e.io_error() {
                                     strip_errno(error)
@@ -688,6 +699,7 @@ impl ChownExecutor {
                 Ok(entry) => entry,
             };
             let path = entry.path();
+            last_dir = entry.file_type().is_dir().then(|| path.to_path_buf());
 
             let Some(meta) = self.obtain_meta(path, self.dereference) else {
                 ret = 1;
@@ -807,7 +819,7 @@ impl ChownExecutor {
                 if self.verbosity.level != VerbosityLevel::Silent {
                     show_error!(
                         "{}",
-                        translate!("perms-cannot-access", "file" => path.quote(), "error" => strip_errno(&e))
+                        translate!("perms-cannot-read-directory", "file" => path.quote(), "error" => strip_errno(&e))
                     );
                 }
             })

tests/by-util/test_chgrp.rs

@@ -89,6 +89,43 @@ fn test_fail_silently() {
     }
 }
 
+#[test]
+#[cfg(unix)]
+fn test_chgrp_recursive_unreadable_directory() {
+    use std::fs::{Permissions, set_permissions};
+    use std::os::unix::fs::PermissionsExt;
+
+    if getegid().is_root() {
+        // root can read any directory
+        return;
+    }
+    let (at, mut ucmd) = at_and_ucmd!();
+    at.mkdir_all("d/a/y");
+    set_permissions(at.plus_as_string("d/a"), Permissions::from_mode(0o311)).unwrap();
+
+    ucmd.args(&["-R", "--reference=d", "d"])
+        .fails_with_code(1)
+        .stderr_is("chgrp: cannot read directory 'd/a': Permission denied\n");
+
+    // -f hides the message; the failure still sets the exit status.
+    let (at, mut ucmd) = at_and_ucmd!();
+    at.mkdir_all("d/a/y");
+    set_permissions(at.plus_as_string("d/a"), Permissions::from_mode(0o311)).unwrap();
+    ucmd.args(&["-f", "-R", "--reference=d", "d"])
+        .fails_with_code(1)
+        .no_output();
+    set_permissions(at.plus_as_string("d/a"), Permissions::from_mode(0o755)).unwrap();
+
+    // The operand itself.
+    let (at, mut ucmd) = at_and_ucmd!();
+    at.mkdir("d");
+    set_permissions(at.plus_as_string("d"), Permissions::from_mode(0o311)).unwrap();
+    ucmd.args(&["-R", "--reference=d", "d"])
+        .fails_with_code(1)
+        .stderr_is("chgrp: cannot read directory 'd': Permission denied\n");
+    set_permissions(at.plus_as_string("d"), Permissions::from_mode(0o755)).unwrap();
+}
+
 #[test]
 fn test_preserve_root() {
     // It's weird that on OS X, `realpath /etc/..` returns '/private'
@@ -336,7 +373,7 @@ fn test_permission_denied() {
             .arg(group.as_raw().to_string())
             .arg("dir")
             .fails()
-            .stderr_only("chgrp: cannot access 'dir': Permission denied\n");
+            .stderr_only("chgrp: cannot read directory 'dir': Permission denied\n");
     }
 }
 
@@ -355,7 +392,7 @@ fn test_subdir_permission_denied() {
             .arg(group.as_raw().to_string())
             .arg("dir")
             .fails()
-            .stderr_only("chgrp: cannot access 'dir/subdir': Permission denied\n");
+            .stderr_only("chgrp: cannot read directory 'dir/subdir': Permission denied\n");
     }
 }
 

tests/by-util/test_chown.rs

@@ -766,6 +766,46 @@ fn test_chown_owner_group_mix() {
         .stderr_contains("failed to change");
 }
 
+#[test]
+#[cfg(unix)]
+fn test_chown_recursive_unreadable_directory() {
+    use std::fs::{Permissions, set_permissions};
+    use std::os::unix::fs::PermissionsExt;
+
+    if rustix::process::geteuid().is_root() {
+        // root can read any directory
+        return;
+    }
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+    at.mkdir_all("d/a/y");
+    set_permissions(at.plus_as_string("d/a"), Permissions::from_mode(0o311)).unwrap();
+
+    scene
+        .ucmd()
+        .args(&["-R", "--reference=d", "d"])
+        .fails_with_code(1)
+        .stderr_is("chown: cannot read directory 'd/a': Permission denied\n");
+
+    // -f hides the message; the failure still sets the exit status.
+    scene
+        .ucmd()
+        .args(&["-f", "-R", "--reference=d", "d"])
+        .fails_with_code(1)
+        .no_output();
+
+    set_permissions(at.plus_as_string("d/a"), Permissions::from_mode(0o755)).unwrap();
+
+    // The operand itself.
+    set_permissions(at.plus_as_string("d"), Permissions::from_mode(0o311)).unwrap();
+    scene
+        .ucmd()
+        .args(&["-R", "--reference=d", "d"])
+        .fails_with_code(1)
+        .stderr_is("chown: cannot read directory 'd': Permission denied\n");
+    set_permissions(at.plus_as_string("d"), Permissions::from_mode(0o755)).unwrap();
+}
+
 #[test]
 fn test_chown_recursive() {
     let scene = TestScenario::new(util_name!());