fix(shims): compare PATH entries case-insensitively on macOS#9468
Conversation
On macOS, `$HOME` can appear in the user's PATH with different casing than the value resolved by `dirs::HOME` (e.g. `/Users/Foo` vs `/Users/foo`). Volumes are case-insensitive by default, so both refer to the same directory, but mise compared PATH entries against the shims directory byte-equal. When the npm backend ran `npm view ... --json` to resolve a package version, it built a child env with the shims directory stripped from PATH so npm would resolve to the real binary. With the case mismatch, the strip was a no-op, so npm re-resolved to the mise shim -> recursive re-entry into mise, eventually crashing the user's session. Introduce `file::paths_eq`, a platform-aware path comparator (case-insensitive on macOS/Windows, byte-equal on Linux, with separator normalisation on Windows), and route every "is this PATH entry the shims directory?" check through it: the dependency-env strip in `Backend::dependency_env`, the standalone helpers in `file.rs`, `PathEnv` partitioning, the doctor's `shims_on_path` report (which incorrectly showed `no` for the affected user), and both branches of program resolution in `cli::exec`. Reported in #9462. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Greptile SummaryFixes a real infinite shim recursion on macOS by introducing Confidence Score: 5/5Safe to merge — the change is well-scoped, consistently applied, and well-tested. No P0 or P1 issues found. The No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["PATH entry p"] --> B["replace_path(p)\nexpands ~ → $HOME"]
B --> C["paths_eq(expanded, dirs::SHIMS)"]
C --> D{Platform?}
D -->|macOS / Windows| E["components().map(lowercase).eq(...)"]
D -->|Linux| F["a == b\n(Path PartialEq, component-based,\ncase-sensitive)"]
E --> G{Match?}
F --> G
G -->|true| H["Strip entry from PATH\n(prevent shim recursion)"]
G -->|false| I["Keep entry in PATH"]
Reviews (2): Last reviewed commit: "fix(shims): compare path components inst..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a paths_eq utility to handle platform-specific path comparisons, specifically addressing case-insensitivity on macOS and Windows to prevent infinite recursion when identifying shim directories. The review feedback identifies a logic issue where the current implementation fails to account for trailing slashes or redundant separators, recommending a component-based comparison. Furthermore, a performance concern was raised regarding the repeated normalization of paths within loops, suggesting that pre-normalizing target paths would be more efficient.
| pub fn paths_eq(a: &Path, b: &Path) -> bool { | ||
| #[cfg(windows)] | ||
| { | ||
| let normalize = |p: &Path| p.to_string_lossy().to_lowercase().replace('/', "\\"); | ||
| normalize(a) == normalize(b) | ||
| } | ||
| #[cfg(target_os = "macos")] | ||
| { | ||
| a.as_os_str().to_string_lossy().to_lowercase() | ||
| == b.as_os_str().to_string_lossy().to_lowercase() | ||
| } | ||
| #[cfg(all(not(windows), not(target_os = "macos")))] | ||
| { | ||
| a == b | ||
| } | ||
| } |
There was a problem hiding this comment.
The current implementation of paths_eq on Windows and macOS uses string comparison on the full path, which fails to recognize equivalence when paths differ by trailing slashes or redundant separators (e.g., /Users/foo vs /Users/foo/). While standard Path equality in Rust handles these, it is strictly case-sensitive.
To correctly handle both case-insensitivity and logical path normalization (separators, trailing slashes, etc.) without hitting the disk, it is better to compare path components individually. This approach also removes the redundant as_os_str() call on macOS.
| pub fn paths_eq(a: &Path, b: &Path) -> bool { | |
| #[cfg(windows)] | |
| { | |
| let normalize = |p: &Path| p.to_string_lossy().to_lowercase().replace('/', "\\"); | |
| normalize(a) == normalize(b) | |
| } | |
| #[cfg(target_os = "macos")] | |
| { | |
| a.as_os_str().to_string_lossy().to_lowercase() | |
| == b.as_os_str().to_string_lossy().to_lowercase() | |
| } | |
| #[cfg(all(not(windows), not(target_os = "macos")))] | |
| { | |
| a == b | |
| } | |
| } | |
| pub fn paths_eq(a: &Path, b: &Path) -> bool { | |
| #[cfg(windows)] | |
| { | |
| let normalize = |c: std::path::Component| c.as_os_str().to_string_lossy().to_lowercase().replace('/', "\\"); | |
| a.components().map(normalize).eq(b.components().map(normalize)) | |
| } | |
| #[cfg(target_os = "macos")] | |
| { | |
| let normalize = |c: std::path::Component| c.as_os_str().to_string_lossy().to_lowercase(); | |
| a.components().map(normalize).eq(b.components().map(normalize)) | |
| } | |
| #[cfg(all(not(windows), not(target_os = "macos")))] | |
| { | |
| a == b | |
| } | |
| } |
| let filtered: Vec<_> = paths | ||
| .into_iter() | ||
| .filter(|p| p.as_path() != *dirs::SHIMS) | ||
| .filter(|p| !file::paths_eq(&file::replace_path(p), &dirs::SHIMS)) | ||
| .collect(); |
There was a problem hiding this comment.
This change introduces a performance regression by moving path normalization inside the filter loop. Previously, the shims directory path was normalized once before the loop. Now, paths_eq (and its internal string allocations/case folding) is called for every entry in the PATH.
While PATH is usually small, this is a hot path during environment setup. Consider pre-normalizing the target path or using a more efficient comparison if this becomes a bottleneck.
…athEnv Address review feedback on #9468: - Switch `paths_eq` to component-based comparison on case-insensitive platforms. This folds trailing slashes (`/foo/shims` vs `/foo/shims/`), redundant separators (`/foo//shims`), and on Windows `/` vs `\` for free, since `Path::components` already treats both as separators. - Expand `~` via `replace_path` in the `PathEnv::from_iter` shim check so it matches every other call site. Defends against any future caller that passes a tilde-prefixed entry. - Add a test for trailing-slash equivalence. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
|
Addressed in a341252:
This comment was generated by an AI coding assistant. |
Hyperfine Performance
|
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.4.25 x -- echo |
24.0 ± 0.4 | 23.3 | 25.8 | 1.00 |
mise x -- echo |
24.7 ± 0.5 | 23.8 | 29.8 | 1.03 ± 0.03 |
mise env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.4.25 env |
23.8 ± 1.0 | 22.8 | 35.4 | 1.00 |
mise env |
24.2 ± 0.5 | 23.3 | 26.7 | 1.02 ± 0.05 |
mise hook-env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.4.25 hook-env |
24.4 ± 0.4 | 23.6 | 26.2 | 1.00 |
mise hook-env |
25.4 ± 0.6 | 24.4 | 29.1 | 1.04 ± 0.03 |
mise ls
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.4.25 ls |
24.8 ± 0.6 | 23.9 | 33.5 | 1.00 |
mise ls |
25.4 ± 0.4 | 24.4 | 28.4 | 1.02 ± 0.03 |
xtasks/test/perf
| Command | mise-2026.4.25 | mise | Variance |
|---|---|---|---|
| install (cached) | 162ms | 167ms | -2% |
| ls (cached) | 83ms | 85ms | -2% |
| bin-paths (cached) | 85ms | 86ms | -1% |
| task-ls (cached) | 813ms | 804ms | +1% |
1 participant
Summary
$HOMEappears in PATH with different casing than the resolveddirs::SHIMSpath.file::paths_eq, a platform-aware path comparator (case-insensitive on macOS/Windows, byte-equal on Linux, with separator normalisation on Windows), and route every "is this PATH entry the shims directory?" check through it.Background
A user on macOS hit a recursive
mise -> npm shim -> mise -> npm shim -> ...loop whilemise up --bumpresolved npm package versions, sometimes crashing their session. Their PATH contained/Users/Olfway/.local/share/mise/shimsbutdirs::SHIMSresolved to/Users/olfway/.local/share/mise/shimsvia a lowercase$HOME. macOS volumes (HFS+/APFS) are case-insensitive by default, so both paths refer to the same directory — but mise compared PATH entries against the shims directory byte-equal.The recursion site is
Backend::dependency_env. When the npm backend rannpm view <pkg> dist-tags --jsonto resolve a "latest" version, it built a child env with the shims directory stripped from PATH sonpmwould resolve to the real binary. With the case mismatch the strip was a no-op, sonpmre-resolved to the mise shim, which exec'dmise … -- npm view …, which spawned anothernpm, ad infinitum.The same case-sensitivity bug existed at every other "compare against shims dir" call site, including the doctor's
shims_on_pathcheck (which incorrectly reportednofor the affected user, masking the real problem).Changes
file::paths_eq(a, b) -> boolwith#[cfg]-gated arms:/->\normalisation (replaces a few inline copies of this logic).paths_eq:Backend::dependency_env(the actual recursion-causing site)file::path_env_without_shims,file::strip_shims_from_path,file::which_no_shimsPathEnv::from_itershim partitioningdoctor::shims_on_pathcli::exec::exec_programprogram resolution\//normalisation (Windows).The Windows-specific normalisation block previously inlined in
Backend::dependency_envandcli::execis now collapsed into a single cross-platform call.Test plan
cargo build --bin misecleancargo test --bin mise file::— 94 tests pass, including newpaths_eqtestsmise run lint-fixclean$HOMEthatmise up --bumpagainst npm packages no longer recurses (covered by the user's reproduction in the linked discussion)🤖 Generated with Claude Code
Note
Medium Risk
Touches multiple call sites that manipulate PATH and executable resolution, so subtle path-comparison behavior changes could affect tool lookup order across platforms despite the changes being narrowly scoped and well-tested.
Overview
Prevents infinite shim recursion on macOS (and hardens Windows handling) by introducing
file::paths_eqand switching all “is this PATH entry the shims dir?” checks to use case-insensitive, component-based path comparison (including~/expansion viareplace_path).This updates shim stripping and PATH reordering in
Backend::dependency_env,cli execprogram resolution (Unix+Windows),PathEnvshim partitioning, anddoctor’sshims_on_path, and adds focused unit tests covering exact match, macOS/Windows case-insensitivity, separator/trailing-slash normalization, and Linux case-sensitivity.Reviewed by Cursor Bugbot for commit a341252. Bugbot is set up for automated code reviews on this repo. Configure here.