Skip to content

fix(env): restore beforeInteractive on the hosted public env script - #6214

Merged
waleedlatif1 merged 1 commit into
stagingfrom
worktree-fix-app-url-deploy-modal
Aug 3, 2026
Merged

fix(env): restore beforeInteractive on the hosted public env script#6214
waleedlatif1 merged 1 commit into
stagingfrom
worktree-fix-app-url-deploy-modal

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • 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 (getBaseUrl() throws during the deploy modal's render) and as the socket falling back to the page origin instead of NEXT_PUBLIC_SOCKET_URL — two symptoms, one cause.
  • Regressed in fix(landing): fix Core Web Vitals regressions across public marketing pages #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.
  • Now renders the library's own <EnvScript>, which defaults to beforeInteractive and does not call unstable_noStore. Hosted and self-hosted share one implementation and one loading strategy, so the two paths can't drift again. Next's appBootstrap drains self.__next_s to completion before calling hydrate().
  • Drops the hand-rolled serialization and < escaping — Next's beforeInteractive path already runs the payload through htmlEscapeJsonString, which escapes & > < U+2028 U+2029 (strictly more than we did).
  • getBaseUrl() now falls back to the browser origin instead of throwing, so a missing injected env can never tear down a page through the error boundary. Server-side callers (webhooks, callbacks, emails) still fail loudly. Note there is no safe static fallback: the image is built with placeholder NEXT_PUBLIC_* values, so the inlined NEXT_PUBLIC_APP_URL in the prod client bundle is http://localhost:3000window.__ENV is the only source of truth in the browser.
  • Reads 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.

Type of Change

  • Bug fix

Testing

Verified the emitted markup by running the app with NEXT_PUBLIC_FORCE_HOSTED=true: the plain <script id="public-env"> is gone and the env is queued into self.__next_s. Confirmed the escaping I removed is genuinely covered by setting NEXT_PUBLIC_BRAND_NAME='</script><img src=x onerror=alert(1)>' — output is </script>..., no breakout.

New public-env-script.test.tsx pins the loading strategy (delegates to EnvScript, no raw script tag, no strategy override); confirmed it fails on the pre-fix shape. New getBaseUrl tests confirmed red without the fallback.

tsc clean, bun run lint:check clean (only pre-existing warnings in untouched files), 45 app tests + 39 testing-package tests pass.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

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
@vercel

vercel Bot commented Aug 3, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Aug 3, 2026 6:59pm

Request Review

@cursor

cursor Bot commented Aug 3, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes affect bootstrap env timing and client URL resolution (webhooks/callbacks still fail server-side without config); incorrect beforeInteractive behavior could still break sockets and pages that depend on window.__ENV.

Overview
Fixes a hydration race where window.__ENV could be unset on the first client render because the hosted PublicEnvScript emitted a plain script tag at the end of head, after Next’s async chunks—so client code could run before env injection.

PublicEnvScript now delegates to next-runtime-env’s EnvScript with a pre-filtered NEXT_PUBLIC_* map (same static read at module load, no unstable_noStore). That restores the default beforeInteractive loading path so env is applied before hydration. Tests lock in delegation, strategy, and env key filtering.

getBaseUrl() no longer throws in the browser when NEXT_PUBLIC_APP_URL is missing or whitespace-only; it falls back to the page origin. Server-side callers still throw if neither env nor origin exists. The shared urls test mock mirrors that behavior.

The SSO form reads NEXT_PUBLIC_EMAIL_PASSWORD_SIGNUP_ENABLED via getEnv() instead of build-time env, so email/password UI matches runtime hosted config.

Reviewed by Cursor Bugbot for commit 599254b. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR restores pre-hydration runtime environment injection for hosted deployments and makes browser URL resolution resilient when injected configuration is unavailable.

  • Replaces the hosted raw environment script with next-runtime-env’s EnvScript.
  • Reads the SSO email/password feature flag through the runtime environment accessor.
  • Falls back to the current browser origin in getBaseUrl() while preserving server-side configuration errors.
  • Adds focused environment-script and URL fallback tests and updates the shared URL mock.

Confidence Score: 5/5

The PR appears safe to merge, with no concrete changed-code defect identified.

The hosted environment script is restored before hydration, runtime feature-flag access follows the established authentication pattern, and the URL fallback remains limited to browser contexts while server callers still fail on missing configuration.

Important Files Changed

Filename Overview
apps/sim/app/_shell/public-env-script.tsx Delegates hosted environment injection to EnvScript, preserving the public-variable filter and restoring the pre-hydration loading strategy.
apps/sim/ee/sso/components/sso-form.tsx Uses the runtime environment accessor for the email/password feature gate, matching sibling authentication components.
apps/sim/lib/core/utils/urls.ts Returns the configured application URL when present, otherwise uses the browser origin while retaining the server-side failure path.
packages/testing/src/mocks/urls.mock.ts Updates the shared URL mock to mirror the new browser-origin fallback.
apps/sim/app/_shell/public-env-script.test.tsx Adds focused assertions for EnvScript delegation, loading strategy, and filtering to public environment variables.
apps/sim/lib/core/utils/urls.test.ts Covers configured, missing, and whitespace-only application URL behavior.

Sequence Diagram

sequenceDiagram
  participant Server as Next.js Server
  participant Document as HTML Document
  participant Bootstrap as Next appBootstrap
  participant Browser as Client Components
  Server->>Document: Render EnvScript with runtime NEXT_PUBLIC values
  Document->>Bootstrap: Queue beforeInteractive script in self.__next_s
  Bootstrap->>Document: Execute environment script
  Document->>Document: Populate window.__ENV
  Bootstrap->>Browser: Begin hydration
  Browser->>Document: Read runtime values through getEnv()
  alt NEXT_PUBLIC_APP_URL exists
    Browser->>Browser: Normalize configured URL
  else Injected URL unavailable
    Browser->>Browser: Use window.location.origin
  end
Loading

Reviews (1): Last reviewed commit: "fix(env): restore beforeInteractive on t..." | Re-trigger Greptile

@waleedlatif1
waleedlatif1 merged commit dd79515 into staging Aug 3, 2026
27 checks passed
@waleedlatif1
waleedlatif1 deleted the worktree-fix-app-url-deploy-modal branch August 3, 2026 19:03
waleedlatif1 added a commit that referenced this pull request Aug 3, 2026
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.
waleedlatif1 added a commit that referenced this pull request Aug 3, 2026
…6217)

* fix(sso): derive the SSO callback URL during render

`callbackUrl` was seeded into `useState('/workspace')` and overwritten from
a `useEffect` that read `searchParams`, so the first painted frame always
carried the default. On any deep link with `?callbackUrl=`, the "Sign in
with email" and "Sign up" links briefly pointed at `/workspace` instead of
the requested destination, and a click landing in that window navigated to
the wrong place.

- derive `callbackUrl` from `searchParams` during render; the validation
  gate is unchanged, so an off-origin or malformed value still falls back
  to `/workspace`
- keep the warning for a rejected value in an effect, now keyed on the
  param itself rather than the `searchParams` object, so it fires once per
  actual change instead of once per identity change
- add first-frame tests via `renderToString`, which runs no effects and so
  pins exactly the window the old code got wrong

* 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

* 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant