From 26db34664bb777f5326d38f29edeae09b38614d8 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Mon, 27 Jul 2026 17:06:41 +0800 Subject: [PATCH] fix(analytics): enforce read scope on the ObjectQL aggregate path (#3597) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ObjectQLStrategy never consumed `getReadScope`, so every analytics query served by that path ran with no RLS or tenant predicate — an authenticated caller received aggregates computed over every tenant's rows. Both belts were off at once. The strategy dropped the pre-resolved read scope, and the engine could not compensate: the `executeAggregate` bridge passes no ExecutionContext, so plugin-security's principal-less fall-open (security-plugin.ts:775) skipped its own RLS injection. Only NativeSQLStrategy was ever wired for ADR-0021 D-C. Exposure was not limited to exotic drivers. NativeSQLStrategy declines — routing to this path — on any date-bucketed query (the most common dashboard shape, on Postgres and SQLite too), on RAW_SQL_UNSUPPORTED, and on federated objects. The scope is composed with `$and`, never by key merge, so a caller filter naming the same field cannot displace the security predicate. A query referencing a joined object that carries its own scope is now rejected rather than run partially-scoped: `engine.aggregate`'s `where` addresses the base object, so a per-join predicate cannot be expressed there. Failing closed matches resolveReadScopes and compileScopedFilterToSql. The gap survived because every case in dataset-rls-integration.test.ts pins `objectqlAggregate: false` — the RLS suite only ever exercised NativeSQLStrategy. The new tests cover the ObjectQL path directly; 7 of the 9 fail without this fix (the other 2 assert unchanged behaviour). Co-Authored-By: Claude --- .changeset/analytics-objectql-read-scope.md | 34 +++ .../src/__tests__/objectql-read-scope.test.ts | 259 ++++++++++++++++++ .../src/strategies/objectql-strategy.ts | 92 ++++++- 3 files changed, 384 insertions(+), 1 deletion(-) create mode 100644 .changeset/analytics-objectql-read-scope.md create mode 100644 packages/services/service-analytics/src/__tests__/objectql-read-scope.test.ts diff --git a/.changeset/analytics-objectql-read-scope.md b/.changeset/analytics-objectql-read-scope.md new file mode 100644 index 0000000000..3e3111ecb1 --- /dev/null +++ b/.changeset/analytics-objectql-read-scope.md @@ -0,0 +1,34 @@ +--- +"@objectstack/service-analytics": patch +--- + +fix(analytics): ObjectQLStrategy now enforces the read scope (RLS + tenant) (#3597) + +`ObjectQLStrategy` never consumed `getReadScope`, so any analytics query served by +that path ran with **no RLS or tenant predicate** — an authenticated caller +received aggregates computed over every tenant's rows. + +Both belts were off at once. The strategy dropped the pre-resolved read scope, and +the engine could not compensate: the `executeAggregate` bridge passes no +`ExecutionContext`, so plugin-security's principal-less fall-open skipped its own +RLS injection. Only `NativeSQLStrategy` was ever wired for ADR-0021 D-C. + +The exposure was **not** limited to exotic drivers. `NativeSQLStrategy` declines — +handing the query to this path — on any date-bucketed query +(`timeDimensions[].granularity`, the most common dashboard shape, on Postgres and +SQLite too), on `RAW_SQL_UNSUPPORTED` (in-memory driver), and on federated objects. + +The scope is composed with `$and`, never by key merge, so a caller filter naming +the same field (e.g. `organization_id`) cannot displace the security predicate. + +**Behaviour change to be aware of:** a query that references a **joined** object +carrying its own read scope is now REJECTED on this path rather than run +partially-scoped. `engine.aggregate`'s `where` addresses the base object, so a +per-join predicate cannot be expressed there; failing closed matches the posture +already taken by `resolveReadScopes` and `compileScopedFilterToSql`. Such a query +previously returned results that omitted the joined object's tenant predicate. +Run it on a native-SQL driver (`NativeSQLStrategy` scopes each join), or drop the +cross-object dimension/measure. + +Deployments with no read-scope provider configured are unaffected — that path +stays unscoped by documented contract. diff --git a/packages/services/service-analytics/src/__tests__/objectql-read-scope.test.ts b/packages/services/service-analytics/src/__tests__/objectql-read-scope.test.ts new file mode 100644 index 0000000000..d1f7583e97 --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/objectql-read-scope.test.ts @@ -0,0 +1,259 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * ADR-0021 D-C on the ObjectQL path (#3597). + * + * `dataset-rls-integration.test.ts` proves tenant scoping end-to-end, but every + * case there pins `objectqlAggregate: false` — so it only ever exercises + * NativeSQLStrategy. That blind spot is exactly why ObjectQLStrategy shipped + * without consuming `getReadScope` at all: an authenticated caller received + * aggregates computed over every tenant's rows. + * + * These cases run the same pipeline with the ObjectQL aggregate path selected — + * which is what the runtime picks whenever NativeSQL declines (date-granularity + * bucketing, `RAW_SQL_UNSUPPORTED`, federated objects). + */ + +import { describe, it, expect } from 'vitest'; +import { DatasetSchema } from '@objectstack/spec/ui'; +import type { ExecutionContext } from '@objectstack/spec/kernel'; +import type { FilterCondition } from '@objectstack/spec/data'; +import { AnalyticsService } from '../analytics-service.js'; +import { compileDataset } from '../dataset-compiler.js'; + +const dataset = DatasetSchema.parse({ + name: 'sales', + label: 'Sales', + object: 'opportunity', + dimensions: [{ name: 'region', field: 'region', type: 'string' }], + measures: [{ name: 'revenue', aggregate: 'sum', field: 'amount' }], +}); + +/** Two tenants' rows in one physical table. */ +const TABLE = [ + { id: 1, organization_id: 'org_A', region: 'West', amount: 100 }, + { id: 2, organization_id: 'org_B', region: 'East', amount: 900 }, +]; + +/** The production-shaped provider: tenant predicate derived from the request. */ +const readScope = (_o: string, context?: ExecutionContext): FilterCondition | undefined => + context?.tenantId ? { organization_id: context.tenantId } : undefined; + +type AggOpts = { + groupBy?: string[]; + aggregations?: Array<{ field: string; method: string; alias: string }>; + filter?: Record; +}; + +/** Match a row against the subset of FilterCondition these tests emit. */ +function matches(row: Record, filter: Record): boolean { + return Object.entries(filter).every(([k, v]) => { + if (k === '$and') return (v as Record[]).every((sub) => matches(row, sub)); + return row[k] === v; + }); +} + +/** + * An HONEST aggregate bridge: it applies whatever `filter` it is handed. So a + * missing tenant predicate produces a real cross-tenant leak rather than an + * artifact of a permissive stub. + */ +function makeAggregate(seen: AggOpts[]) { + return async (_objectName: string, opts: AggOpts) => { + seen.push(opts); + const rows = TABLE.filter((r) => matches(r as Record, opts.filter ?? {})); + const buckets = new Map>(); + for (const r of rows) { + const row = r as Record; + const key = (opts.groupBy ?? []).map((g) => String(row[g as string])).join('|'); + const b = buckets.get(key) + ?? Object.fromEntries((opts.groupBy ?? []).map((g) => [g, row[g as string]])); + for (const a of opts.aggregations ?? []) { + if (a.method === 'sum') b[a.alias] = Number(b[a.alias] ?? 0) + Number(row[a.field] ?? 0); + } + buckets.set(key, b); + } + return [...buckets.values()]; + }; +} + +const ctxA = { tenantId: 'org_A', userId: 'u_a' } as ExecutionContext; + +/** ObjectQL-only capabilities — NativeSQL unavailable. */ +const objectqlOnly = () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }); + +function makeService(seen: AggOpts[], overrides: Record = {}) { + const compiled = compileDataset(dataset); + return new AnalyticsService({ + cubes: [compiled.cube], + queryCapabilities: objectqlOnly, + executeAggregate: makeAggregate(seen), + getReadScope: readScope, + ...overrides, + }); +} + +describe('ObjectQLStrategy — read scope (ADR-0021 D-C, #3597)', () => { + it('scopes a plain aggregate query to the caller tenant', async () => { + const seen: AggOpts[] = []; + const result = await makeService(seen).query( + { cube: 'sales', dimensions: ['region'], measures: ['revenue'] }, + ctxA, + ); + + expect(seen[0].filter).toEqual({ organization_id: 'org_A' }); + expect(result.rows).toEqual([{ region: 'West', revenue: 100 }]); + }); + + it('scopes when NativeSQL declines at runtime (RAW_SQL_UNSUPPORTED fallback)', async () => { + const seen: AggOpts[] = []; + const service = makeService(seen, { + // Both advertised — exactly what the plugin auto-bridge produces. + queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: true, inMemory: false }), + executeRawSql: async () => { + const err = new Error('driver cannot run SQL') as Error & { code: string }; + err.code = 'RAW_SQL_UNSUPPORTED'; + throw err; + }, + }); + + const result = await service.query( + { cube: 'sales', dimensions: ['region'], measures: ['revenue'] }, + ctxA, + ); + + expect(seen[0].filter).toEqual({ organization_id: 'org_A' }); + expect(result.rows).toEqual([{ region: 'West', revenue: 100 }]); + }); + + it('scopes a date-bucketed query (NativeSQL declines on granularity, even on SQL drivers)', async () => { + const compiled = compileDataset( + DatasetSchema.parse({ + name: 'sales_t', + label: 'Sales', + object: 'opportunity', + dimensions: [{ name: 'created', field: 'created_at', type: 'date' }], + measures: [{ name: 'revenue', aggregate: 'sum', field: 'amount' }], + }), + ); + const seen: AggOpts[] = []; + const service = new AnalyticsService({ + cubes: [compiled.cube], + queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: true, inMemory: false }), + executeRawSql: async () => { throw new Error('NativeSQL must decline on granularity'); }, + executeAggregate: makeAggregate(seen), + getReadScope: readScope, + }); + + await service.query( + { + cube: 'sales_t', + measures: ['revenue'], + timeDimensions: [{ dimension: 'created', granularity: 'month' }], + }, + ctxA, + ); + + expect(seen[0].filter).toEqual({ organization_id: 'org_A' }); + }); + + it('ANDs the scope with the query filter instead of key-merging it', async () => { + const seen: AggOpts[] = []; + await makeService(seen).query( + { + cube: 'sales', + dimensions: ['region'], + measures: ['revenue'], + where: { region: 'West' }, + }, + ctxA, + ); + + expect(seen[0].filter).toEqual({ + $and: [{ region: 'West' }, { organization_id: 'org_A' }], + }); + }); + + it('a caller filter on the SAME field cannot displace the security predicate', async () => { + const seen: AggOpts[] = []; + // The caller tries to widen their scope by naming the tenant column itself. + const result = await makeService(seen).query( + { + cube: 'sales', + dimensions: ['region'], + measures: ['revenue'], + where: { organization_id: 'org_B' }, + }, + ctxA, + ); + + // Both predicates survive — and being contradictory, they yield nothing. + expect(seen[0].filter).toEqual({ + $and: [{ organization_id: 'org_B' }, { organization_id: 'org_A' }], + }); + expect(result.rows).toEqual([]); + }); + + it('two tenants stay isolated on one service instance (singleton-safe)', async () => { + const seen: AggOpts[] = []; + const service = makeService(seen); + const q = { cube: 'sales', dimensions: ['region'], measures: ['revenue'] }; + + const a = await service.query(q, ctxA); + const b = await service.query(q, { tenantId: 'org_B', userId: 'u_b' } as ExecutionContext); + + expect(a.rows).toEqual([{ region: 'West', revenue: 100 }]); + expect(b.rows).toEqual([{ region: 'East', revenue: 900 }]); + }); + + it('runs unscoped when no provider is configured (documented contract, unchanged)', async () => { + const seen: AggOpts[] = []; + const result = await makeService(seen, { getReadScope: undefined }).query( + { cube: 'sales', dimensions: ['region'], measures: ['revenue'] }, + ctxA, + ); + + expect(seen[0].filter).toBeUndefined(); + expect(result.rows).toHaveLength(2); + }); +}); + +describe('ObjectQLStrategy — joined-object scope is fail-closed (#3597)', () => { + const joined = DatasetSchema.parse({ + name: 'sales_by_account', + label: 'Sales by account', + object: 'opportunity', + include: ['account'], + dimensions: [{ name: 'region', field: 'account.region', type: 'string' }], + measures: [{ name: 'revenue', aggregate: 'sum', field: 'amount' }], + }); + + function makeJoinedService(scopeFor: (o: string) => FilterCondition | undefined) { + const compiled = compileDataset(joined); + return new AnalyticsService({ + cubes: [compiled.cube], + queryCapabilities: objectqlOnly, + executeAggregate: async () => [], + getReadScope: (o: string) => scopeFor(o), + getAllowedRelationships: () => compiled.allowedRelationships, + }); + } + + it('denies the query when a referenced joined object carries a scope', async () => { + const service = makeJoinedService(() => ({ organization_id: 'org_A' })); + + await expect( + service.query({ cube: 'sales_by_account', dimensions: ['region'], measures: ['revenue'] }, ctxA), + ).rejects.toThrow(/cannot enforce the read scope of joined object\(s\) "account"/); + }); + + it('allows the query when only the base object carries a scope', async () => { + const service = makeJoinedService((o) => + o === 'opportunity' ? { organization_id: 'org_A' } : undefined, + ); + + await expect( + service.query({ cube: 'sales_by_account', dimensions: ['region'], measures: ['revenue'] }, ctxA), + ).resolves.toBeDefined(); + }); +}); diff --git a/packages/services/service-analytics/src/strategies/objectql-strategy.ts b/packages/services/service-analytics/src/strategies/objectql-strategy.ts index cd19102be2..8436e53d84 100644 --- a/packages/services/service-analytics/src/strategies/objectql-strategy.ts +++ b/packages/services/service-analytics/src/strategies/objectql-strategy.ts @@ -79,13 +79,18 @@ export class ObjectQLStrategy implements AnalyticsStrategy { } } + // ADR-0021 D-C — the read scope (tenant + RLS) MUST be ANDed in before the + // query leaves the strategy. Rejects the query outright when a joined object + // carries a scope this path cannot express (#3597). + this.assertJoinedScopesEnforceable(cube, query, groupBy, filter, ctx); + const rows = await ctx.executeAggregate!(objectName, { // Structured groupBy items ({field, dateGranularity}) pass through the // executeAggregate bridge to engine.aggregate, which buckets them. The // contract types groupBy as string[]; the cast carries the richer shape. groupBy: groupBy.length > 0 ? (groupBy as unknown as string[]) : undefined, aggregations: aggregations.length > 0 ? aggregations : undefined, - filter: Object.keys(filter).length > 0 ? filter : undefined, + filter: this.withReadScope(objectName, filter, ctx), // ADR-0053 Phase 2 (D2): forward the reference tz so date buckets resolve // on that zone's calendar days. A non-UTC zone makes the engine bucket // in-memory (uniform across drivers); UTC/unset keeps the DB fast path. @@ -150,6 +155,91 @@ export class ObjectQLStrategy implements AnalyticsStrategy { // ── Helpers ────────────────────────────────────────────────────── + /** + * ADR-0021 D-C (#3597) — AND the object's read scope (tenant + RLS) into the + * filter handed to `engine.aggregate`. + * + * This path used to drop the scope entirely, and the engine could not make up + * for it: the aggregate bridge passes no `ExecutionContext`, so the security + * middleware's principal-less fall-open skipped its own RLS injection. Both + * belts were off at once — an authenticated caller received aggregates + * computed over EVERY tenant's rows. + * + * Composed with `$and`, never by key merge: the query's own filter and the + * scope can name the SAME field (e.g. a dashboard filtering `organization_id`), + * and a spread would let caller input silently overwrite the security + * predicate. `$and` makes that structurally impossible. + */ + private withReadScope( + objectName: string, + filter: Record, + ctx: StrategyContext, + ): Record | undefined { + const userFilter = Object.keys(filter).length > 0 ? filter : undefined; + if (typeof ctx.getReadScope !== 'function') return userFilter; + const scope = ctx.getReadScope(objectName); + if (scope === undefined || scope === null) return userFilter; + const scopeFilter = scope as Record; + if (!userFilter) return scopeFilter; + return { $and: [userFilter, scopeFilter] }; + } + + /** + * Fail-closed guard for cross-object queries (#3597). + * + * `engine.aggregate`'s `where` addresses the BASE object. A dotted member + * (`account.region`) is traversed by the engine through the lookup field, but + * there is no place in this call shape to hang a predicate on the JOINED + * object — so a joined object's read scope cannot be enforced here. + * + * `NativeSQLStrategy` can express it (alias-qualified WHERE per join) and does. + * When this path cannot, we reject rather than run a partially-scoped query: + * the same posture as `resolveReadScopes` (throws rather than emit unscoped + * SQL) and `compileScopedFilterToSql` (throws rather than drop a predicate). + * + * Only joins the query ACTUALLY references are considered — the scope map is + * a deliberate superset of what gets scanned, so keying off the map alone + * would reject queries that never touch the joined table. + */ + private assertJoinedScopesEnforceable( + cube: Cube, + query: AnalyticsQuery, + groupBy: Array, + filter: Record, + ctx: StrategyContext, + ): void { + if (typeof ctx.getReadScope !== 'function') return; + + // Field names as they will reach the engine. A dotted name is a relationship + // traversal; its first segment is the join alias. + const referenced = [ + ...groupBy.map((g) => (typeof g === 'string' ? g : g.field)), + ...Object.keys(filter), + ...(query.measures ?? []).map((m) => this.resolveMeasureAggregation(cube, m).field), + ]; + + const offending = new Set(); + for (const fieldName of referenced) { + if (!fieldName.includes('.')) continue; + const alias = fieldName.split('.')[0]; + const joinedObject = cube.joins?.[alias]?.name ?? alias; + if (joinedObject === this.extractObjectName(cube)) continue; + const scope = ctx.getReadScope(joinedObject); + if (scope !== undefined && scope !== null) offending.add(joinedObject); + } + if (offending.size === 0) return; + + throw new Error( + `[Analytics] ObjectQLStrategy cannot enforce the read scope of joined ` + + `object(s) ${[...offending].map((o) => `"${o}"`).join(', ')} — denying the ` + + `query (fail-closed, ADR-0021 D-C). This path reaches the joined table ` + + `through the base object's lookup, where a per-join security predicate ` + + `cannot be expressed. Run this query on a driver that supports native SQL ` + + `(NativeSQLStrategy scopes each join), or drop the cross-object ` + + `dimension/measure from the query.`, + ); + } + /** * Resolve a member ref to a `{ sql, type? }` definition. *