From d01ae810a404cd1b3acefb2659723eeeb5f18ebe Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 06:09:23 +0000 Subject: [PATCH 1/2] fix(data): gate unknown fields on the explicit filter axes (#7534) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 Claude-Session: https://claude.ai/code/session_01NxE7c6qf7Bi9ZQ7HtYrNUj --- ...rest-list-explicit-filter-unknown-field.md | 59 +++ packages/metadata-protocol/src/protocol.ts | 150 +++++++ ...rotocol-explicit-filter-field-gate.test.ts | 417 ++++++++++++++++++ 3 files changed, 626 insertions(+) create mode 100644 .changeset/rest-list-explicit-filter-unknown-field.md create mode 100644 packages/objectql/src/protocol-explicit-filter-field-gate.test.ts diff --git a/.changeset/rest-list-explicit-filter-unknown-field.md b/.changeset/rest-list-explicit-filter-unknown-field.md new file mode 100644 index 0000000000..a2f09bdcbe --- /dev/null +++ b/.changeset/rest-list-explicit-filter-unknown-field.md @@ -0,0 +1,59 @@ +--- +"@objectstack/metadata-protocol": patch +--- + +fix(data): an unknown field inside `where` / `$filter` / a filter AST is rejected, not answered with an empty list (#7534) + +`POST /api/v1/data/showcase_invoice/query` with `{"where":{"not_a_field":"x"}}` +answered `200 {"records":[],"total":0}` — no `code`, no mention of the unknown +name — and identically through the `$filter` door and the filter-AST door. The +bare-key door on the same object with the same field name, in the same run, +answered `400 INVALID_FIELD`. + +So one endpoint family gave **two verdicts for one mistake**, chosen by which +door the caller used, and the losing verdict is indistinguishable from "no +data". That is the exact failure #4134 was filed about: an unknown name is +lowered into a field-equality predicate that can only match zero rows. + +This is **not** a regression of #4134 — that gate still holds on the door it +covers (measured at the branch point alongside the three failures). It is the +sibling door its fix never reached: `assertQueryParamsAreFields` gated only the +**implicit** filters `findData` derives from leftover query parameters, while +the **explicit** axes reached the driver ungated — even though +`resolveQueryFields` was written as "ONE resolution shared by all four read +axes". + +**The gate.** A new `assertFilterFieldsExist` calls that same existing +resolution — additively; `resolveQueryFields` itself is unchanged — on the +normalized `where`. One call covers all three doors because they are not three +code paths: `where` / `filter` / `filters` / `$filter` resolve to one slot at +the #3795 fold, and a filter AST is lowered by `parseFilterAST` — the single +sink for that sugar — before the gate runs. The gate therefore reads the same +`FilterCondition` the driver will read, which is what keeps "the field the gate +saw" from drifting away from "the column that reached the driver". + +Rejections carry the envelope the write path and the bare-key door already +produce — `400 INVALID_FIELD` + `field` + `fields` + `object` — plus `param` +naming the caller's own wire spelling (`$filter`, not `where`), and a message +that states the zero-row consequence, since that is the part a caller cannot +infer from a `200`. + +**Deliberately unchanged.** + +- **Precedence.** The gate runs *after* the #4134 param gate, so a request that + gets both a bare key and its filter wrong answers exactly as it did before; + and *before* the #4164 implicit/explicit merge, which is what still lets it + name the axis the caller actually used. +- **Reach.** Structure is discarded — `$and` / `$or` / `$not` are recursed + into — but a field key's VALUE is not descended into: it is either an operator + bag (`{$gte: 18}`) or a nested-relation condition (`{owner_id: {region: + 'NA'}}`) whose keys belong to a *different* object. Judging those against this + object's field map would refuse legitimate relation filters. A dotted path is + judged on its head segment, the same reach the bare-key door has on + `owner_id.name`. An unrecognised `$`-combinator is skipped without descending — + a hole rather than a false rejection, the right failure direction for a gate + that exists to stop wrong answers. +- **The honest zero.** A real field that genuinely matches nothing is still a + `200` with `total: 0`. A filter that cannot be *run* at all is still + `INVALID_FILTER` (#4121 / #4181), which answers first; this gate answers only + "does this field exist". diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 7ee497373a..ed2967f59c 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -2174,6 +2174,69 @@ function suggestFieldName(name: string, knownFields: readonly string[]): string return ''; } +/** + * [#7534] The logical combinators a `FilterCondition` may carry. These hold + * NESTED CONDITIONS rather than naming a field, so {@link collectFilterFieldKeys} + * descends through them instead of judging them. + * + * Exactly the three the contract declares (`FilterConditionSchema`, + * `@objectstack/spec`) — `$and` / `$or` / `$not`. `$nor` is deliberately absent: + * it is a driver-INTERNAL lowering (`driver-memory` rewrites an input `$not` + * into a one-operand `$nor`, MongoDB's document-level negation) and is REFUSED + * as input vocabulary by that same driver, so a `$nor` arriving on the wire is + * not a combinator this layer should silently descend into. + */ +const FILTER_LOGICAL_KEYS: ReadonlySet = new Set(['$and', '$or', '$not']); + +/** + * [#7534] Every key of a `FilterCondition` that NAMES A FIELD, structure + * discarded — whether a predicate sits under an `$or` changes nothing about + * whether its column exists. + * + * Two rules, and both are deliberately conservative in the direction that + * cannot invent a rejection: + * + * - **A `$`-prefixed key is never a field.** `$and`/`$or`/`$not` are recursed + * into; any OTHER `$` key is skipped WITHOUT descending. An unrecognised + * combinator therefore leaves the fields beneath it ungated — a hole, not a + * false 400 — which is the right failure direction for a gate whose whole + * purpose is to stop wrong answers, not to invent new ones. + * - **A field key's VALUE is not descended into.** It is either an operator bag + * (`{$gte: 18}`) or a nested-relation condition (`{owner: {region: 'NA'}}`), + * and the latter's keys belong to a DIFFERENT object whose field map this + * gate has not resolved. Judging them against THIS object's fields would + * refuse legitimate relation filters. The head segment — `owner` — is a field + * of this object and IS judged, which is the same reach + * {@link ObjectStackProtocolImplementation.assertQueryParamsAreFields} has on + * a dotted path (`owner_id.name`). + * + * `depth` is a cheap backstop against a self-referential `where`. JSON cannot + * produce one, but `POST /data/:object/query` is not the only door — the RPC + * dispatcher and in-process callers hand over live objects — and a gate that + * can hang the read path is worse than the defect it closes. + */ +function collectFilterFieldKeys( + where: unknown, + out: string[] = [], + depth = 0, +): string[] { + if (depth > 32) return out; + if (!where || typeof where !== 'object' || Array.isArray(where)) return out; + for (const [key, value] of Object.entries(where as Record)) { + if (key.startsWith('$')) { + if (!FILTER_LOGICAL_KEYS.has(key)) continue; + if (Array.isArray(value)) { + for (const arm of value) collectFilterFieldKeys(arm, out, depth + 1); + } else { + collectFilterFieldKeys(value, out, depth + 1); + } + continue; + } + out.push(key); + } + return out; +} + /** * Service Configuration for Discovery * Maps service names to their routes and plugin providers. @@ -5183,6 +5246,84 @@ export class ObjectStackProtocolImplementation implements throw err; } + /** + * [#7534] The same read-path gate, on the EXPLICIT filter axes — the `where` + * object, the `$filter` string and the filter AST. + * + * #4134 closed this defect for the filters `findData` DERIVES from leftover + * query parameters, and {@link resolveQueryFields} was written for "ONE + * resolution shared by all four read axes". The explicit axes never called + * it, so one endpoint family answered ONE mistake two ways, chosen by which + * door the caller used: + * + * ``` + * GET /data/showcase_invoice?not_a_field=x -> 400 INVALID_FIELD + * POST /data/showcase_invoice/query {where:{not_a_field:'x'}} -> 200 {records:[],total:0} + * ``` + * + * The losing answer is the exact failure #4134 was filed about: an unknown + * name lowers into a field-equality predicate that can only match zero rows, + * so the response is indistinguishable from "no data" — and it cost a real + * investigation once already, where an empty list was read as an RLS / + * org-scope visibility bug rather than a typo. + * + * ONE call covers all three doors because they are not three code paths: + * `where` / `filter` / `filters` / `$filter` resolve to one slot at the + * #3795 fold, and a filter AST is lowered by `parseFilterAST` — the single + * sink for that sugar — before this runs. So this gate reads the same + * `FilterCondition` the driver will read, which is what keeps "the field the + * gate saw" and "the column that reached the driver" from drifting apart. + * + * # Ordering: after the #4134 param gate, before the #4164 merge + * + * Deliberately NOT reordered relative to its siblings. Running it AFTER + * {@link assertQueryParamsAreFields} keeps that gate's verdict first when a + * request gets both wrong, so no existing precedence moves; running it + * BEFORE the #4164 implicit/explicit merge is what lets it name the axis the + * caller actually used, since after the merge the two are one `$and` and the + * distinction is gone. + * + * # What it does NOT do + * + * The `param` in the message is the caller's own wire spelling (#4226's + * discipline — telling someone who sent `?$filter=…` that "'where' is + * invalid" names a parameter absent from their request). The message states + * the zero-row consequence rather than just the bad name, because that is + * the part a caller cannot infer from a `200`. + * + * Value shapes are NOT judged here: a wrong-typed or unrunnable filter is + * `INVALID_FILTER`'s job (#4121 / #4181), already answered upstream in this + * same block. This gate answers exactly one question — does this field + * exist — with exactly the envelope the write path and the bare-key door + * already give it. + */ + private assertFilterFieldsExist(object: string, where: unknown, param: string): void { + if (!where || typeof where !== 'object') return; + const names = collectFilterFieldKeys(where); + if (names.length === 0) return; + const gate = this.resolveQueryFields(object); + if (!gate) return; + // Head segment only, exactly as the bare-key door judges `owner_id.name`. + const unknown = names.filter((f) => !gate.known.has(f.split('.')[0])); + if (unknown.length === 0) return; + const first = unknown[0]; + const err: any = new Error( + `Query parameter '${param}' filters on '${first}', which is not a field on object ` + + `'${object}'` + + (unknown.length > 1 ? ` (also: ${unknown.slice(1).join(', ')})` : '') + + '. A filter on a field that does not exist can only match zero records, so the ' + + 'query was refused instead of answered with an empty list.' + + suggestFieldName(first, gate.declared), + ); + err.code = 'INVALID_FIELD'; + err.status = 400; + err.field = first; + err.fields = unknown; + err.object = object; + err.param = param; + throw err; + } + /** * [#4226] SORT axis. A sort naming a field the object does not have is * refused (`400 INVALID_SORT`) instead of being dropped on the floor. @@ -6293,6 +6434,15 @@ export class ObjectStackProtocolImplementation implements this.assertQueryParamsAreFields(request.object, leftoverParams); } + // [#7534] The same question, on the EXPLICIT filter the caller wrote — + // the sibling door #4134's fix never reached. `options.where` is a + // lowered `FilterCondition` by this point whichever of the three doors + // carried it (`where` object, `$filter` string, filter AST), so one call + // covers all three. Placed here, and not earlier, on purpose: see + // `assertFilterFieldsExist` for why it runs after the param gate above + // and before the #4164 merge below. + this.assertFilterFieldsExist(request.object, options.where, filterKey); + // Flat field filters: REST-style query params like ?id=abc&status=open // are implicit field-level equality predicates. Every leftover key is a // verified field name by this point — the #4134 gate above runs FIRST, diff --git a/packages/objectql/src/protocol-explicit-filter-field-gate.test.ts b/packages/objectql/src/protocol-explicit-filter-field-gate.test.ts new file mode 100644 index 0000000000..88ee728a6b --- /dev/null +++ b/packages/objectql/src/protocol-explicit-filter-field-gate.test.ts @@ -0,0 +1,417 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #7534 — the EXPLICIT filter axes had no field-existence gate, so one endpoint + * family answered ONE mistake two ways depending on which door the caller used: + * + * ``` + * GET /data/showcase_task?not_a_field=x -> 400 INVALID_FIELD + * POST /data/showcase_task/query {"where":{"not_a_field":"x"}} -> 200 {records:[],total:0} + * ``` + * + * Measured at the branch point, all four doors on one object and one field + * name: the bare-key control REJECTED (400/INVALID_FIELD), while the `where` + * object, the `$filter` string and the filter AST each RESOLVED with + * `total: 0`. So this is NOT a regression of #4134 — that gate still holds on + * the door it covers. It is the sibling door its fix never reached. + * + * The losing answer is the exact failure #4134 was filed about: an unknown name + * lowers into a field-equality predicate that can only match zero rows, and the + * response is indistinguishable from "this object really is empty". + * + * Harness shape is deliberately the same as `protocol-unknown-query-param.test.ts` + * — a REAL {@link ObjectQL} engine rather than an engine double — because the + * gate's authority is the REGISTRY's field map, not the author's declaration: + * `applySystemFields` injects the audit / tenant / owner columns at + * registration, and only a real registry can show that `owner_id` still filters + * through the explicit door while `not_a_field` is refused. + * + * Tests marked GUARD pass in BOTH directions (with the fix and with it + * reverted). They are here to pin what must NOT change — the #4134 control, the + * honest zero, the gate's reach — not to demonstrate the fix. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { ObjectQL } from './engine.js'; + +/** 10 rows, 2 of them `status: 'done'`. */ +const taskObject = { + name: 'showcase_task', + label: 'Task', + fields: { + id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true }, + title: { name: 'title', label: 'Title', type: 'text' as const, required: true }, + status: { name: 'status', label: 'Status', type: 'text' as const }, + }, +}; + +function makeStubDriver() { + const stores = new Map>>(); + const storeFor = (obj: string) => { + let s = stores.get(obj); + if (!s) { s = new Map(); stores.set(obj, s); } + return s; + }; + let nextId = 0; + const matchesWhere = (row: Record, where: any): boolean => { + if (!where || typeof where !== 'object') return true; + for (const [k, v] of Object.entries(where)) { + // `$and` / `$or` must be honored, not skipped — #4164 composes the + // explicit filter and the implicit field predicates through `$and`, + // and this suite files unknown names under `$or` / `$not` too. A + // matcher that ignored them would green-light a filter that never ran. + if (k === '$and' && Array.isArray(v)) { + if (!v.every((arm) => matchesWhere(row, arm))) return false; + continue; + } + if (k === '$or' && Array.isArray(v)) { + if (!v.some((arm) => matchesWhere(row, arm))) return false; + continue; + } + if (k === '$not') { + if (matchesWhere(row, v)) return false; + continue; + } + if (k.startsWith('$')) continue; + const expected = (v && typeof v === 'object' && '$eq' in (v as any)) ? (v as any).$eq : v; + const a = row[k] === undefined ? null : row[k]; + const b = expected === undefined ? null : expected; + if (a !== b) return false; + } + return true; + }; + const driver: any = { + name: 'memory', + version: '0.0.0', + supports: {} as any, + async connect() {}, + async disconnect() {}, + async checkHealth() { return true; }, + async execute() { return null; }, + async find(object: string, ast: any) { + const all = Array.from(storeFor(object).values()).filter((r) => matchesWhere(r, ast?.where)); + const from = typeof ast?.offset === 'number' ? ast.offset : 0; + return typeof ast?.limit === 'number' ? all.slice(from, from + ast.limit) : all.slice(from); + }, + async findOne(object: string, ast: any) { + for (const r of storeFor(object).values()) if (matchesWhere(r, ast?.where)) return r; + return null; + }, + async create(object: string, data: Record) { + nextId += 1; + const id = (data.id as string) ?? `r_${nextId}`; + const row = { ...data, id }; + storeFor(object).set(id, row); + return row; + }, + async update(object: string, id: string, data: Record) { + const s = storeFor(object); + const cur = s.get(id); + if (!cur) throw new Error(`not found: ${object}/${id}`); + const updated = { ...cur, ...data, id }; + s.set(id, updated); + return updated; + }, + async upsert(object: string, data: Record) { + const id = data.id as string | undefined; + if (id && storeFor(object).has(id)) return this.update(object, id, data); + return this.create(object, data); + }, + async delete(object: string, id: string) { return storeFor(object).delete(id); }, + async count(object: string, ast: any) { + return Array.from(storeFor(object).values()).filter((r) => matchesWhere(r, ast?.where)).length; + }, + async aggregate(object: string, ast: any) { return [{ count: await this.count(object, ast) }]; }, + async bulkCreate(object: string, rows: Record[]) { + return Promise.all(rows.map((r) => this.create(object, r))); + }, + async bulkUpdate() { return []; }, + async bulkDelete() {}, + async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, + async rollback() {}, + }; + return { driver, stores }; +} + +/** + * The three EXPLICIT doors of the issue, each spelled the way a caller reaches + * it, as a function of the field name under test. + * + * Kept as ONE table so every door-agreement assertion below is driven from it + * rather than from a hand-copied list — the copies are how two doors drift into + * two verdicts in the first place, which is the entire subject of this issue. + */ +const EXPLICIT_DOORS: ReadonlyArray<{ + door: string, + param: string, + /** Both the field AND the value are parameters — a door whose value is + * patched in afterwards by string surgery is a fixture that can silently + * miss (the `$filter` door nests JSON inside JSON, so its quotes are + * escaped and a naive replace does nothing but leave the test green-looking + * and wrong). */ + query: (field: string, value: string) => any, +}> = [ + // `where` object — `POST /data/:object/query` body. + { door: 'where object', param: 'where', query: (f, v) => ({ where: { [f]: v } }) }, + // `$filter` string — the OData spelling, JSON in a querystring value. + { door: '$filter string', param: '$filter', query: (f, v) => ({ $filter: JSON.stringify({ [f]: v }) }) }, + // Filter AST — the `FilterArray` sugar the ObjectUI client and FilterBuilder emit. + { door: 'filter AST', param: 'filter', query: (f, v) => ({ filter: [[f, '=', v]] }) }, +]; + +describe('#7534 — unknown field on the EXPLICIT filter axes (real ObjectQL engine)', () => { + let protocol: ObjectStackProtocolImplementation; + + beforeEach(async () => { + const engine = new ObjectQL(); + const { driver, stores } = makeStubDriver(); + engine.registerDriver(driver, true); + await engine.init(); + engine.registry.registerObject(taskObject, 'test-package'); + protocol = new ObjectStackProtocolImplementation(engine); + + const rows = new Map>(); + for (let i = 1; i <= 10; i++) { + rows.set(`t${i}`, { + id: `t${i}`, + title: `Task ${i}`, + status: i <= 2 ? 'done' : 'open', + owner_id: 'usr_1', + created_at: '2026-07-30T00:00:00.000Z', + }); + } + stores.set('showcase_task', rows); + }); + + const find = (query?: any) => protocol.findData({ object: 'showcase_task', query }); + + // ───────────────────────────────────────────────────────────── + // Positive identity FIRST — every rejection below is only meaningful + // against a door that demonstrably works when the field is real. + // ───────────────────────────────────────────────────────────── + + it('GUARD baseline — no filter returns every row', async () => { + await expect(find()).resolves.toMatchObject({ total: 10 }); + }); + + it('GUARD every explicit door APPLIES a real filter — the positive identity the refusals are measured against', async () => { + // Scale guard: if a door is added to the table and not to the issue's + // coverage, this count is what says so. + expect(EXPLICIT_DOORS).toHaveLength(3); + for (const { door, query } of EXPLICIT_DOORS) { + const r: any = await find(query('status', 'done')); + expect(r.total, `${door} must apply a real filter`).toBe(2); + expect( + r.records.map((x: any) => x.id).sort(), + `${door} must return the two 'done' rows, not an arbitrary page`, + ).toEqual(['t1', 't2']); + } + }); + + // ───────────────────────────────────────────────────────────── + // The defect: each door, on its own, for the issue's own field name + // ───────────────────────────────────────────────────────────── + + it.each(EXPLICIT_DOORS)( + 'the $door door refuses an unknown field with 400 INVALID_FIELD instead of 200 + an empty list', + async ({ param, query }) => { + await expect(find(query('not_a_field', 'x'))).rejects.toMatchObject({ + status: 400, + code: 'INVALID_FIELD', + field: 'not_a_field', + object: 'showcase_task', + param, + }); + }, + ); + + it('the AST single-comparison and and-group spellings are refused too, not just the flat list', async () => { + // `['not_a_field','=','x']` and `['and', …]` are separate branches of + // `parseFilterAST`; the flat-list case above exercises neither. + for (const filter of [ + ['not_a_field', '=', 'x'], + ['and', ['status', '=', 'open'], ['not_a_field', '=', 'x']], + ]) { + await expect(find({ filter })).rejects.toMatchObject({ + status: 400, code: 'INVALID_FIELD', field: 'not_a_field', + }); + } + }); + + it('an unknown field is found however deeply the combinators bury it', async () => { + // Structure is discarded when collecting field keys: whether a predicate + // sits under an `$or` changes nothing about whether its column exists. + for (const where of [ + { $and: [{ status: 'open' }, { not_a_field: 'x' }] }, + { $or: [{ status: 'open' }, { not_a_field: 'x' }] }, + { $not: { not_a_field: 'x' } }, + { $and: [{ $or: [{ $not: { not_a_field: 'x' } }] }] }, + ]) { + await expect(find({ where })).rejects.toMatchObject({ + status: 400, code: 'INVALID_FIELD', field: 'not_a_field', + }); + } + }); + + it('an unknown field carrying an OPERATOR bag is refused on the same terms', async () => { + // `{not_a_field: {$gte: 5}}` is still a predicate that can only match + // zero rows — the operator does not make the column exist. + await expect(find({ where: { not_a_field: { $gte: 5 } } })) + .rejects.toMatchObject({ status: 400, code: 'INVALID_FIELD', field: 'not_a_field' }); + }); + + // ───────────────────────────────────────────────────────────── + // The issue's actual complaint: the doors DISAGREED + // ───────────────────────────────────────────────────────────── + + it('all four doors now give ONE verdict for one mistake', async () => { + const doors: ReadonlyArray<{ door: string, query: any }> = [ + // The #4134 control — the door that already answered correctly. + { door: 'bare key (control)', query: { not_a_field: 'x' } }, + ...EXPLICIT_DOORS.map(({ door, query }) => ({ door, query: query('not_a_field', 'x') })), + ]; + // Scale guard: the issue names four doors. If this list shrinks, the + // loop below silently stops covering one of them. + expect(doors).toHaveLength(4); + + const verdicts: string[] = []; + for (const { door, query } of doors) { + try { + const r: any = await find(query); + verdicts.push(`${door} => 200 total=${r.total}`); + } catch (e: any) { + verdicts.push(`${door} => ${e.status} ${e.code}`); + } + } + // Pinned positively, not as a "must not contain": the exact verdict for + // every door is written out, so a door that starts answering something + // else — including a NEW wrong answer — fails here. + expect(verdicts).toEqual([ + 'bare key (control) => 400 INVALID_FIELD', + 'where object => 400 INVALID_FIELD', + '$filter string => 400 INVALID_FIELD', + 'filter AST => 400 INVALID_FIELD', + ]); + }); + + it('GUARD the bare-key control is untouched — this fix is #4134\'s sibling, not its replacement', async () => { + await expect(find({ not_a_field: 'x' })).rejects.toMatchObject({ + status: 400, + code: 'INVALID_FIELD', + field: 'not_a_field', + object: 'showcase_task', + }); + // Its message is the PARAM-shaped one, and stays that way: the two doors + // give one verdict but not one sentence, because the advice differs. + await expect(find({ pageSize: '5' })) + .rejects.toThrow(/Did you mean the 'top' query parameter/); + }); + + // ───────────────────────────────────────────────────────────── + // The rejection has to be usable + // ───────────────────────────────────────────────────────────── + + it('names the parameter the caller actually wrote, not the canonical slot', async () => { + // Telling someone who sent `?$filter=…` that "'where' is invalid" names + // a parameter absent from their request (#4226's discipline). + for (const [query, param] of [ + [{ $filter: JSON.stringify({ not_a_field: 'x' }) }, '$filter'], + [{ filter: JSON.stringify({ not_a_field: 'x' }) }, 'filter'], + [{ filters: JSON.stringify({ not_a_field: 'x' }) }, 'filters'], + [{ where: { not_a_field: 'x' } }, 'where'], + ] as const) { + await expect(find(query)).rejects.toMatchObject({ code: 'INVALID_FIELD', param }); + } + }); + + it('states the zero-row consequence and suggests the field that was meant', async () => { + await expect(find({ where: { not_a_field: 'x' } })) + .rejects.toThrow(/can only match zero records/); + await expect(find({ where: { stauts: 'done' } })) + .rejects.toThrow(/Did you mean the field 'status'/); + }); + + it('reports the first unknown name and still discloses the rest', async () => { + await expect(find({ where: { not_a_field: 'x', other_bogus: 'y' } })) + .rejects.toMatchObject({ + field: 'not_a_field', + fields: ['not_a_field', 'other_bogus'], + }); + }); + + // ───────────────────────────────────────────────────────────── + // GUARDs — the reach of the gate, which must not grow + // ───────────────────────────────────────────────────────────── + + it('GUARD a real field that matches nothing is still an HONEST 200 + total 0', async () => { + // The one empty list that must never become a 400: the predicate was + // applied and genuinely matched nothing. + for (const query of [ + { where: { status: 'zzz' } }, + { $filter: JSON.stringify({ status: 'zzz' }) }, + { filter: [['status', '=', 'zzz']] }, + ]) { + await expect(find(query)).resolves.toMatchObject({ total: 0 }); + } + }); + + it('GUARD filters on the system fields the registry injected, which the author never declared', async () => { + expect(taskObject.fields).not.toHaveProperty('owner_id'); + await expect(find({ where: { owner_id: 'usr_1' } })).resolves.toMatchObject({ total: 10 }); + await expect(find({ where: { created_at: '2026-07-30T00:00:00.000Z' } })) + .resolves.toMatchObject({ total: 10 }); + await expect(find({ where: { id: 't1' } })).resolves.toMatchObject({ total: 1 }); + }); + + it('GUARD a dotted path on a REAL head passes through — the gate must not narrow what already worked', async () => { + // `owner_id` is a real field, so `owner_id.name` is not this gate's + // business: it must pass, the same way `?owner_id.name=` does on the + // bare-key door. Green in both directions by construction — nothing + // refused it before this fix and nothing may refuse it after. + await expect(find({ where: { 'owner_id.name': 'Ada' } })).resolves.toBeDefined(); + }); + + it('a dotted path on an UNKNOWN head is refused, exactly as the bare-key door judges one', async () => { + // The other half of the head-segment rule, and — unlike the pass-through + // above — this one is the fix talking: it is a 200 + empty list without + // it. Kept as its own test for that reason; folding the two halves into + // one case mislabels a fix-dependent assertion as a guard. + await expect(find({ where: { 'not_a_field.name': 'Ada' } })) + .rejects.toMatchObject({ code: 'INVALID_FIELD', field: 'not_a_field.name' }); + }); + + it('GUARD a nested-relation condition is not descended into — its keys belong to another object', async () => { + // `{owner_id: {region: 'NA'}}` names `owner_id` on THIS object and + // `region` on the related one. Judging `region` against this object's + // field map would refuse a legitimate relation filter. + await expect(find({ where: { owner_id: { region: 'NA' } } })).resolves.toBeDefined(); + }); + + it('GUARD an unknown object stays a 404 — the filter gate must not turn it into a 400', async () => { + await expect( + protocol.findData({ object: 'no_such_object', query: { where: { not_a_field: 'x' } } }), + ).rejects.toMatchObject({ status: 404, code: 'OBJECT_NOT_FOUND' }); + }); + + it('GUARD an unrunnable filter is still INVALID_FILTER — this gate answers only "does the field exist"', async () => { + // #4181 / #4121 own the shape verdicts and run first; a field gate that + // pre-empted them would relabel a parse failure as a typo. + await expect(find({ filter: '{status:done' })) + .rejects.toMatchObject({ status: 400, code: 'INVALID_FILTER' }); + }); + + it('GUARD #4164 composition is unchanged — a bare field param still NARROWS an explicit filter', async () => { + const r: any = await find({ filter: JSON.stringify({ status: 'open' }), title: 'Task 3' }); + expect(r.total).toBe(1); + expect(r.records.map((x: any) => x.id)).toEqual(['t3']); + }); + + it('GUARD the param gate still reports FIRST when both a bare key and the filter are wrong', async () => { + // Precedence deliberately unmoved: this gate runs after #4134's, so a + // request that gets both wrong answers exactly as it did before. + await expect(find({ where: { not_a_field: 'x' }, pageSize: '5' })) + .rejects.toThrow(/Did you mean the 'top' query parameter/); + }); +}); From 74cc5a877330948cd3688cae9bf254d8dc2c0114 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 06:51:41 +0000 Subject: [PATCH 2/2] docs(changeset): name the import-path consequence of the #7534 filter gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate sits at the `findData` ingress, so it also reaches the record-matching lookup the import runner performs for `update` / `upsert` writes. That is a user-visible behaviour change on a second surface, and someone who hits it must be able to find out why from the release notes rather than from a support thread. Records the old behaviour (a `matchField` naming no field silently degraded an upsert into an insert, returning `'none'`), the new one (that row fails with `400 INVALID_FIELD`, contained by the row loop's own try/catch so the rest of the import proceeds), the remedy, and that `resolveRef`'s speculative probes are unaffected because they already catch the absent-field case deliberately. Ruled on #7534: the failure stays. Exempting the import path would have meant ADDING code to preserve a silent data-correctness bug of the same family this change closes. Changeset only — no code changed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NxE7c6qf7Bi9ZQ7HtYrNUj --- ...rest-list-explicit-filter-unknown-field.md | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/.changeset/rest-list-explicit-filter-unknown-field.md b/.changeset/rest-list-explicit-filter-unknown-field.md index a2f09bdcbe..02b555f7f7 100644 --- a/.changeset/rest-list-explicit-filter-unknown-field.md +++ b/.changeset/rest-list-explicit-filter-unknown-field.md @@ -57,3 +57,39 @@ infer from a `200`. `200` with `total: 0`. A filter that cannot be *run* at all is still `INVALID_FILTER` (#4121 / #4181), which answers first; this gate answers only "does this field exist". + +## Upgrade note — data import: a `matchField` naming no field now fails the row + +The gate sits at the `findData` ingress, so it also reaches the record-matching +lookup the CSV/JSON import runner performs for `update` and `upsert` writes. +This is a **user-visible behaviour change on a second surface**, and it is +deliberate — the ruling on #7534 was to keep it rather than exempt the import +path. + +**Before.** A `matchFields` entry naming a field the target object does not have +produced a filter that could only match zero rows. The lookup read that as +`'none'` — "no existing record" — and an `upsert` therefore fell through to a +**create**. The import reported success while writing duplicate rows the caller +believed were being matched and updated, and nothing in the response +distinguished that from a genuinely new record. + +**Now.** That row fails with `400 INVALID_FIELD` naming the field. The failure +is contained by the row loop's own `try`/`catch`, so it is reported as one +failed row in the import results and **the rest of the import proceeds** — it is +not an aborted job. + +**Remedy.** Correct the `matchFields` name to a field that exists on the object. +The rejection names the offending field and, when it reads like a typo, suggests +the closest real field name. + +Exempting the import path would have meant *adding* code — catching +`INVALID_FIELD` and restoring `'none'` — to preserve a silent data-correctness +bug of exactly the family this change closes, so the invariant is restored +instead. + +**Unaffected: reference resolution.** The import runner's `resolveRef` probes +candidate display fields (`name`, `title`, `label`, `full_name`, `email`, +`username`) that legitimately may not exist on the object being referenced, and +it already wraps each probe in a deliberate `catch` that moves on to the next +candidate. A `400` lands exactly where the empty result did, so reference +resolution behaves as before.