From 3e1ea39ff3cdb9288c15009da9c4b828bc5da7d4 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Tue, 11 Aug 2026 13:23:03 -0400 Subject: [PATCH] fix(inbox): honor saved policy for explicit targets (#11221) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source control plane people use to manage AI-agent companies and their work > - Each user can let agents tidy that user's Mine inbox > - The profile control saves either an open policy or an agent allowlist > - Explicit inbox archive requests checked only the separate `inbox:manage` grant > - This made the saved profile control ineffective for explicit user targets > - This pull request makes authorization honor the target user's saved policy > - The benefit is that the UI control and the API now enforce the same user choice ## Linked Issues or Issue Description **What happened?** An agent received `403 inbox_cross_user_grant_required` when it archived an issue with an explicit `userId`. The denial occurred even when that user had enabled inbox management for the agent in Profile Settings. The authorization service checked only `principal_permission_grants` for explicit targets and ignored the saved user inbox policy. **Expected behavior** An explicit target is allowed when the target user saved an `open` policy or an allowlist that contains the agent. An unsaved default-open policy must remain limited to the responsible-user path. A scoped `inbox:manage` grant must remain an administrative override. **Steps to reproduce** 1. Save an inbox-agent allowlist for a user. 2. Include the acting agent in that allowlist. 3. Call `POST /api/issues/{issueId}/inbox-archive` with that user's explicit `userId`. 4. Observe the incorrect `403 inbox_cross_user_grant_required` response on the previous implementation. **Paperclip version or commit** Reproduced on `7ea2068ef8`. **Deployment mode** Self-hosted server. **Installation method** Built from source with pnpm. **Agent adapter(s) involved** Not adapter-specific. This is a core authorization bug. **Database mode** External Postgres in production. The regression tests use embedded PostgreSQL. **Access context** Agent bearer authentication. Related foundations: #9658 and #9724. ## What Changed - Read the target user's saved inbox-agent policy before the explicit-target decision. - Allow saved `open` policies and matching allowlists for explicit targets. - Keep unsaved implicit-open policies responsible-user-only. - Keep scoped `inbox:manage` grants as administrative overrides. - Add service and route regressions for allow, deny, archive, unarchive, and audit metadata. - Update the implementation contract and agent-facing inbox API guidance. ## Verification - `pnpm exec vitest run server/src/__tests__/authorization-service.test.ts server/src/__tests__/inbox-archive-routes.test.ts` — 66 tests passed. - `pnpm --filter @paperclipai/server typecheck` — passed. - `git diff --check origin/master...HEAD` — passed. ## Risks - Low risk. The change is limited to explicit inbox targets with a saved policy. - A missing policy row still denies explicit cross-user access. - A non-matching allowlist and a disabled policy still deny access unless a scoped administrative grant applies. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI Codex based on GPT-5. The runtime did not expose the exact model build or context-window size. The agent used reasoning, repository tools, code execution, and focused test execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip --- doc/SPEC-implementation.md | 6 +- .../__tests__/authorization-service.test.ts | 72 ++++++++++++ .../__tests__/inbox-archive-routes.test.ts | 31 ++++++ server/src/services/authorization.ts | 105 ++++++++++++------ skills/paperclip/SKILL.md | 2 +- skills/paperclip/references/api-reference.md | 4 +- 6 files changed, 177 insertions(+), 43 deletions(-) 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`) |