Skip to content

Commit e15bf7e

Browse files
os-zhuangclaude
andauthored
fix(analytics): refuse an undefined comparand in a read scope instead of binding it (#6125) (#6390)
`compileScopedFilterToSql` compiled a comparand-position `undefined` into legal SQL with `undefined` in the bind list. The driver renders that as NULL, every comparison against NULL is UNKNOWN, and the read scope matched ZERO rows with no log line — indistinguishable from a scope that worked. Re-measured on d8e8d9c with the refusal disabled, alias `t`: { d: undefined } -> "t"."d" = ? [undefined] { d: { $gt: undefined } } -> "t"."d" > ? [undefined] { d: { $in: [undefined] } } -> "t"."d" IN (?) [undefined] { $not: { d: undefined } } -> NOT (("t"."d" IS NOT NULL AND "t"."d" = ?)) [undefined] One gate at the top of `compileField` — after `quoteIdent`, before any `bind()` — refuses all four positions with ONE wording that varies only by path (#5240). The envelope is this module's existing `READ_SCOPE_COMPILE_FAILED` / 500, not #6050's `INVALID_FILTER` / 400: a read scope is compiled by the platform from CEL and stored metadata, so a 400 would bill the caller for something they neither wrote nor can change. `null` is untouched — SQL and binds byte for byte, pinned as its own control group, because refusing it alongside `undefined` is the way this change could do harm. Per the #6125 ruling, `@objectstack/formula` (a third semantics, #5299), `driver-memory` / `driver-mongodb` (#5499 freeze) and `driver-sql` / `driver-turso` (#6050, already landed) are deliberately not touched. Fixes #6125 Claude-Session: https://claude.ai/code/session_01WyvqvKMG6asi9aXjKE6xtx Co-authored-by: Claude <noreply@anthropic.com>
1 parent cb3b6cd commit e15bf7e

5 files changed

Lines changed: 527 additions & 11 deletions

File tree

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
---
2+
"@objectstack/service-analytics": patch
3+
---
4+
5+
fix(analytics): read scope 里的 `undefined` 比较数改为拒收,不再编成绑了 `undefined` 的合法 SQL (#6125)
6+
7+
**⚠️ 行为变更。** `compileScopedFilterToSql` 遇到比较数位置上的 `undefined`,从「编出合法 SQL、绑一个 `undefined`、匹配零行、零日志」改为 `READ_SCOPE_COMPILE_FAILED` / **500** 拒收。
8+
9+
## 实测到的毛病
10+
11+
#6050 于 2026-08-07 裁定(B 案):比较数位置的 `undefined` 一律拒收,并落在了**已证实可触达**`driver-sql` / `driver-turso` 两面。#6125 在同一轮把仓内其余求值面逐格实测,同一个形状拿到五种读法;本条改的是其中一格 —— `service-analytics``read-scope-sql.ts`。在 `d8e8d9cbc` 上把本次拒收关掉复测,alias `t`、字段 `d`,四格与 #6125 正文表一致:
12+
13+
| read scope | 编译结果 | 绑定表 |
14+
|---|---|---|
15+
| `{ d: undefined }` | `"t"."d" = ?` | `[undefined]` |
16+
| `{ d: { $gt: undefined } }` | `"t"."d" > ?` | `[undefined]` |
17+
| `{ d: { $in: [undefined] } }` | `"t"."d" IN (?)` | `[undefined]` |
18+
| `{ $not: { d: undefined } }` | `NOT (("t"."d" IS NOT NULL AND "t"."d" = ?))` | `[undefined]` |
19+
20+
绑定表里是 JS 的 `undefined` 本身,不是 `null``applyReadScope``native-sql-strategy.ts`)在把 `?` 改写成 `$N` 时原样 `push(scopeParams[i])`。所以 NULL 是**驱动**对一个 JS `undefined` 的读法 —— 同一格在不肯猜的驱动上则是一句裸 `Undefined binding(s)` 崩溃。一次绑定、两种败法,取决于数据源恰好挂的是哪个驱动,这正是它该在编译器处拒收、而不是在某一个消费者处修补的理由。
21+
22+
方向与 #6050 不同,如实记:那边是**越权**`{ owner_id: ctx.user?.id }` 在 Turso remote 上编成 `IS NULL`,匹配全环境行);这边是 fail-**closed** —— 匹配零行,永远不会多给行。所以它不是潜伏的权限绕过,#6125 也没有按那个级别定级。之所以照样拒收:一个「答了没人问的问题、且一条日志都不报」的 read scope,与一个真的生效了的 read scope 在外部完全无法区分。本次改动的价值就是把沉默变成响亮。
23+
24+
## 修法
25+
26+
一道闸落在 `compileField` 的开头 —— 在 `quoteIdent` 之后(不安全标识符是注入向量,保留它自己的措辞与优先级),在任何 `bind()` 之前。
27+
28+
拒收的**位置**逐个清点,因为「比较数」是位置而不是类型:直接比较数(`{ d: undefined }`)、单值算子的比较数(`$eq`/`$ne`/`$gt`/`$gte`/`$lt`/`$lte` 与 LIKE 族)、列表算子数组的**成员**`$in`/`$nin`/`$between`)。四格共用**一条**措辞,只有 `path` 不同(#5240「一个条件,一种措辞」)。
29+
30+
信封沿用本模块自述的那一个(`READ_SCOPE_COMPILE_FAILED` / 500),不是 #6050`INVALID_FILTER` / 400:read scope 的 filter 由平台自己从 CEL 与库存 metadata 编译而来,不是调用方输入 —— 报 400 等于让调用方去修一个他既没写、也改不动的东西。消息里指名要修的是**生产者**(管理员写的共享规则 / 权限集、它的 CEL 下降、或进程内拼这条 FilterCondition 的代码),并按 #5367 只进日志、不进响应体。
31+
32+
三个位置**故意不扫**,各自因为本模块已经用更贴切的诊断拒了它:`$null` / `$exists`(比较数是声明的布尔量,不是比较数位置)、直接位置上的裸数组(`compileField` 整体拒「用 `{ $in: [...] }`」)、以及约束对象里的非 `$` 键(那是嵌套关系,改写成 `null` 一样编不过 —— 这一条是与 `driver-sql` 孪生实现的唯一有意分歧,来自本模块拒收嵌套关系,而不是对 #6050 的另一种读法)。
33+
34+
## `null` 一字未动
35+
36+
`{ d: null }` / `{ $eq: null }``IS NULL``{ $ne: null }``IS NOT NULL``$null` / `$exists``$in: [null]``$nin: [null]``$between: [null, 5]``$contains: null``%null%`#5526)、以及 `$not` 下的各式 —— SQL 与绑定表逐字节不变。这是本次改动唯一可能造成伤害的方向(模块里每张极性表都只用一个 `===``null``undefined` 分开),所以它有自己的对照组回归 pin。
37+
38+
## 刻意不动的邻居
39+
40+
-`@objectstack/formula` 把同一个 `undefined` 读作「这个键在记录里不存在」—— 那是**第三种语义**,不是第三个 bug 拼写,也正是 #5299 在争的问题。在这里顺手改掉等于替 #5299 拍板。
41+
-`driver-memory` / `driver-mongodb` 维持 #5499 投入冻结,只 pin 不改。后果是本编译器与 `driver-memory` 在这一格上从此不一致 —— 这是裁决接受的代价,解冻时一并还,账记在 #6125
42+
-`driver-sql` / `driver-turso` 已由 #6050 落地,未触碰。

packages/services/service-analytics/src/__tests__/read-scope-refusal-envelope.test.ts

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,18 @@
4141
* code and all ten fail with `code` / `status` `undefined`, while the block above
4242
* stays green.
4343
*
44+
* ## [#6125] An eleventh site, and why it is registered HERE
45+
*
46+
* The `undefined` comparand refusal (row ⑥) was added on 2026-08-07 by #6125's
47+
* ruling. It is in this table for the invariant at the bottom of the file rather
48+
* than for its own sake: what #5352 cost was a module where SOME refusals
49+
* carried the envelope, which is indistinguishable from none of them at the HTTP
50+
* boundary. So the rule this table encodes is that the inventory grows whenever
51+
* the module gains a refusing site — a new `throw` that is not listed here is
52+
* the defect returning. Row ⑥'s own behaviour (four comparand positions, and the
53+
* `null` control group that must NOT move) is pinned in
54+
* `read-scope-undefined-comparand.test.ts`; only its ENVELOPE is asserted here.
55+
*
4456
* The end-to-end consequence — that the REST face answers
4557
* `500 ANALYTICS_QUERY_FAILED` with the policy content WITHHELD from the body and
4658
* intact in `logError` — is `packages/rest`'s
@@ -69,7 +81,7 @@ function refusalFor(filter: unknown, alias = 'crm_opportunity'): Refusal | undef
6981
/**
7082
* Every refusing site in `read-scope-sql.ts`, in source order.
7183
*
72-
* ELEVEN rows over TEN throw sites: `quoteIdent` is one site reached with two
84+
* TWELVE rows over ELEVEN throw sites: `quoteIdent` is one site reached with two
7385
* `kind` values, and both are listed on purpose. That alias-vs-field split was
7486
* option C on #5367's decision card — the one place where the two triggers are
7587
* genuinely different (a bad alias is OUR generator, a bad field is the admin's
@@ -129,42 +141,49 @@ const REFUSALS: Array<{
129141
sensitive: '$nor',
130142
},
131143
{
132-
name: '⑥ bare array value',
144+
name: '⑥ undefined in a comparand position',
145+
site: 'compileField: undefined comparand',
146+
filter: { owner_id: undefined },
147+
message: /comparand at "owner_id" is undefined refusing to build read scope \(fail-closed\)/,
148+
sensitive: 'owner_id',
149+
},
150+
{
151+
name: '⑦ bare array value',
133152
site: 'compileField: bare array value',
134153
filter: { region_code: ['emea', 'apac'] },
135154
message: /bare array value for "region_code" use \{ \$in: \[\.\.\.\] \} \(fail-closed\)/,
136155
sensitive: 'region_code',
137156
},
138157
{
139-
name: ' nested / relation value',
158+
name: ' nested / relation value',
140159
site: 'compileField: nested/relation value',
141160
filter: { owner: { manager_id: 'u1' } },
142161
message: /"owner" has a nested\/relation value which is not supported in a read scope \(fail-closed\)/,
143162
sensitive: 'owner',
144163
},
145164
{
146-
name: ' $in without an array',
165+
name: ' $in without an array',
147166
site: 'compileOperator: $in needs an array',
148167
filter: { region_code: { $in: 'emea' } },
149168
message: /\$in for "region_code" needs an array \(fail-closed\)/,
150169
sensitive: 'region_code',
151170
},
152171
{
153-
name: ' $nin without an array',
172+
name: ' $nin without an array',
154173
site: 'compileOperator: $nin needs an array',
155174
filter: { region_code: { $nin: 'emea' } },
156175
message: /\$nin for "region_code" needs an array \(fail-closed\)/,
157176
sensitive: 'region_code',
158177
},
159178
{
160-
name: ' $between without [min, max]',
179+
name: ' $between without [min,max]',
161180
site: 'compileOperator: $between needs [min,max]',
162181
filter: { credit_limit: { $between: [10] } },
163182
message: /\$between for "credit_limit" needs \[min,max\] \(fail-closed\)/,
164183
sensitive: 'credit_limit',
165184
},
166185
{
167-
name: ' unsupported operator',
186+
name: ' unsupported operator',
168187
site: 'compileOperator: unsupported operator',
169188
filter: { owner_email: { $regex: 'admin@' } },
170189
message: /unsupported operator "\$regex" on "owner_email" \(fail-closed\)/,
@@ -255,10 +274,12 @@ describe('[#5367] every read-scope refusal carries the ADR-0112 envelope (READ_S
255274
// #5352's lesson, stated as a guard: seven of `filter-normalizer.ts`'s nine
256275
// sites carrying an envelope was indistinguishable from none of them at the
257276
// HTTP boundary, because the commonest input hit one of the two bare ones.
258-
// Eleven inputs over the module's ten throw sites (see the table's note on
259-
// `quoteIdent`), and every one of them enveloped.
260-
expect(REFUSALS).toHaveLength(11);
261-
expect(new Set(REFUSALS.map((c) => c.site)).size).toBe(10);
277+
// Twelve inputs over the module's ELEVEN throw sites (see the table's note
278+
// on `quoteIdent`), and every one of them enveloped. [#6125] added the
279+
// eleventh site; these two numbers are the ratchet that makes a future
280+
// unenveloped `throw` fail HERE instead of at an HTTP boundary.
281+
expect(REFUSALS).toHaveLength(12);
282+
expect(new Set(REFUSALS.map((c) => c.site)).size).toBe(11);
262283
for (const c of REFUSALS) {
263284
expect(refusalFor(c.filter, c.alias)?.code, `${c.site} is still bare`).toBe('READ_SCOPE_COMPILE_FAILED');
264285
}

0 commit comments

Comments
 (0)