Skip to content

Commit 7c1039b

Browse files
fix(service-analytics)!: the NativeSQL face and the /analytics/sql echo resolve a read-scope placeholder with the caller's context, as the ObjectQL face does (#20075) (#20111)
Fixes #20075 Clause-②: no ## What this changes A read scope now gets one placeholder verdict on all three analytics faces: the same resolved values and rows, or the same withheld 500. `compileScopedFilterToSql` (`read-scope-sql.ts`) is the lowering behind the NativeSQL execute face (`NativeSQLStrategy.applyReadScope`, the base table and every joined hop) and the `/analytics/sql` echo (`ObjectQLStrategy.generateSql`). It never resolved a filter placeholder, so both faces bound `{current_user_id}`, `{current_org_id}` or a date macro as literal text. The ObjectQL execute face hands the same scope to the engine, which resolves it with the caller's context and, since #19995, refuses one it cannot resolve. The compiler now takes an optional `ReadScopeCompileOptions.context`. It resolves the scope with `resolveFilterTokens(scope, filterTokenContextFrom(context))` from `@objectstack/core`, the published resolver the engine calls, and lowers the **resolved** tree. Both call sites pass `ctx.context`. A placeholder the resolver refuses is refused as `READ_SCOPE_COMPILE_FAILED` / 500 with the message withheld. That judgement has one spelling: a module-local `resolveReadScopePlaceholders`. The existing `assertReadScopePlaceholdersResolvable` (PR #20072, used at the ObjectQL merge sites) now calls it too. There is no second implementation of token classification or resolution, and no second spelling of the envelope or its sentence. ## Measured before the fix Recorded on this branch (`8bf233a1fc`, an empty commit whose message carries the full table), on base `a8bcce69c8`. The setup: a real `SqliteWasmDriver`, a real `ObjectQL` behind the ObjectQL face, and the echo's SQL run on the same database. Every face is driven through `AnalyticsService` with the caller `{ userId: 'u_me' }` and no active org. Rows d1 and d3 are owned by u_me, d2 by u_other, and d4 has no owner. | read scope | compiler binds | native base | echo (SQL run) | ObjectQL base (control) | |:--|:--|:--|:--|:--| | `owner = {current_user_id}` | the literal | none | none | d1 d3 | | `owner $ne {current_user_id}` | the literal | d1 d2 d3 d4 | d1 d2 d3 d4 | d2 d4 | | `owner $in [{current_user_id}, u_nobody]` | the literal | none | none | d1 d3 | | `closed_on $lt {today}` | the literal | d1 d2 d3 | d1 d2 d3 | d1 d3 | | an unknown `{token}` | the literal | served, none | served, none | 500 withheld | | `{current_org_id}` with no active org | the literal | served, none | served, none | 500 withheld | The `$ne` row widened the scope: the literal matches nobody, so the exclusion also admitted the caller's own rows. The joined hop was measured too. Each face is compared with its own literal twin there, because the two strategies shape a hop differently by design: NativeSQL filters base rows, and the ObjectQL cross-object path uses the `RESTRICTED_BUCKET`. On the ObjectQL hop every placeholder row equalled its twin. On the NativeSQL hop none did. With **no** context, the ObjectQL control refused every context-token scope and still **resolved** the date macro (UTC), serving d1 d3. **HTTP doors** (throwaway harnesses in `packages/runtime` and `packages/rest`, read and measured only, not committed): - `POST /api/v1/analytics/query` and `/api/v1/analytics/sql`: 200 with the literal bound, and the echo printed the literal. - `POST /analytics/dataset/query`: 200 with the literal bound. **Reach:** the card's reach is confirmed by reading. It takes a host-supplied `getReadScope`. The auto-bridged `security.getReadFilter` composes concrete values: the sharing predicate, the RLS compiler's output and the controlled-by-parent derivation. ## Where the resolution sits among the scope gates, and why - **Before the lowering**, because the lowering binds values. What is bound, and what the echo prints, is the value the engine resolves. - **The refusal is raised after the lowering's own gates** (the #20068 `$icontains` arm among them) **and after the #20018 comparand faces.** This is the engine's order: its lowering doors run before its resolver. No gate's verdict moves either way. Resolution only replaces a fully-wrapped placeholder string with a non-empty string, and it copies the tree around it, where a non-`Date` class-instance comparand becomes a plain object; every such comparand is refused in both forms. So a scope that another gate also refuses keeps that gate's sentence. This is pinned. - **The #13926 vacancy guard** still runs at the two merge sites after the compiler returns, unchanged. - **Context at the call sites:** `native-sql-strategy.ts:654` and `objectql-strategy.ts:564` both hold `ctx.context`. That is the spec `StrategyContext.context`, which `AnalyticsService.callCtx` binds from the `context` argument of `query()` / `generateSql()`. It is the same value `withReadScope` already hands the ObjectQL-face assertion. Nothing in `spec`, `core`, `objectql`, `rest` or the service changes. ## The public export without a context I did not follow the default suggestion to refuse every placeholder when there is no context, because it disagrees with the control face. The engine resolves a date macro for a context-less operation (UTC now) and refuses a context token. Refusing every placeholder would give the NativeSQL face and the echo a second answer whenever `ctx.context` is `undefined`. So an absent `context` resolves the way the engine does: a date macro resolves against UTC, and a context token or an unknown placeholder gets the withheld 500. A placeholder is never bound as its literal text. Both halves are pinned, and so is the context's time zone being read. ## Tests (at `eafeaccea2`) - New file: `read-scope-placeholder-three-faces.test.ts`, 22 tests. It uses a real SQLite database and a real `ObjectQL`, with the ObjectQL face, the echo (its SQL executed) and the NativeSQL face, plus both joined hops. It pins: - each resolvable scope admits its literal twin's rows on every face; - both echoes print the resolved value; - each refused scope gets the withheld 500 on all three faces and both hops, before any native statement runs; - the context-less answer is the same on every face; - the sentence ordering; - placeholder-free scopes compile byte-for-byte as before, with or without a context; - the caller's own `where` is unchanged: it still resolves, and still answers `FILTER_TOKEN_UNKNOWN` / 400 with its message; - the public export's context-less decision. - `pnpm --filter @objectstack/service-analytics test`: 125 files / 2908 tests passed. `typecheck` exit 0, and `--listFiles` includes the new test file. - **Ablations**, through `node scripts/ablation-replace.mjs` in WRAP mode. The tests import the mutated sources by relative path (src), so no build sits between mutation and measurement. Each leg was proven restored: `git hash-object` equals the HEAD blob, and `git diff HEAD` is empty. | leg | mutation | red | |:--|:--|:--| | A1 | resolution removed (`lowered = filter`) | 18 of 22; the 4 survivors are the controls, including the byte-for-byte pin, which shows those pins encode the pre-fix output | | A2 | refusal raised early (`throw e`) | 1: the sentence-ordering pin | | A3 | refusal dropped | 9: every refusal, no-statement, context-less and sentence pin | | A4 | `context: ctx.context` removed at `applyReadScope` | 6 | | A5 | `context: ctx.context` removed at the echo's compile | 5 | - **HTTP doors after the fix** (the same throwaway harnesses, not committed): - anonymous `/analytics/query` and `/analytics/sql`: a context token gets the withheld 500; a date macro gets 200 with the resolved date bound and printed. - `/analytics/dataset/query` with a user: 200 with the user id bound; an unknown or unresolvable placeholder gets the withheld 500. ## Gates (at `eafeaccea2`) - `dispatch-gates --commands` derived 61 families. 59 exited 0. - Two are NOT MEASURED because a prerequisite is missing (exit 3): - `check:dual-build-cjs-loads` needs every package's `dist`. A narrowed spot check does pass: the `service-analytics` CJS build loads and resolves a placeholder. - `check:type-check-debt` needs the whole-workspace closure built. - `--ran` reconciliation: 61 accounted, 0 UNRUN. - `check-issue-citations` with a token: 14 citations, all resolve. - ESLint, narrowed to the 4 changed `.ts` files: 0 errors and 0 warnings over 4 files, read from the `--format json` output. ESLint's own `--print-config` shows no `parserOptions.project` and no `projectService`, so this config is not type-aware and the diff cannot move any untouched file's verdict. The repo-wide `pnpm lint` is CI's. - `main` moved twice during the work and was merged both times, never rebased (`7c2146223c`, `eafeaccea2`). The dependencies each merge touched were rebuilt before the suite and every gate above were re-run on the final head. ## Changeset classification: differs from the dispatch default `.changeset/20075-native-scope-placeholders.md` is `minor`, `Clause-②: no (narrowing)`, **BREAKING**, with an ADR-0087 `not-required (no-migration-prescription)` disposition. The dispatch's default was `patch`. Two reasons: - The NativeSQL face, the echo and the public export now refuse scopes they used to serve. #20018 made the same alignment on the same faces and was declared `no (narrowing)` / `minor`. - The `Check Changeset` level rule counts a new accepted key on a published package's public surface as at least `minor`, and `context` is such a key. The declaration line at the top of this body is the claim's, verbatim. Reverting to `patch` means dropping the arm, the `!`, the banner and the marker in that one file. ## Acceptance notes - **Date-macro instant.** A scope's date macro resolves when the compiler runs, not at the query's pinned instant. That is the same property as the ObjectQL face, where the engine resolves at `aggregate` time. - **Log sentence for a scope with two defects.** If a scope carries a vacancy shape and also a bad placeholder, the log sentence differs between faces: the vacancy guard runs after the compiler on NativeSQL and the echo, but first on the ObjectQL face. The wire envelope is identical. The #20018 comparand faces already have the same asymmetry. - **Joined hops.** The two strategies still differ in shape on a joined hop (row filter versus `RESTRICTED_BUCKET`). That is a documented design, not this card's. --- _Generated by [Claude Code](https://claude.ai/code/session_01Evb5jFDZGKQE9KG4jbMfMF)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 7a13e05 commit 7c1039b

5 files changed

Lines changed: 664 additions & 3 deletions

File tree

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
---
2+
'@objectstack/service-analytics': minor
3+
---
4+
5+
fix(service-analytics)!: the NativeSQL execute face and the `/analytics/sql` echo resolve a read-scope filter placeholder with the caller's context, and refuse one they cannot resolve, as the ObjectQL execute face already does (#20075)
6+
7+
Clause-②: no (narrowing)
8+
9+
<!-- adr-0087: not-required (no-migration-prescription) Nothing authorable moves: `packages/spec` is untouched and no metadata key or value changes meaning. What changes is which runtime faces resolve a read-scope placeholder and refuse one they cannot resolve; the engine behind the ObjectQL face already did both, with the same `@objectstack/core` resolver. `objectstack migrate meta` has nothing to act on and the ledger has no row to gain. The other categories are closed on facts: the package publishes (not `unpublished`); no ADR-0087 id covers a read-scope placeholder (not `registered` / `already-registered`); and the change is runtime behaviour of a function, not a TypeScript declaration removed or narrowed (not `runtime-interface-only` / `type-surface-only`). -->
10+
11+
**BREAKING**: an accept-set narrowing on the read-scope lowering, shipped as `minor` under the launch-window convention. `minor` is also the level a new accepted option key takes.
12+
13+
`compileScopedFilterToSql` is the read-scope lowering behind the NativeSQL execute face (`NativeSQLStrategy.applyReadScope`, the base table and every joined hop) and the `/analytics/sql` echo (`ObjectQLStrategy.generateSql`), and a public export of this package. It never resolved a filter placeholder, so both faces bound `{current_user_id}`, `{current_org_id}` or a date macro as its literal text. The ObjectQL execute face hands the same scope to the engine, which resolves it with the caller's context. One read scope, two row sets.
14+
15+
It now takes an optional `context` (`ReadScopeCompileOptions.context`) and resolves the scope with `resolveFilterTokens(scope, filterTokenContextFrom(context))` from `@objectstack/core`, the resolver the engine calls, before lowering it. Both strategies pass the request's context.
16+
17+
| a read scope carrying | before, on the NativeSQL face and the echo | now |
18+
|:--|:--|:--|
19+
| a placeholder the caller's context resolves | its literal text was bound: an equality matched no row, and a `$ne` exclusion admitted every row, the caller's own included | the resolved value is bound and the echo prints it; the same rows as the ObjectQL face |
20+
| an unknown placeholder, or a context token the request has no value for | served, with the literal bound | `READ_SCOPE_COMPILE_FAILED` / 500, message withheld, as on the ObjectQL face |
21+
22+
Without a context the answer is the engine's for a context-less operation: a date macro resolves against UTC now, and a context token is refused. A placeholder is never bound as its literal text.
23+
24+
Who is affected: a host whose own `getReadScope` returns scopes carrying placeholders, and a direct caller of `compileScopedFilterToSql`. The security service's read filter, the auto-bridged default, composes concrete values and is not affected.
25+
26+
Not changed: a scope with no placeholder compiles to the same SQL and parameters as before. The caller's own `where` is untouched: `AnalyticsService` already resolves its placeholders and answers an unresolvable one `FILTER_TOKEN_UNKNOWN` / `FILTER_TOKEN_UNRESOLVED` / 400 with its message. The ObjectQL execute face is untouched.

0 commit comments

Comments
 (0)