Skip to content

Commit c4cdfc9

Browse files
committed
fix(ps1_shim): decouple node_modules/.bin scope from VP_HOME resolution
`rewrite_cmd_to_powershell` previously bailed early with `vp_home()?`, which silently blocked rewrites for `node_modules/.bin/*.cmd` paths whenever `vite_shared::get_vp_home()` failed (e.g. CI containers without `$HOME`, sandboxed shells). The two scopes are documented as architecturally independent — one targets vp-managed shims under `$VP_HOME`, the other targets npm/pnpm/yarn-emitted shims under `node_modules/.bin/` — but the implementation coupled them by short-circuiting on the vp_home lookup. Make `vp_home` optional through `rewrite_in_scope` and `is_in_managed_scope` so an unresolvable `$VP_HOME` only disables the vp-home arm of the `||`; `node_modules/.bin` rewrites still apply. Add a regression test covering the unresolved-vp_home case. Reported by Cursor Bugbot on PR #1498.
1 parent 8548795 commit c4cdfc9

1 file changed

Lines changed: 37 additions & 16 deletions

File tree

crates/vite_command/src/ps1_shim.rs

Lines changed: 37 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -38,24 +38,22 @@ use vite_powershell::{POWERSHELL_PREFIX, find_ps1_sibling, powershell_host};
3838
/// Returns `None` when:
3939
/// - not on Windows,
4040
/// - no PowerShell host (`pwsh.exe` or `powershell.exe`) is on PATH,
41-
/// - `$VP_HOME` could not be resolved,
42-
/// - the resolved path is outside `$VP_HOME` AND not under any
43-
/// `node_modules/.bin/`,
41+
/// - the resolved path is outside `$VP_HOME` (or `$VP_HOME` is
42+
/// unresolvable) AND not under any `node_modules/.bin/`,
4443
/// - the resolved path is not a `.cmd` (case-insensitive),
4544
/// - the `.cmd` has no sibling `.ps1`.
4645
#[must_use]
4746
pub fn rewrite_cmd_to_powershell(
4847
resolved: &AbsolutePath,
4948
) -> Option<(AbsolutePathBuf, Vec<OsString>)> {
5049
let host = powershell_host()?;
51-
let vp_home = vp_home()?;
52-
rewrite_in_scope(resolved, vp_home, host)
50+
rewrite_in_scope(resolved, vp_home().map(AsRef::as_ref), host)
5351
}
5452

5553
/// Cached `$VP_HOME` (`~/.vite-plus` by default; overridable via env var).
56-
/// `None` only if `vite_shared::get_vp_home()` failed to resolve a home —
57-
/// in that case we conservatively skip the rewrite rather than retarget
58-
/// arbitrary PATH commands.
54+
/// Returns `None` if `vite_shared::get_vp_home()` failed; the rewrite still
55+
/// applies to `node_modules/.bin/*.cmd` paths in that case (the two scopes
56+
/// are independent).
5957
fn vp_home() -> Option<&'static AbsolutePathBuf> {
6058
use std::sync::LazyLock;
6159

@@ -68,7 +66,7 @@ fn vp_home() -> Option<&'static AbsolutePathBuf> {
6866
/// without depending on a real `powershell.exe` or a real `$VP_HOME`.
6967
fn rewrite_in_scope(
7068
resolved: &AbsolutePath,
71-
vp_home: &AbsolutePath,
69+
vp_home: Option<&AbsolutePath>,
7270
host: &AbsolutePath,
7371
) -> Option<(AbsolutePathBuf, Vec<OsString>)> {
7472
if !is_in_managed_scope(resolved, vp_home) {
@@ -90,8 +88,9 @@ fn rewrite_in_scope(
9088
Some((host.to_absolute_path_buf(), prefix_args))
9189
}
9290

93-
fn is_in_managed_scope(resolved: &AbsolutePath, vp_home: &AbsolutePath) -> bool {
94-
resolved.as_path().starts_with(vp_home.as_path()) || is_in_node_modules_bin(resolved)
91+
fn is_in_managed_scope(resolved: &AbsolutePath, vp_home: Option<&AbsolutePath>) -> bool {
92+
let in_vp_home = vp_home.is_some_and(|home| resolved.as_path().starts_with(home.as_path()));
93+
in_vp_home || is_in_node_modules_bin(resolved)
9594
}
9695

9796
/// `true` when `resolved` is `<...>/node_modules/.bin/<file>` (matched
@@ -139,7 +138,7 @@ mod tests {
139138
let resolved = abs(bin_dir.join("npm.cmd"));
140139

141140
let (program, prefix_args) =
142-
rewrite_in_scope(&resolved, &vp_home, &host).expect("should rewrite");
141+
rewrite_in_scope(&resolved, Some(&vp_home), &host).expect("should rewrite");
143142

144143
assert_eq!(program.as_path(), host.as_path());
145144
let as_strs: Vec<&str> = prefix_args.iter().filter_map(|a| a.to_str()).collect();
@@ -171,10 +170,32 @@ mod tests {
171170
let host = host_buf(&root);
172171
let resolved = abs(bin.join("vite.cmd"));
173172

174-
let result = rewrite_in_scope(&resolved, &vp_home, &host);
173+
let result = rewrite_in_scope(&resolved, Some(&vp_home), &host);
175174
assert!(result.is_some(), "any node_modules/.bin/*.cmd must rewrite");
176175
}
177176

177+
/// `vp_home` may be unresolvable in unusual environments (CI containers
178+
/// missing $HOME, sandboxed shells); when that happens the
179+
/// `node_modules/.bin` scope must still rewrite, since it is
180+
/// architecturally independent from the `$VP_HOME` scope.
181+
#[test]
182+
fn rewrites_cmd_in_node_modules_bin_when_vp_home_unresolved() {
183+
let dir = tempdir().unwrap();
184+
let root = abs(dir.path().canonicalize().unwrap());
185+
let bin = root.as_path().join("my-project").join("node_modules").join(".bin");
186+
fs::create_dir_all(&bin).unwrap();
187+
fs::write(bin.join("vite.cmd"), "").unwrap();
188+
fs::write(bin.join("vite.ps1"), "").unwrap();
189+
190+
let host = host_buf(&root);
191+
let resolved = abs(bin.join("vite.cmd"));
192+
193+
assert!(
194+
rewrite_in_scope(&resolved, None, &host).is_some(),
195+
"node_modules/.bin must rewrite even without a resolvable vp_home"
196+
);
197+
}
198+
178199
/// The `.bin`/`node_modules` component check is case-insensitive so
179200
/// a `.CMD` shim under `Node_Modules\.Bin\` (or any casing variant)
180201
/// still matches.
@@ -193,7 +214,7 @@ mod tests {
193214
let host = host_buf(&root);
194215
let resolved = abs(bin.join("vite.cmd"));
195216

196-
assert!(rewrite_in_scope(&resolved, &vp_home, &host).is_some());
217+
assert!(rewrite_in_scope(&resolved, Some(&vp_home), &host).is_some());
197218
}
198219

199220
/// A `.cmd`+`.ps1` pair outside `$VP_HOME` AND outside any
@@ -216,7 +237,7 @@ mod tests {
216237
let resolved = abs(outside_bin.join("foo.cmd"));
217238

218239
assert!(
219-
rewrite_in_scope(&resolved, &vp_home, &host).is_none(),
240+
rewrite_in_scope(&resolved, Some(&vp_home), &host).is_none(),
220241
"rewrite must stay hands-off for .cmd outside both vp_home and node_modules/.bin"
221242
);
222243
}
@@ -230,6 +251,6 @@ mod tests {
230251
let host = host_buf(&vp_home);
231252
let resolved = abs(vp_home.as_path().join("npm.cmd"));
232253

233-
assert!(rewrite_in_scope(&resolved, &vp_home, &host).is_none());
254+
assert!(rewrite_in_scope(&resolved, Some(&vp_home), &host).is_none());
234255
}
235256
}

0 commit comments

Comments
 (0)