rm:fix safe traversal/access (#9577)

* chmod:fix safe traversal/access (#9554)

* 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.

* Merge pull request #9561 from ChrisDryden/seq_benches

seq: adding large integers benchmarks

* 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 <ecordonnier@snap.com>

* 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 <sylvestre@debian.org>

* 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.

* 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.

* 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.

* 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.

* 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.

* 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.

* 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.

---------

Signed-off-by: Etienne Cordonnier <ecordonnier@snap.com>
Co-authored-by: Chris Dryden <christopher.paul.dryden@gmail.com>
Co-authored-by: Etienne Cordonnier <ecordonnier@snap.com>
Co-authored-by: Daniel Hofstetter <daniel.hofstetter@42dh.com>
Co-authored-by: Sylvestre Ledru <sylvestre@debian.org>
This commit is contained in:
mattsu
2025-12-25 02:23:50 +09:00
committed by GitHub
parent 8689fb17c4
commit 3edf14eefd
2 changed files with 110 additions and 13 deletions
+102 -13
View File
@@ -5,24 +5,106 @@
// 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;
use std::fs;
use std::io::{IsTerminal, stdin};
use std::os::unix::fs::PermissionsExt;
use std::path::Path;
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;
}
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; 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 writable {
return if len == 0 {
prompt_yes!("remove regular empty file {}?", path.quote())
} else {
prompt_yes!("remove file {}?", path.quote())
};
}
// Not writable: use protected wording below
}
// 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()),
}
}
/// 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 +116,8 @@ pub fn safe_remove_file(
options: &Options,
progress_bar: Option<&ProgressBar>,
) -> Option<bool> {
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 +148,7 @@ pub fn safe_remove_empty_dir(
options: &Options,
progress_bar: Option<&ProgressBar>,
) -> Option<bool> {
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 +279,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 +316,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 +337,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 +413,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 +424,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);
}
}
+8
View File
@@ -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