diff --git a/src/uu/rm/locales/en-US.ftl b/src/uu/rm/locales/en-US.ftl index 8f24f935943..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 @@ -45,6 +45,8 @@ 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-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 c7ef3f581d9..eb344bc4a99 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; @@ -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, @@ -271,16 +294,26 @@ 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); } }; + // 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, @@ -301,7 +334,8 @@ pub fn safe_remove_dir_recursive( } }; - let error = safe_remove_dir_recursive_impl(path, &dir_fd, options); + // 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 { @@ -338,7 +372,13 @@ 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, + parent_dev: u64, +) -> bool { // Read directory entries using safe traversal let entries = match dir_fd.read_dir() { Ok(entries) => entries, @@ -372,6 +412,28 @@ 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 { + // 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()) + ); + error = true; + 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_dev != 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) @@ -404,7 +466,13 @@ 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, + entry_dev, + ); error |= child_error; // Ask user permission if needed for this subdirectory @@ -431,7 +499,13 @@ 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, + _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 true // Return error diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index 0f25b517579..39bd99ba311 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -151,11 +151,12 @@ 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` pub preserve_root: bool, + /// `--preserve-root=all` + pub preserve_root_all: bool, /// `-r`, `--recursive` pub recursive: bool, /// `-d`, `--dir` @@ -177,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, @@ -233,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 { @@ -252,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), @@ -389,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) @@ -503,7 +514,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..e43cfcbb3b2 100644 --- a/tests/by-util/test_rm.rs +++ b/tests/by-util/test_rm.rs @@ -195,6 +195,63 @@ 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_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!();