From 8e5959c759bb0f8a0b4c4489904cfa0525fa92d3 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Sun, 8 Nov 2026 22:01:11 -0600 Subject: [PATCH] fix(agents): keep pause board-only and resume grant-gated on the PATCH status route (#20164) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The board governs the agent lifecycle: pause, resume and terminate > - The V1 permission matrix says agents may never pause an agent, and may resume one only with a direct `agents:configure` change grant > - `POST /api/agents/:id/pause` and `POST /api/agents/:id/resume` enforce this, but `PATCH /api/agents/:id` with a `status` ran the same lifecycle change after only the agent-configuration check > - The `agents.pause-agent` chat eval caught it: the CEO agent paused Morgan itself through `PATCH`, although the case (and the spec) expect a board user to do it > - This pull request applies the dedicated routes' checks to `PATCH` status changes, as #19955 did for termination > - The benefit is one lifecycle authority, with no second route around it ## Linked Issues or Issue Description Refs #19955 **What happened** An agent that may configure another agent could pause it with `PATCH /api/agents/{id}` `{ "status": "paused" }`, although `POST /api/agents/{id}/pause` is board-only. The same `PATCH` with `{ "status": "idle" }` resumed an agent without the change-grant decision and without the invalid-org-chain guard that `POST /resume` applies. In a chat eval run, the CEO agent paused Morgan this way and reported "Morgan is paused". **Expected behavior** Pause is board-only on every route (`doc/SPEC-implementation.md` section 9.3: "Pause/resume agent ... pause: no; resume: direct `agents:configure` grant only"; "Pause, clear-error, terminate, approval, and key-management routes remain board-only"). An agent gets the standard final "Board access required" denial with a link to the agent's profile. Resume through `PATCH` needs the same grant and org-chain checks as `POST /resume`. **Steps to reproduce** 1. As an agent with agent-configuration permission over another agent, call `POST /api/agents/{id}/pause`. The result is 403. 2. Call `PATCH /api/agents/{id}` with `{ "status": "paused" }`. Before this change the result is 200 and the agent is paused. ## What Changed - `server/src/routes/agents.ts` (`PATCH /agents/:id`): `status: "paused"` now calls `assertBoard`, the same check as `POST /pause`. Agents get the structured `board_access_required` denial that says the denial is final and links `//agents/`. - `status: "idle"` now calls `assertCanResumeAgent` (the `agent_config:update` decision with `requiresChangeGrant`) and returns 409 with the repair guidance when the org chain is invalid, the same as `POST /resume`. - `server/src/__tests__/agent-permissions-routes.test.ts`: pause is denied through both routes for an agent allowed to configure the target, with no lifecycle change, run cancel or activity. A board user can still pause through `PATCH`. `PATCH` idle applies the change-grant decision, allows a granted agent, and keeps the org-chain guard. ## Verification - `cd server && pnpm exec vitest run src/__tests__/agent-permissions-routes.test.ts src/__tests__/agent-cross-tenant-authz-routes.test.ts src/__tests__/board-access-denial.test.ts`: 147 passed. - Without the route change, three new tests fail (PATCH pause by an agent, PATCH idle without the change grant, PATCH idle with an invalid org chain). - Policy evidence: `doc/SPEC-implementation.md` section 9.3 and the `agents.pause-agent` scenario ("Pausing an agent requires board control"). ## Risks - Non-board automation that paused agents through `PATCH` status now gets 403. That is the intended governance boundary. Board users are not affected. - Agents that resumed agents through `PATCH` status now need the same change grant as `POST /resume`. Standard agents get `agents:configure` on creation, so ordinary behavior does not change. - `PATCH` idle now returns 409 for an agent with an invalid org chain, the same as `POST /resume`. ## Model Used Anthropic Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code, with tool use (shell, file editing). ## 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 - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 --- .../agent-permissions-routes.test.ts | 120 ++++++++++++++++++ server/src/routes/agents.ts | 10 +- 2 files changed, 129 insertions(+), 1 deletion(-) diff --git a/server/src/__tests__/agent-permissions-routes.test.ts b/server/src/__tests__/agent-permissions-routes.test.ts index 866f5d5734b..555bc23ccd1 100644 --- a/server/src/__tests__/agent-permissions-routes.test.ts +++ b/server/src/__tests__/agent-permissions-routes.test.ts @@ -88,6 +88,7 @@ const mockHeartbeatService = vi.hoisted(() => ({ getRun: vi.fn(), cancelRun: vi.fn(), cancelInvocationsForAgents: vi.fn(), + cancelActiveForAgent: vi.fn(), })); const mockIssueApprovalService = vi.hoisted(() => ({ @@ -1790,6 +1791,125 @@ describe("agent permission routes", () => { expect(mockAgentService.updateAndTransition).toHaveBeenCalledWith(agentId, "terminate", expect.anything(), expect.anything()); }); + it.each([ + ["PATCH status", (base: string) => request(base).patch(`/api/agents/${agentId}`).send({ status: "paused" })], + ["POST pause", (base: string) => request(base).post(`/api/agents/${agentId}/pause`).send({})], + ])("keeps pausing an agent board-only through %s, even for an agent allowed to configure the target", async (_route, send) => { + mockAgentService.getById.mockResolvedValue({ ...baseAgent, status: "running" }); + const app = await createApp({ + type: "agent", + agentId: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", + companyId, + source: "agent_key", + runId: "run-1", + }); + + const res = await requestApp(app, send); + + expect(res.status, JSON.stringify(res.body)).toBe(403); + expect(res.body.error).toBe("Board access required"); + expect(res.body.code).toBe("board_access_required"); + expect(res.body.remediation).toContain("this denial is final"); + expect(res.body.remediation).toContain(`[Open the agent's profile](//agents/${agentId})`); + expect(mockAgentService.updateAndTransition).not.toHaveBeenCalled(); + expect(mockAgentService.update).not.toHaveBeenCalled(); + expect(mockHeartbeatService.cancelActiveForAgent).not.toHaveBeenCalled(); + expect(mockLogActivity).not.toHaveBeenCalled(); + }); + + it("still lets a board user pause through PATCH status", async () => { + mockAgentService.getById.mockResolvedValue({ ...baseAgent, status: "idle" }); + mockAgentService.updateAndTransition.mockResolvedValue({ ...baseAgent, status: "paused" }); + const app = await createApp({ + type: "board", + userId: "board-user", + source: "local_implicit", + isInstanceAdmin: true, + companyIds: [companyId], + }); + + const res = await requestApp(app, (baseUrl) => request(baseUrl) + .patch(`/api/agents/${agentId}`) + .send({ status: "paused" })); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(mockAgentService.updateAndTransition).toHaveBeenCalledWith(agentId, "pause", expect.anything(), expect.anything()); + }); + + it("applies the resume change-grant decision to PATCH status idle from an agent", async () => { + mockAgentService.getById.mockResolvedValue({ ...baseAgent, status: "paused" }); + mockAccessService.decide.mockImplementation(async (input: { action?: string; scope?: { requiresChangeGrant?: boolean } }) => { + const allowed = !input.scope?.requiresChangeGrant; + return { + allowed, + action: input.action, + reason: allowed ? "allow_explicit_grant" : "deny_missing_consent", + explanation: allowed ? "Allowed by test grant" : "Accepted consent is required for this suggested change.", + }; + }); + const app = await createApp({ + type: "agent", + agentId: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", + companyId, + source: "agent_key", + runId: "run-1", + }); + + const res = await requestApp(app, (baseUrl) => request(baseUrl) + .patch(`/api/agents/${agentId}`) + .send({ status: "idle" })); + + expect(res.status, JSON.stringify(res.body)).toBe(403); + expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({ + action: "agent_config:update", + scope: { requiresChangeGrant: true }, + })); + expect(mockAgentService.updateAndTransition).not.toHaveBeenCalled(); + expect(mockLogActivity).not.toHaveBeenCalled(); + }); + + it("lets a granted agent resume through PATCH status idle", async () => { + mockAgentService.getById.mockResolvedValue({ ...baseAgent, status: "paused" }); + mockAgentService.updateAndTransition.mockResolvedValue({ ...baseAgent, status: "idle" }); + const app = await createApp({ + type: "agent", + agentId: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", + companyId, + source: "agent_key", + runId: "run-1", + }); + + const res = await requestApp(app, (baseUrl) => request(baseUrl) + .patch(`/api/agents/${agentId}`) + .send({ status: "idle" })); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(mockAgentService.updateAndTransition).toHaveBeenCalledWith(agentId, "resume", expect.anything(), expect.anything()); + }); + + it("keeps the invalid-org-chain resume guard on PATCH status idle", async () => { + mockAgentService.getById.mockResolvedValue({ + ...baseAgent, + status: "paused", + orgChainHealth: { status: "invalid_org_chain", reason: "missing_manager", repairGuidance: "Repair the reporting chain first." }, + }); + const app = await createApp({ + type: "board", + userId: "board-user", + source: "local_implicit", + isInstanceAdmin: true, + companyIds: [companyId], + }); + + const res = await requestApp(app, (baseUrl) => request(baseUrl) + .patch(`/api/agents/${agentId}`) + .send({ status: "idle" })); + + expect(res.status).toBe(409); + expect(res.body.error).toBe("Repair the reporting chain first."); + expect(mockAgentService.updateAndTransition).not.toHaveBeenCalled(); + }); + it("terminates directly when no open hire approval is linked", async () => { const idleAgent = { ...baseAgent, status: "idle" }; const terminatedAgent = { ...baseAgent, status: "terminated" }; diff --git a/server/src/routes/agents.ts b/server/src/routes/agents.ts index 1c2de8ccc7a..e95253f8cbd 100644 --- a/server/src/routes/agents.ts +++ b/server/src/routes/agents.ts @@ -5553,8 +5553,16 @@ export function agentRoutes( const requestedStatus = patchData.status; delete patchData.status; if (requestedStatus !== undefined && !["paused", "idle", "terminated"].includes(String(requestedStatus))) throw badRequest("Use an agent lifecycle command to change status"); - // Termination is board-only and irreversible on every route, not just POST /terminate. + // Status changes are lifecycle commands. Apply the authority of the dedicated + // routes (POST /terminate, /pause, /resume) so PATCH is never a way around them. if (requestedStatus === "terminated") await assertBoardCanTerminateAgent(req, existing); + if (requestedStatus === "paused") assertBoard(req); + if (requestedStatus === "idle") { + await assertCanResumeAgent(req, existing); + if (existing.orgChainHealth?.status === "invalid_org_chain") { + throw conflict(existing.orgChainHealth.repairGuidance ?? "Repair this agent's reporting chain before resuming it"); + } + } const replaceAdapterConfig = patchData.replaceAdapterConfig === true; delete patchData.replaceAdapterConfig; // The apply-existing flag is not an agent column. The server binds the fixed