From f6a4ed309f24ec500110dbd29608ed128afd01e2 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Tue, 2 Jun 2026 12:46:40 +0200 Subject: [PATCH] ci: fix Windows runner PATH casing for mise, not in cli (#25972) Co-authored-by: Claude Opus 4.8 (1M context) --- .github/actions/setup-mise/action.yml | 31 +++++++++----------- cli/agent_test.go | 4 +-- cli/configssh_internal_test.go | 11 ++----- cli/configssh_windows.go | 10 +------ cli/root.go | 41 ++------------------------- cli/root_test.go | 6 ++-- enterprise/cli/proxyserver_test.go | 2 +- 7 files changed, 22 insertions(+), 83 deletions(-) diff --git a/.github/actions/setup-mise/action.yml b/.github/actions/setup-mise/action.yml index 39aa9ab27d..751124ed42 100644 --- a/.github/actions/setup-mise/action.yml +++ b/.github/actions/setup-mise/action.yml @@ -166,23 +166,18 @@ runs: mise_dir: ${{ steps.mise-data-dir.outputs.path }} install_args: ${{ steps.cache-key.outputs.install-args }} cache: "false" + # Do not export mise's resolved env (every tool install dir) into + # GITHUB_ENV. Tools resolve through the shims dir on GITHUB_PATH, so + # the export only bloats PATH. On Windows the mise go shim re-prepends + # those dirs at invocation, and the resulting PATH crosses cmd.exe's + # ~8191 character limit, which makes cmd.exe drop PATH entirely and + # fail to resolve native executables in subprocesses spawned by tests. + env: false - - name: Ensure Git usr/bin is in PATH (Windows) + - name: Add Git usr/bin to PATH (Windows) if: runner.os == 'Windows' - shell: pwsh - # jdx/mise-action exports "Path" via GITHUB_ENV which may - # collide with bash's "PATH". Ensure Git usr/bin is present - # and remove any duplicate Path/PATH entries from GITHUB_ENV - # by writing both forms. - run: | # zizmor: ignore[github-env] - $gitdir = "C:\Program Files\Git\usr\bin" - $current = $env:Path - if ($current -notlike "*$gitdir*") { - $current = "$gitdir;$current" - } - # Write both Path and PATH to GITHUB_ENV so that both - # cmd.exe (uses Path) and bash/Go (uses PATH) see the - # same value including Git usr/bin. - "Path=$current" | Out-File -Append -FilePath $env:GITHUB_ENV -Encoding utf8 - "PATH=$current" | Out-File -Append -FilePath $env:GITHUB_ENV -Encoding utf8 - + shell: bash + # GITHUB_PATH is the casing-safe channel and keeps the entry short. + # cmd.exe subprocesses spawned by Go tests need MSYS coreutils such as + # printf, which live here. + run: echo "C:\Program Files\Git\usr\bin" >> "$GITHUB_PATH" diff --git a/cli/agent_test.go b/cli/agent_test.go index 9ea7afdcb1..60e8f68642 100644 --- a/cli/agent_test.go +++ b/cli/agent_test.go @@ -146,10 +146,8 @@ func TestWorkspaceAgent(t *testing.T) { }).WithAgent().Do() coderURLEnv := "$CODER_URL" - headerCmd := "printf X-Process-Testing=very-wow-" + coderURLEnv + "'\\r\\n'X-Process-Testing2=more-wow" if runtime.GOOS == "windows" { coderURLEnv = "%CODER_URL%" - headerCmd = "echo X-Process-Testing=very-wow-" + coderURLEnv + "& echo X-Process-Testing2=more-wow" } logDir := t.TempDir() @@ -161,7 +159,7 @@ func TestWorkspaceAgent(t *testing.T) { "--log-dir", logDir, "--agent-header", "X-Testing=agent", "--agent-header", "Cool-Header=Ethan was Here!", - "--agent-header-command", headerCmd, + "--agent-header-command", "printf X-Process-Testing=very-wow-"+coderURLEnv+"'\\r\\n'X-Process-Testing2=more-wow", "--socket-path", testutil.AgentSocketPath(t), ) clitest.Start(t, agentInv) diff --git a/cli/configssh_internal_test.go b/cli/configssh_internal_test.go index 59b57439af..0ea2ae6ea5 100644 --- a/cli/configssh_internal_test.go +++ b/cli/configssh_internal_test.go @@ -229,15 +229,8 @@ func Test_sshConfigMatchExecEscape(t *testing.T) { // OpenSSH processes %% escape sequences into % escaped = strings.ReplaceAll(escaped, "%%", "%") - c := exec.Command(cmd, arg, escaped) //nolint:gosec - if runtime.GOOS == "windows" { - // Deduplicate Path/PATH env vars so cmd.exe - // subprocesses (like powershell.exe used for - // paths with spaces) resolve correctly. - c.Env = appendAndDedupEnv(os.Environ()) - } - b, err := c.CombinedOutput() - require.NoError(t, err, "command output: %s", string(b)) + b, err := exec.Command(cmd, arg, escaped).CombinedOutput() //nolint:gosec + require.NoError(t, err) got := strings.TrimSpace(string(b)) require.Equal(t, "yay", got) }) diff --git a/cli/configssh_windows.go b/cli/configssh_windows.go index 53473c7aa4..db81bce1ff 100644 --- a/cli/configssh_windows.go +++ b/cli/configssh_windows.go @@ -4,8 +4,6 @@ package cli import ( "fmt" - "os" - "path/filepath" "strings" "golang.org/x/xerrors" @@ -52,13 +50,7 @@ func sshConfigMatchExecEscape(path string) (string, error) { 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. + path = fmt.Sprintf("for /f %%%%a in ('powershell.exe -Command [char]34') do @cmd.exe /c %%%%a%s%%%%a", path) //nolint:gocritic // We don't want %q here. } return path, nil } diff --git a/cli/root.go b/cli/root.go index 2e4aa7dd17..a40ac7c3c2 100644 --- a/cli/root.go +++ b/cli/root.go @@ -1701,44 +1701,7 @@ func (r roundTripper) RoundTrip(req *http.Request) (*http.Response, error) { return r(req) } -// appendAndDedupEnv appends extra environment variables and -// deduplicates entries with the same key (case-insensitive on -// Windows). For the PATH variable specifically, it prefers the -// value that contains native Windows paths (with backslashes) -// over MSYS-translated paths (with forward slashes). For all -// other variables, the last value wins. -func appendAndDedupEnv(env []string, extra ...string) []string { - env = append(env, extra...) - if runtime.GOOS != "windows" { - return env - } - seen := make(map[string]int, len(env)) - result := make([]string, 0, len(env)) - for _, e := range env { - key, val, ok := strings.Cut(e, "=") - if !ok { - result = append(result, e) - continue - } - upper := strings.ToUpper(key) - if idx, exists := seen[upper]; exists { - if upper == "PATH" { - // Prefer the value with native Windows paths. - existingVal := result[idx][len(key)+1:] - if strings.Contains(existingVal, "\\") && !strings.Contains(val, "\\") { - continue - } - } - result[idx] = e - continue - } - seen[upper] = len(result) - result = append(result, e) - } - return result -} - -// headerTransport creates a new transport that executes `--header-command` +// HeaderTransport creates a new transport that executes `--header-command` // if it is set to add headers for all outbound requests. func headerTransport(ctx context.Context, serverURL *url.URL, header []string, headerCommand string) (*codersdk.HeaderTransport, error) { transport := &codersdk.HeaderTransport{ @@ -1756,7 +1719,7 @@ func headerTransport(ctx context.Context, serverURL *url.URL, header []string, h var outBuf bytes.Buffer // #nosec cmd := exec.CommandContext(ctx, shell, caller, headerCommand) - cmd.Env = appendAndDedupEnv(os.Environ(), "CODER_URL="+serverURL.String()) + cmd.Env = append(os.Environ(), "CODER_URL="+serverURL.String()) cmd.Stdout = &outBuf cmd.Stderr = io.Discard err := cmd.Run() diff --git a/cli/root_test.go b/cli/root_test.go index aaf81f574e..fefb87382c 100644 --- a/cli/root_test.go +++ b/cli/root_test.go @@ -177,17 +177,15 @@ func TestRoot(t *testing.T) { url = srv.URL buf := new(bytes.Buffer) coderURLEnv := "$CODER_URL" - headerCmd := "printf X-Process-Testing=very-wow-" + coderURLEnv + "'\\r\\n'X-Process-Testing2=more-wow" if runtime.GOOS == "windows" { coderURLEnv = "%CODER_URL%" - headerCmd = "echo X-Process-Testing=very-wow-" + coderURLEnv + "& echo X-Process-Testing2=more-wow" } inv, _ := clitest.New(t, "--no-feature-warning", "--no-version-warning", "--header", "X-Testing=wow", "--header", "Cool-Header=Dean was Here!", - "--header-command", headerCmd, + "--header-command", "printf X-Process-Testing=very-wow-"+coderURLEnv+"'\\r\\n'X-Process-Testing2=more-wow", "login", srv.URL, ) inv.Stdout = buf @@ -268,7 +266,7 @@ func TestDERPHeaders(t *testing.T) { "--no-version-warning", "ping", workspace.Name, "-n", "1", - "--header-command", "echo X-Process-Testing=very-wow", + "--header-command", "printf X-Process-Testing=very-wow", } for k, v := range expectedHeaders { if k != "X-Process-Testing" { diff --git a/enterprise/cli/proxyserver_test.go b/enterprise/cli/proxyserver_test.go index 15f0003099..556597ab76 100644 --- a/enterprise/cli/proxyserver_test.go +++ b/enterprise/cli/proxyserver_test.go @@ -48,7 +48,7 @@ func Test_ProxyServer_Headers(t *testing.T) { "--access-url", "http://localhost:8080", "--http-address", ":0", "--header", fmt.Sprintf("%s=%s", headerName1, headerVal1), - "--header-command", fmt.Sprintf("echo %s=%s", headerName2, headerVal2), + "--header-command", fmt.Sprintf("printf %s=%s", headerName2, headerVal2), ) pty := ptytest.New(t) inv.Stdout = pty.Output()