Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions .changeset/field-when-user-root-aliases.md
Original file line number Diff line number Diff line change
@@ -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.
164 changes: 164 additions & 0 deletions packages/lint/src/validate-expressions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) ────────────────────
*
Expand Down
58 changes: 51 additions & 7 deletions packages/lint/src/validate-expressions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down Expand Up @@ -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\` ` +
Expand Down
Loading