mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-04 19:28:46 +01:00
fix(inbox): honor saved policy for explicit targets (#11221)
<!-- Write all pull request text in Simplified Technical English
(ASD-STE100): short sentences, one instruction per sentence, simple
approved vocabulary, and the active voice. -->
## 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 <noreply@paperclip.ing>
This commit is contained in:
1 parent
71e9d6bb0b
commit
3e1ea39ff3
6 files changed
+177
-43
No files matched your search
@@ -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);
|
||||
|
||||
@@ -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",
|
||||
}),
|
||||
]));
|
||||
});
|
||||
});
|
||||
@@ -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,
|
||||
|
||||
Reference in new issue
Block a user