diff --git a/apps/sim/app/api/folders/route.test.ts b/apps/sim/app/api/folders/route.test.ts index a72c0a7bbf8..baafe6fd2ad 100644 --- a/apps/sim/app/api/folders/route.test.ts +++ b/apps/sim/app/api/folders/route.test.ts @@ -9,6 +9,7 @@ import { createMockRequest, permissionsMock, permissionsMockFns, + workflowAuthzMockFns, } from '@sim/testing' import { drizzleOrmMock } from '@sim/testing/mocks' import { beforeEach, describe, expect, it, vi } from 'vitest' @@ -390,6 +391,26 @@ describe('Folders API Route', () => { }) }) + it('should reject creating a subfolder inside a locked parent folder', async () => { + mockAuthenticatedUser() + + const { FolderLockedError } = await import('@sim/workflow-authz') + workflowAuthzMockFns.mockAssertFolderMutable.mockRejectedValueOnce( + new FolderLockedError('Folder is locked') + ) + + const req = createMockRequest('POST', { + name: 'Subfolder', + workspaceId: 'workspace-123', + parentId: 'locked-folder', + }) + + const response = await POST(req) + + expect(response.status).toBe(423) + expect(mockTransaction).not.toHaveBeenCalled() + }) + it('should reject a parentId that does not resolve to a folder in the workspace', async () => { mockAuthenticatedUser() diff --git a/apps/sim/app/api/folders/route.ts b/apps/sim/app/api/folders/route.ts index 404ebe0873c..f359e376542 100644 --- a/apps/sim/app/api/folders/route.ts +++ b/apps/sim/app/api/folders/route.ts @@ -1,6 +1,7 @@ import { db } from '@sim/db' import { workflowFolder } from '@sim/db/schema' import { createLogger } from '@sim/logger' +import { assertFolderMutable, FolderLockedError } from '@sim/workflow-authz' import { and, asc, eq, isNotNull, isNull } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' import { createFolderContract, listFoldersContract } from '@/lib/api/contracts' @@ -93,6 +94,8 @@ export const POST = withRouteHandler(async (request: NextRequest) => { ) } + await assertFolderMutable(parentId ?? null) + const result = await performCreateFolder({ id: clientId, userId: session.user.id, @@ -123,6 +126,9 @@ export const POST = withRouteHandler(async (request: NextRequest) => { return NextResponse.json({ folder: newFolder }) } catch (error) { + if (error instanceof FolderLockedError) { + return NextResponse.json({ error: error.message }, { status: error.status }) + } logger.error('Error creating folder:', { error }) return NextResponse.json({ error: 'Internal server error' }, { status: 500 }) } diff --git a/apps/sim/app/api/workflows/route.test.ts b/apps/sim/app/api/workflows/route.test.ts index 30e75ad2ed7..2bfddb39343 100644 --- a/apps/sim/app/api/workflows/route.test.ts +++ b/apps/sim/app/api/workflows/route.test.ts @@ -7,6 +7,7 @@ import { hybridAuthMockFns, permissionsMock, permissionsMockFns, + workflowAuthzMockFns, workflowsApiUtilsMock, workflowsPersistenceUtilsMock, workflowsPersistenceUtilsMockFns, @@ -80,11 +81,30 @@ describe('Workflows API Route - POST ordering', () => { userEmail: 'test@example.com', }) mockGetUserEntityPermissions.mockResolvedValue('write') + workflowAuthzMockFns.mockAssertFolderMutable.mockResolvedValue(undefined) workflowsPersistenceUtilsMockFns.mockSaveWorkflowToNormalizedTables.mockResolvedValue({ success: true, }) }) + it('rejects creating a workflow inside a locked folder', async () => { + const { FolderLockedError } = await import('@sim/workflow-authz') + workflowAuthzMockFns.mockAssertFolderMutable.mockRejectedValueOnce( + new FolderLockedError('Folder is locked') + ) + + const req = createMockRequest('POST', { + name: 'New Workflow', + description: 'desc', + workspaceId: 'workspace-123', + folderId: 'locked-folder', + }) + + const response = await POST(req) + expect(response.status).toBe(423) + expect(mockDbInsert).not.toHaveBeenCalled() + }) + it('uses top insertion against mixed siblings (folders + workflows)', async () => { const minResultsQueue: Array> = [ [], diff --git a/apps/sim/app/api/workflows/route.ts b/apps/sim/app/api/workflows/route.ts index 92a17985965..224cb68417d 100644 --- a/apps/sim/app/api/workflows/route.ts +++ b/apps/sim/app/api/workflows/route.ts @@ -1,4 +1,5 @@ import { createLogger } from '@sim/logger' +import { assertFolderMutable, FolderLockedError } from '@sim/workflow-authz' import { type NextRequest, NextResponse } from 'next/server' import { createWorkflowContract, workflowListQuerySchema } from '@/lib/api/contracts/workflows' import { parseRequest } from '@/lib/api/server' @@ -116,6 +117,8 @@ export const POST = withRouteHandler(async (req: NextRequest) => { ) } + await assertFolderMutable(folderId ?? null) + const result = await performCreateWorkflow({ id: clientId, name: requestedName, @@ -180,6 +183,9 @@ export const POST = withRouteHandler(async (req: NextRequest) => { subBlockValues: createdWorkflow.subBlockValues, }) } catch (error) { + if (error instanceof FolderLockedError) { + return NextResponse.json({ error: error.message }, { status: error.status }) + } logger.error(`[${requestId}] Error creating workflow`, error) return NextResponse.json({ error: 'Failed to create workflow' }, { status: 500 }) } diff --git a/apps/sim/lib/copilot/tools/handlers/workflow/mutations.test.ts b/apps/sim/lib/copilot/tools/handlers/workflow/mutations.test.ts index 582c42c6a6b..037894bf30c 100644 --- a/apps/sim/lib/copilot/tools/handlers/workflow/mutations.test.ts +++ b/apps/sim/lib/copilot/tools/handlers/workflow/mutations.test.ts @@ -1,7 +1,7 @@ /** * @vitest-environment node */ -import { createEnvMock } from '@sim/testing' +import { createEnvMock, workflowAuthzMockFns } from '@sim/testing' import { beforeEach, describe, expect, it, vi } from 'vitest' const { @@ -88,7 +88,16 @@ vi.mock('../access', () => ({ getDefaultWorkspaceId: vi.fn(), })) -import { executeRunFromBlock, executeSetGlobalWorkflowVariables } from './mutations' +import { performUpdateWorkflow } from '@/lib/workflows/orchestration' +import { verifyFolderWorkspace } from '@/lib/workflows/utils' +import { + executeMoveWorkflow, + executeRunFromBlock, + executeSetGlobalWorkflowVariables, +} from './mutations' + +const performUpdateWorkflowMock = vi.mocked(performUpdateWorkflow) +const verifyFolderWorkspaceMock = vi.mocked(verifyFolderWorkspace) describe('executeSetGlobalWorkflowVariables', () => { beforeEach(() => { @@ -134,6 +143,56 @@ describe('executeSetGlobalWorkflowVariables', () => { }) }) +describe('lock enforcement', () => { + beforeEach(() => { + vi.clearAllMocks() + global.fetch = vi.fn().mockResolvedValue(new Response(null, { status: 200 })) as typeof fetch + workflowAuthzMockFns.mockAssertWorkflowMutable.mockResolvedValue(undefined) + workflowAuthzMockFns.mockAssertFolderMutable.mockResolvedValue(undefined) + }) + + it('does not persist variable changes when the workflow is locked', async () => { + ensureWorkflowAccessMock.mockResolvedValue({ + workflow: { id: 'workflow-1', variables: {} }, + }) + workflowAuthzMockFns.mockAssertWorkflowMutable.mockRejectedValueOnce( + new Error('Workflow is locked') + ) + + const result = await executeSetGlobalWorkflowVariables( + { + workflowId: 'workflow-1', + operations: [{ operation: 'add', name: 'threshold', type: 'number', value: '5' }], + }, + { userId: 'user-1' } as any + ) + + expect(result.success).toBe(false) + expect(result.error).toBe('Workflow is locked') + expect(setWorkflowVariablesMock).not.toHaveBeenCalled() + }) + + it('does not move a workflow into a locked target folder', async () => { + ensureWorkflowAccessMock.mockResolvedValue({ + workspaceId: 'workspace-1', + workflow: { id: 'workflow-1', name: 'WF', folderId: null }, + }) + verifyFolderWorkspaceMock.mockResolvedValue(true) + workflowAuthzMockFns.mockAssertFolderMutable.mockRejectedValueOnce( + new Error('Folder is locked') + ) + + const result = await executeMoveWorkflow( + { workflowIds: ['workflow-1'], folderId: 'locked-folder' }, + { userId: 'user-1' } as any + ) + + expect(result.success).toBe(false) + expect(result.error).toBe('Folder is locked') + expect(performUpdateWorkflowMock).not.toHaveBeenCalled() + }) +}) + describe('executeRunFromBlock', () => { beforeEach(() => { vi.clearAllMocks() diff --git a/apps/sim/lib/copilot/tools/handlers/workflow/mutations.ts b/apps/sim/lib/copilot/tools/handlers/workflow/mutations.ts index 115f8ff3590..7ae8c2e6a4d 100644 --- a/apps/sim/lib/copilot/tools/handlers/workflow/mutations.ts +++ b/apps/sim/lib/copilot/tools/handlers/workflow/mutations.ts @@ -3,6 +3,7 @@ import { db, workflow as workflowTable } from '@sim/db' import { createLogger } from '@sim/logger' import { toError } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' +import { assertFolderMutable, assertWorkflowMutable } from '@sim/workflow-authz' import { mergeSubblockStateWithValues } from '@sim/workflow-persistence/subblocks' import { eq } from 'drizzle-orm' import { performCreateWorkspaceApiKey } from '@/lib/api-key/orchestration' @@ -353,6 +354,7 @@ export async function executeCreateWorkflow( const folderId = params?.folderId || null await ensureWorkspaceAccess(workspaceId, context.userId, 'write') + await assertFolderMutable(folderId) assertWorkflowMutationNotAborted(context) const result = await performCreateWorkflow({ @@ -422,6 +424,7 @@ export async function executeCreateFolder( const parentId = params?.parentId || null await ensureWorkspaceAccess(workspaceId, context.userId, 'write') + await assertFolderMutable(parentId) assertWorkflowMutationNotAborted(context) const result = await performCreateFolder({ @@ -512,6 +515,7 @@ export async function executeSetGlobalWorkflowVariables( context.userId, 'write' ) + await assertWorkflowMutable(workflowId) interface WorkflowVariable { id: string @@ -633,9 +637,9 @@ export async function executeRenameWorkflow( return { success: false, error: 'Workflow name must be 200 characters or less' } } - await ensureWorkflowAccess(workflowId, context.userId, 'write') - assertWorkflowMutationNotAborted(context) const current = await ensureWorkflowAccess(workflowId, context.userId, 'write') + await assertWorkflowMutable(workflowId) + assertWorkflowMutationNotAborted(context) if (!current.workspaceId) { return { success: false, error: 'Workflow workspace is required' } } @@ -671,6 +675,8 @@ export async function executeMoveWorkflow( const moved: string[] = [] const failed: string[] = [] + await assertFolderMutable(folderId) + for (const workflowId of workflowIds) { try { const { workspaceId, workflow } = await ensureWorkflowAccess( @@ -688,6 +694,7 @@ export async function executeMoveWorkflow( continue } } + await assertWorkflowMutable(workflowId) assertWorkflowMutationNotAborted(context) const result = await performUpdateWorkflow({ workflowId, @@ -735,6 +742,8 @@ export async function executeMoveFolder( return { success: false, error: 'Parent folder not found' } } + await assertFolderMutable(folderId) + await assertFolderMutable(parentId) assertWorkflowMutationNotAborted(context) const result = await performUpdateFolder({ folderId, @@ -941,6 +950,7 @@ async function executeUpdateWorkflow( if (!current.workspaceId) { return { success: false, error: 'Workflow workspace is required' } } + await assertWorkflowMutable(workflowId) assertWorkflowMutationNotAborted(context) const result = await performUpdateWorkflow({ workflowId, @@ -984,6 +994,7 @@ export async function executeSetBlockEnabled( context.userId, 'write' ) + await assertWorkflowMutable(workflowId) assertWorkflowMutationNotAborted(context) const normalized = await loadWorkflowFromNormalizedTables(workflowId) @@ -1114,6 +1125,7 @@ export async function executeDeleteWorkflow( context.userId, 'write' ) + await assertWorkflowMutable(workflowId) assertWorkflowMutationNotAborted(context) const result = await performDeleteWorkflow({ workflowId, userId: context.userId }) @@ -1162,16 +1174,22 @@ export async function executeDeleteFolder( assertWorkflowMutationNotAborted(context) - const result = await performDeleteFolder({ - folderId, - workspaceId, - userId: context.userId, - folderName: folder.folderName, - }) + try { + await assertFolderMutable(folderId) - if (result.success) { - deleted.push(folderId) - } else { + const result = await performDeleteFolder({ + folderId, + workspaceId, + userId: context.userId, + folderName: folder.folderName, + }) + + if (result.success) { + deleted.push(folderId) + } else { + failed.push(folderId) + } + } catch { failed.push(folderId) } } @@ -1206,6 +1224,7 @@ async function executeRenameFolder( return { success: false, error: 'Folder not found' } } + await assertFolderMutable(folderId) assertWorkflowMutationNotAborted(context) const result = await performUpdateFolder({ folderId, diff --git a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/index.ts b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/index.ts index 44bb0ee4a8c..96a4fa98353 100644 --- a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/index.ts +++ b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/index.ts @@ -2,7 +2,7 @@ import { db } from '@sim/db' import { workflow as workflowTable } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { toError } from '@sim/utils/errors' -import { authorizeWorkflowByWorkspacePermission } from '@sim/workflow-authz' +import { assertWorkflowMutable, authorizeWorkflowByWorkspacePermission } from '@sim/workflow-authz' import { eq } from 'drizzle-orm' import { EditWorkflow } from '@/lib/copilot/generated/tool-catalog-v1' import { @@ -106,6 +106,8 @@ export const editWorkflowServerTool: BaseServerTool throw new Error(authorization.message || 'Unauthorized workflow access') } + await assertWorkflowMutable(workflowId) + const workspaceId = authorization.workflow?.workspaceId ?? undefined const workflowName = authorization.workflow?.name ?? undefined