diff --git a/.changeset/sql-driver-boolean-identity.md b/.changeset/sql-driver-boolean-identity.md new file mode 100644 index 0000000000..6e360944ed --- /dev/null +++ b/.changeset/sql-driver-boolean-identity.md @@ -0,0 +1,44 @@ +--- +"@objectstack/driver-sql": patch +--- + +fix(driver-sql): 空 `$and`/`$or`/`$not` 按布尔单位元编译 —— `$or: []` 不再返回全表 + +**这是一处查询行为变更,且直接关系到 RLS。** `{ $or: [] }` 以前返回**整张表**, +现在返回**零行**。如果你的代码依赖了旧行为,它依赖的是一个 filter 旁路。 + +`applyFilterCondition` 把每个组合子都编译成一个 knex 分组回调,而 knex 对「一个子句 +都没加进去的分组」不产出任何 SQL。于是「这个组是空的」和「这个组已被满足」编译成了 +同一条查询。**丢弃子句不等于套用单位元**,而两个单位元的方向是相反的: + +| 写法 | 布尔代数 | 旧编译 | 错的方向 | +|---|---|---|---| +| `{ $and: [] }` | TRUE → 全部行 | 全表 | 碰巧正确 | +| `{ $or: [] }` | FALSE → **零行** | 全表 | **静默放松** | +| `{ $or: [{a}, {}] }` | `{}` 是 TRUE 析取项 → 全部行 | `(a = ?)` | 静默收紧 | +| `{ $not: {} }` | `NOT TRUE ≡ FALSE` → **零行** | 全表 | **静默放松** | + +`$and: []` 恰好正确的理由不是代码理解了单位元,而是「丢掉」在 AND 侧碰巧等价于 +TRUE —— 同一段代码在 OR 与 NOT 侧就必然错。放松的那两格是安全相关的:`$or: []` +最常见的来源正是「本该有条件、但循环一个析取项都没填进去」的 RLS read scope, +把它当成全表意味着**本该看不到任何行的人拿到了整表**。 + +同仓另外两个后端(`formula` 的 `matchesFilterCondition`、`driver-memory`)三条 +本来就都是对的,`driver-sql` 是唯一的例外;现在四个答案统一。 + +**配套的形状拒收(否则修复会变得更糟)。** 套用单位元的前提是「编译成空」只剩一个 +成因。在此之前 `$or: [null]`、`$or: ['x']`、`$or: [[…]]`、`$or: [new Date()]` +同样会无痕消失;不先拦掉它们就上单位元,会把它们从「被静默忽略」**升级成「匹配所有 +行」**,比原 bug 更坏。因此 `$and`/`$or` 的元素与 `$not` 的操作数现在必须是 +**plain object** 的 filter 节点,否则按 ADR-0112 响亮拒收 +(`INVALID_FILTER` / 400,报错指明出错位置,如 `filter.$or[1]`)。原型检查是关键 +的一半:`Date`/`RegExp`/class 实例都满足 `typeof x === 'object'` 却枚举为空, +若被接受就会被读成 TRUE。同理 `$and: 'x'` 这类非数组操作数也不再被当成一个名为 +`$and` 的字段列。 + +判定是**结构性**的(编译前先归约整棵树),而不是「编译完再问 knex 有没有产出」—— +原缺陷本身就是后者那种观察,而观察分不清「因为本来就是空」和「因为有东西没编译 +出来」。结构判定没有这个盲区,并且保证编译器打开的每个分组都至少收到一条子句, +knex 再没有机会静默丢弃一个组。 + +非空的 `$and`/`$or`/`$not` 编译方式完全未变。 diff --git a/packages/plugins/driver-sql/src/sql-driver-boolean-identity.test.ts b/packages/plugins/driver-sql/src/sql-driver-boolean-identity.test.ts new file mode 100644 index 0000000000..4dc834319e --- /dev/null +++ b/packages/plugins/driver-sql/src/sql-driver-boolean-identity.test.ts @@ -0,0 +1,264 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#5134] An empty `$and` / `$or` / `$not` group compiles to its BOOLEAN + * IDENTITY, never to "nothing". + * + * `applyFilterCondition` used to build every combinator as a Knex group callback + * and let the callback add nothing when the group was empty. Knex emits no SQL + * for a group that received no clause, so "the group is empty" and "the group is + * satisfied" became the same query. Dropping a clause is not the same as + * applying an identity, and the two identities point in OPPOSITE directions: + * + * | filter | boolean algebra | old compile | direction of the error | + * |-----------------|------------------------|-------------|------------------------| + * | `{$and: []}` | TRUE → every row | every row | accidentally right | + * | `{$or: []}` | FALSE → **zero rows** | every row | silently WIDENED | + * | `{$or:[{a},{}]}`| `{}` is a TRUE disjunct → every row | `(a = ?)` | silently narrowed | + * | `{$not: {}}` | NOT TRUE ≡ FALSE → zero rows | every row | silently WIDENED | + * + * `$and: []` was right for the wrong reason — "drop it" happens to equal TRUE on + * the AND side, so the same line is necessarily wrong on the OR side. The + * widening direction is the security-relevant one: `$or: []` is what an RLS read + * scope compiles to when the loop that should have filled its disjuncts produced + * nothing, and answering that with the WHOLE TABLE hands a user every row the + * scope existed to hide. `matchesFilterCondition` (formula) and `driver-memory` + * already answer all three correctly; this driver was the outlier. + * + * # Why the shape rejection below is part of the same fix + * + * Identity reduction is only safe once "this group compiled to empty" has + * EXACTLY ONE cause. Before it, `$or: [null]`, `$or: ['x']`, `$or: [[…]]` and + * `$or: [new Date()]` also vanished without a trace. Applying the identity + * without rejecting those first would have PROMOTED every one of them from + * "silently ignored" to "matches all rows" — strictly worse than the bug. So + * non-node elements are refused loudly (ADR-0112 `INVALID_FILTER`, the envelope + * every sibling filter refusal in this driver speaks) BEFORE any identity is + * applied. Same discipline as cloud#1073, which fixed the identical defect in + * Turso's `RemoteTransport.buildWhereSQL`. + * + * The conformance table (`FILTER_LOGIC_CASES` in `@objectstack/spec/data`) is + * where these cases ultimately belong so all four backends are held to them at + * once — filed as #5239, because driver-mongodb needs its own identity reduction + * to pass them (it passes an empty `$and`/`$or` straight to MongoDB, which + * ERRORS) and the two must land together. + * + * One neighbouring shape is deliberately NOT ruled on here: `{ field: {} }`, a + * field constrained by zero operators, which this driver compiles to no SQL + * inside a combinator while `matchesFilter` and `driver-memory` both answer + * FALSE and this driver's own top-level path refuses it. Three answers to one + * filter — filed as #5240. The reduction classifies any node carrying a field + * key as `'clause'`, so that shape compiles exactly as it did before this fix. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { SqlDriver } from '../src/index.js'; +import type { FilterCondition } from '@objectstack/spec/data'; + +const FIXTURE = [ + { id: '1', stage: 'won', owner: 'u1', amount: 10 }, + { id: '2', stage: 'lost', owner: 'u2', amount: 20 }, + { id: '3', stage: 'open', owner: 'u1', amount: 30 }, +]; + +const ALL = ['1', '2', '3']; + +/** The shape `mapDataError` / `sendError` read off a thrown driver error. */ +interface WireBearingError extends Error { + code?: string; + status?: number; +} + +describe('[#5134] SqlDriver compiles empty $and/$or/$not to their boolean identity', () => { + let driver: SqlDriver; + let knex: any; + + beforeEach(async () => { + driver = new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); + knex = (driver as any).knex; + await knex.schema.createTable('deal', (t: any) => { + t.string('id').primary(); + t.string('stage'); + t.string('owner'); + t.float('amount'); + }); + await knex('deal').insert(FIXTURE); + }); + + afterEach(async () => { + await knex.destroy(); + }); + + // The cast is deliberate: several `where`s below are shapes the schema permits + // but no sane author writes, fed in to prove the compiler answers them the way + // boolean algebra says rather than by accident of what Knex renders. + const ids = async (where: unknown): Promise => { + const rows = await driver.find('deal', { + object: 'deal', + fields: ['id'], + where: where as FilterCondition, + }); + return rows.map((r: any) => String(r.id)).sort(); + }; + + const refusalOf = async (where: unknown): Promise => { + try { + await ids(where); + } catch (e) { + return e as WireBearingError; + } + throw new Error('expected the driver to refuse this filter, but it resolved'); + }; + + // ── The three identities, in one batch ──────────────────────────────────── + + describe('the identity batch', () => { + it('empty $and is TRUE — every row (deliberate, not an accident of dropping)', async () => { + expect(await ids({ $and: [] })).toEqual(ALL); + }); + + it('empty $or is FALSE — ZERO rows, not the whole table', async () => { + expect(await ids({ $or: [] })).toEqual([]); + }); + + it('empty $not is FALSE — NOT TRUE ≡ FALSE, so zero rows', async () => { + expect(await ids({ $not: {} })).toEqual([]); + }); + }); + + // ── The regression these identities exist to prevent ────────────────────── + + it('an RLS read scope whose disjunct list came out empty hides every row', async () => { + // The exact production shape: a scope builder looped over zero grants and + // handed the driver `{$or: []}`. Answering it with the full table is the + // filter bypass #5134 reports. + expect(await ids({ $or: [] })).not.toEqual(ALL); + expect(await ids({ $or: [] })).toHaveLength(0); + }); + + it('a scope that AND-s a real predicate with an empty $or still hides every row', async () => { + expect(await ids({ owner: 'u1', $or: [] })).toEqual([]); + }); + + // ── `{}` is a TRUE operand wherever it appears ──────────────────────────── + + it('an empty branch makes the whole $or TRUE (it is a TRUE disjunct)', async () => { + expect(await ids({ $or: [{ stage: 'won' }, {}] })).toEqual(ALL); + }); + + it('an empty branch inside $and is the AND identity — siblings still apply', async () => { + expect(await ids({ $and: [{ stage: 'won' }, {}] })).toEqual(['1']); + }); + + it('an empty $or branch is dropped as the OR identity, siblings survive', async () => { + expect(await ids({ $or: [{ stage: 'won' }, { $or: [] }] })).toEqual(['1']); + }); + + // ── The identities compose through nesting ──────────────────────────────── + + it('a FALSE branch makes the enclosing $and FALSE', async () => { + expect(await ids({ $and: [{ stage: 'won' }, { $or: [] }] })).toEqual([]); + }); + + it('$not of a FALSE group is TRUE', async () => { + expect(await ids({ $not: { $or: [] } })).toEqual(ALL); + }); + + it('$not of a TRUE group is FALSE', async () => { + expect(await ids({ $not: { $and: [] } })).toEqual([]); + }); + + it('a nested empty $not still collapses to FALSE under $and', async () => { + expect(await ids({ $and: [{ stage: 'won' }, { $not: {} }] })).toEqual([]); + }); + + it('an empty $not as a $or branch is dropped, not promoted', async () => { + expect(await ids({ $or: [{ stage: 'won' }, { $not: {} }] })).toEqual(['1']); + }); + + // ── Shape rejection: an empty compile must have exactly ONE cause ───────── + + describe('non-filter-node operands are refused loudly, never reduced', () => { + const cases: Array<[string, unknown, string]> = [ + ['null element', { $or: [null] }, 'filter.$or[0]'], + ['string element', { $or: ['x'] }, 'filter.$or[0]'], + ['array element', { $or: [[{ stage: 'won' }]] }, 'filter.$or[0]'], + ['Date element', { $or: [new Date()] }, 'filter.$or[0]'], + ['number element in $and', { $and: [42] }, 'filter.$and[0]'], + ['non-node deeper in the list', { $or: [{ stage: 'won' }, null] }, 'filter.$or[1]'], + ['nested under a good branch', { $and: [{ $or: [null] }] }, 'filter.$and[0].$or[0]'], + ['$not operand is an array', { $not: [] }, 'filter.$not'], + ['$not operand is null', { $not: null }, 'filter.$not'], + ['$not operand is a string', { $not: 'x' }, 'filter.$not'], + ['$or is not an array at all', { $or: 'x' }, 'filter.$or'], + ['$and is not an array at all', { $and: { stage: 'won' } }, 'filter.$and'], + ]; + + for (const [name, where, position] of cases) { + it(`${name} → 400 INVALID_FILTER naming ${position}`, async () => { + const err = await refusalOf(where); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + expect(err.message).toContain(position); + // #3867 — driver-internal wording never reaches the wire. + expect(err.message).not.toContain('[sql-driver]'); + }); + } + + it('garbage is NOT upgraded to match-all by the identity reduction', async () => { + // The regression the rejection exists to prevent: before identity + // reduction `{$or:[null]}` silently returned every row via the dropped + // group; a naive identity would have made it match-all *on purpose*. + await expect(ids({ $or: [null] })).rejects.toThrow(); + await expect(ids({ $or: [new Date()] })).rejects.toThrow(); + }); + + it('a class instance is not a filter node either', async () => { + // `Object.entries(new Foo())` can be empty, which would reduce to TRUE and + // hand back the whole table. Prototype identity is what separates a filter + // node from an arbitrary object. + class NotAFilter { + stage = 'won'; + } + const err = await refusalOf({ $or: [new NotAFilter()] }); + expect(err.code).toBe('INVALID_FILTER'); + }); + }); + + // ── Nothing that worked before changes ──────────────────────────────────── + + describe('existing compilation is untouched', () => { + it('a plain $or still ORs its branches', async () => { + expect(await ids({ $or: [{ stage: 'won' }, { stage: 'lost' }] })).toEqual(['1', '2']); + }); + + it('a $or branch still ANDs its own keys (#3774)', async () => { + expect(await ids({ $or: [{ stage: 'won', owner: 'u1' }, { stage: 'nope' }] })).toEqual(['1']); + }); + + it('a non-empty $not still negates', async () => { + expect(await ids({ $not: { stage: 'won' } })).toEqual(['2', '3']); + }); + + it('$not still ANDs with its sibling keys', async () => { + expect(await ids({ $not: { stage: 'won' }, owner: 'u1' })).toEqual(['3']); + }); + + it('a nested $and still intersects', async () => { + expect(await ids({ $and: [{ owner: 'u1' }, { stage: 'open' }] })).toEqual(['3']); + }); + + it('an absent filter is not a failed filter', async () => { + expect(await ids({})).toEqual(ALL); + expect(await ids(undefined)).toEqual(ALL); + }); + + it('operators inside a branch still compile', async () => { + expect(await ids({ $or: [{ amount: { $gte: 25 } }, { stage: 'lost' }] })).toEqual(['2', '3']); + }); + }); +}); diff --git a/packages/plugins/driver-sql/src/sql-driver.ts b/packages/plugins/driver-sql/src/sql-driver.ts index 730345d968..ef0cf19fe5 100644 --- a/packages/plugins/driver-sql/src/sql-driver.ts +++ b/packages/plugins/driver-sql/src/sql-driver.ts @@ -608,6 +608,152 @@ function safeShapePreview(value: unknown): string { } } +/** + * [#5134] What a filter node is worth as a boolean, before any SQL is emitted. + * + * - `'true'` — matches every row; the compiler emits NO clause for it. + * - `'false'` — matches no row; the compiler emits the dialect FALSE constant. + * - `'clause'` — carries at least one real predicate; compile it normally. + */ +type FilterVerdict = 'true' | 'false' | 'clause'; + +/** + * [#5134] Is `value` a Filter Protocol NODE — the shape `FilterConditionSchema` + * declares for every element of `$and`/`$or` and for the operand of `$not`? + * + * The prototype check is the load-bearing half, not pedantry. The identity + * reduction below turns "this node has no predicates" into "matches every row", + * so any object whose OWN ENUMERABLE KEYS are empty is read as TRUE. A `Date`, + * a `RegExp`, a `Map` or a class instance all satisfy `typeof x === 'object' && + * !Array.isArray(x)` while enumerating to nothing — accepting them would + * PROMOTE garbage from "silently ignored" (the old bug) to "matches all rows" + * (strictly worse). A filter condition always arrives as JSON or as the output + * of `compileCelToFilter`, i.e. a plain object, so requiring one costs nothing + * real and makes the reduction total. + */ +function isFilterNode(value: unknown): value is Record { + if (value === null || typeof value !== 'object' || Array.isArray(value)) return false; + const proto = Object.getPrototypeOf(value); + return proto === Object.prototype || proto === null; +} + +/** A short type name for an operand the filter compiler refuses. */ +function describeFilterOperand(value: unknown): string { + if (value === null) return 'null'; + if (Array.isArray(value)) return 'array'; + const kind = typeof value; + if (kind !== 'object') return kind; + const ctor = (value as { constructor?: { name?: string } }).constructor; + return ctor?.name && ctor.name !== 'Object' ? ctor.name : 'object'; +} + +/** + * [#5134] The gate that gives "this group compiled to empty" exactly ONE cause. + * + * Identity reduction is only sound once an empty compile can mean "the author + * wrote an empty group" and nothing else. Before this gate, `$or: [null]`, + * `$or: ['x']`, `$or: [[…]]` and `$or: [new Date()]` also produced no clause and + * vanished without a trace; reducing on top of that would have turned each of + * them into "matches every row". Refusing them here — loudly, in the ADR-0112 + * envelope every sibling filter refusal in this driver speaks — is what makes + * the reduction safe. Same discipline as cloud#1073 on Turso's + * `RemoteTransport.buildWhereSQL`. + */ +function assertFilterNode(value: unknown, path: string): asserts value is Record { + if (isFilterNode(value)) return; + throw unsupportedFilterError( + `Filter node at ${path} is a ${describeFilterOperand(value)} (${safeShapePreview(value)}), not a filter ` + + `condition object. Every element of "$and"/"$or" and the operand of "$not" must be a plain object of ` + + `field constraints (e.g. { "status": "active" }) or nested combinators — @objectstack/spec ` + + `FilterConditionSchema declares this position as a FilterCondition. It is refused rather than skipped ` + + `because skipping it would silently change which rows match.`, + ); +} + +/** [#5134] `$and`/`$or` take a list; anything else is refused, never coerced. */ +function assertFilterNodeList(value: unknown, key: string, path: string): asserts value is unknown[] { + if (Array.isArray(value)) return; + throw unsupportedFilterError( + `Filter combinator "${key}" at ${path} requires an array of filter conditions, but received a ` + + `${describeFilterOperand(value)} (${safeShapePreview(value)}). @objectstack/spec FilterConditionSchema ` + + `declares "${key}" as FilterCondition[].`, + ); +} + +/** + * [#5134] Reduce one filter node to its boolean verdict, validating shapes on + * the way down. + * + * A node is the AND of its entries, so FALSE dominates, and a node with no + * entries at all is TRUE (the empty conjunction) — which is why `{}` is a TRUE + * disjunct inside `$or` and why `{ $not: {} }` is FALSE. + * + * This walks the WHOLE tree without short-circuiting: a `$or: []` sibling must + * not stop the walk from reaching — and refusing — a malformed node further + * along, or the shape gate would be conditional on evaluation order. + * + * Deciding structurally, rather than by compiling and then asking Knex whether + * anything came out, is deliberate. The old defect WAS an observation of + * emptiness ("the group callback added nothing"), and an observation cannot tell + * "empty because the author wrote nothing" from "empty because something failed + * to compile". A structural verdict has no such blind spot, and it lets the + * emitter guarantee that every group it opens receives at least one clause. + */ +function reduceFilterNode(node: Record, path: string): FilterVerdict { + let sawFalse = false; + let sawClause = false; + for (const [key, value] of Object.entries(node)) { + const verdict = reduceFilterKey(key, value, path); + if (verdict === 'false') sawFalse = true; + else if (verdict === 'clause') sawClause = true; + } + // AND over the node's keys: FALSE dominates, then a real predicate, else TRUE. + return sawFalse ? 'false' : sawClause ? 'clause' : 'true'; +} + +/** [#5134] The verdict of ONE key of a filter node. */ +function reduceFilterKey(key: string, value: unknown, path: string): FilterVerdict { + const here = path ? `${path}.${key}` : key; + + if (key === '$and' || key === '$or') { + assertFilterNodeList(value, key, here); + let sawTrue = false; + let sawFalse = false; + let sawClause = false; + value.forEach((element, index) => { + const elementPath = `${here}[${index}]`; + assertFilterNode(element, elementPath); + const verdict = reduceFilterNode(element, elementPath); + if (verdict === 'true') sawTrue = true; + else if (verdict === 'false') sawFalse = true; + else sawClause = true; + }); + // `$and: []` → no FALSE, no clause → TRUE (the AND identity). + if (key === '$and') return sawFalse ? 'false' : sawClause ? 'clause' : 'true'; + // `$or: []` → no TRUE, no clause → FALSE (the OR identity). This is the + // half the old compile got backwards: it answered the whole table. + return sawTrue ? 'true' : sawClause ? 'clause' : 'false'; + } + + if (key === '$not') { + assertFilterNode(value, here); + const inner = reduceFilterNode(value, here); + // NOT TRUE ≡ FALSE — so `{ $not: {} }` matches nothing. + return inner === 'true' ? 'false' : inner === 'false' ? 'true' : 'clause'; + } + + // A field key always contributes a predicate. Note this stays `'clause'` even + // for `{ field: {} }` (a field constrained by zero operators), which compiles + // to no SQL today. That shape is a SEPARATE divergence tracked in #5240 — + // `matchesFilter` and `driver-memory` both answer FALSE for it, this driver's + // combinator path answers TRUE, and its own top-level `applyFilters` path + // refuses it outright via `assertCompilableComparand` — and picking the winner + // is a semantic ruling, not this change's call. Classifying it as `'clause'` + // rather than `'true'` is precisely what keeps the identity reduction from + // silently ruling on it: the shape compiles exactly as it did before. + return 'clause'; +} + // ── Introspection Types ────────────────────────────────────────────────────── export interface IntrospectedColumn { @@ -5904,29 +6050,71 @@ export class SqlDriver implements IDataDriver { * dead weight to prune: the method is `protected`, i.e. subclass API, and * the flag is the seam an override needs to attach a condition into an OR * group. Do not "fix" them by making a branch propagate `'or'` again. + * + * # Boolean identities (#5134) + * + * Every combinator is decided by {@link reduceFilterNode} BEFORE anything is + * emitted, because Knex renders no SQL for a group that received no clause — + * so "the group is empty" and "the group is satisfied" used to compile to the + * same query. That is not an identity, it is a dropped clause, and the two + * identities point in opposite directions: empty `$and` is TRUE, empty `$or` + * is FALSE, and `$not` of an empty (TRUE) group is FALSE. The old code + * answered the whole table to all three; `$and` was right only because + * "dropped" coincides with TRUE on the AND side. + * + * The reduction also makes every group this method opens provably NON-EMPTY: + * a `'true'` combinator is skipped outright, `'true'` members of a `$and` and + * `'false'` members of a `$or` are dropped as their identities, and a node + * that reduces to `'false'` never reaches the loop at all. So Knex is never + * again in a position to silently discard a group. */ protected applyFilterCondition(builder: Knex.QueryBuilder, condition: any, logicalOp: 'and' | 'or' = 'and', tableHint?: string | null) { if (!condition || typeof condition !== 'object') return; const table = tableHint ?? this.coercionKey(builder); + // #5134 — shape-validate the whole tree and decide its boolean value first. + // A malformed node throws here, before any identity is applied, so "compiled + // to empty" can only ever mean "genuinely empty". + const verdict = reduceFilterNode(condition as Record, 'filter'); + if (verdict === 'true') return; + if (verdict === 'false') { + this.applyFalseConstant(builder, logicalOp); + return; + } + for (const [key, value] of Object.entries(condition)) { if (key === '$and' && Array.isArray(value)) { + // #5134 — an all-TRUE `$and` (including `$and: []`) IS the AND identity; + // emitting nothing for it is now a decision, not an accident. A FALSE + // member cannot reach here: it would have made the node FALSE above. + if (reduceFilterKey(key, value, 'filter') === 'true') continue; + const branches = value.filter( + (sub) => reduceFilterNode(sub as Record, 'filter') === 'clause', + ); // Attach this group to the parent the way `logicalOp` asks, matching // `$or`/`$not` below. Nothing passes 'or' today (the sole caller uses // 'and' and no branch propagates 'or' any more), but leaving one of the // four combinators deaf to the flag is how the rules drift apart again. const method = logicalOp === 'or' ? 'orWhere' : 'where'; (builder as any)[method]((qb: any) => { - for (const sub of value) { + for (const sub of branches) { qb.where((subQb: any) => { this.applyFilterCondition(subQb, sub, 'and', table); }); } }); } else if (key === '$or' && Array.isArray(value)) { + // #5134 — one TRUE disjunct makes the whole `$or` TRUE, so `{$or:[{a},{}]}` + // matches every row instead of quietly compiling to just `(a = ?)`. + if (reduceFilterKey(key, value, 'filter') === 'true') continue; + // FALSE disjuncts are the OR identity — dropped. At least one `'clause'` + // member survives, or the key would have been TRUE/FALSE above. + const branches = value.filter( + (sub) => reduceFilterNode(sub as Record, 'filter') === 'clause', + ); const method = logicalOp === 'or' ? 'orWhere' : 'where'; (builder as any)[method]((qb: any) => { - for (const sub of value) { + for (const sub of branches) { // The `orWhere` on THIS line is what OR-s the branches together. // The branch body is still compiled with 'and', because every key // inside one filter object is AND-ed at every depth (Filter @@ -5940,13 +6128,18 @@ export class SqlDriver implements IDataDriver { }); } }); - } else if (key === '$not' && value !== null && typeof value === 'object' && !Array.isArray(value)) { + } else if (key === '$not') { // Spec LOGICAL_OPERATORS declares `$not` alongside `$and`/`$or`; both // driver-mongodb and driver-memory implement it, and CEL `!expr` in a // permission/scope rule compiles to `{ $not: {...} }` (cel-to-filter.ts). // Without this branch `$not` fell through to the field handler, was // treated as a column named "$not", and produced wrong SQL — the same // class of silent filter-bypass this fix (issue #2704) closes. + // + // #5134 — `$not` of a FALSE group is TRUE: skip it. `$not` of a TRUE + // group is FALSE and never reaches here (the node reduced to FALSE), and + // a non-node operand was refused by the reduction, so `value` is a node. + if (reduceFilterKey(key, value, 'filter') === 'true') continue; const notMethod = logicalOp === 'or' ? 'orWhereNot' : 'whereNot'; (builder as any)[notMethod]((qb: any) => { this.applyFilterCondition(qb, value, 'and', table); @@ -6078,6 +6271,20 @@ export class SqlDriver implements IDataDriver { } } + /** + * [#5134] Emit the dialect FALSE constant — a predicate that matches no row. + * + * `1 = 0` is the spelling already used for this condition on both sides of the + * repo: `read-scope-sql.ts` compiles an empty `$in` to it, and Knex itself + * renders an empty `whereIn` as `1 = 0`. It is valid on every dialect this + * driver targets (unlike a bare `FALSE`, which MySQL accepts but older SQL + * Server does not), needs no bindings, and keeps the query a normal SELECT so + * `LIMIT`/`ORDER BY`/aggregates all still behave. + */ + private applyFalseConstant(builder: any, logicalOp: 'and' | 'or'): void { + builder[logicalOp === 'or' ? 'orWhereRaw' : 'whereRaw']('1 = 0'); + } + // ── Field mapping ─────────────────────────────────────────────────────────── protected mapSortField(field: string): string {