diff --git a/README.md b/README.md index ac16e31ef..e00ebb16f 100644 --- a/README.md +++ b/README.md @@ -86,8 +86,9 @@ unset OPENAI_API_KEY CODEX_API_KEY ``` Scan history is stored in the Codex Security workbench state directory. If that -directory cannot be written, set `CODEX_SECURITY_STATE_DIR` to a writable -directory outside the repository. +directory cannot be written or has unsafe parent permissions, set +`CODEX_SECURITY_STATE_DIR` to a private directory with trusted parents outside +the repository. See [output and state directory permissions](sdk/typescript/README.md#output-and-state-directory-permissions). `findings list [repository]` shows open findings across a repository's scans and identifies findings not confirmed in its latest scan. diff --git a/sdk/typescript/README.md b/sdk/typescript/README.md index cd1d449b5..b5da73c57 100644 --- a/sdk/typescript/README.md +++ b/sdk/typescript/README.md @@ -331,7 +331,9 @@ reduction and result delivery, including at the 96-hour maximum. `bulk-scan --workers` controls how many repositories are scanned concurrently. On macOS/Linux, an existing output directory must be private to the current -user (`chmod 700`). +user (`chmod 700`) and have trusted parent directories. See [output and state +directory permissions](#output-and-state-directory-permissions) if a parent is +group- or world-writable. If the output directory already contains results, add `--archive-existing`. The CLI moves them to `.previous--` and starts the @@ -765,6 +767,50 @@ const directPublication = await publishScan("/path/to/completed-scan", { }); ``` +### Output and state directory permissions + +On macOS and Linux, scan output must be private to the current user. Every +parent directory must be owned by the current user or root. A group- or +world-writable parent is accepted only when it has the sticky bit. The state +directory itself must be private; the sticky-bit exception applies only to its +parents. History, sign-in, and publication operations use the same private-state +rule. State-directory aliases must have trusted link owners and trusted lexical +and resolved parents. New state directories are created privately; existing +directories are not automatically changed. + +On Windows, an existing state directory must use a protected access-control +list that grants access only to the current user, `SYSTEM`, and local +Administrators, and its parents must not let another identity replace it. New +state directories receive that private access-control list. Existing access +rules are inspected but not rewritten. + +An error that names a parent with mode `0775` refers to that parent, not just +the final output directory. Creating a `0700` child beneath it does not stop +another user from renaming or replacing that child. Choose a location with +trusted parents, or remove group- and world-write access from the named parent +only if you own it and it is safe to change. Do not change a shared home, +workspace, mount, or system directory merely to suppress the error. + +Use the setting for the location that failed: + +- For explicitly selected scan results, choose another `--output-dir` (SDK + `outputDir`) outside the scanned repository. +- For persistent history, default artifacts, and stored sign-in, choose a + private `CODEX_SECURITY_STATE_DIR` outside the repository. On macOS and Linux, + an existing state directory must be owned by you and private (`0700`). Change + its permissions only if it is your dedicated state directory and is safe to + change. On Windows, choose a directory with private access rules rather than + changing a shared parent. Selecting a new directory does not move existing + history, results, or credentials. +- For temporary runtime files, choose a suitable `TMPDIR` (`TEMP` on Windows). + A fresh private child under a trusted, sticky system temporary directory is + suitable for temporary work; it is not a replacement for persistent history. + +The CLI does not automatically change parent permissions or move an explicitly +selected directory. `scan --dry-run` checks the ancestry of an explicitly +selected output directory and the configured state root without creating scan +output or initializing the runtime. + ### Scan history and reruns `scans` or `scans list` lists scans for the current repository. Pass a repository diff --git a/sdk/typescript/_bundled_plugin/.codex-plugin/plugin.json b/sdk/typescript/_bundled_plugin/.codex-plugin/plugin.json index 76688fe96..93d0724aa 100644 --- a/sdk/typescript/_bundled_plugin/.codex-plugin/plugin.json +++ b/sdk/typescript/_bundled_plugin/.codex-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "codex-security", - "version": "0.1.22", + "version": "0.1.24", "description": "Codex Security workflows for security scans, analysis, and investigation.", "author": { "name": "OpenAI" diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_db.py b/sdk/typescript/_bundled_plugin/scripts/workbench_db.py index 33332a490..23e531028 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_db.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_db.py @@ -13,6 +13,7 @@ import re import sqlite3 import stat +import subprocess import sys import tempfile import time @@ -137,6 +138,33 @@ FINDING_WRITEUP_REPORT_PATH = re.compile(r"^findings/([a-z0-9][a-z0-9._-]*)/\1\.md$") SCAN_RECIPE_MAX_BYTES = 256 * 1024 +WINDOWS_SYSTEM_SID = "S-1-5-18" +WINDOWS_ADMINISTRATORS_SID = "S-1-5-32-544" +WINDOWS_TRUSTED_INSTALLER_SID = ( + "S-1-5-80-956008885-3418522649-1831038044-1853292631-2271478464" +) +WINDOWS_SID = re.compile(r"^S-1-(?:\d+-)*\d+$") +WINDOWS_ALLOW_ACE_TYPES = {0, 5, 9, 11} +WINDOWS_DENY_ACE_TYPES = {1, 6, 10, 12} +WINDOWS_ACE_OBJECT_INHERIT = 0x01 +WINDOWS_ACE_CONTAINER_INHERIT = 0x02 +WINDOWS_ACE_NO_PROPAGATE = 0x04 +WINDOWS_ACE_INHERIT_ONLY = 0x08 +WINDOWS_ACE_FLAGS = 0x1F +WINDOWS_DACL_PRESENT = 0x0004 +WINDOWS_DACL_PROTECTED = 0x1000 +WINDOWS_FULL_CONTROL = 0x1F01FF +WINDOWS_GENERIC_ALL = 0x10000000 +WINDOWS_REPLACE_DIRECTORY = 0x100D0040 +WINDOWS_WRITE_DIRECTORY = 0x500D0046 +WINDOWS_WORKBENCH_FILES = ( + "workbench.sqlite3", + "workbench.sqlite3-journal", + "workbench.sqlite3-shm", + "workbench.sqlite3-wal", +) +windows_acl_context: tuple[Path, Path, str, dict[str, str]] | None = None + def now() -> str: return datetime.now(timezone.utc).isoformat().replace("+00:00", "Z") @@ -148,23 +176,381 @@ def stale_claim_before(seconds: int = CLAIM_LEASE_SECONDS) -> str: ) -def state_dir() -> Path: +def requested_state_dir() -> str: state_dir = os.environ.get("CODEX_SECURITY_STATE_DIR") if state_dir: - return Path(state_dir).expanduser().resolve() - codex_home = Path(os.environ.get("CODEX_HOME", "~/.codex")).expanduser() - return (codex_home / "state" / "plugins" / "codex-security").resolve() + requested = state_dir + else: + codex_home = os.environ.get("CODEX_HOME", "~/.codex") + requested = os.path.join(codex_home, "state", "plugins", "codex-security") + requested = os.path.expanduser(requested) + if requested.startswith("~"): + raise RuntimeError("Could not determine home directory.") + return requested if os.path.isabs(requested) else os.path.join(os.getcwd(), requested) + + +def state_dir() -> Path: + return Path(requested_state_dir()).resolve() def database_path() -> Path: return state_dir() / "workbench.sqlite3" +def run_windows_state_acl_command( + command: Path, + arguments: list[str], + environment: dict[str, str], +) -> str: + try: + result = subprocess.run( + [str(command), *arguments], + capture_output=True, + check=False, + creationflags=getattr(subprocess, "CREATE_NO_WINDOW", 0), + encoding="utf-8", + errors="replace", + env=environment, + text=True, + ) + except OSError as exc: + raise RuntimeError(f"Could not run Windows state ACL tool: {command.name}.") from exc + if result.returncode != 0: + detail = result.stderr.strip() or result.stdout.strip() + raise RuntimeError( + f"Windows state ACL tool failed: {command.name}." + + (f" {detail}" if detail else "") + ) + return result.stdout + + +def windows_state_acl_context() -> tuple[Path, Path, str, dict[str, str]]: + global windows_acl_context + if windows_acl_context is not None: + return windows_acl_context + system_directory = Path(os.environ.get("SystemRoot", r"C:\Windows")) / "System32" + powershell = system_directory / "WindowsPowerShell" / "v1.0" / "powershell.exe" + environment = { + name: value + for name, value in os.environ.items() + if name.upper() != "PSMODULEPATH" + } + environment["PSModulePath"] = str( + system_directory / "WindowsPowerShell" / "v1.0" / "Modules" + ) + identity = run_windows_state_acl_command( + system_directory / "whoami.exe", + ["/user", "/fo", "csv", "/nh"], + environment, + ) + try: + sid = next(csv.reader([identity.strip()]))[-1] + except (IndexError, StopIteration) as exc: + raise RuntimeError("Unable to identify the current Windows user SID.") from exc + if WINDOWS_SID.fullmatch(sid) is None: + raise RuntimeError("Unable to identify the current Windows user SID.") + windows_acl_context = powershell, system_directory / "icacls.exe", sid, environment + return windows_acl_context + + +def windows_state_acl_records( + path: Path, + context: tuple[Path, Path, str, dict[str, str]], + *, + workbench_files: bool = False, +) -> list[dict[str, Any]]: + powershell, _, _, base_environment = context + script = [ + "$ErrorActionPreference = 'Stop'", + "function Write-CodexSecurityAcl {", + "param($kind, $path)", + "$sddl = Microsoft.PowerShell.Security\\Get-Acl -LiteralPath $path | Microsoft.PowerShell.Utility\\Select-Object -ExpandProperty Sddl", + "$raw = Microsoft.PowerShell.Utility\\ConvertFrom-SddlString -Sddl $sddl | Microsoft.PowerShell.Utility\\Select-Object -ExpandProperty RawDescriptor", + "$owner = $raw | Microsoft.PowerShell.Utility\\Select-Object -ExpandProperty Owner | Microsoft.PowerShell.Utility\\Select-Object -ExpandProperty Value", + "$rules = @($raw | Microsoft.PowerShell.Utility\\Select-Object -ExpandProperty DiscretionaryAcl | Microsoft.PowerShell.Core\\ForEach-Object {", + "$sid = $_ | Microsoft.PowerShell.Utility\\Select-Object -ExpandProperty SecurityIdentifier | Microsoft.PowerShell.Utility\\Select-Object -ExpandProperty Value", + "[pscustomobject]@{ type = [int]$_.AceType; flags = [int]$_.AceFlags; mask = [int64]$_.AccessMask; sid = $sid }", + "})", + "[pscustomobject]@{ kind = $kind; owner = $owner; control = [int]$raw.ControlFlags; rules = $rules } | Microsoft.PowerShell.Utility\\ConvertTo-Json -Compress -Depth 4", + "}", + "$path = $env:CODEX_SECURITY_STATE_ACL_PATH", + ] + if workbench_files: + names = ", ".join(f"'{name}'" for name in WINDOWS_WORKBENCH_FILES) + script.extend( + [ + "while ($true) { $parent = Microsoft.PowerShell.Management\\Split-Path -Path $path -Parent; if (-not $parent -or $parent -eq $path) { break }; Write-CodexSecurityAcl 'ancestor' $parent; $path = $parent }", + "Write-CodexSecurityAcl 'root' $env:CODEX_SECURITY_STATE_ACL_PATH", + f"foreach ($name in @({names})) {{ $child = Microsoft.PowerShell.Management\\Join-Path -Path $env:CODEX_SECURITY_STATE_ACL_PATH -ChildPath $name; if (Microsoft.PowerShell.Management\\Test-Path -LiteralPath $child) {{ try {{ Write-CodexSecurityAcl 'entry' $child }} catch {{ if ($_.FullyQualifiedErrorId -notlike 'GetAcl_PathNotFound*') {{ throw }} }} }} }}", + ] + ) + else: + script.append( + "while ($true) { Write-CodexSecurityAcl 'ancestor' $path; $parent = Microsoft.PowerShell.Management\\Split-Path -Path $path -Parent; if (-not $parent -or $parent -eq $path) { break }; $path = $parent }" + ) + environment = {**base_environment, "CODEX_SECURITY_STATE_ACL_PATH": str(path)} + output = run_windows_state_acl_command( + powershell, + ["-NoLogo", "-NoProfile", "-NonInteractive", "-Command", "\n".join(script)], + environment, + ) + records: list[dict[str, Any]] = [] + for line in output.splitlines(): + if not line.strip(): + continue + try: + record = json.loads(line) + except json.JSONDecodeError as exc: + raise RuntimeError("Windows state ACL inspection returned invalid data.") from exc + if not isinstance(record, dict): + raise RuntimeError("Windows state ACL inspection returned invalid data.") + records.append(record) + if not records: + raise RuntimeError("Windows state ACL could not be inspected.") + return records + + +def require_windows_state_acl( + record: dict[str, Any], + current_user_sid: str, + scope: str, +) -> None: + owner = record.get("owner") + control = record.get("control") + rules = record.get("rules") + if ( + not isinstance(owner, str) + or WINDOWS_SID.fullmatch(owner) is None + or not isinstance(control, int) + or not isinstance(rules, list) + or not rules + or control & WINDOWS_DACL_PRESENT == 0 + ): + raise RuntimeError("Windows state ACL is incomplete.") + + trusted = {current_user_sid, WINDOWS_SYSTEM_SID, WINDOWS_ADMINISTRATORS_SID} + if scope in {"ancestor", "creation-parent"}: + trusted.add(WINDOWS_TRUSTED_INSTALLER_SID) + if owner not in trusted: + raise RuntimeError(f"Windows state ACL owner is not trusted: {owner}.") + + grants_current_user = False + foreign_access = False + denied_access = False + for rule in rules: + if not isinstance(rule, dict) or set(rule) != {"type", "flags", "mask", "sid"}: + raise RuntimeError("Windows state ACL contains an invalid access rule.") + ace_type = rule["type"] + flags = rule["flags"] + mask = rule["mask"] + principal = rule["sid"] + if ( + not isinstance(ace_type, int) + or ace_type not in WINDOWS_ALLOW_ACE_TYPES | WINDOWS_DENY_ACE_TYPES + or not isinstance(flags, int) + or flags < 0 + or flags & ~WINDOWS_ACE_FLAGS + or not isinstance(mask, int) + or not isinstance(principal, str) + or WINDOWS_SID.fullmatch(principal) is None + ): + raise RuntimeError("Windows state ACL contains an unsupported access rule.") + mask &= 0xFFFFFFFF + if ace_type in WINDOWS_DENY_ACE_TYPES: + denied_access = True + continue + if principal not in trusted: + if scope in {"root", "entry"}: + foreign_access = True + elif flags & WINDOWS_ACE_INHERIT_ONLY == 0 and mask & WINDOWS_REPLACE_DIRECTORY: + foreign_access = True + elif ( + scope == "creation-parent" + and (flags & WINDOWS_ACE_INHERIT_ONLY == 0 or flags & WINDOWS_ACE_CONTAINER_INHERIT) + and mask & WINDOWS_WRITE_DIRECTORY + ): + foreign_access = True + continue + if ( + principal == current_user_sid + and ace_type == 0 + and flags & WINDOWS_ACE_INHERIT_ONLY == 0 + and ( + mask & WINDOWS_FULL_CONTROL == WINDOWS_FULL_CONTROL + or mask & WINDOWS_GENERIC_ALL + ) + ): + grants_current_user = scope != "root" or ( + flags & WINDOWS_ACE_OBJECT_INHERIT + and flags & WINDOWS_ACE_CONTAINER_INHERIT + and flags & WINDOWS_ACE_NO_PROPAGATE == 0 + ) + + if scope in {"ancestor", "creation-parent"}: + if foreign_access: + raise RuntimeError( + "Windows state ancestor allows another identity to replace or populate the directory." + ) + return + if scope == "root" and control & WINDOWS_DACL_PROTECTED == 0: + raise RuntimeError("Windows state ACL must be protected from inheritance.") + if not grants_current_user: + raise RuntimeError("Windows state ACL does not grant the current user access.") + if foreign_access: + raise RuntimeError("Windows state ACL grants access to another identity.") + if denied_access: + raise RuntimeError("Windows state ACL contains a deny rule.") + + +def existing_windows_state_ancestor(path: Path) -> Path: + current = Path(os.path.abspath(path)) + while True: + try: + current.lstat() + return current + except FileNotFoundError: + parent = current.parent + if parent == current: + raise + current = parent + + +def require_secure_windows_state_ancestry( + path: Path, + context: tuple[Path, Path, str, dict[str, str]], + *, + allow_state_creation: bool = False, +) -> None: + records = windows_state_acl_records(path, context) + if any(record.get("kind") != "ancestor" for record in records): + raise RuntimeError("Windows state ancestry could not be verified.") + for index, record in enumerate(records): + require_windows_state_acl( + record, + context[2], + "creation-parent" if allow_state_creation and index == 0 else "ancestor", + ) + + +def require_private_windows_state_path( + path: Path, + context: tuple[Path, Path, str, dict[str, str]], +) -> None: + records = windows_state_acl_records(path, context) + if any(record.get("kind") != "ancestor" for record in records): + raise RuntimeError("Windows state path ACL could not be verified.") + require_windows_state_acl(records[0], context[2], "entry") + for record in records[1:]: + require_windows_state_acl(record, context[2], "ancestor") + + +def require_regular_windows_state_file(path: Path) -> bool: + reparse_point = getattr(stat, "FILE_ATTRIBUTE_REPARSE_POINT", 0) + try: + metadata = path.lstat() + except FileNotFoundError: + return False + attributes = getattr(metadata, "st_file_attributes", 0) + if ( + not stat.S_ISREG(metadata.st_mode) + or stat.S_ISLNK(metadata.st_mode) + or attributes & reparse_point + ): + raise RuntimeError(f"Windows state file is not a regular file: {path.name}.") + return True + + +def require_regular_windows_workbench_files(root: Path) -> None: + for name in WINDOWS_WORKBENCH_FILES: + require_regular_windows_state_file(root / name) + + +def require_private_windows_state_directory( + root: Path, + context: tuple[Path, Path, str, dict[str, str]], +) -> None: + require_regular_windows_workbench_files(root) + records = windows_state_acl_records(root, context, workbench_files=True) + ancestors = [record for record in records if record.get("kind") == "ancestor"] + roots = [record for record in records if record.get("kind") == "root"] + entries = [record for record in records if record.get("kind") == "entry"] + if len(roots) != 1 or len(ancestors) + len(roots) + len(entries) != len(records): + raise RuntimeError("Windows state ACL snapshot could not be verified.") + for record in ancestors: + require_windows_state_acl(record, context[2], "ancestor") + require_windows_state_acl(roots[0], context[2], "root") + for record in entries: + require_windows_state_acl(record, context[2], "entry") + + +def install_private_windows_state_acl( + path: Path, + context: tuple[Path, Path, str, dict[str, str]], +) -> None: + _, icacls, current_user_sid, environment = context + run_windows_state_acl_command( + icacls, + [ + str(path), + "/inheritance:r", + "/grant:r", + f"*{current_user_sid}:(OI)(CI)F", + f"*{WINDOWS_SYSTEM_SID}:(OI)(CI)F", + f"*{WINDOWS_ADMINISTRATORS_SID}:(OI)(CI)F", + ], + environment, + ) + + +def require_secure_state_ancestry(path: str) -> None: + if os.name == "nt": + return + geteuid = getattr(os, "geteuid", None) + effective_uid = geteuid() if geteuid is not None else None + pending = [path] + checked: set[str] = set() + while pending: + current = pending.pop() + while True: + current = current.rstrip(os.sep) or os.sep + if current in checked: + break + checked.add(current) + parent = os.path.dirname(current) + try: + metadata = os.lstat(current) + except FileNotFoundError: + metadata = None + if metadata is not None: + if stat.S_ISLNK(metadata.st_mode): + require_trusted_output_owner(metadata, effective_uid) + canonical = os.path.realpath(current, strict=True) + target = os.readlink(current) + # Preserve dot segments until the filesystem resolves the target. + lexical_target = ( + target if os.path.isabs(target) else os.path.join(parent, target) + ) + pending.extend((canonical, lexical_target)) + else: + require_trusted_output_ancestor(current, effective_uid) + if parent == current: + break + current = parent + + @contextmanager def scan_completion_lock(scan_id: str) -> Any: lock_dir = state_dir() / "completion-locks" lock_dir.mkdir(parents=True, exist_ok=True) + windows_acl = windows_state_acl_context() if os.name == "nt" else None + if windows_acl is not None: + lock_dir = require_canonical_scan_directory(lock_dir) lock_path = lock_dir / f"{require_uuid(scan_id, 'scan-id')}.lock" + existing_windows_lock = False + if windows_acl is not None: + existing_windows_lock = require_regular_windows_state_file(lock_path) + if existing_windows_lock: + require_private_windows_state_path(lock_path, windows_acl) descriptor = os.open( lock_path, os.O_RDWR | os.O_CREAT | getattr(os, "O_BINARY", 0), @@ -172,6 +558,8 @@ def scan_completion_lock(scan_id: str) -> Any: ) locked = False try: + if windows_acl is not None and not existing_windows_lock: + require_private_windows_state_path(lock_path, windows_acl) acquire_completion_file_lock(descriptor) locked = True yield @@ -225,8 +613,70 @@ def release_completion_file_lock(descriptor: int) -> None: def connect() -> sqlite3.Connection: - path = database_path() - path.parent.mkdir(parents=True, exist_ok=True) + requested = requested_state_dir() + root = Path(requested) + try: + windows_acl = windows_state_acl_context() if os.name == "nt" else None + lexical_existing = None + if windows_acl is not None: + lexical_existing = existing_windows_state_ancestor(root) + require_secure_windows_state_ancestry(lexical_existing, windows_acl) + require_secure_state_ancestry(requested) + root = root.resolve() + missing = [] + existing = root + while True: + try: + metadata = existing.lstat() + break + except FileNotFoundError: + missing.append(existing) + parent = existing.parent + if parent == existing: + raise + existing = parent + if not stat.S_ISDIR(metadata.st_mode) or stat.S_ISLNK(metadata.st_mode): + raise SystemExit("State path must use real directories.") + if windows_acl is not None: + if missing or lexical_existing is None or os.path.normcase( + str(lexical_existing) + ) != os.path.normcase(str(existing)): + require_secure_windows_state_ancestry( + existing, + windows_acl, + allow_state_creation=bool(missing), + ) + else: + geteuid = getattr(os, "geteuid", None) + effective_uid = geteuid() if geteuid is not None else None + for parent in (existing, *existing.parents): + require_trusted_output_ancestor(parent, effective_uid) + for directory in reversed(missing): + try: + directory.mkdir(mode=0o700) + except FileExistsError: + require_canonical_scan_directory(directory, validate_windows_acl=False) + if windows_acl is not None: + require_secure_windows_state_ancestry( + directory, + windows_acl, + allow_state_creation=True, + ) + continue + if windows_acl is not None: + install_private_windows_state_acl(directory, windows_acl) + require_private_windows_state_directory(directory, windows_acl) + elif stat.S_IMODE(directory.stat().st_mode) & 0o700 != 0o700: + directory.chmod(0o700) + root = require_canonical_scan_directory(root, validate_windows_acl=False) + if windows_acl is not None: + require_private_windows_state_directory(root, windows_acl) + except (OSError, RuntimeError, SystemExit) as exc: + raise SystemExit( + f"Codex Security state directory is unsafe: {root}. {exc}" + ) from exc + os.environ["CODEX_SECURITY_STATE_DIR"] = str(root) + path = root / "workbench.sqlite3" for attempt in range(SQLITE_RETRY_ATTEMPTS): connection = sqlite3.connect(path, timeout=5) try: @@ -3750,7 +4200,30 @@ def artifact_path(scan_dir: Path, file_name: str, *, required: bool) -> Path | N return resolved -def require_canonical_scan_directory(scan_dir: Path) -> Path: +def require_trusted_output_owner(metadata: os.stat_result, effective_uid: int | None) -> None: + if effective_uid is not None and metadata.st_uid not in {0, effective_uid}: + raise SystemExit("Scan output parent must have a trusted owner.") + + +def require_trusted_output_ancestor(parent: Path | str, effective_uid: int | None) -> None: + try: + metadata = os.lstat(parent) + except OSError as exc: + raise SystemExit("Scan output parent could not be inspected.") from exc + if not stat.S_ISDIR(metadata.st_mode) or stat.S_ISLNK(metadata.st_mode): + raise SystemExit("Scan output parent must be a non-symlink directory.") + require_trusted_output_owner(metadata, effective_uid) + if stat.S_IMODE(metadata.st_mode) & 0o022 and not metadata.st_mode & stat.S_ISVTX: + raise SystemExit( + "Scan output parent must not be group- or world-writable without the sticky bit." + ) + + +def require_canonical_scan_directory( + scan_dir: Path, + *, + validate_windows_acl: bool = True, +) -> Path: scan_dir = scan_dir.absolute() try: metadata = scan_dir.lstat() @@ -3765,7 +4238,12 @@ def require_canonical_scan_directory(scan_dir: Path) -> Path: raise SystemExit("Scan directory must be an existing canonical non-symlink directory.") # Re-check privacy on every resolution so a mid-scan rename/replace under a # shared parent cannot substitute another user's forged artifact tree. - if os.name != "nt": + if os.name == "nt" and validate_windows_acl: + try: + require_private_windows_state_path(scan_dir, windows_state_acl_context()) + except (OSError, RuntimeError) as exc: + raise SystemExit("Scan directory has an unsafe Windows ACL.") from exc + elif os.name != "nt": if stat.S_IMODE(metadata.st_mode) & 0o077: raise SystemExit( "Scan directory must not be accessible to other users (chmod 700)." @@ -3775,26 +4253,7 @@ def require_canonical_scan_directory(scan_dir: Path) -> Path: if effective_uid is not None and metadata.st_uid != effective_uid: raise SystemExit("Scan directory must be owned by the current user.") for parent in scan_dir.parents: - try: - parent_metadata = parent.lstat() - except OSError as exc: - raise SystemExit("Scan output parent could not be inspected.") from exc - if not stat.S_ISDIR(parent_metadata.st_mode) or stat.S_ISLNK( - parent_metadata.st_mode - ): - raise SystemExit("Scan output parent must be a non-symlink directory.") - if effective_uid is not None and parent_metadata.st_uid not in { - 0, - effective_uid, - }: - raise SystemExit("Scan output parent must have a trusted owner.") - if ( - stat.S_IMODE(parent_metadata.st_mode) & 0o022 - and not parent_metadata.st_mode & stat.S_ISVTX - ): - raise SystemExit( - "Scan output parent must not be group- or world-writable without the sticky bit." - ) + require_trusted_output_ancestor(parent, effective_uid) return scan_dir diff --git a/sdk/typescript/src/api.ts b/sdk/typescript/src/api.ts index dc58bb92f..0c637c530 100644 --- a/sdk/typescript/src/api.ts +++ b/sdk/typescript/src/api.ts @@ -128,6 +128,7 @@ import { type PluginInstall, type ProcessEnvironment, type WorkbenchCommandOptions, + validateCodexSecurityStateDirectory, validateOutputDir, } from "./runtime.js"; import { @@ -1598,6 +1599,11 @@ export class CodexSecurity { this.#runtime?.environment ?? this.#dependencies.environment, "chatgpt", ); + if (this.#runtime?.persistentCredentialHome === true) { + await validateCodexSecurityStateDirectory( + codexSecurityStateDirectory(environment), + ); + } const codexHome = this.#runtime?.codexHome ?? (await prepareCodexSecurityCredentialHome(environment)); @@ -1740,11 +1746,14 @@ export class CodexSecurity { `Set ${externalProvider.env_key} to run a scan through ${externalProvider.name}.`, ); } - const scanEnvironment = selectedScanEnvironment( - this.#dependencies.environment, - options.auth, - modelProvider, - ); + const scanEnvironment = { + ...selectedScanEnvironment( + this.#dependencies.environment, + options.auth, + modelProvider, + ), + CODEX_SECURITY_STATE_DIR: stateDirectory, + }; if (this.#dependencies.prepareRuntime === undefined) { const credentialHome = await prepareCodexSecurityCredentialHome( scanEnvironment, @@ -1758,11 +1767,11 @@ export class CodexSecurity { } const previousRuntime = this.#runtime; const runtime = await this.#ensureRuntime( + scanEnvironment, signal, temporaryRoot, (path) => requireOutputOutsideRepository(protectedRoot, path, "runtime"), - options.auth, requestedConfig, ); if ( @@ -1892,20 +1901,20 @@ export class CodexSecurity { } async #ensureRuntime( + processEnvironment: ProcessEnvironment, signal?: AbortSignal, temporaryRoot?: string, validateLocation?: (path: string) => void, - auth: ScanAuthMode = "auto", requestedConfig?: JsonObject, ): Promise { this.#requireOpen(); if (this.#runtime !== null) return this.#runtime; if (this.#runtimePromise === null) { const runtimePromise = this.#prepareRuntime( + processEnvironment, signal ?? this.#abortController.signal, temporaryRoot, validateLocation, - auth, requestedConfig, ); this.#runtimePromise = runtimePromise; @@ -2021,25 +2030,10 @@ export class CodexSecurity { if (requestedOutput !== null) { requireOutputOutsideRepository(protectedRoot, requestedOutput); } - const stateDirectory = codexSecurityStateDirectory( - this.#dependencies.environment, + const stateDirectory = await validateCodexSecurityStateDirectory( + codexSecurityStateDirectory(this.#dependencies.environment), + (canonical) => requireOutputOutsideRepository(protectedRoot, canonical), ); - let canonicalStateDirectory = stateDirectory; - while (true) { - try { - canonicalStateDirectory = join( - await realpath(canonicalStateDirectory), - relative(canonicalStateDirectory, stateDirectory), - ); - break; - } catch (error) { - if ((error as NodeJS.ErrnoException).code !== "ENOENT") throw error; - const parent = dirname(canonicalStateDirectory); - if (parent === canonicalStateDirectory) throw error; - canonicalStateDirectory = parent; - } - } - requireOutputOutsideRepository(protectedRoot, canonicalStateDirectory); return { repository: repo, target: normalized, @@ -2051,10 +2045,10 @@ export class CodexSecurity { } async #prepareRuntime( + processEnvironment: ProcessEnvironment, signal: AbortSignal, temporaryRoot?: string, validateLocation?: (path: string) => void, - auth: ScanAuthMode = "auto", requestedConfig?: JsonObject, ): Promise { if (this.#dependencies.prepareRuntime !== undefined) { @@ -2064,11 +2058,6 @@ export class CodexSecurity { requestedConfig === undefined ? undefined : scanModelProvider(requestedConfig); - const processEnvironment = selectedScanEnvironment( - this.#dependencies.environment, - auth, - modelProvider, - ); const codexHome = validateLocation === undefined ? await prepareCodexSecurityCredentialHome(processEnvironment) diff --git a/sdk/typescript/src/publication-store.ts b/sdk/typescript/src/publication-store.ts index 866f5e06a..c491be5fb 100644 --- a/sdk/typescript/src/publication-store.ts +++ b/sdk/typescript/src/publication-store.ts @@ -8,6 +8,7 @@ import { codexSecurityStateDirectory, resolvePluginPython, runWorkbench, + validateCodexSecurityStateDirectory, } from "./runtime.js"; export async function preparePublicationStore( @@ -92,7 +93,13 @@ async function runPublicationWorkbench( environment: NodeJS.ProcessEnv, issues?: readonly PublishedScanIssue[], ): Promise> { - const stateDirectory = codexSecurityStateDirectory(environment); + const stateDirectory = await validateCodexSecurityStateDirectory( + codexSecurityStateDirectory(environment), + ); + const workbenchEnvironment = { + ...environment, + CODEX_SECURITY_STATE_DIR: stateDirectory, + }; const database = join(stateDirectory, "workbench.sqlite3"); try { if (!(await stat(database)).isFile()) throw new Error("not a regular file"); @@ -104,7 +111,7 @@ async function runPublicationWorkbench( } const [python, pluginRoot] = await Promise.all([ resolvePluginPython({ - environment, + environment: workbenchEnvironment, protectedRoot: publication.scanDirectory, }), bundledPluginRoot(), @@ -113,6 +120,7 @@ async function runPublicationWorkbench( findingId, occurrenceId, })); + await validateCodexSecurityStateDirectory(stateDirectory); const directory = await mkdtemp(join(stateDirectory, "publication-")); try { const input = join(directory, "publication.json"); @@ -131,7 +139,7 @@ async function runPublicationWorkbench( { python, pluginRoot, - environment, + environment: workbenchEnvironment, failureMessage: command === "prepare-linear-publication" ? "Cannot publish findings without their existing local Codex Security scan history" diff --git a/sdk/typescript/src/publish.ts b/sdk/typescript/src/publish.ts index 93639959b..fceeea625 100644 --- a/sdk/typescript/src/publish.ts +++ b/sdk/typescript/src/publish.ts @@ -36,7 +36,9 @@ import { } from "./publication-store.js"; import { codexSecurityStateDirectory, + prepareCodexSecurityStateDirectory, resolveCodexCommand, + validateCodexSecurityStateDirectory, type CodexCommand, } from "./runtime.js"; @@ -144,8 +146,11 @@ export async function publishScanInternal( ); } - const environment = dependencies.environment ?? process.env; - const linearApiKey = resolveLinearApiKey(environment, options.linearApiKey); + const inheritedEnvironment = dependencies.environment ?? process.env; + const linearApiKey = resolveLinearApiKey( + inheritedEnvironment, + options.linearApiKey, + ); if (options.assigneeId !== undefined && linearApiKey === undefined) { throw new ConfigurationError( "A Linear API key is required to select a publication assignee.", @@ -174,6 +179,13 @@ export async function publishScanInternal( } if (prepared.issues.length === 0) return result; + const stateDirectory = await validateCodexSecurityStateDirectory( + codexSecurityStateDirectory(inheritedEnvironment), + ); + const environment = { + ...inheritedEnvironment, + CODEX_SECURITY_STATE_DIR: stateDirectory, + }; await (dependencies.preparePublicationStore ?? preparePublicationStore)( prepared, environment, @@ -207,7 +219,7 @@ export async function publishScanInternal( ? (dependencies.resolveCodex ?? resolveCodexCommand)(environment) : undefined; options.signal?.throwIfAborted(); - const handoff = await createPublicationHandoff(prepared, environment); + const handoff = await createPublicationHandoff(prepared, stateDirectory); const progressObserver = options.onProgress; reportPublicationProgress(progressObserver, { type: "started", @@ -319,12 +331,18 @@ export async function publishScanInternal( result.failed = handoffResults.failed; result.counts.created = result.created.length; result.counts.failed = result.failed.length; + const saveReceipt = async (): Promise => { + const receiptStateDirectory = + await prepareCodexSecurityStateDirectory(stateDirectory); + if (dependencies.writeReceipt !== undefined) { + await dependencies.writeReceipt(result, environment); + } else { + await writePublicationReceipt(result, receiptStateDirectory); + } + }; if (options.signal?.aborted) { try { - await (dependencies.writeReceipt ?? writePublicationReceipt)( - result, - environment, - ); + await saveReceipt(); } catch (error) { const detail = error instanceof Error ? error.message : String(error); throw new CodexSecurityError( @@ -356,10 +374,7 @@ export async function publishScanInternal( } } try { - await (dependencies.writeReceipt ?? writePublicationReceipt)( - result, - environment, - ); + await saveReceipt(); } catch (error) { if (result.created.length === 0 || options.signal?.aborted) throw error; result.warnings = [ @@ -593,10 +608,10 @@ function publicationPrompt( async function createPublicationHandoff( publication: PreparedScanPublication, - environment: NodeJS.ProcessEnv, + stateDirectory: string, ): Promise<{ directory: string; file: string; publicationFile: string }> { const root = join( - codexSecurityStateDirectory(environment), + await prepareCodexSecurityStateDirectory(stateDirectory), "publications", "linear", "handoffs", @@ -1059,13 +1074,9 @@ function reportCodexEvent( async function writePublicationReceipt( result: PublishScanResult, - environment: NodeJS.ProcessEnv, + stateDirectory: string, ): Promise { - const directory = join( - codexSecurityStateDirectory(environment), - "publications", - "linear", - ); + const directory = join(stateDirectory, "publications", "linear"); await mkdir(directory, { mode: 0o700, recursive: true }); const name = createHash("sha256").update(result.scanId).digest("hex"); const contents = JSON.stringify(result); diff --git a/sdk/typescript/src/runtime.ts b/sdk/typescript/src/runtime.ts index 15063adf6..fe72ee69a 100644 --- a/sdk/typescript/src/runtime.ts +++ b/sdk/typescript/src/runtime.ts @@ -18,6 +18,7 @@ import { open, opendir, readFile, + readlink, readdir, realpath, rename, @@ -152,6 +153,252 @@ export function codexSecurityStateDirectory( return path; } +interface WindowsPrivateDirectoryOptions { + repair: boolean; + creationAncestor?: boolean; + inspectDescendants?: boolean; +} + +interface StateDirectorySecurityOptions { + platform?: NodeJS.Platform; + windowsAcl?: ( + path: string, + options: WindowsPrivateDirectoryOptions, + ) => Promise; +} + +async function nearestExistingStateAncestor(path: string): Promise { + for (let current = path; ; current = dirname(current)) { + const metadata = await lstat(current).catch((error: unknown) => { + if (nodeErrorCode(error) === "ENOENT") return null; + throw error; + }); + if (metadata !== null) { + if (!metadata.isDirectory() && !metadata.isSymbolicLink()) { + throw new OutputDirectoryError( + `Codex Security state path must use directories: ${current}`, + ); + } + return current; + } + if (current === dirname(current)) { + throw new OutputDirectoryError( + `Unable to find an existing Codex Security state ancestor: ${path}`, + ); + } + } +} + +export async function validateCodexSecurityStateDirectory( + path: string, + validateLocation?: (canonical: string) => void, + securityOptions: StateDirectorySecurityOptions = {}, +): Promise { + const requested = resolve(expandHome(path)); + const platform = securityOptions.platform ?? process.platform; + try { + const canonical = await canonicalizeModelSafePath(requested); + const windowsAcl = + securityOptions.windowsAcl ?? secureWindowsPrivateDirectory; + const inspectWindowsAcl = async ( + inspected: string, + options: WindowsPrivateDirectoryOptions, + ): Promise => { + try { + await windowsAcl(inspected, options); + } catch (error) { + const detail = windowsCredentialAclFailure(error); + throw new OutputDirectoryError( + `Configured Codex Security state directory must have a private Windows ACL: ${inspected}${detail}`, + { cause: error }, + ); + } + }; + validateLocation?.(canonical); + const metadata = await lstat(canonical).catch((error: unknown) => { + if (nodeErrorCode(error) === "ENOENT") return null; + throw error; + }); + if (metadata !== null) { + if (!metadata.isDirectory() || metadata.isSymbolicLink()) { + throw new OutputDirectoryError( + `Configured Codex Security state path must be a directory: ${canonical}`, + ); + } + if (platform === "win32") { + const paths = + relative(requested, canonical) === "" + ? [canonical] + : [requested, canonical]; + for (const inspected of paths) { + await inspectWindowsAcl(inspected, { + repair: false, + inspectDescendants: false, + }); + } + } else { + const effectiveUid = process.geteuid?.(); + try { + requirePrivateOutputDirectory(metadata, canonical, effectiveUid); + } catch (error) { + if (!(error instanceof OutputDirectoryError)) throw error; + const mode = (metadata.mode & 0o7777).toString(8).padStart(4, "0"); + if (effectiveUid !== undefined && metadata.uid !== effectiveUid) { + throw new OutputDirectoryError( + `Configured Codex Security state directory must be owned by the current user: ${canonical} (mode ${mode}). Set CODEX_SECURITY_STATE_DIR to a private directory owned by the current user.`, + { cause: error }, + ); + } + throw new OutputDirectoryError( + `Configured Codex Security state directory must be private to the current user: ${canonical} (mode ${mode}). Set CODEX_SECURITY_STATE_DIR to a private directory, or set this directory's permissions to 0700 only if you own it and can safely change it.`, + { cause: error }, + ); + } + } + } else if (platform === "win32") { + const ancestors: string[] = []; + for (const candidate of [requested, canonical]) { + const ancestor = await nearestExistingStateAncestor(candidate); + if ( + ancestors.some((inspected) => relative(inspected, ancestor) === "") + ) { + continue; + } + ancestors.push(ancestor); + await inspectWindowsAcl(ancestor, { + repair: false, + creationAncestor: true, + inspectDescendants: false, + }); + } + } + if (platform !== "win32") await requireSecureStateAncestry(requested); + return canonical; + } catch (error) { + if (error instanceof OutputDirectoryError) throw error; + throw new OutputDirectoryError( + `Unable to inspect configured Codex Security state directory: ${requested}`, + { cause: error }, + ); + } +} + +export async function prepareCodexSecurityStateDirectory( + path: string, + validateLocation?: (canonical: string) => void, + securityOptions: StateDirectorySecurityOptions = {}, +): Promise { + const canonical = await validateCodexSecurityStateDirectory( + path, + validateLocation, + securityOptions, + ); + const platform = securityOptions.platform ?? process.platform; + const windowsAcl = + securityOptions.windowsAcl ?? secureWindowsPrivateDirectory; + const validatePinned = async (): Promise => { + const validated = await validateCodexSecurityStateDirectory( + canonical, + validateLocation, + securityOptions, + ); + if (relative(canonical, validated) !== "") { + throw new OutputDirectoryError( + `Configured Codex Security state directory changed during preparation: ${canonical}`, + ); + } + return validated; + }; + try { + const missing: string[] = []; + let current = canonical; + while (true) { + try { + await lstat(current); + break; + } catch (error) { + if (nodeErrorCode(error) !== "ENOENT") throw error; + missing.push(current); + const parent = dirname(current); + if (parent === current) throw error; + current = parent; + } + } + await validatePinned(); + for (const directory of missing.reverse()) { + try { + await mkdir(directory, { mode: 0o700 }); + } catch (error) { + if (nodeErrorCode(error) !== "EEXIST") throw error; + await validatePinned(); + continue; + } + if (platform === "win32") { + await windowsAcl(directory, { + repair: true, + inspectDescendants: false, + }); + } else { + // Restore owner access before creating a child under a restrictive + // umask. + if (((await lstat(directory)).mode & 0o700) !== 0o700) { + await chmod(directory, 0o700); + } + } + } + return await validatePinned(); + } catch (error) { + if (error instanceof OutputDirectoryError) throw error; + throw new OutputDirectoryError( + `Unable to prepare configured Codex Security state directory: ${canonical}`, + { cause: error }, + ); + } +} + +async function requireSecureStateAncestry( + path: string, + effectiveUid = process.geteuid?.(), +): Promise { + if (process.platform === "win32") return; + const pending = [path]; + const checked = new Set(); + while (pending.length > 0) { + let current = pending.pop()!; + while (true) { + while (current.length > 1 && current.endsWith(sep)) { + current = current.slice(0, -1); + } + if (checked.has(current)) break; + checked.add(current); + const parent = dirname(current); + const metadata = await lstat(current).catch((error: unknown) => { + if (nodeErrorCode(error) === "ENOENT") return null; + throw error; + }); + if (metadata?.isSymbolicLink()) { + requireTrustedOutputOwner(metadata, current, effectiveUid); + const canonical = await realpath(current); + const target = await readlink(current); + // Preserve dot segments until the filesystem resolves the link target. + const lexicalTarget = isAbsolute(target) + ? target + : `${parent}${parent.endsWith(sep) ? "" : sep}${target}`; + pending.push(canonical, lexicalTarget); + } else if (metadata !== null) { + if (!metadata.isDirectory()) { + throw new OutputDirectoryError( + `Codex Security state path must use directories: ${current}`, + ); + } + requireTrustedOutputAncestor(metadata, current, effectiveUid); + } + if (parent === current) break; + current = parent; + } + } +} + export function codexSecurityCredentialHome( environment: ProcessEnvironment = process.env, ): string { @@ -162,7 +409,13 @@ export async function prepareCodexSecurityCredentialHome( environment: ProcessEnvironment = process.env, validateLocation?: (path: string) => void, ): Promise { - const path = codexSecurityCredentialHome(environment); + const stateDirectory = await prepareCodexSecurityStateDirectory( + codexSecurityStateDirectory(environment), + validateLocation === undefined + ? undefined + : (canonical) => validateLocation(join(canonical, "codex-home")), + ); + const path = join(stateDirectory, "codex-home"); try { try { await mkdir(path, { recursive: true, mode: 0o700 }); @@ -418,7 +671,7 @@ export function inspectWindowsCredentialAcl( currentUserSid: string, options: { resolvedAliases?: Readonly>; - scope?: "directory" | "file" | "ancestor"; + scope?: "directory" | "file" | "ancestor" | "creation-ancestor"; } = {}, ): WindowsCredentialAcl { if (!WINDOWS_SID.test(currentUserSid)) { @@ -439,7 +692,7 @@ export function inspectWindowsCredentialAcl( WINDOWS_ADMINISTRATORS_SID, principalAliases["LA"] ?? "LA", ]); - if (options.scope === "ancestor") { + if (options.scope === "ancestor" || options.scope === "creation-ancestor") { trustedPrincipals.add(WINDOWS_TRUSTED_INSTALLER_SID); } const normalizePrincipal = (principal: string): string => @@ -495,8 +748,11 @@ export function inspectWindowsCredentialAcl( if (type === "A" || type === "OA" || type === "XA" || type === "ZA") { if (!trustedPrincipal(principal)) { if ( - options.scope !== "ancestor" || - windowsAceAllowsAncestorReplacement(rights!, inheritanceFlags) + (options.scope !== "ancestor" && + options.scope !== "creation-ancestor") || + (options.scope === "ancestor" + ? windowsAceAllowsAncestorReplacement(rights!, inheritanceFlags) + : windowsAceAllowsStateCreationRace(rights!, inheritanceFlags)) ) { untrustedPrincipals.add(principal); } @@ -596,6 +852,10 @@ function windowsAceAllowsAncestorReplacement( inheritanceFlags: ReadonlySet, ): boolean { if (inheritanceFlags.has("IO")) return false; + return windowsRightsAllowAncestorReplacement(rights); +} + +function windowsRightsAllowAncestorReplacement(rights: string): boolean { if (/^0x[\da-f]+$/iu.test(rights)) { return (BigInt(rights) & 0x100d0040n) !== 0n; } @@ -611,6 +871,23 @@ function windowsAceAllowsAncestorReplacement( return false; } +function windowsAceAllowsStateCreationRace( + rights: string, + inheritanceFlags: ReadonlySet, +): boolean { + if (inheritanceFlags.has("IO") && !inheritanceFlags.has("CI")) { + return false; + } + if (/^0x[\da-f]+$/iu.test(rights)) { + return (BigInt(rights) & 0x500d0046n) !== 0n; + } + if (windowsRightsAllowAncestorReplacement(rights)) return true; + for (let index = 0; index < rights.length; index += 2) { + if (["CC", "AD"].includes(rights.slice(index, index + 2))) return true; + } + return false; +} + export async function verifyStableWindowsCredentialDescendants( path: string, inspectDescriptors: () => Promise, @@ -719,6 +996,7 @@ export async function inspectWindowsCredentialAclSnapshot( command: string; args: readonly string[]; environment?: NodeJS.ProcessEnv; + inspectDescendants?: boolean; resolvedAliases?: Readonly>; resolveDescriptorAliases?: (descriptor: string) => Promise; }, @@ -734,77 +1012,80 @@ export async function inspectWindowsCredentialAclSnapshot( let home: WindowsCredentialAcl | undefined; let descendantsArePrivate = true; - await verifyStableWindowsCredentialDescendants( - path, - async () => { - home = undefined; - descendantsArePrivate = true; - let inspected = 0; - const descriptors = await streamWindowsCredentialAclDescriptors( - options.command, - options.args, - async (descriptor) => { - const index = inspected; - inspected += 1; - await options.resolveDescriptorAliases?.(descriptor); - - if (index < ancestors) { - const ancestor = inspectWindowsCredentialAcl( - descriptor, - currentUserSid, - { - resolvedAliases: options.resolvedAliases, - scope: "ancestor", - }, - ); - if (ancestor.untrustedPrincipals.length !== 0) { - throw new Error( - "Windows credential-home ancestor allows another identity to replace the directory", - ); - } - return; - } - - if (index === ancestors) { - try { - home = inspectWindowsCredentialAcl(descriptor, currentUserSid, { - resolvedAliases: options.resolvedAliases, - }); - } catch (error) { - if (error instanceof UntrustedWindowsCredentialOwnerError) { - throw new RepairableWindowsCredentialOwnerError(error); - } - throw new RepairableWindowsCredentialAclError(error); - } - return; - } - - const descendant = inspectWindowsCredentialAcl( + const inspectDescriptors = async (): Promise => { + home = undefined; + descendantsArePrivate = true; + let inspected = 0; + const descriptors = await streamWindowsCredentialAclDescriptors( + options.command, + options.args, + async (descriptor) => { + const index = inspected; + inspected += 1; + await options.resolveDescriptorAliases?.(descriptor); + + if (index < ancestors) { + const ancestor = inspectWindowsCredentialAcl( descriptor, currentUserSid, { resolvedAliases: options.resolvedAliases, - scope: "file", + scope: "ancestor", }, ); - if ( - !descendant.grantsCurrentUserAccess || - descendant.untrustedPrincipals.length !== 0 - ) { - descendantsArePrivate = false; + if (ancestor.untrustedPrincipals.length !== 0) { + throw new Error( + "Windows credential-home ancestor allows another identity to replace the directory", + ); } - }, - { environment: options.environment }, - ); - if (descriptors <= ancestors) { - throw new Error( - "Windows credential-home ancestry could not be verified", + return; + } + + if (index === ancestors) { + try { + home = inspectWindowsCredentialAcl(descriptor, currentUserSid, { + resolvedAliases: options.resolvedAliases, + }); + } catch (error) { + if (error instanceof UntrustedWindowsCredentialOwnerError) { + throw new RepairableWindowsCredentialOwnerError(error); + } + throw new RepairableWindowsCredentialAclError(error); + } + return; + } + + const descendant = inspectWindowsCredentialAcl( + descriptor, + currentUserSid, + { + resolvedAliases: options.resolvedAliases, + scope: "file", + }, ); - } - return descriptors - ancestors - 1; - }, - { inspectEmpty: true }, - ); + if ( + !descendant.grantsCurrentUserAccess || + descendant.untrustedPrincipals.length !== 0 + ) { + descendantsArePrivate = false; + } + }, + { environment: options.environment }, + ); + if (descriptors <= ancestors) { + throw new Error("Windows credential-home ancestry could not be verified"); + } + return descriptors - ancestors - 1; + }; + if (options.inspectDescendants === false) { + if ((await inspectDescriptors()) !== 0) { + throw new Error("Windows credential ACL inspection was not root-only"); + } + } else { + await verifyStableWindowsCredentialDescendants(path, inspectDescriptors, { + inspectEmpty: true, + }); + } if (home === undefined) { throw new Error("Windows credential ACL could not be verified"); @@ -812,7 +1093,51 @@ export async function inspectWindowsCredentialAclSnapshot( return { home, descendantsArePrivate }; } +async function inspectWindowsStateCreationAncestry( + path: string, + currentUserSid: string, + options: { + command: string; + args: readonly string[]; + environment?: NodeJS.ProcessEnv; + resolvedAliases?: Readonly>; + resolveDescriptorAliases?: (descriptor: string) => Promise; + }, +): Promise { + let inspected = 0; + const descriptors = await streamWindowsCredentialAclDescriptors( + options.command, + options.args, + async (descriptor) => { + await options.resolveDescriptorAliases?.(descriptor); + const acl = inspectWindowsCredentialAcl(descriptor, currentUserSid, { + resolvedAliases: options.resolvedAliases, + scope: inspected === 0 ? "creation-ancestor" : "ancestor", + }); + inspected += 1; + if (acl.untrustedPrincipals.length !== 0) { + throw new Error( + "Windows state-creation ancestor allows another identity to create or replace a directory", + ); + } + }, + { environment: options.environment }, + ); + if (descriptors === 0) { + throw new Error( + `Windows state-creation ancestry could not be verified: ${path}`, + ); + } +} + async function secureWindowsCredentialHome(path: string): Promise { + await secureWindowsPrivateDirectory(path, { repair: true }); +} + +async function secureWindowsPrivateDirectory( + path: string, + options: WindowsPrivateDirectoryOptions, +): Promise { const systemRoot = process.env["SystemRoot"] ?? "C:\\Windows"; const systemDirectory = join(systemRoot, "System32"); const powershell = join( @@ -855,14 +1180,24 @@ async function secureWindowsCredentialHome(path: string): Promise { // Signed built-in cmdlets remain available under ConstrainedLanguage; // arbitrary .NET constructors, static methods, and SID translation do not. - const script = [ + const inspectionCommands = [ "$ErrorActionPreference = 'Stop'", "$path = $env:CODEX_SECURITY_CREDENTIAL_ACL_PATH", "while ($true) { $parent = Microsoft.PowerShell.Management\\Split-Path -Path $path -Parent; if (-not $parent -or $parent -eq $path) { break }; Microsoft.PowerShell.Security\\Get-Acl -LiteralPath $parent | Microsoft.PowerShell.Utility\\Select-Object -ExpandProperty Sddl; $path = $parent }", "Microsoft.PowerShell.Security\\Get-Acl -LiteralPath $env:CODEX_SECURITY_CREDENTIAL_ACL_PATH | Microsoft.PowerShell.Utility\\Select-Object -ExpandProperty Sddl", + ]; + if (options.inspectDescendants !== false) { // A temporary descendant can disappear after enumeration. Its missing // descriptor reduces the count and retries the stable snapshot. - "Microsoft.PowerShell.Management\\Get-ChildItem -LiteralPath $env:CODEX_SECURITY_CREDENTIAL_ACL_PATH -Recurse -Force | Microsoft.PowerShell.Core\\ForEach-Object { try { Microsoft.PowerShell.Security\\Get-Acl -LiteralPath $_.FullName | Microsoft.PowerShell.Utility\\Select-Object -ExpandProperty Sddl } catch { if ($_.FullyQualifiedErrorId -notlike 'GetAcl_PathNotFound,*') { throw } } }", + inspectionCommands.push( + "Microsoft.PowerShell.Management\\Get-ChildItem -LiteralPath $env:CODEX_SECURITY_CREDENTIAL_ACL_PATH -Recurse -Force | Microsoft.PowerShell.Core\\ForEach-Object { try { Microsoft.PowerShell.Security\\Get-Acl -LiteralPath $_.FullName | Microsoft.PowerShell.Utility\\Select-Object -ExpandProperty Sddl } catch { if ($_.FullyQualifiedErrorId -notlike 'GetAcl_PathNotFound,*') { throw } } }", + ); + } + const script = inspectionCommands.join("; "); + const creationAncestryScript = [ + "$ErrorActionPreference = 'Stop'", + "$path = $env:CODEX_SECURITY_CREDENTIAL_ACL_PATH", + "while ($true) { Microsoft.PowerShell.Security\\Get-Acl -LiteralPath $path | Microsoft.PowerShell.Utility\\Select-Object -ExpandProperty Sddl; $parent = Microsoft.PowerShell.Management\\Split-Path -Path $path -Parent; if (-not $parent -or $parent -eq $path) { break }; $path = $parent }", ].join("; "); const resolvePrincipalScript = [ "$ErrorActionPreference = 'Stop'", @@ -917,12 +1252,29 @@ async function secureWindowsCredentialHome(path: string): Promise { remaining = rest; } }; + if (options.creationAncestor === true) { + await inspectWindowsStateCreationAncestry(path, sid, { + command: powershell, + args: [ + "-NoLogo", + "-NoProfile", + "-NonInteractive", + "-Command", + creationAncestryScript, + ], + environment: processOptions.env, + resolvedAliases, + resolveDescriptorAliases, + }); + return; + } let descendantsArePrivate = true; const readAcl = async (): Promise => { const snapshot = await inspectWindowsCredentialAclSnapshot(path, sid, { command: powershell, args: ["-NoLogo", "-NoProfile", "-NonInteractive", "-Command", script], environment: processOptions.env, + inspectDescendants: options.inspectDescendants, resolvedAliases, resolveDescriptorAliases, }); @@ -950,6 +1302,7 @@ async function secureWindowsCredentialHome(path: string): Promise { try { existing = await readAcl(); } catch (error) { + if (!options.repair) throw error; if (error instanceof RepairableWindowsCredentialOwnerError) { await execFile(icacls, [path, "/setowner", `*${sid}`], processOptions); } else if (error instanceof RepairableWindowsCredentialAclError) { @@ -962,9 +1315,14 @@ async function secureWindowsCredentialHome(path: string): Promise { if (existing === undefined) { throw new Error("Windows credential ACL could not be repaired"); } + if (!options.repair) { + requirePrivateWindowsCredentialAcl(existing, descendantsArePrivate); + return; + } if ( existing.grantsCurrentUserAccess && existing.untrustedPrincipals.length === 0 && + existing.deniedPrincipals.length === 0 && !existing.protected ) { await execFile(icacls, [path, "/inheritance:d"], processOptions); @@ -975,7 +1333,8 @@ async function secureWindowsCredentialHome(path: string): Promise { if ( !verified.protected || !verified.grantsCurrentUserAccess || - verified.untrustedPrincipals.length !== 0 + verified.untrustedPrincipals.length !== 0 || + verified.deniedPrincipals.length !== 0 ) { await installTrustedAcl(); verified = await readAcl(); @@ -1010,17 +1369,6 @@ async function secureWindowsCredentialHome(path: string): Promise { verified = await readAcl(); } } - if (!verified.protected) { - throw new Error("Windows credential ACL still inherits access rules"); - } - if (!verified.grantsCurrentUserAccess) { - throw new Error( - "Windows credential ACL does not grant the current user access", - ); - } - if (verified.untrustedPrincipals.length !== 0) { - throw new Error("Windows credential ACL grants access to another identity"); - } if (!descendantsArePrivate) { await execFile( icacls, @@ -1032,6 +1380,57 @@ async function secureWindowsCredentialHome(path: string): Promise { throw new Error("Windows credential descendants remain accessible"); } } + requirePrivateWindowsCredentialAcl(verified, descendantsArePrivate); +} + +function requirePrivateWindowsCredentialAcl( + acl: WindowsCredentialAcl, + descendantsArePrivate: boolean, +): void { + if (!acl.protected) { + throw new Error("Windows credential ACL still inherits access rules"); + } + if (!acl.grantsCurrentUserAccess) { + throw new Error( + "Windows credential ACL does not grant the current user access", + ); + } + if (acl.untrustedPrincipals.length !== 0) { + throw new Error("Windows credential ACL grants access to another identity"); + } + if (acl.deniedPrincipals.length !== 0) { + throw new Error("Windows credential ACL denies access to an identity"); + } + if (!descendantsArePrivate) { + throw new Error("Windows credential descendants remain accessible"); + } +} + +interface CredentialLockReadRetry { + deadline?: number; + device?: bigint; + inode?: bigint; +} + +function clearCredentialLockReadRetry( + readRetry: CredentialLockReadRetry, +): void { + delete readRetry.deadline; + delete readRetry.device; + delete readRetry.inode; +} + +function credentialLockReadDeadline( + readRetry: CredentialLockReadRetry, + metadata: BigIntStats, +): number { + if (readRetry.device !== metadata.dev || readRetry.inode !== metadata.ino) { + clearCredentialLockReadRetry(readRetry); + readRetry.device = metadata.dev; + readRetry.inode = metadata.ino; + } + return (readRetry.deadline ??= + Date.now() + INCOMPLETE_CREDENTIAL_LOCK_MILLISECONDS); } export async function acquireCodexSecurityCredentialHomeLock( @@ -1048,9 +1447,11 @@ export async function acquireCodexSecurityCredentialHomeLock( ); const expectedDevice = homeMetadata.dev; const expectedInode = homeMetadata.ino; + const platform = securityOptions.platform ?? process.platform; const lock = join(codexHome, CREDENTIAL_LOCK_NAME); const ownerPath = join(lock, "owner.json"); const token = randomUUID(); + const readRetry: CredentialLockReadRetry = {}; while (true) { throwIfSignalAborted(signal); @@ -1065,10 +1466,12 @@ export async function acquireCodexSecurityCredentialHomeLock( throw error; }); if (existingLock !== null) { - if (await recoverStaleCredentialHomeLock(lock)) continue; + if (await recoverStaleCredentialHomeLock(lock, platform, readRetry)) + continue; await delay(CREDENTIAL_LOCK_POLL_MILLISECONDS, undefined, { signal }); continue; } + clearCredentialLockReadRetry(readRetry); await requireSecureCredentialHome(codexHome, { ...securityOptions, expectedDevice, @@ -1078,7 +1481,8 @@ 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, platform, readRetry)) + continue; await delay(CREDENTIAL_LOCK_POLL_MILLISECONDS, undefined, { signal }); continue; } @@ -1116,12 +1520,21 @@ export async function acquireCodexSecurityCredentialHomeLock( } } -async function recoverStaleCredentialHomeLock(lock: string): Promise { - const metadata = await lstat(lock).catch((error: unknown) => { - if (nodeErrorCode(error) === "ENOENT") return null; - throw error; - }); - if (metadata === null) return true; +async function recoverStaleCredentialHomeLock( + lock: string, + platform: NodeJS.Platform, + readRetry: CredentialLockReadRetry, +): Promise { + const metadata = await lstat(lock, { bigint: true }).catch( + (error: unknown) => { + if (nodeErrorCode(error) === "ENOENT") return null; + throw error; + }, + ); + if (metadata === null) { + clearCredentialLockReadRetry(readRetry); + return true; + } if (!metadata.isDirectory() || metadata.isSymbolicLink()) { throw new OutputDirectoryError( `Codex Security credential-home lock is not a directory: ${lock}`, @@ -1129,19 +1542,53 @@ async function recoverStaleCredentialHomeLock(lock: string): Promise { } let owner: unknown; + const deadline = + platform === "win32" + ? credentialLockReadDeadline(readRetry, metadata) + : undefined; try { owner = JSON.parse(await readFile(join(lock, "owner.json"), "utf8")); } catch (error) { - if (nodeErrorCode(error) !== "ENOENT" && !(error instanceof SyntaxError)) { + const code = nodeErrorCode(error); + // Re-enter the acquisition loop so a Windows read retry rechecks the lock. + const transientWindowsRead = + deadline !== undefined && (code === "EPERM" || code === "EBUSY"); + if (transientWindowsRead) { + if (Date.now() < deadline) return false; + // The failed read can race a lock replacement. Only charge an expired + // retry window to the lock identity that established it. + const current = await lstat(lock, { bigint: true }).catch( + (statError: unknown) => { + if (nodeErrorCode(statError) === "ENOENT") return null; + throw statError; + }, + ); + if (current === null) { + clearCredentialLockReadRetry(readRetry); + return true; + } + if (!current.isDirectory() || current.isSymbolicLink()) { + throw new OutputDirectoryError( + `Codex Security credential-home lock is not a directory: ${lock}`, + ); + } + if (current.dev !== metadata.dev || current.ino !== metadata.ino) { + credentialLockReadDeadline(readRetry, current); + return false; + } + } + clearCredentialLockReadRetry(readRetry); + if (code !== "ENOENT" && !(error instanceof SyntaxError)) { throw error; } if ( - Date.now() - metadata.mtimeMs < + Date.now() - Number(metadata.mtimeMs) < INCOMPLETE_CREDENTIAL_LOCK_MILLISECONDS ) { return false; } } + clearCredentialLockReadRetry(readRetry); // 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. @@ -1162,7 +1609,7 @@ async function recoverStaleCredentialHomeLock(lock: string): Promise { } } } else if ( - Date.now() - metadata.mtimeMs < + Date.now() - Number(metadata.mtimeMs) < INCOMPLETE_CREDENTIAL_LOCK_MILLISECONDS ) { return false; @@ -1172,10 +1619,14 @@ async function recoverStaleCredentialHomeLock(lock: string): Promise { try { await rename(lock, quarantine); } catch (error) { - if (nodeErrorCode(error) === "ENOENT") return true; + if (nodeErrorCode(error) === "ENOENT") { + clearCredentialLockReadRetry(readRetry); + return true; + } throw error; } await rm(quarantine, { recursive: true, force: true }); + clearCredentialLockReadRetry(readRetry); return true; } @@ -1354,8 +1805,7 @@ export async function preparePersistentOutputRoot( repositoryName: string, ): Promise { requireModelSafeOutputDir(stateDirectory); - await mkdir(stateDirectory, { recursive: true, mode: 0o700 }); - let root = await realpath(stateDirectory); + let root = await prepareCodexSecurityStateDirectory(stateDirectory); for (const directory of [category, safePrefix(repositoryName)]) { root = join(root, directory); await mkdir(root, { recursive: true, mode: 0o700 }); @@ -1373,17 +1823,29 @@ export async function runWorkbench( args: readonly string[], input?: string, ): Promise { + const stateDirectory = ["inspect-target", "inspect-setup"].includes( + args[0] ?? "", + ) + ? undefined + : await validateCodexSecurityStateDirectory( + codexSecurityStateDirectory(options.environment), + ); + 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" && + (stateDirectory === undefined || + name.toUpperCase() !== "CODEX_SECURITY_STATE_DIR"), + ), + ); + if (stateDirectory !== undefined) { + environment["CODEX_SECURITY_STATE_DIR"] = stateDirectory; + } 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 }, [ @@ -1418,7 +1880,7 @@ export async function runWorkbench( throw new CodexSecurityError( databaseFailure ? `${failure}: cannot open the workbench database at ${join( - codexSecurityStateDirectory(options.environment), + stateDirectory ?? codexSecurityStateDirectory(options.environment), "workbench.sqlite3", )}. Ensure the state directory and SQLite journal files are writable, or set CODEX_SECURITY_STATE_DIR to a writable directory outside the scanned repository.` : `${failure}: ${detail}`, @@ -1731,6 +2193,21 @@ export function requireTrustedOutputAncestor( metadata: Pick, path: string, effectiveUid = process.geteuid?.(), +): void { + requireTrustedOutputOwner(metadata, path, effectiveUid); + if ((metadata.mode & 0o022) === 0) return; + if ((metadata.mode & 0o1000) === 0) { + const mode = (metadata.mode & 0o7777).toString(8).padStart(4, "0"); + throw new OutputDirectoryError( + `Scan output parent must not be group- or world-writable without the sticky bit: ${path} (mode ${mode}). A private child directory does not make an unsafe ancestor safe. Choose a location with secure parent directories, or remove group- and world-write permissions from this ancestor only if you own it and can safely change it.`, + ); + } +} + +function requireTrustedOutputOwner( + metadata: Pick, + path: string, + effectiveUid: number | undefined, ): void { if ( effectiveUid !== undefined && @@ -1741,12 +2218,6 @@ export function requireTrustedOutputAncestor( `Scan output parent must have a trusted owner: ${path}`, ); } - if ((metadata.mode & 0o022) === 0) return; - if ((metadata.mode & 0o1000) === 0) { - throw new OutputDirectoryError( - `Scan output parent must not be group- or world-writable without the sticky bit: ${path}`, - ); - } } async function removeEmptyDirectories( diff --git a/sdk/typescript/src/version.ts b/sdk/typescript/src/version.ts index 241666585..c0cf1dfe7 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.22" as const; +export const BUNDLED_PLUGIN_VERSION = "0.1.24" 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..eb0b9757d 100644 --- a/sdk/typescript/tests-ts/api-post-scan.test.ts +++ b/sdk/typescript/tests-ts/api-post-scan.test.ts @@ -36,7 +36,7 @@ describe("completed scan follow-up instructions", () => { const client = new TestClient( {}, { - environment: {}, + environment: { CODEX_SECURITY_STATE_DIR: join(root, "state") }, prepareRuntime: async () => preparedRuntime(codexHome), resolvePluginPython: async () => "/managed/python", prepareOutputDir: async () => scanDir, diff --git a/sdk/typescript/tests-ts/api-surface.test.ts b/sdk/typescript/tests-ts/api-surface.test.ts index dc4072d47..2a71fdeb9 100644 --- a/sdk/typescript/tests-ts/api-surface.test.ts +++ b/sdk/typescript/tests-ts/api-surface.test.ts @@ -34,7 +34,7 @@ async function scanResponseSurface(runtimeOptions?: { const client = new InternalCodexSecurity( {}, { - environment: {}, + environment: { CODEX_SECURITY_STATE_DIR: join(root, "state") }, prepareRuntime: async () => preparedRuntime(codexHome), resolvePluginPython: async () => "/managed/python", prepareOutputDir: async () => scanDir, diff --git a/sdk/typescript/tests-ts/api.test.ts b/sdk/typescript/tests-ts/api.test.ts index 35e85126c..6b4ace8c2 100644 --- a/sdk/typescript/tests-ts/api.test.ts +++ b/sdk/typescript/tests-ts/api.test.ts @@ -1,5 +1,6 @@ import { appendFile, + chmod, copyFile, cp, mkdir, @@ -15,6 +16,7 @@ import * as fsPromises from "node:fs/promises"; import { execFileSync } from "node:child_process"; import { createHash } from "node:crypto"; import { existsSync } from "node:fs"; +import { tmpdir } from "node:os"; import { basename, join } from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; import { Codex, type CodexOptions, type ThreadEvent } from "@openai/codex-sdk"; @@ -69,6 +71,7 @@ type ScanObserverName = Parameters< const REPOSITORY_ROOT = fileURLToPath(new URL("../../..", import.meta.url)); const EXAMPLE = join(PLUGIN_ROOT, "examples", "completed-scan"); +const testPosix = process.platform === "win32" ? test.skip : test; const { cleanup, copyCompletedScan, temporaryDirectory } = createApiTestFixtures(); afterEach(cleanup); @@ -1621,7 +1624,7 @@ describe("CodexSecurity orchestration", () => { const client = new TestClient( {}, { - environment: {}, + environment: { CODEX_SECURITY_STATE_DIR: join(root, "state") }, prepareRuntime: async () => { runtimeStarted = true; throw new Error("runtime should not initialize"); @@ -4176,6 +4179,66 @@ describe("CodexSecurity orchestration", () => { expect(existsSync(join(result.scanDir, "scan-manifest.json"))).toBe(true); }); + test("preflights state initialized by a history command", async () => { + const root = await temporaryDirectory(); + const repository = join(root, "repository"); + const stateDirectory = join(root, "state"); + const outputDir = join(root, "output"); + await mkdir(repository); + const python = Bun.which("python3") ?? Bun.which("python"); + expect(python).not.toBeNull(); + const environment = { + PATH: process.env["PATH"], + CODEX_SECURITY_STATE_DIR: stateDirectory, + }; + const history = execFileSync( + python!, + [ + "-I", + "-B", + "-c", + [ + "import os, runpy, sys", + "os.umask(0o022)", + "sys.argv = sys.argv[1:]", + "sys.path.insert(0, os.path.dirname(sys.argv[0]))", + 'runpy.run_path(sys.argv[0], run_name="__main__")', + ].join("\n"), + join(PLUGIN_ROOT, "scripts", "workbench_db.py"), + "list-scans", + "--repository", + repository, + ], + { encoding: "utf8", env: environment }, + ); + expect(JSON.parse(history)).toMatchObject({ scans: [] }); + if (process.platform !== "win32") { + expect((await stat(stateDirectory)).mode & 0o777).toBe(0o700); + } + const initialize = mock(() => { + throw new Error("runtime must not initialize"); + }); + const client = new TestClient( + {}, + { + environment, + prepareRuntime: initialize, + resolvePluginPython: initialize, + createCodex: initialize, + }, + ); + + try { + await expect( + client.preflight(repository, { outputDir }), + ).resolves.toMatchObject({ outputDir }); + expect(initialize).not.toHaveBeenCalled(); + expect(existsSync(outputDir)).toBe(false); + } finally { + await client.close(); + } + }); + test("rejects state directories overlapping the selected repository", async () => { const root = await temporaryDirectory(); const repository = join(root, "repository"); @@ -4187,6 +4250,7 @@ describe("CodexSecurity orchestration", () => { process.platform === "win32" ? "junction" : "dir", ); for (const stateDirectory of [ + repository, join(repository, "state"), root, linkedState, @@ -4211,12 +4275,184 @@ describe("CodexSecurity orchestration", () => { client[operation](repository, { outputDir: join(root, "output") }), ).rejects.toBeInstanceOf(OutputInsideProtectedRootError); } - if (stateDirectory !== root && stateDirectory !== linkedState) + if ( + stateDirectory !== repository && + stateDirectory !== root && + stateDirectory !== linkedState + ) expect(existsSync(stateDirectory)).toBe(false); await client.close(); } }); + testPosix( + "rejects unsafe output and state ancestry before runtime initialization", + async () => { + const root = await temporaryDirectory(); + const repository = join(root, "repository"); + const shared = join(root, "shared"); + const privateChild = join(shared, "private"); + const linkedParent = join(root, "linked-parent"); + const safeState = join(root, "state"); + const privateState = join(root, "private-state"); + const stateLink = join(shared, "state-link"); + const indirectState = join(root, "indirect-state"); + const missingState = join(privateChild, "state"); + const linkedState = join(linkedParent, "private", "missing", "state"); + const safeOutput = join(root, "output"); + const unsafeOutput = join(privateChild, "output"); + await mkdir(repository); + await mkdir(privateChild, { recursive: true, mode: 0o700 }); + await mkdir(privateState, { mode: 0o700 }); + await chmod(shared, 0o775); + await symlink(shared, linkedParent, "dir"); + await symlink(privateState, stateLink, "dir"); + await symlink(stateLink, indirectState, "dir"); + + for (const [stateDirectory, outputDir] of [ + [safeState, unsafeOutput], + [shared, undefined], + [shared, safeOutput], + [missingState, undefined], + [missingState, safeOutput], + [linkedState, safeOutput], + [stateLink, safeOutput], + [indirectState, safeOutput], + [join(indirectState, "missing", "state"), safeOutput], + ] as const) { + const initialize = mock(() => { + throw new Error("runtime must not initialize"); + }); + const client = new TestClient( + {}, + { + environment: { CODEX_SECURITY_STATE_DIR: stateDirectory }, + prepareRuntime: initialize, + resolvePluginPython: initialize, + createCodex: initialize, + }, + ); + + try { + for (const operation of ["preflight", "run"] as const) { + await expect( + client[operation](repository, { outputDir }), + ).rejects.toMatchObject({ + name: OutputDirectoryError.name, + message: expect.stringContaining(`${shared} (mode 0775)`), + }); + } + expect(initialize).not.toHaveBeenCalled(); + } finally { + await client.close(); + } + } + + expect((await stat(shared)).mode & 0o7777).toBe(0o775); + expect((await stat(privateChild)).mode & 0o7777).toBe(0o700); + expect((await readdir(shared)).sort()).toEqual(["private", "state-link"]); + expect(await readdir(privateChild)).toEqual([]); + expect(await readdir(privateState)).toEqual([]); + for (const path of [ + safeState, + missingState, + linkedState, + safeOutput, + unsafeOutput, + ]) { + expect(existsSync(path)).toBe(false); + } + }, + ); + + testPosix( + "rejects non-private existing state before runtime initialization", + async () => { + const root = await temporaryDirectory(); + const repository = join(root, "repository"); + const stateDirectory = join(root, "state"); + const outputDir = join(root, "output"); + await mkdir(repository); + await mkdir(stateDirectory, { mode: 0o700 }); + const initialize = mock(() => { + throw new Error("runtime must not initialize"); + }); + const client = new TestClient( + {}, + { + environment: { CODEX_SECURITY_STATE_DIR: stateDirectory }, + prepareRuntime: initialize, + resolvePluginPython: initialize, + createCodex: initialize, + }, + ); + + try { + for (const requestedMode of [0o755, 0o1777]) { + await chmod(stateDirectory, requestedMode); + const mode = (await stat(stateDirectory)).mode & 0o7777; + for (const operation of ["preflight", "run"] as const) { + await expect( + client[operation](repository, { outputDir }), + ).rejects.toThrow( + "Configured Codex Security state directory must be private", + ); + } + expect((await stat(stateDirectory)).mode & 0o7777).toBe(mode); + } + expect(initialize).not.toHaveBeenCalled(); + expect(await readdir(stateDirectory)).toEqual([]); + expect(existsSync(outputDir)).toBe(false); + } finally { + await client.close(); + } + }, + ); + + testPosix( + "preflights persistent state under a sticky shared parent without creating it", + async () => { + const root = await temporaryDirectory(); + const repository = join(root, "repository"); + const outputDir = join(root, "output"); + let stickyParent = join(root, "shared"); + await mkdir(repository); + await mkdir(stickyParent, { mode: 0o1777 }); + await chmod(stickyParent, 0o1777); + if (((await stat(stickyParent)).mode & 0o1000) === 0) { + stickyParent = await realpath(tmpdir()); + if (((await stat(stickyParent)).mode & 0o1000) === 0) return; + } + const parentMode = (await stat(stickyParent)).mode & 0o7777; + const stateDirectory = join(stickyParent, `${basename(root)}-state`); + const initialize = mock(() => { + throw new Error("runtime must not initialize"); + }); + const client = new TestClient( + {}, + { + environment: { CODEX_SECURITY_STATE_DIR: stateDirectory }, + prepareRuntime: initialize, + resolvePluginPython: initialize, + createCodex: initialize, + }, + ); + + try { + await expect( + client.preflight(repository, { outputDir }), + ).resolves.toMatchObject({ outputDir }); + expect(initialize).not.toHaveBeenCalled(); + expect((await stat(stickyParent)).mode & 0o7777).toBe(parentMode); + expect(existsSync(stateDirectory)).toBe(false); + expect(existsSync(outputDir)).toBe(false); + } finally { + await client.close(); + await rm(stateDirectory, { recursive: true, force: true }); + } + }, + ); + test("rejects reruns when the original plugin version is unavailable", async () => { const root = await temporaryDirectory(); const repository = join(root, "repository"); @@ -4240,6 +4476,65 @@ describe("CodexSecurity orchestration", () => { await client.close(); }); + testPosix( + "revalidates state before reusing cached persistent authentication", + async () => { + const root = await temporaryDirectory(); + const repository = join(root, "repository"); + const stateDirectory = join(root, "state"); + const codexHome = join(stateDirectory, "codex-home"); + const outputDir = join(root, "output"); + await mkdir(repository); + await mkdir(codexHome, { recursive: true, mode: 0o700 }); + const authenticationCommand = mock(() => { + throw new Error("authentication command must not start"); + }); + const initialize = mock(() => { + throw new Error("scan must not start"); + }); + const client = new TestClient( + {}, + { + environment: { CODEX_SECURITY_STATE_DIR: stateDirectory }, + prepareRuntime: async () => ({ + ...preparedRuntime(codexHome), + persistentCredentialHome: true, + environment: { + CODEX_HOME: codexHome, + CODEX_SECURITY_STATE_DIR: stateDirectory, + }, + }), + resolveCodexCommand: authenticationCommand, + resolvePluginPython: initialize, + createCodex: initialize, + }, + ); + + try { + await expect( + client.run(repository, { outputDir, expectedPluginVersion: "0.0.0" }), + ).rejects.toThrow("original scan used plugin version"); + await chmod(stateDirectory, 0o755); + for (const operation of [ + () => client.account(), + () => client.logout(), + () => client.loginApiKey("synthetic-key"), + ]) { + await expect(operation()).rejects.toThrow( + "Configured Codex Security state directory must be private", + ); + } + expect(authenticationCommand).not.toHaveBeenCalled(); + expect(initialize).not.toHaveBeenCalled(); + expect((await stat(stateDirectory)).mode & 0o7777).toBe(0o755); + expect(await readdir(codexHome)).toEqual([]); + expect(existsSync(outputDir)).toBe(false); + } finally { + await client.close(); + } + }, + ); + test.each([ ["OpenAI", undefined, "OPENAI_API_KEY", "gpt-5.6-sol", undefined], ...EXTERNAL_PROVIDER_CASES, @@ -4284,11 +4579,20 @@ describe("CodexSecurity orchestration", () => { resolvePluginPython: async () => "/managed/python", prepareOutputDir: async () => scanDir, repositoryRevision: async () => "deadbeef", + runWorkbench: async (options, args, input) => { + expect(options.environment["CODEX_SECURITY_STATE_DIR"]).toBe( + stateDirectory, + ); + return mockWorkbench(args, input); + }, createCodex: (options: CodexOptions) => ({ startThread: () => ({ id: null, async runStreamed() { expect(options.env?.["CODEX_HOME"]).toBe(codexHome); + expect(options.env?.["CODEX_SECURITY_STATE_DIR"]).toBe( + stateDirectory, + ); expect(options.apiKey).toBe( provider === undefined ? "synthetic-transient-key" @@ -4332,6 +4636,13 @@ describe("CodexSecurity orchestration", () => { expect(persistentConfigText).not.toContain("synthetic-transient-key"); const persistentConfig = parseToml(persistentConfigText); expect(persistentConfig["model"]).toBeUndefined(); + expect(persistentConfig).toMatchObject({ + permissions: { + codex_security_scan: { + filesystem: { [stateDirectory]: "write" }, + }, + }, + }); if (provider !== undefined) { expect(persistentConfig).toMatchObject({ model_provider: provider, @@ -5583,6 +5894,7 @@ if ([basename(process.argv[1]), ...process.argv.slice(2)].join(" ") !== "login s await mkdir(codexHome); await mkdir(scanDir, { mode: 0o700 }); const environment: Record = { + CODEX_SECURITY_STATE_DIR: join(root, "state"), OPENAI_API_KEY: "first-key", }; const selectedKeys: Array = []; @@ -5619,6 +5931,7 @@ if ([basename(process.argv[1]), ...process.argv.slice(2)].join(" ") !== "login s await mkdir(repository); await mkdir(codexHome); const environment: Record = { + CODEX_SECURITY_STATE_DIR: join(root, "state"), openai_api_key: "ambient-key", }; const client = new TestClient( diff --git a/sdk/typescript/tests-ts/cli.test.ts b/sdk/typescript/tests-ts/cli.test.ts index fbf7520e1..75eb7c566 100644 --- a/sdk/typescript/tests-ts/cli.test.ts +++ b/sdk/typescript/tests-ts/cli.test.ts @@ -4089,6 +4089,42 @@ describe("CLI", () => { } }); + test("keeps unsafe ancestry errors local and off JSON stdout", async () => { + const failure = new OutputDirectoryError( + "Scan output parent has unsafe permissions (mode 0775).", + ); + + for (const extraArgs of [[], ["--dry-run"]]) { + const stdout = capture(); + const stderr = capture(); + const deps = dependencies(); + deps.createSecurity = () => ({ + run: async () => { + throw failure; + }, + preflight: async () => { + throw failure; + }, + close: async () => {}, + }); + + expect( + await main( + ["scan", ".", "--json", "--verbose", ...extraArgs], + stdout.stream, + stderr.stream, + deps, + ), + ).toBe(2); + expect(stdout.text()).toBe(""); + expect(stderr.text()).toContain(`${failure.message}\n`); + expect(stderr.text()).toContain('scan.failed classification="local"'); + expect(stderr.text()).not.toContain("cannot access the configured model"); + expect(stderr.text()).not.toContain("Authentication failed"); + expect(stderr.text()).not.toContain("model service could not be reached"); + } + }); + test("keeps model authorization advice for genuine transport failures", async () => { // The bypass must not swallow real 401/403 handling, and the advice must // still replace upstream text that can name the organization or project. diff --git a/sdk/typescript/tests-ts/compact-diff-scan.test.ts b/sdk/typescript/tests-ts/compact-diff-scan.test.ts index 106fd38e5..9cd62d9e9 100644 --- a/sdk/typescript/tests-ts/compact-diff-scan.test.ts +++ b/sdk/typescript/tests-ts/compact-diff-scan.test.ts @@ -421,7 +421,7 @@ describe("compact diff scan", () => { const { root, repository } = createRepository(); writeSource(repository, "src/handler.py", "value = 1\n"); mkdirSync(join(root, "scans")); - mkdirSync(join(root, "state")); + mkdirSync(join(root, "state"), { mode: 0o700 }); const client = await startMcp(root); const owner = "preflight-stdin-owner"; @@ -456,7 +456,7 @@ describe("compact diff scan", () => { const { root, repository } = createRepository(); writeSource(repository, "src/handler.py", "value = 1\n"); mkdirSync(join(root, "scans")); - mkdirSync(join(root, "state")); + mkdirSync(join(root, "state"), { mode: 0o700 }); const client = await startMcp(root); const userContext = `--${"é".repeat(64 * 1_024)}`; @@ -516,7 +516,7 @@ describe("compact diff scan", () => { git(repository, "commit", "-qm", "changed"); const headRevision = git(repository, "rev-parse", "HEAD"); mkdirSync(join(root, "scans")); - mkdirSync(join(root, "state")); + mkdirSync(join(root, "state"), { mode: 0o700 }); const client = await startMcp(root); const owner = "compact-diff-owner"; const call = (name: string, args: JsonObject) => diff --git a/sdk/typescript/tests-ts/publication-store.test.ts b/sdk/typescript/tests-ts/publication-store.test.ts index 194230bfb..9c8252d06 100644 --- a/sdk/typescript/tests-ts/publication-store.test.ts +++ b/sdk/typescript/tests-ts/publication-store.test.ts @@ -1,7 +1,15 @@ import { spawnSync } from "node:child_process"; import { randomUUID } from "node:crypto"; import { existsSync } from "node:fs"; -import { mkdir, mkdtemp, realpath, rm } from "node:fs/promises"; +import { + chmod, + mkdir, + mkdtemp, + readdir, + realpath, + rm, + stat, +} from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, describe, expect, test } from "bun:test"; @@ -323,6 +331,51 @@ connection.close() expect(existsSync(fixture.stateDirectory)).toBe(false); }); + test.skipIf(process.platform === "win32")( + "rejects shared state before inspecting a missing database", + async () => { + const fixture = await publicationFixture({ createDatabase: false }); + await mkdir(fixture.stateDirectory, { mode: 0o700 }); + await chmod(fixture.stateDirectory, 0o755); + + await expect( + preparePublicationStore(fixture.publication, fixture.environment), + ).rejects.toThrow(/state directory must be private/u); + + expect(await readdir(fixture.stateDirectory)).toEqual([]); + expect((await stat(fixture.stateDirectory)).mode & 0o777).toBe(0o755); + }, + ); + + test.skipIf(process.platform === "win32")( + "rejects publication reads and writes when existing state becomes shared", + async () => { + const fixture = await publicationFixture({ count: 1 }); + const contents = await readdir(fixture.stateDirectory); + await chmod(fixture.stateDirectory, 0o755); + + await expect( + preparePublicationStore(fixture.publication, fixture.environment), + ).rejects.toThrow(/state directory must be private/u); + await expect( + recordPublishedIssues( + fixture.publication, + [publishedIssue(fixture.publication, 0)], + fixture.environment, + ), + ).rejects.toThrow(/state directory must be private/u); + + expect(await readdir(fixture.stateDirectory)).toEqual(contents); + expect((await stat(fixture.stateDirectory)).mode & 0o777).toBe(0o755); + expect( + databaseRows( + fixture, + "SELECT COUNT(*) AS count FROM finding_publications", + ), + ).toEqual([{ count: 0 }]); + }, + ); + test("rejects a scan absent from existing local scan history", async () => { const fixture = await publicationFixture({ seedScan: false }); diff --git a/sdk/typescript/tests-ts/publish.test.ts b/sdk/typescript/tests-ts/publish.test.ts index abd6b9adf..bcd021c8b 100644 --- a/sdk/typescript/tests-ts/publish.test.ts +++ b/sdk/typescript/tests-ts/publish.test.ts @@ -1,12 +1,17 @@ import { execFileSync } from "node:child_process"; import { createHash, randomUUID } from "node:crypto"; +import { existsSync } from "node:fs"; import { appendFile, + chmod, + mkdir, mkdtemp, readFile, readdir, + realpath, rm, stat, + symlink, writeFile, } from "node:fs/promises"; import { tmpdir } from "node:os"; @@ -268,6 +273,295 @@ async function processHasExited(pid: number): Promise { return false; } +describe("private publication state", () => { + test("does not inspect configured state for previews or empty scans", async () => { + const root = await mkdtemp( + join(tmpdir(), "codex-security-unused-publication-state-"), + ); + temporaryDirectories.push(root); + const marker = join(root, "not-a-directory"); + await writeFile(marker, "preserved\n"); + + for (const scenario of [ + { count: 1, dryRun: true }, + { count: 0, dryRun: false }, + ]) { + const publication = preparedPublication(scenario.count); + const result = await publishScanInternal( + publication.scanDirectory, + { ...OPTIONS, dryRun: scenario.dryRun }, + dependencies( + publication, + {}, + { + environment: { + CODEX_SECURITY_STATE_DIR: join(marker, "state"), + }, + preparePublicationStore: async () => { + throw new Error("Unused publication state must not be opened."); + }, + }, + ), + ); + expect(result.counts.findings).toBe(scenario.count); + } + + expect(await readFile(marker, "utf8")).toBe("preserved\n"); + expect(await readdir(root)).toEqual(["not-a-directory"]); + }); + + test("leaves a missing history directory absent when publication cannot start", async () => { + const publication = preparedPublication(); + let started = false; + const injected = dependencies( + publication, + {}, + { + runCodex: async () => { + started = true; + return { exitCode: 0, stdout: "", stderr: "" }; + }, + }, + ); + delete injected.preparePublicationStore; + const stateDirectory = injected.environment!["CODEX_SECURITY_STATE_DIR"]!; + + await expect( + publishScanInternal(publication.scanDirectory, OPTIONS, injected), + ).rejects.toThrow(/scan-history database does not exist/u); + + expect(started).toBe(false); + expect(existsSync(stateDirectory)).toBe(false); + }); + + test.skipIf(process.platform === "win32")( + "rejects shared state before opening history or contacting a publisher", + async () => { + const stateDirectory = await mkdtemp( + join(tmpdir(), "codex-security-shared-publication-state-"), + ); + temporaryDirectories.push(stateDirectory); + await chmod(stateDirectory, 0o755); + const publication = preparedPublication(); + const calls: string[] = []; + + for (const direct of [false, true]) { + await expect( + publishScanInternal( + publication.scanDirectory, + { + ...OPTIONS, + ...(direct ? { linearApiKey: "synthetic-key" } : {}), + }, + dependencies( + publication, + {}, + { + environment: { CODEX_SECURITY_STATE_DIR: stateDirectory }, + preparePublicationStore: async () => { + calls.push("history"); + }, + linearClient: linearApiClient(publication, { + configured: () => calls.push("client"), + create: () => { + calls.push("create"); + }, + }), + resolveCodex: () => { + calls.push("resolve"); + return { command: "synthetic-codex" }; + }, + runCodex: async () => { + calls.push("publish"); + return { exitCode: 0, stdout: "", stderr: "" }; + }, + }, + ), + ), + ).rejects.toThrow(/state directory must be private/u); + } + + expect(calls).toEqual([]); + expect(await readdir(stateDirectory)).toEqual([]); + expect((await stat(stateDirectory)).mode & 0o777).toBe(0o755); + }, + ); + + test.skipIf(process.platform === "win32")( + "revalidates state before creating a publication handoff", + async () => { + const stateDirectory = await mkdtemp( + join(tmpdir(), "codex-security-publication-handoff-state-"), + ); + temporaryDirectories.push(stateDirectory); + const publication = preparedPublication(); + let started = false; + + await expect( + publishScanInternal( + publication.scanDirectory, + OPTIONS, + dependencies( + publication, + {}, + { + environment: { CODEX_SECURITY_STATE_DIR: stateDirectory }, + preparePublicationStore: async () => { + await chmod(stateDirectory, 0o755); + }, + runCodex: async () => { + started = true; + return { exitCode: 0, stdout: "", stderr: "" }; + }, + }, + ), + ), + ).rejects.toThrow(/state directory must be private/u); + + expect(started).toBe(false); + expect(await readdir(stateDirectory)).toEqual([]); + expect((await stat(stateDirectory)).mode & 0o777).toBe(0o755); + }, + ); + + test("pins a trusted state alias for the full publication", async () => { + const root = await realpath( + await mkdtemp(join(tmpdir(), "codex-security-publication-state-alias-")), + ); + temporaryDirectories.push(root); + const selected = join(root, "selected"); + const other = join(root, "other"); + const alias = join(root, "state-alias"); + await mkdir(selected, { mode: 0o700 }); + await mkdir(other, { mode: 0o700 }); + const linkType = process.platform === "win32" ? "junction" : "dir"; + await symlink(selected, alias, linkType); + const canonicalState = await realpath(selected); + const environment = { CODEX_SECURITY_STATE_DIR: alias }; + const inherited: NodeJS.ProcessEnv[] = []; + const publication = preparedPublication(); + const injected = dependencies( + publication, + {}, + { + environment, + preparePublicationStore: async (_publication, env) => { + inherited.push(env); + }, + resolveCodex: (env) => { + inherited.push(env); + return { command: "synthetic-codex" }; + }, + runCodex: async (_command, _args, input, env) => { + inherited.push(env); + expect(publicationData(input).handoffFile).toStartWith( + join(canonicalState, "publications"), + ); + await rm(alias, { recursive: true, force: true }); + await symlink(other, alias, linkType); + environment.CODEX_SECURITY_STATE_DIR = other; + return { + exitCode: 0, + stdout: issueEvent(publication.issues[0]!), + stderr: "", + }; + }, + recordPublishedIssues: async (_publication, issues, env) => { + inherited.push(env); + return [...issues]; + }, + }, + ); + delete injected.writeReceipt; + + const result = await publishScanInternal( + publication.scanDirectory, + OPTIONS, + injected, + ); + + expect(inherited).toHaveLength(4); + for (const env of inherited) { + expect(env).toBe(inherited[0]!); + expect(env).not.toBe(environment); + expect(env["CODEX_SECURITY_STATE_DIR"]).toBe(canonicalState); + } + expect(environment.CODEX_SECURITY_STATE_DIR).toBe(other); + const digest = createHash("sha256") + .update(publication.scanId) + .digest("hex"); + const receipt = join( + canonicalState, + "publications", + "linear", + `${digest}.json`, + ); + expect(JSON.parse(await readFile(receipt, "utf8"))).toEqual(result); + expect(await readdir(other)).toEqual([]); + }); + + test.skipIf(process.platform === "win32")( + "blocks final and partial receipt writers if state becomes shared", + async () => { + for (const interrupted of [false, true]) { + const stateDirectory = await mkdtemp( + join(tmpdir(), "codex-security-publication-receipt-state-"), + ); + temporaryDirectories.push(stateDirectory); + const publication = preparedPublication(); + const controller = new AbortController(); + let persisted = false; + let receiptWrites = 0; + const pending = publishScanInternal( + publication.scanDirectory, + { ...OPTIONS, signal: controller.signal }, + dependencies( + publication, + {}, + { + environment: { CODEX_SECURITY_STATE_DIR: stateDirectory }, + recordPublishedIssues: async (_publication, issues) => { + persisted = true; + await chmod(stateDirectory, 0o755); + if (interrupted) controller.abort("Publication interrupted."); + return [...issues]; + }, + writeReceipt: async () => { + receiptWrites += 1; + }, + }, + ), + ); + + if (interrupted) { + await expect(pending).rejects.toThrow( + /partial receipt could not be saved: .*state directory must be private/u, + ); + } else { + const result = await pending; + expect(result.counts).toEqual({ + findings: 1, + created: 1, + failed: 0, + }); + expect(result.warnings).toHaveLength(1); + expect(result.warnings![0]).toContain( + "state directory must be private", + ); + expect(result.warnings![0]).toContain("do not retry publication"); + } + + expect(persisted).toBe(true); + expect(receiptWrites).toBe(0); + expect((await stat(stateDirectory)).mode & 0o777).toBe(0o755); + expect( + await readdir(join(stateDirectory, "publications", "linear")), + ).toEqual(["handoffs"]); + } + }, + ); +}); + describe("direct Linear API publication", () => { test("leaves issues unassigned unless an email or user ID is selected", async () => { for (const scenario of [ @@ -631,6 +925,7 @@ describe("connected Linear publication", () => { join(tmpdir(), "codex-security-publication-environment-"), ); temporaryDirectories.push(stateDirectory); + const canonicalState = await realpath(stateDirectory); const environment = { CODEX_HOME: "/existing/connected-codex-home", CODEX_SECURITY_STATE_DIR: stateDirectory, @@ -666,7 +961,7 @@ describe("connected Linear publication", () => { }, writeReceipt: async (receipt, env) => { receiptScanId = receipt.scanId; - expect(env).toBe(environment); + expect(env).toBe(inheritedEnvironment!); }, }, ), @@ -675,7 +970,7 @@ describe("connected Linear publication", () => { expect(command).toBe("synthetic-codex"); const handoffDirectory = args![args!.indexOf("--cd") + 1]!; expect( - handoffDirectory.startsWith(join(stateDirectory, "publications")), + handoffDirectory.startsWith(join(canonicalState, "publications")), ).toBe(true); expect(args).toEqual([ "exec", @@ -694,7 +989,11 @@ describe("connected Linear publication", () => { ]); expect(args).not.toContain("--ignore-user-config"); expect(args).not.toContain("--disable"); - expect(inheritedEnvironment).toBe(environment); + expect(inheritedEnvironment).not.toBe(environment); + expect(inheritedEnvironment).toEqual({ + ...environment, + CODEX_SECURITY_STATE_DIR: canonicalState, + }); expect(input).toContain("already-connected hosted Linear application"); expect(input).toContain("untrusted inert data"); expect(input).toContain("track-findings"); diff --git a/sdk/typescript/tests-ts/runtime.test.ts b/sdk/typescript/tests-ts/runtime.test.ts index 7dbf80c8f..31bd1a48b 100644 --- a/sdk/typescript/tests-ts/runtime.test.ts +++ b/sdk/typescript/tests-ts/runtime.test.ts @@ -40,6 +40,7 @@ import { createMarketplace, extractPluginZip, importAmbientAuth, + OutputDirectoryError, pluginExecutionEnvironment, PluginBootstrapError, PluginPythonUnavailableError, @@ -62,6 +63,7 @@ import { isPythonPathCandidate, planOutputArchive, prepareCodexSecurityCredentialHome, + prepareCodexSecurityStateDirectory, preparePersistentOutputRoot, preserveCodexSecurityPluginRegistration, requirePrivateCredentialHome, @@ -73,6 +75,7 @@ import { runWorkbench, setCodexSecurityCredentialLogout, streamWindowsCredentialAclDescriptors, + validateCodexSecurityStateDirectory, verifyStableWindowsCredentialDescendants, } from "../src/runtime.js"; import { loadBundledRuntime, PLUGIN_ROOT } from "./plugin-root.js"; @@ -2129,6 +2132,7 @@ describe("runtime directories and plugin Python boundary", () => { await expect( requireSecureOutputAncestry(join(shared, "state")), ).rejects.toThrow("sticky bit"); + expect(existsSync(join(shared, "state"))).toBe(false); }, ); @@ -2248,7 +2252,7 @@ describe("runtime directories and plugin Python boundary", () => { test("identifies a credential home that already exists as a regular file", async () => { const root = await temporaryDirectory(); const stateDirectory = join(root, "state"); - await mkdir(stateDirectory); + await prepareCodexSecurityStateDirectory(stateDirectory); await writeFile(join(stateDirectory, "codex-home"), "not a directory\n"); await expect( @@ -2301,6 +2305,374 @@ describe("runtime directories and plugin Python boundary", () => { } }); + test.each(["EPERM", "EBUSY"])( + "retries temporarily unreadable Windows credential-lock owners with %s", + async (code) => { + if ( + runTestInSubprocess( + import.meta.path, + `retries temporarily unreadable Windows credential-lock owners with ${code}`, + ) + ) { + return; + } + const root = await temporaryDirectory(); + const home = join(root, "credential-home"); + const lock = join(home, ".codex-security-scan.lock"); + const ownerPath = join(lock, "owner.json"); + await mkdir(home, { mode: 0o700 }); + const securityOptions = { + platform: "win32" as const, + secureWindowsHome: async () => {}, + }; + const releaseFirst = await acquireCodexSecurityCredentialHomeLock( + home, + undefined, + securityOptions, + ); + const originalReadFile = fsPromises.readFile; + let ownerReads = 0; + let retryObserved!: () => void; + const retried = new Promise((resolve) => { + retryObserved = resolve; + }); + mock.module("node:fs/promises", () => ({ + ...fsPromises, + readFile: async (...args: Parameters) => { + if (String(args[0]) === ownerPath) { + ownerReads += 1; + if (ownerReads === 1) { + throw Object.assign(new Error("temporarily unreadable owner"), { + code, + }); + } + if (ownerReads === 2) retryObserved(); + } + return originalReadFile(...args); + }, + })); + const controller = new AbortController(); + const waiting = acquireCodexSecurityCredentialHomeLock( + home, + controller.signal, + securityOptions, + ); + void waiting.catch(() => undefined); + + try { + await Promise.race([ + retried, + waiting.then(() => { + throw new Error("The held credential lock was acquired early."); + }), + ]); + expect(ownerReads).toBeGreaterThanOrEqual(2); + expect(existsSync(ownerPath)).toBe(true); + await releaseFirst(); + const releaseSecond = await waiting; + await releaseSecond(); + expect(existsSync(lock)).toBe(false); + } finally { + controller.abort(); + mock.module("node:fs/promises", () => ({ + ...fsPromises, + readFile: originalReadFile, + })); + await releaseFirst(); + await waiting.then( + (release) => release(), + () => undefined, + ); + } + }, + ); + + test.each(["EPERM", "EBUSY"])( + "reports persistent Windows credential-lock read failures with %s", + async (code) => { + if ( + runTestInSubprocess( + import.meta.path, + `reports persistent Windows credential-lock read failures with ${code}`, + ) + ) { + return; + } + const root = await temporaryDirectory(); + const home = join(root, "credential-home"); + const ownerPath = join(home, ".codex-security-scan.lock", "owner.json"); + await mkdir(home, { mode: 0o700 }); + const securityOptions = { + platform: "win32" as const, + secureWindowsHome: async () => {}, + }; + const release = await acquireCodexSecurityCredentialHomeLock( + home, + undefined, + securityOptions, + ); + const originalReadFile = fsPromises.readFile; + const failure = Object.assign( + new Error("persistent owner read failure"), + { + code, + }, + ); + let now = Date.now(); + let ownerReads = 0; + const clock = spyOn(Date, "now").mockImplementation(() => now); + mock.module("node:fs/promises", () => ({ + ...fsPromises, + readFile: async (...args: Parameters) => { + if (String(args[0]) === ownerPath) { + ownerReads += 1; + now += 15_000; + throw failure; + } + return originalReadFile(...args); + }, + })); + const controller = new AbortController(); + const watchdog = setTimeout(() => controller.abort(), 1_000); + try { + await expect( + acquireCodexSecurityCredentialHomeLock( + home, + controller.signal, + securityOptions, + ), + ).rejects.toBe(failure); + expect(ownerReads).toBe(2); + expect(existsSync(ownerPath)).toBe(true); + } finally { + clearTimeout(watchdog); + controller.abort(); + clock.mockRestore(); + mock.module("node:fs/promises", () => ({ + ...fsPromises, + readFile: originalReadFile, + })); + await release(); + } + }, + ); + + test("resets Windows credential-lock read retries for a replacement lock", async () => { + if ( + runTestInSubprocess( + import.meta.path, + "resets Windows credential-lock read retries for a replacement lock", + ) + ) { + return; + } + const root = await temporaryDirectory(); + const home = join(root, "credential-home"); + const lock = join(home, ".codex-security-scan.lock"); + const ownerPath = join(lock, "owner.json"); + await mkdir(home, { mode: 0o700 }); + const securityOptions = { + platform: "win32" as const, + secureWindowsHome: async () => {}, + }; + const releaseFirst = await acquireCodexSecurityCredentialHomeLock( + home, + undefined, + securityOptions, + ); + const originalReadFile = fsPromises.readFile; + const originalLstat = fsPromises.lstat; + const controller = new AbortController(); + let now = Date.now(); + const clock = spyOn(Date, "now").mockImplementation(() => now); + let releasing = false; + let replacementCreated = false; + let ownerReads = 0; + let retriedReplacement!: () => void; + const retried = new Promise((resolve) => { + retriedReplacement = resolve; + }); + mock.module("node:fs/promises", () => ({ + ...fsPromises, + lstat: async (...args: Parameters) => { + const metadata = await originalLstat(...args); + if (String(args[0]) === lock && typeof metadata.dev === "bigint") { + Object.defineProperties(metadata, { + dev: { value: replacementCreated ? 2n : 1n }, + ino: { value: replacementCreated ? 4n : 3n }, + }); + } + return metadata; + }, + readFile: async (...args: Parameters) => { + if (String(args[0]) !== ownerPath || releasing) { + return originalReadFile(...args); + } + ownerReads += 1; + if (ownerReads === 1) { + now += 29_999; + throw Object.assign(new Error("original owner is unreadable"), { + code: "EPERM", + }); + } + if (ownerReads === 2) { + releasing = true; + await releaseFirst(); + releasing = false; + await mkdir(lock, { mode: 0o700 }); + replacementCreated = true; + now += 2; + throw Object.assign(new Error("owner replaced during read"), { + code: "EBUSY", + }); + } + if (ownerReads === 3) { + retriedReplacement(); + controller.abort(); + } + throw Object.assign(new Error("replacement owner is busy"), { + code: "EBUSY", + }); + }, + })); + const waiting = acquireCodexSecurityCredentialHomeLock( + home, + controller.signal, + securityOptions, + ); + void waiting.catch(() => undefined); + try { + await Promise.race([ + retried, + waiting.then(() => { + throw new Error( + "The replacement lock was acquired before its owner was ready.", + ); + }), + ]); + await expect(waiting).rejects.toMatchObject({ name: "AbortError" }); + expect(ownerReads).toBe(3); + expect(existsSync(lock)).toBe(true); + expect(existsSync(ownerPath)).toBe(false); + } finally { + controller.abort(); + clock.mockRestore(); + mock.module("node:fs/promises", () => ({ + ...fsPromises, + lstat: originalLstat, + readFile: originalReadFile, + })); + await waiting.then( + (release) => release(), + () => undefined, + ); + await releaseFirst(); + await rm(lock, { recursive: true, force: true }); + } + }); + + test("cancels unreadable Windows credential-lock retries", async () => { + if ( + runTestInSubprocess( + import.meta.path, + "cancels unreadable Windows credential-lock retries", + ) + ) { + return; + } + const root = await temporaryDirectory(); + const home = join(root, "credential-home"); + const ownerPath = join(home, ".codex-security-scan.lock", "owner.json"); + await mkdir(home, { mode: 0o700 }); + const securityOptions = { + platform: "win32" as const, + secureWindowsHome: async () => {}, + }; + const release = await acquireCodexSecurityCredentialHomeLock( + home, + undefined, + securityOptions, + ); + const originalReadFile = fsPromises.readFile; + const controller = new AbortController(); + mock.module("node:fs/promises", () => ({ + ...fsPromises, + readFile: async (...args: Parameters) => { + if (String(args[0]) === ownerPath) { + controller.abort(); + throw Object.assign(new Error("unreadable owner"), { code: "EPERM" }); + } + return originalReadFile(...args); + }, + })); + try { + await expect( + acquireCodexSecurityCredentialHomeLock( + home, + controller.signal, + securityOptions, + ), + ).rejects.toMatchObject({ name: "AbortError" }); + expect(existsSync(ownerPath)).toBe(true); + } finally { + controller.abort(); + mock.module("node:fs/promises", () => ({ + ...fsPromises, + readFile: originalReadFile, + })); + await release(); + } + }); + + test("preserves other Windows credential-lock read errors", async () => { + if ( + runTestInSubprocess( + import.meta.path, + "preserves other Windows credential-lock read errors", + ) + ) { + return; + } + const root = await temporaryDirectory(); + const home = join(root, "credential-home"); + const ownerPath = join(home, ".codex-security-scan.lock", "owner.json"); + await mkdir(home, { mode: 0o700 }); + const securityOptions = { + platform: "win32" as const, + secureWindowsHome: async () => {}, + }; + const release = await acquireCodexSecurityCredentialHomeLock( + home, + undefined, + securityOptions, + ); + const originalReadFile = fsPromises.readFile; + mock.module("node:fs/promises", () => ({ + ...fsPromises, + readFile: async (...args: Parameters) => { + if (String(args[0]) === ownerPath) { + throw Object.assign(new Error("access denied"), { code: "EACCES" }); + } + return originalReadFile(...args); + }, + })); + try { + await expect( + acquireCodexSecurityCredentialHomeLock( + home, + undefined, + securityOptions, + ), + ).rejects.toMatchObject({ code: "EACCES" }); + } finally { + mock.module("node:fs/promises", () => ({ + ...fsPromises, + readFile: originalReadFile, + })); + await release(); + } + }); + test("does not rewrite Windows credential ACLs while polling a held lock", async () => { const root = await temporaryDirectory(); const home = join(root, "credential-home"); @@ -2639,6 +3011,41 @@ describe("runtime directories and plugin Python boundary", () => { }); }); + test("can inspect only a Windows state boundary without walking descendants", async () => { + const root = await temporaryDirectory(); + const state = join(root, "state"); + const external = join(root, "external"); + await mkdir(state); + await mkdir(external); + await symlink( + external, + join(state, "accumulated-scan"), + process.platform === "win32" ? "junction" : "dir", + ); + const sid = "S-1-5-21-111-222-333-1001"; + const directory = `O:${sid}G:SYD:P(A;OICI;FA;;;${sid})`; + const ancestors: string[] = []; + for (let ancestor = dirname(state); ; ancestor = dirname(ancestor)) { + ancestors.push(directory); + if (ancestor === dirname(ancestor)) break; + } + const descriptors = [...ancestors, directory]; + + await expect( + inspectWindowsCredentialAclSnapshot(state, sid, { + command: process.execPath, + args: [ + "--eval", + `process.stdout.write(${JSON.stringify(`${descriptors.join("\n")}\n`)})`, + ], + inspectDescendants: false, + }), + ).resolves.toMatchObject({ + home: { owner: sid, protected: true }, + descendantsArePrivate: true, + }); + }); + test("rejects unsafe Windows credential ancestry during combined ACL inspection", async () => { const root = await temporaryDirectory(); const home = join(root, "home"); @@ -2854,6 +3261,39 @@ describe("runtime directories and plugin Python boundary", () => { ).toThrow("owner is not a trusted principal"); }); + test("rejects Windows grants that can race state-directory creation", () => { + const user = "S-1-5-21-111-222-333-1001"; + for (const [flags, rights] of [ + ["CIIO", "GA"], + ["CI", "0x2"], + ["CI", "0x4"], + ["CI", "0x40000000"], + ["CI", "CC"], + ["", "CC"], + ] as const) { + expect( + inspectWindowsCredentialAcl( + `O:${user}G:SYD:(A;OICI;FA;;;${user})(A;${flags};${rights};;;WD)`, + user, + { scope: "creation-ancestor" }, + ).untrustedPrincipals, + ).toEqual(["S-1-1-0"]); + } + for (const [flags, rights] of [ + ["OIIO", "GA"], + ["CIIO", "FR"], + ["CI", "FR"], + ] as const) { + expect( + inspectWindowsCredentialAcl( + `O:${user}G:SYD:(A;OICI;FA;;;${user})(A;${flags};${rights};;;WD)`, + user, + { scope: "creation-ancestor" }, + ).untrustedPrincipals, + ).toEqual([]); + } + }); + test("accepts private credential-file ACLs without inheritance flags", () => { const user = "S-1-5-21-111-222-333-1001"; const descriptor = `O:${user}G:SYD:P(A;;FA;;;${user})(A;;FA;;;SY)`; @@ -3373,20 +3813,22 @@ describe("runtime directories and plugin Python boundary", () => { async () => { const root = await temporaryDirectory(); const state = join(root, "state"); - await mkdir(state); + await prepareCodexSecurityStateDirectory(state); + const home = join(state, "codex-home"); + await mkdir(home); const systemDirectory = join( process.env["SystemRoot"] ?? "C:\\Windows", "System32", ); const shared = spawnSync( join(systemDirectory, "icacls.exe"), - [state, "/grant", "*S-1-1-0:(OI)(CI)R"], + [home, "/grant", "*S-1-1-0:(OI)(CI)R"], { encoding: "utf8", windowsHide: true }, ); expect(shared.status).toBe(0); - const home = await prepareCodexSecurityCredentialHome({ - CODEX_SECURITY_STATE_DIR: state, + await requirePrivateCredentialHome(await lstat(home), home, { + platform: "win32", }); const result = spawnSync( join(systemDirectory, "WindowsPowerShell", "v1.0", "powershell.exe"), @@ -3421,7 +3863,8 @@ describe("runtime directories and plugin Python boundary", () => { const root = await temporaryDirectory(); const state = join(root, "state"); const home = join(state, "codex-home"); - await mkdir(home, { recursive: true }); + await prepareCodexSecurityStateDirectory(state); + await mkdir(home); const systemDirectory = join( process.env["SystemRoot"] ?? "C:\\Windows", "System32", @@ -3433,11 +3876,9 @@ describe("runtime directories and plugin Python boundary", () => { ); expect(configured.status).toBe(0); - expect( - await prepareCodexSecurityCredentialHome({ - CODEX_SECURITY_STATE_DIR: state, - }), - ).toBe(await realpath(home)); + await requirePrivateCredentialHome(await lstat(home), home, { + platform: "win32", + }); const result = spawnSync( join(systemDirectory, "WindowsPowerShell", "v1.0", "powershell.exe"), [ @@ -3539,6 +3980,7 @@ describe("runtime directories and plugin Python boundary", () => { const state = join(root, "state"); const home = join(state, "codex-home"); const nested = join(home, "sessions"); + await prepareCodexSecurityStateDirectory(state); await mkdir(nested, { recursive: true }); const auth = join(home, "auth.json"); const nestedAuth = join(nested, "credentials.json"); @@ -3567,11 +4009,9 @@ describe("runtime directories and plugin Python boundary", () => { expect(unsafe.status).toBe(0); } - expect( - await prepareCodexSecurityCredentialHome({ - CODEX_SECURITY_STATE_DIR: state, - }), - ).toBe(await realpath(home)); + await requirePrivateCredentialHome(await lstat(home), home, { + platform: "win32", + }); const inspection = spawnSync( join(systemDirectory, "WindowsPowerShell", "v1.0", "powershell.exe"), @@ -3628,16 +4068,18 @@ describe("runtime directories and plugin Python boundary", () => { ), ).rejects.toThrow("Windows-ambiguous components"); } - const scanRoot = await preparePersistentOutputRoot( - join(root, "state"), - "scans", - "repository with spaces", - ); - expect(scanRoot).toBe( - join(root, "state", "scans", "repository-with-spaces"), - ); - if (process.platform !== "win32") { - expect((await stat(scanRoot)).mode & 0o777).toBe(0o700); + for (const category of ["scans", "policies"] as const) { + const outputRoot = await preparePersistentOutputRoot( + join(root, "state"), + category, + "repository with spaces", + ); + expect(outputRoot).toBe( + join(root, "state", category, "repository-with-spaces"), + ); + if (process.platform !== "win32") { + expect((await stat(outputRoot)).mode & 0o777).toBe(0o700); + } } const linkedState = join(root, "linked-state"); @@ -3655,6 +4097,592 @@ describe("runtime directories and plugin Python boundary", () => { ).toBe(join(root, "state", "scans", "linked-repository")); }); + test("validates private state aliases and missing paths without creating them", async () => { + const root = await temporaryDirectory(); + const state = join(root, "state"); + const alias = join(root, "state-alias"); + const missing = join(alias, "missing", "state"); + await prepareCodexSecurityStateDirectory(state); + await writeFile(join(state, "preserved.txt"), "preserved\n"); + await symlink( + state, + alias, + process.platform === "win32" ? "junction" : "dir", + ); + let location: string | undefined; + + expect( + await validateCodexSecurityStateDirectory(alias, (canonical) => { + location = canonical; + }), + ).toBe(state); + expect(location).toBe(state); + expect(await validateCodexSecurityStateDirectory(missing)).toBe( + join(state, "missing", "state"), + ); + expect(existsSync(missing)).toBe(false); + expect(await readdir(state)).toEqual(["preserved.txt"]); + expect(await readFile(join(state, "preserved.txt"), "utf8")).toBe( + "preserved\n", + ); + if (process.platform !== "win32") { + expect((await stat(state)).mode & 0o7777).toBe(0o700); + } + }); + + test("inspects existing Windows state ACLs without repairing them", async () => { + const root = await temporaryDirectory(); + const state = join(root, "state"); + await mkdir(state, { mode: 0o700 }); + const inspections: [ + string, + { + repair: boolean; + creationAncestor?: boolean; + inspectDescendants?: boolean; + }, + ][] = []; + const security = { + platform: "win32" as const, + windowsAcl: async ( + path: string, + options: { + repair: boolean; + creationAncestor?: boolean; + inspectDescendants?: boolean; + }, + ) => { + inspections.push([path, options]); + }, + }; + + expect( + await validateCodexSecurityStateDirectory(state, undefined, security), + ).toBe(state); + expect(inspections).toEqual([ + [state, { repair: false, inspectDescendants: false }], + ]); + const alias = join(root, "state-alias"); + await symlink( + state, + alias, + process.platform === "win32" ? "junction" : "dir", + ); + expect( + await validateCodexSecurityStateDirectory(alias, undefined, security), + ).toBe(state); + expect(inspections.slice(1)).toEqual([ + [alias, { repair: false, inspectDescendants: false }], + [state, { repair: false, inspectDescendants: false }], + ]); + + const unsafe = new Error("unsafe synthetic Windows state ACL"); + security.windowsAcl = async (path, options) => { + inspections.push([path, options]); + throw unsafe; + }; + await expect( + prepareCodexSecurityStateDirectory(state, undefined, security), + ).rejects.toMatchObject({ cause: unsafe }); + expect(inspections.at(-1)).toEqual([ + state, + { repair: false, inspectDescendants: false }, + ]); + expect(await readdir(state)).toEqual([]); + }); + + test("rejects unsafe Windows creation ancestors during non-mutating validation", async () => { + const root = await temporaryDirectory(); + type AclOptions = { + repair: boolean; + creationAncestor?: boolean; + inspectDescendants?: boolean; + }; + const options: AclOptions = { + repair: false, + creationAncestor: true, + inspectDescendants: false, + }; + const inspections: [string, AclOptions][] = []; + const security = { + platform: "win32" as const, + windowsAcl: async (path: string, aclOptions: AclOptions) => { + inspections.push([path, aclOptions]); + }, + }; + const directState = join(root, "direct", "state"); + + expect( + await validateCodexSecurityStateDirectory( + directState, + undefined, + security, + ), + ).toBe(directState); + expect(inspections).toEqual([[root, options]]); + expect(existsSync(join(root, "direct"))).toBe(false); + + const target = join(root, "target"); + const alias = join(root, "target-alias"); + const state = join(alias, "missing", "state"); + await mkdir(target, { mode: 0o700 }); + await symlink( + target, + alias, + process.platform === "win32" ? "junction" : "dir", + ); + inspections.length = 0; + const unsafe = new Error("unsafe synthetic Windows creation ancestor"); + security.windowsAcl = async (path, aclOptions) => { + inspections.push([path, aclOptions]); + if (path === target) throw unsafe; + }; + + await expect( + validateCodexSecurityStateDirectory(state, undefined, security), + ).rejects.toMatchObject({ cause: unsafe }); + expect(inspections).toEqual([ + [alias, options], + [target, options], + ]); + expect(existsSync(join(target, "missing"))).toBe(false); + }); + + test("repairs only Windows state directories created during preparation", async () => { + const root = await temporaryDirectory(); + const state = join(root, "missing", "state"); + const inspections: [ + string, + { + repair: boolean; + creationAncestor?: boolean; + inspectDescendants?: boolean; + }, + ][] = []; + const security = { + platform: "win32" as const, + windowsAcl: async ( + path: string, + options: { + repair: boolean; + creationAncestor?: boolean; + inspectDescendants?: boolean; + }, + ) => { + inspections.push([path, options]); + }, + }; + + expect( + await prepareCodexSecurityStateDirectory(state, undefined, security), + ).toBe(state); + expect(inspections).toEqual([ + [ + root, + { + repair: false, + creationAncestor: true, + inspectDescendants: false, + }, + ], + [ + root, + { + repair: false, + creationAncestor: true, + inspectDescendants: false, + }, + ], + [join(root, "missing"), { repair: true, inspectDescendants: false }], + [state, { repair: true, inspectDescendants: false }], + [state, { repair: false, inspectDescendants: false }], + ]); + }); + + test("rejects unsafe Windows state creation ancestors before mkdir", async () => { + const root = await temporaryDirectory(); + const state = join(root, "missing", "state"); + const unsafe = new Error("unsafe synthetic Windows creation ancestor"); + const inspections: [ + string, + { + repair: boolean; + creationAncestor?: boolean; + inspectDescendants?: boolean; + }, + ][] = []; + + await expect( + prepareCodexSecurityStateDirectory(state, undefined, { + platform: "win32", + windowsAcl: async (path, options) => { + inspections.push([path, options]); + if (options.creationAncestor === true) throw unsafe; + }, + }), + ).rejects.toMatchObject({ cause: unsafe }); + expect(inspections).toEqual([ + [ + root, + { + repair: false, + creationAncestor: true, + inspectDescendants: false, + }, + ], + ]); + expect(existsSync(join(root, "missing"))).toBe(false); + }); + + test.skipIf(process.platform !== "win32")( + "creates private Windows state ACLs and never repairs existing state", + async () => { + const root = await temporaryDirectory(); + const state = join(root, "missing", "state"); + expect(await prepareCodexSecurityStateDirectory(state)).toBe(state); + const systemDirectory = join( + process.env["SystemRoot"] ?? "C:\\Windows", + "System32", + ); + const identity = spawnSync( + join(systemDirectory, "whoami.exe"), + ["/user", "/fo", "csv", "/nh"], + { encoding: "utf8", windowsHide: true }, + ); + expect(identity.status).toBe(0); + const sid = /"(S-1-(?:\d+-)*\d+)"\s*$/u.exec(identity.stdout)?.[1]; + expect(sid).toBeDefined(); + const powershell = join( + systemDirectory, + "WindowsPowerShell", + "v1.0", + "powershell.exe", + ); + const inspect = (): { + protected: boolean; + principals: string[]; + } => { + const result = spawnSync( + powershell, + [ + "-NoLogo", + "-NoProfile", + "-NonInteractive", + "-Command", + [ + "$acl = Get-Acl -LiteralPath $env:CODEX_SECURITY_TEST_ACL_PATH", + "$principals = @($acl.Access | Where-Object { $_.AccessControlType -eq 'Allow' } | ForEach-Object { $_.IdentityReference.Translate([System.Security.Principal.SecurityIdentifier]).Value })", + "[pscustomobject]@{ protected = $acl.AreAccessRulesProtected; principals = $principals } | ConvertTo-Json -Compress", + ].join("; "), + ], + { + encoding: "utf8", + env: { ...process.env, CODEX_SECURITY_TEST_ACL_PATH: state }, + windowsHide: true, + }, + ); + expect(result.status).toBe(0); + return JSON.parse(result.stdout) as { + protected: boolean; + principals: string[]; + }; + }; + const privateAcl = inspect(); + expect(privateAcl.protected).toBe(true); + expect(new Set(privateAcl.principals)).toEqual( + new Set([sid!, "S-1-5-18", "S-1-5-32-544"]), + ); + + const shared = spawnSync( + join(systemDirectory, "icacls.exe"), + [state, "/grant", "*S-1-1-0:(OI)(CI)R"], + { encoding: "utf8", windowsHide: true }, + ); + expect(shared.status).toBe(0); + await expect( + validateCodexSecurityStateDirectory(state), + ).rejects.toBeInstanceOf(OutputDirectoryError); + await expect( + prepareCodexSecurityStateDirectory(state), + ).rejects.toBeInstanceOf(OutputDirectoryError); + expect(inspect().principals).toContain("S-1-1-0"); + }, + ); + + test("prepares a canonical private state root without replacing existing data", async () => { + const root = await temporaryDirectory(); + const state = join(root, "missing", "state"); + const alias = join(root, "state-alias"); + expect(await prepareCodexSecurityStateDirectory(state)).toBe(state); + await writeFile(join(state, "preserved.txt"), "preserved\n"); + await symlink( + state, + alias, + process.platform === "win32" ? "junction" : "dir", + ); + + expect(await prepareCodexSecurityStateDirectory(alias)).toBe(state); + expect(await readFile(join(state, "preserved.txt"), "utf8")).toBe( + "preserved\n", + ); + if (process.platform !== "win32") { + expect((await stat(state)).mode & 0o7777).toBe(0o700); + } + }); + + testPosix( + "changes owner permissions only on newly created state roots", + async () => { + const root = await temporaryDirectory(); + const existing = join(root, "existing"); + await mkdir(existing, { mode: 0o755 }); + await chmod(existing, 0o755); + for (const mask of [0o700, 0o777]) { + const first = join(existing, `nested-${mask.toString(8)}`); + const second = join(first, "parent"); + const state = join(second, "state"); + const previousUmask = process.umask(mask); + try { + expect(await prepareCodexSecurityStateDirectory(state)).toBe(state); + } finally { + process.umask(previousUmask); + } + for (const directory of [first, second, state]) { + expect((await stat(directory)).mode & 0o7777).toBe(0o700); + } + expect((await stat(existing)).mode & 0o7777).toBe(0o755); + await chmod(state, 0o755); + await expect(prepareCodexSecurityStateDirectory(state)).rejects.toThrow( + "Configured Codex Security state directory must be private", + ); + expect((await stat(state)).mode & 0o7777).toBe(0o755); + } + }, + ); + + testPosix( + "revalidates colliding state components without changing or redirecting them", + async () => { + if ( + runTestInSubprocess( + import.meta.path, + "revalidates colliding state components without changing or redirecting them", + ) + ) { + return; + } + const root = await temporaryDirectory(); + const parent = join(root, "parent"); + const state = join(parent, "state"); + const nonprivateState = join(root, "nonprivate-state"); + const alias = join(root, "alias"); + const destination = join(root, "destination"); + await mkdir(destination, { mode: 0o700 }); + const pending = new Set([parent, nonprivateState, alias]); + const originalMkdir = fsPromises.mkdir; + const originalChmod = fsPromises.chmod; + const changed: string[] = []; + mock.module("node:fs/promises", () => ({ + ...fsPromises, + mkdir: async (...args: Parameters) => { + const path = String(args[0]); + if (!pending.delete(path)) return await originalMkdir(...args); + if (path === alias) { + await symlink(destination, alias, "dir"); + } else { + await originalMkdir(path, { mode: 0o755 }); + await originalChmod(path, 0o755); + } + throw Object.assign(new Error("Directory already exists."), { + code: "EEXIST", + }); + }, + chmod: async (...args: Parameters) => { + changed.push(String(args[0])); + return await originalChmod(...args); + }, + })); + const previousUmask = process.umask(0o777); + try { + const locations: [string, boolean][] = []; + expect( + await prepareCodexSecurityStateDirectory(state, (path) => { + locations.push([path, existsSync(path)]); + }), + ).toBe(state); + expect(locations[0]).toEqual([state, false]); + expect(locations.at(-1)).toEqual([state, true]); + expect((await stat(parent)).mode & 0o7777).toBe(0o755); + expect((await stat(state)).mode & 0o7777).toBe(0o700); + + await expect( + prepareCodexSecurityStateDirectory(nonprivateState), + ).rejects.toThrow( + "Configured Codex Security state directory must be private", + ); + expect((await stat(nonprivateState)).mode & 0o7777).toBe(0o755); + process.umask(previousUmask); + await expect( + prepareCodexSecurityStateDirectory(join(alias, "state")), + ).rejects.toThrow("state directory changed during preparation"); + expect(existsSync(join(destination, "state"))).toBe(false); + expect(pending.size).toBe(0); + expect(changed).toEqual([state]); + } finally { + process.umask(previousUmask); + mock.module("node:fs/promises", () => ({ + ...fsPromises, + mkdir: originalMkdir, + chmod: originalChmod, + })); + } + }, + ); + + testPosix( + "rejects non-private state before preparing credentials or output roots", + async () => { + const root = await temporaryDirectory(); + const state = join(root, "state"); + await mkdir(state, { mode: 0o700 }); + await chmod(state, 0o755); + + await expect( + prepareCodexSecurityCredentialHome({ CODEX_SECURITY_STATE_DIR: state }), + ).rejects.toBeInstanceOf(OutputDirectoryError); + for (const category of ["scans", "policies"] as const) { + await expect( + preparePersistentOutputRoot(state, category, "repository"), + ).rejects.toBeInstanceOf(OutputDirectoryError); + } + expect((await stat(state)).mode & 0o7777).toBe(0o755); + expect(await readdir(state)).toEqual([]); + }, + ); + + testPosix( + "requires existing configured state roots to be private", + async () => { + const root = await temporaryDirectory(); + const state = join(root, "state"); + await mkdir(state, { mode: 0o700 }); + await writeFile(join(state, "preserved.txt"), "preserved\n"); + + for (const requestedMode of [0o755, 0o775, 0o1777]) { + await chmod(state, requestedMode); + const mode = (await stat(state)).mode & 0o7777; + await expect( + validateCodexSecurityStateDirectory(state), + ).rejects.toThrow( + `${state} (mode ${mode.toString(8).padStart(4, "0")})`, + ); + await expect( + validateCodexSecurityStateDirectory(state), + ).rejects.toThrow("only if you own it and can safely change it"); + expect((await stat(state)).mode & 0o7777).toBe(mode); + } + expect(() => + requirePrivateOutputDirectory( + { mode: 0o41777, uid: 1000 }, + "state", + 1000, + ), + ).toThrow("must not be accessible to other users"); + expect(await readdir(state)).toEqual(["preserved.txt"]); + }, + ); + + testPosix( + "reports foreign-owned state roots without permission-change advice", + async () => { + if ( + runTestInSubprocess( + import.meta.path, + "reports foreign-owned state roots without permission-change advice", + ) + ) { + return; + } + const root = await temporaryDirectory(); + const state = join(root, "state"); + await mkdir(state, { mode: 0o700 }); + const originalLstat = fsPromises.lstat; + const originalMetadata = await originalLstat(state); + const foreignUid = originalMetadata.uid === 0 ? 1 : 0; + mock.module("node:fs/promises", () => ({ + ...fsPromises, + lstat: async (...args: Parameters) => { + const metadata = await originalLstat(...args); + if (String(args[0]) === state) { + Object.defineProperty(metadata, "uid", { value: foreignUid }); + } + return metadata; + }, + })); + + try { + const failure = await validateCodexSecurityStateDirectory(state).then( + () => undefined, + (error: unknown) => error, + ); + expect(failure).toBeInstanceOf(OutputDirectoryError); + if (!(failure instanceof OutputDirectoryError)) { + throw new Error("foreign-owned state must be rejected"); + } + expect(failure.message).toContain("must be owned by the current user"); + expect(failure.message).toContain(`${state} (mode 0700)`); + expect(failure.message).toContain("CODEX_SECURITY_STATE_DIR"); + expect(failure.message).not.toContain("chmod"); + expect(failure.message).not.toContain("permissions to 0700"); + expect(failure.cause).toBeInstanceOf(OutputDirectoryError); + expect(failure.cause).toMatchObject({ + message: expect.stringContaining("must be owned by the current user"), + }); + } finally { + mock.module("node:fs/promises", () => ({ + ...fsPromises, + lstat: originalLstat, + })); + } + expect((await originalLstat(state)).uid).toBe(originalMetadata.uid); + expect((await originalLstat(state)).mode & 0o7777).toBe(0o700); + expect(await readdir(state)).toEqual([]); + }, + ); + + testPosix("rejects unsafe lexical and chained state aliases", async () => { + const root = await temporaryDirectory(); + const state = join(root, "state"); + const shared = join(root, "shared"); + const trusted = join(root, "trusted"); + const unsafeLink = join(shared, "state-link"); + const nextLink = join(trusted, "next-link"); + const alias = join(root, "state-alias"); + const missing = join(alias, "missing", "state"); + await mkdir(state, { mode: 0o700 }); + await mkdir(shared, { mode: 0o700 }); + await mkdir(trusted, { mode: 0o700 }); + await chmod(shared, 0o775); + await symlink(state, unsafeLink, "dir"); + await symlink(unsafeLink, nextLink, "dir"); + await symlink(`${nextLink}${sep}`, alias, "dir"); + + for (const path of [unsafeLink, nextLink, alias, missing]) { + await expect(validateCodexSecurityStateDirectory(path)).rejects.toThrow( + `${shared} (mode 0775)`, + ); + await expect(prepareCodexSecurityStateDirectory(path)).rejects.toThrow( + `${shared} (mode 0775)`, + ); + } + expect((await stat(shared)).mode & 0o7777).toBe(0o775); + expect((await stat(state)).mode & 0o7777).toBe(0o700); + expect(await readdir(state)).toEqual([]); + expect(await readdir(shared)).toEqual(["state-link"]); + expect(await readdir(trusted)).toEqual(["next-link"]); + expect(existsSync(missing)).toBe(false); + }); + test("rejects symbolic children beneath persistent scan state", async () => { const root = await temporaryDirectory(); const external = join(root, "external"); @@ -3666,7 +4694,8 @@ describe("runtime directories and plugin Python boundary", () => { ] as const) { const state = join(root, `state-${name}`); const linked = join(state, path); - await mkdir(dirname(linked), { recursive: true }); + await prepareCodexSecurityStateDirectory(state); + await mkdir(dirname(linked), { recursive: true, mode: 0o700 }); await symlink( external, linked, @@ -3941,6 +4970,7 @@ describe("runtime directories and plugin Python boundary", () => { pluginRoot, environment: { PATH: process.env["PATH"], + CODEX_SECURITY_STATE_DIR: join(root, "state"), OPENAI_API_KEY: "must-not-reach-python", CODEX_API_KEY: "also-must-not-reach-python", OPENROUTER_API_KEY: "openrouter-must-not-reach-python", @@ -3956,13 +4986,79 @@ describe("runtime directories and plugin Python boundary", () => { expect(result["details"]).toHaveLength(5 * 1024 * 1024); }); + test("pins canonical state without preparing it for workbench commands", async () => { + const root = await temporaryDirectory(); + const pluginRoot = join(root, "plugin"); + const state = join(root, "state"); + const alias = join(root, "state-alias"); + await mkdir(join(pluginRoot, "scripts"), { recursive: true }); + await writeFile( + join(pluginRoot, "scripts", "workbench_db.py"), + [ + "import json, os, sys", + "print(json.dumps({'command': sys.argv[1], 'state': os.environ.get('CODEX_SECURITY_STATE_DIR'), 'stateKeys': sorted(name for name in os.environ if name.upper() == 'CODEX_SECURITY_STATE_DIR')}))", + ].join("\n"), + ); + const python = Bun.which("python3") ?? Bun.which("python"); + expect(python).not.toBeNull(); + const options = { + python: python!, + pluginRoot, + environment: { CODEX_SECURITY_STATE_DIR: state }, + }; + + for (const command of ["inspect-target", "inspect-setup"]) { + expect(await runWorkbench(options, [command])).toMatchObject({ + command, + state, + }); + expect(existsSync(state)).toBe(false); + } + expect(await runWorkbench(options, ["list-scans"])).toMatchObject({ + state, + stateKeys: ["CODEX_SECURITY_STATE_DIR"], + }); + expect(existsSync(state)).toBe(false); + await prepareCodexSecurityStateDirectory(state); + await symlink( + state, + alias, + process.platform === "win32" ? "junction" : "dir", + ); + const aliased = { + ...options, + environment: { + CODEX_SECURITY_STATE_DIR: alias, + codex_security_state_dir: join(root, "unused"), + }, + }; + expect(await runWorkbench(aliased, ["list-scans"])).toMatchObject({ + state, + stateKeys: ["CODEX_SECURITY_STATE_DIR"], + }); + expect(aliased.environment.CODEX_SECURITY_STATE_DIR).toBe(alias); + expect(existsSync(join(root, "unused"))).toBe(false); + if (process.platform !== "win32") { + expect((await stat(state)).mode & 0o7777).toBe(0o700); + await chmod(state, 0o755); + await expect( + runWorkbench(options, ["list-scans"]), + ).rejects.toBeInstanceOf(OutputDirectoryError); + expect(await runWorkbench(options, ["inspect-target"])).toMatchObject({ + state, + }); + expect((await stat(state)).mode & 0o7777).toBe(0o755); + expect(await readdir(state)).toEqual([]); + } + }); + test("upgrades colliding legacy execution-profile and public CLI migrations", async () => { const root = await temporaryDirectory("codex-security-legacy-migrations-"); const repository = join(root, "repository"); const stateDirectory = join(root, "state"); const scanDirectory = join(root, "scan"); await mkdir(repository); - await mkdir(stateDirectory); + await prepareCodexSecurityStateDirectory(stateDirectory); await mkdir(scanDirectory, { mode: 0o700 }); const python = Bun.which("python3") ?? Bun.which("python"); @@ -4124,7 +5220,7 @@ describe("runtime directories and plugin Python boundary", () => { "codex-security-migration-history-", ); const stateDirectory = join(root, "state"); - await mkdir(stateDirectory); + await prepareCodexSecurityStateDirectory(stateDirectory); const database = join(stateDirectory, "workbench.sqlite3"); const python = Bun.which("python3") ?? Bun.which("python"); expect(python).not.toBeNull(); @@ -4244,7 +5340,7 @@ describe("runtime directories and plugin Python boundary", () => { const stateDirectory = join(root, "state"); const scanDirectory = join(root, "scan"); await mkdir(repository); - await mkdir(stateDirectory); + await prepareCodexSecurityStateDirectory(stateDirectory); await mkdir(scanDirectory, { mode: 0o700 }); const python = Bun.which("python3") ?? Bun.which("python"); @@ -4639,6 +5735,34 @@ describe("runtime directories and plugin Python boundary", () => { }, ); + testPosix( + "explains unsafe ancestry above a private output directory without changing it", + async () => { + const root = await temporaryDirectory(); + const shared = join(root, "shared"); + const privateChild = join(shared, "private"); + const output = join(privateChild, "results"); + await mkdir(privateChild, { recursive: true, mode: 0o700 }); + await chmod(shared, 0o775); + + await expect(validateOutputDir(privateChild)).rejects.toThrow( + `${shared} (mode 0775)`, + ); + await expect(validateOutputDir(output)).rejects.toThrow( + "A private child directory does not make an unsafe ancestor safe", + ); + await expect(prepareOutputDir(output, "repository")).rejects.toThrow( + "Choose a location with secure parent directories", + ); + await expect(requireSecureOutputAncestry(output)).rejects.toThrow( + "only if you own it and can safely change it", + ); + expect((await lstat(shared)).mode & 0o7777).toBe(0o775); + expect((await lstat(privateChild)).mode & 0o7777).toBe(0o700); + expect(existsSync(output)).toBe(false); + }, + ); + testPosix( "accepts scan output under a sticky shared parent directory", async () => { diff --git a/sdk/typescript/tests-ts/support/api-client.ts b/sdk/typescript/tests-ts/support/api-client.ts index 851a440cd..24009de86 100644 --- a/sdk/typescript/tests-ts/support/api-client.ts +++ b/sdk/typescript/tests-ts/support/api-client.ts @@ -1,4 +1,8 @@ -import { CodexSecurity } from "../../src/api.js"; +import { mkdtempSync, realpathSync } from "node:fs"; +import { rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { CodexSecurity, environmentValue } from "../../src/api.js"; import type { JsonObject } from "../../src/config.js"; type ClientArguments = ConstructorParameters; @@ -57,23 +61,54 @@ export function mockWorkbench( } export class TestClient extends CodexSecurity { + readonly #temporaryStateDirectory: string | undefined; + public constructor( config: ClientArguments[0], dependencies: Partial, ) { + let environment = dependencies.environment ?? {}; + let temporaryStateDirectory: string | undefined; + if ( + !["CODEX_SECURITY_STATE_DIR", "CODEX_HOME", "HOME", "USERPROFILE"].some( + (name) => environmentValue(environment, name) !== undefined, + ) + ) { + temporaryStateDirectory = realpathSync( + mkdtempSync(join(tmpdir(), "codex-security-api-state-")), + ); + environment = { + ...environment, + CODEX_SECURITY_STATE_DIR: temporaryStateDirectory, + }; + } super( config, { createCodex: () => { throw new Error("Unexpected Codex invocation in test"); }, - environment: {}, runWorkbench: async (_options, args, input) => mockWorkbench(args, input), ...dependencies, + environment, }, { surface: "sdk" }, ); + this.#temporaryStateDirectory = temporaryStateDirectory; + } + + public override async close(): Promise { + try { + await super.close(); + } finally { + if (this.#temporaryStateDirectory !== undefined) { + await rm(this.#temporaryStateDirectory, { + recursive: true, + force: true, + }); + } + } } } diff --git a/sdk/typescript/tests-ts/workbench-state.test.ts b/sdk/typescript/tests-ts/workbench-state.test.ts new file mode 100644 index 000000000..89d0762d1 --- /dev/null +++ b/sdk/typescript/tests-ts/workbench-state.test.ts @@ -0,0 +1,617 @@ +import { spawnSync } from "node:child_process"; +import { existsSync } from "node:fs"; +import { + chmod, + mkdir, + mkdtemp, + readdir, + realpath, + rm, + stat, + symlink, + writeFile, +} from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join, sep } from "node:path"; +import { afterEach, expect, test } from "bun:test"; +import { PLUGIN_ROOT } from "./plugin-root.js"; + +const temporaryDirectories: string[] = []; +const testPosix = process.platform === "win32" ? test.skip : test; +const testWindows = process.platform === "win32" ? test : test.skip; +let windowsUserSid: string | undefined; + +afterEach(async () => { + await Promise.all( + temporaryDirectories + .splice(0) + .map((path) => rm(path, { recursive: true, force: true })), + ); +}); + +async function temporaryDirectory(): Promise { + const path = await realpath( + await mkdtemp(join(tmpdir(), "codex-security-workbench-state-")), + ); + temporaryDirectories.push(path); + return path; +} + +function runPython(stateDirectory: string, args: string[]) { + const python = Bun.which("python3") ?? Bun.which("python") ?? Bun.which("py"); + if (python === null) throw new Error("A Python interpreter is required."); + return spawnSync(python, ["-I", "-B", ...args], { + encoding: "utf8", + timeout: 30_000, + env: { + PATH: process.env["PATH"], + SystemRoot: process.env["SystemRoot"], + CODEX_SECURITY_STATE_DIR: stateDirectory, + }, + }); +} + +function connectDirectly(stateDirectory: string) { + return runPython(stateDirectory, [ + "-c", + [ + "import sys", + "sys.path.insert(0, sys.argv[1])", + "import workbench_db as workbench", + "workbench.connect().close()", + ].join("\n"), + join(PLUGIN_ROOT, "scripts"), + ]); +} + +function windowsSystemDirectory(): string { + return join(process.env["SystemRoot"] ?? "C:\\Windows", "System32"); +} + +function currentWindowsUserSid(): string { + if (windowsUserSid !== undefined) return windowsUserSid; + const result = spawnSync( + join(windowsSystemDirectory(), "whoami.exe"), + ["/user", "/fo", "csv", "/nh"], + { encoding: "utf8", windowsHide: true }, + ); + expect(result.status).toBe(0); + const sid = /"(S-1-(?:\d+-)*\d+)"\s*$/u.exec(result.stdout)?.[1]; + expect(sid).toBeDefined(); + windowsUserSid = sid!; + return windowsUserSid; +} + +function runIcacls(path: string, args: string[]) { + const result = spawnSync( + join(windowsSystemDirectory(), "icacls.exe"), + [path, ...args], + { encoding: "utf8", windowsHide: true }, + ); + expect(result.status).toBe(0); +} + +function protectWindowsDirectory(path: string): void { + runIcacls(path, [ + "/inheritance:r", + "/grant:r", + `*${currentWindowsUserSid()}:(OI)(CI)F`, + "*S-1-5-18:(OI)(CI)F", + "*S-1-5-32-544:(OI)(CI)F", + ]); +} + +function windowsAclSddl(path: string): string { + const powershell = join( + windowsSystemDirectory(), + "WindowsPowerShell", + "v1.0", + "powershell.exe", + ); + const result = spawnSync( + powershell, + [ + "-NoLogo", + "-NoProfile", + "-NonInteractive", + "-Command", + "Microsoft.PowerShell.Security\\Get-Acl -LiteralPath $env:CODEX_SECURITY_TEST_ACL_PATH | Microsoft.PowerShell.Utility\\Select-Object -ExpandProperty Sddl", + ], + { + encoding: "utf8", + env: { ...process.env, CODEX_SECURITY_TEST_ACL_PATH: path }, + windowsHide: true, + }, + ); + expect(result.status).toBe(0); + return result.stdout.trim(); +} + +function expectPrivateWindowsDirectory(path: string): void { + const descriptor = windowsAclSddl(path); + expect(descriptor).toMatch(/D:[A-Z_]*P/u); + expect(descriptor).toContain(`;FA;;;${currentWindowsUserSid()})`); + expect(descriptor).toContain("OICI"); +} + +test("direct workbench initialization creates and pins private state", async () => { + const root = await temporaryDirectory(); + const actual = join(root, "actual"); + const alias = join(root, "alias"); + await mkdir(actual, { mode: 0o700 }); + await symlink( + actual, + alias, + process.platform === "win32" ? "junction" : "dir", + ); + for (const mask of [0o002, 0o700]) { + const nested = `nested-${mask.toString(8)}`; + const state = `${alias}${sep}.${sep}${nested}${sep}state`; + const canonical = join(actual, nested, "state"); + const result = runPython(state, [ + "-c", + [ + "import json, os, sys", + "os.umask(int(sys.argv[2], 8))", + "sys.path.insert(0, sys.argv[1])", + "import workbench_db as workbench", + "workbench.connect().close()", + 'print(json.dumps({"state": str(workbench.state_dir()), "configured": os.environ["CODEX_SECURITY_STATE_DIR"]}))', + ].join("\n"), + join(PLUGIN_ROOT, "scripts"), + mask.toString(8), + ]); + + expect(result.status).toBe(0); + expect(result.stderr).toBe(""); + const paths = JSON.parse(result.stdout) as { + state: string; + configured: string; + }; + expect(paths.configured).toBe(paths.state); + expect(await realpath(paths.state)).toBe(await realpath(canonical)); + expect(existsSync(join(canonical, "workbench.sqlite3"))).toBe(true); + if (process.platform !== "win32") { + expect((await stat(join(actual, nested))).mode & 0o777).toBe(0o700); + expect((await stat(canonical)).mode & 0o777).toBe(0o700); + expect( + (await stat(join(canonical, "workbench.sqlite3"))).mode & 0o777, + ).toBe(0o600); + } + } +}); + +test("Windows state creation ACLs distinguish metadata from directory writes", () => { + const result = runPython("", [ + "-c", + [ + "import json, sys", + "sys.path.insert(0, sys.argv[1])", + "import workbench_db as workbench", + "user = 'S-1-5-21-1-2-3-1001'", + "results = {}", + "for mask in (0x10, 0x100, 0x2, 0x4, 0x40, 0x40000000):", + " record = {'owner': user, 'control': workbench.WINDOWS_DACL_PRESENT, 'rules': [{'type': 0, 'flags': 0, 'mask': mask, 'sid': 'S-1-1-0'}]}", + " try:", + " workbench.require_windows_state_acl(record, user, 'creation-parent')", + " except RuntimeError:", + " results[hex(mask)] = 'rejected'", + " else:", + " results[hex(mask)] = 'accepted'", + "print(json.dumps(results, sort_keys=True))", + ].join("\n"), + join(PLUGIN_ROOT, "scripts"), + ]); + + expect(result.status).toBe(0); + expect(result.stderr).toBe(""); + expect(JSON.parse(result.stdout)).toEqual({ + "0x10": "accepted", + "0x100": "accepted", + "0x2": "rejected", + "0x4": "rejected", + "0x40": "rejected", + "0x40000000": "rejected", + }); +}); + +test("Windows state inspection tolerates both missing Get-Acl sidecar errors", () => { + const result = runPython("", [ + "-c", + [ + "import json, sys", + "from pathlib import Path", + "from unittest.mock import patch", + "sys.path.insert(0, sys.argv[1])", + "import workbench_db as workbench", + "scripts = []", + "def inspect(command, arguments, environment):", + " scripts.append(arguments[-1])", + " return json.dumps({'kind': 'root'})", + "context = (Path('powershell.exe'), Path('icacls.exe'), 'S-1-5-21-1-2-3-1001', {})", + "with patch.object(workbench, 'run_windows_state_acl_command', side_effect=inspect):", + " workbench.windows_state_acl_records(Path('state'), context, workbench_files=True)", + "print(json.dumps({'missing': 'GetAcl_PathNotFound*' in scripts[0], 'narrow': 'GetAcl_PathNotFound,*' in scripts[0]}))", + ].join("\n"), + join(PLUGIN_ROOT, "scripts"), + ]); + + expect(result.status).toBe(0); + expect(result.stderr).toBe(""); + expect(JSON.parse(result.stdout)).toEqual({ missing: true, narrow: false }); +}); + +testWindows( + "direct workbench rejects unsafe Windows state without changing its ACL", + async () => { + const root = await temporaryDirectory(); + const state = join(root, "state"); + await mkdir(state); + protectWindowsDirectory(root); + protectWindowsDirectory(state); + runIcacls(state, ["/grant", "*S-1-1-0:(OI)(CI)R"]); + const before = windowsAclSddl(state); + const command = [ + join(PLUGIN_ROOT, "scripts", "workbench_db.py"), + "list-scans", + "--repository", + root, + ]; + + const unsafeRoot = runPython(state, command); + expect(unsafeRoot.status).not.toBe(0); + expect(unsafeRoot.stderr).toContain("state directory is unsafe"); + expect(windowsAclSddl(state)).toBe(before); + expect(existsSync(join(state, "workbench.sqlite3"))).toBe(false); + + runIcacls(state, ["/remove:g", "*S-1-1-0"]); + const sidecar = join(state, "workbench.sqlite3-wal"); + await writeFile(sidecar, "unsafe sidecar\n", "utf8"); + runIcacls(sidecar, ["/grant", "*S-1-1-0:R"]); + const sidecarBefore = windowsAclSddl(sidecar); + const unsafeSidecar = runPython(state, command); + expect(unsafeSidecar.status).not.toBe(0); + expect(unsafeSidecar.stderr).toContain("state directory is unsafe"); + expect(windowsAclSddl(sidecar)).toBe(sidecarBefore); + expect(existsSync(join(state, "workbench.sqlite3"))).toBe(false); + }, +); + +testWindows( + "direct workbench gives missing Windows state a private protected ACL", + async () => { + const root = await temporaryDirectory(); + const component = join(root, "nested"); + const state = join(component, "state"); + protectWindowsDirectory(root); + const result = connectDirectly(state); + + expect(result.status).toBe(0); + expect(result.stderr).toBe(""); + expect(existsSync(join(state, "workbench.sqlite3"))).toBe(true); + for (const directory of [component, state]) { + expectPrivateWindowsDirectory(directory); + } + }, +); + +testWindows( + "direct workbench rejects writable ACLs inherited by missing Windows state", + async () => { + const root = await temporaryDirectory(); + const parent = join(root, "parent"); + const state = join(parent, "state"); + await mkdir(parent); + protectWindowsDirectory(root); + protectWindowsDirectory(parent); + runIcacls(parent, ["/grant", "*S-1-1-0:(CI)(IO)F"]); + const before = windowsAclSddl(parent); + const result = connectDirectly(state); + + expect(result.status).not.toBe(0); + expect(result.stderr).toContain("state directory is unsafe"); + expect(windowsAclSddl(parent)).toBe(before); + expect(existsSync(state)).toBe(false); + }, +); + +testWindows( + "direct workbench skips unrelated Windows state while preserving its ACL", + async () => { + const root = await temporaryDirectory(); + const state = join(root, "state"); + await mkdir(state); + protectWindowsDirectory(root); + protectWindowsDirectory(state); + const unrelated = join(state, "codex-home"); + await mkdir(unrelated); + runIcacls(unrelated, ["/grant", "*S-1-1-0:(OI)(CI)R"]); + const before = windowsAclSddl(state); + const unrelatedBefore = windowsAclSddl(unrelated); + const result = connectDirectly(state); + + expect(result.status).toBe(0); + expect(result.stderr).toBe(""); + expect(windowsAclSddl(state)).toBe(before); + expect(windowsAclSddl(unrelated)).toBe(unrelatedBefore); + expectPrivateWindowsDirectory(state); + expect(existsSync(join(state, "workbench.sqlite3"))).toBe(true); + const accessed = runPython(state, [ + "-c", + [ + "import sys", + "from pathlib import Path", + "sys.path.insert(0, sys.argv[1])", + "import workbench_db as workbench", + "workbench.require_canonical_scan_directory(Path(sys.argv[2]))", + ].join("\n"), + join(PLUGIN_ROOT, "scripts"), + unrelated, + ]); + expect(accessed.status).not.toBe(0); + expect(accessed.stderr).toContain("unsafe Windows ACL"); + expect(windowsAclSddl(unrelated)).toBe(unrelatedBefore); + }, +); + +testWindows( + "direct workbench tolerates Windows SQLite sidecar churn", + async () => { + const root = await temporaryDirectory(); + const state = join(root, "state"); + protectWindowsDirectory(root); + const initialized = connectDirectly(state); + expect(initialized.status).toBe(0); + const result = runPython(state, [ + "-c", + [ + "import sys, threading", + "from pathlib import Path", + "sys.path.insert(0, sys.argv[1])", + "import workbench_db as workbench", + "sidecar = workbench.state_dir() / 'workbench.sqlite3-wal'", + "started = threading.Event()", + "stop = threading.Event()", + "def churn():", + " started.set()", + " while not stop.is_set():", + " try:", + " sidecar.write_bytes(b'wal')", + " except PermissionError:", + " pass", + " try:", + " sidecar.unlink()", + " except (FileNotFoundError, PermissionError):", + " pass", + "thread = threading.Thread(target=churn)", + "thread.start()", + "started.wait()", + "try:", + " context = workbench.windows_state_acl_context()", + " for _ in range(8):", + " workbench.require_private_windows_state_directory(workbench.state_dir(), context)", + "finally:", + " stop.set()", + " thread.join()", + " sidecar.unlink(missing_ok=True)", + ].join("\n"), + join(PLUGIN_ROOT, "scripts"), + ]); + + expect(result.status).toBe(0); + expect(result.stderr).toBe(""); + }, +); + +test("direct workbench validates a competing private initializer", async () => { + const root = await temporaryDirectory(); + const component = join(root, "nested"); + const state = join(component, "state"); + const result = runPython(state, [ + "-c", + [ + "import json, sys", + "from pathlib import Path", + "from unittest.mock import patch", + "sys.path.insert(0, sys.argv[1])", + "import workbench_db as workbench", + "component = Path(sys.argv[2])", + "original_mkdir = Path.mkdir", + "def competing_mkdir(path, *args, **kwargs):", + " if path == component and not path.exists():", + " original_mkdir(path, *args, **kwargs)", + " raise FileExistsError(str(path))", + " return original_mkdir(path, *args, **kwargs)", + 'with patch.object(Path, "mkdir", competing_mkdir), patch.object(workbench, "require_canonical_scan_directory", wraps=workbench.require_canonical_scan_directory) as validate:', + " workbench.connect().close()", + " print(json.dumps([str(call.args[0]) for call in validate.call_args_list]))", + ].join("\n"), + join(PLUGIN_ROOT, "scripts"), + component, + ]); + + expect(result.status).toBe(0); + expect(result.stderr).toBe(""); + expect(JSON.parse(result.stdout)).toContain(component); + expect(existsSync(join(state, "workbench.sqlite3"))).toBe(true); +}); + +testPosix( + "direct workbench rejects unsafe state before opening its database", + async () => { + const root = await temporaryDirectory(); + const state = join(root, "state"); + const shared = join(root, "shared"); + const nestedState = join(shared, "state"); + const missingState = join(shared, "missing", "state"); + await mkdir(state, { mode: 0o700 }); + await mkdir(nestedState, { recursive: true, mode: 0o700 }); + await chmod(shared, 0o775); + const command = [ + join(PLUGIN_ROOT, "scripts", "workbench_db.py"), + "list-scans", + "--repository", + root, + ]; + + for (const mode of [0o755, 0o1777]) { + await chmod(state, mode); + const actualMode = (await stat(state)).mode & 0o7777; + const result = runPython(state, command); + expect(result.status).not.toBe(0); + expect(result.stderr).toContain("state directory"); + expect((await stat(state)).mode & 0o7777).toBe(actualMode); + expect(existsSync(join(state, "workbench.sqlite3"))).toBe(false); + } + + const nested = runPython(nestedState, command); + expect(nested.status).not.toBe(0); + expect(nested.stderr).toContain("group- or world-writable"); + expect((await stat(shared)).mode & 0o7777).toBe(0o775); + expect((await stat(nestedState)).mode & 0o7777).toBe(0o700); + expect(existsSync(join(nestedState, "workbench.sqlite3"))).toBe(false); + const missing = runPython(missingState, command); + expect(missing.status).not.toBe(0); + expect(missing.stderr).toContain("group- or world-writable"); + expect(existsSync(join(shared, "missing"))).toBe(false); + }, +); + +testPosix( + "direct workbench rejects unsafe lexical and chained state aliases", + async () => { + const root = await temporaryDirectory(); + const state = join(root, "state"); + const shared = join(root, "shared"); + const trusted = join(root, "trusted"); + const unsafeLink = join(shared, "state-link"); + const nextLink = join(trusted, "next-link"); + const alias = join(root, "alias"); + const dottedAlias = join(root, "dotted-alias"); + const dottedState = `${shared}${sep}..${sep}state`; + const missing = join(alias, "missing", "state"); + await mkdir(state, { mode: 0o700 }); + await mkdir(shared, { mode: 0o700 }); + await mkdir(trusted, { mode: 0o700 }); + await chmod(shared, 0o775); + await symlink(state, unsafeLink, "dir"); + await symlink(unsafeLink, nextLink, "dir"); + await symlink(`${nextLink}${sep}`, alias, "dir"); + await symlink(dottedState, dottedAlias, "dir"); + const command = [ + join(PLUGIN_ROOT, "scripts", "workbench_db.py"), + "list-scans", + "--repository", + root, + ]; + + for (const path of [ + unsafeLink, + nextLink, + alias, + dottedAlias, + dottedState, + missing, + ]) { + const result = runPython(path, command); + expect(result.status).not.toBe(0); + expect(result.stdout).toBe(""); + expect(result.stderr).toContain("state directory is unsafe"); + expect(result.stderr).toContain("group- or world-writable"); + } + expect((await stat(shared)).mode & 0o7777).toBe(0o775); + expect((await stat(state)).mode & 0o7777).toBe(0o700); + expect(await readdir(state)).toEqual([]); + expect(await readdir(shared)).toEqual(["state-link"]); + expect(await readdir(trusted)).toEqual(["next-link"]); + expect(existsSync(missing)).toBe(false); + }, +); + +testPosix( + "direct workbench rejects untrusted state-link ownership before creation", + async () => { + const root = await temporaryDirectory(); + const state = join(root, "state"); + const alias = join(root, "alias"); + await mkdir(state, { mode: 0o700 }); + await symlink(state, alias, "dir"); + const result = runPython(alias, [ + "-c", + [ + "import os, sys", + "from pathlib import Path", + "from unittest.mock import patch", + "sys.path.insert(0, sys.argv[1])", + "import workbench_db as workbench", + "original_lstat = os.lstat", + "def synthetic_lstat(path, *args, **kwargs):", + " metadata = original_lstat(path, *args, **kwargs)", + " if os.fspath(path) == sys.argv[2]:", + " values = list(metadata)", + " values[4] = os.geteuid() + 1", + " return os.stat_result(values)", + " return metadata", + "with (", + ' patch.object(os, "lstat", synthetic_lstat),', + ' patch.object(Path, "mkdir", side_effect=AssertionError("unexpected creation")),', + ' patch.object(workbench.sqlite3, "connect", side_effect=AssertionError("unexpected database access")),', + "):", + " try:", + " workbench.connect()", + " except SystemExit as error:", + " print(error)", + " else:", + ' raise AssertionError("untrusted state link was accepted")', + ].join("\n"), + join(PLUGIN_ROOT, "scripts"), + alias, + ]); + + expect(result.status).toBe(0); + expect(result.stderr).toBe(""); + expect(result.stdout).toContain("state directory is unsafe"); + expect(result.stdout).toContain("trusted owner"); + expect((await stat(state)).mode & 0o7777).toBe(0o700); + expect(await readdir(state)).toEqual([]); + }, +); + +test("direct workbench preserves unresolved home expansion failures", () => { + const result = runPython("", [ + "-c", + [ + "import json, os, sys", + "from pathlib import Path", + "from unittest.mock import patch", + "sys.path.insert(0, sys.argv[1])", + "import workbench_db as workbench", + "errors = []", + "with (", + ' patch.object(os.path, "expanduser", side_effect=lambda path: path),', + ' patch.object(Path, "mkdir", side_effect=AssertionError("unexpected creation")),', + ' patch.object(workbench.sqlite3, "connect", side_effect=AssertionError("unexpected database access")),', + "):", + " for environment in (", + ' {"CODEX_SECURITY_STATE_DIR": "~unresolved/state"},', + ' {"CODEX_SECURITY_STATE_DIR": "", "CODEX_HOME": "~/home"},', + " ):", + " with patch.dict(os.environ, environment, clear=True):", + " for select in (workbench.state_dir, workbench.connect):", + " try:", + " select()", + " except RuntimeError as error:", + " errors.append(str(error))", + " else:", + ' raise AssertionError("unresolved home was accepted")', + "print(json.dumps(errors))", + ].join("\n"), + join(PLUGIN_ROOT, "scripts"), + ]); + + expect(result.status).toBe(0); + expect(result.stderr).toBe(""); + expect(JSON.parse(result.stdout)).toEqual( + Array(4).fill("Could not determine home directory."), + ); +});