fix(plugin-auth): single written definition for the auth basePath default

.changeset/16384-auth-base-path-single-definition.md

@@ -0,0 +1,27 @@
+---
+'@objectstack/plugin-auth': minor
+---
+
+fix(plugin-auth): give the auth `basePath` default a single written definition (#16384)
+
+`'/api/v1/auth'`, the shipped default for `AuthPlugin`'s `basePath` option, was
+written independently at four sites: the `AuthPlugin` constructor, two later
+re-derivations inside `AuthPlugin` (`registerAuthRoutes`, the OIDC discovery
+`.well-known` alias), and `AuthManager.configuredBasePath()`'s own fallback.
+Nothing was broken by the duplication — `AuthPlugin` always supplies `basePath`
+to `AuthManager`, so the manager's copy was dead on the live path and
+unfalsifiable by construction: no test could have caught one copy drifting from
+the other three.
+
+The default now lives in exactly one place, `DEFAULT_AUTH_BASE_PATH` (exported
+from `@objectstack/plugin-auth`, declared beside `readMcpServerEnabledEnv` in
+`auth-manager.ts`); all four sites import it instead of retyping the literal.
+Every site evaluates byte-identically to before — this is a consolidation of
+where the value is *written*, not a change to what any site *evaluates to*, and
+in particular does **not** touch `AuthManager`'s `configuredBasePath` →
+`rootedBasePath` → `getBasePath` normalisation chain (#16399) or the published
+OAuth `iss` / RFC 8707 `aud` identifiers those getters produce.
+
+This is additive and non-breaking — no existing call site's behaviour changes —
+but it does add one new named export (`DEFAULT_AUTH_BASE_PATH`) to the
+package's public surface, which is what makes this `minor` rather than `patch`.

packages/plugins/plugin-auth/src/auth-manager-base-path.test.ts

@@ -48,7 +48,10 @@
 // that can hold a live better-auth and this manager at once.
 
 import { describe, it, expect } from 'vitest';
-import { AuthManager } from './auth-manager';
+import { readFileSync } from 'node:fs';
+import { fileURLToPath } from 'node:url';
+import { resolve, dirname } from 'node:path';
+import { AuthManager, DEFAULT_AUTH_BASE_PATH } from './auth-manager';
 import type { AuthManagerOptions } from './auth-manager';
 
 const managerWith = (basePath?: unknown) =>
@@ -332,3 +335,68 @@ describe('#16399 one normalisation chain, and an MCP resource URL that is always
     }
   });
 });
+
+/**
+ * #16384 — the literal `'/api/v1/auth'` used to be written independently at
+ * FOUR sites: this file's own `configuredBasePath()` fallback, plus three in
+ * `auth-plugin.ts` (the constructor default, `registerAuthRoutes`'s fallback,
+ * `registerOidcDiscoveryRoutes`'s fallback). Nothing was BROKEN by that — see
+ * the card: `AuthPlugin` always supplies `basePath`, so this manager's own
+ * fallback is dead on the live path, unfalsifiable by construction (measured
+ * during #16025's round: mutating it to `/api/v7/elsewhere` was INERT). That
+ * is exactly why a VALUE-comparison test cannot catch a future divergence:
+ * two independently-typed copies of the same string are byte-identical today
+ * and would stay green right up until someone edited only one of them — and
+ * the edited one might be the manager's, which no test on the live path can
+ * observe at all.
+ *
+ * The fix is structural, not behavioural: `DEFAULT_AUTH_BASE_PATH` (declared
+ * above, next to `readMcpServerEnabledEnv`) is now the only place the literal
+ * is WRITTEN; every one of the four readers imports it instead of retyping
+ * it. What this describes pins is that structural fact, by reading the two
+ * files' own source text — a future edit that reintroduces a hardcoded
+ * default at any of the four sites (however well-intentioned: "it's just a
+ * string, inlining it is simpler") reappears here as a second literal and
+ * fails immediately, before it has any chance to drift from the other three.
+ */
+describe('#16384 the default base path is written in exactly ONE place', () => {
+  const HERE = dirname(fileURLToPath(import.meta.url));
+  const AUTH_MANAGER_SRC = readFileSync(resolve(HERE, 'auth-manager.ts'), 'utf8');
+  const AUTH_PLUGIN_SRC = readFileSync(resolve(HERE, 'auth-plugin.ts'), 'utf8');
+
+  /**
+   * Lines of source that carry the literal, MINUS comment lines (this file's
+   * docblocks use a leading `*` per continuation line; ordinary comments use
+   * `//`). A `@default` TSDoc annotation or a worked example inside a
+   * docblock is documentation, not a second definition, and stays out of
+   * this card's scope by design — the dispatch that opened it counts four
+   * CODE sites and two doc comments separately, and only the four move here.
+   */
+  const codeLinesCarryingTheLiteral = (source: string): string[] =>
+    source
+      .split('\n')
+      .map((line) => line.trim())
+      .filter((line) => line.includes(`'${DEFAULT_AUTH_BASE_PATH}'`))
+      .filter((line) => !line.startsWith('*') && !line.startsWith('//'));
+
+  it('sanity: DEFAULT_AUTH_BASE_PATH is still the shipped literal this pin reasons about', () => {
+    // Hardcoded on purpose, not a re-read of the constant — if this constant's
+    // VALUE ever moves, this line (not the structural assertions below) is
+    // what should turn red.
+    expect(DEFAULT_AUTH_BASE_PATH).toBe('/api/v1/auth');
+  });
+
+  it('auth-manager.ts writes the literal exactly once — the constant declaration itself', () => {
+    expect(codeLinesCarryingTheLiteral(AUTH_MANAGER_SRC)).toEqual([
+      `export const DEFAULT_AUTH_BASE_PATH = '${DEFAULT_AUTH_BASE_PATH}';`,
+    ]);
+  });
+
+  it('auth-plugin.ts never writes the literal — every site imports the shared constant', () => {
+    expect(codeLinesCarryingTheLiteral(AUTH_PLUGIN_SRC)).toEqual([]);
+    // And it isn't simply missing the import either — the three collapsed
+    // sites (constructor default, registerAuthRoutes, registerOidcDiscoveryRoutes)
+    // must actually reference the shared binding.
+    expect(AUTH_PLUGIN_SRC.match(/DEFAULT_AUTH_BASE_PATH/g)?.length ?? 0).toBeGreaterThanOrEqual(4);
+  });
+});

packages/plugins/plugin-auth/src/auth-manager.ts

@@ -305,6 +305,21 @@ function readSsoOnlyEnv(): boolean | undefined {
   return readBooleanEnv('OS_AUTH_SSO_ONLY');
 }
 
+/**
+ * [#16384] The auth plugin's shipped `basePath` default — the ONE place this
+ * literal is written. Before this card it existed independently at four
+ * sites: this file's own `configuredBasePath()` fallback below, plus three in
+ * `auth-plugin.ts` (the constructor default and two later re-derivations of
+ * "what if the caller cleared `basePath`?"). A future edit to one could drift
+ * from the other three silently — `configuredBasePath()`'s own fallback is
+ * unfalsifiable by construction on the live path (`AuthPlugin` always supplies
+ * `basePath`, per its constructor default below), so no runtime test could
+ * have caught that drift. Every one of the four readers now evaluates BYTE
+ * IDENTICALLY to before this constant existed — see `configuredBasePath()`'s
+ * docblock for the normalisation chain this does NOT touch.
+ */
+export const DEFAULT_AUTH_BASE_PATH = '/api/v1/auth';
+
 /**
  * Whether this runtime serves the HTTP MCP surface (`/api/v1/mcp`).
  * Delegates to the platform-wide decision point (`isMcpServerEnabled` in
@@ -5667,6 +5682,9 @@ export class AuthManager {
    * configured. Unchanged from before this card — only the reading of it moved
    * here, so `getBasePath()` and this cannot drift apart by accident.
    *
+   * [#16384] The fallback is `DEFAULT_AUTH_BASE_PATH`, not a re-typed literal
+   * — see that constant's docblock above.
+   *
    * ## ⛔ Never normalise here
    *
    * better-auth stamps the OAuth access-token `iss` from `ctx.context.baseURL`,
@@ -5692,7 +5710,7 @@ export class AuthManager {
    * #16399's decision, not this card's.
    */
   private configuredBasePath(): string {
-    return this.config.basePath || '/api/v1/auth';
+    return this.config.basePath || DEFAULT_AUTH_BASE_PATH;
   }
 
   /**

packages/plugins/plugin-auth/src/auth-plugin.test.ts

@@ -1240,6 +1240,57 @@ describe('AuthPlugin', () => {
       );
     });
 
+    // [#16384] Before this card, the DEFAULT base path was independently
+    // written at the constructor (line ~326) AND re-derived here in
+    // `registerAuthRoutes` (`this.options.basePath || '/api/v1/auth'`) — two
+    // literals that happened to agree, with nothing wiring them together. This
+    // is the sibling of "should use custom base path" above: that test never
+    // exercised the DEFAULT reaching the mount at all. Asserted against the
+    // external contract literal, not the `DEFAULT_AUTH_BASE_PATH` binding the
+    // implementation now shares — a typo in that constant must still fail
+    // this test.
+    it('should mount the default base path when none is configured', async () => {
+      const { hookFn, trigger } = createHookCapture();
+      mockContext.hook = hookFn;
+
+      authPlugin = new AuthPlugin({
+        secret: 'test-secret-at-least-32-chars-long',
+        baseUrl: 'http://localhost:3000',
+      });
+
+      await authPlugin.init(mockContext);
+
+      const mockRawApp = {
+        all: vi.fn(),
+        get: vi.fn(),
+        post: vi.fn(),
+      };
+
+      const mockHttpServer = {
+        post: vi.fn(),
+        get: vi.fn(),
+        put: vi.fn(),
+        delete: vi.fn(),
+        patch: vi.fn(),
+        use: vi.fn(),
+        getRawApp: vi.fn(() => mockRawApp),
+      };
+
+      // Cast, not the bare assignment the sibling test above uses: the bare
+      // form is TS2322 against `getService`'s generic signature — an
+      // ALREADY-tracked debt (`test-typecheck-debt.json`, 11 instances) this
+      // new test must not grow to 12.
+      mockContext.getService = vi.fn(() => mockHttpServer) as unknown as typeof mockContext.getService;
+
+      await authPlugin.start(mockContext);
+      await trigger('kernel:ready');
+
+      expect(mockRawApp.all).toHaveBeenCalledWith(
+        '/api/v1/auth/*',
+        expect.any(Function)
+      );
+    });
+
     it('should configure session options', async () => {
       authPlugin = new AuthPlugin({
         secret: 'test-secret-at-least-32-chars-long',

packages/plugins/plugin-auth/src/auth-plugin.ts

@@ -35,6 +35,9 @@ import {
   AuthManager,
   resolveOidcProviderEnabled,
   readMcpServerEnabledEnv,
+  // [#16384] The one place `'/api/v1/auth'` is written — see its docblock in
+  // auth-manager.ts. This file no longer carries an independent copy.
+  DEFAULT_AUTH_BASE_PATH,
   type AuthManagerOptions,
 } from './auth-manager.js';
 import {
@@ -323,7 +326,7 @@ export class AuthPlugin implements Plugin {
   constructor(options: AuthPluginOptions = {}) {
     this.options = {
       registerRoutes: true,
-      basePath: '/api/v1/auth',
+      basePath: DEFAULT_AUTH_BASE_PATH,
       ...options
     };
   }
@@ -2023,7 +2026,7 @@ export class AuthPlugin implements Plugin {
   private registerAuthRoutes(httpServer: IHttpServer, ctx: PluginContext): void {
     if (!this.authManager) return;
 
-    const basePath = this.options.basePath || '/api/v1/auth';
+    const basePath = this.options.basePath || DEFAULT_AUTH_BASE_PATH;
 
     // Get raw Hono app to use native wildcard routing
     // Type assertion is safe here because we explicitly require Hono server as a dependency
@@ -3096,7 +3099,7 @@ export class AuthPlugin implements Plugin {
     // (including every MCP client bootstrapping from protected-resource
     // metadata) request `/.well-known/oauth-authorization-server/api/v1/auth`
     // — alias it to the same document.
-    const basePath = (this.options.basePath ?? '/api/v1/auth').replace(/\/$/, '');
+    const basePath = (this.options.basePath ?? DEFAULT_AUTH_BASE_PATH).replace(/\/$/, '');
     rawApp.get(`/.well-known/oauth-authorization-server${basePath}`, (c: any) =>
       withDiscoveryCache(authServerHandler, c.req.raw),
     );

scripts/check-auth-mount-ledger.mjs

@@ -137,7 +137,7 @@ import { isEntrypoint } from './invoked-as.mjs';
 // must not red. A battery BELOW its floor means cases stopped running; the
 // remedy is to find what stopped registering.
 const SELF_TEST_BATTERIES = Object.freeze({
-  'The base path is DERIVED, and its absence is not an empty population.': 2,
+  'The base path is DERIVED, and its absence is not an empty population.': 4,
   'LOAD-BEARING NEGATIVE: a mount with an exact row is clean.': 1,
   'LOAD-BEARING POSITIVE: a mount added with no row REDDENS, naming the route.': 2,
   'THE RIGHT BOUNDARY, both directions. This is the defect class #10534 fell into.': 4,
@@ -154,7 +154,7 @@ const SELF_TEST_BATTERIES = Object.freeze({
   '#8435 remedy authority. PLACEMENT is pinned here, per-gate, because the': 4,
   'Parse anchors: a moved anchor is a REFUSAL input, never an empty population.': 2,
   'The escaped-quote shape the real notes use is measured, not truncated.': 1,
-  'And the real inputs on disk are readable, so the anchors have not moved.': 2,
+  'And the real inputs on disk are readable, so the anchors have not moved.': 3,
 });
 
 // DELETING an entry silences that battery's floor exactly as effectively as
@@ -168,11 +168,15 @@ const UNATTRIBUTED_BATTERY = '(no battery open)';
 
 const ROOT = resolve(new URL('..', import.meta.url).pathname);
 
-/** The two inputs. Module-scope literals, so `dispatch-gates` derives this
- *  family for a diff touching either of them (#10309: a gate nothing can
- *  derive is a gate that runs only when someone remembers it). */
+/** The three inputs. Module-scope literals, so `dispatch-gates` derives this
+ *  family for a diff touching any of them (#10309: a gate nothing can
+ *  derive is a gate that runs only when someone remembers it).
+ *  [#16384] `AUTH_MANAGER_SOURCE` joined the set the day the `basePath`
+ *  fallback in `MOUNT_SOURCE` stopped being a re-typed literal and became an
+ *  identifier declared over there instead — see `deriveBasePath`. */
 const MOUNT_SOURCE = 'packages/plugins/plugin-auth/src/auth-plugin.ts';
 const LEDGER_SOURCE = 'packages/plugins/plugin-auth/src/auth-route-ledger.ts';
+const AUTH_MANAGER_SOURCE = 'packages/plugins/plugin-auth/src/auth-manager.ts';
 
 export const EXIT_CLEAN = 0;
 export const EXIT_FINDINGS = 1;
@@ -230,11 +234,26 @@ const LANE_VERBS = new Set(['all', 'use']);
 /**
  * The auth base path, DERIVED from the plugin rather than re-typed here, so a
  * rename moves this gate with it instead of silently emptying its population.
+ *
+ * [#16384] The fallback used to be a re-typed string literal at this exact
+ * spot; it is now the package's single `DEFAULT_AUTH_BASE_PATH` definition,
+ * imported from `auth-manager.ts` rather than retyped in the plugin. The
+ * literal-string match stays first (still the shape of every OTHER fallback
+ * this gate might meet); when the fallback is a bare identifier instead,
+ * resolve it the same way this function was already justified — one hop
+ * through the module that actually DECLARES it, never a second hardcoded
+ * copy — via `importedSource`, the text of the module the plugin imports the
+ * identifier from.
  */
-export function deriveBasePath(mountSource) {
+export function deriveBasePath(mountSource, importedSource) {
   const masked = maskComments(mountSource);
-  const m = /basePath\s*=\s*this\.options\.basePath\s*(?:\|\||\?\?)\s*'([^']+)'/.exec(masked);
-  return m ? m[1] : null;
+  const m =
+    /basePath\s*=\s*this\.options\.basePath\s*(?:\|\||\?\?)\s*(?:'([^']+)'|([A-Za-z_$][\w$]*))/.exec(masked);
+  if (!m) return null;
+  if (m[1] != null) return m[1];
+  if (importedSource == null) return null;
+  const decl = new RegExp(`export const ${m[2]}\\s*=\\s*'([^']+)'`).exec(maskComments(importedSource));
+  return decl ? decl[1] : null;
 }
 
 /**
@@ -627,6 +646,20 @@ function selfTest() {
   battery('The base path is DERIVED, and its absence is not an empty population.');
   ok(deriveBasePath(FIXTURE_PREAMBLE) === FIXTURE_BASE, 'basePath was not derived from the plugin');
   ok(deriveBasePath('const basePath = 42;') === null, 'a plugin with no derivable basePath did not refuse');
+  // [#16384] The fallback is now a named identifier import in the real plugin,
+  // not a re-typed literal — resolved one hop through the module it is
+  // DECLARED in, never guessed at.
+  ok(
+    deriveBasePath(
+      "const basePath = this.options.basePath || DEFAULT_AUTH_BASE_PATH;\n",
+      "export const DEFAULT_AUTH_BASE_PATH = '/api/v1/auth';\n",
+    ) === FIXTURE_BASE,
+    'an identifier fallback was not resolved through the imported module',
+  );
+  ok(
+    deriveBasePath("const basePath = this.options.basePath || DEFAULT_AUTH_BASE_PATH;\n") === null,
+    'an identifier fallback resolved to something without an imported module to read',
+  );
 
   // -- LOAD-BEARING NEGATIVE: a mount with an exact row is clean.
   battery('LOAD-BEARING NEGATIVE: a mount with an exact row is clean.');
@@ -895,7 +928,7 @@ function selfTest() {
 
   // -- And the real inputs on disk are readable, so the anchors have not moved.
   battery('And the real inputs on disk are readable, so the anchors have not moved.');
-  for (const rel of [MOUNT_SOURCE, LEDGER_SOURCE]) {
+  for (const rel of [MOUNT_SOURCE, LEDGER_SOURCE, AUTH_MANAGER_SOURCE]) {
     ok(existsSync(join(ROOT, rel)), `${rel} does not exist -- this gate's anchor moved`);
   }
 
@@ -981,14 +1014,17 @@ function main() {
 
   const mountAbs = join(ROOT, MOUNT_SOURCE);
   const ledgerAbs = join(ROOT, LEDGER_SOURCE);
+  const authManagerAbs = join(ROOT, AUTH_MANAGER_SOURCE);
   if (!existsSync(mountAbs)) refuse(`${MOUNT_SOURCE} does not exist`);
   if (!existsSync(ledgerAbs)) refuse(`${LEDGER_SOURCE} does not exist`);
+  if (!existsSync(authManagerAbs)) refuse(`${AUTH_MANAGER_SOURCE} does not exist`);
 
   const mountText = readFileSync(mountAbs, 'utf8');
   const ledgerText = readFileSync(ledgerAbs, 'utf8');
+  const authManagerText = readFileSync(authManagerAbs, 'utf8');
 
-  const basePath = deriveBasePath(mountText);
-  if (!basePath) refuse(`no \`basePath\` default could be derived from ${MOUNT_SOURCE}`);
+  const basePath = deriveBasePath(mountText, authManagerText);
+  if (!basePath) refuse(`no \`basePath\` default could be derived from ${MOUNT_SOURCE} (or its import from ${AUTH_MANAGER_SOURCE})`);
 
   const rows = parseLedgerRows(ledgerText);
   if (rows === null) refuse(`the AUTH_ROUTE_LEDGER anchor was not found in ${LEDGER_SOURCE}`);