Skip to content

Commit 674457a

Browse files
os-zhuangclaude
andauthored
fix(objectql): enforce per-option visibleWhen on checkboxes + match option values by string (objectui#2729) (#3350)
Server-side per-option gating covered select/multiselect/radio but two holes let gated values through on write: - checkboxes was excluded from CHOICE_FIELD_TYPES, so a gated `checkboxes` option (client widget cascades like multiselect since objectui#2715) was hidden in the UI but accepted from a crafted write. Now enforced element-wise on insert/update/bulk-update like multiselect. - Option matching used strict `===` while the enum validator compares by String(...); a numeric option value sent as a string passed the enum check but missed its visibleWhen gate (fail-open). Now coerces both sides. select/multiselect/radio behavior unchanged; fail-open on unbound current_user / unevaluable predicates preserved. Adds checkboxes + coercion cases to rule-validator.option-visibility.test.ts. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 10c2b85 commit 674457a

3 files changed

Lines changed: 79 additions & 6 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
"@objectstack/objectql": patch
3+
---
4+
5+
**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:
6+
7+
- **`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`.
8+
- **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.
9+
10+
Behavior for `select` / `multiselect` / `radio` is unchanged. Fail-open on unbound `current_user` / unevaluable predicates is preserved.

packages/objectql/src/validation/rule-validator.option-visibility.test.ts

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,62 @@ describe('per-option visibleWhen — multi-select element-wise', () => {
128128
});
129129
});
130130

131+
describe('per-option visibleWhen — checkboxes element-wise (objectui#2715)', () => {
132+
// `checkboxes` is the multi-value sibling of `multiselect`; its gated options
133+
// must be enforced server-side too (client cascading shipped in objectui#2735).
134+
const checks = {
135+
fields: {
136+
country: { type: 'select', options: [{ value: 'cn' }, { value: 'us' }] },
137+
provinces: {
138+
type: 'checkboxes',
139+
options: [
140+
{ value: 'zj', visibleWhen: "record.country == 'cn'" },
141+
{ value: 'gd', visibleWhen: "record.country == 'cn'" },
142+
{ value: 'ca', visibleWhen: "record.country == 'us'" },
143+
],
144+
},
145+
},
146+
};
147+
it('rejects when any checked element is invalid for the parent', () => {
148+
expect(() => evaluateValidationRules(checks, { country: 'cn', provinces: ['zj', 'ca'] }, 'insert')).toThrow(
149+
ValidationError,
150+
);
151+
});
152+
it('accepts when every checked element is valid', () => {
153+
expect(() =>
154+
evaluateValidationRules(checks, { country: 'cn', provinces: ['zj', 'gd'] }, 'insert'),
155+
).not.toThrow();
156+
});
157+
it('accounts for a gated checkboxes option in needsPriorRecord', () => {
158+
expect(needsPriorRecord(checks)).toBe(true);
159+
});
160+
});
161+
162+
describe('per-option visibleWhen — value/option type coercion', () => {
163+
// A numeric option value submitted as a string (a common REST/JSON round-trip)
164+
// must still hit its gate — matching the enum validator's String(...) compare.
165+
const numeric = {
166+
fields: {
167+
country: { type: 'select', options: [{ value: 'cn' }, { value: 'us' }] },
168+
zone: {
169+
type: 'select',
170+
options: [
171+
{ value: 1, visibleWhen: "record.country == 'cn'" },
172+
{ value: 2, visibleWhen: "record.country == 'us'" },
173+
],
174+
},
175+
},
176+
};
177+
it('gates a numeric option value sent as a string', () => {
178+
expect(() => evaluateValidationRules(numeric, { country: 'us', zone: '1' }, 'insert')).toThrow(
179+
ValidationError,
180+
);
181+
});
182+
it('accepts the string form when the gate passes', () => {
183+
expect(() => evaluateValidationRules(numeric, { country: 'cn', zone: '1' }, 'insert')).not.toThrow();
184+
});
185+
});
186+
131187
describe('needsPriorRecord accounts for option visibleWhen', () => {
132188
it('is true when a choice field has a gated option (cascade may reference a prior sibling)', () => {
133189
expect(needsPriorRecord(schema)).toBe(true);

packages/objectql/src/validation/rule-validator.ts

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -374,12 +374,16 @@ interface ConditionalFieldDef {
374374
readonly?: boolean;
375375
/** Field type — scopes per-option `visibleWhen` enforcement to choice fields. */
376376
type?: string;
377-
/** Select/multiselect/radio options; an option may gate itself with `visibleWhen`. */
377+
/** select/multiselect/radio/checkboxes options; an option may gate itself with `visibleWhen`. */
378378
options?: Array<ConditionalFieldOption | null | undefined>;
379379
}
380380

381-
/** Choice fields whose picked value(s) are drawn from `options`. */
382-
const CHOICE_FIELD_TYPES = new Set(['select', 'multiselect', 'radio']);
381+
/**
382+
* Choice fields whose picked value(s) are drawn from `options`. Includes the
383+
* multi-value `checkboxes` (its client widget cascades identically to
384+
* `multiselect`, objectui#2715) so its gated options are enforced server-side too.
385+
*/
386+
const CHOICE_FIELD_TYPES = new Set(['select', 'multiselect', 'radio', 'checkboxes']);
383387

384388
/** True when a choice field carries at least one option gated by `visibleWhen`. */
385389
function fieldHasOptionVisibility(def: ConditionalFieldDef | undefined | null): boolean {
@@ -410,8 +414,8 @@ function toExpression(cond: string | Expression): Expression {
410414
/**
411415
* Per-option authorization / cascade enforcement (objectui#2284).
412416
*
413-
* A `select` / `multiselect` / `radio` option may gate itself with a
414-
* `visibleWhen` CEL predicate, evaluated against the live record + `current_user`
417+
* A `select` / `multiselect` / `radio` / `checkboxes` option may gate itself with
418+
* a `visibleWhen` CEL predicate, evaluated against the live record + `current_user`
415419
* (the same predicate the client uses to hide the option). Client-side hiding is
416420
* UX, not a security boundary — a caller can still submit a hidden value — so on
417421
* write we re-evaluate the predicate for the *picked* value(s) and reject any
@@ -443,7 +447,10 @@ function evaluateOptionVisibility(
443447
if (raw === undefined || raw === null || raw === '') continue;
444448
const picked = Array.isArray(raw) ? raw : [raw];
445449
for (const value of picked) {
446-
const opt = def.options!.find((o) => o != null && o.value === value);
450+
// Match by string form, mirroring the enum validator (record-validator
451+
// compares `String(...)`). A numeric option value sent/stored as a string
452+
// (or vice-versa) must still hit its gate, not silently fail-open.
453+
const opt = def.options!.find((o) => o != null && String(o.value) === String(value));
447454
// Unknown value (not in options) or an ungated option → nothing to enforce
448455
// here; an out-of-set value is the enum validator's concern, not ours.
449456
if (!opt || opt.visibleWhen == null) continue;

0 commit comments

Comments
 (0)