safe traversal: adjust chmod & chgrp to use it (#8632)

* chown: implement safe traversal

* uucore/safe_traversal: add secure chmod functions

* safe traversal: adjust chmod & chgrp to use it

* chmod dedup some code

Co-authored-by: Daniel Hofstetter <daniel.hofstetter@42dh.com>

* address review comments

---------

Co-authored-by: Daniel Hofstetter <daniel.hofstetter@42dh.com>

src/uu/chmod/Cargo.toml

@@ -20,7 +20,13 @@ path = "src/chmod.rs"
 [dependencies]
 clap = { workspace = true }
 thiserror = { workspace = true }
-uucore = { workspace = true, features = ["entries", "fs", "mode", "perms"] }
+uucore = { workspace = true, features = [
+  "entries",
+  "fs",
+  "mode",
+  "perms",
+  "safe-traversal",
+] }
 fluent = { workspace = true }
 
 [[bin]]

src/uu/chmod/src/chmod.rs

@@ -17,6 +17,9 @@ use uucore::fs::display_permissions_unix;
 use uucore::libc::mode_t;
 use uucore::mode;
 use uucore::perms::{TraverseSymlinks, configure_symlink_and_recursion};
+
+#[cfg(target_os = "linux")]
+use uucore::safe_traversal::DirFd;
 use uucore::{format_usage, show, show_error};
 
 use uucore::translate;
@@ -266,6 +269,104 @@ struct Chmoder {
 }
 
 impl Chmoder {
+    /// Calculate the new mode based on the current mode and the chmod specification.
+    /// Returns (`new_mode`, `naively_expected_new_mode`) for symbolic modes, or (`new_mode`, `new_mode`) for numeric/reference modes.
+    fn calculate_new_mode(&self, current_mode: u32, is_dir: bool) -> UResult<(u32, u32)> {
+        match self.fmode {
+            Some(mode) => Ok((mode, mode)),
+            None => {
+                let cmode_unwrapped = self.cmode.clone().unwrap();
+                let mut new_mode = current_mode;
+                let mut naively_expected_new_mode = current_mode;
+
+                for mode in cmode_unwrapped.split(',') {
+                    let result = if mode.chars().any(|c| c.is_ascii_digit()) {
+                        mode::parse_numeric(new_mode, mode, is_dir).map(|v| (v, v))
+                    } else {
+                        mode::parse_symbolic(new_mode, mode, mode::get_umask(), is_dir).map(|m| {
+                            // calculate the new mode as if umask was 0
+                            let naive_mode =
+                                mode::parse_symbolic(naively_expected_new_mode, mode, 0, is_dir)
+                                    .unwrap(); // we know that mode must be valid, so this cannot fail
+                            (m, naive_mode)
+                        })
+                    };
+
+                    match result {
+                        Ok((mode, naive_mode)) => {
+                            new_mode = mode;
+                            naively_expected_new_mode = naive_mode;
+                        }
+                        Err(f) => {
+                            return if self.quiet {
+                                Err(ExitCode::new(1))
+                            } else {
+                                Err(USimpleError::new(1, f))
+                            };
+                        }
+                    }
+                }
+                Ok((new_mode, naively_expected_new_mode))
+            }
+        }
+    }
+
+    /// Report permission changes based on verbose and changes flags
+    fn report_permission_change(&self, file_path: &Path, old_mode: u32, new_mode: u32) {
+        if self.verbose || self.changes {
+            let current_permissions = display_permissions_unix(old_mode as mode_t, false);
+            let new_permissions = display_permissions_unix(new_mode as mode_t, false);
+
+            if new_mode != old_mode {
+                println!(
+                    "mode of {} changed from {:04o} ({}) to {:04o} ({})",
+                    file_path.quote(),
+                    old_mode,
+                    current_permissions,
+                    new_mode,
+                    new_permissions
+                );
+            } else if self.verbose {
+                println!(
+                    "mode of {} retained as {:04o} ({})",
+                    file_path.quote(),
+                    old_mode,
+                    current_permissions
+                );
+            }
+        }
+    }
+
+    /// Handle symlinks during directory traversal based on traversal mode
+    #[cfg(not(target_os = "linux"))]
+    fn handle_symlink_during_traversal(
+        &self,
+        path: &Path,
+        is_command_line_arg: bool,
+    ) -> UResult<()> {
+        let should_follow_symlink = match self.traverse_symlinks {
+            TraverseSymlinks::All => true,
+            TraverseSymlinks::First => is_command_line_arg,
+            TraverseSymlinks::None => false,
+        };
+
+        if !should_follow_symlink {
+            return self.chmod_file_internal(path, false);
+        }
+
+        match fs::metadata(path) {
+            Ok(meta) if meta.is_dir() => self.walk_dir_with_context(path, false),
+            Ok(_) => {
+                // It's a file symlink, chmod it
+                self.chmod_file(path)
+            }
+            Err(_) => {
+                // Dangling symlink, chmod it without dereferencing
+                self.chmod_file_internal(path, false)
+            }
+        }
+    }
+
     fn chmod(&self, files: &[OsString]) -> UResult<()> {
         let mut r = Ok(());
 
@@ -322,6 +423,7 @@ impl Chmoder {
         r
     }
 
+    #[cfg(not(target_os = "linux"))]
     fn walk_dir_with_context(&self, file_path: &Path, is_command_line_arg: bool) -> UResult<()> {
         let mut r = self.chmod_file(file_path);
 
@@ -352,7 +454,90 @@ impl Chmoder {
         r
     }
 
-    fn handle_symlink_during_recursion(&self, path: &Path) -> UResult<()> {
+    #[cfg(target_os = "linux")]
+    fn walk_dir_with_context(&self, file_path: &Path, is_command_line_arg: bool) -> UResult<()> {
+        let mut r = self.chmod_file(file_path);
+
+        // Determine whether to traverse symlinks based on context and traversal mode
+        let should_follow_symlink = match self.traverse_symlinks {
+            TraverseSymlinks::All => true,
+            TraverseSymlinks::First => is_command_line_arg, // Only follow symlinks that are command line args
+            TraverseSymlinks::None => false,
+        };
+
+        // If the path is a directory (or we should follow symlinks), recurse into it using safe traversal
+        if (!file_path.is_symlink() || should_follow_symlink) && file_path.is_dir() {
+            match DirFd::open(file_path) {
+                Ok(dir_fd) => {
+                    r = self.safe_traverse_dir(&dir_fd, file_path).and(r);
+                }
+                Err(err) => {
+                    // Handle permission denied errors with proper file path context
+                    if err.kind() == std::io::ErrorKind::PermissionDenied {
+                        r = r.and(Err(ChmodError::PermissionDenied(
+                            file_path.to_string_lossy().to_string(),
+                        )
+                        .into()));
+                    } else {
+                        r = r.and(Err(err.into()));
+                    }
+                }
+            }
+        }
+        r
+    }
+
+    #[cfg(target_os = "linux")]
+    fn safe_traverse_dir(&self, dir_fd: &DirFd, dir_path: &Path) -> UResult<()> {
+        let mut r = Ok(());
+
+        let entries = dir_fd.read_dir()?;
+
+        // Determine if we should follow symlinks (doesn't depend on entry_name)
+        let should_follow_symlink = self.traverse_symlinks == TraverseSymlinks::All;
+
+        for entry_name in entries {
+            let entry_path = dir_path.join(&entry_name);
+
+            let dir_meta = dir_fd.metadata_at(&entry_name, should_follow_symlink);
+            let Ok(meta) = dir_meta else {
+                // Handle permission denied with proper file path context
+                let e = dir_meta.unwrap_err();
+                let error = if e.kind() == std::io::ErrorKind::PermissionDenied {
+                    ChmodError::PermissionDenied(entry_path.to_string_lossy().to_string()).into()
+                } else {
+                    e.into()
+                };
+                r = r.and(Err(error));
+                continue;
+            };
+
+            if entry_path.is_symlink() {
+                r = self
+                    .handle_symlink_during_safe_recursion(&entry_path, dir_fd, &entry_name)
+                    .and(r);
+            } else {
+                // For regular files and directories, chmod them
+                r = self
+                    .safe_chmod_file(&entry_path, dir_fd, &entry_name, meta.mode() & 0o7777)
+                    .and(r);
+
+                // Recurse into subdirectories
+                if meta.is_dir() {
+                    r = self.walk_dir_with_context(&entry_path, false).and(r);
+                }
+            }
+        }
+        r
+    }
+
+    #[cfg(target_os = "linux")]
+    fn handle_symlink_during_safe_recursion(
+        &self,
+        path: &Path,
+        dir_fd: &DirFd,
+        entry_name: &std::ffi::OsStr,
+    ) -> UResult<()> {
         // During recursion, determine behavior based on traversal mode
         match self.traverse_symlinks {
             TraverseSymlinks::All => {
@@ -360,9 +545,9 @@ impl Chmoder {
                 // Check if the symlink target is a directory, but handle dangling symlinks gracefully
                 match fs::metadata(path) {
                     Ok(meta) if meta.is_dir() => self.walk_dir_with_context(path, false),
-                    Ok(_) => {
-                        // It's a file symlink, chmod it
-                        self.chmod_file(path)
+                    Ok(meta) => {
+                        // It's a file symlink, chmod it using safe traversal
+                        self.safe_chmod_file(path, dir_fd, entry_name, meta.mode() & 0o7777)
                     }
                     Err(_) => {
                         // Dangling symlink, chmod it without dereferencing
@@ -378,12 +563,50 @@ impl Chmoder {
         }
     }
 
+    #[cfg(target_os = "linux")]
+    fn safe_chmod_file(
+        &self,
+        file_path: &Path,
+        dir_fd: &DirFd,
+        entry_name: &std::ffi::OsStr,
+        current_mode: u32,
+    ) -> UResult<()> {
+        // Calculate the new mode using the helper method
+        let (new_mode, _) = self.calculate_new_mode(current_mode, file_path.is_dir())?;
+
+        // Use safe traversal to change the mode
+        let follow_symlinks = self.dereference;
+        if let Err(_e) = dir_fd.chmod_at(entry_name, new_mode, follow_symlinks) {
+            if self.verbose {
+                println!(
+                    "failed to change mode of {} to {:o}",
+                    file_path.quote(),
+                    new_mode
+                );
+            }
+            return Err(
+                ChmodError::PermissionDenied(file_path.to_string_lossy().to_string()).into(),
+            );
+        }
+
+        // Report the change using the helper method
+        self.report_permission_change(file_path, current_mode, new_mode);
+
+        Ok(())
+    }
+
+    #[cfg(not(target_os = "linux"))]
+    fn handle_symlink_during_recursion(&self, path: &Path) -> UResult<()> {
+        // Use the common symlink handling logic
+        self.handle_symlink_during_traversal(path, false)
+    }
+
     fn chmod_file(&self, file: &Path) -> UResult<()> {
         self.chmod_file_internal(file, self.dereference)
     }
 
     fn chmod_file_internal(&self, file: &Path, dereference: bool) -> UResult<()> {
-        use uucore::{mode::get_umask, perms::get_metadata};
+        use uucore::perms::get_metadata;
 
         let metadata = get_metadata(file, dereference);
 
@@ -409,45 +632,14 @@ impl Chmoder {
             }
         };
 
-        // Determine the new permissions to apply
+        // Calculate the new mode using the helper method
+        let (new_mode, naively_expected_new_mode) =
+            self.calculate_new_mode(fperm, file.is_dir())?;
+
+        // Determine how to apply the permissions
         match self.fmode {
             Some(mode) => self.change_file(fperm, mode, file)?,
             None => {
-                let cmode_unwrapped = self.cmode.clone().unwrap();
-                let mut new_mode = fperm;
-                let mut naively_expected_new_mode = new_mode;
-                for mode in cmode_unwrapped.split(',') {
-                    let result = if mode.chars().any(|c| c.is_ascii_digit()) {
-                        mode::parse_numeric(new_mode, mode, file.is_dir()).map(|v| (v, v))
-                    } else {
-                        mode::parse_symbolic(new_mode, mode, get_umask(), file.is_dir()).map(|m| {
-                            // calculate the new mode as if umask was 0
-                            let naive_mode = mode::parse_symbolic(
-                                naively_expected_new_mode,
-                                mode,
-                                0,
-                                file.is_dir(),
-                            )
-                            .unwrap(); // we know that mode must be valid, so this cannot fail
-                            (m, naive_mode)
-                        })
-                    };
-
-                    match result {
-                        Ok((mode, naive_mode)) => {
-                            new_mode = mode;
-                            naively_expected_new_mode = naive_mode;
-                        }
-                        Err(f) => {
-                            return if self.quiet {
-                                Err(ExitCode::new(1))
-                            } else {
-                                Err(USimpleError::new(1, f))
-                            };
-                        }
-                    }
-                }
-
                 // Special handling for symlinks when not dereferencing
                 if file.is_symlink() && !dereference {
                     // TODO: On most Unix systems, symlink permissions are ignored by the kernel,
@@ -479,13 +671,8 @@ impl Chmoder {
 
     fn change_file(&self, fperm: u32, mode: u32, file: &Path) -> Result<(), i32> {
         if fperm == mode {
-            if self.verbose && !self.changes {
-                println!(
-                    "mode of {} retained as {fperm:04o} ({})",
-                    file.quote(),
-                    display_permissions_unix(fperm as mode_t, false),
-                );
-            }
+            // Use the helper method for consistent reporting
+            self.report_permission_change(file, fperm, mode);
             Ok(())
         } else if let Err(err) = fs::set_permissions(file, fs::Permissions::from_mode(mode)) {
             if !self.quiet {
@@ -501,14 +688,8 @@ impl Chmoder {
             }
             Err(1)
         } else {
-            if self.verbose || self.changes {
-                println!(
-                    "mode of {} changed from {fperm:04o} ({}) to {mode:04o} ({})",
-                    file.quote(),
-                    display_permissions_unix(fperm as mode_t, false),
-                    display_permissions_unix(mode as mode_t, false)
-                );
-            }
+            // Use the helper method for consistent reporting
+            self.report_permission_change(file, fperm, mode);
             Ok(())
         }
     }

src/uu/chown/Cargo.toml

@@ -19,7 +19,12 @@ path = "src/chown.rs"
 
 [dependencies]
 clap = { workspace = true }
-uucore = { workspace = true, features = ["entries", "fs", "perms"] }
+uucore = { workspace = true, features = [
+  "entries",
+  "fs",
+  "perms",
+  "safe-traversal",
+] }
 fluent = { workspace = true }
 
 [[bin]]

src/uucore/Cargo.toml

@@ -87,6 +87,7 @@ nix = { workspace = true, features = [
   "zerocopy",
   "signal",
   "dir",
+  "user",
 ] }
 xattr = { workspace = true, optional = true }
 

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

@@ -5,7 +5,7 @@
 
 //! Common functions to manage permissions
 
-// spell-checker:ignore (jargon) TOCTOU
+// spell-checker:ignore (jargon) TOCTOU fchownat
 
 use crate::display::Quotable;
 use crate::error::{UResult, USimpleError, strip_errno};
@@ -17,8 +17,13 @@ use clap::{Arg, ArgMatches, Command};
 use libc::{gid_t, uid_t};
 use options::traverse;
 use std::ffi::OsString;
+
+#[cfg(not(all(target_os = "linux", feature = "safe-traversal")))]
 use walkdir::WalkDir;
 
+#[cfg(all(target_os = "linux", feature = "safe-traversal"))]
+use crate::features::safe_traversal::DirFd;
+
 use std::ffi::CString;
 use std::fs::Metadata;
 use std::io::Error as IOError;
@@ -333,12 +338,151 @@ impl ChownExecutor {
         };
 
         if self.recursive {
-            ret | self.dive_into(&root)
+            #[cfg(all(target_os = "linux", feature = "safe-traversal"))]
+            {
+                ret | self.safe_dive_into(&root)
+            }
+            #[cfg(not(all(target_os = "linux", feature = "safe-traversal")))]
+            {
+                ret | self.dive_into(&root)
+            }
         } else {
             ret
         }
     }
 
+    #[cfg(all(target_os = "linux", feature = "safe-traversal"))]
+    fn safe_dive_into<P: AsRef<Path>>(&self, root: P) -> i32 {
+        let root = root.as_ref();
+
+        // Don't traverse into symlinks if configured not to
+        if self.traverse_symlinks == TraverseSymlinks::None && root.is_symlink() {
+            return 0;
+        }
+
+        // Only try to traverse if the root is actually a directory
+        // This matches WalkDir's behavior with min_depth(1) - if root is not a directory,
+        // there are no children to traverse, so we return early with success
+        if !root.is_dir() {
+            return 0;
+        }
+
+        // Open directory with safe traversal
+        let Some(dir_fd) = self.try_open_dir(root) else {
+            return 1;
+        };
+
+        let mut ret = 0;
+        self.safe_traverse_dir(&dir_fd, root, &mut ret);
+        ret
+    }
+
+    #[cfg(all(target_os = "linux", feature = "safe-traversal"))]
+    fn safe_traverse_dir(&self, dir_fd: &DirFd, dir_path: &Path, ret: &mut i32) {
+        // Read directory entries
+        let entries = match dir_fd.read_dir() {
+            Ok(entries) => entries,
+            Err(e) => {
+                *ret = 1;
+                if self.verbosity.level != VerbosityLevel::Silent {
+                    show_error!(
+                        "cannot read directory '{}': {}",
+                        dir_path.display(),
+                        strip_errno(&e)
+                    );
+                }
+                return;
+            }
+        };
+
+        for entry_name in entries {
+            let entry_path = dir_path.join(&entry_name);
+
+            // Get metadata for the entry
+            let follow = self.traverse_symlinks == TraverseSymlinks::All;
+
+            let meta = match dir_fd.metadata_at(&entry_name, follow) {
+                Ok(m) => m,
+                Err(e) => {
+                    *ret = 1;
+                    if self.verbosity.level != VerbosityLevel::Silent {
+                        show_error!(
+                            "cannot access '{}': {}",
+                            entry_path.display(),
+                            strip_errno(&e)
+                        );
+                    }
+                    continue;
+                }
+            };
+
+            if self.preserve_root
+                && is_root(&entry_path, self.traverse_symlinks == TraverseSymlinks::All)
+            {
+                *ret = 1;
+                return;
+            }
+
+            // Check if we should chown this entry
+            if self.matched(meta.uid(), meta.gid()) {
+                // Use fchownat for the actual ownership change
+                let follow_symlinks =
+                    self.dereference || self.traverse_symlinks == TraverseSymlinks::All;
+
+                // Only pass the IDs that should actually be changed
+                let chown_uid = self.dest_uid;
+                let chown_gid = self.dest_gid;
+
+                if let Err(e) = dir_fd.chown_at(&entry_name, chown_uid, chown_gid, follow_symlinks)
+                {
+                    *ret = 1;
+                    if self.verbosity.level != VerbosityLevel::Silent {
+                        let msg = format!(
+                            "changing {} of {}: {}",
+                            if self.verbosity.groups_only {
+                                "group"
+                            } else {
+                                "ownership"
+                            },
+                            entry_path.quote(),
+                            strip_errno(&e)
+                        );
+                        show_error!("{}", msg);
+                    }
+                } else {
+                    // Report the successful ownership change using the shared helper
+                    self.report_ownership_change_success(&entry_path, meta.uid(), meta.gid());
+                }
+            } else {
+                self.print_verbose_ownership_retained_as(
+                    &entry_path,
+                    meta.uid(),
+                    self.dest_gid.map(|_| meta.gid()),
+                );
+            }
+
+            // Recurse into subdirectories
+            if meta.is_dir() && (follow || !meta.file_type().is_symlink()) {
+                match dir_fd.open_subdir(&entry_name) {
+                    Ok(subdir_fd) => {
+                        self.safe_traverse_dir(&subdir_fd, &entry_path, ret);
+                    }
+                    Err(e) => {
+                        *ret = 1;
+                        if self.verbosity.level != VerbosityLevel::Silent {
+                            show_error!(
+                                "cannot access '{}': {}",
+                                entry_path.display(),
+                                strip_errno(&e)
+                            );
+                        }
+                    }
+                }
+            }
+        }
+    }
+
+    #[cfg(not(all(target_os = "linux", feature = "safe-traversal")))]
     #[allow(clippy::cognitive_complexity)]
     fn dive_into<P: AsRef<Path>>(&self, root: P) -> i32 {
         let root = root.as_ref();
@@ -473,6 +617,78 @@ impl ChownExecutor {
             }
         }
     }
+
+    /// Try to open directory with error reporting
+    #[cfg(all(target_os = "linux", feature = "safe-traversal"))]
+    fn try_open_dir(&self, path: &Path) -> Option<DirFd> {
+        DirFd::open(path)
+            .map_err(|e| {
+                if self.verbosity.level != VerbosityLevel::Silent {
+                    show_error!("cannot access '{}': {}", path.display(), strip_errno(&e));
+                }
+            })
+            .ok()
+    }
+
+    /// Report ownership change with proper verbose output
+    /// Returns 0 on success
+    #[cfg(all(target_os = "linux", feature = "safe-traversal"))]
+    fn report_ownership_change_success(
+        &self,
+        path: &Path,
+        original_uid: u32,
+        original_gid: u32,
+    ) -> i32 {
+        let dest_uid = self.dest_uid.unwrap_or(original_uid);
+        let dest_gid = self.dest_gid.unwrap_or(original_gid);
+        let changed = dest_uid != original_uid || dest_gid != original_gid;
+
+        if changed {
+            match self.verbosity.level {
+                VerbosityLevel::Changes | VerbosityLevel::Verbose => {
+                    let output = if self.verbosity.groups_only {
+                        format!(
+                            "changed group of {} from {} to {}",
+                            path.quote(),
+                            entries::gid2grp(original_gid)
+                                .unwrap_or_else(|_| original_gid.to_string()),
+                            entries::gid2grp(dest_gid).unwrap_or_else(|_| dest_gid.to_string())
+                        )
+                    } else {
+                        format!(
+                            "changed ownership of {} from {}:{} to {}:{}",
+                            path.quote(),
+                            entries::uid2usr(original_uid)
+                                .unwrap_or_else(|_| original_uid.to_string()),
+                            entries::gid2grp(original_gid)
+                                .unwrap_or_else(|_| original_gid.to_string()),
+                            entries::uid2usr(dest_uid).unwrap_or_else(|_| dest_uid.to_string()),
+                            entries::gid2grp(dest_gid).unwrap_or_else(|_| dest_gid.to_string())
+                        )
+                    };
+                    show_error!("{output}");
+                }
+                _ => (),
+            }
+        } else if self.verbosity.level == VerbosityLevel::Verbose {
+            let output = if self.verbosity.groups_only {
+                format!(
+                    "group of {} retained as {}",
+                    path.quote(),
+                    entries::gid2grp(dest_gid).unwrap_or_default()
+                )
+            } else {
+                format!(
+                    "ownership of {} retained as {}:{}",
+                    path.quote(),
+                    entries::uid2usr(dest_uid).unwrap_or_else(|_| dest_uid.to_string()),
+                    entries::gid2grp(dest_gid).unwrap_or_else(|_| dest_gid.to_string())
+                )
+            };
+            show_error!("{output}");
+        }
+        0
+    }
 }
 
 pub mod options {

src/uucore/src/lib/features/safe_traversal.rs

@@ -9,7 +9,7 @@
 // Only available on Linux
 //
 // spell-checker:ignore CLOEXEC RDONLY TOCTOU closedir dirp fdopendir fstatat openat REMOVEDIR unlinkat smallfile
-// spell-checker:ignore RAII dirfd
+// spell-checker:ignore RAII dirfd fchownat fchown FchmodatFlags fchmodat fchmod
 
 #![cfg(target_os = "linux")]
 
@@ -24,8 +24,8 @@ use std::path::Path;
 
 use nix::dir::Dir;
 use nix::fcntl::{OFlag, openat};
-use nix::sys::stat::{FileStat, Mode, fstatat};
-use nix::unistd::{UnlinkatFlags, unlinkat};
+use nix::sys::stat::{FchmodatFlags, FileStat, Mode, fchmodat, fstatat};
+use nix::unistd::{Gid, Uid, UnlinkatFlags, fchown, fchownat, unlinkat};
 
 use crate::translate;
 
@@ -209,6 +209,72 @@ impl DirFd {
         Ok(())
     }
 
+    /// Change ownership of a file relative to this directory
+    /// Use uid/gid of None to keep the current value
+    pub fn chown_at(
+        &self,
+        name: &OsStr,
+        uid: Option<u32>,
+        gid: Option<u32>,
+        follow_symlinks: bool,
+    ) -> io::Result<()> {
+        let name_cstr =
+            CString::new(name.as_bytes()).map_err(|_| SafeTraversalError::PathContainsNull)?;
+
+        let flags = if follow_symlinks {
+            nix::fcntl::AtFlags::empty()
+        } else {
+            nix::fcntl::AtFlags::AT_SYMLINK_NOFOLLOW
+        };
+
+        let uid = uid.map(Uid::from_raw);
+        let gid = gid.map(Gid::from_raw);
+
+        fchownat(&self.fd, name_cstr.as_c_str(), uid, gid, flags)
+            .map_err(|e| io::Error::from_raw_os_error(e as i32))?;
+
+        Ok(())
+    }
+
+    /// Change ownership of this directory
+    pub fn fchown(&self, uid: Option<u32>, gid: Option<u32>) -> io::Result<()> {
+        let uid = uid.map(Uid::from_raw);
+        let gid = gid.map(Gid::from_raw);
+
+        fchown(&self.fd, uid, gid).map_err(|e| io::Error::from_raw_os_error(e as i32))?;
+
+        Ok(())
+    }
+
+    /// Change mode of a file relative to this directory
+    pub fn chmod_at(&self, name: &OsStr, mode: u32, follow_symlinks: bool) -> io::Result<()> {
+        let flags = if follow_symlinks {
+            FchmodatFlags::FollowSymlink
+        } else {
+            FchmodatFlags::NoFollowSymlink
+        };
+
+        let mode = Mode::from_bits_truncate(mode);
+
+        let name_cstr =
+            CString::new(name.as_bytes()).map_err(|_| SafeTraversalError::PathContainsNull)?;
+
+        fchmodat(&self.fd, name_cstr.as_c_str(), mode, flags)
+            .map_err(|e| io::Error::from_raw_os_error(e as i32))?;
+
+        Ok(())
+    }
+
+    /// Change mode of this directory
+    pub fn fchmod(&self, mode: u32) -> io::Result<()> {
+        let mode = Mode::from_bits_truncate(mode);
+
+        nix::sys::stat::fchmod(&self.fd, mode)
+            .map_err(|e| io::Error::from_raw_os_error(e as i32))?;
+
+        Ok(())
+    }
+
     /// Create a DirFd from an existing file descriptor (takes ownership)
     pub fn from_raw_fd(fd: RawFd) -> io::Result<Self> {
         if fd < 0 {

tests/by-util/test_chgrp.rs

@@ -615,3 +615,28 @@ fn test_chgrp_non_utf8_paths() {
 
     ucmd.arg(current_gid.to_string()).arg(&filename).succeeds();
 }
+
+#[test]
+fn test_chgrp_recursive_on_file() {
+    // Test for regression where `chgrp -R` on a regular file would fail
+    // with "Not a directory" error. This should succeed since there's nothing
+    // to recurse into, similar to GNU chgrp behavior.
+    // equivalent of tests/chgrp/recurse in GNU coreutils
+    use std::os::unix::fs::MetadataExt;
+    let (at, mut ucmd) = at_and_ucmd!();
+
+    at.touch("regular_file");
+
+    let current_gid = getegid();
+
+    ucmd.arg("-R")
+        .arg(current_gid.to_string())
+        .arg("regular_file")
+        .succeeds()
+        .no_stderr();
+
+    assert_eq!(
+        at.plus("regular_file").metadata().unwrap().gid(),
+        current_gid
+    );
+}

tests/by-util/test_chmod.rs

@@ -391,6 +391,10 @@ fn test_chmod_recursive() {
     make_file(&at.plus_as_string("a/b/b"), 0o100444);
     make_file(&at.plus_as_string("a/b/c/c"), 0o100444);
     make_file(&at.plus_as_string("z/y"), 0o100444);
+    #[cfg(not(target_os = "linux"))]
+    let err_msg = "chmod: Permission denied\n";
+    #[cfg(target_os = "linux")]
+    let err_msg = "chmod: 'z': Permission denied\n";
 
     // only the permissions of folder `a` and `z` are changed
     // folder can't be read after read permission is removed
@@ -401,7 +405,7 @@ fn test_chmod_recursive() {
         .arg("z")
         .umask(0)
         .fails()
-        .stderr_is("chmod: Permission denied\n");
+        .stderr_is(err_msg);
 
     assert_eq!(at.metadata("z/y").permissions().mode(), 0o100444);
     assert_eq!(at.metadata("a/a").permissions().mode(), 0o100444);