fix(workflows): harden workflow API auth routes#3559
fix(workflows): harden workflow API auth routes#3559PlaneInABottle wants to merge 50 commits intosimstudioai:stagingfrom
Conversation
PR SummaryMedium Risk Overview Improves deployment reliability and auditing: deploy now preserves prior Updates revert-to-deployment to restore draft state (including variables) in a transaction, adds broader test coverage for auth/JSON parsing/error cases across deploy/activate/revert/deployed/status routes, and clarifies docs that lifecycle/deployment endpoints can be used with session auth or API keys where supported. Written by Cursor Bugbot for commit 0981aaa. This will update automatically on new commits. Configure here. |
|
Someone is attempting to deploy a commit to the Sim Team on Vercel. A member of the Team first needs to authorize it. |
Greptile SummaryThis PR hardens the workflow API auth layer by introducing a shared Key observations:
Confidence Score: 3/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Incoming Request] --> B{Auth Type?}
B -->|Bearer JWT| C[verifyInternalToken\ndeployed/route only]
C -->|valid| D[Skip validateWorkflowAccess\nload deployed state directly]
C -->|invalid| E[validateWorkflowAccess]
B -->|Session / API Key / Internal Secret| E
E --> F[getWorkflowById]
F -->|not found| G[404 Workflow not found]
F -->|found, no workspaceId| H[403 Personal workflow deprecated]
F -->|requireDeployment: false| I[checkHybridAuth]
I -->|fail / no userId| J[401 Unauthorized]
I -->|success + userId| K[authorizeWorkflowByWorkspacePermission\naction: read / write / admin]
K -->|denied| L[403 / 404 Access denied]
K -->|allowed| M[Return workflow + auth]
F -->|requireDeployment: true| N{isDeployed?}
N -->|false| O[403 Workflow not deployed]
N -->|true| P{Internal Secret header?}
P -->|valid + allowInternalSecret| Q[Return workflow — no userId]
P -->|missing / invalid| R[Check X-Api-Key header]
R -->|missing| S[401 API key required]
R -->|present| T[authenticateApiKeyFromHeader\nworkspace or personal]
T -->|fail| U[401 Invalid API key]
T -->|success| V[updateApiKeyLastUsed\nReturn workflow]
M --> W[Route handler\nDELETE / PUT / POST / PATCH / GET]
V --> W
Q --> W
|
888a246 to
aab58cb
Compare
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
@PlaneInABottle this PR might have to wait till our v0.6 release. Lot of auth changes there that might overwrite this potentially |
|
@PlaneInABottle can you rebase against staging |
Co-authored-by: GitHub Copilot <github-copilot[bot]@users.noreply.github.com>
Co-authored-by: GitHub Copilot <github-copilot[bot]@users.noreply.github.com>
Co-authored-by: GitHub Copilot <github-copilot[bot]@users.noreply.github.com>
Use the shared audit actor helper consistently so workflow deletion matches deploy behavior and remove the redundant deploy wrapper raised in review.
Call validateWorkflowAccess directly in workflow deployment lifecycle routes and clean up the related test helper formatting raised in review.
a264ee9 to
2c267c0
Compare
|
bugbot run |
| action: 'admin', | ||
| }) | ||
| if (access.error) { | ||
| return createErrorResponse(access.error.message, access.error.status) |
There was a problem hiding this comment.
Deploy PATCH uses admin instead of write for initial access
Medium Severity
The PATCH handler for the deploy route (toggle isPublicApi) calls validateWorkflowAccess with action: 'admin', but this endpoint only updates a boolean flag on the workflow record. The previous code used validateWorkflowPermissions(id, requestId, 'admin') which was session-only, but now that API keys can also reach this endpoint, the admin requirement may be unnecessarily restrictive compared to the write-level permission used by the deployment version PATCH route for similar metadata operations.
|
this pr became too long, I guess it is better to do it in smaller prs, but I am not sure |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| workflowId, | ||
| userId, | ||
| const validation = await validateWorkflowAccess(request, workflowId, { | ||
| requireDeployment: false, |
There was a problem hiding this comment.
Redundant auth call doubles session lookups for GET
Low Severity
The GET handler calls checkHybridAuth at line 72, and then for all non-internal-precheck requests (session auth, internal JWT with userId), validateWorkflowAccess calls checkHybridAuth again internally at line 147 of middleware.ts. This causes a redundant session/auth lookup on every session-based read of a workflow. The first checkHybridAuth result could be passed into the middleware or the precheck could be restructured to avoid the double call.


Summary
What changed
stagingname/emailinto deploy, activate, and revert audit recordsValidation