mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-20 15:09:57 +02:00
## Thinking Path
> - Paperclip manages AI agents for work; agents can run in remote
sandboxes via the codex-local adapter
> - When a remote sandbox run starts, the codex-local adapter syncs the
host CODEX_HOME directory into the sandbox so Codex can authenticate and
load its configuration
> - The old approach staged the entire CODEX_HOME and relied on a 4-name
denylist (`tmp`, `.tmp`, `sessions`, `shell_snapshots`) to exclude
unnecessary state
> - The denylist was too narrow: large host-local runtime files
(`logs_2.sqlite`, `plugins/`, etc.) still uploaded on every remote run —
state the sandbox never uses
> - Additionally, `config.toml` (which embeds the managed MCP
`Authorization: Bearer …` header) was staged at 0644, relying solely on
the staging directory's 0700 permission for protection — a
credential-bearing file without per-file least-privilege
> - This pull request replaces the denylist with an explicit allowlist:
only `{auth.json, config.toml, config.json, instructions.md, skills/}`
are staged into a private `0700` temp dir; all staged regular files are
written `0600` (fail-closed on any staging error)
> - The fix is adapter-local to `packages/adapters/codex-local` with
zero changes to the shared `packages/adapter-utils` seam; inbound
auth-merge and outbound copy-back are preserved unchanged
## Linked Issues or Issue Description
No public GitHub issue for this specific bug. Describing the problem
directly:
**Bug:** The codex-local remote sandbox sync uploads more CODEX_HOME
content than the sandbox uses, and stages a credential-bearing file with
an over-permissive mode.
- **What happened:** The codex-local adapter synced the entire
CODEX_HOME directory minus four directory names (`tmp`, `.tmp`,
`sessions`, `shell_snapshots`). Large host-local runtime artifacts — a
SQLite write-ahead log that can reach 420 MB, plugin directories, crash
logs — uploaded on every remote run. `config.toml` (which embeds the
managed MCP `Authorization: Bearer …` header) was staged at `0644`; only
the staging temp dir's `0700` mode prevented broader exposure.
- **Expected behavior:** Only the files Codex needs inside the sandbox
should sync: `auth.json`, `config.toml`, `config.json`,
`instructions.md`, and `skills/`. All staged regular files should use
mode `0600` (single-layer credential protection is insufficient for
secret-bearing files).
- **Steps to reproduce:** Run a remote codex sandbox from a host with a
mature CODEX_HOME. Observe the sync payload includes `sessions/`,
`*.sqlite`, `plugins/`, etc.
Related work: Refs #9621 (outbound auth sync-back in the same
codex-local adapter area; this PR covers the inbound staging direction)
## What Changed
- New `stageCodexHomeForSync(codexHomeDir)` helper in
`packages/adapters/codex-local/src/server/codex-home.ts` stages only
allowlisted files into a fresh `fs.mkdtemp`-created `0700` temp dir,
then registers that dir as the `home` asset (dropping the old `exclude`
field entirely)
- All staged regular files written with mode `0600` — auth.json holds
single-use OAuth tokens; config.toml embeds MCP bearer headers; least
privilege applies throughout
- Symlinks dereferenced to resolved file bytes (`auth.json` single-use
token, each `skills/` entry land as real files)
- Missing-but-optional allowlist entries skipped silently (keyring-only
mode with no `auth.json`; absent `config.json`)
- Staged temp dir removed on teardown AND on error; staging is
fail-closed — a staging error aborts the run rather than shipping a
partial CODEX_HOME
- `execute.ts` updated to call `stageCodexHomeForSync` instead of the
old whole-dir + denylist path; the `exclude` field is removed from the
`home` asset
- Adapter-local fix: zero diff to `packages/adapter-utils` (shared
managed-runtime seam untouched); inbound auth-merge (`provision`) and
outbound copy-back (`restore`) preserved unchanged
## Verification
```sh
# Full codex-local test suite
npx vitest run packages/adapters/codex-local
# 174/174 pass
# Shared seam regression — must be zero diff to packages/adapter-utils
npx vitest run packages/adapter-utils
# 14/14 pass; zero changes to packages/adapter-utils source
# TypeScript (both packages)
npx tsc --noEmit -p packages/adapters/codex-local/tsconfig.json
npx tsc --noEmit -p packages/adapter-utils/tsconfig.json
# Both clean
```
Key new test coverage
(`packages/adapters/codex-local/src/server/codex-home.test.ts`):
- Stager copies exactly the allowlisted entries and nothing else
- Staged `auth.json` dereferenced from symlink and written mode `0600`
- Staged temp dir created with mode `0700`
- ALL staged regular files mode `0600`, including `config.toml` carrying
an MCP bearer token (regression test fails against the pre-fix `0644`
path)
- Fail-closed: I/O error during staging removes the temp dir and
re-throws
- Missing-but-optional entries (`config.json`) skipped without error
- Dangling symlink entries skipped without error
Execute-path integration test (`execute.remote.test.ts`): home asset
uses staged dir; staged config preserves provider block; skills survive;
staged temp dir removed after run teardown.
## Risks
- **Allowlist narrower than denylist:** Any CODEX_HOME file not in
`{auth.json, config.toml, config.json, instructions.md, skills/}` is now
excluded. If a codex-local configuration stores additional files in
CODEX_HOME that the sandbox requires, those runs will miss that state.
This tradeoff is intentional — only known-necessary files sync.
- **Permission change:** `config.toml` previously staged at `0644`, now
staged at `0600`. Strictly more restrictive; no known behavioral impact
(the sandbox reads as the file owner). This was the security gap that
motivated the fix.
- **Fail-closed staging:** A staging error (disk full, temp dir creation
failed) now aborts the run rather than falling back to the raw
CODEX_HOME. This is the safer behavior, but it changes the failure mode
from "run with potentially wrong state" to "run aborted cleanly."
## Model Used
Claude Opus 4.8 (claude-opus-4-8), Anthropic. Reasoning/agentic tool-use
mode (extended thinking, multi-step tool calls).
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [ ] I will address all Greptile and reviewer comments before
requesting merge
---------
Co-authored-by: Harold Kim <harold@paperclip.ing>
Co-authored-by: Paperclip <noreply@paperclip.ing>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
170 lines
5.8 KiB
TypeScript
170 lines
5.8 KiB
TypeScript
import fs from "node:fs/promises";
|
|
import os from "node:os";
|
|
import path from "node:path";
|
|
import { afterEach, describe, expect, it, vi } from "vitest";
|
|
import {
|
|
CODEX_SANDBOX_AUTH_EXISTS_COMMAND,
|
|
CODEX_SANDBOX_AUTH_PRECEDENCE_WARNING,
|
|
CODEX_SANDBOX_AUTH_PRECEDENCE_WARNING_LOG_LINE,
|
|
} from "./auth-precedence.js";
|
|
|
|
const {
|
|
ensureAdapterExecutionTargetCommandResolvable,
|
|
ensureAdapterExecutionTargetRuntimeCommandInstalled,
|
|
prepareAdapterExecutionTargetRuntime,
|
|
resolveAdapterExecutionTargetCommandForLogs,
|
|
runAdapterExecutionTargetProcess,
|
|
runAdapterExecutionTargetShellCommand,
|
|
startAdapterExecutionTargetPaperclipBridge,
|
|
} = vi.hoisted(() => ({
|
|
ensureAdapterExecutionTargetCommandResolvable: vi.fn(async () => undefined),
|
|
ensureAdapterExecutionTargetRuntimeCommandInstalled: vi.fn(async () => undefined),
|
|
prepareAdapterExecutionTargetRuntime: vi.fn(async () => ({
|
|
target: { kind: "remote", transport: "sandbox", remoteCwd: "/sandbox/workspace" },
|
|
workspaceRemoteDir: "/sandbox/workspace",
|
|
runtimeRootDir: "/sandbox/.paperclip-runtime",
|
|
assetDirs: { home: "/sandbox/.paperclip-runtime/codex/home" },
|
|
restoreWorkspace: vi.fn(async () => undefined),
|
|
})),
|
|
resolveAdapterExecutionTargetCommandForLogs: vi.fn(async () => "/usr/bin/codex"),
|
|
runAdapterExecutionTargetProcess: vi.fn(async () => ({
|
|
exitCode: 0,
|
|
signal: null,
|
|
timedOut: false,
|
|
stdout: "",
|
|
stderr: "",
|
|
pid: 123,
|
|
startedAt: new Date().toISOString(),
|
|
})),
|
|
runAdapterExecutionTargetShellCommand: vi.fn(async () => ({
|
|
exitCode: 0,
|
|
signal: null,
|
|
timedOut: false,
|
|
stdout: "",
|
|
stderr: "",
|
|
pid: null,
|
|
startedAt: new Date().toISOString(),
|
|
})),
|
|
startAdapterExecutionTargetPaperclipBridge: vi.fn(async () => null),
|
|
}));
|
|
|
|
vi.mock("@paperclipai/adapter-utils/execution-target", async () => {
|
|
const actual = await vi.importActual<typeof import("@paperclipai/adapter-utils/execution-target")>(
|
|
"@paperclipai/adapter-utils/execution-target",
|
|
);
|
|
return {
|
|
...actual,
|
|
ensureAdapterExecutionTargetCommandResolvable,
|
|
ensureAdapterExecutionTargetRuntimeCommandInstalled,
|
|
prepareAdapterExecutionTargetRuntime,
|
|
resolveAdapterExecutionTargetCommandForLogs,
|
|
runAdapterExecutionTargetProcess,
|
|
runAdapterExecutionTargetShellCommand,
|
|
startAdapterExecutionTargetPaperclipBridge,
|
|
};
|
|
});
|
|
|
|
import { execute } from "./execute.js";
|
|
|
|
describe("codex sandbox auth precedence warning", () => {
|
|
const cleanupDirs: string[] = [];
|
|
|
|
afterEach(async () => {
|
|
vi.clearAllMocks();
|
|
while (cleanupDirs.length > 0) {
|
|
const dir = cleanupDirs.pop();
|
|
if (!dir) continue;
|
|
await fs.rm(dir, { recursive: true, force: true }).catch(() => undefined);
|
|
}
|
|
});
|
|
|
|
it("logs and emits a run event when sandbox login is shadowed by host auth", async () => {
|
|
const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-codex-auth-precedence-"));
|
|
cleanupDirs.push(root);
|
|
const workspaceDir = path.join(root, "workspace");
|
|
const hostCodexHome = path.join(root, "host-codex-home");
|
|
await fs.mkdir(workspaceDir, { recursive: true });
|
|
await fs.mkdir(hostCodexHome, { recursive: true });
|
|
await fs.writeFile(
|
|
path.join(hostCodexHome, "auth.json"),
|
|
JSON.stringify({ OPENAI_API_KEY: "fake-host-auth" }),
|
|
"utf8",
|
|
);
|
|
|
|
const logs: Array<{ stream: "stdout" | "stderr"; chunk: string }> = [];
|
|
const events: Array<{ eventType: string; level?: string; message?: string; payload?: Record<string, unknown> }> = [];
|
|
|
|
await execute({
|
|
runId: "run-auth-precedence",
|
|
agent: {
|
|
id: "agent-1",
|
|
companyId: "company-1",
|
|
name: "CodexCoder",
|
|
adapterType: "codex_local",
|
|
adapterConfig: { engine: "cli" },
|
|
},
|
|
runtime: {
|
|
sessionId: null,
|
|
sessionParams: null,
|
|
sessionDisplayId: null,
|
|
taskKey: null,
|
|
},
|
|
config: {
|
|
engine: "cli",
|
|
command: "codex",
|
|
cwd: workspaceDir,
|
|
env: { CODEX_HOME: hostCodexHome },
|
|
},
|
|
context: {},
|
|
executionTarget: {
|
|
kind: "remote",
|
|
transport: "sandbox",
|
|
providerKey: "fixture",
|
|
remoteCwd: "/workspace",
|
|
runner: { execute: vi.fn() },
|
|
},
|
|
onLog: async (stream, chunk) => {
|
|
logs.push({ stream, chunk });
|
|
},
|
|
onEvent: async (event) => {
|
|
events.push(event);
|
|
},
|
|
});
|
|
|
|
// 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" }),
|
|
CODEX_SANDBOX_AUTH_EXISTS_COMMAND,
|
|
expect.objectContaining({ env: {}, timeoutSec: 5 }),
|
|
);
|
|
expect(logs).toContainEqual({
|
|
stream: "stderr",
|
|
chunk: CODEX_SANDBOX_AUTH_PRECEDENCE_WARNING_LOG_LINE,
|
|
});
|
|
expect(events).toContainEqual({
|
|
eventType: "codex.auth_precedence_warning",
|
|
stream: "system",
|
|
level: "warn",
|
|
message: CODEX_SANDBOX_AUTH_PRECEDENCE_WARNING,
|
|
payload: {
|
|
configuredApiKey: false,
|
|
hostAuthJson: true,
|
|
sandboxAuthJson: true,
|
|
winner: "host_auth_json",
|
|
sandboxLoginShadowed: true,
|
|
},
|
|
});
|
|
expect(runAdapterExecutionTargetProcess).toHaveBeenCalledOnce();
|
|
});
|
|
});
|