fix(rest): honour ?id= on GET /api/v1/meta/app instead of dropping it (#7566) (#7662)

`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=<no such package>` and
  `/meta/view?object=<no such 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=<unpublished app>` 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.


Claude-Session: https://claude.ai/code/session_01BjEdc4MCajjEPDk4G33tga

Co-authored-by: Claude <noreply@anthropic.com>

Author: os-help

.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=<no such
+package>` and `/meta/view?object=<no such 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=<an unpublished app>` answers the
+same empty list to a non-builder as `?id=<nonexistent>` — 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 `<select>` submits. Other metadata
+types are untouched: `?id=` on `/meta/view` and friends keeps being ignored
+exactly as before, since #7566 is filed on the app list and teaching every type an
+`id` filter in the same change would be surface expansion with nothing measured
+behind it.

packages/rest/src/meta-app-publish-gate.test.ts

@@ -95,11 +95,11 @@ function setup(perms: string[]) {
     return { rest, protocol };
 }
 
-async function getList(rest: any, type = 'app') {
+async function getList(rest: any, type = 'app', query: Record<string, unknown> = {}) {
     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: {}, headers: {} }, res);
+    await route.handler({ method: 'GET', params: { type }, query, body: {}, headers: {} }, res);
     return res;
 }
 
@@ -183,3 +183,148 @@ describe('#4829 — `GET /meta/app` gates on `_unpublished`, never on `hidden`',
         expect(allowed.body?.item?.name).toBe('production_management');
     });
 });
+
+// ── #7566 ───────────────────────────────────────────────────────────────────
+//
+// `GET /api/v1/meta/app?id=…` accepted the parameter and then dropped it: the
+// SAME apps came back for every value, including one that names no app. The
+// acceptance criterion is therefore a BODY fact, not a status code — a route
+// that 200s either way is exactly what the reporter saw. Every case below
+// asserts the app names in the response.
+//
+// The defect's cost is that 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.
+//
+// This file already owns `GET /meta/app`'s list body (the #4829 gate above),
+// and the filter has to COMPOSE with that gate rather than sit beside it — an
+// `?id=` naming an unpublished app must still be withheld from a non-builder —
+// so the cases live here with the fixture that has an unpublished app in it.
+
+describe('#7566 — `GET /meta/app?id=` narrows the list instead of being dropped', () => {
+    it('a MATCHING id returns exactly that app', async () => {
+        const { rest } = setup(['manage_users']);
+        const res = await getList(rest, 'app', { id: 'crm' });
+
+        expect(res.statusCode).toBe(200);
+        expect(namesFrom(res.body)).toEqual(['crm']);
+        // The stated failure mode: the unasked-for apps are gone. Before this
+        // change the assertion below is what failed — `account` came back too.
+        expect(namesFrom(res.body)).not.toContain('account');
+    });
+
+    it('a NON-MATCHING id returns an empty list — a 200, not a 404, and not every app', async () => {
+        const { rest } = setup(['manage_users']);
+        const res = await getList(rest, 'app', { id: 'no_such_app' });
+
+        // Empty-vs-404 is measured off this route's siblings, not chosen: the
+        // list route serves an empty list for `?package=<no such package>` and
+        // for `/meta/view?object=<no such object>`, and the only 404 on the meta
+        // surface is the single-item address `GET /meta/:type/:name` (pinned
+        // above). A list filter that matched nothing is a list of nothing.
+        expect(res.statusCode).toBe(200);
+        expect(namesFrom(res.body)).toEqual([]);
+        expect(res.body?.error).toBeUndefined();
+    });
+
+    it('an ABSENT id still returns the whole published set (the preservation half)', async () => {
+        const { rest } = setup(['manage_users']);
+
+        expect(namesFrom((await getList(rest, 'app')).body).sort()).toEqual(['account', 'crm']);
+        // `?id=` (empty) is the "no filter" spelling an unset `<select>` 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=<unpublished>` 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']);
+    });
+});