diff --git a/.changeset/27829-search-fields-credential-refused.md b/.changeset/27829-search-fields-credential-refused.md new file mode 100644 index 00000000000..6b332e28313 --- /dev/null +++ b/.changeset/27829-search-fields-credential-refused.md @@ -0,0 +1,19 @@ +--- +'@objectstack/metadata-protocol': patch +--- + +fix(metadata-protocol)!: a `$searchFields` entry naming a `secret` / `password` field is refused 400 `INVALID_FIELD` at the data door, even when the object's `searchableFields` declares it, instead of searching the rest of the declared set (#27829) + +Clause-②: no (narrowing) — the data doors (`GET /data/:object`, `POST /data/:object/query`, and every other caller of `findData`) refuse a search-field override naming a credential field, which they answered 200 over the rest of the declared 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.** On an object whose `searchableFields` declares a `secret` or `password` field, `?$search=TERM&$searchFields=api_key` passed the door, the engine withheld the credential column from the scan (ADR-0100), and its tolerant intersection then searched the rest of the declared set: 200 with rows matched in `name`. `$searchFields=name,api_key` silently dropped `api_key`. The credential itself was never searched. +- **Now.** `400 INVALID_FIELD` naming the field (`Field 'api_key' on object 'OBJ' is type 'secret', a credential, and cannot be searched`), before any row is read — the same answer the auto-default path already gave a credential name. It covers the standalone `searchFields` / `$searchFields` slot and the object-form `search.fields`. Judged by `isCredentialFieldType` (`@objectstack/spec/data`), the predicate the engine's withhold reads. +- **Unchanged.** A request that names no credential field is served as before; with no `$searchFields`, the declared set is searched and the declared credential entry is never scanned. The engine's withhold stays as the floor for direct engine callers. +- **Who sees it.** An API caller naming a credential field in `$searchFields`, typically a client echoing the object's `searchableFields` verbatim on an object that declares one (the linter has flagged that declaration since `validate-searchable-fields` learned the credential check). + +**The remedy.** Remove the credential field from the object's `searchableFields` and from the request; search another column. diff --git a/content/docs/api/data-api.mdx b/content/docs/api/data-api.mdx index 70053546d1c..dfd4fcd4415 100644 --- a/content/docs/api/data-api.mdx +++ b/content/docs/api/data-api.mdx @@ -156,6 +156,7 @@ corrupts something the earlier axes do not: | `?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=api_key` (a `secret` or `password` field, even one `searchableFields` declares) | `400 INVALID_FIELD` — a credential is never searched; remove it from `searchableFields` too | | `?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) | | `groupBy: ["status"]` | one bucket per status value | diff --git a/packages/metadata-protocol/src/protocol-query-validation.ts b/packages/metadata-protocol/src/protocol-query-validation.ts index 1d6fa7dd27d..3028c46e833 100644 --- a/packages/metadata-protocol/src/protocol-query-validation.ts +++ b/packages/metadata-protocol/src/protocol-query-validation.ts @@ -46,7 +46,7 @@ import type { ObjectStackProtocolImplementation } 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'; +import { AggregationFunction, DateGranularity, REFERENCE_VALUE_TYPES, SEARCHABLE_ENUM_TYPES, SEARCHABLE_TEXTUAL_TYPES, SEARCH_AUTO_EXCLUDED_FIELDS, apiExposureDenialReason, canServeApiOperation, classifyDottedFilterHead, effectiveOperationsArray, isCredentialFieldType, isFilterAST, isVirtualSearchField, parseFilterAST, referenceTargetOf, resolveEffectiveApiMethods, resolveSearchFieldResolution } from '@objectstack/spec/data'; /** * [#20802] The nested-relation spelling of a dotted relation path, as the @@ -959,10 +959,22 @@ export function assertSearchFieldsAreSearchable( // types have a column. const virtual = names.filter((n) => !allowedSet.has(n) && gate.known.has(n) && isVirtualSearchField(gate.fields[n])); const unsearchable = names.filter((n) => !allowedSet.has(n) && gate.known.has(n) && !isVirtualSearchField(gate.fields[n])); + // [#27829] A CREDENTIAL field (`secret` / `password`) the allowed set kept — + // only a declared `searchableFields` keeps one; the auto default never does, + // and refuses it below by its type — is refused here too. The engine + // withholds every credential column from the scan (ADR-0100, the + // fail-closed floor), so admitting the name let the engine's tolerant + // intersection search the REST of the declared set: a narrower search + // answered with a wider one, the #4254 posture this gate exists for. + // Judged by the predicate the engine's withhold reads + // (`isCredentialFieldType`, `@objectstack/spec/data`), so the door and the + // engine cannot disagree about which fields are credentials. + const credential = names.filter((n) => allowedSet.has(n) && isCredentialFieldType(gate.fields[n]?.type)); const [offenders, reason] = unknown.length > 0 ? [unknown, 'unknown' as const] : staleDeclared.length > 0 ? [staleDeclared, 'stale-declared' as const] : virtual.length > 0 ? [virtual, 'virtual' as const] + : credential.length > 0 ? [credential, 'credential' as const] : [unsearchable, 'unsearchable' as const]; if (offenders.length === 0) return; const first = offenders[0]; @@ -985,6 +997,18 @@ export function assertSearchFieldsAreSearchable( + 'look like coverage; remove it there as well.' : '') + ` Mirror the computed value onto a stored text field on '${object}' and search that instead.`; + } else if (reason === 'credential') { + detail = `Field '${first}' on object '${object}' is type '${gate.fields[first]?.type}', a credential, ` + + 'and cannot be searched' + + (offenders.length > 1 ? ` (also: ${offenders.slice(1).join(', ')})` : '') + + `. 'search' never scans a credential column, so '${param}' naming it cannot narrow ` + + 'anything — the engine used to drop it and scan the rest of the declared columns instead, ' + + 'answering a NARROWER search with a WIDER one.' + + (declaredSet.has(first) + ? " The object's 'searchableFields' declares it, which is what made the entry look searchable; " + + 'remove it there as well.' + : '') + + ' Search another column instead.'; } else if (reason === 'unknown') { // A dotted path is a special unknown: plausible vocabulary from the // select/sort axes, but search scans this object's own columns. diff --git a/packages/rest/src/data-search-fields-credential-door.test.ts b/packages/rest/src/data-search-fields-credential-door.test.ts new file mode 100644 index 00000000000..3688cb4e758 --- /dev/null +++ b/packages/rest/src/data-search-fields-credential-door.test.ts @@ -0,0 +1,153 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#27829] A `$searchFields` entry naming a CREDENTIAL field (`secret` / + * `password`) is refused `400 INVALID_FIELD` at the Data API door, naming the + * field, even when the object's own `searchableFields` declares it — over a + * real `SqlDriver` (SQLite), the harness of `data-empty-search-fields-door.test.ts`. + * + * Before: the door (`assertSearchFieldsAreSearchable`) resolved the allowed set + * from the declaration, which kept the credential entry, so the request passed; + * the engine then withheld the credential column from the scan (ADR-0100) and, + * by its tolerant intersection, searched the REST of the declared set. + * `?$search=…&$searchFields=api_key` answered 200 with rows matched in `name` — + * a narrower search answered with a wider one. The engine's withhold stays as + * the fail-closed floor; this file pins the door's half: the status, `code`, + * named field, that the driver was never asked, and the controls unchanged. + */ + +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'; + +/** object name · the credential type its `api_key` field carries. */ +const OBJECTS = [ + ['rest_search_credential_secret_27829', 'secret'], + ['rest_search_credential_password_27829', 'password'], +] as const; + +const NAMES = ['qq7-a', 'qq7-b', 'other']; + +/** The term every case searches: in two rows' `name`. */ +const TERM = 'qq7'; + +function createMockServer() { + const noop = () => {}; + return { get: noop, post: noop, put: noop, delete: noop, patch: noop, use: noop, listen: async () => {}, close: async () => {} }; +} + +function makeRes() { + const res: any = { + write: () => true, end: () => {}, + header: () => res, + status: (code: number) => { res._status = code; return res; }, + json: (body: any) => { res._json = body; return res; }, + }; + return res; +} + +describe('[#27829] $searchFields naming a declared credential field on the Data API door — sqlite', () => { + let engine: ObjectQL; + let driver: any; + const reads = { n: 0 }; + let list: (object: string, query: Record) => Promise<{ status: number; body: any }>; + let query: (object: string, body: Record) => Promise<{ status: number; body: any }>; + + 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(); + for (const [name, ctype] of OBJECTS) { + engine.registry.registerObject({ + name, + label: `Credential ${ctype} 27829`, + searchableFields: ['name', 'api_key'], + fields: { + name: { name: 'name', type: 'text' as const }, + api_key: { name: 'api_key', type: ctype }, + }, + } as any); + } + await engine.syncSchemas(); + // A real `single` install's Default Organization (ADR-0131 D9, D1). + await installDefaultOrganization(engine); + for (const [object] of OBJECTS) { + for (const [i, name] of NAMES.entries()) await engine.insert(object, { id: `r${i}`, name } as any); + } + + const watched = new Set(OBJECTS.map(([o]) => o)); + for (const verb of ['find', 'findOne', 'count', 'aggregate'] as const) { + const real = driver[verb].bind(driver); + driver[verb] = (o: string, ...rest: unknown[]) => { if (watched.has(o)) 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 getRoute = rest.getRoutes().find((r: any) => r.method === 'GET' && r.path === '/api/v1/data/:object'); + expect(getRoute, 'GET /api/v1/data/:object').toBeDefined(); + list = async (object, q) => { + const res = makeRes(); + // What a querystring carries: strings. + await getRoute!.handler({ params: { object }, query: { $orderby: 'name', $search: TERM, ...q }, headers: {} } as any, res); + return { status: res._status ?? 200, body: res._json }; + }; + const postRoute = rest.getRoutes().find((r: any) => r.method === 'POST' && r.path === '/api/v1/data/:object/query'); + expect(postRoute, 'POST /api/v1/data/:object/query').toBeDefined(); + query = async (object, body) => { + const res = makeRes(); + // What the wire carries: JSON. + await postRoute!.handler({ params: { object }, body: JSON.parse(JSON.stringify(body)), query: {}, headers: {} } as any, res); + return { status: res._status ?? 200, body: res._json }; + }; + }); + + afterAll(async () => { + try { await engine?.destroy(); } catch { /* noop */ } + }); + + for (const [object, ctype] of OBJECTS) { + it(`${ctype}: $searchFields=api_key and $searchFields=name,api_key are 400 INVALID_FIELD naming api_key — no read`, async () => { + const before = reads.n; + for (const searchFields of ['api_key', 'name,api_key']) { + const res = await list(object, { $searchFields: searchFields }); + const label = `$searchFields=${searchFields}: ${JSON.stringify(res.body)}`; + expect(res.status, label).toBe(400); + expect(res.body.code, label).toBe('INVALID_FIELD'); + expect(res.body.error, label).toContain(`Field 'api_key' on object '${object}' is type '${ctype}'`); + } + expect(reads.n - before, 'no read of the object — the refusal precedes the driver').toBe(0); + }); + + it(`${ctype}: POST query — searchFields and the object-form search.fields naming api_key are the same refusal — no read`, async () => { + const before = reads.n; + for (const body of [{ search: TERM, searchFields: ['api_key'] }, { search: { query: TERM, fields: ['name', 'api_key'] } }]) { + const res = await query(object, body); + const label = `${JSON.stringify(body)}: ${JSON.stringify(res.body)}`; + expect(res.status, label).toBe(400); + expect(res.body.code, label).toBe('INVALID_FIELD'); + expect(res.body.error, label).toContain(`Field 'api_key' on object '${object}' is type '${ctype}'`); + } + expect(reads.n - before, 'no read of the object — the refusal precedes the driver').toBe(0); + }); + + it(`${ctype}: controls — $searchFields=name narrows, no $searchFields searches the declared set, as before`, async () => { + const narrowed = await list(object, { $searchFields: 'name' }); + expect(narrowed.status, JSON.stringify(narrowed.body)).toBe(200); + expect(narrowed.body.records.map((r: any) => r.name)).toEqual(['qq7-a', 'qq7-b']); + + const all = await list(object, {}); + expect(all.status, JSON.stringify(all.body)).toBe(200); + expect(all.body.records.map((r: any) => r.name)).toEqual(['qq7-a', 'qq7-b']); + + const posted = await query(object, { search: TERM, searchFields: ['name'] }); + expect(posted.status, JSON.stringify(posted.body)).toBe(200); + expect(posted.body.records).toHaveLength(2); + }); + } +});