Skip to content

Commit 0f17114

Browse files
os-zhuangos-support-aiclaude
authored
fix(driver-sql,driver-memory,formula)!: { field: {} } is refused by all four backends (#5240) (#5327)
A field constrained by ZERO operators is a shape `FilterConditionSchema` still declares legal, and one filter carrying it had three answers in this repo: - driver-sql refused it at the top level (the #5041 comparand gate) but DROPPED it inside `$and`/`$or`/`$not`, where a predicate that emits nothing means "matches every row" — so `{ $or: [{ a: {} }, { b: 2 } ] }` compiled to `(b = 2)` by losing a clause, and the same `{ a: {} }` was a 400 at the top level and a silent match-all one combinator deep; - driver-memory answered "matches nothing" incidentally, and did so through TWO independent paths that had never been compared: the live query path (mingo reads `{ a: {} }` as "deep-equals the empty document") and the reference matcher (`JSON.stringify` structural equality); - formula answered `false` from an explicit fail-closed arm. Ruled on #5240: refuse it everywhere, with one `INVALID_FILTER` / 400 envelope and a message naming the position (`filter.$or[0].stage`). The shape is almost always an authoring accident — a filter builder that recorded a field and never its operator — and both silent readings answer it with a row count the author never asked for. The gate sits on the #5134 validation walk, beside `assertFilterNode`, not in the emitter: the walk is exhaustive, while the emitter returns early whenever an identity settles a node, so an emitter-side gate would let `{ $or: [{ a: {} }, {} ] }` through and make the refusal depend on the shape's siblings. The reduction's VERDICT is unchanged (a field key still contributes `'clause'`), so every filter that compiled before compiles byte-identically. `nullGuardForFieldSpec`'s `entries.length === 0` escape — added by #5146 so the NULL-safe rewrite would not rule on #5240 from there — is removed with the ambiguity it protected: the refusal now fires before that rewrite runs. BREAKING: `matchesFilterCondition` is the RLS `check` evaluation path, so a `check` policy carrying `{ field: {} }` now fails the operation (#4775 posture) instead of evaluating to `false`. Where such a constraint sat under an `$or` beside a satisfied branch, or under a `$not`, the old `false` was absorbed and the write was ALLOWED; those writes now fail. Implementation is stricter than the declared contract: narrowing `FilterConditionSchema` and adding the case to `FILTER_LOGIC_CASES` is the spec lane's half of #5240. Fixes #5240 Co-authored-by: os-zhuang <support@objectstack.ai> Co-authored-by: Claude <noreply@anthropic.com>
1 parent b49ccfd commit 0f17114

11 files changed

Lines changed: 1020 additions & 49 deletions
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
---
2+
"@objectstack/driver-sql": minor
3+
"@objectstack/driver-memory": minor
4+
"@objectstack/formula": minor
5+
---
6+
7+
fix(driver-sql,driver-memory,formula)!: `{ field: {} }` 一律拒收 —— 零个操作符的字段约束不再在四个后端有三个答案 (#5240)
8+
9+
`{ a: {} }`(一个字段,后面跟零个操作符)是 `FilterConditionSchema` 今天**声明合法**的形状,
10+
而同一个 filter 在同仓四条路径上有三个答案:
11+
12+
| 路径 | 改前 | 改后 |
13+
|---|---|---|
14+
| `driver-sql`,顶层 plain map | 抛 `INVALID_FILTER`(#5041 的比较数闸门) | 抛 `INVALID_FILTER`(专用消息) |
15+
| `driver-sql`,`$and`/`$or`/`$not` 内 | 遍历零个操作符 → 不产出任何 SQL → **TRUE(匹配全表)** | 抛 `INVALID_FILTER` |
16+
| `driver-memory` | 实时路径经 mingo 变成「字段深等于空文档」;参考匹配器落到 `JSON.stringify` 结构相等 → 顺带 FALSE | 抛 `INVALID_FILTER` |
17+
| `@objectstack/formula` | `keys.length === 0` 显式 fail-closed → FALSE | 抛 `INVALID_FILTER` |
18+
19+
于是 `{ $or: [ { a: {} }, { b: 2 } ] }` 在 SQL 上编译成 `(b = 2)` —— 既不是「零约束即 TRUE」
20+
该给的全表,也不是两个 JS 后端给的 FALSE,而是**子句被 knex 连同空分组一起丢掉**的结果;
21+
而 `driver-sql` 自己内部就不自洽:同一个 `{ a: {} }` 写在顶层被响亮拒收,包进一层 `$or`
22+
就变成静默的 TRUE。
23+
24+
维护者拍板取**拒收**(不取 TRUE、不取 FALSE):这个形状几乎必然是编写期事故 ——
25+
筛选器记下了字段却没记下操作符,或生成的元数据把操作符弄丢了 —— 让它在编写期就炸,
26+
好过在某个后端上安静地多返回或少返回几行。与 #5041 已在 driver-sql 顶层建立的先例一致,
27+
本次只是把同一道闸门补进组合子内部。四个后端(第四个是继承 `SqlDriver` 的
28+
`driver-sqlite-wasm`)现在给出同一个 `INVALID_FILTER` / 400,消息里指名出事的位置
29+
(如 `filter.$or[0].stage`)。
30+
31+
**⚠️ 可观察的行为变更 —— RLS `check` 求值路径。** `@objectstack/formula` 的
32+
`matchesFilterCondition` 是 `plugin-security` 对 insert/update **后像**执行行级 `check`
33+
的那条路径(没有查询可下推,这个求值器就是执行本身)。它改为抛出后,落在 #4775
34+
「求不出值 = 该次操作失败」的既定姿态上。这不只是「拒绝得更响」——有一类结果直接翻转:
35+
36+
| `check` 策略 | 改前 | 改后 |
37+
|---|---|---|
38+
| `{ a: {} }` | FALSE → 写入被拒(403) | 抛出 → 该次写入失败(400) |
39+
| `{ $or: [ { a: {} }, { owner: '{userId}' } ] }` | FALSE 被另一析取项吸收 → 写入**放行** | 抛出 → 该次写入失败 |
40+
| `{ $not: { a: {} } }` | `!false` → 写入**放行** | 抛出 → 该次写入失败 |
41+
42+
后两行是**原本能成功、现在会失败**的写入。这是拍板的目的而非副作用:一条含
43+
`{ field: {} }` 的权限规则,是一条作者弄丢了操作符的规则,它的含义不该取决于四个后端里
44+
哪一个在求值。升级后请检查 `check`/`using` 策略里是否存在零操作符的字段约束——
45+
错误消息会指名位置。
46+
47+
同一条改动也让 `@objectstack/driver-memory` 的两个过滤面(经 mingo 的实时查询路径,
48+
与跨后端一致性套件所用的 `memory-matcher` 参考匹配器)第一次对这个形状给出同一个答案。
49+
50+
非空形状**逐字符不变**:普通比较、`$in`、`$or`/`$and` 组合、`$not` 的 #5146 NULL-safe 改写,
51+
编译出的 SQL 文本与匹配结果都与改前相同;`{}`(零个键的**节点**,#5134 的布尔单位元)
52+
与 `{ field: {} }` 是两个不同形状,前者的语义不受本次影响。
53+
54+
注:本次收紧的是**实现**。`packages/spec` 的 `FilterConditionSchema` 仍然声明这个形状合法
55+
(非递归半边是 `z.record(z.string(), z.unknown())`),即实现现在比已声明的契约更严;
56+
契约收窄与 `FILTER_LOGIC_CASES` 补条归 spec 车道另行处理。
Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#5240] `{ field: {} }` — a field constrained by ZERO operators — is REFUSED
5+
* by `matchesFilterCondition`, with the same `INVALID_FILTER` envelope
6+
* `driver-sql` and `driver-memory` raise.
7+
*
8+
* # This is the RLS `check` evaluation path, so the change is observable
9+
*
10+
* `matchesFilterCondition` is what `plugin-security` runs against the POST-IMAGE
11+
* of an insert/update to enforce a row-level `check` — there is no query to push
12+
* down to, so this evaluator IS the enforcement. It answered `false` for the
13+
* shape from an explicit fail-closed arm (`keys.length === 0 || …`), which is
14+
* why the divergence never surfaced as an error: the write was simply denied.
15+
*
16+
* Refusing instead lands on the #4775 posture (a `check` that cannot be
17+
* evaluated fails the operation), and the direction is honest about ONE case
18+
* that flips outright:
19+
*
20+
* | `check` policy | before | after |
21+
* |---|---|---|
22+
* | `{ a: {} }` | `false` → write DENIED (403) | throws → write FAILS (400) |
23+
* | `{ $or: [ { a: {} }, { owner: '{userId}' } ] }` | the `false` disjunct was absorbed → write **ALLOWED** | throws → write FAILS |
24+
* | `{ $not: { a: {} } }` | `!false` → write **ALLOWED** | throws → write FAILS |
25+
*
26+
* The last two rows are writes that used to succeed and now do not. That is the
27+
* point of the ruling rather than a side effect of it: a permission rule whose
28+
* meaning depends on which of four backends evaluated it is the defect, and a
29+
* rule carrying `{ field: {} }` is a rule whose author lost an operator.
30+
*/
31+
32+
import { describe, it, expect } from 'vitest';
33+
import { matchesFilterCondition } from './matches-filter';
34+
35+
interface WireBearingError extends Error {
36+
code?: string;
37+
status?: number;
38+
}
39+
40+
const RECORD = { id: '1', stage: 'won', owner: 'u1', amount: 10 };
41+
42+
const refusalOf = (filter: unknown): WireBearingError => {
43+
try {
44+
matchesFilterCondition(RECORD, filter as never);
45+
} catch (e) {
46+
return e as WireBearingError;
47+
}
48+
throw new Error('expected the evaluator to refuse this filter, but it answered');
49+
};
50+
51+
describe('[#5240] matchesFilterCondition refuses a zero-operator field constraint', () => {
52+
const positions: Array<[string, unknown, string]> = [
53+
['top level', { stage: {} }, 'filter.stage'],
54+
['inside $or', { $or: [{ stage: {} }, { owner: 'u2' }] }, 'filter.$or[0].stage'],
55+
['inside $and', { $and: [{ stage: 'won' }, { owner: {} }] }, 'filter.$and[1].owner'],
56+
['inside $not', { $not: { stage: {} } }, 'filter.$not.stage'],
57+
['nested two combinators deep', { $and: [{ $or: [{ stage: {} }] }] }, 'filter.$and[0].$or[0].stage'],
58+
];
59+
60+
for (const [name, filter, position] of positions) {
61+
it(`${name} → INVALID_FILTER naming ${position}`, () => {
62+
const err = refusalOf(filter);
63+
expect(err.code).toBe('INVALID_FILTER');
64+
expect(err.status).toBe(400);
65+
expect(err.message).toContain(position);
66+
expect(err.message).toContain('zero operators');
67+
});
68+
}
69+
70+
// ── The behaviour change, stated as tests ─────────────────────────────────
71+
72+
describe('the RLS `check` consequence, pinned explicitly', () => {
73+
it('a check that used to DENY now fails the operation instead', () => {
74+
// Same outcome for the caller's data (the write does not land), different
75+
// failure: INVALID_FILTER names the broken policy instead of blaming the
76+
// writer with a permission denial.
77+
expect(() => matchesFilterCondition(RECORD, { stage: {} } as never)).toThrow(/zero operators/);
78+
});
79+
80+
it('a check that used to ALLOW (the false disjunct was absorbed) now fails', () => {
81+
// Pre-fix: `{ a: {} }` → false, `{ owner: 'u1' }` → true, `$or` → true →
82+
// the write was permitted by a policy half of which was meaningless.
83+
expect(() => matchesFilterCondition(RECORD, { $or: [{ stage: {} }, { owner: 'u1' }] } as never))
84+
.toThrow(/zero operators/);
85+
});
86+
87+
it('a check under $not that used to ALLOW now fails', () => {
88+
// Pre-fix: `!false` → true → permitted.
89+
expect(() => matchesFilterCondition(RECORD, { $not: { stage: {} } } as never)).toThrow(/zero operators/);
90+
});
91+
92+
it('the refusal does not depend on the RECORD, so a policy is not row-dependent', () => {
93+
const rows = [RECORD, { id: '2', stage: 'lost', owner: 'u2', amount: 20 }, {}];
94+
for (const row of rows) {
95+
expect(() => matchesFilterCondition(row as never, { stage: 'no-match', owner: {} } as never))
96+
.toThrow(/zero operators/);
97+
}
98+
});
99+
});
100+
101+
// ── The fail-closed posture is otherwise untouched ────────────────────────
102+
103+
describe('every other unevaluable shape still fails CLOSED (returns false)', () => {
104+
it('an unknown operator', () => {
105+
expect(matchesFilterCondition(RECORD, { stage: { $sounds_like: 'won' } } as never)).toBe(false);
106+
});
107+
108+
it('a nested relation object a flat record cannot satisfy', () => {
109+
expect(matchesFilterCondition(RECORD, { stage: { nested: 'won' } } as never)).toBe(false);
110+
});
111+
112+
it('a bare array field spec', () => {
113+
expect(matchesFilterCondition(RECORD, { stage: ['won'] } as never)).toBe(false);
114+
});
115+
116+
it('an unknown top-level operator', () => {
117+
expect(matchesFilterCondition(RECORD, { $nope: 1 } as never)).toBe(false);
118+
});
119+
120+
it('a non-object filter', () => {
121+
expect(matchesFilterCondition(RECORD, 'x' as never)).toBe(false);
122+
});
123+
});
124+
125+
describe('ordinary evaluation is byte-identical', () => {
126+
it('answers the same booleans as before', () => {
127+
expect(matchesFilterCondition(RECORD, { stage: 'won' } as never)).toBe(true);
128+
expect(matchesFilterCondition(RECORD, { stage: 'lost' } as never)).toBe(false);
129+
expect(matchesFilterCondition(RECORD, { amount: { $gt: 5 } } as never)).toBe(true);
130+
expect(matchesFilterCondition(RECORD, { $or: [{ stage: 'lost' }, { owner: 'u1' }] } as never)).toBe(true);
131+
expect(matchesFilterCondition(RECORD, { $not: { stage: 'won' } } as never)).toBe(false);
132+
expect(matchesFilterCondition(RECORD, {} as never)).toBe(true);
133+
expect(matchesFilterCondition(RECORD, null)).toBe(true);
134+
});
135+
136+
it('an EMPTY NODE keeps its #5134 identity meaning, it is not this shape', () => {
137+
expect(matchesFilterCondition(RECORD, { $or: [{ stage: 'lost' }, {}] } as never)).toBe(true);
138+
expect(matchesFilterCondition(RECORD, { $not: {} } as never)).toBe(false);
139+
});
140+
141+
it('a Date comparand enumerates to nothing but is NOT a zero-operator constraint', () => {
142+
const d = new Date('2026-01-01T00:00:00Z');
143+
expect(matchesFilterCondition({ due: d }, { due: d } as never)).toBe(true);
144+
});
145+
});
146+
});

‎packages/formula/src/matches-filter.ts‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,18 +14,112 @@
1414
* node, an unknown operator, a nested relation object a flat record can't
1515
* satisfy — returns `false` (the write is denied), never `true`. The operator
1616
* vocabulary mirrors `read-scope-sql.ts` so the in-memory and SQL backends agree.
17+
*
18+
* ONE shape is refused instead of answered (#5240): `{ field: {} }`, a field
19+
* constrained by zero operators, throws `INVALID_FILTER` rather than returning
20+
* `false`. It is the shape the four backends could not agree on, so no answer
21+
* here is defensible; the operation fails, which is the #4775 posture for a
22+
* `check` that cannot be evaluated. Note this is not merely a louder denial:
23+
* where such a constraint sat under an `$or` beside a satisfied branch, or under
24+
* a `$not`, the old `false` was ABSORBED and the write was allowed. Those writes
25+
* now fail. See {@link emptyFieldConstraintError}.
1726
*/
1827

1928
import type { FilterCondition } from '@objectstack/spec/data';
2029
import { nextUtcCalendarDay, utcInstantMs } from '@objectstack/spec/data';
30+
import { StandardErrorCode } from '@objectstack/spec/api';
31+
32+
/**
33+
* [#5240] `{ field: {} }` — a field constrained by ZERO operators — is REFUSED,
34+
* not evaluated, and this is the ONE place this fail-closed evaluator throws.
35+
*
36+
* The shape had three answers in the repo: `driver-sql` refused it at the top
37+
* level but dropped it inside `$and`/`$or`/`$not` (a predicate that emits
38+
* nothing matches every row), `driver-memory` answered "matches nothing" by
39+
* accident of structural equality, and THIS evaluator answered `false` from the
40+
* explicit `keys.length === 0` arm below. Ruled on #5240: refused in all four
41+
* backends, with the same `INVALID_FILTER` code, so an authoring accident — a
42+
* filter builder that recorded a field and never its operator — fails loudly at
43+
* the producer instead of quietly changing a row count per backend.
44+
*
45+
* # Why this one throw does not weaken the fail-closed posture
46+
*
47+
* "Fail closed" is about what an UNEVALUABLE condition does to an ANSWER: it
48+
* must never widen access. Throwing is the strongest form of that — there is no
49+
* answer to widen — and it lands on the posture #4775 already settled for this
50+
* surface: a `check` that cannot be evaluated fails the operation. What changes
51+
* is the shape of the failure, and one case where the outcome flips outright:
52+
* a `check` whose broken constraint sat under an `$or` beside a satisfied
53+
* branch, or under a `$not`, used to evaluate to ALLOW. Those writes now fail.
54+
* That is a real, observable behaviour change and it is the point of the ruling
55+
* — the alternative is a permission rule whose meaning depends on which of four
56+
* backends evaluated it.
57+
*/
58+
function emptyFieldConstraintError(field: string, path: string): Error {
59+
const err = new Error(
60+
`Field constraint at ${path} carries zero operators ({ "${field}": {} }). A field constraint ` +
61+
`must name at least one operator (e.g. { "${field}": { "$eq": "value" } }) or be a direct ` +
62+
`comparand (e.g. { "${field}": "value" }). It is refused rather than evaluated because the ` +
63+
`backends disagreed on what it means — driver-sql dropped it inside $and/$or/$not (matching ` +
64+
`EVERY row) while refusing it at the top level, and driver-memory / this evaluator ` +
65+
`answered "matches nothing". #5240.`,
66+
) as Error & { code?: string; status?: number };
67+
err.code = StandardErrorCode.enum.INVALID_FILTER;
68+
err.status = 400;
69+
return err;
70+
}
2171

2272
/** True iff `record` satisfies `filter`. A null/empty filter matches everything. */
2373
export function matchesFilterCondition(record: Record<string, unknown>, filter: FilterCondition | null | undefined): boolean {
2474
if (filter == null) return true;
2575
if (typeof filter !== 'object' || Array.isArray(filter)) return false;
76+
// [#5240] Shape first, then evaluate. The refusal is raised by a walk of the
77+
// WHOLE tree, up front, rather than from inside `evalField` — because the
78+
// evaluator short-circuits (`every`/`some`, and a node returns on its first
79+
// false entry), so a refusal raised mid-evaluation would fire or not fire
80+
// depending on the RECORD being tested. A malformed permission rule must be
81+
// refused for every record or none. Evaluation below is untouched.
82+
assertFilterShape(filter as Record<string, unknown>, 'filter');
2683
return evalNode(record, filter as Record<string, unknown>);
2784
}
2885

86+
/**
87+
* [#5240] Walk the whole condition tree and refuse any zero-operator field
88+
* constraint. Shapes this evaluator already answers fail-closed (a non-node
89+
* `$and` element, an unknown `$`-operator, a bare array field spec) are left to
90+
* it — this walk adds exactly one refusal and changes nothing else.
91+
*/
92+
function assertFilterShape(node: unknown, path: string): void {
93+
if (node == null || typeof node !== 'object' || Array.isArray(node)) return;
94+
for (const [key, val] of Object.entries(node as Record<string, unknown>)) {
95+
const here = `${path}.${key}`;
96+
if (key === '$and' || key === '$or') {
97+
if (Array.isArray(val)) val.forEach((child, i) => assertFilterShape(child, `${here}[${i}]`));
98+
continue;
99+
}
100+
if (key === '$not') {
101+
assertFilterShape(val, here);
102+
continue;
103+
}
104+
if (key.startsWith('$')) continue;
105+
if (isEmptyFieldConstraint(val)) throw emptyFieldConstraintError(key, here);
106+
}
107+
}
108+
109+
/**
110+
* [#5240] Is this field spec `{}` — a field constrained by ZERO operators?
111+
*
112+
* A plain object with no own enumerable keys, and nothing else: a `Date` also
113+
* enumerates to nothing but is a COMPARAND (`evalField` treats it as implicit
114+
* equality), not a constraint.
115+
*/
116+
function isEmptyFieldConstraint(spec: unknown): boolean {
117+
if (spec === null || typeof spec !== 'object' || Array.isArray(spec) || spec instanceof Date) return false;
118+
const proto = Object.getPrototypeOf(spec);
119+
if (proto !== Object.prototype && proto !== null) return false;
120+
return Object.keys(spec as Record<string, unknown>).length === 0;
121+
}
122+
29123
function evalNode(record: Record<string, unknown>, node: Record<string, unknown>): boolean {
30124
// A node is the AND of all its entries.
31125
for (const [key, val] of Object.entries(node)) {
@@ -58,6 +152,12 @@ function evalField(record: Record<string, unknown>, field: string, spec: unknown
58152
const keys = Object.keys(ops);
59153
// Must be all-operators; a non-`$` key means a nested relation a flat record
60154
// cannot satisfy → fail closed.
155+
//
156+
// [#5240] `keys.length === 0` no longer reaches this arm on the public entry
157+
// point: `assertFilterShape` refuses `{ field: {} }` before evaluation starts.
158+
// The clause stays because this function is also reachable from a recursive
159+
// `evalNode` on a subtree, and a total function must stay total — but it is a
160+
// floor, no longer this backend's ANSWER to the shape.
61161
if (keys.length === 0 || keys.some((k) => !k.startsWith('$'))) return false;
62162
for (const op of keys) {
63163
if (!evalOp(actual, op, ops[op], record)) return false;

0 commit comments

Comments
 (0)