From b8951189d0e56b6b59a1519f4d0c67ae159db228 Mon Sep 17 00:00:00 2001 From: Mahdi Ali-Raihan Date: Tue, 23 Dec 2025 17:51:51 -0500 Subject: [PATCH 1/3] chmod: output permission denied error messages for inaccesible directories/files as regular user. Fixes #9789. --- src/uu/chmod/locales/en-US.ftl | 2 +- src/uu/chmod/locales/fr-FR.ftl | 2 +- src/uu/chmod/src/chmod.rs | 85 ++++++++++++++++++++-------------- tests/by-util/test_chmod.rs | 24 +++++++++- 4 files changed, 73 insertions(+), 40 deletions(-) diff --git a/src/uu/chmod/locales/en-US.ftl b/src/uu/chmod/locales/en-US.ftl index 52447f26399..12df1e2b7a5 100644 --- a/src/uu/chmod/locales/en-US.ftl +++ b/src/uu/chmod/locales/en-US.ftl @@ -9,7 +9,7 @@ chmod-error-dangling-symlink = cannot operate on dangling symlink {$file} chmod-error-no-such-file = cannot access {$file}: No such file or directory chmod-error-preserve-root = it is dangerous to operate recursively on {$file} chmod: use --no-preserve-root to override this failsafe -chmod-error-permission-denied = {$file}: Permission denied +chmod-error-permission-denied = cannot access {$file}: Permission denied chmod-error-new-permissions = {$file}: new permissions are {$actual}, not {$expected} chmod-error-missing-operand = missing operand diff --git a/src/uu/chmod/locales/fr-FR.ftl b/src/uu/chmod/locales/fr-FR.ftl index 97a3b673294..f4e21b1b725 100644 --- a/src/uu/chmod/locales/fr-FR.ftl +++ b/src/uu/chmod/locales/fr-FR.ftl @@ -21,7 +21,7 @@ chmod-error-dangling-symlink = impossible d'opérer sur le lien symbolique pendo chmod-error-no-such-file = impossible d'accéder à {$file} : Aucun fichier ou répertoire de ce type chmod-error-preserve-root = il est dangereux d'opérer récursivement sur {$file} chmod: utiliser --no-preserve-root pour outrepasser cette protection -chmod-error-permission-denied = {$file} : Permission refusée +chmod-error-permission-denied = impossible d'accéder à {$file} : Permission refusée chmod-error-new-permissions = {$file} : les nouvelles permissions sont {$actual}, pas {$expected} chmod-error-missing-operand = opérande manquant diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index 15b608af6b2..ed89ea80fe6 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; +use std::{fs, io}; use thiserror::Error; use uucore::display::Quotable; use uucore::error::{ExitCode, UError, UResult, USimpleError, UUsageError, set_exit_code}; @@ -372,52 +372,65 @@ impl Chmoder { for filename in files { let file = Path::new(filename); - if !file.exists() { - if file.is_symlink() { - if !self.dereference && !self.recursive { + + match file.try_exists() { + Ok(_) => { + if !self.dereference && file.is_symlink() { // 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 { + // 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( + show!(ChmodError::PermissionDenied( filename.to_string_lossy().to_string() )); - set_exit_code(1); } + } + Err(_) => { + 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.verbose { - println!( - "{}", - translate!("chmod-verbose-failed-dangling", "file" => filename.to_string_lossy().quote()) - ); + 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() + )); } - } 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); } - // 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("/".to_string()).into()); - } - if self.recursive { - r = self.walk_dir_with_context(file, true); - } else { - r = self.chmod_file(file).and(r); } } r diff --git a/tests/by-util/test_chmod.rs b/tests/by-util/test_chmod.rs index e4d4b028474..2d9fe45ff43 100644 --- a/tests/by-util/test_chmod.rs +++ b/tests/by-util/test_chmod.rs @@ -372,7 +372,7 @@ fn test_permission_denied() { .arg("o=r") .arg("d") .fails() - .stderr_is("chmod: 'd/no-x/y': Permission denied\n"); + .stderr_is("chmod: cannot access 'd/no-x/y': Permission denied\n"); } #[test] @@ -395,7 +395,7 @@ fn test_chmod_recursive() { #[cfg(not(target_os = "linux"))] let err_msg = "chmod: Permission denied\n"; #[cfg(target_os = "linux")] - let err_msg = "chmod: 'z': Permission denied\n"; + let err_msg = "chmod: cannot access 'z': Permission denied\n"; // only the permissions of folder `a` and `z` are changed // folder can't be read after read permission is removed @@ -1357,3 +1357,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"); +} From 9fe4f6aa1b4f9aa1bc1cef11297fb2b6f60a684e Mon Sep 17 00:00:00 2001 From: Mahdi Ali-Raihan Date: Tue, 23 Dec 2025 18:10:58 -0500 Subject: [PATCH 2/3] fix a few already made failing test cases --- src/uu/chmod/src/chmod.rs | 73 ++++++++++++++++++++++----------------- 1 file changed, 41 insertions(+), 32 deletions(-) diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index ed89ea80fe6..94f607a316f 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -374,8 +374,40 @@ impl Chmoder { let file = Path::new(filename); match file.try_exists() { - Ok(_) => { - if !self.dereference && file.is_symlink() { + 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); + } 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 @@ -397,38 +429,15 @@ impl Chmoder { filename.to_string_lossy().to_string() )); } + set_exit_code(1); } + // error must be no such file Err(_) => { - 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. + // if !self.quiet { + // show!(ChmodError::NoSuchFile( + // filename.to_string_lossy().to_string() + // )); + // } set_exit_code(1); } } From e77c34354f948d1b63e8bb0fdfe84c9a5f945d8b Mon Sep 17 00:00:00 2001 From: Mahdi Ali-Raihan Date: Tue, 23 Dec 2025 18:23:23 -0500 Subject: [PATCH 3/3] fixed failing test errors on linux; may need to check for errors on windows --- src/uu/chmod/src/chmod.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index 94f607a316f..165115098cb 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -407,6 +407,7 @@ impl Chmoder { // 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 @@ -433,11 +434,11 @@ impl Chmoder { } // error must be no such file Err(_) => { - // if !self.quiet { - // show!(ChmodError::NoSuchFile( - // filename.to_string_lossy().to_string() - // )); - // } + if !self.quiet { + show!(ChmodError::NoSuchFile( + filename.to_string_lossy().to_string() + )); + } set_exit_code(1); } }