fix(approvals): 记录锁对谓词式(multi)更新同样生效 (#4778)

[os-zhuang]

Fixes #4778

问题

bindApprovalLockHook 注册的全局 beforeUpdate(ADR-0019 记录锁)开头是 if (!id) return。而 update() 只在 where.id 是标量时才提取 input.id,任何谓词式更新都走 updateMany 且 input.id 为 undefined —— 于是「没解析到行」被当成「没有东西需要授权」,实际是「从来没有查询过」。

这是 #4757 / #4630 同一族的 fail-open,但绕过成本最低:不需要 admin、不需要 isSystem、不需要 lockRecord: false、也不需要命中 approvalStatusField 白名单,只要把同一次编辑写成 multi: true:

await ql.update('crm_opportunity', { amount: 999 }, { where: { id: 'rec_1' } });                        // RECORD_LOCKED
await ql.update('crm_opportunity', { amount: 999 }, { where: { id: { $in: ['rec_1'] } }, multi: true }); // 曾经通过
await ql.update('crm_opportunity', { amount: 999 }, { where: { name: 'x' }, multi: true });              // 曾经通过

修法

hook 现在先解析这次写入会碰到哪些行,再逐行判定。

• 按 id 的路径不变:driver 按主键写,where 的其余部分不参与,所以这里不做任何收窄 —— 收窄反而会变成新的 fail-open(where: { id: 'rec_1', stage: 'x' } 匹配不上时会误放行,而 driver 照写不误)。
• 谓词路径:把调用方的 where 与该对象当前被锁的记录求交({ $and: [where, { id: { $in: lockedIds } }] })。方向是反过来的,这正是它便宜的原因 —— 上界落在对象上的 pending 审批数,而不是更新的匹配集:5 万行的无锁批量更新只多一次 bookkeeping 查询,照常放行。
• 完全无谓词的整表 multi 更新碰到该对象的每一条被锁记录,只要有一条被锁就拒绝。
• 失败关闭:被锁记录超过 1000 条(与 MULTI_DELETE_AUTH_LIMIT / MULTI_WRITE_AUTH_LIMIT 同一上界),或求交查询本身失败 —— 都拒绝写入,因为锁无法证明这次写入避开了被锁行。
• 唯一保留的 fail-open 是「审批表整个读不出来」:这个 hook 对所有对象全局生效,若在此失败关闭,一个没有 sys_approval_request 的 kernel 会拒绝掉部署里的每一次更新。这一条是既有行为,注释里写明了为什么它与前面几条不同(攻击者无法操纵它)。
• bookkeeping 与匹配集解析都用 system context 读:守卫自己的输入不能被调用方的可见性收窄 —— 读不到的被锁行,仍然是不许写的行。

豁免逐条随守卫一起搬过去

把守卫扩到多行最常见的错误是只搬拒绝逻辑,于是从 fail open 直接翻成误伤。五条既有豁免在多行路径上都逐一有测试(且在 $in 与非 id 谓词两种形状上各测一遍):

豁免	多行路径
isSystem(引擎自写 / 状态镜像)	✅
admin 覆盖	✅
approvalStatusField 镜像写	✅(只改镜像字段才放行,多带一个字段仍然拒绝)
lockRecord: false	✅
flowRunId 同源写(#3456 / #3712)	✅(别的 run 仍然拒绝)

测试

• approval-service.test.ts —— 在现有 record-lock hook (node era) 用例旁新增 record-lock hook — predicate (multi) updates (#4778):拒绝、不误伤(谓词匹配不到被锁行时放行)、上界失败关闭、求交查询失败时失败关闭、无 pending 时完全不扫描业务表(用 _finds 钉住),以及上表五条豁免 × 两种谓词形状。
• 新增 record-lock-multi-update.integration.test.ts —— 真实 ObjectQL 引擎 + 支持 $in / $and / updateMany 的内存 driver,由引擎自己决定走 by-id 还是 updateMany 并构造 hook context(即产生这个 bug 的那一跳,不 stub),复现 issue 里的三行。
• 回归证明:把 if (!id) return 临时放回去,这两个文件里 14 条用例失败;恢复后 pnpm --filter @objectstack/plugin-approvals test 402 passed / 18 files,typecheck 干净,eslint 干净。

范围

改动只在 packages/plugins/plugin-approvals/src/ 与一个 changeset(patch:没有新增包根导出)。未改 packages/objectql/src/engine.ts —— engine 的 id 提取规则是刻意的,这次修的是消费方对「解析不到行」的错误推理。

顺带发现(未在本 PR 处理)

session.roles 在 ObjectQL 的 buildSession() 里从未被填充,所以记录锁与 delegation 守卫里的 roles.includes('admin') 在真实引擎路径上大概率永不生效,而且与 ADR-0095 / isOverrideActor 的 permissions/positions/posture 词汇不一致。两处都是失败关闭(admin 被误拒,不是越权),因此按 PD #10 单独立 issue,不在本 PR 扩范围。

🤖 Generated with Claude Code

https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny

---

Generated by Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
objectstack	Ignored		Aug 3, 2026 9:39am

[github-actions]

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-approvals.

5 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

• content/docs/automation/approvals.mdx (via @objectstack/plugin-approvals)
• content/docs/kernel/services-checklist.mdx (via @objectstack/plugin-approvals)
• content/docs/plugins/packages.mdx (via @objectstack/plugin-approvals)
• content/docs/releases/implementation-status.mdx (via @objectstack/plugin-approvals)
• content/docs/releases/v9.mdx (via @objectstack/plugin-approvals)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

[os-zhuang]

复核通过 —— ACCEPT,已标 ready 并送合并队列

逐条核过,记录几点判断依据:

1. 修复位置对。 守卫不是在六个调用点各补一遍,而是把"这次写入会碰到哪些行"的解析放进 hook 本身,与 #4757(sys_attachment)/ #4630(sys_comment)同源。if (!id) return 把「没解析出行」读成「没有东西需要授权」,而真相是「压根没查过」——这个判断在注释里写清楚了,不是靠猜。

2. 复杂度的方向选对了,这是本 PR 最容易做错而没做错的地方。 上界落在该对象上待审批的记录数(正常为 0,一次查询就返回),而不是更新的匹配集。所以 5 万行无锁的批量更新只多一次 bookkeeping 查询、照常放行。若把上界放在匹配集上,这个守卫会把每一次大批量更新都变成灾难。

3. 五条豁免在多行路径上确实各有测试。 我第一次核查时用 ^\+\s*it\( 去 grep,漏掉了 it.each(SHAPES)('allows an admin override via %s') 这条,一度以为 admin 那条缺测。是我查法不对,更正在此。五条齐:isSystem / admin / approvalStatusField 镜像写 / lockRecord: false / flowRunId 同源写。派发时我强调过「扩守卫最常见的错误是只搬拒绝逻辑、不搬豁免逻辑,从 fail open 直接翻到误伤合法路径」——这一条守住了。

4. 唯一保留的 fail-open 是被论证过的,不是遗漏。 pendingRequestsForObject 读不到 bookkeeping 时当作「无锁」。理由站得住:这个 hook 对每个对象全局生效,若在此 fail closed,一个 sys_approval_request 缺表或瞬时不可读的 kernel 会拒绝整个部署的所有更新。而攻击者真正能操纵的那些决策(自选谓词、超上界、交集查询失败)全部 fail closed。这个取舍写进了代码注释,符合"静默降级必须响亮"的要求。

5. changeset patch 正确。 我查了仓内同族先例:#4757(附件未限定多删)、#3456 / #3712(审批锁的两次修复)、#4728(DDL 响亮化)全部是 patch。lifecycle-hooks 不在包根 index.ts 导出,无新增公共 API,与既有做法一致。

6. 反向验证做得对。 把 if (!id) return 临时放回去后 14 failed | 258 passed,恢复后全绿——证明这批测试确实钉住了缺陷,而不是恰好能过。

范围外发现已妥善处理

#4839(admin 豁免读 session.roles,而 buildSession() 从不填充该字段,故记录锁与 delegation 守卫的 admin 覆盖在真实引擎路径上永不生效)没有在本 PR 里顺手改——这是正确的。它涉及权限放宽或 spec 字段退役,需维护者裁定,混进一个安全修复里会让两件事互相绑架。

值得指出它与 #4837 是同一个形态:声明了、消费端也有读取代码、gate 全绿,但没有任何生产者传值。#4837 是 Seed.env(六个调用点都不传),#4839 是 session.roles(全仓仅两处读、无一处写)。这已经是今天第三次遇到这个形状了。

⚠️ 若合并队列把本 PR 踢出,大概率是 #4796 那条 spec flaky(今晚已 5 次命中无关 PR),止血单 #4850 在飞。届时原样重投,不必返工。

---

Generated by Claude Code