From 6e212e332ec265ef4cd7f72ced6a13ea55c5d09a Mon Sep 17 00:00:00 2001 From: Nicky Leach Date: Wed, 15 Jul 2026 17:42:33 +0000 Subject: [PATCH] fix: harden codex auth sync-back guards Co-authored-by: Paperclip --- .../src/command-managed-runtime.ts | 8 + .../src/sandbox-managed-runtime.test.ts | 202 +++++++++++++++++- .../src/sandbox-managed-runtime.ts | 122 +++++++++-- 3 files changed, 305 insertions(+), 27 deletions(-) diff --git a/packages/adapter-utils/src/command-managed-runtime.ts b/packages/adapter-utils/src/command-managed-runtime.ts index 77acad78cbe..93670ebd94a 100644 --- a/packages/adapter-utils/src/command-managed-runtime.ts +++ b/packages/adapter-utils/src/command-managed-runtime.ts @@ -1,5 +1,6 @@ import path from "node:path"; import { + createSandboxReadFileTooLargeError, prepareSandboxManagedRuntime, type PreparedSandboxManagedRuntime, type SandboxManagedRuntimeAsset, @@ -178,6 +179,13 @@ export function createCommandManagedRuntimeClient(input: { if (!Number.isFinite(totalBytes) || totalBytes < 0) { throw new Error(`Could not determine remote file size for ${remotePath}`); } + if (options?.maxBytes != null && totalBytes > options.maxBytes) { + throw createSandboxReadFileTooLargeError({ + remotePath, + actualBytes: totalBytes, + maxBytes: options.maxBytes, + }); + } // Read in bounded remote chunks so the runner never has to materialize a // single base64 stdout string for the whole archive. The client API still diff --git a/packages/adapter-utils/src/sandbox-managed-runtime.test.ts b/packages/adapter-utils/src/sandbox-managed-runtime.test.ts index 99c816fb15b..d23660a5e6b 100644 --- a/packages/adapter-utils/src/sandbox-managed-runtime.test.ts +++ b/packages/adapter-utils/src/sandbox-managed-runtime.test.ts @@ -1,4 +1,4 @@ -import { lstat, mkdir, mkdtemp, readFile, readdir, realpath, rm, symlink, writeFile } from "node:fs/promises"; +import { lstat, mkdir, mkdtemp, readFile, readdir, realpath, rm, stat, symlink, writeFile } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { execFile as execFileCallback } from "node:child_process"; @@ -7,6 +7,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { resetLocalGitIndexToHead } from "./git-workspace-sync.js"; import { + createSandboxReadFileTooLargeError, mirrorDirectory, prepareSandboxManagedRuntime, type SandboxManagedRuntimeClient, @@ -37,7 +38,17 @@ function createLocalSandboxClient(): SandboxManagedRuntimeClient { await mkdir(path.dirname(remotePath), { recursive: true }); await writeFile(remotePath, Buffer.from(bytes)); }, - readFile: async (remotePath) => await readFile(remotePath), + readFile: async (remotePath, options) => { + const stats = await stat(remotePath); + if (options?.maxBytes != null && stats.size > options.maxBytes) { + throw createSandboxReadFileTooLargeError({ + remotePath, + actualBytes: stats.size, + maxBytes: options.maxBytes, + }); + } + return await readFile(remotePath); + }, listFiles: async (remotePath) => { const entries = await readdir(remotePath, { withFileTypes: true }).catch(() => []); return entries @@ -56,11 +67,13 @@ function createLocalSandboxClient(): SandboxManagedRuntimeClient { function codexSubscriptionAuth(input: { accountId?: string; - lastRefresh: string; + lastRefresh?: string; refreshToken: string; accessToken?: string; idToken?: string; padding?: string; + extraPayload?: Record; + extraTokens?: Record; }): string { return `${JSON.stringify({ tokens: { @@ -68,9 +81,11 @@ function codexSubscriptionAuth(input: { id_token: input.idToken ?? "id-token", access_token: input.accessToken ?? "access-token", refresh_token: input.refreshToken, + ...(input.extraTokens ?? {}), }, - last_refresh: input.lastRefresh, + ...(input.lastRefresh ? { last_refresh: input.lastRefresh } : {}), ...(input.padding ? { padding: input.padding } : {}), + ...(input.extraPayload ?? {}), })}\n`; } @@ -227,12 +242,22 @@ describe("sandbox managed runtime", () => { accessToken: "access-old", idToken: "id-old", }); + const newRefreshToken = `refresh-new-secret-${"r".repeat(8192)}`; + const newAccessToken = `access-new-secret-${"a".repeat(8192)}`; + const newIdToken = `id-new-secret-${"i".repeat(8192)}`; const newAuth = codexSubscriptionAuth({ lastRefresh: "2026-01-01T00:01:00.000Z", - refreshToken: "refresh-new-secret", - accessToken: "access-new-secret", - idToken: "id-new-secret", - padding: "x".repeat(1024 * 1024), + refreshToken: newRefreshToken, + accessToken: newAccessToken, + idToken: newIdToken, + padding: "x".repeat(1024), + extraTokens: { sandbox_token_only: "drop-token" }, + }); + const expectedSyncedAuth = codexSubscriptionAuth({ + lastRefresh: "2026-01-01T00:01:00.000Z", + refreshToken: newRefreshToken, + accessToken: newAccessToken, + idToken: newIdToken, }); await writeFile(hostAuthSource, oldAuth, { mode: 0o600 }); await symlink(hostAuthSource, path.join(localCodexHome, "auth.json")); @@ -285,7 +310,7 @@ describe("sandbox managed runtime", () => { } expect(invalidReads).toEqual([]); - await expect(readFile(hostAuthSource, "utf8")).resolves.toBe(newAuth); + await expect(readFile(hostAuthSource, "utf8")).resolves.toBe(expectedSyncedAuth); expect((await lstat(path.join(localCodexHome, "auth.json"))).isSymbolicLink()).toBe(true); expect(await realpath(path.join(localCodexHome, "auth.json"))).toBe(await realpath(hostAuthSource)); expect((await lstat(hostAuthSource)).mode & 0o777).toBe(0o600); @@ -296,6 +321,8 @@ describe("sandbox managed runtime", () => { expect(combinedTelemetry).not.toContain("refresh-new-secret"); expect(combinedTelemetry).not.toContain("access-new-secret"); expect(combinedTelemetry).not.toContain("id-new-secret"); + expect(await readFile(hostAuthSource, "utf8")).not.toContain("padding"); + expect(await readFile(hostAuthSource, "utf8")).not.toContain("sandbox_token_only"); const nextPrepared = await prepareSandboxManagedRuntime({ spec: { @@ -311,7 +338,58 @@ describe("sandbox managed runtime", () => { workspaceLocalDir: localWorkspaceDir, assets: [{ key: "home", localDir: localCodexHome, followSymlinks: true }], }); - await expect(readFile(path.join(nextPrepared.assetDirs.home, "auth.json"), "utf8")).resolves.toBe(newAuth); + await expect(readFile(path.join(nextPrepared.assetDirs.home, "auth.json"), "utf8")).resolves.toBe(expectedSyncedAuth); + }); + + it("does not sync oversized sandbox Codex auth.json back to host", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-sandbox-codex-auth-too-large-")); + cleanupDirs.push(rootDir); + const localWorkspaceDir = path.join(rootDir, "local-workspace"); + const remoteWorkspaceDir = path.join(rootDir, "remote-workspace"); + const localCodexHome = path.join(rootDir, "local-codex-home"); + const hostCodexHome = path.join(rootDir, "host-codex-home"); + const hostAuthSource = path.join(hostCodexHome, "auth.json"); + await mkdir(localWorkspaceDir, { recursive: true }); + await mkdir(localCodexHome, { recursive: true }); + await mkdir(hostCodexHome, { recursive: true }); + + const hostAuth = codexSubscriptionAuth({ + lastRefresh: "2026-01-01T00:00:00.000Z", + refreshToken: "refresh-host-before-too-large", + }); + const sandboxAuth = codexSubscriptionAuth({ + lastRefresh: "2026-01-01T00:10:00.000Z", + refreshToken: "refresh-sandbox-too-large-secret", + padding: "x".repeat(70 * 1024), + }); + await writeFile(hostAuthSource, hostAuth, { mode: 0o600 }); + await symlink(hostAuthSource, path.join(localCodexHome, "auth.json")); + + const lines: string[] = []; + const prepared = await prepareSandboxManagedRuntime({ + spec: { + transport: "sandbox", + provider: "test", + sandboxId: "sandbox-1", + remoteCwd: remoteWorkspaceDir, + timeoutMs: 30_000, + apiKey: null, + }, + adapterKey: "codex", + client: createLocalSandboxClient(), + workspaceLocalDir: localWorkspaceDir, + assets: [{ key: "home", localDir: localCodexHome, followSymlinks: true }], + onProgress: (line) => { + lines.push(line); + }, + }); + await writeFile(path.join(prepared.assetDirs.home, "auth.json"), sandboxAuth, "utf8"); + + await prepared.restoreWorkspace(); + + await expect(readFile(hostAuthSource, "utf8")).resolves.toBe(hostAuth); + expect(lines.join("")).toContain("not usable subscription auth"); + expect(lines.join("")).not.toContain("refresh-sandbox-too-large-secret"); }); it("does not replace newer host Codex auth.json with older sandbox auth", async () => { @@ -364,6 +442,110 @@ describe("sandbox managed runtime", () => { expect(lines.join("")).not.toContain("refresh-sandbox-older"); }); + it("does not sync sandbox Codex auth.json when refresh freshness is equal or unknown", async () => { + const cases = [ + { + name: "equal", + hostAuth: codexSubscriptionAuth({ + lastRefresh: "2026-01-01T00:10:00.000Z", + refreshToken: "refresh-host-equal", + }), + sandboxAuth: codexSubscriptionAuth({ + lastRefresh: "2026-01-01T00:10:00.000Z", + refreshToken: "refresh-sandbox-equal", + }), + redactedNeedle: "refresh-sandbox-equal", + }, + { + name: "missing-sandbox-refresh", + hostAuth: codexSubscriptionAuth({ + lastRefresh: "2026-01-01T00:10:00.000Z", + refreshToken: "refresh-host-missing-sandbox", + }), + sandboxAuth: codexSubscriptionAuth({ + refreshToken: "refresh-sandbox-missing-refresh", + }), + redactedNeedle: "refresh-sandbox-missing-refresh", + }, + { + name: "missing-host-refresh", + hostAuth: codexSubscriptionAuth({ + refreshToken: "refresh-host-missing-refresh", + }), + sandboxAuth: codexSubscriptionAuth({ + lastRefresh: "2026-01-01T00:10:00.000Z", + refreshToken: "refresh-sandbox-host-unknown", + }), + redactedNeedle: "refresh-sandbox-host-unknown", + }, + { + name: "unparseable-sandbox-refresh", + hostAuth: codexSubscriptionAuth({ + lastRefresh: "2026-01-01T00:10:00.000Z", + refreshToken: "refresh-host-sandbox-bad-refresh", + }), + sandboxAuth: codexSubscriptionAuth({ + lastRefresh: "not-a-date", + refreshToken: "refresh-sandbox-bad-refresh", + }), + redactedNeedle: "refresh-sandbox-bad-refresh", + }, + { + name: "unparseable-host-refresh", + hostAuth: codexSubscriptionAuth({ + lastRefresh: "not-a-date", + refreshToken: "refresh-host-bad-refresh", + }), + sandboxAuth: codexSubscriptionAuth({ + lastRefresh: "2026-01-01T00:10:00.000Z", + refreshToken: "refresh-sandbox-host-bad-refresh", + }), + redactedNeedle: "refresh-sandbox-host-bad-refresh", + }, + ]; + + for (const entry of cases) { + const rootDir = await mkdtemp(path.join(os.tmpdir(), `paperclip-sandbox-codex-auth-${entry.name}-`)); + cleanupDirs.push(rootDir); + const localWorkspaceDir = path.join(rootDir, "local-workspace"); + const remoteWorkspaceDir = path.join(rootDir, "remote-workspace"); + const localCodexHome = path.join(rootDir, "local-codex-home"); + const hostCodexHome = path.join(rootDir, "host-codex-home"); + const hostAuthSource = path.join(hostCodexHome, "auth.json"); + await mkdir(localWorkspaceDir, { recursive: true }); + await mkdir(localCodexHome, { recursive: true }); + await mkdir(hostCodexHome, { recursive: true }); + await writeFile(hostAuthSource, entry.hostAuth, { mode: 0o600 }); + await symlink(hostAuthSource, path.join(localCodexHome, "auth.json")); + + const lines: string[] = []; + const prepared = await prepareSandboxManagedRuntime({ + spec: { + transport: "sandbox", + provider: "test", + sandboxId: `sandbox-${entry.name}`, + remoteCwd: remoteWorkspaceDir, + timeoutMs: 30_000, + apiKey: null, + }, + adapterKey: "codex", + client: createLocalSandboxClient(), + workspaceLocalDir: localWorkspaceDir, + assets: [{ key: "home", localDir: localCodexHome, followSymlinks: true }], + onProgress: (line) => { + lines.push(line); + }, + }); + await writeFile(path.join(prepared.assetDirs.home, "auth.json"), entry.sandboxAuth, "utf8"); + + await prepared.restoreWorkspace(); + + await expect(readFile(hostAuthSource, "utf8"), entry.name).resolves.toBe(entry.hostAuth); + expect(lines.join(""), entry.name).toContain("sandbox auth is not newer"); + expect(lines.join(""), entry.name).not.toContain(entry.redactedNeedle); + } + }); + it("does not sync sandbox Codex auth.json when account ids differ", async () => { const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-sandbox-codex-auth-mismatch-")); cleanupDirs.push(rootDir); diff --git a/packages/adapter-utils/src/sandbox-managed-runtime.ts b/packages/adapter-utils/src/sandbox-managed-runtime.ts index 3efafc7e116..630db3fdca9 100644 --- a/packages/adapter-utils/src/sandbox-managed-runtime.ts +++ b/packages/adapter-utils/src/sandbox-managed-runtime.ts @@ -53,6 +53,8 @@ const SANDBOX_WORKSPACE_HEAVY_DIR_EXCLUDES = SANDBOX_WORKSPACE_HEAVY_DIR_NAMES.f `*/${entry}/*`, ]); const CODEX_AUTH_JSON_NAME = "auth.json"; +const CODEX_AUTH_JSON_MAX_BYTES = 64 * 1024; +export const SANDBOX_READ_FILE_TOO_LARGE_ERROR_CODE = "ERR_PAPERCLIP_SANDBOX_FILE_TOO_LARGE"; export interface SandboxRemoteExecutionSpec { transport: "sandbox"; @@ -74,10 +76,13 @@ export interface SandboxManagedRuntimeAsset { * Per-call byte-level progress hook. `transferredBytes`/`totalBytes` are decoded * file bytes (not the base64 wire size). `totalBytes` is null when the size is * not known up front. The transport is the source of truth for byte counts; the - * orchestrator owns the phase label and direction. + * orchestrator owns the phase label and direction. Read callers may set + * `maxBytes` to ask transports to reject oversized files before downloading the + * full payload when the remote size is knowable. */ export interface SandboxTransferProgressOptions { onProgress?: (transferredBytes: number, totalBytes: number | null) => void | Promise; + maxBytes?: number; } export interface SandboxManagedRuntimeClient { @@ -103,8 +108,25 @@ export interface PreparedSandboxManagedRuntime { type ParsedCodexAuth = | { kind: "unusable" } - | { kind: "apikey" } - | { kind: "subscription"; accountId: string; lastRefresh: number | null }; + | { kind: "apikey"; canonicalPayload: CanonicalCodexApiKeyAuth } + | { + kind: "subscription"; + accountId: string; + lastRefresh: number | null; + canonicalPayload: CanonicalCodexSubscriptionAuth; + }; + +type CodexAuthTokenField = "id_token" | "access_token" | "refresh_token"; +const CODEX_AUTH_TOKEN_FIELDS = ["id_token", "access_token", "refresh_token"] as const satisfies readonly CodexAuthTokenField[]; + +interface CanonicalCodexSubscriptionAuth { + tokens: { account_id: string } & Partial>; + last_refresh?: string; +} + +interface CanonicalCodexApiKeyAuth { + OPENAI_API_KEY: string; +} type CodexAuthSyncBackOutcome = | "updated" @@ -364,6 +386,33 @@ function toBuffer(bytes: Buffer | Uint8Array | ArrayBuffer): Buffer { return Buffer.from(bytes.buffer, bytes.byteOffset, bytes.byteLength); } +type SandboxReadFileTooLargeError = Error & { + code: typeof SANDBOX_READ_FILE_TOO_LARGE_ERROR_CODE; + actualBytes: number; + maxBytes: number; +}; + +export function createSandboxReadFileTooLargeError(input: { + remotePath: string; + actualBytes: number; + maxBytes: number; +}): SandboxReadFileTooLargeError { + const error = new Error( + `Remote file ${input.remotePath} is ${input.actualBytes} bytes, exceeding the ${input.maxBytes} byte limit`, + ) as SandboxReadFileTooLargeError; + error.code = SANDBOX_READ_FILE_TOO_LARGE_ERROR_CODE; + error.actualBytes = input.actualBytes; + error.maxBytes = input.maxBytes; + return error; +} + +function isSandboxReadFileTooLargeError(error: unknown): boolean { + const code = typeof error === "object" && error !== null + ? (error as { code?: unknown }).code + : null; + return code === SANDBOX_READ_FILE_TOO_LARGE_ERROR_CODE; +} + // Co-change notice: this parser mirrors hasUsableAuthPayload in // packages/adapters/codex-local/src/server/codex-home.ts and parseAuth in // packages/adapter-utils/src/codex-auth-merge-decision.cjs. If the auth format @@ -376,7 +425,7 @@ function parseCodexAuthPayload(authPayload: unknown): ParsedCodexAuth { const parsedPayload = authPayload as Record; const apiKey = parsedPayload.OPENAI_API_KEY; if (typeof apiKey === "string" && apiKey.trim().length > 0) { - return { kind: "apikey" }; + return { kind: "apikey", canonicalPayload: { OPENAI_API_KEY: apiKey } }; } const tokens = parsedPayload.tokens; @@ -386,7 +435,7 @@ function parseCodexAuthPayload(authPayload: unknown): ParsedCodexAuth { const parsedTokens = tokens as Record; const accountId = typeof parsedTokens.account_id === "string" ? parsedTokens.account_id.trim() : ""; - const hasTokenMaterial = ["id_token", "access_token", "refresh_token"].some((key) => { + const hasTokenMaterial = CODEX_AUTH_TOKEN_FIELDS.some((key) => { const value = parsedTokens[key]; return typeof value === "string" && value.trim().length > 0; }); @@ -394,6 +443,19 @@ function parseCodexAuthPayload(authPayload: unknown): ParsedCodexAuth { return { kind: "unusable" }; } + const canonicalTokens: CanonicalCodexSubscriptionAuth["tokens"] = { account_id: accountId }; + for (const key of CODEX_AUTH_TOKEN_FIELDS) { + const value = parsedTokens[key]; + if (typeof value === "string") { + canonicalTokens[key] = value; + } + } + + const canonicalPayload: CanonicalCodexSubscriptionAuth = { tokens: canonicalTokens }; + if (typeof parsedPayload.last_refresh === "string") { + canonicalPayload.last_refresh = parsedPayload.last_refresh; + } + const lastRefresh = typeof parsedPayload.last_refresh === "string" ? Date.parse(parsedPayload.last_refresh) : NaN; @@ -401,6 +463,7 @@ function parseCodexAuthPayload(authPayload: unknown): ParsedCodexAuth { kind: "subscription", accountId, lastRefresh: Number.isFinite(lastRefresh) ? lastRefresh : null, + canonicalPayload, }; } @@ -412,6 +475,33 @@ function parseCodexAuthBytes(bytes: Buffer): ParsedCodexAuth { } } +function canonicalCodexAuthBytes(auth: Extract): Buffer { + return Buffer.from(`${JSON.stringify(auth.canonicalPayload)}\n`, "utf8"); +} + +type SandboxCodexAuthReadResult = + | { kind: "found"; bytes: Buffer } + | { kind: "missing" } + | { kind: "too_large" }; + +async function readSandboxCodexAuthBytes(input: { + client: SandboxManagedRuntimeClient; + remoteHomeDir: string; +}): Promise { + const remotePath = path.posix.join(input.remoteHomeDir, CODEX_AUTH_JSON_NAME); + try { + const bytes = toBuffer(await input.client.readFile(remotePath, { maxBytes: CODEX_AUTH_JSON_MAX_BYTES })); + if (bytes.byteLength > CODEX_AUTH_JSON_MAX_BYTES) { + return { kind: "too_large" }; + } + return { kind: "found", bytes }; + } catch (error) { + if (isMissingFileError(error)) return { kind: "missing" }; + if (isSandboxReadFileTooLargeError(error)) return { kind: "too_large" }; + throw error; + } +} + function isMissingFileError(error: unknown): boolean { const code = typeof error === "object" && error !== null ? (error as { code?: unknown }).code @@ -530,29 +620,27 @@ async function syncCodexHomeAuthJsonBackToHost(input: { outcome = "skipped_no_host_symlink"; } else { const remoteHomeDir = input.assetDirs[homeAsset.key]; - const sandboxAuthBytes = remoteHomeDir - ? await input.client.readFile(path.posix.join(remoteHomeDir, CODEX_AUTH_JSON_NAME)) - .then((bytes) => toBuffer(bytes)) - .catch((error: unknown) => { - if (isMissingFileError(error)) return null; - throw error; - }) - : null; + const sandboxAuthRead = remoteHomeDir + ? await readSandboxCodexAuthBytes({ client: input.client, remoteHomeDir }) + : { kind: "missing" as const }; - if (!sandboxAuthBytes) { + if (sandboxAuthRead.kind === "missing") { outcome = "skipped_sandbox_auth_missing"; + } else if (sandboxAuthRead.kind === "too_large") { + outcome = "skipped_unusable_auth"; } else { const hostAuthBytes = await fs.readFile(hostAuthSource) .catch((error: unknown) => { if (isMissingFileError(error)) return null; throw error; }); + const sandboxAuth = parseCodexAuthBytes(sandboxAuthRead.bytes); outcome = decideCodexAuthSyncBack({ - sandboxAuth: parseCodexAuthBytes(sandboxAuthBytes), + sandboxAuth, hostAuth: hostAuthBytes ? parseCodexAuthBytes(hostAuthBytes) : { kind: "unusable" }, }); - if (outcome === "updated") { - await writeFileAtomic0600(hostAuthSource, sandboxAuthBytes); + if (outcome === "updated" && sandboxAuth.kind === "subscription") { + await writeFileAtomic0600(hostAuthSource, canonicalCodexAuthBytes(sandboxAuth)); } } }