fix(workflows): harden workflow API auth routes#3559
fix(workflows): harden workflow API auth routes#3559PlaneInABottle wants to merge 53 commits intosimstudioai:stagingfrom
Conversation
PR SummaryMedium Risk Overview Strengthens deployment/lifecycle reliability and auditability: rollbacks now preserve prior Adds extensive Vitest coverage for the new auth paths and error/rollback cases, and clarifies docs that lifecycle/deployment endpoints can be used programmatically with session auth or API keys where supported. Written by Cursor Bugbot for commit 2882d3b. 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.
| rollbackError = error instanceof Error ? error : new Error('Rollback cleanup failed') | ||
| } | ||
|
|
||
| return rollbackError |
There was a problem hiding this comment.
Rollback helper mixes Error and NextResponse return types
Low Severity
The rollbackDeployment closure returns three implicitly-mixed types: null on success, an Error when cleanup partially fails, and a NextResponse (from createErrorResponse) when undeploy fails. The caller differentiates these via instanceof checks, but the untyped contract is fragile — a future change to createErrorResponse or a new caller could easily miss one branch, and the instanceof NextResponse check couples internal rollback logic to HTTP response construction.
|
too many bug came out, I am just closing this one. |


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