fix(auth): update browser command arguments for Windows and clean URL…

package.json

@@ -31,5 +31,8 @@
     "husky": "9.1.7",
     "neostandard": "0.13.0",
     "tsx": "4.22.4"
+  },
+  "dependencies": {
+    "mcp-remote": "0.1.38"
   }
 }

packages/core/src/auth/auth-manager.ts

@@ -1,5 +1,6 @@
 import { randomUUID } from 'node:crypto'
 import { execFile } from 'node:child_process'
+import { win32 } from 'node:path'
 import type { AuthConfig, Credentials, Logger, AuthConfirmation, HarnessType } from '../types.js'
 import { loadCredentials, saveCredentials, isExpired } from './token-storage.js'
 import { validateToken } from './token-validator.js'
@@ -8,17 +9,35 @@ import { PermissionError, InvalidCredentialsError } from './errors.js'
 import { formatPluginError, toPluginError } from '../errors.js'
 import { deriveMcpUrlFromConsoleUrl } from './mcp-url.js'
 
-function openBrowser (url: string, logger?: Logger): void {
+export function openBrowser (url: string, logger?: Logger): void {
   try {
     // eslint-disable-next-line no-new
     new URL(url)
   } catch {
     logger?.warn('auth.openBrowser.invalidUrl', { url })
     return
   }
-  const cmd = process.platform === 'darwin' ? 'open' : process.platform === 'win32' ? 'cmd' : 'xdg-open'
-  const args = process.platform === 'win32' ? ['/c', 'start', url] : [url]
-  execFile(cmd, args, (err) => {
+  if (process.platform === 'win32') {
+    // Use ShellExecute directly instead of cmd /c start to avoid & being
+    // interpreted as a command separator on Windows. Resolve rundll32 from
+    // the Windows directory explicitly: a bare executable name can otherwise
+    // be found in the current working directory before the system directory.
+    const systemRoot = process.env.SystemRoot ?? process.env.WINDIR
+    // win32.isAbsolute accepts root-relative paths such as `\\Windows`, which
+    // would still depend on the current drive. Only accept a drive-qualified
+    // or UNC system root.
+    if (!systemRoot || !/^(?:[a-zA-Z]:[\\/]|\\\\[^\\/]+[\\/][^\\/]+)/.test(systemRoot)) {
+      logger?.warn('auth.openBrowser.failed', { error: 'Windows system root is unavailable' })
+      return
+    }
+    const rundll32 = win32.join(systemRoot, 'System32', 'rundll32.exe')
+    execFile(rundll32, ['url.dll,FileProtocolHandler', url], (err) => {
+      if (err) logger?.warn('auth.openBrowser.failed', { error: err.message })
+    })
+    return
+  }
+  const cmd = process.platform === 'darwin' ? 'open' : 'xdg-open'
+  execFile(cmd, [url], (err) => {
     if (err) logger?.warn('auth.openBrowser.failed', { error: err.message })
   })
 }

packages/core/test/integration/auth/auth-manager.test.ts

@@ -28,9 +28,14 @@ const authConfig: AuthConfig = {
 }
 
 function getUrlFromExecFileCall (): URL {
-  const args = execFileCalls[execFileCalls.length - 1][1] as string[]
-  const urlStr = args.find((a: string) => a.startsWith('http'))!
-  return new URL(urlStr)
+  const call = execFileCalls[execFileCalls.length - 1]
+  const cmd = call[0] as string
+  const args = call[1] as string[]
+  const urlStr = cmd === 'rundll32'
+    ? args[args.length - 1]
+    : args.find((a: string) => a.startsWith('http') || a.startsWith('"http'))!
+  const cleaned = urlStr.replace(/^"|"$/g, '')
+  return new URL(cleaned)
 }
 
 function getStateFromExecFileCall (): string {
@@ -510,9 +515,11 @@ describe('ensureAuthenticated - requiredPermissions', () => {
 })
 
 describe('ensureAuthenticated - Windows browser launch', () => {
-  it('uses cmd /c start on Windows', { timeout: 10000 }, async () => {
+  it('uses the trusted rundll32 path with url.dll,FileProtocolHandler on Windows', { timeout: 10000 }, async () => {
     const originalPlatform = process.platform
+    const originalSystemRoot = process.env.SystemRoot
     Object.defineProperty(process, 'platform', { value: 'win32' })
+    process.env.SystemRoot = 'C:\\Windows'
 
     try {
       const { saveCredentials } = await import('../../../src/auth/token-storage.js')
@@ -539,16 +546,18 @@ describe('ensureAuthenticated - Windows browser launch', () => {
       await new Promise((resolve) => setTimeout(resolve, 50))
 
       const lastCall = execFileCalls[execFileCalls.length - 1]
-      assert.strictEqual(lastCall[0], 'cmd')
+      assert.strictEqual(lastCall[0], 'C:\\Windows\\System32\\rundll32.exe')
       const lastArgs = lastCall[1] as string[]
-      assert.ok(lastArgs.includes('/c'))
-      assert.ok(lastArgs.includes('start'))
+      assert.strictEqual(lastArgs[0], 'url.dll,FileProtocolHandler')
+      assert.ok(lastArgs[1].startsWith('https://accounts.example.com/sign-in'))
 
       const state = getStateFromExecFileCall()
       await sendCallback(8767, state)
       await promise
     } finally {
       Object.defineProperty(process, 'platform', { value: originalPlatform })
+      if (originalSystemRoot === undefined) delete process.env.SystemRoot
+      else process.env.SystemRoot = originalSystemRoot
     }
   })
 })

packages/core/test/unit/auth/open-browser.test.ts

@@ -0,0 +1,159 @@
+import { describe, it, beforeEach, afterEach } from 'node:test'
+import assert from 'node:assert/strict'
+import { createRequire, syncBuiltinESMExports } from 'node:module'
+
+const require = createRequire(import.meta.url)
+const cp = require('node:child_process')
+let moduleLoadId = 0
+
+async function loadOpenBrowser () {
+  return await import(`../../../src/auth/auth-manager.js?open-browser-test=${moduleLoadId++}`)
+}
+
+describe('openBrowser', () => {
+  let originalPlatform: PropertyDescriptor | undefined
+  let execFileCalls: unknown[][]
+  let originalExecFile: typeof cp.execFile
+  let originalSystemRoot: string | undefined
+  let originalWindir: string | undefined
+
+  beforeEach(() => {
+    execFileCalls = []
+    originalExecFile = cp.execFile
+    cp.execFile = (...args: unknown[]) => {
+      execFileCalls.push(args)
+    }
+    syncBuiltinESMExports()
+    originalPlatform = Object.getOwnPropertyDescriptor(process, 'platform')
+    originalSystemRoot = process.env.SystemRoot
+    originalWindir = process.env.WINDIR
+  })
+
+  afterEach(() => {
+    cp.execFile = originalExecFile
+    syncBuiltinESMExports()
+    if (originalPlatform) {
+      Object.defineProperty(process, 'platform', originalPlatform)
+    }
+    if (originalSystemRoot === undefined) delete process.env.SystemRoot
+    else process.env.SystemRoot = originalSystemRoot
+    if (originalWindir === undefined) delete process.env.WINDIR
+    else process.env.WINDIR = originalWindir
+  })
+
+  it('opens the URL with the trusted rundll32 path on Windows', async () => {
+    Object.defineProperty(process, 'platform', { value: 'win32' })
+    process.env.SystemRoot = 'C:\\Windows'
+
+    const { openBrowser } = await loadOpenBrowser()
+    const url = 'https://accounts.example.com/sign-in?extension=nsolid-plugin&port=8765&state=abc'
+    openBrowser(url)
+
+    assert.strictEqual(execFileCalls.length, 1)
+    const [cmd, args] = execFileCalls[0] as [string, string[]]
+    assert.strictEqual(cmd, 'C:\\Windows\\System32\\rundll32.exe')
+    assert.deepStrictEqual(args.slice(0, -1), ['url.dll,FileProtocolHandler'])
+    assert.strictEqual(args[args.length - 1], url)
+  })
+
+  it('does not launch a relative rundll32 when Windows system root is unavailable', async () => {
+    Object.defineProperty(process, 'platform', { value: 'win32' })
+    delete process.env.SystemRoot
+    delete process.env.WINDIR
+
+    const warnings: unknown[][] = []
+    const logger = { warn: (...args: unknown[]) => warnings.push(args) }
+    const { openBrowser } = await loadOpenBrowser()
+    openBrowser('https://accounts.example.com/sign-in', logger as never)
+
+    assert.strictEqual(execFileCalls.length, 0)
+    assert.deepStrictEqual(warnings, [['auth.openBrowser.failed', {
+      error: 'Windows system root is unavailable',
+    }]])
+  })
+
+  it('does not use a root-relative Windows system directory', async () => {
+    Object.defineProperty(process, 'platform', { value: 'win32' })
+    process.env.SystemRoot = '\\Windows'
+
+    const warnings: unknown[][] = []
+    const logger = { warn: (...args: unknown[]) => warnings.push(args) }
+    const { openBrowser } = await loadOpenBrowser()
+    openBrowser('https://accounts.example.com/sign-in', logger as never)
+
+    assert.strictEqual(execFileCalls.length, 0)
+    assert.deepStrictEqual(warnings, [['auth.openBrowser.failed', {
+      error: 'Windows system root is unavailable',
+    }]])
+  })
+
+  it('uses WINDIR when SystemRoot is unavailable', async () => {
+    Object.defineProperty(process, 'platform', { value: 'win32' })
+    delete process.env.SystemRoot
+    process.env.WINDIR = 'D:\\Windows'
+
+    const { openBrowser } = await loadOpenBrowser()
+    openBrowser('https://accounts.example.com/sign-in')
+
+    const [cmd, args] = execFileCalls[0] as [string, string[]]
+    assert.strictEqual(cmd, 'D:\\Windows\\System32\\rundll32.exe')
+    assert.deepStrictEqual(args, ['url.dll,FileProtocolHandler', 'https://accounts.example.com/sign-in'])
+  })
+
+  it('preserves a Windows URL as one exact argument and reports spawn errors', async () => {
+    Object.defineProperty(process, 'platform', { value: 'win32' })
+    process.env.SystemRoot = 'C:\\Windows'
+    cp.execFile = (...args: unknown[]) => {
+      execFileCalls.push(args)
+      const callback = args[2] as (err: Error | null) => void
+      callback(new Error('browser launch failed'))
+    }
+    syncBuiltinESMExports()
+
+    const warnings: unknown[][] = []
+    const logger = { warn: (...args: unknown[]) => warnings.push(args) }
+    const { openBrowser } = await loadOpenBrowser()
+    const url = 'https://accounts.example.com/sign-in?next=a%2Fb&value=one%20two&emoji=%F0%9F%9A%80'
+    openBrowser(url, logger as never)
+
+    const [cmd, args] = execFileCalls[0] as [string, string[]]
+    assert.strictEqual(cmd, 'C:\\Windows\\System32\\rundll32.exe')
+    assert.deepStrictEqual(args, ['url.dll,FileProtocolHandler', url])
+    assert.deepStrictEqual(warnings, [['auth.openBrowser.failed', { error: 'browser launch failed' }]])
+  })
+
+  it('opens the URL with open on macOS', async () => {
+    Object.defineProperty(process, 'platform', { value: 'darwin' })
+
+    const { openBrowser } = await loadOpenBrowser()
+    const url = 'https://accounts.example.com/sign-in?extension=nsolid-plugin&port=8765&state=abc'
+    openBrowser(url)
+
+    assert.strictEqual(execFileCalls.length, 1)
+    const [cmd, args] = execFileCalls[0] as [string, string[]]
+    assert.strictEqual(cmd, 'open')
+    assert.deepStrictEqual(args, [url])
+  })
+
+  it('opens the URL with xdg-open on Linux', async () => {
+    Object.defineProperty(process, 'platform', { value: 'linux' })
+
+    const { openBrowser } = await loadOpenBrowser()
+    const url = 'https://accounts.example.com/sign-in?extension=nsolid-plugin&port=8765&state=abc'
+    openBrowser(url)
+
+    assert.strictEqual(execFileCalls.length, 1)
+    const [cmd, args] = execFileCalls[0] as [string, string[]]
+    assert.strictEqual(cmd, 'xdg-open')
+    assert.deepStrictEqual(args, [url])
+  })
+
+  it('does nothing for an invalid URL', async () => {
+    Object.defineProperty(process, 'platform', { value: 'win32' })
+
+    const { openBrowser } = await loadOpenBrowser()
+    openBrowser('not a url')
+
+    assert.strictEqual(execFileCalls.length, 0)
+  })
+})