Merge pull request #11 from NeverEndingCode/fix-boot-probe-404

v1.8.4: dual mode would have refused to start against a good core

Author: NeverEndingCode

CHANGELOG.md

@@ -1,5 +1,25 @@
 # Changelog
 
+## v1.8.4
+
+- **`AUTH_MODE=dual` would have refused to start against a correctly-secured
+  core.** v1.8.3 fixed the wrong-endpoint bug in the preflight but left a second
+  copy of the same probe in the boot path, still using `/recipe/users/count` —
+  which SuperTokens core 12 does not implement. That guard *throws* on anything
+  that is not a 401, so the 404 would have been read as "the core is running
+  without API_KEYS" and stopped the container from booting.
+
+  Unlike the preflight, where the bug printed an alarming line, here it would
+  have blocked the cutover entirely and blamed the operator for a URL this code
+  got wrong.
+
+  The probe now lives in `server/supertokens/coreProbe.js` and is imported by
+  both callers, so they cannot drift again — asserted by a test. The boot guard
+  now throws **only** on a confirmed-open core (a known endpoint answering an
+  unkeyed request with 200); every other outcome warns and lets the boot
+  proceed.
+
+
 ## v1.8.3
 
 - **`supertokens:check` no longer reports a correctly-secured core as wide

Dockerfile

@@ -44,7 +44,7 @@ LABEL org.opencontainers.image.licenses="MIT"
 # only on a pushed vX.Y.Z tag, and docker/metadata-action derives the
 # published image's version label from that tag - so this literal only
 # affects locally-built images, not what GHCR publishes.
-LABEL org.opencontainers.image.version="1.8.3"
+LABEL org.opencontainers.image.version="1.8.4"
 
 VOLUME ["/app/data"]
 EXPOSE 3000

package.json

@@ -1,6 +1,6 @@
 {
   "name": "rackstack-server",
-  "version": "1.8.3",
+  "version": "1.8.4",
   "private": true,
   "type": "module",
   "scripts": {

server/supertokens/coreProbe.js

@@ -0,0 +1,56 @@
+// Asking a SuperTokens core whether it requires authentication.
+//
+// ONE implementation, imported by both the boot-time guard (init.js) and the
+// operator preflight (preflight.js). It lives in its own module because it
+// previously did not: the same probe was written twice, the path was corrected
+// in the preflight, and the copy in the boot path was left behind - where a
+// wrong answer does not print a scary line, it stops the container from
+// starting. If you add a third caller, import this; do not copy it.
+
+/**
+ * Endpoints gated by the core's API key, newest path first.
+ *
+ * NOT `/hello` - that answers unauthenticated by design as a health check, so
+ * a 200 there proves nothing about whether the core is locked down.
+ *
+ * More than one because the path is tenant-scoped on modern cores
+ * (`/<tenantId>/users/count`, per supertokens-node's own querier) and was not
+ * on older ones. v1.8.2 shipped a single guessed path that core 12 does not
+ * implement, so every probe returned 404.
+ */
+export const AUTHED_ENDPOINTS = Object.freeze([
+  '/public/users/count', // cores with multitenancy (the default tenant)
+  '/recipe/users/count', // older cores
+]);
+
+/**
+ * Probes the first endpoint this core actually implements.
+ *
+ * Returns `{ status, path }`. `status` is `null` when every candidate 404s,
+ * which means "this core exposes no path we know how to ask" - NOT "the core
+ * answered". That distinction is the entire point of this module: a 404 is
+ * evidence about our URL, not about the core's authentication, and inferring
+ * a security verdict from one is how v1.8.2 reported a correctly-secured core
+ * as running wide open.
+ */
+export async function probeAuthedEndpoint({
+  connectionURI, apiKey, fetchImpl = fetch, timeoutMs = 5000,
+}) {
+  const base = connectionURI.replace(/\/$/, '');
+  const headers = { 'api-version': '3.0' };
+  if (apiKey) headers['api-key'] = apiKey;
+
+  for (const path of AUTHED_ENDPOINTS) {
+    // eslint-disable-next-line no-await-in-loop
+    const res = await fetchImpl(`${base}${path}`, {
+      method: 'GET', headers, signal: AbortSignal.timeout(timeoutMs),
+    });
+    if (res.status !== 404) return { status: res.status, path };
+  }
+  return { status: null, path: null };
+}
+
+/** A refusal, however the core spells it. */
+export function isRefused(status) {
+  return status === 401 || status === 403;
+}

server/supertokens/init.js

@@ -37,6 +37,7 @@
 import { isSuperTokensEnabled } from '../authMode.js';
 import { buildProviders, resolvePublicOrigin } from './providers.js';
 import { buildSignInUpOverride } from './mapping.js';
+import { probeAuthedEndpoint, isRefused } from './coreProbe.js';
 
 // SuperTokens' own default API base path. It is also why the runbook widens
 // the GitHub OAuth registration to /auth: SuperTokens serves its callbacks at
@@ -135,26 +136,25 @@ export function isLoopback(uri) {
 /**
  * Confirms the core actually refuses unauthenticated callers.
  *
- * Probes an endpoint that requires an API key when one is configured. A 401
- * means the core is closed and all is well; a 200 means it answered a caller
- * holding no key at all, which is the state where anyone who can reach it can
- * mint a session for any user id.
+ * Throws ONLY on a confirmed-open core - an endpoint we know exists answering
+ * an unkeyed request with 200. Every other outcome warns and lets the boot
+ * proceed, because this guard sits on the startup path and a wrong answer here
+ * does not print a scary line, it stops the container.
  *
- * `/hello` is deliberately NOT used - it answers unauthenticated by design as
- * a health check, so probing it would prove nothing.
+ * That distinction was missing in v1.8.2: this probed a single hardcoded path
+ * that core 12 does not implement, and treated the resulting 404 as proof the
+ * core was open - so setting AUTH_MODE=dual against a perfectly well-secured
+ * core would have refused to start, blaming the operator for a URL this code
+ * got wrong. The probe now lives in ./coreProbe.js and is shared with the
+ * preflight, so the two cannot drift again.
  */
 export async function assertCoreRejectsAnonymous({ connectionURI, hasKey, fetchImpl = fetch }) {
-  const url = `${connectionURI.replace(/\/$/, '')}/recipe/users/count`;
-  let response;
+  let probe;
   try {
-    response = await fetchImpl(url, {
-      method: 'GET',
-      headers: { 'api-version': '3.0' },
-      signal: AbortSignal.timeout(5000),
-    });
+    probe = await probeAuthedEndpoint({ connectionURI, fetchImpl });
   } catch (e) {
     // Unreachable, DNS failure, timeout. Cannot establish anything; the core
-    // may simply still be starting. Warn rather than refuse - see the caller.
+    // may simply still be starting. Warn rather than refuse.
     console.warn(
       `[auth] could not verify that the SuperTokens core at ${connectionURI} requires `
       + `authentication (${e.message}). If it is running without API_KEYS, anyone who can `
@@ -163,16 +163,28 @@ export async function assertCoreRejectsAnonymous({ connectionURI, hasKey, fetchI
     return 'unverified';
   }
 
-  if (response.status === 401) return 'closed';
+  if (isRefused(probe.status)) return 'closed';
 
-  throw new Error(
-    `The SuperTokens core at ${connectionURI} answered an unauthenticated request with `
-    + `HTTP ${response.status}, which means it is running without API_KEYS. Anyone who can `
-    + 'reach it can mint a session for any user id, including every value in SUPER_ADMIN_IDS, '
-    + 'without any request reaching RackStack. Set API_KEYS on the core to the same value as '
-    + `SUPERTOKENS_API_KEY here${hasKey ? '' : ' (which is also unset)'}, and do not publish `
-    + 'its port.',
+  if (probe.status === 200) {
+    throw new Error(
+      `The SuperTokens core at ${connectionURI} answered an unauthenticated request to `
+      + `${probe.path} with HTTP 200, which means it is running without API_KEYS. Anyone who `
+      + 'can reach it can mint a session for any user id, including every value in '
+      + 'SUPER_ADMIN_IDS, without any request reaching RackStack. Set API_KEYS on the core to '
+      + `the same value as SUPERTOKENS_API_KEY here${hasKey ? '' : ' (which is also unset)'}, `
+      + 'and do not publish its port.',
+    );
+  }
+
+  // A 404 from every candidate, or any other unexpected status, says our URL
+  // is wrong for this core version - not that the core is open. Refusing to
+  // boot on that would be punishing the operator for our own mistake.
+  console.warn(
+    `[auth] could not verify that the SuperTokens core at ${connectionURI} requires `
+    + `authentication (${probe.status === null ? 'no known endpoint answered' : `unexpected HTTP ${probe.status}`}). `
+    + 'This is NOT evidence that it is open, but do check it by hand.',
   );
+  return 'unverified';
 }
 
 /**

server/supertokens/preflight.js

@@ -16,6 +16,7 @@
 
 import { buildProviders, resolvePublicOrigin, PROVIDER_IDS } from './providers.js';
 import { isLoopback } from './init.js';
+import { AUTHED_ENDPOINTS, probeAuthedEndpoint, isRefused } from './coreProbe.js';
 
 const PASS = 'PASS';
 const FAIL = 'FAIL';
@@ -26,48 +27,12 @@ function result(status, name, detail) {
   return { status, name, detail };
 }
 
-/**
- * Endpoints that require an API key when one is configured, newest path first.
- *
- * NOT `/hello` - that answers unauthenticated by design as a health check, so a
- * 200 there proves nothing about whether the core is locked down.
- *
- * More than one, because the path is tenant-scoped on modern cores
- * (`/<tenantId>/users/count`, per supertokens-node's own querier) and was not
- * on older ones. The first version of this shipped a single guessed path,
- * `/recipe/users/count`, which does not exist on core 12 - so every probe came
- * back 404 and the check reported a correctly-locked-down core as running wide
- * open. See the 404 handling below: that false alarm is the reason this is a
- * list and not a constant.
- */
-const AUTHED_ENDPOINTS = Object.freeze([
-  '/public/users/count', // core with multitenancy (the default tenant)
-  '/recipe/users/count', // older cores
-]);
-
 async function probe(url, { apiKey, fetchImpl, timeoutMs = 5000 }) {
   const headers = { 'api-version': '3.0' };
   if (apiKey) headers['api-key'] = apiKey;
   return fetchImpl(url, { method: 'GET', headers, signal: AbortSignal.timeout(timeoutMs) });
 }
 
-/**
- * Probes the first endpoint this core actually implements.
- *
- * Returns `{ status, path }`, or `{ status: null }` when every candidate 404s -
- * which means "this core does not expose any path we know how to ask", NOT
- * "the core answered". The distinction is the whole point: a 404 is evidence
- * about our URL, not about the core's authentication.
- */
-async function probeAuthedEndpoint({ connectionURI, apiKey, fetchImpl }) {
-  for (const path of AUTHED_ENDPOINTS) {
-    // eslint-disable-next-line no-await-in-loop
-    const res = await probe(`${connectionURI}${path}`, { apiKey, fetchImpl });
-    if (res.status !== 404) return { status: res.status, path };
-  }
-  return { status: null, path: null };
-}
-
 /**
  * Runs every deployment check and returns the results.
  *
@@ -158,7 +123,7 @@ export async function runPreflight({
   // is exactly why it is checked here rather than left to be noticed.
   try {
     const anon = await probeAuthedEndpoint({ connectionURI, fetchImpl });
-    if (anon.status === 401 || anon.status === 403) {
+    if (isRefused(anon.status)) {
       checks.push(result(PASS, 'core requires authentication', `anonymous request rejected (${anon.status})`));
     } else if (anon.status === 200) {
       checks.push(result(
@@ -239,7 +204,7 @@ export async function runPreflight({
       const authed = await probeAuthedEndpoint({ connectionURI, apiKey, fetchImpl });
       if (authed.status === 200) {
         checks.push(result(PASS, 'SUPERTOKENS_API_KEY', 'accepted by the core'));
-      } else if (authed.status === 401 || authed.status === 403) {
+      } else if (isRefused(authed.status)) {
         checks.push(result(
           FAIL, 'SUPERTOKENS_API_KEY',
           `The core rejected it (HTTP ${authed.status}). It must be byte-identical to a value in `

tests/supertokens.security.test.js

@@ -108,7 +108,52 @@ describe('rejectRawOAuthTokens (authentication bypass guard)', () => {
       fetchImpl: async (url) => { probed = url; return { status: 401 }; },
     });
     expect(probed).not.toContain('/hello');
-    expect(probed).toContain('/recipe/');
+    // A key-gated endpoint. The exact path is version-dependent, so assert the
+    // property rather than a literal - the literal is what broke in v1.8.2.
+    expect(probed).toContain('users/count');
+  });
+
+  it('BOOTS against a core whose endpoints all 404, rather than calling it open', async () => {
+    // The v1.8.3 near-miss. The preflight's 404 handling was fixed, but this
+    // guard kept its own copy of the probe with the old hardcoded path - and
+    // unlike the preflight, a wrong answer here does not print a scary line,
+    // it stops the container from starting.
+    //
+    // So on core 12 (which does not implement /recipe/users/count) setting
+    // AUTH_MODE=dual against a perfectly well-secured core would have refused
+    // to boot, blaming the operator for a URL this code got wrong. The probe
+    // now lives in coreProbe.js and is shared, so the two cannot drift again.
+    const { assertCoreRejectsAnonymous } = await import('../server/supertokens/init.js');
+    const noKnownEndpoint = async () => ({ status: 404 });
+
+    await expect(assertCoreRejectsAnonymous({
+      connectionURI: 'http://core:3567', hasKey: true, fetchImpl: noKnownEndpoint,
+    })).resolves.toBe('unverified');
+  });
+
+  it('finds the tenant-scoped path a modern core actually implements', async () => {
+    const { assertCoreRejectsAnonymous } = await import('../server/supertokens/init.js');
+    const core12 = async (url) => (url.endsWith('/public/users/count')
+      ? { status: 401 }
+      : { status: 404 });
+
+    await expect(assertCoreRejectsAnonymous({
+      connectionURI: 'http://core:3567', hasKey: true, fetchImpl: core12,
+    })).resolves.toBe('closed');
+  });
+
+  it('shares one probe implementation with the preflight', async () => {
+    // The bug was duplication, so this asserts the de-duplication rather than
+    // the behaviour: both callers must import from coreProbe.js. A future
+    // caller that re-copies the paths reintroduces exactly this class of bug.
+    const init = readFileSync(new URL('../server/supertokens/init.js', import.meta.url), 'utf8');
+    const preflight = readFileSync(new URL('../server/supertokens/preflight.js', import.meta.url), 'utf8');
+
+    expect(init).toMatch(/from '\.\/coreProbe\.js'/);
+    expect(preflight).toMatch(/from '\.\/coreProbe\.js'/);
+    // Neither may hardcode a probe path of its own.
+    expect(init).not.toMatch(/users\/count/);
+    expect(preflight).not.toMatch(/'\/(public|recipe)\/users\/count'/);
   });
 
   it('refuses to boot against a core too old for this SDK', async () => {