From 61ed4ef90c5a94a910b905766cb2e155d172f117 Mon Sep 17 00:00:00 2001 From: dotta Date: Thu, 9 Apr 2026 07:29:56 -0500 Subject: [PATCH] fix(server): reject non-participant stage mutations --- .../issue-execution-policy-routes.test.ts | 77 +++++++++++++++++-- server/src/routes/issues.ts | 34 +++++++- 2 files changed, 102 insertions(+), 9 deletions(-) diff --git a/server/src/__tests__/issue-execution-policy-routes.test.ts b/server/src/__tests__/issue-execution-policy-routes.test.ts index 4f8d9fc6..190cb077 100644 --- a/server/src/__tests__/issue-execution-policy-routes.test.ts +++ b/server/src/__tests__/issue-execution-policy-routes.test.ts @@ -60,17 +60,19 @@ vi.mock("../services/index.js", () => ({ workProductService: () => ({}), })); -function createApp() { +function createApp( + actor: Record = { + type: "board", + userId: "local-board", + companyIds: ["company-1"], + source: "local_implicit", + isInstanceAdmin: false, + }, +) { const app = express(); app.use(express.json()); app.use((req, _res, next) => { - (req as any).actor = { - type: "board", - userId: "local-board", - companyIds: ["company-1"], - source: "local_implicit", - isInstanceAdmin: false, - }; + (req as any).actor = actor; next(); }); app.use("/api", issueRoutes({} as any, {} as any)); @@ -137,4 +139,63 @@ describe("issue execution policy routes", () => { expect(updatePatch.executionState).toBeUndefined(); expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled(); }); + + it("rejects agent stage advances from non-participants", async () => { + const reviewerAgentId = "33333333-3333-4333-8333-333333333333"; + const approverAgentId = "44444444-4444-4444-8444-444444444444"; + const executorAgentId = "22222222-2222-4222-8222-222222222222"; + const policy = normalizeIssueExecutionPolicy({ + stages: [ + { + id: "11111111-1111-4111-8111-111111111111", + type: "review", + participants: [{ type: "agent", agentId: reviewerAgentId }], + }, + { + id: "55555555-5555-4555-8555-555555555555", + type: "approval", + participants: [{ type: "agent", agentId: approverAgentId }], + }, + ], + })!; + const issue = { + id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", + companyId: "company-1", + status: "in_review", + assigneeAgentId: reviewerAgentId, + assigneeUserId: null, + createdByUserId: "local-board", + identifier: "PAP-1000", + title: "Execution policy guard", + executionPolicy: policy, + executionState: { + status: "pending", + currentStageId: "11111111-1111-4111-8111-111111111111", + currentStageIndex: 0, + currentStageType: "review", + currentParticipant: { type: "agent", agentId: reviewerAgentId }, + returnAssignee: { type: "agent", agentId: executorAgentId }, + completedStageIds: [], + lastDecisionId: null, + lastDecisionOutcome: null, + }, + }; + mockIssueService.getById.mockResolvedValue(issue); + + const res = await request( + createApp({ + type: "agent", + agentId: approverAgentId, + companyId: "company-1", + source: "api_key", + runId: "run-1", + }), + ) + .patch("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa") + .send({ status: "done", comment: "Skipping review." }); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("active review participant"); + expect(mockIssueService.update).not.toHaveBeenCalled(); + }); }); diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index 561c1331..344c262c 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -96,6 +96,13 @@ function executionPrincipalsEqual( return left.type === "agent" ? left.agentId === right.agentId : left.userId === right.userId; } +function executionParticipantMatchesAgent( + participant: ParsedExecutionState["currentParticipant"] | null, + agentId: string | null | undefined, +) { + return Boolean(agentId) && participant?.type === "agent" && participant.agentId === agentId; +} + function buildExecutionStageWakeContext(input: { state: ParsedExecutionState; wakeRole: ExecutionStageWakeContext["wakeRole"]; @@ -1379,10 +1386,14 @@ export function issueRoutes( ? (updateFields.executionPolicy as NormalizedExecutionPolicy | null) : previousExecutionPolicy; + const requestedStatus = typeof updateFields.status === "string" ? updateFields.status : undefined; + const requestedAssigneePatchProvided = + req.body.assigneeAgentId !== undefined || req.body.assigneeUserId !== undefined; + const transition = applyIssueExecutionPolicyTransition({ issue: existing, policy: nextExecutionPolicy, - requestedStatus: typeof updateFields.status === "string" ? updateFields.status : undefined, + requestedStatus, requestedAssigneePatch: { assigneeAgentId: req.body.assigneeAgentId === undefined ? undefined : (req.body.assigneeAgentId as string | null), @@ -1408,6 +1419,27 @@ export function issueRoutes( } Object.assign(updateFields, transition.patch); + const effectiveExecutionState = parseIssueExecutionState( + transition.patch.executionState !== undefined ? transition.patch.executionState : existing.executionState, + ); + const isUnauthorizedAgentStageMutation = + req.actor.type === "agent" && + req.actor.agentId && + existing.status === "in_review" && + transition.workflowControlledAssignment && + !transition.decision && + effectiveExecutionState?.status === "pending" && + ( + (requestedStatus !== undefined && requestedStatus !== "in_review") || + requestedAssigneePatchProvided + ) && + !executionParticipantMatchesAgent(effectiveExecutionState.currentParticipant, req.actor.agentId); + if (isUnauthorizedAgentStageMutation) { + const stageLabel = effectiveExecutionState.currentStageType ?? "execution"; + res.status(403).json({ error: `Only the active ${stageLabel} participant can update this stage` }); + return; + } + const nextAssigneeAgentId = updateFields.assigneeAgentId === undefined ? existing.assigneeAgentId : (updateFields.assigneeAgentId as string | null); const nextAssigneeUserId =