From 7fac7eec30bc8d9cd3196d3317f8f1adb2d085bb Mon Sep 17 00:00:00 2001 From: JongKyung Lee Date: Sun, 25 Oct 2026 12:28:50 +0900 Subject: [PATCH 1/4] fix(implode): prevent deletion of home and filesystem roots Reject protected deletion targets before modifying shell profiles, PATH, or managed files, including with --yes. Check canonical paths to catch symlink aliases and validate all roots before starting split-layout cleanup. Add regression coverage for protected roots, normal installations, symlink aliases, and preservation of files when validation fails. --- crates/vp_global_cli/src/commands/implode.rs | 137 ++++++++++++++++++- 1 file changed, 136 insertions(+), 1 deletion(-) diff --git a/crates/vp_global_cli/src/commands/implode.rs b/crates/vp_global_cli/src/commands/implode.rs index 71796fa508..75e3cb68e8 100644 --- a/crates/vp_global_cli/src/commands/implode.rs +++ b/crates/vp_global_cli/src/commands/implode.rs @@ -50,6 +50,33 @@ fn lexical_path(path: &Path) -> PathBuf { normalized } +fn validate_delete_roots( + roots: &[AbsolutePathBuf], + user_home: &AbsolutePathBuf, +) -> Result<(), Error> { + let home = lexical_path(user_home.as_path()); + let canonical_home = dunce::canonicalize(user_home)?; + for root in roots { + let path = root.as_path(); + let canonical = match dunce::canonicalize(path) { + Ok(path) => path, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => path.to_path_buf(), + Err(error) => return Err(error.into()), + }; + if path.parent().is_none() + || home.starts_with(path) + || canonical.parent().is_none() + || canonical_home.starts_with(&canonical) + { + return Err(Error::Other(vt_str::format!( + "Refusing to remove protected directory {}: it is a filesystem root, the user home, or an ancestor of the user home. Check your Vite+ directory overrides.", + path.display() + ))); + } + } + Ok(()) +} + pub fn execute(yes: bool) -> Result { let env_config = vp_shared::EnvConfig::get(); let dirs = &env_config.dirs; @@ -88,8 +115,9 @@ pub fn execute(yes: bool) -> Result { return Ok(exit_status(0)); } - // Use the user home to resolve shell-profile paths. + // Validate every deletion target before changing profiles, PATH, or files, even with --yes. let user_home = &env_config.user_home; + validate_delete_roots(&delete_set, user_home)?; let source_matcher = VitePlusSourceMatcher::new(&dirs.config, user_home); @@ -1544,6 +1572,113 @@ mod tests { }); } + #[test] + fn validate_delete_roots_rejects_home_ancestors_and_filesystem_roots() { + let temp_dir = tempfile::tempdir().unwrap(); + let home = AbsolutePathBuf::new(temp_dir.path().join("home")).unwrap(); + std::fs::create_dir_all(&home).unwrap(); + for ancestor in home.as_path().ancestors() { + let root = AbsolutePathBuf::new(ancestor.to_path_buf()).unwrap(); + let error = validate_delete_roots(std::slice::from_ref(&root), &home).unwrap_err(); + assert!(error.to_string().contains("Refusing to remove protected directory")); + } + } + + #[test] + fn validate_delete_roots_allows_dedicated_installations() { + let temp_dir = tempfile::tempdir().unwrap(); + let home = AbsolutePathBuf::new(temp_dir.path().join("home")).unwrap(); + std::fs::create_dir_all(&home).unwrap(); + let roots = [home.join(".vite-plus"), home.join("../home-other"), home.join("../custom")]; + for root in &roots { + std::fs::create_dir_all(root).unwrap(); + } + validate_delete_roots(&roots, &home).unwrap(); + validate_delete_roots(&[home.join("missing")], &home).unwrap(); + } + + #[cfg(unix)] + #[test] + fn validate_delete_roots_checks_symlinked_roots_and_home() { + let temp_dir = tempfile::tempdir().unwrap(); + let base = AbsolutePathBuf::new(temp_dir.path().to_path_buf()).unwrap(); + let home = base.join("home"); + let alias = base.join("alias"); + std::fs::create_dir_all(home.join(".vite-plus")).unwrap(); + std::os::unix::fs::symlink(&base, &alias).unwrap(); + for root in [&alias, &alias.join("home")] { + assert!(validate_delete_roots(std::slice::from_ref(root), &home).is_err()); + } + let aliased_home = alias.join("home"); + assert!(validate_delete_roots(std::slice::from_ref(&home), &aliased_home).is_err()); + validate_delete_roots(&[alias.join("home/.vite-plus")], &home).unwrap(); + } + + #[cfg(unix)] + #[test] + fn execute_rejects_user_home_before_modifying_files() { + let temp_dir = tempfile::tempdir().unwrap(); + let home = temp_dir.path().join("home"); + let xdg = temp_dir.path().join("xdg"); + std::fs::create_dir_all(&home).unwrap(); + std::fs::write(home.join("personal.txt"), "keep").unwrap(); + let profile = "# Vite+ bin\n. \"$HOME/env\"\n"; + std::fs::write(home.join(".bashrc"), profile).unwrap(); + + for root in [&home, &home.join("child/.."), &home.join("..")] { + vp_shared::EnvConfig::with_vars( + [ + (env_vars::VP_HOME, root.as_os_str()), + ("HOME", home.as_os_str()), + ("USERPROFILE", home.as_os_str()), + ("ZDOTDIR", home.as_os_str()), + (env_vars::XDG_CONFIG_HOME, xdg.as_os_str()), + (env_vars::XDG_DATA_HOME, xdg.as_os_str()), + ], + |_| { + let error = execute(true).unwrap_err(); + assert!(error.to_string().contains("Refusing to remove protected directory")); + assert_eq!(std::fs::read_to_string(home.join("personal.txt")).unwrap(), "keep"); + assert_eq!(std::fs::read_to_string(home.join(".bashrc")).unwrap(), profile); + }, + ); + } + } + + #[cfg(unix)] + #[test] + fn execute_validates_all_split_roots_before_deleting_any() { + let temp_dir = tempfile::tempdir().unwrap(); + let home = temp_dir.path().join("z-home"); + let data = temp_dir.path().join("a-data"); + let bin = temp_dir.path().join("bin"); + let xdg = temp_dir.path().join("xdg"); + std::fs::create_dir_all(&home).unwrap(); + std::fs::create_dir_all(&data).unwrap(); + std::fs::write(home.join("personal.txt"), "keep").unwrap(); + std::fs::write(data.join("data.txt"), "keep").unwrap(); + vp_shared::EnvConfig::with_vars( + [ + (env_vars::VP_HOME, None), + (env_vars::VP_BIN_DIR, Some(bin.as_os_str())), + (env_vars::VP_DATA_DIR, Some(data.as_os_str())), + (env_vars::VP_CACHE_DIR, Some(home.as_os_str())), + (env_vars::XDG_CONFIG_HOME, Some(xdg.as_os_str())), + (env_vars::XDG_DATA_HOME, Some(xdg.as_os_str())), + (env_vars::XDG_STATE_HOME, Some(xdg.as_os_str())), + ("HOME", Some(home.as_os_str())), + ("USERPROFILE", Some(home.as_os_str())), + ("ZDOTDIR", Some(home.as_os_str())), + ], + |_| { + let error = execute(true).unwrap_err(); + assert!(error.to_string().contains("Refusing to remove protected directory")); + assert_eq!(std::fs::read_to_string(home.join("personal.txt")).unwrap(), "keep"); + assert_eq!(std::fs::read_to_string(data.join("data.txt")).unwrap(), "keep"); + }, + ); + } + #[test] fn execute_normalizes_category_roots_before_deduplication() { let temp_dir = tempfile::tempdir().unwrap(); From e64845e2b13a152e9726ae7dbf0dd321a680e47f Mon Sep 17 00:00:00 2001 From: JongKyung Lee Date: Sun, 25 Oct 2026 14:34:51 +0900 Subject: [PATCH 2/4] fix(implode): verify ownership of every removal root Require ownership markers for every existing data, cache, config, and state root before changing files, shell profiles, or PATH. Validate nested roots independently and collapse them only when building the final recursive deletion targets. Preserve the home and filesystem root safeguards. Check that new installation roots are empty before recording ownership. Keep nonempty legacy roots unmarked so existing installations remain usable without granting permission to delete unrelated contents. Cover ownership checks, nested and split roots, fresh setup, and legacy setup with isolated unit tests and CLI snapshots. --- .../snapshots.toml | 1 + .../command_env_setup_external_vp.md | 6 + .../fixtures/command_implode/snapshots.toml | 22 ++ .../snapshots/implode_confirmation.md | 17 ++ .../snapshots/implode_unowned_directory.md | 60 ++++++ .../command_self_setup/snapshots.toml | 9 +- .../snapshots/command_self_setup.md | 10 +- .../command_self_setup_bootstrap_options.md | 12 ++ .../snapshots/command_self_setup_handoff.md | 6 + .../command_self_setup_mixed_shim_refresh.md | 6 + .../snapshots.toml | 1 + .../command_self_setup_external_bare.md | 6 + .../command_self_setup_external_doctor.md | 10 +- .../verify-refresh.mjs | 5 +- .../tests/cli_snapshots/main.rs | 2 + crates/vp_global_cli/src/commands/implode.rs | 79 +++---- crates/vp_global_cli/src/install_dirs.rs | 195 ++++++++++++++++++ crates/vp_global_cli/src/main.rs | 1 + crates/vp_global_cli/src/self_setup.rs | 4 + 19 files changed, 404 insertions(+), 48 deletions(-) create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_unowned_directory.md create mode 100644 crates/vp_global_cli/src/install_dirs.rs diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_env_setup_external_vp/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_env_setup_external_vp/snapshots.toml index 266b63b1ba..18b629836b 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_env_setup_external_vp/snapshots.toml +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_env_setup_external_vp/snapshots.toml @@ -5,6 +5,7 @@ skip-platforms = ["windows"] env = { VP_NODE_MANAGER = "yes", VP_SELF_SETUP_NO_MODIFY_PATH = "1" } steps = [ { argv = ["vpt", "mkdir", "-p", "external/bin", "home"], comment = "Prepare isolated external install and VP_HOME", snapshot = false }, + { argv = ["vpt", "write-file", "home/.vp-owned", "vite-plus-owned-directory\n"], comment = "The fixture owns the preseeded runtime directory", snapshot = false }, { argv = ["vpt", "cp", "$VP_HOME/bin/vp", "external/bin/vp"], comment = "Simulate a Homebrew-style vp outside VP_HOME", snapshot = false }, { argv = ["vpt", "chmod", "+x", "external/bin/vp"], snapshot = false }, { argv = ["vpt", "write-file", "external/node_modules/vite-plus/package.json", "{}"], snapshot = false }, diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_env_setup_external_vp/snapshots/command_env_setup_external_vp.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_env_setup_external_vp/snapshots/command_env_setup_external_vp.md index 8313c2227c..6268dee4bb 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_env_setup_external_vp/snapshots/command_env_setup_external_vp.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_env_setup_external_vp/snapshots/command_env_setup_external_vp.md @@ -5,6 +5,12 @@ Prepare isolated external install and VP_HOME +## `vpt write-file home/.vp-owned 'vite-plus-owned-directory +'` + +The fixture owns the preseeded runtime directory + + ## `vpt cp $VP_HOME/bin/vp external/bin/vp` Simulate a Homebrew-style vp outside VP_HOME diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots.toml index 8a760e8b29..8164496b3d 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots.toml +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots.toml @@ -3,6 +3,9 @@ name = "implode_confirmation" vp = "global" seed-runtime = false steps = [ + { argv = ["vpt", "rm", "$VP_HOME/cache/.vp-owned"], snapshot = false }, + { argv = ["vp", "implode", "--yes"], comment = "A nested cache needs its own ownership marker before any deletion", continue-on-failure = true }, + { argv = ["vpt", "write-file", "$VP_HOME/cache/.vp-owned", "vite-plus-owned-directory\n"], snapshot = false }, { argv = ["vp", "implode"], comment = "The old confirmation word leaves the installation intact", interactions = [ { expect-milestone = "text:implode:ready" }, { write-line = "uninstall" }, @@ -25,3 +28,22 @@ steps = [ ] }, ["vpt", "stat-file", "$VP_HOME", "--assert", "missing"], ] + +[[case]] +name = "implode_unowned_directory" +vp = "global" +seed-runtime = false +skip-platforms = ["windows"] +env = { VP_SKIP_DEPS_INSTALL = "1", VP_NODE_MANAGER = "no" } +steps = [ + { argv = ["vpt", "mkdir", "external"], snapshot = false }, + { argv = ["vpt", "cp", "$VP_HOME/bin/vp", "external/vp"], snapshot = false }, + { argv = ["vpt", "chmod", "+x", "external/vp"], snapshot = false }, + { argv = ["vpt", "write-file", "$HOME/Documents/personal.txt", "keep my documents\n"], snapshot = false }, + { argv = ["vpt", "write-file", "$HOME/.bashrc", "# personal config\n# Vite+ bin\n. \"$HOME/Documents/env\"\n"], snapshot = false }, + { argv = ["./external/vp"], tty = false, envs = [["VP_HOME", "${HOME}/Documents"]], comment = "A fresh install cannot claim a nonempty personal directory", continue-on-failure = true }, + { argv = ["vp", "implode", "--yes"], envs = [["VP_HOME", "${HOME}/Documents"]], comment = "Changing VP_HOME cannot authorize recursive deletion", continue-on-failure = true }, + ["vpt", "stat-file", "$HOME/Documents/.vp-owned", "--assert", "missing"], + ["vpt", "print-file", "$HOME/Documents/personal.txt"], + ["vpt", "print-file", "$HOME/.bashrc"], +] diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_confirmation.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_confirmation.md index 9f23cb0e5a..93e3d0d209 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_confirmation.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_confirmation.md @@ -1,5 +1,22 @@ # implode_confirmation +## `vpt rm $VP_HOME/cache/.vp-owned` + + +## `vp implode --yes` + +A nested cache needs its own ownership marker before any deletion + +**Exit code:** 1 + +``` +error: Refusing to remove /.vite-plus/cache: Vite+ directory ownership could not be verified. Unmarked directories, including older installations, must be inspected and removed manually. +``` + +## `vpt write-file $VP_HOME/cache/.vp-owned 'vite-plus-owned-directory +'` + + ## `vp implode` The old confirmation word leaves the installation intact diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_unowned_directory.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_unowned_directory.md new file mode 100644 index 0000000000..fd6027c05f --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_unowned_directory.md @@ -0,0 +1,60 @@ +# implode_unowned_directory + +## `vpt mkdir external` + + +## `vpt cp $VP_HOME/bin/vp external/vp` + + +## `vpt chmod +x external/vp` + + +## `vpt write-file $HOME/Documents/personal.txt 'keep my documents +'` + + +## `vpt write-file $HOME/.bashrc '# personal config +# Vite+ bin +. "$HOME/Documents/env" +'` + + +## `VP_HOME=${HOME}/Documents ./external/vp` + +A fresh install cannot claim a nonempty personal directory + +**Exit code:** 1 + +``` +error: Refusing to initialize /Documents: the directory is not empty and Vite+ ownership could not be verified. Choose an empty installation directory. +``` + +## `VP_HOME=${HOME}/Documents vp implode --yes` + +Changing VP_HOME cannot authorize recursive deletion + +**Exit code:** 1 + +``` +error: Refusing to remove /Documents: Vite+ directory ownership could not be verified. Unmarked directories, including older installations, must be inspected and removed manually. +``` + +## `vpt stat-file $HOME/Documents/.vp-owned --assert missing` + +``` +/Documents/.vp-owned: missing +``` + +## `vpt print-file $HOME/Documents/personal.txt` + +``` +keep my documents +``` + +## `vpt print-file $HOME/.bashrc` + +``` +# personal config +# Vite+ bin +. "$HOME/Documents/env" +``` diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots.toml index 887e0cf973..d09be4b917 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots.toml +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots.toml @@ -2,13 +2,14 @@ name = "command_self_setup" vp = "global" # The runner provides local dependencies; this case checks setup and dispatch. -env = { VP_SKIP_DEPS_INSTALL = "1" } +env = { VP_SKIP_DEPS_INSTALL = "1", VP_SELF_SETUP_NO_MODIFY_PATH = "1" } steps = [ - { argv = ["vpt", "rm", "$VP_HOME/current/bin/.vp-setup-complete"], snapshot = false }, + { argv = ["vpt", "rm", "$VP_HOME/current/bin/.vp-setup-complete", "$VP_HOME/.vp-owned"], comment = "An older installation has no directory ownership marker", snapshot = false }, { argv = ["vp", "--help"], envs = [["VP_SELF_SETUP_SUPPORT_CHECK", "1"]], comment = "The capability probe does not perform setup" }, ["vpt", "stat-file", "$VP_HOME/current/bin/.vp-setup-complete", "--assert", "missing"], { argv = ["vp", "--help"], comment = "An unmarked deployed binary completes setup and executes the requested command", snapshot = false }, ["vpt", "stat-file", "$VP_HOME/current/bin/.vp-setup-complete", "--assert", "file"], + ["vpt", "stat-file", "$VP_HOME/.vp-owned", "--assert", "missing"], { argv = ["vp", "not-a-command"], comment = "Once marked, the binary dispatches commands normally", continue-on-failure = true }, ] @@ -19,6 +20,7 @@ requires = ["bash"] skip-platforms = ["windows"] steps = [ { argv = ["vpt", "mkdir", "-p", "external", "project", "home/js_runtime/node"], snapshot = false }, + { argv = ["vpt", "write-file", "home/.vp-owned", "vite-plus-owned-directory\n"], comment = "The fixture owns the preseeded runtime directory", snapshot = false }, { argv = ["vpt", "cp", "-r", "$VP_HOME/js_runtime/node/22.18.0", "home/js_runtime/node/22.18.0"], snapshot = false }, { argv = ["vpt", "cp", "$VP_HOME/bin/vp", "external/vp"], snapshot = false }, { argv = ["vpt", "chmod", "+x", "external/vp"], snapshot = false }, @@ -54,6 +56,8 @@ steps = [ ["vpt", "print-file", "home/config.json"], ["vpt", "print-file", "user/.bashrc"], ["vpt", "stat-file", "user/.zshenv", "--assert", "missing"], + ["vpt", "stat-file", "home/.vp-owned", "--assert", "file"], + ["vpt", "stat-file", "home/cache/.vp-owned", "--assert", "file"], ] [[case]] @@ -108,6 +112,7 @@ skip-platforms = ["windows"] env = { VP_SKIP_DEPS_INSTALL = "1", VP_SELF_SETUP_NO_MODIFY_PATH = "1" } steps = [ { argv = ["vpt", "mkdir", "-p", "external", "home/bin", "user-bin"], snapshot = false }, + { argv = ["vpt", "write-file", "home/.vp-owned", "vite-plus-owned-directory\n"], comment = "The fixture owns the preseeded shim directory", snapshot = false }, { argv = ["vpt", "cp", "$VP_HOME/bin/vp", "external/vp"], snapshot = false }, { argv = ["vpt", "chmod", "+x", "external/vp"], snapshot = false }, { argv = ["node", "seed-owned-shims.cjs"], snapshot = false }, diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots/command_self_setup.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots/command_self_setup.md index b9d93a547a..52a75ab305 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots/command_self_setup.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots/command_self_setup.md @@ -1,6 +1,8 @@ # command_self_setup -## `vpt rm $VP_HOME/current/bin/.vp-setup-complete` +## `vpt rm $VP_HOME/current/bin/.vp-setup-complete $VP_HOME/.vp-owned` + +An older installation has no directory ownership marker ## `VP_SELF_SETUP_SUPPORT_CHECK=1 vp --help` @@ -28,6 +30,12 @@ An unmarked deployed binary completes setup and executes the requested command /.vite-plus/current/bin/.vp-setup-complete: file ``` +## `vpt stat-file $VP_HOME/.vp-owned --assert missing` + +``` +/.vite-plus/.vp-owned: missing +``` + ## `vp not-a-command` Once marked, the binary dispatches commands normally diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots/command_self_setup_bootstrap_options.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots/command_self_setup_bootstrap_options.md index 57f4a5d195..c7e99707f2 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots/command_self_setup_bootstrap_options.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots/command_self_setup_bootstrap_options.md @@ -49,3 +49,15 @@ home/current/bin/.vp-setup-complete: file ``` user/.zshenv: missing ``` + +## `vpt stat-file home/.vp-owned --assert file` + +``` +home/.vp-owned: file +``` + +## `vpt stat-file home/cache/.vp-owned --assert file` + +``` +home/cache/.vp-owned: file +``` diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots/command_self_setup_handoff.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots/command_self_setup_handoff.md index f240ee7efd..47a35452b1 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots/command_self_setup_handoff.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots/command_self_setup_handoff.md @@ -3,6 +3,12 @@ ## `vpt mkdir -p external project home/js_runtime/node` +## `vpt write-file home/.vp-owned 'vite-plus-owned-directory +'` + +The fixture owns the preseeded runtime directory + + ## `vpt cp -r $VP_HOME/js_runtime/node/22.18.0 home/js_runtime/node/22.18.0` diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots/command_self_setup_mixed_shim_refresh.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots/command_self_setup_mixed_shim_refresh.md index fd2e910e05..a1626e1717 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots/command_self_setup_mixed_shim_refresh.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup/snapshots/command_self_setup_mixed_shim_refresh.md @@ -3,6 +3,12 @@ ## `vpt mkdir -p external home/bin user-bin` +## `vpt write-file home/.vp-owned 'vite-plus-owned-directory +'` + +The fixture owns the preseeded shim directory + + ## `vpt cp $VP_HOME/bin/vp external/vp` diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/snapshots.toml index 1a99fcb201..78f224d487 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/snapshots.toml +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/snapshots.toml @@ -55,6 +55,7 @@ skip-platforms = ["windows"] env = { VP_SKIP_DEPS_INSTALL = "1", VP_VERSION = "external-test", VP_SELF_SETUP_NO_MODIFY_PATH = "1" } steps = [ { argv = ["vpt", "mkdir", "-p", "external", "home/external-test/bin"], snapshot = false }, + { argv = ["vpt", "write-file", "home/.vp-owned", "vite-plus-owned-directory\n"], comment = "An interrupted fresh install already claimed its empty root", snapshot = false }, { argv = ["vpt", "cp", "$VP_HOME/bin/vp", "external/vp"], snapshot = false }, { argv = ["vpt", "chmod", "555", "external/vp"], snapshot = false }, { argv = ["vpt", "cp", "external/vp", "home/external-test/bin/vp"], snapshot = false }, diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/snapshots/command_self_setup_external_bare.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/snapshots/command_self_setup_external_bare.md index 8ac9251661..970e020fd9 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/snapshots/command_self_setup_external_bare.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/snapshots/command_self_setup_external_bare.md @@ -3,6 +3,12 @@ ## `vpt mkdir -p external home/external-test/bin` +## `vpt write-file home/.vp-owned 'vite-plus-owned-directory +'` + +An interrupted fresh install already claimed its empty root + + ## `vpt cp $VP_HOME/bin/vp external/vp` diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/snapshots/command_self_setup_external_doctor.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/snapshots/command_self_setup_external_doctor.md index 22f1ad7291..d31929e11d 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/snapshots/command_self_setup_external_doctor.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/snapshots/command_self_setup_external_doctor.md @@ -6,8 +6,8 @@ Homebrew with shims on PATH CLI source Homebrew CLI binary /doctor/cellar/vite-plus/0.3.2/bin/vp - ✓ vp ~/bin/vp - ✓ Shim dir ~/bin + ✓ vp ~/.vite-plus/bin/vp + ✓ Shim dir ~/.vite-plus/bin Homebrew without shims on PATH CLI source Homebrew CLI binary /doctor/cellar/vite-plus/0.3.2/bin/vp @@ -17,8 +17,8 @@ Missing vp in the shim directory CLI source Homebrew CLI binary /doctor/cellar/vite-plus/0.3.2/bin/vp ✗ vp not in PATH - ✓ Shim dir ~/bin + ✓ Shim dir ~/.vite-plus/bin External package without a Homebrew receipt - ✓ vp ~/bin/vp - ✓ Shim dir ~/bin + ✓ vp ~/.vite-plus/bin/vp + ✓ Shim dir ~/.vite-plus/bin ``` diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/verify-refresh.mjs b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/verify-refresh.mjs index 5389615478..3fdba650eb 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/verify-refresh.mjs +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_self_setup_external/verify-refresh.mjs @@ -26,11 +26,12 @@ function createBundle(source, prefix, label) { } function createEnvironment(directory) { - const home = path.join(directory, 'home'); + const home = path.join(directory, 'home', '.vite-plus'); const system = path.join(directory, 'system/bin'); const runtime = path.join(home, 'js_runtime/node', process.versions.node, 'bin'); fs.mkdirSync(system, { recursive: true }); fs.mkdirSync(runtime, { recursive: true }); + fs.writeFileSync(path.join(home, '.vp-owned'), 'vite-plus-owned-directory\n'); fs.symlinkSync(process.execPath, path.join(system, 'node')); fs.symlinkSync(process.execPath, path.join(runtime, 'node')); fs.writeFileSync(path.join(directory, '.node-version'), process.versions.node); @@ -40,7 +41,7 @@ function createEnvironment(directory) { } return { ...env, - HOME: home, + HOME: path.dirname(home), VP_HOME: home, VP_SELF_SETUP_NO_MODIFY_PATH: '1', NPM_CONFIG_REGISTRY: 'http://127.0.0.1:9', diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs b/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs index 35a0a7b280..b3f5f4d65a 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs @@ -579,6 +579,8 @@ impl CaseHome { let this = Self { home: root.join("home") }; let vp_home = this.vp_home(); std::fs::create_dir_all(&vp_home).unwrap(); + // Provisioning emulates a fresh install into this case-owned directory. + std::fs::write(vp_home.join(".vp-owned"), b"vite-plus-owned-directory\n").unwrap(); std::fs::create_dir_all(root.join("npm-global/lib")).unwrap(); // Best-effort: if linking fails, the case downloads the runtime. if seed_runtime && let Some(seed) = flavor::js_runtime_seed_dir() { diff --git a/crates/vp_global_cli/src/commands/implode.rs b/crates/vp_global_cli/src/commands/implode.rs index 75e3cb68e8..e80d6ad1eb 100644 --- a/crates/vp_global_cli/src/commands/implode.rs +++ b/crates/vp_global_cli/src/commands/implode.rs @@ -1,10 +1,8 @@ //! `vp implode` — completely remove vp and all its data from this system. -use std::{ - io::Write, - path::{Component, Path, PathBuf}, - process::ExitStatus, -}; +#[cfg(any(windows, test))] +use std::path::{Path, PathBuf}; +use std::{io::Write, process::ExitStatus}; use console::style; use rustc_hash::FxHashSet; @@ -20,6 +18,7 @@ use crate::{ shell::{ALL_SHELL_PROFILES, ShellProfileKind, abbreviate_home_path, resolve_profile_path}, }, error::Error, + install_dirs::{self, lexical_path}, shim::DEFAULT_SHIM_TOOLS, }; @@ -36,20 +35,6 @@ const DEFERRED_DELETE_ATTEMPTS: usize = 100; #[cfg(windows)] const DEFERRED_DELETE_RETRY_DELAY: std::time::Duration = std::time::Duration::from_millis(100); -fn lexical_path(path: &Path) -> PathBuf { - let mut normalized = PathBuf::new(); - for component in path.components() { - match component { - Component::CurDir => {} - Component::ParentDir => { - normalized.pop(); - } - _ => normalized.push(component.as_os_str()), - } - } - normalized -} - fn validate_delete_roots( roots: &[AbsolutePathBuf], user_home: &AbsolutePathBuf, @@ -89,21 +74,7 @@ pub fn execute(yes: bool) -> Result { // The default Unix `` is under ``, so removal of `` also // removes it. Never remove a separately resolved `` because a bin from // an explicit override group can be shared. Remove only Vite+-owned shims. - let mut roots: Vec = [&dirs.data, &dirs.cache, &dirs.config, &dirs.state] - .into_iter() - .map(|root| { - AbsolutePathBuf::new(lexical_path(root.as_path())) - .expect("resolved Vite+ roots remain absolute after lexical normalization") - }) - .collect(); - roots.sort_by(|a, b| a.as_path().cmp(b.as_path())); - roots.dedup(); - let mut delete_set: Vec = Vec::new(); - for root in roots { - if !delete_set.iter().any(|kept| root.as_path().starts_with(kept.as_path())) { - delete_set.push(root); - } - } + let mut delete_set = install_dirs::roots(dirs); if !delete_set.iter().any(|root| root.as_path().exists()) { if homebrew { @@ -118,6 +89,9 @@ pub fn execute(yes: bool) -> Result { // Validate every deletion target before changing profiles, PATH, or files, even with --yes. let user_home = &env_config.user_home; validate_delete_roots(&delete_set, user_home)?; + install_dirs::require_owned(&delete_set)?; + // Sorted roots can be collapsed only after every category has passed validation. + delete_set.dedup_by(|child, parent| child.as_path().starts_with(parent.as_path())); let source_matcher = VitePlusSourceMatcher::new(&dirs.config, user_home); @@ -1679,8 +1653,9 @@ mod tests { ); } + #[cfg(unix)] #[test] - fn execute_normalizes_category_roots_before_deduplication() { + fn execute_requires_each_owned_root_before_removing_split_installation() { let temp_dir = tempfile::tempdir().unwrap(); let home = temp_dir.path().join("home"); let bin = temp_dir.path().join("bin"); @@ -1692,8 +1667,6 @@ mod tests { std::fs::create_dir_all(&home).unwrap(); std::fs::create_dir_all(&data).unwrap(); std::fs::create_dir_all(&normalized_cache).unwrap(); - std::fs::write(data.join("data.txt"), b"data").unwrap(); - std::fs::write(normalized_cache.join("cache.txt"), b"cache").unwrap(); vp_shared::EnvConfig::with_vars( [ @@ -1702,13 +1675,43 @@ mod tests { (env_vars::VP_DATA_DIR, Some(data.as_os_str())), (env_vars::VP_CACHE_DIR, Some(cache.as_os_str())), (env_vars::XDG_CONFIG_HOME, Some(xdg_config.as_os_str())), + (env_vars::XDG_DATA_HOME, Some(xdg_config.as_os_str())), (env_vars::XDG_STATE_HOME, Some(xdg_state.as_os_str())), ("HOME", Some(home.as_os_str())), ("USERPROFILE", Some(home.as_os_str())), + ("ZDOTDIR", Some(home.as_os_str())), ], - |_| { + |config| { + install_dirs::prepare(&config.dirs, false).unwrap(); + std::fs::write(data.join("data.txt"), b"data").unwrap(); + std::fs::write(normalized_cache.join("cache.txt"), b"cache").unwrap(); + let profile = vt_str::format!( + "# personal config\n# Vite+ bin\n. \"{}/env\"\n", + config.dirs.config.as_path().display() + ); + std::fs::write(home.join(".bashrc"), profile.as_bytes()).unwrap(); + for root in install_dirs::roots(&config.dirs) { + let marker = root.join(install_dirs::OWNERSHIP_MARKER); + let contents = std::fs::read(&marker).unwrap(); + std::fs::remove_file(&marker).unwrap(); + assert!(execute(true).unwrap_err().to_string().contains("ownership")); + assert_eq!(std::fs::read(data.join("data.txt")).unwrap(), b"data"); + assert_eq!( + std::fs::read(normalized_cache.join("cache.txt")).unwrap(), + b"cache" + ); + assert_eq!( + std::fs::read_to_string(home.join(".bashrc")).unwrap(), + profile.as_str() + ); + std::fs::write(&marker, contents).unwrap(); + } let result = execute(true).unwrap(); assert!(result.success()); + assert_eq!( + std::fs::read_to_string(home.join(".bashrc")).unwrap(), + "# personal config\n" + ); assert!(!data.exists(), "data root must be removed"); assert!( !normalized_cache.exists(), diff --git a/crates/vp_global_cli/src/install_dirs.rs b/crates/vp_global_cli/src/install_dirs.rs new file mode 100644 index 0000000000..fb78877058 --- /dev/null +++ b/crates/vp_global_cli/src/install_dirs.rs @@ -0,0 +1,195 @@ +//! Ownership of directories that `vp implode` may remove recursively. + +use std::{ + fs, + io::Write, + path::{Component, Path, PathBuf}, +}; + +use vp_shared::VpDirs; +use vt_path::AbsolutePathBuf; + +use crate::error::Error; + +pub(crate) const OWNERSHIP_MARKER: &str = ".vp-owned"; +const OWNERSHIP_CONTENT: &[u8] = b"vite-plus-owned-directory\n"; + +pub(crate) fn lexical_path(path: &Path) -> PathBuf { + let mut normalized = PathBuf::new(); + for component in path.components() { + match component { + Component::CurDir => {} + Component::ParentDir => { + normalized.pop(); + } + _ => normalized.push(component.as_os_str()), + } + } + normalized +} + +/// Distinct data, cache, config, and state roots, including nested categories. +pub(crate) fn roots(dirs: &VpDirs) -> Vec { + let mut roots: Vec<_> = [&dirs.data, &dirs.cache, &dirs.config, &dirs.state] + .into_iter() + .map(|root| { + AbsolutePathBuf::new(lexical_path(root.as_path())) + .expect("resolved Vite+ roots remain absolute after lexical normalization") + }) + .collect(); + roots.sort_by(|a, b| a.as_path().cmp(b.as_path())); + roots.dedup(); + roots +} + +fn is_owned(root: &AbsolutePathBuf) -> Result { + match fs::symlink_metadata(root.join(OWNERSHIP_MARKER)) { + Ok(metadata) => Ok(metadata.is_file()), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(false), + Err(error) => Err(error.into()), + } +} + +pub(crate) fn require_owned(roots: &[AbsolutePathBuf]) -> Result<(), Error> { + for root in roots { + if root.as_path().try_exists()? && !is_owned(root)? { + return Err(Error::Other(vt_str::format!( + "Refusing to remove {}: Vite+ directory ownership could not be verified. Unmarked directories, including older installations, must be inspected and removed manually.", + root.as_path().display() + ))); + } + } + Ok(()) +} + +/// Claim only empty or newly created roots. Legacy installs may remain unowned, +/// but updating them must never grant permission to delete their entire roots. +pub(crate) fn prepare(dirs: &VpDirs, existing_install: bool) -> Result<(), Error> { + let mut empty = Vec::new(); + // Preflight all roots before creating directories or ownership markers. + for root in roots(dirs) { + if is_owned(&root)? { + continue; + } + let nonempty = match fs::read_dir(&root) { + Ok(mut entries) => entries.next().transpose()?.is_some(), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => false, + Err(error) => return Err(error.into()), + }; + if nonempty { + if existing_install { + continue; + } + return Err(Error::Other(vt_str::format!( + "Refusing to initialize {}: the directory is not empty and Vite+ ownership could not be verified. Choose an empty installation directory.", + root.as_path().display() + ))); + } + empty.push(root); + } + for root in empty { + fs::create_dir_all(&root)?; + let mut marker = fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(root.join(OWNERSHIP_MARKER))?; + marker.write_all(OWNERSHIP_CONTENT)?; + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use vp_shared::EnvConfig; + + use super::*; + + #[test] + fn roots_normalize_category_paths_without_discarding_nested_roots() { + EnvConfig::scoped(|env| { + let mut dirs = env.dirs.clone(); + dirs.data = env.dirs.data.join("data"); + dirs.cache = dirs.data.join("../cache"); + dirs.config = dirs.data.join("./config"); + dirs.state = dirs.data.join("../data"); + assert_eq!( + roots(&dirs), + vec![env.dirs.data.join("cache"), dirs.data.clone(), dirs.data.join("config")] + ); + }); + } + + #[test] + fn prepare_claims_empty_roots_and_preserves_shared_bin() { + for split in [false, true] { + EnvConfig::scoped(|env| { + let mut dirs = env.dirs.clone(); + if split { + dirs.data = env.dirs.data.join("data"); + dirs.cache = env.dirs.data.join("cache"); + dirs.config = env.dirs.data.join("config"); + dirs.state = env.dirs.data.join("state"); + fs::create_dir_all(&dirs.bin).unwrap(); + fs::write(dirs.bin.join("other-tool"), b"keep").unwrap(); + } + prepare(&dirs, false).unwrap(); + for root in roots(&dirs) { + assert!(is_owned(&root).unwrap()); + } + fs::write(dirs.data.join("payload"), b"vp").unwrap(); + prepare(&dirs, false).unwrap(); + assert_eq!(fs::read(dirs.data.join("payload")).unwrap(), b"vp"); + assert!(!dirs.bin.join(OWNERSHIP_MARKER).as_path().exists()); + if split { + assert_eq!(fs::read(dirs.bin.join("other-tool")).unwrap(), b"keep"); + } + }); + } + } + + #[test] + fn prepare_preflights_all_roots_and_never_claims_legacy_contents() { + EnvConfig::scoped(|env| { + let mut dirs = env.dirs.clone(); + dirs.data = env.dirs.data.join("a-data"); + dirs.config = env.dirs.data.join("b-config"); + dirs.state = env.dirs.data.join("c-state"); + dirs.cache = env.dirs.data.join("z-cache"); + fs::create_dir_all(&dirs.cache).unwrap(); + fs::write(dirs.cache.join("personal.txt"), b"keep").unwrap(); + assert!(prepare(&dirs, false).is_err()); + for root in [&dirs.data, &dirs.config, &dirs.state] { + assert!(!root.as_path().exists()); + } + prepare(&dirs, true).unwrap(); + assert!(is_owned(&dirs.data).unwrap()); + assert!(!dirs.cache.join(OWNERSHIP_MARKER).as_path().exists()); + assert_eq!(fs::read(dirs.cache.join("personal.txt")).unwrap(), b"keep"); + assert!(require_owned(&roots(&dirs)).is_err()); + }); + } + + #[test] + fn ownership_requires_a_regular_marker_file() { + EnvConfig::scoped(|env| { + prepare(&env.dirs, false).unwrap(); + let roots = roots(&env.dirs); + let marker = env.dirs.cache.join(OWNERSHIP_MARKER); + fs::remove_file(&marker).unwrap(); + assert!(require_owned(&roots).is_err()); + fs::create_dir(&marker).unwrap(); + assert!(require_owned(&roots).is_err()); + fs::remove_dir(&marker).unwrap(); + #[cfg(unix)] + { + let target = env.dirs.data.join("elsewhere"); + fs::write(&target, OWNERSHIP_CONTENT).unwrap(); + std::os::unix::fs::symlink(&target, &marker).unwrap(); + assert!(require_owned(&roots).is_err()); + fs::remove_file(&marker).unwrap(); + } + fs::write(&marker, b"").unwrap(); + require_owned(&roots).unwrap(); + }); + } +} diff --git a/crates/vp_global_cli/src/main.rs b/crates/vp_global_cli/src/main.rs index 5259962b08..d72c6533f3 100644 --- a/crates/vp_global_cli/src/main.rs +++ b/crates/vp_global_cli/src/main.rs @@ -18,6 +18,7 @@ mod commands; mod error; mod help; mod homebrew; +mod install_dirs; mod js_executor; mod self_setup; mod shim; diff --git a/crates/vp_global_cli/src/self_setup.rs b/crates/vp_global_cli/src/self_setup.rs index b220b5d88b..c7df91b322 100644 --- a/crates/vp_global_cli/src/self_setup.rs +++ b/crates/vp_global_cli/src/self_setup.rs @@ -227,6 +227,10 @@ async fn run(source: &Path, bundled: bool) -> Result { } else { version_dir.join("bin").join(VP_BINARY_NAME) }; + crate::install_dirs::prepare( + dirs, + in_place || configured || active_binary.as_path().is_file(), + )?; if deploy { tokio::fs::create_dir_all(version_dir.join("bin")).await?; install::clear_self_setup_marker(&version_dir).await?; From ca1a7958495a6ce22f46b8dd018ff322e94823f8 Mon Sep 17 00:00:00 2001 From: JongKyung Lee Date: Sun, 25 Oct 2026 14:53:40 +0900 Subject: [PATCH 3/4] fix(setup): allow repair of incomplete legacy installations Recognize an existing current entry without following its target, matching installation directory resolution even when the active binary is missing. Keep repaired legacy roots unmarked so implode still refuses their removal. Extend the existing installer CI cases to verify ownership is not claimed when reusing a legacy root or repairing a dangling junction. --- .github/workflows/test-standalone-install.yml | 4 ++++ crates/vp_global_cli/src/self_setup.rs | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test-standalone-install.yml b/.github/workflows/test-standalone-install.yml index 06768611ef..87aaa3c3ed 100644 --- a/.github/workflows/test-standalone-install.yml +++ b/.github/workflows/test-standalone-install.yml @@ -532,6 +532,7 @@ jobs: VP_LOCAL_TGZ="$FAKE_TGZ" VP_VERSION=local-test bash packages/cli/install.sh test -f "$GRAND/.vite-plus/.keep" + test ! -e "$GRAND/.vite-plus/.vp-owned" test -e "$GRAND/.vite-plus/current" test -e "$GRAND/.vite-plus/bin/vp" test ! -d "$GRAND/.local/share/vite-plus" @@ -2168,6 +2169,9 @@ jobs: if (-not (Test-Path (Join-Path $env:VP_HOME "current/bin/.vp-setup-complete"))) { throw "vp-setup.exe did not complete the binary's self-setup" } + if (Test-Path (Join-Path $env:VP_HOME ".vp-owned")) { + throw "Repair claimed ownership of an unmarked installation root" + } if ([Environment]::GetEnvironmentVariable("Path", "User") -cne $pathBefore) { throw "--no-modify-path changed User PATH" } diff --git a/crates/vp_global_cli/src/self_setup.rs b/crates/vp_global_cli/src/self_setup.rs index c7df91b322..8a3be5a531 100644 --- a/crates/vp_global_cli/src/self_setup.rs +++ b/crates/vp_global_cli/src/self_setup.rs @@ -229,7 +229,7 @@ async fn run(source: &Path, bundled: bool) -> Result { }; crate::install_dirs::prepare( dirs, - in_place || configured || active_binary.as_path().is_file(), + in_place || configured || std::fs::symlink_metadata(dirs.data.join("current")).is_ok(), )?; if deploy { tokio::fs::create_dir_all(version_dir.join("bin")).await?; From 03ddbdfc7ce1b84326afcf11fe5a8d7b685eb1b4 Mon Sep 17 00:00:00 2001 From: JongKyung Lee Date: Sun, 25 Oct 2026 17:22:01 +0900 Subject: [PATCH 4/4] 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?;