From 50a4d25feda6665ab5f7684af5215ccf9b3ce09b Mon Sep 17 00:00:00 2001 From: Dotta Date: Sun, 5 Jul 2026 16:34:00 +0000 Subject: [PATCH] Fix multi-project scoped issue authorization Co-Authored-By: Paperclip --- .../__tests__/authorization-service.test.ts | 53 +++++++ .../issue-execution-policy-routes.test.ts | 65 ++++++++ .../low-trust-red-team-routes.test.ts | 50 ++++++ server/src/routes/issues.ts | 150 ++++++++++++++---- server/src/services/authorization.ts | 22 ++- server/src/services/trust-preset-resolver.ts | 3 +- 6 files changed, 313 insertions(+), 30 deletions(-) diff --git a/server/src/__tests__/authorization-service.test.ts b/server/src/__tests__/authorization-service.test.ts index de0484939e3..4a1078151e6 100644 --- a/server/src/__tests__/authorization-service.test.ts +++ b/server/src/__tests__/authorization-service.test.ts @@ -1521,4 +1521,57 @@ describeEmbeddedPostgres("authorization service", () => { reason: "deny_scope", }); }); + + it("requires every requested project membership to fit task bridge project scope", async () => { + const company = await createCompany(db, "TaskBridgeProjects"); + const bridgeAgent = await createAgent(db, company.id); + const targetAgent = await createAgent(db, company.id); + const allowedProject = await createProject(db, company.id, "Bridge allowed"); + const deniedProject = await createProject(db, company.id, "Bridge denied"); + const parentIssue = await createIssue(db, company.id, { projectId: allowedProject.id }); + const actor = { + type: "agent" as const, + agentId: bridgeAgent.id, + companyId: company.id, + source: "agent_key" as const, + keyId: randomUUID(), + keyScope: { + kind: "task_bridge" as const, + projectIds: [allowedProject.id], + allowedAssigneeAgentIds: [targetAgent.id], + }, + }; + const authz = authorizationService(db); + + await expect(authz.decide({ + actor, + action: "tasks:assign", + resource: { + type: "issue", + companyId: company.id, + parentIssueId: parentIssue.id, + projectId: allowedProject.id, + projectIds: [allowedProject.id], + assigneeAgentId: targetAgent.id, + }, + })).resolves.toMatchObject({ + allowed: true, + }); + + await expect(authz.decide({ + actor, + action: "tasks:assign", + resource: { + type: "issue", + companyId: company.id, + parentIssueId: parentIssue.id, + projectId: allowedProject.id, + projectIds: [allowedProject.id, deniedProject.id], + assigneeAgentId: targetAgent.id, + }, + })).resolves.toMatchObject({ + allowed: false, + reason: "deny_scope", + }); + }); }); diff --git a/server/src/__tests__/issue-execution-policy-routes.test.ts b/server/src/__tests__/issue-execution-policy-routes.test.ts index cce87c1b4eb..84258220077 100644 --- a/server/src/__tests__/issue-execution-policy-routes.test.ts +++ b/server/src/__tests__/issue-execution-policy-routes.test.ts @@ -650,4 +650,69 @@ describe("issue execution policy routes", () => { }), ); }); + + it("rejects scoped task-bridge child creation when projectIds includes an unauthorized secondary project", async () => { + const allowedProjectId = "11111111-1111-4111-8111-111111111111"; + const deniedProjectId = "22222222-2222-4222-8222-222222222222"; + mockIssueService.getById.mockResolvedValue({ + id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", + companyId: "company-1", + projectId: null, + projects: [ + { id: allowedProjectId, name: "Allowed", color: null, icon: null, isPrimary: true }, + ], + status: "in_progress", + assigneeAgentId: "33333333-3333-4333-8333-333333333333", + assigneeUserId: null, + createdByUserId: "local-board", + identifier: "PAP-1001", + title: "Parent issue", + executionPolicy: null, + executionState: null, + }); + mockAccessService.decide.mockImplementation(async (input: { + actor?: { type?: string; source?: string }; + action?: string; + resource?: { projectId?: string | null }; + }) => { + if (input.action === "tasks:assign") { + const allowed = input.resource?.projectId === allowedProjectId; + return { + allowed, + action: input.action, + reason: allowed ? "allow_explicit_grant" : "deny_scope", + explanation: allowed ? "Allowed by scoped project grant." : "Project is outside this task bridge scope.", + }; + } + return { + allowed: true, + action: input.action, + reason: "allow_explicit_grant", + explanation: "Allowed by test grant.", + }; + }); + + const res = await request(await createApp({ + type: "agent", + agentId: "33333333-3333-4333-8333-333333333333", + companyId: "company-1", + runId: "run-1", + source: "agent_key", + keyId: "task-bridge-key", + keyScope: { kind: "task_bridge", projectIds: [allowedProjectId] }, + } as any)) + .post("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa/children") + .send({ + title: "Unauthorized secondary", + projectIds: [allowedProjectId, deniedProjectId], + }); + + expect(res.status).toBe(403); + expect(res.body.error).toBe("Project is outside this task bridge scope."); + expect(mockIssueService.createChild).not.toHaveBeenCalled(); + expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({ + action: "tasks:assign", + resource: expect.objectContaining({ projectId: deniedProjectId }), + })); + }); }); diff --git a/server/src/__tests__/low-trust-red-team-routes.test.ts b/server/src/__tests__/low-trust-red-team-routes.test.ts index bf972d0251f..214294884ae 100644 --- a/server/src/__tests__/low-trust-red-team-routes.test.ts +++ b/server/src/__tests__/low-trust-red-team-routes.test.ts @@ -21,6 +21,7 @@ import { issueApprovals, issueComments, issueDocuments, + issueProjects, issueRelations, issues, issueThreadInteractions, @@ -858,6 +859,55 @@ describeEmbeddedPostgres("low-trust red-team HTTP route regression suite", () => expect(lowTrustCount.body.count).toBe(1); }); + it("keeps low-trust list, detail, and count aligned for secondary project memberships", async () => { + const fixture = await seedLowTrustFixture(db); + const [secondaryMembershipIssue] = await db.insert(issues).values({ + companyId: fixture.company.id, + projectId: fixture.projects.outOfScope.id, + title: "Secondary membership blocked vendor wait", + status: "blocked", + priority: "medium", + description: "external owner: Secondary vendor\nexternal action: Finish secondary review", + }).returning(); + await db.insert(issueProjects).values([ + { + companyId: fixture.company.id, + issueId: secondaryMembershipIssue!.id, + projectId: fixture.projects.outOfScope.id, + isPrimary: true, + }, + { + companyId: fixture.company.id, + issueId: secondaryMembershipIssue!.id, + projectId: fixture.projects.allowed.id, + isPrimary: false, + }, + ]); + + const lowTrustApp = createApp(db, agentActor(fixture)); + const query = { attention: "blocked", q: "Secondary membership blocked vendor wait" }; + const count = await request(lowTrustApp) + .get(`/api/companies/${fixture.company.id}/issues/count`) + .query(query); + expect(count.status, JSON.stringify(count.body)).toBe(200); + expect(count.body.count).toBe(1); + + const list = await request(lowTrustApp) + .get(`/api/companies/${fixture.company.id}/issues`) + .query(query); + expect(list.status, JSON.stringify(list.body)).toBe(200); + expect(list.body.map((issue: { id: string }) => issue.id)).toContain(secondaryMembershipIssue!.id); + + const detail = await request(lowTrustApp) + .get(`/api/issues/${secondaryMembershipIssue!.id}`); + expect(detail.status, JSON.stringify(detail.body)).toBe(200); + expect(detail.body.id).toBe(secondaryMembershipIssue!.id); + expect(detail.body.projects.map((project: { id: string }) => project.id)).toEqual([ + fixture.projects.outOfScope.id, + fixture.projects.allowed.id, + ]); + }); + it("redacts quarantined low-trust output from higher-trust wake and continuation contexts", async () => { const fixture = await seedLowTrustFixture(db); const lowTrustApp = createApp(db, agentActor(fixture)); diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index 971ba857c21..4be2de0a3cc 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -2020,21 +2020,59 @@ export function issueRoutes( type TaskAssignmentAuthorizationScope = { issueId?: string | null; projectId?: string | null; + projectIds?: string[] | null; parentIssueId?: string | null; assigneeAgentId?: string | null; assigneeUserId?: string | null; }; - async function resolveAssignmentProjectId(input: { + function dedupeProjectIds(projectIds: readonly string[]) { + const seen = new Set(); + const deduped: string[] = []; + for (const projectId of projectIds) { + if (seen.has(projectId)) continue; + seen.add(projectId); + deduped.push(projectId); + } + return deduped; + } + + function issueProjectIdsForAuthorization(issue?: { + projectId?: string | null; + projects?: Array<{ id?: string | null }> | null; + } | null) { + const projectedProjectIds = Array.isArray(issue?.projects) + ? issue.projects + .map((project) => project.id) + .filter((projectId): projectId is string => typeof projectId === "string" && projectId.length > 0) + : []; + if (projectedProjectIds.length > 0) return dedupeProjectIds(projectedProjectIds); + return typeof issue?.projectId === "string" && issue.projectId.length > 0 ? [issue.projectId] : []; + } + + function effectiveProjectIdsFromIssueBody( + body: { projectId?: string | null; projectIds?: string[] }, + fallbackProjectIds: readonly string[] = [], + ) { + if (Array.isArray(body.projectIds)) return dedupeProjectIds(body.projectIds); + if (body.projectId !== undefined) { + return body.projectId + ? dedupeProjectIds([body.projectId, ...fallbackProjectIds.filter((projectId) => projectId !== body.projectId)]) + : []; + } + return dedupeProjectIds(fallbackProjectIds); + } + + async function resolveAssignmentProjectIds(input: { companyId: string; - projectId: string | null | undefined; + projectIds: string[] | null | undefined; parentIssueId?: string | null; }) { - if (input.projectId !== undefined) return input.projectId; + if (input.projectIds !== undefined && input.projectIds !== null) return input.projectIds; if (!input.parentIssueId) return null; const parent = await svc.getById(input.parentIssueId); if (!parent || parent.companyId !== input.companyId) return null; - return parent.projectId ?? null; + return issueProjectIdsForAuthorization(parent); } function primaryProjectIdFromIssueBody( @@ -2052,22 +2090,37 @@ export function issueRoutes( assignmentScope?: TaskAssignmentAuthorizationScope, ) { assertCompanyAccess(req, companyId); - const decision = await access.decide({ - actor: req.actor, - action: "tasks:assign", - resource: { - type: "issue", - companyId, - issueId: assignmentScope?.issueId ?? null, - projectId: assignmentScope?.projectId ?? null, - parentIssueId: assignmentScope?.parentIssueId ?? null, - assigneeAgentId: assignmentScope?.assigneeAgentId ?? null, - assigneeUserId: assignmentScope?.assigneeUserId ?? null, - }, - scope: assignmentScope ?? null, - }); - if (decision.allowed) return; - throw forbidden(decision.explanation, authorizationDeniedDetails(decision)); + const projectIds = assignmentScope?.projectIds !== undefined && assignmentScope.projectIds !== null + ? dedupeProjectIds(assignmentScope.projectIds) + : assignmentScope?.projectId + ? [assignmentScope.projectId] + : []; + const projectScopes: Array = projectIds.length > 0 + ? projectIds + : [assignmentScope?.projectId ?? null]; + for (const projectId of projectScopes) { + const decision = await access.decide({ + actor: req.actor, + action: "tasks:assign", + resource: { + type: "issue", + companyId, + issueId: assignmentScope?.issueId ?? null, + projectId, + projectIds, + parentIssueId: assignmentScope?.parentIssueId ?? null, + assigneeAgentId: assignmentScope?.assigneeAgentId ?? null, + assigneeUserId: assignmentScope?.assigneeUserId ?? null, + }, + scope: { + ...(assignmentScope ?? {}), + projectId, + projectIds, + }, + }); + if (decision.allowed) continue; + throw forbidden(decision.explanation, authorizationDeniedDetails(decision)); + } } function isTaskBridgeKeyActor(req: Request) { @@ -2089,7 +2142,7 @@ export function issueRoutes( await assertCanAssignTasks(req, companyId, assignmentScope); } - async function decideIssueAccess( + async function decideIssueAccessForProjectId( req: Request, issue: { id: string; @@ -2101,6 +2154,8 @@ export function issueRoutes( status: string; }, action: "issue:comment" | "issue:read" | "issue:mutate", + projectId: string | null, + projectIds: string[], ) { return access.decide({ actor: req.actor, @@ -2109,7 +2164,8 @@ export function issueRoutes( type: "issue", companyId: issue.companyId, issueId: issue.id, - projectId: issue.projectId, + projectId, + projectIds, parentIssueId: issue.parentId, assigneeAgentId: issue.assigneeAgentId, assigneeUserId: issue.assigneeUserId, @@ -2117,7 +2173,8 @@ export function issueRoutes( }, scope: { issueId: issue.id, - projectId: issue.projectId, + projectId, + projectIds, parentIssueId: issue.parentId, assigneeAgentId: issue.assigneeAgentId, assigneeUserId: issue.assigneeUserId, @@ -2125,6 +2182,31 @@ export function issueRoutes( }); } + async function decideIssueAccess( + req: Request, + issue: { + id: string; + companyId: string; + projectId: string | null; + projects?: Array<{ id?: string | null }> | null; + parentId: string | null; + assigneeAgentId: string | null; + assigneeUserId: string | null; + status: string; + }, + action: "issue:comment" | "issue:read" | "issue:mutate", + ) { + const projectIds = issueProjectIdsForAuthorization(issue); + const projectScopes: Array = projectIds.length > 0 ? projectIds : [issue.projectId ?? null]; + let firstDecision: Awaited> | null = null; + for (const projectId of projectScopes) { + const decision = await decideIssueAccessForProjectId(req, issue, action, projectId, projectIds); + firstDecision ??= decision; + if (decision.allowed) return decision; + } + return firstDecision ?? await decideIssueAccessForProjectId(req, issue, action, issue.projectId ?? null, projectIds); + } + async function assertIssueReadAllowed(req: Request, res: Response, issue: Parameters[1]) { const decision = await decideIssueAccess(req, issue, "issue:read"); if (decision.allowed) return true; @@ -5344,11 +5426,16 @@ export function issueRoutes( : {}), }; const createPrimaryProjectId = primaryProjectIdFromIssueBody(createBody); + const createProjectIds = effectiveProjectIdsFromIssueBody( + createBody, + createParent ? issueProjectIdsForAuthorization(createParent) : [], + ); if (!(await assertCheapRecoveryIssueAssigneeProfileAllowed(req, res, { companyId }, createBody))) return; const createAssignmentScope = { - projectId: await resolveAssignmentProjectId({ + projectId: createPrimaryProjectId, + projectIds: await resolveAssignmentProjectIds({ companyId, - projectId: createPrimaryProjectId, + projectIds: createProjectIds, parentIssueId: createBody.parentId, }), parentIssueId: createBody.parentId ?? null, @@ -5515,8 +5602,10 @@ export function issueRoutes( }; if (!(await assertCheapRecoveryIssueAssigneeProfileAllowed(req, res, parent, createBody))) return; const childPrimaryProjectId = primaryProjectIdFromIssueBody(createBody, parent.projectId ?? null); + const childProjectIds = effectiveProjectIdsFromIssueBody(createBody, issueProjectIdsForAuthorization(parent)); const childAssignmentScope = { projectId: childPrimaryProjectId, + projectIds: childProjectIds, parentIssueId: parent.id, assigneeAgentId: createBody.assigneeAgentId ?? null, assigneeUserId: createBody.assigneeUserId ?? null, @@ -5701,6 +5790,7 @@ export function issueRoutes( if (childBody.assigneeAgentId || childBody.assigneeUserId) { await assertCanAssignTasks(req, sourceIssue.companyId, { projectId: primaryProjectIdFromIssueBody(childBody, sourceIssue.projectId ?? null), + projectIds: effectiveProjectIdsFromIssueBody(childBody, issueProjectIdsForAuthorization(sourceIssue)), parentIssueId: sourceIssue.id, assigneeAgentId: childBody.assigneeAgentId ?? null, assigneeUserId: childBody.assigneeUserId ?? null, @@ -6222,11 +6312,15 @@ export function issueRoutes( if (!isAgentReturningIssueToCreator) { await assertCanAssignTasks(req, existing.companyId, { issueId: existing.id, - projectId: await resolveAssignmentProjectId({ + projectId: primaryProjectIdFromIssueBody( + updateFields as { projectId?: string | null; projectIds?: string[] }, + existing.projectId, + ), + projectIds: await resolveAssignmentProjectIds({ companyId: existing.companyId, - projectId: primaryProjectIdFromIssueBody( + projectIds: effectiveProjectIdsFromIssueBody( updateFields as { projectId?: string | null; projectIds?: string[] }, - existing.projectId, + issueProjectIdsForAuthorization(existing), ), parentIssueId: (updateFields.parentId === undefined ? existing.parentId diff --git a/server/src/services/authorization.ts b/server/src/services/authorization.ts index dccd0ea6c14..fe541bad0f0 100644 --- a/server/src/services/authorization.ts +++ b/server/src/services/authorization.ts @@ -69,6 +69,7 @@ export type AuthorizationResource = companyId: string; issueId?: string | null; projectId?: string | null; + projectIds?: string[] | null; parentIssueId?: string | null; assigneeAgentId?: string | null; assigneeUserId?: string | null; @@ -795,10 +796,14 @@ export function authorizationService(db: Db) { resource: Extract, ) { const issue = resource.issueId ? await loadIssue(resource.issueId) : null; + const projectIds = Array.isArray(resource.projectIds) && resource.projectIds.length > 0 + ? [...new Set(resource.projectIds)] + : []; const candidate = { companyId: resource.companyId, id: issue?.id ?? resource.issueId ?? null, projectId: issue?.projectId ?? resource.projectId ?? null, + projectIds, }; if (isIssueWithinLowTrustBoundary(boundary, candidate)) return true; if (candidate.id && boundary.rootIssueId) { @@ -976,11 +981,26 @@ export function authorizationService(db: Db) { ) { const allowedProjectIds = taskBridgeScopeIds(scope, "projectId", "projectIds"); const allowedParentIssueIds = taskBridgeScopeIds(scope, "parentIssueId", "parentIssueIds"); - if (resource.projectId && allowedProjectIds.includes(resource.projectId)) return true; + const requestedProjectIds = [ + ...new Set( + (Array.isArray(resource.projectIds) && resource.projectIds.length > 0 + ? resource.projectIds + : resource.projectId + ? [resource.projectId] + : []) + .filter((projectId): projectId is string => typeof projectId === "string" && projectId.length > 0), + ), + ]; + if (requestedProjectIds.length > 0 && allowedProjectIds.length > 0) { + if (requestedProjectIds.every((projectId) => allowedProjectIds.includes(projectId))) return true; + } else if (resource.projectId && allowedProjectIds.includes(resource.projectId)) { + return true; + } if (await parentIssueMatchesTaskBridgeBoundary(resource.parentIssueId, resource.companyId, allowedParentIssueIds)) { return true; } if (resource.parentIssueId && allowedProjectIds.length > 0) { + if (requestedProjectIds.some((projectId) => !allowedProjectIds.includes(projectId))) return false; const parent = await loadIssue(resource.parentIssueId); if (parent?.companyId === resource.companyId && parent.projectId && allowedProjectIds.includes(parent.projectId)) { return true; diff --git a/server/src/services/trust-preset-resolver.ts b/server/src/services/trust-preset-resolver.ts index 97529840bcd..e10692307dd 100644 --- a/server/src/services/trust-preset-resolver.ts +++ b/server/src/services/trust-preset-resolver.ts @@ -339,11 +339,12 @@ export function resolveCoreTrustPreset(input: ResolveCoreTrustPresetInput): Trus export function isIssueWithinLowTrustBoundary( boundary: LowTrustBoundary & { companyId: string }, - issue: { companyId: string; id?: string | null; projectId?: string | null }, + issue: { companyId: string; id?: string | null; projectId?: string | null; projectIds?: string[] | null }, ): boolean { if (issue.companyId !== boundary.companyId) return false; if (issue.id && issue.id === boundary.rootIssueId) return true; if (issue.id && boundary.issueIds?.includes(issue.id)) return true; if (issue.projectId && boundary.projectIds?.includes(issue.projectId)) return true; + if (issue.projectIds?.some((projectId) => boundary.projectIds?.includes(projectId))) return true; return false; }