Skip to content

Commit 415254c

Browse files
os-zhuangclaude
andauthored
fix(analytics): scope the dimension-label lookup to the referenced object's RLS (#3602 residual 1) (#3639)
* fix(analytics): scope dimension-label lookup to the referenced object's RLS (#3602) A dataset that groups by a lookup/master_detail dimension 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 read a name the referenced object's own RLS would hide. (Same-object and looser-referenced cases were already safe: the ids come from the post-#3597 scoped aggregate. This closes the stricter-referenced case, the one residual flagged in #3602.) 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 the id predicate can't displace it. Fail-closed: if that object's scope can't be resolved, the dimension's labels are skipped (raw id renders) instead of fetched unscoped. No change when no read-scope provider is configured. `DimensionLabelDeps.fetchRecordLabels` gains an optional `scope` arg and `resolveDimensionLabels` an optional `resolveScope` resolver — both service-analytics-internal, no spec/contract change. Tests: dimension-labels unit cases prove the referenced-object scope is threaded and that resolution failure fails closed; a query-dataset integration case proves AnalyticsService.queryDataset binds the referenced object's scope (not the base object's) — reverting the wiring turns it red. Co-Authored-By: Claude <noreply@anthropic.com> * test(dogfood): e2e gate for the #3602 dimension-label read-scope leak Boots the real stack (HTTP → SecurityPlugin → AnalyticsServicePlugin auto-bridges → the plugin's real fetchRecordLabels closure) with a two-object owner-scoped fixture: a member owns a deal pointing at a vendor OWNED BY THE ADMIN. Grouping deals by vendor, resolving the grouped id to a NAME reads the vendor object — which the member cannot read. Asserts the member gets the vendor id RAW (the name does not leak) while the admin, who owns the vendor, still gets the resolved name — proving the fix scopes rather than blanks. Reverting the plugin's scope application reproduces the leak (member receives "Admin-Owned Vendor"). This is the only gate that exercises the plugin's real label closure; the service-analytics unit/integration tests fake fetchRecordLabels. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent ce1f100 commit 415254c

8 files changed

Lines changed: 387 additions & 6 deletions

File tree

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
---
2+
"@objectstack/service-analytics": patch
3+
---
4+
5+
fix(analytics): scope the dimension-label lookup to the referenced object's RLS (#3602)
6+
7+
When a dataset groups by a `lookup`/`master_detail` dimension, analytics resolves
8+
the grouped FK ids to the related record's display name via a per-record read
9+
(`group by id`) dressed as an aggregate. That read carried **no read scope**, so
10+
it revealed related-record display names whenever the referenced object's RLS is
11+
stricter than the base object whose rows carry the id — a user could see a name
12+
the referenced object's own RLS would hide. (Same-object and looser-referenced
13+
cases were already safe because the ids come from the post-#3597 scoped
14+
aggregate; this closes the stricter-referenced case.)
15+
16+
The label lookup now applies the **referenced object's own** read scope — bound
17+
to the request via the same `getReadScope` provider the aggregate path uses,
18+
composed with `$and` (never key-merge) so it can't be displaced by the id
19+
predicate. Fail-closed: if that object's scope can't be resolved, the dimension's
20+
labels are skipped (the raw id renders) rather than fetched unscoped. No behaviour
21+
change when no read-scope provider is configured.
22+
23+
Internal `DimensionLabelDeps.fetchRecordLabels` gains an optional `scope` argument
24+
and `resolveDimensionLabels` an optional `resolveScope` resolver; both are
25+
service-analytics-internal (no spec/contract change).
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// END-TO-END gate for #3602 residual 1 — "the dimension-label lookup is scoped
4+
// to the referenced object's RLS" — proven through the REAL stack: HTTP → REST
5+
// exec context → SecurityPlugin → AnalyticsServicePlugin auto-bridges → the
6+
// plugin's real fetchRecordLabels closure that ANDs the referenced object's
7+
// scope into the label read.
8+
//
9+
// The member owns a deal that points at a vendor OWNED BY THE ADMIN. Grouping
10+
// deals by vendor, the member's bucket carries the vendor id; resolving that id
11+
// to a NAME reads the vendor object, which the member cannot read. Before the
12+
// fix the member got the vendor's name (leak); after it, the vendor scope
13+
// excludes the admin's vendor so the raw id renders. The admin — who owns the
14+
// vendor — still sees the name, proving the fix SCOPES rather than blanks.
15+
//
16+
// This is the only gate that exercises the plugin's real label closure; the
17+
// service-analytics unit/integration tests fake fetchRecordLabels.
18+
19+
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
20+
import { bootStack, type VerifyStack } from '@objectstack/verify';
21+
import { labelScopeStack, labelScopeSecurity } from './fixtures/label-scope-fixture.js';
22+
23+
const VENDOR_NAME = 'Admin-Owned Vendor';
24+
25+
const dealsByVendor = {
26+
name: 'deals_by_vendor',
27+
label: 'Deals by vendor',
28+
object: 'lbl_deal',
29+
dimensions: [{ name: 'vendor', label: 'Vendor', field: 'vendor', type: 'lookup' }],
30+
measures: [{ name: 'cnt', label: 'Count', aggregate: 'count' }],
31+
};
32+
33+
describe('dogfood: dimension-label lookup is RLS-scoped to the referenced object (#3602)', () => {
34+
let stack: VerifyStack;
35+
let adminToken: string;
36+
let memberToken: string;
37+
let vendorId: string;
38+
39+
beforeAll(async () => {
40+
stack = await bootStack(labelScopeStack as never, { security: labelScopeSecurity() });
41+
adminToken = await stack.signIn();
42+
memberToken = await stack.signUp('lbl-member@verify.test');
43+
44+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
45+
const ql = await stack.kernel.getServiceAsync<any>('objectql');
46+
const sys = { context: { isSystem: true } };
47+
const uid = async (email: string): Promise<string> => {
48+
const u = await ql.findOne('sys_user', { where: { email }, ...sys });
49+
if (!u?.id) throw new Error(`no sys_user for ${email}`);
50+
return u.id as string;
51+
};
52+
const adminId = await uid('admin@objectos.ai');
53+
const memberId = await uid('lbl-member@verify.test');
54+
55+
// Vendor owned by the admin — the member must NOT be able to read it.
56+
const vendor = await ql.insert('lbl_vendor', { name: VENDOR_NAME, created_by: adminId }, sys);
57+
vendorId = vendor.id as string;
58+
59+
// One deal per principal, BOTH pointing at the admin's vendor. Authored as
60+
// system with an explicit owner so the owner policy binds deterministically
61+
// (and the member's deal can reference a vendor it cannot itself read).
62+
await ql.insert('lbl_deal', { name: 'member deal', amount: 10, vendor: vendorId, created_by: memberId }, sys);
63+
await ql.insert('lbl_deal', { name: 'admin deal', amount: 20, vendor: vendorId, created_by: adminId }, sys);
64+
65+
// Preconditions: the member reads their deal but NOT the admin's vendor.
66+
const deals = await stack.apiAs(memberToken, 'GET', '/data/lbl_deal');
67+
expect(((await deals.json()).records ?? []).map((r: any) => r.name)).toEqual(['member deal']);
68+
const vendors = await stack.apiAs(memberToken, 'GET', '/data/lbl_vendor');
69+
expect((await vendors.json()).records ?? []).toHaveLength(0);
70+
}, 90_000);
71+
72+
afterAll(async () => {
73+
await stack?.stop();
74+
});
75+
76+
async function vendorCellFor(token: string): Promise<unknown> {
77+
const res = await stack.apiAs(token, 'POST', '/analytics/dataset/query', {
78+
dataset: dealsByVendor,
79+
selection: { dimensions: ['vendor'], measures: ['cnt'] },
80+
});
81+
expect(res.status).toBe(200);
82+
const body = (await res.json()) as { rows?: Array<Record<string, unknown>> };
83+
// Exactly one deal is visible to each principal → one vendor bucket.
84+
expect(body.rows ?? []).toHaveLength(1);
85+
return (body.rows ?? [])[0].vendor;
86+
}
87+
88+
it('the member sees the vendor id RAW — the name it cannot read does not leak', async () => {
89+
const cell = await vendorCellFor(memberToken);
90+
expect(cell).toBe(vendorId);
91+
expect(cell).not.toBe(VENDOR_NAME);
92+
});
93+
94+
it('the admin — who owns the vendor — still sees the resolved name (scopes, not blanks)', async () => {
95+
const cell = await vendorCellFor(adminToken);
96+
expect(cell).toBe(VENDOR_NAME);
97+
});
98+
});
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// Two-object fixture for the #3602 dimension-label leak (residual 1).
4+
//
5+
// A dataset that groups by a LOOKUP dimension resolves each grouped FK id to the
6+
// related record's display NAME. That label read used to carry no read scope, so
7+
// it revealed the referenced record's name even when the reader could not read
8+
// that record — the leak fires whenever the base object's rows point at a
9+
// referenced record the reader's RLS hides.
10+
//
11+
// This fixture reproduces exactly that with zero dependence on org-scoping:
12+
// • `lbl_vendor` — owner-scoped on `created_by`.
13+
// • `lbl_deal` — owner-scoped on `created_by`, with a `vendor` lookup.
14+
// A member owns a deal that points at a vendor OWNED BY THE ADMIN. The member can
15+
// read their deal (owner match) but NOT the vendor (owned by someone else), so a
16+
// deals-by-vendor dataset must show the vendor's raw id to the member and its
17+
// display name only to the admin who owns it.
18+
19+
import { defineStack } from '@objectstack/spec';
20+
import { ObjectSchema, Field } from '@objectstack/spec/data';
21+
import { PermissionSetSchema, RLS, type PermissionSet } from '@objectstack/spec/security';
22+
import { SecurityPlugin, securityDefaultPermissionSets } from '@objectstack/plugin-security';
23+
24+
export const Vendor = ObjectSchema.create({
25+
name: 'lbl_vendor',
26+
// [ADR-0090 D1] grandfather stamp — the gate under test is permission-set RLS,
27+
// not owner-sharing.
28+
sharingModel: 'public_read_write',
29+
label: 'Vendor',
30+
pluralLabel: 'Vendors',
31+
fields: {
32+
name: Field.text({ label: 'Name', required: true }),
33+
},
34+
});
35+
36+
export const Deal = ObjectSchema.create({
37+
name: 'lbl_deal',
38+
sharingModel: 'public_read_write',
39+
label: 'Deal',
40+
pluralLabel: 'Deals',
41+
fields: {
42+
name: Field.text({ label: 'Name', required: true }),
43+
amount: Field.number({ label: 'Amount' }),
44+
vendor: Field.lookup('lbl_vendor', { label: 'Vendor' }),
45+
},
46+
});
47+
48+
export const labelScopeStack = defineStack({
49+
manifest: {
50+
id: 'com.dogfood.label_scope',
51+
namespace: 'lbl',
52+
version: '0.0.0',
53+
type: 'app',
54+
name: 'Label Scope Fixture',
55+
description: 'Deal → vendor lookup exercising the #3602 label read-scope leak.',
56+
},
57+
objects: [Vendor, Deal],
58+
});
59+
60+
const MEMBER_SET = 'lbl_fixture_member';
61+
62+
/**
63+
* The fallback set a fresh member resolves to: CRUD on both objects (so requests
64+
* reach the RLS layer rather than being denied by RBAC) plus an OWNER policy on
65+
* each. The member therefore reads only rows they own — the precondition the
66+
* label leak needs (their deal points at the admin's vendor).
67+
*/
68+
export const memberSet: PermissionSet = PermissionSetSchema.parse({
69+
name: MEMBER_SET,
70+
label: 'Label Fixture Member — owner-scoped',
71+
objects: {
72+
lbl_deal: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true },
73+
lbl_vendor: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true },
74+
},
75+
rowLevelSecurity: [
76+
RLS.ownerPolicy('lbl_deal', 'created_by'),
77+
RLS.ownerPolicy('lbl_vendor', 'created_by'),
78+
],
79+
});
80+
81+
export function labelScopeSecurity(): SecurityPlugin {
82+
return new SecurityPlugin({
83+
defaultPermissionSets: [...securityDefaultPermissionSets, memberSet],
84+
fallbackPermissionSet: memberSet.name,
85+
});
86+
}

packages/services/service-analytics/src/__tests__/dimension-labels.test.ts

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,71 @@ describe('resolveDimensionLabels', () => {
117117
expect(calls).toBe(1);
118118
expect(rows.map((r) => r.account)).toEqual(['Acme Corp', 'Acme Corp', 'Globex']);
119119
});
120+
121+
// ── #3602 — the label lookup must carry the REFERENCED object's read scope ──
122+
describe('lookup label read scope (#3602)', () => {
123+
it('passes the referenced object scope through to fetchRecordLabels', async () => {
124+
let seenScope: unknown = 'UNSET';
125+
let seenTarget: string | undefined;
126+
const d = deps({
127+
fetchRecordLabels: async (target, ids, scope) => {
128+
seenTarget = target;
129+
seenScope = scope;
130+
return new Map<unknown, string>(ids.map((id) => [id, `name-${String(id)}`]));
131+
},
132+
});
133+
const rows = [{ account: 'acc1', n: 1 }];
134+
// resolveScope returns the referenced object's own RLS predicate.
135+
await resolveDimensionLabels('task', [{ name: 'account', field: 'account' }], rows, d, (target) => {
136+
expect(target).toBe('crm_account'); // the REFERENCED object, not the base
137+
return { organization_id: 'org_A' };
138+
});
139+
expect(seenTarget).toBe('crm_account');
140+
expect(seenScope).toEqual({ organization_id: 'org_A' });
141+
});
142+
143+
it('fails CLOSED: when the scope cannot be resolved, the id is left raw (no unscoped fetch)', async () => {
144+
let fetched = false;
145+
const d = deps({
146+
fetchRecordLabels: async (_t, ids) => {
147+
fetched = true;
148+
return new Map<unknown, string>(ids.map((id) => [id, 'LEAKED NAME']));
149+
},
150+
});
151+
const rows = [{ account: 'acc1', n: 1 }];
152+
await resolveDimensionLabels('task', [{ name: 'account', field: 'account' }], rows, d, () => {
153+
throw new Error('security service unavailable');
154+
});
155+
// The label fetch must NOT have run, and the raw id must survive.
156+
expect(fetched).toBe(false);
157+
expect(rows).toEqual([{ account: 'acc1', n: 1 }]);
158+
});
159+
160+
it('a select dimension is unaffected by scope resolution (no referenced object)', async () => {
161+
let scopeCalls = 0;
162+
const rows = [{ status: 'backlog', n: 1 }];
163+
await resolveDimensionLabels('task', [{ name: 'status', field: 'status' }], rows, deps(), () => {
164+
scopeCalls++;
165+
return undefined;
166+
});
167+
expect(scopeCalls).toBe(0); // scope is only resolved for lookup/master_detail dims
168+
expect(rows).toEqual([{ status: 'Backlog', n: 1 }]);
169+
});
170+
171+
it('no resolver (no security configured) → unscoped fetch, unchanged behaviour', async () => {
172+
let seenScope: unknown = 'UNSET';
173+
const d = deps({
174+
fetchRecordLabels: async (_t, ids, scope) => {
175+
seenScope = scope;
176+
return new Map<unknown, string>(ids.map((id) => [id, 'Acme Corp']));
177+
},
178+
});
179+
const rows = [{ account: 'acc1', n: 1 }];
180+
await resolveDimensionLabels('task', [{ name: 'account', field: 'account' }], rows, d /* no resolveScope */);
181+
expect(seenScope).toBeUndefined();
182+
expect(rows).toEqual([{ account: 'Acme Corp', n: 1 }]);
183+
});
184+
});
120185
});
121186

122187
describe('formatDateBucket', () => {

packages/services/service-analytics/src/__tests__/query-dataset.test.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,4 +393,52 @@ describe('AnalyticsService.queryDataset', () => {
393393
expect(result.totals[1].dimensions).toEqual([]);
394394
expect(result.drillRawTotals[1]).toEqual([{}]);
395395
});
396+
397+
// ── #3602 — the lookup label read is scoped to the REFERENCED object's RLS ──
398+
it('threads the referenced object read scope (not the base object) into the label fetch', async () => {
399+
const byAccount = DatasetSchema.parse({
400+
name: 'sales_acct_scoped', label: 'Sales', object: 'opportunity', include: [],
401+
dimensions: [{ name: 'account', field: 'account', type: 'lookup', label: 'Account' }],
402+
measures: [{ name: 'revenue', aggregate: 'sum', field: 'amount' }],
403+
});
404+
const labelScopes: Array<{ target: string; scope: unknown }> = [];
405+
const svc = new AnalyticsService({
406+
queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }),
407+
executeRawSql: async () => [{ account: 'acc1', revenue: 1000 }],
408+
// Per-object scope: opportunity and crm_account get DIFFERENT predicates, so
409+
// the assertion proves the label fetch used the referenced object's scope,
410+
// not the base object's.
411+
getReadScope: (object, ctx?: ExecutionContext) => {
412+
if (!ctx?.tenantId) return undefined;
413+
return object === 'crm_account'
414+
? { organization_id: ctx.tenantId, is_public: true }
415+
: { organization_id: ctx.tenantId };
416+
},
417+
labelResolver: {
418+
getObjectFields: (obj) => ({
419+
opportunity: { account: { type: 'lookup', reference: 'crm_account' } },
420+
crm_account: { name: { type: 'text' } },
421+
} as Record<string, Record<string, { type?: string; reference?: string }>>)[obj],
422+
fetchRecordLabels: async (target, ids, scope) => {
423+
labelScopes.push({ target, scope });
424+
const m = new Map<unknown, string>();
425+
for (const id of ids) m.set(id, `name-${String(id)}`);
426+
return m;
427+
},
428+
},
429+
});
430+
431+
const result = await svc.queryDataset(
432+
byAccount,
433+
{ dimensions: ['account'], measures: ['revenue'] },
434+
{ tenantId: 'org_A' } as ExecutionContext,
435+
) as any;
436+
437+
// The label fetch ran against the referenced object, carrying ITS scope.
438+
expect(labelScopes).toEqual([
439+
{ target: 'crm_account', scope: { organization_id: 'org_A', is_public: true } },
440+
]);
441+
// And the label actually resolved (end-to-end sanity).
442+
expect(result.rows).toEqual([{ account: 'name-acc1', revenue: 1000 }]);
443+
});
396444
});

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -604,14 +604,23 @@ export class AnalyticsService implements IAnalyticsService {
604604
.filter((d) => !!d.field)
605605
.map((d) => ({ name: d.name, field: d.field, type: d.type, dateGranularity: d.dateGranularity }));
606606
if (dims.length) {
607+
// #3602 — bind the referenced object's read scope to THIS request so the
608+
// label lookup (a per-record read of the related object) cannot surface a
609+
// record the referenced object's RLS would hide. Same provider the
610+
// aggregate path uses; `undefined` when no provider is configured, in
611+
// which case labels fetch unscoped exactly as before.
612+
const provider = this.readScopeProvider;
613+
const resolveScope = provider
614+
? (targetObject: string) => provider(targetObject, context)
615+
: undefined;
607616
try {
608-
await resolveDimensionLabels(dataset.object, dims, result.rows, this.labelResolver);
617+
await resolveDimensionLabels(dataset.object, dims, result.rows, this.labelResolver, resolveScope);
609618
// Totals rows (#1753) carry dimension values too (a row subtotal is
610619
// keyed by its row bucket) — resolve each grouping's own subset.
611620
for (const total of result.totals ?? []) {
612621
const subset = dims.filter((d) => total.dimensions.includes(d.name));
613622
if (subset.length) {
614-
await resolveDimensionLabels(dataset.object, subset, total.rows, this.labelResolver);
623+
await resolveDimensionLabels(dataset.object, subset, total.rows, this.labelResolver, resolveScope);
615624
}
616625
}
617626
} catch (e) {

0 commit comments

Comments
 (0)