From 632b80c776d1bc6e60c998576e17e6dbf86a0386 Mon Sep 17 00:00:00 2001 From: Kunj Hirapara Date: Sun, 13 Sep 2026 20:54:57 +0530 Subject: [PATCH] fix(auth): never set AUTH_URL to empty, and catch the case that broke Production reported auth:true while every Auth.js route returned 500. Both halves of that were my fault. `AUTH_URL: ${AUTH_URL:-}` was the obvious way to make the variable optional in compose. It sets it to the empty string, which is a set variable, and that is strictly worse than an absent one in two ways at once: it shadows the default the Dockerfile bakes in from NEXT_PUBLIC_APP_URL, and it stops @auth/core's trustHost ??= !!(AUTH_URL ?? AUTH_TRUST_HOST ?? VERCEL ?? ...) at the first term, because `??` falls through only on null/undefined and `!!""` is false. trustHost:false makes assertConfig return UntrustedHost for every request, which renders as "There was a problem with the server configuration" -- indistinguishable from a missing secret, which is where the debugging went. It now defaults to NEXT_PUBLIC_APP_URL, so it stays overridable without ever being blank. The readiness check said this deployment was fine. A green light that is wrong is worse than no light, because it teaches people to stop looking at it, so it now encodes both faults it missed: It mirrors the trustHost expression, including the `??` semantics, and distinguishes "AUTH_URL is set but empty" from "AUTH_URL is not set" -- the first is not a thing anyone guesses while debugging. It treats a half-configured OAuth provider as fatal. The previous version ignored the OAuth credentials, reasoning that a deployment with no Google app still supports credentials and magic-link sign-in. That is true only when the provider is absent entirely; an id without its secret makes Auth.js throw while building the provider list, taking down every sign-in method including the ones that need no provider at all. `missing` becomes `problems`, since these are diagnoses rather than a list of absent names. Still names and sentences only, never values -- /api/health is public. --- docker-compose.yml | 23 ++++-- src/app/api/health/route.ts | 2 +- src/lib/auth/readiness.test.ts | 125 ++++++++++++++++++++++++++------- src/lib/auth/readiness.ts | 97 ++++++++++++++++++++----- 4 files changed, 195 insertions(+), 52 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 9363550..712ca7b 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -52,13 +52,24 @@ services: AUTH_GOOGLE_SECRET: ${AUTH_GOOGLE_SECRET} AUTH_GITHUB_ID: ${AUTH_GITHUB_ID} AUTH_GITHUB_SECRET: ${AUTH_GITHUB_SECRET} - # Overrides the origin Auth.js builds redirect and error URLs from. + # The origin Auth.js builds redirect and error URLs from. # - # The image already defaults this to the NEXT_PUBLIC_APP_URL it was built - # with, so it normally needs no value here. Left overridable because the - # image default is baked at build time and a deployment may need to differ - # from it without a rebuild. - AUTH_URL: ${AUTH_URL:-} + # MUST NOT be empty. `${AUTH_URL:-}` was the obvious way to make this + # optional and it took the whole auth system down: an empty string is + # still a set variable, so it both shadows the image's own default and + # stops @auth/core's + # + # trustHost ??= !!(AUTH_URL ?? AUTH_TRUST_HOST ?? VERCEL ?? ...) + # + # at the first term -- `??` only falls through on null/undefined, and + # `!!""` is false. trustHost:false makes every Auth.js route answer + # UntrustedHost, rendered as "There was a problem with the server + # configuration", which reads like a missing secret rather than an empty + # one. + # + # Defaulting to NEXT_PUBLIC_APP_URL keeps it overridable without ever + # being blank. + AUTH_URL: ${AUTH_URL:-$NEXT_PUBLIC_APP_URL} # App NEXT_PUBLIC_APP_URL: ${NEXT_PUBLIC_APP_URL} diff --git a/src/app/api/health/route.ts b/src/app/api/health/route.ts index 4f68e10..426a4f1 100644 --- a/src/app/api/health/route.ts +++ b/src/app/api/health/route.ts @@ -30,7 +30,7 @@ export async function GET() { if (!auth.ready) { console.error( - `[health] auth is not configured; sign-in will fail. Missing: ${auth.missing.join(", ")}`, + `[health] auth is not configured; sign-in will fail: ${auth.problems.join("; ")}`, ); } diff --git a/src/lib/auth/readiness.test.ts b/src/lib/auth/readiness.test.ts index 0bba652..52a376e 100644 --- a/src/lib/auth/readiness.test.ts +++ b/src/lib/auth/readiness.test.ts @@ -4,6 +4,8 @@ import { describe, it } from "node:test"; import { getAuthReadiness } from "./readiness.ts"; const complete = { + NODE_ENV: "production", + AUTH_URL: "https://commit.example.com", AUTH_SECRET: "s", AUTH_ADAPTER_SECRET: "s", AUTH_JWT_PRIVATE_KEY: "-----BEGIN PRIVATE KEY-----", @@ -11,54 +13,125 @@ const complete = { AUTH_JWT_KID: "k1", }; +const problemsOf = (env: Record) => + getAuthReadiness(env).problems.join(" | "); + describe("getAuthReadiness", () => { - it("is ready when all five are set", () => { - assert.deepEqual(getAuthReadiness(complete), { ready: true, missing: [] }); + it("is ready when everything required is present", () => { + assert.deepEqual(getAuthReadiness(complete), { ready: true, problems: [] }); }); - it("reports every missing name rather than stopping at the first", () => { + it("reports every missing variable rather than stopping at the first", () => { // An operator setting one variable per deploy is the slowest possible way // to fix this, and it is what a first-failure-only report produces. - const result = getAuthReadiness({}); + const result = getAuthReadiness({ NODE_ENV: "production" }); assert.equal(result.ready, false); - assert.deepEqual(result.missing, [ + for (const name of [ "AUTH_SECRET", "AUTH_ADAPTER_SECRET", "AUTH_JWT_PRIVATE_KEY", "AUTH_JWT_PUBLIC_KEY", "AUTH_JWT_KID", - ]); + ]) { + assert.match(result.problems.join(" | "), new RegExp(name)); + } }); - it("treats whitespace as unset", () => { - // An easy thing to paste by accident, and identical to unset for all five. - const result = getAuthReadiness({ ...complete, AUTH_JWT_KID: " " }); - - assert.equal(result.ready, false); - assert.deepEqual(result.missing, ["AUTH_JWT_KID"]); + it("treats whitespace and empty strings as unset", () => { + assert.match(problemsOf({ ...complete, AUTH_JWT_KID: " " }), /AUTH_JWT_KID/); + assert.match(problemsOf({ ...complete, AUTH_SECRET: "" }), /AUTH_SECRET/); }); - it("treats an empty string as unset", () => { - const result = getAuthReadiness({ ...complete, AUTH_SECRET: "" }); + /** + * The failure this file exists to have caught, and did not. + * + * Production reported auth:true while every Auth.js route returned 500, + * because AUTH_URL was set to the empty string by a compose default. + */ + describe("trusted host", () => { + it("catches AUTH_URL set to the empty string", () => { + // @auth/core does `trustHost ??= !!(AUTH_URL ?? ...)`. `??` falls through + // only on null/undefined, so "" stops the chain and yields false. An + // empty variable is strictly worse than an absent one. + const result = getAuthReadiness({ ...complete, AUTH_URL: "" }); - assert.equal(result.ready, false); - assert.deepEqual(result.missing, ["AUTH_SECRET"]); + assert.equal(result.ready, false); + assert.match(result.problems.join(" | "), /AUTH_URL is set but empty/); + }); + + it("catches AUTH_URL absent in production", () => { + const { AUTH_URL: _omitted, ...withoutUrl } = complete; + const result = getAuthReadiness(withoutUrl); + + assert.equal(result.ready, false); + assert.match(result.problems.join(" | "), /untrusted host/); + }); + + it("accepts AUTH_TRUST_HOST as an alternative", () => { + const { AUTH_URL: _omitted, ...withoutUrl } = complete; + + assert.equal( + getAuthReadiness({ ...withoutUrl, AUTH_TRUST_HOST: "true" }).ready, + true, + ); + }); + + it("does not complain outside production, matching Auth.js", () => { + const { AUTH_URL: _omitted, ...withoutUrl } = complete; + + assert.equal( + getAuthReadiness({ ...withoutUrl, NODE_ENV: "development" }).ready, + true, + ); + }); }); - it("does not require the OAuth client IDs", () => { - // A deployment with no Google app is degraded, not broken: credentials and - // magic-link sign-in still work. Firing this flag for something a user can - // work around would make it mean less when it fires for something they - // cannot. - assert.equal(getAuthReadiness(complete).ready, true); + describe("OAuth providers", () => { + it("is ready when a provider is absent entirely", () => { + // Degraded, not broken: credentials and magic-link sign-in still work, so + // firing here would make the flag mean less when it fires for something + // that cannot be worked around. + assert.equal(getAuthReadiness(complete).ready, true); + }); + + it("catches an id with no secret", () => { + // Fatal rather than degraded: Auth.js throws while building the provider + // list, which takes down every sign-in method including the ones that + // need no provider at all. + const result = getAuthReadiness({ ...complete, AUTH_GOOGLE_ID: "id" }); + + assert.equal(result.ready, false); + assert.match(result.problems.join(" | "), /AUTH_GOOGLE_SECRET is missing/); + }); + + it("catches a secret with no id", () => { + const result = getAuthReadiness({ ...complete, AUTH_GITHUB_SECRET: "s" }); + + assert.equal(result.ready, false); + assert.match(result.problems.join(" | "), /AUTH_GITHUB_ID is missing/); + }); + + it("is ready when a provider is configured completely", () => { + assert.equal( + getAuthReadiness({ + ...complete, + AUTH_GOOGLE_ID: "id", + AUTH_GOOGLE_SECRET: "secret", + }).ready, + true, + ); + }); }); - it("never returns a value, only a name", () => { - const secret = "super-secret-value"; - const result = getAuthReadiness({ ...complete, AUTH_SECRET: undefined, OTHER: secret }); + it("never returns a value, only names and diagnoses", () => { + const secret = "GOCSPX-a-real-looking-secret"; + const result = getAuthReadiness({ + ...complete, + AUTH_SECRET: undefined, + AUTH_GOOGLE_SECRET: secret, + }); - assert.deepEqual(result.missing, ["AUTH_SECRET"]); assert.ok(!JSON.stringify(result).includes(secret)); }); }); diff --git a/src/lib/auth/readiness.ts b/src/lib/auth/readiness.ts index c5c3cf0..5e85eb5 100644 --- a/src/lib/auth/readiness.ts +++ b/src/lib/auth/readiness.ts @@ -2,34 +2,32 @@ * Whether this deployment can actually sign anyone in. * * Written for a specific hazard in the Clerk migration. The image that switches - * sign-in to Auth.js can reach the VM before anyone sets the Auth.js - * environment variables, and the failure mode is total: the app boots, serves - * pages, reports healthy, and nobody can log in. `/api/health` would have said - * "healthy" throughout, because it only ever checked the Clerk-era variables. + * sign-in to Auth.js can reach the VM before anyone configures it, and the + * failure mode is total: the app boots, serves pages, reports healthy, and + * nobody can log in. `/api/health` would have said "healthy" throughout, + * because it only ever checked the Clerk-era variables. * - * So this is a pre-flight, not a diagnostic after the fact. Check it against a - * deployment *before* letting the new image roll out to it. + * The first version of this file checked that five variables were non-empty and + * called that ready. It then reported `auth: true` on a deployment where every + * Auth.js route was returning 500, which is worse than not having checked at + * all — a green light that is wrong teaches people to stop looking at it. The + * two faults it missed are both encoded below, and both were real. * - * Names only, never values. The caller logs the missing names server-side and + * Names and diagnoses only, never values. The caller logs these server-side and * publishes a boolean, because /api/health is public — it is polled by the * container healthcheck, which carries no session. A public endpoint listing - * exactly which secrets a deployment is missing is a map for anyone probing it. + * exactly what a deployment is missing is a map for anyone probing it. */ export type AuthReadiness = { ready: boolean; - /** Variable names, in the order an operator should set them. Never values. */ - missing: string[]; + /** Short diagnoses, in the order an operator should work through them. */ + problems: string[]; }; /** * The five that auth cannot function without. * - * Deliberately not the OAuth client IDs. Credentials and magic-link sign-in work - * without them — a deployment with no Google app is degraded, not broken, and - * conflating the two would make this flag fire for something a user can work - * around. These five have no workaround: - * * AUTH_SECRET signs and encrypts the session cookie * AUTH_ADAPTER_SECRET every adapter call to Convex is rejected without it * AUTH_JWT_PRIVATE_KEY mints the token Convex verifies @@ -44,12 +42,73 @@ const REQUIRED = [ "AUTH_JWT_KID", ] as const; +/** OAuth providers, as `AUTH__ID` / `AUTH__SECRET` pairs. */ +const OAUTH_PROVIDERS = ["GOOGLE", "GITHUB"] as const; + +const present = (value: string | undefined) => Boolean(value?.trim()); + +/** + * Mirrors `config.trustHost ??= !!(AUTH_URL ?? AUTH_TRUST_HOST ?? VERCEL ?? CF_PAGES ?? NODE_ENV !== "production")` + * from @auth/core. + * + * When this is false, `assertConfig` returns UntrustedHost and every Auth.js + * route answers 500 — rendered to the user as "There was a problem with the + * server configuration", which reads like a missing secret. + * + * The subtlety worth reproducing exactly is `??`. It falls through only on + * null/undefined, so an AUTH_URL set to the empty string stops the chain at the + * first term and yields false. An empty variable is therefore strictly worse + * than an absent one, which is not a thing anyone guesses while debugging. + */ +const willTrustHost = (env: Record): boolean => { + const first = [env.AUTH_URL, env.AUTH_TRUST_HOST, env.VERCEL, env.CF_PAGES].find( + (value) => value !== undefined && value !== null, + ); + + if (first !== undefined) return Boolean(first); + + return env.NODE_ENV !== "production"; +}; + export const getAuthReadiness = ( env: Record, ): AuthReadiness => { - // Trimmed, because an environment variable set to whitespace is the same as - // unset for every one of these and is an easy thing to paste by accident. - const missing = REQUIRED.filter((name) => !env[name]?.trim()); + const problems: string[] = []; + + for (const name of REQUIRED) { + // Trimmed, because a variable set to whitespace is the same as unset for + // every one of these and is an easy thing to paste by accident. + if (!present(env[name])) problems.push(`${name} is not set`); + } + + if (!willTrustHost(env)) { + problems.push( + env.AUTH_URL === "" + ? "AUTH_URL is set but empty, so Auth.js will reject every request as an untrusted host" + : "AUTH_URL is not set, so Auth.js will reject every request as an untrusted host", + ); + } + + /** + * A half-configured provider is fatal, not degraded. + * + * The earlier version of this file deliberately ignored the OAuth credentials, + * reasoning that a deployment with no Google app still supports credentials + * and magic-link sign-in. True — but only when the provider is absent + * entirely. An id without its secret makes Auth.js throw while building the + * provider list, which takes down every sign-in method including the ones + * that need no provider at all. + */ + for (const provider of OAUTH_PROVIDERS) { + const id = present(env[`AUTH_${provider}_ID`]); + const secret = present(env[`AUTH_${provider}_SECRET`]); + + if (id !== secret) { + problems.push( + `AUTH_${provider}_${id ? "SECRET" : "ID"} is missing while its pair is set; a half-configured provider breaks all sign-in`, + ); + } + } - return { ready: missing.length === 0, missing: [...missing] }; + return { ready: problems.length === 0, problems }; };