fix(auth): never set AUTH_URL to empty, and catch the case that broke

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}

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("; ")}`,
       );
     }
 

src/lib/auth/readiness.test.ts

@@ -4,61 +4,134 @@ 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-----",
   AUTH_JWT_PUBLIC_KEY: "-----BEGIN PUBLIC KEY-----",
   AUTH_JWT_KID: "k1",
 };
 
+const problemsOf = (env: Record<string, string | undefined>) =>
+  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));
   });
 });

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>_ID` / `AUTH_<ID>_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<string, string | undefined>): 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<string, string | undefined>,
 ): 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 };
 };