Skip to content

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

Open
waleedlatif1 wants to merge 3 commits intostagingfrom
waleedlatif1/exclude-locked-from-diff
Open

feat(workflows): add isLocked to workflows and folders with cascade lock#4031
waleedlatif1 wants to merge 3 commits 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

@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 11:09pm

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

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.

waleedlatif1 and others added 2 commits April 8, 2026 14:05
…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>
…urity gaps

- Split lock utilities into pure functions (lock.ts) and DB-backed functions (lock-db.ts)
- Add cascade lock checks to all API routes: workflow CRUD, folder CRUD, state save, reorder/move
- Add workflow lock check to Socket.IO collaborative operations
- Block drag-and-drop for locked items and into locked folders
- Fix lock bypass when isLocked is included alongside other fields in request body
- Disable duplicate action for locked workflows and folders
- Make isLocked required in WorkflowMetadata type (matches NOT NULL schema)
- Add 16 unit tests for lock utility functions
- Re-export pure functions from hooks/use-effective-lock.ts for client compatibility

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- 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 <noreply@anthropic.com>
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