Skip to content

[spec] NormalizedFilterSchema accepts ANY field-condition shape — its union's second branch is a non-strict catch-all #7711

Description

@os-zhuang

Measured while implementing #7596 (removing FieldReferenceSchema from the $between endpoints). Filing unassigned — recording, not claiming.

The fact

NormalizedFilterSchema (packages/spec/src/data/filter.zod.ts) declares each $and / $or member, and the $not operand, as:

z.union([
  // Field condition: { field: { $op: value } }
  z.record(z.string(), FieldOperatorsSchema),
  // Nested logical group
  NormalizedFilterSchema,
])

The second branch is z.object({ $and, $or, $not }) with every key optional and no .strict(). So when the record branch rejects a field condition, the object branch accepts the very same value — any object whatsoever satisfies "all three of my optional keys are absent".

The whole-filter face therefore validates the LOGICAL skeleton and nothing else. No comparand shape it declares can ever make it fail.

Measured

Run against origin/main @ 6a9dec6, and again on the #7596 branch — same answers on both, so this is not #7596's doing:

input NormalizedFilterSchema.safeParse().success FieldOperatorsSchema on the same operator map
{ $and: [{ c: { $null: 'not-a-boolean' } }] } true false
{ $and: [{ c: { $between: [1, 2, 3] } }] } true false
{ $and: [{ c: { $between: [{ $field: 'b' }, 100] } }] } true (both before and after #7596) false after #7596

$null: 'not-a-boolean' is the clean control: it has never been a declared comparand, and three drivers carry hand-written refusals whose message quotes FieldOperatorsSchema declares $null as a boolean — a declaration the schema face itself does not enforce at this level.

Why it matters

It is a declaration-face gap rather than a live defect today, which is why this is filed as a finding:

  • Nothing calls it. No .parse / .safeParse of NormalizedFilterSchema exists outside packages/spec's own tests; every driver reference to it and to FieldOperatorsSchema is prose inside a docblock. So no request path currently depends on the verdict.
  • The comments say otherwise. FieldOperatorsSchema is annotated twice as "the ENFORCED one — NormalizedFilterSchema validates against it". It does, on one branch of a union whose other branch accepts everything, which makes the sentence true in letter and misleading in effect. A future consumer wiring the whole-filter face in as a validation step would get a green on comparands no backend accepts.
  • The AI-authoring axis. ADR-0033's population reads the declared surface to decide what is writable. A face that answers "valid" to { $between: [1, 2, 3] } teaches exactly the wrong thing.

Possible directions (not decided here)

  1. .strict() on the recursive object branch, so a field-condition object cannot slip past as an empty logical group. Cheapest; needs a check for filters that legitimately mix logical keys and field keys at one level.
  2. Reorder / discriminate the union on the presence of a $and / $or / $not key, so a field condition is always judged by the record branch.
  3. Leave it and delete the "ENFORCED" claim from the two comments, on the grounds that the whole-filter face is a shape declaration and the per-operator face is the enforcement point.

Option 1 or 2 is what "declared = enforced" (ADR-0049) points at; option 3 is honest but gives up a face that reads like it validates.

Refs

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions