Skip to content

Commit 73587c7

Browse files
committed
fix(auth): resolve callback URLs against the app origin server-side
`validateCallbackUrl` compared against a sentinel base (`https://callback-url-validator.invalid`) when `window` was undefined, so the server rejected every absolute URL — including the same-origin ones the function documents as valid. A component deriving a callback URL during render therefore produced one destination in the SSR markup and a different one after hydration. The exposure was not new to the SSO form: `login-form.tsx` and `signup-form.tsx` already derive their callback URL during render on `force-dynamic` pages, so both carried the same divergence. - resolve against the deployment's own origin server-side, so the server reaches the same verdict the browser will after hydration - fall back to the sentinel when the app URL is unset or unparseable, which keeps the server fail-closed: absolute URLs are rejected, as before - cover the absolute same-origin case and the unset-app-URL fallback in the existing suite; all 15 open-redirect rejection cases are unchanged
1 parent fb97b27 commit 73587c7

2 files changed

Lines changed: 46 additions & 3 deletions

File tree

apps/sim/lib/core/security/input-validation.test.ts

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { envFlagsMock, resetEnvFlagsMock } from '@sim/testing'
1+
import { defaultMockEnv, envFlagsMock, resetEnvFlagsMock, resetEnvMock, setEnv } from '@sim/testing'
22
import { afterAll, afterEach, beforeEach, describe, expect, it } from 'vitest'
33
import {
44
validateAirtableId,
@@ -2136,6 +2136,7 @@ describe('validateCallbackUrl', () => {
21362136
})
21372137

21382138
afterEach(() => {
2139+
resetEnvMock()
21392140
if (originalWindow === undefined) {
21402141
;(globalThis as { window?: unknown }).window = undefined
21412142
} else {
@@ -2187,12 +2188,28 @@ describe('validateCallbackUrl', () => {
21872188
;(globalThis as { window?: unknown }).window = undefined
21882189
})
21892190

2190-
it('falls back to placeholder origin and still rejects cross-origin URLs', () => {
2191+
it('resolves against the configured app origin and still rejects cross-origin URLs', () => {
21912192
expect(validateCallbackUrl('/workspace')).toBe(true)
21922193
expect(validateCallbackUrl('//evil.com')).toBe(false)
21932194
expect(validateCallbackUrl('https://evil.com')).toBe(false)
21942195
expect(validateCallbackUrl('javascript:alert(1)')).toBe(false)
21952196
})
2197+
2198+
/**
2199+
* The server verdict has to match what the browser will decide once it
2200+
* hydrates, or a callback URL derived during render yields one destination
2201+
* in the SSR markup and another after hydration.
2202+
*/
2203+
it('accepts an absolute same-origin URL, matching the browser verdict', () => {
2204+
expect(validateCallbackUrl(`${defaultMockEnv.NEXT_PUBLIC_APP_URL}/workspace/abc`)).toBe(true)
2205+
})
2206+
2207+
it('stays fail-closed on absolute URLs when the app URL is unset', () => {
2208+
setEnv({ NEXT_PUBLIC_APP_URL: undefined })
2209+
2210+
expect(validateCallbackUrl(`${defaultMockEnv.NEXT_PUBLIC_APP_URL}/workspace/abc`)).toBe(false)
2211+
expect(validateCallbackUrl('/workspace')).toBe(true)
2212+
})
21962213
})
21972214
})
21982215

apps/sim/lib/core/security/input-validation.ts

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { createLogger } from '@sim/logger'
22
import { isLoopbackIp, isPrivateIp, unwrapIpv6Brackets } from '@sim/security/ssrf'
33
import * as ipaddr from 'ipaddr.js'
44
import { isHosted } from '@/lib/core/config/env-flags'
5+
import { getBaseUrl } from '@/lib/core/utils/urls'
56

67
const logger = createLogger('InputValidation')
78

@@ -1234,6 +1235,31 @@ export function validatePaginationCursor(
12341235

12351236
const CALLBACK_URL_SERVER_BASE = 'https://callback-url-validator.invalid'
12361237

1238+
/**
1239+
* Origin a callback URL is resolved and compared against.
1240+
*
1241+
* The browser uses its own origin. Server-side there is no `window`, so it uses
1242+
* the deployment's configured origin — which is what the browser will compare
1243+
* against once it hydrates. Using a sentinel here instead made the server reject
1244+
* every absolute URL, including the same-origin ones this function documents as
1245+
* valid, so a component deriving a callback URL during render produced one
1246+
* destination in the SSR markup and a different one after hydration.
1247+
*
1248+
* Falls back to the sentinel when the app URL is unset or unparseable, which
1249+
* keeps the server fail-closed: every absolute URL is rejected, as before.
1250+
*/
1251+
function getCallbackValidationOrigin(): string {
1252+
if (typeof window !== 'undefined') {
1253+
return window.location.origin
1254+
}
1255+
1256+
try {
1257+
return new URL(getBaseUrl()).origin
1258+
} catch {
1259+
return CALLBACK_URL_SERVER_BASE
1260+
}
1261+
}
1262+
12371263
/**
12381264
* Validates a callback URL to prevent open redirect attacks.
12391265
*
@@ -1263,7 +1289,7 @@ export function validateCallbackUrl(url: string): boolean {
12631289
try {
12641290
if (typeof url !== 'string' || url.length === 0) return false
12651291

1266-
const base = typeof window === 'undefined' ? CALLBACK_URL_SERVER_BASE : window.location.origin
1292+
const base = getCallbackValidationOrigin()
12671293
const parsed = new URL(url, base)
12681294
return parsed.origin === base
12691295
} catch (error) {

0 commit comments

Comments
 (0)