From 44aa098fe48e64f695fc5ad2bc2c098a9e28c6ba Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 08:38:10 +0000 Subject: [PATCH] fix(plugin-hono-server): stop gating the current-user endpoints behind registerStandardEndpoints (#4073) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `registerStandardEndpoints` gated two unrelated things behind one flag: - DUPLICATE supply — raw `POST/GET /api/v1/data/:object` (create + read only), which `@objectstack/rest` also serves and, registering first, is what actually answers; plus `/api/v1/discovery` and `/.well-known/objectstack`, which the dispatcher and REST own and which this surface already cedes to them (#4018). - SOLE supply — `/api/v1/auth/me/permissions`, `/auth/me/localization` and `/me/apps`. Nothing else in the platform mounts these: neither `packages/rest` nor `packages/runtime` registers any `/me/*` route, the console's entire permission layer reads `/auth/me/permissions` (objectui MePermissionsProvider), the console reads `/auth/me/localization` for regional defaults, and `core/security/auth-gate.ts` allow-lists `/me/apps` + `/me/localization` as endpoints a gated user MUST still reach to bootstrap the remediation UI. `os serve` gets all of it only because the flag defaults to true — the CLI constructs `new HonoServerPlugin({ port })`. So turning off a flag whose documented job is the optional CRUD/discovery convenience surface silently took the console's permissions and localization with it, and #4073's original plan (flip the default to false, then retire the surface) would have shipped exactly that outage. The three current-user endpoints now register unconditionally, from their own `kernel:ready` hook wired ahead of the gated block — same position in the ready order they had before, which is load-bearing: plugin-auth mounts a TERMINAL `rawApp.all('/api/v1/auth/*')` from its own `kernel:ready` hook, so `/auth/me/*` only wins the match by being registered first. The session → ExecutionContext resolver both groups need is extracted to `makeExecutionContextResolver` so the two agree on who the caller is by construction. Tests drive the real `start()` and fire the hooks it registered, rather than calling the registration methods by hand: with the flag OFF the three `/me/*` routes are mounted and answer (200, anonymous branch) while `/data/:object` and `/discovery` are absent; with it ON nothing changed; and `/auth/me/*` is pinned to register before the CRUD block. Also removes three unreferenced `*_ENDPOINT_PRIORITY` constants — `DISCOVERY_ENDPOINT_PRIORITY = 900` implied a route-priority mechanism that does not exist (precedence here is Hono's first-registration-wins). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_013VZLsKypjrGhiLpio2uWKu --- .../hono-current-user-endpoints-ungated.md | 42 ++++ .../src/hono-current-user-endpoints.test.ts | 118 ++++++++++ .../src/hono-discovery.test.ts | 6 + .../plugin-hono-server/src/hono-plugin.ts | 218 ++++++++++++------ 4 files changed, 312 insertions(+), 72 deletions(-) create mode 100644 .changeset/hono-current-user-endpoints-ungated.md create mode 100644 packages/plugins/plugin-hono-server/src/hono-current-user-endpoints.test.ts diff --git a/.changeset/hono-current-user-endpoints-ungated.md b/.changeset/hono-current-user-endpoints-ungated.md new file mode 100644 index 0000000000..89062221f5 --- /dev/null +++ b/.changeset/hono-current-user-endpoints-ungated.md @@ -0,0 +1,42 @@ +--- +"@objectstack/plugin-hono-server": minor +--- + +fix(plugin-hono-server): stop gating the current-user endpoints behind `registerStandardEndpoints` (#4073) + +`registerStandardEndpoints` gated two unrelated things behind one flag: + +- **Duplicate supply** — raw `POST/GET /api/v1/data/:object` (create + read + only), which `@objectstack/rest` also serves and, registering first, is what + actually answers; plus `GET /api/v1/discovery` and + `/.well-known/objectstack`, which the dispatcher and REST own and which this + surface already cedes to them (#4018). +- **Sole supply** — `GET /api/v1/auth/me/permissions`, + `/api/v1/auth/me/localization` and `/api/v1/me/apps`. Nothing else in the + platform mounts these: neither `@objectstack/rest` nor `@objectstack/runtime` + registers any `/me/*` route, the console's entire permission layer reads + `/auth/me/permissions`, the console reads `/auth/me/localization` for regional + defaults, and `core`'s auth gate allow-lists `/me/apps` + `/me/localization` + as endpoints a gated user MUST still reach to bootstrap the remediation UI. + +`os serve` gets all of it only because the flag defaults to `true` — the CLI +constructs `new HonoServerPlugin({ port })`. So `registerStandardEndpoints: +false`, whose documented job is the optional CRUD/discovery convenience surface, +silently took the console's permissions and localization down with it. + +The three current-user endpoints now register **unconditionally**, and the flag +covers the duplicate half only — what its name and docs always claimed. + +**FROM → TO.** If you set `registerStandardEndpoints: false` and worked around +the missing endpoints (proxying `/auth/me/permissions` yourself, or pinning the +flag to `true` purely to keep them), you can drop that workaround: the endpoints +are now present either way. No route is removed and no response shape changes, +so a host that left the flag at its default sees no difference. If you relied on +`false` meaning "this plugin mounts no `/api/v1` routes at all", that is no +longer true — it never was for `os serve`, which is the only host that shipped +the flag's default. + +Also removes three unreferenced `*_ENDPOINT_PRIORITY` constants; +`DISCOVERY_ENDPOINT_PRIORITY = 900` in particular implied a route-priority +mechanism that does not exist (precedence here is Hono's +first-registration-wins). diff --git a/packages/plugins/plugin-hono-server/src/hono-current-user-endpoints.test.ts b/packages/plugins/plugin-hono-server/src/hono-current-user-endpoints.test.ts new file mode 100644 index 0000000000..7995ee9594 --- /dev/null +++ b/packages/plugins/plugin-hono-server/src/hono-current-user-endpoints.test.ts @@ -0,0 +1,118 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #4073 — `registerStandardEndpoints` used to gate two unrelated things: +// +// * DUPLICATE supply — raw `/data` C+R that `@objectstack/rest` also serves +// (and, registering first, really serves), plus a discovery the dispatcher +// and REST own (#4018). +// * SOLE supply — `/auth/me/permissions`, `/auth/me/localization`, `/me/apps`. +// Nothing else in the platform mounts these: `packages/rest` and +// `packages/runtime` register no `/me/*` route, the console's whole +// permission layer reads `/auth/me/permissions`, and `core`'s auth gate +// allow-lists `/me/apps` + `/me/localization` as endpoints a gated user MUST +// still reach. `os serve` gets them only via the flag's `true` default. +// +// So turning the flag off took the console down with it. These tests pin the +// split: the flag now covers the duplicate half only, and the current-user +// endpoints are registered whatever it says. + +import { describe, it, expect, vi } from 'vitest'; +import { HonoServerPlugin } from './hono-plugin'; + +const ME_ROUTES = [ + '/api/v1/auth/me/permissions', + '/api/v1/auth/me/localization', + '/api/v1/me/apps', +]; + +/** + * Boot a plugin through its real `start()` and fire the `kernel:ready` hooks it + * registered — the actual wiring, not a hand-picked pair of method calls, so a + * regression that re-gates the current-user endpoints is caught here. + */ +async function boot(registerStandardEndpoints: boolean) { + const plugin = new HonoServerPlugin({ port: 0, registerStandardEndpoints, cors: false }); + const readyHooks: Array<() => unknown> = []; + const ctx: any = { + logger: { info() {}, debug() {}, warn() {}, error() {} }, + getKernel: () => ({ hasPlugin: () => false, getService: () => undefined }), + registerService: () => {}, + hook: (event: string, fn: () => unknown) => { + if (event === 'kernel:ready') readyHooks.push(fn); + }, + getService: vi.fn(() => undefined), + }; + + await plugin.init(ctx); + await plugin.start(ctx); + for (const fn of readyHooks) await fn(); + + return (plugin as any).server.getRawApp(); +} + +/** Registered paths on the live Hono app, ignoring middleware catch-alls. */ +function paths(app: any): string[] { + return (app.routes ?? []).map((r: any) => r.path); +} + +describe('current-user endpoints are not gated by registerStandardEndpoints (#4073)', () => { + it('mounts /me/* with the convenience surface OFF', async () => { + const app = await boot(false); + for (const route of ME_ROUTES) { + expect(paths(app), `${route} must survive registerStandardEndpoints:false`).toContain(route); + } + }); + + it('mounts /me/* with the convenience surface ON (unchanged for os serve)', async () => { + const app = await boot(true); + for (const route of ME_ROUTES) expect(paths(app)).toContain(route); + }); + + it('still gates the duplicate half — no /data CRUD, no /discovery when OFF', async () => { + const app = await boot(false); + const registered = paths(app); + + expect(registered).not.toContain('/api/v1/data/:object'); + expect(registered).not.toContain('/api/v1/discovery'); + expect(registered).not.toContain('/.well-known/objectstack'); + }); + + it('registers the duplicate half when ON', async () => { + const registered = paths(await boot(true)); + + expect(registered).toContain('/api/v1/data/:object'); + expect(registered).toContain('/api/v1/discovery'); + }); + + it('answers /me/* with the flag OFF instead of 404ing', async () => { + const app = await boot(false); + + // No auth service is wired, so each endpoint takes its anonymous branch + // — which is a real answer, not the "route does not exist" 404 the + // console used to get when the flag was off. + const permissions = await app.request('http://localhost/api/v1/auth/me/permissions'); + expect(permissions.status).toBe(200); + expect(await permissions.json()).toEqual({ authenticated: false }); + + const localization = await app.request('http://localhost/api/v1/auth/me/localization'); + expect(localization.status).toBe(200); + expect(await localization.json()).toEqual({ authenticated: false }); + + const apps = await app.request('http://localhost/api/v1/me/apps'); + expect(apps.status).toBe(200); + expect(await apps.json()).toEqual({ apps: [] }); + }); + + it('registers /me/* BEFORE the CRUD block — the order plugin-auth collides with', async () => { + // plugin-auth mounts a TERMINAL `rawApp.all('/api/v1/auth/*')` from its + // own kernel:ready hook, so `/auth/me/*` only wins the match by being + // registered first. The split must not have moved these later. + const registered = paths(await boot(true)); + const firstMe = registered.indexOf('/api/v1/auth/me/permissions'); + const firstData = registered.indexOf('/api/v1/data/:object'); + + expect(firstMe).toBeGreaterThanOrEqual(0); + expect(firstData).toBeGreaterThanOrEqual(0); + expect(firstMe).toBeLessThan(firstData); + }); +}); diff --git a/packages/plugins/plugin-hono-server/src/hono-discovery.test.ts b/packages/plugins/plugin-hono-server/src/hono-discovery.test.ts index 155637e193..5a7a70012f 100644 --- a/packages/plugins/plugin-hono-server/src/hono-discovery.test.ts +++ b/packages/plugins/plugin-hono-server/src/hono-discovery.test.ts @@ -35,6 +35,12 @@ function bootStandardEndpoints(installedPlugins: string[] = []) { hook: () => {}, getService: () => undefined, }; + // Same order `start()` wires the two `kernel:ready` hooks in: the + // current-user endpoints are registered unconditionally and first (#4073), + // the CRUD + discovery surface only under `registerStandardEndpoints`. + // Discovery is computed from what is really mounted, so a boot that skipped + // the `/auth/me/*` helpers would under-report `routes.auth`. + (plugin as any).registerCurrentUserEndpoints(ctx); (plugin as any).registerDiscoveryAndCrudEndpoints(ctx); return (plugin as any).server.getRawApp(); } diff --git a/packages/plugins/plugin-hono-server/src/hono-plugin.ts b/packages/plugins/plugin-hono-server/src/hono-plugin.ts index ee4dbe6619..42f8be23f8 100644 --- a/packages/plugins/plugin-hono-server/src/hono-plugin.ts +++ b/packages/plugins/plugin-hono-server/src/hono-plugin.ts @@ -52,7 +52,20 @@ export interface HonoPluginOptions { */ restConfig?: RestServerConfig; /** - * Whether to register standard ObjectStack CRUD endpoints + * Whether to register the standalone CRUD + discovery convenience surface: + * raw `POST/GET /api/v1/data/:object` (create + read only) and + * `GET /api/v1/discovery` / `/.well-known/objectstack`. + * + * Every one of these is DUPLICATE supply. `@objectstack/rest` serves full + * `/data` CRUD and, registering first, is what actually answers; the + * dispatcher and REST own discovery and this surface cedes it to them when + * either is present (#4018). The flag exists for a bare host that mounts + * neither. + * + * It does NOT gate the current-user endpoints (`/auth/me/permissions`, + * `/auth/me/localization`, `/me/apps`) — this plugin is their only provider + * anywhere, so they register unconditionally (#4073). + * * @default true */ registerStandardEndpoints?: boolean; @@ -414,10 +427,12 @@ export class HonoServerPlugin implements Plugin { type = 'server'; version = '0.9.0'; - // Constants - private static readonly DEFAULT_ENDPOINT_PRIORITY = 100; - private static readonly CORE_ENDPOINT_PRIORITY = 950; - private static readonly DISCOVERY_ENDPOINT_PRIORITY = 900; + // No endpoint-priority constants: three of them (DEFAULT/CORE/DISCOVERY) + // sat here unreferenced by anything in the repo, and `DISCOVERY_ENDPOINT_ + // PRIORITY = 900` in particular implied a priority mechanism that does not + // exist — route precedence here is Hono's first-registration-wins, which is + // exactly what the #4018 cede and the `kernel:ready` ordering above turn on. + // Removed (#4073) so the file stops advertising a dead concept. private options: HonoPluginOptions; private server: HonoHttpServer; @@ -756,8 +771,21 @@ export class HonoServerPlugin implements Plugin { }); } - // Register standard endpoints during kernel:ready so they're - // wired up alongside other plugins' route registrations. + // Register endpoints during kernel:ready so they're wired up alongside + // other plugins' route registrations. + // + // The current-user endpoints go first and are NOT gated (#4073): this + // plugin is their only provider on any host, so they must not depend on + // a flag whose stated job is the optional CRUD/discovery convenience + // surface. Registering them ahead of that block also keeps their + // position in the `kernel:ready` order exactly where it was, which + // matters: plugin-auth mounts a TERMINAL `rawApp.all('/api/v1/auth/*')` + // from its own `kernel:ready` hook, and `/auth/me/*` only wins the match + // by being registered first. + ctx.hook('kernel:ready', async () => { + this.registerCurrentUserEndpoints(ctx); + }); + if (this.options.registerStandardEndpoints) { ctx.hook('kernel:ready', async () => { this.registerDiscoveryAndCrudEndpoints(ctx); @@ -938,6 +966,87 @@ export class HonoServerPlugin implements Plugin { // Basic CRUD data endpoints — delegate to ObjectQL service directly const getObjectQL = () => ctx.getService('objectql'); + // Session → ExecutionContext. Shared with the always-registered + // current-user endpoints below, which resolve the same principal. + const resolveCtx = this.makeExecutionContextResolver(ctx); + + // Create + rawApp.post(`${prefix}/data/:object`, async (c: any) => { + const ql = getObjectQL(); + if (!ql) return c.json({ error: 'Data service not available' }, 503); + const object = c.req.param('object'); + const data = await c.req.json().catch(() => ({})); + const execCtx = await resolveCtx(c); + const denied = denyAnonymous(c, execCtx); + if (denied) return denied; + try { + const res = await ql.insert(object, data, { context: execCtx } as any); + const record = { ...data, ...res }; + return c.json({ object, id: record.id, record }); + } catch (err: any) { + if (err?.code === 'PERMISSION_DENIED' || err?.name === 'PermissionDeniedError') { + return c.json({ error: err.message ?? 'Forbidden' }, 403); + } + throw err; + } + }); + + // Get by ID + rawApp.get(`${prefix}/data/:object/:id`, async (c: any) => { + const ql = getObjectQL(); + if (!ql) return c.json({ error: 'Data service not available' }, 503); + const object = c.req.param('object'); + const id = c.req.param('id'); + const execCtx = await resolveCtx(c); + const denied = denyAnonymous(c, execCtx); + if (denied) return denied; + try { + let all = await ql.find(object, { context: execCtx } as any); + if (!all) all = []; + const match = all.find((i: any) => i.id === id); + return match ? c.json({ object, id, record: match }) : c.json({ error: 'Not found' }, 404); + } catch (err: any) { + if (err?.code === 'PERMISSION_DENIED' || err?.name === 'PermissionDeniedError') { + return c.json({ error: err.message ?? 'Forbidden' }, 403); + } + throw err; + } + }); + + // Find / List + rawApp.get(`${prefix}/data/:object`, async (c: any) => { + const ql = getObjectQL(); + if (!ql) return c.json({ error: 'Data service not available' }, 503); + const object = c.req.param('object'); + const execCtx = await resolveCtx(c); + const denied = denyAnonymous(c, execCtx); + if (denied) return denied; + try { + let all = await ql.find(object, { context: execCtx } as any); + if (!Array.isArray(all) && all && (all as any).value) all = (all as any).value; + if (!all) all = []; + return c.json({ object, records: all, total: all.length }); + } catch (err: any) { + if (err?.code === 'PERMISSION_DENIED' || err?.name === 'PermissionDeniedError') { + return c.json({ error: err.message ?? 'Forbidden' }, 403); + } + throw err; + } + }); + + ctx.logger.debug('Registered standard CRUD data endpoints', { prefix }); + } + + /** + * Build the session → `ExecutionContext` resolver both route groups need. + * + * Extracted from `registerDiscoveryAndCrudEndpoints` when the current-user + * endpoints stopped being gated on `registerStandardEndpoints` (#4073): they + * resolve the same principal the `/data` routes do, and one resolver is the + * only way the two groups can agree on who the caller is. + */ + private makeExecutionContextResolver(ctx: PluginContext) { + const getObjectQL = () => ctx.getService('objectql'); // Helper: resolve ExecutionContext from request headers (cookie session // or API key). Mirrors the runtime's resolveExecutionContext but // self-contained to avoid a cross-package dep. We DO query the @@ -1116,70 +1225,36 @@ export class HonoServerPlugin implements Plugin { return undefined; } }; + return resolveCtx; + } - // Create - rawApp.post(`${prefix}/data/:object`, async (c: any) => { - const ql = getObjectQL(); - if (!ql) return c.json({ error: 'Data service not available' }, 503); - const object = c.req.param('object'); - const data = await c.req.json().catch(() => ({})); - const execCtx = await resolveCtx(c); - const denied = denyAnonymous(c, execCtx); - if (denied) return denied; - try { - const res = await ql.insert(object, data, { context: execCtx } as any); - const record = { ...data, ...res }; - return c.json({ object, id: record.id, record }); - } catch (err: any) { - if (err?.code === 'PERMISSION_DENIED' || err?.name === 'PermissionDeniedError') { - return c.json({ error: err.message ?? 'Forbidden' }, 403); - } - throw err; - } - }); - - // Get by ID - rawApp.get(`${prefix}/data/:object/:id`, async (c: any) => { - const ql = getObjectQL(); - if (!ql) return c.json({ error: 'Data service not available' }, 503); - const object = c.req.param('object'); - const id = c.req.param('id'); - const execCtx = await resolveCtx(c); - const denied = denyAnonymous(c, execCtx); - if (denied) return denied; - try { - let all = await ql.find(object, { context: execCtx } as any); - if (!all) all = []; - const match = all.find((i: any) => i.id === id); - return match ? c.json({ object, id, record: match }) : c.json({ error: 'Not found' }, 404); - } catch (err: any) { - if (err?.code === 'PERMISSION_DENIED' || err?.name === 'PermissionDeniedError') { - return c.json({ error: err.message ?? 'Forbidden' }, 403); - } - throw err; - } - }); - - // Find / List - rawApp.get(`${prefix}/data/:object`, async (c: any) => { - const ql = getObjectQL(); - if (!ql) return c.json({ error: 'Data service not available' }, 503); - const object = c.req.param('object'); - const execCtx = await resolveCtx(c); - const denied = denyAnonymous(c, execCtx); - if (denied) return denied; - try { - let all = await ql.find(object, { context: execCtx } as any); - if (!Array.isArray(all) && all && (all as any).value) all = (all as any).value; - if (!all) all = []; - return c.json({ object, records: all, total: all.length }); - } catch (err: any) { - if (err?.code === 'PERMISSION_DENIED' || err?.name === 'PermissionDeniedError') { - return c.json({ error: err.message ?? 'Forbidden' }, 403); - } - throw err; - } - }); + /** + * Current-user endpoints — `/auth/me/permissions`, `/auth/me/localization` + * and `/me/apps`. Registered UNCONDITIONALLY, unlike the CRUD + discovery + * block above (#4073). + * + * They used to ride on `registerStandardEndpoints`, which conflated two + * unrelated things. That flag covers DUPLICATE supply — raw `/data` CRUD + * that `@objectstack/rest` also serves (and, being registered first, really + * serves), plus a discovery that the dispatcher/REST own (#4018). These + * three are the opposite: nothing else in the platform mounts them. + * `packages/rest` and `packages/runtime` register no `/me/*` route at all, + * the console reads `/auth/me/permissions` for its whole permission layer + * and `/auth/me/localization` for regional defaults, and + * `core/security/auth-gate.ts` allow-lists `/me/apps` + `/me/localization` + * as endpoints a gated user MUST still reach to bootstrap the remediation + * UI. `os serve` gets them only because `registerStandardEndpoints` + * defaults to true (`cli/src/commands/serve.ts` passes just `{ port }`), so + * turning that flag off — or retiring the convenience surface it names — + * would have taken the console down with it. + * + * Splitting them out is what makes that flag mean what it says, and is the + * precondition for retiring the duplicate half. + */ + private registerCurrentUserEndpoints(ctx: PluginContext) { + const rawApp = this.server.getRawApp(); + const prefix = '/api/v1'; + const resolveCtx = this.makeExecutionContextResolver(ctx); // Effective permissions for the current user — single aggregation // endpoint that resolves session → roles → permission sets → merged @@ -1518,8 +1593,7 @@ export class HonoServerPlugin implements Plugin { return c.json({ apps: [] }); } }); - - ctx.logger.debug('Registered standard CRUD data endpoints', { prefix }); + ctx.logger.debug('Registered current-user endpoints', { prefix }); } /**