Skip to content

Commit cb45469

Browse files
fix(service-analytics)!: the analytics native-SQL path declines an object an engine middleware is registered for, so its read gates apply (#21170)
Fixes #21080 Clause-②: yes (narrowing) `yes`: one optional member widens a published contract interface (`IObjectQLEngine.hasObjectMiddleware?`), and `ObjectQL` and `AnalyticsServiceConfig` each gain one additive member. `(narrowing)`: a query the native path served is now refused when it reads a gated object and the ObjectQL strategy cannot serve it (measured below). The changeset carries the BREAKING banner and the ADR-0087 marker. ## What changed Triage's ruling (`5925681388`) is implemented as written. There is no per-object list in `service-analytics`, and no gate registers twice. The middleware chain, `registerMiddleware` and `executeWithMiddleware` are unchanged. - **`@objectstack/objectql`.** `ObjectQL.hasObjectMiddleware(objectName)` sits beside `registerMiddleware`. It reads what `registerMiddleware` already records: `true` when a registration's `object` names the object. A global registration (no `object`, or `'*'`) is keyed to no object and is not counted. It runs and registers nothing. - **`@objectstack/spec`.** The matching optional member on `IObjectQLEngine`, as a declaration and docblock only. - **`@objectstack/service-analytics`.** - One context hook, `DatasetScopedStrategyContext.hasObjectMiddleware` in `strategies/types.ts`. - `AnalyticsService` passes it through from a new `AnalyticsServiceConfig.hasObjectMiddleware` at the `baseCtx` construction site. The plugin wires that config member from the data engine (`DataEngineLike` gains the member). - `NativeSQLStrategy.canHandle` declines a query that reads such an object. The objects it checks are the set the door admitted and scoped (`readScopedObjects`: the base object, declared joins and relationship-path objects). For a context built without that set, it checks the cube's own objects. The declined query routes to the ObjectQL strategy, which hands it to the engine with the caller's context, so the engine's middlewares run. - **Fail closed.** If the engine lacks the member, or no engine is registered, the plugin's answer is "cannot say", and the strategy declines then too. The plugin says this once at `warn`. The cost is the native fast path for every object on such a host: its queries are served by the ObjectQL strategy, and a query that strategy cannot serve is refused. A host that builds `AnalyticsService` itself with `executeRawSql` and without the new config member keeps today's native path for every object. It is told so once, at construction. - **Surface notes, declared.** - The `AnalyticsServiceConfig` member sits outside the "context construction sites" region of `analytics-service.ts` that the claim names. The service cannot learn the engine's answer any other way, and it follows the same pattern as `judgeFilter`. - Six plugin-level suites' engine doubles now carry `hasObjectMiddleware: () => false`, which models the engine they stand in for. Without it they measured the fail-closed tier: 22 tests went from served to refused. The suites are `admission-bridge-resolution`, `effective-datasource-probe`, `field-query-admission-gate`, `field-read-admission-gate`, `raw-sql-object-routing` and `read-scope-bridge-resolution`. ## Per gate, by class The boot is real and org-bound: better-auth sign-ups, the real security plugin, and the real audit, storage and approvals plugins writing their rows, on SQLite. The restricted member is admitted to the gated object at object level and cannot read some of the parents. The admin is the unrestricted control. The door is the analytics dataset door. The strategy was read by counting each strategy's `execute`. The reference is the generic data door's list for the same caller. | gated object (gate) | strategy before → after | restricted member, before | restricted member, after | data door, same member | |:--|:--|:--|:--|:--| | comment threads (`plugin-audit` comment read gate) | native → ObjectQL | groups and count include a thread about a parent it cannot read | equal to the data door | only threads about readable parents | | activity rows (`plugin-audit` activity read gate) | native → ObjectQL | groups include a parent it cannot read; the count is the system total | equal to the data door | only rows about readable parents | | attachments (`service-storage` attachment read gate) | native → ObjectQL | groups include an unreadable parent; the count is the system total | equal to the data door | only rows on readable parents | | approval requests (`plugin-approvals` snapshot redaction) | native → ObjectQL | a group key carries a snapshot field the data plane masks for this member | unchanged: still carried | list: redacted; grouped query: carried (out-of-scope finding below) | - **Admin control.** For comment threads and attachments, the admin's analytics answer already equalled the data door before the change and still does. For activity rows, the admin's analytics answer before also carried rows that the gate excludes for every caller: the rows about a record that no longer exists, or that name none. After the change it equals the data door. - **Control object.** An object no middleware names (`cmt_open`) is served by the native strategy before and after. ## Measured first (H1 to H4) - **H1** is reproduced on current `main` as the card describes, for comment threads and activity rows. The table above has both readings. - **H2, attachments.** The attachment read gate is object-keyed and served nothing on this path before the change. It is covered by construction, and the row above has the reading. - **H2, approvals.** - The snapshot redaction is not global. It has been object-keyed since it landed, `{ object: 'sys_approval_request' }`, so the engine's answer covers its routing by construction. Its registration is unchanged and not in this PR. - It served something on the native path: the masked snapshot value in a group key. - After the decline the engine path serves the query. The redaction runs on `find` and `findOne` only, so an aggregate still carries the value, as the generic data door's grouped query does today. That gap is in the redaction's operation set, and it is a separate finding (below). It is not this card's mechanism. - **H3, multi-organization.** A boot with the organizations plugin under the `isolated` posture and a declared membership policy. A caller outside the admin's organization gets the same answer from the native analytics path as from the data door (none of the admin organization's rows), and the admin gets the same answer from both. The organization wall reaches this path through the security service's read filter (Layer 0 in `getReadFilter`), so the native path applies it. NOT MEASURED: a member of a second organization that holds rows of its own (the outsider in this boot was bound to no organization). - **H4.** The route is the three parts described above. The fail-closed tier and its cost are stated above and pinned below. ## Census (H5) The census was read off the engine's registrations on a showcase boot with the stock plugin set (`serve` auto-registers audit, storage, sharing and approvals, beside security): - **Read gates (4 objects):** `sys_comment` and `sys_activity` (`plugin-audit`; `sys_activity` also carries the field-value redaction), `sys_attachment` (`service-storage`), and `sys_approval_request` (`plugin-approvals`, the snapshot redaction). - **Write-only middlewares (2 objects), moved as a side effect:** `sys_user_position` (the position-catalog refusal, on insert and update) and `sys_permission_set` (the data-door write-through, on insert, update, delete and restore). A middleware does not declare its operation, so these leave the native path too. - **Global registrations** (8 on that boot) are keyed to no object and move nothing. - **Datasets and dashboards that leave the native path: none.** - The showcase datasets read `showcase_task`, `showcase_project`, `showcase_invoice` and `showcase_account`. - The platform system dashboards' datasets read `sys_user`, `sys_organization`, `sys_session`, `sys_package_installation` and `sys_audit_log`. - None of these is in the set above, and none joins one. ## What is newly refused (the narrowing) This is a narrowing, measured. The ObjectQL strategy refuses a dimension reached through a relationship path combined with a measure that cannot be recombined across it (`avg`, `count_distinct`), with `INVALID_FIELD` / `400`. The native strategy served that query. - When such a query reads a gated object, it is now refused. - On a host whose engine cannot say, the same query is refused for every object. - The same query on an ungated object is still served natively. Correctness wins over speed for a gated object, per the ruling. ## Pins (committed red, before the fix) The pins were committed at `5c6d445e3a`. The fix is at `bae287b92e` and the changeset at `e74617add7`. - **`packages/objectql/src/engine-has-object-middleware.test.ts`, the engine accessor.** It covers object-keyed `true`, unnamed `false`, global-only `false`, an empty engine, and read-only: the accessor runs no middleware, with a positive control on a later read. At the pin commit 4 of 4 failed. After the fix 4 of 4 pass. - **`packages/services/service-analytics/src/__tests__/engine-middleware-decline.test.ts`, the decline and the fail-closed tier.** It covers the base object, a declared join, and a relationship-path object in the door's set. It also covers a hook that cannot answer, plus two controls (an ungated object, and a context with no hook), and it runs end to end through `AnalyticsService` and through `AnalyticsServicePlugin`: an engine that names the object, and an engine without the member, which declines every object and says so once. At the pin commit 7 failed and 3 passed of 10 (the 3 controls passed). After the fix 10 of 10 pass. - **`packages/qa/dogfood/test/analytics-engine-middleware-objects.dogfood.test.ts`, the per-object answers.** For comment threads, activity rows and attachments, the restricted member's analytics groups and count equal the data door's for the same member, with the admin as the control. Against the pre-fix build 4 failed and 2 passed of 6 (the two admin controls whose answers already agreed passed). After the fix 6 of 6 pass. ## Ablation (the decline removed, from the committed state at `e74617add7`) The prediction was written down before the run. The mutation was made through `scripts/ablation-replace.mjs` (anchor 1 → 0, blob `9e382c78` → `84976adb`), and a trap restored it by absolute path. The mutation replaced the one decline call in `canHandle` with a bare reference to the helper, so the helper stays referenced and the DTS build stays clean. - **Leg A, resolved from source.** The decline pin file failed 7 and passed 3 of 10, the same 7 that were red at the pin commit. The six double-carrying suites stayed green: 101 of 108 passed across the seven files. - **Leg B, resolved from `dist/`.** `service-analytics` was rebuilt. `ablation-dist-preflight --absent` proved the decline call gone from all 6 built files. The dogfood pin failed 4 and passed 2 of 6, exactly the 4 that were red at the pin commit. - **The accessor pin**, which this ablation does not touch, passed 4 of 4. - **Restore.** `git checkout HEAD --` on the absolute path brought the blob back to `9e382c78`, equal to the HEAD blob, and `git diff HEAD` on the path was empty. After a rebuild, the preflight in default mode found the decline call present in 2 built files. The decline pin passed 10 of 10 and the dogfood pin 6 of 6. The tree was clean. ## Verification All of it ran at HEAD `82c4552cbf` (this branch with `main` merged at `bafb8c9498`), as ONE locked script. Each exit code was captured before any pipe. - **Gate families.** `dispatch-gates --commands` derived 90 families from this diff. 89 exited 0. `check:dual-build-cjs-loads` is NOT MEASURED: it refused on its own PREREQUISITE NOT MET (exit 3), because it needs a whole-repo build, which CI runs. `dispatch-gates --ran` reports 90 accounted for: 89 run, 1 NOT MEASURED, 0 unrun. - **Roster families.** The six roster families the derivation marked for these paths all exited 0: `check-changeset-fixed`, spec `check:meta-url-spelling`, spec `check:spec-changes`, `check:authz-resolver`, `check:error-code-casing` and `check:filter-alias-parity`. Spec `check:generated` also exited 0, so every generated artifact is current. - **Typecheck, per touched package:** `@objectstack/spec`, `@objectstack/objectql`, `@objectstack/service-analytics` and `@objectstack/dogfood` all exited 0. - **Tests, per touched package, every vitest project:** | package | project | files | tests | |:--|:--|:--|:--| | `@objectstack/objectql` | `local` | 359 | 7,055 passed | | `@objectstack/objectql` | `repo` | 1 | 5 passed | | `@objectstack/spec` | `local` | 594 | 17,399 passed, 1 todo | | `@objectstack/spec` | `repo` | 47 | 832 passed | | `@objectstack/service-analytics` | (one project) | 158 | 3,614 passed, 21 skipped | - **Dogfood, the analytics pins and the gate pins this change routes:** 12 files, 94 tests, all passed. They are the new pin, the eight analytics dogfood files, the comment matrix, the activity gate pin, the attachment count-parity pin and the approval snapshot pin. - **Lint, a declared narrowing.** `eslint --no-inline-config --format json` over the 15 changed TypeScript files linted 15 files, with 0 errors and 0 warnings. `eslint.config.mjs` enables no type-aware linting, so this diff cannot move a verdict on an untouched file. The whole-repo `pnpm lint` runs in CI. ## Acceptance notes - **Out-of-scope finding, reported to the seat and not fixed here:** - The approval snapshot redaction and the activity field-value redaction run on `find` and `findOne` only. - An `aggregate` that groups by the snapshot column of `sys_approval_request` carries a field value the data plane masks for that caller. This was measured on the generic data door's grouped query, and on the analytics door before and after this change. - The activity field-value redaction shares the mechanism. NOT MEASURED. - NOT MEASURED: PostgreSQL. Every reading above is on SQLite. - NOT MEASURED, by this branch: the whole dogfood suite (CI's Dogfood Regression Gate runs it). The analytics, comment, activity, attachment and approval dogfood pins listed under Verification ran. - `README.md` in `service-analytics` shows a hand-built `AnalyticsService` without the new config member. Such a host is told at construction. The README is outside this claim's surface. - `main` moved after this branch's merge. The three newer commits touch `driver-sql`, `driver-turso`, `lint` and `service-storage`, and they share no file with this diff. PR #21144 (`engine.ts` and `analytics-service.ts`, other regions) had not landed. Whichever lands second merges `main` and re-runs its pins. --- _Generated by [Claude Code](https://claude.ai/code/session_01XY5uCwTjZj7884yYtyur4H)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0b12b9e commit cb45469

16 files changed

Lines changed: 762 additions & 2 deletions
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
---
2+
'@objectstack/spec': minor
3+
'@objectstack/objectql': minor
4+
'@objectstack/service-analytics': minor
5+
---
6+
7+
fix(service-analytics)!: the analytics native-SQL strategy declines an object an engine middleware is registered for, so the engine serves it and that object's read gates apply; the engine answers which objects carry one (`IObjectQLEngine.hasObjectMiddleware`) (#21080)
8+
9+
Clause-②: yes (narrowing)
10+
11+
<!-- adr-0087: not-required (no-migration-prescription) a correction of which analytics strategy serves a query that reads an object the data engine holds a per-object middleware for, decided at request time. No authorable key, spelling, value domain or stored metadata shape moves: every dataset, cube and dashboard parses as before, nothing stored is rewritten, and the only declaration change is ADDITIVE (one optional member on IObjectQLEngine, one public method on ObjectQL, one optional member on AnalyticsServiceConfig), so there is nothing for an author to convert and nothing for `objectstack migrate meta` to reach. The queries newly refused are refused at request time by the ObjectQL strategy's existing envelope, not by a schema. The other categories are closed on facts: the packages publish (not unpublished); no ADR-0087 id covers strategy routing and this diff adds none (not registered / already-registered); and the change is runtime behaviour plus additive declarations, not a removal from a published interface (not runtime-interface-only / type-surface-only). -->
12+
13+
**BREAKING**: this narrows what the analytics doors serve for one class of query. It ships as `minor` under the launch-window convention for narrowings.
14+
15+
**What changes.** On a SQL driver, `NativeSQLStrategy` compiled a query to SQL and ran it through the driver's raw-SQL seam, so no engine operation ran and no engine middleware did. It applied the security service's object admission and read filter and nothing else, so the read gates that live in the engine as per-object middlewares did not apply there: a caller admitted to such an object at object level read grouped results and counts over every row, rows about parent records that caller cannot read included. It now declines a query that reads (as its base object, a declared join, or through a relationship path) an object the data engine holds a middleware registered for. The ObjectQL strategy serves it through the engine with the caller's context, so the engine's middlewares run, and the analytics answer for that caller equals the data door's. On the stock composition the objects that move off the native path are `sys_comment`, `sys_activity` and `sys_attachment` (read gates), `sys_approval_request` (the snapshot redaction), and `sys_user_position` and `sys_permission_set` (write-side middlewares, which move as a side effect: a middleware does not declare its operation). No shipped dataset or dashboard reads any of them.
16+
17+
**What is newly refused.** A query on such an object that the ObjectQL strategy cannot serve is refused with that strategy's existing `400`, where the native strategy used to serve it: for example a dimension reached through a relationship path combined with a measure that cannot be recombined across it (`avg`, `count_distinct`). Correctness wins over the fast path for a gated object.
18+
19+
**It fails closed.** `AnalyticsServicePlugin` asks the data engine. An engine without `hasObjectMiddleware`, or no engine, cannot say, and the strategy declines then too: every query on such a host is served by the ObjectQL strategy, and the plugin says so once at `warn`. A host that constructs `AnalyticsService` with `executeRawSql` and without the new `hasObjectMiddleware` config member keeps the native path for every object and is told so once at construction.
20+
21+
**New, additive.** `IObjectQLEngine.hasObjectMiddleware?(objectName): boolean` (`@objectstack/spec`), `ObjectQL.hasObjectMiddleware(objectName)` (`@objectstack/objectql`): whether a `registerMiddleware(fn, { object })` names the object; a global registration (no `object`, or `'*'`) is keyed to none and is not counted. `AnalyticsServiceConfig.hasObjectMiddleware` (`@objectstack/service-analytics`), which the plugin fills from the data engine.
22+
23+
**Unchanged.** Objects no middleware names keep the native path. The middleware chain, `registerMiddleware` and every gate are unchanged.
24+
25+
**What to do after upgrading.** Nothing on the stock composition. A host whose `"data"` service is not ObjectQL should implement `hasObjectMiddleware` to keep the native path for ungated objects. A host that builds `AnalyticsService` itself with `executeRawSql` should pass `hasObjectMiddleware` from its engine.
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#21080] `ObjectQL.hasObjectMiddleware`, the engine's read-only answer to
5+
* "is a middleware registered FOR this object?" (`IObjectQLEngine.hasObjectMiddleware`).
6+
*
7+
* The analytics native-SQL strategy runs no engine operation, so no engine
8+
* middleware runs on it. It asks this accessor and declines an object that
9+
* carries one, so the engine path serves that object and its middlewares run.
10+
* What each case pins:
11+
*
12+
* - **Object-keyed.** A registration naming the object answers `true`; an
13+
* object no registration names answers `false`.
14+
* - **Global is not object-keyed.** A registration with no `object`, or with
15+
* `'*'`, matches every object at dispatch, but it is keyed to none, so it
16+
* answers `false` for every object.
17+
* - **Read-only.** Asking runs no middleware and changes nothing the dispatch
18+
* reads: on a later read the same middlewares run, once each.
19+
*/
20+
21+
import { describe, it, expect, vi, beforeEach } from 'vitest';
22+
import { ObjectQL } from './engine';
23+
import { SchemaRegistry } from './registry';
24+
25+
vi.mock('./registry', async () => {
26+
// [#10551] The one shared factory — see `registry-module-mock.ts`.
27+
const { createRegistryModuleMock } = await import('./registry-module-mock.js');
28+
return createRegistryModuleMock();
29+
});
30+
31+
const GATED = 'gated_note';
32+
const PLAIN = 'plain_note';
33+
34+
const passThrough = () => vi.fn(async (_ctx: unknown, next: () => Promise<void>) => next());
35+
36+
async function makeEngine(): Promise<ObjectQL> {
37+
vi.mocked((SchemaRegistry as any).getObject).mockImplementation((name: string) =>
38+
name === GATED || name === PLAIN ? { name, fields: { title: { type: 'text' } } } : undefined,
39+
);
40+
const driver: any = {
41+
name: 'memory',
42+
supports: {},
43+
connect: vi.fn().mockResolvedValue(undefined),
44+
disconnect: vi.fn().mockResolvedValue(undefined),
45+
find: vi.fn(async () => []),
46+
findOne: vi.fn(async () => null),
47+
count: vi.fn(async () => 0),
48+
aggregate: vi.fn(async () => []),
49+
create: vi.fn(),
50+
update: vi.fn(),
51+
delete: vi.fn(),
52+
};
53+
const ql = new ObjectQL();
54+
ql.registerDriver(driver, true);
55+
await ql.init();
56+
return ql;
57+
}
58+
59+
describe('ObjectQL.hasObjectMiddleware — the engine answers which objects carry an object-keyed middleware', () => {
60+
beforeEach(() => {
61+
vi.clearAllMocks();
62+
});
63+
64+
it('answers true for an object a registration names, and false for one none names', async () => {
65+
const ql = await makeEngine();
66+
ql.registerMiddleware(passThrough(), { object: GATED });
67+
expect(ql.hasObjectMiddleware(GATED)).toBe(true);
68+
expect(ql.hasObjectMiddleware(PLAIN)).toBe(false);
69+
});
70+
71+
it('answers false for every object when only global registrations exist', async () => {
72+
const ql = await makeEngine();
73+
ql.registerMiddleware(passThrough());
74+
ql.registerMiddleware(passThrough(), { object: '*' });
75+
expect(ql.hasObjectMiddleware(GATED)).toBe(false);
76+
expect(ql.hasObjectMiddleware('*')).toBe(false);
77+
});
78+
79+
it('answers false on an engine with no registration at all', async () => {
80+
const ql = await makeEngine();
81+
expect(ql.hasObjectMiddleware(GATED)).toBe(false);
82+
});
83+
84+
it('runs no middleware and leaves the dispatch set unchanged', async () => {
85+
const ql = await makeEngine();
86+
const keyed = passThrough();
87+
const global = passThrough();
88+
ql.registerMiddleware(keyed, { object: GATED });
89+
ql.registerMiddleware(global);
90+
expect(ql.hasObjectMiddleware(GATED)).toBe(true);
91+
expect(ql.hasObjectMiddleware(PLAIN)).toBe(false);
92+
expect(keyed).not.toHaveBeenCalled();
93+
expect(global).not.toHaveBeenCalled();
94+
// Positive control: a read through the chain reaches both, once each, and
95+
// a read of the other object reaches the global one alone.
96+
await ql.count(GATED, {});
97+
expect(keyed).toHaveBeenCalledTimes(1);
98+
expect(global).toHaveBeenCalledTimes(1);
99+
await ql.count(PLAIN, {});
100+
expect(keyed).toHaveBeenCalledTimes(1);
101+
expect(global).toHaveBeenCalledTimes(2);
102+
});
103+
});

‎packages/objectql/src/engine.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5112,6 +5112,24 @@ export class ObjectQL implements IObjectQLEngine {
51125112
this.logger.debug('Registered middleware', { object: options?.object, total: this.middlewares.length });
51135113
}
51145114

5115+
/**
5116+
* [#21080] Whether a middleware is registered FOR `objectName` — a
5117+
* {@link registerMiddleware} call whose `object` names it
5118+
* (`IObjectQLEngine.hasObjectMiddleware`).
5119+
*
5120+
* A read of what the registrations above recorded, and nothing else: it
5121+
* runs no middleware and registers none. A global registration (no
5122+
* `object`, or `'*'`) matches every object in {@link executeWithMiddleware}
5123+
* but is keyed to none, so it is not counted.
5124+
*
5125+
* Asked by a read path that runs no engine operation — the analytics
5126+
* native-SQL strategy, which executes raw SQL through the driver — so it can
5127+
* decline an object whose gates live here and let this engine serve it.
5128+
*/
5129+
hasObjectMiddleware(objectName: string): boolean {
5130+
return this.middlewares.some((m) => !!m.object && m.object !== '*' && m.object === objectName);
5131+
}
5132+
51155133
/**
51165134
* Execute an operation through the middleware chain
51175135
*/

0 commit comments

Comments
 (0)