diff --git a/.changeset/endpoint-surfaces-announce-only-served.md b/.changeset/endpoint-surfaces-announce-only-served.md new file mode 100644 index 0000000000..27b7cc1da8 --- /dev/null +++ b/.changeset/endpoint-surfaces-announce-only-served.md @@ -0,0 +1,38 @@ +--- +'@objectstack/rest': minor +--- + +The two machine-readable endpoint surfaces announce only the declarations the runtime actually serves + +`GET {basePath}/meta/api` and `GET {basePath}/openapi.json` enumerated declared `api` items +through the metadata protocol (ObjectQL SchemaRegistry + `sys_metadata`). Whether a declared +route is SERVED is decided by a different reader — `IMetadataService.matchEndpoint` and the +endpoint matcher behind it, which sees the metadata manager's registry and its registered +loaders. A real boot measured the two disagreeing: an `api` row written through +`PUT /meta/api/{name}` was enumerated by both surfaces — the OpenAPI document publishing it as +a path with `security: []`, i.e. as needing no credentials — while every request to it answered +404. + +Both surfaces now ask the matcher, per declaration, and announce only what comes back. An +`/openapi.json` is what SDKs, codegen and AI clients generate from, so an endpoint advertised +there that does not exist propagates into everything built on top of it. + +**What changes for you:** an `api` declaration that this runtime will not serve disappears from +both surfaces. That covers a row created by a runtime/Studio metadata write rather than +published from a stack artifact, and one excluded at load by the ADR-0121 publish gates (for +example `authRequired: false` with no armed `rateLimit`). If a declaration you expected has +vanished, it was already answering 404 — the surface has stopped mis-reporting it, and the +server log now names each omitted declaration, its route, and why. Publish it through a gated +path (a stack artifact, or `publishPackage` with the package's `manifest.namespace`) to make it +real. Endpoints declared in a stack artifact are unaffected: they are served, so they are still +listed and still documented in full. + +Two surfaces deliberately keep their previous behaviour: `GET /meta/api?preview=draft` answers +"what is pending", which is by construction not the served set, and the single-item +`GET|PUT|DELETE /meta/api/{name}` routes stay reachable so an unserved declaration can still be +inspected and removed. + +Hosts that embed `RestServer` directly get a new optional final constructor argument, +`metadataServiceProvider`, resolving the `metadata` service. `rest-api-plugin` wires it; a host +that does not pass it keeps the old enumerate-everything behaviour and logs, once, that the +surfaces can no longer promise they describe only served routes. diff --git a/packages/rest/package.json b/packages/rest/package.json index c753ac5847..c4138ad002 100644 --- a/packages/rest/package.json +++ b/packages/rest/package.json @@ -29,6 +29,7 @@ "zod": "^4.4.3" }, "devDependencies": { + "@objectstack/metadata": "workspace:*", "@objectstack/metadata-protocol": "workspace:*", "@objectstack/objectql": "workspace:*", "@objectstack/service-analytics": "workspace:*", diff --git a/packages/rest/src/openapi-endpoints.ts b/packages/rest/src/openapi-endpoints.ts index 67b3ff5228..aed3bb58c9 100644 --- a/packages/rest/src/openapi-endpoints.ts +++ b/packages/rest/src/openapi-endpoints.ts @@ -64,6 +64,24 @@ * `/openapi.json` describing its two declared endpoints * (`packages/qa/dogfood/test/showcase-declarative-endpoints.dogfood.test.ts`). * + * ## What it is HANDED (#5224) + * + * Its production caller no longer passes the enumerated `api` items. It passes + * the set the endpoint matcher confirmed it will serve (`served-endpoints.ts`), + * because enumeration and service are two different readers and a real boot + * measured them disagreeing: a row written through `PUT /meta/api/{name}` was + * enumerated, never matched, and published here as a live path with + * `security: []` while every request to it answered 404. + * + * That makes the parse-and-resolve-duplicates pass below DOWNSTREAM of the + * authority rather than a second opinion beside it — with a matcher-narrowed + * input its duplicate branch cannot fire, since the matcher already resolved + * every route to one owner. It is kept because this function is exported and + * unit-tested as a unit: it must still refuse to document a half-valid shape + * when handed raw items directly. What it must never become is the place where + * "will this be served" is decided; that question has one owner, and asking it + * is the caller's job. + * * The empty-set case is still exact rather than approximate — with nothing to * add, {@link enrichOpenApiWithEndpoints} returns its input document BY * REFERENCE — which is what keeps a deployment that declares no endpoint diff --git a/packages/rest/src/rest-api-plugin.ts b/packages/rest/src/rest-api-plugin.ts index 5ad7627bdb..57b4692c4e 100644 --- a/packages/rest/src/rest-api-plugin.ts +++ b/packages/rest/src/rest-api-plugin.ts @@ -6,6 +6,7 @@ import { RestServerConfig } from '@objectstack/spec/api'; import { registerPackageRoutes } from './package-routes.js'; import { registerExternalDatasourceRoutes } from './external-datasource-routes.js'; import type { PackageService } from '@objectstack/service-package'; +import type { IMetadataService } from '@objectstack/spec/contracts'; import { SysImportJob } from '@objectstack/platform-objects/audit'; export interface RestApiPluginConfig { @@ -236,6 +237,21 @@ export function createRestApiPlugin(config: RestApiPluginConfig = {}): Plugin { } catch { return undefined; } }; + // [#5224] Metadata service resolver — the endpoint matcher behind + // `IMetadataService.matchEndpoint`. The two machine-readable + // endpoint faces (`GET /meta/api`, `GET /openapi.json`) ask it + // whether a declared route is actually served before announcing it, + // so neither can publish an endpoint that answers 404. Returns + // undefined when no `metadata` service is registered; the faces + // then say so loudly rather than inventing a verdict. + const metadataServiceProvider = async ( + _environmentId?: string, + ): Promise => { + try { + return ctx.getService('metadata'); + } catch { return undefined; } + }; + // Security service resolver — used by the ADR-0090 D5/D9 // /security/suggested-bindings routes and the D6 /security/explain // route (plugin-security). Returns undefined when plugin-security @@ -264,7 +280,7 @@ export function createRestApiPlugin(config: RestApiPluginConfig = {}): Plugin { try { return ctx.getService(name) != null; } catch { return false; } }; try { - const restServer = new RestServer(server, protocol, config.api as any, kernelManager, envRegistry, defaultEnvironmentIdProvider, authServiceProvider, objectQLProvider, emailServiceProvider, sharingServiceProvider, reportsServiceProvider, approvalsServiceProvider, sharingRulesServiceProvider, i18nServiceProvider, analyticsServiceProvider, settingsServiceProvider, serviceExistsProvider, securityServiceProvider, requestEnvResolver); + const restServer = new RestServer(server, protocol, config.api as any, kernelManager, envRegistry, defaultEnvironmentIdProvider, authServiceProvider, objectQLProvider, emailServiceProvider, sharingServiceProvider, reportsServiceProvider, approvalsServiceProvider, sharingRulesServiceProvider, i18nServiceProvider, analyticsServiceProvider, settingsServiceProvider, serviceExistsProvider, securityServiceProvider, requestEnvResolver, metadataServiceProvider); restServer.registerRoutes(); ctx.logger.info('REST API successfully registered'); diff --git a/packages/rest/src/rest-endpoint-surfaces-served-only.test.ts b/packages/rest/src/rest-endpoint-surfaces-served-only.test.ts new file mode 100644 index 0000000000..5c4412da12 --- /dev/null +++ b/packages/rest/src/rest-endpoint-surfaces-served-only.test.ts @@ -0,0 +1,366 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#5224] The two machine-readable endpoint faces announce ONLY what the +// endpoint matcher will serve. +// +// --------------------------------------------------------------------------- +// The measured break +// --------------------------------------------------------------------------- +// On a real showcase boot (`objectstack dev --fresh`, 47 plugins) a single +// metadata write reproduced it end to end: +// +// PUT /api/v1/meta/api/e8_backdoor -> 200 {"success":true,...} +// GET /api/v1/apps/showcase/backdoor -> 404 (anonymous AND authenticated) +// GET /api/v1/meta/api -> 3 items, incl. e8_backdoor +// GET /api/v1/openapi.json -> paths["/api/v1/apps/showcase/backdoor"] +// = {"get":{...,"security":[]}} +// +// Two readers, two answers. The faces enumerate through +// `protocol.getMetaItems` (ObjectQL SchemaRegistry + `sys_metadata`); service +// is decided by `IMetadataService.matchEndpoint` -> `EndpointMatcher` -> +// `MetadataManager.listForIndex('api')` (the manager's registry + its +// registered loaders, `filesystem` / `memory` on dev and serve). A row written +// through the metadata write path lands only in the first, so the faces +// advertised an anonymous, unmetered endpoint that does not exist — and +// `/openapi.json` is what SDKs, codegen and AI clients generate from +// (AGENTS.md Route & surface ownership Rule 4, ADR-0076 D12). +// +// --------------------------------------------------------------------------- +// Why a real MetadataManager and not a stub +// --------------------------------------------------------------------------- +// Two of the three pins are only meaningful against the real matcher: pin (1) +// needs an item the manager genuinely cannot see, and pin (3) needs the #5189 / +// #5040 E7b load gate to genuinely exclude one. A stubbed authority would pin +// this file's own idea of those rules instead of the runtime's. +// +// --------------------------------------------------------------------------- +// Reverse verification, direction predicted BEFORE running +// --------------------------------------------------------------------------- +// Reverting the narrowing (documenting `apiItems` again, dropping the `api` +// branch in `GET /meta/:type`) turns pins (1) and (3) RED on both faces — they +// assert on absence that only the filter produces. +// +// Pin (2) is deliberately NOT of that shape and does not flip: a stack-artifact +// endpoint is announced before AND after. Stating that plainly matters more +// than a uniform-looking table — it is the no-collateral-damage guard, and a +// guard that changed colour with the fix would be pinning the fix rather than +// the invariant. Both predictions were confirmed by running it (see the PR). + +import { describe, it, expect, vi } from 'vitest'; +import { MetadataManager } from '@objectstack/metadata'; +import { MemoryLoader } from '@objectstack/metadata'; +import { RestServer } from './rest-server'; + +// --------------------------------------------------------------------------- +// The three declarations, each standing for one way a route can exist +// --------------------------------------------------------------------------- + +/** (2) Published through a stack artifact — the matcher serves it. */ +const SERVED = { + name: 'showcase_task_feed', + path: '/api/v1/apps/showcase/tasks', + method: 'GET', + summary: 'Task feed', + type: 'object_operation', + target: 'showcase_task', + objectParams: { object: 'showcase_task', operation: 'find' }, + authRequired: true, + _packageId: 'com.example.showcase', + _provenance: 'package', +}; + +/** + * (1) The measured row: written straight through the metadata write path, so + * `getMetaItems` enumerates it and the matcher's index never contains it. + */ +const RUNTIME_WRITTEN = { + name: 'e8_backdoor', + path: '/api/v1/apps/showcase/backdoor', + method: 'GET', + type: 'object_operation', + target: 'showcase_task', + objectParams: { object: 'showcase_task', operation: 'find' }, + authRequired: false, +}; + +/** + * (3) Reaches the matcher's store but is EXCLUDED at load by the identity-free + * publish gates (#5189, #5040 E7b): ADR-0121 D6 forbids an anonymous endpoint + * with no armed `rateLimit`, and the runtime honours `authRequired: false` + * faithfully, so a bypassed D6 would mint an anonymous zero-quota entry point. + * The gate makes it answer 404; this pins that the faces agree with the gate. + */ +const GATE_EXCLUDED = { + name: 'anon_unmetered', + path: '/api/v1/apps/showcase/open', + method: 'GET', + type: 'object_operation', + target: 'showcase_task', + objectParams: { object: 'showcase_task', operation: 'find' }, + authRequired: false, +}; + +const ALL_ENUMERATED = [SERVED, RUNTIME_WRITTEN, GATE_EXCLUDED]; + +// --------------------------------------------------------------------------- +// Harness +// --------------------------------------------------------------------------- + +function createMockServer() { + return { + get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn(), patch: vi.fn(), use: vi.fn(), + listen: vi.fn().mockResolvedValue(undefined), close: vi.fn().mockResolvedValue(undefined), + }; +} + +function makeRes() { + const res: any = { statusCode: 200, body: undefined }; + res.status = vi.fn((c: number) => { res.statusCode = c; return res; }); + res.json = vi.fn((b: any) => { res.body = b; return res; }); + res.header = vi.fn(() => res); res.setHeader = vi.fn(); res.write = vi.fn(); res.end = vi.fn(); res.send = vi.fn(); + return res; +} + +/** + * A metadata manager holding what a dev/serve boot's manager holds: the stack + * artifact's endpoints and anything a plugin registered — never the + * `sys_metadata` rows, which is the whole point. + */ +async function managerHolding(items: Array>): Promise { + const manager = new MetadataManager({ formats: ['json'], loaders: [new MemoryLoader()] }); + for (const item of items) await manager.register('api', String(item.name), item); + return manager; +} + +/** + * Mount the REST server with the endpoint-match authority wired exactly where + * `rest-api-plugin` wires it — POSITIONALLY, so this also pins the parameter + * slot the plugin passes it in. + */ +function mountRest(enumerated: Array>, authority: unknown | undefined) { + const protocol: any = { + getDiscovery: vi.fn().mockResolvedValue({ + version: 'v0', endpoints: { data: '', metadata: '', ui: '', auth: '/auth' }, + }), + getMetaTypes: vi.fn().mockResolvedValue([]), + getMetaItems: vi.fn(async ({ type }: any) => { + const t = String(type ?? ''); + if (t === 'api' || t === 'apis') return { type: 'api', items: enumerated }; + return { type: t, items: [] }; + }), + getMetaItem: vi.fn().mockResolvedValue({}), + findData: vi.fn().mockResolvedValue([]), + }; + + const rest = new RestServer( + createMockServer() as any, + protocol, + { api: { requireAuth: false, version: 'v1' } } as any, + undefined, undefined, undefined, undefined, undefined, undefined, undefined, + undefined, undefined, undefined, undefined, undefined, undefined, undefined, + undefined, undefined, + authority === undefined ? undefined : async () => authority, + ); + // The metadata routes deny anonymous callers unconditionally (#3963); an + // identity is a precondition of reaching the handler, not the subject here. + (rest as any).resolveExecCtx = async () => ({ userId: 'u1' }); + rest.registerRoutes(); + (rest as any).registerOpenApiEndpoints('/api/v1'); + return { rest, protocol }; +} + +async function getMetaApi(rest: any, type = 'api', query: Record = {}) { + const route = rest.getRoutes().find((r: any) => r.method === 'GET' && r.path === '/api/v1/meta/:type'); + if (!route) throw new Error('meta/:type route not registered'); + const res = makeRes(); + await route.handler({ method: 'GET', params: { type }, query, body: {}, headers: {} }, res); + return res; +} + +async function getOpenApi(rest: any) { + const entry = (rest as any).routeManager.get('GET', '/api/v1/openapi.json'); + if (!entry) throw new Error('openapi.json route not registered'); + const res = makeRes(); + await entry.handler({ method: 'GET', headers: { host: 'example.test' }, params: {}, query: {}, path: '/api/v1/openapi.json' }, res); + return res; +} + +const announcedNames = (body: any): string[] => { + const list = Array.isArray(body) ? body : (body?.items ?? []); + return list.map((i: any) => i?.name).sort(); +}; + +const declaredPaths = (doc: any): string[] => + Object.keys(doc?.paths ?? {}).filter((p) => p.startsWith('/api/v1/apps/')).sort(); + +// --------------------------------------------------------------------------- +// Pins +// --------------------------------------------------------------------------- + +describe('#5224 — GET /meta/api announces only what the matcher serves', () => { + it('omits the runtime-written row the matcher cannot see, and the gate-excluded one', async () => { + const manager = await managerHolding([SERVED, GATE_EXCLUDED]); + const { rest } = mountRest(ALL_ENUMERATED, manager); + + // Ground truth first: this is what the runtime will and will not serve. + expect((await manager.matchEndpoint({ method: 'GET', path: SERVED.path }))?.endpoint.name) + .toBe('showcase_task_feed'); + expect(await manager.matchEndpoint({ method: 'GET', path: RUNTIME_WRITTEN.path })).toBeUndefined(); + expect(await manager.matchEndpoint({ method: 'GET', path: GATE_EXCLUDED.path })).toBeUndefined(); + + const res = await getMetaApi(rest); + + expect(res.statusCode).toBe(200); + // Before the fix: ['anon_unmetered', 'e8_backdoor', 'showcase_task_feed']. + expect(announcedNames(res.body)).toEqual(['showcase_task_feed']); + }, 60_000); + + it('answers the STORED item, decorations intact — the narrowing is a filter, not a rewrite', async () => { + const manager = await managerHolding([SERVED]); + const { rest } = mountRest(ALL_ENUMERATED, manager); + + const res = await getMetaApi(rest); + const item = (res.body?.items ?? res.body)[0]; + + // `_packageId` / `_provenance` are what the Studio list reads; replacing + // the row with the matcher's parsed value would drop them silently. + expect(item._packageId).toBe('com.example.showcase'); + expect(item._provenance).toBe('package'); + expect(item.summary).toBe('Task feed'); + }, 60_000); + + it('applies to the PLURAL spelling too — `/meta/apis` is the canonical REST form', async () => { + const manager = await managerHolding([SERVED]); + const { rest } = mountRest(ALL_ENUMERATED, manager); + + expect(announcedNames((await getMetaApi(rest, 'apis')).body)).toEqual(['showcase_task_feed']); + }, 60_000); + + it('leaves `?preview=draft` unfiltered — that surface answers "what is PENDING"', async () => { + // A draft is by construction not served and not meant to look served, so + // narrowing the drafts view to the served set would empty it of exactly the + // items it exists to show. Codegen and SDK clients read the plain list. + const manager = await managerHolding([SERVED]); + const { rest } = mountRest(ALL_ENUMERATED, manager); + + expect(announcedNames((await getMetaApi(rest, 'api', { preview: 'draft' })).body)) + .toEqual(['anon_unmetered', 'e8_backdoor', 'showcase_task_feed']); + }, 60_000); + + it('does not narrow any OTHER metadata type — the special case is `api`-only', async () => { + const manager = await managerHolding([]); + const { rest, protocol } = mountRest(ALL_ENUMERATED, manager); + + await getMetaApi(rest, 'view'); + // For every other type, "listed" and "in effect" are the same fact and the + // enumerating reader already answered it; nothing extra is asked. + expect(protocol.getMetaItems).toHaveBeenCalledWith(expect.objectContaining({ type: 'view' })); + }, 60_000); + + it('reports a store outage rather than an empty declaration set', async () => { + // `matchEndpoint` throws when its store cannot be read (contract; ADR-0110 + // D3). Answering 200 with zero items would state, confidently, that the + // deployment declares no endpoints. + const outage = { matchEndpoint: async () => { throw new Error('metadata store unreachable'); } }; + const { rest } = mountRest(ALL_ENUMERATED, outage); + + const res = await getMetaApi(rest); + // The pin is that the request FAILS rather than answering a set. The exact + // status is not this change's to decide: an unrecognised error reaching + // `handleRouteError` lands on `mapDataError`'s terminal fallback, which + // this route measured at 400 — a pre-existing classification shared by + // every error on the metadata routes, not a consequence of the narrowing. + // Asserting 5xx here would pin someone else's bug as if it were fixed. + expect(res.statusCode).toBeGreaterThanOrEqual(400); + expect(res.body?.items ?? res.body).not.toEqual([SERVED]); + }, 60_000); +}); + +describe('#5224 — GET /openapi.json documents only what the matcher serves', () => { + it('drops the runtime-written path that used to ship with `security: []`', async () => { + const manager = await managerHolding([SERVED, GATE_EXCLUDED]); + const { rest } = mountRest(ALL_ENUMERATED, manager); + + const doc = (await getOpenApi(rest)).body; + + expect(declaredPaths(doc)).toEqual(['/api/v1/apps/showcase/tasks']); + // The measured lie, gone: a path advertised as needing no credentials while + // answering 404 propagates into every client generated from this document. + expect(doc.paths['/api/v1/apps/showcase/backdoor']).toBeUndefined(); + expect(doc.paths['/api/v1/apps/showcase/open']).toBeUndefined(); + }, 60_000); + + it('keeps the served endpoint fully described — no collateral damage', async () => { + const manager = await managerHolding([SERVED]); + const { rest } = mountRest(ALL_ENUMERATED, manager); + + const op = (await getOpenApi(rest)).body.paths['/api/v1/apps/showcase/tasks'].get; + + expect(op.operationId).toBe('showcase_task_feed'); + expect(op.summary).toBe('Task feed'); + expect(op.responses['200']).toBeDefined(); + expect(op.responses['401']).toBeDefined(); + // `security: []` belongs ONLY to a genuinely anonymous served endpoint. + expect(op.security).not.toEqual([]); + }, 60_000); + + it('documents the MATCHER’s value when the two readers disagree about the same route', async () => { + // The whole defect is that the two readers can hold different states of + // the same `api` row. When they do, the document must describe the one the + // runtime will act on — the matcher's parsed endpoint — not the enumerated + // copy. Written as a discriminating pin on purpose: asserting only that + // `authRequired` defaults to `true` would pass either way, because the + // enrichment parses whatever it is handed. + const stale = { ...SERVED, summary: 'STALE — the enumerated copy', authRequired: false }; + const manager = await managerHolding([SERVED]); + const { rest } = mountRest([stale], manager); + + const op = (await getOpenApi(rest)).body.paths['/api/v1/apps/showcase/tasks'].get; + expect(op.summary).toBe('Task feed'); + // …and therefore no `security: []` derived from the stale copy's + // `authRequired: false`. + expect(op.security).not.toEqual([]); + expect(op.responses['401']).toBeDefined(); + }, 60_000); + + it('ships a document with no declared paths when the matcher serves nothing', async () => { + const manager = await managerHolding([]); + const { rest } = mountRest(ALL_ENUMERATED, manager); + + expect(declaredPaths((await getOpenApi(rest)).body)).toEqual([]); + }, 60_000); +}); + +describe('#5224 — an ABSENT authority degrades loudly, never silently', () => { + it('keeps enumerating and says so once when no matcher is reachable', async () => { + // A host that never wired the metadata service cannot be told a verdict + // that was never computed — inventing "nothing is served" would blank a + // correct document. It falls back to the stored set and NAMES the loss. + const errors: string[] = []; + const spy = vi.spyOn(console, 'error').mockImplementation((...a: unknown[]) => { errors.push(String(a[0])); }); + try { + const { rest } = mountRest(ALL_ENUMERATED, undefined); + + expect(announcedNames((await getMetaApi(rest)).body)).toHaveLength(3); + expect(errors.some((e) => e.includes('no endpoint matcher is reachable'))).toBe(true); + + // Once per server, not once per request. + const before = errors.filter((e) => e.includes('no endpoint matcher is reachable')).length; + await getMetaApi(rest); + await getOpenApi(rest); + expect(errors.filter((e) => e.includes('no endpoint matcher is reachable')).length).toBe(before); + } finally { + spy.mockRestore(); + } + }, 60_000); + + it('treats an occupant WITHOUT `matchEndpoint` as absent, not as "serves nothing"', async () => { + const spy = vi.spyOn(console, 'error').mockImplementation(() => {}); + try { + const { rest } = mountRest(ALL_ENUMERATED, { register: () => undefined }); + expect(announcedNames((await getMetaApi(rest)).body)).toHaveLength(3); + } finally { + spy.mockRestore(); + } + }, 60_000); +}); diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 80ce7fb0a5..0348bb755b 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -44,6 +44,11 @@ import { import { runImport } from './import-runner.js'; import { prepareImportRequest, isMetaEnvelope } from './import-prepare.js'; import { enrichOpenApiWithEndpoints } from './openapi-endpoints.js'; +import { + isEndpointMatchAuthority, + selectServedEndpoints, + type EndpointMatchAuthority, +} from './served-endpoints.js'; // Node-safe logger — avoids importing 'console' which is absent from ES2020 lib typings. const logError = (...args: unknown[]) => (globalThis as any).console?.error(...args); @@ -1494,6 +1499,20 @@ export class RestServer { * capability gates (ADR-0057 D10) — resolveExecCtx sets no kernel in * single-kernel deployments, so this prevents the gate failing open. */ private serviceExistsProvider?: (name: string) => boolean; + /** + * [#5224] `metadata` service resolver — the endpoint matcher behind + * `IMetadataService.matchEndpoint`, and therefore the ONE authority on + * which declared `api` route the runtime actually serves. Read by the two + * machine-readable endpoint faces (`GET /meta/api`, `GET /openapi.json`) + * so neither announces a declaration that answers 404. + */ + private metadataServiceProvider?: (environmentId?: string) => Promise; + /** + * One-shot latch for the "no matcher wired" degradation notice below, so a + * host that never wired {@link metadataServiceProvider} says so once per + * server rather than once per request. + */ + private warnedMissingEndpointAuthority = false; /** * In-flight async import jobs the caller has asked to cancel. The worker * checks membership at each progress boundary and stops cooperatively. This @@ -1522,6 +1541,7 @@ export class RestServer { serviceExistsProvider?: (name: string) => boolean, securityServiceProvider?: (environmentId?: string) => Promise, requestEnvResolver?: RestRequestEnvResolver, + metadataServiceProvider?: (environmentId?: string) => Promise, ) { this.protocol = protocol; this.config = this.normalizeConfig(config); @@ -1542,6 +1562,73 @@ export class RestServer { this.serviceExistsProvider = serviceExistsProvider; this.securityServiceProvider = securityServiceProvider; this.requestEnvResolver = requestEnvResolver; + this.metadataServiceProvider = metadataServiceProvider; + } + + /** + * Resolve the endpoint matcher for this request — the authority the two + * machine-readable endpoint faces consult before announcing anything + * (#5224). + * + * Same lookup chain as {@link resolveProtocol}: the per-request kernel when + * one is resolvable (a multi-tenant host must ask the REQUEST's own + * matcher, or one environment's declarations would describe another's + * URLs), else the single-kernel provider `rest-api-plugin` wires. + * + * Returns `undefined` when nothing in the chain can answer — including when + * the resolved occupant of the `metadata` slot carries no `matchEndpoint`, + * which is a legal shape (the contract method is optional). Callers must + * decide what an ABSENT authority means for their surface rather than + * having a verdict invented here; see the call sites. + */ + private async resolveEndpointMatchAuthority( + environmentId?: string, + req?: any, + ): Promise { + let envId: string | undefined; + try { + // Shared resolution entry point (ADR-0076 D11 step 4), the same one + // `resolveProtocol` / `resolveI18nService` use — so the face reads + // the matcher of the environment whose items it just enumerated. + envId = await this.resolveRequestEnvironmentId(environmentId, req); + } catch { /* fall through to the single-kernel provider */ } + + if (envId && envId !== 'platform' && this.kernelManager) { + try { + const kernel = await this.kernelManager.getOrCreate(envId); + const svc = await kernel.getServiceAsync('metadata'); + if (isEndpointMatchAuthority(svc)) return svc; + } catch { /* fall through */ } + } + if (this.metadataServiceProvider) { + try { + const svc = await this.metadataServiceProvider(envId); + if (isEndpointMatchAuthority(svc)) return svc; + } catch { /* an unreachable provider is an ABSENT authority */ } + } + return undefined; + } + + /** + * Say — once per server — that no endpoint matcher is reachable, so the + * endpoint faces cannot promise they describe only served routes. + * + * Loud rather than silent (AGENTS.md, Route & surface ownership Rule 3): + * without the authority these surfaces fall back to enumerating what is + * STORED, which is exactly the pre-#5224 behaviour and exactly the state + * that can advertise a route answering 404. Reported at `error` because the + * consequence is a contract face that may lie, and the remedy is a wiring + * change the operator can make. + */ + private notifyMissingEndpointAuthority(surface: string): void { + if (this.warnedMissingEndpointAuthority) return; + this.warnedMissingEndpointAuthority = true; + logError( + `[REST] no endpoint matcher is reachable (no \`metadata\` service with \`matchEndpoint\`), so ${surface} ` + + `cannot narrow declared \`api\` items to the ones this runtime actually serves. It is enumerating ` + + `what is STORED instead, which may advertise routes that answer 404. Wire the metadata service ` + + `into the REST server (rest-api-plugin does this) to restore the guarantee.`, + ); } /** @@ -3027,10 +3114,38 @@ export class RestServer { const apiItems: unknown[] = Array.isArray((apiResult as any)?.items) ? (apiResult as any).items : Array.isArray(apiResult) ? apiResult as unknown[] : []; - enriched = enrichOpenApiWithEndpoints(enriched, apiItems, { + const endpointLogger = { error: (message: string, meta?: unknown) => meta === undefined ? logError(message) : logError(message, meta), - }); + }; + + // [#5224] Enumerated is not served. This document is what + // SDKs, codegen and AI clients build clients FROM, so it + // describes only the declarations the endpoint matcher will + // actually answer — asked of the matcher itself, the sole + // holder of that verdict. A stored row the matcher cannot + // see used to arrive here and be published as a real path, + // `security: []` and all, while every request to it 404'd. + let documentable: unknown[] = apiItems; + const authority = apiItems.length > 0 + ? await this.resolveEndpointMatchAuthority( + isScoped ? req.params?.environmentId : undefined, + req, + ) + : undefined; + if (apiItems.length > 0 && !authority) { + this.notifyMissingEndpointAuthority('GET /openapi.json'); + } else if (authority) { + // The matcher's OWN parsed endpoint is documented, not + // the stored JSON: schema defaults are materialized on + // it (most importantly `authRequired`), so the document + // describes the value the runtime acts on rather than a + // re-parse of the same row on a second code path. + documentable = (await selectServedEndpoints(apiItems, authority, endpointLogger)) + .map((s) => s.endpoint); + } + + enriched = enrichOpenApiWithEndpoints(enriched, documentable, endpointLogger); } catch (err: any) { // A store that cannot be read must not take the document // down with it — but say so, because a silently endpoint- @@ -3431,6 +3546,65 @@ export class RestServer { // objectql implementation actually returns the raw // array. Handle both shapes defensively. let visible: any = items; + + // [#5224] `api` is a CONTRACT face, so it announces only + // the declarations the endpoint matcher will actually + // serve — the same set `/openapi.json` documents, asked + // of the same authority. + // + // The special case is `api`-only and stays that way on + // purpose: for every other type "listed" and "in effect" + // are the same fact, resolved by the one reader that + // enumerated them. For `api` they are not — a declared + // route is served by `IMetadataService.matchEndpoint`, + // whose index is a different reader with a different + // reach, and it is the SOLE holder of that verdict. So + // the special case is not "api is special", it is "api + // is the one type whose service verdict lives somewhere + // this route cannot see without asking". + // + // `?preview=draft` is exempt: that surface exists to + // answer "what is PENDING", which is by construction not + // the served set (a draft is not live and is not meant to + // look live). Filtering it would empty the drafts view of + // a type whose drafts are legitimately unserved. Codegen + // and SDK clients read the plain list, which is filtered. + if (RestServer.metaTypeSingular(req.params.type) === 'api' && !previewDrafts) { + const raw = visible as unknown; + const list = RestServer.metaItemsArray(raw); + if (list.length > 0) { + const authority = await this.resolveEndpointMatchAuthority(environmentId, req); + if (!authority) { + this.notifyMissingEndpointAuthority('GET /meta/api'); + } else { + // A `matchEndpoint` throw propagates: its + // contract distinguishes an unreadable store + // from a miss, so this route FAILS (through + // `handleRouteError`) rather than claiming + // the deployment declares nothing. Measured: + // that failure is currently reported as 400, + // because an unrecognised error lands on + // `mapDataError`'s terminal fallback — a + // pre-existing classification on every error + // this route reports, not something this + // narrowing chose. Filed separately; do not + // read the propagation here as a promise + // about which status arrives. + const servedList = await selectServedEndpoints(list, authority, { + error: (message: string, meta?: unknown) => + meta === undefined ? logError(message) : logError(message, meta), + }); + // The STORED item is what this face answers + // with — its `_packageId` / `_provenance` / + // `_diagnostics` decorations are read by the + // Studio list, and dropping them here would + // be a second, unannounced change. + const filtered = servedList.map((s) => s.item); + visible = Array.isArray(raw) ? filtered : { ...(raw as any), items: filtered }; + } + } + } + if (RestServer.metaTypeSingular(req.params.type) === 'app') { const raw = items as unknown; const list: any[] | null = Array.isArray(raw) diff --git a/packages/rest/src/served-endpoints.test.ts b/packages/rest/src/served-endpoints.test.ts new file mode 100644 index 0000000000..69dc145fb0 --- /dev/null +++ b/packages/rest/src/served-endpoints.test.ts @@ -0,0 +1,163 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#5224] `selectServedEndpoints` — the unit under the two machine-readable +// endpoint faces. +// +// The break it closes was measured on a real showcase boot: an `api` row +// written straight through `PUT /meta/api/{name}` is enumerated by +// `protocol.getMetaItems` (ObjectQL SchemaRegistry + `sys_metadata`) and +// invisible to `IMetadataService.matchEndpoint` (the manager's registry + +// its registered loaders), so `/meta/api` and `/openapi.json` both announced +// `GET /api/v1/apps/showcase/backdoor` — the latter with `security: []` — while +// every request to it answered 404. +// +// What is pinned here is the SEMANTICS of the narrowing; the wiring into both +// faces is pinned by `rest-endpoint-surfaces-served-only.test.ts` against the +// real `MetadataManager`. + +import { describe, it, expect, vi } from 'vitest'; +import type { ApiEndpointMatch } from '@objectstack/spec/contracts'; +import { + isEndpointMatchAuthority, + selectServedEndpoints, + type EndpointMatchAuthority, +} from './served-endpoints'; + +const TASKS = { + name: 'showcase_task_feed', + path: '/api/v1/apps/showcase/tasks', + method: 'GET', + type: 'object_operation', + target: 'showcase_task', + objectParams: { object: 'showcase_task', operation: 'find' }, + authRequired: true, + _packageId: 'com.example.showcase', +}; + +const BACKDOOR = { + name: 'e8_backdoor', + path: '/api/v1/apps/showcase/backdoor', + method: 'GET', + type: 'object_operation', + target: 'showcase_task', + objectParams: { object: 'showcase_task', operation: 'find' }, + authRequired: false, +}; + +/** An authority that serves exactly the routes it was given. */ +function authorityServing(...endpoints: Array>): EndpointMatchAuthority { + const index = new Map(); + for (const e of endpoints) { + index.set(`${String(e.method).toUpperCase()} ${String(e.path)}`, { + endpoint: e as never, + params: {}, + }); + } + return { + matchEndpoint: vi.fn(async ({ method, path }) => + index.get(`${String(method).toUpperCase()} ${String(path)}`)), + }; +} + +const silent = { error: vi.fn() }; + +describe('selectServedEndpoints — announce only what the matcher serves', () => { + it('keeps a declaration the matcher resolves to the SAME name', async () => { + const served = await selectServedEndpoints([TASKS], authorityServing(TASKS), silent); + + expect(served.map((s) => s.item)).toEqual([TASKS]); + // The matcher's own parsed value is carried alongside the stored item — + // the OpenAPI face documents that one, so schema defaults are materialized + // rather than re-derived on a second code path. + expect(served[0]?.endpoint.name).toBe('showcase_task_feed'); + }); + + it('drops the declaration the matcher does not know — the #5224 defect', async () => { + // The exact measured shape: both rows are enumerated, only one is served. + const served = await selectServedEndpoints([TASKS, BACKDOOR], authorityServing(TASKS), silent); + + expect(served.map((s) => (s.item as { name: string }).name)).toEqual(['showcase_task_feed']); + }); + + it('names every omission at `error`, in one aggregated line', async () => { + const logger = { error: vi.fn() }; + await selectServedEndpoints([TASKS, BACKDOOR], authorityServing(TASKS), logger); + + expect(logger.error).toHaveBeenCalledTimes(1); + const [message, meta] = logger.error.mock.calls[0]!; + expect(message).toContain('e8_backdoor'); + expect(message).toContain('GET /api/v1/apps/showcase/backdoor'); + // Absence must be loud AND actionable: the line says what happens to the + // route and how to make the declaration real. + expect(message).toContain('404'); + expect(message).toContain('publishPackage'); + expect((meta as { omitted: unknown[] }).omitted).toHaveLength(1); + }); + + it('says nothing when every declaration is served', async () => { + const logger = { error: vi.fn() }; + await selectServedEndpoints([TASKS], authorityServing(TASKS), logger); + + expect(logger.error).not.toHaveBeenCalled(); + }); + + it('drops the LOSER of a duplicate route claim, keeping the matcher its winner', async () => { + // Two items claim `GET /api/v1/apps/showcase/tasks`. `buildEndpointIndex` + // resolves that lexicographically-first-`name`-wins, and the loser is + // served by nobody — so it must not be announced either. The rule itself is + // never restated here: the answer comes back from the matcher. + const loser = { ...TASKS, name: 'z_late_claim' }; + const served = await selectServedEndpoints([TASKS, loser], authorityServing(TASKS), silent); + + expect(served.map((s) => (s.item as { name: string }).name)).toEqual(['showcase_task_feed']); + }); + + it('drops an item with no usable name / method / path', async () => { + const logger = { error: vi.fn() }; + const served = await selectServedEndpoints( + [{ name: 'half', method: 'GET' }, { path: '/api/v1/apps/x/y', method: 'GET' }], + authorityServing(TASKS), + logger, + ); + + expect(served).toEqual([]); + expect(logger.error).toHaveBeenCalledTimes(1); + expect(logger.error.mock.calls[0]![0]).toContain('no usable name/method/path'); + }); + + it('an empty input asks the matcher nothing and reports nothing', async () => { + const authority = authorityServing(TASKS); + const logger = { error: vi.fn() }; + + expect(await selectServedEndpoints([], authority, logger)).toEqual([]); + expect(authority.matchEndpoint).not.toHaveBeenCalled(); + expect(logger.error).not.toHaveBeenCalled(); + }); + + it('PROPAGATES a store outage instead of reporting an empty served set', async () => { + // `matchEndpoint` throws when its store cannot be read, precisely so an + // outage cannot masquerade as a miss (its contract; ADR-0110 D3 for the + // singular read). Swallowing it here would turn an unreadable store into + // the confident claim that the deployment declares no endpoints. + const authority: EndpointMatchAuthority = { + matchEndpoint: vi.fn(async () => { throw new Error('metadata store unreachable'); }), + }; + + await expect(selectServedEndpoints([TASKS], authority, silent)) + .rejects.toThrow('metadata store unreachable'); + }); +}); + +describe('isEndpointMatchAuthority', () => { + it('accepts a service carrying `matchEndpoint`', () => { + expect(isEndpointMatchAuthority({ matchEndpoint: () => undefined })).toBe(true); + }); + + it('rejects an occupant without it — the contract method is OPTIONAL', () => { + // `runAppEndpointStep` makes the identical probe and serves nothing when it + // fails; a face that assumed the method exists would throw on a legal shape. + expect(isEndpointMatchAuthority({ register: () => undefined })).toBe(false); + expect(isEndpointMatchAuthority(undefined)).toBe(false); + expect(isEndpointMatchAuthority(null)).toBe(false); + }); +}); diff --git a/packages/rest/src/served-endpoints.ts b/packages/rest/src/served-endpoints.ts new file mode 100644 index 0000000000..25f48c7adf --- /dev/null +++ b/packages/rest/src/served-endpoints.ts @@ -0,0 +1,209 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * DECLARED -> SERVED: the filter both machine-readable endpoint faces run + * before they announce anything (#5224). + * + * ## The break this closes + * + * `GET /meta/api` and `GET /openapi.json` enumerated `api` items through + * `protocol.getMetaItems` — ObjectQL's SchemaRegistry plus the `sys_metadata` + * rows. The thing that decides whether a declared route is actually SERVED is + * a different reader entirely: `IMetadataService.matchEndpoint` -> + * `EndpointMatcher` -> `MetadataManager.listForIndex('api')`, which sees the + * manager's own registry and its registered loaders (`filesystem`, `memory`) + * and nothing else. A real showcase boot measured the gap exactly: a row + * written straight through `PUT /meta/api/{name}` is enumerated by + * `getMetaItems` and invisible to the matcher, so both faces advertised + * `GET /api/v1/apps/showcase/backdoor` — the OpenAPI one with `security: []`, + * i.e. as an endpoint needing no credentials — while every request to it + * answered 404. + * + * A wrong answer on `/openapi.json` does not stay on `/openapi.json`: it is + * the document SDKs, codegen and AI clients build from, so a fabricated public + * endpoint propagates into everything generated from it. "Machine-readable + * surfaces must not lie" (AGENTS.md, Route & surface ownership Rule 4; + * ADR-0076 D12) is the invariant, and this module is how the two faces keep it. + * + * ## The shape: the faces ASK the matcher, they do not re-derive it + * + * There is exactly one authority on "will this declaration be served" — the + * endpoint matcher — and it is reachable through a contract method that + * already exists, `IMetadataService.matchEndpoint`. So this module asks it, + * once per candidate, and keeps only what comes back. It deliberately does + * NOT re-implement the matcher's judgement (parse, the ADR-0121 identity-free + * publish gates, duplicate resolution): a second copy of that judgement is a + * second truth source, which is the disease and not the cure — and it would go + * stale the first time the matcher's rules changed, silently, in the direction + * of announcing more than is served. + * + * It equally does not touch the matcher's own data source. Whether + * `matchEndpoint` and `getMetaItems` should read ONE store is a real and + * larger architectural question (the issue's first option); this module is + * correct under either answer, because it never assumes where the matcher + * looks — only that it is the one that decides. + * + * ## Why the match is confirmed by NAME, not by mere presence + * + * Two stored items may claim the same METHOD+path. `buildEndpointIndex` + * resolves that deterministically — lexicographically-first `name` keeps the + * route — and the loser is served by nobody. Asking "does something own this + * route" would announce both claimants; asking "does THIS declaration own this + * route" announces the winner alone, which is what the runtime does. The + * duplicate rule therefore stays spelled in exactly one place, and this file is + * not one of them. + * + * ## An outage is not an empty set + * + * `matchEndpoint` throws when its store cannot be read, precisely so an outage + * cannot masquerade as a miss (its contract in + * `packages/spec/src/contracts/metadata-service.ts`, and ADR-0110 D3 for the + * singular read). This module preserves that: a throw propagates to the caller + * untouched. Swallowing it into "nothing matched" would turn a store outage + * into the confident claim that a deployment declares no endpoints — the same + * class of lie, pointing the other way. + * + * ## Omissions are named + * + * Every dropped declaration is reported at `error`, in ONE aggregated line per + * call rather than one per item, naming each omitted declaration and the fact + * that its route answers 404. An endpoint an author declared and the runtime + * will not serve is a capability that silently went missing, and absence must + * be loud (AGENTS.md, Route & surface ownership Rule 3). The message says the + * face is now telling the truth, so the line is not misread as this filter + * having broken something. + */ + +import type { ApiEndpoint } from '@objectstack/spec/api'; +import type { ApiEndpointMatch } from '@objectstack/spec/contracts'; + +/** + * The one question the faces ask: does the runtime serve this METHOD+path, and + * which declaration owns it? + * + * Structurally `Pick` with the optionality + * removed — the same narrow slice `runAppEndpointStep` takes in + * `@objectstack/runtime`, for the same reason: the caller needs one method, and + * naming only that method keeps this package free of any dependency on the + * metadata engine. + */ +export interface EndpointMatchAuthority { + matchEndpoint(query: { path: string; method: string }): Promise; +} + +/** Where an omitted declaration is reported. */ +export interface ServedEndpointLogger { + error(message: string, meta?: unknown): void; +} + +/** + * A declaration the matcher confirmed it will serve. + * + * Both halves are kept on purpose, because the two faces need different ones: + * `/meta/api` answers with the STORED item (its `_packageId` / `_provenance` / + * `_diagnostics` decorations are what the Studio list reads), while the OpenAPI + * document is built from `endpoint` — the matcher's own parsed value, with + * `ApiEndpointSchema` defaults materialized, i.e. literally the object the + * runtime will act on. + */ +export interface ServedEndpoint { + /** The stored item as the enumerating face received it. */ + item: T; + /** The parsed declaration the matcher resolved this route to. */ + endpoint: ApiEndpoint; +} + +/** Read a string property off an unknown stored item. */ +function readString(item: unknown, key: string): string | undefined { + if (!item || typeof item !== 'object') return undefined; + const value = (item as Record)[key]; + return typeof value === 'string' ? value : undefined; +} + +/** + * Whether a resolved service can answer the served question at all. + * + * `matchEndpoint` is OPTIONAL on `IMetadataService`, so an occupant of the + * `metadata` slot may not implement it — in which case nothing declared is + * served either (`runAppEndpointStep` returns `undefined` on exactly this + * probe), and the caller must decide what to do with an authority it does not + * have. Probed here so both faces spell the test once. + */ +export function isEndpointMatchAuthority(candidate: unknown): candidate is EndpointMatchAuthority { + return ( + candidate != null && + typeof (candidate as { matchEndpoint?: unknown }).matchEndpoint === 'function' + ); +} + +/** + * Narrow enumerated `api` items to the ones the matcher will actually serve. + * + * @param items stored `api` items, exactly as the enumerating face received them. + * @param authority the endpoint matcher, reached through `matchEndpoint`. + * @param logger where omissions are named. + * @returns one entry per served declaration, in input order. + * @throws whatever `matchEndpoint` threw — an outage is never an empty set. + */ +export async function selectServedEndpoints( + items: readonly T[], + authority: EndpointMatchAuthority, + logger: ServedEndpointLogger, +): Promise[]> { + const served: ServedEndpoint[] = []; + const omitted: Array<{ name: string; route: string; reason: string }> = []; + + for (const item of items) { + const name = readString(item, 'name'); + const path = readString(item, 'path'); + const method = readString(item, 'method'); + + if (!name || !path || !method) { + omitted.push({ + name: name ?? '', + route: `${method ?? ''} ${path ?? ''}`, + reason: 'declares no usable name/method/path, so no route can resolve to it', + }); + continue; + } + + // Deliberately unguarded — see the module header. A store that cannot be + // read must reach the caller as a failure, not as "nothing is declared". + const match = await authority.matchEndpoint({ path, method }); + + if (!match) { + omitted.push({ + name, + route: `${method} ${path}`, + reason: 'the endpoint matcher resolves this route to no declaration', + }); + continue; + } + + if (match.endpoint.name !== name) { + omitted.push({ + name, + route: `${method} ${path}`, + reason: `the endpoint matcher resolves this route to '${match.endpoint.name}' instead`, + }); + continue; + } + + served.push({ item, endpoint: match.endpoint }); + } + + if (omitted.length > 0) { + logger.error( + `[REST] ${omitted.length} declared api item(s) are OMITTED from this surface because the endpoint ` + + `matcher will not serve them — their routes answer 404, so announcing them would describe ` + + `endpoints that do not exist: ` + + omitted.map((o) => `'${o.name}' (${o.route}) — ${o.reason}`).join('; ') + + `. This surface now reports only what the runtime serves; to make one of these live, publish it ` + + `through a gated path (a stack artifact, or \`publishPackage\` with the package's ` + + `\`manifest.namespace\`) rather than a direct metadata write.`, + { omitted }, + ); + } + + return served; +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6c058d4715..f4bfbd4294 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1824,6 +1824,9 @@ importers: specifier: ^4.4.3 version: 4.4.3 devDependencies: + '@objectstack/metadata': + specifier: workspace:* + version: link:../metadata '@objectstack/metadata-protocol': specifier: workspace:* version: link:../metadata-protocol