Skip to content

审批记录锁 bindApprovalLockHook 对谓词式(multi)更新完全失效:if (!id) return 把「没解析到行」当成「允许」 #4778

Closed

Description

@os-zhuang

在 #4757 的排查中发现(那条 issue 末尾要求「看看仓库里还有没有别的 beforeDelete 守卫共享『没解析到行 ⇒ 放行』这个形状」)。未认领,归属 plugin-approvals,不在 #4757 的 PR(#4780)范围内。

现象

packages/plugins/plugin-approvals/src/lifecycle-hooks.ts,bindApprovalLockHook 注册的全局 beforeUpdate(ADR-0019 记录锁)开头就是:

engine.registerHook('beforeUpdate', async (ctx: any) => {
  const id = String((ctx?.input?.id ?? '') as string);
  if (!id) return;

而 packages/objectql/src/engine.ts 的 update() 只在 where.id 是标量时才把它提取成 input.id(与 delete() 同源的规则,注释里写明了:操作符对象 { $in: [...] } 是多行谓词,要走 updateMany)。于是任何谓词式更新到达这个 hook 时 input.id 都是 undefined,锁直接 return 放行:

// 记录 rec_1 上挂着 pending 的 sys_approval_request,lockRecord 未关闭
await ql.update('crm_opportunity', { where: { id: 'rec_1' }, data: { amount: 999 } });   // 被 RECORD_LOCKED 拦下 ✅
await ql.update('crm_opportunity', { where: { id: { $in: ['rec_1'] } }, multi: true, data: { amount: 999 } }); // 通过 ❌
await ql.update('crm_opportunity', { where: { name: 'x' }, multi: true, data: { amount: 999 } });             // 通过 ❌

也就是说,只要把同一次编辑改写成 multi: true 的形式,审批期间的记录锁就形同虚设 —— 不需要 admin 角色,不需要 isSystem,不需要 lockRecord: false,也不需要命中 approvalStatusField 白名单。SDK / ObjectQL / flow 的 update_record 类调用方都能产生这个形状。

为什么这是同一个 fail-open 家族

和 #4757(sys_attachment 的 beforeDelete)是同一个错误推理:「没有解析到目标行」被当作「没有东西需要授权」,而实际上是「从来没有查询过」。#4630 给 sys_comment 立的 resolveTargetRows 是正确姿势的参考。

区别在于修法不同 —— 这里不能照抄「无 id 无 where ⇒ 拒绝」:记录锁是按行判定的守卫(某条记录是否有 pending 审批),不是「整表操作要不要拒绝」。正确的做法应该是把 where 解析成匹配行集合(带上界,超限失败关闭),再对每一行查 pending 请求 —— 与 attachment / comment 两个守卫解析行集合的方式一致。

验收建议

同轮排查的其它位置(仅记录,不必在本 issue 处理)

同一形状在这些地方也存在,但它们不是授权守卫,后果是数据/可观测性而非越权,且多数在注释里已被声明为已知边界:

  • packages/plugins/plugin-audit/src/audit-writers.ts 的 captureBefore:if (!id) return; // bulk update/delete — too costly to snapshot every row here —— 批量删除不留审计快照(已声明的取舍)。
  • plugin-webhooks / plugin-email / plugin-sharing 三处 provenance 盖章 hook:if (!id) return; // multi-row update — see boundary note above(注释里明确写了 known boundary)。
  • plugin-sharing/src/primary-bu-projection.ts 的 beforeDelete:批量删成员行时 sys_user.primary_business_unit_id 投影不更新(设计上 best-effort,靠下次写入或 boot backfill 自愈)。

plugin-auth/src/identity-write-guard.ts 不在此列 —— 它的 beforeDelete 是无条件拒绝,beforeUpdate 走字段白名单,不依赖行解析。plugin-sharing/src/rule-hooks.ts 单独立在 #4779。

Activity

  1. os-zhuang commented on Aug 3, 2026

    @os-zhuang
    ContributorAuthor

    分诊:入队 pm:queue,下一轮派发(本轮 4 个位置已满,且 plugin-approvals 正被在飞的 #4771 改动,同批会撞车)。无需维护者裁定 —— 修法在 issue 里已经写清,且与 #4630 / #4757 已确立的姿势同源。

    这条是本轮排查最有价值的产出。 值得指出它比 #4757 更严重的地方:#4757 需要 app 发一份带 sys_attachment 删除位的领域授权才够得着;而这条不需要任何特权 —— 不需要 admin 角色、不需要 isSystem、不需要 lockRecord: false、也不需要命中白名单字段,只要把同一次编辑写成 multi: true 的形式,审批期间的记录锁就形同虚设。绕过成本是「换一种写法」,这是最坏的一种绕过成本。

    派发时会带上 issue 里已列的四条验收,另加一条:既有豁免必须在多行路径上同样生效(lockRecord: false、admin 覆盖、approvalStatusField 镜像写、isSystem、flowRunId 同源写)。把守卫扩到多行时最容易犯的错是只搬了拒绝逻辑、没搬豁免逻辑,结果从 fail open 直接翻到误伤合法路径。

    关联:#4776(这一族 fail-open 的结构性防线决策单)—— 本条与 #4757 的共同错误推理「没解析到行 ⇒ 没有东西需要授权」是 #4776 讨论的形状在授权面的变体。


    Generated by Claude Code

  2. added
    domain:servicesRetired 2026-10-22, alias of domain:server (services surface); in-flight cards keep it until landed
    pm:dispatchedDispatched to a dev agent by /pm-dispatch
    and removed
    pm:queueReady for the PM dispatch loop
    on Aug 3, 2026
  3. self-assigned this
    on Aug 3, 2026
  4. os-zhuang commented on Aug 3, 2026

    @os-zhuang
    ContributorAuthor

    认领:PM 循环第 5 轮
    会话:session_015Br2xsJsczFsTR9bvbh2Ny
    分支:claude/issue-4778-approval-lock-multi-update
    Worktree:objectstack-issue-4778
    域:domain:services
    文件面:packages/plugins/plugin-approvals/src/(越界即停)


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingdomain:servicesRetired 2026-10-22, alias of domain:server (services surface); in-flight cards keep it until landedpm:dispatchedDispatched to a dev agent by /pm-dispatchsecurity

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions