diff --git a/src/codex/cli-install-provenance.ts b/src/codex/cli-install-provenance.ts index 3841710957a..9e44ae48677 100644 --- a/src/codex/cli-install-provenance.ts +++ b/src/codex/cli-install-provenance.ts @@ -9,8 +9,9 @@ import { readSync, realpathSync, statSync, - type Stats, + type BigIntStats, } from "node:fs"; +import { sameFileIdentity, statTimeMs } from "../lib/file-identity"; import { posix, win32 } from "node:path"; import { getConfigDir } from "../config"; import { parseStrictSemver } from "../lib/strict-semver"; @@ -503,10 +504,10 @@ function readBoundedFile( // injected stat/read behavior. const inspectLexical = deps.lstat ?? (deps.stat === undefined && deps.readFile === undefined ? lstatSync : null); - let lexicalBefore: ReturnType | null = null; + let lexicalBefore: BigIntStats | null = null; if (inspectLexical) { try { - lexicalBefore = inspectLexical(path); + lexicalBefore = inspectLexical(path, { bigint: true }); if (lexicalBefore.isSymbolicLink() || !lexicalBefore.isFile()) return null; } catch { return null; @@ -520,9 +521,9 @@ function readBoundedFile( ? fsConstants.O_RDONLY : fsConstants.O_RDONLY | fsConstants.O_NOFOLLOW | fsConstants.O_NONBLOCK; fd = openSync(path, flags); - const before = fstatSync(fd); + const before = fstatSync(fd, { bigint: true }); if (!before.isFile() || before.size > maxBytes) return null; - const bytes = Buffer.allocUnsafe(before.size); + const bytes = Buffer.allocUnsafe(Number(before.size)); let offset = 0; while (offset < bytes.length) { const count = readSync(fd, bytes, offset, bytes.length - offset, offset); @@ -531,16 +532,16 @@ function readBoundedFile( } const extra = Buffer.allocUnsafe(1); if (readSync(fd, extra, 0, 1, offset) !== 0) return null; - const after = fstatSync(fd); - const lexicalAfter = inspectLexical ? inspectLexical(path) : null; + const after = fstatSync(fd, { bigint: true }); + const lexicalAfter = inspectLexical ? inspectLexical(path, { bigint: true }) : null; if (lexicalAfter?.isSymbolicLink()) return null; - if (before.dev !== after.dev || before.ino !== after.ino || before.size !== after.size - || before.mtimeMs !== after.mtimeMs || before.ctimeMs !== after.ctimeMs + if (!sameFileIdentity(before, after) || before.size !== after.size + || statTimeMs(before.mtimeNs) !== statTimeMs(after.mtimeNs) || statTimeMs(before.ctimeNs) !== statTimeMs(after.ctimeNs) || (lexicalBefore !== null && (lexicalAfter === null || lexicalAfter.isSymbolicLink() - || lexicalBefore.dev !== before.dev || lexicalBefore.ino !== before.ino - || lexicalAfter.dev !== after.dev || lexicalAfter.ino !== after.ino - || lexicalAfter.size !== after.size || lexicalAfter.mtimeMs !== after.mtimeMs - || lexicalAfter.ctimeMs !== after.ctimeMs))) return null; + || !sameFileIdentity(lexicalBefore, before) + || !sameFileIdentity(lexicalAfter, after) + || lexicalAfter.size !== after.size || statTimeMs(lexicalAfter.mtimeNs) !== statTimeMs(after.mtimeNs) + || statTimeMs(lexicalAfter.ctimeNs) !== statTimeMs(after.ctimeNs)))) return null; return bytes; } catch { return null; @@ -552,20 +553,19 @@ function readBoundedFile( const stat = deps.stat; const read = deps.readFile; try { - const before = stat(path); + const before = stat(path, { bigint: true }); if (!before.isFile() || before.size > maxBytes) return null; const bytes = read(path); - const after = stat(path); - const lexicalAfter = inspectLexical ? inspectLexical(path) : null; + const after = stat(path, { bigint: true }); + const lexicalAfter = inspectLexical ? inspectLexical(path, { bigint: true }) : null; if ( - bytes.byteLength !== before.size - || before.dev !== after.dev - || before.ino !== after.ino + BigInt(bytes.byteLength) !== before.size + || !sameFileIdentity(before, after) || before.size !== after.size - || before.mtimeMs !== after.mtimeMs + || statTimeMs(before.mtimeNs) !== statTimeMs(after.mtimeNs) || (lexicalBefore !== null && (lexicalAfter === null || lexicalAfter.isSymbolicLink() - || lexicalBefore.dev !== before.dev || lexicalBefore.ino !== before.ino - || lexicalAfter.dev !== after.dev || lexicalAfter.ino !== after.ino)) + || !sameFileIdentity(lexicalBefore, before) + || !sameFileIdentity(lexicalAfter, after))) ) return null; return bytes; } catch { @@ -830,16 +830,16 @@ function observeHeldFileSet( const read = deps.readFile; if (!stat || !read) return null; const results = new Map(); - const identities: { before: Stats; req: InstallContentFile }[] = []; + const identities: { before: BigIntStats; req: InstallContentFile }[] = []; try { for (const req of requests) { - const before = stat(req.path); + const before = stat(req.path, { bigint: true }); if (!before.isFile() || before.size > req.maxBytes) return null; const bytes = read(req.path); - const after = stat(req.path); - if (bytes.byteLength !== before.size - || before.dev !== after.dev || before.ino !== after.ino - || before.size !== after.size || before.mtimeMs !== after.mtimeMs) return null; + const after = stat(req.path, { bigint: true }); + if (BigInt(bytes.byteLength) !== before.size + || !sameFileIdentity(before, after) + || before.size !== after.size || statTimeMs(before.mtimeNs) !== statTimeMs(after.mtimeNs)) return null; identities.push({ before, req }); results.set(req.role, { digest: sha256(fileContentDomain(req.role), bytes), @@ -848,9 +848,9 @@ function observeHeldFileSet( } // A replacement that slipped in after a file's own read still failed. for (const { before, req } of identities) { - const now = stat(req.path); - if (before.dev !== now.dev || before.ino !== now.ino - || before.size !== now.size || before.mtimeMs !== now.mtimeMs) return null; + const now = stat(req.path, { bigint: true }); + if (!sameFileIdentity(before, now) + || before.size !== now.size || statTimeMs(before.mtimeNs) !== statTimeMs(now.mtimeNs)) return null; } if (finalize && !finalize()) return null; return results; @@ -858,7 +858,7 @@ function observeHeldFileSet( return null; } } - const held: { fd: number; req: InstallContentFile; before: Stats }[] = []; + const held: { fd: number; req: InstallContentFile; before: BigIntStats }[] = []; const openedFds: number[] = []; try { const flags = process.platform === "win32" @@ -870,7 +870,7 @@ function observeHeldFileSet( // The descriptor joins the close set before any check that can bail, so // an fstat failure or a refused type/size cannot leak it. openedFds.push(fd); - const before = fstatSync(fd); + const before = fstatSync(fd, { bigint: true }); // O_NONBLOCK keeps a FIFO open nonblocking, so the type check must come // first: anything that is not a regular file is refused before a read. if (!before.isFile() || before.size > req.maxBytes) return null; @@ -881,11 +881,12 @@ function observeHeldFileSet( const hasher = createHash("sha256") .update(fileContentDomain(req.role), "utf8") .update("\0", "utf8"); - const bytes = keepBytes(req.role) ? Buffer.allocUnsafe(before.size) : null; - const chunk = Buffer.allocUnsafe(Math.max(1, Math.min(HELD_READ_CHUNK_BYTES, before.size))); + const size = Number(before.size); + const bytes = keepBytes(req.role) ? Buffer.allocUnsafe(size) : null; + const chunk = Buffer.allocUnsafe(Math.max(1, Math.min(HELD_READ_CHUNK_BYTES, size))); let offset = 0; - while (offset < before.size) { - const count = readSync(fd, chunk, 0, Math.min(chunk.length, before.size - offset), offset); + while (offset < size) { + const count = readSync(fd, chunk, 0, Math.min(chunk.length, size - offset), offset); if (count <= 0) return null; hasher.update(chunk.subarray(0, count)); if (bytes) chunk.copy(bytes, offset, 0, count); @@ -896,15 +897,15 @@ function observeHeldFileSet( results.set(req.role, { digest: hasher.digest("hex"), bytes }); } for (const { fd, req, before } of held) { - const after = fstatSync(fd); - const lexical = lstatSync(req.path); - const followed = statSync(req.path); - if (before.dev !== after.dev || before.ino !== after.ino - || before.size !== after.size || before.mtimeMs !== after.mtimeMs - || before.ctimeMs !== after.ctimeMs + const after = fstatSync(fd, { bigint: true }); + const lexical = lstatSync(req.path, { bigint: true }); + const followed = statSync(req.path, { bigint: true }); + if (!sameFileIdentity(before, after) + || before.size !== after.size || statTimeMs(before.mtimeNs) !== statTimeMs(after.mtimeNs) + || statTimeMs(before.ctimeNs) !== statTimeMs(after.ctimeNs) || lexical.isSymbolicLink() || !lexical.isFile() - || lexical.dev !== before.dev || lexical.ino !== before.ino - || followed.dev !== before.dev || followed.ino !== before.ino) return null; + || !sameFileIdentity(lexical, before) + || !sameFileIdentity(followed, before)) return null; } if (finalize && !finalize()) return null; return results; diff --git a/src/codex/cli-update-artifacts.ts b/src/codex/cli-update-artifacts.ts index f40d83d0453..8d333cedbec 100644 --- a/src/codex/cli-update-artifacts.ts +++ b/src/codex/cli-update-artifacts.ts @@ -4,6 +4,8 @@ import { closeSync, constants, createReadStream, fstatSync, lstatSync, openSync import { Readable, Transform } from "node:stream"; import { pipeline } from "node:stream/promises"; import { parseStrictSemver } from "../lib/strict-semver"; +import { sameFileIdentity, statTimeMs } from "../lib/file-identity"; +import type { BigIntStats } from "node:fs"; import { codexCliContentFileDigest, codexCliContentSetDigest, codexCliPlatformBacking } from "./cli-install-provenance"; import { windowsArchivePathValidator, WINDOWS_CODEX_HEADER_BYTES, WINDOWS_CODEX_NATIVE_MEMBER } from "./cli-update-windows-paths"; @@ -105,10 +107,10 @@ export async function verifyCodexCliArchive(source: Buffer | string, target: Cod options: { windowsX64?: boolean } = {}): Promise { if (!isSha512Integrity(target.integrity)) throw new Error("artifact_integrity_mismatch"); let fd: number | undefined; - let before: ReturnType | undefined; + let before: BigIntStats | undefined; if (typeof source === "string") { fd = openSync(source, constants.O_RDONLY | constants.O_NOFOLLOW | constants.O_NONBLOCK); - try { before = fstatSync(fd); } catch (error) { closeSync(fd); throw error; } + try { before = fstatSync(fd, { bigint: true }); } catch (error) { closeSync(fd); throw error; } if (!before.isFile() || before.size > MAX_CODEX_ARCHIVE_BYTES) { closeSync(fd); throw new Error("artifact_file_unverified"); } } let input: Readable; @@ -208,10 +210,10 @@ export async function verifyCodexCliArchive(source: Buffer | string, target: Cod if (!ended || trailer < 512 || current || padding || headerSize) throw new Error("artifact_tar_truncated"); if ("sha512-" + archiveHash.digest("base64") !== target.integrity) throw new Error("artifact_integrity_mismatch"); if (fd !== undefined && before && typeof source === "string") { - const after = fstatSync(fd), lexical = lstatSync(source); - if (before.dev !== after.dev || before.ino !== after.ino || before.size !== after.size - || before.mtimeMs !== after.mtimeMs || before.ctimeMs !== after.ctimeMs || lexical.isSymbolicLink() - || lexical.dev !== before.dev || lexical.ino !== before.ino) throw new Error("artifact_file_changed"); + const after = fstatSync(fd, { bigint: true }), lexical = lstatSync(source, { bigint: true }); + if (!sameFileIdentity(before, after) || before.size !== after.size + || statTimeMs(before.mtimeNs) !== statTimeMs(after.mtimeNs) || statTimeMs(before.ctimeNs) !== statTimeMs(after.ctimeNs) || lexical.isSymbolicLink() + || !sameFileIdentity(lexical, before)) throw new Error("artifact_file_changed"); } verified = true; } finally { diff --git a/tests/codex-integration/codex-cli-install-provenance.test.ts b/tests/codex-integration/codex-cli-install-provenance.test.ts index 9d282870d1c..b482bdc83e7 100644 --- a/tests/codex-integration/codex-cli-install-provenance.test.ts +++ b/tests/codex-integration/codex-cli-install-provenance.test.ts @@ -696,13 +696,37 @@ describe("Codex CLI install provenance", () => { }); describe("install content binding", () => { + test("a manifest path identity change above numeric precision refuses", async () => { + const fs = virtualManagedNpmGlobal(); + let changed = false; + const deps = virtualInspectionDeps(fs, { + lstat: (path: string, options?: { bigint?: boolean }) => { + const value = fs.lstat(path, options) as object; + return path === VIRTUAL_MANIFEST + ? { ...value, ino: options?.bigint ? 9007199254740992n + BigInt(changed) : 9007199254740992 } + : value; + }, + stat: (path: string, options?: { bigint?: boolean }) => { + const value = fs.stat(path, options) as object; + return path === VIRTUAL_MANIFEST ? { ...value, ino: options?.bigint ? 9007199254740992n : 9007199254740992 } : value; + }, + readFile: (path: string) => { + const value = fs.readFile(path); + if (path === VIRTUAL_MANIFEST) changed = true; + return value; + }, + }); + expect((await inspectCodexCliInstall(deps)).managed).toBe(false); + expect(changed).toBe(true); + }); + test("an ownership-only read never opens the backing bytes", async () => { const fs = virtualManagedNpmGlobal(); const probed: string[] = []; const deps = virtualInspectionDeps(fs, { - lstat: (path: string) => { + lstat: (path: string, options?: { bigint?: boolean }) => { if (path.includes("@openai/codex-")) probed.push(path); - return fs.lstat(path); + return fs.lstat(path, options); }, readFile: (path: string) => { if (path === VIRTUAL_BACKING_BINARY) { @@ -795,8 +819,8 @@ describe("install content binding", () => { const stat = fs.stat; const deps = virtualInspectionDeps(fs, { observeInstallContent: true, - stat: (path: string) => { - const result = stat(path); + stat: (path: string, options?: { bigint?: boolean }) => { + const result = stat(path, options); if (path === VIRTUAL_BACKING_BINARY) { backingStats.count += 1; if (backingStats.count === 1) fs.bumpIdentity(path); @@ -808,6 +832,22 @@ describe("install content binding", () => { expect(report.installContentDigest).toBeNull(); }); + test("a backing identity change above numeric precision refuses", async () => { + const fs = virtualManagedNpmGlobal(); + let count = 0; + const deps = virtualInspectionDeps(fs, { + observeInstallContent: true, + stat: (path: string, options?: { bigint?: boolean }) => { + const value = fs.stat(path, options) as object; + if (path !== VIRTUAL_BACKING_BINARY) return value; + const changed = count++ > 0; + return { ...value, ino: options?.bigint ? 9007199254740992n + BigInt(changed) : 9007199254740992 }; + }, + }); + expect((await inspectCodexCliInstall(deps)).installContentDigest).toBeNull(); + expect(count).toBeGreaterThan(1); + }); + test("the package-vendor fallback binds when no resolve hit exists and globals are fully probed", async () => { const fs = virtualPosixFs({ [VIRTUAL_LAUNCHER]: { link: VIRTUAL_ENTRYPOINT }, diff --git a/tests/codex-integration/codex-cli-update-artifacts.test.ts b/tests/codex-integration/codex-cli-update-artifacts.test.ts index 2c19159983e..7b2d91fc487 100644 --- a/tests/codex-integration/codex-cli-update-artifacts.test.ts +++ b/tests/codex-integration/codex-cli-update-artifacts.test.ts @@ -87,6 +87,23 @@ describe("Codex CLI apply artifacts", () => { const f = fixtureArtifactSet(); const changed = Buffer.from(f.main); changed[changed.length - 1] ^= 1; await expect(verifyCodexCliArchive(changed, f.artifacts.main)).rejects.toThrow(); }); + test("rejects an archive path identity differing above numeric precision", async () => { + const root = createCodexCliTestRoot("ocx-archive-identity-"); + const f = fixtureArtifactSet(), path = join(root, "artifact.tgz"); + writeFileSync(path, f.main); + const fstat = fs.fstatSync, lstat = fs.lstatSync; + const opened = spyOn(fs, "fstatSync").mockImplementation(((fd: number, options?: { bigint?: boolean }) => { + const value = fstat(fd, options as never); + return { ...value, isFile: () => value.isFile(), dev: options?.bigint ? 42n : 42, ino: options?.bigint ? 9007199254740992n : 9007199254740992 }; + }) as typeof fs.fstatSync); + const lexical = spyOn(fs, "lstatSync").mockImplementation(((name: fs.PathLike, options?: { bigint?: boolean }) => { + const value = lstat(name, options as never); + return { ...value, isSymbolicLink: () => value.isSymbolicLink(), dev: options?.bigint ? 42n : 42, ino: options?.bigint ? 9007199254740993n : 9007199254740992 }; + }) as typeof fs.lstatSync); + try { + await expect(verifyCodexCliArchive(path, f.artifacts.main)).rejects.toThrow("artifact_file_changed"); + } finally { opened.mockRestore(); lexical.mockRestore(); removeTreeWithRetry(root); } + }); test.each(["valid", "corrupt"] as const)("real-file archive %s closes its stream before return and preserves a reused sentinel fd", async kind => { const root = createCodexCliTestRoot("ocx-archive-lifetime-"); const f = fixtureArtifactSet(), path = join(root, "artifact.tgz"), sentinelPath = join(root, "sentinel"); diff --git a/tests/helpers/codex-cli-apply-admission.ts b/tests/helpers/codex-cli-apply-admission.ts index 5d3507676ef..5768c0dd2eb 100644 --- a/tests/helpers/codex-cli-apply-admission.ts +++ b/tests/helpers/codex-cli-apply-admission.ts @@ -43,11 +43,15 @@ const lstat = fs.lstatSync, uid = process.geteuid?.() ?? 1234; const originalUid = Object.getOwnPropertyDescriptor(process, "geteuid"); let barrier = false; Object.defineProperty(process, "geteuid", { configurable: true, value: () => uid }); -const stat = spyOn(fs, "lstatSync").mockImplementation(((path: fs.PathLike) => { - const value = lstat(path); - return value.isDirectory() ? Object.assign(value, { uid, - mode: 0o40700 | (path === temporaryRoot && lane === "unsafe-stage" ? 0o002 : 0) - | (barrier && path === mutationDirectory && lane === "unsafe-install" ? 0o020 : 0) }) : value; +// Honor the options argument: identity checks request bigint stats. +const stat = spyOn(fs, "lstatSync").mockImplementation(((path: fs.PathLike, options?: fs.StatOptions) => { + const value = lstat(path, options as fs.StatSyncOptions & { bigint: false }); + if (!value?.isDirectory()) return value; + const mode = 0o40700 | (path === temporaryRoot && lane === "unsafe-stage" ? 0o002 : 0) + | (barrier && path === mutationDirectory && lane === "unsafe-install" ? 0o020 : 0); + return options?.bigint + ? Object.assign(value, { uid: BigInt(uid), mode: BigInt(mode) }) + : Object.assign(value, { uid, mode }); }) as typeof fs.lstatSync); let arms = 0, unavailable = false; const recoveryTimers: (() => void)[] = []; diff --git a/tests/helpers/codex-cli-content-fs.ts b/tests/helpers/codex-cli-content-fs.ts index bef0c687720..8f06cef717f 100644 --- a/tests/helpers/codex-cli-content-fs.ts +++ b/tests/helpers/codex-cli-content-fs.ts @@ -8,9 +8,8 @@ import type { CodexCliInstallProvenanceDeps } from "../../src/codex/cli-install- * Every path is absolute and lexical; only regular files and final-component * symlinks exist (directories are implied by path prefixes, which is all the * provenance inspection ever stats). stat follows the whole link chain, lstat - * reports the final component as written. Inode and mtime fields are plain - * numbers so a test can model a mid-observation identity change by mutating - * them between calls. + * reports the final component as written. Observations honor the bigint option; + * tests can mutate inode and timestamp fields between calls. */ export interface VirtualFileSpec { readonly data?: string | Buffer; @@ -42,8 +41,8 @@ const enoent = (): never => { export interface VirtualPosixFs { readonly exists: (path: string) => boolean; - readonly lstat: (path: string) => unknown; - readonly stat: (path: string) => unknown; + readonly lstat: (path: string, options?: { bigint?: boolean }) => unknown; + readonly stat: (path: string, options?: { bigint?: boolean }) => unknown; readonly readFile: (path: string) => Buffer; readonly realpath: (path: string) => string; /** Replace a regular file's bytes in place, keeping inode and timestamps. */ @@ -92,17 +91,28 @@ export function virtualPosixFs(initial: Record): Virtua } }; - const statLike = (entry: VirtualEntry) => ({ - isFile: () => entry.kind === "file", - isSymbolicLink: () => entry.kind === "symlink", - isDirectory: () => false, - mode: entry.kind === "file" ? entry.mode : 0o777, - size: entry.kind === "file" ? entry.bytes.length : entry.target.length, - dev: 42, - ino: entry.kind === "file" ? entry.ino : 0, - mtimeMs: entry.kind === "file" ? entry.mtimeMs : 0, - ctimeMs: entry.kind === "file" ? entry.ctimeMs : 0, - }); + // Fractional milliseconds to nanoseconds without requiring an integral value. + const nanoseconds = (ms: number): bigint => BigInt(Math.trunc(ms)) * 1_000_000n + BigInt(Math.round((ms % 1) * 1e6)); + // Same field types as node:fs Stats and BigIntStats: bigint observations carry bigint mode, + // truncated bigint *Ms and exact *Ns; numeric observations carry no *Ns fields. + const statLike = (entry: VirtualEntry, options?: { bigint?: boolean }) => { + const mode = entry.kind === "file" ? entry.mode : 0o777; + const size = entry.kind === "file" ? entry.bytes.length : entry.target.length; + const ino = entry.kind === "file" ? entry.ino : 0; + const mtimeMs = entry.kind === "file" ? entry.mtimeMs : 0; + const ctimeMs = entry.kind === "file" ? entry.ctimeMs : 0; + const kind = { + isFile: () => entry.kind === "file", + isSymbolicLink: () => entry.kind === "symlink", + isDirectory: () => false, + }; + if (!options?.bigint) return { ...kind, mode, size, dev: 42, ino, mtimeMs, ctimeMs }; + return { + ...kind, mode: BigInt(mode), size: BigInt(size), dev: 42n, ino: BigInt(ino), + mtimeMs: BigInt(Math.trunc(mtimeMs)), ctimeMs: BigInt(Math.trunc(ctimeMs)), + mtimeNs: nanoseconds(mtimeMs), ctimeNs: nanoseconds(ctimeMs), + }; + }; const fileAt = (path: string): VirtualFileEntry => { const resolved = entries.get(resolvePath(path)); @@ -112,12 +122,12 @@ export function virtualPosixFs(initial: Record): Virtua return { exists: path => entries.get(resolvePath(path))?.kind === "file", - lstat: path => { + lstat: (path, options) => { const entry = entries.get(norm(path)); if (!entry) enoent(); - return statLike(entry); + return statLike(entry, options); }, - stat: path => statLike(fileAt(path)), + stat: (path, options) => statLike(fileAt(path), options), readFile: path => fileAt(path).bytes, realpath: path => { const resolved = resolvePath(path); diff --git a/tests/lib/file-identity-callers.test.ts b/tests/lib/file-identity-callers.test.ts index d11920d7080..43004e286f2 100644 --- a/tests/lib/file-identity-callers.test.ts +++ b/tests/lib/file-identity-callers.test.ts @@ -7,6 +7,8 @@ import { repoPath } from "../helpers/repo-root"; // full parser: it catches the regression forms below and fails closed on scopes it cannot find. const scopes: Record = { "src/codex/codex-home-owner.ts": ["readCodexHomeJournal"], + "src/codex/cli-update-artifacts.ts": ["verifyCodexCliArchive"], + "src/codex/cli-install-provenance.ts": ["readBoundedFile", "observeHeldFileSet"], "src/codex/orca-auth-source.ts": ["readBoundedLocalFile"], "src/codex/inject/bounded-config-reader.ts": ["readBoundedCodexConfig"], "src/codex/main-account.ts": ["statIdentity"],