From c2fe45ff1cb88dd77c9af53fe11e6e31bb59d016 Mon Sep 17 00:00:00 2001 From: Mamavee001 <307202201+Mamavee001@users.noreply.github.com> Date: Fri, 28 Aug 2026 18:12:03 +0100 Subject: [PATCH 1/2] fix(web): fail loudly in production when NEXT_PUBLIC_API_URL is unset (issue 5.9) apps/web/lib/api.ts's BROWSER_BASE fallback to http://localhost:8787 was unconditional - right for local dev, wrong (and previously actually broken production - see docs/FIXLOG.md, BUG-1.4) everywhere else, since NEXT_PUBLIC_* values are baked in at build time and a missing one isn't caught until a visitor's own browser silently tries to reach localhost on their own machine. - The localhost fallback now only applies when NODE_ENV !== "production". - A production build with NEXT_PUBLIC_API_URL unset throws at module load, in the browser bundle specifically (checked via `typeof window`, which is a reliable environment split here since Next.js produces genuinely separate server/browser bundles - the server bundle doesn't need this variable at all if API_URL is set, so it isn't punished for a client-only var it never uses). - The error message names the variable, states it must be set AND rebuilt (not just redeployed), and points at the prior incident. - docs/MAINNET.md's existing NEXT_PUBLIC_STELLAR_NETWORK footgun section gained a parallel callout for NEXT_PUBLIC_API_URL with the same "baked at build time, rebuild after changing" framing, plus the real incident history. .env.public.example got the matching inline comment. BUG-1.4's own "Fix" column already claimed this was "covered by the build-time env check," but the actual code (this file, before this change) still had the bare unconditional fallback - that fix was a deploy-checklist reminder, not an enforced guard, so the class of bug was never actually closed. This is the enforced version. No Node.js/npm/pnpm runtime is available in the environment this was authored in, and apps/web has no existing unit-test harness to hook a regression test into (only e2e Playwright specs, which don't exercise a production Next.js build's env-var baking). I verified the four branches (var set; unset+dev; unset+production+browser; unset+ production+server) by hand rather than by running anything. Please build with NEXT_PUBLIC_API_URL deliberately unset and NODE_ENV=production to confirm the throw fires with the right message, and confirm a normal `pnpm --filter @checkout/web dev` still works unaffected. --- .env.public.example | 9 +++++++++ apps/web/lib/api.ts | 39 ++++++++++++++++++++++++++++++++++++++- docs/MAINNET.md | 17 ++++++++++++++++- 3 files changed, 63 insertions(+), 2 deletions(-) diff --git a/.env.public.example b/.env.public.example index 4fe67c05c..4d8a1f353 100644 --- a/.env.public.example +++ b/.env.public.example @@ -119,10 +119,19 @@ CORS_ORIGINS=https:// METRICS_TOKEN=<64 hex chars> # ---- Web -------------------------------------------------------------------- +# Every NEXT_PUBLIC_* value below is baked into the client bundle AT BUILD +# TIME - changing one and redeploying without rebuilding does nothing; the +# old value is still what ships in the bundle a visitor's browser downloads. +# # MUST match STELLAR_NETWORK above. The browser signs with the passphrase this # selects; a mismatch means every wallet signature is rejected by the network # the API is watching, with no error that names the cause. NEXT_PUBLIC_STELLAR_NETWORK=public +# Leaving this unset on a production build has actually broken production +# before (docs/FIXLOG.md, BUG-1.4): the code's local-dev localhost fallback +# got baked into the deployed bundle, so every visitor's browser silently +# tried to reach localhost on their OWN machine. A production build with +# this unset now fails loudly in the browser instead - set it and rebuild. NEXT_PUBLIC_API_URL=https:// API_URL=https:// # Must mirror OFFRAMP above. "none" hides the cash-out button, the KYC panel diff --git a/apps/web/lib/api.ts b/apps/web/lib/api.ts index 97c6f04db..c51059cd5 100644 --- a/apps/web/lib/api.ts +++ b/apps/web/lib/api.ts @@ -73,7 +73,44 @@ export interface KycView { } // Browser calls go to NEXT_PUBLIC_API_URL; server-side calls fall back to API_URL. -const BROWSER_BASE = process.env.NEXT_PUBLIC_API_URL ?? "http://localhost:8787"; +// +// This has actually broken production once already (docs/FIXLOG.md, BUG-1.4, +// 2026-07-14): a Vercel build ran with NEXT_PUBLIC_API_URL unset, so this +// fallback got baked into the client bundle, and every visitor's browser +// silently tried (and failed) to reach `localhost:8787` on their own +// machine - no error naming the real cause, just "Create link" doing +// nothing. The fix that shipped afterward was procedural (a deploy-checklist +// reminder), not code - nothing here actually stopped it from recurring. +// This does: the fallback only applies outside production, and a production +// build (NODE_ENV=production) missing the variable fails loudly, in the +// browser, at load time - before any component gets a chance to issue a +// doomed request. See docs/MAINNET.md's "NEXT_PUBLIC_*" footgun section. +const DEV_FALLBACK = "http://localhost:8787"; + +const BROWSER_BASE = ((): string => { + if (process.env.NEXT_PUBLIC_API_URL) return process.env.NEXT_PUBLIC_API_URL; + if (process.env.NODE_ENV !== "production") return DEV_FALLBACK; + + // `typeof window` is a reliable environment check here (not a runtime + // toggle): Next.js produces genuinely separate server and browser + // bundles, and each evaluates this module's top level for the first time + // in its own environment - a browser bundle really does run this inside + // an actual browser. The server bundle doesn't need NEXT_PUBLIC_API_URL at + // all if API_URL is set (see apiBase() below), so it isn't punished for a + // client-only variable it never uses. + if (typeof window !== "undefined") { + throw new Error( + "NEXT_PUBLIC_API_URL is not set. This is a production build, so there is no " + + "localhost fallback - without it, every request from this browser would " + + "silently target the visitor's own machine (this exact failure has happened " + + "before - see docs/FIXLOG.md, BUG-1.4). Set NEXT_PUBLIC_API_URL and REBUILD: " + + "NEXT_PUBLIC_* values are baked in at build time, so redeploying alone will " + + "not pick up a newly-set value.", + ); + } + + return DEV_FALLBACK; +})(); export function apiBase(): string { if (typeof window === "undefined") { diff --git a/docs/MAINNET.md b/docs/MAINNET.md index bf8c8fca4..113760591 100644 --- a/docs/MAINNET.md +++ b/docs/MAINNET.md @@ -185,7 +185,22 @@ testnet one. This one is easy to miss and fails opaquely: the browser signs with the passphrase this variable selects, so leaving it unset means every wallet signature is built for testnet and rejected by the network the API is watching — -with no error message that names the cause. Also set: +with no error message that names the cause. Like every `NEXT_PUBLIC_*` +variable, this is baked into the client bundle **at build time** — changing +it and redeploying without rebuilding does nothing; the old value is still +what's in the bundle a visitor's browser downloads. + +`NEXT_PUBLIC_API_URL` has the exact same failure shape, and it has actually +happened: a Vercel build once ran with this unset, so the code's own +`http://localhost:8787` local-dev fallback got baked into the production +bundle instead, and every visitor's browser silently tried (and failed) to +reach `localhost:8787` **on their own machine** — "Create link" just did +nothing, no error naming the cause (`docs/FIXLOG.md`, BUG-1.4). The fallback +now only applies outside a production build; a production build with this +unset fails loudly in the browser instead, at load time, rather than issuing +doomed requests — but that guard only catches "unset," not "wrong region/ +wrong deployment," so still set it deliberately rather than relying on the +guard to catch a typo'd URL. Also set: - `NEXT_PUBLIC_API_URL` / `API_URL` — the mainnet API origin - `NEXT_PUBLIC_ENABLE_WALLET_PAY=true` — enable the lazy-loaded desktop wallet From 25c2e0c61093c75bac52715321bad2d6b2cf7c31 Mon Sep 17 00:00:00 2001 From: determined-001 <241968004+determined-001@users.noreply.github.com> Date: Mon, 31 Aug 2026 17:09:14 +0100 Subject: [PATCH 2/2] test(web): pin the four BROWSER_BASE branches (issue 5.9 / BUG-1.4) --- apps/web/test/api-base.test.ts | 81 ++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 apps/web/test/api-base.test.ts diff --git a/apps/web/test/api-base.test.ts b/apps/web/test/api-base.test.ts new file mode 100644 index 000000000..397972515 --- /dev/null +++ b/apps/web/test/api-base.test.ts @@ -0,0 +1,81 @@ +import { describe, it, expect, afterEach, vi } from "vitest"; + +/** + * Regression for issue 5.9 / BUG-1.4 (2026-07-14). + * + * A Vercel build ran with NEXT_PUBLIC_API_URL unset, so the `http://localhost:8787` + * local-dev fallback was baked into the client bundle and every visitor's browser + * silently tried to reach localhost on their own machine. The fix that shipped at + * the time was a deploy-checklist reminder, not code — so nothing stopped it from + * recurring. These pin the four branches of `BROWSER_BASE`. + * + * `BROWSER_BASE` is resolved at module load, so each case re-imports the module + * under a fresh environment rather than calling a function. + */ +const DEV_FALLBACK = "http://localhost:8787"; + +async function loadApiBase(): Promise { + vi.resetModules(); + const mod = await import("../lib/api"); + return mod.apiBase(); +} + +function withBrowser(present: boolean): void { + if (present) { + (globalThis as { window?: unknown }).window = globalThis; + } else { + delete (globalThis as { window?: unknown }).window; + } +} + +describe("BROWSER_BASE — a production build must not ship the localhost fallback", () => { + afterEach(() => { + vi.unstubAllEnvs(); + withBrowser(false); + vi.resetModules(); + }); + + it("uses NEXT_PUBLIC_API_URL when it is set, in production", async () => { + vi.stubEnv("NODE_ENV", "production"); + vi.stubEnv("NEXT_PUBLIC_API_URL", "https://api.example.com"); + withBrowser(true); + + await expect(loadApiBase()).resolves.toBe("https://api.example.com"); + }); + + it("keeps the localhost fallback outside production — local dev is unaffected", async () => { + vi.stubEnv("NODE_ENV", "development"); + vi.stubEnv("NEXT_PUBLIC_API_URL", ""); + withBrowser(true); + + await expect(loadApiBase()).resolves.toBe(DEV_FALLBACK); + }); + + it("throws at module load in a production browser bundle with the variable unset", async () => { + vi.stubEnv("NODE_ENV", "production"); + vi.stubEnv("NEXT_PUBLIC_API_URL", ""); + withBrowser(true); + + await expect(loadApiBase()).rejects.toThrow(/NEXT_PUBLIC_API_URL is not set/); + }); + + it("names the variable and says to rebuild, not just redeploy", async () => { + vi.stubEnv("NODE_ENV", "production"); + vi.stubEnv("NEXT_PUBLIC_API_URL", ""); + withBrowser(true); + + await expect(loadApiBase()).rejects.toThrow(/REBUILD/); + }); + + // The server bundle reaches the API via API_URL and never needs the + // NEXT_PUBLIC_ one, so it must not be punished for a client-only variable. + // Throwing here would take down prerendering during `next build`. + it("does not throw in the production server bundle — API_URL is that path's variable", async () => { + vi.stubEnv("NODE_ENV", "production"); + vi.stubEnv("NEXT_PUBLIC_API_URL", ""); + vi.stubEnv("API_URL", "https://api.example.com"); + withBrowser(false); + + await expect(loadApiBase()).resolves.toBe("https://api.example.com"); + }); +});