Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions e2e/cli/test_doctor_shim_shadowing
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#!/usr/bin/env bash

mise use dummy@latest

shim_dir="$MISE_DATA_DIR/shims"
shadow_dir="$PWD/shadow-bin"
mkdir -p "$shadow_dir"
printf '#!/bin/sh\nexit 0\n' >"$shadow_dir/dummy"
chmod +x "$shadow_dir/dummy"

# A command before the shim directory is an intentional PATH override, but doctor
# should identify the concrete executable that shadows the mise-managed command.
output="$(PATH="$shadow_dir:$shim_dir:$PATH" mise doctor 2>&1 || true)"
assert_contains_text "$output" "mise shims are shadowed by executables earlier in PATH"
assert_contains_text "$output" "dummy:"
assert_contains_text "$output" "shadow-bin/dummy"

# No warning is needed when the mise shim wins command resolution.
output="$(PATH="$shim_dir:$shadow_dir:$PATH" mise doctor 2>&1 || true)"
assert_not_contains_text "$output" "shadow-bin/dummy"
85 changes: 77 additions & 8 deletions src/cli/doctor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,12 @@ impl Doctor {

let config = Config::get().await?;
let ts = config.get_toolset().await?;
self.analyze_shims(&config, ts).await;
let desired_shims = self.analyze_shims(&config, ts).await;
self.analyze_plugins();
self.analyze_backend_mismatches();
self.analyze_system_deps(ts).await;
self.check_path_ordering(ts, &config).await;
self.check_shim_shadowing(&desired_shims).await;
data.insert(
"paths".into(),
self.paths(ts)
Expand Down Expand Up @@ -451,10 +452,11 @@ impl Doctor {

match ToolsetBuilder::new().build(config).await {
Ok(ts) => {
self.analyze_shims(config, &ts).await;
let desired_shims = self.analyze_shims(config, &ts).await;
self.analyze_toolset(&ts).await?;
self.analyze_paths(&ts).await?;
self.check_path_ordering(&ts, config).await;
self.check_shim_shadowing(&desired_shims).await;
}
Err(err) => self.errors.push(format!("failed to load toolset: {err}")),
}
Expand Down Expand Up @@ -786,29 +788,32 @@ impl Doctor {
Ok(())
}

async fn analyze_shims(&mut self, config: &Arc<Config>, toolset: &Toolset) {
async fn analyze_shims(&mut self, config: &Arc<Config>, toolset: &Toolset) -> HashSet<String> {
let mise_bin = file::which_no_shims("mise").unwrap_or(env::MISE_BIN.clone());

if let Ok((missing, extra)) = shims::get_shim_diffs(config, mise_bin, toolset).await {
if let Ok(diffs) = shims::get_shim_diffs(config, mise_bin, toolset).await {
let cmd = style::nyellow("mise reshim");

if !missing.is_empty() {
if !diffs.missing.is_empty() {
self.errors.push(formatdoc!(
"shims are missing, run {cmd} to create them
Missing shims: {missing}",
missing = missing.into_iter().join(", ")
missing = diffs.missing.into_iter().join(", ")
));
}

if !extra.is_empty() {
if !diffs.extra.is_empty() {
self.errors.push(formatdoc!(
"unused shims are present, run {cmd} to remove them
Unused shims: {extra}",
extra = extra.into_iter().join(", ")
extra = diffs.extra.into_iter().join(", ")
));
}
time!("doctor::analyze_shims");
return diffs.desired;
}
time!("doctor::analyze_shims");
HashSet::new()
}

fn analyze_plugins(&mut self) {
Expand Down Expand Up @@ -961,6 +966,70 @@ impl Doctor {
Ensure `mise activate` runs after other PATH modifications in your shell rc file."#
));
}

/// Check whether commands provided by mise shims resolve to another executable first.
/// Paths before the shim directory are allowed to take precedence intentionally, so report
/// only concrete command collisions instead of warning merely because shims are not first.
async fn check_shim_shadowing(&mut self, desired_shims: &HashSet<String>) {
if env::is_activated() || !shims_on_path() {
return;
}

let path = match std::env::join_paths(&*env::PATH) {
Ok(path) => path,
Err(_) => return,
};
let cwd = dirs::CWD.clone().unwrap_or_default();
let mut shadowed = BTreeMap::new();
let mut checked_commands = HashSet::new();

for shim in desired_shims {
if !dirs::SHIMS.join(shim).exists() {
continue;
}
let command = shim_command_name(shim);
if !checked_commands.insert(command.clone()) {
continue;
}
Comment thread
greptile-apps[bot] marked this conversation as resolved.
Resolved
let Ok(resolved) = which::which_in(&command, Some(&path), &cwd) else {
continue;
};
let is_mise_shim = resolved
.parent()
.is_some_and(|parent| file::paths_eq(&file::replace_path(parent), &dirs::SHIMS));
if !is_mise_shim {
shadowed.insert(command, resolved);
}
}

if shadowed.is_empty() {
return;
}

let shadowed = shadowed
.into_iter()
.map(|(command, path)| format!("{command}: {}", display_path(path)))
.join("\n ");
self.warnings.push(formatdoc!(
r#"mise shims are shadowed by executables earlier in PATH:
{shadowed}
Move {} earlier in PATH to use the mise-managed versions."#,
display_path(*dirs::SHIMS),
));
}
}

fn shim_command_name(shim: &str) -> String {
if cfg!(windows) {
let lower = shim.to_ascii_lowercase();
if lower.ends_with(".exe") || lower.ends_with(".cmd") {
shim[..shim.len() - 4].to_string()
} else {
shim.to_string()
}
} else {
shim.to_string()
}
}
Comment thread
risu729 marked this conversation as resolved.
Resolved

fn shims_on_path() -> bool {
Expand Down
26 changes: 17 additions & 9 deletions src/shims.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,8 @@ pub async fn reshim(config: &Arc<Config>, ts: &Toolset, force: bool) -> Result<(
BTreeSet::new(),
)
} else {
get_shim_diffs(config, &mise_bin, ts).await?
let diffs = get_shim_diffs(config, &mise_bin, ts).await?;
(diffs.missing, diffs.extra)
};

for shim in shims_to_add {
Expand Down Expand Up @@ -433,26 +434,33 @@ fn add_shim(mise_bin: &Path, symlink_path: &Path, _shim: &str) -> Result<()> {
Ok(())
}

pub struct ShimDiffs {
pub missing: BTreeSet<String>,
pub extra: BTreeSet<String>,
pub desired: HashSet<String>,
}

// get_shim_diffs contrasts the actual shims on disk
// with the desired shims specified by the Toolset
// and returns a tuple of (missing shims, extra shims)
pub async fn get_shim_diffs(
config: &Arc<Config>,
mise_bin: impl AsRef<Path>,
toolset: &Toolset,
) -> Result<(BTreeSet<String>, BTreeSet<String>)> {
) -> Result<ShimDiffs> {
let mise_bin = mise_bin.as_ref();
let (actual_shims, desired_shims) = tokio::join!(
get_actual_shims(mise_bin),
get_desired_shims(config, mise_bin, toolset)
);
let (actual_shims, desired_shims) = (actual_shims?, desired_shims?);
let out: (BTreeSet<String>, BTreeSet<String>) = (
desired_shims.difference(&actual_shims).cloned().collect(),
actual_shims.difference(&desired_shims).cloned().collect(),
);
time!("get_shim_diffs sizes: ({},{})", out.0.len(), out.1.len());
Ok(out)
let missing: BTreeSet<_> = desired_shims.difference(&actual_shims).cloned().collect();
let extra: BTreeSet<_> = actual_shims.difference(&desired_shims).cloned().collect();
time!("get_shim_diffs sizes: ({},{})", missing.len(), extra.len());
Ok(ShimDiffs {
missing,
extra,
desired: desired_shims,
})
}

async fn get_actual_shims(mise_bin: impl AsRef<Path>) -> Result<HashSet<String>> {
Expand Down