diff --git a/Cargo.lock b/Cargo.lock index b86229f32..2036089c8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3178,6 +3178,21 @@ dependencies = [ "version_check", ] +[[package]] +name = "proptest" +version = "1.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4b45fcc2344c680f5025fe57779faef368840d0bd1f42f216291f0dc4ace4744" +dependencies = [ + "bitflags 2.13.0", + "num-traits", + "rand 0.9.4", + "rand_chacha 0.9.0", + "rand_xorshift", + "regex-syntax", + "unarray", +] + [[package]] name = "pulldown-cmark" version = "0.12.2" @@ -3334,6 +3349,15 @@ dependencies = [ "getrandom 0.3.4", ] +[[package]] +name = "rand_xorshift" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "513962919efc330f829edb2535844d1b912b0fbe2ca165d613e4e8788bb05a5a" +dependencies = [ + "rand_core 0.9.5", +] + [[package]] name = "ratatui" version = "0.30.2" @@ -3672,6 +3696,7 @@ dependencies = [ "cc", "directories", "libc", + "proptest", "rand 0.9.4", "regex", "rsa", @@ -5219,6 +5244,12 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "unarray" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "eaea85b334db583fe3274d12b4cd1880032beab409c0d774be044d4480ab9a94" + [[package]] name = "unicase" version = "2.9.0" diff --git a/MANIFEST.md b/MANIFEST.md index 5e6c7ed20..1c9c7c242 100644 --- a/MANIFEST.md +++ b/MANIFEST.md @@ -353,6 +353,7 @@ repository. | proc-macro-crate | 3.5.0 | MIT OR Apache-2.0 | | proc-macro2 | 1.0.106 | MIT OR Apache-2.0 | | proc-macro2-diagnostics | 0.10.1 | MIT/Apache-2.0 | +| proptest | 1.11.0 | MIT OR Apache-2.0 | | pulldown-cmark | 0.12.2 | MIT | | quick-xml | 0.39.4 | MIT | | quinn | 0.11.11 | MIT OR Apache-2.0 | @@ -367,6 +368,7 @@ repository. | rand_chacha | 0.9.0 | MIT OR Apache-2.0 | | rand_core | 0.6.4 | MIT OR Apache-2.0 | | rand_core | 0.9.5 | MIT OR Apache-2.0 | +| rand_xorshift | 0.4.0 | MIT OR Apache-2.0 | | ratatui | 0.30.2 | MIT | | ratatui-core | 0.1.2 | MIT | | ratatui-crossterm | 0.1.2 | MIT | @@ -515,6 +517,7 @@ repository. | typenum | 1.20.1 | MIT OR Apache-2.0 | | ucd-trie | 0.1.7 | MIT OR Apache-2.0 | | uds_windows | 1.2.1 | MIT | +| unarray | 0.1.4 | MIT OR Apache-2.0 | | unicase | 2.9.0 | MIT OR Apache-2.0 | | unicode-ident | 1.0.24 | (MIT OR Apache-2.0) AND Unicode-3.0 | | unicode-linebreak | 0.1.5 | Apache-2.0 | diff --git a/apps/rocm/src/main.rs b/apps/rocm/src/main.rs index e600faf20..1dac9e994 100644 --- a/apps/rocm/src/main.rs +++ b/apps/rocm/src/main.rs @@ -33986,6 +33986,64 @@ ID_LIKE="suse opensuse" assert!(err.to_string().contains("protected system location")); } + /// The gate above is the last thing between a registry entry and + /// `fs::remove_dir_all`, and the path it is handed is text somebody else + /// wrote. One folder has many spellings, so the refusal has to survive all + /// of them — a trailing separator, a doubled one, a `.`, or a `..` that + /// walks back out of the user's home directory. Each of these names `/etc`. + #[test] + #[cfg(unix)] + fn ensure_runtime_install_root_rejects_every_spelling_of_a_protected_path() { + let home = rocm_core::runtime_home_dir().expect("a home directory"); + let home = home.display().to_string(); + let spellings = [ + "/etc/".to_owned(), + "//etc".to_owned(), + "/./etc".to_owned(), + "/etc/.".to_owned(), + format!("{home}/../../etc"), + ]; + + let accepted: Vec = spellings + .into_iter() + .filter(|text| ensure_runtime_install_root_is_safe_to_remove(Path::new(text)).is_ok()) + .collect(); + + assert!( + accepted.is_empty(), + "accepted as safe to recursively delete: {accepted:?}" + ); + } + + /// The same question — "is this a system location?" — also keeps the local + /// assistant from installing into one, and there the folder arrives as + /// free-form JSON from a model rather than from a file the user wrote. A + /// trailing separator is exactly the sort of thing generated text carries, + /// so the refusal has to survive every spelling here too. + #[test] + #[cfg(unix)] + fn the_assistant_cannot_name_a_system_install_folder_by_respelling_it() { + let home = rocm_core::runtime_home_dir().expect("a home directory"); + let home = home.display().to_string(); + let spellings = [ + "/usr/".to_owned(), + "/opt/".to_owned(), + "//usr".to_owned(), + "/./opt".to_owned(), + format!("{home}/../../usr"), + ]; + + let accepted: Vec = spellings + .into_iter() + .filter(|text| !chat_install_prefix_is_system(Path::new(text))) + .collect(); + + assert!( + accepted.is_empty(), + "accepted as a user install folder: {accepted:?}" + ); + } + #[test] fn plan_runtime_uninstall_does_not_mutate() -> Result<()> { let (root, paths) = test_paths("runtime-uninstall-plan-dry-run"); diff --git a/apps/rocmd/src/lib.rs b/apps/rocmd/src/lib.rs index 3762212e3..c65f057e4 100644 --- a/apps/rocmd/src/lib.rs +++ b/apps/rocmd/src/lib.rs @@ -6233,6 +6233,28 @@ mod tests { ); } + /// The test above only ever hands `system_prefix_requires_ack` an + /// already-canonical path, so it cannot catch the bug this crate's fix + /// addresses: a `..`-respelled prefix that escapes `$HOME` used to compare + /// equal to a path still inside it (`Path::ancestors()` treats `..` as an + /// ordinary component), so acknowledgement was never required. Drive the + /// same check with a prefix built by walking `..` out of the real home + /// directory, which is exactly the shape the original bug let through. + #[test] + #[cfg(unix)] + fn install_sdk_rejects_system_prefix_reached_by_escaping_home() { + let home = rocm_core::runtime_home_dir().expect("a home directory"); + let escaped_prefix = format!("{}/../../usr", home.display()); + + let arguments = + serde_json::Map::from_iter([("prefix".to_owned(), Value::String(escaped_prefix))]); + let error = build_install_sdk_args(&arguments, false).unwrap_err(); + assert!( + error.to_string().contains("allow_system_prefix=true"), + "{error:#}" + ); + } + /// The `install_sdk` MCP tool spawns `rocm` with null stdin, so a real /// install over an active default managed runtime would hit the approval /// gate's non-interactive refusal and bail asking for a flag no MCP caller diff --git a/crates/rocm-core/Cargo.toml b/crates/rocm-core/Cargo.toml index e96bc4af9..05969b4cc 100644 --- a/crates/rocm-core/Cargo.toml +++ b/crates/rocm-core/Cargo.toml @@ -27,5 +27,15 @@ sysinfo.workspace = true toml = "0.8" ureq = { version = "2.12", features = ["native-certs"] } +[dev-dependencies] +# Property-based tests for the path comparison that decides whether a folder +# may be recursively deleted. The contract — "a path that resolves inside a +# protected system root is never reported unprotected" — has to hold for EVERY +# spelling of that path, which is exactly the shape an example test misses: the +# pre-existing example used `/etc/rocm-cli-test-runtime`, the one shape that +# happened to work, while `/etc/` and `$HOME/../../etc` sailed through. +# Test-only, so it adds nothing to any shipped binary. +proptest = { version = "1", default-features = false, features = ["std"] } + [target.'cfg(target_os = "windows")'.dependencies] windows-sys = { version = "0.61", features = ["Win32_Foundation", "Win32_Security", "Win32_System_Registry", "Win32_System_SystemInformation", "Win32_System_Threading"] } diff --git a/crates/rocm-core/proptest-regressions/runtime.txt b/crates/rocm-core/proptest-regressions/runtime.txt new file mode 100644 index 000000000..0f1be8be5 --- /dev/null +++ b/crates/rocm-core/proptest-regressions/runtime.txt @@ -0,0 +1,11 @@ +# Seeds for failure cases proptest has generated in the past. It is +# automatically read and these particular cases re-run before any +# novel cases are generated. +# +# It is recommended to check this file in to source control so that +# everyone who runs the test benefits from these saved cases. +cc 44838f606145dd05b27407447aa76a8a775e5a0d7a547bd898e4fb6488871799 # shrinks to text = "/bin/" +cc 5dc93bbc0c3b8a6fda0a1f14eba8364422f99adb63c0a4c3df90a55c903fd631 # shrinks to text = "/bin/" +cc a25d13c5221f176eaf8f3b0647babe58fe2984d7f0d3190b1a1b1fdc2e74d273 # shrinks to path = "/opt/", base = "/opt" +cc aa4f3b9ad94cb072d17df2d8c448929748d0a76fad6f04a0a76463f97ada9d60 # shrinks to text = "/bin/..\\tmp/.." +cc 00244038f28f97b7497323fe552197d8efe1e5006c07a42a7e8bbd83ba7f956a # shrinks to text = "/bin/..\\tmp" diff --git a/crates/rocm-core/src/runtime.rs b/crates/rocm-core/src/runtime.rs index c7842e778..011a64cbd 100644 --- a/crates/rocm-core/src/runtime.rs +++ b/crates/rocm-core/src/runtime.rs @@ -485,16 +485,123 @@ pub fn runtime_drive_root_for_key(ch: char) -> Option { path.is_dir().then_some(path) } +/// Split a forward-slash path into the prefix that `..` must never climb past +/// and the components that follow it. +/// +/// An absolute path's root, a Windows drive, and a UNC share are all anchors: +/// `/..` is `/` on every POSIX system, and no amount of `..` leaves `C:\`. A +/// relative path has no anchor, so a leading `..` there is meaningful and is +/// kept. +fn split_runtime_path_anchor(value: &str, platform: RuntimePlatform) -> (&str, &str) { + // A drive letter and a UNC share are anchors only where they mean anything. + // On Linux `C:/x` is an ordinary relative name and `//etc` is just `/etc`, + // so reading either as a root would invent a path that is not there. + // + // The platform is a parameter rather than `runtime_is_windows()` so that the + // Windows side of a guard against recursive deletion can be tested from a + // Linux host, which is the only host this repository runs clippy and most + // of its unit tests on. + if platform.is_windows() { + if let Some(rest) = value.strip_prefix("//") { + // `//server/share/...`: the share itself is the anchor. + let share_end = rest + .match_indices('/') + .nth(1) + .map_or(rest.len(), |(index, _)| index); + let (anchor_tail, rest) = rest.split_at(share_end); + return (&value[..2 + anchor_tail.len()], rest); + } + let bytes = value.as_bytes(); + if bytes.len() >= 2 && bytes[0].is_ascii_alphabetic() && bytes[1] == b':' { + let drive_end = if bytes.len() > 2 && bytes[2] == b'/' { + 3 + } else { + 2 + }; + return value.split_at(drive_end); + } + } + if value.starts_with('/') { + return value.split_at(1); + } + ("", value) +} + +/// Collapse `.`, `..`, repeated separators and a trailing separator, so that +/// every spelling of one folder reduces to one string. +/// +/// Purely lexical, and deliberately so: this feeds the comparisons that decide +/// whether a folder may be recursively deleted, and those run against paths +/// that often do not exist yet (or exist only in a registry entry), where +/// `canonicalize` has nothing to resolve. +/// +/// This is NOT symlink-safe, and the gap is not merely theoretical. Because +/// [`runtime_install_root_is_protected`] exempts anything inside the user's +/// home before it consults the protected-root list, a symlink the user owns is +/// enough: with `$HOME/link -> /etc`, the path `$HOME/link/child` is lexically +/// inside home, so the guard reports it removable while the kernel lands in +/// `/etc`. Resolving lexically is strictly better than the raw-text comparison +/// it replaces — it closes every *spelling* of a protected path — but it does +/// not close that hole, and closing it needs a decision about canonicalizing +/// the part of the path that does exist, which is a separate change. +fn lexically_resolved_runtime_path_text(value: &str, platform: RuntimePlatform) -> String { + let (anchor, rest) = split_runtime_path_anchor(value, platform); + let anchored = !anchor.is_empty(); + let mut parts: Vec<&str> = Vec::new(); + for part in rest.split('/') { + match part { + "" | "." => {} + ".." => match parts.last() { + // A relative path keeps the `..` it cannot resolve: `../a` names + // a real place, just not one this function can name differently. + None if !anchored => parts.push(".."), + Some(&"..") => parts.push(".."), + // Anchored and already at the top: `/..` is `/`. + None => {} + Some(_) => { + parts.pop(); + } + }, + other => parts.push(other), + } + } + let joined = parts.join("/"); + if anchored { + format!("{}{joined}", anchor.trim_end_matches('/').to_owned() + "/") + } else if joined.is_empty() { + ".".to_owned() + } else { + joined + } +} + +/// Reduce a path to the single text that names its folder, whatever spelling it +/// arrived in. +fn comparable_runtime_path_text(path: &Path, platform: RuntimePlatform) -> String { + let text = normalize_runtime_path_text_for_platform(&path.display().to_string(), platform); + // Folding `\` to `/` is a WINDOWS rule and must stay gated on the platform. + // Off Windows a backslash is an ordinary filename byte, so a directory + // genuinely named `..\tmp` inside `/usr` would otherwise be rewritten to + // `/usr/../tmp` and the `..` resolution below would walk it straight out of + // the protected root — turning a guard into a bypass. Raw-text comparison + // tolerated the unconditional fold because equality never *removed* + // components; resolving `..` does. + let text = if platform.is_windows() { + text.replace('\\', "/") + } else { + text + }; + lexically_resolved_runtime_path_text(&text, platform) +} + pub fn runtime_paths_equivalent(left: &Path, right: &Path) -> bool { - let left = normalize_runtime_path_for_host(left) - .display() - .to_string() - .replace('\\', "/"); - let right = normalize_runtime_path_for_host(right) - .display() - .to_string() - .replace('\\', "/"); - if runtime_is_windows() { + runtime_paths_equivalent_on(left, right, RuntimePlatform::current()) +} + +fn runtime_paths_equivalent_on(left: &Path, right: &Path, platform: RuntimePlatform) -> bool { + let left = comparable_runtime_path_text(left, platform); + let right = comparable_runtime_path_text(right, platform); + if platform.is_windows() { left.eq_ignore_ascii_case(&right) } else { left == right @@ -502,14 +609,27 @@ pub fn runtime_paths_equivalent(left: &Path, right: &Path) -> bool { } pub fn runtime_path_is_same_or_inside(path: &Path, base: &Path) -> bool { - let path = normalize_runtime_path_for_host(path); - let base = normalize_runtime_path_for_host(base); - if runtime_paths_equivalent(&path, &base) { - return true; + runtime_path_is_same_or_inside_on(path, base, RuntimePlatform::current()) +} + +fn runtime_path_is_same_or_inside_on(path: &Path, base: &Path, platform: RuntimePlatform) -> bool { + // Compared after resolution rather than by walking `Path::ancestors`, which + // treats `..` as an ordinary component and so counts a path that climbs + // back OUT of `base` as still inside it. + let path = comparable_runtime_path_text(path, platform); + let base = comparable_runtime_path_text(base, platform); + let inside_prefix = format!("{}/", base.trim_end_matches('/')); + if path.len() <= inside_prefix.len() { + return runtime_paths_equivalent_on(Path::new(&path), Path::new(&base), platform); + } + // Compared as bytes: a path may hold any UTF-8, and slicing a `str` at a + // byte offset that lands mid-character panics. + let head = &path.as_bytes()[..inside_prefix.len()]; + if platform.is_windows() { + head.eq_ignore_ascii_case(inside_prefix.as_bytes()) + } else { + head == inside_prefix.as_bytes() } - path.ancestors() - .skip(1) - .any(|ancestor| runtime_paths_equivalent(ancestor, &base)) } const MANAGED_RUNTIME_FORMATS: [&str; 2] = ["wheel", "tarball"]; @@ -1278,4 +1398,415 @@ mod tests { assert_eq!(split_windows_path_list_text(&joined), entries.to_vec()); } + + /// The Windows side of the same comparisons, exercised from whatever host + /// runs the tests. `runtime_is_windows()` is decided at compile time, so + /// without a platform parameter this branch would be checked only by the + /// one CI lane that runs on Windows — and it is the branch that decides + /// whether `C:\Windows` may be recursively deleted. + #[test] + fn windows_containment_sees_through_spelling_and_case() { + let windows = RuntimePlatform::Windows; + let system_root = Path::new("C:/Windows"); + + for inside in [ + r"C:\Windows", + "C:/Windows/", + "c:/windows", + "C:/Windows/./System32", + "C:/Program Files/../Windows/System32", + "C:/Windows//System32", + ] { + assert!( + runtime_path_is_same_or_inside_on(Path::new(inside), system_root, windows), + "{inside} names C:/Windows or something under it" + ); + } + + for outside in [ + "C:/Users/dev/.rocm", + "C:/Windows/../Users/dev", + "C:/WindowsApps", + "D:/Windows", + ] { + assert!( + !runtime_path_is_same_or_inside_on(Path::new(outside), system_root, windows), + "{outside} is not inside C:/Windows" + ); + } + + // A drive is an anchor: `..` can never climb off it onto another one. + // (The resolver is handed forward slashes; the conversion happens in + // `comparable_runtime_path_text`, which the assertions above go through.) + assert_eq!( + lexically_resolved_runtime_path_text("C:/../../Windows", windows), + "C:/Windows" + ); + // A UNC share is an anchor too. + assert_eq!( + lexically_resolved_runtime_path_text("//server/share/../../rocm", windows), + "//server/share/rocm" + ); + } + + /// The same text means different things on the two platforms, so the + /// resolution must not borrow Windows' reading on Linux: `//etc` is `/etc` + /// there, and `C:/x` is an ordinary relative name, not a drive. + #[test] + fn linux_resolution_does_not_borrow_windows_anchors() { + let linux = RuntimePlatform::Linux; + + assert_eq!(lexically_resolved_runtime_path_text("//etc", linux), "/etc"); + assert_eq!( + lexically_resolved_runtime_path_text("/etc/./../etc/", linux), + "/etc" + ); + // `/..` is `/` on every POSIX system. + assert_eq!(lexically_resolved_runtime_path_text("/../..", linux), "/"); + // Relative: a leading `..` names a real place this cannot rename. + assert_eq!( + lexically_resolved_runtime_path_text("../a/../b", linux), + "../b" + ); + assert_eq!(lexically_resolved_runtime_path_text("a/..", linux), "."); + assert_eq!( + lexically_resolved_runtime_path_text("C:/x", linux), + "C:/x", + "a drive letter is not a root on Linux" + ); + } + + /// `windows_containment_sees_through_spelling_and_case` above exercises the + /// Windows path-resolution rules from a Linux host, but only through the + /// platform-parameterised helpers. The public gate itself, + /// [`runtime_install_root_is_protected`], still decides which protected-root + /// list to consult by reading [`runtime_is_windows()`] — a compile-time + /// answer — so its Windows arm is reachable only when this binary is + /// actually running on Windows. Not `#[cfg(unix)]`-gated, so it compiles + /// and runs on every lane: it follows the same `cfg!(windows)`-at-runtime + /// shape `ensure_runtime_install_root_rejects_protected_system_path` (in + /// `apps/rocm`) already uses to pick host-appropriate fixtures, so a lane + /// that is not Windows still exercises the gate end-to-end against the + /// Unix answer it already owns. + #[test] + fn the_public_gate_refuses_every_spelling_of_a_protected_root_on_its_own_host() { + let spellings: Vec = if cfg!(windows) { + vec![ + PathBuf::from("C:/Windows/"), // trailing separator + PathBuf::from("c:/windows"), // mixed case + PathBuf::from("C:/Windows//System32"), // doubled separator + PathBuf::from("C:/Program Files//"), // doubled separator + PathBuf::from("C:/PROGRAM FILES (X86)"), // mixed case + ] + } else { + vec![PathBuf::from("/etc/"), PathBuf::from("//etc")] + }; + + let accepted: Vec = spellings + .into_iter() + .filter(|path| !runtime_install_root_is_protected(path)) + .map(|path| path.display().to_string()) + .collect(); + + assert!(accepted.is_empty(), "accepted as removable: {accepted:?}"); + } + + // ── Properties: the recursive-delete guard ───────────────────── + // + // `runtime_install_root_is_protected` is the single source of truth for + // "may ROCm CLI `remove_dir_all` this folder?". Everything below states a + // contract it must satisfy for EVERY spelling of a path, because the whole + // point of the guard is that it is handed a path somebody (or something) + // else wrote down — a registry entry, an `--prefix` argument, a tool call + // from the local assistant. Such a path is text, and text has many + // spellings for one folder. + // + // Unix-only: the protected-root list the guard consults is the Unix one, + // and `runtime_is_windows()` is decided at compile time, so the Windows + // branch cannot be exercised from here. All pure and in-process. + #[cfg(unix)] + mod delete_guard_properties { + use super::*; + use proptest::prelude::*; + + /// The Unix roots `runtime_install_root_is_protected` refuses, restated + /// here so a property compares the guard against the policy rather than + /// against itself. + const PROTECTED_ROOTS: [&str; 13] = [ + "/bin", "/boot", "/dev", "/etc", "/lib", "/lib64", "/opt", "/proc", "/root", "/sbin", + "/sys", "/usr", "/var", + ]; + + /// Resolve `.`, `..`, repeated and trailing separators lexically — what + /// `realpath --no-symlinks`, every shell, and `Path::components` on + /// Windows all do, and what the kernel does for a path with no symlinks + /// in it. + /// + /// This is the reference the guard is measured against, and it is + /// derived from POSIX semantics rather than from the implementation on + /// purpose: an oracle restated from the code under test shares that + /// code's mistakes and proves nothing. Keep it that way — note it does + /// NOT treat `\` specially, which is exactly what catches an + /// unconditional backslash fold leaking into `..` resolution. + /// + /// It says nothing about symlinks. The guard is not symlink-safe (see + /// `lexically_resolved_runtime_path_text`); this reference only pins + /// that every *spelling* of one folder resolves alike. + /// + /// Absolute input only: every generator seed in this module + /// (`PROTECTED_ROOTS`, `prefix()`, `home_text()`) is already rooted at + /// `/`, so a relative path never reaches this oracle today. That is a + /// property of the generators, not of this function's logic, so it is + /// asserted rather than merely assumed — a generator change that starts + /// seeding relative text would otherwise desync the oracle from + /// `runtime_install_root_is_protected` (which does handle relative + /// paths) without a single property failing to say so. + fn lexically_resolved(path: &str) -> String { + assert!( + path.starts_with('/'), + "lexically_resolved is a POSIX-absolute-path oracle; got relative input {path:?}" + ); + let mut parts: Vec<&str> = Vec::new(); + for part in path.split('/') { + match part { + "" | "." => {} + // POSIX: `/..` is `/`, so popping an empty stack is a no-op. + ".." => { + parts.pop(); + } + other => parts.push(other), + } + } + format!("/{}", parts.join("/")) + } + + /// Does `path` really name `base` or something under it, once both are + /// resolved? + fn lexically_same_or_inside(path: &str, base: &str) -> bool { + let path = lexically_resolved(path); + let base = lexically_resolved(base); + path == base || path.starts_with(&format!("{}/", base.trim_end_matches('/'))) + } + + /// Does `path` really resolve to a protected system location? + fn lexically_protected(path: &str) -> bool { + let resolved = lexically_resolved(path); + resolved == "/" + || PROTECTED_ROOTS + .iter() + .any(|root| lexically_same_or_inside(&resolved, root)) + } + + fn home_text() -> Option { + runtime_home_dir().map(|home| home.display().to_string()) + } + + /// The guard deliberately exempts anything STRICTLY inside the user's + /// own home directory, so a user install under `~/.rocm` stays + /// removable even when home itself sits under a protected root (`/root` + /// for a root user). A property about protection has to grant the same + /// exemption, or it would just be re-litigating that decision. + fn exempt_as_user_owned(path: &str) -> bool { + home_text().is_some_and(|home| { + lexically_same_or_inside(path, &home) + && lexically_resolved(path) != lexically_resolved(&home) + }) + } + + /// The spellings [`a_protected_location_is_refused_however_it_is_spelled`] + /// shrank to, pinned as examples so each stays named even if the + /// generator is retuned. Every one of these names `/etc` (or `/`), and + /// every one of them is a plausible way for a path to be written down + /// by hand, assembled by a script, or produced by joining. + #[test] + fn the_delete_guard_refuses_every_spelling_of_a_protected_root() { + let home = runtime_home_dir().expect("a home directory"); + let home = home.display().to_string(); + let spellings = [ + "/etc".to_owned(), + "/etc/".to_owned(), + "//etc".to_owned(), + "/./etc".to_owned(), + "/etc/.".to_owned(), + "/etc//".to_owned(), + "/usr/../etc".to_owned(), + "/etc/..".to_owned(), + format!("{home}/../../etc"), + format!("{home}/.rocm/../../../etc"), + ]; + let reported: Vec<(String, bool)> = spellings + .into_iter() + .map(|text| { + let protected = runtime_install_root_is_protected(Path::new(&text)); + (text, protected) + }) + .collect(); + let removable: Vec<&str> = reported + .iter() + .filter(|(_, protected)| !protected) + .map(|(text, _)| text.as_str()) + .collect(); + + assert!( + removable.is_empty(), + "reported removable: {removable:?}, full result: {reported:?}" + ); + } + + /// A naive generator is worthless here. Drawing arbitrary strings would + /// spend every draw on paths that resolve nowhere near a protected + /// root, and would pass against a guard that is wide open. So the + /// alphabet is tiny and entirely made of the components that matter: + /// `..` and `.` (the ones nothing in the guard resolves), the empty + /// string (which produces a doubled separator once joined), and the + /// names of real protected roots. + fn component() -> impl Strategy { + prop_oneof![ + 6 => Just(".."), + 3 => Just("."), + 2 => Just(""), + 3 => Just("etc"), + 2 => Just("usr"), + 2 => Just("rocm"), + 2 => Just("runtimes"), + // Off Windows a backslash is an ordinary filename byte, so this + // is ONE legitimate directory name, not two components. It is in + // the alphabet because folding `\` to `/` unconditionally and + // then resolving `..` silently walks out of a protected root; + // without this component every property below still passes. + 2 => Just(r"..\tmp"), + // The leaf names a real managed `install_root` ends in, so the + // generated paths look like the ones the guard actually sees. + 2 => Just("wheel"), + 2 => Just("tarball"), + ] + } + + /// Seeded with the places a real `install_root` is written down: the + /// protected roots themselves, the user's home, and an ordinary + /// unprotected folder. + fn prefix() -> impl Strategy { + let mut seeds: Vec = PROTECTED_ROOTS + .iter() + .map(|&root| root.to_owned()) + .collect(); + seeds.push("/".to_owned()); + seeds.push("/tmp".to_owned()); + seeds.push("/home".to_owned()); + if let Some(home) = home_text() { + seeds.push(format!("{home}/.rocm")); + seeds.push(home); + } + proptest::sample::select(seeds) + } + + /// Paths rooted at the user's own home, so the exemption that keeps + /// `~/.rocm/...` removable is sampled densely instead of being drowned + /// out by the thirteen protected prefixes. + fn user_owned_text() -> impl Strategy { + let home = home_text().unwrap_or_else(|| "/home/rocm".to_owned()); + let seeds = vec![ + home.clone(), + format!("{home}/.rocm"), + format!("{home}/.rocm/data"), + ]; + path_text(proptest::sample::select(seeds)) + } + + /// A path is text, and the guard must answer for the folder that text + /// names, not for the characters it happens to be spelled with. + fn install_root_text() -> impl Strategy { + path_text(prefix()) + } + + fn path_text(prefix: impl Strategy) -> impl Strategy { + ( + prefix, + proptest::collection::vec(component(), 0..4), + prop_oneof![4 => Just(""), 2 => Just("/"), 1 => Just("/."), 1 => Just("/..")], + ) + .prop_map(|(prefix, parts, trailing)| { + let mut text = prefix; + for part in parts { + text.push('/'); + text.push_str(part); + } + text.push_str(trailing); + text + }) + } + + proptest::proptest! { + /// The contract the guard exists for: no path that really resolves + /// into a protected system location may be reported removable, + /// however it is spelled. A counterexample here is `remove_dir_all` + /// on a system directory. + #[test] + fn a_protected_location_is_refused_however_it_is_spelled( + text in install_root_text(), + ) { + prop_assume!(!exempt_as_user_owned(&text)); + prop_assume!(lexically_protected(&text)); + + prop_assert!( + runtime_install_root_is_protected(Path::new(&text)), + "{text} resolves to {} but was reported removable", + lexically_resolved(&text) + ); + } + + /// The guard must not swing the other way either: a folder that + /// really is the user's own stays removable, or `runtimes uninstall` + /// refuses the very folder it created. + #[test] + fn a_user_owned_location_stays_removable(text in user_owned_text()) { + // Strictly-inside-home is the whole contract: the exemption is + // unconditional, so this must hold even when home itself sits + // under a protected root (`/root` for a root user). Filtering + // those draws out with `!lexically_protected` would both assume + // away the interesting case AND reject every draw on such a + // host, which proptest reports as a hard abort rather than a + // skip. + prop_assume!(exempt_as_user_owned(&text)); + + prop_assert!( + !runtime_install_root_is_protected(Path::new(&text)), + "{text} resolves to {} but was refused", + lexically_resolved(&text) + ); + } + + /// Containment is what both the home exemption and the protected + /// -root check are built out of, so it has to agree with where the + /// paths actually resolve — in both directions. Answering "inside" + /// for a path that escaped lets a caller out of the guard; + /// answering "outside" for one that did not hides a protected root. + #[test] + fn containment_agrees_with_where_the_paths_resolve( + path in install_root_text(), + base in prefix(), + ) { + prop_assert_eq!( + runtime_path_is_same_or_inside(Path::new(&path), Path::new(&base)), + lexically_same_or_inside(&path, &base), + "containment of {} in {} disagrees with {} in {}", + path.clone(), + base.clone(), + lexically_resolved(&path), + lexically_resolved(&base) + ); + } + + /// Two spellings of one folder are one folder. + #[test] + fn equivalence_sees_through_spelling(text in install_root_text()) { + let resolved = lexically_resolved(&text); + prop_assert!( + runtime_paths_equivalent(Path::new(&text), Path::new(&resolved)), + "{text} and {resolved} name the same folder but compared unequal" + ); + } + + } + } }