From 859202cb3820ead96f0523d6ab06ee0ec1ccd9df Mon Sep 17 00:00:00 2001 From: Mahdi Ali-Raihan Date: Thu, 1 Jan 2026 09:02:53 -0500 Subject: [PATCH 1/6] git rebase with #9793 --- src/uu/chmod/src/chmod.rs | 141 +++++++++++++++++++++++------------- tests/by-util/test_chmod.rs | 20 +++++ 2 files changed, 109 insertions(+), 52 deletions(-) diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index b7e0f3fd965..44afc0c9687 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -7,9 +7,9 @@ use clap::{Arg, ArgAction, Command}; use std::ffi::OsString; -use std::fs; use std::os::unix::fs::{MetadataExt, PermissionsExt}; -use std::path::{Path, PathBuf}; +use std::path::Path; +use std::{fs, io}; use thiserror::Error; use uucore::display::Quotable; use uucore::error::{ExitCode, UError, UResult, USimpleError, UUsageError, set_exit_code}; @@ -27,17 +27,17 @@ use uucore::translate; #[derive(Debug, Error)] enum ChmodError { #[error("{}", translate!("chmod-error-cannot-stat", "file" => _0.quote()))] - CannotStat(PathBuf), + CannotStat(String), #[error("{}", translate!("chmod-error-dangling-symlink", "file" => _0.quote()))] - DanglingSymlink(PathBuf), + DanglingSymlink(String), #[error("{}", translate!("chmod-error-no-such-file", "file" => _0.quote()))] - NoSuchFile(PathBuf), + NoSuchFile(String), #[error("{}", translate!("chmod-error-preserve-root", "file" => _0.quote()))] - PreserveRoot(PathBuf), + PreserveRoot(String), #[error("{}", translate!("chmod-error-permission-denied", "file" => _0.quote()))] - PermissionDenied(PathBuf), - #[error("{}", translate!("chmod-error-new-permissions", "file" => _0.maybe_quote(), "actual" => _1.clone(), "expected" => _2.clone()))] - NewPermissions(PathBuf, String, String), + PermissionDenied(String), + #[error("{}", translate!("chmod-error-new-permissions", "file" => _0.clone(), "actual" => _1.clone(), "expected" => _2.clone()))] + NewPermissions(String, String, String), } impl UError for ChmodError {} @@ -123,7 +123,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { Some(fref) => match fs::metadata(fref) { Ok(meta) => Some(meta.mode() & 0o7777), Err(_) => { - return Err(ChmodError::CannotStat(fref.into()).into()); + return Err(ChmodError::CannotStat(fref.to_string_lossy().to_string()).into()); } }, None => None, @@ -372,48 +372,75 @@ impl Chmoder { for filename in files { let file = Path::new(filename); - if !file.exists() { - if file.is_symlink() { - if !self.dereference && !self.recursive { - // The file is a symlink and we should not follow it - // Don't try to change the mode of the symlink itself + + match file.try_exists() { + Ok(exists) => { + if !(exists) { + if file.is_symlink() { + if !self.dereference && !self.recursive { + // The file is a symlink and we should not follow it + // Don't try to change the mode of the symlink itself + continue; + } + if self.recursive && self.traverse_symlinks == TraverseSymlinks::None { + continue; + } + + if !self.quiet { + show!(ChmodError::DanglingSymlink( + filename.to_string_lossy().to_string() + )); + set_exit_code(1); + } + + if self.verbose { + println!( + "{}", + translate!("chmod-verbose-failed-dangling", "file" => filename.to_string_lossy().quote()) + ); + } + } else if !self.quiet { + show!(ChmodError::NoSuchFile( + filename.to_string_lossy().to_string() + )); + } + // GNU exits with exit code 1 even if -q or --quiet are passed + // So we set the exit code, because it hasn't been set yet if `self.quiet` is true. + set_exit_code(1); continue; - } - if self.recursive && self.traverse_symlinks == TraverseSymlinks::None { + } else if !self.dereference && file.is_symlink() { + // The file is a symlink and we should not follow it + // chmod 755 --no-dereference a/link + // should not change the permissions in this case continue; } + if self.recursive && self.preserve_root && file == Path::new("/") { + return Err(ChmodError::PreserveRoot("/".to_string()).into()); + } + if self.recursive { + r = self.walk_dir_with_context(file, true); + } else { + r = self.chmod_file(file).and(r); + } + } + Err(e) if e.kind() == io::ErrorKind::PermissionDenied => { if !self.quiet { - show!(ChmodError::DanglingSymlink(filename.into())); - set_exit_code(1); + show!(ChmodError::PermissionDenied( + filename.to_string_lossy().to_string() + )); } - - if self.verbose { - println!( - "{}", - translate!("chmod-verbose-failed-dangling", "file" => filename.quote()) - ); + set_exit_code(1); + } + // error must be no such file + Err(_) => { + if !self.quiet { + show!(ChmodError::NoSuchFile( + filename.to_string_lossy().to_string() + )); } - } else if !self.quiet { - show!(ChmodError::NoSuchFile(filename.into())); + set_exit_code(1); } - // GNU exits with exit code 1 even if -q or --quiet are passed - // So we set the exit code, because it hasn't been set yet if `self.quiet` is true. - set_exit_code(1); - continue; - } else if !self.dereference && file.is_symlink() { - // The file is a symlink and we should not follow it - // chmod 755 --no-dereference a/link - // should not change the permissions in this case - continue; - } - if self.recursive && self.preserve_root && file == Path::new("/") { - return Err(ChmodError::PreserveRoot("/".into()).into()); - } - if self.recursive { - r = self.walk_dir_with_context(file, true).and(r); - } else { - r = self.chmod_file(file).and(r); } } r @@ -470,7 +497,10 @@ impl Chmoder { 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.into()).into())); + r = r.and(Err(ChmodError::PermissionDenied( + file_path.to_string_lossy().to_string(), + ) + .into())); } else { r = r.and(Err(err.into())); } @@ -497,7 +527,7 @@ impl Chmoder { // 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).into() + ChmodError::PermissionDenied(entry_path.to_string_lossy().to_string()).into() } else { e.into() }; @@ -523,7 +553,10 @@ impl Chmoder { } Err(err) => { let error = if err.kind() == std::io::ErrorKind::PermissionDenied { - ChmodError::PermissionDenied(entry_path).into() + ChmodError::PermissionDenied( + entry_path.to_string_lossy().to_string(), + ) + .into() } else { err.into() }; @@ -589,7 +622,9 @@ impl Chmoder { new_mode ); } - return Err(ChmodError::PermissionDenied(file_path.into()).into()); + return Err( + ChmodError::PermissionDenied(file_path.to_string_lossy().to_string()).into(), + ); } // Report the change using the helper method @@ -626,9 +661,11 @@ impl Chmoder { } Ok(()) // Skip dangling symlinks } else if err.kind() == std::io::ErrorKind::PermissionDenied { - Err(ChmodError::PermissionDenied(file.into()).into()) + // These two filenames would normally be conditionally + // quoted, but GNU's tests expect them to always be quoted + Err(ChmodError::PermissionDenied(file.to_string_lossy().to_string()).into()) } else { - Err(ChmodError::CannotStat(file.into()).into()) + Err(ChmodError::CannotStat(file.to_string_lossy().to_string()).into()) }; } }; @@ -658,7 +695,7 @@ impl Chmoder { // if a permission would have been removed if umask was 0, but it wasn't because umask was not 0, print an error and fail if (new_mode & !naively_expected_new_mode) != 0 { return Err(ChmodError::NewPermissions( - file.into(), + file.to_string_lossy().to_string(), display_permissions_unix(new_mode as mode_t, false), display_permissions_unix(naively_expected_new_mode as mode_t, false), ) @@ -725,4 +762,4 @@ mod tests { assert_eq!(c, None); assert_eq!(a, ["--", "-r", "file"]); } -} +} \ No newline at end of file diff --git a/tests/by-util/test_chmod.rs b/tests/by-util/test_chmod.rs index 5e340732832..e7a30f8bfad 100644 --- a/tests/by-util/test_chmod.rs +++ b/tests/by-util/test_chmod.rs @@ -1389,3 +1389,23 @@ fn test_chmod_colored_output() { .stderr_contains("\x1b[31merreur\x1b[0m") // Red "erreur" in French .stderr_contains("\x1b[33m--invalid-option\x1b[0m"); // Yellow invalid option } + +#[test] +fn test_chmod_locked_dir_permission_denied() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let locked_dir = "locked"; + let file = "file"; + + at.mkdir(locked_dir); + at.touch(format!("{locked_dir}/{file}")); + at.set_mode(locked_dir, 0o000); + + scene + .ucmd() + .arg("000") + .arg(format!("{locked_dir}/{file}")) + .fails() + .stderr_contains("chmod: cannot access 'locked/file': Permission denied"); +} \ No newline at end of file From be0f522c8d79e8e798aa518355c23937136404b3 Mon Sep 17 00:00:00 2001 From: Mahdi Ali-Raihan Date: Tue, 30 Dec 2025 00:43:24 -0500 Subject: [PATCH 2/6] cargo fmted --- src/uu/chmod/src/chmod.rs | 2 +- tests/by-util/test_chmod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index 44afc0c9687..165115098cb 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -762,4 +762,4 @@ mod tests { assert_eq!(c, None); assert_eq!(a, ["--", "-r", "file"]); } -} \ No newline at end of file +} diff --git a/tests/by-util/test_chmod.rs b/tests/by-util/test_chmod.rs index e7a30f8bfad..578ae639e9d 100644 --- a/tests/by-util/test_chmod.rs +++ b/tests/by-util/test_chmod.rs @@ -1408,4 +1408,4 @@ fn test_chmod_locked_dir_permission_denied() { .arg(format!("{locked_dir}/{file}")) .fails() .stderr_contains("chmod: cannot access 'locked/file': Permission denied"); -} \ No newline at end of file +} From 7d1150c280e0aabde1601aa4849fdd6ea1eb2cbb Mon Sep 17 00:00:00 2001 From: Mahdi Ali-Raihan Date: Tue, 30 Dec 2025 00:53:29 -0500 Subject: [PATCH 3/6] fix: bring back String -> PathBuf types in ChmodError and use .into() --- src/uu/chmod/src/chmod.rs | 42 +++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index 165115098cb..8ca789e3a19 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -8,7 +8,7 @@ use clap::{Arg, ArgAction, Command}; use std::ffi::OsString; use std::os::unix::fs::{MetadataExt, PermissionsExt}; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::{fs, io}; use thiserror::Error; use uucore::display::Quotable; @@ -27,17 +27,17 @@ use uucore::translate; #[derive(Debug, Error)] enum ChmodError { #[error("{}", translate!("chmod-error-cannot-stat", "file" => _0.quote()))] - CannotStat(String), + CannotStat(PathBuf), #[error("{}", translate!("chmod-error-dangling-symlink", "file" => _0.quote()))] - DanglingSymlink(String), + DanglingSymlink(PathBuf), #[error("{}", translate!("chmod-error-no-such-file", "file" => _0.quote()))] - NoSuchFile(String), + NoSuchFile(PathBuf), #[error("{}", translate!("chmod-error-preserve-root", "file" => _0.quote()))] - PreserveRoot(String), + PreserveRoot(PathBuf), #[error("{}", translate!("chmod-error-permission-denied", "file" => _0.quote()))] - PermissionDenied(String), - #[error("{}", translate!("chmod-error-new-permissions", "file" => _0.clone(), "actual" => _1.clone(), "expected" => _2.clone()))] - NewPermissions(String, String, String), + PermissionDenied(PathBuf), + #[error("{}", translate!("chmod-error-new-permissions", "file" => _0.maybe_quote(), "actual" => _1.clone(), "expected" => _2.clone()))] + NewPermissions(PathBuf, String, String), } impl UError for ChmodError {} @@ -123,7 +123,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { Some(fref) => match fs::metadata(fref) { Ok(meta) => Some(meta.mode() & 0o7777), Err(_) => { - return Err(ChmodError::CannotStat(fref.to_string_lossy().to_string()).into()); + return Err(ChmodError::CannotStat(fref.into()).into()); } }, None => None, @@ -388,7 +388,7 @@ impl Chmoder { if !self.quiet { show!(ChmodError::DanglingSymlink( - filename.to_string_lossy().to_string() + filename.into() )); set_exit_code(1); } @@ -401,7 +401,7 @@ impl Chmoder { } } else if !self.quiet { show!(ChmodError::NoSuchFile( - filename.to_string_lossy().to_string() + filename.into() )); } // GNU exits with exit code 1 even if -q or --quiet are passed @@ -416,7 +416,7 @@ impl Chmoder { } if self.recursive && self.preserve_root && file == Path::new("/") { - return Err(ChmodError::PreserveRoot("/".to_string()).into()); + return Err(ChmodError::PreserveRoot("/".into()).into()); } if self.recursive { r = self.walk_dir_with_context(file, true); @@ -427,7 +427,7 @@ impl Chmoder { Err(e) if e.kind() == io::ErrorKind::PermissionDenied => { if !self.quiet { show!(ChmodError::PermissionDenied( - filename.to_string_lossy().to_string() + filename.into() )); } set_exit_code(1); @@ -436,7 +436,7 @@ impl Chmoder { Err(_) => { if !self.quiet { show!(ChmodError::NoSuchFile( - filename.to_string_lossy().to_string() + filename.into() )); } set_exit_code(1); @@ -498,7 +498,7 @@ impl Chmoder { // 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(), + file_path.into(), ) .into())); } else { @@ -527,7 +527,7 @@ impl Chmoder { // 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() + ChmodError::PermissionDenied(entry_path.into()).into() } else { e.into() }; @@ -554,7 +554,7 @@ impl Chmoder { Err(err) => { let error = if err.kind() == std::io::ErrorKind::PermissionDenied { ChmodError::PermissionDenied( - entry_path.to_string_lossy().to_string(), + entry_path.into(), ) .into() } else { @@ -623,7 +623,7 @@ impl Chmoder { ); } return Err( - ChmodError::PermissionDenied(file_path.to_string_lossy().to_string()).into(), + ChmodError::PermissionDenied(file_path.into()).into(), ); } @@ -663,9 +663,9 @@ impl Chmoder { } else if err.kind() == std::io::ErrorKind::PermissionDenied { // These two filenames would normally be conditionally // quoted, but GNU's tests expect them to always be quoted - Err(ChmodError::PermissionDenied(file.to_string_lossy().to_string()).into()) + Err(ChmodError::PermissionDenied(file.into()).into()) } else { - Err(ChmodError::CannotStat(file.to_string_lossy().to_string()).into()) + Err(ChmodError::CannotStat(file.into()).into()) }; } }; @@ -695,7 +695,7 @@ impl Chmoder { // if a permission would have been removed if umask was 0, but it wasn't because umask was not 0, print an error and fail if (new_mode & !naively_expected_new_mode) != 0 { return Err(ChmodError::NewPermissions( - file.to_string_lossy().to_string(), + file.into(), display_permissions_unix(new_mode as mode_t, false), display_permissions_unix(naively_expected_new_mode as mode_t, false), ) From a3441cf7a3e9bb048011b40ac1bc0f55bd1d893d Mon Sep 17 00:00:00 2001 From: Mahdi Ali-Raihan Date: Tue, 30 Dec 2025 00:54:33 -0500 Subject: [PATCH 4/6] cargo clippy --- src/uu/chmod/src/chmod.rs | 30 +++++++----------------------- 1 file changed, 7 insertions(+), 23 deletions(-) diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index 8ca789e3a19..884fcec1bf1 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -387,9 +387,7 @@ impl Chmoder { } if !self.quiet { - show!(ChmodError::DanglingSymlink( - filename.into() - )); + show!(ChmodError::DanglingSymlink(filename.into())); set_exit_code(1); } @@ -400,9 +398,7 @@ impl Chmoder { ); } } else if !self.quiet { - show!(ChmodError::NoSuchFile( - filename.into() - )); + show!(ChmodError::NoSuchFile(filename.into())); } // GNU exits with exit code 1 even if -q or --quiet are passed // So we set the exit code, because it hasn't been set yet if `self.quiet` is true. @@ -426,18 +422,14 @@ impl Chmoder { } Err(e) if e.kind() == io::ErrorKind::PermissionDenied => { if !self.quiet { - show!(ChmodError::PermissionDenied( - filename.into() - )); + show!(ChmodError::PermissionDenied(filename.into())); } set_exit_code(1); } // error must be no such file Err(_) => { if !self.quiet { - show!(ChmodError::NoSuchFile( - filename.into() - )); + show!(ChmodError::NoSuchFile(filename.into())); } set_exit_code(1); } @@ -497,10 +489,7 @@ impl Chmoder { 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.into(), - ) - .into())); + r = r.and(Err(ChmodError::PermissionDenied(file_path.into()).into())); } else { r = r.and(Err(err.into())); } @@ -553,10 +542,7 @@ impl Chmoder { } Err(err) => { let error = if err.kind() == std::io::ErrorKind::PermissionDenied { - ChmodError::PermissionDenied( - entry_path.into(), - ) - .into() + ChmodError::PermissionDenied(entry_path.into()).into() } else { err.into() }; @@ -622,9 +608,7 @@ impl Chmoder { new_mode ); } - return Err( - ChmodError::PermissionDenied(file_path.into()).into(), - ); + return Err(ChmodError::PermissionDenied(file_path.into()).into()); } // Report the change using the helper method From 8cfc79d07690444760ab610731a321e6dc19c65c Mon Sep 17 00:00:00 2001 From: Mahdi Ali-Raihan Date: Tue, 30 Dec 2025 00:54:58 -0500 Subject: [PATCH 5/6] cargo clippy --- src/uu/chmod/src/chmod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index 884fcec1bf1..d3084ec19a5 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -516,7 +516,7 @@ impl Chmoder { // 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.into()).into() + ChmodError::PermissionDenied(entry_path).into() } else { e.into() }; @@ -542,7 +542,7 @@ impl Chmoder { } Err(err) => { let error = if err.kind() == std::io::ErrorKind::PermissionDenied { - ChmodError::PermissionDenied(entry_path.into()).into() + ChmodError::PermissionDenied(entry_path).into() } else { err.into() }; From 07746a416ec1c236c91ed8a0f4a06ee3a50e9176 Mon Sep 17 00:00:00 2001 From: Mahdi Ali-Raihan Date: Thu, 1 Jan 2026 09:03:33 -0500 Subject: [PATCH 6/6] apply changes made by #9793 manually --- src/uu/chmod/src/chmod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index d3084ec19a5..fbbd2105713 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -415,7 +415,7 @@ impl Chmoder { return Err(ChmodError::PreserveRoot("/".into()).into()); } if self.recursive { - r = self.walk_dir_with_context(file, true); + r = self.walk_dir_with_context(file, true).and(r); } else { r = self.chmod_file(file).and(r); }