From c1f2e11fe3f78c227a75774e4ce0ed22ea2701c6 Mon Sep 17 00:00:00 2001 From: Jason Duffy Date: Fri, 12 Jun 2026 17:58:52 -0400 Subject: [PATCH 1/3] rm: implement --one-file-system Previously --one-file-system was parsed but ignored, so a recursive removal would descend into a mount point and fail with a raw "Device or resource busy" error. Capture the device of the tree root once and, while recursing on Unix, skip any directory whose device differs from it, printing "skipping 'PATH', since it's on a different device" and exiting non-zero. --- src/uu/rm/locales/en-US.ftl | 1 + src/uu/rm/src/platform/unix.rs | 36 +++++++++++++++++++++++++++------- src/uu/rm/src/rm.rs | 2 -- tests/by-util/test_rm.rs | 24 +++++++++++++++++++++++ 4 files changed, 54 insertions(+), 9 deletions(-) diff --git a/src/uu/rm/locales/en-US.ftl b/src/uu/rm/locales/en-US.ftl index 8f24f935943..c5eec714ea8 100644 --- a/src/uu/rm/locales/en-US.ftl +++ b/src/uu/rm/locales/en-US.ftl @@ -45,6 +45,7 @@ rm-error-dangerous-recursive-operation = it is dangerous to operate recursively rm-error-dangerous-recursive-operation-same-as-root = it is dangerous to operate recursively on '{$path}' (same as '/') rm-error-use-no-preserve-root = use --no-preserve-root to override this failsafe rm-error-refusing-to-remove-directory = refusing to remove '.' or '..' directory: skipping {$path} +rm-error-skipping-different-device = skipping {$file}, since it's on a different device rm-error-cannot-remove = cannot remove {$file} rm-error-may-not-abbreviate-no-preserve-root = you may not abbreviate the --no-preserve-root option diff --git a/src/uu/rm/src/platform/unix.rs b/src/uu/rm/src/platform/unix.rs index c7ef3f581d9..05967a3ba04 100644 --- a/src/uu/rm/src/platform/unix.rs +++ b/src/uu/rm/src/platform/unix.rs @@ -11,7 +11,7 @@ use indicatif::ProgressBar; use std::ffi::OsStr; use std::fs; use std::io::{IsTerminal, stdin}; -use std::os::unix::fs::PermissionsExt; +use std::os::unix::fs::{MetadataExt, PermissionsExt}; use std::path::Path; use uucore::display::Quotable; use uucore::error::FromIo; @@ -271,11 +271,13 @@ 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 - let initial_mode = match fs::symlink_metadata(path) { + let (initial_mode, root_dev) = match fs::symlink_metadata(path) { Ok(metadata) if !metadata.is_dir() => { return remove_file(path, options, progress_bar); } - Ok(metadata) => metadata.permissions().mode(), + // root_dev is the tree-root device, captured once and compared against + // every subdirectory for --one-file-system (not recomputed per level). + Ok(metadata) => (metadata.permissions().mode(), metadata.dev()), Err(e) => { return show_removal_error(e, path); } @@ -301,7 +303,7 @@ pub fn safe_remove_dir_recursive( } }; - let error = safe_remove_dir_recursive_impl(path, &dir_fd, options); + let error = safe_remove_dir_recursive_impl(path, &dir_fd, options, root_dev); // After processing all children, remove the directory itself if error { @@ -338,7 +340,12 @@ pub fn safe_remove_dir_recursive( } #[cfg(not(target_os = "redox"))] -pub fn safe_remove_dir_recursive_impl(path: &Path, dir_fd: &DirFd, options: &Options) -> bool { +pub fn safe_remove_dir_recursive_impl( + path: &Path, + dir_fd: &DirFd, + options: &Options, + root_dev: u64, +) -> bool { // Read directory entries using safe traversal let entries = match dir_fd.read_dir() { Ok(entries) => entries, @@ -372,6 +379,15 @@ pub fn safe_remove_dir_recursive_impl(path: &Path, dir_fd: &DirFd, options: &Opt let is_dir = ((entry_stat.st_mode as libc::mode_t) & libc::S_IFMT) == libc::S_IFDIR; if is_dir { + if options.one_fs && entry_stat.st_dev as u64 != root_dev { + show_error!( + "{}", + translate!("rm-error-skipping-different-device", "file" => entry_path.quote()) + ); + error = true; + continue; + } + // Ask user if they want to descend into this directory if options.interactive == InteractiveMode::Always && !is_dir_empty(&entry_path) @@ -404,7 +420,8 @@ pub fn safe_remove_dir_recursive_impl(path: &Path, dir_fd: &DirFd, options: &Opt } }; - let child_error = safe_remove_dir_recursive_impl(&entry_path, &child_dir_fd, options); + let child_error = + safe_remove_dir_recursive_impl(&entry_path, &child_dir_fd, options, root_dev); error |= child_error; // Ask user permission if needed for this subdirectory @@ -431,7 +448,12 @@ pub fn safe_remove_dir_recursive_impl(path: &Path, dir_fd: &DirFd, options: &Opt } #[cfg(target_os = "redox")] -pub fn safe_remove_dir_recursive_impl(_path: &Path, _dir_fd: &DirFd, _options: &Options) -> bool { +pub fn safe_remove_dir_recursive_impl( + _path: &Path, + _dir_fd: &DirFd, + _options: &Options, + _root_dev: u64, +) -> bool { // safe_traversal stat_at is not supported on Redox // This shouldn't be called on Redox, but provide a stub for compilation true // Return error diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index 0f25b517579..9077e0e85db 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -151,7 +151,6 @@ pub struct Options { /// If no other option sets this mode, [`InteractiveMode::PromptProtected`] /// is used pub interactive: InteractiveMode, - #[allow(dead_code)] /// `--one-file-system` pub one_fs: bool, /// `--preserve-root`/`--no-preserve-root` @@ -503,7 +502,6 @@ fn count_files_in_directory(p: &Path) -> u64 { 1 + entries_count } -// TODO: implement one-file-system (this may get partially implemented in walkdir) /// Remove (or unlink) the given files /// /// Returns true if it has encountered an error. diff --git a/tests/by-util/test_rm.rs b/tests/by-util/test_rm.rs index 3ac54a3bb58..a3e39e9cb67 100644 --- a/tests/by-util/test_rm.rs +++ b/tests/by-util/test_rm.rs @@ -195,6 +195,30 @@ fn test_recursive() { assert!(!at.file_exists(file_b)); } +#[test] +fn test_one_file_system_same_device() { + // Cross-device skipping needs a mount point (root privileges), so here we + // only guard the single-device case: the whole tree must still be removed. + let (at, mut ucmd) = at_and_ucmd!(); + let dir = "test_rm_one_file_system_dir"; + let subdir = format!("{dir}/subdir"); + let file = format!("{subdir}/file"); + + at.mkdir(dir); + at.mkdir(&subdir); + at.touch(&file); + + ucmd.arg("--one-file-system") + .arg("-rf") + .arg(dir) + .succeeds() + .no_stderr(); + + assert!(!at.file_exists(&file)); + assert!(!at.dir_exists(&subdir)); + assert!(!at.dir_exists(dir)); +} + #[test] fn test_recursive_multiple() { let (at, mut ucmd) = at_and_ucmd!(); From 96f2dbf8ff6bb6f01213b2a863c767cd8aceb9c9 Mon Sep 17 00:00:00 2001 From: Jason Duffy Date: Fri, 12 Jun 2026 18:21:13 -0400 Subject: [PATCH 2/3] rm: implement --preserve-root=all MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GNU's --preserve-root=all refuses to recurse across a device boundary, i.e. into a mount point. Accept the optional =all value and, on Unix, skip any directory whose device differs from its parent's — both when named directly on the command line and when reached during recursion — printing the two-line diagnostic and exiting non-zero. This complements --one-file-system, completing GNU-compatible handling of mount points encountered during recursive removal. --- src/uu/rm/locales/en-US.ftl | 3 +- src/uu/rm/src/platform/unix.rs | 54 ++++++++++++++++++++++++++++++++-- src/uu/rm/src/rm.rs | 14 ++++++++- tests/by-util/test_rm.rs | 33 +++++++++++++++++++++ 4 files changed, 99 insertions(+), 5 deletions(-) diff --git a/src/uu/rm/locales/en-US.ftl b/src/uu/rm/locales/en-US.ftl index c5eec714ea8..1b29a2c32c8 100644 --- a/src/uu/rm/locales/en-US.ftl +++ b/src/uu/rm/locales/en-US.ftl @@ -24,7 +24,7 @@ rm-help-one-file-system = when removing a hierarchy recursively, skip any direct system different from that of the corresponding command line argument (NOT IMPLEMENTED) rm-help-no-preserve-root = do not treat '/' specially -rm-help-preserve-root = do not remove '/' (default) +rm-help-preserve-root = do not remove '/' (default); with 'all', reject any command line argument on a separate device from its parent rm-help-recursive = remove directories and their contents recursively rm-help-dir = remove empty directories rm-help-verbose = explain what is being done @@ -46,6 +46,7 @@ rm-error-dangerous-recursive-operation-same-as-root = it is dangerous to operate rm-error-use-no-preserve-root = use --no-preserve-root to override this failsafe rm-error-refusing-to-remove-directory = refusing to remove '.' or '..' directory: skipping {$path} rm-error-skipping-different-device = skipping {$file}, since it's on a different device +rm-error-and-preserve-root-all-in-effect = and --preserve-root=all is in effect rm-error-cannot-remove = cannot remove {$file} rm-error-may-not-abbreviate-no-preserve-root = you may not abbreviate the --no-preserve-root option diff --git a/src/uu/rm/src/platform/unix.rs b/src/uu/rm/src/platform/unix.rs index 05967a3ba04..37e07f5fb41 100644 --- a/src/uu/rm/src/platform/unix.rs +++ b/src/uu/rm/src/platform/unix.rs @@ -264,6 +264,29 @@ pub fn remove_dir_with_special_cases(path: &Path, options: &Options, error_occur } } +/// `None` when `path` has no parent (the filesystem root). A directory whose +/// own device differs from this is a mount point, which `--preserve-root=all` +/// refuses to cross. +fn parent_device(path: &Path) -> Option { + let parent = match path.parent() { + // A bare name like "b" has an empty parent, meaning the current dir. + Some(p) if p.as_os_str().is_empty() => Path::new("."), + Some(p) => p, + None => return None, + }; + fs::metadata(parent).ok().map(|m| m.dev()) +} + +/// GNU prints two lines, not one, when `--preserve-root=all` stops at a device +/// boundary. +fn show_preserve_root_all_skip(path: &Path) { + show_error!( + "{}", + translate!("rm-error-skipping-different-device", "file" => path.quote()) + ); + show_error!("{}", translate!("rm-error-and-preserve-root-all-in-effect")); +} + pub fn safe_remove_dir_recursive( path: &Path, options: &Options, @@ -283,6 +306,14 @@ pub fn safe_remove_dir_recursive( } }; + // A directory named directly on the command line is itself a mount point + // when its device differs from its parent's; the recursion below only ever + // sees its children, so this boundary has to be caught here. + if options.preserve_root_all && parent_device(path).is_some_and(|dev| dev != root_dev) { + show_preserve_root_all_skip(path); + return true; + } + // Try to open the directory using DirFd for secure traversal let dir_fd = match DirFd::open(path, SymlinkBehavior::Follow) { Ok(fd) => fd, @@ -303,7 +334,8 @@ pub fn safe_remove_dir_recursive( } }; - let error = safe_remove_dir_recursive_impl(path, &dir_fd, options, root_dev); + // Entries of the root directory have the root itself as their parent. + let error = safe_remove_dir_recursive_impl(path, &dir_fd, options, root_dev, root_dev); // After processing all children, remove the directory itself if error { @@ -345,6 +377,7 @@ pub fn safe_remove_dir_recursive_impl( dir_fd: &DirFd, options: &Options, root_dev: u64, + parent_dev: u64, ) -> bool { // Read directory entries using safe traversal let entries = match dir_fd.read_dir() { @@ -388,6 +421,15 @@ pub fn safe_remove_dir_recursive_impl( continue; } + // --preserve-root=all compares against the immediate parent rather + // than the tree root, so a mount nested anywhere in the tree is + // caught even when --one-file-system is not in effect. + if options.preserve_root_all && entry_stat.st_dev as u64 != parent_dev { + show_preserve_root_all_skip(&entry_path); + error = true; + continue; + } + // Ask user if they want to descend into this directory if options.interactive == InteractiveMode::Always && !is_dir_empty(&entry_path) @@ -420,8 +462,13 @@ pub fn safe_remove_dir_recursive_impl( } }; - let child_error = - safe_remove_dir_recursive_impl(&entry_path, &child_dir_fd, options, root_dev); + let child_error = safe_remove_dir_recursive_impl( + &entry_path, + &child_dir_fd, + options, + root_dev, + entry_stat.st_dev as u64, + ); error |= child_error; // Ask user permission if needed for this subdirectory @@ -453,6 +500,7 @@ pub fn safe_remove_dir_recursive_impl( _dir_fd: &DirFd, _options: &Options, _root_dev: u64, + _parent_dev: u64, ) -> bool { // safe_traversal stat_at is not supported on Redox // This shouldn't be called on Redox, but provide a stub for compilation diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index 9077e0e85db..39bd99ba311 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -155,6 +155,8 @@ pub struct Options { pub one_fs: bool, /// `--preserve-root`/`--no-preserve-root` pub preserve_root: bool, + /// `--preserve-root=all` + pub preserve_root_all: bool, /// `-r`, `--recursive` pub recursive: bool, /// `-d`, `--dir` @@ -176,6 +178,7 @@ impl Default for Options { interactive: InteractiveMode::PromptProtected, one_fs: false, preserve_root: true, + preserve_root_all: false, recursive: false, dir: false, verbose: false, @@ -232,6 +235,9 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { }; let preserve_root = !matches.get_flag(OPT_NO_PRESERVE_ROOT); + let preserve_root_all = matches + .get_one::(OPT_PRESERVE_ROOT) + .is_some_and(|value| value == "all"); let recursive = matches.get_flag(OPT_RECURSIVE); let options = Options { @@ -251,6 +257,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { }, one_fs: matches.get_flag(OPT_ONE_FILE_SYSTEM), preserve_root, + preserve_root_all, recursive, dir: matches.get_flag(OPT_DIR), verbose: matches.get_flag(OPT_VERBOSE), @@ -388,7 +395,12 @@ pub fn uu_app() -> Command { Arg::new(OPT_PRESERVE_ROOT) .long(OPT_PRESERVE_ROOT) .help(translate!("rm-help-preserve-root")) - .action(ArgAction::SetTrue), + // `--preserve-root` alone protects only '/'; `--preserve-root=all` + // additionally refuses to cross into another file system. + .num_args(0..=1) + .require_equals(true) + .value_name("all") + .value_parser(["all"]), ) .arg( Arg::new(OPT_RECURSIVE) diff --git a/tests/by-util/test_rm.rs b/tests/by-util/test_rm.rs index a3e39e9cb67..e43cfcbb3b2 100644 --- a/tests/by-util/test_rm.rs +++ b/tests/by-util/test_rm.rs @@ -219,6 +219,39 @@ fn test_one_file_system_same_device() { assert!(!at.dir_exists(dir)); } +#[test] +fn test_preserve_root_all_same_device() { + // Refusing to cross a device boundary needs a mount point (root + // privileges); without one, --preserve-root=all must remove the tree as + // usual since it stays on a single device. + let (at, mut ucmd) = at_and_ucmd!(); + let dir = "test_rm_preserve_root_all_dir"; + let subdir = format!("{dir}/subdir"); + let file = format!("{subdir}/file"); + + at.mkdir(dir); + at.mkdir(&subdir); + at.touch(&file); + + ucmd.arg("--preserve-root=all") + .arg("-rf") + .arg(dir) + .succeeds() + .no_stderr(); + + assert!(!at.dir_exists(dir)); +} + +#[test] +fn test_preserve_root_rejects_unknown_value() { + new_ucmd!() + .arg("--preserve-root=bogus") + .arg("-rf") + .arg("anything") + .fails() + .stderr_contains("invalid value 'bogus'"); +} + #[test] fn test_recursive_multiple() { let (at, mut ucmd) = at_and_ucmd!(); From 3471175298990d88b7dade993b1237dfaec04ae1 Mon Sep 17 00:00:00 2001 From: Jason Duffy Date: Sat, 13 Jun 2026 11:02:50 -0400 Subject: [PATCH 3/3] rm: silence unnecessary_cast warning where dev_t is u64 clippy's unnecessary_cast fires on `st_dev as u64` on platforms where libc::dev_t is already u64 (Linux), but the cast is required where it is narrower (i32 on macOS). Compute the device once with the same #[allow(clippy::unnecessary_cast)] the rest of the tree uses for st_dev, which also removes the repeated cast at the comparison sites. --- src/uu/rm/src/platform/unix.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/uu/rm/src/platform/unix.rs b/src/uu/rm/src/platform/unix.rs index 37e07f5fb41..eb344bc4a99 100644 --- a/src/uu/rm/src/platform/unix.rs +++ b/src/uu/rm/src/platform/unix.rs @@ -412,7 +412,11 @@ pub fn safe_remove_dir_recursive_impl( let is_dir = ((entry_stat.st_mode as libc::mode_t) & libc::S_IFMT) == libc::S_IFDIR; if is_dir { - if options.one_fs && entry_stat.st_dev as u64 != root_dev { + // st_dev's type varies by platform (i32 on macOS, u64 on Linux). + #[allow(clippy::unnecessary_cast)] + let entry_dev = entry_stat.st_dev as u64; + + if options.one_fs && entry_dev != root_dev { show_error!( "{}", translate!("rm-error-skipping-different-device", "file" => entry_path.quote()) @@ -424,7 +428,7 @@ pub fn safe_remove_dir_recursive_impl( // --preserve-root=all compares against the immediate parent rather // than the tree root, so a mount nested anywhere in the tree is // caught even when --one-file-system is not in effect. - if options.preserve_root_all && entry_stat.st_dev as u64 != parent_dev { + if options.preserve_root_all && entry_dev != parent_dev { show_preserve_root_all_skip(&entry_path); error = true; continue; @@ -467,7 +471,7 @@ pub fn safe_remove_dir_recursive_impl( &child_dir_fd, options, root_dev, - entry_stat.st_dev as u64, + entry_dev, ); error |= child_error;