rm --preserve-root should work on symlink too

sylvestre · sylvestre · commit a94c6987a4d6 · 2025-12-18T22:40:45.000+01:00
Closes: #9705
diff --git a/src/uu/rm/locales/en-US.ftl b/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}
diff --git a/src/uu/rm/locales/fr-FR.ftl b/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}
diff --git a/src/uu/rm/src/rm.rs b/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(target_os = "linux")]
 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.to_string_lossy()))]
@@ -153,6 +184,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`
@@ -174,6 +209,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,
@@ -226,6 +263,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: {
@@ -242,8 +292,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),
@@ -684,6 +736,62 @@ 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 symlink_metadata to get the actual target's dev/ino
+            // after following symlinks (we need to follow the symlink to see
+            // where it points)
+            if let Ok(metadata) = fs::metadata(path) {
+                if root_dev_ino.is_root(&metadata) {
+                    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;
 
@@ -696,14 +804,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!(
diff --git a/tests/by-util/test_rm.rs b/tests/by-util/test_rm.rs
@@ -1217,3 +1217,73 @@ fn test_progress_no_output_on_error() {
         .stderr_contains("cannot remove")
         .stderr_contains("No such file or directory");
 }
+
+/// 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");
+}
