Merge remote-tracking branch 'origin/main' into claude/issue-21042-native-aggregate-policies

Author: claude

.changeset/21066-memory-json-column-family-refusal.md

@@ -0,0 +1,21 @@
+---
+"@objectstack/driver-memory": minor
+---
+
+fix(driver-memory)!: on a declared JSON-stored field, the query path and the analytics face refuse `$eq` / `$ne` / an ordering / `$between` / `$in` / `$nin` / implicit equality with `INVALID_FILTER` / 400, in the words the SQL family refuses them in, instead of answering each per element
+
+Clause-②: yes (narrowing)
+
+<!-- adr-0087: not-required (no-migration-prescription) a refusal of a QUERY shape at this driver's filter gate: the operator x declared-type pairs refused are exactly the pairs driver-sql's where refuses on a JSON-stored column and the engine's per-aggregation filter refuses on the same declared fields, read from the one set @objectstack/core holds. No authorable key, spelling or stored metadata shape moves: FilterConditionSchema and every object, view and dataset definition parse and save as before, and nothing reads or rewrites a stored row. There is nothing for objectstack migrate meta to rewrite, since what changes is which query this driver answers, not what any metadata says; the refusal itself names the spelling to use. The other categories are closed on facts: the bumped package publishes (not unpublished); no ADR-0087 id covers a filter operator on a JSON-stored column and this diff adds none (not registered / already-registered); and the change is runtime behaviour, with no published export or type narrowed or removed (not runtime-interface-only / type-surface-only). -->
+
+**BREAKING** (`@objectstack/driver-memory`): this narrows what the driver's filter doors accept, for every caller that reaches them: `find`, `findOne`, `count`, `updateMany`, `deleteMany` and `aggregate` with a `where`, through the engine or called directly, and `MemoryAnalyticsService`'s `query` and `generateSql`. It ships as `minor` under the launch-window convention for accept-set narrowings.
+
+**What is refused.** On a field the object declares JSON-stored (a structured-JSON type such as `json` or `address`, an inherently multi-value option type such as `tags`, `multiselect` or `checkboxes`, or a `select`, `radio`, `lookup`, `user`, `file` or `image` field declared `multiple: true`), a `where` that compares the field with `$eq`, `$ne`, `$gt`, `$gte`, `$lt`, `$lte`, `$between`, `$in`, `$nin` or implicit equality (`{ "owners": "u1" }`) is refused with `INVALID_FILTER` / 400, whatever the comparand (`null` and an empty list included), at any depth under `$and` / `$or` / `$not`, before any row is read. On the analytics face a `where` key is a cube member, judged by the field it resolves to. That is the set `driver-sql` refuses on such a column, for the same reason.
+
+**What an author sees now.** The body `driver-sql` answers for the same filter: the filter WAS NOT APPLIED, the comparison can never equal one member of a stored list, and the spelling to use, `{ "FIELD": { "$contains": "a" } }` for membership, or an `$or` of `$contains` for any-of. The field and the operator are withheld from the message, and the full diagnostic, naming both and the position in the filter, is written to the driver's (or the analytics service's) logger at `warn`.
+
+**Why a refusal.** This driver answered each of those operators per element, through mingo's array semantics. Measured through `engine.find` over six rows of a `multiple: true` lookup, two of them holding `u1`: `{ owners: { $eq: 'u1' } }` and `{ owners: { $in: ['u1', 'u9'] } }` returned those two rows, `$nin` the other four, and `{ owners: { $gt: 'u1' } }` four rows by comparing each member as text, where every SQL dialect answers the same filters 400. An application whose tests run on this driver passed on a filter its production backend refuses.
+
+**Who is affected.** A test suite, demo or dev setup on this driver that filters a JSON-stored field with one of those operators and read the per-element rows as the answer. Write `$contains` for "holds this member", an `$or` of `$contains` for "holds any of these", and `$not` around either for the exclusion.
+
+**Unchanged.** `$contains` and `$notContains` (membership on such a field), `$exists`, `$null` and `$empty`; every operator on a field that is not declared JSON-stored; and an object this driver holds no declaration for (one never passed through `syncSchema`), where nothing is judged and every operator answers as before. `InMemoryDriver` gains one method, `filterFieldDeclarations`, tagged `@internal`: it exists so the analytics face judges its `where` by the same declarations, and it is not a consumer contract.

.changeset/21080-native-sql-engine-middleware-decline.md

@@ -0,0 +1,25 @@
+---
+'@objectstack/spec': minor
+'@objectstack/objectql': minor
+'@objectstack/service-analytics': minor
+---
+
+fix(service-analytics)!: the analytics native-SQL strategy declines an object an engine middleware is registered for, so the engine serves it and that object's read gates apply; the engine answers which objects carry one (`IObjectQLEngine.hasObjectMiddleware`) (#21080)
+
+Clause-②: yes (narrowing)
+
+<!-- adr-0087: not-required (no-migration-prescription) a correction of which analytics strategy serves a query that reads an object the data engine holds a per-object middleware for, decided at request time. No authorable key, spelling, value domain or stored metadata shape moves: every dataset, cube and dashboard parses as before, nothing stored is rewritten, and the only declaration change is ADDITIVE (one optional member on IObjectQLEngine, one public method on ObjectQL, one optional member on AnalyticsServiceConfig), so there is nothing for an author to convert and nothing for `objectstack migrate meta` to reach. The queries newly refused are refused at request time by the ObjectQL strategy's existing envelope, not by a schema. The other categories are closed on facts: the packages publish (not unpublished); no ADR-0087 id covers strategy routing and this diff adds none (not registered / already-registered); and the change is runtime behaviour plus additive declarations, not a removal from a published interface (not runtime-interface-only / type-surface-only). -->
+
+**BREAKING**: this narrows what the analytics doors serve for one class of query. It ships as `minor` under the launch-window convention for narrowings.
+
+**What changes.** On a SQL driver, `NativeSQLStrategy` compiled a query to SQL and ran it through the driver's raw-SQL seam, so no engine operation ran and no engine middleware did. It applied the security service's object admission and read filter and nothing else, so the read gates that live in the engine as per-object middlewares did not apply there: a caller admitted to such an object at object level read grouped results and counts over every row, rows about parent records that caller cannot read included. It now declines a query that reads (as its base object, a declared join, or through a relationship path) an object the data engine holds a middleware registered for. The ObjectQL strategy serves it through the engine with the caller's context, so the engine's middlewares run, and the analytics answer for that caller equals the data door's. On the stock composition the objects that move off the native path are `sys_comment`, `sys_activity` and `sys_attachment` (read gates), `sys_approval_request` (the snapshot redaction), and `sys_user_position` and `sys_permission_set` (write-side middlewares, which move as a side effect: a middleware does not declare its operation). No shipped dataset or dashboard reads any of them.
+
+**What is newly refused.** A query on such an object that the ObjectQL strategy cannot serve is refused with that strategy's existing `400`, where the native strategy used to serve it: for example a dimension reached through a relationship path combined with a measure that cannot be recombined across it (`avg`, `count_distinct`). Correctness wins over the fast path for a gated object.
+
+**It fails closed.** `AnalyticsServicePlugin` asks the data engine. An engine without `hasObjectMiddleware`, or no engine, cannot say, and the strategy declines then too: every query on such a host is served by the ObjectQL strategy, and the plugin says so once at `warn`. A host that constructs `AnalyticsService` with `executeRawSql` and without the new `hasObjectMiddleware` config member keeps the native path for every object and is told so once at construction.
+
+**New, additive.** `IObjectQLEngine.hasObjectMiddleware?(objectName): boolean` (`@objectstack/spec`), `ObjectQL.hasObjectMiddleware(objectName)` (`@objectstack/objectql`): whether a `registerMiddleware(fn, { object })` names the object; a global registration (no `object`, or `'*'`) is keyed to none and is not counted. `AnalyticsServiceConfig.hasObjectMiddleware` (`@objectstack/service-analytics`), which the plugin fills from the data engine.
+
+**Unchanged.** Objects no middleware names keep the native path. The middleware chain, `registerMiddleware` and every gate are unchanged.
+
+**What to do after upgrading.** Nothing on the stock composition. A host whose `"data"` service is not ObjectQL should implement `hasObjectMiddleware` to keep the native path for ungated objects. A host that builds `AnalyticsService` itself with `executeRawSql` should pass `hasObjectMiddleware` from its engine.

.changeset/21156-analytics-caller-member-column-reference.md

@@ -0,0 +1,7 @@
+---
+"@objectstack/service-analytics": patch
+---
+
+Clause-②: no
+
+Security: refuse a caller-named analytics member that is neither a declared cube member nor a column reference at the query door, in every tier — including a deployment with no security service and an object the field-level read gate does not judge — so caller-supplied member text can no longer reach a native statement unjudged. The refusal reuses the existing field-read gate's envelope (`PERMISSION_DENIED` / 403); no new error code, and the declared-cube paths are unchanged.

packages/drivers/driver-memory/src/filter-refusal.ts

@@ -46,6 +46,11 @@ import { BUCKET_GRANULARITIES } from '@objectstack/core';
 // refusal can tell a value the contract never declared apart from one it
 // declares and this backend cannot label.
 import { RETIRED_SUB_DAY_INTERVALS, TimeUpdateInterval } from '@objectstack/spec/data';
+// [#21066] The scalar-comparison family a JSON-stored field refuses, and the
+// words of that refusal — the ONE set and sentence `driver-sql`'s `where` and
+// `@objectstack/objectql`'s per-aggregation `filter` already read (#21007), so
+// this driver's faces refuse the same operators in the same words.
+import { JSON_COLUMN_INCOMPATIBLE_OPERATORS, jsonColumnOperatorRefusalText } from '@objectstack/core';
 import { StandardErrorCode } from '@objectstack/spec/api';
 
 /**
@@ -821,6 +826,91 @@ export function arrayComparandError(field: string, value: unknown, path: string,
   );
 }
 
+/**
+ * [#21066] What the shape gate is told about the DECLARED fields a filter names.
+ *
+ * Every other rule on the walk reads the filter alone. This one cannot: whether
+ * `{ owners: { $in: ['u1'] } }` is a well-formed question depends on how
+ * `owners` is STORED, which is declared metadata the calling face holds and the
+ * filter does not carry. So the face hands the walk the two things it needs, and
+ * the walk keeps deciding everything in one pass, before any face evaluates.
+ *
+ * Omitted (the default) ⇒ no field is judged JSON-stored, which is the answer
+ * for a face holding no declarations: the walk does not guess a storage shape
+ * from a name or a value.
+ */
+export interface FilterFieldDeclarations {
+  /**
+   * Is `field`, as the filter spells it, DECLARED JSON-stored on the object
+   * being read — a structured-JSON type or a multi-valued field? A field the
+   * face holds no declaration for answers `false`.
+   */
+  readonly isJsonStoredField: (field: string) => boolean;
+  /**
+   * Handed the withheld half of a refusal — the field and the operator named,
+   * with the position in the filter — just before the refusal is thrown, so the
+   * face writes it to its server log. The caller is told only that it is there.
+   */
+  readonly reportWithheld: (diagnostic: string) => void;
+}
+
+/**
+ * [#21066] The server-log line for a withheld filter diagnostic, one wording
+ * for every face of this package.
+ */
+export function withheldFilterLogLine(diagnostic: string): string {
+  return (
+    `[driver-memory] INVALID_FILTER — refusal detail withheld from the response. ` +
+    `Full diagnostic: ${diagnostic}`
+  );
+}
+
+/**
+ * [#21066] A scalar comparison — the equality and ordering family, `$between`,
+ * `$in` / `$nin`, or implicit equality — aimed at a field DECLARED JSON-stored.
+ *
+ * ## What this driver answered instead
+ *
+ * Each operator, PER ELEMENT: mingo applies a scalar comparison to every member
+ * of an array value, so on six rows whose `owners` (a `multiple: true` lookup)
+ * hold `['u1','u2']`, `['u2']`, `['u3','u1']`, `[]`, `['u10']` and `null`,
+ * `{ owners: { $eq: 'u1' } }` and `$in ['u1','u9']` returned the two rows
+ * holding `u1`, `$nin` the other four, and `$gt 'u1'` four rows by comparing
+ * each member as text. `driver-sql` refuses every one of those filters with
+ * `INVALID_FILTER` / 400 on every dialect (#7398), and so does the engine's
+ * per-aggregation `filter` (#21007). One filter, rows on this driver and a
+ * refusal on the SQL family, breaks the conformance invariant every backend is
+ * held to — the same rows as `find()`, or a refusal — and it is the worse
+ * direction for a test double: an application's tests pass here on a filter
+ * its production backend refuses.
+ *
+ * No declared contract gives the family a per-element reading: the spec's
+ * `$contains` docblock names `$contains` as the membership spelling on such a
+ * column and the one operator the equality family's refusal left working, and
+ * that is what the refusal prescribes.
+ *
+ * ## The words, and what they withhold
+ *
+ * The set and the sentence are `@objectstack/core`'s, read rather than copied —
+ * the text `driver-sql` refuses with, byte for byte. The message names neither
+ * the field nor the operator (on a read scope the predicate is an
+ * administrator's), and says the full diagnostic is in the server log; the
+ * diagnostic, prefixed with the position in the filter, goes to
+ * `reportWithheld` just before the refusal is thrown, so that sentence is true
+ * here too. The constructor is this package's, as each face keeps its own.
+ */
+export function jsonStoredFieldOperatorError(
+  field: string,
+  op: string,
+  bare: boolean,
+  path: string,
+  reportWithheld: (diagnostic: string) => void,
+): Error {
+  const { message, diagnostic } = jsonColumnOperatorRefusalText(field, op, bare);
+  reportWithheld(`At ${path}: ${diagnostic}`);
+  return unsupportedFilterError(message);
+}
+
 /** [#5324] `$and`/`$or` take a list of nodes; anything else is refused. */
 export function filterNodeListExpectedError(key: string, value: unknown, path: string): Error {
   return unsupportedFilterError(
@@ -890,11 +980,23 @@ export function filterNodeExpectedError(value: unknown, path: string): Error {
  * deliberately: on a face that cannot compile `$or` at all, reporting that its
  * operand should have been an array would send the author to fix the wrong
  * thing, then refuse the corrected filter anyway.
+ *
+ * ## What `declarations` adds (#21066)
+ *
+ * The one rule on this walk that reads the DECLARATION rather than the filter:
+ * a scalar comparison aimed at a field declared JSON-stored is refused (see
+ * {@link jsonStoredFieldOperatorError}). It is a MEANING the filter would get
+ * wrong rather than a shape, and it lives here anyway, because here is where
+ * every face of this package already stops a filter before evaluating it — on
+ * the whole tree at once, `$not` and every `$or` branch included, so the
+ * refusal cannot depend on which row reaches which arm. Omitted ⇒ nothing is
+ * judged JSON-stored (see {@link FilterFieldDeclarations}).
  */
 export function assertFilterConditionShape(
   node: unknown,
   path: string,
   capabilities: FilterFaceCapabilities = DRIVER_FILTER_CAPABILITIES,
+  declarations?: FilterFieldDeclarations,
 ): void {
   if (!isFilterNode(node)) return;
   for (const [key, value] of Object.entries(node)) {
@@ -905,18 +1007,18 @@ export function assertFilterConditionShape(
       value.forEach((child, index) => {
         const childPath = `${here}[${index}]`;
         if (!isFilterNode(child)) throw filterNodeExpectedError(child, childPath);
-        assertFilterConditionShape(child, childPath, capabilities);
+        assertFilterConditionShape(child, childPath, capabilities, declarations);
       });
       continue;
     }
     if (key === '$not') {
       if (!capabilities.combinators.has(key)) throw uncompilableCombinatorError(key, here, capabilities);
       if (!isFilterNode(value)) throw filterNodeExpectedError(value, here);
-      assertFilterConditionShape(value, here, capabilities);
+      assertFilterConditionShape(value, here, capabilities, declarations);
       continue;
     }
     if (key.startsWith('$')) throw unknownLogicalOperatorError(key, here);
-    assertFieldConstraintShape(key, value, here, capabilities);
+    assertFieldConstraintShape(key, value, here, capabilities, declarations);
   }
 }
 
@@ -936,13 +1038,25 @@ function assertFieldConstraintShape(
   spec: unknown,
   path: string,
   capabilities: FilterFaceCapabilities,
+  declarations: FilterFieldDeclarations | undefined,
 ): void {
   // [#16810] The IMPLICIT-equality position, checked before the plain-object
   // test below because an array is not a filter node and would otherwise leave
   // this walk unjudged — which is how it reached the matcher's `==` arm and the
   // live path's deep equality with nobody reconciling the two.
   if (Array.isArray(spec)) throw arrayComparandError(field, spec, path);
-  if (!isFilterNode(spec)) return;
+  if (!isFilterNode(spec)) {
+    // [#21066] The bare `{ field: value }` spelling IS equality, and on a
+    // declared JSON-stored field it is refused as `$eq` is — reported as `=`,
+    // bare, as `driver-sql` reports it — whatever the comparand, `null`
+    // included, because `driver-sql`'s `where` refuses that one too. After the
+    // array refusal above, the order `driver-sql` takes (its comparand gate,
+    // then its column-type gate).
+    if (declarations?.isJsonStoredField(field)) {
+      throw jsonStoredFieldOperatorError(field, '=', true, path, declarations.reportWithheld);
+    }
+    return;
+  }
   // [#5240] The zero-operator constraint keeps its own predicate rather than an
   // inlined `keys.length === 0`, so the reasoning for what does and does not
   // count as one (a `Date` enumerates to nothing but is a comparand) stays
@@ -1028,6 +1142,18 @@ function assertFieldConstraintShape(
         throw nulLikePatternError(field, op, spec[op] as string, `${path}.${op}`);
       }
     }
+    // [#21066] The column-type question, after every comparand-shape rule above
+    // — the order `driver-sql` takes, so a malformed `$between` or an array
+    // under `$eq` is still told what is wrong with its COMPARAND first. The set
+    // is the shared one, so `$contains` / `$notContains` (membership), the null
+    // predicates and `$empty` keep answering on such a field.
+    if (
+      declarations &&
+      JSON_COLUMN_INCOMPATIBLE_OPERATORS.has(op) &&
+      declarations.isJsonStoredField(field)
+    ) {
+      throw jsonStoredFieldOperatorError(field, op, false, `${path}.${op}`, declarations.reportWithheld);
+    }
   }
   // [#5702] The `$options`-without-`$regex` companion check that stood here is
   // GONE. It was needed while `$options` was an allowlisted MODIFIER — a key the