From 0d782197fe899ffc03f2089d1362c762b5b7aca6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 13:16:02 +0000 Subject: [PATCH] =?UTF-8?q?fix(lint):=20=E5=AD=97=E6=AE=B5=E7=BA=A7=20*Whe?= =?UTF-8?q?n=20=E7=9A=84=E7=94=A8=E6=88=B7=E6=A0=B9=E6=8B=92=E7=BB=9D?= =?UTF-8?q?=E8=A6=86=E7=9B=96=20ADR-0068=20=E7=9A=84=E5=85=A8=E9=83=A8?= =?UTF-8?q?=E4=B8=89=E7=A7=8D=E6=8B=BC=E5=86=99=20(#6585)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #6290(PR #6584)给字段级 `visibleWhen` / `readonlyWhen` / `requiredWhen` 加了一条按面的用户根拒绝,处方也指向真实存在的面。但它只匹配 `current_user` 一个拼写,而 ADR-0068 D1 把 `user` / `ctx.user` 定为**同一个对象的别名** (`buildScope` 把同一个 `EvalUser` 引用挂在四个名字下)。于是同一个语义错误 在一种拼写下报错、在另外两种下**完全静默** —— 两者一直在 `SCOPE_ROOTS` 里, 裸引用检查也从不报它们。作者随手挑了三个 ADR 认定等价的拼写里的哪一个, 决定了他拿不拿得到构建期诊断。 失败方向就是 #6146 那个:未绑定根 ⇒ fault ⇒ 可见性 fallback 为 `true`, 本想按角色藏起来的字段对所有人恒可见,且无任何信号。 三个根现在共用一条判定、一条处方、一条文案,只有文案里点名的那个根不同。 选项级不受影响:per-option `visibleWhen` 走宿主谓词作用域,那里每种拼写都 绑定用户,所以 showcase 的角色门控选项在别名下同样合法(已钉)。 **`ctx` 按整根判,而不是只判 `ctx.user` 形态。** 在这个面上这就是事实: `buildScope` 只在求值携带用户时才创建 `ctx` 根,而字段级没有任何调用点传 用户 —— 服务端绑 `record` + `previous`(+ `parent`),客户端 `evalFieldPredicate` 绑 `record` + `previous` + 调用方 scope,而后者实测 只可能是 `{ parent }`。所以 `ctx.locale` 与 `ctx.user.id` 在这里同样 fault。 否掉窄读法的理由:它需要源码层面的拼写匹配,而拼写匹配正是本条规则要消灭 的东西 —— 那会把同一个分岔下移一层(`ctx["user"].id` 静默、`ctx.user.id` 报错),同时为了一个更窄的规则**名字**放掉一个真实的 fail-open fault。 `ctx` 在别处仍是 ActionEngine 的谓词根,不受影响:平台自己的 `ctx.user` 谓词全在 action `visible` 上(`sys-user.object.ts`、`sys-invitation.object.ts`), 本规则从不读那个面,该放行已钉为测试。 实测扫描:字段级 `*When` 读取任一用户根的用例,在 `examples/`、`packages/` 与下游 `objectui` 仓中均为**零** —— 按槽位名扫一遍、再按别名拼写本身扫 一遍(防止带逗号的谓词藏起来),两遍都是零。三个示例 app 的 `objectstack validate` 全绿,无新增发现。 反向验证(方向在运行前已预判):把根集合改回 `['current_user']`,新增用例 中恰好 9 条转红,失败形态是 `expected [] to have a length of 1 but got +0` —— 即完全静默,正是本单描述的缺陷;三条接受类钉子(选项级别名、action `ctx.user`、`record` 成员同名)在两个版本下都绿,因为它们断言的是零发现。 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AZgRyPVwi1jLb1mNNuUQ9o --- .changeset/field-when-user-root-aliases.md | 45 +++++ .../lint/src/validate-expressions.test.ts | 164 ++++++++++++++++++ packages/lint/src/validate-expressions.ts | 58 ++++++- 3 files changed, 260 insertions(+), 7 deletions(-) create mode 100644 .changeset/field-when-user-root-aliases.md diff --git a/.changeset/field-when-user-root-aliases.md b/.changeset/field-when-user-root-aliases.md new file mode 100644 index 0000000000..805e869e58 --- /dev/null +++ b/.changeset/field-when-user-root-aliases.md @@ -0,0 +1,45 @@ +--- +"@objectstack/lint": patch +--- + +fix(lint): the field-level user-root rejection covers all three ADR-0068 spellings, not just `current_user` (#6585) + +#6290 gave field-level `visibleWhen` / `readonlyWhen` / `requiredWhen` a +surface-level rejection when the predicate reaches for the signed-in user, with +a prescription that names surfaces which actually bind one. That check matched a +single spelling — `current_user` — while ADR-0068 D1 makes `user` and `ctx.user` +**the same object under different names**: `buildScope` hangs one `EvalUser` +reference on `current_user` / `user` / `ctx.user` / `os.user`. So the identical +semantic error produced an error under one spelling and **total silence** under +the other two (both have always been in `SCOPE_ROOTS`, so the bare-reference +check never fired on them either). Which of three ADR-equivalent spellings the +author happened to pick decided whether they got a build-time diagnostic at all. + +The failure direction is the one #6146 named: an unbound root faults, the fault +falls back, and visibility's fallback is `true` — so a predicate written to HIDE +a field by role left it visible to everyone, silently. + +All three roots now share one verdict, one prescription and one message; only +the root named in the message varies. Nothing about the option level changes: +per-option `visibleWhen` resolves against the host's predicate scope, which +binds the user under every spelling, so the showcase's role-gated option +(`'admin' in current_user.positions`) stays legal — under the aliases too. + +**`ctx` is judged as a whole root, not only in `ctx.user` form.** At this +surface that is simply what is true: `buildScope` creates the `ctx` root *only* +when the evaluation carries a user, and no field-level site passes one — the +server binds `record` + `previous` (+ `parent`) and the client's +`evalFieldPredicate` binds `record` + `previous` + a caller scope that is only +ever `{ parent }`. `ctx.locale` therefore faults exactly like `ctx.user.id` +here. The narrower reading was rejected because it needs a source-level spelling +match, which would re-open this very fork one level down (`ctx["user"].id` +silent, `ctx.user.id` rejected) while leaving a real fail-open fault +unreported. `ctx` remains ActionEngine's predicate root elsewhere and is +untouched there — the platform's own `ctx.user` predicates all sit on action +`visible` (`sys-user.object.ts`, `sys-invitation.object.ts`), a surface this +rule never reads, and that acceptance is pinned. + +Sweep: field-level `*When` predicates reading any user root measure **zero** +across `examples/`, `packages/` and the downstream `objectui` repo, by both a +slot-keyed scan and an alias-keyed one — so no shipping metadata is refused by +the widening. `objectstack validate` stays clean on all three example apps. diff --git a/packages/lint/src/validate-expressions.test.ts b/packages/lint/src/validate-expressions.test.ts index 2ae16de090..c6d981eca6 100644 --- a/packages/lint/src/validate-expressions.test.ts +++ b/packages/lint/src/validate-expressions.test.ts @@ -751,6 +751,170 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => { }); }); + /** + * ── The ADR-0068 aliases get the SAME field-level verdict (#6585) ──────── + * + * D1 makes `user` and `ctx.user` aliases of `current_user` — `buildScope` + * hangs one `EvalUser` reference on all three spellings — so the semantic + * error above is the same error under any of them. #6584's first cut + * matched only the canonical spelling: `'admin' in user.positions` and + * `'admin' in ctx.user.positions` sailed through in silence (both roots + * have always been in `SCOPE_ROOTS`, so the bare-ref check never fired + * either), and which spelling the author picked decided whether they got a + * diagnostic. These tests close that fork and pin its edges. + * + * `ctx` is pinned WHOLE-ROOT deliberately: at field level `buildScope` + * never creates `ctx` at all (it exists only when the evaluation carries a + * user, which no field-level site passes), so `ctx.locale` faults exactly + * like `ctx.user.id`. The other side of that decision is pinned too — + * `ctx.user` on an ACTION `visible` (ActionEngine's surface, where `ctx` + * genuinely binds — the platform's own `sys_user` actions ship it) must + * stay accepted. + */ + describe('`user` / `ctx.user` aliases at field level (#6585)', () => { + const slots = ['visibleWhen', 'readonlyWhen', 'requiredWhen'] as const; + + it.each(slots)('rejects `user` on %s — same object as `current_user`, same unbound surface', (slot) => { + const issues = validateStackExpressions({ + objects: [{ + name: 'showcase_deal', + fields: { amount: { type: 'number', [slot]: "'admin' in user.positions" } }, + }], + }); + const hit = issues.filter((i) => i.where === `object 'showcase_deal' · field 'amount' ${slot}`); + expect(hit).toHaveLength(1); + expect(hit[0]!.severity).toBe('error'); + // The message names the spelling the author WROTE — a diagnostic that + // talks about `current_user` to an author who typed `user` sends them + // hunting for text that is not in their file. + expect(hit[0]!.message).toMatch(/`\w+` reads `user`/); + }); + + it.each(slots)('rejects `ctx.user` on %s', (slot) => { + const issues = validateStackExpressions({ + objects: [{ + name: 'showcase_deal', + fields: { amount: { type: 'number', [slot]: "'admin' in ctx.user.positions" } }, + }], + }); + const hit = issues.filter((i) => i.where === `object 'showcase_deal' · field 'amount' ${slot}`); + expect(hit).toHaveLength(1); + expect(hit[0]!.severity).toBe('error'); + expect(hit[0]!.message).toMatch(/`\w+` reads `ctx`/); + }); + + /** + * The whole-root half of the `ctx` decision: a `ctx` read that never + * touches `.user` is just as unbound at field level — `buildScope` only + * creates the root when a user is carried, and no field-level site + * carries one — so it must not slip through a `.user`-form-only match. + */ + it('rejects a bare-`ctx` NON-user read too — the root itself is unbound at field level', () => { + const issues = validateStackExpressions({ + objects: [{ + name: 'showcase_deal', + fields: { amount: { type: 'number', visibleWhen: "ctx.locale == 'en'" } }, + }], + }); + const hit = issues.filter((i) => i.where.includes("field 'amount' visibleWhen")); + expect(hit).toHaveLength(1); + expect(hit[0]!.message).toMatch(/`visibleWhen` reads `ctx`/); + }); + + it.each(["'admin' in user.positions", "'admin' in ctx.user.positions"])( + 'gives %s the SAME prescriptions as the canonical spelling — no per-spelling fork', + (predicate) => { + const [issue] = validateStackExpressions({ + objects: [{ + name: 'showcase_deal', + fields: { amount: { type: 'number', visibleWhen: predicate } }, + }], + }).filter((i) => i.where.includes('visibleWhen')); + // Same three prescriptions the #6290 message test pins for + // `current_user`, plus the same direction-of-failure sentence. + expect(issue!.message).toMatch(/falls back to VISIBLE/); + expect(issue!.message).toMatch(/option's own `visibleWhen`/); + expect(issue!.message).toMatch(/readable: false/); + expect(issue!.message).not.toContain('record.current_user'); + }, + ); + + /** + * The whole-root widening makes root-vs-MEMBER discrimination newly + * load-bearing: `record.user_id` and `record.ctx_key` name the very + * strings this rule now rejects, but as MEMBERS of `record` — and + * `collectCelRootIdentifiers` drops member names by design. A rule that + * confused the two would reject the single most ordinary predicate an + * author writes (an owner check), which is the failure mode that would + * make this widening worse than the hole it closes. + */ + it('does NOT trip on a `record` MEMBER merely spelled like one of the roots', () => { + const issues = validateStackExpressions({ + objects: [{ + name: 'showcase_deal', + fields: { + user_id: { type: 'text' }, + ctx_key: { type: 'text' }, + amount: { + type: 'number', + visibleWhen: "record.user_id != '' && record.ctx_key == 'x'", + }, + }, + }], + }); + expect(issues).toHaveLength(0); + }); + + /** + * The widening is FIELD-level only. Option-level `visibleWhen` resolves + * against the host predicate scope, where `buildScope` mounts the SAME + * user object under every ADR-0068 spelling — so an option predicate is + * legal under the aliases exactly as it is under `current_user`. + */ + it('still ACCEPTS an option-level `visibleWhen` spelling the `user` alias', () => { + const issues = validateStackExpressions({ + objects: [{ + name: 'showcase_cascading_select', + fields: { + tier: { + type: 'select', + options: [ + { label: 'Standard', value: 'standard', default: true }, + { label: 'Restricted', value: 'restricted', visibleWhen: "'admin' in user.positions" }, + ], + }, + }, + }], + }); + expect(issues).toHaveLength(0); + }); + + /** + * The blast-radius pin the #6585 measurement was for: `ctx` IS + * ActionEngine's predicate root, and the platform's own metadata ships + * `ctx.user` action predicates (`sys-user.object.ts` "visible: + * record.id == ctx.user.id", `sys-invitation.object.ts`). The field-level + * rejection must not leak onto the action surface. + */ + it('still ACCEPTS `ctx.user` on an action `visible` — ActionEngine binds `ctx`', () => { + const issues = validateStackExpressions({ + objects: [{ + name: 'sys_user', + fields: { email: { type: 'text' } }, + actions: [{ + // The exact shape `packages/platform-objects/src/identity/ + // sys-user.object.ts:291` ships (`id` is a registry-injected + // column, so the field-existence pass resolves it too). + name: 'change_password', + type: 'script', + visible: 'record.id == ctx.user.id', + }], + }], + }); + expect(issues).toHaveLength(0); + }); + }); + /** * ── The option-level traversal itself (#6290 half 3) ──────────────────── * diff --git a/packages/lint/src/validate-expressions.ts b/packages/lint/src/validate-expressions.ts index 7d7688ad6e..3673b5d6b2 100644 --- a/packages/lint/src/validate-expressions.ts +++ b/packages/lint/src/validate-expressions.ts @@ -393,10 +393,14 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] { /** * A FIELD-level conditional rule (`visibleWhen` / `readonlyWhen` / - * `requiredWhen`) that reaches for `current_user` — the one root the field - * level does not bind (#6146, measured at both ends: `evalFieldPredicate` / - * `resolveFieldRuleState` bind `record` + `previous` + `parent` and nothing - * else, and objectui#1582's authoring autocomplete pins the same three). + * `requiredWhen`) that reaches for the signed-in user — under its canonical + * spelling `current_user` or either of its ADR-0068 D1 aliases, `user` and + * `ctx.user` — the one thing the field level does not bind (#6146, measured + * at both ends: `evalFieldPredicate` / `resolveFieldRuleState` bind `record` + * + `previous` + `parent` and nothing else, and objectui#1582's authoring + * autocomplete pins the same three). The third root is matched WHOLE (any + * `ctx` read, not only `ctx.user`) — see "Why THREE roots" below for the + * measurement that decides it. * * ## Why this is a rule of its own rather than a missing root * @@ -430,18 +434,58 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] { * options resolve against the host's predicate scope, which binds * `current_user` (ADR-0068 / objectui#2284) — that surface is where such a * predicate belongs, which is why it is also the first prescription below. + * + * ## Why THREE roots, and why `ctx` is judged whole-root (#6585) + * + * ADR-0068 D1 makes `user` and `ctx.user` ALIASES of `current_user` — one + * `EvalUser` object under every spelling (`buildScope` in + * `formula/stdlib.ts` hangs the same reference on `current_user` / `user` / + * `ctx.user` / `os.user`). Matching only the canonical spelling meant the + * identical semantic error got a diagnostic under `current_user` and total + * silence under the aliases — which spelling the author picked decided + * whether they got the diagnostic, the exact fork AI authors cannot + * self-check. All three roots now share one verdict and one prescription; + * the message names the spelling found, nothing else varies. + * + * `ctx` is judged as a WHOLE root, not only in `ctx.user` form, because at + * this surface that is simply what is true: `buildScope` creates the `ctx` + * root ONLY when the evaluation carries a user (`scope.ctx = { user }` + * inside `if (ctx.user !== undefined)`), and no field-level site passes one + * — the server binds `record`+`previous`(+`parent`) (`rule-validator.ts` + * `readonlyWhenBindings` / the `requiredWhen` block) and the client's + * `evalFieldPredicate` binds `record`+`previous`+caller `scope` (only ever + * `{ parent }`). So `ctx.locale` faults exactly like `ctx.user.id` here. + * `ctx` IS ActionEngine's predicate root elsewhere — the platform's real + * `ctx.user` predicates all sit on action `visible` (`sys-user.object.ts`, + * `sys-invitation.object.ts`), a surface this helper never reads — and + * measured usage of field-level `*When` with ANY user root is zero across + * examples/, packages/ and objectui (#6585's sweep). + * + * The rejected alternative was matching `ctx` only in its `ctx.user` form. + * `collectCelRootIdentifiers` reports ROOTS and drops member names by + * design, so that reading needs a source-level spelling match — and a + * spelling match is precisely the defect this rule exists to remove: it + * would re-open the same fork one level down (`ctx["user"].id` silent, + * `ctx.user.id` rejected), while leaving a real fail-open fault (`ctx.locale`) + * unreported for the sake of a narrower rule NAME. */ + const FIELD_UNBOUND_USER_ROOTS = ['current_user', 'user', 'ctx'] as const; const checkFieldRuleUserRoot = (where: string, slot: string, raw: unknown): void => { const source = celSourceOf(raw); if (!source) return; const roots = collectCelRootIdentifiers(source); - if (!roots.ok || !roots.roots.includes('current_user')) return; + if (!roots.ok) return; + // One issue per slot even when a predicate reaches for two of them; the + // tie-break is this list's order (canonical spelling first), so the message + // is stable rather than dependent on AST walk order. + const root = FIELD_UNBOUND_USER_ROOTS.find((r) => roots.roots.includes(r)); + if (root === undefined) return; issues.push({ where, message: - `\`${slot}\` reads \`current_user\`, but a field-level conditional rule binds only ` + + `\`${slot}\` reads \`${root}\`, but a field-level conditional rule binds only ` + `\`record\` (plus \`previous\`, and \`parent\` on a master-detail line item) — ` + - `\`current_user\` is unbound here, so the predicate faults and falls back to VISIBLE, ` + + `\`${root}\` is unbound here, so the predicate faults and falls back to VISIBLE, ` + `leaving the field the test was meant to hide showing for everyone (#6146). ` + `To gate the CHOICES of a select by user, move the predicate to the option's own ` + `\`visibleWhen\` (\`options: [{ …, visibleWhen: … }]\`) — per-option is the one \`*When\` ` +