Skip to content

Commit ce4e205

Browse files
fix(service-analytics)!: refuse a caller-supplied analytics member that is not a column reference at the door (#21190)
Fixes #21177 Clause-②: no (narrowing) ## What The analytics doors evaluated **caller-supplied content** at query time that neither the object-level nor the field-level read admission could judge. After merging `main`, this PR is narrowed to the residual that `main` does not yet judge. ### The `/analytics/query` half is superseded `main` landed #21156 (as #21173): a caller-named member that is neither a declared cube member nor a column reference is refused at the `/analytics/query` door in every tier, through the single field-read judge (`PERMISSION_DENIED` / 403 — "one judge, one shape, no new error code"). This PR therefore: - drops its own duplicate handling of that class, and - reverts its edits to `field-read-admission-gate.test.ts` back to #20965's pin as `main` has it. ### The residual: a dataset's own `field` text The remaining gap is content `main` does not judge: the dimension and measure `field` text of a dataset sent to `POST /api/v1/analytics/dataset/query`. The service compiles that dataset into a cube whose members read as **declared**, so a dimension's or measure's `field` that is a raw expression resolves to a declared cube member — and #21156 leaves a declared expression member to the field-level gate, which stands down with no security service and on an object its reader answers `undefined` for. In those tiers the expression reached the native statement as written. This PR judges the dataset's own `field` text at the dataset door, **before the dataset is compiled and before any strategy runs**, through `main`'s existing judge (`assertCallerMembersJudgeable` / `fieldReadUnjudgeableError`): `PERMISSION_DENIED` / 403, naming the member and never the expression text, for **every caller** (admin included) and whether or not a security service is wired. No new error code and no second admission module — `caller-content-admission.ts` is removed. What is left to the existing gates, unchanged: - The dataset's own `filter`, the selection's runtime filter and cube-query members are lowered into the compiled query and already judged on the query path by #21156. - A dataset registered through the configuration door and queried by cube name runs through `query()`, where #21156 leaves its members to the existing gates. **Inline and saved alike.** The route hands this door an inline dataset (`body.dataset`) and a saved one alike: it loads `body.datasetName` from metadata and calls the same `queryDataset`, and the build probe calls it with a saved dataset too. The service cannot tell the branches apart, so the refusal is uniform, which is the safer reading. A saved dataset whose `field` is an expression is refused the same way as an inline one. No shipped dataset carries a non-column `field` (`examples/app-crm`, `examples/app-showcase`, `examples/app-todo` and `platform-objects` were read; the showcase `done_rate` is a derived measure, not a field expression). Refusing such a `field` when it is authored belongs to the dataset schema's own retirement of expression fields, a separate change. This aligns the caller-supplied dataset surface with ADR-0021's "zero raw expressions". ## What stays answerable Every dataset (inline or saved) and cube query that names columns, relationship paths and declared members — the whole legitimate author and query surface — is unchanged. A plain-column inline dataset, a saved plain-column dataset and a dataset registered through the configuration door are all still answered. Only a dataset `field` whose text is a raw expression is refused, inline or saved; the BREAKING note in the changeset covers both. ## Scope / serial - The fix is at the **analytics door** (`analytics-service.ts`), never in a strategy. `read-scope-sql.ts`, `strategies/native-sql-strategy.ts` and `contains-membership-sql.ts` (#21117 / #20987) are untouched. - `origin/main` was merged in (no rebase, no stacking). The residual gate sits beside #21120's stored-metadata-body refusal and #21156's caller-member gate, on the same unconditional seam. ## Tests - `inline-dataset-field-admission-door.test.ts` — the residual, on the dataset door × both strategies × four provider tiers (no provider, admin-unrestricted, non-admin field list, reader answers `undefined` for the object): refused `403 PERMISSION_DENIED`, the strategy/driver never called; a plain-column inline dataset and a dataset registered through the configuration door still answered. - `field-read-admission-gate.test.ts` — reverted to `main` (#20965's pin stays). - `packages/rest/src/analytics-16019-driver-declared-fault.test.ts` — its first block drove a driver fault through exactly the inline-dataset `field` expression path this change closes, so it now pins the door refusal (`403 PERMISSION_DENIED`) plus a positive control. It also pins the route's saved branch end to end: a saved dataset reached through `body.datasetName` whose `field` is not a column reference is refused `403 PERMISSION_DENIED` with the driver never called, and a saved plain-column dataset is answered `200`. The #16019 relay stays covered by its second block and by `driver-sql`'s own `sql-driver-16019-raw-statement-fault-envelope.test.ts`. - Ablation: disabling the new seam on disk turns the residual pins red (the three tiers the field gate stands down in, and the saved-branch pin, which is then served `200`) while the controls stay green; restored. The full `@objectstack/service-analytics` suite is green, as is the non-SQL temporal step under a skewed process zone. ## Review Security boundary. The maintainer has authorized landing; the first merge-queue build failed on the semantic overlap with #21156 described above, and this push carries the narrowing. Its draft/ready state is unchanged by this push, and it is not queued or armed for auto-merge here — the owning seat re-enqueues once CI is green on this head. ## Thread receipts (round 5) - **Docs Drift Check** (`5934340689`): computed on the previous head `5428f7e627`; both release pages it lists were reached through a string literal in `assertQueryMembersJudgeable`, which this push deletes. Release pages are read-only here and none is edited. - **Landing note** (`5934790041`): the maintainer's landing authorization on #21177 is recorded there. The ready state and auto-merge it describes belong to the owning seat; this push does not change either. - **Merge-queue triage** (`5935112816`, run `36885164397`): triaged as case 1 — every failing test is in `@objectstack/service-analytics`, which this PR changes, and each is the semantic overlap with #21156 (an expected `INVALID_FIELD` / 400 met `main`'s `PERMISSION_DENIED` / 403). This push resolves both failing files: `caller-content-admission-door.test.ts` is removed with the duplicate `/analytics/query` handling, and `field-read-admission-gate.test.ts` is back to `main`'s pin. The full `@objectstack/service-analytics` suite and the non-SQL temporal step (the two failing jobs' steps) pass locally on this head. <sub>read 2026-10-01T17:23Z.</sub> --- _Generated by [Claude Code](https://claude.ai/code/session_01MRdbfpy4sQT8bUjmMhxsN7)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent a29a0ea commit ce4e205

4 files changed

Lines changed: 339 additions & 68 deletions

File tree

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
'@objectstack/service-analytics': minor
3+
---
4+
5+
fix(service-analytics)!: a dataset's own `field` text that is not a column reference is refused at the analytics dataset door, inline or saved
6+
7+
Clause-②: no (narrowing)
8+
9+
<!-- adr-0087: not-required (no-migration-prescription) a dataset's own dimension or measure `field` text that is not a column reference (a field, a relationship path ending in one, or `*`) is refused at the analytics dataset door — whichever branch supplied the dataset, an inline `body.dataset` or a saved `body.datasetName` — before the dataset is compiled and before any strategy runs, for every caller and whether or not a security service is wired, through the field-read gate's existing judge and envelope (`PERMISSION_DENIED` / 403). No authorable key, spelling, export or stored shape moves, and no stored row is read differently by any metadata consumer; the published surface gains and loses nothing. The other categories are closed on facts: the package publishes (not `unpublished`); no ADR-0087 id covers a dataset `field` (not `registered` / `already-registered`); and the change is runtime behaviour, not a declaration (not `runtime-interface-only` / `type-surface-only`). A dataset names a column instead of writing an expression, which is ADR-0021's author surface already ("zero raw expressions"), so there is no FROM → TO mapping to carry. -->
10+
11+
**BREAKING**: this narrows what the analytics dataset door accepts. A dataset whose dimension or measure `field` is not a column reference is now refused with `403 PERMISSION_DENIED` instead of being evaluated — an inline dataset and a saved dataset queried by name alike, since both reach the same door. No shipped dataset carries a non-column `field`. It ships as `minor` under the launch-window convention for accept-set narrowings. No export or published type changes.
12+
13+
**What changes.** The service compiles a dataset into a cube whose members read as declared, so a dimension or measure whose `field` was a raw expression resolved to a declared cube member and was left to the field-level read gate, which stands down with no security service and on an object its reader answers `undefined` for; in those tiers the expression reached the native statement as written. The dataset's own `field` text is now judged at the dataset door, before compile and before any strategy runs, through the field-read gate's existing judge (`PERMISSION_DENIED` / 403, naming the member and never the expression text), for every caller, admin included, and with or without a security service. There is no new error code and no new admission module.
14+
15+
**What stays answerable.** Every dataset whose fields are columns or relationship paths is unchanged, inline or saved. A saved dataset whose `field` is an expression is refused the same way as an inline one; refusing such a `field` when it is authored belongs to the dataset schema's own retirement of expression fields, not to this door. The dataset's own filter, the selection's runtime filter and cube-query members are lowered into the compiled query and already judged on the query path, so they are unchanged.

‎packages/rest/src/analytics-16019-driver-declared-fault.test.ts‎

Lines changed: 96 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -5,29 +5,31 @@
55
* reaches the caller by DECLARATION, and the declaration wins over the
66
* phrasing heuristic.
77
*
8-
* ## The two shapes, and why this file drives the real driver
8+
* ## The ruling, and where each half is proved
99
*
10-
* The card was filed on a hand-made `Error('no such function: translate')` and
11-
* re-scoped when the Clause-② review of PR #16020 measured the shape production
12-
* actually raises — knex's `<statement> - no such function: translate` — and
13-
* found it withheld already, by accident: `looksLikeInternalErrorLeak` fires on
14-
* the `select ` prefix, never on the phrase. On `origin/main`:
10+
* Under the 2026-09-06 ruling (decision batch #57, option 3) `SqlDriver.execute()`
11+
* declares the fault — `DATABASE_ERROR`/500, the dialect error under a
12+
* non-enumerable `cause` — and this door's ③a arm relays a declared 5xx with the
13+
* producer's code and `INTERNAL_ERROR_MESSAGE`. The two halves are proved
14+
* separately: `driver-sql`'s `sql-driver-16019-raw-statement-fault-envelope.test.ts`
15+
* proves `SqlDriver.execute` declares the fault, and the SECOND block here proves
16+
* this route relays a declared `DATABASE_ERROR`/500 with the prose withheld (a
17+
* throwing double hands the door the exact declared shape).
1518
*
16-
* | shape reaching the door | body |
17-
* |-----------------------------------------------|-------------------------------------------------------|
18-
* | bare `Error('no such function: translate')` | `500 ANALYTICS_QUERY_FAILED`, raw engine text |
19-
* | knex-shaped — what `SqlDriver.execute` raised | `500 ANALYTICS_QUERY_FAILED`, `Internal server error` |
20-
*
21-
* Both were UNDECLARED. Under the 2026-09-06 ruling (decision batch #57,
22-
* option 3) `SqlDriver.execute()` declares the fault — `DATABASE_ERROR`/500,
23-
* the dialect error under a non-enumerable `cause` — and this door's ③a arm
24-
* relays a declared 5xx with the producer's code and `INTERNAL_ERROR_MESSAGE`.
25-
* The first block drives that END TO END: a real `AnalyticsService` on the
26-
* native-SQL strategy, a real better-sqlite3 `SqlDriver` behind the exact
27-
* bridge `service-analytics`'s plugin wires (`engine.execute` → `driver.execute`),
28-
* and a dataset dimension whose expression calls `translate()` — the function
29-
* SQLite lacks, the #16028 fault verbatim — so the refusal is the ENGINE's,
30-
* not a fixture's, and the compiled statement is the real one.
19+
* [#21177] This file once drove the two halves END TO END through one call — a
20+
* real `AnalyticsService` on the native-SQL strategy over a real better-sqlite3
21+
* `SqlDriver`, fed a dataset dimension whose expression called `translate()`, the
22+
* function SQLite lacks. That path is gone: #21177 refuses a caller-supplied
23+
* inline-dataset dimension/measure `field` that is not a column reference at the
24+
* analytics door, before any strategy or driver runs, so a raw `translate(...)`
25+
* can no longer reach the engine from caller content. The FIRST block now pins
26+
* THAT — the door refuses the expression `PERMISSION_DENIED` / 403, the one
27+
* judge #21156 reaches (no new error code) — beside a positive control that a
28+
* legitimate dataset on declared fields is still served 200 by the real driver.
29+
* It pins the route's SAVED branch (`body.datasetName`) the same way: that branch
30+
* loads the dataset from metadata and calls the same `queryDataset`, so a saved
31+
* dataset whose `field` is not a column reference is refused too, and a saved
32+
* plain-column dataset is still served.
3133
*
3234
* The second block pins the ordering the ruling's execution notes name. A
3335
* DECLARED fault is withheld even when its text is one the heuristic does not
@@ -44,19 +46,14 @@
4446
*
4547
* ## Reverse verification, direction predicted BEFORE running
4648
*
47-
* Restore the bare `await builder` in `SqlDriver.execute()` and REBUILD
48-
* driver-sql: the first block's `DATABASE_ERROR` assertions go RED
49-
* (`ANALYTICS_QUERY_FAILED` returns) while its `INTERNAL_ERROR_MESSAGE`
50-
* assertion stays GREEN — the accident the re-scope measured, the heuristic's
51-
* `select ` limb — and the driver-log assertion goes RED (the driver no longer
52-
* logs; the route's `logError` becomes the only copy). The second block stays
53-
* GREEN throughout: it hands the door shapes that never touch the driver.
54-
*
5549
* The ORDERING pin in block 2 has its own leg: gate the door's ③a relay
5650
* behind `looksLikeInternalErrorLeak` being false (i.e. consult the heuristic
5751
* first) and only that case goes RED (`ANALYTICS_QUERY_FAILED` in place of the
5852
* producer's code); the neighbouring "phrase the heuristic does not know"
5953
* case stays GREEN, which is precisely why it could not stand in for this one.
54+
* The first block's leg: remove the caller-content gate and the expression
55+
* reaches the real driver again — the refusal flips from `403 PERMISSION_DENIED`
56+
* to the `500 DATABASE_ERROR` relay this file once asserted.
6057
*/
6158

6259
import { describe, it, expect, vi, beforeEach, afterEach, beforeAll, afterAll } from 'vitest';
@@ -82,11 +79,12 @@ function mockServer() {
8279
use: vi.fn(), listen: vi.fn().mockResolvedValue(undefined), close: vi.fn().mockResolvedValue(undefined),
8380
};
8481
}
85-
function mockProtocol() {
82+
/** `savedDatasets` is what the route's `body.datasetName` branch loads from metadata. */
83+
function mockProtocol(savedDatasets: unknown[] = []) {
8684
return {
8785
getDiscovery: vi.fn().mockResolvedValue({ version: 'v0', routes: { data: '', metadata: '' } }),
8886
getMetaTypes: vi.fn().mockResolvedValue([]),
89-
getMetaItems: vi.fn().mockResolvedValue([]),
87+
getMetaItems: vi.fn().mockResolvedValue(savedDatasets),
9088
};
9189
}
9290
function mockRes() {
@@ -97,9 +95,9 @@ function mockRes() {
9795
return res;
9896
}
9997

100-
function buildRoute(analyticsProvider?: any) {
98+
function buildRoute(analyticsProvider?: any, savedDatasets: unknown[] = []) {
10199
const rest = new RestServer(
102-
mockServer() as any, mockProtocol() as any, { api: { requireAuth: false } } as any,
100+
mockServer() as any, mockProtocol(savedDatasets) as any, { api: { requireAuth: false } } as any,
103101
undefined, undefined, undefined, undefined, undefined, undefined, undefined,
104102
undefined, undefined, undefined, undefined,
105103
analyticsProvider,
@@ -123,23 +121,39 @@ async function post(route: any, body: unknown) {
123121
const ACCOUNT_FIELDS = ['id', 'name', 'industry'];
124122

125123
/**
126-
* One plain dimension for the control, and one whose expression calls the
127-
* function SQLite lacks. The compiler passes an expression through verbatim
128-
* (`qualifyAndRegisterJoin` leaves anything that is not a bare identifier or
129-
* a dotted path alone) and the source-field gate judges bare identifiers only,
130-
* so this is the real statement the strategy emits, refused by the real engine.
124+
* A legitimate dataset on declared fields — the positive control, and the body
125+
* block 2 posts to the throwing double (whose `queryDataset` is mocked, so the
126+
* definition never reaches the real door).
131127
*/
132128
const dataset = {
133129
name: 'account_metrics',
134130
label: 'Account metrics',
135131
object: 'crm_account',
136132
dimensions: [
137133
{ name: 'industry', field: 'industry', type: 'string' },
138-
{ name: 'folded_name', field: "translate(name, 'ABC', 'abc')", type: 'string' },
139134
],
140135
measures: [{ name: 'account_count', aggregate: 'count' }],
141136
};
142137

138+
/**
139+
* [#21177] A caller-supplied inline dataset whose dimension `field` is a raw
140+
* expression. Until #21177 the compiler passed it through verbatim and the real
141+
* engine ran it (the #16028 `translate()` fault this file once drove end to end);
142+
* now the caller-content gate refuses it at the door, before any strategy or
143+
* driver runs. The real-driver declaration and the door relay it once proved
144+
* jointly are each covered on their own: `driver-sql`'s
145+
* `sql-driver-16019-raw-statement-fault-envelope.test.ts` proves `SqlDriver`
146+
* declares the fault, and block 2 below proves this route relays a declared
147+
* `DATABASE_ERROR` / 500 with the prose withheld.
148+
*/
149+
const expressionDataset = {
150+
...dataset,
151+
dimensions: [
152+
{ name: 'industry', field: 'industry', type: 'string' },
153+
{ name: 'folded_name', field: "translate(name, 'ABC', 'abc')", type: 'string' },
154+
],
155+
};
156+
143157
async function realDriver(): Promise<SqlDriver> {
144158
const driver = new SqlDriver({
145159
client: 'better-sqlite3',
@@ -208,48 +222,62 @@ describe('[#16019] a driver fault on the raw-SQL path reaches the caller by decl
208222
await driver.disconnect();
209223
});
210224

211-
it('the statement SQLite refuses answers 500 DATABASE_ERROR with the prose withheld — the ③a relay, not ③b', async () => {
225+
it('[#21177] a caller-supplied dimension-field expression is refused 403 PERMISSION_DENIED at the door — before any strategy or driver runs', async () => {
212226
const route = buildRoute(async () => realAnalytics(driver));
213-
const res = await post(route, { dataset, selection: { measures: ['account_count'], dimensions: ['folded_name'] } });
214-
215-
expect(res.statusCode).toBe(500);
216-
// The producer's code, relayed — where an undeclared fault answered the
217-
// route's generic one.
218-
expect(res.body.code).toBe('DATABASE_ERROR');
219-
expect(res.body.code).not.toBe('ANALYTICS_QUERY_FAILED');
220-
expect(res.body.error).toBe(INTERNAL_ERROR_MESSAGE);
227+
const res = await post(route, { dataset: expressionDataset, selection: { measures: ['account_count'], dimensions: ['folded_name'] } });
221228

229+
expect(res.statusCode).toBe(403);
230+
expect(res.body.code).toBe('PERMISSION_DENIED');
231+
// Caller text that names no attributable field is refused at the door,
232+
// not evaluated — the driver never ran, so there is no driver fault line.
233+
expect(warned.filter((m) => m.includes('[sql-driver] DATABASE_ERROR'))).toHaveLength(0);
234+
// ⛔ The refusal names the member and its object (both the caller's own
235+
// input), never the caller's `field` expression text or the compiled statement.
222236
const body = JSON.stringify(res.body);
223237
expect(body).not.toMatch(/translate/i);
224-
expect(body).not.toMatch(/no such function/i);
225-
expect(body).not.toMatch(/SELECT|GROUP BY|crm_account/i);
238+
expect(body).not.toMatch(/SELECT|GROUP BY/i);
226239
});
227240

228-
it('the operator keeps the whole diagnostic: the driver logged the statement and the engine text, the route logged the envelope', async () => {
229-
const route = buildRoute(async () => realAnalytics(driver));
230-
await post(route, { dataset, selection: { measures: ['account_count'], dimensions: ['folded_name'] } });
231-
232-
// The driver's warn line is now the only copy of the dialect text.
233-
const driverLine = warned.find((m) => m.includes('[sql-driver] DATABASE_ERROR'));
234-
expect(driverLine).toBeDefined();
235-
expect(driverLine).toContain('no such function: translate');
236-
expect(driverLine).toMatch(/translate\(name, 'ABC', 'abc'\)/i);
237-
// The route still logs the fault it relayed — the composed envelope, which
238-
// names no statement (`logError` prints `error.message`).
239-
const routeLine = errored.find((m) => m.includes('[REST] Analytics dataset query error'));
240-
expect(routeLine).toBeDefined();
241-
expect(routeLine).toContain('refused to run a raw statement');
242-
expect(routeLine).not.toContain('no such function');
243-
});
244-
245-
it('POSITIVE CONTROL: the same wiring with the plain dimension → 200 with rows', async () => {
241+
it('POSITIVE CONTROL: a legitimate dataset on declared fields → 200 with rows', async () => {
246242
const route = buildRoute(async () => realAnalytics(driver));
247243
const res = await post(route, { dataset, selection: { measures: ['account_count'], dimensions: ['industry'] } });
248244

249245
expect(res.statusCode).toBe(200);
250246
expect(res.body.rows).toEqual([{ industry: 'tech', account_count: 1 }]);
251247
expect(warned.filter((m) => m.includes('[sql-driver] DATABASE_ERROR'))).toHaveLength(0);
252248
});
249+
250+
// [#21177] The route's SAVED branch: `body.datasetName` loads the dataset from
251+
// metadata and calls the same `queryDataset`, so the door judges a saved
252+
// dataset's own `field` text exactly as it judges an inline one. The expression
253+
// here is one SQLite can run, so without the door it would be served (200).
254+
it('[#21177] a SAVED dataset (body.datasetName) whose dimension field is not a column reference is refused 403 PERMISSION_DENIED — nothing executed', async () => {
255+
const saved = {
256+
...dataset,
257+
name: 'account_metrics_saved_expr',
258+
dimensions: [{ name: 'lowered_name', field: 'lower(name)', type: 'string' }],
259+
};
260+
const execute = vi.spyOn(driver, 'execute');
261+
const route = buildRoute(async () => realAnalytics(driver), [saved]);
262+
const res = await post(route, { datasetName: saved.name, selection: { measures: ['account_count'], dimensions: ['lowered_name'] } });
263+
264+
expect(res.statusCode).toBe(403);
265+
expect(res.body.code).toBe('PERMISSION_DENIED');
266+
expect(execute).not.toHaveBeenCalled();
267+
const body = JSON.stringify(res.body);
268+
expect(body).toContain('lowered_name');
269+
expect(body).not.toMatch(/lower\(name\)/i);
270+
});
271+
272+
it('[#21177] CONTROL: a SAVED plain-column dataset (body.datasetName) → 200 with rows', async () => {
273+
const execute = vi.spyOn(driver, 'execute');
274+
const route = buildRoute(async () => realAnalytics(driver), [dataset]);
275+
const res = await post(route, { datasetName: dataset.name, selection: { measures: ['account_count'], dimensions: ['industry'] } });
276+
277+
expect(res.statusCode).toBe(200);
278+
expect(res.body.rows).toEqual([{ industry: 'tech', account_count: 1 }]);
279+
expect(execute).toHaveBeenCalled();
280+
});
253281
});
254282

255283
describe('[#16019] at the door: a declaration wins over the heuristic, and the heuristic stays as the fallback', () => {

0 commit comments

Comments
 (0)