Skip to content

Commit 948de4e

Browse files
committed
fix(login): open the browser on Windows without mangling the auth URL
rundll32 url.dll,FileProtocolHandler strips/percent-encodes a URL's query string on Windows (the ? becomes %3F), so `memcode login` opened /api/cli/auth?port=&state= as /api/cli/auth%3Fport=... which 404s. This was Windows-only; macOS/Linux (open/xdg-open) pass the URL verbatim. Switch to `cmd /c start`, caret-escaping & so cmd doesn't treat it as a command separator and drop &state=. Factored into windowsBrowserCmd with a behavioral test asserting both query params survive and the ? is never re-encoded (final sign-off still wants a real Windows run). Also point DefaultWebAppURL at www.memcode.ai directly to skip the apex->www 308 hop on this load-bearing flow.
1 parent db1f5d1 commit 948de4e

2 files changed

Lines changed: 75 additions & 3 deletions

File tree

internal/authflow/authflow.go

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,11 @@ import (
2929
// this default lets login work even when the callback doesn't include it.
3030
const DefaultGatewayURL = provider.DefaultAPIURL
3131

32-
// DefaultWebAppURL is where the browser opens to authenticate.
33-
const DefaultWebAppURL = "https://memcode.ai"
32+
// DefaultWebAppURL is where the browser opens to authenticate. Use the www
33+
// host directly: the apex (memcode.ai) 308-redirects to www, and that extra
34+
// cross-host hop on the /api/cli/auth?port=&state= URL is needless churn on a
35+
// load-bearing flow. Point straight at the canonical host.
36+
const DefaultWebAppURL = "https://www.memcode.ai"
3437

3538
// Result is a successful login: the minted org key and the gateway to use it
3639
// against.
@@ -279,7 +282,8 @@ func openBrowser(url string) error {
279282
case "darwin":
280283
return exec.Command("open", url).Start()
281284
case "windows":
282-
return exec.Command("rundll32", "url.dll,FileProtocolHandler", url).Start()
285+
prog, args := windowsBrowserCmd(url)
286+
return exec.Command(prog, args...).Start()
283287
default: // linux, *bsd
284288
for _, cmd := range []string{"xdg-open", "wslview", "gio"} {
285289
if _, err := exec.LookPath(cmd); err == nil {
@@ -290,6 +294,27 @@ func openBrowser(url string) error {
290294
}
291295
}
292296

297+
// windowsBrowserCmd builds the argv that opens url in the default browser on
298+
// Windows. It is split out from openBrowser so the shell quoting — the part
299+
// that actually broke — is unit-testable off a Windows host.
300+
//
301+
// It deliberately does NOT use `rundll32 url.dll,FileProtocolHandler`: that
302+
// strips/percent-encodes the query string (the ? became %3F), so
303+
//
304+
// /api/cli/auth?port=&state=
305+
//
306+
// reached the browser as /api/cli/auth%3Fport=… and 404'd — the Windows-only
307+
// login failure. `cmd /c start` preserves the URL verbatim, with two caveats:
308+
// - & is a cmd command separator, so every & is caret-escaped (^&); cmd
309+
// collapses ^& back to a literal & before the browser sees it. (Go does not
310+
// quote this arg — no spaces — so the caret survives to cmd unquoted, which
311+
// is exactly where it must be to take effect.)
312+
// - the empty "" is start's title argument, so a URL that ever ends up quoted
313+
// is not mistaken for the window title.
314+
func windowsBrowserCmd(url string) (string, []string) {
315+
return "cmd", []string{"/c", "start", "", strings.ReplaceAll(url, "&", "^&")}
316+
}
317+
293318
// SetGlobalEnv writes key=value pairs into the global env file
294319
// (~/.config/memcode/.env), replacing any existing lines for those keys and
295320
// leaving everything else intact. The wizard uses it to persist a credential

internal/authflow/browser_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
package authflow
2+
3+
import (
4+
"strings"
5+
"testing"
6+
)
7+
8+
// The Windows-only login failure was `rundll32 url.dll,FileProtocolHandler`
9+
// mangling the auth URL's ? into %3F. windowsBrowserCmd replaced it with
10+
// `cmd /c start`, which reintroduces a different hazard: cmd treats & as a
11+
// command separator, so the &state= half of the query can be lost. This test
12+
// verifies the argv is built so BOTH query params survive and the ? is never
13+
// re-encoded. (OS-level `start` behavior still needs a real Windows box; this
14+
// pins the quoting logic that we can reason about deterministically.)
15+
func TestWindowsBrowserCmdPreservesFullQuery(t *testing.T) {
16+
const url = "https://www.memcode.ai/api/cli/auth?port=54321&state=abc123"
17+
18+
prog, args := windowsBrowserCmd(url)
19+
20+
if prog != "cmd" {
21+
t.Fatalf("prog = %q, want cmd", prog)
22+
}
23+
if len(args) != 4 || args[0] != "/c" || args[1] != "start" || args[2] != "" {
24+
t.Fatalf("args = %v, want [/c start \"\" <url>]", args)
25+
}
26+
27+
got := args[3] // the URL argument as cmd will receive it on the command line
28+
29+
if strings.Contains(got, "%3F") {
30+
t.Fatalf("the ? was percent-encoded to %%3F (the original bug): %q", got)
31+
}
32+
if !strings.Contains(got, "/api/cli/auth?port=") {
33+
t.Fatalf("the ? was not preserved literally: %q", got)
34+
}
35+
36+
// Every & must be caret-escaped, or cmd reads it as a command separator and
37+
// drops everything after it (losing &state=).
38+
if strings.Contains(strings.ReplaceAll(got, "^&", ""), "&") {
39+
t.Fatalf("found an unescaped & — cmd would truncate the URL there: %q", got)
40+
}
41+
42+
// cmd collapses ^& back to & before launching the browser. Simulate that and
43+
// assert we recover the EXACT original URL — including &state=abc123.
44+
if recovered := strings.ReplaceAll(got, "^&", "&"); recovered != url {
45+
t.Fatalf("browser would receive %q, want %q", recovered, url)
46+
}
47+
}

0 commit comments

Comments
 (0)