diff --git a/.changeset/objectql-checkboxes-option-gating.md b/.changeset/objectql-checkboxes-option-gating.md new file mode 100644 index 0000000000..e1de36fa02 --- /dev/null +++ b/.changeset/objectql-checkboxes-option-gating.md @@ -0,0 +1,10 @@ +--- +"@objectstack/objectql": patch +--- + +**Enforce per-option `visibleWhen` on `checkboxes` fields, and match option values by string form (objectui#2729).** Server-side per-option gating already covered `select` / `multiselect` / `radio`, but two holes let gated values through on write: + +- **`checkboxes` was not enforced.** `CHOICE_FIELD_TYPES` omitted `checkboxes`, so a gated `checkboxes` option (whose client widget cascades identically to `multiselect` since objectui#2715) was hidden in the UI but accepted from a crafted write. Added `checkboxes` to the enforced set — its picked values are now re-evaluated against each option's `visibleWhen` (record + `current_user`) on insert/update/bulk-update, element-wise, like `multiselect`. +- **Numeric option values could slip the gate.** Option matching used strict `===`, but the enum-membership validator compares by `String(...)`. A numeric option value submitted as a string (a normal REST/JSON round-trip) passed the enum check yet missed its `visibleWhen` gate (fail-open). Matching now coerces both sides with `String(...)`, so the two validators agree on which option a written value denotes. + +Behavior for `select` / `multiselect` / `radio` is unchanged. Fail-open on unbound `current_user` / unevaluable predicates is preserved. diff --git a/packages/objectql/src/validation/rule-validator.option-visibility.test.ts b/packages/objectql/src/validation/rule-validator.option-visibility.test.ts index 6c6ec07a45..5935a36554 100644 --- a/packages/objectql/src/validation/rule-validator.option-visibility.test.ts +++ b/packages/objectql/src/validation/rule-validator.option-visibility.test.ts @@ -128,6 +128,62 @@ describe('per-option visibleWhen — multi-select element-wise', () => { }); }); +describe('per-option visibleWhen — checkboxes element-wise (objectui#2715)', () => { + // `checkboxes` is the multi-value sibling of `multiselect`; its gated options + // must be enforced server-side too (client cascading shipped in objectui#2735). + const checks = { + fields: { + country: { type: 'select', options: [{ value: 'cn' }, { value: 'us' }] }, + provinces: { + type: 'checkboxes', + options: [ + { value: 'zj', visibleWhen: "record.country == 'cn'" }, + { value: 'gd', visibleWhen: "record.country == 'cn'" }, + { value: 'ca', visibleWhen: "record.country == 'us'" }, + ], + }, + }, + }; + it('rejects when any checked element is invalid for the parent', () => { + expect(() => evaluateValidationRules(checks, { country: 'cn', provinces: ['zj', 'ca'] }, 'insert')).toThrow( + ValidationError, + ); + }); + it('accepts when every checked element is valid', () => { + expect(() => + evaluateValidationRules(checks, { country: 'cn', provinces: ['zj', 'gd'] }, 'insert'), + ).not.toThrow(); + }); + it('accounts for a gated checkboxes option in needsPriorRecord', () => { + expect(needsPriorRecord(checks)).toBe(true); + }); +}); + +describe('per-option visibleWhen — value/option type coercion', () => { + // A numeric option value submitted as a string (a common REST/JSON round-trip) + // must still hit its gate — matching the enum validator's String(...) compare. + const numeric = { + fields: { + country: { type: 'select', options: [{ value: 'cn' }, { value: 'us' }] }, + zone: { + type: 'select', + options: [ + { value: 1, visibleWhen: "record.country == 'cn'" }, + { value: 2, visibleWhen: "record.country == 'us'" }, + ], + }, + }, + }; + it('gates a numeric option value sent as a string', () => { + expect(() => evaluateValidationRules(numeric, { country: 'us', zone: '1' }, 'insert')).toThrow( + ValidationError, + ); + }); + it('accepts the string form when the gate passes', () => { + expect(() => evaluateValidationRules(numeric, { country: 'cn', zone: '1' }, 'insert')).not.toThrow(); + }); +}); + describe('needsPriorRecord accounts for option visibleWhen', () => { it('is true when a choice field has a gated option (cascade may reference a prior sibling)', () => { expect(needsPriorRecord(schema)).toBe(true); diff --git a/packages/objectql/src/validation/rule-validator.ts b/packages/objectql/src/validation/rule-validator.ts index 1e6c7ce967..608d951fd2 100644 --- a/packages/objectql/src/validation/rule-validator.ts +++ b/packages/objectql/src/validation/rule-validator.ts @@ -374,12 +374,16 @@ interface ConditionalFieldDef { readonly?: boolean; /** Field type — scopes per-option `visibleWhen` enforcement to choice fields. */ type?: string; - /** Select/multiselect/radio options; an option may gate itself with `visibleWhen`. */ + /** select/multiselect/radio/checkboxes options; an option may gate itself with `visibleWhen`. */ options?: Array; } -/** Choice fields whose picked value(s) are drawn from `options`. */ -const CHOICE_FIELD_TYPES = new Set(['select', 'multiselect', 'radio']); +/** + * Choice fields whose picked value(s) are drawn from `options`. Includes the + * multi-value `checkboxes` (its client widget cascades identically to + * `multiselect`, objectui#2715) so its gated options are enforced server-side too. + */ +const CHOICE_FIELD_TYPES = new Set(['select', 'multiselect', 'radio', 'checkboxes']); /** True when a choice field carries at least one option gated by `visibleWhen`. */ function fieldHasOptionVisibility(def: ConditionalFieldDef | undefined | null): boolean { @@ -410,8 +414,8 @@ function toExpression(cond: string | Expression): Expression { /** * Per-option authorization / cascade enforcement (objectui#2284). * - * A `select` / `multiselect` / `radio` option may gate itself with a - * `visibleWhen` CEL predicate, evaluated against the live record + `current_user` + * A `select` / `multiselect` / `radio` / `checkboxes` option may gate itself with + * a `visibleWhen` CEL predicate, evaluated against the live record + `current_user` * (the same predicate the client uses to hide the option). Client-side hiding is * UX, not a security boundary — a caller can still submit a hidden value — so on * write we re-evaluate the predicate for the *picked* value(s) and reject any @@ -443,7 +447,10 @@ function evaluateOptionVisibility( if (raw === undefined || raw === null || raw === '') continue; const picked = Array.isArray(raw) ? raw : [raw]; for (const value of picked) { - const opt = def.options!.find((o) => o != null && o.value === value); + // Match by string form, mirroring the enum validator (record-validator + // compares `String(...)`). A numeric option value sent/stored as a string + // (or vice-versa) must still hit its gate, not silently fail-open. + const opt = def.options!.find((o) => o != null && String(o.value) === String(value)); // Unknown value (not in options) or an ungated option → nothing to enforce // here; an out-of-set value is the enum validator's concern, not ours. if (!opt || opt.visibleWhen == null) continue;