From 7fac7eec30bc8d9cd3196d3317f8f1adb2d085bb Mon Sep 17 00:00:00 2001 From: JongKyung Lee Date: Sun, 25 Oct 2026 12:28:50 +0900 Subject: [PATCH 1/5] fix(implode): prevent deletion of home and filesystem roots Reject protected deletion targets before modifying shell profiles, PATH, or managed files, including with --yes. Check canonical paths to catch symlink aliases and validate all roots before starting split-layout cleanup. Add regression coverage for protected roots, normal installations, symlink aliases, and preservation of files when validation fails. --- crates/vp_global_cli/src/commands/implode.rs | 137 ++++++++++++++++++- 1 file changed, 136 insertions(+), 1 deletion(-) diff --git a/crates/vp_global_cli/src/commands/implode.rs b/crates/vp_global_cli/src/commands/implode.rs index 71796fa508..75e3cb68e8 100644 --- a/crates/vp_global_cli/src/commands/implode.rs +++ b/crates/vp_global_cli/src/commands/implode.rs @@ -50,6 +50,33 @@ fn lexical_path(path: &Path) -> PathBuf { normalized } +fn validate_delete_roots( + roots: &[AbsolutePathBuf], + user_home: &AbsolutePathBuf, +) -> Result<(), Error> { + let home = lexical_path(user_home.as_path()); + let canonical_home = dunce::canonicalize(user_home)?; + for root in roots { + let path = root.as_path(); + let canonical = match dunce::canonicalize(path) { + Ok(path) => path, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => path.to_path_buf(), + Err(error) => return Err(error.into()), + }; + if path.parent().is_none() + || home.starts_with(path) + || canonical.parent().is_none() + || canonical_home.starts_with(&canonical) + { + return Err(Error::Other(vt_str::format!( + "Refusing to remove protected directory {}: it is a filesystem root, the user home, or an ancestor of the user home. Check your Vite+ directory overrides.", + path.display() + ))); + } + } + Ok(()) +} + pub fn execute(yes: bool) -> Result { let env_config = vp_shared::EnvConfig::get(); let dirs = &env_config.dirs; @@ -88,8 +115,9 @@ pub fn execute(yes: bool) -> Result { return Ok(exit_status(0)); } - // Use the user home to resolve shell-profile paths. + // Validate every deletion target before changing profiles, PATH, or files, even with --yes. let user_home = &env_config.user_home; + validate_delete_roots(&delete_set, user_home)?; let source_matcher = VitePlusSourceMatcher::new(&dirs.config, user_home); @@ -1544,6 +1572,113 @@ mod tests { }); } + #[test] + fn validate_delete_roots_rejects_home_ancestors_and_filesystem_roots() { + let temp_dir = tempfile::tempdir().unwrap(); + let home = AbsolutePathBuf::new(temp_dir.path().join("home")).unwrap(); + std::fs::create_dir_all(&home).unwrap(); + for ancestor in home.as_path().ancestors() { + let root = AbsolutePathBuf::new(ancestor.to_path_buf()).unwrap(); + let error = validate_delete_roots(std::slice::from_ref(&root), &home).unwrap_err(); + assert!(error.to_string().contains("Refusing to remove protected directory")); + } + } + + #[test] + fn validate_delete_roots_allows_dedicated_installations() { + let temp_dir = tempfile::tempdir().unwrap(); + let home = AbsolutePathBuf::new(temp_dir.path().join("home")).unwrap(); + std::fs::create_dir_all(&home).unwrap(); + let roots = [home.join(".vite-plus"), home.join("../home-other"), home.join("../custom")]; + for root in &roots { + std::fs::create_dir_all(root).unwrap(); + } + validate_delete_roots(&roots, &home).unwrap(); + validate_delete_roots(&[home.join("missing")], &home).unwrap(); + } + + #[cfg(unix)] + #[test] + fn validate_delete_roots_checks_symlinked_roots_and_home() { + let temp_dir = tempfile::tempdir().unwrap(); + let base = AbsolutePathBuf::new(temp_dir.path().to_path_buf()).unwrap(); + let home = base.join("home"); + let alias = base.join("alias"); + std::fs::create_dir_all(home.join(".vite-plus")).unwrap(); + std::os::unix::fs::symlink(&base, &alias).unwrap(); + for root in [&alias, &alias.join("home")] { + assert!(validate_delete_roots(std::slice::from_ref(root), &home).is_err()); + } + let aliased_home = alias.join("home"); + assert!(validate_delete_roots(std::slice::from_ref(&home), &aliased_home).is_err()); + validate_delete_roots(&[alias.join("home/.vite-plus")], &home).unwrap(); + } + + #[cfg(unix)] + #[test] + fn execute_rejects_user_home_before_modifying_files() { + let temp_dir = tempfile::tempdir().unwrap(); + let home = temp_dir.path().join("home"); + let xdg = temp_dir.path().join("xdg"); + std::fs::create_dir_all(&home).unwrap(); + std::fs::write(home.join("personal.txt"), "keep").unwrap(); + let profile = "# Vite+ bin\n. \"$HOME/env\"\n"; + std::fs::write(home.join(".bashrc"), profile).unwrap(); + + for root in [&home, &home.join("child/.."), &home.join("..")] { + vp_shared::EnvConfig::with_vars( + [ + (env_vars::VP_HOME, root.as_os_str()), + ("HOME", home.as_os_str()), + ("USERPROFILE", home.as_os_str()), + ("ZDOTDIR", home.as_os_str()), + (env_vars::XDG_CONFIG_HOME, xdg.as_os_str()), + (env_vars::XDG_DATA_HOME, xdg.as_os_str()), + ], + |_| { + let error = execute(true).unwrap_err(); + assert!(error.to_string().contains("Refusing to remove protected directory")); + assert_eq!(std::fs::read_to_string(home.join("personal.txt")).unwrap(), "keep"); + assert_eq!(std::fs::read_to_string(home.join(".bashrc")).unwrap(), profile); + }, + ); + } + } + + #[cfg(unix)] + #[test] + fn execute_validates_all_split_roots_before_deleting_any() { + let temp_dir = tempfile::tempdir().unwrap(); + let home = temp_dir.path().join("z-home"); + let data = temp_dir.path().join("a-data"); + let bin = temp_dir.path().join("bin"); + let xdg = temp_dir.path().join("xdg"); + std::fs::create_dir_all(&home).unwrap(); + std::fs::create_dir_all(&data).unwrap(); + std::fs::write(home.join("personal.txt"), "keep").unwrap(); + std::fs::write(data.join("data.txt"), "keep").unwrap(); + vp_shared::EnvConfig::with_vars( + [ + (env_vars::VP_HOME, None), + (env_vars::VP_BIN_DIR, Some(bin.as_os_str())), + (env_vars::VP_DATA_DIR, Some(data.as_os_str())), + (env_vars::VP_CACHE_DIR, Some(home.as_os_str())), + (env_vars::XDG_CONFIG_HOME, Some(xdg.as_os_str())), + (env_vars::XDG_DATA_HOME, Some(xdg.as_os_str())), + (env_vars::XDG_STATE_HOME, Some(xdg.as_os_str())), + ("HOME", Some(home.as_os_str())), + ("USERPROFILE", Some(home.as_os_str())), + ("ZDOTDIR", Some(home.as_os_str())), + ], + |_| { + let error = execute(true).unwrap_err(); + assert!(error.to_string().contains("Refusing to remove protected directory")); + assert_eq!(std::fs::read_to_string(home.join("personal.txt")).unwrap(), "keep"); + assert_eq!(std::fs::read_to_string(data.join("data.txt")).unwrap(), "keep"); + }, + ); + } + #[test] fn execute_normalizes_category_roots_before_deduplication() { let temp_dir = tempfile::tempdir().unwrap(); From e64845e2b13a152e9726ae7dbf0dd321a680e47f Mon Sep 17 00:00:00 2001 From: JongKyung Lee Date: Sun, 25 Oct 2026 14:34:51 +0900 Subject: [PATCH 2/5] fix(implode): verify ownership of every removal root Require ownership markers for every existing data, cache, config, and state root before changing files, shell profiles, or PATH. Validate nested roots independently and collapse them only when building the final recursive deletion targets. Preserve the home and filesystem root safeguards. Check that new installation roots are empty before recording ownership. Keep nonempty legacy roots unmarked so existing installations remain usable without granting permission to delete unrelated contents. Cover ownership checks, nested and split roots, fresh setup, and legacy setup with isolated unit tests and CLI snapshots. --- .../snapshots.toml | 1 + .../command_env_setup_external_vp.md | 6 + .../fixtures/command_implode/snapshots.toml | 22 ++ .../snapshots/implode_confirmation.md | 17 ++ .../snapshots/implode_unowned_directory.md | 60 ++++++ .../command_self_setup/snapshots.toml | 9 +- .../snapshots/command_self_setup.md | 10 +- .../command_self_setup_bootstrap_options.md | 12 ++ .../snapshots/command_self_setup_handoff.md | 6 + .../command_self_setup_mixed_shim_refresh.md | 6 + .../snapshots.toml | 1 + .../command_self_setup_external_bare.md | 6 + .../command_self_setup_external_doctor.md | 10 +- .../verify-refresh.mjs | 5 +- .../tests/cli_snapshots/main.rs | 2 + crates/vp_global_cli/src/commands/implode.rs | 79 +++---- crates/vp_global_cli/src/install_dirs.rs | 195 ++++++++++++++++++ crates/vp_global_cli/src/main.rs | 1 + crates/vp_global_cli/src/self_setup.rs | 4 + 19 files changed, 404 insertions(+), 48 deletions(-) create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_unowned_directory.md create mode 100644 crates/vp_global_cli/src/install_dirs.rs diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_env_setup_external_vp/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_env_setup_external_vp/snapshots.toml index 266b63b1ba..18b629836b 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_env_setup_external_vp/snapshots.toml +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_env_setup_external_vp/snapshots.toml @@ -5,6 +5,7 @@ skip-platforms = ["windows"] env = { VP_NODE_MANAGER = "yes", VP_SELF_SETUP_NO_MODIFY_PATH = "1" } steps = [ { argv = ["vpt", "mkdir", "-p", "external/bin", "home"], comment = "Prepare isolated external install and VP_HOME", snapshot = false }, + { argv = ["vpt", "write-file", "home/.vp-owned", "vite-plus-owned-directory\n"], comment = "The fixture owns the preseeded runtime directory", snapshot = false }, { argv = ["vpt", "cp", "$VP_HOME/bin/vp", "external/bin/vp"], comment = "Simulate a Homebrew-style vp outside VP_HOME", snapshot = false }, { argv = ["vpt", "chmod", "+x", "external/bin/vp"], snapshot = false }, { argv = ["vpt", "write-file", "external/node_modules/vite-plus/package.json", "{}"], snapshot = false }, diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_env_setup_external_vp/snapshots/command_env_setup_external_vp.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_env_setup_external_vp/snapshots/command_env_setup_external_vp.md index 8313c2227c..6268dee4bb 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_env_setup_external_vp/snapshots/command_env_setup_external_vp.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_env_setup_external_vp/snapshots/command_env_setup_external_vp.md @@ -5,6 +5,12 @@ Prepare isolated external install and VP_HOME +## `vpt write-file home/.vp-owned 'vite-plus-owned-directory +'` + +The fixture owns the preseeded runtime directory + + ## `vpt cp $VP_HOME/bin/vp external/bin/vp` Simulate a Homebrew-style vp outside VP_HOME diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots.toml index 8a760e8b29..8164496b3d 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots.toml +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots.toml @@ -3,6 +3,9 @@ name = "implode_confirmation" vp = "global" seed-runtime = false steps = [ + { argv = ["vpt", "rm", "$VP_HOME/cache/.vp-owned"], snapshot = false }, + { argv = ["vp", "implode", "--yes"], comment = "A nested cache needs its own ownership marker before any deletion", continue-on-failure = true }, + { argv = ["vpt", "write-file", "$VP_HOME/cache/.vp-owned", "vite-plus-owned-directory\n"], snapshot = false }, { argv = ["vp", "implode"], comment = "The old confirmation word leaves the installation intact", interactions = [ { expect-milestone = "text:implode:ready" }, { write-line = "uninstall" }, @@ -25,3 +28,22 @@ steps = [ ] }, ["vpt", "stat-file", "$VP_HOME", "--assert", "missing"], ] + +[[case]] +name = "implode_unowned_directory" +vp = "global" +seed-runtime = false +skip-platforms = ["windows"] +env = { VP_SKIP_DEPS_INSTALL = "1", VP_NODE_MANAGER = "no" } +steps = [ + { argv = ["vpt", "mkdir", "external"], snapshot = false }, + { argv = ["vpt", "cp", "$VP_HOME/bin/vp", "external/vp"], snapshot = false }, + { argv = ["vpt", "chmod", "+x", "external/vp"], snapshot = false }, + { argv = ["vpt", "write-file", "$HOME/Documents/personal.txt", "keep my documents\n"], snapshot = false }, + { argv = ["vpt", "write-file", "$HOME/.bashrc", "# personal config\n# Vite+ bin\n. \"$HOME/Documents/env\"\n"], snapshot = false }, + { argv = ["./external/vp"], tty = false, envs = [["VP_HOME", "${HOME}/Documents"]], comment = "A fresh install cannot claim a nonempty personal directory", continue-on-failure = true }, + { argv = ["vp", "implode", "--yes"], envs = [["VP_HOME", "${HOME}/Documents"]], comment = "Changing VP_HOME cannot authorize recursive deletion", continue-on-failure = true }, + ["vpt", "stat-file", "$HOME/Documents/.vp-owned", "--assert", "missing"], + ["vpt", "print-file", "$HOME/Documents/personal.txt"], + ["vpt", "print-file", "$HOME/.bashrc"], +] diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_confirmation.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_confirmation.md index 9f23cb0e5a..93e3d0d209 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_confirmation.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_confirmation.md @@ -1,5 +1,22 @@ # implode_confirmation +## `vpt rm $VP_HOME/cache/.vp-owned` + + +## `vp implode --yes` + +A nested cache needs its own ownership marker before any deletion + +**Exit code:** 1 + +``` +error: Refusing to remove /.vite-plus/cache: Vite+ directory ownership could not be verified. Unmarked directories, including older installations, must be inspected and removed manually. +``` + +## `vpt write-file $VP_HOME/cache/.vp-owned 'vite-plus-owned-directory +'` + + ## `vp implode` The old confirmation word leaves the installation intact diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_unowned_directory.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_unowned_directory.md new file mode 100644 index 0000000000..fd6027c05f --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_unowned_directory.md @@ -0,0 +1,60 @@ +# implode_unowned_directory + +## `vpt mkdir external` + + +## `vpt cp $VP_HOME/bin/vp external/vp` + + +## `vpt chmod +x external/vp` + + +## `vpt write-file $HOME/Documents/personal.txt 'keep my documents +'` + + +## `vpt write-file $HOME/.bashrc '# personal config +# Vite+ bin +. "$HOME/Documents/env" +'` + + +## `VP_HOME=${HOME}/Documents ./external/vp` + +A fresh install cannot claim a nonempty personal directory + +**Exit code:** 1 + +``` +error: Refusing to initialize /Documents: the directory is not empty and Vite+ ownership could not be verified. Choose an empty installation directory. +``` + +## `VP_HOME=${HOME}/Documents vp implode --yes` + +Changing VP_HOME cannot authorize recursive deletion + +**Exit code:** 1 + +``` +error: Refusing to remove /Documents: Vite+ directory ownership could not be verified. Unmarked directories, including older installations, must be inspected and removed manually. +``` + +## `vpt stat-file $HOME/Documents/.vp-owned --assert missing` + +``` +/Documents/.vp-owned: missing +``` + +## `vpt print-file $HOME/Documents/personal.txt` + +``` +keep my documents +``` + +## `vpt print-file $HOME/.bashrc` + +``` +# personal config +# Vite+ bin +. "$HOME/Documents/env" +``` diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots.toml index 887e0cf973..d09be4b917 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots.toml +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots.toml @@ -2,13 +2,14 @@ name = "command_self_setup" vp = "global" # The runner provides local dependencies; this case checks setup and dispatch. -env = { VP_SKIP_DEPS_INSTALL = "1" } +env = { VP_SKIP_DEPS_INSTALL = "1", VP_SELF_SETUP_NO_MODIFY_PATH = "1" } steps = [ - { argv = ["vpt", "rm", "$VP_HOME/current/bin/.vp-setup-complete"], snapshot = false }, + { argv = ["vpt", "rm", "$VP_HOME/current/bin/.vp-setup-complete", "$VP_HOME/.vp-owned"], comment = "An older installation has no directory ownership marker", snapshot = false }, { argv = ["vp", "--help"], envs = [["VP_SELF_SETUP_SUPPORT_CHECK", "1"]], comment = "The capability probe does not perform setup" }, ["vpt", "stat-file", "$VP_HOME/current/bin/.vp-setup-complete", "--assert", "missing"], { argv = ["vp", "--help"], comment = "An unmarked deployed binary completes setup and executes the requested command", snapshot = false }, ["vpt", "stat-file", "$VP_HOME/current/bin/.vp-setup-complete", "--assert", "file"], + ["vpt", "stat-file", "$VP_HOME/.vp-owned", "--assert", "missing"], { argv = ["vp", "not-a-command"], comment = "Once marked, the binary dispatches commands normally", continue-on-failure = true }, ] @@ -19,6 +20,7 @@ requires = ["bash"] skip-platforms = ["windows"] steps = [ { argv = ["vpt", "mkdir", "-p", "external", "project", "home/js_runtime/node"], snapshot = false }, + { argv = ["vpt", "write-file", "home/.vp-owned", "vite-plus-owned-directory\n"], comment = "The fixture owns the preseeded runtime directory", snapshot = false }, { argv = ["vpt", "cp", "-r", "$VP_HOME/js_runtime/node/22.18.0", "home/js_runtime/node/22.18.0"], snapshot = false }, { argv = ["vpt", "cp", "$VP_HOME/bin/vp", "external/vp"], snapshot = false }, { argv = ["vpt", "chmod", "+x", "external/vp"], snapshot = false }, @@ -54,6 +56,8 @@ steps = [ ["vpt", "print-file", "home/config.json"], ["vpt", "print-file", "user/.bashrc"], ["vpt", "stat-file", "user/.zshenv", "--assert", "missing"], + ["vpt", "stat-file", "home/.vp-owned", "--assert", "file"], + ["vpt", "stat-file", "home/cache/.vp-owned", "--assert", "file"], ] [[case]] @@ -108,6 +112,7 @@ skip-platforms = ["windows"] env = { VP_SKIP_DEPS_INSTALL = "1", VP_SELF_SETUP_NO_MODIFY_PATH = "1" } steps = [ { argv = ["vpt", "mkdir", "-p", "external", "home/bin", "user-bin"], snapshot = false }, + { argv = ["vpt", "write-file", "home/.vp-owned", "vite-plus-owned-directory\n"], comment = "The fixture owns the preseeded shim directory", snapshot = false }, { argv = ["vpt", "cp", "$VP_HOME/bin/vp", "external/vp"], snapshot = false }, { argv = ["vpt", "chmod", "+x", "external/vp"], snapshot = false }, { argv = ["node", "seed-owned-shims.cjs"], snapshot = false }, diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots/command_self_setup.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots/command_self_setup.md index b9d93a547a..52a75ab305 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots/command_self_setup.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots/command_self_setup.md @@ -1,6 +1,8 @@ # command_self_setup -## `vpt rm $VP_HOME/current/bin/.vp-setup-complete` +## `vpt rm $VP_HOME/current/bin/.vp-setup-complete $VP_HOME/.vp-owned` + +An older installation has no directory ownership marker ## `VP_SELF_SETUP_SUPPORT_CHECK=1 vp --help` @@ -28,6 +30,12 @@ An unmarked deployed binary completes setup and executes the requested command /.vite-plus/current/bin/.vp-setup-complete: file ``` +## `vpt stat-file $VP_HOME/.vp-owned --assert missing` + +``` +/.vite-plus/.vp-owned: missing +``` + ## `vp not-a-command` Once marked, the binary dispatches commands normally diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots/command_self_setup_bootstrap_options.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots/command_self_setup_bootstrap_options.md index 57f4a5d195..c7e99707f2 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots/command_self_setup_bootstrap_options.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots/command_self_setup_bootstrap_options.md @@ -49,3 +49,15 @@ home/current/bin/.vp-setup-complete: file ``` user/.zshenv: missing ``` + +## `vpt stat-file home/.vp-owned --assert file` + +``` +home/.vp-owned: file +``` + +## `vpt stat-file home/cache/.vp-owned --assert file` + +``` +home/cache/.vp-owned: file +``` diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots/command_self_setup_handoff.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots/command_self_setup_handoff.md index f240ee7efd..47a35452b1 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots/command_self_setup_handoff.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots/command_self_setup_handoff.md @@ -3,6 +3,12 @@ ## `vpt mkdir -p external project home/js_runtime/node` +## `vpt write-file home/.vp-owned 'vite-plus-owned-directory +'` + +The fixture owns the preseeded runtime directory + + ## `vpt cp -r $VP_HOME/js_runtime/node/22.18.0 home/js_runtime/node/22.18.0` diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots/command_self_setup_mixed_shim_refresh.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots/command_self_setup_mixed_shim_refresh.md index fd2e910e05..a1626e1717 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots/command_self_setup_mixed_shim_refresh.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots/command_self_setup_mixed_shim_refresh.md @@ -3,6 +3,12 @@ ## `vpt mkdir -p external home/bin user-bin` +## `vpt write-file home/.vp-owned 'vite-plus-owned-directory +'` + +The fixture owns the preseeded shim directory + + ## `vpt cp $VP_HOME/bin/vp external/vp` diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/snapshots.toml index 1a99fcb201..78f224d487 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/snapshots.toml +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/snapshots.toml @@ -55,6 +55,7 @@ skip-platforms = ["windows"] env = { VP_SKIP_DEPS_INSTALL = "1", VP_VERSION = "external-test", VP_SELF_SETUP_NO_MODIFY_PATH = "1" } steps = [ { argv = ["vpt", "mkdir", "-p", "external", "home/external-test/bin"], snapshot = false }, + { argv = ["vpt", "write-file", "home/.vp-owned", "vite-plus-owned-directory\n"], comment = "An interrupted fresh install already claimed its empty root", snapshot = false }, { argv = ["vpt", "cp", "$VP_HOME/bin/vp", "external/vp"], snapshot = false }, { argv = ["vpt", "chmod", "555", "external/vp"], snapshot = false }, { argv = ["vpt", "cp", "external/vp", "home/external-test/bin/vp"], snapshot = false }, diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/snapshots/command_self_setup_external_bare.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/snapshots/command_self_setup_external_bare.md index 8ac9251661..970e020fd9 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/snapshots/command_self_setup_external_bare.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/snapshots/command_self_setup_external_bare.md @@ -3,6 +3,12 @@ ## `vpt mkdir -p external home/external-test/bin` +## `vpt write-file home/.vp-owned 'vite-plus-owned-directory +'` + +An interrupted fresh install already claimed its empty root + + ## `vpt cp $VP_HOME/bin/vp external/vp` diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/snapshots/command_self_setup_external_doctor.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/snapshots/command_self_setup_external_doctor.md index 22f1ad7291..d31929e11d 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/snapshots/command_self_setup_external_doctor.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/snapshots/command_self_setup_external_doctor.md @@ -6,8 +6,8 @@ Homebrew with shims on PATH CLI source Homebrew CLI binary /doctor/cellar/vite-plus/0.3.2/bin/vp - ✓ vp ~/bin/vp - ✓ Shim dir ~/bin + ✓ vp ~/.vite-plus/bin/vp + ✓ Shim dir ~/.vite-plus/bin Homebrew without shims on PATH CLI source Homebrew CLI binary /doctor/cellar/vite-plus/0.3.2/bin/vp @@ -17,8 +17,8 @@ Missing vp in the shim directory CLI source Homebrew CLI binary /doctor/cellar/vite-plus/0.3.2/bin/vp ✗ vp not in PATH - ✓ Shim dir ~/bin + ✓ Shim dir ~/.vite-plus/bin External package without a Homebrew receipt - ✓ vp ~/bin/vp - ✓ Shim dir ~/bin + ✓ vp ~/.vite-plus/bin/vp + ✓ Shim dir ~/.vite-plus/bin ``` diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/verify-refresh.mjs b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/verify-refresh.mjs index 5389615478..3fdba650eb 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/verify-refresh.mjs +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/verify-refresh.mjs @@ -26,11 +26,12 @@ function createBundle(source, prefix, label) { } function createEnvironment(directory) { - const home = path.join(directory, 'home'); + const home = path.join(directory, 'home', '.vite-plus'); const system = path.join(directory, 'system/bin'); const runtime = path.join(home, 'js_runtime/node', process.versions.node, 'bin'); fs.mkdirSync(system, { recursive: true }); fs.mkdirSync(runtime, { recursive: true }); + fs.writeFileSync(path.join(home, '.vp-owned'), 'vite-plus-owned-directory\n'); fs.symlinkSync(process.execPath, path.join(system, 'node')); fs.symlinkSync(process.execPath, path.join(runtime, 'node')); fs.writeFileSync(path.join(directory, '.node-version'), process.versions.node); @@ -40,7 +41,7 @@ function createEnvironment(directory) { } return { ...env, - HOME: home, + HOME: path.dirname(home), VP_HOME: home, VP_SELF_SETUP_NO_MODIFY_PATH: '1', NPM_CONFIG_REGISTRY: 'http://127.0.0.1:9', diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs b/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs index 35a0a7b280..b3f5f4d65a 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs @@ -579,6 +579,8 @@ impl CaseHome { let this = Self { home: root.join("home") }; let vp_home = this.vp_home(); std::fs::create_dir_all(&vp_home).unwrap(); + // Provisioning emulates a fresh install into this case-owned directory. + std::fs::write(vp_home.join(".vp-owned"), b"vite-plus-owned-directory\n").unwrap(); std::fs::create_dir_all(root.join("npm-global/lib")).unwrap(); // Best-effort: if linking fails, the case downloads the runtime. if seed_runtime && let Some(seed) = flavor::js_runtime_seed_dir() { diff --git a/crates/vp_global_cli/src/commands/implode.rs b/crates/vp_global_cli/src/commands/implode.rs index 75e3cb68e8..e80d6ad1eb 100644 --- a/crates/vp_global_cli/src/commands/implode.rs +++ b/crates/vp_global_cli/src/commands/implode.rs @@ -1,10 +1,8 @@ //! `vp implode` — completely remove vp and all its data from this system. -use std::{ - io::Write, - path::{Component, Path, PathBuf}, - process::ExitStatus, -}; +#[cfg(any(windows, test))] +use std::path::{Path, PathBuf}; +use std::{io::Write, process::ExitStatus}; use console::style; use rustc_hash::FxHashSet; @@ -20,6 +18,7 @@ use crate::{ shell::{ALL_SHELL_PROFILES, ShellProfileKind, abbreviate_home_path, resolve_profile_path}, }, error::Error, + install_dirs::{self, lexical_path}, shim::DEFAULT_SHIM_TOOLS, }; @@ -36,20 +35,6 @@ const DEFERRED_DELETE_ATTEMPTS: usize = 100; #[cfg(windows)] const DEFERRED_DELETE_RETRY_DELAY: std::time::Duration = std::time::Duration::from_millis(100); -fn lexical_path(path: &Path) -> PathBuf { - let mut normalized = PathBuf::new(); - for component in path.components() { - match component { - Component::CurDir => {} - Component::ParentDir => { - normalized.pop(); - } - _ => normalized.push(component.as_os_str()), - } - } - normalized -} - fn validate_delete_roots( roots: &[AbsolutePathBuf], user_home: &AbsolutePathBuf, @@ -89,21 +74,7 @@ pub fn execute(yes: bool) -> Result { // The default Unix `` is under ``, so removal of `` also // removes it. Never remove a separately resolved `` because a bin from // an explicit override group can be shared. Remove only Vite+-owned shims. - let mut roots: Vec = [&dirs.data, &dirs.cache, &dirs.config, &dirs.state] - .into_iter() - .map(|root| { - AbsolutePathBuf::new(lexical_path(root.as_path())) - .expect("resolved Vite+ roots remain absolute after lexical normalization") - }) - .collect(); - roots.sort_by(|a, b| a.as_path().cmp(b.as_path())); - roots.dedup(); - let mut delete_set: Vec = Vec::new(); - for root in roots { - if !delete_set.iter().any(|kept| root.as_path().starts_with(kept.as_path())) { - delete_set.push(root); - } - } + let mut delete_set = install_dirs::roots(dirs); if !delete_set.iter().any(|root| root.as_path().exists()) { if homebrew { @@ -118,6 +89,9 @@ pub fn execute(yes: bool) -> Result { // Validate every deletion target before changing profiles, PATH, or files, even with --yes. let user_home = &env_config.user_home; validate_delete_roots(&delete_set, user_home)?; + install_dirs::require_owned(&delete_set)?; + // Sorted roots can be collapsed only after every category has passed validation. + delete_set.dedup_by(|child, parent| child.as_path().starts_with(parent.as_path())); let source_matcher = VitePlusSourceMatcher::new(&dirs.config, user_home); @@ -1679,8 +1653,9 @@ mod tests { ); } + #[cfg(unix)] #[test] - fn execute_normalizes_category_roots_before_deduplication() { + fn execute_requires_each_owned_root_before_removing_split_installation() { let temp_dir = tempfile::tempdir().unwrap(); let home = temp_dir.path().join("home"); let bin = temp_dir.path().join("bin"); @@ -1692,8 +1667,6 @@ mod tests { std::fs::create_dir_all(&home).unwrap(); std::fs::create_dir_all(&data).unwrap(); std::fs::create_dir_all(&normalized_cache).unwrap(); - std::fs::write(data.join("data.txt"), b"data").unwrap(); - std::fs::write(normalized_cache.join("cache.txt"), b"cache").unwrap(); vp_shared::EnvConfig::with_vars( [ @@ -1702,13 +1675,43 @@ mod tests { (env_vars::VP_DATA_DIR, Some(data.as_os_str())), (env_vars::VP_CACHE_DIR, Some(cache.as_os_str())), (env_vars::XDG_CONFIG_HOME, Some(xdg_config.as_os_str())), + (env_vars::XDG_DATA_HOME, Some(xdg_config.as_os_str())), (env_vars::XDG_STATE_HOME, Some(xdg_state.as_os_str())), ("HOME", Some(home.as_os_str())), ("USERPROFILE", Some(home.as_os_str())), + ("ZDOTDIR", Some(home.as_os_str())), ], - |_| { + |config| { + install_dirs::prepare(&config.dirs, false).unwrap(); + std::fs::write(data.join("data.txt"), b"data").unwrap(); + std::fs::write(normalized_cache.join("cache.txt"), b"cache").unwrap(); + let profile = vt_str::format!( + "# personal config\n# Vite+ bin\n. \"{}/env\"\n", + config.dirs.config.as_path().display() + ); + std::fs::write(home.join(".bashrc"), profile.as_bytes()).unwrap(); + for root in install_dirs::roots(&config.dirs) { + let marker = root.join(install_dirs::OWNERSHIP_MARKER); + let contents = std::fs::read(&marker).unwrap(); + std::fs::remove_file(&marker).unwrap(); + assert!(execute(true).unwrap_err().to_string().contains("ownership")); + assert_eq!(std::fs::read(data.join("data.txt")).unwrap(), b"data"); + assert_eq!( + std::fs::read(normalized_cache.join("cache.txt")).unwrap(), + b"cache" + ); + assert_eq!( + std::fs::read_to_string(home.join(".bashrc")).unwrap(), + profile.as_str() + ); + std::fs::write(&marker, contents).unwrap(); + } let result = execute(true).unwrap(); assert!(result.success()); + assert_eq!( + std::fs::read_to_string(home.join(".bashrc")).unwrap(), + "# personal config\n" + ); assert!(!data.exists(), "data root must be removed"); assert!( !normalized_cache.exists(), diff --git a/crates/vp_global_cli/src/install_dirs.rs b/crates/vp_global_cli/src/install_dirs.rs new file mode 100644 index 0000000000..fb78877058 --- /dev/null +++ b/crates/vp_global_cli/src/install_dirs.rs @@ -0,0 +1,195 @@ +//! Ownership of directories that `vp implode` may remove recursively. + +use std::{ + fs, + io::Write, + path::{Component, Path, PathBuf}, +}; + +use vp_shared::VpDirs; +use vt_path::AbsolutePathBuf; + +use crate::error::Error; + +pub(crate) const OWNERSHIP_MARKER: &str = ".vp-owned"; +const OWNERSHIP_CONTENT: &[u8] = b"vite-plus-owned-directory\n"; + +pub(crate) fn lexical_path(path: &Path) -> PathBuf { + let mut normalized = PathBuf::new(); + for component in path.components() { + match component { + Component::CurDir => {} + Component::ParentDir => { + normalized.pop(); + } + _ => normalized.push(component.as_os_str()), + } + } + normalized +} + +/// Distinct data, cache, config, and state roots, including nested categories. +pub(crate) fn roots(dirs: &VpDirs) -> Vec { + let mut roots: Vec<_> = [&dirs.data, &dirs.cache, &dirs.config, &dirs.state] + .into_iter() + .map(|root| { + AbsolutePathBuf::new(lexical_path(root.as_path())) + .expect("resolved Vite+ roots remain absolute after lexical normalization") + }) + .collect(); + roots.sort_by(|a, b| a.as_path().cmp(b.as_path())); + roots.dedup(); + roots +} + +fn is_owned(root: &AbsolutePathBuf) -> Result { + match fs::symlink_metadata(root.join(OWNERSHIP_MARKER)) { + Ok(metadata) => Ok(metadata.is_file()), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(false), + Err(error) => Err(error.into()), + } +} + +pub(crate) fn require_owned(roots: &[AbsolutePathBuf]) -> Result<(), Error> { + for root in roots { + if root.as_path().try_exists()? && !is_owned(root)? { + return Err(Error::Other(vt_str::format!( + "Refusing to remove {}: Vite+ directory ownership could not be verified. Unmarked directories, including older installations, must be inspected and removed manually.", + root.as_path().display() + ))); + } + } + Ok(()) +} + +/// Claim only empty or newly created roots. Legacy installs may remain unowned, +/// but updating them must never grant permission to delete their entire roots. +pub(crate) fn prepare(dirs: &VpDirs, existing_install: bool) -> Result<(), Error> { + let mut empty = Vec::new(); + // Preflight all roots before creating directories or ownership markers. + for root in roots(dirs) { + if is_owned(&root)? { + continue; + } + let nonempty = match fs::read_dir(&root) { + Ok(mut entries) => entries.next().transpose()?.is_some(), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => false, + Err(error) => return Err(error.into()), + }; + if nonempty { + if existing_install { + continue; + } + return Err(Error::Other(vt_str::format!( + "Refusing to initialize {}: the directory is not empty and Vite+ ownership could not be verified. Choose an empty installation directory.", + root.as_path().display() + ))); + } + empty.push(root); + } + for root in empty { + fs::create_dir_all(&root)?; + let mut marker = fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(root.join(OWNERSHIP_MARKER))?; + marker.write_all(OWNERSHIP_CONTENT)?; + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use vp_shared::EnvConfig; + + use super::*; + + #[test] + fn roots_normalize_category_paths_without_discarding_nested_roots() { + EnvConfig::scoped(|env| { + let mut dirs = env.dirs.clone(); + dirs.data = env.dirs.data.join("data"); + dirs.cache = dirs.data.join("../cache"); + dirs.config = dirs.data.join("./config"); + dirs.state = dirs.data.join("../data"); + assert_eq!( + roots(&dirs), + vec![env.dirs.data.join("cache"), dirs.data.clone(), dirs.data.join("config")] + ); + }); + } + + #[test] + fn prepare_claims_empty_roots_and_preserves_shared_bin() { + for split in [false, true] { + EnvConfig::scoped(|env| { + let mut dirs = env.dirs.clone(); + if split { + dirs.data = env.dirs.data.join("data"); + dirs.cache = env.dirs.data.join("cache"); + dirs.config = env.dirs.data.join("config"); + dirs.state = env.dirs.data.join("state"); + fs::create_dir_all(&dirs.bin).unwrap(); + fs::write(dirs.bin.join("other-tool"), b"keep").unwrap(); + } + prepare(&dirs, false).unwrap(); + for root in roots(&dirs) { + assert!(is_owned(&root).unwrap()); + } + fs::write(dirs.data.join("payload"), b"vp").unwrap(); + prepare(&dirs, false).unwrap(); + assert_eq!(fs::read(dirs.data.join("payload")).unwrap(), b"vp"); + assert!(!dirs.bin.join(OWNERSHIP_MARKER).as_path().exists()); + if split { + assert_eq!(fs::read(dirs.bin.join("other-tool")).unwrap(), b"keep"); + } + }); + } + } + + #[test] + fn prepare_preflights_all_roots_and_never_claims_legacy_contents() { + EnvConfig::scoped(|env| { + let mut dirs = env.dirs.clone(); + dirs.data = env.dirs.data.join("a-data"); + dirs.config = env.dirs.data.join("b-config"); + dirs.state = env.dirs.data.join("c-state"); + dirs.cache = env.dirs.data.join("z-cache"); + fs::create_dir_all(&dirs.cache).unwrap(); + fs::write(dirs.cache.join("personal.txt"), b"keep").unwrap(); + assert!(prepare(&dirs, false).is_err()); + for root in [&dirs.data, &dirs.config, &dirs.state] { + assert!(!root.as_path().exists()); + } + prepare(&dirs, true).unwrap(); + assert!(is_owned(&dirs.data).unwrap()); + assert!(!dirs.cache.join(OWNERSHIP_MARKER).as_path().exists()); + assert_eq!(fs::read(dirs.cache.join("personal.txt")).unwrap(), b"keep"); + assert!(require_owned(&roots(&dirs)).is_err()); + }); + } + + #[test] + fn ownership_requires_a_regular_marker_file() { + EnvConfig::scoped(|env| { + prepare(&env.dirs, false).unwrap(); + let roots = roots(&env.dirs); + let marker = env.dirs.cache.join(OWNERSHIP_MARKER); + fs::remove_file(&marker).unwrap(); + assert!(require_owned(&roots).is_err()); + fs::create_dir(&marker).unwrap(); + assert!(require_owned(&roots).is_err()); + fs::remove_dir(&marker).unwrap(); + #[cfg(unix)] + { + let target = env.dirs.data.join("elsewhere"); + fs::write(&target, OWNERSHIP_CONTENT).unwrap(); + std::os::unix::fs::symlink(&target, &marker).unwrap(); + assert!(require_owned(&roots).is_err()); + fs::remove_file(&marker).unwrap(); + } + fs::write(&marker, b"").unwrap(); + require_owned(&roots).unwrap(); + }); + } +} diff --git a/crates/vp_global_cli/src/main.rs b/crates/vp_global_cli/src/main.rs index 5259962b08..d72c6533f3 100644 --- a/crates/vp_global_cli/src/main.rs +++ b/crates/vp_global_cli/src/main.rs @@ -18,6 +18,7 @@ mod commands; mod error; mod help; mod homebrew; +mod install_dirs; mod js_executor; mod self_setup; mod shim; diff --git a/crates/vp_global_cli/src/self_setup.rs b/crates/vp_global_cli/src/self_setup.rs index b220b5d88b..c7df91b322 100644 --- a/crates/vp_global_cli/src/self_setup.rs +++ b/crates/vp_global_cli/src/self_setup.rs @@ -227,6 +227,10 @@ async fn run(source: &Path, bundled: bool) -> Result { } else { version_dir.join("bin").join(VP_BINARY_NAME) }; + crate::install_dirs::prepare( + dirs, + in_place || configured || active_binary.as_path().is_file(), + )?; if deploy { tokio::fs::create_dir_all(version_dir.join("bin")).await?; install::clear_self_setup_marker(&version_dir).await?; From ca1a7958495a6ce22f46b8dd018ff322e94823f8 Mon Sep 17 00:00:00 2001 From: JongKyung Lee Date: Sun, 25 Oct 2026 14:53:40 +0900 Subject: [PATCH 3/5] fix(setup): allow repair of incomplete legacy installations Recognize an existing current entry without following its target, matching installation directory resolution even when the active binary is missing. Keep repaired legacy roots unmarked so implode still refuses their removal. Extend the existing installer CI cases to verify ownership is not claimed when reusing a legacy root or repairing a dangling junction. --- .github/workflows/test-standalone-install.yml | 4 ++++ crates/vp_global_cli/src/self_setup.rs | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test-standalone-install.yml b/.github/workflows/test-standalone-install.yml index 06768611ef..87aaa3c3ed 100644 --- a/.github/workflows/test-standalone-install.yml +++ b/.github/workflows/test-standalone-install.yml @@ -532,6 +532,7 @@ jobs: VP_LOCAL_TGZ="$FAKE_TGZ" VP_VERSION=local-test bash packages/cli/install.sh test -f "$GRAND/.vite-plus/.keep" + test ! -e "$GRAND/.vite-plus/.vp-owned" test -e "$GRAND/.vite-plus/current" test -e "$GRAND/.vite-plus/bin/vp" test ! -d "$GRAND/.local/share/vite-plus" @@ -2168,6 +2169,9 @@ jobs: if (-not (Test-Path (Join-Path $env:VP_HOME "current/bin/.vp-setup-complete"))) { throw "vp-setup.exe did not complete the binary's self-setup" } + if (Test-Path (Join-Path $env:VP_HOME ".vp-owned")) { + throw "Repair claimed ownership of an unmarked installation root" + } if ([Environment]::GetEnvironmentVariable("Path", "User") -cne $pathBefore) { throw "--no-modify-path changed User PATH" } diff --git a/crates/vp_global_cli/src/self_setup.rs b/crates/vp_global_cli/src/self_setup.rs index c7df91b322..8a3be5a531 100644 --- a/crates/vp_global_cli/src/self_setup.rs +++ b/crates/vp_global_cli/src/self_setup.rs @@ -229,7 +229,7 @@ async fn run(source: &Path, bundled: bool) -> Result { }; crate::install_dirs::prepare( dirs, - in_place || configured || active_binary.as_path().is_file(), + in_place || configured || std::fs::symlink_metadata(dirs.data.join("current")).is_ok(), )?; if deploy { tokio::fs::create_dir_all(version_dir.join("bin")).await?; From 03ddbdfc7ce1b84326afcf11fe5a8d7b685eb1b4 Mon Sep 17 00:00:00 2001 From: JongKyung Lee Date: Sun, 25 Oct 2026 17:22:01 +0900 Subject: [PATCH 4/5] fix(setup): allow installs into unowned nonempty directories Keep ownership checks focused on recursive removal. Setup may use an existing nonempty root without marking it as owned, so implode still refuses to delete it. Remove the unnecessary existing-install distinction. Verify that setup succeeds without claiming personal files and that implode preserves those files and shell configuration. Keep preflight coverage for invalid roots and update the doctor cache-directory snapshot. --- .../command_env_doctor_broken_config.md | 2 +- .../fixtures/command_implode/snapshots.toml | 5 +-- .../snapshots/implode_unowned_directory.md | 7 ++-- crates/vp_global_cli/src/commands/implode.rs | 2 +- crates/vp_global_cli/src/install_dirs.rs | 35 +++++++++---------- crates/vp_global_cli/src/self_setup.rs | 5 +-- 6 files changed, 27 insertions(+), 29 deletions(-) diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_env_doctor_broken_config/snapshots/command_env_doctor_broken_config.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_env_doctor_broken_config/snapshots/command_env_doctor_broken_config.md index b08ff5faf1..5ccaf33e0b 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_env_doctor_broken_config/snapshots/command_env_doctor_broken_config.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_env_doctor_broken_config/snapshots/command_env_doctor_broken_config.md @@ -20,7 +20,7 @@ VITE+ - The Unified Toolchain for the Web Installation ✓ Bin dir ~/.vite-plus/bin ✓ Data dir ~/.vite-plus - ✓ Cache dir ~/.vite-plus/cache (not created yet) + ✓ Cache dir ~/.vite-plus/cache ✓ Config dir ~/.vite-plus ✓ State dir ~/.vite-plus diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots.toml index 8164496b3d..fef98667bd 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots.toml +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots.toml @@ -34,14 +34,15 @@ name = "implode_unowned_directory" vp = "global" seed-runtime = false skip-platforms = ["windows"] -env = { VP_SKIP_DEPS_INSTALL = "1", VP_NODE_MANAGER = "no" } +env = { VP_SKIP_DEPS_INSTALL = "1", VP_NODE_MANAGER = "no", VP_SELF_SETUP_NO_MODIFY_PATH = "1" } steps = [ { argv = ["vpt", "mkdir", "external"], snapshot = false }, { argv = ["vpt", "cp", "$VP_HOME/bin/vp", "external/vp"], snapshot = false }, { argv = ["vpt", "chmod", "+x", "external/vp"], snapshot = false }, { argv = ["vpt", "write-file", "$HOME/Documents/personal.txt", "keep my documents\n"], snapshot = false }, { argv = ["vpt", "write-file", "$HOME/.bashrc", "# personal config\n# Vite+ bin\n. \"$HOME/Documents/env\"\n"], snapshot = false }, - { argv = ["./external/vp"], tty = false, envs = [["VP_HOME", "${HOME}/Documents"]], comment = "A fresh install cannot claim a nonempty personal directory", continue-on-failure = true }, + { argv = ["./external/vp"], tty = false, envs = [["VP_HOME", "${HOME}/Documents"]], comment = "Setup succeeds without claiming a nonempty personal directory", snapshot = false }, + ["vpt", "stat-file", "$HOME/Documents/current/bin/.vp-setup-complete", "--assert", "file"], { argv = ["vp", "implode", "--yes"], envs = [["VP_HOME", "${HOME}/Documents"]], comment = "Changing VP_HOME cannot authorize recursive deletion", continue-on-failure = true }, ["vpt", "stat-file", "$HOME/Documents/.vp-owned", "--assert", "missing"], ["vpt", "print-file", "$HOME/Documents/personal.txt"], diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_unowned_directory.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_unowned_directory.md index fd6027c05f..b760f1f383 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_unowned_directory.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_unowned_directory.md @@ -21,12 +21,13 @@ ## `VP_HOME=${HOME}/Documents ./external/vp` -A fresh install cannot claim a nonempty personal directory +Setup succeeds without claiming a nonempty personal directory -**Exit code:** 1 + +## `vpt stat-file $HOME/Documents/current/bin/.vp-setup-complete --assert file` ``` -error: Refusing to initialize /Documents: the directory is not empty and Vite+ ownership could not be verified. Choose an empty installation directory. +/Documents/current/bin/.vp-setup-complete: file ``` ## `VP_HOME=${HOME}/Documents vp implode --yes` diff --git a/crates/vp_global_cli/src/commands/implode.rs b/crates/vp_global_cli/src/commands/implode.rs index e80d6ad1eb..c32aef0bfc 100644 --- a/crates/vp_global_cli/src/commands/implode.rs +++ b/crates/vp_global_cli/src/commands/implode.rs @@ -1682,7 +1682,7 @@ mod tests { ("ZDOTDIR", Some(home.as_os_str())), ], |config| { - install_dirs::prepare(&config.dirs, false).unwrap(); + install_dirs::prepare(&config.dirs).unwrap(); std::fs::write(data.join("data.txt"), b"data").unwrap(); std::fs::write(normalized_cache.join("cache.txt"), b"cache").unwrap(); let profile = vt_str::format!( diff --git a/crates/vp_global_cli/src/install_dirs.rs b/crates/vp_global_cli/src/install_dirs.rs index fb78877058..6489721f4a 100644 --- a/crates/vp_global_cli/src/install_dirs.rs +++ b/crates/vp_global_cli/src/install_dirs.rs @@ -62,9 +62,9 @@ pub(crate) fn require_owned(roots: &[AbsolutePathBuf]) -> Result<(), Error> { Ok(()) } -/// Claim only empty or newly created roots. Legacy installs may remain unowned, -/// but updating them must never grant permission to delete their entire roots. -pub(crate) fn prepare(dirs: &VpDirs, existing_install: bool) -> Result<(), Error> { +/// Claim only empty or newly created roots. Installing into an unowned, nonempty +/// directory must never grant permission to delete its existing contents. +pub(crate) fn prepare(dirs: &VpDirs) -> Result<(), Error> { let mut empty = Vec::new(); // Preflight all roots before creating directories or ownership markers. for root in roots(dirs) { @@ -77,13 +77,7 @@ pub(crate) fn prepare(dirs: &VpDirs, existing_install: bool) -> Result<(), Error Err(error) => return Err(error.into()), }; if nonempty { - if existing_install { - continue; - } - return Err(Error::Other(vt_str::format!( - "Refusing to initialize {}: the directory is not empty and Vite+ ownership could not be verified. Choose an empty installation directory.", - root.as_path().display() - ))); + continue; } empty.push(root); } @@ -132,12 +126,12 @@ mod tests { fs::create_dir_all(&dirs.bin).unwrap(); fs::write(dirs.bin.join("other-tool"), b"keep").unwrap(); } - prepare(&dirs, false).unwrap(); + prepare(&dirs).unwrap(); for root in roots(&dirs) { assert!(is_owned(&root).unwrap()); } fs::write(dirs.data.join("payload"), b"vp").unwrap(); - prepare(&dirs, false).unwrap(); + prepare(&dirs).unwrap(); assert_eq!(fs::read(dirs.data.join("payload")).unwrap(), b"vp"); assert!(!dirs.bin.join(OWNERSHIP_MARKER).as_path().exists()); if split { @@ -148,21 +142,26 @@ mod tests { } #[test] - fn prepare_preflights_all_roots_and_never_claims_legacy_contents() { + fn prepare_preflights_all_roots_and_never_claims_existing_contents() { EnvConfig::scoped(|env| { let mut dirs = env.dirs.clone(); dirs.data = env.dirs.data.join("a-data"); dirs.config = env.dirs.data.join("b-config"); dirs.state = env.dirs.data.join("c-state"); dirs.cache = env.dirs.data.join("z-cache"); + fs::create_dir_all(&env.dirs.data).unwrap(); + fs::write(&dirs.cache, b"not a directory").unwrap(); + assert!(prepare(&dirs).is_err()); + for root in [&dirs.data, &dirs.config, &dirs.state] { + assert!(!root.as_path().exists()); + } + fs::remove_file(&dirs.cache).unwrap(); fs::create_dir_all(&dirs.cache).unwrap(); fs::write(dirs.cache.join("personal.txt"), b"keep").unwrap(); - assert!(prepare(&dirs, false).is_err()); + prepare(&dirs).unwrap(); for root in [&dirs.data, &dirs.config, &dirs.state] { - assert!(!root.as_path().exists()); + assert!(is_owned(root).unwrap()); } - prepare(&dirs, true).unwrap(); - assert!(is_owned(&dirs.data).unwrap()); assert!(!dirs.cache.join(OWNERSHIP_MARKER).as_path().exists()); assert_eq!(fs::read(dirs.cache.join("personal.txt")).unwrap(), b"keep"); assert!(require_owned(&roots(&dirs)).is_err()); @@ -172,7 +171,7 @@ mod tests { #[test] fn ownership_requires_a_regular_marker_file() { EnvConfig::scoped(|env| { - prepare(&env.dirs, false).unwrap(); + prepare(&env.dirs).unwrap(); let roots = roots(&env.dirs); let marker = env.dirs.cache.join(OWNERSHIP_MARKER); fs::remove_file(&marker).unwrap(); diff --git a/crates/vp_global_cli/src/self_setup.rs b/crates/vp_global_cli/src/self_setup.rs index 8a3be5a531..cc5bb635d3 100644 --- a/crates/vp_global_cli/src/self_setup.rs +++ b/crates/vp_global_cli/src/self_setup.rs @@ -227,10 +227,7 @@ async fn run(source: &Path, bundled: bool) -> Result { } else { version_dir.join("bin").join(VP_BINARY_NAME) }; - crate::install_dirs::prepare( - dirs, - in_place || configured || std::fs::symlink_metadata(dirs.data.join("current")).is_ok(), - )?; + crate::install_dirs::prepare(dirs)?; if deploy { tokio::fs::create_dir_all(version_dir.join("bin")).await?; install::clear_self_setup_marker(&version_dir).await?; From 4a3a71894bd7ce11ddafe86949fa6e8a98f04023 Mon Sep 17 00:00:00 2001 From: JongKyung Lee Date: Mon, 26 Oct 2026 00:57:24 +0900 Subject: [PATCH 5/5] fix(implode): support partial removal of legacy installations Identify legacy CLI version directories using matching package metadata and frozen installation receipts instead of claiming their parent roots. Preserve unverified data and report the paths left for manual inspection. Keep protected paths and unowned nested categories safe, and remove only verified shims from shared bin directories. Cover the legacy upgrade refresh handoff, partial cleanup, invalid receipts, and symlink boundaries. --- .../command_implode/legacy-upgrade.mjs | 168 ++++++++ .../fixtures/command_implode/snapshots.toml | 36 +- .../snapshots/implode_confirmation.md | 35 +- .../snapshots/implode_legacy_upgrade.md | 10 + .../snapshots/implode_legacy_upgrade_split.md | 10 + .../snapshots/implode_nested_unowned_cache.md | 31 ++ .../snapshots/implode_unowned_directory.md | 47 ++- .../command_self_setup_external_homebrew.md | 4 +- .../vp_global_cli/src/commands/env/setup.rs | 2 +- crates/vp_global_cli/src/commands/implode.rs | 265 ++++++++---- .../src/commands/implode/plan.rs | 377 ++++++++++++++++++ crates/vp_global_cli/src/install_dirs.rs | 25 +- crates/vp_setup/src/lockfile.rs | 9 +- 13 files changed, 885 insertions(+), 134 deletions(-) create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/legacy-upgrade.mjs create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_legacy_upgrade.md create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_legacy_upgrade_split.md create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_nested_unowned_cache.md create mode 100644 crates/vp_global_cli/src/commands/implode/plan.rs diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/legacy-upgrade.mjs b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/legacy-upgrade.mjs new file mode 100644 index 0000000000..7186f2e3c0 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/legacy-upgrade.mjs @@ -0,0 +1,168 @@ +import assert from 'node:assert/strict'; +import { spawnSync } from 'node:child_process'; +import { createHash } from 'node:crypto'; +import { + copyFileSync, + existsSync, + lstatSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + symlinkSync, + writeFileSync, +} from 'node:fs'; +import { dirname, join } from 'node:path'; + +// Model 1.4.0's records and its handoff to the new binary without registry downloads. +const windows = process.platform === 'win32'; +const binaryName = windows ? 'vp.exe' : 'vp'; +const sourceBin = join(process.env.VP_HOME, 'current/bin'); +const split = process.argv.includes('--split'); +assert.ok(!split || !windows, 'The split fixture requires XDG-isolated config and state roots.'); +const user = mkdtempSync(join(process.cwd(), 'legacy-user-')); +const data = join(user, split ? 'data' : '.vite-plus'); +const bin = split ? join(user, 'shared-bin') : join(data, 'bin'); +const cache = split ? join(user, 'cache') : join(data, 'cache'); +const config = split ? join(user, '.config/vite-plus') : data; +const state = split ? join(user, '.local/state/vite-plus') : data; +const env = { + ...process.env, + HOME: user, + USERPROFILE: user, + ZDOTDIR: user, + XDG_CONFIG_HOME: join(user, '.config'), + XDG_DATA_HOME: join(user, '.local/share'), + XDG_STATE_HOME: join(user, '.local/state'), + APPDATA: join(user, 'AppData/Roaming'), + LOCALAPPDATA: join(user, 'AppData/Local'), + VP_SELF_SETUP_NO_MODIFY_PATH: '1', + VP_SKIP_DEPS_INSTALL: '1', + CI: '1', + NO_COLOR: '1', +}; +for (const key of ['VP_HOME', 'VP_DATA_DIR', 'VP_CACHE_DIR', 'VP_BIN_DIR']) delete env[key]; +Object.assign( + env, + split ? { VP_DATA_DIR: data, VP_CACHE_DIR: cache, VP_BIN_DIR: bin } : { VP_HOME: data }, +); + +function write(path, contents) { + mkdirSync(dirname(path), { recursive: true }); + writeFileSync(path, contents); +} + +function version(name, active = false) { + const directory = join(data, name); + const packageVersion = name.split('+')[0]; + const lockfile = `lockfileVersion: '9.0'\nimporters:\n .:\n dependencies:\n vite-plus:\n specifier: ${packageVersion}\n version: ${packageVersion}\n`; + write(join(directory, 'bin/vp-install-lock.yaml'), lockfile); + write( + join(directory, 'node_modules/.vp-install-complete'), + createHash('sha256').update(lockfile).digest(), + ); + write( + join(directory, 'package.json'), + JSON.stringify({ + name: 'vp-global', + version: packageVersion, + private: true, + dependencies: { 'vite-plus': packageVersion }, + }), + ); + write( + join(directory, 'node_modules/vite-plus/package.json'), + JSON.stringify({ name: 'vite-plus', version: packageVersion }), + ); + if (active) { + copyFileSync(join(sourceBin, binaryName), join(directory, 'bin', binaryName)); + if (windows) copyFileSync(join(sourceBin, 'vp-shim.exe'), join(directory, 'bin/vp-shim.exe')); + } else { + write(join(directory, 'bin', binaryName), 'Historical binaries must not be executed.\n'); + write(join(directory, 'bin/.vp-setup-complete'), ''); + } + return directory; +} + +function run(binary, args) { + const result = spawnSync(binary, args, { env, encoding: 'utf8', timeout: 30000 }); + assert.equal(result.status, 0, `${result.error ?? ''}\n${result.stdout}\n${result.stderr}`); + return result.stdout + result.stderr; +} + +try { + const old = version('1.4.0'); + const forced = version('1.4.0+force.123.456'); + const current = version('1.5.0', true); + const active = join(current, 'bin', binaryName); + symlinkSync(current, join(data, 'current'), windows ? 'junction' : 'dir'); + const retained = [ + [join(data, 'personal.txt'), 'personal data'], + [ + join(data, 'js_runtime/node/22.0.0', windows ? 'node.exe' : 'bin/node'), + 'legacy runtime without an ownership receipt', + ], + [join(data, 'package_manager/pnpm/10.0.0/pnpm'), 'legacy package manager'], + [join(cache, 'downloads'), 'legacy cache'], + [join(config, 'config.json'), JSON.stringify({ nodeShimMode: 'system_first' })], + [join(state, 'legacy-state'), 'legacy state'], + [join(bin, 'unrelated-tool'), 'another tool'], + ]; + for (const [path, contents] of retained) write(path, contents); + const incomplete = version('1.3.0'); + rmSync(join(incomplete, 'node_modules/.vp-install-complete')); + const unlocked = version('1.3.1'); + write(join(unlocked, 'node_modules/.vp-install-complete'), 'unlocked'); + const mismatched = version('1.3.2'); + write(join(mismatched, 'bin/vp-install-lock.yaml'), 'different lockfile'); + + // This is the final command issued by the 1.4.0 upgrader after switching current. + run(active, ['env', 'setup', '--refresh']); + for (const root of new Set([data, cache, config, state])) { + assert.equal(existsSync(join(root, '.vp-owned')), false, `Upgrade claimed ${root}`); + } + assert.equal(existsSync(join(current, 'bin/.vp-setup-complete')), true); + console.log('Upgrade refresh completes without claiming existing roots.'); + const preservedContents = retained.map(([path]) => [path, readFileSync(path)]); + const profile = join(env.XDG_CONFIG_HOME, 'nushell/config.nu'); + write( + profile, + `# personal config\n# Vite+ bin\nsource "${config.replaceAll('\\', '/')}/env.nu"\n`, + ); + const output = run(active, ['implode', '--yes']); + for (const directory of [old, forced, current]) + assert.equal(existsSync(directory), false, directory); + assert.equal(lstatSync(join(data, 'current'), { throwIfNoEntry: false }), undefined); + assert.equal(lstatSync(join(bin, binaryName), { throwIfNoEntry: false }), undefined); + assert.equal( + lstatSync(join(data, 'fallback-bin', windows ? 'node.exe' : 'node'), { throwIfNoEntry: false }), + undefined, + ); + assert.equal(readFileSync(profile, 'utf8'), '# personal config\n'); + console.log('Verified current and historical CLI versions, links, and shims are removed.'); + for (const [path, contents] of preservedContents) + assert.deepEqual(readFileSync(path), contents, path); + for (const directory of [incomplete, unlocked, mismatched]) + assert.equal(existsSync(directory), true); + assert.match(output, /preserved to protect unverified data/); + for (const name of [ + 'personal.txt', + 'js_runtime', + 'package_manager', + 'cache', + 'config.json', + 'legacy-state', + '1.3.0', + '1.3.1', + '1.3.2', + ]) + assert.ok(output.includes(name), output); + console.log( + 'Unverified runtimes, package managers, cache, config, and incomplete versions remain and are reported.', + ); + console.log( + 'Personal files and unrelated tools are unchanged; only the matching shell entry is removed.', + ); +} finally { + rmSync(user, { recursive: true, force: true, maxRetries: 10, retryDelay: 100 }); +} diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots.toml index fef98667bd..79bb293646 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots.toml +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots.toml @@ -2,10 +2,8 @@ name = "implode_confirmation" vp = "global" seed-runtime = false +env = { VP_SELF_SETUP_NO_MODIFY_PATH = "1" } steps = [ - { argv = ["vpt", "rm", "$VP_HOME/cache/.vp-owned"], snapshot = false }, - { argv = ["vp", "implode", "--yes"], comment = "A nested cache needs its own ownership marker before any deletion", continue-on-failure = true }, - { argv = ["vpt", "write-file", "$VP_HOME/cache/.vp-owned", "vite-plus-owned-directory\n"], snapshot = false }, { argv = ["vp", "implode"], comment = "The old confirmation word leaves the installation intact", interactions = [ { expect-milestone = "text:implode:ready" }, { write-line = "uninstall" }, @@ -34,17 +32,45 @@ name = "implode_unowned_directory" vp = "global" seed-runtime = false skip-platforms = ["windows"] -env = { VP_SKIP_DEPS_INSTALL = "1", VP_NODE_MANAGER = "no", VP_SELF_SETUP_NO_MODIFY_PATH = "1" } +env = { VP_SKIP_DEPS_INSTALL = "1", VP_VERSION = "0.0.0-test", VP_NODE_MANAGER = "no", VP_SELF_SETUP_NO_MODIFY_PATH = "1" } steps = [ { argv = ["vpt", "mkdir", "external"], snapshot = false }, { argv = ["vpt", "cp", "$VP_HOME/bin/vp", "external/vp"], snapshot = false }, { argv = ["vpt", "chmod", "+x", "external/vp"], snapshot = false }, { argv = ["vpt", "write-file", "$HOME/Documents/personal.txt", "keep my documents\n"], snapshot = false }, { argv = ["vpt", "write-file", "$HOME/.bashrc", "# personal config\n# Vite+ bin\n. \"$HOME/Documents/env\"\n"], snapshot = false }, + { argv = ["vp", "implode", "--yes"], envs = [["VP_HOME", "${HOME}/Documents"]], comment = "An unrelated override fails before touching files or profiles", continue-on-failure = true }, + ["vpt", "print-file", "$HOME/.bashrc"], { argv = ["./external/vp"], tty = false, envs = [["VP_HOME", "${HOME}/Documents"]], comment = "Setup succeeds without claiming a nonempty personal directory", snapshot = false }, ["vpt", "stat-file", "$HOME/Documents/current/bin/.vp-setup-complete", "--assert", "file"], - { argv = ["vp", "implode", "--yes"], envs = [["VP_HOME", "${HOME}/Documents"]], comment = "Changing VP_HOME cannot authorize recursive deletion", continue-on-failure = true }, + { argv = ["vp", "implode", "--yes"], envs = [["VP_HOME", "${HOME}/Documents"]], comment = "Only the newly owned cache is removed; the unmarked parent is preserved" }, + ["vpt", "stat-file", "$HOME/Documents/cache", "--assert", "missing"], ["vpt", "stat-file", "$HOME/Documents/.vp-owned", "--assert", "missing"], ["vpt", "print-file", "$HOME/Documents/personal.txt"], ["vpt", "print-file", "$HOME/.bashrc"], ] + +[[case]] +name = "implode_nested_unowned_cache" +vp = "global" +seed-runtime = false +env = { VP_SELF_SETUP_NO_MODIFY_PATH = "1" } +steps = [ + { argv = ["vpt", "rm", "$VP_HOME/cache/.vp-owned"], snapshot = false }, + { argv = ["vpt", "write-file", "$VP_HOME/cache/personal.txt", "keep\n"], snapshot = false }, + { argv = ["vp", "implode", "--yes"], comment = "A marked ancestor cannot authorize removal of an unmarked category", snapshot = false }, + ["vpt", "stat-file", "$VP_HOME", "--assert", "dir"], + ["vpt", "print-file", "$VP_HOME/cache/personal.txt"], + ["vpt", "stat-file", "$VP_HOME/bin", "--assert", "dir"], +] + +[[case]] +name = "implode_legacy_upgrade" +vp = "global" +steps = [["node", "legacy-upgrade.mjs"]] + +[[case]] +name = "implode_legacy_upgrade_split" +vp = "global" +skip-platforms = ["windows"] +steps = [["node", "legacy-upgrade.mjs", "--split"]] diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_confirmation.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_confirmation.md index 93e3d0d209..1ce7d9eae9 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_confirmation.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_confirmation.md @@ -1,22 +1,5 @@ # implode_confirmation -## `vpt rm $VP_HOME/cache/.vp-owned` - - -## `vp implode --yes` - -A nested cache needs its own ownership marker before any deletion - -**Exit code:** 1 - -``` -error: Refusing to remove /.vite-plus/cache: Vite+ directory ownership could not be verified. Unmarked directories, including older installations, must be inspected and removed manually. -``` - -## `vpt write-file $VP_HOME/cache/.vp-owned 'vite-plus-owned-directory -'` - - ## `vp implode` The old confirmation word leaves the installation intact @@ -28,7 +11,8 @@ warn: This will completely remove vite-plus from your system! Directories to remove: - /.vite-plus - Shim files to remove from: /.vite-plus/bin + Verified shims to remove from: /.vite-plus/bin + Verified shims to remove from: /.vite-plus/fallback-bin Type boom to confirm: ``` @@ -40,7 +24,8 @@ warn: This will completely remove vite-plus from your system! Directories to remove: - /.vite-plus - Shim files to remove from: /.vite-plus/bin + Verified shims to remove from: /.vite-plus/bin + Verified shims to remove from: /.vite-plus/fallback-bin Type boom to confirm: uninstall @@ -64,7 +49,8 @@ warn: This will completely remove vite-plus from your system! Directories to remove: - /.vite-plus - Shim files to remove from: /.vite-plus/bin + Verified shims to remove from: /.vite-plus/bin + Verified shims to remove from: /.vite-plus/fallback-bin Type boom to confirm: ``` @@ -76,7 +62,8 @@ warn: This will completely remove vite-plus from your system! Directories to remove: - /.vite-plus - Shim files to remove from: /.vite-plus/bin + Verified shims to remove from: /.vite-plus/bin + Verified shims to remove from: /.vite-plus/fallback-bin Type boom to confirm: implode @@ -100,7 +87,8 @@ warn: This will completely remove vite-plus from your system! Directories to remove: - /.vite-plus - Shim files to remove from: /.vite-plus/bin + Verified shims to remove from: /.vite-plus/bin + Verified shims to remove from: /.vite-plus/fallback-bin Type boom to confirm: ``` @@ -112,7 +100,8 @@ warn: This will completely remove vite-plus from your system! Directories to remove: - /.vite-plus - Shim files to remove from: /.vite-plus/bin + Verified shims to remove from: /.vite-plus/bin + Verified shims to remove from: /.vite-plus/fallback-bin Type boom to confirm: diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_legacy_upgrade.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_legacy_upgrade.md new file mode 100644 index 0000000000..06218bcca0 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_legacy_upgrade.md @@ -0,0 +1,10 @@ +# implode_legacy_upgrade + +## `node legacy-upgrade.mjs` + +``` +Upgrade refresh completes without claiming existing roots. +Verified current and historical CLI versions, links, and shims are removed. +Unverified runtimes, package managers, cache, config, and incomplete versions remain and are reported. +Personal files and unrelated tools are unchanged; only the matching shell entry is removed. +``` diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_legacy_upgrade_split.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_legacy_upgrade_split.md new file mode 100644 index 0000000000..04dea42929 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_legacy_upgrade_split.md @@ -0,0 +1,10 @@ +# implode_legacy_upgrade_split + +## `node legacy-upgrade.mjs --split` + +``` +Upgrade refresh completes without claiming existing roots. +Verified current and historical CLI versions, links, and shims are removed. +Unverified runtimes, package managers, cache, config, and incomplete versions remain and are reported. +Personal files and unrelated tools are unchanged; only the matching shell entry is removed. +``` diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_nested_unowned_cache.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_nested_unowned_cache.md new file mode 100644 index 0000000000..9b53ccb286 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_nested_unowned_cache.md @@ -0,0 +1,31 @@ +# implode_nested_unowned_cache + +## `vpt rm $VP_HOME/cache/.vp-owned` + + +## `vpt write-file $VP_HOME/cache/personal.txt 'keep +'` + + +## `vp implode --yes` + +A marked ancestor cannot authorize removal of an unmarked category + + +## `vpt stat-file $VP_HOME --assert dir` + +``` +/.vite-plus: dir +``` + +## `vpt print-file $VP_HOME/cache/personal.txt` + +``` +keep +``` + +## `vpt stat-file $VP_HOME/bin --assert dir` + +``` +/.vite-plus/bin: dir +``` diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_unowned_directory.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_unowned_directory.md index b760f1f383..354e24ac77 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_unowned_directory.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_unowned_directory.md @@ -19,6 +19,24 @@ '` +## `VP_HOME=${HOME}/Documents vp implode --yes` + +An unrelated override fails before touching files or profiles + +**Exit code:** 1 + +``` +error: Refusing to remove /Documents: Vite+ installation ownership could not be verified. Check your directory overrides and inspect unmarked directories manually. +``` + +## `vpt print-file $HOME/.bashrc` + +``` +# personal config +# Vite+ bin +. "$HOME/Documents/env" +``` + ## `VP_HOME=${HOME}/Documents ./external/vp` Setup succeeds without claiming a nonempty personal directory @@ -32,12 +50,33 @@ Setup succeeds without claiming a nonempty personal directory ## `VP_HOME=${HOME}/Documents vp implode --yes` -Changing VP_HOME cannot authorize recursive deletion +Only the newly owned cache is removed; the unmarked parent is preserved -**Exit code:** 1 +``` +✓ Cleaned ~/.bashrc +✓ Removed /Documents/cache + +✓ Vite+ removed its verified files and shell entries from your system. +note: Some paths were preserved to protect unverified data. Inspect them before removing anything: + /Documents/0.0.0-test + /Documents/bin + /Documents/config.json + /Documents/current + /Documents/env + /Documents/env.fish + /Documents/env.nu + /Documents/env.ps1 + /Documents/fallback-bin + /Documents/personal.txt + /Documents/self-setup + /Documents/vp.ps1 +note: Restart your terminal to apply shell changes. +``` + +## `vpt stat-file $HOME/Documents/cache --assert missing` ``` -error: Refusing to remove /Documents: Vite+ directory ownership could not be verified. Unmarked directories, including older installations, must be inspected and removed manually. +/Documents/cache: missing ``` ## `vpt stat-file $HOME/Documents/.vp-owned --assert missing` @@ -56,6 +95,4 @@ keep my documents ``` # personal config -# Vite+ bin -. "$HOME/Documents/env" ``` diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/snapshots/command_self_setup_external_homebrew.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/snapshots/command_self_setup_external_homebrew.md index fc840de00b..bdad2cd215 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/snapshots/command_self_setup_external_homebrew.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/snapshots/command_self_setup_external_homebrew.md @@ -109,10 +109,10 @@ brew-prefix/bin/.vp-setup-complete: missing Cleanup removes user data and leaves the Homebrew package installed ``` -✓ Vite+ removed 3 shims from /home/bin +✓ Vite+ removed 14 shims. ✓ Removed /home -✓ Vite+ removed its managed files and shell entries from your system. +✓ Vite+ removed its verified files and shell entries from your system. note: The Homebrew package remains installed. Run `brew uninstall vite-plus` to remove it. note: To run `vp` again, restart your terminal or run `hash -r` in Bash. The remaining Homebrew package will start setup again. note: Restart your terminal to apply shell changes. diff --git a/crates/vp_global_cli/src/commands/env/setup.rs b/crates/vp_global_cli/src/commands/env/setup.rs index bdcf0303f4..176ee3691d 100644 --- a/crates/vp_global_cli/src/commands/env/setup.rs +++ b/crates/vp_global_cli/src/commands/env/setup.rs @@ -1188,7 +1188,7 @@ exit $LASTEXITCODE // cmd.exe wrapper for `vp env use` (cmd.exe cannot define shell functions). // Users run `vp-use 24` in cmd.exe instead of `vp env use 24`. #[cfg(windows)] -fn vp_use_cmd_content(config: &vp_shared::EnvConfig) -> String { +pub(crate) fn vp_use_cmd_content(config: &vp_shared::EnvConfig) -> String { let vp_exe = config.dirs.data.join("current").join("bin").join("vp.exe"); let mut exports: Vec<_> = config.dir_envs.iter().collect(); exports.sort_unstable_by_key(|(name, _)| *name); diff --git a/crates/vp_global_cli/src/commands/implode.rs b/crates/vp_global_cli/src/commands/implode.rs index c32aef0bfc..806a05d83b 100644 --- a/crates/vp_global_cli/src/commands/implode.rs +++ b/crates/vp_global_cli/src/commands/implode.rs @@ -1,10 +1,15 @@ -//! `vp implode` — completely remove vp and all its data from this system. +//! `vp implode` removes verified Vite+ artifacts and preserves unverified data. + +#![warn(clippy::print_stdout)] + +mod plan; #[cfg(any(windows, test))] use std::path::{Path, PathBuf}; use std::{io::Write, process::ExitStatus}; use console::style; +use plan::RemovalPlan; use rustc_hash::FxHashSet; use vp_shared::output; use vt_path::AbsolutePathBuf; @@ -14,7 +19,6 @@ use crate::{ cli::exit_status, commands::{ env::setup::shim_filename, - global::install::is_vp_shim_target, shell::{ALL_SHELL_PROFILES, ShellProfileKind, abbreviate_home_path, resolve_profile_path}, }, error::Error, @@ -67,16 +71,15 @@ pub fn execute(yes: bool) -> Result { let dirs = &env_config.dirs; let homebrew = crate::homebrew::owns_current_exe(); - // Build a unique set of Vite+-owned roots. In a single-root layout, data, - // config, and state use the same directory. Cache is inside that directory. - // Removing each category separately could remove the same path twice. + // In a single-root layout, data, config, and state share a directory. + // Keep the nested cache root until its ownership has been checked separately. // // The default Unix `` is under ``, so removal of `` also // removes it. Never remove a separately resolved `` because a bin from // an explicit override group can be shared. Remove only Vite+-owned shims. - let mut delete_set = install_dirs::roots(dirs); + let roots = install_dirs::roots(dirs); - if !delete_set.iter().any(|root| root.as_path().exists()) { + if !roots.iter().any(|root| root.as_path().exists()) { if homebrew { output::info("No Vite+-managed data directories exist."); print_homebrew_uninstall_notice(); @@ -88,10 +91,21 @@ pub fn execute(yes: bool) -> Result { // Validate every deletion target before changing profiles, PATH, or files, even with --yes. let user_home = &env_config.user_home; - validate_delete_roots(&delete_set, user_home)?; - install_dirs::require_owned(&delete_set)?; - // Sorted roots can be collapsed only after every category has passed validation. - delete_set.dedup_by(|child, parent| child.as_path().starts_with(parent.as_path())); + // Resolving symlinks before `..` can differ from lexical normalization. + validate_delete_roots( + &[dirs.data.clone(), dirs.cache.clone(), dirs.config.clone(), dirs.state.clone()], + user_home, + )?; + validate_delete_roots(&roots, user_home)?; + let plan = RemovalPlan::new(dirs, &roots)?; + validate_delete_roots(&plan.directories, user_home)?; + let shims = collect_shim_files(dirs, &plan.binary_targets); + if plan.directories.is_empty() && shims.is_empty() { + return Err(Error::Other(vt_str::format!( + "Refusing to remove {}: Vite+ installation ownership could not be verified. Check your directory overrides and inspect unmarked directories manually.", + dirs.data.as_path().display() + ))); + } let source_matcher = VitePlusSourceMatcher::new(&dirs.config, user_home); @@ -99,7 +113,7 @@ pub fn execute(yes: bool) -> Result { let affected_profiles = collect_affected_profiles(user_home, &source_matcher); // Confirmation - if !yes && !confirm_implode(&delete_set, &dirs.bin, &affected_profiles)? { + if !yes && !confirm_implode(&plan, dirs, &affected_profiles)? { return Ok(exit_status(0)); } @@ -107,7 +121,7 @@ pub fn execute(yes: bool) -> Result { // running executable before changing any path because the data root can // contain that executable. #[cfg(windows)] - let cleanup_source = match prepare_deferred_delete_source(&delete_set) { + let cleanup_source = match prepare_deferred_delete_source(&plan.directories) { Ok(source) => source, Err(e) => { output::error(&vt_str::format!("Failed to prepare the removal helper: {e}")); @@ -118,9 +132,9 @@ pub fn execute(yes: bool) -> Result { // Clean shell profiles with the stored content. Do not read them again. clean_affected_profiles(&affected_profiles, &source_matcher); - // Remove Windows PATH entry + // Honor the installer's PATH opt-out during isolated installation and removal tests. #[cfg(windows)] - { + if std::env::var(vp_shared::env_vars::VP_SELF_SETUP_NO_MODIFY_PATH).as_deref() != Ok("1") { if let Err(e) = remove_windows_path_entry(&dirs.bin) { output::warn(&vt_str::format!("Vite+ could not clean the Windows PATH: {e}")); } else { @@ -128,10 +142,15 @@ pub fn execute(yes: bool) -> Result { } } - // Remove vp-owned shim files from the (potentially shared) bin directory, - // then the owned roots. - remove_shim_files(dirs); - for root in &delete_set { + remove_shim_files(dirs, &shims); + if let Some(current) = &plan.current_link { + #[cfg(unix)] + std::fs::remove_file(current)?; + #[cfg(windows)] + std::fs::remove_dir(current)?; + output::success(&vt_str::format!("Removed {}", current.as_path().display())); + } + for root in &plan.directories { if root.as_path().exists() { #[cfg(unix)] remove_vite_plus_dir(root)?; @@ -141,7 +160,16 @@ pub fn execute(yes: bool) -> Result { } output::raw(""); - output::success("Vite+ removed its managed files and shell entries from your system."); + output::success("Vite+ removed its verified files and shell entries from your system."); + report_preserved_paths(&plan.preserved_roots); + if dirs.bin.as_path().exists() + && !plan.preserved_roots.iter().any(|root| dirs.bin.as_path().starts_with(root.as_path())) + { + output::note(&vt_str::format!( + "Shared bin directory preserved: {}", + dirs.bin.as_path().display() + )); + } if homebrew { print_homebrew_uninstall_notice(); } @@ -150,6 +178,31 @@ pub fn execute(yes: bool) -> Result { Ok(exit_status(0)) } +fn report_preserved_paths(roots: &[AbsolutePathBuf]) { + let mut paths = Vec::new(); + for root in roots { + if !root.as_path().exists() { + continue; + } + match std::fs::read_dir(root).and_then(|entries| { + entries.map(|entry| entry.map(|entry| entry.path())).collect::, _>>() + }) { + Ok(entries) if !entries.is_empty() => paths.extend(entries), + _ => paths.push(root.as_path().to_path_buf()), + } + } + paths.sort(); + paths.dedup(); + if !paths.is_empty() { + output::note( + "Some paths were preserved to protect unverified data. Inspect them before removing anything:", + ); + for path in paths { + output::raw(&vt_str::format!(" {}", path.display())); + } + } +} + fn print_homebrew_uninstall_notice() { output::note( "The Homebrew package remains installed. Run `brew uninstall vite-plus` to remove it.", @@ -159,35 +212,67 @@ fn print_homebrew_uninstall_notice() { ); } -/// Remove the shim files vite-plus owns from the bin directory. -/// -/// Do not remove the bin directory directly because a bin from an explicit -/// override group can be shared with other tools. Removal of the default Unix -/// `` root also removes its bin directory. -/// -/// Get package-shim names from `/bins/*.json`. Also check `vp` and the -/// default environment shims because those files are not in the metadata. -/// Remove a Unix candidate only if it links to this install's `vp`. Remove a -/// Windows candidate only if Vite+ created the trampoline. -fn remove_shim_files(dirs: &vp_shared::VpDirs) { +/// Shared bin directories are never removal units. Identify each shim separately. +fn collect_shim_files( + dirs: &vp_shared::VpDirs, + binary_targets: &[AbsolutePathBuf], +) -> Vec { let mut names = recorded_bin_shim_names(dirs); names.insert(shim_filename("vp")); names.extend(DEFAULT_SHIM_TOOLS.iter().map(|tool| shim_filename(tool))); #[cfg(windows)] names.insert("vp-use.cmd".to_string()); + let mut paths = Vec::new(); + for bin in [&dirs.bin, &dirs.fallback_bin()] { + for name in &names { + let path = bin.join(name); + let owned = match std::fs::read_link(&path) { + Ok(target) => { + let target = lexical_path(&bin.as_path().join(target)); + binary_targets.iter().any(|binary| { + target == lexical_path(binary.as_path()) + || same_file::is_same_file(&path, binary).unwrap_or(false) + }) + } + Err(_) => { + #[cfg(windows)] + { + dirs.owns_windows_trampoline(path.as_path()) + || (name == "vp-use.cmd" + && std::fs::read_to_string(&path).is_ok_and(|content| { + content + == crate::commands::env::setup::vp_use_cmd_content( + &vp_shared::EnvConfig::get(), + ) + })) + } + #[cfg(not(windows))] + { + false + } + } + }; + if owned { + paths.push(path); + } + } + } + paths.sort(); + paths.dedup(); + paths +} + +fn remove_shim_files(dirs: &vp_shared::VpDirs, paths: &[AbsolutePathBuf]) { let mut removed = 0; #[cfg(windows)] let mut scheduled = 0; - for name in names { - let path = dirs.bin.join(&name); - if !is_vp_shim_target(&path) { - continue; - } - let pointer = std::path::Path::new(&name) - .file_stem() - .and_then(|stem| stem.to_str()) - .map(|stem| dirs.bin.join(vp_shared::shim_pointer_file_name(stem))); + for path in paths { + let name = path.as_path().file_name().unwrap().to_string_lossy(); + let pointer = (cfg!(windows) && dirs.owns_windows_trampoline(path.as_path())).then(|| { + AbsolutePathBuf::new(path.as_path().with_extension(vp_shared::SHIM_POINTER_EXTENSION)) + .unwrap() + }); match std::fs::remove_file(path.as_path()) { Ok(()) => { removed += 1; @@ -221,17 +306,15 @@ fn remove_shim_files(dirs: &vp_shared::VpDirs) { } if removed > 0 { output::success(&vt_str::format!( - "Vite+ removed {removed} shim{} from {}", - if removed == 1 { "" } else { "s" }, - dirs.bin.as_path().display() + "Vite+ removed {removed} shim{}.", + if removed == 1 { "" } else { "s" } )); } #[cfg(windows)] if scheduled > 0 { output::success(&vt_str::format!( - "Vite+ scheduled removal of {scheduled} locked shim{} from {}", - if scheduled == 1 { "" } else { "s" }, - dirs.bin.as_path().display() + "Vite+ scheduled removal of {scheduled} locked shim{}.", + if scheduled == 1 { "" } else { "s" } )); } } @@ -320,8 +403,8 @@ fn collect_affected_profiles( /// Show confirmation prompt and require the user to type "boom". /// Returns `Ok(true)` if confirmed, `Ok(false)` if aborted. fn confirm_implode( - delete_set: &[AbsolutePathBuf], - bin_dir: &vt_path::AbsolutePath, + plan: &RemovalPlan, + dirs: &vp_shared::VpDirs, affected_profiles: &[AffectedProfile], ) -> Result { if !vp_shared::is_stdin_terminal() { @@ -335,15 +418,32 @@ fn confirm_implode( output::warn( "This will remove Vite+-managed data, shims, and shell entries. The Homebrew package will remain installed.", ); + } else if plan.preserved_roots.iter().any(|root| root.as_path().exists()) { + output::warn( + "This will remove verified Vite+ files and shell entries. Unverified data will be preserved.", + ); } else { output::warn("This will completely remove vite-plus from your system!"); } output::raw(""); output::raw(" Directories to remove:"); - for root in delete_set { + for root in &plan.directories { output::raw(&vt_str::format!(" - {}", root.as_path().display())); } - output::raw(&vt_str::format!(" Shim files to remove from: {}", bin_dir.as_path().display())); + if let Some(current) = &plan.current_link { + output::raw(&vt_str::format!(" Current link to remove: {}", current.as_path().display())); + } + for bin in [&dirs.bin, &dirs.fallback_bin()] { + output::raw(&vt_str::format!( + " Verified shims to remove from: {}", + bin.as_path().display() + )); + } + for root in &plan.preserved_roots { + if root.as_path().exists() { + output::raw(&vt_str::format!(" Root to preserve: {}", root.as_path().display())); + } + } if !affected_profiles.is_empty() { output::raw(" Shell profiles to clean:"); for profile in affected_profiles { @@ -1599,7 +1699,11 @@ mod tests { let profile = "# Vite+ bin\n. \"$HOME/env\"\n"; std::fs::write(home.join(".bashrc"), profile).unwrap(); - for root in [&home, &home.join("child/.."), &home.join("..")] { + let elsewhere = temp_dir.path().join("elsewhere"); + std::fs::create_dir_all(&elsewhere).unwrap(); + std::fs::create_dir(home.join("child")).unwrap(); + std::os::unix::fs::symlink(home.join("child"), elsewhere.join("route")).unwrap(); + for root in [&home, &home.join("child/.."), &home.join(".."), &elsewhere.join("route/..")] { vp_shared::EnvConfig::with_vars( [ (env_vars::VP_HOME, root.as_os_str()), @@ -1655,7 +1759,7 @@ mod tests { #[cfg(unix)] #[test] - fn execute_requires_each_owned_root_before_removing_split_installation() { + fn execute_preserves_unowned_cache_while_removing_owned_split_roots() { let temp_dir = tempfile::tempdir().unwrap(); let home = temp_dir.path().join("home"); let bin = temp_dir.path().join("bin"); @@ -1690,33 +1794,19 @@ mod tests { config.dirs.config.as_path().display() ); std::fs::write(home.join(".bashrc"), profile.as_bytes()).unwrap(); - for root in install_dirs::roots(&config.dirs) { - let marker = root.join(install_dirs::OWNERSHIP_MARKER); - let contents = std::fs::read(&marker).unwrap(); - std::fs::remove_file(&marker).unwrap(); - assert!(execute(true).unwrap_err().to_string().contains("ownership")); - assert_eq!(std::fs::read(data.join("data.txt")).unwrap(), b"data"); - assert_eq!( - std::fs::read(normalized_cache.join("cache.txt")).unwrap(), - b"cache" - ); - assert_eq!( - std::fs::read_to_string(home.join(".bashrc")).unwrap(), - profile.as_str() - ); - std::fs::write(&marker, contents).unwrap(); - } + let marker = normalized_cache.join(install_dirs::OWNERSHIP_MARKER); + std::fs::remove_file(&marker).unwrap(); let result = execute(true).unwrap(); assert!(result.success()); assert_eq!( std::fs::read_to_string(home.join(".bashrc")).unwrap(), "# personal config\n" ); - assert!(!data.exists(), "data root must be removed"); - assert!( - !normalized_cache.exists(), - "lexically distinct cache root must be removed" - ); + assert!(!data.exists(), "owned data root must be removed"); + assert_eq!(std::fs::read(normalized_cache.join("cache.txt")).unwrap(), b"cache"); + std::fs::write(&marker, b"").unwrap(); + assert!(execute(true).unwrap().success()); + assert!(!normalized_cache.exists(), "marked cache root must be removed"); }, ); } @@ -1744,7 +1834,17 @@ mod tests { std::fs::write(bins_dir.join("eslint.json").as_path(), "{}").unwrap(); std::os::unix::fs::symlink("/usr/bin/eslint", bin.join("eslint").as_path()).unwrap(); - remove_shim_files(&config.dirs); + let other = config.dirs.data.join("other/bin/vp"); + std::fs::create_dir_all(other.parent().unwrap()).unwrap(); + std::fs::write(&other, b"another vp").unwrap(); + std::os::unix::fs::symlink(&other, bin.join("pnpm")).unwrap(); + std::fs::create_dir_all(config.dirs.fallback_bin()).unwrap(); + let fallback = config.dirs.fallback_bin().join("node"); + std::os::unix::fs::symlink(&vp_target, &fallback).unwrap(); + let shims = collect_shim_files(&config.dirs, &[vp_target]); + remove_shim_files(&config.dirs, &shims); + assert!(!fallback.as_path().exists()); + assert!(bin.join("pnpm").as_path().exists(), "another installation's shim must remain"); assert!(bin.join("node").as_path().is_file(), "unrelated node binary must be kept"); assert!( @@ -1764,8 +1864,8 @@ mod tests { "recorded package shim that points at vp must be removed" ); assert!( - !bin.join("tsc.shim").as_path().exists(), - "sidecar next to a removed shim must be removed" + bin.join("tsc.shim").as_path().exists(), + "Unix symlinks do not grant ownership of neighboring files" ); }); } @@ -1781,7 +1881,18 @@ mod tests { std::fs::write(bin.join("node.exe").as_path(), b"owned-node").unwrap(); config.dirs.write_shim_pointer("node").unwrap(); - remove_shim_files(&config.dirs); + std::fs::write(bin.join("vp-use.cmd"), b"foreign script").unwrap(); + let shims = collect_shim_files(&config.dirs, &[]); + remove_shim_files(&config.dirs, &shims); + assert_eq!(std::fs::read(bin.join("vp-use.cmd")).unwrap(), b"foreign script"); + std::fs::write( + bin.join("vp-use.cmd"), + crate::commands::env::setup::vp_use_cmd_content(&config), + ) + .unwrap(); + let shims = collect_shim_files(&config.dirs, &[]); + remove_shim_files(&config.dirs, &shims); + assert!(!bin.join("vp-use.cmd").as_path().exists()); assert_eq!(std::fs::read(bin.join("vp.exe").as_path()).unwrap(), b"foreign-vp"); assert_eq!(std::fs::read(bin.join("unrelated.txt").as_path()).unwrap(), b"keep"); diff --git a/crates/vp_global_cli/src/commands/implode/plan.rs b/crates/vp_global_cli/src/commands/implode/plan.rs new file mode 100644 index 0000000000..ed662daea1 --- /dev/null +++ b/crates/vp_global_cli/src/commands/implode/plan.rs @@ -0,0 +1,377 @@ +//! Select removal units without granting ownership to a legacy installation's parent. + +use std::{fs, path::Path}; + +use vp_setup::lockfile::{INSTALL_LOCKFILE, INSTALL_RECEIPT, matches_install_receipt}; +use vp_shared::{VP_BINARY_NAME, VpDirs}; +use vt_path::AbsolutePathBuf; + +use crate::{error::Error, install_dirs}; + +pub(super) struct RemovalPlan { + pub directories: Vec, + pub current_link: Option, + pub binary_targets: Vec, + pub preserved_roots: Vec, +} + +impl RemovalPlan { + pub fn new(dirs: &VpDirs, roots: &[AbsolutePathBuf]) -> Result { + let mut owned = Vec::new(); + let mut unowned = Vec::new(); + for root in roots { + match fs::symlink_metadata(root) { + Ok(_) => {} + Err(error) if error.kind() == std::io::ErrorKind::NotFound => continue, + Err(error) => return Err(error.into()), + } + if install_dirs::is_owned(root)? { + owned.push(root.clone()); + } else { + let canonical = match dunce::canonicalize(root) { + Ok(path) => path, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + root.as_path().to_path_buf() + } + Err(error) => return Err(error.into()), + }; + unowned.push((root.clone(), canonical)); + } + } + let mut plan = Self { + directories: Vec::new(), + current_link: None, + binary_targets: vec![ + AbsolutePathBuf::new(dunce::canonicalize(std::env::current_exe()?)?) + .ok_or(Error::CliBinaryNotFound)?, + ], + preserved_roots: Vec::new(), + }; + if install_dirs::is_owned(&dirs.data)? { + plan.binary_targets.push(dirs.data.join("current/bin").join(VP_BINARY_NAME)); + } + for root in owned { + let canonical = dunce::canonicalize(&root)?; + // A marked ancestor must not bypass an unmarked category, including aliases. + if !unowned.iter().any(|(path, boundary)| { + path.as_path().starts_with(root.as_path()) || boundary.starts_with(&canonical) + }) { + plan.directories.push(root); + } + } + let current = dirs.data.join("current"); + let current_target = fs::symlink_metadata(¤t) + .ok() + .filter(|metadata| metadata.is_symlink()) + .and_then(|_| dunce::canonicalize(¤t).ok()); + if !covered(&dirs.data, &plan.directories) { + match fs::read_dir(&dirs.data) { + Ok(entries) => { + for entry in entries { + let entry = entry?; + if !entry.file_type()?.is_dir() { + continue; + } + let version_dir = dirs.data.join(entry.file_name()); + if verified_version(&version_dir)? { + plan.binary_targets.push(version_dir.join("bin").join(VP_BINARY_NAME)); + let canonical = dunce::canonicalize(&version_dir)?; + if !unowned.iter().any(|(path, boundary)| { + path.as_path().starts_with(version_dir.as_path()) + || boundary.starts_with(&canonical) + }) { + if current_target.as_ref() == Some(&canonical) { + plan.current_link = Some(current.clone()); + plan.binary_targets + .push(current.join("bin").join(VP_BINARY_NAME)); + } + plan.directories.push(version_dir); + } + } + } + } + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(error) => return Err(error.into()), + } + } + plan.directories.sort(); + plan.directories.dedup_by(|child, parent| child.as_path().starts_with(parent.as_path())); + plan.preserved_roots = + roots.iter().filter(|root| !covered(root, &plan.directories)).cloned().collect(); + plan.preserved_roots + .dedup_by(|child, parent| child.as_path().starts_with(parent.as_path())); + Ok(plan) + } +} + +fn covered(path: &AbsolutePathBuf, directories: &[AbsolutePathBuf]) -> bool { + directories.iter().any(|directory| path.as_path().starts_with(directory.as_path())) +} + +/// A usable executable or familiar layout alone is not an installation receipt. +fn verified_version(directory: &AbsolutePathBuf) -> Result { + let Some(name) = directory.as_path().file_name().and_then(|name| name.to_str()) else { + return Ok(false); + }; + if js_semver::Version::parse(name).is_err() { + return Ok(false); + } + let canonical = dunce::canonicalize(directory)?; + let Some(receipt) = read_contained_file(&canonical, INSTALL_RECEIPT)? else { + return Ok(false); + }; + let Some(lockfile) = read_contained_file(&canonical, &format!("bin/{INSTALL_LOCKFILE}"))? + else { + return Ok(false); + }; + if !matches_install_receipt(&lockfile, &receipt) { + return Ok(false); + } + let Some(wrapper) = read_contained_file(&canonical, "package.json")? else { + return Ok(false); + }; + let Some(package) = read_contained_file(&canonical, "node_modules/vite-plus/package.json")? + else { + return Ok(false); + }; + let (Ok(wrapper), Ok(package)) = ( + serde_json::from_slice::(&wrapper), + serde_json::from_slice::(&package), + ) else { + return Ok(false); + }; + let Some(version) = wrapper["version"].as_str() else { return Ok(false) }; + Ok(js_semver::Version::parse(version).is_ok() + && (name == version || name.starts_with(&format!("{version}+force."))) + && wrapper["name"] == "vp-global" + && wrapper["private"] == true + && wrapper["dependencies"]["vite-plus"] == version + && package["name"] == "vite-plus" + && package["version"] == version) +} + +// pnpm's package link may stay inside the version directory, but evidence from outside cannot authorize deletion. +fn read_contained_file(directory: &Path, relative: &str) -> Result>, Error> { + let path = directory.join(relative); + match fs::symlink_metadata(&path) { + Ok(metadata) if metadata.is_file() => {} + Ok(_) => return Ok(None), + Err(error) + if matches!( + error.kind(), + std::io::ErrorKind::NotFound | std::io::ErrorKind::NotADirectory + ) => + { + return Ok(None); + } + Err(error) => return Err(error.into()), + } + if !dunce::canonicalize(&path)?.starts_with(directory) { + return Ok(None); + } + Ok(Some(fs::read(path)?)) +} + +#[cfg(test)] +mod tests { + use super::*; + + const LOCKFILE: &[u8] = b"lockfileVersion: '9.0'\n"; + const RECEIPT: &[u8] = b"\xf0\xbc\xde\x46\x3f\xa2\x01\x48\x00\x15\xb9\xca\xa7\xdb\x20\x17\xd3\xc1\xb6\xca\x9c\x7e\x13\x3d\xf9\x55\x03\x8c\x54\x33\x3d\x48"; + + fn create_version(data: &AbsolutePathBuf, version: &str) -> AbsolutePathBuf { + let directory = data.join(version); + fs::create_dir_all(directory.join("bin")).unwrap(); + fs::create_dir_all(directory.join("node_modules/vite-plus")).unwrap(); + fs::write(directory.join("bin").join(INSTALL_LOCKFILE), LOCKFILE).unwrap(); + fs::write(directory.join(INSTALL_RECEIPT), RECEIPT).unwrap(); + fs::write( + directory.join("package.json"), + serde_json::json!({ + "name": "vp-global", "version": version, "private": true, + "dependencies": { "vite-plus": version }, + }) + .to_string(), + ) + .unwrap(); + fs::write( + directory.join("node_modules/vite-plus/package.json"), + serde_json::json!({ + "name": "vite-plus", "version": version, + }) + .to_string(), + ) + .unwrap(); + directory + } + + #[test] + fn versions_require_matching_metadata_and_a_frozen_install_receipt() { + vp_shared::EnvConfig::scoped(|env| { + let directory = create_version(&env.dirs.data, "1.4.0"); + assert!(verified_version(&directory).unwrap()); + let receipt = directory.join(INSTALL_RECEIPT); + fs::remove_file(&receipt).unwrap(); + fs::write(directory.join("bin/.vp-setup-complete"), b"").unwrap(); + assert!(!verified_version(&directory).unwrap(), "setup completion is not ownership"); + for invalid in [b"".as_slice(), b"unlocked", b"different digest"] { + fs::write(&receipt, invalid).unwrap(); + assert!(!verified_version(&directory).unwrap()); + } + fs::write(&receipt, RECEIPT).unwrap(); + let lockfile = directory.join("bin").join(INSTALL_LOCKFILE); + fs::remove_file(&lockfile).unwrap(); + assert!(!verified_version(&directory).unwrap()); + fs::write(&lockfile, b"changed lockfile").unwrap(); + assert!(!verified_version(&directory).unwrap()); + fs::write(&lockfile, LOCKFILE).unwrap(); + for (file, field, value) in [ + ("package.json", "/name", serde_json::json!("personal-project")), + ("package.json", "/version", serde_json::json!("1.3.0")), + ("package.json", "/private", serde_json::json!(false)), + ("package.json", "/dependencies/vite-plus", serde_json::json!("1.3.0")), + ("node_modules/vite-plus/package.json", "/name", serde_json::json!("other")), + ("node_modules/vite-plus/package.json", "/version", serde_json::json!("1.3.0")), + ] { + let path = directory.join(file); + let original = fs::read(&path).unwrap(); + let mut manifest: serde_json::Value = serde_json::from_slice(&original).unwrap(); + *manifest.pointer_mut(field).unwrap() = value; + fs::write(&path, manifest.to_string()).unwrap(); + assert!(!verified_version(&directory).unwrap(), "{file} {field}"); + fs::write(&path, b"malformed json").unwrap(); + assert!(!verified_version(&directory).unwrap()); + fs::write(&path, original).unwrap(); + } + let forced = env.dirs.data.join("1.4.0+force.123.456"); + fs::rename(&directory, &forced).unwrap(); + assert!(verified_version(&forced).unwrap()); + let wrong_version = env.dirs.data.join("2.0.0"); + fs::rename(&forced, &wrong_version).unwrap(); + assert!(!verified_version(&wrong_version).unwrap()); + }); + } + + #[test] + fn unowned_nested_categories_block_ancestor_and_version_removal() { + vp_shared::EnvConfig::scoped(|env| { + fs::write(env.dirs.data.join(install_dirs::OWNERSHIP_MARKER), b"").unwrap(); + fs::create_dir_all(&env.dirs.cache).unwrap(); + let version = create_version(&env.dirs.data, "1.4.0"); + let plan = RemovalPlan::new(&env.dirs, &install_dirs::roots(&env.dirs)).unwrap(); + assert_eq!(plan.directories, vec![version.clone()]); + let nested = version.join("personal"); + fs::create_dir(&nested).unwrap(); + fs::write(nested.join("notes"), b"keep").unwrap(); + for category in ["cache", "config", "state"] { + let mut dirs = env.dirs.clone(); + match category { + "cache" => dirs.cache = nested.clone(), + "config" => dirs.config = nested.clone(), + "state" => dirs.state = nested.clone(), + _ => unreachable!(), + } + let plan = RemovalPlan::new(&dirs, &install_dirs::roots(&dirs)).unwrap(); + assert!(plan.directories.is_empty(), "{category} must block recursive removal"); + } + assert_eq!(fs::read(nested.join("notes")).unwrap(), b"keep"); + }); + } + + #[cfg(unix)] + #[test] + fn verified_version_links_and_shims_do_not_require_a_surviving_binary() { + vp_shared::EnvConfig::scoped(|env| { + let version = create_version(&env.dirs.data, "1.4.0"); + let current = env.dirs.data.join("current"); + std::os::unix::fs::symlink(&version, ¤t).unwrap(); + fs::create_dir_all(&env.dirs.bin).unwrap(); + let shim = env.dirs.bin.join("vp"); + std::os::unix::fs::symlink("../current/bin/vp", &shim).unwrap(); + let plan = RemovalPlan::new(&env.dirs, &install_dirs::roots(&env.dirs)).unwrap(); + assert_eq!(plan.current_link, Some(current)); + assert_eq!(plan.directories, vec![version]); + assert_eq!( + super::super::collect_shim_files(&env.dirs, &plan.binary_targets), + vec![shim] + ); + }); + } + + #[cfg(unix)] + #[test] + fn current_link_to_an_owned_cache_is_not_a_verified_cli_link() { + vp_shared::EnvConfig::scoped(|env| { + fs::create_dir_all(&env.dirs.cache).unwrap(); + fs::write(env.dirs.cache.join(install_dirs::OWNERSHIP_MARKER), b"").unwrap(); + std::os::unix::fs::symlink(&env.dirs.cache, env.dirs.data.join("current")).unwrap(); + let plan = RemovalPlan::new(&env.dirs, &install_dirs::roots(&env.dirs)).unwrap(); + assert_eq!(plan.directories, vec![env.dirs.cache.clone()]); + assert!(plan.current_link.is_none()); + }); + } + + #[cfg(unix)] + #[test] + fn symlinked_evidence_and_category_aliases_do_not_expand_ownership() { + let temporary = tempfile::tempdir().unwrap(); + let outside_root = AbsolutePathBuf::new(temporary.path().to_path_buf()).unwrap(); + vp_shared::EnvConfig::scoped(|env| { + let version = create_version(&env.dirs.data, "1.4.0"); + let modules = version.join("node_modules"); + let package = modules.join(".pnpm/vite-plus/node_modules/vite-plus"); + fs::create_dir_all(package.parent().unwrap()).unwrap(); + fs::rename(modules.join("vite-plus"), &package).unwrap(); + std::os::unix::fs::symlink( + ".pnpm/vite-plus/node_modules/vite-plus", + modules.join("vite-plus"), + ) + .unwrap(); + assert!(verified_version(&version).unwrap(), "contained pnpm links are supported"); + + let personal = version.join("personal"); + fs::create_dir(&personal).unwrap(); + let mut dirs = env.dirs.clone(); + dirs.cache = outside_root.join("cache-alias"); + std::os::unix::fs::symlink(&personal, &dirs.cache).unwrap(); + assert!( + RemovalPlan::new(&dirs, &install_dirs::roots(&dirs)) + .unwrap() + .directories + .is_empty() + ); + + let outside = outside_root.join("outside-modules"); + fs::rename(&modules, &outside).unwrap(); + std::os::unix::fs::symlink(&outside, &modules).unwrap(); + assert!( + !verified_version(&version).unwrap(), + "borrowed evidence cannot authorize removal" + ); + fs::remove_file(&modules).unwrap(); + fs::rename(&outside, &modules).unwrap(); + + let outside_version = outside_root.join("outside-version"); + fs::rename(&version, &outside_version).unwrap(); + std::os::unix::fs::symlink(&outside_version, &version).unwrap(); + assert!( + RemovalPlan::new(&env.dirs, &install_dirs::roots(&env.dirs)) + .unwrap() + .directories + .is_empty() + ); + + fs::write(env.dirs.data.join(install_dirs::OWNERSHIP_MARKER), b"").unwrap(); + fs::create_dir(&outside).unwrap(); + std::os::unix::fs::symlink(&outside, &env.dirs.cache).unwrap(); + assert!( + RemovalPlan::new(&env.dirs, &install_dirs::roots(&env.dirs)) + .unwrap() + .directories + .is_empty(), + "a marked parent must preserve an unowned category link" + ); + assert!(outside_version.as_path().exists()); + }); + } +} diff --git a/crates/vp_global_cli/src/install_dirs.rs b/crates/vp_global_cli/src/install_dirs.rs index 6489721f4a..0031481fc3 100644 --- a/crates/vp_global_cli/src/install_dirs.rs +++ b/crates/vp_global_cli/src/install_dirs.rs @@ -42,7 +42,7 @@ pub(crate) fn roots(dirs: &VpDirs) -> Vec { roots } -fn is_owned(root: &AbsolutePathBuf) -> Result { +pub(crate) fn is_owned(root: &AbsolutePathBuf) -> Result { match fs::symlink_metadata(root.join(OWNERSHIP_MARKER)) { Ok(metadata) => Ok(metadata.is_file()), Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(false), @@ -50,18 +50,6 @@ fn is_owned(root: &AbsolutePathBuf) -> Result { } } -pub(crate) fn require_owned(roots: &[AbsolutePathBuf]) -> Result<(), Error> { - for root in roots { - if root.as_path().try_exists()? && !is_owned(root)? { - return Err(Error::Other(vt_str::format!( - "Refusing to remove {}: Vite+ directory ownership could not be verified. Unmarked directories, including older installations, must be inspected and removed manually.", - root.as_path().display() - ))); - } - } - Ok(()) -} - /// Claim only empty or newly created roots. Installing into an unowned, nonempty /// directory must never grant permission to delete its existing contents. pub(crate) fn prepare(dirs: &VpDirs) -> Result<(), Error> { @@ -164,7 +152,7 @@ mod tests { } assert!(!dirs.cache.join(OWNERSHIP_MARKER).as_path().exists()); assert_eq!(fs::read(dirs.cache.join("personal.txt")).unwrap(), b"keep"); - assert!(require_owned(&roots(&dirs)).is_err()); + assert!(!is_owned(&dirs.cache).unwrap()); }); } @@ -172,23 +160,22 @@ mod tests { fn ownership_requires_a_regular_marker_file() { EnvConfig::scoped(|env| { prepare(&env.dirs).unwrap(); - let roots = roots(&env.dirs); let marker = env.dirs.cache.join(OWNERSHIP_MARKER); fs::remove_file(&marker).unwrap(); - assert!(require_owned(&roots).is_err()); + assert!(!is_owned(&env.dirs.cache).unwrap()); fs::create_dir(&marker).unwrap(); - assert!(require_owned(&roots).is_err()); + assert!(!is_owned(&env.dirs.cache).unwrap()); fs::remove_dir(&marker).unwrap(); #[cfg(unix)] { let target = env.dirs.data.join("elsewhere"); fs::write(&target, OWNERSHIP_CONTENT).unwrap(); std::os::unix::fs::symlink(&target, &marker).unwrap(); - assert!(require_owned(&roots).is_err()); + assert!(!is_owned(&env.dirs.cache).unwrap()); fs::remove_file(&marker).unwrap(); } fs::write(&marker, b"").unwrap(); - require_owned(&roots).unwrap(); + assert!(is_owned(&env.dirs.cache).unwrap()); }); } } diff --git a/crates/vp_setup/src/lockfile.rs b/crates/vp_setup/src/lockfile.rs index c3aeac47c5..9d5bab7af9 100644 --- a/crates/vp_setup/src/lockfile.rs +++ b/crates/vp_setup/src/lockfile.rs @@ -11,7 +11,7 @@ use vt_path::{AbsolutePath, AbsolutePathBuf}; use crate::{error::Error, integrity, platform, registry}; pub const INSTALL_LOCKFILE: &str = "vp-install-lock.yaml"; -const INSTALL_RECEIPT: &str = "node_modules/.vp-install-complete"; +pub const INSTALL_RECEIPT: &str = "node_modules/.vp-install-complete"; // This can never match a frozen install's 32-byte SHA-256 digest. const UNLOCKED_INSTALL_RECEIPT: &[u8] = b"unlocked"; @@ -66,6 +66,11 @@ pub(crate) async fn record_install(version_dir: &AbsolutePath) -> Result<(), Err write(version_dir.join(INSTALL_RECEIPT), Sha256::digest(content).to_vec()).await } +/// Whether the completion receipt records a frozen install of this companion lockfile. +pub fn matches_install_receipt(lockfile: &[u8], receipt: &[u8]) -> bool { + receipt == Sha256::digest(lockfile).as_slice() +} + /// Restore the release's lockfile before a frozen install. Returns whether it changed. pub async fn restore(version_dir: &AbsolutePath) -> Result { let content = tokio::fs::read(version_dir.join("bin").join(INSTALL_LOCKFILE)).await?; @@ -135,7 +140,7 @@ pub async fn prepare( return Ok(true); }; let content = tokio::fs::read(target).await?; - Ok(receipt != Sha256::digest(content).as_slice()) + Ok(!matches_install_receipt(&content, &receipt)) } fn from_archive(data: &[u8]) -> Result>, Error> {