mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-24 10:13:11 +02:00
feat(auth): default-open visible issue writes (#10804)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agents coordinate through company-visible issues, comments, child tasks, and assignments. > - The current authorization rules give these write channels different and narrow ownership grants. > - Those differences prevent standard-trust agents from coordinating on work that they can already read. > - A responsible human user must still bound every agent action. > - This pull request gives the four issue-write channels one default-open rule based on issue visibility. > - The benefit is consistent multi-agent coordination without weakening company, user, trust-scope, or run-lifecycle controls. ## Linked Issues or Issue Description **What existing behavior does this improve?** This improves authorization for comments, issue updates, child creation, and assignment on company-visible issues. **Subsystem affected** `server/` REST API authorization and issue routes. **Current behavior** Standard-trust agents can read company-visible issues, but narrow ownership, parent, or mention grants can still deny related writes. Each write channel also applies a different rule. **Proposed behavior** Allow standard-trust agents to comment, update fields, create child issues, and assign work when they can read the target issue and the responsible user is also authorized. Keep company boundaries, low-trust scopes, checkout conflicts, status rules, pause gates, budget gates, and explicit reopen rules unchanged. **Reason and benefit** Agents can coordinate on visible company work without relay issues or unnecessary manager runs. One shared rule also makes the authorization model easier to test and maintain. **Breaking changes** This intentionally broadens write access for standard-trust agents on visible issues. Existing company boundaries and governance controls remain in force. Related prior approaches: Refs #10233 and Refs #9768. This change unifies the comment case with visible issue updates, child creation, and assignment while preserving the responsible-user ceiling and excluded trust scopes. ## What Changed - Added a shared default-open authorization decision for visible issue writes. - Applied the shared rule to comments, issue updates, child creation, and assignment. - Preserved low-trust, `skill_test`, `task_bridge`, responsible-user, checkout, lifecycle, pause, and budget controls. - Preserved explicit resume/restore authority for direct peer lifecycle transitions on blocked, completed, and cancelled issues. - Added regression coverage for cross-company denial, user intersection, excluded scopes, comment-read structure, closed issues, child creation, assignment, and peer updates. - Updated the V1 implementation contract for the shared rule. ## Verification - `pnpm exec vitest run server/src/__tests__/authorization-service.test.ts server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts server/src/__tests__/issue-comment-reopen-routes.test.ts server/src/__tests__/low-trust-red-team-routes.test.ts --reporter=dot` — 217 tests passed. - `pnpm --filter @paperclipai/server typecheck` — passed. - `git diff --check public-gh/master...HEAD` — passed. - Independent security review covered broken access control, object-level authorization, excessive agency, cross-company access, responsible-user intersection, excluded scopes, and lifecycle controls; its peer lifecycle-transition finding is fixed with regression coverage. ## Risks - Standard-trust agents gain broader write influence on issues that they can already read. - Future issue-visibility controls must keep `issue:read` as the canonical authorization hook. - Regression tests cover company boundaries, responsible-user intersection, excluded scopes, active checkout conflicts, closed-issue behavior, and non-transitive mention authority. > 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 GPT-5 through Codex. The runtime does not expose the exact deployment revision or context-window size. Agentic reasoning, repository tools, shell execution, and test execution were enabled. ## 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:
DottaandPaperclip authored and GitHub committed
1 parent
cb52f0b750
commit
dfcda67650
7 files changed
+434
-49
No files matched your search
@@ -501,6 +501,34 @@ describeEmbeddedPostgres("authorization service", () => {
|
||||
expect(decision.explanation).toContain("Agent key cannot access another company");
|
||||
});
|
||||
|
||||
it("denies cross-company default-open issue writes", async () => {
|
||||
const sourceCompany = await createCompany(db, "WriteSource");
|
||||
const targetCompany = await createCompany(db, "WriteTarget");
|
||||
const actorAgent = await createAgent(db, sourceCompany.id);
|
||||
const targetIssue = await createIssue(db, targetCompany.id);
|
||||
const authorization = authorizationService(db);
|
||||
|
||||
for (const action of ["issue:comment", "issue:mutate"] as const) {
|
||||
await expect(authorization.decide({
|
||||
actor: {
|
||||
type: "agent",
|
||||
agentId: actorAgent.id,
|
||||
companyId: sourceCompany.id,
|
||||
source: "agent_jwt",
|
||||
},
|
||||
action,
|
||||
resource: {
|
||||
type: "issue",
|
||||
companyId: targetCompany.id,
|
||||
issueId: targetIssue.id,
|
||||
},
|
||||
})).resolves.toMatchObject({
|
||||
allowed: false,
|
||||
reason: "deny_company_boundary",
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
it("allows simple-mode task assignment between same-company agents without explicit grants", async () => {
|
||||
const company = await createCompany(db, "AssignmentDefault");
|
||||
const actorAgent = await createAgent(db, company.id, { role: "engineer" });
|
||||
@@ -522,9 +550,153 @@ describeEmbeddedPostgres("authorization service", () => {
|
||||
|
||||
expect(decision).toMatchObject({
|
||||
allowed: true,
|
||||
reason: "allow_simple_company_member",
|
||||
reason: "allow_visible_issue_write",
|
||||
});
|
||||
expect(decision.explanation).toContain("shared default-open");
|
||||
});
|
||||
|
||||
it("allows standard-trust agents to comment on and update visible peer-owned issues", async () => {
|
||||
const company = await createCompany(db, "DefaultOpenPeerWrites");
|
||||
const actorAgent = await createAgent(db, company.id);
|
||||
const ownerAgent = await createAgent(db, company.id);
|
||||
const issue = await createIssue(db, company.id, { assigneeAgentId: ownerAgent.id });
|
||||
const actor = {
|
||||
type: "agent" as const,
|
||||
agentId: actorAgent.id,
|
||||
companyId: company.id,
|
||||
source: "agent_jwt" as const,
|
||||
};
|
||||
const resource = {
|
||||
type: "issue" as const,
|
||||
companyId: company.id,
|
||||
issueId: issue.id,
|
||||
assigneeAgentId: ownerAgent.id,
|
||||
status: issue.status,
|
||||
};
|
||||
const authorization = authorizationService(db);
|
||||
|
||||
for (const action of ["issue:comment", "issue:mutate"] as const) {
|
||||
await expect(authorization.decide({ actor, action, resource })).resolves.toMatchObject({
|
||||
allowed: true,
|
||||
reason: "allow_visible_issue_write",
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps the responsible-user ceiling on every default-open peer write", async () => {
|
||||
const company = await createCompany(db, "DefaultOpenPeerWriteCeiling");
|
||||
const actorAgent = await createAgent(db, company.id);
|
||||
const ownerAgent = await createAgent(db, company.id);
|
||||
const issue = await createIssue(db, company.id, { assigneeAgentId: ownerAgent.id });
|
||||
const unavailableUserId = await createUser(db);
|
||||
const actor = {
|
||||
type: "agent" as const,
|
||||
agentId: actorAgent.id,
|
||||
companyId: company.id,
|
||||
onBehalfOfUserId: unavailableUserId,
|
||||
source: "agent_jwt" as const,
|
||||
};
|
||||
const resource = {
|
||||
type: "issue" as const,
|
||||
companyId: company.id,
|
||||
issueId: issue.id,
|
||||
assigneeAgentId: ownerAgent.id,
|
||||
status: issue.status,
|
||||
};
|
||||
const authorization = authorizationService(db);
|
||||
|
||||
for (const action of ["issue:comment", "issue:mutate"] as const) {
|
||||
await expect(authorization.decide({ actor, action, resource })).resolves.toMatchObject({
|
||||
allowed: false,
|
||||
code: "RESPONSIBLE_USER_UNAVAILABLE",
|
||||
reason: "deny_missing_membership",
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
it("structurally keeps default-open comments inside issue visibility", async () => {
|
||||
const company = await createCompany(db, "CommentReadSubset");
|
||||
const project = await createProject(db, company.id, "Visible");
|
||||
const outsideProject = await createProject(db, company.id, "Outside");
|
||||
const rootIssueId = randomUUID();
|
||||
const actorAgent = await createAgent(db, company.id, {
|
||||
permissions: {
|
||||
trustPreset: LOW_TRUST_REVIEW_PRESET,
|
||||
authorizationPolicy: {
|
||||
trustBoundary: {
|
||||
mode: LOW_TRUST_REVIEW_PRESET,
|
||||
projectIds: [project.id],
|
||||
rootIssueId,
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
const ownerAgent = await createAgent(db, company.id);
|
||||
const visibleIssue = await createIssue(db, company.id, {
|
||||
id: rootIssueId,
|
||||
projectId: project.id,
|
||||
assigneeAgentId: ownerAgent.id,
|
||||
});
|
||||
const hiddenIssue = await createIssue(db, company.id, {
|
||||
projectId: outsideProject.id,
|
||||
assigneeAgentId: ownerAgent.id,
|
||||
});
|
||||
const actor = {
|
||||
type: "agent" as const,
|
||||
agentId: actorAgent.id,
|
||||
companyId: company.id,
|
||||
source: "agent_key" as const,
|
||||
};
|
||||
const authorization = authorizationService(db);
|
||||
|
||||
for (const issue of [visibleIssue, hiddenIssue]) {
|
||||
const resource = {
|
||||
type: "issue" as const,
|
||||
companyId: company.id,
|
||||
issueId: issue.id,
|
||||
projectId: issue.projectId,
|
||||
parentIssueId: issue.parentId,
|
||||
assigneeAgentId: issue.assigneeAgentId,
|
||||
status: issue.status,
|
||||
};
|
||||
const read = await authorization.decide({ actor, action: "issue:read", resource });
|
||||
const comment = await authorization.decide({ actor, action: "issue:comment", resource });
|
||||
expect(comment.allowed && !read.allowed).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
it("does not let default-open non-assignee comments mint mention grants", async () => {
|
||||
const company = await createCompany(db, "DefaultOpenMentionNonTransitive");
|
||||
const ownerAgent = await createAgent(db, company.id);
|
||||
const commentingAgent = await createAgent(db, company.id);
|
||||
const mentionedAgent = await createAgent(db, company.id);
|
||||
const issue = await createIssue(db, company.id, { assigneeAgentId: ownerAgent.id });
|
||||
await db.insert(issueComments).values({
|
||||
companyId: company.id,
|
||||
issueId: issue.id,
|
||||
authorAgentId: commentingAgent.id,
|
||||
body: `[@Mentioned](agent://${mentionedAgent.id}) please take a look`,
|
||||
});
|
||||
|
||||
await expect(authorizationService(db).decide({
|
||||
actor: {
|
||||
type: "agent",
|
||||
agentId: mentionedAgent.id,
|
||||
companyId: company.id,
|
||||
source: "agent_jwt",
|
||||
},
|
||||
action: "issue:comment",
|
||||
resource: {
|
||||
type: "issue",
|
||||
companyId: company.id,
|
||||
issueId: issue.id,
|
||||
assigneeAgentId: ownerAgent.id,
|
||||
status: issue.status,
|
||||
},
|
||||
})).resolves.toMatchObject({
|
||||
allowed: true,
|
||||
reason: "allow_visible_issue_write",
|
||||
});
|
||||
expect(decision.explanation).toContain("simple mode");
|
||||
});
|
||||
|
||||
it("denies delegated protected assignment when the responsible user lacks matching authority", async () => {
|
||||
@@ -822,6 +994,17 @@ describeEmbeddedPostgres("authorization service", () => {
|
||||
action: "company_scope:read",
|
||||
resource: { type: "company", companyId: company.id },
|
||||
})).resolves.toMatchObject({ allowed: false, reason: "deny_low_trust_boundary" });
|
||||
await expect(authorization.decide({
|
||||
actor,
|
||||
action: "tasks:assign",
|
||||
resource: {
|
||||
type: "issue",
|
||||
companyId: company.id,
|
||||
projectId: project.id,
|
||||
assigneeAgentId: collaborator.id,
|
||||
},
|
||||
scope: { projectId: project.id, assigneeAgentId: collaborator.id },
|
||||
})).resolves.toMatchObject({ allowed: true, reason: "allow_simple_company_member" });
|
||||
await expect(authorization.decide({
|
||||
actor,
|
||||
action: "tasks:assign",
|
||||
|
||||
@@ -803,21 +803,43 @@ describe("agent issue mutation checkout ownership", () => {
|
||||
expect(mockIssueService.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects non-mentioned peer agents from posting comments", async () => {
|
||||
it("allows non-mentioned peer agents to post comments on visible issues", async () => {
|
||||
mockAccessService.decide.mockImplementation(async (input: { action: string }) => ({
|
||||
allowed: input.action === "issue:read",
|
||||
allowed: input.action === "issue:read" || input.action === "issue:comment",
|
||||
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.",
|
||||
reason: input.action === "issue:comment" ? "allow_visible_issue_write" : "allow_explicit_grant",
|
||||
explanation: "Allowed by the shared visible-issue write rule.",
|
||||
}));
|
||||
|
||||
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();
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(201);
|
||||
expect(mockIssueService.addComment).toHaveBeenCalledWith(
|
||||
issueId,
|
||||
"I was not mentioned.",
|
||||
expect.any(Object),
|
||||
expect.any(Object),
|
||||
);
|
||||
});
|
||||
|
||||
it("keeps default-open peer comments on closed issues inert", async () => {
|
||||
mockIssueService.getById.mockResolvedValue(makeIssue({ status: "done", assigneeAgentId: ownerAgentId }));
|
||||
mockAccessService.decide.mockImplementation(async (input: { action: string }) => ({
|
||||
allowed: input.action === "issue:comment" || input.action === "issue:read",
|
||||
action: input.action,
|
||||
reason: input.action === "issue:comment" ? "allow_visible_issue_write" : "allow_company_agent",
|
||||
explanation: "Allowed by the shared visible-issue write rule.",
|
||||
}));
|
||||
|
||||
const res = await request(await createApp(peerActor()))
|
||||
.post(`/api/issues/${issueId}/comments`)
|
||||
.send({ body: "Closed issue context only." });
|
||||
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(201);
|
||||
expect(mockIssueService.addComment).toHaveBeenCalled();
|
||||
expect(mockIssueService.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects peer agents from listing comments when issue read is outside their boundary", async () => {
|
||||
@@ -899,7 +921,7 @@ describe("agent issue mutation checkout ownership", () => {
|
||||
expect(mockIssueService.getComment).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("keeps true issue mutations denied for mentioned peer agents", async () => {
|
||||
it("allows visible issue field updates for 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",
|
||||
@@ -922,9 +944,11 @@ describe("agent issue mutation checkout ownership", () => {
|
||||
.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();
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(200);
|
||||
expect(mockIssueService.update).toHaveBeenCalledWith(
|
||||
issueId,
|
||||
expect.objectContaining({ status: "done" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("denies cross-company agents before comment authorization is evaluated", async () => {
|
||||
@@ -1207,6 +1231,24 @@ describe("agent issue mutation checkout ownership", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("authorizes child creation through the shared visible-issue write path", async () => {
|
||||
mockIssueService.getById.mockResolvedValue(makeIssue({ status: "todo", assigneeAgentId: ownerAgentId }));
|
||||
|
||||
const res = await request(await createApp(peerActor()))
|
||||
.post(`/api/issues/${issueId}/children`)
|
||||
.send({ title: "Peer-created child" });
|
||||
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(201);
|
||||
expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({
|
||||
action: "issue:mutate",
|
||||
resource: expect.objectContaining({ issueId }),
|
||||
}));
|
||||
expect(mockIssueService.createChild).toHaveBeenCalledWith(
|
||||
issueId,
|
||||
expect.objectContaining({ title: "Peer-created child" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("preserves explicit workspace choices on agent-created root issues", async () => {
|
||||
const app = await createApp(
|
||||
ownerActor(),
|
||||
@@ -1492,18 +1534,36 @@ describe("agent issue mutation checkout ownership", () => {
|
||||
it.each([
|
||||
["todo", "patch", (app: express.Express) => request(app).patch(`/api/issues/${issueId}`).send({ title: "Todo update" })],
|
||||
["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) => {
|
||||
])("allows peer agent %s issue %s updates outside active checkout ownership", async (status, _kind, sendRequest) => {
|
||||
mockIssueService.getById.mockResolvedValue(makeIssue({ status: status as "todo" | "blocked", assigneeAgentId: ownerAgentId }));
|
||||
|
||||
const res = await sendRequest(await createApp(peerActor()));
|
||||
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(403);
|
||||
expect(res.body.error).toBe("Agent cannot mutate another agent's issue");
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(200);
|
||||
expect(mockIssueService.assertCheckoutOwner).not.toHaveBeenCalled();
|
||||
expect(mockIssueService.update).not.toHaveBeenCalled();
|
||||
expect(mockIssueService.update).toHaveBeenCalled();
|
||||
expect(mockIssueService.addComment).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each([
|
||||
["done", "todo", 403, "Agent cannot request follow-up for another agent's issue"],
|
||||
["cancelled", "todo", 409, "Cancelled issues must be restored through the dedicated restore flow"],
|
||||
["blocked", "done", 403, "Agent cannot request follow-up for another agent's issue"],
|
||||
])(
|
||||
"rejects peer agent direct status transitions from %s to %s",
|
||||
async (status, nextStatus, expectedStatus, expectedError) => {
|
||||
mockIssueService.getById.mockResolvedValue(makeIssue({ status, assigneeAgentId: ownerAgentId }));
|
||||
|
||||
const res = await request(await createApp(peerActor()))
|
||||
.patch(`/api/issues/${issueId}`)
|
||||
.send({ status: nextStatus });
|
||||
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(expectedStatus);
|
||||
expect(res.body.error).toBe(expectedError);
|
||||
expect(mockIssueService.update).not.toHaveBeenCalled();
|
||||
},
|
||||
);
|
||||
|
||||
it("allows same-company agent mutations on unassigned in-progress issues", async () => {
|
||||
mockIssueService.getById.mockResolvedValue(makeIssue({ assigneeAgentId: null }));
|
||||
mockIssueService.update.mockImplementation(async (_id: string, patch: Record<string, unknown>) => ({
|
||||
|
||||
@@ -537,7 +537,7 @@ describe.sequential("issue comment reopen routes", () => {
|
||||
));
|
||||
});
|
||||
|
||||
it("rejects non-assignee agent POST comments on closed issues", async () => {
|
||||
it("allows default-open non-assignee POST comments on closed issues without reopening", async () => {
|
||||
mockIssueService.getById.mockResolvedValue(makeIssue("done"));
|
||||
mockIssueService.addComment.mockResolvedValue({
|
||||
id: "comment-1",
|
||||
@@ -549,6 +549,12 @@ describe.sequential("issue comment reopen routes", () => {
|
||||
authorAgentId: "33333333-3333-4333-8333-333333333333",
|
||||
authorUserId: null,
|
||||
});
|
||||
mockAccessService.decide.mockImplementation(async (input: { action?: string }) => ({
|
||||
allowed: input.action !== "tasks:manage_active_checkouts",
|
||||
action: input.action,
|
||||
reason: input.action === "issue:comment" ? "allow_visible_issue_write" : "allow_explicit_grant",
|
||||
explanation: "Allowed by the shared visible-issue write rule.",
|
||||
}));
|
||||
|
||||
const res = await request(await installActor(createApp(), {
|
||||
type: "agent",
|
||||
@@ -560,10 +566,9 @@ describe.sequential("issue comment reopen routes", () => {
|
||||
.post("/api/issues/11111111-1111-4111-8111-111111111111/comments")
|
||||
.send({ body: "hello" });
|
||||
|
||||
expect(res.status).toBe(403);
|
||||
expect(res.body.error).toBe("Agent cannot mutate another agent's issue");
|
||||
expect(res.status).toBe(201);
|
||||
expect(mockIssueService.update).not.toHaveBeenCalled();
|
||||
expect(mockIssueService.addComment).not.toHaveBeenCalled();
|
||||
expect(mockIssueService.addComment).toHaveBeenCalled();
|
||||
expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
@@ -1419,7 +1424,7 @@ describe.sequential("issue comment reopen routes", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects non-assignee agent PATCH comments on closed issues", async () => {
|
||||
it("allows default-open non-assignee PATCH comments on closed issues without reopening", async () => {
|
||||
mockIssueService.getById.mockResolvedValue(makeIssue("done"));
|
||||
mockIssueService.addComment.mockResolvedValue({
|
||||
id: "comment-1",
|
||||
@@ -1446,10 +1451,9 @@ describe.sequential("issue comment reopen routes", () => {
|
||||
.patch("/api/issues/11111111-1111-4111-8111-111111111111")
|
||||
.send({ comment: "hello" });
|
||||
|
||||
expect(res.status).toBe(403);
|
||||
expect(res.body.error).toBe("Agent cannot mutate another agent's issue");
|
||||
expect(mockIssueService.update).not.toHaveBeenCalled();
|
||||
expect(mockIssueService.addComment).not.toHaveBeenCalled();
|
||||
expect(res.status).toBe(200);
|
||||
expect(mockIssueService.update).toHaveBeenCalled();
|
||||
expect(mockIssueService.addComment).toHaveBeenCalled();
|
||||
expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
|
||||
@@ -786,7 +786,7 @@ describeEmbeddedPostgres("low-trust red-team HTTP route regression suite", () =>
|
||||
});
|
||||
});
|
||||
|
||||
it("allows only standard checked-out runs to comment one hop upward", async () => {
|
||||
it("preserves direct-parent reporting while default-opening visible standard-trust writes", async () => {
|
||||
const fixture = await seedLowTrustFixture(db);
|
||||
const standardApp = createApp(db, standardReportActor(fixture));
|
||||
const lowTrustApp = createApp(db, agentActor(fixture));
|
||||
@@ -810,25 +810,31 @@ describeEmbeddedPostgres("low-trust red-team HTTP route regression suite", () =>
|
||||
.send({ body: "Contained report must not cross" });
|
||||
expect(lowTrustParentComment.status, JSON.stringify(lowTrustParentComment.body)).toBe(403);
|
||||
|
||||
const forbiddenStandardWrites = [
|
||||
const defaultOpenComments = [
|
||||
request(standardApp)
|
||||
.post(`/api/issues/${fixture.issues.reviewGrandparent.id}/comments`)
|
||||
.send({ body: "No grandparent report" }),
|
||||
.send({ body: "Visible grandparent context" }),
|
||||
request(standardApp)
|
||||
.post(`/api/issues/${fixture.issues.sameBoundaryChild.id}/comments`)
|
||||
.send({ body: "No sibling report" }),
|
||||
request(standardApp)
|
||||
.patch(`/api/issues/${fixture.issues.reviewRoot.id}`)
|
||||
.send({ status: "blocked" }),
|
||||
request(standardApp)
|
||||
.put(`/api/issues/${fixture.issues.reviewRoot.id}/documents/upward-write`)
|
||||
.send({ format: "markdown", body: "No upward document write" }),
|
||||
.send({ body: "Visible sibling context" }),
|
||||
];
|
||||
for (const forbiddenWrite of forbiddenStandardWrites) {
|
||||
const response = await forbiddenWrite;
|
||||
expect(response.status, JSON.stringify(response.body)).toBe(403);
|
||||
for (const defaultOpenComment of defaultOpenComments) {
|
||||
const response = await defaultOpenComment;
|
||||
expect(response.status, JSON.stringify(response.body)).toBe(201);
|
||||
}
|
||||
|
||||
const checkedOutPeerUpdate = await request(standardApp)
|
||||
.patch(`/api/issues/${fixture.issues.reviewRoot.id}`)
|
||||
.send({ status: "blocked" });
|
||||
expect(checkedOutPeerUpdate.status, JSON.stringify(checkedOutPeerUpdate.body)).toBe(409);
|
||||
expect(checkedOutPeerUpdate.body.error).toBe("Issue is checked out by another agent");
|
||||
|
||||
const documentWrite = await request(standardApp)
|
||||
.put(`/api/issues/${fixture.issues.reviewRoot.id}/documents/upward-write`)
|
||||
.send({ format: "markdown", body: "No upward document write" });
|
||||
expect(documentWrite.status, JSON.stringify(documentWrite.body)).toBe(409);
|
||||
expect(documentWrite.body.error).toBe("Issue is checked out by another agent");
|
||||
|
||||
for (const closedParent of [
|
||||
{ assigneeAgentId: null, intent: { reopen: true } },
|
||||
{ assigneeAgentId: fixture.agents.standard.id, intent: { resume: true } },
|
||||
|
||||
@@ -3469,6 +3469,22 @@ export function issueRoutes(
|
||||
return false;
|
||||
}
|
||||
|
||||
async function assertIssueWriteInfluenceAllowed(
|
||||
req: Request,
|
||||
res: Response,
|
||||
issue: Parameters<typeof decideIssueAccess>[1],
|
||||
) {
|
||||
if (req.actor.type !== "agent") return true;
|
||||
// Watchdog child creation keeps its dedicated subtree/revalidation grant;
|
||||
// assertTaskWatchdogCreateIssueAllowed performs that check immediately
|
||||
// after this generic collaboration gate at both create call sites.
|
||||
if ((await resolveTaskWatchdogMutationScope(db, req.actor)).kind !== "none") return true;
|
||||
const decision = await decideIssueAccess(req, issue, "issue:mutate");
|
||||
if (decision.allowed) return true;
|
||||
res.status(403).json({ error: "Issue is outside this actor's authorization boundary" });
|
||||
return false;
|
||||
}
|
||||
|
||||
async function assertAgentIssueCommentAllowed(
|
||||
req: Request,
|
||||
res: Response,
|
||||
@@ -3519,6 +3535,10 @@ export function issueRoutes(
|
||||
return decision !== true && decision.reason === "allow_direct_parent_report";
|
||||
}
|
||||
|
||||
function isDefaultOpenIssueWriteDecision(decision: true | Awaited<ReturnType<typeof decideIssueAccess>>) {
|
||||
return decision !== true && decision.reason === "allow_visible_issue_write";
|
||||
}
|
||||
|
||||
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);
|
||||
@@ -3566,6 +3586,7 @@ export function issueRoutes(
|
||||
assigneeAgentId: string | null;
|
||||
assigneeUserId: string | null;
|
||||
},
|
||||
options: { allowVisibleIssueWrite?: boolean } = {},
|
||||
) {
|
||||
if (req.actor.type !== "agent") return true;
|
||||
const actorAgentId = req.actor.agentId;
|
||||
@@ -3617,7 +3638,7 @@ export function issueRoutes(
|
||||
actorAgentId,
|
||||
},
|
||||
});
|
||||
} else {
|
||||
} else if (!options.allowVisibleIssueWrite) {
|
||||
res.status(403).json({
|
||||
error: "Agent cannot mutate another agent's issue",
|
||||
details: {
|
||||
@@ -3629,7 +3650,7 @@ export function issueRoutes(
|
||||
},
|
||||
});
|
||||
}
|
||||
return false;
|
||||
return issue.status !== "in_progress" && options.allowVisibleIssueWrite === true;
|
||||
}
|
||||
if (issue.status !== "in_progress") {
|
||||
return true;
|
||||
@@ -7168,7 +7189,7 @@ export function issueRoutes(
|
||||
res.status(404).json({ error: "Parent issue not found" });
|
||||
return;
|
||||
}
|
||||
if (!isTaskBridgeKeyActor(req) && !(await assertIssueReadAllowed(req, res, createParent))) return;
|
||||
if (!isTaskBridgeKeyActor(req) && !(await assertIssueWriteInfluenceAllowed(req, res, createParent))) return;
|
||||
}
|
||||
if (
|
||||
!watchdogProductBugFollowUp &&
|
||||
@@ -7386,7 +7407,7 @@ export function issueRoutes(
|
||||
const parentId = req.params.id as string;
|
||||
const parent = await getAccessibleResource(req, res, svc.getById(parentId), "Parent issue not found");
|
||||
if (!parent) return;
|
||||
if (!isTaskBridgeKeyActor(req) && !(await assertIssueReadAllowed(req, res, parent))) return;
|
||||
if (!isTaskBridgeKeyActor(req) && !(await assertIssueWriteInfluenceAllowed(req, res, parent))) return;
|
||||
if (!(await assertTaskWatchdogCreateIssueAllowed(req, res, parent.companyId, parent))) return;
|
||||
if (await assertLowTrustControlPlaneDenied(req, res, parent.companyId, parent)) return;
|
||||
assertNoAgentHostWorkspaceCommandMutation(req, collectIssueWorkspaceCommandPaths(req.body));
|
||||
@@ -7821,7 +7842,7 @@ export function issueRoutes(
|
||||
const existing = await getAccessibleResource(req, res, svc.getById(id), "Issue not found");
|
||||
if (!existing) return;
|
||||
assertNoAgentHostWorkspaceCommandMutation(req, collectIssueWorkspaceCommandPaths(req.body));
|
||||
if (!(await assertAgentIssueMutationAllowed(req, res, existing))) return;
|
||||
if (!(await assertAgentIssueMutationAllowed(req, res, existing, { allowVisibleIssueWrite: true }))) return;
|
||||
if (!(await assertCheapRecoveryIssueAssigneeProfileAllowed(req, res, existing, req.body))) return;
|
||||
|
||||
const actor = getActorInfo(req);
|
||||
@@ -7861,7 +7882,12 @@ export function issueRoutes(
|
||||
return;
|
||||
}
|
||||
if (resumeRequested === true && !(await assertExplicitResumeIntentAllowed(req, res, existing))) return;
|
||||
if (resumeRequested !== true && reopenRequested === true && req.actor.type === "agent") {
|
||||
const agentStatusTransitionRequiresResumeAuthority =
|
||||
req.actor.type === "agent" &&
|
||||
typeof updateFields.status === "string" &&
|
||||
updateFields.status !== existing.status &&
|
||||
(isBlocked || (isClosed && !isClosedIssueStatus(updateFields.status)));
|
||||
if (resumeRequested !== true && req.actor.type === "agent" && reopenRequested === true) {
|
||||
if (!(await assertExplicitResumeIntentAllowed(req, res, existing))) return;
|
||||
}
|
||||
await assertIssueEnvironmentSelection(existing.companyId, updateFields.executionWorkspaceSettings?.environmentId);
|
||||
@@ -7890,6 +7916,13 @@ export function issueRoutes(
|
||||
) {
|
||||
return;
|
||||
}
|
||||
if (
|
||||
resumeRequested !== true &&
|
||||
agentStatusTransitionRequiresResumeAuthority &&
|
||||
!(await assertExplicitResumeIntentAllowed(req, res, existing))
|
||||
) {
|
||||
return;
|
||||
}
|
||||
const scheduledRetryForHumanComment =
|
||||
shouldHumanCommentResumeInProgressScheduledRetry({
|
||||
hasComment: !!commentBody,
|
||||
@@ -10133,7 +10166,8 @@ export function issueRoutes(
|
||||
issue.assigneeAgentId !== req.actor.agentId &&
|
||||
!reopenRequested &&
|
||||
!resumeRequested &&
|
||||
isIssueMentionGrantDecision(commentAccessDecision)));
|
||||
(isIssueMentionGrantDecision(commentAccessDecision) ||
|
||||
isDefaultOpenIssueWriteDecision(commentAccessDecision))));
|
||||
const effectiveReopenRequested = crossIssueCommentOnlyGrant ? false : reopenRequested;
|
||||
const effectiveResumeRequested = crossIssueCommentOnlyGrant ? false : resumeRequested;
|
||||
if (
|
||||
|
||||
@@ -106,6 +106,7 @@ export type AuthorizationDecision = {
|
||||
| "allow_legacy_agent_creator"
|
||||
| "allow_issue_mention_grant"
|
||||
| "allow_direct_parent_report"
|
||||
| "allow_visible_issue_write"
|
||||
| "allow_self"
|
||||
| "allow_company_agent"
|
||||
| "allow_company_member"
|
||||
@@ -1405,6 +1406,76 @@ export function authorizationService(db: Db) {
|
||||
const permissionKey = permissionForAction(input.action);
|
||||
const companyId = companyIdForResource(input.resource);
|
||||
|
||||
/**
|
||||
* Shared default-open decision for issue write-influence channels.
|
||||
*
|
||||
* Keep visibility structurally upstream of every standard-trust write so
|
||||
* future visibility scoping can be implemented in issue:read without
|
||||
* recreating per-action scope checks. The responsible-user ceiling remains
|
||||
* in decide(), after this base decision.
|
||||
*/
|
||||
async function decideVisibleIssueWrite(): Promise<AuthorizationDecision> {
|
||||
let visibilityResource: AuthorizationResource = input.resource;
|
||||
|
||||
// New-child assignment decisions identify the issue being influenced by
|
||||
// parentIssueId. Resolve that parent into the same resource shape used by
|
||||
// issue:read so child-create and assign share the visibility hook.
|
||||
if (
|
||||
input.resource.type === "issue" &&
|
||||
!input.resource.issueId &&
|
||||
input.resource.parentIssueId
|
||||
) {
|
||||
const parent = await loadIssue(input.resource.parentIssueId);
|
||||
if (!parent || parent.companyId !== companyId) {
|
||||
return deny({
|
||||
action: input.action,
|
||||
reason: "deny_company_boundary",
|
||||
explanation: "The issue write target is not visible in this company.",
|
||||
});
|
||||
}
|
||||
visibilityResource = {
|
||||
type: "issue",
|
||||
companyId: parent.companyId,
|
||||
issueId: parent.id,
|
||||
projectId: parent.projectId,
|
||||
parentIssueId: parent.parentId,
|
||||
assigneeAgentId: parent.assigneeAgentId,
|
||||
assigneeUserId: parent.assigneeUserId,
|
||||
originKind: parent.originKind,
|
||||
originId: parent.originId,
|
||||
status: parent.status,
|
||||
};
|
||||
}
|
||||
|
||||
const visibilityDecision = visibilityResource.type === "issue" && visibilityResource.issueId
|
||||
? await decideBase({
|
||||
actor: input.actor,
|
||||
action: "issue:read",
|
||||
resource: visibilityResource,
|
||||
scope: input.scope,
|
||||
})
|
||||
: await decideBase({
|
||||
actor: input.actor,
|
||||
action: "company_scope:read",
|
||||
resource: { type: "company", companyId },
|
||||
scope: input.scope,
|
||||
});
|
||||
|
||||
if (!visibilityDecision.allowed) {
|
||||
return {
|
||||
...visibilityDecision,
|
||||
action: input.action,
|
||||
explanation: `Issue write denied because the target is not visible: ${visibilityDecision.explanation}`,
|
||||
};
|
||||
}
|
||||
|
||||
return allow({
|
||||
action: input.action,
|
||||
reason: "allow_visible_issue_write",
|
||||
explanation: "Allowed by the shared default-open visible-issue write rule.",
|
||||
});
|
||||
}
|
||||
|
||||
async function decideWithTaskAssignmentGrants(
|
||||
principalType: PrincipalType,
|
||||
principalId: string,
|
||||
@@ -1809,6 +1880,15 @@ export function authorizationService(db: Db) {
|
||||
}
|
||||
}
|
||||
|
||||
const visibleIssueWriteDecision =
|
||||
trustResolution.kind === "standard" &&
|
||||
(input.action === "issue:comment" || input.action === "issue:mutate")
|
||||
? await decideVisibleIssueWrite()
|
||||
: null;
|
||||
if (visibleIssueWriteDecision && !visibleIssueWriteDecision.allowed) {
|
||||
return visibleIssueWriteDecision;
|
||||
}
|
||||
|
||||
if (
|
||||
trustResolution.kind === "standard" &&
|
||||
input.action === "issue:comment" &&
|
||||
@@ -1994,10 +2074,11 @@ export function authorizationService(db: Db) {
|
||||
if (grantDecision.allowed) return grantDecision;
|
||||
return denyRestrictedAssignmentPolicy(policyEffect);
|
||||
}
|
||||
if (trustResolution.kind === "standard") return decideVisibleIssueWrite();
|
||||
return allow({
|
||||
action: input.action,
|
||||
reason: "allow_simple_company_member",
|
||||
explanation: "Allowed by simple mode company-wide task assignment default.",
|
||||
explanation: "Allowed by the existing bounded task assignment rule.",
|
||||
});
|
||||
}
|
||||
|
||||
@@ -2030,6 +2111,7 @@ export function authorizationService(db: Db) {
|
||||
) {
|
||||
return allowIssueMentionGrant(input.action);
|
||||
}
|
||||
if (visibleIssueWriteDecision) return visibleIssueWriteDecision;
|
||||
}
|
||||
if (
|
||||
input.action === "agent_config:update" &&
|
||||
|
||||
Reference in new issue
Block a user