Skip to content

Commit 7e5ac28

Browse files
fix(approvals): delete the two session.roles admin exemptions — record lock and delegation guard (#4839) (#5049)
`lifecycle-hooks.ts` carried two admin bypasses, both reading `ctx.session.roles`: the approval **record lock** (`bindApprovalLockHook`) and the `sys_approval_delegation` write guard (`bindDelegationWriteGuard`). Both are removed. Neither ever ran. `session.roles` has no producer anywhere in the platform — ObjectQL's `buildSession()` builds its session field by field and never writes it — so both branches were dead on every real engine path: the record lock has always applied to admins, and delegation has always been self-managed. The spec declares `roles` on the HookContext session, two consumers read it, nothing fills it: declared != enforced. They were also a second privilege dialect. Privilege here is judged by the ADR-0095 vocabulary (`permissions` / `positions` / derived posture) — ADR-0090 D3 bans the `role` spelling outright — and the sibling `ApprovalService.isOverrideActor` already does exactly that. Per the maintainer's ruling both sites are DELETED rather than rewired to the correct predicate: - Record lock: the sanctioned admin rescue path already exists (#3424 — recall / reject / reassign, gated by `isOverrideActor`, audited via `via_override`). Finalising the request is what releases the lock, so a record is never edited under a live approval. - Delegation: evidence-first, and the evidence says coverage exists. A delegation is consulted only at slate-resolution time (`applyOooDelegation` inside `resolveApproverSpec`), so forging one for an already-unavailable approver never moved their pending work; `reassign` / `recall` / reject do, and are gated by `isOverrideActor`. "Delegation is self-managed" is now the final semantic. Tests: `admin-exemption-retired.test.ts` turns that evidence into executable assertions and adds a source-level pin — no non-test source in this package may name a `roles` identifier or compare against the string 'admin'. The existing "allows an admin override" tests become "does NOT exempt ..." pins over both the retired dialect and every live ADR-0095 admin shape, on the by-id and predicate paths. The spec-side retirement of `session.roles` (now with zero consumers) is a separate protocol change under ADR-0049 enforce-or-remove — not in this PR. Claude-Session: https://claude.ai/code/session_01NrmBxj8rK2uGCnh9aipjwX Co-authored-by: Claude <noreply@anthropic.com>
1 parent 79822b5 commit 7e5ac28

4 files changed

Lines changed: 624 additions & 23 deletions

File tree

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
---
2+
"@objectstack/plugin-approvals": patch
3+
---
4+
5+
fix(approvals): 删除两处读 `session.roles` 的 admin 豁免 —— 记录锁与委托守卫回到单一权限词汇 (#4839)
6+
7+
`plugin-approvals` 的 `lifecycle-hooks.ts` 里有两处 admin 豁免,都读
8+
`ctx.session.roles`:审批**记录锁**的 `bindApprovalLockHook`,以及
9+
`sys_approval_delegation` 的 `bindDelegationWriteGuard`。两处都已删除。
10+
11+
**这不是行为变更。** `session.roles` 在整个平台没有生产者 —— ObjectQL 的
12+
`buildSession()` 逐字段构造 session,从不写 `roles` —— 所以两个分支在任何真实引擎
13+
路径上都是死代码,记录锁一直就对 admin 生效,委托一直就只能本人管理。删除让代码
14+
说出运行时本来就在做的事(spec 的 `HookContext` 声明了 `roles`,消费方在读,生产方
15+
从不写:典型的 declared ≠ enforced)。
16+
17+
**为什么不是「改用正确判据」而是删除。** `roles.includes('admin')` 还是第二套权限
18+
方言:本仓库的权限一律由 ADR-0095 词汇裁决(能力授予 `permissions`、任职
19+
`positions`、由其派生的 posture),ADR-0090 D3 更是直接禁掉 `role` 这个拼法。同包的
20+
`ApprovalService.isOverrideActor` 已经这么做了。维护者裁定两处都取「删除」而非改判据:
21+
22+
- **记录锁**:admin 释放锁定记录的正规路径已经存在(#3424 —— `recall` /
23+
`decideNode` 驳回 / `reassign`,全部由 `isOverrideActor` 把关并留痕
24+
`via_override`)。让审批终结来释放锁,记录就永远不会在审批在途时被改写 —— 这正是
25+
合规场景购买记录锁所要的保证。
26+
- **委托**:最终语义确定为**仅本人管理**(`delegator_id` 必须等于写入者;只有 system
27+
上下文旁路)。审批人临时不可用时,替他处置**在途**审批用的是
28+
`reassign`(把该审批人的名额交给替代人,连 per_group 分组归属一起带过去)/
29+
`recall` / 驳回。反过来,「替别人建一条委托」本来也做不到这件事:委托只在请求
30+
**开启**时(`resolveApproverSpec` 内的 `applyOooDelegation`)被查询,对已经挂在该
31+
审批人名下的在途审批毫无作用。
32+
33+
新增 `admin-exemption-retired.test.ts`,把上述证据变成可执行断言,并加了一道源码级
34+
pin:本包非测试源码中不得再出现 `roles` 标识符或与字符串 `'admin'` 的比较。
35+
36+
spec 侧 `session.roles` 的退役(至此零消费方)按 ADR-0049 enforce-or-remove 另立协议
37+
单处理,不在本次改动内。

0 commit comments

Comments
 (0)