diff --git a/.changeset/read-scope-boolean-flag-comparand-refusal.md b/.changeset/read-scope-boolean-flag-comparand-refusal.md new file mode 100644 index 0000000000..df1e4f5409 --- /dev/null +++ b/.changeset/read-scope-boolean-flag-comparand-refusal.md @@ -0,0 +1,46 @@ +--- +"@objectstack/service-analytics": patch +--- + +fix(analytics): read scope 里非布尔的 `$null` / `$exists` 比较数改为拒收，不再按真值性编成相反的谓词 (#6387) + +**⚠️ 行为变更。** `compileScopedFilterToSql` 遇到 `$null` / `$exists` 上的非布尔比较数，从「按 JS 真值性归入两个声明答案之一、静默编出合法 SQL」改为 `READ_SCOPE_COMPILE_FAILED` / **500** 拒收。今天靠这个静默翻转在跑的 read scope，从此会响亮地失败。 + +## 实测到的毛病 + +发射器读的是 `val ? … : …` —— **真值性**，不是 `@objectstack/spec` `FieldOperatorsSchema` 声明的 `z.boolean()`。在 `5faa23ca3` 上直接调 `compileScopedFilterToSql`，alias `t`： + +| read scope | 编译结果 | | +|---|---|---| +| `{ owner_id: { $null: "false" } }` | `"t"."owner_id" IS NULL` | ⛔ 与作者写的意思**相反** | +| `{ owner_id: { $null: "true" } }` | `"t"."owner_id" IS NULL` | | +| `{ owner_id: { $null: 0 } }` | `"t"."owner_id" IS NOT NULL` | | +| `{ owner_id: { $null: null } }` | `"t"."owner_id" IS NOT NULL` | | +| `{ owner_id: { $null: undefined } }` | `"t"."owner_id" IS NOT NULL` | | +| `{ owner_id: { $exists: "false" } }` | `"t"."owner_id" IS NOT NULL` | ⛔ 与作者写的意思**相反** | +| `{ owner_id: { $exists: 0 } }` | `"t"."owner_id" IS NULL` | | +| `{ owner_id: { $exists: "no" } }` | `"t"."owner_id" IS NOT NULL` | | + +两行 ⛔ 是要害：字符串 `"false"` 是**真值**，于是它落在它被写下来所要表达的 `false` 的**对面** —— `{ $exists: "false" }` 写来表示「没有 owner 的行」，编出来是「**有** owner 的行」。这与 #6125 那一格方向相反：那边是 fail-**closed**（匹配零行、只是安静），这边是**加宽** —— admit 了策略要排除的行，出现在一个自述「A read-scope predicate must never be silently dropped、fail-closed」的模块里。 + +## 修法 + +按 #5347（`$null`）/ #5369（`$exists`）在 `driver-sql` 面确立的先例，理由逐字适用：非布尔比较数**按声明拒收**，不做强转。闸落在 `compileField`，紧挨 #6125 的 `undefined` 闸 —— 两道闸的作用域互不相交（那一道按名字跳过这两个算子），所以谁也盖不住谁的措辞。 + +两个算子**共用一条措辞**（#5240「一个条件一种措辞」），只有算子名与 `path` 不同：`driver-sql` 给孪生实现两条措辞，是因为各自要指名**自己**发射器默认倒向哪边；本模块只有一条规则（真值性）同时管着两个算子，两者失败方式完全一样，所以一条措辞才是诚实的写法。测试里有一条断言把「只有这两处不同」钉死。 + +信封沿用本模块自述的那一个（`READ_SCOPE_COMPILE_FAILED` / 500），不是 #5347 的 `INVALID_FILTER` / 400：read scope 由平台自己从 CEL 与库存 metadata 编出来，报 400 等于让调用方去修一个他既没写、也改不动的东西。继承的是**处置**（拒收），不是信封。 + +极性表**同 PR 一起改**：`nullValueSatisfiesOperator` 的 `$null` / `$exists` 两臂从真值性（`Boolean(value)` / `!value`）改为恒等（`value === true` / `value === false`）。每张极性表钉的是它**自己**发射器的拼写（#5146 / #5298），只改发射器不改表，不变量会安静地断在定义处。这条差异消失后，本编译器与 `driver-sql` 的同名表第一次逐臂一致。 + +## ⚠️ 触达性：实测结论是**库存 metadata 走不通** + +定级依据是测量，不是立单时的措辞。`{ $null: <非布尔> }` **无法**从库存 metadata 走到本编译器，三道闸各自独立关死：`RowLevelSecurityPolicySchema` 把 `using` / `check` 声明为 `z.string()`（CEL 谓词，不是 FilterCondition），存对象直接被拒；CEL 下降只在两处发射 `$null` 且比较数是**硬编码布尔**（`== null` → `{$null: true}`，`!= null` → `{$null: false}`），`$exists` 一次都不发射；绕开 schema 塞裸对象会在 `sqlPredicateToCel` 里抛错，被 `getReadFilter` 的 catch 变成 `RLS_DENY_FILTER`。其余 read scope 生产者（Layer 0 租户过滤、`plugin-sharing` 的 `buildReadFilter`、controlled-by-parent、deny 哨兵）压根不含这两个算子。 + +**仍然开着的那条**：`getReadScope` 是 `AnalyticsPluginOptions` 上有文档的公开扩展点，宿主自带的 read scope（来自 JSON 配置或没走类型检查的 JS）与本编译器之间没有任何闸 —— 本单也确认了 `plugin-security` 全路径无 `FilterConditionSchema` / `safeParse`。所以：今天不从库存 metadata 触达，但没有任何结构性的东西挡住下一个生产者。在编译器处拒收，才让「声明为布尔」等于「强制为布尔」，与谁写这条 scope 无关。 + +## ⛔ 一字未动的邻居 + +- **合法布尔**：`$null: true/false`、`$exists: true/false` 的 SQL 逐字节不变（`IS NULL` 下降正是 RLS 用来圈无主行的写法，也是 CEL 唯一能产出的四种形状）。有自己的对照组回归 pin。 +- **比较数位置上的 `null`**：`{ d: null }`、`{ $eq: null }`、`{ $ne: null }`、`$in: [null]` 等 #6125 的 `NULL_CONTROL` 全部保持绿。 +- `driver-sql` / `driver-turso`（#5347 / #5369 已落地）、`packages/spec`（声明已是 `z.boolean()`）、以及本包的 `where` 门 `strategies/filter-normalizer.ts` 均未触碰。 diff --git a/packages/services/service-analytics/src/__tests__/read-scope-boolean-flag-comparand.test.ts b/packages/services/service-analytics/src/__tests__/read-scope-boolean-flag-comparand.test.ts new file mode 100644 index 0000000000..2b2f459994 --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/read-scope-boolean-flag-comparand.test.ts @@ -0,0 +1,310 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#6387, applying #5347 / #5369] A non-boolean `$null` / `$exists` comparand in + * a READ SCOPE is refused — `READ_SCOPE_COMPILE_FAILED` / 500 — and a boolean + * one is untouched. + * + * ## What was wrong + * + * `compileOperator` read both flags by plain TRUTHINESS (`val ? … : …`), so + * every non-boolean was silently sorted into one of the two declared answers + * instead of being refused. Measured on `origin/main` (`5faa23ca3`) by calling + * `compileScopedFilterToSql` directly, alias `t`: + * + * | read scope | compiled to | | + * |---|---|---| + * | `{ owner_id: { $null: "false" } }` | `"t"."owner_id" IS NULL` | ⛔ opposite | + * | `{ owner_id: { $null: "true" } }` | `"t"."owner_id" IS NULL` | | + * | `{ owner_id: { $null: 0 } }` | `"t"."owner_id" IS NOT NULL` | | + * | `{ owner_id: { $null: null } }` | `"t"."owner_id" IS NOT NULL` | | + * | `{ owner_id: { $null: undefined } }` | `"t"."owner_id" IS NOT NULL` | | + * | `{ owner_id: { $exists: "false" } }` | `"t"."owner_id" IS NOT NULL` | ⛔ opposite | + * | `{ owner_id: { $exists: 0 } }` | `"t"."owner_id" IS NULL` | | + * | `{ owner_id: { $exists: "no" } }` | `"t"."owner_id" IS NOT NULL` | | + * + * The two ⛔ rows are the defect. The string `"false"` is TRUTHY, so a scope + * written to mean "rows with NO owner" compiled to "rows that HAVE one" — this + * module ADMITTING the rows the policy excludes, in a compiler whose own header + * promises "a read-scope predicate must never be silently dropped". Unlike + * #6125's cell (fail-closed, zero rows, merely silent) this direction WIDENS, + * which is why it needed no fresh judgement: #5347 (`$null`) and #5369 + * (`$exists`) already refused the shape on `driver-sql` and their reason + * transfers word for word. + * + * ## ⚠️ Reachability, measured — the half that came back NEGATIVE + * + * #6387 required a decisive answer to "can `{ $null: }` reach this + * compiler from STORED metadata". Measured, it cannot, and that is recorded here + * because the issue's severity argument rested on the opposite: + * + * 1. `RowLevelSecurityPolicySchema` declares `using` / `check` as `z.string()` + * (a CEL predicate, not a `FilterCondition`). Storing an object is rejected: + * `expected string, received object`. + * 2. `@objectstack/formula`'s `cel-to-filter.ts` emits `$null` at exactly two + * sites, both with a HARD-CODED boolean (`== null` → `{ $null: true }`, + * `!= null` → `{ $null: false }`), and emits `$exists` nowhere at all. An + * unresolved `current_user.*` yields `unresolved-variable`, dropping the + * policy to the deny sentinel rather than producing a stray comparand. + * 3. Bypassing the schema does not help: a raw object predicate throws inside + * `sqlPredicateToCel` (`expression.replace is not a function`) and + * `getReadFilter`'s catch converts that to `RLS_DENY_FILTER`; a JSON STRING + * of a FilterCondition stores fine and then fails to parse as CEL → deny. + * + * The other read-scope producers cannot emit it either (Layer 0 tenant filter, + * `plugin-sharing`'s `buildReadFilter`, the controlled-by-parent filter, + * `RLS_DENY_FILTER` — none contains `$null` or `$exists`). What IS open: + * `getReadScope` is a documented public option on `AnalyticsPluginOptions`, so a + * host supplying its own read scope from JSON config or untyped JS is a live + * producer with no gate in between — and #6387 confirmed the issue's other + * measurement, that `plugin-security` runs no `FilterConditionSchema` / + * `safeParse` anywhere on this path. So: not reachable from stored metadata + * today, nothing structural stopping the next producer. + * + * ## The three parts of this file + * + * `describe('the eight measured cells …')` is the change: run it against + * pre-#6387 code and every row fails, because every row COMPILES. + * + * `describe('the boolean control group …')` is the risk. `true` / `false` are + * the declared domain and `IS NULL` lowering is precisely what an RLS policy + * uses to scope unowned rows, so the way this change could do harm is by + * refusing a correct policy. Every row there passes before AND after, SQL and + * binds byte for byte. + * + * `describe('the polarity table …')` covers the second half of the change — + * `nullValueSatisfiesOperator`'s `$null` / `$exists` arms moving from truthiness + * to identity — and is honest about what can and cannot be observed from + * outside; see its own comment. + */ + +import { describe, it, expect } from 'vitest'; +import type { FilterCondition } from '@objectstack/spec/data'; +import { compileScopedFilterToSql } from '../read-scope-sql.js'; + +/** The ADR-0112 fields an HTTP boundary classifies on. */ +interface Refusal extends Error { + code?: unknown; + status?: unknown; +} + +const ALIAS = 't'; + +function refusalFor(filter: FilterCondition): Refusal | undefined { + try { + compileScopedFilterToSql(filter, ALIAS); + return undefined; + } catch (e) { + return e as Refusal; + } +} + +/** Every non-boolean comparand #6387 measured, with the lowering it used to get. */ +const REFUSED: Array<{ op: '$null' | '$exists'; label: string; comparand: unknown; wasSql: string }> = [ + { op: '$null', label: 'the STRING "false" — truthy, so the OPPOSITE side', comparand: 'false', wasSql: '"t"."d" IS NULL' }, + { op: '$null', label: 'the string "true"', comparand: 'true', wasSql: '"t"."d" IS NULL' }, + { op: '$null', label: 'the number 0', comparand: 0, wasSql: '"t"."d" IS NOT NULL' }, + { op: '$null', label: 'null', comparand: null, wasSql: '"t"."d" IS NOT NULL' }, + { op: '$null', label: 'undefined (#6390 pinned this one as the cell left alone)', comparand: undefined, wasSql: '"t"."d" IS NOT NULL' }, + { op: '$exists', label: 'the STRING "false" — truthy, so the OPPOSITE side', comparand: 'false', wasSql: '"t"."d" IS NOT NULL' }, + { op: '$exists', label: 'the number 0', comparand: 0, wasSql: '"t"."d" IS NULL' }, + { op: '$exists', label: 'the string "no"', comparand: 'no', wasSql: '"t"."d" IS NOT NULL' }, + { op: '$exists', label: 'undefined (#6390 pinned this one as the cell left alone)', comparand: undefined, wasSql: '"t"."d" IS NULL' }, +]; + +const flag = (op: string, comparand: unknown): FilterCondition => + ({ d: { [op]: comparand } }) as FilterCondition; + +/** + * The DECLARED domain — the control group, and the reason it is spelled out per + * operator rather than assumed. + * + * `$null: true` is how an RLS policy scopes to unowned rows and `$exists: false` + * is its mirror; a gate written one predicate wider (`!== true`, say, or a + * `typeof` test that forgets `false` is a boolean) takes this whole table with + * it, and takes it with a 500 on a policy that is correct. These four rows are + * also the four the CEL lowering can actually produce, so they are the only + * `$null` shapes reachable from stored metadata at all. + */ +const BOOLEAN_CONTROL: Array<{ name: string; filter: FilterCondition; sql: string }> = [ + { name: '{ $null: true }', filter: { d: { $null: true } }, sql: '"t"."d" IS NULL' }, + { name: '{ $null: false }', filter: { d: { $null: false } }, sql: '"t"."d" IS NOT NULL' }, + { name: '{ $exists: true }', filter: { d: { $exists: true } }, sql: '"t"."d" IS NOT NULL' }, + { name: '{ $exists: false }', filter: { d: { $exists: false } }, sql: '"t"."d" IS NULL' }, +]; + +// ───────────────────────────────────────────────────────────────────────────── + +describe('[#6387] the eight measured cells are REFUSED, in this module’s own envelope', () => { + for (const c of REFUSED) { + it(`refuses ${c.op} with ${c.label}`, () => { + const err = refusalFor(flag(c.op, c.comparand)); + expect(err, `it still COMPILED — pre-#6387 this lowered to ${c.wasSql}`).toBeInstanceOf(Error); + expect(err?.code).toBe('READ_SCOPE_COMPILE_FAILED'); + // 500, not #5347's `INVALID_FILTER` / 400: a read scope is compiled by the + // platform from CEL and stored metadata, so the caller is not its author. + // The precedent transferred is the DISPOSITION (refuse), not the envelope + // — this module has exactly one, and #6125 reused it for the same reason. + expect(err?.status).toBe(500); + expect(err?.code).not.toBe('INVALID_FILTER'); + }); + + it(`names the operator and the position: "d".${c.op}`, () => { + const msg = String(refusalFor(flag(c.op, c.comparand))?.message); + expect(msg).toContain(`comparand for "${c.op}" at "d".${c.op} is not a boolean`); + }); + + it(`no longer emits its old lowering: ${c.op} → ${c.wasSql}`, () => { + // The other direction of the same fact. A refusal thrown for some unrelated + // reason would satisfy the assertions above; this one fails if the pre-#6387 + // SQL ever comes back, whatever else the module does. + let sql: string | undefined; + try { + sql = compileScopedFilterToSql(flag(c.op, c.comparand), ALIAS).sql; + } catch { + sql = undefined; + } + expect(sql).toBeUndefined(); + }); + } + + it('says it ONE way — only the operator name and the path differ (#5240)', () => { + // Two operators, one sentence. `driver-sql` gives its twins two messages + // because each names the direction ITS OWN emitter defaulted to; this module + // had a single rule (truthiness) covering both, so both failed identically + // and one wording is the honest spelling. If a later change gives one of them + // a bespoke phrasing, this is the line that objects. + const skeletons = REFUSED.map((c) => { + const msg = String(refusalFor(flag(c.op, c.comparand))?.message); + return msg.split(`"${c.op}"`).join('""').split(`"d".${c.op}`).join('"d".'); + }); + expect(new Set(skeletons).size).toBe(1); + expect(skeletons[0]).toContain('comparand for "" at "d". is not a boolean'); + // …and the shared sentence still carries the fact that made the two ⛔ rows + // the defect, rather than being generic enough to fit anything. + expect(skeletons[0]).toContain('The string "false" is TRUTHY'); + }); + + it('the message tells the operator to fix the PRODUCER, not the caller', () => { + // The disclosure/attribution half of #5367: the message is for the log, and + // what it must not do is send a tenant to "fix their request". + const msg = String(refusalFor(flag('$null', 'false'))?.message); + expect(msg).toContain('never the caller of this query'); + expect(msg).toContain('sharing rule'); + // The in-process producer measured as the one that IS open, named so the + // operator has somewhere to look when no sharing rule is involved. + expect(msg).toContain('getReadScope'); + }); + + it('refuses through every route into compileField, not just the flat one', () => { + // `compileField` is the one road every field constraint travels — the + // evaluation-order trap #5348 / #5327 named on the driver side does not + // exist here, and these four assertions are what says so out loud. + expect(refusalFor({ $not: { d: { $null: 'false' } } })?.code).toBe('READ_SCOPE_COMPILE_FAILED'); + expect(refusalFor({ $or: [{}, { d: { $exists: 'false' } }] })?.code).toBe('READ_SCOPE_COMPILE_FAILED'); + expect(refusalFor({ $and: [{}, { d: { $null: 0 } }] })?.code).toBe('READ_SCOPE_COMPILE_FAILED'); + // Mixed with a legitimate operator on the same field — the constraint is the + // AND of its operators, so one bad flag refuses the whole field. + expect(refusalFor({ d: { $null: 'false', $ne: 'x' } })?.code).toBe('READ_SCOPE_COMPILE_FAILED'); + }); + + it('an INHERITED $null cannot trip the gate', () => { + // `hasOwnProperty`, not `in` — matching `driver-sql`'s twin line for line. + // A prototype-borne key is not something an author wrote. + const spec = Object.create({ $null: 'false' }) as Record; + spec.$eq = 'u1'; + expect(compileScopedFilterToSql({ d: spec } as FilterCondition, ALIAS).sql).toBe('"t"."d" = ?'); + }); +}); + +describe('[#6387] the boolean control group is UNTOUCHED — SQL byte for byte', () => { + for (const c of BOOLEAN_CONTROL) { + it(`still compiles: ${c.name}`, () => { + const out = compileScopedFilterToSql(c.filter, ALIAS); + expect(out.sql).toBe(c.sql); + expect(out.params).toEqual([]); + }); + } + + it('the two flags are still each other’s mirror', () => { + // `$null: true` and `$exists: false` are the same question. The identity + // spelling that replaced truthiness had to preserve that, and a sign error + // in either emitter arm shows up here rather than as a subtly wrong scope. + const sql = (f: FilterCondition) => compileScopedFilterToSql(f, ALIAS).sql; + expect(sql({ d: { $null: true } })).toBe(sql({ d: { $exists: false } })); + expect(sql({ d: { $null: false } })).toBe(sql({ d: { $exists: true } })); + }); + + it('a boolean flag and a refused one are told apart at the SAME position', () => { + expect(compileScopedFilterToSql({ d: { $null: false } }, ALIAS).sql).toBe('"t"."d" IS NOT NULL'); + expect(refusalFor({ d: { $null: 'false' } })?.code).toBe('READ_SCOPE_COMPILE_FAILED'); + }); +}); + +describe('[#6387] the polarity table moved WITH the emitter (#5146 / #5298)', () => { + /** + * ⚠️ Read this before adding an assertion that "proves" the table changed. + * + * `nullValueSatisfiesOperator`'s `$null` / `$exists` arms went from truthiness + * (`Boolean(value)` / `!value`) to identity (`value === true` / + * `value === false`) in this change, because a polarity table pins the + * spelling of ITS OWN emitter — leaving them truthy while the emitter stopped + * guessing is how that invariant breaks silently at its own definition, which + * is the trap #6387 called out in advance. + * + * The honest part: that edit is NOT independently observable from outside, and + * pretending otherwise would be a test that passes for the wrong reason. The + * gate refuses every value on which the two spellings disagree, and over the + * two that survive — `true` and `false` — truthiness and identity give the + * same answer. So what is observable is the table's PRECONDITION, and that is + * what these three assertions pin: + * + * - the boolean rows still route through `$not` exactly as before (the + * control group for the tightening — it did no harm), and + * - a non-boolean can never have the table's verdict SURVIVE, whichever way + * the rewrite classifies it, because the rewritten leaf still reaches + * `compileField` and is refused there. + */ + const sql = (f: FilterCondition) => compileScopedFilterToSql(f, ALIAS).sql; + + it('allowNull polarity: a NULL column satisfies $null: true', () => { + // `$nin` makes the constraint non-total, so `nullGuardForFieldSpec` has to + // consult the table; `$null: true` says a NULL row DOES satisfy it, so the + // leaf is guarded with `IS NULL OR (…)`. + expect(sql({ $not: { d: { $null: true, $nin: ['x'] } } })).toBe( + 'NOT ((("t"."d" IS NULL OR ("t"."d" IS NULL AND ("t"."d" IS NULL OR "t"."d" NOT IN (?))))))', + ); + }); + + it('requireValue polarity: a NULL column does NOT satisfy $null: false', () => { + expect(sql({ $not: { d: { $null: false, $nin: ['x'] } } })).toBe( + 'NOT (("t"."d" IS NOT NULL AND ("t"."d" IS NOT NULL AND ("t"."d" IS NULL OR "t"."d" NOT IN (?)))))', + ); + }); + + it('$exists is the mirror of $null at the same position', () => { + // `$exists: false` asks the same question as `$null: true`, so it must pick + // the same GUARD, not merely the same leaf SQL — the two arms of the table + // are each other's mirror and a copy-paste that made them equal would show + // up as the wrong guard on one of these two lines. + expect(sql({ $not: { d: { $exists: false, $nin: ['x'] } } })).toBe(sql({ $not: { d: { $null: true, $nin: ['x'] } } })); + expect(sql({ $not: { d: { $exists: true, $nin: ['x'] } } })).toBe(sql({ $not: { d: { $null: false, $nin: ['x'] } } })); + }); + + it('the table can never answer for a value outside the domain', () => { + // The rewrite runs BEFORE `compileField`, so a non-boolean IS classified by + // the table on the way through. This is the assertion that says the + // classification is DISCARDED: whichever guard it chose, the rewritten leaf + // still carries the bad flag into `compileField` and is refused there. Both + // guard directions are exercised — `$null` and `$exists` sit on opposite + // sides of the identity test for the same comparand. + for (const op of ['$null', '$exists'] as const) { + for (const comparand of ['false', 'true', 0, 1, null, undefined]) { + const err = refusalFor({ $not: { d: { [op]: comparand, $nin: ['x'] } } } as FilterCondition); + expect(err?.code, `${op}: ${String(comparand)} escaped through the $not rewrite`).toBe( + 'READ_SCOPE_COMPILE_FAILED', + ); + } + } + }); +}); 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 ba60e1edb1..faf0de7cc4 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 @@ -53,6 +53,18 @@ * `null` control group that must NOT move) is pinned in * `read-scope-undefined-comparand.test.ts`; only its ENVELOPE is asserted here. * + * ## [#6387] A twelfth site — TWO rows, because it has two triggers + * + * Rows ⑦ and ⑧ (non-boolean `$null` / `$exists`) were added on 2026-08-07 by + * #6387, pushing #5347 / #5369's boolean-domain refusal down from `driver-sql`. + * They are the second instance of the `quoteIdent` shape the note below + * describes: ONE gate, reached by two genuinely different triggers, both listed + * so the pair is asserted to answer the SAME envelope. They deliberately share + * one wording (#5240 — one condition, one wording; only the operator name and + * the path vary), and that "only those vary" is pinned in + * `read-scope-boolean-flag-comparand.test.ts` along with the behaviour itself. + * Here, as always, only the ENVELOPE is asserted. + * * The end-to-end consequence — that the REST face answers * `500 ANALYTICS_QUERY_FAILED` with the policy content WITHHELD from the body and * intact in `logError` — is `packages/rest`'s @@ -81,14 +93,20 @@ function refusalFor(filter: unknown, alias = 'crm_opportunity'): Refusal | undef /** * Every refusing site in `read-scope-sql.ts`, in source order. * - * TWELVE rows over ELEVEN throw sites: `quoteIdent` is one site reached with two - * `kind` values, and both are listed on purpose. That alias-vs-field split was - * option C on #5367's decision card — the one place where the two triggers are - * genuinely different (a bad alias is OUR generator, a bad field is the admin's - * policy) and where a per-branch verdict was on the table. Option B collapsed it: - * both answer the same 500, so the two rows must produce the same envelope, and - * asserting that is what makes the collapse a tested decision rather than an - * omission. + * FOURTEEN rows over TWELVE 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 + * C on #5367's decision card — the one place where the two triggers are + * genuinely different (a bad alias is OUR generator, a bad field is the + * admin's policy) and where a per-branch verdict was on the table. Option B + * collapsed it: both answer the same 500, so the two rows must produce the + * same envelope, and asserting that is what makes the collapse a tested + * decision rather than an omission. + * - [#6387] `assertBooleanFlagComparands`, with two operators. Same shape, + * same reason it is not collapsed to one row: `$null` and `$exists` are + * different triggers whose messages differ (by operator name and path) and + * whose envelope must not. * * `site` names the `throw readScopeCompileError(...)` the row reaches, so the * table can be checked against the source without guessing. `sensitive` records @@ -148,42 +166,56 @@ const REFUSALS: Array<{ sensitive: 'owner_id', }, { - name: '⑦ bare array value', + name: '⑦ non-boolean $null comparand', + site: 'compileField: non-boolean $null/$exists comparand', + filter: { owner_id: { $null: 'false' } }, + message: /comparand for "\$null" at "owner_id"\.\$null is not a boolean — refusing to build read scope \(fail-closed\)/, + sensitive: 'owner_id', + }, + { + name: '⑧ non-boolean $exists comparand', + site: 'compileField: non-boolean $null/$exists comparand', + filter: { owner_id: { $exists: 'false' } }, + message: /comparand for "\$exists" at "owner_id"\.\$exists is not a boolean — refusing to build read scope \(fail-closed\)/, + sensitive: 'owner_id', + }, + { + name: '⑨ bare array value', site: 'compileField: bare array value', filter: { region_code: ['emea', 'apac'] }, message: /bare array value for "region_code" — use \{ \$in: \[\.\.\.\] \} \(fail-closed\)/, sensitive: 'region_code', }, { - name: '⑧ nested / relation value', + name: '⑩ nested / relation value', site: 'compileField: nested/relation value', filter: { owner: { manager_id: 'u1' } }, message: /"owner" has a nested\/relation value which is not supported in a read scope \(fail-closed\)/, sensitive: 'owner', }, { - name: '⑨ $in without an array', + name: '⑪ $in without an array', site: 'compileOperator: $in needs an array', filter: { region_code: { $in: 'emea' } }, message: /\$in for "region_code" needs an array \(fail-closed\)/, sensitive: 'region_code', }, { - name: '⑩ $nin without an array', + name: '⑫ $nin without an array', site: 'compileOperator: $nin needs an array', filter: { region_code: { $nin: 'emea' } }, message: /\$nin for "region_code" needs an array \(fail-closed\)/, sensitive: 'region_code', }, { - name: '⑪ $between without [min,max]', + 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\)/, @@ -274,12 +306,13 @@ 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. - // Twelve inputs over the module's ELEVEN throw sites (see the table's note - // on `quoteIdent`), and every one of them enveloped. [#6125] added the - // eleventh site; these two numbers are the ratchet that makes a future - // unenveloped `throw` fail HERE instead of at an HTTP boundary. - expect(REFUSALS).toHaveLength(12); - expect(new Set(REFUSALS.map((c) => c.site)).size).toBe(11); + // 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 + // 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); 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/__tests__/read-scope-undefined-comparand.test.ts b/packages/services/service-analytics/src/__tests__/read-scope-undefined-comparand.test.ts index 1a729e0742..00373a461a 100644 --- a/packages/services/service-analytics/src/__tests__/read-scope-undefined-comparand.test.ts +++ b/packages/services/service-analytics/src/__tests__/read-scope-undefined-comparand.test.ts @@ -228,15 +228,26 @@ describe('[#6125] the null control group is UNTOUCHED — SQL and binds, byte fo }); describe('[#6125] what the sweep deliberately leaves alone', () => { - it('$null / $exists take a declared BOOLEAN — not a comparand position', () => { - // Their comparand is a flag, so `undefined` there is not the shape #6050 - // ruled on. `driver-sql`'s twin skips them for the same reason — but it can - // hand them to a boolean-domain gate (#5347 / #5369) and THIS module has - // none, so today they lower by truthiness. That is a different cell, - // measured and filed as #6387 rather than decided as a rider here; when it - // is ruled on, these two lines are what changes. - expect(compileScopedFilterToSql({ d: { $null: undefined } }, ALIAS).sql).toBe('"t"."d" IS NOT NULL'); - expect(compileScopedFilterToSql({ d: { $exists: undefined } }, ALIAS).sql).toBe('"t"."d" IS NULL'); + it('$null / $exists take a declared BOOLEAN — refused by DOMAIN, not by position', () => { + // ⚠️ [#6387] These are the two lines this block said would change "when it is + // ruled on", and they have. What #6125 left alone is still exactly what it + // said it left alone: `undefined` in these two slots is not a COMPARAND + // POSITION, so `assertDefinedComparands` skips them by name to this day, and + // the sentence above it in `read-scope-sql.ts` is unchanged. + // + // What changed is the OTHER side of that skip. #6125 recorded that + // `driver-sql`'s twin skips them to a boolean-domain gate (#5347 / #5369) + // while this module had none, so they lowered by truthiness. #6387 pushed + // that gate down, so `undefined` is now refused HERE too — as a value + // outside a declared domain, with the domain's own wording, which is the + // truer diagnosis for a flag. Two rulings, two reasons, one outcome. + for (const filter of [{ d: { $null: undefined } }, { d: { $exists: undefined } }] as const) { + const err = refusalFor(filter); + expect(err?.code).toBe('READ_SCOPE_COMPILE_FAILED'); + expect(String(err?.message)).toContain('is not a boolean'); + // NOT #6125's wording: the position gate is still the one that skipped them. + expect(String(err?.message)).not.toContain('is undefined'); + } }); it('a bare array keeps its OWN refusal, undefined member or not', () => { diff --git a/packages/services/service-analytics/src/read-scope-sql.ts b/packages/services/service-analytics/src/read-scope-sql.ts index cebe0c420c..21d3c75763 100644 --- a/packages/services/service-analytics/src/read-scope-sql.ts +++ b/packages/services/service-analytics/src/read-scope-sql.ts @@ -180,6 +180,29 @@ import { * for this family: it does for all four positions, so the new refusal is * withheld from the response BY DECLARATION exactly like the other ten, and no * message-sniffing list learns a twelfth phrase. + * + * ## A non-boolean `$null` / `$exists` is refused too (#6387, applying #5347 / #5369) + * + * TWELVE refusing sites, and {@link nonBooleanFlagComparandError} is the + * twelfth's — ONE message for BOTH operators, because both failed the same way + * (see that function for the measured table and the #5240 argument). #5347 and + * #5369 refused a non-boolean comparand for these two operators on `driver-sql`; + * #6387 measured that neither ruling had been pushed down here, where the + * emitter still read them by plain TRUTHINESS. The consequence was sharper than + * on the driver face: `{ $exists: "false" }`, written to scope to rows with NO + * owner, compiled to `IS NOT NULL` — the rows that HAVE one. In a module whose + * contract is fail-closed, that is a WIDENING, which is why this cell was graded + * above #6125's silent-zero-rows one even though its reachability is narrower + * (measured: not reachable from stored metadata — the CEL lowering emits `$null` + * only with hard-coded booleans and `$exists` never; reachable only from an + * in-process `getReadScope` producer). + * + * The twelfth site is one gate over TWO triggers, exactly like `quoteIdent`'s + * alias-vs-field split, and the refusal-envelope inventory lists it as two rows + * over one site for that reason. The message was measured against + * `looksLikeInternalErrorLeak` too — FALSE, like the other eleven — so it is + * withheld from the response BY DECLARATION and teaches no sniffing list a new + * phrase. */ const IDENT = /^[a-z_][a-z0-9_]*$/i; @@ -332,6 +355,13 @@ function compileField(field: string, value: unknown, qAlias: string, params: unk // why this one call site covers the whole tree. assertDefinedComparands(field, value); + // [#6387] …and the two comparands that are NOT positions but DOMAINS: `$null` + // and `$exists` take a declared boolean. Deliberately a second call rather + // than a widened first one — the two gates gate different things, and their + // domains are disjoint by construction (`assertDefinedComparands` skips these + // two operators by name), so neither can shadow the other's message. + assertBooleanFlagComparands(field, value); + // Scalar / null → implicit equality. if (value === null) return `${col} IS NULL`; if (typeof value !== 'object' || value instanceof Date) { @@ -472,7 +502,12 @@ function assertRenderableText(op: string, field: string, val: unknown): void { * ⛔ What deliberately does NOT move: `null`. `{ d: null }`, `{ $eq: null }`, * `{ $ne: null }`, `$null` and `$exists` keep their exact lowering — `null` IS a * declared comparand and IS the null predicate, and the whole point of this - * refusal is the JS value that cannot be told apart from an ABSENT key. Pinned + * refusal is the JS value that cannot be told apart from an ABSENT key. ⚠️ Read + * `$null` / `$exists` there as "with their declared BOOLEAN comparand": #6387 + * later refused every other comparand for those two, `{ $null: null }` included, + * on the separate domain grounds {@link assertBooleanFlagComparands} states. The + * `null` this paragraph promises not to move is `null` in a COMPARAND position, + * which is untouched by both changes and still pinned row for row. Pinned * as its own control group in `read-scope-undefined-comparand.test.ts`, because * refusing `null` along with `undefined` is the way this change could do harm. * @@ -525,13 +560,16 @@ function undefinedComparandError(field: string, path: string): Error { * * - `$null` / `$exists`. Their comparand is a declared BOOLEAN — a flag, not a * value to compare against — so `undefined` there is not a comparand at all. - * `driver-sql`'s twin skips them for the same reason. ⚠️ Unlike that twin, - * THIS module has no boolean-domain gate to hand them to (#5347 / #5369 were - * never pushed down here): `{ $null: undefined }` still lowers by truthiness - * to `IS NOT NULL` and `{ $null: "false" }` — the STRING, which is truthy — - * lands on the side opposite the `false` it was written to mean. That is a - * different cell, measured and filed as #6387 rather than decided as a rider - * on this one. + * `driver-sql`'s twin skips them for the same reason. ✅ [#6387] And it now + * skips them the way that twin does: to a boolean-DOMAIN gate, + * {@link assertBooleanFlagComparands}, which #6387 pushed down from #5347 / + * #5369. When this note was written that gate did not exist here, so + * `{ $null: undefined }` lowered by truthiness to `IS NOT NULL` and + * `{ $null: "false" }` — the STRING, which is truthy — landed on the side + * opposite the `false` it was written to mean. Both are refused today, and + * `undefined` is refused there rather than here on purpose: outside the + * declared domain is a truer diagnosis for a flag than "this comparand + * position is undefined". * - a bare ARRAY in direct comparand position (`{ d: [1, undefined] }`). * {@link compileField} refuses the array as a whole ("use `{ $in: [...] }`"), * and inspecting its members here would relabel a shape refused either way. @@ -582,6 +620,137 @@ function assertDefinedComparands(field: string, spec: unknown): void { } } +/** + * [#6387, applying #5347 / #5369] `$null` / `$exists` whose comparand is not a + * boolean. + * + * ## ONE wording for BOTH operators (#5240), and why that is right here + * + * `driver-sql` gives its twins two messages, because each names the direction + * ITS OWN emitter defaulted to and those directions differ. This module had one + * emitter rule covering both — plain TRUTHINESS — so both operators failed the + * same way, in the same sentence, and #5240's rule applies in the direction it + * usually does: one condition, one wording. Only the operator NAME and the + * `path` vary, and `read-scope-boolean-flag-comparand.test.ts` pins that "only + * those vary" so a later change cannot give one of them a bespoke phrasing. + * + * ## What it used to do — measured on `origin/main` (`5faa23ca3`), alias `t` + * + * The emitter read `val ? … : …`, so every non-boolean was sorted by JS + * truthiness into one of the two declared answers: + * + * | read scope | compiled to | | + * |---|---|---| + * | `{ owner_id: { $null: "false" } }` | `"t"."owner_id" IS NULL` | ⛔ the OPPOSITE of what was written | + * | `{ owner_id: { $null: "true" } }` | `"t"."owner_id" IS NULL` | | + * | `{ owner_id: { $null: 0 } }` | `"t"."owner_id" IS NOT NULL` | | + * | `{ owner_id: { $null: null } }` | `"t"."owner_id" IS NOT NULL` | | + * | `{ owner_id: { $null: undefined } }`| `"t"."owner_id" IS NOT NULL` | | + * | `{ owner_id: { $exists: "false" } }`| `"t"."owner_id" IS NOT NULL` | ⛔ the OPPOSITE of what was written | + * | `{ owner_id: { $exists: 0 } }` | `"t"."owner_id" IS NULL` | | + * | `{ owner_id: { $exists: "no" } }` | `"t"."owner_id" IS NOT NULL` | | + * + * The string `"false"` is TRUTHY, so the two rows marked ⛔ are the ones that + * matter: a scope written to say "rows with NO owner" compiled to "rows that + * HAVE one". Unlike #6125's cell — which was fail-CLOSED, zero rows, merely + * silent — this direction ADMITS the rows the policy meant to exclude, in a + * module whose own contract is "a read-scope predicate must never be silently + * dropped". That is why the disposition needed no new judgement: #5347 (`$null`) + * and #5369 (`$exists`) already refused this shape on `driver-sql`, and their + * stated reason transfers word for word. + * + * ## ⚠️ Reachability, measured — and the half that came back NEGATIVE + * + * #6387 asked for a decisive answer to "can `{ $null: }` travel + * from STORED metadata to this compiler". Measured on `5faa23ca3`, it cannot — + * three independent gates close that road, and this is recorded because the + * issue's severity argument rested on it: + * + * 1. `RowLevelSecurityPolicySchema` declares `using` / `check` as `z.string()` + * — a CEL predicate, not a `FilterCondition`. A stored object is rejected + * at write ("expected string, received object"). + * 2. The CEL lowering never emits this shape. `@objectstack/formula`'s + * `cel-to-filter.ts` emits `$null` at exactly two sites, both with a + * HARD-CODED boolean (`== null` → `{ $null: true }`, `!= null` → + * `{ $null: false }`), and emits `$exists` nowhere at all. An unresolved + * `current_user.*` yields `unresolved-variable` → the policy drops → the + * deny sentinel, never a stray comparand. + * 3. Even bypassing the schema, a raw object predicate throws inside + * `sqlPredicateToCel` (`expression.replace is not a function`), and + * `getReadFilter`'s catch turns that into `RLS_DENY_FILTER`. A JSON STRING + * of a FilterCondition stores fine and then fails to parse as CEL → `null` + * → deny. Both roads end fail-closed. + * + * The other read-scope producers cannot emit it either: the Layer 0 tenant + * filter, `plugin-sharing`'s `buildReadFilter` (`{owner: id}` / `$in` / `$or` / + * `{id:'__deny_all__'}`), the controlled-by-parent filter (`{fk: {$in: […]}}`) + * and `RLS_DENY_FILTER` contain no `$null` or `$exists` at all. + * + * ⚠️ What IS open, and why this gate is still worth having: `getReadScope` is a + * DOCUMENTED public option on `AnalyticsPluginOptions` (`plugin.ts`), so a host + * that supplies its own read scope — from JSON config, or from JS where the + * `FilterCondition` type is not checked — is a live producer with no gate + * between it and here. #6387 also confirmed the issue's other measurement: + * `plugin-security` performs no `FilterConditionSchema` / `safeParse` anywhere + * on this path. So the shape is not reachable from stored metadata TODAY, and + * nothing structural stops the next producer; refusing it at the compiler is + * what makes "declared boolean" mean enforced boolean regardless of who writes + * the scope. Graded on that measurement, not on the issue's opening wording. + */ +function nonBooleanFlagComparandError(op: string, field: string, path: string): Error { + return readScopeCompileError( + `[read-scope-sql] comparand for "${op}" at ${path} is not a boolean — refusing to build read scope ` + + `(fail-closed). @objectstack/spec FieldOperatorsSchema declares both $null and $exists as ` + + `z.boolean(), and this compiler used to read the comparand by TRUTHINESS instead — so a ` + + `non-boolean was silently sorted into one of the two declared answers rather than refused. The ` + + `string "false" is TRUTHY, which is the case that matters: it landed on the side OPPOSITE the ` + + `false it was written to mean, turning "rows with no ${field}" into "rows that have one" — a ` + + `read scope that ADMITS the rows the policy excludes. Write the boolean itself (true or false), ` + + `not a string, a number, null or undefined. The producer to fix is whoever BUILT this read ` + + `scope — an admin-authored sharing rule / permission set, its CEL lowering, or the in-process ` + + `code (a getReadScope option) that assembled the FilterCondition — never the caller of this ` + + `query, who cannot author it (#5347 / #5369, pushed down to this compiler by #6387).`, + ); +} + +/** + * [#6387] Refuse a non-boolean `$null` / `$exists` comparand on ONE field + * constraint. + * + * `hasOwnProperty` rather than `in`, so an inherited key can never trip the + * gate, and rather than `Object.hasOwn` to match `driver-sql`'s twin + * (`reduceFilterKey`) line for line. `{ $null: undefined }` DOES count: the key + * is own and enumerable, and `undefined` is one of the comparands #6387 + * measured a flip on — it lowered to `IS NOT NULL`, which + * `read-scope-undefined-comparand.test.ts` pinned as "the cell #6125 + * deliberately left alone". This is the ruling that picks it up. Refusing it + * here rather than in {@link assertDefinedComparands} keeps that gate's claim + * honest — `undefined` is refused as a value OUTSIDE the declared BOOLEAN + * DOMAIN, which is a truer diagnosis than "a comparand position is undefined" + * for a flag that was never a comparand position. + * + * ## Why this call site, and not the `$not` pre-pass + * + * Same reason {@link assertDefinedComparands} sits here: {@link compileField} is + * the one road every field constraint travels, because {@link compileNode} + * `.map()`s every child into its own buffer BEFORE any boolean identity is + * applied, so no sibling can absorb a malformed one. It runs AFTER + * {@link nullSafeNegationOperand} for a `$not` operand — harmless, and worth + * stating: that rewrite consults {@link nullValueSatisfiesOperator}, which now + * reads these two by identity, so a non-boolean is classified before it is + * refused. The classification is DISCARDED either way (the leaf still reaches + * `compileField` and still throws), and the rewrite's own synthesised leaves + * (`{ $null: false }`, `{ $null: true }`) are literal booleans by construction. + */ +function assertBooleanFlagComparands(field: string, spec: unknown): void { + if (!isFilterNode(spec)) return; + for (const op of ['$null', '$exists'] as const) { + if (!Object.prototype.hasOwnProperty.call(spec, op)) continue; + if (typeof spec[op] === 'boolean') continue; + throw nonBooleanFlagComparandError(op, field, `"${field}".${op}`); + } +} + function compileOperator(col: string, op: string, val: unknown, field: string, params: unknown[]): string { switch (op) { case '$eq': return val === null ? `${col} IS NULL` : `${col} = ${bind(params, val)}`; @@ -621,8 +790,15 @@ function compileOperator(col: string, op: string, val: unknown, field: string, p case '$notContains': assertRenderableText(op, field, val); return nullSafeNegative(col, `${col} NOT LIKE ${bindLike(params, likePattern('contains', val))}`); case '$startsWith': assertRenderableText(op, field, val); return `${col} LIKE ${bindLike(params, likePattern('starts', val))}`; case '$endsWith': assertRenderableText(op, field, val); return `${col} LIKE ${bindLike(params, likePattern('ends', val))}`; - case '$null': return val ? `${col} IS NULL` : `${col} IS NOT NULL`; - case '$exists': return val ? `${col} IS NOT NULL` : `${col} IS NULL`; + // [#6387] `val` is a boolean here — {@link assertBooleanFlagComparands} + // refused anything else at {@link compileField}, before this emitter runs. + // So `=== true` is an exhaustive TWO-WAY choice over the declared domain, + // not the "anything truthy is IS NULL" rule it used to be. That old rule is + // what put the STRING `"false"` on the side opposite the `false` it was + // written to mean; the identity spelling cannot, and it is the spelling + // {@link nullValueSatisfiesOperator} now mirrors (#5146 / #5298). + case '$null': return val === true ? `${col} IS NULL` : `${col} IS NOT NULL`; + case '$exists': return val === true ? `${col} IS NOT NULL` : `${col} IS NULL`; default: throw readScopeCompileError(`[read-scope-sql] unsupported operator "${op}" on "${field}" (fail-closed).`); } @@ -648,17 +824,26 @@ type NullGuard = 'none' | 'requireValue' | 'allowNull'; * !== 'won'` is simply `true` — and #5146 ruled that answer canonical. * * This is `sql-driver.ts`'s `nullValueSatisfiesOperator` table, entry for entry, - * with two deliberate differences that come from THIS file's emitter rather than + * with ONE deliberate difference that comes from THIS file's emitter rather than * from a different reading of #5146: * - * - `$null` / `$exists` are read by TRUTHINESS here, because - * {@link compileOperator} writes them as `val ? … : …`. `driver-sql` reads - * them by identity against `false` because its emitter does. Each guard - * matches its own emitter — that is the invariant, not the literal test. * - `$between` exists in this compiler and not in that table; it is a * positive comparison, so it takes the default (a value that is not there * does not lie between two bounds) exactly as the other comparisons do. * + * ⚠️ [#6387] There used to be a SECOND difference, and its removal is half of + * that change rather than a tidy-up. `$null` / `$exists` were read here by + * TRUTHINESS — `Boolean(value)` / `!value` — because {@link compileOperator} + * wrote them as `val ? … : …`, while `driver-sql` read them by identity because + * its emitter did. That was correct under the invariant #5146 / #5298 state: + * each polarity table pins the spelling of ITS OWN emitter, not the other + * file's. So when the emitter stopped guessing at a non-boolean, these two arms + * had to move WITH it in the same change — leaving them truthy would have + * broken the invariant silently, at its own definition, with nothing red. The + * divergence is gone now because its cause is: both emitters read the declared + * boolean domain, so both tables spell it by identity, and the two files agree + * on every arm for the first time. + * * The default is the large positive-comparison family (`$gt`/`$in`/`$contains`/ * …), every member of which answers `false` for a value that is not there. An * operator this compiler does not support also lands here; it is guarded and @@ -670,9 +855,20 @@ function nullValueSatisfiesOperator(op: string, value: unknown): boolean { case '$eq': return value === null; // Mirror image: `$ne: null` compiles to `IS NOT NULL`, which a NULL fails. case '$ne': return value !== null; - // Truthiness, matching this file's emitter (see the note above). - case '$null': return Boolean(value); - case '$exists': return !value; + // [#6387] Identity, matching this file's emitter (see the note above). + // `assertBooleanFlagComparands` refuses anything but `true` / `false` before + // this table is consulted, so each arm is an exhaustive TWO-WAY choice over + // the declared domain — and the strict spelling is chosen over the lenient + // one it replaces for the reason #5347 gave: `Boolean(value)` and + // `value === true` are equivalent only while the gate upstream holds, and + // the lenient spelling would quietly resume answering for shapes nobody + // ruled on if that gate were ever moved. A NULL column satisfies `$null` + // exactly when the author asked for null… + case '$null': return value === true; + // …and satisfies `$exists` exactly when the author asked for "no value". + // `$null: true` and `$exists: false` are the same question, so these two + // arms are correctly each other's MIRROR, not each other's copy (#5369). + case '$exists': return value === false; // Negative-polarity set / substring tests hold vacuously for an absent value. case '$nin': return true; // `$notContains` is the one operator where the two JS backends disagree for