rm: fix --preserve-root detection for nested symlinks with trailing slash

Author: sylvestre

src/uu/rm/locales/en-US.ftl

@@ -40,6 +40,7 @@ rm-error-cannot-remove-no-such-file = cannot remove {$file}: No such file or dir
 rm-error-cannot-remove-permission-denied = cannot remove {$file}: Permission denied
 rm-error-cannot-remove-is-directory = cannot remove {$file}: Is a directory
 rm-error-dangerous-recursive-operation = it is dangerous to operate recursively on '/'
+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-cannot-remove = cannot remove {$file}

src/uu/rm/locales/fr-FR.ftl

@@ -40,6 +40,7 @@ rm-error-cannot-remove-no-such-file = impossible de supprimer {$file} : Aucun fi
 rm-error-cannot-remove-permission-denied = impossible de supprimer {$file} : Permission refusée
 rm-error-cannot-remove-is-directory = impossible de supprimer {$file} : C'est un répertoire
 rm-error-dangerous-recursive-operation = il est dangereux d'opérer récursivement sur '/'
+rm-error-dangerous-recursive-operation-same-as-root = il est dangereux d'opérer récursivement sur '{$path}' (identique à '/')
 rm-error-use-no-preserve-root = utilisez --no-preserve-root pour outrepasser cette protection
 rm-error-refusing-to-remove-directory = refus de supprimer le répertoire '.' ou '..' : ignorer {$path}
 rm-error-cannot-remove = impossible de supprimer {$file}

src/uu/rm/src/rm.rs

@@ -15,6 +15,8 @@ use std::ops::BitOr;
 #[cfg(unix)]
 use std::os::unix::ffi::OsStrExt;
 #[cfg(unix)]
+use std::os::unix::fs::MetadataExt;
+#[cfg(unix)]
 use std::os::unix::fs::PermissionsExt;
 use std::path::MAIN_SEPARATOR;
 use std::path::{Path, PathBuf};
@@ -29,6 +31,32 @@ mod platform;
 #[cfg(all(unix, not(target_os = "redox")))]
 use platform::{safe_remove_dir_recursive, safe_remove_empty_dir, safe_remove_file};
 
+/// Cached device and inode numbers for the root directory.
+/// Used for --preserve-root to detect when a path resolves to "/".
+#[cfg(unix)]
+#[derive(Debug, Clone, Copy)]
+pub struct RootDevIno {
+    pub dev: u64,
+    pub ino: u64,
+}
+
+#[cfg(unix)]
+impl RootDevIno {
+    /// Get the device and inode numbers for "/".
+    /// Returns None if lstat("/") fails.
+    pub fn new() -> Option<Self> {
+        fs::symlink_metadata("/").ok().map(|meta| Self {
+            dev: meta.dev(),
+            ino: meta.ino(),
+        })
+    }
+
+    /// Check if the given metadata matches the root device/inode.
+    pub fn is_root(&self, meta: &Metadata) -> bool {
+        meta.dev() == self.dev && meta.ino() == self.ino
+    }
+}
+
 #[derive(Debug, Error)]
 enum RmError {
     #[error("{}", translate!("rm-error-missing-operand", "util_name" => uucore::execution_phrase()))]
@@ -41,6 +69,9 @@ enum RmError {
     CannotRemoveIsDirectory(OsString),
     #[error("{}", translate!("rm-error-dangerous-recursive-operation"))]
     DangerousRecursiveOperation,
+    #[cfg(unix)]
+    #[error("{}", translate!("rm-error-dangerous-recursive-operation-same-as-root", "path" => _0.to_string_lossy()))]
+    DangerousRecursiveOperationSameAsRoot(OsString),
     #[error("{}", translate!("rm-error-use-no-preserve-root"))]
     UseNoPreserveRoot,
     #[error("{}", translate!("rm-error-refusing-to-remove-directory", "path" => _0.quote()))]
@@ -155,6 +186,10 @@ pub struct Options {
     pub one_fs: bool,
     /// `--preserve-root`/`--no-preserve-root`
     pub preserve_root: bool,
+    /// Cached device/inode for "/" when preserve_root is enabled.
+    /// Used to detect symlinks or paths that resolve to root.
+    #[cfg(unix)]
+    pub root_dev_ino: Option<RootDevIno>,
     /// `-r`, `--recursive`
     pub recursive: bool,
     /// `-d`, `--dir`
@@ -176,6 +211,8 @@ impl Default for Options {
             interactive: InteractiveMode::PromptProtected,
             one_fs: false,
             preserve_root: true,
+            #[cfg(unix)]
+            root_dev_ino: None,
             recursive: false,
             dir: false,
             verbose: false,
@@ -229,6 +266,19 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
             })
     };
 
+    let preserve_root = !matches.get_flag(OPT_NO_PRESERVE_ROOT);
+    let recursive = matches.get_flag(OPT_RECURSIVE);
+
+    // Cache the device/inode of "/" at startup when preserve_root is enabled
+    // and we're doing recursive operations. This allows us to detect symlinks
+    // or paths that resolve to root by comparing device/inode numbers.
+    #[cfg(unix)]
+    let root_dev_ino = if preserve_root && recursive {
+        RootDevIno::new()
+    } else {
+        None
+    };
+
     let options = Options {
         force: force_flag,
         interactive: {
@@ -245,8 +295,10 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
             }
         },
         one_fs: matches.get_flag(OPT_ONE_FILE_SYSTEM),
-        preserve_root: !matches.get_flag(OPT_NO_PRESERVE_ROOT),
-        recursive: matches.get_flag(OPT_RECURSIVE),
+        preserve_root,
+        #[cfg(unix)]
+        root_dev_ino,
+        recursive,
         dir: matches.get_flag(OPT_DIR),
         verbose: matches.get_flag(OPT_VERBOSE),
         progress: matches.get_flag(OPT_PROGRESS),
@@ -487,6 +539,25 @@ pub fn remove(files: &[&OsStr], options: &Options) -> bool {
     for filename in files {
         let file = Path::new(filename);
 
+        // Check if the path (potentially with trailing slash) resolves to root
+        // This needs to happen before symlink_metadata to catch cases like "rootlink/"
+        // where rootlink is a symlink to root.
+        #[cfg(unix)]
+        {
+            // When a path has a trailing slash and points to a symlink to a directory,
+            // we should check if it resolves to root before processing.
+            if uucore::fs::path_ends_with_terminator(file)
+                && options.recursive
+                && options.preserve_root
+            {
+                if is_root_path(file, options) {
+                    show_preserve_root_error(file, options);
+                    had_err = true;
+                    continue;
+                }
+            }
+        }
+
         had_err = match file.symlink_metadata() {
             Ok(metadata) => {
                 // Create progress bar on first successful file metadata read
@@ -673,6 +744,70 @@ fn remove_dir_recursive(
     }
 }
 
+/// Check if a path resolves to the root directory by comparing device/inode.
+/// Returns true if the path is root, false otherwise.
+/// On non-Unix systems, falls back to path-based check only.
+#[cfg(unix)]
+fn is_root_path(path: &Path, options: &Options) -> bool {
+    // First check the simple path-based case (e.g., "/")
+    let path_looks_like_root = path.has_root() && path.parent().is_none();
+
+    // If preserve_root is enabled and we have cached root dev/ino,
+    // also check if the path resolves to root via symlink or mount
+    if options.preserve_root {
+        if let Some(ref root_dev_ino) = options.root_dev_ino {
+            // Use fs::metadata to get the target's dev/ino after following ALL symlinks
+            // This should handle nested symlinks automatically
+            if let Ok(metadata) = fs::metadata(path) {
+                if root_dev_ino.is_root(&metadata) {
+                    return true;
+                }
+            }
+
+            // Fallback: canonicalize the path and check if it resolves to "/"
+            // This handles cases where metadata retrieval fails but the path
+            // still resolves to root (e.g., on MacOS in some test scenarios)
+            if let Ok(canonical_path) = path.canonicalize() {
+                if canonical_path.has_root() && canonical_path.parent().is_none() {
+                    return true;
+                }
+            }
+        }
+    }
+
+    path_looks_like_root
+}
+
+#[cfg(not(unix))]
+fn is_root_path(path: &Path, _options: &Options) -> bool {
+    path.has_root() && path.parent().is_none()
+}
+
+/// Show appropriate error message for attempting to remove root.
+/// Differentiates between literal "/" and paths that resolve to root (e.g., symlinks).
+#[cfg(unix)]
+fn show_preserve_root_error(path: &Path, _options: &Options) {
+    let path_looks_like_root = path.has_root() && path.parent().is_none();
+
+    if path_looks_like_root {
+        // Path is literally "/"
+        show_error!("{}", RmError::DangerousRecursiveOperation);
+    } else {
+        // Path resolves to root but isn't literally "/" (e.g., symlink to /)
+        show_error!(
+            "{}",
+            RmError::DangerousRecursiveOperationSameAsRoot(path.as_os_str().to_os_string())
+        );
+    }
+    show_error!("{}", RmError::UseNoPreserveRoot);
+}
+
+#[cfg(not(unix))]
+fn show_preserve_root_error(_path: &Path, _options: &Options) {
+    show_error!("{}", RmError::DangerousRecursiveOperation);
+    show_error!("{}", RmError::UseNoPreserveRoot);
+}
+
 fn handle_dir(path: &Path, options: &Options, progress_bar: Option<&ProgressBar>) -> bool {
     let mut had_err = false;
 
@@ -685,14 +820,13 @@ fn handle_dir(path: &Path, options: &Options, progress_bar: Option<&ProgressBar>
         return true;
     }
 
-    let is_root = path.has_root() && path.parent().is_none();
+    let is_root = is_root_path(path, options);
     if options.recursive && (!is_root || !options.preserve_root) {
         had_err = remove_dir_recursive(path, options, progress_bar);
     } else if options.dir && (!is_root || !options.preserve_root) {
         had_err = remove_dir(path, options, progress_bar).bitor(had_err);
     } else if options.recursive {
-        show_error!("{}", RmError::DangerousRecursiveOperation);
-        show_error!("{}", RmError::UseNoPreserveRoot);
+        show_preserve_root_error(path, options);
         had_err = true;
     } else {
         show_error!(

tests/by-util/test_rm.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 rootlink
 #![allow(clippy::stable_sort_primitive)]
 
 use std::process::Stdio;
@@ -1290,3 +1291,73 @@ fn test_symlink_to_readonly_no_prompt() {
 
     assert!(!at.symlink_exists("bar"));
 }
+
+/// Test that --preserve-root properly detects symlinks pointing to root.
+#[cfg(unix)]
+#[test]
+fn test_preserve_root_symlink_to_root() {
+    let (at, mut ucmd) = at_and_ucmd!();
+
+    // Create a symlink pointing to the root directory
+    at.symlink_dir("/", "rootlink");
+
+    // Attempting to recursively delete through this symlink should fail
+    // because it resolves to the same device/inode as "/"
+    ucmd.arg("-rf")
+        .arg("--preserve-root")
+        .arg("rootlink/")
+        .fails()
+        .stderr_contains("it is dangerous to operate recursively on")
+        .stderr_contains("(same as '/')");
+
+    // The symlink itself should still exist (we didn't delete it)
+    assert!(at.symlink_exists("rootlink"));
+}
+
+/// Test that --preserve-root properly detects nested symlinks pointing to root.
+#[cfg(unix)]
+#[test]
+fn test_preserve_root_nested_symlink_to_root() {
+    let (at, mut ucmd) = at_and_ucmd!();
+
+    // Create a symlink pointing to the root directory
+    at.symlink_dir("/", "rootlink");
+    // Create another symlink pointing to the first symlink
+    at.symlink_dir("rootlink", "rootlink2");
+
+    // Attempting to recursively delete through nested symlinks should also fail
+    ucmd.arg("-rf")
+        .arg("--preserve-root")
+        .arg("rootlink2/")
+        .fails()
+        .stderr_contains("it is dangerous to operate recursively on")
+        .stderr_contains("(same as '/')");
+}
+
+/// Test that removing the symlink itself (not the target) still works.
+#[cfg(unix)]
+#[test]
+fn test_preserve_root_symlink_removal_without_trailing_slash() {
+    let (at, mut ucmd) = at_and_ucmd!();
+
+    // Create a symlink pointing to the root directory
+    at.symlink_dir("/", "rootlink");
+
+    // Removing the symlink itself (without trailing slash) should succeed
+    // because we're removing the link, not traversing through it
+    ucmd.arg("--preserve-root").arg("rootlink").succeeds();
+
+    assert!(!at.symlink_exists("rootlink"));
+}
+
+/// Test that literal "/" is still properly protected.
+#[test]
+fn test_preserve_root_literal_root() {
+    new_ucmd!()
+        .arg("-rf")
+        .arg("--preserve-root")
+        .arg("/")
+        .fails()
+        .stderr_contains("it is dangerous to operate recursively on '/'")
+        .stderr_contains("use --no-preserve-root to override this failsafe");
+}