mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-04 19:28:46 +01:00
Harden grouped artifact tenant joins
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
fa4360fe69
commit
f1268ab39b
2 files changed
+176
-13
No files matched your search
@@ -330,7 +330,7 @@ describeEmbeddedPostgres("companyArtifactsService", () => {
|
||||
updatedAt: new Date("2026-01-02T12:00:00.000Z"),
|
||||
});
|
||||
|
||||
return { companyId, projectId, issueId, secondIssueId, otherIssueId, otherRunId };
|
||||
return { companyId, otherCompanyId, projectId, issueId, secondIssueId, otherIssueId, otherRunId };
|
||||
}
|
||||
|
||||
it("projects agent-created documents, direct attachments, and work products while excluding noisy sources", async () => {
|
||||
@@ -497,6 +497,96 @@ describeEmbeddedPostgres("companyArtifactsService", () => {
|
||||
expect(result.artifacts.some((artifact) => artifact.createdByAgent?.name === "Other")).toBe(false);
|
||||
});
|
||||
|
||||
it("does not leak foreign issue or project metadata through malformed artifact link rows", async () => {
|
||||
const { companyId, otherCompanyId, otherIssueId } = await seedArtifacts();
|
||||
const foreignProjectId = "1b1b1b1b-1b1b-4b1b-8b1b-1b1b1b1b1b1b";
|
||||
const malformedAttachmentId = "1c1c1c1c-1c1c-4c1c-8c1c-1c1c1c1c1c1c";
|
||||
|
||||
await db.insert(projects).values({
|
||||
id: foreignProjectId,
|
||||
companyId: otherCompanyId,
|
||||
name: "Foreign Project",
|
||||
status: "in_progress",
|
||||
});
|
||||
await db.update(issues).set({ projectId: foreignProjectId }).where(eq(issues.id, otherIssueId));
|
||||
await db.insert(documents).values({
|
||||
id: "1d1d1d1d-1d1d-4d1d-8d1d-1d1d1d1d1d1d",
|
||||
companyId,
|
||||
title: "Forged Link Document",
|
||||
latestBody: "This row is company-owned but points at a foreign issue.",
|
||||
createdByAgentId: "33333333-3333-4333-8333-333333333333",
|
||||
updatedAt: new Date("2026-01-30T00:00:00.000Z"),
|
||||
});
|
||||
await db.insert(issueDocuments).values({
|
||||
companyId,
|
||||
issueId: otherIssueId,
|
||||
documentId: "1d1d1d1d-1d1d-4d1d-8d1d-1d1d1d1d1d1d",
|
||||
key: "forged-link-document",
|
||||
});
|
||||
await db.insert(assets).values({
|
||||
id: "1e1e1e1e-1e1e-4e1e-8e1e-1e1e1e1e1e1e",
|
||||
companyId,
|
||||
provider: "local_disk",
|
||||
objectKey: "forged-link.txt",
|
||||
contentType: "text/plain",
|
||||
byteSize: 42,
|
||||
sha256: "sha256-forged-link",
|
||||
originalFilename: "forged-link.txt",
|
||||
createdByAgentId: "33333333-3333-4333-8333-333333333333",
|
||||
});
|
||||
await db.insert(issueAttachments).values({
|
||||
id: malformedAttachmentId,
|
||||
companyId,
|
||||
issueId: otherIssueId,
|
||||
assetId: "1e1e1e1e-1e1e-4e1e-8e1e-1e1e1e1e1e1e",
|
||||
updatedAt: new Date("2026-01-29T00:00:00.000Z"),
|
||||
});
|
||||
await db.insert(issueWorkProducts).values({
|
||||
id: "1f1f1f1f-1f1f-4f1f-8f1f-1f1f1f1f1f1f",
|
||||
companyId,
|
||||
issueId: otherIssueId,
|
||||
type: "artifact",
|
||||
provider: "paperclip",
|
||||
title: "Forged Link Work Product",
|
||||
status: "ready_for_review",
|
||||
summary: "This row is company-owned but points at a foreign issue.",
|
||||
metadata: { contentType: "text/plain" },
|
||||
createdByRunId: "99999999-9999-4999-8999-999999999999",
|
||||
updatedAt: new Date("2026-01-28T00:00:00.000Z"),
|
||||
});
|
||||
|
||||
const flat = await companyArtifactsService(db, createStorageService()).list(companyId, { limit: 20 });
|
||||
expect(flat.artifacts.map((artifact) => artifact.title)).not.toEqual(expect.arrayContaining([
|
||||
"Forged Link Document",
|
||||
"forged-link.txt",
|
||||
"Forged Link Work Product",
|
||||
]));
|
||||
expect(flat.artifacts.some((artifact) => artifact.issue.identifier === "OTH-1")).toBe(false);
|
||||
expect(flat.artifacts.some((artifact) => artifact.issue.title === "Other output")).toBe(false);
|
||||
expect(flat.artifacts.some((artifact) => artifact.project?.name === "Foreign Project")).toBe(false);
|
||||
|
||||
const grouped = await companyArtifactsService(db, createStorageService()).list(companyId, {
|
||||
groupBy: "task",
|
||||
limit: 20,
|
||||
});
|
||||
expect(grouped.groups?.some((group) => group.issue.identifier === "OTH-1")).toBe(false);
|
||||
expect(grouped.groups?.some((group) => group.issue.title === "Other output")).toBe(false);
|
||||
expect(grouped.groups?.some((group) =>
|
||||
group.previewArtifacts.some((artifact) => artifact.project?.name === "Foreign Project")
|
||||
)).toBe(false);
|
||||
|
||||
const selectedForeignGroup = await companyArtifactsService(db, createStorageService()).list(companyId, {
|
||||
groupBy: "task",
|
||||
groupIssueId: otherIssueId,
|
||||
limit: 20,
|
||||
});
|
||||
expect(selectedForeignGroup).toEqual({
|
||||
artifacts: [],
|
||||
selectedGroup: null,
|
||||
nextCursor: null,
|
||||
});
|
||||
});
|
||||
|
||||
it("groups artifacts by task after applying media, project, and search filters", async () => {
|
||||
const { companyId, projectId, issueId } = await seedArtifacts();
|
||||
const storage = createStorageService({ "notes.txt": Buffer.from("Searchable notes preview") });
|
||||
|
||||
@@ -322,6 +322,7 @@ export function companyArtifactsService(db: Db, storage?: StorageService) {
|
||||
const updatedAgent = alias(agents, "document_updated_agent");
|
||||
const documentArtifactId = sql<string>`concat('document:', ${documents.id})`;
|
||||
const documentConditions: SQL[] = [
|
||||
eq(issueDocuments.companyId, companyId),
|
||||
eq(documents.companyId, companyId),
|
||||
or(isNotNull(documents.createdByAgentId), isNotNull(documents.updatedByAgentId))!,
|
||||
notInArray(issueDocuments.key, [...SYSTEM_ISSUE_DOCUMENT_KEYS]),
|
||||
@@ -355,11 +356,41 @@ export function companyArtifactsService(db: Db, storage?: StorageService) {
|
||||
updatedAt: documents.updatedAt,
|
||||
})
|
||||
.from(issueDocuments)
|
||||
.innerJoin(documents, eq(issueDocuments.documentId, documents.id))
|
||||
.innerJoin(issues, eq(issueDocuments.issueId, issues.id))
|
||||
.leftJoin(projects, eq(issues.projectId, projects.id))
|
||||
.leftJoin(createdAgent, eq(documents.createdByAgentId, createdAgent.id))
|
||||
.leftJoin(updatedAgent, eq(documents.updatedByAgentId, updatedAgent.id))
|
||||
.innerJoin(
|
||||
documents,
|
||||
and(
|
||||
eq(issueDocuments.documentId, documents.id),
|
||||
eq(documents.companyId, issueDocuments.companyId),
|
||||
),
|
||||
)
|
||||
.innerJoin(
|
||||
issues,
|
||||
and(
|
||||
eq(issueDocuments.issueId, issues.id),
|
||||
eq(issues.companyId, issueDocuments.companyId),
|
||||
),
|
||||
)
|
||||
.leftJoin(
|
||||
projects,
|
||||
and(
|
||||
eq(issues.projectId, projects.id),
|
||||
eq(projects.companyId, issues.companyId),
|
||||
),
|
||||
)
|
||||
.leftJoin(
|
||||
createdAgent,
|
||||
and(
|
||||
eq(documents.createdByAgentId, createdAgent.id),
|
||||
eq(createdAgent.companyId, documents.companyId),
|
||||
),
|
||||
)
|
||||
.leftJoin(
|
||||
updatedAgent,
|
||||
and(
|
||||
eq(documents.updatedByAgentId, updatedAgent.id),
|
||||
eq(updatedAgent.companyId, documents.companyId),
|
||||
),
|
||||
)
|
||||
.where(and(...documentConditions))
|
||||
.orderBy(desc(documents.updatedAt), desc(documentArtifactId));
|
||||
const documentRows = groupBy ? await documentRowsQuery : await documentRowsQuery.limit(fetchLimit);
|
||||
@@ -439,8 +470,20 @@ export function companyArtifactsService(db: Db, storage?: StorageService) {
|
||||
updatedAt: issueWorkProducts.updatedAt,
|
||||
})
|
||||
.from(issueWorkProducts)
|
||||
.innerJoin(issues, eq(issueWorkProducts.issueId, issues.id))
|
||||
.leftJoin(projects, eq(issues.projectId, projects.id))
|
||||
.innerJoin(
|
||||
issues,
|
||||
and(
|
||||
eq(issueWorkProducts.issueId, issues.id),
|
||||
eq(issues.companyId, issueWorkProducts.companyId),
|
||||
),
|
||||
)
|
||||
.leftJoin(
|
||||
projects,
|
||||
and(
|
||||
eq(issues.projectId, projects.id),
|
||||
eq(projects.companyId, issueWorkProducts.companyId),
|
||||
),
|
||||
)
|
||||
.leftJoin(
|
||||
heartbeatRuns,
|
||||
and(
|
||||
@@ -464,7 +507,13 @@ export function companyArtifactsService(db: Db, storage?: StorageService) {
|
||||
attachmentId: sql<string | null>`${issueWorkProducts.metadata}->>'attachmentId'`,
|
||||
})
|
||||
.from(issueWorkProducts)
|
||||
.innerJoin(issues, eq(issueWorkProducts.issueId, issues.id))
|
||||
.innerJoin(
|
||||
issues,
|
||||
and(
|
||||
eq(issueWorkProducts.issueId, issues.id),
|
||||
eq(issues.companyId, issueWorkProducts.companyId),
|
||||
),
|
||||
)
|
||||
.where(and(...workProductBaseConditions, sql`${issueWorkProducts.metadata}->>'attachmentId' IS NOT NULL`));
|
||||
|
||||
for (const row of workProductAttachmentRows) {
|
||||
@@ -542,10 +591,34 @@ export function companyArtifactsService(db: Db, storage?: StorageService) {
|
||||
updatedAt: issueAttachments.updatedAt,
|
||||
})
|
||||
.from(issueAttachments)
|
||||
.innerJoin(assets, eq(issueAttachments.assetId, assets.id))
|
||||
.innerJoin(issues, eq(issueAttachments.issueId, issues.id))
|
||||
.leftJoin(projects, eq(issues.projectId, projects.id))
|
||||
.leftJoin(attachmentAgent, eq(assets.createdByAgentId, attachmentAgent.id))
|
||||
.innerJoin(
|
||||
assets,
|
||||
and(
|
||||
eq(issueAttachments.assetId, assets.id),
|
||||
eq(assets.companyId, issueAttachments.companyId),
|
||||
),
|
||||
)
|
||||
.innerJoin(
|
||||
issues,
|
||||
and(
|
||||
eq(issueAttachments.issueId, issues.id),
|
||||
eq(issues.companyId, issueAttachments.companyId),
|
||||
),
|
||||
)
|
||||
.leftJoin(
|
||||
projects,
|
||||
and(
|
||||
eq(issues.projectId, projects.id),
|
||||
eq(projects.companyId, issues.companyId),
|
||||
),
|
||||
)
|
||||
.leftJoin(
|
||||
attachmentAgent,
|
||||
and(
|
||||
eq(assets.createdByAgentId, attachmentAgent.id),
|
||||
eq(attachmentAgent.companyId, assets.companyId),
|
||||
),
|
||||
)
|
||||
.where(and(...attachmentConditions))
|
||||
.orderBy(desc(issueAttachments.updatedAt), desc(attachmentArtifactId));
|
||||
const attachmentRows = groupBy ? await attachmentRowsQuery : await attachmentRowsQuery.limit(fetchLimit);
|
||||
|
||||
Reference in new issue
Block a user