diff --git a/e2e/backend/test_npm_shim_recursion_system_dir b/e2e/backend/test_npm_shim_recursion_system_dir new file mode 100755 index 0000000000..9b095370e3 --- /dev/null +++ b/e2e/backend/test_npm_shim_recursion_system_dir @@ -0,0 +1,63 @@ +#!/usr/bin/env bash +# Regression test for the dual-dir gap left by #8475. +# +# #8475 fixed `dependency_env` to strip the user shims dir (`dirs::SHIMS`) so +# backend version-resolution subprocesses don't recurse through a mise shim. +# But it only stripped the user dir — when tools are installed via +# `mise install --system` (devcontainer / Docker setups), there's a *second* +# shims dir at `$MISE_SYSTEM_DATA_DIR/shims` that's also on PATH. The npm +# backend's `npm view yarn versions time --json` call found that system shim, +# re-entered mise, and the same fork bomb fired — reported as "ws agent run +# OOMs the sandbox" when an `npm:yarn = "1"` pin met an uninstalled node. +# +# This test mirrors `test_go_shim_recursion` but uses the SYSTEM shims dir +# specifically to validate the dual-dir fix. + +export MISE_SYSTEM_DATA_DIR="$HOME/.local/share/mise-system" +mkdir -p "$MISE_SYSTEM_DATA_DIR/shims" + +# Fake npm shim in the SYSTEM shims dir (not the user one). Mirrors how +# `mise install --system node@lts` creates `/usr/local/share/mise/shims/npm` +# pointing at the mise binary. +cat >"$MISE_SYSTEM_DATA_DIR/shims/npm" <<'SHIM' +#!/usr/bin/env bash +exec mise exec -- npm "$@" +SHIM +chmod +x "$MISE_SYSTEM_DATA_DIR/shims/npm" + +# System shims dir on PATH, exactly as a wsdev/devcontainer image would +# have it via `ENV PATH=".../usr/local/share/mise/shims:..."`. +export PATH="$MISE_SYSTEM_DATA_DIR/shims:$PATH" + +# Configure an npm:* backend tool + node uninstalled, so the npm backend's +# _list_remote_versions runs `npm view ... --json` and the only `npm` on +# PATH is the system shim we just planted. Use a real-but-uninstalled node +# version (matches the pattern in test_go_shim_recursion using +# `go = "1.23.3"`) — picking a non-existent version like `99.0.0` would risk +# `mise ls` failing on version resolution and masking a fork-bomb regression. +cat >>mise.toml <<'EOF' +[tools] +"npm:left-pad" = "latest" +node = "22.0.0" +EOF +mise trust --yes + +# Empty the remote-version cache so `_list_remote_versions` actually shells +# out to `npm view` instead of returning a cached list. +mise cache clear + +# Without the fix: dependency_env leaves the SYSTEM shims dir on PATH, the +# `npm view` call resolves to our fake shim, that shim re-runs `mise exec`, +# which re-resolves the toolset, calls `npm view` again — fork bomb until +# the timeout kills us. +# With the fix: both shim dirs are stripped, no `npm` is reachable in the +# dependency PATH, `npm view` fails cleanly, mise reports the resolution +# failure and exits. +# +# `|| true` keeps the test focused on "no fork bomb" rather than "ls returns +# zero" — `mise ls` may exit non-zero when npm:left-pad's version resolution +# fails (the expected failure when shims are stripped), and we don't want +# that to be confused with a recursion regression. The `assert_contains` +# on the actual output still verifies mise made it past resolution. +output="$(run_with_timeout 15 mise ls --json -c || true)" +assert_contains "echo '$output'" '"node"' diff --git a/src/backend/mod.rs b/src/backend/mod.rs index e8eb65968d..a45e1763bb 100644 --- a/src/backend/mod.rs +++ b/src/backend/mod.rs @@ -2165,16 +2165,18 @@ pub trait Backend: Debug + Send + Sync { // the shim for the dependency would call `mise exec` which would call the // shim again infinitely. // - // `paths_eq` handles case-insensitive matching on macOS/Windows: e.g. if - // `$HOME` is mixed-case in PATH (`/Users/Foo`) but lowercase in the - // resolved shims path, byte-equal comparison would miss it and the shim - // would survive in the child env. + // `is_mise_shims_dir` covers both the user shims dir (`dirs::SHIMS`) and + // the system shims dir (`MISE_SYSTEM_DATA_DIR/shims`). Both are typically + // on PATH in devcontainer/Docker setups built with `mise install --system`; + // filtering only one used to leak recursion through the other (#8475 + // closed the user dir for `dependency_env`, this PR closes the system dir + // and aligns with the dual-dir guard already in `which_shim` (#8816)). if let Some(path_val) = env.get(&*env::PATH_KEY) { let paths: Vec<_> = env::split_paths(path_val).collect(); let original_len = paths.len(); let filtered: Vec<_> = paths .into_iter() - .filter(|p| !file::paths_eq(&file::replace_path(p), &dirs::SHIMS)) + .filter(|p| !file::is_mise_shims_dir(p)) .collect(); if filtered.len() != original_len { let joined = env::join_paths(&filtered)?; diff --git a/src/file.rs b/src/file.rs index 3265f5f5c9..deeecda493 100644 --- a/src/file.rs +++ b/src/file.rs @@ -766,6 +766,32 @@ pub fn canonicalize_or_self(path: &Path) -> PathBuf { canonicalize_cached(path).unwrap_or_else(|| path.to_path_buf()) } +/// Returns true if `path` is one of mise's shim directories. +/// +/// Two dirs qualify: the user shims dir (`dirs::SHIMS`) and the system shims +/// dir (`$MISE_SYSTEM_DATA_DIR/shims`). Devcontainer / Docker setups built +/// with `mise install --system` put both on PATH, so subprocess-env filters +/// that strip "the shims dir" must consider both — otherwise the recursion +/// these filters were added to prevent (#8475 for `dependency_env`, #8816 +/// for `which_shim`, this for the file.rs helpers) leaks back in through the +/// remaining dir. +/// +/// Uses `paths_eq` + `replace_path` for the fast path (expands `~`, +/// case-insensitive on macOS/Windows), then falls back to `canonicalize_or_self` +/// so symlinked roots (e.g. `/usr/local/share` → `/private/usr/local/share` on +/// macOS) still match — the cached helper keeps this off the filesystem hot path. +pub fn is_mise_shims_dir(path: &Path) -> bool { + let resolved = replace_path(path); + let sys_shims = env::MISE_SYSTEM_DATA_DIR.join("shims"); + if paths_eq(&resolved, &dirs::SHIMS) || paths_eq(&resolved, &sys_shims) { + return true; + } + let canon_input = canonicalize_or_self(&resolved); + let canon_user = canonicalize_or_self(&dirs::SHIMS); + let canon_sys = canonicalize_or_self(&sys_shims); + paths_eq(&canon_input, &canon_user) || paths_eq(&canon_input, &canon_sys) +} + /// Build a PATH value with mise shims filtered out, suitable for passing to /// subprocesses via `.env("PATH", ...)`. Prevents infinite recursion when a /// subprocess (e.g. `gh auth token`, `git credential fill`) resolves to a @@ -775,10 +801,9 @@ pub fn canonicalize_or_self(path: &Path) -> PathBuf { /// shims from an arbitrary PATH string (e.g. from `PRISTINE_ENV`), use /// `strip_shims_from_path` instead. pub fn path_env_without_shims() -> std::ffi::OsString { - let shim_dir = &*dirs::SHIMS; let filtered: Vec<_> = env::PATH_NON_PRISTINE .iter() - .filter(|p| !paths_eq(&replace_path(p), shim_dir)) + .filter(|p| !is_mise_shims_dir(p)) .cloned() .collect(); std::env::join_paths(filtered) @@ -789,22 +814,20 @@ pub fn path_env_without_shims() -> std::ffi::OsString { /// subprocess receives a custom env map (e.g. `PRISTINE_ENV`) rather /// than inheriting the current process's PATH. pub fn strip_shims_from_path(path_val: &str) -> String { - let shim_dir = &*dirs::SHIMS; - let filtered = env::split_paths(path_val).filter(|p| !paths_eq(&replace_path(p), shim_dir)); + let filtered = env::split_paths(path_val).filter(|p| !is_mise_shims_dir(p)); std::env::join_paths(filtered) .unwrap_or_else(|_| std::ffi::OsString::from(path_val)) .to_string_lossy() .into_owned() } -/// returns the first executable in PATH, excluding the mise shim directory +/// returns the first executable in PATH, excluding the mise shim directories /// use this for internal tool lookups to avoid recursive shim invocations /// (shims call `mise exec`, which would re-enter the same code path) pub fn which_no_shims>(name: P) -> Option { - let shim_dir = &*dirs::SHIMS; let paths: Vec = env::PATH_NON_PRISTINE .iter() - .filter(|p| !paths_eq(&replace_path(p), shim_dir)) + .filter(|p| !is_mise_shims_dir(p)) .cloned() .collect(); _which(name, &paths)