From 2c13cfed5dd7048312db5a756a983837a0a1302d Mon Sep 17 00:00:00 2001 From: Max Hsu Date: Thu, 2 Jul 2026 07:48:41 +0800 Subject: [PATCH 1/2] fix(login): avoid cmd.exe when opening the login URL on Windows (#1114) openBrowser() launches the device-login verification URL returned by the login server. On Windows it ran the URL through `cmd /c start`, whose parser (and the `start` builtin) re-interprets shell metacharacters (& | ^ "). A malicious or compromised login server (or a CODEGRAPH_LOGIN_URL override) could embed such characters to inject commands. Launch via `rundll32 url.dll,FileProtocolHandler ` instead, which receives the URL as a single opaque argument with no shell re-parsing. macOS (open) and Linux (xdg-open) already pass the URL as a lone argv element and are unaffected. --- __tests__/open-browser-security.test.ts | 42 +++++++++++++++++++++++++ src/reasoning/login.ts | 4 ++- 2 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 __tests__/open-browser-security.test.ts diff --git a/__tests__/open-browser-security.test.ts b/__tests__/open-browser-security.test.ts new file mode 100644 index 000000000..66b7e3732 --- /dev/null +++ b/__tests__/open-browser-security.test.ts @@ -0,0 +1,42 @@ +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); + }); +}); diff --git a/src/reasoning/login.ts b/src/reasoning/login.ts index 70faf1857..1b687bfed 100644 --- a/src/reasoning/login.ts +++ b/src/reasoning/login.ts @@ -77,7 +77,9 @@ export async function pollForToken(deviceCode: string, intervalSec: number, expi export async function openBrowser(url: string): Promise { 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 }); From ff7ad336e317f67ebad33cfefa38f3b582b3d493 Mon Sep 17 00:00:00 2001 From: Max Hsu Date: Thu, 2 Jul 2026 08:59:39 +0800 Subject: [PATCH 2/2] fix(login): refuse non-http(s) URLs before launching the browser (#1114) --- __tests__/open-browser-security.test.ts | 24 ++++++++++++++++++++++++ src/reasoning/login.ts | 3 +++ 2 files changed, 27 insertions(+) diff --git a/__tests__/open-browser-security.test.ts b/__tests__/open-browser-security.test.ts index 66b7e3732..5885d2b52 100644 --- a/__tests__/open-browser-security.test.ts +++ b/__tests__/open-browser-security.test.ts @@ -38,5 +38,29 @@ describe('openBrowser — Windows shell-injection hardening (#1114)', () => { // 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); }); }); diff --git a/src/reasoning/login.ts b/src/reasoning/login.ts index 1b687bfed..5d1dbaade 100644 --- a/src/reasoning/login.ts +++ b/src/reasoning/login.ts @@ -75,6 +75,9 @@ 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 { + // 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]] // Avoid `cmd /c start`: cmd.exe re-parses shell metacharacters (& | ^ ") in the