|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * [#5134] An empty `$and` / `$or` / `$not` group compiles to its BOOLEAN |
| 5 | + * IDENTITY, never to "nothing". |
| 6 | + * |
| 7 | + * `applyFilterCondition` used to build every combinator as a Knex group callback |
| 8 | + * and let the callback add nothing when the group was empty. Knex emits no SQL |
| 9 | + * for a group that received no clause, so "the group is empty" and "the group is |
| 10 | + * satisfied" became the same query. Dropping a clause is not the same as |
| 11 | + * applying an identity, and the two identities point in OPPOSITE directions: |
| 12 | + * |
| 13 | + * | filter | boolean algebra | old compile | direction of the error | |
| 14 | + * |-----------------|------------------------|-------------|------------------------| |
| 15 | + * | `{$and: []}` | TRUE → every row | every row | accidentally right | |
| 16 | + * | `{$or: []}` | FALSE → **zero rows** | every row | silently WIDENED | |
| 17 | + * | `{$or:[{a},{}]}`| `{}` is a TRUE disjunct → every row | `(a = ?)` | silently narrowed | |
| 18 | + * | `{$not: {}}` | NOT TRUE ≡ FALSE → zero rows | every row | silently WIDENED | |
| 19 | + * |
| 20 | + * `$and: []` was right for the wrong reason — "drop it" happens to equal TRUE on |
| 21 | + * the AND side, so the same line is necessarily wrong on the OR side. The |
| 22 | + * widening direction is the security-relevant one: `$or: []` is what an RLS read |
| 23 | + * scope compiles to when the loop that should have filled its disjuncts produced |
| 24 | + * nothing, and answering that with the WHOLE TABLE hands a user every row the |
| 25 | + * scope existed to hide. `matchesFilterCondition` (formula) and `driver-memory` |
| 26 | + * already answer all three correctly; this driver was the outlier. |
| 27 | + * |
| 28 | + * # Why the shape rejection below is part of the same fix |
| 29 | + * |
| 30 | + * Identity reduction is only safe once "this group compiled to empty" has |
| 31 | + * EXACTLY ONE cause. Before it, `$or: [null]`, `$or: ['x']`, `$or: [[…]]` and |
| 32 | + * `$or: [new Date()]` also vanished without a trace. Applying the identity |
| 33 | + * without rejecting those first would have PROMOTED every one of them from |
| 34 | + * "silently ignored" to "matches all rows" — strictly worse than the bug. So |
| 35 | + * non-node elements are refused loudly (ADR-0112 `INVALID_FILTER`, the envelope |
| 36 | + * every sibling filter refusal in this driver speaks) BEFORE any identity is |
| 37 | + * applied. Same discipline as cloud#1073, which fixed the identical defect in |
| 38 | + * Turso's `RemoteTransport.buildWhereSQL`. |
| 39 | + * |
| 40 | + * The conformance table (`FILTER_LOGIC_CASES` in `@objectstack/spec/data`) is |
| 41 | + * where these cases ultimately belong so all four backends are held to them at |
| 42 | + * once — filed as #5239, because driver-mongodb needs its own identity reduction |
| 43 | + * to pass them (it passes an empty `$and`/`$or` straight to MongoDB, which |
| 44 | + * ERRORS) and the two must land together. |
| 45 | + * |
| 46 | + * One neighbouring shape is deliberately NOT ruled on here: `{ field: {} }`, a |
| 47 | + * field constrained by zero operators, which this driver compiles to no SQL |
| 48 | + * inside a combinator while `matchesFilter` and `driver-memory` both answer |
| 49 | + * FALSE and this driver's own top-level path refuses it. Three answers to one |
| 50 | + * filter — filed as #5240. The reduction classifies any node carrying a field |
| 51 | + * key as `'clause'`, so that shape compiles exactly as it did before this fix. |
| 52 | + */ |
| 53 | + |
| 54 | +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; |
| 55 | +import { SqlDriver } from '../src/index.js'; |
| 56 | +import type { FilterCondition } from '@objectstack/spec/data'; |
| 57 | + |
| 58 | +const FIXTURE = [ |
| 59 | + { id: '1', stage: 'won', owner: 'u1', amount: 10 }, |
| 60 | + { id: '2', stage: 'lost', owner: 'u2', amount: 20 }, |
| 61 | + { id: '3', stage: 'open', owner: 'u1', amount: 30 }, |
| 62 | +]; |
| 63 | + |
| 64 | +const ALL = ['1', '2', '3']; |
| 65 | + |
| 66 | +/** The shape `mapDataError` / `sendError` read off a thrown driver error. */ |
| 67 | +interface WireBearingError extends Error { |
| 68 | + code?: string; |
| 69 | + status?: number; |
| 70 | +} |
| 71 | + |
| 72 | +describe('[#5134] SqlDriver compiles empty $and/$or/$not to their boolean identity', () => { |
| 73 | + let driver: SqlDriver; |
| 74 | + let knex: any; |
| 75 | + |
| 76 | + beforeEach(async () => { |
| 77 | + driver = new SqlDriver({ |
| 78 | + client: 'better-sqlite3', |
| 79 | + connection: { filename: ':memory:' }, |
| 80 | + useNullAsDefault: true, |
| 81 | + }); |
| 82 | + knex = (driver as any).knex; |
| 83 | + await knex.schema.createTable('deal', (t: any) => { |
| 84 | + t.string('id').primary(); |
| 85 | + t.string('stage'); |
| 86 | + t.string('owner'); |
| 87 | + t.float('amount'); |
| 88 | + }); |
| 89 | + await knex('deal').insert(FIXTURE); |
| 90 | + }); |
| 91 | + |
| 92 | + afterEach(async () => { |
| 93 | + await knex.destroy(); |
| 94 | + }); |
| 95 | + |
| 96 | + // The cast is deliberate: several `where`s below are shapes the schema permits |
| 97 | + // but no sane author writes, fed in to prove the compiler answers them the way |
| 98 | + // boolean algebra says rather than by accident of what Knex renders. |
| 99 | + const ids = async (where: unknown): Promise<string[]> => { |
| 100 | + const rows = await driver.find('deal', { |
| 101 | + object: 'deal', |
| 102 | + fields: ['id'], |
| 103 | + where: where as FilterCondition, |
| 104 | + }); |
| 105 | + return rows.map((r: any) => String(r.id)).sort(); |
| 106 | + }; |
| 107 | + |
| 108 | + const refusalOf = async (where: unknown): Promise<WireBearingError> => { |
| 109 | + try { |
| 110 | + await ids(where); |
| 111 | + } catch (e) { |
| 112 | + return e as WireBearingError; |
| 113 | + } |
| 114 | + throw new Error('expected the driver to refuse this filter, but it resolved'); |
| 115 | + }; |
| 116 | + |
| 117 | + // ── The three identities, in one batch ──────────────────────────────────── |
| 118 | + |
| 119 | + describe('the identity batch', () => { |
| 120 | + it('empty $and is TRUE — every row (deliberate, not an accident of dropping)', async () => { |
| 121 | + expect(await ids({ $and: [] })).toEqual(ALL); |
| 122 | + }); |
| 123 | + |
| 124 | + it('empty $or is FALSE — ZERO rows, not the whole table', async () => { |
| 125 | + expect(await ids({ $or: [] })).toEqual([]); |
| 126 | + }); |
| 127 | + |
| 128 | + it('empty $not is FALSE — NOT TRUE ≡ FALSE, so zero rows', async () => { |
| 129 | + expect(await ids({ $not: {} })).toEqual([]); |
| 130 | + }); |
| 131 | + }); |
| 132 | + |
| 133 | + // ── The regression these identities exist to prevent ────────────────────── |
| 134 | + |
| 135 | + it('an RLS read scope whose disjunct list came out empty hides every row', async () => { |
| 136 | + // The exact production shape: a scope builder looped over zero grants and |
| 137 | + // handed the driver `{$or: []}`. Answering it with the full table is the |
| 138 | + // filter bypass #5134 reports. |
| 139 | + expect(await ids({ $or: [] })).not.toEqual(ALL); |
| 140 | + expect(await ids({ $or: [] })).toHaveLength(0); |
| 141 | + }); |
| 142 | + |
| 143 | + it('a scope that AND-s a real predicate with an empty $or still hides every row', async () => { |
| 144 | + expect(await ids({ owner: 'u1', $or: [] })).toEqual([]); |
| 145 | + }); |
| 146 | + |
| 147 | + // ── `{}` is a TRUE operand wherever it appears ──────────────────────────── |
| 148 | + |
| 149 | + it('an empty branch makes the whole $or TRUE (it is a TRUE disjunct)', async () => { |
| 150 | + expect(await ids({ $or: [{ stage: 'won' }, {}] })).toEqual(ALL); |
| 151 | + }); |
| 152 | + |
| 153 | + it('an empty branch inside $and is the AND identity — siblings still apply', async () => { |
| 154 | + expect(await ids({ $and: [{ stage: 'won' }, {}] })).toEqual(['1']); |
| 155 | + }); |
| 156 | + |
| 157 | + it('an empty $or branch is dropped as the OR identity, siblings survive', async () => { |
| 158 | + expect(await ids({ $or: [{ stage: 'won' }, { $or: [] }] })).toEqual(['1']); |
| 159 | + }); |
| 160 | + |
| 161 | + // ── The identities compose through nesting ──────────────────────────────── |
| 162 | + |
| 163 | + it('a FALSE branch makes the enclosing $and FALSE', async () => { |
| 164 | + expect(await ids({ $and: [{ stage: 'won' }, { $or: [] }] })).toEqual([]); |
| 165 | + }); |
| 166 | + |
| 167 | + it('$not of a FALSE group is TRUE', async () => { |
| 168 | + expect(await ids({ $not: { $or: [] } })).toEqual(ALL); |
| 169 | + }); |
| 170 | + |
| 171 | + it('$not of a TRUE group is FALSE', async () => { |
| 172 | + expect(await ids({ $not: { $and: [] } })).toEqual([]); |
| 173 | + }); |
| 174 | + |
| 175 | + it('a nested empty $not still collapses to FALSE under $and', async () => { |
| 176 | + expect(await ids({ $and: [{ stage: 'won' }, { $not: {} }] })).toEqual([]); |
| 177 | + }); |
| 178 | + |
| 179 | + it('an empty $not as a $or branch is dropped, not promoted', async () => { |
| 180 | + expect(await ids({ $or: [{ stage: 'won' }, { $not: {} }] })).toEqual(['1']); |
| 181 | + }); |
| 182 | + |
| 183 | + // ── Shape rejection: an empty compile must have exactly ONE cause ───────── |
| 184 | + |
| 185 | + describe('non-filter-node operands are refused loudly, never reduced', () => { |
| 186 | + const cases: Array<[string, unknown, string]> = [ |
| 187 | + ['null element', { $or: [null] }, 'filter.$or[0]'], |
| 188 | + ['string element', { $or: ['x'] }, 'filter.$or[0]'], |
| 189 | + ['array element', { $or: [[{ stage: 'won' }]] }, 'filter.$or[0]'], |
| 190 | + ['Date element', { $or: [new Date()] }, 'filter.$or[0]'], |
| 191 | + ['number element in $and', { $and: [42] }, 'filter.$and[0]'], |
| 192 | + ['non-node deeper in the list', { $or: [{ stage: 'won' }, null] }, 'filter.$or[1]'], |
| 193 | + ['nested under a good branch', { $and: [{ $or: [null] }] }, 'filter.$and[0].$or[0]'], |
| 194 | + ['$not operand is an array', { $not: [] }, 'filter.$not'], |
| 195 | + ['$not operand is null', { $not: null }, 'filter.$not'], |
| 196 | + ['$not operand is a string', { $not: 'x' }, 'filter.$not'], |
| 197 | + ['$or is not an array at all', { $or: 'x' }, 'filter.$or'], |
| 198 | + ['$and is not an array at all', { $and: { stage: 'won' } }, 'filter.$and'], |
| 199 | + ]; |
| 200 | + |
| 201 | + for (const [name, where, position] of cases) { |
| 202 | + it(`${name} → 400 INVALID_FILTER naming ${position}`, async () => { |
| 203 | + const err = await refusalOf(where); |
| 204 | + expect(err.code).toBe('INVALID_FILTER'); |
| 205 | + expect(err.status).toBe(400); |
| 206 | + expect(err.message).toContain(position); |
| 207 | + // #3867 — driver-internal wording never reaches the wire. |
| 208 | + expect(err.message).not.toContain('[sql-driver]'); |
| 209 | + }); |
| 210 | + } |
| 211 | + |
| 212 | + it('garbage is NOT upgraded to match-all by the identity reduction', async () => { |
| 213 | + // The regression the rejection exists to prevent: before identity |
| 214 | + // reduction `{$or:[null]}` silently returned every row via the dropped |
| 215 | + // group; a naive identity would have made it match-all *on purpose*. |
| 216 | + await expect(ids({ $or: [null] })).rejects.toThrow(); |
| 217 | + await expect(ids({ $or: [new Date()] })).rejects.toThrow(); |
| 218 | + }); |
| 219 | + |
| 220 | + it('a class instance is not a filter node either', async () => { |
| 221 | + // `Object.entries(new Foo())` can be empty, which would reduce to TRUE and |
| 222 | + // hand back the whole table. Prototype identity is what separates a filter |
| 223 | + // node from an arbitrary object. |
| 224 | + class NotAFilter { |
| 225 | + stage = 'won'; |
| 226 | + } |
| 227 | + const err = await refusalOf({ $or: [new NotAFilter()] }); |
| 228 | + expect(err.code).toBe('INVALID_FILTER'); |
| 229 | + }); |
| 230 | + }); |
| 231 | + |
| 232 | + // ── Nothing that worked before changes ──────────────────────────────────── |
| 233 | + |
| 234 | + describe('existing compilation is untouched', () => { |
| 235 | + it('a plain $or still ORs its branches', async () => { |
| 236 | + expect(await ids({ $or: [{ stage: 'won' }, { stage: 'lost' }] })).toEqual(['1', '2']); |
| 237 | + }); |
| 238 | + |
| 239 | + it('a $or branch still ANDs its own keys (#3774)', async () => { |
| 240 | + expect(await ids({ $or: [{ stage: 'won', owner: 'u1' }, { stage: 'nope' }] })).toEqual(['1']); |
| 241 | + }); |
| 242 | + |
| 243 | + it('a non-empty $not still negates', async () => { |
| 244 | + expect(await ids({ $not: { stage: 'won' } })).toEqual(['2', '3']); |
| 245 | + }); |
| 246 | + |
| 247 | + it('$not still ANDs with its sibling keys', async () => { |
| 248 | + expect(await ids({ $not: { stage: 'won' }, owner: 'u1' })).toEqual(['3']); |
| 249 | + }); |
| 250 | + |
| 251 | + it('a nested $and still intersects', async () => { |
| 252 | + expect(await ids({ $and: [{ owner: 'u1' }, { stage: 'open' }] })).toEqual(['3']); |
| 253 | + }); |
| 254 | + |
| 255 | + it('an absent filter is not a failed filter', async () => { |
| 256 | + expect(await ids({})).toEqual(ALL); |
| 257 | + expect(await ids(undefined)).toEqual(ALL); |
| 258 | + }); |
| 259 | + |
| 260 | + it('operators inside a branch still compile', async () => { |
| 261 | + expect(await ids({ $or: [{ amount: { $gte: 25 } }, { stage: 'lost' }] })).toEqual(['2', '3']); |
| 262 | + }); |
| 263 | + }); |
| 264 | +}); |
0 commit comments