Skip to content

Commit 1aec450

Browse files
committed
improvement(forking): fork time ux
1 parent af87de0 commit 1aec450

42 files changed

Lines changed: 4112 additions & 403 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,202 @@
1+
/**
2+
* Tests for the workflow deployed-state API route.
3+
* Covers internal-JWT authorization (acting user required + workspace read
4+
* permission) and the unchanged session path.
5+
*
6+
* @vitest-environment node
7+
*/
8+
9+
import {
10+
workflowAuthzMockFns,
11+
workflowsPersistenceUtilsMock,
12+
workflowsPersistenceUtilsMockFns,
13+
workflowsUtilsMock,
14+
workflowsUtilsMockFns,
15+
} from '@sim/testing'
16+
import { NextRequest } from 'next/server'
17+
import { beforeEach, describe, expect, it, vi } from 'vitest'
18+
19+
const { mockVerifyInternalToken } = vi.hoisted(() => ({
20+
mockVerifyInternalToken: vi.fn(),
21+
}))
22+
23+
vi.mock('@/lib/auth/internal', () => ({
24+
verifyInternalToken: mockVerifyInternalToken,
25+
}))
26+
27+
vi.mock('@/lib/workflows/persistence/utils', () => workflowsPersistenceUtilsMock)
28+
29+
vi.mock('@/lib/workflows/utils', () => workflowsUtilsMock)
30+
31+
import { GET } from './route'
32+
33+
const mockAuthorizeWorkflowByWorkspacePermission =
34+
workflowAuthzMockFns.mockAuthorizeWorkflowByWorkspacePermission
35+
const mockLoadDeployedWorkflowState = workflowsPersistenceUtilsMockFns.mockLoadDeployedWorkflowState
36+
const mockValidateWorkflowPermissions = workflowsUtilsMockFns.mockValidateWorkflowPermissions
37+
38+
const DEPLOYED_STATE = {
39+
blocks: { 'block-1': { id: 'block-1', type: 'starter' } },
40+
edges: [],
41+
loops: {},
42+
parallels: {},
43+
variables: {},
44+
}
45+
46+
function createRequest(options?: { bearerToken?: string }) {
47+
const headers: Record<string, string> = {}
48+
if (options?.bearerToken) {
49+
headers.Authorization = `Bearer ${options.bearerToken}`
50+
}
51+
return new NextRequest('http://localhost:3000/api/workflows/workflow-123/deployed', { headers })
52+
}
53+
54+
const routeParams = () => ({ params: Promise.resolve({ id: 'workflow-123' }) })
55+
56+
describe('GET /api/workflows/[id]/deployed', () => {
57+
beforeEach(() => {
58+
vi.clearAllMocks()
59+
mockVerifyInternalToken.mockResolvedValue({ valid: false })
60+
mockLoadDeployedWorkflowState.mockResolvedValue(DEPLOYED_STATE)
61+
})
62+
63+
describe('internal JWT path', () => {
64+
it('returns 200 when the token carries a user with read permission', async () => {
65+
mockVerifyInternalToken.mockResolvedValue({ valid: true, userId: 'user-123' })
66+
mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({
67+
allowed: true,
68+
status: 200,
69+
workflow: { id: 'workflow-123', workspaceId: 'workspace-456' },
70+
workspacePermission: 'read',
71+
})
72+
73+
const response = await GET(createRequest({ bearerToken: 'internal-token' }), routeParams())
74+
75+
expect(response.status).toBe(200)
76+
const data = await response.json()
77+
expect(data.deployedState).toEqual(DEPLOYED_STATE)
78+
expect(mockAuthorizeWorkflowByWorkspacePermission).toHaveBeenCalledWith({
79+
workflowId: 'workflow-123',
80+
userId: 'user-123',
81+
action: 'read',
82+
})
83+
expect(mockValidateWorkflowPermissions).not.toHaveBeenCalled()
84+
})
85+
86+
it('returns 403 when the acting user lacks read permission', async () => {
87+
mockVerifyInternalToken.mockResolvedValue({ valid: true, userId: 'user-123' })
88+
mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({
89+
allowed: false,
90+
status: 403,
91+
message: 'Unauthorized: Access denied to read this workflow',
92+
workflow: { id: 'workflow-123', workspaceId: 'workspace-456' },
93+
workspacePermission: null,
94+
})
95+
96+
const response = await GET(createRequest({ bearerToken: 'internal-token' }), routeParams())
97+
98+
expect(response.status).toBe(403)
99+
const data = await response.json()
100+
expect(data.error).toBe('Unauthorized: Access denied to read this workflow')
101+
expect(mockLoadDeployedWorkflowState).not.toHaveBeenCalled()
102+
})
103+
104+
it('returns 403 when the token carries no acting user (fail closed)', async () => {
105+
mockVerifyInternalToken.mockResolvedValue({ valid: true, userId: undefined })
106+
107+
const response = await GET(createRequest({ bearerToken: 'internal-token' }), routeParams())
108+
109+
expect(response.status).toBe(403)
110+
const data = await response.json()
111+
expect(data.error).toBe('Forbidden')
112+
expect(mockAuthorizeWorkflowByWorkspacePermission).not.toHaveBeenCalled()
113+
expect(mockLoadDeployedWorkflowState).not.toHaveBeenCalled()
114+
})
115+
116+
it('returns 404 when the workflow does not exist', async () => {
117+
mockVerifyInternalToken.mockResolvedValue({ valid: true, userId: 'user-123' })
118+
mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({
119+
allowed: false,
120+
status: 404,
121+
message: 'Workflow not found',
122+
workflow: null,
123+
workspacePermission: null,
124+
})
125+
126+
const response = await GET(createRequest({ bearerToken: 'internal-token' }), routeParams())
127+
128+
expect(response.status).toBe(404)
129+
const data = await response.json()
130+
expect(data.error).toBe('Workflow not found')
131+
expect(mockLoadDeployedWorkflowState).not.toHaveBeenCalled()
132+
})
133+
})
134+
135+
describe('session path', () => {
136+
it('returns 200 when session permissions validate', async () => {
137+
mockValidateWorkflowPermissions.mockResolvedValue({
138+
error: null,
139+
session: { user: { id: 'user-123' } },
140+
workflow: { id: 'workflow-123' },
141+
})
142+
143+
const response = await GET(createRequest(), routeParams())
144+
145+
expect(response.status).toBe(200)
146+
const data = await response.json()
147+
expect(data.deployedState).toEqual(DEPLOYED_STATE)
148+
expect(mockValidateWorkflowPermissions).toHaveBeenCalledWith(
149+
'workflow-123',
150+
expect.any(String),
151+
'read'
152+
)
153+
expect(mockAuthorizeWorkflowByWorkspacePermission).not.toHaveBeenCalled()
154+
})
155+
156+
it('propagates validateWorkflowPermissions errors unchanged', async () => {
157+
mockValidateWorkflowPermissions.mockResolvedValue({
158+
error: { message: 'Unauthorized', status: 401 },
159+
session: null,
160+
workflow: null,
161+
})
162+
163+
const response = await GET(createRequest(), routeParams())
164+
165+
expect(response.status).toBe(401)
166+
const data = await response.json()
167+
expect(data.error).toBe('Unauthorized')
168+
})
169+
170+
it('falls back to session validation when the bearer token is not a valid internal token', async () => {
171+
mockVerifyInternalToken.mockResolvedValue({ valid: false })
172+
mockValidateWorkflowPermissions.mockResolvedValue({
173+
error: { message: 'Unauthorized', status: 401 },
174+
session: null,
175+
workflow: null,
176+
})
177+
178+
const response = await GET(createRequest({ bearerToken: 'not-internal' }), routeParams())
179+
180+
expect(response.status).toBe(401)
181+
expect(mockValidateWorkflowPermissions).toHaveBeenCalled()
182+
expect(mockAuthorizeWorkflowByWorkspacePermission).not.toHaveBeenCalled()
183+
})
184+
})
185+
186+
it('returns null deployedState when loading the snapshot fails', async () => {
187+
mockVerifyInternalToken.mockResolvedValue({ valid: true, userId: 'user-123' })
188+
mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({
189+
allowed: true,
190+
status: 200,
191+
workflow: { id: 'workflow-123', workspaceId: 'workspace-456' },
192+
workspacePermission: 'admin',
193+
})
194+
mockLoadDeployedWorkflowState.mockRejectedValue(new Error('no active deployment'))
195+
196+
const response = await GET(createRequest({ bearerToken: 'internal-token' }), routeParams())
197+
198+
expect(response.status).toBe(200)
199+
const data = await response.json()
200+
expect(data.deployedState).toBeNull()
201+
})
202+
})

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

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { createLogger } from '@sim/logger'
2+
import { authorizeWorkflowByWorkspacePermission } from '@sim/platform-authz/workflow'
23
import type { NextRequest, NextResponse } from 'next/server'
34
import { getDeployedWorkflowStateContract } from '@/lib/api/contracts/deployments'
45
import { parseRequest } from '@/lib/api/server'
@@ -19,6 +20,18 @@ function addNoCacheHeaders(response: NextResponse): NextResponse {
1920
return response
2021
}
2122

23+
/**
24+
* GET /api/workflows/[id]/deployed
25+
* Returns the active deployed state snapshot for a workflow.
26+
*
27+
* Internal (server-to-server) calls must carry the acting user in the internal
28+
* JWT payload (`generateInternalToken(userId)` — the executor's
29+
* `buildAuthHeaders(ctx.userId)` always embeds it) and are authorized as that
30+
* user with the same workspace-read semantics as the sibling
31+
* `/api/workflows/[id]` route. Internal calls without a user id are rejected
32+
* (fail closed). Session calls are authorized via
33+
* `validateWorkflowPermissions` as before.
34+
*/
2235
export const GET = withRouteHandler(
2336
async (request: NextRequest, context: { params: Promise<{ id: string }> }) => {
2437
const requestId = generateRequestId()
@@ -29,14 +42,39 @@ export const GET = withRouteHandler(
2942
try {
3043
const authHeader = request.headers.get('authorization')
3144
let isInternalCall = false
45+
let internalCallUserId: string | undefined
3246

3347
if (authHeader?.startsWith('Bearer ')) {
3448
const token = authHeader.split(' ')[1]
3549
const verification = await verifyInternalToken(token)
3650
isInternalCall = verification.valid
51+
internalCallUserId = verification.userId
3752
}
3853

39-
if (!isInternalCall) {
54+
if (isInternalCall) {
55+
if (!internalCallUserId) {
56+
logger.warn(`[${requestId}] Internal call without acting user denied for workflow ${id}`)
57+
return addNoCacheHeaders(createErrorResponse('Forbidden', 403))
58+
}
59+
60+
const authorization = await authorizeWorkflowByWorkspacePermission({
61+
workflowId: id,
62+
userId: internalCallUserId,
63+
action: 'read',
64+
})
65+
if (!authorization.workflow) {
66+
logger.warn(`[${requestId}] Workflow ${id} not found for internal call`)
67+
return addNoCacheHeaders(createErrorResponse('Workflow not found', 404))
68+
}
69+
if (!authorization.allowed) {
70+
logger.warn(
71+
`[${requestId}] Internal call user ${internalCallUserId} denied read access to workflow ${id}`
72+
)
73+
return addNoCacheHeaders(
74+
createErrorResponse(authorization.message || 'Access denied', authorization.status)
75+
)
76+
}
77+
} else {
4078
const { error } = await validateWorkflowPermissions(id, requestId, 'read')
4179
if (error) {
4280
const response = createErrorResponse(error.message, error.status)

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -527,6 +527,7 @@ async function handleExecutePost(
527527
startBlockId,
528528
stopAfterBlockId,
529529
runFromBlock: rawRunFromBlock,
530+
parentWorkspaceId,
530531
} = validation.data
531532
const triggerBlockId = parsedTriggerBlockId ?? startBlockId
532533

@@ -642,6 +643,7 @@ async function handleExecutePost(
642643
stopAfterBlockId: _stopAfterBlockId,
643644
runFromBlock: _runFromBlock,
644645
workflowId: _workflowId, // Also exclude workflowId used for internal JWT auth
646+
parentWorkspaceId: _parentWorkspaceId,
645647
...rest
646648
} = body
647649
return Object.keys(rest).length > 0 ? rest : validatedInput
@@ -729,6 +731,25 @@ async function handleExecutePost(
729731
)
730732
}
731733

734+
/**
735+
* Workflow-in-workflow invocations (e.g. the agent `workflow_executor`
736+
* tool) declare the parent execution's workspace. Reject execution when
737+
* the target workflow lives in a different workspace so a stale or
738+
* foreign workflow id cannot silently execute with the parent's context.
739+
* The error intentionally omits the target's workspace id.
740+
*/
741+
if (parentWorkspaceId && workflowAuthorization.workflow?.workspaceId !== parentWorkspaceId) {
742+
reqLogger.warn('Blocked cross-workspace child workflow execution', {
743+
parentWorkspaceId,
744+
})
745+
return NextResponse.json(
746+
{
747+
error: `Child workflow ${workflowId} belongs to a different workspace and cannot be executed`,
748+
},
749+
{ status: 403 }
750+
)
751+
}
752+
732753
if (req.signal.aborted) {
733754
return clientCancelledResponse()
734755
}

apps/sim/app/api/workspaces/[id]/fork/diff/route.ts

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,10 @@ import {
1919
loadForkDependentValues,
2020
} from '@/lib/workspaces/fork/mapping/dependent-value-store'
2121
import { listForkResourceCandidates } from '@/lib/workspaces/fork/mapping/resources'
22-
import { collectForkClearedRefCandidates } from '@/lib/workspaces/fork/promote/cleared-refs'
22+
import {
23+
annotateForkClearedRefSourceLiveness,
24+
collectForkClearedRefCandidates,
25+
} from '@/lib/workspaces/fork/promote/cleared-refs'
2326
import { computeForkPromotePlan } from '@/lib/workspaces/fork/promote/promote-plan'
2427
import { buildForkBlockIdResolver } from '@/lib/workspaces/fork/remap/block-identity'
2528
import { readTargetDraftDependentValue } from '@/lib/workspaces/fork/remap/remap-references'
@@ -127,15 +130,21 @@ export const GET = withRouteHandler(
127130
sourceLabels.set(`${kind}:${candidate.id}`, candidate.label)
128131
}
129132
const sourceWorkflowNames = new Map(sourceWorkflowRows.map((row) => [row.id, row.name]))
130-
const clearedRefs = collectForkClearedRefCandidates({
131-
items: plan.items,
132-
sourceStates,
133-
resolver: plan.resolver,
134-
workflowIdMap: plan.workflowIdMap,
135-
resolveBlockId,
136-
sourceLabels,
137-
sourceWorkflowNames,
138-
})
133+
// Annotate each reference-cause entry's source liveness so the client can phrase the blocker
134+
// reason (a deleted source can't be copied - it must be mapped to a live target resource).
135+
const clearedRefs = await annotateForkClearedRefSourceLiveness(
136+
db,
137+
auth.sourceWorkspaceId,
138+
collectForkClearedRefCandidates({
139+
items: plan.items,
140+
sourceStates,
141+
resolver: plan.resolver,
142+
workflowIdMap: plan.workflowIdMap,
143+
resolveBlockId,
144+
sourceLabels,
145+
sourceWorkflowNames,
146+
})
147+
)
139148

140149
const toRef = (reference: (typeof plan.unmappedRequired)[number]) => ({
141150
kind: reference.kind,

apps/sim/app/api/workspaces/[id]/fork/promote/route.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ export const POST = withRouteHandler(
4848
redeployed: result.redeployed,
4949
deployFailed: result.deployFailed,
5050
unmappedRequired: result.unmappedRequired,
51+
blockers: result.blockers,
5152
needsConfiguration: result.needsConfiguration,
5253
clearedOptional: result.clearedOptional,
5354
}

0 commit comments

Comments
 (0)