mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-24 15:45:35 +08:00
fix(env): restore beforeInteractive on the hosted public env script (#6214)
The hosted `<PublicEnvScript>` rendered a plain `<script>`, which lands at the end of `<head>` — after the ~40 `<script async>` chunk tags Next emits at the top of the document. An async script runs as soon as its fetch resolves, so on a warm cache a Next chunk could execute (and hydration begin) before the parser reached the env tag, leaving `window.__ENV` undefined for the first render. That surfaced as "Something went wrong" on the workflow page, since `getBaseUrl()` throws during the deploy modal's render, and as the socket falling back to the page origin instead of NEXT_PUBLIC_SOCKET_URL. Regressed in #5522, which replaced next-runtime-env's PublicEnvScript (to avoid its unstable_noStore forcing dynamic rendering) with a static equivalent that dropped the beforeInteractive strategy. - render the library's own `<EnvScript>`, which defaults to beforeInteractive and does not call unstable_noStore — hosted and self-hosted now share one implementation and one loading strategy - drop the hand-rolled serialization and `<` escaping; Next's beforeInteractive path already runs the payload through htmlEscapeJsonString, which escapes `& > < U+2028 U+2029` - fall back to the browser origin in getBaseUrl() rather than throwing, so a missing injected env can never tear down a page through the error boundary; server-side callers still fail loudly - read NEXT_PUBLIC_EMAIL_PASSWORD_SIGNUP_ENABLED via getEnv() in the SSO form, matching login/signup/auth-modal — `env.X` returns the build-time placeholder, not the runtime value
This commit is contained in:
@@ -0,0 +1,35 @@
|
||||
/**
|
||||
* @vitest-environment node
|
||||
*/
|
||||
import { EnvScript } from 'next-runtime-env'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { PublicEnvScript } from '@/app/_shell/public-env-script'
|
||||
|
||||
/**
|
||||
* Guards the loading strategy, not the markup. A plain `<script>` rendered from
|
||||
* the root layout lands after the `<script async>` chunk tags Next emits at the
|
||||
* top of the document, so a chunk can execute - and hydration can begin - before
|
||||
* `window.__ENV` is populated. Delegating to `<EnvScript>` keeps the
|
||||
* `beforeInteractive` guarantee that `next-runtime-env` applies by default.
|
||||
*/
|
||||
describe('PublicEnvScript', () => {
|
||||
it('delegates to next-runtime-env EnvScript rather than emitting a raw script tag', () => {
|
||||
const element = PublicEnvScript()
|
||||
|
||||
expect(element.type).toBe(EnvScript)
|
||||
expect(element.type).not.toBe('script')
|
||||
})
|
||||
|
||||
it('does not opt out of the beforeInteractive strategy', () => {
|
||||
const { disableNextScript, nextScriptProps } = PublicEnvScript().props
|
||||
|
||||
expect(disableNextScript).toBeUndefined()
|
||||
expect(nextScriptProps?.strategy ?? 'beforeInteractive').toBe('beforeInteractive')
|
||||
})
|
||||
|
||||
it('passes only NEXT_PUBLIC_ variables through to the browser', () => {
|
||||
const keys = Object.keys(PublicEnvScript().props.env)
|
||||
|
||||
expect(keys.every((key) => /^NEXT_PUBLIC_/i.test(key))).toBe(true)
|
||||
})
|
||||
})
|
||||
@@ -1,36 +1,38 @@
|
||||
import { PUBLIC_ENV_KEY } from 'next-runtime-env'
|
||||
import { EnvScript } from 'next-runtime-env'
|
||||
|
||||
/**
|
||||
* `NEXT_PUBLIC_*` values, captured once at module load (build time / server
|
||||
* start) rather than read per-request - correct on the hosted deployment,
|
||||
* where a build's env never changes between requests. Filter matches
|
||||
* `next-runtime-env`'s own `getPublicEnv()` exactly.
|
||||
* `NEXT_PUBLIC_*` values, captured once when this module is first loaded - i.e.
|
||||
* at server start on the hosted deployment, where a build's env never changes
|
||||
* between requests. Filter matches `next-runtime-env`'s own `getPublicEnv()`
|
||||
* exactly.
|
||||
*
|
||||
* These are deliberately NOT the values Next inlines into the client bundle:
|
||||
* the image is built with placeholder `NEXT_PUBLIC_*` values and the real ones
|
||||
* are supplied to the container at start, so `window.__ENV` is the only source
|
||||
* of truth in the browser.
|
||||
*/
|
||||
const HOSTED_PUBLIC_ENV = Object.fromEntries(
|
||||
Object.entries(process.env).filter(([key]) => /^NEXT_PUBLIC_/i.test(key))
|
||||
)
|
||||
|
||||
/**
|
||||
* Static, build-time equivalent of `next-runtime-env`'s `<PublicEnvScript>`
|
||||
* for the hosted deployment. It populates `window[PUBLIC_ENV_KEY]` with the
|
||||
* exact same shape `getEnv()` (`lib/core/config/env.ts`) reads client-side,
|
||||
* but without `next-runtime-env`'s unconditional `unstable_noStore()` call -
|
||||
* that call opts the entire app into dynamic rendering, which only pays off
|
||||
* for self-hosted Docker images that re-inject env per deploy without a
|
||||
* rebuild. On hosted, env is fixed per build, so this is safe to render
|
||||
* statically alongside the marketing pages' `revalidate`.
|
||||
* Static equivalent of `next-runtime-env`'s `<PublicEnvScript>` for the hosted
|
||||
* deployment. It renders the library's own `<EnvScript>`, so the emitted markup
|
||||
* and its `beforeInteractive` loading strategy are identical to the self-hosted
|
||||
* path - only the env read differs. `<PublicEnvScript>` additionally calls
|
||||
* `unstable_noStore()`, which opts the entire app into dynamic rendering; that
|
||||
* only pays off for self-hosted Docker images that re-inject env per deploy
|
||||
* without a rebuild, so hosted reads the env once here and stays static.
|
||||
*
|
||||
* Escapes `<` in the serialized JSON so an env value containing `</script>`
|
||||
* can't close this tag early and inject markup into every hosted page.
|
||||
* `beforeInteractive` is load-bearing, not an optimization. A plain `<script>`
|
||||
* rendered from the root layout lands at the end of `<head>`, after the ~40
|
||||
* `<script async>` chunk tags Next emits at the top of the document; an `async`
|
||||
* script runs as soon as its fetch resolves, so on a warm cache a Next chunk
|
||||
* can execute - and hydration can begin - before the parser reaches the env
|
||||
* tag, leaving `window.__ENV` undefined for the first render.
|
||||
* `beforeInteractive` instead queues the script into `self.__next_s`, which
|
||||
* Next's `appBootstrap` drains to completion before calling `hydrate()`.
|
||||
*/
|
||||
export function PublicEnvScript() {
|
||||
const serialized = JSON.stringify(HOSTED_PUBLIC_ENV).replace(/</g, '\\u003c')
|
||||
return (
|
||||
<script
|
||||
id='public-env'
|
||||
dangerouslySetInnerHTML={{
|
||||
__html: `window['${PUBLIC_ENV_KEY}'] = ${serialized}`,
|
||||
}}
|
||||
/>
|
||||
)
|
||||
return <EnvScript env={HOSTED_PUBLIC_ENV} />
|
||||
}
|
||||
|
||||
@@ -6,7 +6,7 @@ import { createLogger } from '@sim/logger'
|
||||
import Link from 'next/link'
|
||||
import { useRouter, useSearchParams } from 'next/navigation'
|
||||
import { client } from '@/lib/auth/auth-client'
|
||||
import { env, isFalsy } from '@/lib/core/config/env'
|
||||
import { getEnv, isFalsy } from '@/lib/core/config/env'
|
||||
import { validateCallbackUrl } from '@/lib/core/security/input-validation'
|
||||
import { quickValidateEmail } from '@/lib/messaging/email/validation'
|
||||
import { AuthSubmitButton } from '@/app/(auth)/components'
|
||||
@@ -38,6 +38,8 @@ export default function SSOForm() {
|
||||
const [showEmailValidationError, setShowEmailValidationError] = useState(false)
|
||||
const [callbackUrl, setCallbackUrl] = useState('/workspace')
|
||||
|
||||
const emailEnabled = !isFalsy(getEnv('NEXT_PUBLIC_EMAIL_PASSWORD_SIGNUP_ENABLED'))
|
||||
|
||||
useEffect(() => {
|
||||
if (searchParams) {
|
||||
const callback = searchParams.get('callbackUrl')
|
||||
@@ -184,8 +186,7 @@ export default function SSOForm() {
|
||||
</AuthSubmitButton>
|
||||
</form>
|
||||
|
||||
{/* Only show divider and email signin button if email/password is enabled */}
|
||||
{!isFalsy(env.NEXT_PUBLIC_EMAIL_PASSWORD_SIGNUP_ENABLED) && (
|
||||
{emailEnabled && (
|
||||
<>
|
||||
<div className='relative my-6 font-light'>
|
||||
<div className='absolute inset-0 flex items-center'>
|
||||
@@ -208,8 +209,7 @@ export default function SSOForm() {
|
||||
</>
|
||||
)}
|
||||
|
||||
{/* Only show signup link if email/password signup is enabled */}
|
||||
{!isFalsy(env.NEXT_PUBLIC_EMAIL_PASSWORD_SIGNUP_ENABLED) && (
|
||||
{emailEnabled && (
|
||||
<div className='pt-6 text-center font-light text-base'>
|
||||
<span className='font-normal'>Don't have an account? </span>
|
||||
<Link
|
||||
|
||||
@@ -14,6 +14,7 @@ vi.mock('@/lib/core/config/env', () => ({
|
||||
}))
|
||||
|
||||
import {
|
||||
getBaseUrl,
|
||||
getBrowserOrigin,
|
||||
getSocketUrl,
|
||||
isLocalhostUrl,
|
||||
@@ -37,6 +38,36 @@ describe('getBrowserOrigin', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('getBaseUrl', () => {
|
||||
beforeEach(() => {
|
||||
mockGetEnv.mockReset()
|
||||
mockGetEnv.mockReturnValue(undefined)
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks()
|
||||
})
|
||||
|
||||
it('uses NEXT_PUBLIC_APP_URL when set', () => {
|
||||
mockGetEnv.mockImplementation((key) =>
|
||||
key === 'NEXT_PUBLIC_APP_URL' ? 'https://app.example.com' : undefined
|
||||
)
|
||||
setLocation('https://other.example.com/workspace/w/1')
|
||||
expect(getBaseUrl()).toBe('https://app.example.com')
|
||||
})
|
||||
|
||||
it('falls back to the page origin instead of throwing when the injected env is missing', () => {
|
||||
setLocation('https://www.sim.ai/workspace/ws-1/w/wf-1')
|
||||
expect(getBaseUrl()).toBe('https://www.sim.ai')
|
||||
})
|
||||
|
||||
it('treats a whitespace-only NEXT_PUBLIC_APP_URL as unset', () => {
|
||||
mockGetEnv.mockImplementation((key) => (key === 'NEXT_PUBLIC_APP_URL' ? ' ' : undefined))
|
||||
setLocation('https://www.sim.ai/')
|
||||
expect(getBaseUrl()).toBe('https://www.sim.ai')
|
||||
})
|
||||
})
|
||||
|
||||
describe('getSocketUrl', () => {
|
||||
beforeEach(() => {
|
||||
mockGetEnv.mockReset()
|
||||
|
||||
@@ -24,19 +24,32 @@ function normalizeBaseUrl(url: string): string {
|
||||
/**
|
||||
* Returns the base URL of the application from NEXT_PUBLIC_APP_URL
|
||||
* This ensures webhooks, callbacks, and other integrations always use the correct public URL
|
||||
*
|
||||
* In the browser, falls back to the page's own origin when the injected env is
|
||||
* unavailable. Client-side callers only ever want a URL back to the app they are
|
||||
* already served from, so the origin is a correct answer — and a throw here
|
||||
* during render tears down the whole page through the error boundary. Server-side
|
||||
* callers (webhooks, callbacks, emails) have no origin to fall back to and must
|
||||
* still fail loudly on a misconfigured deployment.
|
||||
*
|
||||
* @returns The base URL string (e.g., 'http://localhost:3000' or 'https://example.com')
|
||||
* @throws Error if NEXT_PUBLIC_APP_URL is not configured
|
||||
* @throws Error if NEXT_PUBLIC_APP_URL is not configured and no browser origin exists
|
||||
*/
|
||||
export function getBaseUrl(): string {
|
||||
const baseUrl = getEnv('NEXT_PUBLIC_APP_URL')?.trim()
|
||||
|
||||
if (!baseUrl) {
|
||||
throw new Error(
|
||||
'NEXT_PUBLIC_APP_URL must be configured for webhooks and callbacks to work correctly'
|
||||
)
|
||||
if (baseUrl) {
|
||||
return normalizeBaseUrl(baseUrl)
|
||||
}
|
||||
|
||||
return normalizeBaseUrl(baseUrl)
|
||||
const browserOrigin = getBrowserOrigin()
|
||||
if (browserOrigin) {
|
||||
return browserOrigin
|
||||
}
|
||||
|
||||
throw new Error(
|
||||
'NEXT_PUBLIC_APP_URL must be configured for webhooks and callbacks to work correctly'
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -26,14 +26,18 @@ function hasHttpProtocol(url: string): boolean {
|
||||
|
||||
function getBaseUrlImpl(): string {
|
||||
const baseUrl = readEnv('NEXT_PUBLIC_APP_URL')?.trim()
|
||||
if (!baseUrl) {
|
||||
throw new Error(
|
||||
'NEXT_PUBLIC_APP_URL must be configured for webhooks and callbacks to work correctly'
|
||||
)
|
||||
if (baseUrl) {
|
||||
// Mirrors the real module: protocol-less values get https:// under isProd.
|
||||
const protocol = envFlagsMock.isProd ? 'https://' : 'http://'
|
||||
return hasHttpProtocol(baseUrl) ? baseUrl : `${protocol}${baseUrl}`
|
||||
}
|
||||
// Mirrors the real module: protocol-less values get https:// under isProd.
|
||||
const protocol = envFlagsMock.isProd ? 'https://' : 'http://'
|
||||
return hasHttpProtocol(baseUrl) ? baseUrl : `${protocol}${baseUrl}`
|
||||
// Mirrors the real module: the browser falls back to its own origin, only
|
||||
// server-side (no `window`) callers throw.
|
||||
const browserOrigin = getBrowserOriginImpl()
|
||||
if (browserOrigin) return browserOrigin
|
||||
throw new Error(
|
||||
'NEXT_PUBLIC_APP_URL must be configured for webhooks and callbacks to work correctly'
|
||||
)
|
||||
}
|
||||
|
||||
function getInternalApiBaseUrlImpl(): string {
|
||||
|
||||
Reference in New Issue
Block a user