diff --git a/.changeset/area-prescription-both-trees.md b/.changeset/area-prescription-both-trees.md new file mode 100644 index 0000000000..62440dcd2c --- /dev/null +++ b/.changeset/area-prescription-both-trees.md @@ -0,0 +1,43 @@ +--- +'@objectstack/spec': patch +--- + +fix(spec): `areas[].requiredPermissions` 的退役处方改口径 —— 项级闸门在**两棵树**都由服务端剥离(#4749) + +`AREA_REQUIRED_PERMISSIONS_RETIRED` 是作者写错 area 级键时唯一能读到的文字(strict +schema 的 unknown-key 报错正文)。它此前写着: + +> Items nested under `areas[]` are gated in the shell only — the server does not +> walk `areas` — so anything that must never reach the browser belongs in the +> top-level tree, or in its own app. + +这句话在 #4722 之后已经过时。`filterAppForUser` 现在对每一棵 `areas[].navigation` +跑**同一个** `filterNav`,所以导航**项**上的 `requiredPermissions` / `requiresService` +在顶层树和 area 内部被同等强制,被闸住的条目(连同它的 `objectName` / `pageName` / +`componentRef` 目标)根本不会进入 `/meta` 响应体。 + +方向上这是「过度保守」而非不安全 —— 它劝作者把敏感项挪到顶层树,那仍然可行 —— 但它错了, +而且错在**说给作者听的**那一份上:一个本可以就地把 `requiredPermissions` 写在 area 内部项 +上的作者,会被这段话劝去重构导航树。 + +处方正文改后陈述当前事实,并保住 #4722 **没有**改变的那一半非对称性:`visible`(CEL)与 +`requiresObject` 在任何层级**依然只在客户端求值**(服务端跑 CEL 需要读层没有的 `user` +绑定上下文)。所以「必须永不到达浏览器」的东西写 `requiredPermissions`,不要写 `visible` +—— 这一条在 #4722 之后反而更容易被误读,因此写进了正文并单独钉了 pin。 + +同一次改口径也覆盖姊妹处方 `AREA_VISIBLE_RETIRED`(作者写错 `areas[].visible` 时读到的那 +份)。它此前把服务端强制的落点枚举成「on the app itself, or on items of the app's top-level +`navigation` tree」—— 不是假话,但 #4722 之后这份**枚举**漏了 `areas[].navigation` 的项, +效果与上面那句相同:一个已经站在 area 内部、本可以就地把闸门写在该 area 项上的作者,被劝去 +重构导航树。现在两处落点都列全。 + +⚠️ 这条**没有**改变 `visible` 自身的口径:项级 `visible` 依然是 CEL、依然只在浏览器里求值, +#4722 没有碰这一半。正文里把两者的分工写死 —— `visible` 隐藏的是**已经发出去**的条目, +`requiredPermissions` 才让条目根本不被发出 —— 因为作者是攥着一个 CEL 表达式走到这条报错前 +的,「就近改写成项级 `visible`」正是此刻最顺手也最危险的那个落点。 + +退役裁决本身不动:被强制的是 area **内部的项**,area **级**键(`areas[].visible` / +`areas[].requiredPermissions`,#4651)保持退役,没有复活。 + +行为零变化 —— 改的是两条报错正文与它们的 pin 断言,以及 `app.zod.ts` 里同一段已随 #4722 +过期的说明性注释。 diff --git a/packages/spec/src/ui/app.test.ts b/packages/spec/src/ui/app.test.ts index 53d5db66f5..b9712ee493 100644 --- a/packages/spec/src/ui/app.test.ts +++ b/packages/spec/src/ui/app.test.ts @@ -1433,10 +1433,29 @@ describe('unknown keys are rejected, not stripped (#4001 PR B)', () => { // The two enforced layers, named. expect(msg).toMatch(/on the APP/s); expect(msg).toMatch(/navigation ITEM/s); - // The honest caveat: the server does not walk areas, so an item gate - // INSIDE an area is shell-side only. Without this the prescription would - // trade one false belief for a weaker one. - expect(msg).toMatch(/shell only|does not walk/is); + // #4722 closed the caveat this prescription used to carry ("the server + // does not walk `areas`", so an item gate inside an area was shell-side + // only). The pin moves to the CORRECTED fact rather than being deleted: + // an author who reads a prescription that has simply gone quiet about + // `areas[]` concludes the old boundary still holds, which is the same + // false belief one step further from the evidence. + expect(msg).toMatch(/BOTH trees/s); + expect(msg).toMatch(/areas\[\]\.navigation/s); + expect(msg).toMatch(/#4722/s); + // The asymmetry that SURVIVES #4722 — and the reason this is the layer + // an author must reach for: `requiredPermissions` is enforced before the + // body ships, `visible` is not evaluated until it is already in the + // browser. Dropping this half would trade the old false belief for the + // one #4722 makes newly tempting ("areas are gated now, `visible` is + // fine"). + expect(msg).toMatch(/client-side ONLY/s); + expect(msg).toMatch(/`visible` \(CEL\)/s); + expect(msg).toMatch(/never in `visible`/s); + // The retirement itself is untouched: enforcing the items inside an area + // is not a revived area-LEVEL gate, and the message must not read as one. + expect(msg).toMatch(/not revived/s); + // Belt and braces on the specific claim this pin replaced. + expect(msg).not.toMatch(/does not walk/i); }); it('rejects `areas[].visible` and points at the item-level CEL gate that is evaluated', () => { @@ -1444,6 +1463,25 @@ describe('unknown keys are rejected, not stripped (#4001 PR B)', () => { expect(msg).toMatch(/visible.*removed.*17\.0\.0/s); expect(msg).toMatch(/fails open|EVERYONE/s); expect(msg).toMatch(/navigation ITEM's `visible`/s); + // #4722 also moved where a SERVER-enforced gate may live, and this + // prescription's closing enumeration had not followed. It used to name + // the top-level tree as the only item-level destination, which sends an + // author already standing inside an area off to restructure their + // navigation for a gate they can now write in place. Pin the corrected + // enumeration rather than merely the absence of the old one. + expect(msg).toMatch(/either navigation tree/s); + expect(msg).toMatch(/areas\[\]\.navigation/s); + expect(msg).toMatch(/#4722/s); + // …and do NOT let that drag `visible`'s own verdict along: #4722 changed + // nothing about CEL, which is still evaluated in the browser at every + // level. That asymmetry is the entire reason this prescription can send a + // must-never-ship gate to `requiredPermissions` rather than to an item's + // `visible` — the destination it would otherwise be natural to reach for, + // since the author arrived here holding a CEL expression. + expect(msg).toMatch(/evaluated in the browser/s); + expect(msg).toMatch(/navigation ITEM's `visible` takes the same CEL expression/s); + // Negative pin on the enumeration this replaced. + expect(msg).not.toMatch(/on items of the app's top-level/s); }); it('routes the retired gating aliases to the same prescriptions', () => { diff --git a/packages/spec/src/ui/app.zod.ts b/packages/spec/src/ui/app.zod.ts index 4095617917..e146bf38e8 100644 --- a/packages/spec/src/ui/app.zod.ts +++ b/packages/spec/src/ui/app.zod.ts @@ -647,13 +647,13 @@ const AREA_ORDER_RETIRED = * 17.0.0 (#4651, ADR-0049). * * These were not ordinary dead keys. They were **fail-open capability gates**: - * the authoritative server-side filter (`filterAppForUser`, - * `packages/rest/src/rest-server.ts`) reads the app's `requiredPermissions` and - * then walks ONLY `item.navigation` — it returns early when that tree is - * absent and never touches `item.areas` at all — while the client renders every - * area in the switcher. So an author who wrote `requiredPermissions: - * ['sales.admin']` on an area got a clean parse, a stored value, and an area - * visible to everyone. + * at the time of the retirement the authoritative server-side filter + * (`filterAppForUser`, `packages/rest/src/rest-server.ts`) read the app's + * `requiredPermissions` and then walked ONLY `item.navigation` — it returned + * early when that tree was absent and never touched `item.areas` at all — while + * the client rendered every area in the switcher. So an author who wrote + * `requiredPermissions: ['sales.admin']` on an area got a clean parse, a stored + * value, and an area visible to everyone. * * What made them read alive is that the SAME key names are genuinely enforced * one level up and one level down: app-level `requiredPermissions` drops the @@ -668,6 +668,17 @@ const AREA_ORDER_RETIRED = * remove its items everywhere? does the server bind `user` for area CEL?), and * a retirement PR must not invent an authorization mechanism. Removing a gate * that never gated is strictly safer than shipping a major with it in place. + * + * SINCE #4722 the paragraph above is history on one point, and the prescription + * below states the current fact: `filterAppForUser` now runs the same + * `filterNav` over every `areas[].navigation`, so an ITEM's + * `requiredPermissions` / `requiresService` is enforced server-side in both + * trees and a gated entry never ships in the `/meta` body. That closed the + * shell-only boundary these prescriptions used to warn about; it did NOT revive + * the area-LEVEL keys, which stay retired. `visible` (CEL) and `requiresObject` + * remain client-side only at every level — server-side CEL needs a bound `user` + * context the read layer does not have. Mirrored in `liveness/app.json` + * (`areas.navigation`) and pinned in `packages/rest/src/rest.test.ts`. */ const AREA_VISIBLE_RETIRED = '`areas[].visible` was removed in @objectstack/spec 17.0.0 (#4651, ADR-0049) — nothing ever ' @@ -675,9 +686,12 @@ const AREA_VISIBLE_RETIRED = + 'gate that fails open, which is worse than no gate at all. Delete the key and gate the ' + 'items INSIDE the area — a navigation ITEM\'s `visible` takes the same CEL expression and ' + 'IS evaluated per item by the shell. For a gate the SERVER enforces, use ' - + '`requiredPermissions`: on the app itself, or on items of the app\'s top-level ' - + '`navigation` tree. Run `os migrate meta --from 16` to rewrite existing sources ' - + 'automatically.'; + + '`requiredPermissions` instead: on the app itself, or on the ITEMS of either navigation ' + + 'tree — the app\'s top-level `navigation` AND every `areas[].navigation`, both stripped ' + + 'server-side since #4722. The distinction survives at every level: `visible` is CEL ' + + 'evaluated in the browser, so it hides an entry that has already been sent, while ' + + '`requiredPermissions` stops that entry from being served at all. Run ' + + '`os migrate meta --from 16` to rewrite existing sources automatically.'; const AREA_REQUIRED_PERMISSIONS_RETIRED = '`areas[].requiredPermissions` was removed in @objectstack/spec 17.0.0 (#4651, ADR-0049) — ' @@ -686,10 +700,13 @@ const AREA_REQUIRED_PERMISSIONS_RETIRED = + 'gate to a layer that is actually enforced. `requiredPermissions` on the APP is checked ' + 'server-side (the app is dropped from /meta entirely for a caller who lacks them), and ' + '`requiredPermissions` / `requiresService` on a navigation ITEM are stripped server-side ' - + 'from the app\'s top-level `navigation` tree and re-checked in the shell. Items nested ' - + 'under `areas[]` are gated in the shell only — the server does not walk `areas` — so ' - + 'anything that must never reach the browser belongs in the top-level tree, or in its own ' - + 'app. Run `os migrate meta --from 16` to rewrite existing sources automatically.'; + + 'in BOTH trees — the app\'s top-level `navigation` AND every `areas[].navigation`, through ' + + 'the same filter since #4722 — then re-checked in the shell, so an item gated inside an ' + + 'area never reaches the browser either. That enforces the items INSIDE an area; the ' + + 'area-level key is not revived. Still evaluated client-side ONLY, at every level: ' + + '`visible` (CEL) and `requiresObject` — so anything that must never reach the browser ' + + 'goes in `requiredPermissions`, never in `visible`. Run `os migrate meta --from 16` to ' + + 'rewrite existing sources automatically.'; /** * Navigation Area Schema