fix(service-analytics): the field-level read gate refuses a cube member that names no field instead of standing down (#20965)

.changeset/20965-analytics-expression-member-refused.md

@@ -0,0 +1,37 @@
+---
+'@objectstack/service-analytics': patch
+---
+
+fix(service-analytics): the analytics field-level read gate refuses a cube member whose `sql` names no field, instead of letting the query run (#20965)
+
+Clause-②: no
+
+**What changed.** Where the analytics field-level read gate judges a cube's
+object (a security service is registered and gives a field answer for that
+object), a query that names a cube member whose `sql` is neither a column
+reference (a field of the cube's object, or a relationship path ending in one)
+nor `'*'` is now refused `403 PERMISSION_DENIED` on
+`POST /api/v1/analytics/query` and `POST /api/v1/analytics/sql`, before either
+strategy runs. Whatever
+the caller may read, the member is refused. That covers an expression member
+of a cube that reached the service without the spec's parse (the cube
+registry never parses: `analyticsCubes` and `AnalyticsServicePlugin({ cubes })`
+arrive as written), a declared member with no `sql` string, and a member the
+query names itself that is not a column reference. The gate used to stand down
+on such a member, because it names no field, and the native-SQL strategy then
+compiled it into its statement as written: a read of fields no permission
+verdict was reached for. The refusal names the member and the object, and
+never the member's `sql`.
+
+**What is not affected.** A member that is a column reference is judged by the
+field it resolves to, as before. A `count` over `'*'` names no field and is
+served. A deployment with no security service, and an object the security
+service gives no field answer for, apply no field-level check, as before. The
+spec's parse already refuses an expression member, so a cube that parses is
+unaffected.
+
+**If a widget stopped answering,** its cube carries an expression member from
+before the parse refused one. Re-author the member as a column reference, or
+declare the derived value on an ADR-0021 dataset: a conditional count or sum
+is a dataset measure with its own `filter`, and a ratio of measures is
+`derived: { op: 'ratio', of: [...] }`.

packages/services/service-analytics/src/__tests__/field-read-admission-gate.test.ts

@@ -15,11 +15,18 @@
  * The route-level half, over the real `SecurityPlugin`, `ObjectQL` and
  * `SqlDriver`, compares each refusal with the engine's own answer:
  * `packages/rest/src/analytics-field-permission-gate.test.ts`.
+ *
+ * [#20965] A member that resolves to neither a field nor `'*'` is refused, in
+ * the same envelope, and never stood down — see the block of that name below.
+ * Its fixture, `AUTHORED`'s two expression members, is written around the
+ * parse: `CubeSchema` refuses both, and the registry never parses, which is
+ * how a cube configured before that refusal (or never put through it) still
+ * reaches the gate.
  */
 
 import { describe, it, expect, vi } from 'vitest';
 import { DatasetSchema } from '@objectstack/spec/ui';
-import type { Cube } from '@objectstack/spec/data';
+import { CubeSchema, type Cube } from '@objectstack/spec/data';
 import type { ExecutionContext } from '@objectstack/spec/kernel';
 import { AnalyticsService } from '../analytics-service.js';
 import { AnalyticsServicePlugin } from '../plugin.js';
@@ -62,6 +69,16 @@ const AUTHORED: Cube = {
   joins: { owner: { name: OWNER }, hidden_link: { name: OWNER } },
 } as Cube;
 
+/** [#20965] A declared member with no `sql` string at all — refused at parse, unparsed here. */
+const NO_SQL: Cube = {
+  name: 'fr_no_sql',
+  title: 'No sql',
+  sql: LEDGER,
+  public: true,
+  measures: { count: { type: 'count', sql: '*', label: 'Count' } },
+  dimensions: { bare: { type: 'string', label: 'Bare' } },
+} as unknown as Cube;
+
 /** A registered dataset whose OWN filter, and one of whose measures' filter, name a hidden field. */
 const SCOPED_DATASET = DatasetSchema.parse({
   name: 'fr_scoped',
@@ -105,7 +122,7 @@ function makeService(opts: {
 }) {
   const executed: string[] = [];
   const service = new AnalyticsService({
-    cubes: [AUTHORED],
+    cubes: [AUTHORED, NO_SQL],
     datasets: [SCOPED_DATASET, MEASURE_FILTER_DATASET],
     queryCapabilities: opts.capabilities,
     getReadableFields: opts.getReadableFields,
@@ -201,11 +218,7 @@ describe('[#20917] analytics — the field-level read gate at the door', () => {
       expect(grouped).toBe(aggregateSentence(LEDGER, ['hidden_text']));
     });
 
-    it('stands down where no field can be named: an authored expression member, and an object the reader has no answer for', async () => {
-      const { service, executed } = makeService({ capabilities: nativeSqlOnly, getReadableFields: readable });
-      await service.query({ cube: 'fr_authored', measures: ['expression_total'], dimensions: ['expression_flag'] } as never, CALLER);
-      expect(executed).toHaveLength(1);
-
+    it('stands down for an object the reader has no answer for', async () => {
       const unanswered = makeService({ capabilities, getReadableFields: (object) => (object === OWNER ? undefined : READABLE[object]) });
       await unanswered.service.query({ cube: 'fr_authored', measures: ['count'], dimensions: ['alias_owner_code'] } as never, CALLER);
       expect(unanswered.executed.length).toBeGreaterThan(0);
@@ -276,6 +289,94 @@ describe('[#20917] analytics — the field-level read gate at the door', () => {
   });
 });
 
+// ── [#20965] A member that names no field ─────────────────────────────────────
+
+/** The cube author's expression text — which no refusal may hand back to the caller. */
+const AUTHORED_EXPRESSIONS = [
+  (AUTHORED.measures as Record<string, { sql: string }>).expression_total.sql,
+  (AUTHORED.dimensions as Record<string, { sql: string }>).expression_flag.sql,
+];
+
+/** A member the query names itself, spelled as no column is. */
+const NAMED_EXPRESSION = 'hidden_number * 2';
+
+interface ExpressionCase {
+  label: string;
+  query: Record<string, unknown>;
+  member: string;
+}
+
+const EXPRESSION_REFUSED: readonly ExpressionCase[] = [
+  { label: 'an aggregated expression measure', query: { cube: 'fr_authored', measures: ['expression_total'] }, member: 'expression_total' },
+  { label: 'a grouped expression dimension', query: { cube: 'fr_authored', measures: ['count'], dimensions: ['expression_flag'] }, member: 'expression_flag' },
+  { label: 'a filtered expression member', query: { cube: 'fr_authored', measures: ['count'], where: { expression_flag: 1 } }, member: 'expression_flag' },
+  { label: 'an expression member as an order key', query: { cube: 'fr_authored', measures: ['count'], dimensions: ['title'], order: { expression_flag: 'asc' } }, member: 'expression_flag' },
+  { label: 'a declared member with no sql string', query: { cube: 'fr_no_sql', measures: ['count'], dimensions: ['bare'] }, member: 'bare' },
+  { label: 'a member the query names itself that is not a column reference', query: { cube: 'fr_authored', measures: ['count'], dimensions: [NAMED_EXPRESSION] }, member: NAMED_EXPRESSION },
+];
+
+describe('[#20965] the field-level read gate — a member that names no field is refused, never stood down', () => {
+  it('the fixture is written around the parse: CubeSchema refuses exactly the members the gate now refuses', () => {
+    for (const [cube, paths] of [
+      [AUTHORED, ['dimensions.expression_flag.sql', 'measures.expression_total.sql']],
+      [NO_SQL, ['dimensions.bare.sql']],
+    ] as const) {
+      const parsed = CubeSchema.safeParse(cube);
+      expect(parsed.success).toBe(false);
+      expect(parsed.error?.issues.map((issue) => issue.path.join('.')).sort()).toEqual(paths);
+    }
+  });
+
+  describe.each(STRATEGY_PATHS)('$label', ({ capabilities }) => {
+    it.each(EXPRESSION_REFUSED)('$label: refused PERMISSION_DENIED / 403 on both doors, before any strategy ran, without the author\'s text', async ({ query, member }) => {
+      const { service, executed } = makeService({ capabilities, getReadableFields: readable });
+      for (const run of [() => service.query(query as never, CALLER), () => service.generateSql(query as never, CALLER)]) {
+        const refusal = await run().then(() => null, (e: unknown) => e as Record<string, unknown>);
+        expect(refusal).toMatchObject({ code: 'PERMISSION_DENIED', status: 403, object: LEDGER, member });
+        for (const text of AUTHORED_EXPRESSIONS) expect(String(refusal?.message)).not.toContain(text);
+      }
+      expect(executed).toEqual([]);
+    });
+
+    it('no grant makes it judgeable: a reader answering every field of the object still refuses it', async () => {
+      const { service, executed } = makeService({ capabilities, getReadableFields: (object) => FIELDS[object] });
+      await expect(
+        service.query({ cube: 'fr_authored', measures: ['expression_total'] } as never, CALLER),
+      ).rejects.toMatchObject({ code: 'PERMISSION_DENIED', status: 403, object: LEDGER, member: 'expression_total' });
+      expect(executed).toEqual([]);
+    });
+
+    it('it is refused ahead of a hidden field on the same object', async () => {
+      const { service } = makeService({ capabilities, getReadableFields: readable });
+      await expect(
+        service.query({ cube: 'fr_authored', measures: ['expression_total'], dimensions: ['alias_code'] } as never, CALLER),
+      ).rejects.toMatchObject({ code: 'PERMISSION_DENIED', status: 403, object: LEDGER, member: 'expression_total' });
+    });
+
+    it('every field member is judged as before: a query naming none of the cube\'s expression members gets the field verdicts', async () => {
+      const { service, executed } = makeService({ capabilities, getReadableFields: readable });
+      await expect(
+        service.query({ cube: 'fr_authored', measures: ['alias_total'] } as never, CALLER),
+      ).rejects.toMatchObject({ code: 'PERMISSION_DENIED', status: 403, object: LEDGER, fields: ['hidden_number'] });
+      expect(executed).toEqual([]);
+      await service.query({ cube: 'fr_authored', measures: ['count'], dimensions: ['title', 'alias_owner_region'] } as never, CALLER);
+      expect(executed.length).toBeGreaterThan(0);
+    });
+
+    it('\'*\' still counts (the control): a count names no field and is served, beside a field member and alone', async () => {
+      const beside = makeService({ capabilities, getReadableFields: vi.fn(readable) });
+      await beside.service.query({ cube: 'fr_authored', measures: ['count'], dimensions: ['title'] } as never, CALLER);
+      expect(beside.executed.length).toBeGreaterThan(0);
+
+      const getReadableFields = vi.fn(readable);
+      const alone = makeService({ capabilities, getReadableFields });
+      await alone.service.query({ cube: 'fr_authored', measures: ['count'] } as never, CALLER);
+      expect(alone.executed.length).toBeGreaterThan(0);
+      expect(getReadableFields).not.toHaveBeenCalled();
+    });
+  });
+});
+
 // ── The plugin's bridge to the `security` service ─────────────────────────────
 
 function fakeEngine() {
@@ -298,7 +399,7 @@ function fakeEngine() {
   };
 }
 
-async function bootPlugin(security?: () => unknown) {
+async function bootPlugin(security?: () => unknown, cubes?: Cube[]) {
   const { engine, reads } = fakeEngine();
   const registered: Record<string, unknown> = {};
   const error = vi.fn();
@@ -312,7 +413,7 @@ async function bootPlugin(security?: () => unknown) {
     replaceService: (name: string, svc: unknown) => { registered[name] = svc; },
     logger: { info() {}, warn() {}, error, debug() {} },
   };
-  await new AnalyticsServicePlugin({ queryCapabilities: nativeSqlOnly }).init(ctx as never);
+  await new AnalyticsServicePlugin({ queryCapabilities: nativeSqlOnly, ...(cubes ? { cubes } : {}) }).init(ctx as never);
   return { service: registered.analytics as AnalyticsService, reads, error };
 }
 
@@ -344,4 +445,16 @@ describe('[#20917] analytics plugin — the field-level half of the "security" b
     await service.query(groupedHidden as never, CALLER);
     expect(reads).toHaveLength(1);
   });
+
+  it('[#20965] refuses a configured cube\'s expression member through the security service\'s reader — `cubes` reach the registry unparsed', async () => {
+    const getReadableFields = vi.fn(async (object: string) => READABLE[object]);
+    const { service, reads } = await bootPlugin(() => ({ ...objectAndRowsOpen, getReadableFields }), [AUTHORED]);
+    await expect(
+      service.query({ cube: 'fr_authored', measures: ['expression_total'] } as never, CALLER),
+    ).rejects.toMatchObject({ code: 'PERMISSION_DENIED', status: 403, object: LEDGER, member: 'expression_total' });
+    expect(reads).toEqual([]);
+    expect(getReadableFields).toHaveBeenCalledWith(LEDGER, CALLER);
+    await service.query({ cube: 'fr_authored', measures: ['count'], dimensions: ['title'] } as never, CALLER);
+    expect(reads).toHaveLength(1);
+  });
 });

packages/services/service-analytics/src/analytics-service.ts

@@ -51,6 +51,7 @@ import {
   assertNamedFieldsReadable,
   type QueryableFieldsProvider,
   type NamedField,
+  type NamedRead,
   type FieldReadRole,
   type ReadableFieldsProvider,
 } from './field-read-admission.js';
@@ -418,21 +419,26 @@ const IDENTIFIER_PATH = /^[A-Za-z_][A-Za-z0-9_]*(\.[A-Za-z_][A-Za-z0-9_]*)+$/;
  *   hidden relationship field discloses which record each row points to, and
  *   the engine judges a path's first segment on the local object for the same
  *   reason.
- * - Anything else is an EXPRESSION the cube's author wrote (`CASE WHEN …`,
- *   `SUM(…)`, `*`). It names no field this gate can attribute, so it adds
- *   nothing — the author's declaration of a derived value, the way a formula
- *   field is.
+ * - `'*'` reads no field value — the row wildcard a `count` uses — so it
+ *   names nothing (`[]`).
+ * - [#20965] Anything else is not a column reference: `null`. It names no
+ *   field this gate can judge, so the caller refuses the member
+ *   (`NamedExpression`, `field-read-admission.ts`) — ⛔ never an empty list,
+ *   which would pass it. The parse refuses such a `sql` (#20943), but the
+ *   registry never parses, so a cube built before that, or never put through
+ *   it, still reaches here.
  */
 function fieldsOfColumnSql(
   cube: Cube,
   baseObject: string,
   sql: string,
   role: FieldReadRole,
   referenceOf: HopReference | undefined,
-): NamedField[] {
+): NamedField[] | null {
   const path = sql.trim();
+  if (path === '*') return [];
   if (BARE_IDENTIFIER.test(path)) return [{ object: baseObject, field: path, role }];
-  if (!IDENTIFIER_PATH.test(path)) return [];
+  if (!IDENTIFIER_PATH.test(path)) return null;
   const segments = path.split('.');
   const hops = resolvePathHops(cube, baseObject, segments.slice(0, -1), referenceOf);
   const out: NamedField[] = hops.map((hop) => ({ object: hop.from, field: hop.field, role }));
@@ -466,6 +472,13 @@ function fieldsOfColumnSql(
  * it is policy, and the engine's own field guard never judges policy
  * predicates — they may name fields the caller cannot read.
  *
+ * [#20965] A member that resolves to neither a field nor `'*'` — a declared
+ * member whose `sql` is not a column reference (or is not a string), an
+ * undeclared one whose own spelling is not — is read as a `NamedExpression`
+ * on the base object, which the field gate refuses. `NativeSQLStrategy` emits
+ * such a `sql` into its statement as written, so passing it would let the
+ * statement read fields no verdict was reached for.
+ *
  * A cube whose `sql` is not a bare object name names no attributable field.
  *
  * [#20986] `referenceOf` answers a relationship field's declared target: the
@@ -476,15 +489,20 @@ function namedQueryFields(
   cube: Cube,
   datasetScope: DatasetScope | undefined,
   referenceOf: HopReference | undefined,
-): NamedField[] {
+): NamedRead[] {
   const baseObject = typeof cube.sql === 'string' ? cube.sql.trim() : '';
   if (!baseObject || !BARE_IDENTIFIER.test(baseObject)) return [];
-  const out: NamedField[] = [];
+  const out: NamedRead[] = [];
   const name = (member: string, kind: 'dimension' | 'measure' | 'any', role: FieldReadRole) => {
     if (typeof member !== 'string' || member === '') return;
     const entry = declaredMemberEntry(cube, member, kind);
-    const sql = entry ? entry.sql : kind === 'measure' ? undefined : member;
-    if (typeof sql === 'string') out.push(...fieldsOfColumnSql(cube, baseObject, sql, role, referenceOf));
+    // An undeclared measure reads nothing: both strategies refuse a measure the
+    // cube does not carry.
+    if (!entry && kind === 'measure') return;
+    const sql = entry ? entry.sql : member;
+    const fields = typeof sql === 'string' ? fieldsOfColumnSql(cube, baseObject, sql, role, referenceOf) : null;
+    if (fields) out.push(...fields);
+    else out.push({ object: baseObject, member, expression: true });
   };
   const filterMembers = (where: unknown): string[] => {
     if (!where || typeof where !== 'object') return [];
@@ -1779,6 +1797,10 @@ export class AnalyticsService implements IAnalyticsService {
    * members straight into a statement no middleware sees, so the engine's field
    * guard could never reach it. See `field-read-admission.ts`.
    *
+   * [#20965] A member that resolves to neither a field nor `'*'` is refused
+   * here too, `PERMISSION_DENIED` / 403, ahead of the field verdicts on its
+   * object ({@link namedQueryFields}).
+   *
    * A no-op when no provider is wired (no security service) or when the query
    * names no field.
    */