From 87b475bec4e387f91036f1b40a62bca1a02fbc88 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 02:10:46 +0000 Subject: [PATCH] fix(rest): enforce nav-item gates inside `areas[]`, not just the top-level tree (#4722) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `filterAppForUser` is the server-side authority for app metadata visibility, but it walked only the app's top-level `navigation` tree: it returned early when `navigation` was absent and never read `item.areas` at all. An item gated inside an area was therefore enforced by the shell alone — the entry, and the `objectName` / `pageName` / `componentRef` it points at, still shipped in the `/meta` body, so reading the JSON (or poking client state) defeated the gate. The same `filterNav` now runs over every `areas[].navigation`. Reusing it is the point: one implementation means the two trees cannot drift on what `requiredPermissions` / `requiresService` mean. Both read paths are covered because both go through this function — the list `GET /meta/:type` and the single-item `GET /meta/:type/:name` (which already bypasses the shared cache for apps so this per-user filter can run). Collapse rule, taken from what the top-level tree already does to a `group`: an area emptied BY the gate is dropped (a bare label with nothing reachable under it has no consumer value), while an area authored empty is passed through — filtering reports what the caller may not see, it does not tidy the metadata. `resolveRegisteredServices` had to learn the same walk. It took one child list per node (`navigation` / `children` / `widgets`, first match wins) and never descended into `areas`, so a service named only inside an area went unprobed — and an unprobed name is absent from `registered`, which the gate reads as "service missing" and would strip a live entry. The probe set now covers exactly what the filter walks. Deliberately unchanged: `visible` (CEL) stays a client-side gate at every level — evaluating it server-side needs a bound `user` context this read layer does not have, and is its own change. The asymmetry is recorded in the code comment, in the liveness ledger, and in a characterisation pin. The retired AREA-level keys (#4651) are not revived; what is enforced is the item gate inside an area. The `areas is not walked` characterisation pin fails as its own comment predicted and is rewritten here, together with the `areas.navigation` note in `packages/spec/liveness/app.json` that asserted the same thing. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny --- .changeset/filter-app-areas-nav-gate.md | 33 +++ .../rest/src/meta-app-area-nav-gate.test.ts | 184 +++++++++++++++ packages/rest/src/rest-server.ts | 63 +++++- packages/rest/src/rest.test.ts | 214 ++++++++++++++++-- packages/spec/liveness/app.json | 10 +- 5 files changed, 476 insertions(+), 28 deletions(-) create mode 100644 .changeset/filter-app-areas-nav-gate.md create mode 100644 packages/rest/src/meta-app-area-nav-gate.test.ts diff --git a/.changeset/filter-app-areas-nav-gate.md b/.changeset/filter-app-areas-nav-gate.md new file mode 100644 index 0000000000..aa1d9103fb --- /dev/null +++ b/.changeset/filter-app-areas-nav-gate.md @@ -0,0 +1,33 @@ +--- +"@objectstack/rest": minor +--- + +fix(rest)!: 服务端权威闸门现在也过滤 `areas[].navigation` —— area 内导航项的权限/能力闸门不再只是渲染层的礼貌 (#4722) + +`filterAppForUser` 是 `/meta` 上 app 元数据的**服务端权威可见性闸门**,但它此前只走 app +的顶层 `navigation` 树:读到 `item.navigation` 不存在就原样返回,`item.areas` 从头到尾没被 +读过。后果是,写在 **area 内部**导航项上的 `requiredPermissions` / `requiresService` +只有客户端 `NavigationRenderer` 会执行 —— 该条目连同它的 `objectName` / `pageName` / +`componentRef` 指向,照常出现在 `/meta` 响应体里。改一次前端状态、或者直接读 `/meta` 的 +JSON,就能看到本该被 gate 掉的条目。对 areas 型 app 而言,导航项级闸门此前**不是**服务端强制。 + +**现在**:同一个 `filterNav` 被复用到每一棵 `areas[].navigation` 上 —— 不是第二份实现, +所以两棵树对同一个键的语义不可能漂移。列表 `GET /meta/apps` 与单项 `GET /meta/apps/:name` +两条路径都覆盖(两者都经过这个函数;单项读对 app 类型本就绕过缓存)。 + +**响应形状收紧(可能影响消费方)**:无权限用户拿到的 app 元数据里,被 gate 掉的 area 内 +导航项**不再出现**。被闸门滤空的 area 整个剥离 —— 与顶层树对「被滤空的 group」的既有处理 +同形(空壳标签没有消费价值);作者本就写成空的 area 原样返回(过滤只报告调用方看不到什么, +不负责整理元数据)。任何依赖「服务端会把 area 内条目全量下发、由客户端自己藏」的消费方需要 +改为信任服务端已过滤后的树 —— 这正是本次收紧的目的。 + +同一提交修正了 `resolveRegisteredServices` 的探测面:它此前每个节点只取第一个命中的子数组 +(`navigation` / `children` / `widgets` 三选一),不会下钻 `areas`。若不改,只在 area 内被 +引用的服务名不会被探测,而未探测的名字在闸门看来等同于「服务不存在」,会把一个本该存活的 +条目误剥离 —— 探测面必须与过滤面完全一致。 + +**明确不做**:`visible`(CEL)在任何层级仍然只在客户端求值 —— 服务端求值需要绑定 `user` +上下文,不是这个读路径现有的能力,另立单处理。这个不对称写进了代码注释、`packages/spec/liveness/app.json` +的账本 note,以及 `rest.test.ts` 的 characterisation pin。必须永不到达浏览器的东西,写 +`requiredPermissions`,不要写 `visible`。#4651 退役的 **area 级**键(`areas[].visible` / +`areas[].requiredPermissions`)未被复活:本次强制的是 area **里面**的项级闸门。 diff --git a/packages/rest/src/meta-app-area-nav-gate.test.ts b/packages/rest/src/meta-app-area-nav-gate.test.ts new file mode 100644 index 0000000000..3f75125385 --- /dev/null +++ b/packages/rest/src/meta-app-area-nav-gate.test.ts @@ -0,0 +1,184 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #4722 — the server is the authoritative visibility gate for app metadata, so +// the proof that matters is the RESPONSE BODY, not the private filter in +// isolation: an entry the caller may not see must not be in the JSON at all. +// +// Before this change `filterAppForUser` walked only the app's top-level +// `navigation` tree, so an item gated inside `areas[]` was hidden by the shell +// alone — the entry, and with it the `objectName` / `pageName` / `componentRef` +// it points at, still shipped in `/meta`. Reading the JSON (or poking client +// state) defeated the gate entirely. +// +// Both call sites are exercised here, because they are separate handlers that +// each re-derive the gate: the LIST read `GET /meta/:type` and the single-item +// read `GET /meta/:type/:name` (which bypasses the shared cache for apps +// precisely so this per-user filter can run). + +import { describe, it, expect, vi } from 'vitest'; +import { RestServer } from './rest-server'; + +// An areas-shaped app: no top-level navigation at all, which is the shape the +// pre-#4722 early return (`if (!nav) return item`) handed back untouched. +const AREA_APP = { + name: 'crm', + label: 'CRM', + areas: [ + { + id: 'area_sales', + label: 'Sales', + navigation: [ + { id: 'nav_leads', type: 'object', objectName: 'lead' }, + { id: 'nav_forecast', type: 'object', objectName: 'secret_forecast', requiredPermissions: ['sales.admin'] }, + { id: 'nav_ops_page', type: 'page', pageName: 'secret_ops_page', requiredPermissions: ['sales.admin'] }, + { id: 'nav_widget', type: 'component', componentRef: 'secret_widget', requiresService: 'org-scoping' }, + ], + }, + { + id: 'area_admin', + label: 'Admin', + navigation: [ + { id: 'nav_users', type: 'object', objectName: 'secret_sys_user', requiredPermissions: ['admin.access'] }, + ], + }, + ], +}; + +function createMockServer() { + return { + get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn(), patch: vi.fn(), use: vi.fn(), + listen: vi.fn().mockResolvedValue(undefined), close: vi.fn().mockResolvedValue(undefined), + }; +} + +function makeRes() { + const res: any = { statusCode: 200, body: undefined }; + res.status = vi.fn((c: number) => { res.statusCode = c; return res; }); + res.json = vi.fn((b: any) => { res.body = b; return res; }); + res.header = vi.fn(); res.setHeader = vi.fn(); res.write = vi.fn(); res.end = vi.fn(); + return res; +} + +/** + * @param perms system permissions the caller holds + * @param services which `requiresService` names the runtime has registered + */ +function setup(perms: string[], services: string[] = ['org-scoping']) { + const protocol: any = { + getDiscovery: vi.fn().mockResolvedValue({ version: 'v0', endpoints: { data: '', metadata: '', ui: '', auth: '/auth' } }), + getMetaTypes: vi.fn().mockResolvedValue([]), + // Deep-clone per call: the filter must never mutate stored metadata, and + // a shared object would hide that by carrying a prior call's damage. + getMetaItems: vi.fn(async ({ type }: any) => { + const t = String(type ?? ''); + return t === 'app' || t === 'apps' ? [JSON.parse(JSON.stringify(AREA_APP))] : []; + }), + getMetaItem: vi.fn(async () => JSON.parse(JSON.stringify(AREA_APP))), + findData: vi.fn().mockResolvedValue([]), + }; + const rest: any = new RestServer(createMockServer() as any, protocol, { api: { requireAuth: false } } as any); + // The RBAC filter only runs for a resolved caller; stubbing the context is + // the established pattern in this package for exercising it by route. + rest.resolveExecCtx = async () => ({ userId: 'u1', systemPermissions: perms }); + // ADR-0057 D10 capability probe (what `rest-api-plugin` wires in production). + rest.serviceExistsProvider = (n: string) => services.includes(n); + rest.registerRoutes(); + return { rest, protocol }; +} + +async function getList(rest: any, type = 'apps') { + 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); + return res; +} + +async function getItem(rest: any, name = 'crm', type = 'apps') { + const route = rest.getRoutes().find((r: any) => r.method === 'GET' && r.path === '/api/v1/meta/:type/:name'); + if (!route) throw new Error('meta/:type/:name route not registered'); + const res = makeRes(); + await route.handler({ method: 'GET', params: { type, name }, query: {}, body: {}, headers: {} }, res); + return res; +} + +const appFrom = (body: any): any => { + const raw = Array.isArray(body) ? body[0] : (body?.items ? body.items[0] : body); + return raw && raw.item ? raw.item : raw; +}; +const areaNav = (app: any, areaId: string): string[] => + (app?.areas?.find((a: any) => a.id === areaId)?.navigation ?? []).map((e: any) => e.id); + +describe('#4722 — `/meta` strips gated nav entries inside `areas[]`, on both read paths', () => { + it('LIST: a caller without the permissions receives neither the entries nor their targets', async () => { + const { rest } = setup([]); + const res = await getList(rest); + + expect(res.statusCode).toBe(200); + const app = appFrom(res.body); + // `nav_widget` survives on purpose: it carries only a capability gate, + // and this runtime HAS `org-scoping`. Permission gating is what is + // under test here — over-filtering would be a bug of its own. + expect(areaNav(app, 'area_sales')).toEqual(['nav_leads', 'nav_widget']); + // area_admin was emptied by the gate → dropped whole, no bare label. + expect(app.areas.map((a: any) => a.id)).toEqual(['area_sales']); + + // The acceptance criterion is about the wire bytes: every gated item's + // objectName / pageName / componentRef target is gone from the body. + const wire = JSON.stringify(res.body); + expect(wire).not.toContain('secret_forecast'); + expect(wire).not.toContain('secret_ops_page'); + expect(wire).not.toContain('secret_sys_user'); + // …while what the caller may see is still served. + expect(wire).toContain('lead'); + }); + + it('SINGLE ITEM: the same stripping applies to GET /meta/apps/:name', async () => { + const { rest } = setup([]); + const res = await getItem(rest); + + expect(res.statusCode).toBe(200); + const app = appFrom(res.body); + expect(areaNav(app, 'area_sales')).toEqual(['nav_leads', 'nav_widget']); + expect(app.areas.map((a: any) => a.id)).toEqual(['area_sales']); + + const wire = JSON.stringify(res.body); + expect(wire).not.toContain('secret_forecast'); + expect(wire).not.toContain('secret_ops_page'); + expect(wire).not.toContain('secret_sys_user'); + }); + + it('a caller who HOLDS the permissions still receives the whole tree (no over-filtering)', async () => { + const { rest } = setup(['sales.admin', 'admin.access']); + const list = appFrom((await getList(rest)).body); + const single = appFrom((await getItem(rest)).body); + + for (const app of [list, single]) { + expect(app.areas.map((a: any) => a.id)).toEqual(['area_sales', 'area_admin']); + expect(areaNav(app, 'area_sales')).toEqual(['nav_leads', 'nav_forecast', 'nav_ops_page', 'nav_widget']); + expect(areaNav(app, 'area_admin')).toEqual(['nav_users']); + } + }); + + it('ADR-0057 D10: a `requiresService` entry inside an area is stripped when the service is absent', async () => { + // Holds every permission — only the capability gate can remove the item. + const { rest } = setup(['sales.admin', 'admin.access'], []); + const list = appFrom((await getList(rest)).body); + const single = appFrom((await getItem(rest)).body); + + for (const app of [list, single]) { + expect(areaNav(app, 'area_sales')).toEqual(['nav_leads', 'nav_forecast', 'nav_ops_page']); + expect(JSON.stringify(app)).not.toContain('secret_widget'); + } + }); + + it('the probe covers services named only inside an area — a REGISTERED one is not stripped', async () => { + // The complement of the test above, and the reason + // `resolveRegisteredServices` had to learn to descend into `areas`: a + // name it never probes is absent from the registered set, which the + // gate reads as "service missing" and would strip a live entry. + const { rest } = setup(['sales.admin', 'admin.access'], ['org-scoping']); + const app = appFrom((await getList(rest)).body); + expect(areaNav(app, 'area_sales')).toContain('nav_widget'); + }); +}); diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index c79c1e7fa1..083b4f6ff0 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -1788,9 +1788,21 @@ export class RestServer { * - Recursively strips child navigation entries (groups, items) whose * `requiredPermissions` are not satisfied. Empty groups collapse so * the sidebar doesn't render a label with no children. + * - [#4722] Applies the SAME item gate to every `areas[].navigation` tree. + * Both trees are the same shape and the keys mean the same thing in both, + * so `filterNav` is reused — there is deliberately no second + * implementation to drift. Before this, an item gated inside an area was + * enforced by the shell alone: the entry (with its `objectName` / + * `pageName` / `componentRef` target) still shipped in the `/meta` body, + * so reading the JSON defeated it. + * + * NOT gated here: `visible` (CEL) at any level, and `requiresObject` — both + * are still evaluated client-side only. That asymmetry is deliberate and + * pinned in `rest.test.ts`: server-side CEL needs a bound `user` context + * that this layer does not have, and is its own change. * * Returns `null` when the app should be hidden from the user. Returns a - * shallow copy with a filtered `navigation` tree otherwise — the original + * shallow copy with filtered `navigation` / `areas` otherwise — the original * is never mutated so cached metadata stays clean. */ private filterAppForUser(item: any, sysPerms: Set, serviceGate?: (name: string) => boolean): any | null { @@ -1821,7 +1833,8 @@ export class RestServer { return null; } const nav = Array.isArray(item.navigation) ? item.navigation : null; - if (!nav) return item; + const areas = Array.isArray(item.areas) ? item.areas : null; + if (!nav && !areas) return item; const filterNav = (entries: any[]): any[] => { const out: any[] = []; @@ -1843,7 +1856,35 @@ export class RestServer { return out; }; - return { ...item, navigation: filterNav(nav) }; + // [#4722] `areas[]` carries no gate of its own — the area-level `visible` + // / `requiredPermissions` keys were retired in 17.0.0 (#4651, ADR-0049) + // and are NOT revived here. What is enforced is the gate on the items + // INSIDE an area, through the very same `filterNav` the top-level tree + // uses, so the two trees can never disagree about what a key means. + // + // Collapse rule, taken from what `filterNav` already does to a `group`: + // an area whose authored tree is emptied BY the gate is dropped (a bare + // area label with nothing reachable under it is not a useful response), + // while an area authored empty is passed through untouched — filtering + // reports what the caller may not see, it does not tidy the metadata. + const filterAreas = (list: any[]): any[] => { + const out: any[] = []; + for (const a of list) { + if (!a || typeof a !== 'object') continue; + const anav = Array.isArray(a.navigation) ? a.navigation : null; + if (!anav || anav.length === 0) { out.push(a); continue; } + const kids = filterNav(anav); + if (kids.length === 0) continue; + out.push({ ...a, navigation: kids }); + } + return out; + }; + + return { + ...item, + ...(nav ? { navigation: filterNav(nav) } : {}), + ...(areas ? { areas: filterAreas(areas) } : {}), + }; } /** @@ -1896,11 +1937,17 @@ export class RestServer { if (!e || typeof e !== 'object') return; if (isMetaEnvelope(e)) { walk((e as any).item); return; } if (typeof e.requiresService === 'string') wanted.add(e.requiresService); - const kids = Array.isArray(e.navigation) ? e.navigation - : Array.isArray(e.children) ? e.children - // Dashboard widgets carry their own `requiresService` gate. - : Array.isArray(e.widgets) ? e.widgets : null; - if (kids) for (const k of kids) walk(k); + // [#4722] EVERY child list, not the first one that happens to be an + // array. An app may carry `navigation` AND `areas` at once, and now + // that `filterAppForUser` gates the trees under `areas[]` too, a + // service named only in there must be probed — an unprobed name is + // absent from `registered`, and the gate would read that as "service + // missing" and strip a live entry. Fail-closed by omission is still + // wrong; the probe set must cover exactly what the gate walks. + for (const key of ['navigation', 'areas', 'children', 'widgets'] as const) { + const kids = (e as any)[key]; + if (Array.isArray(kids)) for (const k of kids) walk(k); + } }; for (const it of items) walk(it); if (wanted.size === 0) return new Set(); diff --git a/packages/rest/src/rest.test.ts b/packages/rest/src/rest.test.ts index ceb50597c7..6226d72333 100644 --- a/packages/rest/src/rest.test.ts +++ b/packages/rest/src/rest.test.ts @@ -2897,16 +2897,23 @@ describe('filterAppForUser — ADR-0045 hidden-app gate', () => { // // `app.areas[].visible` / `app.areas[].requiredPermissions` left the spec in // 17.0.0 because nothing evaluated them: this function reads the APP's -// `requiredPermissions` and then walks ONLY `item.navigation`. Removing a gate +// `requiredPermissions` and then walks the navigation trees. Removing a gate // that never gated is safe exactly and only while the gates that DO exist keep // working — otherwise the change trades "a gate that fails open" for "nobody -// checks whether the real gates are still there". Both surviving layers are +// checks whether the real gates are still there". Every surviving layer is // pinned here, at the server that is the authority for them. +// +// #4722 closed the real gap that #4651 left behind: the walk covered the +// top-level `navigation` tree ONLY, so an item gate written inside an +// `areas[]` tree was enforced by the shell alone and the entry still shipped +// in the `/meta` body. The area-LEVEL keys stay retired — what is enforced is +// the item gate, through the one `filterNav` both trees share. // --------------------------------------------------------------------------- -describe('filterAppForUser — the enforced permission layers (#4651)', () => { +describe('filterAppForUser — the enforced permission layers (#4651, #4722)', () => { const make = () => new RestServer(createMockServer() as any, createMockProtocol() as any, ANON_API as any); const ids = (a: any): string[] => (a?.navigation ?? []).map((e: any) => e.id); + const areaIds = (a: any, i: number): string[] => (a?.areas?.[i]?.navigation ?? []).map((e: any) => e.id); it('APP level: an app whose requiredPermissions the caller lacks is dropped entirely', () => { const rest: any = make(); @@ -2960,27 +2967,156 @@ describe('filterAppForUser — the enforced permission layers (#4651)', () => { expect(ids(rest.filterAppForUser(app, new Set(['admin.access'])))).toEqual(['grp_admin']); }); - it('characterises the boundary the #4651 prescription warns about: `areas` is not walked', () => { - // NOT an endorsement — a characterisation. The server filters the top-level - // `navigation` tree only, so an item gate nested under `areas[]` is enforced - // by the shell alone. That is exactly why the retirement's guidance tells an - // author to put anything that must never reach the browser in the top-level - // tree or its own app, and why route A (enforce area gates server-side) is a - // separate decision with its own semantics to settle. Whoever makes this - // walk areas should see THIS expectation fail and rewrite it deliberately, - // updating the `areas.navigation` ledger note in the same change. + it('AREA level: the item gate applies inside `areas[]` too, not just the top-level tree', () => { + // Rewritten in #4722 — this used to be a characterisation pinning the + // opposite ("`areas` is not walked"), with a note telling whoever made the + // server walk areas to rewrite it deliberately and update the + // `areas.navigation` liveness note in the same change. This is that + // rewrite: an item gate nested under an area is now enforced by the SERVER, + // so the gated entry — and its `objectName` / `pageName` / `componentRef` + // target with it — never reaches the browser at all. const rest: any = make(); - const app = { + const app = () => ({ name: 'crm', navigation: [{ id: 'nav_home', type: 'object' }], + areas: [{ + id: 'area_sales', label: 'Sales', + navigation: [ + { id: 'nav_leads', type: 'object', objectName: 'lead' }, + { id: 'nav_forecast', type: 'object', objectName: 'forecast', requiredPermissions: ['sales.admin'] }, + ], + }, { + id: 'area_admin', label: 'Admin', + navigation: [{ id: 'nav_users', type: 'object', objectName: 'sys_user', requiredPermissions: ['admin.access'] }], + }], + }); + + const out = rest.filterAppForUser(app(), new Set()); + expect(ids(out)).toEqual(['nav_home']); + // area_sales keeps only the ungated item; area_admin was emptied by the + // gate and is dropped whole — see the group-collapse pin below. + expect(out.areas.map((a: any) => a.id)).toEqual(['area_sales']); + expect(areaIds(out, 0)).toEqual(['nav_leads']); + // The gated targets are gone from the body, not merely marked. + expect(JSON.stringify(out)).not.toContain('forecast'); + expect(JSON.stringify(out)).not.toContain('sys_user'); + + const admin = rest.filterAppForUser(app(), new Set(['sales.admin', 'admin.access'])); + expect(admin.areas.map((a: any) => a.id)).toEqual(['area_sales', 'area_admin']); + expect(areaIds(admin, 0)).toEqual(['nav_leads', 'nav_forecast']); + expect(areaIds(admin, 1)).toEqual(['nav_users']); + }); + + it('AREA level: one implementation — nested groups inside an area collapse exactly as at top level', () => { + const rest: any = make(); + const app = () => ({ + name: 'crm', + areas: [{ + id: 'area_ops', label: 'Ops', + navigation: [ + { id: 'nav_tasks', type: 'object' }, + { + id: 'grp_admin', type: 'group', children: [ + { id: 'nav_users', type: 'object', requiredPermissions: ['admin.access'] }, + ], + }, + { + id: 'grp_mixed', type: 'group', children: [ + { id: 'nav_audit', type: 'object', requiredPermissions: ['admin.access'] }, + { id: 'nav_about', type: 'url' }, + ], + }, + ], + }], + }); + const out = rest.filterAppForUser(app(), new Set()); + // grp_admin emptied → dropped; grp_mixed keeps its ungated child. + expect(areaIds(out, 0)).toEqual(['nav_tasks', 'grp_mixed']); + expect(out.areas[0].navigation[1].children.map((c: any) => c.id)).toEqual(['nav_about']); + + const admin = rest.filterAppForUser(app(), new Set(['admin.access'])); + expect(areaIds(admin, 0)).toEqual(['nav_tasks', 'grp_admin', 'grp_mixed']); + }); + + it('AREA level: an area emptied BY the gate is dropped; an area authored empty is passed through', () => { + // Same shape as the top-level group-collapse rule pinned above: collapse is + // a consequence of filtering, never a tidy-up of what the author wrote. + const rest: any = make(); + const gatedEmpty = { + name: 'crm', areas: [{ id: 'area_admin', label: 'Admin', navigation: [{ id: 'nav_users', type: 'object', requiredPermissions: ['admin.access'] }], }], }; + expect(rest.filterAppForUser(gatedEmpty, new Set()).areas).toEqual([]); + expect( + rest.filterAppForUser(gatedEmpty, new Set(['admin.access'])).areas.map((a: any) => a.id), + ).toEqual(['area_admin']); + + const authoredEmpty = { name: 'crm', areas: [{ id: 'area_soon', label: 'Soon', navigation: [] }] }; + expect(rest.filterAppForUser(authoredEmpty, new Set()).areas.map((a: any) => a.id)) + .toEqual(['area_soon']); + }); + + it('AREA level: an app with areas but no top-level navigation is still filtered', () => { + // The pre-#4722 early return (`if (!nav) return item`) handed this shape + // back untouched — which is precisely the areas-only app the bug hurt most. + const rest: any = make(); + const app = { + name: 'crm', + areas: [{ + id: 'area_sales', label: 'Sales', + navigation: [ + { id: 'nav_leads', type: 'object' }, + { id: 'nav_forecast', type: 'object', requiredPermissions: ['sales.admin'] }, + ], + }], + }; const out = rest.filterAppForUser(app, new Set()); - expect(ids(out)).toEqual(['nav_home']); - expect(out.areas[0].navigation.map((e: any) => e.id)).toEqual(['nav_users']); + expect(out.navigation).toBeUndefined(); + expect(areaIds(out, 0)).toEqual(['nav_leads']); + }); + + it('does not mutate the app it filters (cached metadata stays whole)', () => { + const rest: any = make(); + const app = { + name: 'crm', + navigation: [{ id: 'nav_home', type: 'object' }], + areas: [{ + id: 'area_admin', label: 'Admin', + navigation: [ + { id: 'nav_users', type: 'object', requiredPermissions: ['admin.access'] }, + { id: 'nav_docs', type: 'url' }, + ], + }], + }; + const before = JSON.stringify(app); + rest.filterAppForUser(app, new Set()); + expect(JSON.stringify(app)).toBe(before); + }); + + it('characterises what the server still does NOT evaluate: `visible` CEL, at any level', () => { + // NOT an endorsement — a characterisation, and a deliberate #4722 + // non-goal. `visible` is a CEL expression that needs a bound `user` + // context the REST read layer does not have, so it stays a client-side + // gate at BOTH levels while `requiredPermissions` / `requiresService` are + // enforced at both. Anything that must never reach the browser goes in + // `requiredPermissions`, not `visible`. Whoever binds CEL server-side + // should see this expectation fail and rewrite it, updating the + // `navigation.visible` liveness note in the same change. + const rest: any = make(); + const app = { + name: 'crm', + navigation: [{ id: 'nav_secret_top', type: 'object', visible: 'false' }], + areas: [{ + id: 'area_admin', label: 'Admin', + navigation: [{ id: 'nav_secret_area', type: 'object', visible: 'false' }], + }], + }; + const out = rest.filterAppForUser(app, new Set()); + expect(ids(out)).toEqual(['nav_secret_top']); + expect(areaIds(out, 0)).toEqual(['nav_secret_area']); }); }); @@ -3034,6 +3170,54 @@ describe('filterAppForUser — ADR-0057 D10 requiresService gate', () => { expect(reg.size).toBe(1); }); + it('[#4722] the gate strips requiresService entries inside `areas[]` as well', () => { + const rest: any = make(); + const areaApp = () => ({ + name: 'setup', + areas: [{ + id: 'area_admin', label: 'Admin', + navigation: [ + { id: 'nav_users', type: 'object' }, + { id: 'nav_organizations', type: 'object', objectName: 'sys_organization', requiresService: 'org-scoping' }, + ], + }], + }); + const nav = (a: any): string[] => (a?.areas?.[0]?.navigation ?? []).map((e: any) => e.id); + expect(nav(rest.filterAppForUser(areaApp(), new Set(), (n: string) => n !== 'org-scoping'))) + .toEqual(['nav_users']); + expect(nav(rest.filterAppForUser(areaApp(), new Set(), () => true))) + .toEqual(['nav_users', 'nav_organizations']); + // Fail-open when the gate cannot be probed at all, as at top level. + expect(nav(rest.filterAppForUser(areaApp(), new Set()))) + .toEqual(['nav_users', 'nav_organizations']); + }); + + it('[#4722] resolveRegisteredServices probes services named only inside `areas[]`', async () => { + // Without this the area gate would fail CLOSED by omission: an unprobed + // name is missing from `registered`, which the gate reads as "absent" and + // strips a live entry. The probe set must cover what the filter walks. + const rest: any = make(); + const kernel = { getServiceAsync: async (n: string) => (n === 'org-scoping' ? {} : null) }; + const areaApp = { + name: 'setup', + navigation: [{ id: 'nav_home', type: 'object' }], + areas: [{ + id: 'area_admin', label: 'Admin', + navigation: [ + { id: 'nav_organizations', type: 'object', requiresService: 'org-scoping' }, + { id: 'grp', type: 'group', children: [{ id: 'nav_x', type: 'object', requiresService: 'nope' }] }, + ], + }], + }; + const reg = await rest.resolveRegisteredServices(kernel, [areaApp]); + expect(reg.has('org-scoping')).toBe(true); + expect(reg.has('nope')).toBe(false); + + // End-to-end: probe + gate keeps the registered one, drops the absent one. + const gated = rest.filterAppForUser(areaApp, new Set(), (n: string) => reg.has(n)); + expect(gated.areas[0].navigation.map((e: any) => e.id)).toEqual(['nav_organizations']); + }); + it('unwraps the getMetaItem envelope and gates the inner app (regression)', () => { const rest: any = make(); const envelope = { diff --git a/packages/spec/liveness/app.json b/packages/spec/liveness/app.json index 47067c83de..9dc223a60b 100644 --- a/packages/spec/liveness/app.json +++ b/packages/spec/liveness/app.json @@ -1,6 +1,6 @@ { "type": "app", - "_note": "AppSchema — the navigation shell, the densest hand-authored surface on the platform. Consumers: the REST read layer's filterAppForUser (packages/rest/src/rest-server.ts:1796-1847 — the SERVER-side authority for app/nav permission + capability gating and ADR-0045 hidden-app visibility), the spec i18n translateApp (i18n-resolver.ts:472), and objectui's shell (@940ba24: app-shell AppSidebar/ConsoleLayout/ContextSelectors, layout NavigationRenderer, console RootLandingRedirect). The #4001/#4142 app step already retired seven dead keys as retiredKey tombstones — they stay in the walked shape, so their rows stay here (tombstone rule, orphans.mts). WALK BOUNDARY (#3095 union rule): `navigation` drills into the union's FIRST member (the `object` variant + base keys); the other variants' payload keys sit outside the walk and were verified by hand — dashboardName (NavigationRenderer.tsx:433), pageName (:435-442), url/target (:462), reportName (:460), componentRef (:464,:644), group `expanded` (:856) all live. The one GAP found there is now CLOSED (#4509, objectui @e8bec83): an `action` item's click dispatches through a host-supplied `onAction` prop that no shipped shell passed, so `actionDef.actionName` reached no dispatcher and every such item dead-clicked. objectui's `useNavActionDispatch` (objectui: packages/app-shell/src/hooks/useNavActionDispatch.ts) resolves the name against `action` metadata and dispatches through the console action runtime, and UnifiedSidebar passes it (objectui: packages/app-shell/src/layout/UnifiedSidebar.tsx:473). A shell that still passes no handler now HIDES action items rather than rendering them dead (objectui: packages/layout/src/NavigationRenderer.tsx:971) — the renderer stops manufacturing the trap. Also note filterAppForUser walks ONLY the top-level `navigation` tree — it never reads `item.areas` at all (rest-server.ts returns early when `navigation` is absent), and the client area switcher renders every area. That made area-level `visible` / `requiredPermissions` FAIL-OPEN gates, not merely unread: a \"hidden\" or permission-gated area showed to everyone. AREA GATES, 17.0.0 (#4651): both keys REMOVED and their rows DELETED — NavigationAreaSchema is strict, so the keys left the walked shape and retained rows would report ORPHAN. Route B (remove) over route A (enforce) was the maintainer's call: enforcing needs semantics decided first (does filtering an area remove its items everywhere? does the server bind `user` for area CEL?), which the 17.0.0 window could not hold, and a gate that never gated is strictly safer removed than shipped for a whole major. The strict rejection carries the prescription (ui/app.zod.ts AREA_VISIBLE_RETIRED / AREA_REQUIRED_PERMISSIONS_RETIRED) and names the layers that DO enforce. The underlying boundary is unchanged and still recorded on `areas.navigation` below: per-item gating inside an area is shell-side only. Seeded 2026-08-01 (#4488). CONTEXT SELECTORS, 17.0.0 (#4509): `includeAll` and `placement` rows DELETED — AppContextSelectorSchema is strict, so the keys left the walked shape and retained rows would report ORPHAN. Both were unwarnable (schema defaults materialize at parse, so the lint could not tell authored from supplied), which made removal the only channel that could reach an author. `includeAll` was the sharp one: not unread but deliberately DISOBEYED — selectors are mandatory-scope, and an All row would clear the scope, which on Studio's package selector means listing the platform's own system/cloud kernel packages. STUDIO_APP authored `includeAll: true` against a renderer that ignored it, and that authoring site went with the key.", + "_note": "AppSchema — the navigation shell, the densest hand-authored surface on the platform. Consumers: the REST read layer's filterAppForUser (packages/rest/src/rest-server.ts:1808-1888 — the SERVER-side authority for app/nav permission + capability gating and ADR-0045 hidden-app visibility), the spec i18n translateApp (i18n-resolver.ts:472), and objectui's shell (@940ba24: app-shell AppSidebar/ConsoleLayout/ContextSelectors, layout NavigationRenderer, console RootLandingRedirect). The #4001/#4142 app step already retired seven dead keys as retiredKey tombstones — they stay in the walked shape, so their rows stay here (tombstone rule, orphans.mts). WALK BOUNDARY (#3095 union rule): `navigation` drills into the union's FIRST member (the `object` variant + base keys); the other variants' payload keys sit outside the walk and were verified by hand — dashboardName (NavigationRenderer.tsx:433), pageName (:435-442), url/target (:462), reportName (:460), componentRef (:464,:644), group `expanded` (:856) all live. The one GAP found there is now CLOSED (#4509, objectui @e8bec83): an `action` item's click dispatches through a host-supplied `onAction` prop that no shipped shell passed, so `actionDef.actionName` reached no dispatcher and every such item dead-clicked. objectui's `useNavActionDispatch` (objectui: packages/app-shell/src/hooks/useNavActionDispatch.ts) resolves the name against `action` metadata and dispatches through the console action runtime, and UnifiedSidebar passes it (objectui: packages/app-shell/src/layout/UnifiedSidebar.tsx:473). A shell that still passes no handler now HIDES action items rather than rendering them dead (objectui: packages/layout/src/NavigationRenderer.tsx:971) — the renderer stops manufacturing the trap. Also note that filterAppForUser USED to walk only the top-level `navigation` tree — it never read `item.areas` at all (it returned early when `navigation` was absent), while the client area switcher renders every area. That made area-level `visible` / `requiredPermissions` FAIL-OPEN gates, not merely unread: a \"hidden\" or permission-gated area showed to everyone. Closed in #4722 for the layer that survived the retirement — the server now runs the same filterNav over every `areas[].navigation` — see the `areas.navigation` row below. AREA GATES, 17.0.0 (#4651): both keys REMOVED and their rows DELETED — NavigationAreaSchema is strict, so the keys left the walked shape and retained rows would report ORPHAN. Route B (remove) over route A (enforce) was the maintainer's call: enforcing needs semantics decided first (does filtering an area remove its items everywhere? does the server bind `user` for area CEL?), which the 17.0.0 window could not hold, and a gate that never gated is strictly safer removed than shipped for a whole major. The strict rejection carries the prescription (ui/app.zod.ts AREA_VISIBLE_RETIRED / AREA_REQUIRED_PERMISSIONS_RETIRED) and names the layers that DO enforce. The boundary those prescriptions pointed at — per-item gating inside an area being shell-side only — was the real gap #4651 left behind, and #4722 closed it: item-level `requiredPermissions` / `requiresService` are now stripped server-side inside `areas[]` too, `visible` (CEL) deliberately not. Recorded on `areas.navigation` below. The area-LEVEL keys remain retired; they were not revived. Seeded 2026-08-01 (#4488). CONTEXT SELECTORS, 17.0.0 (#4509): `includeAll` and `placement` rows DELETED — AppContextSelectorSchema is strict, so the keys left the walked shape and retained rows would report ORPHAN. Both were unwarnable (schema defaults materialize at parse, so the lint could not tell authored from supplied), which made removal the only channel that could reach an author. `includeAll` was the sharp one: not unread but deliberately DISOBEYED — selectors are mandatory-scope, and an All row would clear the scope, which on Studio's package selector means listing the platform's own system/cloud kernel packages. STUDIO_APP authored `includeAll: true` against a renderer that ignored it, and that authoring site went with the key.", "props": { "name": { "status": "live", @@ -91,13 +91,13 @@ "status": "live", "verifiedAt": "2026-08-01", "evidence": "objectui @940ba24: packages/layout/src/NavigationRenderer.tsx:891 (item gate); objectui packages/app-shell/src/layout/AppSidebar.tsx:236 (CEL evaluation via ExpressionProvider)", - "note": "the CEL visibility gate — enforced per item. This is the layer the retired `areas[].visible` (#4651) prescribes moving to: same CEL dialect, one level down, and actually evaluated." + "note": "the CEL visibility gate — enforced per item. This is the layer the retired `areas[].visible` (#4651) prescribes moving to: same CEL dialect, one level down, and actually evaluated. CLIENT-side only, at both levels, and deliberately so: #4722 made the server enforce `requiredPermissions` / `requiresService` inside `areas[]` but left CEL where it was, because evaluating it server-side needs a bound `user` context the REST read layer does not have. Anything that must never reach the browser belongs in `requiredPermissions`, not `visible` — the asymmetry is pinned in packages/rest/src/rest.test.ts." }, "requiredPermissions": { "status": "live", "verifiedAt": "2026-08-01", - "evidence": "packages/rest/src/rest-server.ts:1830; objectui @940ba24: packages/layout/src/NavigationRenderer.tsx:894", - "note": "enforced in BOTH layers: the server strips unsatisfied entries from the top-level navigation tree before serving, and the client re-gates per item." + "evidence": "packages/rest/src/rest-server.ts:1844; objectui @940ba24: packages/layout/src/NavigationRenderer.tsx:894", + "note": "enforced in BOTH layers: the server strips unsatisfied entries before serving — from the top-level navigation tree AND, since #4722, from every `areas[].navigation` tree through the same filterNav — and the client re-gates per item." }, "requiresObject": { "status": "live", @@ -181,7 +181,7 @@ "status": "live", "verifiedAt": "2026-08-01", "evidence": "objectui @940ba24: packages/app-shell/src/layout/AppSidebar.tsx:210, packages/layout/src/AppSchemaRenderer.tsx:469", - "note": "the active area's tree replaces the top-level navigation. NOTE: area trees are NOT server-side permission-stripped (filterAppForUser reads only `item.navigation`) — per-item gating inside an area is client-side only." + "note": "the active area's tree replaces the top-level navigation. Since #4722 area trees ARE server-side gated: filterAppForUser (packages/rest/src/rest-server.ts:1870) runs the SAME filterNav over every `areas[].navigation`, so an item's `requiredPermissions` / `requiresService` is enforced identically in both trees and a gated entry (with its objectName/pageName/componentRef target) never reaches the browser. An area emptied BY the gate is dropped, mirroring the top-level group collapse; an area authored empty is passed through. Still client-only at both levels: `visible` (CEL — needs a bound user context the read layer lacks) and `requiresObject`. The area-LEVEL keys stay retired (#4651) — this enforces the items inside, not a revived area gate." } }, "note": "Drilled because the gating keys diverged sharply from the live identity/tree keys — and they are gone: `visible` and `requiredPermissions` were RETIRED in 17.0.0 (#4651), rows DELETED because NavigationAreaSchema is strict, so the keys left the walked shape and retained rows would report ORPHAN. Keep drilling: `description` is the surviving benign dead key, and the drill is what would catch a new gate being added here."