diff --git a/.changeset/analytics-label-read-scope.md b/.changeset/analytics-label-read-scope.md new file mode 100644 index 000000000..54f8f2d78 --- /dev/null +++ b/.changeset/analytics-label-read-scope.md @@ -0,0 +1,25 @@ +--- +"@objectstack/service-analytics": patch +--- + +fix(analytics): scope the dimension-label lookup to the referenced object's RLS (#3602) + +When a dataset groups by a `lookup`/`master_detail` dimension, analytics resolves +the grouped FK ids to the related record's display name via a per-record read +(`group by id`) dressed as an aggregate. That read carried **no read scope**, so +it revealed related-record display names whenever the referenced object's RLS is +stricter than the base object whose rows carry the id — a user could see a name +the referenced object's own RLS would hide. (Same-object and looser-referenced +cases were already safe because the ids come from the post-#3597 scoped +aggregate; this closes the stricter-referenced case.) + +The label lookup now applies the **referenced object's own** read scope — bound +to the request via the same `getReadScope` provider the aggregate path uses, +composed with `$and` (never key-merge) so it can't be displaced by the id +predicate. Fail-closed: if that object's scope can't be resolved, the dimension's +labels are skipped (the raw id renders) rather than fetched unscoped. No behaviour +change when no read-scope provider is configured. + +Internal `DimensionLabelDeps.fetchRecordLabels` gains an optional `scope` argument +and `resolveDimensionLabels` an optional `resolveScope` resolver; both are +service-analytics-internal (no spec/contract change). diff --git a/packages/qa/dogfood/test/analytics-label-scope.dogfood.test.ts b/packages/qa/dogfood/test/analytics-label-scope.dogfood.test.ts new file mode 100644 index 000000000..727c69c04 --- /dev/null +++ b/packages/qa/dogfood/test/analytics-label-scope.dogfood.test.ts @@ -0,0 +1,98 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// END-TO-END gate for #3602 residual 1 — "the dimension-label lookup is scoped +// to the referenced object's RLS" — proven through the REAL stack: HTTP → REST +// exec context → SecurityPlugin → AnalyticsServicePlugin auto-bridges → the +// plugin's real fetchRecordLabels closure that ANDs the referenced object's +// scope into the label read. +// +// The member owns a deal that points at a vendor OWNED BY THE ADMIN. Grouping +// deals by vendor, the member's bucket carries the vendor id; resolving that id +// to a NAME reads the vendor object, which the member cannot read. Before the +// fix the member got the vendor's name (leak); after it, the vendor scope +// excludes the admin's vendor so the raw id renders. The admin — who owns the +// vendor — still sees the name, proving the fix SCOPES rather than blanks. +// +// This is the only gate that exercises the plugin's real label closure; the +// service-analytics unit/integration tests fake fetchRecordLabels. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; +import { labelScopeStack, labelScopeSecurity } from './fixtures/label-scope-fixture.js'; + +const VENDOR_NAME = 'Admin-Owned Vendor'; + +const dealsByVendor = { + name: 'deals_by_vendor', + label: 'Deals by vendor', + object: 'lbl_deal', + dimensions: [{ name: 'vendor', label: 'Vendor', field: 'vendor', type: 'lookup' }], + measures: [{ name: 'cnt', label: 'Count', aggregate: 'count' }], +}; + +describe('dogfood: dimension-label lookup is RLS-scoped to the referenced object (#3602)', () => { + let stack: VerifyStack; + let adminToken: string; + let memberToken: string; + let vendorId: string; + + beforeAll(async () => { + stack = await bootStack(labelScopeStack as never, { security: labelScopeSecurity() }); + adminToken = await stack.signIn(); + memberToken = await stack.signUp('lbl-member@verify.test'); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const ql = await stack.kernel.getServiceAsync('objectql'); + const sys = { context: { isSystem: true } }; + const uid = async (email: string): Promise => { + const u = await ql.findOne('sys_user', { where: { email }, ...sys }); + if (!u?.id) throw new Error(`no sys_user for ${email}`); + return u.id as string; + }; + const adminId = await uid('admin@objectos.ai'); + const memberId = await uid('lbl-member@verify.test'); + + // Vendor owned by the admin — the member must NOT be able to read it. + const vendor = await ql.insert('lbl_vendor', { name: VENDOR_NAME, created_by: adminId }, sys); + vendorId = vendor.id as string; + + // One deal per principal, BOTH pointing at the admin's vendor. Authored as + // system with an explicit owner so the owner policy binds deterministically + // (and the member's deal can reference a vendor it cannot itself read). + await ql.insert('lbl_deal', { name: 'member deal', amount: 10, vendor: vendorId, created_by: memberId }, sys); + await ql.insert('lbl_deal', { name: 'admin deal', amount: 20, vendor: vendorId, created_by: adminId }, sys); + + // Preconditions: the member reads their deal but NOT the admin's vendor. + const deals = await stack.apiAs(memberToken, 'GET', '/data/lbl_deal'); + expect(((await deals.json()).records ?? []).map((r: any) => r.name)).toEqual(['member deal']); + const vendors = await stack.apiAs(memberToken, 'GET', '/data/lbl_vendor'); + expect((await vendors.json()).records ?? []).toHaveLength(0); + }, 90_000); + + afterAll(async () => { + await stack?.stop(); + }); + + async function vendorCellFor(token: string): Promise { + const res = await stack.apiAs(token, 'POST', '/analytics/dataset/query', { + dataset: dealsByVendor, + selection: { dimensions: ['vendor'], measures: ['cnt'] }, + }); + expect(res.status).toBe(200); + const body = (await res.json()) as { rows?: Array> }; + // Exactly one deal is visible to each principal → one vendor bucket. + expect(body.rows ?? []).toHaveLength(1); + return (body.rows ?? [])[0].vendor; + } + + it('the member sees the vendor id RAW — the name it cannot read does not leak', async () => { + const cell = await vendorCellFor(memberToken); + expect(cell).toBe(vendorId); + expect(cell).not.toBe(VENDOR_NAME); + }); + + it('the admin — who owns the vendor — still sees the resolved name (scopes, not blanks)', async () => { + const cell = await vendorCellFor(adminToken); + expect(cell).toBe(VENDOR_NAME); + }); +}); diff --git a/packages/qa/dogfood/test/fixtures/label-scope-fixture.ts b/packages/qa/dogfood/test/fixtures/label-scope-fixture.ts new file mode 100644 index 000000000..ed6510468 --- /dev/null +++ b/packages/qa/dogfood/test/fixtures/label-scope-fixture.ts @@ -0,0 +1,86 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// Two-object fixture for the #3602 dimension-label leak (residual 1). +// +// A dataset that groups by a LOOKUP dimension resolves each grouped FK id to the +// related record's display NAME. That label read used to carry no read scope, so +// it revealed the referenced record's name even when the reader could not read +// that record — the leak fires whenever the base object's rows point at a +// referenced record the reader's RLS hides. +// +// This fixture reproduces exactly that with zero dependence on org-scoping: +// • `lbl_vendor` — owner-scoped on `created_by`. +// • `lbl_deal` — owner-scoped on `created_by`, with a `vendor` lookup. +// A member owns a deal that points at a vendor OWNED BY THE ADMIN. The member can +// read their deal (owner match) but NOT the vendor (owned by someone else), so a +// deals-by-vendor dataset must show the vendor's raw id to the member and its +// display name only to the admin who owns it. + +import { defineStack } from '@objectstack/spec'; +import { ObjectSchema, Field } from '@objectstack/spec/data'; +import { PermissionSetSchema, RLS, type PermissionSet } from '@objectstack/spec/security'; +import { SecurityPlugin, securityDefaultPermissionSets } from '@objectstack/plugin-security'; + +export const Vendor = ObjectSchema.create({ + name: 'lbl_vendor', + // [ADR-0090 D1] grandfather stamp — the gate under test is permission-set RLS, + // not owner-sharing. + sharingModel: 'public_read_write', + label: 'Vendor', + pluralLabel: 'Vendors', + fields: { + name: Field.text({ label: 'Name', required: true }), + }, +}); + +export const Deal = ObjectSchema.create({ + name: 'lbl_deal', + sharingModel: 'public_read_write', + label: 'Deal', + pluralLabel: 'Deals', + fields: { + name: Field.text({ label: 'Name', required: true }), + amount: Field.number({ label: 'Amount' }), + vendor: Field.lookup('lbl_vendor', { label: 'Vendor' }), + }, +}); + +export const labelScopeStack = defineStack({ + manifest: { + id: 'com.dogfood.label_scope', + namespace: 'lbl', + version: '0.0.0', + type: 'app', + name: 'Label Scope Fixture', + description: 'Deal → vendor lookup exercising the #3602 label read-scope leak.', + }, + objects: [Vendor, Deal], +}); + +const MEMBER_SET = 'lbl_fixture_member'; + +/** + * The fallback set a fresh member resolves to: CRUD on both objects (so requests + * reach the RLS layer rather than being denied by RBAC) plus an OWNER policy on + * each. The member therefore reads only rows they own — the precondition the + * label leak needs (their deal points at the admin's vendor). + */ +export const memberSet: PermissionSet = PermissionSetSchema.parse({ + name: MEMBER_SET, + label: 'Label Fixture Member — owner-scoped', + objects: { + lbl_deal: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true }, + lbl_vendor: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true }, + }, + rowLevelSecurity: [ + RLS.ownerPolicy('lbl_deal', 'created_by'), + RLS.ownerPolicy('lbl_vendor', 'created_by'), + ], +}); + +export function labelScopeSecurity(): SecurityPlugin { + return new SecurityPlugin({ + defaultPermissionSets: [...securityDefaultPermissionSets, memberSet], + fallbackPermissionSet: memberSet.name, + }); +} diff --git a/packages/services/service-analytics/src/__tests__/dimension-labels.test.ts b/packages/services/service-analytics/src/__tests__/dimension-labels.test.ts index 38d0a6ea7..01149cc45 100644 --- a/packages/services/service-analytics/src/__tests__/dimension-labels.test.ts +++ b/packages/services/service-analytics/src/__tests__/dimension-labels.test.ts @@ -117,6 +117,71 @@ describe('resolveDimensionLabels', () => { expect(calls).toBe(1); expect(rows.map((r) => r.account)).toEqual(['Acme Corp', 'Acme Corp', 'Globex']); }); + + // ── #3602 — the label lookup must carry the REFERENCED object's read scope ── + describe('lookup label read scope (#3602)', () => { + it('passes the referenced object scope through to fetchRecordLabels', async () => { + let seenScope: unknown = 'UNSET'; + let seenTarget: string | undefined; + const d = deps({ + fetchRecordLabels: async (target, ids, scope) => { + seenTarget = target; + seenScope = scope; + return new Map(ids.map((id) => [id, `name-${String(id)}`])); + }, + }); + const rows = [{ account: 'acc1', n: 1 }]; + // resolveScope returns the referenced object's own RLS predicate. + await resolveDimensionLabels('task', [{ name: 'account', field: 'account' }], rows, d, (target) => { + expect(target).toBe('crm_account'); // the REFERENCED object, not the base + return { organization_id: 'org_A' }; + }); + expect(seenTarget).toBe('crm_account'); + expect(seenScope).toEqual({ organization_id: 'org_A' }); + }); + + it('fails CLOSED: when the scope cannot be resolved, the id is left raw (no unscoped fetch)', async () => { + let fetched = false; + const d = deps({ + fetchRecordLabels: async (_t, ids) => { + fetched = true; + return new Map(ids.map((id) => [id, 'LEAKED NAME'])); + }, + }); + const rows = [{ account: 'acc1', n: 1 }]; + await resolveDimensionLabels('task', [{ name: 'account', field: 'account' }], rows, d, () => { + throw new Error('security service unavailable'); + }); + // The label fetch must NOT have run, and the raw id must survive. + expect(fetched).toBe(false); + expect(rows).toEqual([{ account: 'acc1', n: 1 }]); + }); + + it('a select dimension is unaffected by scope resolution (no referenced object)', async () => { + let scopeCalls = 0; + const rows = [{ status: 'backlog', n: 1 }]; + await resolveDimensionLabels('task', [{ name: 'status', field: 'status' }], rows, deps(), () => { + scopeCalls++; + return undefined; + }); + expect(scopeCalls).toBe(0); // scope is only resolved for lookup/master_detail dims + expect(rows).toEqual([{ status: 'Backlog', n: 1 }]); + }); + + it('no resolver (no security configured) → unscoped fetch, unchanged behaviour', async () => { + let seenScope: unknown = 'UNSET'; + const d = deps({ + fetchRecordLabels: async (_t, ids, scope) => { + seenScope = scope; + return new Map(ids.map((id) => [id, 'Acme Corp'])); + }, + }); + const rows = [{ account: 'acc1', n: 1 }]; + await resolveDimensionLabels('task', [{ name: 'account', field: 'account' }], rows, d /* no resolveScope */); + expect(seenScope).toBeUndefined(); + expect(rows).toEqual([{ account: 'Acme Corp', n: 1 }]); + }); + }); }); describe('formatDateBucket', () => { diff --git a/packages/services/service-analytics/src/__tests__/query-dataset.test.ts b/packages/services/service-analytics/src/__tests__/query-dataset.test.ts index 6f7f81107..46ca3894f 100644 --- a/packages/services/service-analytics/src/__tests__/query-dataset.test.ts +++ b/packages/services/service-analytics/src/__tests__/query-dataset.test.ts @@ -393,4 +393,52 @@ describe('AnalyticsService.queryDataset', () => { expect(result.totals[1].dimensions).toEqual([]); expect(result.drillRawTotals[1]).toEqual([{}]); }); + + // ── #3602 — the lookup label read is scoped to the REFERENCED object's RLS ── + it('threads the referenced object read scope (not the base object) into the label fetch', async () => { + const byAccount = DatasetSchema.parse({ + name: 'sales_acct_scoped', label: 'Sales', object: 'opportunity', include: [], + dimensions: [{ name: 'account', field: 'account', type: 'lookup', label: 'Account' }], + measures: [{ name: 'revenue', aggregate: 'sum', field: 'amount' }], + }); + const labelScopes: Array<{ target: string; scope: unknown }> = []; + const svc = new AnalyticsService({ + queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }), + executeRawSql: async () => [{ account: 'acc1', revenue: 1000 }], + // Per-object scope: opportunity and crm_account get DIFFERENT predicates, so + // the assertion proves the label fetch used the referenced object's scope, + // not the base object's. + getReadScope: (object, ctx?: ExecutionContext) => { + if (!ctx?.tenantId) return undefined; + return object === 'crm_account' + ? { organization_id: ctx.tenantId, is_public: true } + : { organization_id: ctx.tenantId }; + }, + labelResolver: { + getObjectFields: (obj) => ({ + opportunity: { account: { type: 'lookup', reference: 'crm_account' } }, + crm_account: { name: { type: 'text' } }, + } as Record>)[obj], + fetchRecordLabels: async (target, ids, scope) => { + labelScopes.push({ target, scope }); + const m = new Map(); + for (const id of ids) m.set(id, `name-${String(id)}`); + return m; + }, + }, + }); + + const result = await svc.queryDataset( + byAccount, + { dimensions: ['account'], measures: ['revenue'] }, + { tenantId: 'org_A' } as ExecutionContext, + ) as any; + + // The label fetch ran against the referenced object, carrying ITS scope. + expect(labelScopes).toEqual([ + { target: 'crm_account', scope: { organization_id: 'org_A', is_public: true } }, + ]); + // And the label actually resolved (end-to-end sanity). + expect(result.rows).toEqual([{ account: 'name-acc1', revenue: 1000 }]); + }); }); diff --git a/packages/services/service-analytics/src/analytics-service.ts b/packages/services/service-analytics/src/analytics-service.ts index 9a01611aa..04064de87 100644 --- a/packages/services/service-analytics/src/analytics-service.ts +++ b/packages/services/service-analytics/src/analytics-service.ts @@ -604,14 +604,23 @@ export class AnalyticsService implements IAnalyticsService { .filter((d) => !!d.field) .map((d) => ({ name: d.name, field: d.field, type: d.type, dateGranularity: d.dateGranularity })); if (dims.length) { + // #3602 — bind the referenced object's read scope to THIS request so the + // label lookup (a per-record read of the related object) cannot surface a + // record the referenced object's RLS would hide. Same provider the + // aggregate path uses; `undefined` when no provider is configured, in + // which case labels fetch unscoped exactly as before. + const provider = this.readScopeProvider; + const resolveScope = provider + ? (targetObject: string) => provider(targetObject, context) + : undefined; try { - await resolveDimensionLabels(dataset.object, dims, result.rows, this.labelResolver); + await resolveDimensionLabels(dataset.object, dims, result.rows, this.labelResolver, resolveScope); // Totals rows (#1753) carry dimension values too (a row subtotal is // keyed by its row bucket) — resolve each grouping's own subset. for (const total of result.totals ?? []) { const subset = dims.filter((d) => total.dimensions.includes(d.name)); if (subset.length) { - await resolveDimensionLabels(dataset.object, subset, total.rows, this.labelResolver); + await resolveDimensionLabels(dataset.object, subset, total.rows, this.labelResolver, resolveScope); } } } catch (e) { diff --git a/packages/services/service-analytics/src/dimension-labels.ts b/packages/services/service-analytics/src/dimension-labels.ts index c26840c25..684098973 100644 --- a/packages/services/service-analytics/src/dimension-labels.ts +++ b/packages/services/service-analytics/src/dimension-labels.ts @@ -39,10 +39,34 @@ export interface DimensionLabelDeps { * Fetch a map of `id → display label` for the given ids of a target object. * The implementation chooses the target's display field. Returning an empty * map (e.g. no display field, no data access) leaves the ids unresolved. + * + * `scope` (ADR-0021 D-C, #3602) is the TARGET object's own read scope — the + * RLS/tenant `FilterCondition` the implementation must AND into the label + * lookup so this never reveals a related record the target object's RLS would + * hide. The label lookup is a per-record read (`group by id`) dressed as an + * aggregate; without the scope it leaks display names whenever the referenced + * object is more restricted than the base object whose rows carry the id. + * `undefined` means "no scope for this object" (global table / unrestricted + * caller) — the same contract as the read-scope provider. */ - fetchRecordLabels(targetObject: string, ids: unknown[]): Promise>; + fetchRecordLabels( + targetObject: string, + ids: unknown[], + scope?: Record, + ): Promise>; } +/** + * Resolve the TARGET object's read scope for a label lookup (#3602). Returns the + * object's RLS/tenant `FilterCondition`, `null`/`undefined` when the object is + * unscoped, or a rejected promise when the scope cannot be resolved — in which + * case the resolver fails CLOSED (skips that dimension's labels) rather than + * fetching unscoped names. + */ +export type LabelScopeResolver = ( + targetObject: string, +) => Promise | null | undefined> | Record | null | undefined; + const LOOKUP_TYPES = new Set(['lookup', 'master_detail']); /** Date-dimension granularity (mirrors the dataset `dateGranularity` enum). */ @@ -100,12 +124,18 @@ export function formatDateBucket(value: unknown, granularity?: DateGranularity | * (row key = `name`) * @param rows - result rows, mutated in place * @param deps - injected runtime capabilities + * @param resolveScope - (ADR-0021 D-C, #3602) resolves the referenced object's + * own read scope for a lookup/master_detail dimension's label fetch. When it + * throws, that dimension's labels are SKIPPED (fail-closed — the raw id renders + * instead) rather than fetched unscoped. Omit when no read-scope provider is + * configured (labels then fetch unscoped, as before — no security in play). */ export async function resolveDimensionLabels( baseObject: string, dims: Array<{ name: string; field: string; type?: string; dateGranularity?: DateGranularity | string }>, rows: Record[], deps: DimensionLabelDeps, + resolveScope?: LabelScopeResolver, ): Promise { if (!rows.length || !dims.length) return; const fields = deps.getObjectFields(baseObject); @@ -148,7 +178,20 @@ export async function resolveDimensionLabels( new Set(rows.map((r) => r[dim.name]).filter((v) => v != null)), ); if (ids.length === 0) continue; - const labelById = await deps.fetchRecordLabels(meta.reference, ids); + // #3602 — the label lookup reads the REFERENCED object by id. Scope it to + // that object's own RLS so it never surfaces a related record the target's + // RLS would hide (leak fires when the referenced object is stricter than + // the base). Fail closed: if the scope can't be resolved, skip this + // dimension's labels (raw id renders) rather than fetch unscoped. + let scope: Record | null | undefined; + if (resolveScope) { + try { + scope = await resolveScope(meta.reference); + } catch { + continue; + } + } + const labelById = await deps.fetchRecordLabels(meta.reference, ids, scope ?? undefined); if (!labelById || labelById.size === 0) continue; for (const row of rows) { const label = labelById.get(row[dim.name]); diff --git a/packages/services/service-analytics/src/plugin.ts b/packages/services/service-analytics/src/plugin.ts index 6c97fb026..be2cdd08d 100644 --- a/packages/services/service-analytics/src/plugin.ts +++ b/packages/services/service-analytics/src/plugin.ts @@ -353,17 +353,24 @@ export class AnalyticsServicePlugin implements Plugin { }; const labelResolver: DimensionLabelDeps = { getObjectFields: (objectName) => dataEngine()?.getObject?.(objectName)?.fields, - fetchRecordLabels: async (targetObject, ids) => { + fetchRecordLabels: async (targetObject, ids, scope) => { const map = new Map(); const displayField = pickDisplayField(dataEngine()?.getObject?.(targetObject)?.fields); if (!displayField || !executeAggregate || ids.length === 0) return map; + // #3602 — AND the referenced object's own read scope into the id filter, + // with `$and` (never key-merge) so it cannot be displaced by the id + // predicate — the same composition the strategy uses for the aggregate. + // Without it this per-record read leaks display names the target's RLS + // would hide (fires when the referenced object is stricter than the base). + const idFilter: Record = { id: { $in: ids } }; + const filter = scope ? { $and: [idFilter, scope] } : idFilter; // Group by (id, displayField) — one row per record — reusing the aggregate // bridge rather than adding a record-fetch capability. A count keeps engines // that require ≥1 aggregation happy; the count itself is unused. const rows = await executeAggregate(targetObject, { groupBy: ['id', displayField], aggregations: [{ field: 'id', method: 'count', alias: '_c' }], - filter: { id: { $in: ids } }, + filter, }); for (const r of rows) { if (r.id != null && r[displayField] != null) map.set(r.id, String(r[displayField]));