Revert "fix(execute): block cross-origin session-authenticated workfl…#5065
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryHigh Risk Overview The The async route test that asserted early rejection on Reviewed by Cursor Bugbot for commit 148e4b8. Bugbot is set up for automated code reviews on this repo. Configure here. |
Greptile SummaryThis PR reverts #5062, which added a
Confidence Score: 3/5Not safe to merge without a documented rationale or a replacement CSRF control — the change leaves the session-authenticated workflow execute endpoint unprotected against cross-origin browser-driven requests. The revert removes the only origin-header check on a state-changing endpoint for session-cookie callers, and the PR description provides no explanation of what went wrong with the original fix or what alternative protection exists. apps/sim/app/api/workflows/[id]/execute/route.ts — the removed guard is the only origin validation for session-authenticated execution requests.
|
| Filename | Overview |
|---|---|
| apps/sim/app/api/workflows/[id]/execute/route.ts | Removes the isCrossOriginSessionRequest CSRF guard that blocked cross-origin browsers from triggering session-authenticated workflow execution; no alternative protection is introduced. |
| apps/sim/lib/core/security/same-origin.ts | Deleted entirely — this was the isCrossOriginSessionRequest utility that checked Sec-Fetch-Site and Origin headers to detect cross-origin browser requests. |
| apps/sim/lib/core/security/same-origin.test.ts | Deleted test suite for the removed isCrossOriginSessionRequest utility — no issues beyond the loss of coverage. |
| apps/sim/app/api/workflows/[id]/execute/route.async.test.ts | Removes the integration test asserting that cross-origin session requests receive a 403 — consistent with the revert but loses the regression guard. |
Sequence Diagram
sequenceDiagram
participant Attacker as evil.example.com
participant Browser
participant API as /api/workflows/[id]/execute
participant Auth as checkHybridAuth
participant Executor as Workflow Executor
Note over Attacker,Executor: Before revert (PR 5062 in place)
Attacker->>Browser: Embed cross-origin form/fetch
Browser->>API: POST (session cookie, Sec-Fetch-Site: cross-site)
API->>Auth: checkHybridAuth returns AuthType.SESSION
API->>API: isCrossOriginSessionRequest returns true
API-->>Browser: 403 Access denied
Note over Attacker,Executor: After this revert
Attacker->>Browser: Embed cross-origin form/fetch
Browser->>API: POST (session cookie, Sec-Fetch-Site: cross-site)
API->>Auth: checkHybridAuth returns AuthType.SESSION
Note over API: Guard removed, no origin check
API->>Executor: Execute workflow as victim user
Executor-->>Browser: 200 OK
Reviews (1): Last reviewed commit: "Revert "fix(execute): block cross-origin..." | Re-trigger Greptile
| @@ -394,17 +393,6 @@ async function handleExecutePost( | |||
|
|
|||
There was a problem hiding this comment.
CSRF guard removed without replacement
This revert strips the only origin-header-based CSRF mitigation for session-cookie-authenticated workflow execution. Any page on any origin can now embed a form or fire a fetch with credentials to POST /api/workflows/[id]/execute, and the user's session cookie will be attached by the browser — triggering an arbitrary workflow under their identity. The deleted guard was narrowly scoped: it only fired when authType === AuthType.SESSION and the request was provably cross-origin via Sec-Fetch-Site or a mismatched Origin. If it was causing false-positives for a legitimate caller, that caller (API-key / public-API / internal-JWT) would never have been gated by the check in the first place. The PR description doesn't document what went wrong or what alternative protection replaces this.
…ow runs (#5062)"
This reverts commit 67e02fa.
Summary
Brief description of what this PR does and why.
Fixes #(issue)
Type of Change
Testing
How has this been tested? What should reviewers focus on?
Checklist
Screenshots/Videos