fix(runtime): canonicalise the API root to the discovery route so a gated bare-root request is not refused (#17625)

.changeset/17625-api-root-is-the-discovery-route.md

@@ -0,0 +1,76 @@
+---
+'@objectstack/runtime': minor
+---
+
+fix(runtime): the API root is the discovery route, under a second spelling — a gated session's `GET ${prefix}/` reaches discovery again (#17625)
+
+`HttpDispatcher.dispatch()` strips one trailing slash, so both root spellings it
+accepts collapsed onto the empty string: `${prefix}/` arrives as `/` and
+`${prefix}` arrives as `` (the MSW / base-URL-stripped form). Only the discovery
+branch at the foot of the method knew that empty string meant the API root. The
+ADR-0069 authentication-policy gate, which runs far above it, did not.
+
+That disagreement was invisible while `isAuthGateAllowlisted` answered `true`
+for a falsy path. objectstack#7898 made the predicate fail-closed at the source
+— exemption is now something a path EARNS by naming an allow-listed route — and
+the bare-root discovery request started answering 403 for a session carrying an
+`authGate` posture (expired password, required MFA):
+
+```
+FROM  GET ${prefix}/   (session with user.authGate)  ->  200  discovery document
+TO    GET ${prefix}/   (session with user.authGate)  ->  403  PASSWORD_EXPIRED   // regression
+NOW   GET ${prefix}/   (session with user.authGate)  ->  200  discovery document
+```
+
+**Normalising the root to `/` is measured insufficient and is not what landed.**
+`isAuthGateAllowlisted('/')` is `false` — a segment-less path matches no
+`ALLOW_ROUTES` entry — and the discovery branch tests `/discovery` or the empty
+string, neither of which `/` satisfies. `'' -> '/'` therefore relocates the 403
+rather than removing it. Both legs are pinned upstream in
+`packages/core/src/security/auth-gate.test.ts` ("does not exempt the dispatcher
+bare-root `cleanPath` — step 2 is #17625").
+
+The root is canonicalised to `/discovery` instead — the route it has always
+served — read from one constant by both the canonicalisation and the branch that
+serves it, so the two cannot drift into a third disagreement about what the
+empty path means.
+
+**⛔ No allow-list was widened and `packages/core` is untouched.** The only input
+whose gate answer moves is the API root, and it gains exactly the exemption
+`/discovery` already carried, by BEING that route — no new information is
+reachable, since `/discovery` was already exempt and already outside the
+project-membership skip check. A caller that reaches the gate with no path at
+all is still refused at the predicate, and the pathless case stays declared
+where it lives (`shouldDenyAnonymous`) rather than re-derived at this seam.
+
+**What does NOT change.** `${prefix}` with no trailing slash keeps serving the
+same document; the named `/discovery` route is untouched; the
+environment-scoped root `${prefix}/environments/<id>` keeps its own answer,
+which matched no allow-listed route before objectstack#7898 either. `//` strips
+to `/`, not to the empty string, so it is not the root and is not canonicalised.
+
+**Why `minor` on a change whose commit type is `fix`.** The two are independent
+and the floor is mechanical, not editorial: this PR's clause ② is declared
+affirmative, and the maintainer's ruling of 2026-09-04 (decision batch #35, on
+objectstack#15294) puts an affirmative clause ② on a package whose
+`packages/**/src/**` the diff moves at AT LEAST `minor` — *the commit type may
+raise a bump but never lower it below what the act requires*, written out under
+"WHICH LEVEL" in the `Check Changeset` step of
+`.github/workflows/pr-automation.yml`. ⛔ So the reading that this is "a 403 that
+should be a 200, therefore a patch" is an argument about INTENT and does not
+reach the level: the act re-admits an input class the merged tree refuses, on an
+authorisation surface, and that is what the level grades. The commit type stays
+`fix(runtime)`, because the type describes the act and the level prices it.
+
+**ADR-0087 disposition: no ledger entry is owed and no marker is required.**
+This changeset declares no breaking change, which is the only condition under
+which `check:adr-0087-registration` demands a disposition marker. On the
+substance: no ADR-0087 shape surface moved — the diff touches one
+`packages/runtime` transport file and its sibling test, no `*.zod.ts`, no
+`packages/spec/**`, no `packages/spec/src/contracts/**` entry and no object
+definition — so `objectstack migrate meta` has nothing to reach, and no
+authorable metadata key, accept set or stored shape changes. Nor is this an
+ADR-0087 conversion-layer entry: nothing lenient is being accepted from a
+metadata producer. One transport's two spellings of its own route are being
+reconciled to the route's own name, which is the opposite direction — a dialect
+removed, not tolerated.

packages/runtime/src/http-dispatcher.root-auth-gate.test.ts

@@ -0,0 +1,200 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * [#17625, the runtime half of #7898's ruling A] The API root reaches the
+ * discovery payload for a GATED session.
+ *
+ * ## What moved, and why this file exists at all
+ *
+ * `dispatch()` strips one trailing slash, so BOTH root spellings the
+ * dispatcher accepts — `${prefix}/` (arriving as `'/'`) and `${prefix}`
+ * (arriving as `''`, the MSW/base-URL-stripped form) — used to travel on as
+ * `''`. Only the discovery branch at the foot of the method knew that meant
+ * the API root; the ADR-0069 gate, which runs far above it, did not. While
+ * `isAuthGateAllowlisted` answered `true` for a falsy path that disagreement
+ * was invisible. #7898 made the predicate fail-closed, and the bare-root
+ * discovery request started answering 403.
+ *
+ * ⚠️ The obvious repair is measured WRONG upstream and must not be re-tried
+ * here: normalising `'' → '/'` relocates the 403 instead of removing it.
+ * `isAuthGateAllowlisted('/')` is `false` (a segment-less path matches no
+ * `ALLOW_ROUTES` entry) and the discovery branch tests `'/discovery'` or `''`,
+ * which `'/'` satisfies neither. Both legs are pinned in
+ * `packages/core/src/security/auth-gate.test.ts` → "does not exempt the
+ * dispatcher bare-root `cleanPath` — step 2 is #17625".
+ *
+ * The delivered repair canonicalises the root to `'/discovery'` — the route it
+ * has always served — so the gate and the branch read one spelling. ⛔ Core is
+ * untouched and `ALLOW_ROUTES` is unchanged: the root gains exactly the
+ * exemption `/discovery` already carried, and gains it by BEING that route.
+ *
+ * ## The genuinely-absent-path leg is NOT restated here
+ *
+ * "A caller that reaches the gate with no path at all is still refused" is
+ * `packages/core`'s pin, delivered by #7898's own round
+ * (`auth-gate.test.ts` → "[#7898] a falsy path is not exempt (fail-closed)",
+ * which drives `isAuthGateAllowlisted` over `undefined`, `null` and `''`).
+ * ⛔ Restating it against `HttpDispatcher` would measure nothing new: this
+ * transport has no pathless call shape — `dispatch()` takes `path: string` and
+ * the root canonicalisation below is reached only from the two ROOT spellings.
+ * Referenced, not duplicated.
+ *
+ * ## Why every case runs on a fixture whose gate is provably ON
+ *
+ * `enforceAuthGate` fails open in a great many ways — no `auth` service, no
+ * `isAuthGateActive`, no `getSession`, an unreadable header bag, any thrown
+ * error — and under every one of them a 200 on the root is indistinguishable
+ * from the repair working. So the gated fixture below is paired with a
+ * POSITIVE CONTROL on a protected path in the same `describe`: if the control
+ * stops answering 403 with the gate's own code, every other case in this file
+ * is measuring a gate that is simply off, and the file says so by going red.
+ */
+
+import { describe, it, expect, vi } from 'vitest';
+import { HttpDispatcher } from './http-dispatcher.js';
+
+/** A session user carrying an ADR-0069 gate posture (`normalizeAuthGate`'s shape). */
+const GATED_USER = {
+    id: 'u_gated',
+    authGate: { code: 'PASSWORD_EXPIRED', message: 'Your password has expired.' },
+};
+
+/** The same user with no gate — the negative-direction control. */
+const UNGATED_USER = { id: 'u_clear' };
+
+/** A path nothing allow-lists, used as the gate's positive control. */
+const PROTECTED_PATH = '/data/task';
+
+function makeDispatcher(sessionUser: unknown, gateActive = true) {
+    const services: Record<string, any> = {
+        objectql: {
+            find: vi.fn().mockResolvedValue([]),
+            getObjects: vi.fn().mockReturnValue({}),
+            registry: {
+                getObject: vi.fn().mockReturnValue(null),
+                getRegisteredTypes: vi.fn().mockReturnValue([]),
+            },
+        },
+        auth: {
+            isAuthGateActive: () => gateActive,
+            getApi: async () => ({ getSession: async () => ({ user: sessionUser }) }),
+        },
+    };
+    const kernel: any = {
+        getState: () => 'running',
+        getService: (n: string) => services[n] ?? null,
+        getServiceAsync: async (n: string) => services[n] ?? null,
+        context: { getService: (n: string) => services[n] ?? null },
+    };
+    return new HttpDispatcher(kernel, undefined, { enforceProjectMembership: false });
+}
+
+/**
+ * Drive one request and hand back the result plus the context the dispatcher
+ * wrote through. `routePath` is the value `prepareResolverHints` recorded, and
+ * therefore the spelling every stage below it — the gate included — was handed.
+ */
+async function dispatch(sessionUser: unknown, method: string, path: string, gateActive = true) {
+    const dispatcher = makeDispatcher(sessionUser, gateActive);
+    const context: any = { request: new Request(`http://localhost/api/v1${path}`) };
+    const result = await dispatcher.dispatch(method, path, undefined, {}, context, '/api/v1');
+    return { result, context };
+}
+
+/** The gate's 403 carries its `code` in the envelope's `details` (`error(msg, 403, { code })`). */
+const gateCodeOf = (result: any) =>
+    result.response?.body?.error?.details?.code ?? result.response?.body?.error?.code;
+
+describe('[#17625] the API root resolves to the discovery route for a gated session', () => {
+    it('⭐ POSITIVE CONTROL — this fixture really does gate: a protected path answers 403 with the gate code', async () => {
+        // ⛔ Do not delete or weaken this. `enforceAuthGate` fails open on any
+        // hiccup, so without a request that the SAME fixture refuses, every
+        // 200 below is compatible with "the gate never ran".
+        const { result } = await dispatch(GATED_USER, 'GET', PROTECTED_PATH);
+        expect(result.handled).toBe(true);
+        expect(result.response?.status).toBe(403);
+        expect(gateCodeOf(result)).toBe('PASSWORD_EXPIRED');
+    });
+
+    it('PIN 1 — `GET ${prefix}/` returns the discovery payload (was 403 after #7898)', async () => {
+        const { result } = await dispatch(GATED_USER, 'GET', '/');
+        expect(result.handled).toBe(true);
+        expect(result.response?.status).toBe(200);
+        // The discovery document itself, not merely "not a 403".
+        expect(result.response?.body?.data?.name).toBe('ObjectOS');
+        expect(result.response?.body?.data?.routes).toBeDefined();
+    });
+
+    it('PIN 2 — `GET ${prefix}` (no trailing slash) is unchanged: still the discovery payload', async () => {
+        const { result } = await dispatch(GATED_USER, 'GET', '');
+        expect(result.handled).toBe(true);
+        expect(result.response?.status).toBe(200);
+        expect(result.response?.body?.data?.name).toBe('ObjectOS');
+        expect(result.response?.body?.data?.routes).toBeDefined();
+    });
+
+    it('serves the SAME document for both root spellings and for the named route', async () => {
+        // One route, three spellings — the property the canonicalisation buys.
+        const [slash, bare, named] = await Promise.all([
+            dispatch(GATED_USER, 'GET', '/'),
+            dispatch(GATED_USER, 'GET', ''),
+            dispatch(GATED_USER, 'GET', '/discovery'),
+        ]);
+        for (const r of [slash, bare, named]) expect(r.result.response?.status).toBe(200);
+        expect(slash.result.response?.body?.data).toEqual(named.result.response?.body?.data);
+        expect(bare.result.response?.body?.data).toEqual(named.result.response?.body?.data);
+    });
+
+    it('⭐ THE MECHANISM — the gate is handed the allow-listed route NAME, never `""` or `"/"`', async () => {
+        // This is the assertion that makes the repair the RULED one rather than
+        // a coincidence: both root spellings are canonicalised BEFORE the gate,
+        // so what the gate evaluates is `/discovery` — a name `ALLOW_ROUTES`
+        // already carries. ⛔ If this ever reads `'/'`, the fix has regressed to
+        // the shape upstream measured insufficient, and PIN 1 would only still
+        // pass because something else started exempting the root.
+        for (const path of ['/', '']) {
+            const { context } = await dispatch(GATED_USER, 'GET', path);
+            expect(context.routePath, path).toBe('/discovery');
+        }
+        // …and a path that is NOT the root is not rewritten.
+        const { context } = await dispatch(GATED_USER, 'GET', PROTECTED_PATH);
+        expect(context.routePath).toBe(PROTECTED_PATH);
+    });
+
+    it('⭐ NEGATIVE-DIRECTION CONTROL — the repair narrows nothing: an UNGATED session still reads the root', async () => {
+        for (const path of ['/', '', '/discovery']) {
+            const { result } = await dispatch(UNGATED_USER, 'GET', path);
+            expect(result.response?.status, path).toBe(200);
+            expect(result.response?.body?.data?.name, path).toBe('ObjectOS');
+        }
+    });
+
+    it('the named `/discovery` route keeps answering for a gated session', async () => {
+        const { result } = await dispatch(GATED_USER, 'GET', '/discovery');
+        expect(result.response?.status).toBe(200);
+        expect(result.response?.body?.data?.name).toBe('ObjectOS');
+    });
+});
+
+describe('[#17625] the boundary — what the canonicalisation deliberately does NOT move', () => {
+    it('the ENVIRONMENT-SCOPED root keeps its own answer: `${prefix}/environments/<id>` is still gated', async () => {
+        // ⚠️ A different input class, and deliberately untouched. The gate runs
+        // BEFORE the scoped-URL strip, so this request is judged on
+        // `/environments/<id>` — which matched no `ALLOW_ROUTES` entry before
+        // #7898 either, so its answer did not move in that card and must not
+        // move in this one. The `''` the strip produces afterwards is why the
+        // discovery branch keeps its `''` arm.
+        const { result } = await dispatch(GATED_USER, 'GET', '/environments/env-1');
+        expect(result.response?.status).toBe(403);
+        expect(gateCodeOf(result)).toBe('PASSWORD_EXPIRED');
+    });
+
+    it('a non-root path that merely LOOKS empty after the strip is not the root', async () => {
+        // `//` strips to `'/'`, not to `''`, so it is not canonicalised and is
+        // not exempt — recorded so a later reader does not widen the rule into
+        // "any number of trailing slashes is the root".
+        const { result, context } = await dispatch(GATED_USER, 'GET', '//');
+        expect(context.routePath).toBe('/');
+        expect(result.response?.status).toBe(403);
+    });
+});