diff --git a/.changeset/meta-type-gate-plural.md b/.changeset/meta-type-gate-plural.md new file mode 100644 index 0000000000..664aad91e3 --- /dev/null +++ b/.changeset/meta-type-gate-plural.md @@ -0,0 +1,33 @@ +--- +"@objectstack/rest": patch +--- + +fix(rest): the /meta per-type gates are enforced on both spellings of the type segment (#3984) + +Every per-type filter on `GET /meta/:type` and `GET /meta/:type/:name` compared +`req.params.type` to a literal SINGULAR name, while the protocol's `getMetaItems` +normalizes singular↔plural and serves either. Prime Directive #3 makes plural the +canonical REST spelling, so the form a client is most likely to use — +`/api/v1/meta/books` — reached the handler with every gate skipped. + +Three of those gates are authorization: + +- **ADR-0046 §6.7 book / doc audience** (three sites: the list, the single-item + read, and the doc effective-audience union). `GET /meta/books` returned a + `{ permissionSet }`-gated book — an *Admin Guide* — to a caller who does not + hold the set, and `GET /meta/books/admin_guide` answered `200` where the + singular spelling answers `401`. On a publicly-served deployment the same skip + handed an `org` book to an anonymous reader. +- **App RBAC filter** — hides privileged apps (Studio, Setup) and gated nav + entries from callers without the grants. `GET /meta/apps` skipped it. +- **Dashboard `requiresService` gate** (ADR-0057 D10). `GET /meta/dashboards` + skipped it. + +The remaining spelling-sensitive branches are behavioural rather than +authorization — doc i18n locale collapse, and the list-response `content` strip — +and were inconsistent between the two spellings for the same reason. + +Each handler now normalizes the type ONCE (`RestServer.metaTypeSingular`, backed +by the same `PLURAL_TO_SINGULAR` table the protocol uses) and every gate keys on +that value, so the two spellings of one route can no longer diverge. Found while +scoping #3963. diff --git a/content/docs/api/metadata-api.mdx b/content/docs/api/metadata-api.mdx index a949b03c10..9d31db5f2f 100644 --- a/content/docs/api/metadata-api.mdx +++ b/content/docs/api/metadata-api.mdx @@ -23,7 +23,13 @@ List all items of a metadata type. | Parameter | Location | Description | |:----------|:---------|:------------| -| `type` | path | Metadata type name (e.g. `object`, `view`) | +| `type` | path | Metadata type name, singular or plural — `object` and `objects` address the same type | + +Both spellings are accepted and behave **identically**: the same audience gate +(ADR-0046 §6.7 books/docs), the same RBAC filtering of privileged apps, and the +same response shaping apply either way. Metadata type names are singular by +convention (Prime Directive #3) while REST paths are plural, so both forms exist +in the wild. **Response**: `{ type: "object", items: [{ name: "account", ... }, ...] }` diff --git a/packages/rest/src/meta-audience-plural.test.ts b/packages/rest/src/meta-audience-plural.test.ts new file mode 100644 index 0000000000..2f5d1227b5 --- /dev/null +++ b/packages/rest/src/meta-audience-plural.test.ts @@ -0,0 +1,128 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// Does the ADR-0046 §6.7 audience gate survive the PLURAL spelling of the type +// segment? The three gates in rest-server key on the exact singular +// (`req.params.type === 'book'` / `=== 'doc'`), while the protocol's +// `getMetaItems` normalizes singular↔plural and serves either. Prime Directive +// #3 makes plural the canonical REST spelling, so `/meta/books` is the form a +// client is most likely to use. + +import { describe, it, expect, vi } from 'vitest'; +import { RestServer } from './rest-server'; + +const PUBLIC_BOOK = { name: 'manual', label: 'Manual', audience: 'public', groups: [] }; +const GATED_BOOK = { name: 'admin_guide', label: 'Admin Guide', audience: { permissionSet: 'crm_admin' }, groups: [] }; + +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.setHeader = vi.fn(); res.write = vi.fn(); res.end = vi.fn(); + return res; +} + +function setup() { + const protocol: any = { + getDiscovery: vi.fn().mockResolvedValue({ version: 'v0', endpoints: { data: '', metadata: '', ui: '', auth: '/auth' } }), + getMetaTypes: vi.fn().mockResolvedValue([]), + // The real implementation normalizes singular↔plural, so BOTH spellings + // resolve to the same items. + getMetaItems: vi.fn(async ({ type }: any) => { + const t = String(type ?? ''); + if (t === 'book' || t === 'books') return [PUBLIC_BOOK, GATED_BOOK]; + return []; + }), + getMetaItem: vi.fn(async ({ name }: any) => { + if (name === PUBLIC_BOOK.name) return PUBLIC_BOOK; + if (name === GATED_BOOK.name) return GATED_BOOK; + return {}; + }), + findData: vi.fn().mockResolvedValue([]), + }; + // requireAuth:false so the anonymous caller reaches the handler at all — the + // meta routes are otherwise wrapped in the anonymous gate. The audience gate + // is what is under test, not the auth gate. + const rest = new RestServer(createMockServer() as any, protocol, { api: { requireAuth: false } } as any); + rest.registerRoutes(); + return { rest, protocol }; +} + +async function getType(rest: any, type: string) { + 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: {} }, res); + return res; +} + +async function getItem(rest: any, type: string, name: string) { + const route = rest.getRoutes().find((r: any) => r.method === 'GET' && r.path === '/api/v1/meta/:type/:name'); + if (!route) throw new Error('meta/:type/:name route not registered'); + const res = makeRes(); + await route.handler({ method: 'GET', params: { type, name }, query: {}, body: {} }, res); + return res; +} + +const names = (body: any) => { + const list = Array.isArray(body) ? body : (body?.items ?? []); + return list.map((b: any) => b?.name).sort(); +}; + +describe('ADR-0046 §6.7 audience gate vs the plural type segment', () => { + it('singular /meta/book hides the permissionSet-gated book from an anonymous caller', async () => { + const { rest } = setup(); + const res = await getType(rest, 'book'); + + expect(res.statusCode).toBe(200); + expect(names(res.body)).toEqual(['manual']); + }); + + it('plural /meta/books applies the SAME gate', async () => { + const { rest } = setup(); + const res = await getType(rest, 'books'); + + expect(res.statusCode).toBe(200); + // Before the fix this returned ['admin_guide', 'manual'] — the gated book + // leaked because the filter only fires on the singular spelling. + expect(names(res.body)).toEqual(['manual']); + }); + + it('the single-item read is gated on both spellings', async () => { + const { rest } = setup(); + // A `{ permissionSet }`-gated book is 401 for an anonymous reader — + // whichever way the type segment is spelled. + expect((await getItem(rest, 'book', 'admin_guide')).statusCode).toBe(401); + expect((await getItem(rest, 'books', 'admin_guide')).statusCode).toBe(401); + // …and the public one is readable either way. + expect((await getItem(rest, 'book', 'manual')).statusCode).toBe(200); + expect((await getItem(rest, 'books', 'manual')).statusCode).toBe(200); + }); +}); + +describe('the same spelling sensitivity on the other per-type gates', () => { + // The `/meta/:type` handler runs several per-type filters, and every one of + // them keyed on the literal singular. The app one is an RBAC filter (it hides + // privileged apps like Studio / Setup and gated nav entries), so the plural + // spelling skipped an authorization filter, not just a cosmetic one. + it('the app filter runs on /meta/apps too', async () => { + const { rest, protocol } = setup(); + protocol.getMetaItems = vi.fn(async ({ type }: any) => { + const t = String(type ?? ''); + return t === 'app' || t === 'apps' ? [{ name: 'crm' }] : []; + }); + // The filter needs a resolved context to do RBAC work; with none it must + // still take the same branch for both spellings (observable via the + // identical response rather than a leak, since anonymous resolves nothing). + const singular = await getType(rest, 'app'); + const plural = await getType(rest, 'apps'); + expect(plural.statusCode).toBe(singular.statusCode); + expect(names(plural.body)).toEqual(names(singular.body)); + }); +}); diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index c74a613553..7b36a48c70 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -10,6 +10,7 @@ import { RouteManager } from './route-manager.js'; import { RestServerConfig, RestApiConfig, CrudEndpointsConfig, MetadataEndpointsConfig, BatchEndpointsConfig, RouteGenerationConfig } from '@objectstack/spec/api'; import { DataProtocol, MetadataProtocol } from '@objectstack/spec/api'; import { PUBLIC_FORM_SERVER_MANAGED_FIELDS } from '@objectstack/spec/security'; +import { PLURAL_TO_SINGULAR } from '@objectstack/spec/shared'; import type { DroppedFieldsEvent } from '@objectstack/spec/data'; import type { ISecurityService } from '@objectstack/spec/contracts'; import { @@ -1412,6 +1413,25 @@ export class RestServer { } } + /** + * Canonical SINGULAR form of the `:type` path segment. + * + * The metadata routes accept either spelling — the protocol's `getMetaItems` + * normalizes singular↔plural and serves both — and Prime Directive #3 makes + * PLURAL the canonical REST spelling (`/api/v1/meta/books`). So every gate + * keyed on the type must compare against the normalized form. The three + * ADR-0046 §6.7 audience gates below each tested `req.params.type === 'book'` + * literally, which meant `GET /meta/books` served the list with the gate + * never running: a `{ permissionSet }`-gated book (an *Admin Guide*) came + * back to a caller who does not hold the set, and an `org` book came back to + * an anonymous reader on a publicly-served deployment. Same route, gate + * enforced on one spelling of it. + */ + private static metaTypeSingular(type: unknown): string { + const t = typeof type === 'string' ? type : ''; + return PLURAL_TO_SINGULAR[t] ?? t; + } + /** Whether any of these books carries a `{ permissionSet }` audience. */ private static anyPermissionSetAudience(books: readonly any[]): boolean { return books.some( @@ -2568,7 +2588,7 @@ export class RestServer { // objectql implementation actually returns the raw // array. Handle both shapes defensively. let visible: any = items; - if (req.params.type === 'app') { + if (RestServer.metaTypeSingular(req.params.type) === 'app') { const raw = items as unknown; const list: any[] | null = Array.isArray(raw) ? (raw as any[]) @@ -2595,7 +2615,7 @@ export class RestServer { // ADR-0057 D10: gate dashboard widgets by `requiresService` // the same way app nav entries are gated above. - if (req.params.type === 'dashboard') { + if (RestServer.metaTypeSingular(req.params.type) === 'dashboard') { const raw = visible as unknown; const list: any[] | null = Array.isArray(raw) ? (raw as any[]) @@ -2623,7 +2643,7 @@ export class RestServer { // excluded. Runtime `shared` / `personal` views // (sys_view_definition) are merged client-side via the // generic data API. - if (req.params.type === 'view' && req.query?.object) { + if (RestServer.metaTypeSingular(req.params.type) === 'view' && req.query?.object) { const obj = String(req.query.object); const raw = visible as unknown; const list: any[] | null = Array.isArray(raw) @@ -2645,7 +2665,7 @@ export class RestServer { // callers see only `public` books; `{ permissionSet }`-gated // books require the caller to hold the named set (resolved // through the security service; unresolvable → fail closed). - if (req.params.type === 'book') { + if (RestServer.metaTypeSingular(req.params.type) === 'book') { const raw = visible as unknown; const list = RestServer.metaItemsArray(raw); if (list.length > 0) { @@ -2664,7 +2684,7 @@ export class RestServer { // claim it; unclaimed docs default to `org`). Runs on the // raw items (before locale collapse) so `_packageId` // provenance is still present for membership scoping. - if (req.params.type === 'doc') { + if (RestServer.metaTypeSingular(req.params.type) === 'doc') { const raw = visible as unknown; const list = RestServer.metaItemsArray(raw); if (list.length > 0) { @@ -2705,7 +2725,7 @@ export class RestServer { // ADR-0046 i18n: collapse each doc to the request // locale (localized label/description, `translations` // map dropped) before the content-strip step below. - if (req.params.type === 'doc') { + if (RestServer.metaTypeSingular(req.params.type) === 'doc') { const locale = this.extractLocale(req); const { resolveDocLocale } = await import('@objectstack/spec/system'); const raw = visible as unknown; @@ -2727,7 +2747,7 @@ export class RestServer { // name + label. `?include=content` opts back in; the // single-item GET /meta/doc/:name always returns the // full body. - if (req.params.type === 'doc' && req.query?.include !== 'content') { + if (RestServer.metaTypeSingular(req.params.type) === 'doc' && req.query?.include !== 'content') { const raw = visible as unknown; const list: any[] | null = Array.isArray(raw) ? (raw as any[]) @@ -2926,7 +2946,7 @@ export class RestServer { // viewers of the same app schema. Drafts also // bypass cache: the cache is keyed on the // published checksum and drafts are out-of-band. - const isAppType = req.params.type === 'app'; + const isAppType = RestServer.metaTypeSingular(req.params.type) === 'app'; const isDraftRead = typeof req.query?.state === 'string' && req.query.state.toLowerCase() === 'draft'; // ADR-0033/0037 — `?preview=draft` overlays a pending @@ -3043,7 +3063,7 @@ export class RestServer { // ADR-0057 D10: gate dashboard widgets by `requiresService` // (mirrors the app-nav gate above) so the console never // renders a tile bound to an absent optional service. - if (req.params.type === 'dashboard' && visible) { + if (RestServer.metaTypeSingular(req.params.type) === 'dashboard' && visible) { const ctx = await this.resolveExecCtx(environmentId, req).catch(() => undefined); const registered = await this.resolveRegisteredServices((ctx as any)?.__kernel, [visible]); const serviceGate = registered ? (n: string) => registered.has(n) : undefined; @@ -3056,13 +3076,14 @@ export class RestServer { // it, unclaimed → org). 401 for anonymous, 403 for an // authenticated non-holder; fail closed when holdings // cannot be resolved (ADR-0049). - if ((req.params.type === 'book' || req.params.type === 'doc') && visible) { + const audienceGatedType = RestServer.metaTypeSingular(req.params.type); + if ((audienceGatedType === 'book' || audienceGatedType === 'doc') && visible) { const { audienceAllows, docAudienceAllows, resolveDocAudiences } = await import('@objectstack/spec/system'); const target = isMetaEnvelope(visible) ? (visible as any).item : visible; let caller: { authenticated: boolean; permissionSets?: string[] }; let allowed: boolean; - if (req.params.type === 'book') { + if (audienceGatedType === 'book') { caller = await this.resolveAudienceCaller(environmentId, req, { needPermissionSets: RestServer.anyPermissionSetAudience([target]), }); @@ -3104,7 +3125,7 @@ export class RestServer { // ADR-0046 i18n: collapse the doc to the request // locale (label/description/content) and drop the // `translations` map so consumers get one body. - if (req.params.type === 'doc' && visible) { + if (audienceGatedType === 'doc' && visible) { const locale = this.extractLocale(req); const { resolveDocLocale } = await import('@objectstack/spec/system'); visible = isMetaEnvelope(visible)