Skip to content

Commit 91abbb4

Browse files
authored
fix(server-core): validate workflow ownership on control endpoints (#1318)
* fix(server-core): validate workflow ownership on control endpoints * chore(ci): rerun flaky build checks * fix(server-core): harden workflow control request validation --------- Co-authored-by: Ruan Chaves <ruanchaves@users.noreply.github.com>
1 parent 5be7626 commit 91abbb4

7 files changed

Lines changed: 221 additions & 11 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
"@voltagent/server-core": patch
3+
"@voltagent/server-hono": patch
4+
"@voltagent/serverless-hono": patch
5+
"@voltagent/server-elysia": patch
6+
---
7+
8+
Validate workflow ownership before suspend and cancel control routes act on an execution.
9+
10+
Fixes #1316.

packages/server-core/src/handlers/workflow.handlers.spec.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { ServerProviderDeps, WorkflowStateEntry } from "@voltagent/core";
22
import { describe, expect, it, vi } from "vitest";
3-
import { handleListWorkflowRuns } from "./workflow.handlers";
3+
import { createWorkflowControlRequestBody, handleListWorkflowRuns } from "./workflow.handlers";
44

55
function createWorkflowState(
66
id: string,
@@ -190,3 +190,15 @@ describe("handleListWorkflowRuns", () => {
190190
);
191191
});
192192
});
193+
194+
describe("createWorkflowControlRequestBody", () => {
195+
it("rejects primitive JSON bodies and annotates object bodies with the route workflow id", () => {
196+
expect(createWorkflowControlRequestBody("invalid", "wf-1")).toBeUndefined();
197+
expect(createWorkflowControlRequestBody(1, "wf-1")).toBeUndefined();
198+
expect(createWorkflowControlRequestBody(["invalid"], "wf-1")).toBeUndefined();
199+
expect(createWorkflowControlRequestBody({ reason: "pause" }, "wf-1")).toEqual({
200+
__workflowId: "wf-1",
201+
reason: "pause",
202+
});
203+
});
204+
});

packages/server-core/src/handlers/workflow.handlers.ts

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -630,13 +630,51 @@ export async function handleAttachWorkflowStream(
630630
}
631631
}
632632

633+
async function isWorkflowExecutionOwnedByRoute(
634+
body: WorkflowControlRequestBody | undefined,
635+
executionId: string,
636+
deps: ServerProviderDeps,
637+
) {
638+
const workflowId = body?.__workflowId;
639+
if (typeof workflowId !== "string" || workflowId.trim().length === 0) {
640+
return false;
641+
}
642+
643+
return (
644+
(
645+
await deps.workflowRegistry
646+
.getWorkflow(workflowId)
647+
?.workflow.memory.getWorkflowState(executionId)
648+
)?.workflowId === workflowId
649+
);
650+
}
651+
652+
export type WorkflowControlRequestBody = Record<string, unknown> & {
653+
__workflowId: string;
654+
reason?: string;
655+
};
656+
657+
export function createWorkflowControlRequestBody(
658+
body: unknown,
659+
workflowId: string,
660+
): WorkflowControlRequestBody | undefined {
661+
if (!body || typeof body !== "object" || Array.isArray(body)) {
662+
return undefined;
663+
}
664+
665+
return {
666+
...(body as Record<string, unknown>),
667+
__workflowId: workflowId,
668+
};
669+
}
670+
633671
/**
634672
* Handler for suspending a workflow
635673
* Returns suspension result
636674
*/
637675
export async function handleSuspendWorkflow(
638676
executionId: string,
639-
body: any,
677+
body: WorkflowControlRequestBody | undefined,
640678
deps: ServerProviderDeps,
641679
logger: Logger,
642680
): Promise<ApiResponse> {
@@ -650,6 +688,13 @@ export async function handleSuspendWorkflow(
650688
};
651689
}
652690

691+
if (!(await isWorkflowExecutionOwnedByRoute(body, executionId, deps))) {
692+
return {
693+
success: false,
694+
error: "Workflow execution not found or already completed",
695+
};
696+
}
697+
653698
const suspendController = deps.workflowRegistry.activeExecutions.get(executionId);
654699

655700
if (!suspendController) {
@@ -694,7 +739,7 @@ export async function handleSuspendWorkflow(
694739
*/
695740
export async function handleCancelWorkflow(
696741
executionId: string,
697-
body: any,
742+
body: WorkflowControlRequestBody | undefined,
698743
deps: ServerProviderDeps,
699744
logger: Logger,
700745
): Promise<ApiResponse> {
@@ -708,6 +753,13 @@ export async function handleCancelWorkflow(
708753
};
709754
}
710755

756+
if (!(await isWorkflowExecutionOwnedByRoute(body, executionId, deps))) {
757+
return {
758+
success: false,
759+
error: "No active execution found or workflow already completed",
760+
};
761+
}
762+
711763
const suspendController = deps.workflowRegistry.activeExecutions.get(executionId);
712764

713765
if (!suspendController) {

packages/server-core/src/handlers/workflow.stream-attach.handlers.spec.ts

Lines changed: 125 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,12 @@ import type { Logger } from "@voltagent/internal";
33
import { describe, expect, it, vi } from "vitest";
44
import type { ErrorResponse } from "../types/responses";
55
import { isErrorResponse } from "../types/responses";
6-
import { handleAttachWorkflowStream, handleStreamWorkflow } from "./workflow.handlers";
6+
import {
7+
handleAttachWorkflowStream,
8+
handleCancelWorkflow,
9+
handleStreamWorkflow,
10+
handleSuspendWorkflow,
11+
} from "./workflow.handlers";
712

813
type ParsedSSEEvent = {
914
id?: string;
@@ -48,7 +53,7 @@ function createDeps(options?: {
4853
const stream = options?.streamFactory ? options.streamFactory : vi.fn();
4954

5055
const workflow = {
51-
createSuspendController: vi.fn().mockReturnValue(createSuspendController()),
56+
createSuspendController: vi.fn().mockImplementation(createSuspendController),
5257
stream,
5358
memory: {
5459
getWorkflowState,
@@ -120,6 +125,65 @@ function assertErrorResponse(
120125
expect(isErrorResponse(value)).toBe(true);
121126
}
122127

128+
async function startBlockingWorkflowExecution(action: string) {
129+
const logger = createLogger();
130+
let releaseStream = () => {};
131+
const streamBlocked = new Promise<void>((resolve) => {
132+
releaseStream = resolve;
133+
});
134+
const executionId = `exec-${action}-1`;
135+
const { deps } = createDeps({
136+
workflowState: {
137+
id: executionId,
138+
workflowId: "wf-1",
139+
workflowName: "Workflow 1",
140+
status: "running",
141+
createdAt: new Date(),
142+
updatedAt: new Date(),
143+
},
144+
streamFactory: () => ({
145+
executionId,
146+
[Symbol.asyncIterator]: async function* () {
147+
yield {
148+
type: "workflow-start",
149+
executionId,
150+
from: "Workflow 1",
151+
status: "running",
152+
timestamp: new Date().toISOString(),
153+
};
154+
await streamBlocked;
155+
},
156+
result: Promise.resolve({ ok: true }),
157+
status: Promise.resolve("completed"),
158+
endAt: Promise.resolve(new Date("2026-01-01T00:00:00.000Z")),
159+
}),
160+
});
161+
162+
const streamResponse = await handleStreamWorkflow("wf-1", { input: {} }, deps, logger);
163+
expect(isErrorResponse(streamResponse)).toBe(false);
164+
165+
if (isErrorResponse(streamResponse)) {
166+
throw new Error("Expected active workflow stream");
167+
}
168+
169+
const streamReader = streamResponse.getReader();
170+
await readSSEEvent(streamReader);
171+
172+
const controller = deps.workflowRegistry.activeExecutions?.get(executionId) as ReturnType<
173+
typeof createSuspendController
174+
>;
175+
expect(controller).toBeDefined();
176+
177+
return {
178+
controller,
179+
deps,
180+
executionId,
181+
logger,
182+
releaseStream,
183+
streamReader,
184+
};
185+
}
186+
123187
describe("workflow stream attach handler", () => {
124188
it("returns 404 when workflow does not exist", async () => {
125189
const logger = createLogger();
@@ -260,4 +324,63 @@ describe("workflow stream attach handler", () => {
260324
const attachedFinal = await readSSEEvent(attachedReader);
261325
expect(attachedFinal.data.type).toBe("workflow-result");
262326
});
327+
328+
it.each([
329+
{
330+
action: "suspend",
331+
error: "not found",
332+
handler: handleSuspendWorkflow,
333+
method: "suspend",
334+
},
335+
{
336+
action: "cancel",
337+
error: "No active execution found",
338+
handler: handleCancelWorkflow,
339+
method: "cancel",
340+
},
341+
])(
342+
"rejects $action requests on the wrong workflow route",
343+
async ({ action, error, handler, method }) => {
344+
const { controller, deps, executionId, logger, releaseStream, streamReader } =
345+
await startBlockingWorkflowExecution(action);
346+
347+
const wrongRouteResponse = await handler(
348+
executionId,
349+
{ __workflowId: "wf-2", reason: "wrong route" },
350+
deps,
351+
logger,
352+
);
353+
354+
expect(wrongRouteResponse.success).toBe(false);
355+
expect((wrongRouteResponse as ErrorResponse).error).toContain(error);
356+
expect(controller[method]).not.toHaveBeenCalled();
357+
expect(deps.workflowRegistry.activeExecutions?.has(executionId)).toBe(true);
358+
359+
const missingRouteResponse = await handler(
360+
executionId,
361+
{ reason: "missing route id" } as any,
362+
deps,
363+
logger,
364+
);
365+
366+
expect(missingRouteResponse.success).toBe(false);
367+
expect((missingRouteResponse as ErrorResponse).error).toContain(error);
368+
expect(controller[method]).not.toHaveBeenCalled();
369+
expect(deps.workflowRegistry.activeExecutions?.has(executionId)).toBe(true);
370+
371+
const correctRouteResponse = await handler(
372+
executionId,
373+
{ __workflowId: "wf-1", reason: "correct route" },
374+
deps,
375+
logger,
376+
);
377+
378+
expect(correctRouteResponse.success).toBe(true);
379+
expect(controller[method]).toHaveBeenCalledWith("correct route");
380+
expect(deps.workflowRegistry.activeExecutions?.has(executionId)).toBe(false);
381+
382+
releaseStream();
383+
await streamReader.cancel();
384+
},
385+
);
263386
});

packages/server-elysia/src/routes/workflow.routes.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { ServerProviderDeps } from "@voltagent/core";
22
import type { Logger } from "@voltagent/internal";
33
import {
4+
createWorkflowControlRequestBody,
45
handleAttachWorkflowStream,
56
handleCancelWorkflow,
67
handleExecuteWorkflow,
@@ -287,7 +288,12 @@ export function registerWorkflowRoutes(
287288
app.post(
288289
"/workflows/:id/executions/:executionId/suspend",
289290
async ({ params, body, set }) => {
290-
const response = await handleSuspendWorkflow(params.executionId, body, deps, logger);
291+
const response = await handleSuspendWorkflow(
292+
params.executionId,
293+
createWorkflowControlRequestBody(body, params.id),
294+
deps,
295+
logger,
296+
);
291297
if (!response.success) {
292298
const errorMessage = response.error || "";
293299
set.status = errorMessage.includes("not found")
@@ -320,7 +326,12 @@ export function registerWorkflowRoutes(
320326
app.post(
321327
"/workflows/:id/executions/:executionId/cancel",
322328
async ({ params, body, set }) => {
323-
const response = await handleCancelWorkflow(params.executionId, body, deps, logger);
329+
const response = await handleCancelWorkflow(
330+
params.executionId,
331+
createWorkflowControlRequestBody(body, params.id),
332+
deps,
333+
logger,
334+
);
324335
if (!response.success) {
325336
const errorMessage = response.error || "";
326337
set.status = errorMessage.includes("not found")

packages/server-hono/src/routes/index.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { ServerProviderDeps } from "@voltagent/core";
22
import type { Logger } from "@voltagent/internal";
33
import {
44
UPDATE_ROUTES,
5+
createWorkflowControlRequestBody,
56
handleAttachWorkflowStream,
67
handleCancelWorkflow,
78
handleChatStream,
@@ -412,7 +413,7 @@ export function registerWorkflowRoutes(
412413
if (!executionId) {
413414
throw new Error("Missing execution id parameter");
414415
}
415-
const body = await c.req.json();
416+
const body = createWorkflowControlRequestBody(await c.req.json(), c.req.param("id"));
416417
const response = await handleSuspendWorkflow(executionId, body, deps, logger);
417418
if (!response.success) {
418419
return c.json(response, 500);
@@ -426,7 +427,7 @@ export function registerWorkflowRoutes(
426427
if (!executionId) {
427428
throw new Error("Missing execution id parameter");
428429
}
429-
const body = await c.req.json();
430+
const body = createWorkflowControlRequestBody(await c.req.json(), c.req.param("id"));
430431
const response = await handleCancelWorkflow(executionId, body, deps, logger);
431432
if (!response.success) {
432433
const errorMessage = response.error || "";

packages/serverless-hono/src/routes.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import {
3333
type TriggerHttpRequestContext,
3434
UPDATE_ROUTES,
3535
WORKFLOW_ROUTES,
36+
createWorkflowControlRequestBody,
3637
executeA2ARequest,
3738
executeTriggerHandler,
3839
getConversationMessagesHandler,
@@ -528,7 +529,7 @@ export function registerWorkflowRoutes(app: Hono, deps: ServerProviderDeps, logg
528529

529530
app.post(WORKFLOW_ROUTES.suspendWorkflow.path, async (c) => {
530531
const executionId = c.req.param("executionId");
531-
const body = await readJsonBody(c, logger);
532+
const body = createWorkflowControlRequestBody(await readJsonBody(c, logger), c.req.param("id"));
532533
if (!body) {
533534
return c.json({ success: false, error: "Invalid JSON body" }, 400);
534535
}
@@ -547,7 +548,7 @@ export function registerWorkflowRoutes(app: Hono, deps: ServerProviderDeps, logg
547548

548549
app.post(WORKFLOW_ROUTES.cancelWorkflow.path, async (c) => {
549550
const executionId = c.req.param("executionId");
550-
const body = await readJsonBody(c, logger);
551+
const body = createWorkflowControlRequestBody(await readJsonBody(c, logger), c.req.param("id"));
551552
if (!body) {
552553
return c.json({ success: false, error: "Invalid JSON body" }, 400);
553554
}

0 commit comments

Comments
 (0)