Skip to content

Commit ea23522

Browse files
committed
fix(formula,lint): current_user 收进 SCOPE_ROOTS,字段级拒绝改为按面的真规则 (#6290)
`@objectstack/formula` 对同一个根说了两套话。`introspectScope` 把 `current_user` 作为合法命名空间交给作者,`checkRoleCatalog` 的四条 position 成员判定正则也全以它打头 —— 两处都对:ADR-0068 D1 把 `current_user` 定为**规范**拼写,`buildScope` 也确实把同一个 `EvalUser` 挂在它下面。只有 `cel-engine.ts` 的 `SCOPE_ROOTS` 不认,于是严格环境把 这个被祝福的拼写读成**裸字段引用**,而它的两个别名(`user`、`ctx`) 一路放行。 三处改动: 1. `SCOPE_ROOTS` 收下 `current_user`。该表是「永不 fault」的基线,不是 逐面契约,现在它宣告的与本包别处宣告的一致。新增行为钉: `introspectScope` 报出的每个根都必须能在严格环境里解析。 2. 删掉错误修法提示。旧拒绝是基线遗漏的副产物,作者拿到的是**通用** 裸字段诊断 ——「Write `record.current_user`」。这个形状在平台的任何 一层都不绑定,照做的作者得到的东西比原来更糟,而且照样静默。字段级 判定现在由 `@objectstack/lint` 里一条自己的规则给出,写明真实失败链 (未绑定 ⇒ fault ⇒ 可见性 fallback 为 `true` ⇒ 本想藏起来的字段对所有 人恒可见,#6146),并给出**真实存在**的处方:把谓词移到选项自己的 `visibleWhen`、在权限集上声明字段级安全 (`fields: { '<object>.<field>': { readable: false } }`)、或改写成 `record` 谓词。覆盖共用同一求值器的 `visibleWhen` / `readonlyWhen` / `requiredWhen`。 3. option 级 `visibleWhen` 首次被校验。`validate-expressions.ts` 走完 字段级条件规则就停了,于是 `SelectOption.visibleWhen` —— 一个客户端 过滤、服务端强制的可授权 CEL 槽 —— 穿过 compile / validate / 运行期 无人校验。裸字段引用、指向不存在字段、语法错误、误用 template 方言 全部静默通过,选项只是从此不再出现。现在按 option value 定位逐条走查, 与宿主字段同为 `record` scope。 两个面**故意**对 `current_user` 给出相反判定,因为求值器不同:字段级走 `evalFieldPredicate`(`record` + `previous` + `parent`,从不绑用户), option 级走 `resolveCascadingOptions`,对宿主 predicate scope 求值,确实 绑定它(ADR-0068 / objectui#2284)。showcase 的角色门控选项 (`'admin' in current_user.positions`)此前从未撞上本规则,现在作为合法 用法被钉住。 Sweep:option 遍历生效后,三个示例应用(`app-showcase` / `app-crm` / `app-todo`)的 `objectstack validate` 全部通过 —— 零新增 finding,包括 同时携带 record 级联与角色门控选项的那个 showcase 对象。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Q7oc7ASjh8yxyS3Yz78We
1 parent 8e13ca8 commit ea23522

5 files changed

Lines changed: 445 additions & 2 deletions

File tree

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
---
2+
"@objectstack/formula": patch
3+
"@objectstack/lint": patch
4+
---
5+
6+
fix(formula,lint): `current_user` becomes a declared root, and its field-level rejection becomes a real rule (#6290)
7+
8+
`@objectstack/formula` told two stories about one root. `introspectScope` handed
9+
`current_user` to authors as a legal namespace and `checkRoleCatalog`'s four
10+
position-membership regexes all lead with it — both correct, because ADR-0068 D1
11+
makes `current_user` THE canonical spelling and `buildScope` really does mount
12+
the same `EvalUser` under it. Only `cel-engine.ts`'s `SCOPE_ROOTS` disagreed, so
13+
the strict environment read the blessed spelling as a BARE FIELD REFERENCE while
14+
its two aliases (`user`, `ctx`) passed unremarked.
15+
16+
Three things change.
17+
18+
**1. `SCOPE_ROOTS` declares `current_user`.** That list is a "never faults"
19+
baseline, not a per-surface contract, and it now advertises exactly what the
20+
package advertises elsewhere. A new pin asserts the property directly: every
21+
root `introspectScope` reports must resolve in the strict env.
22+
23+
**2. The wrong prescription is gone.** Because the rejection used to fall out of
24+
the baseline's omission, the author got the GENERIC bare-field diagnostic —
25+
"Write `record.current_user`". That shape binds on no layer of the platform, so
26+
an author who followed the message ended up with something strictly worse than
27+
what they started with, still silent. The field-level verdict now comes from a
28+
rule of its own in `@objectstack/lint`, which names the real failure (unbound ⇒
29+
fault ⇒ visibility falls back to `true` ⇒ the field a `current_user` test was
30+
meant to hide stays visible for everyone, #6146) and prescribes surfaces that
31+
exist: move the predicate to the option's own `visibleWhen`, declare field-level
32+
security on a permission set (`fields: { '<object>.<field>': { readable: false } }`),
33+
or rewrite it against `record`. It covers `visibleWhen`, `readonlyWhen` and
34+
`requiredWhen`, which share the one evaluator.
35+
36+
**3. Per-option `visibleWhen` is validated at all.** `validate-expressions.ts`
37+
walked field-level conditional rules and stopped there, so `SelectOption.visibleWhen`
38+
— an authorable CEL slot the client filters on AND the server enforces — reached
39+
compile, validate and run time checked by nobody. A bare field reference, a
40+
reference to a field that does not exist, a syntax error or a template-dialect
41+
predicate in an option all shipped in silence, and the option simply never
42+
offered itself. Options are now walked, located by option value, on the same
43+
`record` scope as their host field.
44+
45+
The two surfaces deliberately give opposite verdicts on `current_user`, because
46+
their evaluators differ: field-level rules go through `evalFieldPredicate`
47+
(`record` + `previous` + `parent`, never a user), options through
48+
`resolveCascadingOptions` against the host's predicate scope, which does bind it
49+
(ADR-0068 / objectui#2284). The showcase's role-gated option
50+
(`'admin' in current_user.positions`) had never met this rule before and is now
51+
pinned as the legal usage it is.
52+
53+
Sweep: `objectstack validate` is clean on all three example apps
54+
(`app-showcase`, `app-crm`, `app-todo`) with the option walk active — zero new
55+
findings, including the showcase object that carries both a record-scoped
56+
cascade and the role-gated option.

packages/formula/src/cel-engine.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,40 @@ const SCOPE_ROOTS = [
6666
// (the submit-time snapshot) and `vars`. Declared here so the strict lint
6767
// env doesn't misread `current.x` as a bare field reference.
6868
'current',
69+
// ADR-0068 D1's CANONICAL user root, and the last one this list was missing
70+
// (#6290). `buildScope` mounts the same `EvalUser` object under
71+
// `current_user` / `user` / `ctx.user` / `os.user` whenever the evaluation
72+
// carries a user, and this package already told the rest of the platform so:
73+
// `introspectScope` lists `current_user` among the roots it hands an author,
74+
// and `checkRoleCatalog`'s four position-membership regexes all lead with it.
75+
// Only this list disagreed — so the one spelling ADR-0068 calls canonical was
76+
// the one spelling the strict env read as a BARE FIELD, while its two aliases
77+
// (`user`, `ctx`) passed. One package, two accounts of the same root.
78+
'current_user',
6979
] as const;
7080

81+
/*
82+
* Why widening this list is the safe direction, and where the narrow verdict
83+
* lives instead (#6290).
84+
*
85+
* This list is a "never faults" BASELINE, not a per-surface contract — the
86+
* doc-comment above says so, and every entry is generous by construction. A
87+
* surface that binds a CLOSED set of roots does not express that by hoping the
88+
* baseline omits the others; it says so at the surface, through
89+
* `collectCelRootIdentifiers` (that helper reads the AST and is completely
90+
* independent of this list — see the approval-node approvers in #3447 P2, and
91+
* `@objectstack/lint`'s field-level `*When` gate for `current_user`).
92+
*
93+
* That matters here because field- and section-level `visibleWhen` genuinely do
94+
* NOT bind `current_user` (#6146, measured at both ends: `evalFieldPredicate`
95+
* binds `record` + `previous` + `parent` and nothing else). Before #6290 that
96+
* surface's rejection came out of this list's omission as a SIDE EFFECT, and it
97+
* showed: the diagnostic was the generic bare-field one, so it prescribed
98+
* "Write `record.current_user`" — a shape that binds on no layer at all. A
99+
* verdict that belongs to one surface now reads as that surface's own rule,
100+
* with that surface's own prescription.
101+
*/
102+
71103
/**
72104
* A `record`-scoped environment (`unlistedVariablesAreDyn: false`) for detecting
73105
* bare field references. It reuses the real stdlib so function calls don't fault;

packages/formula/src/validate.test.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { describe, it, expect } from 'vitest';
22
import { validateExpression, introspectScope, expectedDialect, inferExpressionType } from './validate';
3+
import { firstUndeclaredReference } from './cel-engine';
34

45
describe('validateExpression (ADR-0032)', () => {
56
describe('predicates (CEL)', () => {
@@ -343,6 +344,51 @@ describe('validateExpression (ADR-0032)', () => {
343344
expect(scope.roots).toContain('record');
344345
expect(scope.functions).toContain('daysFromNow');
345346
});
347+
348+
/**
349+
* [#6290] The package must give ONE answer about what a root is.
350+
*
351+
* `introspectScope` is the roots list this package HANDS an author (and the
352+
* agent authoring tool); `firstUndeclaredReference` is the strict env that
353+
* JUDGES what an author wrote, off `cel-engine.ts`'s `SCOPE_ROOTS`. Nothing
354+
* kept the two in step, and they had drifted on exactly the root ADR-0068
355+
* D1 calls canonical: `current_user` was advertised here and read as a bare
356+
* field reference there — so the one spelling the ADR blesses was the one
357+
* spelling the validator refused, while its two aliases (`user`, `ctx`)
358+
* passed.
359+
*
360+
* Pinned as behaviour rather than as list equality: `SCOPE_ROOTS` is a
361+
* generous baseline and stays free to declare MORE than it advertises (it
362+
* carries `trigger`, `step`, `parent`, … for sites this introspection does
363+
* not describe). What it may never do again is advertise a root it then
364+
* faults on. Delete `'current_user'` from `SCOPE_ROOTS` and this goes red.
365+
*/
366+
it('every root `introspectScope` advertises really resolves in the strict env (#6290)', () => {
367+
const advertised = introspectScope('predicate').roots;
368+
expect(advertised).toContain('current_user');
369+
const faulting = advertised.filter((root) => firstUndeclaredReference(`${root}.x`) !== null);
370+
expect(faulting).toEqual([]);
371+
});
372+
373+
/**
374+
* [#6290] The same drift, seen from `checkRoleCatalog`'s side: its four
375+
* position-membership regexes accept `current_user` / `user` / `ctx.user`
376+
* as the user subject, so a role-catalog verdict on a `current_user`
377+
* predicate was only ever reachable at sites that do not run the
378+
* `record`-scope bare-ref check. All three spellings now reach it.
379+
*/
380+
it('a role-catalog verdict is reachable through every ADR-0068 user spelling (#6290)', () => {
381+
for (const subject of ['current_user', 'user', 'ctx.user']) {
382+
const r = validateExpression('predicate', `'org_admni' in ${subject}.positions`, {
383+
scope: 'record',
384+
roleCatalog: ['org_admin', 'org_member'],
385+
});
386+
expect(r.ok).toBe(false);
387+
// The role typo is the finding — not a bare reference to the subject.
388+
expect(r.errors.map((e) => e.message).join('\n')).toContain('unknown role `org_admni`');
389+
expect(r.errors.map((e) => e.message).join('\n')).not.toContain('bare reference');
390+
}
391+
});
346392
});
347393
});
348394

0 commit comments

Comments
 (0)