diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index c05288e213f..c51de50f9bc 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -16,6 +16,7 @@ use uucore::fs::display_permissions_unix; use uucore::libc::mode_t; #[cfg(not(windows))] use uucore::mode; +use uucore::perms::TraverseSymlinks; use uucore::{format_usage, help_about, help_section, help_usage, show, show_error}; const ABOUT: &str = help_about!("chmod.md"); @@ -33,6 +34,11 @@ mod options { pub const RECURSIVE: &str = "recursive"; pub const MODE: &str = "MODE"; pub const FILE: &str = "FILE"; + // TODO remove duplication with perms.rs + pub mod dereference { + pub const DEREFERENCE: &str = "dereference"; + pub const NO_DEREFERENCE: &str = "no-dereference"; + } } /// Extract negative modes (starting with '-') from the rest of the arguments. @@ -99,7 +105,6 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let quiet = matches.get_flag(options::QUIET); let verbose = matches.get_flag(options::VERBOSE); let preserve_root = matches.get_flag(options::PRESERVE_ROOT); - let recursive = matches.get_flag(options::RECURSIVE); let fmode = match matches.get_one::(options::REFERENCE) { Some(fref) => match fs::metadata(fref) { Ok(meta) => Some(meta.mode() & 0o7777), @@ -138,6 +143,34 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { return Err(UUsageError::new(1, "missing operand".to_string())); } + let mut dereference = if matches.get_flag(options::dereference::DEREFERENCE) { + Some(true) // Follow symlinks + } else if matches.get_flag(options::dereference::NO_DEREFERENCE) { + Some(false) // Do not follow symlinks + } else { + None // Default behavior + }; + + let mut traverse_symlinks = if matches.get_flag("L") { + TraverseSymlinks::All + } else if matches.get_flag("H") { + TraverseSymlinks::First + } else { + TraverseSymlinks::None + }; + + let recursive = matches.get_flag(options::RECURSIVE); + if recursive { + if traverse_symlinks == TraverseSymlinks::None { + if dereference == Some(true) { + return Err(USimpleError::new(1, "-R --dereference requires -H or -L")); + } + dereference = Some(false); + } + } else { + traverse_symlinks = TraverseSymlinks::None; + } + let chmoder = Chmoder { changes, quiet, @@ -146,6 +179,8 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { recursive, fmode, cmode, + traverse_symlinks, + dereference: dereference.unwrap_or(true), }; chmoder.chmod(&files) @@ -237,6 +272,8 @@ struct Chmoder { recursive: bool, fmode: Option, cmode: Option, + traverse_symlinks: TraverseSymlinks, + dereference: bool, } impl Chmoder { @@ -248,12 +285,19 @@ impl Chmoder { 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 + continue; + } if !self.quiet { show!(USimpleError::new( 1, format!("cannot operate on dangling symlink {}", filename.quote()), )); + set_exit_code(1); } + if self.verbose { println!( "failed to change mode of {} from 0000 (---------) to 1500 (r-x-----T)", @@ -273,6 +317,11 @@ impl Chmoder { // 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 && filename == "/" { return Err(USimpleError::new( @@ -294,11 +343,23 @@ impl Chmoder { fn walk_dir(&self, file_path: &Path) -> UResult<()> { let mut r = self.chmod_file(file_path); - if !file_path.is_symlink() && file_path.is_dir() { + // Determine whether to traverse symlinks based on `self.traverse_symlinks` + let should_follow_symlink = match self.traverse_symlinks { + TraverseSymlinks::All => true, + TraverseSymlinks::First => { + file_path == file_path.canonicalize().unwrap_or(file_path.to_path_buf()) + } + TraverseSymlinks::None => false, + }; + + // If the path is a directory (or we should follow symlinks), recurse into it + if (!file_path.is_symlink() || should_follow_symlink) && file_path.is_dir() { for dir_entry in file_path.read_dir()? { let path = dir_entry?.path(); if !path.is_symlink() { r = self.walk_dir(path.as_path()); + } else if should_follow_symlink { + r = self.chmod_file(path.as_path()).and(r); } } } @@ -316,17 +377,25 @@ impl Chmoder { fn chmod_file(&self, file: &Path) -> UResult<()> { use uucore::mode::get_umask; - let fperm = match fs::metadata(file) { + // Determine metadata based on dereference flag + let metadata = if self.dereference { + file.metadata() // Follow symlinks + } else { + file.symlink_metadata() // Act on the symlink itself + }; + + let fperm = match metadata { Ok(meta) => meta.mode() & 0o7777, Err(err) => { - if file.is_symlink() { + // Handle dangling symlinks or other errors + if file.is_symlink() && !self.dereference { if self.verbose { println!( "neither symbolic link {} nor referent has been changed", file.quote() ); } - return Ok(()); + return Ok(()); // Skip dangling symlinks } 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 @@ -339,6 +408,8 @@ impl Chmoder { } } }; + + // Determine the new permissions to apply match self.fmode { Some(mode) => self.change_file(fperm, mode, file)?, None => { diff --git a/tests/by-util/test_chmod.rs b/tests/by-util/test_chmod.rs index 26ad104e9be..d9ae0dad850 100644 --- a/tests/by-util/test_chmod.rs +++ b/tests/by-util/test_chmod.rs @@ -231,20 +231,50 @@ fn test_chmod_ugoa() { run_tests(tests); } +#[test] +#[cfg(any(target_os = "linux", target_os = "macos"))] +// TODO fix android, it has 0777 +// We should for the umask on startup +fn test_chmod_umask_expected() { + let current_umask = uucore::mode::get_umask(); + assert_eq!( + current_umask, + 0o022, + "Unexpected umask value: expected 002 (octal), but got {:03o}. Please adjust the test environment.", + current_umask + ); +} + +fn get_expected_symlink_permissions() -> u32 { + #[cfg(any(target_os = "linux", target_os = "android"))] + { + 0o120_777 + } + #[cfg(not(any(target_os = "linux", target_os = "android")))] + { + 0o120_755 + } +} + #[test] fn test_chmod_error_permissions() { // check that we print an error if umask prevents us from removing a permission let (at, mut ucmd) = at_and_ucmd!(); + at.touch("file"); set_permissions(at.plus("file"), Permissions::from_mode(0o777)).unwrap(); + ucmd.args(&["-w", "file"]) + .umask(0o022) .fails() .code_is(1) - // spell-checker:disable-next-line - .stderr_is("chmod: file: new permissions are r-xrwxrwx, not r-xr-xr-x\n"); + .stderr_is( + // spell-checker:disable-next-line + "chmod: file: new permissions are r-xrwxrwx, not r-xr-xr-x\n", + ); assert_eq!( metadata(at.plus("file")).unwrap().permissions().mode(), - 0o100577 + 0o100_577 ); } @@ -645,7 +675,10 @@ fn test_chmod_file_symlink_after_non_existing_file() { .stderr_contains(expected_stderr); assert_eq!( at.metadata(test_existing_symlink).permissions().mode(), - 0o100_764 + 0o100_764, + "Expected mode: {:o}, but got: {:o}", + 0o100_764, + at.metadata(test_existing_symlink).permissions().mode() ); } @@ -749,3 +782,252 @@ fn test_gnu_special_options() { scene.ucmd().arg("--").arg("--").arg("file").succeeds(); scene.ucmd().arg("--").arg("--").fails(); } + +#[test] +fn test_chmod_dereference_symlink() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let target = "file"; + let symlink = "symlink"; + + at.touch(target); + set_permissions(at.plus(target), Permissions::from_mode(0o664)).unwrap(); + at.symlink_file(target, symlink); + + // Use --dereference: should modify the target file's permissions + scene + .ucmd() + .arg("--dereference") + .arg("u+x") + .arg(symlink) + .succeeds() + .no_stderr(); + assert_eq!(at.metadata(target).permissions().mode(), 0o100_764); + assert_eq!( + at.symlink_metadata(symlink).permissions().mode(), + get_expected_symlink_permissions() + ); +} + +#[test] +fn test_chmod_no_dereference_symlink() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let target = "file"; + let symlink = "symlink"; + + at.touch(target); + set_permissions(at.plus(target), Permissions::from_mode(0o664)).unwrap(); + at.symlink_file(target, symlink); + + // Use --no-dereference: should modify the symlink itself + scene + .ucmd() + .arg("--no-dereference") + .arg("u+x") + .arg(symlink) + .succeeds() + .no_stderr(); + assert_eq!(at.metadata(target).permissions().mode(), 0o100_664); + assert_eq!( + at.symlink_metadata(symlink).permissions().mode(), + get_expected_symlink_permissions() + ); +} + +#[test] +fn test_chmod_traverse_symlink_h() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let target = "file"; + let symlink = "symlink"; + let directory = "dir"; + + at.mkdir(directory); + at.touch(target); + at.symlink_file(target, &format!("{directory}/{symlink}")); + + set_permissions(at.plus(target), Permissions::from_mode(0o664)).unwrap(); + + // Use -H: should only follow symlinks specified as arguments + scene + .ucmd() + .arg("-R") + .arg("-H") + .arg("u+x") + .arg(directory) + .succeeds() + .no_stderr(); + + let target_permissions = at.metadata(target).permissions().mode(); + assert_eq!( + target_permissions, 0o100_664, + "Expected target permissions to be 0o100_664, but got {:o}", + target_permissions + ); + + let symlink_path = format!("{directory}/{symlink}"); + let symlink_permissions = at.symlink_metadata(&symlink_path).permissions().mode(); + assert_eq!( + symlink_permissions, + get_expected_symlink_permissions(), + "Expected symlink permissions to be 0o120_777, but got {:o} for symlink at {}", + symlink_permissions, + symlink_path + ); +} + +#[test] +fn test_chmod_traverse_symlink_l() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let target = "file"; + let symlink = "symlink"; + let directory = "dir"; + + at.mkdir(directory); + at.touch(target); + at.symlink_file(target, &format!("{directory}/{symlink}")); + + set_permissions(at.plus(target), Permissions::from_mode(0o664)).unwrap(); + + // Use -L: should follow all symlinks + scene + .ucmd() + .arg("-R") + .arg("-L") + .arg("u+x") + .umask(0o022) + .arg(directory) + .succeeds() + .no_stderr(); + + let target_permissions = at.metadata(target).permissions().mode(); + assert_eq!( + target_permissions, 0o100_764, + "Expected target permissions to be 0o100_764, but got {:o}", + target_permissions + ); + + let symlink_path = format!("{directory}/{symlink}"); + let symlink_permissions = at.symlink_metadata(&symlink_path).permissions().mode(); + assert_eq!( + symlink_permissions, + get_expected_symlink_permissions(), + "Expected symlink permissions to be 0o120_777, but got {:o} for symlink at {}", + symlink_permissions, + symlink_path + ); +} + +#[test] +fn test_chmod_traverse_symlink_p() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let target = "file"; + let symlink = "symlink"; + let directory = "dir"; + + at.mkdir(directory); + at.touch(target); + at.symlink_file(target, &format!("{directory}/{symlink}")); + + set_permissions(at.plus(target), Permissions::from_mode(0o664)).unwrap(); + + // Use -P: should not follow any symlinks + scene + .ucmd() + .arg("-R") + .arg("-P") + .arg("u+x") + .umask(0o022) + .arg(directory) + .succeeds() + .no_stderr(); + assert_eq!(at.metadata(target).permissions().mode(), 0o100_664); + assert_eq!( + at.symlink_metadata(&format!("{directory}/{symlink}")) + .permissions() + .mode(), + get_expected_symlink_permissions() + ); +} + +#[test] +fn test_chmod_symlink_to_dangling_target_dereference() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let dangling_target = "nonexistent_file"; + let symlink = "symlink"; + + at.symlink_file(dangling_target, symlink); + + // Use --dereference: should fail due to dangling symlink + scene + .ucmd() + .arg("--dereference") + .arg("u+x") + .arg(symlink) + .fails() + .stderr_contains(format!("cannot operate on dangling symlink '{}'", symlink)); +} + +#[test] +fn test_chmod_symlink_target_no_dereference() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let file = "a"; + let symlink = "symlink"; + at.touch(file); + at.symlink_file(file, symlink); + set_permissions(at.plus(file), Permissions::from_mode(0o644)).unwrap(); + + scene + .ucmd() + .arg("--no-dereference") + .arg("755") + .arg(symlink) + .succeeds() + .no_stderr(); + assert_eq!( + at.symlink_metadata(file).permissions().mode(), + 0o100_644, + "Expected symlink permissions: {:o}, but got: {:o}", + 0o100_644, + at.symlink_metadata(file).permissions().mode() + ); +} + +#[test] +fn test_chmod_symlink_to_dangling_recursive() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let dangling_target = "nonexistent_file"; + let symlink = "symlink"; + + at.symlink_file(dangling_target, symlink); + + // Use --no-dereference: should succeed and modify the symlink itself + scene + .ucmd() + .arg("755") + .arg("-R") + .arg(symlink) + .fails() + .stderr_is("chmod: cannot operate on dangling symlink 'symlink'\n"); + assert_eq!( + at.symlink_metadata(symlink).permissions().mode(), + get_expected_symlink_permissions(), + "Expected symlink permissions: {:o}, but got: {:o}", + get_expected_symlink_permissions(), + at.symlink_metadata(symlink).permissions().mode() + ); +}