Skip to content

feat(workflows): add isLocked to workflows and folders with cascade lock#4031

Open
waleedlatif1 wants to merge 1 commit intostagingfrom
waleedlatif1/exclude-locked-from-diff
Open

feat(workflows): add isLocked to workflows and folders with cascade lock#4031
waleedlatif1 wants to merge 1 commit intostagingfrom
waleedlatif1/exclude-locked-from-diff

Conversation

@waleedlatif1
Copy link
Copy Markdown
Collaborator

Summary

  • Adds isLocked boolean column to workflow and workflow_folder tables with safe NOT NULL DEFAULT false migration
  • Locked workflows are fully read-only: canvas editing disabled, sidebar actions (rename, color, move, delete, duplicate) blocked
  • Locked folders cascade to all contained workflows and sub-folders via client-side parent chain walk
  • Lock icon shown inline in sidebar; admin-only toggle via context menu
  • Server-side enforcement: PUT/DELETE return 403 for locked resources; admin required to toggle isLocked
  • Excludes block-level locked from diff detection so locking no longer flips deploy badge from "Live" to "Update"
  • Coexists with block-level locked for granular protection within unlocked workflows

Test plan

  • Run migration — verify two ALTER TABLE ADD COLUMN statements execute cleanly
  • Lock a workflow from sidebar context menu (even non-active) — verify lock icon appears
  • Lock a folder — verify all child workflows/folders show lock icon
  • Verify locked items disable rename, delete, color, move, duplicate in context menu
  • Open a locked workflow — verify canvas is read-only (can't add/edit/delete blocks or edges)
  • Open a workflow inside a locked folder — verify same read-only behavior
  • In an unlocked workflow, verify block-level lock still works independently
  • Lock/unlock a workflow — verify deploy badge stays "Live" (no false diff)
  • Verify non-admin users cannot toggle lock (403 from API)
  • bun run lint passes

🤖 Generated with Claude Code

…ock support

Add first-class `isLocked` property to workflows and folders that makes locked items fully read-only (canvas, sidebar rename/color/move/delete). Locked folders cascade to all contained workflows and sub-folders. Lock icon shown in sidebar, admin-only toggle via context menu. Coexists with block-level `locked` for granular protection. Also excludes block-level `locked` from diff detection so locking no longer flips deploy status.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel bot commented Apr 8, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview, Comment Apr 8, 2026 0:52am

Request Review

@cursor
Copy link
Copy Markdown

cursor bot commented Apr 8, 2026

PR Summary

Medium Risk
Adds a new lock state persisted in DB and enforced across API and UI, affecting update/delete flows and edit permissions. Risk is moderate due to new authorization checks (admin-only toggles) and potential for unexpected read-only behavior if lock cascade logic misapplies.

Overview
Introduces a persisted isLocked flag for workflows and folders (DB migration + query/type mapping) and a new client-side effective lock helper that cascades folder locks to descendants and contained workflows.

Enforces lock semantics end-to-end: APIs now restrict updates/deletes on locked resources (admin required to toggle isLocked; locked folders allow only isExpanded UI toggles), and the workspace UI shows lock indicators, disables rename/create/delete/color actions, and treats locked workflows as read-only on the canvas.

Updates workflow diffing to ignore block-level locked changes so lock toggles no longer count as workflow content changes.

Reviewed by Cursor Bugbot for commit de71cd7. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 8, 2026

Greptile Summary

This PR introduces an isLocked boolean column to both the workflow and workflow_folder tables, wires it through the full stack (DB migration → Drizzle schema → API routes → React Query hooks → sidebar UI), and adds cascade-lock logic so locking a folder propagates to all children client-side. A new use-effective-lock.ts hook encapsulates the parent-chain walk, and the deploy-badge diff comparison is confirmed to correctly ignore block-level locked changes.

Key changes:

  • Safe ALTER TABLE … ADD COLUMN is_locked boolean DEFAULT false NOT NULL migration for both tables.
  • Server-side admin-only guard on isLocked toggling for both /api/workflows/[id] and /api/folders/[id], with 403 responses for locked-resource mutations.
  • isWorkflowEffectivelyLocked / isFolderEffectivelyLocked utilities for client-side cascade.
  • Sidebar lock icon, context-menu disableLock (admin-only, blocked when locked by parent), and read-only canvas when a workflow is effectively locked.

Issues found:

  • The server-side enforcement in DELETE /api/workflows/[id] and PUT /api/workflows/[id] only checks workflowData.isLocked, not the folder ancestry. A direct API call can bypass a folder-level lock on any child workflow.
  • disableDuplicate in both workflow-item.tsx and folder-item.tsx is missing the isEffectivelyLocked guard, contradicting the PR's stated intent to block duplication on locked items.

Confidence Score: 4/5

Not safe to merge as-is: folder-level lock can be bypassed via direct API calls on child workflows, and locked items can still be duplicated from the sidebar.

Two P1 issues exist: (1) the server-side workflow DELETE/PUT guards only check the workflow's own isLocked flag, not the folder ancestry, so the folder cascade lock is trivially bypassed via direct API calls; (2) disableDuplicate in both workflow-item.tsx and folder-item.tsx is missing the isEffectivelyLocked check, contradicting the stated design. The migration, schema, folder API enforcement, cascade hook, and canvas read-only path are all correct, but these two gaps need to be closed before merging.

apps/sim/app/api/workflows/[id]/route.ts (server-side cascade gap), workflow-item.tsx and folder-item.tsx (missing isEffectivelyLocked on disableDuplicate).

Vulnerabilities

  • Authorization bypass via folder cascade gap (apps/sim/app/api/workflows/[id]/route.ts): The DELETE and PUT endpoints only enforce workflowData.isLocked (the workflow's own flag). A workflow inside a locked folder has isLocked = false, so direct API calls bypass the intended access control. Any workspace member with write access can modify or delete a workflow that is only protected by a folder-level lock.
  • No hardcoded secrets, SQL injection vectors, or insecure data exposure otherwise introduced.

Important Files Changed

Filename Overview
apps/sim/app/api/workflows/[id]/route.ts Adds isLocked to the UpdateWorkflow schema and guards DELETE/PUT with 403; critical gap: checks only workflowData.isLocked, missing the server-side folder-cascade walk.
apps/sim/app/api/folders/[id]/route.ts Adds isLocked to folder schema, correctly gates admin-only lock toggle and blocks non-expand mutations + DELETE on locked folders.
apps/sim/hooks/use-effective-lock.ts New hook correctly walks the folder parent chain to compute effective lock state for both workflows and folders.
apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/workflow-item/workflow-item.tsx Adds lock icon, handleToggleLock, and disables rename/color/delete correctly; but disableDuplicate is missing the isEffectivelyLocked guard, allowing duplication of locked workflows.
apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/folder-item/folder-item.tsx Adds lock icon and toggle for folders with correct rename/delete/create guards; same disableDuplicate gap as workflow-item — missing isEffectivelyLocked check.
packages/db/migrations/0187_certain_pretty_boy.sql Safe migration: adds is_locked boolean DEFAULT false NOT NULL to both workflow and workflow_folder tables; no data risk.
apps/sim/lib/workflows/comparison/compare.test.ts Adds a regression test confirming that toggling block-level locked does not flip the deploy-badge diff; correctly validates existing behavior.
apps/sim/hooks/queries/folders.ts Adds isLocked to mapFolder, UpdateFolderVariables, and the optimistic create/duplicate folder payloads; correctly defaults to false.
packages/db/schema.ts Adds isLocked: boolean('is_locked').notNull().default(false) to both workflow and workflowFolder Drizzle table definitions; correct and consistent.
apps/sim/hooks/queries/utils/workflow-list-query.ts Adds isLocked to WorkflowApiRow and maps it through mapWorkflow with a safe ?? false default.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User action: rename / delete / duplicate / move] --> B{Client-side\nisEffectivelyLocked?}
    B -- Yes --> C[Block action in UI\nrename/delete/color/move disabled]
    B -- No --> D[Allow action → API call]

    D --> E{API route\nworkflowData.isLocked?}
    E -- true --> F[Return 403]
    E -- false --> G[Execute mutation ✓]

    H[Folder locked\nworkflow.isLocked = false] -.->|Cascade walk\nclient-side only| B
    H -.->|NOT checked\nserver-side| E

    style H fill:#ffd6d6,stroke:#cc0000
    style E fill:#ffd6d6,stroke:#cc0000
    style G fill:#d4edda,stroke:#28a745
Loading

Reviews (1): Last reviewed commit: "feat(workflows): add isLocked to workflo..." | Re-trigger Greptile

Comment on lines +186 to +188
if (workflowData.isLocked) {
return NextResponse.json({ error: 'Workflow is locked' }, { status: 403 })
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 security Server-side folder cascade not enforced

The DELETE and PUT handlers only check workflowData.isLocked (the workflow's own flag) but not whether the workflow lives inside a locked folder. A client that knows a workflow's ID can bypass a folder-level lock entirely by calling the API directly.

For example:

  1. Admin locks folder F containing workflow W.
  2. Any editor calls DELETE /api/workflows/W directly.
  3. workflowData.isLocked is false, so the 403 guard is skipped and the deletion succeeds.

The same bypass exists in the PUT handler at line 312. The server-side logic should mirror the client-side isWorkflowEffectivelyLocked check by also looking up the workflow's folderId chain and checking isLocked on each ancestor.

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 isEffectivelyLocked utility in the server context.

showColorChange={!isMixedSelection && selectedWorkflows.size <= 1}
disableRename={!userPermissions.canEdit}
disableRename={!userPermissions.canEdit || isEffectivelyLocked}
disableDuplicate={!userPermissions.canEdit || isDuplicatingSelection}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Locked state not checked for disableDuplicate

The PR description explicitly states that "sidebar actions (rename, color, move, delete, duplicate) blocked" for locked workflows. However, disableDuplicate does not check isEffectivelyLocked, so any editor can still duplicate a locked workflow from the context menu.

Suggested change
disableDuplicate={!userPermissions.canEdit || isDuplicatingSelection}
disableDuplicate={!userPermissions.canEdit || isDuplicatingSelection || isEffectivelyLocked}

The same issue exists in folder-item.tsx at line 570:

disableDuplicate={!userPermissions.canEdit || isDuplicatingSelection || !hasExportableContent}

That should include || isEffectivelyLocked as well.

disableCreateFolder={
!userPermissions.canEdit || createFolderMutation.isPending || isEffectivelyLocked
}
disableDuplicate={
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Locked state not checked for disableDuplicate on folders

Mirrors the issue in workflow-item.tsx. A locked folder can still be duplicated because isEffectivelyLocked is not included in the disableDuplicate condition.

Suggested change
disableDuplicate={
disableDuplicate={!userPermissions.canEdit || isDuplicatingSelection || !hasExportableContent || isEffectivelyLocked}

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit de71cd7. Configure here.

if (hasNonExpandUpdates) {
return NextResponse.json({ error: 'Folder is locked' }, { status: 403 })
}
}
Copy link
Copy Markdown

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 including isLocked in 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 toggling isLocked."

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit de71cd7. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant