diff --git a/.changeset/28930-bracketed-app-id-refused.md b/.changeset/28930-bracketed-app-id-refused.md new file mode 100644 index 00000000000..44ef8ad28f4 --- /dev/null +++ b/.changeset/28930-bracketed-app-id-refused.md @@ -0,0 +1,19 @@ +--- +'@objectstack/rest': patch +--- + +fix(rest)!: a bracketed `id` key on the app list (`?id[]=NAME`, `?id[0]=NAME`) is refused 400 naming the key instead of being ignored and answered with the whole list + +Clause-②: no (narrowing) — `GET /meta/app` / `GET /meta/apps` with any query key that opens `id[` (on `RestServer` and on the runtime dispatcher, which share the list chain) is refused with `400 VALIDATION_ERROR`; it answered 200 with the caller's whole visible list before. Nothing is widened: no export, answer member, error code or status is added. +Metadata-protocol: none + + + +**BREAKING** accept-set narrowing on the app list read (`@objectstack/rest`). + +- **Before.** `?id[]=setup` and `?id[0]=setup` (the qs-style array spellings) reached the list chain under the key as spelled, which the one-app filter never read, so a caller who asked for one app got every app it may see. +- **Now.** The list chain refuses a key that opens `id[` after the per-caller visibility gate: `400 VALIDATION_ERROR`, message `Query parameter 'id[]' names no app; the 'id' filter takes exactly one app name, spelled ?id=NAME. …` (the message names the key as sent, in the frame the empty `?id=` refusal uses), and no list in the body. A bracketed key beside a plain `?id=` is refused too. +- **Unchanged.** `?id=NAME` narrows to that app (or an empty list when it names none); no `id` lists every app the caller may see; an empty `?id=` and a repeated `id` keep their 400s; a bracketed `id` key on any type but `app` is still ignored, as `?id=` is there. +- **Who sees it.** An API client that serialises arrays qs-style. `@objectstack/client` and the console send no `id` on this read. + +**The remedy.** FROM `?id[]=NAME` (or `?id[0]=NAME`) TO `?id=NAME`; leave `id` out to list every app. diff --git a/packages/qa/dogfood/test/meta-read-doors-live.dogfood.test.ts b/packages/qa/dogfood/test/meta-read-doors-live.dogfood.test.ts index 13cc218c588..1fea8754958 100644 --- a/packages/qa/dogfood/test/meta-read-doors-live.dogfood.test.ts +++ b/packages/qa/dogfood/test/meta-read-doors-live.dogfood.test.ts @@ -164,4 +164,51 @@ describe('[#26572] the v18 /meta read doors, over HTTP on a booted showcase', () expect((await sortability(admin)).budget?.sortable).toBe(true); }); }); + + // A client that serialises arrays qs-style sends `?id[]=NAME` or + // `?id[0]=NAME`. The adapter hands the key through as spelled (`id[]`), so + // the app list's one-app filter never read it and the caller's whole list + // came back for a request that asked for one app. It is refused like the + // empty `?id=`: 400 VALIDATION_ERROR naming the key, no list. + describe('GET /meta/app with a bracketed `id` key is refused, never answered with the whole list', () => { + const appNames = (body: any): string[] => ((Array.isArray(body) ? body : body?.items) ?? []).map((a: any) => a?.name); + + for (const spelling of ['id[]', 'id[0]']) { + it(`?${spelling}=setup: 400 VALIDATION_ERROR naming the key, for the admin and for a member`, async () => { + for (const [who, token] of [['admin', admin], ['member', liaison]] as const) { + const res = await stack.apiAs(token, 'GET', `/meta/app?${spelling}=setup`); + const body: any = await res.json(); + expect({ who, status: res.status, code: codeOf(body) }, JSON.stringify(body).slice(0, 400)) + .toEqual({ who, status: 400, code: 'VALIDATION_ERROR' }); + expect(messageOf(body)).toMatch(new RegExp(`^Query parameter '${spelling.replace(/[[\]]/g, '\\$&')}' names no app`)); + expect(appNames(body)).toEqual([]); + } + }); + } + + it('CONTROLS — ?id=NAME is one app, ?id= and a repeated id are 400, no id is the pruned list', async () => { + const one = await stack.apiAs(admin, 'GET', '/meta/app?id=showcase_app'); + expect(one.status).toBe(200); + expect(appNames(await one.json())).toEqual(['showcase_app']); + // The filter narrows: `setup` names no app on this stack, so `?id=setup` + // is an empty list where `?id[]=setup` answered the whole one. + const none = await stack.apiAs(admin, 'GET', '/meta/app?id=setup'); + expect(none.status).toBe(200); + expect(appNames(await none.json())).toEqual([]); + for (const query of ['id=', 'id=showcase_app&id=showcase_app']) { + const res = await stack.apiAs(admin, 'GET', `/meta/app?${query}`); + expect({ query, status: res.status, code: codeOf(await res.json()) }) + .toEqual({ query, status: 400, code: 'VALIDATION_ERROR' }); + } + const adminList = await stack.apiAs(admin, 'GET', '/meta/app'); + const memberList = await stack.apiAs(liaison, 'GET', '/meta/app'); + expect(adminList.status).toBe(200); + expect(memberList.status).toBe(200); + const adminApps = appNames(await adminList.json()); + const memberApps = appNames(await memberList.json()); + expect(adminApps, JSON.stringify({ adminApps, memberApps })).toContain('showcase_app'); + expect(memberApps, JSON.stringify({ adminApps, memberApps })).toContain('showcase_app'); + expect(memberApps, 'a member never sees setup').not.toContain('setup'); + }); + }); }); diff --git a/packages/rest/src/meta-app-publish-gate.test.ts b/packages/rest/src/meta-app-publish-gate.test.ts index b0ee9e78c2e..94bf0d9dc7c 100644 --- a/packages/rest/src/meta-app-publish-gate.test.ts +++ b/packages/rest/src/meta-app-publish-gate.test.ts @@ -338,6 +338,42 @@ describe('#7566 — `GET /meta/app?id=` narrows the list instead of being droppe } }); + // A client that serialises arrays qs-style sends `?id[]=crm` or + // `?id[0]=crm`. The adapters hand the key through as spelled, so the + // filter, which reads `query.id`, never saw it and the caller who asked for + // one app got every app it may see. Refused with the empty `?id=`'s + // answer, naming the key as sent; the dispatcher's twin is pinned in + // `@objectstack/runtime`'s list parity census. + it.each(['id[]', 'id[0]', 'id[name]'])('a BRACKETED `%s` key is refused with 400 naming it — never the whole list', async (key) => { + for (const perms of [['studio.access'], ['manage_users'], []]) { + const { rest } = setup(perms); + const res = await getList(rest, 'app', { [key]: 'crm' }); + + expect(res.statusCode, `caller holding [${perms}]`).toBe(400); + expect(res.body?.code).toBe('VALIDATION_ERROR'); + expect(String(res.body?.error).startsWith(`Query parameter '${key}' names no app`)).toBe(true); + expect(res.body?.items).toBeUndefined(); + expect(Array.isArray(res.body)).toBe(false); + } + // The plural spelling refuses identically, and so does a bracketed key + // beside a plain one: two spellings of one filter. + for (const [type, query] of [['apps', { [key]: 'crm' }], ['app', { id: 'crm', [key]: 'account' }]] as const) { + const res = await getList(setup(['manage_users']).rest, type, query); + expect(res.statusCode, `${type} ${JSON.stringify(query)}`).toBe(400); + expect(res.body?.code).toBe('VALIDATION_ERROR'); + } + }); + + it('a bracketed `id` key on another list type is not read, as no other `id` spelling is (control)', async () => { + const { rest, protocol } = setup(['manage_users']); + protocol.getMetaItems = vi.fn(async ({ type: t }: any) => + String(t ?? '').startsWith('view') ? [{ name: 'all_leads', viewKind: 'list', object: 'lead' }] : []); + const res = await getList(rest, 'view', { 'id[]': 'nope' }); + + expect(res.statusCode).toBe(200); + expect(namesFrom(res.body)).toEqual(['all_leads']); + }); + it('a WHITESPACE id keeps its answer — a value no app is named, so an empty list (#28548 control)', async () => { const { rest } = setup(['manage_users']); const res = await getList(rest, 'app', { id: ' ' }); diff --git a/packages/rest/src/meta-item-read-gate.ts b/packages/rest/src/meta-item-read-gate.ts index d4588c4c418..9c8674a8195 100644 --- a/packages/rest/src/meta-item-read-gate.ts +++ b/packages/rest/src/meta-item-read-gate.ts @@ -3418,9 +3418,46 @@ export type MetaListAnswer = * the dispatcher's `errorFromThrown`), as {@link refuseUnknownMetaListType} does. */ function refuseEmptyListFilter(param: 'id' | 'object', names: 'app' | 'object', lists: 'app' | 'view'): never { + throwListFilterRefusal(param, param, names, lists, 'got ""', `an empty ${names} name`); +} + +/** + * The app list's answer to a BRACKETED spelling of its one-app filter — + * `?id[]=NAME`, `?id[0]=NAME`, any key that opens `id[`. A client that + * serialises arrays qs-style sends it; every adapter hands the key through as + * spelled (`readQuery` in `plugin-hono-server`, `readDispatchQuery` in + * `@objectstack/hono`), so step 3 of {@link createMetaListAnswer}, which reads + * `query.id`, never saw it, and the caller who asked for one app was answered + * every app it may see. Refused with {@link refuseEmptyListFilter}'s answer — + * one sentence frame, `400 VALIDATION_ERROR` — naming the key as sent: the + * filter takes exactly one name, spelled `?id=NAME`. A bracketed key beside a + * plain `?id=` is refused too: the request carries two spellings of one filter. + * + * Only the app list reads `id`, so only it judges the bracketed spelling; on + * every other type the key is not read, as no other `id` spelling is. + */ +function refuseBracketedAppIdFilter(query: Readonly> | undefined): void { + if (!query || typeof query !== 'object') return; + const key = Object.keys(query).find((k) => k.startsWith('id[')); + if (key === undefined) return; + throwListFilterRefusal( + 'id', key, 'app', 'app', + "the 'id' filter takes exactly one app name, spelled ?id=NAME", "a bracketed 'id' key", + ); +} + +/** The one sentence frame of the list chain's narrowing-filter refusals. */ +function throwListFilterRefusal( + param: 'id' | 'object', + key: string, + names: 'app' | 'object', + lists: 'app' | 'view', + got: string, + notAFilter: string, +): never { const err: any = new Error( - `Query parameter '${param}' names no ${names}; got "". The list was not served: an empty ${names} ` - + `name is not a filter. Name one ${names}, or leave the parameter out to list every ${lists} you can see.`, + `Query parameter '${key}' names no ${names}; ${got}. The list was not served: ${notAFilter} ` + + `is not a filter. Name one ${names}, or leave the parameter out to list every ${lists} you can see.`, ); err.status = 400; err.code = 'VALIDATION_ERROR'; @@ -3516,8 +3553,9 @@ async function describeMetaPositionList(sources: MetaListAnswerSources, list: an * items, before the locale collapse (`_packageId` scopes membership). * 3. `app` — `?id=` narrows to the app of that `name` (#7566); absent * means no filter, and [#28548] an EMPTY value is refused (`400 - * VALIDATION_ERROR`), never read as absent; no match is an EMPTY list, - * never a 404. + * VALIDATION_ERROR`), never read as absent, as is a bracketed key + * (`?id[]=`, `?id[0]=`), never read as no filter; no match is an EMPTY + * list, never a 404. * 4. `view` — `?object=` keeps the independent views bound to that * object, sorted by `order` then `name` (the switcher); [#28548] an EMPTY * value is refused (`400 VALIDATION_ERROR`), never read as absent. @@ -3575,7 +3613,9 @@ export function createMetaListAnswer( } // 3. [#7566] `?id=`. [#28548] An EMPTY one is refused, never - // read as absent (see {@link refuseEmptyListFilter}). + // read as absent (see {@link refuseEmptyListFilter}), and so is a + // bracketed spelling of the key (see {@link refuseBracketedAppIdFilter}). + if (metaType === 'app') refuseBracketedAppIdFilter(query); const appIdFilter = metaType === 'app' ? query?.id : undefined; if (appIdFilter === '') refuseEmptyListFilter('id', 'app', 'app'); if (typeof appIdFilter === 'string') { diff --git a/packages/runtime/src/domains/meta-list-projection-parity.test.ts b/packages/runtime/src/domains/meta-list-projection-parity.test.ts index 802d8d53e9c..269a7905eb6 100644 --- a/packages/runtime/src/domains/meta-list-projection-parity.test.ts +++ b/packages/runtime/src/domains/meta-list-projection-parity.test.ts @@ -525,6 +525,10 @@ const PARAM_PROBES: ReadonlyArray<{ label: string; param?: string; query: Record // [#28548] An EMPTY app name is refused (400 VALIDATION_ERROR) on both // transports, never read as absent. { label: '?id= (empty)', param: 'id', query: { id: '' } }, + // …and so is a BRACKETED `id` key (a qs-style array spelling), never + // read as no filter. + { label: '?id[]=crm', param: 'id', query: { 'id[]': 'crm' } }, + { label: '?id[0]=crm', param: 'id', query: { 'id[0]': 'crm' } }, { label: '?object=lead', param: 'object', query: { object: 'lead' } }, { label: '?object=nope', param: 'object', query: { object: 'nope' } }, // [#28548] …and so is an EMPTY view object. @@ -687,6 +691,8 @@ describe('[#20320] row A: the reference moves — each probe changes RestServer\ expect((await read('/meta/app', { id: 'nope' })).items).toEqual([]); const blank = await read('/meta/app', { id: '' }); expect({ status: blank.status, code: blank.code, items: blank.items }).toEqual({ status: 400, code: 'VALIDATION_ERROR', items: undefined }); + const bracketed = await read('/meta/app', { 'id[]': 'crm' }); + expect({ status: bracketed.status, code: bracketed.code, items: bracketed.items }).toEqual({ status: 400, code: 'VALIDATION_ERROR', items: undefined }); expect(names((await read('/meta/view', { object: 'lead' })).items)).toEqual(['lead_mine', 'lead_all']); expect((await read('/meta/views', { object: 'nope' })).items).toEqual([]); const blankObject = await read('/meta/views', { object: '' });