diff --git a/src/codex/app-server-processes.ts b/src/codex/app-server-processes.ts index 93594d028..d0d9963a8 100644 --- a/src/codex/app-server-processes.ts +++ b/src/codex/app-server-processes.ts @@ -10,6 +10,7 @@ import { execFileSync } from "node:child_process"; import { existsSync, readdirSync, readFileSync, statSync } from "node:fs"; import { isProcessAlive, waitForExit } from "../lib/process-control"; +import { resolveTrustedWindowsPowerShellExe } from "../lib/windows-elevation"; import { readCodexCatalogPath } from "./catalog/parsing"; export const STALE_CODEX_APP_SERVER_HINT = @@ -345,8 +346,10 @@ export function listWindowsSnapshots(): ProcessSnapshot[] { " } catch { \"__OCX_ENUM_INCOMPLETE__\" }", "}", ].join("\n"); - // Top-level exec failure propagates (see listDarwinSnapshots note). - const output = execFileSync("powershell.exe", [ + // Top-level exec failure propagates (see listDarwinSnapshots note). The + // executable resolves from the trusted System32 directory (never PATH), and + // windowsHide keeps the enumeration console-less on desktop sessions (#1278). + const output = execFileSync(resolveTrustedWindowsPowerShellExe(), [ "-NoProfile", "-NoLogo", "-NonInteractive", "-WindowStyle", "Hidden", "-Command", psCommand, @@ -457,7 +460,7 @@ function readDarwinProcStartMs(pid: number): number | null { /** Win32_Process.CreationDate → epoch ms, or null (Windows). */ function readWindowsProcStartMs(pid: number): number | null { try { - const out = execFileSync("powershell.exe", [ + const out = execFileSync(resolveTrustedWindowsPowerShellExe(), [ "-NoProfile", "-NoLogo", "-NonInteractive", "-WindowStyle", "Hidden", "-Command", `(Get-CimInstance Win32_Process -Filter "ProcessId=${pid}").CreationDate.ToUniversalTime().ToString("o")`, @@ -513,7 +516,7 @@ export function readProcessStartMsBatch( if (platform === "win32") { try { const filter = pids.map(pid => `ProcessId=${pid}`).join(" OR "); - const stdout = execFileSync("powershell.exe", [ + const stdout = execFileSync(resolveTrustedWindowsPowerShellExe(), [ "-NoProfile", "-NoLogo", "-NonInteractive", "-WindowStyle", "Hidden", "-Command", `Get-CimInstance Win32_Process -Filter "${filter}" | ForEach-Object { "$($_.ProcessId)\t$($_.CreationDate.ToUniversalTime().ToString("o"))" }`, diff --git a/src/codex/user-identity.ts b/src/codex/user-identity.ts index 072593a56..eb6d31700 100644 --- a/src/codex/user-identity.ts +++ b/src/codex/user-identity.ts @@ -17,6 +17,8 @@ import { } from "node:fs"; import { isAbsolute, join, resolve } from "node:path"; +import { resolveTrustedWindowsPowerShellExe } from "../lib/windows-elevation"; + import type { ResolveCodexCoordinatorDatabasePath, ResolveCodexCatalogSerializationDatabasePath, @@ -30,6 +32,13 @@ const POSIX_TMP_REQUIRED_MODE = 0o1003; const POSIX_TMP_PATH = "/tmp"; const SID_PATTERN = /^S-1-(?:\d+-)+\d+$/i; +/** + * Hard budget for the Windows identity-lookup PowerShell child. These lookups + * run at startup and on config writes; a hung child must fail the lookup + * (recoverable — the caller refuses) rather than wedge the proxy indefinitely. + */ +const WINDOWS_POWERSHELL_LOOKUP_TIMEOUT_MS = 8_000; + export class CodexUserIdentityRefusal extends Error { readonly code = "CODEX_USER_IDENTITY_REFUSED"; @@ -43,24 +52,66 @@ function refuse(message: string, cause?: unknown): never { throw new CodexUserIdentityRefusal(message, cause === undefined ? undefined : { cause }); } +function windowsIdentityPowerShellCommand(expression: string): string[] { + return [ + resolveTrustedWindowsPowerShellExe(), + "-NoLogo", + "-NoProfile", + "-NonInteractive", + "-WindowStyle", + "Hidden", + "-Command", + expression, + ]; +} + +function windowsIdentityPowerShellSpawnOptions(): { + stdin: "ignore"; + stdout: "pipe"; + stderr: "pipe"; + timeout: number; + windowsHide: boolean; +} { + return { + stdin: "ignore", + stdout: "pipe", + stderr: "pipe", + timeout: WINDOWS_POWERSHELL_LOOKUP_TIMEOUT_MS, + windowsHide: true, + }; +} + +/** Test-only readback of the trusted executable and static arguments (#1278). */ +export function windowsIdentityPowerShellCommandForTests(expression: string): string[] { + return windowsIdentityPowerShellCommand(expression); +} + +/** Test-only readback of the spawn options shared by the identity lookups (#1278). */ +export function windowsIdentityPowerShellSpawnOptionsForTests(): ReturnType< + typeof windowsIdentityPowerShellSpawnOptions +> { + return windowsIdentityPowerShellSpawnOptions(); +} + function powershellValue(expression: string): string { + let command: string[]; + try { + command = windowsIdentityPowerShellCommand(expression); + } catch (cause) { + refuse("Windows effective-account lookup could not start.", cause); + } let result: ReturnType; try { - result = Bun.spawnSync([ - "powershell.exe", - "-NoLogo", - "-NoProfile", - "-NonInteractive", - "-Command", - expression, - ], { - stdin: "ignore", - stdout: "pipe", - stderr: "pipe", - }); + // `windowsHide` is the popup fix (#1278): the desktop proxy parent runs + // without a console, so a console-subsystem child spawned without + // CREATE_NO_WINDOW gets a fresh visible console window at startup and on + // every config write. `-WindowStyle Hidden` alone does not stop the + // allocation; the flag behind `windowsHide` does. + result = Bun.spawnSync(command, windowsIdentityPowerShellSpawnOptions()); } catch (cause) { refuse("Windows effective-account lookup could not start.", cause); } + if (result.exitedDueToTimeout) refuse("Windows effective-account lookup timed out."); if (result.exitCode !== 0) refuse("Windows effective-account lookup failed."); const value = new TextDecoder().decode(result.stdout).trim(); if (!value) refuse("Windows effective-account lookup returned an empty value."); diff --git a/tests/windows-popup-fix.test.ts b/tests/windows-popup-fix.test.ts new file mode 100644 index 000000000..79cb3a01b --- /dev/null +++ b/tests/windows-popup-fix.test.ts @@ -0,0 +1,78 @@ +/** + * Regression coverage for the Windows console-popup fix (#1278). + * + * The desktop proxy parent runs without a console. Every console-subsystem + * child it spawns without CREATE_NO_WINDOW (`windowsHide`) gets a fresh + * visible console window — observed at startup, on config writes, and on + * shutdown. The proxy-internal identity and process lookups must therefore + * spawn PowerShell hidden, from the trusted System32 directory (never PATH), + * and under a bounded timeout so a hung child cannot wedge those paths. + */ +import { afterEach, describe, expect, test } from "bun:test"; + +import { readProcessStartMsBatch } from "../src/codex/app-server-processes"; +import { + resolveEffectiveUserIdentity, + windowsIdentityPowerShellCommandForTests, + windowsIdentityPowerShellSpawnOptionsForTests, +} from "../src/codex/user-identity"; +import { setTrustedWindowsElevationExecutablesForTests } from "../src/lib/windows-elevation"; + +const TRUSTED_POWERSHELL = "C:\\trusted-system32\\WindowsPowerShell\\v1.0\\powershell.exe"; + +afterEach(() => { + setTrustedWindowsElevationExecutablesForTests(null); +}); + +describe("Windows identity lookup popup fix (#1278)", () => { + test("builds a hidden non-interactive command from the trusted PowerShell path", () => { + setTrustedWindowsElevationExecutablesForTests({ powershell: TRUSTED_POWERSHELL }); + const command = windowsIdentityPowerShellCommandForTests( + "[System.Security.Principal.WindowsIdentity]::GetCurrent().User.Value", + ); + expect(command[0]).toBe(TRUSTED_POWERSHELL); + expect(command).toContain("-NoProfile"); + expect(command).toContain("-NonInteractive"); + const windowStyle = command.indexOf("-WindowStyle"); + expect(windowStyle).toBeGreaterThan(0); + expect(command[windowStyle + 1]).toBe("Hidden"); + expect(command[command.length - 2]).toBe("-Command"); + expect(command[command.length - 1]) + .toBe("[System.Security.Principal.WindowsIdentity]::GetCurrent().User.Value"); + }); + + test("spawn options are hidden and bounded", () => { + const options = windowsIdentityPowerShellSpawnOptionsForTests(); + expect(options.windowsHide).toBe(true); + expect(options.stdin).toBe("ignore"); + // Assert the exact budget: the identity lookup contract is an 8-second + // bound, and a looser assertion would let a silent re-tune through. + expect(options.timeout).toBe(8_000); + }); + + test("the hidden trusted lookup resolves the real token on Windows", () => { + if (process.platform !== "win32") return; + const identity = resolveEffectiveUserIdentity(); + expect(identity.platform).toBe("win32"); + if (identity.platform === "win32") { + expect(identity.sid).toMatch(/^S-1-(?:\d+-)+\d+$/i); + } + }); +}); + +describe("Windows process-lookup popup fix (#1278)", () => { + test("batch start-time lookup is trusted, bounded, and never throws cross-platform", () => { + // On POSIX hosts the trusted System32 resolver throws inside the win32 + // branch's catch — the batch must degrade to nulls, exactly like a + // missing/failed PowerShell, instead of propagating. + const batch = readProcessStartMsBatch([process.pid], "win32"); + const startedAtMs = batch.get(process.pid) ?? null; + if (process.platform === "win32") { + expect(startedAtMs).not.toBeNull(); + expect(Number.isFinite(startedAtMs)).toBe(true); + expect(startedAtMs!).toBeGreaterThan(0); + } else { + expect(startedAtMs).toBeNull(); + } + }); +});