forked from farhoodlabs/paperclip
fix(server): reject non-participant stage mutations
This commit is contained in:
@@ -60,17 +60,19 @@ vi.mock("../services/index.js", () => ({
|
||||
workProductService: () => ({}),
|
||||
}));
|
||||
|
||||
function createApp() {
|
||||
function createApp(
|
||||
actor: Record<string, unknown> = {
|
||||
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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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 =
|
||||
|
||||
Reference in New Issue
Block a user