Skip to content

Commit 2c5b529

Browse files
gtrrz-victorclaude
andcommitted
login: use explorer.exe for the WSL no-wslview browser fallback
cmd.exe `/c start` reparses its command line, so the `&` separators in an OAuth authorization URL split it into multiple commands and open a truncated URL (WSL interop only quotes argv entries containing whitespace). explorer.exe takes the URL as a plain argument and hands it to the shell URL handler: no reparsing, no escaping needed, and no UNC working-directory warning — which also removes the /mnt/c cmd.Dir workaround and the dir return value from resolveBrowserLauncher. explorer.exe's exit code is meaningless (1 even on success); openBrowser only checks process start, and a comment now guards that invariant. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 5391daf commit 2c5b529

2 files changed

Lines changed: 21 additions & 30 deletions

File tree

cmd/entire/cli/login.go

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -595,21 +595,12 @@ func openBrowser(ctx context.Context, browserURL string) error {
595595
return errors.New("browser unavailable under test")
596596
}
597597

598-
command, args, dir, err := resolveBrowserLauncher(runtime.GOOS, isWSL(), exec.LookPath, browserURL)
598+
command, args, err := resolveBrowserLauncher(runtime.GOOS, isWSL(), exec.LookPath, browserURL)
599599
if err != nil {
600600
return err
601601
}
602602

603603
cmd := exec.CommandContext(ctx, command, args...)
604-
// dir is set only for the WSL cmd.exe fallback, to keep cmd.exe from
605-
// warning about the unsupported WSL (UNC) working directory. It's
606-
// best-effort: a customized automount root means /mnt/c may not exist, and
607-
// suppressing a cosmetic warning must not become a hard chdir failure.
608-
if dir != "" {
609-
if _, statErr := os.Stat(dir); statErr == nil {
610-
cmd.Dir = dir
611-
}
612-
}
613604
if err := cmd.Start(); err != nil {
614605
return fmt.Errorf("start browser command %q: %w", command, err)
615606
}
@@ -631,24 +622,28 @@ func openBrowser(ctx context.Context, browserURL string) error {
631622
// both $BROWSER and the xdg default, so the user lands in a browser with none
632623
// of their sessions. So on WSL we open the Windows default browser directly
633624
// via wslview (from wslu, preinstalled on Ubuntu-on-WSL), falling back to
634-
// cmd.exe on stripped-down distros without it. The cmd.exe working directory
635-
// is a Windows path so cmd doesn't warn about the unsupported WSL (UNC) cwd.
636-
func resolveBrowserLauncher(goos string, wsl bool, lookPath func(string) (string, error), browserURL string) (command string, args []string, dir string, err error) {
625+
// explorer.exe on stripped-down distros without it. explorer.exe takes the
626+
// URL as a plain argument and hands it to the shell URL handler, so unlike
627+
// cmd.exe `start` it never reparses the command line (`&` in OAuth query
628+
// strings would split a cmd.exe line) and doesn't warn about the WSL (UNC)
629+
// working directory. Its exit code is meaningless (1 even on success), which
630+
// is fine here: openBrowser only checks that the process starts.
631+
func resolveBrowserLauncher(goos string, wsl bool, lookPath func(string) (string, error), browserURL string) (command string, args []string, err error) {
637632
switch goos {
638633
case "darwin":
639-
return "open", []string{browserURL}, "", nil
634+
return "open", []string{browserURL}, nil
640635
case "windows":
641-
return "cmd", []string{"/c", "start", "", browserURL}, "", nil
636+
return "cmd", []string{"/c", "start", "", browserURL}, nil
642637
case "linux":
643638
if wsl {
644639
if path, lerr := lookPath("wslview"); lerr == nil {
645-
return path, []string{browserURL}, "", nil
640+
return path, []string{browserURL}, nil
646641
}
647-
return "cmd.exe", []string{"/c", "start", "", browserURL}, "/mnt/c", nil
642+
return "explorer.exe", []string{browserURL}, nil
648643
}
649-
return "xdg-open", []string{browserURL}, "", nil
644+
return "xdg-open", []string{browserURL}, nil
650645
default:
651-
return "", nil, "", fmt.Errorf("unsupported platform %s", goos)
646+
return "", nil, fmt.Errorf("unsupported platform %s", goos)
652647
}
653648
}
654649

cmd/entire/cli/login_test.go

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -748,20 +748,19 @@ func TestResolveBrowserLauncher(t *testing.T) {
748748
look func(string) (string, error)
749749
wantCmd string
750750
wantArgs []string
751-
wantDir string
752751
wantErr bool
753752
}{
754-
{"darwin", "darwin", false, missing, "open", []string{browserURL}, "", false},
755-
{"windows", "windows", false, missing, "cmd", []string{"/c", "start", "", browserURL}, "", false},
756-
{"linux-non-wsl", "linux", false, found, "xdg-open", []string{browserURL}, "", false},
757-
{"wsl-with-wslview", "linux", true, found, "/usr/bin/wslview", []string{browserURL}, "", false},
758-
{"wsl-without-wslview", "linux", true, missing, "cmd.exe", []string{"/c", "start", "", browserURL}, "/mnt/c", false},
759-
{"unsupported", "plan9", false, missing, "", nil, "", true},
753+
{"darwin", "darwin", false, missing, "open", []string{browserURL}, false},
754+
{"windows", "windows", false, missing, "cmd", []string{"/c", "start", "", browserURL}, false},
755+
{"linux-non-wsl", "linux", false, found, "xdg-open", []string{browserURL}, false},
756+
{"wsl-with-wslview", "linux", true, found, "/usr/bin/wslview", []string{browserURL}, false},
757+
{"wsl-without-wslview", "linux", true, missing, "explorer.exe", []string{browserURL}, false},
758+
{"unsupported", "plan9", false, missing, "", nil, true},
760759
}
761760
for _, tc := range cases {
762761
t.Run(tc.name, func(t *testing.T) {
763762
t.Parallel()
764-
cmd, args, dir, err := resolveBrowserLauncher(tc.goos, tc.wsl, tc.look, browserURL)
763+
cmd, args, err := resolveBrowserLauncher(tc.goos, tc.wsl, tc.look, browserURL)
765764
if tc.wantErr {
766765
if err == nil {
767766
t.Fatalf("expected error, got cmd=%q", cmd)
@@ -777,9 +776,6 @@ func TestResolveBrowserLauncher(t *testing.T) {
777776
if !reflect.DeepEqual(args, tc.wantArgs) {
778777
t.Errorf("args = %v, want %v", args, tc.wantArgs)
779778
}
780-
if dir != tc.wantDir {
781-
t.Errorf("dir = %q, want %q", dir, tc.wantDir)
782-
}
783779
})
784780
}
785781
}

0 commit comments

Comments
 (0)