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"), ),