Repository navigation
Commit 7b76fff
fix(service-analytics): the ObjectQL execute face refuses an unrunnable read scope in the withheld READ_SCOPE_COMPILE_FAILED / 500 envelope (#20017)
Part of #19995 — this PR closes the comparand half of the card: every
scope shape the engine's two SHARED comparand faces refuse. The card
stays open for the residue: scope shapes refused by engine doors that
read the object's schema or the request's context, and by `driver-sql`,
still answer a relayed 400. Why that half is not done here, and the
decision it needs, is under "Residue" below.
Clause-②: no
## What changed
The ObjectQL execute face composed a row-level read scope into the
engine's `where` without judging it. A scope carrying a comparand the
engine refuses came back as the engine's `INVALID_FILTER` / 400. The
HTTP doors relay a 4xx's message, and that message named the policy's
field and comparand. The NativeSQL face and the `/analytics/sql` echo
refuse the same scope as `READ_SCOPE_COMPILE_FAILED` / 500 with the
message withheld (the #5367 ruling, re-affirmed as #7598 Q2 = A). So one
scope got two envelopes, depending on which analytics face served it.
- **`read-scope-sql.ts`: new `assertReadScopeComparandsRunnable(scope,
objectName)`**, next to the #13640 vacancy guard. It runs the two faces
the engine itself runs on every object-form `where`
(`assertListComparandShapes` and `normalizeFilterComparandTypes` from
`@objectstack/spec/data`) on the scope alone. It re-raises any refusal
through the module's one envelope helper, `readScopeCompileError`,
keeping the walk's sentence for the operator's log. It is exported from
the file only, not from the package entry, so the published surface is
unchanged.
- **`objectql-strategy.ts`: called at both engine-bound merges.**
- `withReadScope` covers the direct path and the cross-object base
aggregate.
- `resolveFkAttr` covers the referenced object's own scope.
- It runs after the vacancy guard and before the `'policy'` mark.
**Why at the merge boundary, on the scope alone.** At that point the
scope is still a distinguishable object. One line later it is
`$and`-composed with the caller's filter, and then nothing downstream
can tell whose clause a refusal came from.
**Why the refusal set does not move.** The engine's comparand-shape
refusal does not read the `'policy'` provenance mark (measured below).
Both faces are pure walks whose verdict on a subtree depends neither on
the rest of the tree nor on any schema. So the scope alone answers
exactly as the scope inside `{ $and: [userFilter, scope] }` does, and
nothing the engine serves is refused. A `{ $field }` scope (served on
this path under #7598 Q1 = B) is stepped around by both faces, as the
engine steps around it.
⛔ This is not a catch around `executeAggregate`. The caller's own
`where` still reaches the engine's doors for some shapes, and those keep
their `INVALID_FILTER` / 400 with the message. This is pinned, and
ablation A5 below shows that pin going red under misattribution.
## Measurement: recorded before the fix
Measured on base `14add487b4`, using a scratch probe that is not
committed: `AnalyticsService` (ObjectQL only) over a real `ObjectQL`,
run twice, once over `SqlDriver` (better-sqlite3) and once over
`SqliteWasmDriver`. The two drivers gave the same answer in every row.
The HTTP leg went through the real routes: `@objectstack/runtime`'s
dispatcher `POST /api/v1/analytics/query` and `@objectstack/rest`'s
`POST /analytics/dataset/query`. The read scopes are the `getReadScope`
contract filled by hand. The same probe was re-run after the fix,
against a rebuilt `dist`.
| Read-scope shape class | Layer that answers on the ObjectQL face |
Before (ObjectQL face and both HTTP doors) | Refusal text names policy
content | After |
|---|---|---|---|---|
| List in the implicit equality slot | Engine, shared list-shape face |
`INVALID_FILTER` / 400, relayed | yes | `READ_SCOPE_COMPILE_FAILED` /
500, withheld |
| List under `$eq` | Engine, shared list-shape face | 400, relayed | yes
| 500, withheld |
| Scalar under `$in` or `$nin` | Engine, shared list-shape face | 400,
relayed | yes | 500, withheld |
| One-bound `$between` | Engine, shared list-shape face | 400, relayed |
yes | 500, withheld |
| Null member in `$in` | Engine, shared list-shape face | 400, relayed |
yes | 500, withheld |
| Plain-object member in `$in`; plain-object comparand under `$eq`;
`undefined` comparand | Engine, shared comparand-type face | 400,
relayed | yes | 500, withheld |
| Emptied `$nin` (control) | Analytics vacancy guard (#13640) | 500,
withheld | no (withheld) | unchanged |
| List under `$ne` or `$gt`; nested relation value; cross-field
reference | `driver-sql`, which reads the mark | 400, message withheld |
no | unchanged |
| Column the object does not have | `driver-sql`, missing column | 400,
relayed | yes | unchanged (residue) |
| Retired or unknown operator | `driver-sql` operator vocabulary | 400,
relayed | yes (the field) | unchanged (residue) |
| Text operator on a non-text field | Engine, declared-type door | 400,
relayed | yes | unchanged (residue) |
| Temporal comparand the platform cannot read | Engine, temporal door |
400, relayed | yes | unchanged (residue) |
| Combinator with a non-array operand | `driver-sql` | 400, relayed |
yes | unchanged (residue) |
| Non-boolean `$null` or `$exists` | `driver-sql` | 400, relayed | yes
(the field) | unchanged (residue) |
| Unknown filter placeholder | Engine token resolver |
`FILTER_TOKEN_UNKNOWN` / 400, relayed | yes | unchanged (residue) |
| Well-formed scope (control) | — | 200, scoped rows | — | unchanged |
- **Hypothesis 2 (does the engine refusal read the mark?): confirmed.**
The scope reached the engine stamped `'policy'` by `withReadScope`, and
the comparand-shape refusal still returned its full text. Of the
refusals in the table, only `driver-sql`'s bind and cross-field refusals
read the mark.
- **Hypothesis 3 (does the ObjectQL face reach the read-scope
compiler?): confirmed.** The ObjectQL execute face never reaches
`compileScopedFilterToSql`. The NativeSQL face and the echo answered
`READ_SCOPE_COMPILE_FAILED` / 500 for every fixed class except two: a
plain-object comparand under `$eq` and a null member in `$in`. That
compiler compiles both of those (see Acceptance notes).
- **Scope reads quoted.** The strategy has four:
- `generateSql`'s echo merge compiles through
`compileScopedFilterToSql`, is already withheld, and is untouched.
- `withReadScope` and `resolveFkAttr` are the two engine-bound merges,
and both are now guarded.
- `NativeSQLStrategy.applyReadScope` compiles and is untouched.
## Tests
The new file is
`packages/services/service-analytics/src/__tests__/objectql-read-scope-refusal-envelope.test.ts`,
with 19 cases over a real `ObjectQL` and `SqliteWasmDriver`.
- **Refusal cases.** Each refused shape class asserts:
- `code` `READ_SCOPE_COMPILE_FAILED` and `status` 500;
- that the reads every analytics HTTP door takes before relaying prose
(`serverFaultProvenance(resolveThrownHttpError(err, 500))` is
`'declared'`, `declaredRefusalMessage(err)` is undefined) mean the prose
is withheld;
- that the thrown message, which is the operator's log channel, still
carries the detail.
- **Paths covered:** the direct path, a well-formed caller `where`
beside a refused scope, the cross-object base scope, and the
referenced-object scope.
- **Controls:**
- a well-formed scope is served with its rows;
- a cross-field scope is served;
- an emptied `$in` beside an own-rows grant is served;
- a well-formed referenced-object scope buckets what it hides as
`(restricted)`;
- the caller's own `where` refused by the ENGINE stays `INVALID_FILTER`
/ 400 with its message;
- the caller's own `where` in the scope's refused shape stays 400 with
its message.
**Results:**
- Measurement commit `e4908254bc` (test only, before the fix): `Tests 13
failed | 6 passed (19)`. Every refusal case received `INVALID_FILTER`,
and every control was green.
- At `9a40317b15`:
- the new file gives `Tests 19 passed (19)`;
- the whole package gives `Test Files 118 passed (118)`, `Tests 2560
passed (2560)`;
- `pnpm --filter @objectstack/service-analytics typecheck` exits 0, and
`tsc --listFiles` includes the new test.
**Ablations.** Each leg ran through `scripts/ablation-replace.mjs` (the
anchor must hit exactly once, and the blob must change) with a trap.
Each restore was proven: the blob equals HEAD, and `git diff HEAD` is
empty.
| Leg | Mutation | Result |
|---|---|---|
| A1 | Delete the `withReadScope` call | 12 failed, 7 passed. Every
direct and cross-object-base refusal case received `INVALID_FILTER`. |
| A2 | Delete the `resolveFkAttr` call | 1 failed (the referenced-object
case), 18 passed |
| A3 | Delete the comparand-type face call | 3 failed (the three
type-face rows) |
| A4 | Delete the list-shape face call | 10 failed (the shape rows, the
composed case, both cross-object cases) |
| A5 | Judge the COMPOSED tree instead of the scope alone | 1 failed:
the caller's engine-refused `where` received `READ_SCOPE_COMPILE_FAILED`
instead of `INVALID_FILTER` |
## Gates
- **Derived gates.** Derived at `9a40317b15` with `node
scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack
--commands`: 72 families, a superset of the dispatch-time list. All 72
exited 0.
- `check:dual-build-cjs-loads` and `check:type-check-debt` first
answered PREREQUISITE NOT MET (exit 3). I built the `./packages/*`
closure and re-ran both, and both exited 0.
- The `--ran` reconciliation reported 72 derived, 72 run, 0 unrun.
- **Issue citations.** `GITHUB_TOKEN=… node
scripts/check-issue-citations.mjs` exited 0: 10 citations judged, 10
resolve.
- **Lint, narrowed to the four touched TypeScript files.** `eslint
--no-inline-config --format json` read 4 files and reported 0 errors and
0 warnings. `eslint.config.mjs` enables no type-aware linting (no
`parserOptions.project`, no `projectService`), so this diff cannot move
a verdict on an untouched file. `pnpm lint` itself is CI's.
## Acceptance notes
- **`@objectstack/objectql` is a new devDependency**, aliased to source
in the package's `vitest.config.ts` with an anchored regex. This is the
repair `check:test-source-alias` prescribes; the ledger does not grow,
and the gate is green. The refusal under test is the engine's own, so a
stub bridge would only have tested the stub.
- **Envelope reuse.** The guard lives in `read-scope-sql.ts` because
`readScopeCompileError` is module-local by design. A guard in the
strategy file would have needed a second spelling of the envelope.
- **A host with its own `executeAggregate` bridge:** the scope is now
judged against the spec's shared comparand faces whichever bridge
executes, the same posture as the vacancy guard. A custom bridge that
used to tolerate one of these off-contract scope shapes now gets the
withheld 500. The engine bridge refused all of them already.
- **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 now answers first. A caller clause refused by the analytics
`where` door still answers first, with its 400.
- **NativeSQL face and echo:** unchanged. They compile two shapes (a
plain-object comparand under `$eq`, a null member in `$in`) that the
shared faces and the ObjectQL face refuse. This was measured with a
stubbed `executeRawSql`, so the database-level outcome is unmeasured. It
is reported to the seat and not touched here.
- **`origin/main` has moved on by one commit** (`fc6ddb87a4`,
driver-sqlite-wasm text round-trip). It shares no path with this diff
and is not merged.
- **Files not touched:** `filter-normalizer.ts`, `packages/spec`,
`packages/objectql`, `packages/rest`, `packages/runtime`.
## Residue: why "Part of"
These classes still answer the engine's or the driver's 400 with the
message relayed:
- a column the object does not have;
- a retired or unknown operator;
- a text operator on a non-text field;
- a temporal comparand the platform cannot read;
- a combinator with a non-array operand;
- a non-boolean `$null` or `$exists`;
- an unknown filter placeholder.
They are refused by engine doors that read the object's schema or the
request's context, and by `driver-sql`'s own compile. This package
cannot judge them without a second copy of those rules. The sound fix is
at the refusing layer: the refusal reads the `'policy'` provenance mark
(#8220), as `driver-sql`'s bind and cross-field refusals already do.
That change is in the engine and driver lanes, and it opens an envelope
question between the #5367 and #7929 rulings. It is handed back to the
seat as a decision.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01Evb5jFDZGKQE9KG4jbMfMF)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 4463966 commit 7b76fff
7 files changed
Lines changed: 501 additions & 2 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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
31 | 31 | | |
32 | 32 | | |
33 | 33 | | |
| 34 | + | |
34 | 35 | | |
35 | 36 | | |
36 | 37 | | |
| |||
| 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 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
| 350 | + | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
0 commit comments