Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions .changeset/sql-driver-boolean-identity.md
Original file line number Diff line number Diff line change
@@ -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` 编译方式完全未变。
264 changes: 264 additions & 0 deletions packages/plugins/driver-sql/src/sql-driver-boolean-identity.test.ts
Original file line number Diff line number Diff line change
@@ -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<string[]> => {
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<WireBearingError> => {
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']);
});
});
});
Loading
Loading