diff --git a/plugins/codex-security/.codex-plugin/plugin.json b/plugins/codex-security/.codex-plugin/plugin.json index bfeb6cd23..72eaf506a 100644 --- a/plugins/codex-security/.codex-plugin/plugin.json +++ b/plugins/codex-security/.codex-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "codex-security", - "version": "0.1.60", + "version": "0.1.79", "description": "Codex Security workflows for security scans, analysis, and investigation.", "author": { "name": "OpenAI" diff --git a/plugins/codex-security/scripts/finalize_scan_contract.py b/plugins/codex-security/scripts/finalize_scan_contract.py index 982e4ae6b..04f5a33bd 100644 --- a/plugins/codex-security/scripts/finalize_scan_contract.py +++ b/plugins/codex-security/scripts/finalize_scan_contract.py @@ -320,7 +320,7 @@ def _descriptor_relative_writes_available() -> bool: def _windows_scan_local_files() -> Any: - """Load the Win32 backend only on runtimes that need it.""" + """Load the Win32 backend and shared stream comparison lazily.""" global _WINDOWS_SCAN_LOCAL_FILES if _WINDOWS_SCAN_LOCAL_FILES is None: @@ -334,24 +334,56 @@ def _windows_scan_local_files() -> Any: return _WINDOWS_SCAN_LOCAL_FILES -def _open_verified_scan_directory(scan_dir: Path) -> int: +def _open_verified_scan_directory( + scan_dir: Path, expected_root_identity: tuple[int, int] | None = None +) -> int: scan_dir = scan_dir.absolute() try: expected = scan_dir.lstat() + observed_identity = (expected.st_dev, expected.st_ino) + if ( + expected_root_identity is not None + and observed_identity != expected_root_identity + ): + raise ContractError("scan directory: changed after artifact restoration setup") canonical = _require_scan_directory(scan_dir) descriptor = os.open( canonical, os.O_RDONLY | getattr(os, "O_DIRECTORY", 0) | getattr(os, "O_NOFOLLOW", 0), ) except OSError as exc: - raise ContractError("scan directory: expected an existing non-symlink directory") from exc + raise ContractError( + "scan directory: expected an existing non-symlink directory" + ) from exc opened = os.fstat(descriptor) - if (opened.st_dev, opened.st_ino) != (expected.st_dev, expected.st_ino): + opened_identity = (opened.st_dev, opened.st_ino) + if opened_identity != observed_identity or ( + expected_root_identity is not None + and opened_identity != expected_root_identity + ): os.close(descriptor) raise ContractError("scan directory: changed while it was being opened") return descriptor +def scan_root_identity(scan_dir: Path) -> tuple[Path, tuple[int, int]]: + """Return a canonical scan root and its identity from a held handle.""" + + scan_dir = _require_scan_directory(scan_dir) + if not _descriptor_relative_writes_available(): + if not _is_windows(): + raise ContractError( + "scan-local output requires descriptor-relative file operations" + ) + return _windows_scan_local_files().scan_root_identity(scan_dir) + descriptor = _open_verified_scan_directory(scan_dir) + try: + metadata = os.fstat(descriptor) + return scan_dir, (metadata.st_dev, metadata.st_ino) + finally: + os.close(descriptor) + + def _open_scan_local_directory(root_fd: int, parts: tuple[str, ...], *, create: bool) -> int: descriptor = os.dup(root_fd) try: @@ -500,7 +532,12 @@ def _sha256_scan_local_file(scan_dir: Path, relative_path: str, context: str) -> def write_scan_local_bytes( - scan_dir: Path, relative_path: str, payload: bytes, *, external_name: bool = False + scan_dir: Path, + relative_path: str, + payload: bytes, + *, + external_name: bool = False, + expected_root_identity: tuple[int, int] | None = None, ) -> None: scan_dir = _require_scan_directory(scan_dir) if external_name: @@ -513,7 +550,12 @@ def write_scan_local_bytes( if not _is_windows(): raise ContractError("scan-local output requires descriptor-relative file operations") try: - _windows_scan_local_files().atomic_write(scan_dir, relative_path, payload) + _windows_scan_local_files().atomic_write( + scan_dir, + relative_path, + payload, + expected_root_identity=expected_root_identity, + ) except OSError as exc: raise ContractError(f"{relative_path}: {exc}") from exc return @@ -521,7 +563,7 @@ def write_scan_local_bytes( parent_fd: int | None = None temp_name: str | None = None try: - root_fd = _open_verified_scan_directory(scan_dir) + root_fd = _open_verified_scan_directory(scan_dir, expected_root_identity) parts = PurePosixPath(relative_path).parts try: parent_fd = _open_scan_local_directory(root_fd, parts[:-1], create=True) @@ -529,6 +571,9 @@ def write_scan_local_bytes( raise ContractError( f"{relative_path}: expected a path inside the scan directory" ) from exc + # The held descriptor is the authority for the validated parent. A + # concurrent rename cannot redirect later operations through a + # replacement path or link. try: metadata = os.stat(parts[-1], dir_fd=parent_fd, follow_symlinks=False) except FileNotFoundError: @@ -536,6 +581,47 @@ def write_scan_local_bytes( else: if not stat.S_ISREG(metadata.st_mode): raise ContractError(f"{relative_path}: expected a regular non-symlink file") + try: + existing_fd = os.open( + parts[-1], + os.O_RDONLY + | getattr(os, "O_NOFOLLOW", 0) + | getattr(os, "O_NONBLOCK", 0), + dir_fd=parent_fd, + ) + except OSError as exc: + if exc.errno not in {errno.ENOENT, errno.EACCES, errno.EPERM}: + raise + else: + try: + opened = os.fstat(existing_fd) + if not stat.S_ISREG(opened.st_mode): + raise ContractError( + f"{relative_path}: expected a regular non-symlink file" + ) + if (opened.st_dev, opened.st_ino) != ( + metadata.st_dev, + metadata.st_ino, + ): + raise ContractError( + f"{relative_path}: changed while it was being opened" + ) + if ( + expected_root_identity is not None + and opened.st_size == len(payload) + ): + try: + with os.fdopen(existing_fd, "rb") as handle: + existing_fd = -1 + if _windows_scan_local_files().stream_matches_payload( + handle, payload + ): + return + except OSError: + pass + finally: + if existing_fd >= 0: + os.close(existing_fd) temp_name = f".{path.name}.{secrets.token_hex(8)}.tmp" temp_fd = os.open(temp_name, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600, dir_fd=parent_fd) with os.fdopen(temp_fd, "wb") as handle: diff --git a/plugins/codex-security/scripts/windows_scan_local_files.py b/plugins/codex-security/scripts/windows_scan_local_files.py index baef9508f..b89c6ef8b 100644 --- a/plugins/codex-security/scripts/windows_scan_local_files.py +++ b/plugins/codex-security/scripts/windows_scan_local_files.py @@ -11,8 +11,8 @@ write, or delete. Writes rename the exact temporary-file handle into place so an attacker cannot substitute another file at the temporary name. -Importing this module is safe on non-Windows hosts. Its public operations -raise ``WindowsScanLocalFileError`` when called anywhere other than Windows. +Importing this module and comparing streams work on every platform. Filesystem +operations raise ``WindowsScanLocalFileError`` outside Windows. """ from __future__ import annotations @@ -28,6 +28,7 @@ from collections.abc import Iterator from ctypes import wintypes from pathlib import Path, PurePosixPath +from typing import BinaryIO _msvcrt = importlib.import_module("msvcrt") if os.name == "nt" else None @@ -63,11 +64,15 @@ class WindowsScanLocalFileError(OSError): _FILE_NAME_OPENED = 0x00000008 _ERROR_FILE_NOT_FOUND = 2 _ERROR_PATH_NOT_FOUND = 3 +_ERROR_ACCESS_DENIED = 5 +_ERROR_SHARING_VIOLATION = 32 +_ERROR_LOCK_VIOLATION = 33 _ERROR_FILE_EXISTS = 80 _ERROR_ALREADY_EXISTS = 183 _MISSING_ERRORS = {_ERROR_FILE_NOT_FOUND, _ERROR_PATH_NOT_FOUND} _COLLISION_ERRORS = {_ERROR_FILE_EXISTS, _ERROR_ALREADY_EXISTS} _INVALID_HANDLE_VALUE = ctypes.c_void_p(-1).value +_MAX_COMPARE_CHUNK = 64 * 1024 _MAX_WRITE_CHUNK = 1024 * 1024 _INVALID_COMPONENT_CHARACTERS = frozenset('<>:"|?*') @@ -398,12 +403,20 @@ def _locked_parent( relative_path: str, *, create: bool, + expected_root_identity: tuple[int, int] | None = None, ) -> Iterator[tuple[Path, str]]: """Hold non-deletable handles for every directory in the absolute target path.""" _require_windows() parts = _validated_parts(relative_path) - root_path, expected_root_identity = _canonical_scan_directory(scan_dir) + root_path, observed_root_identity = _canonical_scan_directory(scan_dir) + if ( + expected_root_identity is not None + and observed_root_identity != expected_root_identity + ): + raise _invalid_path( + scan_dir, "scan directory changed after artifact restoration setup" + ) handles: list[_OwnedHandle] = [] try: # Absolute-path Win32 calls remain safe only while every ancestor is @@ -414,8 +427,14 @@ def _locked_parent( assert directory_handle is not None handles.append(directory_handle) current_root = root_path.lstat() - if (current_root.st_dev, current_root.st_ino) != expected_root_identity: - raise _invalid_path(scan_dir, "scan directory changed while it was being opened") + current_root_identity = (current_root.st_dev, current_root.st_ino) + if current_root_identity != observed_root_identity or ( + expected_root_identity is not None + and current_root_identity != expected_root_identity + ): + raise _invalid_path( + scan_dir, "scan directory changed while it was being opened" + ) current_path = root_path for component in parts[:-1]: current_path /= component @@ -431,6 +450,14 @@ def _locked_parent( handle.close() +def scan_root_identity(scan_dir: Path) -> tuple[Path, tuple[int, int]]: + """Return a canonical scan root and its identity while holding it fixed.""" + + with _locked_parent(scan_dir, ".identity", create=False) as (root_path, _): + metadata = root_path.lstat() + return root_path, (metadata.st_dev, metadata.st_ino) + + def open_read_fd(scan_dir: Path, relative_path: str, context: str) -> int: """Open a verified regular file and return an owned binary read descriptor.""" @@ -465,6 +492,16 @@ def open_read_fd(scan_dir: Path, relative_path: str, context: str) -> int: ) from exc +def stream_matches_payload(stream: BinaryIO, payload: bytes) -> bool: + offset = 0 + while offset < len(payload): + chunk = stream.read(min(_MAX_COMPARE_CHUNK, len(payload) - offset)) + if not chunk or chunk != payload[offset : offset + len(chunk)]: + return False + offset += len(chunk) + return stream.read(1) == b"" + + def _write_all(handle: int, payload: bytes) -> None: view = memoryview(payload) offset = 0 @@ -532,12 +569,69 @@ def _validate_existing_output(path: Path) -> None: _verify_regular_file(handle.value, path) -def atomic_write(scan_dir: Path, relative_path: str, payload: bytes) -> None: +def _existing_output_matches(path: Path, payload: bytes) -> bool: + try: + handle = _create_file( + path, + access=_GENERIC_READ | _FILE_READ_ATTRIBUTES, + # Deny write and delete sharing while comparing the opened contents. + share=_FILE_SHARE_READ, + disposition=_OPEN_EXISTING, + flags=_FILE_FLAG_OPEN_REPARSE_POINT | _FILE_FLAG_BACKUP_SEMANTICS, + missing_ok=True, + ) + except WindowsScanLocalFileError as exc: + if exc.errno in { + _ERROR_ACCESS_DENIED, + _ERROR_SHARING_VIOLATION, + _ERROR_LOCK_VIOLATION, + }: + return False + raise + if handle is None: + return False + with handle: + assert handle.value is not None + _verify_regular_file(handle.value, path) + raw_handle = handle.detach() + try: + assert _msvcrt is not None + descriptor = _msvcrt.open_osfhandle( + raw_handle, os.O_RDONLY | os.O_BINARY + ) + except BaseException: + _close_handle(raw_handle) + raise + try: + with os.fdopen(descriptor, "rb") as stream: + if os.fstat(stream.fileno()).st_size != len(payload): + return False + return stream_matches_payload(stream, payload) + except OSError: + return False + + +def atomic_write( + scan_dir: Path, + relative_path: str, + payload: bytes, + *, + expected_root_identity: tuple[int, int] | None = None, +) -> None: """Atomically replace a scan-local regular file with ``payload``.""" - with _locked_parent(scan_dir, relative_path, create=True) as (parent_path, leaf_name): + with _locked_parent( + scan_dir, + relative_path, + create=True, + expected_root_identity=expected_root_identity, + ) as (parent_path, leaf_name): destination_path = parent_path / leaf_name _validate_existing_output(destination_path) + if expected_root_identity is not None and _existing_output_matches( + destination_path, payload + ): + return temp_handle: _OwnedHandle | None = None temp_path: Path | None = None diff --git a/plugins/codex-security/tests/test_finalize_scan_contract.py b/plugins/codex-security/tests/test_finalize_scan_contract.py index 1be7029e4..bb5dd5010 100644 --- a/plugins/codex-security/tests/test_finalize_scan_contract.py +++ b/plugins/codex-security/tests/test_finalize_scan_contract.py @@ -1670,7 +1670,13 @@ def test_finalize_uses_windows_backend_without_dir_fd(self) -> None: def open_read_fd(scan_dir: Path, relative_path: str, _context: str) -> int: return os.open(scan_dir / relative_path, os.O_RDONLY) - def atomic_write(scan_dir: Path, relative_path: str, payload: bytes) -> None: + def atomic_write( + scan_dir: Path, + relative_path: str, + payload: bytes, + *, + expected_root_identity: tuple[int, int] | None = None, + ) -> None: path = scan_dir / relative_path path.parent.mkdir(parents=True, exist_ok=True) path.write_bytes(payload) diff --git a/plugins/codex-security/tests/test_windows_report_e2e.py b/plugins/codex-security/tests/test_windows_report_e2e.py index d46634615..dfb20af36 100644 --- a/plugins/codex-security/tests/test_windows_report_e2e.py +++ b/plugins/codex-security/tests/test_windows_report_e2e.py @@ -53,7 +53,13 @@ def test_workbench_completion_and_exports_use_windows_file_backend(tmp_path: Pat def open_read_fd(root: Path, relative_path: str, _context: str) -> int: return os.open(root / relative_path, os.O_RDONLY) - def atomic_write(root: Path, relative_path: str, payload: bytes) -> None: + def atomic_write( + root: Path, + relative_path: str, + payload: bytes, + *, + expected_root_identity: tuple[int, int] | None = None, + ) -> None: path = root / relative_path path.parent.mkdir(parents=True, exist_ok=True) path.write_bytes(payload) diff --git a/sdk/typescript/src/api.ts b/sdk/typescript/src/api.ts index 426452c9b..c85f1a060 100644 --- a/sdk/typescript/src/api.ts +++ b/sdk/typescript/src/api.ts @@ -6,7 +6,6 @@ import { mkdir, readFile, realpath, - rename, rm, writeFile, } from "node:fs/promises"; @@ -57,6 +56,7 @@ import { } from "./cost.js"; import { loadContract, + readScanFile, requireScanFile, type ScanExpectation, } from "./contract.js"; @@ -118,6 +118,7 @@ import { preserveCodexSecurityPluginRegistration, pluginExecutionEnvironment, planOutputArchive, + prepareScanArtifactRestorer, prepareOutputDir, preparePersistentOutputRoot, requireModelSafeOutputDir, @@ -130,6 +131,7 @@ import { type CodexCommand, type PluginInstall, type ProcessEnvironment, + type ScanArtifactRestorer, type WorkbenchCommandOptions, validateOutputDir, } from "./runtime.js"; @@ -368,6 +370,7 @@ interface ClientDependencies { ) => Promise; resolvePluginPython?: typeof resolvePluginPython; prepareOutputDir?: typeof prepareOutputDir; + prepareScanArtifactRestorer?: typeof prepareScanArtifactRestorer; repositoryRevision?: typeof repositoryRevision; resolveCodexCommand?: () => CodexCommand; runWorkbench?: typeof runWorkbench; @@ -642,6 +645,9 @@ export class CodexSecurity { id: string; options: WorkbenchCommandOptions; } | null = null; + const prepareArtifactRestorer = + this.#dependencies.prepareScanArtifactRestorer ?? + prepareScanArtifactRestorer; const workbench = this.#dependencies.runWorkbench ?? runWorkbench; try { const checkOpen = (): void => { @@ -1359,13 +1365,15 @@ export class CodexSecurity { ]), ].map(async (name) => ({ name, - contents: await readFile( - await requireScanFile(scanDir, name, name, signal), - { signal }, - ), + contents: await readScanFile(scanDir, name, name, signal), })), ); + let artifactRestorer: ScanArtifactRestorer | null = null; try { + artifactRestorer = await prepareArtifactRestorer( + workbenchOptions, + scanDir, + ); await runScanEvents({ thread, events: (await followUp()).events, @@ -1381,28 +1389,20 @@ export class CodexSecurity { checkOpen(); } catch (error) { if (signal.aborted || this.#closed) throw error; - for (const artifact of completedArtifacts) { - const path = join(scanDir, artifact.name); - const current = await readFile(path, { signal }).catch( - (readError: NodeJS.ErrnoException) => { - if (readError.code !== "ENOENT") throw readError; - return null; - }, - ); - if (current?.equals(artifact.contents)) continue; - const temporary = join( - dirname(path), - `.${randomUUID()}.${basename(path)}.restore`, - ); - try { - await writeFile(temporary, artifact.contents, { - flag: "wx", - mode: 0o600, - signal, - }); - await rename(temporary, path); - } finally { - await rm(temporary, { force: true }); + if (artifactRestorer !== null) { + for (const artifact of completedArtifacts) { + try { + await artifactRestorer.restore( + artifact.name, + artifact.contents, + ); + } catch (cause) { + if (signal.aborted || this.#closed) throw cause; + throw new OutputDirectoryError( + "Cannot restore an artifact outside the scan directory.", + { cause }, + ); + } } } await collectResult( diff --git a/sdk/typescript/src/contract.ts b/sdk/typescript/src/contract.ts index 5db97542d..7f75b6c5d 100644 --- a/sdk/typescript/src/contract.ts +++ b/sdk/typescript/src/contract.ts @@ -537,6 +537,25 @@ export async function requireScanFile( ).path; } +export async function readScanFile( + scanDirectory: string, + relativePath: string, + context: string, + signal?: AbortSignal, +): Promise { + const file = await openCheckedScanFile( + scanDirectory, + relativePath, + context, + signal, + ); + try { + return await file.readFile({ signal }); + } finally { + await file.close(); + } +} + async function requireCheckedScanFile( scanDirectory: string, relativePath: string, diff --git a/sdk/typescript/src/runtime.ts b/sdk/typescript/src/runtime.ts index bbbfd394f..cdd0e2c11 100644 --- a/sdk/typescript/src/runtime.ts +++ b/sdk/typescript/src/runtime.ts @@ -81,6 +81,42 @@ const CREDENTIAL_LOCK_POLL_MILLISECONDS = 25; const INCOMPLETE_CREDENTIAL_LOCK_MILLISECONDS = 30_000; const MAX_PROCESS_ID = 2_147_483_647; const MAX_WINDOWS_CREDENTIAL_ACL_STDERR = 64 * 1024; +const PLUGIN_HELPER_SECRET_ENVIRONMENT_VARIABLES = new Set([ + "OPENAI_API_KEY", + "CODEX_API_KEY", + "OPENROUTER_API_KEY", + "FIREWORKS_API_KEY", +]); +const PREPARE_SCAN_ARTIFACT_RESTORER_PROGRAM = ` +from pathlib import Path +from runpy import run_path +import json +import sys + +module = run_path(sys.argv[1]) +canonical_path, root_identity = module["scan_root_identity"](Path(sys.argv[2])) +print(json.dumps({ + "canonicalPath": str(canonical_path), + "dev": str(root_identity[0]), + "ino": str(root_identity[1]), +}, ensure_ascii=False)) +`.trim(); +const RESTORE_SCAN_ARTIFACT_PROGRAM = ` +from pathlib import Path +from runpy import run_path +import sys + +module = run_path(sys.argv[1]) +try: + module["write_scan_local_bytes"]( + Path(sys.argv[2]), + sys.argv[3], + sys.stdin.buffer.read(), + expected_root_identity=(int(sys.argv[4]), int(sys.argv[5])), + ) +except (module["ContractError"], OSError) as error: + raise SystemExit(str(error)) +`.trim(); const WINDOWS_CREDENTIAL_ACL_COMPLETE_PREFIX = "CODEX_SECURITY_ACL_COMPLETE:"; const WINDOWS_CREDENTIAL_DESCENDANTS_CHANGED_EXIT_CODE = 2; @@ -123,6 +159,10 @@ export interface WorkbenchCommandOptions { failureMessage?: string; } +export interface ScanArtifactRestorer { + restore(relativePath: string, contents: Uint8Array): Promise; +} + function environmentValue( environment: ProcessEnvironment, requested: string, @@ -1490,15 +1530,6 @@ export async function runWorkbench( ): Promise { let stdout: string; try { - const environment = Object.fromEntries( - Object.entries(options.environment).filter( - ([name]) => - name.toUpperCase() !== "OPENAI_API_KEY" && - name.toUpperCase() !== "CODEX_API_KEY" && - name.toUpperCase() !== "OPENROUTER_API_KEY" && - name.toUpperCase() !== "FIREWORKS_API_KEY", - ), - ); const result = await runCodexCommand( { command: options.python }, [ @@ -1509,7 +1540,7 @@ export async function runWorkbench( join(options.pluginRoot, "scripts", "workbench_db.py"), ...args, ], - pythonUtf8Environment(environment), + pluginHelperEnvironment(options.environment), input, options.signal, ); @@ -1638,6 +1669,107 @@ export async function validateOutputDir( } } +export async function prepareScanArtifactRestorer( + options: WorkbenchCommandOptions, + scanDirectory: string, +): Promise { + let helperPath: string; + let canonicalPath: string; + let dev: string; + let ino: string; + try { + // Recovery uses the SDK-owned writer, even if the scan selected a custom plugin. + helperPath = join( + await bundledPluginRoot(), + "scripts", + "finalize_scan_contract.py", + ); + const result = await runCodexCommand( + { command: options.python }, + [ + "-I", + "-X", + "utf8", + "-B", + "-c", + PREPARE_SCAN_ARTIFACT_RESTORER_PROGRAM, + helperPath, + scanDirectory, + ], + pluginHelperEnvironment(options.environment), + undefined, + options.signal, + ); + if (!result.success) { + throw new Error( + result.stderr.trim() || + result.stdout.trim() || + `Artifact restoration setup exited with status ${result.exitCode}.`, + ); + } + const prepared: unknown = JSON.parse(result.stdout); + if ( + !isRecord(prepared) || + typeof prepared["canonicalPath"] !== "string" || + prepared["canonicalPath"].length === 0 || + typeof prepared["dev"] !== "string" || + !/^(?:0|[1-9]\d*)$/u.test(prepared["dev"]) || + typeof prepared["ino"] !== "string" || + !/^(?:0|[1-9]\d*)$/u.test(prepared["ino"]) + ) { + throw new Error("Artifact restoration setup returned invalid output."); + } + canonicalPath = prepared["canonicalPath"]; + dev = prepared["dev"]; + ino = prepared["ino"]; + } catch (error) { + if (options.signal?.aborted) throw error; + throw new OutputDirectoryError( + "Could not securely prepare completed scan artifact restoration.", + { cause: error }, + ); + } + + return { + async restore(relativePath, contents) { + try { + const result = await runCodexCommand( + { command: options.python }, + [ + "-I", + "-X", + "utf8", + "-B", + "-c", + RESTORE_SCAN_ARTIFACT_PROGRAM, + helperPath, + canonicalPath, + relativePath, + dev, + ino, + ], + pluginHelperEnvironment(options.environment), + contents, + options.signal, + ); + if (!result.success) { + throw new Error( + result.stderr.trim() || + result.stdout.trim() || + `Artifact restoration exited with status ${result.exitCode}.`, + ); + } + } catch (error) { + if (options.signal?.aborted) throw error; + throw new OutputDirectoryError( + "Could not safely restore a completed scan artifact.", + { cause: error }, + ); + } + }, + }; +} + export async function planOutputArchive( outputDirectory: string | null, ): Promise { @@ -2469,6 +2601,19 @@ export function pythonUtf8Environment( return normalized; } +function pluginHelperEnvironment( + environment: ProcessEnvironment, +): ProcessEnvironment { + return pythonUtf8Environment( + Object.fromEntries( + Object.entries(environment).filter( + ([name]) => + !PLUGIN_HELPER_SECRET_ENVIRONMENT_VARIABLES.has(name.toUpperCase()), + ), + ), + ); +} + export async function cleanupSdkDirectory(path: string): Promise { await rm(path, { recursive: true, force: true }); } @@ -2477,7 +2622,7 @@ export async function runCodexCommand( command: CodexCommand, args: readonly string[], environment: ProcessEnvironment, - input?: string, + input?: string | Uint8Array, signal?: AbortSignal, ): Promise { const child = spawn(command.command, [...args], { diff --git a/sdk/typescript/src/version.ts b/sdk/typescript/src/version.ts index 95861c52e..0fcfbbb6d 100644 --- a/sdk/typescript/src/version.ts +++ b/sdk/typescript/src/version.ts @@ -9,7 +9,7 @@ const PACKAGE_VERSIONS = packageVersions( export const VERSION = PACKAGE_VERSIONS.package; export const CODEX_SDK_VERSION = PACKAGE_VERSIONS.sdk; export const CODEX_EXECUTABLE_VERSION = PACKAGE_VERSIONS.executable; -export const BUNDLED_PLUGIN_VERSION = "0.1.60" as const; +export const BUNDLED_PLUGIN_VERSION = "0.1.79" as const; const PACKAGE_NAME = "@openai/codex-security"; diff --git a/sdk/typescript/tests-ts/api-post-scan.test.ts b/sdk/typescript/tests-ts/api-post-scan.test.ts index 1c17fe13b..6282fe358 100644 --- a/sdk/typescript/tests-ts/api-post-scan.test.ts +++ b/sdk/typescript/tests-ts/api-post-scan.test.ts @@ -1,8 +1,24 @@ import { createHash } from "node:crypto"; -import { mkdir, readFile, rm, writeFile } from "node:fs/promises"; +import { + chmod, + cp, + mkdir, + open, + readFile, + rename, + rm, + stat, + symlink, + writeFile, +} from "node:fs/promises"; import { dirname, join } from "node:path"; import type { ThreadEvent } from "@openai/codex-sdk"; import { afterEach, describe, expect, test } from "bun:test"; +import { + prepareScanArtifactRestorer, + type ScanArtifactRestorer, +} from "../src/runtime.js"; +import { PLUGIN_ROOT } from "./plugin-root.js"; import { TestClient } from "./support/api-client.js"; import { completedEvents, @@ -15,81 +31,488 @@ const { cleanup, copyCompletedScan, temporaryDirectory } = afterEach(cleanup); +interface FailedPostScanContext { + artifactPath: string; + outside: string; + scanDir: string; +} + +interface FailedPostScanScenario { + artifact: string; + initialContents?: string | Uint8Array; + selectedPluginFinalizer?: string; + mutate(context: FailedPostScanContext): Promise; + wrapRestorer?( + restorer: ScanArtifactRestorer, + context: FailedPostScanContext, + ): ScanArtifactRestorer; +} + +async function* failedEvents(): AsyncGenerator { + yield { + type: "turn.failed", + error: { message: "Could not draft fixes." }, + }; +} + +async function startFailedPostScan(scenario: FailedPostScanScenario) { + const root = await temporaryDirectory(); + const repository = join(root, "repository"); + const codexHome = join(root, "codex-home"); + const scanDir = join(root, "scan"); + const outside = join(root, "outside"); + const artifactPath = join(scanDir, scenario.artifact); + const context = { artifactPath, outside, scanDir }; + const python = Bun.which("python3") ?? Bun.which("python"); + expect(python).not.toBeNull(); + await mkdir(repository); + await mkdir(codexHome); + await mkdir(scanDir, { mode: 0o700 }); + const runtime = preparedRuntime(codexHome); + if (scenario.selectedPluginFinalizer !== undefined) { + const selectedPluginRoot = join(root, "selected-plugin"); + await cp(PLUGIN_ROOT, selectedPluginRoot, { recursive: true }); + await writeFile( + join(selectedPluginRoot, "scripts", "finalize_scan_contract.py"), + scenario.selectedPluginFinalizer, + ); + runtime.plugin = { + ...runtime.plugin, + pluginRoot: selectedPluginRoot, + installedRoot: selectedPluginRoot, + }; + } + let turns = 0; + let original = Buffer.alloc(0); + const client = new TestClient( + {}, + { + environment: {}, + prepareRuntime: async () => runtime, + resolvePluginPython: async () => python!, + prepareOutputDir: async () => scanDir, + repositoryRevision: async () => "deadbeef", + prepareScanArtifactRestorer: async (...args) => { + const restorer = await prepareScanArtifactRestorer(...args); + return scenario.wrapRestorer?.(restorer, context) ?? restorer; + }, + createCodex: () => ({ + startThread: () => ({ + id: "thread-1", + async runStreamed() { + turns += 1; + if (turns === 1) { + await copyCompletedScan(root); + if (scenario.initialContents !== undefined) { + await mkdir(dirname(artifactPath), { recursive: true }); + await writeFile(artifactPath, scenario.initialContents); + const manifestPath = join(scanDir, "scan-manifest.json"); + const manifest = JSON.parse( + await readFile(manifestPath, "utf8"), + ); + manifest.scan.artifacts.push({ + path: scenario.artifact, + sha256: createHash("sha256") + .update(await readFile(artifactPath)) + .digest("hex"), + mediaType: scenario.artifact.endsWith(".bin") + ? "application/octet-stream" + : "application/json", + }); + await writeFile(manifestPath, JSON.stringify(manifest)); + } + original = await readFile(artifactPath); + return { events: completedEvents() }; + } + await scenario.mutate(context); + return { events: failedEvents() }; + }, + }), + }), + }, + ); + const scan = client.run(repository, { + postScanPrompt: "Draft confirmed fixes.", + }); + return { + client, + scan, + scanDir, + artifactPath, + outside, + get turns() { + return turns; + }, + get original() { + return original; + }, + }; +} + +const ordinaryRestorationCases: ReadonlyArray< + readonly [string, FailedPostScanScenario] +> = [ + [ + "missing report", + { artifact: "report.md", mutate: ({ artifactPath }) => rm(artifactPath) }, + ], + [ + "partial report", + { + artifact: "report.md", + mutate: async ({ artifactPath }) => { + await writeFile(artifactPath, "# Incomplete draft\n"); + }, + }, + ], + [ + "replaced report", + { + artifact: "report.md", + mutate: async ({ artifactPath }) => { + await rm(artifactPath); + await writeFile(artifactPath, "# Replacement\n"); + }, + }, + ], + [ + "invalid findings", + { + artifact: "findings.json", + mutate: async ({ artifactPath }) => { + await writeFile(artifactPath, "{invalid"); + }, + }, + ], + [ + "sealed nested artifact", + { + artifact: "artifacts/worker.json", + initialContents: '{"complete":true}\n', + mutate: async ({ artifactPath }) => { + await writeFile(artifactPath, '{"partial":true}'); + }, + }, + ], + [ + "binary artifact", + { + artifact: "artifacts/worker.bin", + initialContents: Buffer.from([0, 255, 10, 1]), + mutate: async ({ artifactPath }) => { + await writeFile(artifactPath, Buffer.from([9, 0, 8])); + }, + }, + ], + [ + "selected custom plugin", + { + artifact: "report.md", + selectedPluginFinalizer: + "raise RuntimeError('selected plugin helper must not run')\n", + mutate: ({ artifactPath }) => + writeFile(artifactPath, "# Incomplete draft\n"), + }, + ], + [ + "nested artifact with a missing parent", + { + artifact: "artifacts/worker.json", + initialContents: '{"complete":true}\n', + mutate: ({ artifactPath }) => + rm(dirname(artifactPath), { recursive: true }), + }, + ], +]; + describe("completed scan follow-up instructions", () => { - test.each([ - ["missing report", "report.md", undefined], - ["partial report", "report.md", "# Incomplete draft\n"], - ["invalid findings", "findings.json", "{invalid"], - ["sealed nested artifact", "artifacts/worker.json", '{"partial":true}'], - ] as const)( - "restores completed scan artifacts damaged by post-scan instructions: %s", - async (_scenario, artifact, replacement) => { + test.each(ordinaryRestorationCases)( + "restores completed scan artifacts after failed post-scan instructions: %s", + async (_name, scenario) => { + const fixture = await startFailedPostScan(scenario); + expect(await fixture.scan).toMatchObject({ scanDir: fixture.scanDir }); + expect(fixture.turns).toBe(2); + expect(await readFile(fixture.artifactPath)).toEqual(fixture.original); + await fixture.client.close(); + }, + ); + + test("does not rewrite artifacts unchanged by a failed follow-up", async () => { + let before: { dev: number; ino: number; mtimeMs: number } | null = null; + const fixture = await startFailedPostScan({ + artifact: "report.md", + mutate: async ({ artifactPath }) => { + const metadata = await stat(artifactPath); + before = { + dev: Number(metadata.dev), + ino: Number(metadata.ino), + mtimeMs: Number(metadata.mtimeMs), + }; + }, + }); + + expect(await fixture.scan).toMatchObject({ scanDir: fixture.scanDir }); + const after = await stat(fixture.artifactPath); + expect(before).not.toBeNull(); + expect(Number(after.dev)).toBe(before!.dev); + expect(Number(after.ino)).toBe(before!.ino); + expect(Number(after.mtimeMs)).toBe(before!.mtimeMs); + await fixture.client.close(); + }); + + test.skipIf(process.platform === "win32")( + "ordinary identical writes retain private replacement semantics", + async () => { const root = await temporaryDirectory(); - const repository = join(root, "repository"); - const codexHome = join(root, "codex-home"); const scanDir = join(root, "scan"); - await mkdir(repository); - await mkdir(codexHome); + const artifactPath = join(scanDir, "artifact.bin"); + const payload = Buffer.from("unchanged\n"); + const python = Bun.which("python3") ?? Bun.which("python"); + expect(python).not.toBeNull(); await mkdir(scanDir, { mode: 0o700 }); - let turns = 0; - let original = Buffer.alloc(0); - const client = new TestClient( - {}, - { - environment: {}, - prepareRuntime: async () => preparedRuntime(codexHome), - resolvePluginPython: async () => "/managed/python", - prepareOutputDir: async () => scanDir, - repositoryRevision: async () => "deadbeef", - createCodex: () => ({ - startThread: () => ({ - id: "thread-1", - async runStreamed() { - turns += 1; - if (turns === 1) { - await copyCompletedScan(root); - if (artifact.startsWith("artifacts/")) { - const artifactPath = join(scanDir, artifact); - await mkdir(dirname(artifactPath), { recursive: true }); - await writeFile(artifactPath, '{"complete":true}\n'); - const manifestPath = join(scanDir, "scan-manifest.json"); - const manifest = JSON.parse( - await readFile(manifestPath, "utf8"), - ); - manifest.scan.artifacts.push({ - path: artifact, - sha256: createHash("sha256") - .update(await readFile(artifactPath)) - .digest("hex"), - mediaType: "application/json", - }); - await writeFile(manifestPath, JSON.stringify(manifest)); - } - original = await readFile(join(scanDir, artifact)); - return { events: completedEvents() }; - } - const artifactPath = join(scanDir, artifact); - if (replacement === undefined) await rm(artifactPath); - else await writeFile(artifactPath, replacement); - async function* failedEvents(): AsyncGenerator { - yield { - type: "turn.failed", - error: { message: "Could not draft fixes." }, - }; - } - return { events: failedEvents() }; - }, - }), - }), - }, - ); + await writeFile(artifactPath, payload); + await chmod(artifactPath, 0o644); + const before = await stat(artifactPath); + const script = [ + "from pathlib import Path", + "from runpy import run_path", + "import sys", + "module = run_path(sys.argv[1])", + "scan_dir = Path(sys.argv[2])", + "module['write_scan_local_bytes'](scan_dir, 'artifact.bin', b'unchanged\\n')", + ].join("\n"); + const execution = Bun.spawnSync([ + python!, + "-I", + "-B", + "-c", + script, + join(PLUGIN_ROOT, "scripts", "finalize_scan_contract.py"), + scanDir, + ]); - const result = await client.run(repository, { - postScanPrompt: "Draft confirmed fixes.", + expect( + execution.exitCode, + new TextDecoder().decode(execution.stderr), + ).toBe(0); + const after = await stat(artifactPath); + expect(after.mode & 0o777).toBe(0o600); + expect(after.ino).not.toBe(before.ino); + expect(await readFile(artifactPath)).toEqual(payload); + }, + ); + + test.skipIf(process.platform !== "linux")( + "replaces a large sparse artifact within bounded comparison memory", + async () => { + const root = await temporaryDirectory(); + const scanDir = join(root, "scan"); + const artifactPath = join(scanDir, "artifact.bin"); + const artifactSize = 32 * 1024 * 1024; + const python = Bun.which("python3") ?? Bun.which("python"); + expect(python).not.toBeNull(); + await mkdir(scanDir, { mode: 0o700 }); + const script = [ + "from pathlib import Path", + "from runpy import run_path", + "import os", + "import resource", + "import sys", + "module = run_path(sys.argv[1])", + "scan_dir = Path(sys.argv[2])", + "artifact = scan_dir / 'artifact.bin'", + "size = 32 * 1024 * 1024", + "payload = b'x' * size", + "with artifact.open('wb') as stream:", + " stream.truncate(size)", + "canonical, identity = module['scan_root_identity'](scan_dir)", + "pages = int(Path('/proc/self/statm').read_text().split()[0])", + "current_vms = pages * os.sysconf('SC_PAGE_SIZE')", + "_, hard_limit = resource.getrlimit(resource.RLIMIT_AS)", + "resource.setrlimit(resource.RLIMIT_AS, (current_vms + 8 * 1024 * 1024, hard_limit))", + "module['write_scan_local_bytes'](canonical, 'artifact.bin', payload, expected_root_identity=identity)", + ].join("\n"); + const execution = Bun.spawnSync([ + python!, + "-I", + "-B", + "-c", + script, + join(PLUGIN_ROOT, "scripts", "finalize_scan_contract.py"), + scanDir, + ]); + + expect( + execution.exitCode, + new TextDecoder().decode(execution.stderr), + ).toBe(0); + expect((await stat(artifactPath)).size).toBe(artifactSize); + const artifact = await open(artifactPath, "r"); + try { + const first = Buffer.alloc(1); + const last = Buffer.alloc(1); + await artifact.read(first, 0, 1, 0); + await artifact.read(last, 0, 1, artifactSize - 1); + expect(first).toEqual(Buffer.from("x")); + expect(last).toEqual(Buffer.from("x")); + } finally { + await artifact.close(); + } + }, + ); + + test.skipIf(process.platform === "win32")( + "restores a changed artifact that cannot be read for comparison", + async () => { + const fixture = await startFailedPostScan({ + artifact: "report.md", + mutate: async ({ artifactPath }) => { + await writeFile(artifactPath, "# Incomplete draft\n"); + await chmod(artifactPath, 0); + }, }); - expect(result).toMatchObject({ scanDir }); - expect(await readFile(join(scanDir, artifact))).toEqual(original); - await client.close(); + + expect(await fixture.scan).toMatchObject({ scanDir: fixture.scanDir }); + expect(await readFile(fixture.artifactPath)).toEqual(fixture.original); + await fixture.client.close(); + }, + ); + + test("rejects a replaced artifact parent without writing through it", async () => { + const fixture = await startFailedPostScan({ + artifact: "artifacts/worker.json", + initialContents: '{"complete":true}\n', + mutate: async ({ artifactPath, outside }) => { + await mkdir(outside); + await writeFile(join(outside, "worker.json"), "untouched\n"); + await rm(dirname(artifactPath), { recursive: true }); + await symlink( + outside, + dirname(artifactPath), + process.platform === "win32" ? "junction" : "dir", + ); + }, + }); + + await expect(fixture.scan).rejects.toThrow("scan directory"); + expect(await readFile(join(fixture.outside, "worker.json"), "utf8")).toBe( + "untouched\n", + ); + await fixture.client.close(); + }); + + test("rejects an artifact parent swapped immediately before the bound write", async () => { + let swapped = false; + const artifact = "artifacts/worker.json"; + const fixture = await startFailedPostScan({ + artifact, + initialContents: '{"complete":true}\n', + mutate: async ({ artifactPath, outside }) => { + await mkdir(outside); + await writeFile(join(outside, "worker.json"), "untouched\n"); + await writeFile(artifactPath, '{"partial":true}'); + }, + wrapRestorer: (restorer, { outside, scanDir }) => ({ + ...restorer, + async restore(relativePath, contents) { + if (!swapped && relativePath === artifact) { + const parent = dirname(join(scanDir, relativePath)); + await rename(parent, `${parent}.original`); + await symlink( + outside, + parent, + process.platform === "win32" ? "junction" : "dir", + ); + swapped = true; + } + await restorer.restore(relativePath, contents); + }, + }), + }); + + await expect(fixture.scan).rejects.toThrow("scan directory"); + expect(await readFile(join(fixture.outside, "worker.json"), "utf8")).toBe( + "untouched\n", + ); + await fixture.client.close(); + }); + + test("rejects a scan root replaced after restoration setup", async () => { + const fixture = await startFailedPostScan({ + artifact: "report.md", + mutate: async ({ scanDir }) => { + await rename(scanDir, `${scanDir}.original`); + await mkdir(scanDir, { mode: 0o700 }); + await writeFile(join(scanDir, "scan-manifest.json"), "untouched\n"); + }, + }); + + await expect(fixture.scan).rejects.toThrow("scan directory"); + expect( + await readFile(join(fixture.scanDir, "scan-manifest.json"), "utf8"), + ).toBe("untouched\n"); + await fixture.client.close(); + }); + + test.skipIf(process.platform === "win32")( + "keeps the final rename bound to the validated parent when its path is replaced", + async () => { + const root = await temporaryDirectory(); + const scanDir = join(root, "scan"); + const parent = join(scanDir, "artifacts"); + const movedParent = join(root, "moved-artifacts"); + const outside = join(root, "outside"); + const python = Bun.which("python3") ?? Bun.which("python"); + expect(python).not.toBeNull(); + await mkdir(scanDir, { mode: 0o700 }); + await mkdir(parent); + await mkdir(outside); + await writeFile(join(parent, "worker.bin"), Buffer.from([1])); + await writeFile(join(outside, "worker.bin"), "untouched\n"); + const script = [ + "from pathlib import Path", + "from runpy import run_path", + "import sys", + "module = run_path(sys.argv[1])", + "scan_dir = Path(sys.argv[2])", + "parent = scan_dir / 'artifacts'", + "moved_parent = Path(sys.argv[4])", + "outside = Path(sys.argv[3])", + "canonical, identity = module['scan_root_identity'](scan_dir)", + "original_replace = module['os'].replace", + "swapped = False", + "def replace(source, destination, *, src_dir_fd=None, dst_dir_fd=None):", + " global swapped", + " if not swapped:", + " parent.rename(moved_parent)", + " parent.symlink_to(outside, target_is_directory=True)", + " swapped = True", + " return original_replace(source, destination, src_dir_fd=src_dir_fd, dst_dir_fd=dst_dir_fd)", + "module['os'].replace = replace", + "module['write_scan_local_bytes'](canonical, 'artifacts/worker.bin', bytes([0, 255, 10, 1]), expected_root_identity=identity)", + ].join("\n"); + const execution = Bun.spawnSync([ + python!, + "-I", + "-B", + "-c", + script, + join(PLUGIN_ROOT, "scripts", "finalize_scan_contract.py"), + scanDir, + outside, + movedParent, + ]); + + expect( + execution.exitCode, + new TextDecoder().decode(execution.stderr), + ).toBe(0); + expect(await readFile(join(outside, "worker.bin"), "utf8")).toBe( + "untouched\n", + ); + expect(await readFile(join(movedParent, "worker.bin"))).toEqual( + Buffer.from([0, 255, 10, 1]), + ); }, ); }); diff --git a/sdk/typescript/tests-ts/api.test.ts b/sdk/typescript/tests-ts/api.test.ts index 74836e58d..98ee9ac75 100644 --- a/sdk/typescript/tests-ts/api.test.ts +++ b/sdk/typescript/tests-ts/api.test.ts @@ -3753,75 +3753,92 @@ describe("CodexSecurity orchestration", () => { await client.close(); }); - test("warns about post-scan failures without failing a completed scan", async () => { - const root = await temporaryDirectory(); - const repository = join(root, "repository"); - const codexHome = join(root, "codex-home"); - const scanDir = join(root, "scan"); - await mkdir(repository); - await mkdir(codexHome); - await mkdir(scanDir, { mode: 0o700 }); - const commands: Array = []; - const warnings: string[] = []; - let turns = 0; + test.each([ + ["the follow-up turn", false, "Could not draft fixes."], + ["artifact restoration setup", true, "restoration setup failed"], + ] as const)( + "warns when %s fails without failing a completed scan", + async (_scenario, setupFails, failureMessage) => { + const root = await temporaryDirectory(); + const repository = join(root, "repository"); + const codexHome = join(root, "codex-home"); + const scanDir = join(root, "scan"); + await mkdir(repository); + await mkdir(codexHome); + await mkdir(scanDir, { mode: 0o700 }); + const commands: Array = []; + const warnings: string[] = []; + let turns = 0; - const client = new TestClient( - {}, - { - environment: {}, - prepareRuntime: async () => preparedRuntime(codexHome), - resolvePluginPython: async () => "/managed/python", - prepareOutputDir: async () => scanDir, - repositoryRevision: async () => "deadbeef", - runWorkbench: async ( - _options: unknown, - args: readonly string[], - input?: string, - ): Promise => { - commands.push(args); - return mockWorkbench(args, input); - }, - createCodex: () => ({ - startThread: () => ({ - id: "thread-1", - async runStreamed() { - turns += 1; - if (turns === 1) { - await copyCompletedScan(root); - return { events: completedEvents() }; - } - async function* failedEvents(): AsyncGenerator { - yield { - type: "turn.failed", - error: { message: "Could not draft fixes." }, - }; + const client = new TestClient( + {}, + { + environment: {}, + prepareRuntime: async () => preparedRuntime(codexHome), + resolvePluginPython: async () => "/managed/python", + prepareOutputDir: async () => scanDir, + repositoryRevision: async () => "deadbeef", + ...(setupFails + ? { + prepareScanArtifactRestorer: async () => { + throw new Error(failureMessage); + }, } - return { events: failedEvents() }; - }, + : {}), + runWorkbench: async ( + _options: unknown, + args: readonly string[], + input?: string, + ): Promise => { + commands.push(args); + return mockWorkbench(args, input); + }, + createCodex: () => ({ + startThread: () => ({ + id: "thread-1", + async runStreamed() { + turns += 1; + if (turns === 1) { + await copyCompletedScan(root); + return { events: completedEvents() }; + } + if (setupFails) { + throw new Error("post-scan turn started after setup failed"); + } + async function* failedEvents(): AsyncGenerator { + yield { + type: "turn.failed", + error: { message: "Could not draft fixes." }, + }; + } + return { events: failedEvents() }; + }, + }), }), - }), - }, - ); + }, + ); - await expect( - client.run(repository, { - postScanPrompt: "Draft confirmed fixes.", - onWarning: (warning) => warnings.push(warning), - }), - ).resolves.toMatchObject({ scanDir }); - expect(warnings).toEqual([ - "Could not run post-scan instructions: Could not draft fixes.", - ]); - expect(commands.map((command) => command[0])).toEqual([ - "register-cli-scan", - "get-scan-feedback", - "set-scan-thread", - "prepare-scan-completion", - "complete-scan", - "list-global-findings", - ]); - await client.close(); - }); + await expect( + client.run(repository, { + postScanPrompt: "Draft confirmed fixes.", + onWarning: (warning) => warnings.push(warning), + }), + ).resolves.toMatchObject({ scanDir }); + expect(warnings).toEqual([ + `Could not run post-scan instructions: ${failureMessage}`, + ]); + expect(turns).toBe(setupFails ? 1 : 2); + expect(commands.map((command) => command[0])).toEqual([ + "register-cli-scan", + "get-scan-feedback", + "set-scan-thread", + "prepare-scan-completion", + "complete-scan", + "list-global-findings", + ]); + await client.close(); + }, + ); test.each([ ["partial coverage", "partial", false], diff --git a/sdk/typescript/tests-ts/runtime.test.ts b/sdk/typescript/tests-ts/runtime.test.ts index 556dca3e6..3bc32a8bc 100644 --- a/sdk/typescript/tests-ts/runtime.test.ts +++ b/sdk/typescript/tests-ts/runtime.test.ts @@ -69,6 +69,7 @@ import { planOutputArchive, prepareCodexSecurityCredentialHome, preparePersistentOutputRoot, + prepareScanArtifactRestorer, preserveCodexSecurityPluginRegistration, requirePrivateCredentialHome, requirePrivateCredentialFile, @@ -1946,19 +1947,30 @@ describe("plugin runtime preparation", () => { ]); }); - test("upgrades a cached 0.1.37 plugin with the real bundled Codex executable", async () => { + test("upgrades the predecessor cache and restores with the SDK-owned helper", async () => { const root = await temporaryDirectory(); - const previous = await plugin(join(root, "previous"), "0.1.37"); + const previous = await plugin(join(root, "previous"), "0.1.60"); + // Keep the stale MCP configuration regression covered while upgrading the + // current predecessor cache to the generated bundle. await writeFile( join(previous, ".mcp.json"), JSON.stringify({ mcpServers: { "codex-security": { env_vars: [] } } }), ); + await copyFile( + join(PLUGIN_ROOT, "scripts", "workbench_target.py"), + join(previous, "scripts", "workbench_target.py"), + ); const home = join(root, "home"); + const unrelatedProject = join(root, "unrelated-project"); await mkdir(home, { mode: 0o700 }); + await mkdir(unrelatedProject); await writeFile( join(home, "config.toml"), - 'cli_auth_credentials_store = "file"\n\n[features]\nplugins = true\n', + 'cli_auth_credentials_store = "file"\n\n[features]\nplugins = true\n\n[projects.' + + JSON.stringify(unrelatedProject) + + ']\ntrust_level = "trusted"\n', ); + await writeFile(join(home, "unrelated-state"), "preserved\n"); const command = resolveCodexCommand(); const environment = { @@ -1977,9 +1989,18 @@ describe("plugin runtime preparation", () => { const credentials = await readFile(join(home, "auth.json"), "utf8"); const options = { codexCommand: command, environment }; - const first = await bootstrapPlugin(home, previous, options); - expect(first.version).toBe("0.1.37"); + const stale = await bootstrapPlugin(home, previous, options); const upgraded = await bootstrapPlugin(home, PLUGIN_ROOT, options); + + expect(stale.version).toBe("0.1.60"); + expect(upgraded.version).toBe(BUNDLED_PLUGIN_VERSION); + expect(upgraded.version).not.toBe(stale.version); + expect(upgraded.installedRoot).not.toBe(stale.installedRoot); + for (const script of ["workbench_target.py", "finalize_scan_contract.py"]) { + expect( + await readFile(join(upgraded.installedRoot, "scripts", script)), + ).toEqual(await readFile(join(PLUGIN_ROOT, "scripts", script))); + } const configuration = JSON.parse( await readFile(join(upgraded.installedRoot, ".mcp.json"), "utf8"), ) as { @@ -1987,13 +2008,17 @@ describe("plugin runtime preparation", () => { }; const server = configuration.mcpServers["codex-security"]; - expect(upgraded.version).toBe(BUNDLED_PLUGIN_VERSION); - expect(upgraded.version).not.toBe(first.version); - expect(upgraded.installedRoot).not.toBe(first.installedRoot); expect(server?.command).toBe("./scripts/launch_codex_security_mcp"); + expect(server?.env_vars).toContain("CODEX_SAFETY_IDENTIFIER"); expect(server?.env_vars).toContain("CODEX_MANAGED_PACKAGE_ROOT"); expect(server?.env_vars).toContain("CODEX_MCP_NODE_PATH"); expect(await readFile(join(home, "auth.json"), "utf8")).toBe(credentials); + expect(await readFile(join(home, "unrelated-state"), "utf8")).toBe( + "preserved\n", + ); + expect(await readFile(join(home, "config.toml"), "utf8")).toContain( + "[projects." + JSON.stringify(unrelatedProject) + "]", + ); expect( spawnSync(command.command, ["login", "status"], { env: environment, @@ -2001,6 +2026,27 @@ describe("plugin runtime preparation", () => { windowsHide: true, }).status, ).toBe(0); + + const scanDir = join(root, "scan"); + const artifact = "artifacts/worker.bin"; + const expected = Buffer.from([0, 255, 10, 1]); + await mkdir(join(scanDir, "artifacts"), { + recursive: true, + mode: 0o700, + }); + await writeFile(join(scanDir, artifact), expected); + const python = await resolvePluginPython({ environment }); + const restorer = await prepareScanArtifactRestorer( + { + python, + pluginRoot: upgraded.installedRoot, + environment, + }, + scanDir, + ); + await writeFile(join(scanDir, artifact), Buffer.from([9, 0, 8])); + await restorer.restore(artifact, expected); + expect(await readFile(join(scanDir, artifact))).toEqual(expected); }); test("resolves the exact npm Codex executable", () => { diff --git a/sdk/typescript/tests-ts/support/api-client.ts b/sdk/typescript/tests-ts/support/api-client.ts index 851a440cd..2195e73ae 100644 --- a/sdk/typescript/tests-ts/support/api-client.ts +++ b/sdk/typescript/tests-ts/support/api-client.ts @@ -68,6 +68,9 @@ export class TestClient extends CodexSecurity { throw new Error("Unexpected Codex invocation in test"); }, environment: {}, + prepareScanArtifactRestorer: async () => ({ + restore: async () => {}, + }), runWorkbench: async (_options, args, input) => mockWorkbench(args, input), ...dependencies,