diff --git a/.changeset/22394-option-visible-when-evaluser-members.md b/.changeset/22394-option-visible-when-evaluser-members.md new file mode 100644 index 00000000000..f52c2beecae --- /dev/null +++ b/.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. + + diff --git a/packages/lint/src/validate-expressions.test.ts b/packages/lint/src/validate-expressions.test.ts index 837a2f47671..1519ab6ead3 100644 --- a/packages/lint/src/validate-expressions.test.ts +++ b/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).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 = { + 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>(); + const walk = (prefix: string, value: Record): 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); + walk(path, child as Record); + } + }; + 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([]); }); diff --git a/packages/lint/src/validate-expressions.ts b/packages/lint/src/validate-expressions.ts index 72a3bd4c10d..7343ff874ed 100644 --- a/packages/lint/src/validate-expressions.ts +++ b/packages/lint/src/validate-expressions.ts @@ -84,7 +84,9 @@ import { parseCelToAst, SCOPE_ROOTS, analyzeRelationshipTraversals, + buildScope, } from '@objectstack/formula'; +import type { RelationshipTraversalAnalysis } from '@objectstack/formula'; import { collectFlowGraphs, flowNodeConfigRefusals, @@ -111,6 +113,10 @@ import type { FlowNodeParsed, FlowEdgeParsed } from '@objectstack/spec/automatio // shared refusal to prevent. import { EvaluatedExpressionInputSchema, EVALUATED_EXPRESSION_SOURCE_REQUIRED } from '@objectstack/spec/shared'; import { referenceCarrierOf, REFERENCE_VALUE_TYPES } from '@objectstack/spec/data'; +// [#22394] The acting user's declared members — the contract half of the option +// verdict's member allowlist for `current_user` and its ADR-0068 aliases. Read +// off the schema at first use, never restated here. +import { EvalUserSchema } from '@objectstack/spec/identity'; import { collectFlowVariableNames, shadowedFieldReads, shadowedFieldMessage } from './flow-variable-scope.js'; import { injectedColumnsFor, unprovisionedInjectedColumnsFor } from './system-fields.js'; @@ -1112,7 +1118,9 @@ function fieldTraversalMessage( * * Two of these roots are accepted WHOLE here but bound only IN PART: `ctx` and * `os` carry just their `user` member at the option check. What they carry is - * {@link OPTION_VISIBLE_WHEN_BOUND_MEMBERS}'s to say (#22274). + * {@link OPTION_VISIBLE_WHEN_BOUND_MEMBERS}'s to say (#22274). And the acting + * user itself — under all four spellings — carries only the members + * {@link optionVisibleWhenUserMembers} derives (#22394). */ const OPTION_VISIBLE_WHEN_BOUND_ROOTS: readonly string[] = ['record', 'previous', 'current_user', 'user', 'ctx', 'os']; @@ -1134,11 +1142,13 @@ const OPTION_VISIBLE_WHEN_BOUND_ROOTS: readonly string[] = ['record', 'previous' * `evaluateValidationRules` with an authenticated caller, each faults * (`No such key: org` / `env` / `locale`) and the value is admitted. * - * Only `ctx` and `os` are listed. `current_user` and `user` ARE the `EvalUser`; - * their members are that object's own fields, the same at every site that - * binds a user, so no member of theirs is unbound here in particular. That is - * also why this verdict stops at the first member: `ctx.user.positions` reads - * the `EvalUser` and is judged like `current_user.positions`, not here. + * Only `ctx` and `os` are listed, and every member listed IS the acting user + * (`buildScope` hangs the one `EvalUser` object on `current_user`, `user`, + * `ctx.user` and `os.user`). So this map judges the FIRST member under each + * namespace, and what the acting user carries one level further down + * (`ctx.user.positions`, `os.user.roles`) is judged against + * {@link optionVisibleWhenUserMembers}, exactly as `current_user.positions` is + * (#22394). * * The test suite derives this list from the real `buildScope` and evaluator, * given the option check's context, so a member `buildScope` starts mounting @@ -1151,6 +1161,97 @@ const OPTION_VISIBLE_WHEN_BOUND_MEMBERS: Readonly boundUserMembers.includes(m)); + return optionVisibleWhenUserMembersMemo; +} + +/** `` `a`, `b` and `c` `` — names spelled for a message, in the order given. */ +function tickedList(listedNames: readonly string[]): string { + const tickedNames = listedNames.map((n) => '`' + n + '`'); + return tickedNames.length > 1 + ? `${tickedNames.slice(0, -1).join(', ')} and ${tickedNames[tickedNames.length - 1]}` + : tickedNames.join(''); +} + +/** The members read under one root, in name order: hop, value and deeper reads alike. */ +function membersReadUnder(found: RelationshipTraversalAnalysis): string[] { + const { traversals, bareFields, multiHopFields } = found; + return [...new Set([...Array.from(traversals, ([m]) => m), ...bareFields, ...multiHopFields])].sort(); +} + +/** The members read one hop below `member` of the analysed root, in name order. */ +function membersReadThrough(found: RelationshipTraversalAnalysis, member: string): string[] { + const { traversals } = found; + for (const [m, reads] of traversals) { + if (m === member) return [...reads].sort(); + } + return []; +} + /** A CEL member path, spelled for a message: `` `root.member` ``. */ function celMemberPath(root: string, member: string): string { // Assembled with `+`: #5017's receiver scan reads string literals too, so a @@ -1196,6 +1297,24 @@ function celMemberPath(root: string, member: string): string { * root, so `os.org`, `os.?org`, `os['org']` and `has(os.org)` are one read. A * computed key (`os[k]`) names no member and is not judged. * + * ## Members of the acting user (#22394) + * + * The same verdict judges what the acting user carries, under all four of its + * spellings: the first member of `current_user` / `user`, and the member one + * hop below `ctx.user` / `os.user`. Each is held to + * {@link optionVisibleWhenUserMembers} — declared by `EvalUserSchema` AND bound + * by the option check — so `current_user.roles` (renamed `positions` by + * ADR-0090 D3) and `current_user.email` (declared, never set there) are + * refused, read by the same reader and so in every spelling. + * + * With #22157 (roots) and #22274 (namespace members) this closes the family: + * every receiver the option check binds has its members judged against a + * declared source — `record` / `previous` against the object's fields (the + * `unknown field` check in `validateExpression`), `ctx` / `os` against + * {@link OPTION_VISIBLE_WHEN_BOUND_MEMBERS}, and the four spellings of the + * acting user against {@link optionVisibleWhenUserMembers}. The test suite + * enumerates the receivers off the real `buildScope` and pins that table. + * * `null` = nothing to report: the source does not parse (the syntax pass owns * that), or every root and member it reads is one the option check binds. */ @@ -1234,18 +1353,30 @@ function optionVisibleWhenRootIssue( }; } +/** + * The clause every member refusal ends its consequence with: a member the + * option check never sets is not only a fault, it also defeats the two + * spellings an author reaches for to guard it. + */ +const OPTION_MEMBER_GUARD_CONSEQUENCE = + 'a `has()` test or an optional read of it never finds it set, so the option is refused on every ' + + 'write instead, or admitted on every write when the test is negated or the read\'s default passes'; + /** * [#22274] The member half of {@link optionVisibleWhenRootIssue}: a member of * `ctx` or `os` the option check does not bind - * ({@link OPTION_VISIBLE_WHEN_BOUND_MEMBERS}). One finding per option, the - * first unbound member in `SCOPE_ROOTS` order and then in name order: stable, - * never AST walk order. - * - * The message names the member, says what the option check DOES bind under - * that root, and gives the remedy that is true for the member. For `os.org` - * that is `current_user.organizationId`: the engine builds the acting user - * with the caller's organization id (`null` outside one), so that one - * organization fact IS bound at the option check. + * ({@link OPTION_VISIBLE_WHEN_BOUND_MEMBERS}) — and [#22394] a member of the + * acting user it does not bind ({@link optionVisibleWhenUserMembers}), under + * `current_user` / `user` or one hop below `ctx.user` / `os.user`. One finding + * per option, the first unbound member in `SCOPE_ROOTS` order, then (under a + * namespace) the namespace's own member before the acting user's, then in name + * order: stable, never AST walk order. + * + * The message names the member, says what the option check DOES bind there, + * and gives the remedy that is true for the member. For `os.org` that is + * `current_user.organizationId`: the engine builds the acting user with the + * caller's organization id (`null` outside one), so that one organization fact + * IS bound at the option check. */ function optionVisibleWhenMemberIssue( objectName: string | undefined, @@ -1255,44 +1386,117 @@ function optionVisibleWhenMemberIssue( roots: readonly string[], ): { root: string; message: string } | null { for (const root of SCOPE_ROOTS) { - if (OPTION_VISIBLE_WHEN_BOUND_MEMBERS[root] === undefined || !roots.includes(root)) continue; + if (!roots.includes(root)) continue; + const namespaceMembers = OPTION_VISIBLE_WHEN_BOUND_MEMBERS[root]; + const isUserRoot = OPTION_VISIBLE_WHEN_USER_ROOTS.includes(root); + if (namespaceMembers === undefined && !isUserRoot) continue; const found = analyzeRelationshipTraversals(source, root); if (found === null) continue; - const { traversals, bareFields, multiHopFields } = found; - const member = [...new Set([...Array.from(traversals, ([m]) => m), ...bareFields, ...multiHopFields])] - .sort() - .find((m) => !OPTION_VISIBLE_WHEN_BOUND_MEMBERS[root]!.includes(m)); - if (member === undefined) continue; - const path = celMemberPath(root, member); - const owner = objectName ? `'${objectName}'` : 'this object'; - const prescription = root === 'os' && member === 'org' - ? `The option check binds no organization. The one organization fact it carries is the acting ` + - `user's: ${celMemberPath('current_user', 'organizationId')} holds the caller's organization ` + - `id (\`null\` outside one), so compare that instead. Any other fact about the organization ` + - `cannot gate an option; read a column ${owner} declares.` - : root === 'os' && member === 'env' - ? `The option check binds no deployment environment (${path} is bound only where an ` + - `evaluation is given one, such as a seed value), so the choices a record offers cannot ` + - `depend on it there. Gate on a column ${owner} declares, or on the acting user as ` + - `\`current_user\`.` - : `Whatever other evaluation sites bind under \`${root}\`, the option check binds only its ` + - `\`user\` member. Rewrite the predicate against \`record\` (plus \`previous\`), or against ` + - `the acting user as \`current_user\`.`; - return { - root, - message: - `\`visibleWhen\` of option ${option} on field '${field}' reads ${path}, but the server's ` + - `option check binds only \`record\`, \`previous\` and the acting user (\`current_user\`, and ` + - `its ADR-0068 aliases \`user\`, \`ctx\` and \`os\`), and under \`${root}\` it binds the ` + - `\`user\` member and nothing else, so ${path} is unbound there. ` + - `${FIELD_TRAVERSAL_CONSEQUENCE['option visibleWhen']}; a \`has()\` test or an optional read ` + - `of it never finds it set, so the option is refused on every write instead, or admitted on ` + - `every write when the test is negated or the read's default passes. ${prescription}`, - }; + const membersRead = membersReadUnder(found); + if (isUserRoot) { + const member = membersRead.find((m) => !optionVisibleWhenUserMembers().includes(m)); + if (member !== undefined) return optionVisibleWhenUserMemberIssue(objectName, field, option, root, member); + continue; + } + const member = membersRead.find((m) => !namespaceMembers!.includes(m)); + if (member !== undefined) { + return optionVisibleWhenNamespaceMemberIssue(objectName, field, option, root, member); + } + // [#22394] Every member the namespace binds IS the acting user, so what is + // read one hop below it is the acting user's member. + for (const userMember of namespaceMembers!) { + const below = membersReadThrough(found, userMember).find((m) => !optionVisibleWhenUserMembers().includes(m)); + if (below !== undefined) { + return optionVisibleWhenUserMemberIssue(objectName, field, option, `${root}.${userMember}`, below); + } + } } return null; } +/** [#22274] The refusal of a `ctx` / `os` member the option check does not bind. */ +function optionVisibleWhenNamespaceMemberIssue( + objectName: string | undefined, + field: string, + option: string, + root: string, + member: string, +): { root: string; message: string } { + const path = celMemberPath(root, member); + const owner = objectName ? `'${objectName}'` : 'this object'; + const prescription = root === 'os' && member === 'org' + ? `The option check binds no organization. The one organization fact it carries is the acting ` + + `user's: ${celMemberPath('current_user', 'organizationId')} holds the caller's organization ` + + `id (\`null\` outside one), so compare that instead. Any other fact about the organization ` + + `cannot gate an option; read a column ${owner} declares.` + : root === 'os' && member === 'env' + ? `The option check binds no deployment environment (${path} is bound only where an ` + + `evaluation is given one, such as a seed value), so the choices a record offers cannot ` + + `depend on it there. Gate on a column ${owner} declares, or on the acting user as ` + + `\`current_user\`.` + : `Whatever other evaluation sites bind under \`${root}\`, the option check binds only its ` + + `\`user\` member. Rewrite the predicate against \`record\` (plus \`previous\`), or against ` + + `the acting user as \`current_user\`.`; + return { + root, + message: + `\`visibleWhen\` of option ${option} on field '${field}' reads ${path}, but the server's ` + + `option check binds only \`record\`, \`previous\` and the acting user (\`current_user\`, and ` + + `its ADR-0068 aliases \`user\`, \`ctx\` and \`os\`), and under \`${root}\` it binds the ` + + `\`user\` member and nothing else, so ${path} is unbound there. ` + + `${FIELD_TRAVERSAL_CONSEQUENCE['option visibleWhen']}; ${OPTION_MEMBER_GUARD_CONSEQUENCE}. ${prescription}`, + }; +} + +/** + * [#22394] The refusal of a member of the acting user the option check does + * not bind. `receiver` is the spelling the author wrote (`current_user`, + * `user`, `ctx.user` or `os.user`), so the remedy is written back in it. + * + * Three remedies, by what the member is: + * + * - `roles` / `role` — not an `EvalUser` member at all: ADR-0090 D3 renamed + * the membership array `positions`, one-step, with no alias. The remedy is + * the rename, `'NAME' in RECEIVER.positions`. + * - a member `EvalUserSchema` declares but the option check never sets + * (`name`, `email`) — real on a surface that is handed a fuller user, never + * here: the engine builds the acting user from the caller's id, positions + * and organization id only. The remedy is a bound member, or a column. + * - anything else — a member nobody declares. The remedy is the bound list. + */ +function optionVisibleWhenUserMemberIssue( + objectName: string | undefined, + field: string, + option: string, + receiver: string, + member: string, +): { root: string; message: string } { + const path = celMemberPath(receiver, member); + const owner = objectName ? `'${objectName}'` : 'this object'; + const carried = tickedList(optionVisibleWhenUserMembers()); + const declared = Object.keys(EvalUserSchema.shape).includes(member); + const prescription = member === 'roles' || member === 'role' + ? `The acting user has no \`${member}\`: ADR-0090 D3 renamed its membership array ` + + `${celMemberPath(receiver, 'positions')} — the built-in identity and position names the user ` + + `holds — with no alias. Write \`'NAME' in ${receiver}.positions\` instead.` + : declared + ? `\`${member}\` is declared on the acting user (\`EvalUser\`) but the server builds the user ` + + `for this check from the caller's ${tickedList(Object.keys(OPTION_CHECK_ACTING_USER))} only, ` + + `so it is never set here, whichever surface sets it elsewhere. Gate on a member the check ` + + `binds, or on a column ${owner} declares.` + : `\`${member}\` is not a member of the acting user (\`EvalUser\`). Gate on a member the check ` + + `binds, or on a column ${owner} declares.`; + return { + root: receiver, + message: + `\`visibleWhen\` of option ${option} on field '${field}' reads ${path}, but the acting user the ` + + `server's option check binds (\`current_user\`, and its ADR-0068 aliases \`user\`, ` + + `${celMemberPath('ctx', 'user')} and ${celMemberPath('os', 'user')}) carries ${carried} and ` + + `nothing else, so ${path} is unbound there. ` + + `${FIELD_TRAVERSAL_CONSEQUENCE['option visibleWhen']}; ${OPTION_MEMBER_GUARD_CONSEQUENCE}. ${prescription}`, + }; +} + /** * [#20078] The master object of an object with exactly ONE `master_detail` * relationship — the record the field level binds as `parent` — or `undefined` @@ -1387,7 +1591,9 @@ export interface StackExpressionOptions { * the server's option check cannot evaluate it and fails open (logged, * allowed through), so the gate it declares is never enforced. Since * #22157 the pass also refuses a root that option check does not bind - * (`parent` above all), at both doors through this same call; + * (`parent` above all), at both doors through this same call; since + * #22274 a member of `ctx` / `os` it does not fill, and since #22394 a + * member of the acting user it does not bind (`current_user.roles`); * - [#22032, pass 4] the object's own `actions[]` pass — each action's * `visible`, and its `disabled` unless that is a boolean literal, as a * `record`-scoped predicate (`checkAction` below). The same sentence of @@ -2258,7 +2464,9 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression // two surfaces now differ by exactly what their evaluators bind: this // one accepts the acting user and refuses `parent`, the field-rule slots // the other way round. [#22274] The same verdict judges the members of - // `ctx` and `os`, which the option check fills with `user` only. + // `ctx` and `os`, which the option check fills with `user` only, and + // [#22394] the acting user's own members under all four of its spellings, + // held to what `EvalUserSchema` declares AND the option check binds. // // [#22032, pass 3] NOT fenced on an object write: the option's `check`, // its root verdict (#22157) and its traversal refusal are the pass the diff --git a/packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts b/packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts index 18f14ee6ae9..0bfeb1d313e 100644 --- a/packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts +++ b/packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts @@ -2469,6 +2469,125 @@ describe('runtime authoring gate on OBJECT writes — an option visibleWhen read }); }); +/** + * [#22394] The object save door refuses a field option's `visibleWhen` that + * reads a member of the acting user the server's option check never binds, as + * `os build` does — the last card of the #22157 (roots) → #22274 (`ctx` / `os` + * members) family. + * + * The option check binds the acting user under `current_user`, `user`, + * `ctx.user` and `os.user`, carrying what `EvalUserSchema` declares AND the + * engine sets: its id, `positions`, `isPlatformAdmin` and `organizationId`. + * Measured before this change: an option whose `visibleWhen` read + * `current_user.roles` (renamed `positions` by ADR-0090 D3), `ctx.user.roles` + * or `current_user.email` (declared, never set there) saved through this door, + * and every write that picked the option faulted (`No such key`) and was + * admitted unchecked. + * + * The refusal is in `@objectstack/lint` (the option pass's member verdict); no + * code here moves. Pinned through the REAL `saveMetaItem`: + * + * (a) each measured body is refused on an active save, a 422 + * `INVALID_METADATA` carrying the build's located finding, and nothing + * lands; + * (b) control: every member the option check binds, under every ADR-0068 + * spelling, and `current_user.can(…)` save, and the row lands; + * (c) the door's issue and the build's finding are the same finding. + * + * ⚠️ As in the blocks above: this package reaches `@objectstack/lint` through + * its built `dist/`, so an edit to the rule is invisible here until + * `pnpm --filter @objectstack/lint build` has run. + */ +describe('runtime authoring gate on OBJECT writes — an option visibleWhen reading an unbound member of the acting user (#22394)', () => { + const item = (visibleWhen: unknown) => ({ + name: 'fx_item', + label: 'Item', + sharingModel: 'private', + fields: { + x: { type: 'text', label: 'X' }, + tier: { + type: 'select', + label: 'Tier', + options: [{ label: 'Standard', value: 'standard' }, { label: 'Gold', value: 'gold', visibleWhen }], + }, + }, + }); + /** The card's measured bodies, each with the member path the finding names. */ + const REFUSED: Array<[string, string]> = [ + ["'admin' in current_user.roles", '`current_user.roles`'], + ["ctx.user.roles == ['a']", '`ctx.user.roles`'], + ["current_user.email == 'a@b.c'", '`current_user.email`'], + ]; + /** Where the build locates it — the option the author edits. */ + const WHERE = "object 'fx_item' · field 'tier' option 'gold' visibleWhen"; + + const host = () => { + const { engine, rows } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine, () => new Map(), 'env_test') as any; + return { protocol, rows }; + }; + const itemRows = (rows: Map) => + Array.from(rows.values()).filter((r) => r.type === 'object' && r.name === 'fx_item'); + const save = (protocol: any, visibleWhen: unknown) => + protocol.saveMetaItem({ type: 'object', name: 'fx_item', item: item(visibleWhen) }); + const buildFindings = (visibleWhen: unknown) => { + const stack = { objects: [item(visibleWhen)] }; + return runAuthoringRules('build', { normalized: stack, parsed: stack }) + .filter((f) => f.rule === EXPRESSION_INVALID); + }; + + it.each(REFUSED)('(a) REFUSES %s on an active save with a 422 carrying the located finding, and nothing lands', async (body, path) => { + const { protocol, rows } = host(); + + const err = await save(protocol, body).catch((e: any) => e); + + expect(err, 'the save resolved — the door still accepts the option predicate').toBeInstanceOf(Error); + expect({ code: err.code, status: err.status }).toEqual({ code: 'INVALID_METADATA', status: 422 }); + const issues = err.issues.filter((i: any) => i.rule === EXPRESSION_INVALID); + expect(issues, `issues: ${JSON.stringify(err.issues)}`).toHaveLength(1); + expect(issues[0].path).toBe(WHERE); + expect(issues[0].severity).toBe('error'); + // The named subject: the option, the field, the member, and the members that ARE bound. + expect(issues[0].message).toContain(`option 'gold' on field 'tier' reads ${path}`); + for (const m of ['id', 'positions', 'isPlatformAdmin', 'organizationId']) { + expect(issues[0].message, m).toContain('`' + m + '`'); + } + expect(itemRows(rows)).toEqual([]); + }); + + it('(b) control: every bound member under every ADR-0068 spelling, and `can`, save, and the row lands', async () => { + for (const body of [ + "'org_admin' in current_user.positions", + "'org_admin' in ctx.user.positions", + "user.isPlatformAdmin == true", + "os.user.organizationId != ''", + "current_user.id != ''", + "current_user.can('fx_item', 'edit')", + ]) { + const { protocol, rows } = host(); + const result = await save(protocol, body); + expect(result.success, body).toBe(true); + expect(itemRows(rows).map((r) => r.state), body).toEqual(['active']); + } + }); + + it.each(REFUSED)('(c) the door and `os build` give the SAME finding for %s', async (body) => { + const { protocol } = host(); + const err = await save(protocol, body).catch((e: any) => e); + const atDoor = (err.issues ?? []).filter((i: any) => i.rule === EXPRESSION_INVALID); + + const atBuild = buildFindings(body); + + // Non-vacuous on both sides. + expect(atBuild).toHaveLength(1); + expect(atBuild[0]!.severity).toBe('error'); + expect(atDoor).toHaveLength(1); + for (const key of ['rule', 'where', 'path', 'message', 'hint'] as const) { + expect(atDoor[0][key], `door and build disagree on '${key}'`).toBe(atBuild[0]![key]); + } + }); +}); + /** * [#22118] The object save door judges a stored sibling's finding against the * STORED universe — the registry's objects WITH the written object's stored