Skip to content

Commit 87b475b

Browse files
committed
fix(rest): enforce nav-item gates inside areas[], not just the top-level tree (#4722)
`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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny
1 parent dcb1dad commit 87b475b

5 files changed

Lines changed: 476 additions & 28 deletions

File tree

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
---
2+
"@objectstack/rest": minor
3+
---
4+
5+
fix(rest)!: 服务端权威闸门现在也过滤 `areas[].navigation` —— area 内导航项的权限/能力闸门不再只是渲染层的礼貌 (#4722)
6+
7+
`filterAppForUser``/meta` 上 app 元数据的**服务端权威可见性闸门**,但它此前只走 app
8+
的顶层 `navigation` 树:读到 `item.navigation` 不存在就原样返回,`item.areas` 从头到尾没被
9+
读过。后果是,写在 **area 内部**导航项上的 `requiredPermissions` / `requiresService`
10+
只有客户端 `NavigationRenderer` 会执行 —— 该条目连同它的 `objectName` / `pageName` /
11+
`componentRef` 指向,照常出现在 `/meta` 响应体里。改一次前端状态、或者直接读 `/meta`
12+
JSON,就能看到本该被 gate 掉的条目。对 areas 型 app 而言,导航项级闸门此前**不是**服务端强制。
13+
14+
**现在**:同一个 `filterNav` 被复用到每一棵 `areas[].navigation` 上 —— 不是第二份实现,
15+
所以两棵树对同一个键的语义不可能漂移。列表 `GET /meta/apps` 与单项 `GET /meta/apps/:name`
16+
两条路径都覆盖(两者都经过这个函数;单项读对 app 类型本就绕过缓存)。
17+
18+
**响应形状收紧(可能影响消费方)**:无权限用户拿到的 app 元数据里,被 gate 掉的 area 内
19+
导航项**不再出现**。被闸门滤空的 area 整个剥离 —— 与顶层树对「被滤空的 group」的既有处理
20+
同形(空壳标签没有消费价值);作者本就写成空的 area 原样返回(过滤只报告调用方看不到什么,
21+
不负责整理元数据)。任何依赖「服务端会把 area 内条目全量下发、由客户端自己藏」的消费方需要
22+
改为信任服务端已过滤后的树 —— 这正是本次收紧的目的。
23+
24+
同一提交修正了 `resolveRegisteredServices` 的探测面:它此前每个节点只取第一个命中的子数组
25+
(`navigation` / `children` / `widgets` 三选一),不会下钻 `areas`。若不改,只在 area 内被
26+
引用的服务名不会被探测,而未探测的名字在闸门看来等同于「服务不存在」,会把一个本该存活的
27+
条目误剥离 —— 探测面必须与过滤面完全一致。
28+
29+
**明确不做**:`visible`(CEL)在任何层级仍然只在客户端求值 —— 服务端求值需要绑定 `user`
30+
上下文,不是这个读路径现有的能力,另立单处理。这个不对称写进了代码注释、`packages/spec/liveness/app.json`
31+
的账本 note,以及 `rest.test.ts` 的 characterisation pin。必须永不到达浏览器的东西,写
32+
`requiredPermissions`,不要写 `visible`#4651 退役的 **area 级**键(`areas[].visible` /
33+
`areas[].requiredPermissions`)未被复活:本次强制的是 area **里面**的项级闸门。
Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// #4722 — the server is the authoritative visibility gate for app metadata, so
4+
// the proof that matters is the RESPONSE BODY, not the private filter in
5+
// isolation: an entry the caller may not see must not be in the JSON at all.
6+
//
7+
// Before this change `filterAppForUser` walked only the app's top-level
8+
// `navigation` tree, so an item gated inside `areas[]` was hidden by the shell
9+
// alone — the entry, and with it the `objectName` / `pageName` / `componentRef`
10+
// it points at, still shipped in `/meta`. Reading the JSON (or poking client
11+
// state) defeated the gate entirely.
12+
//
13+
// Both call sites are exercised here, because they are separate handlers that
14+
// each re-derive the gate: the LIST read `GET /meta/:type` and the single-item
15+
// read `GET /meta/:type/:name` (which bypasses the shared cache for apps
16+
// precisely so this per-user filter can run).
17+
18+
import { describe, it, expect, vi } from 'vitest';
19+
import { RestServer } from './rest-server';
20+
21+
// An areas-shaped app: no top-level navigation at all, which is the shape the
22+
// pre-#4722 early return (`if (!nav) return item`) handed back untouched.
23+
const AREA_APP = {
24+
name: 'crm',
25+
label: 'CRM',
26+
areas: [
27+
{
28+
id: 'area_sales',
29+
label: 'Sales',
30+
navigation: [
31+
{ id: 'nav_leads', type: 'object', objectName: 'lead' },
32+
{ id: 'nav_forecast', type: 'object', objectName: 'secret_forecast', requiredPermissions: ['sales.admin'] },
33+
{ id: 'nav_ops_page', type: 'page', pageName: 'secret_ops_page', requiredPermissions: ['sales.admin'] },
34+
{ id: 'nav_widget', type: 'component', componentRef: 'secret_widget', requiresService: 'org-scoping' },
35+
],
36+
},
37+
{
38+
id: 'area_admin',
39+
label: 'Admin',
40+
navigation: [
41+
{ id: 'nav_users', type: 'object', objectName: 'secret_sys_user', requiredPermissions: ['admin.access'] },
42+
],
43+
},
44+
],
45+
};
46+
47+
function createMockServer() {
48+
return {
49+
get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn(), patch: vi.fn(), use: vi.fn(),
50+
listen: vi.fn().mockResolvedValue(undefined), close: vi.fn().mockResolvedValue(undefined),
51+
};
52+
}
53+
54+
function makeRes() {
55+
const res: any = { statusCode: 200, body: undefined };
56+
res.status = vi.fn((c: number) => { res.statusCode = c; return res; });
57+
res.json = vi.fn((b: any) => { res.body = b; return res; });
58+
res.header = vi.fn(); res.setHeader = vi.fn(); res.write = vi.fn(); res.end = vi.fn();
59+
return res;
60+
}
61+
62+
/**
63+
* @param perms system permissions the caller holds
64+
* @param services which `requiresService` names the runtime has registered
65+
*/
66+
function setup(perms: string[], services: string[] = ['org-scoping']) {
67+
const protocol: any = {
68+
getDiscovery: vi.fn().mockResolvedValue({ version: 'v0', endpoints: { data: '', metadata: '', ui: '', auth: '/auth' } }),
69+
getMetaTypes: vi.fn().mockResolvedValue([]),
70+
// Deep-clone per call: the filter must never mutate stored metadata, and
71+
// a shared object would hide that by carrying a prior call's damage.
72+
getMetaItems: vi.fn(async ({ type }: any) => {
73+
const t = String(type ?? '');
74+
return t === 'app' || t === 'apps' ? [JSON.parse(JSON.stringify(AREA_APP))] : [];
75+
}),
76+
getMetaItem: vi.fn(async () => JSON.parse(JSON.stringify(AREA_APP))),
77+
findData: vi.fn().mockResolvedValue([]),
78+
};
79+
const rest: any = new RestServer(createMockServer() as any, protocol, { api: { requireAuth: false } } as any);
80+
// The RBAC filter only runs for a resolved caller; stubbing the context is
81+
// the established pattern in this package for exercising it by route.
82+
rest.resolveExecCtx = async () => ({ userId: 'u1', systemPermissions: perms });
83+
// ADR-0057 D10 capability probe (what `rest-api-plugin` wires in production).
84+
rest.serviceExistsProvider = (n: string) => services.includes(n);
85+
rest.registerRoutes();
86+
return { rest, protocol };
87+
}
88+
89+
async function getList(rest: any, type = 'apps') {
90+
const route = rest.getRoutes().find((r: any) => r.method === 'GET' && r.path === '/api/v1/meta/:type');
91+
if (!route) throw new Error('meta/:type route not registered');
92+
const res = makeRes();
93+
await route.handler({ method: 'GET', params: { type }, query: {}, body: {}, headers: {} }, res);
94+
return res;
95+
}
96+
97+
async function getItem(rest: any, name = 'crm', type = 'apps') {
98+
const route = rest.getRoutes().find((r: any) => r.method === 'GET' && r.path === '/api/v1/meta/:type/:name');
99+
if (!route) throw new Error('meta/:type/:name route not registered');
100+
const res = makeRes();
101+
await route.handler({ method: 'GET', params: { type, name }, query: {}, body: {}, headers: {} }, res);
102+
return res;
103+
}
104+
105+
const appFrom = (body: any): any => {
106+
const raw = Array.isArray(body) ? body[0] : (body?.items ? body.items[0] : body);
107+
return raw && raw.item ? raw.item : raw;
108+
};
109+
const areaNav = (app: any, areaId: string): string[] =>
110+
(app?.areas?.find((a: any) => a.id === areaId)?.navigation ?? []).map((e: any) => e.id);
111+
112+
describe('#4722 — `/meta` strips gated nav entries inside `areas[]`, on both read paths', () => {
113+
it('LIST: a caller without the permissions receives neither the entries nor their targets', async () => {
114+
const { rest } = setup([]);
115+
const res = await getList(rest);
116+
117+
expect(res.statusCode).toBe(200);
118+
const app = appFrom(res.body);
119+
// `nav_widget` survives on purpose: it carries only a capability gate,
120+
// and this runtime HAS `org-scoping`. Permission gating is what is
121+
// under test here — over-filtering would be a bug of its own.
122+
expect(areaNav(app, 'area_sales')).toEqual(['nav_leads', 'nav_widget']);
123+
// area_admin was emptied by the gate → dropped whole, no bare label.
124+
expect(app.areas.map((a: any) => a.id)).toEqual(['area_sales']);
125+
126+
// The acceptance criterion is about the wire bytes: every gated item's
127+
// objectName / pageName / componentRef target is gone from the body.
128+
const wire = JSON.stringify(res.body);
129+
expect(wire).not.toContain('secret_forecast');
130+
expect(wire).not.toContain('secret_ops_page');
131+
expect(wire).not.toContain('secret_sys_user');
132+
// …while what the caller may see is still served.
133+
expect(wire).toContain('lead');
134+
});
135+
136+
it('SINGLE ITEM: the same stripping applies to GET /meta/apps/:name', async () => {
137+
const { rest } = setup([]);
138+
const res = await getItem(rest);
139+
140+
expect(res.statusCode).toBe(200);
141+
const app = appFrom(res.body);
142+
expect(areaNav(app, 'area_sales')).toEqual(['nav_leads', 'nav_widget']);
143+
expect(app.areas.map((a: any) => a.id)).toEqual(['area_sales']);
144+
145+
const wire = JSON.stringify(res.body);
146+
expect(wire).not.toContain('secret_forecast');
147+
expect(wire).not.toContain('secret_ops_page');
148+
expect(wire).not.toContain('secret_sys_user');
149+
});
150+
151+
it('a caller who HOLDS the permissions still receives the whole tree (no over-filtering)', async () => {
152+
const { rest } = setup(['sales.admin', 'admin.access']);
153+
const list = appFrom((await getList(rest)).body);
154+
const single = appFrom((await getItem(rest)).body);
155+
156+
for (const app of [list, single]) {
157+
expect(app.areas.map((a: any) => a.id)).toEqual(['area_sales', 'area_admin']);
158+
expect(areaNav(app, 'area_sales')).toEqual(['nav_leads', 'nav_forecast', 'nav_ops_page', 'nav_widget']);
159+
expect(areaNav(app, 'area_admin')).toEqual(['nav_users']);
160+
}
161+
});
162+
163+
it('ADR-0057 D10: a `requiresService` entry inside an area is stripped when the service is absent', async () => {
164+
// Holds every permission — only the capability gate can remove the item.
165+
const { rest } = setup(['sales.admin', 'admin.access'], []);
166+
const list = appFrom((await getList(rest)).body);
167+
const single = appFrom((await getItem(rest)).body);
168+
169+
for (const app of [list, single]) {
170+
expect(areaNav(app, 'area_sales')).toEqual(['nav_leads', 'nav_forecast', 'nav_ops_page']);
171+
expect(JSON.stringify(app)).not.toContain('secret_widget');
172+
}
173+
});
174+
175+
it('the probe covers services named only inside an area — a REGISTERED one is not stripped', async () => {
176+
// The complement of the test above, and the reason
177+
// `resolveRegisteredServices` had to learn to descend into `areas`: a
178+
// name it never probes is absent from the registered set, which the
179+
// gate reads as "service missing" and would strip a live entry.
180+
const { rest } = setup(['sales.admin', 'admin.access'], ['org-scoping']);
181+
const app = appFrom((await getList(rest)).body);
182+
expect(areaNav(app, 'area_sales')).toContain('nav_widget');
183+
});
184+
});

packages/rest/src/rest-server.ts

Lines changed: 55 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1788,9 +1788,21 @@ export class RestServer {
17881788
* - Recursively strips child navigation entries (groups, items) whose
17891789
* `requiredPermissions` are not satisfied. Empty groups collapse so
17901790
* the sidebar doesn't render a label with no children.
1791+
* - [#4722] Applies the SAME item gate to every `areas[].navigation` tree.
1792+
* Both trees are the same shape and the keys mean the same thing in both,
1793+
* so `filterNav` is reused — there is deliberately no second
1794+
* implementation to drift. Before this, an item gated inside an area was
1795+
* enforced by the shell alone: the entry (with its `objectName` /
1796+
* `pageName` / `componentRef` target) still shipped in the `/meta` body,
1797+
* so reading the JSON defeated it.
1798+
*
1799+
* NOT gated here: `visible` (CEL) at any level, and `requiresObject` — both
1800+
* are still evaluated client-side only. That asymmetry is deliberate and
1801+
* pinned in `rest.test.ts`: server-side CEL needs a bound `user` context
1802+
* that this layer does not have, and is its own change.
17911803
*
17921804
* Returns `null` when the app should be hidden from the user. Returns a
1793-
* shallow copy with a filtered `navigation` tree otherwise — the original
1805+
* shallow copy with filtered `navigation` / `areas` otherwise — the original
17941806
* is never mutated so cached metadata stays clean.
17951807
*/
17961808
private filterAppForUser(item: any, sysPerms: Set<string>, serviceGate?: (name: string) => boolean): any | null {
@@ -1821,7 +1833,8 @@ export class RestServer {
18211833
return null;
18221834
}
18231835
const nav = Array.isArray(item.navigation) ? item.navigation : null;
1824-
if (!nav) return item;
1836+
const areas = Array.isArray(item.areas) ? item.areas : null;
1837+
if (!nav && !areas) return item;
18251838

18261839
const filterNav = (entries: any[]): any[] => {
18271840
const out: any[] = [];
@@ -1843,7 +1856,35 @@ export class RestServer {
18431856
return out;
18441857
};
18451858

1846-
return { ...item, navigation: filterNav(nav) };
1859+
// [#4722] `areas[]` carries no gate of its own — the area-level `visible`
1860+
// / `requiredPermissions` keys were retired in 17.0.0 (#4651, ADR-0049)
1861+
// and are NOT revived here. What is enforced is the gate on the items
1862+
// INSIDE an area, through the very same `filterNav` the top-level tree
1863+
// uses, so the two trees can never disagree about what a key means.
1864+
//
1865+
// Collapse rule, taken from what `filterNav` already does to a `group`:
1866+
// an area whose authored tree is emptied BY the gate is dropped (a bare
1867+
// area label with nothing reachable under it is not a useful response),
1868+
// while an area authored empty is passed through untouched — filtering
1869+
// reports what the caller may not see, it does not tidy the metadata.
1870+
const filterAreas = (list: any[]): any[] => {
1871+
const out: any[] = [];
1872+
for (const a of list) {
1873+
if (!a || typeof a !== 'object') continue;
1874+
const anav = Array.isArray(a.navigation) ? a.navigation : null;
1875+
if (!anav || anav.length === 0) { out.push(a); continue; }
1876+
const kids = filterNav(anav);
1877+
if (kids.length === 0) continue;
1878+
out.push({ ...a, navigation: kids });
1879+
}
1880+
return out;
1881+
};
1882+
1883+
return {
1884+
...item,
1885+
...(nav ? { navigation: filterNav(nav) } : {}),
1886+
...(areas ? { areas: filterAreas(areas) } : {}),
1887+
};
18471888
}
18481889

18491890
/**
@@ -1896,11 +1937,17 @@ export class RestServer {
18961937
if (!e || typeof e !== 'object') return;
18971938
if (isMetaEnvelope(e)) { walk((e as any).item); return; }
18981939
if (typeof e.requiresService === 'string') wanted.add(e.requiresService);
1899-
const kids = Array.isArray(e.navigation) ? e.navigation
1900-
: Array.isArray(e.children) ? e.children
1901-
// Dashboard widgets carry their own `requiresService` gate.
1902-
: Array.isArray(e.widgets) ? e.widgets : null;
1903-
if (kids) for (const k of kids) walk(k);
1940+
// [#4722] EVERY child list, not the first one that happens to be an
1941+
// array. An app may carry `navigation` AND `areas` at once, and now
1942+
// that `filterAppForUser` gates the trees under `areas[]` too, a
1943+
// service named only in there must be probed — an unprobed name is
1944+
// absent from `registered`, and the gate would read that as "service
1945+
// missing" and strip a live entry. Fail-closed by omission is still
1946+
// wrong; the probe set must cover exactly what the gate walks.
1947+
for (const key of ['navigation', 'areas', 'children', 'widgets'] as const) {
1948+
const kids = (e as any)[key];
1949+
if (Array.isArray(kids)) for (const k of kids) walk(k);
1950+
}
19041951
};
19051952
for (const it of items) walk(it);
19061953
if (wanted.size === 0) return new Set();

0 commit comments

Comments
 (0)