diff --git a/internal/process/process.go b/internal/process/process.go index 44827071be..a7d6cb1371 100644 --- a/internal/process/process.go +++ b/internal/process/process.go @@ -86,9 +86,11 @@ func ParseSignal(sig string) (Signal, error) { // Configuration for a Process type Config struct { - PTY bool - Path string - Args []string + PTY bool + Path string + Args []string + // WindowsCmdLine bypasses os/exec argument quoting when non-empty. + WindowsCmdLine string Env []string Stdin io.Reader Stdout io.Writer diff --git a/internal/process/signal_windows.go b/internal/process/signal_windows.go index fb45c1af32..3ad89b16aa 100644 --- a/internal/process/signal_windows.go +++ b/internal/process/signal_windows.go @@ -50,6 +50,7 @@ func (p *Process) setupProcessGroup() { } p.command.SysProcAttr = &windows.SysProcAttr{ CreationFlags: windows.CREATE_UNICODE_ENVIRONMENT | windows.CREATE_NEW_PROCESS_GROUP, + CmdLine: p.conf.WindowsCmdLine, } jobHandle, err := newJobObject() if err != nil { diff --git a/internal/shell/lookpath.go b/internal/shell/lookpath.go index a6634df010..0af4bc8dad 100644 --- a/internal/shell/lookpath.go +++ b/internal/shell/lookpath.go @@ -51,3 +51,7 @@ func LookPath(file, path, fileExtensions string) (string, error) { } return "", &exec.Error{Name: file, Err: exec.ErrNotFound} } + +func systemCommandProcessor() (string, error) { + return "cmd.exe", nil +} diff --git a/internal/shell/lookpath_windows.go b/internal/shell/lookpath_windows.go index d5087cccaf..ee60c4b381 100644 --- a/internal/shell/lookpath_windows.go +++ b/internal/shell/lookpath_windows.go @@ -9,10 +9,13 @@ package shell import ( + "fmt" "os" "os/exec" "path/filepath" "strings" + + "golang.org/x/sys/windows" ) func chkStat(file string) error { @@ -89,3 +92,11 @@ func LookPath(file, path, fileExtensions string) (string, error) { } return "", &exec.Error{file, exec.ErrNotFound} } + +func systemCommandProcessor() (string, error) { + systemDirectory, err := windows.GetSystemDirectory() + if err != nil { + return "", fmt.Errorf("finding Windows system directory: %w", err) + } + return filepath.Join(systemDirectory, "cmd.exe"), nil +} diff --git a/internal/shell/shell.go b/internal/shell/shell.go index 39cc5fed49..b8592abf11 100644 --- a/internal/shell/shell.go +++ b/internal/shell/shell.go @@ -267,9 +267,10 @@ retryLoop: // Command represents a command that can be run in a shell. type Command struct { - shell *Shell - command string - args []string + shell *Shell + command string + args []string + windowsCmdLine string } // Command returns a command that can be run in the shell. @@ -295,6 +296,9 @@ func (s *Shell) Script(path, commandOverride string) (Command, error) { isSh := filepath.Ext(path) == "" || filepath.Ext(path) == ".sh" isWindows := runtime.GOOS == "windows" isPwsh := filepath.Ext(path) == ".ps1" + ext := strings.ToLower(filepath.Ext(path)) + isBatch := isWindows && (ext == ".bat" || ext == ".cmd") + var windowsCmdLine string if commandOverride != "" { // first element is the command, all others are args to which we append path @@ -311,6 +315,21 @@ func (s *Shell) Script(path, commandOverride string) (Command, error) { } switch { + case isBatch: + // Batch files are interpreted by cmd.exe, not executed directly by + // CreateProcess. Use a raw command line because cmd.exe's quoting rules + // differ from the CommandLineToArgvW rules used by os/exec. + if strings.Contains(path, "%") { + return Command{}, fmt.Errorf("cannot run Windows batch script %q: path contains '%%', which cmd.exe expands", path) + } + cmdPath, err := systemCommandProcessor() + if err != nil { + return Command{}, fmt.Errorf("finding command processor for Windows batch script: %w", err) + } + command = cmdPath + args = []string{"/D", "/V:OFF", "/S", "/C", path} + windowsCmdLine = `"` + command + `" /D /V:OFF /S /C ""` + path + `""` + case isWindows && isSh: if s.debug { s.Commentf("Attempting to run %s with Bash for Windows", path) @@ -386,9 +405,10 @@ func (s *Shell) Script(path, commandOverride string) (Command, error) { } return Command{ - shell: s, - command: command, - args: args, + shell: s, + command: command, + args: args, + windowsCmdLine: windowsCmdLine, }, nil } @@ -420,6 +440,7 @@ func (c Command) Run(ctx context.Context, opts ...RunCommandOpt) error { c.shell.Errorf("Error building command: %v", err) return err } + cmdCfg.WindowsCmdLine = c.windowsCmdLine // Merge in any extra env vars. if cfg.extraEnv != nil { diff --git a/internal/shell/shell_test.go b/internal/shell/shell_test.go index 05bec456c8..6cede199ed 100644 --- a/internal/shell/shell_test.go +++ b/internal/shell/shell_test.go @@ -130,6 +130,76 @@ func TestRun(t *testing.T) { } } +func TestRunWindowsBatchScriptWithPipedOutput(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skip("batch scripts require Windows") + } + t.Parallel() + + for _, ext := range []string{".bat", ".CMD"} { + t.Run(ext, func(t *testing.T) { + dir := filepath.Join(t.TempDir(), "hooks with spaces & parentheses (test) ! caret ^") + if err := os.Mkdir(dir, 0o755); err != nil { + t.Fatalf("os.Mkdir(%q) = %v", dir, err) + } + path := filepath.Join(dir, "agent-startup"+ext) + if err := os.WriteFile(path, []byte("@echo off\r\necho stdout=%BATCH_TEST_VALUE%\r\n>&2 echo stderr-line\r\n"), 0o755); err != nil { + t.Fatalf("os.WriteFile(%q) = %v", path, err) + } + + out := new(bytes.Buffer) + sh := newShellForTest(t, shell.WithStdout(out), shell.WithPTY(false)) + sh.Env.Set("BATCH_TEST_VALUE", "expanded") + script, err := sh.Script(path, "") + if err != nil { + t.Fatalf("sh.Script(%q, %q) = %v", path, "", err) + } + if err := script.Run(t.Context(), shell.ShowPrompt(false)); err != nil { + t.Fatalf("script.Run() = %v", err) + } + + if diff := cmp.Diff(out.String(), "stdout=expanded\r\nstderr-line\r\n"); diff != "" { + t.Errorf("batch output diff (-got +want):\n%s", diff) + } + }) + } +} + +func TestRunWindowsBatchScriptPreservesExitCode(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skip("batch scripts require Windows") + } + t.Parallel() + + path := filepath.Join(t.TempDir(), "nonzero.cmd") + if err := os.WriteFile(path, []byte("@exit /b 23\r\n"), 0o755); err != nil { + t.Fatalf("os.WriteFile(%q) = %v", path, err) + } + + sh := newShellForTest(t, shell.WithPTY(false)) + script, err := sh.Script(path, "") + if err != nil { + t.Fatalf("sh.Script(%q, %q) = %v", path, "", err) + } + if err := script.Run(t.Context(), shell.ShowPrompt(false)); shell.ExitCode(err) != 23 { + t.Fatalf("shell.ExitCode(script.Run()) = %d, want 23 (error: %v)", shell.ExitCode(err), err) + } +} + +func TestWindowsBatchScriptRejectsPercentInPath(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skip("batch scripts require Windows") + } + t.Parallel() + + sh := newShellForTest(t) + path := filepath.Join(t.TempDir(), "%TEMP%", "agent-startup.bat") + _, err := sh.Script(path, "") + if err == nil || !strings.Contains(err.Error(), "path contains '%'") { + t.Fatalf("sh.Script(%q, %q) error = %v, want percent-path error", path, "", err) + } +} + func TestRunWithStdin(t *testing.T) { t.Parallel()