From 55f3b40aff549871a42e83759726783e1893296b Mon Sep 17 00:00:00 2001 From: HKTITAN Date: Thu, 11 Jun 2026 19:33:32 +0530 Subject: [PATCH] review: viewer carve-out for runtime:manage/secrets:read + issue:mutate regression test Per Greptile review: viewers keep the read-only visibility actions (agent:read, company_scope:read, issue:read, project:read) but are denied runtime:manage and secrets:read with deny_missing_grant, matching the tasks:assign viewer carve-out in the same board block. Adds the missing issue:mutate deny_unsupported_action assertion and a viewer-role coverage test. Co-Authored-By: Claude Fable 5 --- .../__tests__/authorization-service.test.ts | 54 +++++++++++++++++-- server/src/services/authorization.ts | 13 ++++- 2 files changed, 62 insertions(+), 5 deletions(-) diff --git a/server/src/__tests__/authorization-service.test.ts b/server/src/__tests__/authorization-service.test.ts index b5dc54111b..1d156105c6 100644 --- a/server/src/__tests__/authorization-service.test.ts +++ b/server/src/__tests__/authorization-service.test.ts @@ -598,16 +598,62 @@ describeEmbeddedPostgres("authorization service", () => { membershipRole: "member", }); - const decision = await authorizationService(db).decide({ + const authorization = authorizationService(db); + + await expect(authorization.decide({ actor: { type: "board", userId, source: "session" }, action: "agent:wake", resource: { type: "agent", companyId: company.id, agentId: targetAgent.id }, - }); - - expect(decision).toMatchObject({ + })).resolves.toMatchObject({ allowed: false, reason: "deny_unsupported_action", }); + const issue = await createIssue(db, company.id, { title: "Wake denied issue" }); + await expect(authorization.decide({ + actor: { type: "board", userId, source: "session" }, + action: "issue:mutate", + resource: { type: "issue", companyId: company.id, issueId: issue.id }, + })).resolves.toMatchObject({ + allowed: false, + reason: "deny_unsupported_action", + }); + }); + + it("limits viewer members to read-only visibility actions", async () => { + const company = await createCompany(db, "BoardViewerVisibility"); + const userId = `user-${randomUUID()}`; + const targetAgent = await createAgent(db, company.id, { role: "engineer" }); + await db.insert(companyMemberships).values({ + companyId: company.id, + principalType: "user", + principalId: userId, + status: "active", + membershipRole: "viewer", + }); + + const authorization = authorizationService(db); + const actor = { type: "board", userId, source: "session" } as const; + + await expect(authorization.decide({ + actor, + action: "agent:read", + resource: { type: "agent", companyId: company.id, agentId: targetAgent.id }, + })).resolves.toMatchObject({ allowed: true, reason: "allow_simple_company_member" }); + await expect(authorization.decide({ + actor, + action: "company_scope:read", + resource: { type: "company", companyId: company.id }, + })).resolves.toMatchObject({ allowed: true, reason: "allow_simple_company_member" }); + await expect(authorization.decide({ + actor, + action: "runtime:manage", + resource: { type: "company", companyId: company.id }, + })).resolves.toMatchObject({ allowed: false, reason: "deny_missing_grant" }); + await expect(authorization.decide({ + actor, + action: "secrets:read", + resource: { type: "company", companyId: company.id }, + })).resolves.toMatchObject({ allowed: false, reason: "deny_missing_grant" }); }); it("denies legacy board assignment context for viewers", async () => { diff --git a/server/src/services/authorization.ts b/server/src/services/authorization.ts index 4461ced2e8..df7af2a595 100644 --- a/server/src/services/authorization.ts +++ b/server/src/services/authorization.ts @@ -967,13 +967,24 @@ export function authorizationService(db: Db) { input.action === "secrets:read" ) { const membership = await getActiveMembership(companyId, "user", input.actor.userId); - if (membership) { + // Mirroring the tasks:assign carve-out above, viewers keep the + // read-only visibility actions but not the privileged ones. + const requiresNonViewer = + input.action === "runtime:manage" || input.action === "secrets:read"; + if (membership && (!requiresNonViewer || membership.membershipRole !== "viewer")) { return allow({ action: input.action, reason: "allow_simple_company_member", explanation: "Allowed by standard same-company board membership visibility.", }); } + if (membership) { + return deny({ + action: input.action, + reason: "deny_missing_grant", + explanation: `Viewer membership does not grant ${input.action}.`, + }); + } return deny({ action: input.action, reason: "deny_missing_membership",