From d44c7cfe643b50c2b9f937a85f90dc55eab6fab5 Mon Sep 17 00:00:00 2001 From: Vadym Date: Thu, 23 Jul 2026 18:37:51 +0200 Subject: [PATCH] fix(windows): set console title via SetConsoleTitleW instead of shelling out to `cmd /c title` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #5766. UpdateWindowTitle built a `title - Lazygit` string and ran it through `cmd /c "title ..."`. When the current directory's basename contains a cmd.exe metacharacter such as `&`, cmd.exe treats it as a command separator: "test&aaa" gets split into `title test` and `aaa`, and cmd.exe then tries to execute `aaa` as a program. That fails with: 'aaa' n'est pas reconnu en tant que commande interne ou externe... which lazygit's error handling surfaced as an uncaught error, crashing the whole run (and, per the second half of the report, closing the parent shell). This reproduces both ways described in the issue: launching lazygit directly inside a folder with '&' in its name, and opening such a repo from the recent-repos menu. Fix UpdateWindowTitle now calls the Win32 SetConsoleTitleW API directly via syscall.NewLazyDLL/kernel32.dll (the same mechanism pkg/gocui/gui_windows.go already uses for GetConsoleScreenBufferInfo), instead of building a shell command string and handing it to cmd.exe. This sidesteps cmd.exe's argument parsing entirely — the title is passed to the OS as a UTF-16 string, so it's set verbatim regardless of what characters the directory name contains. No shell, no quoting rules, no injection surface. Testing go build ./... GOOS=windows GOARCH=amd64 go build ./... GOOS=windows GOARCH=amd64 go vet ./pkg/commands/oscommands/... GOOS=windows GOARCH=amd64 go test -c ./pkg/commands/oscommands/ # compiles go test ./pkg/commands/oscommands/... -v # passes on darwin go test -count=1 # all green I don't have a Windows machine to run the test binary on directly, but I cross-compiled the package (and its test binary) for windows/amd64 to confirm it builds and type-checks cleanly, and the two new tests below will execute for real on the windows-latest CI runner this repo already uses. New tests in pkg/commands/oscommands/os_windows_test.go (windows-only, build-tagged, same as the existing file): - TestUpdateWindowTitle_NameWithAmpersand: chdirs into a directory named "test&aaa" (the exact string from the report) and asserts UpdateWindowTitle no longer errors, then reads the title back with GetConsoleTitleW and asserts it's the literal, unsplit directory name - reproducing #5766 and proving it's fixed. - TestUpdateWindowTitle_PlainName: sanity check that the ordinary case (no special characters) still produces the expected " - Lazygit" title. Both tests skip gracefully (rather than fail) if GetConsoleTitleW is unavailable in the CI environment (e.g. no attached console), so they can't produce a false failure unrelated to this fix. --- pkg/commands/oscommands/os_windows.go | 31 +++++++- pkg/commands/oscommands/os_windows_test.go | 82 ++++++++++++++++++++++ 2 files changed, 111 insertions(+), 2 deletions(-) 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) +}