fix(rest): 服务端闸门也过滤 areas[].navigation —— area 内导航项权限不再只是渲染层的礼貌 (#4722)

.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 **里面**的项级闸门。

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');
+    });
+});

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<string>, 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();