diff --git a/Dockerfile b/Dockerfile index 2e46c9d..9dbb1f5 100644 --- a/Dockerfile +++ b/Dockerfile @@ -35,14 +35,27 @@ ENV NEXT_TELEMETRY_DISABLED=1 ENV PORT=3000 ENV HOSTNAME="0.0.0.0" -# Session-signing key. sessionToken.ts throws when this is unset under -# NODE_ENV=production, which is why /api/auth/login answered 500 on valid -# credentials while still returning 401/400 correctly on bad ones. +# ── Runtime configuration: supplied by the orchestrator, never baked in ── # -# This value is in git: anyone who can read the repo can forge a session -# cookie for any user. Rotate by replacing it here (invalidates live -# sessions), or move it to a Dokploy env var, which overrides this line. -ENV AUTH_SECRET=a0123c7b1508b647cf0f3985ac95644f520d3bcb2695c96a85c3f5db8b0461da +# Two variables are REQUIRED at runtime and are deliberately absent from this +# image. Set them as Dokploy environment variables / secrets: +# +# AUTH_SECRET signs the session cookie and encrypts the platform token +# bundle. Generate with: openssl rand -base64 48 +# LOYALY_API_BASE the Behavision API origin — https://mcp.loyaly.ai +# (NOT platform.loyaly.ai, which serves this console) +# +# AUTH_SECRET used to be an ENV line here with a literal value, which put a +# session-forging key in git: anyone who could read the repo could mint a +# cookie for any user, and every built image carried it in a layer that +# `docker history` prints. Docker's own linter flags the pattern +# (SecretsUsedInArgOrEnv). It is gone; rotate the old value. +# +# Neither is needed to BUILD. LOYALY_API_BASE is resolved on first use rather +# than at module load (see apiClient.ts), and sessionToken/tokenStore derive +# their key per call, so page-data collection never reads either one. Both are +# read on the first request that needs them, and a missing one fails loudly +# there instead of silently guessing a host or a key. # Run as a non-root user; nextjs owns nothing it does not need to write. RUN addgroup -g 1001 -S nodejs && adduser -u 1001 -S nextjs -G nodejs diff --git a/src/services/api/apiClient.ts b/src/services/api/apiClient.ts index cf9c154..03038e5 100644 --- a/src/services/api/apiClient.ts +++ b/src/services/api/apiClient.ts @@ -34,32 +34,129 @@ import type {ApiRole} from './types'; * 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 now - * refuses to run 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. + * 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'; -function resolveBase(): string { - const configured = process.env.LOYALY_API_BASE?.trim(); - if (configured) return configured.replace(/\/+$/, ''); +/** + * 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'; - if (process.env.NODE_ENV === 'production') { +/** + * 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', +}; + +function configError(detail: string): Error { + return new Error( + `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 Error( 'LOYALY_API_BASE is required in production — refusing to guess the ' + 'Loyaly platform host. Set it to https://mcp.loyaly.ai.', ); } - return DEV_API_BASE; + return (cachedBase = DEV_API_BASE); } -const BASE = resolveBase(); - -export {BASE as UPSTREAM_BASE}; +/** The upstream origin, resolved on first use. Call it; do not hoist it. */ +export {resolveBase as upstreamBase}; /** Upstream error codes this app branches on. Others pass through as strings. */ export type UpstreamCode = @@ -117,7 +214,7 @@ export interface UpstreamRequest { } function buildUrl(path: string, query?: Record): string { - const url = new URL(path.startsWith('/') ? path : `/${path}`, BASE); + const url = new URL(path.startsWith('/') ? path : `/${path}`, resolveBase()); if (query) { for (const [key, value] of Object.entries(query)) { // Undefined is "not asked for" and must not become the string