diff --git a/src/codex/desktop-app/types.ts b/src/codex/desktop-app/types.ts index 2a4d8774b4..c90862f329 100644 --- a/src/codex/desktop-app/types.ts +++ b/src/codex/desktop-app/types.ts @@ -116,11 +116,20 @@ export interface DesktopAppAdapter { * * `root` is expected to be `realpath`-resolved by discovery already. */ +function isMembershipSeparator(character: string): boolean { + // `/` separates on every platform this runs on, and Windows accepts it wherever it + // accepts `\`. `\` is only a separator where the host says so: it is a legal + // FILENAME character on POSIX, so admitting it there would reopen the sibling hole + // this function exists to close. + return character === "/" || (sep === "\\" && character === "\\"); +} + export function isUnderRoot(executable: string, root: string): boolean { if (!executable || !root) return false; if (executable === root) return true; - const prefix = root.endsWith(sep) ? root : root + sep; - return executable.startsWith(prefix); + if (!executable.startsWith(root)) return false; + if (isMembershipSeparator(root[root.length - 1]!)) return true; + return isMembershipSeparator(executable[root.length] ?? ""); } /** diff --git a/src/codex/desktop-app/windows.ts b/src/codex/desktop-app/windows.ts index 863a20057a..c73e067e5b 100644 --- a/src/codex/desktop-app/windows.ts +++ b/src/codex/desktop-app/windows.ts @@ -29,16 +29,16 @@ const SHELL_BASENAME = "chatgpt.exe"; const POWERSHELL_PROBE_OPTIONS = { timeout: PROBE_TIMEOUT_MS, windowsHide: true } as const; /** - * isUnderRoot prefixes with the host path.sep and is case-sensitive. Windows + * isUnderRoot checks a lexical path boundary and is case-sensitive. Windows * membership is case-insensitive, and this file is executed by Unix CI against - * backslash paths, so both sides are folded onto the host separator first. + * mixed slash paths, so both slash forms are folded onto the host separator first. * The boundary itself — sibling `OpenAI.Codex-evil` must not match root * `OpenAI.Codex` — is still isUnderRoot's, which is why the PowerShell * StartsWith is only a cheap pre-filter. */ function toHostMembershipPath(windowsPath: string): string { const lowered = windowsPath.toLowerCase(); - return sep === "\\" ? lowered : lowered.replaceAll("\\", "/"); + return lowered.replace(/[\\/]/g, sep); } function isMemberExecutable(executable: string, root: string): boolean { @@ -91,10 +91,10 @@ function listPackageProcesses(exec: DesktopExec, install: DesktopAppInstall): De const literal = install.root.replace(/'/g, "''"); const script = [ "$ErrorActionPreference='SilentlyContinue'", - `$root = '${literal}'`, + `$root = '${literal}'.Replace('/', '\\')`, "$me = ([Security.Principal.WindowsIdentity]::GetCurrent()).Name", "Get-CimInstance Win32_Process -Filter \"Name='ChatGPT.exe'\" |", - " Where-Object { $_.ExecutablePath -and $_.ExecutablePath.StartsWith($root, 'OrdinalIgnoreCase') } |", + " Where-Object { $_.ExecutablePath -and $_.ExecutablePath.Replace('/', '\\').StartsWith($root, 'OrdinalIgnoreCase') } |", " ForEach-Object {", " $o = Invoke-CimMethod -InputObject $_ -MethodName GetOwner", " if ($o -and $o.ReturnValue -eq 0 -and $o.User) {", diff --git a/structure/catalog.md b/structure/catalog.md index 2fd03722df..da7c81cff5 100644 --- a/structure/catalog.md +++ b/structure/catalog.md @@ -138,6 +138,8 @@ removal markers. These presentation operations do not grant routing or account e ## Startup readiness +When the desktop app is explicitly restarted to reload synchronized state, [process membership](runtime.md#codex-desktop-process-membership) is determined from its installation path; catalog model selectors do not identify restart targets. + Each `startServer` invocation owns a private, one-shot readiness gate created before the listener binds. `handleStart` supplies its gate and transitions it only after the shared catalog sync and best-effort Claude Code roster reconciliation have both settled. The catalog sync remains the diff --git a/structure/codex-home.md b/structure/codex-home.md index 339358ea9b..1a391dceb9 100644 --- a/structure/codex-home.md +++ b/structure/codex-home.md @@ -232,6 +232,8 @@ to snapshot persistence instead of relying on the progress argument alone. ## Codex-home diagnostics +Desktop executable membership uses the [discovered installation root](runtime.md#codex-desktop-process-membership), independently of the Codex state directory resolved here. + Some Codex-home conditions are reported rather than repaired, because repairing them would overwrite a deliberate user choice: diff --git a/structure/config.md b/structure/config.md index d00b36b06b..54ae5c11f5 100644 --- a/structure/config.md +++ b/structure/config.md @@ -74,6 +74,8 @@ management API. Retirement does not migrate user-selected model ids or erase usa ## Config injection +An explicit desktop restart after injection uses the [runtime process-membership contract](runtime.md#codex-desktop-process-membership); mixed Windows path spelling does not change which installation the restart targets. + `src/codex/inject.ts` writes one of two forms. The choice is not cosmetic: it decides whether Codex keeps its native provider id, which decides whether existing thread history still resolves. diff --git a/structure/gui-and-management-api.md b/structure/gui-and-management-api.md index 4628ca5e50..23fadc5b8b 100644 --- a/structure/gui-and-management-api.md +++ b/structure/gui-and-management-api.md @@ -241,7 +241,7 @@ unvalidated Bun builds is unchanged (`src/lib/bun-stream-caps.ts`). sidebar entry: it is entered from the dashboard's startup-state row, which links there whether the current state needs remediation or merely reports how routing is protected. Its warning state is derived from active Codex routing plus the actual service and launcher-shim installation state; the -`codexAutoStart` preference alone is never presented as proof of restart protection. The page shows +`codexAutoStart` preference alone is never presented as proof of restart protection. Desktop restart target selection follows the [runtime membership contract](runtime.md#codex-desktop-process-membership); finding an installed app does not establish background-service protection. The page shows copyable repair commands (`ocx service repair` for an installed service or `ocx service install` when none is registered, `ocx codex-shim install`, and `ocx restore`). On Windows it can also install an owned, per-user system tray. The resident tray owns only its icon, home-scoped singleton, and HKCU Run registration; fixed proxy actions delegate to the CLI so drain, @@ -458,7 +458,7 @@ estimated` split exists for, and why coverage is reported alongside totals. The main Dashboard surfaces a 30d token / coverage summary. The in-memory `requestLog` is capped at 200 entries and is **not** the source of truth for aggregation — the JSONL on disk is. -A row also carries what its logical request cost upstream. `logicalRequestId` names the turn +A row also records the upstream cost of its logical request. `logicalRequestId` names the turn that a retry leg, a repair refetch and a combo child all belong to, and `spend` aggregates their physical sends: `sends` totals every attempt on the row, `settled` counts the sends whose attempt reached a terminal status, and `unresolved` holds the rest — an attempt abandoned in flight, or a diff --git a/structure/ops/docs-and-release.md b/structure/ops/docs-and-release.md index 0c0ffe38d0..f7b5acd0ae 100644 --- a/structure/ops/docs-and-release.md +++ b/structure/ops/docs-and-release.md @@ -278,6 +278,8 @@ preview has closed that stable patch line. ## Cross-platform CI +The [desktop membership contract](../runtime.md#codex-desktop-process-membership) has adapter regression coverage on every host and real PowerShell prefilter regression coverage with synthetic CIM rows on Windows in `tests/clients/desktop-app-restart.test.ts`. A skipped Windows lane does not exercise that native filter; uid-dependent POSIX cases in `tests/clients/desktop-app-restart-posix.test.ts` are skipped on Windows. + `.github/workflows/ci.yml` is the ordinary quality gate for runtime/package changes. Linux runs the suite in four shards with a separate `gates` job, and macOS runs it in two shards. Windows runs the full suite in six shards only on manual `workflow_dispatch` with `lane=all` (or an diff --git a/structure/providers/openai-tiers.md b/structure/providers/openai-tiers.md index ab7511c3b0..98ed6ddcc3 100644 --- a/structure/providers/openai-tiers.md +++ b/structure/providers/openai-tiers.md @@ -431,6 +431,8 @@ Pool mode needs stable public names and a store that survives concurrent refresh ## Sidecars, management, and UI +The desktop restart adapter uses [Windows process ownership and installation membership](../runtime.md#codex-desktop-process-membership), independently of Pool/Direct credential selection. + HTTP/SSE, Responses WebSocket, compact, images, search, and vision resolve the same account mode. There is one mode-aware `openai` forward sidecar candidate; `openai-apikey` is not a ChatGPT-forward sidecar candidate and cannot hide a failed Codex credential with separately billed API usage. diff --git a/structure/runtime.md b/structure/runtime.md index bd9ebbd561..df16246065 100644 --- a/structure/runtime.md +++ b/structure/runtime.md @@ -24,6 +24,16 @@ Catalog-derived reasoning-level diagnostics are escaped only at the human-output When hub management ingress is enabled, `src/cli/dispatch.ts` opens the dashboard on the literal IPv4 loopback address and configured ingress port, matching the listener in `src/server/index.ts`. Other dashboard address selection is unchanged. +## Codex desktop process membership + +`src/codex/desktop-app/windows.ts` discovers the installed package and limits process ownership to the current Windows user. +Its PowerShell prefilter normalizes both the install root and candidate executable from `/` to `\` before a case-insensitive prefix comparison. +The adapter then folds both slash forms onto the host separator before calling `isUnderRoot()` in `src/codex/desktop-app/types.ts`. +That shared lexical boundary check rejects sibling prefixes such as `OpenAI.Codex-evil`; Windows path folding stays in the Windows adapter, so a POSIX backslash remains a filename character. +The prefilter is only an optimization, not final process-membership authority. +`tests/clients/desktop-app-restart.test.ts` covers both mixed-slash directions through the adapter and runs the real PowerShell filter against synthetic CIM rows on Windows. +`tests/clients/desktop-app-restart-posix.test.ts` keeps the POSIX separator contract covered; uid-dependent macOS/Linux cases skip on Windows. + ## Entrypoints | Path | Responsibility | diff --git a/structure/subagents.md b/structure/subagents.md index c0c92891f7..9926cbcfd2 100644 --- a/structure/subagents.md +++ b/structure/subagents.md @@ -265,6 +265,8 @@ cause delegation. The TOML edit owns only marker-tagged values, preserves existi user-owned `[agents]` defaults rather than overwriting them, and rejects ambiguous table shapes without changing the file. +An explicit desktop restart to load those defaults follows the [runtime membership checks](runtime.md#codex-desktop-process-membership); selecting a delegation model does not authorize additional restart targets. + V2 proxy guidance uses `` for both built-in metadata and custom `injectionPrompt` bodies. The built-in text reports the resolved preferred model, effort, roster and fallback chain without prescribing delegation, spawn overrides or diff --git a/tests/clients/desktop-app-restart-posix.test.ts b/tests/clients/desktop-app-restart-posix.test.ts index dd1bbd8ec6..585c86dcb7 100644 --- a/tests/clients/desktop-app-restart-posix.test.ts +++ b/tests/clients/desktop-app-restart-posix.test.ts @@ -97,9 +97,27 @@ describe("desktop restart membership is a path boundary, not a prefix", () => { expect(isUnderRoot("/usr/lib/chatgpt-evil/ChatGPT", "/usr/lib/chatgpt")).toBe(false); expect(isUnderRoot("/usr/lib/chatgpt/ChatGPT", "/usr/lib/chatgpt")).toBe(true); }); + + test("a forward slash separates on every host, a backslash only where the host says so", () => { + // Windows accepts `/` wherever it accepts `\`, and a probe can return either. Reading a + // forward-slash member as "outside the tree" is fail-closed but wrong: the restart the + // user asked for silently becomes a no-op. + expect(isUnderRoot("C:/Program Files/OpenAI.Codex/chatgpt.exe", "C:/Program Files/OpenAI.Codex")).toBe(true); + expect(isUnderRoot("C:/Program Files/OpenAI.Codex-evil/chatgpt.exe", "C:/Program Files/OpenAI.Codex")).toBe(false); + // The reverse is NOT symmetric. On POSIX a backslash is an ordinary filename + // character, so admitting it as a separator would reopen the sibling hole. + expect(isUnderRoot("/usr/lib/chatgpt\\evil", "/usr/lib/chatgpt")).toBe(process.platform === "win32"); + }); }); -describe("macOS desktop restart", () => { +/** + * The POSIX adapters scope enumeration to the current user through `process.getuid()`, + * which a Windows host does not provide. There the probe correctly reports that it could + * not run, so these cases cannot be driven from Windows at all - the shared ladder they + * exercise is covered by the Ubuntu and macOS shards. The membership and lock cases above + * have no such dependency and keep running everywhere. + */ +describe.skipIf(process.platform === "win32")("macOS desktop restart", () => { test("quits through the Apple event and relaunches by bundle id", () => { const calls: Call[] = []; const result = restartCodexDesktopApp(darwinIo({ calls })); @@ -196,7 +214,7 @@ describe("macOS desktop restart", () => { }); }); -describe("a stop is only ever claimed when the enumeration agrees (measured on Windows)", () => { +describe.skipIf(process.platform === "win32")("a stop is only ever claimed when the enumeration agrees (measured on Windows)", () => { // The defect this pins was invisible to ten rounds of code review and surfaced in the // first thirty seconds of running the ladder on a real Windows host: it reported // {"stopped":[27788],"surviving":[],"relaunch":"started"} while the app kept its @@ -325,4 +343,3 @@ describe("a restart already in flight does not start a second one", () => { expect(calls).toEqual([]); }); }); - diff --git a/tests/clients/desktop-app-restart.test.ts b/tests/clients/desktop-app-restart.test.ts index 4df9c7a6e9..fd7ae597dc 100644 --- a/tests/clients/desktop-app-restart.test.ts +++ b/tests/clients/desktop-app-restart.test.ts @@ -1,8 +1,10 @@ import { describe, expect, test } from "bun:test"; +import { execFileSync } from "node:child_process"; import { mkdtempSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { restartCodexDesktopApp, type DesktopAppRestartIo } from "../../src/codex/desktop-app-restart"; +import { windowsDesktopAppAdapter } from "../../src/codex/desktop-app/windows"; import { setTrustedWindowsElevationExecutablesForTests } from "../../src/lib/windows-elevation"; /** @@ -27,6 +29,53 @@ function withTrustedExes(run: () => T): T { interface Call { file: string; args: string[] } +describe.skipIf(process.platform !== "win32")("Windows membership through the real PowerShell prefilter", () => { + for (const [label, root, executable] of [ + ["forward-slash images under a backslash root", INSTALL, INSTALL.replaceAll("\\", "/") + "/ChatGPT.exe"], + ["backslash images under a forward-slash root", INSTALL.replaceAll("\\", "/"), INSTALL + "\\ChatGPT.exe"], + ] as const) { + test(label, () => { + const sibling = executable.replace(/([\\/])ChatGPT\.exe$/, "-evil$1ChatGPT.exe"); + const psLiteral = (value: string) => "'" + value.replaceAll("'", "''") + "'"; + const fixture = [ + // These functions shadow the CIM cmdlets: the generated list-only script + // sees synthetic rows and never enumerates or controls real processes. + "function Get-CimInstance {", + " param([string]$ClassName, [string]$Filter)", + " if ($ClassName -cne 'Win32_Process' -or $Filter -cne \"Name='ChatGPT.exe'\") { throw 'Unexpected fixture query' }", + " @(", + ` [pscustomobject]@{ ProcessId = 1000; ParentProcessId = 900; CreationDate = [datetime]'2026-09-15T00:00:00Z'; ExecutablePath = ${psLiteral(executable)} }`, + ` [pscustomobject]@{ ProcessId = 2000; ParentProcessId = 900; CreationDate = [datetime]'2026-09-15T00:00:00Z'; ExecutablePath = ${psLiteral(sibling)} }`, + " )", + "}", + "function Invoke-CimMethod {", + " param($InputObject, [string]$MethodName)", + " if ($MethodName -cne 'GetOwner' -or $InputObject.ProcessId -notin @(1000, 2000)) { throw 'Unexpected fixture owner query' }", + " [pscustomobject]@{ ReturnValue = 0; Domain = ''; User = ([Security.Principal.WindowsIdentity]::GetCurrent()).Name }", + "}", + ].join("\n"); + let rawListing = ""; + const listed = withTrustedExes(() => windowsDesktopAppAdapter.listProcesses((file, args) => { + expect(file).toBe(PS); + expect(args.slice(0, 3)).toEqual(["-NoProfile", "-NonInteractive", "-Command"]); + const script = args[3]!; + rawListing = execFileSync(file, [...args.slice(0, 3), fixture + "\n" + script], { + encoding: "utf8", + timeout: 10_000, + windowsHide: true, + }); + return rawListing; + }, { id: AUMID.replace("!App", ""), root, relaunch: AUMID })); + // The real prefilter admits both lexical prefixes despite mixed slashes. + // The shared JS boundary check then removes the similarly named sibling. + expect(rawListing.trim().split(/\r?\n/).map(line => Number(line.split(" ")[0]))).toEqual([1000, 2000]); + expect(listed?.map(entry => ({ pid: entry.pid, executable: entry.executable }))).toEqual([ + { pid: 1000, executable }, + ]); + }, 15_000); + } +}); + /** Scripted exec seam: discovery, then process list, then whatever the branch does. */ /** * A lock path this case owns. The restart takes a singleton lock, so a case using the @@ -141,6 +190,34 @@ describe("Codex desktop app restart (#2292)", () => { expect(launch?.args.join(" ")).toContain(AUMID); }); + + for (const [label, root, executable, isMember] of [ + ["forward-slash executable under backslash root", INSTALL, INSTALL.replaceAll("\\", "/") + "/ChatGPT.exe", true], + ["backslash executable under forward-slash root", INSTALL.replaceAll("\\", "/"), INSTALL + "\\ChatGPT.exe", true], + ["forward-slash sibling outside backslash root", INSTALL, INSTALL.replaceAll("\\", "/") + "-evil/ChatGPT.exe", false], + ["backslash sibling outside forward-slash root", INSTALL.replaceAll("\\", "/"), INSTALL + "-evil\\ChatGPT.exe", false], + ] as const) { + test(label, () => { + const calls: Call[] = []; + const result = withTrustedExes(() => restartCodexDesktopApp(scriptedIo({ + discovery: [AUMID.replace("!App", ""), root, AUMID].join("\n"), + processes: `1000 900 T0 ${executable}`, + calls, + aliveFor: (_pid, poll) => poll <= 2, + }))); + if (isMember) { + expect(result).toEqual({ attempted: true, stopped: [1000], surviving: [], relaunch: "started" }); + expect(calls.some(c => c.args.join(" ").includes("CloseMainWindow"))).toBe(true); + } else { + expect(result.reason).toBe("no_targets"); + expect(result.attempted).toBe(false); + expect(calls.some(c => c.args.join(" ").includes("CloseMainWindow"))).toBe(false); + expect(calls.some(c => c.args.join(" ").includes("Start-Process"))).toBe(false); + } + expect(calls.some(c => c.file === TASKKILL)).toBe(false); + }); + } + test("forces only after the graceful window elapses", () => { const calls: Call[] = []; const result = withTrustedExes(() => restartCodexDesktopApp(scriptedIo({