From f899cc4be2d84b768e2d79b2e865374251e72caf Mon Sep 17 00:00:00 2001 From: Epinephrine Date: Fri, 23 Oct 2026 19:17:05 +0900 Subject: [PATCH 01/16] feat(codex): add handle-pinned Windows CLI update job store Introduce cli-update-job-store-windows.ts as the NTFS backend for the durable codex-cli-update ledger: ancestors are opened relative to the held volume-root pin under trusted-ancestry and write-private ACL policies, the mkdir plus owner-record admission mutex releases only while held pins still show this call identity, and ledger writes publish through a private hardened temp that is byte, ACL and identity verified before and after the handle-bound rename. cli-update-job.ts exposes async admit, read, finish and run and dispatches to the backend through a lazy import so the FFI module stays out of the POSIX graph; cli-update-job-execution.ts shares the store bounds, lets JobWork return promises, and makes the retry helper await the operation. CLI status and apply and the management routes await the async APIs. windows-owner-acl.ts gains a shared strict line-protocol parser with per-entry private, write-private and trusted-ancestry policies plus an async verifier whose bounded PowerShell read abandons a timed-out lookup child instead of waiting for reap, which exists for handle-bearing icacls children; confined-policy trusted ownership no longer depends on the serving token elevation state. Backend is x64-only; other-SID and ARM64 behavior is unverified. --- scripts/test-layout/layout.json | 1 + src/cli/codex-cli-update.ts | 6 +- src/codex/cli-update-job-execution.ts | 25 +- src/codex/cli-update-job-store-windows.ts | 653 ++++++++++++++++++ src/codex/cli-update-job.ts | 129 ++-- src/lib/windows-owner-acl.ts | 263 ++++++- .../management/codex-cli-update-routes.ts | 4 +- structure/INDEX.md | 1 + structure/cli-management.md | 1 + structure/codex-cli-update.md | 2 + structure/manifest.json | 1 + structure/remote-link.md | 2 + tests/cli/cli-codex-cli-update.test.ts | 113 +-- .../codex-cli-update-apply.test.ts | 4 +- tests/fixtures/file-size-baseline.json | 1 + tests/fixtures/test-layout-expected.json | 1 + .../windows-codex-cli-job-store.test.ts | 316 +++++++++ 17 files changed, 1383 insertions(+), 140 deletions(-) create mode 100644 src/codex/cli-update-job-store-windows.ts create mode 100644 tests/windows/windows-codex-cli-job-store.test.ts diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index f26d909ca71..edbab9503fe 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -1929,6 +1929,7 @@ "win-paths.test.ts": "windows", "windows-acl-start-cost.test.ts": "windows", "windows-atomic-replace.test.ts": "windows", + "windows-codex-cli-job-store.test.ts": "windows", "windows-deploy-close-regressions.test.ts": "windows", "windows-elevation-spawn.test.ts": "windows", "windows-elevation.test.ts": "windows", diff --git a/src/cli/codex-cli-update.ts b/src/cli/codex-cli-update.ts index 5faf4576cd1..06ebf084336 100644 --- a/src/cli/codex-cli-update.ts +++ b/src/cli/codex-cli-update.ts @@ -309,8 +309,8 @@ export async function handleCodexCliUpdateCommand( return; } if (parsed.action === "status") { - let job: ReturnType; - try { job = (deps.readJob ?? readCodexCliUpdateJob)(parsed.jobId); } + let job: Awaited>; + try { job = await (deps.readJob ?? readCodexCliUpdateJob)(parsed.jobId); } catch { unavailableJob(); return; } if (!job) { unavailableJob(); return; } printData(job, parsed.json, [`job: ${job.jobId}`, `status: ${job.status}`, `outcome: ${job.outcome ?? "pending"}`, @@ -328,7 +328,7 @@ export async function handleCodexCliUpdateCommand( } if (parsed.action === "apply") { const apply = deps.applyUpdate ?? (await import("../codex/cli-update-apply")).applyCodexCliUpdate; - const admitted = admitCodexCliUpdateJob({ requestId: parsed.requestId, planId: parsed.planId, confirmed: true }); + const admitted = await admitCodexCliUpdateJob({ requestId: parsed.requestId, planId: parsed.planId, confirmed: true }); const job = admitted.kind === "accepted" ? await runCodexCliUpdateJob(admitted.job.jobId, admitted.generation, () => apply(parsed.planId, { inspectionDeps: codexCliUpdateInspectionDeps(deps) })) diff --git a/src/codex/cli-update-job-execution.ts b/src/codex/cli-update-job-execution.ts index e3ae63ed765..90dea74ccd4 100644 --- a/src/codex/cli-update-job-execution.ts +++ b/src/codex/cli-update-job-execution.ts @@ -13,6 +13,11 @@ export const CODEX_CLI_JOB_CLEANUP_ERRORS: ReadonlySet = new Set(["lease export class CodexCliJobStoreBusyError extends Error { constructor() { super("job_store_busy"); } } +/** Durable ledger bounds and names shared by the POSIX and Windows store backends. */ +export const CODEX_CLI_JOB_STORE_DIR = "codex-cli-update-jobs"; +export const CODEX_CLI_JOB_STORE_FILE = "jobs.json"; +export const CODEX_CLI_JOB_STORE_LOCK = "admission.lock"; +export const CODEX_CLI_JOB_STORE_MAX_BYTES = 256 * 1024; export class CodexCliJobResultPersistenceError extends Error { constructor(readonly result: CodexCliApplyResult) { super("job_result_persistence_failed"); } } @@ -29,7 +34,7 @@ export async function retryCodexCliJobStoreOperation(operation: () => T, delay: (milliseconds: number) => Promise = ms => new Promise(resolve => setTimeout(resolve, ms)), persistence = false): Promise { for (let attempt = 0; ; attempt++) { - try { return operation(); } + try { return await operation(); } catch (error) { const transientIo = persistence && ["EIO", "ENOSPC", "EBUSY", "EAGAIN", "EINTR", "EMFILE", "ENFILE"] .includes((error as NodeJS.ErrnoException)?.code ?? ""); @@ -39,20 +44,20 @@ export async function retryCodexCliJobStoreOperation(operation: () => T, } } interface JobWork { - begin(): number; - finish(revision: number, result: CodexCliApplyResult): CodexCliUpdateJobDto | null; - read(): CodexCliUpdateJobDto | null; - release(): void; + begin(): number | Promise; + finish(revision: number, result: CodexCliApplyResult): CodexCliUpdateJobDto | null | Promise; + read(): CodexCliUpdateJobDto | null | Promise; + release(): void | Promise; } /** Execution and persistence are separate ownership phases. */ export async function runCodexCliUpdateJobWork(run: () => Promise, work: JobWork): Promise { let released = false; try { let revision: number; - try { revision = await retryCodexCliJobStoreOperation(() => work.begin()); } + try { revision = await retryCodexCliJobStoreOperation(async () => work.begin()); } catch { - work.release(); released = true; - try { return work.read(); } + await work.release(); released = true; + try { return await work.read(); } catch (error) { const code = error instanceof Error ? error.message : ""; throw new Error(["unsafe_store", "invalid_store", "unsupported_store"].includes(code) ? code : "job_state_unavailable"); @@ -66,7 +71,7 @@ export async function runCodexCliUpdateJobWork(run: () => Promise work.finish(revision, result) ?? work.read(), undefined, true); + return await retryCodexCliJobStoreOperation(async () => (await work.finish(revision, result)) ?? (await work.read()), undefined, true); } catch { throw new CodexCliJobResultPersistenceError(result); } - } finally { if (!released) work.release(); } + } finally { if (!released) await work.release(); } } diff --git a/src/codex/cli-update-job-store-windows.ts b/src/codex/cli-update-job-store-windows.ts new file mode 100644 index 00000000000..f94a1eaf701 --- /dev/null +++ b/src/codex/cli-update-job-store-windows.ts @@ -0,0 +1,653 @@ +/** + * Windows backend for the Codex CLI update job ledger. + * + * Parallel contract to the POSIX primitives in cli-update-job.ts, mapped onto NTFS + * handle semantics instead of uid/mode/O_NOFOLLOW: + * + * - Ancestry: the volume root is opened through its GUID mount path and pinned + * as the trust anchor; its own ACL is not part of the batch — the held handle, + * not a policy verdict, is what anchors the chain. Every directory below it + * down to the config parent is opened handle-relative from the already-held + * parent (never a re-parsed path), held with FILE_SHARE_READ|FILE_SHARE_WRITE + * so nothing can rename or delete it while held, and refused on any reparse + * attribute. Each of those directories has its ACL read once per phase through + * the shared PowerShell protocol under the "trusted-ancestry" policy: the + * owner must be the serving token or a kernel-equivalent owner + * (SYSTEM, Administrators, TrustedInstaller — each can already take ownership + * of any object, so an ACL entry grants them nothing they lack), and no other + * grantee may hold write, create-inside, delete or DACL/owner control. Broad + * create-only grants are accepted only on admin-owned ancestors, which is the + * NTFS analog of a root-owned sticky directory: the stock C:\ ACL grants + * Authenticated Users add-subdirectory and nothing else. + * - The config root's parent must be owned by the serving token and grant no + * untrusted write access ("write-private", the POSIX mode&022==0 analog). A + * pre-existing config root is verified only — never rehardened — under the + * same policy. A config root or store directory created by this call is + * icacls-hardened and then held to the strict "private" rule. + * - The store directory and every ledger file must satisfy strict "private": + * a protected DACL owned by the serving token with grantees limited to the + * user plus SYSTEM/Administrators — the same shape the POSIX 0700/0600 modes + * express. + * - File identity is the NTFS volume serial + 128-bit file ID (the dev:ino + * analog), with NumberOfLinks == 1 and DeletePending clear, mirroring the + * POSIX nlink and lstat-vs-fstat checks. + * - Writes go through a private temp file: exclusive create, harden, fsync, + * identity+ACL+byte verify, handle-bound rename, then identity/ACL/byte + * readback on the renamed object and a directory flush. The temp handle + * denies foreign write/delete opens, so the object renamed is provably the + * one written — and the destination is never opened deny-delete, which would + * otherwise block the rename itself. + * - The admission lock is a mkdir mutex like POSIX: EEXIST maps to + * job_store_busy with no reclamation. owner.json is written under the held + * lock pin; release deletes the owner file and lock directory only through + * the pins whose identity and content still match what this call created, + * so a replacement is never removed by this process and a leftover lock + * blocks later admission, exactly like POSIX. + * + * Honesty notes: pins bind each object to its verified state for the length of + * the operation and the private DACL restricts writers to the trusted set; that + * is the strongest binding Windows affords, but it is not a proof against + * concurrent mutation by the serving account or an elevated process, which sit + * inside the trust boundary exactly as on POSIX. Each mutation step is bracketed + * by a fresh identity observation or ACL read rather than claimed TOCTOU-free. + * x64 only: the FFI layouts below are the x64 ABI. Cross-account (other-SID) + * behavior is unverified: the DACL policy is the enforcement claim, not an + * empirical cross-user proof. + */ +import { randomUUID } from "node:crypto"; +import { closeSync, fsyncSync, mkdirSync, openSync, unlinkSync, writeFileSync } from "node:fs"; +import { join, resolve } from "node:path"; +import type { Library, Symbols } from "bun:ffi"; +import { getConfigDir } from "../config"; +import { assertNotRealHomeUnderTest } from "../lib/test-home-guard"; +import { forgetEphemeralSecretPath, hardenSecretDirAsync, hardenSecretPathAsync } from "../lib/windows-secret-acl"; +import { verifyWindowsEntriesAclAsync, type WindowsAclEntry } from "../lib/windows-owner-acl"; +import { retryWindowsFileOperation } from "../lib/windows-atomic-replace"; +import { + CODEX_CLI_JOB_STORE_DIR, CODEX_CLI_JOB_STORE_FILE, CODEX_CLI_JOB_STORE_LOCK, + CODEX_CLI_JOB_STORE_MAX_BYTES, CodexCliJobStoreBusyError, +} from "./cli-update-job-execution"; + +const KERNEL_SYMBOLS = { + GetDriveTypeW: { args: ["ptr"], returns: "u32" }, + GetVolumeNameForVolumeMountPointW: { args: ["ptr", "ptr", "u32"], returns: "i32" }, + CreateFileW: { args: ["ptr", "u32", "u32", "ptr", "u32", "u32", "u64"], returns: "u64" }, + GetFileInformationByHandleEx: { args: ["u64", "i32", "ptr", "u32"], returns: "i32" }, + SetFileInformationByHandle: { args: ["u64", "i32", "ptr", "u32"], returns: "i32" }, + GetFileType: { args: ["u64"], returns: "u32" }, + ReadFile: { args: ["u64", "ptr", "u32", "ptr", "ptr"], returns: "i32" }, + SetFilePointerEx: { args: ["u64", "i64", "ptr", "u32"], returns: "i32" }, + FlushFileBuffers: { args: ["u64"], returns: "i32" }, + CloseHandle: { args: ["u64"], returns: "i32" }, +} as const satisfies Symbols; +const NT_SYMBOLS = { + NtCreateFile: { args: ["ptr", "u32", "ptr", "ptr", "ptr", "u32", "u32", "u32", "u32", "ptr", "u32"], returns: "i32" }, +} as const satisfies Symbols; + +const REPARSE = 0x400; +const DIRECTORY_ATTR = 0x10; +const INVALID_HANDLE = 0xffffffffffffffffn; +/** Directory pins deny delete (and therefore rename) but allow read/write opens. */ +const SHARE_DIR = 0x3; // FILE_SHARE_READ | FILE_SHARE_WRITE +/** Ledger-file pins deny write and delete opens; only reads are shared. */ +const SHARE_LEAF = 0x1; // FILE_SHARE_READ +/** Read-verify opens must also permit a held delete-capable sibling pin to coexist. */ +const SHARE_LEAF_VERIFY = 0x5; // FILE_SHARE_READ | FILE_SHARE_DELETE +// SYNCHRONIZE | FILE_READ_ATTRIBUTES | FILE_READ_DATA — ancestors and read-only anchors. +const ACCESS_READ = 0x100081; +// SYNCHRONIZE | READ_CONTROL | DELETE | FILE_WRITE_ATTRIBUTES | FILE_READ_ATTRIBUTES +// | FILE_DELETE_CHILD | FILE_ADD_SUBDIRECTORY | FILE_ADD_FILE | FILE_LIST_DIRECTORY +// Owned directories we may flush or remove; the DACL we verified grants them anyway. +const ACCESS_DIR_WRITE = 0x100000 | 0x20000 | 0x10000 | 0x100 | 0x80 | 0x40 | 0x4 | 0x2 | 0x1; +// The store directory pin needs the same write side minus DELETE: it is flushed and +// used as a rename root but is never itself deleted. Holding DELETE would force every +// coexisting opener (e.g., a concurrent status reader) to offer share-delete. +const ACCESS_DIR_PIN = ACCESS_DIR_WRITE & ~0x10000; +// SYNCHRONIZE | READ_CONTROL | DELETE | FILE_READ_ATTRIBUTES | FILE_READ_DATA — ledger +// files held for read/verify/disposition-delete. +const ACCESS_LEAF = 0x130081; +const ACL_VERIFY_TIMEOUT_MS = 5_000; +const FILE_OPEN = 1; +// FILE_OPEN_REPARSE_POINT | FILE_SYNCHRONOUS_IO_NONALERT, plus the object-kind bit. +const DIR_OPTIONS = 0x200020 | 0x1; +const FILE_OPTIONS = 0x200020 | 0x40; + +interface StoreIdentity { readonly volumeSerial: string; readonly fileId: string } +interface StoreInspection extends StoreIdentity { + readonly size: number; + readonly links: number; + readonly deletePending: boolean; +} + +interface Native { + ffi: typeof import("bun:ffi"); + kernel: Library; + nt: Library; +} + +let nativePromise: Promise | undefined; +function loadNative(): Promise { + if (process.platform !== "win32" || process.arch !== "x64") { + return Promise.reject(new Error("unsupported_store")); + } + nativePromise ??= (async () => { + const ffi = await import("bun:ffi"); + return { ffi, kernel: ffi.dlopen("kernel32.dll", KERNEL_SYMBOLS), nt: ffi.dlopen("ntdll.dll", NT_SYMBOLS) }; + })().catch(() => { nativePromise = undefined; throw new Error("unsupported_store"); }); + return nativePromise; +} + +const unsafe = (): Error => new Error("unsafe_store"); +const enoent = (): NodeJS.ErrnoException => Object.assign(new Error("ENOENT"), { code: "ENOENT" }); + +class PinSet { + private readonly handles: bigint[] = []; + keep(handle: bigint): bigint { this.handles.push(handle); return handle; } + release(handle: bigint, kernel: Library): void { + const index = this.handles.lastIndexOf(handle); + if (index >= 0) this.handles.splice(index, 1); + kernel.symbols.CloseHandle(handle); + } + closeAll(kernel: Library): void { + for (const handle of this.handles.splice(0).reverse()) kernel.symbols.CloseHandle(handle); + } +} + +interface Prepared { + readonly config: string; + readonly dir: string; + readonly file: string; + readonly lock: string; + /** Write-access handle on the store directory; anchors leaf opens and the rename flush. */ + readonly jobdir: bigint; +} + +function components(path: string): { drive: string; names: string[] } | null { + if (typeof path !== "string" || path.length > 8192 || !/^[A-Za-z]:[\\/]/.test(path)) return null; + const names = path.slice(3).split(/[\\/]/); + if (!names.length || names.length > 64 || names.some(name => !name || name === "." || name === ".." + || /[\x00-\x1f<>:"|?*]/.test(name) || /[. ]$/.test(name) + || /^(?:con|prn|aux|nul|com[1-9\xb9\xb2\xb3]|lpt[1-9\xb9\xb2\xb3])(?:\.|$)/i.test(name))) return null; + return { drive: path.slice(0, 2).toUpperCase() + "\\", names }; +} + +interface Ctx { + readonly ffi: typeof import("bun:ffi"); + readonly kernel: Library; + readonly nt: Library; + readonly pins: PinSet; +} + +const wide = (text: string): Buffer => Buffer.from(text + "\0", "utf16le"); + +function keep(ctx: Ctx, handle: bigint): bigint { + if (handle === 0n || handle === INVALID_HANDLE) throw unsafe(); + return ctx.pins.keep(handle); +} + +function info(ctx: Ctx, handle: bigint, infoClass: number, length: number): Buffer { + const buffer = Buffer.alloc(length); + if (!ctx.kernel.symbols.GetFileInformationByHandleEx!(handle, infoClass, ctx.ffi.ptr(buffer), length)) { + throw unsafe(); + } + return buffer; +} + +/** Full identity/type observation for one held handle; kind is asserted by the caller. */ +function inspect(ctx: Ctx, handle: bigint, directory: boolean): StoreInspection { + if (ctx.kernel.symbols.GetFileType!(handle) !== 1) throw unsafe(); // FILE_TYPE_DISK + const basic = info(ctx, handle, 0, 40); // FILE_BASIC_INFO: 4 LARGE_INTEGERs, attributes. + const attributes = basic.readUInt32LE(32); + if (attributes & REPARSE) throw unsafe(); + if (Boolean(attributes & DIRECTORY_ATTR) !== directory) throw unsafe(); + const id = info(ctx, handle, 18, 24); // FILE_ID_INFO: volume serial and 128-bit file ID. + const standard = info(ctx, handle, 1, 24); // FILE_STANDARD_INFO. + const size = standard.readBigInt64LE(8); + if (size < 0n || size > BigInt(Number.MAX_SAFE_INTEGER)) throw unsafe(); + if (standard.readUInt8(20) !== 0) throw unsafe(); // DeletePending is one byte; offset 21 is Directory. + return { volumeSerial: id.readBigUInt64LE(0).toString(16), fileId: id.subarray(8, 24).toString("hex"), + size: Number(size), links: standard.readUInt32LE(16), deletePending: false }; +} + +function sameIdentity(a: StoreIdentity, b: StoreIdentity): boolean { + return a.volumeSerial === b.volumeSerial && a.fileId === b.fileId; +} + +/** Handle-relative open of one component below an already-pinned parent. */ +function relOpen( + ctx: Ctx, + parent: bigint, + component: string, + directory: boolean, + access: number, + share: number, +): bigint { + const name = wide(component); + const unicode = Buffer.alloc(16); + unicode.writeUInt16LE(name.length - 2, 0); + unicode.writeUInt16LE(name.length, 2); + unicode.writeBigUInt64LE(BigInt(ctx.ffi.ptr(name)), 8); + const attributes = Buffer.alloc(48); + attributes.writeUInt32LE(48, 0); + attributes.writeBigUInt64LE(parent, 8); + attributes.writeBigUInt64LE(BigInt(ctx.ffi.ptr(unicode)), 16); + attributes.writeUInt32LE(0x40, 24); // OBJ_CASE_INSENSITIVE; one component only. + const output = Buffer.alloc(8); + const status = Buffer.alloc(16); + const result = ctx.nt.symbols.NtCreateFile!(ctx.ffi.ptr(output), access, ctx.ffi.ptr(attributes), + ctx.ffi.ptr(status), null, 0, share, FILE_OPEN, directory ? DIR_OPTIONS : FILE_OPTIONS, null, 0); + if (result < 0) { + const status32 = result >>> 0; + if (status32 === 0xc0000034 || status32 === 0xc000003a) throw enoent(); // object/path not found + throw unsafe(); + } + const handle = keep(ctx, output.readBigUInt64LE(0)); + return handle; +} + +/** Volume root via the strict GUID mount path; remote or absent volumes refuse. */ +function openVolumeRoot(ctx: Ctx, drive: string): bigint { + const mount = wide(drive); + const name = Buffer.alloc(100 * 2); + if (![2, 3, 5, 6].includes(Number(ctx.kernel.symbols.GetDriveTypeW!(ctx.ffi.ptr(mount))))) { + throw unsafe(); + } + if (!ctx.kernel.symbols.GetVolumeNameForVolumeMountPointW!(ctx.ffi.ptr(mount), ctx.ffi.ptr(name), 100)) { + throw unsafe(); + } + const guid = name.toString("utf16le").split("\0", 1)[0]!; + if (!/^\\\\\?\\Volume\{[0-9a-f-]{36}\}\\$/i.test(guid)) throw unsafe(); + const path = wide(guid); + const handle = keep(ctx, BigInt(ctx.kernel.symbols.CreateFileW!(ctx.ffi.ptr(path), ACCESS_READ, + SHARE_DIR, null, 3, 0x02200000, 0n))); + inspect(ctx, handle, true); + return handle; +} + +/** Read up to limit bytes from a held file handle, then require exact end-of-file. */ +function readAll(ctx: Ctx, handle: bigint, limit: number): Buffer { + // Held pins are re-read across phases — owner.json is re-read at release after + // the acquire-time read left the position at EOF — so rewind first, or a second + // readAll on the same handle sees an empty file and fails closed. + if (!ctx.kernel.symbols.SetFilePointerEx!(handle, 0n, null, 0)) throw unsafe(); // FILE_BEGIN + const bytes = Buffer.alloc(limit); + const chunk = Buffer.alloc(Math.min(1024 * 1024, Math.max(1, limit))); + const read = Buffer.alloc(4); + let offset = 0; + while (offset < limit) { + const length = Math.min(chunk.length, limit - offset); + if (!ctx.kernel.symbols.ReadFile!(handle, ctx.ffi.ptr(chunk), length, ctx.ffi.ptr(read), null)) { + throw unsafe(); + } + const count = read.readUInt32LE(0); + if (!count || count > length) throw unsafe(); + chunk.copy(bytes, offset, 0, count); + offset += count; + } + // Any byte past the bound means the file changed while held open. + if (!ctx.kernel.symbols.ReadFile!(handle, ctx.ffi.ptr(chunk), 1, ctx.ffi.ptr(read), null) + || read.readUInt32LE(0) !== 0) throw unsafe(); + return bytes; +} + +function flushDir(ctx: Ctx, handle: bigint): void { + if (!ctx.kernel.symbols.FlushFileBuffers!(handle)) throw unsafe(); +} + +/** Mark a held object delete-pending; removal completes when the handle closes. */ +function markDelete(ctx: Ctx, handle: bigint): void { + const d = Buffer.alloc(1); + d.writeUInt8(1, 0); + if (!ctx.kernel.symbols.SetFileInformationByHandle!(handle, 4, ctx.ffi.ptr(d), 1)) throw unsafe(); +} + +/** + * Rename the held file to an absolute destination. Handle-bound, so the object + * moved is provably the one verified; the verbatim \?\ path is the fully + * qualified form the API parses unambiguously. FileNameLength counts the string + * INCLUDING its terminator — this API reads the NUL inside the counted span, + * and a shorter count makes it read past the buffer into adjacent memory. + * ReplaceIfExists matches POSIX rename. A sharing-violation blink (scanner/AV + * indexing) retries through the shared bounded helper; a foreign deny-delete + * pin on the destination fails closed. + */ +function renamePinned(ctx: Ctx, handle: bigint, destination: string): void { + const run = () => { + const name = wide("\\\\?\\" + destination); + const buffer = Buffer.alloc(20 + name.length); + buffer.writeUInt32LE(1, 0); // ReplaceIfExists + buffer.writeBigUInt64LE(0n, 8); // RootDirectory: NULL means absolute FileName. + buffer.writeUInt32LE(name.length, 16); // Terminator-inclusive length. + name.copy(buffer, 20); + if (!ctx.kernel.symbols.SetFileInformationByHandle!(handle, 3, ctx.ffi.ptr(buffer), buffer.length)) { + const error = unsafe() as NodeJS.ErrnoException; + error.code = "EBUSY"; + throw error; + } + }; + retryWindowsFileOperation({ run }); +} + +async function verifyAcl(entries: readonly WindowsAclEntry[]): Promise { + if (!await verifyWindowsEntriesAclAsync(entries, ACL_VERIFY_TIMEOUT_MS)) throw unsafe(); +} + +interface ChainEntry { + readonly path: string; + readonly handle: bigint; + readonly inspection: StoreInspection; +} + +/** + * Pin every component of an absolute path, volume root down, each opened relative + * to the held parent so no checked-then-reopened gap exists. leafAccess applies + * to the final component; all ancestors are read-pinned deny-delete. + */ +function pinChain(ctx: Ctx, absolutePath: string, leafAccess: number): ChainEntry[] { + const parsed = components(absolutePath); + if (!parsed) throw unsafe(); + const chain: ChainEntry[] = []; + let parent = openVolumeRoot(ctx, parsed.drive); + let current = parsed.drive; + for (let index = 0; index < parsed.names.length; index++) { + const access = index === parsed.names.length - 1 ? leafAccess : ACCESS_READ; + const handle = relOpen(ctx, parent, parsed.names[index]!, true, access, SHARE_DIR); + const inspection = inspect(ctx, handle, true); + current = join(current, parsed.names[index]!); + chain.push({ path: current, handle, inspection }); + parent = handle; + } + return chain; +} + +/** + * Open and trust the ledger tree: trusted ancestry to the config parent, the + * write-private parent and config root, then the strict-private store directory. + * Ancestors are verified before anything is created; EEXIST never rehardens — + * only objects this call creates get hardened, everything else is verify-only. + * Returns null only for a read flow whose store does not exist yet. + */ +async function prepareStore( + ctx: Ctx, + absoluteConfig: string, + opts: { createJobDir: boolean; createConfigRoot: boolean }, +): Promise { + const config = absoluteConfig; + const dir = join(config, CODEX_CLI_JOB_STORE_DIR); + const file = join(dir, CODEX_CLI_JOB_STORE_FILE); + const lock = join(dir, CODEX_CLI_JOB_STORE_LOCK); + const parentPath = join(config, ".."); + const chain = pinChain(ctx, resolve(parentPath), ACCESS_READ); + const ancestry: WindowsAclEntry[] = chain.map((entry, index) => ({ + path: entry.path, + directory: true, + policy: index === chain.length - 1 ? "write-private" : "trusted-ancestry", + })); + // Trust the ancestry before any write, matching the POSIX order. + await verifyAcl(ancestry); + const parentPin = chain.at(-1); + if (!parentPin) throw unsafe(); + const configName = config.slice(parentPath.length + 1); + let configPin: bigint; + let configPrivate = false; + try { + configPin = keep(ctx, relOpen(ctx, parentPin.handle, configName, true, ACCESS_READ, SHARE_DIR)); + inspect(ctx, configPin, true); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ENOENT" || !opts.createConfigRoot) { + if ((error as NodeJS.ErrnoException).code === "ENOENT" && !opts.createJobDir) return null; + throw error; + } + // Only a confirmed apply admission may create one absent config-root level. + mkdirSync(config); + configPin = keep(ctx, relOpen(ctx, parentPin.handle, configName, true, ACCESS_READ, SHARE_DIR)); + inspect(ctx, configPin, true); + await hardenSecretDirAsync(config, { required: true }); + configPrivate = true; + } + await verifyAcl([{ path: config, directory: true, policy: configPrivate ? "private" : "write-private" }]); + const jobdirAccess = opts.createJobDir ? ACCESS_DIR_PIN : ACCESS_READ; + let jobdir: bigint; + try { + jobdir = keep(ctx, relOpen(ctx, configPin, CODEX_CLI_JOB_STORE_DIR, true, jobdirAccess, SHARE_DIR)); + inspect(ctx, jobdir, true); + await verifyAcl([{ path: dir, directory: true, policy: "private" }]); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ENOENT" || !opts.createJobDir) { + if ((error as NodeJS.ErrnoException).code === "ENOENT" && !opts.createJobDir) return null; + throw error; + } + mkdirSync(dir); + jobdir = keep(ctx, relOpen(ctx, configPin, CODEX_CLI_JOB_STORE_DIR, true, jobdirAccess, SHARE_DIR)); + inspect(ctx, jobdir, true); + await hardenSecretDirAsync(dir, { required: true }); + await verifyAcl([{ path: dir, directory: true, policy: "private" }]); + } + return { config, dir, file, lock, jobdir }; +} + +/** Read the ledger body through a held leaf pin; null when the file is absent. */ +async function readStoreBody(ctx: Ctx, prepared: Prepared): Promise { + let leaf: bigint; + try { + // Verify-only opens stay read-only: ACCESS_LEAF carries DELETE, which a + // read-only-shared pin cannot admit; share-delete is offered so a held + // delete-capable pin (this process or another trusted writer) can coexist. + leaf = keep(ctx, relOpen(ctx, prepared.jobdir, CODEX_CLI_JOB_STORE_FILE, false, ACCESS_READ, SHARE_LEAF_VERIFY)); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") return null; + throw error; + } + try { + const stat = inspect(ctx, leaf, false); + if (stat.links !== 1 || stat.size > CODEX_CLI_JOB_STORE_MAX_BYTES) throw unsafe(); + await verifyAcl([{ path: prepared.file, directory: false, policy: "private" }]); + return readAll(ctx, leaf, stat.size).toString("utf8"); + } finally { + ctx.pins.release(leaf, ctx.kernel); + } +} + +/** + * Publish the ledger body through a private temp. The held temp handle binds the + * verified object through harden, ACL read, byte readback and the rename itself; + * after the rename a fresh name-open proves the published name resolves to the + * identical object before the directory is flushed. + */ +async function writeStoreBody(ctx: Ctx, prepared: Prepared, body: string): Promise { + const bytes = Buffer.byteLength(body); + if (bytes > CODEX_CLI_JOB_STORE_MAX_BYTES) throw new Error("store_full"); + const tempName = ".jobs-" + randomUUID() + ".tmp"; + const temp = join(prepared.dir, tempName); + let leaf: bigint | undefined; + try { + const fd = openSync(temp, "wx", 0o600); + try { writeFileSync(fd, body); fsyncSync(fd); } finally { closeSync(fd); } + leaf = keep(ctx, relOpen(ctx, prepared.jobdir, tempName, false, ACCESS_LEAF, SHARE_LEAF)); + const stat = inspect(ctx, leaf, false); + if (stat.links !== 1 || stat.size !== bytes) throw unsafe(); + if (!readAll(ctx, leaf, stat.size).equals(Buffer.from(body))) throw unsafe(); + await hardenSecretPathAsync(temp, { required: true, timeoutMemoKey: prepared.file }); + await verifyAcl([{ path: temp, directory: false, policy: "private" }]); + renamePinned(ctx, leaf, prepared.file); + // The held handle now names jobs.json. A fresh name-open proves the published + // path resolves to the identical object while the original handle is still held. + let published: bigint | undefined; + try { + // The renamed object is still held by `leaf` with share-read: only a + // read-access reopen can coexist, which is all the identity/byte proof needs. + published = keep(ctx, relOpen(ctx, prepared.jobdir, CODEX_CLI_JOB_STORE_FILE, false, + ACCESS_READ, SHARE_LEAF_VERIFY)); + const fresh = inspect(ctx, published, false); + if (!sameIdentity(fresh, stat) || fresh.links !== 1 || fresh.size !== bytes) throw unsafe(); + await verifyAcl([{ path: prepared.file, directory: false, policy: "private" }]); + if (!readAll(ctx, published, bytes).equals(Buffer.from(body))) throw unsafe(); + } finally { + if (published !== undefined) ctx.pins.release(published, ctx.kernel); + } + flushDir(ctx, prepared.jobdir); + } finally { + try { unlinkSync(temp); } catch { /* already renamed or never created */ } + forgetEphemeralSecretPath(temp); + if (leaf !== undefined) ctx.pins.release(leaf, ctx.kernel); + } +} + +interface HeldLock { + readonly dir: bigint; + readonly dirIdentity: StoreInspection; + readonly owner: bigint; + readonly ownerIdentity: StoreInspection; + readonly record: string; + readonly ownerPath: string; + readonly lockPath: string; +} + +/** + * Prove-and-delete the objects this call created during lock initialization. + * A held pin IS the created object — its identity cannot change underneath — + * so a verified pin may be marked delete-pending. An object that never got a + * proven pin stays behind, which keeps the lock directory occupied and leaves + * later admission fail-closed, exactly like the POSIX rollback. + */ +function rollbackInitializingLock( + ctx: Ctx, + lockDir: bigint, + lockIdentity: StoreInspection, + owner: bigint | undefined, + ownerIdentity: StoreInspection | undefined, +): void { + try { + if (owner !== undefined && ownerIdentity !== undefined) { + try { + if (sameIdentity(inspect(ctx, owner, false), ownerIdentity)) markDelete(ctx, owner); + } catch { /* unprovable stays */ } + ctx.pins.release(owner, ctx.kernel); + } + try { + if (sameIdentity(inspect(ctx, lockDir, true), lockIdentity)) markDelete(ctx, lockDir); + } catch { /* keep the lock fail-closed */ } + ctx.pins.release(lockDir, ctx.kernel); + } catch { /* rollback never masks the original error */ } +} + +/** + * Acquire the admission mutex: mkdir the lock directory, then write owner.json + * under the held pins. EEXIST means another admission is in flight and maps to + * job_store_busy; a crashed leftover stays busy forever, like POSIX. + */ +async function acquireLock(ctx: Ctx, prepared: Prepared): Promise { + try { mkdirSync(prepared.lock); } + catch (error) { + if ((error as NodeJS.ErrnoException).code === "EEXIST") throw new CodexCliJobStoreBusyError(); + throw error; + } + const lockDir = keep(ctx, relOpen(ctx, prepared.jobdir, CODEX_CLI_JOB_STORE_LOCK, true, + ACCESS_DIR_WRITE, SHARE_DIR)); + const lockIdentity = inspect(ctx, lockDir, true); + const ownerPath = join(prepared.lock, "owner.json"); + const record = JSON.stringify({ version: 1, token: randomUUID() }); + const recordBytes = Buffer.byteLength(record); + let owner: bigint | undefined; + let ownerIdentity: StoreInspection | undefined; + try { + const fd = openSync(ownerPath, "wx", 0o600); + try { writeFileSync(fd, record); fsyncSync(fd); } finally { closeSync(fd); } + owner = keep(ctx, relOpen(ctx, lockDir, "owner.json", false, ACCESS_LEAF, SHARE_LEAF)); + ownerIdentity = inspect(ctx, owner, false); + if (ownerIdentity.links !== 1 || ownerIdentity.size !== recordBytes) throw unsafe(); + if (!readAll(ctx, owner, recordBytes).equals(Buffer.from(record))) throw unsafe(); + await hardenSecretDirAsync(prepared.lock, { required: true }); + await hardenSecretPathAsync(ownerPath, { required: true }); + await verifyAcl([ + { path: prepared.lock, directory: true, policy: "private" }, + { path: ownerPath, directory: false, policy: "private" }, + ]); + } catch (error) { + rollbackInitializingLock(ctx, lockDir, lockIdentity, owner, ownerIdentity); + throw error; + } + return { dir: lockDir, dirIdentity: lockIdentity, owner, + ownerIdentity, record, ownerPath, lockPath: prepared.lock }; +} + +/** + * Token-fenced release: the owner file is deleted only while its held pin still + * shows this call's identity and exact content, and the lock directory only + * while its pin shows this call's identity. A replaced owner or lock survives, + * which keeps admission fail-closed on the leftover. + */ +function releaseLock(ctx: Ctx, held: HeldLock): void { + try { + const now = inspect(ctx, held.owner, false); + if (sameIdentity(now, held.ownerIdentity) && now.links === 1 + && now.size === Buffer.byteLength(held.record) + && now.size <= 1024 + && readAll(ctx, held.owner, now.size).equals(Buffer.from(held.record))) { + markDelete(ctx, held.owner); + } + } catch { /* fail closed; a leftover owner keeps the lock alive */ } + ctx.pins.release(held.owner, ctx.kernel); + try { + if (sameIdentity(inspect(ctx, held.dir, true), held.dirIdentity)) markDelete(ctx, held.dir); + } catch { /* fail closed; a leftover lock blocks later admission */ } + ctx.pins.release(held.dir, ctx.kernel); +} + +export interface WindowsJobStoreIo { + /** Ledger body text; null when the file is absent. */ + readStore(): Promise; + /** Atomically publish a replacement ledger body. */ + writeStore(body: string): Promise; +} + +/** + * Read-only store access outside the mutex, for status. Returns null when the + * store tree or ledger file does not exist. + */ +export async function readWindowsJobStore(configDir?: string): Promise { + const absolute = resolve(configDir ?? getConfigDir()); + const { ffi, kernel, nt } = await loadNative(); + const ctx: Ctx = { ffi, kernel, nt, pins: new PinSet() }; + try { + const prepared = await prepareStore(ctx, absolute, { createJobDir: false, createConfigRoot: false }); + if (prepared === null) return null; + return await readStoreBody(ctx, prepared); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") return null; + throw error; + } finally { + ctx.pins.closeAll(kernel); + } +} + +/** + * Run fn under the admission mutex with read/write access to the ledger. + * createConfigRoot is the confirmed-admission privilege to create one absent + * config-root level; status reads and completion writes never create it. + */ +export async function withWindowsJobStoreLock( + configDir: string | undefined, + createConfigRoot: boolean, + fn: (io: WindowsJobStoreIo) => T | Promise, +): Promise { + const absolute = resolve(configDir ?? getConfigDir()); + assertNotRealHomeUnderTest(absolute); + const { ffi, kernel, nt } = await loadNative(); + const ctx: Ctx = { ffi, kernel, nt, pins: new PinSet() }; + try { + const prepared = await prepareStore(ctx, absolute, { createJobDir: true, createConfigRoot }); + if (prepared === null) throw unsafe(); // unreachable: write flow never returns null + const held = await acquireLock(ctx, prepared); + try { + return await fn({ + readStore: () => readStoreBody(ctx, prepared), + writeStore: body => writeStoreBody(ctx, prepared, body), + }); + } finally { + releaseLock(ctx, held); + } + } finally { + ctx.pins.closeAll(kernel); + } +} diff --git a/src/codex/cli-update-job.ts b/src/codex/cli-update-job.ts index 0e8d1821c6f..5856fc6452a 100644 --- a/src/codex/cli-update-job.ts +++ b/src/codex/cli-update-job.ts @@ -5,13 +5,10 @@ import { dirname, join, resolve } from "node:path"; import { getConfigDir } from "../config"; import type { CodexCliApplyResult } from "./cli-update-apply"; import { CODEX_CLI_JOB_REASONS as SAFE_REASONS, CODEX_CLI_JOB_CLEANUP_ERRORS, + CODEX_CLI_JOB_STORE_DIR, CODEX_CLI_JOB_STORE_FILE, CODEX_CLI_JOB_STORE_LOCK, CODEX_CLI_JOB_STORE_MAX_BYTES, CodexCliJobStoreBusyError, normalizeCodexCliJobResult, runCodexCliUpdateJobWork } from "./cli-update-job-execution"; -const MAX_BYTES = 256 * 1024; const MAX_JOBS = 64; -const JOB_DIR = "codex-cli-update-jobs"; -const FILE = "jobs.json"; -const LOCK = "admission.lock"; type JobStatus = "queued" | "running" | "applied" | "not_applied" | "refused" | "ambiguous"; interface StoredJob { @@ -38,8 +35,8 @@ const now = () => new Date().toISOString(); function paths(configDir = getConfigDir()) { const config = resolve(configDir); - const dir = join(config, JOB_DIR); - return { config, dir, file: join(dir, FILE), lock: join(dir, LOCK) }; + const dir = join(config, CODEX_CLI_JOB_STORE_DIR); + return { config, dir, file: join(dir, CODEX_CLI_JOB_STORE_FILE), lock: join(dir, CODEX_CLI_JOB_STORE_LOCK) }; } function assertTrustedConfigAncestry(parent: string) { const euid = process.geteuid?.(); @@ -101,6 +98,23 @@ function prepare(configDir?: string, create = false, createConfigRoot = false) { || realpathSync.native(p.dir) !== p.dir) throw new Error("unsafe_store"); return p; } +/** Shared ledger-body validation used by the POSIX reader and the Windows backend adapter. */ +function parseStoreBody(body: string | null): Store { + if (body === null) return { version: 1, jobs: [] }; + const value = JSON.parse(body) as Store; + if (value.version !== 1 || !Array.isArray(value.jobs) || value.jobs.length > MAX_JOBS + || value.jobs.some(job => !job || typeof job !== "object" || !/^[0-9a-f-]{36}$/.test(job.id) + || !/^[0-9a-f]{64}$/.test(job.requestKey) || !/^[0-9a-f]{64}$/.test(job.payloadHash) + || !/^[0-9a-f-]{36}$/.test(job.generation) || !Number.isSafeInteger(job.revision) || job.revision < 0 + || !["queued", "running", "applied", "not_applied", "refused", "ambiguous"].includes(job.status) + || typeof job.planId !== "string" || !/^[0-9a-f]{32}$/.test(job.planId) + || (job.reason !== null && (typeof job.reason !== "string" || !SAFE_REASONS.has(job.reason))) + || !Array.isArray(job.cleanupErrors) || job.cleanupErrors.length > 8 + || job.cleanupErrors.some((item: unknown) => typeof item !== "string" || !CODEX_CLI_JOB_CLEANUP_ERRORS.has(item)) + || typeof job.leaseRetained !== "boolean" + || typeof job.createdAt !== "string" || typeof job.updatedAt !== "string")) throw new Error("invalid_store"); + return value; +} function readStore(configDir?: string): Store { let p: ReturnType; try { p = prepare(configDir); } @@ -109,26 +123,14 @@ function readStore(configDir?: string): Store { try { fd = openSync(p.file, constants.O_RDONLY | constants.O_NOFOLLOW | constants.O_NONBLOCK); const stat = fstatSync(fd); - if (!stat.isFile() || stat.size > MAX_BYTES || (stat.mode & 0o077) !== 0 + if (!stat.isFile() || stat.size > CODEX_CLI_JOB_STORE_MAX_BYTES || (stat.mode & 0o077) !== 0 || stat.uid !== process.geteuid?.() || stat.nlink !== 1) throw new Error("unsafe_store"); const lexical = lstatSync(p.file); if (lexical.isSymbolicLink() || lexical.dev !== stat.dev || lexical.ino !== stat.ino) throw new Error("unsafe_store"); - const bytes = Buffer.alloc(MAX_BYTES + 1); + const bytes = Buffer.alloc(CODEX_CLI_JOB_STORE_MAX_BYTES + 1); const length = readSync(fd, bytes, 0, bytes.length, 0); - if (length > MAX_BYTES) throw new Error("unsafe_store"); - const value = JSON.parse(bytes.subarray(0, length).toString("utf8")) as Store; - if (value.version !== 1 || !Array.isArray(value.jobs) || value.jobs.length > MAX_JOBS - || value.jobs.some(job => !job || typeof job !== "object" || !/^[0-9a-f-]{36}$/.test(job.id) - || !/^[0-9a-f]{64}$/.test(job.requestKey) || !/^[0-9a-f]{64}$/.test(job.payloadHash) - || !/^[0-9a-f-]{36}$/.test(job.generation) || !Number.isSafeInteger(job.revision) || job.revision < 0 - || !["queued", "running", "applied", "not_applied", "refused", "ambiguous"].includes(job.status) - || typeof job.planId !== "string" || !/^[0-9a-f]{32}$/.test(job.planId) - || (job.reason !== null && (typeof job.reason !== "string" || !SAFE_REASONS.has(job.reason))) - || !Array.isArray(job.cleanupErrors) || job.cleanupErrors.length > 8 - || job.cleanupErrors.some((item: unknown) => typeof item !== "string" || !CODEX_CLI_JOB_CLEANUP_ERRORS.has(item)) - || typeof job.leaseRetained !== "boolean" - || typeof job.createdAt !== "string" || typeof job.updatedAt !== "string")) throw new Error("invalid_store"); - return value; + if (length > CODEX_CLI_JOB_STORE_MAX_BYTES) throw new Error("unsafe_store"); + return parseStoreBody(bytes.subarray(0, length).toString("utf8")); } finally { if (fd !== undefined) closeSync(fd); } } function rollbackInitializingLock(p: ReturnType, lockIdentity: { dev: number; ino: number }, ownerIdentity?: { dev: number; ino: number }) { @@ -154,7 +156,11 @@ function rollbackInitializingLock(p: ReturnType, lockIdentity: { d if (ownsDirectory()) rmdirSync(p.lock); } catch { /* Unprovable initialization ownership stays fail-closed. */ } } -function withLock(configDir: string | undefined, fn: (p: ReturnType) => T, createConfigRoot = false): T { +/** + * POSIX admission mutex. Acquisition and release stay synchronous; the ledger body + * may await, so the lock directory is held until fn settles rather than until it returns. + */ +async function withLock(configDir: string | undefined, fn: (p: ReturnType) => T | Promise, createConfigRoot = false): Promise { const p = prepare(configDir, true, createConfigRoot); let lockIdentity: { dev: number; ino: number } | undefined; let ownerIdentity: { dev: number; ino: number } | undefined; @@ -178,7 +184,7 @@ function withLock(configDir: string | undefined, fn: (p: ReturnType(configDir: string | undefined, fn: (p: ReturnType, store: Store) { const body = JSON.stringify(store); - if (Buffer.byteLength(body) > MAX_BYTES) throw new Error("store_full"); + if (Buffer.byteLength(body) > CODEX_CLI_JOB_STORE_MAX_BYTES) throw new Error("store_full"); const temp = join(p.dir, `.jobs-${randomUUID()}.tmp`); let fd: number | undefined; try { @@ -253,21 +259,60 @@ function dto(job: StoredJob): CodexCliUpdateJobDto { cleanupErrors: Object.freeze([...job.cleanupErrors]) }); } -export function readCodexCliUpdateJob(jobId: string, configDir?: string): CodexCliUpdateJobDto | null { +/** + * Platform ledger primitives behind one admission lock. The POSIX branch wraps the + * synchronous fs helpers; the Windows branch binds the handle-pinned backend in + * cli-update-job-store-windows.ts. Every lifecycle rule below is written once + * against this surface so both backends project the identical DTO. + */ +interface LedgerIo { + readStore(): Promise; + writeStore(store: Store): Promise; +} + +/** + * One admission lock, one store body. The Windows backend loads lazily so the + * FFI-bearing module stays out of the POSIX import graph. + */ +async function withLedgerLock(configDir: string | undefined, createConfigRoot: boolean, + fn: (io: LedgerIo) => Promise): Promise { + if (process.platform === "win32") { + const { withWindowsJobStoreLock } = await import("./cli-update-job-store-windows"); + return withWindowsJobStoreLock(configDir, createConfigRoot, io => fn({ + readStore: async () => parseStoreBody(await io.readStore()), + writeStore: async store => { await io.writeStore(JSON.stringify(store)); }, + })); + } + return withLock(configDir, p => fn({ + readStore: async () => validStore(configDir), + writeStore: async store => { writeStore(p, store); }, + }), createConfigRoot); +} + +/** Lock-free status read through the platform backend. */ +async function readLedgerStore(configDir?: string): Promise { + if (process.platform === "win32") { + const { readWindowsJobStore } = await import("./cli-update-job-store-windows"); + return parseStoreBody(await readWindowsJobStore(configDir)); + } + return readStore(configDir); +} + +export async function readCodexCliUpdateJob(jobId: string, configDir?: string): Promise { if (!/^[0-9a-f-]{36}$/.test(jobId)) return null; - const job = readStore(configDir).jobs.find(item => item.id === jobId); + const job = (await readLedgerStore(configDir)).jobs.find(item => item.id === jobId); return job ? dto(job) : null; } /** Persist dedupe admission before returning; a duplicate key never schedules a second installer. */ -export function admitCodexCliUpdateJob(input: { requestId: string; planId: string; confirmed: true; configDir?: string }): - { kind: "accepted" | "duplicate"; job: CodexCliUpdateJobDto; generation: string } { +export async function admitCodexCliUpdateJob(input: { requestId: string; planId: string; confirmed: true; configDir?: string }): + Promise<{ kind: "accepted" | "duplicate"; job: CodexCliUpdateJobDto; generation: string }> { if (input.confirmed !== true || !/^[A-Za-z0-9._:-]{8,128}$/.test(input.requestId) || !/^[0-9a-f]{32}$/.test(input.planId)) throw new Error("invalid_request"); const requestKey = digest(input.requestId); const payloadHash = digest(JSON.stringify({ planId: input.planId })); - return withLock(input.configDir, p => { - const store = validStore(input.configDir); + return withLedgerLock(input.configDir, true, async io => { + const store = await io.readStore(); const existing = store.jobs.find(job => job.requestKey === requestKey); if (existing) { if (existing.payloadHash !== payloadHash) throw new Error("idempotency_conflict"); @@ -278,17 +323,17 @@ export function admitCodexCliUpdateJob(input: { requestId: string; planId: strin const record: StoredJob = { id: randomUUID(), requestKey, payloadHash, generation: randomUUID(), revision: 0, status: "queued", createdAt: stamp, updatedAt: stamp, planId: input.planId, outcome: null, reason: null, cleanupErrors: [], leaseRetained: false }; - store.jobs.push(record); writeStore(p, store); + store.jobs.push(record); await io.writeStore(store); live.set(record.id, record.generation); return { kind: "accepted", job: dto(record), generation: record.generation }; - }, true); + }); } /** Updates are generation/revision fenced so an old worker cannot replace a newer state. */ -export function finishCodexCliUpdateJob(jobId: string, generation: string, expectedRevision: number, - result: CodexCliApplyResult, configDir?: string): CodexCliUpdateJobDto | null { - return withLock(configDir, p => { - const store = validStore(configDir); +export async function finishCodexCliUpdateJob(jobId: string, generation: string, expectedRevision: number, + result: CodexCliApplyResult, configDir?: string): Promise { + return withLedgerLock(configDir, false, async io => { + const store = await io.readStore(); const index = store.jobs.findIndex(job => job.id === jobId); if (index < 0) return null; const current = store.jobs[index]!; @@ -300,13 +345,13 @@ export function finishCodexCliUpdateJob(jobId: string, generation: string, expec && current.outcome === safe.outcome && current.reason === safe.reason && current.leaseRetained === safe.leaseRetained && JSON.stringify(current.cleanupErrors) === JSON.stringify(safe.cleanupErrors)) { - writeStore(p, store); live.delete(jobId); started.delete(jobId); return dto(current); + await io.writeStore(store); live.delete(jobId); started.delete(jobId); return dto(current); } if (current.generation !== generation || current.revision !== expectedRevision || live.get(jobId) !== generation || !started.has(jobId)) return dto(current); const next: StoredJob = { ...current, revision: current.revision + 1, status: safe.outcome, outcome: safe.outcome, reason: safe.reason, cleanupErrors: safe.cleanupErrors, leaseRetained: safe.leaseRetained, updatedAt: now() }; - store.jobs[index] = next; writeStore(p, store); live.delete(jobId); started.delete(jobId); + store.jobs[index] = next; await io.writeStore(store); live.delete(jobId); started.delete(jobId); return dto(next); }); } @@ -314,12 +359,12 @@ export function finishCodexCliUpdateJob(jobId: string, generation: string, expec export async function runCodexCliUpdateJob(jobId: string, generation: string, run: () => Promise, configDir?: string): Promise { let ownsExecution = false; return runCodexCliUpdateJobWork(run, { - begin: () => withLock(configDir, p => { - const store = validStore(configDir); const index = store.jobs.findIndex(job => job.id === jobId); + begin: () => withLedgerLock(configDir, false, async io => { + const store = await io.readStore(); const index = store.jobs.findIndex(job => job.id === jobId); if (index < 0 || store.jobs[index]!.generation !== generation) throw new Error("job_generation_changed"); if (live.get(jobId) !== generation || started.has(jobId)) throw new Error("job_not_owned"); const current = store.jobs[index]!; const next = { ...current, revision: current.revision + 1, status: "running" as const, updatedAt: now() }; - store.jobs[index] = next; writeStore(p, store); started.add(jobId); ownsExecution = true; return next.revision; + store.jobs[index] = next; await io.writeStore(store); started.add(jobId); ownsExecution = true; return next.revision; }), finish: (revision, result) => finishCodexCliUpdateJob(jobId, generation, revision, result, configDir), read: () => readCodexCliUpdateJob(jobId, configDir), diff --git a/src/lib/windows-owner-acl.ts b/src/lib/windows-owner-acl.ts index b40954a02e5..08490fa983c 100644 --- a/src/lib/windows-owner-acl.ts +++ b/src/lib/windows-owner-acl.ts @@ -1,15 +1,40 @@ import { resolveTrustedWindowsPowerShellExe } from "./windows-elevation"; +import { waitForSubprocessExit } from "./bounded-subprocess"; export interface WindowsPrivateEntry { path: string; directory: boolean; } +/** + * Per-entry verification policy for {@link windowsEntriesAclMatches}. + * + * - "private" (default): the exact hardened-object rule — protected DACL, owner is the + * serving token (or Administrators only for an enabled elevated owner), and the only + * grantees are the token user with Full Control plus SYSTEM/Administrators. + * - "write-private": the POSIX mode&022==0 analog for objects we did NOT create — the + * serving token or a kernel-equivalent owner (SYSTEM, Administrators, TrustedInstaller) + * must own the object, and no grantee outside the trusted write set may hold write, + * create, delete or DACL/owner control. Read grants to anyone are fine. + * - "trusted-ancestry": the POSIX uid∈{euid,0} + non-shared-writable ancestor analog. + * The owner must be the serving token or a kernel-equivalent owner (SYSTEM, + * Administrators, TrustedInstaller — each can already take ownership of any object, + * so an ACL entry grants them nothing they lack). Untrusted grantees get read access + * only, except that an admin-owned ancestor may grant broad create-only rights + * (FILE_ADD_FILE / FILE_ADD_SUBDIRECTORY without delete or write-existing): that is + * the closest NTFS analog of a root-owned sticky directory such as C:\, whose stock + * ACL grants Authenticated Users add-subdirectory while refusing every other write. + */ +export type WindowsAclPolicy = "private" | "write-private" | "trusted-ancestry"; +export interface WindowsAclEntry extends WindowsPrivateEntry { + readonly policy?: WindowsAclPolicy; +} export interface WindowsOwnerAclResult { success: boolean; timedOut: boolean; stdout: string | Uint8Array; } export type WindowsOwnerAclRunner = (entries: readonly WindowsPrivateEntry[], timeoutMs: number) => WindowsOwnerAclResult; +export type WindowsOwnerAclAsyncRunner = (entries: readonly WindowsAclEntry[], timeoutMs: number) => Promise; export const WINDOWS_OWNER_ACL_TIMEOUT_MS = 5_000; const SID = /^S-1-\d+(?:-\d+)+$/i; @@ -17,6 +42,24 @@ const ADMINISTRATORS = "S-1-5-32-544"; const PRIVILEGED_GRANTEES = new Set(["S-1-5-18", ADMINISTRATORS]); // LocalSystem, BUILTIN\Administrators const PRIVATE_DACL_FLAGS = 0x1004; // DiscretionaryAclPresent | DiscretionaryAclProtected const FULL_CONTROL = 2032127; +/** TrustedInstaller owns Windows component trees; it can already take ownership of any object. */ +const TRUSTED_INSTALLER = "S-1-5-80-956008885-3418522649-1831038044-1853292631-2271478464"; +/** Grantees a confined/ancestor policy treats as capable of anything already. */ +const TRUSTED_WRITE_SIDS = new Set(["S-1-5-18", ADMINISTRATORS, TRUSTED_INSTALLER]); +/** Owners acceptable for an ancestor (POSIX uid 0 analogs). */ +const TRUSTED_ANCESTOR_OWNERS = new Set(["S-1-5-18", ADMINISTRATORS, TRUSTED_INSTALLER]); +/** Admin-owned ancestors may carry broad create-only grants (the C:\ Authenticated-Users pattern). */ +const STICKY_OWNERS = TRUSTED_ANCESTOR_OWNERS; + +// Access-mask bits (unsigned 32-bit). Read-side grants any entry may hold without +// weakening confinement; create bits only a trusted-owner ancestor may share broadly; +// everything else counts as write and is confined to the trusted set. +const READ_MASK = 0x80000000 /* GENERIC_READ */ | 0x20000000 /* GENERIC_EXECUTE */ + | 0x100000 /* SYNCHRONIZE */ | 0x20000 /* READ_CONTROL */ + | 0x80 /* FILE_READ_ATTRIBUTES */ | 0x20 /* FILE_EXECUTE/TRAVERSE */ + | 0x8 /* FILE_READ_EA */ | 0x1 /* FILE_READ_DATA/LIST */; +const CREATE_MASK = 0x4 /* FILE_APPEND_DATA/ADD_SUBDIRECTORY */ | 0x2 /* FILE_WRITE_DATA/ADD_FILE */; +const INHERIT_ONLY_ACE = 0x8; // This script is constant. Literal paths are environment data, never PowerShell source. // Read the descriptor through .NET (Owner, Group and Access, as Get-Acl does). Get-Acl is a module-autoloaded @@ -89,48 +132,159 @@ export function setWindowsOwnerAclRunnerForTests(runner: WindowsOwnerAclRunner | ownerAclRunner = runner ?? defaultWindowsOwnerAclRunner; } -/** Pure, strict line-protocol parser and owner/effective-DACL policy. No account names or SDDL. */ -export function windowsPrivateEntriesAclMatches(stdout: string | Uint8Array, entries: readonly WindowsPrivateEntry[]): boolean { - if (entries.length === 0) return false; +/** Parsed ACE record; opaque marks the four-field non-CommonAce marker the policy cannot evaluate. */ +interface ParsedAce { + readonly opaque: boolean; + readonly type: number | null; + readonly flags: number | null; + readonly mask: number | null; + readonly sid: string | null; + readonly callback: boolean | null; + readonly malformed: boolean; + readonly raw: readonly string[]; +} +interface ParsedEntry { + readonly owner: string; + readonly controlFlags: number; + /** null when the object reported a NULL DACL, which grants Everyone full access. */ + readonly aces: readonly ParsedAce[] | null; +} +interface ParsedAclRun { + readonly user: string; + readonly acceptsAdministrators: boolean; + readonly entries: readonly (ParsedEntry | null)[]; +} + +/** Shared strict line-protocol parser; per-entry ACE semantics live in the policies. */ +function parseAclProtocol(stdout: string | Uint8Array, count: number): ParsedAclRun | null { + if (count <= 0) return null; // The script emits ASCII only. Reject BOMs, NULs, non-ASCII bytes and replacement decoding. - if (typeof stdout !== "string" && stdout.some(byte => byte > 127)) return false; + if (typeof stdout !== "string" && stdout.some(byte => byte > 127)) return null; const text = typeof stdout === "string" ? stdout : Buffer.from(stdout).toString("ascii"); - if (/[^\x20-\x7e\r\n]/.test(text)) return false; + if (/[^\x20-\x7e\r\n]/.test(text)) return null; const normalized = text.replace(/\r\n/g, "\n"); - if (normalized.includes("\r")) return false; + if (normalized.includes("\r")) return null; const lines = (normalized.endsWith("\n") ? normalized.slice(0, -1) : normalized).split("\n"); - if (lines.at(-1) !== "END") return false; + if (lines.at(-1) !== "END") return null; const token = lines[0]!.split("|"); if (token.length !== 4 || token[0] !== "U" || !SID.test(token[1]!) || !SID.test(token[2]!) - || (token[3] !== "True" && token[3] !== "False")) return false; - const user = token[1]!.toUpperCase(); - const acceptsAdministrators = token[2]!.toUpperCase() === ADMINISTRATORS && token[3] === "True"; + || (token[3] !== "True" && token[3] !== "False")) return null; + const entries: (ParsedEntry | null)[] = []; let at = 1; - for (let i = 0; i < entries.length; i++) { + for (let i = 0; i < count; i++) { const entry = (lines[at++] ?? "").split("|"); + // ACE count is unbounded for confined policies: ancestors legitimately carry many entries. + // -1 is the NULL-DACL marker (no ACE lines follow); the private policy still refuses it below. if (entry.length !== 5 || entry[0] !== "E" || entry[1] !== String(i) || !SID.test(entry[2]!) - || !/^(0|[1-9]\d{0,4})$/.test(entry[3]!) || !/^[1-3]$/.test(entry[4]!)) return false; - const flags = Number(entry[3]); - if (flags > 0xffff || (flags & PRIVATE_DACL_FLAGS) !== PRIVATE_DACL_FLAGS) return false; + || !/^(0|[1-9]\d{0,4})$/.test(entry[3]!) || !/^(-1|0|[1-9]\d{0,3})$/.test(entry[4]!)) return null; const owner = entry[2]!.toUpperCase(); - if (owner !== user && !(owner === ADMINISTRATORS && acceptsAdministrators)) return false; - // The serving account needs its own Full Control entry. SYSTEM and Administrators may also hold plain - // allow entries: LocalSystem and an elevated administrator can already take ownership of any object, and a - // filtered administrator token holds Administrators only as deny-only, so the allow entry grants it nothing. - const grantees = new Set(); - for (let k = Number(entry[4]); k > 0; k--) { - const ace = (lines[at++] ?? "").split("|"); - if (ace.length !== 7 || ace[0] !== "A" || ace[1] !== String(i) || ace[2] !== "0" - || ace[3] !== (entries[i]!.directory ? "3" : "0") || !/^[1-9]\d{0,9}$/.test(ace[4]!) - || !SID.test(ace[5]!) || ace[6] !== "False") return false; - const grantee = ace[5]!.toUpperCase(); - if (grantees.has(grantee)) return false; - grantees.add(grantee); - if (grantee === user ? ace[4] !== String(FULL_CONTROL) : !PRIVILEGED_GRANTEES.has(grantee)) return false; + const controlFlags = Number(entry[3]); + const aceCount = Number(entry[4]); + if (aceCount === -1) { + // NULL DACL: the script emits count -1 and no ACE lines. It grants Everyone full access. + entries.push({ owner, controlFlags, aces: null }); + continue; } - if (!grantees.has(user)) return false; + const aces: ParsedAce[] = []; + for (let k = 0; k < aceCount; k++) { + const raw = (lines[at++] ?? "").split("|"); + if (raw.length === 4 && raw[0] === "A" && raw[1] === String(i) && raw[3] === "X" && /^\d+$/.test(raw[2]!)) { + aces.push({ opaque: true, type: Number(raw[2]), flags: null, mask: null, sid: null, callback: null, malformed: false, raw }); + } else if (raw.length === 7 && raw[0] === "A" && raw[1] === String(i) && /^-?\d+$/.test(raw[2]!) + && /^\d+$/.test(raw[3]!) && /^-?\d+$/.test(raw[4]!) && SID.test(raw[5]!) + && (raw[6] === "True" || raw[6] === "False")) { + aces.push({ opaque: false, type: Number(raw[2]), flags: Number(raw[3]), mask: Number(raw[4]), + sid: raw[5]!.toUpperCase(), callback: raw[6] === "True", malformed: false, raw }); + } else { + aces.push({ opaque: false, type: null, flags: null, mask: null, sid: null, callback: null, malformed: true, raw }); + } + } + entries.push({ owner, controlFlags, aces }); } - return at === lines.length - 1; + if (at !== lines.length - 1) return null; + return { user: token[1]!.toUpperCase(), + acceptsAdministrators: token[2]!.toUpperCase() === ADMINISTRATORS && token[3] === "True", + entries }; +} + +/** The exact hardened-object policy; unchanged by the shared parser. */ +function privateEntryOk(entry: ParsedEntry | null, directory: boolean, run: ParsedAclRun): boolean { + if (!entry || entry.aces === null) return false; + const flags = entry.controlFlags; + if (flags > 0xffff || (flags & PRIVATE_DACL_FLAGS) !== PRIVATE_DACL_FLAGS) return false; + const owner = entry.owner; + const user = run.user; + if (owner !== user && !(owner === ADMINISTRATORS && run.acceptsAdministrators)) return false; + // The serving account needs its own Full Control entry. SYSTEM and Administrators may also hold plain + // allow entries: LocalSystem and an elevated administrator can already take ownership of any object, and a + // filtered administrator token holds Administrators only as deny-only, so the allow entry grants it nothing. + const grantees = new Set(); + for (const ace of entry.aces) { + if (ace.opaque || ace.malformed || ace.raw.length !== 7 || ace.raw[2] !== "0" + || ace.raw[3] !== (directory ? "3" : "0") || !/^[1-9]\d{0,9}$/.test(ace.raw[4]!) + || ace.callback !== false) return false; + const grantee = ace.sid!; + if (grantees.has(grantee)) return false; + grantees.add(grantee); + if (grantee === user ? ace.raw[4] !== String(FULL_CONTROL) : !PRIVILEGED_GRANTEES.has(grantee)) return false; + } + return grantees.has(user); +} + +/** + * Confined-write and trusted-ancestry policies for objects this process did not create. + * + * Deny ACEs are permitted because they can only remove access; allow ACEs must be + * ordinary non-callback CommonAce entries. Inherit-only ACEs are skipped: they are + * templates for children, and every object this module protects carries its own + * hardened protected DACL, so an ancestor's template cannot grant anything inside it. + */ +function confinedEntryOk(entry: ParsedEntry | null, run: ParsedAclRun, policy: WindowsAclPolicy): boolean { + if (!entry || entry.aces === null || (entry.controlFlags & 0x4 /* SE_DACL_PRESENT */) === 0) return false; + const owner = entry.owner; + // Trusted ownership is independent of this token's elevation state: the owner + // controls the DACL, and a kernel-equivalent owner can already take ownership of + // any object, so accepting it grants nothing. A filtered token must still pass + // an Administrators-owned profile parent — gating on acceptsAdministrators + // would refuse the identical, legitimate directory when run non-elevated. + const owned = owner === run.user || TRUSTED_ANCESTOR_OWNERS.has(owner); + if (!owned) return false; + for (const ace of entry.aces) { + if (ace.malformed || ace.opaque || ace.callback === true || ace.type === null) return false; + if (ace.type === 1) continue; // ACCESS_DENIED: removes access, never grants it. + if (ace.type !== 0) return false; // only ACCESS_ALLOWED records can be evaluated. + if (ace.flags === null || ace.mask === null || ace.sid === null) return false; + if ((ace.flags & INHERIT_ONLY_ACE) !== 0) continue; + if (ace.sid === run.user || TRUSTED_WRITE_SIDS.has(ace.sid)) continue; + const createOk = policy === "trusted-ancestry" && STICKY_OWNERS.has(owner) ? CREATE_MASK : 0; + const allowed = (READ_MASK | createOk) >>> 0; + if (((ace.mask >>> 0) & ~allowed) !== 0) return false; + } + return true; +} + +/** Pure, strict line-protocol parser and owner/effective-DACL policy. No account names or SDDL. */ +export function windowsPrivateEntriesAclMatches(stdout: string | Uint8Array, entries: readonly WindowsPrivateEntry[]): boolean { + if (entries.length === 0) return false; + const run = parseAclProtocol(stdout, entries.length); + if (!run) return false; + return entries.every((entry, index) => privateEntryOk(run.entries[index] ?? null, entry.directory, run)); +} + +/** + * Mixed-policy evaluation for a batch of entries, each defaulting to "private". + * One PowerShell read covers the whole chain (ancestors, config root, store objects) + * so the ACL snapshot is taken once per operation phase, not per path. + */ +export function windowsEntriesAclMatches(stdout: string | Uint8Array, entries: readonly WindowsAclEntry[]): boolean { + if (entries.length === 0) return false; + const run = parseAclProtocol(stdout, entries.length); + if (!run) return false; + return entries.every((entry, index) => { + const policy = entry.policy ?? "private"; + return policy === "private" ? privateEntryOk(run.entries[index] ?? null, entry.directory, run) + : confinedEntryOk(run.entries[index] ?? null, run, policy); + }); } /** Fresh, read-only verification; runner failures never disclose paths, SIDs or ACL output. */ @@ -141,3 +295,52 @@ export function verifyWindowsPrivateEntries(entries: readonly WindowsPrivateEntr return result.success && !result.timedOut && windowsPrivateEntriesAclMatches(result.stdout, entries); } catch { return false; } } + +/** + * Async counterpart of the sync runner: the PowerShell read is identical, but the + * caller's event loop is not held for the child lifetime. The deadline is enforced by + * the shared bounded-subprocess helper, which kills the child and still waits for the + * kernel to release its handles before reporting the timeout. + */ +async function defaultWindowsOwnerAclAsyncRunner( + entries: readonly WindowsAclEntry[], + timeoutMs: number, +): Promise { + const env: Record = { ...process.env, OCX_ACL_COUNT: String(entries.length) }; + for (let i = 0; i < entries.length; i++) env["OCX_ACL_PATH_" + i] = entries[i]!.path; + let proc; + try { + proc = Bun.spawn([ + resolveTrustedWindowsPowerShellExe(), "-NoLogo", "-NoProfile", "-NonInteractive", + "-ExecutionPolicy", "Bypass", "-EncodedCommand", ENCODED_ACL_SCRIPT, + ], { + env, stdin: "ignore", stdout: "pipe", stderr: "ignore", windowsHide: true, + }); + } catch { + return { success: false, timedOut: false, stdout: new Uint8Array() }; + } + // No reap wait: the post-kill wait exists for handle-bearing children whose open + // handles gate a later removal (the icacls hardening case). This read-only lookup + // holds no such path, and waiting for a child that survived kill() would turn one + // transient stall into an unbounded caller stall; the principal-lookup runner in + // windows-user-principal.ts opts out for the same reason. + const { exitCode, timedOut } = await waitForSubprocessExit(proc, timeoutMs, 0); + const stdout = !timedOut && proc.stdout + ? await new Response(proc.stdout).text().catch(() => "") + : ""; + return { success: !timedOut && exitCode === 0, timedOut, stdout }; +} +let ownerAclAsyncRunner: WindowsOwnerAclAsyncRunner = defaultWindowsOwnerAclAsyncRunner; + +export function setWindowsOwnerAclAsyncRunnerForTests(runner: WindowsOwnerAclAsyncRunner | null): void { + ownerAclAsyncRunner = runner ?? defaultWindowsOwnerAclAsyncRunner; +} + +/** Async mixed-policy verification; runner failures never disclose paths, SIDs or ACL output. */ +export async function verifyWindowsEntriesAclAsync(entries: readonly WindowsAclEntry[], timeoutMs: number): Promise { + if (entries.length === 0 || !Number.isFinite(timeoutMs) || timeoutMs <= 0) return false; + try { + const result = await ownerAclAsyncRunner(entries, Math.max(1, Math.min(WINDOWS_OWNER_ACL_TIMEOUT_MS, Math.floor(timeoutMs)))); + return result.success && !result.timedOut && windowsEntriesAclMatches(result.stdout, entries); + } catch { return false; } +} diff --git a/src/server/management/codex-cli-update-routes.ts b/src/server/management/codex-cli-update-routes.ts index 98e302e2282..3b9ac180dce 100644 --- a/src/server/management/codex-cli-update-routes.ts +++ b/src/server/management/codex-cli-update-routes.ts @@ -20,7 +20,7 @@ export async function handleCodexCliUpdateRoutes(ctx: ManagementContext): Promis const match = STATUS.exec(ctx.url.pathname); if (match && ctx.req.method === "GET") { try { - const job = readCodexCliUpdateJob(match[1]!); + const job = await readCodexCliUpdateJob(match[1]!); return job ? jsonResponse(job, 200, ctx.req, ctx.config) : jsonResponse({ error: "job_not_found" }, 404, ctx.req, ctx.config); } catch { return jsonResponse({ error: "job_state_unavailable" }, 503, ctx.req, ctx.config); } } @@ -39,7 +39,7 @@ export async function handleCodexCliUpdateRoutes(ctx: ManagementContext): Promis return jsonResponse({ error: "invalid_request" }, 400, ctx.req, ctx.config); } try { - const admitted = admitCodexCliUpdateJob({ requestId: value.requestId, planId: value.planId, confirmed: true }); + const admitted = await admitCodexCliUpdateJob({ requestId: value.requestId, planId: value.planId, confirmed: true }); if (admitted.kind === "accepted") { const work = runCodexCliUpdateJob(admitted.job.jobId, admitted.generation, () => applyCodexCliUpdate(value.planId as string, { inspectionDeps: codexCliUpdateInspectionDeps() })); diff --git a/structure/INDEX.md b/structure/INDEX.md index b1b5065bbb2..5647acb6007 100644 --- a/structure/INDEX.md +++ b/structure/INDEX.md @@ -216,6 +216,7 @@ A source area can be described by more than one doc, because these docs are orga | `src/codex/cli-update-files.ts` | [`codex-cli-update.md`](codex-cli-update.md) | | `src/codex/cli-update-inspection-context.ts` | [`codex-cli-update.md`](codex-cli-update.md) | | `src/codex/cli-update-job-execution.ts` | [`codex-cli-update.md`](codex-cli-update.md) | +| `src/codex/cli-update-job-store-windows.ts` | [`codex-cli-update.md`](codex-cli-update.md) | | `src/codex/cli-update-job.ts` | [`codex-cli-update.md`](codex-cli-update.md) | | `src/codex/cli-update-lease.ts` | [`codex-cli-update.md`](codex-cli-update.md) | | `src/codex/cli-update-plan.ts` | [`codex-cli-update.md`](codex-cli-update.md) | diff --git a/structure/cli-management.md b/structure/cli-management.md index b594dd73456..93377e427fa 100644 --- a/structure/cli-management.md +++ b/structure/cli-management.md @@ -155,5 +155,6 @@ no prompt is issued during background start/sync. The eligibility, drift refusal and recovery. No endpoint or credential is printed by this command. `ocx system codex-cli-update apply` durably admits and records the apply result under the protected config root; `--request-id` provides a retry key, and reusing it with a different plan conflicts. `ocx system codex-cli-update status ` reads the same path-free terminal DTO without a GUI token. A record left queued/running after owner restart is reported ambiguous with unknown owner state. Apply remains limited to the supported POSIX npm-global installation. +On Windows the same durable record and status read are served by the handle-bound job-store backend described in `codex-cli-update.md`. The capability declarations document `ocx system codex-cli-update plan`, `apply` and `status` as the local counterparts of `GET /api/codex/cli-update/plan`, `POST /api/codex/cli-update/apply` and `GET /api/codex/cli-update/jobs/{jobId}`. They share the plan builder and durable job owner with the dashboard rather than sending HTTP management requests, so their driven-route lists remain empty. CLI apply requires `--yes`; HTTP apply separately requires explicit confirmation from a trusted loopback GUI session. The GUI parity test resolves these registered capabilities, checks their mutation classifications and documented resource counterparts, and includes all three resources in endpoint coverage. diff --git a/structure/codex-cli-update.md b/structure/codex-cli-update.md index 122a743bdb6..2f610bf33e7 100644 --- a/structure/codex-cli-update.md +++ b/structure/codex-cli-update.md @@ -143,6 +143,8 @@ Windows apply remains refused. Shims and non-npm managers remain unsupported. `src/cli/codex-cli-update.ts` records confirmed CLI apply in that ledger and exposes `status ` from the same protected record, without requiring a GUI token. `src/server/management/codex-cli-update-routes.ts` exposes a proof-bound read-only plan, confirmed `gui-session` plus trusted-loopback apply admission, and authenticated status. HTTP apply returns after durable admission; disconnect does not cancel the worker. Dashboard display data is never accepted back as a candidate, prefix, executable or managed-selection attestation. There is no dashboard job UI in this contract. +On Windows, `src/codex/cli-update-job-store-windows.ts` maps the same ledger contract onto NTFS handle semantics and the public admit/read/finish API in `src/codex/cli-update-job.ts` dispatches to it asynchronously. Each ancestor from the volume root is opened relative to its already-held parent — never a re-parsed path — and refused on any reparse attribute, then evaluated under the "trusted-ancestry" ACL policy (owner is the serving token or SYSTEM/Administrators/TrustedInstaller; untrusted grantees may hold read and, on admin-owned ancestors only, broad create-only rights). The config-root parent and a pre-existing config root are verified under the "write-private" policy and are never rehardened; only a directory the call itself creates is icacls-hardened and held to the strict "private" rule. File identity is the volume serial plus 128-bit file ID with single link and no pending delete, the dev:ino analog. Ledger writes go through a private hardened temp that is byte/ACL/identity-verified, renamed while its handle is held, re-read through the published name, and flushed at the directory. The admission mutex is a mkdir lock plus an owner record released only while both held pins still show this call identity and content, so a leftover lock blocks later admission exactly like POSIX. ACL snapshots are taken per phase through the shared bounded PowerShell read in `src/lib/windows-owner-acl.ts` (async variant), not per path. Pins bind each object to its verified state for the operation and the private DACL confines writers, but this is not a proof against concurrent mutation by the serving account or an elevated process, which sit inside the trust boundary as on POSIX. The backend is x64-only; cross-account (other-SID) and ARM64 behavior are unverified. + The ledger retains at most 64 jobs, including terminal jobs, and never prunes request IDs automatically. At capacity it refuses new request IDs with `store_full`; existing matching retries still return the same job. Capacity expansion and operator recovery require a separate design that preserves deduplication and job-status evidence. diff --git a/structure/manifest.json b/structure/manifest.json index 77abe172306..5ad6c0054af 100644 --- a/structure/manifest.json +++ b/structure/manifest.json @@ -179,6 +179,7 @@ "src/codex/cli-update-apply.ts", "src/codex/cli-update-job.ts", "src/codex/cli-update-job-execution.ts", + "src/codex/cli-update-job-store-windows.ts", "src/codex/cli-update-inspection-context.ts", "bin/ocx.mjs", "src/codex/cli-update-artifacts.ts", diff --git a/structure/remote-link.md b/structure/remote-link.md index ca7404d3b16..0c538eced3b 100644 --- a/structure/remote-link.md +++ b/structure/remote-link.md @@ -123,6 +123,8 @@ On Windows, `src/lib/windows-owner-acl.ts` independently verifies the intent dir This Windows check covers the intent directory and record, not ancestor replacement, parent delete rights, privileged restore authority or handles opened before permissions became private. Current ACL snapshots cannot revoke previously granted handle access, and final check/unlink is not atomic with path substitution. A matching commitment, including one in an unsafe subtree, reaches a synchronous verifier that can block the server for its bounded five-second subprocess interval inside the CLI's ten-second fetch timeout; executable resolution and spawn setup precede that timeout. CLI disposal may pay one additional verification. +The same module also offers per-entry confined policies evaluated by the identical line protocol in one batch read: "write-private" (serving-token ownership with no untrusted write, create, delete or owner/DACL control) and "trusted-ancestry" (owner may also be SYSTEM, Administrators or TrustedInstaller; untrusted grantees may hold read and, on admin-owned ancestors only, broad create-only rights). Deny ACEs are accepted there because they only remove access, and inherit-only ACEs are skipped as templates for children; allow ACEs must stay ordinary non-callback records. An async verifier reports through the shared bounded-subprocess deadline rather than holding the event loop. The Codex CLI update job store consumes these policies for ancestry and non-hardened verification; the strict per-entry "private" rule and every existing caller contract are unchanged. + Publication/consumption reject symlinked homes and intent directories; ancestors and final check/unlink are not pinned by atomic directory handles, so adversarial replacement needs path-write authority outside the runtime-reader threat model. A capacity-refused mint burns its intent; a fresh command can retry after capacity recovers. Headless services need no terminal, password, biometric API or security key. Hub authorization and redemption/CSRF/expiry/role-change/paired-join checks remain. Capability version `v2` rejects mixed CLI/server versions for standalone and Hub commands. Coverage: `tests/gui/gui-pair-delivery.test.ts`, `tests/gui/gui-pair-client.test.ts`, and real headless CLI/server `tests/gui/gui-pair-http.test.ts`. Stale pairing-intent cleanup considers records older than sixty seconds and examines at most 256 directory entries per command; fresh and foreign entries count toward that budget. Unexamined records are left for later best-effort sweeps, without a guarantee that a record behind persistent foreign entries will be reached. This bounds per-entry metadata/deletion work, not directory-enumeration memory or individual filesystem-call duration. ## Home enrollment and CLI diff --git a/tests/cli/cli-codex-cli-update.test.ts b/tests/cli/cli-codex-cli-update.test.ts index 40e436b2d83..efb89c52443 100644 --- a/tests/cli/cli-codex-cli-update.test.ts +++ b/tests/cli/cli-codex-cli-update.test.ts @@ -127,7 +127,7 @@ describe("Codex CLI job persistence failures", () => { try { const kinds = status === "queued" || status === "running" ? ["duplicate"] as const : ["accepted", "duplicate"] as const; for (const kind of kinds) { - admission.mockReturnValue({ kind, job, generation: randomUUID() }); runs = 0; worker.mockClear(); + admission.mockResolvedValue({ kind, job, generation: randomUUID() }); runs = 0; worker.mockClear(); for (const json of [false, true]) { log.mockClear(); expect(await handleCodexCliUpdateCommand(["apply", "--plan-id", result.planId, "--yes", ...(json ? ["--json"] : [])], @@ -141,7 +141,7 @@ describe("Codex CLI job persistence failures", () => { } finally { log.mockRestore(); admission.mockRestore(); worker.mockRestore(); } }); - test.skipIf(process.platform === "win32" || process.geteuid?.() === undefined).each(["open", "partial-write"] as const)("owned lock initialization %s failure rolls back and permits later admission", failure => { + test.skipIf(process.platform === "win32" || process.geteuid?.() === undefined).each(["open", "partial-write"] as const)("owned lock initialization %s failure rolls back and permits later admission", async failure => { const { outer, configDir } = privateStoreRoot("ocx-cli-update-lock-init-"); const lock = join(configDir, "codex-cli-update-jobs", "admission.lock"), owner = join(lock, "owner.json"); const nativeOpen = fs.openSync, nativeWrite = fs.writeFileSync; @@ -157,15 +157,15 @@ describe("Codex CLI job persistence failures", () => { return nativeWrite(...args); }); try { - expect(() => admitCodexCliUpdateJob({ requestId: "initializer-fault-01", planId: result.planId, confirmed: true, configDir })).toThrow(); + await expect(admitCodexCliUpdateJob({ requestId: "initializer-fault-01", planId: result.planId, confirmed: true, configDir })).rejects.toThrow(); expect(existsSync(lock)).toBe(false); open.mockRestore(); write.mockRestore(); - expect(admitCodexCliUpdateJob({ requestId: "initializer-fault-01", planId: result.planId, confirmed: true, configDir }).kind).toBe("accepted"); + expect((await admitCodexCliUpdateJob({ requestId: "initializer-fault-01", planId: result.planId, confirmed: true, configDir })).kind).toBe("accepted"); expect(existsSync(lock)).toBe(false); } finally { open.mockRestore(); write.mockRestore(); rmSync(outer, { recursive: true }); } }); - test.skipIf(process.platform === "win32" || process.geteuid?.() === undefined).each(["directory", "owner-file"] as const)("initialization rollback refuses a replaced %s identity", replaced => { + test.skipIf(process.platform === "win32" || process.geteuid?.() === undefined).each(["directory", "owner-file"] as const)("initialization rollback refuses a replaced %s identity", async replaced => { const { outer, configDir } = privateStoreRoot("ocx-cli-update-lock-replaced-"); const lock = join(configDir, "codex-cli-update-jobs", "admission.lock"), owner = join(lock, "owner.json"); const nativeOpen = fs.openSync, nativeWrite = fs.writeFileSync; @@ -188,7 +188,7 @@ describe("Codex CLI job persistence failures", () => { return nativeWrite(...args); }); try { - expect(() => admitCodexCliUpdateJob({ requestId: "initializer-replaced-01", planId: result.planId, confirmed: true, configDir })).toThrow(); + await expect(admitCodexCliUpdateJob({ requestId: "initializer-replaced-01", planId: result.planId, confirmed: true, configDir })).rejects.toThrow(); open.mockRestore(); write.mockRestore(); const replacementLock = lstatSync(lock), replacementOwner = lstatSync(owner); expect(replacementLock.mode & 0o777).toBe(0o700); @@ -202,7 +202,7 @@ describe("Codex CLI job persistence failures", () => { } expect(readFileSync(owner, "utf8")).toBe("replacement"); expect(existsSync(replaced === "directory" ? `${lock}-original` : `${owner}-original`)).toBe(true); - expect(() => admitCodexCliUpdateJob({ requestId: "initializer-replaced-02", planId: result.planId, confirmed: true, configDir })).toThrow("job_store_busy"); + await expect(admitCodexCliUpdateJob({ requestId: "initializer-replaced-02", planId: result.planId, confirmed: true, configDir })).rejects.toThrow("job_store_busy"); } finally { open.mockRestore(); write.mockRestore(); rmSync(outer, { recursive: true }); } }); @@ -238,6 +238,17 @@ describe("Codex CLI job persistence failures", () => { expect(attempts).toBe(8); expect(waits).toHaveLength(7); }); + test("an asynchronously rejected busy contention yields and retries the awaited result", async () => { + let attempts = 0; const waits: number[] = []; + expect(await retryCodexCliJobStoreOperation(async () => { if (++attempts < 3) throw new CodexCliJobStoreBusyError(); return 7; }, + async ms => { waits.push(ms); })).toBe(7); + expect(attempts).toBe(3); expect(waits).toEqual([25, 25]); + attempts = 0; waits.length = 0; + await expect(retryCodexCliJobStoreOperation(async () => { attempts++; throw new CodexCliJobStoreBusyError(); }, + async ms => { waits.push(ms); })).rejects.toThrow("job_store_busy"); + expect(attempts).toBe(8); expect(waits).toHaveLength(7); + }); + test("unknown accepted result codes cannot poison a later store read", () => { const safe = normalizeCodexCliJobResult({ ...result, reason: "private/unknown-reason", cleanupErrors: ["private/cleanup"] as never }); @@ -259,7 +270,7 @@ describe("Codex CLI job persistence failures", () => { const messages: string[] = [], previous = console.log; try { console.log = value => { messages.push(String(value)); }; - expect(await handleCodexCliUpdateCommand(["status", randomUUID(), "--json"], { readJob: () => null })).toBe(1); + expect(await handleCodexCliUpdateCommand(["status", randomUUID(), "--json"], { readJob: async () => null })).toBe(1); expect(JSON.parse(messages.join("\n"))).toEqual({ error: "job_unavailable" }); } finally { console.log = previous; } }); @@ -269,14 +280,14 @@ describe("Codex CLI job persistence failures", () => { const lock = join(configDir, "codex-cli-update-jobs", "admission.lock"); let timer: ReturnType | undefined, runs = 0; try { - const admitted = admitCodexCliUpdateJob({ requestId: "lock-retry-00001", planId: result.planId, confirmed: true, configDir }); + const admitted = await admitCodexCliUpdateJob({ requestId: "lock-retry-00001", planId: result.planId, confirmed: true, configDir }); const job = await runCodexCliUpdateJob(admitted.job.jobId, admitted.generation, async () => { runs++; mkdirSync(lock, { mode: 0o700 }); timer = setTimeout(() => fs.rmdirSync(lock), 50); return result; }, configDir); expect(job).toMatchObject({ status: "applied", outcome: "applied", reason: "verified_target" }); - expect(readCodexCliUpdateJob(admitted.job.jobId, configDir)).toMatchObject({ outcome: "applied" }); + expect(await readCodexCliUpdateJob(admitted.job.jobId, configDir)).toMatchObject({ outcome: "applied" }); expect(runs).toBe(1); } finally { clearTimeout(timer); rmSync(outer, { recursive: true }); } }); @@ -285,7 +296,7 @@ describe("Codex CLI job persistence failures", () => { const { outer, configDir } = privateStoreRoot("ocx-cli-update-fsync-retry-"); let sync: ReturnType | undefined; try { - const admitted = admitCodexCliUpdateJob({ requestId: "fsync-retry-0001", planId: result.planId, confirmed: true, configDir }); + const admitted = await admitCodexCliUpdateJob({ requestId: "fsync-retry-0001", planId: result.planId, confirmed: true, configDir }); const nativeSync = fs.fsyncSync; let calls = 0; const job = await runCodexCliUpdateJob(admitted.job.jobId, admitted.generation, async () => { sync = spyOn(fs, "fsyncSync").mockImplementation(fd => { @@ -296,7 +307,7 @@ describe("Codex CLI job persistence failures", () => { }, configDir); expect(job).toMatchObject({ status: "applied", reason: "verified_target" }); expect(calls).toBeGreaterThan(2); - expect(readCodexCliUpdateJob(admitted.job.jobId, configDir)).toMatchObject({ outcome: "applied", reason: "verified_target" }); + expect(await readCodexCliUpdateJob(admitted.job.jobId, configDir)).toMatchObject({ outcome: "applied", reason: "verified_target" }); expect(JSON.parse(readFileSync(join(configDir, "codex-cli-update-jobs", "jobs.json"), "utf8")).jobs[0].revision).toBe(2); } finally { sync?.mockRestore(); rmSync(outer, { recursive: true }); } }); @@ -304,10 +315,10 @@ describe("Codex CLI job persistence failures", () => { test.skipIf(process.platform === "win32" || process.geteuid?.() === undefined)("an accepted unknown result is persisted as readable fixed-code uncertainty", async () => { const { outer, configDir } = privateStoreRoot("ocx-cli-update-unknown-result-"); try { - const admitted = admitCodexCliUpdateJob({ requestId: "unknown-result-01", planId: result.planId, confirmed: true, configDir }); + const admitted = await admitCodexCliUpdateJob({ requestId: "unknown-result-01", planId: result.planId, confirmed: true, configDir }); await runCodexCliUpdateJob(admitted.job.jobId, admitted.generation, async () => ({ ...result, reason: "late_result", cleanupErrors: ["private/path"] as never }), configDir); - expect(readCodexCliUpdateJob(admitted.job.jobId, configDir)).toMatchObject({ status: "ambiguous", + expect(await readCodexCliUpdateJob(admitted.job.jobId, configDir)).toMatchObject({ status: "ambiguous", reason: "job_result_unknown", leaseState: "unknown", cleanupErrors: [] }); } finally { rmSync(outer, { recursive: true }); } }); @@ -316,7 +327,7 @@ describe("Codex CLI job persistence failures", () => { const { outer, configDir } = privateStoreRoot("ocx-cli-update-worker-fence-"); let release = () => {}, pending: Promise | undefined; try { - const admitted = admitCodexCliUpdateJob({ requestId: "worker-fence-001", planId: result.planId, confirmed: true, configDir }); + const admitted = await admitCodexCliUpdateJob({ requestId: "worker-fence-001", planId: result.planId, confirmed: true, configDir }); pending = runCodexCliUpdateJob(admitted.job.jobId, admitted.generation, () => new Promise(resolve => { release = () => resolve(result); }), configDir); await Promise.resolve(); @@ -333,29 +344,29 @@ describe("Codex CLI job persistence failures", () => { try { let first = ""; for (let index = 0; index < 64; index++) { - const admitted = admitCodexCliUpdateJob({ requestId: `retained-key-${index}`, planId: result.planId, confirmed: true, configDir }); + const admitted = await admitCodexCliUpdateJob({ requestId: `retained-key-${index}`, planId: result.planId, confirmed: true, configDir }); if (index === 0) first = admitted.job.jobId; expect(await runCodexCliUpdateJob(admitted.job.jobId, admitted.generation, async () => result, configDir)).toMatchObject({ outcome: "applied" }); } - expect(() => admitCodexCliUpdateJob({ requestId: "retained-key-65", planId: result.planId, confirmed: true, configDir })).toThrow("store_full"); - expect(admitCodexCliUpdateJob({ requestId: "retained-key-0", planId: result.planId, confirmed: true, configDir })) + await expect(admitCodexCliUpdateJob({ requestId: "retained-key-65", planId: result.planId, confirmed: true, configDir })).rejects.toThrow("store_full"); + expect(await admitCodexCliUpdateJob({ requestId: "retained-key-0", planId: result.planId, confirmed: true, configDir })) .toMatchObject({ kind: "duplicate", job: { jobId: first, outcome: "applied" } }); } finally { rmSync(outer, { recursive: true }); } }, 30_000); }); describe("Codex CLI update CLI", () => { - test.skipIf(process.platform === "win32" || process.geteuid?.() === undefined)("confirmed admission creates one missing config root without making reads write", () => { + test.skipIf(process.platform === "win32" || process.geteuid?.() === undefined)("confirmed admission creates one missing config root without making reads write", async () => { const { outer, parent } = privateStoreParent("ocx-cli-update-missing-parent-"); const configDir = join(parent, "ocx"); try { - expect(readCodexCliUpdateJob("12345678-1234-1234-1234-123456789012", configDir)).toBeNull(); + expect(await readCodexCliUpdateJob("12345678-1234-1234-1234-123456789012", configDir)).toBeNull(); expect(existsSync(configDir)).toBe(false); const missingParent = join(parent, "not-created"); - expect(() => admitCodexCliUpdateJob({ requestId: "no-recursive-root-001", planId: "c".repeat(32), confirmed: true, - configDir: join(missingParent, "ocx") })).toThrow(); + await expect(admitCodexCliUpdateJob({ requestId: "no-recursive-root-001", planId: "c".repeat(32), confirmed: true, + configDir: join(missingParent, "ocx") })).rejects.toThrow(); expect(existsSync(missingParent)).toBe(false); - const accepted = admitCodexCliUpdateJob({ requestId: "missing-root-admission-01", planId: "a".repeat(32), confirmed: true, configDir }); + const accepted = await admitCodexCliUpdateJob({ requestId: "missing-root-admission-01", planId: "a".repeat(32), confirmed: true, configDir }); const stat = lstatSync(configDir); expect({ canonicalEqual: realpathSync.native(configDir) === configDir, @@ -368,11 +379,11 @@ describe("Codex CLI update CLI", () => { } finally { rmSync(outer, { recursive: true }); } }); - test.skipIf(process.platform === "win32" || process.geteuid?.() === undefined)("confirmed admission reuses an existing canonical config root", () => { + test.skipIf(process.platform === "win32" || process.geteuid?.() === undefined)("confirmed admission reuses an existing canonical config root", async () => { const { outer, configDir } = privateStoreRoot("ocx-cli-update-existing-root-"); try { const before = lstatSync(configDir); - const accepted = admitCodexCliUpdateJob({ requestId: "existing-root-admission-01", planId: "b".repeat(32), confirmed: true, configDir }); + const accepted = await admitCodexCliUpdateJob({ requestId: "existing-root-admission-01", planId: "b".repeat(32), confirmed: true, configDir }); const after = lstatSync(configDir); expect(accepted.kind).toBe("accepted"); expect(after.dev).toBe(before.dev); @@ -381,11 +392,11 @@ describe("Codex CLI update CLI", () => { } finally { rmSync(outer, { recursive: true }); } }); - test.skipIf(!hasTrustedStickyTmp())("missing config root admission allows a root-owned sticky temporary ancestor", () => { + test.skipIf(!hasTrustedStickyTmp())("missing config root admission allows a root-owned sticky temporary ancestor", async () => { const stickyTmp = realpathSync.native("/tmp"); const directConfig = join(stickyTmp, `ocx-cli-update-direct-sticky-${randomUUID()}`); - expect(() => admitCodexCliUpdateJob({ requestId: "direct-sticky-parent-001", planId: "d".repeat(32), - confirmed: true, configDir: directConfig })).toThrow("unsafe_store"); + await expect(admitCodexCliUpdateJob({ requestId: "direct-sticky-parent-001", planId: "d".repeat(32), + confirmed: true, configDir: directConfig })).rejects.toThrow("unsafe_store"); expect(existsSync(directConfig)).toBe(false); const outer = realpathSync.native(mkdtempSync(join(stickyTmp, "ocx-cli-update-sticky-parent-"))); try { @@ -394,7 +405,7 @@ describe("Codex CLI update CLI", () => { mkdirSync(parent, { mode: 0o700 }); chmodSync(parent, 0o700); const configDir = join(parent, "ocx"); - const accepted = admitCodexCliUpdateJob({ requestId: "sticky-parent-admission-001", planId: "d".repeat(32), confirmed: true, configDir }); + const accepted = await admitCodexCliUpdateJob({ requestId: "sticky-parent-admission-001", planId: "d".repeat(32), confirmed: true, configDir }); const stat = lstatSync(configDir); expect(accepted.kind).toBe("accepted"); expect((stat.mode & 0o077) === 0 && stat.isDirectory() && !stat.isSymbolicLink() @@ -403,13 +414,13 @@ describe("Codex CLI update CLI", () => { } finally { rmSync(outer, { recursive: true }); } }); - test.skipIf(process.platform === "win32" || process.geteuid?.() === undefined)("missing config root admission refuses a non-sticky shared ancestor before mkdir", () => { + test.skipIf(process.platform === "win32" || process.geteuid?.() === undefined)("missing config root admission refuses a non-sticky shared ancestor before mkdir", async () => { const { outer, parent } = privateStoreParent("ocx-cli-update-shared-ancestor-"); const configDir = join(parent, "ocx"); try { chmodSync(outer, 0o777); - expect(() => admitCodexCliUpdateJob({ requestId: "shared-ancestor-admission-01", planId: "e".repeat(32), - confirmed: true, configDir })).toThrow("unsafe_store"); + await expect(admitCodexCliUpdateJob({ requestId: "shared-ancestor-admission-01", planId: "e".repeat(32), + confirmed: true, configDir })).rejects.toThrow("unsafe_store"); expect(existsSync(configDir)).toBe(false); } finally { chmodSync(outer, 0o700); @@ -422,7 +433,7 @@ describe("Codex CLI update CLI", () => { let release: (() => void) | undefined; let pending: Promise | undefined; try { - const accepted = admitCodexCliUpdateJob({ requestId: "existing-shared-admission-001", planId: "f".repeat(32), + const accepted = await admitCodexCliUpdateJob({ requestId: "existing-shared-admission-001", planId: "f".repeat(32), confirmed: true, configDir }); pending = runCodexCliUpdateJob(accepted.job.jobId, accepted.generation, () => new Promise(resolve => { release = () => resolve({ outcome: "applied", reason: "verified_target", planId: "f".repeat(32), @@ -434,13 +445,13 @@ describe("Codex CLI update CLI", () => { const ledger = join(configDir, "codex-cli-update-jobs", "jobs.json"); const before = readFileSync(ledger); chmodSync(outer, 0o777); - expect(() => admitCodexCliUpdateJob({ requestId: "blocked-existing-admission-001", planId: "1".repeat(32), - confirmed: true, configDir })).toThrow("unsafe_store"); - expect(() => readCodexCliUpdateJob(accepted.job.jobId, configDir)).toThrow("unsafe_store"); - expect(() => finishCodexCliUpdateJob(accepted.job.jobId, accepted.generation, 1, { + await expect(admitCodexCliUpdateJob({ requestId: "blocked-existing-admission-001", planId: "1".repeat(32), + confirmed: true, configDir })).rejects.toThrow("unsafe_store"); + await expect(readCodexCliUpdateJob(accepted.job.jobId, configDir)).rejects.toThrow("unsafe_store"); + await expect(finishCodexCliUpdateJob(accepted.job.jobId, accepted.generation, 1, { outcome: "applied", reason: "verified_target", planId: "f".repeat(32), installedVersion: "1.0.0", targetVersion: "1.1.0", installerExitCode: 0, leaseRetained: false, verificationPid: null, cleanupErrors: [], - }, configDir)).toThrow("unsafe_store"); + }, configDir)).rejects.toThrow("unsafe_store"); await expect(runCodexCliUpdateJob(accepted.job.jobId, accepted.generation, async () => { throw new Error("unsafe ancestry must reject before running"); }, configDir)).rejects.toThrow("unsafe_store"); expect(readFileSync(ledger)).toEqual(before); @@ -479,12 +490,12 @@ describe("Codex CLI update CLI", () => { try { const requestId = "retry-key-0000001"; const planId = "a".repeat(32); - const accepted = admitCodexCliUpdateJob({ requestId, planId, confirmed: true, configDir }); - const replay = admitCodexCliUpdateJob({ requestId, planId, confirmed: true, configDir }); + const accepted = await admitCodexCliUpdateJob({ requestId, planId, confirmed: true, configDir }); + const replay = await admitCodexCliUpdateJob({ requestId, planId, confirmed: true, configDir }); expect(accepted.kind).toBe("accepted"); expect(replay.kind).toBe("duplicate"); expect(replay.job.jobId).toBe(accepted.job.jobId); - expect(() => admitCodexCliUpdateJob({ requestId, planId: "b".repeat(32), confirmed: true, configDir })).toThrow("idempotency_conflict"); + await expect(admitCodexCliUpdateJob({ requestId, planId: "b".repeat(32), confirmed: true, configDir })).rejects.toThrow("idempotency_conflict"); const previousHome = process.env.OPENCODEX_HOME; process.env.OPENCODEX_HOME = configDir; const statusLogs: string[] = []; @@ -500,7 +511,7 @@ describe("Codex CLI update CLI", () => { let installerRuns = 0; const terminal = await runCodexCliUpdateJob(accepted.job.jobId, accepted.generation, async () => { - expect(readCodexCliUpdateJob(accepted.job.jobId, configDir)).toMatchObject({ status: "running", ownerState: "active" }); + expect(await readCodexCliUpdateJob(accepted.job.jobId, configDir)).toMatchObject({ status: "running", ownerState: "active" }); installerRuns += 1; return { outcome: "applied", reason: "verified_target", planId, installedVersion: "1.0.0", targetVersion: "1.1.0", installerExitCode: 0, leaseRetained: false, verificationPid: null, cleanupErrors: [] }; @@ -508,11 +519,11 @@ describe("Codex CLI update CLI", () => { expect(installerRuns).toBe(1); expect(terminal).toMatchObject({ status: "applied", outcome: "applied", ownerState: "finished" }); expect(JSON.stringify(terminal)).not.toMatch(/ownerPid|generation|payloadHash|requestKey|token|path/i); - const stale = finishCodexCliUpdateJob(accepted.job.jobId, accepted.generation, 1, + const stale = await finishCodexCliUpdateJob(accepted.job.jobId, accepted.generation, 1, { outcome: "refused", reason: "late_result", planId, installedVersion: null, targetVersion: null, installerExitCode: null, leaseRetained: false, verificationPid: null, cleanupErrors: [] }, configDir); expect(stale?.status).toBe("applied"); - expect(readCodexCliUpdateJob(accepted.job.jobId, configDir)?.status).toBe("applied"); + expect((await readCodexCliUpdateJob(accepted.job.jobId, configDir))?.status).toBe("applied"); } finally { rmSync(outer, { recursive: true }); } }); @@ -521,16 +532,16 @@ describe("Codex CLI update CLI", () => { let release: () => void = () => {}; let pending: Promise | undefined; try { - const accepted = admitCodexCliUpdateJob({ requestId: "restart-key-00001", planId: "c".repeat(32), confirmed: true, configDir }); + const accepted = await admitCodexCliUpdateJob({ requestId: "restart-key-00001", planId: "c".repeat(32), confirmed: true, configDir }); pending = runCodexCliUpdateJob(accepted.job.jobId, accepted.generation, () => new Promise(resolve => { release = () => resolve({ outcome: "applied", reason: "verified_target", planId: "c".repeat(32), installedVersion: "1.0.0", targetVersion: "1.1.0", installerExitCode: 0, leaseRetained: false, verificationPid: null, cleanupErrors: [] }); }), configDir); await Promise.resolve(); - expect(readCodexCliUpdateJob(accepted.job.jobId, configDir)).toMatchObject({ status: "running", ownerState: "active" }); + expect(await readCodexCliUpdateJob(accepted.job.jobId, configDir)).toMatchObject({ status: "running", ownerState: "active" }); const modulePath = new URL("../../src/codex/cli-update-job.ts", import.meta.url).href; - const script = `import { readCodexCliUpdateJob } from ${JSON.stringify(modulePath)}; console.log(JSON.stringify(readCodexCliUpdateJob(${JSON.stringify(accepted.job.jobId)})));`; + const script = `import { readCodexCliUpdateJob } from ${JSON.stringify(modulePath)}; console.log(JSON.stringify(await readCodexCliUpdateJob(${JSON.stringify(accepted.job.jobId)})));`; const child = spawnSync(process.execPath, ["-e", script], { encoding: "utf8", timeout: 5_000, env: { ...process.env, OPENCODEX_HOME: configDir } }); expect(child.error).toBeUndefined(); @@ -541,10 +552,10 @@ describe("Codex CLI update CLI", () => { release(); const completed = await pending; expect(completed).toMatchObject({ status: "applied", ownerState: "finished" }); - expect(finishCodexCliUpdateJob(accepted.job.jobId, accepted.generation, 1, + expect((await finishCodexCliUpdateJob(accepted.job.jobId, accepted.generation, 1, { outcome: "refused", reason: "late_result", planId: "c".repeat(32), installedVersion: null, targetVersion: null, installerExitCode: null, leaseRetained: false, verificationPid: null, - cleanupErrors: [] }, configDir)?.status).toBe("applied"); + cleanupErrors: [] }, configDir))?.status).toBe("applied"); } finally { release(); if (pending) await pending.catch(() => undefined); @@ -555,7 +566,7 @@ describe("Codex CLI update CLI", () => { test.skipIf(process.platform === "win32" || process.geteuid?.() === undefined)("an exception without close evidence preserves child and lease uncertainty", async () => { const { outer, configDir } = privateStoreRoot("ocx-cli-update-unknown-child-test-"); try { - const accepted = admitCodexCliUpdateJob({ requestId: "unknown-child-0001", planId: "f".repeat(32), confirmed: true, configDir }); + const accepted = await admitCodexCliUpdateJob({ requestId: "unknown-child-0001", planId: "f".repeat(32), confirmed: true, configDir }); const result = await runCodexCliUpdateJob(accepted.job.jobId, accepted.generation, async () => { throw new Error("unobservable child exception"); }, configDir); expect(result).toMatchObject({ status: "ambiguous", outcome: "ambiguous", ownerState: "finished", @@ -567,7 +578,7 @@ describe("Codex CLI update CLI", () => { const { outer, configDir } = privateStoreRoot("ocx-cli-update-live-child-test-"); let child: ChildProcess | undefined; try { - const accepted = admitCodexCliUpdateJob({ requestId: "live-child-exception-001", planId: "9".repeat(32), confirmed: true, configDir }); + const accepted = await admitCodexCliUpdateJob({ requestId: "live-child-exception-001", planId: "9".repeat(32), confirmed: true, configDir }); const result = await runCodexCliUpdateJob(accepted.job.jobId, accepted.generation, async () => { child = spawn(process.execPath, ["-e", "setInterval(() => {}, 1000)"], { stdio: "ignore", windowsHide: true }); await new Promise((resolve, reject) => { diff --git a/tests/codex-integration/codex-cli-update-apply.test.ts b/tests/codex-integration/codex-cli-update-apply.test.ts index d4c5a28ff99..1233694732a 100644 --- a/tests/codex-integration/codex-cli-update-apply.test.ts +++ b/tests/codex-integration/codex-cli-update-apply.test.ts @@ -416,7 +416,7 @@ describe.skipIf(!posix)("Codex CLI private-prefix POSIX apply", () => { // original generation's late completion. const configDir = join(f.home, "ocx"); const restartPlanId = "b".repeat(32); - const oldOwner = admitCodexCliUpdateJob({ requestId: "server-restart-owner-unknown-01", planId: restartPlanId, confirmed: true, configDir }); + const oldOwner = await admitCodexCliUpdateJob({ requestId: "server-restart-owner-unknown-01", planId: restartPlanId, confirmed: true, configDir }); oldOwnerWork = runCodexCliUpdateJob(oldOwner.job.jobId, oldOwner.generation, () => new Promise(resolve => { finishOldOwner = () => resolve({ outcome: "applied", reason: "verified_target", planId: restartPlanId, installedVersion: "1.0.0", targetVersion: "1.1.0", installerExitCode: 0, leaseRetained: false, @@ -441,7 +441,7 @@ describe.skipIf(!posix)("Codex CLI private-prefix POSIX apply", () => { } finishOldOwner?.(); const lateResult = await oldOwnerWork; - const staleWrite = finishCodexCliUpdateJob(oldOwner.job.jobId, oldOwner.generation, 1, + const staleWrite = await finishCodexCliUpdateJob(oldOwner.job.jobId, oldOwner.generation, 1, { outcome: "refused", reason: "late_result", planId: restartPlanId, installedVersion: null, targetVersion: null, installerExitCode: null, leaseRetained: false, verificationPid: null, cleanupErrors: [] }, configDir); diff --git a/tests/fixtures/file-size-baseline.json b/tests/fixtures/file-size-baseline.json index 1997a97adf3..aa7a0c8c200 100644 --- a/tests/fixtures/file-size-baseline.json +++ b/tests/fixtures/file-size-baseline.json @@ -19,6 +19,7 @@ ".github/scripts/issue-quality.test.cjs": 2143, "gui/src/pages/Models.tsx": 2792, "gui/src/styles.css": 2958, + "scripts/test-layout/layout.json": 2000, "src/adapters/openai-chat.ts": 822, "src/adapters/openai-responses.ts": 6, "src/bridge.ts": 7, diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index be96044d4ad..458f3cd0396 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -1306,6 +1306,7 @@ "win-exec.test.ts": "windows", "win-paths.test.ts": "windows", "windows-acl-start-cost.test.ts": "windows", "windows-atomic-replace.test.ts": "windows", + "windows-codex-cli-job-store.test.ts": "windows", "windows-deploy-close-regressions.test.ts": "windows", "windows-elevation-spawn.test.ts": "windows", "windows-elevation.test.ts": "windows", diff --git a/tests/windows/windows-codex-cli-job-store.test.ts b/tests/windows/windows-codex-cli-job-store.test.ts new file mode 100644 index 00000000000..dfeb1ef55df --- /dev/null +++ b/tests/windows/windows-codex-cli-job-store.test.ts @@ -0,0 +1,316 @@ +/** + * Windows Codex CLI update job store — the real handle-bound backend in + * src/codex/cli-update-job-store-windows.ts exercised through the public + * admit/run/read/finish API in src/codex/cli-update-job.ts, plus the + * mixed-policy ACL parser in src/lib/windows-owner-acl.ts it consumes. + * + * Parser cases are pure and run everywhere. Backend cases require Windows x64: + * they run the actual PowerShell ACL verifier and icacls hardening against a + * private fixture under %TEMP%, so each case carries a real-subprocess budget. + * Cross-account (other-SID) and ARM64 behavior are unverified by design. + */ +import { afterEach, describe, expect, spyOn, test } from "bun:test"; +import { spawnSync } from "node:child_process"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import * as fs from "node:fs"; +import { join } from "node:path"; +import { admitCodexCliUpdateJob, readCodexCliUpdateJob, runCodexCliUpdateJob } from "../../src/codex/cli-update-job"; +import { setWindowsOwnerAclAsyncRunnerForTests, windowsEntriesAclMatches, + type WindowsAclEntry } from "../../src/lib/windows-owner-acl"; +import { hardenSecretDirAsync } from "../../src/lib/windows-secret-acl"; +import { handleCodexCliUpdateRoutes } from "../../src/server/management/codex-cli-update-routes"; +import { handleCodexCliUpdateCommand } from "../../src/cli/codex-cli-update"; +import type { ManagementContext } from "../../src/server/management/context"; +import type { OcxConfig } from "../../src/types"; +import type { CodexCliApplyResult } from "../../src/codex/cli-update-apply"; + +const USER_SID = "S-1-5-21-9-9-9-1001"; +const FOREIGN_SID = "S-1-5-21-8-8-8-2002"; +const ADMIN_SID = "S-1-5-32-544"; +const SYSTEM_SID = "S-1-5-18"; +const TRUSTED_INSTALLER_SID = "S-1-5-80-956008885-3418522649-1831038044-1853292631-2271478464"; +const FULL_CONTROL = 2032127; +// Matches READ_MASK in src/lib/windows-owner-acl.ts: generic read/execute plus +// SYNCHRONIZE, READ_CONTROL and the attribute/EA/list bits. Nothing write-side. +const READ_ONLY_MASK = (0x80000000 | 0x20000000 | 0x100000 | 0x20000 | 0x80 | 0x20 | 0x8 | 0x1) >>> 0; +const CREATE_ONLY_MASK = 0x4 | 0x2; +const WRITE_BITS = 0x100 /* FILE_WRITE_ATTRIBUTES */ | 0x2 /* FILE_WRITE_DATA */; + +function aclRun(user: string, tokenOwner: string, elevated: boolean, entries: readonly { owner: string; flags?: number; aces: readonly string[] }[]): string { + const lines = [`U|${user}|${tokenOwner}|${elevated ? "True" : "False"}`]; + entries.forEach((entry, index) => { + lines.push(`E|${index}|${entry.owner}|${entry.flags ?? 4}|${entry.aces.length}`); + for (const ace of entry.aces) lines.push(`A|${index}|${ace}`); + }); + lines.push("END"); + return lines.join("\n"); +} +const allow = (mask: number, sid: string, flags = 0) => `0|${flags}|${mask}|${sid}|False`; +const deny = (mask: number, sid: string) => `1|0|${mask}|${sid}|False`; +const opaqueAce = (objectType = 5) => `${objectType}|X`; + +describe("windows owner ACL mixed policies", () => { + const userRun = (entries: readonly { owner: string; flags?: number; aces: readonly string[] }[], + user = USER_SID, tokenOwner = USER_SID, elevated = false) => aclRun(user, tokenOwner, elevated, entries); + + test("a foreign read-only grantee passes write-private but an untrusted writer refuses", () => { + const entries: WindowsAclEntry[] = [{ path: "C:\\cfg", directory: true, policy: "write-private" }]; + expect(windowsEntriesAclMatches(userRun([{ owner: USER_SID, aces: [allow(FULL_CONTROL, USER_SID), allow(READ_ONLY_MASK, FOREIGN_SID)] }]), entries)).toBe(true); + expect(windowsEntriesAclMatches(userRun([{ owner: USER_SID, aces: [allow(FULL_CONTROL, USER_SID), allow(READ_ONLY_MASK | WRITE_BITS, FOREIGN_SID)] }]), entries)).toBe(false); + expect(windowsEntriesAclMatches(userRun([{ owner: USER_SID, aces: [allow(FULL_CONTROL, USER_SID), allow(CREATE_ONLY_MASK, FOREIGN_SID)] }]), entries)).toBe(false); + // Kernel-equivalent owners are trusted independent of this token's elevation + // state: a SYSTEM-owned entry granting only SYSTEM is confined either way. + expect(windowsEntriesAclMatches(userRun([{ owner: SYSTEM_SID, aces: [allow(FULL_CONTROL, SYSTEM_SID)] }]), entries)).toBe(true); + // Regression: an Administrators-owned profile parent must pass under a + // filtered token exactly as under an elevated one — elevation only gates the + // strict "private" objects this process hardened itself. + const adminOwned = [{ owner: ADMIN_SID, aces: [allow(FULL_CONTROL, USER_SID), allow(FULL_CONTROL, ADMIN_SID)] }]; + expect(windowsEntriesAclMatches(userRun(adminOwned, USER_SID, USER_SID, false), entries)).toBe(true); + expect(windowsEntriesAclMatches(userRun(adminOwned, USER_SID, ADMIN_SID, true), entries)).toBe(true); + // A trusted owner does not launder an untrusted writer. + const adminOwnedLeaky = [{ owner: ADMIN_SID, aces: [allow(FULL_CONTROL, USER_SID), allow(READ_ONLY_MASK | WRITE_BITS, FOREIGN_SID)] }]; + expect(windowsEntriesAclMatches(userRun(adminOwnedLeaky, USER_SID, USER_SID, false), entries)).toBe(false); + }); + + test("trusted ancestry accepts kernel-owned read-sharing and admin-owned create-only ancestors", () => { + const entries: WindowsAclEntry[] = [{ path: "C:\\", directory: true, policy: "trusted-ancestry" }]; + for (const owner of [SYSTEM_SID, ADMIN_SID, TRUSTED_INSTALLER_SID]) { + expect(windowsEntriesAclMatches(userRun([{ owner, aces: [allow(READ_ONLY_MASK, FOREIGN_SID)] }]), entries)).toBe(true); + } + // The stock C:\\ shape: a trusted owner may share create-only rights broadly. + expect(windowsEntriesAclMatches(userRun([{ owner: ADMIN_SID, aces: [allow(READ_ONLY_MASK | CREATE_ONLY_MASK, FOREIGN_SID)] }]), entries)).toBe(true); + // A user-owned ancestor may not share create rights, and nobody may share delete/write. + expect(windowsEntriesAclMatches(userRun([{ owner: USER_SID, aces: [allow(CREATE_ONLY_MASK, FOREIGN_SID)] }]), entries)).toBe(false); + expect(windowsEntriesAclMatches(userRun([{ owner: ADMIN_SID, aces: [allow(READ_ONLY_MASK | CREATE_ONLY_MASK | 0x10000, FOREIGN_SID)] }]), entries)).toBe(false); + expect(windowsEntriesAclMatches(userRun([{ owner: FOREIGN_SID, aces: [allow(FULL_CONTROL, USER_SID)] }]), entries)).toBe(false); + }); + + test("confined policies tolerate deny ACEs but refuse callback, opaque and NULL-DACL output", () => { + const entries: WindowsAclEntry[] = [{ path: "C:\\cfg", directory: true, policy: "write-private" }]; + expect(windowsEntriesAclMatches(userRun([{ owner: USER_SID, aces: [allow(FULL_CONTROL, USER_SID), deny(FULL_CONTROL, FOREIGN_SID)] }]), entries)).toBe(true); + expect(windowsEntriesAclMatches(userRun([{ owner: USER_SID, aces: [allow(FULL_CONTROL, USER_SID), "9|0|2032127|S-1-5-11|True"] }]), entries)).toBe(false); + expect(windowsEntriesAclMatches(userRun([{ owner: USER_SID, aces: [allow(FULL_CONTROL, USER_SID), opaqueAce()] }]), entries)).toBe(false); + // NULL DACL: count -1 grants Everyone full access; never acceptable anywhere. + const nullDacl = [`U|${USER_SID}|${USER_SID}|False`, `E|0|${USER_SID}|4|-1`, "END"].join("\n"); + expect(windowsEntriesAclMatches(nullDacl, entries)).toBe(false); + }); + + test("a mixed batch evaluates ancestry, parent and store policies in one protocol run", () => { + const entries: WindowsAclEntry[] = [ + { path: "C:\\", directory: true, policy: "trusted-ancestry" }, + { path: "C:\\parent", directory: true, policy: "write-private" }, + { path: "C:\\parent\\ocx\\codex-cli-update-jobs", directory: true }, + ]; + const good = userRun([ + { owner: ADMIN_SID, aces: [allow(READ_ONLY_MASK | CREATE_ONLY_MASK, FOREIGN_SID)] }, + { owner: USER_SID, aces: [allow(FULL_CONTROL, USER_SID)] }, + { owner: USER_SID, flags: 4100, aces: [`0|3|${FULL_CONTROL}|${USER_SID}|False`] }, + ]); + expect(windowsEntriesAclMatches(good, entries)).toBe(true); + // The private leaf still requires the hardened shape; a relaxed leaf refuses the batch. + const weakLeaf = userRun([ + { owner: ADMIN_SID, aces: [allow(READ_ONLY_MASK | CREATE_ONLY_MASK, FOREIGN_SID)] }, + { owner: USER_SID, aces: [allow(FULL_CONTROL, USER_SID)] }, + { owner: USER_SID, flags: 4100, aces: [`0|3|${FULL_CONTROL}|${USER_SID}|False`, `0|3|${READ_ONLY_MASK}|${FOREIGN_SID}|False`] }, + ]); + expect(windowsEntriesAclMatches(weakLeaf, entries)).toBe(false); + }); +}); + +describe("Windows CLI update job store backend", () => { + const result: CodexCliApplyResult = { outcome: "applied", reason: "verified_target", planId: "a".repeat(32), + installedVersion: "1.0.0", targetVersion: "1.1.0", installerExitCode: 0, leaseRetained: false, + verificationPid: null, cleanupErrors: [] }; + const fixtures: string[] = []; + const envStack: (string | undefined)[] = []; + + afterEach(() => { + setWindowsOwnerAclAsyncRunnerForTests(null); + while (envStack.length) { + const previous = envStack.pop()!; + if (previous === undefined) delete process.env.OPENCODEX_HOME; else process.env.OPENCODEX_HOME = previous; + } + while (fixtures.length) rmSync(fixtures.pop()!, { recursive: true, force: true }); + }); + + const winOnly = process.platform === "win32" && process.arch === "x64"; + // The backend verifies ancestry instead of trusting a well-known root, so the + // fixture must sit under ancestors that actually pass: %TEMP% carries a foreign + // full-control grantee here and an automatic mkdir anywhere inherits foreign + // write. One hardened root under the volume root gives a clean write-private + // parent with a single trusted ancestor — the same hardening the store applies + // to directories it creates itself. + async function fixture(prefix: string): Promise<{ outer: string; configDir: string }> { + const outer = mkdtempSync("C:\\" + prefix); + fixtures.push(outer); + await hardenSecretDirAsync(outer, { required: true }); + return { outer, configDir: join(outer, "ocx") }; + } + function useHome(configDir: string): void { + envStack.push(process.env.OPENCODEX_HOME); + process.env.OPENCODEX_HOME = configDir; + } + function managementCtx(req: Request): ManagementContext { + return { req, url: new URL(req.url), + config: { port: 10100, providers: {} } as OcxConfig, deps: {}, version: "test", + principal: "gui-session", trustedLoopbackIngress: true, guiSessionIssuance: "loopback", + convergeCodexCatalog: async () => ({ status: "skipped", reason: "not-requested", retryable: false }), + syncClaudeAgentDefsBestEffort: async () => {} }; + } + + test.skipIf(!winOnly)("admission dedupes, conflicts, applies and reads back through the real private store", async () => { + const { configDir } = await fixture("ocx-winjob-admit-"); + const accepted = await admitCodexCliUpdateJob({ requestId: "win-admit-0001", planId: result.planId, confirmed: true, configDir }); + expect(accepted.kind).toBe("accepted"); + const replay = await admitCodexCliUpdateJob({ requestId: "win-admit-0001", planId: result.planId, confirmed: true, configDir }); + expect(replay).toMatchObject({ kind: "duplicate", job: { jobId: accepted.job.jobId } }); + await expect(admitCodexCliUpdateJob({ requestId: "win-admit-0001", planId: "b".repeat(32), confirmed: true, configDir })) + .rejects.toThrow("idempotency_conflict"); + let runs = 0; + const terminal = await runCodexCliUpdateJob(accepted.job.jobId, accepted.generation, async () => { runs++; return result; }, configDir); + expect(terminal).toMatchObject({ status: "applied", outcome: "applied", ownerState: "finished" }); + expect(runs).toBe(1); + expect(await readCodexCliUpdateJob(accepted.job.jobId, configDir)).toMatchObject({ status: "applied", ownerState: "finished" }); + expect(existsSync(join(configDir, "codex-cli-update-jobs", "jobs.json"))).toBe(true); + expect(existsSync(join(configDir, "codex-cli-update-jobs", "admission.lock"))).toBe(false); + expect(JSON.stringify(terminal)).not.toMatch(/ownerPid|generation|payloadHash|requestKey|token|path/i); + }, 180_000); + + test.skipIf(!winOnly)("a second process dedupes the admitted request key and conflicts on a different payload", async () => { + const { configDir } = await fixture("ocx-winjob-twoproc-"); + mkdirSync(configDir); + const modulePath = new URL("../../src/codex/cli-update-job.ts", import.meta.url).href; + const accepted = await admitCodexCliUpdateJob({ requestId: "win-two-proc-01", planId: result.planId, confirmed: true, configDir }); + const script = `import { admitCodexCliUpdateJob } from ${JSON.stringify(modulePath)}; + const planId = process.argv.at(-1); + try { + const admitted = await admitCodexCliUpdateJob({ requestId: "win-two-proc-01", planId, confirmed: true }); + console.log(JSON.stringify({ kind: admitted.kind, jobId: admitted.job.jobId })); + } catch (error) { console.error(error instanceof Error ? error.message : String(error)); process.exit(3); }`; + const env = { ...process.env, OPENCODEX_HOME: configDir }; + const duplicate = spawnSync(process.execPath, ["-e", script, result.planId], { encoding: "utf8", timeout: 120_000, env }); + expect(duplicate.error).toBeUndefined(); + expect(duplicate.stderr).toBe(""); + expect(duplicate.status).toBe(0); + expect(JSON.parse(duplicate.stdout.trim())).toEqual({ kind: "duplicate", jobId: accepted.job.jobId }); + const conflict = spawnSync(process.execPath, ["-e", script, "d".repeat(32)], { encoding: "utf8", timeout: 120_000, env }); + expect(conflict.status).toBe(3); + expect(conflict.stderr).toContain("idempotency_conflict"); + }, 300_000); + + test.skipIf(!winOnly)("an ACL verification failure refuses before the engine runs", async () => { + const { configDir } = await fixture("ocx-winjob-aclfail-"); + const accepted = await admitCodexCliUpdateJob({ requestId: "win-acl-fail-01", planId: result.planId, confirmed: true, configDir }); + let runs = 0; + setWindowsOwnerAclAsyncRunnerForTests(async () => ({ success: false, timedOut: false, stdout: "" })); + try { + await expect(runCodexCliUpdateJob(accepted.job.jobId, accepted.generation, + async () => { runs++; return result; }, configDir)).rejects.toThrow("unsafe_store"); + expect(runs).toBe(0); + } finally { setWindowsOwnerAclAsyncRunnerForTests(null); } + // The refused begin left the durable record intact; with this process having + // released ownership it reads back honestly as an owner-unknown queued record. + expect(await readCodexCliUpdateJob(accepted.job.jobId, configDir)).toMatchObject( + { status: "ambiguous", ownerState: "unknown", reason: "owner_state_unknown" }); + // A replaced (corrupt) ledger body refuses reads and later admission the same closed way. + writeFileSync(join(configDir, "codex-cli-update-jobs", "jobs.json"), "not a ledger", "utf8"); + await expect(readCodexCliUpdateJob(accepted.job.jobId, configDir)).rejects.toThrow(); + await expect(admitCodexCliUpdateJob({ requestId: "win-acl-fail-02", planId: result.planId, confirmed: true, configDir })).rejects.toThrow(); + }, 180_000); + + test.skipIf(!winOnly)("a transient persistence failure retries the completed revision without rerunning the engine", async () => { + const { configDir } = await fixture("ocx-winjob-retry-"); + const accepted = await admitCodexCliUpdateJob({ requestId: "win-retry-0001", planId: result.planId, confirmed: true, configDir }); + const nativeSync = fs.fsyncSync; + let calls = 0, runs = 0; + let sync: ReturnType | undefined; + try { + const terminal = await runCodexCliUpdateJob(accepted.job.jobId, accepted.generation, async () => { + runs++; + // Installed here so call numbering starts at completion: call 1 is the + // finish admission-lock owner fsync, call 2 the ledger temp-body fsync — + // a transient failure there retries the same revision, never the engine. + sync = spyOn(fs, "fsyncSync").mockImplementation(fd => { + if (++calls === 2) throw Object.assign(new Error("injected transient sync failure"), { code: "EBUSY" }); + return nativeSync(fd); + }); + return result; + }, configDir); + expect(terminal).toMatchObject({ status: "applied", reason: "verified_target" }); + } finally { sync?.mockRestore(); } + expect(runs).toBe(1); + expect(calls).toBeGreaterThan(2); + expect(await readCodexCliUpdateJob(accepted.job.jobId, configDir)).toMatchObject({ outcome: "applied" }); + const ledger = JSON.parse(readFileSync(join(configDir, "codex-cli-update-jobs", "jobs.json"), "utf8")) as { jobs: { revision: number }[] }; + expect(ledger.jobs[0]!.revision).toBe(2); + }, 180_000); + + test.skipIf(!winOnly)("a restarted reader reports an unfinished owner unknown without claiming it stopped", async () => { + const { configDir } = await fixture("ocx-winjob-restart-"); + const accepted = await admitCodexCliUpdateJob({ requestId: "win-restart-001", planId: result.planId, confirmed: true, configDir }); + let release: () => void = () => {}; + const pending = runCodexCliUpdateJob(accepted.job.jobId, accepted.generation, () => new Promise(resolve => { + release = () => resolve(result); + }), configDir); + // A status probe may see a transient unsafe_store (e.g. a killed ACL-read child + // racing the running phase); the poll retries rather than asserting transport. + const probe = async () => { + try { return await readCodexCliUpdateJob(accepted.job.jobId, configDir); } + catch { return null; } + }; + try { + for (let i = 0; i < 200 && (await probe())?.status !== "running"; i++) await Bun.sleep(10); + expect(await probe()).toMatchObject({ status: "running", ownerState: "active" }); + const modulePath = new URL("../../src/codex/cli-update-job.ts", import.meta.url).href; + const script = `import { readCodexCliUpdateJob } from ${JSON.stringify(modulePath)}; + console.log(JSON.stringify(await readCodexCliUpdateJob(${JSON.stringify(accepted.job.jobId)})));`; + const child = spawnSync(process.execPath, ["-e", script], { encoding: "utf8", timeout: 120_000, + env: { ...process.env, OPENCODEX_HOME: configDir } }); + expect(child.error).toBeUndefined(); + expect(child.status).toBe(0); + expect(JSON.parse(child.stdout.trim())).toMatchObject({ status: "ambiguous", outcome: "ambiguous", + ownerState: "unknown", lifetimeUncertain: true, reason: "owner_state_unknown" }); + } finally { + release(); + await pending.catch(() => undefined); + } + expect(await pending).toMatchObject({ status: "applied", ownerState: "finished" }); + }, 300_000); + + test.skipIf(!winOnly)("HTTP apply advertises retry on a leftover lock and status reads the same durable record", async () => { + const { configDir } = await fixture("ocx-winjob-http-"); + const accepted = await admitCodexCliUpdateJob({ requestId: "win-http-00001", planId: result.planId, confirmed: true, configDir }); + useHome(configDir); + // Crash-shaped residue: an admission.lock the backend never wrote keeps later admission busy. + mkdirSync(join(configDir, "codex-cli-update-jobs", "admission.lock")); + const applyReq = () => new Request("http://localhost/api/codex/cli-update/apply", { method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ requestId: "win-http-other-1", planId: "e".repeat(32), confirmed: true }) }); + const busy = await handleCodexCliUpdateRoutes(managementCtx(applyReq())); + expect(busy?.status).toBe(503); + expect(await busy?.json()).toEqual({ error: "job_store_busy" }); + expect(busy?.headers.get("Retry-After")).toBe("1"); + rmSync(join(configDir, "codex-cli-update-jobs", "admission.lock"), { recursive: true }); + const status = await handleCodexCliUpdateRoutes(managementCtx( + new Request(`http://localhost/api/codex/cli-update/jobs/${accepted.job.jobId}`))); + expect(status?.status).toBe(200); + expect(await status?.json()).toMatchObject({ jobId: accepted.job.jobId, status: "queued", ownerState: "active" }); + const logs: string[] = []; + const oldLog = console.log; + try { + console.log = (...values: unknown[]) => logs.push(values.map(String).join(" ")); + expect(await handleCodexCliUpdateCommand(["status", accepted.job.jobId, "--json"])).toBe(0); + } finally { console.log = oldLog; } + expect(JSON.parse(logs[0]!)).toMatchObject({ jobId: accepted.job.jobId, status: "queued" }); + }, 180_000); + + test.skipIf(!winOnly)("a config path that is a file refuses before any write", async () => { + const { configDir } = await fixture("ocx-winjob-filecfg-"); + writeFileSync(configDir, "not a directory", "utf8"); + let runs = 0; + await expect(admitCodexCliUpdateJob({ requestId: "win-file-cfg-01", planId: result.planId, confirmed: true, configDir })).rejects.toThrow(); + await expect(runCodexCliUpdateJob("12345678-1234-1234-1234-123456789012", "12345678-1234-1234-1234-123456789012", + async () => { runs++; return result; }, configDir)).rejects.toThrow(); + expect(runs).toBe(0); + }, 60_000); +}); From 6a2340ff3bd81052c916a022ed59ad54ef07321a Mon Sep 17 00:00:00 2001 From: Epinephrine Date: Fri, 23 Oct 2026 19:34:19 +0900 Subject: [PATCH 02/16] fix(codex): shard the Windows job-store layout entry and bound the ACL lookup comment Move windows-codex-cli-job-store.test.ts out of layout.json explicit into the new explicitShards map layout-windows-cli-update.json so layout.json stays at 1999 lines and needs no file-size cap entry; drop the baseline cap. Rewrite the async ACL runner comment: the read-only lookup is abandoned rather than awaited because the post-kill reap wait exists for handle-bearing icacls children, and a lingering killed child can still hold a transient Get-Acl handle on the verified owner, lock or ledger files and leave the lock busy until the kernel reaps it -- an availability limit, not a safety one; the verdict stays timedOut, the late result is never read, and the runner fails closed. --- scripts/test-layout/layout-windows-cli-update.json | 3 +++ scripts/test-layout/layout.json | 3 +-- src/lib/windows-owner-acl.ts | 14 +++++++++----- tests/fixtures/file-size-baseline.json | 1 - 4 files changed, 13 insertions(+), 8 deletions(-) create mode 100644 scripts/test-layout/layout-windows-cli-update.json diff --git a/scripts/test-layout/layout-windows-cli-update.json b/scripts/test-layout/layout-windows-cli-update.json new file mode 100644 index 00000000000..b30ad659eff --- /dev/null +++ b/scripts/test-layout/layout-windows-cli-update.json @@ -0,0 +1,3 @@ +{ + "windows-codex-cli-job-store.test.ts": "windows" +} diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index edbab9503fe..a2e791794ff 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -1,7 +1,7 @@ { "version": 1, "root": "tests", - "explicitShards": ["layout-clients.json", "layout-codex-collision.json", "layout-agent-assertion.json", "layout-chat.json", "layout-adapters.json", "layout-jev.json"], + "explicitShards": ["layout-clients.json", "layout-codex-collision.json", "layout-agent-assertion.json", "layout-chat.json", "layout-adapters.json", "layout-jev.json", "layout-windows-cli-update.json"], "explicit": { "combo-description.test.ts": "routing", "combo-description-catalog.test.ts": "codex-integration", "codex-provider-routing.test.ts": "codex-integration", "config-native-reasoning-retention.test.ts": "config", "server-native-reasoning-retention-merge.test.ts": "server", "server-native-reasoning-retention.test.ts": "server", "responses-retained-reasoning-stream-recovery.test.ts": "responses", "openai-native-hosted-search-history.test.ts": "adapters/openai", "responses-native-compact-search-history.test.ts": "responses", "config-hosted-search-projection.test.ts": "config", "gcp-credential-boundary.test.ts": "adapters/google", "gcp-sa-marker.test.ts": "adapters/google", "gcp-sa-adapter.test.ts": "adapters/google", "management-gcp-credential-input.test.ts": "server", "codex-preserve-signed-in.test.ts": "codex-integration", "cli-account-preservation.test.ts": "cli", "cli-provider-list-active.test.ts": "cli", "service-state-isolation.test.ts": "service", "compaction-history-framing.test.ts": "responses", "catalog-native-max-context.test.ts": "codex-integration", "protocol-argon-thought-boundary.test.ts": "responses", "service-log.test.ts": "service", "messages-native-empty-inline.test.ts": "claude-integration", "catalog-auto-review-override.test.ts": "codex-integration", "management-auto-review-settings.test.ts": "server", "management-auto-review-auth.test.ts": "server", "prefer-model-alias-ids.test.ts": "server", "responses-mimo-envelope.test.ts": "responses", "responses-call-id-linkage.test.ts": "responses", "bridge-terminal-refusal.test.ts": "adapters", "loop-terminal-refusal.test.ts": "images", "web-search-terminal-refusal.test.ts": "web-search", "combo-terminal-refusal.test.ts": "routing", "server-terminal-refusal-wiring.test.ts": "server", "incomplete-reason-normalization.test.ts": "responses", "advisor-retention.test.ts": "advisor", "responses-native-key-cooldown.test.ts": "responses", "responses-run-turn-key-readmission.test.ts": "responses", "qoder-spawn-readmission.test.ts": "adapters", "api-key-selection-cooldown.test.ts": "providers", "api-key-auto-pause-distribution.test.ts": "providers", "explicit-key-pause-oauth-boundary.test.ts": "providers", "server-chat-native-key-selection.test.ts": "server", "compact-key-selection.test.ts": "responses", "messages-native-key-selection.test.ts": "claude-integration", "service-proxy-env.test.ts": "service", "openai-chat-standalone-bare-block.test.ts": "adapters/openai", "owner-registry-isolation.test.ts": "config", @@ -1929,7 +1929,6 @@ "win-paths.test.ts": "windows", "windows-acl-start-cost.test.ts": "windows", "windows-atomic-replace.test.ts": "windows", - "windows-codex-cli-job-store.test.ts": "windows", "windows-deploy-close-regressions.test.ts": "windows", "windows-elevation-spawn.test.ts": "windows", "windows-elevation.test.ts": "windows", diff --git a/src/lib/windows-owner-acl.ts b/src/lib/windows-owner-acl.ts index 08490fa983c..3d57b3ed018 100644 --- a/src/lib/windows-owner-acl.ts +++ b/src/lib/windows-owner-acl.ts @@ -319,11 +319,15 @@ async function defaultWindowsOwnerAclAsyncRunner( } catch { return { success: false, timedOut: false, stdout: new Uint8Array() }; } - // No reap wait: the post-kill wait exists for handle-bearing children whose open - // handles gate a later removal (the icacls hardening case). This read-only lookup - // holds no such path, and waiting for a child that survived kill() would turn one - // transient stall into an unbounded caller stall; the principal-lookup runner in - // windows-user-principal.ts opts out for the same reason. + // No post-kill reap wait: that wait exists for children whose still-open handles + // gate a later removal (the icacls hardening case). This lookup holds no path it + // must release for the caller's own writes, but its transient Get-Acl handle can + // touch the verified owner, lock or ledger files, so abandoning a stalled child + // is not free: while it lingers it can refuse a markDelete and leave the lock + // busy until the kernel reaps it. That residual availability cost beats an + // unbounded caller stall — the verdict stays timedOut, the late result is never + // read, and the runner fails closed. windows-user-principal.ts opts out the + // same way for its read-only lookup. const { exitCode, timedOut } = await waitForSubprocessExit(proc, timeoutMs, 0); const stdout = !timedOut && proc.stdout ? await new Response(proc.stdout).text().catch(() => "") diff --git a/tests/fixtures/file-size-baseline.json b/tests/fixtures/file-size-baseline.json index aa7a0c8c200..1997a97adf3 100644 --- a/tests/fixtures/file-size-baseline.json +++ b/tests/fixtures/file-size-baseline.json @@ -19,7 +19,6 @@ ".github/scripts/issue-quality.test.cjs": 2143, "gui/src/pages/Models.tsx": 2792, "gui/src/styles.css": 2958, - "scripts/test-layout/layout.json": 2000, "src/adapters/openai-chat.ts": 822, "src/adapters/openai-responses.ts": 6, "src/bridge.ts": 7, From 9a75d220d8b42ed8ce6c013266c9e83ad832dffa Mon Sep 17 00:00:00 2001 From: Epinephrine Date: Fri, 23 Oct 2026 21:15:06 +0900 Subject: [PATCH 03/16] test(codex): wait for async job fixture entry --- tests/cli/cli-codex-cli-update.test.ts | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/tests/cli/cli-codex-cli-update.test.ts b/tests/cli/cli-codex-cli-update.test.ts index 66b2a9dba06..acc6b22e060 100644 --- a/tests/cli/cli-codex-cli-update.test.ts +++ b/tests/cli/cli-codex-cli-update.test.ts @@ -483,18 +483,25 @@ describe("Codex CLI update CLI", () => { test.skipIf(process.platform === "win32" || process.geteuid?.() === undefined)("existing config root refuses unsafe ancestry before admission or completion writes", async () => { const { outer, parent, configDir } = privateStoreRoot("ocx-cli-update-existing-shared-ancestor-"); - let release: (() => void) | undefined; + let release!: () => void; + let signalEngineEntered!: () => void; + const engineEntered = new Promise(resolve => { signalEngineEntered = resolve; }); + const engineResult = new Promise(resolve => { + release = () => resolve({ outcome: "applied", reason: "verified_target", planId: "f".repeat(32), + installedVersion: "1.0.0", targetVersion: "1.1.0", installerExitCode: 0, leaseRetained: false, + verificationPid: null, cleanupErrors: [] }); + }); let pending: Promise | undefined; try { const accepted = await admitCodexCliUpdateJob({ requestId: "existing-shared-admission-001", planId: "f".repeat(32), confirmed: true, configDir }); - pending = runCodexCliUpdateJob(accepted.job.jobId, accepted.generation, () => new Promise(resolve => { - release = () => resolve({ outcome: "applied", reason: "verified_target", planId: "f".repeat(32), - installedVersion: "1.0.0", targetVersion: "1.1.0", installerExitCode: 0, leaseRetained: false, - verificationPid: null, cleanupErrors: [] }); - }), configDir); - await Promise.resolve(); // Worker admission now yields before invoking the engine. - if (!release) throw new Error("fixture job owner did not enter its awaited operation"); + pending = runCodexCliUpdateJob(accepted.job.jobId, accepted.generation, () => { + signalEngineEntered(); + return engineResult; + }, configDir); + await Promise.race([engineEntered, pending.then(() => { + throw new Error("fixture job owner completed before entering its awaited operation"); + })]); const ledger = join(configDir, "codex-cli-update-jobs", "jobs.json"); const before = readFileSync(ledger); chmodSync(outer, 0o777); @@ -510,7 +517,7 @@ describe("Codex CLI update CLI", () => { expect(readFileSync(ledger)).toEqual(before); } finally { chmodSync(outer, 0o700); - release?.(); + release(); if (pending) await pending.catch(() => undefined); rmSync(outer, { recursive: true }); } From 2350f295f7444431646d4517b9684f8a45bbeac4 Mon Sep 17 00:00:00 2001 From: Epinephrine Date: Fri, 23 Oct 2026 21:17:03 +0900 Subject: [PATCH 04/16] docs(codex): clarify ACL timeout reap contract --- src/lib/windows-owner-acl.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/lib/windows-owner-acl.ts b/src/lib/windows-owner-acl.ts index 3d57b3ed018..0fc9440505d 100644 --- a/src/lib/windows-owner-acl.ts +++ b/src/lib/windows-owner-acl.ts @@ -298,9 +298,15 @@ export function verifyWindowsPrivateEntries(entries: readonly WindowsPrivateEntr /** * Async counterpart of the sync runner: the PowerShell read is identical, but the - * caller's event loop is not held for the child lifetime. The deadline is enforced by - * the shared bounded-subprocess helper, which kills the child and still waits for the - * kernel to release its handles before reporting the timeout. + * caller's event loop is not held while waiting. This read-only lookup opts out of the + * post-kill reap wait (`waitForSubprocessExit(..., 0)`: see the helper contract below): + * on timeout it requests termination and returns `timedOut` without waiting for the + * kernel to release the child's handles. The late exit/result is ignored and stdout is + * not read. Although this lookup holds no path its caller must release for its own + * writes, a stalled Get-Acl child may transiently touch owner, lock or ledger files and + * keep them busy (for example, causing a markDelete to fail) until the OS reaps it. + * This bounded availability risk avoids an unbounded caller stall; verification fails + * closed. */ async function defaultWindowsOwnerAclAsyncRunner( entries: readonly WindowsAclEntry[], From 7769bfc60b0626f3701a09dc281524253ab8ae9f Mon Sep 17 00:00:00 2001 From: Epinephrine Date: Fri, 23 Oct 2026 21:19:16 +0900 Subject: [PATCH 05/16] docs(codex): clarify ACL timeout residual handles --- src/lib/windows-owner-acl.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/lib/windows-owner-acl.ts b/src/lib/windows-owner-acl.ts index 0fc9440505d..b578460a83b 100644 --- a/src/lib/windows-owner-acl.ts +++ b/src/lib/windows-owner-acl.ts @@ -302,9 +302,8 @@ export function verifyWindowsPrivateEntries(entries: readonly WindowsPrivateEntr * post-kill reap wait (`waitForSubprocessExit(..., 0)`: see the helper contract below): * on timeout it requests termination and returns `timedOut` without waiting for the * kernel to release the child's handles. The late exit/result is ignored and stdout is - * not read. Although this lookup holds no path its caller must release for its own - * writes, a stalled Get-Acl child may transiently touch owner, lock or ledger files and - * keep them busy (for example, causing a markDelete to fail) until the OS reaps it. + * not read. A stalled Get-Acl child may transiently touch owner, lock or ledger files + * and keep them busy (for example, causing a markDelete to fail) until the OS reaps it. * This bounded availability risk avoids an unbounded caller stall; verification fails * closed. */ From 18a5d0e63d3c205e69e1a40a5cf70d1c6f253841 Mon Sep 17 00:00:00 2001 From: Epinephrine Date: Fri, 23 Oct 2026 22:18:11 +0900 Subject: [PATCH 06/16] test(codex): synchronize restart fixture with async owner --- tests/cli/cli-codex-cli-update.test.ts | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/tests/cli/cli-codex-cli-update.test.ts b/tests/cli/cli-codex-cli-update.test.ts index acc6b22e060..d02f0555a9f 100644 --- a/tests/cli/cli-codex-cli-update.test.ts +++ b/tests/cli/cli-codex-cli-update.test.ts @@ -589,16 +589,24 @@ describe("Codex CLI update CLI", () => { test.skipIf(process.platform === "win32" || process.geteuid?.() === undefined)("a new owner reports a live owner's job unknown; the original generation can finish without stale overwrite", async () => { const { outer, configDir } = privateStoreRoot("ocx-cli-update-restart-test-"); - let release: () => void = () => {}; + let release!: () => void; + let signalEngineEntered!: () => void; + const engineEntered = new Promise(resolve => { signalEngineEntered = resolve; }); + const engineResult = new Promise(resolve => { + release = () => resolve({ outcome: "applied", reason: "verified_target", planId: "c".repeat(32), + installedVersion: "1.0.0", targetVersion: "1.1.0", installerExitCode: 0, leaseRetained: false, + verificationPid: null, cleanupErrors: [] }); + }); let pending: Promise | undefined; try { const accepted = await admitCodexCliUpdateJob({ requestId: "restart-key-00001", planId: "c".repeat(32), confirmed: true, configDir }); - pending = runCodexCliUpdateJob(accepted.job.jobId, accepted.generation, () => new Promise(resolve => { - release = () => resolve({ outcome: "applied", reason: "verified_target", planId: "c".repeat(32), - installedVersion: "1.0.0", targetVersion: "1.1.0", installerExitCode: 0, leaseRetained: false, - verificationPid: null, cleanupErrors: [] }); - }), configDir); - await Promise.resolve(); + pending = runCodexCliUpdateJob(accepted.job.jobId, accepted.generation, () => { + signalEngineEntered(); + return engineResult; + }, configDir); + await Promise.race([engineEntered, pending.then(() => { + throw new Error("fixture job owner completed before entering its awaited operation"); + })]); expect(await readCodexCliUpdateJob(accepted.job.jobId, configDir)).toMatchObject({ status: "running", ownerState: "active" }); const modulePath = new URL("../../src/codex/cli-update-job.ts", import.meta.url).href; const script = `import { readCodexCliUpdateJob } from ${JSON.stringify(modulePath)}; console.log(JSON.stringify(await readCodexCliUpdateJob(${JSON.stringify(accepted.job.jobId)})));`; From cbfa1f3227efbb29943260a766f7f75226fb9e2f Mon Sep 17 00:00:00 2001 From: Epinephrine Date: Sun, 25 Oct 2026 08:09:55 +0900 Subject: [PATCH 07/16] test(codex): own Windows fixture before async preparation --- .../windows-codex-cli-job-store.test.ts | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/tests/windows/windows-codex-cli-job-store.test.ts b/tests/windows/windows-codex-cli-job-store.test.ts index e90405c6645..2b4bbba7894 100644 --- a/tests/windows/windows-codex-cli-job-store.test.ts +++ b/tests/windows/windows-codex-cli-job-store.test.ts @@ -158,12 +158,22 @@ describe("Windows CLI update job store backend", () => { // write. One hardened root under the volume root gives a clean write-private // parent with a single trusted ancestor — the same hardening the store applies // to directories it creates itself. - async function fixture(prefix: string): Promise<{ outer: string; configDir: string }> { + async function fixture(prefix: string, onCreated?: (root: string) => void): Promise<{ outer: string; configDir: string }> { const outer = mkdtempSync("C:\\" + prefix); + onCreated?.(outer); fixtures.push(outer); await hardenSecretDirAsync(outer, { required: true }); return { outer, configDir: join(outer, "ocx") }; } + async function withOwnedFixture(prefix: string, + run: (input: { outer: string; configDir: string }) => Promise): Promise { + let releaseBase: (() => void) | undefined; + try { + // Acquire before fixture()'s first await, including ACL hardening and admission. + const input = await fixture(prefix, root => { releaseBase = holdFixture(root); }); + await run(input); + } finally { releaseBase?.(); } + } function useHome(configDir: string): void { envStack.push(process.env.OPENCODEX_HOME); process.env.OPENCODEX_HOME = configDir; @@ -176,8 +186,7 @@ describe("Windows CLI update job store backend", () => { syncClaudeAgentDefsBestEffort: async () => {} }; } - test.skipIf(!winOnly)("private-store admission and status fence one production Worker completion", async () => { - const { outer, configDir } = await fixture("ocx-winjob-worker-"); + test.skipIf(!winOnly)("private-store admission and status fence one production Worker completion", () => withOwnedFixture("ocx-winjob-worker-", async ({ outer, configDir }) => { useHome(configDir); const accepted = await admitCodexCliUpdateJob({ requestId: "win-worker-once-01", planId: result.planId, confirmed: true, configDir }); expect(accepted.kind).toBe("accepted"); @@ -225,10 +234,9 @@ describe("Windows CLI update job store backend", () => { expect(await readCodexCliUpdateJob(accepted.job.jobId, configDir)).toMatchObject({ status: "refused", ownerState: "finished" }); const ledger = JSON.parse(readFileSync(join(configDir, "codex-cli-update-jobs", "jobs.json"), "utf8")) as { jobs: { revision: number }[] }; expect(ledger.jobs[0]!.revision).toBe(2); - }, 60_000); + }), 60_000); - test.skipIf(!winOnly)("private-store completion preserves an unknown live Worker without replaying its engine", async () => { - const { outer, configDir } = await fixture("ocx-winjob-worker-unknown-"); + test.skipIf(!winOnly)("private-store completion preserves an unknown live Worker without replaying its engine", () => withOwnedFixture("ocx-winjob-worker-unknown-", async ({ outer, configDir }) => { useHome(configDir); const accepted = await admitCodexCliUpdateJob({ requestId: "win-worker-unknown-01", planId: result.planId, confirmed: true, configDir }); const control = new Int32Array(new SharedArrayBuffer(Int32Array.BYTES_PER_ELEMENT * FIXTURE_CONTROL_WORDS)); @@ -271,7 +279,7 @@ describe("Windows CLI update job store backend", () => { } expect(codexCliUpdateUnjoinedWorkerCount()).toBe(initial); expect(await readCodexCliUpdateJob(accepted.job.jobId, configDir)).toMatchObject({ status: "ambiguous", reason: "job_result_unknown" }); - }, 60_000); + }), 60_000); test.skipIf(!winOnly)("admission dedupes, conflicts, applies and reads back through the real private store", async () => { const { configDir } = await fixture("ocx-winjob-admit-"); From 84930edb219de32e4d59dcd77a4709125233cf13 Mon Sep 17 00:00:00 2001 From: Epinephrine Date: Sun, 25 Oct 2026 13:40:13 +0900 Subject: [PATCH 08/16] fix(codex): register Windows store handles once --- src/codex/cli-update-job-store-windows.ts | 22 +++++++++++----------- structure/codex-cli-update.md | 5 +++++ 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/codex/cli-update-job-store-windows.ts b/src/codex/cli-update-job-store-windows.ts index f94a1eaf701..84f340346f8 100644 --- a/src/codex/cli-update-job-store-windows.ts +++ b/src/codex/cli-update-job-store-windows.ts @@ -391,7 +391,7 @@ async function prepareStore( let configPin: bigint; let configPrivate = false; try { - configPin = keep(ctx, relOpen(ctx, parentPin.handle, configName, true, ACCESS_READ, SHARE_DIR)); + configPin = relOpen(ctx, parentPin.handle, configName, true, ACCESS_READ, SHARE_DIR); inspect(ctx, configPin, true); } catch (error) { if ((error as NodeJS.ErrnoException).code !== "ENOENT" || !opts.createConfigRoot) { @@ -400,7 +400,7 @@ async function prepareStore( } // Only a confirmed apply admission may create one absent config-root level. mkdirSync(config); - configPin = keep(ctx, relOpen(ctx, parentPin.handle, configName, true, ACCESS_READ, SHARE_DIR)); + configPin = relOpen(ctx, parentPin.handle, configName, true, ACCESS_READ, SHARE_DIR); inspect(ctx, configPin, true); await hardenSecretDirAsync(config, { required: true }); configPrivate = true; @@ -409,7 +409,7 @@ async function prepareStore( const jobdirAccess = opts.createJobDir ? ACCESS_DIR_PIN : ACCESS_READ; let jobdir: bigint; try { - jobdir = keep(ctx, relOpen(ctx, configPin, CODEX_CLI_JOB_STORE_DIR, true, jobdirAccess, SHARE_DIR)); + jobdir = relOpen(ctx, configPin, CODEX_CLI_JOB_STORE_DIR, true, jobdirAccess, SHARE_DIR); inspect(ctx, jobdir, true); await verifyAcl([{ path: dir, directory: true, policy: "private" }]); } catch (error) { @@ -418,7 +418,7 @@ async function prepareStore( throw error; } mkdirSync(dir); - jobdir = keep(ctx, relOpen(ctx, configPin, CODEX_CLI_JOB_STORE_DIR, true, jobdirAccess, SHARE_DIR)); + jobdir = relOpen(ctx, configPin, CODEX_CLI_JOB_STORE_DIR, true, jobdirAccess, SHARE_DIR); inspect(ctx, jobdir, true); await hardenSecretDirAsync(dir, { required: true }); await verifyAcl([{ path: dir, directory: true, policy: "private" }]); @@ -433,7 +433,7 @@ async function readStoreBody(ctx: Ctx, prepared: Prepared): Promise { if ((error as NodeJS.ErrnoException).code === "EEXIST") throw new CodexCliJobStoreBusyError(); throw error; } - const lockDir = keep(ctx, relOpen(ctx, prepared.jobdir, CODEX_CLI_JOB_STORE_LOCK, true, - ACCESS_DIR_WRITE, SHARE_DIR)); + const lockDir = relOpen(ctx, prepared.jobdir, CODEX_CLI_JOB_STORE_LOCK, true, + ACCESS_DIR_WRITE, SHARE_DIR); const lockIdentity = inspect(ctx, lockDir, true); const ownerPath = join(prepared.lock, "owner.json"); const record = JSON.stringify({ version: 1, token: randomUUID() }); @@ -553,7 +553,7 @@ async function acquireLock(ctx: Ctx, prepared: Prepared): Promise { try { const fd = openSync(ownerPath, "wx", 0o600); try { writeFileSync(fd, record); fsyncSync(fd); } finally { closeSync(fd); } - owner = keep(ctx, relOpen(ctx, lockDir, "owner.json", false, ACCESS_LEAF, SHARE_LEAF)); + owner = relOpen(ctx, lockDir, "owner.json", false, ACCESS_LEAF, SHARE_LEAF); ownerIdentity = inspect(ctx, owner, false); if (ownerIdentity.links !== 1 || ownerIdentity.size !== recordBytes) throw unsafe(); if (!readAll(ctx, owner, recordBytes).equals(Buffer.from(record))) throw unsafe(); diff --git a/structure/codex-cli-update.md b/structure/codex-cli-update.md index 5409a885136..56df4b6c4cc 100644 --- a/structure/codex-cli-update.md +++ b/structure/codex-cli-update.md @@ -228,6 +228,11 @@ documented residual it always was. On Windows, `src/codex/cli-update-job-store-windows.ts` maps the same ledger contract onto NTFS handle semantics and the public admit/read/finish API in `src/codex/cli-update-job.ts` dispatches to it asynchronously. Each ancestor from the volume root is opened relative to its already-held parent — never a re-parsed path — and refused on any reparse attribute, then evaluated under the "trusted-ancestry" ACL policy (owner is the serving token or SYSTEM/Administrators/TrustedInstaller; untrusted grantees may hold read and, on admin-owned ancestors only, broad create-only rights). The config-root parent and a pre-existing config root are verified under the "write-private" policy and are never rehardened; only a directory the call itself creates is icacls-hardened and held to the strict "private" rule. File identity is the volume serial plus 128-bit file ID with single link and no pending delete, the dev:ino analog. Ledger writes go through a private hardened temp that is byte/ACL/identity-verified, renamed while its handle is held, re-read through the published name, and flushed at the directory. The admission mutex is a mkdir lock plus an owner record released only while both held pins still show this call identity and content, so a leftover lock blocks later admission exactly like POSIX. ACL snapshots are taken per phase through the shared bounded PowerShell read in `src/lib/windows-owner-acl.ts` (async variant), not per path. Pins bind each object to its verified state for the operation and the private DACL confines writers, but this is not a proof against concurrent mutation by the serving account or an elevated process, which sit inside the trust boundary as on POSIX. The backend is x64-only; cross-account (other-SID) and ARM64 behavior are unverified. +Each successful handle-relative `relOpen` registers its handle exactly once before returning it; +callers use that already-owned handle without wrapping it in another `keep` registration. +This applies to config/store pins, read/write/published leaves and lock/owner pins, so disposal +does not revisit a stale duplicate registration after the original handle has closed. + An ACL-query timeout fails closed without accepting a late result. Its PowerShell child may still hold transient handles while termination settles, so lock cleanup may fail and retain a busy mutex; the store does not automatically reclaim that leftover lock. Filtered-token behavior is also unverified. From 6ad702600e15804059166178cf88201a629e1227 Mon Sep 17 00:00:00 2001 From: Epinephrine Date: Sun, 25 Oct 2026 14:24:58 +0900 Subject: [PATCH 09/16] fix(scripts): detect diagnostic spawn timeout via exitedDueToTimeout Bun.spawnSync returns SyncSubprocess, whose bun-types 1.4.0 declaration has no signal member; exitCode is non-nullable number. The intended check is the declared exitedDueToTimeout flag, which is also correct where POSIX signals do not exist. --- scripts/test-temp-lock.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/test-temp-lock.ts b/scripts/test-temp-lock.ts index 86ad30dea1e..ee83fb254b7 100644 --- a/scripts/test-temp-lock.ts +++ b/scripts/test-temp-lock.ts @@ -78,7 +78,7 @@ const defaultDiagnostic: LockedTempDiagnostic = { return { stdout: decode(result.stdout), stderr: decode(result.stderr), - timedOut: result.signal === "SIGKILL" || result.exitCode === null, + timedOut: result.exitedDueToTimeout === true, }; }, children: targetBasename => listDirectChildren(process.pid, targetBasename), From 70b4e1eddb91d09219918ca8129b0e726459419c Mon Sep 17 00:00:00 2001 From: Codex Date: Sun, 25 Oct 2026 21:57:01 +0900 Subject: [PATCH 10/16] test(codex): assert unsafe-ledger admission refusal for disabled Windows apply Backport the one unsafe-ledger case from ecd1e887 (PR 7648): the Windows job store exists at this head, so a plan-id apply POST is admitted (202) and the disabled apply still refuses in the Worker. Keep the admission guard by making the config root an actual reparse point: the store refuses unsafe_store (503) before creating any ledger, and the direct apply stays refused with zero lease/install attempts. --- .../codex-integration/codex-cli-windows-plan.test.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/codex-integration/codex-cli-windows-plan.test.ts b/tests/codex-integration/codex-cli-windows-plan.test.ts index 551d3bd0853..e1409013fb1 100644 --- a/tests/codex-integration/codex-cli-windows-plan.test.ts +++ b/tests/codex-integration/codex-cli-windows-plan.test.ts @@ -1,6 +1,6 @@ import { afterEach, expect, test } from "bun:test"; import { spawnSync } from "node:child_process"; -import { copyFileSync, mkdirSync, mkdtempSync, readFileSync, renameSync, symlinkSync, truncateSync, writeFileSync } from "node:fs"; +import { copyFileSync, existsSync, mkdirSync, mkdtempSync, readFileSync, renameSync, symlinkSync, truncateSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { inspectCodexCliInstall, MAX_NATIVE_BACKING_BYTES } from "../../src/codex/cli-install-provenance"; @@ -112,8 +112,12 @@ native("a production plan is deterministic while Windows apply stays explicitly expect((await createCodexCliUpdatePlan(deps)).planId).not.toBe(first.planId); }); -native("a Windows plan id with applicable=false cannot enter a management apply job", async () => { +native("an unsafe Windows ledger refuses admission and a disabled plan never installs", async () => { const f = fixture(); + // A TEMP ancestry can be safe on hosted Windows. Create an actual unsupported + // config-root reparse point instead of inferring unsafe ACLs from the host path. + const ledgerTarget = join(f.root, "ledger-target"); mkdirSync(ledgerTarget); + symlinkSync(ledgerTarget, f.deps.configDir, "junction"); const deps = { inspectionDeps: f.deps, resolveTarget: () => ({ kind: "resolved" as const, version: "1.2.4", integrity: "sha512-" + Buffer.alloc(64, 1).toString("base64") }), scanProcesses: () => ({ kind: "observed" as const, processes: [] }) }; @@ -134,11 +138,13 @@ native("a Windows plan id with applicable=false cannot enter a management apply syncClaudeAgentDefsBestEffort: async () => {} }; const response = await handleCodexCliUpdateRoutes(ctx); expect(response?.status).toBe(503); - expect(await response?.json()).toEqual({ error: "unsupported_store" }); + expect(await response?.json()).toEqual({ error: "unsafe_store" }); + expect(existsSync(join(ledgerTarget, "codex-cli-update-jobs"))).toBe(false); let leaseAttempts = 0, installAttempts = 0; const apply = await applyCodexCliUpdate(plan.planId!, { platform: "win32", inspectionDeps: f.deps, acquireLease: () => { leaseAttempts += 1; return null; }, + resolveTarget: deps.resolveTarget, scanProcesses: deps.scanProcesses, install: async () => { installAttempts += 1; return { terminal: true, exitCode: 0 }; } }); expect(apply).toMatchObject({ outcome: "refused", reason: "windows_apply_disabled" }); expect(leaseAttempts).toBe(0); From 9197113c1fc6bcee02bd73538657bfe091d04b1a Mon Sep 17 00:00:00 2001 From: Codex Date: Sun, 25 Oct 2026 22:41:43 +0900 Subject: [PATCH 11/16] test(codex): bound the Windows restart-reader wait and surface child diagnostics The restart-reader poll was bounded by iterations, not wall-time, and could not observe pending settling early: a failed begin resolves with the queued DTO, so the loop spun heavy store reads to the 300s cap with no diagnostic. The poll now exits on pending settlement or a 120s deadline and asserts the settled or last-observed DTO. The fixture is held for the async job and the reader child through the existing fixtureOwners contract, and the child assertion surfaces error/status/signal/stdout/stderr. The plan file surfaces the same spawn diagnostics for the fsutil sparse precondition and the published Node CLI run. No limits, assertions, ACL or metadata coverage changed; production untouched. --- .../codex-cli-windows-plan.test.ts | 8 +++- .../windows-codex-cli-job-store.test.ts | 39 +++++++++++++------ 2 files changed, 33 insertions(+), 14 deletions(-) diff --git a/tests/codex-integration/codex-cli-windows-plan.test.ts b/tests/codex-integration/codex-cli-windows-plan.test.ts index e1409013fb1..74a0aa7bc8e 100644 --- a/tests/codex-integration/codex-cli-windows-plan.test.ts +++ b/tests/codex-integration/codex-cli-windows-plan.test.ts @@ -163,7 +163,9 @@ native("a published-size Windows npm backing binds; one past the shared finite l expect(process.env.SystemRoot).toBeDefined(); const sparse = spawnSync(join(process.env.SystemRoot!, "System32", "fsutil.exe"), ["sparse", "setflag", binary], { windowsHide: true, encoding: "utf8", timeout: 5_000 }); - expect(sparse.error).toBeUndefined(); expect(sparse.status).toBe(0); + expect({ error: sparse.error ?? null, status: sparse.status, signal: sparse.signal, + stdout: sparse.stdout?.trim() ?? null, stderr: sparse.stderr?.trim() ?? null }) + .toMatchObject({ error: null, status: 0 }); truncateSync(binary, 295_408_944); const report = await inspectCodexCliInstall(f.deps); expect(report.managed).toBe(true); expect(report.installContentDigest).toMatch(/^[a-f0-9]{64}$/); @@ -363,7 +365,9 @@ native("published Node CLI uses the production inspector and observed toolchain env: { ...process.env, CODEX_CLI_PATH: f.deps.env.CODEX_CLI_PATH, OPENCODEX_HOME: f.deps.configDir, OPENCODEX_BUN_PATH: process.execPath, OCX_NO_DESKTOP_HANDOFF: "1" }, }); - expect(run.error).toBeUndefined(); expect(run.status).toBe(0); + expect({ error: run.error ?? null, status: run.status, signal: run.signal, + stdout: run.stdout?.trim() ?? null, stderr: run.stderr?.trim() ?? null }) + .toMatchObject({ error: null, status: 0 }); const plan = JSON.parse(run.stdout); expect(plan.managed).toBe(true); expect(plan.targetVersion).toBe("1.2.4"); expect(plan.targetIntegrity).toBe(integrity); diff --git a/tests/windows/windows-codex-cli-job-store.test.ts b/tests/windows/windows-codex-cli-job-store.test.ts index 2b4bbba7894..07ae96cdc54 100644 --- a/tests/windows/windows-codex-cli-job-store.test.ts +++ b/tests/windows/windows-codex-cli-job-store.test.ts @@ -368,13 +368,14 @@ describe("Windows CLI update job store backend", () => { expect(ledger.jobs[0]!.revision).toBe(2); }, 180_000); - test.skipIf(!winOnly)("a restarted reader reports an unfinished owner unknown without claiming it stopped", async () => { - const { configDir } = await fixture("ocx-winjob-restart-"); + test.skipIf(!winOnly)("a restarted reader reports an unfinished owner unknown without claiming it stopped", + () => withOwnedFixture("ocx-winjob-restart-", async ({ outer, configDir }) => { const accepted = await admitCodexCliUpdateJob({ requestId: "win-restart-001", planId: result.planId, confirmed: true, configDir }); let release: () => void = () => {}; + const releaseStore = holdFixture(outer); const pending = runCodexCliUpdateJob(accepted.job.jobId, accepted.generation, () => new Promise(resolve => { release = () => resolve(result); - }), configDir); + }), configDir).finally(releaseStore); // A status probe may see a transient unsafe_store (e.g. a killed ACL-read child // racing the running phase); the poll retries rather than asserting transport. const probe = async () => { @@ -382,23 +383,37 @@ describe("Windows CLI update job store backend", () => { catch { return null; } }; try { - for (let i = 0; i < 200 && (await probe())?.status !== "running"; i++) await Bun.sleep(10); - expect(await probe()).toMatchObject({ status: "running", ownerState: "active" }); + // Bounded wall-time poll that also watches pending's own settlement: a failed + // begin resolves early with the queued DTO, so the job never reads "running". + let settled: Awaited | undefined; + void pending.then(value => { settled = value; }, () => { settled = null; }); + const deadline = Date.now() + 120_000; + let seen: Awaited> = null; + while (settled === undefined && Date.now() < deadline) { + seen = await probe(); + if (seen?.status === "running") break; + await Bun.sleep(10); + } + expect(settled !== undefined ? settled : seen).toMatchObject({ status: "running", ownerState: "active" }); const modulePath = new URL("../../src/codex/cli-update-job.ts", import.meta.url).href; const script = `import { readCodexCliUpdateJob } from ${JSON.stringify(modulePath)}; console.log(JSON.stringify(await readCodexCliUpdateJob(${JSON.stringify(accepted.job.jobId)})));`; - const child = spawnSync(process.execPath, ["-e", script], { encoding: "utf8", timeout: 120_000, - env: { ...process.env, OPENCODEX_HOME: configDir } }); - expect(child.error).toBeUndefined(); - expect(child.status).toBe(0); - expect(JSON.parse(child.stdout.trim())).toMatchObject({ status: "ambiguous", outcome: "ambiguous", - ownerState: "unknown", lifetimeUncertain: true, reason: "owner_state_unknown" }); + const releaseChild = holdFixture(outer); + try { + const child = spawnSync(process.execPath, ["-e", script], { encoding: "utf8", timeout: 120_000, + env: { ...process.env, OPENCODEX_HOME: configDir } }); + expect({ error: child.error ?? null, status: child.status, signal: child.signal, + stdout: child.stdout?.trim() ?? null, stderr: child.stderr?.trim() ?? null }) + .toMatchObject({ error: null, status: 0 }); + expect(JSON.parse(child.stdout.trim())).toMatchObject({ status: "ambiguous", outcome: "ambiguous", + ownerState: "unknown", lifetimeUncertain: true, reason: "owner_state_unknown" }); + } finally { releaseChild(); } } finally { release(); await pending.catch(() => undefined); } expect(await pending).toMatchObject({ status: "applied", ownerState: "finished" }); - }, 300_000); + }), 300_000); test.skipIf(!winOnly)("HTTP apply advertises retry on a leftover lock and status reads the same durable record", async () => { const { configDir } = await fixture("ocx-winjob-http-"); From 0eec93c3ba2a5790268b820f9b22b7995372c490 Mon Sep 17 00:00:00 2001 From: Codex Date: Sun, 25 Oct 2026 22:43:22 +0900 Subject: [PATCH 12/16] test(codex): create the restart-reader engine gate before job dispatch The noop release only became a real resolver when the engine callback was entered, which happens after begin() completes. If the bounded poll exits during a slow begin, finally called the noop and await pending then waited on an engine promise nobody resolves. The gate promise and its resolve are now created before runCodexCliUpdateJob and the engine returns that gate, so release() is effective at any point the finally can run. --- tests/windows/windows-codex-cli-job-store.test.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/windows/windows-codex-cli-job-store.test.ts b/tests/windows/windows-codex-cli-job-store.test.ts index 07ae96cdc54..3cd2577178b 100644 --- a/tests/windows/windows-codex-cli-job-store.test.ts +++ b/tests/windows/windows-codex-cli-job-store.test.ts @@ -371,11 +371,12 @@ describe("Windows CLI update job store backend", () => { test.skipIf(!winOnly)("a restarted reader reports an unfinished owner unknown without claiming it stopped", () => withOwnedFixture("ocx-winjob-restart-", async ({ outer, configDir }) => { const accepted = await admitCodexCliUpdateJob({ requestId: "win-restart-001", planId: result.planId, confirmed: true, configDir }); - let release: () => void = () => {}; + // The gate resolve exists before the engine callback can be entered: whenever + // finally runs, release() settles the gate even if the engine starts late or never. + let release!: () => void; + const engineGate = new Promise(resolve => { release = () => resolve(result); }); const releaseStore = holdFixture(outer); - const pending = runCodexCliUpdateJob(accepted.job.jobId, accepted.generation, () => new Promise(resolve => { - release = () => resolve(result); - }), configDir).finally(releaseStore); + const pending = runCodexCliUpdateJob(accepted.job.jobId, accepted.generation, () => engineGate, configDir).finally(releaseStore); // A status probe may see a transient unsafe_store (e.g. a killed ACL-read child // racing the running phase); the poll retries rather than asserting transport. const probe = async () => { From 0d836ec93991ef697cbe91b67316492be18e0013 Mon Sep 17 00:00:00 2001 From: Epinephrine Date: Sun, 25 Oct 2026 23:14:48 +0900 Subject: [PATCH 13/16] test(windows): wait on engine entry for the restart-reader observation Replace the 10ms ACL-read poll loop with an engineEntered gate raced against early pending settlement and the existing 120s deadline. Engine entry is the causal precondition (begin committed the running revision with live+started registered), so one read at that point is stable evidence; a failed begin still fails through its settled DTO or thrown error, never retried or normalized. The 300s test cap, the second-process reader, and the final applied/finished checks are unchanged. --- .../windows-codex-cli-job-store.test.ts | 46 ++++++++++--------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/tests/windows/windows-codex-cli-job-store.test.ts b/tests/windows/windows-codex-cli-job-store.test.ts index 3cd2577178b..8697bf20d5c 100644 --- a/tests/windows/windows-codex-cli-job-store.test.ts +++ b/tests/windows/windows-codex-cli-job-store.test.ts @@ -371,31 +371,35 @@ describe("Windows CLI update job store backend", () => { test.skipIf(!winOnly)("a restarted reader reports an unfinished owner unknown without claiming it stopped", () => withOwnedFixture("ocx-winjob-restart-", async ({ outer, configDir }) => { const accepted = await admitCodexCliUpdateJob({ requestId: "win-restart-001", planId: result.planId, confirmed: true, configDir }); - // The gate resolve exists before the engine callback can be entered: whenever - // finally runs, release() settles the gate even if the engine starts late or never. + // Both resolves exist before the engine callback can be entered: entered marks + // that begin committed the running revision, and whenever finally runs release() + // settles the engine even if the engine starts late or never. + let entered!: () => void; let release!: () => void; + const engineEntered = new Promise(resolve => { entered = resolve; }); const engineGate = new Promise(resolve => { release = () => resolve(result); }); const releaseStore = holdFixture(outer); - const pending = runCodexCliUpdateJob(accepted.job.jobId, accepted.generation, () => engineGate, configDir).finally(releaseStore); - // A status probe may see a transient unsafe_store (e.g. a killed ACL-read child - // racing the running phase); the poll retries rather than asserting transport. - const probe = async () => { - try { return await readCodexCliUpdateJob(accepted.job.jobId, configDir); } - catch { return null; } - }; + const pending = runCodexCliUpdateJob(accepted.job.jobId, accepted.generation, + () => { entered(); return engineGate; }, configDir).finally(releaseStore); try { - // Bounded wall-time poll that also watches pending's own settlement: a failed - // begin resolves early with the queued DTO, so the job never reads "running". - let settled: Awaited | undefined; - void pending.then(value => { settled = value; }, () => { settled = null; }); - const deadline = Date.now() + 120_000; - let seen: Awaited> = null; - while (settled === undefined && Date.now() < deadline) { - seen = await probe(); - if (seen?.status === "running") break; - await Bun.sleep(10); - } - expect(settled !== undefined ? settled : seen).toMatchObject({ status: "running", ownerState: "active" }); + // Wait on the causal precondition instead of polling the store for its + // projection: engine entry means begin committed "running" with live+started + // registered, so a single read at that point is stable evidence. A failed + // begin settles pending early with its read-back DTO (production swallows the + // begin error), so that DTO or error is asserted as-is; the 120s deadline + // still bounds a begin that neither settles nor commits. + let deadline: ReturnType | undefined; + const observation = await Promise.race([ + engineEntered.then(() => "entered" as const), + pending.then(() => "settled" as const, () => "settled" as const), + new Promise<"timeout">(resolve => { deadline = setTimeout(() => resolve("timeout"), 120_000); }), + ]); + if (deadline) clearTimeout(deadline); + const observed = observation === "entered" + ? await readCodexCliUpdateJob(accepted.job.jobId, configDir) + : observation === "settled" ? await pending + : null; + expect(observed).toMatchObject({ status: "running", ownerState: "active" }); const modulePath = new URL("../../src/codex/cli-update-job.ts", import.meta.url).href; const script = `import { readCodexCliUpdateJob } from ${JSON.stringify(modulePath)}; console.log(JSON.stringify(await readCodexCliUpdateJob(${JSON.stringify(accepted.job.jobId)})));`; From 6577d5508209590d63fc76f8cd2e2fbed0754f0b Mon Sep 17 00:00:00 2001 From: Epinephrine Date: Mon, 26 Oct 2026 02:01:32 +0900 Subject: [PATCH 14/16] test(windows): keep unsettled Node CLI fixtures and name timed-out runs --- .../codex-cli-windows-plan.test.ts | 27 ++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/tests/codex-integration/codex-cli-windows-plan.test.ts b/tests/codex-integration/codex-cli-windows-plan.test.ts index 74a0aa7bc8e..8e098e5ccfb 100644 --- a/tests/codex-integration/codex-cli-windows-plan.test.ts +++ b/tests/codex-integration/codex-cli-windows-plan.test.ts @@ -11,23 +11,24 @@ import { withWindowsCodexCliPlanToolchain } from "../../src/codex/cli-windows-pl import { handleCodexCliUpdateRoutes } from "../../src/server/management/codex-cli-update-routes"; import type { ManagementContext } from "../../src/server/management/context"; import { setWindowsInstallationFilesOpenedForTests, setWindowsInstallationFilesReadForTests } from "../../src/codex/windows-installation-files"; -import { removeTreeWithRetry } from "../helpers/remove-tree"; +import { removeTreeAfterBackgroundWork } from "../helpers/remove-tree"; import { repoPath, repoRoot } from "../helpers/repo-root"; import { handleCodexCliUpdateCommand } from "../../src/cli/codex-cli-update"; import { initializeNodeLauncherContext, NODE_LAUNCH_CONTEXT_ENV, NODE_LAUNCH_PROOF_PREFIX } from "../../src/cli/launcher-context"; +import { trackBackgroundWork } from "../../src/lib/background-work"; const native = process.platform === "win32" && process.arch === "x64" ? test : test.skip; const roots: string[] = []; const liveWorkerRoots = new Set(); -afterEach(() => { +afterEach(async () => { initializeNodeLauncherContext(["bun", "cli"], {}); setWindowsInstallationFilesOpenedForTests(); setWindowsInstallationFilesReadForTests(); for (const root of roots.splice(0)) { - // A test timeout does not cancel its async body. Keep even that body's - // fixture owned until a real Worker close, never remove beneath native I/O. + // A test timeout does not cancel its async body. Retain fixtures until a real + // Worker close, or external reconciliation of an unsettled CLI process tree. if (liveWorkerRoots.has(root)) { roots.push(root); continue; } - removeTreeWithRetry(root); + await removeTreeAfterBackgroundWork(root); } }); // npm's independent cmd-shim fixture; no candidate is ever executed. @@ -329,7 +330,11 @@ native("a fresh production Worker rebuilds private Windows plan evidence before // Pin this integration to current source, not a leftover generated bundle. worker = new Worker(url, { type: "module" }); liveWorkerRoots.add(f.root); - worker.addEventListener("close", () => { closed = true; liveWorkerRoots.delete(f.root); }, { once: true }); + const actualClose = Promise.withResolvers(); + void trackBackgroundWork("windows-plan-fixture-worker-close", actualClose.promise); + worker.addEventListener("close", () => { + closed = true; liveWorkerRoots.delete(f.root); actualClose.resolve(); + }, { once: true }); return worker; }, }); @@ -350,6 +355,7 @@ native("a fresh production Worker rebuilds private Windows plan evidence before }, 60_000); native("published Node CLI uses the production inspector and observed toolchain for read-only metadata", () => { + const started = performance.now(); const f = fixture(); const hostNode = Bun.which("node"); expect(hostNode).not.toBeNull(); const node = join(f.toolchainPrefix, "node.exe"); copyFileSync(hostNode!, node); const integrity = "sha512-" + Buffer.alloc(64, 1).toString("base64"); @@ -365,6 +371,15 @@ native("published Node CLI uses the production inspector and observed toolchain env: { ...process.env, CODEX_CLI_PATH: f.deps.env.CODEX_CLI_PATH, OPENCODEX_HOME: f.deps.configDir, OPENCODEX_BUN_PATH: process.execPath, OCX_NO_DESKTOP_HANDOFF: "1" }, }); + if (run.error || run.status !== 0 || run.signal) { + // A failed synchronous result does not prove Node/Bun descendants have exited. + liveWorkerRoots.add(f.root); + // Keep the preload's ancestor sandbox too; only external ownership checks can release it. + void trackBackgroundWork("windows-plan-fixture-cli-unsettled", new Promise(() => {})); + console.error(JSON.stringify({ layout: "prefix-local", + elapsedMs: Math.round(performance.now() - started), status: run.status, signal: run.signal, + errorCode: (run.error as NodeJS.ErrnoException | undefined)?.code ?? null })); + } expect({ error: run.error ?? null, status: run.status, signal: run.signal, stdout: run.stdout?.trim() ?? null, stderr: run.stderr?.trim() ?? null }) .toMatchObject({ error: null, status: 0 }); From 6a9a3f1d9a88147323fcf02d9e8558bdd457b8c1 Mon Sep 17 00:00:00 2001 From: JUN <243035832+lidge-jun@users.noreply.github.com> Date: Tue, 27 Oct 2026 01:29:15 +0900 Subject: [PATCH 15/16] fix(codex): preserve job execution ownership and clean unpublished ledgers --- src/codex/cli-update-job-store-windows.ts | 14 ++- src/codex/cli-update-job.ts | 13 +-- structure/codex-cli-update.md | 6 ++ .../windows-codex-cli-job-store.test.ts | 90 ++++++++++++++++++- 4 files changed, 115 insertions(+), 8 deletions(-) diff --git a/src/codex/cli-update-job-store-windows.ts b/src/codex/cli-update-job-store-windows.ts index 84f340346f8..986e428d9d7 100644 --- a/src/codex/cli-update-job-store-windows.ts +++ b/src/codex/cli-update-job-store-windows.ts @@ -460,6 +460,8 @@ async function writeStoreBody(ctx: Ctx, prepared: Prepared, body: string): Promi const tempName = ".jobs-" + randomUUID() + ".tmp"; const temp = join(prepared.dir, tempName); let leaf: bigint | undefined; + let tempIdentity: StoreIdentity | undefined; + let publishedName = false; try { const fd = openSync(temp, "wx", 0o600); try { writeFileSync(fd, body); fsyncSync(fd); } finally { closeSync(fd); } @@ -467,9 +469,11 @@ async function writeStoreBody(ctx: Ctx, prepared: Prepared, body: string): Promi const stat = inspect(ctx, leaf, false); if (stat.links !== 1 || stat.size !== bytes) throw unsafe(); if (!readAll(ctx, leaf, stat.size).equals(Buffer.from(body))) throw unsafe(); + tempIdentity = stat; await hardenSecretPathAsync(temp, { required: true, timeoutMemoKey: prepared.file }); await verifyAcl([{ path: temp, directory: false, policy: "private" }]); renamePinned(ctx, leaf, prepared.file); + publishedName = true; // The held handle now names jobs.json. A fresh name-open proves the published // path resolves to the identical object while the original handle is still held. let published: bigint | undefined; @@ -487,7 +491,15 @@ async function writeStoreBody(ctx: Ctx, prepared: Prepared, body: string): Promi } flushDir(ctx, prepared.jobdir); } finally { - try { unlinkSync(temp); } catch { /* already renamed or never created */ } + // A published leaf is jobs.json even if readback/flush subsequently fails. + // Only the verified unpublished object may be marked for delete-on-close. + if (!publishedName) { + try { + if (leaf !== undefined && tempIdentity !== undefined) { + if (sameIdentity(inspect(ctx, leaf, false), tempIdentity)) markDelete(ctx, leaf); + } else if (leaf === undefined) unlinkSync(temp); + } catch { /* unprovable cleanup stays fail-closed */ } + } forgetEphemeralSecretPath(temp); if (leaf !== undefined) ctx.pins.release(leaf, ctx.kernel); } diff --git a/src/codex/cli-update-job.ts b/src/codex/cli-update-job.ts index 5856fc6452a..559fc17f893 100644 --- a/src/codex/cli-update-job.ts +++ b/src/codex/cli-update-job.ts @@ -357,20 +357,23 @@ export async function finishCodexCliUpdateJob(jobId: string, generation: string, } export async function runCodexCliUpdateJob(jobId: string, generation: string, run: () => Promise, configDir?: string): Promise { - let ownsExecution = false; + // Reserve this invocation before any asynchronous store admission/publication. + // A contender must never release the admitted generation while its owner yields. + const ownsExecution = live.get(jobId) === generation && !started.has(jobId); + if (ownsExecution) started.add(jobId); return runCodexCliUpdateJobWork(run, { begin: () => withLedgerLock(configDir, false, async io => { const store = await io.readStore(); const index = store.jobs.findIndex(job => job.id === jobId); if (index < 0 || store.jobs[index]!.generation !== generation) throw new Error("job_generation_changed"); - if (live.get(jobId) !== generation || started.has(jobId)) throw new Error("job_not_owned"); + if (!ownsExecution || live.get(jobId) !== generation) throw new Error("job_not_owned"); const current = store.jobs[index]!; const next = { ...current, revision: current.revision + 1, status: "running" as const, updatedAt: now() }; - store.jobs[index] = next; await io.writeStore(store); started.add(jobId); ownsExecution = true; return next.revision; + store.jobs[index] = next; await io.writeStore(store); return next.revision; }), finish: (revision, result) => finishCodexCliUpdateJob(jobId, generation, revision, result, configDir), read: () => readCodexCliUpdateJob(jobId, configDir), release: () => { - if (live.get(jobId) === generation && (ownsExecution || !started.has(jobId))) { - live.delete(jobId); if (ownsExecution) started.delete(jobId); + if (ownsExecution && live.get(jobId) === generation) { + live.delete(jobId); started.delete(jobId); } }, }); diff --git a/structure/codex-cli-update.md b/structure/codex-cli-update.md index 56df4b6c4cc..78acbd6bd62 100644 --- a/structure/codex-cli-update.md +++ b/structure/codex-cli-update.md @@ -233,6 +233,12 @@ callers use that already-owned handle without wrapping it in another `keep` regi This applies to config/store pins, read/write/published leaves and lock/owner pins, so disposal does not revisit a stale duplicate registration after the original handle has closed. +An execution invocation reserves its admitted generation before asynchronous admission or +running-record publication; only that invocation releases the reservation after a failed begin. +Competing deliveries cannot clear the live owner's completion authority. An unpublished verified +ledger temp is deleted through its held handle before close. Successful rename disables that +cleanup permanently for the invocation, including failures during published-name readback or flush. + An ACL-query timeout fails closed without accepting a late result. Its PowerShell child may still hold transient handles while termination settles, so lock cleanup may fail and retain a busy mutex; the store does not automatically reclaim that leftover lock. Filtered-token behavior is also unverified. diff --git a/tests/windows/windows-codex-cli-job-store.test.ts b/tests/windows/windows-codex-cli-job-store.test.ts index 8697bf20d5c..6774bc40fce 100644 --- a/tests/windows/windows-codex-cli-job-store.test.ts +++ b/tests/windows/windows-codex-cli-job-store.test.ts @@ -11,13 +11,13 @@ */ import { afterEach, describe, expect, spyOn, test } from "bun:test"; import { spawnSync } from "node:child_process"; -import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, rmSync, writeFileSync } from "node:fs"; import * as fs from "node:fs"; import { join } from "node:path"; import { admitCodexCliUpdateJob, finishCodexCliUpdateJob, readCodexCliUpdateJob, runCodexCliUpdateJob } from "../../src/codex/cli-update-job"; import { codexCliUpdateUnjoinedWorkerCount, runCodexCliUpdateApplyInWorker } from "../../src/codex/cli-update-job-worker"; import { FIXTURE_CONTROL_WORDS, FIXTURE_RELEASE } from "../helpers/cli-update-fixture-worker"; -import { setWindowsOwnerAclAsyncRunnerForTests, windowsEntriesAclMatches, +import { setWindowsOwnerAclAsyncRunnerForTests, windowsEntriesAclMatches, windowsOwnerAclDefaultRunnerForTests, type WindowsAclEntry } from "../../src/lib/windows-owner-acl"; import { hardenSecretDirAsync } from "../../src/lib/windows-secret-acl"; import { handleCodexCliUpdateRoutes } from "../../src/server/management/codex-cli-update-routes"; @@ -299,6 +299,92 @@ describe("Windows CLI update job store backend", () => { expect(JSON.stringify(terminal)).not.toMatch(/ownerPid|generation|payloadHash|requestKey|token|path/i); }, 180_000); + test.skipIf(!winOnly)("a competing begin preserves ownership while running publication is pending", () => withOwnedFixture("ocx-winjob-begin-", async ({ configDir }) => { + const accepted = await admitCodexCliUpdateJob({ requestId: "win-begin-race-01", planId: result.planId, confirmed: true, configDir }); + const entered = Promise.withResolvers(), release = Promise.withResolvers(); + let gated = false, runs = 0; + // Real native handles and hardened objects; synthetic ACL replies hold the + // first publication before engine entry while the contender exhausts retries. + setWindowsOwnerAclAsyncRunnerForTests(async entries => { + if (!gated && entries.length === 1 && entries[0]!.path.endsWith(".tmp")) { + gated = true; entered.resolve(); await release.promise; + } + return { success: true, timedOut: false, stdout: aclRun(USER_SID, USER_SID, false, + entries.map(entry => ({ owner: USER_SID, flags: 4100, + aces: [allow(FULL_CONTROL, USER_SID, entry.directory ? 3 : 0)] }))) }; + }); + const engine = async () => { runs++; return result; }; + const first = runCodexCliUpdateJob(accepted.job.jobId, accepted.generation, engine, configDir); + try { + await Promise.race([entered.promise, first.then(() => { throw new Error("begin settled before barrier"); })]); + expect(runs).toBe(0); + await runCodexCliUpdateJob(accepted.job.jobId, accepted.generation, engine, configDir); + expect(runs).toBe(0); + release.resolve(); + await first; + setWindowsOwnerAclAsyncRunnerForTests(null); + expect(runs).toBe(1); + expect(await readCodexCliUpdateJob(accepted.job.jobId, configDir)).toMatchObject( + { status: "applied", reason: "verified_target", ownerState: "finished" }); + const stored = JSON.parse(readFileSync(join(configDir, "codex-cli-update-jobs", "jobs.json"), "utf8")); + expect(stored.jobs[0]).toMatchObject({ status: "applied", revision: 2 }); + } finally { + release.resolve(); await first.catch(() => {}); + setWindowsOwnerAclAsyncRunnerForTests(null); + } + }), 180_000); + + test.skipIf(!winOnly)("a pre-rename ACL failure removes the unpublished temp and preserves jobs.json", async () => { + const { configDir } = await fixture("ocx-winjob-temp-"); + const input = { requestId: "win-temp-clean-01", planId: result.planId, confirmed: true as const, configDir }; + const accepted = await admitCodexCliUpdateJob(input); + const dir = join(configDir, "codex-cli-update-jobs"), file = join(dir, "jobs.json"); + const before = readFileSync(file, "utf8"); + let injected = 0; + setWindowsOwnerAclAsyncRunnerForTests(async (entries, timeout) => { + if (entries.some(entry => entry.path.endsWith(".tmp"))) { + injected++; + return { success: false, timedOut: false, stdout: "" }; + } + return windowsOwnerAclDefaultRunnerForTests(entries, timeout); + }); + try { + for (const requestId of ["win-temp-clean-02", "win-temp-clean-03"]) { + await expect(admitCodexCliUpdateJob({ ...input, requestId })).rejects.toThrow("unsafe_store"); + expect(readFileSync(file, "utf8")).toBe(before); + expect(readdirSync(dir).filter(name => /^\.jobs-.*\.tmp$/.test(name))).toEqual([]); + expect(existsSync(join(dir, "admission.lock"))).toBe(false); + } + expect(injected).toBe(2); + } finally { setWindowsOwnerAclAsyncRunnerForTests(null); } + expect(await readCodexCliUpdateJob(accepted.job.jobId, configDir)).toMatchObject({ status: "queued" }); + }, 180_000); + + test.skipIf(!winOnly)("a post-rename verification failure never deletes the published jobs.json", async () => { + const { configDir } = await fixture("ocx-winjob-published-"); + const input = { requestId: "win-published-01", planId: result.planId, confirmed: true as const, configDir }; + const accepted = await admitCodexCliUpdateJob(input); + let verifiedTemp = false, injected = 0; + setWindowsOwnerAclAsyncRunnerForTests(async (entries, timeout) => { + if (entries.some(entry => entry.path.endsWith(".tmp"))) verifiedTemp = true; + if (verifiedTemp && entries.some(entry => entry.path.endsWith("jobs.json"))) { + injected++; + return { success: false, timedOut: false, stdout: "" }; + } + return windowsOwnerAclDefaultRunnerForTests(entries, timeout); + }); + try { + await expect(admitCodexCliUpdateJob({ ...input, requestId: "win-published-02" })).rejects.toThrow("unsafe_store"); + expect(injected).toBe(1); + } finally { setWindowsOwnerAclAsyncRunnerForTests(null); } + const dir = join(configDir, "codex-cli-update-jobs"); + const stored = JSON.parse(readFileSync(join(dir, "jobs.json"), "utf8")); + expect(stored.jobs).toHaveLength(2); + expect(stored.jobs[0].id).toBe(accepted.job.jobId); + expect(readdirSync(dir).sort()).toEqual(["jobs.json"]); + expect(await readCodexCliUpdateJob(accepted.job.jobId, configDir)).toMatchObject({ status: "queued" }); + }, 180_000); + test.skipIf(!winOnly)("a second process dedupes the admitted request key and conflicts on a different payload", async () => { const { configDir } = await fixture("ocx-winjob-twoproc-"); mkdirSync(configDir); From ac7045750fdf8bd6eff97b87407ae141b33ad3eb Mon Sep 17 00:00:00 2001 From: JUN <243035832+lidge-jun@users.noreply.github.com> Date: Tue, 27 Oct 2026 01:40:29 +0900 Subject: [PATCH 16/16] fix(codex): share held canonical path checks with the job store --- src/codex/cli-update-job-store-windows.ts | 11 +++++++++++ src/codex/windows-installation-files.ts | 8 ++++++-- structure/codex-cli-update.md | 3 +++ tests/windows/windows-codex-cli-job-store.test.ts | 7 +++++++ 4 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/codex/cli-update-job-store-windows.ts b/src/codex/cli-update-job-store-windows.ts index 986e428d9d7..9b48b8b3417 100644 --- a/src/codex/cli-update-job-store-windows.ts +++ b/src/codex/cli-update-job-store-windows.ts @@ -67,6 +67,7 @@ import { CODEX_CLI_JOB_STORE_DIR, CODEX_CLI_JOB_STORE_FILE, CODEX_CLI_JOB_STORE_LOCK, CODEX_CLI_JOB_STORE_MAX_BYTES, CodexCliJobStoreBusyError, } from "./cli-update-job-execution"; +import { windowsCanonicalPathMatches } from "./windows-installation-files"; const KERNEL_SYMBOLS = { GetDriveTypeW: { args: ["ptr"], returns: "u32" }, @@ -75,6 +76,7 @@ const KERNEL_SYMBOLS = { GetFileInformationByHandleEx: { args: ["u64", "i32", "ptr", "u32"], returns: "i32" }, SetFileInformationByHandle: { args: ["u64", "i32", "ptr", "u32"], returns: "i32" }, GetFileType: { args: ["u64"], returns: "u32" }, + GetFinalPathNameByHandleW: { args: ["u64", "ptr", "u32", "u32"], returns: "u32" }, ReadFile: { args: ["u64", "ptr", "u32", "ptr", "ptr"], returns: "i32" }, SetFilePointerEx: { args: ["u64", "i64", "ptr", "u32"], returns: "i32" }, FlushFileBuffers: { args: ["u64"], returns: "i32" }, @@ -213,6 +215,13 @@ function sameIdentity(a: StoreIdentity, b: StoreIdentity): boolean { return a.volumeSerial === b.volumeSerial && a.fileId === b.fileId; } +function assertCanonicalPath(ctx: Ctx, handle: bigint, path: string): void { + const output = Buffer.alloc(32768 * 2); + const length = ctx.kernel.symbols.GetFinalPathNameByHandleW!(handle, ctx.ffi.ptr(output), 32768, 0); + if (!length || length >= 32768 + || !windowsCanonicalPathMatches(path, output.toString("utf16le", 0, length * 2))) throw unsafe(); +} + /** Handle-relative open of one component below an already-pinned parent. */ function relOpen( ctx: Ctx, @@ -354,6 +363,7 @@ function pinChain(ctx: Ctx, absolutePath: string, leafAccess: number): ChainEntr const handle = relOpen(ctx, parent, parsed.names[index]!, true, access, SHARE_DIR); const inspection = inspect(ctx, handle, true); current = join(current, parsed.names[index]!); + assertCanonicalPath(ctx, handle, current); chain.push({ path: current, handle, inspection }); parent = handle; } @@ -405,6 +415,7 @@ async function prepareStore( await hardenSecretDirAsync(config, { required: true }); configPrivate = true; } + assertCanonicalPath(ctx, configPin, config); await verifyAcl([{ path: config, directory: true, policy: configPrivate ? "private" : "write-private" }]); const jobdirAccess = opts.createJobDir ? ACCESS_DIR_PIN : ACCESS_READ; let jobdir: bigint; diff --git a/src/codex/windows-installation-files.ts b/src/codex/windows-installation-files.ts index ce5806f66cc..a4aba288d3b 100644 --- a/src/codex/windows-installation-files.ts +++ b/src/codex/windows-installation-files.ts @@ -66,6 +66,11 @@ const MAX_REQUEST_BYTES = MAX_NATIVE_BACKING_BYTES + (128 + 12) * MIB; const REPARSE = 0x400; const DIRECTORY = 0x10; const INVALID_HANDLE = 0xffffffffffffffffn; +/** Compare the normalized DOS long name returned by an already-held handle. */ +export function windowsCanonicalPathMatches(path: string, canonical: string): boolean { + return canonical.startsWith("\\\\?\\") + && canonical.slice(4).toLowerCase() === win32.normalize(path).toLowerCase(); +} let openedForTests: (() => void) | undefined; /** Runs after handles are held, for real Windows sharing/rename regression fixtures. */ export function setWindowsInstallationFilesOpenedForTests(callback?: () => void): void { @@ -211,8 +216,7 @@ export async function inspectWindowsInstallationFiles( const length = k.GetFinalPathNameByHandleW!(handle, ffi.ptr(output), 32768, 0); if (!length || length >= 32768) throw new InspectionRefusal("inspection-failed"); const canonical = output.toString("utf16le", 0, length * 2); - if (!canonical.startsWith("\\\\?\\") - || canonical.slice(4).toLowerCase() !== win32.normalize(path).toLowerCase()) { + if (!windowsCanonicalPathMatches(path, canonical)) { throw new InspectionRefusal("invalid-request"); } }; diff --git a/structure/codex-cli-update.md b/structure/codex-cli-update.md index 3da6a219737..13d07b6017c 100644 --- a/structure/codex-cli-update.md +++ b/structure/codex-cli-update.md @@ -238,6 +238,9 @@ running-record publication; only that invocation releases the reservation after Competing deliveries cannot clear the live owner's completion authority. An unpublished verified ledger temp is deleted through its held handle before close. Successful rename disables that cleanup permanently for the invocation, including failures during published-name readback or flush. +The store checks the held ancestors and config root against the normalized DOS long name with +the same comparison used by `src/codex/windows-installation-files.ts`; 8.3 aliases refuse before +ledger access, and a missing config root is created only beneath an already canonical held parent. An ACL-query timeout fails closed without accepting a late result. Its PowerShell child may still hold transient handles while termination settles, so lock cleanup may fail and retain a busy mutex; diff --git a/tests/windows/windows-codex-cli-job-store.test.ts b/tests/windows/windows-codex-cli-job-store.test.ts index 6774bc40fce..16d21e26f3d 100644 --- a/tests/windows/windows-codex-cli-job-store.test.ts +++ b/tests/windows/windows-codex-cli-job-store.test.ts @@ -20,6 +20,7 @@ import { FIXTURE_CONTROL_WORDS, FIXTURE_RELEASE } from "../helpers/cli-update-fi import { setWindowsOwnerAclAsyncRunnerForTests, windowsEntriesAclMatches, windowsOwnerAclDefaultRunnerForTests, type WindowsAclEntry } from "../../src/lib/windows-owner-acl"; import { hardenSecretDirAsync } from "../../src/lib/windows-secret-acl"; +import { windowsCanonicalPathMatches } from "../../src/codex/windows-installation-files"; import { handleCodexCliUpdateRoutes } from "../../src/server/management/codex-cli-update-routes"; import { handleCodexCliUpdateCommand } from "../../src/cli/codex-cli-update"; import type { ManagementContext } from "../../src/server/management/context"; @@ -55,6 +56,12 @@ describe("windows owner ACL mixed policies", () => { const userRun = (entries: readonly { owner: string; flags?: number; aces: readonly string[] }[], user = USER_SID, tokenOwner = USER_SID, elevated = false) => aclRun(user, tokenOwner, elevated, entries); + test("held DOS long-name comparison refuses short aliases and accepts canonical casing", () => { + expect(windowsCanonicalPathMatches("C:\\fixture\\LONGPR~1\\ocx", "\\\\?\\C:\\fixture\\LongProfileName\\ocx")).toBe(false); + expect(windowsCanonicalPathMatches("c:/fixture/LongProfileName/ocx", "\\\\?\\C:\\FIXTURE\\LongProfileName\\ocx")).toBe(true); + expect(windowsCanonicalPathMatches("C:\\fixture\\LongProfileName\\ocx", "C:\\fixture\\LongProfileName\\ocx")).toBe(false); + }); + test("a foreign read-only grantee passes write-private but an untrusted writer refuses", () => { const entries: WindowsAclEntry[] = [{ path: "C:\\cfg", directory: true, policy: "write-private" }]; expect(windowsEntriesAclMatches(userRun([{ owner: USER_SID, aces: [allow(FULL_CONTROL, USER_SID), allow(READ_ONLY_MASK, FOREIGN_SID)] }]), entries)).toBe(true);