From 37dc83c7701dad54ccde706a7c670d1ed137eb45 Mon Sep 17 00:00:00 2001 From: JongKyung Lee Date: Sun, 25 Oct 2026 12:28:50 +0900 Subject: [PATCH 01/10] 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 63ea7c12d056fea36e6eaf22e1489839f1e09337 Mon Sep 17 00:00:00 2001 From: JongKyung Lee Date: Sun, 25 Oct 2026 14:34:51 +0900 Subject: [PATCH 02/10] 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 4d688159504b622e01d6735aa707aea880c8169f Mon Sep 17 00:00:00 2001 From: JongKyung Lee Date: Sun, 25 Oct 2026 14:53:40 +0900 Subject: [PATCH 03/10] 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 770f4e4bcecbea4d0bc7a5ca1ebf3f88c1d12e75 Mon Sep 17 00:00:00 2001 From: JongKyung Lee Date: Sun, 25 Oct 2026 17:22:01 +0900 Subject: [PATCH 04/10] 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 18001051d176e3c9fabe30e57ee55b5b1073415b Mon Sep 17 00:00:00 2001 From: JongKyung Lee Date: Mon, 26 Oct 2026 00:57:24 +0900 Subject: [PATCH 05/10] 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> { From ff4e85a143e84f0c9b52cae515ddfd49a2af3950 Mon Sep 17 00:00:00 2001 From: JongKyung Lee Date: Mon, 26 Oct 2026 09:24:13 +0900 Subject: [PATCH 06/10] fix(implode): describe directory management in removal errors --- .../command_implode/snapshots/implode_unowned_directory.md | 2 +- crates/vp_global_cli/src/commands/implode.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 354e24ac77..3b2d533636 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 @@ -26,7 +26,7 @@ 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. +error: Refusing to remove /Documents: Vite+ could not verify that it manages this directory. Check your directory overrides and inspect the directory before removing it manually. ``` ## `vpt print-file $HOME/.bashrc` diff --git a/crates/vp_global_cli/src/commands/implode.rs b/crates/vp_global_cli/src/commands/implode.rs index 806a05d83b..22db316646 100644 --- a/crates/vp_global_cli/src/commands/implode.rs +++ b/crates/vp_global_cli/src/commands/implode.rs @@ -102,7 +102,7 @@ pub fn execute(yes: bool) -> Result { 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.", + "Refusing to remove {}: Vite+ could not verify that it manages this directory. Check your directory overrides and inspect the directory before removing it manually.", dirs.data.as_path().display() ))); } From 3fbeaead06e9a06018cc651171ca4a5b409fb8a5 Mon Sep 17 00:00:00 2001 From: JongKyung Lee Date: Mon, 26 Oct 2026 10:16:24 +0900 Subject: [PATCH 07/10] fix(implode): reject root paths changed by symlink traversal Validate parent traversal before preparing roots or planning removal so lexical normalization cannot hide an unowned nested category. Preserve the existing legacy partial-removal rules. Cover unsafe traversal, unchanged directory identities, missing paths, dangling links, and CLI refusal before modifying files, profiles, or shims. --- .../fixtures/command_implode/snapshots.toml | 6 + .../implode_symlink_parent_override.md | 8 ++ .../fixtures/command_implode/unsafe-root.mjs | 67 +++++++++++ .../src/commands/implode/plan.rs | 37 ++++++ crates/vp_global_cli/src/install_dirs.rs | 107 ++++++++++++++++++ 5 files changed, 225 insertions(+) create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_symlink_parent_override.md create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/unsafe-root.mjs 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 79bb293646..3253b99ca1 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 @@ -64,6 +64,12 @@ steps = [ ["vpt", "stat-file", "$VP_HOME/bin", "--assert", "dir"], ] +[[case]] +name = "implode_symlink_parent_override" +vp = "global" +skip-platforms = ["windows"] +steps = [["node", "unsafe-root.mjs"]] + [[case]] name = "implode_legacy_upgrade" vp = "global" diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_symlink_parent_override.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_symlink_parent_override.md new file mode 100644 index 0000000000..a28d45e0de --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_symlink_parent_override.md @@ -0,0 +1,8 @@ +# implode_symlink_parent_override + +## `node unsafe-root.mjs` + +``` +A symlink followed by parent traversal is rejected even with --yes. +The marked data root, personal files, shell profile, and shim remain unchanged. +``` diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/unsafe-root.mjs b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/unsafe-root.mjs new file mode 100644 index 0000000000..f163dd18a5 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/unsafe-root.mjs @@ -0,0 +1,67 @@ +import assert from 'node:assert/strict'; +import { spawnSync } from 'node:child_process'; +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + readlinkSync, + rmSync, + symlinkSync, + writeFileSync, +} from 'node:fs'; +import { join } from 'node:path'; + +const binary = join(process.env.VP_HOME, 'current/bin/vp'); +const user = mkdtempSync(join(process.cwd(), 'unsafe-root-')); +const data = join(user, 'data'); +const personal = join(data, 'personal'); +const links = join(user, 'links'); +const bin = join(user, 'shared-bin'); +const config = join(user, '.config/vite-plus'); +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'), + VP_DATA_DIR: data, + // Keep '..' intact so the CLI, not node:path, interprets the override. + VP_CACHE_DIR: `${links}/route/../personal`, + VP_BIN_DIR: bin, + VP_SELF_SETUP_NO_MODIFY_PATH: '1', + CI: '1', + NO_COLOR: '1', +}; +delete env.VP_HOME; + +try { + for (const directory of [join(data, 'child'), personal, links, bin, config]) + mkdirSync(directory, { recursive: true }); + writeFileSync(join(data, '.vp-owned'), 'vite-plus-owned-directory\n'); + writeFileSync(join(personal, 'notes.txt'), 'keep my notes\n'); + symlinkSync(join(data, 'child'), join(links, 'route')); + symlinkSync(binary, join(bin, 'vp')); + const profile = `# personal config\n# Vite+ bin\n. "${config}/env"\n`; + writeFileSync(join(user, '.bashrc'), profile); + + const result = spawnSync(binary, ['implode', '--yes'], { + env, + encoding: 'utf8', + timeout: 30000, + }); + assert.equal(result.status, 1, `${result.error ?? ''}\n${result.stdout}\n${result.stderr}`); + assert.match(result.stdout + result.stderr, /Refusing to normalize directory/); + assert.match(result.stdout + result.stderr, /Specify the directory without '\.\.'/); + assert.equal(readFileSync(join(personal, 'notes.txt'), 'utf8'), 'keep my notes\n'); + assert.equal(readFileSync(join(user, '.bashrc'), 'utf8'), profile); + assert.equal(readlinkSync(join(bin, 'vp')), binary); + assert.equal(existsSync(join(data, '.vp-owned')), true); + assert.equal(existsSync(join(links, 'personal')), false); + console.log('A symlink followed by parent traversal is rejected even with --yes.'); + console.log('The marked data root, personal files, shell profile, and shim remain unchanged.'); +} finally { + rmSync(user, { recursive: true, force: true }); +} diff --git a/crates/vp_global_cli/src/commands/implode/plan.rs b/crates/vp_global_cli/src/commands/implode/plan.rs index ed662daea1..b9d78ba3c1 100644 --- a/crates/vp_global_cli/src/commands/implode/plan.rs +++ b/crates/vp_global_cli/src/commands/implode/plan.rs @@ -17,6 +17,7 @@ pub(super) struct RemovalPlan { impl RemovalPlan { pub fn new(dirs: &VpDirs, roots: &[AbsolutePathBuf]) -> Result { + install_dirs::validate_root_normalization(dirs)?; let mut owned = Vec::new(); let mut unowned = Vec::new(); for root in roots { @@ -278,6 +279,42 @@ mod tests { }); } + #[cfg(unix)] + #[test] + fn unowned_nested_cache_rejects_symlink_parent_path() { + vp_shared::EnvConfig::scoped(|env| { + let mut dirs = env.dirs.clone(); + dirs.data = env.dirs.data.join("data"); + dirs.config = dirs.data.clone(); + dirs.state = dirs.data.clone(); + let personal = dirs.data.join("personal"); + let links = env.dirs.data.join("links"); + fs::create_dir_all(dirs.data.join("child")).unwrap(); + fs::create_dir(&personal).unwrap(); + fs::create_dir(&links).unwrap(); + fs::write(dirs.data.join(install_dirs::OWNERSHIP_MARKER), b"").unwrap(); + fs::write(personal.join("notes.txt"), b"keep").unwrap(); + std::os::unix::fs::symlink(dirs.data.join("child"), links.join("route")).unwrap(); + let alias = links.join("route/../personal"); + assert_eq!( + dunce::canonicalize(&alias).unwrap(), + dunce::canonicalize(&personal).unwrap() + ); + assert!(!links.join("personal").as_path().exists()); + + dirs.cache = personal.clone(); + let plan = RemovalPlan::new(&dirs, &install_dirs::roots(&dirs)).unwrap(); + assert!(plan.directories.is_empty()); + + dirs.cache = alias; + let error = RemovalPlan::new(&dirs, &install_dirs::roots(&dirs)) + .err() + .expect("a symlink parent path must be rejected before removal"); + assert!(error.to_string().contains("Refusing to normalize directory")); + assert_eq!(fs::read(personal.join("notes.txt")).unwrap(), b"keep"); + }); + } + #[cfg(unix)] #[test] fn verified_version_links_and_shims_do_not_require_a_surviving_binary() { diff --git a/crates/vp_global_cli/src/install_dirs.rs b/crates/vp_global_cli/src/install_dirs.rs index 0031481fc3..5d1ee5b132 100644 --- a/crates/vp_global_cli/src/install_dirs.rs +++ b/crates/vp_global_cli/src/install_dirs.rs @@ -28,6 +28,39 @@ pub(crate) fn lexical_path(path: &Path) -> PathBuf { normalized } +/// Reject parent traversal when lexical normalization would change the directory target. +pub(crate) fn validate_root_normalization(dirs: &VpDirs) -> Result<(), Error> { + for root in [&dirs.data, &dirs.cache, &dirs.config, &dirs.state] { + let mut prefix = PathBuf::new(); + for component in root.as_path().components() { + match component { + Component::CurDir => {} + Component::ParentDir => { + if let Some(parent) = prefix.parent() { + match fs::symlink_metadata(&prefix) { + Ok(_) => { + let resolved = dunce::canonicalize(&prefix)?; + let expected_parent = dunce::canonicalize(parent)?; + if resolved.parent() != Some(expected_parent.as_path()) { + return Err(Error::Other(vt_str::format!( + "Refusing to normalize directory {}: resolving '..' across a link changes its target. Specify the directory without '..'.", + root.as_path().display() + ))); + } + } + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(error) => return Err(error.into()), + } + } + prefix.pop(); + } + _ => prefix.push(component.as_os_str()), + } + } + } + Ok(()) +} + /// 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] @@ -53,6 +86,7 @@ pub(crate) fn is_owned(root: &AbsolutePathBuf) -> Result { /// 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> { + validate_root_normalization(dirs)?; let mut empty = Vec::new(); // Preflight all roots before creating directories or ownership markers. for root in roots(dirs) { @@ -156,6 +190,79 @@ mod tests { }); } + #[test] + fn prepare_allows_parent_components_in_existing_and_missing_directories() { + EnvConfig::scoped(|env| { + let mut dirs = env.dirs.clone(); + fs::create_dir(env.dirs.data.join("existing")).unwrap(); + dirs.data = env.dirs.data.join("existing/../data"); + dirs.cache = env.dirs.data.join("missing/../cache"); + dirs.config = dirs.data.clone(); + dirs.state = dirs.data.clone(); + + prepare(&dirs).unwrap(); + + assert!(is_owned(&env.dirs.data.join("data")).unwrap()); + assert!(is_owned(&env.dirs.data.join("cache")).unwrap()); + assert!(!env.dirs.data.join("missing").as_path().exists()); + }); + } + + #[cfg(unix)] + #[test] + fn prepare_rejects_changed_parent_before_creating_any_roots() { + EnvConfig::scoped(|env| { + let mut dirs = env.dirs.clone(); + let personal = env.dirs.data.join("personal"); + fs::create_dir_all(personal.join("child")).unwrap(); + fs::write(personal.join("notes.txt"), b"keep").unwrap(); + let route = env.dirs.data.join("route"); + std::os::unix::fs::symlink(personal.join("child"), &route).unwrap(); + dirs.data = env.dirs.data.join("new-data"); + dirs.cache = route.join("../cache"); + dirs.config = env.dirs.data.join("new-config"); + dirs.state = env.dirs.data.join("new-state"); + + let error = prepare(&dirs).unwrap_err(); + + assert!(error.to_string().contains("Refusing to normalize directory")); + for root in roots(&dirs) { + assert!(!root.as_path().exists()); + } + assert_eq!(fs::read(personal.join("notes.txt")).unwrap(), b"keep"); + assert!(!personal.join(OWNERSHIP_MARKER).as_path().exists()); + }); + } + + #[cfg(unix)] + #[test] + fn validate_root_normalization_allows_links_that_preserve_parent_traversal() { + EnvConfig::scoped(|env| { + let mut dirs = env.dirs.clone(); + let target = env.dirs.data.join("target"); + fs::create_dir_all(target.join("child")).unwrap(); + let alias = env.dirs.data.join("alias"); + std::os::unix::fs::symlink(&target, &alias).unwrap(); + for cache in [alias.clone(), alias.join("child/.."), alias.join("../new-cache")] { + dirs.cache = cache; + validate_root_normalization(&dirs).unwrap(); + } + }); + } + + #[cfg(unix)] + #[test] + fn validate_root_normalization_rejects_dangling_links_before_parent_traversal() { + EnvConfig::scoped(|env| { + let mut dirs = env.dirs.clone(); + let alias = env.dirs.data.join("alias"); + std::os::unix::fs::symlink(env.dirs.data.join("missing"), &alias).unwrap(); + dirs.cache = alias.join("../cache"); + + assert!(validate_root_normalization(&dirs).is_err()); + }); + } + #[test] fn ownership_requires_a_regular_marker_file() { EnvConfig::scoped(|env| { From 0d6f1df75c1b318fb912081373aac9b30ffff034 Mon Sep 17 00:00:00 2001 From: JongKyung Lee Date: Mon, 26 Oct 2026 10:21:11 +0900 Subject: [PATCH 08/10] test(implode): drop redundant symlink alias precondition The lexical-path guard and the rejection assertion already cover the alias premise. --- crates/vp_global_cli/src/commands/implode/plan.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/crates/vp_global_cli/src/commands/implode/plan.rs b/crates/vp_global_cli/src/commands/implode/plan.rs index b9d78ba3c1..65467341b7 100644 --- a/crates/vp_global_cli/src/commands/implode/plan.rs +++ b/crates/vp_global_cli/src/commands/implode/plan.rs @@ -296,10 +296,6 @@ mod tests { fs::write(personal.join("notes.txt"), b"keep").unwrap(); std::os::unix::fs::symlink(dirs.data.join("child"), links.join("route")).unwrap(); let alias = links.join("route/../personal"); - assert_eq!( - dunce::canonicalize(&alias).unwrap(), - dunce::canonicalize(&personal).unwrap() - ); assert!(!links.join("personal").as_path().exists()); dirs.cache = personal.clone(); From 63d871a474286bdea04ccec1a2064691498db242 Mon Sep 17 00:00:00 2001 From: JongKyung Lee Date: Mon, 26 Oct 2026 14:52:18 +0900 Subject: [PATCH 09/10] fix(setup): allow aliased empty installation roots Recheck ownership before creating each queued marker so a marker created through another alias does not fail first-time setup. Keep exclusive creation and preserve unowned nonempty directories. Add a regression test for data and cache roots sharing one empty directory through a symlink. --- crates/vp_global_cli/src/install_dirs.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/crates/vp_global_cli/src/install_dirs.rs b/crates/vp_global_cli/src/install_dirs.rs index 5d1ee5b132..0bf2cb1b85 100644 --- a/crates/vp_global_cli/src/install_dirs.rs +++ b/crates/vp_global_cli/src/install_dirs.rs @@ -104,6 +104,9 @@ pub(crate) fn prepare(dirs: &VpDirs) -> Result<(), Error> { empty.push(root); } for root in empty { + if is_owned(&root)? { + continue; + } fs::create_dir_all(&root)?; let mut marker = fs::OpenOptions::new() .write(true) @@ -163,6 +166,25 @@ mod tests { } } + #[cfg(unix)] + #[test] + fn prepare_accepts_aliased_empty_roots() { + EnvConfig::scoped(|env| { + let mut dirs = env.dirs.clone(); + dirs.data = env.dirs.data.join("data"); + dirs.cache = env.dirs.data.join("cache"); + dirs.config = dirs.data.clone(); + dirs.state = dirs.data.clone(); + fs::create_dir(&dirs.data).unwrap(); + std::os::unix::fs::symlink(&dirs.data, &dirs.cache).unwrap(); + + prepare(&dirs).unwrap(); + + assert!(is_owned(&dirs.data).unwrap()); + assert!(is_owned(&dirs.cache).unwrap()); + }); + } + #[test] fn prepare_preflights_all_roots_and_never_claims_existing_contents() { EnvConfig::scoped(|env| { From 2ce69bca9b59753c0c7622c8300ca8f75db54365 Mon Sep 17 00:00:00 2001 From: MK Date: Mon, 26 Oct 2026 15:52:31 +0900 Subject: [PATCH 10/10] refactor(implode): simplify installation ownership checks --- .../command_implode/legacy-upgrade.mjs | 36 ++++++------ .../fixtures/command_implode/unsafe-root.mjs | 5 +- .../verify-refresh.mjs | 15 ++--- crates/vp_global_cli/src/commands/implode.rs | 28 +++++---- .../src/commands/implode/plan.rs | 57 ++++++++++--------- crates/vp_global_cli/src/install_dirs.rs | 54 ++++++++++-------- 6 files changed, 104 insertions(+), 91 deletions(-) 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 index 7186f2e3c0..3fd055041f 100644 --- 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 @@ -42,17 +42,18 @@ const env = { 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 }, -); +if (split) { + Object.assign(env, { VP_DATA_DIR: data, VP_CACHE_DIR: cache, VP_BIN_DIR: bin }); +} else { + env.VP_HOME = data; +} function write(path, contents) { mkdirSync(dirname(path), { recursive: true }); writeFileSync(path, contents); } -function version(name, active = false) { +function createVersion(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`; @@ -91,9 +92,9 @@ function run(binary, args) { } try { - const old = version('1.4.0'); - const forced = version('1.4.0+force.123.456'); - const current = version('1.5.0', true); + const old = createVersion('1.4.0'); + const forced = createVersion('1.4.0+force.123.456'); + const current = createVersion('1.5.0', true); const active = join(current, 'bin', binaryName); symlinkSync(current, join(data, 'current'), windows ? 'junction' : 'dir'); const retained = [ @@ -109,11 +110,11 @@ try { [join(bin, 'unrelated-tool'), 'another tool'], ]; for (const [path, contents] of retained) write(path, contents); - const incomplete = version('1.3.0'); + const incomplete = createVersion('1.3.0'); rmSync(join(incomplete, 'node_modules/.vp-install-complete')); - const unlocked = version('1.3.1'); + const unlocked = createVersion('1.3.1'); write(join(unlocked, 'node_modules/.vp-install-complete'), 'unlocked'); - const mismatched = version('1.3.2'); + const mismatched = createVersion('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. @@ -132,12 +133,13 @@ try { 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, - ); + for (const path of [ + join(data, 'current'), + join(bin, binaryName), + join(data, 'fallback-bin', windows ? 'node.exe' : 'node'), + ]) { + assert.equal(lstatSync(path, { 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) diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/unsafe-root.mjs b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/unsafe-root.mjs index f163dd18a5..10c9dc80df 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/unsafe-root.mjs +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/unsafe-root.mjs @@ -53,8 +53,9 @@ try { timeout: 30000, }); assert.equal(result.status, 1, `${result.error ?? ''}\n${result.stdout}\n${result.stderr}`); - assert.match(result.stdout + result.stderr, /Refusing to normalize directory/); - assert.match(result.stdout + result.stderr, /Specify the directory without '\.\.'/); + const output = result.stdout + result.stderr; + assert.match(output, /Refusing to normalize directory/); + assert.match(output, /Specify the directory without '\.\.'/); assert.equal(readFileSync(join(personal, 'notes.txt'), 'utf8'), 'keep my notes\n'); assert.equal(readFileSync(join(user, '.bashrc'), 'utf8'), profile); assert.equal(readlinkSync(join(bin, 'vp')), binary); 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 3fdba650eb..c1c125e828 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,12 +26,13 @@ function createBundle(source, prefix, label) { } function createEnvironment(directory) { - const home = path.join(directory, 'home', '.vite-plus'); + const home = path.join(directory, 'home'); + const vpHome = path.join(home, '.vite-plus'); const system = path.join(directory, 'system/bin'); - const runtime = path.join(home, 'js_runtime/node', process.versions.node, 'bin'); + const runtime = path.join(vpHome, '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.writeFileSync(path.join(vpHome, '.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); @@ -41,16 +42,16 @@ function createEnvironment(directory) { } return { ...env, - HOME: path.dirname(home), - VP_HOME: home, + HOME: home, + VP_HOME: vpHome, VP_SELF_SETUP_NO_MODIFY_PATH: '1', NPM_CONFIG_REGISTRY: 'http://127.0.0.1:9', PATH: [ - path.join(home, 'bin'), + path.join(vpHome, 'bin'), path.join(directory, 'brew/bin'), system, env.PATH, - path.join(home, 'fallback-bin'), + path.join(vpHome, 'fallback-bin'), ].join(path.delimiter), }; } diff --git a/crates/vp_global_cli/src/commands/implode.rs b/crates/vp_global_cli/src/commands/implode.rs index 22db316646..7bd6b0d046 100644 --- a/crates/vp_global_cli/src/commands/implode.rs +++ b/crates/vp_global_cli/src/commands/implode.rs @@ -193,13 +193,14 @@ fn report_preserved_paths(roots: &[AbsolutePathBuf]) { } 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())); - } + if paths.is_empty() { + return; + } + 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())); } } @@ -276,15 +277,8 @@ fn remove_shim_files(dirs: &vp_shared::VpDirs, paths: &[AbsolutePathBuf]) { match std::fs::remove_file(path.as_path()) { Ok(()) => { removed += 1; - if let Some(pointer) = pointer.as_ref() { - remove_shim_pointer(pointer, &name); - } - } - Err(e) if e.kind() == std::io::ErrorKind::NotFound => { - if let Some(pointer) = pointer.as_ref() { - remove_shim_pointer(pointer, &name); - } } + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} Err(e) => { #[cfg(windows)] if let Some(pointer) = pointer.as_ref() { @@ -301,8 +295,12 @@ fn remove_shim_files(dirs: &vp_shared::VpDirs, paths: &[AbsolutePathBuf]) { } } output::warn(&vt_str::format!("Vite+ could not remove shim {name}: {e}")); + continue; } } + if let Some(pointer) = pointer.as_ref() { + remove_shim_pointer(pointer, &name); + } } if removed > 0 { output::success(&vt_str::format!( diff --git a/crates/vp_global_cli/src/commands/implode/plan.rs b/crates/vp_global_cli/src/commands/implode/plan.rs index 65467341b7..49765967cd 100644 --- a/crates/vp_global_cli/src/commands/implode/plan.rs +++ b/crates/vp_global_cli/src/commands/implode/plan.rs @@ -2,7 +2,10 @@ use std::{fs, path::Path}; -use vp_setup::lockfile::{INSTALL_LOCKFILE, INSTALL_RECEIPT, matches_install_receipt}; +use vp_setup::{ + install::is_install_dir_for_version, + lockfile::{INSTALL_LOCKFILE, INSTALL_RECEIPT, matches_install_receipt}, +}; use vp_shared::{VP_BINARY_NAME, VpDirs}; use vt_path::AbsolutePathBuf; @@ -18,8 +21,8 @@ pub(super) struct RemovalPlan { impl RemovalPlan { pub fn new(dirs: &VpDirs, roots: &[AbsolutePathBuf]) -> Result { install_dirs::validate_root_normalization(dirs)?; - let mut owned = Vec::new(); - let mut unowned = Vec::new(); + let mut owned_roots = Vec::new(); + let mut unowned_roots = Vec::new(); for root in roots { match fs::symlink_metadata(root) { Ok(_) => {} @@ -27,7 +30,7 @@ impl RemovalPlan { Err(error) => return Err(error.into()), } if install_dirs::is_owned(root)? { - owned.push(root.clone()); + owned_roots.push(root.clone()); } else { let canonical = match dunce::canonicalize(root) { Ok(path) => path, @@ -36,9 +39,15 @@ impl RemovalPlan { } Err(error) => return Err(error.into()), }; - unowned.push((root.clone(), canonical)); + unowned_roots.push((root.clone(), canonical)); } } + // Recursive removal must preserve unmarked categories, including aliases. + let contains_unowned_root = |directory: &AbsolutePathBuf, canonical: &Path| { + unowned_roots.iter().any(|(path, boundary)| { + path.as_path().starts_with(directory.as_path()) || boundary.starts_with(canonical) + }) + }; let mut plan = Self { directories: Vec::new(), current_link: None, @@ -51,12 +60,9 @@ impl RemovalPlan { if install_dirs::is_owned(&dirs.data)? { plan.binary_targets.push(dirs.data.join("current/bin").join(VP_BINARY_NAME)); } - for root in owned { + for root in owned_roots { 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) - }) { + if !contains_unowned_root(&root, &canonical) { plan.directories.push(root); } } @@ -74,21 +80,19 @@ impl RemovalPlan { 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); - } + if !verified_version(&version_dir)? { + continue; + } + plan.binary_targets.push(version_dir.join("bin").join(VP_BINARY_NAME)); + let canonical = dunce::canonicalize(&version_dir)?; + if contains_unowned_root(&version_dir, &canonical) { + continue; + } + 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 => {} @@ -121,7 +125,8 @@ fn verified_version(directory: &AbsolutePathBuf) -> Result { let Some(receipt) = read_contained_file(&canonical, INSTALL_RECEIPT)? else { return Ok(false); }; - let Some(lockfile) = read_contained_file(&canonical, &format!("bin/{INSTALL_LOCKFILE}"))? + let Some(lockfile) = + read_contained_file(&canonical, &vt_str::format!("bin/{INSTALL_LOCKFILE}"))? else { return Ok(false); }; @@ -143,7 +148,7 @@ fn verified_version(directory: &AbsolutePathBuf) -> Result { }; 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."))) + && is_install_dir_for_version(name, version) && wrapper["name"] == "vp-global" && wrapper["private"] == true && wrapper["dependencies"]["vite-plus"] == version diff --git a/crates/vp_global_cli/src/install_dirs.rs b/crates/vp_global_cli/src/install_dirs.rs index 0bf2cb1b85..ba3f88eb53 100644 --- a/crates/vp_global_cli/src/install_dirs.rs +++ b/crates/vp_global_cli/src/install_dirs.rs @@ -36,22 +36,7 @@ pub(crate) fn validate_root_normalization(dirs: &VpDirs) -> Result<(), Error> { match component { Component::CurDir => {} Component::ParentDir => { - if let Some(parent) = prefix.parent() { - match fs::symlink_metadata(&prefix) { - Ok(_) => { - let resolved = dunce::canonicalize(&prefix)?; - let expected_parent = dunce::canonicalize(parent)?; - if resolved.parent() != Some(expected_parent.as_path()) { - return Err(Error::Other(vt_str::format!( - "Refusing to normalize directory {}: resolving '..' across a link changes its target. Specify the directory without '..'.", - root.as_path().display() - ))); - } - } - Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} - Err(error) => return Err(error.into()), - } - } + validate_parent_traversal(root, &prefix)?; prefix.pop(); } _ => prefix.push(component.as_os_str()), @@ -61,6 +46,27 @@ pub(crate) fn validate_root_normalization(dirs: &VpDirs) -> Result<(), Error> { Ok(()) } +fn validate_parent_traversal(root: &AbsolutePathBuf, prefix: &Path) -> Result<(), Error> { + let Some(parent) = prefix.parent() else { + return Ok(()); + }; + match fs::symlink_metadata(prefix) { + Ok(_) => {} + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(()), + Err(error) => return Err(error.into()), + } + + let resolved = dunce::canonicalize(prefix)?; + let expected_parent = dunce::canonicalize(parent)?; + if resolved.parent() != Some(expected_parent.as_path()) { + return Err(Error::Other(vt_str::format!( + "Refusing to normalize directory {}: resolving '..' across a link changes its target. Specify the directory without '..'.", + root.as_path().display() + ))); + } + Ok(()) +} + /// 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] @@ -87,23 +93,23 @@ pub(crate) fn is_owned(root: &AbsolutePathBuf) -> Result { /// directory must never grant permission to delete its existing contents. pub(crate) fn prepare(dirs: &VpDirs) -> Result<(), Error> { validate_root_normalization(dirs)?; - let mut empty = Vec::new(); + let mut empty_roots = 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, + let is_empty = match fs::read_dir(&root) { + Ok(mut entries) => entries.next().transpose()?.is_none(), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => true, Err(error) => return Err(error.into()), }; - if nonempty { - continue; + if is_empty { + empty_roots.push(root); } - empty.push(root); } - for root in empty { + for root in empty_roots { + // An earlier root may have created the marker through a filesystem alias. if is_owned(&root)? { continue; }