Skip to content

fix(semver): stop calling a mangled value a semver range - #11949

Merged
jdx merged 1 commit into
jdx:mainfrom
JamBalaya56562:fix/semver-range-control-chars
Aug 13, 2026
Merged

jdx merged 1 commit into
jdx:mainfrom
JamBalaya56562:fix/semver-range-control-chars

Conversation

@JamBalaya56562

@JamBalaya56562 JamBalaya56562 commented Aug 13, 2026 •

Copy link
Copy Markdown
Contributor

A tool version that was mangled before it reached mise gets reported as an unsupported semver
range, which sends the reader looking at versions when the value itself is what is wrong.

The problem

Measured on 2026.8.2 windows-x64, with node = { path = "C:\tools\node" } in mise.toml:

$ mise ls
mise WARN  semver range "path:C:<TAB>ools<LF>ode" is not supported for node (source: …/mise.toml);
           use a concrete version or version prefix
mise ERROR invalid version for node in …/mise.toml
mise ERROR invalid tool path "C:\tools\node": contains forbidden character '\t'

The error is already right. The warning above it is not, and it is what a reader sees first —
so the advice they act on is "use a concrete version", which has nothing to do with the problem.

TOML reads \t as a tab and \n as a newline inside a basic (double-quoted) string, so the
value arrives here with control characters embedded. That is easy to write by accident on Windows;
#11937 documents the quoting side of it.

Why it is detected as a range

is_npm_semver_range_query ends with:

if query.split_whitespace().count() > 1 {
    return true;
}

split_whitespace counts tabs and newlines as separators, so path:C:<TAB>ools<LF>ode looks like
a multi-part range. Confirmed with a standalone rustc probe of the predicate:

"path:C:\tools\node"     is_range=true   has_control=true    <- the false positive
"path:C:/tools/node"     is_range=false  has_control=false
"^1.2.3"                 is_range=true   has_control=false
">=1.0 <2.0"             is_range=true   has_control=false
"1.x"                    is_range=true   has_control=false
"*"                      is_range=true   has_control=false

A real npm range never contains a control character. That is what makes the two cases
separable rather than a judgement call.

The change

One early return in the predicate:

if query.chars().any(|c| c.is_control()) {
    return false;
}

Fixed in is_npm_semver_range_query rather than at the call site in tool_request.rs, because
the predicate answers "is this npm range syntax?" and returning true for a string with control
characters in it is wrong by that contract — any other caller would inherit the same mistake.

What does not change

  • Range detection for ^1.2.3, >=1.0 <2.0, 1.x, *, 1.2 - 1.5, 1.0 || 2.0 — none contain
    a control character.
  • The value is still rejected. validate_version_string and validate_path_string both reject
    characters below 0x20, so suppressing the warning does not let anything through; it just stops
    the misleading line from preceding the accurate one.
  • The invalid tool path …: contains forbidden character '\t' error is untouched.

Tests

  • unit — added to src/semver.rs's existing test module: tab/newline values are not ranges
    (including the path:C:\tools\node case), with a control asserting the six range forms above
    are still detected and that ordinary values — 1.0.0, latest, and a well-formed
    path:C:/tools/node — stay non-ranges. Without that control this could pass by disabling range
    detection entirely.
  • e2e — e2e/config/test_semver_range_warning already covered the warning firing for real
    ranges; it now also covers a mangled value producing invalid tool path and no semver range line. mise ls exits non-zero for that config, so those two assertions use the failure
    helpers rather than assert_contains, which treats a non-zero status as the test failing.

Not run

cargo fmt --all and a standalone rustc probe of the predicate covering every assertion the new
unit tests make (0 failures); no full local build. The transcript above is from a released binary
reproducing the bug. CI is what exercises the fix.

Found while working on #11937, and split out of it as a separate concern.

Summary by CodeRabbit

  • Bug Fixes
    • Improved validation of semantic version range queries.
    • Windows-style paths with escaped whitespace are no longer incorrectly interpreted as version ranges.
    • Values containing control characters are correctly rejected as semantic version ranges.
    • Valid semantic version range formats continue to be recognized.

A tool version broken before it reached mise was reported as an unsupported
semver range, which points the reader at versions when the value itself is
what is wrong:

    mise WARN  semver range "path:C:<TAB>ools<LF>ode" is not supported for
               node (source: .../mise.toml); use a concrete version or
               version prefix
    mise ERROR invalid tool path "C:\tools\node": contains forbidden
               character '\t'

The error underneath already names the real cause. The warning above it does
not, and it comes first.

TOML reads `\t` as a tab and `\n` as a newline inside a basic string, so
`path = "C:\tools\node"` arrives here with control characters in it.
`is_npm_semver_range_query` then hits `split_whitespace().count() > 1`, which
counts those as separators, and calls the value a multi-part range.

A real npm range never contains a control character, so skipping such values
in the predicate separates the two cases cleanly. Range detection is
unchanged for `^1.2.3`, `>=1.0 <2.0`, `1.x`, `*`, `1.2 - 1.5` and
`1.0 || 2.0`, and a value with a control character is still rejected by the
validation that follows.
@coderabbitai

coderabbitai Bot commented Aug 13, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 5799744b-9388-446d-8abe-bebf754ce392

📥 Commits

Reviewing files that changed from the base of the PR and between 110233a and 2ceaf82.

📒 Files selected for processing (2)
  • e2e/config/test_semver_range_warning
  • src/semver.rs

📝 Walkthrough

Walkthrough

The semver query classifier now rejects control characters before range detection. Unit tests cover malformed and valid queries. An end-to-end test verifies escaped Windows-style paths fail as invalid paths without semver-range warnings.

Changes

Semver detection hardening

Layer / File(s) Summary
Reject control characters and preserve valid ranges
src/semver.rs
is_npm_semver_range_query rejects control-character and mangled path values. Tests confirm valid semver ranges remain recognized.
Validate escaped Windows-style paths
e2e/config/test_semver_range_warning
The end-to-end test checks the invalid tool path error and confirms that no semver-range warning is emitted.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Mergeability Score: ⚪ Minimal · up to 2ceaf

The change only prevents malformed values containing control characters from being mislabeled as semver ranges while preserving validation errors and real range detection; no actionable merge-blocking risk remains after normal checks and review.

Possibly related PRs

Suggested reviewers: risu729

Poem

A rabbit checks each path with care,
No hidden tabs or newlines there.
Valid ranges still pass the gate,
Bad paths now report their state.
Hop, hop—warnings stay away!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the semver range detection fix implemented by the pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@JamBalaya56562
JamBalaya56562 marked this pull request as ready for review August 13, 2026 17:39
@greptile-apps

greptile-apps Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR prevents malformed tool values containing control characters from being mislabeled as unsupported semantic-version ranges.

  • Adds an early control-character rejection to npm semantic-version range detection.
  • Adds unit coverage for mangled values, valid ranges, and ordinary non-range values.
  • Adds end-to-end coverage confirming that malformed Windows paths retain the accurate path error without emitting the misleading range warning.

Confidence Score: 5/5

The PR appears safe to merge, with the malformed values still rejected by existing validation while the misleading warning is suppressed.

The new predicate guard only changes diagnostic range classification for control-character-containing values, and the added unit and end-to-end coverage preserves genuine range detection and confirms that invalid paths still fail.

Important Files Changed

Filename Overview
src/semver.rs Narrows range classification for control-character-containing inputs and adds focused positive and negative regression tests; no actionable defect found.
e2e/config/test_semver_range_warning Verifies that a TOML-mangled Windows path fails with the path diagnostic and does not emit a semantic-version range warning.

Reviews (1): Last reviewed commit: "fix(semver): stop calling a mangled valu..." | Re-trigger Greptile

@jdx
jdx merged commit 5c2b280 into jdx:main Aug 13, 2026
30 checks passed
@JamBalaya56562
JamBalaya56562 deleted the fix/semver-range-control-chars branch August 13, 2026 18:10
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