diff --git a/sdk/typescript/src/runtime.ts b/sdk/typescript/src/runtime.ts index 15063adf..2ce27a75 100644 --- a/sdk/typescript/src/runtime.ts +++ b/sdk/typescript/src/runtime.ts @@ -24,6 +24,7 @@ import { rm, rmdir, stat, + utimes, writeFile, } from "node:fs/promises"; import { homedir, tmpdir } from "node:os"; @@ -74,6 +75,7 @@ const MAX_ZIP_EXPANDED_SIZE = 512 * 1024 * 1024; const MODEL_UNSAFE_PATH = /[\u0000-\u001f\u007f-\u009f\u2028\u2029]/u; const CREDENTIAL_LOCK_NAME = ".codex-security-scan.lock"; const CREDENTIAL_LOGOUT_MARKER = ".codex-security-logged-out"; +const CREDENTIAL_LOCK_HEARTBEAT_MILLISECONDS = 5_000; const CREDENTIAL_LOCK_POLL_MILLISECONDS = 25; const INCOMPLETE_CREDENTIAL_LOCK_MILLISECONDS = 30_000; const MAX_PROCESS_ID = 2_147_483_647; @@ -1065,7 +1067,7 @@ export async function acquireCodexSecurityCredentialHomeLock( throw error; }); if (existingLock !== null) { - if (await recoverStaleCredentialHomeLock(lock)) continue; + if (await recoverStaleCredentialHomeLock(lock, signal)) continue; await delay(CREDENTIAL_LOCK_POLL_MILLISECONDS, undefined, { signal }); continue; } @@ -1078,7 +1080,7 @@ export async function acquireCodexSecurityCredentialHomeLock( await mkdir(lock, { mode: 0o700 }); } catch (error) { if (nodeErrorCode(error) !== "EEXIST") throw error; - if (await recoverStaleCredentialHomeLock(lock)) continue; + if (await recoverStaleCredentialHomeLock(lock, signal)) continue; await delay(CREDENTIAL_LOCK_POLL_MILLISECONDS, undefined, { signal }); continue; } @@ -1094,9 +1096,18 @@ export async function acquireCodexSecurityCredentialHomeLock( throw error; } + const heartbeat = setInterval(async () => { + try { + const now = new Date(); + await utimes(lock, now, now); + } catch {} + }, CREDENTIAL_LOCK_HEARTBEAT_MILLISECONDS); + heartbeat.unref(); + let released = false; return async () => { if (released) return; + clearInterval(heartbeat); await requireSecureCredentialHome(codexHome, { ...securityOptions, expectedDevice, @@ -1116,7 +1127,10 @@ export async function acquireCodexSecurityCredentialHomeLock( } } -async function recoverStaleCredentialHomeLock(lock: string): Promise { +async function recoverStaleCredentialHomeLock( + lock: string, + signal?: AbortSignal, +): Promise { const metadata = await lstat(lock).catch((error: unknown) => { if (nodeErrorCode(error) === "ENOENT") return null; throw error; @@ -1135,17 +1149,13 @@ async function recoverStaleCredentialHomeLock(lock: string): Promise { if (nodeErrorCode(error) !== "ENOENT" && !(error instanceof SyntaxError)) { throw error; } - if ( - Date.now() - metadata.mtimeMs < - INCOMPLETE_CREDENTIAL_LOCK_MILLISECONDS - ) { - return false; - } } // Only positive signed-32-bit PIDs identify an owner. Other values can name // process groups or fail argument validation, so use the stale-age check. const ownerPid = isRecord(owner) ? owner["pid"] : undefined; + let ownerExited = false; + let ownerIsAlive = false; if ( typeof ownerPid === "number" && Number.isInteger(ownerPid) && @@ -1154,20 +1164,38 @@ async function recoverStaleCredentialHomeLock(lock: string): Promise { ) { try { process.kill(ownerPid, 0); - return false; + ownerIsAlive = true; } catch (error) { - if (nodeErrorCode(error) !== "ESRCH") { - if (nodeErrorCode(error) === "EPERM") return false; - throw error; - } + if (nodeErrorCode(error) === "ESRCH") ownerExited = true; + else if (nodeErrorCode(error) === "EPERM") ownerIsAlive = true; + else throw error; } - } else if ( - Date.now() - metadata.mtimeMs < - INCOMPLETE_CREDENTIAL_LOCK_MILLISECONDS + } + if ( + !ownerExited && + Date.now() - metadata.mtimeMs < INCOMPLETE_CREDENTIAL_LOCK_MILLISECONDS ) { return false; } + if (ownerIsAlive) { + await delay(CREDENTIAL_LOCK_HEARTBEAT_MILLISECONDS, undefined, { signal }); + const refreshedMetadata = await lstat(lock).catch((error: unknown) => { + if (nodeErrorCode(error) === "ENOENT") return null; + throw error; + }); + if (refreshedMetadata === null) return true; + if ( + !refreshedMetadata.isDirectory() || + refreshedMetadata.isSymbolicLink() + ) { + throw new OutputDirectoryError( + `Codex Security credential-home lock is not a directory: ${lock}`, + ); + } + if (refreshedMetadata.mtimeMs > metadata.mtimeMs) return false; + } + const quarantine = `${lock}.stale-${randomUUID()}`; try { await rename(lock, quarantine); diff --git a/sdk/typescript/tests-ts/runtime.test.ts b/sdk/typescript/tests-ts/runtime.test.ts index 7dbf80c8..179db5b5 100644 --- a/sdk/typescript/tests-ts/runtime.test.ts +++ b/sdk/typescript/tests-ts/runtime.test.ts @@ -29,6 +29,7 @@ import { sep, } from "node:path"; import { promisify } from "node:util"; +import * as timersPromises from "node:timers/promises"; import { brotliDecompressSync } from "node:zlib"; import { afterEach, describe, expect, mock, spyOn, test } from "bun:test"; import { strToU8, zipSync } from "fflate"; @@ -97,6 +98,104 @@ async function temporaryDirectory( return path; } +async function plantCredentialHomeLock( + home: string, + pid: number, + modifiedAt?: Date, +): Promise { + const lock = join(home, ".codex-security-scan.lock"); + await mkdir(lock, { mode: 0o700 }); + await writeFile( + join(lock, "owner.json"), + `${JSON.stringify({ pid, token: "planted-owner" })}\n`, + { mode: 0o600 }, + ); + if (modifiedAt !== undefined) await utimes(lock, modifiedAt, modifiedAt); + return lock; +} + +async function acquireCredentialHomeLockWithTimeout( + home: string, +): Promise<() => Promise> { + const controller = new AbortController(); + const timeout = setTimeout( + () => controller.abort(new DOMException("timed out", "AbortError")), + 10_000, + ); + timeout.unref(); + try { + return await acquireCodexSecurityCredentialHomeLock( + home, + controller.signal, + ); + } finally { + clearTimeout(timeout); + } +} + +function abortCredentialLockWaitWhenOwnerIsInspected( + controller: AbortController, +): ReturnType { + const killProcess = process.kill.bind(process); + return spyOn(process, "kill").mockImplementation((( + pid: number, + signal: number, + ) => { + const result = killProcess(pid, signal); + controller.abort(new DOMException("canceled", "AbortError")); + return result; + }) as typeof process.kill); +} + +function captureCredentialLockHeartbeatTimer() { + const realSetInterval = globalThis.setInterval.bind(globalThis); + const realClearInterval = globalThis.clearInterval.bind(globalThis); + const unref = mock(() => undefined); + const interval = { unref } as unknown as NodeJS.Timeout; + let callback: (() => Promise) | undefined; + let milliseconds: number | undefined; + let cleared = false; + const setIntervalSpy = spyOn(globalThis, "setInterval").mockImplementation((( + candidate: (...args: unknown[]) => void, + delay?: number, + ...args: unknown[] + ) => { + if (delay === 5_000 && callback === undefined) { + callback = candidate as () => Promise; + milliseconds = delay; + return interval; + } + return realSetInterval(candidate, delay, ...args); + }) as typeof setInterval); + const clearIntervalSpy = spyOn( + globalThis, + "clearInterval", + ).mockImplementation(((timer: NodeJS.Timeout) => { + if (timer === interval) { + cleared = true; + return; + } + realClearInterval(timer); + }) as typeof clearInterval); + + return { + get callback() { + return callback; + }, + get cleared() { + return cleared; + }, + get milliseconds() { + return milliseconds; + }, + restore() { + clearIntervalSpy.mockRestore(); + setIntervalSpy.mockRestore(); + }, + unref, + }; +} + async function plugin(root: string, version = "1.2.3"): Promise { const path = join(root, "plugin"); await mkdir(join(path, ".codex-plugin"), { recursive: true }); @@ -2281,24 +2380,70 @@ describe("runtime directories and plugin Python boundary", () => { expect(existsSync(join(home, ".codex-security-scan.lock"))).toBe(false); }); - test("cancels a scan waiting for the persistent credential-home lock", async () => { + test("keeps a fresh live credential-home lock and cancels the waiter", async () => { const root = await temporaryDirectory(); const home = await prepareCodexSecurityCredentialHome({ CODEX_SECURITY_STATE_DIR: join(root, "state"), }); const release = await acquireCodexSecurityCredentialHomeLock(home); const controller = new AbortController(); - const waiting = acquireCodexSecurityCredentialHomeLock( - home, - controller.signal, - ); - controller.abort(new DOMException("canceled", "AbortError")); + const inspectOwner = + abortCredentialLockWaitWhenOwnerIsInspected(controller); try { + const waiting = acquireCodexSecurityCredentialHomeLock( + home, + controller.signal, + ); await expect(waiting).rejects.toMatchObject({ name: "AbortError" }); + expect(inspectOwner).toHaveBeenCalledWith(process.pid, 0); + expect(existsSync(join(home, ".codex-security-scan.lock"))).toBe(true); } finally { + inspectOwner.mockRestore(); await release(); } + expect(existsSync(join(home, ".codex-security-scan.lock"))).toBe(false); + }); + + test("heartbeats a held credential-home lock across the stale horizon", async () => { + const root = await temporaryDirectory(); + const home = await prepareCodexSecurityCredentialHome({ + CODEX_SECURITY_STATE_DIR: join(root, "state"), + }); + const lock = join(home, ".codex-security-scan.lock"); + const heartbeatTimer = captureCredentialLockHeartbeatTimer(); + let release: (() => Promise) | undefined; + let inspectOwner: ReturnType | undefined; + + try { + release = await acquireCodexSecurityCredentialHomeLock(home); + expect(heartbeatTimer.milliseconds).toBe(5_000); + expect(heartbeatTimer.unref).toHaveBeenCalledTimes(1); + expect(heartbeatTimer.callback).toBeDefined(); + + const stale = new Date(Date.now() - 60_000); + await utimes(lock, stale, stale); + await heartbeatTimer.callback!(); + expect(Date.now() - (await stat(lock)).mtimeMs).toBeLessThan(30_000); + + const controller = new AbortController(); + inspectOwner = abortCredentialLockWaitWhenOwnerIsInspected(controller); + const waiting = acquireCodexSecurityCredentialHomeLock( + home, + controller.signal, + ); + await expect(waiting).rejects.toMatchObject({ name: "AbortError" }); + expect(existsSync(lock)).toBe(true); + } finally { + inspectOwner?.mockRestore(); + try { + if (release !== undefined) await release(); + } finally { + heartbeatTimer.restore(); + } + } + expect(heartbeatTimer.cleared).toBe(true); + expect(existsSync(lock)).toBe(false); }); test("does not rewrite Windows credential ACLs while polling a held lock", async () => { @@ -2363,6 +2508,118 @@ describe("runtime directories and plugin Python boundary", () => { expect(existsSync(lock)).toBe(false); }); + test("recovers a stale credential-home lock naming a live process", async () => { + const root = await temporaryDirectory(); + const home = await prepareCodexSecurityCredentialHome({ + CODEX_SECURITY_STATE_DIR: join(root, "state"), + }); + const stale = new Date(Date.now() - 10 * 60_000); + const lock = await plantCredentialHomeLock(home, process.pid, stale); + let release: (() => Promise) | undefined; + + try { + release = await acquireCredentialHomeLockWithTimeout(home); + expect(existsSync(lock)).toBe(true); + } finally { + if (release !== undefined) await release(); + } + expect(existsSync(lock)).toBe(false); + }); + + test("recovers a stale credential-home lock when PID inspection is denied", async () => { + const root = await temporaryDirectory(); + const home = await prepareCodexSecurityCredentialHome({ + CODEX_SECURITY_STATE_DIR: join(root, "state"), + }); + const stale = new Date(Date.now() - 10 * 60_000); + const lock = await plantCredentialHomeLock(home, process.pid, stale); + const inspectOwner = spyOn(process, "kill").mockImplementation((() => { + const error = new Error( + "operation not permitted", + ) as NodeJS.ErrnoException; + error.code = "EPERM"; + throw error; + }) as typeof process.kill); + let release: (() => Promise) | undefined; + + try { + release = await acquireCredentialHomeLockWithTimeout(home); + expect(existsSync(lock)).toBe(true); + } finally { + inspectOwner.mockRestore(); + if (release !== undefined) await release(); + } + expect(existsSync(lock)).toBe(false); + }); + + test("abandons stale recovery when the owner heartbeat advances", async () => { + if ( + runTestInSubprocess( + import.meta.path, + "abandons stale recovery when the owner heartbeat advances", + ) + ) { + return; + } + const root = await temporaryDirectory(); + const home = await prepareCodexSecurityCredentialHome({ + CODEX_SECURITY_STATE_DIR: join(root, "state"), + }); + const lock = join(home, ".codex-security-scan.lock"); + const controller = new AbortController(); + const heartbeatTimer = captureCredentialLockHeartbeatTimer(); + const realDelay = timersPromises.setTimeout; + let waitedForHeartbeat = false; + let observedDelaySignal: AbortSignal | undefined; + mock.module("node:timers/promises", () => ({ + ...timersPromises, + setTimeout: (async ( + milliseconds: number, + value?: unknown, + options?: { signal?: AbortSignal }, + ) => { + observedDelaySignal = options?.signal; + if (milliseconds === 5_000) { + waitedForHeartbeat = true; + await heartbeatTimer.callback!(); + return value; + } + controller.abort(new DOMException("canceled", "AbortError")); + return await realDelay(0, value, { signal: controller.signal }); + }) as typeof timersPromises.setTimeout, + })); + let release: (() => Promise) | undefined; + + try { + release = await acquireCodexSecurityCredentialHomeLock(home); + expect(heartbeatTimer.milliseconds).toBe(5_000); + expect(heartbeatTimer.unref).toHaveBeenCalledTimes(1); + expect(heartbeatTimer.callback).toBeDefined(); + const stale = new Date(Date.now() - 10 * 60_000); + await utimes(lock, stale, stale); + + const waiting = acquireCodexSecurityCredentialHomeLock( + home, + controller.signal, + ); + await expect(waiting).rejects.toMatchObject({ name: "AbortError" }); + expect(waitedForHeartbeat).toBe(true); + expect(observedDelaySignal).toBe(controller.signal); + expect(Date.now() - (await stat(lock)).mtimeMs).toBeLessThan(30_000); + expect(existsSync(lock)).toBe(true); + } finally { + mock.module("node:timers/promises", () => ({ + ...timersPromises, + setTimeout: realDelay, + })); + try { + if (release !== undefined) await release(); + } finally { + heartbeatTimer.restore(); + } + } + }); + test("recovers credential-home locks whose owner names no process", async () => { const root = await temporaryDirectory(); const home = await prepareCodexSecurityCredentialHome({ @@ -2379,19 +2636,9 @@ describe("runtime directories and plugin Python boundary", () => { const aged = new Date(Date.now() - 10 * 60_000); await utimes(lock, aged, aged); - // Bound acquisition so a false live-owner result cannot hang the test. - const abort = new AbortController(); - const timer = setTimeout(() => abort.abort(), 5_000); - try { - const release = await acquireCodexSecurityCredentialHomeLock( - home, - abort.signal, - ); - expect(existsSync(lock)).toBe(true); - await release(); - } finally { - clearTimeout(timer); - } + const release = await acquireCredentialHomeLockWithTimeout(home); + expect(existsSync(lock)).toBe(true); + await release(); expect(existsSync(lock)).toBe(false); } });