From 2e9bb582da56696981a2beb7d1c2300906538ab8 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 23:38:19 +0000 Subject: [PATCH 1/2] =?UTF-8?q?fix(spec):=20correct=20the=20area=20prescri?= =?UTF-8?q?ption=20after=20#4722=20=E2=80=94=20item=20gates=20are=20stripp?= =?UTF-8?q?ed=20in=20BOTH=20trees=20(#4749)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `AREA_REQUIRED_PERMISSIONS_RETIRED` is the only text an author who wrote an area-level gating key ever reads (it is the strict schema's unknown-key error body). It still claimed: 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 closed that gap: `filterAppForUser` now runs the same `filterNav` over every `areas[].navigation`, so a navigation ITEM's `requiredPermissions` / `requiresService` is enforced server-side in both trees and a gated entry never ships in the `/meta` body. The stale wording erred toward over-caution rather than danger — it sent authors to the top-level tree, which still works — but it was wrong in the one copy written FOR the author. The rewritten prescription states the current fact and keeps the half #4722 did NOT change: `visible` (CEL) and `requiresObject` are still evaluated client-side only at every level, so a must-never-ship gate goes in `requiredPermissions`, never in `visible`. That asymmetry gets newly tempting to misread once areas are server-gated, so it is spelled out and pinned. The retirement itself is untouched: what is enforced is the items INSIDE an area, not a revived area-LEVEL key (#4651 stands). - app.test.ts: the pin moves to the corrected wording rather than being deleted — a prescription that merely goes quiet about `areas[]` leaves the author believing the old boundary. Adds a negative assertion on the retired claim. - app.zod.ts: the JSDoc above these two constants asserted the same expired fact in the present tense; re-tensed as history plus one paragraph recording #4722, so the file does not contradict its own prescription. Generated artifacts: `check:generated` reports all 9 green — this prose lives only in a Zod error string and is projected into no generated artifact. Fixes #4749 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01FTszibd6C8sUCCZnM4VcrL --- .changeset/area-prescription-both-trees.md | 32 +++++++++++++++++++ packages/spec/src/ui/app.test.ts | 27 +++++++++++++--- packages/spec/src/ui/app.zod.ts | 36 +++++++++++++++------- 3 files changed, 80 insertions(+), 15 deletions(-) create mode 100644 .changeset/area-prescription-both-trees.md diff --git a/.changeset/area-prescription-both-trees.md b/.changeset/area-prescription-both-trees.md new file mode 100644 index 0000000000..05f3120745 --- /dev/null +++ b/.changeset/area-prescription-both-trees.md @@ -0,0 +1,32 @@ +--- +'@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 **内部的项**,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..09037d3a0b 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', () => { diff --git a/packages/spec/src/ui/app.zod.ts b/packages/spec/src/ui/app.zod.ts index 4095617917..afd76408e1 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 ' @@ -686,10 +697,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 From 23ace26903cc46cad92ce1e58d4a10fbc68df50d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 23:52:02 +0000 Subject: [PATCH 2/2] =?UTF-8?q?fix(spec):=20fold=20the=20sibling=20prescri?= =?UTF-8?q?ption=20in=20=E2=80=94=20AREA=5FVISIBLE=5FRETIRED's=20enumerati?= =?UTF-8?q?on=20also=20missed=20areas[]=20(#4749)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PM review widened this issue's file surface, correctly: #4749's body already listed "confirm AREA_VISIBLE_RETIRED" as in-scope work, and the confirmation came back needing a change rather than clearing it. `AREA_VISIBLE_RETIRED` closed by naming where a SERVER-enforced gate may live: use `requiredPermissions`: on the app itself, or on items of the app's top-level `navigation` tree. Nothing there is false — both named layers are enforced — but after #4722 it is an ENUMERATION missing its third entry, and the omission does the same damage as the sentence fixed in the previous commit: an author already standing inside an area is sent off to restructure their navigation tree for a gate they could now write in place. Both destinations are now named. What deliberately did NOT change is `visible`'s own verdict. #4722 touched `requiredPermissions` / `requiresService` and nothing else, so item-level `visible` is still CEL evaluated in the browser at every level. The prescription now states the division of labour outright — `visible` hides an entry that has already been sent, `requiredPermissions` stops it being served — because the author reaching this message is holding a CEL expression, which makes "just move it to the item's `visible`" the nearest and worst destination available. The retirement itself still stands: the area-LEVEL keys stay retired, and no wording here may be read as reviving them. - app.test.ts: same discipline as the first pin — re-nailed onto the corrected enumeration (`either navigation tree`, `areas[].navigation`, `#4722`), plus a positive pin on the surviving CEL semantics and a negative pin on the enumeration this replaced. - Changeset: extended the existing `.changeset/area-prescription-both-trees.md` rather than adding a second file. Out of scope by review decision and left on #5337: the protocol-17 migration rationale and its generated upgrade-guide projection, which need `gen:upgrade-guide` and an os-regen-driven artifact. Fixes #4749 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01FTszibd6C8sUCCZnM4VcrL --- .changeset/area-prescription-both-trees.md | 15 +++++++++++++-- packages/spec/src/ui/app.test.ts | 19 +++++++++++++++++++ packages/spec/src/ui/app.zod.ts | 9 ++++++--- 3 files changed, 38 insertions(+), 5 deletions(-) diff --git a/.changeset/area-prescription-both-trees.md b/.changeset/area-prescription-both-trees.md index 05f3120745..62440dcd2c 100644 --- a/.changeset/area-prescription-both-trees.md +++ b/.changeset/area-prescription-both-trees.md @@ -25,8 +25,19 @@ schema 的 unknown-key 报错正文)。它此前写着: 绑定上下文)。所以「必须永不到达浏览器」的东西写 `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 过期的 -说明性注释。 +行为零变化 —— 改的是两条报错正文与它们的 pin 断言,以及 `app.zod.ts` 里同一段已随 #4722 +过期的说明性注释。 diff --git a/packages/spec/src/ui/app.test.ts b/packages/spec/src/ui/app.test.ts index 09037d3a0b..b9712ee493 100644 --- a/packages/spec/src/ui/app.test.ts +++ b/packages/spec/src/ui/app.test.ts @@ -1463,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 afd76408e1..e146bf38e8 100644 --- a/packages/spec/src/ui/app.zod.ts +++ b/packages/spec/src/ui/app.zod.ts @@ -686,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) — '