Skip to content

Commit c8f5724

Browse files
committed
rm --preserve-root should work on symlink too
Closes: #9705
1 parent 45f81bb commit c8f5724

File tree

4 files changed

+183
-5
lines changed

4 files changed

+183
-5
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ rm-error-cannot-remove-no-such-file = cannot remove {$file}: No such file or dir
4040
rm-error-cannot-remove-permission-denied = cannot remove {$file}: Permission denied
4141
rm-error-cannot-remove-is-directory = cannot remove {$file}: Is a directory
4242
rm-error-dangerous-recursive-operation = it is dangerous to operate recursively on '/'
43+
rm-error-dangerous-recursive-operation-same-as-root = it is dangerous to operate recursively on '{$path}' (same as '/')
4344
rm-error-use-no-preserve-root = use --no-preserve-root to override this failsafe
4445
rm-error-refusing-to-remove-directory = refusing to remove '.' or '..' directory: skipping '{$path}'
4546
rm-error-cannot-remove = cannot remove {$file}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ rm-error-cannot-remove-no-such-file = impossible de supprimer {$file} : Aucun fi
4040
rm-error-cannot-remove-permission-denied = impossible de supprimer {$file} : Permission refusée
4141
rm-error-cannot-remove-is-directory = impossible de supprimer {$file} : C'est un répertoire
4242
rm-error-dangerous-recursive-operation = il est dangereux d'opérer récursivement sur '/'
43+
rm-error-dangerous-recursive-operation-same-as-root = il est dangereux d'opérer récursivement sur '{$path}' (identique à '/')
4344
rm-error-use-no-preserve-root = utilisez --no-preserve-root pour outrepasser cette protection
4445
rm-error-refusing-to-remove-directory = refus de supprimer le répertoire '.' ou '..' : ignorer '{$path}'
4546
rm-error-cannot-remove = impossible de supprimer {$file}

src/uu/rm/src/rm.rs

Lines changed: 111 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ use std::ops::BitOr;
1515
#[cfg(unix)]
1616
use std::os::unix::ffi::OsStrExt;
1717
#[cfg(unix)]
18+
use std::os::unix::fs::MetadataExt;
19+
#[cfg(unix)]
1820
use std::os::unix::fs::PermissionsExt;
1921
use std::path::MAIN_SEPARATOR;
2022
use std::path::{Path, PathBuf};
@@ -29,6 +31,32 @@ mod platform;
2931
#[cfg(target_os = "linux")]
3032
use platform::{safe_remove_dir_recursive, safe_remove_empty_dir, safe_remove_file};
3133

34+
/// Cached device and inode numbers for the root directory.
35+
/// Used for --preserve-root to detect when a path resolves to "/".
36+
#[cfg(unix)]
37+
#[derive(Debug, Clone, Copy)]
38+
pub struct RootDevIno {
39+
pub dev: u64,
40+
pub ino: u64,
41+
}
42+
43+
#[cfg(unix)]
44+
impl RootDevIno {
45+
/// Get the device and inode numbers for "/".
46+
/// Returns None if lstat("/") fails.
47+
pub fn new() -> Option<Self> {
48+
fs::symlink_metadata("/").ok().map(|meta| Self {
49+
dev: meta.dev(),
50+
ino: meta.ino(),
51+
})
52+
}
53+
54+
/// Check if the given metadata matches the root device/inode.
55+
pub fn is_root(&self, meta: &Metadata) -> bool {
56+
meta.dev() == self.dev && meta.ino() == self.ino
57+
}
58+
}
59+
3260
#[derive(Debug, Error)]
3361
enum RmError {
3462
#[error("{}", translate!("rm-error-missing-operand", "util_name" => uucore::execution_phrase()))]
@@ -41,6 +69,8 @@ enum RmError {
4169
CannotRemoveIsDirectory(OsString),
4270
#[error("{}", translate!("rm-error-dangerous-recursive-operation"))]
4371
DangerousRecursiveOperation,
72+
#[error("{}", translate!("rm-error-dangerous-recursive-operation-same-as-root", "path" => _0.to_string_lossy()))]
73+
DangerousRecursiveOperationSameAsRoot(OsString),
4474
#[error("{}", translate!("rm-error-use-no-preserve-root"))]
4575
UseNoPreserveRoot,
4676
#[error("{}", translate!("rm-error-refusing-to-remove-directory", "path" => _0.to_string_lossy()))]
@@ -153,6 +183,10 @@ pub struct Options {
153183
pub one_fs: bool,
154184
/// `--preserve-root`/`--no-preserve-root`
155185
pub preserve_root: bool,
186+
/// Cached device/inode for "/" when preserve_root is enabled.
187+
/// Used to detect symlinks or paths that resolve to root.
188+
#[cfg(unix)]
189+
pub root_dev_ino: Option<RootDevIno>,
156190
/// `-r`, `--recursive`
157191
pub recursive: bool,
158192
/// `-d`, `--dir`
@@ -174,6 +208,8 @@ impl Default for Options {
174208
interactive: InteractiveMode::PromptProtected,
175209
one_fs: false,
176210
preserve_root: true,
211+
#[cfg(unix)]
212+
root_dev_ino: None,
177213
recursive: false,
178214
dir: false,
179215
verbose: false,
@@ -226,6 +262,19 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
226262
})
227263
};
228264

265+
let preserve_root = !matches.get_flag(OPT_NO_PRESERVE_ROOT);
266+
let recursive = matches.get_flag(OPT_RECURSIVE);
267+
268+
// Cache the device/inode of "/" at startup when preserve_root is enabled
269+
// and we're doing recursive operations. This allows us to detect symlinks
270+
// or paths that resolve to root by comparing device/inode numbers.
271+
#[cfg(unix)]
272+
let root_dev_ino = if preserve_root && recursive {
273+
RootDevIno::new()
274+
} else {
275+
None
276+
};
277+
229278
let options = Options {
230279
force: force_flag,
231280
interactive: {
@@ -242,8 +291,10 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
242291
}
243292
},
244293
one_fs: matches.get_flag(OPT_ONE_FILE_SYSTEM),
245-
preserve_root: !matches.get_flag(OPT_NO_PRESERVE_ROOT),
246-
recursive: matches.get_flag(OPT_RECURSIVE),
294+
preserve_root,
295+
#[cfg(unix)]
296+
root_dev_ino,
297+
recursive,
247298
dir: matches.get_flag(OPT_DIR),
248299
verbose: matches.get_flag(OPT_VERBOSE),
249300
progress: matches.get_flag(OPT_PROGRESS),
@@ -684,6 +735,62 @@ fn remove_dir_recursive(
684735
}
685736
}
686737

738+
/// Check if a path resolves to the root directory by comparing device/inode.
739+
/// Returns true if the path is root, false otherwise.
740+
/// On non-Unix systems, falls back to path-based check only.
741+
#[cfg(unix)]
742+
fn is_root_path(path: &Path, options: &Options) -> bool {
743+
// First check the simple path-based case (e.g., "/")
744+
let path_looks_like_root = path.has_root() && path.parent().is_none();
745+
746+
// If preserve_root is enabled and we have cached root dev/ino,
747+
// also check if the path resolves to root via symlink or mount
748+
if options.preserve_root {
749+
if let Some(ref root_dev_ino) = options.root_dev_ino {
750+
// Use symlink_metadata to get the actual target's dev/ino
751+
// after following symlinks (we need to follow the symlink to see
752+
// where it points)
753+
if let Ok(metadata) = fs::metadata(path) {
754+
if root_dev_ino.is_root(&metadata) {
755+
return true;
756+
}
757+
}
758+
}
759+
}
760+
761+
path_looks_like_root
762+
}
763+
764+
#[cfg(not(unix))]
765+
fn is_root_path(path: &Path, _options: &Options) -> bool {
766+
path.has_root() && path.parent().is_none()
767+
}
768+
769+
/// Show appropriate error message for attempting to remove root.
770+
/// Differentiates between literal "/" and paths that resolve to root (e.g., symlinks).
771+
#[cfg(unix)]
772+
fn show_preserve_root_error(path: &Path, _options: &Options) {
773+
let path_looks_like_root = path.has_root() && path.parent().is_none();
774+
775+
if path_looks_like_root {
776+
// Path is literally "/"
777+
show_error!("{}", RmError::DangerousRecursiveOperation);
778+
} else {
779+
// Path resolves to root but isn't literally "/" (e.g., symlink to /)
780+
show_error!(
781+
"{}",
782+
RmError::DangerousRecursiveOperationSameAsRoot(path.as_os_str().to_os_string())
783+
);
784+
}
785+
show_error!("{}", RmError::UseNoPreserveRoot);
786+
}
787+
788+
#[cfg(not(unix))]
789+
fn show_preserve_root_error(_path: &Path, _options: &Options) {
790+
show_error!("{}", RmError::DangerousRecursiveOperation);
791+
show_error!("{}", RmError::UseNoPreserveRoot);
792+
}
793+
687794
fn handle_dir(path: &Path, options: &Options, progress_bar: Option<&ProgressBar>) -> bool {
688795
let mut had_err = false;
689796

@@ -696,14 +803,13 @@ fn handle_dir(path: &Path, options: &Options, progress_bar: Option<&ProgressBar>
696803
return true;
697804
}
698805

699-
let is_root = path.has_root() && path.parent().is_none();
806+
let is_root = is_root_path(path, options);
700807
if options.recursive && (!is_root || !options.preserve_root) {
701808
had_err = remove_dir_recursive(path, options, progress_bar);
702809
} else if options.dir && (!is_root || !options.preserve_root) {
703810
had_err = remove_dir(path, options, progress_bar).bitor(had_err);
704811
} else if options.recursive {
705-
show_error!("{}", RmError::DangerousRecursiveOperation);
706-
show_error!("{}", RmError::UseNoPreserveRoot);
812+
show_preserve_root_error(path, options);
707813
had_err = true;
708814
} else {
709815
show_error!(

tests/by-util/test_rm.rs

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1217,3 +1217,73 @@ fn test_progress_no_output_on_error() {
12171217
.stderr_contains("cannot remove")
12181218
.stderr_contains("No such file or directory");
12191219
}
1220+
1221+
/// Test that --preserve-root properly detects symlinks pointing to root.
1222+
#[cfg(unix)]
1223+
#[test]
1224+
fn test_preserve_root_symlink_to_root() {
1225+
let (at, mut ucmd) = at_and_ucmd!();
1226+
1227+
// Create a symlink pointing to the root directory
1228+
at.symlink_dir("/", "rootlink");
1229+
1230+
// Attempting to recursively delete through this symlink should fail
1231+
// because it resolves to the same device/inode as "/"
1232+
ucmd.arg("-rf")
1233+
.arg("--preserve-root")
1234+
.arg("rootlink/")
1235+
.fails()
1236+
.stderr_contains("it is dangerous to operate recursively on")
1237+
.stderr_contains("(same as '/')");
1238+
1239+
// The symlink itself should still exist (we didn't delete it)
1240+
assert!(at.symlink_exists("rootlink"));
1241+
}
1242+
1243+
/// Test that --preserve-root properly detects nested symlinks pointing to root.
1244+
#[cfg(unix)]
1245+
#[test]
1246+
fn test_preserve_root_nested_symlink_to_root() {
1247+
let (at, mut ucmd) = at_and_ucmd!();
1248+
1249+
// Create a symlink pointing to the root directory
1250+
at.symlink_dir("/", "rootlink");
1251+
// Create another symlink pointing to the first symlink
1252+
at.symlink_dir("rootlink", "rootlink2");
1253+
1254+
// Attempting to recursively delete through nested symlinks should also fail
1255+
ucmd.arg("-rf")
1256+
.arg("--preserve-root")
1257+
.arg("rootlink2/")
1258+
.fails()
1259+
.stderr_contains("it is dangerous to operate recursively on")
1260+
.stderr_contains("(same as '/')");
1261+
}
1262+
1263+
/// Test that removing the symlink itself (not the target) still works.
1264+
#[cfg(unix)]
1265+
#[test]
1266+
fn test_preserve_root_symlink_removal_without_trailing_slash() {
1267+
let (at, mut ucmd) = at_and_ucmd!();
1268+
1269+
// Create a symlink pointing to the root directory
1270+
at.symlink_dir("/", "rootlink");
1271+
1272+
// Removing the symlink itself (without trailing slash) should succeed
1273+
// because we're removing the link, not traversing through it
1274+
ucmd.arg("--preserve-root").arg("rootlink").succeeds();
1275+
1276+
assert!(!at.symlink_exists("rootlink"));
1277+
}
1278+
1279+
/// Test that literal "/" is still properly protected.
1280+
#[test]
1281+
fn test_preserve_root_literal_root() {
1282+
new_ucmd!()
1283+
.arg("-rf")
1284+
.arg("--preserve-root")
1285+
.arg("/")
1286+
.fails()
1287+
.stderr_contains("it is dangerous to operate recursively on '/'")
1288+
.stderr_contains("use --no-preserve-root to override this failsafe");
1289+
}

0 commit comments

Comments
 (0)