mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 13:24:55 +01:00
Validate work product run attribution
This commit is contained in:
1 parent
3b0b3e37c8
commit
86b0fe7d01
4 files changed
+210
-33
No files matched your search
@@ -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", () => {
|
||||
|
||||
@@ -201,35 +201,43 @@ function makeAgent(id: string, overrides: Record<string, unknown> = {}) {
|
||||
|
||||
function createRunContextDb(
|
||||
contextSnapshot: Record<string, unknown> = {},
|
||||
runAgentId: string = ownerAgentId,
|
||||
runAgentOrRows: string | Record<string, unknown>[] = 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<string, unknown>) => {
|
||||
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<string, unknown>) => {
|
||||
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<string, never>) => Promise<unknown>) => callback({}),
|
||||
select: vi.fn((selection: Record<string, unknown> = {}) => ({
|
||||
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",
|
||||
|
||||
@@ -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<string | null | undefined> {
|
||||
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({
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in new issue
Block a user