From 5dc0f1c2985e1e01e0c9cd46ae4a8053eef3096a Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Sat, 15 Aug 2026 12:53:44 -0700 Subject: [PATCH 01/27] fix(git): pin trusted executables across scan hosts --- .../_bundled_plugin/.codex-plugin/plugin.json | 2 +- sdk/typescript/_bundled_plugin/.mcp.json | 1 + .../scripts/generate_in_scope_files.py | 24 +- .../scripts/generate_rank_input.py | 34 +- .../scripts/workbench_target.py | 110 ++++- sdk/typescript/src/api.ts | 41 +- sdk/typescript/src/targets.ts | 2 +- sdk/typescript/src/trusted-executable.ts | 29 +- sdk/typescript/src/version.ts | 2 +- .../tests-ts/workbench-trusted-git.test.ts | 457 ++++++++++++++++++ 10 files changed, 639 insertions(+), 63 deletions(-) create mode 100644 sdk/typescript/tests-ts/workbench-trusted-git.test.ts diff --git a/sdk/typescript/_bundled_plugin/.codex-plugin/plugin.json b/sdk/typescript/_bundled_plugin/.codex-plugin/plugin.json index 04c30b9c6..76688fe96 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.20", + "version": "0.1.22", "description": "Codex Security workflows for security scans, analysis, and investigation.", "author": { "name": "OpenAI" diff --git a/sdk/typescript/_bundled_plugin/.mcp.json b/sdk/typescript/_bundled_plugin/.mcp.json index 9a4fc836e..40fbec048 100644 --- a/sdk/typescript/_bundled_plugin/.mcp.json +++ b/sdk/typescript/_bundled_plugin/.mcp.json @@ -28,6 +28,7 @@ "AWS_CONTAINER_AUTHORIZATION_TOKEN", "AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE", "PYTHON", + "CODEX_SECURITY_GIT", "CODEX_SECURITY_KNOWLEDGE_BASE", "CODEX_SECURITY_DEEP_SCAN_CONFIG_PATH", "CODEX_SECURITY_SCAN_ROOT", diff --git a/sdk/typescript/_bundled_plugin/scripts/generate_in_scope_files.py b/sdk/typescript/_bundled_plugin/scripts/generate_in_scope_files.py index 6449bf747..65f268815 100644 --- a/sdk/typescript/_bundled_plugin/scripts/generate_in_scope_files.py +++ b/sdk/typescript/_bundled_plugin/scripts/generate_in_scope_files.py @@ -107,20 +107,18 @@ def generate_in_scope_files(repository: Path, scope: str, output: Path) -> int: def committed_changed_paths(repository: Path, base: str, head: str) -> list[tuple[Path, str]]: - result = subprocess.run( - [ - "git", - "-C", - str(repository), - "diff", - "--raw", - "-z", - "--diff-filter=ACMRD", - f"{base}..{head}", - ], - capture_output=True, - check=True, + from workbench_target import git_command + + result = git_command( + repository, + "diff", + "--raw", + "-z", + "--diff-filter=ACMRD", + f"{base}..{head}", + text=False, ) + result.check_returncode() fields = result.stdout.split(b"\0") changed: list[tuple[Path, str]] = [] index = 0 diff --git a/sdk/typescript/_bundled_plugin/scripts/generate_rank_input.py b/sdk/typescript/_bundled_plugin/scripts/generate_rank_input.py index 92018d74b..ef5722d6a 100644 --- a/sdk/typescript/_bundled_plugin/scripts/generate_rank_input.py +++ b/sdk/typescript/_bundled_plugin/scripts/generate_rank_input.py @@ -43,7 +43,7 @@ preview_for, preview_for_bytes, ) -from workbench_target import git_blob_bytes, git_directory_snapshot_paths +from workbench_target import git_blob_bytes, git_command, git_directory_snapshot_paths EXCLUDED_DIRS = { ".cache", @@ -608,21 +608,16 @@ def bind_repo_scopes(args: argparse.Namespace) -> None: def run_git_changed_paths(repo: Path, diff_args: list[str]) -> list[tuple[Path, str]]: - result = subprocess.run( - [ - "git", - "-C", - str(repo), - "diff", - "--name-status", - "-z", - "--diff-filter=ACMRD", - *diff_args, - ], - check=True, - capture_output=True, + result = git_command( + repo, + "diff", + "--name-status", + "-z", + "--diff-filter=ACMRD", + *diff_args, text=True, ) + result.check_returncode() fields = result.stdout.split("\0") if fields and not fields[-1]: fields.pop() @@ -646,12 +641,15 @@ def git_changed_paths(repo: Path, base: str, head: str, mode: str) -> list[tuple if mode == "local-patch": unstaged = run_git_changed_paths(repo, [base]) staged = run_git_changed_paths(repo, ["--cached", base]) - untracked = subprocess.run( - ["git", "-C", str(repo), "ls-files", "--others", "--exclude-standard", "-z"], - capture_output=True, + untracked = git_command( + repo, + "ls-files", + "--others", + "--exclude-standard", + "-z", text=True, - check=True, ) + untracked.check_returncode() combined = dict(staged) combined.update(unstaged) combined.update( diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_target.py b/sdk/typescript/_bundled_plugin/scripts/workbench_target.py index 05faf6e03..4115a7149 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_target.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_target.py @@ -120,6 +120,83 @@ def _read_sized_nul_field( return output[offset:end], end + 1 +def _protected_git_root(target: Path) -> Path: + root = target.resolve() + for ancestor in (root, *root.parents): + try: + (ancestor / ".git").lstat() + except FileNotFoundError: + continue + root = ancestor + return root + + +def _trusted_git_executable(target: Path, environment: dict[str, str]) -> str | None: + root = _protected_git_root(target) + configured = environment.get("CODEX_SECURITY_GIT") + if configured is not None: + if not configured: + return None + candidate = Path(configured) + windows = sys.platform == "win32" + if not candidate.is_absolute(): + raise SystemExit("CODEX_SECURITY_GIT must name an absolute trusted executable.") + try: + parent = candidate.parent.resolve(strict=True) + canonical = candidate.resolve(strict=True) + except OSError as error: + raise SystemExit("CODEX_SECURITY_GIT does not name an available executable.") from error + if parent.is_relative_to(root) or canonical.is_relative_to(root): + raise SystemExit("CODEX_SECURITY_GIT must stay outside the protected repository.") + if ( + not canonical.is_file() + or ( + windows + and ( + candidate.suffix.lower() not in {".exe", ".com"} + or canonical.suffix.lower() not in {".exe", ".com"} + ) + ) + or not os.access(canonical, os.F_OK if windows else os.X_OK) + ): + raise SystemExit("CODEX_SECURITY_GIT does not name an available executable.") + return str(canonical) + + entries: list[str] = [] + executable: str | None = None + names = ("git.exe", "git.com") if sys.platform == "win32" else ("git",) + for entry in os.get_exec_path(environment): + if not entry: + continue + try: + directory = Path(entry).resolve(strict=True) + except OSError: + continue + if directory.is_relative_to(root): + continue + candidate: str | None = None + safe = True + for name in names: + path = directory / name + try: + canonical = path.resolve(strict=True) + except OSError: + continue + if canonical.is_relative_to(root): + safe = False + break + if canonical.is_file() and os.access( + canonical, os.F_OK if sys.platform == "win32" else os.X_OK + ): + candidate = candidate or str(path) + if not safe: + continue + executable = executable or candidate + entries.append(str(directory)) + environment["PATH"] = os.pathsep.join(entries) + return executable + + def git_command( target: Path, *args: str, @@ -134,25 +211,28 @@ def git_command( for name in GIT_REPOSITORY_ENVIRONMENT: environment.pop(name, None) environment["GIT_LITERAL_PATHSPECS"] = "1" + executable = _trusted_git_executable(target, environment) # Repository-local config is untrusted; fsmonitor may name an executable hook. - command = ["git", "-c", "core.fsmonitor=false", "-C", str(target)] + command = [executable or "git", "-c", "core.fsmonitor=false", "-C", str(target)] if git_dir is not None and work_tree is not None: command.extend(["--git-dir", str(git_dir), "--work-tree", str(work_tree)]) full_command = [*command, *args] - try: - return subprocess.run( - full_command, - check=False, - capture_output=True, - env=environment, - text=text, - input=input_data, - ) - except FileNotFoundError: - # Git is optional for Codebase scans. Treat an unavailable executable like - # any other failed Git probe so the target falls back to a directory snapshot. - empty_output = "" if text else b"" - return subprocess.CompletedProcess(full_command, 127, empty_output, empty_output) + if executable is not None: + try: + return subprocess.run( + full_command, + check=False, + capture_output=True, + env=environment, + text=text, + input=input_data, + ) + except FileNotFoundError: + pass + # Git is optional for Codebase scans. Treat an unavailable executable like + # any other failed Git probe so the target falls back to a directory snapshot. + empty_output = "" if text else b"" + return subprocess.CompletedProcess(full_command, 127, empty_output, empty_output) def update_digest_field(digest: Any, label: bytes, value: bytes) -> None: diff --git a/sdk/typescript/src/api.ts b/sdk/typescript/src/api.ts index fd60107e5..3334dc63a 100644 --- a/sdk/typescript/src/api.ts +++ b/sdk/typescript/src/api.ts @@ -109,6 +109,7 @@ import { enclosingGitWorktreeRoot, normalizeRepository, normalizeTarget, + outermostGitMarkerRoot, repositoryRevision, resolveRepositoryPath, type NormalizedTarget, @@ -118,6 +119,10 @@ import { validateCommittedDiffCheckout, validateMode, } from "./targets.js"; +import { + resolveTrustedExecutable, + trustedExecutableEnvironment, +} from "./trusted-executable.js"; interface CodexThreadLike { readonly id: string | null; @@ -635,6 +640,28 @@ export class CodexSecurity { protectedRoot, signal, }); + const pluginEnvironment = selectedScanEnvironment( + runtime.environment, + options.auth, + modelProvider, + ); + const protectedGitRoot = await outermostGitMarkerRoot( + protectedRoot, + signal, + ); + const git = await resolveTrustedExecutable( + "git", + pluginEnvironment, + protectedGitRoot, + ); + const trustedPluginEnvironment = { + ...(await trustedExecutableEnvironment( + "rg", + git?.environment ?? pluginEnvironment, + protectedGitRoot, + )), + CODEX_SECURITY_GIT: git?.executable ?? "", + }; checkOpen(); const scanOutputRoot = requestedOutput === null && @@ -812,11 +839,7 @@ export class CodexSecurity { python, pluginRoot: runtime.plugin.pluginRoot, environment: { - ...selectedScanEnvironment( - runtime.environment, - options.auth, - modelProvider, - ), + ...trustedPluginEnvironment, CODEX_SECURITY_STATE_DIR: stateDirectory, }, signal, @@ -1010,13 +1033,7 @@ export class CodexSecurity { const environment = { ...pluginExecutionEnvironment( python, - withoutCodexHome( - selectedScanEnvironment( - runtime.environment, - options.auth, - modelProvider, - ), - ), + withoutCodexHome(trustedPluginEnvironment), ), ...(externalProvider === null ? {} diff --git a/sdk/typescript/src/targets.ts b/sdk/typescript/src/targets.ts index f13858af2..025052e11 100644 --- a/sdk/typescript/src/targets.ts +++ b/sdk/typescript/src/targets.ts @@ -418,7 +418,7 @@ async function gitOutput( return stdout.trim(); } -async function outermostGitMarkerRoot( +export async function outermostGitMarkerRoot( repository: string, signal?: AbortSignal, ): Promise { diff --git a/sdk/typescript/src/trusted-executable.ts b/sdk/typescript/src/trusted-executable.ts index 5a0729cf0..3567b5d62 100644 --- a/sdk/typescript/src/trusted-executable.ts +++ b/sdk/typescript/src/trusted-executable.ts @@ -12,6 +12,33 @@ export async function resolveTrustedExecutable( environment: Readonly>, protectedRoot: string, ): Promise { + const inspected = await inspectTrustedExecutable( + candidate, + environment, + protectedRoot, + ); + return inspected.executable === null + ? null + : { executable: inspected.executable, environment: inspected.environment }; +} + +export async function trustedExecutableEnvironment( + candidate: string, + environment: Readonly>, + protectedRoot: string, +): Promise> { + return (await inspectTrustedExecutable(candidate, environment, protectedRoot)) + .environment; +} + +async function inspectTrustedExecutable( + candidate: string, + environment: Readonly>, + protectedRoot: string, +): Promise<{ + executable: string | null; + environment: Record; +}> { const root = await realpath(protectedRoot).catch(() => resolve(protectedRoot), ); @@ -70,8 +97,6 @@ export async function resolveTrustedExecutable( continue; } } - if (executable === null) return null; - const sanitizedEnvironment = { ...environment }; for (const name of Object.keys(sanitizedEnvironment)) { if (name.toUpperCase() === "PATH") delete sanitizedEnvironment[name]; diff --git a/sdk/typescript/src/version.ts b/sdk/typescript/src/version.ts index 01dd50067..1bf429fb8 100644 --- a/sdk/typescript/src/version.ts +++ b/sdk/typescript/src/version.ts @@ -8,7 +8,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.20" as const; +export const BUNDLED_PLUGIN_VERSION = "0.1.22" as const; const PACKAGE_NAME = "@openai/codex-security"; const VERSION_PATTERN = diff --git a/sdk/typescript/tests-ts/workbench-trusted-git.test.ts b/sdk/typescript/tests-ts/workbench-trusted-git.test.ts new file mode 100644 index 000000000..d8f5ea434 --- /dev/null +++ b/sdk/typescript/tests-ts/workbench-trusted-git.test.ts @@ -0,0 +1,457 @@ +import { execFileSync, spawnSync } from "node:child_process"; +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + realpathSync, + rmSync, + symlinkSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { delimiter, dirname, join } from "node:path"; +import type { CodexOptions } from "@openai/codex-sdk"; +import { afterEach, describe, expect, test } from "bun:test"; +import { CodexSecurity } from "../src/api.js"; +import { runWorkbench } from "../src/runtime.js"; +import { PLUGIN_ROOT } from "./plugin-root.js"; +import { preparedRuntime } from "./support/api-events.js"; + +const roots: string[] = []; +const testPosix = process.platform === "win32" ? test.skip : test; +const statusProbe = ["git_command(Path(sys.argv[2]), 'status', text=True)"]; +const unsafeExecutable = "must stay outside the protected repository"; +const TestClient = CodexSecurity as unknown as new ( + config: Record, + dependencies: Record, +) => CodexSecurity; + +afterEach(() => { + for (const root of roots.splice(0)) { + const exfiltrated = existsSync(join(root, "exfiltrated-credential")); + rmSync(root, { recursive: true, force: true }); + expect(exfiltrated).toBe(false); + } +}); + +function fixture() { + const root = realpathSync(mkdtempSync(join(tmpdir(), "trusted-git-"))); + roots.push(root); + const repository = join(root, "repository"); + const shimDirectory = join(repository, "node_modules", ".bin"); + mkdirSync(shimDirectory, { recursive: true }); + const shim = join( + shimDirectory, + process.platform === "win32" ? "git.exe" : "git", + ); + writeFileSync( + shim, + '#!/bin/sh\nprintf "%s" "$GITHUB_TOKEN" > "$CODEX_SECURITY_TEST_MARKER"\nexit 1\n', + { mode: 0o700 }, + ); + const git = Bun.which("git"); + const python = Bun.which("python3") ?? Bun.which("python"); + expect(git).not.toBeNull(); + expect(python).not.toBeNull(); + return { + root, + repository, + shim, + git: git!, + python: python!, + environment: { + HOME: root, + USERPROFILE: root, + ...(process.env["SystemRoot"] === undefined + ? {} + : { SystemRoot: process.env["SystemRoot"] }), + PATH: `${shimDirectory}${delimiter}${dirname(git!)}`, + GITHUB_TOKEN: "synthetic-github-credential", + CODEX_SECURITY_TEST_MARKER: join(root, "exfiltrated-credential"), + GIT_CONFIG_COUNT: "1", + GIT_CONFIG_KEY_0: "codexsecurity.synthetic", + GIT_CONFIG_VALUE_0: "operator-config-preserved", + PYTHONDONTWRITEBYTECODE: "1", + }, + }; +} + +function git( + target: ReturnType, + directory: string, + ...args: string[] +) { + return execFileSync( + target.git, + ["-C", directory, "-c", "user.name=x", "-c", "user.email=x@y", ...args], + { encoding: "utf8" }, + ).trim(); +} + +function probe( + target: ReturnType, + source: readonly string[], + options: { + repository?: string; + environment?: NodeJS.ProcessEnv; + } = {}, +) { + return spawnSync( + target.python, + [ + "-I", + "-B", + "-c", + [ + "import json, sys; from pathlib import Path", + "sys.path.insert(0, sys.argv[1]); from workbench_target import git_command", + ...source, + ].join("\n"), + join(PLUGIN_ROOT, "scripts"), + options.repository ?? target.repository, + ], + { + encoding: "utf8", + env: { ...target.environment, ...options.environment }, + cwd: target.repository, + }, + ); +} + +describe("bundled workbench trusted Git", () => { + testPosix("avoids repository shims and preserves user Git settings", () => { + const target = fixture(); + git(target, target.repository, "init", "-q"); + const nested = join(target.repository, "src", "nested"); + mkdirSync(nested, { recursive: true }); + + for (const [repository, environment] of [ + [target.repository, { CODEX_SECURITY_GIT: target.git }], + [nested, {}], + ] as const) { + const result = probe( + target, + [ + "result = git_command(Path(sys.argv[2]), 'config', '--get', 'codexsecurity.synthetic', text=True)", + "print(json.dumps({'git': result.args[0], 'value': result.stdout.strip(), 'status': result.returncode}))", + ], + { repository, environment }, + ); + expect(result.status, result.stderr).toBe(0); + expect(JSON.parse(result.stdout)).toMatchObject({ + value: "operator-config-preserved", + status: 0, + }); + expect(realpathSync(JSON.parse(result.stdout).git)).toBe( + realpathSync(target.git), + ); + } + }); + + test("keeps Git optional when no trusted executable exists", () => { + const target = fixture(); + const result = probe( + target, + [ + "result = git_command(Path(sys.argv[2]), 'status', text=True)", + "print(json.dumps({'status': result.returncode, 'output': result.stdout}))", + ], + { environment: { CODEX_SECURITY_GIT: "" } }, + ); + expect(result.status, result.stderr).toBe(0); + expect(JSON.parse(result.stdout)).toEqual({ status: 127, output: "" }); + }); + + test("rejects explicitly selected repository-controlled Git", () => { + const target = fixture(); + const result = probe(target, statusProbe, { + environment: { CODEX_SECURITY_GIT: target.shim }, + }); + expect(result.status).toBe(1); + expect(result.stderr).toContain(unsafeExecutable); + }); + + testPosix("rejects repository symlinks and aliased parents", () => { + const target = fixture(); + const linkedGit = join(target.repository, "safe-looking-git"); + const repositoryAlias = join(target.root, "repository-alias"); + symlinkSync(target.git, linkedGit); + symlinkSync(target.repository, repositoryAlias, "junction"); + + const aliases = [linkedGit, join(repositoryAlias, "safe-looking-git")]; + for (const executable of aliases) { + const result = probe(target, statusProbe, { + environment: { CODEX_SECURITY_GIT: executable }, + }); + expect(result.status).toBe(1); + expect(result.stderr).toContain(unsafeExecutable); + } + }); + + test("ignores Windows batch shims, PATHEXT order, and the working directory", () => { + const target = fixture(); + const trustedDirectory = join(target.root, "trusted-bin"); + mkdirSync(trustedDirectory); + const executable = join(trustedDirectory, "git.exe"); + writeFileSync(executable, "synthetic native executable\n"); + for (const directory of [trustedDirectory, target.repository]) { + writeFileSync(join(directory, "git.cmd"), "synthetic batch shim\n"); + } + const result = probe( + target, + [ + "import os, workbench_target", + "workbench_target.sys.platform = 'win32'", + "selected = workbench_target._trusted_git_executable(Path(sys.argv[2]), dict(os.environ))", + "batch = Path(sys.argv[2]).parent / 'trusted-bin' / 'git.cmd'", + "try: workbench_target._trusted_git_executable(Path(sys.argv[2]), {**os.environ, 'CODEX_SECURITY_GIT': str(batch)})", + "except SystemExit: batch_rejected = True", + "else: batch_rejected = False", + "print(json.dumps({'executable': selected, 'batchRejected': batch_rejected}))", + ], + { + environment: { PATH: trustedDirectory, PATHEXT: ".CMD;.BAT;.EXE;.COM" }, + }, + ); + expect(result.status, result.stderr).toBe(0); + expect(JSON.parse(result.stdout)).toEqual({ + executable, + batchRejected: true, + }); + }); + + testPosix( + "uses trusted Git for real diff ranking and committed inventory", + () => { + const target = fixture(); + git(target, target.repository, "init", "-q"); + writeFileSync(join(target.repository, "source.py"), "before = True\n"); + git(target, target.repository, "add", "source.py"); + git(target, target.repository, "commit", "-qm", "base"); + const revision = git(target, target.repository, "rev-parse", "HEAD"); + writeFileSync(join(target.repository, "source.py"), "after = True\n"); + const output = join(target.root, "rank-input.jsonl"); + const revisionArgs = ["--base", revision, "--head", revision]; + const environment = { + ...target.environment, + CODEX_SECURITY_GIT: target.git, + }; + const result = spawnSync( + target.python, + [ + "-I", + "-B", + join(PLUGIN_ROOT, "scripts", "generate_rank_input.py"), + "make-diff-rank-input", + "--repo", + target.repository, + ...revisionArgs, + "--mode", + "local-patch", + "--out", + output, + ], + { encoding: "utf8", env: environment }, + ); + expect(result.status, result.stderr).toBe(0); + expect(JSON.parse(readFileSync(output, "utf8"))).toMatchObject({ + path: "source.py", + }); + + git(target, target.repository, "add", "source.py"); + git(target, target.repository, "commit", "-qm", "head"); + const inventoryPath = join(target.root, "in-scope-files.txt"); + const inventory = spawnSync( + target.python, + [ + "-I", + "-B", + join(PLUGIN_ROOT, "scripts", "generate_in_scope_files.py"), + "--repo", + target.repository, + "--scope", + ".", + "--diff-base", + revision, + "--diff-head", + "HEAD", + "--out", + inventoryPath, + ], + { encoding: "utf8", env: environment }, + ); + expect(inventory.status, inventory.stderr).toBe(0); + expect(readFileSync(inventoryPath, "utf8")).toBe("source.py\n"); + }, + ); + + testPosix( + "keeps optional-Git scans and real inventory safe from repository ripgrep", + async () => { + const target = fixture(); + writeFileSync(join(target.repository, "source.py"), "value = 1\n"); + const unsafeDirectory = dirname(target.shim); + const repositoryRipgrep = join(unsafeDirectory, "rg"); + writeFileSync(repositoryRipgrep, readFileSync(target.shim), { + mode: 0o700, + }); + const aliasDirectory = join(target.root, "alias-bin"); + const safeDirectory = join(target.root, "trusted-bin"); + const codexHome = join(target.root, "codex-home"); + for (const directory of [aliasDirectory, safeDirectory, codexHome]) { + mkdirSync(directory); + } + symlinkSync(repositoryRipgrep, join(aliasDirectory, "rg")); + writeFileSync( + join(safeDirectory, "rg"), + '#!/bin/sh\nprintf "source.py\\n"\n', + { mode: 0o700 }, + ); + const environment = { + ...target.environment, + CODEX_SECURITY_STATE_DIR: join(target.root, "state"), + PATH: [unsafeDirectory, aliasDirectory, safeDirectory].join(delimiter), + }; + const observed: Array> = []; + const client = new TestClient( + {}, + { + environment, + prepareRuntime: async () => ({ + ...preparedRuntime(codexHome), + environment, + }), + resolvePluginPython: async () => target.python, + repositoryRevision: async () => null, + runWorkbench: async (...args: Parameters) => { + observed.push(args[0].environment); + return await runWorkbench(...args); + }, + createCodex: (options: CodexOptions) => { + observed.push(options.env ?? {}); + throw new Error("captured optional-Git environment"); + }, + }, + ); + try { + await expect( + client.run(target.repository, { + outputDir: join(target.root, "scan"), + }), + ).rejects.toThrow("captured optional-Git environment"); + } finally { + await client.close(); + } + + expect(observed.length).toBeGreaterThan(1); + for (const candidate of observed) { + expect(candidate["CODEX_SECURITY_GIT"]).toBe(""); + expect(candidate["PATH"]?.split(delimiter)).toEqual([safeDirectory]); + } + const output = join(target.root, "inventory.txt"); + const inventory = spawnSync( + target.python, + [ + "-I", + "-B", + join(PLUGIN_ROOT, "scripts", "generate_in_scope_files.py"), + "--repo", + target.repository, + "--scope", + ".", + "--out", + output, + ], + { encoding: "utf8", env: observed[0] }, + ); + expect(inventory.status, inventory.stderr).toBe(0); + expect(readFileSync(output, "utf8")).toBe("source.py\n"); + }, + ); + + testPosix( + "propagates outermost-root trusted Git through SDK, workbench, and MCP", + async () => { + const target = fixture(); + git(target, target.repository, "init", "-q"); + const nested = join(target.repository, "submodule"); + mkdirSync(nested); + git(target, nested, "init", "-q"); + writeFileSync(join(nested, "source.py"), "value = 1\n"); + git(target, nested, "add", "source.py"); + git(target, nested, "commit", "-qm", "base"); + const revision = git(target, nested, "rev-parse", "HEAD"); + const codexHome = join(target.root, "codex-home"); + mkdirSync(codexHome); + const aliasDirectory = join(target.root, "alias-bin"); + mkdirSync(aliasDirectory); + symlinkSync(target.shim, join(aliasDirectory, "rg")); + const environment = { + ...target.environment, + CODEX_SECURITY_STATE_DIR: join(target.root, "state"), + PATH: [dirname(target.shim), aliasDirectory, dirname(target.git)].join( + delimiter, + ), + }; + const environments: Array> = []; + const client = new TestClient( + {}, + { + environment, + prepareRuntime: async () => ({ + ...preparedRuntime(codexHome), + environment, + }), + resolvePluginPython: async () => target.python, + repositoryRevision: async () => revision, + runWorkbench: async (...args: Parameters) => { + environments.push(args[0].environment); + return await runWorkbench(...args); + }, + createCodex: (options: CodexOptions) => { + environments.push(options.env ?? {}); + throw new Error("captured scan environment"); + }, + }, + ); + + try { + await expect( + client.run(nested, { outputDir: join(target.root, "scan") }), + ).rejects.toThrow("captured scan environment"); + } finally { + await client.close(); + } + + expect(environments.length).toBeGreaterThan(1); + for (const observed of environments) { + expect(observed).toMatchObject({ + CODEX_SECURITY_GIT: target.git, + GIT_CONFIG_COUNT: "1", + GIT_CONFIG_KEY_0: "codexsecurity.synthetic", + GIT_CONFIG_VALUE_0: "operator-config-preserved", + }); + expect(observed?.["PATH"]?.split(delimiter)).not.toContain( + dirname(target.shim), + ); + expect(observed?.["PATH"]?.split(delimiter)).not.toContain( + aliasDirectory, + ); + } + + const configuration = JSON.parse( + readFileSync(join(PLUGIN_ROOT, ".mcp.json"), "utf8"), + ) as { + mcpServers: Record; + }; + const allowed = configuration.mcpServers["codex-security"]!.env_vars; + const mcpEnvironment = Object.fromEntries( + Object.entries(environments.at(-1) ?? {}).filter(([name]) => + allowed.includes(name), + ), + ); + expect(mcpEnvironment).toMatchObject({ CODEX_SECURITY_GIT: target.git }); + }, + ); +}); From 9be47a60bbf82e634cd81a816f72290b4929d160 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Sat, 15 Aug 2026 13:10:58 -0700 Subject: [PATCH 02/27] fix(git): preserve sanitized and inherited executable paths --- sdk/typescript/src/api.ts | 9 +++- sdk/typescript/src/trusted-executable.ts | 12 +++-- .../tests-ts/workbench-trusted-git.test.ts | 48 +++++++++++++++++-- 3 files changed, 59 insertions(+), 10 deletions(-) diff --git a/sdk/typescript/src/api.ts b/sdk/typescript/src/api.ts index 3334dc63a..7444bbf17 100644 --- a/sdk/typescript/src/api.ts +++ b/sdk/typescript/src/api.ts @@ -649,15 +649,20 @@ export class CodexSecurity { protectedRoot, signal, ); - const git = await resolveTrustedExecutable( + const gitEnvironment = await trustedExecutableEnvironment( "git", pluginEnvironment, protectedGitRoot, ); + const git = await resolveTrustedExecutable( + "git", + gitEnvironment, + protectedGitRoot, + ); const trustedPluginEnvironment = { ...(await trustedExecutableEnvironment( "rg", - git?.environment ?? pluginEnvironment, + git?.environment ?? gitEnvironment, protectedGitRoot, )), CODEX_SECURITY_GIT: git?.executable ?? "", diff --git a/sdk/typescript/src/trusted-executable.ts b/sdk/typescript/src/trusted-executable.ts index 3567b5d62..1b0ebe5cb 100644 --- a/sdk/typescript/src/trusted-executable.ts +++ b/sdk/typescript/src/trusted-executable.ts @@ -98,12 +98,14 @@ async function inspectTrustedExecutable( } } const sanitizedEnvironment = { ...environment }; - for (const name of Object.keys(sanitizedEnvironment)) { - if (name.toUpperCase() === "PATH") delete sanitizedEnvironment[name]; + if (path !== undefined) { + for (const name of Object.keys(sanitizedEnvironment)) { + if (name.toUpperCase() === "PATH") delete sanitizedEnvironment[name]; + } + sanitizedEnvironment["PATH"] = entries + .filter((entry) => !unsafeEntries.has(entry)) + .join(delimiter); } - sanitizedEnvironment["PATH"] = entries - .filter((entry) => !unsafeEntries.has(entry)) - .join(delimiter); return { executable, environment: sanitizedEnvironment }; } diff --git a/sdk/typescript/tests-ts/workbench-trusted-git.test.ts b/sdk/typescript/tests-ts/workbench-trusted-git.test.ts index d8f5ea434..5dd274028 100644 --- a/sdk/typescript/tests-ts/workbench-trusted-git.test.ts +++ b/sdk/typescript/tests-ts/workbench-trusted-git.test.ts @@ -15,6 +15,7 @@ import type { CodexOptions } from "@openai/codex-sdk"; import { afterEach, describe, expect, test } from "bun:test"; import { CodexSecurity } from "../src/api.js"; import { runWorkbench } from "../src/runtime.js"; +import { trustedExecutableEnvironment } from "../src/trusted-executable.js"; import { PLUGIN_ROOT } from "./plugin-root.js"; import { preparedRuntime } from "./support/api-events.js"; @@ -163,6 +164,30 @@ describe("bundled workbench trusted Git", () => { expect(JSON.parse(result.stdout)).toEqual({ status: 127, output: "" }); }); + test("preserves absent PATH while clearing explicitly unsafe PATH", async () => { + const target = fixture(); + const missing = await trustedExecutableEnvironment( + "git", + { HOME: target.root }, + target.repository, + ); + expect(missing).not.toHaveProperty("PATH"); + + const undefinedPath = await trustedExecutableEnvironment( + "git", + { HOME: target.root, PATH: undefined }, + target.repository, + ); + expect(undefinedPath["PATH"]).toBeUndefined(); + + const unsafe = await trustedExecutableEnvironment( + "git", + { HOME: target.root, PATH: dirname(target.shim) }, + target.repository, + ); + expect(unsafe["PATH"]).toBe(""); + }); + test("rejects explicitly selected repository-controlled Git", () => { const target = fixture(); const result = probe(target, statusProbe, { @@ -287,7 +312,7 @@ describe("bundled workbench trusted Git", () => { ); testPosix( - "keeps optional-Git scans and real inventory safe from repository ripgrep", + "keeps optional-Git scans safe from repository Git and ripgrep aliases", async () => { const target = fixture(); writeFileSync(join(target.repository, "source.py"), "value = 1\n"); @@ -296,12 +321,19 @@ describe("bundled workbench trusted Git", () => { writeFileSync(repositoryRipgrep, readFileSync(target.shim), { mode: 0o700, }); + const gitAliasDirectory = join(target.root, "git-alias-bin"); const aliasDirectory = join(target.root, "alias-bin"); const safeDirectory = join(target.root, "trusted-bin"); const codexHome = join(target.root, "codex-home"); - for (const directory of [aliasDirectory, safeDirectory, codexHome]) { + for (const directory of [ + gitAliasDirectory, + aliasDirectory, + safeDirectory, + codexHome, + ]) { mkdirSync(directory); } + symlinkSync(target.shim, join(gitAliasDirectory, "git")); symlinkSync(repositoryRipgrep, join(aliasDirectory, "rg")); writeFileSync( join(safeDirectory, "rg"), @@ -311,7 +343,12 @@ describe("bundled workbench trusted Git", () => { const environment = { ...target.environment, CODEX_SECURITY_STATE_DIR: join(target.root, "state"), - PATH: [unsafeDirectory, aliasDirectory, safeDirectory].join(delimiter), + PATH: [ + unsafeDirectory, + gitAliasDirectory, + aliasDirectory, + safeDirectory, + ].join(delimiter), }; const observed: Array> = []; const client = new TestClient( @@ -345,6 +382,11 @@ describe("bundled workbench trusted Git", () => { } expect(observed.length).toBeGreaterThan(1); + const unexpectedGit = spawnSync("git", ["--version"], { + encoding: "utf8", + env: observed[0], + }); + expect(unexpectedGit.error).toMatchObject({ code: "ENOENT" }); for (const candidate of observed) { expect(candidate["CODEX_SECURITY_GIT"]).toBe(""); expect(candidate["PATH"]?.split(delimiter)).toEqual([safeDirectory]); From 49000ab013d2307b42a780cc49cb258a1581348b Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Sat, 15 Aug 2026 13:33:01 -0700 Subject: [PATCH 03/27] fix(git): anchor trusted tools to the scanned repository --- .../scripts/workbench_target.py | 2 +- sdk/typescript/src/api.ts | 16 +- .../tests-ts/workbench-trusted-git.test.ts | 170 +++++++++++++++++- 3 files changed, 180 insertions(+), 8 deletions(-) diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_target.py b/sdk/typescript/_bundled_plugin/scripts/workbench_target.py index 4115a7149..e9d38f14c 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_target.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_target.py @@ -160,7 +160,7 @@ def _trusted_git_executable(target: Path, environment: dict[str, str]) -> str | or not os.access(canonical, os.F_OK if windows else os.X_OK) ): raise SystemExit("CODEX_SECURITY_GIT does not name an available executable.") - return str(canonical) + return configured entries: list[str] = [] executable: str | None = None diff --git a/sdk/typescript/src/api.ts b/sdk/typescript/src/api.ts index 7444bbf17..d34ec1181 100644 --- a/sdk/typescript/src/api.ts +++ b/sdk/typescript/src/api.ts @@ -645,10 +645,7 @@ export class CodexSecurity { options.auth, modelProvider, ); - const protectedGitRoot = await outermostGitMarkerRoot( - protectedRoot, - signal, - ); + const protectedGitRoot = await outermostGitMarkerRoot(repo, signal); const gitEnvironment = await trustedExecutableEnvironment( "git", pluginEnvironment, @@ -1794,8 +1791,17 @@ export class CodexSecurity { validateMode(normalized, mode); await validateCommittedDiffCheckout(repo, normalized, signal); throwIfAborted(signal); + const enclosingRoot = await enclosingGitWorktreeRoot(repo, signal); + const repositoryRelative = + enclosingRoot === null ? null : relative(enclosingRoot, repo); const protectedRoot = - (await enclosingGitWorktreeRoot(repo, signal)) ?? repo; + enclosingRoot !== null && + repositoryRelative !== null && + repositoryRelative !== ".." && + !repositoryRelative.startsWith(`..${sep}`) && + !isAbsolute(repositoryRelative) + ? enclosingRoot + : repo; const requestedOutput = await validateOutputDir( options.outputDir, options.archiveExisting, diff --git a/sdk/typescript/tests-ts/workbench-trusted-git.test.ts b/sdk/typescript/tests-ts/workbench-trusted-git.test.ts index 5dd274028..f3fc1132d 100644 --- a/sdk/typescript/tests-ts/workbench-trusted-git.test.ts +++ b/sdk/typescript/tests-ts/workbench-trusted-git.test.ts @@ -14,7 +14,7 @@ import { delimiter, dirname, join } from "node:path"; import type { CodexOptions } from "@openai/codex-sdk"; import { afterEach, describe, expect, test } from "bun:test"; import { CodexSecurity } from "../src/api.js"; -import { runWorkbench } from "../src/runtime.js"; +import { resolvePluginPython, runWorkbench } from "../src/runtime.js"; import { trustedExecutableEnvironment } from "../src/trusted-executable.js"; import { PLUGIN_ROOT } from "./plugin-root.js"; import { preparedRuntime } from "./support/api-events.js"; @@ -197,14 +197,49 @@ describe("bundled workbench trusted Git", () => { expect(result.stderr).toContain(unsafeExecutable); }); + testPosix("preserves trusted multicall Git symlink invocation", () => { + const target = fixture(); + const trustedDirectory = join(target.root, "trusted-bin"); + mkdirSync(trustedDirectory); + const multicall = join(trustedDirectory, "multicall"); + const invocation = join(trustedDirectory, "git"); + writeFileSync( + multicall, + '#!/bin/sh\ncase "$0" in */git) printf "%s" "$0";; *) exit 23;; esac\n', + { mode: 0o700 }, + ); + symlinkSync(multicall, invocation); + + const result = probe( + target, + [ + "result = git_command(Path(sys.argv[2]), 'status', text=True)", + "print(json.dumps({'git': result.args[0], 'invocation': result.stdout, 'status': result.returncode}))", + ], + { environment: { CODEX_SECURITY_GIT: invocation } }, + ); + expect(result.status, result.stderr).toBe(0); + expect(JSON.parse(result.stdout)).toEqual({ + git: invocation, + invocation, + status: 0, + }); + }); + testPosix("rejects repository symlinks and aliased parents", () => { const target = fixture(); const linkedGit = join(target.repository, "safe-looking-git"); const repositoryAlias = join(target.root, "repository-alias"); + const externalAlias = join(target.root, "external-git"); symlinkSync(target.git, linkedGit); symlinkSync(target.repository, repositoryAlias, "junction"); + symlinkSync(target.shim, externalAlias); - const aliases = [linkedGit, join(repositoryAlias, "safe-looking-git")]; + const aliases = [ + linkedGit, + join(repositoryAlias, "safe-looking-git"), + externalAlias, + ]; for (const executable of aliases) { const result = probe(target, statusProbe, { environment: { CODEX_SECURITY_GIT: executable }, @@ -311,6 +346,137 @@ describe("bundled workbench trusted Git", () => { }, ); + testPosix( + "keeps core.worktree redirection from trusting scanned repository aliases", + async () => { + const target = fixture(); + git(target, target.repository, "init", "-q"); + const redirectedRoot = join(target.root, "redirected-worktree"); + mkdirSync(redirectedRoot); + git(target, target.repository, "config", "core.worktree", redirectedRoot); + expect( + git(target, target.repository, "rev-parse", "--show-toplevel"), + ).toBe(redirectedRoot); + writeFileSync(join(target.repository, "source.py"), "value = 1\n"); + + const unsafeDirectory = dirname(target.shim); + const repositoryRipgrep = join(unsafeDirectory, "rg"); + const repositoryPython = join(unsafeDirectory, "python"); + writeFileSync(repositoryRipgrep, readFileSync(target.shim), { + mode: 0o700, + }); + writeFileSync(repositoryPython, readFileSync(target.shim), { + mode: 0o700, + }); + const gitAlias = join(target.root, "git-alias-bin"); + const ripgrepAlias = join(target.root, "rg-alias-bin"); + const safeRipgrep = join(target.root, "trusted-rg-bin"); + const codexHome = join(target.root, "codex-home"); + for (const directory of [ + gitAlias, + ripgrepAlias, + safeRipgrep, + codexHome, + ]) { + mkdirSync(directory); + } + symlinkSync(target.shim, join(gitAlias, "git")); + symlinkSync(repositoryRipgrep, join(ripgrepAlias, "rg")); + writeFileSync( + join(safeRipgrep, "rg"), + '#!/bin/sh\nprintf "source.py\\n"\n', + { mode: 0o700 }, + ); + const environment = { + ...target.environment, + CODEX_SECURITY_STATE_DIR: join(target.root, "state"), + PATH: [ + unsafeDirectory, + gitAlias, + ripgrepAlias, + safeRipgrep, + dirname(target.git), + ].join(delimiter), + }; + const observed: Array> = []; + let pythonProtectedRoot: string | undefined; + const client = new TestClient( + {}, + { + environment, + prepareRuntime: async () => ({ + ...preparedRuntime(codexHome), + environment, + }), + resolvePluginPython: async (options: { protectedRoot?: string }) => { + pythonProtectedRoot = options.protectedRoot; + return target.python; + }, + repositoryRevision: async () => null, + runWorkbench: async (...args: Parameters) => { + observed.push(args[0].environment); + throw new Error("captured redirected-worktree environment"); + }, + }, + ); + try { + await expect( + client.preflight(target.repository, { + outputDir: join(target.repository, "scan-output"), + }), + ).rejects.toThrow("outside"); + await expect( + client.run(target.repository, { + outputDir: join(target.root, "scan"), + }), + ).rejects.toThrow("captured redirected-worktree environment"); + } finally { + await client.close(); + } + + expect(observed.length).toBe(1); + expect(pythonProtectedRoot).toBe(target.repository); + await expect( + resolvePluginPython({ + configuredPath: repositoryPython, + environment, + protectedRoot: pythonProtectedRoot, + }), + ).rejects.toThrow(); + for (const candidate of observed) { + expect(candidate["CODEX_SECURITY_GIT"]).toBe(target.git); + expect(candidate["PATH"]?.split(delimiter)).toEqual([ + safeRipgrep, + dirname(target.git), + ]); + } + const trustedGit = spawnSync("git", ["--version"], { + encoding: "utf8", + env: observed[0], + }); + expect(trustedGit.status, trustedGit.stderr).toBe(0); + + const inventoryPath = join(target.root, "inventory.txt"); + const inventory = spawnSync( + target.python, + [ + "-I", + "-B", + join(PLUGIN_ROOT, "scripts", "generate_in_scope_files.py"), + "--repo", + target.repository, + "--scope", + ".", + "--out", + inventoryPath, + ], + { encoding: "utf8", env: observed[0] }, + ); + expect(inventory.status, inventory.stderr).toBe(0); + expect(readFileSync(inventoryPath, "utf8")).toBe("source.py\n"); + }, + ); + testPosix( "keeps optional-Git scans safe from repository Git and ripgrep aliases", async () => { From 78123a7e995c25415cf7a3244393be90025e6981 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Sat, 15 Aug 2026 13:56:44 -0700 Subject: [PATCH 04/27] fix(git): reject repository aliases by filesystem identity --- .../scripts/workbench_target.py | 30 ++-- .../tests-ts/workbench-trusted-git.test.ts | 130 ++++++++++++++++++ 2 files changed, 151 insertions(+), 9 deletions(-) diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_target.py b/sdk/typescript/_bundled_plugin/scripts/workbench_target.py index e9d38f14c..311c73dfe 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_target.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_target.py @@ -131,6 +131,13 @@ def _protected_git_root(target: Path) -> Path: return root +def _inside_protected_git_root(candidate: Path, root: Path) -> bool: + return candidate.is_relative_to(root) or ( + len(candidate.parts) >= len(root.parts) + and Path(*candidate.parts[: len(root.parts)]).samefile(root) + ) + + def _trusted_git_executable(target: Path, environment: dict[str, str]) -> str | None: root = _protected_git_root(target) configured = environment.get("CODEX_SECURITY_GIT") @@ -141,20 +148,25 @@ def _trusted_git_executable(target: Path, environment: dict[str, str]) -> str | windows = sys.platform == "win32" if not candidate.is_absolute(): raise SystemExit("CODEX_SECURITY_GIT must name an absolute trusted executable.") + invocation = Path(os.path.abspath(configured)) + if invocation.is_relative_to(root): + raise SystemExit("CODEX_SECURITY_GIT must stay outside the protected repository.") try: - parent = candidate.parent.resolve(strict=True) + for ancestor in (candidate, *candidate.parents, invocation, *invocation.parents): + if _inside_protected_git_root(ancestor.resolve(strict=True), root): + raise SystemExit( + "CODEX_SECURITY_GIT must stay outside the protected repository." + ) canonical = candidate.resolve(strict=True) except OSError as error: raise SystemExit("CODEX_SECURITY_GIT does not name an available executable.") from error - if parent.is_relative_to(root) or canonical.is_relative_to(root): - raise SystemExit("CODEX_SECURITY_GIT must stay outside the protected repository.") if ( not canonical.is_file() or ( windows and ( candidate.suffix.lower() not in {".exe", ".com"} - or canonical.suffix.lower() not in {".exe", ".com"} + or canonical.suffix.lower() in {".bat", ".cmd"} ) ) or not os.access(canonical, os.F_OK if windows else os.X_OK) @@ -170,21 +182,21 @@ def _trusted_git_executable(target: Path, environment: dict[str, str]) -> str | continue try: directory = Path(entry).resolve(strict=True) + if _inside_protected_git_root(directory, root): + continue except OSError: continue - if directory.is_relative_to(root): - continue candidate: str | None = None safe = True for name in names: path = directory / name try: canonical = path.resolve(strict=True) + if _inside_protected_git_root(canonical, root): + safe = False + break except OSError: continue - if canonical.is_relative_to(root): - safe = False - break if canonical.is_file() and os.access( canonical, os.F_OK if sys.platform == "win32" else os.X_OK ): diff --git a/sdk/typescript/tests-ts/workbench-trusted-git.test.ts b/sdk/typescript/tests-ts/workbench-trusted-git.test.ts index f3fc1132d..692798c0b 100644 --- a/sdk/typescript/tests-ts/workbench-trusted-git.test.ts +++ b/sdk/typescript/tests-ts/workbench-trusted-git.test.ts @@ -6,6 +6,7 @@ import { readFileSync, realpathSync, rmSync, + statSync, symlinkSync, writeFileSync, } from "node:fs"; @@ -148,6 +149,51 @@ describe("bundled workbench trusted Git", () => { realpathSync(target.git), ); } + + const uppercaseRepository = join(target.root, "REPOSITORY"); + if (existsSync(uppercaseRepository)) { + const repositoryIdentity = statSync(target.repository); + const uppercaseIdentity = statSync(uppercaseRepository); + if ( + repositoryIdentity.dev === uppercaseIdentity.dev && + repositoryIdentity.ino === uppercaseIdentity.ino + ) { + const unsafeDirectory = join( + uppercaseRepository, + "node_modules", + ".bin", + ); + const externalAlias = join(target.root, "casefold-alias-bin"); + mkdirSync(externalAlias); + symlinkSync(join(unsafeDirectory, "git"), join(externalAlias, "git")); + const result = probe( + target, + [ + "import os, workbench_target", + "environment = dict(os.environ)", + "selected = workbench_target._trusted_git_executable(Path(sys.argv[2]), environment)", + "result = git_command(Path(sys.argv[2]), 'config', '--get', 'codexsecurity.synthetic', text=True)", + "print(json.dumps({'configured': 'CODEX_SECURITY_GIT' in os.environ, 'git': result.args[0], 'selected': selected, 'path': environment['PATH'], 'status': result.returncode, 'value': result.stdout.strip()}))", + ], + { + environment: { + PATH: [unsafeDirectory, externalAlias, dirname(target.git)].join( + delimiter, + ), + }, + }, + ); + expect(result.status, result.stderr).toBe(0); + expect(JSON.parse(result.stdout)).toEqual({ + configured: false, + git: target.git, + selected: target.git, + path: dirname(target.git), + status: 0, + value: "operator-config-preserved", + }); + } + } }); test("keeps Git optional when no trusted executable exists", () => { @@ -231,15 +277,60 @@ describe("bundled workbench trusted Git", () => { const linkedGit = join(target.repository, "safe-looking-git"); const repositoryAlias = join(target.root, "repository-alias"); const externalAlias = join(target.root, "external-git"); + const repositoryOwnedLink = join(target.repository, "trusted-link"); + const outside = join(target.root, "outside"); + mkdirSync(outside); symlinkSync(target.git, linkedGit); symlinkSync(target.repository, repositoryAlias, "junction"); symlinkSync(target.shim, externalAlias); + symlinkSync(dirname(target.git), repositoryOwnedLink, "junction"); + const rawRepositoryAlias = join(outside, "repo-alias"); + symlinkSync(target.repository, rawRepositoryAlias, "junction"); + for (const directory of [target.root, outside]) { + symlinkSync( + dirname(target.git), + join(directory, "escaped-bin"), + "junction", + ); + } const aliases = [ linkedGit, join(repositoryAlias, "safe-looking-git"), externalAlias, + join(repositoryOwnedLink, "git"), + join(repositoryAlias, "trusted-link", "git"), + `${outside}/../repository/trusted-link/git`, + `${rawRepositoryAlias}/../escaped-bin/git`, ]; + const mixedCaseAlias = join( + target.root, + "REPOSITORY", + "trusted-link", + "git", + ); + if (existsSync(mixedCaseAlias)) { + const repositoryIdentity = statSync(target.repository); + const aliasIdentity = statSync(join(target.root, "REPOSITORY")); + if ( + repositoryIdentity.dev === aliasIdentity.dev && + repositoryIdentity.ino === aliasIdentity.ino + ) { + aliases.push(mixedCaseAlias); + const mixedCaseExternal = join(outside, "case-git"); + symlinkSync( + join(target.root, "REPOSITORY", "node_modules", ".bin", "git"), + mixedCaseExternal, + ); + aliases.push(mixedCaseExternal); + const nested = join(target.repository, "nested"); + mkdirSync(nested); + symlinkSync(dirname(target.git), join(nested, "escape"), "junction"); + const nestedAlias = join(outside, "nested-alias"); + symlinkSync(join(target.root, "REPOSITORY", "nested"), nestedAlias); + aliases.push(join(nestedAlias, "escape", "git")); + } + } for (const executable of aliases) { const result = probe(target, statusProbe, { environment: { CODEX_SECURITY_GIT: executable }, @@ -281,6 +372,45 @@ describe("bundled workbench trusted Git", () => { }); }); + testPosix( + "accepts Windows Git aliases to extensionless executables, not batch files", + () => { + const target = fixture(); + const trustedDirectory = join(target.root, "trusted-bin"); + mkdirSync(trustedDirectory); + const multicall = join(trustedDirectory, "multicall"); + const executable = join(trustedDirectory, "git.exe"); + const batch = join(trustedDirectory, "git.cmd"); + const batchAlias = join(trustedDirectory, "batch.exe"); + writeFileSync(multicall, "synthetic native executable\n"); + writeFileSync(batch, "synthetic batch shim\n"); + symlinkSync(multicall, executable); + symlinkSync(batch, batchAlias); + + const result = probe( + target, + [ + "import os, workbench_target", + "workbench_target.sys.platform = 'win32'", + "root = Path(sys.argv[2]).parent / 'trusted-bin'", + "selected = workbench_target._trusted_git_executable(Path(sys.argv[2]), dict(os.environ))", + "configured = workbench_target._trusted_git_executable(Path(sys.argv[2]), {**os.environ, 'CODEX_SECURITY_GIT': str(root / 'git.exe')})", + "try: workbench_target._trusted_git_executable(Path(sys.argv[2]), {**os.environ, 'CODEX_SECURITY_GIT': str(root / 'batch.exe')})", + "except SystemExit: batch_rejected = True", + "else: batch_rejected = False", + "print(json.dumps({'executable': selected, 'configured': configured, 'batchRejected': batch_rejected}))", + ], + { environment: { PATH: trustedDirectory } }, + ); + expect(result.status, result.stderr).toBe(0); + expect(JSON.parse(result.stdout)).toEqual({ + executable, + configured: executable, + batchRejected: true, + }); + }, + ); + testPosix( "uses trusted Git for real diff ranking and committed inventory", () => { From 15a97eff00830ba32beb2564434808ad6899b346 Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Sat, 15 Aug 2026 23:11:07 -0700 Subject: [PATCH 05/27] fix(git): reuse trusted lookup and preserve platform defaults --- .../scripts/workbench_target.py | 45 +++++++++---------- sdk/typescript/src/api.ts | 21 +++------ sdk/typescript/src/trusted-executable.ts | 30 +++++-------- .../tests-ts/workbench-trusted-git.test.ts | 36 ++++++++++----- 4 files changed, 65 insertions(+), 67 deletions(-) diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_target.py b/sdk/typescript/_bundled_plugin/scripts/workbench_target.py index 311c73dfe..29a07d378 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_target.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_target.py @@ -138,6 +138,21 @@ def _inside_protected_git_root(candidate: Path, root: Path) -> bool: ) +def _is_git_executable(candidate: Path, canonical: Path) -> bool: + windows = sys.platform == "win32" + return ( + canonical.is_file() + and ( + not windows + or ( + candidate.suffix.lower() in {".exe", ".com"} + and canonical.suffix.lower() not in {".bat", ".cmd"} + ) + ) + and os.access(canonical, os.F_OK if windows else os.X_OK) + ) + + def _trusted_git_executable(target: Path, environment: dict[str, str]) -> str | None: root = _protected_git_root(target) configured = environment.get("CODEX_SECURITY_GIT") @@ -145,32 +160,18 @@ def _trusted_git_executable(target: Path, environment: dict[str, str]) -> str | if not configured: return None candidate = Path(configured) - windows = sys.platform == "win32" if not candidate.is_absolute(): raise SystemExit("CODEX_SECURITY_GIT must name an absolute trusted executable.") - invocation = Path(os.path.abspath(configured)) - if invocation.is_relative_to(root): - raise SystemExit("CODEX_SECURITY_GIT must stay outside the protected repository.") try: - for ancestor in (candidate, *candidate.parents, invocation, *invocation.parents): - if _inside_protected_git_root(ancestor.resolve(strict=True), root): - raise SystemExit( - "CODEX_SECURITY_GIT must stay outside the protected repository." - ) canonical = candidate.resolve(strict=True) + if _inside_protected_git_root(canonical, root) or any( + _inside_protected_git_root(ancestor.resolve(strict=True), root) + for ancestor in candidate.parents + ): + raise SystemExit("CODEX_SECURITY_GIT must stay outside the protected repository.") except OSError as error: raise SystemExit("CODEX_SECURITY_GIT does not name an available executable.") from error - if ( - not canonical.is_file() - or ( - windows - and ( - candidate.suffix.lower() not in {".exe", ".com"} - or canonical.suffix.lower() in {".bat", ".cmd"} - ) - ) - or not os.access(canonical, os.F_OK if windows else os.X_OK) - ): + if not _is_git_executable(candidate, canonical): raise SystemExit("CODEX_SECURITY_GIT does not name an available executable.") return configured @@ -197,9 +198,7 @@ def _trusted_git_executable(target: Path, environment: dict[str, str]) -> str | break except OSError: continue - if canonical.is_file() and os.access( - canonical, os.F_OK if sys.platform == "win32" else os.X_OK - ): + if _is_git_executable(path, canonical): candidate = candidate or str(path) if not safe: continue diff --git a/sdk/typescript/src/api.ts b/sdk/typescript/src/api.ts index e75c833b1..5f7bb474e 100644 --- a/sdk/typescript/src/api.ts +++ b/sdk/typescript/src/api.ts @@ -120,10 +120,7 @@ import { validateCommittedDiffCheckout, validateMode, } from "./targets.js"; -import { - resolveTrustedExecutable, - trustedExecutableEnvironment, -} from "./trusted-executable.js"; +import { inspectTrustedExecutable } from "./trusted-executable.js"; interface CodexThreadLike { readonly id: string | null; @@ -648,23 +645,19 @@ export class CodexSecurity { modelProvider, ); const protectedGitRoot = await outermostGitMarkerRoot(repo, signal); - const gitEnvironment = await trustedExecutableEnvironment( + const git = await inspectTrustedExecutable( "git", pluginEnvironment, protectedGitRoot, ); - const git = await resolveTrustedExecutable( - "git", - gitEnvironment, + const ripgrep = await inspectTrustedExecutable( + "rg", + git.environment, protectedGitRoot, ); const trustedPluginEnvironment = { - ...(await trustedExecutableEnvironment( - "rg", - git?.environment ?? gitEnvironment, - protectedGitRoot, - )), - CODEX_SECURITY_GIT: git?.executable ?? "", + ...ripgrep.environment, + CODEX_SECURITY_GIT: git.executable ?? "", }; checkOpen(); const scanOutputRoot = diff --git a/sdk/typescript/src/trusted-executable.ts b/sdk/typescript/src/trusted-executable.ts index 1b0ebe5cb..10d11559b 100644 --- a/sdk/typescript/src/trusted-executable.ts +++ b/sdk/typescript/src/trusted-executable.ts @@ -22,16 +22,7 @@ export async function resolveTrustedExecutable( : { executable: inspected.executable, environment: inspected.environment }; } -export async function trustedExecutableEnvironment( - candidate: string, - environment: Readonly>, - protectedRoot: string, -): Promise> { - return (await inspectTrustedExecutable(candidate, environment, protectedRoot)) - .environment; -} - -async function inspectTrustedExecutable( +export async function inspectTrustedExecutable( candidate: string, environment: Readonly>, protectedRoot: string, @@ -45,8 +36,13 @@ async function inspectTrustedExecutable( const path = Object.entries(environment).find( ([name]) => name.toUpperCase() === "PATH", )?.[1]; + // Match child_process lookup defaults without broadening an explicit PATH. + const searchPath = + path ?? + (process.platform === "win32" ? process.env["PATH"] : "/usr/bin:/bin") ?? + ""; const entries: string[] = []; - for (const entry of path?.split(delimiter) ?? []) { + for (const entry of searchPath.split(delimiter)) { if (entry.length === 0) continue; const canonical = await realpath(entry).catch(() => null); if (canonical === null || isWithin(root, canonical)) continue; @@ -98,14 +94,12 @@ async function inspectTrustedExecutable( } } const sanitizedEnvironment = { ...environment }; - if (path !== undefined) { - for (const name of Object.keys(sanitizedEnvironment)) { - if (name.toUpperCase() === "PATH") delete sanitizedEnvironment[name]; - } - sanitizedEnvironment["PATH"] = entries - .filter((entry) => !unsafeEntries.has(entry)) - .join(delimiter); + for (const name of Object.keys(sanitizedEnvironment)) { + if (name.toUpperCase() === "PATH") delete sanitizedEnvironment[name]; } + sanitizedEnvironment["PATH"] = entries + .filter((entry) => !unsafeEntries.has(entry)) + .join(delimiter); return { executable, environment: sanitizedEnvironment }; } diff --git a/sdk/typescript/tests-ts/workbench-trusted-git.test.ts b/sdk/typescript/tests-ts/workbench-trusted-git.test.ts index 692798c0b..736df477d 100644 --- a/sdk/typescript/tests-ts/workbench-trusted-git.test.ts +++ b/sdk/typescript/tests-ts/workbench-trusted-git.test.ts @@ -16,7 +16,7 @@ import type { CodexOptions } from "@openai/codex-sdk"; import { afterEach, describe, expect, test } from "bun:test"; import { CodexSecurity } from "../src/api.js"; import { resolvePluginPython, runWorkbench } from "../src/runtime.js"; -import { trustedExecutableEnvironment } from "../src/trusted-executable.js"; +import { inspectTrustedExecutable } from "../src/trusted-executable.js"; import { PLUGIN_ROOT } from "./plugin-root.js"; import { preparedRuntime } from "./support/api-events.js"; @@ -210,28 +210,40 @@ describe("bundled workbench trusted Git", () => { expect(JSON.parse(result.stdout)).toEqual({ status: 127, output: "" }); }); - test("preserves absent PATH while clearing explicitly unsafe PATH", async () => { + test("uses default lookup only when PATH is absent", async () => { const target = fixture(); - const missing = await trustedExecutableEnvironment( + const defaultPath = + process.platform === "win32" ? process.env["PATH"] : "/usr/bin:/bin"; + const expected = await inspectTrustedExecutable( "git", - { HOME: target.root }, + { HOME: target.root, PATH: defaultPath ?? "" }, target.repository, ); - expect(missing).not.toHaveProperty("PATH"); - - const undefinedPath = await trustedExecutableEnvironment( + const missing = await inspectTrustedExecutable( "git", - { HOME: target.root, PATH: undefined }, + { HOME: target.root }, target.repository, ); - expect(undefinedPath["PATH"]).toBeUndefined(); + expect(missing).toEqual(expected); - const unsafe = await trustedExecutableEnvironment( + const undefinedPath = await inspectTrustedExecutable( "git", - { HOME: target.root, PATH: dirname(target.shim) }, + { HOME: target.root, PATH: undefined }, target.repository, ); - expect(unsafe["PATH"]).toBe(""); + expect(undefinedPath).toEqual(expected); + + for (const path of ["", dirname(target.shim)]) { + const unavailable = await inspectTrustedExecutable( + "git", + { HOME: target.root, PATH: path }, + target.repository, + ); + expect(unavailable).toEqual({ + executable: null, + environment: { HOME: target.root, PATH: "" }, + }); + } }); test("rejects explicitly selected repository-controlled Git", () => { From 357b437555ff6d0083f6ec5ac5c1c716f7dffe02 Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Sun, 16 Aug 2026 01:12:07 -0700 Subject: [PATCH 06/27] fix(git): bind optional tools with platform-aware environments --- sdk/typescript/_bundled_plugin/.mcp.json | 1 + .../scripts/generate_in_scope_files.py | 16 +- .../scripts/generate_rank_input.py | 11 +- .../scripts/workbench_target.py | 57 ++++- sdk/typescript/src/api.ts | 2 + sdk/typescript/src/trusted-executable.ts | 14 +- .../workbench-tool-environment.test.ts | 227 ++++++++++++++++++ 7 files changed, 296 insertions(+), 32 deletions(-) create mode 100644 sdk/typescript/tests-ts/workbench-tool-environment.test.ts diff --git a/sdk/typescript/_bundled_plugin/.mcp.json b/sdk/typescript/_bundled_plugin/.mcp.json index 40fbec048..72f155a66 100644 --- a/sdk/typescript/_bundled_plugin/.mcp.json +++ b/sdk/typescript/_bundled_plugin/.mcp.json @@ -29,6 +29,7 @@ "AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE", "PYTHON", "CODEX_SECURITY_GIT", + "CODEX_SECURITY_RG", "CODEX_SECURITY_KNOWLEDGE_BASE", "CODEX_SECURITY_DEEP_SCAN_CONFIG_PATH", "CODEX_SECURITY_SCAN_ROOT", diff --git a/sdk/typescript/_bundled_plugin/scripts/generate_in_scope_files.py b/sdk/typescript/_bundled_plugin/scripts/generate_in_scope_files.py index 65f268815..158b1ca63 100644 --- a/sdk/typescript/_bundled_plugin/scripts/generate_in_scope_files.py +++ b/sdk/typescript/_bundled_plugin/scripts/generate_in_scope_files.py @@ -10,6 +10,10 @@ import tempfile from pathlib import Path +# Some plugin hosts launch Python with safe-path isolation enabled. +sys.path.insert(0, str(Path(__file__).resolve().parent)) +from workbench_target import git_command, ripgrep_command + class InventoryError(ValueError): """Raised when the repository, scope, or inventory cannot be used safely.""" @@ -70,7 +74,6 @@ def resolve_output(value: str) -> Path: def generate_in_scope_files(repository: Path, scope: str, output: Path) -> int: """Atomically write the exact ripgrep inventory sorted as ``LC_ALL=C``.""" command = [ - "rg", "--files", "--hidden", "--no-ignore", @@ -83,13 +86,7 @@ def generate_in_scope_files(repository: Path, scope: str, output: Path) -> int: ] with tempfile.TemporaryFile(mode="w+b") as inventory: try: - result = subprocess.run( - command, - cwd=repository, - stdout=inventory, - stderr=subprocess.PIPE, - check=False, - ) + result = ripgrep_command(repository, *command, stdout=inventory) except OSError as error: raise InventoryError(f"could not run ripgrep: {error}") from error @@ -107,8 +104,6 @@ def generate_in_scope_files(repository: Path, scope: str, output: Path) -> int: def committed_changed_paths(repository: Path, base: str, head: str) -> list[tuple[Path, str]]: - from workbench_target import git_command - result = git_command( repository, "diff", @@ -144,7 +139,6 @@ def generate_diff_in_scope_files( output: Path, ) -> int: """Reuse the existing diff selection without generating previews or duplicate worklists.""" - sys.path.insert(0, str(Path(__file__).resolve().parent)) from generate_rank_input import git_changed_paths, path_is_excluded from rank_preview import ( DEFAULT_PREVIEW_BYTES, diff --git a/sdk/typescript/_bundled_plugin/scripts/generate_rank_input.py b/sdk/typescript/_bundled_plugin/scripts/generate_rank_input.py index ef5722d6a..b7a277028 100644 --- a/sdk/typescript/_bundled_plugin/scripts/generate_rank_input.py +++ b/sdk/typescript/_bundled_plugin/scripts/generate_rank_input.py @@ -29,7 +29,6 @@ import json import os import re -import subprocess import sys from collections import Counter from collections.abc import Callable @@ -43,7 +42,12 @@ preview_for, preview_for_bytes, ) -from workbench_target import git_blob_bytes, git_command, git_directory_snapshot_paths +from workbench_target import ( + git_blob_bytes, + git_command, + git_directory_snapshot_paths, + ripgrep_command, +) EXCLUDED_DIRS = { ".cache", @@ -521,7 +525,6 @@ def make_repo_scope_input(args: argparse.Namespace) -> None: candidates = git_candidates else: command = [ - "rg", "--files", "--hidden", "--no-require-git", @@ -532,7 +535,7 @@ def make_repo_scope_input(args: argparse.Namespace) -> None: str(scope_path.relative_to(repo)), ] try: - result = subprocess.run(command, cwd=repo, capture_output=True, check=False) + result = ripgrep_command(repo, *command) except OSError as exc: ignore_names = (".gitignore", ".ignore", ".rgignore") ancestors = (scope_path, *scope_path.parents) diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_target.py b/sdk/typescript/_bundled_plugin/scripts/workbench_target.py index 29a07d378..f1acdc691 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_target.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_target.py @@ -11,7 +11,7 @@ import subprocess import sys from pathlib import Path -from typing import Any +from typing import IO, Any # Some plugin hosts launch Python with safe-path isolation enabled. sys.path.insert(0, str(Path(__file__).resolve().parent)) @@ -138,7 +138,7 @@ def _inside_protected_git_root(candidate: Path, root: Path) -> bool: ) -def _is_git_executable(candidate: Path, canonical: Path) -> bool: +def _is_native_executable(candidate: Path, canonical: Path) -> bool: windows = sys.platform == "win32" return ( canonical.is_file() @@ -153,31 +153,43 @@ def _is_git_executable(candidate: Path, canonical: Path) -> bool: ) -def _trusted_git_executable(target: Path, environment: dict[str, str]) -> str | None: +def _trusted_executable( + target: Path, + environment: dict[str, str], + name: str, +) -> str | None: root = _protected_git_root(target) - configured = environment.get("CODEX_SECURITY_GIT") + setting = f"CODEX_SECURITY_{name.upper()}" + configured = environment.get(setting) if configured is not None: if not configured: return None candidate = Path(configured) if not candidate.is_absolute(): - raise SystemExit("CODEX_SECURITY_GIT must name an absolute trusted executable.") + raise SystemExit(f"{setting} must name an absolute trusted executable.") try: canonical = candidate.resolve(strict=True) if _inside_protected_git_root(canonical, root) or any( _inside_protected_git_root(ancestor.resolve(strict=True), root) for ancestor in candidate.parents ): - raise SystemExit("CODEX_SECURITY_GIT must stay outside the protected repository.") + raise SystemExit(f"{setting} must stay outside the protected repository.") except OSError as error: - raise SystemExit("CODEX_SECURITY_GIT does not name an available executable.") from error - if not _is_git_executable(candidate, canonical): - raise SystemExit("CODEX_SECURITY_GIT does not name an available executable.") + raise SystemExit(f"{setting} does not name an available executable.") from error + if not _is_native_executable(candidate, canonical): + raise SystemExit(f"{setting} does not name an available executable.") return configured entries: list[str] = [] executable: str | None = None - names = ("git.exe", "git.com") if sys.platform == "win32" else ("git",) + names = (f"{name}.exe", f"{name}.com") if sys.platform == "win32" else (name,) + if sys.platform == "win32": + path_keys = sorted(key for key in environment if key.upper() == "PATH") + if path_keys: + path = environment[path_keys[0]] + for key in path_keys: + del environment[key] + environment["PATH"] = path for entry in os.get_exec_path(environment): if not entry: continue @@ -198,7 +210,7 @@ def _trusted_git_executable(target: Path, environment: dict[str, str]) -> str | break except OSError: continue - if _is_git_executable(path, canonical): + if _is_native_executable(path, canonical): candidate = candidate or str(path) if not safe: continue @@ -208,6 +220,29 @@ def _trusted_git_executable(target: Path, environment: dict[str, str]) -> str | return executable +def _trusted_git_executable(target: Path, environment: dict[str, str]) -> str | None: + return _trusted_executable(target, environment, "git") + + +def ripgrep_command( + target: Path, + *args: str, + stdout: IO[bytes] | int = subprocess.PIPE, +) -> subprocess.CompletedProcess[bytes]: + environment = os.environ.copy() + executable = _trusted_executable(target, environment, "rg") + if executable is None: + raise FileNotFoundError("ripgrep is not available on a trusted PATH.") + return subprocess.run( + [executable, *args], + cwd=target, + stdout=stdout, + stderr=subprocess.PIPE, + env=environment, + check=False, + ) + + def git_command( target: Path, *args: str, diff --git a/sdk/typescript/src/api.ts b/sdk/typescript/src/api.ts index 5f7bb474e..6d6737056 100644 --- a/sdk/typescript/src/api.ts +++ b/sdk/typescript/src/api.ts @@ -658,6 +658,8 @@ export class CodexSecurity { const trustedPluginEnvironment = { ...ripgrep.environment, CODEX_SECURITY_GIT: git.executable ?? "", + // The Codex runtime can add its bundled tools to PATH after this point. + CODEX_SECURITY_RG: ripgrep.executable ?? undefined, }; checkOpen(); const scanOutputRoot = diff --git a/sdk/typescript/src/trusted-executable.ts b/sdk/typescript/src/trusted-executable.ts index 10d11559b..82942e151 100644 --- a/sdk/typescript/src/trusted-executable.ts +++ b/sdk/typescript/src/trusted-executable.ts @@ -33,9 +33,13 @@ export async function inspectTrustedExecutable( const root = await realpath(protectedRoot).catch(() => resolve(protectedRoot), ); - const path = Object.entries(environment).find( - ([name]) => name.toUpperCase() === "PATH", - )?.[1]; + const pathKeys = + process.platform === "win32" + ? Object.keys(environment) + .filter((name) => name.toUpperCase() === "PATH") + .sort() + : ["PATH"]; + const path = environment[pathKeys[0] ?? "PATH"]; // Match child_process lookup defaults without broadening an explicit PATH. const searchPath = path ?? @@ -94,9 +98,7 @@ export async function inspectTrustedExecutable( } } const sanitizedEnvironment = { ...environment }; - for (const name of Object.keys(sanitizedEnvironment)) { - if (name.toUpperCase() === "PATH") delete sanitizedEnvironment[name]; - } + for (const name of pathKeys) delete sanitizedEnvironment[name]; sanitizedEnvironment["PATH"] = entries .filter((entry) => !unsafeEntries.has(entry)) .join(delimiter); diff --git a/sdk/typescript/tests-ts/workbench-tool-environment.test.ts b/sdk/typescript/tests-ts/workbench-tool-environment.test.ts new file mode 100644 index 000000000..94d4a0655 --- /dev/null +++ b/sdk/typescript/tests-ts/workbench-tool-environment.test.ts @@ -0,0 +1,227 @@ +import { spawnSync } from "node:child_process"; +import { readFileSync } from "node:fs"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { describe, expect, test } from "bun:test"; +import { inspectTrustedExecutable } from "../src/trusted-executable.js"; +import { PLUGIN_ROOT } from "./plugin-root.js"; + +function childEnvironment(path: string): NodeJS.ProcessEnv { + return { + PATH: path, + ...(process.env["SystemRoot"] === undefined + ? {} + : { SystemRoot: process.env["SystemRoot"] }), + }; +} + +function inspectPlatformEnvironment( + platform: "linux" | "win32", + environment: Record, +) { + const result = spawnSync( + process.execPath, + [ + "-e", + ` + Object.defineProperty(process, "platform", { value: process.argv[1] }); + const { inspectTrustedExecutable } = await import(process.argv[2]); + console.log(JSON.stringify(await inspectTrustedExecutable( + "rg", JSON.parse(process.argv[3]), process.argv[4], + ))); + `, + platform, + fileURLToPath(new URL("../src/trusted-executable.ts", import.meta.url)), + JSON.stringify(environment), + process.cwd(), + ], + { + encoding: "utf8", + env: childEnvironment(process.env["PATH"] ?? ""), + }, + ); + expect(result.status, result.stderr).toBe(0); + return JSON.parse(result.stdout) as { + executable: string | null; + environment: Record; + }; +} + +function runPythonMocks(source: string): void { + const python = Bun.which("python3") ?? Bun.which("python"); + expect(python).not.toBeNull(); + const result = spawnSync( + python!, + [ + "-I", + "-B", + "-c", + ` +import argparse, io, os, subprocess, sys +from pathlib import Path +from unittest.mock import patch +sys.path.insert(0, sys.argv[1]) +import workbench_target as workbench +${source} +`, + join(PLUGIN_ROOT, "scripts"), + ], + { encoding: "utf8", env: childEnvironment(dirname(python!)) }, + ); + expect(result.status, result.stderr).toBe(0); +} + +describe("workbench tool environments", () => { + test("preserves case-distinct POSIX environment keys", () => { + const environment = { + Path: "case-distinct value", + pAtH: "another value", + PATH: "", + KEEP: "yes", + }; + expect(inspectPlatformEnvironment("linux", environment)).toEqual({ + executable: null, + environment, + }); + }); + + test("normalizes Windows PATH aliases using the effective key", () => { + expect( + inspectPlatformEnvironment("win32", { + Path: "other value", + pAtH: "another value", + PATH: "", + KEEP: "yes", + }), + ).toEqual({ + executable: null, + environment: { KEEP: "yes", PATH: "" }, + }); + }); + + test("keeps default lookup separate from an explicitly empty PATH", async () => { + const defaultPath = + process.platform === "win32" + ? process.env["PATH"] ?? "" + : "/usr/bin:/bin"; + const expected = await inspectTrustedExecutable( + "rg", + { KEEP: "yes", PATH: defaultPath }, + process.cwd(), + ); + expect( + await inspectTrustedExecutable("rg", { KEEP: "yes" }, process.cwd()), + ).toEqual(expected); + expect( + await inspectTrustedExecutable( + "rg", + { KEEP: "yes", PATH: "" }, + process.cwd(), + ), + ).toEqual({ + executable: null, + environment: { KEEP: "yes", PATH: "" }, + }); + }); + + test("uses only a resolved ripgrep command and never spawns when unavailable", () => { + runPythonMocks(` +repository = Path.cwd() +directory = Path(sys.executable).parent +executable = directory / ("rg.exe" if sys.platform == "win32" else "rg") +completed = subprocess.CompletedProcess([str(executable)], 0, b"", b"") +with ( + patch.object(workbench, "_protected_git_root", return_value=repository), + patch.object(Path, "resolve", autospec=True, side_effect=lambda path, strict=False: path), + patch.object(workbench, "_inside_protected_git_root", return_value=False), + patch.object(workbench, "_is_native_executable", side_effect=lambda path, canonical: path == executable), + patch.object(workbench.subprocess, "run", return_value=completed) as run, +): + for environment in ( + {"PATH": str(directory)}, + {"PATH": "", "CODEX_SECURITY_RG": str(executable)}, + ): + with patch.dict(workbench.os.environ, environment, clear=True): + assert workbench.ripgrep_command(repository, "--files") is completed + assert run.call_args.args[0] == [str(executable), "--files"] + assert Path(run.call_args.args[0][0]).is_absolute() + assert run.call_args.kwargs["cwd"] == repository + assert run.call_args.kwargs["env"]["PATH"] == environment["PATH"] + + run.reset_mock() + for environment in ( + {"PATH": ""}, + {"PATH": str(directory), "CODEX_SECURITY_RG": ""}, + ): + with patch.dict(workbench.os.environ, environment, clear=True): + try: + workbench.ripgrep_command(repository, "--files") + except FileNotFoundError: + pass + else: + raise AssertionError("unavailable ripgrep was accepted") + run.assert_not_called() +`); + }); + + test("applies platform-specific PATH names in the Python resolver", () => { + runPythonMocks(` +repository = Path.cwd() +original = {"Path": "case-distinct value", "pAtH": "another value", "PATH": "", "KEEP": "yes"} +with patch.object(workbench, "_protected_git_root", return_value=repository): + for platform in ("linux", "win32"): + environment = dict(original) + with patch.object(workbench.sys, "platform", platform): + assert workbench._trusted_executable(repository, environment, "rg") is None + expected = original if platform == "linux" else {"PATH": "", "KEEP": "yes"} + assert environment == expected +`); + }); + + test("routes inventory and scoped ranking through the unavailable-tool guard", () => { + runPythonMocks(` +import generate_in_scope_files as inventory +import generate_rank_input as ranking +repository = Path.cwd().resolve() +with ( + patch.object(workbench, "_trusted_executable", return_value=None) as resolve, + patch.object(workbench.subprocess, "run") as run, + patch.dict(workbench.os.environ, {"PATH": ""}, clear=True), +): + with patch.object(inventory.tempfile, "TemporaryFile", return_value=io.BytesIO()): + try: + inventory.generate_in_scope_files(repository, ".", Path("unused-inventory")) + except inventory.InventoryError: + pass + else: + raise AssertionError("unavailable inventory tool was accepted") + + with ( + patch.object(Path, "is_dir", return_value=True), + patch.object(Path, "is_file", return_value=False), + patch.object(Path, "exists", return_value=False), + patch.object(Path, "rglob", return_value=()), + patch.object(ranking, "load_scopes_file", return_value=["."]), + patch.object(ranking, "resolve_scope", return_value=repository), + patch.object(ranking, "git_directory_snapshot_paths", return_value=None), + patch.object(ranking, "write_jsonl") as write, + patch("builtins.print"), + ): + ranking.make_repo_scope_input(argparse.Namespace( + repo=str(repository), scopes_file="unused-scopes", out="unused-ranking", + )) + write.assert_called_once_with(Path("unused-ranking"), []) + assert resolve.call_count == 2 + run.assert_not_called() +`); + }); + + test("forwards both trusted tool bindings to the MCP host", () => { + const configuration = JSON.parse( + readFileSync(join(PLUGIN_ROOT, ".mcp.json"), "utf8"), + ) as { mcpServers: Record }; + expect(configuration.mcpServers["codex-security"]!.env_vars).toEqual( + expect.arrayContaining(["CODEX_SECURITY_GIT", "CODEX_SECURITY_RG"]), + ); + }); +}); From b70181855dff40acefec02a2cc59be461f5ef65d Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Sun, 16 Aug 2026 01:40:19 -0700 Subject: [PATCH 07/27] fix(git): tolerate unavailable historical targets --- .../scripts/workbench_target.py | 24 ++++-- .../workbench-tool-environment.test.ts | 76 +++++++++++++++++++ 2 files changed, 92 insertions(+), 8 deletions(-) diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_target.py b/sdk/typescript/_bundled_plugin/scripts/workbench_target.py index f1acdc691..93950d976 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_target.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_target.py @@ -120,14 +120,20 @@ def _read_sized_nul_field( return output[offset:end], end + 1 -def _protected_git_root(target: Path) -> Path: - root = target.resolve() - for ancestor in (root, *root.parents): - try: - (ancestor / ".git").lstat() - except FileNotFoundError: - continue - root = ancestor +def _protected_git_root(target: Path) -> Path | None: + """Return the outermost repository root, or None for a stale target.""" + try: + root = target.resolve(strict=True) + if not stat.S_ISDIR(root.stat().st_mode): + return None + for ancestor in (root, *root.parents): + try: + (ancestor / ".git").lstat() + except FileNotFoundError: + continue + root = ancestor + except (FileNotFoundError, NotADirectoryError): + return None return root @@ -159,6 +165,8 @@ def _trusted_executable( name: str, ) -> str | None: root = _protected_git_root(target) + if root is None: + return None setting = f"CODEX_SECURITY_{name.upper()}" configured = environment.get(setting) if configured is not None: diff --git a/sdk/typescript/tests-ts/workbench-tool-environment.test.ts b/sdk/typescript/tests-ts/workbench-tool-environment.test.ts index 94d4a0655..bf3dfc34d 100644 --- a/sdk/typescript/tests-ts/workbench-tool-environment.test.ts +++ b/sdk/typescript/tests-ts/workbench-tool-environment.test.ts @@ -124,6 +124,82 @@ describe("workbench tool environments", () => { }); }); + test("treats stale target roots as unavailable without hiding other errors", () => { + runPythonMocks(` +import stat +from types import SimpleNamespace +repository = Path.cwd() +directory = SimpleNamespace(st_mode=stat.S_IFDIR) +with ( + patch.object(Path, "resolve", autospec=True, return_value=repository) as resolve, + patch.object(Path, "stat", return_value=directory) as metadata, + patch.object(Path, "lstat", side_effect=FileNotFoundError) as marker, +): + assert workbench._protected_git_root(repository) == repository + resolve.assert_called_once_with(repository, strict=True) + + metadata.return_value = SimpleNamespace(st_mode=stat.S_IFREG) + marker.reset_mock() + assert workbench._protected_git_root(repository) is None + marker.assert_not_called() + metadata.return_value = directory + + for error in (FileNotFoundError, NotADirectoryError): + for operation in (resolve, metadata): + operation.side_effect = error + assert workbench._protected_git_root(repository) is None + operation.side_effect = None + marker.side_effect = NotADirectoryError + assert workbench._protected_git_root(repository) is None + + for operation in (resolve, metadata, marker): + marker.side_effect = FileNotFoundError + failure = PermissionError("target metadata is unavailable") + operation.side_effect = failure + try: + workbench._protected_git_root(repository) + except PermissionError as error: + assert error is failure + else: + raise AssertionError("unrelated filesystem error was hidden") + operation.side_effect = None +`); + }); + + test("keeps stale history probes unavailable without spawning a tool", () => { + runPythonMocks(` +import workbench_scan_history as history +repository = Path.cwd() +before = {"target_id": "historical", "target_path": str(repository)} +after = {"target_id": "selected", "target_path": str(repository.parent)} +with ( + patch.object(workbench, "_protected_git_root", return_value=None), + patch.object(workbench, "_inside_protected_git_root") as inside, + patch.object(workbench.os, "get_exec_path") as lookup, + patch.object(workbench.subprocess, "run") as run, +): + for environment in ( + {"PATH": ""}, + {"PATH": "", "CODEX_SECURITY_GIT": sys.executable, "CODEX_SECURITY_RG": sys.executable}, + ): + with patch.dict(workbench.os.environ, environment, clear=True): + completed = workbench.git_command(repository, "rev-parse", "--show-toplevel", text=True) + assert (completed.returncode, completed.stdout, completed.stderr) == (127, "", "") + assert workbench.git_output(repository, "rev-parse", "--git-common-dir") is None + assert workbench.git_bytes(repository, "rev-parse", "--git-common-dir") is None + assert not history._same_repository(before, after, after_identity=(None, None)) + try: + workbench.ripgrep_command(repository, "--files") + except FileNotFoundError: + pass + else: + raise AssertionError("unavailable target was accepted") + inside.assert_not_called() + lookup.assert_not_called() + run.assert_not_called() +`); + }); + test("uses only a resolved ripgrep command and never spawns when unavailable", () => { runPythonMocks(` repository = Path.cwd() From 743e32f028ab8bb71e5d2607f1b30832269aabba Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Sun, 16 Aug 2026 03:13:37 -0700 Subject: [PATCH 08/27] fix(runtime): stage packaged ripgrep for local installs --- sdk/typescript/src/api.ts | 45 ++++- sdk/typescript/src/runtime.ts | 89 ++++++++- sdk/typescript/tests-ts/api.test.ts | 232 ++++++++++++++++++++++++ sdk/typescript/tests-ts/runtime.test.ts | 182 +++++++++++++++++++ 4 files changed, 538 insertions(+), 10 deletions(-) diff --git a/sdk/typescript/src/api.ts b/sdk/typescript/src/api.ts index 6d6737056..462f82ab1 100644 --- a/sdk/typescript/src/api.ts +++ b/sdk/typescript/src/api.ts @@ -100,6 +100,7 @@ import { resolvePluginPython, runWorkbench, setCodexSecurityCredentialLogout, + stageBundledRipgrep, type CodexCommand, type PluginInstall, type ProcessEnvironment, @@ -147,6 +148,7 @@ interface PreparedRuntime { codexHome: string; persistentCredentialHome?: boolean; bootstrapWorkspace?: string; + bundledRipgrep?: string; configPath?: string; deepScanConfigPath?: string; plugin: PluginInstall; @@ -296,6 +298,7 @@ interface ClientDependencies { prepareOutputDir?: typeof prepareOutputDir; repositoryRevision?: typeof repositoryRevision; resolveCodexCommand?: () => CodexCommand; + stageBundledRipgrep?: typeof stageBundledRipgrep; runWorkbench?: typeof runWorkbench; matchFindings?: typeof matchScanFindings; } @@ -650,17 +653,49 @@ export class CodexSecurity { pluginEnvironment, protectedGitRoot, ); - const ripgrep = await inspectTrustedExecutable( + let ripgrep = await inspectTrustedExecutable( "rg", git.environment, protectedGitRoot, ); - const trustedPluginEnvironment = { + const ripgrepKeys = + process.platform === "win32" + ? Object.keys(pluginEnvironment) + .filter((name) => name.toUpperCase() === "CODEX_SECURITY_RG") + .sort() + : ["CODEX_SECURITY_RG"]; + const ripgrepDisabled = + pluginEnvironment[ripgrepKeys[0] ?? "CODEX_SECURITY_RG"] === ""; + if ( + !ripgrepDisabled && + ripgrep.executable === null && + runtime.bootstrapWorkspace !== undefined + ) { + const workspace = await realpath(runtime.bootstrapWorkspace); + requireOutputOutsideRepository(protectedGitRoot, workspace, "runtime"); + if (runtime.bundledRipgrep === undefined) { + const bundled = await ( + this.#dependencies.stageBundledRipgrep ?? stageBundledRipgrep + )(workspace, signal); + if (bundled !== null) runtime.bundledRipgrep = bundled; + } + if (runtime.bundledRipgrep !== undefined) { + ripgrep = await inspectTrustedExecutable( + runtime.bundledRipgrep, + ripgrep.environment, + protectedGitRoot, + ); + } + } + const trustedPluginEnvironment: ProcessEnvironment = { ...ripgrep.environment, - CODEX_SECURITY_GIT: git.executable ?? "", - // The Codex runtime can add its bundled tools to PATH after this point. - CODEX_SECURITY_RG: ripgrep.executable ?? undefined, }; + for (const name of ripgrepKeys) delete trustedPluginEnvironment[name]; + trustedPluginEnvironment["CODEX_SECURITY_GIT"] = git.executable ?? ""; + // The Codex runtime can add its bundled tools to PATH after this point. + trustedPluginEnvironment["CODEX_SECURITY_RG"] = ripgrepDisabled + ? "" + : ripgrep.executable ?? undefined; checkOpen(); const scanOutputRoot = requestedOutput === null && diff --git a/sdk/typescript/src/runtime.ts b/sdk/typescript/src/runtime.ts index 13c41f2c2..cf5000a9a 100644 --- a/sdk/typescript/src/runtime.ts +++ b/sdk/typescript/src/runtime.ts @@ -2,6 +2,7 @@ import { execFile as execFileCallback, spawn } from "node:child_process"; import { randomUUID } from "node:crypto"; import { constants, existsSync, readdirSync, type Stats } from "node:fs"; import { + access, chmod, cp, copyFile, @@ -22,7 +23,16 @@ import { } from "node:fs/promises"; import { homedir, tmpdir } from "node:os"; import { createRequire } from "node:module"; -import { basename, dirname, extname, join, relative, resolve } from "node:path"; +import { + basename, + dirname, + extname, + isAbsolute, + join, + relative, + resolve, + sep, +} from "node:path"; import { createInterface } from "node:readline"; import { fileURLToPath } from "node:url"; import { promisify } from "node:util"; @@ -1984,7 +1994,14 @@ export function resolveCodexCommand( ) { return { command: resolve(configured) }; } + return { command: resolveBundledCodexPackage().command }; +} +function resolveBundledCodexPackage(): { + packageRoot: string; + root: string; + command: string; +} { const platform = process.platform === "android" ? "linux" : process.platform; const packageName = `@openai/codex-${platform}-${process.arch}`; let packageJson: string; @@ -2000,13 +2017,14 @@ export function resolveCodexCommand( { cause: error }, ); } - const vendor = join(dirname(packageJson), "vendor"); + const packageRoot = dirname(packageJson); + const vendor = join(packageRoot, "vendor"); const target = readdirSync(vendor, { withFileTypes: true }).find((entry) => entry.isDirectory(), ); + const root = join(vendor, target?.name ?? ""); const command = join( - vendor, - target?.name ?? "", + root, "bin", process.platform === "win32" ? "codex.exe" : "codex", ); @@ -2015,7 +2033,68 @@ export function resolveCodexCommand( `The ${packageName} package does not contain the Codex executable. Reinstall @openai/codex with optional dependencies enabled, or set CODEX_CLI_PATH to an installed Codex executable.`, ); } - return { command }; + return { packageRoot, root, command }; +} + +export async function stageBundledRipgrep( + workspace: string, + signal?: AbortSignal, +): Promise { + throwIfSignalAborted(signal); + const name = process.platform === "win32" ? "rg.exe" : "rg"; + let source: string; + try { + // This is the running SDK's own dependency, not a tool found in the scan. + const bundled = resolveBundledCodexPackage(); + const packageRoot = await realpath(bundled.packageRoot); + const candidate = join(bundled.root, "codex-path", name); + const marker = await lstat(join(bundled.root, "codex-package.json")); + const metadata = await lstat(candidate); + source = await realpath(candidate); + const inside = relative(packageRoot, source); + if ( + !marker.isFile() || + marker.isSymbolicLink() || + !metadata.isFile() || + metadata.isSymbolicLink() || + inside === "" || + inside === ".." || + inside.startsWith(`..${sep}`) || + isAbsolute(inside) + ) { + return null; + } + await access( + source, + process.platform === "win32" ? constants.F_OK : constants.X_OK, + ); + } catch (error) { + const cause = error instanceof PluginBootstrapError ? error.cause : error; + if ( + (error instanceof PluginBootstrapError && cause === undefined) || + ["MODULE_NOT_FOUND", "ENOENT", "ENOTDIR", "EACCES"].includes( + nodeErrorCode(cause) ?? "", + ) + ) { + return null; + } + throw error; + } + + const destination = join(workspace, name); + let copied = false; + try { + throwIfSignalAborted(signal); + await copyFile(source, destination, constants.COPYFILE_EXCL); + copied = true; + if (process.platform !== "win32") await chmod(destination, 0o700); + const canonical = await realpath(destination); + throwIfSignalAborted(signal); + return canonical; + } catch (error) { + if (copied) await rm(destination, { force: true }).catch(() => undefined); + throw error; + } } export async function bootstrapPlugin( diff --git a/sdk/typescript/tests-ts/api.test.ts b/sdk/typescript/tests-ts/api.test.ts index 37896482a..870e137a7 100644 --- a/sdk/typescript/tests-ts/api.test.ts +++ b/sdk/typescript/tests-ts/api.test.ts @@ -49,7 +49,10 @@ import { resolveCodexCommand, runWorkbench, setCodexSecurityCredentialLogout, + type WorkbenchCommandOptions, } from "../src/runtime.js"; +import * as runtimeModule from "../src/runtime.js"; +import * as trustedExecutable from "../src/trusted-executable.js"; import { normalizeTarget } from "../src/targets.js"; import { SYNTHETIC_CREDENTIALS } from "./cli-fixtures.js"; import { INTEGRATION_TARGET, PLUGIN_ROOT } from "./plugin-root.js"; @@ -5455,6 +5458,235 @@ describe("CodexSecurity orchestration", () => { await client.close(); }); + test("binds staged bundled ripgrep only when a trusted host tool is unavailable", async () => { + if ( + runMockInSubprocess( + import.meta.path, + "binds staged bundled ripgrep only when a trusted host tool is unavailable", + ) + ) { + return; + } + const originalTrusted = { ...trustedExecutable }; + const originalRuntime = { ...runtimeModule }; + const originalPlatform = Object.getOwnPropertyDescriptor( + process, + "platform", + )!; + const inspected: [string, string][] = []; + let host: string | null = null; + let rejected: string | null = null; + mock.module("../src/trusted-executable.js", () => ({ + ...originalTrusted, + resolveTrustedExecutable: async () => null, + inspectTrustedExecutable: async ( + candidate: string, + environment: Record, + protectedRoot: string, + ) => { + inspected.push([candidate, protectedRoot]); + return { + executable: + candidate === "git" || candidate === rejected + ? null + : candidate === "rg" + ? host + : candidate, + environment: { ...environment, PATH: "" }, + }; + }, + })); + mock.module("../src/runtime.js", () => ({ + ...originalRuntime, + pluginExecutionEnvironment: ( + python: string, + environment: Record, + ) => ({ + ...environment, + PYTHON: python, + CODEX_CLI_PATH: process.execPath, + }), + })); + const cases: { + scenario: string; + platform?: NodeJS.Platform; + host: boolean; + bindings?: Record; + expected: "host" | "staged" | "disabled" | "missing"; + }[] = [ + { scenario: "host", host: true, expected: "host" }, + { scenario: "bundled", host: false, expected: "staged" }, + { scenario: "missing", host: false, expected: "missing" }, + { + scenario: "disabled", + host: true, + bindings: { CODEX_SECURITY_RG: "" }, + expected: "disabled", + }, + { scenario: "rejected-copy", host: false, expected: "missing" }, + { scenario: "overlapping-workspace", host: false, expected: "missing" }, + { + scenario: "case-distinct POSIX binding", + platform: "linux", + host: true, + bindings: { Codex_Security_Rg: "" }, + expected: "host", + }, + { + scenario: "Windows alias disable", + platform: "win32", + host: true, + bindings: { Codex_Security_Rg: "" }, + expected: "disabled", + }, + { + scenario: "Windows effective binding", + platform: "win32", + host: true, + bindings: { CODEX_SECURITY_RG: "previous", Codex_Security_Rg: "" }, + expected: "host", + }, + ]; + + try { + for (const entry of cases) { + const { scenario } = entry; + Object.defineProperty(process, "platform", { + value: entry.platform ?? originalPlatform.value, + }); + const root = await temporaryDirectory(); + const repository = join(root, "repository"); + const nextRepository = join(root, "next-repository"); + const codexHome = join(root, "codex-home"); + const scanDir = join(root, "scan"); + const workspace = + scenario === "overlapping-workspace" + ? repository + : join(root, "bootstrap-workspace"); + for (const path of new Set([ + repository, + nextRepository, + codexHome, + scanDir, + workspace, + ])) { + await mkdir(path, { mode: 0o700 }); + } + const filename = process.platform === "win32" ? "rg.exe" : "rg"; + const staged = + scenario === "rejected-copy" + ? join(repository, filename) + : join(workspace, filename); + host = entry.host ? join(root, "host-tools", filename) : null; + rejected = scenario === "rejected-copy" ? staged : null; + inspected.length = 0; + const stageCalls: string[] = []; + const workbenchEnvironments: WorkbenchCommandOptions["environment"][] = + []; + const codexEnvironments: CodexOptions["env"][] = []; + const environment = { + PATH: "", + CODEX_CLI_PATH: process.execPath, + CODEX_SECURITY_STATE_DIR: join(root, "state"), + OPENAI_API_KEY: "synthetic-key", + ...entry.bindings, + }; + const runtime = { + ...preparedRuntime(codexHome), + bootstrapWorkspace: workspace, + environment, + }; + const client = new TestClient( + {}, + { + environment, + prepareRuntime: async () => runtime, + resolvePluginPython: async () => "/managed/python", + prepareOutputDir: async () => scanDir, + repositoryRevision: async () => null, + resolveCodexCommand: () => ({ command: process.execPath }), + stageBundledRipgrep: async (path: string) => { + stageCalls.push(path); + return scenario === "missing" ? null : staged; + }, + runWorkbench: async ( + options: WorkbenchCommandOptions, + args: readonly string[], + ) => { + if (args[0] === "register-cli-scan") { + workbenchEnvironments.push(options.environment); + } + return mockWorkbench(args); + }, + createCodex: (options: CodexOptions) => { + codexEnvironments.push(options.env); + throw new Error("captured tool environment"); + }, + }, + ); + try { + if (scenario === "overlapping-workspace") { + await expect(client.run(repository)).rejects.toBeInstanceOf( + OutputInsideProtectedRootError, + ); + expect(stageCalls).toEqual([]); + expect(codexEnvironments).toEqual([]); + continue; + } + await expect(client.run(repository)).rejects.toThrow( + "captured tool environment", + ); + if (scenario === "bundled") { + await expect(client.run(nextRepository)).rejects.toThrow( + "captured tool environment", + ); + expect( + inspected.filter(([candidate]) => candidate === staged), + ).toEqual([ + [staged, repository], + [staged, nextRepository], + ]); + } + const expected = + entry.expected === "disabled" + ? "" + : entry.expected === "host" + ? host ?? undefined + : entry.expected === "staged" + ? staged + : undefined; + for (const selected of [ + ...workbenchEnvironments, + ...codexEnvironments, + ]) { + expect(selected?.["CODEX_SECURITY_RG"]).toBe(expected); + expect(selected?.["PATH"]).toBe(""); + expect(selected?.["Codex_Security_Rg"]).toBe( + process.platform === "win32" + ? undefined + : entry.bindings?.["Codex_Security_Rg"], + ); + } + expect(workbenchEnvironments).toHaveLength( + scenario === "bundled" ? 2 : 1, + ); + expect(codexEnvironments).toHaveLength( + scenario === "bundled" ? 2 : 1, + ); + expect(stageCalls).toEqual( + entry.host || entry.expected === "disabled" ? [] : [workspace], + ); + } finally { + await client.close(); + } + } + } finally { + Object.defineProperty(process, "platform", originalPlatform); + mock.module("../src/trusted-executable.js", () => originalTrusted); + mock.module("../src/runtime.js", () => originalRuntime); + } + }); + test("authenticates without initializing the plugin runtime", async () => { const root = await temporaryDirectory(); const stateDirectory = join(root, "state"); diff --git a/sdk/typescript/tests-ts/runtime.test.ts b/sdk/typescript/tests-ts/runtime.test.ts index 16280e70e..02ae769ff 100644 --- a/sdk/typescript/tests-ts/runtime.test.ts +++ b/sdk/typescript/tests-ts/runtime.test.ts @@ -1,5 +1,6 @@ import { execFile, spawnSync } from "node:child_process"; import { existsSync, renameSync, symlinkSync } from "node:fs"; +import * as fsSync from "node:fs"; import { chmod, copyFile, @@ -17,6 +18,7 @@ import { writeFile, } from "node:fs/promises"; import * as fsPromises from "node:fs/promises"; +import * as nodeModule from "node:module"; import { tmpdir } from "node:os"; import { delimiter, @@ -70,6 +72,7 @@ import { requireTrustedOutputAncestor, runWorkbench, setCodexSecurityCredentialLogout, + stageBundledRipgrep, streamWindowsCredentialAclDescriptors, verifyStableWindowsCredentialDescendants, } from "../src/runtime.js"; @@ -1794,6 +1797,185 @@ describe("plugin runtime preparation", () => { ); }); + test("stages only package-owned native ripgrep and cleans failed copies", async () => { + if ( + runMockInSubprocess( + import.meta.path, + "stages only package-owned native ripgrep and cleans failed copies", + ) + ) { + return; + } + const originalFs = { ...fsSync }; + const originalPromises = { ...fsPromises }; + const originalModule = { ...nodeModule }; + const originalPlatform = Object.getOwnPropertyDescriptor( + process, + "platform", + )!; + const root = join(tmpdir(), "codex-security-package-mock"); + const codexPackageJson = join(root, "codex", "package.json"); + const packageRoot = join(root, "native-package"); + const nativePackageJson = join(packageRoot, "package.json"); + const vendor = join(packageRoot, "vendor"); + const bundle = join(vendor, "native-target"); + const workspace = join(root, "private-workspace"); + const marker = join(bundle, "codex-package.json"); + const filename = () => (process.platform === "win32" ? "rg.exe" : "rg"); + const source = () => join(bundle, "codex-path", filename()); + const destination = () => join(workspace, filename()); + const failure = (code: string) => Object.assign(new Error(code), { code }); + let scenario = "available"; + let cancelCopy: AbortController | undefined; + const resolutions: [string, string][] = []; + const copies: unknown[][] = []; + const modes: unknown[][] = []; + const removed: string[] = []; + const reset = (next: string) => { + scenario = next; + cancelCopy = undefined; + resolutions.length = 0; + copies.length = 0; + modes.length = 0; + removed.length = 0; + }; + mock.module("node:module", () => ({ + ...originalModule, + createRequire: (from: string | URL) => ({ + resolve: (specifier: string) => { + resolutions.push([String(from), specifier]); + if (scenario === "missing-package") throw failure("MODULE_NOT_FOUND"); + if (specifier === "@openai/codex/package.json") { + return codexPackageJson; + } + expect(String(from)).toBe(codexPackageJson); + expect(specifier).toBe( + `@openai/codex-${process.platform}-${process.arch}/package.json`, + ); + return nativePackageJson; + }, + }), + })); + mock.module("node:fs", () => ({ + ...originalFs, + readdirSync: (path: string) => { + expect(path).toBe(vendor); + return [{ name: "native-target", isDirectory: () => true }]; + }, + existsSync: () => scenario !== "missing-codex", + })); + mock.module("node:fs/promises", () => ({ + ...originalPromises, + realpath: async (path: string) => + scenario === "outside-package" && path === source() + ? join(root, "other", filename()) + : path, + lstat: async (path: string) => { + if ( + (scenario === "missing-rg" && path === source()) || + (scenario === "missing-marker" && path === marker) + ) { + throw failure("ENOENT"); + } + expect([source(), marker]).toContain(path); + return { + isFile: () => scenario !== "not-file" || path !== source(), + isSymbolicLink: () => scenario === "symlink" && path === source(), + }; + }, + access: async (path: string, mode: number) => { + expect(path).toBe(source()); + expect(mode).toBe( + process.platform === "win32" + ? originalFs.constants.F_OK + : originalFs.constants.X_OK, + ); + if (scenario === "not-executable") throw failure("EACCES"); + if (scenario === "io-error") throw failure("EIO"); + }, + copyFile: async (...args: unknown[]) => { + copies.push(args); + if (scenario === "existing-destination") throw failure("EEXIST"); + cancelCopy?.abort(new DOMException("canceled", "AbortError")); + }, + chmod: async (...args: unknown[]) => { + modes.push(args); + if (scenario === "chmod-error") throw failure("EACCES"); + }, + rm: async (path: string) => { + removed.push(path); + }, + })); + + try { + for (const platform of ["linux", "win32"] as const) { + Object.defineProperty(process, "platform", { value: platform }); + reset("available"); + expect(await stageBundledRipgrep(workspace)).toBe(destination()); + expect(resolutions).toEqual([ + [ + new URL("../src/runtime.ts", import.meta.url).href, + "@openai/codex/package.json", + ], + [ + codexPackageJson, + `@openai/codex-${platform}-${process.arch}/package.json`, + ], + ]); + expect(copies).toEqual([ + [source(), destination(), originalFs.constants.COPYFILE_EXCL], + ]); + expect(modes).toEqual( + platform === "win32" ? [] : [[destination(), 0o700]], + ); + expect(removed).toEqual([]); + } + + Object.defineProperty(process, "platform", { value: "linux" }); + for (const unavailable of [ + "missing-package", + "missing-codex", + "missing-marker", + "missing-rg", + "not-file", + "symlink", + "outside-package", + "not-executable", + ]) { + reset(unavailable); + expect(await stageBundledRipgrep(workspace)).toBeNull(); + expect(copies).toEqual([]); + } + reset("io-error"); + await expect(stageBundledRipgrep(workspace)).rejects.toMatchObject({ + code: "EIO", + }); + expect(copies).toEqual([]); + + reset("existing-destination"); + await expect(stageBundledRipgrep(workspace)).rejects.toMatchObject({ + code: "EEXIST", + }); + expect(removed).toEqual([]); + reset("chmod-error"); + await expect(stageBundledRipgrep(workspace)).rejects.toMatchObject({ + code: "EACCES", + }); + expect(removed).toEqual([destination()]); + reset("available"); + cancelCopy = new AbortController(); + await expect( + stageBundledRipgrep(workspace, cancelCopy.signal), + ).rejects.toMatchObject({ name: "AbortError" }); + expect(removed).toEqual([destination()]); + } finally { + Object.defineProperty(process, "platform", originalPlatform); + mock.module("node:module", () => originalModule); + mock.module("node:fs", () => originalFs); + mock.module("node:fs/promises", () => originalPromises); + } + }); + test("uses an explicit Codex executable override", () => { const executable = process.platform === "win32" ? "codex.exe" : "codex"; const configured = join(tmpdir(), "custom codex", executable); From c0142c5739652cff10467b6b5285ca5193c013a9 Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Sun, 16 Aug 2026 04:36:24 -0700 Subject: [PATCH 09/27] fix(api): preserve explicit Git disable bindings --- sdk/typescript/src/api.ts | 20 +++-- sdk/typescript/tests-ts/api.test.ts | 110 +++++++++++++++++++++++++--- 2 files changed, 114 insertions(+), 16 deletions(-) diff --git a/sdk/typescript/src/api.ts b/sdk/typescript/src/api.ts index 462f82ab1..c01b79ec2 100644 --- a/sdk/typescript/src/api.ts +++ b/sdk/typescript/src/api.ts @@ -658,12 +658,18 @@ export class CodexSecurity { git.environment, protectedGitRoot, ); - const ripgrepKeys = + const toolBindingKeys = ( + setting: "CODEX_SECURITY_GIT" | "CODEX_SECURITY_RG", + ): string[] => process.platform === "win32" ? Object.keys(pluginEnvironment) - .filter((name) => name.toUpperCase() === "CODEX_SECURITY_RG") + .filter((name) => name.toUpperCase() === setting) .sort() - : ["CODEX_SECURITY_RG"]; + : [setting]; + const gitKeys = toolBindingKeys("CODEX_SECURITY_GIT"); + const ripgrepKeys = toolBindingKeys("CODEX_SECURITY_RG"); + const gitDisabled = + pluginEnvironment[gitKeys[0] ?? "CODEX_SECURITY_GIT"] === ""; const ripgrepDisabled = pluginEnvironment[ripgrepKeys[0] ?? "CODEX_SECURITY_RG"] === ""; if ( @@ -690,8 +696,12 @@ export class CodexSecurity { const trustedPluginEnvironment: ProcessEnvironment = { ...ripgrep.environment, }; - for (const name of ripgrepKeys) delete trustedPluginEnvironment[name]; - trustedPluginEnvironment["CODEX_SECURITY_GIT"] = git.executable ?? ""; + for (const name of [...gitKeys, ...ripgrepKeys]) { + delete trustedPluginEnvironment[name]; + } + trustedPluginEnvironment["CODEX_SECURITY_GIT"] = gitDisabled + ? "" + : git.executable ?? ""; // The Codex runtime can add its bundled tools to PATH after this point. trustedPluginEnvironment["CODEX_SECURITY_RG"] = ripgrepDisabled ? "" diff --git a/sdk/typescript/tests-ts/api.test.ts b/sdk/typescript/tests-ts/api.test.ts index 870e137a7..27201b372 100644 --- a/sdk/typescript/tests-ts/api.test.ts +++ b/sdk/typescript/tests-ts/api.test.ts @@ -4556,11 +4556,6 @@ describe("CodexSecurity orchestration", () => { startThread: () => ({ id: null, async runStreamed() { - expect( - existsSync( - join(credentialHome, ".codex-security-scan.lock"), - ), - ).toBe(false); activeScans += 1; maximumActiveScans = Math.max( maximumActiveScans, @@ -4585,6 +4580,11 @@ describe("CodexSecurity orchestration", () => { concurrentScans, new Promise((resolve) => setTimeout(resolve, 5_000)), ]); + expect( + existsSync( + join(credentialHome, ".codex-security-scan.lock"), + ), + ).toBe(false); const after = parseToml( await readFile(deepScanConfigPath!, "utf8"), ); @@ -5474,8 +5474,10 @@ describe("CodexSecurity orchestration", () => { "platform", )!; const inspected: [string, string][] = []; + let gitHost: string | null = null; let host: string | null = null; let rejected: string | null = null; + let sanitizedGitEnvironment: Record | undefined; mock.module("../src/trusted-executable.js", () => ({ ...originalTrusted, resolveTrustedExecutable: async () => null, @@ -5485,14 +5487,22 @@ describe("CodexSecurity orchestration", () => { protectedRoot: string, ) => { inspected.push([candidate, protectedRoot]); + const sanitizedEnvironment = { ...environment, PATH: "" }; + if (candidate === "git") { + sanitizedGitEnvironment = sanitizedEnvironment; + } else if (candidate === "rg") { + expect(sanitizedGitEnvironment).toBe(environment); + } return { executable: - candidate === "git" || candidate === rejected - ? null - : candidate === "rg" - ? host - : candidate, - environment: { ...environment, PATH: "" }, + candidate === "git" + ? gitHost + : candidate === rejected + ? null + : candidate === "rg" + ? host + : candidate, + environment: sanitizedEnvironment, }; }, })); @@ -5511,8 +5521,10 @@ describe("CodexSecurity orchestration", () => { scenario: string; platform?: NodeJS.Platform; host: boolean; + gitAvailable?: boolean; bindings?: Record; expected: "host" | "staged" | "disabled" | "missing"; + expectedGit?: "host" | "disabled"; }[] = [ { scenario: "host", host: true, expected: "host" }, { scenario: "bundled", host: false, expected: "staged" }, @@ -5546,6 +5558,66 @@ describe("CodexSecurity orchestration", () => { bindings: { CODEX_SECURITY_RG: "previous", Codex_Security_Rg: "" }, expected: "host", }, + { + scenario: "Git binding absent", + host: true, + gitAvailable: true, + expected: "host", + expectedGit: "host", + }, + { + scenario: "Git binding nonempty", + host: true, + gitAvailable: true, + bindings: { CODEX_SECURITY_GIT: "previous" }, + expected: "host", + expectedGit: "host", + }, + { + scenario: "Git binding disabled", + platform: "linux", + host: true, + gitAvailable: true, + bindings: { CODEX_SECURITY_GIT: "" }, + expected: "host", + expectedGit: "disabled", + }, + { + scenario: "case-distinct POSIX Git binding", + platform: "linux", + host: true, + gitAvailable: true, + bindings: { Codex_Security_Git: "" }, + expected: "host", + expectedGit: "host", + }, + { + scenario: "Windows Git alias disable", + platform: "win32", + host: true, + gitAvailable: true, + bindings: { Codex_Security_Git: "" }, + expected: "host", + expectedGit: "disabled", + }, + { + scenario: "Windows effective Git binding", + platform: "win32", + host: true, + gitAvailable: true, + bindings: { CODEX_SECURITY_GIT: "previous", Codex_Security_Git: "" }, + expected: "host", + expectedGit: "host", + }, + { + scenario: "Windows effective Git disable", + platform: "win32", + host: true, + gitAvailable: true, + bindings: { CODEX_SECURITY_GIT: "", Codex_Security_Git: "previous" }, + expected: "host", + expectedGit: "disabled", + }, ]; try { @@ -5573,6 +5645,13 @@ describe("CodexSecurity orchestration", () => { await mkdir(path, { mode: 0o700 }); } const filename = process.platform === "win32" ? "rg.exe" : "rg"; + gitHost = entry.gitAvailable + ? join( + root, + "host-tools", + process.platform === "win32" ? "git.exe" : "git", + ) + : null; const staged = scenario === "rejected-copy" ? join(repository, filename) @@ -5580,6 +5659,7 @@ describe("CodexSecurity orchestration", () => { host = entry.host ? join(root, "host-tools", filename) : null; rejected = scenario === "rejected-copy" ? staged : null; inspected.length = 0; + sanitizedGitEnvironment = undefined; const stageCalls: string[] = []; const workbenchEnvironments: WorkbenchCommandOptions["environment"][] = []; @@ -5659,8 +5739,16 @@ describe("CodexSecurity orchestration", () => { ...workbenchEnvironments, ...codexEnvironments, ]) { + expect(selected?.["CODEX_SECURITY_GIT"]).toBe( + entry.expectedGit === "host" ? gitHost ?? undefined : "", + ); expect(selected?.["CODEX_SECURITY_RG"]).toBe(expected); expect(selected?.["PATH"]).toBe(""); + expect(selected?.["Codex_Security_Git"]).toBe( + process.platform === "win32" + ? undefined + : entry.bindings?.["Codex_Security_Git"], + ); expect(selected?.["Codex_Security_Rg"]).toBe( process.platform === "win32" ? undefined From 62bfcd13d43785b578086d1ffe2cfdb90bc11c3a Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Sun, 16 Aug 2026 05:48:54 -0700 Subject: [PATCH 10/27] fix(runtime): reject canonical Windows batch targets --- sdk/typescript/src/trusted-executable.ts | 3 + .../tests-ts/trusted-executable.test.ts | 152 +++++++++++++++++- 2 files changed, 153 insertions(+), 2 deletions(-) diff --git a/sdk/typescript/src/trusted-executable.ts b/sdk/typescript/src/trusted-executable.ts index 82942e151..fece1394e 100644 --- a/sdk/typescript/src/trusted-executable.ts +++ b/sdk/typescript/src/trusted-executable.ts @@ -85,6 +85,9 @@ export async function inspectTrustedExecutable( if (current.entry !== null) unsafeEntries.add(current.entry); continue; } + if (process.platform === "win32" && /\.(?:bat|cmd)$/iu.test(canonical)) { + continue; + } if (!current.runnable) continue; try { await access( diff --git a/sdk/typescript/tests-ts/trusted-executable.test.ts b/sdk/typescript/tests-ts/trusted-executable.test.ts index f1e06baac..7e4a4406e 100644 --- a/sdk/typescript/tests-ts/trusted-executable.test.ts +++ b/sdk/typescript/tests-ts/trusted-executable.test.ts @@ -1,4 +1,5 @@ import { spawnSync } from "node:child_process"; +import { constants } from "node:fs"; import { chmod, mkdir, @@ -8,11 +9,16 @@ import { symlink, writeFile, } from "node:fs/promises"; +import * as fsPromises from "node:fs/promises"; import { tmpdir } from "node:os"; import { basename, delimiter, dirname, join, relative } from "node:path"; import { fileURLToPath } from "node:url"; -import { afterEach, describe, expect, test } from "bun:test"; -import { resolveTrustedExecutable } from "../src/trusted-executable.js"; +import { afterEach, describe, expect, mock, test } from "bun:test"; +import { + inspectTrustedExecutable, + resolveTrustedExecutable, +} from "../src/trusted-executable.js"; +import { runMockInSubprocess } from "./support/isolated-mock.js"; const temporaryDirectories: string[] = []; @@ -146,6 +152,148 @@ describe("trusted executable resolution", () => { }, ); + test("rejects canonical Windows batch targets without rejecting native aliases", async () => { + if ( + runMockInSubprocess( + import.meta.path, + "rejects canonical Windows batch targets without rejecting native aliases", + ) + ) { + return; + } + const originalPromises = { ...fsPromises }; + const originalPlatform = Object.getOwnPropertyDescriptor( + process, + "platform", + )!; + const root = join(tmpdir(), "trusted-executable-metadata-mock"); + const repository = join(root, "repository"); + const first = join(root, "first"); + const second = join(root, "second"); + const firstExe = join(first, "rg.exe"); + const firstCom = join(first, "rg.com"); + const secondExe = join(second, "rg.exe"); + const native = join(root, "native-target"); + const command = join(root, "target.CmD"); + const batch = join(root, "target.BaT"); + let paths = new Map(); + let files = new Set(); + const accesses: [string, number][] = []; + const missing = () => + Object.assign(new Error("missing mock path"), { code: "ENOENT" }); + mock.module("node:fs/promises", () => ({ + ...originalPromises, + realpath: async (path: string) => { + const canonical = paths.get(path); + if (canonical === undefined) throw missing(); + return canonical; + }, + access: async (path: string, mode: number) => { + accesses.push([path, mode]); + if (!files.has(path)) throw missing(); + }, + stat: async (path: string) => ({ isFile: () => files.has(path) }), + })); + const cases: { + platform: NodeJS.Platform; + entries: string[]; + targets: [string, string][]; + executable: string | null; + keptEntries?: string[]; + }[] = [ + { + platform: "win32", + entries: [first], + targets: [[firstExe, command]], + executable: null, + }, + { + platform: "win32", + entries: [first], + targets: [[firstExe, batch]], + executable: null, + }, + { + platform: "win32", + entries: [first, second], + targets: [ + [firstExe, command], + [secondExe, native], + ], + executable: secondExe, + }, + { + platform: "win32", + entries: [first], + targets: [[firstExe, native]], + executable: firstExe, + }, + { + platform: "win32", + entries: [first], + targets: [[firstCom, native]], + executable: firstCom, + }, + { + platform: "win32", + entries: [first, second], + targets: [ + [firstExe, join(repository, "target.cmd")], + [secondExe, native], + ], + executable: secondExe, + keptEntries: [second], + }, + { + platform: "linux", + entries: [first], + targets: [[join(first, "rg"), command]], + executable: join(first, "rg"), + }, + ]; + + try { + for (const entry of cases) { + Object.defineProperty(process, "platform", { value: entry.platform }); + paths = new Map([ + [repository, repository], + ...entry.entries.map((path): [string, string] => [path, path]), + ...entry.targets, + ]); + files = new Set(entry.targets.map(([, canonical]) => canonical)); + accesses.length = 0; + expect( + await inspectTrustedExecutable( + "rg", + { PATH: entry.entries.join(delimiter), KEEP: "ok" }, + repository, + ), + ).toEqual({ + executable: entry.executable, + environment: { + KEEP: "ok", + PATH: (entry.keptEntries ?? entry.entries).join(delimiter), + }, + }); + expect( + accesses.every( + ([, mode]) => + mode === + (entry.platform === "win32" ? constants.F_OK : constants.X_OK), + ), + ).toBe(true); + if (entry.platform === "win32") { + expect(accesses.some(([path]) => /\.(?:bat|cmd)$/iu.test(path))).toBe( + false, + ); + } + } + } finally { + Object.defineProperty(process, "platform", originalPlatform); + mock.module("node:fs/promises", () => originalPromises); + } + }); + test("selects runnable Windows executables ahead of extensionless and batch files", async () => { const root = await temporaryDirectory(); const repository = join(root, "repository"); From b83d18e5748c92846c391cb062941e742e2e27dd Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Sun, 16 Aug 2026 06:50:22 -0700 Subject: [PATCH 11/27] test: preserve Python shim startup environment --- sdk/typescript/tests-ts/workbench-tool-environment.test.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/sdk/typescript/tests-ts/workbench-tool-environment.test.ts b/sdk/typescript/tests-ts/workbench-tool-environment.test.ts index bf3dfc34d..27deb564a 100644 --- a/sdk/typescript/tests-ts/workbench-tool-environment.test.ts +++ b/sdk/typescript/tests-ts/workbench-tool-environment.test.ts @@ -57,16 +57,19 @@ function runPythonMocks(source: string): void { "-B", "-c", ` -import argparse, io, os, subprocess, sys +import argparse, io, json, os, subprocess, sys from pathlib import Path from unittest.mock import patch +os.environ.clear() +os.environ.update(json.loads(sys.argv[2])) sys.path.insert(0, sys.argv[1]) import workbench_target as workbench ${source} `, join(PLUGIN_ROOT, "scripts"), + JSON.stringify(childEnvironment(dirname(python!))), ], - { encoding: "utf8", env: childEnvironment(dirname(python!)) }, + { encoding: "utf8" }, ); expect(result.status, result.stderr).toBe(0); } From ab4e036c3fc3e8bc8b6a91d5a9bbcccd3d867ae1 Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Mon, 17 Aug 2026 02:18:16 -0700 Subject: [PATCH 12/27] fix(sdk): preserve explicit tool selections --- sdk/typescript/src/api.ts | 63 +++++++++++++++++++---------- sdk/typescript/tests-ts/api.test.ts | 15 +++++-- 2 files changed, 54 insertions(+), 24 deletions(-) diff --git a/sdk/typescript/src/api.ts b/sdk/typescript/src/api.ts index c01b79ec2..1d69e2095 100644 --- a/sdk/typescript/src/api.ts +++ b/sdk/typescript/src/api.ts @@ -46,6 +46,7 @@ import { import { AuthenticationRequiredError, CodexSecurityError, + ConfigurationError, IncompleteScanError, OutputDirectoryError, OutputInsideProtectedRootError, @@ -647,6 +648,20 @@ export class CodexSecurity { options.auth, modelProvider, ); + const toolBindingKeys = ( + setting: "CODEX_SECURITY_GIT" | "CODEX_SECURITY_RG", + ): string[] => + process.platform === "win32" + ? Object.keys(pluginEnvironment) + .filter((name) => name.toUpperCase() === setting) + .sort() + : [setting]; + const gitKeys = toolBindingKeys("CODEX_SECURITY_GIT"); + const ripgrepKeys = toolBindingKeys("CODEX_SECURITY_RG"); + const configuredGit = + pluginEnvironment[gitKeys[0] ?? "CODEX_SECURITY_GIT"]; + const configuredRipgrep = + pluginEnvironment[ripgrepKeys[0] ?? "CODEX_SECURITY_RG"]; const protectedGitRoot = await outermostGitMarkerRoot(repo, signal); const git = await inspectTrustedExecutable( "git", @@ -658,22 +673,30 @@ export class CodexSecurity { git.environment, protectedGitRoot, ); - const toolBindingKeys = ( - setting: "CODEX_SECURITY_GIT" | "CODEX_SECURITY_RG", - ): string[] => - process.platform === "win32" - ? Object.keys(pluginEnvironment) - .filter((name) => name.toUpperCase() === setting) - .sort() - : [setting]; - const gitKeys = toolBindingKeys("CODEX_SECURITY_GIT"); - const ripgrepKeys = toolBindingKeys("CODEX_SECURITY_RG"); - const gitDisabled = - pluginEnvironment[gitKeys[0] ?? "CODEX_SECURITY_GIT"] === ""; - const ripgrepDisabled = - pluginEnvironment[ripgrepKeys[0] ?? "CODEX_SECURITY_RG"] === ""; + for (const [setting, configured] of [ + ["CODEX_SECURITY_GIT", configuredGit], + ["CODEX_SECURITY_RG", configuredRipgrep], + ] as const) { + if (configured === undefined || configured === "") continue; + if (!isAbsolute(configured)) { + throw new ConfigurationError( + `${setting} must name an absolute trusted executable.`, + ); + } + const inspected = await inspectTrustedExecutable( + configured, + ripgrep.environment, + protectedGitRoot, + ); + if (inspected.executable === null) { + throw new ConfigurationError( + `${setting} does not name an available executable.`, + ); + } + ripgrep.environment = inspected.environment; + } if ( - !ripgrepDisabled && + configuredRipgrep === undefined && ripgrep.executable === null && runtime.bootstrapWorkspace !== undefined ) { @@ -699,13 +722,11 @@ export class CodexSecurity { for (const name of [...gitKeys, ...ripgrepKeys]) { delete trustedPluginEnvironment[name]; } - trustedPluginEnvironment["CODEX_SECURITY_GIT"] = gitDisabled - ? "" - : git.executable ?? ""; + trustedPluginEnvironment["CODEX_SECURITY_GIT"] = + configuredGit ?? git.executable ?? ""; // The Codex runtime can add its bundled tools to PATH after this point. - trustedPluginEnvironment["CODEX_SECURITY_RG"] = ripgrepDisabled - ? "" - : ripgrep.executable ?? undefined; + trustedPluginEnvironment["CODEX_SECURITY_RG"] = + configuredRipgrep ?? ripgrep.executable ?? undefined; checkOpen(); const scanOutputRoot = requestedOutput === null && diff --git a/sdk/typescript/tests-ts/api.test.ts b/sdk/typescript/tests-ts/api.test.ts index 27201b372..1198f792d 100644 --- a/sdk/typescript/tests-ts/api.test.ts +++ b/sdk/typescript/tests-ts/api.test.ts @@ -5522,6 +5522,7 @@ describe("CodexSecurity orchestration", () => { platform?: NodeJS.Platform; host: boolean; gitAvailable?: boolean; + configuredTool?: "git" | "rg"; bindings?: Record; expected: "host" | "staged" | "disabled" | "missing"; expectedGit?: "host" | "disabled"; @@ -5555,7 +5556,8 @@ describe("CodexSecurity orchestration", () => { scenario: "Windows effective binding", platform: "win32", host: true, - bindings: { CODEX_SECURITY_RG: "previous", Codex_Security_Rg: "" }, + configuredTool: "rg", + bindings: { Codex_Security_Rg: "" }, expected: "host", }, { @@ -5569,7 +5571,7 @@ describe("CodexSecurity orchestration", () => { scenario: "Git binding nonempty", host: true, gitAvailable: true, - bindings: { CODEX_SECURITY_GIT: "previous" }, + configuredTool: "git", expected: "host", expectedGit: "host", }, @@ -5605,7 +5607,8 @@ describe("CodexSecurity orchestration", () => { platform: "win32", host: true, gitAvailable: true, - bindings: { CODEX_SECURITY_GIT: "previous", Codex_Security_Git: "" }, + configuredTool: "git", + bindings: { Codex_Security_Git: "" }, expected: "host", expectedGit: "host", }, @@ -5670,6 +5673,12 @@ describe("CodexSecurity orchestration", () => { CODEX_SECURITY_STATE_DIR: join(root, "state"), OPENAI_API_KEY: "synthetic-key", ...entry.bindings, + ...(entry.configuredTool === "git" + ? { CODEX_SECURITY_GIT: gitHost! } + : {}), + ...(entry.configuredTool === "rg" + ? { CODEX_SECURITY_RG: host! } + : {}), }; const runtime = { ...preparedRuntime(codexHome), From b2fa38c9406f30f16e2056373a2e20ee70fd55a1 Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Mon, 17 Aug 2026 16:07:45 -0700 Subject: [PATCH 13/27] fix(sdk): carry explicit Git bindings through target validation --- sdk/typescript/src/api.ts | 78 ++++--- sdk/typescript/src/cli.ts | 6 +- sdk/typescript/src/targets.ts | 116 ++++++++-- sdk/typescript/src/trusted-executable.ts | 23 +- sdk/typescript/tests-ts/api.test.ts | 20 ++ sdk/typescript/tests-ts/cli-export.test.ts | 5 + .../tests-ts/targets-git-binding.test.ts | 202 ++++++++++++++++++ 7 files changed, 392 insertions(+), 58 deletions(-) create mode 100644 sdk/typescript/tests-ts/targets-git-binding.test.ts diff --git a/sdk/typescript/src/api.ts b/sdk/typescript/src/api.ts index e7af87446..d44161b2f 100644 --- a/sdk/typescript/src/api.ts +++ b/sdk/typescript/src/api.ts @@ -124,6 +124,7 @@ import { normalizeTarget, outermostGitMarkerRoot, repositoryRevision, + resolveGitCommand, resolveRepositoryPath, type NormalizedTarget, type ScanMode, @@ -132,7 +133,11 @@ import { validateCommittedDiffCheckout, validateMode, } from "./targets.js"; -import { inspectTrustedExecutable } from "./trusted-executable.js"; +import { + executableBinding, + inspectTrustedExecutable, + type InspectedExecutable, +} from "./trusted-executable.js"; interface CodexThreadLike { readonly id: string | null; @@ -298,6 +303,8 @@ export interface ScanPreflight extends DeepScanOptions { interface LocalScanInputs extends Omit { protectedRoot: string; + protectedGitRoot: string; + gitCommand: InspectedExecutable; stateDirectory: string; } @@ -495,6 +502,8 @@ export class CodexSecurity { mode, outputDir: requestedOutput, protectedRoot, + protectedGitRoot, + gitCommand, stateDirectory, } = await this.#validateLocalInputs(repository, options, signal); checkOpen(); @@ -555,21 +564,15 @@ export class CodexSecurity { options.auth, modelProvider, ); - const toolBindingKeys = ( - setting: "CODEX_SECURITY_GIT" | "CODEX_SECURITY_RG", - ): string[] => - process.platform === "win32" - ? Object.keys(pluginEnvironment) - .filter((name) => name.toUpperCase() === setting) - .sort() - : [setting]; - const gitKeys = toolBindingKeys("CODEX_SECURITY_GIT"); - const ripgrepKeys = toolBindingKeys("CODEX_SECURITY_RG"); - const configuredGit = - pluginEnvironment[gitKeys[0] ?? "CODEX_SECURITY_GIT"]; - const configuredRipgrep = - pluginEnvironment[ripgrepKeys[0] ?? "CODEX_SECURITY_RG"]; - const protectedGitRoot = await outermostGitMarkerRoot(repo, signal); + const { keys: gitKeys } = executableBinding( + pluginEnvironment, + "CODEX_SECURITY_GIT", + ); + const { keys: ripgrepKeys, value: configuredRipgrep } = executableBinding( + pluginEnvironment, + "CODEX_SECURITY_RG", + ); + // Sanitize the runtime PATH without changing the Git selected at preflight. const git = await inspectTrustedExecutable( "git", pluginEnvironment, @@ -580,24 +583,20 @@ export class CodexSecurity { git.environment, protectedGitRoot, ); - for (const [setting, configured] of [ - ["CODEX_SECURITY_GIT", configuredGit], - ["CODEX_SECURITY_RG", configuredRipgrep], - ] as const) { - if (configured === undefined || configured === "") continue; - if (!isAbsolute(configured)) { + if (configuredRipgrep !== undefined && configuredRipgrep !== "") { + if (!isAbsolute(configuredRipgrep)) { throw new ConfigurationError( - `${setting} must name an absolute trusted executable.`, + "CODEX_SECURITY_RG must name an absolute trusted executable.", ); } const inspected = await inspectTrustedExecutable( - configured, + configuredRipgrep, ripgrep.environment, protectedGitRoot, ); if (inspected.executable === null) { throw new ConfigurationError( - `${setting} does not name an available executable.`, + "CODEX_SECURITY_RG does not name an available executable.", ); } ripgrep.environment = inspected.environment; @@ -630,7 +629,7 @@ export class CodexSecurity { delete trustedPluginEnvironment[name]; } trustedPluginEnvironment["CODEX_SECURITY_GIT"] = - configuredGit ?? git.executable ?? ""; + gitCommand.executable ?? ""; // The Codex runtime can add its bundled tools to PATH after this point. trustedPluginEnvironment["CODEX_SECURITY_RG"] = configuredRipgrep ?? ripgrep.executable ?? undefined; @@ -706,7 +705,7 @@ export class CodexSecurity { repository: repo, repositoryRevision: await ( this.#dependencies.repositoryRevision ?? repositoryRevision - )(repo, signal), + )(repo, signal, gitCommand), target: normalized, mode, pluginVersion: runtime.plugin.version, @@ -1980,18 +1979,27 @@ export class CodexSecurity { throwIfAborted(signal); const requestedTarget = options.target ?? "repository"; validatedGitEnvironment(this.#dependencies.environment); - const normalized = await normalizeTarget(repo, requestedTarget, signal); + const protectedGitRoot = await outermostGitMarkerRoot(repo, signal); + const gitCommand = await resolveGitCommand( + this.#dependencies.environment, + protectedGitRoot, + ); + const normalized = await normalizeTarget( + repo, + requestedTarget, + signal, + gitCommand, + ); throwIfAborted(signal); const mode = options.mode ?? "standard"; validateMode(normalized, mode); - await validateCommittedDiffCheckout(repo, normalized, signal); + await validateCommittedDiffCheckout(repo, normalized, signal, gitCommand); throwIfAborted(signal); - const enclosingRoot = await enclosingGitWorktreeRoot(repo, signal); - const repositoryRelative = - enclosingRoot === null ? null : relative(enclosingRoot, repo); + const enclosingRoot = + (await enclosingGitWorktreeRoot(repo, signal, gitCommand)) ?? + protectedGitRoot; + const repositoryRelative = relative(enclosingRoot, repo); const protectedRoot = - enclosingRoot !== null && - repositoryRelative !== null && repositoryRelative !== ".." && !repositoryRelative.startsWith(`..${sep}`) && !isAbsolute(repositoryRelative) @@ -2029,6 +2037,8 @@ export class CodexSecurity { mode, outputDir: requestedOutput, protectedRoot, + protectedGitRoot, + gitCommand, stateDirectory, }; } diff --git a/sdk/typescript/src/cli.ts b/sdk/typescript/src/cli.ts index 495cecd45..c4a2c4190 100644 --- a/sdk/typescript/src/cli.ts +++ b/sdk/typescript/src/cli.ts @@ -126,6 +126,7 @@ import type { ScanWorkerStatus, } from "./worker-progress.js"; import { DiffTarget, type ScanMode, type ScanTarget } from "./targets.js"; +import { executableBinding } from "./trusted-executable.js"; import { BUNDLED_PLUGIN_VERSION, checkForUpdate, @@ -1062,7 +1063,7 @@ async function writeCliOutput( export function exportEnvironment( environment: NodeJS.ProcessEnv = process.env, ): NodeJS.ProcessEnv { - return Object.fromEntries( + const result = Object.fromEntries( [ "PATH", "Path", @@ -1081,6 +1082,9 @@ export function exportEnvironment( .filter((key) => environment[key] !== undefined) .map((key) => [key, environment[key]]), ); + const git = executableBinding(environment, "CODEX_SECURITY_GIT").value; + if (git !== undefined) result["CODEX_SECURITY_GIT"] = git; + return result; } export async function main( diff --git a/sdk/typescript/src/targets.ts b/sdk/typescript/src/targets.ts index 025052e11..12905cadf 100644 --- a/sdk/typescript/src/targets.ts +++ b/sdk/typescript/src/targets.ts @@ -4,8 +4,12 @@ import { lstat, realpath, stat } from "node:fs/promises"; import { homedir } from "node:os"; import { dirname, isAbsolute, join, relative, resolve, sep } from "node:path"; import { promisify } from "node:util"; -import { InvalidTargetError } from "./errors.js"; -import { resolveTrustedExecutable } from "./trusted-executable.js"; +import { ConfigurationError, InvalidTargetError } from "./errors.js"; +import { + executableBinding, + inspectTrustedExecutable, + type InspectedExecutable, +} from "./trusted-executable.js"; const execFile = promisify(execFileCallback); const UNSUPPORTED_GIT_ENVIRONMENT = new Set([ @@ -136,16 +140,25 @@ export function resolveRepositoryPath(repository: string): string { export async function enclosingGitWorktreeRoot( repository: string, signal?: AbortSignal, + gitCommand?: InspectedExecutable, ): Promise { try { + const command = + gitCommand ?? + (await resolveGitCommand( + process.env, + await outermostGitMarkerRoot(repository, signal), + )); const root = await gitOutput( repository, ["rev-parse", "--show-toplevel"], + command, signal, ); return await abortable(() => realpath(root), signal); - } catch { + } catch (error) { throwIfAborted(signal); + if (error instanceof ConfigurationError) throw error; return null; } } @@ -170,6 +183,7 @@ export async function normalizeTarget( repository: string, target: ScanTarget, signal?: AbortSignal, + gitCommand?: InspectedExecutable, ): Promise { const root = await normalizeRepository(repository, signal); throwIfAborted(signal); @@ -199,8 +213,14 @@ export async function normalizeTarget( "Working-tree targets cannot specify a head ref.", ); } - await requireGitRepository(root, signal); - const base = await resolveGitRef(root, target.base, signal); + const command = + gitCommand ?? + (await resolveGitCommand( + process.env, + await outermostGitMarkerRoot(root, signal), + )); + await requireGitRepository(root, command, signal); + const base = await resolveGitRef(root, target.base, command, signal); if (target.kind === "refs") { const head = target.head; if (typeof head !== "string" || head.length === 0) { @@ -212,7 +232,7 @@ export async function normalizeTarget( kind: "refs", paths: [], base, - head: await resolveGitRef(root, head, signal), + head: await resolveGitRef(root, head, command, signal), baseRef: target.base, headRef: head, }; @@ -221,7 +241,7 @@ export async function normalizeTarget( kind: "working_tree", paths: [], base, - head: await resolveGitRef(root, "HEAD", signal), + head: await resolveGitRef(root, "HEAD", command, signal), baseRef: target.base, headRef: "HEAD", }; @@ -288,10 +308,17 @@ export async function validateCommittedDiffCheckout( repository: string, target: NormalizedTarget, signal?: AbortSignal, + gitCommand?: InspectedExecutable, ): Promise { if (target.kind !== "refs") return; - const checkoutHead = await resolveGitRef(repository, "HEAD", signal); + const command = + gitCommand ?? + (await resolveGitCommand( + process.env, + await outermostGitMarkerRoot(repository, signal), + )); + const checkoutHead = await resolveGitRef(repository, "HEAD", command, signal); if (checkoutHead !== target.head) { throw new InvalidTargetError( `Committed-diff scans require the repository checkout to match the requested head revision. Checkout HEAD is ${checkoutHead}; requested head is ${target.head}. Check out the requested head and retry.`, @@ -301,6 +328,7 @@ export async function validateCommittedDiffCheckout( const status = await gitOutput( repository, ["status", "--porcelain=v1", "--untracked-files=all"], + command, signal, ); if (status.length !== 0) { @@ -309,7 +337,12 @@ export async function validateCommittedDiffCheckout( ); } - const tracked = await gitOutput(repository, ["ls-files", "-t", "-z"], signal); + const tracked = await gitOutput( + repository, + ["ls-files", "-t", "-z"], + command, + signal, + ); if (tracked.split("\0").some((entry) => entry.startsWith("S "))) { throw new InvalidTargetError( "Committed-diff scans require a full repository checkout. Sparse checkouts are not supported; materialize skipped tracked files and retry.", @@ -334,21 +367,31 @@ export function validateMode(target: NormalizedTarget, mode: ScanMode): void { export async function repositoryRevision( repository: string, signal?: AbortSignal, + gitCommand?: InspectedExecutable, ): Promise { try { + const command = + gitCommand ?? + (await resolveGitCommand( + process.env, + await outermostGitMarkerRoot(repository, signal), + )); return await gitOutput( repository, ["rev-parse", "--verify", "HEAD^{commit}"], + command, signal, ); - } catch { + } catch (error) { throwIfAborted(signal); + if (error instanceof ConfigurationError) throw error; return null; } } async function requireGitRepository( repository: string, + command: InspectedExecutable, signal?: AbortSignal, ): Promise { let root: string; @@ -356,6 +399,7 @@ async function requireGitRepository( root = await gitOutput( repository, ["rev-parse", "--show-toplevel"], + command, signal, ); } catch (error) { @@ -378,12 +422,14 @@ async function requireGitRepository( async function resolveGitRef( repository: string, ref: string, + command: InspectedExecutable, signal?: AbortSignal, ): Promise { try { return await gitOutput( repository, ["rev-parse", "--verify", "--end-of-options", `${ref}^{commit}`], + command, signal, ); } catch (error) { @@ -395,29 +441,60 @@ async function resolveGitRef( async function gitOutput( repository: string, args: readonly string[], + command: InspectedExecutable, signal?: AbortSignal, ): Promise { throwIfAborted(signal); - const command = await resolveTrustedExecutable( - "git", - isolatedGitEnvironment(args[0] === "rev-parse"), - await outermostGitMarkerRoot(repository, signal), - ); - if (command === null) + if (command.executable === null) throw new Error("Git is not available on a trusted PATH."); - throwIfAborted(signal); const { stdout } = await execFile( command.executable, ["-c", "core.fsmonitor=false", "-C", repository, ...args], { encoding: "utf8", signal, - env: command.environment, + env: isolatedGitEnvironment(command.environment, args[0] === "rev-parse"), }, ); return stdout.trim(); } +export async function resolveGitCommand( + environment: Readonly>, + protectedRoot: string, +): Promise { + const binding = executableBinding(environment, "CODEX_SECURITY_GIT"); + let inspected = await inspectTrustedExecutable( + "git", + environment, + protectedRoot, + ); + let executable = inspected.executable; + if (binding.value === "") { + executable = null; + } else if (binding.value !== undefined) { + if (!isAbsolute(binding.value)) { + throw new ConfigurationError( + "CODEX_SECURITY_GIT must name an absolute trusted executable.", + ); + } + inspected = await inspectTrustedExecutable( + binding.value, + inspected.environment, + protectedRoot, + ); + if (inspected.executable === null) { + throw new ConfigurationError( + "CODEX_SECURITY_GIT does not name an available executable.", + ); + } + executable = binding.value; + } + for (const key of binding.keys) delete inspected.environment[key]; + inspected.environment["CODEX_SECURITY_GIT"] = executable ?? ""; + return { executable, environment: inspected.environment }; +} + export async function outermostGitMarkerRoot( repository: string, signal?: AbortSignal, @@ -439,9 +516,10 @@ export async function outermostGitMarkerRoot( } function isolatedGitEnvironment( + source: Readonly>, preserveGitConfiguration: boolean, ): NodeJS.ProcessEnv { - const environment = { ...process.env }; + const environment = { ...source }; for (const name of Object.keys(environment)) { const normalized = name.toUpperCase(); if ( diff --git a/sdk/typescript/src/trusted-executable.ts b/sdk/typescript/src/trusted-executable.ts index fece1394e..9f4e6340a 100644 --- a/sdk/typescript/src/trusted-executable.ts +++ b/sdk/typescript/src/trusted-executable.ts @@ -7,6 +7,24 @@ export interface TrustedExecutable { environment: Record; } +export interface InspectedExecutable { + executable: string | null; + environment: Record; +} + +export function executableBinding( + environment: Readonly>, + setting: "CODEX_SECURITY_GIT" | "CODEX_SECURITY_RG", +): { keys: string[]; value: string | undefined } { + const keys = + process.platform === "win32" + ? Object.keys(environment) + .filter((name) => name.toUpperCase() === setting) + .sort() + : [setting]; + return { keys, value: environment[keys[0] ?? setting] }; +} + export async function resolveTrustedExecutable( candidate: string, environment: Readonly>, @@ -26,10 +44,7 @@ export async function inspectTrustedExecutable( candidate: string, environment: Readonly>, protectedRoot: string, -): Promise<{ - executable: string | null; - environment: Record; -}> { +): Promise { const root = await realpath(protectedRoot).catch(() => resolve(protectedRoot), ); diff --git a/sdk/typescript/tests-ts/api.test.ts b/sdk/typescript/tests-ts/api.test.ts index dcb00236f..b53256cb6 100644 --- a/sdk/typescript/tests-ts/api.test.ts +++ b/sdk/typescript/tests-ts/api.test.ts @@ -54,6 +54,7 @@ import { } from "../src/runtime.js"; import * as runtimeModule from "../src/runtime.js"; import * as trustedExecutable from "../src/trusted-executable.js"; +import * as targetsModule from "../src/targets.js"; import { normalizeTarget } from "../src/targets.js"; import { SYNTHETIC_CREDENTIALS } from "./cli-fixtures.js"; import { INTEGRATION_TARGET, PLUGIN_ROOT } from "./plugin-root.js"; @@ -5475,11 +5476,13 @@ describe("CodexSecurity orchestration", () => { } const originalTrusted = { ...trustedExecutable }; const originalRuntime = { ...runtimeModule }; + const originalTargets = { ...targetsModule }; const originalPlatform = Object.getOwnPropertyDescriptor( process, "platform", )!; const inspected: [string, string][] = []; + const gitSelections: (string | null | undefined)[] = []; let gitHost: string | null = null; let host: string | null = null; let rejected: string | null = null; @@ -5523,6 +5526,17 @@ describe("CodexSecurity orchestration", () => { CODEX_CLI_PATH: process.execPath, }), })); + mock.module("../src/targets.js", () => ({ + ...originalTargets, + enclosingGitWorktreeRoot: async ( + _repository: string, + _signal?: AbortSignal, + command?: trustedExecutable.InspectedExecutable, + ) => { + gitSelections.push(command?.executable); + return null; + }, + })); const cases: { scenario: string; platform?: NodeJS.Platform; @@ -5668,6 +5682,7 @@ describe("CodexSecurity orchestration", () => { host = entry.host ? join(root, "host-tools", filename) : null; rejected = scenario === "rejected-copy" ? staged : null; inspected.length = 0; + gitSelections.length = 0; sanitizedGitEnvironment = undefined; const stageCalls: string[] = []; const workbenchEnvironments: WorkbenchCommandOptions["environment"][] = @@ -5720,6 +5735,10 @@ describe("CodexSecurity orchestration", () => { }, ); try { + await client.preflight(repository); + expect(gitSelections).toEqual([ + entry.expectedGit === "host" ? gitHost : null, + ]); if (scenario === "overlapping-workspace") { await expect(client.run(repository)).rejects.toBeInstanceOf( OutputInsideProtectedRootError, @@ -5787,6 +5806,7 @@ describe("CodexSecurity orchestration", () => { Object.defineProperty(process, "platform", originalPlatform); mock.module("../src/trusted-executable.js", () => originalTrusted); mock.module("../src/runtime.js", () => originalRuntime); + mock.module("../src/targets.js", () => originalTargets); } }); diff --git a/sdk/typescript/tests-ts/cli-export.test.ts b/sdk/typescript/tests-ts/cli-export.test.ts index 923e247c0..f81b7d1aa 100644 --- a/sdk/typescript/tests-ts/cli-export.test.ts +++ b/sdk/typescript/tests-ts/cli-export.test.ts @@ -40,6 +40,7 @@ describe("CLI", () => { Path: "C:\\Python;C:\\Windows\\System32", PYTHON: "/managed/python", TMPDIR: "/tmp", + CODEX_SECURITY_GIT: "", OPENAI_API_KEY: "openai-secret", CODEX_API_KEY: "codex-secret", GITHUB_TOKEN: "github-secret", @@ -49,7 +50,11 @@ describe("CLI", () => { Path: "C:\\Python;C:\\Windows\\System32", PYTHON: "/managed/python", TMPDIR: "/tmp", + CODEX_SECURITY_GIT: "", }); + expect(exportEnvironment({ CODEX_SECURITY_GIT: "/synthetic/git" })).toEqual( + { CODEX_SECURITY_GIT: "/synthetic/git" }, + ); }); test("exports findings to stdout without initializing Codex", async () => { diff --git a/sdk/typescript/tests-ts/targets-git-binding.test.ts b/sdk/typescript/tests-ts/targets-git-binding.test.ts new file mode 100644 index 000000000..afea6ce1f --- /dev/null +++ b/sdk/typescript/tests-ts/targets-git-binding.test.ts @@ -0,0 +1,202 @@ +import * as childProcess from "node:child_process"; +import { mkdir, mkdtemp, realpath, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { promisify } from "node:util"; +import { expect, mock, test } from "bun:test"; +import { ConfigurationError } from "../src/errors.js"; +import * as trustedExecutable from "../src/trusted-executable.js"; +import { runMockInSubprocess } from "./support/isolated-mock.js"; + +test("preserves raw executable bindings across platform aliases", () => { + const originalPlatform = Object.getOwnPropertyDescriptor( + process, + "platform", + )!; + const environment = { + CODEX_SECURITY_GIT: "", + Codex_Security_Git: "/synthetic/git", + }; + try { + Object.defineProperty(process, "platform", { value: "linux" }); + expect( + trustedExecutable.executableBinding(environment, "CODEX_SECURITY_GIT"), + ).toEqual({ keys: ["CODEX_SECURITY_GIT"], value: "" }); + expect( + trustedExecutable.executableBinding( + { Codex_Security_Git: "/synthetic/git" }, + "CODEX_SECURITY_GIT", + ).value, + ).toBeUndefined(); + Object.defineProperty(process, "platform", { value: "win32" }); + expect( + trustedExecutable.executableBinding(environment, "CODEX_SECURITY_GIT"), + ).toEqual({ + keys: ["CODEX_SECURITY_GIT", "Codex_Security_Git"], + value: "", + }); + expect( + trustedExecutable.executableBinding( + { Codex_Security_Git: "/synthetic/git" }, + "CODEX_SECURITY_GIT", + ).value, + ).toBe("/synthetic/git"); + } finally { + Object.defineProperty(process, "platform", originalPlatform); + } +}); + +test("uses one explicit Git command throughout target validation", async () => { + const name = "uses one explicit Git command throughout target validation"; + if (runMockInSubprocess(import.meta.path, name)) return; + + const root = await realpath(await mkdtemp(join(tmpdir(), "git-binding-"))); + const repository = join(root, "repository"); + await mkdir(repository); + const selected = join(root, "tools", "selected git"); + const discovered = join(root, "tools", "path git"); + const canonical = join(root, "tools", "canonical git"); + const missing = join(root, "tools", "missing git"); + const revision = "a".repeat(40); + const originalChildProcess = { ...childProcess }; + const originalTrusted = { ...trustedExecutable }; + const originalBinding = process.env["CODEX_SECURITY_GIT"]; + const inspections: string[] = []; + const calls: { + executable: string; + args: readonly string[]; + environment: NodeJS.ProcessEnv; + }[] = []; + const execute = Object.assign( + () => { + throw new Error("The callback form is not used by this test"); + }, + { + [promisify.custom]: async ( + executable: string, + args: readonly string[], + options: { env: NodeJS.ProcessEnv }, + ) => { + calls.push({ executable, args, environment: options.env }); + const stdout = args.includes("--show-toplevel") + ? repository + : args.includes("--verify") + ? revision + : args.includes("ls-files") + ? "H file.txt\0" + : ""; + return { stdout, stderr: "" }; + }, + }, + ); + mock.module("node:child_process", () => ({ + ...originalChildProcess, + execFile: execute, + })); + mock.module("../src/trusted-executable.js", () => ({ + ...originalTrusted, + inspectTrustedExecutable: async ( + candidate: string, + environment: Record, + protectedRoot: string, + ) => { + expect(protectedRoot).toBe(repository); + inspections.push(candidate); + return { + executable: + candidate === "git" + ? discovered + : candidate === selected + ? canonical + : null, + environment: { ...environment, PATH: "/synthetic/safe-path" }, + }; + }, + })); + + try { + const targets = await import("../src/targets.js"); + const command = await targets.resolveGitCommand( + { + PATH: "", + CODEX_SECURITY_GIT: selected, + GIT_DIR: "/synthetic/ignored-directory", + GIT_CONFIG_COUNT: "1", + KEEP: "present", + }, + repository, + ); + expect(command.executable).toBe(selected); + expect(inspections).toEqual(["git", selected]); + const target = await targets.normalizeTarget( + repository, + targets.DiffTarget.refs({ base: "HEAD" }), + undefined, + command, + ); + await targets.validateCommittedDiffCheckout( + repository, + target, + undefined, + command, + ); + expect( + await targets.enclosingGitWorktreeRoot(repository, undefined, command), + ).toBe(repository); + expect( + await targets.repositoryRevision(repository, undefined, command), + ).toBe(revision); + expect(calls.length).toBeGreaterThan(0); + for (const call of calls) { + expect(call.executable).toBe(selected); + expect(call.environment["CODEX_SECURITY_GIT"]).toBe(selected); + expect(call.environment["PATH"]).toBe("/synthetic/safe-path"); + expect(call.environment["KEEP"]).toBe("present"); + expect(call.environment["GIT_DIR"]).toBeUndefined(); + expect(call.environment["GIT_ALLOW_PROTOCOL"]).toBe(""); + expect(call.environment["GIT_CONFIG_COUNT"]).toBe( + call.args.includes("rev-parse") ? "1" : undefined, + ); + } + expect(inspections).toEqual(["git", selected]); + + const disabled = await targets.resolveGitCommand( + { CODEX_SECURITY_GIT: "" }, + repository, + ); + const previousCalls = calls.length; + expect( + await targets.repositoryRevision(repository, undefined, disabled), + ).toBeNull(); + expect( + await targets.enclosingGitWorktreeRoot(repository, undefined, disabled), + ).toBeNull(); + await expect( + targets.normalizeTarget( + repository, + targets.DiffTarget.refs({ base: "HEAD" }), + undefined, + disabled, + ), + ).rejects.toThrow("Diff targets require a Git repository"); + expect(calls).toHaveLength(previousCalls); + + await expect( + targets.resolveGitCommand({ CODEX_SECURITY_GIT: "relative" }, repository), + ).rejects.toBeInstanceOf(ConfigurationError); + await expect( + targets.resolveGitCommand({ CODEX_SECURITY_GIT: missing }, repository), + ).rejects.toBeInstanceOf(ConfigurationError); + expect(calls).toHaveLength(previousCalls); + + process.env["CODEX_SECURITY_GIT"] = selected; + expect(await targets.repositoryRevision(repository)).toBe(revision); + expect(calls.at(-1)?.executable).toBe(selected); + } finally { + if (originalBinding === undefined) delete process.env["CODEX_SECURITY_GIT"]; + else process.env["CODEX_SECURITY_GIT"] = originalBinding; + mock.module("node:child_process", () => originalChildProcess); + mock.module("../src/trusted-executable.js", () => originalTrusted); + await rm(root, { recursive: true, force: true }); + } +}); From 2a365a05c9ecff9bf1d7639aa846b723fd7ec0e9 Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Mon, 17 Aug 2026 16:22:41 -0700 Subject: [PATCH 14/27] fix(sdk): exclude internal helpers from public declarations --- sdk/typescript/src/api.ts | 1 + sdk/typescript/tsconfig.build.json | 1 + 2 files changed, 2 insertions(+) diff --git a/sdk/typescript/src/api.ts b/sdk/typescript/src/api.ts index d44161b2f..563a53c94 100644 --- a/sdk/typescript/src/api.ts +++ b/sdk/typescript/src/api.ts @@ -2340,6 +2340,7 @@ interface ScanEventRunOptions { onObserverError?: (observer: ScanObserverName, error: unknown) => void; } +/** @internal */ export async function runScanEvents( options: ScanEventRunOptions, ): Promise { diff --git a/sdk/typescript/tsconfig.build.json b/sdk/typescript/tsconfig.build.json index ccf7d2ecb..cf223fc5e 100644 --- a/sdk/typescript/tsconfig.build.json +++ b/sdk/typescript/tsconfig.build.json @@ -7,6 +7,7 @@ "outDir": "dist", "declaration": true, "declarationMap": true, + "stripInternal": true, "sourceMap": true, "inlineSources": true, "noEmit": false, From 1fa762012731508c89841d5c8abf89719ee95775 Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Mon, 17 Aug 2026 16:31:44 -0700 Subject: [PATCH 15/27] test(sdk): preserve output boundaries when Git is disabled --- sdk/typescript/tests-ts/api.test.ts | 47 ++++++++++++++++------------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/sdk/typescript/tests-ts/api.test.ts b/sdk/typescript/tests-ts/api.test.ts index b53256cb6..a9aea416a 100644 --- a/sdk/typescript/tests-ts/api.test.ts +++ b/sdk/typescript/tests-ts/api.test.ts @@ -1645,29 +1645,34 @@ describe("CodexSecurity orchestration", () => { const repository = join(worktree, "packages", "service"); const output = join(worktree, "scan"); await mkdir(repository, { recursive: true }); - let runtimeStarted = false; - const client = new TestClient( - {}, - { - environment: {}, - prepareRuntime: async () => { - runtimeStarted = true; - throw new Error("runtime should not initialize"); + for (const environment of [{}, { CODEX_SECURITY_GIT: "" }]) { + let runtimeStarted = false; + const client = new TestClient( + {}, + { + environment, + prepareRuntime: async () => { + runtimeStarted = true; + throw new Error("runtime should not initialize"); + }, }, - }, - ); + ); - await expect( - client.run(repository, { outputDir: output }), - ).rejects.toMatchObject({ - name: OutputInsideProtectedRootError.name, - outputDirectory: output, - protectedRoot: worktree, - pathKind: "output", - }); - expect(runtimeStarted).toBe(false); - await expect(stat(output)).rejects.toThrow(); - await client.close(); + try { + await expect( + client.run(repository, { outputDir: output }), + ).rejects.toMatchObject({ + name: OutputInsideProtectedRootError.name, + outputDirectory: output, + protectedRoot: worktree, + pathKind: "output", + }); + expect(runtimeStarted).toBe(false); + await expect(stat(output)).rejects.toThrow(); + } finally { + await client.close(); + } + } } }); From e9afbd94023ab9e0ce5d0506feb1952f79b2ce4e Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Mon, 17 Aug 2026 18:02:54 -0700 Subject: [PATCH 16/27] fix(git): align trusted tool bindings across scan hosts --- .../scripts/workbench_target.py | 4 + sdk/typescript/src/api.ts | 17 +-- sdk/typescript/src/targets.ts | 27 +++- sdk/typescript/src/trusted-executable.ts | 42 +++++- sdk/typescript/tests-ts/api.test.ts | 124 ++++++++++++++++++ .../tests-ts/targets-git-binding.test.ts | 5 +- .../tests-ts/trusted-executable.test.ts | 83 +++++++++++- .../workbench-tool-environment.test.ts | 39 ++++++ 8 files changed, 328 insertions(+), 13 deletions(-) diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_target.py b/sdk/typescript/_bundled_plugin/scripts/workbench_target.py index 93950d976..3992dfebc 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_target.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_target.py @@ -175,6 +175,8 @@ def _trusted_executable( candidate = Path(configured) if not candidate.is_absolute(): raise SystemExit(f"{setting} must name an absolute trusted executable.") + if sys.platform == "win32" and not os.path.splitext(candidate.name)[1]: + candidate = Path(f"{candidate}.exe") try: canonical = candidate.resolve(strict=True) if _inside_protected_git_root(canonical, root) or any( @@ -199,6 +201,8 @@ def _trusted_executable( del environment[key] environment["PATH"] = path for entry in os.get_exec_path(environment): + if sys.platform == "win32" and entry.startswith('"') and entry.endswith('"'): + entry = entry[1:-1] if not entry: continue try: diff --git a/sdk/typescript/src/api.ts b/sdk/typescript/src/api.ts index 563a53c94..56a7ea59d 100644 --- a/sdk/typescript/src/api.ts +++ b/sdk/typescript/src/api.ts @@ -540,7 +540,7 @@ export class CodexSecurity { runtimeHome, effectiveConfig, preflightConfig, - modelProvider, + scanEnvironment, authentication, approvalPolicy, python, @@ -559,11 +559,11 @@ export class CodexSecurity { signal, ); } - const pluginEnvironment = selectedScanEnvironment( - runtime.environment, - options.auth, - modelProvider, - ); + const pluginEnvironment = { + ...withoutCodexHome(scanEnvironment), + CODEX_HOME: runtime.codexHome, + CODEX_SECURITY_STATE_DIR: stateDirectory, + }; const { keys: gitKeys } = executableBinding( pluginEnvironment, "CODEX_SECURITY_GIT", @@ -593,13 +593,14 @@ export class CodexSecurity { configuredRipgrep, ripgrep.environment, protectedGitRoot, + { preserveInvocation: true }, ); if (inspected.executable === null) { throw new ConfigurationError( "CODEX_SECURITY_RG does not name an available executable.", ); } - ripgrep.environment = inspected.environment; + ripgrep = inspected; } if ( configuredRipgrep === undefined && @@ -632,7 +633,7 @@ export class CodexSecurity { gitCommand.executable ?? ""; // The Codex runtime can add its bundled tools to PATH after this point. trustedPluginEnvironment["CODEX_SECURITY_RG"] = - configuredRipgrep ?? ripgrep.executable ?? undefined; + configuredRipgrep === "" ? "" : ripgrep.executable ?? undefined; checkOpen(); const scanOutputRoot = requestedOutput === null && diff --git a/sdk/typescript/src/targets.ts b/sdk/typescript/src/targets.ts index 12905cadf..8692dad67 100644 --- a/sdk/typescript/src/targets.ts +++ b/sdk/typescript/src/targets.ts @@ -179,6 +179,18 @@ export function validatedGitEnvironment( } } +export function normalizeTarget( + repository: string, + target: ScanTarget, + signal?: AbortSignal, +): Promise; +/** @internal */ +export function normalizeTarget( + repository: string, + target: ScanTarget, + signal: AbortSignal | undefined, + gitCommand: InspectedExecutable, +): Promise; export async function normalizeTarget( repository: string, target: ScanTarget, @@ -364,6 +376,16 @@ export function validateMode(target: NormalizedTarget, mode: ScanMode): void { } } +export function repositoryRevision( + repository: string, + signal?: AbortSignal, +): Promise; +/** @internal */ +export function repositoryRevision( + repository: string, + signal: AbortSignal | undefined, + gitCommand: InspectedExecutable, +): Promise; export async function repositoryRevision( repository: string, signal?: AbortSignal, @@ -459,6 +481,7 @@ async function gitOutput( return stdout.trim(); } +/** @internal */ export async function resolveGitCommand( environment: Readonly>, protectedRoot: string, @@ -482,19 +505,21 @@ export async function resolveGitCommand( binding.value, inspected.environment, protectedRoot, + { preserveInvocation: true }, ); if (inspected.executable === null) { throw new ConfigurationError( "CODEX_SECURITY_GIT does not name an available executable.", ); } - executable = binding.value; + executable = inspected.executable; } for (const key of binding.keys) delete inspected.environment[key]; inspected.environment["CODEX_SECURITY_GIT"] = executable ?? ""; return { executable, environment: inspected.environment }; } +/** @internal */ export async function outermostGitMarkerRoot( repository: string, signal?: AbortSignal, diff --git a/sdk/typescript/src/trusted-executable.ts b/sdk/typescript/src/trusted-executable.ts index ddf1c3464..5de6cd606 100644 --- a/sdk/typescript/src/trusted-executable.ts +++ b/sdk/typescript/src/trusted-executable.ts @@ -2,6 +2,7 @@ import { constants } from "node:fs"; import { access, realpath, stat } from "node:fs/promises"; import { delimiter, + dirname, extname, isAbsolute, join, @@ -52,6 +53,7 @@ export async function inspectTrustedExecutable( candidate: string, environment: Readonly>, protectedRoot: string, + { preserveInvocation = false }: { preserveInvocation?: boolean } = {}, ): Promise { const root = await realpath(protectedRoot).catch(() => resolve(protectedRoot), @@ -106,7 +108,9 @@ export async function inspectTrustedExecutable( const candidates = pathLike ? extensions.map((extension) => ({ entry: null, - path: resolve(`${candidate}${extension.suffix}`), + path: preserveInvocation + ? `${candidate}${extension.suffix}` + : resolve(`${candidate}${extension.suffix}`), runnable: extension.runnable, })) : entries.flatMap((entry) => @@ -130,12 +134,23 @@ export async function inspectTrustedExecutable( } if (!current.runnable) continue; try { + if ( + preserveInvocation && + (!isAbsolute(candidate) || + (await hasProtectedAncestor(root, current.path, canonical))) + ) { + continue; + } await access( canonical, process.platform === "win32" ? constants.F_OK : constants.X_OK, ); if (!(await stat(canonical)).isFile()) continue; - executable ??= pathLike ? canonical : current.path; + executable ??= preserveInvocation + ? candidate + : pathLike + ? canonical + : current.path; } catch { continue; } @@ -148,6 +163,29 @@ export async function inspectTrustedExecutable( return { executable, environment: sanitizedEnvironment }; } +async function hasProtectedAncestor( + root: string, + ...paths: string[] +): Promise { + // Identities cover case aliases and junctions in the original invocation. + const protectedIdentity = await stat(root, { bigint: true }); + for (const path of paths) { + for (let directory = dirname(path); ; ) { + const identity = await stat(directory, { bigint: true }); + if ( + identity.dev === protectedIdentity.dev && + identity.ino === protectedIdentity.ino + ) { + return true; + } + const parent = dirname(directory); + if (parent === directory) break; + directory = parent; + } + } + return false; +} + function isWithin(root: string, candidate: string): boolean { const path = relative(root, candidate); return ( diff --git a/sdk/typescript/tests-ts/api.test.ts b/sdk/typescript/tests-ts/api.test.ts index a9aea416a..6b14d879c 100644 --- a/sdk/typescript/tests-ts/api.test.ts +++ b/sdk/typescript/tests-ts/api.test.ts @@ -5815,6 +5815,130 @@ describe("CodexSecurity orchestration", () => { } }); + test("refreshes explicit tool settings when a client reuses its runtime", async () => { + const name = + "refreshes explicit tool settings when a client reuses its runtime"; + if (runMockInSubprocess(import.meta.path, name)) return; + + const originalTrusted = { ...trustedExecutable }; + const root = await temporaryDirectory(); + const repository = join(root, "repository"); + const codexHome = join(root, "codex-home"); + const stateDirectory = join(root, "state"); + const scanDir = join(root, "scan"); + for (const path of [repository, codexHome, scanDir]) { + await mkdir(path, { mode: 0o700 }); + } + const filename = process.platform === "win32" ? "rg.exe" : "rg"; + const firstPath = join(root, "first-tools"); + const nextPath = join(root, "next-tools"); + const explicit = join(root, "selected-tools", filename); + const environment: Record = { + PATH: firstPath, + CODEX_CLI_PATH: process.execPath, + CODEX_HOME: join(root, "ambient-home"), + CODEX_SECURITY_STATE_DIR: stateDirectory, + CODEX_SECURITY_RG: explicit, + OPENAI_API_KEY: "synthetic-key", + }; + const runtimeEnvironment: Record = { + ...environment, + CODEX_HOME: codexHome, + }; + const runtime = { + ...preparedRuntime(codexHome), + environment: runtimeEnvironment, + }; + const workbenchEnvironments: WorkbenchCommandOptions["environment"][] = []; + const codexEnvironments: CodexOptions["env"][] = []; + let runtimePreparations = 0; + mock.module("../src/trusted-executable.js", () => ({ + ...originalTrusted, + inspectTrustedExecutable: async ( + candidate: string, + current: Record, + _protectedRoot: string, + options?: { preserveInvocation?: boolean }, + ) => { + if (candidate !== "git" && candidate !== "rg") { + expect(options?.preserveInvocation).toBe(true); + } + return { + executable: + candidate === "git" + ? null + : candidate === "rg" + ? join(current["PATH"]!, filename) + : candidate, + environment: { ...current }, + }; + }, + })); + const client = new TestClient( + {}, + { + environment, + prepareRuntime: async () => { + runtimePreparations++; + return runtime; + }, + resolvePluginPython: async () => join(root, "managed-python"), + prepareOutputDir: async () => scanDir, + repositoryRevision: async () => null, + stageBundledRipgrep: async () => { + throw new Error("A selected host tool must not be restaged"); + }, + runWorkbench: async ( + options: WorkbenchCommandOptions, + args: readonly string[], + ) => { + if (args[0] === "register-cli-scan") { + workbenchEnvironments.push(options.environment); + } + return mockWorkbench(args); + }, + createCodex: (options: CodexOptions) => { + codexEnvironments.push(options.env); + throw new Error("captured fresh tool environment"); + }, + }, + ); + const run = async (selection: string, path: string) => { + await expect(client.run(repository)).rejects.toThrow( + "captured fresh tool environment", + ); + for (const selected of [ + workbenchEnvironments.at(-1), + codexEnvironments.at(-1), + ]) { + expect(selected).toMatchObject({ + CODEX_HOME: codexHome, + CODEX_SECURITY_STATE_DIR: stateDirectory, + CODEX_SECURITY_GIT: "", + CODEX_SECURITY_RG: selection, + PATH: path, + }); + } + }; + try { + await run(explicit, firstPath); + environment["CODEX_SECURITY_RG"] = ""; + environment["PATH"] = nextPath; + await run("", nextPath); + delete environment["CODEX_SECURITY_RG"]; + await run(join(nextPath, filename), nextPath); + environment["CODEX_SECURITY_RG"] = explicit; + await run(explicit, nextPath); + expect(runtimePreparations).toBe(1); + expect(runtime.environment["CODEX_SECURITY_RG"]).toBe(explicit); + expect(workbenchEnvironments).toHaveLength(4); + expect(codexEnvironments).toHaveLength(4); + } finally { + await client.close(); + mock.module("../src/trusted-executable.js", () => originalTrusted); + } + }); + test("authenticates without initializing the plugin runtime", async () => { const root = await temporaryDirectory(); const stateDirectory = join(root, "state"); diff --git a/sdk/typescript/tests-ts/targets-git-binding.test.ts b/sdk/typescript/tests-ts/targets-git-binding.test.ts index afea6ce1f..060229ec8 100644 --- a/sdk/typescript/tests-ts/targets-git-binding.test.ts +++ b/sdk/typescript/tests-ts/targets-git-binding.test.ts @@ -99,6 +99,7 @@ test("uses one explicit Git command throughout target validation", async () => { candidate: string, environment: Record, protectedRoot: string, + options?: { preserveInvocation?: boolean }, ) => { expect(protectedRoot).toBe(repository); inspections.push(candidate); @@ -107,7 +108,9 @@ test("uses one explicit Git command throughout target validation", async () => { candidate === "git" ? discovered : candidate === selected - ? canonical + ? options?.preserveInvocation + ? selected + : canonical : null, environment: { ...environment, PATH: "/synthetic/safe-path" }, }; diff --git a/sdk/typescript/tests-ts/trusted-executable.test.ts b/sdk/typescript/tests-ts/trusted-executable.test.ts index 67110a4d9..78afccde6 100644 --- a/sdk/typescript/tests-ts/trusted-executable.test.ts +++ b/sdk/typescript/tests-ts/trusted-executable.test.ts @@ -11,7 +11,7 @@ import { } from "node:fs/promises"; import * as fsPromises from "node:fs/promises"; import { tmpdir } from "node:os"; -import { basename, delimiter, dirname, join, relative } from "node:path"; +import { basename, delimiter, dirname, join, relative, sep } from "node:path"; import { fileURLToPath } from "node:url"; import { afterEach, describe, expect, mock, test } from "bun:test"; import { @@ -152,6 +152,87 @@ describe("trusted executable resolution", () => { }, ); + test("preserves explicit invocations only outside the protected directory identity", async () => { + const name = + "preserves explicit invocations only outside the protected directory identity"; + if (runMockInSubprocess(import.meta.path, name)) return; + + const originalPromises = { ...fsPromises }; + const root = join(tmpdir(), "trusted-invocation-metadata-mock"); + const repository = join(root, "repository"); + const trusted = join(root, "trusted"); + const alias = join(root, "repository-alias"); + const caseAlias = join(root, "REPOSITORY"); + const extension = process.platform === "win32" ? ".exe" : ""; + const canonical = join(root, "native", `tool${extension}`); + const caseTarget = join(caseAlias, "native", `tool${extension}`); + const selected = join(trusted, "selected git"); + const inside = join(repository, "git"); + const linkedParent = join(repository, "host-tools", "git"); + const aliasedParent = join(alias, "git"); + const caseParent = join(caseAlias, "git"); + const traversedParent = `${alias}${sep}..${sep}trusted${sep}git`; + const externalCaseTarget = join(trusted, "case-target"); + const unsafe = [ + inside, + linkedParent, + aliasedParent, + caseParent, + traversedParent, + externalCaseTarget, + ]; + const paths = new Map([ + [repository, repository], + ...[selected, ...unsafe].map((path): [string, string] => [ + `${path}${extension}`, + path === externalCaseTarget ? caseTarget : canonical, + ]), + ]); + const protectedInode = 9_007_199_254_740_993n; + const identities = new Map([ + [repository, protectedInode], + [alias, protectedInode], + [caseAlias, protectedInode], + [trusted, protectedInode - 1n], + ]); + let nextInode = 1n; + mock.module("node:fs/promises", () => ({ + ...originalPromises, + realpath: async (path: string) => { + const result = paths.get(path); + if (result === undefined) { + throw Object.assign(new Error("missing synthetic path"), { + code: "ENOENT", + }); + } + return result; + }, + access: async () => {}, + stat: async (path: string, options?: { bigint?: boolean }) => { + if (!options?.bigint) { + return { isFile: () => path === canonical || path === caseTarget }; + } + if (!identities.has(path)) identities.set(path, nextInode++); + return { dev: 1n, ino: identities.get(path)! }; + }, + })); + + try { + const resolver = await import("../src/trusted-executable.js"); + const inspect = (path: string, preserveInvocation = true) => + resolver.inspectTrustedExecutable(path, { PATH: "" }, repository, { + preserveInvocation, + }); + expect((await inspect(selected)).executable).toBe(selected); + expect((await inspect(inside, false)).executable).toBe(canonical); + for (const path of unsafe) { + expect((await inspect(path)).executable).toBeNull(); + } + } finally { + mock.module("node:fs/promises", () => originalPromises); + } + }); + test("rejects canonical Windows batch targets without rejecting native aliases", async () => { if ( runMockInSubprocess( diff --git a/sdk/typescript/tests-ts/workbench-tool-environment.test.ts b/sdk/typescript/tests-ts/workbench-tool-environment.test.ts index 27deb564a..5f78ce18c 100644 --- a/sdk/typescript/tests-ts/workbench-tool-environment.test.ts +++ b/sdk/typescript/tests-ts/workbench-tool-environment.test.ts @@ -127,6 +127,45 @@ describe("workbench tool environments", () => { }); }); + test("accepts extensionless bindings and quoted Windows tool paths", () => { + runPythonMocks(` +import tempfile +with tempfile.TemporaryDirectory(prefix="workbench-tool-paths-") as temporary: + root = Path(temporary).resolve() + repository = root / "repository" + directory = root / "tools with spaces" + repository.mkdir() + directory.mkdir() + for name in ("git", "rg"): + (directory / (name + ".exe")).write_bytes(b"synthetic native executable") + (directory / "missing.com").write_bytes(b"synthetic native executable") + with ( + patch.object(workbench.sys, "platform", "win32"), + patch.object(workbench.subprocess, "run") as run, + ): + for name in ("git", "rg"): + invocation = str(directory / name) + setting = "CODEX_SECURITY_" + name.upper() + assert workbench._trusted_executable( + repository, {"PATH": "", setting: invocation}, name, + ) == invocation + environment = {"PATH": '"' + str(directory) + '"'} + assert workbench._trusted_executable( + repository, environment, name, + ) == str(directory / (name + ".exe")) + assert environment["PATH"] == str(directory) + try: + workbench._trusted_executable( + repository, {setting: str(directory / "missing")}, name, + ) + except SystemExit: + pass + else: + raise AssertionError("extensionless invocation incorrectly selected a .com file") + run.assert_not_called() +`); + }); + test("treats stale target roots as unavailable without hiding other errors", () => { runPythonMocks(` import stat From 30a94f4f5cffc52ca9a964534268ea722dbb6c5d Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Mon, 17 Aug 2026 23:13:58 -0700 Subject: [PATCH 17/27] fix(git): honor selected executable for bulk checkouts --- sdk/typescript/src/multiscan.ts | 12 ++--- sdk/typescript/tests-ts/multiscan.test.ts | 65 +++++++++++++++++++++++ 2 files changed, 70 insertions(+), 7 deletions(-) diff --git a/sdk/typescript/src/multiscan.ts b/sdk/typescript/src/multiscan.ts index 63f3a7909..f3683fa90 100644 --- a/sdk/typescript/src/multiscan.ts +++ b/sdk/typescript/src/multiscan.ts @@ -23,8 +23,7 @@ import type { ScanCost } from "./cost.js"; import { safeErrorMessage, ScanCostLimitExceededError } from "./errors.js"; import type { CoverageDocument } from "./models.js"; import { requireSecureOutputAncestry } from "./runtime.js"; -import type { ScanMode } from "./targets.js"; -import { resolveTrustedExecutable } from "./trusted-executable.js"; +import { resolveGitCommand, type ScanMode } from "./targets.js"; const execFile = promisify(execFileCallback); const REQUIRED_ARTIFACTS = [ @@ -804,19 +803,18 @@ async function checkoutRevision( } environment["GIT_TERMINAL_PROMPT"] = "0"; environment["GIT_LFS_SKIP_SMUDGE"] = "1"; - const command = await resolveTrustedExecutable( - "git", + const { executable, environment: gitEnvironment } = await resolveGitCommand( environment, resolve(process.cwd()), ); - if (command === null) { + if (executable === null) { throw new Error("Git is not available on a trusted PATH."); } const git = async (...args: string[]): Promise => { // Use the resolved absolute path so Windows PATHEXT cannot prefer a // .bat/.cmd shim over the trusted executable selected above. const result = await execFile( - command.executable, + executable, [ "-c", "core.hooksPath=/dev/null", @@ -825,7 +823,7 @@ async function checkoutRevision( path, ...args, ], - { env: command.environment, signal }, + { env: gitEnvironment, signal }, ); return result.stdout.trim(); }; diff --git a/sdk/typescript/tests-ts/multiscan.test.ts b/sdk/typescript/tests-ts/multiscan.test.ts index 33676192a..b2fa3ef4b 100644 --- a/sdk/typescript/tests-ts/multiscan.test.ts +++ b/sdk/typescript/tests-ts/multiscan.test.ts @@ -25,6 +25,7 @@ import type { ScanResult } from "../src/result.js"; import { buildGitHubCredentialArgs, runMultiscan } from "../src/multiscan.js"; import { resolveTrustedExecutable } from "../src/trusted-executable.js"; import { capture, dependencies, fakeResult } from "./cli-fixtures.js"; +import { runMockInSubprocess } from "./support/isolated-mock.js"; type MultiscanOptions = Parameters[0]; type SecurityClient = ReturnType; @@ -133,6 +134,70 @@ async function results(path: string): Promise[]> { } describe("multiscan", () => { + for (const setting of ["selected", "disabled", "invalid"] as const) { + const name = `honors ${setting} Git settings before bulk checkout`; + test(name, async () => { + if (runMockInSubprocess(import.meta.path, name)) return; + + const paths = await fixture(); + const source = await repository(paths.root, "source"); + const trusted = await resolveTrustedExecutable( + "git", + process.env, + process.cwd(), + ); + if (trusted === null) throw new Error("Git is required by this fixture."); + await writeFile( + paths.input, + `id,repository,revision\nsource,${source.path},${source.revision}\n`, + ); + const previousPath = process.env["PATH"]; + const previousGit = process.env["CODEX_SECURITY_GIT"]; + let scans = 0; + try { + process.env["CODEX_SECURITY_GIT"] = + setting === "selected" + ? trusted.executable + : setting === "disabled" + ? "" + : join(paths.root, "missing-git"); + if (setting === "selected") process.env["PATH"] = ""; + + const summary = await runMultiscan( + options( + paths, + client(async (checkout, scanOptions = {}) => { + scans += 1; + expect( + await readFile(join(checkout, "src", "app.ts"), "utf8"), + ).toContain('export const name = "source";'); + return await completedScan(scanOptions.outputDir!); + }), + { maxAttempts: 1 }, + ), + ); + + if (setting === "selected") { + expect(summary).toMatchObject({ completed: 1, failed: 0 }); + expect(scans).toBe(1); + } else { + expect(summary).toMatchObject({ completed: 0, failed: 1 }); + expect(scans).toBe(0); + expect((await results(summary.resultsPath))[0]?.["error"]).toContain( + setting === "disabled" + ? "Git is not available on a trusted PATH." + : "CODEX_SECURITY_GIT does not name an available executable.", + ); + } + } finally { + if (previousPath === undefined) delete process.env["PATH"]; + else process.env["PATH"] = previousPath; + if (previousGit === undefined) delete process.env["CODEX_SECURITY_GIT"]; + else process.env["CODEX_SECURITY_GIT"] = previousGit; + } + }); + } + test("scopes GitHub CLI credentials to the discovered GitHub host", () => { expect(buildGitHubCredentialArgs(undefined)).toEqual([]); expect(buildGitHubCredentialArgs("github.com")).toEqual([ From 633f0e8d071bfe570bb384392327228b838ffffb Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Tue, 18 Aug 2026 00:08:44 -0700 Subject: [PATCH 18/27] fix(multiscan): exclude local source roots from Git selection --- sdk/typescript/src/multiscan.ts | 16 ++++- sdk/typescript/src/targets.ts | 9 +-- sdk/typescript/src/trusted-executable.ts | 37 +++++++---- sdk/typescript/tests-ts/multiscan.test.ts | 42 ++++++++++++ sdk/typescript/tests-ts/targets.test.ts | 18 +++++- .../tests-ts/trusted-executable.test.ts | 64 +++++++++++++++++++ 6 files changed, 166 insertions(+), 20 deletions(-) diff --git a/sdk/typescript/src/multiscan.ts b/sdk/typescript/src/multiscan.ts index f3683fa90..67c0f94ac 100644 --- a/sdk/typescript/src/multiscan.ts +++ b/sdk/typescript/src/multiscan.ts @@ -23,7 +23,11 @@ import type { ScanCost } from "./cost.js"; import { safeErrorMessage, ScanCostLimitExceededError } from "./errors.js"; import type { CoverageDocument } from "./models.js"; import { requireSecureOutputAncestry } from "./runtime.js"; -import { resolveGitCommand, type ScanMode } from "./targets.js"; +import { + outermostGitMarkerRoot, + resolveGitCommand, + type ScanMode, +} from "./targets.js"; const execFile = promisify(execFileCallback); const REQUIRED_ARTIFACTS = [ @@ -803,9 +807,17 @@ async function checkoutRevision( } environment["GIT_TERMINAL_PROMPT"] = "0"; environment["GIT_LFS_SKIP_SMUDGE"] = "1"; + const protectedRoots = [ + await outermostGitMarkerRoot(await realpath(process.cwd()), signal), + ]; + if (isAbsolute(task.repository)) { + protectedRoots.push( + await outermostGitMarkerRoot(await realpath(task.repository), signal), + ); + } const { executable, environment: gitEnvironment } = await resolveGitCommand( environment, - resolve(process.cwd()), + protectedRoots, ); if (executable === null) { throw new Error("Git is not available on a trusted PATH."); diff --git a/sdk/typescript/src/targets.ts b/sdk/typescript/src/targets.ts index 4c0b7ea35..c5c3fbef4 100644 --- a/sdk/typescript/src/targets.ts +++ b/sdk/typescript/src/targets.ts @@ -484,13 +484,13 @@ async function gitOutput( /** @internal */ export async function resolveGitCommand( environment: Readonly>, - protectedRoot: string, + protectedRoots: string | readonly string[], ): Promise { const binding = executableBinding(environment, "CODEX_SECURITY_GIT"); let inspected = await inspectTrustedExecutable( "git", environment, - protectedRoot, + protectedRoots, ); let executable = inspected.executable; if (binding.value === "") { @@ -504,7 +504,7 @@ export async function resolveGitCommand( inspected = await inspectTrustedExecutable( binding.value, inspected.environment, - protectedRoot, + protectedRoots, { preserveInvocation: true }, ); if (inspected.executable === null) { @@ -532,7 +532,8 @@ export async function outermostGitMarkerRoot( await lstat(join(current, ".git")); root = current; } catch (error) { - if ((error as NodeJS.ErrnoException).code !== "ENOENT") throw error; + const code = (error as NodeJS.ErrnoException).code; + if (code !== "ENOENT" && code !== "ENOTDIR") throw error; } const parent = dirname(current); if (parent === current) return root; diff --git a/sdk/typescript/src/trusted-executable.ts b/sdk/typescript/src/trusted-executable.ts index 5de6cd606..152cf98b4 100644 --- a/sdk/typescript/src/trusted-executable.ts +++ b/sdk/typescript/src/trusted-executable.ts @@ -37,12 +37,12 @@ export function executableBinding( export async function resolveTrustedExecutable( candidate: string, environment: Readonly>, - protectedRoot: string, + protectedRoots: string | readonly string[], ): Promise { const inspected = await inspectTrustedExecutable( candidate, environment, - protectedRoot, + protectedRoots, ); return inspected.executable === null ? null @@ -52,11 +52,14 @@ export async function resolveTrustedExecutable( export async function inspectTrustedExecutable( candidate: string, environment: Readonly>, - protectedRoot: string, + protectedRoots: string | readonly string[], { preserveInvocation = false }: { preserveInvocation?: boolean } = {}, ): Promise { - const root = await realpath(protectedRoot).catch(() => - resolve(protectedRoot), + const roots = await Promise.all( + (typeof protectedRoots === "string" + ? [protectedRoots] + : protectedRoots + ).map(async (root) => await realpath(root).catch(() => resolve(root))), ); const pathKeys = process.platform === "win32" @@ -81,7 +84,9 @@ export async function inspectTrustedExecutable( } if (entry.length === 0) continue; const canonical = await realpath(entry).catch(() => null); - if (canonical === null || isWithin(root, canonical)) continue; + if (canonical === null || roots.some((root) => isWithin(root, canonical))) { + continue; + } if (!entries.includes(canonical)) entries.push(canonical); } @@ -125,7 +130,7 @@ export async function inspectTrustedExecutable( for (const current of candidates) { const canonical = await realpath(current.path).catch(() => null); if (canonical === null) continue; - if (isWithin(root, canonical)) { + if (roots.some((root) => isWithin(root, canonical))) { if (current.entry !== null) unsafeEntries.add(current.entry); continue; } @@ -137,7 +142,7 @@ export async function inspectTrustedExecutable( if ( preserveInvocation && (!isAbsolute(candidate) || - (await hasProtectedAncestor(root, current.path, canonical))) + (await hasProtectedAncestor(roots, current.path, canonical))) ) { continue; } @@ -164,17 +169,23 @@ export async function inspectTrustedExecutable( } async function hasProtectedAncestor( - root: string, + roots: readonly string[], ...paths: string[] ): Promise { // Identities cover case aliases and junctions in the original invocation. - const protectedIdentity = await stat(root, { bigint: true }); + const protectedIdentities = await Promise.all( + roots.map((root) => stat(root, { bigint: true })), + ); for (const path of paths) { - for (let directory = dirname(path); ; ) { + // A local Git fetch source can itself be a file. + for (let directory = path; ; ) { const identity = await stat(directory, { bigint: true }); if ( - identity.dev === protectedIdentity.dev && - identity.ino === protectedIdentity.ino + protectedIdentities.some( + (protectedIdentity) => + identity.dev === protectedIdentity.dev && + identity.ino === protectedIdentity.ino, + ) ) { return true; } diff --git a/sdk/typescript/tests-ts/multiscan.test.ts b/sdk/typescript/tests-ts/multiscan.test.ts index b2fa3ef4b..0285100e8 100644 --- a/sdk/typescript/tests-ts/multiscan.test.ts +++ b/sdk/typescript/tests-ts/multiscan.test.ts @@ -134,6 +134,48 @@ async function results(path: string): Promise[]> { } describe("multiscan", () => { + test("rejects selected Git from an outer local inventory repository", async () => { + const name = + "rejects selected Git from an outer local inventory repository"; + if (runMockInSubprocess(import.meta.path, name)) return; + + const paths = await fixture(); + const outer = await repository(paths.root, "outer"); + const source = await repository(outer.path, "source"); + const selected = join( + outer.path, + process.platform === "win32" ? "selected-git.exe" : "selected-git", + ); + await writeFile(selected, "", { mode: 0o700 }); + await writeFile( + paths.input, + `id,repository,revision\nsource,${source.path},${source.revision}\n`, + ); + const previousGit = process.env["CODEX_SECURITY_GIT"]; + let scans = 0; + try { + process.env["CODEX_SECURITY_GIT"] = selected; + const summary = await runMultiscan( + options( + paths, + client(async (_checkout, scanOptions = {}) => { + scans += 1; + return await completedScan(scanOptions.outputDir!); + }), + { maxAttempts: 1 }, + ), + ); + expect(summary).toMatchObject({ completed: 0, failed: 1 }); + expect(scans).toBe(0); + expect((await results(summary.resultsPath))[0]?.["error"]).toContain( + "CODEX_SECURITY_GIT does not name an available executable.", + ); + } finally { + if (previousGit === undefined) delete process.env["CODEX_SECURITY_GIT"]; + else process.env["CODEX_SECURITY_GIT"] = previousGit; + } + }); + for (const setting of ["selected", "disabled", "invalid"] as const) { const name = `honors ${setting} Git settings before bulk checkout`; test(name, async () => { diff --git a/sdk/typescript/tests-ts/targets.test.ts b/sdk/typescript/tests-ts/targets.test.ts index f24f2aa7b..2f6efa7b1 100644 --- a/sdk/typescript/tests-ts/targets.test.ts +++ b/sdk/typescript/tests-ts/targets.test.ts @@ -12,7 +12,7 @@ import { writeFile, } from "node:fs/promises"; import { tmpdir } from "node:os"; -import { delimiter, join } from "node:path"; +import { delimiter, dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; import { afterEach, describe, expect, test } from "bun:test"; import { @@ -23,6 +23,7 @@ import { repositoryRevision, type ScanTarget, } from "../src/index.js"; +import { outermostGitMarkerRoot } from "../src/targets.js"; // @ts-expect-error DiffTarget is intentionally nominal; use its constructor helpers. const structurallyInvalidTarget: ScanTarget = { @@ -62,6 +63,21 @@ function git(repo: string, ...args: string[]): string { return execFileSync("git", args, { cwd: repo, encoding: "utf8" }).trim(); } +test("finds outer Git roots without rejecting local source files", async () => { + const repo = await repository(); + const nested = join(repo, "nested"); + await mkdir(join(nested, ".git"), { recursive: true }); + const inside = join(nested, "source.bundle"); + const outside = join(dirname(repo), "standalone.bundle"); + await Promise.all([ + writeFile(inside, "synthetic bundle fixture\n"), + writeFile(outside, "synthetic bundle fixture\n"), + ]); + + expect(await outermostGitMarkerRoot(inside)).toBe(repo); + expect(await outermostGitMarkerRoot(outside)).toBe(outside); +}); + async function createRepositoryGitShim( directory: string, marker: string, diff --git a/sdk/typescript/tests-ts/trusted-executable.test.ts b/sdk/typescript/tests-ts/trusted-executable.test.ts index 78afccde6..9f3a82861 100644 --- a/sdk/typescript/tests-ts/trusted-executable.test.ts +++ b/sdk/typescript/tests-ts/trusted-executable.test.ts @@ -2,6 +2,7 @@ import { spawnSync } from "node:child_process"; import { constants } from "node:fs"; import { chmod, + link, mkdir, mkdtemp, realpath, @@ -83,6 +84,69 @@ async function resolveWindowsExecutable( } describe("trusted executable resolution", () => { + test("excludes every protected root before selecting an executable", async () => { + const root = await temporaryDirectory(); + const repositories = [join(root, "working"), join(root, "source")]; + const unsafe = repositories.map((repository) => join(repository, "bin")); + const trusted = join(root, "trusted"); + const name = process.platform === "win32" ? "git.exe" : "git"; + for (const directory of [...unsafe, trusted]) { + await mkdir(directory, { recursive: true }); + await writeFile(join(directory, name), "synthetic executable\n", { + mode: 0o700, + }); + } + const linked = join(repositories[1]!, "host-tools"); + await symlink( + trusted, + linked, + process.platform === "win32" ? "junction" : "dir", + ); + + expect( + await resolveTrustedExecutable( + "git", + { PATH: [...unsafe, trusted].join(delimiter), KEEP: "ok" }, + repositories, + ), + ).toEqual({ + executable: join(trusted, name), + environment: { PATH: trusted, KEEP: "ok" }, + }); + for (const path of [...unsafe, linked]) { + const inspected = await inspectTrustedExecutable( + join(path, name), + { PATH: trusted }, + repositories, + { preserveInvocation: true }, + ); + expect(inspected.executable).toBeNull(); + } + expect( + ( + await inspectTrustedExecutable( + join(trusted, name), + { PATH: trusted }, + repositories, + { preserveInvocation: true }, + ) + ).executable, + ).toBe(join(trusted, name)); + + const sourceFile = join(root, "source-file"); + await link(join(trusted, name), sourceFile); + expect( + ( + await inspectTrustedExecutable( + join(trusted, name), + { PATH: "" }, + [repositories[0]!, sourceFile], + { preserveInvocation: true }, + ) + ).executable, + ).toBeNull(); + }); + test("accepts safe relative PATH entries without trusting repository links", async () => { const root = await temporaryDirectory(); const repository = join(root, "repository"); From cff253172d6d6dc576430bbbbd280129be2b5838 Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Tue, 18 Aug 2026 00:32:10 -0700 Subject: [PATCH 19/27] fix(multiscan): protect all campaign source roots --- sdk/typescript/src/multiscan.ts | 38 +++++-- sdk/typescript/tests-ts/multiscan.test.ts | 121 +++++++++++++++++++--- 2 files changed, 133 insertions(+), 26 deletions(-) diff --git a/sdk/typescript/src/multiscan.ts b/sdk/typescript/src/multiscan.ts index 67c0f94ac..74c434a7a 100644 --- a/sdk/typescript/src/multiscan.ts +++ b/sdk/typescript/src/multiscan.ts @@ -131,7 +131,7 @@ async function runCampaign( output: string, ): Promise { const ledger = join(output, "results.jsonl"); - await ensureOutputDirectory(join(output, "checkouts")); + const checkoutRoot = await ensureOutputDirectory(join(output, "checkouts")); await ensureOutputDirectory(join(output, "artifacts")); await ensureManifest(join(output, "manifest.json"), tasks, options); const receipts = await readReceipts(ledger); @@ -197,6 +197,30 @@ async function runCampaign( }; } + const protectedRoots = new Set([ + await outermostGitMarkerRoot(await realpath(process.cwd()), options.signal), + checkoutRoot, + ]); + for (const task of tasks) { + if (!isAbsolute(task.repository)) continue; + const canonical = await realpath(task.repository).catch( + (error: NodeJS.ErrnoException) => { + if (error.code === "ENOENT" || error.code === "ENOTDIR") + return undefined; + throw error; + }, + ); + const root = await outermostGitMarkerRoot( + canonical ?? task.repository, + options.signal, + ); + // Missing sources remain per-task failures; keep any enclosing Git root. + if (canonical !== undefined || root !== task.repository) { + protectedRoots.add(await realpath(root)); + } + } + const gitRoots = [...protectedRoots]; + let next = 0; let failed = 0; const worker = async ( @@ -210,7 +234,7 @@ async function runCampaign( for (let retry = 0; retry < options.maxAttempts; retry += 1) { options.signal?.throwIfAborted(); attempt += 1; - const checkout = join(output, "checkouts", task.id); + const checkout = join(checkoutRoot, task.id); const scanDir = join( output, "artifacts", @@ -231,6 +255,7 @@ async function runCampaign( await checkoutRevision( task, checkout, + gitRoots, options.signal, options.githubHost, ); @@ -791,6 +816,7 @@ function normalizeRepository(repository: string, directory: string): string { async function checkoutRevision( task: MultiscanTask, path: string, + protectedRoots: readonly string[], signal?: AbortSignal, githubHost?: string, ): Promise { @@ -807,14 +833,6 @@ async function checkoutRevision( } environment["GIT_TERMINAL_PROMPT"] = "0"; environment["GIT_LFS_SKIP_SMUDGE"] = "1"; - const protectedRoots = [ - await outermostGitMarkerRoot(await realpath(process.cwd()), signal), - ]; - if (isAbsolute(task.repository)) { - protectedRoots.push( - await outermostGitMarkerRoot(await realpath(task.repository), signal), - ); - } const { executable, environment: gitEnvironment } = await resolveGitCommand( environment, protectedRoots, diff --git a/sdk/typescript/tests-ts/multiscan.test.ts b/sdk/typescript/tests-ts/multiscan.test.ts index 0285100e8..30ba55ce4 100644 --- a/sdk/typescript/tests-ts/multiscan.test.ts +++ b/sdk/typescript/tests-ts/multiscan.test.ts @@ -134,27 +134,113 @@ async function results(path: string): Promise[]> { } describe("multiscan", () => { - test("rejects selected Git from an outer local inventory repository", async () => { - const name = - "rejects selected Git from an outer local inventory repository"; + for (const location of [ + "pending inventory", + "completed inventory", + "managed checkout", + ] as const) { + const name = `rejects selected Git from ${location} campaign roots`; + test(name, async () => { + if (runMockInSubprocess(import.meta.path, name)) return; + + const paths = await fixture(); + const outer = await repository(paths.root, "outer"); + const source = await repository(outer.path, "source"); + const other = await repository(paths.root, "other"); + const selectedDirectory = + location === "managed checkout" + ? join(paths.output, "checkouts", "retained") + : outer.path; + await mkdir(selectedDirectory, { recursive: true, mode: 0o700 }); + const selected = join( + selectedDirectory, + process.platform === "win32" ? "selected-git.exe" : "selected-git", + ); + await writeFile(selected, "", { mode: 0o700 }); + await writeFile( + paths.input, + `id,repository,revision\nother,${other.path},${other.revision}\nsource,${source.path},${source.revision}\n`, + ); + const previousGit = process.env["CODEX_SECURITY_GIT"]; + let previousReceipts = 0; + let scans = 0; + try { + if (location === "completed inventory") { + delete process.env["CODEX_SECURITY_GIT"]; + const initial = await runMultiscan( + options( + paths, + client(async (_checkout, scanOptions = {}) => + completedScan(scanOptions.outputDir!), + ), + { maxAttempts: 1 }, + ), + ); + expect(initial).toMatchObject({ completed: 2, failed: 0 }); + const receipts = await results(initial.resultsPath); + previousReceipts = receipts.length; + const otherReceipt = receipts.find( + (receipt) => receipt["id"] === "other", + )!; + await rm(join(otherReceipt["outputDir"] as string, "report.md")); + } + + process.env["CODEX_SECURITY_GIT"] = selected; + const summary = await runMultiscan( + options( + paths, + client(async (_checkout, scanOptions = {}) => { + scans += 1; + return await completedScan(scanOptions.outputDir!); + }), + { maxAttempts: 1 }, + ), + ); + const resumed = location === "completed inventory"; + expect(summary).toMatchObject({ + completed: resumed ? 1 : 0, + failed: resumed ? 1 : 2, + skipped: resumed ? 1 : 0, + }); + expect(scans).toBe(0); + const receipts = (await results(summary.resultsPath)).slice( + previousReceipts, + ); + expect(receipts).toHaveLength(resumed ? 1 : 2); + for (const receipt of receipts) { + expect(receipt["error"]).toContain( + "CODEX_SECURITY_GIT does not name an available executable.", + ); + } + } finally { + if (previousGit === undefined) delete process.env["CODEX_SECURITY_GIT"]; + else process.env["CODEX_SECURITY_GIT"] = previousGit; + } + }); + } + + test("keeps missing local sources as per-task failures", async () => { + const name = "keeps missing local sources as per-task failures"; if (runMockInSubprocess(import.meta.path, name)) return; const paths = await fixture(); - const outer = await repository(paths.root, "outer"); - const source = await repository(outer.path, "source"); - const selected = join( - outer.path, - process.platform === "win32" ? "selected-git.exe" : "selected-git", + const source = await repository(paths.root, "source"); + const trusted = await resolveTrustedExecutable( + "git", + process.env, + process.cwd(), ); - await writeFile(selected, "", { mode: 0o700 }); + if (trusted === null) throw new Error("Git is required by this fixture."); await writeFile( paths.input, - `id,repository,revision\nsource,${source.path},${source.revision}\n`, + `id,repository,revision\nmissing,${join(paths.root, "missing")},${source.revision}\nsource,${source.path},${source.revision}\n`, ); + const previousPath = process.env["PATH"]; const previousGit = process.env["CODEX_SECURITY_GIT"]; let scans = 0; try { - process.env["CODEX_SECURITY_GIT"] = selected; + process.env["PATH"] = ""; + process.env["CODEX_SECURITY_GIT"] = trusted.executable; const summary = await runMultiscan( options( paths, @@ -165,12 +251,15 @@ describe("multiscan", () => { { maxAttempts: 1 }, ), ); - expect(summary).toMatchObject({ completed: 0, failed: 1 }); - expect(scans).toBe(0); - expect((await results(summary.resultsPath))[0]?.["error"]).toContain( - "CODEX_SECURITY_GIT does not name an available executable.", - ); + expect(summary).toMatchObject({ completed: 1, failed: 1 }); + expect(scans).toBe(1); + expect(await results(summary.resultsPath)).toMatchObject([ + { id: "missing", status: "failed" }, + { id: "source", status: "completed" }, + ]); } finally { + if (previousPath === undefined) delete process.env["PATH"]; + else process.env["PATH"] = previousPath; if (previousGit === undefined) delete process.env["CODEX_SECURITY_GIT"]; else process.env["CODEX_SECURITY_GIT"] = previousGit; } From 5896809e93169d0675fb7a485213ad045d935792 Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Tue, 18 Aug 2026 01:09:04 -0700 Subject: [PATCH 20/27] fix(git): protect repositories supplying scan inputs --- sdk/typescript/src/api.ts | 18 +++- sdk/typescript/src/multiscan.ts | 37 +++---- sdk/typescript/src/targets.ts | 46 ++++++++- sdk/typescript/tests-ts/api.test.ts | 118 +++++++++++++++++++++- sdk/typescript/tests-ts/multiscan.test.ts | 93 ++++++++++++++++- sdk/typescript/tests-ts/targets.test.ts | 29 +++++- 6 files changed, 303 insertions(+), 38 deletions(-) diff --git a/sdk/typescript/src/api.ts b/sdk/typescript/src/api.ts index cc3e92ce8..534d76911 100644 --- a/sdk/typescript/src/api.ts +++ b/sdk/typescript/src/api.ts @@ -127,6 +127,7 @@ import { normalizeRepository, normalizeTarget, outermostGitMarkerRoot, + protectedGitInputRoots, repositoryRevision, resolveGitCommand, resolveRepositoryPath, @@ -308,6 +309,7 @@ interface LocalScanInputs extends Omit { protectedRoot: string; protectedGitRoot: string; + protectedGitRoots: readonly string[]; gitCommand: InspectedExecutable; stateDirectory: string; } @@ -507,6 +509,7 @@ export class CodexSecurity { outputDir: requestedOutput, protectedRoot, protectedGitRoot, + protectedGitRoots, gitCommand, stateDirectory, } = await this.#validateLocalInputs(repository, options, signal); @@ -580,12 +583,12 @@ export class CodexSecurity { const git = await inspectTrustedExecutable( "git", pluginEnvironment, - protectedGitRoot, + protectedGitRoots, ); let ripgrep = await inspectTrustedExecutable( "rg", git.environment, - protectedGitRoot, + protectedGitRoots, ); if (configuredRipgrep !== undefined && configuredRipgrep !== "") { if (!isAbsolute(configuredRipgrep)) { @@ -596,7 +599,7 @@ export class CodexSecurity { const inspected = await inspectTrustedExecutable( configuredRipgrep, ripgrep.environment, - protectedGitRoot, + protectedGitRoots, { preserveInvocation: true }, ); if (inspected.executable === null) { @@ -623,7 +626,7 @@ export class CodexSecurity { ripgrep = await inspectTrustedExecutable( runtime.bundledRipgrep, ripgrep.environment, - protectedGitRoot, + protectedGitRoots, ); } } @@ -1984,9 +1987,13 @@ export class CodexSecurity { const requestedTarget = options.target ?? "repository"; validatedGitEnvironment(this.#dependencies.environment); const protectedGitRoot = await outermostGitMarkerRoot(repo, signal); + const protectedGitRoots = await protectedGitInputRoots( + [repositoryPath, ...(options.knowledgeBasePaths ?? [])], + signal, + ); const gitCommand = await resolveGitCommand( this.#dependencies.environment, - protectedGitRoot, + protectedGitRoots, ); const normalized = await normalizeTarget( repo, @@ -2042,6 +2049,7 @@ export class CodexSecurity { outputDir: requestedOutput, protectedRoot, protectedGitRoot, + protectedGitRoots, gitCommand, stateDirectory, }; diff --git a/sdk/typescript/src/multiscan.ts b/sdk/typescript/src/multiscan.ts index 74c434a7a..b3a711d52 100644 --- a/sdk/typescript/src/multiscan.ts +++ b/sdk/typescript/src/multiscan.ts @@ -24,7 +24,7 @@ import { safeErrorMessage, ScanCostLimitExceededError } from "./errors.js"; import type { CoverageDocument } from "./models.js"; import { requireSecureOutputAncestry } from "./runtime.js"; import { - outermostGitMarkerRoot, + protectedGitInputRoots, resolveGitCommand, type ScanMode, } from "./targets.js"; @@ -197,29 +197,18 @@ async function runCampaign( }; } - const protectedRoots = new Set([ - await outermostGitMarkerRoot(await realpath(process.cwd()), options.signal), - checkoutRoot, - ]); - for (const task of tasks) { - if (!isAbsolute(task.repository)) continue; - const canonical = await realpath(task.repository).catch( - (error: NodeJS.ErrnoException) => { - if (error.code === "ENOENT" || error.code === "ENOTDIR") - return undefined; - throw error; - }, - ); - const root = await outermostGitMarkerRoot( - canonical ?? task.repository, - options.signal, - ); - // Missing sources remain per-task failures; keep any enclosing Git root. - if (canonical !== undefined || root !== task.repository) { - protectedRoots.add(await realpath(root)); - } - } - const gitRoots = [...protectedRoots]; + const gitRoots = await protectedGitInputRoots( + [ + process.cwd(), + options.inputPath, + ...tasks + .filter((task) => isAbsolute(task.repository)) + .map((task) => task.repository), + ...(options.knowledgeBasePaths ?? []), + ], + options.signal, + ); + gitRoots.push(checkoutRoot); let next = 0; let failed = 0; diff --git a/sdk/typescript/src/targets.ts b/sdk/typescript/src/targets.ts index c5c3fbef4..9df0ba9fd 100644 --- a/sdk/typescript/src/targets.ts +++ b/sdk/typescript/src/targets.ts @@ -532,8 +532,7 @@ export async function outermostGitMarkerRoot( await lstat(join(current, ".git")); root = current; } catch (error) { - const code = (error as NodeJS.ErrnoException).code; - if (code !== "ENOENT" && code !== "ENOTDIR") throw error; + if (!isMissingPathError(error)) throw error; } const parent = dirname(current); if (parent === current) return root; @@ -541,6 +540,49 @@ export async function outermostGitMarkerRoot( } } +/** @internal */ +export async function protectedGitInputRoots( + paths: readonly string[], + signal?: AbortSignal, +): Promise { + const roots = new Set(); + for (const requested of new Set(paths.map(resolveRepositoryPath))) { + let existing = requested; + let canonical: string; + while (true) { + throwIfAborted(signal); + try { + canonical = join( + await realpath(existing), + relative(existing, requested), + ); + break; + } catch (error) { + if (!isMissingPathError(error)) throw error; + const parent = dirname(existing); + if (parent === existing) throw error; + existing = parent; + } + } + // Both the link's repository and its resolved target can supply input data. + for (const path of new Set([requested, canonical])) { + const root = await outermostGitMarkerRoot(path, signal); + try { + roots.add(await realpath(root)); + } catch (error) { + // A missing standalone input remains the caller's per-input error. + if (!isMissingPathError(error)) throw error; + } + } + } + return [...roots]; +} + +function isMissingPathError(error: unknown): boolean { + const code = (error as NodeJS.ErrnoException).code; + return code === "ENOENT" || code === "ENOTDIR"; +} + function isolatedGitEnvironment( source: Readonly>, preserveGitConfiguration: boolean, diff --git a/sdk/typescript/tests-ts/api.test.ts b/sdk/typescript/tests-ts/api.test.ts index 8ff675579..1e151c8cf 100644 --- a/sdk/typescript/tests-ts/api.test.ts +++ b/sdk/typescript/tests-ts/api.test.ts @@ -611,6 +611,114 @@ describe("CodexSecurity orchestration", () => { await client.close(); }); + test("rejects selected Git from a knowledge-base repository before preflight", async () => { + const name = + "rejects selected Git from a knowledge-base repository before preflight"; + if (runMockInSubprocess(import.meta.path, name)) return; + + const root = await temporaryDirectory(); + const repository = join(root, "repository"); + const knowledgeRepository = join(root, "knowledge"); + const document = join(knowledgeRepository, "docs", "guide.md"); + const selected = join( + knowledgeRepository, + process.platform === "win32" ? "selected-git.exe" : "selected-git", + ); + await mkdir(repository); + await mkdir(join(knowledgeRepository, ".git"), { recursive: true }); + await mkdir(join(knowledgeRepository, "docs")); + await writeFile(document, "# Synthetic guidance\n"); + await writeFile(selected, "", { mode: 0o700 }); + const originalTargets = { ...targetsModule }; + let gitCalls = 0; + mock.module("../src/targets.js", () => ({ + ...originalTargets, + enclosingGitWorktreeRoot: async () => { + gitCalls += 1; + return null; + }, + })); + const client = new TestClient( + {}, + { + environment: { + PATH: "", + CODEX_SECURITY_GIT: selected, + CODEX_SECURITY_STATE_DIR: join(root, "state"), + }, + }, + ); + try { + await expect( + client.preflight(repository, { knowledgeBasePaths: [document] }), + ).rejects.toThrow( + "CODEX_SECURITY_GIT does not name an available executable.", + ); + expect(gitCalls).toBe(0); + } finally { + await client.close(); + mock.module("../src/targets.js", () => originalTargets); + } + }); + + test("keeps knowledge-base repositories outside runtime tool selection", async () => { + const root = await temporaryDirectory(); + const repository = join(root, "repository"); + const knowledgeRepository = join(root, "knowledge"); + const document = join(knowledgeRepository, "guide.md"); + const selected = join( + knowledgeRepository, + process.platform === "win32" ? "selected-rg.exe" : "selected-rg", + ); + const codexHome = join(root, "codex-home"); + const scanDir = join(root, "scan"); + await mkdir(repository); + await mkdir(join(knowledgeRepository, ".git"), { recursive: true }); + await mkdir(codexHome); + await mkdir(scanDir, { mode: 0o700 }); + await writeFile(document, "# Synthetic guidance\n"); + await writeFile(selected, "", { mode: 0o700 }); + const environment = { + PATH: "", + CODEX_CLI_PATH: process.execPath, + CODEX_SECURITY_GIT: "", + CODEX_SECURITY_RG: selected, + CODEX_SECURITY_STATE_DIR: join(root, "state"), + OPENAI_API_KEY: "synthetic-key", + }; + let modelStarted = false; + const client = new TestClient( + {}, + { + environment, + prepareRuntime: async () => ({ + ...preparedRuntime(codexHome), + environment, + }), + resolvePluginPython: async () => "/managed/python", + prepareOutputDir: async () => scanDir, + repositoryRevision: async () => null, + createCodex: () => { + modelStarted = true; + throw new Error("Model must not start."); + }, + }, + ); + try { + await expect( + client.run(repository, { + outputDir: scanDir, + knowledgeBasePaths: [document], + }), + ).rejects.toThrow( + "CODEX_SECURITY_RG does not name an available executable.", + ); + expect(modelStarted).toBe(false); + } finally { + await client.close(); + } + }); + test("validates knowledge-base documents before initializing the runtime", async () => { const root = await temporaryDirectory(); const repository = join(root, "repository"); @@ -5490,7 +5598,7 @@ describe("CodexSecurity orchestration", () => { process, "platform", )!; - const inspected: [string, string][] = []; + const inspected: [string, string | readonly string[]][] = []; const gitSelections: (string | null | undefined)[] = []; let gitHost: string | null = null; let host: string | null = null; @@ -5502,7 +5610,7 @@ describe("CodexSecurity orchestration", () => { inspectTrustedExecutable: async ( candidate: string, environment: Record, - protectedRoot: string, + protectedRoot: string | readonly string[], ) => { inspected.push([candidate, protectedRoot]); const sanitizedEnvironment = { ...environment, PATH: "" }; @@ -5766,8 +5874,8 @@ describe("CodexSecurity orchestration", () => { expect( inspected.filter(([candidate]) => candidate === staged), ).toEqual([ - [staged, repository], - [staged, nextRepository], + [staged, [repository]], + [staged, [nextRepository]], ]); } const expected = @@ -5861,7 +5969,7 @@ describe("CodexSecurity orchestration", () => { inspectTrustedExecutable: async ( candidate: string, current: Record, - _protectedRoot: string, + _protectedRoot: string | readonly string[], options?: { preserveInvocation?: boolean }, ) => { if (candidate !== "git" && candidate !== "rg") { diff --git a/sdk/typescript/tests-ts/multiscan.test.ts b/sdk/typescript/tests-ts/multiscan.test.ts index 30ba55ce4..874c6419c 100644 --- a/sdk/typescript/tests-ts/multiscan.test.ts +++ b/sdk/typescript/tests-ts/multiscan.test.ts @@ -18,11 +18,12 @@ import { import * as filesystem from "node:fs/promises"; import { hostname, tmpdir } from "node:os"; import { join } from "node:path"; -import { afterEach, describe, expect, spyOn, test } from "bun:test"; +import { afterEach, describe, expect, mock, spyOn, test } from "bun:test"; import { main } from "../src/cli.js"; import { ScanCostLimitExceededError } from "../src/errors.js"; import type { ScanResult } from "../src/result.js"; import { buildGitHubCredentialArgs, runMultiscan } from "../src/multiscan.js"; +import * as targetsModule from "../src/targets.js"; import { resolveTrustedExecutable } from "../src/trusted-executable.js"; import { capture, dependencies, fakeResult } from "./cli-fixtures.js"; import { runMockInSubprocess } from "./support/isolated-mock.js"; @@ -134,6 +135,96 @@ async function results(path: string): Promise[]> { } describe("multiscan", () => { + for (const location of [ + "inventory file", + "knowledge base", + "linked missing source", + ] as const) { + const name = `rejects selected Git from the ${location} repository`; + test(name, async () => { + if (runMockInSubprocess(import.meta.path, name)) return; + + const paths = await fixture(); + const source = await repository(paths.root, "source"); + const inputRepository = await repository(paths.root, "inputs"); + const document = join(inputRepository.path, "guide.md"); + const selected = join( + inputRepository.path, + process.platform === "win32" ? "selected-git.exe" : "selected-git", + ); + await writeFile(document, "# Synthetic guidance\n"); + await writeFile(selected, "", { mode: 0o700 }); + const input = + location === "inventory file" + ? join(inputRepository.path, "repositories.csv") + : paths.input; + let inventory = `id,repository,revision\nsource,${source.path},${source.revision}\n`; + if (location === "linked missing source") { + const link = join(paths.root, "input-link"); + await symlink( + join(inputRepository.path, "src"), + link, + process.platform === "win32" ? "junction" : "dir", + ); + inventory += `missing,${join(link, "missing")},${source.revision}\n`; + } + await writeFile(input, inventory); + + const originalTargets = { ...targetsModule }; + const previousGit = process.env["CODEX_SECURITY_GIT"]; + let selectedAccepted = false; + let scans = 0; + mock.module("../src/targets.js", () => ({ + ...originalTargets, + resolveGitCommand: async ( + ...args: Parameters + ) => { + const command = await originalTargets.resolveGitCommand(...args); + if (command.executable === selected) { + selectedAccepted = true; + throw new Error( + "Inert selected Git reached the execution boundary.", + ); + } + return command; + }, + })); + try { + process.env["CODEX_SECURITY_GIT"] = selected; + const summary = await runMultiscan( + options( + { ...paths, input }, + client(async (_checkout, scanOptions = {}) => { + scans += 1; + return await completedScan(scanOptions.outputDir!); + }), + { + maxAttempts: 1, + ...(location === "knowledge base" + ? { knowledgeBasePaths: [document] } + : {}), + }, + ), + ); + expect(summary).toMatchObject({ + completed: 0, + failed: location === "linked missing source" ? 2 : 1, + }); + expect(selectedAccepted).toBe(false); + expect(scans).toBe(0); + for (const receipt of await results(summary.resultsPath)) { + expect(receipt["error"]).toContain( + "CODEX_SECURITY_GIT does not name an available executable.", + ); + } + } finally { + if (previousGit === undefined) delete process.env["CODEX_SECURITY_GIT"]; + else process.env["CODEX_SECURITY_GIT"] = previousGit; + mock.module("../src/targets.js", () => originalTargets); + } + }); + } + for (const location of [ "pending inventory", "completed inventory", diff --git a/sdk/typescript/tests-ts/targets.test.ts b/sdk/typescript/tests-ts/targets.test.ts index 2f6efa7b1..93db3869f 100644 --- a/sdk/typescript/tests-ts/targets.test.ts +++ b/sdk/typescript/tests-ts/targets.test.ts @@ -23,7 +23,10 @@ import { repositoryRevision, type ScanTarget, } from "../src/index.js"; -import { outermostGitMarkerRoot } from "../src/targets.js"; +import { + outermostGitMarkerRoot, + protectedGitInputRoots, +} from "../src/targets.js"; // @ts-expect-error DiffTarget is intentionally nominal; use its constructor helpers. const structurallyInvalidTarget: ScanTarget = { @@ -78,6 +81,30 @@ test("finds outer Git roots without rejecting local source files", async () => { expect(await outermostGitMarkerRoot(outside)).toBe(outside); }); +test("protects lexical and resolved input repositories without claiming missing standalone paths", async () => { + const lexical = await repository(); + const resolved = await repository(); + const link = join(lexical, "linked-input"); + const standalone = join(dirname(lexical), "standalone.bundle"); + await symlink( + join(resolved, "src"), + link, + process.platform === "win32" ? "junction" : "dir", + ); + await writeFile(standalone, "synthetic bundle fixture\n"); + + expect(await protectedGitInputRoots([join(link, "missing")])).toEqual([ + lexical, + resolved, + ]); + expect( + await protectedGitInputRoots([join(dirname(lexical), "missing")]), + ).toEqual([]); + expect(await protectedGitInputRoots([standalone, standalone])).toEqual([ + standalone, + ]); +}); + async function createRepositoryGitShim( directory: string, marker: string, From bd9ca6c768adedb91173b48db3de695343928808 Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Tue, 18 Aug 2026 01:09:35 -0700 Subject: [PATCH 21/27] test: check shared credential lock after both scans start --- sdk/typescript/tests-ts/api.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/sdk/typescript/tests-ts/api.test.ts b/sdk/typescript/tests-ts/api.test.ts index 1e151c8cf..fe678ec88 100644 --- a/sdk/typescript/tests-ts/api.test.ts +++ b/sdk/typescript/tests-ts/api.test.ts @@ -4689,11 +4689,6 @@ describe("CodexSecurity orchestration", () => { startThread: () => ({ id: null, async runStreamed() { - expect( - existsSync( - join(credentialHome, ".codex-security-scan.lock"), - ), - ).toBe(false); if (++scansStarted === 2) releaseScans(); const credentialConfig = parseToml( await readFile( @@ -4709,6 +4704,11 @@ describe("CodexSecurity orchestration", () => { workers: index + 2, }); await concurrentScans; + expect( + existsSync( + join(credentialHome, ".codex-security-scan.lock"), + ), + ).toBe(false); const after = parseToml( await readFile(deepScanConfigPath!, "utf8"), ); From fa12ba4264cf0e6c13fb9a9399a3cd437d29ea79 Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Tue, 18 Aug 2026 01:46:05 -0700 Subject: [PATCH 22/27] fix(git): retain input roots behind dangling links --- sdk/typescript/src/targets.ts | 33 +++++++++++++- sdk/typescript/tests-ts/multiscan.test.ts | 18 ++++++-- sdk/typescript/tests-ts/targets.test.ts | 52 +++++++++++++++++++++++ 3 files changed, 98 insertions(+), 5 deletions(-) diff --git a/sdk/typescript/src/targets.ts b/sdk/typescript/src/targets.ts index 9df0ba9fd..dfb423e8b 100644 --- a/sdk/typescript/src/targets.ts +++ b/sdk/typescript/src/targets.ts @@ -1,6 +1,6 @@ import { execFile as execFileCallback } from "node:child_process"; import { existsSync } from "node:fs"; -import { lstat, realpath, stat } from "node:fs/promises"; +import { lstat, readlink, realpath, stat } from "node:fs/promises"; import { homedir } from "node:os"; import { dirname, isAbsolute, join, relative, resolve, sep } from "node:path"; import { promisify } from "node:util"; @@ -546,7 +546,36 @@ export async function protectedGitInputRoots( signal?: AbortSignal, ): Promise { const roots = new Set(); - for (const requested of new Set(paths.map(resolveRepositoryPath))) { + const pending = [...new Set(paths.map(resolveRepositoryPath))]; + const queued = new Set(pending); + for (let index = 0; index < pending.length; index += 1) { + const requested = pending[index]!; + let current = requested; + while (true) { + throwIfAborted(signal); + const parent = dirname(current); + const metadata = await lstat(current).catch((error: unknown) => { + if (isMissingPathError(error)) return null; + throw error; + }); + if (metadata?.isSymbolicLink()) { + const target = await readlink(current); + // Preserve link-target dot segments until the filesystem resolves them. + const linked = isAbsolute(target) + ? target + : `${parent}${parent.endsWith(sep) ? "" : sep}${target}`; + const suffix = relative(current, requested); + const redirected = suffix + ? `${linked}${linked.endsWith(sep) ? "" : sep}${suffix}` + : linked; + if (!queued.has(redirected)) { + queued.add(redirected); + pending.push(redirected); + } + } + if (parent === current) break; + current = parent; + } let existing = requested; let canonical: string; while (true) { diff --git a/sdk/typescript/tests-ts/multiscan.test.ts b/sdk/typescript/tests-ts/multiscan.test.ts index 874c6419c..2f13f6c73 100644 --- a/sdk/typescript/tests-ts/multiscan.test.ts +++ b/sdk/typescript/tests-ts/multiscan.test.ts @@ -139,6 +139,7 @@ describe("multiscan", () => { "inventory file", "knowledge base", "linked missing source", + "dangling linked source", ] as const) { const name = `rejects selected Git from the ${location} repository`; test(name, async () => { @@ -159,13 +160,24 @@ describe("multiscan", () => { ? join(inputRepository.path, "repositories.csv") : paths.input; let inventory = `id,repository,revision\nsource,${source.path},${source.revision}\n`; - if (location === "linked missing source") { + const missingSource = + location === "linked missing source" || + location === "dangling linked source"; + if (missingSource) { const link = join(paths.root, "input-link"); + const target = + location === "dangling linked source" + ? join(inputRepository.path, "removed-directory") + : join(inputRepository.path, "src"); + if (location === "dangling linked source") await mkdir(target); await symlink( - join(inputRepository.path, "src"), + target, link, process.platform === "win32" ? "junction" : "dir", ); + if (location === "dangling linked source") { + await rm(target, { recursive: true }); + } inventory += `missing,${join(link, "missing")},${source.revision}\n`; } await writeFile(input, inventory); @@ -208,7 +220,7 @@ describe("multiscan", () => { ); expect(summary).toMatchObject({ completed: 0, - failed: location === "linked missing source" ? 2 : 1, + failed: missingSource ? 2 : 1, }); expect(selectedAccepted).toBe(false); expect(scans).toBe(0); diff --git a/sdk/typescript/tests-ts/targets.test.ts b/sdk/typescript/tests-ts/targets.test.ts index 93db3869f..ff5ea7d19 100644 --- a/sdk/typescript/tests-ts/targets.test.ts +++ b/sdk/typescript/tests-ts/targets.test.ts @@ -103,8 +103,60 @@ test("protects lexical and resolved input repositories without claiming missing expect(await protectedGitInputRoots([standalone, standalone])).toEqual([ standalone, ]); + + const outside = join(dirname(resolved), "outside"); + const outsideFile = join(outside, "source.bundle"); + const outsideLink = join(dirname(lexical), "outside-link"); + await mkdir(outside); + await writeFile(outsideFile, "synthetic bundle fixture\n"); + await symlink( + outside, + outsideLink, + process.platform === "win32" ? "junction" : "dir", + ); + expect( + await protectedGitInputRoots([join(outsideLink, "source.bundle")]), + ).toEqual([outsideFile]); }); +test("finds input repositories behind dangling directory links", async () => { + const lexical = await repository(); + const resolved = await repository(); + const target = join(resolved, "removed-directory"); + const link = join(lexical, "linked-input"); + await mkdir(target); + await symlink( + target, + link, + process.platform === "win32" ? "junction" : "dir", + ); + await rm(target, { recursive: true }); + + expect(await protectedGitInputRoots([join(link, "missing")])).toEqual([ + lexical, + resolved, + ]); +}); + +(process.platform === "win32" ? test.skip : test)( + "preserves relative dangling-link target traversal", + async () => { + const lexical = await repository(); + const resolved = await repository(); + const target = join(resolved, "removed-directory"); + const link = join(lexical, "linked-input"); + await mkdir(target); + await symlink(join(resolved, "src"), join(lexical, "bridge"), "dir"); + await symlink("bridge/../removed-directory", link, "dir"); + await rm(target, { recursive: true }); + + expect(await protectedGitInputRoots([join(link, "missing")])).toEqual([ + lexical, + resolved, + ]); + }, +); + async function createRepositoryGitShim( directory: string, marker: string, From 6fe9d9dfeb22cc59e83ddb4cd034195a1d4f7859 Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Tue, 18 Aug 2026 02:18:00 -0700 Subject: [PATCH 23/27] fix(git): protect scan outputs and isolate cyclic inputs --- sdk/typescript/src/api.ts | 6 +- sdk/typescript/src/multiscan.ts | 2 +- sdk/typescript/src/targets.ts | 60 +++++++----- sdk/typescript/tests-ts/api.test.ts | 106 +++++++++++--------- sdk/typescript/tests-ts/multiscan.test.ts | 112 +++++++++++++--------- sdk/typescript/tests-ts/targets.test.ts | 30 ++++++ 6 files changed, 200 insertions(+), 116 deletions(-) diff --git a/sdk/typescript/src/api.ts b/sdk/typescript/src/api.ts index 534d76911..97aea04f3 100644 --- a/sdk/typescript/src/api.ts +++ b/sdk/typescript/src/api.ts @@ -1988,7 +1988,11 @@ export class CodexSecurity { validatedGitEnvironment(this.#dependencies.environment); const protectedGitRoot = await outermostGitMarkerRoot(repo, signal); const protectedGitRoots = await protectedGitInputRoots( - [repositoryPath, ...(options.knowledgeBasePaths ?? [])], + [ + repositoryPath, + ...(options.knowledgeBasePaths ?? []), + ...(options.outputDir === undefined ? [] : [options.outputDir]), + ], signal, ); const gitCommand = await resolveGitCommand( diff --git a/sdk/typescript/src/multiscan.ts b/sdk/typescript/src/multiscan.ts index b3a711d52..35d591ef0 100644 --- a/sdk/typescript/src/multiscan.ts +++ b/sdk/typescript/src/multiscan.ts @@ -208,7 +208,7 @@ async function runCampaign( ], options.signal, ); - gitRoots.push(checkoutRoot); + gitRoots.push(output); let next = 0; let failed = 0; diff --git a/sdk/typescript/src/targets.ts b/sdk/typescript/src/targets.ts index dfb423e8b..a9b40ebba 100644 --- a/sdk/typescript/src/targets.ts +++ b/sdk/typescript/src/targets.ts @@ -532,7 +532,7 @@ export async function outermostGitMarkerRoot( await lstat(join(current, ".git")); root = current; } catch (error) { - if (!isMissingPathError(error)) throw error; + if (!isUnavailablePathError(error)) throw error; } const parent = dirname(current); if (parent === current) return root; @@ -546,31 +546,43 @@ export async function protectedGitInputRoots( signal?: AbortSignal, ): Promise { const roots = new Set(); - const pending = [...new Set(paths.map(resolveRepositoryPath))]; - const queued = new Set(pending); + const queued = new Set(paths.map(resolveRepositoryPath)); + const pending = [...queued].map((path) => ({ + path, + links: new Set(), + })); for (let index = 0; index < pending.length; index += 1) { - const requested = pending[index]!; + const { path: requested, links } = pending[index]!; let current = requested; while (true) { throwIfAborted(signal); const parent = dirname(current); - const metadata = await lstat(current).catch((error: unknown) => { - if (isMissingPathError(error)) return null; - throw error; - }); + const metadata = await lstat(current, { bigint: true }).catch( + (error: unknown) => { + if (isUnavailablePathError(error)) return null; + throw error; + }, + ); if (metadata?.isSymbolicLink()) { - const target = await readlink(current); - // Preserve link-target dot segments until the filesystem resolves them. - const linked = isAbsolute(target) - ? target - : `${parent}${parent.endsWith(sep) ? "" : sep}${target}`; - const suffix = relative(current, requested); - const redirected = suffix - ? `${linked}${linked.endsWith(sep) ? "" : sep}${suffix}` - : linked; - if (!queued.has(redirected)) { - queued.add(redirected); - pending.push(redirected); + // A relative cycle can produce a new path spelling on every visit. + const identity = `${metadata.dev}:${metadata.ino}`; + if (!links.has(identity)) { + const target = await readlink(current); + // Preserve link-target dot segments until the filesystem resolves them. + const linked = isAbsolute(target) + ? target + : `${parent}${parent.endsWith(sep) ? "" : sep}${target}`; + const suffix = relative(current, requested); + const redirected = suffix + ? `${linked}${linked.endsWith(sep) ? "" : sep}${suffix}` + : linked; + if (!queued.has(redirected)) { + queued.add(redirected); + pending.push({ + path: redirected, + links: new Set([...links, identity]), + }); + } } } if (parent === current) break; @@ -587,7 +599,7 @@ export async function protectedGitInputRoots( ); break; } catch (error) { - if (!isMissingPathError(error)) throw error; + if (!isUnavailablePathError(error)) throw error; const parent = dirname(existing); if (parent === existing) throw error; existing = parent; @@ -600,16 +612,16 @@ export async function protectedGitInputRoots( roots.add(await realpath(root)); } catch (error) { // A missing standalone input remains the caller's per-input error. - if (!isMissingPathError(error)) throw error; + if (!isUnavailablePathError(error)) throw error; } } } return [...roots]; } -function isMissingPathError(error: unknown): boolean { +function isUnavailablePathError(error: unknown): boolean { const code = (error as NodeJS.ErrnoException).code; - return code === "ENOENT" || code === "ENOTDIR"; + return code === "ENOENT" || code === "ENOTDIR" || code === "ELOOP"; } function isolatedGitEnvironment( diff --git a/sdk/typescript/tests-ts/api.test.ts b/sdk/typescript/tests-ts/api.test.ts index fe678ec88..c1ef33290 100644 --- a/sdk/typescript/tests-ts/api.test.ts +++ b/sdk/typescript/tests-ts/api.test.ts @@ -611,55 +611,71 @@ describe("CodexSecurity orchestration", () => { await client.close(); }); - test("rejects selected Git from a knowledge-base repository before preflight", async () => { + for (const input of ["knowledge base", "existing output"] as const) { const name = - "rejects selected Git from a knowledge-base repository before preflight"; - if (runMockInSubprocess(import.meta.path, name)) return; + input === "knowledge base" + ? "rejects selected Git from a knowledge-base repository before preflight" + : "rejects selected Git from existing scan output before preflight"; + test(name, async () => { + if (runMockInSubprocess(import.meta.path, name)) return; - const root = await temporaryDirectory(); - const repository = join(root, "repository"); - const knowledgeRepository = join(root, "knowledge"); - const document = join(knowledgeRepository, "docs", "guide.md"); - const selected = join( - knowledgeRepository, - process.platform === "win32" ? "selected-git.exe" : "selected-git", - ); - await mkdir(repository); - await mkdir(join(knowledgeRepository, ".git"), { recursive: true }); - await mkdir(join(knowledgeRepository, "docs")); - await writeFile(document, "# Synthetic guidance\n"); - await writeFile(selected, "", { mode: 0o700 }); - const originalTargets = { ...targetsModule }; - let gitCalls = 0; - mock.module("../src/targets.js", () => ({ - ...originalTargets, - enclosingGitWorktreeRoot: async () => { - gitCalls += 1; - return null; - }, - })); - const client = new TestClient( - {}, - { - environment: { - PATH: "", - CODEX_SECURITY_GIT: selected, - CODEX_SECURITY_STATE_DIR: join(root, "state"), + const root = await temporaryDirectory(); + const repository = join(root, "repository"); + const inputDirectory = join( + root, + input === "knowledge base" ? "knowledge" : "scan", + ); + const document = join(inputDirectory, "docs", "guide.md"); + const selected = join( + inputDirectory, + process.platform === "win32" ? "selected-git.exe" : "selected-git", + ); + await mkdir(repository); + if (input === "knowledge base") { + await mkdir(join(inputDirectory, ".git"), { recursive: true }); + await mkdir(join(inputDirectory, "docs")); + await writeFile(document, "# Synthetic guidance\n"); + } else { + await mkdir(inputDirectory, { mode: 0o700 }); + } + await writeFile(selected, "", { mode: 0o700 }); + const originalTargets = { ...targetsModule }; + let gitCalls = 0; + mock.module("../src/targets.js", () => ({ + ...originalTargets, + enclosingGitWorktreeRoot: async () => { + gitCalls += 1; + return null; + }, + })); + const client = new TestClient( + {}, + { + environment: { + PATH: "", + CODEX_SECURITY_GIT: selected, + CODEX_SECURITY_STATE_DIR: join(root, "state"), + }, }, - }, - ); - try { - await expect( - client.preflight(repository, { knowledgeBasePaths: [document] }), - ).rejects.toThrow( - "CODEX_SECURITY_GIT does not name an available executable.", ); - expect(gitCalls).toBe(0); - } finally { - await client.close(); - mock.module("../src/targets.js", () => originalTargets); - } - }); + try { + await expect( + client.preflight( + repository, + input === "knowledge base" + ? { knowledgeBasePaths: [document] } + : { outputDir: inputDirectory, archiveExisting: true }, + ), + ).rejects.toThrow( + "CODEX_SECURITY_GIT does not name an available executable.", + ); + expect(gitCalls).toBe(0); + } finally { + await client.close(); + mock.module("../src/targets.js", () => originalTargets); + } + }); + } test("keeps knowledge-base repositories outside runtime tool selection", async () => { const root = await temporaryDirectory(); diff --git a/sdk/typescript/tests-ts/multiscan.test.ts b/sdk/typescript/tests-ts/multiscan.test.ts index 2f13f6c73..885f036f3 100644 --- a/sdk/typescript/tests-ts/multiscan.test.ts +++ b/sdk/typescript/tests-ts/multiscan.test.ts @@ -140,8 +140,12 @@ describe("multiscan", () => { "knowledge base", "linked missing source", "dangling linked source", + "campaign artifacts", ] as const) { - const name = `rejects selected Git from the ${location} repository`; + const name = + location === "campaign artifacts" + ? "rejects selected Git from existing campaign artifacts" + : `rejects selected Git from the ${location} repository`; test(name, async () => { if (runMockInSubprocess(import.meta.path, name)) return; @@ -149,8 +153,13 @@ describe("multiscan", () => { const source = await repository(paths.root, "source"); const inputRepository = await repository(paths.root, "inputs"); const document = join(inputRepository.path, "guide.md"); + const selectedDirectory = + location === "campaign artifacts" + ? join(paths.output, "artifacts", "prior", "attempt-1") + : inputRepository.path; + await mkdir(selectedDirectory, { recursive: true, mode: 0o700 }); const selected = join( - inputRepository.path, + selectedDirectory, process.platform === "win32" ? "selected-git.exe" : "selected-git", ); await writeFile(document, "# Synthetic guidance\n"); @@ -322,51 +331,64 @@ describe("multiscan", () => { }); } - test("keeps missing local sources as per-task failures", async () => { - const name = "keeps missing local sources as per-task failures"; - if (runMockInSubprocess(import.meta.path, name)) return; + for (const kind of ["missing", "cyclic"] as const) { + const name = `keeps ${kind} local sources as per-task failures`; + test(name, async () => { + if (runMockInSubprocess(import.meta.path, name)) return; - const paths = await fixture(); - const source = await repository(paths.root, "source"); - const trusted = await resolveTrustedExecutable( - "git", - process.env, - process.cwd(), - ); - if (trusted === null) throw new Error("Git is required by this fixture."); - await writeFile( - paths.input, - `id,repository,revision\nmissing,${join(paths.root, "missing")},${source.revision}\nsource,${source.path},${source.revision}\n`, - ); - const previousPath = process.env["PATH"]; - const previousGit = process.env["CODEX_SECURITY_GIT"]; - let scans = 0; - try { - process.env["PATH"] = ""; - process.env["CODEX_SECURITY_GIT"] = trusted.executable; - const summary = await runMultiscan( - options( - paths, - client(async (_checkout, scanOptions = {}) => { - scans += 1; - return await completedScan(scanOptions.outputDir!); - }), - { maxAttempts: 1 }, - ), + const paths = await fixture(); + const source = await repository(paths.root, "source"); + let unavailable = join(paths.root, "missing"); + if (kind === "cyclic") { + const target = join(paths.root, "cycle-target"); + const link = join(paths.root, "cycle-link"); + const type = process.platform === "win32" ? "junction" : "dir"; + await mkdir(target); + await symlink(target, link, type); + await rm(target, { recursive: true }); + await symlink(link, target, type); + unavailable = join(link, "missing"); + } + const trusted = await resolveTrustedExecutable( + "git", + process.env, + process.cwd(), ); - expect(summary).toMatchObject({ completed: 1, failed: 1 }); - expect(scans).toBe(1); - expect(await results(summary.resultsPath)).toMatchObject([ - { id: "missing", status: "failed" }, - { id: "source", status: "completed" }, - ]); - } finally { - if (previousPath === undefined) delete process.env["PATH"]; - else process.env["PATH"] = previousPath; - if (previousGit === undefined) delete process.env["CODEX_SECURITY_GIT"]; - else process.env["CODEX_SECURITY_GIT"] = previousGit; - } - }); + if (trusted === null) throw new Error("Git is required by this fixture."); + await writeFile( + paths.input, + `id,repository,revision\n${kind},${unavailable},${source.revision}\nsource,${source.path},${source.revision}\n`, + ); + const previousPath = process.env["PATH"]; + const previousGit = process.env["CODEX_SECURITY_GIT"]; + let scans = 0; + try { + process.env["PATH"] = ""; + process.env["CODEX_SECURITY_GIT"] = trusted.executable; + const summary = await runMultiscan( + options( + paths, + client(async (_checkout, scanOptions = {}) => { + scans += 1; + return await completedScan(scanOptions.outputDir!); + }), + { maxAttempts: 1 }, + ), + ); + expect(summary).toMatchObject({ completed: 1, failed: 1 }); + expect(scans).toBe(1); + expect(await results(summary.resultsPath)).toMatchObject([ + { id: kind, status: "failed" }, + { id: "source", status: "completed" }, + ]); + } finally { + if (previousPath === undefined) delete process.env["PATH"]; + else process.env["PATH"] = previousPath; + if (previousGit === undefined) delete process.env["CODEX_SECURITY_GIT"]; + else process.env["CODEX_SECURITY_GIT"] = previousGit; + } + }); + } for (const setting of ["selected", "disabled", "invalid"] as const) { const name = `honors ${setting} Git settings before bulk checkout`; diff --git a/sdk/typescript/tests-ts/targets.test.ts b/sdk/typescript/tests-ts/targets.test.ts index ff5ea7d19..03c227f15 100644 --- a/sdk/typescript/tests-ts/targets.test.ts +++ b/sdk/typescript/tests-ts/targets.test.ts @@ -138,6 +138,36 @@ test("finds input repositories behind dangling directory links", async () => { ]); }); +test("protects both repositories in a cyclic input link", async () => { + const lexical = await repository(); + const resolved = await repository(); + const left = join(lexical, "loop"); + const right = join(resolved, "loop"); + const type = process.platform === "win32" ? "junction" : "dir"; + await mkdir(right); + await symlink(right, left, type); + await rm(right, { recursive: true }); + await symlink(left, right, type); + + expect(await protectedGitInputRoots([join(left, "missing")])).toEqual([ + lexical, + resolved, + ]); +}); + +(process.platform === "win32" ? test.skip : test)( + "does not revisit a relative cyclic input link", + async () => { + const repo = await repository(); + const link = join(repo, "loop"); + await symlink("./loop/nested", link, "dir"); + + expect(await protectedGitInputRoots([join(link, "missing")])).toEqual([ + repo, + ]); + }, +); + (process.platform === "win32" ? test.skip : test)( "preserves relative dangling-link target traversal", async () => { From 24189bcdb4eea4f6ab1f3297aef24eb5ad7f4e04 Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Tue, 18 Aug 2026 02:53:08 -0700 Subject: [PATCH 24/27] fix: preserve deliberate local Git selections --- sdk/typescript/src/api.ts | 6 +- sdk/typescript/src/multiscan.ts | 2 +- sdk/typescript/tests-ts/api.test.ts | 107 ++++++++++++++++------ sdk/typescript/tests-ts/multiscan.test.ts | 8 +- 4 files changed, 85 insertions(+), 38 deletions(-) diff --git a/sdk/typescript/src/api.ts b/sdk/typescript/src/api.ts index 97aea04f3..534d76911 100644 --- a/sdk/typescript/src/api.ts +++ b/sdk/typescript/src/api.ts @@ -1988,11 +1988,7 @@ export class CodexSecurity { validatedGitEnvironment(this.#dependencies.environment); const protectedGitRoot = await outermostGitMarkerRoot(repo, signal); const protectedGitRoots = await protectedGitInputRoots( - [ - repositoryPath, - ...(options.knowledgeBasePaths ?? []), - ...(options.outputDir === undefined ? [] : [options.outputDir]), - ], + [repositoryPath, ...(options.knowledgeBasePaths ?? [])], signal, ); const gitCommand = await resolveGitCommand( diff --git a/sdk/typescript/src/multiscan.ts b/sdk/typescript/src/multiscan.ts index 35d591ef0..b3a711d52 100644 --- a/sdk/typescript/src/multiscan.ts +++ b/sdk/typescript/src/multiscan.ts @@ -208,7 +208,7 @@ async function runCampaign( ], options.signal, ); - gitRoots.push(output); + gitRoots.push(checkoutRoot); let next = 0; let failed = 0; diff --git a/sdk/typescript/tests-ts/api.test.ts b/sdk/typescript/tests-ts/api.test.ts index c1ef33290..20d664e15 100644 --- a/sdk/typescript/tests-ts/api.test.ts +++ b/sdk/typescript/tests-ts/api.test.ts @@ -611,40 +611,88 @@ describe("CodexSecurity orchestration", () => { await client.close(); }); - for (const input of ["knowledge base", "existing output"] as const) { - const name = - input === "knowledge base" - ? "rejects selected Git from a knowledge-base repository before preflight" - : "rejects selected Git from existing scan output before preflight"; + test("rejects selected Git from a knowledge-base repository before preflight", async () => { + if ( + runMockInSubprocess( + import.meta.path, + "rejects selected Git from a knowledge-base repository before preflight", + ) + ) { + return; + } + + const root = await temporaryDirectory(); + const repository = join(root, "repository"); + const knowledgeRepository = join(root, "knowledge"); + const document = join(knowledgeRepository, "docs", "guide.md"); + const selected = join( + knowledgeRepository, + process.platform === "win32" ? "selected-git.exe" : "selected-git", + ); + await mkdir(repository); + await mkdir(join(knowledgeRepository, ".git"), { recursive: true }); + await mkdir(join(knowledgeRepository, "docs")); + await writeFile(document, "# Synthetic guidance\n"); + await writeFile(selected, "", { mode: 0o700 }); + const originalTargets = { ...targetsModule }; + let gitCalls = 0; + mock.module("../src/targets.js", () => ({ + ...originalTargets, + enclosingGitWorktreeRoot: async () => { + gitCalls += 1; + return null; + }, + })); + const client = new TestClient( + {}, + { + environment: { + PATH: "", + CODEX_SECURITY_GIT: selected, + CODEX_SECURITY_STATE_DIR: join(root, "state"), + }, + }, + ); + try { + await expect( + client.preflight(repository, { knowledgeBasePaths: [document] }), + ).rejects.toThrow( + "CODEX_SECURITY_GIT does not name an available executable.", + ); + expect(gitCalls).toBe(0); + } finally { + await client.close(); + mock.module("../src/targets.js", () => originalTargets); + } + }); + + for (const output of ["explicit", "default"] as const) { + const name = `accepts selected Git from ${output} scan output before preflight`; test(name, async () => { if (runMockInSubprocess(import.meta.path, name)) return; const root = await temporaryDirectory(); const repository = join(root, "repository"); - const inputDirectory = join( - root, - input === "knowledge base" ? "knowledge" : "scan", - ); - const document = join(inputDirectory, "docs", "guide.md"); + const stateDirectory = join(root, "state"); + const outputDirectory = + output === "explicit" + ? join(root, "scan") + : join(stateDirectory, "scans", "previous"); const selected = join( - inputDirectory, + outputDirectory, process.platform === "win32" ? "selected-git.exe" : "selected-git", ); await mkdir(repository); - if (input === "knowledge base") { - await mkdir(join(inputDirectory, ".git"), { recursive: true }); - await mkdir(join(inputDirectory, "docs")); - await writeFile(document, "# Synthetic guidance\n"); - } else { - await mkdir(inputDirectory, { mode: 0o700 }); - } + await mkdir(outputDirectory, { recursive: true, mode: 0o700 }); await writeFile(selected, "", { mode: 0o700 }); const originalTargets = { ...targetsModule }; - let gitCalls = 0; + let selectedExecutable: string | null | undefined; mock.module("../src/targets.js", () => ({ ...originalTargets, - enclosingGitWorktreeRoot: async () => { - gitCalls += 1; + enclosingGitWorktreeRoot: async ( + ...args: Parameters + ) => { + selectedExecutable = args[2]?.executable; return null; }, })); @@ -654,7 +702,7 @@ describe("CodexSecurity orchestration", () => { environment: { PATH: "", CODEX_SECURITY_GIT: selected, - CODEX_SECURITY_STATE_DIR: join(root, "state"), + CODEX_SECURITY_STATE_DIR: stateDirectory, }, }, ); @@ -662,14 +710,15 @@ describe("CodexSecurity orchestration", () => { await expect( client.preflight( repository, - input === "knowledge base" - ? { knowledgeBasePaths: [document] } - : { outputDir: inputDirectory, archiveExisting: true }, + output === "explicit" + ? { outputDir: outputDirectory, archiveExisting: true } + : {}, ), - ).rejects.toThrow( - "CODEX_SECURITY_GIT does not name an available executable.", - ); - expect(gitCalls).toBe(0); + ).resolves.toMatchObject({ + repository, + outputDir: output === "explicit" ? outputDirectory : null, + }); + expect(selectedExecutable).toBe(selected); } finally { await client.close(); mock.module("../src/targets.js", () => originalTargets); diff --git a/sdk/typescript/tests-ts/multiscan.test.ts b/sdk/typescript/tests-ts/multiscan.test.ts index 885f036f3..54f29160e 100644 --- a/sdk/typescript/tests-ts/multiscan.test.ts +++ b/sdk/typescript/tests-ts/multiscan.test.ts @@ -144,7 +144,7 @@ describe("multiscan", () => { ] as const) { const name = location === "campaign artifacts" - ? "rejects selected Git from existing campaign artifacts" + ? "accepts selected Git from existing campaign artifacts" : `rejects selected Git from the ${location} repository`; test(name, async () => { if (runMockInSubprocess(import.meta.path, name)) return; @@ -231,11 +231,13 @@ describe("multiscan", () => { completed: 0, failed: missingSource ? 2 : 1, }); - expect(selectedAccepted).toBe(false); + expect(selectedAccepted).toBe(location === "campaign artifacts"); expect(scans).toBe(0); for (const receipt of await results(summary.resultsPath)) { expect(receipt["error"]).toContain( - "CODEX_SECURITY_GIT does not name an available executable.", + location === "campaign artifacts" + ? "Inert selected Git reached the execution boundary." + : "CODEX_SECURITY_GIT does not name an available executable.", ); } } finally { From 83ab7532db67f55edc6d906f343e72b6e385fe4a Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Tue, 18 Aug 2026 03:20:18 -0700 Subject: [PATCH 25/27] fix(multiscan): isolate unavailable input roots --- sdk/typescript/src/targets.ts | 4 +- sdk/typescript/tests-ts/targets.test.ts | 58 +++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 2 deletions(-) diff --git a/sdk/typescript/src/targets.ts b/sdk/typescript/src/targets.ts index a9b40ebba..6b9bfb72e 100644 --- a/sdk/typescript/src/targets.ts +++ b/sdk/typescript/src/targets.ts @@ -589,7 +589,7 @@ export async function protectedGitInputRoots( current = parent; } let existing = requested; - let canonical: string; + let canonical = requested; while (true) { throwIfAborted(signal); try { @@ -601,7 +601,7 @@ export async function protectedGitInputRoots( } catch (error) { if (!isUnavailablePathError(error)) throw error; const parent = dirname(existing); - if (parent === existing) throw error; + if (parent === existing) break; existing = parent; } } diff --git a/sdk/typescript/tests-ts/targets.test.ts b/sdk/typescript/tests-ts/targets.test.ts index 03c227f15..c5f2c80e8 100644 --- a/sdk/typescript/tests-ts/targets.test.ts +++ b/sdk/typescript/tests-ts/targets.test.ts @@ -119,6 +119,64 @@ test("protects lexical and resolved input repositories without claiming missing ).toEqual([outsideFile]); }); +test("keeps unavailable filesystem roots out of shared input roots", async () => { + const repo = await repository(); + const script = ` + import { mock } from "bun:test"; + import * as original from "node:fs/promises"; + import { dirname, join } from "node:path"; + const [repo, targets] = process.argv.slice(1); + const missing = join(dirname(repo), "unavailable-root", "repo"); + const unavailable = new Set(); + for (let path = missing; ; path = dirname(path)) { + unavailable.add(path); + if (dirname(path) === path) break; + } + const originalPromises = { ...original }; + let errorCode = "ENOENT"; + mock.module("node:fs/promises", () => ({ + ...originalPromises, + realpath: async (path, ...args) => { + if (unavailable.has(path)) { + throw Object.assign(new Error("synthetic unavailable root"), { + code: errorCode, + }); + } + return await originalPromises.realpath(path, ...args); + }, + })); + try { + const { protectedGitInputRoots } = await import(targets); + const roots = await protectedGitInputRoots([missing, repo]); + errorCode = "EACCES"; + let unexpectedError = null; + try { + await protectedGitInputRoots([missing]); + } catch (error) { + unexpectedError = error.code; + } + console.log(JSON.stringify({ roots, unexpectedError })); + } finally { + mock.module("node:fs/promises", () => originalPromises); + } + `; + const result = spawnSync( + process.execPath, + [ + "-e", + script, + repo, + fileURLToPath(new URL("../src/targets.ts", import.meta.url)), + ], + { encoding: "utf8" }, + ); + expect(result.status, result.stderr).toBe(0); + expect(JSON.parse(result.stdout)).toEqual({ + roots: [repo], + unexpectedError: "EACCES", + }); +}); + test("finds input repositories behind dangling directory links", async () => { const lexical = await repository(); const resolved = await repository(); From 76b9e906611b3e9707d4e4b83f0e2bfad81419e7 Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Tue, 18 Aug 2026 03:46:45 -0700 Subject: [PATCH 26/27] fix(git): handle host path compatibility --- .../_bundled_plugin/.codex-plugin/plugin.json | 2 +- .../scripts/workbench_target.py | 14 ++-- sdk/typescript/src/api.ts | 3 +- sdk/typescript/src/targets.ts | 3 +- sdk/typescript/src/trusted-executable.ts | 10 ++- sdk/typescript/src/version.ts | 2 +- sdk/typescript/tests-ts/api.test.ts | 14 ++-- .../tests-ts/trusted-executable.test.ts | 60 ++++++++++++++++ .../tests-ts/workbench-trusted-git.test.ts | 69 +++++++++++++++++++ 9 files changed, 161 insertions(+), 16 deletions(-) diff --git a/sdk/typescript/_bundled_plugin/.codex-plugin/plugin.json b/sdk/typescript/_bundled_plugin/.codex-plugin/plugin.json index 76688fe96..05e5e355d 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.29", "description": "Codex Security workflows for security scans, analysis, and investigation.", "author": { "name": "OpenAI" diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_target.py b/sdk/typescript/_bundled_plugin/scripts/workbench_target.py index 3992dfebc..20ae116ef 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_target.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_target.py @@ -3,6 +3,7 @@ from __future__ import annotations import argparse +import errno import hashlib import os import shutil @@ -132,7 +133,12 @@ def _protected_git_root(target: Path) -> Path | None: except FileNotFoundError: continue root = ancestor - except (FileNotFoundError, NotADirectoryError): + except (FileNotFoundError, NotADirectoryError, RuntimeError): + # pathlib raised RuntimeError for symlink loops before Python 3.13. + return None + except OSError as error: + if error.errno != errno.ELOOP: + raise return None return root @@ -184,7 +190,7 @@ def _trusted_executable( for ancestor in candidate.parents ): raise SystemExit(f"{setting} must stay outside the protected repository.") - except OSError as error: + except (OSError, RuntimeError) as error: raise SystemExit(f"{setting} does not name an available executable.") from error if not _is_native_executable(candidate, canonical): raise SystemExit(f"{setting} does not name an available executable.") @@ -209,7 +215,7 @@ def _trusted_executable( directory = Path(entry).resolve(strict=True) if _inside_protected_git_root(directory, root): continue - except OSError: + except (OSError, RuntimeError): continue candidate: str | None = None safe = True @@ -220,7 +226,7 @@ def _trusted_executable( if _inside_protected_git_root(canonical, root): safe = False break - except OSError: + except (OSError, RuntimeError): continue if _is_native_executable(path, canonical): candidate = candidate or str(path) diff --git a/sdk/typescript/src/api.ts b/sdk/typescript/src/api.ts index 534d76911..1322d2e5d 100644 --- a/sdk/typescript/src/api.ts +++ b/sdk/typescript/src/api.ts @@ -141,6 +141,7 @@ import { import { executableBinding, inspectTrustedExecutable, + isAbsoluteExecutablePath, type InspectedExecutable, } from "./trusted-executable.js"; @@ -591,7 +592,7 @@ export class CodexSecurity { protectedGitRoots, ); if (configuredRipgrep !== undefined && configuredRipgrep !== "") { - if (!isAbsolute(configuredRipgrep)) { + if (!isAbsoluteExecutablePath(configuredRipgrep)) { throw new ConfigurationError( "CODEX_SECURITY_RG must name an absolute trusted executable.", ); diff --git a/sdk/typescript/src/targets.ts b/sdk/typescript/src/targets.ts index 6b9bfb72e..a25bde2d1 100644 --- a/sdk/typescript/src/targets.ts +++ b/sdk/typescript/src/targets.ts @@ -8,6 +8,7 @@ import { ConfigurationError, InvalidTargetError } from "./errors.js"; import { executableBinding, inspectTrustedExecutable, + isAbsoluteExecutablePath, type InspectedExecutable, } from "./trusted-executable.js"; @@ -496,7 +497,7 @@ export async function resolveGitCommand( if (binding.value === "") { executable = null; } else if (binding.value !== undefined) { - if (!isAbsolute(binding.value)) { + if (!isAbsoluteExecutablePath(binding.value)) { throw new ConfigurationError( "CODEX_SECURITY_GIT must name an absolute trusted executable.", ); diff --git a/sdk/typescript/src/trusted-executable.ts b/sdk/typescript/src/trusted-executable.ts index 152cf98b4..978f17851 100644 --- a/sdk/typescript/src/trusted-executable.ts +++ b/sdk/typescript/src/trusted-executable.ts @@ -6,9 +6,11 @@ import { extname, isAbsolute, join, + posix, relative, resolve, sep, + win32, } from "node:path"; export interface TrustedExecutable { @@ -21,6 +23,12 @@ export interface InspectedExecutable { environment: Record; } +export function isAbsoluteExecutablePath(candidate: string): boolean { + if (process.platform !== "win32") return posix.isAbsolute(candidate); + // A Windows root-relative path still depends on the current drive. + return win32.isAbsolute(candidate) && win32.parse(candidate).root.length > 1; +} + export function executableBinding( environment: Readonly>, setting: "CODEX_SECURITY_GIT" | "CODEX_SECURITY_RG", @@ -141,7 +149,7 @@ export async function inspectTrustedExecutable( try { if ( preserveInvocation && - (!isAbsolute(candidate) || + (!isAbsoluteExecutablePath(candidate) || (await hasProtectedAncestor(roots, current.path, canonical))) ) { continue; diff --git a/sdk/typescript/src/version.ts b/sdk/typescript/src/version.ts index 1bf429fb8..e9aea86c7 100644 --- a/sdk/typescript/src/version.ts +++ b/sdk/typescript/src/version.ts @@ -8,7 +8,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.29" as const; const PACKAGE_NAME = "@openai/codex-security"; const VERSION_PATTERN = diff --git a/sdk/typescript/tests-ts/api.test.ts b/sdk/typescript/tests-ts/api.test.ts index 20d664e15..fc98d405f 100644 --- a/sdk/typescript/tests-ts/api.test.ts +++ b/sdk/typescript/tests-ts/api.test.ts @@ -17,7 +17,7 @@ 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 { basename, join, win32 } from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; import { Codex, type CodexOptions, type ThreadEvent } from "@openai/codex-sdk"; import { afterEach, describe, expect, mock, test } from "bun:test"; @@ -5850,18 +5850,18 @@ describe("CodexSecurity orchestration", () => { await mkdir(path, { mode: 0o700 }); } const filename = process.platform === "win32" ? "rg.exe" : "rg"; + const hostTool = (name: string): string => + process.platform === "win32" + ? win32.join("C:\\host-tools", name) + : join(root, "host-tools", name); gitHost = entry.gitAvailable - ? join( - root, - "host-tools", - process.platform === "win32" ? "git.exe" : "git", - ) + ? hostTool(process.platform === "win32" ? "git.exe" : "git") : null; const staged = scenario === "rejected-copy" ? join(repository, filename) : join(workspace, filename); - host = entry.host ? join(root, "host-tools", filename) : null; + host = entry.host ? hostTool(filename) : null; rejected = scenario === "rejected-copy" ? staged : null; inspected.length = 0; gitSelections.length = 0; diff --git a/sdk/typescript/tests-ts/trusted-executable.test.ts b/sdk/typescript/tests-ts/trusted-executable.test.ts index 9f3a82861..335c2a948 100644 --- a/sdk/typescript/tests-ts/trusted-executable.test.ts +++ b/sdk/typescript/tests-ts/trusted-executable.test.ts @@ -84,6 +84,66 @@ async function resolveWindowsExecutable( } describe("trusted executable resolution", () => { + test("requires fully qualified Windows paths for explicit tool bindings", () => { + const cases: [string, string | null][] = [ + [String.raw`\tools\git.exe`, null], + ["/tools/git.exe", null], + [String.raw`C:tools\git.exe`, null], + [String.raw`\\server`, null], + [String.raw`C:\tools\git.exe`, String.raw`C:\tools\git.exe`], + ["C:/tools/git.exe", "C:/tools/git.exe"], + [String.raw`\\server\share\git.exe`, String.raw`\\server\share\git.exe`], + ["//server/share/git.exe", "//server/share/git.exe"], + [String.raw`\\?\C:\tools\git.exe`, String.raw`\\?\C:\tools\git.exe`], + ]; + const script = ` + import { mock } from "bun:test"; + import * as originalPromises from "node:fs/promises"; + import * as originalPath from "node:path"; + const [modulePath, values, repository] = process.argv.slice(1); + const windows = originalPath.win32; + const canonical = (path) => windows.resolve(windows.parse(repository).root, path); + const identities = new Map(); + const identity = (path) => { + const key = canonical(path).toLowerCase(); + if (!identities.has(key)) identities.set(key, BigInt(identities.size + 1)); + return identities.get(key); + }; + Object.defineProperty(process, "platform", { value: "win32" }); + mock.module("node:path", () => ({ ...originalPath, ...windows })); + mock.module("node:fs/promises", () => ({ + ...originalPromises, + realpath: async (path) => canonical(path), + access: async () => {}, + stat: async (path) => ({ dev: 1n, ino: identity(path), isFile: () => true }), + })); + const { inspectTrustedExecutable } = await import(modulePath); + const results = []; + for (const value of JSON.parse(values)) { + const result = await inspectTrustedExecutable(value, { PATH: "" }, repository, { + preserveInvocation: true, + }); + results.push(result.executable); + } + console.log(JSON.stringify(results)); + `; + const result = spawnSync( + process.execPath, + [ + "-e", + script, + fileURLToPath(new URL("../src/trusted-executable.ts", import.meta.url)), + JSON.stringify(cases.map(([value]) => value)), + String.raw`C:\repository`, + ], + { encoding: "utf8" }, + ); + expect(result.status, result.stderr).toBe(0); + expect(JSON.parse(result.stdout)).toEqual( + cases.map(([, executable]) => executable), + ); + }); + test("excludes every protected root before selecting an executable", async () => { const root = await temporaryDirectory(); const repositories = [join(root, "working"), join(root, "source")]; diff --git a/sdk/typescript/tests-ts/workbench-trusted-git.test.ts b/sdk/typescript/tests-ts/workbench-trusted-git.test.ts index 736df477d..b41184171 100644 --- a/sdk/typescript/tests-ts/workbench-trusted-git.test.ts +++ b/sdk/typescript/tests-ts/workbench-trusted-git.test.ts @@ -210,6 +210,75 @@ describe("bundled workbench trusted Git", () => { expect(JSON.parse(result.stdout)).toEqual({ status: 127, output: "" }); }); + test("treats cyclic saved targets as unavailable Git probes", () => { + const target = fixture(); + const result = probe( + target, + [ + "import errno, workbench_target", + "from unittest.mock import patch", + "statuses = []", + "for failure in (RuntimeError('synthetic symlink loop'), OSError(errno.ELOOP, 'synthetic symlink loop')):", + " with patch.object(Path, 'resolve', side_effect=failure), patch.object(workbench_target.subprocess, 'run', side_effect=AssertionError('unexpected Git execution')):", + " statuses.append(git_command(Path(sys.argv[2]), 'status', text=True).returncode)", + "permission_error = False", + "try:", + " with patch.object(Path, 'resolve', side_effect=PermissionError(errno.EACCES, 'synthetic permission error')):", + " git_command(Path(sys.argv[2]), 'status', text=True)", + "except PermissionError:", + " permission_error = True", + "print(json.dumps({'statuses': statuses, 'permissionError': permission_error}))", + ], + { environment: { CODEX_SECURITY_GIT: target.git } }, + ); + expect(result.status, result.stderr).toBe(0); + expect(JSON.parse(result.stdout)).toEqual({ + statuses: [127, 127], + permissionError: true, + }); + }); + + test("handles cyclic configured executables and PATH entries", () => { + const target = fixture(); + const result = probe(target, [ + "import errno, workbench_target", + "from unittest.mock import patch", + "repository = Path(sys.argv[2])", + "directory = repository.parent", + "loop = directory / ('git.exe' if sys.platform == 'win32' else 'git')", + "original_resolve = Path.resolve", + "outcomes = []", + "for failure in (RuntimeError('synthetic symlink loop'), OSError(errno.ELOOP, 'synthetic symlink loop')):", + " def resolve(path, *args, **kwargs):", + " if path == loop:", + " raise failure", + " return original_resolve(path, *args, **kwargs)", + " with patch.object(Path, 'resolve', resolve):", + " configured = None", + " try:", + " workbench_target._trusted_git_executable(repository, {'CODEX_SECURITY_GIT': str(loop), 'PATH': ''})", + " except SystemExit as error:", + " configured = str(error)", + " from_path = workbench_target._trusted_git_executable(repository, {'PATH': str(directory)})", + " def resolve_directory(path, *args, **kwargs):", + " if path == directory:", + " raise failure", + " return original_resolve(path, *args, **kwargs)", + " with patch.object(Path, 'resolve', resolve_directory):", + " from_directory = workbench_target._trusted_git_executable(repository, {'PATH': str(directory)})", + " outcomes.append({'configured': configured, 'path': from_path, 'directory': from_directory})", + "print(json.dumps(outcomes))", + ]); + expect(result.status, result.stderr).toBe(0); + expect(JSON.parse(result.stdout)).toEqual( + Array.from({ length: 2 }, () => ({ + configured: "CODEX_SECURITY_GIT does not name an available executable.", + path: null, + directory: null, + })), + ); + }); + test("uses default lookup only when PATH is absent", async () => { const target = fixture(); const defaultPath = From 055269586a148c0f0fc1fa08025deeb00ef3f434 Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Tue, 18 Aug 2026 04:50:13 -0700 Subject: [PATCH 27/27] fix: align tool locations with scan inputs --- sdk/typescript/src/api.ts | 10 ++-- sdk/typescript/src/multiscan.ts | 1 - sdk/typescript/tests-ts/api.test.ts | 42 +++++++++++++++-- sdk/typescript/tests-ts/multiscan.test.ts | 57 +++++++++++++++++++---- 4 files changed, 90 insertions(+), 20 deletions(-) diff --git a/sdk/typescript/src/api.ts b/sdk/typescript/src/api.ts index 1322d2e5d..c02d8eec4 100644 --- a/sdk/typescript/src/api.ts +++ b/sdk/typescript/src/api.ts @@ -109,6 +109,7 @@ import { prepareOutputDir, preparePersistentOutputRoot, requireModelSafeOutputDir, + requireOutputOutsideRepositories, requireOutputOutsideRepository, resolveCodexCommand, resolvePluginPath, @@ -309,7 +310,6 @@ export interface ScanPreflight extends DeepScanOptions { interface LocalScanInputs extends Omit { protectedRoot: string; - protectedGitRoot: string; protectedGitRoots: readonly string[]; gitCommand: InspectedExecutable; stateDirectory: string; @@ -509,7 +509,6 @@ export class CodexSecurity { mode, outputDir: requestedOutput, protectedRoot, - protectedGitRoot, protectedGitRoots, gitCommand, stateDirectory, @@ -616,7 +615,11 @@ export class CodexSecurity { runtime.bootstrapWorkspace !== undefined ) { const workspace = await realpath(runtime.bootstrapWorkspace); - requireOutputOutsideRepository(protectedGitRoot, workspace, "runtime"); + requireOutputOutsideRepositories( + protectedGitRoots, + workspace, + "runtime", + ); if (runtime.bundledRipgrep === undefined) { const bundled = await ( this.#dependencies.stageBundledRipgrep ?? stageBundledRipgrep @@ -2049,7 +2052,6 @@ export class CodexSecurity { mode, outputDir: requestedOutput, protectedRoot, - protectedGitRoot, protectedGitRoots, gitCommand, stateDirectory, diff --git a/sdk/typescript/src/multiscan.ts b/sdk/typescript/src/multiscan.ts index b3a711d52..044e51c7e 100644 --- a/sdk/typescript/src/multiscan.ts +++ b/sdk/typescript/src/multiscan.ts @@ -199,7 +199,6 @@ async function runCampaign( const gitRoots = await protectedGitInputRoots( [ - process.cwd(), options.inputPath, ...tasks .filter((task) => isAbsolute(task.repository)) diff --git a/sdk/typescript/tests-ts/api.test.ts b/sdk/typescript/tests-ts/api.test.ts index fc98d405f..ec853afb6 100644 --- a/sdk/typescript/tests-ts/api.test.ts +++ b/sdk/typescript/tests-ts/api.test.ts @@ -5740,6 +5740,11 @@ describe("CodexSecurity orchestration", () => { }, { scenario: "rejected-copy", host: false, expected: "missing" }, { scenario: "overlapping-workspace", host: false, expected: "missing" }, + { + scenario: "overlapping-knowledge-base", + host: false, + expected: "missing", + }, { scenario: "case-distinct POSIX binding", platform: "linux", @@ -5836,10 +5841,26 @@ describe("CodexSecurity orchestration", () => { const nextRepository = join(root, "next-repository"); const codexHome = join(root, "codex-home"); const scanDir = join(root, "scan"); + const knowledgeBaseDirectory = join(root, "knowledge-base"); + const knowledgeBasePaths = + scenario === "overlapping-knowledge-base" + ? [join(knowledgeBaseDirectory, "guide.md")] + : undefined; + if (knowledgeBasePaths !== undefined) { + await mkdir(join(knowledgeBaseDirectory, ".git"), { + recursive: true, + mode: 0o700, + }); + await writeFile(knowledgeBasePaths[0]!, "# Synthetic guidance\n"); + } + const scanOptions = + knowledgeBasePaths === undefined ? {} : { knowledgeBasePaths }; const workspace = scenario === "overlapping-workspace" ? repository - : join(root, "bootstrap-workspace"); + : scenario === "overlapping-knowledge-base" + ? join(knowledgeBaseDirectory, "bootstrap-workspace") + : join(root, "bootstrap-workspace"); for (const path of new Set([ repository, nextRepository, @@ -5917,19 +5938,30 @@ describe("CodexSecurity orchestration", () => { }, ); try { - await client.preflight(repository); + await client.preflight(repository, scanOptions); expect(gitSelections).toEqual([ entry.expectedGit === "host" ? gitHost : null, ]); - if (scenario === "overlapping-workspace") { - await expect(client.run(repository)).rejects.toBeInstanceOf( + if ( + scenario === "overlapping-workspace" || + scenario === "overlapping-knowledge-base" + ) { + const failure = client.run(repository, scanOptions); + await expect(failure).rejects.toBeInstanceOf( OutputInsideProtectedRootError, ); + await expect(failure).rejects.toMatchObject({ + protectedRoot: + scenario === "overlapping-workspace" + ? repository + : knowledgeBaseDirectory, + pathKind: "runtime", + }); expect(stageCalls).toEqual([]); expect(codexEnvironments).toEqual([]); continue; } - await expect(client.run(repository)).rejects.toThrow( + await expect(client.run(repository, scanOptions)).rejects.toThrow( "captured tool environment", ); if (scenario === "bundled") { diff --git a/sdk/typescript/tests-ts/multiscan.test.ts b/sdk/typescript/tests-ts/multiscan.test.ts index 54f29160e..522e15bb6 100644 --- a/sdk/typescript/tests-ts/multiscan.test.ts +++ b/sdk/typescript/tests-ts/multiscan.test.ts @@ -1,4 +1,4 @@ -import { execFileSync } from "node:child_process"; +import { execFileSync, spawnSync } from "node:child_process"; import { access, appendFile, @@ -141,12 +141,49 @@ describe("multiscan", () => { "linked missing source", "dangling linked source", "campaign artifacts", + "launch directory", ] as const) { + const acceptsSelected = + location === "campaign artifacts" || location === "launch directory"; const name = location === "campaign artifacts" ? "accepts selected Git from existing campaign artifacts" - : `rejects selected Git from the ${location} repository`; + : location === "launch directory" + ? "accepts selected Git from an unrelated launch directory" + : `rejects selected Git from the ${location} repository`; test(name, async () => { + if (location === "launch directory") { + const launchDirectory = process.env["CODEX_SECURITY_TEST_LAUNCH_CWD"]; + if (launchDirectory === undefined) { + const launchFixture = await fixture(); + const launch = await repository(launchFixture.root, "launch"); + const execution = spawnSync( + process.execPath, + [ + "--no-env-file", + "test", + "--timeout", + "30000", + "--test-name-pattern", + name, + import.meta.path, + ], + { + cwd: launch.path, + encoding: "utf8", + env: { + ...process.env, + CODEX_SECURITY_ISOLATED_MOCK: "1", + CODEX_SECURITY_TEST_LAUNCH_CWD: launch.path, + }, + windowsHide: true, + }, + ); + expect(execution.status, execution.stderr).toBe(0); + return; + } + expect(await realpath(process.cwd())).toBe(launchDirectory); + } if (runMockInSubprocess(import.meta.path, name)) return; const paths = await fixture(); @@ -156,7 +193,9 @@ describe("multiscan", () => { const selectedDirectory = location === "campaign artifacts" ? join(paths.output, "artifacts", "prior", "attempt-1") - : inputRepository.path; + : location === "launch directory" + ? join(process.cwd(), "tools") + : inputRepository.path; await mkdir(selectedDirectory, { recursive: true, mode: 0o700 }); const selected = join( selectedDirectory, @@ -221,7 +260,8 @@ describe("multiscan", () => { }), { maxAttempts: 1, - ...(location === "knowledge base" + ...(location === "knowledge base" || + location === "launch directory" ? { knowledgeBasePaths: [document] } : {}), }, @@ -231,11 +271,11 @@ describe("multiscan", () => { completed: 0, failed: missingSource ? 2 : 1, }); - expect(selectedAccepted).toBe(location === "campaign artifacts"); + expect(selectedAccepted).toBe(acceptsSelected); expect(scans).toBe(0); for (const receipt of await results(summary.resultsPath)) { expect(receipt["error"]).toContain( - location === "campaign artifacts" + acceptsSelected ? "Inert selected Git reached the execution boundary." : "CODEX_SECURITY_GIT does not name an available executable.", ); @@ -1869,7 +1909,7 @@ describe("multiscan", () => { paths.input, `id,repository,revision\nprivate,${source.path},${source.revision}\n`, ); - const shimDirectory = join(paths.root, "node_modules", ".bin"); + const shimDirectory = join(source.path, "node_modules", ".bin"); const leakedCredential = join(paths.root, "leaked-credential"); await mkdir(shimDirectory, { recursive: true }); await writeFile( @@ -1877,7 +1917,6 @@ describe("multiscan", () => { `#!/bin/sh\nprintf '%s' "$GIT_CONFIG_VALUE_0" > "${leakedCredential}"\nexit 1\n`, { mode: 0o700 }, ); - const previousDirectory = process.cwd(); const environment = new Map( [ "PATH", @@ -1888,7 +1927,6 @@ describe("multiscan", () => { ); try { - process.chdir(paths.root); process.env["PATH"] = `${shimDirectory}${process.platform === "win32" ? ";" : ":"}${environment.get("PATH") ?? ""}`; process.env["GIT_CONFIG_COUNT"] = "1"; @@ -1925,7 +1963,6 @@ describe("multiscan", () => { expect(summary).toMatchObject({ completed: 1, failed: 0 }); await expect(access(leakedCredential)).rejects.toThrow(); } finally { - process.chdir(previousDirectory); for (const [name, value] of environment) { if (value === undefined) delete process.env[name]; else process.env[name] = value;