Skip to content

Commit d01ae81

Browse files
committed
fix(data): gate unknown fields on the explicit filter axes (#7534)
`POST /data/:object/query` with `{"where":{"not_a_field":"x"}}` answered `200 {records:[],total:0}` — and identically through the `$filter` door and the filter-AST door — while the bare-key door on the same object and the same field name answered `400 INVALID_FIELD`. One endpoint family, two verdicts for one mistake, and the losing one is indistinguishable from "no data". Not a regression of #4134: the bare-key control still passes at the branch point (measured alongside the three failures). It is the sibling door that fix never reached — `assertQueryParamsAreFields` gated only the implicit filters derived from leftover query params, while the explicit axes reached the driver ungated. `assertFilterFieldsExist` calls the existing `resolveQueryFields` — additively; that shared helper is unchanged — on the normalized `where`. One call covers all three doors because they fold to one slot (#3795) and the AST is lowered by `parseFilterAST` before the gate runs, so it reads the same `FilterCondition` the driver reads. Ordering is deliberately unmoved: after the #4134 param gate (so existing precedence holds) and before the #4164 merge (so the rejection can name the axis the caller used). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NxE7c6qf7Bi9ZQ7HtYrNUj
1 parent 9051802 commit d01ae81

3 files changed

Lines changed: 626 additions & 0 deletions

File tree

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
---
2+
"@objectstack/metadata-protocol": patch
3+
---
4+
5+
fix(data): an unknown field inside `where` / `$filter` / a filter AST is rejected, not answered with an empty list (#7534)
6+
7+
`POST /api/v1/data/showcase_invoice/query` with `{"where":{"not_a_field":"x"}}`
8+
answered `200 {"records":[],"total":0}` — no `code`, no mention of the unknown
9+
name — and identically through the `$filter` door and the filter-AST door. The
10+
bare-key door on the same object with the same field name, in the same run,
11+
answered `400 INVALID_FIELD`.
12+
13+
So one endpoint family gave **two verdicts for one mistake**, chosen by which
14+
door the caller used, and the losing verdict is indistinguishable from "no
15+
data". That is the exact failure #4134 was filed about: an unknown name is
16+
lowered into a field-equality predicate that can only match zero rows.
17+
18+
This is **not** a regression of #4134 — that gate still holds on the door it
19+
covers (measured at the branch point alongside the three failures). It is the
20+
sibling door its fix never reached: `assertQueryParamsAreFields` gated only the
21+
**implicit** filters `findData` derives from leftover query parameters, while
22+
the **explicit** axes reached the driver ungated — even though
23+
`resolveQueryFields` was written as "ONE resolution shared by all four read
24+
axes".
25+
26+
**The gate.** A new `assertFilterFieldsExist` calls that same existing
27+
resolution — additively; `resolveQueryFields` itself is unchanged — on the
28+
normalized `where`. One call covers all three doors because they are not three
29+
code paths: `where` / `filter` / `filters` / `$filter` resolve to one slot at
30+
the #3795 fold, and a filter AST is lowered by `parseFilterAST` — the single
31+
sink for that sugar — before the gate runs. The gate therefore reads the same
32+
`FilterCondition` the driver will read, which is what keeps "the field the gate
33+
saw" from drifting away from "the column that reached the driver".
34+
35+
Rejections carry the envelope the write path and the bare-key door already
36+
produce — `400 INVALID_FIELD` + `field` + `fields` + `object` — plus `param`
37+
naming the caller's own wire spelling (`$filter`, not `where`), and a message
38+
that states the zero-row consequence, since that is the part a caller cannot
39+
infer from a `200`.
40+
41+
**Deliberately unchanged.**
42+
43+
- **Precedence.** The gate runs *after* the #4134 param gate, so a request that
44+
gets both a bare key and its filter wrong answers exactly as it did before;
45+
and *before* the #4164 implicit/explicit merge, which is what still lets it
46+
name the axis the caller actually used.
47+
- **Reach.** Structure is discarded — `$and` / `$or` / `$not` are recursed
48+
into — but a field key's VALUE is not descended into: it is either an operator
49+
bag (`{$gte: 18}`) or a nested-relation condition (`{owner_id: {region:
50+
'NA'}}`) whose keys belong to a *different* object. Judging those against this
51+
object's field map would refuse legitimate relation filters. A dotted path is
52+
judged on its head segment, the same reach the bare-key door has on
53+
`owner_id.name`. An unrecognised `$`-combinator is skipped without descending —
54+
a hole rather than a false rejection, the right failure direction for a gate
55+
that exists to stop wrong answers.
56+
- **The honest zero.** A real field that genuinely matches nothing is still a
57+
`200` with `total: 0`. A filter that cannot be *run* at all is still
58+
`INVALID_FILTER` (#4121 / #4181), which answers first; this gate answers only
59+
"does this field exist".

packages/metadata-protocol/src/protocol.ts

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2174,6 +2174,69 @@ function suggestFieldName(name: string, knownFields: readonly string[]): string
21742174
return '';
21752175
}
21762176

2177+
/**
2178+
* [#7534] The logical combinators a `FilterCondition` may carry. These hold
2179+
* NESTED CONDITIONS rather than naming a field, so {@link collectFilterFieldKeys}
2180+
* descends through them instead of judging them.
2181+
*
2182+
* Exactly the three the contract declares (`FilterConditionSchema`,
2183+
* `@objectstack/spec`) — `$and` / `$or` / `$not`. `$nor` is deliberately absent:
2184+
* it is a driver-INTERNAL lowering (`driver-memory` rewrites an input `$not`
2185+
* into a one-operand `$nor`, MongoDB's document-level negation) and is REFUSED
2186+
* as input vocabulary by that same driver, so a `$nor` arriving on the wire is
2187+
* not a combinator this layer should silently descend into.
2188+
*/
2189+
const FILTER_LOGICAL_KEYS: ReadonlySet<string> = new Set(['$and', '$or', '$not']);
2190+
2191+
/**
2192+
* [#7534] Every key of a `FilterCondition` that NAMES A FIELD, structure
2193+
* discarded — whether a predicate sits under an `$or` changes nothing about
2194+
* whether its column exists.
2195+
*
2196+
* Two rules, and both are deliberately conservative in the direction that
2197+
* cannot invent a rejection:
2198+
*
2199+
* - **A `$`-prefixed key is never a field.** `$and`/`$or`/`$not` are recursed
2200+
* into; any OTHER `$` key is skipped WITHOUT descending. An unrecognised
2201+
* combinator therefore leaves the fields beneath it ungated — a hole, not a
2202+
* false 400 — which is the right failure direction for a gate whose whole
2203+
* purpose is to stop wrong answers, not to invent new ones.
2204+
* - **A field key's VALUE is not descended into.** It is either an operator bag
2205+
* (`{$gte: 18}`) or a nested-relation condition (`{owner: {region: 'NA'}}`),
2206+
* and the latter's keys belong to a DIFFERENT object whose field map this
2207+
* gate has not resolved. Judging them against THIS object's fields would
2208+
* refuse legitimate relation filters. The head segment — `owner` — is a field
2209+
* of this object and IS judged, which is the same reach
2210+
* {@link ObjectStackProtocolImplementation.assertQueryParamsAreFields} has on
2211+
* a dotted path (`owner_id.name`).
2212+
*
2213+
* `depth` is a cheap backstop against a self-referential `where`. JSON cannot
2214+
* produce one, but `POST /data/:object/query` is not the only door — the RPC
2215+
* dispatcher and in-process callers hand over live objects — and a gate that
2216+
* can hang the read path is worse than the defect it closes.
2217+
*/
2218+
function collectFilterFieldKeys(
2219+
where: unknown,
2220+
out: string[] = [],
2221+
depth = 0,
2222+
): string[] {
2223+
if (depth > 32) return out;
2224+
if (!where || typeof where !== 'object' || Array.isArray(where)) return out;
2225+
for (const [key, value] of Object.entries(where as Record<string, unknown>)) {
2226+
if (key.startsWith('$')) {
2227+
if (!FILTER_LOGICAL_KEYS.has(key)) continue;
2228+
if (Array.isArray(value)) {
2229+
for (const arm of value) collectFilterFieldKeys(arm, out, depth + 1);
2230+
} else {
2231+
collectFilterFieldKeys(value, out, depth + 1);
2232+
}
2233+
continue;
2234+
}
2235+
out.push(key);
2236+
}
2237+
return out;
2238+
}
2239+
21772240
/**
21782241
* Service Configuration for Discovery
21792242
* Maps service names to their routes and plugin providers.
@@ -5183,6 +5246,84 @@ export class ObjectStackProtocolImplementation implements
51835246
throw err;
51845247
}
51855248

5249+
/**
5250+
* [#7534] The same read-path gate, on the EXPLICIT filter axes — the `where`
5251+
* object, the `$filter` string and the filter AST.
5252+
*
5253+
* #4134 closed this defect for the filters `findData` DERIVES from leftover
5254+
* query parameters, and {@link resolveQueryFields} was written for "ONE
5255+
* resolution shared by all four read axes". The explicit axes never called
5256+
* it, so one endpoint family answered ONE mistake two ways, chosen by which
5257+
* door the caller used:
5258+
*
5259+
* ```
5260+
* GET /data/showcase_invoice?not_a_field=x -> 400 INVALID_FIELD
5261+
* POST /data/showcase_invoice/query {where:{not_a_field:'x'}} -> 200 {records:[],total:0}
5262+
* ```
5263+
*
5264+
* The losing answer is the exact failure #4134 was filed about: an unknown
5265+
* name lowers into a field-equality predicate that can only match zero rows,
5266+
* so the response is indistinguishable from "no data" — and it cost a real
5267+
* investigation once already, where an empty list was read as an RLS /
5268+
* org-scope visibility bug rather than a typo.
5269+
*
5270+
* ONE call covers all three doors because they are not three code paths:
5271+
* `where` / `filter` / `filters` / `$filter` resolve to one slot at the
5272+
* #3795 fold, and a filter AST is lowered by `parseFilterAST` — the single
5273+
* sink for that sugar — before this runs. So this gate reads the same
5274+
* `FilterCondition` the driver will read, which is what keeps "the field the
5275+
* gate saw" and "the column that reached the driver" from drifting apart.
5276+
*
5277+
* # Ordering: after the #4134 param gate, before the #4164 merge
5278+
*
5279+
* Deliberately NOT reordered relative to its siblings. Running it AFTER
5280+
* {@link assertQueryParamsAreFields} keeps that gate's verdict first when a
5281+
* request gets both wrong, so no existing precedence moves; running it
5282+
* BEFORE the #4164 implicit/explicit merge is what lets it name the axis the
5283+
* caller actually used, since after the merge the two are one `$and` and the
5284+
* distinction is gone.
5285+
*
5286+
* # What it does NOT do
5287+
*
5288+
* The `param` in the message is the caller's own wire spelling (#4226's
5289+
* discipline — telling someone who sent `?$filter=…` that "'where' is
5290+
* invalid" names a parameter absent from their request). The message states
5291+
* the zero-row consequence rather than just the bad name, because that is
5292+
* the part a caller cannot infer from a `200`.
5293+
*
5294+
* Value shapes are NOT judged here: a wrong-typed or unrunnable filter is
5295+
* `INVALID_FILTER`'s job (#4121 / #4181), already answered upstream in this
5296+
* same block. This gate answers exactly one question — does this field
5297+
* exist — with exactly the envelope the write path and the bare-key door
5298+
* already give it.
5299+
*/
5300+
private assertFilterFieldsExist(object: string, where: unknown, param: string): void {
5301+
if (!where || typeof where !== 'object') return;
5302+
const names = collectFilterFieldKeys(where);
5303+
if (names.length === 0) return;
5304+
const gate = this.resolveQueryFields(object);
5305+
if (!gate) return;
5306+
// Head segment only, exactly as the bare-key door judges `owner_id.name`.
5307+
const unknown = names.filter((f) => !gate.known.has(f.split('.')[0]));
5308+
if (unknown.length === 0) return;
5309+
const first = unknown[0];
5310+
const err: any = new Error(
5311+
`Query parameter '${param}' filters on '${first}', which is not a field on object `
5312+
+ `'${object}'`
5313+
+ (unknown.length > 1 ? ` (also: ${unknown.slice(1).join(', ')})` : '')
5314+
+ '. A filter on a field that does not exist can only match zero records, so the '
5315+
+ 'query was refused instead of answered with an empty list.'
5316+
+ suggestFieldName(first, gate.declared),
5317+
);
5318+
err.code = 'INVALID_FIELD';
5319+
err.status = 400;
5320+
err.field = first;
5321+
err.fields = unknown;
5322+
err.object = object;
5323+
err.param = param;
5324+
throw err;
5325+
}
5326+
51865327
/**
51875328
* [#4226] SORT axis. A sort naming a field the object does not have is
51885329
* refused (`400 INVALID_SORT`) instead of being dropped on the floor.
@@ -6293,6 +6434,15 @@ export class ObjectStackProtocolImplementation implements
62936434
this.assertQueryParamsAreFields(request.object, leftoverParams);
62946435
}
62956436

6437+
// [#7534] The same question, on the EXPLICIT filter the caller wrote —
6438+
// the sibling door #4134's fix never reached. `options.where` is a
6439+
// lowered `FilterCondition` by this point whichever of the three doors
6440+
// carried it (`where` object, `$filter` string, filter AST), so one call
6441+
// covers all three. Placed here, and not earlier, on purpose: see
6442+
// `assertFilterFieldsExist` for why it runs after the param gate above
6443+
// and before the #4164 merge below.
6444+
this.assertFilterFieldsExist(request.object, options.where, filterKey);
6445+
62966446
// Flat field filters: REST-style query params like ?id=abc&status=open
62976447
// are implicit field-level equality predicates. Every leftover key is a
62986448
// verified field name by this point — the #4134 gate above runs FIRST,

0 commit comments

Comments
 (0)