fix(plugin-auth): the auth catch-all yields paths better-auth does not own (#4088)

.changeset/auth-catchall-yields-unowned-paths.md

@@ -0,0 +1,36 @@
+---
+"@objectstack/plugin-auth": patch
+---
+
+fix(plugin-auth): the auth catch-all yields paths better-auth does not own (#4088)
+
+`registerAuthRoutes` mounts `rawApp.all('${basePath}/*')` over the whole auth
+namespace (`/api/v1/auth` by default), and that handler was **terminal**: it
+returned better-auth's response unconditionally, including the 404 better-auth
+produces for a path it does not implement. Any other plugin's route under that
+prefix was therefore reachable only if it happened to register **first** — Hono
+runs handlers matching a path in registration order and the first to return a
+Response wins.
+
+That put a load-bearing surface at the mercy of `kernel.use()` order.
+`@objectstack/plugin-hono-server` mounts `/auth/me/permissions` and
+`/auth/me/localization` from its own `kernel:ready` hook; objectui's entire
+permission layer reads the former and `core`'s auth gate allow-lists the latter
+as an endpoint a gated user must still reach. Register `AuthPlugin` before
+`HonoServerPlugin` and all of it silently 404s.
+
+A 404 from better-auth now means "this path is not mine" and the catch-all yields
+to whatever else matched, in either registration order. Deliberately narrow:
+
+- **Only 404 falls through.** 401/403 are real better-auth answers, not
+  disclaimers of ownership.
+- **Precedence still favours the namespace owner.** better-auth wins every path
+  it implements; only its leftovers are up for grabs.
+- **The unclaimed-path wire shape is unchanged.** When nothing downstream
+  answers, better-auth's own 404 is returned verbatim rather than Hono's
+  `404 Not Found`.
+
+No configuration changes and no new routes. The only behavioural difference for
+an existing deployment is that a route another plugin mounts under
+`/api/v1/auth/*` now answers regardless of plugin order — previously it answered
+only in the lucky order.

packages/plugins/plugin-auth/package.json

@@ -33,6 +33,7 @@
   },
   "devDependencies": {
     "@types/node": "^26.1.1",
+    "hono": "^4.12.32",
     "typescript": "^6.0.3",
     "vitest": "^4.1.10"
   },

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

@@ -0,0 +1,140 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * #4088 — the auth catch-all must not swallow other plugins' routes.
+ *
+ * `registerAuthRoutes` mounts `rawApp.all('${basePath}/*')` over the whole auth
+ * namespace. It used to be TERMINAL — better-auth's response was returned
+ * unconditionally, including the 404 better-auth produces for a path it does
+ * not implement. Any other plugin's route under that prefix was therefore
+ * reachable only if it happened to register FIRST (Hono runs handlers matching
+ * a path in registration order; first to return a Response wins).
+ *
+ * That made a load-bearing surface depend on `kernel.use()` order:
+ * `plugin-hono-server` mounts `/auth/me/permissions` + `/auth/me/localization`
+ * from its own `kernel:ready` hook, objectui's whole permission layer reads the
+ * former and `core`'s auth gate allow-lists the latter — and all of it silently
+ * 404s if AuthPlugin is registered first.
+ *
+ * These tests register the catch-all FIRST and the specific route SECOND, i.e.
+ * exactly the order that used to fail, and drive a real Hono app so the
+ * assertions are about the shipped handler rather than a stand-in.
+ */
+
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { Hono } from 'hono';
+import { AuthPlugin } from './auth-plugin';
+import type { PluginContext } from '@objectstack/core';
+
+const BASE = '/api/v1/auth';
+
+/** better-auth's own 404 for a path it does not implement. */
+const BETTER_AUTH_404 = { message: 'Not Found', code: 'NOT_FOUND' };
+
+/**
+ * Mount the plugin's real route registration on a real Hono app, with
+ * better-auth stubbed by a path → Response table so we control exactly which
+ * paths the namespace owner claims.
+ */
+async function mountCatchAll(owned: Record<string, () => Response>) {
+    const app = new Hono();
+    const ctx: PluginContext = {
+        registerService: vi.fn(),
+        getService: vi.fn((name: string) => (name === 'manifest' ? { register: vi.fn() } : undefined)),
+        getServices: vi.fn(() => new Map()),
+        hook: vi.fn(),
+        trigger: vi.fn(),
+        logger: { info: vi.fn(), error: vi.fn(), warn: vi.fn(), debug: vi.fn() },
+        getKernel: vi.fn(),
+    } as any;
+
+    const plugin = new AuthPlugin({ secret: 'test-secret-at-least-32-chars-long!!' });
+    await plugin.init(ctx);
+
+    const handleRequest = vi.fn(async (req: Request) => {
+        const path = new URL(req.url).pathname;
+        const make = owned[path];
+        if (make) return make();
+        return new Response(JSON.stringify(BETTER_AUTH_404), {
+            status: 404,
+            headers: { 'Content-Type': 'application/json' },
+        });
+    });
+    (plugin as any).authManager = { handleRequest };
+
+    const httpServer: any = { getRawApp: () => app, getPort: () => 0 };
+    (plugin as any).registerAuthRoutes(httpServer, ctx);
+
+    return { app, handleRequest };
+}
+
+describe('auth catch-all yields paths better-auth does not own (#4088)', () => {
+    let mounted: Awaited<ReturnType<typeof mountCatchAll>>;
+
+    beforeEach(async () => {
+        mounted = await mountCatchAll({
+            // A path better-auth really implements, plus one that answers 401.
+            [`${BASE}/get-session`]: () => new Response(JSON.stringify({ user: null }), { status: 200 }),
+            [`${BASE}/protected`]: () => new Response(JSON.stringify({ error: 'nope' }), { status: 401 }),
+        });
+    });
+
+    it('lets a LATER-registered route answer — the order that used to 404', async () => {
+        // The real collision: plugin-hono-server mounts this from its own
+        // kernel:ready hook, which may run after AuthPlugin's.
+        mounted.app.get(`${BASE}/me/permissions`, (c) => c.json({ authenticated: true, from: 'hono-plugin' }));
+
+        const res = await mounted.app.request(`http://localhost${BASE}/me/permissions`);
+
+        expect(res.status).toBe(200);
+        expect(await res.json()).toEqual({ authenticated: true, from: 'hono-plugin' });
+    });
+
+    it('does the same for /auth/me/localization', async () => {
+        mounted.app.get(`${BASE}/me/localization`, (c) => c.json({ authenticated: true, currency: 'USD' }));
+
+        const res = await mounted.app.request(`http://localhost${BASE}/me/localization`);
+
+        expect(res.status).toBe(200);
+        expect(await res.json()).toEqual({ authenticated: true, currency: 'USD' });
+    });
+
+    it('keeps better-auth winning every path it DOES own', async () => {
+        // Precedence must still favour the namespace owner: a later route must
+        // not be able to hijack a real better-auth endpoint.
+        mounted.app.get(`${BASE}/get-session`, (c) => c.json({ hijacked: true }));
+
+        const res = await mounted.app.request(`http://localhost${BASE}/get-session`);
+
+        expect(res.status).toBe(200);
+        expect(await res.json()).toEqual({ user: null });
+    });
+
+    it('does NOT fall through on 401 — that is a real answer, not a disclaimer', async () => {
+        mounted.app.get(`${BASE}/protected`, (c) => c.json({ leaked: true }));
+
+        const res = await mounted.app.request(`http://localhost${BASE}/protected`);
+
+        expect(res.status).toBe(401);
+        expect(await res.json()).toEqual({ error: 'nope' });
+    });
+
+    it('returns better-auth’s own 404 verbatim when nothing else matches', async () => {
+        // The wire shape for a genuinely unclaimed auth path must not change:
+        // no route is registered for this path, so the fall-through finds
+        // nothing and better-auth's 404 stands.
+        const res = await mounted.app.request(`http://localhost${BASE}/no-such-endpoint`);
+
+        expect(res.status).toBe(404);
+        expect(await res.json()).toEqual(BETTER_AUTH_404);
+    });
+
+    it('still forwards to better-auth exactly once per request', async () => {
+        mounted.app.get(`${BASE}/me/permissions`, (c) => c.json({ ok: true }));
+        mounted.handleRequest.mockClear();
+
+        await mounted.app.request(`http://localhost${BASE}/me/permissions`);
+
+        expect(mounted.handleRequest).toHaveBeenCalledTimes(1);
+    });
+});

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

@@ -1814,11 +1814,56 @@ export class AuthPlugin implements Plugin {
     // Register wildcard route to forward all auth requests to better-auth.
     // better-auth is configured with basePath matching our route prefix, so we
     // forward the original request directly — no path rewriting needed.
-    rawApp.all(`${basePath}/*`, async (c: any) => {
+    //
+    // [#4088] This catch-all owns the whole auth namespace, and it used to be
+    // TERMINAL: it returned better-auth's response unconditionally, including
+    // the 404 better-auth produces for a path it does not implement. Any OTHER
+    // plugin's route under `${basePath}/*` was therefore reachable only if it
+    // happened to register FIRST — Hono runs handlers matching a path in
+    // registration order and the first to return a Response wins. That made a
+    // load-bearing surface depend on `kernel.use()` order:
+    // `plugin-hono-server` mounts `/auth/me/permissions` and
+    // `/auth/me/localization` from its own `kernel:ready` hook, the console's
+    // entire permission layer reads the former, and `core`'s auth gate
+    // allow-lists the latter — all of it silently 404s if AuthPlugin is used
+    // before HonoServerPlugin. Same class as #2567 and #4018: an invariant held
+    // by ordering luck rather than enforced.
+    //
+    // So a 404 now means "better-auth does not own this path" and we yield to
+    // whatever else matched, in either registration order. Deliberately narrow:
+    // 401/403 are real better-auth answers, not disclaimers of ownership, and
+    // when nothing downstream answers we return better-auth's own 404 verbatim
+    // so the wire shape for a genuinely unclaimed auth path is unchanged.
+    // Precedence still favours the namespace owner — better-auth wins every
+    // path it implements, and only its leftovers are up for grabs.
+    rawApp.all(`${basePath}/*`, async (c: any, next: any) => {
       try {
         // Forward the original request to better-auth handler
         const response = await this.authManager!.handleRequest(c.req.raw);
 
+        if (response.status === 404) {
+          await next();
+          // A non-404 downstream means something else answered — hand that back.
+          // NOT `c.finalized`: reaching the end of the chain with nothing
+          // matched runs Hono's notFound handler, which sets a response and
+          // flips `finalized` to true, so it cannot tell "someone answered"
+          // from "nobody did". Status can. The one thing this trades away is a
+          // downstream route's own 404 BODY (better-auth's 404 is returned in
+          // its place); the status is identical either way, and no route under
+          // this prefix answers 404 today — `/auth/me/*` answer 200 with an
+          // `authenticated: false` payload for an anonymous caller.
+          if (c.res && c.res.status !== 404) return;
+          // Assign, don't `return`: the IP-gate `rawApp.use()` above puts a
+          // middleware in this chain, and Hono's compose only assigns a
+          // handler's returned Response while `c.finalized` is false. The
+          // notFound above already flipped it, so a `return response` here is
+          // silently dropped and the caller gets Hono's "404 Not Found" text
+          // instead of better-auth's JSON. Setting `c.res` is not subject to
+          // that condition.
+          c.res = response;
+          return;
+        }
+
         // better-auth catches internal errors and returns error Responses
         // without throwing, so the catch block below would never trigger.
         // We proactively log server errors here for observability.