Skip to content

Commit b381908

Browse files
fix(workflows): enforce blocker follow-ups
1 parent 6e225ac commit b381908

File tree

4 files changed

+186
-19
lines changed

4 files changed

+186
-19
lines changed

apps/sim/app/api/workflows/[id]/deploy/route.test.ts

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,91 @@ describe('Workflow deploy route', () => {
332332
expect(mockUndeployWorkflow).toHaveBeenCalledWith({ workflowId: 'wf-1' })
333333
})
334334

335+
it('fails redeploy rollback when failed deployment version deletion reports failure', async () => {
336+
mockDbLimit.mockResolvedValue([
337+
{ id: 'prev-1', deployedAt: new Date('2024-01-01T00:00:00.000Z') },
338+
])
339+
mockValidateWorkflowAccess.mockResolvedValue({
340+
workflow: { id: 'wf-1', name: 'Test Workflow', workspaceId: 'ws-1' },
341+
auth: {
342+
success: true,
343+
userId: 'api-user',
344+
userName: 'API Key Actor',
345+
userEmail: 'api@example.com',
346+
authType: 'api_key',
347+
},
348+
})
349+
mockDeployWorkflow.mockResolvedValue({
350+
success: true,
351+
deployedAt: '2024-02-01T00:00:00Z',
352+
deploymentVersionId: 'dep-failed',
353+
})
354+
mockSaveTriggerWebhooksForDeploy.mockResolvedValue({
355+
success: false,
356+
error: { message: 'Failed to save trigger configuration', status: 500 },
357+
})
358+
mockDeleteDeploymentVersionById.mockResolvedValue({
359+
success: false,
360+
error: 'delete failed',
361+
})
362+
363+
const req = new NextRequest('http://localhost:3000/api/workflows/wf-1/deploy', {
364+
method: 'POST',
365+
headers: { 'x-api-key': 'test-key' },
366+
})
367+
const response = await POST(req, { params: Promise.resolve({ id: 'wf-1' }) })
368+
369+
expect(response.status).toBe(500)
370+
expect(await response.json()).toEqual({ error: 'delete failed', code: 'DELETE_FAILED' })
371+
expect(mockReactivateWorkflowVersionForRollback).toHaveBeenCalled()
372+
expect(mockDeleteDeploymentVersionById).toHaveBeenCalledWith({
373+
workflowId: 'wf-1',
374+
deploymentVersionId: 'dep-failed',
375+
})
376+
})
377+
378+
it('fails first deploy rollback when failed deployment version deletion reports failure', async () => {
379+
mockValidateWorkflowAccess.mockResolvedValue({
380+
workflow: { id: 'wf-1', name: 'Test Workflow', workspaceId: 'ws-1' },
381+
auth: {
382+
success: true,
383+
userId: 'api-user',
384+
userName: 'API Key Actor',
385+
userEmail: 'api@example.com',
386+
authType: 'api_key',
387+
},
388+
})
389+
mockDeployWorkflow.mockResolvedValue({
390+
success: true,
391+
deployedAt: '2024-02-01T00:00:00Z',
392+
deploymentVersionId: 'dep-failed',
393+
})
394+
mockSaveTriggerWebhooksForDeploy.mockResolvedValue({
395+
success: false,
396+
error: { message: 'Failed to save trigger configuration', status: 500 },
397+
})
398+
mockDbLimit.mockResolvedValue([])
399+
mockUndeployWorkflow.mockResolvedValue({ success: true })
400+
mockDeleteDeploymentVersionById.mockResolvedValue({
401+
success: false,
402+
error: 'delete failed',
403+
})
404+
405+
const req = new NextRequest('http://localhost:3000/api/workflows/wf-1/deploy', {
406+
method: 'POST',
407+
headers: { 'x-api-key': 'test-key' },
408+
})
409+
const response = await POST(req, { params: Promise.resolve({ id: 'wf-1' }) })
410+
411+
expect(response.status).toBe(500)
412+
expect(await response.json()).toEqual({ error: 'delete failed', code: 'DELETE_FAILED' })
413+
expect(mockUndeployWorkflow).toHaveBeenCalledWith({ workflowId: 'wf-1' })
414+
expect(mockDeleteDeploymentVersionById).toHaveBeenCalledWith({
415+
workflowId: 'wf-1',
416+
deploymentVersionId: 'dep-failed',
417+
})
418+
})
419+
335420
it('allows API-key auth for undeploy using hybrid auth userId', async () => {
336421
mockValidateWorkflowAccess.mockResolvedValue({
337422
workflow: { id: 'wf-1', name: 'Test Workflow', workspaceId: 'ws-1' },

apps/sim/app/api/workflows/[id]/deploy/route.ts

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,21 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{
131131
const previousDeployedAt = currentActiveVersion?.deployedAt ?? null
132132

133133
const rollbackDeployment = async (failedDeploymentVersionId?: string) => {
134+
const ensureFailedDeploymentVersionDeleted = async () => {
135+
if (!failedDeploymentVersionId) {
136+
return
137+
}
138+
139+
const deleteResult = await deleteDeploymentVersionById({
140+
workflowId: id,
141+
deploymentVersionId: failedDeploymentVersionId,
142+
})
143+
144+
if (!deleteResult.success) {
145+
throw new Error(deleteResult.error || 'Failed to delete failed deployment version')
146+
}
147+
}
148+
134149
if (previousVersionId) {
135150
await restorePreviousVersionWebhooks({
136151
request,
@@ -150,24 +165,13 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{
150165
deploymentVersionId: previousVersionId,
151166
})
152167
if (reactivateResult.success) {
153-
if (failedDeploymentVersionId) {
154-
await deleteDeploymentVersionById({
155-
workflowId: id,
156-
deploymentVersionId: failedDeploymentVersionId,
157-
})
158-
}
168+
await ensureFailedDeploymentVersionDeleted()
159169
return
160170
}
161171
}
162172

163-
if (failedDeploymentVersionId) {
164-
await deleteDeploymentVersionById({
165-
workflowId: id,
166-
deploymentVersionId: failedDeploymentVersionId,
167-
})
168-
}
169-
170173
await undeployWorkflow({ workflowId: id })
174+
await ensureFailedDeploymentVersionDeleted()
171175
}
172176

173177
const deployResult = await deployWorkflow({

apps/sim/app/api/workflows/[id]/route.test.ts

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
1919
const mockCheckHybridAuth = vi.fn()
2020
const mockCheckSessionOrInternalAuth = vi.fn()
2121
const mockLoadWorkflowFromNormalizedTables = vi.fn()
22+
const mockGetActiveWorkflowContext = vi.fn()
2223
const mockGetWorkflowById = vi.fn()
2324
const mockAuthorizeWorkflowByWorkspacePermission = vi.fn()
2425
const mockArchiveWorkflow = vi.fn()
@@ -77,6 +78,10 @@ vi.mock('@/lib/workflows/persistence/utils', () => ({
7778
mockLoadWorkflowFromNormalizedTables(workflowId),
7879
}))
7980

81+
vi.mock('@/lib/workflows/active-context', () => ({
82+
getActiveWorkflowContext: (workflowId: string) => mockGetActiveWorkflowContext(workflowId),
83+
}))
84+
8085
vi.mock('@/app/api/workflows/middleware', () => ({
8186
validateWorkflowAccess: (...args: unknown[]) => mockValidateWorkflowAccess(...args),
8287
}))
@@ -113,6 +118,7 @@ describe('Workflow By ID API Route', () => {
113118
})
114119

115120
mockLoadWorkflowFromNormalizedTables.mockResolvedValue(null)
121+
mockGetActiveWorkflowContext.mockResolvedValue(null)
116122
})
117123

118124
afterEach(() => {
@@ -201,7 +207,10 @@ describe('Workflow By ID API Route', () => {
201207
success: true,
202208
authType: 'internal_jwt',
203209
})
204-
mockGetWorkflowById.mockResolvedValue(mockWorkflow)
210+
mockGetActiveWorkflowContext.mockResolvedValue({
211+
workflow: mockWorkflow,
212+
workspaceId: 'workspace-456',
213+
})
205214
mockLoadWorkflowFromNormalizedTables.mockResolvedValue(mockNormalizedData)
206215

207216
const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123', {
@@ -217,9 +226,38 @@ describe('Workflow By ID API Route', () => {
217226
expect(data.data.state.blocks).toEqual(mockNormalizedData.blocks)
218227
expect(data.data.variables).toEqual({})
219228
expect(mockValidateWorkflowAccess).not.toHaveBeenCalled()
229+
expect(mockGetActiveWorkflowContext).toHaveBeenCalledWith('workflow-123')
220230
expect(mockAuthorizeWorkflowByWorkspacePermission).not.toHaveBeenCalled()
221231
})
222232

233+
it('should deny internal compatibility reads for deprecated personal workflows', async () => {
234+
mockCheckHybridAuth.mockResolvedValue({
235+
success: true,
236+
authType: 'internal_jwt',
237+
})
238+
mockGetActiveWorkflowContext.mockResolvedValue(null)
239+
mockGetWorkflowById.mockResolvedValue({
240+
id: 'workflow-123',
241+
userId: 'other-user',
242+
name: 'Personal Workflow',
243+
workspaceId: null,
244+
})
245+
246+
const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123', {
247+
headers: { authorization: 'Bearer internal-token' },
248+
})
249+
const params = Promise.resolve({ id: 'workflow-123' })
250+
251+
const response = await GET(req, { params })
252+
253+
expect(response.status).toBe(403)
254+
expect(await response.json()).toEqual({
255+
error:
256+
'This workflow is not attached to a workspace. Personal workflows are deprecated and cannot be accessed.',
257+
})
258+
expect(mockValidateWorkflowAccess).not.toHaveBeenCalled()
259+
})
260+
223261
it('should deny personal api key reads when middleware rejects workspace policy', async () => {
224262
mockCheckHybridAuth.mockResolvedValue({
225263
success: true,

apps/sim/app/api/workflows/[id]/route.ts

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { getAuditActorMetadata } from '@/lib/audit/actor-metadata'
88
import { AuditAction, AuditResourceType, recordAudit } from '@/lib/audit/log'
99
import { AuthType, checkHybridAuth } from '@/lib/auth/hybrid'
1010
import { generateRequestId } from '@/lib/core/utils/request'
11+
import { getActiveWorkflowContext } from '@/lib/workflows/active-context'
1112
import { archiveWorkflow } from '@/lib/workflows/lifecycle'
1213
import { loadWorkflowFromNormalizedTables } from '@/lib/workflows/persistence/utils'
1314
import { getWorkflowById } from '@/lib/workflows/utils'
@@ -23,6 +24,40 @@ const UpdateWorkflowSchema = z.object({
2324
sortOrder: z.number().int().min(0).optional(),
2425
})
2526

27+
async function getVisibleWorkflowForInternalCompatibility(workflowId: string) {
28+
const context = await getActiveWorkflowContext(workflowId)
29+
if (context) {
30+
return { workflow: context.workflow }
31+
}
32+
33+
const workflow = await getWorkflowById(workflowId, { includeArchived: true })
34+
if (!workflow) {
35+
return {
36+
error: {
37+
message: 'Workflow not found',
38+
status: 404,
39+
},
40+
}
41+
}
42+
43+
if (!workflow.workspaceId) {
44+
return {
45+
error: {
46+
message:
47+
'This workflow is not attached to a workspace. Personal workflows are deprecated and cannot be accessed.',
48+
status: 403,
49+
},
50+
}
51+
}
52+
53+
return {
54+
error: {
55+
message: 'Workflow not found',
56+
status: 404,
57+
},
58+
}
59+
}
60+
2661
/**
2762
* GET /api/workflows/[id]
2863
* Fetch a single workflow by ID
@@ -45,13 +80,18 @@ export async function GET(request: NextRequest, { params }: { params: Promise<{
4580
let workflowData
4681

4782
if (internalNoUserPrecheck) {
48-
workflowData = await getWorkflowById(workflowId)
49-
50-
if (!workflowData) {
51-
logger.warn(`[${requestId}] Workflow ${workflowId} not found`)
52-
return NextResponse.json({ error: 'Workflow not found' }, { status: 404 })
83+
const workflowResult = await getVisibleWorkflowForInternalCompatibility(workflowId)
84+
if (workflowResult.error || !workflowResult.workflow) {
85+
logger.warn(
86+
`[${requestId}] Workflow ${workflowId} not available for internal compatibility read`
87+
)
88+
return NextResponse.json(
89+
{ error: workflowResult.error?.message || 'Workflow not found' },
90+
{ status: workflowResult.error?.status || 404 }
91+
)
5392
}
5493

94+
workflowData = workflowResult.workflow
5595
logger.info(`[${requestId}] Internal bearer compatibility read for workflow ${workflowId}`)
5696
} else {
5797
const validation = await validateWorkflowAccess(request, workflowId, {

0 commit comments

Comments
 (0)