diff --git a/.changeset/19975-read-scope-eq-array.md b/.changeset/19975-read-scope-eq-array.md new file mode 100644 index 00000000000..533cfd51e7f --- /dev/null +++ b/.changeset/19975-read-scope-eq-array.md @@ -0,0 +1,17 @@ +--- +"@objectstack/service-analytics": minor +--- + +fix(service-analytics)!: the read-scope compiler refuses a list under `$eq` instead of binding it (#19975) + +Clause-②: no (narrowing) + + + +**BREAKING**: this narrows what `compileScopedFilterToSql`, exported from `@objectstack/service-analytics`, accepts. A read scope carrying `{ field: { $eq: [...] } }` compiled before this change and is refused after it. It ships as `minor` under the launch-window convention for accept-set narrowings. The remedy is `{ field: { $in: [...] } }` for "one of these values". + +`compileScopedFilterToSql` compiles a row-level read scope into the SQL the analytics NativeSQL path and the `/analytics/sql` echo run. It already refused a list in the implicit equality slot (`{ field: [...] }`). The explicit spelling, `{ field: { $eq: [...] } }`, was compiled to an equality with the whole list bound as one parameter, so what the scope selected depended on how the executing database read a list, not on what the scope said. + +It is now refused, at any depth under `$and` / `$or` / `$not`, with the envelope every other refusal of this compiler carries: `READ_SCOPE_COMPILE_FAILED` / 500, with the message kept for the server log. This applies ruling 乙 of #19757, which the shared comparand-shape face in `@objectstack/spec` already enforces, to a compiler that face never sees. `$ne` with a list is not part of that ruling and is not judged here. + +No policy authored as metadata produces this shape. The refusal therefore reaches only a host-supplied `getReadScope` or a direct caller of `compileScopedFilterToSql`. A scalar, `null` or a `Date` under `$eq` compiles exactly as before, and a `{ $field }` reference there keeps its existing answer. diff --git a/packages/services/service-analytics/src/__tests__/comparand-door-single-source.test.ts b/packages/services/service-analytics/src/__tests__/comparand-door-single-source.test.ts index 86119bc53f0..74f6c97c21f 100644 --- a/packages/services/service-analytics/src/__tests__/comparand-door-single-source.test.ts +++ b/packages/services/service-analytics/src/__tests__/comparand-door-single-source.test.ts @@ -150,10 +150,15 @@ const MATRIX: readonly Row[] = [ bindable: false, renderable: false, whereLike: REFUSED_WHERE, whereIn: REFUSED_WHERE, whereEq: OK, scopeLike: REFUSED_SCOPE, scopeIn: REFUSED_SCOPE, scopeEq: OK }, + // [#19975] One cell of this row moved AFTER the #8186 measurement, on + // purpose: ruling 乙 (#19757) refuses a list in the equality slot, and the + // read-scope lowering now refuses it under `$eq` instead of binding the list + // as one parameter (`read-scope-eq-array-refusal.test.ts`). The `where` + // door's `$eq` cell is not that change's and keeps its measured answer. { label: 'array', value: ['al', 'be'], bindable: false, renderable: false, whereLike: REFUSED_WHERE, whereIn: REFUSED_WHERE, whereEq: OK, - scopeLike: REFUSED_SCOPE, scopeIn: REFUSED_SCOPE, scopeEq: OK }, + scopeLike: REFUSED_SCOPE, scopeIn: REFUSED_SCOPE, scopeEq: REFUSED_SCOPE }, // A `$field` scalar comparand is SERVED on the `where` door since the // 2026-08-12 ruling (NativeSQLStrategy declines, the engine path runs it) and // refused by the read-scope lowering, which cannot honestly render it (#7598). diff --git a/packages/services/service-analytics/src/__tests__/read-scope-eq-array-refusal.test.ts b/packages/services/service-analytics/src/__tests__/read-scope-eq-array-refusal.test.ts new file mode 100644 index 00000000000..5bbc337d919 --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/read-scope-eq-array-refusal.test.ts @@ -0,0 +1,247 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#19975] The read-scope compiler never binds a LIST into an equality — in + * either spelling of the equality slot, at any depth. + * + * Ruling 乙 on #19757 refuses a list in the equality slot at the shared + * comparand-shape face, for every driver at once. `compileScopedFilterToSql` + * never meets that face (a read scope arrives through `getReadScope`), so the + * ruling is pushed down to it here: + * + * - the EXPLICIT spelling, `{ f: { $eq: [...] } }`, used to compile to + * `col = ?` with the whole list bound as one parameter, and what that + * predicate selected was the executing engine's reading of a list. It is + * now refused ({@link assertNoListInEqualitySlot} in `read-scope-sql.ts`); + * - the IMPLICIT spelling, `{ f: [...] }` — the shape a CEL `field == ` + * policy lowers to — was already refused; the depth pins below hold it. + * + * Both refusals carry this module's envelope, `READ_SCOPE_COMPILE_FAILED` / 500 + * (the #5367 ruling), never the shared face's `INVALID_FILTER` / 400: the + * producer is a policy the caller cannot author. + * + * Four blocks: the explicit spelling at every depth, the implicit spelling at + * every depth, the neighbouring shapes that must keep compiling exactly as + * before, and the two strategy faces that reach this compiler, driven over a + * real engine — where the refusal must land before any statement runs. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm'; +import type { Cube, FilterCondition } from '@objectstack/spec/data'; +import type { AnalyticsQuery, StrategyContext } from '@objectstack/spec/contracts'; + +import { compileScopedFilterToSql } from '../read-scope-sql.js'; +import { NativeSQLStrategy } from '../strategies/native-sql-strategy.js'; +import { ObjectQLStrategy } from '../strategies/objectql-strategy.js'; + +interface Refusal extends Error { + code?: unknown; + status?: unknown; +} + +function refusalOf(scope: unknown): Refusal { + try { + const out = compileScopedFilterToSql(scope as FilterCondition, 't'); + throw new Error(`expected a refusal, but the scope compiled to ${JSON.stringify(out)}`); + } catch (e) { + return e as Refusal; + } +} + +function expectEnvelope(err: Refusal): void { + expect(err.code).toBe('READ_SCOPE_COMPILE_FAILED'); + expect(err.status).toBe(500); +} + +// ───────────────────────────────────────────────────────────────────────────── + +describe('[#19975] a list under $eq is refused, never bound', () => { + const CASES: Array<[string, unknown]> = [ + ['two members', { status: { $eq: ['open', 'pending'] } }], + ['one member', { status: { $eq: ['open'] } }], + ['the empty list', { status: { $eq: [] } }], + ['beside another operator, listed first', { status: { $eq: ['open'], $ne: 'closed' } }], + ['beside another operator, listed last', { status: { $ne: 'closed', $eq: ['open'] } }], + ['under $and', { $and: [{ team_id: 't1' }, { status: { $eq: ['open'] } }] }], + ['under $or', { $or: [{ team_id: 't1' }, { status: { $eq: ['open'] } }] }], + // The widening direction: a negated equality that can never hold is + // constant TRUE, so this is the spelling that used to admit every row. + ['under $not', { $not: { status: { $eq: ['open'] } } }], + ['under $not over $or', { $not: { $or: [{ team_id: 't1' }, { status: { $eq: ['open'] } }] } }], + ]; + + for (const [name, scope] of CASES) { + it(`${name}: READ_SCOPE_COMPILE_FAILED / 500, naming the field and $eq`, () => { + const err = refusalOf(scope); + expectEnvelope(err); + expect(err.message).toContain('"status".$eq'); + }); + } + + it('a list is diagnosed as the list, not by one of its members', () => { + // The member gates (#6125's undefined comparand, #7598's field reference) + // would otherwise answer first and send the operator to the wrong repair. + for (const scope of [ + { status: { $eq: [undefined] } }, + { status: { $eq: [{ $field: 'prior_status' }] } }, + ]) { + const err = refusalOf(scope); + expectEnvelope(err); + expect(err.message).toContain('"status".$eq'); + expect(err.message).not.toMatch(/is undefined|field reference/); + } + }); +}); + +describe('[#19975] the implicit spelling — what a CEL `field == ` lowers to — stays refused at every depth', () => { + const CASES: Array<[string, unknown]> = [ + ['top level', { status: ['open', 'pending'] }], + ['the empty list', { status: [] }], + ['under $and', { $and: [{ team_id: 't1' }, { status: ['open'] }] }], + ['under $or', { $or: [{ team_id: 't1' }, { status: ['open'] }] }], + ['under $not', { $not: { status: ['open'] } }], + ]; + + for (const [name, scope] of CASES) { + it(`${name}: READ_SCOPE_COMPILE_FAILED / 500, as a bare array`, () => { + const err = refusalOf(scope); + expectEnvelope(err); + expect(err.message).toContain('bare array value for "status"'); + }); + } +}); + +describe('[#19975] the neighbouring shapes keep compiling exactly as before', () => { + const ACCEPTED: Array<[string, unknown, string, unknown[]]> = [ + ['a scalar under $eq', { status: { $eq: 'open' } }, '"t"."status" = ?', ['open']], + ['a number under $eq', { amount: { $eq: 0 } }, '"t"."amount" = ?', [0]], + ['null under $eq — the has-no-value predicate', { status: { $eq: null } }, '"t"."status" IS NULL', []], + ['the implicit scalar', { status: 'open' }, '"t"."status" = ?', ['open']], + ['a list under $in — the prescribed spelling', { status: { $in: ['open', 'pending'] } }, '"t"."status" IN (?, ?)', ['open', 'pending']], + ['the empty $in — the FALSE constant (#5243)', { status: { $in: [] } }, '1 = 0', []], + ]; + + for (const [name, scope, sql, params] of ACCEPTED) { + it(name, () => { + const out = compileScopedFilterToSql(scope as FilterCondition, 't'); + expect(out.sql).toBe(sql); + expect(out.params).toEqual(params); + }); + } + + it('a Date under $eq still binds', () => { + const at = new Date('2026-01-01T00:00:00.000Z'); + const out = compileScopedFilterToSql({ created_at: { $eq: at } } as FilterCondition, 't'); + expect(out.sql).toBe('"t"."created_at" = ?'); + expect(out.params).toEqual([at]); + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── + +const OBJECT = 'ticket'; +const ROWS = [ + { id: 'r1', status: 'open' }, + { id: 'r2', status: 'pending' }, + { id: 'r3', status: 'closed' }, + { id: 'r4', status: null }, +]; +const CUBE: Cube = { + name: 'tickets', + sql: OBJECT, + measures: { n: { sql: '*', type: 'count', title: 'n' } }, + dimensions: Object.fromEntries( + ['id', 'status'].map((n) => [n, { name: n, label: n, type: 'string', sql: n }]), + ), + public: false, +} as unknown as Cube; +const QUERY = { cube: 'tickets', dimensions: ['id'], measures: ['n'] } as AnalyticsQuery; + +describe('[#19975] both faces that reach the compiler refuse before any statement runs (real engine)', () => { + let driver: SqliteWasmDriver; + let statements = 0; + + const runRawSql = async (sql: string, params: unknown[]): Promise[]> => { + statements++; + const result = await driver.execute(sql.replace(/\$\d+/g, '?'), params); + 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 []; + }; + + const ctxFor = (scope: unknown, nativeSql: boolean): StrategyContext => + ({ + getCube: (name: string) => (name === 'tickets' ? CUBE : undefined), + queryCapabilities: () => ({ nativeSql, objectqlAggregate: !nativeSql, inMemory: false }), + getReadScope: () => (scope ?? undefined) as FilterCondition | undefined, + executeRawSql: (_object: string, sql: string, params: unknown[]) => runRawSql(sql, params), + sqlDialect: () => 'sqlite', + }) as unknown as StrategyContext; + + /** NativeSQL EXECUTE: `applyReadScope` → `ctx.executeRawSql`. */ + const nativeIds = async (scope: unknown): Promise => { + const result = await new NativeSQLStrategy().execute(QUERY, ctxFor(scope, true)); + return result.rows.map((r) => String(r.id)).sort(); + }; + + /** The `/analytics/sql` echo, its SQL then run on the same engine. */ + const echoIds = async (scope: unknown): Promise => { + const { sql, params } = await new ObjectQLStrategy().generateSql(QUERY, ctxFor(scope, false)); + const rows = await runRawSql(sql, params); + return rows.map((r) => String(r.id)).sort(); + }; + + const FACES = [ + ['native execute', nativeIds], + ['echo', echoIds], + ] as const; + + beforeAll(async () => { + driver = new SqliteWasmDriver({ filename: ':memory:' }); + await driver.initObjects([ + { + name: OBJECT, + fields: { id: { type: 'text', name: 'id' }, status: { type: 'text', name: 'status' } }, + } as never, + ]); + for (const row of ROWS) await driver.create(OBJECT, { ...row }); + }); + + afterAll(async () => { + await driver?.disconnect?.(); + }); + + it('CONTROL: no scope serves every row, and the prescribed $in serves exactly the named rows', async () => { + // Without this, the refusals below could pass on a harness that serves nothing. + for (const [face, run] of FACES) { + expect(await run(null), `${face}: no scope`).toEqual(['r1', 'r2', 'r3', 'r4']); + expect(await run({ status: { $in: ['open', 'pending'] } }), `${face}: $in`).toEqual(['r1', 'r2']); + } + }); + + for (const [name, scope] of [ + ['a list under $eq', { status: { $eq: ['open', 'pending'] } }], + ['a list under $eq, negated', { $not: { status: { $eq: ['open'] } } }], + ['the implicit list', { status: ['open', 'pending'] }], + ] as const) { + for (const [face, run] of FACES) { + it(`${face}: ${name} is refused, and no statement reaches the engine`, async () => { + statements = 0; + let err: Refusal | undefined; + let served: string[] | undefined; + try { + served = await run(scope); + } catch (e) { + err = e as Refusal; + } + expect(served, `${face}: expected a refusal, got rows`).toBeUndefined(); + expect(err).toBeInstanceOf(Error); + expectEnvelope(err as Refusal); + expect(statements).toBe(0); + }); + } + } +}); 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 41a3835aee1..bd91df216ed 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 @@ -91,9 +91,10 @@ function refusalFor(filter: unknown, alias = 'crm_opportunity'): Refusal | undef } /** - * Every refusing site in `read-scope-sql.ts`, in source order. + * Every refusing site in `read-scope-sql.ts`, in source order — except row ⑯, + * appended when it was added (its row says where it runs). * - * FIFTEEN rows over THIRTEEN throw sites: TWO sites are each reached by two + * SIXTEEN rows over FOURTEEN 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 @@ -233,6 +234,18 @@ const REFUSALS: Array<{ message: /unsupported operator "\$regex" on "owner_email" \(fail-closed\)/, sensitive: 'owner_email', }, + { + // [#19975] Ruling 乙 (#19757) pushed down to this compiler: the explicit + // spelling of the equality slot, whose implicit spelling is row ⑨. Listed + // last because it was added last; in `compileField` it runs FIRST, ahead + // of row ⑥, so a list under `$eq` is diagnosed as the list and never by + // one of its members. + name: '⑯ a list under $eq', + site: 'compileField: list in the equality slot', + filter: { region_code: { $eq: ['emea', 'apac'] } }, + message: /array value for "region_code"\.\$eq — an equality compares one value, so a list is refused rather than bound; use \{ \$in: \[\.\.\.\] \} \(fail-closed\)/, + sensitive: 'region_code', + }, ]; /** @@ -323,14 +336,15 @@ 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. - // Fifteen inputs over the module's THIRTEEN throw sites (see the table's + // Sixteen inputs over the module's FOURTEEN 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(15); - expect(new Set(REFUSALS.map((c) => c.site)).size).toBe(13); + // enveloped. [#6125] added the eleventh site, [#6387] the twelfth, + // [#13571] the thirteenth (the empty-`$nin` refusal) and [#19975] the + // fourteenth (a list under `$eq`); these two numbers are the ratchet that + // makes a future unenveloped `throw` fail HERE instead of at an HTTP + // boundary. + expect(REFUSALS).toHaveLength(16); + expect(new Set(REFUSALS.map((c) => c.site)).size).toBe(14); 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 3cc3580f67e..2bdfd8b8603 100644 --- a/packages/services/service-analytics/src/read-scope-sql.ts +++ b/packages/services/service-analytics/src/read-scope-sql.ts @@ -406,6 +406,14 @@ import { * 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. + * + * ## A list under `$eq` is refused, not bound (#19975, applying ruling 乙 of #19757) + * + * The explicit spelling of the equality slot, `{ f: { $eq: [...] } }`, used to + * compile with the whole list bound as one parameter; the implicit spelling was + * already refused by the bare-array arm. {@link assertNoListInEqualitySlot} + * refuses it in this module's envelope. See there for the measured answers, the + * reachability reading, and why `$ne` is not judged here. */ const IDENT = /^[a-z_][a-z0-9_]*$/i; @@ -722,6 +730,13 @@ function compileNode(node: unknown, qAlias: string, params: unknown[], opts: Rea function compileField(field: string, value: unknown, qAlias: string, params: unknown[], opts: ReadScopeCompileOptions): string { const col = `${qAlias}.${quoteIdent(field, 'field')}`; + // [#19975] A LIST under `$eq`, refused before any gate reads one of its + // members — so `{ $eq: [undefined] }` is diagnosed as the list it is, the + // precedence the bare-array arm below already gets (every member gate skips + // a non-node spec). After `quoteIdent`, like every gate here. See + // {@link assertNoListInEqualitySlot}. + assertNoListInEqualitySlot(field, value); + // [#6125] `undefined` in a comparand position, refused before anything binds — // and after `quoteIdent`, so an unsafe identifier (the injection vector) keeps // its own message and its precedence. See {@link assertDefinedComparands} for @@ -752,6 +767,8 @@ function compileField(field: string, value: unknown, qAlias: string, params: unk params.push(value); return `${col} = ?`; } + // The implicit spelling of the equality slot {@link assertNoListInEqualitySlot} + // guards under `$eq` — the shape a CEL `field == ` lowers to. if (Array.isArray(value)) { throw readScopeCompileError(`[read-scope-sql] bare array value for "${field}" — use { $in: [...] } (fail-closed).`); } @@ -1261,6 +1278,58 @@ function assertNoFieldReferenceComparand(field: string, spec: unknown): void { } } +/** + * [#19975] A LIST in the explicit equality slot — `{ f: { $eq: [...] } }` — + * refused, never bound. + * + * Ruling 乙 on #19757 (2026-09-23) refuses a list in the equality slot, implicit + * and `$eq` alike, at the shared comparand-shape face (`assertListComparandShapes`, + * `@objectstack/spec/data`) 「for every driver at once」. This compiler never + * meets that face: a read scope arrives through `getReadScope`, not through + * `parseFilterAST` or the engine's lowering seam. So the ruling is pushed down + * here, the way #6125, #6387 and #7598 pushed theirs. + * + * The implicit spelling was already refused ({@link compileField}'s bare-array + * arm). The `$eq` spelling was compiled to `col = ?` with the WHOLE list bound + * as one parameter, which hands the meaning of the predicate to whatever the + * executing engine makes of a list. Measured on the NativeSQL execute path + * (`applyReadScope` → `executeRawSql`), one scope got four answers: a driver + * error, zero rows, the rows whose stored text equals the driver's own + * serialisation of the list (rows the scope never named), and — under `$not` — + * every row. A read-scope compiler must never bind a list into an equality. + * + * ## Reachability, measured before this gate was written + * + * No in-repo producer emits the `$eq` spelling. `@objectstack/formula`'s CEL + * lowering emits `$eq` only around a `{ $field }` reference and lowers + * `field == ` to the implicit spelling, which the bare-array arm refuses; + * the tenant layer, `plugin-sharing`'s read filter and the controlled-by-parent + * filter carry no `$eq` at all. What remains is the door #6387 recorded: a + * host-supplied `getReadScope` (a documented option) and any direct caller of + * the `compileScopedFilterToSql` export. + * + * ## Envelope and wording + * + * `READ_SCOPE_COMPILE_FAILED` / 500, like every other site — not the shared + * face's `INVALID_FILTER` / 400. The #5367 ruling (re-affirmed as #7598 Q2 = A) + * is why, and it is recorded in the module header: the producer is a policy the + * caller cannot author, and a 4xx would echo it. The sentence follows this + * module's own bare-array refusal, so the two spellings of one condition read + * alike in the operator's log (#5240), and it names `$in`, the list operator an + * author holding a list was reaching for. + * + * ⛔ `$ne` is not judged here: ruling 乙 names equality, and `$ne` with a list is + * #19886's ruling A, carried on that card. The other scalar operators carrying a + * list are not this ruling's either. + */ +function assertNoListInEqualitySlot(field: string, spec: unknown): void { + if (!isFilterNode(spec) || !Array.isArray(spec.$eq)) return; + throw readScopeCompileError( + `[read-scope-sql] array value for "${field}".$eq — an equality compares one value, so a list is refused ` + + `rather than bound; use { $in: [...] } (fail-closed).`, + ); +} + function compileOperator( col: string, op: string, @@ -1270,6 +1339,8 @@ function compileOperator( opts: ReadScopeCompileOptions, ): string { switch (op) { + // [#19975] `val` is never a list here: {@link assertNoListInEqualitySlot} + // refused one at {@link compileField}, before this emitter runs. case '$eq': return val === null ? `${col} IS NULL` : `${col} = ${bind(params, val)}`; // [#5298] `$ne: null` stays `IS NOT NULL` — already total, and "has any // value" is false for a row that has none. Only the comparison is guarded.