mirrors/coreutils
1
0
Fork 0
mirror of https://github.com/uutils/coreutils.git synced 2025-12-23 08:47:37 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

chmod: fix recursive symlink handling for -H/-L/-P flags (Closes: #8422)

Browse source
This commit is contained in:
Sylvestre Ledru 2025-08-07 23:09:45 +02:00
parent f7ad1eed25
commit 21c8e42c8f
2 changed files with 228 additions and 22 deletions
Download patch file Download diff file Expand all files Collapse all files

78
src/uu/chmod/src/chmod.rs
View file

@ -311,7 +311,7 @@ impl Chmoder {
return Err(ChmodError::PreserveRoot(filename.to_string()).into());
}
if self.recursive {
r = self.walk_dir(file);
r = self.walk_dir_with_context(file, true);
} else {
r = self.chmod_file(file).and(r);
}
@ -319,31 +319,62 @@ impl Chmoder {
r
}
fn walk_dir(&self, file_path: &Path) -> UResult<()> {
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 `self.traverse_symlinks`
// Determine whether to traverse symlinks based on context and traversal mode
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::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
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);
let path = match dir_entry {
Ok(entry) => entry.path(),
Err(err) => {
r = r.and(Err(err.into()));
continue;
}
};
if path.is_symlink() {
r = self.handle_symlink_during_recursion(&path).and(r);
} else {
r = self.walk_dir_with_context(path.as_path(), false).and(r);
}
}
}
r
}
fn handle_symlink_during_recursion(&self, path: &Path) -> UResult<()> {
// During recursion, determine behavior based on traversal mode
match self.traverse_symlinks {
TraverseSymlinks::All => {
// Follow all symlinks during recursion
// 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)
}
Err(_) => {
// Dangling symlink, chmod it without dereferencing
self.chmod_file_internal(path, false)
}
}
}
TraverseSymlinks::First | TraverseSymlinks::None => {
// Don't follow symlinks encountered during recursion
// For these symlinks, don't dereference them even if dereference is normally true
self.chmod_file_internal(path, false)
}
}
}
#[cfg(windows)]
fn chmod_file(&self, file: &Path) -> UResult<()> {
// chmod is useless on Windows
@ -351,17 +382,23 @@ impl Chmoder {
// instead it just sets the readonly attribute on the file
Ok(())
}
#[cfg(unix)]
fn chmod_file(&self, file: &Path) -> UResult<()> {
self.chmod_file_internal(file, self.dereference)
}
#[cfg(unix)]
fn chmod_file_internal(&self, file: &Path, dereference: bool) -> UResult<()> {
use uucore::{mode::get_umask, perms::get_metadata};
let metadata = get_metadata(file, self.dereference);
let metadata = get_metadata(file, dereference);
let fperm = match metadata {
Ok(meta) => meta.mode() & 0o7777,
Err(err) => {
// Handle dangling symlinks or other errors
return if file.is_symlink() && !self.dereference {
return if file.is_symlink() && !dereference {
if self.verbose {
println!(
"neither symbolic link {} nor referent has been changed",
@ -418,7 +455,20 @@ impl Chmoder {
}
}
self.change_file(fperm, new_mode, file)?;
// Special handling for symlinks when not dereferencing
if file.is_symlink() && !dereference {
// TODO: On most Unix systems, symlink permissions are ignored by the kernel,
// so changing them has no effect. We skip this operation for compatibility.
// Note that "chmod without dereferencing" effectively does nothing on symlinks.
if self.verbose {
println!(
"neither symbolic link {} nor referent has been changed",
file.quote()
);
}
} else {
self.change_file(fperm, new_mode, file)?;
}
// 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(

172
tests/by-util/test_chmod.rs
View file

@ -944,10 +944,8 @@ fn test_chmod_dangling_symlink_recursive_combos() {
at.symlink_file(dangling_target, symlink);
let mut ucmd = scene.ucmd();
for f in &flags {
ucmd.arg(f);
}
ucmd.arg("u+x")
ucmd.args(&flags)
.arg("u+x")
.umask(0o022)
.arg(symlink)
.fails()
@ -1002,10 +1000,8 @@ fn test_chmod_traverse_symlink_combo() {
set_permissions(at.plus(target), Permissions::from_mode(0o664)).unwrap();
let mut ucmd = scene.ucmd();
for f in &flags {
ucmd.arg(f);
}
ucmd.arg("u+x")
ucmd.args(&flags)
.arg("u+x")
.umask(0o022)
.arg(directory)
.succeeds()
@ -1027,3 +1023,163 @@ fn test_chmod_traverse_symlink_combo() {
);
}
}
#[test]
fn test_chmod_recursive_symlink_to_directory_command_line() {
// Test behavior when the symlink itself is a command-line argument
let scenarios = [
(vec!["-R"], true), // Default behavior (-H): follow symlinks that are command line args
(vec!["-R", "-H"], true), // Explicit -H: follow symlinks that are command line args
(vec!["-R", "-L"], true), // -L: follow all symlinks
(vec!["-R", "-P"], false), // -P: never follow symlinks
];
for (flags, should_follow_symlink_dir) in scenarios {
let scene = TestScenario::new(util_name!());
let at = &scene.fixtures;
let target_dir = "target_dir";
let symlink_to_dir = "link_dir";
let file_in_target = "file_in_target";
at.mkdir(target_dir);
at.touch(format!("{target_dir}/{file_in_target}"));
at.symlink_dir(target_dir, symlink_to_dir);
set_permissions(
at.plus(format!("{target_dir}/{file_in_target}")),
Permissions::from_mode(0o644),
)
.unwrap();
let mut ucmd = scene.ucmd();
ucmd.args(&flags)
.arg("go-rwx")
.arg(symlink_to_dir) // The symlink itself is the command-line argument
.succeeds()
.no_stderr();
let actual_file_perms = at
.metadata(&format!("{target_dir}/{file_in_target}"))
.permissions()
.mode();
if should_follow_symlink_dir {
// When following symlinks, the file inside the target directory should have its permissions changed
assert_eq!(
actual_file_perms, 0o100_600,
"For flags {flags:?}, expected file perms when following symlinks = 600, got = {actual_file_perms:o}",
);
} else {
// When not following symlinks, the file inside the target directory should be unchanged
assert_eq!(
actual_file_perms, 0o100_644,
"For flags {flags:?}, expected file perms when not following symlinks = 644, got = {actual_file_perms:o}",
);
}
}
}
#[test]
fn test_chmod_recursive_symlink_during_traversal() {
// Test behavior when symlinks are encountered during directory traversal
let scenarios = [
(vec!["-R"], false), // Default behavior (-H): don't follow symlinks encountered during traversal
(vec!["-R", "-H"], false), // Explicit -H: don't follow symlinks encountered during traversal
(vec!["-R", "-L"], true), // -L: follow all symlinks including those found during traversal
(vec!["-R", "-P"], false), // -P: never follow symlinks
];
for (flags, should_follow_symlink_dir) in scenarios {
let scene = TestScenario::new(util_name!());
let at = &scene.fixtures;
let directory = "dir";
let target_dir = "target_dir";
let symlink_to_dir = "link_dir";
let file_in_target = "file_in_target";
at.mkdir(directory);
at.mkdir(target_dir);
at.touch(format!("{target_dir}/{file_in_target}"));
at.symlink_dir(target_dir, &format!("{directory}/{symlink_to_dir}"));
set_permissions(
at.plus(format!("{target_dir}/{file_in_target}")),
Permissions::from_mode(0o644),
)
.unwrap();
let mut ucmd = scene.ucmd();
ucmd.args(&flags)
.arg("go-rwx")
.arg(directory) // The directory is the command-line argument
.succeeds()
.no_stderr();
let actual_file_perms = at
.metadata(&format!("{target_dir}/{file_in_target}"))
.permissions()
.mode();
if should_follow_symlink_dir {
// When following symlinks, the file inside the target directory should have its permissions changed
assert_eq!(
actual_file_perms, 0o100_600,
"For flags {flags:?}, expected file perms when following symlinks = 600, got = {actual_file_perms:o}",
);
} else {
// When not following symlinks, the file inside the target directory should be unchanged
assert_eq!(
actual_file_perms, 0o100_644,
"For flags {flags:?}, expected file perms when not following symlinks = 644, got = {actual_file_perms:o}",
);
}
}
}
#[test]
fn test_chmod_recursive_symlink_combinations() {
let scene = TestScenario::new(util_name!());
let at = &scene.fixtures;
let directory = "dir";
let target_dir = "target_dir";
let target_file = "target_file";
let symlink_to_dir = "link_dir";
let symlink_to_file = "link_file";
let file_in_target = "file";
at.mkdir(directory);
at.mkdir(target_dir);
at.touch(target_file);
at.touch(format!("{target_dir}/{file_in_target}"));
at.symlink_dir(target_dir, &format!("{directory}/{symlink_to_dir}"));
at.symlink_file(target_file, &format!("{directory}/{symlink_to_file}"));
set_permissions(at.plus(target_file), Permissions::from_mode(0o644)).unwrap();
set_permissions(
at.plus(format!("{target_dir}/{file_in_target}")),
Permissions::from_mode(0o644),
)
.unwrap();
// Test with -R -L (follow all symlinks)
scene
.ucmd()
.arg("-R")
.arg("-L")
.arg("go-rwx")
.arg(directory)
.succeeds()
.no_stderr();
// Both target file and file in target directory should have permissions changed
assert_eq!(at.metadata(target_file).permissions().mode(), 0o100_600);
assert_eq!(
at.metadata(&format!("{target_dir}/{file_in_target}"))
.permissions()
.mode(),
0o100_600
);
}