diff --git a/Dockerfile b/Dockerfile index f8b2c8f..579d94d 100644 --- a/Dockerfile +++ b/Dockerfile @@ -71,16 +71,19 @@ COPY --from=builder --chown=nextjs:nodejs /app/.next/static ./.next/static # The production environment, as a file the server reads at boot. # -# `next build` does NOT fold .env into .next/standalone — the standalone output -# carries server.js and traced node_modules, nothing else — so without this line -# the running container has no LOYALY_API_BASE and every upstream call throws -# "required in production". server.js chdirs to /app and Next calls -# loadEnvConfig on it, which is why the file belongs beside server.js at the -# WORKDIR root and not under .next/. +# Deliberately redundant, and worth keeping. `next build` already copies .env +# (and .env.production, and nothing else — see writeStandaloneDirectory in +# next/dist/build/index.js) into .next/standalone, so the line above lands one +# at /app/.env on its own. But it only does that when .env was in the BUILD +# CONTEXT, and .dockerignore excluded it until recently — which is precisely +# how images shipped with no LOYALY_API_BASE at all. # -# This does not pin the deployment: @next/env never overwrites a variable that -# is already in process.env, so anything set in Dokploy still wins over this -# file. It only removes "unset" from the set of possible states. +# This line turns that silent outcome into a loud one: exclude .env again and +# the Docker build FAILS here with "file not found" instead of producing an +# unconfigured image that starts and then rejects every sign-in. +# +# It does not pin the deployment either way: @next/env never overwrites a +# variable already present in process.env, so anything set in Dokploy wins. COPY --chown=nextjs:nodejs .env ./.env USER nextjs diff --git a/src/instrumentation.ts b/src/instrumentation.ts new file mode 100644 index 0000000..a1cd307 --- /dev/null +++ b/src/instrumentation.ts @@ -0,0 +1,100 @@ +/** + * Boot-time configuration check. + * + * ── The problem this exists to end ─────────────────────────────────────── + * Every required variable in this app is read LAZILY, on the first request + * that needs it. That is deliberate and must stay that way: `next build` + * collects page data with NODE_ENV=production and none of these variables + * present, so reading them at module scope fails the BUILD instead of the + * deployment (it did, once — "Failed to collect page data for /api/assistant", + * see apiClient.ts). + * + * The cost of lazy reads is that a misconfigured container LOOKS healthy. It + * starts, it serves the sign-in page, and the fault only appears when somebody + * tries to use it — as a 500 on a form, with the real reason buried in a + * response body. Three separate misconfigurations were diagnosed that way, one + * round trip at a time. + * + * This closes the gap without touching the lazy reads. `register()` runs once + * when a server instance starts and NEVER during a build — Next itself returns + * early when NEXT_PHASE is 'phase-production-build' (see + * server/lib/router-utils/instrumentation-globals.external.js). So the checks + * below run in exactly the situation they are about: a real server, booting, + * with a real environment. + * + * ── Why it throws ──────────────────────────────────────────────────────── + * A container missing either variable cannot serve a single authenticated + * request. Refusing to start turns that into a failed deploy with a named + * cause in the log pane, which Dokploy surfaces immediately, instead of a + * green healthcheck in front of a console nobody can sign into. It also stops + * a broken image from replacing a working one. + */ +export async function register() { + /** + * Node only. `register()` is invoked once per runtime, and src/proxy.ts makes + * this app compile an Edge one too — without this guard the same check would + * run and log twice per boot. The node server is the process that serves + * every route handler, so validating there is what matters. + */ + if (process.env.NEXT_RUNTIME !== 'nodejs') return; + + const isProduction = process.env.NODE_ENV === 'production'; + + /** + * shared/config/platformApi, NOT apiClient. apiClient is `server-only`, and + * that package resolves to a module which throws on import outside a + * react-server condition — which this bundle is not. Importing it here would + * crash every boot, correctly configured or not. + */ + const {resolvePlatformOrigin} = await import('@/shared/config/platformApi'); + const {ConfigError} = await import('@/shared/errors/configError'); + + const problems: string[] = []; + + /** + * Resolve the platform origin exactly the way a request would — same + * function, same validation, same allowlist. A check that reimplemented the + * rules would be a second source of truth and would drift. + */ + let platform: string | null = null; + try { + platform = resolvePlatformOrigin(); + } catch (err) { + if (!(err instanceof ConfigError)) throw err; + problems.push(err.message); + } + + /** + * AUTH_SECRET is checked by presence rather than by calling the signing + * functions, because those are pure and would have to be handed a throwaway + * payload to probe. Presence is the whole rule — sessionToken and tokenStore + * both refuse the development key in production and accept anything else. + */ + if (isProduction && !process.env.AUTH_SECRET) { + problems.push( + 'AUTH_SECRET is required in production — it signs the session cookie and ' + + 'encrypts the platform token bundle. Generate one with: openssl rand -base64 48', + ); + } + + if (problems.length > 0) { + // Numbered, because a container missing its environment is usually missing + // more than one variable, and fixing them one deploy at a time is the slow + // way to find that out. + const detail = problems.map((p, i) => ` ${i + 1}. ${p}`).join('\n'); + throw new ConfigError( + `refusing to start — ${problems.length} configuration problem(s):\n${detail}\n` + + '\nSet these as environment variables on the container (Dokploy → Environment). ' + + 'LOYALY_API_BASE also ships in the committed .env; AUTH_SECRET never does.', + ); + } + + // The healthy path says what it resolved, so a log reader can confirm the + // host WITHOUT having to trigger a request. Never prints AUTH_SECRET, only + // whether one was supplied. + console.log( + `[loyaly] config ok — platform ${platform}, ` + + `auth secret ${process.env.AUTH_SECRET ? 'set' : 'using development key'}, ` + + `NODE_ENV=${process.env.NODE_ENV}`, + ); +} diff --git a/src/services/api/apiClient.ts b/src/services/api/apiClient.ts index bc6eaeb..7315939 100644 --- a/src/services/api/apiClient.ts +++ b/src/services/api/apiClient.ts @@ -19,152 +19,17 @@ import 'server-only'; * is one API; this file is just the web console's way in. */ -import {ConfigError} from '@/shared/errors/configError'; +import {resolvePlatformOrigin} from '@/shared/config/platformApi'; import type {ApiRole} from './types'; /** - * Server-side only — deliberately NOT NEXT_PUBLIC. Publishing the platform - * host would let a browser bypass the BFF, which is the whole point of it. - * - * ── Why there is no remote fallback ────────────────────────────────────── - * This used to default to `https://platform.loyaly.ai`, which is NOT the - * Behavision API — that host serves this very console. Measured: it answers - * `GET /api/auth/me` with the console's own 404 HTML page, and a login POST - * with the console's own `{error:{code,message}}` envelope rather than the - * platform's flat `{error,message}`. So an unset variable did not fail; it - * quietly pointed the BFF at its own origin, and every upstream call became a - * request the console made to itself. - * - * A wrong host that *works* is worse than a startup failure, so production - * refuses to SERVE without the variable — the same stance `tokenStore.ts` - * takes on AUTH_SECRET, and for the same reason. Development falls back to the - * local backend, which is the only host a dev machine can usefully mean. - * - * local http://127.0.0.1:8088 - * production https://mcp.loyaly.ai - * - * ── Why this is resolved lazily and not at module scope ────────────────── - * It used to be `const BASE = resolveBase()`, evaluated the moment any module - * imported this one. That broke `next build`: the "Collecting page data" step - * imports every route module, the Docker builder stage sets NODE_ENV=production, - * and LOYALY_API_BASE is a RUNTIME value that is not present while building an - * image. So the guard fired against the build instead of against a - * misconfigured server, and the deploy failed with "Failed to collect page data - * for /api/assistant". - * - * Deferring to first use draws the line where it belongs: building an image - * needs no platform host, serving a request does. `tokenStore.key()` is a - * function for exactly this reason — this now matches it rather than only - * claiming to. The result is memoised, so the environment is read once per - * process and a healthy server pays nothing per request. + * The platform origin and its validation live in shared/config/platformApi, + * NOT here, so that src/instrumentation.ts can run the same check at boot + * without importing this `server-only` module. See that file for the rules. */ -const DEV_API_BASE = 'http://127.0.0.1:8088'; - -/** - * The only origin that serves the Loyaly platform API in production. - * - * Production is an allowlist of exactly one entry rather than a shape check, - * because "looks like a URL" is what let the wrong host through before. A new - * environment — staging, a regional deployment — is a deliberate line added - * here, not something a typo in a dashboard can invent. - */ -const PRODUCTION_API_ORIGIN = 'https://mcp.loyaly.ai'; - -/** - * Hosts that are definitely NOT the API, and why. - * - * Rejected in EVERY environment, development included: this is not a - * production-hardening rule, it is a statement of fact about what the host - * serves. Naming the reason matters — "rejected" alone sends somebody looking - * for a firewall or a DNS problem, when the actual fix is one word in a - * variable. - */ -const KNOWN_WRONG_HOSTS: Record = { - 'platform.loyaly.ai': - 'serves this console, not the Loyaly API — pointing the BFF there makes ' + - 'it call its own origin', -}; - -/** - * `buildUrl` — which resolves this variable — is called OUTSIDE the try block - * that turns a failed fetch into `UpstreamError(0, 'network')`. It used to be - * inside it, which made "nobody set LOYALY_API_BASE" indistinguishable from - * "the platform is down" at every call site, and had the login route report - * both as 502 platform_unreachable. - */ -function configError(detail: string): ConfigError { - return new ConfigError( - `LOYALY_API_BASE is invalid: ${detail}. ` + - `Set it to ${PRODUCTION_API_ORIGIN} in production, or ${DEV_API_BASE} locally.`, - ); -} - -/** - * Validate a configured value and reduce it to an origin. - * - * The path is dropped on purpose rather than preserved: `new URL(path, base)` - * has always discarded a base path, so a value like `https://host/v1` never - * did what whoever wrote it expected. Returning the origin makes that visible - * instead of silently ignored. - */ -function validateBase(raw: string, isProduction: boolean): string { - let url: URL; - try { - url = new URL(raw); - } catch { - throw configError(`"${raw}" is not an absolute URL`); - } - - if (url.protocol !== 'https:' && url.protocol !== 'http:') { - throw configError(`"${url.protocol}" is not an http(s) URL`); - } - - const wrong = KNOWN_WRONG_HOSTS[url.hostname]; - if (wrong) throw configError(`${url.hostname} ${wrong}`); - - if (isProduction) { - if (url.origin !== PRODUCTION_API_ORIGIN) { - throw configError( - `${url.origin} is not a supported production API host`, - ); - } - return url.origin; - } - - /** - * Development stays permissive by design. A dev legitimately points this at - * a LAN address, a tunnel or a container host, and breaking that to enforce - * a production rule would cost more than it protects — nothing a dev machine - * reaches is production. The known-wrong list above still applies. - */ - return url.origin; -} - -let cachedBase: string | null = null; - -function resolveBase(): string { - if (cachedBase !== null) return cachedBase; - - const isProduction = process.env.NODE_ENV === 'production'; - const configured = process.env.LOYALY_API_BASE?.trim(); - - if (configured) { - // NOT cached before validating: an invalid value must throw on every - // request, the same way a missing one does. - return (cachedBase = validateBase(configured, isProduction)); - } - - if (isProduction) { - throw new ConfigError( - 'LOYALY_API_BASE is required in production — refusing to guess the ' + - 'Loyaly platform host. Set it to https://mcp.loyaly.ai.', - ); - } - return (cachedBase = DEV_API_BASE); -} /** The upstream origin, resolved on first use. Call it; do not hoist it. */ -export {resolveBase as upstreamBase}; +export {resolvePlatformOrigin as upstreamBase}; /** Upstream error codes this app branches on. Others pass through as strings. */ export type UpstreamCode = @@ -222,7 +87,7 @@ export interface UpstreamRequest { } function buildUrl(path: string, query?: Record): string { - const url = new URL(path.startsWith('/') ? path : `/${path}`, resolveBase()); + const url = new URL(path.startsWith('/') ? path : `/${path}`, resolvePlatformOrigin()); if (query) { for (const [key, value] of Object.entries(query)) { // Undefined is "not asked for" and must not become the string diff --git a/src/shared/config/platformApi.ts b/src/shared/config/platformApi.ts new file mode 100644 index 0000000..8d69f9b --- /dev/null +++ b/src/shared/config/platformApi.ts @@ -0,0 +1,167 @@ +/** + * Where the Loyaly platform API lives, and the rules about what may be called + * one. Pure configuration resolution: no I/O, no crypto, no `server-only`. + * + * ── Why it is not in apiClient ─────────────────────────────────────────── + * apiClient is `server-only`, and that package resolves to a module which + * THROWS ON IMPORT outside a react-server condition. src/instrumentation.ts + * runs this same validation at boot and is NOT compiled in that condition, so + * importing apiClient from it would crash the server on start — for every + * deployment, correctly configured or not. + * + * Splitting it also keeps one source of truth: the boot check and the request + * path call the SAME function against the SAME allowlist, so a check that + * passes at startup cannot be contradicted by the first request. + */ + +import {ConfigError} from '@/shared/errors/configError'; + +/** + * Server-side only — deliberately NOT NEXT_PUBLIC. Publishing the platform + * host would let a browser bypass the BFF, which is the whole point of it. + * + * ── Why there is no remote fallback ────────────────────────────────────── + * This used to default to `https://platform.loyaly.ai`, which is NOT the + * Behavision API — that host serves this very console. Measured: it answers + * `GET /api/auth/me` with the console's own 404 HTML page, and a login POST + * with the console's own `{error:{code,message}}` envelope rather than the + * platform's flat `{error,message}`. So an unset variable did not fail; it + * quietly pointed the BFF at its own origin, and every upstream call became a + * request the console made to itself. + * + * A wrong host that *works* is worse than a startup failure, so production + * refuses to SERVE without the variable — the same stance `tokenStore.ts` + * takes on AUTH_SECRET, and for the same reason. Development falls back to the + * local backend, which is the only host a dev machine can usefully mean. + * + * local http://127.0.0.1:8088 + * production https://mcp.loyaly.ai + * + * ── Why this is resolved lazily and not at module scope ────────────────── + * It used to be `const BASE = resolveBase()`, evaluated the moment any module + * imported this one. That broke `next build`: the "Collecting page data" step + * imports every route module, the Docker builder stage sets NODE_ENV=production, + * and LOYALY_API_BASE is a RUNTIME value that is not present while building an + * image. So the guard fired against the build instead of against a + * misconfigured server, and the deploy failed with "Failed to collect page data + * for /api/assistant". + * + * Deferring to first use draws the line where it belongs: building an image + * needs no platform host, serving a request does. `tokenStore.key()` is a + * function for exactly this reason — this now matches it rather than only + * claiming to. The result is memoised, so the environment is read once per + * process and a healthy server pays nothing per request. + */ +const DEV_API_BASE = 'http://127.0.0.1:8088'; + +/** + * The only origin that serves the Loyaly platform API in production. + * + * Production is an allowlist of exactly one entry rather than a shape check, + * because "looks like a URL" is what let the wrong host through before. A new + * environment — staging, a regional deployment — is a deliberate line added + * here, not something a typo in a dashboard can invent. + */ +const PRODUCTION_API_ORIGIN = 'https://mcp.loyaly.ai'; + +/** + * Hosts that are definitely NOT the API, and why. + * + * Rejected in EVERY environment, development included: this is not a + * production-hardening rule, it is a statement of fact about what the host + * serves. Naming the reason matters — "rejected" alone sends somebody looking + * for a firewall or a DNS problem, when the actual fix is one word in a + * variable. + */ +const KNOWN_WRONG_HOSTS: Record = { + 'platform.loyaly.ai': + 'serves this console, not the Loyaly API — pointing the BFF there makes ' + + 'it call its own origin', +}; + +/** + * `buildUrl` — which resolves this variable — is called OUTSIDE the try block + * that turns a failed fetch into `UpstreamError(0, 'network')`. It used to be + * inside it, which made "nobody set LOYALY_API_BASE" indistinguishable from + * "the platform is down" at every call site, and had the login route report + * both as 502 platform_unreachable. + */ +function configError(detail: string): ConfigError { + return new ConfigError( + `LOYALY_API_BASE is invalid: ${detail}. ` + + `Set it to ${PRODUCTION_API_ORIGIN} in production, or ${DEV_API_BASE} locally.`, + ); +} + +/** + * Validate a configured value and reduce it to an origin. + * + * The path is dropped on purpose rather than preserved: `new URL(path, base)` + * has always discarded a base path, so a value like `https://host/v1` never + * did what whoever wrote it expected. Returning the origin makes that visible + * instead of silently ignored. + */ +function validateBase(raw: string, isProduction: boolean): string { + let url: URL; + try { + url = new URL(raw); + } catch { + throw configError(`"${raw}" is not an absolute URL`); + } + + if (url.protocol !== 'https:' && url.protocol !== 'http:') { + throw configError(`"${url.protocol}" is not an http(s) URL`); + } + + const wrong = KNOWN_WRONG_HOSTS[url.hostname]; + if (wrong) throw configError(`${url.hostname} ${wrong}`); + + if (isProduction) { + if (url.origin !== PRODUCTION_API_ORIGIN) { + throw configError( + `${url.origin} is not a supported production API host`, + ); + } + return url.origin; + } + + /** + * Development stays permissive by design. A dev legitimately points this at + * a LAN address, a tunnel or a container host, and breaking that to enforce + * a production rule would cost more than it protects — nothing a dev machine + * reaches is production. The known-wrong list above still applies. + */ + return url.origin; +} + +let cachedBase: string | null = null; + +function resolveBase(): string { + if (cachedBase !== null) return cachedBase; + + const isProduction = process.env.NODE_ENV === 'production'; + const configured = process.env.LOYALY_API_BASE?.trim(); + + if (configured) { + // NOT cached before validating: an invalid value must throw on every + // request, the same way a missing one does. + return (cachedBase = validateBase(configured, isProduction)); + } + + if (isProduction) { + throw new ConfigError( + 'LOYALY_API_BASE is required in production — refusing to guess the ' + + 'Loyaly platform host. Set it to https://mcp.loyaly.ai.', + ); + } + return (cachedBase = DEV_API_BASE); +} + +/** + * The upstream origin, resolved on first use and memoised. Call it; do not + * hoist it — see the note on lazy resolution above. + */ +export {resolveBase as resolvePlatformOrigin}; + +/** The one production origin, for messages that need to name it. */ +export {PRODUCTION_API_ORIGIN};