From 3af0255fe4ed91e0c8fa56f71299828014561c43 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 23 Aug 2026 14:04:42 -0700 Subject: [PATCH] fix(windows): harden interactive account login --- src/cli/handlers/account.test.ts | 56 ++++++------ src/cli/handlers/account.ts | 20 +++-- ...aude-account-service-login-process.test.ts | 3 +- .../claude-windows-interactive-login.test.ts | 52 ++++++++++- src/main/claude-accounts/service.ts | 4 +- .../codex-windows-interactive-login.test.ts | 3 +- src/main/codex-accounts/service.ts | 1 - src/main/win32-utils.test.ts | 10 +++ src/shared/windows-batch-spawn.ts | 14 +-- src/shared/windows-console-input.test.ts | 60 +++++++++++++ src/shared/windows-console-input.ts | 63 +++++++++++++ src/shared/windows-console-stdio.test.ts | 68 -------------- src/shared/windows-console-stdio.ts | 78 ---------------- .../windows-interactive-login-spawn.test.ts | 89 +++++++++---------- src/shared/windows-interactive-login-spawn.ts | 20 +---- 15 files changed, 282 insertions(+), 259 deletions(-) create mode 100644 src/shared/windows-console-input.test.ts create mode 100644 src/shared/windows-console-input.ts delete mode 100644 src/shared/windows-console-stdio.test.ts delete mode 100644 src/shared/windows-console-stdio.ts diff --git a/src/cli/handlers/account.test.ts b/src/cli/handlers/account.test.ts index 5750b663ec6..8916b124fbc 100644 --- a/src/cli/handlers/account.test.ts +++ b/src/cli/handlers/account.test.ts @@ -8,22 +8,20 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' const { deleteKeychainMock, getVersionManagerBinPathsMock, - openWindowsInteractiveStdioMock, readKeychainMock, resolveCliCommandMock, rmSyncMock, spawnMock, + stdioForWindowsInteractiveChildMock, writeKeychainMock } = vi.hoisted(() => ({ deleteKeychainMock: vi.fn(), getVersionManagerBinPathsMock: vi.fn(), - openWindowsInteractiveStdioMock: vi.fn( - (): { stdio: [number, number, number]; dispose: () => void } | 'inherit' => 'inherit' - ), readKeychainMock: vi.fn(), resolveCliCommandMock: vi.fn(), rmSyncMock: vi.fn(), spawnMock: vi.fn(), + stdioForWindowsInteractiveChildMock: vi.fn(), writeKeychainMock: vi.fn() })) @@ -49,21 +47,8 @@ vi.mock('../../shared/node-cli-command-resolution', () => ({ getVersionManagerBinPaths: getVersionManagerBinPathsMock, resolveCliCommand: resolveCliCommandMock })) -vi.mock('../../shared/windows-console-stdio', () => ({ - openWindowsInteractiveStdio: openWindowsInteractiveStdioMock, - stdioForWindowsInteractiveChild: (json: boolean) => { - const opened = openWindowsInteractiveStdioMock() - if (opened === 'inherit') { - return { - stdio: ['inherit', json ? process.stderr : 'inherit', 'inherit'], - dispose: () => {} - } - } - return { - stdio: [opened.stdio[0], json ? process.stderr : opened.stdio[1], opened.stdio[2]], - dispose: opened.dispose - } - } +vi.mock('../../shared/windows-console-input', () => ({ + stdioForWindowsInteractiveChild: stdioForWindowsInteractiveChildMock })) import { ACCOUNT_HANDLERS } from './account' @@ -125,7 +110,10 @@ describe('account CLI handlers', () => { beforeEach(() => { Object.defineProperty(process, 'platform', originalPlatform) spawnMock.mockReset().mockImplementation(() => successfulChild()) - openWindowsInteractiveStdioMock.mockReset().mockReturnValue('inherit') + stdioForWindowsInteractiveChildMock.mockReset().mockImplementation((json: boolean) => ({ + stdio: ['inherit', json ? process.stderr : 'inherit', 'inherit'], + dispose: vi.fn() + })) resolveCliCommandMock.mockReset().mockImplementation((command: string) => command) getVersionManagerBinPathsMock.mockReset().mockReturnValue([]) readKeychainMock.mockReset().mockResolvedValue(null) @@ -183,8 +171,8 @@ describe('account CLI handlers', () => { it('passes Windows console device handles to the login child instead of inheriting Electron stdio', async () => { Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) const dispose = vi.fn() - openWindowsInteractiveStdioMock.mockReturnValue({ - stdio: [11, 12, 12], + stdioForWindowsInteractiveChildMock.mockReturnValue({ + stdio: [11, 'inherit', 'inherit'], dispose }) @@ -193,15 +181,31 @@ describe('account CLI handlers', () => { expect(spawnMock).toHaveBeenCalledWith( 'codex', ['login', '--device-auth'], - expect.objectContaining({ stdio: [11, 12, 12] }) + expect.objectContaining({ stdio: [11, 'inherit', 'inherit'] }) ) expect(dispose).toHaveBeenCalledTimes(1) }) + it('disposes the Windows console input fd when spawn throws synchronously', async () => { + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + const dispose = vi.fn() + stdioForWindowsInteractiveChildMock.mockReturnValue({ + stdio: [11, 'inherit', 'inherit'], + dispose + }) + spawnMock.mockImplementationOnce(() => { + throw new Error('invalid stdio') + }) + + await expect(ACCOUNT_HANDLERS['account add'](context('codex'))).rejects.toThrow('invalid stdio') + + expect(dispose).toHaveBeenCalledOnce() + }) + it('keeps JSON login prompts off the CLI stdout envelope when console fds are attached', async () => { Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) - openWindowsInteractiveStdioMock.mockReturnValue({ - stdio: [11, 12, 12], + stdioForWindowsInteractiveChildMock.mockReturnValue({ + stdio: [11, process.stderr, 'inherit'], dispose: vi.fn() }) @@ -210,7 +214,7 @@ describe('account CLI handlers', () => { expect(spawnMock).toHaveBeenCalledWith( 'codex', ['login', '--device-auth'], - expect.objectContaining({ stdio: [11, process.stderr, 12] }) + expect.objectContaining({ stdio: [11, process.stderr, 'inherit'] }) ) }) diff --git a/src/cli/handlers/account.ts b/src/cli/handlers/account.ts index 8c7eab5afef..4f5a23164a9 100644 --- a/src/cli/handlers/account.ts +++ b/src/cli/handlers/account.ts @@ -21,7 +21,7 @@ import { UnsafeWindowsBatchArgumentsError, WINDOWS_BATCH_UNSAFE_CHARACTERS_LABEL } from '../../shared/windows-batch-spawn' -import { stdioForWindowsInteractiveChild } from '../../shared/windows-console-stdio' +import { stdioForWindowsInteractiveChild } from '../../shared/windows-console-input' import { ACCOUNT_IMPORT_RUNTIME_CAPABILITY } from '../../shared/protocol-version' import type { RuntimeStatus } from '../../shared/runtime-types' import type { @@ -115,13 +115,17 @@ async function runAgentLoginInTerminal( } const env = addAgentNodePaths({ ...stripElectronRunAsNode(process.env), ...extraEnv }) const consoleStdio = stdioForWindowsInteractiveChild(json) - const child = spawn(spawnCmd, spawnArgs, { - // Why: JSON mode reserves stdout for the response envelope while keeping - // the interactive login attached to the user's terminal via stderr. - stdio: consoleStdio.stdio, - env - }) - consoleStdio.dispose() + let child: ReturnType + try { + child = spawn(spawnCmd, spawnArgs, { + // Why: JSON mode reserves stdout for the response envelope while keeping + // the interactive login attached to the user's terminal via stderr. + stdio: consoleStdio.stdio, + env + }) + } finally { + consoleStdio.dispose() + } session.child = child child.once('error', (error) => rejectPromise( diff --git a/src/main/claude-accounts/claude-account-service-login-process.test.ts b/src/main/claude-accounts/claude-account-service-login-process.test.ts index 60095f49e09..e87981b2956 100644 --- a/src/main/claude-accounts/claude-account-service-login-process.test.ts +++ b/src/main/claude-accounts/claude-account-service-login-process.test.ts @@ -521,8 +521,7 @@ describe('ClaudeAccountService credential capture', () => { command: getCmdExePath(), args: ['/d', '/c', 'start', '', '/wait', 'claude', 'auth', 'login', '--claudeai'], stdio: 'ignore' as const, - windowsHide: true, - dispose: vi.fn() + windowsHide: true })) vi.doMock('node:child_process', () => ({ spawn: spawnMock })) vi.doMock('../../shared/windows-interactive-login-spawn', () => ({ diff --git a/src/main/claude-accounts/claude-windows-interactive-login.test.ts b/src/main/claude-accounts/claude-windows-interactive-login.test.ts index 76b28e1862f..cdacdc9e187 100644 --- a/src/main/claude-accounts/claude-windows-interactive-login.test.ts +++ b/src/main/claude-accounts/claude-windows-interactive-login.test.ts @@ -49,8 +49,7 @@ describe('Claude Windows host interactive login', () => { '--claudeai' ], stdio: 'ignore' as const, - windowsHide: true, - dispose: vi.fn() + windowsHide: true })) vi.doMock('node:child_process', () => ({ spawn: spawnMock })) vi.doMock('../../shared/windows-interactive-login-spawn', () => ({ @@ -97,4 +96,53 @@ describe('Claude Windows host interactive login', () => { vi.doUnmock('../../shared/windows-interactive-login-spawn') } }) + + it('times out a denied Windows login that stays alive without piped output', async () => { + setPlatform('win32') + vi.resetModules() + vi.useFakeTimers() + const child = new EventEmitter() as EventEmitter & { + stdout: null + stderr: null + kill: ReturnType + pid: number + } + child.stdout = null + child.stderr = null + child.kill = vi.fn() + child.pid = 0 + const spawnMock = vi.fn(() => child) + vi.doMock('node:child_process', () => ({ spawn: spawnMock })) + + try { + const { ClaudeAccountService } = await import('./service') + const service = new ClaudeAccountService( + createService() as never, + createService() as never, + createService() as never + ) + const login = ( + service as unknown as { + runClaudeCommand( + args: string[], + configDir: { windowsPath: string; linuxPath: string | null; wslDistro: string | null }, + timeoutMs: number + ): Promise + } + ).runClaudeCommand( + ['auth', 'login', '--claudeai'], + { windowsPath: 'C:\\tmp\\claude-auth', linuxPath: null, wslDistro: null }, + 1000 + ) + const rejection = expect(login).rejects.toThrow('Claude sign-in took too long to finish.') + + await vi.advanceTimersByTimeAsync(1000) + + await rejection + expect(child.kill).toHaveBeenCalledOnce() + } finally { + vi.useRealTimers() + vi.doUnmock('node:child_process') + } + }) }) diff --git a/src/main/claude-accounts/service.ts b/src/main/claude-accounts/service.ts index 4e4427337e2..25ea4da5724 100644 --- a/src/main/claude-accounts/service.ts +++ b/src/main/claude-accounts/service.ts @@ -1106,8 +1106,7 @@ export class ClaudeAccountService { ? interactiveLogin.stdio : [options?.keepStdinOpen ? 'pipe' : 'ignore', 'pipe', 'pipe'], shell: spawnConfig.shell, - // Hide noninteractive commands and the fallback wrapper; direct console - // login handles need the owning window to remain visible. + // Why: hide only the outer wrapper; start creates the visible login console. windowsHide: interactiveLogin?.windowsHide ?? true, windowsVerbatimArguments: spawnConfig.windowsVerbatimArguments, env: spawnConfig.env, @@ -1115,7 +1114,6 @@ export class ClaudeAccountService { // A process group lets cancellation terminate the whole POSIX login tree. detached: process.platform !== 'win32' }) - interactiveLogin?.dispose() const stdout = child.stdout const stderr = child.stderr if (!interactiveLogin && (!stdout || !stderr)) { diff --git a/src/main/codex-accounts/codex-windows-interactive-login.test.ts b/src/main/codex-accounts/codex-windows-interactive-login.test.ts index ed2b9a34f06..f9ceb64b4d3 100644 --- a/src/main/codex-accounts/codex-windows-interactive-login.test.ts +++ b/src/main/codex-accounts/codex-windows-interactive-login.test.ts @@ -50,8 +50,7 @@ describe('Codex Windows host interactive login', () => { command: getCmdExePath(), args: ['/d', '/c', 'start', '', '/wait', 'C:\\Tools\\codex.cmd', 'login'], stdio: 'ignore' as const, - windowsHide: true, - dispose: vi.fn() + windowsHide: true })) vi.doMock('node:child_process', () => ({ execFileSync: vi.fn(), diff --git a/src/main/codex-accounts/service.ts b/src/main/codex-accounts/service.ts index 9dbd9f5d20b..d70d2c6185f 100644 --- a/src/main/codex-accounts/service.ts +++ b/src/main/codex-accounts/service.ts @@ -1745,7 +1745,6 @@ export class CodexAccountService { windowsHide: spawnConfig.interactiveLogin?.windowsHide ?? true, env: spawnConfig.env }) - spawnConfig.interactiveLogin?.dispose() let settled = false let output = '' diff --git a/src/main/win32-utils.test.ts b/src/main/win32-utils.test.ts index 72f93b765ab..4fa9436e19e 100644 --- a/src/main/win32-utils.test.ts +++ b/src/main/win32-utils.test.ts @@ -10,6 +10,7 @@ import { isPermissionError, isWindowsBatchScript, resolveWindowsCommand, + UnsafeWindowsBatchArgumentsError, WINDOWS_BATCH_UNSAFE_CHARACTERS_LABEL } from './win32-utils' @@ -151,6 +152,15 @@ describe('getSpawnArgsForWindows', () => { }) }) + it('rejects cmd metacharacters in executable paths passed through start /wait', () => { + expect(() => wrapWindowsStartWait('C:\\Users\\A%B\\codex.exe', ['login'])).toThrow( + UnsafeWindowsBatchArgumentsError + ) + expect(() => wrapWindowsStartWait('C:\\Tools\\codex.exe', ['log&in'])).toThrow( + UnsafeWindowsBatchArgumentsError + ) + }) + it('leaves .exe GUI launches alone even when detachedGui is requested', () => { withPlatform('win32', () => { const { spawnCmd, spawnArgs } = getSpawnArgsForWindows( diff --git a/src/shared/windows-batch-spawn.ts b/src/shared/windows-batch-spawn.ts index 6c5c7e7d496..3abebf3af69 100644 --- a/src/shared/windows-batch-spawn.ts +++ b/src/shared/windows-batch-spawn.ts @@ -39,6 +39,12 @@ function hasUnsafeWindowsBatchSyntax(value: string): boolean { return UNSAFE_WINDOWS_BATCH_SYNTAX.test(value) } +function assertWindowsCmdSafeTokens(values: readonly string[]): void { + if (values.some(hasUnsafeWindowsBatchSyntax)) { + throw new UnsafeWindowsBatchArgumentsError() + } +} + export type GetSpawnArgsForWindowsOptions = { /** * GUI launchers (Open In apps) should not leave a lingering Command Prompt. @@ -58,11 +64,7 @@ export function getSpawnArgsForWindows( options: GetSpawnArgsForWindowsOptions = {} ): { spawnCmd: string; spawnArgs: string[] } { if (isWindowsBatchScript(command)) { - for (const value of [command, ...args]) { - if (hasUnsafeWindowsBatchSyntax(value)) { - throw new UnsafeWindowsBatchArgumentsError() - } - } + assertWindowsCmdSafeTokens([command, ...args]) // Why: separate argv entries let Node quote spaces without breaking cmd. if (options.detachedGui) { @@ -96,6 +98,8 @@ export function wrapWindowsStartWait( spawnCmd: string, spawnArgs: string[] ): { spawnCmd: string; spawnArgs: string[] } { + // Why: `start` reparses every target through cmd.exe, including .exe paths. + assertWindowsCmdSafeTokens([spawnCmd, ...spawnArgs]) const cmdExePath = getCmdExePath() return { spawnCmd: cmdExePath, diff --git a/src/shared/windows-console-input.test.ts b/src/shared/windows-console-input.test.ts new file mode 100644 index 00000000000..9a8b0aeb372 --- /dev/null +++ b/src/shared/windows-console-input.test.ts @@ -0,0 +1,60 @@ +import { describe, expect, it, vi } from 'vitest' +import { + openWindowsConsoleInput, + stdioForWindowsInteractiveChild, + WINDOWS_CONSOLE_INPUT_DEVICE +} from './windows-console-input' + +describe('openWindowsConsoleInput', () => { + it('returns inherit off Windows without opening the device', () => { + const openSync = vi.fn() + expect(openWindowsConsoleInput({ platform: 'darwin', openSync })).toBe('inherit') + expect(openWindowsConsoleInput({ platform: 'linux', openSync })).toBe('inherit') + expect(openSync).not.toHaveBeenCalled() + }) + + it('opens CONIN$ read-write and disposes the fd once', () => { + const openSync = vi.fn(() => 11) + const closeSync = vi.fn() + const opened = openWindowsConsoleInput({ platform: 'win32', openSync, closeSync }) + expect(opened).not.toBe('inherit') + if (opened === 'inherit') { + return + } + expect(openSync).toHaveBeenCalledWith(WINDOWS_CONSOLE_INPUT_DEVICE, 'r+') + expect(opened.fd).toBe(11) + opened.dispose() + opened.dispose() + expect(closeSync).toHaveBeenCalledOnce() + expect(closeSync).toHaveBeenCalledWith(11) + }) + + it('returns inherit when the console device cannot be opened', () => { + const openSync = vi.fn(() => { + throw new Error('no console') + }) + expect(openWindowsConsoleInput({ platform: 'win32', openSync })).toBe('inherit') + }) +}) + +describe('stdioForWindowsInteractiveChild', () => { + it('keeps JSON stdout on the CLI envelope stream', () => { + const { stdio, dispose } = stdioForWindowsInteractiveChild(true, { + platform: 'win32', + openSync: vi.fn(() => 11), + closeSync: vi.fn() + }) + expect(stdio).toEqual([11, process.stderr, 'inherit']) + dispose() + }) + + it('preserves inherited output outside JSON mode', () => { + const { stdio, dispose } = stdioForWindowsInteractiveChild(false, { + platform: 'win32', + openSync: vi.fn(() => 11), + closeSync: vi.fn() + }) + expect(stdio).toEqual([11, 'inherit', 'inherit']) + dispose() + }) +}) diff --git a/src/shared/windows-console-input.ts b/src/shared/windows-console-input.ts new file mode 100644 index 00000000000..0bc84780b5b --- /dev/null +++ b/src/shared/windows-console-input.ts @@ -0,0 +1,63 @@ +import type { StdioOptions } from 'node:child_process' +import { closeSync as fsCloseSync, openSync as fsOpenSync } from 'node:fs' + +export const WINDOWS_CONSOLE_INPUT_DEVICE = '\\\\.\\CONIN$' + +export type WindowsConsoleInput = { + fd: number + dispose: () => void +} + +type OpenWindowsConsoleInputDeps = { + platform?: NodeJS.Platform + openSync?: (path: string, flags: string) => number + closeSync?: (fd: number) => void +} + +/** Opens the real Windows console input device instead of Electron's inherited stdin. */ +export function openWindowsConsoleInput( + deps: OpenWindowsConsoleInputDeps = {} +): WindowsConsoleInput | 'inherit' { + const platform = deps.platform ?? process.platform + if (platform !== 'win32') { + return 'inherit' + } + const openSync = deps.openSync ?? ((path: string, flags: string) => fsOpenSync(path, flags)) + const closeSync = deps.closeSync ?? fsCloseSync + try { + const fd = openSync(WINDOWS_CONSOLE_INPUT_DEVICE, 'r+') + let disposed = false + return { + fd, + dispose: () => { + if (disposed) { + return + } + disposed = true + try { + closeSync(fd) + } catch { + // Descriptor ownership may already have transferred during spawn. + } + } + } + } catch { + return 'inherit' + } +} + +export function stdioForWindowsInteractiveChild( + json: boolean, + deps: OpenWindowsConsoleInputDeps = {} +): { stdio: StdioOptions; dispose: () => void } { + const opened = openWindowsConsoleInput(deps) + return opened === 'inherit' + ? { + stdio: ['inherit', json ? process.stderr : 'inherit', 'inherit'], + dispose: () => {} + } + : { + stdio: [opened.fd, json ? process.stderr : 'inherit', 'inherit'], + dispose: opened.dispose + } +} diff --git a/src/shared/windows-console-stdio.test.ts b/src/shared/windows-console-stdio.test.ts deleted file mode 100644 index bcd68469c9c..00000000000 --- a/src/shared/windows-console-stdio.test.ts +++ /dev/null @@ -1,68 +0,0 @@ -import { describe, expect, it, vi } from 'vitest' -import { - openWindowsInteractiveStdio, - stdioForWindowsInteractiveChild, - WINDOWS_CONSOLE_INPUT_DEVICE, - WINDOWS_CONSOLE_OUTPUT_DEVICE -} from './windows-console-stdio' - -describe('openWindowsInteractiveStdio', () => { - it('returns inherit off Windows without opening devices', () => { - const openSync = vi.fn() - expect(openWindowsInteractiveStdio({ platform: 'darwin', openSync })).toBe('inherit') - expect(openWindowsInteractiveStdio({ platform: 'linux', openSync })).toBe('inherit') - expect(openSync).not.toHaveBeenCalled() - }) - - it('opens CONIN$ and CONOUT$ on win32 and disposes unique fds once', () => { - const openSync = vi.fn((path: string) => (path === WINDOWS_CONSOLE_INPUT_DEVICE ? 11 : 12)) - const closeSync = vi.fn() - const opened = openWindowsInteractiveStdio({ platform: 'win32', openSync, closeSync }) - expect(opened).not.toBe('inherit') - if (opened === 'inherit') { - return - } - expect(openSync).toHaveBeenCalledWith(WINDOWS_CONSOLE_INPUT_DEVICE, 'r+') - expect(openSync).toHaveBeenCalledWith(WINDOWS_CONSOLE_OUTPUT_DEVICE, 'w') - expect(opened.stdio).toEqual([11, 12, 12]) - opened.dispose() - opened.dispose() - expect(closeSync).toHaveBeenCalledTimes(2) - expect(closeSync).toHaveBeenCalledWith(11) - expect(closeSync).toHaveBeenCalledWith(12) - }) - - it('returns inherit and closes a partial CONIN$ open when CONOUT$ fails', () => { - const openSync = vi.fn((path: string) => { - if (path === WINDOWS_CONSOLE_INPUT_DEVICE) { - return 11 - } - throw new Error('no console') - }) - const closeSync = vi.fn() - expect(openWindowsInteractiveStdio({ platform: 'win32', openSync, closeSync })).toBe('inherit') - expect(closeSync).toHaveBeenCalledWith(11) - }) - - it('returns inherit when neither device can be opened', () => { - const openSync = vi.fn(() => { - throw new Error('no console') - }) - const closeSync = vi.fn() - expect(openWindowsInteractiveStdio({ platform: 'win32', openSync, closeSync })).toBe('inherit') - expect(closeSync).not.toHaveBeenCalled() - }) -}) - -describe('stdioForWindowsInteractiveChild', () => { - it('keeps JSON stdout on the CLI envelope stream', () => { - const openSync = vi.fn((path: string) => (path === WINDOWS_CONSOLE_INPUT_DEVICE ? 11 : 12)) - const { stdio, dispose } = stdioForWindowsInteractiveChild(true, { - platform: 'win32', - openSync, - closeSync: vi.fn() - }) - expect(stdio).toEqual([11, process.stderr, 12]) - dispose() - }) -}) diff --git a/src/shared/windows-console-stdio.ts b/src/shared/windows-console-stdio.ts deleted file mode 100644 index 7ca5c3ca1e4..00000000000 --- a/src/shared/windows-console-stdio.ts +++ /dev/null @@ -1,78 +0,0 @@ -import { closeSync as fsCloseSync, openSync as fsOpenSync } from 'node:fs' -import type { StdioOptions } from 'node:child_process' - -export const WINDOWS_CONSOLE_INPUT_DEVICE = '\\\\.\\CONIN$' -export const WINDOWS_CONSOLE_OUTPUT_DEVICE = '\\\\.\\CONOUT$' - -export type WindowsInteractiveStdio = { - stdio: [number, number, number] - dispose: () => void -} - -type OpenWindowsInteractiveStdioDeps = { - platform?: NodeJS.Platform - openSync?: (path: string, flags: string) => number - closeSync?: (fd: number) => void -} - -function closeUniqueFds(fds: number[], close: typeof fsCloseSync): void { - for (const fd of new Set(fds)) { - try { - close(fd) - } catch { - // Already closed by a previous dispose or a failed partial open. - } - } -} - -/** - * Opens the Windows console devices so an interactive child of a GUI-subsystem - * Electron CLI can actually read typed input. Do not trust `stdin.isTTY`. - */ -export function openWindowsInteractiveStdio( - deps: OpenWindowsInteractiveStdioDeps = {} -): WindowsInteractiveStdio | 'inherit' { - const platform = deps.platform ?? process.platform - if (platform !== 'win32') { - return 'inherit' - } - const openSync = deps.openSync ?? ((path: string, flags: string) => fsOpenSync(path, flags)) - const closeSync = deps.closeSync ?? fsCloseSync - const opened: number[] = [] - try { - opened.push(openSync(WINDOWS_CONSOLE_INPUT_DEVICE, 'r+')) - opened.push(openSync(WINDOWS_CONSOLE_OUTPUT_DEVICE, 'w')) - const [stdin, stdout] = opened - let disposed = false - return { - stdio: [stdin, stdout, stdout], - dispose: () => { - if (disposed) { - return - } - disposed = true - closeUniqueFds(opened, closeSync) - } - } - } catch { - closeUniqueFds(opened, closeSync) - return 'inherit' - } -} - -export function stdioForWindowsInteractiveChild( - json: boolean, - deps: OpenWindowsInteractiveStdioDeps = {} -): { stdio: StdioOptions; dispose: () => void } { - const opened = openWindowsInteractiveStdio(deps) - if (opened === 'inherit') { - return { - stdio: ['inherit', json ? process.stderr : 'inherit', 'inherit'], - dispose: () => {} - } - } - return { - stdio: [opened.stdio[0], json ? process.stderr : opened.stdio[1], opened.stdio[2]], - dispose: opened.dispose - } -} diff --git a/src/shared/windows-interactive-login-spawn.test.ts b/src/shared/windows-interactive-login-spawn.test.ts index 91a8f196e1f..3573d107760 100644 --- a/src/shared/windows-interactive-login-spawn.test.ts +++ b/src/shared/windows-interactive-login-spawn.test.ts @@ -1,57 +1,52 @@ -import { describe, expect, it, vi } from 'vitest' +import { describe, expect, it } from 'vitest' import { getCmdExePath } from './windows-batch-spawn' import { buildWindowsHostInteractiveLoginSpawn } from './windows-interactive-login-spawn' +function withWindows(fn: () => T): T { + const platform = Object.getOwnPropertyDescriptor(process, 'platform')! + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + try { + return fn() + } finally { + Object.defineProperty(process, 'platform', platform) + } +} + describe('buildWindowsHostInteractiveLoginSpawn', () => { - const originalPlatform = Object.getOwnPropertyDescriptor(process, 'platform')! - - it('passes console device fds when CONIN$ opens', () => { - Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) - try { - const dispose = vi.fn() - const spawn = buildWindowsHostInteractiveLoginSpawn('claude.exe', ['auth', 'login'], () => ({ - stdio: [11, 12, 12], - dispose - })) - expect(spawn).toEqual({ - command: 'claude.exe', - args: ['auth', 'login'], - stdio: [11, 12, 12], - windowsHide: false, - dispose - }) - } finally { - Object.defineProperty(process, 'platform', originalPlatform) - } - }) - - it('falls back to a hidden start /wait wrapper when no console exists', () => { - Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) - try { - const spawn = buildWindowsHostInteractiveLoginSpawn( - 'C:\\Tools\\claude.cmd', - ['auth', 'login', '--claudeai'], - () => 'inherit' - ) - expect(spawn.command).toBe(getCmdExePath()) - expect(spawn.args).toEqual([ - '/d', - '/c', - 'start', - '', - '/wait', - getCmdExePath(), - '/d', - '/c', - 'C:\\Tools\\claude.cmd', + it('launches a batch login in a visible console and waits for it', () => { + const spawn = withWindows(() => + buildWindowsHostInteractiveLoginSpawn('C:\\Tools\\claude.cmd', [ 'auth', 'login', '--claudeai' ]) - expect(spawn.stdio).toBe('ignore') - expect(spawn.windowsHide).toBe(true) - } finally { - Object.defineProperty(process, 'platform', originalPlatform) - } + ) + expect(spawn.command).toBe(getCmdExePath()) + expect(spawn.args).toEqual([ + '/d', + '/c', + 'start', + '', + '/wait', + getCmdExePath(), + '/d', + '/c', + 'C:\\Tools\\claude.cmd', + 'auth', + 'login', + '--claudeai' + ]) + expect(spawn.stdio).toBe('ignore') + expect(spawn.windowsHide).toBe(true) + }) + + it('routes executable logins through the same waiting console boundary', () => { + const spawn = buildWindowsHostInteractiveLoginSpawn('C:\\Tools\\codex.exe', ['login']) + expect(spawn).toEqual({ + command: getCmdExePath(), + args: ['/d', '/c', 'start', '', '/wait', 'C:\\Tools\\codex.exe', 'login'], + stdio: 'ignore', + windowsHide: true + }) }) }) diff --git a/src/shared/windows-interactive-login-spawn.ts b/src/shared/windows-interactive-login-spawn.ts index 39615e7bc54..63612b98c57 100644 --- a/src/shared/windows-interactive-login-spawn.ts +++ b/src/shared/windows-interactive-login-spawn.ts @@ -1,36 +1,22 @@ import { getSpawnArgsForWindows, wrapWindowsStartWait } from './windows-batch-spawn' -import { openWindowsInteractiveStdio, type WindowsInteractiveStdio } from './windows-console-stdio' export type WindowsHostInteractiveLoginSpawn = { command: string args: string[] - stdio: WindowsInteractiveStdio['stdio'] | 'ignore' + stdio: 'ignore' windowsHide: boolean - dispose: () => void } export function buildWindowsHostInteractiveLoginSpawn( command: string, - args: string[], - openStdio: typeof openWindowsInteractiveStdio = openWindowsInteractiveStdio + args: string[] ): WindowsHostInteractiveLoginSpawn { const { spawnCmd, spawnArgs } = getSpawnArgsForWindows(command, args) - const opened = openStdio() - if (opened !== 'inherit') { - return { - command: spawnCmd, - args: spawnArgs, - stdio: opened.stdio, - windowsHide: false, - dispose: opened.dispose - } - } const wrapped = wrapWindowsStartWait(spawnCmd, spawnArgs) return { command: wrapped.spawnCmd, args: wrapped.spawnArgs, stdio: 'ignore', - windowsHide: true, - dispose: () => {} + windowsHide: true } }