From 86b0fe7d01c9b9d47767f3f94dab4e2684578f68 Mon Sep 17 00:00:00 2001 From: Dotta Date: Fri, 5 Jun 2026 16:45:12 +0000 Subject: [PATCH] Validate work product run attribution --- .../company-artifacts-service.test.ts | 48 ++++++- ...ue-agent-mutation-ownership-routes.test.ts | 123 ++++++++++++++---- server/src/routes/issues.ts | 56 ++++++++ server/src/services/company-artifacts.ts | 16 ++- 4 files changed, 210 insertions(+), 33 deletions(-) diff --git a/server/src/__tests__/company-artifacts-service.test.ts b/server/src/__tests__/company-artifacts-service.test.ts index 620d17e0bab..0136230cafc 100644 --- a/server/src/__tests__/company-artifacts-service.test.ts +++ b/server/src/__tests__/company-artifacts-service.test.ts @@ -91,6 +91,7 @@ describeEmbeddedPostgres("companyArtifactsService", () => { const secondIssueId = "77777777-7777-4777-8777-777777777777"; const otherIssueId = "88888888-8888-4888-8888-888888888888"; const runId = "99999999-9999-4999-8999-999999999999"; + const otherRunId = "19191919-1919-4191-8191-191919191919"; const directAttachmentId = "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa"; const workProductAttachmentId = "bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb"; @@ -130,12 +131,20 @@ describeEmbeddedPostgres("companyArtifactsService", () => { priority: "medium", }, ]); - await db.insert(heartbeatRuns).values({ - id: runId, - companyId, - agentId, - status: "completed", - }); + await db.insert(heartbeatRuns).values([ + { + id: runId, + companyId, + agentId, + status: "completed", + }, + { + id: otherRunId, + companyId: otherCompanyId, + agentId: otherAgentId, + status: "completed", + }, + ]); await db.insert(documents).values([ { id: "cccccccc-cccc-4ccc-8ccc-cccccccccccc", @@ -320,7 +329,7 @@ describeEmbeddedPostgres("companyArtifactsService", () => { updatedAt: new Date("2026-01-02T12:00:00.000Z"), }); - return { companyId, projectId }; + return { companyId, projectId, issueId, otherRunId }; } it("projects agent-created documents, direct attachments, and work products while excluding noisy sources", async () => { @@ -377,6 +386,31 @@ describeEmbeddedPostgres("companyArtifactsService", () => { }); expect(secondPage.artifacts.map((artifact) => artifact.title)).toEqual(["Primary Cut", "notes.txt"]); }); + + it("does not project a foreign agent from a malformed work product run reference", async () => { + const { companyId, issueId, otherRunId } = await seedArtifacts(); + + await db.insert(issueWorkProducts).values({ + id: "1a1a1a1a-1a1a-4a1a-8a1a-1a1a1a1a1a1a", + companyId, + issueId, + type: "artifact", + provider: "paperclip", + title: "Forged Run Artifact", + status: "ready_for_review", + summary: "Historically malformed run attribution", + metadata: { contentType: "text/plain" }, + createdByRunId: otherRunId, + updatedAt: new Date("2026-01-10T00:00:00.000Z"), + }); + + const result = await companyArtifactsService(db, createStorageService()).list(companyId, { limit: 20 }); + const forged = result.artifacts.find((artifact) => artifact.title === "Forged Run Artifact"); + + expect(forged).toBeTruthy(); + expect(forged?.createdByAgent).toBeNull(); + expect(result.artifacts.some((artifact) => artifact.createdByAgent?.name === "Other")).toBe(false); + }); }); describe("company artifacts route authorization", () => { diff --git a/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts b/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts index 2794a830264..1fed2d477a6 100644 --- a/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts +++ b/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts @@ -201,35 +201,43 @@ function makeAgent(id: string, overrides: Record = {}) { function createRunContextDb( contextSnapshot: Record = {}, - runAgentId: string = ownerAgentId, + runAgentOrRows: string | Record[] = ownerAgentId, runId: string = ownerRunId, ) { + const runRows = Array.isArray(runAgentOrRows) + ? runAgentOrRows + : [{ + id: runId, + companyId, + agentId: runAgentOrRows, + agentCompanyId: companyId, + contextSnapshot, + }]; + const firstRun = runRows[0] ?? {}; + const runAgentId = typeof firstRun.agentId === "string" ? firstRun.agentId : ownerAgentId; + const runAgentCompanyId = typeof firstRun.agentCompanyId === "string" ? firstRun.agentCompanyId : companyId; + const rowsForSelection = (selection: Record) => { + const keys = Object.keys(selection); + if (keys.includes("entityId")) return []; + if (keys.includes("contextSnapshot")) return runRows; + if (keys.includes("agentCompanyId")) return runRows; + return [{ id: runAgentId, companyId: runAgentCompanyId, permissions: {}, role: "engineer", reportsTo: null }]; + }; + const buildQuery = (selection: Record) => { + const whereResult = { + orderBy: vi.fn(async () => []), + then: async (resolve: (rows: unknown[]) => unknown) => resolve(rowsForSelection(selection)), + }; + const query = { + innerJoin: vi.fn(() => query), + where: vi.fn(() => whereResult), + }; + return query; + }; return { transaction: async (callback: (tx: Record) => Promise) => callback({}), select: vi.fn((selection: Record = {}) => ({ - from: vi.fn(() => ({ - where: vi.fn(() => ({ - orderBy: vi.fn(async () => []), - then: async (resolve: (rows: unknown[]) => unknown) => { - const keys = Object.keys(selection); - if (keys.includes("entityId")) { - return resolve([]); - } - if (keys.includes("contextSnapshot")) { - return resolve([{ - id: runId, - companyId, - agentId: runAgentId, - contextSnapshot, - }]); - } - if (keys.includes("permissions")) { - return resolve([{ id: runAgentId, companyId, permissions: {}, role: "engineer", reportsTo: null }]); - } - return resolve([{ id: runAgentId, companyId, permissions: {}, role: "engineer", reportsTo: null }]); - }, - })), - })), + from: vi.fn(() => buildQuery(selection)), })), }; } @@ -622,6 +630,73 @@ describe("agent issue mutation checkout ownership", () => { ); }); + it("stores the authenticated agent run id when creating work products", async () => { + const app = await createApp(ownerActor()); + + await request(app).post(`/api/issues/${issueId}/work-products`).send({ + type: "artifact", + provider: "test", + title: "Artifact", + }).expect(201); + + expect(mockWorkProductService.createForIssue).toHaveBeenCalledWith( + issueId, + companyId, + expect.objectContaining({ createdByRunId: ownerRunId }), + ); + }); + + it("rejects agent-created work products with a forged run id", async () => { + const app = await createApp(ownerActor()); + + const res = await request(app).post(`/api/issues/${issueId}/work-products`).send({ + type: "artifact", + provider: "test", + title: "Artifact", + createdByRunId: "66666666-6666-4666-8666-666666666666", + }); + + expect(res.status, JSON.stringify(res.body)).toBe(403); + expect(res.body.error).toBe("createdByRunId must match the authenticated agent run"); + expect(mockWorkProductService.createForIssue).not.toHaveBeenCalled(); + }); + + it("rejects work product updates with a forged agent run id", async () => { + const app = await createApp(ownerActor()); + + const res = await request(app).patch("/api/work-products/product-1").send({ + createdByRunId: "66666666-6666-4666-8666-666666666666", + }); + + expect(res.status, JSON.stringify(res.body)).toBe(403); + expect(res.body.error).toBe("createdByRunId must match the authenticated agent run"); + expect(mockWorkProductService.update).not.toHaveBeenCalled(); + }); + + it("rejects board-created work products with a foreign-company run id", async () => { + const app = await createApp( + boardActor(), + createRunContextDb({}, [{ + id: "66666666-6666-4666-8666-666666666666", + companyId: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", + agentId: "bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb", + agentCompanyId: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", + contextSnapshot: {}, + }]), + ); + + const res = await request(app).post(`/api/issues/${issueId}/work-products`).send({ + type: "artifact", + provider: "test", + title: "Artifact", + createdByRunId: "66666666-6666-4666-8666-666666666666", + }); + + expect(res.status, JSON.stringify(res.body)).toBe(403); + expect(res.body.error).toBe("createdByRunId is not valid for this company"); + expect(mockWorkProductService.createForIssue).not.toHaveBeenCalled(); + }); + it.each([ [ "work product create", diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index 1136ca15587..d97c73e89cb 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -6,6 +6,7 @@ import { and, desc, eq, inArray, notInArray } from "drizzle-orm"; import type { Db } from "@paperclipai/db"; import { activityLog, + agents, documents, executionWorkspaces, heartbeatRuns, @@ -1926,6 +1927,55 @@ export function issueRoutes( return false; } + async function loadWorkProductRunAttribution(runId: string) { + return await db + .select({ + id: heartbeatRuns.id, + companyId: heartbeatRuns.companyId, + agentId: heartbeatRuns.agentId, + agentCompanyId: agents.companyId, + }) + .from(heartbeatRuns) + .innerJoin(agents, eq(heartbeatRuns.agentId, agents.id)) + .where(eq(heartbeatRuns.id, runId)) + .then((rows) => rows[0] ?? null); + } + + async function resolveWorkProductCreatedByRunId( + req: Request, + res: Response, + companyId: string, + input: { createdByRunId?: string | null }, + mode: "create" | "update", + ): Promise { + const hasCreatedByRunId = Object.prototype.hasOwnProperty.call(input, "createdByRunId"); + if (mode === "update" && !hasCreatedByRunId) return undefined; + + const requestedRunId = input.createdByRunId ?? null; + if (req.actor.type === "agent") { + const actorRunId = req.actor.runId?.trim() || null; + if (requestedRunId && requestedRunId !== actorRunId) { + res.status(403).json({ error: "createdByRunId must match the authenticated agent run" }); + return undefined; + } + if (!actorRunId) return requestedRunId; + const run = await loadWorkProductRunAttribution(actorRunId); + if (!run || run.companyId !== companyId || run.agentCompanyId !== companyId || run.agentId !== req.actor.agentId) { + res.status(403).json({ error: "createdByRunId is not valid for this work product actor" }); + return undefined; + } + return actorRunId; + } + + if (!requestedRunId) return null; + const run = await loadWorkProductRunAttribution(requestedRunId); + if (!run || run.companyId !== companyId || run.agentCompanyId !== companyId) { + res.status(403).json({ error: "createdByRunId is not valid for this company" }); + return undefined; + } + return requestedRunId; + } + function assertStructuredCommentFieldsAllowed( req: Request, res: Response, @@ -3669,6 +3719,9 @@ export function issueRoutes( projectId: req.body.projectId ?? issue.projectId ?? null, sourceTrust: await sourceTrustForActorWrite(issue, actor), }; + const createdByRunId = await resolveWorkProductCreatedByRunId(req, res, issue.companyId, req.body, "create"); + if (createdByRunId === undefined) return; + createInput.createdByRunId = createdByRunId; if (requiresPaperclipAttachmentMetadata(createInput)) { createInput.metadata = await canonicalizePaperclipArtifactMetadata({ issue, @@ -3862,6 +3915,9 @@ export function issueRoutes( if (!(await assertDeliverableMutationAllowedByRunContext(req, res, issue))) return; const actor = getActorInfo(req); const patch = { ...req.body }; + const createdByRunId = await resolveWorkProductCreatedByRunId(req, res, existing.companyId, req.body, "update"); + if (createdByRunId === undefined && Object.prototype.hasOwnProperty.call(req.body, "createdByRunId")) return; + if (createdByRunId !== undefined) patch.createdByRunId = createdByRunId; if (requiresPaperclipAttachmentMetadata(patch, existing)) { if (patch.metadata !== undefined) { patch.metadata = await canonicalizePaperclipArtifactMetadata({ diff --git a/server/src/services/company-artifacts.ts b/server/src/services/company-artifacts.ts index c6556572c29..e2d12592a7b 100644 --- a/server/src/services/company-artifacts.ts +++ b/server/src/services/company-artifacts.ts @@ -267,8 +267,20 @@ export function companyArtifactsService(db: Db, storage?: StorageService) { .from(issueWorkProducts) .innerJoin(issues, eq(issueWorkProducts.issueId, issues.id)) .leftJoin(projects, eq(issues.projectId, projects.id)) - .leftJoin(heartbeatRuns, eq(issueWorkProducts.createdByRunId, heartbeatRuns.id)) - .leftJoin(workProductAgent, eq(heartbeatRuns.agentId, workProductAgent.id)) + .leftJoin( + heartbeatRuns, + and( + eq(issueWorkProducts.createdByRunId, heartbeatRuns.id), + eq(heartbeatRuns.companyId, issueWorkProducts.companyId), + ), + ) + .leftJoin( + workProductAgent, + and( + eq(heartbeatRuns.agentId, workProductAgent.id), + eq(workProductAgent.companyId, issueWorkProducts.companyId), + ), + ) .where(and(...workProductConditions)) .orderBy(desc(issueWorkProducts.updatedAt), desc(workProductArtifactId)) .limit(fetchLimit);