Skip to content

Commit 130e8c0

Browse files
committed
fix(service-analytics): also refuse a caller-named measure that reduces to a non-column source (#21156)
The member-shape door gate skipped the measure position: namedQueryFields drops an undeclared measure (the strategies refuse one the cube does not carry), so a caller-named measure that inference mints — its sql built from the caller's own text by inferMeasure — slipped the gate and reached the aggregate position of the native statement verbatim in the ungated tiers (no security service; an object the reader answers undefined for). The judged tier already refused it (the minted measure reads as a declared expression member and #21153 catches it); the ungated tiers did not. Judge the inferred source here too, against the same rule inference applies: a caller-named measure must reduce — after inferMeasure's suffix strip, the no-suffix default included — to count / '*' or to a column reference (bare identifier / relationship path), or it is refused with the same fieldReadUnjudgeableError (PERMISSION_DENIED / 403). Author-declared measures are untouched; a dotted non-qualifier measure is already refused by #5918 ahead of this. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ
1 parent 31a582c commit 130e8c0

2 files changed

Lines changed: 138 additions & 1 deletion

File tree

‎packages/services/service-analytics/src/__tests__/caller-member-column-reference-gate.test.ts‎

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,25 @@ const REGISTERED: Cube = {
4343
title: 'Ledger',
4444
sql: OBJ,
4545
public: true,
46-
measures: { count: { type: 'count', sql: '*', label: 'Count' } },
46+
measures: {
47+
count: { type: 'count', sql: '*', label: 'Count' },
48+
author_expr_measure: { type: 'number', sql: 'amount + 1', label: 'Author expression measure' },
49+
},
4750
dimensions: {
4851
status: { type: 'string', sql: 'status', label: 'Status' },
4952
author_expr: { type: 'number', sql: 'amount + 1', label: 'Author expression' },
5053
},
5154
} as Cube;
5255

56+
/**
57+
* A measure the CALLER names itself whose inferred source is not a column
58+
* reference: the suffixed form (`inferMeasure` strips `_sum` → `amount + 1`)
59+
* and the no-suffix default (the whole key → `amount + 1`). Both reach the
60+
* aggregate position verbatim before this gate.
61+
*/
62+
const CALLER_MEASURE_SUFFIXED = 'amount + 1_sum';
63+
const CALLER_MEASURE_NOSUFFIX = 'amount + 1';
64+
5365
/** A member the CALLER names itself, spelled as no column is. Neutral, not a payload. */
5466
const CALLER_EXPR = 'amount + 1';
5567

@@ -182,4 +194,64 @@ describe('[#21156] analytics — a caller-named non-column member is refused at
182194
expect(executed.length).toBeGreaterThan(0);
183195
});
184196
});
197+
198+
// An author-declared expression MEASURE is a native-SQL-only feature (the
199+
// ObjectQL aggregate AST cannot carry a raw SQL expression — a pre-existing
200+
// strategy refusal, not this gate's). The non-regression claim is that THIS
201+
// gate does not refuse it: on NativeSQL it still reaches the strategy with no
202+
// security service.
203+
it('an author-declared expression measure is not refused by this gate (served on NativeSQL, no security service)', async () => {
204+
const { service, executed } = makeService({ capabilities: nativeSqlOnly });
205+
await service.query({ cube: 'cm_cube', measures: ['author_expr_measure'] } as never, CALLER);
206+
expect(executed.length).toBeGreaterThan(0);
207+
});
208+
209+
// ── The MEASURE position (#21156 rework) ──────────────────────────────────
210+
//
211+
// A caller-named measure whose inferred source (after inferMeasure's suffix
212+
// strip, the no-suffix default included) is not a column reference reached
213+
// the aggregate position of the statement verbatim in the ungated tiers. It
214+
// is refused here, in every tier, before a strategy compiles it.
215+
describe('a caller-named measure that reduces to a non-column source is refused', () => {
216+
const MEASURES = [
217+
{ label: 'a suffixed inferred measure', measure: CALLER_MEASURE_SUFFIXED },
218+
{ label: 'the no-suffix default', measure: CALLER_MEASURE_NOSUFFIX },
219+
];
220+
describe.each([...UNJUDGED_TIERS, { label: 'judged object', readers: JUDGED }])(
221+
'tier: $label',
222+
({ readers }) => {
223+
describe.each(STRATEGY_PATHS)('$label', ({ capabilities }) => {
224+
describe.each(CUBES)('on $label', ({ cube }) => {
225+
it.each(MEASURES)('$label: refused PERMISSION_DENIED / 403 on both doors, nothing executed', async ({ measure }) => {
226+
const { service, executed } = makeService({ capabilities, ...(readers as object) });
227+
const q = { cube, measures: [measure] };
228+
for (const run of [
229+
() => service.query(q as never, CALLER),
230+
() => service.generateSql(q as never, CALLER),
231+
]) {
232+
const refusal = await run().then(() => null, (e: unknown) => e as Record<string, unknown>);
233+
expect(refusal).toMatchObject({ code: 'PERMISSION_DENIED', status: 403, member: measure });
234+
}
235+
expect(executed).toEqual([]);
236+
});
237+
});
238+
});
239+
},
240+
);
241+
242+
// No false positives: a valid inferred measure over a real column, and a
243+
// bare count, are served in every tier.
244+
describe.each([...UNJUDGED_TIERS, { label: 'judged object', readers: JUDGED }])(
245+
'the positive control in tier: $label',
246+
({ readers }) => {
247+
it.each(STRATEGY_PATHS)('$label: a valid _sum measure and a bare count are served', async ({ capabilities }) => {
248+
for (const cube of ['cm_adhoc', 'cm_cube']) {
249+
const { service, executed } = makeService({ capabilities, ...(readers as object) });
250+
await service.query({ cube, measures: ['amount_sum', 'count'] } as never, CALLER);
251+
expect(executed.length).toBeGreaterThan(0);
252+
}
253+
});
254+
},
255+
);
256+
});
185257
});

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

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ import { readScopeUnresolvedError } from './read-scope-refusal.js';
5050
import {
5151
assertNamedFieldsReadable,
5252
assertCallerMembersJudgeable,
53+
fieldReadUnjudgeableError,
5354
type QueryableFieldsProvider,
5455
type NamedField,
5556
type NamedRead,
@@ -1833,6 +1834,23 @@ export class AnalyticsService implements IAnalyticsService {
18331834
* caller-supplied. A dataset's own filter is author text and is deliberately
18341835
* not judged here (no `datasetScope` is passed); #21153's gate judges it
18351836
* where it applies.
1837+
*
1838+
* ## The measure position
1839+
*
1840+
* {@link namedQueryFields} skips a measure the cube does not declare (both
1841+
* strategies refuse one it does not carry), so a caller-named measure that
1842+
* {@link inferCubeFromQuery} MINTS — its `sql` built from the caller's own
1843+
* text by {@link inferMeasure} — is not in that list. Judged on the inferred
1844+
* cube (the judged tier), the minted measure reads as a declared expression
1845+
* member and #21153 refuses it; in the ungated tiers nothing did, and the
1846+
* caller's text reached the aggregate position of the statement verbatim.
1847+
* So the inferred `sql` is judged here too, against the same rule, before a
1848+
* strategy compiles it: a caller-named measure must reduce — after
1849+
* {@link inferMeasure}'s suffix strip, the no-suffix default included — to
1850+
* `count` / `'*'` or to a column reference, or it is refused. Author-declared
1851+
* measures are skipped (handled where #21153 / the parse #20943 judge them),
1852+
* and a dotted non-qualifier measure has already been refused by #5918 in
1853+
* {@link ensureCube} ahead of this.
18361854
*/
18371855
private assertCallerMembersResolvable(
18381856
query: AnalyticsQuery,
@@ -1845,6 +1863,34 @@ export class AnalyticsService implements IAnalyticsService {
18451863
authorCube ?? ({ name, title: name, sql: name, measures: {}, dimensions: {} } as Cube);
18461864
const named = namedQueryFields(query, probeCube, undefined, this.hopReference);
18471865
assertCallerMembersJudgeable(named, this.logger, context);
1866+
1867+
// [#21156] The measure position, which `namedQueryFields` does not carry.
1868+
const baseObject = typeof probeCube.sql === 'string' ? probeCube.sql.trim() : '';
1869+
if (!baseObject || !BARE_IDENTIFIER.test(baseObject)) return;
1870+
for (const measure of query.measures ?? []) {
1871+
if (typeof measure !== 'string' || measure === '') continue;
1872+
// Author-declared measures are not this gate's business.
1873+
if (authorCube && declaredMemberEntry(authorCube, measure, 'measure')) continue;
1874+
const inferredSql = inferredCallerMeasureSql(measure, name);
1875+
// `null` — a dotted non-qualifier measure #5918 already refused — and a
1876+
// column reference (`'*'`, a bare identifier, a relationship path) both
1877+
// pass; anything else is caller text reaching the aggregate unjudged.
1878+
if (
1879+
inferredSql === null ||
1880+
inferredSql === '*' ||
1881+
BARE_IDENTIFIER.test(inferredSql) ||
1882+
IDENTIFIER_PATH.test(inferredSql)
1883+
) {
1884+
continue;
1885+
}
1886+
this.logger.warn(
1887+
`[Analytics] field-level read admission refused caller-named measure "${measure}" on ` +
1888+
`"${baseObject}" (user ${String((context as { userId?: unknown } | undefined)?.userId ?? 'unknown')}) — ` +
1889+
`it reduces to a source that is not a column reference, so no field it aggregates can be ` +
1890+
`judged, in any tier (fail-closed)`,
1891+
);
1892+
throw fieldReadUnjudgeableError(baseObject, measure);
1893+
}
18481894
}
18491895

18501896
private async assertFieldsReadable(
@@ -3756,6 +3802,25 @@ export function inferMeasure(key: string): { label: string; type: 'count' | 'sum
37563802
return { label: key, type: 'sum', sql: key };
37573803
}
37583804

3805+
/**
3806+
* [#21156] The `sql` {@link inferMeasure} would mint for a caller-named measure
3807+
* — the text that reaches the aggregate position — or `null` for a dotted
3808+
* non-qualifier measure, which {@link mintableMeasureKey} / #5918 refuses in
3809+
* {@link AnalyticsService.ensureCube} ahead of the member-shape gate. Mirrors
3810+
* `mintableMeasureKey`'s qualifier strip WITHOUT its throw (that refusal has
3811+
* already happened), then reads {@link inferMeasure}'s source. The gate judges
3812+
* whether the result is a column reference.
3813+
*/
3814+
function inferredCallerMeasureSql(measure: string, cubeName: string): string | null {
3815+
let key = measure;
3816+
const dot = measure.indexOf('.');
3817+
if (dot >= 0) {
3818+
if (measure.slice(0, dot) === cubeName) key = measure.slice(dot + 1);
3819+
else return null;
3820+
}
3821+
return inferMeasure(key).sql;
3822+
}
3823+
37593824
/**
37603825
* FallbackDelegateStrategy — Internal strategy for fallback service delegation.
37613826
*

0 commit comments

Comments
 (0)