Repository navigation
Commit ab82001
fix(service-analytics): judge each read scope with the engine's own admission before composing it (#20232)
Fixes #19995
Clause-②: no
The analytics ObjectQL face now asks the engine's own `where` admission,
`IObjectQLEngine.judgeFilter` (#20157, ruling C), about each row-level
read scope on its own, before composing it into the `where` it hands
`executeAggregate`. A scope the engine refuses is refused in the
withheld `READ_SCOPE_COMPILE_FAILED` / 500 (#5367). A scope the engine
serves is still served. The caller's own `where` keeps the engine's
answer.
The close condition in the ruling is met on the final head: both
analytics HTTP doors were re-measured over every class the ruling names
(the four here, the eleven withheld by PR #20017 / #20046 / #20072, and
the four `driver-sql` doors from PR #20037), and no response body
carries policy content.
## What changed
All in `packages/services/service-analytics/src/`.
- **`read-scope-sql.ts`: new `assertReadScopeAdmittedByEngine(scope,
objectName, context, host)`.** It calls the host's judge on the scope
alone, under the verb every engine-bound merge runs (`'aggregate'`) and
the context that merge forwards.
- An `ok: false` verdict is raised through the module's one envelope
helper, `readScopeCompileError`. The engine's sentence stays in the
thrown message for the operator's log, and the 500 declaration withholds
it on the wire. The verdict's own `code` / `status` describe a caller's
mistake, so they do not travel either.
- A throw from the judge itself (a fault, not a verdict) is raised in
the same envelope.
- No judge, or an `undefined` answer, means the scope is not judged here
("cannot answer, do not block").
- It is exported from the file only. The package entry is unchanged.
- The header gains a ruling-C section, and the paragraph that said these
doors were out of reach is updated.
- **`strategies/objectql-strategy.ts`: called at both engine-bound
merges**, `withReadScope` (direct path and cross-object base aggregate)
and `resolveFkAttr` (the referenced object's scope). It runs after the
existing guards (vacancy, comparand faces, placeholders), so a scope
they refuse keeps their sentence. It runs before the `'policy'` mark,
like them.
- **`strategies/types.ts`:
`DatasetScopedStrategyContext.judgeFilter?`**, the package-local hook,
typed from the contract member itself (`IObjectQLEngine['judgeFilter']`,
made non-nullable) plus the `undefined` answer. This is the
`declaredFieldType` / `sqlDialect` pattern.
- **`analytics-service.ts`: `AnalyticsServiceConfig.judgeFilter?`**,
passed to the strategy context untouched. A service configured with no
judge logs one `warn` on its first unjudged scoped merge, naming the
consequence and the remedy.
- **`plugin.ts`: the judge is wired to the engine the `executeAggregate`
auto-bridge executes on**, resolved per call through the same
`tryGetDataEngine`. It is wired ONLY when the plugin bridges
`executeAggregate` itself. The judge must be the executor, or it would
refuse scopes the executor serves, and a host that supplies its own
`executeAggregate` has not said which engine that is.
- A `data` engine without `judgeFilter`: `undefined`, plus one `warn`
from the plugin.
- No engine at all: `undefined`, silently, because the executor refuses
that query itself.
- **`plugin.ts`, record-label fetch (the #14329 door): the same checks
as `resolveFkAttr`.** This is a fourth engine-bound merge. It `$and`s
the referenced object's scope into `executeAggregate` to turn a lookup
dimension's ids into labels, and it ran only the vacancy guard. Measured
before this change: on the dataset door, a selection ordered by a lookup
dimension runs the sort-key label pass, and that pass relayed the
engine's 400 with policy content. It did so for the residue classes and
also for classes every other merge already withheld (a list in the
equality slot, an unknown placeholder). It now runs the comparand faces,
the placeholder resolver and the engine's admission on the scope alone.
See "Scope" under Acceptance notes.
⛔ **Not a catch around `executeAggregate`.** The caller's own `where` is
never judged here. Pinned, and ablation E6 shows those pins turning red
under a blanket catch.
**Why a served scope stays served.** The judge is the executing engine,
under the same verb and context. `judgeFilter` runs the engine's two
admission stages, the same functions in the same order execution runs,
and stops before any driver. Every object-form door judges a node
against the field map and the context, never against its siblings. So
the scope alone is admitted exactly when the scope inside `{ $and:
[userFilter, scope] }` is. Ablation E8 turns the served-placeholder
control red when the judge reads a different context.
## Premises, measured before writing the fix
1. **The contract member and its implementation**
(`objectql-engine.ts:306`, `engine.ts:8783` on `ce70876e4c`). Read, and
measured: for one refused filter, `judgeFilter(..., { operation:
'aggregate' })` returned the same `code`, `status` and message string
that `aggregate` raised.
2. **What the `data` service hands out.** In a booted `LiteKernel` with
`ObjectQLPlugin` and `AnalyticsServicePlugin`, `getService('data')` is
the same object as `getService('objectql')`. It is an `ObjectQL`
instance, and `typeof judgeFilter` is `'function'`. It is not a wrapper.
3. **The four classes on current main.** They relayed policy content on
both doors at base `ce70876e4c` (table below).
4. **The verb.** `'aggregate'` reproduces the execution message exactly
(premise 1). The verb changes only the message prefix, never the
verdict.
5. **The existing guards.** Kept. An unwired host relies on them, and
the pins below show such a host still withholds a guarded class while
the four residue classes keep today's engine 400.
## Measurement: both analytics HTTP doors, before and after
**How.** A scratch probe, never committed, lived in
`packages/runtime/src` only while it ran.
- **Kernel:** a real `LiteKernel` booted with `ObjectQLPlugin` and
`AnalyticsServicePlugin`. The plugin auto-bridged `executeAggregate`,
and after the fix it wired the judge, to a real `ObjectQL` over
`SqliteWasmDriver`. The plugin options supplied `getReadScope` and
`admitObjectRead`, and fixed `queryCapabilities` to the ObjectQL face.
Nothing else was stubbed.
- **Doors:** `@objectstack/runtime`'s dispatcher, `POST
/api/v1/analytics/query`, and `@objectstack/rest`, `POST
/analytics/dataset/query`.
- **Runs:** before = base `ce70876e4c`; after = this branch with the
`service-analytics` dist rebuilt (the new sentence is present in
`dist/index.js` and `dist/index.cjs`).
- **"Policy content"** = the synthetic policy field name or comparand
appears anywhere in the response body. **"Log"** = the refusal's detail
reached the door's error-log channel.
| Read-scope class | Both doors, before | Policy content in body, before
| Both doors, after | Policy content in body, after | Detail in the
server log, after |
|---|---|---|---|---|---|
| Text operator over a non-text field | `INVALID_FILTER` / 400 | yes |
`READ_SCOPE_COMPILE_FAILED` / 500 | no | yes |
| Temporal comparand the field cannot interpret | `INVALID_FILTER` / 400
| yes | 500 | no | yes |
| Filter on a virtual (formula) field | `INVALID_FIELD` / 400 | yes |
500 | no | yes |
| Dotted path through a lookup | `INVALID_FIELD` / 400 | yes | 500 | no
| yes |
| A residue class in the BASE scope on the cross-object path | 400 | yes
| 500 | no | yes |
| A residue class in the REFERENCED object's scope (text operator;
dotted path into a scalar) | 400 | yes | 500 | no | yes |
| The nine comparand classes (PR #20017 / #20046): list in the implicit
equality slot, list under `$eq`, scalar under `$in`, scalar under
`$nin`, one-bound `$between`, plain-object member in `$in`, `undefined`
comparand, plain-object comparand under `$eq`, `null` member in `$in` |
`READ_SCOPE_COMPILE_FAILED` / 500 | no | unchanged | no | yes |
| The two placeholder classes (PR #20072): unknown placeholder; known
placeholder the context cannot resolve | 500 | no | unchanged | no | yes
|
| Refused `$icontains` comparand (#20068) | 500 | no | unchanged | no |
yes |
| The four `driver-sql` doors (PR #20037): missing column; retired or
unknown operator; combinator with a non-array operand; non-boolean
`$null` / `$exists` | `INVALID_FILTER` / 400, withheld | no | unchanged
| no | unchanged |
| Record-label fetch, sort-key pass (dataset door): a residue class on
the referenced object | 400 | yes | 500 | no | yes |
| Record-label fetch, sort-key pass (dataset door): list in the equality
slot; unknown placeholder | 400 / `FILTER_TOKEN_UNKNOWN` 400 | yes | 500
| no | yes |
The `/analytics/query` door has no label pass. The display label pass
catches a failed fetch and renders raw ids: 200 before and after, with
the detail in the `warn` log.
**Controls, identical before and after:**
- A well-formed scope answers 200 with exactly its rows.
- A scope with a placeholder the context resolves answers 200 on the
dataset door. The dispatcher harness carries no user, so that door
answers the withheld 500 both before and after.
- A well-formed referenced-object scope buckets what it hides as
`(restricted)`.
- A well-formed referenced scope on the label pass is served.
- The caller's own `where` in each of four shapes (text operator on a
number field, uninterpretable temporal comparand, virtual field, dotted
path) answers its 400 on both doors, and the body carries the caller's
own diagnostic.
## Tests
New file: `src/__tests__/objectql-read-scope-engine-admission.test.ts`,
19 cases. Each builds `AnalyticsServicePlugin`'s own composition over a
real `ObjectQL` + `SqliteWasmDriver` as its `data` service. Only
`queryCapabilities` is fixed to the ObjectQL face.
- **Refusal pins** assert `code` `READ_SCOPE_COMPILE_FAILED` and
`status` 500. They also assert the two reads every analytics HTTP door
takes before relaying prose:
`serverFaultProvenance(resolveThrownHttpError(err, 500))` is
`'declared'`, and `declaredRefusalMessage(err)` is undefined. The thrown
message, which is the log channel, still names the detail.
- The four classes on the direct path.
- A well-formed caller `where` beside a refused scope.
- The cross-object base scope, and the referenced-object scope.
- The record-label sort-key pass.
- A judge that throws.
- **Preservation pins:**
- A well-formed scope, and a placeholder the forwarded context resolves,
are served with exactly their rows.
- A well-formed referenced scope keeps its `(restricted)` bucket.
- The label pass with a well-formed scope is served, sorted by label.
- The caller's own `where` (text operator, temporal comparand, virtual
field) keeps `INVALID_FILTER` / `INVALID_FIELD` / 400 with its message
and no server-fault declaration.
- **Unwired tiers:**
- `AnalyticsService` with no judge keeps the engine's 400 for a residue
class, still withholds a guarded class, serves a well-formed scope, and
logs exactly one line across three queries.
- A plugin host with its own `executeAggregate` is not wired to a
guessed engine: a residue class keeps the engine's 400. At the
record-label fetch, the comparand and placeholder guards this PR adds
there withhold a guarded class and an unresolvable placeholder; they are
new on that host too.
- A `data` engine without `judgeFilter` keeps the 400, and the plugin
logs exactly once across two queries.
**Results on the final head `f6dbebe5`:**
- `pnpm --filter @objectstack/service-analytics test`: `Test Files 129
passed (129)`, `Tests 3041 passed (3041)`.
- `pnpm --filter @objectstack/service-analytics typecheck`: exit 0. `tsc
--noEmit --listFiles` includes the new test (count 1).
- **Downstream consumers**, run because the wire envelope of
already-refused scopes moves. Each run was against the rebuilt
`service-analytics` dist:
- `@objectstack/rest`: 10 `analytics-*` files, 148 tests green.
- `@objectstack/runtime`: the 19 test files that touch analytics, 566
tests green.
- `@objectstack/dogfood`: the 6 analytics-touching files, 36 tests
green. These boot the real stack, where the judge is wired, and include
the label-scope and RLS suites.
- `@objectstack/client`: the analytics test, 7 green.
## Ablations
Each leg ran from committed state through `scripts/ablation-replace.mjs`
in WRAP mode, against the new test file. The anchor had to hit exactly
once and the blob had to change. Every restore was proven: the blob
equals HEAD, and `git diff HEAD` is empty. An outer shell trap restored
all four source files from `HEAD` on any exit. The subject is imported
relatively from `src`, so no dist is involved. Every direction was
predicted before the run; E1 reddened two more pins than predicted
(below).
| Leg | Mutation | Result |
|---|---|---|
| E1 | Delete the `withReadScope` judge call | 9 failed. Predicted 7
(the four classes, the scope beside a caller `where`, the cross-object
base scope, the throwing judge). Also red: both once-log pins, which
need that call to ask at all. |
| E2 | Delete the `resolveFkAttr` judge call | 1 failed: the
referenced-object pin |
| E3 | Delete the label-fetch judge call | 1 failed: the label sort-key
pin |
| E4 | Delete the label-fetch comparand guard | 1 failed: the unwired
plugin host's label pin |
| E4b | Delete the label-fetch placeholder guard | 1 failed: the same
test's placeholder assertion |
| E5 | Wire the judge from the `data` engine even when the host supplied
its own `executeAggregate` | 1 failed: "not wired to a guessed engine" |
| E6 | Wrap the direct `executeAggregate` in a blanket catch re-raised
as the withheld 500 | 6 failed: the three caller-`where` pins and the
three unwired-tier pins that expect the engine's 400 |
| E7 | Judge the COMPOSED tree instead of the scope alone | 3 failed:
the caller's own `where` was misattributed as the scope's 500 |
| E8 | Judge with no context instead of the forwarded one | 1 failed:
the served-placeholder control was refused |
| E9 | Drop the service's once-flag | 1 failed: two warn lines |
| E10 | Drop the plugin's once-flag | 1 failed: two warn lines |
| E11 | The service never logs the missing judge | 1 failed: zero warn
lines |
| E12b | Relay the engine's verdict as-is (its code, status and message)
| 8 failed: every judge-dependent refusal pin |
The first E12 attempt was a no-op: its replacement contained its own
anchor, so the anchor count stayed at 1, the tool refused, and no test
ran. It was re-run as E12b with a replacement that does not contain the
anchor.
## Gates
- **Derived gates.** `node scripts/pm/dispatch-gates.mjs --repo
objectstack-ai/objectstack --commands` on `f6dbebe5` derived 61
commands, the same count as the dispatch-time list. All 61 exited 0,
each exit code captured right after a single redirect. The `--ran`
reconciliation read: `61 derived, 61 run, 0 NOT-MEASURED, 0 UNRUN (a
DERIVED zero — all 61 recorded an exit code and none of them is 3)`.
- `check:dual-build-cjs-loads` first exited 3 (PREREQUISITE NOT MET).
After `turbo run build --filter=./packages/* --filter=./packages/*/*`
(71/71 tasks) it exited 0. `check:dts-closure`,
`check:sourcemap-no-sources-content`, `check:published-files` and
`check:lean-entry-closure` were re-run on that build and exited 0.
- `check-plugin-teardown-shape.mjs --self-test` first exited 3: the
shallow checkout could not reach its pinned positive-control commit.
After fetching that one commit it exited 0.
- **Outside the derivation, run because the diff adds a `warn` in
`plugin.ts` and in the service:** `check:startup-registry-verdict` 0,
`check:durability-log-level` 0.
- **Issue citations.** `node scripts/check-issue-citations.mjs`: 28
citations across 5 files, all resolve.
- **Lint, narrowed to the 6 touched TypeScript files.**
- `eslint --no-inline-config --format json` reported 6 files, 0 errors,
0 warnings, none ignored.
- `eslint --print-config` for each file shows `parserOptions` limited to
`ecmaVersion` / `sourceType`, with no `project` and no `projectService`.
Type-aware linting is off, so this diff cannot move a verdict on an
untouched file.
- `pnpm lint` itself is CI's.
## Acceptance notes
- **Exported types.** `AnalyticsServiceConfig` (exported from the
package index) gains one optional member, `judgeFilter`. Its type,
`ReadScopeFilterJudge`, is exported from `strategies/types.ts` only, not
from the index; it reaches the published declarations through that
member. `DatasetScopedStrategyContext` (not exported) gains
`judgeFilter`. `AnalyticsServicePluginOptions` is unchanged. Changeset:
`@objectstack/service-analytics` patch.
- **Wiring.** The plugin wires the judge only when it auto-bridges
`executeAggregate`. That is how every shipped composition boots (`os
serve`'s capability provider, the verify harness): no host in this
repository passes its own `executeAggregate`. A plugin host that does
keeps today's behaviour, and logs one `warn`, everywhere except the
plugin's record-label fetch, whose new comparand and placeholder guards
run on every host that uses it (see Scope below). A host constructing
`AnalyticsService` directly can pass `judgeFilter`, from the engine its
`executeAggregate` runs on.
- **Precedence.** When the caller's `where` and the scope are both
refused, and only the engine would refuse the caller's clause, the
scope's 500 answers first. This is the same precedence PR #20017 and PR
#20072 recorded.
- **Scope: the record-label fetch.** The fourth merge is repaired in
place: same defect class, a mechanical repeat of the `resolveFkAttr`
form, a file on the claim's surface, and no new gate family. On a host
whose own `executeAggregate` runs on something other than ObjectQL, the
label fetch now refuses off-contract scope shapes that such an executor
tolerated. That is the same note PR #20017 carried for `resolveFkAttr`;
the ObjectQL executor refused every one of them already. The display
label pass's catch is unchanged.
- **The once-lines** are per service instance and per plugin instance,
emitted on first use, never at init. They show up once per test file
that builds a service without a judge.
- **`origin/main`** moved by one commit after the base, `805af4f2`
(`packages/cli` only). It shares no path or behaviour with this diff and
is not merged.
- **Observation, not filed (zero pull).** In a kernel with no `security`
service, the analytics object-read admission bridge answers
`PERMISSION_DENIED` / 403 on every query. The kernel's `getService`
throws for a missing service, and the bridge reads a throw as "unusable"
(fail-closed). Its init warning describes the opposite. Every shipped
composition includes `SecurityPlugin`, and the failure direction is
fail-closed. Measured incidentally by the probe.
- **Files not touched:** `filter-normalizer.ts`, `preview-evaluator.ts`,
`text-match-sql.ts`, `native-sql-strategy.ts`, `packages/objectql`,
`packages/spec`.
## Seat append — patch round 1 (head `d9a1002f`)
- The at-tier contract review of `f6dbebe5` (record `5856105439`) found
two overclaims in the changeset prose. The dev corrected them, plus two
adjacent imprecisions, in `d9a1002f` (`.changeset` only, +3/−3).
- Two sentences of this body carried the same overclaim as the review's
defect 2: the Tests "Unwired tiers" bullet and the Acceptance-notes
"Wiring" sentence. The seat corrected both in place, using the dev's
text from its round-1 report. No other byte of this body changed.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01TEah6PeJGjxJfbHaySJjLQ)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent ca753c0 commit ab82001
7 files changed
Lines changed: 735 additions & 15 deletions
File tree
- .changeset
- packages/services/service-analytics/src
- __tests__
- strategies
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
0 commit comments