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
16 changes: 16 additions & 0 deletions e2e/cli/test_upgrade
Original file line number Diff line number Diff line change
Expand Up @@ -191,4 +191,20 @@ assert_contains "cat mise.toml" '"3"'
assert_contains "cat mise.lock" "3.0.1"
assert_not_contains "cat mise.lock" "2.0.0"
rm -f mise.toml mise.lock

# Test: upgrading a global fuzzy version should update the global lockfile
cat <<'EOF' >"$MISE_CONFIG_DIR/config.toml"
[tools]
dummy = "latest"
EOF
mise uninstall dummy --all
mise install dummy@1.0.0
touch "$MISE_CONFIG_DIR/mise.lock"
mise lock --global --platform linux-x64
assert_contains "cat $MISE_CONFIG_DIR/mise.lock" "1.0.0"
mise upgrade dummy@2.0.0
assert_contains "cat $MISE_CONFIG_DIR/config.toml" '"latest"'
assert_contains "cat $MISE_CONFIG_DIR/mise.lock" "2.0.0"
assert_not_contains "cat $MISE_CONFIG_DIR/mise.lock" "1.0.0"
Comment thread
greptile-apps[bot] marked this conversation as resolved.
rm -f "$MISE_CONFIG_DIR/config.toml" "$MISE_CONFIG_DIR/mise.lock"
unset MISE_LOCKFILE
46 changes: 3 additions & 43 deletions mise.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

118 changes: 83 additions & 35 deletions src/lockfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -921,9 +921,6 @@ pub fn update_lockfiles(config: &Config, ts: &Toolset, new_versions: &[ToolVersi
if !cf.source().is_mise_toml() {
continue;
}
if crate::config::is_global_config(config_path) {
continue;
}
let (lockfile_path, _is_local) = lockfile_path_for_config(config_path);
lockfile_configs
.entry(lockfile_path)
Expand Down Expand Up @@ -951,21 +948,75 @@ pub fn update_lockfiles(config: &Config, ts: &Toolset, new_versions: &[ToolVersi
let mut existing_lockfile = Lockfile::read(&lockfile_path)
.unwrap_or_else(|err| handle_lockfile_read_error(err, &lockfile_path));

// Collect all tools from all contributing configs
let mut tools_by_short: HashMap<String, Vec<LockfileTool>> = HashMap::new();
// Collect all tools from all contributing configs. This starts from the
// resolved toolset, then overlays newly installed versions because a
// fuzzy request may still resolve through the old lockfile entry until
// this update is written.
let mut tool_versions_by_short: HashMap<String, Vec<ToolVersion>> = HashMap::new();

for config_path in &configs {
let tool_source = ToolSource::MiseToml(config_path.clone());
if let Some(tools) = tools_by_source.get(&tool_source) {
for (short, tvl) in tools {
let lockfile_tools: Vec<LockfileTool> = tvl.clone().into();
for tool in lockfile_tools {
tools_by_short.entry(short.clone()).or_default().push(tool);
}
tool_versions_by_short
.entry(short.clone())
.or_default()
.extend(tvl.versions.clone());
}
}
}

for new_version in new_versions {
if let Some(source_path) = new_version.request.source().path() {
if !configs.iter().any(|config| config == source_path) {
continue;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Spurious empty entries added to unrelated lockfiles

Medium Severity

entry().or_default() at line 970–972 creates a new empty vec in tool_versions_by_short for a tool from new_versions before checking whether the tool's source config belongs to this lockfile. When the source path check at line 974 fails and continue is hit, the empty entry persists. It then flows through merge_tool_entries (which returns an empty vec for empty input), and existing_lockfile.tools.insert(short, vec![]) writes it into the lockfile — either adding a spurious empty tool = [] entry or, worse, deleting valid existing lock entries for that tool.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 9057309. Configure here.

}

let versions = tool_versions_by_short
.entry(new_version.short().to_string())
.or_default();
versions.retain(|tv| {
tv.ba() != new_version.ba()
|| tv.request.version() != new_version.request.version()
|| tv.request.source() != new_version.request.source()
});
Comment on lines +978 to +982

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 retain logic here might fail to replace stale lockfile entries if the new_version request version differs from the existing one. For example, if a configuration has dummy = "latest" and the user runs mise upgrade dummy@2.0.0, the new request version will be "2.0.0", which won't match "latest". This could result in both the old and new versions being preserved in the lockfile. Consider if matching by backend and source path alone (when available and unambiguous) would be more effective for replacing fuzzy resolutions with concrete ones.

versions.push(new_version.clone());
} else if let Some(versions) = tool_versions_by_short.get_mut(new_version.short()) {
if let Some((idx, request)) = versions
.iter()
.enumerate()
.exactly_one()
.ok()
.map(|(idx, tv)| (idx, tv.request.clone()))
{
let mut new_version = new_version.clone();
new_version.request = request;
versions.remove(idx);
versions.push(new_version);
} else {
trace!(
"skipping lockfile update for {}@{} in {}: ambiguous config entries",
new_version.short(),
new_version.version,
display_path(&lockfile_path)
);
}
}
}

let tools_by_short: HashMap<String, Vec<LockfileTool>> = tool_versions_by_short
.into_iter()
.map(|(short, versions)| {
(
short,
versions
.iter()
.map(lockfile_tool_from_tool_version)
.collect(),
)
})
.collect();

Comment thread
cursor[bot] marked this conversation as resolved.
// Check for provenance regression before merging (which drops old version entries).
// For github backend tools, error if the highest prior version had provenance but
// the new version does not — this could indicate a supply chain attack.
Expand Down Expand Up @@ -1165,9 +1216,6 @@ pub async fn auto_lock_new_versions(_config: &Config, new_versions: &[ToolVersio
continue;
}
if let Some(source_path) = tv.request.source().path() {
if crate::config::is_global_config(source_path) {
continue;
}
let (lockfile_path, _) = lockfile_path_for_config(source_path);
versions_by_lockfile
.entry(lockfile_path)
Expand Down Expand Up @@ -1755,34 +1803,34 @@ impl LockfileTool {

impl From<ToolVersionList> for Vec<LockfileTool> {
fn from(tvl: ToolVersionList) -> Self {
use crate::backend::platform_target::PlatformTarget;

tvl.versions
.iter()
.map(|tv| {
let mut platforms = BTreeMap::new();
.map(lockfile_tool_from_tool_version)
.collect()
}
}

// Convert tool version lock_platforms to lockfile platforms
for (platform, platform_info) in &tv.lock_platforms {
platforms.insert(platform.clone(), platform_info.clone());
}
fn lockfile_tool_from_tool_version(tv: &ToolVersion) -> LockfileTool {
let mut platforms = BTreeMap::new();

// Resolve lockfile options from the backend
let options = if let Ok(backend) = tv.request.backend() {
let target = PlatformTarget::from_current();
backend.resolve_lockfile_options(&tv.request, &target)
} else {
BTreeMap::new()
};
// Convert tool version lock_platforms to lockfile platforms
for (platform, platform_info) in &tv.lock_platforms {
platforms.insert(platform.clone(), platform_info.clone());
}

LockfileTool {
version: tv.version.clone(),
backend: Some(tv.ba().stored_full()),
options,
platforms,
}
})
.collect()
// Resolve lockfile options from the backend
let options = if let Ok(backend) = tv.request.backend() {
let target = PlatformTarget::from_current();
backend.resolve_lockfile_options(&tv.request, &target)
} else {
BTreeMap::new()
};

LockfileTool {
version: tv.version.clone(),
backend: Some(tv.ba().stored_full()),
options,
platforms,
}
}

Expand Down
Loading