mirror of
https://github.com/jesseduffield/lazygit.git
synced 2026-09-10 07:36:27 -04:00
d44c7cfe64
Fixes #5766. UpdateWindowTitle built a `title <repo-name> - 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 <all non-integration packages> -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 "<name> - 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.
75 lines
2.2 KiB
Go
75 lines
2.2 KiB
Go
package oscommands
|
|
|
|
import (
|
|
"fmt"
|
|
"os"
|
|
"os/exec"
|
|
"path/filepath"
|
|
"syscall"
|
|
"unsafe"
|
|
)
|
|
|
|
// 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",
|
|
Shell: "cmd",
|
|
ShellArg: "/c",
|
|
}
|
|
}
|
|
|
|
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
|
|
}
|
|
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 {
|
|
// Signals other than SIGKILL are not supported on Windows
|
|
return nil
|
|
}
|