From 10e0c3be8441c2d0eb433fedf8a07855b4acd218 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 10:01:52 +0000 Subject: [PATCH] fix(rest): honour `?id=` on `GET /api/v1/meta/app` instead of dropping it (#7566) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `GET /api/v1/meta/app?id=…` accepted the parameter and then ignored it — the same apps came back for every value, including one that names no app. Nothing on `GET /meta/:type` had ever read `id`: the list route narrows by permission (`filterAppForUser`, rest-server.ts:4706) and by `?package=` / `?object=` / `?include=`, and `id` was never among them. A caller could not tell a working filter from a dropped one. - The filter is honoured, matching on `name` — the App document's identity (`AppSchema.name`), the key `GET /meta/app/:name` addresses. Both spellings of the type segment are covered via `metaTypeSingular`. - A filter matching nothing answers 200 with an EMPTY list, not a 404 — measured off this route's siblings (`?package=` and `/meta/view?object=` both serve an empty list; the only meta 404 is the single-item address). - A repeated `?id=a&id=b` is refused 400 `VALIDATION_ERROR` through the same `refuseRepeatedQueryParams` gate this route already opens with (#6877). - Runs after the ADR-0045 §3 publish gate, so `?id=` answers the same empty list to a non-builder as a nonexistent id. - Absent and empty spellings still mean "no filter"; other metadata types are untouched. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BjEdc4MCajjEPDk4G33tga --- .changeset/meta-app-id-filter.md | 54 +++++++ .../rest/src/meta-app-publish-gate.test.ts | 149 +++++++++++++++++- packages/rest/src/rest-server.ts | 91 ++++++++++- 3 files changed, 290 insertions(+), 4 deletions(-) create mode 100644 .changeset/meta-app-id-filter.md diff --git a/.changeset/meta-app-id-filter.md b/.changeset/meta-app-id-filter.md new file mode 100644 index 0000000000..8fcd35877a --- /dev/null +++ b/.changeset/meta-app-id-filter.md @@ -0,0 +1,54 @@ +--- +"@objectstack/rest": patch +--- + +fix(rest): `GET /api/v1/meta/app?id=` narrows the app list instead of being dropped (#7566) + +`GET /api/v1/meta/app?id=…` accepted the parameter and then ignored it. The +same apps came back for **every** value, including one that names no app at all +— `?id=crm` and `?id=no_such_app` produced byte-identical responses. Nothing on +`GET /meta/:type` had ever read `id`: the list route narrows by permission +(`filterAppForUser`) and by `?package=` / `?object=` / `?include=`, and `id` was +never among them. + +Worse than an error, because the answer looks like the one that was asked for: a +caller cannot tell a working filter from a dropped one. A client that asks for +one app and renders `items[0]` gets a plausible, wrong answer, and a bogus id can +never come back empty. + +The filter is now honoured, matching on `name` — the App document's identity +(`AppSchema.name`, "App unique machine name"), the key `GET /meta/app/:name` +addresses and the key the metadata store merges overlays on. `AppSchema` declares +no `id` of its own, so there is no second identity for the two to disagree about. +Both spellings of the type segment are covered (`/meta/app` and `/meta/apps`), +since every other per-type filter on this handler keys off `metaTypeSingular`. + +**A filter that matches nothing answers `200` with an empty list, not a `404`.** +Measured off this route's siblings rather than chosen: `?package=` and `/meta/view?object=` both serve an empty list here, +and the only 404 on the meta surface is the single-item address `GET +/meta/:type/:name`. An empty list is already observably different from the +defect, which answered with all of them. + +**A repeated `?id=a&id=b` is refused with `400`**, through the same +`refuseRepeatedQueryParams` gate this route already opens with for `?package=` / +`?preview=` / `?object=` / `?include=` (#6877) — one route, one dialect for "this +request is malformed". Picking one of two conflicting intents is a wrong answer +delivered as a success, and the alternative the other filters on this line were +bitten by (`String(['crm','account'])` → the single app name `'crm,account'`) +would just have emptied the list silently. + +**The filter narrows within what the caller may observe, never around it.** It +runs after the ADR-0045 §3 publish gate, so `?id=` answers the +same empty list to a non-builder as `?id=` — the two are +indistinguishable by design. It is also not part of the permission branch's +`ctx?.userId` guard, so an anonymous read of a public deployment gets the filter +too. + +**Nothing that worked before changes.** An absent `?id=` still returns the whole +list, and so does the empty spelling `?id=` — the same falsy gate `?package=` on +this route has always used, and what an unset `` submits + // — the same falsy gate `?package=` on this route has always used. It + // must not become a new 400 or an empty list. + expect(namesFrom((await getList(rest, 'app', { id: '' })).body).sort()) + .toEqual(['account', 'crm']); + }); + + it('a MALFORMED id — supplied twice — is refused with 400, not silently resolved', async () => { + const { rest } = setup(['manage_users']); + const res = await getList(rest, 'app', { id: ['crm', 'account'] }); + + // Two conflicting intents in one well-formed request. Picking one is a + // wrong answer delivered as a success, and `String(['crm','account'])` + // would have made it the single app name `'crm,account'` — a name no app + // has, so the filter would silently empty. ADR-0112 nested envelope with + // the standard catalog's 400 member, the same answer this route already + // gives for a repeated `?package=` / `?object=` / `?include=` (#6877). + expect(res.statusCode).toBe(400); + expect(res.body?.error?.code).toBe('VALIDATION_ERROR'); + expect(res.body?.error?.message).toContain('"id"'); + // Refused means refused: no app list rode along with the error. + expect(res.body?.items).toBeUndefined(); + expect(Array.isArray(res.body)).toBe(false); + }); + + it('a one-element array is ONE occurrence and still filters', async () => { + // `?id=crm` reaches some adapters as `['crm']`; that is one occurrence + // encoded differently, not a repetition, so it must narrow rather than + // 400 — and it must not survive as an array into the comparison, where + // `['crm'] === 'crm'` is false and the filter would empty. + const { rest } = setup(['manage_users']); + const res = await getList(rest, 'app', { id: ['crm'] }); + + expect(res.statusCode).toBe(200); + expect(namesFrom(res.body)).toEqual(['crm']); + }); + + it('the PLURAL spelling filters identically — `/meta/apps?id=`', async () => { + // Prime Directive #3 makes plural the canonical REST spelling, and every + // other per-type filter on this handler keys off the singular through + // `metaTypeSingular`. A filter that only ran on `/meta/app` would be the + // #6238-class spelling hole one parameter over. + const { rest } = setup(['manage_users']); + + expect(namesFrom((await getList(rest, 'apps', { id: 'crm' })).body)).toEqual(['crm']); + expect(namesFrom((await getList(rest, 'apps', { id: 'no_such_app' })).body)).toEqual([]); + }); + + it('composes WITH the publish gate — `?id=` stays withheld from a non-builder', async () => { + // The filter narrows within what the caller may observe, never around + // it. ADR-0045 §3 says an unpublished app is externally unobservable, so + // naming it must answer the same empty list as naming a nonexistent one + // — the two are indistinguishable to a non-builder by design. + const denied = await getList(setup(['manage_users']).rest, 'app', { id: 'production_management' }); + expect(denied.statusCode).toBe(200); + expect(namesFrom(denied.body)).toEqual([]); + expect(JSON.stringify(denied.body)).not.toContain('secret_production_line'); + + // …and a builder, who may observe it, gets it — narrowed to just it. + const allowed = await getList(setup(['studio.access']).rest, 'app', { id: 'production_management' }); + expect(namesFrom(allowed.body)).toEqual(['production_management']); + }); + + it('does not depend on what the caller holds — a caller with NO permissions filters too', async () => { + // The permission filter above lives in a branch of its own, guarded by + // a resolved `ctx?.userId`. The `id` filter is deliberately NOT inside + // that branch: narrowing to the app you named is not a privilege, and a + // caller holding nothing asked the same question as an admin. + // + // (An anonymous caller is not the case to state this with: the + // anonymous-deny gate refuses `GET /meta/:type` with 401 before the + // handler body runs at all, unconditionally since #3963 — measured, not + // assumed. The least-privileged caller who reaches the filter is this + // one.) + const { rest } = setup([]); + + expect(namesFrom((await getList(rest, 'app', { id: 'account' })).body)).toEqual(['account']); + expect(namesFrom((await getList(rest, 'app', { id: 'no_such_app' })).body)).toEqual([]); + // Unfiltered, the same caller still receives everything the gate lets + // through — the filter is what changed, not the gate. + expect(namesFrom((await getList(rest, 'app')).body).length).toBeGreaterThan(1); + }); + + it('is scoped to `app` — another type\'s list is not narrowed by `?id=`', async () => { + // Deliberately not generalised: #7566 is filed on the app list, and + // teaching every meta type an `id` filter in the same change would be + // surface expansion with nothing measured behind it. `?id=` on another + // type keeps being ignored exactly as before. + const { rest, protocol } = setup(['manage_users']); + protocol.getMetaItems = vi.fn(async ({ type }: any) => + String(type ?? '') === 'view' ? [{ name: 'all_leads' }, { name: 'my_leads' }] : []); + + const res = await getList(rest, 'view', { id: 'all_leads' }); + expect(res.statusCode).toBe(200); + expect(namesFrom(res.body)).toEqual(['all_leads', 'my_leads']); + }); +}); diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 2eda71f636..72311339e2 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -4592,7 +4592,7 @@ export class RestServer { path: `${metaPath}/:type`, handler: async (req: any, res: any) => { try { - // [#6877] Four single-valued parameters on this list + // [#6877] Five single-valued parameters on this list // route, declared together at the top so the gate cannot // be missed by whichever branch reads its parameter // several hundred lines down: `?object=` (the view @@ -4602,7 +4602,18 @@ export class RestServer { // `?include=` (repeated, it stopped equalling // `'content'`, so a caller who asked for doc bodies got // the slimmed list back with a 200). - if (refuseRepeatedQueryParams(req, res, ['package', 'preview', 'object', 'include'])) return; + // + // [#7566] `?id=` joined them when the app branch below + // started honouring it. It is declared HERE, with the + // rest, rather than beside the filter that reads it, for + // the reason this block exists: a filter that arrives as + // `['crm','account']` and is compared against one app + // name matches nothing, and an empty app list is exactly + // the plausible-looking wrong answer #7566 was filed + // against. Refused, not resolved — see + // `query-multiplicity.ts` for why picking one of two + // conflicting intents is worse than a 400. + if (refuseRepeatedQueryParams(req, res, ['package', 'preview', 'object', 'include', 'id'])) return; const packageId = req.query?.package || undefined; const environmentId = isScoped ? req.params?.environmentId : undefined; const p = await this.resolveProtocol(environmentId, req); @@ -4717,6 +4728,82 @@ export class RestServer { } } + // [#7566] `GET /meta/app?id=` — the app-list filter, + // which until now was accepted and then dropped. + // + // Nothing on this route had ever read `id`: the block + // above narrows the list by PERMISSION and the branches + // around it by `?object=` / `?include=` / `?package=`, so + // `?id=crm` and `?id=not_an_app` produced the same three + // apps, byte for byte. A caller cannot tell a working + // filter from a dropped one — a client that asks for one + // app and renders `items[0]` gets a plausible, wrong + // answer, and a bogus id can never come back empty. + // + // ⚠️ Runs AFTER the RBAC filter above, on `visible` + // rather than on `items`. The two orders produce the same + // SET (both are pure filters), but not the same + // disclosure: narrowing first would hand `?id=` a one-element list to gate, and any + // future non-total gate — one that strips a field instead + // of dropping the document — would then be answering + // about an app the caller may not observe at all + // (ADR-0045 §3). Permission decides what exists for this + // caller; the filter narrows what they asked for within + // it, never the reverse. + // + // The match is on `name`, the App document's identity — + // `AppSchema.name`, "App unique machine name", the same + // key `GET /meta/app/:name` addresses and the same key + // the metadata store merges overlays on. `AppSchema` + // declares no `id` of its own (`id` appears on nav items + // and areas, never on the app), so there is no second + // identity to disagree with. + // + // A filter that matches nothing answers `200` with an + // EMPTY list, not a 404 — measured against this route's + // siblings, not chosen: `?package=` and + // `/meta/view?object=` both serve an + // empty list here, and the only meta 404 is the + // single-item address `GET /meta/:type/:name`. An empty + // list is the honest answer to "which apps have this id", + // and it is already observably different from the defect, + // which answered with all of them. + // + // Empty and absent spellings still mean "no filter", the + // same falsy gate `?package=` on this route has always + // used. The repeated spelling was refused at the top of + // the handler (#6877), so what arrives here is a string. + // + // Its own block rather than a line inside the branch + // above, because the two answer different questions: + // that branch is guarded on a resolved `ctx?.userId` and + // decides what this caller may observe, while narrowing + // to the app you named is not a privilege and must not + // acquire that guard's conditions. + const appIdFilter = RestServer.metaTypeSingular(req.params.type) === 'app' + ? req.query?.id + : undefined; + if (typeof appIdFilter === 'string' && appIdFilter !== '') { + const raw = visible as unknown; + // Only the two shapes this route serves are narrowed + // — a bare array or the `{ items: [] }` envelope. + // Anything else is left alone rather than replaced + // with an invented empty envelope: a filter must not + // be the thing that changes the response's shape. + const list: any[] | null = Array.isArray(raw) + ? (raw as any[]) + : (raw && typeof raw === 'object' && Array.isArray((raw as any).items)) + ? ((raw as any).items as any[]) + : null; + if (list) { + const matched = list.filter( + (a: any) => a && typeof a === 'object' && a.name === appIdFilter, + ); + visible = Array.isArray(raw) ? matched : { ...(raw as any), items: matched }; + } + } + // ADR-0057 D10: gate dashboard widgets by `requiresService` // the same way app nav entries are gated above. if (RestServer.metaTypeSingular(req.params.type) === 'dashboard') {