diff --git a/.changeset/29319-bracketed-drafts-filter-refused.md b/.changeset/29319-bracketed-drafts-filter-refused.md new file mode 100644 index 00000000000..251fcb08af9 --- /dev/null +++ b/.changeset/29319-bracketed-drafts-filter-refused.md @@ -0,0 +1,19 @@ +--- +'@objectstack/rest': patch +--- + +fix(rest)!: a bracketed `packageId` or `type` key on `GET /meta/_drafts` (`?packageId[]=ID`, `?packageId[0]=ID`, `?type[]=TYPE`, `?type[0]=TYPE`) is refused 400 naming the key instead of being ignored and answered with every package's or every type's drafts + +Clause-②: no (narrowing) — `GET /meta/_drafts` with any query key that opens `packageId[` or `type[` (on `RestServer` and on the runtime dispatcher, which share the arity decision) is refused with `400 VALIDATION_ERROR`; it answered 200 with every draft the caller may read before. No package export, answer member, error code, status or `rule` tag is added: the refusal reuses the `bracketed-parameter` rule the list and the book tree already answer. +Metadata-protocol: none + + + +**BREAKING** accept-set narrowing on the `/meta/_drafts` read (`@objectstack/rest`). + +- **Before.** `?packageId[]=ID`, `?packageId[0]=ID`, `?type[]=TYPE` and `?type[0]=TYPE` (the qs-style array spellings) reached the door under the key as spelled, which neither filter read, so an author who asked for one package's or one type's drafts got every draft, as if it were the filtered list. Beside a plain `?packageId=` the bracketed key was silently dropped. +- **Now.** The door refuses a key that opens `packageId[` or `type[` after its capability gate and its multiplicity gate, before any read: `400 VALIDATION_ERROR`, message `Query parameter 'packageId[]' names no package; the 'packageId' filter takes exactly one package id, spelled ?packageId=ID. …` or `Query parameter 'type[]' names no metadata type; the 'type' filter takes exactly one metadata type, spelled ?type=TYPE. …`, and no list. A bracketed key beside the plain one is refused too. +- **Unchanged.** `?packageId=ID` and `?type=TYPE` narrow as before, and leaving them out lists every draft; a repeated `packageId` or `type` keeps its 400; a caller with no authoring capability is still refused 403 first. +- **Who sees it.** An API client that serialises arrays qs-style. `@objectstack/client`'s `listDrafts` builds both keys with `URLSearchParams` (no brackets), and the console sends `?packageId=` plain. + +**The remedy.** FROM `?packageId[]=ID` (or `?packageId[0]=ID`) TO `?packageId=ID`; FROM `?type[]=TYPE` (or `?type[0]=TYPE`) TO `?type=TYPE`; leave the key out to list every draft. 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 f5605a0a3e3..4e1fb1f45ac 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 @@ -264,4 +264,41 @@ describe('[#26572] the v18 /meta read doors, over HTTP on a booted showcase', () expect(objects.length).toBeLessThan(everyPackage.length); }); }); + + // The same class on `_drafts`' own filters: `?packageId[]=` / `?type[]=` + // reached the door under the key as spelled, so the list was not narrowed + // and an author who asked for one package's (or one type's) drafts got + // every draft. Each is refused: 400 VALIDATION_ERROR naming the key, no list. + describe('GET /meta/_drafts with a bracketed `packageId` or `type` key is refused', () => { + const draftsOf = (body: any): any[] => body?.drafts ?? body?.data?.drafts ?? []; + + for (const [key, value, names] of [ + ['packageId[]', PKG, 'package'], ['packageId[0]', PKG, 'package'], + ['type[]', 'view', 'metadata type'], ['type[0]', 'view', 'metadata type'], + ] as const) { + it(`/meta/_drafts?${key}=…: 400 VALIDATION_ERROR naming the key, for the admin`, async () => { + const res = await stack.apiAs(admin, 'GET', `/meta/_drafts?${key}=${encodeURIComponent(value)}`); + const body: any = await res.json(); + expect({ status: res.status, code: codeOf(body) }, JSON.stringify(body).slice(0, 400)) + .toEqual({ status: 400, code: 'VALIDATION_ERROR' }); + expect(messageOf(body)).toMatch(new RegExp(`^Query parameter '${key.replace(/[[\]]/g, '\\$&')}' names no ${names};`)); + expect(draftsOf(body)).toEqual([]); + }); + } + + it('CONTROLS — the plain spellings narrow, and a member is still refused at the capability gate first', async () => { + const scoped = await stack.apiAs(admin, 'GET', `/meta/_drafts?packageId=${encodeURIComponent(PKG)}`); + expect(scoped.status).toBe(200); + const inPackage = draftsOf(await scoped.json()); + expect(inPackage.every((d: any) => d?.packageId === PKG), JSON.stringify(inPackage).slice(0, 400)).toBe(true); + + const typed = await stack.apiAs(admin, 'GET', '/meta/_drafts?type=view'); + expect(typed.status).toBe(200); + const views = draftsOf(await typed.json()); + expect(views.every((d: any) => d?.type === 'view'), JSON.stringify(views).slice(0, 400)).toBe(true); + + const member = await stack.apiAs(liaison, 'GET', `/meta/_drafts?packageId[]=${encodeURIComponent(PKG)}`); + expect(member.status).toBe(403); + }); + }); }); diff --git a/packages/rest/src/meta-collection-routes.ts b/packages/rest/src/meta-collection-routes.ts index 7f5ad47b557..824a9f54dc1 100644 --- a/packages/rest/src/meta-collection-routes.ts +++ b/packages/rest/src/meta-collection-routes.ts @@ -465,6 +465,16 @@ export function registerMetaCollectionRoutes(this: MetaCollectionRouteHost, scop // [#23214] The list lives beside the shared decision, so the // runtime dispatcher's twin refuses the same names. if (refuseRepeatedQueryParams(req, res, metaReadGate.META_QUERY_ARITY_DOORS.drafts)) return; + // A bracketed `packageId` or `type` key (`?type[]=`) is read + // by neither filter below, so it narrowed nothing: refused + // here, by the decision the dispatcher's twin asks too. + const bracketedFilter = metaReadGate.metaBracketedFilterRefusal('drafts', req.query); + if (bracketedFilter) { + res.status(bracketedFilter.status).json({ + error: { code: bracketedFilter.code, message: bracketedFilter.message }, + }); + return; + } // [#29180] A `type` that names no metadata type narrowed the // list to nothing and answered `200 {"drafts":[]}`, a // confident "nothing pending". Refused by the list door's own @@ -651,7 +661,7 @@ export function registerMetaCollectionRoutes(this: MetaCollectionRouteHost, scop // A bracketed `package` key (`?package[]=`) is read by no // binding below, so it scoped nothing: refused here, by the // decision the dispatcher's twin asks too. - const bracketedPackage = metaReadGate.metaBracketedPackageRefusal('list', req.query); + const bracketedPackage = metaReadGate.metaBracketedFilterRefusal('list', req.query); if (bracketedPackage) { res.status(bracketedPackage.status).json({ error: { code: bracketedPackage.code, message: bracketedPackage.message }, diff --git a/packages/rest/src/meta-item-read-gate.ts b/packages/rest/src/meta-item-read-gate.ts index 894bc70c186..977baa3caab 100644 --- a/packages/rest/src/meta-item-read-gate.ts +++ b/packages/rest/src/meta-item-read-gate.ts @@ -2417,7 +2417,7 @@ const LAYERED_VIEW_MEMBER_MESSAGE = * everywhere; `rule` names which of the three refused, because `RestServer`'s * item read answers the third at a different point (see * {@link metaItemQueryRefusal}). The arity doors answer a fourth, - * `bracketed-parameter` ({@link metaBracketedPackageRefusal}). + * `bracketed-parameter` ({@link metaBracketedFilterRefusal}). */ export interface MetaItemQueryRefusal { readonly status: 400; @@ -2558,7 +2558,7 @@ export const META_QUERY_ARITY_DOORS: Readonly, META_QUERY_ARITY_DOORS[door]) - ?? metaBracketedPackageRefusal(door, query); + ?? metaBracketedFilterRefusal(door, query); } /** - * The answer of the two arity doors that read `?package=` — the list and the - * book tree — to a BRACKETED spelling of it: `?package[]=ID`, `?package[0]=ID`, - * any key that opens `package[`. A client that serialises arrays qs-style sends - * it; every adapter hands the key through as spelled, so the door's - * `metaItemPackageBinding(query.package)` read no package, and a caller who - * named ONE package was answered across every package. `400 - * VALIDATION_ERROR`, in the list chain's narrowing-filter frame - * ({@link listFilterRefusalMessage}), naming the key as sent; beside a plain - * `?package=` too (two spellings of one scope). Asked at the door's entry, - * before any read — where the binding is read — by both transports: + * One filter an arity door judges for a BRACKETED spelling: the name as the + * door reads it, what one value of it names, how one value is spelled, what the + * door serves, and what leaving the parameter out reads. Every field goes into + * {@link listFilterRefusalMessage}'s one sentence frame. + */ +interface MetaBracketedFilter { + readonly param: string; + readonly names: string; + readonly takes: string; + readonly served: 'list' | 'tree'; + readonly unscoped: string; +} + +const PACKAGE_ID_TAKES = 'one package id, spelled'; + +/** + * The single-valued filters each arity door judges for a bracketed spelling, in + * the order asked: the names these doors read at their entry whose absence + * WIDENS the answer (no package is every package, no type is every type). The + * list's `id` and `object` are judged a step later, in the list chain + * ({@link refuseBracketedListFilter}). + */ +const META_BRACKETED_FILTERS: Readonly>> = Object.freeze({ + list: Object.freeze([ + { param: 'package', names: 'package', takes: `${PACKAGE_ID_TAKES} ?package=ID`, served: 'list', unscoped: 'list across every package' }, + ] as const), + 'book-tree': Object.freeze([ + { param: 'package', names: 'package', takes: `${PACKAGE_ID_TAKES} ?package=ID`, served: 'tree', unscoped: 'read the tree across every package' }, + ] as const), + drafts: Object.freeze([ + { param: 'packageId', names: 'package', takes: `${PACKAGE_ID_TAKES} ?packageId=ID`, served: 'list', unscoped: 'list the drafts of every package' }, + { param: 'type', names: 'metadata type', takes: 'one metadata type, spelled ?type=TYPE', served: 'list', unscoped: 'list the drafts of every type' }, + ] as const), +}); + +/** + * The answer of an arity door to a BRACKETED spelling of one of its narrowing + * filters ({@link META_BRACKETED_FILTERS}): `?package[]=ID` or `?package[0]=ID` + * on the list and the book tree, `?packageId[]=ID` or `?type[]=TYPE` on + * `_drafts` — any key that opens `param[`. 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 the door, which reads `query[param]`, read no + * filter, and a caller who named ONE package (or one type) was answered across + * every package (or type). `400 VALIDATION_ERROR`, rule `bracketed-parameter`, + * in the list chain's narrowing-filter frame ({@link listFilterRefusalMessage}), + * naming the key as sent; beside a plain spelling too (two spellings of one + * filter). Asked at the door's entry, before any read, by both transports: * {@link metaQueryArityRefusal} for the dispatcher, after * `refuseRepeatedQueryParams` for `RestServer`. * * Every by-name `/meta` door (the item read, `/layers`, the save, the reset, - * `/publish`) and the diagnostics sweep already refuse the key: it is not in - * their closed set (route rule 5). `_drafts` and `state` read no `package`. + * `/publish`) and the diagnostics sweep already refuse such a key: it is not in + * their closed set (route rule 5). */ -export function metaBracketedPackageRefusal(door: MetaQueryArityDoor, query: unknown): MetaItemQueryRefusal | undefined { - if (!META_QUERY_ARITY_DOORS[door].includes('package')) return undefined; - const key = bracketedKeyOf(query, 'package'); - if (key === undefined) return undefined; - const tree = door === 'book-tree'; - return { - status: 400, - code: 'VALIDATION_ERROR', - message: listFilterRefusalMessage( - key, 'package', "the 'package' filter takes exactly one package id, spelled ?package=ID", - "a bracketed 'package' key", tree ? 'tree' : 'list', - tree ? 'read the tree across every package' : 'list across every package', - ), - rule: 'bracketed-parameter', - }; +export function metaBracketedFilterRefusal(door: MetaQueryArityDoor, query: unknown): MetaItemQueryRefusal | undefined { + for (const filter of META_BRACKETED_FILTERS[door] ?? []) { + const key = bracketedKeyOf(query, filter.param); + if (key === undefined) continue; + return { + status: 400, + code: 'VALIDATION_ERROR', + message: listFilterRefusalMessage( + key, filter.names, `the '${filter.param}' filter takes exactly ${filter.takes}`, + `a bracketed '${filter.param}' key`, filter.served, filter.unscoped, + ), + rule: 'bracketed-parameter', + }; + } + return undefined; } /** diff --git a/packages/rest/src/meta-item-read-routes.ts b/packages/rest/src/meta-item-read-routes.ts index 55440b0b242..0b388895d55 100644 --- a/packages/rest/src/meta-item-read-routes.ts +++ b/packages/rest/src/meta-item-read-routes.ts @@ -386,7 +386,7 @@ export function registerMetaItemReadRoutes(this: MetaItemReadRouteHost, scope: M // A bracketed `package` key (`?package[]=`) is read by no // binding below, so it scoped nothing: refused here, by the // decision the dispatcher's twin asks too. - const bracketedPackage = metaReadGate.metaBracketedPackageRefusal('book-tree', req.query); + const bracketedPackage = metaReadGate.metaBracketedFilterRefusal('book-tree', req.query); if (bracketedPackage) { res.status(bracketedPackage.status).json({ error: { code: bracketedPackage.code, message: bracketedPackage.message }, diff --git a/packages/rest/src/meta-query-arity-doors.test.ts b/packages/rest/src/meta-query-arity-doors.test.ts index d61f21c1151..2e0387578e0 100644 --- a/packages/rest/src/meta-query-arity-doors.test.ts +++ b/packages/rest/src/meta-query-arity-doors.test.ts @@ -198,3 +198,64 @@ describe('a BRACKETED `package` key on the two arity doors that read `package`', } }); }); + +/** + * The same class on `_drafts`' own two filters. `?packageId[]=ID`, + * `?packageId[0]=ID`, `?type[]=TYPE` and `?type[0]=TYPE` reached the door under + * the key as spelled, so `listDrafts` was handed no package and no type, and a + * caller who named one package's (or one type's) drafts got every draft it may + * read. Refused at the door's arity step, before `listDrafts`, in the same + * sentence frame as the list's bracketed `package`. + */ +const bracketedDraftsMessage = (key: string, param: 'packageId' | 'type'): string => (param === 'packageId' + ? `Query parameter '${key}' names no package; the 'packageId' filter takes exactly one package id, ` + + `spelled ?packageId=ID. The list was not served: a bracketed 'packageId' key is not a filter. ` + + `Name one package, or leave the parameter out to list the drafts of every package.` + : `Query parameter '${key}' names no metadata type; the 'type' filter takes exactly one metadata type, ` + + `spelled ?type=TYPE. The list was not served: a bracketed 'type' key is not a filter. ` + + `Name one metadata type, or leave the parameter out to list the drafts of every type.`); +const DRAFTS_KEYS = [ + ['packageId[]', 'packageId'], ['packageId[0]', 'packageId'], ['packageId[id]', 'packageId'], + ['type[]', 'type'], ['type[0]', 'type'], ['type[name]', 'type'], +] as const; + +describe('a BRACKETED `packageId` or `type` key on `_drafts`', () => { + it.each(DRAFTS_KEYS)('`%s` is refused as data, naming the key', (key, param) => { + expect(metaQueryArityRefusal('drafts', { [key]: 'x' })).toEqual({ + status: 400, + code: 'VALIDATION_ERROR', + message: bracketedDraftsMessage(key, param), + rule: 'bracketed-parameter', + }); + // Beside the plain key it is two spellings of one filter. + expect(metaQueryArityRefusal('drafts', { [param]: 'x', [key]: 'y' })?.rule).toBe('bracketed-parameter'); + }); + + it('a repeated plain `packageId` still answers the multiplicity sentence first', () => { + expect(metaQueryArityRefusal('drafts', { packageId: ['a', 'b'], 'type[]': 'view' })?.message) + .toBe(repeatedQueryParamMessage('packageId', 2)); + }); + + it('is judged only on `_drafts`, and not for a key that merely starts alike', () => { + expect(metaQueryArityRefusal('list', { 'packageId[]': 'x', 'type[]': 'view' })).toBeUndefined(); + expect(metaQueryArityRefusal('book-tree', { 'type[]': 'view' })).toBeUndefined(); + expect(metaQueryArityRefusal('drafts', { 'types[]': 'view', 'packageIds[]': 'x', 'package[]': 'x' })).toBeUndefined(); + }); + + it.each(DRAFTS_KEYS.filter(([key]) => key.endsWith('[]') || key.endsWith('[0]')))( + 'RestServer: `%s` answers the nested 400 before `listDrafts`', + async (key, param) => { + const { protocol, status, body } = await drive('drafts', { [key]: 'x' }); + expect(status).toBe(400); + expect(body).toEqual({ error: { code: 'VALIDATION_ERROR', message: bracketedDraftsMessage(key, param) } }); + expect(protocol.listDrafts).not.toHaveBeenCalled(); + }, + ); + + it('RestServer: the plain spellings still narrow, and absent ones read every draft (controls)', async () => { + expect((await drive('drafts', { packageId: 'com.acme', type: 'view' })).protocol.listDrafts) + .toHaveBeenCalledWith({ packageId: 'com.acme', type: 'view' }); + expect((await drive('drafts', {})).protocol.listDrafts) + .toHaveBeenCalledWith({ packageId: undefined, type: undefined }); + }); +}); diff --git a/packages/runtime/src/domains/dispatcher-query-arity.test.ts b/packages/runtime/src/domains/dispatcher-query-arity.test.ts index 3c9729bed2a..3c9fb74a396 100644 --- a/packages/runtime/src/domains/dispatcher-query-arity.test.ts +++ b/packages/runtime/src/domains/dispatcher-query-arity.test.ts @@ -243,6 +243,41 @@ describe('[#23214] /meta — the four arity doors, over the list both transports expect(answer.body?.error?.message).toMatch(/^Query parameter 'object\[\]' names no object/); }); }); + + // The same class on `_drafts`' own filters: `?packageId[]=` / `?type[]=` + // reached `listDrafts` as no package and no type, so the answer was every + // draft. Refused at the door's arity step, with `RestServer`'s code and + // sentence, before `listDrafts`. + describe('a BRACKETED `packageId` or `type` key on `_drafts`', () => { + const sentence = (key: string, param: string): string => (param === 'packageId' + ? `Query parameter '${key}' names no package; the 'packageId' filter takes exactly one package id, ` + + `spelled ?packageId=ID. The list was not served: a bracketed 'packageId' key is not a filter. ` + + `Name one package, or leave the parameter out to list the drafts of every package.` + : `Query parameter '${key}' names no metadata type; the 'type' filter takes exactly one metadata type, ` + + `spelled ?type=TYPE. The list was not served: a bracketed 'type' key is not a filter. ` + + `Name one metadata type, or leave the parameter out to list the drafts of every type.`); + + it.each([ + ['packageId[]', 'packageId'], ['packageId[0]', 'packageId'], + ['type[]', 'type'], ['type[0]', 'type'], + ])('GET /meta/_drafts?%s=… is refused before `listDrafts`', async (key, param) => { + const { dispatcher, protocol } = boot(); + const answer = await get(dispatcher, '/meta/_drafts', { [key]: 'x' }); + expect(answer.status, JSON.stringify(answer)).toBe(400); + expect(answer.body?.error?.code).toBe('VALIDATION_ERROR'); + expect(answer.body?.error?.message).toBe(sentence(key, param)); + expect(protocol.listDrafts).not.toHaveBeenCalled(); + }); + + it('beside the plain key it is refused too, and the plain spellings still narrow (control)', async () => { + const { dispatcher, protocol } = boot(); + expect((await get(dispatcher, '/meta/_drafts', { packageId: 'com.acme', 'packageId[]': 'com.other' })).status) + .toBe(400); + expect(protocol.listDrafts).not.toHaveBeenCalled(); + expect((await get(dispatcher, '/meta/_drafts', { packageId: 'com.acme', type: 'view' })).status).toBe(200); + expect(protocol.listDrafts).toHaveBeenCalledWith({ packageId: 'com.acme', type: 'view' }); + }); + }); }); describe('[#23214] GET /ui/view/:object — `?type=` is one view type, read only when the path names none', () => {