diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 6299adc..7880351 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -26,3 +26,16 @@ jobs: - name: Test run: go test -v ./... + + windows-gcloud-launch: + runs-on: windows-latest + steps: + - uses: actions/checkout@v4 + + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version-file: go.mod + + - name: Test Windows batch launch + run: go test -count=1 -timeout 5m -run "TestRunGcloudADCLoginWindowsFakeCmd|TestWindowsBatchDetectionAndQuoting" . diff --git a/reauth.go b/reauth.go index 98f97a4..6670053 100644 --- a/reauth.go +++ b/reauth.go @@ -310,25 +310,61 @@ func gcloudADCLoginArgs(getenv func(string) string) []string { // fixed argument vector. Stdout is attached to stderr so query output is // not mixed with gcloud's instructions. Tokens and ADC JSON are not logged. // -// On Windows the SDK installs gcloud as gcloud.cmd. exec.LookPath resolves -// it through PATHEXT and CreateProcess launches batch files through cmd.exe -// implicitly, so no explicit interpreter is needed (this is the same -// mechanism os/exec documents under its cmd.exe quoting caveat). The cmd.exe -// unquoting differences do not matter here because every argument is a fixed -// literal without spaces or metacharacters; only the resolved path may -// contain spaces, and Go quotes argv[0] like any other argument. +// On Windows the Cloud SDK installs gcloud as gcloud.cmd. CreateProcessW does +// not launch batch files by itself; Go's os/exec calls CreateProcess directly. +// The batch file is therefore started through cmd.exe. See windowsBatchCommand. func runGcloudADCLogin(ctx context.Context, getenv func(string) string, lookPath func(string) (string, error)) error { bin, err := lookPath("gcloud") if err != nil { return err } - cmd := exec.CommandContext(ctx, bin, gcloudADCLoginArgs(getenv)...) + cmd := gcloudCommand(ctx, bin, gcloudADCLoginArgs(getenv)) cmd.Stdin = os.Stdin cmd.Stdout = os.Stderr cmd.Stderr = os.Stderr return cmd.Run() } +// gcloudCommand starts gcloud. CreateProcessW does not run batch files, so a +// resolved .cmd or .bat is started through cmd.exe. An .exe is executed +// directly. os/exec quoting matches CommandLineToArgvW, which cmd.exe does +// not use, so the batch command line is built separately. +func gcloudCommand(ctx context.Context, bin string, args []string) *exec.Cmd { + if runtime.GOOS == "windows" && isWindowsBatch(bin) { + return windowsBatchCommand(ctx, bin, args) + } + return exec.CommandContext(ctx, bin, args...) +} + +func isWindowsBatch(path string) bool { + switch strings.ToLower(filepath.Ext(path)) { + case ".cmd", ".bat": + return true + default: + return false + } +} + +// windowsCmdCommandLine is the tail passed to cmd.exe. The /s switch strips +// one surrounding pair of quotes, leaving a quoted batch path and its +// arguments. Paths and arguments that contain spaces or cmd metacharacters +// are quoted with doubled quotes. +func windowsCmdCommandLine(bin string, args []string) string { + parts := make([]string, 0, 1+len(args)) + parts = append(parts, quoteCmdExeArg(bin)) + for _, arg := range args { + parts = append(parts, quoteCmdExeArg(arg)) + } + return `/d /s /c "` + strings.Join(parts, " ") + `"` +} + +func quoteCmdExeArg(s string) string { + if s == "" || strings.ContainsAny(s, " \t\"&<>()[]{}^=;!'+,`~%") { + return `"` + strings.ReplaceAll(s, `"`, `""`) + `"` + } + return s +} + func (h *reauthHooks) tryLogin(ctx context.Context) error { h.mu.Lock() defer h.mu.Unlock() diff --git a/reauth_batch_other.go b/reauth_batch_other.go new file mode 100644 index 0000000..000292c --- /dev/null +++ b/reauth_batch_other.go @@ -0,0 +1,12 @@ +//go:build !windows + +package main + +import ( + "context" + "os/exec" +) + +func windowsBatchCommand(ctx context.Context, bin string, args []string) *exec.Cmd { + return exec.CommandContext(ctx, bin, args...) +} diff --git a/reauth_batch_test.go b/reauth_batch_test.go new file mode 100644 index 0000000..c70e7f2 --- /dev/null +++ b/reauth_batch_test.go @@ -0,0 +1,25 @@ +package main + +import ( + "strings" + "testing" +) + +func TestWindowsBatchDetectionAndQuoting(t *testing.T) { + if !isWindowsBatch(`C:\Program Files\Google\Cloud SDK\bin\gcloud.cmd`) || !isWindowsBatch(`gcloud.bat`) { + t.Fatal("cmd and bat files must use cmd.exe") + } + if isWindowsBatch(`C:\Program Files\gcloud.exe`) || isWindowsBatch(`gcloud`) { + t.Fatal("executables must not be wrapped in cmd.exe") + } + bin := `C:\Program Files\Google\Cloud SDK\bin\gcloud.cmd` + line := windowsCmdCommandLine(bin, []string{"auth", "application-default", "login", "--no-launch-browser"}) + want := `/d /s /c ""C:\Program Files\Google\Cloud SDK\bin\gcloud.cmd" auth application-default login --no-launch-browser"` + if line != want { + t.Fatalf("line = %q, want %q", line, want) + } + meta := windowsCmdCommandLine(`C:\tools\gcloud.cmd`, []string{`a&b`, `x(y)`}) + if !strings.Contains(meta, `"a&b"`) || !strings.Contains(meta, `"x(y)"`) { + t.Fatalf("metacharacters were not quoted: %s", meta) + } +} diff --git a/reauth_windows.go b/reauth_windows.go new file mode 100644 index 0000000..ee37bea --- /dev/null +++ b/reauth_windows.go @@ -0,0 +1,22 @@ +//go:build windows + +package main + +import ( + "context" + "os" + "os/exec" + "syscall" +) + +func windowsBatchCommand(ctx context.Context, bin string, args []string) *exec.Cmd { + comspec := os.Getenv("ComSpec") + if comspec == "" { + comspec = `C:\Windows\System32\cmd.exe` + } + cmd := exec.CommandContext(ctx, comspec) + cmd.SysProcAttr = &syscall.SysProcAttr{ + CmdLine: comspec + " " + windowsCmdCommandLine(bin, args), + } + return cmd +} diff --git a/reauth_windows_test.go b/reauth_windows_test.go new file mode 100644 index 0000000..45d5dd9 --- /dev/null +++ b/reauth_windows_test.go @@ -0,0 +1,53 @@ +package main + +import ( + "context" + "os" + "path/filepath" + "runtime" + "strings" + "testing" +) + +func TestRunGcloudADCLoginWindowsFakeCmd(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skip("requires a Windows command interpreter to launch a .cmd file") + } + dir := filepath.Join(t.TempDir(), "Cloud SDK") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + argvPath := filepath.Join(dir, "argv.txt") + script := "@echo off\r\n" + + "echo %1>>" + windowsQuote(argvPath) + "\r\n" + + "echo %2>>" + windowsQuote(argvPath) + "\r\n" + + "echo %3>>" + windowsQuote(argvPath) + "\r\n" + + "echo %4>>" + windowsQuote(argvPath) + "\r\n" + + "echo gcloud-login-stdout\r\n" + gcloudPath := filepath.Join(dir, "gcloud.cmd") + if err := os.WriteFile(gcloudPath, []byte(script), 0o644); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", dir) + t.Setenv("PATHEXT", ".CMD;.EXE") + t.Setenv(envSSHConnection, "203.0.113.1 60000 203.0.113.2 22") + + err := runGcloudADCLogin(context.Background(), os.Getenv, func(string) (string, error) { + return gcloudPath, nil + }) + if err != nil { + t.Fatal(err) + } + got, err := os.ReadFile(argvPath) + if err != nil { + t.Fatal(err) + } + want := "auth\r\napplication-default\r\nlogin\r\n--no-launch-browser\r\n" + if strings.ReplaceAll(string(got), "\r\n", "\n") != strings.ReplaceAll(want, "\r\n", "\n") { + t.Fatalf("argv = %q, want %q", got, want) + } +} + +func windowsQuote(path string) string { + return `"` + path + `"` +}