mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 05:11:49 +01:00
fix(search): apply task privacy to artifact results (#20595)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Private tasks are visible only to the people and agents that have access > - Company search already filters task results by the caller's task read condition > - Artifact results (agent documents, work products and attachments) in search, and the company artifacts list, did not use that condition > - So a company member could find a private task's document title and snippet through search > - This pull request applies the same task read condition to artifact results > - The benefit is that private task outputs stay private in search and in the artifacts list ## Linked Issues or Issue Description **What happened** An agent saved a document on a private task. A different company member without access to that task searched for a word in the document. The search returned the document as an artifact result with a snippet. **Expected behavior** Search and the artifacts list show artifacts only from tasks the caller can read. **Steps to reproduce** 1. Create a private task and let an agent save a document on it. 2. Sign in as a company member who cannot read the task. 3. Search for a word in the document, or open the company artifacts list. 4. See the private document in the results. ## What Changed - `company-search.ts`: artifact counts and artifact rows apply `options.issueReadCondition`. - `routes/companies.ts`: `GET /companies/:companyId/artifacts` passes the caller's `issueReadSqlCondition` to the artifacts service. ## Verification - Red then green: `cd server && npx vitest run src/__tests__/company-search-service.test.ts -t "private task artifacts"` (fails on `master` with the private document in the outsider's results). - Red then green: `npx vitest run src/__tests__/company-artifacts-service.test.ts -t "caller can read"`. - `npx vitest run src/__tests__/company-search-service.test.ts src/__tests__/company-search-extract-service.test.ts src/__tests__/company-search-extract-routes.test.ts src/__tests__/company-artifacts-service.test.ts` (all pass). - `tsc --noEmit -p server`: no errors in the changed files. ## Risks - Low risk. The read condition is the same one task results use. Callers with full access (local board, instance admin, privacy mode off) get `true` and see no change. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
491dfa1c6d
commit
accf22076f
5 files changed
+78
-7
No files matched your search
@@ -1,6 +1,6 @@
|
||||
import { Readable } from "node:stream";
|
||||
import express from "express";
|
||||
import { eq } from "drizzle-orm";
|
||||
import { eq, sql } from "drizzle-orm";
|
||||
import request from "supertest";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
@@ -405,6 +405,42 @@ describeEmbeddedPostgres("companyArtifactsService", () => {
|
||||
expect(incompatibleKind.body).toEqual({ artifacts: [], nextCursor: null });
|
||||
});
|
||||
|
||||
it("lists only artifacts on tasks the caller can read", async () => {
|
||||
const previousMode = process.env.PAPERCLIP_ISSUE_PRIVACY_MODE;
|
||||
process.env.PAPERCLIP_ISSUE_PRIVACY_MODE = "enforce";
|
||||
try {
|
||||
const { companyId, issueId } = await seedArtifacts();
|
||||
await db.update(issues).set({ visibility: "private", privacyRootIssueId: sql`${issues.id}`, responsibleUserId: "owner-user" })
|
||||
.where(eq(issues.companyId, companyId));
|
||||
const list = async (userId: string, search = "limit=50") => {
|
||||
const app = express();
|
||||
app.use((req, _res, next) => {
|
||||
req.actor = { type: "board", userId, source: "session", companyIds: [companyId],
|
||||
memberships: [{ companyId, membershipRole: "viewer", status: "active" }] };
|
||||
next();
|
||||
});
|
||||
app.use("/api/companies", companyRoutes(db, createStorageService()));
|
||||
app.use(errorHandler);
|
||||
return request(app).get(`/api/companies/${companyId}/artifacts?${search}`);
|
||||
};
|
||||
const outsider = await list("user-1");
|
||||
expect(outsider.status).toBe(200);
|
||||
expect(outsider.body.artifacts).toEqual([]);
|
||||
expect((await list("owner-user")).body.artifacts.length).toBeGreaterThan(0);
|
||||
// A task id in the group selector reveals nothing about an unreadable task.
|
||||
for (const groupBy of ["task", "parent_task"]) {
|
||||
const grouped = await list("user-1", `groupBy=${groupBy}&groupIssueId=${issueId}`);
|
||||
expect(grouped.status).toBe(200);
|
||||
expect(grouped.body).toEqual({ artifacts: [], selectedGroup: null, nextCursor: null });
|
||||
expect((await list("user-1", `groupBy=${groupBy}`)).body.artifacts).toEqual([]);
|
||||
}
|
||||
expect((await list("owner-user", `groupBy=task&groupIssueId=${issueId}`)).body.selectedGroup).toMatchObject({ issue: { id: issueId } });
|
||||
} finally {
|
||||
if (previousMode === undefined) delete process.env.PAPERCLIP_ISSUE_PRIVACY_MODE;
|
||||
else process.env.PAPERCLIP_ISSUE_PRIVACY_MODE = previousMode;
|
||||
}
|
||||
});
|
||||
|
||||
it("returns an empty starred view for agent callers", async () => {
|
||||
const { companyId, otherRunId } = await seedArtifacts();
|
||||
await db.insert(documentMemberships).values({
|
||||
|
||||
@@ -394,6 +394,37 @@ describeEmbeddedPostgres("companySearchService", () => {
|
||||
expect(result.countsByType.document).toBe(1);
|
||||
});
|
||||
|
||||
it("keeps private task artifacts out of an unrelated member's search", async () => {
|
||||
const previousMode = process.env.PAPERCLIP_ISSUE_PRIVACY_MODE;
|
||||
process.env.PAPERCLIP_ISSUE_PRIVACY_MODE = "enforce";
|
||||
try {
|
||||
const companyId = await createCompany();
|
||||
const agentId = await createAgent(companyId, { name: "Private Writer" });
|
||||
const ownerId = `owner-${randomUUID()}`, outsiderId = `outsider-${randomUUID()}`;
|
||||
const privateId = randomUUID();
|
||||
await createIssue(companyId, { id: privateId, title: "Confidential work", visibility: "private",
|
||||
privacyRootIssueId: privateId, responsibleUserId: ownerId });
|
||||
const documentId = randomUUID();
|
||||
await db.insert(documents).values({ id: documentId, companyId, title: "Confidential draft",
|
||||
latestBody: "Launch code nightjar-secret for internal review.", format: "markdown", createdByAgentId: agentId });
|
||||
await db.insert(issueDocuments).values({ companyId, issueId: privateId, documentId, key: "announcement" });
|
||||
const conditionFor = async (userId: string) => ({
|
||||
issueReadCondition: await issueReadSqlCondition(db, { type: "board", userId, source: "session" }),
|
||||
});
|
||||
const query = companySearchQuerySchema.parse({ q: "nightjar-secret" });
|
||||
|
||||
const outsider = await svc.search(companyId, query, await conditionFor(outsiderId));
|
||||
expect(outsider.results.filter(row => row.type === "artifact")).toEqual([]);
|
||||
expect(outsider.countsByType.artifact).toBe(0);
|
||||
const owner = await svc.search(companyId, query, await conditionFor(ownerId));
|
||||
expect(owner.results.filter(row => row.type === "artifact")).toHaveLength(1);
|
||||
expect(owner.countsByType.artifact).toBe(1);
|
||||
} finally {
|
||||
if (previousMode === undefined) delete process.env.PAPERCLIP_ISSUE_PRIVACY_MODE;
|
||||
else process.env.PAPERCLIP_ISSUE_PRIVACY_MODE = previousMode;
|
||||
}
|
||||
});
|
||||
|
||||
it("searches artifact projections through the artifacts scope", async () => {
|
||||
const companyId = await createCompany();
|
||||
const agentId = await createAgent(companyId, { name: "Artifact Writer" });
|
||||
|
||||
@@ -35,7 +35,7 @@ import {
|
||||
type CompanyImportTransferStatus,
|
||||
} from "@paperclipai/shared/company-import-transfer";
|
||||
import { badRequest, conflict, forbidden, notFound, unprocessable } from "../errors.js";
|
||||
import { authorizationDeniedDetails } from "../services/authorization.js";
|
||||
import { authorizationDeniedDetails, issueReadSqlCondition } from "../services/authorization.js";
|
||||
import { PORTABLE_ZIP_UPLOAD_LIMIT_BYTES } from "../http/body-limits.js";
|
||||
import { logger } from "../middleware/logger.js";
|
||||
import { validate } from "../middleware/validate.js";
|
||||
@@ -427,6 +427,7 @@ export function companyRoutes(db: Db, storage?: StorageService, options?: Compan
|
||||
const query = companyArtifactsQuerySchema.parse(req.query);
|
||||
res.json(await artifacts.list(companyId, query, {
|
||||
userId: query.starred && req.actor.type === "board" ? req.actor.userId : undefined,
|
||||
readCondition: await issueReadSqlCondition(db, req.actor),
|
||||
}));
|
||||
});
|
||||
|
||||
|
||||
@@ -195,7 +195,7 @@ function pageByCursor<T extends { id: string; updatedAt: string }>(
|
||||
return { page, nextCursor };
|
||||
}
|
||||
|
||||
async function loadIssueGroupingRows(db: Db, companyId: string, seedIssueIds: Iterable<string>) {
|
||||
async function loadIssueGroupingRows(db: Db, companyId: string, seedIssueIds: Iterable<string>, readCondition?: SQL) {
|
||||
const rowsById = new Map<string, IssueGroupingRow>();
|
||||
let pending = [...new Set(seedIssueIds)];
|
||||
|
||||
@@ -209,7 +209,7 @@ async function loadIssueGroupingRows(db: Db, companyId: string, seedIssueIds: It
|
||||
updatedAt: issues.updatedAt,
|
||||
})
|
||||
.from(issues)
|
||||
.where(and(eq(issues.companyId, companyId), inArray(issues.id, pending)));
|
||||
.where(and(eq(issues.companyId, companyId), inArray(issues.id, pending), readCondition));
|
||||
|
||||
const nextPending = new Set<string>();
|
||||
for (const row of rows) {
|
||||
@@ -321,7 +321,8 @@ export function companyArtifactsService(db: Db, storage?: StorageService) {
|
||||
list: async (
|
||||
companyId: string,
|
||||
rawQuery: Partial<CompanyArtifactsQuery> = {},
|
||||
options: { issueConditions?: SQL[]; userId?: string } = {},
|
||||
/** `readCondition` limits artifacts and group tasks to tasks the caller can read. */
|
||||
options: { issueConditions?: SQL[]; readCondition?: SQL; userId?: string } = {},
|
||||
): Promise<CompanyArtifactsResponse> => {
|
||||
const query = companyArtifactsQuerySchema.parse(rawQuery);
|
||||
const cursor = decodeCursor(query.cursor);
|
||||
@@ -344,6 +345,7 @@ export function companyArtifactsService(db: Db, storage?: StorageService) {
|
||||
isNull(issues.hiddenAt),
|
||||
isNull(issues.harnessKind),
|
||||
...(options.issueConditions ?? []),
|
||||
...(options.readCondition ? [options.readCondition] : []),
|
||||
];
|
||||
const artifacts: CompanyArtifact[] = [];
|
||||
const artifactSortDates = new Map<string, string>();
|
||||
@@ -742,7 +744,7 @@ export function companyArtifactsService(db: Db, storage?: StorageService) {
|
||||
|
||||
const issueSeedIds = new Set(artifacts.map((artifact) => artifact.issue.id));
|
||||
if (query.groupIssueId) issueSeedIds.add(query.groupIssueId);
|
||||
const issueRows = await loadIssueGroupingRows(db, companyId, issueSeedIds);
|
||||
const issueRows = await loadIssueGroupingRows(db, companyId, issueSeedIds, options.readCondition);
|
||||
const groups = buildArtifactGroups({
|
||||
artifacts: sorted,
|
||||
companyPrefix: company.issuePrefix,
|
||||
|
||||
@@ -951,6 +951,7 @@ export function companySearchService(db: Db) {
|
||||
eq(issues.companyId, companyId),
|
||||
visibleIssueCondition(),
|
||||
...artifactIssueFilters,
|
||||
...(options?.issueReadCondition ? [options.issueReadCondition] : []),
|
||||
];
|
||||
const documentArtifactConditions = [
|
||||
eq(issueDocuments.companyId, companyId),
|
||||
@@ -1035,7 +1036,7 @@ export function companySearchService(db: Db) {
|
||||
const result = await companyArtifactsService(db).list(companyId, {
|
||||
q: normalizedQuery.slice(0, COMPANY_ARTIFACTS_MAX_QUERY_LENGTH),
|
||||
limit: Math.min(fetchLimit, COMPANY_ARTIFACTS_MAX_LIMIT),
|
||||
}, { issueConditions: issueFilters });
|
||||
}, { issueConditions: issueFilters, readCondition: options?.issueReadCondition });
|
||||
return result.artifacts;
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user