From aa8d848db741694e5e5e61428359b8583bd90738 Mon Sep 17 00:00:00 2001 From: wade <280641290+wade19990814-hue@users.noreply.github.com> Date: Sat, 8 Aug 2026 19:10:55 +0800 Subject: [PATCH 1/2] fix(windows): eliminate console windows from proxy-internal identity & process lookups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace PowerShell child processes with hidden windows-native alternatives: - Identity SID: whoami.exe (hidden) instead of powershell.exe (no windowsHide) - LocalAppData: token-based FFI (GetUserProfileDirectoryW) — no child process, environment-independent, verified more reliable than .NET GetFolderPath which expands %USERPROFILE% from env on some hosts - Windows principal ACL lookup: whoami.exe /user instead of powershell.exe - Process snapshots & count probes: WMIC first (faster, no .NET), hidden PowerShell CIM fallback for hosts where the deprecated WMIC is absent - Fix parseWmicListRecords for real WMIC /format:list key ordering (alphabetical — ProcessId closes each record, not opens it) Co-developed from @wade19990814-hue's in-progress fix branch. Closes #1278 --- src/codex/app-server-processes.ts | 119 ++++++++++++++---- src/codex/native-profile-processes.ts | 70 ++++++++++- src/codex/user-identity.ts | 150 +++++++++++++++++++---- src/lib/windows-elevation.ts | 20 ++- src/lib/windows-user-principal.ts | 73 ++++------- src/lib/windows-whoami.ts | 89 ++++++++++++++ src/lib/windows-wmic.ts | 138 +++++++++++++++++++++ tests/codex-app-server-processes.test.ts | 146 ++++++++++++++++++---- tests/native-profile-processes.test.ts | 116 +++++++++++++----- tests/windows-user-principal.test.ts | 19 +-- tests/windows-wmic.test.ts | 102 +++++++++++++++ 11 files changed, 873 insertions(+), 169 deletions(-) create mode 100644 src/lib/windows-whoami.ts create mode 100644 src/lib/windows-wmic.ts create mode 100644 tests/windows-wmic.test.ts diff --git a/src/codex/app-server-processes.ts b/src/codex/app-server-processes.ts index 93594d0284..a590fb98cc 100644 --- a/src/codex/app-server-processes.ts +++ b/src/codex/app-server-processes.ts @@ -9,7 +9,15 @@ */ import { execFileSync } from "node:child_process"; import { existsSync, readdirSync, readFileSync, statSync } from "node:fs"; +import { resolveTrustedWindowsPowerShellExe } from "../lib/windows-elevation"; import { isProcessAlive, waitForExit } from "../lib/process-control"; +import { + currentWindowsAccount, + parseWmicCreationDate, + parseWmicListRecords, + resolveWmicExe, + wmicGetOwner, +} from "../lib/windows-wmic"; import { readCodexCatalogPath } from "./catalog/parsing"; export const STALE_CODEX_APP_SERVER_HINT = @@ -306,17 +314,61 @@ function listDarwinSnapshots(uid: number | undefined): ProcessSnapshot[] { /** * Windows snapshots scoped to the invoking user via Win32_Process GetOwner. - * PowerShell is the sole path: WMIC lacks reliable owner data and is disabled on - * many Windows 11 installs; returning unscoped rows would contradict the - * current-user restart contract. + * WMIC is preferred when present (faster cold start, no .NET runtime) and the + * hidden PowerShell CIM enumeration is the fallback for hosts where WMIC is + * absent (a deprecated optional component on modern Windows 11 images). Both + * paths spawn with windowsHide: true — a console-less proxy presenting a child + * console window is the v2.11.0 popup bug this enumeration must not revive. * - * CIM instance methods must use Invoke-CimMethod (direct .GetOwner() calls fail). * Candidates are pre-filtered to Codex basename / code-mode-host command lines - * so we do not pay GetOwner per every process on the machine. - * Exported for the Windows integration regression that exercises the real - * PowerShell enumeration. + * so we do not pay GetOwner per every process on the machine. A candidate + * whose owner cannot be verified makes the whole enumeration incomplete; WMIC + * absence falls back rather than failing. Exported for the Windows + * integration regression. */ export function listWindowsSnapshots(): ProcessSnapshot[] { + const wmic = resolveWmicExe(); + return wmic ? listWindowsSnapshotsViaWmic(wmic) : listWindowsSnapshotsViaPowerShell(); +} + +function listWindowsSnapshotsViaWmic(wmic: string): ProcessSnapshot[] { + const me = currentWindowsAccount(); + if (!me) throw new Error("windows_enum_incomplete"); + const wql = + "(Name like 'codex%' or CommandLine like '%codex app-server%' or CommandLine like '%codex-code-mode-host%')"; + // Top-level exec failure propagates (see listDarwinSnapshots note). + const output = execFileSync( + wmic, + ["process", "where", wql, "get", "ProcessId,CommandLine,CreationDate", "/format:list"], + { encoding: "utf-8", stdio: ["ignore", "pipe", "ignore"], timeout: 8_000, windowsHide: true }, + ); + const out: ProcessSnapshot[] = []; + for (const record of parseWmicListRecords(output)) { + if (!record.commandLine) continue; + const commandLine = record.commandLine.replace(/\t/g, " ").trim(); + // The WMIC helper's own CommandLine embeds the WQL literals ("codex + // app-server", "codex-code-mode-host"), so it matches the candidate + // pre-filter. Skip any process that is the WMIC executable itself. + if (commandLine.toLowerCase().includes(wmic.toLowerCase())) continue; + if (!isWindowsCodexCandidateCommandLine(commandLine)) continue; + const owner = wmicGetOwner(record.processId); + // A candidate whose owner could not be verified makes the whole + // enumeration incomplete — the staleness collector must not read the + // partial result as "nothing running". + if (!owner) throw new Error("windows_enum_incomplete"); + const ownerName = `${owner.domain}\\${owner.user}`; + if (ownerName.toLowerCase() !== me.toLowerCase()) continue; + out.push({ + pid: record.processId, + commandLine, + owner: ownerName, + startedAtMs: parseWmicCreationDate(record.creationDate) ?? undefined, + }); + } + return out; +} + +function listWindowsSnapshotsViaPowerShell(): ProcessSnapshot[] { const out: ProcessSnapshot[] = []; // Newlines keep -Command as a real script (space-joined statements need ';'). // Double-quoted format string so `t expands to a real tab. @@ -326,6 +378,7 @@ export function listWindowsSnapshots(): ProcessSnapshot[] { // path with "opencodex". const basenameMatch = powerShellSingleQuotedIgnoreCaseMatch(WINDOWS_CODEX_BASENAME_CANDIDATE_RE.source); const codeModeMatch = powerShellSingleQuotedIgnoreCaseMatch(WINDOWS_CODEX_CODE_MODE_HOST_CANDIDATE_RE.source); + // CIM instance methods must use Invoke-CimMethod (direct .GetOwner() calls fail). const psCommand = [ "$ErrorActionPreference='SilentlyContinue'", "$me=[System.Security.Principal.WindowsIdentity]::GetCurrent().Name", @@ -346,7 +399,7 @@ export function listWindowsSnapshots(): ProcessSnapshot[] { "}", ].join("\n"); // Top-level exec failure propagates (see listDarwinSnapshots note). - const output = execFileSync("powershell.exe", [ + const output = execFileSync(resolveTrustedWindowsPowerShellExe(), [ "-NoProfile", "-NoLogo", "-NonInteractive", "-WindowStyle", "Hidden", "-Command", psCommand, @@ -456,8 +509,24 @@ function readDarwinProcStartMs(pid: number): number | null { /** Win32_Process.CreationDate → epoch ms, or null (Windows). */ function readWindowsProcStartMs(pid: number): number | null { + const wmic = resolveWmicExe(); + if (!wmic) return readWindowsProcStartMsViaPowerShell(pid); try { - const out = execFileSync("powershell.exe", [ + const out = execFileSync( + wmic, + ["process", "where", `ProcessId=${pid}`, "get", "CreationDate", "/format:list"], + { encoding: "utf-8", stdio: ["ignore", "pipe", "ignore"], timeout: 5_000, windowsHide: true }, + ); + const match = /^CreationDate=(.*)$/m.exec(out.replace(/\r/g, "")); + return parseWmicCreationDate(match?.[1]); + } catch { + return null; + } +} + +function readWindowsProcStartMsViaPowerShell(pid: number): number | null { + try { + const out = execFileSync(resolveTrustedWindowsPowerShellExe(), [ "-NoProfile", "-NoLogo", "-NonInteractive", "-WindowStyle", "Hidden", "-Command", `(Get-CimInstance Win32_Process -Filter "ProcessId=${pid}").CreationDate.ToUniversalTime().ToString("o")`, @@ -503,7 +572,7 @@ export function readProcessStartMsBatch( const parsed = Date.parse(match[2]!.trim()); if (Number.isSafeInteger(pid) && Number.isFinite(parsed)) byPid.set(pid, parsed); } - for (const pid of pids) out.set(pid, byPid.get(pid) ?? null); + for (const pid of pids) if (!out.has(pid)) out.set(pid, null); return out; } catch { for (const pid of pids) out.set(pid, null); @@ -511,22 +580,24 @@ export function readProcessStartMsBatch( } } if (platform === "win32") { + const wmic = resolveWmicExe(); + if (!wmic) { + for (const pid of pids) out.set(pid, null); + return out; + } try { - const filter = pids.map(pid => `ProcessId=${pid}`).join(" OR "); - const stdout = execFileSync("powershell.exe", [ - "-NoProfile", "-NoLogo", "-NonInteractive", "-WindowStyle", "Hidden", - "-Command", - `Get-CimInstance Win32_Process -Filter "${filter}" | ForEach-Object { "$($_.ProcessId)\t$($_.CreationDate.ToUniversalTime().ToString("o"))" }`, - ], { encoding: "utf-8", stdio: ["ignore", "pipe", "ignore"], timeout: 5_000, windowsHide: true }); - const byPid = new Map(); - for (const line of stdout.split(/\r?\n/)) { - const tab = line.indexOf("\t"); - if (tab <= 0) continue; - const pid = Number(line.slice(0, tab)); - const parsed = Date.parse(line.slice(tab + 1).trim()); - if (Number.isSafeInteger(pid) && Number.isFinite(parsed)) byPid.set(pid, parsed); + const filter = pids.map(pid => `ProcessId=${pid}`).join(" or "); + const stdout = execFileSync( + wmic, + ["process", "where", filter, "get", "ProcessId,CreationDate", "/format:list"], + { encoding: "utf-8", stdio: ["ignore", "pipe", "ignore"], timeout: 5_000, windowsHide: true }, + ); + for (const record of parseWmicListRecords(stdout)) { + if (record.creationDate !== undefined) { + out.set(record.processId, parseWmicCreationDate(record.creationDate)); + } } - for (const pid of pids) out.set(pid, byPid.get(pid) ?? null); + for (const pid of pids) if (!out.has(pid)) out.set(pid, null); return out; } catch { for (const pid of pids) out.set(pid, null); diff --git a/src/codex/native-profile-processes.ts b/src/codex/native-profile-processes.ts index 8eb27f8715..9a98bc5fad 100644 --- a/src/codex/native-profile-processes.ts +++ b/src/codex/native-profile-processes.ts @@ -1,6 +1,10 @@ import { execFile } from "node:child_process"; import { basename } from "node:path"; -import { resolveTrustedWindowsPowerShellExe } from "../lib/windows-elevation"; +import { + resolveTrustedWindowsPowerShellExe, + resolveTrustedWindowsWmicExe, +} from "../lib/windows-elevation"; +import { parseWmicListRecords } from "../lib/windows-wmic"; const PROCESS_LIST_MAX_BUFFER = 16 * 1024 * 1024; const DIRECT_CODEX_BASENAMES = new Set(["codex", "codex.exe"]); @@ -13,6 +17,9 @@ const CODEX_ENTRYPOINT_BASENAMES = new Set([ "codex.ts", ]); +const WINDOWS_CODEX_NAME_RE = /^(?:codex|codex\.exe)$/i; +const WINDOWS_CODEX_CMDLINE_RE = /(?:^|[\\/"\s])codex(?:\.exe|\.cmd)?(?:["\s]|$)/i; + export interface NativeProcessExecOptions { encoding: "utf8"; timeout: number; @@ -57,12 +64,67 @@ export const executeNativeProcess: NativeProcessExecutor = (file, args, options) }); }); -async function windowsProcessCount(run: NativeProcessExecutor): Promise { +/** + * Count Codex processes on Windows. WMIC is preferred when present (faster + * cold start, no .NET runtime); the hidden PowerShell CIM query is the + * fallback for hosts without WMIC (a deprecated optional component on modern + * Windows 11 images). Both paths run hidden (windowsHide) so a console-less + * proxy never presents a child console window. + */ +async function windowsProcessCount(run: NativeProcessExecutor, selfPid: number): Promise { + const wmic = resolveTrustedWindowsWmicExe(); + return wmic + ? windowsProcessCountViaWmic(run, wmic, selfPid) + : windowsProcessCountViaPowerShell(run, selfPid); +} + +async function windowsProcessCountViaWmic( + run: NativeProcessExecutor, + wmic: string, + selfPid: number, +): Promise { + const output = await run(wmic, [ + "process", "where", + "(Name like 'codex%' or CommandLine like '%codex%')", + "get", "ProcessId,Name,CommandLine", "/format:list", + ], { + encoding: "utf8", + timeout: 12_000, + maxBuffer: PROCESS_LIST_MAX_BUFFER, + windowsHide: true, + shell: false, + killSignal: "SIGKILL", + }); + const records = parseWmicListRecords(output); + // A valid enumeration always includes at least our own process (the proxy + // command line contains "opencodex"), so an empty result is a failure. + if (records.length === 0) throw new Error("invalid process list"); + let count = 0; + for (const record of records) { + if (record.processId === selfPid) continue; + const nameMatch = record.name !== undefined && WINDOWS_CODEX_NAME_RE.test(record.name); + const commandLineMatch = record.commandLine !== undefined + && WINDOWS_CODEX_CMDLINE_RE.test(record.commandLine); + if (nameMatch || commandLineMatch) count += 1; + } + return count; +} + +async function windowsProcessCountViaPowerShell( + run: NativeProcessExecutor, + selfPid: number, +): Promise { const powershell = resolveTrustedWindowsPowerShellExe(); + // The script text itself contains "codex", so the PowerShell host process + // matches the candidate filter and must be excluded ($self). The probing + // proxy (opencodex in its command line) is excluded by pid. + const selfFilter = Number.isSafeInteger(selfPid) && selfPid > 0 + ? ` $_.ProcessId -ne ${selfPid} -and` + : ""; const script = [ "$ErrorActionPreference='Stop';", "$self=$PID;", - "$items=Get-CimInstance Win32_Process | Where-Object { $_.ProcessId -ne $self -and ($_.Name -match '^(?i:codex)(?:\\.exe)?$' -or $_.CommandLine -match '(?i)(?:^|[\\\\/\"\\s])codex(?:\\.exe|\\.cmd)?(?:[\"\\s]|$)') };", + `$items=Get-CimInstance Win32_Process | Where-Object { $_.ProcessId -ne $self -and${selfFilter} ($_.Name -match '^(?i:codex)(?:\\.exe)?$' -or $_.CommandLine -match '(?i)(?:^|[\\\\/\"\\s])codex(?:\\.exe|\\.cmd)?(?:[\"\\s]|$)') };`, "@($items).Count", ].join(" "); const output = (await run(powershell, ["-NoLogo", "-NoProfile", "-NonInteractive", "-Command", script], { @@ -112,7 +174,7 @@ export async function probeNativeCodexProcesses({ }: NativeCodexProcessProbeOptions = {}): Promise { try { const count = await (platform === "win32" - ? windowsProcessCount(run) + ? windowsProcessCount(run, pid) : unixProcessCount(run, pid)); return count > 0 ? { status: "busy", count } : { status: "clear", count: 0 }; } catch { diff --git a/src/codex/user-identity.ts b/src/codex/user-identity.ts index 072593a566..068978cb36 100644 --- a/src/codex/user-identity.ts +++ b/src/codex/user-identity.ts @@ -16,6 +16,10 @@ import { statSync, } from "node:fs"; import { isAbsolute, join, resolve } from "node:path"; +import { dlopen, ptr, type Pointer } from "bun:ffi"; + +import { resolveTrustedWindowsWhoamiExe } from "../lib/windows-elevation"; +import { parseWindowsSidFromWhoami } from "../lib/windows-whoami"; import type { ResolveCodexCoordinatorDatabasePath, @@ -43,36 +47,135 @@ function refuse(message: string, cause?: unknown): never { throw new CodexUserIdentityRefusal(message, cause === undefined ? undefined : { cause }); } -function powershellValue(expression: string): string { +function whoamiValue(): string { let result: ReturnType; try { - result = Bun.spawnSync([ - "powershell.exe", - "-NoLogo", - "-NoProfile", - "-NonInteractive", - "-Command", - expression, - ], { + result = Bun.spawnSync([resolveTrustedWindowsWhoamiExe(), "/user"], { stdin: "ignore", stdout: "pipe", stderr: "pipe", + windowsHide: true, }); } catch (cause) { refuse("Windows effective-account lookup could not start.", cause); } 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."); - return value; + const output = new TextDecoder().decode(result.stdout); + const sid = parseWindowsSidFromWhoami(output); + if (!sid) refuse("Windows effective-account lookup returned an invalid SID."); + return sid; } function resolveWindowsSid(): string { - const sid = powershellValue( - "[System.Security.Principal.WindowsIdentity]::GetCurrent().User.Value", - ); + const sid = whoamiValue(); if (!SID_PATTERN.test(sid)) refuse("Windows effective-account lookup returned an invalid SID."); - return sid.toUpperCase(); + return sid; +} + +/** + * Resolve LocalAppData from the effective TOKEN's profile directory + * (OpenProcessToken + GetUserProfileDirectoryW) and never from the + * environment: LOCALAPPDATA and USERPROFILE are writable by whatever launched + * us, and the coordinator namespace must not follow them. The known-folder + * APIs are not usable here — on hosts whose `User Shell Folders` registry + * value embeds `%USERPROFILE%` they expand the variable from the CALLER'S + * environment (verified: SHGetFolderPathW and SHGetKnownFolderPath both fail + * or follow a faked USERPROFILE, even with an explicit token), so they are + * neither environment-independent nor fail-consistent. The token profile + * directory is. Folder redirection is deliberately not honored: a redirected + * path is only reachable through those environment-shaped lookups. + * + * FFI instead of a PowerShell child keeps the lookup window-free (the v2.11.0 + * popup bug), fast on the uncached hot path, and free of PATH trust questions. + */ +type WindowsProfileLibraries = { + getCurrentProcess: () => Pointer; + closeHandle: (handle: number) => number; + openProcessToken: (process: Pointer, desiredAccess: number, tokenOut: Pointer) => number; + getUserProfileDirectoryW: (token: number, buffer: Pointer, size: Pointer) => number; +}; + +let windowsProfileLibrariesCache: WindowsProfileLibraries | null | undefined; + +function loadWindowsProfileLibraries(): WindowsProfileLibraries | null { + if (windowsProfileLibrariesCache !== undefined) return windowsProfileLibrariesCache; + if (process.platform !== "win32") { + windowsProfileLibrariesCache = null; + return null; + } + try { + const kernel32 = dlopen("kernel32.dll", { + GetCurrentProcess: { args: [], returns: "ptr" }, + // Handles travel as pointer-sized integers. + CloseHandle: { args: ["u64"], returns: "i32" }, + }); + const advapi32 = dlopen("advapi32.dll", { + OpenProcessToken: { args: ["ptr", "u32", "ptr"], returns: "i32" }, + }); + const userenv = dlopen("userenv.dll", { + GetUserProfileDirectoryW: { args: ["u64", "ptr", "ptr"], returns: "i32" }, + }); + windowsProfileLibrariesCache = { + getCurrentProcess: () => kernel32.symbols.GetCurrentProcess() as Pointer, + closeHandle: handle => kernel32.symbols.CloseHandle(handle) as number, + openProcessToken: (process, desiredAccess, tokenOut) => + advapi32.symbols.OpenProcessToken(process, desiredAccess, tokenOut) as number, + getUserProfileDirectoryW: (token, buffer, size) => + userenv.symbols.GetUserProfileDirectoryW(token, buffer, size) as number, + }; + } catch { + windowsProfileLibrariesCache = null; + } + return windowsProfileLibrariesCache; +} + +function windowsProfileDirectory(): string { + const libraries = loadWindowsProfileLibraries(); + if (!libraries) { + refuse("Windows profile resolution could not load system libraries."); + } + let token = 0; + let profile = ""; + try { + const TOKEN_QUERY = 0x0008; + const tokenOut = new BigUint64Array(1); + const opened = libraries.openProcessToken(libraries.getCurrentProcess(), TOKEN_QUERY, ptr(tokenOut)); + if (opened === 0 || tokenOut[0] === 0n) { + refuse("Windows profile resolution could not open the process token."); + } + token = Number(tokenOut[0]); + // Profile paths fit MAX_PATH; retry once with the reported size otherwise. + let buffer = new Uint16Array(512); + let size = new Uint32Array([buffer.length]); + let ok = libraries.getUserProfileDirectoryW(token, ptr(buffer), ptr(size)); + if (ok === 0) { + const required = size[0]; + if (!Number.isSafeInteger(required) || required <= 0 || required > 32_768) { + refuse("Windows profile resolution reported an invalid profile directory size."); + } + buffer = new Uint16Array(required); + size = new Uint32Array([buffer.length]); + ok = libraries.getUserProfileDirectoryW(token, ptr(buffer), ptr(size)); + } + if (ok === 0) refuse("Windows profile resolution could not read the profile directory."); + const length = buffer.indexOf(0); + profile = String.fromCharCode(...buffer.subarray(0, length < 0 ? buffer.length : length)); + } catch (cause) { + if (cause instanceof CodexUserIdentityRefusal) throw cause; + refuse("Windows profile resolution failed.", cause); + } finally { + if (token !== 0) { + try { libraries.closeHandle(token); } catch { /* best-effort handle close */ } + } + } + if (!profile) refuse("Windows profile resolution returned an empty profile directory."); + return profile; +} + +function localAppDataValue(): string { + const localAppData = join(windowsProfileDirectory(), "AppData", "Local"); + if (!isAbsolute(localAppData)) refuse("Windows LocalAppData resolution returned a relative path."); + return localAppData; } export const resolveEffectiveUserIdentity: ResolveEffectiveUserIdentity = () => { @@ -184,9 +287,7 @@ export function probeCodexCoordinatorNamespace(identity: UserIdentity): Coordina } if (!SID_PATTERN.test(identity.sid)) refuse("The coordinator identity contains an invalid SID."); - const localAppData = powershellValue( - "[Environment]::GetFolderPath([Environment+SpecialFolder]::LocalApplicationData)", - ); + const localAppData = localAppDataValue(); if (!isAbsolute(localAppData)) refuse("Windows LocalAppData resolution returned a relative path."); const root = resolve(localAppData, "OpenCodex", "Runtime", "v1", identity.sid.toUpperCase()); let entry; @@ -216,14 +317,13 @@ export function probeCodexCoordinatorNamespace(identity: UserIdentity): Coordina function resolveWindowsRuntimeRoot(identity: Extract): string { if (!SID_PATTERN.test(identity.sid)) refuse("The coordinator identity contains an invalid SID."); - const localAppData = powershellValue( - "[Environment]::GetFolderPath([Environment+SpecialFolder]::LocalApplicationData)", - ); + const localAppData = localAppDataValue(); if (!isAbsolute(localAppData)) refuse("Windows LocalAppData resolution returned a relative path."); - // The SID and known-folder values come from the effective token/.NET OS APIs, - // never USERPROFILE or LOCALAPPDATA. WP11 adds descriptor/reparse/ACL checks at - // the stable-database open boundary where those checks can cover SQLite too. + // The SID comes from the effective token via whoami and the known-folder + // value from the token's profile directory — never USERPROFILE or + // LOCALAPPDATA. WP11 adds descriptor/reparse/ACL checks at the + // stable-database open boundary where those checks can cover SQLite too. const root = resolve(localAppData, "OpenCodex", "Runtime", "v1", identity.sid.toUpperCase()); try { mkdirSync(root, { recursive: true }); diff --git a/src/lib/windows-elevation.ts b/src/lib/windows-elevation.ts index 591dec81ee..58f1b7af1d 100644 --- a/src/lib/windows-elevation.ts +++ b/src/lib/windows-elevation.ts @@ -168,7 +168,7 @@ export function assertTrustedSystemExecutableForTests(candidate: string, label: return assertTrustedSystemExecutable(candidate, label); } -type ElevationExeOverrides = { powershell?: string; schtasks?: string }; +type ElevationExeOverrides = { powershell?: string; schtasks?: string; whoami?: string; wmic?: string | null }; let elevationExeOverridesForTests: ElevationExeOverrides | null = null; /** @@ -204,6 +204,24 @@ export function resolveTrustedWindowsSchtasksExe(): string { return assertTrustedSystemExecutable(candidate, "schtasks.exe"); } +/** Absolute path to System32\\whoami.exe from a trusted system directory. */ +export function resolveTrustedWindowsWhoamiExe(): string { + if (elevationExeOverridesForTests?.whoami) { + return elevationExeOverridesForTests.whoami; + } + const candidate = join(resolveTrustedWindowsSystemDirectory(), "whoami.exe"); + return assertTrustedSystemExecutable(candidate, "whoami.exe"); +} + +/** Absolute path to System32\\wbem\\WMIC.exe from a trusted system directory, when present. */ +export function resolveTrustedWindowsWmicExe(): string | null { + // An explicit null override means "WMIC absent" for fallback-path tests. + const override = elevationExeOverridesForTests?.wmic; + if (override !== undefined) return override; + const candidate = join(resolveTrustedWindowsSystemDirectory(), "wbem", "WMIC.exe"); + return existsSync(candidate) ? candidate : null; +} + /** Stable machine-readable marker for a denied `schtasks /create`. Crosses the CLI→proxy boundary. */ export const WINDOWS_SCHTASKS_CREATE_ACCESS_DENIED_MARKER = "OCX_ERROR_CODE=WINDOWS_SCHTASKS_CREATE_ACCESS_DENIED"; diff --git a/src/lib/windows-user-principal.ts b/src/lib/windows-user-principal.ts index 93da3efd54..3c9cba597a 100644 --- a/src/lib/windows-user-principal.ts +++ b/src/lib/windows-user-principal.ts @@ -18,13 +18,20 @@ * timer only arms once `Bun.spawn` returns. Both are small in practice, but the * lookup is not bounded by the deadline to the microsecond. Tightening that * would mean passing an absolute deadline through the runner interface. + * + * The lookup runs `whoami /user` instead of `powershell.exe`. The original + * PowerShell path spawned without `windowsHide`, so every secret-file write + * from the console-less proxy presented a visible console window (the v2.11.0 + * popup bug). `whoami.exe` keeps the popup fixed — it runs hidden via + * CREATE_NO_WINDOW (windowsHide) — while starting far faster than PowerShell + * 5.1 and reading the same effective-token SID. It is resolved from the + * trusted System32 directory, never from PATH. */ -import { resolveTrustedWindowsPowerShellExe } from "./windows-elevation"; +import { resolveTrustedWindowsWhoamiExe } from "./windows-elevation"; +import { parseWindowsSidFromWhoami, runWhoamiAsync, runWhoamiSync } from "./windows-whoami"; const SID_PATTERN = /^S-1-(?:\d+-)+\d+$/i; -const SID_EXPRESSION = - "[System.Security.Principal.WindowsIdentity]::GetCurrent().User.Value"; export interface WindowsPrincipalLookupResult { success: boolean; @@ -41,69 +48,35 @@ export type AsyncWindowsPrincipalRunner = ( timeoutMs: number, ) => Promise; -const POWERSHELL_ARGS = [ - "-NoLogo", - "-NoProfile", - "-NonInteractive", - "-WindowStyle", - "Hidden", - "-Command", - SID_EXPRESSION, -] as const; - -function windowsPrincipalPowerShellCommand(): string[] { - return [resolveTrustedWindowsPowerShellExe(), ...POWERSHELL_ARGS]; +function windowsPrincipalWhoamiCommand(): string[] { + return [resolveTrustedWindowsWhoamiExe(), "/user"]; } /** Test-only readback of the exact trusted executable and static arguments. */ -export function windowsPrincipalPowerShellCommandForTests(): string[] { - return windowsPrincipalPowerShellCommand(); +export function windowsPrincipalCommandForTests(): string[] { + return windowsPrincipalWhoamiCommand(); } function defaultWindowsPrincipalRunner(timeoutMs: number): WindowsPrincipalLookupResult { - const result = Bun.spawnSync(windowsPrincipalPowerShellCommand(), { - stdin: "ignore", - stdout: "pipe", - stderr: "ignore", - timeout: Math.max(1, timeoutMs), - windowsHide: true, - }); + const result = runWhoamiSync(timeoutMs); return { success: result.success, exitCode: result.exitCode, - timedOut: result.exitedDueToTimeout ?? false, - stdout: result.stdout ? result.stdout.toString() : "", + timedOut: result.timedOut, + // stdout carries exactly the SID (principalFromResult validates it). + stdout: result.success ? parseWindowsSidFromWhoami(result.output) ?? "" : "", }; } async function defaultAsyncWindowsPrincipalRunner( timeoutMs: number, ): Promise { - const proc = Bun.spawn(windowsPrincipalPowerShellCommand(), { - stdin: "ignore", - stdout: "pipe", - stderr: "ignore", - windowsHide: true, - }); - let timedOut = false; - const timer = setTimeout(() => { - timedOut = true; - try { proc.kill(); } catch { /* already exited */ } - }, Math.max(1, timeoutMs)); - let exitCode: number | null = null; - try { - exitCode = await proc.exited; - } finally { - clearTimeout(timer); - } - const stdout = proc.stdout - ? await new Response(proc.stdout).text().catch(() => "") - : ""; + const result = await runWhoamiAsync(timeoutMs); return { - success: !timedOut && exitCode === 0, - exitCode: timedOut ? null : exitCode, - timedOut, - stdout, + success: result.success, + exitCode: result.exitCode, + timedOut: result.timedOut, + stdout: result.success ? parseWindowsSidFromWhoami(result.output) ?? "" : "", }; } diff --git a/src/lib/windows-whoami.ts b/src/lib/windows-whoami.ts new file mode 100644 index 0000000000..dd300b7f41 --- /dev/null +++ b/src/lib/windows-whoami.ts @@ -0,0 +1,89 @@ +/** + * Console-free Windows identity lookups. + * + * opencodex previously resolved the effective Windows account by spawning + * `powershell.exe` without `windowsHide`, so every lookup from the + * console-less proxy presented a visible console window (the v2.11.0 popup + * bug). `whoami.exe` replaces the PowerShell SID lookup with the same trust + * posture and the verified fix: it resolves from the trusted System32 + * directory (never PATH) and is spawned with windowsHide (CREATE_NO_WINDOW), + * which prevents any console window. It also starts far faster than + * PowerShell 5.1, which mattered on the startup hot path. + */ +import { resolveTrustedWindowsWhoamiExe } from "./windows-elevation"; + +export const WINDOWS_SID_PATTERN = /^S-1-(?:\d+-)+\d+$/i; + +/** + * Extract the SID from `whoami /user` output. The header text is localized, + * but the SID token itself is numeric, so a locale-independent scan is safe. + * Returns the uppercase SID without the leading `*` (callers add icacls form). + */ +export function parseWindowsSidFromWhoami(output: string): string | null { + for (const line of output.split(/\r?\n/)) { + // whoami prints `DOMAIN\user S-1-5-...`; the SID is the trailing token. + const match = /S-1-(?:\d+-)+\d+/i.exec(line.trim()); + if (match) return match[0].toUpperCase(); + } + return null; +} + +export interface WhoamiResult { + success: boolean; + exitCode: number | null; + timedOut: boolean; + output: string; +} + +/** `whoami /user`, hidden, bounded by the caller's remaining deadline. */ +export function runWhoamiSync(timeoutMs: number): WhoamiResult { + const result = Bun.spawnSync( + [resolveTrustedWindowsWhoamiExe(), "/user"], + { + stdin: "ignore", + stdout: "pipe", + stderr: "ignore", + timeout: Math.max(1, timeoutMs), + windowsHide: true, + }, + ); + return { + success: result.success, + exitCode: result.exitCode, + timedOut: result.exitedDueToTimeout ?? false, + output: result.stdout ? result.stdout.toString() : "", + }; +} + +/** Async counterpart of {@link runWhoamiSync} with the same hidden spawn. */ +export async function runWhoamiAsync(timeoutMs: number): Promise { + const proc = Bun.spawn( + [resolveTrustedWindowsWhoamiExe(), "/user"], + { + stdin: "ignore", + stdout: "pipe", + stderr: "ignore", + windowsHide: true, + }, + ); + let timedOut = false; + const timer = setTimeout(() => { + timedOut = true; + try { proc.kill(); } catch { /* already exited */ } + }, Math.max(1, timeoutMs)); + let exitCode: number | null = null; + try { + exitCode = await proc.exited; + } finally { + clearTimeout(timer); + } + const output = proc.stdout + ? await new Response(proc.stdout).text().catch(() => "") + : ""; + return { + success: !timedOut && exitCode === 0, + exitCode: timedOut ? null : exitCode, + timedOut, + output, + }; +} diff --git a/src/lib/windows-wmic.ts b/src/lib/windows-wmic.ts new file mode 100644 index 0000000000..09d2031d3b --- /dev/null +++ b/src/lib/windows-wmic.ts @@ -0,0 +1,138 @@ +/** + * Console-free Windows process introspection via WMIC. + * + * These helpers replace PowerShell Win32_Process queries where WMIC is + * available: WMIC starts far faster than PowerShell 5.1 and, spawned with + * `windowsHide: true` (CREATE_NO_WINDOW), never presents a console window. + * WMIC is a deprecated optional component and is absent on many modern + * Windows images, so callers must treat a missing WMIC as an enumeration + * failure (fail closed), never as "nothing is running", and keep a hidden + * PowerShell fallback for hosts without it. + */ +import { execFileSync } from "node:child_process"; +import { resolveTrustedWindowsWmicExe, resolveTrustedWindowsWhoamiExe } from "./windows-elevation"; + +/** Resolve the trusted WMIC executable, or null when absent on the host. */ +export function resolveWmicExe(): string | null { + return resolveTrustedWindowsWmicExe(); +} + +export interface WmicProcessRecord { + processId: number; + name?: string; + commandLine?: string; + creationDate?: string; +} + +/** + * Parse `wmic ... /format:list` output. Records are KEY=VALUE blocks separated + * by blank lines. WMIC emits the requested keys in ALPHABETICAL order + * (CommandLine, CreationDate, Name, ProcessId), so ProcessId typically closes + * a record rather than opening it: keys are collected per block in any order + * and the record is materialized at the block boundary. CommandLine may + * contain embedded newlines, so a continuation line that does not look like a + * known key is appended to the running value. + */ +export function parseWmicListRecords(output: string): WmicProcessRecord[] { + const records: WmicProcessRecord[] = []; + let current: Partial | null = null; + const flush = (): void => { + if (current && Number.isSafeInteger(current.processId) && (current.processId ?? 0) > 1) { + records.push({ + processId: current.processId as number, + name: current.name, + commandLine: current.commandLine, + creationDate: current.creationDate, + }); + } + current = null; + }; + for (const rawLine of output.split(/\r?\n/)) { + const line = rawLine.replace(/\s+$/, ""); + if (!line.trim()) { + flush(); + continue; + } + const match = /^([A-Za-z]+)=(.*)$/.exec(line); + if (match) { + const [, key, value] = match; + if (!current) current = {}; + if (key === "ProcessId") current.processId = Number(value.trim()); + else if (key === "Name") current.name = value.trim(); + else if (key === "CommandLine") current.commandLine = value; + else if (key === "CreationDate") current.creationDate = value.trim(); + continue; + } + if (current?.commandLine !== undefined) { + current.commandLine += `\n${line}`; + } + } + flush(); + return records; +} + +const WMIC_CREATION_DATE_RE = /^(\d{4})(\d{2})(\d{2})(\d{2})(\d{2})(\d{2})(?:\.(\d{1,7}))?([+-]\d{1,4})?$/; + +/** + * Convert a WMIC CreationDate value (`YYYYMMDDHHMMSS.microseconds±offset`) + * to epoch milliseconds, or null when unparseable. + */ +export function parseWmicCreationDate(value: string | undefined): number | null { + if (!value) return null; + const match = WMIC_CREATION_DATE_RE.exec(value.trim()); + if (!match) return null; + const year = Number(match[1]); + const month = Number(match[2]); + const day = Number(match[3]); + const hour = Number(match[4]); + const minute = Number(match[5]); + const second = Number(match[6]); + const fraction = match[7] ? Number(match[7].slice(0, 3).padEnd(3, "0")) : 0; + const offsetMinutes = match[8] ? Number(match[8]) : 0; + const utcMs = Date.UTC(year, month - 1, day, hour, minute, second, fraction); + return Number.isFinite(utcMs) ? utcMs - offsetMinutes * 60_000 : null; +} + +export interface WmicOwner { + domain: string; + user: string; +} + +/** Resolve the owner of one process via `wmic ... call getowner`, or null. */ +export function wmicGetOwner(pid: number): WmicOwner | null { + const wmic = resolveWmicExe(); + if (!wmic) return null; + let output: string; + try { + output = execFileSync( + wmic, + ["process", "where", `ProcessId=${pid}`, "call", "getowner"], + { encoding: "utf-8", stdio: ["ignore", "pipe", "ignore"], timeout: 5_000, windowsHide: true }, + ); + } catch { + return null; + } + const returnValue = /ReturnValue\s*=\s*(\d+)/.exec(output); + if (!returnValue || returnValue[1] !== "0") return null; + const user = /User\s*=\s*"([^"]*)"/.exec(output); + const domain = /Domain\s*=\s*"([^"]*)"/.exec(output); + if (!user || !user[1]) return null; + return { domain: domain?.[1] ?? "", user: user[1] }; +} + +/** Current account name in `DOMAIN\user` form via `whoami`, or null. */ +export function currentWindowsAccount(): string | null { + const whoami = resolveTrustedWindowsWhoamiExe(); + let output: string; + try { + output = execFileSync( + whoami, + [], + { encoding: "utf-8", stdio: ["ignore", "pipe", "ignore"], timeout: 5_000, windowsHide: true }, + ); + } catch { + return null; + } + const line = output.split(/\r?\n/).map(line => line.trim()).find(Boolean); + return line && /\\/.test(line) ? line : null; +} diff --git a/tests/codex-app-server-processes.test.ts b/tests/codex-app-server-processes.test.ts index c1b421bd79..d9c7156239 100644 --- a/tests/codex-app-server-processes.test.ts +++ b/tests/codex-app-server-processes.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { spawn } from "node:child_process"; +import { spawn, spawnSync } from "node:child_process"; import { readFileSync } from "node:fs"; import { join } from "node:path"; import { @@ -17,6 +17,7 @@ import { warnIfStaleCodexAppServersAfterStartupWrite, WINDOWS_CODEX_BASENAME_CANDIDATE_RE, } from "../src/codex/app-server-processes"; +import { setTrustedWindowsElevationExecutablesForTests } from "../src/lib/windows-elevation"; describe("collectCodexAppServerCatalogState (#857)", () => { const APP_SERVER_CMD = "/usr/local/bin/codex app-server"; @@ -434,37 +435,55 @@ describe("Windows Win32_Process owner enumeration (#476)", () => { join(import.meta.dir, "..", "src", "codex", "app-server-processes.ts"), "utf8", ); + const wmicSource = readFileSync( + join(import.meta.dir, "..", "src", "lib", "windows-wmic.ts"), + "utf8", + ); - test("PowerShell uses Invoke-CimMethod GetOwner and fails closed on ReturnValue", () => { - expect(processSource).toContain( - "Invoke-CimMethod -InputObject $_ -MethodName GetOwner -ErrorAction Stop", - ); - expect(processSource).toContain("$o.ReturnValue -ne 0"); - expect(processSource).toContain(".join(\"\\n\")"); - expect(processSource).not.toMatch(/\$o=\$_\.GetOwner\(\)/); - // Shared candidate regex (optional closing quote after basename) drives -match. - expect(processSource).toContain("WINDOWS_CODEX_BASENAME_CANDIDATE_RE.source"); - expect(processSource).toContain("powerShellSingleQuotedIgnoreCaseMatch"); + test("Windows enumeration is user-scoped, hidden, and fails closed", () => { + // WMIC is the primary enumeration path... + expect(processSource).toContain('"/format:list"'); + expect(wmicSource).toContain("call getowner"); + expect(processSource).toContain("wmicGetOwner"); + // ...with a PowerShell CIM fallback for hosts where WMIC is absent. The + // fallback must use the trusted System32 executable (never a bare + // PATH-resolved "powershell.exe") and stay hidden — the v2.11.0 popup + // regression guard. + expect(processSource).not.toContain('"powershell.exe"'); + expect(processSource).toContain("resolveTrustedWindowsPowerShellExe()"); + expect(processSource).toContain("windows_enum_incomplete"); + expect(processSource).toContain("isWindowsCodexCandidateCommandLine"); expect(WINDOWS_CODEX_BASENAME_CANDIDATE_RE.source).toContain("['\"]?"); + // Every Windows child spawned on this module's enumeration paths stays + // hidden (the POSIX `ps` sites do not need the flag). + const windowsSpawnSites = processSource + .match(/execFileSync\(\s*(wmic|resolveTrustedWindowsPowerShellExe\(\))/g)?.length ?? 0; + const hiddenSites = processSource.match(/windowsHide: true/g)?.length ?? 0; + expect(windowsSpawnSites).toBeGreaterThan(0); + expect(hiddenSites).toBeGreaterThanOrEqual(windowsSpawnSites); }); test.skipIf(process.platform !== "win32")( - "listWindowsSnapshots returns a current-user Codex-shaped process via real PowerShell enumeration", + "listWindowsSnapshots returns a current-user Codex-shaped process via real WMIC enumeration", () => { + // The probe payload carries a UNIQUE marker. Matching and cleanup only + // ever use that marker: the real Codex app-server's CommandLine contains + // "codex app-server", so matching on that phrase would select (and kill) + // the user's actual Codex app-server. The marker makes that impossible. + const PROBE_MARKER = "ocx-test-probe-7e62"; // Keep a live process whose CommandLine contains a Codex basename token. + // cmd.exe re-spawns a second cmd for the /c payload on some Windows + // builds, so match by CommandLine rather than by the spawn pid. const child = spawn( - "powershell.exe", - [ - "-NoProfile", "-NoLogo", "-NonInteractive", "-WindowStyle", "Hidden", - "-Command", - "Start-Sleep -Seconds 45 # codex app-server integration-probe", - ], + "cmd.exe", + ["/d", "/c", `ping -n 45 127.0.0.1 >nul & rem ${PROBE_MARKER} codex app-server integration-probe`], { stdio: "ignore", windowsHide: true }, ); + let survivor: number | null = null; try { expect(child.pid).toBeGreaterThan(1); // Brief settle so Win32_Process can observe the child. A loaded Windows - // runner can also exhaust one CIM enumeration deadline, so tolerate one + // runner can also exhaust one enumeration deadline, so tolerate one // transient empty result OR one thrown deadline (ETIMEDOUT propagates by // design) while keeping the production timeout unchanged. Bun.sleepSync(250); @@ -476,22 +495,105 @@ describe("Windows Win32_Process owner enumeration (#476)", () => { } }; let snapshots = enumerate() ?? []; - let match = snapshots.find(snapshot => snapshot.pid === child.pid); + let match = snapshots.find(snapshot => snapshot.commandLine.includes(PROBE_MARKER)); if (!match) { Bun.sleepSync(250); snapshots = enumerate() ?? []; - match = snapshots.find(snapshot => snapshot.pid === child.pid); + match = snapshots.find(snapshot => snapshot.commandLine.includes(PROBE_MARKER)); } if (!match) { Bun.sleepSync(1_000); snapshots = enumerate() ?? []; - match = snapshots.find(snapshot => snapshot.pid === child.pid); + match = snapshots.find(snapshot => snapshot.commandLine.includes(PROBE_MARKER)); } expect(match).toBeDefined(); + survivor = match!.pid; expect(match!.owner).toMatch(/\\/); expect(match!.commandLine.toLowerCase()).toContain("codex app-server"); + expect(match!.commandLine).toContain(PROBE_MARKER); + expect(snapshots.every(snapshot => (snapshot.owner?.trim().length ?? 0) > 0)).toBe(true); + } finally { + try { + if (survivor) { + // Double-check the current CommandLine still carries the unique + // marker before taskkill. If the PID was recycled or the marker is + // gone, do NOT kill anything. + const verify = spawnSync( + "wmic", + ["process", "where", `ProcessId=${survivor}`, "get", "CommandLine", "/value"], + { encoding: "utf8", stdio: ["ignore", "pipe", "ignore"], windowsHide: true, timeout: 5_000 }, + ); + const current = (verify.stdout ?? "").replace(/\r/g, ""); + if (/^CommandLine=.*$/m.test(current) && current.includes(PROBE_MARKER)) { + spawnSync("taskkill", ["/F", "/PID", String(survivor)], { stdio: "ignore", windowsHide: true }); + } + } + } catch { + /* already exited */ + } + try { + child.kill(); + } catch { + /* already exited */ + } + } + }, + { timeout: 35_000 }, + ); + + test.skipIf(process.platform !== "win32")( + "listWindowsSnapshots falls back to hidden PowerShell when WMIC is absent", + () => { + // Same unique-marker discipline as the WMIC integration test above: the + // real Codex app-server's CommandLine contains "codex app-server", so a + // probe must only ever be matched by its unique marker. + const PROBE_MARKER = "ocx-test-probe-psfallback-9d41"; + const child = spawn( + "cmd.exe", + ["/d", "/c", `ping -n 45 127.0.0.1 >nul & rem ${PROBE_MARKER} codex app-server fallback-probe`], + { stdio: "ignore", windowsHide: true }, + ); + let survivor: number | null = null; + setTrustedWindowsElevationExecutablesForTests({ wmic: null }); + try { + expect(child.pid).toBeGreaterThan(1); + Bun.sleepSync(250); + const enumerate = (): ReturnType | undefined => { + try { + return listWindowsSnapshots(); + } catch { + return undefined; // transient CIM deadline on a contended runner + } + }; + let snapshots = enumerate() ?? []; + let match = snapshots.find(snapshot => snapshot.commandLine.includes(PROBE_MARKER)); + if (!match) { + Bun.sleepSync(500); + snapshots = enumerate() ?? []; + match = snapshots.find(snapshot => snapshot.commandLine.includes(PROBE_MARKER)); + } + expect(match).toBeDefined(); + survivor = match!.pid; + expect(match!.owner).toMatch(/\\/); + expect(match!.commandLine).toContain(PROBE_MARKER); expect(snapshots.every(snapshot => (snapshot.owner?.trim().length ?? 0) > 0)).toBe(true); } finally { + setTrustedWindowsElevationExecutablesForTests(null); + try { + if (survivor) { + const verify = spawnSync( + "wmic", + ["process", "where", `ProcessId=${survivor}`, "get", "CommandLine", "/value"], + { encoding: "utf8", stdio: ["ignore", "pipe", "ignore"], windowsHide: true, timeout: 5_000 }, + ); + const current = (verify.stdout ?? "").replace(/\r/g, ""); + if (/^CommandLine=.*$/m.test(current) && current.includes(PROBE_MARKER)) { + spawnSync("taskkill", ["/F", "/PID", String(survivor)], { stdio: "ignore", windowsHide: true }); + } + } + } catch { + /* already exited */ + } try { child.kill(); } catch { diff --git a/tests/native-profile-processes.test.ts b/tests/native-profile-processes.test.ts index a62dc0b273..461f36cdef 100644 --- a/tests/native-profile-processes.test.ts +++ b/tests/native-profile-processes.test.ts @@ -1,50 +1,66 @@ import { describe, expect, test } from "bun:test"; -import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { existsSync, mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; -import { dirname, join } from "node:path"; +import { join } from "node:path"; import { executeNativeProcess, probeNativeCodexProcesses, type NativeProcessExecutor, } from "../src/codex/native-profile-processes"; -import { setTrustedWindowsSystemDirectoryResolverForTests } from "../src/lib/windows-elevation"; - -async function withTrustedWindowsPowerShell(run: (powershell: string) => Promise): Promise { - const systemDirectory = mkdtempSync(join(tmpdir(), "ocx-system32-")); - const powershell = join(systemDirectory, "WindowsPowerShell", "v1.0", "powershell.exe"); - mkdirSync(dirname(powershell), { recursive: true }); - writeFileSync(powershell, ""); - setTrustedWindowsSystemDirectoryResolverForTests(() => systemDirectory); +import { setTrustedWindowsElevationExecutablesForTests } from "../src/lib/windows-elevation"; + +const TRUSTED_WMIC = "C:\\trusted-system32\\wbem\\WMIC.exe"; +const TRUSTED_POWERSHELL = "C:\\trusted-system32\\WindowsPowerShell\\v1.0\\powershell.exe"; + +async function withTrustedWindowsWmic(run: (wmic: string) => Promise): Promise { + setTrustedWindowsElevationExecutablesForTests({ wmic: TRUSTED_WMIC }); try { - return await run(powershell); + return await run(TRUSTED_WMIC); } finally { - setTrustedWindowsSystemDirectoryResolverForTests(null); - rmSync(systemDirectory, { recursive: true, force: true }); + setTrustedWindowsElevationExecutablesForTests(null); + } +} + +async function withWindowsWmicAbsent(run: (powershell: string) => Promise): Promise { + setTrustedWindowsElevationExecutablesForTests({ wmic: null, powershell: TRUSTED_POWERSHELL }); + try { + return await run(TRUSTED_POWERSHELL); + } finally { + setTrustedWindowsElevationExecutablesForTests(null); } } describe("native profile process probe", () => { - test("uses the trusted PowerShell path with shell-free bounded execution", async () => { + test("uses WMIC with shell-free bounded execution and counts Codex processes", async () => { const calls: Parameters[] = []; const execFile: NativeProcessExecutor = async (file, args, options) => { calls.push([file, args, options]); - return "2\n"; + // Real WMIC /format:list shape: keys in alphabetical order per block, + // blank lines between records (ProcessId CLOSES a record). + return [ + "CommandLine=codex app-server --serve", + "Name=codex.exe", + "ProcessId=41", + "", + 'CommandLine="C:\\tools\\codex.cmd" serve', + "Name=cmd.exe", + "ProcessId=42", + "", + "CommandLine=bun D:\\tools\\opencodex\\src\\cli\\index.ts start", + "Name=bun.exe", + "ProcessId=99", + ].join("\n"); }; - const script = [ - "$ErrorActionPreference='Stop';", - "$self=$PID;", - "$items=Get-CimInstance Win32_Process | Where-Object { $_.ProcessId -ne $self -and ($_.Name -match '^(?i:codex)(?:\\.exe)?$' -or $_.CommandLine -match '(?i)(?:^|[\\\\/\"\\s])codex(?:\\.exe|\\.cmd)?(?:[\"\\s]|$)') };", - "@($items).Count", - ].join(" "); - await withTrustedWindowsPowerShell(async powershell => { + await withTrustedWindowsWmic(async wmic => { await expect(probeNativeCodexProcesses({ platform: "win32", execFile, + pid: 99, })).resolves.toEqual({ status: "busy", count: 2 }); expect(calls).toEqual([[ - powershell, - ["-NoLogo", "-NoProfile", "-NonInteractive", "-Command", script], + wmic, + ["process", "where", "(Name like 'codex%' or CommandLine like '%codex%')", "get", "ProcessId,Name,CommandLine", "/format:list"], { encoding: "utf8", timeout: 12_000, @@ -57,6 +73,47 @@ describe("native profile process probe", () => { }); }); + test("falls back to hidden PowerShell when WMIC is absent", async () => { + const calls: Parameters[] = []; + const execFile: NativeProcessExecutor = async (file, args, options) => { + calls.push([file, args, options]); + return "2"; + }; + await withWindowsWmicAbsent(async powershell => { + await expect(probeNativeCodexProcesses({ + platform: "win32", + execFile, + pid: 99, + })).resolves.toEqual({ status: "busy", count: 2 }); + + expect(calls).toHaveLength(1); + expect(calls[0]![0]).toBe(powershell); + expect(calls[0]![1].slice(0, 3)).toEqual(["-NoLogo", "-NoProfile", "-NonInteractive"]); + // The PowerShell host (its script text contains "codex") and the + // probing proxy pid are both excluded from the count. + const script = calls[0]![1].join(" "); + expect(script).toContain("$_.ProcessId -ne $self"); + expect(script).toContain(" $_.ProcessId -ne 99 "); + expect(calls[0]![2].windowsHide).toBe(true); + expect(calls[0]![2].shell).toBe(false); + }); + }); + + test("excludes its own pid on Windows", async () => { + const execFile: NativeProcessExecutor = async () => [ + "CommandLine=codex app-server --serve", + "Name=codex.exe", + "ProcessId=100", + ].join("\n"); + await withTrustedWindowsWmic(async () => { + await expect(probeNativeCodexProcesses({ + platform: "win32", + execFile, + pid: 100, + })).resolves.toEqual({ status: "clear", count: 0 }); + }); + }); + test("sets the same buffer for Unix process lists and excludes its own pid", async () => { const calls: Parameters[] = []; const execFile: NativeProcessExecutor = async (file, args, options) => { @@ -192,14 +249,15 @@ describe("native profile process probe", () => { })).rejects.toThrow(); }); - test("fails closed for non-decimal or unsafe Windows counts", async () => { - await withTrustedWindowsPowerShell(async () => { - for (const output of ["", " ", "-1", "1.0", "1e2", "0x10", "9007199254740992"]) { + test("fails closed for unparseable Windows process lists", async () => { + for (const output of ["", " ", "-1", "1.0", "garbage", "ProcessId=not-a-number", "ProcessId=9007199254740992"]) { + await withTrustedWindowsWmic(async () => { await expect(probeNativeCodexProcesses({ platform: "win32", execFile: async () => output, + pid: 42, })).resolves.toEqual({ status: "unknown", count: 0 }); - } - }); + }); + } }); }); diff --git a/tests/windows-user-principal.test.ts b/tests/windows-user-principal.test.ts index c0d01cc574..6f9879f66d 100644 --- a/tests/windows-user-principal.test.ts +++ b/tests/windows-user-principal.test.ts @@ -6,7 +6,7 @@ import { resolveCurrentWindowsPrincipalAsync, setAsyncWindowsPrincipalRunnerForTests, setWindowsPrincipalRunnerForTests, - windowsPrincipalPowerShellCommandForTests, + windowsPrincipalCommandForTests, } from "../src/lib/windows-user-principal"; import { setTrustedWindowsElevationExecutablesForTests } from "../src/lib/windows-elevation"; @@ -25,19 +25,10 @@ afterEach(() => { }); describe("Windows effective ACL principal", () => { - test("builds a hidden non-interactive command from the trusted PowerShell path", () => { - const trusted = "C:\\trusted-system32\\WindowsPowerShell\\v1.0\\powershell.exe"; - setTrustedWindowsElevationExecutablesForTests({ powershell: trusted }); - expect(windowsPrincipalPowerShellCommandForTests()).toEqual([ - trusted, - "-NoLogo", - "-NoProfile", - "-NonInteractive", - "-WindowStyle", - "Hidden", - "-Command", - "[System.Security.Principal.WindowsIdentity]::GetCurrent().User.Value", - ]); + test("builds a hidden command from the trusted whoami path", () => { + const trusted = "C:\\trusted-system32\\whoami.exe"; + setTrustedWindowsElevationExecutablesForTests({ whoami: trusted }); + expect(windowsPrincipalCommandForTests()).toEqual([trusted, "/user"]); }); test("the default trusted runner resolves the real token on Windows", () => { diff --git a/tests/windows-wmic.test.ts b/tests/windows-wmic.test.ts new file mode 100644 index 0000000000..8d06dd49ef --- /dev/null +++ b/tests/windows-wmic.test.ts @@ -0,0 +1,102 @@ +import { describe, expect, test } from "bun:test"; +import { + parseWmicCreationDate, + parseWmicListRecords, +} from "../src/lib/windows-wmic"; + +describe("parseWmicListRecords", () => { + test("parses real /format:list output with alphabetical key order", () => { + // WMIC emits requested keys alphabetically per block: CommandLine, + // CreationDate, Name, ProcessId — ProcessId CLOSES a record. The parser + // must not attribute a record's fields to its predecessor. + const output = [ + "\r", + "\r", + 'CommandLine="C:\\tools\\codex.exe" app-server --serve\r', + "CreationDate=20260808120000.000000+480\r", + "Name=codex.exe\r", + "ProcessId=41\r", + "\r", + "\r", + 'CommandLine="C:\\Windows\\System32\\cmd.exe" /c codex\r', + "CreationDate=20260808120100.000000+480\r", + "Name=cmd.exe\r", + "ProcessId=42\r", + "\r", + ].join("\n"); + const records = parseWmicListRecords(output); + expect(records).toEqual([ + { + processId: 41, + name: "codex.exe", + commandLine: '"C:\\tools\\codex.exe" app-server --serve', + creationDate: "20260808120000.000000+480", + }, + { + processId: 42, + name: "cmd.exe", + commandLine: '"C:\\Windows\\System32\\cmd.exe" /c codex', + creationDate: "20260808120100.000000+480", + }, + ]); + }); + + test("parses a final record without a trailing blank line", () => { + const records = parseWmicListRecords([ + "CommandLine=codex app-server", + "ProcessId=7", + ].join("\n")); + expect(records).toEqual([{ processId: 7, name: undefined, commandLine: "codex app-server", creationDate: undefined }]); + }); + + test("appends continuation lines to a multi-line CommandLine", () => { + const records = parseWmicListRecords([ + "CommandLine=codex app-server", + "--second-line", + "ProcessId=9", + ].join("\n")); + expect(records[0]?.commandLine).toBe("codex app-server\n--second-line"); + }); + + test("drops records with unusable ProcessIds", () => { + for (const block of [ + "CommandLine=x\nProcessId=not-a-number", + "CommandLine=x\nProcessId=9007199254740992", + "CommandLine=x\nProcessId=1", + "CommandLine=x", + ]) { + expect(parseWmicListRecords(block)).toEqual([]); + } + }); + + test("a block without ProcessId never leaks fields into the next record", () => { + const records = parseWmicListRecords([ + "CommandLine=orphan", + "", + "CommandLine=codex app-server", + "ProcessId=5", + ].join("\n")); + expect(records).toEqual([ + { processId: 5, name: undefined, commandLine: "codex app-server", creationDate: undefined }, + ]); + }); +}); + +describe("parseWmicCreationDate", () => { + test("converts a WMIC CreationDate with offset to epoch ms", () => { + const value = parseWmicCreationDate("20260808120000.000000+480"); + expect(value).toBe(Date.UTC(2026, 7, 8, 12, 0, 0, 0) - 480 * 60_000); + }); + + test("handles fractional seconds and missing offset", () => { + const value = parseWmicCreationDate("20260808120000.123456"); + expect(value).toBe(Date.UTC(2026, 7, 8, 12, 0, 0, 123)); + }); + + test("returns null for unparseable values", () => { + expect(parseWmicCreationDate(undefined)).toBeNull(); + expect(parseWmicCreationDate("")).toBeNull(); + expect(parseWmicCreationDate("garbage")).toBeNull(); + expect(parseWmicCreationDate("2026080812")).toBeNull(); + }); +}); From 9574d451e16e5744a999ef41f2121c514fa9475d Mon Sep 17 00:00:00 2001 From: wade <280641290+wade19990814-hue@users.noreply.github.com> Date: Sat, 8 Aug 2026 19:32:10 +0800 Subject: [PATCH 2/2] fix(windows): use u64 instead of ptr for FFI HANDLE types, safer UTF-16 decode --- src/codex/user-identity.ts | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/codex/user-identity.ts b/src/codex/user-identity.ts index 068978cb36..2075c969a0 100644 --- a/src/codex/user-identity.ts +++ b/src/codex/user-identity.ts @@ -89,10 +89,10 @@ function resolveWindowsSid(): string { * popup bug), fast on the uncached hot path, and free of PATH trust questions. */ type WindowsProfileLibraries = { - getCurrentProcess: () => Pointer; - closeHandle: (handle: number) => number; - openProcessToken: (process: Pointer, desiredAccess: number, tokenOut: Pointer) => number; - getUserProfileDirectoryW: (token: number, buffer: Pointer, size: Pointer) => number; + getCurrentProcess: () => number | bigint; + closeHandle: (handle: number | bigint) => number; + openProcessToken: (process: number | bigint, desiredAccess: number, tokenOut: Pointer) => number; + getUserProfileDirectoryW: (token: number | bigint, buffer: Pointer, size: Pointer) => number; }; let windowsProfileLibrariesCache: WindowsProfileLibraries | null | undefined; @@ -105,18 +105,20 @@ function loadWindowsProfileLibraries(): WindowsProfileLibraries | null { } try { const kernel32 = dlopen("kernel32.dll", { - GetCurrentProcess: { args: [], returns: "ptr" }, - // Handles travel as pointer-sized integers. + // Windows HANDLE values are opaque 64-bit identifiers, not memory + // addresses — use u64 everywhere a HANDLE appears (Bun FFI "ptr" applies + // pointer-tagging logic that is inappropriate for handles). + GetCurrentProcess: { args: [], returns: "u64" }, CloseHandle: { args: ["u64"], returns: "i32" }, }); const advapi32 = dlopen("advapi32.dll", { - OpenProcessToken: { args: ["ptr", "u32", "ptr"], returns: "i32" }, + OpenProcessToken: { args: ["u64", "u32", "ptr"], returns: "i32" }, }); const userenv = dlopen("userenv.dll", { GetUserProfileDirectoryW: { args: ["u64", "ptr", "ptr"], returns: "i32" }, }); windowsProfileLibrariesCache = { - getCurrentProcess: () => kernel32.symbols.GetCurrentProcess() as Pointer, + getCurrentProcess: () => kernel32.symbols.GetCurrentProcess() as number | bigint, closeHandle: handle => kernel32.symbols.CloseHandle(handle) as number, openProcessToken: (process, desiredAccess, tokenOut) => advapi32.symbols.OpenProcessToken(process, desiredAccess, tokenOut) as number, @@ -134,7 +136,7 @@ function windowsProfileDirectory(): string { if (!libraries) { refuse("Windows profile resolution could not load system libraries."); } - let token = 0; + let token = 0n; let profile = ""; try { const TOKEN_QUERY = 0x0008; @@ -143,7 +145,7 @@ function windowsProfileDirectory(): string { if (opened === 0 || tokenOut[0] === 0n) { refuse("Windows profile resolution could not open the process token."); } - token = Number(tokenOut[0]); + token = tokenOut[0]; // Profile paths fit MAX_PATH; retry once with the reported size otherwise. let buffer = new Uint16Array(512); let size = new Uint32Array([buffer.length]); @@ -159,12 +161,13 @@ function windowsProfileDirectory(): string { } if (ok === 0) refuse("Windows profile resolution could not read the profile directory."); const length = buffer.indexOf(0); - profile = String.fromCharCode(...buffer.subarray(0, length < 0 ? buffer.length : length)); + const slice = buffer.subarray(0, length < 0 ? buffer.length : length); + profile = new TextDecoder("utf-16le").decode(slice); } catch (cause) { if (cause instanceof CodexUserIdentityRefusal) throw cause; refuse("Windows profile resolution failed.", cause); } finally { - if (token !== 0) { + if (token !== 0n) { try { libraries.closeHandle(token); } catch { /* best-effort handle close */ } } }