From c8ac3e7b30a098e0da83226ef114163ee0932a81 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 01:34:05 +0000 Subject: [PATCH] fix(service-analytics): refuse vacating read scopes on the echo and native-SQL merge sites MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The #13640 guard (assertReadScopeCannotVacate) covered only the ObjectQL ENGINE merges; ObjectQLStrategy.generateSql (the /analytics/sql echo) and NativeSQLStrategy.applyReadScope still compiled the $not-over-$in-empty family to a constant-TRUE predicate — the echo rendered, and the native strategy actually executed, a whole-table WHERE for a scope execute() refused. Call the same guard at both merge sites, after the compiler so its own #13571 refusals keep their messages; compileScopedFilterToSql itself (the ruled #13571 residue included) is unchanged. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8 --- .changeset/read-scope-vacancy-echo-native.md | 5 + ...bjectql-read-scope-vacancy-refusal.test.ts | 12 +- .../read-scope-vacancy-three-faces.test.ts | 426 ++++++++++++++++++ .../service-analytics/src/read-scope-sql.ts | 46 +- .../src/strategies/native-sql-strategy.ts | 15 +- .../src/strategies/objectql-strategy.ts | 12 + 6 files changed, 499 insertions(+), 17 deletions(-) create mode 100644 .changeset/read-scope-vacancy-echo-native.md create mode 100644 packages/services/service-analytics/src/__tests__/read-scope-vacancy-three-faces.test.ts diff --git a/.changeset/read-scope-vacancy-echo-native.md b/.changeset/read-scope-vacancy-echo-native.md new file mode 100644 index 0000000000..3843ca2266 --- /dev/null +++ b/.changeset/read-scope-vacancy-echo-native.md @@ -0,0 +1,5 @@ +--- +'@objectstack/service-analytics': patch +--- + +Refuse a non-binding (vacating) read scope at the two remaining `getReadScope` merge sites: the `/analytics/sql` echo (`ObjectQLStrategy.generateSql`) and `NativeSQLStrategy.applyReadScope`. The `$not`-over-`$in: []` family compiled to a constant-TRUE predicate on those routes, so the echo rendered — and the native strategy actually executed — a whole-table `WHERE` for a scope the ObjectQL execution path already refused (#13640). All three faces now answer one verdict, in the same `READ_SCOPE_COMPILE_FAILED` / 500 envelope; the ruled `$in: []` zero-rows reduction, the live RLS empty-membership composite, and `compileScopedFilterToSql` itself (the ruled #13571 residue included) are unchanged. diff --git a/packages/services/service-analytics/src/__tests__/objectql-read-scope-vacancy-refusal.test.ts b/packages/services/service-analytics/src/__tests__/objectql-read-scope-vacancy-refusal.test.ts index 328f8b4286..1f0aeecef6 100644 --- a/packages/services/service-analytics/src/__tests__/objectql-read-scope-vacancy-refusal.test.ts +++ b/packages/services/service-analytics/src/__tests__/objectql-read-scope-vacancy-refusal.test.ts @@ -343,7 +343,11 @@ describe('[#13640] the FK→attribute resolution is the same door, and is guarde // ── Immobility: the two routes #13649 already settled ─────────────────────── -describe('[#13640] IMMOBILITY — NativeSQL and the `/analytics/sql` echo did not move', () => { +// (#13926 later added the same guard AFTER the compiler at both of those +// routes' merge sites — `read-scope-vacancy-three-faces.test.ts` owns that +// story. What THIS block pins is unchanged either way: the compiler's own +// answers, and that for the shapes it refuses, its message still names it.) +describe('[#13640] IMMOBILITY — `compileScopedFilterToSql` itself did not move', () => { it('`compileScopedFilterToSql` still refuses an empty `$nin` with ITS OWN #13571 message', async () => { // Not the new guard's message. The two refusals stay distinguishable, which // is how a reader can tell which door turned a query away — and how this @@ -380,8 +384,10 @@ describe('[#13640] IMMOBILITY — NativeSQL and the `/analytics/sql` echo did no }); it('the ObjectQL `/analytics/sql` echo refuses through the COMPILER, not the new guard', async () => { - // `generateSql` renders the scope through `compileScopedFilterToSql`, and - // that is deliberately untouched: the echo's disposition is #13571's. + // `generateSql` renders the scope through `compileScopedFilterToSql`, + // which for THIS spelling refuses before the merge-site guard #13926 + // added after it can run — so the compiler's own message still names + // the door that answered. const ctx = { getCube: (name: string) => (name === 'deals' ? CUBE : undefined), queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }), diff --git a/packages/services/service-analytics/src/__tests__/read-scope-vacancy-three-faces.test.ts b/packages/services/service-analytics/src/__tests__/read-scope-vacancy-three-faces.test.ts new file mode 100644 index 0000000000..bb38816f58 --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/read-scope-vacancy-three-faces.test.ts @@ -0,0 +1,426 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#13926] ONE read scope, ONE verdict — on all three faces that serve it. + * + * #13640 guarded the ObjectQL ENGINE path (`execute()`), and its dispatch + * deliberately did not move the other two consumers of a `getReadScope` + * output. The result it declared, and this card closes: for a vacating scope + * spelling the ObjectQL EXECUTION refused while the `/analytics/sql` ECHO + * (`ObjectQLStrategy.generateSql`) still compiled it into a predicate that + * admits every row — and `NativeSQLStrategy.applyReadScope` built a real, + * EXECUTED `WHERE` from the same compiler, so the native route was not an + * echo-truthfulness gap but a latent whole-table read for any out-of-repo + * producer (`StrategyContext.getReadScope` is a spec contract; + * `NativeSQLStrategy.execute()` runs `generateSql`'s output through + * `ctx.executeRawSql` with no other read-scope door in front of it). + * + * ## MEASURED, not read from the compiler + * + * The card flagged its own row consequence as derived from the emitted + * predicate. This file measures it end-to-end on the pre-fix tree — one + * `SqliteWasmDriver` (`driver-sqlite-wasm` over `driver-sql`), three fixture + * rows, all three faces driven against it, echo SQL EXECUTED rather than + * string-matched: + * + * | scope (spelling) | execute() | echo (SQL run) | native execute() | + * |-----------------------------------------------|-------------|----------------|------------------| + * | `{ owner: { $nin: [] } }` | REFUSED | refused (#13571 arm) | refused (#13571 arm) | + * | `{ $not: { owner: { $in: [] } } }` | REFUSED | ALL THREE ROWS | ALL THREE ROWS | + * | `{ $not: { owner: [] } }` | REFUSED | refused (bare-array arm) | refused (bare-array arm) | + * | `{ $not: { owner: { $in: [], $ne: 'u_other' } } }` | REFUSED | ALL THREE ROWS | ALL THREE ROWS | + * | `{ $or: [{ $not: { owner: { $in: [] } } }, …] }` | REFUSED | ALL THREE ROWS | ALL THREE ROWS | + * | `{ $not: { $not: { owner: { $nin: [] } } } }` | REFUSED | refused (#13571 arm) | refused (#13571 arm) | + * + * Post-fix every row of that table reads REFUSED / REFUSED / REFUSED, in the + * module's one envelope (`READ_SCOPE_COMPILE_FAILED` / 500) — which is the + * whole card: the three faces may keep door-specific refusal MESSAGES (see + * below), but never door-specific VERDICTS. + * + * ## Where the guard stands, and why AFTER the compiler + * + * `assertReadScopeCannotVacate` (the #13640 guard, exported from + * `read-scope-sql.ts`) is now called at both remaining merge sites — the echo + * scope merge in `ObjectQLStrategy.generateSql` and + * `NativeSQLStrategy.applyReadScope` — AFTER `compileScopedFilterToSql` + * returns. The ordering is deliberate: the shapes the compiler already + * refuses (`$nin: []` at any depth, the bare array comparand) keep the + * refusal messages #13571 pinned on these very routes + * (`read-scope-empty-nin-refusal.test.ts` message pin stays green, + * unedited), so each door stays distinguishable in the operator's log — the + * property #13640's review called out — while the guard closes exactly the + * shapes that COMPILE and vacate. Coverage is structural, not curated: the + * guard is the same walk `execute()` trusts, so the three faces cannot drift + * without one of them editing shared code this file pins. + * + * ## What must NOT move + * + * - `compileScopedFilterToSql` itself. The #13571 verdict's residue — `$not` + * over `$in: []` still COMPILES there — is ruled to stay until a + * polarity-aware compiler design is ruled first. The controls below assert + * the compiler still compiles all three leaking spellings: IMMOBILITY + * CONTROLS, not contracts — whoever lands the ruled compiler design should + * expect them to redden and rewrite them in that PR. + * - The `$in: []` → zero-rows reduction and the live #13570 RLS composite + * (`{ $or: [{ owner: { $in: [] } }, { owner: 'u_me' }] }` — "own rows keep + * flowing"). Both are asserted on ALL THREE faces. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm'; +import type { AggregationNode, Cube, FilterCondition } from '@objectstack/spec/data'; +import type { AnalyticsQuery, DriverQuery, StrategyContext } from '@objectstack/spec/contracts'; + +import { AnalyticsService } from '../analytics-service.js'; +import { ObjectQLStrategy } from '../strategies/objectql-strategy.js'; +import { NativeSQLStrategy } from '../strategies/native-sql-strategy.js'; +import { compileScopedFilterToSql } from '../read-scope-sql.js'; + +const OBJECT = 'deal'; + +/** `owner` is NULL on r3 so NULL-safety regressions show up as a wrong id set. */ +const ROWS = [ + { id: 'r1', owner: 'u_me' }, + { id: 'r2', owner: 'u_other' }, + { id: 'r3', owner: null }, +]; +const ALL = ['r1', 'r2', 'r3']; + +const OBJECT_FIELDS: Record> = { + id: { type: 'text', name: 'id' }, + owner: { type: 'string', name: 'owner' }, +}; + +const CUBE: Cube = { + name: 'deals', + sql: OBJECT, + measures: { n: { sql: '*', type: 'count', title: 'n' } }, + dimensions: Object.fromEntries( + ['id', 'owner'].map((n) => [n, { name: n, label: n, type: 'string', sql: n }]), + ), + public: false, +} as unknown as Cube; + +const QUERY = { cube: 'deals', dimensions: ['id'], measures: ['n'] } as AnalyticsQuery; + +interface WireBearingError extends Error { + code?: unknown; + status?: unknown; +} + +/** One face's outcome: a refusal, or the sorted ids the face actually served. */ +type Outcome = { refusal?: WireBearingError; admitted?: string[] }; + +describe('[#13926] one read scope, one verdict — execute / echo / native agree', () => { + let driver: SqliteWasmDriver; + /** The ObjectQL EXECUTE face — the #13640-guarded route, via the service. */ + let service: AnalyticsService; + /** Swapped per case; the spec contract filled by hand, never by the RLS compiler. */ + let readScope: unknown = null; + + /** Run raw SQL (either strategy's `$N`-numbered output) on the REAL engine. */ + const runRawSql = async (sql: string, params: unknown[]): Promise[]> => { + const result = await driver.execute(sql.replace(/\$\d+/g, '?'), params as unknown[]); + if (Array.isArray(result)) return result as Record[]; + if (result && typeof result === 'object' && 'rows' in (result as Record)) { + return (result as { rows: Record[] }).rows; + } + return []; + }; + + beforeAll(async () => { + driver = new SqliteWasmDriver({ filename: ':memory:' }); + await driver.initObjects([{ name: OBJECT, fields: OBJECT_FIELDS } as any]); + for (const row of ROWS) await driver.create(OBJECT, { ...row }); + + service = new AnalyticsService({ + cubes: [CUBE], + queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }), + executeAggregate: async (objectName, options) => { + const query: DriverQuery = { + where: options.filter as FilterCondition, + groupBy: options.groupBy, + aggregations: options.aggregations?.map(({ field, method, alias }) => ({ + field, + function: method as AggregationNode['function'], + alias, + })), + }; + return (await driver.aggregate(objectName, query)) as Record[]; + }, + getReadScope: () => (readScope ?? undefined) as FilterCondition | undefined, + }); + }); + + afterAll(async () => { + await driver?.disconnect?.(); + }); + + /** A hand-filled `StrategyContext` for driving one strategy directly. */ + const directCtx = (nativeSql: boolean): StrategyContext => + ({ + getCube: (name: string) => (name === 'deals' ? CUBE : undefined), + queryCapabilities: () => ({ nativeSql, objectqlAggregate: !nativeSql, inMemory: false }), + getReadScope: () => (readScope ?? undefined) as FilterCondition | undefined, + executeRawSql: (_object: string, sql: string, params: unknown[]) => runRawSql(sql, params), + }) as unknown as StrategyContext; + + /** Face 1 — ObjectQL EXECUTE (engine lowering; #13640's door). */ + const executeFace = async (scope: unknown): Promise => { + readScope = scope; + try { + const result = await service.query(QUERY); + return { admitted: result.rows.map((r) => String(r.id)).sort() }; + } catch (e) { + return { refusal: e as WireBearingError }; + } + }; + + /** + * Face 2 — the `/analytics/sql` ECHO, its SQL then EXECUTED on the real + * engine. Running the string is what turns "the predicate admits every row" + * from a reading of the compiler into a measured row set: the echo's whole + * contract is to be the SQL execution would run. + */ + const echoFace = async (scope: unknown): Promise => { + readScope = scope; + try { + const { sql, params } = await new ObjectQLStrategy().generateSql(QUERY, directCtx(false)); + const rows = await runRawSql(sql, params); + return { admitted: rows.map((r) => String(r.id)).sort() }; + } catch (e) { + return { refusal: e as WireBearingError }; + } + }; + + /** Face 3 — NativeSQL EXECUTE (`applyReadScope` → `ctx.executeRawSql`): real rows. */ + const nativeFace = async (scope: unknown): Promise => { + readScope = scope; + try { + const result = await new NativeSQLStrategy().execute(QUERY, directCtx(true)); + return { admitted: result.rows.map((r) => String(r.id)).sort() }; + } catch (e) { + return { refusal: e as WireBearingError }; + } + }; + + const FACES = [ + ['execute', executeFace], + ['echo', echoFace], + ['native', nativeFace], + ] as const; + + const expectRefused = (face: string, o: Outcome): void => { + expect(o.admitted, `${face}: expected a refusal, got rows`).toBeUndefined(); + expect(o.refusal, `${face}: expected an Error`).toBeInstanceOf(Error); + expect(o.refusal?.code, `${face}: envelope code`).toBe('READ_SCOPE_COMPILE_FAILED'); + expect(o.refusal?.status, `${face}: envelope status`).toBe(500); + }; + + // ── Fixture honesty ──────────────────────────────────────────────────────── + + it('CONTROL: with no scope, every face serves the whole fixture', async () => { + // Without this, every refusal assertion below could pass on a harness that + // admits nothing — and the echo face would be executing SQL over no rows. + for (const [face, run] of FACES) { + const o = await run(null); + expect(o.refusal?.message, `${face}: fixture control refusal`).toBeUndefined(); + expect(o.admitted, `${face}: fixture control`).toEqual(ALL); + } + }); + + // ── The six vacating spellings #13640 measured — one verdict per scope ──── + + const VACATING: Array<[string, unknown]> = [ + ['{ owner: { $nin: [] } }', { owner: { $nin: [] } }], + ['{ $not: { owner: { $in: [] } } }', { $not: { owner: { $in: [] } } }], + ['{ $not: { owner: [] } }', { $not: { owner: [] } }], + ["{ $not: { owner: { $in: [], $ne: 'u_other' } } }", { $not: { owner: { $in: [], $ne: 'u_other' } } }], + ["{ $or: [{ $not: { owner: { $in: [] } } }, { owner: 'u_me' }] }", { $or: [{ $not: { owner: { $in: [] } } }, { owner: 'u_me' }] }], + ['{ $not: { $not: { owner: { $nin: [] } } } }', { $not: { $not: { owner: { $nin: [] } } } }], + ]; + + for (const [label, scope] of VACATING) { + it(`${label} is REFUSED on all three faces, in the one envelope`, async () => { + // Pre-fix, the echo and native faces ADMITTED ALL THREE ROWS for the + // spellings the compiler compiles (header table) — the leak this card + // closes. The verdict (code + status) is the cross-face contract; the + // message may differ per door (asserted separately below). + for (const [face, run] of FACES) { + expectRefused(face, await run(scope)); + } + }); + } + + // ── Door-distinguishable messages — same verdict, named doors ───────────── + + it('the compiler-refused spelling keeps #13571\'s OWN message on echo and native', async () => { + // `read-scope-empty-nin-refusal.test.ts` pins this end-to-end for native; + // asserted here beside the guard-message case so the ordering (compiler + // first, guard after) is pinned as a choice rather than an accident. + for (const run of [echoFace, nativeFace]) { + const o = await run({ owner: { $nin: [] } }); + expect(String(o.refusal?.message)).toContain('$nin for "owner" is empty'); + expect(String(o.refusal?.message)).not.toContain('read scope for'); + } + // The engine face has no compiler in front of it — the guard answers. + const engine = await executeFace({ owner: { $nin: [] } }); + expect(String(engine.refusal?.message)).toContain('read scope for "deal"'); + }); + + it('the compiling-but-vacating spelling is refused BY THE GUARD on echo and native', async () => { + for (const run of [echoFace, nativeFace, executeFace]) { + const o = await run({ $not: { owner: { $in: [] } } }); + expect(String(o.refusal?.message)).toContain('read scope for "deal"'); + expect(String(o.refusal?.message)).toContain('empty $in under negation at $not.owner.$in'); + } + }); + + // ── Ordinary scopes — same ADMISSION on all three faces ─────────────────── + + const ORDINARY: Array<[string, unknown, string[]]> = [ + ["{ owner: 'u_me' }", { owner: 'u_me' }, ['r1']], + ["{ owner: { $in: ['u_me', 'u_other'] } }", { owner: { $in: ['u_me', 'u_other'] } }, ['r1', 'r2']], + // Non-empty exclusion keeps NULL-safety — r3 stays in the answer. + ["{ owner: { $nin: ['u_other'] } }", { owner: { $nin: ['u_other'] } }, ['r1', 'r3']], + ]; + + for (const [label, scope, expected] of ORDINARY) { + it(`ORDINARY: ${label} admits the same rows on every face`, async () => { + for (const [face, run] of FACES) { + const o = await run(scope); + expect(o.refusal, `${face}: unexpected refusal`).toBeUndefined(); + expect(o.admitted, `${face}: admitted set`).toEqual(expected); + } + }); + } + + // ── Over-denial controls — the ruled reductions did not move, anywhere ──── + + it('OVER-DENIAL CONTROL: `{ owner: { $in: [] } }` still means zero rows on every face', async () => { + // The ruled #5322 / #5243 identity — narrowing at its own arm, load-bearing + // for the RLS composite below. A guard that catches it is the availability + // regression #13571's verdict rejected. + for (const [face, run] of FACES) { + const o = await run({ owner: { $in: [] } }); + expect(o.refusal, `${face}: unexpected refusal`).toBeUndefined(); + expect(o.admitted, `${face}: zero rows`).toEqual([]); + } + }); + + it('OVER-DENIAL CONTROL: the #13570 RLS composite still admits exactly the own row, everywhere', async () => { + // `{ $or: [{ owner: { $in: [] } }, { owner: 'u_me' }] }` is what the RLS + // compiler really emits for an emptied membership beside an own-rows grant, + // and it reaches ALL THREE faces through `security.getReadFilter`. + for (const [face, run] of FACES) { + const o = await run({ $or: [{ owner: { $in: [] } }, { owner: 'u_me' }] }); + expect(o.refusal, `${face}: unexpected refusal`).toBeUndefined(); + expect(o.admitted, `${face}: own row`).toEqual(['r1']); + } + }); + + it('OVER-DENIAL CONTROL: the denies-by-itself `$and` composite still denies, everywhere', async () => { + for (const [face, run] of FACES) { + const o = await run({ $and: [{ owner: { $in: [] } }, { owner: 'u_me' }] }); + expect(o.refusal, `${face}: unexpected refusal`).toBeUndefined(); + expect(o.admitted, `${face}: zero rows`).toEqual([]); + } + }); +}); + +// ── The joined-object door on the native strategy ─────────────────────────── + +describe('[#13926] `applyReadScope` guards the JOINED object\'s scope too', () => { + /** + * `applyReadScope` runs once per object in the statement — base table plus + * every joined object (the ADR-0021 D-C per-hop RLS injection). A vacating + * scope on the JOINED object silently un-scoped that hop while the base + * stayed scoped, so this door gets its own pin, named for the joined object. + * SQL-build level: the refusal fires before `executeRawSql`, so a stub + * executor proves it can never be reached. + */ + const joinCube: Cube = { + name: 'sales', + title: 'Sales', + sql: 'opportunity', + measures: { revenue: { name: 'revenue', label: 'Revenue', type: 'sum', sql: 'amount' } }, + dimensions: { region: { name: 'region', label: 'Region', type: 'string', sql: 'account.region' } }, + public: false, + }; + + const ctxWith = (accountScope: unknown): StrategyContext => + ({ + getCube: (name: string) => (name === 'sales' ? joinCube : undefined), + queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }), + getAllowedRelationships: () => new Set(['account']), + getReadScope: (obj: string) => + (obj === 'opportunity' ? { organization_id: 'org_A' } : accountScope) as FilterCondition, + executeRawSql: async () => { + throw new Error('unreachable: the refusal must fire before execution'); + }, + }) as unknown as StrategyContext; + + const QUERY = { + cube: 'sales', + measures: ['revenue'], + dimensions: ['region'], + timezone: 'UTC', + } as AnalyticsQuery; + + it('CONTROL: an ordinary joined-object scope still builds a per-hop-scoped statement', async () => { + const ctx = ctxWith({ organization_id: 'org_A' }); + const { sql } = await new NativeSQLStrategy().generateSql(QUERY, ctx); + expect(sql).toContain('"opportunity"."organization_id" ='); + expect(sql).toContain('"account"."organization_id" ='); + }); + + it('a vacating joined-object scope is REFUSED, named for the JOINED object', async () => { + let err: (Error & { code?: unknown; status?: unknown }) | undefined; + try { + await new NativeSQLStrategy().generateSql(QUERY, ctxWith({ $not: { organization_id: { $in: [] } } })); + } catch (e) { + err = e as Error & { code?: unknown; status?: unknown }; + } + expect(err).toBeInstanceOf(Error); + expect(err?.code).toBe('READ_SCOPE_COMPILE_FAILED'); + expect(err?.status).toBe(500); + // The BASE object's scope is ordinary, so only the joined hop can refuse. + expect(String(err?.message)).toContain('read scope for "account"'); + expect(String(err?.message)).toContain('empty $in under negation'); + }); +}); + +// ── Immobility: the compiler itself did not move ──────────────────────────── + +describe('[#13926] IMMOBILITY — `compileScopedFilterToSql` still compiles the residue', () => { + /** + * ⚠️ IMMOBILITY CONTROLS, not contracts (#13640's precedent, kept here on + * purpose): the #13571 verdict's residue — `$not` over `$in: []` compiling + * to constant TRUE at that lowering — stays exactly where that verdict left + * it, because the polarity-aware COMPILER design is ruled to come first. + * #13926 closed the two routes ABOVE the compiler; the compiler's own answer + * is pinned unchanged so this PR can prove it did not sneak the residue shut + * at the ruled-first site. Whoever lands the ruled design should EXPECT + * these to redden and rewrite them in that PR. + */ + const compiles = (scope: unknown): string => + compileScopedFilterToSql(scope as FilterCondition, 'deal').sql; + + it('`$not` over `$in: []` still compiles (to the negated FALSE constant)', () => { + expect(compiles({ $not: { owner: { $in: [] } } })).toContain('NOT'); + }); + + it('the multi-key spelling still compiles too', () => { + expect(compiles({ $not: { owner: { $in: [], $ne: 'u_other' } } })).toContain('NOT'); + }); + + it('the `$or`-nested spelling still compiles too', () => { + expect(compiles({ $or: [{ $not: { owner: { $in: [] } } }, { owner: 'u_me' }] })).toContain('NOT'); + }); + + it('an ordinary scope still compiles to the same bound predicate', () => { + const { sql, params } = compileScopedFilterToSql({ owner: 'u_me' } as FilterCondition, 'deal'); + expect(sql).toBe('"deal"."owner" = ?'); + expect(params).toEqual(['u_me']); + }); +}); diff --git a/packages/services/service-analytics/src/read-scope-sql.ts b/packages/services/service-analytics/src/read-scope-sql.ts index e00e960e9f..dca54a3d39 100644 --- a/packages/services/service-analytics/src/read-scope-sql.ts +++ b/packages/services/service-analytics/src/read-scope-sql.ts @@ -310,6 +310,13 @@ import { * cannot tell net polarity (see #13571's verdict). Separately, the ObjectQL * ENGINE execution path never reaches this compiler at all (#13640): this * refusal guards the NativeSQL path and the `/analytics/sql` echo only. + * [#13926] Both of those routes now ALSO call + * {@link assertReadScopeCannotVacate} at their own merge sites, after this + * compiler returns — so the residue's constant-TRUE predicate no longer + * reaches an engine or an echo from inside this package. The lowering above + * stays exactly as the verdict left it: a direct consumer of this public + * export still receives the compiled constant, and the ruled-first + * polarity-aware compiler design remains open. * * ## The ObjectQL ENGINE path gets the SAME disposition, at a different door (#13640) * @@ -364,10 +371,15 @@ import { * structural, not a change of mind: this guard is a WALK over the scope tree * that never reduces anything, so effective polarity is simply readable and * there is no interaction with the #5322 `$not`-over-identity reductions to - * rule on first. The consequence is declared rather than hidden: for that one - * spelling the ObjectQL echo (which compiles) and the ObjectQL execution - * (which now refuses) disagree, and closing the echo's half is #13571's - * follow-up, not this guard's business. + * rule on first. The consequence that left — the ObjectQL echo (which + * compiled) and the ObjectQL execution (which refused) answering one scope + * two ways — was declared here, tracked as #13926, and is now closed the + * same structural way: the echo's scope merge and + * `NativeSQLStrategy.applyReadScope` call this guard too, AFTER this + * compiler returns, so all three faces answer one verdict while this + * lowering stays as ruled (`read-scope-vacancy-three-faces.test.ts` pins + * the agreement, and pins this compiler's own answers as immobility + * controls). * * ⚠️ It is a SECOND line of defence, not a replacement for #13570's * producer-side guard, and deliberately not shared code with it: that guard @@ -527,15 +539,23 @@ function findEmptyMembership(node: unknown, negated: boolean, path: string): Emp * [#13640] Refuse a read scope that does not BIND, before it is handed to an * engine that would lower it to a boolean constant. * - * The door `ObjectQLStrategy` uses. See the module header's #13640 section for - * the measured table, for why the `$nin` arm is polarity-independent while the - * `$in` arm is not, and for the bound this deliberately keeps (an emptied - * POSITIVE membership at even polarity is a ruled reduction and a live RLS - * composite depends on it). - * - * ⛔ Not called by {@link compileScopedFilterToSql}. Calling it there would move - * the NativeSQL path and the `/analytics/sql` echo, whose disposition #13571 - * settled and #13640 was ruled not to re-open. + * The door every strategy merge uses: `ObjectQLStrategy.withReadScope` and + * `resolveFkAttr` (#13640), and — since #13926 — the `/analytics/sql` echo + * merge in `ObjectQLStrategy.generateSql` and + * `NativeSQLStrategy.applyReadScope`, where it runs AFTER + * {@link compileScopedFilterToSql} so the shapes that compiler already + * refuses keep their own #13571 messages. See the module header's #13640 + * section for the measured table, for why the `$nin` arm is + * polarity-independent while the `$in` arm is not, and for the bound this + * deliberately keeps (an emptied POSITIVE membership at even polarity is a + * ruled reduction and a live RLS composite depends on it). + * + * ⛔ Still not called by {@link compileScopedFilterToSql} itself. Folding it + * into the compiler would move the disposition of every direct consumer of + * that public export and pre-empt the polarity-aware compiler design the + * #13571 verdict ruled must come first; the guard stands at the MERGE SITES + * instead, so the compiler's own answers — the declared residue included — + * stay exactly where that verdict left them. * * @param scope the `StrategyContext.getReadScope` output, exactly as returned * @param objectName the object the scope was requested for — for the operator's diff --git a/packages/services/service-analytics/src/strategies/native-sql-strategy.ts b/packages/services/service-analytics/src/strategies/native-sql-strategy.ts index 6ba101c611..eea5e3f365 100644 --- a/packages/services/service-analytics/src/strategies/native-sql-strategy.ts +++ b/packages/services/service-analytics/src/strategies/native-sql-strategy.ts @@ -12,7 +12,7 @@ import { type NormalizedFilterNode, } from './filter-normalizer.js'; import { findCrossFieldComparand, findUninterpretableTemporalMember } from '../comparand-shape.js'; -import { compileScopedFilterToSql } from '../read-scope-sql.js'; +import { assertReadScopeCannotVacate, compileScopedFilterToSql } from '../read-scope-sql.js'; import { datasetInvalidError, invalidMemberError } from '../dataset-refusal.js'; import { likePattern, LIKE_ESCAPE_CHAR, asciiLowerSqlExpr, type LikeShape } from '../like-pattern.js'; import { nextUtcCalendarDay } from '@objectstack/core'; @@ -617,6 +617,19 @@ export class NativeSQLStrategy implements AnalyticsStrategy { const filter = ctx.getReadScope(objectName); if (filter === undefined || filter === null) return; const { sql, params: scopeParams } = compileScopedFilterToSql(filter, alias); + // [#13926] The #13640 door guard, at THIS strategy's merge site. This is + // not an echo: `execute()` runs this method's output through + // `ctx.executeRawSql`, so a scope the compiler lowers to a boolean + // constant (`$not` over `$in: []` and its measured siblings — the #13571 + // verdict's declared residue) used to become a real whole-table read for + // any producer the `getReadScope` spec contract admits (measured + // end-to-end in `read-scope-vacancy-three-faces.test.ts`; in-repo the + // RLS compiler's #13570 producer guard was the only protection). Runs + // per OBJECT — the base table and every joined hop — and AFTER the + // compiler on purpose: shapes the compiler already refuses keep their + // #13571 messages (door-distinguishable logs), and the guard closes + // exactly the shapes that compile-but-vacate. + assertReadScopeCannotVacate(filter, objectName); if (!sql) return; let i = 0; const rendered = sql.replace(/\?/g, () => { diff --git a/packages/services/service-analytics/src/strategies/objectql-strategy.ts b/packages/services/service-analytics/src/strategies/objectql-strategy.ts index baeb0aee2d..8429e3ed17 100644 --- a/packages/services/service-analytics/src/strategies/objectql-strategy.ts +++ b/packages/services/service-analytics/src/strategies/objectql-strategy.ts @@ -536,6 +536,18 @@ export class ObjectQLStrategy implements AnalyticsStrategy { const scope = ctx.getReadScope?.(tableName); if (scope != null) { const { sql: scopeSql, params: scopeParams } = compileScopedFilterToSql(scope, tableName); + // [#13926] The same door guard `execute()` trusts (`withReadScope`, + // #13640), at the ECHO's own merge — so one read scope gets ONE verdict + // on this strategy, whichever face the caller asked for. Without it the + // echo COMPILED the `$not`-over-`$in: []` family into a predicate that + // admits every row (`NOT (owner IS NOT NULL AND 1 = 0)`) while + // `execute()` refused the same scope — measured end-to-end in + // `read-scope-vacancy-three-faces.test.ts`. AFTER the compiler on + // purpose: the shapes the compiler already refuses (`$nin: []` at any + // depth, the bare array) keep their #13571 messages, so each door stays + // distinguishable in the operator's log, and the guard closes exactly + // the shapes that compile-but-vacate. + assertReadScopeCannotVacate(scope, tableName); if (scopeSql) { let i = 0; // `compileScopedFilterToSql` emits `?`; renumber into this builder's $N.