fix(objectql): strip the hidden __search companion from every record body (#7642)

.changeset/search-companion-default-projection-strip.md

@@ -0,0 +1,49 @@
+---
+"@objectstack/objectql": patch
+---
+
+fix(objectql): strip the hidden `__search` companion column from every record body (#7642)
+
+The `__search` search-normalization companion (#2486) is declared invisible to
+clients — `hidden` + `readonly` + `system` + `searchable: false` — and every one
+of those flags does something real: the column stays out of auto-views, out of
+the `$search` auto-default, and a `$searchFields` override naming it is refused
+with a 400 ("is hidden"). None of them is a **projection** rule. A query that
+names no `fields` reaches the driver with `ast.fields` undefined, drivers answer
+that with `SELECT *`, and the column rode back in the four record bodies a QA
+run measured (#7629): list/query results, `GET /data/:object/:id`,
+`GET /api/v1/search` hits, and the 201 create body.
+
+The strip now runs at the engine, which is the producer all four surfaces share
+(`/search` hits are `engine.find` rows verbatim; the create body is
+`engine.insert`'s return verbatim). Fixing them one consumer at a time is how
+three of the four would have stayed broken. `find`, `findOne`, the nested
+records `expand` produces, the create response and the **update** response are
+all covered; the update response is not one of the four reported surfaces but is
+the same column in the same response shape, and leaving it out would have made
+POST and PATCH on one object disagree about whether a client-invisible column is
+visible. A predicate update resolves to an affected-row count and is unaffected.
+
+Two details the fix is shaped around, both from the report:
+
+- **It is not gated on the schema declaring the column.** The symptom survived a
+ restart with `OS_SEARCH_PINYIN_ENABLED=false`, and that is not a stale process:
+ with the switch off the registry stops declaring the field, but the physical
+ column and its values remain (ADR-0045 migrations are additive) and `SELECT *`
+ keeps returning them. A strip that asked `schema.fields.__search` first would
+ be silent in exactly the deployment that reported the bug, so the key on the
+ row is the signal.
+- **One caller keeps its read.** `plugin-pinyin-search`'s backfill/reconcile walk
+ projects `['id', …sources, '__search']` under a system context and compares the
+ stored blob against a recomputed one; stripping that unconditionally would make
+ it rewrite every row of every object on every pass. A **system** caller that
+ names the column in `fields` still gets it. A non-system caller does not, even
+ by name — `select` only gates on whether a field is *known*, so `?select=__search`
+ would otherwise be a documented way straight through the strip.
+
+Scope is this one column. Hidden system columns do come back generally
+(`organization_id` and its siblings), but they are load-bearing in client
+payloads today; removing them is a contract decision, not a defect fix.
+
+New exports from `@objectstack/objectql`: `stripSearchCompanion` and
+`isSearchCompanionRequested`.

packages/objectql/src/core.ts

@@ -31,6 +31,8 @@ export {
  resolveSearchCompanionSources,
  isCompanionSourceEligible,
  isCompanionMatchableTerm,
+ isSearchCompanionRequested,
+ stripSearchCompanion,
  containsCJK,
 } from './search-companion.js';
 export type { CompanionFieldMeta, CompanionObjectMeta } from './search-companion.js';

packages/objectql/src/engine.ts

@@ -131,6 +131,7 @@ import {
 import { pluralToSingular, ExternalWriteForbiddenError } from '@objectstack/spec/shared';
 import { SchemaRegistry, computeFQN } from './registry.js';
 import { expandSearchToFilter } from './search-filter.js';
+import { isSearchCompanionRequested, stripSearchCompanion } from './search-companion.js';
 import { ExpressionEngine } from '@objectstack/formula';
 import type { Expression } from '@objectstack/spec';
 import { isAggregatedViewContainer, expandViewContainer } from '@objectstack/spec';
@@ -4755,6 +4756,52 @@ export class ObjectQL implements IObjectQLEngine {
  }
  }
 
+ /**
+ * [#7642] Strip the hidden `__search` companion column from what a read
+ * hands back, unless a SYSTEM caller named it in its projection.
+ *
+ * The column is declared client-invisible (`hidden` + `readonly` + `system`
+ * + `searchable: false`) and the enforcement that exists is real: it is kept
+ * out of auto-views, out of the `$search` auto-default, and a `$searchFields`
+ * override naming it is refused with a 400 ("is hidden"). What was missing is
+ * the PROJECTION half — a query that names no `fields` reaches the driver
+ * with `ast.fields` undefined, every driver answers that with `SELECT *`, and
+ * the companion rode back in every record body: list results, GET by id,
+ * `/search` hits (which are `engine.find` rows verbatim) and the 201 create
+ * body. The rule is applied HERE, at the engine, because the engine is the
+ * PRODUCER those four surfaces share; fixing them one consumer at a time is
+ * how three of the four would stay broken.
+ *
+ * Two carve-outs, both measured rather than defensive:
+ *
+ * - **A system caller that asks for it by name keeps it.** The companion has
+ * exactly one such reader: `plugin-pinyin-search`'s backfill/reconcile
+ * walk, which projects `['id', ...sources, '__search']` under
+ * `{ isSystem: true }` and compares the stored blob against the recomputed
+ * one. Strip it unconditionally and that comparison reads `undefined`
+ * every pass — the backfill would rewrite every row of every object on
+ * every run, which is worse than the disclosure it was fixing.
+ * - **A non-system caller does NOT keep it, even by name.** `select` only
+ * gates on whether a field is KNOWN (`assertProjectionFieldsExist`), and
+ * the companion is known once provisioned — so `?select=__search` would
+ * otherwise be an open door straight through this strip, and a
+ * client-invisibility rule with a documented spelling that bypasses it is
+ * not one. `isSystem` is server-derived (never client input), the same
+ * trust the read-only strips on the write path already place in it.
+ *
+ * ⚠️ `requestedFields` must be the CALLER's `fields`, captured before
+ * `planFormulaProjection` — that pass rewrites the projection to every stored
+ * column when a formula is in play, companion included.
+ */
+ private stripSearchCompanionFromRead(
+ rows: unknown,
+ requestedFields: readonly string[] | undefined,
+ context: ExecutionContext | undefined,
+ ): void {
+ if (context?.isSystem && isSearchCompanionRequested(requestedFields)) return;
+ stripSearchCompanion(rows);
+ }
+
  /**
  * Dereference a stored secret ref back to its plaintext. Intended for
  * privileged, server-side consumers (e.g. a datasource connection-pool
@@ -6867,6 +6914,10 @@ export class ObjectQL implements IObjectQLEngine {
  const _findSchema = this._registry.getObject(object);
 
  this.expandSearchOnAst(ast, _findSchema);
+ // [#7642] The caller's OWN projection, captured before any planning pass
+ // rewrites it — the only thing that can answer "did this caller ask for
+ // `__search`?". See `stripSearchCompanionFromRead`.
+ const _findRequestedFields = Array.isArray(ast.fields) ? [...ast.fields] : undefined;
  // [#7095] Before the projection is planned and before anything is handed to
  // a driver: an ORDER BY this engine cannot materialise is refused, not
  // dropped. `fillQueryAstDefaults` has already normalised `orderBy` into
@@ -6953,6 +7004,12 @@ export class ObjectQL implements IObjectQLEngine {
  // resolveSecret() against the stored ref instead.
  this.maskSecretFields(object, hookContext.result);
 
+ // [#7642] …and never let the hidden `__search` companion column out
+ // through the default projection either. After the hooks, for the
+ // same reason the mask is: a server-side `afterFind` handler is not
+ // the client this column is hidden from.
+ this.stripSearchCompanionFromRead(hookContext.result, _findRequestedFields, opCtx.context);
+
  return hookContext.result;
  } catch (e) {
  this.logger.error('Find operation failed', e as Error, { object });
@@ -7023,6 +7080,8 @@ export class ObjectQL implements IObjectQLEngine {
  // dropped sort does not merely reorder the answer, it returns a DIFFERENT
  // record, and the one it returns looks exactly as legitimate.
  assertOrderByIsMaterializable(objectName, 'findOne', _findOneSchema, ast.orderBy);
+ // [#7642] Caller's own projection, before planning rewrites it — see `find`.
+ const _findOneRequestedFields = Array.isArray(ast.fields) ? [...ast.fields] : undefined;
  const _findOneFormula = planFormulaProjection(_findOneSchema, ast.fields);
  if (_findOneFormula.projected) ast.fields = _findOneFormula.projected;
 
@@ -7089,6 +7148,10 @@ export class ObjectQL implements IObjectQLEngine {
 
  // Mask secret fields — plaintext never leaves through the read path.
  this.maskSecretFields(objectName, hookContext.result);
+ // [#7642] Hidden `__search` companion — same door, same rule as `find`.
+ // This is the `GET /data/:object/:id` surface (`getData` reads through
+ // findOne), one of the four the issue measured.
+ this.stripSearchCompanionFromRead(hookContext.result, _findOneRequestedFields, opCtx.context);
 
  return hookContext.result;
  });
@@ -7655,6 +7718,15 @@ export class ObjectQL implements IObjectQLEngine {
  rowCtx.event = 'afterInsert';
  rowCtx.result = coerceBooleanFields(schemaForValidation as any, resultRows[k] as any);
  await this.triggerHooks('afterInsert', rowCtx);
+ // [#7642] The 201 create body is the surface most likely to be missed
+ // on this card, and the one no read-path fix reaches: `createData`
+ // returns `engine.insert`'s value verbatim as `record`, so the
+ // companion the `beforeInsert` stamp just wrote came straight back to
+ // the client. A write has no projection to consult, so there is no
+ // "asked for it by name" case to honour — the strip is unconditional.
+ // AFTER the hook dispatch, matching the read path: `afterInsert`
+ // handlers still observe the whole stored row.
+ stripSearchCompanion(rowCtx.result);
  }
 
  // Roll-up: recompute parent summary fields that aggregate this object.
@@ -8581,6 +8653,17 @@ export class ObjectQL implements IObjectQLEngine {
  }
  }
 
+ // [#7642] Same strip the create body gets, for the same reason: a
+ // by-id update resolves to a RECORD, `updateData` returns it as
+ // `record`, and the `beforeUpdate` companion stamp had just written
+ // `__search` into the row it echoes. The issue measured four
+ // surfaces and this is not one of them — it is the same column, the
+ // same contract and the same response shape, and leaving it out
+ // would mean POST and PATCH on one object disagreed about whether a
+ // client-invisible column is visible. A predicate update resolves to
+ // an affected-row COUNT (#4639), which the strip skips as a
+ // non-object.
+ stripSearchCompanion(hookContext.result);
  // The record IS updated; a summary that could not recompute after
  // retries must surface, not stay silent (framework#3147).
  if (summaryFailures.length > 0) throw new SummaryRecomputeError(summaryFailures, hookContext.result);

packages/objectql/src/index.ts

@@ -37,6 +37,8 @@ export {
  resolveSearchCompanionSources,
  isCompanionSourceEligible,
  isCompanionMatchableTerm,
+ isSearchCompanionRequested,
+ stripSearchCompanion,
  containsCJK,
 } from './search-companion.js';
 export type { CompanionFieldMeta, CompanionObjectMeta } from './search-companion.js';