mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-24 15:45:35 +08:00
fix(execute): block cross-origin session-authenticated workflow runs (#5062)
* fix(execute): block cross-origin session-authenticated workflow runs * fix(execute): scope session origin guard to provable cross-origin Address review on #5062: - Reject session-cookie execution only when provably cross-origin (Sec-Fetch-Site cross-site/same-site/none, or a mismatched Origin) instead of failing closed on absent headers. Fixes route tests that 403'd on header-less session requests, and reflects that this is CSRF protection, not anti-cookie-replay. - Drop same-site from the trusted set: only same-origin is our front-end. - Guard the Origin fallback in try/catch so a getBaseUrl() throw can't escape. - Add a route-level cross-origin rejection test.
This commit is contained in:
@@ -194,6 +194,26 @@ describe('workflow execute async route', () => {
|
||||
)
|
||||
})
|
||||
|
||||
it('rejects cross-origin session requests before authorization work', async () => {
|
||||
const req = createMockRequest(
|
||||
'POST',
|
||||
{ input: { hello: 'world' } },
|
||||
{
|
||||
'Content-Type': 'application/json',
|
||||
'Sec-Fetch-Site': 'cross-site',
|
||||
}
|
||||
)
|
||||
const params = Promise.resolve({ id: 'workflow-1' })
|
||||
|
||||
const response = await POST(req, { params })
|
||||
const body = await response.json()
|
||||
|
||||
expect(response.status).toBe(403)
|
||||
expect(body.error).toBe('Access denied')
|
||||
expect(mockAuthorizeWorkflowByWorkspacePermission).not.toHaveBeenCalled()
|
||||
expect(mockEnqueue).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('rejects oversized request bodies before authorization work', async () => {
|
||||
const req = createMockRequest(
|
||||
'POST',
|
||||
|
||||
@@ -20,6 +20,7 @@ import {
|
||||
getTimeoutErrorMessage,
|
||||
isTimeoutError,
|
||||
} from '@/lib/core/execution-limits'
|
||||
import { isCrossOriginSessionRequest } from '@/lib/core/security/same-origin'
|
||||
import { generateRequestId } from '@/lib/core/utils/request'
|
||||
import { SSE_HEADERS } from '@/lib/core/utils/sse'
|
||||
import {
|
||||
@@ -393,6 +394,17 @@ async function handleExecutePost(
|
||||
|
||||
try {
|
||||
const auth = await checkHybridAuth(req, { requireWorkflowId: false })
|
||||
|
||||
// CSRF guard: reject session-cookie execution that is provably cross-origin
|
||||
// (a different site driving the user's browser). Scoped to session auth —
|
||||
// API-key / public-API / internal-JWT callers don't use cookies. This is not
|
||||
// a defense against a non-browser client forging headers; that surface is
|
||||
// covered by the credit and execution rate-limit gates.
|
||||
if (auth.success && auth.authType === AuthType.SESSION && isCrossOriginSessionRequest(req)) {
|
||||
reqLogger.warn('Rejected cross-origin session-authenticated execute request')
|
||||
return NextResponse.json({ error: 'Access denied' }, { status: 403 })
|
||||
}
|
||||
|
||||
const isMcpBridgeRequest =
|
||||
auth.authType === AuthType.INTERNAL_JWT && req.headers.get(MCP_TOOL_BRIDGE_HEADER) === 'true'
|
||||
const useMcpBridgeAuthenticatedUserAsActor =
|
||||
|
||||
@@ -0,0 +1,53 @@
|
||||
/**
|
||||
* @vitest-environment node
|
||||
*/
|
||||
import type { NextRequest } from 'next/server'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { isCrossOriginSessionRequest } from '@/lib/core/security/same-origin'
|
||||
import { getBaseUrl } from '@/lib/core/utils/urls'
|
||||
|
||||
function makeRequest(headers: Record<string, string>): NextRequest {
|
||||
return { headers: new Headers(headers) } as unknown as NextRequest
|
||||
}
|
||||
|
||||
describe('isCrossOriginSessionRequest', () => {
|
||||
it('allows a same-origin browser fetch', () => {
|
||||
expect(isCrossOriginSessionRequest(makeRequest({ 'sec-fetch-site': 'same-origin' }))).toBe(
|
||||
false
|
||||
)
|
||||
})
|
||||
|
||||
it('rejects same-site requests (sibling subdomains are not our origin)', () => {
|
||||
expect(isCrossOriginSessionRequest(makeRequest({ 'sec-fetch-site': 'same-site' }))).toBe(true)
|
||||
})
|
||||
|
||||
it('rejects cross-site requests', () => {
|
||||
expect(isCrossOriginSessionRequest(makeRequest({ 'sec-fetch-site': 'cross-site' }))).toBe(true)
|
||||
})
|
||||
|
||||
it('rejects navigations not initiated from our front-end', () => {
|
||||
expect(isCrossOriginSessionRequest(makeRequest({ 'sec-fetch-site': 'none' }))).toBe(true)
|
||||
})
|
||||
|
||||
it('falls back to the Origin header when Sec-Fetch-Site is absent (same-origin allowed)', () => {
|
||||
const origin = new URL(getBaseUrl()).origin
|
||||
expect(isCrossOriginSessionRequest(makeRequest({ origin }))).toBe(false)
|
||||
})
|
||||
|
||||
it('rejects a foreign Origin when Sec-Fetch-Site is absent', () => {
|
||||
expect(isCrossOriginSessionRequest(makeRequest({ origin: 'https://evil.example.com' }))).toBe(
|
||||
true
|
||||
)
|
||||
})
|
||||
|
||||
it('allows requests where the origin cannot be determined (no Sec-Fetch-Site, no Origin)', () => {
|
||||
expect(isCrossOriginSessionRequest(makeRequest({}))).toBe(false)
|
||||
})
|
||||
|
||||
it('trusts the unforgeable Sec-Fetch-Site over a spoofed same-origin Origin', () => {
|
||||
const origin = new URL(getBaseUrl()).origin
|
||||
expect(
|
||||
isCrossOriginSessionRequest(makeRequest({ 'sec-fetch-site': 'cross-site', origin }))
|
||||
).toBe(true)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,36 @@
|
||||
import type { NextRequest } from 'next/server'
|
||||
import { isSameOrigin } from '@/lib/core/utils/validation'
|
||||
|
||||
/**
|
||||
* Returns true when a request is provably cross-origin — a browser fetch driven
|
||||
* from a different site than our own. Used to reject session-cookie CSRF on
|
||||
* state-changing routes: a cross-site browser request always carries
|
||||
* `Sec-Fetch-Site: cross-site` or a mismatched `Origin`, and neither header can
|
||||
* be set by in-browser attacker JavaScript (both are forbidden headers).
|
||||
*
|
||||
* `Sec-Fetch-Site` is the primary signal; only `same-origin` is treated as our
|
||||
* own front-end. The app is single-origin, so `same-site` (sibling subdomains),
|
||||
* `cross-site`, and `none` are all rejected. When it is absent, fall back to an
|
||||
* `Origin` same-origin check. When neither header is present the origin cannot
|
||||
* be determined, so the request is allowed — a genuine cross-site browser attack
|
||||
* cannot omit these headers.
|
||||
*
|
||||
* This is CSRF protection only. It does not defend against a non-browser client
|
||||
* that forges headers directly (no header-based check can); that surface is
|
||||
* covered by the credit and execution rate-limit gates.
|
||||
*/
|
||||
export function isCrossOriginSessionRequest(req: NextRequest): boolean {
|
||||
const secFetchSite = req.headers.get('sec-fetch-site')
|
||||
if (secFetchSite) {
|
||||
return secFetchSite !== 'same-origin'
|
||||
}
|
||||
|
||||
const origin = req.headers.get('origin')
|
||||
if (!origin) return false
|
||||
|
||||
try {
|
||||
return !isSameOrigin(origin)
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user