-
Notifications
You must be signed in to change notification settings - Fork 3.5k
feat(workflows): add isLocked to workflows and folders with cascade lock #4031
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: staging
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,7 @@ const UpdateWorkflowSchema = z.object({ | |
| color: z.string().optional(), | ||
| folderId: z.string().nullable().optional(), | ||
| sortOrder: z.number().int().min(0).optional(), | ||
| isLocked: z.boolean().optional(), | ||
| }) | ||
|
|
||
| /** | ||
|
|
@@ -182,6 +183,10 @@ export async function DELETE( | |
| ) | ||
| } | ||
|
|
||
| if (workflowData.isLocked) { | ||
| return NextResponse.json({ error: 'Workflow is locked' }, { status: 403 }) | ||
| } | ||
|
Comment on lines
+186
to
+188
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The DELETE and PUT handlers only check For example:
The same bypass exists in the PUT handler at line 312. The server-side logic should mirror the client-side The same issue is present in the PUT guard at line 312: if (workflowData.isLocked && updates.isLocked === undefined) {
return NextResponse.json({ error: 'Workflow is locked' }, { status: 403 })
}Both of these should also check whether the workflow's folder (or any ancestor folder) is locked, either by fetching and walking the folder chain server-side or by adding an |
||
|
|
||
| const { searchParams } = new URL(request.url) | ||
| const checkTemplates = searchParams.get('check-templates') === 'true' | ||
| const deleteTemplatesParam = searchParams.get('deleteTemplates') | ||
|
|
@@ -288,12 +293,33 @@ export async function PUT(request: NextRequest, { params }: { params: Promise<{ | |
| ) | ||
| } | ||
|
|
||
| // If toggling isLocked, require admin permission | ||
| if (updates.isLocked !== undefined) { | ||
| const adminAuth = await authorizeWorkflowByWorkspacePermission({ | ||
| workflowId, | ||
| userId, | ||
| action: 'admin', | ||
| }) | ||
| if (!adminAuth.allowed) { | ||
| return NextResponse.json( | ||
| { error: 'Admin access required to lock/unlock workflows' }, | ||
| { status: 403 } | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| // If workflow is locked, only allow toggling isLocked (by admins) | ||
| if (workflowData.isLocked && updates.isLocked === undefined) { | ||
| return NextResponse.json({ error: 'Workflow is locked' }, { status: 403 }) | ||
| } | ||
|
|
||
| const updateData: Record<string, unknown> = { updatedAt: new Date() } | ||
| if (updates.name !== undefined) updateData.name = updates.name | ||
| if (updates.description !== undefined) updateData.description = updates.description | ||
| if (updates.color !== undefined) updateData.color = updates.color | ||
| if (updates.folderId !== undefined) updateData.folderId = updates.folderId | ||
| if (updates.sortOrder !== undefined) updateData.sortOrder = updates.sortOrder | ||
| if (updates.isLocked !== undefined) updateData.isLocked = updates.isLocked | ||
|
|
||
| if (updates.name !== undefined || updates.folderId !== undefined) { | ||
| const targetName = updates.name ?? workflowData.name | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -3,7 +3,7 @@ | |||||
| import { useCallback, useMemo, useRef, useState } from 'react' | ||||||
| import { createLogger } from '@sim/logger' | ||||||
| import clsx from 'clsx' | ||||||
| import { ChevronRight, Folder, FolderOpen, MoreHorizontal } from 'lucide-react' | ||||||
| import { ChevronRight, Folder, FolderOpen, Lock, MoreHorizontal } from 'lucide-react' | ||||||
| import { useParams, useRouter } from 'next/navigation' | ||||||
| import { generateId } from '@/lib/core/utils/uuid' | ||||||
| import { getNextWorkflowColor } from '@/lib/workflows/colors' | ||||||
|
|
@@ -27,10 +27,11 @@ import { | |||||
| useExportFolder, | ||||||
| useExportSelection, | ||||||
| } from '@/app/workspace/[workspaceId]/w/hooks' | ||||||
| import { useCreateFolder, useUpdateFolder } from '@/hooks/queries/folders' | ||||||
| import { useCreateFolder, useFolderMap, useUpdateFolder } from '@/hooks/queries/folders' | ||||||
| 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' | ||||||
|
|
@@ -71,6 +72,23 @@ export function FolderItem({ | |||||
| const selectedFolders = useFolderStore((state) => state.selectedFolders) | ||||||
| const isSelected = selectedFolders.has(folder.id) | ||||||
|
|
||||||
| const { data: folderMap } = useFolderMap(workspaceId) | ||||||
| const isEffectivelyLocked = useMemo( | ||||||
| () => isFolderEffectivelyLocked(folder.id, folderMap ?? {}), | ||||||
| [folder.id, folderMap] | ||||||
| ) | ||||||
| const isDirectlyLocked = folder.isLocked ?? false | ||||||
| const isLockedByParent = isEffectivelyLocked && !isDirectlyLocked | ||||||
|
|
||||||
| const handleToggleLock = useCallback(() => { | ||||||
| updateFolderMutation.mutate({ | ||||||
| workspaceId, | ||||||
| id: folder.id, | ||||||
| updates: { isLocked: !isDirectlyLocked }, | ||||||
| }) | ||||||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||||||
| }, [workspaceId, folder.id, isDirectlyLocked]) | ||||||
|
|
||||||
| const { canDeleteFolder, canDeleteWorkflows } = useCanDelete({ workspaceId }) | ||||||
|
|
||||||
| const [isDeleteModalOpen, setIsDeleteModalOpen] = useState(false) | ||||||
|
|
@@ -301,11 +319,12 @@ export function FolderItem({ | |||||
|
|
||||||
| const handleDoubleClick = useCallback( | ||||||
| (e: React.MouseEvent) => { | ||||||
| if (isEffectivelyLocked) return | ||||||
| e.preventDefault() | ||||||
| e.stopPropagation() | ||||||
| handleStartEdit() | ||||||
| }, | ||||||
| [handleStartEdit] | ||||||
| [handleStartEdit, isEffectivelyLocked] | ||||||
| ) | ||||||
|
|
||||||
| const handleClick = useCallback( | ||||||
|
|
@@ -505,6 +524,9 @@ export function FolderItem({ | |||||
| > | ||||||
| {folder.name} | ||||||
| </span> | ||||||
| {isEffectivelyLocked && ( | ||||||
| <Lock className='h-3 w-3 flex-shrink-0 text-[var(--text-icon)]' /> | ||||||
| )} | ||||||
| <button | ||||||
| type='button' | ||||||
| aria-label='Folder options' | ||||||
|
|
@@ -538,14 +560,22 @@ export function FolderItem({ | |||||
| showRename={!isMixedSelection && selectedFolders.size <= 1} | ||||||
| showDuplicate={true} | ||||||
| showExport={true} | ||||||
| disableRename={!userPermissions.canEdit} | ||||||
| disableCreate={!userPermissions.canEdit || createWorkflowMutation.isPending} | ||||||
| disableCreateFolder={!userPermissions.canEdit || createFolderMutation.isPending} | ||||||
| disableRename={!userPermissions.canEdit || isEffectivelyLocked} | ||||||
| disableCreate={ | ||||||
| !userPermissions.canEdit || createWorkflowMutation.isPending || isEffectivelyLocked | ||||||
| } | ||||||
| disableCreateFolder={ | ||||||
| !userPermissions.canEdit || createFolderMutation.isPending || isEffectivelyLocked | ||||||
| } | ||||||
| disableDuplicate={ | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Mirrors the issue in
Suggested change
|
||||||
| !userPermissions.canEdit || isDuplicatingSelection || !hasExportableContent | ||||||
| } | ||||||
| disableExport={!userPermissions.canEdit || isExporting || !hasExportableContent} | ||||||
| disableDelete={!userPermissions.canEdit || !canDeleteSelection} | ||||||
| disableDelete={!userPermissions.canEdit || !canDeleteSelection || isEffectivelyLocked} | ||||||
| onToggleLock={handleToggleLock} | ||||||
| showLock={!isMixedSelection && selectedFolders.size <= 1} | ||||||
| disableLock={!userPermissions.canAdmin || isLockedByParent} | ||||||
| isLocked={isEffectivelyLocked} | ||||||
| /> | ||||||
|
|
||||||
| <DeleteModal | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2,11 +2,10 @@ | |||||
|
|
||||||
| import { useCallback, useMemo, useRef, useState } from 'react' | ||||||
| import clsx from 'clsx' | ||||||
| import { MoreHorizontal } from 'lucide-react' | ||||||
| import { Lock, MoreHorizontal } from 'lucide-react' | ||||||
| import Link from 'next/link' | ||||||
| import { useParams } from 'next/navigation' | ||||||
| import { useUserPermissionsContext } from '@/app/workspace/[workspaceId]/providers/workspace-permissions-provider' | ||||||
| import { getWorkflowLockToggleIds } from '@/app/workspace/[workspaceId]/w/[workflowId]/utils' | ||||||
| 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' | ||||||
| import { Avatars } from '@/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/workflow-item/avatars/avatars' | ||||||
|
|
@@ -25,13 +24,13 @@ import { | |||||
| useExportSelection, | ||||||
| useExportWorkflow, | ||||||
| } from '@/app/workspace/[workspaceId]/w/hooks' | ||||||
| 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 { useWorkflowRegistry } from '@/stores/workflows/registry/store' | ||||||
| import type { WorkflowMetadata } from '@/stores/workflows/registry/types' | ||||||
| import { useWorkflowStore } from '@/stores/workflows/workflow/store' | ||||||
|
|
||||||
| interface WorkflowItemProps { | ||||||
| workflow: WorkflowMetadata | ||||||
|
|
@@ -174,28 +173,21 @@ export function WorkflowItem({ | |||||
| [workflow.id, workspaceId] | ||||||
| ) | ||||||
|
|
||||||
| const activeWorkflowId = useWorkflowRegistry((state) => state.activeWorkflowId) | ||||||
| const isActiveWorkflow = workflow.id === activeWorkflowId | ||||||
|
|
||||||
| const isWorkflowLocked = useWorkflowStore( | ||||||
| useCallback( | ||||||
| (state) => { | ||||||
| if (!isActiveWorkflow) return false | ||||||
| const blockValues = Object.values(state.blocks) | ||||||
| if (blockValues.length === 0) return false | ||||||
| return blockValues.every((block) => block.locked) | ||||||
| }, | ||||||
| [isActiveWorkflow] | ||||||
| ) | ||||||
| const { data: folderMap } = useFolderMap(workspaceId) | ||||||
| const isEffectivelyLocked = useMemo( | ||||||
| () => isWorkflowEffectivelyLocked(workflow, folderMap ?? {}), | ||||||
| [workflow, folderMap] | ||||||
| ) | ||||||
| const isLockedByFolder = isEffectivelyLocked && !workflow.isLocked | ||||||
|
|
||||||
| const handleToggleLock = useCallback(() => { | ||||||
| if (!isActiveWorkflow) return | ||||||
| const blocks = useWorkflowStore.getState().blocks | ||||||
| const blockIds = getWorkflowLockToggleIds(blocks, !isWorkflowLocked) | ||||||
| if (blockIds.length === 0) return | ||||||
| window.dispatchEvent(new CustomEvent('toggle-workflow-lock', { detail: { blockIds } })) | ||||||
| }, [isActiveWorkflow, isWorkflowLocked]) | ||||||
| updateWorkflowMutation.mutate({ | ||||||
| workspaceId, | ||||||
| workflowId: workflow.id, | ||||||
| metadata: { isLocked: !workflow.isLocked }, | ||||||
| }) | ||||||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||||||
| }, [workspaceId, workflow.id, workflow.isLocked]) | ||||||
|
|
||||||
| const isEditingRef = useRef(false) | ||||||
|
|
||||||
|
|
@@ -359,11 +351,12 @@ export function WorkflowItem({ | |||||
|
|
||||||
| const handleDoubleClick = useCallback( | ||||||
| (e: React.MouseEvent) => { | ||||||
| if (isEffectivelyLocked) return | ||||||
| e.preventDefault() | ||||||
| e.stopPropagation() | ||||||
| handleStartEdit() | ||||||
| }, | ||||||
| [handleStartEdit] | ||||||
| [handleStartEdit, isEffectivelyLocked] | ||||||
| ) | ||||||
|
|
||||||
| const handleClick = useCallback( | ||||||
|
|
@@ -447,6 +440,9 @@ export function WorkflowItem({ | |||||
| {workflow.name} | ||||||
| </div> | ||||||
| )} | ||||||
| {!isEditing && isEffectivelyLocked && ( | ||||||
| <Lock className='h-3 w-3 flex-shrink-0 text-[var(--text-icon)]' /> | ||||||
| )} | ||||||
| {!isEditing && <Avatars workflowId={workflow.id} />} | ||||||
| </div> | ||||||
| </div> | ||||||
|
|
@@ -486,15 +482,15 @@ export function WorkflowItem({ | |||||
| showDuplicate={true} | ||||||
| showExport={true} | ||||||
| showColorChange={!isMixedSelection && selectedWorkflows.size <= 1} | ||||||
| disableRename={!userPermissions.canEdit} | ||||||
| disableRename={!userPermissions.canEdit || isEffectivelyLocked} | ||||||
| disableDuplicate={!userPermissions.canEdit || isDuplicatingSelection} | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The PR description explicitly states that "sidebar actions (rename, color, move, delete, duplicate) blocked" for locked workflows. However,
Suggested change
The same issue exists in disableDuplicate={!userPermissions.canEdit || isDuplicatingSelection || !hasExportableContent}That should include |
||||||
| disableExport={!userPermissions.canEdit} | ||||||
| disableColorChange={!userPermissions.canEdit} | ||||||
| disableDelete={!userPermissions.canEdit || !canDeleteSelection} | ||||||
| disableColorChange={!userPermissions.canEdit || isEffectivelyLocked} | ||||||
| disableDelete={!userPermissions.canEdit || !canDeleteSelection || isEffectivelyLocked} | ||||||
| onToggleLock={handleToggleLock} | ||||||
| showLock={isActiveWorkflow && !isMixedSelection && selectedWorkflows.size <= 1} | ||||||
| disableLock={!userPermissions.canAdmin} | ||||||
| isLocked={isWorkflowLocked} | ||||||
| showLock={!isMixedSelection && selectedWorkflows.size <= 1} | ||||||
| disableLock={!userPermissions.canAdmin || isLockedByFolder} | ||||||
| isLocked={isEffectivelyLocked} | ||||||
| /> | ||||||
|
|
||||||
| <DeleteModal | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lock enforcement bypassed when isLocked included in request
Medium Severity
The lock enforcement check only fires when
isLocked === undefined, so an admin can modify any field on a locked resource by also includingisLockedin the request body (even re-sending the current value). For example,{ isLocked: true, name: "modified" }sent to a locked folder skips the guard entirely — renaming succeeds while the folder stays locked. The same pattern applies to the workflow PUT handler at line 312. This contradicts the comments stating "only allow togglingisLocked."Additional Locations (1)
apps/sim/app/api/workflows/[id]/route.ts#L310-L314Reviewed by Cursor Bugbot for commit de71cd7. Configure here.