fix(server): enforce issue read for issue thread lists (#8331)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agents coordinate through issue threads, so issue comments and interactions are part of the task authorization surface. > - Low-trust and boundary-limited agents can receive narrow mention-scoped access, but that must not turn into broad same-company issue-thread reads. > - The comment creation path was narrowed to allow explicit mention replies without granting mutation access. > - The surrounding list/read routes still needed to enforce the same `issue:read` boundary before returning thread data. > - This pull request applies the issue read check to issue comment and interaction listing routes, and locks that behavior with server regressions. > - The benefit is that narrow cross-agent collaboration remains possible without exposing unrelated issue-thread history. ## Linked Issues or Issue Description Bug fix: - What happened: same-company agents outside an issue read boundary could still hit issue-thread listing routes and receive thread data. - Expected behavior: issue comments and issue-thread interactions should only be listed after the actor is allowed to read the issue. - Steps to reproduce: configure a peer agent denied by the issue read boundary, then request `GET /api/issues/:id/comments` or the issue interaction listing route. - Paperclip version/commit: current `master` before this branch. - Deployment mode: applies to server authorization in all modes. Related public context: #7389, #7863, #8024. ## What Changed - Added issue read enforcement before listing issue comments. - Added issue read enforcement before listing issue-thread interactions. - Added server regressions for denied peer-agent issue-thread access while preserving mention-scoped collaboration behavior. - Removed an avoidable per-mentioned-comment issue reload in mention-grant authorization by passing the already-loaded issue assignee through the helper. - Documented cross-agent issue read/comment authorization behavior in the Paperclip API reference. - Added a resilient fallback for pinned external skills when GitHub tree fetches are temporarily unavailable during catalog builds. ## Verification - `pnpm vitest run server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts server/src/__tests__/authorization-service.test.ts` - 2 test files passed - 84 tests passed - Existing mocked recovery revalidation warnings were emitted by the route suite and the command exited 0 - Greptile: 5/5 on commit `b73fc323f192dc44f88374c16865008d4348923b`; no unresolved review threads. - `pnpm vitest run packages/skills-catalog/src/catalog-builder.test.ts` - 1 test file passed - 6 tests passed - CI: all visible PR checks are terminal green on commit `b73fc323f192dc44f88374c16865008d4348923b`. ## Risks Low risk. This tightens read authorization on issue-thread listing routes; any caller that depended on same-company access without `issue:read` will now receive 403 and must use an explicit grant or valid issue read path. ## Model Used OpenAI GPT-5 Codex, Codex coding agent environment, tool use and local command execution enabled, reasoning mode active. Context window not exposed by the runtime. ## 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] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots - [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:
@@ -6,6 +6,7 @@ import {
|
||||
companyMemberships,
|
||||
createDb,
|
||||
instanceUserRoles,
|
||||
issueComments,
|
||||
issues,
|
||||
principalPermissionGrants,
|
||||
projects,
|
||||
@@ -124,6 +125,7 @@ describeEmbeddedPostgres("authorization service", () => {
|
||||
}, 20_000);
|
||||
|
||||
afterEach(async () => {
|
||||
await db.delete(issueComments);
|
||||
await db.delete(principalPermissionGrants);
|
||||
await db.delete(companyMemberships);
|
||||
await db.delete(instanceUserRoles);
|
||||
@@ -619,6 +621,212 @@ describeEmbeddedPostgres("authorization service", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("allows mentioned agents to read and comment on assigned issues without granting issue mutation", async () => {
|
||||
const company = await createCompany(db, "MentionCommentAuth");
|
||||
const allowedProject = await createProject(db, company.id, "MentionAllowed");
|
||||
const targetProject = await createProject(db, company.id, "MentionTarget");
|
||||
const ownerAgent = await createAgent(db, company.id, { role: "engineer" });
|
||||
const mentionedAgent = await createAgent(db, company.id, {
|
||||
role: "engineer",
|
||||
permissions: {
|
||||
trustPreset: LOW_TRUST_REVIEW_PRESET,
|
||||
authorizationPolicy: {
|
||||
trustBoundary: {
|
||||
mode: LOW_TRUST_REVIEW_PRESET,
|
||||
companyId: company.id,
|
||||
projectIds: [allowedProject.id],
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
const issue = await createIssue(db, company.id, {
|
||||
title: "Mention-scoped comment target",
|
||||
projectId: targetProject.id,
|
||||
assigneeAgentId: ownerAgent.id,
|
||||
});
|
||||
|
||||
const authorization = authorizationService(db);
|
||||
const actor = { type: "agent", agentId: mentionedAgent.id, companyId: company.id, source: "agent_key" } as const;
|
||||
const resource = {
|
||||
type: "issue",
|
||||
companyId: company.id,
|
||||
issueId: issue.id,
|
||||
projectId: issue.projectId,
|
||||
assigneeAgentId: ownerAgent.id,
|
||||
status: issue.status,
|
||||
} as const;
|
||||
|
||||
await expect(authorization.decide({
|
||||
actor,
|
||||
action: "issue:comment",
|
||||
resource,
|
||||
})).resolves.toMatchObject({ allowed: false, reason: "deny_low_trust_boundary" });
|
||||
|
||||
const deletedMention = await db.insert(issueComments).values({
|
||||
companyId: company.id,
|
||||
issueId: issue.id,
|
||||
body: `[@Mentioned Agent](agent://${mentionedAgent.id}) this deleted comment should not count`,
|
||||
deletedAt: new Date(),
|
||||
}).returning().then((rows) => rows[0]!);
|
||||
expect(deletedMention.id).toBeTruthy();
|
||||
|
||||
await expect(authorization.decide({
|
||||
actor,
|
||||
action: "issue:comment",
|
||||
resource,
|
||||
})).resolves.toMatchObject({ allowed: false, reason: "deny_low_trust_boundary" });
|
||||
|
||||
await db.insert(issueComments).values({
|
||||
companyId: company.id,
|
||||
issueId: issue.id,
|
||||
body: `[@Mentioned Agent](agent://${mentionedAgent.id}) please respond here`,
|
||||
authorAgentId: ownerAgent.id,
|
||||
});
|
||||
|
||||
await expect(authorization.decide({
|
||||
actor,
|
||||
action: "issue:read",
|
||||
resource,
|
||||
})).resolves.toMatchObject({
|
||||
allowed: true,
|
||||
reason: "allow_issue_mention_grant",
|
||||
});
|
||||
await expect(authorization.decide({
|
||||
actor,
|
||||
action: "issue:comment",
|
||||
resource,
|
||||
})).resolves.toMatchObject({
|
||||
allowed: true,
|
||||
reason: "allow_issue_mention_grant",
|
||||
});
|
||||
await expect(authorization.decide({
|
||||
actor,
|
||||
action: "issue:mutate",
|
||||
resource,
|
||||
})).resolves.toMatchObject({ allowed: false, reason: "deny_low_trust_boundary" });
|
||||
});
|
||||
|
||||
it("does not grant mention-scoped issue access from self-authored or unauthorized-author comments", async () => {
|
||||
const company = await createCompany(db, "MentionCommentDenied");
|
||||
const allowedProject = await createProject(db, company.id, "MentionDeniedAllowed");
|
||||
const targetProject = await createProject(db, company.id, "MentionDeniedTarget");
|
||||
const ownerAgent = await createAgent(db, company.id, { role: "engineer" });
|
||||
const mentionedAgent = await createAgent(db, company.id, {
|
||||
role: "engineer",
|
||||
permissions: {
|
||||
trustPreset: LOW_TRUST_REVIEW_PRESET,
|
||||
authorizationPolicy: {
|
||||
trustBoundary: {
|
||||
mode: LOW_TRUST_REVIEW_PRESET,
|
||||
companyId: company.id,
|
||||
projectIds: [allowedProject.id],
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
const unrelatedAgent = await createAgent(db, company.id, { role: "engineer" });
|
||||
const issue = await createIssue(db, company.id, {
|
||||
title: "Mention-scoped comment denial target",
|
||||
projectId: targetProject.id,
|
||||
assigneeAgentId: ownerAgent.id,
|
||||
});
|
||||
|
||||
const authorization = authorizationService(db);
|
||||
const actor = { type: "agent", agentId: mentionedAgent.id, companyId: company.id, source: "agent_key" } as const;
|
||||
const resource = {
|
||||
type: "issue",
|
||||
companyId: company.id,
|
||||
issueId: issue.id,
|
||||
projectId: issue.projectId,
|
||||
assigneeAgentId: ownerAgent.id,
|
||||
status: issue.status,
|
||||
} as const;
|
||||
|
||||
await db.insert(issueComments).values([
|
||||
{
|
||||
companyId: company.id,
|
||||
issueId: issue.id,
|
||||
body: `Self mention [@Mentioned Agent](agent://${mentionedAgent.id})`,
|
||||
authorAgentId: mentionedAgent.id,
|
||||
},
|
||||
{
|
||||
companyId: company.id,
|
||||
issueId: issue.id,
|
||||
body: `Unauthorized mention [@Mentioned Agent](agent://${mentionedAgent.id})`,
|
||||
authorAgentId: unrelatedAgent.id,
|
||||
},
|
||||
]);
|
||||
|
||||
await expect(authorization.decide({
|
||||
actor,
|
||||
action: "issue:read",
|
||||
resource,
|
||||
})).resolves.toMatchObject({ allowed: false, reason: "deny_low_trust_boundary" });
|
||||
await expect(authorization.decide({
|
||||
actor,
|
||||
action: "issue:comment",
|
||||
resource,
|
||||
})).resolves.toMatchObject({ allowed: false, reason: "deny_low_trust_boundary" });
|
||||
});
|
||||
|
||||
it("allows active board-user comments to create mention-scoped issue grants", async () => {
|
||||
const company = await createCompany(db, "MentionCommentBoardGrant");
|
||||
const allowedProject = await createProject(db, company.id, "MentionBoardAllowed");
|
||||
const targetProject = await createProject(db, company.id, "MentionBoardTarget");
|
||||
const ownerAgent = await createAgent(db, company.id, { role: "engineer" });
|
||||
const mentionedAgent = await createAgent(db, company.id, {
|
||||
role: "engineer",
|
||||
permissions: {
|
||||
trustPreset: LOW_TRUST_REVIEW_PRESET,
|
||||
authorizationPolicy: {
|
||||
trustBoundary: {
|
||||
mode: LOW_TRUST_REVIEW_PRESET,
|
||||
companyId: company.id,
|
||||
projectIds: [allowedProject.id],
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
const issue = await createIssue(db, company.id, {
|
||||
title: "Mention-scoped board grant target",
|
||||
projectId: targetProject.id,
|
||||
assigneeAgentId: ownerAgent.id,
|
||||
});
|
||||
const boardUserId = `user-${randomUUID()}`;
|
||||
await db.insert(companyMemberships).values({
|
||||
companyId: company.id,
|
||||
principalType: "user",
|
||||
principalId: boardUserId,
|
||||
status: "active",
|
||||
membershipRole: "member",
|
||||
});
|
||||
await db.insert(issueComments).values({
|
||||
companyId: company.id,
|
||||
issueId: issue.id,
|
||||
body: `Board mention [@Mentioned Agent](agent://${mentionedAgent.id})`,
|
||||
authorUserId: boardUserId,
|
||||
});
|
||||
|
||||
const actor = { type: "agent", agentId: mentionedAgent.id, companyId: company.id, source: "agent_key" } as const;
|
||||
const resource = {
|
||||
type: "issue",
|
||||
companyId: company.id,
|
||||
issueId: issue.id,
|
||||
projectId: issue.projectId,
|
||||
assigneeAgentId: ownerAgent.id,
|
||||
status: issue.status,
|
||||
} as const;
|
||||
|
||||
await expect(authorizationService(db).decide({
|
||||
actor,
|
||||
action: "issue:comment",
|
||||
resource,
|
||||
})).resolves.toMatchObject({
|
||||
allowed: true,
|
||||
reason: "allow_issue_mention_grant",
|
||||
});
|
||||
});
|
||||
|
||||
it("limits viewer members to read-only visibility actions", async () => {
|
||||
const company = await createCompany(db, "BoardViewerVisibility");
|
||||
const userId = `user-${randomUUID()}`;
|
||||
|
||||
@@ -23,6 +23,7 @@ const mockIssueService = vi.hoisted(() => ({
|
||||
getWakeableParentAfterChildCompletion: vi.fn(),
|
||||
list: vi.fn(),
|
||||
listAttachments: vi.fn(),
|
||||
listComments: vi.fn(),
|
||||
listWakeableBlockedDependents: vi.fn(),
|
||||
remove: vi.fn(),
|
||||
removeAttachment: vi.fn(),
|
||||
@@ -67,6 +68,7 @@ const mockStorageService = vi.hoisted(() => ({
|
||||
const mockIssueThreadInteractionService = vi.hoisted(() => ({
|
||||
expireRequestConfirmationsSupersededByComment: vi.fn(async () => []),
|
||||
expireStaleRequestConfirmationsForIssueDocument: vi.fn(async () => []),
|
||||
expireRequestConfirmationsSupersededByHistoricalComments: vi.fn(async () => []),
|
||||
listForIssue: vi.fn(async () => []),
|
||||
}));
|
||||
const mockIssueApprovalService = vi.hoisted(() => ({
|
||||
@@ -339,12 +341,14 @@ describe("agent issue mutation checkout ownership", () => {
|
||||
mockAccessService.decide.mockImplementation(async (input: { action: string }) => ({
|
||||
allowed:
|
||||
input.action === "tasks:assign" ||
|
||||
input.action === "issue:comment" ||
|
||||
input.action === "issue:read" ||
|
||||
input.action === "issue:mutate" ||
|
||||
input.action === "company_scope:read",
|
||||
action: input.action,
|
||||
reason:
|
||||
input.action === "tasks:assign" ||
|
||||
input.action === "issue:comment" ||
|
||||
input.action === "issue:read" ||
|
||||
input.action === "issue:mutate" ||
|
||||
input.action === "company_scope:read"
|
||||
@@ -352,6 +356,7 @@ describe("agent issue mutation checkout ownership", () => {
|
||||
: "deny_missing_grant",
|
||||
explanation:
|
||||
input.action === "tasks:assign" ||
|
||||
input.action === "issue:comment" ||
|
||||
input.action === "issue:read" ||
|
||||
input.action === "issue:mutate" ||
|
||||
input.action === "company_scope:read"
|
||||
@@ -375,7 +380,16 @@ describe("agent issue mutation checkout ownership", () => {
|
||||
mockIssueService.getWakeableParentAfterChildCompletion.mockReset();
|
||||
mockIssueService.list.mockReset();
|
||||
mockIssueService.listAttachments.mockReset();
|
||||
mockIssueService.listComments.mockReset();
|
||||
mockIssueService.listWakeableBlockedDependents.mockReset();
|
||||
mockIssueThreadInteractionService.expireRequestConfirmationsSupersededByComment.mockReset();
|
||||
mockIssueThreadInteractionService.expireRequestConfirmationsSupersededByComment.mockResolvedValue([]);
|
||||
mockIssueThreadInteractionService.expireStaleRequestConfirmationsForIssueDocument.mockReset();
|
||||
mockIssueThreadInteractionService.expireStaleRequestConfirmationsForIssueDocument.mockResolvedValue([]);
|
||||
mockIssueThreadInteractionService.expireRequestConfirmationsSupersededByHistoricalComments.mockReset();
|
||||
mockIssueThreadInteractionService.expireRequestConfirmationsSupersededByHistoricalComments.mockResolvedValue([]);
|
||||
mockIssueThreadInteractionService.listForIssue.mockReset();
|
||||
mockIssueThreadInteractionService.listForIssue.mockResolvedValue([]);
|
||||
mockIssueRecoveryActionService.getActiveForIssue.mockReset();
|
||||
mockIssueRecoveryActionService.getActiveForIssue.mockResolvedValue(null);
|
||||
mockIssueRecoveryActionService.listActiveForIssues.mockReset();
|
||||
@@ -530,6 +544,14 @@ describe("agent issue mutation checkout ownership", () => {
|
||||
body: "comment",
|
||||
});
|
||||
mockIssueService.listAttachments.mockResolvedValue([]);
|
||||
mockIssueService.listComments.mockResolvedValue([
|
||||
{
|
||||
id: "comment-1",
|
||||
issueId,
|
||||
companyId,
|
||||
body: "Mentioned reply context.",
|
||||
},
|
||||
]);
|
||||
mockIssueService.remove.mockResolvedValue(makeIssue({ status: "cancelled" }));
|
||||
mockIssueService.getAttachmentById.mockResolvedValue({
|
||||
id: "attachment-1",
|
||||
@@ -636,7 +658,6 @@ describe("agent issue mutation checkout ownership", () => {
|
||||
it.each([
|
||||
["patch", (app: express.Express) => request(app).patch(`/api/issues/${issueId}`).send({ title: "Blocked" })],
|
||||
["delete", (app: express.Express) => request(app).delete(`/api/issues/${issueId}`)],
|
||||
["comment", (app: express.Express) => request(app).post(`/api/issues/${issueId}/comments`).send({ body: "blocked" })],
|
||||
[
|
||||
"document upsert",
|
||||
(app: express.Express) =>
|
||||
@@ -676,6 +697,148 @@ describe("agent issue mutation checkout ownership", () => {
|
||||
expect(mockStorageService.deleteObject).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("allows mentioned peer agents to post comments without ownership of an active checkout", async () => {
|
||||
mockAccessService.decide.mockImplementation(async (input: { action: string }) => ({
|
||||
allowed: input.action === "issue:comment",
|
||||
action: input.action,
|
||||
reason: input.action === "issue:comment" ? "allow_issue_mention_grant" : "deny_missing_grant",
|
||||
explanation:
|
||||
input.action === "issue:comment"
|
||||
? "Allowed by a mention-scoped issue comment grant."
|
||||
: "Missing permission.",
|
||||
}));
|
||||
|
||||
const res = await request(await createApp(peerActor()))
|
||||
.post(`/api/issues/${issueId}/comments`)
|
||||
.send({ body: "I can respond here." });
|
||||
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(201);
|
||||
expect(mockIssueService.addComment).toHaveBeenCalledWith(
|
||||
issueId,
|
||||
"I can respond here.",
|
||||
expect.any(Object),
|
||||
expect.any(Object),
|
||||
);
|
||||
expect(mockIssueService.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects non-mentioned peer agents from posting comments", async () => {
|
||||
mockAccessService.decide.mockImplementation(async (input: { action: string }) => ({
|
||||
allowed: input.action === "issue:read",
|
||||
action: input.action,
|
||||
reason: input.action === "issue:read" ? "allow_explicit_grant" : "deny_missing_grant",
|
||||
explanation: input.action === "issue:read" ? "Allowed by test read grant." : "Missing permission.",
|
||||
}));
|
||||
|
||||
const res = await request(await createApp(peerActor()))
|
||||
.post(`/api/issues/${issueId}/comments`)
|
||||
.send({ body: "I was not mentioned." });
|
||||
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(403);
|
||||
expect(res.body.error).toBe("Issue is outside this actor's authorization boundary");
|
||||
expect(mockIssueService.addComment).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects peer agents from listing comments when issue read is outside their boundary", async () => {
|
||||
mockAccessService.decide.mockImplementation(async (input: { action: string }) => ({
|
||||
allowed: false,
|
||||
action: input.action,
|
||||
reason: "deny_low_trust_boundary",
|
||||
explanation: "Issue is outside this low-trust boundary.",
|
||||
}));
|
||||
|
||||
const res = await request(await createApp(peerActor()))
|
||||
.get(`/api/issues/${issueId}/comments`);
|
||||
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(403);
|
||||
expect(res.body.error).toBe("Issue is outside this actor's authorization boundary");
|
||||
expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({ action: "issue:read" }));
|
||||
});
|
||||
|
||||
it("rejects peer agents from listing interactions when issue read is outside their boundary", async () => {
|
||||
mockAccessService.decide.mockImplementation(async (input: { action: string }) => ({
|
||||
allowed: false,
|
||||
action: input.action,
|
||||
reason: "deny_low_trust_boundary",
|
||||
explanation: "Issue is outside this low-trust boundary.",
|
||||
}));
|
||||
|
||||
const res = await request(await createApp(peerActor()))
|
||||
.get(`/api/issues/${issueId}/interactions`);
|
||||
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(403);
|
||||
expect(res.body.error).toBe("Issue is outside this actor's authorization boundary");
|
||||
expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({ action: "issue:read" }));
|
||||
expect(mockIssueThreadInteractionService.listForIssue).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("allows mentioned peer agents to list comments through an issue read grant", async () => {
|
||||
mockAccessService.decide.mockImplementation(async (input: { action: string }) => ({
|
||||
allowed: input.action === "issue:read",
|
||||
action: input.action,
|
||||
reason: input.action === "issue:read" ? "allow_issue_mention_grant" : "deny_missing_grant",
|
||||
explanation:
|
||||
input.action === "issue:read"
|
||||
? "Allowed by a mention-scoped issue comment grant."
|
||||
: "Missing permission.",
|
||||
}));
|
||||
|
||||
const res = await request(await createApp(peerActor()))
|
||||
.get(`/api/issues/${issueId}/comments`);
|
||||
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(200);
|
||||
expect(res.body).toEqual([
|
||||
expect.objectContaining({
|
||||
id: "comment-1",
|
||||
body: "Mentioned reply context.",
|
||||
}),
|
||||
]);
|
||||
expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({ action: "issue:read" }));
|
||||
expect(mockIssueService.listComments).toHaveBeenCalledWith(issueId, {
|
||||
afterCommentId: null,
|
||||
order: "desc",
|
||||
limit: null,
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps true issue mutations denied for mentioned peer agents", async () => {
|
||||
mockIssueService.getById.mockResolvedValue(makeIssue({ status: "todo", assigneeAgentId: ownerAgentId }));
|
||||
mockAccessService.decide.mockImplementation(async (input: { action: string }) => ({
|
||||
allowed: input.action === "issue:comment" || input.action === "issue:mutate",
|
||||
action: input.action,
|
||||
reason:
|
||||
input.action === "issue:comment"
|
||||
? "allow_issue_mention_grant"
|
||||
: input.action === "issue:mutate"
|
||||
? "allow_explicit_grant"
|
||||
: "deny_missing_grant",
|
||||
explanation:
|
||||
input.action === "issue:comment"
|
||||
? "Allowed by a mention-scoped issue comment grant."
|
||||
: input.action === "issue:mutate"
|
||||
? "Allowed by test boundary default."
|
||||
: "Missing permission.",
|
||||
}));
|
||||
|
||||
const res = await request(await createApp(peerActor()))
|
||||
.patch(`/api/issues/${issueId}`)
|
||||
.send({ status: "done" });
|
||||
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(403);
|
||||
expect(res.body.error).toBe("Agent cannot mutate another agent's issue");
|
||||
expect(mockIssueService.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("denies cross-company agents before comment authorization is evaluated", async () => {
|
||||
const res = await request(await createApp(peerActor({ companyId: "99999999-9999-4999-8999-999999999999" })))
|
||||
.post(`/api/issues/${issueId}/comments`)
|
||||
.send({ body: "Wrong company." });
|
||||
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(403);
|
||||
expect(mockAccessService.decide).not.toHaveBeenCalledWith(expect.objectContaining({ action: "issue:comment" }));
|
||||
expect(mockIssueService.addComment).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects the checked-out owner without a run id on attachment upload (401)", async () => {
|
||||
// Regression: an agent-authenticated client (e.g. the CLI's attachment:upload)
|
||||
// that fails to send X-Paperclip-Run-Id must be rejected — mutating your own
|
||||
@@ -1074,7 +1237,6 @@ describe("agent issue mutation checkout ownership", () => {
|
||||
|
||||
it.each([
|
||||
["todo", "patch", (app: express.Express) => request(app).patch(`/api/issues/${issueId}`).send({ title: "Todo update" })],
|
||||
["todo", "comment", (app: express.Express) => request(app).post(`/api/issues/${issueId}/comments`).send({ body: "Todo noise" })],
|
||||
["blocked", "patch", (app: express.Express) => request(app).patch(`/api/issues/${issueId}`).send({ title: "Blocked update" })],
|
||||
])("rejects peer agent %s issue %s mutations outside active checkout ownership", async (status, _kind, sendRequest) => {
|
||||
mockIssueService.getById.mockResolvedValue(makeIssue({ status: status as "todo" | "blocked", assigneeAgentId: ownerAgentId }));
|
||||
|
||||
@@ -1844,7 +1844,7 @@ export function issueRoutes(
|
||||
assigneeUserId: string | null;
|
||||
status: string;
|
||||
},
|
||||
action: "issue:read" | "issue:mutate",
|
||||
action: "issue:comment" | "issue:read" | "issue:mutate",
|
||||
) {
|
||||
return access.decide({
|
||||
actor: req.actor,
|
||||
@@ -1876,6 +1876,48 @@ export function issueRoutes(
|
||||
return false;
|
||||
}
|
||||
|
||||
async function assertAgentIssueCommentAllowed(
|
||||
req: Request,
|
||||
res: Response,
|
||||
issue: {
|
||||
id: string;
|
||||
companyId: string;
|
||||
projectId: string | null;
|
||||
parentId: string | null;
|
||||
status: string;
|
||||
assigneeAgentId: string | null;
|
||||
assigneeUserId: string | null;
|
||||
},
|
||||
) {
|
||||
if (req.actor.type !== "agent") return true;
|
||||
const actorAgentId = req.actor.agentId;
|
||||
if (!actorAgentId) {
|
||||
res.status(403).json({ error: "Agent authentication required" });
|
||||
return false;
|
||||
}
|
||||
const watchdogScope = await resolveTaskWatchdogMutationScope(db, req.actor);
|
||||
if (watchdogScope.kind !== "none") {
|
||||
const scopeResult = await taskWatchdogScopeAllowsIssueMutation(db, watchdogScope, issue);
|
||||
if (scopeResult.kind === "invalid") {
|
||||
res.status(403).json({
|
||||
error: scopeResult.detail,
|
||||
details: {
|
||||
issueId: issue.id,
|
||||
securityPrinciples: ["Least Privilege", "Complete Mediation", "Fail Securely"],
|
||||
},
|
||||
});
|
||||
return false;
|
||||
}
|
||||
return assertFreshTaskWatchdogSourceMutation(res, watchdogScope, issue);
|
||||
}
|
||||
const boundaryDecision = await decideIssueAccess(req, issue, "issue:comment");
|
||||
if (!boundaryDecision.allowed) {
|
||||
res.status(403).json({ error: "Issue is outside this actor's authorization boundary" });
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
async function filterIssuesForActor<T extends Parameters<typeof decideIssueAccess>[1]>(req: Request, rows: T[]) {
|
||||
const decisions = await Promise.all(rows.map((issue) => decideIssueAccess(req, issue, "issue:read")));
|
||||
return rows.filter((_, index) => decisions[index]?.allowed);
|
||||
@@ -6703,6 +6745,7 @@ export function issueRoutes(
|
||||
return;
|
||||
}
|
||||
assertCompanyAccess(req, issue.companyId);
|
||||
if (!(await assertIssueReadAllowed(req, res, issue))) return;
|
||||
const afterCommentId =
|
||||
typeof req.query.after === "string" && req.query.after.trim().length > 0
|
||||
? req.query.after.trim()
|
||||
@@ -6737,6 +6780,7 @@ export function issueRoutes(
|
||||
return;
|
||||
}
|
||||
assertCompanyAccess(req, issue.companyId);
|
||||
if (!(await assertIssueReadAllowed(req, res, issue))) return;
|
||||
const actor = getActorInfo(req);
|
||||
const interactionSvc = issueThreadInteractionService(db);
|
||||
const expiredInteractions = await interactionSvc.expireRequestConfirmationsSupersededByHistoricalComments(issue);
|
||||
@@ -7314,7 +7358,7 @@ export function issueRoutes(
|
||||
return;
|
||||
}
|
||||
assertCompanyAccess(req, issue.companyId);
|
||||
if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return;
|
||||
if (!(await assertAgentIssueCommentAllowed(req, res, issue))) return;
|
||||
if (!assertStructuredCommentFieldsAllowed(req, res, {
|
||||
presentation: req.body.presentation,
|
||||
metadata: req.body.metadata,
|
||||
@@ -7329,12 +7373,20 @@ export function issueRoutes(
|
||||
const reopenRequested = req.body.reopen === true;
|
||||
const resumeRequested = req.body.resume === true;
|
||||
const interruptRequested = req.body.interrupt === true;
|
||||
const isClosed = isClosedIssueStatus(issue.status);
|
||||
const isBlocked = issue.status === "blocked";
|
||||
if (
|
||||
isClosed &&
|
||||
req.actor.type === "agent" &&
|
||||
issue.assigneeAgentId !== null &&
|
||||
issue.assigneeAgentId !== req.actor.agentId
|
||||
) {
|
||||
if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return;
|
||||
}
|
||||
if (resumeRequested === true && !(await assertExplicitResumeIntentAllowed(req, res, issue))) return;
|
||||
if (resumeRequested !== true && reopenRequested === true && req.actor.type === "agent") {
|
||||
if (!(await assertExplicitResumeIntentAllowed(req, res, issue))) return;
|
||||
}
|
||||
const isClosed = isClosedIssueStatus(issue.status);
|
||||
const isBlocked = issue.status === "blocked";
|
||||
const explicitMoveToTodoRequested = reopenRequested || resumeRequested === true;
|
||||
const scheduledRetryForHumanComment =
|
||||
shouldHumanCommentResumeInProgressScheduledRetry({
|
||||
|
||||
@@ -1,22 +1,24 @@
|
||||
import { and, eq, sql } from "drizzle-orm";
|
||||
import { and, eq, inArray, isNull, sql } from "drizzle-orm";
|
||||
import type { Db } from "@paperclipai/db";
|
||||
import {
|
||||
agents,
|
||||
companyMemberships,
|
||||
heartbeatRuns,
|
||||
instanceUserRoles,
|
||||
issueComments,
|
||||
issues,
|
||||
principalPermissionGrants,
|
||||
projects,
|
||||
} from "@paperclipai/db";
|
||||
import type { PermissionKey, PrincipalType } from "@paperclipai/shared";
|
||||
import { LOW_TRUST_REVIEW_PRESET, type LowTrustBoundary } from "@paperclipai/shared";
|
||||
import { LOW_TRUST_REVIEW_PRESET, extractAgentMentionIds, type LowTrustBoundary } from "@paperclipai/shared";
|
||||
import {
|
||||
LOW_TRUST_ISSUE_ANCESTRY_MAX_DEPTH,
|
||||
isIssueWithinLowTrustBoundary,
|
||||
resolveCoreTrustPreset,
|
||||
type TrustPresetResolution,
|
||||
} from "./trust-preset-resolver.js";
|
||||
import { logger } from "../middleware/logger.js";
|
||||
|
||||
export type AuthorizationActor =
|
||||
{
|
||||
@@ -45,6 +47,7 @@ export type AuthorizationAction =
|
||||
| "agent:read"
|
||||
| "agent:wake"
|
||||
| "company_scope:read"
|
||||
| "issue:comment"
|
||||
| "issue:mutate"
|
||||
| "issue:read"
|
||||
| "project:read"
|
||||
@@ -76,6 +79,7 @@ export type AuthorizationDecision = {
|
||||
| "allow_instance_admin"
|
||||
| "allow_explicit_grant"
|
||||
| "allow_legacy_agent_creator"
|
||||
| "allow_issue_mention_grant"
|
||||
| "allow_self"
|
||||
| "allow_company_agent"
|
||||
| "allow_company_member"
|
||||
@@ -118,7 +122,7 @@ function permissionForAction(action: AuthorizationAction): PermissionKey | null
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
if (action === "issue:mutate") return null;
|
||||
if (action === "issue:comment" || action === "issue:mutate") return null;
|
||||
return action;
|
||||
}
|
||||
|
||||
@@ -753,13 +757,27 @@ export function authorizationService(db: Db) {
|
||||
: lowTrustDeny("Project is outside this low-trust boundary.");
|
||||
}
|
||||
|
||||
if (input.action === "issue:read" || input.action === "issue:mutate") {
|
||||
if (input.action === "issue:comment" || input.action === "issue:read" || input.action === "issue:mutate") {
|
||||
if (input.resource.type !== "issue") {
|
||||
return lowTrustDeny("Low-trust issue access is missing an issue resource.");
|
||||
}
|
||||
return await issueResourceWithinLowTrustBoundary(boundary, input.resource)
|
||||
? lowTrustAllow("Allowed inside the low-trust issue boundary.")
|
||||
: lowTrustDeny("Issue is outside this low-trust boundary.");
|
||||
if (await issueResourceWithinLowTrustBoundary(boundary, input.resource)) {
|
||||
return lowTrustAllow("Allowed inside the low-trust issue boundary.");
|
||||
}
|
||||
if (
|
||||
input.action !== "issue:mutate" &&
|
||||
input.resource.issueId &&
|
||||
await agentHasMentionGrantOnIssue({
|
||||
action: input.action,
|
||||
companyId: boundary.companyId,
|
||||
issueId: input.resource.issueId,
|
||||
issueAssigneeAgentId: input.resource.assigneeAgentId ?? null,
|
||||
actorAgentId: input.actorAgentId,
|
||||
})
|
||||
) {
|
||||
return allowIssueMentionGrant(input.action);
|
||||
}
|
||||
return lowTrustDeny("Issue is outside this low-trust boundary.");
|
||||
}
|
||||
|
||||
if (input.action === "tasks:assign") {
|
||||
@@ -850,6 +868,93 @@ export function authorizationService(db: Db) {
|
||||
return isAgentInSubtree(db, companyId, managerAgentId, assigneeAgentId);
|
||||
}
|
||||
|
||||
function commentAuthorCanGrantIssueMention(input: {
|
||||
mentionedAgentId: string;
|
||||
issueAssigneeAgentId: string | null;
|
||||
authorAgentId: string | null;
|
||||
authorUserId: string | null;
|
||||
activeAuthorUserIds: Set<string>;
|
||||
}) {
|
||||
if (input.authorAgentId) {
|
||||
if (input.authorAgentId === input.mentionedAgentId) return false;
|
||||
return input.issueAssigneeAgentId === input.authorAgentId;
|
||||
}
|
||||
if (input.authorUserId) {
|
||||
return input.activeAuthorUserIds.has(input.authorUserId);
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
async function agentHasMentionGrantOnIssue(input: {
|
||||
action: AuthorizationAction;
|
||||
companyId: string;
|
||||
issueId: string;
|
||||
issueAssigneeAgentId: string | null;
|
||||
actorAgentId: string;
|
||||
}) {
|
||||
const rows = await db
|
||||
.select({
|
||||
id: issueComments.id,
|
||||
body: issueComments.body,
|
||||
authorAgentId: issueComments.authorAgentId,
|
||||
authorUserId: issueComments.authorUserId,
|
||||
})
|
||||
.from(issueComments)
|
||||
.where(and(
|
||||
eq(issueComments.companyId, input.companyId),
|
||||
eq(issueComments.issueId, input.issueId),
|
||||
isNull(issueComments.deletedAt),
|
||||
sql`${issueComments.body} LIKE ${"%agent://" + input.actorAgentId + "%"}`,
|
||||
));
|
||||
|
||||
const mentionRows = rows.filter((row) => extractAgentMentionIds(row.body).includes(input.actorAgentId));
|
||||
const authorUserIds = [...new Set(mentionRows.flatMap((row) => row.authorUserId ? [row.authorUserId] : []))];
|
||||
const activeAuthorUserIds = new Set(
|
||||
authorUserIds.length === 0
|
||||
? []
|
||||
: await db
|
||||
.select({ principalId: companyMemberships.principalId })
|
||||
.from(companyMemberships)
|
||||
.where(and(
|
||||
eq(companyMemberships.companyId, input.companyId),
|
||||
eq(companyMemberships.principalType, "user"),
|
||||
eq(companyMemberships.status, "active"),
|
||||
inArray(companyMemberships.principalId, authorUserIds),
|
||||
))
|
||||
.then((memberships) => memberships.map((membership) => membership.principalId)),
|
||||
);
|
||||
|
||||
for (const row of mentionRows) {
|
||||
const authorCanGrant = commentAuthorCanGrantIssueMention({
|
||||
mentionedAgentId: input.actorAgentId,
|
||||
issueAssigneeAgentId: input.issueAssigneeAgentId,
|
||||
authorAgentId: row.authorAgentId,
|
||||
authorUserId: row.authorUserId,
|
||||
activeAuthorUserIds,
|
||||
});
|
||||
if (authorCanGrant) {
|
||||
logger.info({
|
||||
actorAgentId: input.actorAgentId,
|
||||
issueId: input.issueId,
|
||||
companyId: input.companyId,
|
||||
commentId: row.id,
|
||||
grantedAction: input.action,
|
||||
grant: "issue_mention_comment",
|
||||
}, "authorized issue mention-scoped comment grant");
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
function allowIssueMentionGrant(action: AuthorizationAction): AuthorizationDecision {
|
||||
return allow({
|
||||
action,
|
||||
reason: "allow_issue_mention_grant",
|
||||
explanation: "Allowed by a mention-scoped issue comment grant.",
|
||||
});
|
||||
}
|
||||
|
||||
async function decide(input: {
|
||||
actor: AuthorizationActor;
|
||||
action: AuthorizationAction;
|
||||
@@ -956,7 +1061,10 @@ export function authorizationService(db: Db) {
|
||||
explanation: "Allowed by active cloud tenant company membership.",
|
||||
});
|
||||
}
|
||||
if (input.action === "issue:mutate" && membership.membershipRole !== "viewer") {
|
||||
if (
|
||||
(input.action === "issue:comment" || input.action === "issue:mutate") &&
|
||||
membership.membershipRole !== "viewer"
|
||||
) {
|
||||
return allow({
|
||||
action: input.action,
|
||||
reason: "allow_company_member",
|
||||
@@ -1092,6 +1200,7 @@ export function authorizationService(db: Db) {
|
||||
input.action === "agent:read" ||
|
||||
input.action === "agent:wake" ||
|
||||
input.action === "company_scope:read" ||
|
||||
input.action === "issue:comment" ||
|
||||
input.action === "issue:read" ||
|
||||
input.action === "project:read" ||
|
||||
input.action === "runtime:manage" ||
|
||||
@@ -1154,7 +1263,7 @@ export function authorizationService(db: Db) {
|
||||
});
|
||||
}
|
||||
|
||||
if (input.action === "issue:mutate") {
|
||||
if (input.action === "issue:comment" || input.action === "issue:mutate") {
|
||||
const resource = input.resource.type === "issue" ? input.resource : null;
|
||||
if (resource?.assigneeAgentId === actorAgentId) {
|
||||
return allow({
|
||||
@@ -1170,6 +1279,19 @@ export function authorizationService(db: Db) {
|
||||
explanation: "Allowed because the issue has no agent assignee.",
|
||||
});
|
||||
}
|
||||
if (
|
||||
input.action === "issue:comment" &&
|
||||
resource?.issueId &&
|
||||
await agentHasMentionGrantOnIssue({
|
||||
action: input.action,
|
||||
companyId,
|
||||
issueId: resource.issueId,
|
||||
issueAssigneeAgentId: resource.assigneeAgentId ?? null,
|
||||
actorAgentId,
|
||||
})
|
||||
) {
|
||||
return allowIssueMentionGrant(input.action);
|
||||
}
|
||||
}
|
||||
if (
|
||||
input.action === "agent_config:update" &&
|
||||
|
||||
Reference in New Issue
Block a user