Skip to content

Validation rules, requiredWhen and option visibleWhen are silently skipped on multi-row updates (options.multi) #3106

Description

@os-zhuang

Summary

evaluateValidationRules() is the single gate for object-level validation rules, field-level requiredWhen, and per-option visibleWhen authorization. It is wired into insert and single-id update — but not into the multi-row update branch. Every one of those checks is a silent no-op on update(..., { multi: true }).

This is a declared ≠ enforced gap one layer below the one #1475 closed: #1475 fixed the switch (all 6 declared rule types now have real handlers); this is about the call site.

Evidence

Repo-wide, evaluateValidationRules has exactly two call sites (packages/objectql/src/engine.ts):

Path Line Evaluator runs?
insert engine.ts:2265
update by id engine.ts:2440
options.multidriver.updateMany engine.ts:2442-2487 never called

The bulk branch runs validateRecord (field shape), stripReadonlyWhenFieldsMulti, and stripReadonlyFields, then goes straight to driver.updateMany at engine.ts:2487.

What silently doesn't run on multi

Everything inside evaluateValidationRules, not just the rules:

  1. All 6 rule typesscript, state_machine, format, cross_field, json_schema, conditional.
  2. Field-level requiredWhen (rule-validator.ts:501-516) — whose own comment says "enforced server-side so the rule can't be bypassed."
  3. Per-option visibleWhen (rule-validator.ts:517-521evaluateOptionVisibility) — described as "Per-option authorization / cascade gating (objectui#2284)", explicitly "Complements the client-side hiding, which is not a security boundary." On the bulk path the server-side half is absent, so the client-side half is the only thing left — which the comment says is not a boundary. Hence the security label.

The warning doesn't cover the gap

engine.ts:2449-2452 warns — but only if (needsPriorRecord(updateSchema)):

if (needsPriorRecord(updateSchema as any)) {
    this.logger.warn('Object-level validation rules (state_machine/cross_field/script) are not enforced on multi-row updates', { object });
}

ruleNeedsPrior (rule-validator.ts:331-344) returns false for format and json_schema — correctly, since they only inspect the incoming value and genuinely don't need prior state. Consequences:

  • An object carrying only format / json_schema rules gets no enforcement and no warning. Fully silent.
  • The warning's text names only state_machine/cross_field/script, so even when it does fire it under-reports: format, json_schema, conditional, requiredWhen and visibleWhen are skipped too.

Note the irony: format and json_schema are the two types that need nothing from the prior record, so they are the two that could be enforced on the bulk path today at zero cost — and they're the two that fail most quietly.

Repro

Give an object a format rule, then bulk-update through it:

await ql.object('acct').update(undefined, { email: 'not-an-email' }, { multi: true, filter: [...] });
// → writes. No ValidationError, no warning.
// Same payload via update(id, ...) → throws ValidationError (invalid_format).

Blast radius

Bounded but real. Neither packages/rest nor GraphQL surfaces multi, so this is not reachable from an untrusted external caller today. In-process callers do hit it (lifecycle-service.ts, settings-service-plugin.ts:361, sql-outbox.ts), and the ceiling is "any future caller that passes multi: true" — the checks fail open with no signal.

Two docs currently overstate this

Both are accurate about the switch and inaccurate about the system:

  • packages/spec/src/data/validation.zod.ts:17-18"Everything advertised here runs on the write path ...; nothing is a silent no-op."
  • packages/objectql/src/validation/rule-validator.ts:35-38"Every variant declared by ValidationRuleSchema is enforced here ... there are no silent no-ops."

Decision needed (enforce / trim / document)

Not proposing an implementation — flagging for a call:

  1. Enforce what's free now. format / json_schema / non-prior conditional / requiredWhen / visibleWhen need no prior row and could evaluate against the bulk payload immediately. This closes the silent half.
  2. Prior-dependent rules (state_machine / cross_field / script) genuinely can't run per row from one bulk payload without an N-row fetch. Options: fetch the row-scoped set (the readonlyWhen bulk path at engine.ts:2470-2474 already does exactly this, with the same AST, and is the natural precedent) — or fail closed and reject multi on objects carrying them, rather than warn-and-proceed.
  3. At minimum, widen the warning to fire on any rule presence (not just needsPriorRecord) and correct its wording, and fix the two doc comments above.

Related

Separately: BaseValidationSchema allows events: ['insert','update','delete'] (validation.zod.ts:88), but Mode = 'insert' | 'update' (rule-validator.ts:72) and engine.delete never invokes the evaluator — so a rule declaring events: ['delete'] never runs. The evaluator's own docs concede this; the spec's enum does not. Worth folding into the same decision or splitting out.


Found while refreshing the declared ≠ enforced example in AGENTS.md Prime Directive #10 (#3105) — the directive's own example turned out to still be true, just one layer down. Filed per PD #10 rather than fixed in that docs PR.

🤖 Generated with Claude Code

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingsecurity

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions