From 62096fd335283d83e02bcaacad98b585a33eed4c Mon Sep 17 00:00:00 2001 From: Dotta Date: Tue, 8 Sep 2026 16:32:40 -0500 Subject: [PATCH] fix: preserve native sessions and route Git credentials internally Align durable journal validation with the transport bound, preserve authorization on cached storage clients, and avoid Cloud session gates for native Git callbacks. Co-Authored-By: Paperclip --- doc/sandbox-work-folders.md | 13 ++++++ .../work-folder-route-storage.test.ts | 42 +++++++++++++++++ server/src/routes/work-folders.ts | 6 ++- server/src/services/heartbeat.ts | 3 +- .../native-session-executor.test.ts | 45 ++++++++++++++----- .../native-runtime/native-session-executor.ts | 9 ++-- 6 files changed, 101 insertions(+), 17 deletions(-) create mode 100644 server/src/__tests__/work-folder-route-storage.test.ts diff --git a/doc/sandbox-work-folders.md b/doc/sandbox-work-folders.md index 77152315c0..dab84c38f1 100644 --- a/doc/sandbox-work-folders.md +++ b/doc/sandbox-work-folders.md @@ -217,6 +217,19 @@ and save feedback in the deployed browser. Record API/runner operations, two act short-run flushes, independent task checkouts, identity/privacy boundaries, interrupted saves, and recovery without the original sandbox or app volume. +Repository acceptance must also read the private repository through the managed +Git launcher, for example with `git ls-remote origin HEAD`. Do not count a model's +PATH changes or credential workarounds as a pass. Native Git credential callbacks +use the internal API origin, as legacy callbacks do; the public Cloud tenant +origin requires a browser session. + +Native continuation validates the full control-plane journal with the same +64 MiB bound as the runner transport. Tool output in that journal can exceed +2 MiB without invalidating its identity. Oversized, malformed, foreign-session, +and unverifiable state still fail closed and remain recoverable in quarantine. +The cached-file routes initialize storage once per router after authorization; +every request still checks current owner access. + Passing acceptance does not authorize a merge or mainline release. Both require the user's explicit sign-off. diff --git a/server/src/__tests__/work-folder-route-storage.test.ts b/server/src/__tests__/work-folder-route-storage.test.ts new file mode 100644 index 0000000000..4e7ae84e77 --- /dev/null +++ b/server/src/__tests__/work-folder-route-storage.test.ts @@ -0,0 +1,42 @@ +import express from "express"; +import request from "supertest"; +import { beforeEach, expect, it, vi } from "vitest"; +import type { Db } from "@paperclipai/db"; + +const mocks = vi.hoisted(() => ({ + access: vi.fn(), + loadConfig: vi.fn(() => ({ storageProvider: "local_disk" })), + provider: vi.fn(() => ({})), + ensure: vi.fn(async (owner) => owner), + list: vi.fn(async () => ({ files: [] })), +})); +vi.mock("../config.js", () => ({ loadConfig: mocks.loadConfig })); +vi.mock("../storage/provider-registry.js", () => ({ createStorageProviderFromConfig: mocks.provider })); +vi.mock("../services/work-folder-access.js", () => ({ assertWorkFolderAccess: mocks.access })); +vi.mock("../services/work-folders.js", () => ({ + workFolderService: () => ({ ensure: mocks.ensure, list: mocks.list }), +})); +import { workFolderRoutes } from "../routes/work-folders.js"; + +beforeEach(() => vi.clearAllMocks()); + +it("initializes storage only after access succeeds and reuses it while checking every request", async () => { + const app = express(); + app.use("/api", workFolderRoutes({} as Db)); + app.use((_error: Error, _req: express.Request, res: express.Response, _next: express.NextFunction) => { + res.sendStatus(403); + }); + const base = "/api/companies/11111111-1111-4111-8111-111111111111/work-folders/task/22222222-2222-4222-8222-222222222222"; + expect(mocks.loadConfig).not.toHaveBeenCalled(); + mocks.access.mockRejectedValueOnce(new Error("denied")); + await request(app).get(base).expect(403); + expect(mocks.loadConfig).not.toHaveBeenCalled(); + await request(app).get(base).expect(200); + await request(app).get(`${base}?trash=true`).expect(200); + expect(mocks.loadConfig).toHaveBeenCalledTimes(1); + expect(mocks.provider).toHaveBeenCalledTimes(1); + mocks.access.mockRejectedValueOnce(new Error("revoked")); + await request(app).get(base).expect(403); + expect(mocks.access).toHaveBeenCalledTimes(4); + expect(mocks.list).toHaveBeenCalledTimes(2); +}); diff --git a/server/src/routes/work-folders.ts b/server/src/routes/work-folders.ts index c824cf9b67..6c336cb969 100644 --- a/server/src/routes/work-folders.ts +++ b/server/src/routes/work-folders.ts @@ -22,8 +22,10 @@ const querySchema = z.object({ path: z.string().optional(), trash: z.enum(["true export function workFolderRoutes(db: Db, provider?: StorageProvider) { const router = Router(); - // Resolve lazily: route registration and tests need not initialize cloud credentials. - const service = () => workFolderService(db, provider ?? createStorageProviderFromConfig(loadConfig())); + // Resolve once, lazily after authorization. Loading config probes the host + // synchronously; repeating that for every file poll stalls unrelated requests. + let storage = provider; + const service = () => workFolderService(db, storage ??= createStorageProviderFromConfig(loadConfig())); const base = WORK_FOLDER_ROUTE_PATH; router.use(base, async (req, _res, next) => { const owner = ownerSchema.parse(req.params); diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 84a1763e45..077d719f51 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -21433,7 +21433,8 @@ export function heartbeatService( runtimeRootDir: path.posix.join(executionTarget.remoteCwd, ".paperclip-runtime", "github", run.id), adapterKey: "native-github", hostApiToken: adapterEnv.PAPERCLIP_GITHUB_BROKER_TOKEN, - hostApiUrl: adapterEnv.PAPERCLIP_GITHUB_BROKER_URL, + // Forward inside this API process. The public tenant origin + // requires a browser session and rejects runtime capabilities. onLog, }); } catch { diff --git a/server/src/services/native-runtime/native-session-executor.test.ts b/server/src/services/native-runtime/native-session-executor.test.ts index 3969258c95..9076b1506c 100644 --- a/server/src/services/native-runtime/native-session-executor.test.ts +++ b/server/src/services/native-runtime/native-session-executor.test.ts @@ -1,6 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { access, + appendFile, mkdir, mkdtemp, readdir, @@ -4546,7 +4547,7 @@ describe("runnerd provider runtime wiring", () => { } }); - it("migrates a suspended prior-run authority only when its persisted execution has the same full session scope", async () => { + it.each([0, 5 * 1024 * 1024])("migrates a suspended prior-run authority with %i bytes of journal data only within its full session scope", async (journalBytes) => { const stateBase = await mkdtemp( join(tmpdir(), "paperclip-prior-run-session-state-"), ); @@ -4607,12 +4608,15 @@ describe("runnerd provider runtime wiring", () => { await writeFile( join(legacyRoot, "control-plane", "control-plane-state.json"), JSON.stringify( - durableControlPlaneState({ - runId: priorExecution.binding.runId, - normalizedSessionId: priorExecution.session.normalizedSessionId, - runnerInstanceId: "runner-prior-run-scope", - environmentLeaseId: "lease-prior-run-scope", - }), + { + ...durableControlPlaneState({ + runId: priorExecution.binding.runId, + normalizedSessionId: priorExecution.session.normalizedSessionId, + runnerInstanceId: "runner-prior-run-scope", + environmentLeaseId: "lease-prior-run-scope", + }), + journalData: "x".repeat(journalBytes), + }, ), ); await writeFile( @@ -5371,7 +5375,7 @@ describe("runnerd provider runtime wiring", () => { }, ); - it.each(["missing", "malformed", "unknown_schema", "mismatched"] as const)( + it.each(["missing", "malformed", "unknown_schema", "mismatched", "oversized"] as const)( "fails closed on %s durable identity in an existing scoped root", async (caseName) => { const stateBase = await mkdtemp( @@ -5425,13 +5429,32 @@ describe("runnerd provider runtime wiring", () => { : JSON.stringify( durableControlPlaneState({ runId: scopedExecution.binding.runId, - normalizedSessionId: "session-owned-by-another-scope", - runnerInstanceId: "runner-owned-by-another-scope", - environmentLeaseId: "lease-owned-by-another-scope", + normalizedSessionId: caseName === "oversized" + ? scopedExecution.session.normalizedSessionId + : "session-owned-by-another-scope", + runnerInstanceId: `runner-${caseName}-scoped-state`, + environmentLeaseId: scopedExecution.binding.executionWorkspaceId, }), ), ); } + if (caseName === "oversized") { + // Valid JSON beyond the bound: without the size guard this exact + // identity and ready runner would otherwise be accepted. + const padding = " ".repeat(1024 * 1024); + for (let i = 0; i < 64; i++) { + await appendFile(join(scopedRoot, "control-plane", "control-plane-state.json"), padding); + } + await mkdir(join(scopedRoot, "runner"), { recursive: true }); + await writeFile(join(scopedRoot, "runner", "runner-state.json"), JSON.stringify( + durableRunnerState({ + runId: scopedExecution.binding.runId, + normalizedSessionId: scopedExecution.session.normalizedSessionId, + runnerInstanceId: `runner-${caseName}-scoped-state`, + environmentLeaseId: scopedExecution.binding.executionWorkspaceId, + }, "ready"), + )); + } state.createBackend.mockClear(); state.createTransport.mockClear(); diff --git a/server/src/services/native-runtime/native-session-executor.ts b/server/src/services/native-runtime/native-session-executor.ts index 06e49ab7d5..19fcc20884 100644 --- a/server/src/services/native-runtime/native-session-executor.ts +++ b/server/src/services/native-runtime/native-session-executor.ts @@ -150,7 +150,10 @@ export async function detachNativeSessionsForRestart( const MAX_REMOTE_CHECKPOINT_ARCHIVE_BYTES = 64 * 1024 * 1024; const MAX_REMOTE_CHECKPOINT_EXPANDED_BYTES = 64 * 1024 * 1024; const MAX_REMOTE_CHECKPOINT_ENTRIES = 20_000; -const NATIVE_DURABLE_IDENTITY_MAX_BYTES = 2 * 1024 * 1024; +// This file contains the PRP journal as well as its identity. Match the +// runner transport's 64 MiB control-plane state bound; ordinary tool output +// can exceed 2 MiB without invalidating the session identity. +const NATIVE_CONTROL_PLANE_STATE_MAX_BYTES = 64 * 1024 * 1024; const NATIVE_RUNNER_STATE_MAX_BYTES = 16 * 1024 * 1024; const NATIVE_WARM_CHECKPOINT_MAX_BYTES = 8 * 1024 * 1024; const CODEX_HOME_NON_PERSISTENT_ENTRIES = [ @@ -1712,7 +1715,7 @@ export function runnerdStateProvesIncompleteBootstrap(root: string): boolean { JSON.parse( readBoundedNativeFile( statePath, - NATIVE_DURABLE_IDENTITY_MAX_BYTES, + NATIVE_CONTROL_PLANE_STATE_MAX_BYTES, "runner_durable_identity_too_large", ).toString("utf8"), ), @@ -1765,7 +1768,7 @@ function readRunnerdDurableIdentity( JSON.parse( readBoundedNativeFile( statePath, - NATIVE_DURABLE_IDENTITY_MAX_BYTES, + NATIVE_CONTROL_PLANE_STATE_MAX_BYTES, "runner_durable_identity_too_large", ).toString("utf8"), ),