From 1880b33469ad63cd9a23b1e63f93e2c2815c89f3 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 04:49:37 +0000 Subject: [PATCH] fix(service-analytics): refuse an empty $nin on the read-scope lowering instead of folding it to constant TRUE (#13571) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An emptied exclusion folded to '1 = 1' — constant TRUE — which vacates the whole read scope: every row admitted, no $not needed, on the lowering where a wrong answer is ADR-0021 scope over-reach. It now throws in the module's one refusal envelope (READ_SCOPE_COMPILE_FAILED / 500), like the arity check one line above. Deliberately asymmetric (domain:services ruling, 2026-08-31): $in: [] keeps its ruled #5322/#5243 constant-FALSE fold. That fold is narrowing at its own arm and load-bearing — the RLS compiler deliberately emits an emptied positive membership inside composites (PR #13570's 'own rows keep flowing' pin), and that filter reaches this compiler through security.getReadFilter. A uniform throw was measured and rejected: it would 500 every analytics query for any user whose membership set resolves empty beside an own-rows grant. $nin: [] has zero producers (the CEL lowering never emits $nin; the #13570 guard drops even-polarity empty-$nin policies), so this refusal costs no live traffic. Includes a non-RLS getReadScope provider control (the spec contract filled by hand) pinning refusal post-fix, and the over-denial control pinning that the #13570 composite still compiles and still admits exactly the own row. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs --- .changeset/read-scope-empty-nin-refusal.md | 5 + .../__tests__/comparand-shape-refusal.test.ts | 25 ++- .../read-scope-empty-nin-refusal.test.ts | 170 ++++++++++++++++++ .../read-scope-not-null-safe.test.ts | 24 ++- .../read-scope-refusal-envelope.test.ts | 34 +++- .../service-analytics/src/read-scope-sql.ts | 45 ++++- 6 files changed, 289 insertions(+), 14 deletions(-) create mode 100644 .changeset/read-scope-empty-nin-refusal.md create mode 100644 packages/services/service-analytics/src/__tests__/read-scope-empty-nin-refusal.test.ts diff --git a/.changeset/read-scope-empty-nin-refusal.md b/.changeset/read-scope-empty-nin-refusal.md new file mode 100644 index 0000000000..ac3b6e98bc --- /dev/null +++ b/.changeset/read-scope-empty-nin-refusal.md @@ -0,0 +1,5 @@ +--- +'@objectstack/service-analytics': patch +--- + +The analytics read-scope compiler (`read-scope-sql.ts`) now refuses an empty `$nin` (`READ_SCOPE_COMPILE_FAILED` / 500) instead of folding it to constant TRUE. An emptied exclusion ("NOT IN () excludes nothing") vacated the whole read scope — every row admitted — on the ADR-0021 lowering, where a widening is scope over-reach; no in-repo producer can emit the shape (the CEL lowering never emits `$nin`, and the RLS guard drops even-polarity empty-`$nin` policies upstream), so the refusal costs no live traffic. Deliberately asymmetric: `$in: []` keeps its ruled constant-FALSE fold (#5322/#5243), which the RLS compiler's inert positive composite — an emptied membership `$or`-ed beside an own-rows grant — depends on. diff --git a/packages/services/service-analytics/src/__tests__/comparand-shape-refusal.test.ts b/packages/services/service-analytics/src/__tests__/comparand-shape-refusal.test.ts index b8abed50cd..b6134e3ab4 100644 --- a/packages/services/service-analytics/src/__tests__/comparand-shape-refusal.test.ts +++ b/packages/services/service-analytics/src/__tests__/comparand-shape-refusal.test.ts @@ -266,11 +266,30 @@ describe('[#5234] the read-scope lowering refuses the same two shapes, fail-clos expect(scope({ name: { $startsWith: '_admin' } }).params).toEqual(['\\_admin%', '\\']); }); - it('an empty `$in` / `$nin` still lowers to its boolean constant, not a refusal', () => { + it('an empty `$in` still lowers to its FALSE constant, not a refusal', () => { // The member scan runs AFTER the arity identities (#5134), so the empty - // list keeps compiling to `1 = 0` / `1 = 1` rather than becoming an error. + // INCLUSION keeps compiling to `1 = 0` rather than becoming an error. It + // KEEPS its #5322/#5243 reduction because that constant is FALSE — + // narrowing at this arm — and because a live producer depends on it: the + // RLS compiler deliberately emits `$in: []` at positive polarity inside + // composites (#13570's "own rows keep flowing" pin), and that filter + // reaches this compiler through `security.getReadFilter`. expect(scope({ status: { $in: [] } }).sql).toContain('1 = 0'); - expect(scope({ status: { $nin: [] } }).sql).toContain('1 = 1'); + }); + + it('an empty `$nin` is REFUSED — its constant is TRUE, which vacates the scope (#13571)', () => { + // Deliberately ASYMMETRIC with the `$in` case above, and not an + // oversight: the #5322 boundary is "shape errors throw, boolean + // identities reduce", and `$nin: []` sits on the THROW side because its + // faithful reduction is constant TRUE — a read scope silently widened to + // every row, the exact thing the module header forbids. No in-repo + // producer can emit the shape (the CEL lowering never emits `$nin`; + // #13570's guard drops even-polarity empty-`$nin` policies), so the + // refusal costs no live traffic. See read-scope-sql.ts's #13571 section. + const err = refusalOf(() => scope({ status: { $nin: [] } })); + expect(err.code).toBe('READ_SCOPE_COMPILE_FAILED'); + expect(err.status).toBe(500); + expect(err.message).toContain('$nin for "status" is empty'); }); }); }); diff --git a/packages/services/service-analytics/src/__tests__/read-scope-empty-nin-refusal.test.ts b/packages/services/service-analytics/src/__tests__/read-scope-empty-nin-refusal.test.ts new file mode 100644 index 0000000000..5bdfe160f8 --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/read-scope-empty-nin-refusal.test.ts @@ -0,0 +1,170 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#13571] The empty-`$nin` compile refusal, exercised on the path that made it + * matter — a NON-RLS `getReadScope` provider — plus the over-denial control + * that bounds the fix. + * + * ## Why a non-RLS provider, and not an RLS regression + * + * In-repo, the only scope producer is the RLS compiler, and since PR #13570 its + * polarity-aware guard drops every emptied-membership shape at widening + * polarity before it is emitted — so an RLS-path regression for `$nin: []` + * would exercise a route that cannot reach this compiler at all and would test + * nothing. But `StrategyContext.getReadScope` is a spec contract + * (`packages/spec/src/contracts/analytics-service.ts` — its doc carries a + * hand-written example), fillable by ANY provider. The provider below is that + * contract filled by hand, which is exactly the surface the #13571 card named. + * + * ## The two controls + * + * 1. **Refusal (the fix).** A provider handing `{ owner: { $nin: [] } }` used + * to get a scope clause of constant TRUE (`1 = 1`) — on the read-scope + * lowering, the WHOLE TABLE, an ADR-0021 over-reach. It now gets the + * module's one refusal envelope: `READ_SCOPE_COMPILE_FAILED` / 500. + * MEASURED pre-fix (this file run against the pre-#13571 compiler, see the + * PR): the same case admitted every fixture row. + * + * 2. **Over-denial (the bound — the reason #13571 is NOT a uniform throw).** + * The RLS compiler deliberately emits an emptied POSITIVE membership inside + * a composite — `{ $or: [{ owner: { $in: [] } }, { owner: 'u_me' }] }`, + * pinned by #13570's `rls-empty-membership-polarity.test.ts` as "own rows + * keep flowing" — and that filter reaches this compiler through + * `security.getReadFilter`. The refusal must NOT catch it: the scope still + * compiles and still admits exactly the own row. A uniform throw at both + * arms fails this block, which is the availability regression the #13571 + * verdict rejected. + * + * `$in: []` under `$not` from a non-RLS provider (constant TRUE via inversion) + * is the verdict's DECLARED residue, deliberately not asserted here either way + * as a contract — `read-scope-not-null-safe.test.ts` pins its current + * behaviour next to the ruling's reasoning. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import type { Cube } from '@objectstack/spec/data'; +import type { AnalyticsQuery, StrategyContext } from '@objectstack/spec/contracts'; + +import { NativeSQLStrategy } from '../strategies/native-sql-strategy.js'; + +const FIXTURE = [ + { id: 'r1', owner: 'u_me' }, + { id: 'r2', owner: 'u_other' }, + { id: 'r3', owner: null }, +]; + +const CUBE: Cube = { + name: 'deals', + title: 'Deals', + sql: 'deal', + measures: { total: { name: 'total', label: 'Total', type: 'count', sql: '*' } }, + dimensions: Object.fromEntries( + ['id', 'owner'].map((n) => [n, { name: n, label: n, type: 'string', sql: n }]), + ), + public: false, +} as unknown as Cube; + +/** Point sql.js at the `.wasm` shipped inside its own package (Node-safe). */ +async function locateWasm(): Promise<((file: string) => string) | undefined> { + try { + const { createRequire } = await import('node:module'); + const require = createRequire(import.meta.url); + const pkgJsonPath = require.resolve('sql.js/package.json'); + const { dirname, join } = await import('node:path'); + const dir = dirname(pkgJsonPath); + return (file: string) => join(dir, 'dist', file); + } catch { + return undefined; + } +} + +describe('[#13571] empty `$nin` on the read-scope lowering — non-RLS provider control', () => { + let db: any; + + beforeAll(async () => { + const mod: any = await import('sql.js'); + const initSqlJs = mod.default ?? mod; + const locateFile = await locateWasm(); + const SQL = await initSqlJs(locateFile ? { locateFile } : undefined); + + db = new SQL.Database(); + db.run(`CREATE TABLE "deal" ("id" TEXT PRIMARY KEY, "owner" TEXT);`); + const insert = db.prepare(`INSERT INTO "deal" ("id","owner") VALUES (?,?)`); + for (const r of FIXTURE) insert.run([r.id, r.owner]); + insert.free(); + }); + + afterAll(() => { + db?.close(); + }); + + /** + * A `StrategyContext` whose `getReadScope` is filled BY HAND — the spec + * contract's own authoring mode, and deliberately not the RLS compiler. + */ + const ctxWithScope = (scope: unknown): StrategyContext => + ({ + getCube: (name: string) => (name === 'deals' ? CUBE : undefined), + queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }), + getReadScope: () => scope, + executeRawSql: async (_object: string, sql: string, params: unknown[]) => { + const stmt = db.prepare(sql.replace(/\$\d+/g, '?')); + stmt.bind(params as any[]); + const out: Record[] = []; + while (stmt.step()) out.push(stmt.getAsObject()); + stmt.free(); + return out; + }, + }) as StrategyContext; + + const QUERY: AnalyticsQuery = { + cube: 'deals', + measures: ['total'], + dimensions: ['id'], + timezone: 'UTC', + } as AnalyticsQuery; + + /** Run the query under `scope`; return either the refusal or the admitted ids. */ + const outcome = async ( + scope: unknown, + ): Promise<{ refusal?: Error & { code?: unknown; status?: unknown }; admitted?: string[] }> => { + try { + const result = await new NativeSQLStrategy().execute(QUERY, ctxWithScope(scope)); + return { admitted: result.rows.map((r) => String(r.id)).sort((x, y) => x.localeCompare(y)) }; + } catch (e) { + return { refusal: e as Error & { code?: unknown; status?: unknown } }; + } + }; + + it('a provider handing `{ owner: { $nin: [] } }` is REFUSED in the module envelope — it used to get the whole table', async () => { + const { refusal, admitted } = await outcome({ owner: { $nin: [] } }); + // Pre-#13571 this assertion's diff read `admitted: ['r1','r2','r3']` — the + // whole fixture, from a scope clause of constant TRUE. + expect(admitted).toBeUndefined(); + expect(refusal).toBeInstanceOf(Error); + expect(refusal?.code).toBe('READ_SCOPE_COMPILE_FAILED'); + expect(refusal?.status).toBe(500); + expect(String(refusal?.message)).toContain('$nin for "owner" is empty'); + }); + + it('OVER-DENIAL CONTROL: the #13570-pinned RLS composite still compiles and 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 ("own rows keep flowing"), and it arrives here through + // `security.getReadFilter`. The empty-`$nin` refusal must not touch it: + // this block red under a uniform empty-membership throw is the + // availability regression the #13571 verdict exists to avoid. + const { refusal, admitted } = await outcome({ $or: [{ owner: { $in: [] } }, { owner: 'u_me' }] }); + expect(refusal).toBeUndefined(); + expect(admitted).toEqual(['r1']); + }); + + it('the composite denies-by-itself shape stays working too: a lone empty `$in` arm inside `$and`', async () => { + // The other composite #13570's guard deliberately passes through: as an + // `$and` arm the emptied positive membership is constant FALSE — the whole + // scope denies. Zero rows, not a refusal and not the whole table. + const { refusal, admitted } = await outcome({ $and: [{ owner: { $in: [] } }, { owner: 'u_me' }] }); + expect(refusal).toBeUndefined(); + expect(admitted).toEqual([]); + }); +}); diff --git a/packages/services/service-analytics/src/__tests__/read-scope-not-null-safe.test.ts b/packages/services/service-analytics/src/__tests__/read-scope-not-null-safe.test.ts index 7616baebe9..cbcd23640b 100644 --- a/packages/services/service-analytics/src/__tests__/read-scope-not-null-safe.test.ts +++ b/packages/services/service-analytics/src/__tests__/read-scope-not-null-safe.test.ts @@ -347,9 +347,29 @@ describe('[#5297] read-scope `$not` — boolean identities and NULL safety', () expect(ids({ $not: { stage: { $ne: null } } })).toEqual(['3', '4']); }); - it('an empty `$in` / `$nin` under a `$not` keeps its constant value', () => { + it('an empty `$in` under a `$not` keeps its constant value; an empty `$nin` refuses before `$not` matters (#13571)', () => { + // `$in: []` keeps its #5322/#5243 reduction — constant FALSE, total, no + // guard — and the negation flips it to TRUE: every row. That widened + // composition is the #13571 verdict's DECLARED residue for a non-RLS + // producer (the in-repo RLS compiler cannot emit the shape — #13570's + // polarity guard drops it upstream); closing it is a ruled follow-up + // design, not an edit to this pin. expect(ids({ $not: { stage: { $in: [] } } })).toEqual(ALL); - expect(ids({ $not: { stage: { $nin: [] } } })).toEqual([]); + // `$nin: []` no longer HAS a constant to keep: its reduction is TRUE — + // scope-vacating on its own — so `compileOperator` refuses it whatever + // the polarity above it. Deliberately asymmetric with the `$in` line + // above ("shape errors throw, boolean identities reduce" is the #5322 + // boundary, and a scope-vacating reduction is on the THROW side) — see + // read-scope-sql.ts's #13571 header section. + let refusal: (Error & { code?: unknown; status?: unknown }) | undefined; + try { + ids({ $not: { stage: { $nin: [] } } }); + } catch (e) { + refusal = e as Error & { code?: unknown; status?: unknown }; + } + expect(refusal?.code).toBe('READ_SCOPE_COMPILE_FAILED'); + expect(refusal?.status).toBe(500); + expect(String(refusal?.message)).toContain('$nin for "stage" is empty'); }); }); diff --git a/packages/services/service-analytics/src/__tests__/read-scope-refusal-envelope.test.ts b/packages/services/service-analytics/src/__tests__/read-scope-refusal-envelope.test.ts index faf0de7cc4..41a3835aee 100644 --- a/packages/services/service-analytics/src/__tests__/read-scope-refusal-envelope.test.ts +++ b/packages/services/service-analytics/src/__tests__/read-scope-refusal-envelope.test.ts @@ -93,7 +93,7 @@ function refusalFor(filter: unknown, alias = 'crm_opportunity'): Refusal | undef /** * Every refusing site in `read-scope-sql.ts`, in source order. * - * FOURTEEN rows over TWELVE throw sites: TWO sites are each reached by two + * FIFTEEN rows over THIRTEEN throw sites: TWO sites are each reached by two * triggers, and every trigger is listed on purpose. * * - `quoteIdent`, with two `kind` values. That alias-vs-field split was option @@ -208,14 +208,26 @@ const REFUSALS: Array<{ sensitive: 'region_code', }, { - name: '⑬ $between without [min,max]', + // [#13571] The empty EXCLUSION refuses; the empty INCLUSION keeps its + // constant — see the ACCEPTED table's "#5243" row. Deliberate asymmetry + // ("shape errors throw, boolean identities reduce" — #5322; a reduction to + // constant TRUE vacates the scope, so it is on the throw side), not an + // oversight: read-scope-sql.ts's #13571 header section carries the ruling. + name: '⑬ $nin with an EMPTY array', + site: 'compileOperator: empty $nin vacates the scope', + filter: { region_code: { $nin: [] } }, + message: /\$nin for "region_code" is empty — an empty exclusion excludes nothing and would compile the read scope to constant TRUE \(fail-closed\)/, + sensitive: 'region_code', + }, + { + name: '⑭ $between without [min,max]', site: 'compileOperator: $between needs [min,max]', filter: { credit_limit: { $between: [10] } }, message: /\$between for "credit_limit" needs \[min,max\] \(fail-closed\)/, sensitive: 'credit_limit', }, { - name: '⑭ unsupported operator', + name: '⑮ unsupported operator', site: 'compileOperator: unsupported operator', filter: { owner_email: { $regex: 'admin@' } }, message: /unsupported operator "\$regex" on "owner_email" \(fail-closed\)/, @@ -244,6 +256,11 @@ const ACCEPTED: Array<{ name: string; filter: unknown; sql: string; params: unkn params: ['emea', 'apac'], }, { + // [#13571] STAYS accepted while the empty `$nin` refuses (REFUSALS ⑬): + // this constant is FALSE — narrowing at its own arm — and the RLS compiler + // deliberately emits the shape at positive polarity inside composites + // (#13570's "own rows keep flowing" pin), so refusing it here would 500 a + // live, ruled-correct scope. The asymmetry is the #13571 ruling itself. name: 'an empty $in as the FALSE constant (#5243)', filter: { region_code: { $in: [] } }, sql: '1 = 0', @@ -306,13 +323,14 @@ describe('[#5367] every read-scope refusal carries the ADR-0112 envelope (READ_S // #5352's lesson, stated as a guard: seven of `filter-normalizer.ts`'s nine // sites carrying an envelope was indistinguishable from none of them at the // HTTP boundary, because the commonest input hit one of the two bare ones. - // Fourteen inputs over the module's TWELVE throw sites (see the table's note - // on the two sites with two triggers each), and every one of them enveloped. - // [#6125] added the eleventh site, [#6387] the twelfth; these two numbers + // Fifteen inputs over the module's THIRTEEN throw sites (see the table's + // note on the two sites with two triggers each), and every one of them + // enveloped. [#6125] added the eleventh site, [#6387] the twelfth, and + // [#13571] the thirteenth (the empty-`$nin` refusal); these two numbers // are the ratchet that makes a future unenveloped `throw` fail HERE instead // of at an HTTP boundary. - expect(REFUSALS).toHaveLength(14); - expect(new Set(REFUSALS.map((c) => c.site)).size).toBe(12); + expect(REFUSALS).toHaveLength(15); + expect(new Set(REFUSALS.map((c) => c.site)).size).toBe(13); for (const c of REFUSALS) { expect(refusalFor(c.filter, c.alias)?.code, `${c.site} is still bare`).toBe('READ_SCOPE_COMPILE_FAILED'); } diff --git a/packages/services/service-analytics/src/read-scope-sql.ts b/packages/services/service-analytics/src/read-scope-sql.ts index 2c6c3de660..79641a74e8 100644 --- a/packages/services/service-analytics/src/read-scope-sql.ts +++ b/packages/services/service-analytics/src/read-scope-sql.ts @@ -274,6 +274,42 @@ import { * no 4xx (option B reintroduces both defects #5367 closed). The paragraph above * beginning "⚠️ Deliberately NOT a 4xx of any flavour" is that ruling's own text * and is not to be rewritten. + * + * ## An EMPTIED `$nin` is refused, not folded (#13571, `domain:services` ruling 2026-08-31) + * + * FOURTEEN refusing sites. `$nin: []` used to fold to `'1 = 1'` — constant + * TRUE — which on this lowering VACATES the read scope: every row admitted, + * with no `$not` required. That is the first bullet above ("a read-scope + * predicate must never be silently dropped") in its most literal form, so the + * empty exclusion now throws, in the same envelope as the other thirteen. + * + * ⚠️ DELIBERATELY ASYMMETRIC with `$in: []`, which KEEPS folding to + * {@link FALSE_CLAUSE}. The #5322 boundary (section above) is "shape errors + * THROW, boolean identities REDUCE"; both empty memberships have ruled + * reductions, but they land on opposite constants. `$in: []` reduces to + * constant FALSE — narrowing at its own arm, the safe direction on a read + * scope — and is LOAD-BEARING: the RLS compiler deliberately emits it at + * positive polarity inside composites (`{ $or: [{ owner: { $in: [] } }, + * { owner: 'u_me' }] }` — an emptied membership beside an own-rows grant, + * pinned by #13570's `rls-empty-membership-polarity.test.ts` as "own rows keep + * flowing"), and that filter reaches this compiler through + * `security.getReadFilter`. `$nin: []` reduces to constant TRUE — scope- + * vacating — and has ZERO producers: the CEL lowering never emits `$nin`, and + * #13570's guard drops even-polarity empty-`$nin` policies before they are + * emitted. Refusing it therefore costs no live traffic and closes the one + * single-step widening fold this compiler had. A uniform throw at both arms + * was measured and REJECTED (#13571 verdict): it would have 500'd the pinned + * live composite above on every analytics query for any user whose membership + * set resolves empty beside an own-rows grant. + * + * ⚠️ Declared residue, ruled follow-up — NOT covered here: `$not` over + * `$in: []` still compiles to constant TRUE at this lowering for a producer + * that is not the (#13570-guarded) RLS compiler. Closing that needs a + * polarity-aware design whose interaction with the #5322 `$not`-over-identity + * reductions is ruled first — a naive "refuse NOT of the FALSE constant" + * 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. */ const IDENT = /^[a-z_][a-z0-9_]*$/i; @@ -948,7 +984,14 @@ function compileOperator(col: string, op: string, val: unknown, field: string, p } case '$nin': { if (!Array.isArray(val)) throw readScopeCompileError(`[read-scope-sql] $nin for "${field}" needs an array (fail-closed).`); - if (val.length === 0) return '1 = 1'; // NOT IN () excludes nothing + // [#13571] An EMPTIED exclusion is refused, not folded. Its faithful + // reduction is constant TRUE ("NOT IN () excludes nothing"), which on + // this lowering VACATES the whole read scope — every row admitted, no + // `$not` needed. Deliberately ASYMMETRIC with `$in: []` → FALSE_CLAUSE + // two arms above: that fold is a ruled #5322/#5243 identity, narrowing at + // its own arm, and a live RLS composite depends on it. Read the module + // header's #13571 section before "harmonising" the two arms. + if (val.length === 0) throw readScopeCompileError(`[read-scope-sql] $nin for "${field}" is empty — an empty exclusion excludes nothing and would compile the read scope to constant TRUE (fail-closed).`); assertCompilableMembers(op, field, val); // [#5298] NULL-safe: "not among this list" holds vacuously for a value // that is not there.