diff --git a/doc/ISSUE-PRIVACY.md b/doc/ISSUE-PRIVACY.md index 4e70036cf02..9f93c901b99 100644 --- a/doc/ISSUE-PRIVACY.md +++ b/doc/ISSUE-PRIVACY.md @@ -59,17 +59,36 @@ content. Plugins (including any Slack notifier or digest plugin) are **non-member principals**: they hold no company membership, task grant, or private-project -access. The host services that back the `issues.read` capability — -`issues.list`, `issues.get`, `issues.listComments`, `issues.listAttachments`, -`issues.getAttachmentContent`, and `issues.getOrchestrationSummary` — apply the -canonical visibility predicate against a synthetic non-member actor, so a -plugin sees only company-open work. A private task is filtered from list -results, returns `null`/empty on direct read, and reports "not found" as an -orchestration root. Orchestration summaries additionally drop private subtree -descendants and redact blocker/blocks edges that point at a private task, so -its title, status, assignee, cost, and run metadata never enter an outbound -payload. This is why a digest broadcast to a shared channel — whose audience is -non-members by definition — cannot carry private issue content. +access. Rather than an allowlist of individually patched methods, the plugin +host centralizes the synthetic non-member check into shared helpers +(`requirePluginReadableIssue`, `pluginReadableIssueIdsByIds`, +`pluginRedactRelationSummary`) and routes **every** issue-derived read through +them. The complete inventory of issue-derived reads and their disposition: + +- **Direct issue content** — `issues.list` (SQL predicate), `issues.get`, + `issues.listComments`, `issues.listAttachments`, `issues.getAttachmentContent`, + `issueDocuments.list`, `issueDocuments.get`: a private task is filtered from + list results, returns `null`/empty on soft reads, and reports "not found" + (indistinguishable from missing) on document reads. +- **Subtree / orchestration** — `issues.getSubtree` and + `issues.getOrchestrationSummary`: a private *root* reports "not found"; an + open root drops private descendants **before** any relation, document, run, + or assignee is fetched, so no private row or its metadata enters the payload. +- **Relationship metadata** — `issues.getRelations` (and the summaries echoed + by `issues.setBlockedBy` / `addBlockers` / `removeBlockers`, and the subtree + relation maps): blocker/blocks edges pointing at a private task are dropped, + recursing into nested terminal blockers, so its title, status, and assignee + never surface even on a public task. +- **Interactions** — `issues.listInteractions`: a private task yields no + interaction payloads. + +A synthetic `{ type: "none" }` actor resolves to the same public-only scope as +any other non-member. This is why a digest broadcast to a shared channel — +whose audience is non-members by definition — cannot carry private issue +content. Write/coordination paths (`create`, `update`, `requestWakeup`, +`assertCheckoutOwner`, comment/interaction/approval mutations) are governed by +their own write capabilities and are out of scope for this read predicate; any +relation summary they *return* is still redacted. There is intentionally **no plugin oversight exception**: unlike the company-wide audit log, no plugin read path bypasses the predicate. A plugin @@ -116,7 +135,7 @@ it protects: | Blocker and mention identifier-only stubs | `issue-access-grants-routes.test.ts` | | Attachment content | `issue-attachment-routes.test.ts` | | Private-project list and direct read | `projects-list-archived-routes.test.ts` | -| Plugin issue reads (list, get, comments, attachments, orchestration) | `plugin-orchestration-apis.test.ts` | +| Plugin issue reads (list, get, comments, attachments, orchestration, subtree, relations, interactions, documents) | `plugin-orchestration-apis.test.ts` | Adding a new task-derived read surface requires a non-member fixture in this gate before the surface can ship. diff --git a/server/src/__tests__/plugin-orchestration-apis.test.ts b/server/src/__tests__/plugin-orchestration-apis.test.ts index 6ea2a654945..e0a12358457 100644 --- a/server/src/__tests__/plugin-orchestration-apis.test.ts +++ b/server/src/__tests__/plugin-orchestration-apis.test.ts @@ -14,11 +14,13 @@ import { companyMemberships, costEvents, createDb, + documents, executionWorkspaces, heartbeatRunEvents, heartbeatRuns, issueAttachments, issueComments, + issueDocuments, issueRelations, issueThreadInteractions, issues, @@ -1304,6 +1306,91 @@ describeEmbeddedPostgres("plugin orchestration APIs", () => { ).rejects.toThrow("Issue not found"); }); + async function seedIssueDocument(companyId: string, issueId: string, key: string, body: string) { + const documentId = randomUUID(); + await db.insert(documents).values({ id: documentId, companyId, latestBody: body, format: "markdown" }); + await db.insert(issueDocuments).values({ id: randomUUID(), companyId, issueId, documentId, key }); + return documentId; + } + + it("throws not-found from issues.getRelations for a private issue", async () => { + const { companyId, privateIssueId } = await seedOpenAndPrivate(); + const services = buildHostServices(db, "plugin-record-id", "paperclip.slack-digest", createEventBusStub()); + await expect( + services.issues.getRelations({ issueId: privateIssueId, companyId }), + ).rejects.toThrow("Issue not found"); + }); + + it("redacts a private blocker edge from issues.getRelations on a public issue", async () => { + const { companyId, openIssueId, privateIssueId } = await seedOpenAndPrivate(); + // privateIssue blocks the open issue -> open.blockedBy would name it pre-redaction. + await db.insert(issueRelations).values({ + companyId, issueId: privateIssueId, relatedIssueId: openIssueId, type: "blocks", + }); + const services = buildHostServices(db, "plugin-record-id", "paperclip.slack-digest", createEventBusStub()); + const relations = await services.issues.getRelations({ issueId: openIssueId, companyId }); + const edgeIds = [...relations.blockedBy, ...relations.blocks].map((edge) => edge.id); + expect(edgeIds).not.toContain(privateIssueId); + }); + + it("reports a private root issue as not-found from issues.getSubtree", async () => { + const { companyId, privateIssueId } = await seedOpenAndPrivate(); + const services = buildHostServices(db, "plugin-record-id", "paperclip.slack-digest", createEventBusStub()); + await expect( + services.issues.getSubtree({ issueId: privateIssueId, companyId }), + ).rejects.toThrow("Issue not found"); + }); + + it("excludes a private child and its documents from issues.getSubtree on an open root", async () => { + const { companyId } = await seedCompanyAndAgent(); + const rootId = randomUUID(); + const publicChildId = randomUUID(); + const privateChildId = randomUUID(); + await db.insert(issues).values([ + { id: rootId, companyId, title: "Root", status: "todo", priority: "medium", visibility: "open" }, + { id: publicChildId, companyId, parentId: rootId, title: "Public child", status: "todo", priority: "medium", visibility: "open" }, + { id: privateChildId, companyId, parentId: rootId, title: "Private child", status: "todo", priority: "medium", visibility: "private" }, + ]); + // A document on the private child would surface in the subtree summary pre-fix. + await seedIssueDocument(companyId, privateChildId, "plan", "confidential subtree plan"); + + const services = buildHostServices(db, "plugin-record-id", "paperclip.slack-digest", createEventBusStub()); + const subtree = await services.issues.getSubtree({ + issueId: rootId, companyId, includeDocuments: true, includeRelations: true, + }); + expect(new Set(subtree.issueIds)).toEqual(new Set([rootId, publicChildId])); + expect(subtree.issues.map((issue) => issue.id)).not.toContain(privateChildId); + expect(Object.keys(subtree.documents ?? {})).not.toContain(privateChildId); + }); + + it("returns no interactions from issues.listInteractions for a private issue", async () => { + const { companyId, privateIssueId } = await seedOpenAndPrivate(); + await db.insert(issueThreadInteractions).values({ + id: randomUUID(), companyId, issueId: privateIssueId, kind: "request_confirmation", + payload: { version: 1, prompt: "Confirm confidential action?" } as any, + }); + const services = buildHostServices(db, "plugin-record-id", "paperclip.slack-digest", createEventBusStub()); + await expect(services.issues.listInteractions({ issueId: privateIssueId, companyId })).resolves.toEqual([]); + }); + + it("throws not-found from issueDocuments.list for a private issue", async () => { + const { companyId, privateIssueId } = await seedOpenAndPrivate(); + await seedIssueDocument(companyId, privateIssueId, "plan", "confidential plan body"); + const services = buildHostServices(db, "plugin-record-id", "paperclip.slack-digest", createEventBusStub()); + await expect( + services.issueDocuments.list({ issueId: privateIssueId, companyId }), + ).rejects.toThrow("Issue not found"); + }); + + it("throws not-found from issueDocuments.get for a private issue", async () => { + const { companyId, privateIssueId } = await seedOpenAndPrivate(); + await seedIssueDocument(companyId, privateIssueId, "plan", "confidential plan body"); + const services = buildHostServices(db, "plugin-record-id", "paperclip.slack-digest", createEventBusStub()); + await expect( + services.issueDocuments.get({ issueId: privateIssueId, companyId, key: "plan" }), + ).rejects.toThrow("Issue not found"); + }); + it("is pass-through when privacy mode is off (enforce flip is the single switch)", async () => { process.env.PAPERCLIP_ISSUE_PRIVACY_MODE = "off"; const { companyId, privateIssueId } = await seedOpenAndPrivate(); diff --git a/server/src/services/plugin-host-services.ts b/server/src/services/plugin-host-services.ts index ebabf541d17..7b778e2fe48 100644 --- a/server/src/services/plugin-host-services.ts +++ b/server/src/services/plugin-host-services.ts @@ -611,6 +611,80 @@ export function buildHostServices( ); return new Set(decisions.filter((decision) => decision.allowed).map((decision) => decision.id)); }; + // Column projection matching IssuePrivacyRow, for ad-hoc readability lookups + // of bare issue IDs (relation targets, subtree descendants) that arrive + // without their privacy columns loaded. + const issuePrivacyRowColumns = { + id: issuesTable.id, + companyId: issuesTable.companyId, + visibility: issuesTable.visibility, + privacyRootIssueId: issuesTable.privacyRootIssueId, + responsibleUserId: issuesTable.responsibleUserId, + createdByUserId: issuesTable.createdByUserId, + assigneeUserId: issuesTable.assigneeUserId, + assigneeAgentId: issuesTable.assigneeAgentId, + projectId: issuesTable.projectId, + } as const; + // Resolve which of the given issue IDs a non-member plugin may read. Loads + // the privacy-relevant columns for those IDs and evaluates the canonical + // predicate. An ID with no surviving row (deleted, cross-company) is absent + // from the result set — fail closed. + const pluginReadableIssueIdsByIds = async ( + companyId: string, + ids: Iterable, + ): Promise> => { + const uniqueIds = [...new Set(ids)]; + if (issuePrivacyMode() !== "enforce" || uniqueIds.length === 0) { + return new Set(uniqueIds); + } + const rows = await db + .select(issuePrivacyRowColumns) + .from(issuesTable) + .where(and(eq(issuesTable.companyId, companyId), inArray(issuesTable.id, uniqueIds))); + return pluginReadableIssueIds(rows); + }; + // Fetch an issue and require it be both in-company and readable by a + // non-member plugin. A private (unreadable) issue throws the same "not found" + // error a genuinely missing issue would, so its existence never leaks. This + // is the single gate every issue-derived plugin read routes through, so no + // method needs its own bespoke visibility check. + const requirePluginReadableIssue = async (issueId: string, companyId: string): Promise => { + const issue = requireInCompany("Issue", await issues.getById(issueId), companyId); + if (!(await pluginCanReadIssue(issue))) { + throw new Error("Issue not found"); + } + return issue as Issue; + }; + // Redact a relation summary so blocker/blocked edges pointing at an issue the + // plugin cannot read are dropped, recursing into nested terminal blockers. An + // edge's title/status/assignee is relationship metadata a non-member must not + // see even when the issue that owns the edge is itself public. + const pluginRedactRelationSummary = async < + R extends { blockedBy: IssueRelationIssueSummary[]; blocks: IssueRelationIssueSummary[] }, + >( + companyId: string, + relation: R, + ): Promise => { + if (issuePrivacyMode() !== "enforce") return relation; + const edgeIds = new Set(); + const collect = (edges: IssueRelationIssueSummary[]) => { + for (const edge of edges) { + edgeIds.add(edge.id); + if (edge.terminalBlockers?.length) collect(edge.terminalBlockers); + } + }; + collect(relation.blockedBy); + collect(relation.blocks); + if (edgeIds.size === 0) return relation; + const readable = await pluginReadableIssueIdsByIds(companyId, edgeIds); + const redact = (edges: IssueRelationIssueSummary[]): IssueRelationIssueSummary[] => + edges + .filter((edge) => readable.has(edge.id)) + .map((edge) => (edge.terminalBlockers?.length + ? { ...edge, terminalBlockers: redact(edge.terminalBlockers) } + : edge)); + return { ...relation, blockedBy: redact(relation.blockedBy), blocks: redact(relation.blocks) }; + }; // Track active session event subscriptions for cleanup const activeSubscriptions = new Set<{ unsubscribe: () => void; timer: ReturnType }>(); @@ -1839,13 +1913,20 @@ export function buildHostServices( async getRelations(params) { const companyId = ensureCompanyId(params.companyId); await ensurePluginAvailableForCompany(companyId); - requireInCompany("Issue", await issues.getById(params.issueId), companyId); - return await issues.getRelationSummaries(params.issueId); + // Private root -> indistinguishable from missing; readable root -> + // still redact any edge that points at an issue the plugin cannot read. + await requirePluginReadableIssue(params.issueId, companyId); + return await pluginRedactRelationSummary( + companyId, + await issues.getRelationSummaries(params.issueId), + ); }, async setBlockedBy(params) { const companyId = ensureCompanyId(params.companyId); await ensurePluginAvailableForCompany(companyId); - return setBlockedByWithActivity({ + // The echoed relation summary can name a private blocker target, so + // redact unreadable edges before returning it to a non-member plugin. + return pluginRedactRelationSummary(companyId, await setBlockedByWithActivity({ companyId, issueId: params.issueId, blockedByIssueIds: params.blockedByIssueIds, @@ -1853,7 +1934,7 @@ export function buildHostServices( actorAgentId: params.actorAgentId, actorUserId: params.actorUserId, actorRunId: params.actorRunId, - }); + })); }, async addBlockers(params) { const companyId = ensureCompanyId(params.companyId); @@ -1866,7 +1947,7 @@ export function buildHostServices( ...params.blockerIssueIds, ]), ]; - return setBlockedByWithActivity({ + return pluginRedactRelationSummary(companyId, await setBlockedByWithActivity({ companyId, issueId: params.issueId, blockedByIssueIds: nextBlockedByIssueIds, @@ -1874,7 +1955,7 @@ export function buildHostServices( actorAgentId: params.actorAgentId, actorUserId: params.actorUserId, actorRunId: params.actorRunId, - }); + })); }, async removeBlockers(params) { const companyId = ensureCompanyId(params.companyId); @@ -1885,7 +1966,7 @@ export function buildHostServices( const nextBlockedByIssueIds = previous.blockedBy .map((relation) => relation.id) .filter((issueId) => !removals.has(issueId)); - return setBlockedByWithActivity({ + return pluginRedactRelationSummary(companyId, await setBlockedByWithActivity({ companyId, issueId: params.issueId, blockedByIssueIds: nextBlockedByIssueIds, @@ -1893,7 +1974,7 @@ export function buildHostServices( actorAgentId: params.actorAgentId, actorUserId: params.actorUserId, actorRunId: params.actorRunId, - }); + })); }, async assertCheckoutOwner(params) { const companyId = ensureCompanyId(params.companyId); @@ -1932,17 +2013,31 @@ export function buildHostServices( async getSubtree(params) { const companyId = ensureCompanyId(params.companyId); await ensurePluginAvailableForCompany(companyId); - const rootIssue = requireInCompany("Issue", await issues.getById(params.issueId), companyId); + // A private root is invisible to a non-member plugin: report not-found + // rather than leak its existence or any descendant metadata. + const rootIssue = await requirePluginReadableIssue(params.issueId, companyId); const includeRoot = params.includeRoot !== false; const subtreeIssueIds = await collectIssueSubtreeIds(companyId, rootIssue.id); - const issueIds = includeRoot ? subtreeIssueIds : subtreeIssueIds.filter((issueId) => issueId !== rootIssue.id); - const issueRows = issueIds.length > 0 + const candidateIssueIds = includeRoot + ? subtreeIssueIds + : subtreeIssueIds.filter((issueId) => issueId !== rootIssue.id); + const issueRows = candidateIssueIds.length > 0 ? await db .select() .from(issuesTable) - .where(and(eq(issuesTable.companyId, companyId), inArray(issuesTable.id, issueIds))) + .where(and(eq(issuesTable.companyId, companyId), inArray(issuesTable.id, candidateIssueIds))) : []; - const issuesById = new Map(issueRows.map((issue) => [issue.id, issue as Issue])); + // Filter private descendants BEFORE fetching any relations, documents, + // runs, or assignees so private issue rows and their metadata never + // enter the result. The root has already cleared the readability gate. + const readableSubtreeIds = await pluginReadableIssueIds(issueRows); + readableSubtreeIds.add(rootIssue.id); + const issueIds = candidateIssueIds.filter((issueId) => readableSubtreeIds.has(issueId)); + const issuesById = new Map( + issueRows + .filter((issue) => readableSubtreeIds.has(issue.id)) + .map((issue) => [issue.id, issue as Issue]), + ); const outputIssues = issueIds .map((issueId) => issuesById.get(issueId)) .filter((issue): issue is Issue => Boolean(issue)); @@ -1953,7 +2048,10 @@ export function buildHostServices( const [relationPairs, documentPairs, activeRunRows, assigneeRows] = await Promise.all([ params.includeRelations - ? Promise.all(issueIds.map(async (issueId) => [issueId, await issues.getRelationSummaries(issueId)] as const)) + ? Promise.all(issueIds.map(async (issueId) => [ + issueId, + await pluginRedactRelationSummary(companyId, await issues.getRelationSummaries(issueId)), + ] as const)) : Promise.resolve(null), params.includeDocuments ? Promise.all( @@ -2151,86 +2249,28 @@ export function buildHostServices( async getOrchestrationSummary(params): Promise { const companyId = ensureCompanyId(params.companyId); await ensurePluginAvailableForCompany(companyId); - const rootIssue = requireInCompany("Issue", await issues.getById(params.issueId), companyId); // A private root is invisible to a non-member plugin: report not-found // rather than leak its existence or any subtree relationship metadata. - if (!(await pluginCanReadIssue(rootIssue))) { - throw new Error("Issue not found"); - } + const rootIssue = await requirePluginReadableIssue(params.issueId, companyId); const rawSubtreeIssueIds = params.includeSubtree ? await collectIssueSubtreeIds(companyId, rootIssue.id) : [rootIssue.id]; // Drop any private descendant the plugin may not read so its relations, // approvals, runs, costs, and budget blocks never enter the summary. - const subtreePrivacyRows = await db - .select({ - id: issuesTable.id, - companyId: issuesTable.companyId, - visibility: issuesTable.visibility, - privacyRootIssueId: issuesTable.privacyRootIssueId, - responsibleUserId: issuesTable.responsibleUserId, - createdByUserId: issuesTable.createdByUserId, - assigneeUserId: issuesTable.assigneeUserId, - assigneeAgentId: issuesTable.assigneeAgentId, - projectId: issuesTable.projectId, - }) - .from(issuesTable) - .where(and(eq(issuesTable.companyId, companyId), inArray(issuesTable.id, rawSubtreeIssueIds))); - const readableSubtreeIds = await pluginReadableIssueIds(subtreePrivacyRows); + const readableSubtreeIds = await pluginReadableIssueIdsByIds(companyId, rawSubtreeIssueIds); readableSubtreeIds.add(rootIssue.id); const subtreeIssueIds = rawSubtreeIssueIds.filter((id) => readableSubtreeIds.has(id)); - const rawRelationPairs = await Promise.all( - subtreeIssueIds.map(async (issueId) => [issueId, await issues.getRelationSummaries(issueId)] as const), - ); // Blocker edges can point at private issues outside the readable subtree // (e.g. a public task blocked by a private one). Their title/status/ - // assignee are relationship metadata a non-member must not see, so drop - // any edge whose target the plugin cannot read, recursing into nested - // terminal blockers. - const edgeIds = new Set(); - const collectEdgeIds = (edges: IssueRelationIssueSummary[]) => { - for (const edge of edges) { - edgeIds.add(edge.id); - if (edge.terminalBlockers?.length) collectEdgeIds(edge.terminalBlockers); - } - }; - for (const [, relation] of rawRelationPairs) { - collectEdgeIds(relation.blockedBy); - collectEdgeIds(relation.blocks); - } - const edgePrivacyRows = edgeIds.size === 0 - ? [] - : await db - .select({ - id: issuesTable.id, - companyId: issuesTable.companyId, - visibility: issuesTable.visibility, - privacyRootIssueId: issuesTable.privacyRootIssueId, - responsibleUserId: issuesTable.responsibleUserId, - createdByUserId: issuesTable.createdByUserId, - assigneeUserId: issuesTable.assigneeUserId, - assigneeAgentId: issuesTable.assigneeAgentId, - projectId: issuesTable.projectId, - }) - .from(issuesTable) - .where(and(eq(issuesTable.companyId, companyId), inArray(issuesTable.id, [...edgeIds]))); - const readableEdgeIds = await pluginReadableIssueIds(edgePrivacyRows); - const redactEdges = (edges: IssueRelationIssueSummary[]): IssueRelationIssueSummary[] => - edges - .filter((edge) => readableEdgeIds.has(edge.id)) - .map((edge) => ( - edge.terminalBlockers?.length - ? { ...edge, terminalBlockers: redactEdges(edge.terminalBlockers) } - : edge - )); - const relationPairs = rawRelationPairs.map(([issueId, relation]) => [ - issueId, - { - ...relation, - blockedBy: redactEdges(relation.blockedBy), - blocks: redactEdges(relation.blocks), - }, - ] as const); + // assignee are relationship metadata a non-member must not see, so + // pluginRedactRelationSummary drops any edge whose target is unreadable, + // recursing into nested terminal blockers. + const relationPairs = await Promise.all( + subtreeIssueIds.map(async (issueId) => [ + issueId, + await pluginRedactRelationSummary(companyId, await issues.getRelationSummaries(issueId)), + ] as const), + ); const approvalRows = ( await Promise.all( subtreeIssueIds.map(async (issueId) => { @@ -2398,7 +2438,11 @@ export function buildHostServices( async listInteractions(params) { const companyId = ensureCompanyId(params.companyId); await ensurePluginAvailableForCompany(companyId); - if (!inCompany(await issues.getById(params.issueId), companyId)) return []; + const issue = await issues.getById(params.issueId); + if (!inCompany(issue, companyId)) return []; + // Interaction payloads follow the ACL of the issue they belong to; a + // private issue yields no interactions to a non-member plugin. + if (!(await pluginCanReadIssue(issue))) return []; return (await interactions.listForIssue(params.issueId)) as any; }, async respondInteraction(params) { @@ -2628,14 +2672,15 @@ export function buildHostServices( async list(params) { const companyId = ensureCompanyId(params.companyId); await ensurePluginAvailableForCompany(companyId); - requireInCompany("Issue", await issues.getById(params.issueId), companyId); + // Document bodies follow the issue ACL; a private issue is not-found. + await requirePluginReadableIssue(params.issueId, companyId); const rows = await documents.listIssueDocuments(params.issueId); return rows as any; }, async get(params) { const companyId = ensureCompanyId(params.companyId); await ensurePluginAvailableForCompany(companyId); - requireInCompany("Issue", await issues.getById(params.issueId), companyId); + await requirePluginReadableIssue(params.issueId, companyId); const doc = await documents.getIssueDocumentByKey(params.issueId, params.key); return (doc ?? null) as any; },