Repository navigation
Commit 980bc05
fix(service-analytics)!: the NativeSQL execute face and the /analytics/sql echo refuse a read scope the shared comparand faces refuse (#20046)
Fixes #20018
Clause-②: no (narrowing)
## What changed
`compileScopedFilterToSql`
(`packages/services/service-analytics/src/read-scope-sql.ts`) is the
read-scope lowering behind two analytics faces:
- the NativeSQL execute face, `NativeSQLStrategy.applyReadScope`, for
the base table and every joined hop;
- the `/analytics/sql` echo, `ObjectQLStrategy.generateSql`.
It is also a public export of the package. Once its own lowering
returns, it now calls `assertReadScopeComparandsRunnable` on the scope.
That is the helper PR #20017 added in the same module, and it runs
`@objectstack/spec/data`'s `assertListComparandShapes` and
`normalizeFilterComparandTypes` on the scope alone.
A scope those faces refuse now gets `READ_SCOPE_COMPILE_FAILED` / 500
with the message withheld (the #5367 ruling, re-affirmed as #7598 Q2 =
A) on the native face and the echo. The refusal comes before any
statement is built or executed. That is the answer the ObjectQL execute
face has given since PR #20017, so one read scope gets one verdict on
all three analytics faces.
- **Files:** the call itself is one line. The rest of the diff is:
- the module-header section (#20018);
- a note on the helper's docblock;
- a one-paragraph note on the binary extra in `comparand-shape.ts`,
whose "accepted in every bind position" is no longer true of the
read-scope door;
- tests and the changeset.
- **Not touched:** `objectql-strategy.ts` and `native-sql-strategy.ts`.
Both faces reach the guard through the compiler, so neither needs a call
site of its own.
- **Also not touched:** `filter-normalizer.ts`, `preview-evaluator.ts`
and `packages/spec`.
## Measurement, recorded before the fix
Everything in this section was measured on `9d81af714f` (`origin/main`,
pre-fix). The rows are **executed**, not read from the compiled string.
- **Harness:** a scratch probe that is not committed, plus measurement
commit `8c80d9c3d2` (the new test file alone).
- **Database:** one real `SqliteWasmDriver` with four fixture rows.
`region` is NULL on d3, and `owner` and `amount` are NULL on d4.
- **ObjectQL face:** a real `ObjectQL` engine.
- **Echo:** its SQL was run on the same database.
- **Native:** `NativeSQLStrategy.execute` through `executeRawSql` on the
same database. It was not stubbed.
- **Scopes:** the `getReadScope` contract, filled by hand.
| read-scope shape class | ObjectQL execute | echo (SQL executed) |
native execute |
|:--|:--|:--|:--|
| plain-object comparand under `$eq` *(the card's first shape)* |
`READ_SCOPE_COMPILE_FAILED` / 500 | compiles; the database refuses the
bind | `DATABASE_ERROR` / 500 |
| null member in `$in`, `['emea', null]` *(the card's second shape)* |
500 | d1 | d1: the NULL member matches nothing |
| null-only `$in` | 500 | no rows | no rows |
| null member in `$in` under `$not` | 500 | d3 | d3: **only** the NULL
row, which the scope names as excluded |
| null member in `$nin` | 500 | d3 | d3: **only** the NULL row, which
the scope names as excluded |
| null comparand under `$gt` / `$lte` | 500 | no rows | no rows |
| null `$between` bound | 500 | no rows | no rows |
| blank `$between` bound | 500 | d1 d2 d4 | d1 d2 d4 |
| plain-object comparand under `$ne` / `$gt`; empty object under `$eq` |
500 | compiles; DB refuses | `DATABASE_ERROR` / 500 |
| bigint beyond 2^53 (implicit, `$in`) | 500 | no rows | no rows |
| binary comparand, binary `$in` member | 500 | no rows | no rows |
| `Map` or function comparand | 500 | compiles; DB refuses |
`DATABASE_ERROR` / 500 |
| controls (9 well-formed scopes, including the null predicates and the
live RLS composite) | the same rows on all three faces | | |
The card named two shapes. The measurement found the gap is every shape
the two shared faces refuse and this compiler's own gates did not: null
list members and bounds, null ordering comparands, blank bounds,
oversized bigints, binary, and non-scalar objects in a scalar position.
That is the set that moves.
### The mechanism hypotheses
- **Hypothesis 1 is confirmed, and the set is wider than the card's two
shapes.** These are the compile arms that accepted them:
- ``case '$eq': return val === null ? … : `${col} = ${bind(params,
val)}`;``. A plain object is bound, and no gate judges a `$eq`
comparand's type. `assertNoFieldReferenceComparand` steps past an object
that is not `{ $field: string }`.
- `case '$in'` → `assertCompilableMembers(op, field, val)` →
`isBindableComparand(member)`, which admits `null` (an accepted
comparand type) and binary (a package-local extra), then `IN (…)` binds
each member.
- The ordering arms and `$between` bind whatever passes those gates. The
implicit-equality arm binds any non-object.
- **Hypothesis 2: measured.** See the table above, and the producers
section below.
- **Hypothesis 3 is confirmed in verdict, but the placement is better
than "at the entry".** The two walks are pure functions of the scope, so
the placement cannot change which scopes are refused, only which log
sentence a doubly-refused scope carries.
- Ablation A2 below put the call at the entry and turned 37 tests red in
7 files: the new ordering pin, plus 36 existing log-sentence pins, for
example `read-scope-eq-array-refusal` (15) and
`read-scope-undefined-comparand` (8).
- After the lowering, every shape this compiler already refused keeps
its own sentence (the #13926 ordering), and the faces add only what
would otherwise have been lowered.
### Producers: who can emit these shapes today
All readings below are on `9d81af714f`.
| producer | emits a refused shape? | evidence |
|:--|:--|:--|
| RLS `using` predicates, through `compileCelToFilter` →
`RLSCompiler.compileFilter` | **yes, from an authored predicate.** `f in
['a', null]`, `f in [null]`, `!(f in ['a', null])`, `f > null`, `f <=
null` and `f != current_user` each lower verbatim into a refused shape.
| Scratch probe. All six pass `isSupportedRlsExpression`, and
`validateRlsPredicateEnforceability` returns 0 findings for each. |
| authored policies in this repository | **none** | `git grep` of
`using` / `check` predicates for a null list member or a null ordering
comparand, over `examples/**` and `packages/**/*.ts` (tests excluded): 0
matches, exit 1. The control on the same tree and file set, predicates
naming `current_user`, matched 77 lines in 7 files, exit 0. |
| a resolved membership variable with a null member | no | The #13496
guard. Measured: `f in current_user.teams` with a null member lowers to
the deny sentinel. |
| `plugin-sharing` `buildReadFilter` | no | Owner ids are
`String(userId)` or a resolver's `string[]`. `grantedRecordIds` filters
out `null` and `''`. |
| a host `getReadScope` option, or a direct caller of the export |
anything | The door the card names. |
Verdict: **no producer both legitimately authors one of these shapes and
relies on the native answer.** Three facts support that:
- The in-repo producer emits these shapes only from predicates that the
standing rulings, carried by the shared faces, refuse. No policy in this
repository is one of them.
- The ObjectQL analytics face already refused every such scope.
- The native answer was not the predicate's meaning. It dropped the null
member, admitted only the NULL rows the scope excludes, returned zero
rows, or hit a database error.
So this is execution under the rulings, not a `needs_decision`. The
authoring half is reported separately as a finding; see the Acceptance
notes.
## Tests
The new file is
`packages/services/service-analytics/src/__tests__/read-scope-comparand-three-faces.test.ts`,
with 32 cases. It uses one `SqliteWasmDriver`, a real `ObjectQL` behind
the ObjectQL face, and the echo's SQL executed.
- **13 refusal classes.** Each asserts on all three faces: `code`
`READ_SCOPE_COMPILE_FAILED`, `status` 500, and the prose withheld.
"Withheld" means `serverFaultProvenance(resolveThrownHttpError(err,
500))` is `'declared'` and `declaredRefusalMessage(err)` is undefined.
- **The native face refuses before any statement reaches the database.**
Zero `executeRawSql` calls across all 13 classes.
- **The joined hop.** `applyReadScope`'s per-hop lowering refuses a
joined object's scope, named for that hop.
- **The public export** refuses every class, and a well-formed scope
compiles to the same bound predicate as before.
- **Log sentences.** A shape the compiler already refused, a list under
`$eq`, keeps its own sentence. A shape only the faces refuse carries
their sentence, the same on all three faces.
- **Controls:**
- with no scope, every face serves all four rows;
- 9 well-formed scopes admit the same rows on every face. They include
`{ region: null }`, `$ne: null`, a non-empty `$nin`, the live RLS
composite with an emptied `$in` beside an own-rows grant, and the
spelling the null-member ruling prescribes (`$or` of `$in` and `$null:
true`);
- a well-formed caller `where` composes with a well-formed scope;
- a caller `where` in a refused scope shape answers exactly as it does
with no scope, and is never attributed to the read scope.
**Existing pins that asserted the lowering binds a refused shape.** Each
is re-judged with a `[#20018]` note, and each keeps what it was there to
pin:
- `comparand-door-single-source.test.ts`. Three matrix cells now read
`READ_SCOPE_COMPILE_FAILED/500`: `null` under `scopeIn`, `binary` under
`scopeIn` and `scopeEq`, and `plain object` under `scopeEq`. This PR
moves no `where`-door cell.
- After merging #20032 (#20010), the `null` row carries both re-judged
cells: `whereIn` is `INVALID_FILTER/400` (#20010) and `scopeIn` is
`READ_SCOPE_COMPILE_FAILED/500` (this PR).
- The "six accepted types, every position" check names each moved cell
with the change that moved it, and holds every other cell at `accept`.
- `comparand-shape-refusal.test.ts`. "Keeps binding every legitimate
`$in` member" drops `null`, and a new case pins `null` refused with the
face's sentence.
- `cross-field-reference-refusal.test.ts`. `{ $gt: { $field: 5 } }` is
refused as a plain object, with the type face's sentence and not the
field-reference gate's.
- `read-scope-boolean-flag-comparand.test.ts`. An inherited `$null`
still cannot trip the flag gate, because the refusal carries no flag
sentence. The object is refused by the type face as a non-plain value.
- `read-scope-undefined-comparand.test.ts`. `$in: [null]`, `$nin:
[null]` and `$between: [null, 5]` leave the null control group for their
own "refused by the shared face" block. Every null predicate stays in
the group, unmoved.
**Results:**
| run | tree | result |
|:--|:--|:--|
| new file, measurement commit (test only) | `8c80d9c3d2` | `Tests 17
failed \| 15 passed (32)`. Every refusal class and the three pins built
on them are red; every control is green. The first face to fail in each
row is the echo; the ObjectQL face passed first. |
| whole package | `0fbed27877` (final; `origin/main` `adbbc5d01e`
merged) | `Test Files 120 passed (120)`, `Tests 2689 passed (2689)` |
| `pnpm --filter @objectstack/service-analytics typecheck` |
`0fbed27877` | exit 0; `tsc --noEmit --listFiles` includes the new test
file |
**Ablations.** Both ran through `node scripts/ablation-replace.mjs` in
WRAP mode (the anchor must hit exactly once, the blob must change, and
the tool's own trap restores). The subject resolves to `src/` through
relative imports, so no `dist/` leg applies. Each restore was proven:
blob `34705e267dba` equals `HEAD`, and `git diff HEAD` is empty.
A1 was re-run on the final head `0fbed27877`. A2 ran on `f293340e94`.
`read-scope-sql.ts` is the same blob, `34705e267dba`, on both trees
(neither merge touched it), so A2's placement result stands.
| leg | mutation | result (whole package) |
|:--|:--|:--|
| A1 (at `0fbed27877`) | delete the new
`assertReadScopeComparandsRunnable(filter, alias);` call | `26 failed \|
2663 passed (2689)`: all 17 negative pins in the new file, plus the 9
re-judged pins (3, 1, 1, 1 and 3 across the five files). Every control
stayed green. The same 17 + 9 went red at `f293340e94` (`26 failed \|
2567 passed (2593)`). |
| A2 (at `f293340e94`) | call it BEFORE `compileNode` instead of after |
`37 failed \| 2556 passed (2593)`: the new ordering pin, plus 36
existing log-sentence and precedence pins in 6 files |
The first attempt at A2 was a void run. Its replacement text contained
its anchor, so the tool refused with "anchor count moved 1 -> 1" and
restored. Nothing was measured. It was re-run with a three-line anchor,
and that is the result above.
## Gates
**Derived gates.** Derived at `0fbed27877` with `node
scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack
--commands`: 61 families, not stale. They are a superset of the 48-line
dispatch-time list.
- **59 exited 0.**
- **`check:dual-build-cjs-loads` and `check:type-check-debt`** first
answered `PREREQUISITE NOT MET` (exit 3), because the workspace had no
`dist/`. After building every `./packages/**` workspace package
(`VERDICT command-exit 0`), both exited 0: `check:dual-build-cjs-loads`
passes its floors, and `check:type-check-debt` re-measured 4 ledger
entries (53 raw tsc errors) with none above its recorded number.
- **The `--ran` reconciliation:** 61 derived, 61 run, 0 NOT-MEASURED, 0
UNRUN. That zero is derived, not claimed: all 61 records carry an exit
code and none is 3.
**Other checks:**
- `GITHUB_TOKEN=… node scripts/check-issue-citations.mjs`: exit 0; 8
citations judged, 8 resolve.
- `node scripts/check-adr-0087-registration.mjs --base origin/main` (one
of the derived families, at `0fbed27877`): exit 0. The changeset is
`BREAKING+bang+clause-②-narrowing`, disposition `not-required
(no-migration-prescription)`.
- **Lint, narrowed to the 8 touched TypeScript files** with `eslint
--no-inline-config --format json` at `0fbed27877`: 8 files read, 0
errors, 0 warnings, none ignored.
- The changeset is outside eslint's population ("no matching
configuration").
- `eslint.config.mjs` enables no type-aware linting: its `parserOptions`
carry only `ecmaVersion` and `sourceType`, with no `project` and no
`projectService`. So this diff cannot move a verdict on an untouched
file.
- `pnpm lint` itself is CI's.
## Acceptance notes
- **BREAKING, `minor` with `!`.** Read scopes the native face and the
echo used to serve are refused now; the table above lists what they
served. The changeset carries the banner and the fix spellings: the null
predicate beside a membership, `$gte` / `$lte` for a one-sided range,
and a scalar comparand.
- **The refusal set is the shared faces' set, not only the card's two
shapes.** Judging the scope with the same function the ObjectQL face
uses is what makes the verdicts equal. A narrower hand-picked subset
would have left the other rows of the table as "one scope, two answers".
- **Binary.** The package-local binary bindable (`isBindableComparand`)
no longer reaches the read-scope door. The shared type face refuses
binary, and the ObjectQL face already did. The `where` door and the
predicate itself are unchanged.
- **The joined-hop log sentence names the alias.**
`compileScopedFilterToSql` knows only the alias, so the operator's log
reads `read scope for "ALIAS"`: the object name on the base table, the
join alias on a hop. The response withholds it either way.
- **Precedence on the native face.** When a scope carries both a shape
this compiler refuses and one only the faces refuse, the compiler's
sentence answers. The verdict is the same either way.
- **The CRUD path is not an analytics face and was not measured here.**
The security middleware composes the same RLS filter into the engine's
`where` after the engine's shared-face seam has run. What `driver-sql`
answers for these shapes there is outside this card.
- **Finding, class (c), not fixed here, for the seat to file.** The RLS
authoring surface admits predicates that lower into scope shapes the
shared faces refuse: a literal `null` in an `in` list, an ordering
comparison against `null`, or a comparison against the whole
`current_user` object. All three pass `isSupportedRlsExpression`, and
`validateRlsPredicateEnforceability` returns no finding for them. They
are refused (500) on every analytics face after this PR. The evidence is
in the producers table.
- **`origin/main` was merged in twice.**
- At `b3735968ba`: three commits in `packages/objectql` and
`packages/plugins/plugin-security`, sharing no path with this diff.
- At `adbbc5d01e`: #20032 (#20010) and a driver-sql / driver-turso fix.
That merge had one content conflict, in the comparand matrix, resolved
as above.
- `comparand-shape-refusal.test.ts` and
`cross-field-reference-refusal.test.ts` auto-merged and were re-read on
the merged tree. #20032's edits there are to the `where`-door pins and
this PR's are to the read-scope pins, and their notes agree: the null
member is refused by the same ruling at each door, each in its own
envelope. One sentence in the non-string `$field` case ("it binds as
JSON") is now scoped to the `where` door, where it still holds.
- The dependency closure was rebuilt before the final runs.
- **Files not touched:** `filter-normalizer.ts`, `preview-evaluator.ts`,
`objectql-strategy.ts`, `native-sql-strategy.ts`, `packages/spec`.
- **#19995** stays open behind #20020, for its engine- and driver-door
residue. This PR does not address it.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01Evb5jFDZGKQE9KG4jbMfMF)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 57c2b73 commit 980bc05
9 files changed
Lines changed: 691 additions & 31 deletions
File tree
- .changeset
- packages/services/service-analytics/src
- __tests__
| 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 | + | |
Lines changed: 37 additions & 12 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
45 | 45 | | |
46 | 46 | | |
47 | 47 | | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
48 | 51 | | |
49 | 52 | | |
50 | 53 | | |
| |||
131 | 134 | | |
132 | 135 | | |
133 | 136 | | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
134 | 143 | | |
135 | 144 | | |
136 | | - | |
| 145 | + | |
137 | 146 | | |
138 | 147 | | |
139 | 148 | | |
| |||
146 | 155 | | |
147 | 156 | | |
148 | 157 | | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
149 | 164 | | |
150 | 165 | | |
151 | 166 | | |
152 | | - | |
| 167 | + | |
153 | 168 | | |
154 | 169 | | |
155 | 170 | | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
156 | 175 | | |
157 | 176 | | |
158 | 177 | | |
159 | | - | |
| 178 | + | |
160 | 179 | | |
161 | 180 | | |
162 | 181 | | |
| |||
272 | 291 | | |
273 | 292 | | |
274 | 293 | | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
275 | 302 | | |
276 | | - | |
277 | | - | |
278 | | - | |
279 | | - | |
280 | | - | |
281 | | - | |
282 | | - | |
283 | | - | |
284 | | - | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
285 | 310 | | |
286 | 311 | | |
287 | 312 | | |
| |||
Lines changed: 13 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
269 | 269 | | |
270 | 270 | | |
271 | 271 | | |
272 | | - | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
273 | 285 | | |
274 | 286 | | |
275 | 287 | | |
| |||
Lines changed: 12 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
417 | 417 | | |
418 | 418 | | |
419 | 419 | | |
420 | | - | |
421 | | - | |
| 420 | + | |
| 421 | + | |
| 422 | + | |
422 | 423 | | |
423 | 424 | | |
424 | 425 | | |
425 | 426 | | |
426 | | - | |
| 427 | + | |
| 428 | + | |
| 429 | + | |
| 430 | + | |
| 431 | + | |
| 432 | + | |
| 433 | + | |
| 434 | + | |
| 435 | + | |
427 | 436 | | |
428 | 437 | | |
429 | 438 | | |
| |||
Lines changed: 11 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
213 | 213 | | |
214 | 214 | | |
215 | 215 | | |
216 | | - | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
217 | 227 | | |
218 | 228 | | |
219 | 229 | | |
| |||
0 commit comments