diff --git a/sdk/typescript/src/runtime.ts b/sdk/typescript/src/runtime.ts index 4a09a894..66f6baff 100644 --- a/sdk/typescript/src/runtime.ts +++ b/sdk/typescript/src/runtime.ts @@ -64,6 +64,9 @@ const CREDENTIAL_LOCK_NAME = ".codex-security-scan.lock"; const CREDENTIAL_LOGOUT_MARKER = ".codex-security-logged-out"; const CREDENTIAL_LOCK_POLL_MILLISECONDS = 25; const INCOMPLETE_CREDENTIAL_LOCK_MILLISECONDS = 30_000; +// `process.kill` narrows its pid to a 32-bit signed integer and rejects anything that +// does not survive the round trip, so a larger value can never name a process. +const MAX_PROCESS_ID = 2_147_483_647; export interface PluginInstall { pluginRoot: string; @@ -418,9 +421,21 @@ async function recoverStaleCredentialHomeLock(lock: string): Promise { } } - if (isRecord(owner) && typeof owner["pid"] === "number") { + // Only a positive integer within the pid range names a process. `process.kill` reads 0 + // as the caller's own process group and -1 as every process it may signal, so both + // always report a live owner and would hold the lock open forever, and a fractional or + // out-of-range value makes it throw an argument error that is neither ESRCH nor EPERM + // and escapes raw. An owner that cannot be identified is treated like a missing one, so + // the age check below still reclaims the lock. + const ownerPid = isRecord(owner) ? owner["pid"] : undefined; + if ( + typeof ownerPid === "number" && + Number.isInteger(ownerPid) && + ownerPid > 0 && + ownerPid <= MAX_PROCESS_ID + ) { try { - process.kill(owner["pid"], 0); + process.kill(ownerPid, 0); return false; } catch (error) { if (nodeErrorCode(error) !== "ESRCH") { diff --git a/sdk/typescript/tests-ts/runtime.test.ts b/sdk/typescript/tests-ts/runtime.test.ts index 7cb55da3..1624dca2 100644 --- a/sdk/typescript/tests-ts/runtime.test.ts +++ b/sdk/typescript/tests-ts/runtime.test.ts @@ -15,6 +15,7 @@ import { stat, symlink, truncate, + utimes, writeFile, } from "node:fs/promises"; import * as fsPromises from "node:fs/promises"; @@ -1632,6 +1633,45 @@ describe("runtime directories and plugin Python boundary", () => { expect(existsSync(lock)).toBe(false); }); + test("recovers credential-home locks whose owner names no process", async () => { + const root = await temporaryDirectory(); + const home = await prepareCodexSecurityCredentialHome({ + CODEX_SECURITY_STATE_DIR: join(root, "state"), + }); + const lock = join(home, ".codex-security-scan.lock"); + // `process.kill` reads 0 as the caller's own process group and -1 as every process + // it may signal, so both report a live owner forever, and a fractional pid, a pid + // just past the signed 32-bit range, or one past the safe-integer range makes it + // throw an argument error instead. None of them identifies a process holding this + // lock, so an aged lock naming one has to be reclaimed like any other stale lock. + for (const pid of [0, -1, 0.5, 2 ** 31, 2 ** 53]) { + await mkdir(lock, { mode: 0o700 }); + await writeFile( + join(lock, "owner.json"), + `${JSON.stringify({ pid, token: "unidentifiable-owner" })}\n`, + { mode: 0o600 }, + ); + const aged = new Date(Date.now() - 10 * 60_000); + await utimes(lock, aged, aged); + + // An owner that is treated as live is waited on forever, so the acquisition is + // bounded here to fail the test rather than hang it. + 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); + } + expect(existsSync(lock)).toBe(false); + } + }); + test("prevents ambient credential imports after an explicit logout", async () => { const root = await temporaryDirectory(); const home = await prepareCodexSecurityCredentialHome({