Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions apps/sim/app/api/folders/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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()

Expand Down
6 changes: 6 additions & 0 deletions apps/sim/app/api/folders/route.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 })
}
Expand Down
20 changes: 20 additions & 0 deletions apps/sim/app/api/workflows/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
hybridAuthMockFns,
permissionsMock,
permissionsMockFns,
workflowAuthzMockFns,
workflowsApiUtilsMock,
workflowsPersistenceUtilsMock,
workflowsPersistenceUtilsMockFns,
Expand Down Expand Up @@ -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<Array<{ minOrder: number }>> = [
[],
Expand Down
6 changes: 6 additions & 0 deletions apps/sim/app/api/workflows/route.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -116,6 +117,8 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
)
}

await assertFolderMutable(folderId ?? null)

const result = await performCreateWorkflow({
id: clientId,
name: requestedName,
Expand Down Expand Up @@ -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 })
}
Expand Down
63 changes: 61 additions & 2 deletions apps/sim/lib/copilot/tools/handlers/workflow/mutations.test.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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()
Expand Down
41 changes: 30 additions & 11 deletions apps/sim/lib/copilot/tools/handlers/workflow/mutations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -512,6 +515,7 @@ export async function executeSetGlobalWorkflowVariables(
context.userId,
'write'
)
await assertWorkflowMutable(workflowId)

interface WorkflowVariable {
id: string
Expand Down Expand Up @@ -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' }
}
Expand Down Expand Up @@ -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(
Expand All @@ -688,6 +694,7 @@ export async function executeMoveWorkflow(
continue
}
}
await assertWorkflowMutable(workflowId)
assertWorkflowMutationNotAborted(context)
Comment thread
waleedlatif1 marked this conversation as resolved.
const result = await performUpdateWorkflow({
workflowId,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -984,6 +994,7 @@ export async function executeSetBlockEnabled(
context.userId,
'write'
)
await assertWorkflowMutable(workflowId)
assertWorkflowMutationNotAborted(context)

const normalized = await loadWorkflowFromNormalizedTables(workflowId)
Expand Down Expand Up @@ -1114,6 +1125,7 @@ export async function executeDeleteWorkflow(
context.userId,
'write'
)
await assertWorkflowMutable(workflowId)
assertWorkflowMutationNotAborted(context)

const result = await performDeleteWorkflow({ workflowId, userId: context.userId })
Expand Down Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -1206,6 +1224,7 @@ async function executeRenameFolder(
return { success: false, error: 'Folder not found' }
}

await assertFolderMutable(folderId)
assertWorkflowMutationNotAborted(context)
const result = await performUpdateFolder({
folderId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -106,6 +106,8 @@ export const editWorkflowServerTool: BaseServerTool<EditWorkflowParams, unknown>
throw new Error(authorization.message || 'Unauthorized workflow access')
}

await assertWorkflowMutable(workflowId)

const workspaceId = authorization.workflow?.workspaceId ?? undefined
const workflowName = authorization.workflow?.name ?? undefined

Expand Down
Loading