Skip to content

Commit fc05b17

Browse files
committed
fix(command): also rewrite .cmd shims under any node_modules/.bin
Extends the rewrite scope so `<...>/node_modules/.bin/*.cmd` is also routed through PowerShell, in addition to paths inside `$VP_HOME`. The structural check is purely path-component based (case-insensitive on `.bin`/`node_modules` to match Windows semantics) — no project locality gate. npm/pnpm/yarn-emitted shims always come with a sibling `.ps1`, so the rewrite stays self-consistent across single-package projects, hoisted monorepos, and globally-installed shims. Anything outside both scopes — system tools, third-party CLIs whose `.cmd`/`.ps1` wrappers may diverge — still keeps its existing `.cmd` path. Adds tests for the node_modules/.bin path and a casing variant (`Node_Modules\.Bin`); removes the now-redundant cwd-locality tests.
1 parent 434eef8 commit fc05b17

1 file changed

Lines changed: 90 additions & 26 deletions

File tree

crates/vite_command/src/ps1_shim.rs

Lines changed: 90 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -6,22 +6,27 @@
66
//! job (Y/N)?" on Ctrl+C, which leaves the terminal corrupt. Routing through
77
//! `PowerShell` sidesteps the prompt and lets Ctrl+C propagate cleanly.
88
//!
9-
//! The rewrite is intentionally **scoped to paths inside `$VP_HOME`**
10-
//! (`~/.vite-plus` by default). vp's managed shims live there:
11-
//! - `$VP_HOME/js_runtime/node/<ver>/{npm,npx}.cmd` (npm/npx shipped with
12-
//! the managed Node.js),
13-
//! - `$VP_HOME/package_manager/<pm>/<ver>/<pm>/bin/<pm>.cmd` (downloaded
14-
//! pnpm/yarn/bun).
9+
//! The rewrite is scoped to two patterns:
10+
//! - Inside `$VP_HOME` (`~/.vite-plus` by default) — vp's managed shims:
11+
//! - `$VP_HOME/js_runtime/node/<ver>/{npm,npx}.cmd`,
12+
//! - `$VP_HOME/package_manager/<pm>/<ver>/<pm>/bin/<pm>.cmd`.
13+
//! - Any `<...>/node_modules/.bin/*.cmd` — the canonical layout for
14+
//! npm/pnpm/yarn-emitted shims (cmd-shim writes both `.cmd` and `.ps1`
15+
//! so the wrappers stay equivalent).
1516
//!
16-
//! Anything outside `$VP_HOME` — system tools, globally-installed npm
17-
//! shims, third-party CLIs whose `.cmd` and `.ps1` wrappers may diverge —
18-
//! keeps its existing `.cmd` path (Ctrl+C corruption included), so we
19-
//! don't silently change execution semantics for unrelated commands or
20-
//! bypass execution policies on locked-down hosts.
17+
//! Anything outside both patterns — system tools, third-party CLIs whose
18+
//! `.cmd` and `.ps1` wrappers may diverge — keeps its existing `.cmd`
19+
//! path (Ctrl+C corruption included), so we don't silently change
20+
//! execution semantics for unrelated commands or bypass execution
21+
//! policies on locked-down hosts.
2122
//!
22-
//! The task-layer rewrite (`vite_task_plan::ps1_shim`) is scoped
23-
//! differently — to `node_modules/.bin/*.cmd` inside the workspace — and
24-
//! covers `vp run <script>`. The two scopes don't overlap.
23+
//! The task-layer rewrite (`vite_task_plan::ps1_shim`) covers the same
24+
//! `node_modules/.bin/*.cmd` pattern at task-graph plan time for `vp run
25+
//! <script>`. The two are complementary: the task-layer version records
26+
//! cwd-relative `.ps1` paths in the plan's spawn fingerprint (so it stays
27+
//! portable across machines), this one applies absolute-path rewriting at
28+
//! spawn time for paths the task layer doesn't see (pm-routed flows that
29+
//! go through `vite_command::run_command`).
2530
//!
2631
//! See <https://github.com/voidzero-dev/vite-plus/issues/1489>
2732
//! and <https://github.com/voidzero-dev/vite-plus/issues/1176>.
@@ -42,7 +47,8 @@ use vite_powershell::{POWERSHELL_PREFIX, find_ps1_sibling, powershell_host};
4247
/// - not on Windows,
4348
/// - no PowerShell host (`pwsh.exe` or `powershell.exe`) is on PATH,
4449
/// - `$VP_HOME` could not be resolved,
45-
/// - the resolved path is **outside** `$VP_HOME`,
50+
/// - the resolved path is outside `$VP_HOME` AND not under any
51+
/// `node_modules/.bin/`,
4652
/// - the resolved path is not a `.cmd` (case-insensitive),
4753
/// - the `.cmd` has no sibling `.ps1`.
4854
#[must_use]
@@ -73,7 +79,7 @@ fn rewrite_in_scope(
7379
vp_home: &AbsolutePath,
7480
host: &Arc<AbsolutePath>,
7581
) -> Option<(AbsolutePathBuf, Vec<OsString>)> {
76-
if !resolved.as_path().starts_with(vp_home.as_path()) {
82+
if !is_in_managed_scope(resolved, vp_home) {
7783
return None;
7884
}
7985
let ps1 = find_ps1_sibling(resolved)?;
@@ -92,6 +98,24 @@ fn rewrite_in_scope(
9298
Some((host.to_absolute_path_buf(), prefix_args))
9399
}
94100

101+
fn is_in_managed_scope(resolved: &AbsolutePath, vp_home: &AbsolutePath) -> bool {
102+
resolved.as_path().starts_with(vp_home.as_path()) || is_in_node_modules_bin(resolved)
103+
}
104+
105+
/// `true` when `resolved` is `<...>/node_modules/.bin/<file>` (matched
106+
/// case-insensitively on the `.bin`/`node_modules` components — Windows
107+
/// is case-insensitive, and pnpm's hoisted layouts can vary in casing).
108+
fn is_in_node_modules_bin(resolved: &AbsolutePath) -> bool {
109+
let mut parents = resolved.as_path().components().rev();
110+
parents.next(); // shim filename
111+
let Some(bin) = parents.next() else { return false };
112+
if !bin.as_os_str().eq_ignore_ascii_case(".bin") {
113+
return false;
114+
}
115+
let Some(node_modules) = parents.next() else { return false };
116+
node_modules.as_os_str().eq_ignore_ascii_case("node_modules")
117+
}
118+
95119
#[cfg(test)]
96120
mod tests {
97121
use std::fs;
@@ -135,22 +159,62 @@ mod tests {
135159
);
136160
}
137161

138-
/// Regression for the Codex review: the rewrite must NOT retarget
139-
/// `.cmd` files that live outside `$VP_HOME` even when a sibling
140-
/// `.ps1` exists. Otherwise unrelated PATH commands (system tools,
141-
/// globally installed npm wrappers, third-party CLIs whose `.cmd`
142-
/// and `.ps1` wrappers diverge) would silently switch to PowerShell
143-
/// with `-ExecutionPolicy Bypass`.
162+
/// Any `<...>/node_modules/.bin/*.cmd` rewrites, regardless of where
163+
/// the project root sits — covers single-package projects, hoisted
164+
/// monorepos, and globally-installed shims uniformly.
165+
#[test]
166+
fn rewrites_cmd_in_node_modules_bin() {
167+
let dir = tempdir().unwrap();
168+
let root = abs(dir.path().canonicalize().unwrap());
169+
// vp_home points elsewhere — this scope is the node_modules path.
170+
let vp_home_path = root.as_path().join("vite-plus");
171+
fs::create_dir_all(&vp_home_path).unwrap();
172+
let vp_home = abs(vp_home_path);
173+
174+
let bin = root.as_path().join("my-project").join("node_modules").join(".bin");
175+
fs::create_dir_all(&bin).unwrap();
176+
fs::write(bin.join("vite.cmd"), "").unwrap();
177+
fs::write(bin.join("vite.ps1"), "").unwrap();
178+
179+
let host = host_arc(&root);
180+
let resolved = abs(bin.join("vite.cmd"));
181+
182+
let result = rewrite_in_scope(&resolved, &vp_home, &host);
183+
assert!(result.is_some(), "any node_modules/.bin/*.cmd must rewrite");
184+
}
185+
186+
/// The `.bin`/`node_modules` component check is case-insensitive so
187+
/// a `.CMD` shim under `Node_Modules\.Bin\` (or any casing variant)
188+
/// still matches.
189+
#[test]
190+
fn rewrites_cmd_in_node_modules_bin_case_insensitive() {
191+
let dir = tempdir().unwrap();
192+
let root = abs(dir.path().canonicalize().unwrap());
193+
let vp_home = abs(root.as_path().join("vite-plus"));
194+
fs::create_dir_all(vp_home.as_path()).unwrap();
195+
196+
let bin = root.as_path().join("Node_Modules").join(".Bin");
197+
fs::create_dir_all(&bin).unwrap();
198+
fs::write(bin.join("vite.cmd"), "").unwrap();
199+
fs::write(bin.join("vite.ps1"), "").unwrap();
200+
201+
let host = host_arc(&root);
202+
let resolved = abs(bin.join("vite.cmd"));
203+
204+
assert!(rewrite_in_scope(&resolved, &vp_home, &host).is_some());
205+
}
206+
207+
/// A `.cmd`+`.ps1` pair outside `$VP_HOME` AND outside any
208+
/// `node_modules/.bin/` (e.g. a system tool living at `<root>/global/bin/foo.cmd`)
209+
/// must NOT be retargeted.
144210
#[test]
145-
fn returns_none_for_cmd_outside_vp_home() {
211+
fn returns_none_for_cmd_outside_managed_scope() {
146212
let dir = tempdir().unwrap();
147213
let root = abs(dir.path().canonicalize().unwrap());
148214
let vp_home_path = root.as_path().join("vite-plus");
149215
fs::create_dir_all(&vp_home_path).unwrap();
150216
let vp_home = abs(vp_home_path);
151217

152-
// A `.cmd`+`.ps1` pair *outside* vp_home — e.g. a global install at
153-
// `%AppData%\npm\node_modules\.bin\foo.cmd` or any third-party tool.
154218
let outside_bin = root.as_path().join("global").join("bin");
155219
fs::create_dir_all(&outside_bin).unwrap();
156220
fs::write(outside_bin.join("foo.cmd"), "").unwrap();
@@ -161,7 +225,7 @@ mod tests {
161225

162226
assert!(
163227
rewrite_in_scope(&resolved, &vp_home, &host).is_none(),
164-
"rewrite must stay hands-off for .cmd outside $VP_HOME"
228+
"rewrite must stay hands-off for .cmd outside both vp_home and node_modules/.bin"
165229
);
166230
}
167231

0 commit comments

Comments
 (0)