Skip to content

Commit d195710

Browse files
authored
fix(resume): require write access to resume a paused run on the legacy route (#8896)
* fix(resume): require write access to resume a paused run on the legacy route POST /api/resume/[workflowId]/[executionId]/[contextId] authorized with validateWorkflowAccess, which checks workspace read permission, so a read-only member (or their API key) could answer a human-in-the-loop pause and continue the run with arbitrary input. The route now authenticates with the internal workflow session/API-key policy and runs the resumeWorkflowRun use case, the same write-role operation the v2 resume route uses. resumeWorkflowRun takes the calling surface ('legacy' | 'v2') so the legacy route keeps its streaming and job-id polling responses, and forwards the request signal and headers a streamed resume needs. * fix(resume): let unexpected resume failures reach the route handler as a 500 * refactor(resume): fold the legacy route's auth catch into one handler and tighten comments * docs(resume): note the v2 job-id dedupe on the executor's pollingSurface option * fix(resume): withhold unsafe resume failure messages on the legacy route Also drop two unused exports from the v2 Project file search contract that fail check:unused-exports on staging. * test(resume): name the v2 dispatch test after what it asserts
1 parent 22a0333 commit d195710

8 files changed

Lines changed: 292 additions & 264 deletions

File tree

‎apps/sim/app/api/resume/[workflowId]/[executionId]/[contextId]/route.test.ts‎

Lines changed: 133 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1+
import { createWorkspaceApiKeyPrincipal } from '@sim/testing/factories/principal.factory'
12
import { createRouteContext } from '@sim/testing/helpers/http'
23
import { asyncJobsMock, asyncJobsMockFns } from '@sim/testing/mocks/async-jobs.mock'
4+
import { authMockFns } from '@sim/testing/mocks/auth.mock'
35
import {
46
executionPreprocessingMock,
57
executionPreprocessingMockFns,
@@ -10,20 +12,30 @@ import {
1012
} from '@sim/testing/mocks/human-in-the-loop-manager.mock'
1113
import { idMock, idMockFns } from '@sim/testing/mocks/id.mock'
1214
import { createMockRequest } from '@sim/testing/mocks/request.mock'
15+
import {
16+
workflowContextMock,
17+
workflowContextMockFns,
18+
} from '@sim/testing/mocks/workflow-context.mock'
19+
import { workspaceAuthzMock, workspaceAuthzMockFns } from '@sim/testing/mocks/workspace-authz.mock'
1320
import {
1421
workspacesUtilsMock,
1522
workspacesUtilsMockFns,
1623
} from '@sim/testing/mocks/workspaces-utils.mock'
1724
import { beforeEach, describe, expect, it, vi } from 'vitest'
1825

19-
const { mockValidateWorkflowAccess } = vi.hoisted(() => ({
20-
mockValidateWorkflowAccess: vi.fn(),
26+
const { mockAuthenticateApiKey } = vi.hoisted(() => ({
27+
mockAuthenticateApiKey: vi.fn(),
2128
}))
2229

23-
vi.mock('@/app/api/workflows/middleware', () => ({
24-
validateWorkflowAccess: mockValidateWorkflowAccess,
30+
vi.mock('@/lib/api-key/service', () => ({
31+
authenticateApiKeyFromHeader: mockAuthenticateApiKey,
32+
updateApiKeyLastUsed: vi.fn(),
2533
}))
2634

35+
vi.mock('@sim/platform-authz/workspace', () => workspaceAuthzMock)
36+
37+
vi.mock('@/lib/workflows/application/context', () => workflowContextMock)
38+
2739
vi.mock('@/lib/execution/preprocessing', () => executionPreprocessingMock)
2840

2941
vi.mock('@/lib/core/async-jobs', () => asyncJobsMock)
@@ -38,15 +50,17 @@ vi.mock('@/lib/workspaces/utils', () => workspacesUtilsMock)
3850

3951
vi.mock('@/lib/workflows/executor/human-in-the-loop-manager', () => humanInTheLoopManagerMock)
4052

53+
import { resumeWorkflowRun } from '@/lib/workflows/application/resume-run'
4154
import { executeResumeWorkflow } from '@/lib/workflows/executor/resume-execution'
4255
import { POST } from '@/app/api/resume/[workflowId]/[executionId]/[contextId]/route'
43-
import { handleResumeExecution } from '@/app/api/resume/resume-handler'
4456
import { attachExecutionResult, buildBlockExecutionError } from '@/executor/utils/errors'
4557
import type { SerializedBlock } from '@/serializer/types'
4658

4759
const { mockEnqueueOrStartResume, mockGetPausedExecutionDetail, mockStartResumeExecution } =
4860
humanInTheLoopManagerMockFns
4961
const { mockGetWorkspaceBilledAccountUserId: mockGetCurrentPayer } = workspacesUtilsMockFns
62+
const mockResolvePermission = workspaceAuthzMockFns.mockResolveEffectiveWorkspacePermission
63+
const mockResolveRunContext = workflowContextMockFns.mockResolveActiveWorkflowRunApplicationContext
5064

5165
const { mockShouldExecuteInline } = asyncJobsMockFns
5266
mockShouldExecuteInline.mockReturnValue(false)
@@ -61,6 +75,11 @@ const EXECUTION_ID = 'execution-1'
6175
const CONTEXT_ID = 'context-1'
6276
const WORKSPACE_ID = 'workspace-1'
6377
const PERSISTED_ACTOR_ID = 'original-actor'
78+
const WORKSPACE_BILLING_OWNER_ID = 'current-workspace-owner'
79+
const WORKSPACE_KEY_PRINCIPAL = createWorkspaceApiKeyPrincipal({
80+
workspaceId: WORKSPACE_ID,
81+
keyId: 'workspace-key',
82+
})
6483

6584
const PERSISTED_ATTRIBUTION = {
6685
actorUserId: PERSISTED_ACTOR_ID,
@@ -143,13 +162,17 @@ function makeRequest(
143162
executionId: EXECUTION_ID,
144163
contextId: CONTEXT_ID,
145164
},
146-
body = JSON.stringify({ input: { approved: true } })
165+
body = JSON.stringify({ input: { approved: true } }),
166+
apiKey: string | null = 'sim_workspace_key'
147167
) {
148168
return {
149169
request: createMockRequest({
150170
method: 'POST',
151171
url: `http://localhost/api/resume/${params.workflowId}/${params.executionId}/${params.contextId}`,
152-
headers: { 'Content-Type': 'application/json' },
172+
headers: {
173+
'Content-Type': 'application/json',
174+
...(apiKey ? { 'x-api-key': apiKey } : {}),
175+
},
153176
rawBody: body,
154177
}),
155178
context: createRouteContext(params),
@@ -158,20 +181,23 @@ function makeRequest(
158181

159182
describe('POST /api/resume/[workflowId]/[executionId]/[contextId]', () => {
160183
beforeEach(() => {
161-
mockValidateWorkflowAccess.mockResolvedValue({
162-
workflow: {
163-
id: WORKFLOW_ID,
164-
workspaceId: WORKSPACE_ID,
165-
},
166-
auth: {
167-
success: true,
168-
userId: 'current-api-key-user',
169-
authType: 'api_key',
170-
apiKeyType: 'workspace',
171-
workspaceId: WORKSPACE_ID,
172-
},
184+
mockAuthenticateApiKey.mockResolvedValue({
185+
success: true,
186+
keyId: WORKSPACE_KEY_PRINCIPAL.keyId,
187+
keyType: 'workspace',
188+
workspaceId: WORKSPACE_ID,
173189
})
174-
mockGetCurrentPayer.mockResolvedValue('current-workspace-owner')
190+
mockResolvePermission.mockResolvedValue('write')
191+
mockResolveRunContext.mockResolvedValue({
192+
workflowId: WORKFLOW_ID,
193+
workflow: { id: WORKFLOW_ID, workspaceId: WORKSPACE_ID },
194+
workspaceId: WORKSPACE_ID,
195+
workspaceOrganizationId: null,
196+
allowPersonalApiKeys: true,
197+
billedAccountUserId: WORKSPACE_BILLING_OWNER_ID,
198+
runId: EXECUTION_ID,
199+
})
200+
mockGetCurrentPayer.mockResolvedValue(WORKSPACE_BILLING_OWNER_ID)
175201
mockGetPausedExecutionDetail.mockResolvedValue(createPausedExecution())
176202
mockPreprocessExecution.mockResolvedValue({
177203
success: true,
@@ -187,9 +213,7 @@ describe('POST /api/resume/[workflowId]/[executionId]/[contextId]', () => {
187213
})
188214

189215
it('returns 401 before validating malformed route input', async () => {
190-
mockValidateWorkflowAccess.mockResolvedValueOnce({
191-
error: { message: 'Unauthorized', status: 401 },
192-
})
216+
mockAuthenticateApiKey.mockResolvedValueOnce({ success: false })
193217
const { request, context } = makeRequest(
194218
{
195219
workflowId: WORKFLOW_ID,
@@ -202,18 +226,62 @@ describe('POST /api/resume/[workflowId]/[executionId]/[contextId]', () => {
202226
const response = await POST(request, context)
203227

204228
expect(response.status).toBe(401)
205-
expect(await response.json()).toEqual({ error: 'Unauthorized' })
206-
expect(mockValidateWorkflowAccess).toHaveBeenCalledWith(request, WORKFLOW_ID, false)
229+
expect(await response.json()).toMatchObject({ error: 'Unauthorized' })
230+
expect(mockResolveRunContext).not.toHaveBeenCalled()
207231
expect(mockGetPausedExecutionDetail).not.toHaveBeenCalled()
208232
expect(mockPreprocessExecution).not.toHaveBeenCalled()
209233
})
210234

235+
it.each([
236+
{
237+
name: 'session',
238+
apiKey: null,
239+
setup: () =>
240+
authMockFns.mockGetSession.mockResolvedValueOnce({
241+
user: { id: 'read-only-member' },
242+
session: { id: 'session-1' },
243+
}),
244+
},
245+
{
246+
name: 'personal API key',
247+
apiKey: 'sim_personal_key',
248+
setup: () =>
249+
mockAuthenticateApiKey.mockResolvedValue({
250+
success: true,
251+
keyId: 'personal-key',
252+
keyType: 'personal',
253+
userId: 'read-only-member',
254+
}),
255+
},
256+
])('refuses a resume from a read-only member over $name auth', async ({ apiKey, setup }) => {
257+
setup()
258+
mockResolvePermission.mockResolvedValue('read')
259+
const { request, context } = makeRequest(undefined, undefined, apiKey)
260+
261+
const response = await POST(request, context)
262+
263+
expect(response.status).toBe(403)
264+
expect(mockGetPausedExecutionDetail).not.toHaveBeenCalled()
265+
expect(mockPreprocessExecution).not.toHaveBeenCalled()
266+
expect(mockEnqueueOrStartResume).not.toHaveBeenCalled()
267+
})
268+
269+
it('answers an unexpected run-context failure with a generic 500', async () => {
270+
mockResolveRunContext.mockRejectedValueOnce(new Error('connection terminated: db-internal'))
271+
const { request, context } = makeRequest()
272+
273+
const response = await POST(request, context)
274+
275+
expect(response.status).toBe(500)
276+
expect(await response.json()).toMatchObject({ error: 'Internal server error' })
277+
expect(mockEnqueueOrStartResume).not.toHaveBeenCalled()
278+
})
279+
211280
it('reuses the persisted actor and payer snapshot for route preflight', async () => {
212281
const { request, context } = makeRequest()
213282

214283
const response = await POST(request, context)
215284

216-
expect(mockValidateWorkflowAccess).toHaveBeenCalledWith(request, WORKFLOW_ID, false)
217285
expect(response.status).toBe(200)
218286
expect(mockGetCurrentPayer).not.toHaveBeenCalled()
219287
expect(mockGetPausedExecutionDetail).toHaveBeenCalledWith({
@@ -223,7 +291,7 @@ describe('POST /api/resume/[workflowId]/[executionId]/[contextId]', () => {
223291
expect(mockPreprocessExecution).toHaveBeenCalledWith(
224292
expect.objectContaining({
225293
workflowId: WORKFLOW_ID,
226-
userId: 'current-api-key-user',
294+
userId: WORKSPACE_BILLING_OWNER_ID,
227295
workspaceId: WORKSPACE_ID,
228296
billingAttribution: PERSISTED_ATTRIBUTION,
229297
executionId: 'resume-preflight-1',
@@ -238,7 +306,7 @@ describe('POST /api/resume/[workflowId]/[executionId]/[contextId]', () => {
238306
workflowId: WORKFLOW_ID,
239307
contextId: CONTEXT_ID,
240308
resumeInput: { approved: true },
241-
userId: 'current-api-key-user',
309+
userId: WORKSPACE_BILLING_OWNER_ID,
242310
allowedPauseKinds: ['human'],
243311
})
244312
})
@@ -254,7 +322,7 @@ describe('POST /api/resume/[workflowId]/[executionId]/[contextId]', () => {
254322
pausedExecution: { id: 'paused-execution-1' },
255323
contextId: CONTEXT_ID,
256324
resumeInput: { approved: true },
257-
userId: 'current-api-key-user',
325+
userId: WORKSPACE_BILLING_OWNER_ID,
258326
})
259327
const { request, context } = makeRequest()
260328

@@ -279,7 +347,7 @@ describe('POST /api/resume/[workflowId]/[executionId]/[contextId]', () => {
279347
expect(mockEnqueueResume.mock.calls[0]?.[2]).not.toHaveProperty('jobId')
280348
})
281349

282-
it('uses deterministic dispatch and execution polling for the v2 surface', async () => {
350+
it('uses deterministic dispatch for the v2 surface', async () => {
283351
mockGetPausedExecutionDetail.mockResolvedValueOnce(
284352
createPausedExecution({ executionMode: 'async' })
285353
)
@@ -290,30 +358,21 @@ describe('POST /api/resume/[workflowId]/[executionId]/[contextId]', () => {
290358
pausedExecution: { id: 'paused-execution-1' },
291359
contextId: CONTEXT_ID,
292360
resumeInput: { approved: true },
293-
userId: 'current-api-key-user',
361+
userId: WORKSPACE_BILLING_OWNER_ID,
294362
})
295-
const { request } = makeRequest()
296363

297-
const response = await handleResumeExecution({
298-
request,
299-
workflowId: WORKFLOW_ID,
300-
executionId: EXECUTION_ID,
301-
contextId: CONTEXT_ID,
302-
workspaceId: WORKSPACE_ID,
303-
userId: 'current-api-key-user',
304-
resumeInput: { approved: true },
305-
isApiCaller: true,
306-
pollingSurface: 'v2',
364+
const result = await resumeWorkflowRun.execute({
365+
principal: WORKSPACE_KEY_PRINCIPAL,
366+
input: {
367+
workflowId: WORKFLOW_ID,
368+
runId: EXECUTION_ID,
369+
contextId: CONTEXT_ID,
370+
resumeInput: { approved: true },
371+
surface: 'v2',
372+
},
307373
})
308374

309-
expect(response.status).toBe(202)
310-
await expect(response.json()).resolves.toEqual({
311-
success: true,
312-
async: true,
313-
executionId: 'resume-execution-1',
314-
message: 'Resume execution queued',
315-
statusUrl: 'https://test.sim.ai/api/v2/workflows/workflow-1/runs/resume-execution-1',
316-
})
375+
expect(result).toMatchObject({ kind: 'async', executionId: 'resume-execution-1' })
317376
expect(mockEnqueueResume).toHaveBeenCalledWith(
318377
'resume-execution',
319378
expect.objectContaining({ resumeExecutionId: 'resume-execution-1' }),
@@ -335,30 +394,21 @@ describe('POST /api/resume/[workflowId]/[executionId]/[contextId]', () => {
335394
pausedExecution: { id: 'paused-execution-1' },
336395
contextId: CONTEXT_ID,
337396
resumeInput: { approved: true },
338-
userId: 'current-api-key-user',
397+
userId: WORKSPACE_BILLING_OWNER_ID,
339398
})
340-
const { request } = makeRequest()
341399

342-
const response = await handleResumeExecution({
343-
request,
344-
workflowId: WORKFLOW_ID,
345-
executionId: EXECUTION_ID,
346-
contextId: CONTEXT_ID,
347-
workspaceId: WORKSPACE_ID,
348-
userId: 'current-api-key-user',
349-
resumeInput: { approved: true },
350-
isApiCaller: true,
351-
pollingSurface: 'v2',
352-
allowStreaming: false,
400+
const result = await resumeWorkflowRun.execute({
401+
principal: WORKSPACE_KEY_PRINCIPAL,
402+
input: {
403+
workflowId: WORKFLOW_ID,
404+
runId: EXECUTION_ID,
405+
contextId: CONTEXT_ID,
406+
resumeInput: { approved: true },
407+
surface: 'v2',
408+
},
353409
})
354410

355-
expect(response.status).toBe(202)
356-
expect(response.headers.get('Content-Type')).toContain('application/json')
357-
await expect(response.json()).resolves.toMatchObject({
358-
async: true,
359-
executionId: 'resume-execution-1',
360-
statusUrl: 'https://test.sim.ai/api/v2/workflows/workflow-1/runs/resume-execution-1',
361-
})
411+
expect(result).toMatchObject({ kind: 'async', executionId: 'resume-execution-1' })
362412
expect(mockEnqueueResume).toHaveBeenCalledWith(
363413
'resume-execution',
364414
expect.objectContaining({ resumeExecutionId: 'resume-execution-1' }),
@@ -463,12 +513,17 @@ describe('POST /api/resume/[workflowId]/[executionId]/[contextId]', () => {
463513
})
464514

465515
it.each([
466-
{ statusCode: 402, message: 'Member usage limit reached', retryable: false },
467-
{ statusCode: 429, message: 'Target concurrency full', retryable: true },
468-
{ statusCode: 503, message: 'Usage admission unavailable', retryable: true },
516+
{ statusCode: 402, message: 'Member usage limit reached', retryable: false, body: undefined },
517+
{ statusCode: 429, message: 'Target concurrency full', retryable: true, body: undefined },
518+
{
519+
statusCode: 503,
520+
message: 'Usage admission unavailable',
521+
retryable: true,
522+
body: 'Internal server error',
523+
},
469524
])(
470525
'leaves the pause and queued input untouched when readmission returns $statusCode',
471-
async ({ statusCode, message, retryable }) => {
526+
async ({ statusCode, message, retryable, body }) => {
472527
mockPreprocessExecution.mockResolvedValueOnce({
473528
success: false,
474529
error: { statusCode, message, retryable },
@@ -478,7 +533,7 @@ describe('POST /api/resume/[workflowId]/[executionId]/[contextId]', () => {
478533
const response = await POST(request, context)
479534

480535
expect(response.status).toBe(statusCode)
481-
expect(await response.json()).toEqual({ error: message })
536+
expect(await response.json()).toEqual({ error: body ?? message })
482537
expect(mockEnqueueOrStartResume).not.toHaveBeenCalled()
483538
}
484539
)
@@ -493,7 +548,7 @@ describe('POST /api/resume/[workflowId]/[executionId]/[contextId]', () => {
493548

494549
expect(response.status).toBe(500)
495550
expect(await response.json()).toEqual({
496-
error: 'Paused execution billing attribution is missing or invalid',
551+
error: 'Internal server error',
497552
})
498553
expect(mockPreprocessExecution).not.toHaveBeenCalled()
499554
expect(mockEnqueueOrStartResume).not.toHaveBeenCalled()
@@ -526,7 +581,7 @@ describe('POST /api/resume/[workflowId]/[executionId]/[contextId]', () => {
526581

527582
expect(response.status).toBe(500)
528583
expect(await response.json()).toEqual({
529-
error: 'Paused execution billing attribution does not match its workspace or actor',
584+
error: 'Internal server error',
530585
})
531586
expect(mockPreprocessExecution).not.toHaveBeenCalled()
532587
expect(mockEnqueueOrStartResume).not.toHaveBeenCalled()
@@ -543,7 +598,7 @@ describe('POST /api/resume/[workflowId]/[executionId]/[contextId]', () => {
543598

544599
expect(response.status).toBe(500)
545600
expect(await response.json()).toEqual({
546-
error: 'Paused execution snapshot does not match the requested workflow or execution',
601+
error: 'Internal server error',
547602
})
548603
expect(mockPreprocessExecution).not.toHaveBeenCalled()
549604
expect(mockEnqueueOrStartResume).not.toHaveBeenCalled()

0 commit comments

Comments
 (0)