diff --git a/.changeset/objectql-read-scope-vacancy-refusal.md b/.changeset/objectql-read-scope-vacancy-refusal.md new file mode 100644 index 0000000000..7ceef3985d --- /dev/null +++ b/.changeset/objectql-read-scope-vacancy-refusal.md @@ -0,0 +1,5 @@ +--- +'@objectstack/service-analytics': patch +--- + +`ObjectQLStrategy` now refuses a read scope that does not bind, before handing it to the engine (`READ_SCOPE_COMPILE_FAILED` / 500). That strategy merges `StrategyContext.getReadScope` output straight into the `FilterCondition` it gives `engine.aggregate` and never reaches `compileScopedFilterToSql`, so the empty-`$nin` refusal that compiler gained guarded the NativeSQL path and the `/analytics/sql` echo only. Measured against a real engine, a non-RLS scope provider handing `{ f: { $nin: [] } }`, `{ $not: { f: { $in: [] } } }`, `{ $not: { f: [] } }` or `{ $not: { f: { $in: [], $ne: 'x' } } }` received the WHOLE TABLE on any query this strategy served; all four are now refused, at both engine-bound merges (the aggregate filter and the FK→attribute resolution). Deliberately unchanged: `$in: []` keeps its ruled constant-FALSE fold, so the RLS compiler's live composite — an emptied membership `$or`-ed beside an own-rows grant — still admits exactly the own rows; and the NativeSQL path and the SQL echo keep the disposition they already had. 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 new file mode 100644 index 0000000000..328f8b4286 --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/objectql-read-scope-vacancy-refusal.test.ts @@ -0,0 +1,417 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#13640] A read scope that does NOT BIND is refused before it reaches the + * ObjectQL engine — the third route to the rows, and the one no read-scope + * compiler ever saw. + * + * ## Why this file exists next to `read-scope-empty-nin-refusal.test.ts` + * + * That file (#13571) pins the empty-`$nin` refusal inside + * `compileScopedFilterToSql`, and its own header says what that refusal can + * cover: "a compile refusal there can only ever guard the NativeSQL path and + * the echo". `ObjectQLStrategy.execute()` reaches the same rows without + * calling that compiler at all — `withReadScope` ANDs the `getReadScope` + * output into the `FilterCondition` handed to `engine.aggregate`, and the + * ENGINE's own lowering answers it. `driver-sql` lowers `$nin: []` through + * `whereNotIn(field, [])` wrapped null-safe, which is constant TRUE (live + * in-repo pin of that semantics: `filter-normalizer-not-null-safe.test.ts`, + * "`{stage: {$nin: []}}` excludes nothing"). + * + * ## Why a NON-RLS provider, again + * + * `StrategyContext.getReadScope` is a spec contract + * (`packages/spec/src/contracts/analytics-service.ts` carries a hand-written + * example), so an out-of-repo provider is exactly what it exists for. In-repo + * the only producer is the RLS compiler and since PR #13570 its polarity-aware + * guard drops these shapes before emission — so an RLS-path regression would + * exercise a route that cannot produce them and would measure nothing. Every + * provider below is the contract filled BY HAND. + * + * ## The engine is real, because the claim is about a lowering + * + * `SqliteWasmDriver` (`driver-sqlite-wasm` over `driver-sql`) stands behind + * `executeAggregate`, which is what `engine.aggregate` reaches in production. + * A stub bridge would have made every expectation below a statement about the + * stub. MEASURED on the pre-fix tree, this fixture, this driver — the whole + * transcript is in the PR body: + * + * | read scope | pre-fix rows | now | + * |------------------------------------------------|--------------|----------| + * | `{ owner: { $nin: [] } }` | ALL THREE | REFUSED | + * | `{ $not: { owner: { $in: [] } } }` | ALL THREE | REFUSED | + * | `{ $not: { owner: [] } }` | ALL THREE | REFUSED | + * | `{ $not: { owner: { $in: [], $ne: 'u_other' } } }` | ALL THREE | REFUSED | + * | `{ $or: [{ $not: { owner: { $in: [] } } }, …] }`| ALL THREE | REFUSED | + * | `{ $not: { $not: { owner: { $nin: [] } } } }` | ALL THREE | REFUSED | + * | `{ owner: { $in: [] } }` | none | none | + * | `{ $or: [{ owner: { $in: [] } }, { owner: 'u_me' }] }` | own row | own row | + * | `{ $and: [{ owner: { $in: [] } }, { owner: 'u_me' }] }`| none | none | + * | `{ owner: 'u_me' }` | own row | own row | + * + * ## Both spellings, on purpose + * + * The card names two (`{ f: { $nin: [] } }` and `{ $not: { f: { $in: [] } } }`) + * and a pin on one leaves the other open. Measuring found two more that reach + * the engine and come back with the table — the bare `[]` comparand and the + * multi-key operator object — so they are pinned here too rather than left as + * a spelling nobody happened to try. + * + * ## What must NOT move, and how this file checks it + * + * The last block is an IMMOBILITY control over `compileScopedFilterToSql` + * itself. Both other routes (`NativeSQLStrategy.applyReadScope` and the + * `/analytics/sql` echo) consume that one function and have no other + * read-scope translation, so pinning its answers pins theirs — and it does so + * without a second copy of #13571's end-to-end fixture, which stays the + * end-to-end pin for the NativeSQL route. + */ + +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 { DatasetSchema } from '@objectstack/spec/ui'; + +import { AnalyticsService } from '../analytics-service.js'; +import { compileDataset } from '../dataset-compiler.js'; +import { ObjectQLStrategy } from '../strategies/objectql-strategy.js'; +import { compileScopedFilterToSql } from '../read-scope-sql.js'; + +const OBJECT = 'deal'; + +/** `owner` is NULL on r3 so a guard applied to the wrong polarity shows up. */ +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; + +interface WireBearingError extends Error { + code?: unknown; + status?: unknown; +} + +type Outcome = { refusal?: WireBearingError; admitted?: string[] }; + +describe('[#13640] ObjectQL ENGINE path — a read scope that does not bind is refused', () => { + let driver: SqliteWasmDriver; + let service: AnalyticsService; + /** Swapped per case; the spec contract filled by hand, never by the RLS compiler. */ + let readScope: unknown = null; + + 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], + // ObjectQL only — this is the route `compileScopedFilterToSql` never sees. + queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }), + // The production bridge is `engine.aggregate`; here it is the driver's own + // `aggregate`, which is what that engine call reaches. The filter travels + // VERBATIM — the claim under test is about the engine's lowering of it. + 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?.(); + }); + + /** Run one aggregate under `scope`; return the refusal or the admitted ids. */ + const outcome = async (scope: unknown): Promise => { + readScope = scope; + try { + const result = await service.query({ + cube: 'deals', + dimensions: ['id'], + measures: ['n'], + } as AnalyticsQuery); + return { admitted: result.rows.map((r) => String(r.id)).sort() }; + } catch (e) { + return { refusal: e as WireBearingError }; + } + }; + + /** Every refusal on this path carries the module's one envelope. */ + const expectRefused = (o: Outcome, messageFragment: string): void => { + expect(o.admitted).toBeUndefined(); + expect(o.refusal).toBeInstanceOf(Error); + expect(o.refusal?.code).toBe('READ_SCOPE_COMPILE_FAILED'); + expect(o.refusal?.status).toBe(500); + expect(String(o.refusal?.message)).toContain(messageFragment); + }; + + // ── The leak, both spellings the card names ─────────────────────────────── + + it('CONTROL: the fixture and the bridge are honest — no scope admits every row', async () => { + // Without this, every "refused" assertion below could be passing because + // the harness admits nothing in the first place. + expect((await outcome(null)).admitted).toEqual(ALL); + }); + + it('`{ owner: { $nin: [] } }` is REFUSED — pre-fix it admitted the whole table', async () => { + expectRefused(await outcome({ owner: { $nin: [] } }), 'has an empty $nin at owner.$nin'); + }); + + it('`{ $not: { owner: { $in: [] } } }` — the other spelling — is REFUSED too', async () => { + // The same constant by a different road: an empty membership matches + // nothing, so its negation matches everything. Pre-fix: all three rows. + expectRefused( + await outcome({ $not: { owner: { $in: [] } } }), + 'has an empty $in under negation at $not.owner.$in', + ); + }); + + it('the bare `[]` comparand under `$not` is the same emptied membership, and is REFUSED', async () => { + expectRefused(await outcome({ $not: { owner: [] } }), 'under negation at $not.owner: []'); + }); + + it('a sibling operator does not rescue it: `{ $in: [], $ne }` under `$not` is REFUSED', async () => { + // FALSE absorbs the AND a multi-key operator object forms, so this is the + // single-key spelling. Measured pre-fix: all three rows, not two. + expectRefused( + await outcome({ $not: { owner: { $in: [], $ne: 'u_other' } } }), + 'has an empty $in under negation at $not.owner.$in', + ); + }); + + it('nested inside a composite it is still REFUSED — a TRUE `$or` arm absorbs the scope', async () => { + expectRefused( + await outcome({ $or: [{ $not: { owner: { $in: [] } } }, { owner: 'u_me' }] }), + 'at $or[0].$not.owner.$in', + ); + }); + + it('an empty `$nin` is refused at EVERY polarity, matching the compiler arm', async () => { + // Deliberately polarity-INDEPENDENT: `compileScopedFilterToSql`'s `$nin` + // arm throws whatever encloses it (#13571), and a weaker rule here would + // give one read scope two answers depending on which strategy served the + // query. Under one `$not` the clause is merely narrowing; under two it is + // the whole-table fold again (measured pre-fix: all three rows). + expectRefused(await outcome({ $not: { owner: { $nin: [] } } }), 'at $not.owner.$nin'); + expectRefused(await outcome({ $not: { $not: { owner: { $nin: [] } } } }), 'at $not.$not.owner.$nin'); + expectRefused(await outcome({ $and: [{ owner: { $nin: [] } }, { owner: 'u_me' }] }), 'at $and[0].owner.$nin'); + }); + + // ── The asymmetry, and the bound the guard must not cross ──────────────── + + it('ASYMMETRY: `{ owner: { $in: [] } }` still means zero rows — unchanged', async () => { + // The ruled #5322 / #5243 identity. Narrowing at its own arm, the safe + // direction on a read scope; refusing it would 500 a scope that already + // denies correctly. + const { refusal, admitted } = await outcome({ owner: { $in: [] } }); + expect(refusal).toBeUndefined(); + expect(admitted).toEqual([]); + }); + + it('OVER-DENIAL CONTROL: the #13570-pinned RLS composite still admits exactly the own row', async () => { + // `{ $or: [{ owner: { $in: [] } }, { owner: 'u_me' }] }` is what + // `RLSCompiler.compileFilter` returns for an emptied membership set beside + // an own-rows grant, and it reaches THIS strategy too through + // `security.getReadFilter`. A uniform empty-membership throw reddens here — + // the availability regression #13571's verdict rejected, which this guard + // must not reintroduce one strategy over. + const { refusal, admitted } = await outcome({ $or: [{ owner: { $in: [] } }, { owner: 'u_me' }] }); + expect(refusal).toBeUndefined(); + expect(admitted).toEqual(['r1']); + }); + + it('OVER-DENIAL CONTROL: the denies-by-itself composite still denies, and is not a refusal', async () => { + const { refusal, admitted } = await outcome({ $and: [{ owner: { $in: [] } }, { owner: 'u_me' }] }); + expect(refusal).toBeUndefined(); + expect(admitted).toEqual([]); + }); + + it('ORDINARY CASE: a non-empty scope still filters exactly as before', async () => { + expect((await outcome({ owner: 'u_me' })).admitted).toEqual(['r1']); + expect((await outcome({ owner: { $in: ['u_me', 'u_other'] } })).admitted).toEqual(['r1', 'r2']); + // A non-empty EXCLUSION is untouched by the empty-`$nin` rule — including + // its NULL-safety, which is why r3 is in the answer. + expect((await outcome({ owner: { $nin: ['u_other'] } })).admitted).toEqual(['r1', 'r3']); + }); +}); + +// ── The second engine-bound merge on this strategy ───────────────────────── + +describe('[#13640] the FK→attribute resolution is the same door, and is guarded too', () => { + /** + * `resolveFkAttr` ANDs the REFERENCED object's own scope into a filter handed + * to `executeAggregate` — the second place on this strategy where a + * `getReadScope` output reaches an engine without meeting a compiler. A + * vacating scope there does not widen the aggregate: it widens the FK + * ATTRIBUTE MAP, so ids the policy hides resolve to their labels instead of + * bucketing under `(restricted)` (#3654's whole mechanism). + * + * A stub bridge is right HERE — the claim is about which merge site the + * guard stands at, and the lowering it protects against is already measured + * against a real driver in the first block. The dataset is the in-envelope + * cross-object grouping from `objectql-crossobj-expand.test.ts`, which is + * what makes the FK-expand path run at all. + */ + const dataset = DatasetSchema.parse({ + name: 'sales_by_account', + label: 'Sales by account', + object: 'opportunity', + include: ['account'], + dimensions: [{ name: 'region', field: 'account.region', type: 'string' }], + measures: [{ name: 'revenue', aggregate: 'sum', field: 'amount' }], + }); + + const serviceWithRefScope = (refScope: unknown) => { + const compiled = compileDataset(dataset); + return new AnalyticsService({ + cubes: [compiled.cube], + queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }), + getAllowedRelationships: () => compiled.allowedRelationships, + // The BASE object's scope is ordinary; only the REFERENCED object's + // vacates. So a failure here can only be the FK-resolution door. + getReadScope: (o: string) => + (o === 'opportunity' + ? { organization_id: 'org_A' } + : refScope) as unknown as FilterCondition | undefined, + executeAggregate: async (object: string) => + object === 'opportunity' + ? [ + { account: 'acc_w', revenue: 100 }, + { account: 'acc_hidden', revenue: 5 }, + ] + : [{ id: 'acc_w', region: 'West' }], + }); + }; + + const run = (refScope: unknown) => + serviceWithRefScope(refScope).query({ + cube: 'sales_by_account', + dimensions: ['region'], + measures: ['revenue'], + } as AnalyticsQuery); + + it('CONTROL: an ordinary referenced-object scope still resolves and still buckets the hidden ref', async () => { + const result = await run({ is_public: true }); + const byRegion = Object.fromEntries(result.rows.map((r) => [r.region, r.revenue])); + expect(byRegion).toEqual({ West: 100, '(restricted)': 5 }); + }); + + it('a vacating referenced-object scope is REFUSED at that merge too', async () => { + let err: WireBearingError | undefined; + try { + await run({ is_public: { $nin: [] } }); + } catch (e) { + err = e as WireBearingError; + } + expect(err).toBeInstanceOf(Error); + expect(err?.code).toBe('READ_SCOPE_COMPILE_FAILED'); + expect(err?.status).toBe(500); + // Named for the REFERENCED object, which is the whole point of the second + // call site: `withReadScope` never sees this scope. + expect(String(err?.message)).toContain('read scope for "account"'); + expect(String(err?.message)).toContain('has an empty $nin at is_public.$nin'); + }); +}); + +// ── Immobility: the two routes #13649 already settled ─────────────────────── + +describe('[#13640] IMMOBILITY — NativeSQL and the `/analytics/sql` echo 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 + // file proves the guard was ADDED at a new door rather than moved. + let err: WireBearingError | undefined; + try { + compileScopedFilterToSql({ owner: { $nin: [] } } as FilterCondition, 'deal'); + } catch (e) { + err = e as WireBearingError; + } + expect(err).toBeInstanceOf(Error); + expect(err?.code).toBe('READ_SCOPE_COMPILE_FAILED'); + expect(String(err?.message)).toContain('$nin for "owner" is empty'); + expect(String(err?.message)).not.toContain('read scope for'); + }); + + it('`compileScopedFilterToSql` still COMPILES `$not` over `$in: []` — #13571 declared residue, unmoved', async () => { + // ⚠️ An IMMOBILITY CONTROL, not a contract: #13571's verdict deliberately + // left this shape unasserted at this lowering, and closing it there needs + // the polarity-aware design that verdict asked for first. This block + // asserts only that #13640 did not move it. Whoever closes the residue + // should EXPECT this to redden and update it in that PR. + const { sql } = compileScopedFilterToSql( + { $not: { owner: { $in: [] } } } as FilterCondition, + 'deal', + ); + expect(sql).toContain('NOT'); + }); + + it('an ordinary scope still compiles to the same bound predicate', async () => { + const { sql, params } = compileScopedFilterToSql({ owner: 'u_me' } as FilterCondition, 'deal'); + expect(sql).toBe('"deal"."owner" = ?'); + expect(params).toEqual(['u_me']); + }); + + 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. + const ctx = { + getCube: (name: string) => (name === 'deals' ? CUBE : undefined), + queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }), + getReadScope: () => ({ owner: { $nin: [] } }) as unknown as FilterCondition, + } as unknown as StrategyContext; + + let err: WireBearingError | undefined; + try { + await new ObjectQLStrategy().generateSql( + { cube: 'deals', dimensions: ['id'], measures: ['n'] } as AnalyticsQuery, + ctx, + ); + } catch (e) { + err = e as WireBearingError; + } + expect(err).toBeInstanceOf(Error); + expect(String(err?.message)).toContain('$nin for "owner" is empty'); + }); + + it('the echo still renders an ordinary scope', async () => { + const ctx = { + getCube: (name: string) => (name === 'deals' ? CUBE : undefined), + queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }), + getReadScope: () => ({ owner: 'u_me' }) as unknown as FilterCondition, + } as unknown as StrategyContext; + + const { sql } = await new ObjectQLStrategy().generateSql( + { cube: 'deals', dimensions: ['id'], measures: ['n'] } as AnalyticsQuery, + ctx, + ); + expect(sql).toContain('"owner"'); + }); +}); diff --git a/packages/services/service-analytics/src/read-scope-sql.ts b/packages/services/service-analytics/src/read-scope-sql.ts index 79641a74e8..e00e960e9f 100644 --- a/packages/services/service-analytics/src/read-scope-sql.ts +++ b/packages/services/service-analytics/src/read-scope-sql.ts @@ -310,6 +310,71 @@ 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. + * + * ## The ObjectQL ENGINE path gets the SAME disposition, at a different door (#13640) + * + * Everything above describes what THIS compiler emits, and it reaches SQL on + * two routes only: `NativeSQLStrategy.applyReadScope` and the `/analytics/sql` + * echo. `ObjectQLStrategy.execute()` is a THIRD route to the same rows and it + * never calls this compiler — it ANDs the `getReadScope` output into the + * `FilterCondition` it hands `engine.aggregate`, and the ENGINE's lowering + * answers it. Measured there (`driver-sql` through `driver-sqlite-wasm`, three + * fixture rows, transcript in `objectql-read-scope-vacancy-refusal.test.ts`): + * + * | read scope from a non-RLS provider | rows the ENGINE admitted | + * |---------------------------------------------|--------------------------| + * | `{ owner: { $nin: [] } }` | ALL THREE | + * | `{ $not: { owner: { $in: [] } } }` | ALL THREE | + * | `{ $not: { owner: [] } }` | ALL THREE | + * | `{ $not: { owner: { $in: [], $ne: 'u' } } }`| ALL THREE | + * | `{ $or: [{ $not: { owner: { $in: [] } } }, …] }` | ALL THREE | + * | `{ owner: { $in: [] } }` | none | + * | `{ $or: [{ owner: { $in: [] } }, { owner: 'u_me' }] }` | the own row | + * + * So the widening this file closed at its own lowering was open one strategy + * over, on the route a spec-contract provider is most likely to be served by. + * {@link assertReadScopeCannotVacate} is the guard for that door — exported + * from THIS file, next to the compiler, deliberately: the disposition is one + * ruling and a second file holding a second copy of it is how the two answers + * drift apart. It refuses, in the same {@link readScopeCompileError} envelope, + * an EMPTIED MEMBERSHIP that does not bind: + * + * - `$nin: []` at ANY polarity — polarity-INDEPENDENT for one reason only: + * the `$nin` arm of {@link compileOperator} throws whatever encloses it, so + * matching it exactly is what keeps a scope from getting two answers + * depending on which strategy served the query. (At even polarity it is + * the whole-table fold; at odd polarity it is merely narrowing, and + * refusing it there costs no live traffic — the CEL lowering never emits + * `$nin` and #13570's producer guard drops the rest.) + * - `$in: []`, the bare `[]` comparand, and a multi-key operator object + * containing either, when the effective polarity is INVERTED. FALSE + * absorbs the AND its sibling operators form, so the multi-key spelling is + * the single-key one — measured above, not assumed. + * + * ⚠️ What it deliberately does NOT catch is the same bound #13571 drew: an + * emptied POSITIVE membership at even polarity stays a ruled #5322/#5243 + * reduction, because `{ $or: [{ owner: { $in: [] } }, { owner: 'u_me' }] }` is + * a LIVE RLS composite ("own rows keep flowing", #13570's + * `rls-empty-membership-polarity.test.ts`) and it reaches this strategy too. + * + * ⚠️ This does NOT re-decide #13571 and does not move either route above. The + * residue that verdict declared — `$not` over `$in: []` compiling to constant + * TRUE — is still exactly that HERE, in {@link compileScopedFilterToSql}, and + * the reason it could be closed at the other door without the ruled design is + * 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. + * + * ⚠️ 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 + * lives in `plugin-security` (a layer `service-analytics` must not depend on) + * and answers a different question — whether to DROP a degenerate policy + * before emitting it. This one answers whether a scope that arrived from any + * producer at all may be handed to an engine. */ const IDENT = /^[a-z_][a-z0-9_]*$/i; @@ -373,6 +438,126 @@ export function compileScopedFilterToSql( return { sql, params }; } +/** + * [#13640] What an emptied membership was found to be, so the refusal can say + * the true thing about it. `'nin'` is the empty EXCLUSION (constant TRUE at its + * own arm); `'negatedIn'` is an empty MEMBERSHIP under an odd number of `$not`s + * (constant FALSE at its own arm, therefore TRUE once inverted). + */ +type EmptyMembershipFinding = { path: string; kind: 'nin' | 'negatedIn' }; + +/** + * [#13640] Is this FIELD CONSTRAINT an emptied membership that fails to bind at + * `negated` polarity — and where? + * + * The three spellings are the ones measured to reach `engine.aggregate` and + * come back with the whole table (module header's table): + * + * - `{ $nin: [] }` — flagged at EVERY polarity, matching {@link compileOperator}'s + * own `$nin` arm, which throws whatever encloses it (#13571). Any weaker + * rule here would give one read scope two answers, chosen by strategy. + * - `{ $in: [] }` — flagged only when inverted; at even polarity it is the + * ruled #5322 reduction to FALSE and a LIVE RLS composite depends on it. + * - a bare `[]` comparand — the same emptied positive membership written + * without the operator (`{ owner: [] }`), and measured to behave as one. + * + * Sibling operators do not rescue the `$in` case: FALSE absorbs the AND a + * multi-key operator object forms, so `{ $in: [], $ne: 'u' }` is constant FALSE + * exactly like the single-key spelling (measured — the header's fourth row). + */ +function emptyMembershipFinding(spec: unknown, negated: boolean, path: string): EmptyMembershipFinding | null { + if (Array.isArray(spec)) { + return spec.length === 0 && negated ? { path: `${path}: []`, kind: 'negatedIn' } : null; + } + if (spec === null || typeof spec !== 'object') return null; + const rec = spec as Record; + if (Array.isArray(rec.$nin) && rec.$nin.length === 0) return { path: `${path}.$nin`, kind: 'nin' }; + if (Array.isArray(rec.$in) && rec.$in.length === 0 && negated) { + return { path: `${path}.$in`, kind: 'negatedIn' }; + } + return null; +} + +/** + * [#13640] Walk the scope tree tracking EFFECTIVE polarity and return the first + * emptied membership that does not bind. + * + * A walk, never a reduction — which is precisely why this can be polarity-aware + * where {@link compileScopedFilterToSql} could not be (#13571's verdict): there + * is no constant being folded here, so nothing interacts with the #5322 + * `$not`-over-identity reductions and none of them has to be re-ruled. + * + * `$and` and `$or` arms are both walked at the CALLER's polarity and a finding + * in either is returned. That is not sloppiness about which composite widens: + * as an `$or` arm a constant-TRUE clause makes the whole scope allow-all, and + * as an `$and` arm the restriction its author wrote has silently evaporated + * while its siblings carry on — degenerate either way, exactly the reading + * #13570's producer-side guard takes for the same two shapes. + */ +function findEmptyMembership(node: unknown, negated: boolean, path: string): EmptyMembershipFinding | null { + if (node === null || typeof node !== 'object' || Array.isArray(node)) return null; + const rec = node as Record; + // A BARE operator object (an emptied membership with no field key) is not a + // shape any in-repo producer emits, but `getReadScope` is a spec contract and + // this guard's contract is over the FilterCondition SHAPE — so it is judged + // rather than walked past. (`compileNode` refuses it outright on the other + // route, with its own unsupported-top-level-operator message.) + const bare = emptyMembershipFinding(rec, negated, path.length > 0 ? path : ''); + if (bare) return bare; + for (const [key, value] of Object.entries(rec)) { + const here = path.length > 0 ? `${path}.${key}` : key; + if (key === '$not') { + const found = findEmptyMembership(value, !negated, here); + if (found) return found; + } else if (key === '$and' || key === '$or') { + if (!Array.isArray(value)) continue; + for (let i = 0; i < value.length; i++) { + const found = findEmptyMembership(value[i], negated, `${here}[${i}]`); + if (found) return found; + } + } else if (!key.startsWith('$')) { + const found = emptyMembershipFinding(value, negated, here); + if (found) return found; + } + } + return null; +} + +/** + * [#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. + * + * @param scope the `StrategyContext.getReadScope` output, exactly as returned + * @param objectName the object the scope was requested for — for the operator's + * log only; like every message in this module it is withheld from the + * response by the `READ_SCOPE_COMPILE_FAILED` / 500 declaration. + */ +export function assertReadScopeCannotVacate(scope: unknown, objectName: string): void { + const found = findEmptyMembership(scope, false, ''); + if (found === null) return; + if (found.kind === 'nin') { + throw readScopeCompileError( + `[read-scope-sql] read scope for "${objectName}" has an empty $nin at ${found.path} — an empty exclusion excludes nothing, ` + + `so the engine lowers that clause to constant TRUE and the scope does not bind as written. Refused at every ` + + `polarity, matching this module's own $nin arm (fail-closed).`, + ); + } + throw readScopeCompileError( + `[read-scope-sql] read scope for "${objectName}" has an empty $in under negation at ${found.path} — an empty membership matches nothing, ` + + `so its negation matches every row and the read scope admits the whole table (fail-closed).`, + ); +} + /** * Compile a child node into its OWN bind buffer. * diff --git a/packages/services/service-analytics/src/strategies/objectql-strategy.ts b/packages/services/service-analytics/src/strategies/objectql-strategy.ts index a1e0a773ed..baeb0aee2d 100644 --- a/packages/services/service-analytics/src/strategies/objectql-strategy.ts +++ b/packages/services/service-analytics/src/strategies/objectql-strategy.ts @@ -16,7 +16,7 @@ import { type NormalizedFilterNode, } from './filter-normalizer.js'; import { findCrossFieldComparand, isFieldReference } from '../comparand-shape.js'; -import { compileScopedFilterToSql } from '../read-scope-sql.js'; +import { assertReadScopeCannotVacate, compileScopedFilterToSql } from '../read-scope-sql.js'; import { invalidMemberError } from '../dataset-refusal.js'; import { likePattern, LIKE_ESCAPE_CHAR, asciiLowerSqlExpr, type LikeShape } from '../like-pattern.js'; import { nextUtcCalendarDay } from '@objectstack/core'; @@ -601,6 +601,19 @@ export class ObjectQLStrategy implements AnalyticsStrategy { if (typeof ctx.getReadScope !== 'function') return userFilter; const scope = ctx.getReadScope(objectName); if (scope === undefined || scope === null) return userFilter; + // [#13640] The mechanical guard between a spec-contract scope PRODUCER and + // the engine LOWERING. It stands here, at the merge boundary, because this + // is the last place the scope is still a distinguishable object: one line + // down it is `$and`-composed with the caller's own filter, and after that + // no consumer can tell whose half a clause came from. It is also the only + // place on this route where anything reads the scope at all — `execute()` + // hands the merged tree straight to `engine.aggregate`, so a scope that + // lowers to a boolean constant used to reach the driver unexamined and + // come back with the whole table (measured — `read-scope-sql.ts`'s #13640 + // section carries the table). NativeSQL and the `/analytics/sql` echo get + // the same disposition from `compileScopedFilterToSql` itself (#13571); + // this is the same ruling at the door that compiler never sees. + assertReadScopeCannotVacate(scope, objectName); const scopeFilter = markFilterSubtreeProvenance(scope as Record, 'policy'); if (!userFilter) return scopeFilter; return { $and: [userFilter, scopeFilter] }; @@ -1076,6 +1089,14 @@ export class ObjectQLStrategy implements AnalyticsStrategy { // `idFilter` is this method's own plumbing, not the caller's text — it // stays unmarked, which withholds, and that is correct for a filter no // author typed. + // [#13640] The SECOND engine-bound merge on this strategy, and the same + // door: the referenced object's own scope is `$and`-ed into a filter handed + // to `executeAggregate` without ever meeting `compileScopedFilterToSql`. A + // vacating scope here does not widen the aggregate — it widens the FK + // ATTRIBUTE MAP, so ids the policy hides resolve to their labels instead of + // landing in the RESTRICTED bucket. Guarded before the mark, so a refused + // scope is never stamped as vouched-for policy content. + if (scope != null) assertReadScopeCannotVacate(scope, refObject); if (scope != null) markFilterSubtreeProvenance(scope, 'policy'); const filter = scope != null ? { $and: [idFilter, scope] } : idFilter; const rows = await ctx.executeAggregate(refObject, {