fix(workflows): harden lock enforcement and clean up dead code

waleedlatif1 · claude · waleedlatif1 · commit 03b5671c83e7 · 2026-04-09T12:04:11.000-07:00
- Fix infinite loop in pure isFolderEffectivelyLocked on circular parentId chains
- Add lock checks to create/duplicate routes (block writes into locked folders)
- Use isWorkflowEffectivelyLockedDb instead of inline checks in state/workflow routes
- Remove use-effective-lock.ts re-export file; import from @/lib/workflows/lock directly
- Remove dead import and re-export from lock-db.ts
- Use LockableFolder type in reorder routes
- Add test for all-unlocked circular chain edge case

Co-Authored-By: Claude Opus 4.6 &lt;noreply@anthropic.com&gt;
diff --git a/apps/sim/app/api/folders/[id]/duplicate/route.ts b/apps/sim/app/api/folders/[id]/duplicate/route.ts
@@ -8,6 +8,7 @@ import { AuditAction, AuditResourceType, recordAudit } from '@/lib/audit/log'
 import { getSession } from '@/lib/auth'
 import { generateRequestId } from '@/lib/core/utils/request'
 import { generateId } from '@/lib/core/utils/uuid'
+import { isFolderEffectivelyLockedDb } from '@/lib/workflows/lock-db'
 import { duplicateWorkflow } from '@/lib/workflows/persistence/duplicate'
 import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils'
 
@@ -66,11 +67,21 @@ export async function POST(req: NextRequest, { params }: { params: Promise<{ id:
     }
 
     const targetWorkspaceId = workspaceId || sourceFolder.workspaceId
+    const targetParentId = parentId ?? sourceFolder.parentId
+
+    if (targetParentId) {
+      const parentLocked = await isFolderEffectivelyLockedDb(targetParentId)
+      if (parentLocked) {
+        return NextResponse.json(
+          { error: 'Cannot duplicate a folder into a locked folder' },
+          { status: 403 }
+        )
+      }
+    }
 
     const { newFolderId, folderMapping } = await db.transaction(async (tx) => {
       const newFolderId = clientNewId || generateId()
       const now = new Date()
-      const targetParentId = parentId ?? sourceFolder.parentId
 
       const folderParentCondition = targetParentId
         ? eq(workflowFolder.parentId, targetParentId)
diff --git a/apps/sim/app/api/folders/reorder/route.ts b/apps/sim/app/api/folders/reorder/route.ts
@@ -6,7 +6,7 @@ import { type NextRequest, NextResponse } from 'next/server'
 import { z } from 'zod'
 import { getSession } from '@/lib/auth'
 import { generateRequestId } from '@/lib/core/utils/request'
-import { isFolderEffectivelyLocked } from '@/lib/workflows/lock'
+import { isFolderEffectivelyLocked, type LockableFolder } from '@/lib/workflows/lock'
 import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils'
 
 const logger = createLogger('FolderReorderAPI')
@@ -74,7 +74,7 @@ export async function PUT(req: NextRequest) {
       .from(workflowFolder)
       .where(eq(workflowFolder.workspaceId, workspaceId))
 
-    const folderMap: Record<string, { id: string; parentId: string | null; isLocked: boolean }> = {}
+    const folderMap: Record<string, LockableFolder> = {}
     for (const f of allFolders) {
       folderMap[f.id] = f
     }
diff --git a/apps/sim/app/api/folders/route.ts b/apps/sim/app/api/folders/route.ts
@@ -8,6 +8,7 @@ import { AuditAction, AuditResourceType, recordAudit } from '@/lib/audit/log'
 import { getSession } from '@/lib/auth'
 import { generateId } from '@/lib/core/utils/uuid'
 import { captureServerEvent } from '@/lib/posthog/server'
+import { isFolderEffectivelyLockedDb } from '@/lib/workflows/lock-db'
 import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils'
 
 const logger = createLogger('FoldersAPI')
@@ -97,6 +98,16 @@ export async function POST(request: NextRequest) {
       )
     }
 
+    if (parentId) {
+      const parentLocked = await isFolderEffectivelyLockedDb(parentId)
+      if (parentLocked) {
+        return NextResponse.json(
+          { error: 'Cannot create a folder inside a locked folder' },
+          { status: 403 }
+        )
+      }
+    }
+
     const id = clientId || generateId()
 
     const newFolder = await db.transaction(async (tx) => {
diff --git a/apps/sim/app/api/workflows/[id]/duplicate/route.ts b/apps/sim/app/api/workflows/[id]/duplicate/route.ts
@@ -6,6 +6,7 @@ import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
 import { PlatformEvents } from '@/lib/core/telemetry'
 import { generateRequestId } from '@/lib/core/utils/request'
 import { captureServerEvent } from '@/lib/posthog/server'
+import { isFolderEffectivelyLockedDb } from '@/lib/workflows/lock-db'
 import { duplicateWorkflow } from '@/lib/workflows/persistence/duplicate'
 
 const logger = createLogger('WorkflowDuplicateAPI')
@@ -37,6 +38,16 @@ export async function POST(req: NextRequest, { params }: { params: Promise<{ id:
     const { name, description, color, workspaceId, folderId, newId } =
       DuplicateRequestSchema.parse(body)
 
+    if (folderId) {
+      const folderLocked = await isFolderEffectivelyLockedDb(folderId)
+      if (folderLocked) {
+        return NextResponse.json(
+          { error: 'Cannot duplicate a workflow into a locked folder' },
+          { status: 403 }
+        )
+      }
+    }
+
     logger.info(`[${requestId}] Duplicating workflow ${sourceWorkflowId} for user ${userId}`)
 
     const result = await duplicateWorkflow({
diff --git a/apps/sim/app/api/workflows/[id]/route.ts b/apps/sim/app/api/workflows/[id]/route.ts
@@ -7,7 +7,7 @@ import { z } from 'zod'
 import { AuthType, checkHybridAuth, checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
 import { generateRequestId } from '@/lib/core/utils/request'
 import { captureServerEvent } from '@/lib/posthog/server'
-import { isFolderEffectivelyLockedDb } from '@/lib/workflows/lock-db'
+import { isWorkflowEffectivelyLockedDb } from '@/lib/workflows/lock-db'
 import { performDeleteWorkflow } from '@/lib/workflows/orchestration'
 import { loadWorkflowFromNormalizedTables } from '@/lib/workflows/persistence/utils'
 import { authorizeWorkflowByWorkspacePermission, getWorkflowById } from '@/lib/workflows/utils'
@@ -184,10 +184,7 @@ export async function DELETE(
       )
     }
 
-    const isLocked =
-      workflowData.isLocked ||
-      (workflowData.folderId ? await isFolderEffectivelyLockedDb(workflowData.folderId) : false)
-    if (isLocked) {
+    if (await isWorkflowEffectivelyLockedDb(workflowId)) {
       return NextResponse.json({ error: 'Workflow is locked' }, { status: 403 })
     }
 
@@ -313,9 +310,7 @@ export async function PUT(request: NextRequest, { params }: { params: Promise<{
     }
 
     // If workflow is effectively locked, only allow isLocked toggle (by admins)
-    const effectivelyLocked =
-      workflowData.isLocked ||
-      (workflowData.folderId ? await isFolderEffectivelyLockedDb(workflowData.folderId) : false)
+    const effectivelyLocked = await isWorkflowEffectivelyLockedDb(workflowId)
     if (effectivelyLocked) {
       const hasNonLockUpdates =
         updates.name !== undefined ||
diff --git a/apps/sim/app/api/workflows/[id]/state/route.ts b/apps/sim/app/api/workflows/[id]/state/route.ts
@@ -7,12 +7,12 @@ import { z } from 'zod'
 import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
 import { env } from '@/lib/core/config/env'
 import { generateRequestId } from '@/lib/core/utils/request'
+import { isWorkflowEffectivelyLockedDb } from '@/lib/workflows/lock-db'
 import { extractAndPersistCustomTools } from '@/lib/workflows/persistence/custom-tools-persistence'
 import {
   loadWorkflowFromNormalizedTables,
   saveWorkflowToNormalizedTables,
 } from '@/lib/workflows/persistence/utils'
-import { isFolderEffectivelyLockedDb } from '@/lib/workflows/lock-db'
 import { sanitizeAgentToolsInBlocks } from '@/lib/workflows/sanitization/validation'
 import { authorizeWorkflowByWorkspacePermission } from '@/lib/workflows/utils'
 import { validateEdges } from '@/stores/workflows/workflow/edge-validation'
@@ -200,11 +200,7 @@ export async function PUT(request: NextRequest, { params }: { params: Promise<{
       )
     }
 
-    // Check if workflow is effectively locked (directly or via folder cascade)
-    const isLocked =
-      workflowData.isLocked ||
-      (workflowData.folderId ? await isFolderEffectivelyLockedDb(workflowData.folderId) : false)
-    if (isLocked) {
+    if (await isWorkflowEffectivelyLockedDb(workflowId)) {
       return NextResponse.json({ error: 'Workflow is locked' }, { status: 403 })
     }
 
diff --git a/apps/sim/app/api/workflows/[id]/variables/route.ts b/apps/sim/app/api/workflows/[id]/variables/route.ts
@@ -7,6 +7,7 @@ import { z } from 'zod'
 import { AuditAction, AuditResourceType, recordAudit } from '@/lib/audit/log'
 import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
 import { generateRequestId } from '@/lib/core/utils/request'
+import { isWorkflowEffectivelyLockedDb } from '@/lib/workflows/lock-db'
 import { authorizeWorkflowByWorkspacePermission } from '@/lib/workflows/utils'
 import type { Variable } from '@/stores/variables/types'
 
@@ -65,6 +66,10 @@ export async function POST(req: NextRequest, { params }: { params: Promise<{ id:
       )
     }
 
+    if (await isWorkflowEffectivelyLockedDb(workflowId)) {
+      return NextResponse.json({ error: 'Workflow is locked' }, { status: 403 })
+    }
+
     const body = await req.json()
 
     try {
diff --git a/apps/sim/app/api/workflows/reorder/route.ts b/apps/sim/app/api/workflows/reorder/route.ts
@@ -6,7 +6,11 @@ import { type NextRequest, NextResponse } from 'next/server'
 import { z } from 'zod'
 import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
 import { generateRequestId } from '@/lib/core/utils/request'
-import { isFolderEffectivelyLocked, isWorkflowEffectivelyLocked } from '@/lib/workflows/lock'
+import {
+  isFolderEffectivelyLocked,
+  isWorkflowEffectivelyLocked,
+  type LockableFolder,
+} from '@/lib/workflows/lock'
 import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils'
 
 const logger = createLogger('WorkflowReorderAPI')
@@ -74,7 +78,7 @@ export async function PUT(req: NextRequest) {
       .from(workflowFolder)
       .where(eq(workflowFolder.workspaceId, workspaceId))
 
-    const folderMap: Record<string, { id: string; parentId: string | null; isLocked: boolean }> = {}
+    const folderMap: Record<string, LockableFolder> = {}
     for (const f of allFolders) {
       folderMap[f.id] = f
     }
diff --git a/apps/sim/app/api/workflows/route.ts b/apps/sim/app/api/workflows/route.ts
@@ -11,6 +11,7 @@ import { generateId } from '@/lib/core/utils/uuid'
 import { captureServerEvent } from '@/lib/posthog/server'
 import { getNextWorkflowColor } from '@/lib/workflows/colors'
 import { buildDefaultWorkflowArtifacts } from '@/lib/workflows/defaults'
+import { isFolderEffectivelyLockedDb } from '@/lib/workflows/lock-db'
 import { saveWorkflowToNormalizedTables } from '@/lib/workflows/persistence/utils'
 import { deduplicateWorkflowName, listWorkflows, type WorkflowScope } from '@/lib/workflows/utils'
 import { getUserEntityPermissions, workspaceExists } from '@/lib/workspaces/permissions/utils'
@@ -163,6 +164,16 @@ export async function POST(req: NextRequest) {
       )
     }
 
+    if (folderId) {
+      const folderLocked = await isFolderEffectivelyLockedDb(folderId)
+      if (folderLocked) {
+        return NextResponse.json(
+          { error: 'Cannot create a workflow inside a locked folder' },
+          { status: 403 }
+        )
+      }
+    }
+
     const workflowId = clientId || generateId()
     const now = new Date()
 
diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx
@@ -23,6 +23,7 @@ import { generateId } from '@/lib/core/utils/uuid'
 import { consumeOAuthReturnContext, writeOAuthReturnContext } from '@/lib/credentials/client-state'
 import type { OAuthProvider } from '@/lib/oauth'
 import { BLOCK_DIMENSIONS, CONTAINER_DIMENSIONS } from '@/lib/workflows/blocks/block-dimensions'
+import { isWorkflowEffectivelyLocked } from '@/lib/workflows/lock'
 import { TriggerUtils } from '@/lib/workflows/triggers/triggers'
 import { OAuthModal } from '@/app/workspace/[workspaceId]/components/oauth-modal'
 import { useWorkspacePermissionsContext } from '@/app/workspace/[workspaceId]/providers/workspace-permissions-provider'
@@ -79,7 +80,6 @@ import { useAutoConnect, useSnapToGridSize } from '@/hooks/queries/general-setti
 import { useWorkflowMap } from '@/hooks/queries/workflows'
 import { useCanvasViewport } from '@/hooks/use-canvas-viewport'
 import { useCollaborativeWorkflow } from '@/hooks/use-collaborative-workflow'
-import { isWorkflowEffectivelyLocked } from '@/hooks/use-effective-lock'
 import { useOAuthReturnForWorkflow } from '@/hooks/use-oauth-return'
 import { useCanvasModeStore } from '@/stores/canvas-mode'
 import { useChatStore } from '@/stores/chat/store'
diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/folder-item/folder-item.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/folder-item/folder-item.tsx
@@ -8,6 +8,7 @@ import { useParams, useRouter } from 'next/navigation'
 import { SIM_RESOURCES_DRAG_TYPE } from '@/lib/copilot/resource-types'
 import { generateId } from '@/lib/core/utils/uuid'
 import { getNextWorkflowColor } from '@/lib/workflows/colors'
+import { isFolderEffectivelyLocked } from '@/lib/workflows/lock'
 import { useUserPermissionsContext } from '@/app/workspace/[workspaceId]/providers/workspace-permissions-provider'
 import { ContextMenu } from '@/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/context-menu/context-menu'
 import { DeleteModal } from '@/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/delete-modal/delete-modal'
@@ -36,7 +37,6 @@ import { useCreateFolder, useFolderMap, useUpdateFolder } from '@/hooks/queries/
 import { getFolderMap } from '@/hooks/queries/utils/folder-cache'
 import { getWorkflows } from '@/hooks/queries/utils/workflow-cache'
 import { useCreateWorkflow } from '@/hooks/queries/workflows'
-import { isFolderEffectivelyLocked } from '@/hooks/use-effective-lock'
 import { useFolderStore } from '@/stores/folders/store'
 import type { FolderTreeNode } from '@/stores/folders/types'
 import { useWorkflowRegistry } from '@/stores/workflows/registry/store'
diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/workflow-item/workflow-item.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/workflow-item/workflow-item.tsx
@@ -6,6 +6,7 @@ import { Lock, MoreHorizontal } from 'lucide-react'
 import Link from 'next/link'
 import { useParams } from 'next/navigation'
 import { SIM_RESOURCES_DRAG_TYPE } from '@/lib/copilot/resource-types'
+import { isWorkflowEffectivelyLocked } from '@/lib/workflows/lock'
 import { workflowBorderColor } from '@/lib/workspaces/colors'
 import { useUserPermissionsContext } from '@/app/workspace/[workspaceId]/providers/workspace-permissions-provider'
 import { ContextMenu } from '@/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/context-menu/context-menu'
@@ -34,7 +35,6 @@ import { useFolderMap } from '@/hooks/queries/folders'
 import { getFolderMap } from '@/hooks/queries/utils/folder-cache'
 import { getWorkflows } from '@/hooks/queries/utils/workflow-cache'
 import { useUpdateWorkflow } from '@/hooks/queries/workflows'
-import { isWorkflowEffectivelyLocked } from '@/hooks/use-effective-lock'
 import { useFolderStore } from '@/stores/folders/store'
 import type { WorkflowMetadata } from '@/stores/workflows/registry/types'
 
diff --git a/apps/sim/hooks/use-effective-lock.ts b/apps/sim/hooks/use-effective-lock.ts
diff --git a/apps/sim/lib/workflows/lock-db.ts b/apps/sim/lib/workflows/lock-db.ts
@@ -1,7 +1,6 @@
 import { db } from '@sim/db'
-import { workflow as workflowTable, workflowFolder } from '@sim/db/schema'
+import { workflowFolder, workflow as workflowTable } from '@sim/db/schema'
 import { eq } from 'drizzle-orm'
-import { isFolderEffectivelyLocked } from '@/lib/workflows/lock'
 
 /**
  * DB-backed cascade lock check for folders.
@@ -52,5 +51,3 @@ export async function isWorkflowEffectivelyLockedDb(workflowId: string): Promise
   if (wf.folderId) return isFolderEffectivelyLockedDb(wf.folderId)
   return false
 }
-
-export { isFolderEffectivelyLocked, isWorkflowEffectivelyLocked } from '@/lib/workflows/lock'
diff --git a/apps/sim/lib/workflows/lock.test.ts b/apps/sim/lib/workflows/lock.test.ts
@@ -63,6 +63,11 @@ describe('isFolderEffectivelyLocked', () => {
     const map = buildMap(folder('a', 'b', true), folder('b', 'a', false))
     expect(isFolderEffectivelyLocked('b', map)).toBe(true)
   })
+
+  it('terminates on circular chain with no locks', () => {
+    const map = buildMap(folder('a', 'b', false), folder('b', 'a', false))
+    expect(isFolderEffectivelyLocked('a', map)).toBe(false)
+  })
 })
 
 describe('isWorkflowEffectivelyLocked', () => {
diff --git a/apps/sim/lib/workflows/lock.ts b/apps/sim/lib/workflows/lock.ts
@@ -17,8 +17,11 @@ export function isFolderEffectivelyLocked(
   folderId: string,
   folderMap: Record<string, LockableFolder>
 ): boolean {
+  const visited = new Set<string>()
   let current: LockableFolder | undefined = folderMap[folderId]
   while (current) {
+    if (visited.has(current.id)) return false
+    visited.add(current.id)
     if (current.isLocked) return true
     current = current.parentId ? folderMap[current.parentId] : undefined
   }
diff --git a/packages/db/migrations/meta/0189_snapshot.json b/packages/db/migrations/meta/0189_snapshot.json
diff --git a/packages/db/migrations/meta/_journal.json b/packages/db/migrations/meta/_journal.json
