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

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 QA measured
(#7629): query results, GET by id, `/search` hits, and the 201 create body.

The strip runs at the engine, the producer all four surfaces share — `/search`
hits are `engine.find` rows verbatim, the create body is `engine.insert`'s
return verbatim, so fixing consumers one at a time would have left three of the
four broken. Covered: `find`, `findOne`, the nested records `expand` produces,
the create response and the update response. The update response is not one of
the four reported surfaces but is the same column in the same response shape;
leaving it out would make POST and PATCH on one object disagree about whether a
client-invisible column is visible. A predicate update resolves to a count and
is unaffected.

Two shaping details, both from the report:

- Not gated on the schema declaring the column. The symptom survived a restart
  with `OS_SEARCH_PINYIN_ENABLED=false`: 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 filed the bug.
- One caller keeps its read. `plugin-pinyin-search`'s backfill projects
  `['id', ...sources, '__search']` under a system context and compares the
  stored blob against a recomputed one; stripping it unconditionally would make
  the walk rewrite every row of every object on every pass. A SYSTEM caller that
  names the column still gets it — a non-system caller does not, even by name,
  since `select` only gates on whether a field is KNOWN and `?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.

The new suite is a MATRIX over every record-returning door rather than a test
for the door that was fixed, in both provisioning states — one contract, five
places that can break it independently is the shape that rots one door at a
time. Reverse-checked: 17 of its 24 cases fail on the unfixed engine.

Co-authored-by: Claude <noreply@anthropic.com>

Author: huangyiirene

.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';