diff --git a/.changeset/27484-blank-search-fields-refused.md b/.changeset/27484-blank-search-fields-refused.md new file mode 100644 index 00000000000..3e5a5106954 --- /dev/null +++ b/.changeset/27484-blank-search-fields-refused.md @@ -0,0 +1,21 @@ +--- +'@objectstack/metadata-protocol': patch +'@objectstack/rest': patch +--- + +fix(metadata-protocol)!: a `searchFields` override naming no field (`$searchFields=`, `$searchFields=,`, `[]`) is refused 400, not read as absent, so a narrowed search no longer runs over the default search fields (#27484) + +Clause-②: no (narrowing) — the data doors (`GET /data/:object`, `POST /data/:object/query`, `GET /data/:object/export`, and every other caller of `findData`) refuse a `searchFields` value that names no field, which they answered 200 over the default search set before. Nothing is widened. +Metadata-protocol: none + + + +**BREAKING** accept-set narrowing at the query door (`@objectstack/metadata-protocol`), served by every Data API door. + +- **Before.** A blank or comma-only `$searchFields` (or `searchFields`), an empty array, or an empty object-form `search.fields` was read as no override, and the search scanned the object's whole default search set: `?$search=initech&$searchFields=` answered the rows `?$search=initech&$searchFields=industry` excludes, under a 200. The export route dropped a blank `searchFields` itself before the query ran. +- **Now.** `400 VALIDATION_ERROR`, naming the parameter the caller wrote (`Query parameter '$searchFields' names no field`), the envelope `$select` answers for a projection naming no field (#26644). No row is read. On the list and query doors this holds with or without a `search` term: `?$searchFields=` alone is refused too, as a non-empty override naming an unknown field already is. +- **Unchanged.** An absent `searchFields` searches the default set; a named one narrows it; a name outside the searchable set is still `400 INVALID_FIELD`. The engine (`find` / `count` called directly) still reads an empty override as absent. The export route still ignores `searchFields` sent without `search`. +- **Not yet narrowed.** The published declarations (`QueryTransportParamsSchema.$searchFields`, `EngineQueryOptionsSchema.searchFields`, `QuerySchema.searchFields`, `FullTextSearchSchema.fields`) still admit `''` and `[]`; the refusal is the door's. So on `POST /data/:object/query` the sibling `select: []` is the body schema's `400 VALIDATION_FAILED` while `searchFields: []` is this `400 VALIDATION_ERROR`. Narrowing the declarations is follow-up #27683. +- **Who sees it.** An API caller that sends `searchFields` with no field in it. The Console and the bundled view plugins send the parameter only when it holds at least one field. + +**The remedy.** Leave the parameter out to search the default fields, or name the fields to search. diff --git a/content/docs/api/data-api.mdx b/content/docs/api/data-api.mdx index 2eba6a4e499..70053546d1c 100644 --- a/content/docs/api/data-api.mdx +++ b/content/docs/api/data-api.mdx @@ -154,6 +154,7 @@ corrupts something the earlier axes do not: |:---|:---| | `?search=alpha&searchFields=title` | scans only `title` | | `?search=alpha&searchFields=no_such_field` | `400 INVALID_FIELD` | +| `?search=alpha&searchFields=` (or `=,`, or `[]` in a body) | `400 VALIDATION_ERROR` — names no field, the answer `$select=` gets; leave the parameter out to scan the default set | | `?search=alpha&searchFields=amount` | `400 INVALID_FIELD` — real field, but not searchable | | `?search=alpha&searchFields=project_id.name` | `400 INVALID_FIELD` — search scans this object's own columns; mirror the related title instead (see below) | | `?search=alpha` on an object with no searchable field (two lookups and a `formula` title, say) | `400 INVALID_FIELD` — nothing to scan; declare a stored field searchable or mirror the title (see below) | diff --git a/content/docs/data-modeling/queries.mdx b/content/docs/data-modeling/queries.mdx index 91fcd9355ae..acbe6ba6a42 100644 --- a/content/docs/data-modeling/queries.mdx +++ b/content/docs/data-modeling/queries.mdx @@ -540,10 +540,12 @@ Only `query` and `fields` are implemented. The engine expands `search` into a dr **narrow** the scan within the server-resolved searchable set — the object's declared `searchableFields`, or a text-like auto-default when none are declared. Over the REST/protocol ingress a name outside that set is `400 INVALID_FIELD`, with distinct -messages for a field that does not exist and a real field that is not searchable. The -engine used to drop unknown names and fall back to the full searchable set — a parameter -that exists to narrow a search, silently widening it. Internal callers reaching -`engine.find()` directly are unaffected. +messages for a field that does not exist and a real field that is not searchable. An +override that names no field at all (`?$searchFields=`, `?$searchFields=,`, an empty +array) is `400 VALIDATION_ERROR`, as `$select=` is; leave the parameter out to scan the +default set. The engine used to drop unknown names and fall back to the full searchable +set — a parameter that exists to narrow a search, silently widening it. Internal callers +reaching `engine.find()` directly are unaffected. ### Searching by a related record's title — mirror the value diff --git a/packages/metadata-protocol/src/protocol-query-validation.ts b/packages/metadata-protocol/src/protocol-query-validation.ts index d5863116e70..1d6fa7dd27d 100644 --- a/packages/metadata-protocol/src/protocol-query-validation.ts +++ b/packages/metadata-protocol/src/protocol-query-validation.ts @@ -43,7 +43,7 @@ */ import type { ObjectStackProtocolImplementation } from './protocol.js'; -import { FILTER_LOGICAL_KEYS, ODATA_SPELLING, QUERY_PARAM_NEAR_MISS, collectFilterReads, invalidSortError, isObjectOfDisabledPackage, isPlainRecord, nearMissKey, objectPackageDisabledError, suggestFieldName } from './protocol.js'; +import { FILTER_LOGICAL_KEYS, ODATA_SPELLING, QUERY_PARAM_NEAR_MISS, collectFilterReads, invalidSortError, isObjectOfDisabledPackage, isPlainRecord, nearMissKey, objectPackageDisabledError, quotedQueryValue, suggestFieldName } from './protocol.js'; import { objectNotFoundError, relationConditionSites } from '@objectstack/core'; import { collectInternalWriteResponseFields } from './write-response-internal-fields.js'; import { AggregationFunction, DateGranularity, REFERENCE_VALUE_TYPES, SEARCHABLE_ENUM_TYPES, SEARCHABLE_TEXTUAL_TYPES, SEARCH_AUTO_EXCLUDED_FIELDS, apiExposureDenialReason, canServeApiOperation, classifyDottedFilterHead, effectiveOperationsArray, isFilterAST, isVirtualSearchField, parseFilterAST, referenceTargetOf, resolveEffectiveApiMethods, resolveSearchFieldResolution } from '@objectstack/spec/data'; @@ -874,10 +874,26 @@ export function assertSearchFieldsAreSearchable( err.param = param; throw err; } - // An empty override is ABSENT — the engine falls through to the - // allowed set for it, which for a caller who named nothing is the - // answer they asked for, not a widened one. - if (names.length === 0) return; + // [#27484] …and an override naming NO field is refused, not read as + // absent. A blank or comma-only string, an empty array, or an empty + // object-form `search.fields` used to return here, and the engine then + // scanned the object's whole default set: a caller who wrote the + // parameter meant to narrow, so its silent widening to the defaults is + // the failure this gate exists for. Absent stays absent (the caller + // passes no override, and the door never calls this gate). The + // projection's envelope (`projectionFieldsValue`, #26644), so `$select` + // and `$searchFields` answer a value naming no field alike. + if (names.length === 0) { + const err: any = new Error( + `Query parameter '${param}' names no field; got ${quotedQueryValue(requested)}. The query was not ` + + "run: an empty search-field list was read as no narrowing, which scans the object's default " + + 'search fields. Name at least one field, or leave the parameter out to search the default fields.', + ); + err.status = 400; + err.code = 'VALIDATION_ERROR'; + err.param = param; + throw err; + } const gate = this.resolveQueryFields(object); if (!gate) return; // [#22646] A field declared `internal: true` is never a search target, diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 9eac8c081df..dbe3cd4e9e5 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -4640,9 +4640,11 @@ function pageWindowValue(value: unknown, param: string): number { /** * A refused query value as the refusal quotes it: JSON, so a blank or * whitespace-only value is visible, and bounded, so a long one cannot bury - * the sentence around it. + * the sentence around it. [#27484] Exported for the search-fields gate + * (`protocol-query-validation.ts`), which refuses an empty override in the + * projection's words. */ -function quotedQueryValue(value: unknown): string { +export function quotedQueryValue(value: unknown): string { // `String` for a number: `JSON.stringify(Infinity)` is `null`. const shown = typeof value === 'number' ? String(value) : JSON.stringify(value) ?? String(value); return shown.length > 64 ? `${shown.slice(0, 64)}…` : shown; diff --git a/packages/objectql/src/engine-search-empty-set.test.ts b/packages/objectql/src/engine-search-empty-set.test.ts index 575243eb1d9..59a81cfae76 100644 --- a/packages/objectql/src/engine-search-empty-set.test.ts +++ b/packages/objectql/src/engine-search-empty-set.test.ts @@ -226,10 +226,19 @@ describe('[#24284] the palette keeps its skip; the data door surfaces the refusa it('findData over the junction answers the refusal, on the list and the query shapes', async () => { const { protocol, reads } = await makeHarness(); - for (const query of [{ search: 'zzzz-nothing' }, { search: 'Dune', searchFields: '' }, { search: { query: 'Dune' } }]) { + for (const query of [{ search: 'zzzz-nothing' }, { search: { query: 'Dune' } }]) { const err: any = await (protocol as any).findData({ object: 'lib_loan', query }).then(() => null, (e: any) => e); expect(err, JSON.stringify(query)).toMatchObject(REFUSAL); } + // [#27484] Flipped on purpose (triage ruling 6500648325, option A): a + // blank override no longer reaches the engine as "absent", so the door + // refuses it first, in the projection's envelope. The point of this + // leg holds: a search over the junction never silently widens to every + // row; no `lib_loan` read is made on either refusal. + const blankOverride: any = await (protocol as any).findData({ + object: 'lib_loan', query: { search: 'Dune', searchFields: '' }, + }).then(() => null, (e: any) => e); + expect(blankOverride).toMatchObject({ code: 'VALIDATION_ERROR', status: 400, param: 'searchFields' }); expect(reads.filter((r) => r.object === 'lib_loan')).toEqual([]); // …and the control object and the blank term are untouched. const book: any = await (protocol as any).findData({ object: 'lib_book', query: { search: 'Dune' } }); diff --git a/packages/objectql/src/query-expression-conformance.test.ts b/packages/objectql/src/query-expression-conformance.test.ts index 41679f05c20..bd32e26c1e7 100644 --- a/packages/objectql/src/query-expression-conformance.test.ts +++ b/packages/objectql/src/query-expression-conformance.test.ts @@ -2271,12 +2271,29 @@ describe('#4254 — searchFields / groupBy / aggregations on the list path (real })).rejects.toThrow(/scans this object's own columns/); }); - it('an empty override is ABSENT, not malformed', async () => { - for (const searchFields of ['', []]) { - expect(titles(await protocol.findData({ - object: 'showcase_task', query: { search: 'a', searchFields }, - }))).toEqual(['A', 'B']); + // [#27484] Flipped on purpose (triage ruling 6500648325, option A). This + // pin read an empty override as ABSENT; the door now refuses it in the + // projection's envelope (#26644), as `$select` refuses a projection naming + // no field. Read as absent, the search scanned the default set: a caller who + // wrote the parameter meant to narrow, and got the widest scan instead. + it('an override naming no field is refused, not read as absent', async () => { + const named = [ + [{ searchFields: '' }, 'searchFields'], + [{ searchFields: ',' }, 'searchFields'], + [{ searchFields: ' , ' }, 'searchFields'], + [{ searchFields: [] }, 'searchFields'], + [{ $searchFields: '' }, '$searchFields'], + [{ search: { query: 'a', fields: [] } }, 'search'], + [{ search: { query: 'a', fields: '' } }, 'search'], + ] as const; + for (const [query, param] of named) { + await expect(protocol.findData({ + object: 'showcase_task', query: { search: 'a', ...query }, + }), JSON.stringify(query)).rejects.toMatchObject({ status: 400, code: 'VALIDATION_ERROR', param }); } + // …and a search with no override at all is untouched: absent is absent. + expect(titles(await protocol.findData({ object: 'showcase_task', query: { search: 'a' } }))) + .toEqual(['A', 'B']); }); // ───────────────────────────────────────────────────────────── diff --git a/packages/rest/src/data-action-routes.ts b/packages/rest/src/data-action-routes.ts index 11546c5099b..87b3e97715b 100644 --- a/packages/rest/src/data-action-routes.ts +++ b/packages/rest/src/data-action-routes.ts @@ -1132,14 +1132,20 @@ export function registerDataActionEndpoints(this: DataActionRouteHost, basePath: ? q.search.trim() : undefined; // ADR-0061 override for which fields the term scans. Only meaningful - // alongside `search`; ignored on its own, exactly as in findData. + // alongside `search`; ignored on its own. + // + // [#27484] A value naming no field (`searchFields=`, `=,`) is + // kept as the empty list rather than read as absent, so + // `findData`'s search-fields gate refuses it exactly as on the + // list door. Read as absent, the export scanned the object's + // default search fields: a file of rows the caller's narrowing + // excluded. let searchFields: string[] | undefined; - if (typeof q.searchFields === 'string' && q.searchFields.length > 0) { + if (typeof q.searchFields === 'string') { searchFields = q.searchFields.split(',').map((s: string) => s.trim()).filter(Boolean); } else if (Array.isArray(q.searchFields)) { searchFields = q.searchFields.filter((s: any) => typeof s === 'string' && s.length > 0); } - if (searchFields && searchFields.length === 0) searchFields = undefined; let orderby: any = undefined; if (typeof q.orderby === 'string' && q.orderby.length > 0) { diff --git a/packages/rest/src/data-empty-search-fields-door.test.ts b/packages/rest/src/data-empty-search-fields-door.test.ts new file mode 100644 index 00000000000..6cb2b06733b --- /dev/null +++ b/packages/rest/src/data-empty-search-fields-door.test.ts @@ -0,0 +1,193 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#27484] A `searchFields` override naming no field — a blank or comma-only + * string, an empty array — is refused `400 VALIDATION_ERROR`, naming the + * parameter the caller wrote, on every Data API door that hands its search to + * the shared normalizer (`metadata-protocol`'s `findData`) — over a real + * `SqlDriver` (SQLite), three rows. The projection's envelope (#26644), so + * `$searchFields` answers a value naming no field exactly as `$select` does + * (`data-empty-projection-count-door.test.ts`). + * + * Before: the gate (`assertSearchFieldsAreSearchable`) read the empty override + * as ABSENT, and the engine scanned the object's default search set, so + * `?$search=n-&$searchFields=` answered 200 with every row the narrowing + * `?$search=n-&$searchFields=label` excludes. The export route dropped a blank + * value itself (`searchFields=` read as absent) before `findData` saw it. + * + * The per-spelling pin over the protocol is objectql's + * `query-expression-conformance.test.ts`. This file is the door-level half: + * the status and `code` each door answers, the spelling it names, that the + * driver was never asked, and the controls answering exactly as before. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { RestServer } from './rest-server'; +import { installDefaultOrganization } from './default-organization.testkit.js'; + +const OBJECT = 'rest_search_fields_27484'; + +const ITEMS = { + name: OBJECT, + label: 'Items 27484', + fields: { + label: { name: 'label', type: 'text' as const }, + note: { name: 'note', type: 'text' as const }, + }, +}; + +const LABELS = ['a', 'b', 'c']; + +/** The term every case searches: in every row's `note`, in no row's `label`. */ +const TERM = 'n-'; + +/** name · the querystring `GET /data/:object` receives beside `$search` · the spelling the refusal names. */ +const REFUSED_GET: ReadonlyArray, string]> = [ + ['$searchFields= blank (the card)', { $searchFields: '' }, '$searchFields'], + ['$searchFields=, (the card)', { $searchFields: ',' }, '$searchFields'], + ['$searchFields= whitespace and commas', { $searchFields: ' , ' }, '$searchFields'], + ['searchFields= blank, unprefixed (the card)', { searchFields: '' }, 'searchFields'], +]; + +/** name · the `POST /data/:object/query` body beside `search` · the key the refusal names. */ +const REFUSED_POST: ReadonlyArray, string]> = [ + ['searchFields: []', { search: TERM, searchFields: [] }, 'searchFields'], + ['object-form search.fields: []', { search: { query: TERM, fields: [] } }, 'search'], +]; + +function createMockServer() { + const noop = () => {}; + return { get: noop, post: noop, put: noop, delete: noop, patch: noop, use: noop, listen: async () => {}, close: async () => {} }; +} + +function makeRes() { + const chunks: string[] = []; + const res: any = { + write: (s: string) => { chunks.push(String(s)); return true; }, end: () => {}, + header: () => res, + status: (code: number) => { res._status = code; return res; }, + json: (body: any) => { res._json = body; return res; }, + }; + res._chunks = chunks; + return res; +} + +describe('[#27484] a searchFields override naming no field on the Data API doors — sqlite', () => { + let engine: ObjectQL; + let driver: any; + const reads = { n: 0 }; + let list: (query: Record) => Promise<{ status: number; body: any }>; + let query: (body: Record) => Promise<{ status: number; body: any }>; + let exportCsv: (query: Record) => Promise<{ status: number; body: any; rows: string[] }>; + + beforeAll(async () => { + driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true } as any); + engine = new ObjectQL(); + engine.registerDriver(driver, true); + await engine.init(); + engine.registry.registerObject(ITEMS as any); + await engine.syncSchemas(); + // A real `single` install's Default Organization (ADR-0131 D9, D1). + await installDefaultOrganization(engine); + for (const label of LABELS) await engine.insert(OBJECT, { id: `r${label}`, label, note: `n-${label}` } as any); + + // Reads of THIS object — the protocol's own metadata traffic is not the question. + for (const verb of ['find', 'findOne', 'count', 'aggregate'] as const) { + const real = driver[verb].bind(driver); + driver[verb] = (o: string, ...rest: unknown[]) => { if (o === OBJECT) reads.n += 1; return real(o, ...rest); }; + } + + const protocol = new ObjectStackProtocolImplementation(engine as any); + const rest = new RestServer(createMockServer() as any, protocol as any, { api: { requireAuth: false } } as any); + (rest as any).resolveExecCtx = async () => ({ userId: 'test-user' }); + rest.registerRoutes(); + const route = (method: string, path: string) => { + const found = rest.getRoutes().find((r: any) => r.method === method && r.path === path); + expect(found, `${method} ${path}`).toBeDefined(); + return found!; + }; + const getRoute = route('GET', '/api/v1/data/:object'); + const postRoute = route('POST', '/api/v1/data/:object/query'); + const exportRoute = route('GET', '/api/v1/data/:object/export'); + list = async (q) => { + const res = makeRes(); + // What a querystring carries: strings. + await getRoute.handler({ params: { object: OBJECT }, query: { $orderby: 'label', $search: TERM, ...q }, headers: {} } as any, res); + return { status: res._status ?? 200, body: res._json }; + }; + query = async (body) => { + const res = makeRes(); + // What the wire carries: JSON. + await postRoute.handler({ params: { object: OBJECT }, body: JSON.parse(JSON.stringify(body)), query: {}, headers: {} } as any, res); + return { status: res._status ?? 200, body: res._json }; + }; + exportCsv = async (q) => { + const res = makeRes(); + await exportRoute.handler({ params: { object: OBJECT }, query: { format: 'csv', search: TERM, ...q }, headers: {} } as any, res); + const rows = res._chunks.join('').split(/\r?\n/).filter(Boolean).slice(1); + return { status: res._status ?? 200, body: res._json, rows }; + }; + }); + + afterAll(async () => { + try { await engine?.destroy(); } catch { /* noop */ } + }); + + it('GET list: 400 VALIDATION_ERROR naming the querystring spelling — no read', async () => { + const before = reads.n; + for (const [name, q, param] of REFUSED_GET) { + const res = await list(q); + expect(res.status, `${name}: ${JSON.stringify(res.body)}`).toBe(400); + expect(res.body.code, name).toBe('VALIDATION_ERROR'); + expect(res.body.error, name).toContain(`Query parameter '${param}' names no field`); + } + expect(reads.n - before, 'no read of the object — every refusal precedes the driver').toBe(0); + }); + + it('POST query: an empty override, either slot, is 400 VALIDATION_ERROR naming the key — no read', async () => { + const before = reads.n; + for (const [name, body, param] of REFUSED_POST) { + const res = await query(body); + expect(res.status, `${name}: ${JSON.stringify(res.body)}`).toBe(400); + expect(res.body.code, name).toBe('VALIDATION_ERROR'); + expect(res.body.error, name).toContain(`Query parameter '${param}' names no field`); + } + expect(reads.n - before, 'no read of the object — every refusal precedes the driver').toBe(0); + }); + + it('GET export: a blank or comma-only searchFields beside search is the same refusal — no read, no file', async () => { + const before = reads.n; + for (const searchFields of ['', ',']) { + const res = await exportCsv({ searchFields }); + expect(res.status, `searchFields=${JSON.stringify(searchFields)}: ${JSON.stringify(res.body)}`).toBe(400); + expect(res.body.code).toBe('VALIDATION_ERROR'); + expect(res.body.error).toContain("Query parameter 'searchFields' names no field"); + expect(res.rows).toEqual([]); + } + expect(reads.n - before, 'no read of the object — every refusal precedes the driver').toBe(0); + }); + + it('controls: a named override narrows, an absent one scans the default set, exactly as before', async () => { + const all = await list({}); + expect(all.status, JSON.stringify(all.body)).toBe(200); + expect(all.body.records.map((r: any) => r.label)).toEqual(LABELS); + + const narrowed = await list({ $searchFields: 'label' }); + expect(narrowed.status, JSON.stringify(narrowed.body)).toBe(200); + expect(narrowed.body.records).toEqual([]); + + const posted = await query({ search: TERM, searchFields: ['note'] }); + expect(posted.status, JSON.stringify(posted.body)).toBe(200); + expect(posted.body.records).toHaveLength(3); + + const exported = await exportCsv({}); + expect(exported.status).toBe(200); + expect(exported.rows).toHaveLength(3); + const exportedNarrowed = await exportCsv({ searchFields: 'label' }); + expect(exportedNarrowed.status).toBe(200); + expect(exportedNarrowed.rows).toEqual([]); + }); +});