Skip to content

fix(shim): prevent infinite recursion when system shims dir is on PATH#8816

Merged
jdx merged 2 commits intojdx:mainfrom
andrewthauer:andrewthauer/fix-system-shims-recursion
Mar 30, 2026
Merged

fix(shim): prevent infinite recursion when system shims dir is on PATH#8816
jdx merged 2 commits intojdx:mainfrom
andrewthauer:andrewthauer/fix-system-shims-recursion

Conversation

@andrewthauer
Copy link
Copy Markdown
Contributor

Problem

When tools are installed via mise install --system (e.g. in a Docker/devcontainer image), mise creates a second shims directory at MISE_SYSTEM_DATA_DIR/shims (e.g.
/usr/local/share/mise/shims) alongside the user shims dir (~/.local/share/mise/shims). In devcontainer environments both directories are typically on PATH.

If a tool is installed system-wide but not present in any config file (mise.toml, ~/.config/mise/config.toml, etc.), invoking the shim hangs indefinitely. MISE_YES=1 and MISE_OFFLINE=1 do
not help.

Workarounds (until this is fixed):

  • mise use -g — adds the tool to the user global config
  • Populate /etc/mise/config.toml with the system tools — shims resolve via the system config and never hit the PATH fallback

Root Cause

which_shim's PATH fallback — used when a tool is not found in the active toolset — iterated PATH looking for a real binary, skipping only dirs::SHIMS (the user shims dir). When the system
shims dir was also on PATH, its entry (a symlink to the mise binary) was found, exec'd, and re-entered the same shim logic — infinite loop.

Fix

  • src/shims.rs: In the PATH fallback, skip both the user shims dir and MISE_SYSTEM_DATA_DIR/shims. Additionally reject any binary that canonicalises to the mise binary itself, guarding
    against other shim-like wrapper arrangements.
  • src/cli/exec.rs: Filter the system shims dir from the PATH used for binary resolution in exec_program (Unix + Windows), consistent with the existing user shims dir filtering.
  • e2e/cli/test_system_shims_no_recursion: Regression test that places a fake system shims dir on PATH and asserts the shim fails fast with a clear error rather than hanging.

🤖 This fix was investigated and implemented with the assistance of Claude Code (claude-sonnet-4-6).

When tools are installed via `mise install --system` they are placed in
`MISE_SYSTEM_DATA_DIR/installs` (e.g. /usr/local/share/mise/installs) and
a second shims directory is created at `MISE_SYSTEM_DATA_DIR/shims`. In
Docker/devcontainer environments both the user shims dir and the system
shims dir are typically on PATH.

Previously, `which_shim`'s PATH fallback only skipped the user shims dir
(`dirs::SHIMS`). When a tool was not found in any config file the fallback
iterated PATH, found the system shims binary (also a symlink to the mise
binary), exec'd it, which re-entered the same shim logic — infinite loop.

Fix:
- In `which_shim`: skip both the user and system shims dirs in the PATH
  fallback, and additionally reject any binary that canonicalises to the
  mise binary itself to guard against other shim-like wrapper scenarios.
- In `exec_program` (Unix + Windows): filter the system shims dir from
  the PATH used for binary resolution, consistent with the existing user
  shims dir filtering.
- Add regression test `e2e/cli/test_system_shims_no_recursion` that
  simulates both shims dirs on PATH and asserts the shim fails fast with
  a clear error rather than hanging.
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request addresses an infinite recursion bug in mise shims by updating path filtering and shim resolution to skip all mise-managed shim directories. It includes a new regression test. Feedback suggests using a trap for cleanup in the test script and refactoring repeated path normalization logic in the CLI execution module.

# Simulate a second (system-level) shims directory on PATH.
# The binary inside it is a symlink to the mise binary, exactly like a real
# system shim created by `mise install --system`.
sys_shimdir="$(mktemp -d)"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To ensure the temporary directory is always cleaned up, even if the script exits unexpectedly, it's a good practice to use trap for cleanup. This makes the test more robust. The explicit rm -rf at the end of the script can then be removed.

sys_shimdir="$(mktemp -d)"
trap 'rm -rf "$sys_shimdir"' EXIT

assert_contains "echo '$output'" "dummy"

# Cleanup
rm -rf "$sys_shimdir"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This cleanup is now handled by the trap command, so this line can be removed.

Comment on lines +254 to +258
let sys_shims_normalized = crate::env::MISE_SYSTEM_DATA_DIR
.join("shims")
.to_string_lossy()
.to_lowercase()
.replace('/', "\\");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The path normalization logic (.to_string_lossy().to_lowercase().replace('/', "\\")) is repeated multiple times in this function (here, for shims_normalized, in is_shims, pristine, mise_added, and original).

To improve maintainability and reduce duplication, consider extracting this logic into a local helper closure or function within exec_program.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 29, 2026

Greptile Summary

This PR fixes an infinite-recursion hang that occurred when a tool was installed system-wide (mise install --system) but not present in any config file, while both the user shims dir and the system shims dir (MISE_SYSTEM_DATA_DIR/shims) were on PATH (a common devcontainer layout).

Key changes:

  • src/shims.rs: Adds two complementary guards in which_shim's PATH fallback: (1) skip any PATH entry whose canonicalized form matches user_shims or sys_shims, and (2) skip any found binary that canonicalizes to the mise executable itself — this second guard is the more robust protection and handles cases where the system shims dir appears on PATH under an arbitrary name or symlink.
  • src/cli/exec.rs: Extends the existing shims-dir exclusion in exec_program (Unix and Windows) to also strip MISE_SYSTEM_DATA_DIR/shims from the lookup_path passed to which::which_in, preventing that resolution step from picking up a shim binary. The comparison uses raw path equality (no canonicalization), consistent with the pre-existing user_shims handling in the same function.
  • e2e/cli/test_system_shims_no_recursion: New regression test that places a fake system shims dir (with dummy → mise symlink) on PATH and asserts the shim exits with a clear error rather than hanging.

Minor observations (non-blocking):

  • The exec.rs Unix path filter still uses non-canonicalized path equality for both user_shims and sys_shims. A symlink-aliased system shims dir would not be filtered; the shims.rs mise_bin canonicalize guard provides the safety net in that case, though the resulting error is slightly less clean.
  • The regression test validates the mise_bin canonicalize guard but does not exercise the directory-path-skip guard (Guard 1 in shims.rs), because the temp dir created by mktemp -d does not match MISE_SYSTEM_DATA_DIR/shims.

Confidence Score: 5/5

Safe to merge — the fix is sound, the two guards in shims.rs are mutually reinforcing, and all remaining findings are P2 style suggestions.

The core bug is definitively fixed by two layered guards in shims.rs. The exec.rs change is a consistent extension of pre-existing behaviour. The only open items are a non-canonicalized comparison in exec.rs (P2, matches pre-existing style) and a test-coverage gap for Guard 1 (P2). No P0 or P1 issues remain.

No files require special attention; the P2 observations on src/cli/exec.rs and the test are improvement suggestions, not blockers.

Important Files Changed

Filename Overview
src/shims.rs Adds two complementary guards in which_shim's PATH fallback: (1) skip any PATH entry whose canonicalized form matches either the user or system shims directory, and (2) skip any binary that canonicalizes to the mise binary itself. Together these soundly prevent infinite recursion regardless of how the system shims dir appears on PATH.
src/cli/exec.rs Extends the existing shims-dir filter in exec_program (both Unix and Windows) to also exclude MISE_SYSTEM_DATA_DIR/shims from the lookup_path passed to which::which_in. The Unix version uses non-canonicalized path equality (consistent with the pre-existing user_shims check), which could miss the system shims dir if it appears on PATH via a symlink.
e2e/cli/test_system_shims_no_recursion New regression test that uses a mktemp -d fake system shims dir containing a dummy → mise symlink; validates that invoking the shim fails fast with a clear error. The test exercises the mise_bin-canonicalize guard but not the directory-path-skip guard (since the temp dir doesn't match MISE_SYSTEM_DATA_DIR/shims).

Sequence Diagram

sequenceDiagram
    participant Shell
    participant Shim as dummy (symlink→mise)
    participant handle_shim
    participant which_shim
    participant exec_program

    Shell->>Shim: invoke dummy
    Note over Shell,Shim: argv[0]="dummy", found in sys_shimdir via PATH
    Shim->>handle_shim: mise starts (bin_name="dummy")
    handle_shim->>which_shim: which_shim(config, "dummy")

    Note over which_shim: 1. toolset lookup → not found
    Note over which_shim: 2. not_found_auto_install → skipped

    loop PATH fallback
        which_shim->>which_shim: canon(path) == user_shims? → skip (Guard 1a)
        which_shim->>which_shim: canon(path) == sys_shims? → skip (Guard 1b)
        which_shim->>which_shim: bin exists? → canon(bin) == mise_bin? → skip (Guard 2)
    end

    which_shim-->>handle_shim: Err("dummy is not a valid shim")
    handle_shim-->>Shell: exit non-zero (clear error, no recursion)

    Note over exec_program: exec_program also filters sys_shims from lookup_path
    Note over exec_program: preventing which::which_in from finding the shim
Loading

Reviews (2): Last reviewed commit: "fix(shim): address review feedback" | Re-trigger Greptile

- shims.rs: guard sys_shims with p.exists() check to avoid empty PathBuf
  false-match when system shims dir does not exist
- test: use trap for sys_shimdir cleanup instead of explicit rm -rf
@jdx jdx merged commit 698e969 into jdx:main Mar 30, 2026
32 of 34 checks passed
mise-en-dev added a commit that referenced this pull request Mar 31, 2026
### 🚀 Features

- **(python)** add GitHub provenance verification for prebuilt binaries
by @malept in [#8820](#8820)

### 🐛 Bug Fixes

- **(ci)** use rustls-native-roots for Windows CI build by @jdx in
[#8822](#8822)
- **(go)** improve version fetching logic to support deeply nested
sub-modules by @roele in [#8823](#8823)
- **(shim)** prevent infinite recursion when system shims dir is on PATH
by @andrewthauer in [#8816](#8816)
- go backend missing supports_lockfile_url() override by
@palootcenas-outreach in [#8790](#8790)
- strip shims from PATH in credential and template subprocesses by
@antonioacg in [#8802](#8802)

### 📚 Documentation

- fix typo in shims documentation for fish by @roele in
[#8798](#8798)

### 📦️ Dependency Updates

- update ghcr.io/jdx/mise:alpine docker digest to 3e6d001 by
@renovate[bot] in [#8794](#8794)
- pin dependencies by @renovate[bot] in
[#8793](#8793)

### 📦 Registry

- fix flutter version sorting by @roele in
[#8818](#8818)
- add svgo (npm:svgo) by @3w36zj6 in
[#8817](#8817)

### New Contributors

- @antonioacg made their first contribution in
[#8802](#8802)
- @palootcenas-outreach made their first contribution in
[#8790](#8790)

## 📦 Aqua Registry Updates

#### New Packages (3)

- [`RasKrebs/sonar`](https://github.com/RasKrebs/sonar)
- [`emacs-eask/cli`](https://github.com/emacs-eask/cli)
-
[`superradcompany/microsandbox`](https://github.com/superradcompany/microsandbox)

#### Updated Packages (4)

- [`dimo414/bkt`](https://github.com/dimo414/bkt)
- [`lxc/incus`](https://github.com/lxc/incus)
-
[`shinagawa-web/gomarklint`](https://github.com/shinagawa-web/gomarklint)
- [`updatecli/updatecli`](https://github.com/updatecli/updatecli)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants