Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions .changeset/normalized-filter-member-union-strict.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
---
"@objectstack/spec": minor
---

fix(spec): `NormalizedFilterSchema` judges its members instead of catching them all (#7711)

Each `$and` / `$or` member and the `$not` operand was
`z.union([z.record(z.string(), FieldOperatorsSchema), NormalizedFilterSchema])`,
and the second branch was a NON-strict `z.object({ $and, $or, $not })` with every
key optional. "All three of my optional keys are absent" is true of any object
whatsoever, so that branch was a catch-all: whenever the record branch **rejected**
a field condition, the group branch accepted the very same value. The whole-filter
face validated the logical skeleton and nothing else — no comparand shape it
declared could ever make it fail.

Measured before the change, every one of these parsed green while
`FieldOperatorsSchema` — the copy this schema is documented as validating
against — refused the identical operator map:

```
{ $and: [{ c: { $null: 'not-a-boolean' } }] }
{ $and: [{ c: { $between: [1, 2, 3] } }] }
{ $and: [{ hello: 'world' }] }
```

The green was also lossy: an admitted member came back parsed to `{}`, so the
accepted output no longer carried the condition it was asked about — a whole
`{ $not: … }` subtree parsed as a field named `$not` and returned empty.

Now the group branch is `.strict()` and the field-condition branch rules out
`$`-prefixed keys, so a member the operator map refuses has nowhere to land. The
refusal names the offending keys and both valid member shapes:

```
Not a valid $and member — got an object with key(s) "c". A $and member is either
a FIELD CONDITION ({ "field": { "$op": value } }, whose keys are field names and
whose operator map must satisfy FieldOperatorsSchema — comparand shapes
included), or a nested LOGICAL GROUP carrying only $and / $or / $not and nothing
else. Ruled on #7711: declared = enforced (ADR-0049).
```

Graded a narrowing of the accepted surface, so `minor` rather than `patch` — but
the blast radius is measured at zero: nothing in the repo called `.parse` /
`.safeParse` on `NormalizedFilterSchema` outside this package's own tests
(swept with `FilterConditionSchema`'s 20-plus call sites as the positive
control), no driver or evaluator references the normalized AST at all, and the
exported `NormalizedFilter` TYPE is unchanged — every shape that stops parsing
was already outside it. Nothing is removed from the declared surface, which is
what separates this from the ADR-0087 removal grade.

`FilterConditionSchema`, the AUTHORING face every driver, `read-scope-sql` and
`cel-to-filter` actually consume, is untouched and still admits sugar by design,
so no request path's row set can move with this.
16 changes: 8 additions & 8 deletions docs/audits/2026-07-unknown-key-strictness-ledger.counts.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -22,7 +22,7 @@ regenerate.
|---|---|
| Triaged directories | 5 |
| Object sites in them | 438 |
| Still-open (strip) sites | 181 |
| Still-open (strip) sites | 180 |
| Files carrying at least one | 27 |

Remaining strip sites by class:
Expand All@@ -31,7 +31,7 @@ Remaining strip sites by class:
|---|---|
| authorable — the ruling's forced scope | 40 |
| unresolved — needs a per-schema verdict | 34 |
| wire / open — out of forced scope | 105 |
| wire / open — out of forced scope | 104 |
| no door — no carrier, ADR-0049 territory | 1 |
| no gate — carrier live, no parse | 0 |
| covered — no carrier, no parse, guarded at every consumer | 1 |
Expand All@@ -45,11 +45,11 @@ The `strict` column is the one the campaign schedules against; it counts both th
| Dir | Sites | strict | passthrough | catchall | strip |
|---|---|---|---|---|---|
| `ui/` | 161 | 119 | 5 | 0 | 37 |
| `data/` | 165 | 56 | 1 | 0 | 108 |
| `data/` | 165 | 57 | 1 | 0 | 107 |
| `automation/` | 65 | 42 | 0 | 0 | 23 |
| `security/` | 20 | 7 | 0 | 0 | 13 |
| `studio/` | 27 | 27 | 0 | 0 | 0 |
| **total** | **438** | **251** | **6** | **0** | **181** |
| **total** | **438** | **252** | **6** | **0** | **180** |

## File-level triage — site counts

Expand DownExpand Up@@ -179,7 +179,7 @@ over it is here.

### `data/` — open

**108 strip of 165**, in 16 file(s).
**107 strip of 165**, in 16 file(s).

| File | Strip | Sites |
|---|---|---|
Expand All@@ -194,18 +194,18 @@ over it is here.
| `external-lookup.zod.ts` | 12 | 12 |
| `field-value.zod.ts` | 2 | 3 |
| `field.zod.ts` | 2 | 10 |
| `filter.zod.ts` | 11 | 11 |
| `filter.zod.ts` | 10 | 11 |
| `hook.zod.ts` | 5 | 7 |
| `object.zod.ts` | 1 | 20 |
| `query.zod.ts` | 4 | 5 |
| `seed-loader.zod.ts` | 12 | 12 |
| **total** | **108** | **165** |
| **total** | **107** | **165** |

| Bucket | Sites |
|---|---|
| authorable — the ruling's forced scope | 8 |
| unresolved — needs a per-schema verdict | 34 |
| wire / open — out of forced scope | 66 |
| wire / open — out of forced scope | 65 |
| no door — no carrier, ADR-0049 territory | 0 |
| no gate — carrier live, no parse | 0 |
| covered — no carrier, no parse, guarded at every consumer | 0 |
Expand Down
163 changes: 143 additions & 20 deletions packages/spec/src/data/filter.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -432,33 +432,41 @@ describe('RangeOperatorSchema', () => {
});

/**
* ## `NormalizedFilterSchema` cannot go red on this, and that is NOT this
* ruling's doing — measured, and pinned so the next reader does not mistake
* the green for enforcement.
* ## The whole-filter face now goes red on this too — #7711 closed the union's catch-all
*
* A `$and` member is `z.union([z.record(z.string(), FieldOperatorsSchema),
* NormalizedFilterSchema])`. When the record branch rejects a field
* condition, the SECOND branch is a non-strict `z.object({ $and, $or, $not
* })` with every key optional — which accepts any object whatsoever. So the
* whole-filter face admits every field-condition shape, and the control
* below shows it does so for a comparand nobody has ever declared valid.
* The enforcement that this ruling moves lives one level down, in
* `FieldOperatorsSchema`, which the tests above assert directly. Filed
* separately as its own finding; asserting a red here would have been a
* fabricated pin.
* This case was written by #7596 as the OPPOSITE pin: both inputs parsed
* GREEN through `NormalizedFilterSchema`, and it said so on purpose, so the
* green would not be read as enforcement. The reason was structural — a
* `$and` member was `z.union([z.record(z.string(), FieldOperatorsSchema),
* NormalizedFilterSchema])` against a NON-strict group whose every key is
* optional, and "all three of my optional keys are absent" is true of any
* object at all. When the record branch rejected a field condition, the
* group branch accepted the very same value.
*
* #7711 made that branch `.strict()` and ruled `$`-prefixed keys out of the
* field-condition branch, so a member the record branch refuses now has
* nowhere to land. The case is kept in place, flipped rather than deleted:
* the two `FieldOperatorsSchema` assertions below are unchanged (that level
* always rejected both), and the pair above them is what moved — which is
* exactly the evidence that the two faces now agree.
*/
it('the whole-filter face is loose about field conditions — pre-existing, control included', () => {
it('the whole-filter face refuses these field conditions too — #7711 (was green pre-#7711)', () => {
const withReference = NormalizedFilterSchema.safeParse({
$and: [{ amount: { $between: [{ $field: 'budget' }, 100] } }],
});
const alreadyInvalidComparand = NormalizedFilterSchema.safeParse({
$and: [{ close_date: { $null: 'not-a-boolean' } }],
});
// Both green, for the same structural reason — the second has nothing to
// do with #7596 and was green before it.
expect(withReference.success).toBe(true);
expect(alreadyInvalidComparand.success).toBe(true);
// And the level that DOES judge comparands rejects both.
// Both red now, for the same structural reason they were both green
// before: one union branch used to accept whatever the other refused.
expect(withReference.success).toBe(false);
expect(alreadyInvalidComparand.success).toBe(false);
// Message-bearing, and it names the member position rather than the
// generic "Invalid input" zod gives an unexplained union.
expect(withReference.error?.issues[0]?.code).toBe('invalid_union');
expect(withReference.error?.issues[0]?.message).toContain('Not a valid $and member');
expect(alreadyInvalidComparand.error?.issues[0]?.message).toContain('#7711');
// And the level that DOES judge comparands rejects both — unchanged.
expect(FieldOperatorsSchema.safeParse({ $between: [{ $field: 'budget' }, 100] }).success)
.toBe(false);
expect(FieldOperatorsSchema.safeParse({ $null: 'not-a-boolean' }).success).toBe(false);
Expand DownExpand Up@@ -1206,9 +1214,124 @@ describe('NormalizedFilterSchema', () => {
deleted: { $eq: true }
}
};

expect(() => NormalizedFilterSchema.parse(filter)).not.toThrow();
});

// ==========================================================================
// #7711 — the union's group branch is no longer a catch-all.
//
// Every member below parsed GREEN before this change, measured on main @
// `8669e5d`: a member is `z.union([<field condition>, NormalizedFilterSchema])`
// and the second branch was a non-strict `z.object({ $and, $or, $not })` with
// every key optional, so it accepted any object whatsoever — including the
// ones the first branch had just refused. The whole-filter face therefore
// validated the logical SKELETON and nothing else, and no comparand shape it
// declared could make it fail.
//
// The green was also LOSSY, which is what these cases pin on the accept side
// as well: the catch-all returned `{}` for the member it admitted, so the
// parse output no longer carried the condition it was asked about.
// ==========================================================================
describe('#7711 — a member the field-condition branch refuses has nowhere to land', () => {
/** The card's clean control: `$null` has never been anything but a boolean. */
it('rejects a $and member whose operator map FieldOperatorsSchema refuses', () => {
const result = NormalizedFilterSchema.safeParse({
$and: [{ c: { $null: 'not-a-boolean' } }],
});
expect(result.success).toBe(false);
expect(result.error?.issues[0]?.code).toBe('invalid_union');
expect(result.error?.issues[0]?.path).toEqual(['$and', 0]);
expect(result.error?.issues[0]?.message).toContain('Not a valid $and member');
// Message-bearing per lane convention: it names what was rejected and
// what the two valid member shapes are.
expect(result.error?.issues[0]?.message).toContain('"c"');
expect(result.error?.issues[0]?.message).toContain('FieldOperatorsSchema');
expect(result.error?.issues[0]?.message).toContain('$and / $or / $not');
// The other face always said no — that disagreement is the defect.
expect(FieldOperatorsSchema.safeParse({ $null: 'not-a-boolean' }).success).toBe(false);
});

it('rejects the same shape in $or and in the $not operand', () => {
const inOr = NormalizedFilterSchema.safeParse({ $or: [{ c: { $between: [1, 2, 3] } }] });
const inNot = NormalizedFilterSchema.safeParse({ $not: { c: { $null: 'nope' } } });
expect(inOr.success).toBe(false);
expect(inNot.success).toBe(false);
expect(inOr.error?.issues[0]?.message).toContain('Not a valid $or member');
expect(inNot.error?.issues[0]?.message).toContain('Not a valid $not operand');
expect(FieldOperatorsSchema.safeParse({ $between: [1, 2, 3] }).success).toBe(false);
});

it('rejects a bad member nested inside a legitimate group', () => {
const result = NormalizedFilterSchema.safeParse({
$and: [{ $or: [{ c: { $null: 'nope' } }] }],
});
expect(result.success).toBe(false);
});

/**
* The `$not`-as-a-field-name leak, and why the field-condition branch has to
* refuse `$`-keys rather than the group branch merely being `.strict()`.
* `FieldOperatorsSchema` is not `.strict()`, so `{ c: … }` read as an
* OPERATOR MAP has every key stripped and returns `{}` — which made
* `{ $not: <anything> }` parse as a field condition named `$not`.
*/
it('does not let a $not group be read as a field named "$not"', () => {
const result = NormalizedFilterSchema.safeParse({
$and: [{ $not: { c: { $null: 'nope' } } }],
});
expect(result.success).toBe(false);
expect(result.error?.issues[0]?.message).toContain('Not a valid $and member');
});

it('rejects members that are not field conditions at all', () => {
// Arbitrary keys — the shape the catch-all could never refuse.
expect(NormalizedFilterSchema.safeParse({ $and: [{ hello: 'world' }] }).success).toBe(false);
// Implicit equality is AUTHORING sugar; this AST is post-normalization,
// so `{ age: 18 }` is `{ age: { $eq: 18 } }` by the time it gets here.
expect(NormalizedFilterSchema.safeParse({ $and: [{ age: 18 }] }).success).toBe(false);
// A group key and a field key at one level. Pre-#7711 this parsed to
// `{ $and: [{ $or: [] }] }` — the field condition dropped on the floor —
// so no mixed member ever survived a round-trip to begin with.
expect(NormalizedFilterSchema.safeParse({ $and: [{ $or: [], c: { $eq: 1 } }] }).success)
.toBe(false);
});

it('rejects unknown keys at the top level', () => {
const bogusCombinator = NormalizedFilterSchema.safeParse({ $nand: [] });
expect(bogusCombinator.success).toBe(false);
expect(bogusCombinator.error?.issues[0]?.code).toBe('unrecognized_keys');
// A field condition is not a top-level normalized filter either; the AST's
// root is a group. Pre-#7711 this parsed to `{}`.
expect(NormalizedFilterSchema.safeParse({ c: { $null: 'nope' } }).success).toBe(false);
});

/**
* The empty combinators are boolean IDENTITIES under the #5322 ruling
* documented on `FilterConditionSchema`, not malformed members. `{}` reaches
* the field-condition branch as an empty record, which is what lets the
* group branch be `.strict()` without touching any of them.
*/
it('leaves the #5322 empty-combinator identities accepted', () => {
for (const identity of [{}, { $and: [] }, { $or: [] }, { $or: [{}] }, { $not: {} }]) {
expect(NormalizedFilterSchema.safeParse(identity).success).toBe(true);
}
});

/**
* The accept side of the same fix. The catch-all did not merely admit these
* shapes' bad siblings — it ERASED the member it admitted, so a `$not`
* subtree came back as `{}`. Judged by the right branch, the parse output
* now carries what went in.
*/
it('preserves a nested group in the parse output instead of erasing it', () => {
const input = { $and: [{ $not: { deleted: { $eq: true } } }] };
const result = NormalizedFilterSchema.safeParse(input);
expect(result.success).toBe(true);
// Pre-#7711 this was `{ $and: [{ $not: {} }] }` — green, and empty.
expect(result.data).toEqual(input);
});
});
});

// ============================================================================
Expand Down
Loading
Loading