fix(locks): enforce workflow/folder locks on the agent + close manual-UI create gaps#5107
Conversation
…-UI create gaps The copilot/agent workflow & folder mutation tools and the edit_workflow tool bypassed lock enforcement, so the agent could edit a locked workflow and move/create workflows into a locked folder. Add assertWorkflowMutable/ assertFolderMutable guards (from @sim/workflow-authz) to every agent mutation path, mirroring the REST API. Also close two parity gaps on the manual-UI REST side: creating a workflow into a locked folder and creating a subfolder under a locked parent were previously unguarded. The realtime collaborative canvas already enforced workflow-level locks server-side.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Manual UI REST gaps are closed on POST Tests were added/updated for locked-parent folder creation, locked-folder workflow creation, and agent handlers (no persist on locked workflow; move into locked folder rejected). Reviewed by Cursor Bugbot for commit 84affcf. Configure here. |
|
@greptile |
|
@cursor review |
Greptile SummaryThis PR closes the lock-enforcement gap in the agent/copilot layer and two REST API routes. Previously, the copilot mutation handlers and
Confidence Score: 5/5Safe to merge — all mutation paths now enforce lock status before touching the database, and the guard function handles null folder IDs correctly. The changes are additive guard insertions on well-understood paths. The authz package's assertWorkflowMutable / assertFolderMutable are already used by the realtime canvas layer and behave correctly for null inputs. Previous review findings (redundant DB round-trip in rename, per-iteration folder check in bulk move, mock scoping in tests) have all been addressed. No mutation path was left unguarded. No files require special attention. Important Files Changed
Reviews (5): Last reviewed commit: "refactor(locks): drop redundant ensureWo..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit f7a6d31. Configure here.
Greptile SummaryThis PR enforces workflow and folder lock status across two previously unguarded mutation surfaces: the copilot/agent tool handlers in
Confidence Score: 4/5Safe to merge — the lock guards are correctly placed after existing auth checks on every agent mutation path and on the two newly-identified REST CREATE gaps. The implementation is thorough and the guard semantics are correct across all touched paths. The two inline notes are minor: one is a redundant DB call in executeRenameWorkflow (the discard-and-re-fetch pattern), and the other is a per-iteration folder-lock query inside the bulk-move loop that could be hoisted. Neither affects correctness. mutations.ts — the executeRenameWorkflow double-fetch and the executeMoveWorkflow per-iteration folder-lock query are worth a quick look before merge. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
subgraph Agent["Copilot / Agent path (mutations.ts + edit-workflow)"]
A1[ensureWorkspaceAccess / ensureWorkflowAccess] --> A2{assertWorkflowMutable\nor assertFolderMutable}
A2 -- locked --> A3[return success:false / throw]
A2 -- ok --> A4[assertWorkflowMutationNotAborted]
A4 --> A5[performCreate/Update/Delete]
end
subgraph REST["REST API (api/workflows POST + api/folders POST)"]
B1[checkHybridAuth / getSession] --> B2[getUserEntityPermissions]
B2 -- no write --> B3[403 Forbidden]
B2 -- write --> B4{assertFolderMutable}
B4 -- FolderLockedError --> B5[423 Locked]
B4 -- ok --> B6[performCreateWorkflow / performCreateFolder]
B6 --> B7[200 OK]
end
subgraph Canvas["Realtime canvas (unchanged — already guarded)"]
C1[operation handler] --> C2[assertWorkflowMutable]
C2 --> C3[persist]
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
subgraph Agent["Copilot / Agent path (mutations.ts + edit-workflow)"]
A1[ensureWorkspaceAccess / ensureWorkflowAccess] --> A2{assertWorkflowMutable\nor assertFolderMutable}
A2 -- locked --> A3[return success:false / throw]
A2 -- ok --> A4[assertWorkflowMutationNotAborted]
A4 --> A5[performCreate/Update/Delete]
end
subgraph REST["REST API (api/workflows POST + api/folders POST)"]
B1[checkHybridAuth / getSession] --> B2[getUserEntityPermissions]
B2 -- no write --> B3[403 Forbidden]
B2 -- write --> B4{assertFolderMutable}
B4 -- FolderLockedError --> B5[423 Locked]
B4 -- ok --> B6[performCreateWorkflow / performCreateFolder]
B6 --> B7[200 OK]
end
subgraph Canvas["Realtime canvas (unchanged — already guarded)"]
C1[operation handler] --> C2[assertWorkflowMutable]
C2 --> C3[persist]
end
Reviews (2): Last reviewed commit: "fix(locks): normalize optional folderId ..." | Re-trigger Greptile |
…cope test mocks with Once
|
@greptile |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 84affcf. Configure here.
|
@greptile review |
1 similar comment
|
@greptile review |
1 participant
Summary
lib/copilot/tools/handlers/workflow/mutations.ts) and theedit_workflowtool (edit-workflow/index.ts) bypassed lock enforcement entirely — they authorized by workspace permission but never checked lock status. This let the agent (Sim) edit a locked workflow and move/create workflows into a locked folder (the ITSM_PROD report).assertWorkflowMutable/assertFolderMutableguards (from@sim/workflow-authz) to every agent mutation path: edit, create, rename, update, move, delete (workflow + folder), set-variables, and set-block-enabled. This mirrors exactly what the REST API routes already do.api/workflows) and creating a subfolder under a locked parent (api/folders).assertWorkflowMutableserver-side in its operation/subblock/variable handlers. No change needed there.Type of Change
Testing
Checklist