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 numberDiff line numberDiff 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 DownExpand 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 numberDiff line numberDiff 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 DownExpand 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 DownExpand 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 numberDiff line numberDiff line change
Expand Up@@ -7,6 +7,7 @@ import {
hybridAuthMockFns,
permissionsMock,
permissionsMockFns,
workflowAuthzMockFns,
workflowsApiUtilsMock,
workflowsPersistenceUtilsMock,
workflowsPersistenceUtilsMockFns,
Expand DownExpand 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 numberDiff line numberDiff 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 DownExpand 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 DownExpand 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 numberDiff line numberDiff 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 DownExpand 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 DownExpand 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 numberDiff line numberDiff 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 DownExpand 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 DownExpand 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 DownExpand Up@@ -512,6 +515,7 @@ export async function executeSetGlobalWorkflowVariables(
context.userId,
'write'
)
await assertWorkflowMutable(workflowId)

interface WorkflowVariable {
id: string
Expand DownExpand 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 DownExpand 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 DownExpand 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 DownExpand 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 DownExpand Up@@ -984,6 +994,7 @@ export async function executeSetBlockEnabled(
context.userId,
'write'
)
await assertWorkflowMutable(workflowId)
assertWorkflowMutationNotAborted(context)

const normalized = await loadWorkflowFromNormalizedTables(workflowId)
Expand DownExpand 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 DownExpand 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 DownExpand 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 numberDiff line numberDiff 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 DownExpand 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