diff --git a/.github/actions/setup-mise/action.yml b/.github/actions/setup-mise/action.yml index 847eb6ef51..39aa9ab27d 100644 --- a/.github/actions/setup-mise/action.yml +++ b/.github/actions/setup-mise/action.yml @@ -166,3 +166,23 @@ runs: mise_dir: ${{ steps.mise-data-dir.outputs.path }} install_args: ${{ steps.cache-key.outputs.install-args }} cache: "false" + + - name: Ensure Git usr/bin is in 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 + diff --git a/cli/agent_test.go b/cli/agent_test.go index 60e8f68642..9ea7afdcb1 100644 --- a/cli/agent_test.go +++ b/cli/agent_test.go @@ -146,8 +146,10 @@ 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() @@ -159,7 +161,7 @@ func TestWorkspaceAgent(t *testing.T) { "--log-dir", logDir, "--agent-header", "X-Testing=agent", "--agent-header", "Cool-Header=Ethan was Here!", - "--agent-header-command", "printf X-Process-Testing=very-wow-"+coderURLEnv+"'\\r\\n'X-Process-Testing2=more-wow", + "--agent-header-command", headerCmd, "--socket-path", testutil.AgentSocketPath(t), ) clitest.Start(t, agentInv) diff --git a/cli/configssh_internal_test.go b/cli/configssh_internal_test.go index 0ea2ae6ea5..59b57439af 100644 --- a/cli/configssh_internal_test.go +++ b/cli/configssh_internal_test.go @@ -229,8 +229,15 @@ func Test_sshConfigMatchExecEscape(t *testing.T) { // OpenSSH processes %% escape sequences into % escaped = strings.ReplaceAll(escaped, "%%", "%") - b, err := exec.Command(cmd, arg, escaped).CombinedOutput() //nolint:gosec - require.NoError(t, err) + 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)) got := strings.TrimSpace(string(b)) require.Equal(t, "yay", got) }) diff --git a/cli/configssh_windows.go b/cli/configssh_windows.go index db81bce1ff..53473c7aa4 100644 --- a/cli/configssh_windows.go +++ b/cli/configssh_windows.go @@ -4,6 +4,8 @@ package cli import ( "fmt" + "os" + "path/filepath" "strings" "golang.org/x/xerrors" @@ -50,7 +52,13 @@ func sshConfigMatchExecEscape(path string) (string, error) { if strings.ContainsAny(path, " ") { // c.f. function comment for how this works. - 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. + // 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 } diff --git a/cli/root.go b/cli/root.go index a40ac7c3c2..2e4aa7dd17 100644 --- a/cli/root.go +++ b/cli/root.go @@ -1701,7 +1701,44 @@ func (r roundTripper) RoundTrip(req *http.Request) (*http.Response, error) { return r(req) } -// HeaderTransport creates a new transport that executes `--header-command` +// 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` // 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{ @@ -1719,7 +1756,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 = append(os.Environ(), "CODER_URL="+serverURL.String()) + cmd.Env = appendAndDedupEnv(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 fefb87382c..aaf81f574e 100644 --- a/cli/root_test.go +++ b/cli/root_test.go @@ -177,15 +177,17 @@ 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", "printf X-Process-Testing=very-wow-"+coderURLEnv+"'\\r\\n'X-Process-Testing2=more-wow", + "--header-command", headerCmd, "login", srv.URL, ) inv.Stdout = buf @@ -266,7 +268,7 @@ func TestDERPHeaders(t *testing.T) { "--no-version-warning", "ping", workspace.Name, "-n", "1", - "--header-command", "printf X-Process-Testing=very-wow", + "--header-command", "echo 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 556597ab76..15f0003099 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("printf %s=%s", headerName2, headerVal2), + "--header-command", fmt.Sprintf("echo %s=%s", headerName2, headerVal2), ) pty := ptytest.New(t) inv.Stdout = pty.Output()