Merge pull request ActiveState#3389 from ActiveState/DX-2898

Work around Windows bug where arguments are not properly quoted

Author: Naatan

cmd/state-exec/cmd.go

@@ -3,12 +3,11 @@ package main
 import (
 	"fmt"
 	"os"
-	"os/exec"
 )
 
 func runCmd(meta *executorMeta) (int, error) {
 	userArgs := os.Args[1:]
-	cmd := exec.Command(meta.MatchingBin, userArgs...)
+	cmd := Command(meta.MatchingBin, userArgs...)
 	cmd.Stdin = os.Stdin
 	cmd.Stdout = os.Stdout
 	cmd.Stderr = os.Stderr

cmd/state-exec/cmd_lin_mac.go

@@ -0,0 +1,10 @@
+//go:build !windows
+// +build !windows
+
+package main
+
+import "os/exec"
+
+func Command(name string, arg ...string) *exec.Cmd {
+	return exec.Command(name, arg...)
+}

cmd/state-exec/cmd_windows.go

@@ -0,0 +1,99 @@
+package main
+
+import (
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"syscall"
+
+	"github.com/ActiveState/cli/cmd/state-exec/internal/logr"
+)
+
+func Command(name string, arg ...string) *exec.Cmd {
+	cmd := exec.Command(name, arg...)
+
+	exeName := filepath.Base(strings.ToLower(name))
+	if exeName == "cmd" || strings.HasSuffix(exeName, ".bat") || strings.HasSuffix(exeName, ".cmd") {
+		// Go currently does not escape arguments properly on Windows, it account for spaces and tab characters, but not
+		// other characters that need escaping such as `<` and `>`.
+		// This can be dropped once we update to a Go version that fixes this bug: https://github.com/golang/go/issues/68313
+		cmd.SysProcAttr = &syscall.SysProcAttr{CmdLine: makeCmdLine(cmd.Args)}
+		logr.Debug("processed command line: %s", cmd.SysProcAttr.CmdLine)
+	}
+
+	return cmd
+}
+
+// makeCmdLine builds a command line out of args by escaping "special"
+// characters and joining the arguments with spaces.
+// Based on syscall\exec_windows.go
+func makeCmdLine(args []string) string {
+	var b []byte
+	for _, v := range args {
+		if len(b) > 0 {
+			b = append(b, ' ')
+		}
+		b = appendEscapeArg(b, v)
+	}
+	return string(b)
+}
+
+// appendEscapeArg escapes the string s, as per escapeArg,
+// appends the result to b, and returns the updated slice.
+// Based on syscall\exec_windows.go
+func appendEscapeArg(b []byte, s string) []byte {
+	if len(s) == 0 {
+		return append(b, `""`...)
+	}
+
+	needsBackslash := false
+	needsQuotes := false
+	for i := 0; i < len(s); i++ {
+		switch s[i] {
+		case '"', '\\':
+			needsBackslash = true
+		// Based on https://github.com/sebres/PoC/blob/master/SB-0D-001-win-exec/SOLUTION.md#definition
+		case ' ', '\t', '<', '>', '&', '|', '^', '!', '(', ')', '%':
+			needsQuotes = true
+		}
+	}
+
+	if !needsBackslash && !needsQuotes {
+		// No special handling required; normal case.
+		return append(b, s...)
+	}
+	if !needsBackslash {
+		// hasSpace is true, so we need to quote the string.
+		b = append(b, '"')
+		b = append(b, s...)
+		return append(b, '"')
+	}
+
+	if needsQuotes {
+		b = append(b, '"')
+	}
+	slashes := 0
+	for i := 0; i < len(s); i++ {
+		c := s[i]
+		switch c {
+		default:
+			slashes = 0
+		case '\\':
+			slashes++
+		case '"':
+			for ; slashes > 0; slashes-- {
+				b = append(b, '\\')
+			}
+			b = append(b, '\\')
+		}
+		b = append(b, c)
+	}
+	if needsQuotes {
+		for ; slashes > 0; slashes-- {
+			b = append(b, '\\')
+		}
+		b = append(b, '"')
+	}
+
+	return b
+}

internal/osutils/exeutils.go

@@ -97,7 +97,7 @@ func ExecSimple(bin string, args []string, env []string) (string, string, error)
 
 func ExecSimpleFromDir(dir, bin string, args []string, env []string) (string, string, error) {
 	logging.Debug("ExecSimpleFromDir: dir: %s, bin: %s, args: %#v, env: %#v", dir, bin, args, env)
-	c := exec.Command(bin, args...)
+	c := Command(bin, args...)
 	if dir != "" {
 		c.Dir = dir
 	}
@@ -117,7 +117,7 @@ func ExecSimpleFromDir(dir, bin string, args []string, env []string) (string, st
 
 // Execute will run the given command and with optional settings for the exec.Cmd struct
 func Execute(command string, arg []string, optSetter func(cmd *exec.Cmd) error) (int, *exec.Cmd, error) {
-	cmd := exec.Command(command, arg...)
+	cmd := Command(command, arg...)
 	logging.Debug("Executing command: %s, with args: %s", cmd, arg)
 	if optSetter != nil {
 		if err := optSetter(cmd); err != nil {
@@ -145,7 +145,7 @@ func ExecuteAndPipeStd(command string, arg []string, env []string) (int, *exec.C
 // ExecuteAndForget will run the given command in the background, returning immediately.
 func ExecuteAndForget(command string, args []string, opts ...func(cmd *exec.Cmd) error) (*os.Process, error) {
 	logging.Debug("Executing: %s %v", command, args)
-	cmd := exec.Command(command, args...)
+	cmd := Command(command, args...)
 
 	for _, optSetter := range opts {
 		if err := optSetter(cmd); err != nil {
@@ -172,7 +172,7 @@ func ExecuteAndForget(command string, args []string, opts ...func(cmd *exec.Cmd)
 func ExecuteInBackground(command string, args []string, opts ...func(cmd *exec.Cmd) error) (*exec.Cmd, *bytes.Buffer, *bytes.Buffer, error) {
 	logging.Debug("Executing: %s %v", command, args)
 
-	cmd := exec.Command(command, args...)
+	cmd := Command(command, args...)
 	var stdoutBuf, stderrBuf bytes.Buffer
 
 	for _, optSetter := range opts {

internal/osutils/exeutils_unix.go

@@ -3,6 +3,12 @@
 
 package osutils
 
+import "os/exec"
+
 const ExeExtension = ""
 
 var exts = []string{}
+
+func Command(name string, arg ...string) *exec.Cmd {
+	return exec.Command(name, arg...)
+}

internal/osutils/exeutils_windows.go

@@ -2,7 +2,10 @@ package osutils
 
 import (
 	"os"
+	"os/exec"
+	"path/filepath"
 	"strings"
+	"syscall"
 
 	"github.com/thoas/go-funk"
 )
@@ -15,3 +18,91 @@ func init() {
 	PATHEXT := os.Getenv("PATHEXT")
 	exts = funk.Uniq(funk.Map(strings.Split(PATHEXT, string(os.PathListSeparator)), strings.ToLower).([]string)).([]string)
 }
+
+func Command(name string, arg ...string) *exec.Cmd {
+	cmd := exec.Command(name, arg...)
+
+	exeName := filepath.Base(strings.ToLower(name))
+	if exeName == "cmd" || strings.HasSuffix(exeName, ".bat") || strings.HasSuffix(exeName, ".cmd") {
+		// Go currently does not escape arguments properly on Windows, it account for spaces and tab characters, but not
+		// other characters that need escaping such as `<` and `>`.
+		// This can be dropped once we update to a Go version that fixes this bug: https://github.com/golang/go/issues/68313
+		cmd.SysProcAttr = &syscall.SysProcAttr{CmdLine: makeCmdLine(cmd.Args)}
+	}
+
+	return cmd
+}
+
+// makeCmdLine builds a command line out of args by escaping "special"
+// characters and joining the arguments with spaces.
+// Based on syscall\exec_windows.go
+func makeCmdLine(args []string) string {
+	var b []byte
+	for _, v := range args {
+		if len(b) > 0 {
+			b = append(b, ' ')
+		}
+		b = appendEscapeArg(b, v)
+	}
+	return string(b)
+}
+
+// appendEscapeArg escapes the string s, as per escapeArg,
+// appends the result to b, and returns the updated slice.
+// Based on syscall\exec_windows.go
+func appendEscapeArg(b []byte, s string) []byte {
+	if len(s) == 0 {
+		return append(b, `""`...)
+	}
+
+	needsBackslash := false
+	needsQuotes := false
+	for i := 0; i < len(s); i++ {
+		switch s[i] {
+		case '"', '\\':
+			needsBackslash = true
+		// Based on https://github.com/sebres/PoC/blob/master/SB-0D-001-win-exec/SOLUTION.md#definition
+		case ' ', '\t', '<', '>', '&', '|', '^', '!', '(', ')', '%':
+			needsQuotes = true
+		}
+	}
+
+	if !needsBackslash && !needsQuotes {
+		// No special handling required; normal case.
+		return append(b, s...)
+	}
+	if !needsBackslash {
+		// hasSpace is true, so we need to quote the string.
+		b = append(b, '"')
+		b = append(b, s...)
+		return append(b, '"')
+	}
+
+	if needsQuotes {
+		b = append(b, '"')
+	}
+	slashes := 0
+	for i := 0; i < len(s); i++ {
+		c := s[i]
+		switch c {
+		default:
+			slashes = 0
+		case '\\':
+			slashes++
+		case '"':
+			for ; slashes > 0; slashes-- {
+				b = append(b, '\\')
+			}
+			b = append(b, '\\')
+		}
+		b = append(b, c)
+	}
+	if needsQuotes {
+		for ; slashes > 0; slashes-- {
+			b = append(b, '\\')
+		}
+		b = append(b, '"')
+	}
+
+	return b
+}

internal/testhelpers/e2e/session.go

@@ -4,24 +4,24 @@ import (
 	"fmt"
 	"io/fs"
 	"os"
-	"os/exec"
 	"path/filepath"
 	"regexp"
 	"runtime"
 	"strings"
 	"testing"
 	"time"
 
-	"github.com/ActiveState/cli/internal/subshell"
-	"github.com/ActiveState/cli/pkg/platform/model"
-	"github.com/ActiveState/cli/pkg/platform/model/buildplanner"
-	"github.com/ActiveState/cli/pkg/projectfile"
 	"github.com/ActiveState/termtest"
 	"github.com/go-openapi/strfmt"
 	"github.com/google/uuid"
 	"github.com/phayes/permbits"
 	"github.com/stretchr/testify/require"
 
+	"github.com/ActiveState/cli/internal/subshell"
+	"github.com/ActiveState/cli/pkg/platform/model"
+	"github.com/ActiveState/cli/pkg/platform/model/buildplanner"
+	"github.com/ActiveState/cli/pkg/projectfile"
+
 	"github.com/ActiveState/cli/internal/condition"
 	"github.com/ActiveState/cli/internal/config"
 	"github.com/ActiveState/cli/internal/constants"
@@ -298,7 +298,7 @@ func (s *Session) SpawnCmdWithOpts(exe string, optSetters ...SpawnOptSetter) *Sp
 		args = spawnOpts.Args
 	}
 
-	cmd := exec.Command(shell, args...)
+	cmd := osutils.Command(shell, args...)
 
 	cmd.Env = spawnOpts.Env
 	if spawnOpts.Dir != "" {

pkg/platform/runtime/executors/executors.go

@@ -46,6 +46,10 @@ func (es *Executors) ExecutorSrc() (string, error) {
 	return installation.ExecutorExec()
 }
 
+func (es *Executors) SetExecutorSrc(path string) {
+	es.altExecSrcPath = path
+}
+
 func (es *Executors) Apply(sockPath string, targeter Targeter, env map[string]string, exes envdef.ExecutablePaths) error {
 	logging.Debug("Creating executors at %s, exes: %v", es.executorPath, exes)