Skip to content

fix(windows): set console title via SetConsoleTitleW instead of shelling out to cmd /c title#5850

Open
myUdav4iik wants to merge 1 commit into
jesseduffield:masterfrom
myUdav4iik:fix/console-title-ampersand-crash
Open

fix(windows): set console title via SetConsoleTitleW instead of shelling out to cmd /c title#5850
myUdav4iik wants to merge 1 commit into
jesseduffield:masterfrom
myUdav4iik:fix/console-title-ampersand-crash

Conversation

@myUdav4iik

Copy link
Copy Markdown

What / Why

Fixes #5766: lazygit crashes when the current folder's name contains & (e.g. Rapports&Notes, test&aaa), whether launched directly in such a folder or opened via the recent-repos menu.

UpdateWindowTitle built a title <repo-name> - Lazygit string and ran it through cmd /c "title ...". cmd.exe treats & as a command separator, so "test&aaa" gets split into two commands: title test and aaa. cmd.exe then tries to execute aaa as a program, fails, and that failure propagates up as an uncaught error that crashes the whole run (matching the exact error message and stack trace in the report).

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 and set verbatim, regardless of what characters the directory name contains. No shell, no quoting rules, no injection surface for this code path.

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 execute 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 CI matrix already includes windows-latest, so the two new tests below will run for real there.

New tests in pkg/commands/oscommands/os_windows_test.go (windows-only, build-tagged, same file the existing TestOSCommandOpenFileWindows lives in):

  • TestUpdateWindowTitle_NameWithAmpersand - chdirs into a directory literally named test&aaa (the exact string from the report), asserts UpdateWindowTitle no longer errors, then reads the title back with GetConsoleTitleW and asserts it's the unsplit, literal directory name. This reproduces Crashes when folder/repo has a "&" in the name #5766 and proves it's fixed.
  • TestUpdateWindowTitle_PlainName - sanity check that the ordinary case (no special characters) still produces "<name> - Lazygit".

Both new tests skip gracefully rather than fail if GetConsoleTitleW is unavailable in the CI sandbox (e.g. no attached console), so they can't produce a false failure unrelated to this change.

Fixes #5766

…ing out to `cmd /c title`

Fixes jesseduffield#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 jesseduffield#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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crashes when folder/repo has a "&" in the name

1 participant