diff --git a/server/src/__tests__/task-workspace-repositories-trust.test.ts b/server/src/__tests__/task-workspace-repositories-trust.test.ts index 06dcdfaa8c2..b54a9b527b2 100644 --- a/server/src/__tests__/task-workspace-repositories-trust.test.ts +++ b/server/src/__tests__/task-workspace-repositories-trust.test.ts @@ -185,12 +185,18 @@ const support = await getEmbeddedPostgresTestSupport(); expect(await db.select().from(executionWorkspaceRepositoryAccess).where(eq(executionWorkspaceRepositoryAccess.companyId, f.companyId))).toEqual(beforeRead); }); - it("preserves legacy private publishers before moving their workspace away from shared files", async () => { + it.each(["update", "archive"] as const)("preserves legacy private publishers before %s moves their workspace away from shared files", async operation => { const f = await fixture(), peer = await standardPeer(f); const repository = { kind: "url" as const, url: "https://github.com/public/legacy-shared" }; await f.service.request({ companyId: f.companyId, issueId: peer.issueId, actor: peer.actor, request: { repository, requestKey: randomUUID() } }); await privateTask(f); + const [project] = await db.insert(projects).values({ companyId: f.companyId, name: "Private legacy source", visibility: "private" }).returning(); + await db.insert(projectAccessMembers).values([ + { companyId: f.companyId, projectId: project.id, subjectType: "agent", subjectId: f.agentId }, + { companyId: f.companyId, projectId: project.id, subjectType: "user", subjectId: f.userId }, + ]); + await db.update(executionWorkspaces).set({ projectId: project.id, sourceIssueId: f.issueId }).where(eq(executionWorkspaces.id, f.workspace.id)); await f.request(repository); await localPrivateClone(f); const [checkout] = await f.service.prepareForAdmission(f.admission); @@ -200,12 +206,24 @@ const support = await getEmbeddedPostgresTestSupport(); await db.delete(executionWorkspaceRepositoryAccess).where(eq(executionWorkspaceRepositoryAccess.companyId, f.companyId)); const moved = path.join(root, `${f.issueId}-legacy-moved`); await mkdir(moved); - await executionWorkspaceService(db).update(f.workspace.id, { cwd: moved }); + const patch = { cwd: moved, projectId: null, sourceIssueId: null, metadata: {} }; + const svc = executionWorkspaceService(db); + if (operation === "archive") { + const result = await svc.archiveWorkspaceUnderLifecycleLock({ id: f.workspace.id, patch, closedAt: new Date() }); + expect(result?.outcome).toBe("archived"); + if (result?.outcome === "archived") { + expect(result.capturedGeneration).toBeGreaterThan(0); + expect(result.workspace.status).toBe("archived"); + expect(result.workspace.metadata?._issuePrivacySources).toHaveProperty(f.issueId); + } + } else await svc.update(f.workspace.id, patch); + await db.update(issues).set({ executionWorkspaceId: null }).where(eq(issues.id, f.issueId)); await expect(assertExecutionWorkspaceRootReadAccess(db, { companyId: f.companyId, cwd: peer.admission.cwd, - sourceWorkspaceId: peer.workspace.id, actor: peer.actor })).rejects.toThrow(/workspace access|authorization boundary/); - await expect(f.service.prepareForAdmission(peer.admission)).rejects.toThrow(/workspace access|authorization boundary/); + sourceWorkspaceId: peer.workspace.id, actor: peer.actor })).rejects.toThrow(/workspace access|authorization boundary|source project/); + await expect(f.service.prepareForAdmission(peer.admission)).rejects.toThrow(/workspace access|authorization boundary|source project/); const retained = await db.select().from(executionWorkspaceRepositoryAccess).where(eq(executionWorkspaceRepositoryAccess.companyId, f.companyId)); expect(retained.some(entry => entry.kind === "source" && entry.scopeId === f.workspace.id)).toBe(true); + expect(retained.some(entry => entry.kind === "project_source" && entry.scopeId === project.id)).toBe(true); expect(retained.some(entry => entry.kind === "repository" || entry.kind === "reader")).toBe(false); expect(await readFile(path.join(checkout.cwd, "private.txt"), "utf8")).toBe("Private task edits\n"); }); diff --git a/server/src/services/execution-workspaces.ts b/server/src/services/execution-workspaces.ts index a869a3c63fd..85e44aa9a70 100644 --- a/server/src/services/execution-workspaces.ts +++ b/server/src/services/execution-workspaces.ts @@ -234,6 +234,34 @@ async function acquireExecutionWorkspaceLifecycleLock( ); } +const workspaceSourceKeys = ["cwd", "projectId", "projectWorkspaceId", "sourceIssueId"] as const; + +/** Caller holds the company root fence and the current workspace row lock. */ +async function preserveWorkspaceOwnershipForPatch( + tx: DbTransaction, current: typeof executionWorkspaces.$inferSelect, patch: Partial, +) { + if (workspaceSourceKeys.some(key => patch[key] !== undefined && patch[key] !== current[key])) { + await preserveExecutionWorkspaceRootProvenance(tx, { + companyId: current.companyId, workspaceId: current.id, cwd: current.cwd, + }); + } + const nextPatch = { ...patch }; + const retainedSources = { + ...parseObject(parseObject(patch.metadata)._issuePrivacySources), + ...parseObject(current.metadata?._issuePrivacySources), + }; + if (patch.sourceIssueId !== undefined && patch.sourceIssueId !== current.sourceIssueId && current.sourceIssueId) { + retainedSources[current.sourceIssueId] = true; + } + if (Object.keys(retainedSources).length > 0) { + nextPatch.metadata = { + ...(patch.metadata !== undefined ? patch.metadata ?? {} : current.metadata ?? {}), + _issuePrivacySources: retainedSources, + }; + } + return nextPatch; +} + export type ReopenClosedIsolatedExecutionWorkspaceResult = // `generation` is the lifecycle generation the reopen published the active row // at. It owns the reopen-pending flag. A later clear must present this same @@ -3189,8 +3217,7 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic }, update: async (id: string, patch: Partial) => { - const sourceKeys = ["cwd", "projectId", "projectWorkspaceId", "sourceIssueId"] as const; - if (patch.metadata === undefined && !sourceKeys.some(key => patch[key] !== undefined)) { + if (patch.metadata === undefined && !workspaceSourceKeys.some(key => patch[key] !== undefined)) { const row = await db.update(executionWorkspaces).set({ ...patch, updatedAt: new Date() }) .where(eq(executionWorkspaces.id, id)).returning().then(rows => rows[0] ?? null); return row ? toExecutionWorkspace(row) : null; @@ -3206,25 +3233,7 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic eq(executionWorkspaces.id, id), eq(executionWorkspaces.companyId, identity.companyId), )).for("no key update"); if (!current) return null; - if (sourceKeys.some(key => patch[key] !== undefined && patch[key] !== current[key])) { - await preserveExecutionWorkspaceRootProvenance(tx, { - companyId: current.companyId, workspaceId: current.id, cwd: current.cwd, - }); - } - const nextPatch = { ...patch }; - const retainedSources = { - ...parseObject(parseObject(patch.metadata)._issuePrivacySources), - ...parseObject(current.metadata?._issuePrivacySources), - }; - if (patch.sourceIssueId !== undefined && patch.sourceIssueId !== current.sourceIssueId && current.sourceIssueId) { - retainedSources[current.sourceIssueId] = true; - } - if (Object.keys(retainedSources).length > 0) { - nextPatch.metadata = { - ...(patch.metadata !== undefined ? patch.metadata ?? {} : current.metadata ?? {}), - _issuePrivacySources: retainedSources, - }; - } + const nextPatch = await preserveWorkspaceOwnershipForPatch(tx, current, patch); const [row] = await tx.update(executionWorkspaces).set({ ...nextPatch, updatedAt: new Date() }) .where(and(eq(executionWorkspaces.id, id), eq(executionWorkspaces.companyId, current.companyId))).returning(); return row ? toExecutionWorkspace(row) : null; @@ -3588,12 +3597,17 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic | { outcome: "reopen_pending" } | null > => { + const [identity] = await db.select({ companyId: executionWorkspaces.companyId }).from(executionWorkspaces) + .where(eq(executionWorkspaces.id, input.id)); + if (!identity) return null; return db.transaction(async (tx) => { + await lockExecutionWorkspaceRootProvenance(tx, identity.companyId); await acquireExecutionWorkspaceLifecycleLock(tx, input.id); const fresh = await tx .select() .from(executionWorkspaces) - .where(eq(executionWorkspaces.id, input.id)) + .where(and(eq(executionWorkspaces.id, input.id), eq(executionWorkspaces.companyId, identity.companyId))) + .for("no key update") .then((rows) => rows[0] ?? null); if (!fresh || isClosedExecutionWorkspaceStatus(fresh.status)) { // The row is missing or already closed by a concurrent path. Do not @@ -3607,8 +3621,9 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic // rebuilt worktree. The route maps this to HTTP 409. return { outcome: "reopen_pending" }; } + const nextPatch = await preserveWorkspaceOwnershipForPatch(tx, fresh, input.patch); const baseMetadata = - (input.patch.metadata as Record | null | undefined) + (nextPatch.metadata as Record | null | undefined) ?? (fresh.metadata as Record | null); const archiveMetadata = clearMetadataReopenPendingConsumption( bumpExecutionWorkspaceLifecycleGeneration(baseMetadata), @@ -3616,7 +3631,7 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic const archived = await tx .update(executionWorkspaces) .set({ - ...input.patch, + ...nextPatch, status: "archived", closedAt: input.closedAt, cleanupReason: null, @@ -3625,6 +3640,7 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic }) .where(and( eq(executionWorkspaces.id, input.id), + eq(executionWorkspaces.companyId, fresh.companyId), // Defense in depth: never archive a reopen-pending row even if the // flag appears between the read above and this write. The lifecycle // lock already serializes reopen and archive, so this predicate only