fix(auth): caret-escape cmd metacharacters when opening the browser on WSL (#61)

scottlovegrove · claude · web-flow · commit 9abcfea502ba · 2026-06-05T15:13:50.000+01:00
* fix(auth): caret-escape cmd metacharacters when opening the browser on WSL

`openViaCmdExe` wrapped the URL in double quotes to stop `&amp;` from splitting the
`cmd.exe /c start` command line. Under WSL that doesn't work: the interop layer
re-quotes argv entries itself, mangling the literal quotes, so `&amp;` leaks through
and acts as a statement separator — only the prefix up to the first `&amp;` reaches
`start`, and Windows tries to open that fragment as a path ("Windows cannot find
'\https://…'"). OAuth authorize URLs (several `&amp;`, percent-encoded params) hit
this every time.

Two compounding bugs:
- The surrounding quotes don't survive WSL interop, so `&amp;` was never protected.
- `url.replaceAll('%','%%')` does not collapse on the `cmd /c` command line (only
  inside batch files), so it would have corrupted every `%HH` byte even when the
  command did run.

Fix: caret-escape cmd's metacharacters (`&amp; | &lt; &gt; ^ ( ) "`) — which survives
interop since there are no quotes to mangle — and leave `%` alone (OAuth URLs are
`%HH`, which never matches `%NAME%` env-expansion). Extracted as a testable
`escapeUrlForCmd`; verified the round-trip through a real `cmd.exe` on WSL.

Co-Authored-By: Claude Opus 4.8 (1M context) &lt;noreply@anthropic.com&gt;

* fix(auth): open the WSL browser via rundll32 instead of cmd.exe

Addresses review: caret-escaping cmd metacharacters still left `%` exposed —
a percent-encoded multi-byte UTF-8 byte like `%C3%A9` (é) contains `%C3%`, which
`cmd /c` would treat as `%VAR%` env-expansion and mangle. Since runOAuthFlow is
public API for custom AuthProviders, that's a real hazard, not just a built-in
concern.

Bypass the shell entirely: launch via `rundll32.exe url.dll,FileProtocolHandler
&lt;url&gt;`. CreateProcess hands the single argv to the protocol handler verbatim —
no cmd parsing pass — so neither `&amp;` (statement separator) nor `%HH`
(env-expansion) can corrupt the URL, and there's no fragile escaping to
maintain. Drops the now-unneeded escapeUrlForCmd helper.

Co-Authored-By: Claude Opus 4.8 (1M context) &lt;noreply@anthropic.com&gt;

---------

Co-authored-by: Claude Opus 4.8 (1M context) &lt;noreply@anthropic.com&gt;
diff --git a/src/auth/flow.test.ts b/src/auth/flow.test.ts
@@ -488,13 +488,11 @@ describe('runOAuthFlow default opener selection', () => {
         Object.defineProperty(process, 'platform', { value, configurable: true })
     }
 
-    it('routes WSL via cmd.exe with the URL quoted and `%` doubled', async () => {
-        // Two escapes are load-bearing for cmd.exe:
-        //   1. quote the URL so `&` doesn't split the command line
-        //   2. double `%` so cmd.exe doesn't try to expand percent-encoded
-        //      OAuth params (`%3A`, `%2F`, …) as env-var references.
-        // Use an authorize URL with both `&` and `%` so the assertion
-        // exercises both escapes.
+    it('routes WSL via rundll32, passing the URL verbatim (no shell, no escaping)', async () => {
+        // rundll32 is not a shell, so the URL is handed to the protocol handler
+        // as-is: `&` can't split the command and `%HH` (incl. multi-byte UTF-8
+        // like `%C3%A9`) can't be mistaken for `%VAR%` env-expansion. The URL
+        // here carries `&`, ASCII `%HH`, and an encoded `é` to lock that in.
         stubPlatform('linux')
         vi.mocked(readFileSync).mockReturnValue('Linux 5.15 #1 SMP microsoft-WSL2')
         const execFileMock = vi.mocked(execFile)
@@ -510,7 +508,7 @@ describe('runOAuthFlow default opener selection', () => {
 
         const { provider, getRedirect } = instrument({
             authorize: async (input) => ({
-                authorizeUrl: `https://example.com/oauth/authorize?state=${input.state}&redirect_uri=http%3A%2F%2Flocalhost%3A8080`,
+                authorizeUrl: `https://example.com/oauth/authorize?state=${input.state}&redirect_uri=http%3A%2F%2Flocalhost%3A8080&name=Andr%C3%A9`,
                 handshake: { codeVerifier: 'v1' },
             }),
         })
@@ -522,11 +520,11 @@ describe('runOAuthFlow default opener selection', () => {
         expect(execFileMock).toHaveBeenCalledTimes(1)
         const [cmd, args] = execFileMock.mock.calls[0]
         const url = onAuthorizeUrl.mock.calls[0][0] as string
-        expect(cmd).toBe('cmd.exe')
-        expect(args).toEqual(['/c', 'start', '""', `"${url.replaceAll('%', '%%')}"`])
-        // Sanity: the URL we built does contain both special chars.
-        expect(url).toMatch(/&/)
-        expect(url).toMatch(/%/)
+        expect(cmd).toBe('rundll32.exe')
+        // URL is passed verbatim — no quoting, no caret-escaping, no `%%`.
+        expect(args).toEqual(['url.dll,FileProtocolHandler', url])
+        expect(url).toContain('&')
+        expect(url).toContain('%C3%A9')
     })
 
     it('skips the default opener entirely on headless Linux', async () => {
diff --git a/src/auth/flow.ts b/src/auth/flow.ts
@@ -484,9 +484,9 @@ async function openOrFallback(
 
 async function loadDefaultOpener(): Promise<((url: string) => Promise<void>) | null> {
     // WSL check must run before the headless check: WSL is `platform === 'linux'`
-    // and often has no DISPLAY, but `cmd.exe` does work and reaches the user's
-    // real Windows browser, so it's worth the spawn.
-    if (isWsl()) return openViaCmdExe
+    // and often has no DISPLAY, but the Windows interop opener does work and
+    // reaches the user's real Windows browser, so it's worth the spawn.
+    if (isWsl()) return openViaWindows
     if (isHeadlessLinux()) return null
     try {
         const mod = (await import('open')) as { default: (url: string) => Promise<unknown> }
@@ -500,20 +500,21 @@ async function loadDefaultOpener(): Promise<((url: string) => Promise<void>) | n
 
 const execFileAsync = promisify(execFile)
 
-// Two layers of escaping are needed because `cmd.exe /c` is a shell:
-//   1. Wrap the URL in literal double quotes. WSL interop only auto-quotes
-//      args that contain spaces; an OAuth URL (no spaces, plenty of `&`s)
-//      would otherwise be re-parsed by cmd.exe with `&` acting as a
-//      statement separator, so only the prefix up to the first `&` would
-//      reach `start`.
-//   2. Double every `%` to `%%`. cmd.exe expands `%NAME%` even inside
-//      quoted strings; OAuth URLs are full of percent-encoded bytes
-//      (`%3A`, `%2F`, …) and a chance match against a defined env var
-//      (`%PATH%`, `%TEMP%`, …) would silently mangle the URL.
-// `start ""` — the empty title arg is mandatory; otherwise `start` consumes
-// the URL as a window title and never launches a browser. (`execFile`'s
-// no-shell guarantee doesn't apply when the target is itself a shell.)
-async function openViaCmdExe(url: string): Promise<void> {
-    const escaped = url.replaceAll('%', '%%')
-    await execFileAsync('cmd.exe', ['/c', 'start', '""', `"${escaped}"`], { windowsHide: true })
+// Launch the Windows default browser through rundll32's URL protocol handler.
+// Crucially this is NOT a shell, so the URL is passed as a single argv that
+// `CreateProcess` hands to the handler verbatim — there is no `cmd.exe` parsing
+// pass, so `&` can't split the command and percent-encoded bytes (including
+// multi-byte UTF-8 like `%C3%A9` for `é`) can't be mistaken for `%VAR%`
+// env-expansion.
+//
+// The previous `cmd.exe /c start "" "<url>"` approach was doubly broken on WSL:
+// its literal double-quotes don't survive WSL interop's own argv re-quoting (so
+// `&` leaked through and Windows opened the truncated fragment as a path —
+// "Windows cannot find '\https://…'"), and `%`→`%%` doubling doesn't collapse on
+// the `cmd /c` command line, so it corrupted every `%HH` byte. Avoiding the
+// shell sidesteps both, with no fragile escaping to maintain.
+async function openViaWindows(url: string): Promise<void> {
+    await execFileAsync('rundll32.exe', ['url.dll,FileProtocolHandler', url], {
+        windowsHide: true,
+    })
 }
