From 90dca1fd1b2dec0573b493528e61268441906c72 Mon Sep 17 00:00:00 2001 From: mattsu <35655889+mattsu2020@users.noreply.github.com> Date: Sat, 6 Dec 2025 03:50:34 +0900 Subject: [PATCH 01/11] chmod:fix safe traversal/access (#9554) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(chmod): use dirfd for recursive subdirectory traversal - Update chmod recursive logic to use directory file descriptors instead of full paths for subdirectories - Improves performance, avoids path length issues, and ensures dirfd-relative openat calls - Add test to verify strace output shows no AT_FDCWD with multi-component paths * test(chmod): add spell-check ignore for dirfd, subdirs, openat, FDCWD Added a spell-checker ignore directive in the chmod test file to suppress false positives for legitimate technical terms used in Unix API calls. * test(chmod): enforce strace requirement in recursive test, fail fast instead of skip Previously, the test_chmod_recursive_uses_dirfd_for_subdirs test skipped gracefully if strace was unavailable, without failing. This change enforces the strace dependency by failing the test immediately if strace is not installed or runnable, ensuring the test runs reliably in environments where it is expected to pass, and preventing silent skips. * ci: install strace in Ubuntu CI jobs for debugging system calls Add installation of strace tool on Ubuntu runners in both individual build/test and feature build/test jobs. This enables tracing system calls during execution, aiding in debugging and performance analysis within the CI/CD pipeline. Updated existing apt-get commands and added conditional steps for Linux-only installations. * ci: Add strace installation to Ubuntu-based CI workflows Install strace on ubuntu-latest runners across multiple jobs to enable system call tracing for testing purposes, ensuring compatibility with tests that require this debugging tool. This includes updating package lists in existing installation steps. * chore(build): install strace and prevent apt prompts in Cross.toml pre-build Modified the pre-build command to install strace utility for debugging and added -y flag to apt-get install to skip prompts, ensuring non-interactive builds. * feat(build): support Alpine-based cross images in pre-build Detect package manager (apt vs apk) to install tzdata and strace in both Debian/Ubuntu and Alpine *-musl targets. Added fallback warning for unsupported managers. This ensures strace is available for targets using Alpine, which doesn't have apt-get. * refactor(build): improve pre-build script readability by using multi-line strings Replace escaped multi-line string with triple-quoted string for better readability in Cross.toml. * feat(ci): install strace in WSL2 GitHub Actions workflow Install strace utility in the WSL2 environment to support tracing system calls during testing. Minor update to Cross.toml spell-checker ignore list for consistency with change. * ci(wsl2): install strace as root with non-interactive apt-get Updated the WSL2 workflow step to use root shell (wsl-bash-root) for installing strace, removing sudo calls and adding DEBIAN_FRONTEND=noninteractive to prevent prompts. This improves CI reliability by ensuring direct root access and automated, interrupt-free package installation. * ci: Move strace installation to user shell and update spell ignore Fix WSL2 GitHub Actions workflow by installing strace as the user instead of root for better permission handling, and add "noninteractive" to the spell-checker ignore comment for consistency with the new apt-get command. This ensures the tool is available in the testing environment without unnecessary privilege escalation. * chore: ci: remove unused strace installation from CI workflows Remove strace package installation from multiple GitHub Actions workflow files (CICD.yml, l10n.yml, wsl2.yml). Strace was historically installed in Ubuntu jobs for debugging system calls, but it's no longer required for the tests and builds, reducing CI setup time and dependencies. * ci: add strace installation and fix spell-checker comments in CI files - Install strace package in CICD workflow to support safe traversal verification for utilities like rm, chmod, chown, chgrp, mv, and du, enabling syscall tracing for testing. - Clean up spell-checker ignore comments in wsl2.yml and Cross.toml by removing misplaced flags.第二个测试产品**ci: add strace installation and fix spell-checker comments in CI files** - Install strace package in CICD workflow to support safe traversal verification for utilities like rm, chmod, chown, chgrp, mv, and du, enabling syscall tracing for testing. - Clean up spell-checker ignore comments in wsl2.yml and Cross.toml by removing misplaced flags. * test: add regression guard for recursive chmod dirfd-relative traversal Add a check in check-safe-traversal.sh to ensure recursive chmod operations use dirfd-relative openat calls instead of AT_FDCWD with multi-component paths, preventing potential race conditions. Ignore the corresponding Rust test as it is now covered by this shell script guard. --- src/uu/chmod/src/chmod.rs | 19 +++++++++++++-- tests/by-util/test_chmod.rs | 46 ++++++++++++++++++++++++++++++++++++ util/check-safe-traversal.sh | 5 ++++ 3 files changed, 68 insertions(+), 2 deletions(-) diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index c782ad429a4..15b608af6b2 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -522,9 +522,24 @@ impl Chmoder { .safe_chmod_file(&entry_path, dir_fd, &entry_name, meta.mode() & 0o7777) .and(r); - // Recurse into subdirectories + // Recurse into subdirectories using the existing directory fd if meta.is_dir() { - r = self.walk_dir_with_context(&entry_path, false).and(r); + match dir_fd.open_subdir(&entry_name) { + Ok(child_dir_fd) => { + r = self.safe_traverse_dir(&child_dir_fd, &entry_path).and(r); + } + Err(err) => { + let error = if err.kind() == std::io::ErrorKind::PermissionDenied { + ChmodError::PermissionDenied( + entry_path.to_string_lossy().to_string(), + ) + .into() + } else { + err.into() + }; + r = r.and(Err(error)); + } + } } } } diff --git a/tests/by-util/test_chmod.rs b/tests/by-util/test_chmod.rs index 1378aab00d2..e4d4b028474 100644 --- a/tests/by-util/test_chmod.rs +++ b/tests/by-util/test_chmod.rs @@ -2,6 +2,7 @@ // // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. +// spell-checker:ignore (words) dirfd subdirs openat FDCWD use std::fs::{OpenOptions, Permissions, metadata, set_permissions}; use std::os::unix::fs::{OpenOptionsExt, PermissionsExt}; @@ -1280,6 +1281,51 @@ fn test_chmod_non_utf8_paths() { ); } +#[cfg(all(target_os = "linux", feature = "chmod"))] +#[test] +#[ignore = "covered by util/check-safe-traversal.sh"] +fn test_chmod_recursive_uses_dirfd_for_subdirs() { + use std::process::Command; + use uutests::get_tests_binary; + + // strace is required; fail fast if it is missing or not runnable + let output = Command::new("strace") + .arg("-V") + .output() + .expect("strace not found; install strace to run this test"); + assert!( + output.status.success(), + "strace -V failed; ensure strace is installed and usable" + ); + + let (at, _ucmd) = at_and_ucmd!(); + at.mkdir("x"); + at.mkdir("x/y"); + at.mkdir("x/y/z"); + + let log_path = at.plus_as_string("strace.log"); + + let status = Command::new("strace") + .arg("-e") + .arg("openat") + .arg("-o") + .arg(&log_path) + .arg(get_tests_binary!()) + .args(["chmod", "-R", "+x", "x"]) + .current_dir(&at.subdir) + .status() + .expect("failed to run strace"); + assert!(status.success(), "strace run failed"); + + let log = at.read("strace.log"); + + // Regression guard: ensure recursion uses dirfd-relative openat instead of AT_FDCWD with a multi-component path + assert!( + !log.contains("openat(AT_FDCWD, \"x/y"), + "chmod recursed using AT_FDCWD with a multi-component path; expected dirfd-relative openat" + ); +} + #[test] fn test_chmod_colored_output() { // Test colored help message diff --git a/util/check-safe-traversal.sh b/util/check-safe-traversal.sh index ed3c5a78eff..8dc9b04cf52 100755 --- a/util/check-safe-traversal.sh +++ b/util/check-safe-traversal.sh @@ -173,6 +173,11 @@ fi if echo "$AVAILABLE_UTILS" | grep -q "chmod"; then cp -r test_dir test_chmod check_utility "chmod" "openat,fchmodat,newfstatat,chmod" "openat fchmodat" "-R 755 test_chmod" "recursive_chmod" + + # Additional regression guard: ensure recursion uses dirfd-relative openat, not AT_FDCWD with a multi-component path + if grep -q 'openat(AT_FDCWD, "test_chmod/' strace_chmod_recursive_chmod.log; then + fail_immediately "chmod recursed using AT_FDCWD with a multi-component path; expected dirfd-relative openat" + fi fi # Test chown - should use openat, fchownat, newfstatat From 5abbbe687c92f748bdb7d74fd70abfacf4bc17bd Mon Sep 17 00:00:00 2001 From: Chris Dryden Date: Fri, 5 Dec 2025 14:02:44 -0500 Subject: [PATCH 02/11] Merge pull request #9561 from ChrisDryden/seq_benches seq: adding large integers benchmarks --- src/uu/seq/benches/seq_bench.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/uu/seq/benches/seq_bench.rs b/src/uu/seq/benches/seq_bench.rs index d8c52131d1c..11956e8c04a 100644 --- a/src/uu/seq/benches/seq_bench.rs +++ b/src/uu/seq/benches/seq_bench.rs @@ -15,6 +15,14 @@ fn seq_integers(bencher: Bencher) { }); } +/// Benchmark large integer +#[divan::bench] +fn seq_large_integers(bencher: Bencher) { + bencher.bench(|| { + black_box(run_util_function(uumain, &["4e10003", "4e10003"])); + }); +} + /// Benchmark sequence with custom separator #[divan::bench] fn seq_custom_separator(bencher: Bencher) { From 4a9628ca732f049df61d87474e40449a39552d8e Mon Sep 17 00:00:00 2001 From: Etienne Cordonnier Date: Tue, 25 Nov 2025 00:40:16 +0100 Subject: [PATCH 03/11] install: do not call chown when called as root - `pseudo` is a tool which simulates being root by intercepting calls to e.g. `geteuid` and `chown` (by using the `LD_PRELOAD` mechanism). This is used e.g. to build filesystems for embedded devices without running as root on the build machine. - the `chown` call getting removed in this commit does not work when running with `pseudo` and using `PSEUDO_IGNORE_PATHS`: in this case, the call to `geteuid()` gets intercepted by `libpseudo.so` and returns 0, however the call to `chown()` isn't intercepted by `libpseudo.so` in case it is in a path from `PSEUDO_IGNORE_PATHS`, and will thus fail since the process is not really root - the call to `chown()` was added in https://github.com/uutils/coreutils/pull/5735 with the intent of making the test `install-C-root.sh` pass, however it isn't required (GNU coreutils also does not call `chown` just because `install` was called as root) Fixes https://github.com/uutils/coreutils/issues/9116 Signed-off-by: Etienne Cordonnier --- src/uu/install/src/install.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/uu/install/src/install.rs b/src/uu/install/src/install.rs index 49252dcf9e2..ab05c7ca059 100644 --- a/src/uu/install/src/install.rs +++ b/src/uu/install/src/install.rs @@ -711,10 +711,9 @@ fn copy_files_into_dir(files: &[PathBuf], target_dir: &Path, b: &Behavior) -> UR Ok(()) } -/// Handle incomplete user/group parings for chown. +/// Handle ownership changes when -o/--owner or -g/--group flags are used. /// /// Returns a Result type with the Err variant containing the error message. -/// If the user is root, revert the uid & gid /// /// # Parameters /// @@ -735,11 +734,8 @@ fn chown_optional_user_group(path: &Path, b: &Behavior) -> UResult<()> { // Determine the owner and group IDs to be used for chown. let (owner_id, group_id) = if b.owner_id.is_some() || b.group_id.is_some() { (b.owner_id, b.group_id) - } else if geteuid() == 0 { - // Special case for root user. - (Some(0), Some(0)) } else { - // No chown operation needed. + // No chown operation needed - file ownership comes from process naturally. return Ok(()); }; From 61e760b29cdb1397b21c59fadb20e08c12cbd38a Mon Sep 17 00:00:00 2001 From: Daniel Hofstetter Date: Fri, 5 Dec 2025 23:07:16 +0100 Subject: [PATCH 04/11] du: handle `--files0-from=-` with piped in `-` (#8985) * du: handle --files0-from=- with piped in '-' * build-gnu.sh: remove incorrect string replacement in tests/du/files0-from.pl --------- Co-authored-by: Sylvestre Ledru --- src/uu/du/locales/en-US.ftl | 1 + src/uu/du/locales/fr-FR.ftl | 1 + src/uu/du/src/du.rs | 10 ++++++---- tests/by-util/test_du.rs | 16 +++++++++++++--- util/build-gnu.sh | 1 - 5 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/uu/du/locales/en-US.ftl b/src/uu/du/locales/en-US.ftl index b503d8d539f..bd6c095bacb 100644 --- a/src/uu/du/locales/en-US.ftl +++ b/src/uu/du/locales/en-US.ftl @@ -69,6 +69,7 @@ du-error-printing-thread-panicked = Printing thread panicked. du-error-invalid-suffix = invalid suffix in --{ $option } argument { $value } du-error-invalid-argument = invalid --{ $option } argument { $value } du-error-argument-too-large = --{ $option } argument { $value } too large +du-error-hyphen-file-name-not-allowed = when reading file names from standard input, no file name of '-' allowed # Verbose/status messages du-verbose-ignored = { $path } ignored diff --git a/src/uu/du/locales/fr-FR.ftl b/src/uu/du/locales/fr-FR.ftl index e89385213aa..81bc80c719f 100644 --- a/src/uu/du/locales/fr-FR.ftl +++ b/src/uu/du/locales/fr-FR.ftl @@ -69,6 +69,7 @@ du-error-printing-thread-panicked = Le thread d'affichage a paniqué. du-error-invalid-suffix = suffixe invalide dans l'argument --{ $option } { $value } du-error-invalid-argument = argument --{ $option } invalide { $value } du-error-argument-too-large = argument --{ $option } { $value } trop grand +du-error-hyphen-file-name-not-allowed = le nom de fichier '-' n'est pas autorisé lors de la lecture de l'entrée standard # Messages verbeux/de statut du-verbose-ignored = { $path } ignoré diff --git a/src/uu/du/src/du.rs b/src/uu/du/src/du.rs index 522252a8b71..f57228d7625 100644 --- a/src/uu/du/src/du.rs +++ b/src/uu/du/src/du.rs @@ -2,16 +2,15 @@ // // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. +// // spell-checker:ignore fstatat openat dirfd use clap::{Arg, ArgAction, ArgMatches, Command, builder::PossibleValue}; use glob::Pattern; use std::collections::HashSet; use std::env; -use std::ffi::OsStr; -use std::ffi::OsString; -use std::fs::Metadata; -use std::fs::{self, DirEntry, File}; +use std::ffi::{OsStr, OsString}; +use std::fs::{self, DirEntry, File, Metadata}; use std::io::{BufRead, BufReader, stdout}; #[cfg(not(windows))] use std::os::unix::fs::MetadataExt; @@ -942,6 +941,9 @@ fn read_files_from(file_name: &OsStr) -> Result, std::io::Error> { translate!("du-error-invalid-zero-length-file-name", "file" => file_name.to_string_lossy(), "line" => line_number) ); set_exit_code(1); + } else if path == b"-" && file_name == "-" { + show_error!("{}", translate!("du-error-hyphen-file-name-not-allowed")); + set_exit_code(1); } else { let p = PathBuf::from(&*uucore::os_str_from_bytes(&path).unwrap()); if !paths.contains(&p) { diff --git a/tests/by-util/test_du.rs b/tests/by-util/test_du.rs index 224626b210a..bc97cb28f55 100644 --- a/tests/by-util/test_du.rs +++ b/tests/by-util/test_du.rs @@ -5,17 +5,16 @@ // spell-checker:ignore (paths) atim sublink subwords azerty azeaze xcwww azeaz amaz azea qzerty tazerty tsublink testfile1 testfile2 filelist fpath testdir testfile // spell-checker:ignore selfref ELOOP smallfile + #[cfg(not(windows))] use regex::Regex; -use uutests::at_and_ucmd; -use uutests::new_ucmd; #[cfg(not(target_os = "windows"))] use uutests::unwrap_or_return; use uutests::util::TestScenario; #[cfg(not(target_os = "windows"))] use uutests::util::expected_result; -use uutests::util_name; +use uutests::{at_and_ucmd, new_ucmd, util_name}; #[cfg(not(target_os = "openbsd"))] const SUB_DIR: &str = "subdir/deeper"; @@ -1399,6 +1398,17 @@ fn test_du_files0_from_stdin_with_invalid_zero_length_file_names() { .stderr_contains("-:2: invalid zero-length file name"); } +#[test] +fn test_du_files0_from_stdin_with_stdin_as_input() { + new_ucmd!() + .arg("--files0-from=-") + .pipe_in("-") + .fails_with_code(1) + .stderr_is( + "du: when reading file names from standard input, no file name of '-' allowed\n", + ); +} + #[test] fn test_du_files0_from_dir() { let ts = TestScenario::new(util_name!()); diff --git a/util/build-gnu.sh b/util/build-gnu.sh index 532e905929d..223fc895b6c 100755 --- a/util/build-gnu.sh +++ b/util/build-gnu.sh @@ -285,7 +285,6 @@ test -f "${UU_BUILD_DIR}/getlimits" || cp src/getlimits "${UU_BUILD_DIR}" # Remove the extra output check "${SED}" -i -e "s|Try '\$prog --help' for more information.\\\n||" tests/du/files0-from.pl -"${SED}" -i -e "s|when reading file names from stdin, no file name of\"|-: No such file or directory\n\"|" -e "s| '-' allowed\\\n||" tests/du/files0-from.pl "${SED}" -i -e "s|-: No such file or directory|cannot access '-': No such file or directory|g" tests/du/files0-from.pl # Skip the move-dir-while-traversing test - our implementation uses safe traversal with openat() From 5c6de96355634226641f0cb1503b24216017d2f6 Mon Sep 17 00:00:00 2001 From: mattsu Date: Sat, 6 Dec 2025 11:28:21 +0900 Subject: [PATCH 05/11] perf: optimize rm prompts by reusing stat data to avoid extra syscalls This change adds inline functions for checking file modes and refactors prompt functions to accept pre-fetched stat data. It modifies safe_remove_* functions to handle paths without parents and updates safe_remove_dir_recursive to fetch and reuse initial mode. This reduces redundant statx system calls, improving performance during recursive removals. --- src/uu/rm/src/platform/linux.rs | 93 ++++++++++++++++++++++++++++----- util/check-safe-traversal.sh | 8 +++ 2 files changed, 89 insertions(+), 12 deletions(-) diff --git a/src/uu/rm/src/platform/linux.rs b/src/uu/rm/src/platform/linux.rs index 6c7d3239572..f652dcf54a7 100644 --- a/src/uu/rm/src/platform/linux.rs +++ b/src/uu/rm/src/platform/linux.rs @@ -10,19 +10,81 @@ use indicatif::ProgressBar; use std::ffi::OsStr; use std::fs; +use std::io::{self, stdin, IsTerminal}; use std::path::Path; +use std::os::unix::fs::PermissionsExt; use uucore::display::Quotable; use uucore::error::FromIo; +use uucore::prompt_yes; use uucore::safe_traversal::DirFd; use uucore::show_error; use uucore::translate; use super::super::{ - InteractiveMode, Options, is_dir_empty, is_readable_metadata, prompt_descend, prompt_dir, - prompt_file, remove_file, show_permission_denied_error, show_removal_error, - verbose_removed_directory, verbose_removed_file, + InteractiveMode, Options, is_dir_empty, is_readable_metadata, prompt_descend, remove_file, + show_permission_denied_error, show_removal_error, verbose_removed_directory, + verbose_removed_file, }; +#[inline] +fn mode_readable(mode: libc::mode_t) -> bool { + (mode & libc::S_IRUSR) != 0 +} + +#[inline] +fn mode_writable(mode: libc::mode_t) -> bool { + (mode & libc::S_IWUSR) != 0 +} + +/// File prompt that reuses existing stat data to avoid extra statx calls +fn prompt_file_with_stat(path: &Path, stat: &libc::stat, options: &Options) -> bool { + if options.interactive == InteractiveMode::Never { + return true; + } + + if options.interactive == InteractiveMode::Always { + if (stat.st_mode & libc::S_IFMT) == libc::S_IFLNK { + return prompt_yes!("remove symbolic link {}?", path.quote()); + } + } + + let writable = mode_writable(stat.st_mode); + let len = stat.st_size as u64; + let stdin_ok = options.__presume_input_tty.unwrap_or(false) || stdin().is_terminal(); + + match (stdin_ok, writable, options.interactive) { + (false, _, InteractiveMode::PromptProtected) => true, + (_, true, _) => true, + (_, false, _) if len == 0 => { + prompt_yes!("remove write-protected regular empty file {}?", path.quote()) + } + _ => prompt_yes!("remove write-protected regular file {}?", path.quote()), + } +} + +/// Directory prompt that reuses existing stat data to avoid extra statx calls +fn prompt_dir_with_mode(path: &Path, mode: libc::mode_t, options: &Options) -> bool { + if options.interactive == InteractiveMode::Never { + return true; + } + + let readable = mode_readable(mode); + let writable = mode_writable(mode); + let stdin_ok = options.__presume_input_tty.unwrap_or(false) || stdin().is_terminal(); + + match (stdin_ok, readable, writable, options.interactive) { + (false, _, _, InteractiveMode::PromptProtected) => true, + (false, false, false, InteractiveMode::Never) => true, + (_, false, false, _) => prompt_yes!("attempt removal of inaccessible directory {}?", path.quote()), + (_, false, true, InteractiveMode::Always) => { + prompt_yes!("attempt removal of inaccessible directory {}?", path.quote()) + } + (_, true, false, _) => prompt_yes!("remove write-protected directory {}?", path.quote()), + (_, _, _, InteractiveMode::Always) => prompt_yes!("remove directory {}?", path.quote()), + (_, _, _, _) => true, + } +} + /// Whether the given file or directory is readable. pub fn is_readable(path: &Path) -> bool { fs::metadata(path).is_ok_and(|metadata| is_readable_metadata(&metadata)) @@ -34,7 +96,8 @@ pub fn safe_remove_file( options: &Options, progress_bar: Option<&ProgressBar>, ) -> Option { - let parent = path.parent()?; + // If there is no parent (path is directly under cwd), unlinkat relative to "." + let parent = path.parent().unwrap_or(Path::new(".")); let file_name = path.file_name()?; let dir_fd = DirFd::open(parent).ok()?; @@ -65,7 +128,7 @@ pub fn safe_remove_empty_dir( options: &Options, progress_bar: Option<&ProgressBar>, ) -> Option { - let parent = path.parent()?; + let parent = path.parent().unwrap_or(Path::new(".")); let dir_name = path.file_name()?; let dir_fd = DirFd::open(parent).ok()?; @@ -196,15 +259,15 @@ pub fn safe_remove_dir_recursive( ) -> bool { // Base case 1: this is a file or a symbolic link. // Use lstat to avoid race condition between check and use - match fs::symlink_metadata(path) { + let initial_mode = match fs::symlink_metadata(path) { Ok(metadata) if !metadata.is_dir() => { return remove_file(path, options, progress_bar); } - Ok(_) => {} + Ok(metadata) => metadata.permissions().mode(), Err(e) => { return show_removal_error(e, path); } - } + }; // Try to open the directory using DirFd for secure traversal let dir_fd = match DirFd::open(path) { @@ -233,7 +296,9 @@ pub fn safe_remove_dir_recursive( error } else { // Ask user permission if needed - if options.interactive == InteractiveMode::Always && !prompt_dir(path, options) { + if options.interactive == InteractiveMode::Always + && !prompt_dir_with_mode(path, initial_mode, options) + { return false; } @@ -252,7 +317,11 @@ pub fn safe_remove_dir_recursive( } // Directory is empty and user approved removal - remove_dir_with_special_cases(path, options, error) + if let Some(result) = safe_remove_empty_dir(path, options, progress_bar) { + result + } else { + remove_dir_with_special_cases(path, options, error) + } } } @@ -324,7 +393,7 @@ pub fn safe_remove_dir_recursive_impl(path: &Path, dir_fd: &DirFd, options: &Opt // Ask user permission if needed for this subdirectory if !child_error && options.interactive == InteractiveMode::Always - && !prompt_dir(&entry_path, options) + && !prompt_dir_with_mode(&entry_path, entry_stat.st_mode, options) { continue; } @@ -335,7 +404,7 @@ pub fn safe_remove_dir_recursive_impl(path: &Path, dir_fd: &DirFd, options: &Opt } } else { // Remove file - check if user wants to remove it first - if prompt_file(&entry_path, options) { + if prompt_file_with_stat(&entry_path, &entry_stat, options) { error = handle_unlink(dir_fd, entry_name.as_ref(), &entry_path, false, options); } } diff --git a/util/check-safe-traversal.sh b/util/check-safe-traversal.sh index 8dc9b04cf52..3ce1574aaaa 100755 --- a/util/check-safe-traversal.sh +++ b/util/check-safe-traversal.sh @@ -167,6 +167,14 @@ fi if echo "$AVAILABLE_UTILS" | grep -q "rm"; then cp -r test_dir test_rm check_utility "rm" "openat,unlinkat,newfstatat,unlink,rmdir" "openat" "-rf test_rm" "recursive_remove" + + # Regression guard: rm must not issue path-based statx calls (should rely on dirfd-relative newfstatat) + if grep -qE 'statx\(AT_FDCWD, "/' strace_rm_recursive_remove.log; then + fail_immediately "rm is using path-based statx (absolute path); expected dirfd-relative newfstatat" + fi + if grep -qE 'statx\(AT_FDCWD, "[^"]*/' strace_rm_recursive_remove.log; then + fail_immediately "rm is using path-based statx (multi-component relative path); expected dirfd-relative newfstatat" + fi fi # Test chmod - should use openat, fchmodat, newfstatat From d3e938df5e3d35aadac0eaf69e34bc51491f9f38 Mon Sep 17 00:00:00 2001 From: mattsu Date: Sat, 6 Dec 2025 11:35:24 +0900 Subject: [PATCH 06/11] feat(rm/linux): Refine interactive file removal prompts - Add specific prompts for symlinks and empty files in 'Always' mode - Refactor matching logic for better clarity and to match GNU rm behavior - Improve handling of write-protected and non-terminal stdin scenarios This enhances the user experience by providing more accurate and targeted confirmations during file removal on Linux. --- src/uu/rm/src/platform/linux.rs | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/src/uu/rm/src/platform/linux.rs b/src/uu/rm/src/platform/linux.rs index f652dcf54a7..c81079e1295 100644 --- a/src/uu/rm/src/platform/linux.rs +++ b/src/uu/rm/src/platform/linux.rs @@ -42,20 +42,29 @@ fn prompt_file_with_stat(path: &Path, stat: &libc::stat, options: &Options) -> b return true; } + let is_symlink = (stat.st_mode & libc::S_IFMT) == libc::S_IFLNK; + let writable = mode_writable(stat.st_mode); + let len = stat.st_size as u64; + let stdin_ok = options.__presume_input_tty.unwrap_or(false) || stdin().is_terminal(); + + // Match original behaviour: + // - Interactive::Always: always prompt (with symlink/file specific message) + // - Otherwise: prompt only when write-protected if options.interactive == InteractiveMode::Always { - if (stat.st_mode & libc::S_IFMT) == libc::S_IFLNK { + if is_symlink { return prompt_yes!("remove symbolic link {}?", path.quote()); } + if len == 0 { + return prompt_yes!("remove regular empty file {}?", path.quote()); + } + return prompt_yes!("remove file {}?", path.quote()); } - let writable = mode_writable(stat.st_mode); - let len = stat.st_size as u64; - let stdin_ok = options.__presume_input_tty.unwrap_or(false) || stdin().is_terminal(); - - match (stdin_ok, writable, options.interactive) { - (false, _, InteractiveMode::PromptProtected) => true, - (_, true, _) => true, - (_, false, _) if len == 0 => { + // Interactive::Once or ::PromptProtected paths + match (stdin_ok, writable) { + (false, _) if options.interactive == InteractiveMode::PromptProtected => true, + (_, true) => true, + (_, false) if len == 0 => { prompt_yes!("remove write-protected regular empty file {}?", path.quote()) } _ => prompt_yes!("remove write-protected regular file {}?", path.quote()), From 83108241081e2a7bc7fd827e662419ccdd256d66 Mon Sep 17 00:00:00 2001 From: mattsu Date: Sat, 6 Dec 2025 11:38:09 +0900 Subject: [PATCH 07/11] refactor(rm/linux): reformat prompt_yes! macros for improved readability Refactored multiple call sites of the prompt_yes! macro in linux.rs to use consistent multi-line formatting, enhancing code readability and adhering to style guidelines without altering functionality. Adjusted import ordering slightly for better organization. --- src/uu/rm/src/platform/linux.rs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/uu/rm/src/platform/linux.rs b/src/uu/rm/src/platform/linux.rs index c81079e1295..b79af57812e 100644 --- a/src/uu/rm/src/platform/linux.rs +++ b/src/uu/rm/src/platform/linux.rs @@ -10,9 +10,9 @@ use indicatif::ProgressBar; use std::ffi::OsStr; use std::fs; -use std::io::{self, stdin, IsTerminal}; -use std::path::Path; +use std::io::{self, IsTerminal, stdin}; use std::os::unix::fs::PermissionsExt; +use std::path::Path; use uucore::display::Quotable; use uucore::error::FromIo; use uucore::prompt_yes; @@ -65,7 +65,10 @@ fn prompt_file_with_stat(path: &Path, stat: &libc::stat, options: &Options) -> b (false, _) if options.interactive == InteractiveMode::PromptProtected => true, (_, true) => true, (_, false) if len == 0 => { - prompt_yes!("remove write-protected regular empty file {}?", path.quote()) + prompt_yes!( + "remove write-protected regular empty file {}?", + path.quote() + ) } _ => prompt_yes!("remove write-protected regular file {}?", path.quote()), } @@ -84,9 +87,15 @@ fn prompt_dir_with_mode(path: &Path, mode: libc::mode_t, options: &Options) -> b match (stdin_ok, readable, writable, options.interactive) { (false, _, _, InteractiveMode::PromptProtected) => true, (false, false, false, InteractiveMode::Never) => true, - (_, false, false, _) => prompt_yes!("attempt removal of inaccessible directory {}?", path.quote()), + (_, false, false, _) => prompt_yes!( + "attempt removal of inaccessible directory {}?", + path.quote() + ), (_, false, true, InteractiveMode::Always) => { - prompt_yes!("attempt removal of inaccessible directory {}?", path.quote()) + prompt_yes!( + "attempt removal of inaccessible directory {}?", + path.quote() + ) } (_, true, false, _) => prompt_yes!("remove write-protected directory {}?", path.quote()), (_, _, _, InteractiveMode::Always) => prompt_yes!("remove directory {}?", path.quote()), From 34f12f0898a241a1ad23b1a6a91ce59fff0e6188 Mon Sep 17 00:00:00 2001 From: mattsu Date: Sat, 6 Dec 2025 11:52:47 +0900 Subject: [PATCH 08/11] refactor(src/uu/rm/src/platform/linux.rs): remove unused 'self' import from std::io Removed the unused 'self' import from the std::io module to clean up the code and avoid potential confusion, as it was not referenced anywhere in the file. This is a minor refactoring for better maintainability. --- src/uu/rm/src/platform/linux.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/rm/src/platform/linux.rs b/src/uu/rm/src/platform/linux.rs index b79af57812e..4e1374e538e 100644 --- a/src/uu/rm/src/platform/linux.rs +++ b/src/uu/rm/src/platform/linux.rs @@ -10,7 +10,7 @@ use indicatif::ProgressBar; use std::ffi::OsStr; use std::fs; -use std::io::{self, IsTerminal, stdin}; +use std::io::{IsTerminal, stdin}; use std::os::unix::fs::PermissionsExt; use std::path::Path; use uucore::display::Quotable; From 7bd092f25e9834b959c2736ff2a012d5336d43f3 Mon Sep 17 00:00:00 2001 From: mattsu Date: Sat, 6 Dec 2025 12:04:05 +0900 Subject: [PATCH 09/11] chore(spell-checker): update ignore list to include statx and behaviour Add "statx" (a Linux system call name) and "behaviour" (potential spelling variant) to the spell-checker ignore comment in the rm utility's Linux platform code, preventing false positives in linting. --- src/uu/rm/src/platform/linux.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/rm/src/platform/linux.rs b/src/uu/rm/src/platform/linux.rs index 4e1374e538e..fc4dca63074 100644 --- a/src/uu/rm/src/platform/linux.rs +++ b/src/uu/rm/src/platform/linux.rs @@ -5,7 +5,7 @@ // Linux-specific implementations for the rm utility -// spell-checker:ignore fstatat unlinkat +// spell-checker:ignore fstatat unlinkat statx behaviour use indicatif::ProgressBar; use std::ffi::OsStr; From be318623e491bcc73af8e0bb41a6659ab7dfdc26 Mon Sep 17 00:00:00 2001 From: mattsu Date: Sat, 6 Dec 2025 12:12:02 +0900 Subject: [PATCH 10/11] fix(linux/rm): correct prompting logic for write-protected files in Interactive::Always mode Refactor the prompt_file_with_stat function in src/uu/rm/src/platform/linux.rs to fix inconsistent prompting for Interactive::Always. Previously, it always used non-protected wording regardless of file writability. Now, it checks if the file is writable and uses appropriate messaging (simple for writable, protected for non-writable). The match logic for Interactive::Once and PromptProtected is also simplified using a triple condition for better readability and to ensure empty vs non-empty protected files are distinguished correctly, matching expected rm behavior. --- src/uu/rm/src/platform/linux.rs | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/src/uu/rm/src/platform/linux.rs b/src/uu/rm/src/platform/linux.rs index fc4dca63074..cc3eb1b1f57 100644 --- a/src/uu/rm/src/platform/linux.rs +++ b/src/uu/rm/src/platform/linux.rs @@ -48,28 +48,27 @@ fn prompt_file_with_stat(path: &Path, stat: &libc::stat, options: &Options) -> b let stdin_ok = options.__presume_input_tty.unwrap_or(false) || stdin().is_terminal(); // Match original behaviour: - // - Interactive::Always: always prompt (with symlink/file specific message) - // - Otherwise: prompt only when write-protected + // - Interactive::Always: always prompt; use non-protected wording when writable, + // otherwise fall through to protected wording. if options.interactive == InteractiveMode::Always { if is_symlink { return prompt_yes!("remove symbolic link {}?", path.quote()); } - if len == 0 { - return prompt_yes!("remove regular empty file {}?", path.quote()); + if writable { + return if len == 0 { + prompt_yes!("remove regular empty file {}?", path.quote()) + } else { + prompt_yes!("remove file {}?", path.quote()) + }; } - return prompt_yes!("remove file {}?", path.quote()); + // Not writable: use protected wording below } - // Interactive::Once or ::PromptProtected paths - match (stdin_ok, writable) { - (false, _) if options.interactive == InteractiveMode::PromptProtected => true, - (_, true) => true, - (_, false) if len == 0 => { - prompt_yes!( - "remove write-protected regular empty file {}?", - path.quote() - ) - } + // Interactive::Once or ::PromptProtected (and non-writable Always) paths + match (stdin_ok, writable, len == 0) { + (false, _, _) if options.interactive == InteractiveMode::PromptProtected => true, + (_, true, _) => true, + (_, false, true) => prompt_yes!("remove write-protected regular empty file {}?", path.quote()), _ => prompt_yes!("remove write-protected regular file {}?", path.quote()), } } From c08c802c5f4ac1e7e562c9ccc926c72ee6501094 Mon Sep 17 00:00:00 2001 From: mattsu Date: Sat, 6 Dec 2025 12:14:09 +0900 Subject: [PATCH 11/11] style(rm): wrap long line in prompt_file_with_stat macro for readability Reformatted the prompt_yes! macro call across multiple lines to improve code readability and adhere to line length conventions. No functional changes. --- src/uu/rm/src/platform/linux.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/uu/rm/src/platform/linux.rs b/src/uu/rm/src/platform/linux.rs index cc3eb1b1f57..3e29bf85e7f 100644 --- a/src/uu/rm/src/platform/linux.rs +++ b/src/uu/rm/src/platform/linux.rs @@ -68,7 +68,10 @@ fn prompt_file_with_stat(path: &Path, stat: &libc::stat, options: &Options) -> b match (stdin_ok, writable, len == 0) { (false, _, _) if options.interactive == InteractiveMode::PromptProtected => true, (_, true, _) => true, - (_, false, true) => prompt_yes!("remove write-protected regular empty file {}?", path.quote()), + (_, false, true) => prompt_yes!( + "remove write-protected regular empty file {}?", + path.quote() + ), _ => prompt_yes!("remove write-protected regular file {}?", path.quote()), } }