diff --git a/packages/adapters/codex-local/src/server/codex-home.test.ts b/packages/adapters/codex-local/src/server/codex-home.test.ts index 92e0a1ec0d..81f1de80d9 100644 --- a/packages/adapters/codex-local/src/server/codex-home.test.ts +++ b/packages/adapters/codex-local/src/server/codex-home.test.ts @@ -3,6 +3,7 @@ import os from "node:os"; import path from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; import { + CODEX_SYNC_ALLOWLIST, codexHomeHasUsableAuth, ensureSymlink, evaluateCodexCredentialReadiness, @@ -11,6 +12,7 @@ import { prepareManagedCodexHome, reconcileManagedCodexHome, seedManagedCodexHome, + stageCodexHomeForSync, writeManagedCodexMcpConfig, } from "./codex-home.js"; @@ -766,3 +768,363 @@ describe("evaluateCodexCredentialReadiness", () => { } }); }); + +describe("stageCodexHomeForSync", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + // Builds a fake managed CODEX_HOME containing the full allowlist (with + // `auth.json` as a symlink into a separate source-bytes file and a populated + // `skills/` symlink, mirroring the real managed home) plus decoy runtime + // state the allowlist must NOT copy. + async function buildFakeHome(root: string): Promise<{ home: string; authBytes: string; skillBytes: string }> { + const home = path.join(root, "codex-home"); + const authSource = path.join(root, "shared", "auth.json"); + const skillSource = path.join(root, "shared", "skill-src.md"); + const authBytes = '{"tokens":{"account_id":"acct","refresh_token":"r"}}\n'; + const skillBytes = "# injected skill\n"; + + await fs.mkdir(path.join(root, "shared"), { recursive: true }); + await fs.writeFile(authSource, authBytes, "utf8"); + await fs.writeFile(skillSource, skillBytes, "utf8"); + + await fs.mkdir(home, { recursive: true }); + // auth.json is a symlink into the shared source (single-use rotating tokens). + await fs.symlink(authSource, path.join(home, "auth.json")); + await fs.writeFile(path.join(home, "config.toml"), "model_provider = \"paperclip\"\n", "utf8"); + await fs.writeFile(path.join(home, "config.json"), "{}\n", "utf8"); + await fs.writeFile(path.join(home, "instructions.md"), "hi\n", "utf8"); + // skills/ is a directory of symlinks. + await fs.mkdir(path.join(home, "skills"), { recursive: true }); + await fs.symlink(skillSource, path.join(home, "skills", "demo.md")); + + // Decoys: large runtime state the 4-name denylist missed. + await fs.writeFile(path.join(home, "logs_2.sqlite"), "x", "utf8"); + await fs.writeFile(path.join(home, "state_5.sqlite"), "x", "utf8"); + await fs.mkdir(path.join(home, "plugins", "cache"), { recursive: true }); + await fs.writeFile(path.join(home, "plugins", "cache", "x"), "x", "utf8"); + await fs.mkdir(path.join(home, "sessions"), { recursive: true }); + await fs.writeFile(path.join(home, "sessions", "y"), "x", "utf8"); + await fs.mkdir(path.join(home, "tmp"), { recursive: true }); + await fs.symlink("/usr/bin/env", path.join(home, "tmp", "arg0")); + + return { home, authBytes, skillBytes }; + } + + it("stages exactly the allowlist, derefs auth.json to bytes, and excludes decoys", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-codex-stage-")); + let staged: string | null = null; + try { + const { home, authBytes, skillBytes } = await buildFakeHome(root); + staged = await stageCodexHomeForSync(home, { runId: "run-1" }); + + const entries = (await fs.readdir(staged)).sort(); + expect(entries).toEqual([...CODEX_SYNC_ALLOWLIST].sort()); + + // Decoys must be absent. + for (const decoy of ["logs_2.sqlite", "state_5.sqlite", "plugins", "sessions", "tmp"]) { + expect(entries).not.toContain(decoy); + } + + // auth.json is a regular file (symlink dereferenced) whose bytes equal the target. + const stagedAuth = path.join(staged, "auth.json"); + expect((await fs.lstat(stagedAuth)).isSymbolicLink()).toBe(false); + expect(await fs.readFile(stagedAuth, "utf8")).toBe(authBytes); + + // skills/ copied recursively with the symlink dereferenced to bytes. + const stagedSkill = path.join(staged, "skills", "demo.md"); + expect((await fs.lstat(stagedSkill)).isSymbolicLink()).toBe(false); + expect(await fs.readFile(stagedSkill, "utf8")).toBe(skillBytes); + + // config.toml (post-rewrite state) carried through. + expect(await fs.readFile(path.join(staged, "config.toml"), "utf8")).toContain("model_provider"); + } finally { + if (staged) await fs.rm(staged, { recursive: true, force: true }); + await fs.rm(root, { recursive: true, force: true }); + } + }); + + // C1 — staged credential file must be mode 0600 (not the world-readable default). + it("writes the staged auth.json with mode 0600", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-codex-stage-mode-")); + let staged: string | null = null; + try { + const { home } = await buildFakeHome(root); + staged = await stageCodexHomeForSync(home, { runId: "run-mode" }); + const mode = (await fs.stat(path.join(staged, "auth.json"))).mode & 0o777; + expect(mode).toBe(0o600); + } finally { + if (staged) await fs.rm(staged, { recursive: true, force: true }); + await fs.rm(root, { recursive: true, force: true }); + } + }); + + // config.toml carries the managed MCP `Authorization: Bearer …` header and is + // secret-bearing; the staged copy must be 0600, not the world-readable default. + it("writes the staged config.toml (managed MCP bearer header) with mode 0600", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-codex-stage-toml-mode-")); + let staged: string | null = null; + try { + const home = path.join(root, "codex-home"); + await fs.mkdir(home, { recursive: true }); + // Mirror the source writer: config.toml holds an MCP gateway bearer token + // and is persisted 0600 on disk. + await fs.writeFile( + path.join(home, "config.toml"), + "[mcp_servers.paperclip]\nheaders = { Authorization = \"Bearer secret-token\" }\n", + { mode: 0o600 }, + ); + staged = await stageCodexHomeForSync(home, { runId: "run-toml-mode" }); + const mode = (await fs.stat(path.join(staged, "config.toml"))).mode & 0o777; + expect(mode).toBe(0o600); + } finally { + if (staged) await fs.rm(staged, { recursive: true, force: true }); + await fs.rm(root, { recursive: true, force: true }); + } + }); + + // Least privilege: no staged regular file needs group/other read, so every + // one (config.json, instructions.md — not just credentials) is staged 0600. + it("writes every staged regular file with mode 0600", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-codex-stage-all-mode-")); + let staged: string | null = null; + try { + const { home } = await buildFakeHome(root); + staged = await stageCodexHomeForSync(home, { runId: "run-all-mode" }); + for (const entry of ["auth.json", "config.toml", "config.json", "instructions.md"]) { + const mode = (await fs.stat(path.join(staged, entry))).mode & 0o777; + expect(mode, `${entry} should be staged 0600`).toBe(0o600); + } + } finally { + if (staged) await fs.rm(staged, { recursive: true, force: true }); + await fs.rm(root, { recursive: true, force: true }); + } + }); + + // C2 — staged dir must be 0700 (mkdtemp guarantees this on POSIX). + it("creates the staged dir with mode 0700", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-codex-stage-dir-")); + let staged: string | null = null; + try { + const { home } = await buildFakeHome(root); + staged = await stageCodexHomeForSync(home, { runId: "run-dir" }); + const mode = (await fs.stat(staged)).mode & 0o777; + expect(mode).toBe(0o700); + } finally { + if (staged) await fs.rm(staged, { recursive: true, force: true }); + await fs.rm(root, { recursive: true, force: true }); + } + }); + + it("skips absent optional entries without throwing", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-codex-stage-absent-")); + let staged: string | null = null; + try { + // Keyring-credential mode: no auth.json, no config.json. + const home = path.join(root, "codex-home"); + await fs.mkdir(home, { recursive: true }); + await fs.writeFile(path.join(home, "config.toml"), "x\n", "utf8"); + + staged = await stageCodexHomeForSync(home, { runId: "run-absent" }); + const entries = await fs.readdir(staged); + expect(entries).toEqual(["config.toml"]); + } finally { + if (staged) await fs.rm(staged, { recursive: true, force: true }); + await fs.rm(root, { recursive: true, force: true }); + } + }); + + it("treats a dangling auth.json symlink as absent (skips it, no throw)", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-codex-stage-dangling-")); + let staged: string | null = null; + try { + const home = path.join(root, "codex-home"); + await fs.mkdir(home, { recursive: true }); + await fs.symlink(path.join(root, "gone", "auth.json"), path.join(home, "auth.json")); + await fs.writeFile(path.join(home, "config.toml"), "x\n", "utf8"); + + staged = await stageCodexHomeForSync(home, { runId: "run-dangling" }); + expect(await fs.readdir(staged)).toEqual(["config.toml"]); + } finally { + if (staged) await fs.rm(staged, { recursive: true, force: true }); + await fs.rm(root, { recursive: true, force: true }); + } + }); + + // C3 + C4 — an unexpected I/O error must reject (fail-closed, not partial) + // AND remove the temp dir it created (cleanup on the error path). + it("fails closed and removes the temp dir on an unexpected I/O error", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-codex-stage-fail-")); + try { + const { home } = await buildFakeHome(root); + + let createdDir: string | null = null; + const realMkdtemp = fs.mkdtemp.bind(fs); + vi.spyOn(fs, "mkdtemp").mockImplementation(async (prefix: string, ...rest: unknown[]) => { + const dir = await (realMkdtemp as typeof fs.mkdtemp)(prefix, ...(rest as [])); + createdDir = dir as string; + return dir; + }); + vi.spyOn(fs, "readFile").mockRejectedValue( + Object.assign(new Error("boom"), { code: "EACCES" }), + ); + + await expect(stageCodexHomeForSync(home, { runId: "run-fail" })).rejects.toThrow("boom"); + expect(createdDir).not.toBeNull(); + // The staged temp dir was cleaned up despite the failure. + await expect(fs.access(createdDir as unknown as string)).rejects.toMatchObject({ code: "ENOENT" }); + } finally { + await fs.rm(root, { recursive: true, force: true }); + } + }); + + // Circular symlinks inside skills/ must be silently skipped (not throw ELOOP). + // Skill symlinks that point OUTSIDE skills/ are intentional design (Paperclip + // stores skill packages in a shared location) and are dereferenced normally; + // all resulting files land 0600 inside the 0700 staged dir. + it("skips circular skill symlinks (ELOOP) without throwing", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-codex-stage-circular-")); + let staged: string | null = null; + try { + const home = path.join(root, "codex-home"); + await fs.mkdir(path.join(home, "skills"), { recursive: true }); + // Self-referential symlink: would loop forever — must be skipped. + const circularLink = path.join(home, "skills", "loop.md"); + await fs.symlink(circularLink, circularLink); + // A normal skill file — must still be staged. + await fs.writeFile(path.join(home, "skills", "legit.md"), "# ok\n", "utf8"); + + staged = await stageCodexHomeForSync(home, { runId: "run-circular" }); + const stagedSkillEntries = await fs.readdir(path.join(staged, "skills")); + // Circular link must be absent. + expect(stagedSkillEntries).not.toContain("loop.md"); + // Normal skill file must be present. + expect(stagedSkillEntries).toContain("legit.md"); + } finally { + if (staged) await fs.rm(staged, { recursive: true, force: true }); + await fs.rm(root, { recursive: true, force: true }); + } + }); + + // Mode normalization: nested skill files must be staged 0600 regardless of + // their source mode (0644 documents, 0755 scripts, etc.). + it("writes nested skill files with mode 0600 regardless of source mode", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-codex-stage-skill-mode-")); + let staged: string | null = null; + try { + const home = path.join(root, "codex-home"); + await fs.mkdir(path.join(home, "skills", "my-skill"), { recursive: true }); + // Typical source modes: readable doc (0644) and executable script (0755); + // both must land 0600 in the staged dir. + await fs.writeFile(path.join(home, "skills", "my-skill", "SKILL.md"), "# skill\n", { mode: 0o644 }); + await fs.writeFile(path.join(home, "skills", "my-skill", "run.sh"), "#!/bin/sh\n", { mode: 0o755 }); + + staged = await stageCodexHomeForSync(home, { runId: "run-skill-mode" }); + for (const rel of ["my-skill/SKILL.md", "my-skill/run.sh"]) { + const mode = (await fs.stat(path.join(staged, "skills", rel))).mode & 0o777; + expect(mode, `skills/${rel} should be staged 0600`).toBe(0o600); + } + } finally { + if (staged) await fs.rm(staged, { recursive: true, force: true }); + await fs.rm(root, { recursive: true, force: true }); + } + }); + + // Finding 1 (Greptile): a directory symlink such as `back -> .` inside a skill + // resolves to an ancestor directory (it does NOT raise ELOOP), so naive + // recursion would traverse the same tree forever until disk/memory is + // exhausted. Cycle detection must let staging finish while still copying the + // real content and skipping the self-referential link. + it("does not infinitely traverse an ancestor directory link (back -> .) inside a skill", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-codex-stage-cycle-")); + let staged: string | null = null; + try { + const home = path.join(root, "codex-home"); + const skillDir = path.join(home, "skills", "my-skill"); + await fs.mkdir(skillDir, { recursive: true }); + await fs.writeFile(path.join(skillDir, "SKILL.md"), "# skill\n", "utf8"); + // Directory symlink pointing at the skill's own dir — resolves to a + // directory already on the active traversal path; must be skipped. + await fs.symlink(".", path.join(skillDir, "back")); + + staged = await stageCodexHomeForSync(home, { runId: "run-cycle" }); + + // Completed without hanging; the real file is staged and the cyclic link + // produced no runaway nested `back/back/…` chain (it is skipped entirely). + expect(await fs.readdir(path.join(staged, "skills", "my-skill"))).toEqual(["SKILL.md"]); + expect(await fs.readFile(path.join(staged, "skills", "my-skill", "SKILL.md"), "utf8")).toBe( + "# skill\n", + ); + } finally { + if (staged) await fs.rm(staged, { recursive: true, force: true }); + await fs.rm(root, { recursive: true, force: true }); + } + }); + + // Finding 2 (Greptile): a symlink *inside* a skill that points to a host file + // or directory OUTSIDE that skill (e.g. `~/.ssh/id_rsa`) must NOT be + // dereferenced into the staged asset — otherwise a malformed/compromised skill + // could smuggle host secrets past CODEX_SYNC_ALLOWLIST. + it("does not stage a nested skill symlink that escapes the skill root", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-codex-stage-escape-")); + let staged: string | null = null; + try { + const home = path.join(root, "codex-home"); + const skillDir = path.join(home, "skills", "my-skill"); + await fs.mkdir(skillDir, { recursive: true }); + await fs.writeFile(path.join(skillDir, "SKILL.md"), "# skill\n", "utf8"); + + // A host "secret" file and directory living OUTSIDE the skill dir. + const secretFile = path.join(root, "host-secret.txt"); + await fs.writeFile(secretFile, "TOP SECRET\n", "utf8"); + const secretDir = path.join(root, "host-secret-dir"); + await fs.mkdir(secretDir, { recursive: true }); + await fs.writeFile(path.join(secretDir, "creds"), "creds\n", "utf8"); + + // Nested symlinks inside the skill escaping to those host paths. + await fs.symlink(secretFile, path.join(skillDir, "stolen.txt")); + await fs.symlink(secretDir, path.join(skillDir, "stolen-dir")); + + staged = await stageCodexHomeForSync(home, { runId: "run-escape" }); + + const stagedEntries = await fs.readdir(path.join(staged, "skills", "my-skill")); + // The legit in-skill file is staged… + expect(stagedEntries).toContain("SKILL.md"); + // …but neither escaping link is followed into the staged asset. + expect(stagedEntries).not.toContain("stolen.txt"); + expect(stagedEntries).not.toContain("stolen-dir"); + } finally { + if (staged) await fs.rm(staged, { recursive: true, force: true }); + await fs.rm(root, { recursive: true, force: true }); + } + }); + + // Defense-in-depth for finding 1: a degenerate top-level `skills/ -> ..` + // link resolves to an ancestor of `skills/` (the home). It must be skipped, + // never adopted as a containment root — otherwise the whole home + // (`sessions/`, `*.sqlite`, …) would be dragged into the staged skills asset. + it("skips a top-level skills entry that resolves to an ancestor of skills/", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-codex-stage-ancestor-")); + let staged: string | null = null; + try { + const home = path.join(root, "codex-home"); + await fs.mkdir(path.join(home, "skills"), { recursive: true }); + await fs.writeFile(path.join(home, "skills", "legit.md"), "# ok\n", "utf8"); + // Runtime state in the home that must never reach the staged asset. + await fs.writeFile(path.join(home, "logs.sqlite"), "x", "utf8"); + // `up -> ..` resolves to the home dir (an ancestor of skills/). + await fs.symlink("..", path.join(home, "skills", "up")); + + staged = await stageCodexHomeForSync(home, { runId: "run-ancestor" }); + + const stagedSkillEntries = await fs.readdir(path.join(staged, "skills")); + expect(stagedSkillEntries).toContain("legit.md"); + // The ancestor link is skipped, so the home's runtime state is not dragged in. + expect(stagedSkillEntries).not.toContain("up"); + } finally { + if (staged) await fs.rm(staged, { recursive: true, force: true }); + await fs.rm(root, { recursive: true, force: true }); + } + }); +}); diff --git a/packages/adapters/codex-local/src/server/codex-home.ts b/packages/adapters/codex-local/src/server/codex-home.ts index 0739066a76..84313ae83d 100644 --- a/packages/adapters/codex-local/src/server/codex-home.ts +++ b/packages/adapters/codex-local/src/server/codex-home.ts @@ -10,6 +10,22 @@ const SYMLINKED_SHARED_FILES = ["auth.json"] as const; const MANAGED_MCP_BLOCK_START = "# BEGIN PAPERCLIP MANAGED MCP"; const MANAGED_MCP_BLOCK_END = "# END PAPERCLIP MANAGED MCP"; +/** + * The allowlist of managed `CODEX_HOME` entries that the codex-local adapter + * stages into the sandbox `home` asset (see {@link stageCodexHomeForSync}). + * Derived from the seeding constants so it can never drift from what the adapter + * actually writes into the home: the copied static config files, the symlinked + * credential file, and the injected `skills/` directory. Everything else the + * stock upstream `codex` binary writes at runtime (`*.sqlite`, `*-wal`, + * `plugins/`, `cache/`, `sessions/`, `shell_snapshots/`, …) is intentionally + * excluded — it is large host-local runtime state the sandbox run never needs. + */ +export const CODEX_SYNC_ALLOWLIST = [ + ...COPIED_SHARED_FILES, + ...SYMLINKED_SHARED_FILES, + "skills", +] as const; + export type ManagedCodexMcpGateway = { name: string; endpointPath: string; @@ -321,6 +337,236 @@ export async function writeApiKeyAuthJson(home: string, apiKey: string): Promise await fs.writeFile(target, JSON.stringify({ OPENAI_API_KEY: apiKey }), { mode: 0o600 }); } +export interface StageCodexHomeForSyncOptions { + /** Run id, used only to make the staged temp-dir name traceable in logs. */ + runId?: string; +} + +/** + * True when `candidate` is `root` itself or a descendant of it. Both arguments + * must be absolute, already-resolved (symlink-free) paths — callers pass + * `fs.realpath` output — so `path.relative` is a reliable containment test that + * is not fooled by `..` segments or a trailing-separator prefix collision + * (`/a/skills` vs `/a/skills-evil`). + */ +function isResolvedPathInside(candidate: string, root: string): boolean { + if (candidate === root) return true; + const rel = path.relative(root, candidate); + return rel.length > 0 && !rel.startsWith("..") && !path.isAbsolute(rel); +} + +/** + * Recursively copies one skill subtree — rooted at its real directory + * `containmentRoot` — into `targetDir`, dereferencing symlinks to bytes (so the + * sandbox receives real file content, not host-relative links) and normalizing + * every copied regular file to mode `0600`. Created directories get mode `0700`. + * + * Two containment guards protect the staged upload: + * + * - **Allowlist escape (finding 2).** After dereferencing, a symlink whose real + * target falls *outside* `containmentRoot` is skipped. A malformed or + * compromised skill could otherwise smuggle host files that are not in + * `CODEX_SYNC_ALLOWLIST` (e.g. `~/.ssh/id_rsa`) into the upload by pointing a + * nested link at them. The skill's own top-level link into the shared skill + * store is still honoured — it is what establishes `containmentRoot` in + * {@link stageDirectorySecure}; only links that escape *that* root are cut. + * - **Directory cycles (finding 1).** A directory symlink such as `back -> .` or + * `back -> ..` resolves to an ancestor directory instead of raising `ELOOP`; + * recursing into it would traverse the same tree forever until disk/memory is + * exhausted. A resolved directory already on the active traversal path + * (`activePath`) is therefore skipped. + * + * Dangling symlinks (`ENOENT`) and self-referential links that do trip `ELOOP` + * are silently skipped, as are non-file/dir entries (sockets, devices). + */ +async function stageContainedSubtree( + sourceDir: string, + targetDir: string, + containmentRoot: string, + activePath: Set, +): Promise { + await fs.mkdir(targetDir, { recursive: true, mode: 0o700 }); + const entries = await fs.readdir(sourceDir, { withFileTypes: true }); + for (const entry of entries) { + const entrySource = path.join(sourceDir, entry.name); + const entryTarget = path.join(targetDir, entry.name); + // Resolve the real path; dangling or self-referential (`ELOOP`) links skip. + const resolved = await fs.realpath(entrySource).catch((error) => { + const code = (error as NodeJS.ErrnoException).code; + if (code === "ENOENT" || code === "ELOOP") return null; + throw error; + }); + if (!resolved) continue; + // Allowlist containment: never dereference a link that escapes this skill's + // real root (host files outside CODEX_SYNC_ALLOWLIST) into the upload. + if (!isResolvedPathInside(resolved, containmentRoot)) continue; + const entryStat = await fs.stat(resolved).catch((error) => { + if ((error as NodeJS.ErrnoException).code === "ENOENT") return null; + throw error; + }); + if (!entryStat) continue; + if (entryStat.isDirectory()) { + // Cycle guard: a directory already open on the active path (reached via a + // `back -> .`-style link) would otherwise recurse forever. + if (activePath.has(resolved)) continue; + activePath.add(resolved); + await stageContainedSubtree(resolved, entryTarget, containmentRoot, activePath); + activePath.delete(resolved); + } else if (entryStat.isFile()) { + const bytes = await fs.readFile(resolved); + await fs.writeFile(entryTarget, bytes, { mode: 0o600 }); + await fs.chmod(entryTarget, 0o600); + } + // Other types (sockets, devices) are silently skipped. + } +} + +/** + * Recursively copies `sourceDir` (a directory allowlist entry — currently only + * `skills/`) into `targetDir`, dereferencing symlinks to bytes and normalizing + * every copied regular file to mode `0600`. Created directories get mode `0700`. + * + * This replaces `fs.cp({ dereference: true })` which preserves source file modes, + * leaving `0644` documents and `0755` scripts group/other-readable in the staged + * asset; here all regular files are normalized to `0600` regardless of source mode. + * + * `sourceDir`'s *direct* children are the Paperclip-injected skill symlinks that + * intentionally point into a shared skill store *outside* `CODEX_HOME/skills/`, + * so each child is allowed to resolve anywhere — and when it resolves to a + * directory it becomes the containment root for its own subtree. Everything + * *below* that root is copied via {@link stageContainedSubtree}, which refuses to + * follow a nested symlink out of the skill (finding 2) and detects directory + * cycles (finding 1). A direct child that resolves to `sourceDir` itself or to + * an ancestor of it (a degenerate `-> .` / `-> ..` link at the top level) is + * skipped rather than used as a root, so it can never drag the wider home into + * the staged skills asset. The `0700` staged directory and per-file `0600` mode + * together ensure even externally-sourced skill content is not group/world-readable. + */ +async function stageDirectorySecure( + sourceDir: string, + targetDir: string, +): Promise { + await fs.mkdir(targetDir, { recursive: true, mode: 0o700 }); + const realSourceDir = await fs.realpath(sourceDir); + const entries = await fs.readdir(sourceDir, { withFileTypes: true }); + for (const entry of entries) { + const entrySource = path.join(sourceDir, entry.name); + const entryTarget = path.join(targetDir, entry.name); + // Resolve the real path; dangling or self-referential (`ELOOP`) links skip. + const resolved = await fs.realpath(entrySource).catch((error) => { + const code = (error as NodeJS.ErrnoException).code; + if (code === "ENOENT" || code === "ELOOP") return null; + throw error; + }); + if (!resolved) continue; + // A top-level child that resolves to the skills dir itself or an ancestor + // of it (`back -> .` / `back -> ..`) is degenerate: using it as a root would + // re-stage the whole home under `skills/`. Skip it. + if (isResolvedPathInside(realSourceDir, resolved)) continue; + const entryStat = await fs.stat(resolved).catch((error) => { + if ((error as NodeJS.ErrnoException).code === "ENOENT") return null; + throw error; + }); + if (!entryStat) continue; + if (entryStat.isDirectory()) { + // This child skill establishes its own containment root: nested links may + // not escape it, and the root seeds the cycle-detection active path. + await stageContainedSubtree(resolved, entryTarget, resolved, new Set([resolved])); + } else if (entryStat.isFile()) { + const bytes = await fs.readFile(resolved); + await fs.writeFile(entryTarget, bytes, { mode: 0o600 }); + await fs.chmod(entryTarget, 0o600); + } + // Other types (sockets, devices) are silently skipped. + } +} + +/** + * Copies a single allowlist entry from the managed home into the staged dir, + * dereferencing symlinks to bytes. Missing entries are skipped (keyring mode has + * no `auth.json`; some homes have no `config.json`). Every staged regular file is + * written `0600` (least privilege). Any non-`ENOENT` error propagates to the caller. + */ +async function stageCodexHomeEntry( + sourceHome: string, + stagedHome: string, + entry: string, +): Promise { + const source = path.join(sourceHome, entry); + // `fs.stat` follows symlinks, so a dangling link (e.g. a removed auth source) + // reports ENOENT and is skipped exactly like a genuinely absent file. + const stat = await fs.stat(source).catch((error) => { + if ((error as NodeJS.ErrnoException).code === "ENOENT") return null; + throw error; + }); + if (!stat) return; + + const target = path.join(stagedHome, entry); + if (stat.isDirectory()) { + // Recursively copy with mode normalization — nested regular files land + // `0600` and dangling/circular symlinks are skipped. + await stageDirectorySecure(source, target); + return; + } + + // `fs.readFile` follows the symlink into the shared source and returns the + // resolved bytes (the live single-use auth token), which we write as a plain + // regular file so copy-back and in-sandbox auth read real bytes. + const bytes = await fs.readFile(source); + // Stage every regular file `0600`, not just `auth.json`. The staged dir is a + // 0700 mkdtemp and each file is read back only by the owner (Codex in-sandbox + + // copy-back), so nothing needs group/other read. This is least privilege and, + // critically, keeps secret-bearing entries protected: `config.toml` embeds the + // managed MCP `Authorization = "Bearer …"` header (and the source writer + // persists it 0600), so a per-file credential allowlist would silently + // downgrade it to 0644 in a world-readable tmpdir. + await fs.writeFile(target, bytes, { mode: 0o600 }); + // Explicit chmod so the mode is 0600 regardless of the process umask. + await fs.chmod(target, 0o600); +} + +/** + * Stages exactly {@link CODEX_SYNC_ALLOWLIST} from `effectiveCodexHome` into a + * fresh private temp dir and returns its path, for registration as the sandbox + * `home` asset. This replaces syncing the whole managed home + a name denylist: + * only the files Codex actually needs are uploaded, so oversized runtime state + * (`sessions/`, `*.sqlite`, `plugins/`, …) never reaches the sandbox. + * + * - **Symlinks are dereferenced to bytes** — the single-use `auth.json` + * credential (a symlink into the shared source home) and each `skills/` entry + * land as real files, never dangling links. + * - **Missing-but-optional entries are skipped** — no `auth.json` in + * keyring-credential mode, or no `config.json`, is not an error. + * - **`mkdtemp` guarantees the staged dir is `0700`** on POSIX, and every staged + * regular file is written `0600` (least privilege), so staged credentials — + * `auth.json` (OAuth token) and `config.toml` (managed MCP bearer header) — + * are never group/other-readable. + * - **Fail-closed** — any *unexpected* I/O error removes the partial temp dir + * and re-throws, so a run never proceeds with a partial or empty home. + * + * The caller owns removing the returned dir on run teardown. + */ +export async function stageCodexHomeForSync( + effectiveCodexHome: string, + options: StageCodexHomeForSyncOptions = {}, +): Promise { + const runIdPart = nonEmpty(options.runId ?? undefined); + const stagedHome = await fs.mkdtemp( + path.join(os.tmpdir(), `paperclip-codex-home-sync-${runIdPart ? `${runIdPart}-` : ""}`), + ); + try { + for (const entry of CODEX_SYNC_ALLOWLIST) { + await stageCodexHomeEntry(effectiveCodexHome, stagedHome, entry); + } + return stagedHome; + } catch (error) { + // Fail-closed: never hand back a partial home. Remove the temp dir we + // created before propagating the failure. + await fs.rm(stagedHome, { recursive: true, force: true }).catch(() => {}); + throw error; + } +} + /** * Seeds auth/config into an explicit Paperclip-managed `targetHome`. Symlinks * `auth.json` from the shared source home (so ChatGPT-subscription credentials diff --git a/packages/adapters/codex-local/src/server/execute.auth-precedence.test.ts b/packages/adapters/codex-local/src/server/execute.auth-precedence.test.ts index 1f66d4c567..5bbbf53af8 100644 --- a/packages/adapters/codex-local/src/server/execute.auth-precedence.test.ts +++ b/packages/adapters/codex-local/src/server/execute.auth-precedence.test.ts @@ -131,9 +131,16 @@ describe("codex sandbox auth precedence warning", () => { }, }); - expect(prepareAdapterExecutionTargetRuntime).toHaveBeenCalledWith(expect.objectContaining({ - assets: [expect.objectContaining({ key: "home", localDir: hostCodexHome })], - })); + // The home asset now ships a curated *staged* allowlist dir (not the raw + // host CODEX_HOME) and carries no `exclude` denylist. + const runtimeCall = (prepareAdapterExecutionTargetRuntime.mock.calls[0] as unknown[])?.[0] as { + assets: Array<{ key: string; localDir: string; exclude?: string[] }>; + }; + const homeAsset = runtimeCall.assets.find((asset) => asset.key === "home"); + expect(homeAsset).toBeDefined(); + expect(homeAsset?.localDir).not.toBe(hostCodexHome); + expect(homeAsset?.localDir).toContain("paperclip-codex-home-sync"); + expect(homeAsset?.exclude).toBeUndefined(); expect(runAdapterExecutionTargetShellCommand).toHaveBeenCalledWith( "run-auth-precedence", expect.objectContaining({ kind: "remote", transport: "sandbox", remoteCwd: "/sandbox/workspace" }), diff --git a/packages/adapters/codex-local/src/server/execute.remote.test.ts b/packages/adapters/codex-local/src/server/execute.remote.test.ts index dc9582dec4..44c2ed036d 100644 --- a/packages/adapters/codex-local/src/server/execute.remote.test.ts +++ b/packages/adapters/codex-local/src/server/execute.remote.test.ts @@ -1,4 +1,4 @@ -import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { mkdir, mkdtemp, readdir, readFile, rm, symlink, writeFile } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; @@ -165,11 +165,19 @@ describe("codex remote execution", () => { remoteDir: managedRemoteWorkspace, })); expect(syncDirectoryToSsh).toHaveBeenCalledTimes(1); - expect(syncDirectoryToSsh).toHaveBeenCalledWith(expect.objectContaining({ - localDir: codexHomeDir, - remoteDir: `${managedRemoteWorkspace}/.paperclip-runtime/codex/home`, - followSymlinks: true, - })); + // The home asset now syncs a curated *staged* allowlist dir, not the raw + // managed CODEX_HOME, and carries no `exclude` denylist. + const homeSyncArgs = (syncDirectoryToSsh.mock.calls[0] as unknown[])?.[0] as { + localDir: string; + remoteDir: string; + followSymlinks?: boolean; + exclude?: string[]; + }; + expect(homeSyncArgs.localDir).not.toBe(codexHomeDir); + expect(homeSyncArgs.localDir).toContain("paperclip-codex-home-sync"); + expect(homeSyncArgs.remoteDir).toBe(`${managedRemoteWorkspace}/.paperclip-runtime/codex/home`); + expect(homeSyncArgs.followSymlinks).toBe(true); + expect(homeSyncArgs.exclude).toBeUndefined(); expect(runChildProcess).toHaveBeenCalledTimes(1); const call = runChildProcess.mock.calls[0] as unknown as @@ -203,6 +211,124 @@ describe("codex remote execution", () => { })); }); + it("stages only the allowlist into the home asset: keeps config.toml/skills/auth, drops session+sqlite state, no exclude", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-codex-allowlist-")); + cleanupDirs.push(rootDir); + const workspaceDir = path.join(rootDir, "workspace"); + const codexHomeDir = path.join(rootDir, "codex-home"); + await mkdir(workspaceDir, { recursive: true }); + await mkdir(codexHomeDir, { recursive: true }); + + // Seed the managed home with the files Codex needs (config.toml carries a + // provider-routing block; skills injected) plus large runtime decoys the + // old 4-name denylist missed. + await writeFile(path.join(codexHomeDir, "auth.json"), '{"tokens":{"account_id":"a","refresh_token":"r"}}', "utf8"); + await writeFile( + path.join(codexHomeDir, "config.toml"), + 'model_provider = "bifrost"\n\n[model_providers.bifrost]\nname = "bifrost"\n', + "utf8", + ); + await writeFile(path.join(codexHomeDir, "instructions.md"), "hi\n", "utf8"); + await mkdir(path.join(codexHomeDir, "skills", "demo"), { recursive: true }); + await writeFile(path.join(codexHomeDir, "skills", "demo", "SKILL.md"), "# demo\n", "utf8"); + // Decoys: + await writeFile(path.join(codexHomeDir, "logs_2.sqlite"), "x", "utf8"); + await writeFile(path.join(codexHomeDir, "state_5.sqlite"), "x", "utf8"); + await mkdir(path.join(codexHomeDir, "sessions"), { recursive: true }); + await writeFile(path.join(codexHomeDir, "sessions", "rollout.jsonl"), "x", "utf8"); + await mkdir(path.join(codexHomeDir, "tmp"), { recursive: true }); + await symlink("/usr/bin/env", path.join(codexHomeDir, "tmp", "arg0")); + + // Snapshot the staged dir contents at sync time — execute() removes the + // staged temp dir on teardown, so we cannot read it after execute returns. + let stagedSnapshot: + | { localDir: string; entries: string[]; skillEntries: string[]; configToml: string; authJson: string } + | null = null; + (syncDirectoryToSsh as unknown as { + mockImplementationOnce: (fn: (args: { localDir: string }) => Promise) => void; + }).mockImplementationOnce(async (args: { localDir: string }) => { + const entries = (await readdir(args.localDir)).sort(); + const skillEntries = entries.includes("skills") + ? (await readdir(path.join(args.localDir, "skills"))).sort() + : []; + const configToml = entries.includes("config.toml") + ? await readFile(path.join(args.localDir, "config.toml"), "utf8") + : ""; + const authJson = entries.includes("auth.json") + ? await readFile(path.join(args.localDir, "auth.json"), "utf8") + : ""; + stagedSnapshot = { localDir: args.localDir, entries, skillEntries, configToml, authJson }; + }); + + await execute({ + runId: "run-allowlist", + agent: { + id: "agent-1", + companyId: "company-1", + name: "CodexCoder", + adapterType: "codex_local", + adapterConfig: {}, + }, + runtime: { + sessionId: null, + sessionParams: null, + sessionDisplayId: null, + taskKey: null, + }, + config: { + command: "codex", + env: { + CODEX_HOME: codexHomeDir, + }, + }, + context: { + paperclipWorkspace: { + cwd: workspaceDir, + source: "project_primary", + }, + }, + executionTransport: { + remoteExecution: { + host: "127.0.0.1", + port: 2222, + username: "fixture", + remoteWorkspacePath: "/remote/workspace", + remoteCwd: "/remote/workspace", + privateKey: "PRIVATE KEY", + knownHosts: "[127.0.0.1]:2222 ssh-ed25519 AAAA", + strictHostKeyChecking: true, + }, + }, + onLog: async () => {}, + }); + + expect(stagedSnapshot).not.toBeNull(); + const snap = stagedSnapshot as unknown as { + localDir: string; + entries: string[]; + skillEntries: string[]; + configToml: string; + authJson: string; + }; + // Allowlist present; decoys gone. + expect(snap.entries).toEqual( + ["auth.json", "config.json", "config.toml", "instructions.md", "skills"] + .filter((e) => e !== "config.json") // no config.json was seeded + .sort(), + ); + for (const decoy of ["logs_2.sqlite", "state_5.sqlite", "sessions", "tmp", "plugins"]) { + expect(snap.entries).not.toContain(decoy); + } + // Phase-3 behavioral invariants: provider routing + skills + auth survive staging. + expect(snap.configToml).toContain("[model_providers.bifrost]"); + expect(snap.configToml).toContain("model_provider"); + expect(snap.skillEntries).toContain("demo"); + expect(snap.authJson).toContain("refresh_token"); + + // The staged temp dir is removed after execute completes (cleanup on teardown). + await expect(readdir(snap.localDir)).rejects.toMatchObject({ code: "ENOENT" }); + }); + it("does not resume saved Codex sessions for remote SSH execution without a matching remote identity", async () => { const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-codex-remote-resume-")); cleanupDirs.push(rootDir); diff --git a/packages/adapters/codex-local/src/server/execute.ts b/packages/adapters/codex-local/src/server/execute.ts index a81deaad26..6feabc491d 100644 --- a/packages/adapters/codex-local/src/server/execute.ts +++ b/packages/adapters/codex-local/src/server/execute.ts @@ -66,6 +66,7 @@ import { resolveManagedCodexHomeDir, resolveSharedCodexHomeDir, seedManagedCodexHome, + stageCodexHomeForSync, mergeManagedCodexMcpGateways, writeManagedCodexMcpConfig, type ManagedCodexMcpGateway, @@ -565,6 +566,10 @@ export async function execute(ctx: AdapterExecutionContext): Promise onLog("stdout", `${line}\n`), })), - // Exclude state that the sandbox run never needs so we don't - // tar/upload hundreds of MB on every run: - // - `tmp`/`.tmp`: transient dirs that can hold symlinks to the - // host Codex binary (e.g. `tmp/arg0`); followSymlinks would - // inline those binaries and bloat the archive. - // - `sessions`: prior conversation rollouts (host-local history, - // typically the bulk of CODEX_HOME) — irrelevant to a fresh run. - // - `shell_snapshots`: host shell captures that don't apply to - // the sandbox's (different) shell/OS. - // Auth, config, and skills (the bits Codex actually needs) are - // small and still uploaded. - exclude: ["tmp", ".tmp", "sessions", "shell_snapshots"], + // No `exclude` denylist: `stagedCodexHomeDir` already contains + // ONLY the allowlisted files (auth/config/skills), so there is + // nothing to filter out. }, ], }); @@ -1329,6 +1334,19 @@ export async function execute(ctx: AdapterExecutionContext): Promise { + await onLog( + "stderr", + `[paperclip] Failed to remove staged Codex home "${stagedCodexHomeDir}": ${ + error instanceof Error ? error.message : String(error) + }\n`, + ); + }); + } // Restore the managed config.toml so PAPERCLIP_CODEX_PROVIDERS changes // (or removal) between runs never leave stale provider routing behind. This // finally starts the moment prepareCodexRuntimeConfig returns, so a throw