Skip to content

Commit ba89105

Browse files
committed
fix(env): drop the getBaseUrl browser-origin fallback
The fallback was added in #6214 as a safety net while the real cause — the hosted env script losing its `beforeInteractive` strategy — was fixed in the same PR. With the injection ordering restored, `window.__ENV` is populated before hydration, so the fallback is unreachable in any correctly configured deployment. Guessing the origin was also unsafe in the one case it could still fire. An opaque origin — a sandboxed iframe, and `/chat/*` is deliberately embeddable — serializes to the string `'null'`, which is truthy, so `getBaseUrl()` would have returned `'null'` and every call site would have silently built `null/api/...`. A throw surfaces the misconfiguration instead of encoding it into request URLs. - restore the unconditional throw when NEXT_PUBLIC_APP_URL is unset or blank - mirror it back in the shared testing mock - flip the two fallback tests to assert the throw, keeping whitespace-only coverage Server-side behavior is unchanged: there was never a `window` to fall back to, so callers that already guard `getBaseUrl()` (`getBaseDomain`, `validateCallbackUrl`) keep their existing fail-closed paths.
1 parent 83988c1 commit ba89105

3 files changed

Lines changed: 27 additions & 31 deletions

File tree

apps/sim/lib/core/utils/urls.test.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,15 +56,20 @@ describe('getBaseUrl', () => {
5656
expect(getBaseUrl()).toBe('https://app.example.com')
5757
})
5858

59-
it('falls back to the page origin instead of throwing when the injected env is missing', () => {
59+
/**
60+
* Never guesses from `window.location.origin`: an opaque origin (a sandboxed
61+
* iframe) serializes to the truthy string `'null'`, which would silently
62+
* produce `null/api/...` rather than surfacing the misconfiguration.
63+
*/
64+
it('throws in the browser rather than guessing from the page origin', () => {
6065
setLocation('https://www.sim.ai/workspace/ws-1/w/wf-1')
61-
expect(getBaseUrl()).toBe('https://www.sim.ai')
66+
expect(() => getBaseUrl()).toThrow('NEXT_PUBLIC_APP_URL must be configured')
6267
})
6368

6469
it('treats a whitespace-only NEXT_PUBLIC_APP_URL as unset', () => {
6570
mockGetEnv.mockImplementation((key) => (key === 'NEXT_PUBLIC_APP_URL' ? ' ' : undefined))
6671
setLocation('https://www.sim.ai/')
67-
expect(getBaseUrl()).toBe('https://www.sim.ai')
72+
expect(() => getBaseUrl()).toThrow('NEXT_PUBLIC_APP_URL must be configured')
6873
})
6974
})
7075

apps/sim/lib/core/utils/urls.ts

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -25,31 +25,26 @@ function normalizeBaseUrl(url: string): string {
2525
* Returns the base URL of the application from NEXT_PUBLIC_APP_URL
2626
* This ensures webhooks, callbacks, and other integrations always use the correct public URL
2727
*
28-
* In the browser, falls back to the page's own origin when the injected env is
29-
* unavailable. Client-side callers only ever want a URL back to the app they are
30-
* already served from, so the origin is a correct answer — and a throw here
31-
* during render tears down the whole page through the error boundary. Server-side
32-
* callers (webhooks, callbacks, emails) have no origin to fall back to and must
33-
* still fail loudly on a misconfigured deployment.
28+
* Deliberately has no browser fallback to `window.location.origin`. The value is
29+
* injected before hydration by `<PublicEnvScript>`, so an empty read means the
30+
* deployment is misconfigured — and a same-origin guess would hide that. It also
31+
* would not be safe to guess: an opaque origin (a sandboxed iframe, and `/chat/*`
32+
* is embeddable) serializes to the string `'null'`, which is truthy and would
33+
* silently produce `null/api/...` at every call site.
3434
*
3535
* @returns The base URL string (e.g., 'http://localhost:3000' or 'https://example.com')
36-
* @throws Error if NEXT_PUBLIC_APP_URL is not configured and no browser origin exists
36+
* @throws Error if NEXT_PUBLIC_APP_URL is not configured
3737
*/
3838
export function getBaseUrl(): string {
3939
const baseUrl = getEnv('NEXT_PUBLIC_APP_URL')?.trim()
4040

41-
if (baseUrl) {
42-
return normalizeBaseUrl(baseUrl)
43-
}
44-
45-
const browserOrigin = getBrowserOrigin()
46-
if (browserOrigin) {
47-
return browserOrigin
41+
if (!baseUrl) {
42+
throw new Error(
43+
'NEXT_PUBLIC_APP_URL must be configured for webhooks and callbacks to work correctly'
44+
)
4845
}
4946

50-
throw new Error(
51-
'NEXT_PUBLIC_APP_URL must be configured for webhooks and callbacks to work correctly'
52-
)
47+
return normalizeBaseUrl(baseUrl)
5348
}
5449

5550
/**

packages/testing/src/mocks/urls.mock.ts

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,18 +26,14 @@ function hasHttpProtocol(url: string): boolean {
2626

2727
function getBaseUrlImpl(): string {
2828
const baseUrl = readEnv('NEXT_PUBLIC_APP_URL')?.trim()
29-
if (baseUrl) {
30-
// Mirrors the real module: protocol-less values get https:// under isProd.
31-
const protocol = envFlagsMock.isProd ? 'https://' : 'http://'
32-
return hasHttpProtocol(baseUrl) ? baseUrl : `${protocol}${baseUrl}`
29+
if (!baseUrl) {
30+
throw new Error(
31+
'NEXT_PUBLIC_APP_URL must be configured for webhooks and callbacks to work correctly'
32+
)
3333
}
34-
// Mirrors the real module: the browser falls back to its own origin, only
35-
// server-side (no `window`) callers throw.
36-
const browserOrigin = getBrowserOriginImpl()
37-
if (browserOrigin) return browserOrigin
38-
throw new Error(
39-
'NEXT_PUBLIC_APP_URL must be configured for webhooks and callbacks to work correctly'
40-
)
34+
// Mirrors the real module: protocol-less values get https:// under isProd.
35+
const protocol = envFlagsMock.isProd ? 'https://' : 'http://'
36+
return hasHttpProtocol(baseUrl) ? baseUrl : `${protocol}${baseUrl}`
4137
}
4238

4339
function getInternalApiBaseUrlImpl(): string {

0 commit comments

Comments
 (0)