diff --git a/plugins/codex-security/mcp-app/package.json b/plugins/codex-security/mcp-app/package.json index 6dfeb1cd7..a08f6242d 100644 --- a/plugins/codex-security/mcp-app/package.json +++ b/plugins/codex-security/mcp-app/package.json @@ -1,6 +1,6 @@ { "name": "codex-security-mcp-app", - "version": "0.1.158", + "version": "0.1.159", "type": "module", "private": true, "scripts": { diff --git a/plugins/codex-security/mcp-app/src/deep-scan/executor.ts b/plugins/codex-security/mcp-app/src/deep-scan/executor.ts index 2d1753ebe..4c5e202d5 100644 --- a/plugins/codex-security/mcp-app/src/deep-scan/executor.ts +++ b/plugins/codex-security/mcp-app/src/deep-scan/executor.ts @@ -37,7 +37,24 @@ export interface CodexSdkWorkerArtifactContext { pythonCommand?: string; } +interface BoundCodexExecutable { + readonly path: string; + readonly identity: CodexExecutableIdentity; +} + +interface CodexExecutableIdentity { + readonly device: bigint; + readonly inode: bigint; + readonly size: bigint; + readonly modifiedAt: bigint; + readonly changedAt: bigint; +} + export class CodexSdkWorkerExecutor implements CodexWorkerExecutor { + // A live coordinator owns one executor. Keep its CLI stable even if Desktop + // installs a newer cached binary while later workers are still pending. + private codexExecutable: Promise | undefined; + constructor(private readonly modelSettings: CodexSdkWorkerModelSettings = {}) {} async run(request: CodexWorkerRequest): Promise { @@ -52,14 +69,10 @@ export class CodexSdkWorkerExecutor implements CodexWorkerExecutor { const configOverrides = workerPermissionProfileConfigOverrides(workerProfile); const originalCwd = process.cwd(); const childEnv = await snapshotWorkerEnvironment(); - const codexPath = resolveCodexPath( - childEnv, - process.platform, - process.arch, - originalCwd - ); + const executable = await this.boundCodexExecutable(childEnv, originalCwd); + await verifyBoundCodexExecutable(executable); await preflightDeepScanWorkerPermissionProfile({ - codexPath, + codexPath: executable.path, cwd: request.workingDirectory, profileId: DEEP_SCAN_WORKER_PERMISSION_PROFILE_ID, configOverrides, @@ -69,7 +82,7 @@ export class CodexSdkWorkerExecutor implements CodexWorkerExecutor { }); const prompt = await fs.readFile(request.promptPath, "utf8"); const codex = new Codex({ - codexPathOverride: executablePathForSpawn(codexPath), + codexPathOverride: executablePathForSpawn(executable.path), env: childEnv, config: { // The CLI can add effort levels before the pinned SDK widens ThreadOptions. @@ -110,6 +123,9 @@ export class CodexSdkWorkerExecutor implements CodexWorkerExecutor { } try { + // Keep the final identity check at the launch boundary. The preflight + // and SDK worker must never silently use different CLI installations. + await verifyBoundCodexExecutable(executable); const { events } = await thread.runStreamed(input, { signal: controller.signal }); let finalResponse = ""; let threadId: string | undefined; @@ -166,6 +182,14 @@ export class CodexSdkWorkerExecutor implements CodexWorkerExecutor { } } + private boundCodexExecutable( + env: NodeJS.ProcessEnv, + originalCwd: string + ): Promise { + this.codexExecutable ??= bindCodexExecutable(env, originalCwd); + return this.codexExecutable; + } + private compactArtifactServer(request: CodexWorkerRequest): Record { return typeof value === "object" && value !== null; } +async function bindCodexExecutable( + env: NodeJS.ProcessEnv, + originalCwd: string +): Promise { + const path = resolveCodexPath(env, process.platform, process.arch, originalCwd); + let identity: CodexExecutableIdentity; + try { + identity = await codexExecutableIdentity(path); + } catch (error) { + throw new DeepScanNonRetryableError( + "Deep Scan cannot start a worker because the selected Codex executable " + + `"${path}" is not an available file. ` + + "Check CODEX_CLI_PATH/PATH, then retry.", + { cause: error } + ); + } + return { path, identity }; +} + +async function verifyBoundCodexExecutable( + executable: BoundCodexExecutable +): Promise { + let identity: CodexExecutableIdentity; + try { + identity = await codexExecutableIdentity(executable.path); + } catch (error) { + throw codexExecutableChangedError(executable.path, error); + } + if (!sameCodexExecutable(executable.identity, identity)) { + throw codexExecutableChangedError(executable.path); + } +} + +async function codexExecutableIdentity(path: string): Promise { + const metadata = await fs.stat(path, { bigint: true }); + if (!metadata.isFile() || metadata.size === 0n) { + throw new Error("Codex executable is not a non-empty file."); + } + return { + device: metadata.dev, + inode: metadata.ino, + size: metadata.size, + modifiedAt: metadata.mtimeNs, + changedAt: metadata.ctimeNs + }; +} + +function sameCodexExecutable( + expected: CodexExecutableIdentity, + actual: CodexExecutableIdentity +): boolean { + return expected.device === actual.device + && expected.inode === actual.inode + && expected.size === actual.size + && expected.modifiedAt === actual.modifiedAt + && expected.changedAt === actual.changedAt; +} + +function codexExecutableChangedError(path: string, cause?: unknown): Error { + return new DeepScanNonRetryableError( + "Deep Scan cannot start this worker because the Codex executable " + + `"${path}" changed after this Deep Scan started or is no longer available. ` + + "This usually means Codex updated while the scan was running. Retry after the update " + + "finishes; completed work remains in the retained partial scan.", + { cause } + ); +} + async function snapshotWorkerEnvironment(): Promise> { const environment = Object.fromEntries( Object.entries(process.env) diff --git a/plugins/codex-security/mcp-app/tests/test_deep_scan_executor.mjs b/plugins/codex-security/mcp-app/tests/test_deep_scan_executor.mjs index b703cd2bb..e191c4beb 100644 --- a/plugins/codex-security/mcp-app/tests/test_deep_scan_executor.mjs +++ b/plugins/codex-security/mcp-app/tests/test_deep_scan_executor.mjs @@ -77,6 +77,9 @@ try { await testDisallowedWorkerProfileFailsBeforeWorkerLaunch(); await testRuntimePermissionProfileFallbackStopsAndDiscards(); await testWorkerLaunchesWithoutGlobalCodex(); + await testExecutorPinsCodexExecutableAcrossWorkers(); + await testExecutorRejectsReplacedPinnedExecutable(); + await testExecutorRejectsReplacementAfterPreflight(); await testPreflightBindsExecutableAndHomeBeforeChangingCwd(); await testSdkInvocationAndThreadCapture(); await testBedrockCredentialsReachWorker(); @@ -534,6 +537,122 @@ async function testWorkerLaunchesWithoutGlobalCodex() { } } +async function testExecutorPinsCodexExecutableAcrossWorkers() { + const firstFixture = await fakeCodexFixture(); + const replacementFixture = await fakeCodexFixture(); + const previousCodexPath = process.env.CODEX_CLI_PATH; + process.env.CODEX_CLI_PATH = firstFixture.executablePath; + + try { + const promptPath = path.join(firstFixture.root, "prompt.md"); + const workingDirectory = path.join(firstFixture.root, "artifacts"); + await mkdir(workingDirectory); + await writeFile(promptPath, "fixture pinned worker executable\n"); + const executor = new CodexSdkWorkerExecutor({ + parentSandbox: trustedParentSandbox + }); + const request = { + kind: "discovery", + promptPath, + workingDirectory, + subagents: 0, + signal: new AbortController().signal + }; + + assert.equal((await executor.run(request)).finalResponse, "fixture final response"); + process.env.CODEX_CLI_PATH = replacementFixture.executablePath; + assert.equal((await executor.run(request)).finalResponse, "fixture final response"); + + await assert.rejects( + readFile(replacementFixture.preflightMarkerPath), + (error) => error?.code === "ENOENT" + ); + const preflight = JSON.parse(await readFile(firstFixture.preflightMarkerPath, "utf8")); + assert.deepEqual(preflight.requests, [ + { method: "config/read", cwd: workingDirectory }, + { method: "permissionProfile/list", cwd: workingDirectory } + ]); + } finally { + restoreEnv("CODEX_CLI_PATH", previousCodexPath); + } +} + +async function testExecutorRejectsReplacedPinnedExecutable() { + const fixture = await fakeCodexFixture(); + const previousCodexPath = process.env.CODEX_CLI_PATH; + process.env.CODEX_CLI_PATH = fixture.executablePath; + + try { + const promptPath = path.join(fixture.root, "prompt.md"); + const workingDirectory = path.join(fixture.root, "artifacts"); + await mkdir(workingDirectory); + await writeFile(promptPath, "fixture replaced pinned worker executable\n"); + const executor = new CodexSdkWorkerExecutor({ + parentSandbox: trustedParentSandbox + }); + const request = { + kind: "discovery", + promptPath, + workingDirectory, + subagents: 0, + signal: new AbortController().signal + }; + + assert.equal((await executor.run(request)).finalResponse, "fixture final response"); + await rm(fixture.preflightMarkerPath); + const originalExecutable = await readFile(fixture.executablePath, "utf8"); + await writeFile(fixture.executablePath, `${originalExecutable}\n// replaced after scan start\n`); + + await assert.rejects( + executor.run(request), + (error) => error?.name === "DeepScanNonRetryableError" + && /changed after this Deep Scan started/i.test(error.message) + ); + await assert.rejects( + readFile(fixture.preflightMarkerPath), + (error) => error?.code === "ENOENT" + ); + } finally { + restoreEnv("CODEX_CLI_PATH", previousCodexPath); + } +} + +async function testExecutorRejectsReplacementAfterPreflight() { + const fixture = await fakeCodexFixture(); + const previousCodexPath = process.env.CODEX_CLI_PATH; + const previousReplaceAfterPreflight = process.env.FAKE_CODEX_REPLACE_AFTER_PREFLIGHT; + process.env.CODEX_CLI_PATH = fixture.executablePath; + process.env.FAKE_CODEX_REPLACE_AFTER_PREFLIGHT = "1"; + + try { + const promptPath = path.join(fixture.root, "prompt.md"); + const workingDirectory = path.join(fixture.root, "artifacts"); + await mkdir(workingDirectory); + await writeFile(promptPath, "fixture replacement after preflight\n"); + + await assert.rejects( + new CodexSdkWorkerExecutor({ + parentSandbox: trustedParentSandbox + }).run({ + kind: "discovery", + promptPath, + workingDirectory, + subagents: 0, + signal: new AbortController().signal + }), + (error) => error?.name === "DeepScanNonRetryableError" + && /changed after this Deep Scan started/i.test(error.message) + ); + await assert.rejects( + readFile(fixture.markerPath), + (error) => error?.code === "ENOENT" + ); + } finally { + restoreEnv("CODEX_CLI_PATH", previousCodexPath); + restoreEnv("FAKE_CODEX_REPLACE_AFTER_PREFLIGHT", previousReplaceAfterPreflight); + } +} + async function testPreflightBindsExecutableAndHomeBeforeChangingCwd() { const fixture = await fakeCodexFixture(); const originalCwd = process.cwd(); @@ -1378,6 +1497,7 @@ async function fakeCodexFixture( " continue;", " }", " process.stdout.write(JSON.stringify({ jsonrpc: '2.0', id: message.id, result }) + '\\n');", + " if (message.method === 'permissionProfile/list' && process.env.FAKE_CODEX_REPLACE_AFTER_PREFLIGHT === '1') writeFileSync(process.argv[1], '#!/usr/bin/env node\\nprocess.exit(97);\\n');", " }", " });", " process.stdin.on('end', () => process.exit(0));", diff --git a/sdk/typescript/tests-ts/deep-scan-worker-shutdown.test.ts b/sdk/typescript/tests-ts/deep-scan-worker-shutdown.test.ts index ddcda61fe..53eaedab8 100644 --- a/sdk/typescript/tests-ts/deep-scan-worker-shutdown.test.ts +++ b/sdk/typescript/tests-ts/deep-scan-worker-shutdown.test.ts @@ -54,6 +54,8 @@ async function bundledWorkerExecutor( "workerPermissionProfile", "workerPermissionProfileConfigOverrides", "snapshotWorkerEnvironment", + "bindCodexExecutable", + "verifyBoundCodexExecutable", "preflightDeepScanWorkerPermissionProfile", "DEEP_SCAN_WORKER_PERMISSION_PROFILE_ID", "deepScanPermissionProfileFallbackError", @@ -69,6 +71,8 @@ async function bundledWorkerExecutor( () => ({}), () => [], async () => ({}), + async () => ({ path: "/fixture/codex", identity: {} }), + async () => {}, preflight, "codex_security_deep_scan_worker", () => undefined,