Skip to content

fix(docker): restore NEXT_PUBLIC_APP_URL build arg with dummy fallback#4665

Merged
waleedlatif1 merged 2 commits into
stagingfrom
waleedlatif1/fix-docker-app-url-build
May 19, 2026
Merged

fix(docker): restore NEXT_PUBLIC_APP_URL build arg with dummy fallback#4665
waleedlatif1 merged 2 commits into
stagingfrom
waleedlatif1/fix-docker-app-url-build

Conversation

@waleedlatif1
Copy link
Copy Markdown
Collaborator

@waleedlatif1 waleedlatif1 commented May 19, 2026

Summary

  • PR fix(security): remove localhost CORS origin, consolidate CORS in proxy #4658 removed ARG NEXT_PUBLIC_APP_URL from the Dockerfile. That broke next build in the Docker image: getBaseUrl() in apps/sim/lib/core/utils/urls.ts throws if NEXT_PUBLIC_APP_URL is unset, and it's evaluated at module load during page-data collection (fails at /_not-found).
  • Restore the dummy build-time fallback (mirrors the existing DATABASE_URL pattern).

Why this is safe (CORS fix from #4658 still holds)

The original bug was next.config.ts reading NEXT_PUBLIC_APP_URL at build time and baking it into static API CORS headers. #4658 removed that headers block. This change only restores a build-time arg so module evaluation succeeds; it does not reintroduce build-time CORS resolution:

  • next.config.ts no longer reads NEXT_PUBLIC_APP_URL at build time
  • proxy.ts calls getEnv('NEXT_PUBLIC_APP_URL') at request time
  • No module-level expression captures getBaseUrl() (verified by grep) — every caller invokes it lazily
  • The dummy http://localhost:3000 only exists inside the build environment

Test plan

  • Docker image builds cleanly (no /_not-found page-data collection failure)
  • Deployed container with real NEXT_PUBLIC_APP_URL env var serves correct Access-Control-Allow-Origin (not localhost)

getBaseUrl() in lib/core/utils/urls is evaluated at module load during
next build's page-data collection and throws if NEXT_PUBLIC_APP_URL is
unset. PR #4658 removed the build arg, breaking the Docker build at the
"/_not-found" page-data collection step.

Restore the dummy localhost fallback (mirroring DATABASE_URL). The CORS
fix from #4658 is preserved: next.config.ts no longer reads
NEXT_PUBLIC_APP_URL at build time, and no module-level expression
captures getBaseUrl() — every caller invokes it at request time, where
getEnv() reads the deployed container env. The dummy localhost value
cannot leak into runtime CORS response headers.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented May 19, 2026

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

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview, Comment May 19, 2026 8:24pm

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented May 19, 2026

PR Summary

Low Risk
Low risk: Docker build-time env wiring change only, intended to prevent next build failures due to module-eval-time env validation; runtime envs still override the value.

Overview
Fixes Docker image builds by reintroducing ARG/ENV NEXT_PUBLIC_APP_URL (defaulting to http://localhost:3000) alongside the existing dummy DATABASE_URL, so env-dependent module evaluation doesn’t crash during next build.

Adds clarifying comments that this value is only a build-time fallback and should not affect production CORS behavior.

Reviewed by Cursor Bugbot for commit 19a9821. Configure here.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 19, 2026

Greptile Summary

Restores ARG NEXT_PUBLIC_APP_URL=\"http://localhost:3000\" to the builder stage of the Dockerfile so getBaseUrl() (which throws when NEXT_PUBLIC_APP_URL is unset) does not crash next build during page-data collection. The CORS fix from #4658 is preserved because next.config.ts no longer derives CORS headers from this variable at build time.

  • Mirrors the existing DATABASE_URL dummy-value pattern; the env var is scoped to the builder stage and is not forwarded to the runner stage.
  • One pre-existing side effect is that env.NEXT_PUBLIC_APP_URL (the statically-inlined t3-oss object used in csp.ts's module-level buildTimeCSPDirectives) will embed http://localhost:3000 into the static connect-src CSP header configured in next.config.ts — this is benign (additive only) and was also the case before fix(security): remove localhost CORS origin, consolidate CORS in proxy #4658 removed the arg.
  • All runtime callers — getBaseUrl(), generateRuntimeCSP(), proxy.ts — use getEnv(), which reads window.__ENV (populated by <PublicEnvScript> at request time on the client) and process.env at request time on the server, so the dummy build-time value does not reach production.

Confidence Score: 5/5

Safe to merge — the change is a targeted one-liner that restores the build-time dummy value needed to unblock Docker image builds without reintroducing the CORS regression fixed in #4658.

The Dockerfile change is minimal and self-contained: it restores a single ARG/ENV pair scoped exclusively to the builder stage. The runtime code paths (getEnv, generateRuntimeCSP, proxy.ts) all read env vars lazily at request time, so the localhost placeholder cannot surface in a deployed container. The claim that next.config.ts no longer derives CORS headers from this variable is confirmed by reading the file — the headers block that caused the original bug is absent. The pre-existing CSP side effect (env.NEXT_PUBLIC_APP_URL baked into buildTimeCSPDirectives) is additive and benign.

No files require special attention.

Important Files Changed

Filename Overview
docker/app.Dockerfile Adds ARG/ENV NEXT_PUBLIC_APP_URL with a localhost dummy to the builder stage, unblocking next build in Docker; mirrors the existing DATABASE_URL pattern and does not affect the runner stage or production CORS.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Docker builder stage\nARG NEXT_PUBLIC_APP_URL=http://localhost:3000\nENV NEXT_PUBLIC_APP_URL=..."] --> B["next build\n(page-data collection)"]
    B --> C["getBaseUrl() called\nduring module eval\n— reads process.env at build time\n→ returns localhost:3000, no throw"]
    B --> D[".next/standalone artifact\nbuilt successfully"]
    D --> E["Runner stage\n(no NEXT_PUBLIC_APP_URL ENV)"]
    E --> F["Container starts\nOperator provides runtime\nNEXT_PUBLIC_APP_URL=https://prod-url"]
    F --> G["Server-side getEnv()\nreads process.env at request time\n→ returns https://prod-url ✓"]
    F --> H["Client-side getEnv()\nreads window.__ENV via PublicEnvScript\n→ returns https://prod-url ✓"]
    F --> I["proxy.ts generateRuntimeCSP()\nreads getEnv() at request time\n→ CORS uses https://prod-url ✓"]
Loading

Reviews (1): Last reviewed commit: "chore(docker): trim verbose comment on b..." | Re-trigger Greptile

@waleedlatif1 waleedlatif1 merged commit 49f70bc into staging May 19, 2026
13 checks passed
@waleedlatif1 waleedlatif1 deleted the waleedlatif1/fix-docker-app-url-build branch May 19, 2026 20:34
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