Fix quoting of shell commands on Windows

lazygit builds a shell command by interpolating Quote'd arguments into a
template and running the result via `cmd /c`. Several things were wrong
on Windows:

- Quote emitted bash-style `\"…\"` quoting, which cmd.exe doesn't
  understand. Making it usable at all previously required a fragile
  round-trip through str.ToArgv and re-escaping.
- The assembled command line was handed to `cmd /c` without `/s`, so
  cmd's default rules stripped the wrong quotes once the line contained
  more than two of them (e.g. a quoted editor path at a location with
  spaces, plus a quoted filename that also contains spaces).
- Shell metacharacters were escaped with `^` (`&` → `^&`, etc.), which
  neutralised command chaining, pipes, redirection and `%VAR%` expansion
  in custom commands.

Quote now emits the standard Windows convention directly, and NewShell
hands cmd.exe the fully-assembled line verbatim via SysProcAttr.CmdLine,
wrapped as `cmd /s /c "<command>"`. The /s flag strips exactly the outer
quote pair we add, leaving each argument's own quoting intact. With the
`^` escaping gone, metacharacters in a custom command reach cmd as the
author intended; this also removes the spurious `^` reported in #3092.

Fixes #5560
Fixes #2427
Fixes #4147

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

pkg/commands/oscommands/cmd_obj_builder.go

@@ -48,26 +48,34 @@ func (self *CmdObjBuilder) NewShell(commandStr string, shellFunctionsFile string
 	if len(shellFunctionsFile) > 0 {
 		commandStr = fmt.Sprintf("%ssource %s\n%s", self.platform.PrefixForShellFunctionsFile, shellFunctionsFile, commandStr)
 	}
-	quotedCommand := self.quotedCommandString(commandStr)
+
+	if self.platform.OS == "windows" {
+		return self.newWindowsShell(commandStr)
+	}
+
+	quotedCommand := self.Quote(commandStr)
 	cmdArgs := str.ToArgv(fmt.Sprintf("%s %s %s", self.platform.Shell, self.platform.ShellArg, quotedCommand))
 
 	return self.New(cmdArgs)
 }
 
-func (self *CmdObjBuilder) quotedCommandString(commandStr string) string {
-	// Windows does not seem to like quotes around the command
-	if self.platform.OS == "windows" {
-		return strings.NewReplacer(
-			"^", "^^",
-			"&", "^&",
-			"|", "^|",
-			"<", "^<",
-			">", "^>",
-			"%", "^%",
-		).Replace(commandStr)
-	}
+// newWindowsShell wraps the command in `cmd.exe /s /c "<command>"`. The /s
+// flag tells cmd to strip exactly the outermost pair of quotes and pass the
+// rest through unchanged, which preserves any quoting the command itself
+// contains (e.g. `"C:\Program Files\my-editor.exe" file.txt`). Without /s,
+// cmd's default rules drop the wrong quotes once the command line contains
+// more than two of them.
+//
+// We bypass Go's standard arg quoting via SysProcAttr.CmdLine: it follows the
+// CommandLineToArgvW convention (`\"` for inner quotes), but cmd.exe doesn't.
+func (self *CmdObjBuilder) newWindowsShell(commandStr string) *CmdObj {
+	args := []string{self.platform.Shell, "/s", self.platform.ShellArg, commandStr}
+	cmdObj := self.New(args)
 
-	return self.Quote(commandStr)
+	cmdLine := fmt.Sprintf(`%s /s %s "%s"`, self.platform.Shell, self.platform.ShellArg, commandStr)
+	setRawCmdLine(cmdObj.GetCmd(), cmdLine)
+
+	return cmdObj
 }
 
 func (self *CmdObjBuilder) CloneWithNewRunner(decorate func(ICmdObjRunner) ICmdObjRunner) *CmdObjBuilder {
@@ -80,21 +88,47 @@ func (self *CmdObjBuilder) CloneWithNewRunner(decorate func(ICmdObjRunner) ICmdO
 }
 
 func (self *CmdObjBuilder) Quote(message string) string {
-	var quote string
 	if self.platform.OS == "windows" {
-		quote = `\"`
-		message = strings.NewReplacer(
-			`"`, `"'"'"`,
-			`\"`, `\\"`,
-		).Replace(message)
-	} else {
-		quote = `"`
-		message = strings.NewReplacer(
-			`\`, `\\`,
-			`"`, `\"`,
-			`$`, `\$`,
-			"`", "\\`",
-		).Replace(message)
+		return quoteForWindows(message)
 	}
-	return quote + message + quote
+	message = strings.NewReplacer(
+		`\`, `\\`,
+		`"`, `\"`,
+		`$`, `\$`,
+		"`", "\\`",
+	).Replace(message)
+	return `"` + message + `"`
+}
+
+// quoteForWindows encodes a value using the standard Windows command-line
+// convention (the algorithm behind syscall.EscapeArg, reimplemented here so
+// it's available on all platforms). The result is always wrapped in double
+// quotes so cmd.exe and CommandLineToArgvW treat it as a single argument
+// regardless of what shell metacharacters it contains.
+func quoteForWindows(s string) string {
+	var b strings.Builder
+	b.WriteByte('"')
+	slashes := 0
+	for i := range len(s) {
+		c := s[i]
+		switch c {
+		case '\\':
+			slashes++
+			b.WriteByte(c)
+		case '"':
+			for ; slashes > 0; slashes-- {
+				b.WriteByte('\\')
+			}
+			b.WriteByte('\\')
+			b.WriteByte(c)
+		default:
+			slashes = 0
+			b.WriteByte(c)
+		}
+	}
+	for ; slashes > 0; slashes-- {
+		b.WriteByte('\\')
+	}
+	b.WriteByte('"')
+	return b.String()
 }

pkg/commands/oscommands/os_default_platform.go

@@ -40,6 +40,11 @@ func (c *OSCommand) UpdateWindowTitle() error {
 	return nil
 }
 
+// setRawCmdLine is the non-Windows no-op counterpart of the Windows shim
+// (see the comment there). NewShell's shell-building logic is portable, so
+// this call is reached on every host; only the Windows build does anything.
+func setRawCmdLine(cmd *exec.Cmd, cmdLine string) {}
+
 func TerminateProcessGracefully(cmd *exec.Cmd) error {
 	if cmd.Process == nil {
 		return nil

pkg/commands/oscommands/os_test.go

@@ -75,10 +75,7 @@ func TestOSCommandQuoteWindows(t *testing.T) {
 
 	actual := osCommand.Quote(`hello "test" 'test2'`)
 
-	/* EXPECTED:
 	expected := `"hello \"test\" 'test2'"`
-	ACTUAL: */
-	expected := `\"hello "'"'"test"'"'" 'test2'\"`
 
 	assert.EqualValues(t, expected, actual)
 }
@@ -93,10 +90,7 @@ func TestNewShellWindowsPassesMetacharactersVerbatim(t *testing.T) {
 	command := `echo a && echo b | sort > out.txt < in.txt %PATH%`
 
 	assert.Equal(t,
-		/* EXPECTED:
 		[]string{"cmd", "/s", "/c", command},
-		ACTUAL: */
-		[]string{"cmd", "/c", "echo", "a", "^&^&", "echo", "b", "^|", "sort", "^>", "out.txt", "^<", "in.txt", "^%PATH^%"},
 		osCommand.Cmd.NewShell(command, "").Args(),
 	)
 }

pkg/commands/oscommands/os_windows.go

@@ -5,8 +5,25 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"syscall"
 )
 
+// setRawCmdLine hands cmd.exe the exact command line we built, bypassing
+// os/exec's default composition (which quotes args with the
+// CommandLineToArgvW `\"` convention that cmd.exe doesn't understand).
+//
+// The shell-building logic in NewShell is portable and dispatches on
+// platform.OS, which keeps it (and its quoting) unit-testable on any host.
+// Assigning SysProcAttr.CmdLine is the only step that needs a Windows-only
+// field, so it's the single piece split out behind a build tag; every other
+// platform gets the no-op in os_default_platform.go.
+func setRawCmdLine(cmd *exec.Cmd, cmdLine string) {
+	if cmd.SysProcAttr == nil {
+		cmd.SysProcAttr = &syscall.SysProcAttr{}
+	}
+	cmd.SysProcAttr.CmdLine = cmdLine
+}
+
 func GetPlatform() *Platform {
 	return &Platform{
 		OS:       "windows",

pkg/commands/oscommands/os_windows_test.go

@@ -20,7 +20,7 @@ func TestOSCommandOpenFileWindows(t *testing.T) {
 		{
 			filename: "test",
 			runner: NewFakeRunner(t).
-				ExpectArgs([]string{"cmd", "/c", "start", "", "test"}, "", errors.New("error")),
+				ExpectArgs([]string{"cmd", "/s", "/c", `start "" "test"`}, "", errors.New("error")),
 			test: func(err error) {
 				assert.Error(t, err)
 			},
@@ -28,7 +28,7 @@ func TestOSCommandOpenFileWindows(t *testing.T) {
 		{
 			filename: "test",
 			runner: NewFakeRunner(t).
-				ExpectArgs([]string{"cmd", "/c", "start", "", "test"}, "", nil),
+				ExpectArgs([]string{"cmd", "/s", "/c", `start "" "test"`}, "", nil),
 			test: func(err error) {
 				assert.NoError(t, err)
 			},
@@ -36,7 +36,7 @@ func TestOSCommandOpenFileWindows(t *testing.T) {
 		{
 			filename: "filename with spaces",
 			runner: NewFakeRunner(t).
-				ExpectArgs([]string{"cmd", "/c", "start", "", "filename with spaces"}, "", nil),
+				ExpectArgs([]string{"cmd", "/s", "/c", `start "" "filename with spaces"`}, "", nil),
 			test: func(err error) {
 				assert.NoError(t, err)
 			},
@@ -44,7 +44,7 @@ func TestOSCommandOpenFileWindows(t *testing.T) {
 		{
 			filename: "let's_test_with_single_quote",
 			runner: NewFakeRunner(t).
-				ExpectArgs([]string{"cmd", "/c", "start", "", "let's_test_with_single_quote"}, "", nil),
+				ExpectArgs([]string{"cmd", "/s", "/c", `start "" "let's_test_with_single_quote"`}, "", nil),
 			test: func(err error) {
 				assert.NoError(t, err)
 			},
@@ -52,7 +52,7 @@ func TestOSCommandOpenFileWindows(t *testing.T) {
 		{
 			filename: "$USER.txt",
 			runner: NewFakeRunner(t).
-				ExpectArgs([]string{"cmd", "/c", "start", "", "$USER.txt"}, "", nil),
+				ExpectArgs([]string{"cmd", "/s", "/c", `start "" "$USER.txt"`}, "", nil),
 			test: func(err error) {
 				assert.NoError(t, err)
 			},