diff --git a/.env b/.env index 64cb0e0..303636b 100644 --- a/.env +++ b/.env @@ -32,6 +32,17 @@ # the variable there makes the BFF call its own origin, which fails in a way # that looks like a broken login form rather than a misconfiguration. # apiClient.ts rejects that hostname by name for exactly this reason. +# +# NOT REQUIRED in production any more. Production accepts exactly one origin, so +# an unset variable could never have meant another one, and platformApi resolves +# it to that origin on its own. It stays here so `docker run` is self-describing +# and so development has something to read. +# +# Why that change was needed: @next/env only fills a variable that is ABSENT. +# Verified against the installed copy — a real environment variable set to the +# EMPTY STRING stays empty and this file is NOT consulted. So one blank field in +# a dashboard silently defeated the value below and took production down with +# "LOYALY_API_BASE is required in production". LOYALY_API_BASE=https://mcp.loyaly.ai # Browser → this app's own BFF routes, which are same-origin. Empty is correct @@ -45,15 +56,22 @@ LOYALY_API_BASE=https://mcp.loyaly.ai # Changing it in Dokploy's environment panel would do nothing without a rebuild. NEXT_PUBLIC_API_BASE= -# AUTH_SECRET is deliberately NOT in this file. +# AUTH_SECRET is deliberately NOT in this file. It is the ONLY variable this +# deployment requires, and the only one that cannot ship. # # It signs the session cookie and encrypts the platform token bundle, so a # value committed here is a session-forging key in git — anyone who can read # the repo could mint a cookie for any user. It was already removed from the # Dockerfile once for that reason; do not reintroduce it here. # -# Set it as a Dokploy environment variable / secret. Production refuses to sign -# sessions without it. Generate with: openssl rand -hex 32 +# Set it as a Dokploy environment variable in the RUNTIME panel — a value set as +# a BUILD argument is not present when the server runs, which looks exactly like +# never having set it. Alternatively mount the value and set AUTH_SECRET_FILE to +# its path (the Docker/Swarm secret convention); AUTH_SECRET wins if both exist. +# +# Production refuses to sign sessions without it. Generate with: +# +# openssl rand -hex 32 # # Hex, not base64: a base64 value ends in '=' and can contain '+' and '/', and # an environment editor that splits a line on the first '=' can store that diff --git a/.env.example b/.env.example index 0ced718..a2ab085 100644 --- a/.env.example +++ b/.env.example @@ -18,16 +18,20 @@ # the variable there makes the BFF call its own origin, which fails in a way # that looks like a broken login form rather than a misconfiguration. # -# There is no remote fallback: production refuses to serve without this set. -# That is why it is committed in `.env` rather than left to a dashboard. +# Production no longer requires this: it accepts exactly one origin, so an unset +# value can only have meant that one, and platformApi resolves it. Any OTHER +# host set explicitly is still rejected. Locally it is worth setting, because a +# dev machine legitimately means a different address. LOYALY_API_BASE=http://127.0.0.1:8088 # Signs the session cookie and encrypts the platform token bundle. # -# The ONLY variable that is a real secret, and the only one production takes -# solely from the environment — it is in no committed file, by design. Set it -# as a Dokploy environment variable / secret. Locally, any string works; leave -# it blank and a development key is used. +# The ONLY variable production requires, the only real secret, and the only one +# taken solely from the environment — it is in no committed file, by design. +# Set it as a Dokploy environment variable in the RUNTIME panel (a build +# argument is not present at runtime), or mount it and set AUTH_SECRET_FILE to +# its path. Locally, any string works; leave it blank and a development key is +# used. # # Generate with: openssl rand -hex 32 (hex, not base64 — a trailing '=' can be # mangled by a dashboard env editor that splits on the first '=') diff --git a/Dockerfile b/Dockerfile index 1710ee0..d82e645 100644 --- a/Dockerfile +++ b/Dockerfile @@ -37,41 +37,62 @@ ENV HOSTNAME="0.0.0.0" # ── Runtime configuration ──────────────────────────────────────────────── # -# Two variables are required to SERVE a request. Neither is required 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 reads neither. -# -# LOYALY_API_BASE the Behavision API origin — https://mcp.loyaly.ai -# (NOT platform.loyaly.ai, which serves this console) -# → SHIPPED, in the .env copied below. Not a secret. +# EXACTLY ONE variable must be supplied to this container. That is the whole +# deployment contract, and it is one because everything else either ships in +# the image or can only have one legal value. # # AUTH_SECRET signs the session cookie and encrypts the platform token # bundle. Generate with: openssl rand -hex 32 -# → NOT shipped. Set it as a Dokploy secret. +# → NOT shipped, and never can be: a secret in the image is +# readable with `docker history`, and a secret in git is a +# session-forging key for anyone who can read the repo. +# → Set it in Dokploy → Environment (the RUNTIME panel — a +# BUILD argument is not present when the server runs), or +# mount it and set AUTH_SECRET_FILE to its path. # -# Hex rather than base64 on purpose. `openssl rand -base64 48` ends in '=' and -# may contain '+' and '/'. Pasted into a dashboard field or a KEY=VALUE env -# editor that splits on the first '=', that value can be stored truncated — or -# not at all — and the result is indistinguishable from never having set it. -# Hex is [0-9a-f] only, so there is nothing for a parser to mangle. 32 bytes is -# 256 bits, more than the HMAC and the AES-256 key derived from it need. +# AUTH_SECRET_FILE optional alternative: a path to read the secret from, the +# standard Docker/Swarm secret convention. AUTH_SECRET wins +# when both are set. Use this when a dashboard field mangles +# the value. # -# A container started WITHOUT AUTH_SECRET no longer dies. It boots, prints the -# missing variable on stderr, answers 503 with `x-loyaly-config: misconfigured` -# on every request, and fails the HEALTHCHECK below. Exiting instead is what -# made this fault present as a bare 502 Bad Gateway from the reverse proxy on -# every url — /favicon.ico first, in the browser console — with the one line -# that explained it trapped inside a restart-looping container. See -# src/instrumentation-node.ts. +# LOYALY_API_BASE no longer required. Production accepts exactly one origin +# (https://mcp.loyaly.ai), so an unset variable could never +# have meant anything else; shared/config/platformApi now +# resolves it to that origin. Setting it to any OTHER host +# is still rejected by name. It also still ships in the .env +# copied below, which keeps `docker run` self-describing. # -# 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. That is why the -# split above exists — "inject everything" also meant injecting the one value -# that is public knowledge, and forgetting it took the console down. +# Hex rather than base64 for the secret, on purpose. `openssl rand -base64 48` +# ends in '=' and may contain '+' and '/'. Pasted into a dashboard field or a +# KEY=VALUE editor that splits on the first '=', that value can be stored +# truncated — or not at all — and the result is indistinguishable from never +# having set it. Hex is [0-9a-f] only, so there is nothing for a parser to +# mangle. 32 bytes is 256 bits, more than the HMAC and the AES-256 key derived +# from it need. +# +# A container started without the secret does not die and does not 502. It +# boots, names the missing variable on stderr (including any environment +# variable whose NAME looks like a near-miss for AUTH_SECRET, which is the one +# cause invisible from a dashboard), and answers 503 with +# `x-loyaly-config: misconfigured` on every gated request. +# +# ── Where the secret must be set in Dokploy ────────────────────────────── +# The "Environment Variables" tab. NOT "Build Arguments" and NOT "Build +# Secrets": Dokploy's own documentation is explicit that both of those are +# build-time only and are absent from the running container, so a secret placed +# there is indistinguishable, from inside the container, from never having been +# set at all. The boot log says which of the two happened. +# +# ── Two probe endpoints, deliberately separate ─────────────────────────── +# /api/health LIVENESS — 200 whenever the process answers. Safe to probe +# unconditionally; can never remove a serving +# container from rotation. +# /api/ready READINESS — 503 while a required variable is missing. Meant +# for a DEPLOY gate, in Dokploy → Advanced → Swarm +# Settings, paired with Update Config +# `Order: start-first` + `FailureAction: rollback` +# so a misconfigured new task is rolled back while +# the previous good one keeps serving. # 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/app/api/health/route.ts b/src/app/api/health/route.ts index b195082..7c645e6 100644 --- a/src/app/api/health/route.ts +++ b/src/app/api/health/route.ts @@ -1,50 +1,61 @@ import {configStatus} from '@/shared/config/configCheck'; /** - * GET /api/health — can this container serve? + * GET /api/health — LIVENESS. "Is this process answering HTTP?" * - * ── Why a container needs this ─────────────────────────────────────────── - * The boot check used to answer the same question by killing the process, on - * the reasoning that a dead container is the only signal a platform cannot - * misread. It is also the only signal a BROWSER cannot read: the reverse proxy - * in front of it had nothing to connect to and returned 502 for every url, - * which is what a missing AUTH_SECRET looked like from the outside. + * Always 200 when the server can respond at all. It does NOT fail on a + * configuration problem, and that is the entire point of separating it from + * /api/ready. * - * This is the half of that trade worth keeping. The container stays up and - * explains itself, while the Dockerfile's HEALTHCHECK polls this route and - * drives the container `unhealthy` when it answers 503 — so a misconfigured - * deploy still cannot present itself as a working one. + * ── Why the split exists ───────────────────────────────────────────────── + * This route used to answer 503 while a required variable was missing, which + * is READINESS semantics living on the name every orchestrator probes by + * default. A Docker HEALTHCHECK was pointed at it for one commit, and because + * Dokploy runs applications as Docker Swarm services, Swarm did not merely + * report the task unhealthy — it removed it from the service load balancer and + * rescheduled it. Traefik then had no backend and answered 502 Bad Gateway on + * every url: the container was up, serving a 503 that named the fault, and + * nothing could reach it to read that 503. * - * ── What it deliberately does not say ──────────────────────────────────── - * Anonymous and public, so it publishes a state and a count, never the problem - * messages: those name environment variables, which is operator information - * (configError.ts states the rule; the login route follows it too). The names - * are printed once at boot, in the container log, where only an operator sees - * them. + * Splitting the two makes that choice explicit instead of accidental. A probe + * wired here can never remove a serving container from rotation. A probe wired + * to /api/ready can, deliberately, and is the right thing for a DEPLOY gate + * (Swarm Order=start-first + FailureAction=rollback) where failing keeps the + * PREVIOUS healthy task serving. * - * Exempt from the proxy's session gate — see HEALTH_PATH in src/proxy.ts — or - * an unauthenticated healthcheck would read 401 as "unhealthy" on a perfectly - * good container. + * The body still reports configuration, so this one endpoint answers both + * "is it alive?" and "why is it unhappy?" — it just never lies about the first + * to signal the second. */ export const dynamic = 'force-dynamic'; export function GET(): Response { - const {problems} = configStatus(); - const healthy = problems.length === 0; + const {problems, authSecretSource, authSecretLength} = configStatus(); return Response.json( { - status: healthy ? 'ok' : 'misconfigured', - // A count, not the messages. Enough to tell "one variable missing" from - // "this container has nothing set at all" without publishing which. + status: 'alive', + /** + * Reported, never enforced here. `ok` vs `misconfigured` tells a human + * what is wrong without letting a probe tear the container down for it. + */ + configuration: problems.length === 0 ? 'ok' : 'misconfigured', problems: problems.length, + /** + * Source and length only — never the value. `env` / `file` / + * `development` / `missing` plus a length distinguishes "not set" from + * "set but truncated", which is the distinction that costs the most time + * to make from outside a container. A length is not a meaningful + * disclosure about a 256-bit random value. + */ + authSecret: {source: authSecretSource, length: authSecretLength}, }, { - status: healthy ? 200 : 503, + status: 200, headers: { 'cache-control': 'no-store', - 'x-loyaly-config': healthy ? 'ok' : 'misconfigured', + 'x-loyaly-config': problems.length === 0 ? 'ok' : 'misconfigured', }, }, ); diff --git a/src/app/api/ready/route.ts b/src/app/api/ready/route.ts new file mode 100644 index 0000000..9748712 --- /dev/null +++ b/src/app/api/ready/route.ts @@ -0,0 +1,54 @@ +import {configStatus} from '@/shared/config/configCheck'; + +/** + * GET /api/ready — READINESS. "Can this container serve real traffic?" + * + * 200 only when every required variable is present and acceptable; 503 + * otherwise. Unlike /api/health (liveness), this one is MEANT to fail. + * + * ── What to point at it, and what not to ───────────────────────────────── + * Point a DEPLOY gate here: Dokploy → Advanced → Swarm Settings, a Health + * Check whose Test hits this path, with Update Config `Order: start-first` and + * `FailureAction: rollback`. A newly deployed task that is missing AUTH_SECRET + * then never becomes healthy, never replaces the running one, and is rolled + * back — the previous good version keeps serving, and the broken deploy is + * rejected before any production traffic reaches it. That is the contract this + * endpoint exists for. + * + * Understand the one case it cannot save: if NO healthy task exists to fall + * back to — a first deploy, or a service that is already broken — then a gate + * here means nothing is in rotation and Traefik answers 502. A readiness gate + * cannot invent a working version. Get production healthy FIRST, then turn the + * gate on; it protects every deploy after that. + * + * Do NOT put this path in a Dockerfile HEALTHCHECK. That applies to the + * container unconditionally, including when there is no predecessor, which is + * exactly how a 502 was recreated once already. The deploy gate belongs in + * Dokploy's Swarm settings, where start-first and rollback give it somewhere + * safe to fail to. + */ + +export const dynamic = 'force-dynamic'; + +export function GET(): Response { + const {problems, authSecretSource, authSecretLength} = configStatus(); + const ready = problems.length === 0; + + return Response.json( + { + status: ready ? 'ready' : 'not_ready', + // A count, not the messages: this is public and anonymous, and the + // messages name environment variables, which is operator information. + // The names are printed once at boot, in the container log. + problems: problems.length, + authSecret: {source: authSecretSource, length: authSecretLength}, + }, + { + status: ready ? 200 : 503, + headers: { + 'cache-control': 'no-store', + 'x-loyaly-config': ready ? 'ok' : 'misconfigured', + }, + }, + ); +} diff --git a/src/features/auth/services/sessionToken.ts b/src/features/auth/services/sessionToken.ts index 24d5bb8..189b33d 100644 --- a/src/features/auth/services/sessionToken.ts +++ b/src/features/auth/services/sessionToken.ts @@ -1,5 +1,5 @@ import {createHmac, timingSafeEqual} from 'node:crypto'; -import {ConfigError} from '@/shared/errors/configError'; +import {authSecret} from '@/shared/config/authSecret'; /** * The session cookie format, and the only place that knows how to mint or @@ -27,23 +27,15 @@ import {ConfigError} from '@/shared/errors/configError'; * name, never anything secret. */ -const DEV_SECRET = 'loyaly-dev-secret-not-for-production'; - /** - * Falls back to a constant in development so a fresh clone runs with no setup. - * In production a missing AUTH_SECRET is fatal rather than silently signing - * every session with a value that is checked into git. + * The signing key comes from shared/config/authSecret and nowhere else. + * + * It used to be read here from process.env directly, with tokenStore.ts doing + * the same a second time under slightly different rules — one accepted a + * whitespace-only value that the other rejected. Signing and encryption now + * resolve through one function, so the identity cookie and the token bundle + * cannot disagree about whether this deployment has a secret. */ -function secret(): string { - const fromEnv = process.env.AUTH_SECRET; - if (fromEnv) return fromEnv; - if (process.env.NODE_ENV === 'production') { - throw new ConfigError( - 'AUTH_SECRET is required in production — refusing to sign sessions with the development key.', - ); - } - return DEV_SECRET; -} export const SESSION_COOKIE = 'loyaly_session'; @@ -68,7 +60,7 @@ function b64url(input: Buffer | string): string { } function sign(data: string): string { - return createHmac('sha256', secret()).update(data).digest('base64url'); + return createHmac('sha256', authSecret()).update(data).digest('base64url'); } export function createSessionToken( diff --git a/src/features/auth/services/tokenStore.ts b/src/features/auth/services/tokenStore.ts index fd8952f..a2ce91e 100644 --- a/src/features/auth/services/tokenStore.ts +++ b/src/features/auth/services/tokenStore.ts @@ -1,6 +1,6 @@ import 'server-only'; import {createCipheriv, createDecipheriv, createHash, randomBytes} from 'node:crypto'; -import {ConfigError} from '@/shared/errors/configError'; +import {authSecret} from '@/shared/config/authSecret'; /** * Where the platform's access and refresh tokens live. @@ -20,19 +20,17 @@ import {ConfigError} from '@/shared/errors/configError'; * Both cookies are httpOnly, so neither is reachable from JavaScript at all. */ -const DEV_SECRET = 'loyaly-dev-secret-not-for-production'; - +/** + * Derived from the SAME resolved secret the identity cookie is signed with — + * see shared/config/authSecret. Both used to read process.env separately, which + * meant a value this module accepted could be one sessionToken rejected. + * + * scrypt would be better against an offline attack on the secret itself, but + * this key is derived per process from a value that is already high-entropy and + * never transmitted; sha256 keeps cookie reads off the event loop. + */ function key(): Buffer { - const fromEnv = process.env.AUTH_SECRET; - if (!fromEnv && process.env.NODE_ENV === 'production') { - throw new ConfigError( - 'AUTH_SECRET is required in production — refusing to encrypt platform tokens with the development key.', - ); - } - // scrypt would be better against an offline attack on the secret itself, but - // this key is derived per process from a value that is already high-entropy - // and never transmitted; sha256 keeps cookie reads off the event loop. - return createHash('sha256').update(fromEnv ?? DEV_SECRET).digest(); + return createHash('sha256').update(authSecret()).digest(); } export const TOKEN_COOKIE = 'loyaly_tokens'; diff --git a/src/instrumentation-node.ts b/src/instrumentation-node.ts index 8d6005d..0522a97 100644 --- a/src/instrumentation-node.ts +++ b/src/instrumentation-node.ts @@ -32,18 +32,33 @@ import {configStatus} from '@/shared/config/configCheck'; * without taking away the thing that tells you what broke. */ export async function checkConfiguration() { - const {problems, platform} = configStatus(); + const {problems, platform, authSecretSource, authSecretLength, nearMissNames} = + configStatus(); if (problems.length > 0) { // Numbered, because a container missing its environment is usually missing // more than one variable, and finding that out one deploy at a time is the // slow way. const detail = problems.map((p, i) => ` ${i + 1}. ${p}`).join('\n'); + /** + * The near-miss line is the one that can end a guessing loop from outside + * the container. "AUTH_SECRET did not arrive" has three causes that look + * identical in a dashboard — never set, set as a BUILD argument rather than + * a runtime one, or set under a slightly different name — and only the last + * is visible from in here. Printing it costs one line and rules one out. + */ + const nearMiss = + nearMissNames.length > 0 + ? `\nEnvironment variables present with AUTH_SECRET-like names (names only): ` + + `${nearMissNames.join(', ')} — check for a typo in the variable NAME.\n` + : ''; + console.error( `\n[loyaly] configuration problem — ${problems.length} problem(s); ` + - `serving 503 until they are fixed:\n${detail}\n` + - '\nSet these as environment variables on the container (Dokploy → Environment), ' + - 'then redeploy. LOYALY_API_BASE also ships in the committed .env; AUTH_SECRET never does.\n', + `serving 503 until they are fixed:\n${detail}\n${nearMiss}` + + '\nSet AUTH_SECRET in Dokploy → Environment (the RUNTIME panel; a build ' + + 'argument is not present at runtime), or mount the value and point ' + + 'AUTH_SECRET_FILE at its path. Then redeploy.\n', ); return; } @@ -51,14 +66,13 @@ export async function checkConfiguration() { // 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. - const secret = process.env.AUTH_SECRET; console.log( `[loyaly] config ok — platform ${platform}, ` + - // The LENGTH, never the value. `openssl rand -hex 32` gives 64 - // characters, so anything much shorter here is a secret that arrived + // SOURCE and LENGTH, never the value. `openssl rand -hex 32` gives 64 + // characters, so a much smaller number here is a secret that arrived // truncated — which otherwise presents as sessions that mysteriously do // not verify, with nothing in any log to suggest why. - `auth secret ${secret ? `set (${secret.length} chars)` : 'using development key'}, ` + + `auth secret from ${authSecretSource} (${authSecretLength} chars), ` + `NODE_ENV=${process.env.NODE_ENV}`, ); } diff --git a/src/proxy.ts b/src/proxy.ts index 2ef0def..020a4f8 100644 --- a/src/proxy.ts +++ b/src/proxy.ts @@ -33,10 +33,17 @@ const LOGIN_PATH = '/login'; const HOME_PATH = '/dashboard'; /** - * Never gated, by session or by configuration — it is how a container reports - * which of those two states it is in. See src/app/api/health/route.ts. + * Never gated, by session or by configuration — they are how a container + * reports which of those two states it is in, and a probe that had to + * authenticate could not report anything. + * + * /api/health liveness — always 200 while the process answers + * /api/ready readiness — 503 while a required variable is missing + * + * See those two route files for which one a deploy gate should probe and why + * pointing a Dockerfile HEALTHCHECK at either is what recreated a 502. */ -const HEALTH_PATH = '/api/health'; +const PROBE_PATHS = new Set(['/api/health', '/api/ready']); /** * A deployment that cannot serve says so, in one place, before anything that @@ -99,9 +106,9 @@ const PUBLIC_PATHS = new Set([LOGIN_PATH]); export function proxy(request: NextRequest): NextResponse { const {pathname, search} = request.nextUrl; - // Answers in both states — that is the entire point of it — so it is let + // Answer in both states — that is the entire point of them — so they are let // through ahead of the configuration gate and the session gate alike. - if (pathname === HEALTH_PATH) return NextResponse.next(); + if (PROBE_PATHS.has(pathname)) return NextResponse.next(); /** * Configuration first, session second. A server missing AUTH_SECRET cannot diff --git a/src/shared/config/authSecret.ts b/src/shared/config/authSecret.ts new file mode 100644 index 0000000..ae638db --- /dev/null +++ b/src/shared/config/authSecret.ts @@ -0,0 +1,146 @@ +import {readFileSync} from 'node:fs'; +import {ConfigError} from '@/shared/errors/configError'; + +/** + * The ONE place the session-signing secret is resolved. + * + * ── Why this file exists ───────────────────────────────────────────────── + * Before it, `process.env.AUTH_SECRET` was read independently in + * sessionToken.ts (which signs the identity cookie), tokenStore.ts (which + * derives the AES key for the platform token bundle) and configCheck.ts (which + * decides whether the deployment is serviceable). Three readers, three copies + * of the "is it missing?" rule, and three different messages — so a secret that + * arrived empty could satisfy one and fail another. + * + * Next 16 compiles `src/proxy.ts` for the NODE runtime (the docs shipped with + * this version: "Proxy defaults to using the Node.js runtime... Setting the + * runtime config option in Proxy will throw an error"), so every reader now + * lives in one process reading one `process.env`. They are still SEPARATE + * BUNDLES, though — `.next/server/chunks/…` holds one copy of this module for + * the proxy entry and another for the route entries — so anything derived here + * must be a pure function of the environment. A value invented in module scope + * (say, a generated fallback) would differ per bundle, the proxy would reject + * every cookie the login route signed, and /login would redirect forever. + * That is why there is no generated fallback, and why there must not be one. + * + * ── The two accepted sources ───────────────────────────────────────────── + * AUTH_SECRET the value itself. What Dokploy → Environment sets. + * AUTH_SECRET_FILE a path to read it from. The standard Docker/Swarm + * secret convention (/run/secrets/...), and the way to + * supply it when a dashboard field mangles the value. + * + * Env wins when both are set, so a dashboard override never has to fight a + * mounted file. Both are trimmed: a trailing newline is what `echo > secret` + * leaves behind, and an untrimmed newline would make the key silently differ + * from the same secret pasted into a form. + */ + +const DEV_SECRET = 'loyaly-dev-secret-not-for-production'; + +/** Where a resolved secret came from. Reported at boot; never its value. */ +export type AuthSecretSource = 'env' | 'file' | 'development' | 'missing'; + +export interface AuthSecretResolution { + secret: string | null; + source: AuthSecretSource; + /** Set when AUTH_SECRET_FILE was given but could not be read. */ + fileError: string | null; +} + +let cached: AuthSecretResolution | null = null; + +/** + * Resolve without throwing — the form the boot report and /api/health need, + * because they have to describe a broken deployment rather than fail with it. + * + * Memoised: both inputs are fixed for the life of the process, and `key()` in + * tokenStore is called on every cookie read. + */ +export function resolveAuthSecret(): AuthSecretResolution { + if (cached) return cached; + + /** + * `.trim()` before the emptiness test, because the failure being caught here + * is a value that ARRIVED but arrived blank — a dashboard field that stored + * whitespace, or a KEY=VALUE line whose value was eaten by a parser splitting + * on the first '='. An AUTH_SECRET of " " is not a configured secret, and + * accepting it would put a one-character key behind every session. + */ + const fromEnv = process.env.AUTH_SECRET?.trim(); + if (fromEnv) { + return (cached = {secret: fromEnv, source: 'env', fileError: null}); + } + + const path = process.env.AUTH_SECRET_FILE?.trim(); + if (path) { + try { + const fromFile = readFileSync(path, 'utf8').trim(); + if (fromFile) { + return (cached = {secret: fromFile, source: 'file', fileError: null}); + } + // A readable but empty file is a misconfiguration, not a missing one — + // say so, rather than reporting the path as simply absent. + return (cached = { + secret: null, + source: 'missing', + fileError: `AUTH_SECRET_FILE (${path}) is empty`, + }); + } catch (err) { + return (cached = { + secret: null, + source: 'missing', + fileError: `AUTH_SECRET_FILE (${path}) could not be read: ${ + err instanceof Error ? err.message : String(err) + }`, + }); + } + } + + /** + * Development falls back to a constant so a fresh clone runs with no setup. + * Production does not: a secret checked into git is a session-forging key, + * which is exactly why the literal that used to sit in the Dockerfile was + * removed (8b3fbab). + */ + if (process.env.NODE_ENV !== 'production') { + return (cached = {secret: DEV_SECRET, source: 'development', fileError: null}); + } + + return (cached = {secret: null, source: 'missing', fileError: null}); +} + +/** + * The secret, or a ConfigError naming what to do about it. This is what the + * signing and encryption paths call; they cannot proceed without a value and + * must not invent one. + */ +export function authSecret(): string { + const {secret, fileError} = resolveAuthSecret(); + if (secret) return secret; + throw new ConfigError(fileError ?? authSecretProblem()); +} + +/** + * The operator-facing description of what is wrong. Kept next to the resolver + * so the boot report, /api/health and the thrown error cannot describe the same + * state in three different ways. + */ +export function authSecretProblem(): string { + const {secret, fileError} = resolveAuthSecret(); + if (secret) return ''; + if (fileError) return fileError; + + const arrivedButBlank = process.env.AUTH_SECRET !== undefined; + return ( + (arrivedButBlank + ? 'AUTH_SECRET is set but empty — the variable reached the container with no value. ' + : 'AUTH_SECRET is not set — the variable never reached the container. ') + + 'It signs the session cookie and encrypts the platform token bundle, and ' + + 'production refuses to fall back to the development key. Set it on the ' + + 'container (Dokploy → Environment, the RUNTIME panel — a build argument is ' + + 'not present at runtime), or mount it and point AUTH_SECRET_FILE at the ' + + 'path. Generate one with: openssl rand -hex 32 — hex, not base64, because a ' + + "base64 value ends in '=' and an editor that splits a line on the first '=' " + + 'can store it truncated or empty.' + ); +} diff --git a/src/shared/config/configCheck.ts b/src/shared/config/configCheck.ts index cd52e06..7bab436 100644 --- a/src/shared/config/configCheck.ts +++ b/src/shared/config/configCheck.ts @@ -20,12 +20,33 @@ import {ConfigError} from '@/shared/errors/configError'; import {resolvePlatformOrigin} from '@/shared/config/platformApi'; +import { + authSecretProblem, + resolveAuthSecret, + type AuthSecretSource, +} from '@/shared/config/authSecret'; export interface ConfigStatus { /** Operator-facing messages. Empty means the server can serve. */ problems: string[]; /** The resolved upstream origin, or null when it could not be resolved. */ platform: string | null; + /** Where the session secret came from. Never the secret itself. */ + authSecretSource: AuthSecretSource; + /** Its length, for spotting a value that arrived truncated. Never the value. */ + authSecretLength: number; + /** + * Environment variable NAMES present in this container that look like a + * near-miss for AUTH_SECRET. Names only — never values. + * + * This exists because the question "why did the variable not arrive?" was + * unanswerable from outside the container, and the three real answers all + * look identical from a browser: it was never set, it was set in a build-args + * panel instead of the runtime one, or it was set under a slightly different + * name. The first two are invisible from in here; the third is not, and it is + * the one a person cannot see by re-reading their own dashboard. + */ + nearMissNames: string[]; } let cached: ConfigStatus | null = null; @@ -62,24 +83,73 @@ export function configStatus(): ConfigStatus { * inventing a throwaway payload to sign. */ /** - * `.trim()` rather than a bare falsiness check, because the failure this is - * most often covering for is a value that ARRIVED but arrived empty — a - * dashboard field that stored whitespace, or a KEY=VALUE line whose value was - * eaten. An AUTH_SECRET of " " is not a configured secret, and treating it as - * one would put a one-character key behind every session in the deployment. + * Asked of the SAME resolver that signing and encryption use, rather than + * re-reading process.env with a fourth copy of the rule. A check that decided + * "configured" by different criteria than the code doing the signing is how a + * container passes its own boot check and then fails every sign-in. */ - if (process.env.NODE_ENV === 'production' && !process.env.AUTH_SECRET?.trim()) { - problems.push( - 'AUTH_SECRET is not set — it signs the session cookie and encrypts the ' + - 'platform token bundle, and production refuses to fall back to the ' + - 'development key. Set it on the container (Dokploy → Environment). ' + - 'Generate one with: openssl rand -hex 32 — hex, not base64, because a ' + - "base64 value ends in '=' and a dashboard field that splits on the " + - 'first = can silently store a truncated or empty value.', - ); + const {secret, source} = resolveAuthSecret(); + if (!secret) problems.push(authSecretProblem()); + + return (cached = { + problems, + platform, + authSecretSource: source, + authSecretLength: secret?.length ?? 0, + nearMissNames: findNearMissNames(), + }); +} + +/** + * Names in the container's environment that resemble AUTH_SECRET without being + * it. Deliberately name-only: the values are secrets, and this is printed to a + * log and served from /api/health. + */ +const TARGET = 'AUTHSECRET'; + +/** + * Edit distance, capped. Substring matching alone catches a WRAPPED name + * (NEXT_PUBLIC_AUTH_SECRET) but not a MISTYPED one — AUTH_SECERT is a + * transposition, and transpositions and single-character slips are what people + * actually type into a dashboard field at the end of a long day. + * + * Bounded at `max`: the loop returns early once every cell in a row exceeds it, + * so an unrelated 40-character variable name costs two rows, not a full matrix. + */ +function withinEditDistance(candidate: string, max: number): boolean { + if (Math.abs(candidate.length - TARGET.length) > max) return false; + + let previous = Array.from({length: TARGET.length + 1}, (_, i) => i); + + for (let i = 1; i <= candidate.length; i++) { + const current = [i]; + let rowMin = i; + for (let j = 1; j <= TARGET.length; j++) { + const cost = candidate[i - 1] === TARGET[j - 1] ? 0 : 1; + const value = Math.min( + current[j - 1] + 1, + previous[j] + 1, + previous[j - 1] + cost, + ); + current.push(value); + if (value < rowMin) rowMin = value; + } + if (rowMin > max) return false; + previous = current; } - return (cached = {problems, platform}); + return previous[TARGET.length] <= max; +} + +function findNearMissNames(): string[] { + if (process.env.AUTH_SECRET?.trim()) return []; + + return Object.keys(process.env).filter((name) => { + if (name === 'AUTH_SECRET' || name === 'AUTH_SECRET_FILE') return false; + const squashed = name.toUpperCase().replace(/[^A-Z]/g, ''); + // Wrapped (NEXT_PUBLIC_AUTH_SECRET) or mistyped (AUTH_SECERT). + return squashed.includes(TARGET) || withinEditDistance(squashed, 2); + }); } /** True when every required variable is present and acceptable. */ diff --git a/src/shared/config/platformApi.ts b/src/shared/config/platformApi.ts index 8d69f9b..be74037 100644 --- a/src/shared/config/platformApi.ts +++ b/src/shared/config/platformApi.ts @@ -148,12 +148,29 @@ function resolveBase(): string { 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.', - ); - } + /** + * ── Why an unset variable is no longer fatal in production ─────────────── + * Production accepts exactly ONE origin (the allowlist below), so an unset + * LOYALY_API_BASE could never have meant anything other than that origin. + * Requiring an operator to type the single permitted value added a failure + * mode without adding a choice — and it is a failure mode that fires easily: + * `.env` ships this value inside the image, but @next/env only fills a + * variable that is ABSENT. Measured against the installed @next/env: a real + * environment variable set to the EMPTY STRING is left empty, and the file is + * not consulted. So one blank field in a dashboard defeated the shipped + * default and took production down with "required in production". + * + * This is not the remote fallback that 759f3b7 removed. That one defaulted to + * `https://platform.loyaly.ai` — the console's OWN origin, a host that is not + * the API at all and that answers wrongly instead of failing. This defaults to + * the one host the validator already insists on, and every other value, + * including that old wrong one, is still rejected by name below. + * + * The result is that production has exactly one required variable — + * AUTH_SECRET — which is the only value that genuinely cannot be shipped. + */ + if (isProduction) return (cachedBase = PRODUCTION_API_ORIGIN); + return (cachedBase = DEV_API_BASE); }