diff --git a/server/src/__tests__/privacy-production-review.test.ts b/server/src/__tests__/privacy-production-review.test.ts index 20e77b0fd32..1e0b37396ca 100644 --- a/server/src/__tests__/privacy-production-review.test.ts +++ b/server/src/__tests__/privacy-production-review.test.ts @@ -244,6 +244,42 @@ describe("private task production review", () => { expect(await canActorReadIssuePrivacy(db, f.actor(f.outsider), grandchild)).toBe(false); }); + it("binds child-manager grant writes to the child and rejects ancestor or sibling grants", async () => { + const f = await fixture(); + const { issueService } = await import("../services/issues.js"); + const { issueRoutes } = await import("../routes/issues.js"); + const { errorHandler } = await import("../middleware/index.js"); + const svc = issueService(db); + const root = await svc.create(f.company.id, { title: "Ancestor", visibility: "private", createdByUserId: f.outsider }); + const child = await svc.create(f.company.id, { title: "Managed child", parentId: root.id, createdByUserId: f.owner }); + const grandchild = await svc.create(f.company.id, { title: "Descendant", parentId: child.id, createdByUserId: f.outsider }); + const sibling = await svc.create(f.company.id, { title: "Sibling", parentId: root.id, createdByUserId: f.outsider }); + const [rootGrant] = await db.insert(issueAccessGrants).values({ issueId: root.id, subjectType: "user", subjectId: f.owner, source: "explicit" }).returning(); + const [siblingGrant] = await db.insert(issueAccessGrants).values({ issueId: sibling.id, subjectType: "user", subjectId: f.owner, source: "explicit" }).returning(); + const app = express(); app.use(express.json()); + app.use((req, _res, next) => { req.actor = f.actor(f.owner); next(); }); + app.use("/api", issueRoutes(db, {} as any)); app.use(errorHandler); + const audience = await request(app).get(`/api/issues/${child.id}/access-grants`).expect(200); + expect(audience.body).toContainEqual(expect.objectContaining({ id: rootGrant.id, inherited: true })); + for (const grant of [rootGrant, siblingGrant]) { + await request(app).post(`/api/issues/${child.id}/access-grants/${grant.id}/revoke`).send({}).expect(404); + const [unchanged] = await db.select().from(issueAccessGrants).where(eq(issueAccessGrants.id, grant.id)); + expect(unchanged.revokedAt).toBeNull(); + } + for (const task of [root, sibling]) { + await request(app).post(`/api/issues/${task.id}/access-grants`).send({ subjectType: "agent", subjectId: f.agent.id }).expect(403); + } + const added = await request(app).post(`/api/issues/${child.id}/access-grants`).send({ subjectType: "agent", subjectId: f.agent.id }).expect(201); + expect(added.body.issueId).toBe(child.id); + const agent: AuthorizationActor = { type: "agent", companyId: f.company.id, agentId: f.agent.id }; + for (const [task, expected] of [[root, false], [child, true], [grandchild, true], [sibling, false]] as const) { + expect(await canActorReadIssuePrivacy(db, agent, task)).toBe(expected); + } + await request(app).post(`/api/issues/${child.id}/access-grants/${added.body.id}/revoke`).send({}).expect(200); + expect(await canActorReadIssuePrivacy(db, agent, child)).toBe(false); + expect(await canActorReadIssuePrivacy(db, agent, grandchild)).toBe(false); + }); + it("retains a child assignment grant after unassignment without granting the root", async () => { const f = await fixture(); const { issueService } = await import("../services/issues.js"); diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index 465e6b52764..579756c9ce4 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -5214,7 +5214,7 @@ export function issueRoutes( return false; } - async function resolveIssuePrivacyManagementRoot( + async function resolveManagedIssueForPrivacy( req: Request, res: Response, issue: Parameters[1] & { @@ -5225,6 +5225,8 @@ export function issueRoutes( ) { if (!(await assertIssueReadAllowed(req, res, issue))) return null; if (!(await assertCanManageIssuePrivacy(req, res, issue))) return null; + // Management is scoped to this exact task; an ancestor privacy root is + // inheritance provenance, never authority to mutate its grants. return issue; } @@ -13070,7 +13072,7 @@ export function issueRoutes( router.get("/issues/:id/privacy-constraints", async (req, res) => { const issue = await getAccessibleResource(req, res, svc.getById(req.params.id as string), "Issue not found"); - if (!issue || !(await resolveIssuePrivacyManagementRoot(req, res, issue))) return; + if (!issue || !(await resolveManagedIssueForPrivacy(req, res, issue))) return; // A task owner need not be able to read its surrounding private scope. // Read policy internally and return only action hints, never scope identity. const [project] = issue.projectId ? await db.select({ visibility: projects.visibility, personalOwnerUserId: projects.personalOwnerUserId }) @@ -13090,8 +13092,8 @@ export function issueRoutes( router.get("/issues/:id/access-grants", async (req, res) => { const issue = await getAccessibleResource(req, res, svc.getById(req.params.id as string), "Issue not found"); if (!issue || !(await assertIssueReadAllowed(req, res, issue))) return; - const privacyRoot = await resolveIssuePrivacyManagementRoot(req, res, issue); - if (!privacyRoot) return; + const managedIssue = await resolveManagedIssueForPrivacy(req, res, issue); + if (!managedIssue) return; const ancestry = await db.select().from(issueRows).where(inArray(issueRows.id, sql`( with recursive ancestry as ( select id, company_id, privacy_parent_issue_id from issues where id = ${issue.id} and company_id = ${issue.companyId} @@ -13128,9 +13130,9 @@ export function issueRoutes( async (req, res) => { const issue = await getAccessibleResource(req, res, svc.getById(req.params.id as string), "Issue not found"); if (!issue || !(await assertIssueReadAllowed(req, res, issue))) return; - const privacyRoot = await resolveIssuePrivacyManagementRoot(req, res, issue); - if (!privacyRoot) return; - if (privacyRoot.privacyRootIssueId === null) { + const managedIssue = await resolveManagedIssueForPrivacy(req, res, issue); + if (!managedIssue) return; + if (managedIssue.privacyRootIssueId === null) { res.status(409).json({ error: "Open issues do not require access grants" }); return; } @@ -13139,7 +13141,7 @@ export function issueRoutes( await assertIssueGrantSubjectExists(issue.companyId, subjectType, subjectId); const actor = getActorInfo(req); const result = await db.transaction((tx) => ensureActiveIssueAccessGrant(tx, { - issueId: privacyRoot.id, + issueId: managedIssue.id, subjectType, subjectId, source: "explicit", @@ -13176,15 +13178,14 @@ export function issueRoutes( async (req, res) => { const issue = await getAccessibleResource(req, res, svc.getById(req.params.id as string), "Issue not found"); if (!issue || !(await assertIssueReadAllowed(req, res, issue))) return; - const privacyRoot = await resolveIssuePrivacyManagementRoot(req, res, issue); - if (!privacyRoot) return; - const grantIssueIds = [...new Set([issue.id, privacyRoot.id])]; + const managedIssue = await resolveManagedIssueForPrivacy(req, res, issue); + if (!managedIssue) return; const [grant] = await db .update(issueAccessGrants) .set({ revokedAt: new Date() }) .where(and( eq(issueAccessGrants.id, req.params.grantId as string), - inArray(issueAccessGrants.issueId, grantIssueIds), + eq(issueAccessGrants.issueId, managedIssue.id), isNull(issueAccessGrants.revokedAt), )) .returning(); @@ -13395,7 +13396,7 @@ export function issueRoutes( } // An authorized agent may restrict a task; widening access remains an owner/admin action. if (req.body.visibility !== undefined && !(req.actor.type === "agent" && req.body.visibility === "private") - && !(await resolveIssuePrivacyManagementRoot(req, res, existing))) return; + && !(await resolveManagedIssueForPrivacy(req, res, existing))) return; // MCP task changes cannot stop a run or implicitly decide a pending review. const mcpParticipationChange = req.actor.source === "mcp_oauth" && ["status", "assigneeAgentId", "assigneeUserId", "blockedByIssueIds", "parentId", "projectId"].some(key => req.body[key] !== undefined);