Skip to content

Commit a40c0f9

Browse files
claude[bot]claude
andauthored
fix(service-analytics): guard fetchRecordLabels with assertReadScopeCannotVacate (#14400)
The record-label hook is a fourth consumer of the same readScopeProvider output the three unified faces guard, and it met neither compileScopedFilterToSql nor the vacancy guard: it $ands the referenced object's scope with `id $in [...]` and hands that straight to executeAggregate. A vacating spelling from an out-of-repo getReadScope producer therefore let a row-granular per-record read run effectively unscoped, surfacing exactly the display names the referenced object's RLS exists to hide. Call the already-exported assertReadScopeCannotVacate before the filter composition, in the same envelope as the siblings (READ_SCOPE_COMPILE_FAILED / 500). Placement mirrors ObjectQLStrategy.resolveFkAttr, this hook's structural twin: after the early returns (a call that reads nothing cannot widen anything) and before the chunk loop (one scope, one verdict). Zero compiler change. Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 953a81f commit a40c0f9

3 files changed

Lines changed: 342 additions & 0 deletions

File tree

.changeset/tidy-ducks-tickle.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
'@objectstack/service-analytics': patch
3+
---
4+
5+
Guard the analytics record-label lookup with `assertReadScopeCannotVacate` — the fourth read-scope door
6+
7+
`AnalyticsServicePlugin`'s `fetchRecordLabels` hook `$and`s the **referenced** object's read scope with an `id $in [...]` filter and hands the result straight to `executeAggregate`. Unlike the three faces unified previously (the ObjectQL engine merge, the `/analytics/sql` echo merge, and `NativeSQLStrategy.applyReadScope`), it met neither `compileScopedFilterToSql` nor the vacancy guard, so a read scope that lowers to a boolean constant — the `$not`-over-`$in: []` family reachable from any out-of-repo `StrategyContext.getReadScope` producer — let that per-record read run effectively unscoped for the ids in hand, surfacing the display names the referenced object's RLS exists to hide.
8+
9+
The hook now calls the already-exported `assertReadScopeCannotVacate` on the referenced object's scope before composing the filter, refusing in the same envelope as its siblings (`READ_SCOPE_COMPILE_FAILED` / 500). No behaviour changes for scopes that bind: an ordinary referenced-object scope still narrows the label lookup, and the `$in: []` zero-rows reduction (including the live RLS composite that pairs it with an own-rows grant) still passes through untouched. The read-scope SQL compiler is unchanged.
Lines changed: 308 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,308 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#14329] The FOURTH read-scope door — `AnalyticsServicePlugin`'s
5+
* `fetchRecordLabels` hook — answers the same verdict as the other three.
6+
*
7+
* #13640 guarded the ObjectQL ENGINE merge and #13926 the `/analytics/sql`
8+
* ECHO merge plus `NativeSQLStrategy.applyReadScope`; the three-faces file
9+
* next door pins those. This hook is a FOURTH consumer of the very same
10+
* `readScopeProvider` output, reached by a different route entirely
11+
* (`AnalyticsService.queryDataset` → `resolveScope` → `dimension-labels.ts` →
12+
* `DimensionLabelDeps.fetchRecordLabels`, the closure `plugin.ts` builds), and
13+
* it met NEITHER `compileScopedFilterToSql` nor `assertReadScopeCannotVacate`:
14+
* it `$and`s the REFERENCED object's scope with `id $in [...]` and hands that
15+
* straight to `executeAggregate`.
16+
*
17+
* So a vacating scope spelling from an out-of-repo `getReadScope` producer
18+
* (`StrategyContext.getReadScope` is a spec contract — that population is
19+
* exactly who this contract exists for, and the one with no producer-side
20+
* #13570 guard) let this per-record read run effectively unscoped for the ids
21+
* in hand, surfacing the display names the referenced object's RLS exists to
22+
* hide. The leak is row-granular by construction: `group by (id, name)` is a
23+
* record read dressed as an aggregate.
24+
*
25+
* ## What is measured here, and what is NOT
26+
*
27+
* These cases drive the REAL plugin wiring — `new AnalyticsServicePlugin(...).init(ctx)`
28+
* — so the closure under test is the one `plugin.ts` actually ships, not a
29+
* stub standing in for it. What they do NOT re-measure is the ENGINE's
30+
* lowering of a vacating scope: that table (which spellings come back with the
31+
* whole table, driven against a real `SqliteWasmDriver`) is
32+
* `read-scope-vacancy-three-faces.test.ts`'s, and re-deriving it here would be
33+
* a second copy of one ruling. The fixture engine below therefore honours the
34+
* filter it is handed by a small, deliberately obvious evaluator — which is
35+
* the right authority for THIS seam's question: *does the hook forward a scope
36+
* that a scope-honouring engine can narrow by, and does it refuse the
37+
* spellings that cannot narrow anything at all?*
38+
*
39+
* ## Two label passes, two DIFFERENT dispositions — both fail closed
40+
*
41+
* A refusal from this hook surfaces differently depending on which of
42+
* `queryDataset`'s two label passes raised it, and both are asserted below
43+
* because a reader who checks only one will conclude the other is unguarded:
44+
*
45+
* - **sort-key pass** (`order` on a lookup dimension, #3680) runs inside
46+
* `DatasetExecutor.execute`, whose catch in `queryDataset` re-throws a
47+
* DECLARED ADR-0112 envelope untouched (`hasDeclaredErrorEnvelope`). The
48+
* refusal reaches the caller as itself — `READ_SCOPE_COMPILE_FAILED` / 500.
49+
* - **display pass** (#3602) is wrapped in its own try/catch that degrades to
50+
* a `warn` and leaves raw ids rendering. That is not this card weakening:
51+
* it is the disposition #3602 already chose for this surface one frame up
52+
* (`dimension-labels.ts` skips a dimension's labels rather than fetch
53+
* unscoped when the scope cannot be resolved), and it is fail-CLOSED — no
54+
* name is fetched, so none can leak.
55+
*
56+
* The security property is therefore identical on both passes and is asserted
57+
* as such: **the referenced object is never read at all**. A bare "it threw"
58+
* would not distinguish that from a read that happened and then threw.
59+
*/
60+
61+
import { describe, it, expect, vi } from 'vitest';
62+
import { DatasetSchema } from '@objectstack/spec/ui';
63+
import type { ExecutionContext } from '@objectstack/spec/kernel';
64+
import type { FilterCondition } from '@objectstack/spec/data';
65+
import { AnalyticsService } from '../analytics-service.js';
66+
import { AnalyticsServicePlugin } from '../plugin.js';
67+
68+
const CTX = { tenantId: 'org_A', userId: 'u_me' } as ExecutionContext;
69+
70+
/** Tasks grouped by a lookup dimension whose target is `crm_account`. */
71+
const DATASET = DatasetSchema.parse({
72+
name: 'tasks_by_account',
73+
label: 'Tasks by account',
74+
object: 'task',
75+
dimensions: [{ name: 'account', field: 'account', type: 'lookup', label: 'Account' }],
76+
measures: [{ name: 'cnt', aggregate: 'count' }],
77+
});
78+
79+
/**
80+
* Referenced-object fixture rows. `organization_id` is what an ordinary
81+
* tenant scope narrows by; `owner` is what the emptied-membership spellings
82+
* address. `acc2` is the row an ordinary `org_A` scope must NOT surface.
83+
*/
84+
const ACCOUNTS = [
85+
{ id: 'acc1', name: 'Acme Corp', organization_id: 'org_A', owner: 'u_me' },
86+
{ id: 'acc2', name: 'Umbrella Ltd', organization_id: 'org_B', owner: 'u_other' },
87+
];
88+
89+
/** The grouped base aggregate: both FK ids reach the label pass. */
90+
const TASK_ROWS = [
91+
{ account: 'acc1', cnt: 3 },
92+
{ account: 'acc2', cnt: 1 },
93+
];
94+
95+
/**
96+
* A deliberately small filter evaluator for the FIXTURE rows — equality,
97+
* `$in`, `$and`, `$or`. It exists so "an ordinary scope still narrows" and
98+
* "`$in: []` still reduces to zero rows" are read off real returned rows
99+
* rather than off the filter object, which would only echo the assertion.
100+
*
101+
* ⛔ Not an engine-lowering model, and not where a vacating spelling's row
102+
* consequence is established: an unrecognised operator throws rather than
103+
* quietly matching, so a spelling this cannot judge fails loudly instead of
104+
* manufacturing a comfortable answer. The measured lowering table lives in
105+
* `read-scope-vacancy-three-faces.test.ts`, against a real driver.
106+
*/
107+
function matches(row: Record<string, unknown>, filter: unknown): boolean {
108+
if (filter == null) return true;
109+
if (typeof filter !== 'object' || Array.isArray(filter)) {
110+
throw new Error(`[fixture] not a filter node: ${JSON.stringify(filter)}`);
111+
}
112+
return Object.entries(filter as Record<string, unknown>).every(([key, value]) => {
113+
if (key === '$and') return (value as unknown[]).every((n) => matches(row, n));
114+
if (key === '$or') return (value as unknown[]).some((n) => matches(row, n));
115+
if (value !== null && typeof value === 'object' && !Array.isArray(value)) {
116+
const ops = Object.entries(value as Record<string, unknown>);
117+
return ops.every(([op, comparand]) => {
118+
if (op === '$in') return (comparand as unknown[]).includes(row[key]);
119+
throw new Error(`[fixture] unsupported operator ${op} — this evaluator judges no spelling it was not written for`);
120+
});
121+
}
122+
return row[key] === value;
123+
});
124+
}
125+
126+
type EngineCall = { object: string; where?: Record<string, unknown> };
127+
128+
function fakePluginContext(services: Record<string, unknown>) {
129+
const registered: Record<string, unknown> = {};
130+
const warn = vi.fn();
131+
return {
132+
registered,
133+
warn,
134+
ctx: {
135+
getService: (name: string) => services[name] ?? registered[name],
136+
registerService: (name: string, svc: unknown) => { registered[name] = svc; },
137+
replaceService: (name: string, svc: unknown) => { registered[name] = svc; },
138+
logger: { info() {}, warn, error() {}, debug() {} },
139+
},
140+
};
141+
}
142+
143+
const objectqlOnly = () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false });
144+
145+
/**
146+
* Drive the label path through the real plugin wiring.
147+
*
148+
* `order` selects WHICH label pass runs: with it, the sort-key pass (#3680)
149+
* resolves labels inside `DatasetExecutor.execute`; without it, only the
150+
* display pass (#3602) does. The two have different refusal dispositions, so
151+
* every case below states which one it is exercising.
152+
*/
153+
async function runLabels(opts: { scope: FilterCondition | undefined; order?: boolean }) {
154+
const seen: EngineCall[] = [];
155+
const engine = {
156+
aggregate: async (object: string, options: Record<string, unknown>) => {
157+
seen.push({ object, where: options.where as Record<string, unknown> | undefined });
158+
if (object === 'task') return TASK_ROWS;
159+
return ACCOUNTS.filter((r) => matches(r, options.where)).map((r) => ({ id: r.id, name: r.name, _c: 1 }));
160+
},
161+
getObject: (name: string) =>
162+
name === 'task'
163+
? { fields: { account: { type: 'lookup', reference: 'crm_account' } } }
164+
: name === 'crm_account'
165+
? { fields: { name: { type: 'text' } } }
166+
: undefined,
167+
};
168+
const { ctx, registered, warn } = fakePluginContext({ data: engine });
169+
170+
await new AnalyticsServicePlugin({
171+
queryCapabilities: objectqlOnly,
172+
getReadScope: (object: string) => (object === 'crm_account' ? opts.scope : undefined),
173+
}).init(ctx as never);
174+
175+
const run = () =>
176+
(registered.analytics as AnalyticsService).queryDataset(
177+
DATASET as never,
178+
{
179+
dimensions: ['account'],
180+
measures: ['cnt'],
181+
...(opts.order ? { order: { account: 'asc' } } : {}),
182+
} as never,
183+
CTX,
184+
);
185+
186+
return { run, seen, warn };
187+
}
188+
189+
/** Did anything read the REFERENCED object? The security question, directly. */
190+
const readReferenced = (seen: EngineCall[]) => seen.filter((c) => c.object === 'crm_account');
191+
192+
/**
193+
* The vacating family, as measured in `read-scope-sql.ts`'s #13640 section:
194+
* every one of these came back with the whole table from a real engine.
195+
* `$nin: []` is refused at any polarity (matching `compileOperator`'s own
196+
* `$nin` arm); the rest are emptied POSITIVE memberships under an odd number
197+
* of negations, which is what makes them vacate.
198+
*/
199+
const VACATING: Array<[string, FilterCondition]> = [
200+
['empty $nin', { owner: { $nin: [] } } as FilterCondition],
201+
['$not over empty $in', { $not: { owner: { $in: [] } } } as FilterCondition],
202+
['$not over a bare empty array', { $not: { owner: [] } } as FilterCondition],
203+
['$not over a multi-key operator object holding an empty $in', { $not: { owner: { $in: [], $ne: 'u_other' } } } as FilterCondition],
204+
['a vacating arm inside an $or', { $or: [{ $not: { owner: { $in: [] } } }, { owner: 'u_me' }] } as FilterCondition],
205+
];
206+
207+
describe('#14329 — a vacating referenced-object scope is refused before the label lookup runs', () => {
208+
it.each(VACATING)('sort-key pass: %s refuses in the sibling envelope', async (_name, scope) => {
209+
const { run, seen } = await runLabels({ scope, order: true });
210+
211+
// ADR-0112 envelope, `code` AND `status` — the same two the three sibling
212+
// faces answer with. A bare `toThrow` would stay green against a driver
213+
// throwing a naked `Error`, which is the failure this assertion exists to
214+
// exclude.
215+
const err = await run().then(
216+
() => { throw new Error('expected a refusal, got a result'); },
217+
(e: unknown) => e as { code?: unknown; status?: unknown; message?: string },
218+
);
219+
expect(err.code).toBe('READ_SCOPE_COMPILE_FAILED');
220+
expect(err.status).toBe(500);
221+
expect(String(err.message)).toContain('read scope for "crm_account"');
222+
223+
// The other half of a refusal pin: the referenced object was NEVER read.
224+
// "It threw" alone does not distinguish a guard from a leak followed by a
225+
// throw — and the leak is precisely a read that happened.
226+
expect(readReferenced(seen)).toEqual([]);
227+
// The base aggregate still ran: the refusal is scoped to the label door.
228+
expect(seen.map((c) => c.object)).toEqual(['task']);
229+
});
230+
231+
it.each(VACATING)('display pass: %s fails closed to raw ids without reading the target', async (_name, scope) => {
232+
const { run, seen, warn } = await runLabels({ scope });
233+
234+
// The display pass has its own catch (analytics-service.ts) that degrades
235+
// to a warn — the #3602 disposition for this surface. So the CALLER sees
236+
// rows, and what matters is that no name was fetched to put in them.
237+
const result = await run() as unknown as { rows: Record<string, unknown>[] };
238+
expect(readReferenced(seen)).toEqual([]);
239+
expect(result.rows.map((r) => r.account)).toEqual(['acc1', 'acc2']);
240+
expect(warn).toHaveBeenCalledWith(expect.stringContaining('dimension label resolution failed'));
241+
});
242+
});
243+
244+
describe('#14329 over-denial controls — the guard refuses ONLY the vacating shapes', () => {
245+
it('an ordinary referenced-object scope still narrows the label lookup', async () => {
246+
const { run, seen } = await runLabels({ scope: { organization_id: 'org_A' } as FilterCondition });
247+
248+
const result = await run() as unknown as { rows: Record<string, unknown>[] };
249+
250+
// Preservation pin — the scope reached the engine `$and`-composed with the
251+
// id filter, never key-merged, so it cannot be displaced by the ids.
252+
const labelCall = readReferenced(seen);
253+
expect(labelCall).toHaveLength(1);
254+
expect(labelCall[0].where).toEqual({
255+
$and: [{ id: { $in: ['acc1', 'acc2'] } }, { organization_id: 'org_A' }],
256+
});
257+
258+
// ...and the NARROWED RESULT SET, not merely "no throw": `acc1` is in the
259+
// tenant and renders its name; `acc2` is out and keeps its raw id, which is
260+
// the whole point of scoping this read.
261+
expect(result.rows.map((r) => r.account)).toEqual(['Acme Corp', 'acc2']);
262+
});
263+
264+
it('the `$in: []` zero-rows reduction still yields no labels and no refusal', async () => {
265+
// Positive polarity: the ruled #5322/#5243 reduction to constant FALSE.
266+
// Narrowing at its own arm — the SAFE direction on a read scope — and
267+
// deliberately NOT refused, here or at any sibling door.
268+
const { run, seen } = await runLabels({ scope: { owner: { $in: [] } } as FilterCondition });
269+
270+
const result = await run() as unknown as { rows: Record<string, unknown>[] };
271+
272+
expect(readReferenced(seen)).toHaveLength(1);
273+
expect(readReferenced(seen)[0].where).toEqual({
274+
$and: [{ id: { $in: ['acc1', 'acc2'] } }, { owner: { $in: [] } }],
275+
});
276+
// Zero rows came back, so no label overwrites a raw id — and no refusal.
277+
expect(result.rows.map((r) => r.account)).toEqual(['acc1', 'acc2']);
278+
});
279+
280+
it('the live #13570 RLS composite keeps own rows flowing', async () => {
281+
// `{ $or: [{ owner: { $in: [] } }, { owner: 'u_me' }] }` — an emptied
282+
// membership beside an own-rows grant, which the RLS compiler really emits
283+
// when a membership set resolves empty. Refusing it would 500 every
284+
// analytics query for such a user, the outcome #13571's verdict rejected.
285+
const { run, seen } = await runLabels({
286+
scope: { $or: [{ owner: { $in: [] } }, { owner: 'u_me' }] } as FilterCondition,
287+
});
288+
289+
const result = await run() as unknown as { rows: Record<string, unknown>[] };
290+
291+
expect(readReferenced(seen)).toHaveLength(1);
292+
expect(result.rows.map((r) => r.account)).toEqual(['Acme Corp', 'acc2']);
293+
});
294+
295+
it('no scope at all still reads the target, unchanged', async () => {
296+
// The `undefined` arm — "no scope for this object" is a legitimate answer
297+
// from the provider contract, and the guard must not turn it into a
298+
// refusal. Without this case a guard that refused everything would pass
299+
// every refusal assertion above.
300+
const { run, seen } = await runLabels({ scope: undefined });
301+
302+
const result = await run() as unknown as { rows: Record<string, unknown>[] };
303+
304+
expect(readReferenced(seen)).toHaveLength(1);
305+
expect(readReferenced(seen)[0].where).toEqual({ id: { $in: ['acc1', 'acc2'] } });
306+
expect(result.rows.map((r) => r.account)).toEqual(['Acme Corp', 'Umbrella Ltd']);
307+
});
308+
});

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { AnalyticsService } from './analytics-service.js';
99
import type { AnalyticsServiceConfig } from './analytics-service.js';
1010
import type { AnalyticsDriverCapabilities } from './strategies/types.js';
1111
import { pickDisplayField, type DimensionLabelDeps } from './dimension-labels.js';
12+
import { assertReadScopeCannotVacate } from './read-scope-sql.js';
1213

1314
/**
1415
* The slice of the DECLARED engine contracts this plugin's auto-bridges
@@ -518,6 +519,30 @@ export class AnalyticsServicePlugin implements Plugin {
518519
const map = new Map<unknown, string>();
519520
const displayField = pickDisplayField(dataEngine()?.getObject?.(targetObject)?.fields);
520521
if (!displayField || !executeAggregate || ids.length === 0) return map;
522+
// [#14329] The FOURTH read-scope door, and the same guard the other
523+
// three answer with. #13640 guarded the ObjectQL engine merge and
524+
// #13926 the echo merge and `NativeSQLStrategy.applyReadScope`; this
525+
// hook is a fourth consumer of the same `readScopeProvider` output
526+
// that meets NEITHER `compileScopedFilterToSql` nor the guard — the
527+
// `$and` below hands the scope straight to `executeAggregate`, so a
528+
// vacating spelling (`$not` over `$in: []` and its measured siblings,
529+
// reachable from any out-of-repo `getReadScope` producer the
530+
// `StrategyContext` spec contract admits) used to let this per-record
531+
// read run effectively unscoped for the ids in hand — leaking exactly
532+
// the display names the referenced object's RLS exists to hide.
533+
//
534+
// Placement mirrors `ObjectQLStrategy.resolveFkAttr`, this hook's
535+
// structural twin (same id-`$in` `$and` scope, same `executeAggregate`,
536+
// guarded since #13640): AFTER the early returns, because a call that
537+
// reads nothing cannot widen anything and refusing it would be pure
538+
// over-denial; and BEFORE the chunk loop, so one scope gets one verdict
539+
// rather than one per 500 ids. The condition is spelled to match the
540+
// composition on the next line exactly, so the set of scopes guarded
541+
// and the set of scopes `$and`-ed are provably the same set.
542+
//
543+
// ⛔ Zero compiler change: the #13571 lowering residue is ruled and
544+
// untouched. This guard is the walk, not the lowering.
545+
if (scope) assertReadScopeCannotVacate(scope, targetObject);
521546
// #3680 — the sort-key pass hands over the PRE-window id set (every
522547
// grouped value, not just the displayed page), so a high-cardinality
523548
// lookup dimension can push thousands of ids through here. Chunk the

0 commit comments

Comments
 (0)