fix(command): restrict .cmd to PowerShell rewrite to vp-managed shims

Per the Codex adversarial review: the previous implementation
rewrote any `.cmd` with a sibling `.ps1` to spawn through
`PowerShell -ExecutionPolicy Bypass`. That covers vp's own pm
shims but also silently retargets unrelated PATH commands —
system tools, globally-installed npm wrappers, third-party CLIs
whose `.cmd`/`.ps1` wrappers may not be semantically equivalent —
and broadens execution semantics on locked-down hosts that
intentionally block unsigned `.ps1` execution.

Gate the rewrite on `resolved.starts_with($VP_HOME)`, where
`$VP_HOME` is the vp install root (`~/.vite-plus` by default).
That covers the legitimate cases (`$VP_HOME/js_runtime/node/<v>/npm.cmd`
and `$VP_HOME/package_manager/<pm>/<v>/<pm>/bin/<pm>.cmd`) and
leaves anything else alone. `$VP_HOME` is read once via
`vite_shared::get_vp_home()` and cached in a `LazyLock`.

Adds a regression test (`returns_none_for_cmd_outside_vp_home`)
that puts a matching `.cmd`+`.ps1` pair outside the scope and
asserts no rewrite happens.

Author: fengmk2

Cargo.lock

@@ -15,6 +15,7 @@ tracing = { workspace = true }
 vite_error = { workspace = true }
 vite_path = { workspace = true }
 vite_powershell = { workspace = true }
+vite_shared = { workspace = true }
 which = { workspace = true, features = ["tracing"] }
 
 [target.'cfg(not(target_os = "windows"))'.dependencies]

crates/vite_command/Cargo.toml

@@ -1,22 +1,28 @@
-//! Windows-specific: when a resolved binary is a `.cmd` shim with a sibling
-//! `.ps1`, rewrite the spawn to go through `powershell.exe -File <sibling.ps1>`.
+//! Windows-specific: when a vp-managed package-manager `.cmd` shim has a
+//! sibling `.ps1`, rewrite the spawn to go through
+//! `powershell.exe -File <sibling.ps1>`.
 //!
 //! Running a `.cmd` from any shell makes `cmd.exe` prompt "Terminate batch
 //! job (Y/N)?" on Ctrl+C, which leaves the terminal corrupt. Routing through
 //! `PowerShell` sidesteps the prompt and lets Ctrl+C propagate cleanly.
 //!
-//! Unlike the task-layer rewrite (`vite_task_plan::ps1_shim`, scoped to
-//! `node_modules/.bin/*.cmd` inside the workspace), this one applies to any
-//! `.cmd` whose `.ps1` sibling exists. Package manager shims (`npm.cmd`,
-//! `pnpm.cmd`, `yarn.cmd`, `npx.cmd`) live in `~/.vite-plus/js_runtime/...`
-//! or system PATH — outside any `node_modules/.bin` — so the structural
-//! check there is too narrow for this code path.
+//! The rewrite is intentionally **scoped to paths inside `$VP_HOME`**
+//! (`~/.vite-plus` by default). vp's managed shims live there:
+//!   - `$VP_HOME/js_runtime/node/<ver>/{npm,npx}.cmd` (npm/npx shipped with
+//!     the managed Node.js),
+//!   - `$VP_HOME/package_manager/<pm>/<ver>/<pm>/bin/<pm>.cmd` (downloaded
+//!     pnpm/yarn/bun).
 //!
-//! The cross-platform primitives (`POWERSHELL_PREFIX`, `powershell_host`,
-//! `find_ps1_sibling`) live in the `vite_powershell` crate and are shared
-//! with `vite_task_plan::ps1_shim`. This module composes them with vp's
-//! own conventions: absolute `.ps1` path in args (no cache fingerprint to
-//! keep portable) and `Vec<OsString>` return type (matches the spawn API).
+//! Anything outside `$VP_HOME` — system tools, globally-installed npm
+//! shims, third-party CLIs whose `.cmd` and `.ps1` wrappers may not be
+//! semantically equivalent, hosts that intentionally block unsigned
+//! `.ps1` execution — is left alone. Stricter scoping means the prior
+//! `.cmd` path still runs there, but that matches the pre-fix status quo
+//! and avoids broadening execution semantics for unrelated commands.
+//!
+//! The task-layer rewrite (`vite_task_plan::ps1_shim`) is scoped
+//! differently — to `node_modules/.bin/*.cmd` inside the workspace — and
+//! covers `vp run <script>`. The two scopes don't overlap.
 //!
 //! See <https://github.com/voidzero-dev/vite-plus/issues/1489>
 //! and <https://github.com/voidzero-dev/vite-plus/issues/1176>.
@@ -26,32 +32,51 @@ use std::{ffi::OsString, sync::Arc};
 use vite_path::{AbsolutePath, AbsolutePathBuf};
 use vite_powershell::{POWERSHELL_PREFIX, find_ps1_sibling, powershell_host};
 
-/// Rewrite a resolved `.cmd` invocation to go through PowerShell.
+/// Rewrite a vp-managed `.cmd` invocation to go through PowerShell.
 ///
-/// Returns `Some((powershell_host, prefix_args))` when the rewrite applies,
-/// where `prefix_args` is `["-NoProfile", "-NoLogo", "-ExecutionPolicy",
-/// "Bypass", "-File", <abs ps1 path>]`. Caller prepends `prefix_args` to the
-/// user args and spawns `powershell_host`.
+/// Returns `Some((powershell_host, prefix_args))` when the rewrite applies.
+/// `prefix_args` is `["-NoProfile", "-NoLogo", "-ExecutionPolicy", "Bypass",
+/// "-File", <abs ps1 path>]`; callers prepend it to the user args and spawn
+/// `powershell_host`.
 ///
 /// Returns `None` when:
 /// - not on Windows,
 /// - no PowerShell host (`pwsh.exe` or `powershell.exe`) is on PATH,
+/// - `$VP_HOME` could not be resolved,
+/// - the resolved path is **outside** `$VP_HOME`,
 /// - the resolved path is not a `.cmd` (case-insensitive),
 /// - the `.cmd` has no sibling `.ps1`.
 #[must_use]
 pub fn rewrite_cmd_to_powershell(
     resolved: &AbsolutePath,
 ) -> Option<(AbsolutePathBuf, Vec<OsString>)> {
+    let vp_home = vp_home()?;
     let host = powershell_host()?;
-    rewrite_with_host(resolved, host)
+    rewrite_in_scope(resolved, vp_home, host)
+}
+
+/// Cached `$VP_HOME` (`~/.vite-plus` by default; overridable via env var).
+/// `None` only if `vite_shared::get_vp_home()` failed to resolve a home —
+/// in that case we conservatively skip the rewrite rather than retarget
+/// arbitrary PATH commands.
+fn vp_home() -> Option<&'static AbsolutePathBuf> {
+    use std::sync::LazyLock;
+
+    static VP_HOME: LazyLock<Option<AbsolutePathBuf>> =
+        LazyLock::new(|| vite_shared::get_vp_home().ok());
+    VP_HOME.as_ref()
 }
 
 /// Pure rewrite logic. Factored out so tests can drive it on any platform
-/// without depending on a real `powershell.exe`.
-fn rewrite_with_host(
+/// without depending on a real `powershell.exe` or a real `$VP_HOME`.
+fn rewrite_in_scope(
     resolved: &AbsolutePath,
+    vp_home: &AbsolutePath,
     host: &Arc<AbsolutePath>,
 ) -> Option<(AbsolutePathBuf, Vec<OsString>)> {
+    if !resolved.as_path().starts_with(vp_home.as_path()) {
+        return None;
+    }
     let ps1 = find_ps1_sibling(resolved)?;
 
     tracing::debug!(
@@ -86,36 +111,70 @@ mod tests {
     }
 
     #[test]
-    fn rewrites_cmd_to_powershell_with_sibling_ps1() {
+    fn rewrites_cmd_inside_vp_home_to_powershell() {
         let dir = tempdir().unwrap();
-        let root = abs(dir.path().canonicalize().unwrap());
-        fs::write(root.as_path().join("npm.cmd"), "").unwrap();
-        fs::write(root.as_path().join("npm.ps1"), "").unwrap();
+        let vp_home = abs(dir.path().canonicalize().unwrap());
+        // Mimic the real layout: $VP_HOME/js_runtime/node/<ver>/npm.cmd.
+        let bin_dir = vp_home.as_path().join("js_runtime").join("node").join("24.0.0");
+        fs::create_dir_all(&bin_dir).unwrap();
+        fs::write(bin_dir.join("npm.cmd"), "").unwrap();
+        fs::write(bin_dir.join("npm.ps1"), "").unwrap();
 
-        let host = host_arc(&root);
-        let resolved = abs(root.as_path().join("npm.cmd"));
+        let host = host_arc(&vp_home);
+        let resolved = abs(bin_dir.join("npm.cmd"));
 
-        let (program, prefix_args) = rewrite_with_host(&resolved, &host).expect("should rewrite");
+        let (program, prefix_args) =
+            rewrite_in_scope(&resolved, &vp_home, &host).expect("should rewrite");
 
         assert_eq!(program.as_path(), host.as_path());
         let as_strs: Vec<&str> = prefix_args.iter().filter_map(|a| a.to_str()).collect();
-        let ps1_path = root.as_path().join("npm.ps1");
-        let ps1_str = ps1_path.to_str().unwrap();
+        let ps1_str = bin_dir.join("npm.ps1");
+        let ps1_str = ps1_str.to_str().unwrap();
         assert_eq!(
             as_strs,
             vec!["-NoProfile", "-NoLogo", "-ExecutionPolicy", "Bypass", "-File", ps1_str]
         );
     }
 
+    /// Regression for the Codex review: the rewrite must NOT retarget
+    /// `.cmd` files that live outside `$VP_HOME` even when a sibling
+    /// `.ps1` exists. Otherwise unrelated PATH commands (system tools,
+    /// globally installed npm wrappers, third-party CLIs whose `.cmd`
+    /// and `.ps1` wrappers diverge) would silently switch to PowerShell
+    /// with `-ExecutionPolicy Bypass`.
     #[test]
-    fn returns_none_when_no_ps1_sibling() {
+    fn returns_none_for_cmd_outside_vp_home() {
         let dir = tempdir().unwrap();
         let root = abs(dir.path().canonicalize().unwrap());
-        fs::write(root.as_path().join("npm.cmd"), "").unwrap();
+        let vp_home_path = root.as_path().join("vite-plus");
+        fs::create_dir_all(&vp_home_path).unwrap();
+        let vp_home = abs(vp_home_path);
+
+        // A `.cmd`+`.ps1` pair *outside* vp_home — e.g. a global install at
+        // `%AppData%\npm\node_modules\.bin\foo.cmd` or any third-party tool.
+        let outside_bin = root.as_path().join("global").join("bin");
+        fs::create_dir_all(&outside_bin).unwrap();
+        fs::write(outside_bin.join("foo.cmd"), "").unwrap();
+        fs::write(outside_bin.join("foo.ps1"), "").unwrap();
 
         let host = host_arc(&root);
-        let resolved = abs(root.as_path().join("npm.cmd"));
+        let resolved = abs(outside_bin.join("foo.cmd"));
+
+        assert!(
+            rewrite_in_scope(&resolved, &vp_home, &host).is_none(),
+            "rewrite must stay hands-off for .cmd outside $VP_HOME"
+        );
+    }
+
+    #[test]
+    fn returns_none_when_no_ps1_sibling() {
+        let dir = tempdir().unwrap();
+        let vp_home = abs(dir.path().canonicalize().unwrap());
+        fs::write(vp_home.as_path().join("npm.cmd"), "").unwrap();
+
+        let host = host_arc(&vp_home);
+        let resolved = abs(vp_home.as_path().join("npm.cmd"));
 
-        assert!(rewrite_with_host(&resolved, &host).is_none());
+        assert!(rewrite_in_scope(&resolved, &vp_home, &host).is_none());
     }
 }