fix(toolset): accept a Windows tool path spelled with backslashes

docs/configuration.md

@@ -418,7 +418,11 @@ Both `mise.toml` and `.tool-versions` support "scopes" which modify the behavior
 - `prefix:<PREFIX>` - use the latest version that matches the prefix. Useful for Go since `1.20`
   would only match `1.20` exactly but `prefix:1.20` will match `1.20.1` and `1.20.2` etc.
 - `path:<PATH>` - use a custom compiled version at the given path. One use-case is to re-use
-  Homebrew tools (e.g.: `path:/opt/homebrew/opt/node@20`).
+  Homebrew tools (e.g.: `path:/opt/homebrew/opt/node@20`). On Windows both separators work and
+  mise stores the forward-slash form either way, but mind the TOML quoting: a backslash is an
+  escape inside a _basic_ (double-quoted) string, so write `{ path = 'C:\tools\node' }` as a
+  literal string, or double them as `"C:\\tools\\node"`. `"C:\tools\node"` is not rejected — TOML
+  reads `\t` as a tab — so the path silently becomes something else.
 - `sub-<PARTIAL_VERSION>:<ORIG_VERSION>` - resolves `ORIG_VERSION`, subtracts the numeric components
   in `PARTIAL_VERSION` from the corresponding resolved version components, then resolves the result
   as a version prefix. For example, `sub-2:lts` resolves `lts` and subtracts 2 from its major

e2e-win/tool_path.Tests.ps1

@@ -0,0 +1,120 @@
+Describe 'tool-path' {
+    BeforeAll {
+        $script:OriginalDir = Get-Location
+        $script:TestRoot = Join-Path $TestDrive ([System.Guid]::NewGuid().ToString())
+        New-Item -ItemType Directory -Path $script:TestRoot | Out-Null
+
+        # A directory to point `path:` at. Nothing is installed into it -- `path:` means "the
+        # tool is already here", so parsing and resolving the value is the whole of it, and
+        # parsing is where this used to fail.
+        $script:ToolDir = Join-Path $script:TestRoot "tools\bin"
+        New-Item -ItemType Directory -Path $script:ToolDir | Out-Null
+
+        # `mise ls` reports the resolved path with forward slashes, so this is what the native
+        # spelling below has to come back as.
+        $script:Forward = $script:ToolDir -replace '\\', '/'
+
+        # Saved rather than just cleared afterwards: Pester runs every suite in one process, so
+        # removing an inherited value would leave the next file without it.
+        $script:OriginalTrusted = [Environment]::GetEnvironmentVariable('MISE_TRUSTED_CONFIG_PATHS', 'Process')
+        $env:MISE_TRUSTED_CONFIG_PATHS = $script:TestRoot
+        Set-Location $script:TestRoot
+    }
+
+    AfterAll {
+        Set-Location $script:OriginalDir
+        Remove-Item -Path $script:TestRoot -Recurse -Force -ErrorAction Ignore
+        if ($null -eq $script:OriginalTrusted) {
+            Remove-Item Env:MISE_TRUSTED_CONFIG_PATHS -ErrorAction Ignore
+        } else {
+            [Environment]::SetEnvironmentVariable('MISE_TRUSTED_CONFIG_PATHS', $script:OriginalTrusted, 'Process')
+        }
+    }
+
+    It 'accepts a tool path spelled the way Windows spells it' {
+        # Backslashes are what Explorer shows and what `pwd` prints, so this is the spelling a
+        # Windows user actually has. It used to be rejected outright:
+        #   invalid tool path "C:\\...": contains forbidden character '\'
+        #
+        # Written as a TOML literal string, which is what takes backslashes as-is. In a basic
+        # string they are escapes -- see the doubled form below.
+        @"
+[tools]
+node = { path = '$($script:ToolDir)' }
+"@ | Out-File (Join-Path $script:TestRoot "mise.toml")
+
+        $out = mise ls --current 2>&1 | Out-String
+        $LASTEXITCODE | Should -Be 0
+        # Asserting the resolved value, not just that the command survived: the separators are
+        # rewritten rather than allowed through, and this is where that shows.
+        $out | Should -BeLike "*path:$($script:Forward)*"
+    }
+
+    It 'accepts the same path written as a TOML basic string' {
+        # The other spelling that actually delivers backslashes to mise, and the one the docs
+        # offer as an alternative: doubled inside a double-quoted string. `"C:\tools\node"` is a
+        # third thing entirely -- TOML reads `\t` as a tab and the path silently becomes
+        # something else -- which is why the docs say which two to use.
+        $doubled = $script:ToolDir -replace '\\', '\\'
+        @"
+[tools]
+node = { path = "$doubled" }
+"@ | Out-File (Join-Path $script:TestRoot "mise.toml")
+
+        $out = mise ls --current 2>&1 | Out-String
+        $LASTEXITCODE | Should -Be 0
+        $out | Should -BeLike "*path:$($script:Forward)*"
+    }
+
+    It 'accepts the same path written with forward slashes' {
+        # The workaround people were given while the above was broken. It has to keep working.
+        @"
+[tools]
+node = { path = '$($script:Forward)' }
+"@ | Out-File (Join-Path $script:TestRoot "mise.toml")
+
+        $out = mise ls --current 2>&1 | Out-String
+        $LASTEXITCODE | Should -Be 0
+        $out | Should -BeLike "*path:$($script:Forward)*"
+    }
+
+    It 'still rejects a path containing a quote' {
+        # The control for the two above. Only the separator is rewritten; the rest of the list
+        # that `path:` values are checked against is untouched, and a quote is on it because the
+        # resolved path reaches plugin hooks that build shell commands with it.
+        @"
+[tools]
+node = { path = 'C:/a"b' }
+"@ | Out-File (Join-Path $script:TestRoot "mise.toml")
+
+        $out = mise ls --current 2>&1 | Out-String
+        $LASTEXITCODE | Should -Not -Be 0
+        $out | Should -BeLike '*invalid tool path*'
+    }
+
+    # `\\?\` and `\\.\` are the one place Windows does not accept `/`, so they are the branch that
+    # is deliberately *not* rewritten. Both assert the message rather than only the failure: it is
+    # the one that branch produces, so a change that started rewriting them would surface here as
+    # the generic "forbidden character" wording instead of as a passing test.
+    It 'still rejects an extended-length path, and says why' {
+        @"
+[tools]
+node = { path = '\\?\C:\Users\foo' }
+"@ | Out-File (Join-Path $script:TestRoot "mise.toml")
+
+        $out = mise ls --current 2>&1 | Out-String
+        $LASTEXITCODE | Should -Not -Be 0
+        $out | Should -BeLike '*extended-length and device paths*'
+    }
+
+    It 'still rejects a device path, and says why' {
+        @"
+[tools]
+node = { path = '\\.\COM1' }
+"@ | Out-File (Join-Path $script:TestRoot "mise.toml")
+
+        $out = mise ls --current 2>&1 | Out-String
+        $LASTEXITCODE | Should -Not -Be 0
+        $out | Should -BeLike '*extended-length and device paths*'
+    }
+}

e2e/config/test_tool_path_validation

@@ -0,0 +1,30 @@
+#!/usr/bin/env bash
+
+# A `path:` value is resolved into `ctx.rootPath` for path-mode tools, which plugin hooks build
+# shell commands with, so it is checked against a list of shell-quote-breaking characters (#9814).
+#
+# On Windows `\` is the path separator rather than a shell escape, so it is rewritten to `/` before
+# that check. Here it is a shell escape and a legal character in a filename, so it stays rejected.
+# This is the end-to-end guard that the rewrite did not leak off Windows.
+
+mkdir -p tools/bin
+
+cat >mise.toml <<'EOF'
+[tools]
+node = { path = '/tmp/\rm' }
+EOF
+assert_fail_contains "mise ls --current" "invalid tool path"
+
+cat >mise.toml <<'EOF'
+[tools]
+node = { path = '/tmp/$(id)' }
+EOF
+assert_fail_contains "mise ls --current" "invalid tool path"
+
+# The control: an ordinary path resolves, so the assertions above are about the character and not
+# about `path:` being broken in this sandbox.
+cat >mise.toml <<'EOF'
+[tools]
+node = { path = "./tools/bin" }
+EOF
+assert_contains "mise ls --current" "tools/bin"

src/toolset/tool_request.rs

@@ -107,8 +107,9 @@ impl ToolRequest {
                 }
             }
             Some(("path", p)) => {
-                validate_path_string(p)?;
-                let path = resolve_path(p, &source);
+                let p = windows_path_separators(p);
+                validate_path_string(&p)?;
+                let path = resolve_path(&p, &source);
                 Self::Path {
                     path,
                     options: backend.opts(),
@@ -519,6 +520,10 @@ fn validate_ref_string(s: &str) -> Result<()> {
 /// entry in a project config cannot inject shell syntax. Path traversal is
 /// intentionally not rejected here because `path:../tools/foo` is a normal
 /// relative-path use case.
+///
+/// The list is written for a POSIX shell, which is why `\` is on it. On Windows `\` is a path
+/// separator instead, so it is rewritten by [`windows_path_separators`] before it gets here rather
+/// than being allowed through — see that function.
 fn validate_path_string(s: &str) -> Result<()> {
     if s.is_empty() {
         return Ok(());
@@ -528,11 +533,49 @@ fn validate_path_string(s: &str) -> Result<()> {
         // and quote/expansion rejection, but allow `/` since paths need it.
         is_forbidden_version_char(*c)
     }) {
+        // The only `\` that survives the rewrite is an extended-length or device prefix, so say
+        // what is actually wrong instead of naming a character the user cannot avoid.
+        #[cfg(windows)]
+        if c == '\\' {
+            bail!(
+                "invalid tool path {s:?}: extended-length and device paths (\\\\?\\, \\\\.\\) are not supported"
+            );
+        }
         bail!("invalid tool path {s:?}: contains forbidden character {c:?}");
     }
     Ok(())
 }
 
+/// Rewrite `\` to `/` in a `path:` value on Windows.
+///
+/// `\` is the path separator there, not a shell metacharacter, so [`validate_path_string`] used to
+/// reject every native path — anything copied out of Explorer or printed by `pwd`. Win32 accepts
+/// `/` wherever it accepts `\`, so rewriting is what makes those usable *without* letting a `\`
+/// reach a vfox hook's `ctx.rootPath`, which is the thing the list exists to prevent (#9814). The
+/// alternative — dropping `\` from the list on Windows — would have weakened that.
+///
+/// Extended-length and device prefixes (`\\?\`, `\\.\`) are left alone. Those are the one place
+/// Windows does not accept `/`, so rewriting would hand back a path that looks right and does not
+/// resolve; they keep being rejected, as they are today.
+#[cfg(windows)]
+fn windows_path_separators(s: &str) -> std::borrow::Cow<'_, str> {
+    if s.starts_with(r"\\?\") || s.starts_with(r"\\.\") {
+        return std::borrow::Cow::Borrowed(s);
+    }
+    if s.contains('\\') {
+        std::borrow::Cow::Owned(s.replace('\\', "/"))
+    } else {
+        std::borrow::Cow::Borrowed(s)
+    }
+}
+
+/// No-op off Windows: `\` is a legal character in a unix filename, so rewriting it would change
+/// which file is meant, and there it really is a shell escape — it stays rejected.
+#[cfg(not(windows))]
+fn windows_path_separators(s: &str) -> std::borrow::Cow<'_, str> {
+    std::borrow::Cow::Borrowed(s)
+}
+
 fn is_forbidden_version_char(c: char) -> bool {
     if (c as u32) < 0x20 || c == '\x7f' {
         return true;
@@ -783,6 +826,8 @@ mod tests {
 
     #[test]
     fn test_validate_path_string() {
+        // `validate_path_string` in isolation. On Windows the `path:` arm rewrites separators
+        // first, so a `\` in a real config does not reach here — that pair is covered below.
         use super::validate_path_string;
         // valid paths
         for p in [
@@ -815,6 +860,77 @@ mod tests {
         }
     }
 
+    /// Rewrite then validate, in the order the `path:` arm does it.
+    fn accept_tool_path(p: &str) -> Result<String, eyre::Report> {
+        let p = super::windows_path_separators(p);
+        super::validate_path_string(&p)?;
+        Ok(p.into_owned())
+    }
+
+    #[test]
+    fn test_tool_path_rewrite_does_not_widen_the_rest() {
+        // The control for the two platform-specific tests below: quote and expansion characters
+        // stay rejected everywhere, and a path with no separator to rewrite comes back untouched.
+        assert_eq!(
+            accept_tool_path("/home/user/tools/foo").unwrap(),
+            "/home/user/tools/foo"
+        );
+        assert_eq!(accept_tool_path("C:/Users/foo").unwrap(), "C:/Users/foo");
+        for p in [
+            "/tmp/$HOME",
+            "/tmp/`id`",
+            "/tmp/$(id)",
+            "/tmp/'rm",
+            "/tmp/\"rm",
+            "/tmp/\nrm",
+        ] {
+            assert!(
+                accept_tool_path(p).is_err(),
+                "expected path {p:?} to be rejected"
+            );
+        }
+    }
+
+    #[cfg(windows)]
+    #[test]
+    fn test_native_windows_tool_path_is_accepted() {
+        // The reported case: a path spelled the way Windows spells it. Rewritten rather than
+        // allowed through, so no `\` reaches a plugin hook's `ctx.rootPath`.
+        for (input, expected) in [
+            (r"C:\Users\foo\bin", "C:/Users/foo/bin"),
+            (r"~\.local\bin", "~/.local/bin"),
+            (r"..\tools\foo", "../tools/foo"),
+            (r"C:\Program Files\tool", "C:/Program Files/tool"),
+        ] {
+            assert_eq!(accept_tool_path(input).unwrap(), expected, "{input:?}");
+        }
+    }
+
+    #[cfg(windows)]
+    #[test]
+    fn test_extended_length_tool_path_is_left_alone_and_rejected() {
+        // `\\?\` and `\\.\` are the one place Windows does not accept `/`, so rewriting them
+        // would produce a path that looks right and does not resolve. They stay rejected, as
+        // they are on every version before this change.
+        for input in [r"\\?\C:\Users\foo", r"\\.\COM1"] {
+            assert_eq!(
+                super::windows_path_separators(input),
+                input,
+                "{input:?} should not be rewritten"
+            );
+            assert!(accept_tool_path(input).is_err(), "{input:?}");
+        }
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn test_backslash_is_still_rejected_on_unix() {
+        // The control for the Windows tests: `\` is a legal character in a unix filename and a
+        // shell escape there, so none of the rewriting reaches this platform.
+        assert_eq!(super::windows_path_separators(r"/tmp/\rm"), r"/tmp/\rm");
+        assert!(accept_tool_path(r"/tmp/\rm").is_err());
+    }
+
     #[test]
     fn test_version_sub() {
         assert_str_eq!(version_sub("18.2.3", "2").unwrap(), "16");