mirror of
https://github.com/coder/coder.git
synced 2026-06-02 20:48:20 +00:00
Mathias Fredriksson
ed4311b2cb
## Summary Fixes all 9 Windows CI test failures caused by the mise CI refactor (`fe257666d7`, PR #25727). ### Root cause `jdx/mise-action` exports `Path` (Windows convention) via `GITHUB_ENV`. Bash on Windows maintains its own `PATH`. When Go's `os.Environ()` returns both, `cmd.exe` subprocesses non-deterministically pick the MSYS-translated `PATH` (forward slashes), causing Windows executables (`printf`, `powershell.exe`, `cmd.exe`) to be unresolvable. These failures only appeared on `main` (where `-count=1` forces real test execution) and were masked on PRs by Go test cache. ### Fixes applied **CI (`setup-mise` action)**: - Write both `Path` and `PATH` to `GITHUB_ENV` with Git usr/bin prepended **Code (`cli/root.go`)**: - Add `appendAndDedupEnv` helper that deduplicates case-insensitive env vars on Windows, preferring native Windows paths (backslashes) over MSYS paths **Code (`cli/configssh_windows.go`)**: - Use absolute paths for `powershell.exe` and `cmd.exe` in the SSH config `Match exec` escape function, avoiding PATH resolution entirely **Tests**: - Switch `--header-command` tests from `printf` to `echo` (cmd.exe builtin) for reliable cross-platform execution - Add env dedup in `Test_sshConfigMatchExecEscape` for subprocess PATH consistency Fixes coder/internal#1556, coder/internal#1558, coder/internal#1559 > 🤖 Generated by Coder agent, will be reviewed by @mafredri. 🏂🏻
65 lines
3.1 KiB
Go
65 lines
3.1 KiB
Go
//go:build windows
|
|
|
|
package cli
|
|
|
|
import (
|
|
"fmt"
|
|
"os"
|
|
"path/filepath"
|
|
"strings"
|
|
|
|
"golang.org/x/xerrors"
|
|
)
|
|
|
|
// sshConfigMatchExecEscape prepares the path for use in `Match exec` statement.
|
|
//
|
|
// OpenSSH parses the Match line with a very simple tokenizer that accepts "-enclosed strings for the exec command, and
|
|
// has no supported escape sequences for ". This means we cannot include " within the command to execute.
|
|
//
|
|
// To make matters worse, on Windows, OpenSSH passes the string directly to cmd.exe for execution, and as far as I can
|
|
// tell, the only supported way to call a path that has spaces in it is to surround it with ".
|
|
//
|
|
// So, we can't actually include " directly, but here is a horrible workaround:
|
|
//
|
|
// "for /f %%a in ('powershell.exe -Command [char]34') do @cmd.exe /c %%aC:\Program Files\Coder\bin\coder.exe%%a connect exists %h"
|
|
//
|
|
// The key insight here is to store the character " in a variable (%a in this case, but the % itself needs to be
|
|
// escaped, so it becomes %%a), and then use that variable to construct the double-quoted path:
|
|
//
|
|
// %%aC:\Program Files\Coder\bin\coder.exe%%a.
|
|
//
|
|
// How do we generate a single " character without actually using that character? I couldn't find any command in cmd.exe
|
|
// to do it, but powershell.exe can convert ASCII to characters like this: `[char]34` (where 34 is the code point for ").
|
|
//
|
|
// Other notes:
|
|
// - @ in `@cmd.exe` suppresses echoing it, so you don't get this command printed
|
|
// - we need another invocation of cmd.exe (e.g. `do @cmd.exe /c %%aC:\Program Files\Coder\bin\coder.exe%%a`). Without
|
|
// it the double-quote gets interpreted as part of the path, and you get: '"C:\Program' is not recognized.
|
|
// Constructing the string and then passing it to another instance of cmd.exe does this trick here.
|
|
// - OpenSSH passes the `Match exec` command to cmd.exe regardless of whether the user has a unix-like shell like
|
|
// git bash, so we don't have a `forceUnixPath` option like for the ProxyCommand which does respect the user's
|
|
// configured shell on Windows.
|
|
func sshConfigMatchExecEscape(path string) (string, error) {
|
|
// This is unlikely to ever happen, but newlines are allowed on
|
|
// certain filesystems, but cannot be used inside ssh config.
|
|
if strings.ContainsAny(path, "\n") {
|
|
return "", xerrors.Errorf("invalid path: %s", path)
|
|
}
|
|
// Windows does not allow double-quotes or tabs in paths. If we get one it is an error.
|
|
if strings.ContainsAny(path, "\"\t") {
|
|
return "", xerrors.Errorf("path must not contain quotes or tabs: %q", path)
|
|
}
|
|
|
|
if strings.ContainsAny(path, " ") {
|
|
// c.f. function comment for how this works.
|
|
// Use absolute paths for powershell.exe and cmd.exe
|
|
// to avoid PATH resolution issues when both Path and
|
|
// PATH (MSYS-translated) exist in the environment.
|
|
sysRoot := os.Getenv("SYSTEMROOT")
|
|
pwsh := filepath.Join(sysRoot, "System32", "WindowsPowerShell", "v1.0", "powershell.exe")
|
|
cmd := filepath.Join(sysRoot, "System32", "cmd.exe")
|
|
path = fmt.Sprintf("for /f %%%%a in ('%s -Command [char]34') do @%s /c %%%%a%s%%%%a", pwsh, cmd, path) //nolint:gocritic // We don't want %q here.
|
|
}
|
|
return path, nil
|
|
}
|