Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions .changeset/28930-bracketed-app-id-refused.md
Original file line number Diff line number Diff line change
@@ -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

<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped, and no export or type changes: a list request that spells its one-app filter as an array key is refused instead of served unfiltered. There is nothing for a migration to rewrite and no tombstone. -->

**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.
47 changes: 47 additions & 0 deletions packages/qa/dogfood/test/meta-read-doors-live.dogfood.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
});
36 changes: 36 additions & 0 deletions packages/rest/src/meta-app-publish-gate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: ' ' });
Expand Down
50 changes: 45 additions & 5 deletions packages/rest/src/meta-item-read-gate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<Record<string, unknown>> | 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';
Expand Down Expand Up @@ -3516,8 +3553,9 @@ async function describeMetaPositionList(sources: MetaListAnswerSources, list: an
* items, before the locale collapse (`_packageId` scopes membership).
* 3. `app` — `?id=<app>` 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=<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.
Expand Down Expand Up @@ -3575,7 +3613,9 @@ export function createMetaListAnswer(
}

// 3. [#7566] `?id=<app>`. [#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') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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: '' });
Expand Down
Loading