fix(workflows): harden workflow API auth routes#3559
fix(workflows): harden workflow API auth routes#3559PlaneInABottle wants to merge 49 commits intosimstudioai:stagingfrom
Conversation
PR SummaryMedium Risk Overview Hardens lifecycle flows with safer failure handling: preserve Written by Cursor Bugbot for commit b381908. 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. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
PlaneInABottle
force-pushed
the
fix/workflow-api-auth-hardening
branch
2 times, most recently
from
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.
PlaneInABottle
force-pushed
the
fix/workflow-api-auth-hardening
branch
from
a264ee9 to
2c267c0
Compare
|
bugbot run |
| status: 404, | ||
| }, | ||
| } | ||
| } |
There was a problem hiding this comment.
Middleware skips workspace validation for active workflow records
Medium Severity
getValidatedWorkflow returns the result of getActiveWorkflowRecord without checking workspaceId, while the fallback path through getWorkflowById explicitly validates workspaceId (returning 403 for personal workflows and 404 for archived workspaces). If getActiveWorkflowRecord ever returns a record with a null workspaceId, it bypasses the deprecation guard. Downstream in the requireDeployment=true path, workflow.workspaceId as string would then cast null, causing authenticateApiKeyFromHeader to receive an incorrect workspace scope.
| 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.
| auth.userName = scopedApiKeyResult.userName | ||
| auth.userEmail = scopedApiKeyResult.userEmail | ||
| auth.apiKeyType = scopedApiKeyResult.keyType | ||
| } |
There was a problem hiding this comment.
Double API key authentication causes redundant bcrypt and DB writes
Medium Severity
In the requireDeployment: false path, when auth.authType is API_KEY, the middleware calls authenticateApiKeyFromHeader with workspace scoping (line 101) and updateApiKeyLastUsed (line 125). However, checkHybridAuth on line 74 already called authenticateApiKeyFromHeader globally and updateApiKeyLastUsed during the same request. This results in two bcrypt hash comparisons and two updateApiKeyLastUsed DB writes per API-key-authenticated request on all lifecycle/deployment routes.
Additional Locations (1)
|
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 4 total unresolved issues (including 3 from previous reviews).
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| status: 404, | ||
| }, | ||
| } | ||
| } |
There was a problem hiding this comment.
Unreachable workspace-mismatch check after scoped re-authentication
Low Severity
The workspace API key mismatch check at lines 135–146 is unreachable dead code. The preceding authenticateApiKeyFromHeader call is scoped to workflow.workspaceId, and its result is used to overwrite auth.workspaceId. If the key doesn't match that workspace, the scoped auth fails and returns 401 earlier. If it succeeds, auth.workspaceId always equals workflow.workspaceId, so the inequality can never be true.
2 participants
Summary
What changed
stagingname/emailinto deploy, activate, and revert audit recordsValidation