diff --git a/pkg/commands/oscommands/os_windows.go b/pkg/commands/oscommands/os_windows.go index d0252f5b1b5..91207352201 100644 --- a/pkg/commands/oscommands/os_windows.go +++ b/pkg/commands/oscommands/os_windows.go @@ -6,6 +6,7 @@ import ( "os/exec" "path/filepath" "syscall" + "unsafe" ) // setRawCmdLine hands cmd.exe the exact command line we built, bypassing @@ -32,13 +33,39 @@ func GetPlatform() *Platform { } } +var ( + kernel32 = syscall.NewLazyDLL("kernel32.dll") + procSetConsoleTitle = kernel32.NewProc("SetConsoleTitleW") +) + +// UpdateWindowTitle sets the console window title directly via the +// SetConsoleTitleW Win32 API instead of shelling out to `cmd /c title ...`. +// +// The repo name is attacker/user-controlled input (it's just the current +// directory's basename), and cmd.exe treats characters such as & as command +// separators. A directory named e.g. "test&aaa" would previously be split +// into two commands ("title test" and "aaa"), and cmd.exe would then try to +// run "aaa" as a program, printing an error to stderr that crashed the run +// (see #5766). Calling the Win32 API directly sidesteps cmd.exe's argument +// parsing entirely: the title string is passed as-is, verbatim, regardless +// of what characters it contains. func (c *OSCommand) UpdateWindowTitle() error { path, getWdErr := os.Getwd() if getWdErr != nil { return getWdErr } - argString := fmt.Sprint("title ", filepath.Base(path), " - Lazygit") - return c.Cmd.NewShell(argString, c.UserConfig().OS.ShellFunctionsFile).Run() + title := fmt.Sprint(filepath.Base(path), " - Lazygit") + + titlePtr, err := syscall.UTF16PtrFromString(title) + if err != nil { + return err + } + + r1, _, callErr := procSetConsoleTitle.Call(uintptr(unsafe.Pointer(titlePtr))) + if r1 == 0 { + return callErr + } + return nil } func TerminateProcessGracefully(proc *os.Process) error { diff --git a/pkg/commands/oscommands/os_windows_test.go b/pkg/commands/oscommands/os_windows_test.go index 495e23f7257..71cfcd2b768 100644 --- a/pkg/commands/oscommands/os_windows_test.go +++ b/pkg/commands/oscommands/os_windows_test.go @@ -1,7 +1,12 @@ package oscommands import ( + "os" + "path/filepath" + "strings" + "syscall" "testing" + "unsafe" "github.com/go-errors/errors" "github.com/stretchr/testify/assert" @@ -73,3 +78,80 @@ func TestOSCommandOpenFileWindows(t *testing.T) { s.test(oSCmd.OpenFile(s.filename)) } } + +var procGetConsoleTitle = kernel32.NewProc("GetConsoleTitleW") + +// getConsoleTitle reads back the current console window title via the +// Win32 GetConsoleTitleW API, mirroring the SetConsoleTitleW call that +// UpdateWindowTitle makes. +func getConsoleTitle(t *testing.T) string { + t.Helper() + buf := make([]uint16, 1024) + r1, _, err := procGetConsoleTitle.Call( + uintptr(unsafe.Pointer(&buf[0])), + uintptr(len(buf)), + ) + if r1 == 0 { + t.Skipf("GetConsoleTitleW unavailable in this environment (no attached console?): %v", err) + } + return syscall.UTF16ToString(buf[:r1]) +} + +// UpdateWindowTitle previously shelled out to `cmd /c title - Lazygit`, +// which crashed lazygit whenever the current directory's basename contained +// a cmd.exe metacharacter such as & (see #5766: "test&aaa" was split into +// two commands, and cmd.exe tried to run "aaa" as a program). Calling +// SetConsoleTitleW directly bypasses cmd.exe's parsing, so the title is set +// verbatim regardless of what characters the directory name contains. +func TestUpdateWindowTitle_NameWithAmpersand(t *testing.T) { + origWd, err := os.Getwd() + if err != nil { + t.Fatalf("os.Getwd: %v", err) + } + defer func() { _ = os.Chdir(origWd) }() + + dir := t.TempDir() + // t.TempDir() names are safe; nest a directory whose name reproduces + // the exact character that crashed cmd.exe in #5766. + problematic := filepath.Join(dir, "test&aaa") + if err := os.Mkdir(problematic, 0o755); err != nil { + t.Fatalf("os.Mkdir: %v", err) + } + if err := os.Chdir(problematic); err != nil { + t.Fatalf("os.Chdir: %v", err) + } + + osCommand := NewDummyOSCommand() + err = osCommand.UpdateWindowTitle() + assert.NoError(t, err, "UpdateWindowTitle must not error out for directory names containing '&'") + + title := getConsoleTitle(t) + assert.True(t, strings.HasPrefix(title, "test&aaa"), + "expected console title to start with the literal directory name %q, got %q", "test&aaa", title) + assert.Contains(t, title, "Lazygit") +} + +func TestUpdateWindowTitle_PlainName(t *testing.T) { + origWd, err := os.Getwd() + if err != nil { + t.Fatalf("os.Getwd: %v", err) + } + defer func() { _ = os.Chdir(origWd) }() + + dir := t.TempDir() + plain := filepath.Join(dir, "plain-repo") + if err := os.Mkdir(plain, 0o755); err != nil { + t.Fatalf("os.Mkdir: %v", err) + } + if err := os.Chdir(plain); err != nil { + t.Fatalf("os.Chdir: %v", err) + } + + osCommand := NewDummyOSCommand() + if err := osCommand.UpdateWindowTitle(); err != nil { + t.Fatalf("UpdateWindowTitle: %v", err) + } + + title := getConsoleTitle(t) + assert.Equal(t, "plain-repo - Lazygit", title) +}