fix(lint)!: os build and the object save door refuse a select option's visibleWhen that reads an unbound member of the acting user (#22394)

.changeset/22394-option-visible-when-evaluser-members.md

@@ -0,0 +1,36 @@
+---
+"@objectstack/lint": minor
+"@objectstack/metadata-protocol": minor
+---
+
+fix(lint)!: `os build` and the object save door refuse a select option's `visibleWhen` that reads a member of the acting user the server's option check never binds, such as `current_user.roles` (renamed `positions`), `ctx.user.roles` or `current_user.email` (#22394)
+
+Clause-②: no (narrowing: a select option's `visibleWhen` that reads an `EvalUser` member the schema does not declare is refused at build and at the object save door)
+
+A select option's `visibleWhen` is a gate the server enforces on write. The option check binds the acting user under four spellings, `current_user` and its ADR-0068 aliases `user`, `ctx.user` and `os.user`, all one `EvalUser` object. That object carries the caller's `id`, `positions`, `isPlatformAdmin` and `organizationId`, and nothing else. The build already refused a root the option check does not bind (such as `parent`) and a member of `ctx` or `os` other than `user`, but it stopped there. So an option predicate that read `'admin' in current_user.roles`, `ctx.user.roles == ['a']` or `current_user.email == 'a@b.c'` passed `os build` and the object save door with no finding. `roles` has not been a member of the acting user since ADR-0090 D3 renamed it `positions`; `name` and `email` are declared by `EvalUserSchema` but the server never sets them for this check. On every write that picked the option the predicate then faulted (`No such key: roles`, `email`), the server logged "the option's gate was NOT enforced on this write", and the value was admitted.
+
+The build's expression rule (`validateStackExpressions`) now judges the members of the acting user in an option's `visibleWhen`, under all four spellings, in the same verdict that judges its roots and the members of `ctx` and `os`. A member is accepted only when `EvalUserSchema` (`@objectstack/spec`) declares it AND the option check binds it; the allowlist is read off the schema and off `@objectstack/formula`'s `buildScope`, never written out. Any other member is refused at `error` and located at the option (`object 'NAME' · field 'FIELD' option 'VALUE' visibleWhen`). The message names the member, names the members the acting user does carry there, and gives the remedy. Every spelling of the read is judged the same: `current_user.roles`, `current_user.?roles`, `current_user['roles']`, `has(current_user.roles)`, and the same below `ctx.user` and `os.user`. The object save door runs the same pass, so its verdict is the build's finding: the same rule id (`expression-invalid`), location, message and hint.
+
+**BREAKING — what moves for consumers.**
+
+- `os build`, `os validate` and `os lint` refuse an option `visibleWhen` that reads a member of `current_user`, `user`, `ctx.user` or `os.user` other than `id`, `positions`, `isPlatformAdmin` and `organizationId`, such as `current_user.roles`, `current_user.role`, `ctx.user.roles`, `current_user.email` or `user.name`.
+- An object write in publish mode that carries such an option answered 200. It now answers `422 INVALID_METADATA`, with an `expression-invalid` issue located at that option. This covers `PUT /api/v1/meta/object/:name` (and `saveMetaItem` in publish mode), the promotion of a draft (`POST /api/v1/meta/object/:name/publish`, `publishMetaItem`), and a package draft publish (`publishPackageDrafts`).
+
+**Remedy.**
+
+- `roles` / `role` → `positions`: `'admin' in current_user.roles` becomes `'admin' in current_user.positions`, in whichever spelling the predicate used (`ctx.user.roles` becomes `ctx.user.positions`).
+- `name`, `email` and any other member: rewrite the predicate against a member the option check binds (`current_user.id`, `current_user.positions`, `current_user.isPlatformAdmin`, `current_user.organizationId`), or against a column the object declares (`record.FIELD`, `previous.FIELD`).
+- Saving the object as a draft (`mode: 'draft'`) is still allowed, because drafts are never gated; publishing that draft is judged.
+
+**Unchanged.**
+
+- The server's option check is unchanged. It binds what it bound before, and an option predicate that faults is still logged and admitted. If the runtime comes to bind a member such as `email` for an option, this refusal lifts for that member in the same change, because the allowlist is derived from what the option check binds.
+- The same members are still accepted where they are bound, such as `current_user.email` in a row-level security policy or an object action's `visible` predicate. The refusal is the option slot's alone.
+- A grant check such as `current_user.can('OBJECT', 'edit')` is a call, not a member read, and is still accepted.
+- A computed key such as `current_user[name]` names no member, so it is not judged.
+- Stored rows are not migrated, and they are not refused on read. An object stored before this change keeps loading until it is next saved, and that save is judged.
+- `OS_ALLOW_UNLINTED_METADATA_WRITES=1` still turns a refusal into a logged write.
+- Measured before crossing: the objects this repository ships carry 5 option predicates, all on `showcase_cascade`, which read `record` (four) and `current_user` (one, `current_user.positions`). None reads a member the option check does not bind. That holds over every object in its `*.object.ts` files and the two `app-multi-package` sub-stacks. They have 0 refusals at the build, before this change and after it.
+- No public export or signature moves. `validateStackExpressions(stack)` keeps its signature, and no registry entry changes.
+
+<!-- adr-0087: not-required (no-migration-prescription) a refusal at `os build` and at the object save door of a select option's visibleWhen predicate that reads a member of the acting user the server's option check does not bind: no authorable key, spelling, export or stored shape moves, and no stored row is read, rewritten or converted. A stored object whose option predicate is refused keeps loading until it is next saved, and the repair is the author's rewrite of the predicate against what the option check binds, which no ledger entry can derive. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id covers this verdict (not already-registered); and the change is a build and door verdict, not a declaration (not runtime-interface-only or type-surface-only). -->

packages/lint/src/validate-expressions.test.ts

@@ -8,6 +8,7 @@ import { describe, it, expect } from 'vitest';
 import { SCOPE_ROOTS, buildScope, ExpressionEngine } from '@objectstack/formula';
 import { EVALUATED_EXPRESSION_SOURCE_REQUIRED, ExpressionInputSchema, ObjectStackSchema } from '@objectstack/spec';
 import { FieldSchema, ObjectSchema, SelectOptionSchema } from '@objectstack/spec/data';
+import { EvalUserSchema } from '@objectstack/spec/identity';
 import { SharingRuleSchema } from '@objectstack/spec/security';
 // [#15137] The published refusal sentence a `value`-slot finding must lead
 // with — asserted from the spec's own export, never re-spelled in a test.
@@ -2225,6 +2226,219 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
       });
     });
 
+    /**
+     * ── The acting user's own members (#22394) — the family's closing card ───
+     *
+     * The option check binds the acting user under four spellings
+     * (`current_user`, `user`, `ctx.user`, `os.user`), one `EvalUser` object.
+     * Its members are held to what is BOTH declared by `EvalUserSchema`
+     * (`@objectstack/spec`) AND bound by the option check: ObjectQL builds the
+     * acting user from the caller's id, positions and organization id, and
+     * `buildScope` normalises it (deriving `isPlatformAdmin`). `roles` —
+     * renamed `positions` by ADR-0090 D3 — is declared nowhere; `name` and
+     * `email` are declared but never set there. Measured through the built
+     * `evaluateValidationRules` with an authenticated caller before this
+     * verdict existed, each faulted (`No such key`) and the value was admitted,
+     * with 0 findings at the build; so the build refuses them.
+     */
+    describe('a per-option `visibleWhen` member of the acting user the option check does not bind is refused (#22394)', () => {
+      const detail = (visibleWhen: unknown) => ({
+        objects: [
+          {
+            name: 'fx_line',
+            fields: {
+              x: { type: 'text' },
+              // A `record` field spelled like a refused member of the acting user.
+              roles: { type: 'text' },
+              tier: {
+                type: 'select',
+                options: [{ label: 'Standard', value: 'standard' }, { label: 'Gold', value: 'gold', visibleWhen }],
+              },
+            },
+          },
+        ],
+      });
+      const WHERE = "object 'fx_line' · field 'tier' option 'gold' visibleWhen";
+      /**
+       * The context the option check hands the evaluator, mirrored from its one
+       * call (`evaluateOptionVisibility` in ObjectQL's `rule-validator.ts`): the
+       * merged record, `previous`, the acting user exactly as ObjectQL's
+       * `buildEvalUser` builds it (`{ id, positions, organizationId }`) and the
+       * permission map. Nothing else.
+       */
+      const OPTION_CHECK_CONTEXT = {
+        record: { x: 'a', roles: 'r' },
+        previous: { x: 'a', roles: 'r' },
+        user: { id: 'u1', positions: ['member'], organizationId: 'org_1' },
+        permissions: {},
+      };
+      const cel = (source: string) => ({ dialect: 'cel' as const, source });
+      /** The four spellings of the acting user (ADR-0068 D1). */
+      const USER_RECEIVERS = ['current_user', 'user', 'ctx.user', 'os.user'];
+      /** What the acting user carries at the option check — the members the refusal names as bound. */
+      const BOUND = ['id', 'positions', 'isPlatformAdmin', 'organizationId'];
+
+      it.each([
+        ["'admin' in current_user.roles", '`current_user.roles`'],
+        ["ctx.user.roles == ['a']", '`ctx.user.roles`'],
+        ["'admin' in user.roles", '`user.roles`'],
+        ["'admin' in os.user.roles", '`os.user.roles`'],
+        ["current_user.role == 'admin'", '`current_user.role`'],
+        ["current_user.email == 'a@b.c'", '`current_user.email`'],
+        ["ctx.user.name != ''", '`ctx.user.name`'],
+      ])('⭐ refuses %s at error, located at the option, naming the member and the members that are bound', (body, path) => {
+        const issues = validateStackExpressions(detail(body));
+        expect(issues, JSON.stringify(issues, null, 2)).toHaveLength(1);
+        expect(issues[0]).toMatchObject({ where: WHERE, severity: 'error', source: body });
+        expect(issues[0]!.message).toContain(`option 'gold' on field 'tier' reads ${path}`);
+        for (const m of BOUND) expect(issues[0]!.message, m).toContain('`' + m + '`');
+        // …and the server's option check really cannot evaluate it.
+        expect(ExpressionEngine.evaluate(cel(body), OPTION_CHECK_CONTEXT).ok, body).toBe(false);
+      });
+
+      it('⭐ the `roles` refusal names `positions` in the spelling the author wrote, and that replacement passes and evaluates', () => {
+        for (const receiver of USER_RECEIVERS) {
+          const body = `'member' in ${receiver}.roles`;
+          expect(validateStackExpressions(detail(body))[0]!.message, body).toContain('`' + receiver + '.positions`');
+          const fixed = `'member' in ${receiver}.positions`;
+          expect(validateStackExpressions(detail(fixed)), fixed).toEqual([]);
+          expect(ExpressionEngine.evaluate(cel(fixed), OPTION_CHECK_CONTEXT), fixed).toEqual({ ok: true, value: true });
+        }
+      });
+
+      it('⭐ CONTROL — every bound member under every spelling, a grant check, and a `record` field named like a refused member pass', () => {
+        for (const receiver of USER_RECEIVERS) {
+          for (const body of [
+            `${receiver}.id != ''`,
+            `'member' in ${receiver}.positions`,
+            `${receiver}.isPlatformAdmin == false`,
+            `${receiver}.organizationId == 'org_1'`,
+            `${receiver}.can('fx_line', 'edit') || true`,
+          ]) {
+            expect(validateStackExpressions(detail(body)), body).toEqual([]);
+            expect(ExpressionEngine.evaluate(cel(body), OPTION_CHECK_CONTEXT), body).toEqual({ ok: true, value: true });
+          }
+        }
+        expect(validateStackExpressions(detail("record.roles == 'r' && previous.roles == 'r'"))).toEqual([]);
+      });
+
+      it('⭐ POSITIVE CONTROL — the same `current_user.email` read in an object action predicate, a surface the refusal does not judge, is not refused', () => {
+        const stack = detail("record.x == 'a'");
+        (stack.objects[0] as Record<string, unknown>).actions = [
+          { name: 'mark', label: 'Mark', type: 'script', visible: "current_user.email == 'a@b.c'" },
+        ];
+        expect(validateStackExpressions(stack)).toEqual([]);
+      });
+
+      it('judges every member spelling as one read: optional, indexed and `has()`', () => {
+        for (const body of [
+          'has(current_user.roles)',
+          'current_user.?roles.orValue([]) == []',
+          "current_user['roles'] == []",
+          "ctx.user['roles'] == []",
+          "ctx['user'].roles == []",
+          'has(os.user.email)',
+        ]) {
+          const issues = validateStackExpressions(detail(body));
+          expect(issues, body).toHaveLength(1);
+          expect(issues[0]!.where, body).toBe(WHERE);
+        }
+      });
+
+      it('one finding per option: an unbound root, then a namespace member, then members in `SCOPE_ROOTS` and name order', () => {
+        const withRoot = validateStackExpressions(detail("'a' in current_user.roles && input.k == 1"));
+        expect(withRoot).toHaveLength(1);
+        expect(withRoot[0]!.message).toContain('reads `input`');
+        const namespaceFirst = validateStackExpressions(detail("ctx.locale == 'en' && ctx.user.roles == []"));
+        expect(namespaceFirst).toHaveLength(1);
+        expect(namespaceFirst[0]!.message).toContain('reads `ctx.locale`');
+        // `os` precedes `current_user` in `SCOPE_ROOTS`.
+        const byRoot = validateStackExpressions(detail("'a' in current_user.roles && os.user.email == ''"));
+        expect(byRoot).toHaveLength(1);
+        expect(byRoot[0]!.message).toContain('reads `os.user.email`');
+        const byName = validateStackExpressions(detail("current_user.roles == [] && current_user.email == ''"));
+        expect(byName).toHaveLength(1);
+        expect(byName[0]!.message).toContain('reads `current_user.email`');
+      });
+
+      /**
+       * ⭐ THE ENUMERATION PIN — this card closes the #22157 → #22274 family.
+       *
+       * Every receiver the option check binds, read off the REAL `buildScope`
+       * given the option check's context, with the source its members are
+       * judged against:
+       *
+       *   receiver                          member source
+       *   record, previous                  the object's declared fields (`unknown field`)
+       *   ctx, os                           `buildScope`: the `user` member only (#22274)
+       *   current_user, user,               `EvalUserSchema`, as far as the option
+       *   ctx.user, os.user                 check binds it (#22394)
+       *
+       * `permissions` mounts no receiver: it answers `can()` through the
+       * environment. Red when `buildScope` mounts a receiver for the option
+       * check that has no row here; when any receiver's undeclared member
+       * passes the build or evaluates; when the acting user carries a member
+       * `EvalUserSchema` does not declare; or when a declared member's verdict
+       * disagrees with whether the option check binds it.
+       */
+      it('every receiver the option check binds judges its members against a declared source', () => {
+        const SOURCES: Record<string, 'object fields' | 'buildScope namespace' | 'EvalUserSchema'> = {
+          record: 'object fields',
+          previous: 'object fields',
+          ctx: 'buildScope namespace',
+          os: 'buildScope namespace',
+          current_user: 'EvalUserSchema',
+          user: 'EvalUserSchema',
+          'ctx.user': 'EvalUserSchema',
+          'os.user': 'EvalUserSchema',
+        };
+        const optionScope = buildScope(OPTION_CHECK_CONTEXT);
+        const receivers = new Map<string, Record<string, unknown>>();
+        const walk = (prefix: string, value: Record<string, unknown>): void => {
+          for (const [key, child] of Object.entries(value)) {
+            if (child === null || typeof child !== 'object' || Array.isArray(child)) continue;
+            const path = prefix ? `${prefix}.${key}` : key;
+            receivers.set(path, child as Record<string, unknown>);
+            walk(path, child as Record<string, unknown>);
+          }
+        };
+        walk('', optionScope);
+        expect([...receivers.keys()].sort()).toEqual(Object.keys(SOURCES).sort());
+
+        const declaredUser = Object.keys(EvalUserSchema.shape);
+        const accepted: string[] = [];
+        for (const [path, value] of receivers) {
+          const probe = `${path}.zz_undeclared != null`;
+          expect(validateStackExpressions(detail(probe)), probe).toHaveLength(1);
+          expect(ExpressionEngine.evaluate(cel(probe), OPTION_CHECK_CONTEXT).ok, probe).toBe(false);
+          const mounted = Object.keys(value);
+          if (SOURCES[path] !== 'EvalUserSchema') {
+            for (const m of mounted) expect(validateStackExpressions(detail(`${path}.${m} != null`)), `${path}.${m}`).toEqual([]);
+            continue;
+          }
+          // No member is mounted on the acting user without a declaration.
+          expect(mounted.filter((m) => !declaredUser.includes(m)), path).toEqual([]);
+          for (const m of declaredUser) {
+            const body = `${path}.${m} != null`;
+            const issues = validateStackExpressions(detail(body));
+            const evaluated = ExpressionEngine.evaluate(cel(body), OPTION_CHECK_CONTEXT);
+            if (mounted.includes(m)) {
+              expect(issues, body).toEqual([]);
+              expect(evaluated.ok, body).toBe(true);
+              accepted.push(body);
+            } else {
+              expect(issues, body).toHaveLength(1);
+              expect(evaluated.ok, body).toBe(false);
+            }
+          }
+        }
+        // The readings, pinned: what the acting user is declared to carry, and
+        // what it carries at the option check.
+        expect(declaredUser).toEqual(['id', 'name', 'email', 'positions', 'isPlatformAdmin', 'organizationId']);
+        expect(accepted.filter((b) => b.startsWith('current_user.'))).toEqual(BOUND.map((m) => `current_user.${m} != null`));
+      });
+    });
+
     it('flags a bare-field sharing-rule condition', () => {
       const issues = validateStackExpressions({
         objects: [{ name: 'crm_account', fields: { region: { type: 'text' } } }],
@@ -3253,6 +3467,11 @@ describe('validateStackExpressions — reads only keys the spec declares (meta-t
       // `grammar` excuse #19938 added here left with the import it excused:
       // this file no longer imports `'./flow-template-grammar.js'`.)
       'templateRefusal',
+      // [#22394] The acting user's member allowlist, derived in this file from
+      // `EvalUserSchema` and `buildScope`, and the message helpers around it.
+      // Every one is a `string[]` of member NAMES whose keys are Array methods,
+      // never metadata keys — each named so no metadata receiver hides behind it.
+      'declaredUserMembers', 'boundUserMembers', 'listedNames', 'tickedNames', 'membersRead',
     ]);
     expect(receivers.filter((r) => !tabled.has(r) && !PLUMBING.has(r))).toEqual([]);
   });