mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-27 02:44:50 +01:00
fix(server): keep mention-grant closed comments comment-only
Treat a mention-granted non-assignee agent reply on a closed issue as comment-only only when the payload has no explicit resume or reopen intent. Resume/reopen still requires mutation authorization. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
2 files changed
+86
-6
No files matched your search
@@ -552,6 +552,72 @@ describe.sequential("issue comment reopen routes", () => {
|
||||
expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("allows mention-granted non-assignee agent POST comments on closed issues without reopening", async () => {
|
||||
const mentionedAgentId = "33333333-3333-4333-8333-333333333333";
|
||||
mockIssueService.getById.mockResolvedValue(makeIssue("done"));
|
||||
mockIssueService.addComment.mockResolvedValue({
|
||||
id: "comment-1",
|
||||
issueId: "11111111-1111-4111-8111-111111111111",
|
||||
companyId: "company-1",
|
||||
body: "I can answer the mention without reopening.",
|
||||
createdAt: new Date(),
|
||||
updatedAt: new Date(),
|
||||
authorAgentId: mentionedAgentId,
|
||||
authorUserId: null,
|
||||
});
|
||||
mockAccessService.decide.mockImplementation(async (input: { action?: string }) => {
|
||||
const allowed = input.action === "issue:comment";
|
||||
return {
|
||||
allowed,
|
||||
action: input.action,
|
||||
reason: allowed ? "allow_issue_mention_grant" : "deny_missing_grant",
|
||||
explanation: allowed ? "Allowed by a mention-scoped issue comment grant." : "Missing permission.",
|
||||
};
|
||||
});
|
||||
|
||||
const res = await request(await installActor(createApp(), agentActor(mentionedAgentId)))
|
||||
.post("/api/issues/11111111-1111-4111-8111-111111111111/comments")
|
||||
.send({ body: "I can answer the mention without reopening." });
|
||||
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(201);
|
||||
expect(mockIssueService.addComment).toHaveBeenCalled();
|
||||
expect(mockIssueService.update).not.toHaveBeenCalled();
|
||||
expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled();
|
||||
expect(mockAccessService.decide).not.toHaveBeenCalledWith(expect.objectContaining({ action: "issue:mutate" }));
|
||||
});
|
||||
|
||||
it.each([
|
||||
["resume", { resume: true }],
|
||||
["reopen", { reopen: true }],
|
||||
])(
|
||||
"rejects mention-granted non-assignee agent POST comments on closed issues with %s intent",
|
||||
async (_name, intent) => {
|
||||
const mentionedAgentId = "33333333-3333-4333-8333-333333333333";
|
||||
mockIssueService.getById.mockResolvedValue(makeIssue("done"));
|
||||
mockAccessService.decide.mockImplementation(async (input: { action?: string }) => {
|
||||
const allowed = input.action === "issue:comment";
|
||||
return {
|
||||
allowed,
|
||||
action: input.action,
|
||||
reason: allowed ? "allow_issue_mention_grant" : "deny_missing_grant",
|
||||
explanation: allowed ? "Allowed by a mention-scoped issue comment grant." : "Missing permission.",
|
||||
};
|
||||
});
|
||||
|
||||
const res = await request(await installActor(createApp(), agentActor(mentionedAgentId)))
|
||||
.post("/api/issues/11111111-1111-4111-8111-111111111111/comments")
|
||||
.send({ body: "Please continue this closed issue.", ...intent });
|
||||
|
||||
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:comment" }));
|
||||
expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({ action: "issue:mutate" }));
|
||||
expect(mockIssueService.update).not.toHaveBeenCalled();
|
||||
expect(mockIssueService.addComment).not.toHaveBeenCalled();
|
||||
expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled();
|
||||
},
|
||||
);
|
||||
|
||||
// POST self-comment from the assignee agent on a done issue with explicit
|
||||
// reopen=true is the same log-class signal — the guard must suppress reopen.
|
||||
it("does not reopen via POST comment+reopen when the assignee agent is the actor on a done issue", async () => {
|
||||
|
||||
@@ -1915,7 +1915,11 @@ export function issueRoutes(
|
||||
res.status(403).json({ error: "Issue is outside this actor's authorization boundary" });
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
return boundaryDecision;
|
||||
}
|
||||
|
||||
function isIssueMentionGrantDecision(decision: true | Awaited<ReturnType<typeof decideIssueAccess>>) {
|
||||
return decision !== true && decision.reason === "allow_issue_mention_grant";
|
||||
}
|
||||
|
||||
async function filterIssuesForActor<T extends Parameters<typeof decideIssueAccess>[1]>(req: Request, rows: T[]) {
|
||||
@@ -7359,7 +7363,8 @@ export function issueRoutes(
|
||||
return;
|
||||
}
|
||||
assertCompanyAccess(req, issue.companyId);
|
||||
if (!(await assertAgentIssueCommentAllowed(req, res, issue))) return;
|
||||
const commentAccessDecision = await assertAgentIssueCommentAllowed(req, res, issue);
|
||||
if (!commentAccessDecision) return;
|
||||
if (!assertStructuredCommentFieldsAllowed(req, res, {
|
||||
presentation: req.body.presentation,
|
||||
metadata: req.body.metadata,
|
||||
@@ -7376,19 +7381,28 @@ export function issueRoutes(
|
||||
const interruptRequested = req.body.interrupt === true;
|
||||
const isClosed = isClosedIssueStatus(issue.status);
|
||||
const isBlocked = issue.status === "blocked";
|
||||
const mentionGrantedPeerAgentCommentOnly =
|
||||
isClosed &&
|
||||
req.actor.type === "agent" &&
|
||||
issue.assigneeAgentId !== null &&
|
||||
issue.assigneeAgentId !== req.actor.agentId &&
|
||||
isIssueMentionGrantDecision(commentAccessDecision);
|
||||
const effectiveReopenRequested = mentionGrantedPeerAgentCommentOnly ? false : reopenRequested;
|
||||
const effectiveResumeRequested = mentionGrantedPeerAgentCommentOnly ? false : resumeRequested;
|
||||
if (
|
||||
isClosed &&
|
||||
req.actor.type === "agent" &&
|
||||
issue.assigneeAgentId !== null &&
|
||||
issue.assigneeAgentId !== req.actor.agentId
|
||||
issue.assigneeAgentId !== req.actor.agentId &&
|
||||
!mentionGrantedPeerAgentCommentOnly
|
||||
) {
|
||||
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 (effectiveResumeRequested === true && !(await assertExplicitResumeIntentAllowed(req, res, issue))) return;
|
||||
if (effectiveResumeRequested !== true && effectiveReopenRequested === true && req.actor.type === "agent") {
|
||||
if (!(await assertExplicitResumeIntentAllowed(req, res, issue))) return;
|
||||
}
|
||||
const explicitMoveToTodoRequested = reopenRequested || resumeRequested === true;
|
||||
const explicitMoveToTodoRequested = effectiveReopenRequested || effectiveResumeRequested === true;
|
||||
const scheduledRetryForHumanComment =
|
||||
shouldHumanCommentResumeInProgressScheduledRetry({
|
||||
hasComment: true,
|
||||
|
||||
Reference in new issue
Block a user