fix(login): avoid cmd.exe when opening the login URL on Windows (#1114)

__tests__/open-browser-security.test.ts

@@ -0,0 +1,66 @@
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+
+// openBrowser() launches a URL that originates from the device-login server
+// response (verification_uri_complete). On Windows the URL must never be handed
+// to cmd.exe, which re-parses shell metacharacters (& | ^ ") and would allow a
+// malicious/compromised login server to inject commands. See issue #1114.
+const { spawnMock } = vi.hoisted(() => ({ spawnMock: vi.fn() }));
+vi.mock('child_process', () => ({ spawn: spawnMock }));
+
+import { openBrowser } from '../src/reasoning/login';
+
+describe('openBrowser — Windows shell-injection hardening (#1114)', () => {
+  const realPlatform = process.platform;
+
+  beforeEach(() => {
+    spawnMock.mockReset();
+    spawnMock.mockReturnValue({ on: vi.fn(), unref: vi.fn() });
+    Object.defineProperty(process, 'platform', { value: 'win32', configurable: true });
+  });
+
+  afterEach(() => {
+    Object.defineProperty(process, 'platform', { value: realPlatform, configurable: true });
+  });
+
+  it('does not launch the URL through cmd.exe on Windows', async () => {
+    const url = 'https://app.getcodegraph.com/device?user_code=ABCD-1234&calc';
+
+    await openBrowser(url);
+
+    expect(spawnMock).toHaveBeenCalledTimes(1);
+    const [command, args] = spawnMock.mock.calls[0] as [string, string[]];
+
+    // cmd.exe (and its `start` builtin) re-parse metacharacters, so the launcher
+    // must not be cmd and must not carry cmd's `/c` switch.
+    expect(command.toLowerCase()).not.toContain('cmd');
+    expect(args).not.toContain('/c');
+
+    // The server-supplied URL is passed to the launcher as one intact argument,
+    // never concatenated into a shell command string.
+    expect(args).toContain(url);
+
+    // And never through a shell at all.
+    const options = spawnMock.mock.calls[0][2] as { shell?: boolean } | undefined;
+    expect(options?.shell).toBeFalsy();
+  });
+
+  it.each([
+    ['file:///C:/Windows/System32/calc.exe'],
+    ['\\\\evil.example\\share\\payload.exe'],
+    ['C:\\Windows\\System32\\calc.exe'],
+    ['javascript:alert(1)'],
+  ])('refuses to launch a non-http(s) target: %s', async (target) => {
+    // rundll32 url.dll,FileProtocolHandler (like `open` / `xdg-open`) hands the
+    // string to the OS handler, which happily executes file/UNC paths — so the
+    // server-supplied string must be scheme-checked before launching anything.
+    await openBrowser(target);
+
+    expect(spawnMock).not.toHaveBeenCalled();
+  });
+
+  it('still launches plain http:// URLs', async () => {
+    await openBrowser('http://localhost:8080/device?user_code=ABCD-1234');
+
+    expect(spawnMock).toHaveBeenCalledTimes(1);
+  });
+});

src/reasoning/login.ts

@@ -75,9 +75,14 @@ export async function pollForToken(deviceCode: string, intervalSec: number, expi
 
 /** Best-effort: open a URL in the default browser. Never throws — the URL is also printed. */
 export async function openBrowser(url: string): Promise<void> {
+  // The OS handlers below (open / rundll32 / xdg-open) execute file and UNC
+  // paths as readily as they open URLs — never hand them a non-http(s) string.
+  if (!/^https?:\/\//i.test(url)) return;
   const [cmd, args] =
     process.platform === 'darwin' ? ['open', [url]]
-    : process.platform === 'win32' ? ['cmd', ['/c', 'start', '', url]]
+    // Avoid `cmd /c start`: cmd.exe re-parses shell metacharacters (& | ^ ") in the
+    // server-supplied URL. rundll32 receives the URL as a single, opaque argument.
+    : process.platform === 'win32' ? ['rundll32', ['url.dll,FileProtocolHandler', url]]
     : ['xdg-open', [url]];
   try {
     const child = spawn(cmd as string, args as string[], { stdio: 'ignore', detached: true, windowsHide: true });