From f693d703734b3980440e8540af336e99b8324198 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 03:04:34 +0000 Subject: [PATCH 1/3] fix(service-analytics)!: refuse an analytics order key that names no member the query selects An `order` key must be a column the answer carries: a `dimensions` entry, a `measures` entry, or a `timeDimensions` entry with a `granularity`, spelled exactly as selected. Anything else is refused INVALID_FIELD / 400 at the analytics door (query and the generateSql dry run), after ensureCube and before strategy selection, so the native-SQL and the ObjectQL face answer alike. The ObjectQL strategy's projection rule moves into the door module so both read one definition. Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ Co-authored-by: Claude --- .../src/__tests__/order-key-selected.test.ts | 313 ++++++++++++++++++ .../src/analytics-service.ts | 8 + .../service-analytics/src/order-key-door.ts | 113 +++++++ .../src/strategies/objectql-strategy.ts | 10 +- 4 files changed, 438 insertions(+), 6 deletions(-) create mode 100644 packages/services/service-analytics/src/__tests__/order-key-selected.test.ts create mode 100644 packages/services/service-analytics/src/order-key-door.ts diff --git a/packages/services/service-analytics/src/__tests__/order-key-selected.test.ts b/packages/services/service-analytics/src/__tests__/order-key-selected.test.ts new file mode 100644 index 00000000000..278281f526c --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/order-key-selected.test.ts @@ -0,0 +1,313 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#21267] An analytics `order` key must name a member the query selects — a + * `dimensions` entry, a `measures` entry, or a `timeDimensions` entry with a + * `granularity` — or the analytics door refuses the query `INVALID_FIELD` / + * 400, on the native-SQL and the ObjectQL face alike, before either strategy + * runs (`order-key-door.ts`). + * + * ## The shape this closes + * + * Measured through `POST /api/v1/analytics/query` on the real dispatcher route + * at `3196ef1a1`, SQLite and PostgreSQL 16.14. The cube over `deal` declares no + * join; `owner` is a lookup whose target also declares `note` and `amount`: + * + * | query | native SQLite | native PostgreSQL | ObjectQL face | + * |:--|:--|:--|:--| + * | `dimensions: ['owner.email']`, `order: { note }` | 500 | 500 (42702) | 200 | + * | `dimensions: ['note']`, `order: { amount }` | 200, arbitrary order | 500 (42803) | 200 | + * | `dimensions: ['note']`, `order: { 'owner.email' }` | 500 | 500 (42703) | 200 | + * + * Every refusal pin below also asserts nothing ran: no raw statement and no + * engine aggregate, on either face. + * + * ## The dialect axis of THIS file + * + * The SQLite cell always runs. The PostgreSQL cell runs where + * `OS_TEST_POSTGRES_URL` is set and is a named skip otherwise; no CI step + * provisions that variable for this package, so the live cell is red-capable + * and un-run in CI, and the PR that landed this file carries its local + * PostgreSQL 16 run. The live cell owns its tables, dropped before and after. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import type { Cube } from '@objectstack/spec/data'; +import type { AnalyticsService } from '../analytics-service.js'; +import { AnalyticsServicePlugin } from '../plugin.js'; + +const PERSON = 'os21267_person'; +const DEAL = 'os21267_deal'; + +/** The lookup's target declares `note` and `amount` too, so an unqualified ORDER BY over a join is ambiguous. */ +const PERSON_OBJECT = { + name: PERSON, + label: 'Order key person', + fields: { + email: { name: 'email', type: 'text' as const }, + note: { name: 'note', type: 'text' as const }, + amount: { name: 'amount', type: 'number' as const }, + }, +}; + +const DEAL_OBJECT = { + name: DEAL, + label: 'Order key deal', + fields: { + note: { name: 'note', type: 'text' as const }, + amount: { name: 'amount', type: 'number' as const }, + closed_on: { name: 'closed_on', type: 'date' as const }, + owner: { name: 'owner', type: 'lookup' as const, reference: PERSON }, + }, +}; + +const PEOPLE = [ + { id: 'p1', email: 'a@x', note: 'pn1', amount: 100 }, + { id: 'p2', email: 'b@x', note: 'pn2', amount: 200 }, +] as const; +// Groups by `note`: x (2 rows, amount 15), y (1, 7), z (1, 1). Neither the +// note order nor the amount order matches insertion order reversed. +const DEALS = [ + { id: 'd1', note: 'x', amount: 10, closed_on: '2026-03-01', owner: 'p1' }, + { id: 'd2', note: 'x', amount: 5, closed_on: '2026-03-02', owner: 'p1' }, + { id: 'd3', note: 'y', amount: 7, closed_on: '2026-04-03', owner: 'p2' }, + { id: 'd4', note: 'z', amount: 1, closed_on: '2026-05-01', owner: 'p2' }, +] as const; + +/** The card's cube: it declares NO join, so `owner` is joined through the lookup's declared `reference`. */ +const CUBE = 'os21267_cube'; +const CUBES = [ + { + name: CUBE, + title: 'Order key cube', + sql: DEAL, + public: true, + measures: { + count: { type: 'count', sql: '*', label: 'Rows' }, + amount_sum: { type: 'sum', sql: 'amount', label: 'Amount' }, + }, + dimensions: { + note: { type: 'string', sql: 'note', label: 'Note' }, + closed_on: { type: 'time', sql: 'closed_on', label: 'Closed on' }, + }, + }, +] as unknown as Cube[]; + +/** The card's three rows: each orders by a key the query does not select. */ +const CARD_ROWS = [ + { + label: 'an unselected base column beside a relationship path (ambiguous on the native face)', + query: { cube: CUBE, measures: ['count'], dimensions: ['owner.email'], order: { note: 'asc' } }, + key: 'note', + selected: ['owner.email', 'count'], + }, + { + label: 'an unselected base column with no join (an arbitrary row on SQLite, 42803 on PostgreSQL)', + query: { cube: CUBE, measures: ['count'], dimensions: ['note'], order: { amount: 'asc' } }, + key: 'amount', + selected: ['note', 'count'], + }, + { + label: 'an unselected relationship path', + query: { cube: CUBE, measures: ['count'], dimensions: ['note'], order: { 'owner.email': 'asc' } }, + key: 'owner.email', + selected: ['note', 'count'], + }, +] as const; + +interface Cell { + id: 'sqlite' | 'pg'; + label: string; + env: string | null; + config: () => Record | null; +} + +const CELLS: readonly Cell[] = [ + { id: 'sqlite', label: 'sqlite', env: null, config: () => ({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }) }, + { + id: 'pg', + label: 'live postgres', + env: 'OS_TEST_POSTGRES_URL', + config: () => (process.env.OS_TEST_POSTGRES_URL ? { client: 'pg', connection: process.env.OS_TEST_POSTGRES_URL } : null), + }, +]; + +const FACES = ['native', 'objectql'] as const; +type Face = (typeof FACES)[number]; + +const quiet = { debug() {}, info() {}, warn() {}, error() {}, child() { return quiet; } }; + +type Row = Record; +type Refusal = Error & { code?: string; status?: number; field?: string; param?: string }; + +/** Rows as tuples of the named columns, in the order they arrived; a measure reads as a number on every dialect. */ +const tuples = (rows: unknown, columns: readonly string[]) => + (rows as Row[]).map((row) => columns.map((c) => (typeof row[c] === 'number' || /^-?\d+(\.\d+)?$/.test(String(row[c])) ? Number(row[c]) : row[c]))); +const sorted = (list: unknown[][]) => [...list].sort((a, b) => JSON.stringify(a).localeCompare(JSON.stringify(b))); + +for (const cell of CELLS) { + const config = cell.config(); + describe.skipIf(!config)( + `[#21267] an analytics order key must name a member the query selects (${cell.label})${config ? '' : ` (skipped: set ${cell.env} to run this cell)`}`, + () => { + let driver: any; + let engine: ObjectQL; + /** Raw-SQL statements and engine aggregates, on any object. */ + const reads = { rawSql: 0, aggregate: 0 }; + /** `native`: the plugin's own capabilities. `objectql`: narrowed to the engine-aggregate path. */ + const services: Partial> = {}; + + const dropTables = async () => { + if (cell.id !== 'pg') return; + for (const table of [DEAL, PERSON]) await driver?.execute(`drop table if exists ${table}`).catch(() => {}); + }; + + /** One call on one face: its result or its error, and the reads it caused. */ + const call = async (face: Face, door: 'query' | 'generateSql', query: Record) => { + const before = { ...reads }; + const outcome = await (services[face]![door] as (q: any) => Promise).call(services[face], query as any).then( + (res) => ({ res, err: undefined as Refusal | undefined }), + (err) => ({ res: undefined, err: err as Refusal }), + ); + return { ...outcome, rawSql: reads.rawSql - before.rawSql, aggregate: reads.aggregate - before.aggregate }; + }; + + /** Both doors on both faces refuse `query` with the order-key envelope, and nothing ran. */ + const refusedEverywhere = async (query: Record, key: string, selected: readonly string[]) => { + for (const face of FACES) { + for (const door of ['query', 'generateSql'] as const) { + const { res, err, rawSql, aggregate } = await call(face, door, query); + const where = `${face} ${door}`; + expect(res, `${where}: answered instead of refusing`).toBeUndefined(); + expect(err?.code, `${where}: ${err?.message}`).toBe('INVALID_FIELD'); + expect(err?.status, where).toBe(400); + expect(err?.param, where).toBe('order'); + expect(err?.field, where).toBe(key); + // The refusal names the offending key and every member the query does select. + for (const name of [key, ...selected]) expect(err?.message, where).toContain(`${name}`); + expect(rawSql, `${where}: no raw statement ran`).toBe(0); + expect(aggregate, `${where}: no engine aggregate ran`).toBe(0); + } + } + }; + + beforeAll(async () => { + driver = new SqlDriver(config as any); + await dropTables(); + engine = new ObjectQL({ logger: quiet } as any); + engine.registerDriver(driver, true); + await engine.init(); + for (const object of [PERSON_OBJECT, DEAL_OBJECT]) engine.registry.registerObject(object as any); + await engine.syncSchemas(); + for (const row of PEOPLE) await engine.insert(PERSON, { ...row } as any); + for (const row of DEALS) await engine.insert(DEAL, { ...row } as any); + + const realExecute = (engine as any).execute.bind(engine); + (engine as any).execute = (sql: unknown, opts?: unknown) => { + reads.rawSql += 1; + return realExecute(sql, opts); + }; + const realAggregate = engine.aggregate.bind(engine); + (engine as any).aggregate = (...args: unknown[]) => { + reads.aggregate += 1; + return (realAggregate as any)(...args); + }; + + for (const [face, caps] of [ + ['native', undefined], + ['objectql', () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false })], + ] as const) { + const registered: Record = {}; + await new AnalyticsServicePlugin({ cubes: CUBES, ...(caps ? { queryCapabilities: caps } : {}) } as any).init({ + getService: (name: string) => (name === 'data' ? engine : registered[name]), + registerService: (name: string, svc: unknown) => { registered[name] = svc; }, + replaceService: (name: string, svc: unknown) => { registered[name] = svc; }, + hook: () => {}, + logger: quiet, + } as never); + services[face] = registered.analytics as AnalyticsService; + } + }); + + afterAll(async () => { + await dropTables(); + try { await engine?.destroy(); } catch { /* noop */ } + }); + + for (const row of CARD_ROWS) { + it(`the card's row is refused 400 INVALID_FIELD on both faces and both doors: ${row.label}`, async () => { + await refusedEverywhere(row.query, row.key, row.selected); + }); + } + + it('a time-dimension entry that only windows the rows is not a column, so ordering by it is refused', async () => { + await refusedEverywhere( + { + cube: CUBE, + measures: ['count'], + dimensions: ['note'], + timeDimensions: [{ dimension: 'closed_on', dateRange: ['2026-03-01', '2026-05-31'] }], + order: { closed_on: 'asc' }, + }, + 'closed_on', + ['note', 'count'], + ); + }); + + it('a key matches by its exact spelling: a qualified spelling of a measure selected bare names no column', async () => { + await refusedEverywhere( + { cube: CUBE, measures: ['amount_sum'], dimensions: ['note'], order: { [`${CUBE}.amount_sum`]: 'desc' } }, + `${CUBE}.amount_sum`, + ['note', 'amount_sum'], + ); + }); + + it('CONTROL ordering by a selected dimension is served unchanged on both faces', async () => { + const query = { cube: CUBE, measures: ['count'], dimensions: ['note'], order: { note: 'desc' } }; + for (const face of FACES) { + const { res, err } = await call(face, 'query', query); + expect(err, `${face}: ${err?.message}`).toBeUndefined(); + const got = tuples(res.rows, ['note', 'count']); + // The native face runs the ORDER BY; the ObjectQL face serves the same groups. + if (face === 'native') expect(got).toEqual([['z', 1], ['y', 1], ['x', 2]]); + else expect(sorted(got)).toEqual([['x', 2], ['y', 1], ['z', 1]]); + const { res: dry, err: dryErr } = await call(face, 'generateSql', query); + expect(dryErr, `${face} dry run: ${dryErr?.message}`).toBeUndefined(); + expect(dry.sql, face).toContain('ORDER BY "note" DESC'); + } + }); + + it('CONTROL ordering by a selected measure is served unchanged on both faces', async () => { + const query = { cube: CUBE, measures: ['amount_sum'], dimensions: ['note'], order: { amount_sum: 'desc' } }; + for (const face of FACES) { + const { res, err } = await call(face, 'query', query); + expect(err, `${face}: ${err?.message}`).toBeUndefined(); + const got = tuples(res.rows, ['note', 'amount_sum']); + if (face === 'native') expect(got).toEqual([['x', 15], ['y', 7], ['z', 1]]); + else expect(sorted(got)).toEqual([['x', 15], ['y', 7], ['z', 1]]); + const { res: dry, err: dryErr } = await call(face, 'generateSql', query); + expect(dryErr, `${face} dry run: ${dryErr?.message}`).toBeUndefined(); + expect(dry.sql, face).toContain('ORDER BY "amount_sum" DESC'); + } + }); + + it('CONTROL a bucketed time dimension is a column the answer carries, so ordering by it is served', async () => { + const query = { + cube: CUBE, + measures: ['count'], + timeDimensions: [{ dimension: 'closed_on', granularity: 'month' }], + order: { closed_on: 'asc' }, + }; + for (const face of FACES) { + const { res, err } = await call(face, 'query', query); + expect(err, `${face}: ${err?.message}`).toBeUndefined(); + // Three month buckets (March, April, May), whichever face served them. + expect((res.rows as Row[]).length, face).toBe(3); + expect(sorted(tuples(res.rows, ['count'])), face).toEqual([[1], [1], [2]]); + } + }); + }, + ); +} diff --git a/packages/services/service-analytics/src/analytics-service.ts b/packages/services/service-analytics/src/analytics-service.ts index 16d63d721b6..e579e2a659e 100644 --- a/packages/services/service-analytics/src/analytics-service.ts +++ b/packages/services/service-analytics/src/analytics-service.ts @@ -108,6 +108,7 @@ import { invalidMemberError } from './dataset-refusal.js'; // [#20807] A grouped dimension on a structured-JSON field is refused in // `ensureCube`, ahead of both strategies, naming the member the caller wrote. import { assertNoStructuredJsonDimension } from './structured-json-dimension-door.js'; +import { assertOrderKeysSelected } from './order-key-door.js'; // [#16206] The `sqlDialect` hook's DECLARED accept set, and the predicate that // says whether a host answered outside it. Both live next to the membership set // the compilers read, so the contract has one definition and this file states @@ -2212,6 +2213,10 @@ export class AnalyticsService implements IAnalyticsService { // names no declared member — in every tier, before a strategy compiles it. // After `ensureCube` so a non-existent cube/object still answers 404 first. this.assertCallerMembersResolvable(query, authorCube, context); + // [#21267] An `order` key must name a member this query selects — one + // refusal ahead of strategy selection, so both faces answer alike + // (`order-key-door.ts`). + assertOrderKeysSelected(query); const ctx = await this.callCtx(query, context, tokenCtx, scope); let skip: Set | undefined; for (;;) { @@ -3018,6 +3023,9 @@ export class AnalyticsService implements IAnalyticsService { const authorCube = scope.getCube(query.cube!); this.ensureCube(query, scope); this.assertCallerMembersResolvable(query, authorCube, context); + // [#21267] Same order-key door as `query()`: the dry run must not hand back + // an `ORDER BY` naming a column the statement does not select. + assertOrderKeysSelected(query); const ctx = await this.callCtx(query, context, tokenCtx, scope); const strategy = this.resolveStrategy(query, ctx); this.logger.debug(`[Analytics] generateSql on cube "${query.cube}" → ${strategy.name}`); diff --git a/packages/services/service-analytics/src/order-key-door.ts b/packages/services/service-analytics/src/order-key-door.ts new file mode 100644 index 00000000000..bf116c3dc74 --- /dev/null +++ b/packages/services/service-analytics/src/order-key-door.ts @@ -0,0 +1,113 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#21267] An analytics `order` key must name a member the query SELECTS — a + * column the answer carries — or the query is refused `INVALID_FIELD` / 400 at + * the analytics door, before either strategy runs. + * + * ## Why a key outside the selection names nothing + * + * The answer of an aggregate query is its selected dimensions and measures, one + * row per group. A key outside them names nothing in that answer: SQL refuses + * it, and the one dialect that accepts it orders the groups by an arbitrary + * row's value. So no reading of an unselected key is both defined and portable. + * Both strategies write the key into `ORDER BY` as an output-column name + * (`ORDER BY ""`), which names a column only when the member is selected. + * + * Measured through `POST /api/v1/analytics/query` on the real dispatcher route + * at `origin/main` `3196ef1a1`, SQLite and PostgreSQL 16.14. A cube over a + * `deal` object declares no join; `owner` is a lookup whose target also + * declares `note`: + * + * | query | native SQLite | native PostgreSQL | ObjectQL face | + * |:--|:--|:--|:--| + * | `dimensions: ['owner.email']`, `order: { note }` | 500 | 500 (42702, `note` ambiguous) | 200 | + * | `dimensions: ['note']`, `order: { amount }` | 200, ordered by an arbitrary row | 500 (42803, must appear in GROUP BY) | 200 | + * | `dimensions: ['note']`, `order: { 'owner.email' }` | 500 | 500 (42703, no such column) | 200 | + * + * The ObjectQL face answered 200 because its execution never applies `order` + * at all; its echoed statement and `/analytics/sql` showed the same `ORDER BY` + * the native face could not run. + * + * ## What "selected" means + * + * {@link selectedMembers}: every `dimensions` entry, every `measures` entry, + * and every `timeDimensions` entry that carries a `granularity` — a bucket the + * answer carries as a column. A `timeDimensions` entry with no granularity only + * bounds the rows by a window and is not a column, so it is not orderable. + * + * A key matches by its EXACT spelling. The column the answer carries is keyed + * by the spelling the caller sent (a `.`-qualified measure keeps its + * qualifier) and both strategies emit the key as that column's name, so + * `order: { 'account.revenue_sum' }` beside `measures: ['revenue_sum']` names no + * column either. + * + * ## Where it runs + * + * `AnalyticsService.queryIn` — `query()` (`/analytics/query`) and every query a + * dataset selection runs through `DatasetExecutor` — and `generateSql()` + * (`/analytics/sql`), after `ensureCube` and the caller-member gate, so an + * unknown cube still answers 404 first, and before strategy selection, so the + * native-SQL and the ObjectQL face give one answer on every driver. The dataset + * door already refuses an unselected `selection.order` key itself + * (`resolveOrdering`, `DATASET_INVALID`) and pushes an `order` down into its + * query only when every key is a dimension or measure that query selects, so + * this door never refuses a dataset selection it accepted. + */ + +import type { AnalyticsQuery } from '@objectstack/spec/contracts'; + +/** + * The dimensions a query PROJECTS, in the order the answer carries them: every + * `dimensions` entry, then every `timeDimensions` entry with a `granularity` + * that is not already one of them. + * + * `timeDimensions` is not merely a filter carrier. An entry with a + * `granularity` is GROUPED BY, so its bucket is a COLUMN of the answer; an + * entry without one only contributes a `dateRange` predicate and is NOT + * projected. One definition: `ObjectQLStrategy` groups, maps rows and describes + * `fields` by it, and this door orders by it. + */ +export function projectedDimensions(query: AnalyticsQuery): string[] { + const out = [...(query.dimensions ?? [])]; + for (const td of query.timeDimensions ?? []) { + if (td.granularity && !out.includes(td.dimension)) out.push(td.dimension); + } + return out; +} + +/** Every column an analytics query's answer carries: its projected dimensions, then its measures. */ +export function selectedMembers(query: AnalyticsQuery): string[] { + return [...projectedDimensions(query), ...(query.measures ?? [])]; +} + +/** + * Refuse a query whose `order` names a key that is not one of + * {@link selectedMembers} — `INVALID_FIELD` / 400, the envelope the door's + * member gates answer with, naming every such key and the members the query + * does select. `field` is the first such key and `param` is `order`. + */ +export function assertOrderKeysSelected(query: AnalyticsQuery): void { + const order = (query as { order?: unknown }).order; + if (!order || typeof order !== 'object' || Array.isArray(order)) return; + const keys = Object.keys(order); + if (keys.length === 0) return; + const selected = selectedMembers(query); + const unselected = keys.filter((key) => !selected.includes(key)); + if (unselected.length === 0) return; + + const named = unselected.map((key) => `'${key}'`).join(', '); + const several = unselected.length > 1; + const err = new Error( + `Order key${several ? 's' : ''} ${named} on cube '${query.cube}' name${several ? '' : 's'} no member ` + + `this query selects, so the query was not run. An \`order\` key must be a column the answer carries — ` + + `a \`dimensions\` entry, a \`measures\` entry, or a \`timeDimensions\` entry with a \`granularity\` — ` + + `spelled exactly as it is selected. Selected here: ${selected.join(', ') || '(none)'}. ` + + `Select ${several ? 'each key' : 'the key'} (add it to \`dimensions\` or \`measures\`) or drop it from \`order\`.`, + ) as Error & { code?: string; status?: number; field?: string; param?: string }; + err.code = 'INVALID_FIELD'; + err.status = 400; + err.field = unselected[0]; + err.param = 'order'; + throw err; +} diff --git a/packages/services/service-analytics/src/strategies/objectql-strategy.ts b/packages/services/service-analytics/src/strategies/objectql-strategy.ts index 4fba6f7c724..33aa4e573ed 100644 --- a/packages/services/service-analytics/src/strategies/objectql-strategy.ts +++ b/packages/services/service-analytics/src/strategies/objectql-strategy.ts @@ -30,6 +30,7 @@ import { declaredValueShapeResolver, whereEmptyLeafSql } from '../empty-operator // [#20986] The one resolver of the object a relationship-path hop reads. import { columnObjectOf, relationshipReferenceOf, resolvePathHops, type HopReference } from '../hop-object.js'; import { invalidMemberError } from '../dataset-refusal.js'; +import { projectedDimensions } from '../order-key-door.js'; import { type LikeShape } from '../like-pattern.js'; import { textMatchPredicateSql, sqlDialectFor } from '../text-match-sql.js'; import { nextUtcCalendarDay, resolveAnalyticsDateRangeString, isUnboundedAbove } from '@objectstack/core'; @@ -2094,14 +2095,11 @@ export class ObjectQLStrategy implements AnalyticsStrategy { * the measures and a `fields` list that never mentioned the bucket — a trend * chart got N values and no x-axis (#4033) — even though the SQL had * selected `date_trunc(…) AS ""` all along. One definition, every - * consumer. + * consumer — [#21267] including the analytics door's order-key rule, which + * is why the body lives in `order-key-door.ts`. */ private projectedDimensions(query: AnalyticsQuery): string[] { - const out = [...(query.dimensions ?? [])]; - for (const td of query.timeDimensions ?? []) { - if (td.granularity && !out.includes(td.dimension)) out.push(td.dimension); - } - return out; + return projectedDimensions(query); } private buildFieldMeta(query: AnalyticsQuery, cube: Cube): Array<{ name: string; type: string }> { From 4362776a34ca5b65922e8c46dbd18ed810847ddc Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 03:08:58 +0000 Subject: [PATCH 2/3] fix(service-analytics): run the order-key door on callCtx, after the admission verdicts callCtx is the one seam query() and generateSql() share, so the door has one call site. Running it after the object, stored-metadata-body, field read and field query gates keeps the 403 a field the caller may not read gets in every position, the order key included. Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ Co-authored-by: Claude --- .../service-analytics/src/analytics-service.ts | 15 ++++++++------- .../service-analytics/src/order-key-door.ts | 18 ++++++++++++------ 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/packages/services/service-analytics/src/analytics-service.ts b/packages/services/service-analytics/src/analytics-service.ts index e579e2a659e..627d13ed97b 100644 --- a/packages/services/service-analytics/src/analytics-service.ts +++ b/packages/services/service-analytics/src/analytics-service.ts @@ -1705,6 +1705,14 @@ export class AnalyticsService implements IAnalyticsService { query.cube ? reads.getDatasetScope(query.cube) : undefined, context, ); + // [#21267] …and the order-key door: an `order` key must name a member this + // query selects, or the query is refused `INVALID_FIELD` / 400 + // (`order-key-door.ts`). On this seam because it is the one `query()` and + // `generateSql()` share, so both doors and both faces answer alike. After + // the admission verdicts, so a key naming a field the caller may not read + // keeps the 403 that field gets in every other position; before the read + // scopes are resolved and before any strategy is selected. + assertOrderKeysSelected(query); // #3602 — `context` rides along unconditionally. It is the ENGINE-side belt // (forwarded to `engine.aggregate`, where the middleware chain applies its // own RLS), so it must not be gated on the analytics-side belt being wired: @@ -2213,10 +2221,6 @@ export class AnalyticsService implements IAnalyticsService { // names no declared member — in every tier, before a strategy compiles it. // After `ensureCube` so a non-existent cube/object still answers 404 first. this.assertCallerMembersResolvable(query, authorCube, context); - // [#21267] An `order` key must name a member this query selects — one - // refusal ahead of strategy selection, so both faces answer alike - // (`order-key-door.ts`). - assertOrderKeysSelected(query); const ctx = await this.callCtx(query, context, tokenCtx, scope); let skip: Set | undefined; for (;;) { @@ -3023,9 +3027,6 @@ export class AnalyticsService implements IAnalyticsService { const authorCube = scope.getCube(query.cube!); this.ensureCube(query, scope); this.assertCallerMembersResolvable(query, authorCube, context); - // [#21267] Same order-key door as `query()`: the dry run must not hand back - // an `ORDER BY` naming a column the statement does not select. - assertOrderKeysSelected(query); const ctx = await this.callCtx(query, context, tokenCtx, scope); const strategy = this.resolveStrategy(query, ctx); this.logger.debug(`[Analytics] generateSql on cube "${query.cube}" → ${strategy.name}`); diff --git a/packages/services/service-analytics/src/order-key-door.ts b/packages/services/service-analytics/src/order-key-door.ts index bf116c3dc74..21103a6b488 100644 --- a/packages/services/service-analytics/src/order-key-door.ts +++ b/packages/services/service-analytics/src/order-key-door.ts @@ -44,12 +44,18 @@ * * ## Where it runs * - * `AnalyticsService.queryIn` — `query()` (`/analytics/query`) and every query a - * dataset selection runs through `DatasetExecutor` — and `generateSql()` - * (`/analytics/sql`), after `ensureCube` and the caller-member gate, so an - * unknown cube still answers 404 first, and before strategy selection, so the - * native-SQL and the ObjectQL face give one answer on every driver. The dataset - * door already refuses an unselected `selection.order` key itself + * `AnalyticsService.callCtx`, the one seam `query()` (`/analytics/query`, and + * every query a dataset selection runs through `DatasetExecutor`) and + * `generateSql()` (`/analytics/sql`) share — so both doors and the native-SQL + * and the ObjectQL face give one answer on every driver: + * + * - after `ensureCube`, so an unknown cube still answers 404 first; + * - after the admission verdicts (object, stored metadata body, field read + * and field query), so a key naming a field the caller may not read keeps + * the 403 that field gets in every other position; + * - before the read scopes are resolved and before any strategy is selected. + * + * The dataset door already refuses an unselected `selection.order` key itself * (`resolveOrdering`, `DATASET_INVALID`) and pushes an `order` down into its * query only when every key is a dimension or measure that query selects, so * this door never refuses a dataset selection it accepted. From c515c61fdc1c6fe4daa3c3cf9b8abb67b3d33f0f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 03:23:10 +0000 Subject: [PATCH 3/3] docs(service-analytics): document the analytics order-key rule, and add its changeset Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ Co-authored-by: Claude --- .../21267-analytics-order-key-selected.md | 27 +++++++++++++++++++ content/docs/api/data-api.mdx | 9 +++++++ packages/services/service-analytics/README.md | 2 +- 3 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 .changeset/21267-analytics-order-key-selected.md diff --git a/.changeset/21267-analytics-order-key-selected.md b/.changeset/21267-analytics-order-key-selected.md new file mode 100644 index 00000000000..4d9953e23a5 --- /dev/null +++ b/.changeset/21267-analytics-order-key-selected.md @@ -0,0 +1,27 @@ +--- +"@objectstack/service-analytics": minor +--- + +fix(service-analytics)!: an analytics `order` key that names no member the query selects is refused with `INVALID_FIELD` / 400 at the analytics door, on both strategies, before either runs + +Clause-②: no (narrowing) + + + +**BREAKING**: this narrows what `POST /api/v1/analytics/query` and its dry run `POST /api/v1/analytics/sql` accept, on both strategies and every driver. It ships as `minor` under the launch-window convention for accept-set narrowings. No export or published type changes. + +**The rule.** Each `order` key must be a column the answer carries: one of the query's own `dimensions` entries, one of its `measures` entries, or a `timeDimensions` entry that carries a `granularity`, spelled exactly as it is selected (a `.`-qualified measure keeps its qualifier in the answer, so the bare spelling names no column beside it, and the other way round). A `timeDimensions` entry with only a `dateRange` bounds the rows and is not a column. Any other key is refused with `400 INVALID_FIELD`, naming every such key and the members the query does select, and nothing is executed. The thrown error carries `param: 'order'` and `field` (the first such key). + +**Before**, measured through `POST /api/v1/analytics/query` on SQLite and PostgreSQL 16.14, for a cube that declares no join over an object whose lookup target also declares `note`: + +- `dimensions: ['owner.email']` with `order: { note: 'asc' }`: native-SQL strategy `500` on both drivers (PostgreSQL 42702, `note` is ambiguous); ObjectQL strategy `200`. +- `dimensions: ['note']` with `order: { amount: 'asc' }`: native-SQL strategy `200` on SQLite, ordered by an arbitrary row's `amount`, and `500` on PostgreSQL (42803, must appear in GROUP BY); ObjectQL strategy `200`. +- `dimensions: ['note']` with `order: { 'owner.email': 'asc' }`: native-SQL strategy `500` on both drivers (PostgreSQL 42703, no such column); ObjectQL strategy `200`. + +**Now** each of those answers `400 INVALID_FIELD` on both strategies and both drivers, and `POST /api/v1/analytics/sql` refuses them the same way instead of returning a statement whose `ORDER BY` cannot run. + +**What to write instead.** Add the key to the query's `dimensions` (or `measures`), so the answer carries it, or drop it from `order`. + +**Who is affected.** A caller that posted an `order` key it did not select. On the native-SQL strategy those queries were already a 500 everywhere but the one SQLite shape, whose order was arbitrary. No example app, shipped dashboard, report, dataset or cube authors such a key, and the console's analytics adapter sends no `order` to this route. + +**Unchanged.** Ordering by a selected dimension, a selected measure or a bucketed time dimension; the dataset door (`POST /api/v1/analytics/dataset/query`), which already refused an unselected `selection.order` key with `400 DATASET_INVALID` and pushes an `order` down only when the selection selects every key; and a key naming a field the caller may not read, which keeps the `403 PERMISSION_DENIED` the field-level read gate answers for every position. diff --git a/content/docs/api/data-api.mdx b/content/docs/api/data-api.mdx index 0e2cf7d0786..af2b77c4495 100644 --- a/content/docs/api/data-api.mdx +++ b/content/docs/api/data-api.mdx @@ -430,6 +430,15 @@ in its own `sql`. Filtering uses the canonical Query DSL `where` object (the same MongoDB-style `FilterCondition` accepted by `find()`), not a `filters` array. + +**An `order` key names a column the answer carries.** Each key must be one of the query's own +`dimensions` or `measures` entries, or a `timeDimensions` entry that carries a `granularity`, +spelled exactly as it is selected. Any other key — a field the query does not select, a +relationship path it does not group by, a time dimension that only sets a `dateRange` window — is +refused with `400 INVALID_FIELD` naming the key and the members the query does select, and nothing +is executed. `POST /analytics/sql` refuses the same keys. To order by a member, select it. + + **Response**: the runtime dispatcher wraps the `AnalyticsResult` as `{ success: true, data: { rows, fields, sql?, totals? } }`: ```json { diff --git a/packages/services/service-analytics/README.md b/packages/services/service-analytics/README.md index ddbbb4dc762..0f5f8331483 100644 --- a/packages/services/service-analytics/README.md +++ b/packages/services/service-analytics/README.md @@ -97,7 +97,7 @@ is rejected rather than dropped. | `dimensions` | `string[]?` | | | `where` | `FilterCondition?` | Canonical Query DSL filter — the same shape `find()` takes. | | `timeDimensions` | `{ dimension, granularity?, dateRange? }[]?` | Also strict per item. | -| `order` | `Record?` | | +| `order` | `Record?` | Each key must be a selected `dimensions` / `measures` entry, or a `timeDimensions` entry with a `granularity`, spelled as selected. Any other key is refused with `400 INVALID_FIELD`. | | `limit` | `number?` | | | `offset` | `number?` | | | `timezone` | `string?` | IANA name. No default — an absent timezone means the engine resolves it. |