diff --git a/doc/SPEC-implementation.md b/doc/SPEC-implementation.md index 64b58326d1..2584bdc724 100644 --- a/doc/SPEC-implementation.md +++ b/doc/SPEC-implementation.md @@ -556,7 +556,7 @@ Detailed ownership, execution, blocker, active-run watchdog, crash-recovery, and | Set company budget | yes | no | | Set subordinate budget | yes | yes (manager subtree only) | | Manage responsible user's inbox state | yes | yes (default-open policy) | -| Manage another user's inbox state | yes | scoped `inbox:manage` grant | +| Manage another user's inbox state | yes | saved target-user opt-in or scoped `inbox:manage` grant | | Set work-object visibility (issue/project) | no | no (pro gate) | ### 9.3.1 Shared default-open issue writes @@ -624,7 +624,7 @@ The approved term set is: | Work-object visibility | All issues and projects in-company are visible to board and agents | Project/issue ACLs and reviewer-only channels | | Tool/secret policy | Secret refs, log redaction, and adapter-level command/webhook restrictions | Tool allowlists with centralized policy evaluation | | Company skills | Open to authenticated company agents; core enforces invariants and any stored restriction policy | Paperclip EE policy editor, protected-skill controls, presets, simulation, and policy audit UX | -| Inbox management | Responsible agent may archive/unarchive its responsible user's Mine items under a default-open user policy; cross-user access requires `inbox:manage`; all mutations are audited | Policy administration UX, organization presets, simulations, bulk controls, and richer audit/reporting surfaces | +| Inbox management | Responsible agent may archive/unarchive its responsible user's Mine items under a default-open user policy; explicit cross-user access requires saved target-user opt-in or `inbox:manage`; all mutations are audited | Policy administration UX, organization presets, simulations, bulk controls, and richer audit/reporting surfaces | | Escalation | Escalate from agent to manager to board; board approval/budget gates remain authoritative | Escalation routing and SLA windows | ## 9.7 Recommended first-slice implementation order @@ -870,7 +870,7 @@ Core authorization follows these rules: - Board users may archive or unarchive inbox entries for users in the company. - An agent may manage the responsible user's inbox without an explicit grant when the authenticated run resolves that user and the user's inbox-agent policy permits the agent. This is the default-open path. - A user may set inbox-agent policy to `disabled` or `allowlist`. Policy restrictions override the default-open path, and low-trust agents are denied. -- An agent targeting any user other than its resolved responsible user requires an explicit `inbox:manage` grant. Grants may be unscoped or constrained by `scope.userIds`. +- An agent targeting any user other than its resolved responsible user requires either a materialized target-user policy that permits that agent (`open` or matching `allowlist`) or an explicit `inbox:manage` grant. The implicit default-open policy for a missing row remains responsible-user-only, so it never becomes a blanket cross-user grant. Grants may be unscoped or constrained by `scope.userIds` and act as administrative overrides, including over a disabled target-user policy. - Archive and unarchive operations are company-scoped, reversible, and activity logged with actor, agent, run, target user, target-resolution source, and policy mode. - New qualifying issue activity may invalidate an archive so the item resurfaces; archival is not a substitute for resolving or closing work. - Viewing an issue may update its per-user read receipt, but read receipts alone do not enroll the issue in Mine. Mine participation begins with a user-authored comment, issue creation/assignment, or another audited user mutation; explicit product actions such as manually running a routine may record an audited inbox touch. diff --git a/server/src/__tests__/authorization-service.test.ts b/server/src/__tests__/authorization-service.test.ts index 22b8f38bbb..821eae7d46 100644 --- a/server/src/__tests__/authorization-service.test.ts +++ b/server/src/__tests__/authorization-service.test.ts @@ -2276,6 +2276,78 @@ describeEmbeddedPostgres("authorization service", () => { })).resolves.toMatchObject({ allowed: false, reason: "deny_missing_grant" }); }); + it("honors a target user's saved inbox policy for cross-user management", async () => { + const company = await createCompany(db, "InboxCrossUserPolicy"); + const actorAgent = await createAgent(db, company.id); + const deniedAgent = await createAgent(db, company.id); + const responsibleUserId = await createUser(db); + const allowlistTargetUserId = await createUser(db); + const openTargetUserId = await createUser(db); + await db.insert(companyMemberships).values([ + { + companyId: company.id, + principalType: "user", + principalId: responsibleUserId, + status: "active", + membershipRole: "operator", + }, + { + companyId: company.id, + principalType: "user", + principalId: allowlistTargetUserId, + status: "active", + membershipRole: "operator", + }, + { + companyId: company.id, + principalType: "user", + principalId: openTargetUserId, + status: "active", + membershipRole: "operator", + }, + ]); + await db.insert(userInboxAgentPolicies).values([ + { + companyId: company.id, + userId: allowlistTargetUserId, + mode: "allowlist", + allowedAgentIds: [actorAgent.id], + }, + { + companyId: company.id, + userId: openTargetUserId, + mode: "open", + }, + ]); + const decideFor = (agentId: string, userId: string) => authorizationService(db).decide({ + actor: { + type: "agent" as const, + agentId, + companyId: company.id, + onBehalfOfUserId: responsibleUserId, + source: "agent_jwt" as const, + }, + action: "inbox:manage" as const, + resource: { type: "company" as const, companyId: company.id }, + scope: { userId }, + }); + + await expect(decideFor(actorAgent.id, allowlistTargetUserId)).resolves.toMatchObject({ + allowed: true, + reason: "allow_user_inbox_policy", + inboxPolicyMode: "allowlist", + }); + await expect(decideFor(deniedAgent.id, allowlistTargetUserId)).resolves.toMatchObject({ + allowed: false, + reason: "inbox_agent_not_allowed", + }); + await expect(decideFor(actorAgent.id, openTargetUserId)).resolves.toMatchObject({ + allowed: true, + reason: "allow_user_inbox_policy", + inboxPolicyMode: "open", + }); + }); + it("allows cross-user inbox management with an unscoped grant", async () => { const company = await createCompany(db, "InboxCrossUserGranted"); const actorAgent = await createAgent(db, company.id); diff --git a/server/src/__tests__/inbox-archive-routes.test.ts b/server/src/__tests__/inbox-archive-routes.test.ts index 563996fb50..b33663e8ca 100644 --- a/server/src/__tests__/inbox-archive-routes.test.ts +++ b/server/src/__tests__/inbox-archive-routes.test.ts @@ -488,4 +488,35 @@ describeEmbeddedPostgres("inbox archive routes", () => { }), ])); }); + + it("honors the target user's allowlist for explicit archive and unarchive", async () => { + const seeded = await seed(); + const app = appFor(agentActor(seeded)); + await db.insert(userInboxAgentPolicies).values({ + companyId: seeded.companyId, + userId: seeded.targetUserId, + mode: "allowlist", + allowedAgentIds: [seeded.agentId], + }); + + await request(app) + .post(`/api/issues/${seeded.issueId}/inbox-archive`) + .send({ userId: seeded.targetUserId }) + .expect(200) + .expect(({ body }) => expect(body).toMatchObject({ userId: seeded.targetUserId })); + await request(app) + .delete(`/api/issues/${seeded.issueId}/inbox-archive`) + .send({ userId: seeded.targetUserId }) + .expect(200) + .expect(({ body }) => expect(body).toMatchObject({ userId: seeded.targetUserId })); + + const auditRows = await db.select().from(activityLog); + expect(auditRows.map((row) => row.details)).toEqual(expect.arrayContaining([ + expect.objectContaining({ + userId: seeded.targetUserId, + targetResolvedFrom: "explicit", + policyMode: "allowlist", + }), + ])); + }); }); diff --git a/server/src/services/authorization.ts b/server/src/services/authorization.ts index 1cf34ab64d..807cde2f47 100644 --- a/server/src/services/authorization.ts +++ b/server/src/services/authorization.ts @@ -102,6 +102,7 @@ export type AuthorizationDecision = { | "allow_local_board" | "allow_instance_admin" | "allow_explicit_grant" + | "allow_user_inbox_policy" | "allow_direct_change" | "allow_consented_change" | "allow_legacy_agent_creator" @@ -1949,43 +1950,6 @@ export function authorizationService(db: Db) { }); } - if (targetUserId !== responsibleUserId) { - // Cross-user grants are board-admin overrides; user policies only govern responsible-user default access. - const grant = await findGrant(companyId, "agent", actorAgentId, "inbox:manage"); - if (!grant) { - return deny({ - action: input.action, - reason: "deny_missing_grant", - explanation: "Missing permission: inbox:manage.", - }); - } - if (!(await scopeAllows(db, companyId, grant.scope, { userId: targetUserId }))) { - return deny({ - action: input.action, - reason: "deny_scope", - explanation: "Permission inbox:manage does not cover the requested user.", - grant: { - principalType: "agent", - principalId: actorAgentId, - permissionKey: "inbox:manage", - scope: grant.scope ?? null, - }, - }); - } - return allow({ - action: input.action, - reason: "allow_explicit_grant", - explanation: "Allowed by explicit grant inbox:manage.", - inboxPolicyMode: "grant_override", - grant: { - principalType: "agent", - principalId: actorAgentId, - permissionKey: "inbox:manage", - scope: grant.scope ?? null, - }, - }); - } - const policy = await db .select({ mode: userInboxAgentPolicies.mode, @@ -2000,6 +1964,73 @@ export function authorizationService(db: Db) { ) .then((rows) => rows[0] ?? null); + if (targetUserId !== responsibleUserId) { + // A scoped grant remains an administrative override, including over a + // disabled user policy. Otherwise, a materialized target-user policy is + // explicit consent for agents selected in the profile control. The + // implicit default-open policy remains responsible-user-only so an + // absent row never becomes a company-wide cross-user grant. + const grant = await findGrant(companyId, "agent", actorAgentId, "inbox:manage"); + if (grant && (await scopeAllows(db, companyId, grant.scope, { userId: targetUserId }))) { + return allow({ + action: input.action, + reason: "allow_explicit_grant", + explanation: "Allowed by explicit grant inbox:manage.", + inboxPolicyMode: "grant_override", + grant: { + principalType: "agent", + principalId: actorAgentId, + permissionKey: "inbox:manage", + scope: grant.scope ?? null, + }, + }); + } + + if (policy?.mode === "disabled") { + return deny({ + action: input.action, + reason: "inbox_management_disabled", + explanation: `Inbox management is disabled for user ${targetUserId}.`, + }); + } + if (policy?.mode === "allowlist" && !policy.allowedAgentIds.includes(actorAgentId)) { + return deny({ + action: input.action, + reason: "inbox_agent_not_allowed", + explanation: `Agent ${actorAgentId} is not allowed to manage user ${targetUserId}'s inbox.`, + }); + } + if (policy?.mode === "open" || policy?.mode === "allowlist") { + return allow({ + action: input.action, + reason: "allow_user_inbox_policy", + inboxPolicyMode: policy.mode, + explanation: policy.mode === "allowlist" + ? "Allowed by the target user's inbox agent allowlist." + : "Allowed by the target user's saved open inbox policy.", + }); + } + + if (grant) { + return deny({ + action: input.action, + reason: "deny_scope", + explanation: "Permission inbox:manage does not cover the requested user.", + grant: { + principalType: "agent", + principalId: actorAgentId, + permissionKey: "inbox:manage", + scope: grant.scope ?? null, + }, + }); + } + return deny({ + action: input.action, + reason: "deny_missing_grant", + explanation: "Missing permission: inbox:manage.", + }); + } + if (policy?.mode === "disabled") { return deny({ action: input.action, diff --git a/skills/paperclip/SKILL.md b/skills/paperclip/SKILL.md index 728395f5ba..cf1a97cd5f 100644 --- a/skills/paperclip/SKILL.md +++ b/skills/paperclip/SKILL.md @@ -177,7 +177,7 @@ Run-scoped writes are subtree-scoped: the delegate's run can write to its own is ## Managing A User's Inbox -Agents may archive an issue from a user's Mine inbox with `POST /api/issues/{issueId}/inbox-archive` and reverse it with `DELETE /api/issues/{issueId}/inbox-archive`. Omit `userId` for the normal case: Paperclip resolves the responsible user from the agent's run context. An explicit `userId` targets another user and requires a matching `inbox:manage` grant. +Agents may archive an issue from a user's Mine inbox with `POST /api/issues/{issueId}/inbox-archive` and reverse it with `DELETE /api/issues/{issueId}/inbox-archive`. Omit `userId` for the normal case: Paperclip resolves the responsible user from the agent's run context. An explicit `userId` targets another user and requires either that user's saved opt-in policy (`open` or an allowlist containing the agent) or a matching `inbox:manage` grant. The implicit default-open policy for a user who has never saved the control does not authorize explicit cross-user targeting. Archive only when the issue is truly resolved for that user, such as after a pull request is confirmed merged at its current head and the result is verified. Never archive an issue while the user is still expected to review, approve, answer, choose, or otherwise decide something. Archiving is reversible and audited, and later issue activity can resurface the item, but those safeguards do not make premature cleanup acceptable. diff --git a/skills/paperclip/references/api-reference.md b/skills/paperclip/references/api-reference.md index db5d5a09e5..e41f31b462 100644 --- a/skills/paperclip/references/api-reference.md +++ b/skills/paperclip/references/api-reference.md @@ -540,7 +540,7 @@ DELETE /api/issues/issue-310/inbox-archive Both mutations require `X-Paperclip-Run-Id` and write activity-log entries. Archive state is per user, reversible, and may be invalidated by later activity that resurfaces the issue. Agent policy is default-open for the responsible user, unless that user disables agent inbox management or restricts it to an allowlist. -Pass `{ "userId": "user-9" }` only for an intentional cross-user operation. The agent must have `inbox:manage`, optionally scoped to that user. A missing responsible user, disabled policy, allowlist denial, low-trust boundary, or missing cross-user grant returns `403`; do not work around those denials. +Pass `{ "userId": "user-9" }` only for an intentional cross-user operation. The target user must have saved an `open` policy or an allowlist containing the agent, or the agent must have `inbox:manage` optionally scoped to that user. An unsaved implicit-open policy is responsible-user-only. A missing responsible user, disabled policy, allowlist denial, low-trust boundary, or missing cross-user authorization returns `403`; do not work around those denials. ### Worked Example: Reviewer / Approver Heartbeat @@ -1253,7 +1253,7 @@ Terminal states: `done`, `cancelled` | GET | `/api/issues/:issueId/comments` | List comments | | GET | `/api/issues/:issueId/comments/:commentId` | Get a specific comment by ID | | POST | `/api/issues/:issueId/comments` | Add comment (@-mentions trigger wakeups) | -| POST | `/api/issues/:issueId/inbox-archive` | Archive issue from responsible user's inbox; optional `userId` requires cross-user grant | +| POST | `/api/issues/:issueId/inbox-archive` | Archive issue from responsible user's inbox; optional `userId` requires saved target-user opt-in or cross-user grant | | DELETE | `/api/issues/:issueId/inbox-archive` | Reverse inbox archive; same target and policy rules | | GET | `/api/issues/:issueId/interactions` | List issue-thread interactions | | POST | `/api/issues/:issueId/interactions` | Create issue-thread interaction (`suggest_tasks`, `ask_user_questions`, `request_confirmation`, `request_checkbox_confirmation`, `request_item_verdicts`) |