diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index 96426bf35b9..a239c243d77 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -1732,8 +1732,11 @@ Files reads use the current committed saved version when remote authority is clean and has no active writer, even while its old allocation remains recorded. The saved loader verifies the company, workspace, snapshot, and archive before reading. Dirty or writer-owned authority uses the live allocation and cannot -substitute an older save if that allocation is unavailable. Reads retain their -authority-change fence and remove private saved-file copies after use. +substitute an older save if that allocation is unavailable. A saved read permits +cleanup to clear its recorded allocation while the epoch, snapshot, source, +clean state, and absent writer stay identical. A new or reintroduced allocation +still invalidates that read. Live reads retain the full allocation fence, and +private saved-file copies are removed after use. Embedders that omit the optional saved-file loader keep valid live reads. They still report unavailability when neither a live location nor a loader exists. diff --git a/maps/agent-access-map/workspaces.md b/maps/agent-access-map/workspaces.md index 551170cf4f7..98bf33bc8be 100644 --- a/maps/agent-access-map/workspaces.md +++ b/maps/agent-access-map/workspaces.md @@ -298,7 +298,9 @@ authorized live allocation or a verified committed saved version. Returned `source.kind` is `live` or `saved`, with `savedAt` for a saved copy. An unavailable clean allocation does not prevent reads of its current committed save once writer authority is clear; saved reads validate the exact company, workspace, -and snapshot even while a remote location remains recorded. An unavailable +and snapshot even while a remote location remains recorded. Cleanup may clear +only that location during a saved read; logical-authority changes and foreign +or reintroduced locations still reject it. Live reads retain their full fence. An unavailable dirty allocation cannot silently fall back to stale saved bytes. Remote paths never become controller filesystem authority. Confined path/symlink checks, bounded search/preview, download streaming, and cancellation remain enforced by diff --git a/maps/feature-map/workspaces.md b/maps/feature-map/workspaces.md index 95c2ac8a274..f5e0ccc51da 100644 --- a/maps/feature-map/workspaces.md +++ b/maps/feature-map/workspaces.md @@ -106,6 +106,10 @@ After a final save clears dirty state and writer authority, Files must read the current committed saved version even while the old allocation is still recorded or its read transport is unavailable. Confirm the original bytes and Saved copy label; a new writer or dirty state must retain the live-read requirement. +Clear the recorded allocation while a saved preview is loading: the pinned copy +must remain readable. Epoch, snapshot, source, writer, dirty-state, and foreign +or reintroduced location changes must still reject the read. Live reads keep +their physical-location fence, and all private saved copies must be removed. Close must retain dirty/unfinished work and reject custom cleanup/teardown commands; a clean saved workspace can release compute while retaining files. Manual runtime commands, jobs, repairs, and service starts must reject remote diff --git a/server/src/__tests__/remote-workspace-file-resources.test.ts b/server/src/__tests__/remote-workspace-file-resources.test.ts index 393917f0c33..402d0c7f812 100644 --- a/server/src/__tests__/remote-workspace-file-resources.test.ts +++ b/server/src/__tests__/remote-workspace-file-resources.test.ts @@ -345,6 +345,50 @@ describeDatabase("remote workspace ordinary file routes", () => { return { ...f, storage, saved, input: { companyId: f.ids.company, workspaceId: f.ids.workspace, snapshotId: snapshot.id } }; } + it("keeps a production saved preview available when cleanup clears its location during archive loading", async () => { + const f = await savedFileFixture(true); + const requestTemp = await fs.mkdtemp(path.join(os.tmpdir(), "saved-file-location-request-")); + directories.push(requestTemp); + const { createApp } = await import("../app.js"); + const app = await createApp(db, { uiMode: "none", serverPort: 3100, storageService: f.storage, + deploymentMode: "local_trusted", deploymentExposure: "public", bindHost: "127.0.0.1", + allowedHostnames: ["127.0.0.1", "localhost"], authReady: false, companyDeletionEnabled: false, + managedPluginAutoInstall: [], decisionServiceOptions: { wakeOriginAgent: async () => null } }); + const originalTmpdir = process.env.TMPDIR; + process.env.TMPDIR = requestTemp; + let extractedCopy: string | undefined; + const getObject = f.storage.getObject.bind(f.storage); + const retrieve = vi.spyOn(f.storage, "getObject").mockImplementation(async (...args) => { + const object = await getObject(...args); + const copies = (await fs.readdir(requestTemp)).filter(name => name.startsWith("pc-workspace-files-")); + expect(copies).toHaveLength(1); + extractedCopy = path.join(requestTemp, copies[0]!); + // The committed bytes and writer epoch remain unchanged. Only the + // allocation cleanup recorded by production has finished meanwhile. + await db.update(executionWorkspaces).set({ metadata: { remoteWorkspace: { + ...f.authority, snapshotId: f.saved.id, location: null, + } } }).where(eq(executionWorkspaces.id, f.ids.workspace)); + await db.update(environmentLeases).set({ status: "expired", releasedAt: new Date() }) + .where(eq(environmentLeases.id, f.ids.lease)); + return object; + }); + try { + await app.locals.bundledPluginsStartup; + const response = await request(app).get(`/api/issues/${f.ids.issue}/file-resources/content`) + .query({ workspace: "execution", path: "docs/result.md" }); + expect({ status: response.status, code: response.body.details?.code }).toEqual({ status: 200, code: undefined }); + expect(response.body).toMatchObject({ resource: { source: { kind: "saved", snapshotId: f.saved.id } }, + content: { encoding: "utf8", data: "Remote useful result\n" } }); + expect(retrieve).toHaveBeenCalledTimes(1); + expect(extractedCopy).toBeDefined(); + await expect(fs.stat(extractedCopy!)).rejects.toMatchObject({ code: "ENOENT" }); + } finally { + retrieve.mockRestore(); + try { await app.locals.paperclipShutdown(); } + finally { if (originalTmpdir === undefined) delete process.env.TMPDIR; else process.env.TMPDIR = originalTmpdir; } + } + }, 60_000); + it("serves committed saved bytes through ordinary routes while the live allocation is unavailable", async () => { const f = await savedFileFixture(true); f.execute.mockRejectedValue(new Error("synthetic live allocation unavailable")); @@ -447,6 +491,64 @@ describeDatabase("remote workspace ordinary file routes", () => { expect(f.execute).not.toHaveBeenCalled(); expect(f.syncOut).not.toHaveBeenCalled(); }); + it.each(["epoch", "snapshot", "dirty", "writer", "source", "foreign_location"] as const)( + "still fences a saved read when %s changes during extraction", async changed => { + const f = await savedFileFixture(true), load = createSavedWorkspaceLoader(db, f.storage); + let savedRoot: string | undefined; + const loadSavedWorkspace = async (input: Parameters[0]) => { + const saved = await load(input); savedRoot = saved.root; + const next: RemoteWorkspaceAuthority = { ...f.authority, snapshotId: f.saved.id, location: null }; + if (changed === "epoch") next.epoch++; + if (changed === "snapshot") next.snapshotId = randomUUID(); + if (changed === "dirty") next.dirty = true; + if (changed === "writer") next.writerRunId = randomUUID(); + if (changed === "source") next.source = { kind: "git", repoUrl: "https://example.test/other.git", ref: null }; + if (changed === "foreign_location") next.location = { ...f.authority.location!, leaseId: randomUUID() }; + await db.update(executionWorkspaces).set({ metadata: { remoteWorkspace: next } }).where(eq(executionWorkspaces.id, f.ids.workspace)); + return saved; + }; + const remoteFiles = createRemoteWorkspaceFiles(db, { loadSavedWorkspace, environmentRuntime: { execute: f.execute, syncOut: f.syncOut } }); + const response = await request(filesApp(f.ids.company, remoteFiles)).get(`/api/issues/${f.ids.issue}/file-resources/content`) + .query({ workspace: "execution", path: "docs/result.md" }).expect(422); + expect(response.body.details.code).toBe("workspace_changed"); expect(response.body).not.toHaveProperty("content"); + expect(savedRoot).toBeDefined(); await expect(fs.stat(savedRoot!)).rejects.toMatchObject({ code: "ENOENT" }); + expect(f.execute).not.toHaveBeenCalled(); expect(f.syncOut).not.toHaveBeenCalled(); + }, + ); + + it.each([true, false])("rejects a location reappearing on an open saved copy (initial location=%s)", async keepLocation => { + const f = await savedFileFixture(keepLocation); + const remoteFiles = createRemoteWorkspaceFiles(db, { loadSavedWorkspace: createSavedWorkspaceLoader(db, f.storage) }); + const access = await remoteFiles.open({ companyId: f.ids.company, workspaceId: f.ids.workspace }); + expect(access?.source.kind).toBe("saved"); + try { + await db.update(executionWorkspaces).set({ metadata: { remoteWorkspace: { ...f.authority, snapshotId: f.saved.id, location: null } } }) + .where(eq(executionWorkspaces.id, f.ids.workspace)); + expect((await access!.stat("docs/result.md")).kind).toBe("file"); + await db.update(executionWorkspaces).set({ metadata: { remoteWorkspace: { ...f.authority, snapshotId: f.saved.id } } }) + .where(eq(executionWorkspaces.id, f.ids.workspace)); + await expect(access!.stat("docs/result.md")).rejects.toMatchObject({ status: 422, details: { code: "workspace_changed" } }); + } finally { await access?.close(); } + }); + + it("keeps a live read fenced when its physical location is cleared", async () => { + const f = await savedFileFixture(true), live = { ...f.authority, snapshotId: f.saved.id, dirty: true }; + await db.update(executionWorkspaces).set({ metadata: { remoteWorkspace: live } }).where(eq(executionWorkspaces.id, f.ids.workspace)); + const execute = f.execute.getMockImplementation()!; + f.execute.mockImplementationOnce(async input => { + const result = await execute(input); + await db.update(executionWorkspaces).set({ metadata: { remoteWorkspace: { ...live, location: null } } }) + .where(eq(executionWorkspaces.id, f.ids.workspace)); + return result; + }); + const loadSavedWorkspace = vi.fn(createSavedWorkspaceLoader(db, f.storage)); + const remoteFiles = createRemoteWorkspaceFiles(db, { loadSavedWorkspace, environmentRuntime: { execute: f.execute, syncOut: f.syncOut } }); + const response = await request(filesApp(f.ids.company, remoteFiles)).get(`/api/issues/${f.ids.issue}/file-resources/content`) + .query({ workspace: "execution", path: "docs/result.md" }).expect(422); + expect(response.body.details.code).toBe("workspace_changed"); expect(response.body).not.toHaveProperty("content"); + expect(f.execute).toHaveBeenCalledTimes(1); expect(loadSavedWorkspace).not.toHaveBeenCalled(); + }); + it("shares fail-fast saved Files admission across loader instances until owned cleanup completes", async () => { const f = await savedFileFixture(), availableBytes = async () => 8n * 1024n ** 3n; const first = createSavedWorkspaceLoader(db, f.storage, { availableBytes }); diff --git a/server/src/services/remote-workspace-files.ts b/server/src/services/remote-workspace-files.ts index fd3d5ae3d42..d929e8be3db 100644 --- a/server/src/services/remote-workspace-files.ts +++ b/server/src/services/remote-workspace-files.ts @@ -269,16 +269,27 @@ export function createRemoteWorkspaceFiles(db: Db, options: RemoteWorkspaceFileO const authority = readRemoteWorkspaceAuthority(workspace.metadata); if (!authority) return null; const initialAuthority = JSON.stringify(authority); + let saved: SavedWorkspaceFiles | undefined; + let savedLocationReleased = false; const checkCurrent = async () => { input.signal?.throwIfAborted(); const [current] = await db.select({ metadata: executionWorkspaces.metadata }).from(executionWorkspaces).where(and( eq(executionWorkspaces.companyId, input.companyId), eq(executionWorkspaces.id, input.workspaceId), )).limit(1); - if (!current || JSON.stringify(readRemoteWorkspaceAuthority(current.metadata)) !== initialAuthority) { + const currentAuthority = current ? readRemoteWorkspaceAuthority(current.metadata) : null; + // An extracted committed save no longer depends on the allocation. + // Allow only its monotonic location removal; every logical field stays + // pinned, and a foreign or reintroduced location remains a conflict. + if (saved && authority.location && currentAuthority?.location === null + && JSON.stringify({ ...currentAuthority, location: authority.location }) === initialAuthority) { + savedLocationReleased = true; + return; + } + if (!currentAuthority || JSON.stringify(currentAuthority) !== initialAuthority + || savedLocationReleased && currentAuthority.location !== null) { throw fileFailure("workspace_changed"); } }; - let saved: SavedWorkspaceFiles | undefined; let environment: Environment | undefined; let lease: EnvironmentLease | undefined; let source: WorkspaceFileSource;