feat(codex-local): add outbound direction to codex-auth-merge decision predicate (#9787)
## Thinking Path > - Paperclip runs Codex as a sandboxed agent with its own CODEX_HOME; at the start of each run Paperclip merges the sandbox's auth.json with the host's so the agent can authenticate to the Codex API > - The merge uses `codex-auth-merge-decision.cjs`, a small CJS script that reads two `auth.json` snapshots and exits `10` (use source copy) or `20` (keep destination copy) > - Until now the script only handled the **inbound** direction (host → sandbox) with hard-coded sandbox/host semantics > - Sandbox-to-host copy-back (**outbound**) requires the same freshness guard but with swapped roles — and exposing both directions via a flag requires hard-coding knowledge about which role is "sandbox" and which is "host" into the predicate itself > - A cleaner design: the predicate is **direction-agnostic** — it accepts a `source` path and a `destination` path; the caller decides direction by which argument it passes first. The script answers one question: "should destination be replaced by source?" Exit `10` = yes (use source); exit `20` = no (keep destination) > - This PR refactors `codex-auth-merge-decision.cjs` to this source/destination model, removes the `--direction` flag, and extends coverage with a 15-row source/destination predicate matrix plus updated extract integration tests ## Linked Issues or Issue Description No public GitHub issue exists. The underlying problem: **Context** — Paperclip merges `auth.json` files between sandbox and host to keep Codex credentials in sync across agent runs. The existing decision predicate (`codex-auth-merge-decision.cjs`) handled the inbound direction (host→sandbox) with hard-coded sandbox/host naming. A copy-back (outbound) direction was needed, but rather than add a `--direction` flag that hard-codes role knowledge into the predicate, the design was simplified: the predicate is now **direction-agnostic**, taking `source` and `destination` paths and answering "replace destination with source?" The freshness guard is unified: exit `10` only when source and destination share the same `account_id`, same subscription-kind, both carry a parseable `last_refresh`, and `source.last_refresh > destination.last_refresh` (strict). Ties and null freshness keep the destination copy so a spent single-use refresh token (openai/codex#15502) is never written back over a valid one. Related PRs: - Refs #9785 — Phase 2: relocated the decision predicate from `adapter-utils` into `codex-local`; this PR builds on that merged change - Refs #9621 — community PR targeting Codex sandbox auth sync-back via a different approach; operates on different files (runtime layer vs. decision script), non-conflicting ## What Changed - `packages/adapters/codex-local/src/server/codex-auth-merge-decision.cjs` — refactored from direction-parameterized (sandbox/host with `--direction`) to **direction-agnostic source/destination** semantics. Args: first = source path, second = destination path. Exit `10` (use source) only when: same `account_id`, same subscription-kind, both carry a parseable `last_refresh`, and `source.last_refresh > destination.last_refresh` (strict). Every other case exits `20` (keep destination). Unknown/extra flags are still rejected with exit `1`. No token bytes are emitted to stdout/stderr. - `packages/adapters/codex-local/src/server/codex-auth-merge.test.ts` — rewritten to source/destination API: 15-row predicate matrix (strictly-newer→`10`; tie/older/identity-mismatch/null-refresh/apikey/kind-mismatch/unusable→`20`) plus token-bytes non-emission guard. The inbound extract integration suite updated to match. - Runtime callers (`codex-auth-merge-extract.sh`) already invoke `node decision <sandbox> <host>` = `(source, destination)` and branch on exit `10` → keep sandbox, so **no caller changes are needed**. **Behavioral note:** unifying the two directions under a single "use source only when strictly fresher" rule means a tie (exact equal `last_refresh`) now keeps the **destination** for both the inbound and outbound frames. For inbound restore, this means an exact timestamp tie keeps the host copy. On a tie, both credentials are the same generation — preferring the canonical destination (host) copy is more conservative. This is an intentional consequence of the unified guard. ## Verification ```sh # From the codex-local package root: pnpm vitest run src/server/codex-auth-merge.test.ts # Expected: all predicate rows green (15-row source/destination matrix + token-leak guard) # Plus: codex-auth-merge-extract integration rows green ``` The test suite drives the `.cjs` as a child process over temporary `auth.json` fixtures: - Source/destination predicate matrix (15 rows): strictly-newer→`10`; tie→`20`; older→`20`; identity-mismatch→`20`; source last_refresh unparseable→`20`; destination last_refresh unparseable→`20`; apikey either side→`20`; kind-mismatch→`20`; unusable either side→`20` - Token-bytes non-emission guard: asserts no auth token bytes appear in `stdout`/`stderr` - Inbound extract integration: `codex-auth-merge-extract.sh` end-to-end rows including tie/identity-mismatch/sandbox-newer cases ## Risks - **Tie behavior change**: exact `last_refresh` ties now always keep the destination copy (was: keep-sandbox for inbound). On a tie both credentials are the same generation; keeping the canonical destination (host) copy is the more conservative choice. Covered by test assertions. - **Token non-emission**: `stdout`/`stderr` of the `.cjs` must not echo credential bytes; covered by the token-bytes non-emission guard test. - **Unknown/extra flags**: handled with `process.exit(1)` rather than a silent default, so misconfigured callers surface errors immediately. - **Caller compatibility**: the only runtime caller, `codex-auth-merge-extract.sh`, already calls `node decision <sandbox> <host>` with no flags — compatible with the new source/destination API with no changes required. - **Low risk overall**: the script is exercised end-to-end via the extract integration tests; all paths covered. ## Model Used Claude claude-sonnet-4-6 (Anthropic, `claude-sonnet-4-6`), 200 k context window, tool use and agentic code execution enabled. Automated Paperclip multi-agent system. ## 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 - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] 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>
This commit is contained in:
Nicky Leach
GitHub
parent
7ffeafad1f
commit
936215693c
|
|
@ -41,32 +41,47 @@ function parseAuth(filePath) {
|
|||
};
|
||||
}
|
||||
|
||||
const [sandboxAuthPath, hostAuthPath] = process.argv.slice(2);
|
||||
const sandboxAuth = parseAuth(sandboxAuthPath);
|
||||
const hostAuth = parseAuth(hostAuthPath);
|
||||
// This predicate answers a single, direction-agnostic question: should the
|
||||
// caller replace the `destination` auth.json with the `source` auth.json? The
|
||||
// caller picks which copy is source and which is destination from its own frame
|
||||
// of reference (an inbound restore, an outbound copy-back, …) purely by argument
|
||||
// order — there is no `--direction` flag and no hard-coded sandbox/host notion:
|
||||
//
|
||||
// argv[0] (first positional) = source auth.json path
|
||||
// argv[1] (second positional) = destination auth.json path
|
||||
//
|
||||
// Exit 10 = use source; exit 20 = keep destination. The predicate only ever
|
||||
// reads the two files and exits with a code — it never prints token bytes.
|
||||
const USE_SOURCE = 10;
|
||||
const KEEP_DESTINATION = 20;
|
||||
|
||||
const [sourceAuthPath, destinationAuthPath] = process.argv.slice(2);
|
||||
const sourceAuth = parseAuth(sourceAuthPath);
|
||||
const destinationAuth = parseAuth(destinationAuthPath);
|
||||
|
||||
// Fail closed to the destination unless both sides are the same usable,
|
||||
// subscription-kind identity — an unusable side, an api-key credential, a kind
|
||||
// mismatch, or a different account_id all keep the destination copy.
|
||||
if (
|
||||
hostAuth.kind === "unusable" ||
|
||||
sandboxAuth.kind === "unusable" ||
|
||||
sandboxAuth.kind !== hostAuth.kind
|
||||
destinationAuth.kind === "unusable" ||
|
||||
sourceAuth.kind === "unusable" ||
|
||||
sourceAuth.kind !== destinationAuth.kind ||
|
||||
destinationAuth.kind === "apikey" ||
|
||||
sourceAuth.accountId !== destinationAuth.accountId
|
||||
) {
|
||||
process.exit(20);
|
||||
}
|
||||
|
||||
if (hostAuth.kind === "apikey") {
|
||||
process.exit(20);
|
||||
}
|
||||
|
||||
if (sandboxAuth.accountId !== hostAuth.accountId) {
|
||||
process.exit(20);
|
||||
process.exit(KEEP_DESTINATION);
|
||||
}
|
||||
|
||||
// Use the source credential only when it is strictly fresher: both sides must
|
||||
// carry a parseable last_refresh and the source one must be strictly greater.
|
||||
// Ties and null/unparseable freshness keep the destination copy so a spent
|
||||
// single-use refresh token is never written over a good one.
|
||||
if (
|
||||
hostAuth.lastRefresh !== null &&
|
||||
sandboxAuth.lastRefresh !== null &&
|
||||
hostAuth.lastRefresh > sandboxAuth.lastRefresh
|
||||
sourceAuth.lastRefresh !== null &&
|
||||
destinationAuth.lastRefresh !== null &&
|
||||
sourceAuth.lastRefresh > destinationAuth.lastRefresh
|
||||
) {
|
||||
process.exit(20);
|
||||
process.exit(USE_SOURCE);
|
||||
}
|
||||
|
||||
process.exit(10);
|
||||
process.exit(KEEP_DESTINATION);
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ import { execFile as execFileCallback } from "node:child_process";
|
|||
import { lstat, mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { promisify } from "node:util";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
|
||||
|
|
@ -230,7 +231,11 @@ describe("codex home auth merge on sandbox asset extract", () => {
|
|||
}
|
||||
});
|
||||
|
||||
it("keeps same-account sandbox auth when freshness is equal, missing, or unparseable", async () => {
|
||||
it("keeps host auth when the sandbox copy is not strictly newer (equal, missing, or unparseable freshness)", async () => {
|
||||
// The extract path stages the sandbox copy as `source` and the host copy as
|
||||
// `destination`. The decision predicate only adopts the source when it is
|
||||
// strictly fresher, so a tie, a missing last_refresh on either side, or an
|
||||
// unparseable stamp all fall through to the host destination.
|
||||
const cases = [
|
||||
{
|
||||
name: "equal last_refresh",
|
||||
|
|
@ -302,7 +307,7 @@ describe("codex home auth merge on sandbox asset extract", () => {
|
|||
sandboxAuth: entry.sandboxAuth,
|
||||
hostAuth: entry.hostAuth,
|
||||
});
|
||||
expect(result.finalAuth, entry.name).toBe(entry.sandboxAuth);
|
||||
expect(result.finalAuth, entry.name).toBe(entry.hostAuth);
|
||||
expect(result.finalMode, entry.name).toBe(0o600);
|
||||
}
|
||||
});
|
||||
|
|
@ -348,3 +353,197 @@ describe("codex home auth merge on sandbox asset extract", () => {
|
|||
}
|
||||
});
|
||||
});
|
||||
|
||||
// The extract shell script consumes the decision predicate as a child process
|
||||
// and only branches on its exit code (10 = use the source auth.json, 20 = keep
|
||||
// the destination auth.json). The predicate is direction-agnostic: the caller
|
||||
// decides which file is `source` and which is `destination` by argument order
|
||||
// (first = source, second = destination), so there is no `--direction` flag.
|
||||
// This suite drives the `.cjs` directly the same way, asserting the exit code
|
||||
// per row. Both the inbound restore (extract.sh: source = the sandbox copy,
|
||||
// destination = the host copy) and the future outbound copy-back guard reduce to
|
||||
// the same single question — adopt the source only when it is strictly newer,
|
||||
// same-identity, subscription-kind.
|
||||
describe("codex-auth-merge-decision predicate (source/destination)", () => {
|
||||
const cleanupDirs: string[] = [];
|
||||
|
||||
afterEach(async () => {
|
||||
while (cleanupDirs.length > 0) {
|
||||
const dir = cleanupDirs.pop();
|
||||
if (!dir) continue;
|
||||
await rm(dir, { recursive: true, force: true }).catch(() => undefined);
|
||||
}
|
||||
});
|
||||
|
||||
const decisionScriptPath = fileURLToPath(
|
||||
new URL("./codex-auth-merge-decision.cjs", import.meta.url),
|
||||
);
|
||||
|
||||
const KEEP_DESTINATION = 20;
|
||||
const USE_SOURCE = 10;
|
||||
|
||||
function subscriptionAuth(input: {
|
||||
accountId: string;
|
||||
lastRefresh?: string;
|
||||
marker?: string;
|
||||
}): string {
|
||||
const suffix = input.marker ?? input.accountId;
|
||||
return JSON.stringify({
|
||||
tokens: {
|
||||
id_token: `id-token-${suffix}`,
|
||||
access_token: `access-token-${suffix}`,
|
||||
refresh_token: `refresh-token-${suffix}`,
|
||||
account_id: input.accountId,
|
||||
},
|
||||
...(input.lastRefresh ? { last_refresh: input.lastRefresh } : {}),
|
||||
});
|
||||
}
|
||||
|
||||
function apiKeyAuth(marker: string): string {
|
||||
return JSON.stringify({ OPENAI_API_KEY: `sk-${marker}` });
|
||||
}
|
||||
|
||||
async function runDecision(input: {
|
||||
sourceAuth: string;
|
||||
destinationAuth: string;
|
||||
}): Promise<{ code: number; output: string }> {
|
||||
const dir = await mkdtemp(path.join(os.tmpdir(), "paperclip-codex-auth-decision-"));
|
||||
cleanupDirs.push(dir);
|
||||
const sourcePath = path.join(dir, "source-auth.json");
|
||||
const destinationPath = path.join(dir, "destination-auth.json");
|
||||
await writeFile(sourcePath, input.sourceAuth, { mode: 0o600 });
|
||||
await writeFile(destinationPath, input.destinationAuth, { mode: 0o600 });
|
||||
|
||||
// Arg order is the whole contract: first = source, second = destination.
|
||||
const args = [decisionScriptPath, sourcePath, destinationPath];
|
||||
|
||||
try {
|
||||
const result = await execFile("node", args);
|
||||
return { code: 0, output: `${result.stdout}\n${result.stderr}` };
|
||||
} catch (error) {
|
||||
const failure = error as { code?: unknown; stdout?: string; stderr?: string };
|
||||
const output = `${failure.stdout ?? ""}\n${failure.stderr ?? ""}`;
|
||||
if (typeof failure.code === "number") return { code: failure.code, output };
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
const NEWER = "2026-07-09T02:00:00Z";
|
||||
const OLDER = "2026-07-09T01:00:00Z";
|
||||
|
||||
const cases: { name: string; sourceAuth: string; destinationAuth: string; expected: number }[] = [
|
||||
{
|
||||
name: "source strictly newer, same identity → use source",
|
||||
sourceAuth: subscriptionAuth({ accountId: "acct", lastRefresh: NEWER }),
|
||||
destinationAuth: subscriptionAuth({ accountId: "acct", lastRefresh: OLDER }),
|
||||
expected: USE_SOURCE,
|
||||
},
|
||||
{
|
||||
name: "equal last_refresh (tie) → keep destination",
|
||||
sourceAuth: subscriptionAuth({ accountId: "acct", lastRefresh: NEWER, marker: "src" }),
|
||||
destinationAuth: subscriptionAuth({ accountId: "acct", lastRefresh: NEWER, marker: "dst" }),
|
||||
expected: KEEP_DESTINATION,
|
||||
},
|
||||
{
|
||||
name: "source older → keep destination",
|
||||
sourceAuth: subscriptionAuth({ accountId: "acct", lastRefresh: OLDER }),
|
||||
destinationAuth: subscriptionAuth({ accountId: "acct", lastRefresh: NEWER }),
|
||||
expected: KEEP_DESTINATION,
|
||||
},
|
||||
{
|
||||
name: "identity mismatch even when source newer → keep destination",
|
||||
sourceAuth: subscriptionAuth({ accountId: "acct-source", lastRefresh: NEWER }),
|
||||
destinationAuth: subscriptionAuth({ accountId: "acct-destination", lastRefresh: OLDER }),
|
||||
expected: KEEP_DESTINATION,
|
||||
},
|
||||
{
|
||||
name: "source last_refresh missing → keep destination",
|
||||
sourceAuth: subscriptionAuth({ accountId: "acct" }),
|
||||
destinationAuth: subscriptionAuth({ accountId: "acct", lastRefresh: OLDER }),
|
||||
expected: KEEP_DESTINATION,
|
||||
},
|
||||
{
|
||||
name: "destination last_refresh missing → keep destination",
|
||||
sourceAuth: subscriptionAuth({ accountId: "acct", lastRefresh: NEWER }),
|
||||
destinationAuth: subscriptionAuth({ accountId: "acct" }),
|
||||
expected: KEEP_DESTINATION,
|
||||
},
|
||||
{
|
||||
name: "both last_refresh missing → keep destination",
|
||||
sourceAuth: subscriptionAuth({ accountId: "acct" }),
|
||||
destinationAuth: subscriptionAuth({ accountId: "acct" }),
|
||||
expected: KEEP_DESTINATION,
|
||||
},
|
||||
{
|
||||
name: "source last_refresh unparseable → keep destination",
|
||||
sourceAuth: subscriptionAuth({ accountId: "acct", lastRefresh: "not-a-date" }),
|
||||
destinationAuth: subscriptionAuth({ accountId: "acct", lastRefresh: OLDER }),
|
||||
expected: KEEP_DESTINATION,
|
||||
},
|
||||
{
|
||||
name: "destination last_refresh unparseable → keep destination",
|
||||
sourceAuth: subscriptionAuth({ accountId: "acct", lastRefresh: NEWER }),
|
||||
destinationAuth: subscriptionAuth({ accountId: "acct", lastRefresh: "not-a-date" }),
|
||||
expected: KEEP_DESTINATION,
|
||||
},
|
||||
{
|
||||
name: "source apikey → keep destination",
|
||||
sourceAuth: apiKeyAuth("source"),
|
||||
destinationAuth: subscriptionAuth({ accountId: "acct", lastRefresh: OLDER }),
|
||||
expected: KEEP_DESTINATION,
|
||||
},
|
||||
{
|
||||
name: "destination apikey → keep destination",
|
||||
sourceAuth: subscriptionAuth({ accountId: "acct", lastRefresh: NEWER }),
|
||||
destinationAuth: apiKeyAuth("destination"),
|
||||
expected: KEEP_DESTINATION,
|
||||
},
|
||||
{
|
||||
name: "both apikey → keep destination",
|
||||
sourceAuth: apiKeyAuth("source"),
|
||||
destinationAuth: apiKeyAuth("destination"),
|
||||
expected: KEEP_DESTINATION,
|
||||
},
|
||||
{
|
||||
name: "kind mismatch (source subscription, destination apikey) → keep destination",
|
||||
sourceAuth: subscriptionAuth({ accountId: "acct", lastRefresh: NEWER }),
|
||||
destinationAuth: apiKeyAuth("destination"),
|
||||
expected: KEEP_DESTINATION,
|
||||
},
|
||||
{
|
||||
name: "source unusable JSON → keep destination",
|
||||
sourceAuth: "{not valid json",
|
||||
destinationAuth: subscriptionAuth({ accountId: "acct", lastRefresh: OLDER }),
|
||||
expected: KEEP_DESTINATION,
|
||||
},
|
||||
{
|
||||
name: "destination unusable JSON → keep destination",
|
||||
sourceAuth: subscriptionAuth({ accountId: "acct", lastRefresh: NEWER }),
|
||||
destinationAuth: "{not valid json",
|
||||
expected: KEEP_DESTINATION,
|
||||
},
|
||||
];
|
||||
|
||||
for (const entry of cases) {
|
||||
it(entry.name, async () => {
|
||||
const result = await runDecision({
|
||||
sourceAuth: entry.sourceAuth,
|
||||
destinationAuth: entry.destinationAuth,
|
||||
});
|
||||
expect(result.code).toBe(entry.expected);
|
||||
});
|
||||
}
|
||||
|
||||
it("never emits source token bytes", async () => {
|
||||
const result = await runDecision({
|
||||
sourceAuth: subscriptionAuth({
|
||||
accountId: "acct",
|
||||
lastRefresh: NEWER,
|
||||
marker: "SECRET-SENTINEL",
|
||||
}),
|
||||
destinationAuth: subscriptionAuth({ accountId: "acct", lastRefresh: OLDER }),
|
||||
});
|
||||
expect(result.code).toBe(USE_SOURCE);
|
||||
expect(result.output).not.toContain("SENTINEL");
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue