|
@@ -1,308 +1,304 @@
|
|
|
-# 第 12 轮深度审计报告
|
|
|
|
|
|
|
+# 第 13 轮深度审计报告
|
|
|
|
|
|
|
|
审计对象:`perms-system/server`(不含测试代码)
|
|
审计对象:`perms-system/server`(不含测试代码)
|
|
|
审计维度:逻辑一致性、并发/竞态、资源管理、数据完整性、安全漏洞、边界、DB 性能、僵尸代码、接口契约
|
|
审计维度:逻辑一致性、并发/竞态、资源管理、数据完整性、安全漏洞、边界、DB 性能、僵尸代码、接口契约
|
|
|
-说明:本轮基于真实业务量级(数千用户 / 数十产品 / 单产品 <100 role / 一次 SyncPerms < 1k perm / 单 user 10~30 role)做判定。
|
|
|
|
|
|
|
+业务量级假设:数千级用户 / 数十级产品 / 单产品 < 100 角色 / 单次 SyncPermissions < 1k 权限 / 单用户 10~30 角色 / 峰值 QPS 数百
|
|
|
|
|
|
|
|
-## 🚩 核心逻辑漏洞 (High Risk)
|
|
|
|
|
|
|
+---
|
|
|
|
|
+
|
|
|
|
|
+## R12 闭环复核(均已修复,本轮抽检通过)
|
|
|
|
|
|
|
|
-本轮未发现新的 High Risk 漏洞。R11 的 H-R11-1 是上一轮可复现的密码 TOCTOU,本轮密码链路已经收敛;auth / token / dept / member / sync 四条主链已在锁序、乐观锁、CAS 三层护栏兜底,暂未看到可直接越权/篡改数据的 High 条目。
|
|
|
|
|
|
|
+| R12 条目 | 状态 | 落地证据 |
|
|
|
|
|
+| --- | --- | --- |
|
|
|
|
|
+| M-R12-1 `BindRoles`/`DeleteRole` 孤儿 `sys_user_role` | ✅ 已修 | `internal/model/role/sysRoleModel.go:LockRolesForShareTx` + `internal/logic/user/bindRolesLogic.go:122-129` 事务内 S 锁闭环 |
|
|
|
|
|
+| L-R12-1 `*WithTx` 预提交缓存失效 | ✅ 已修 | `internal/model/user/sysUserModel.go:UpdateProfileWithTx` 绕过 `m.ExecCtx`;`InvalidateProfileCache` 由 Logic 层 post-commit 显式调用(`updateUserLogic.go:205`) |
|
|
|
|
|
+| L-R12-2 `CreateDept` 父部门 Status 复核 | ✅ 已修 | `createDeptLogic.go:67-85` 事务内 `FindOneForShareTx` 取完整行并校验 `Status` |
|
|
|
|
|
+| L-R12-3 `UpdateRole` 注释与代码反向 | ✅ 已修 | `updateRoleLogic.go:34-41 / 64-70` 注释已与"数字越小 = 权限越高"钉死,代码语义一致 |
|
|
|
|
|
+| L-R12-4 / L-R12-5 | ✅ 保持接受契约 | 代码位置无回退;风险评估与 R12 一致 |
|
|
|
|
|
+
|
|
|
|
|
+结论:R12 输出的 P1/P2/P3 全部闭环,未观察到回退。
|
|
|
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
-## ⚠️ 健壮性与性能建议 (Medium/Low)
|
|
|
|
|
|
|
+## 🚩 核心逻辑漏洞 (High Risk)
|
|
|
|
|
|
|
|
-### M-R12-1(Medium · 并发/数据完整性) · `BindRoles` 与 `DeleteRole` 之间无锁链,会留下孤儿 `sys_user_role`
|
|
|
|
|
|
|
+本轮未发现新的 High Risk 漏洞。
|
|
|
|
|
|
|
|
-**描述**:`internal/logic/user/bindRolesLogic.go` 在 `TransactCtx` 里只锁 `sys_product_member` 行(`FindOneForUpdateTx(member.Id)`),对即将绑定的 `sys_role` 行**完全不持锁**——连事务外的 `FindByIds` 也不 `FOR SHARE`。
|
|
|
|
|
|
|
+R5~R11 期间暴露过的四条主链路(Auth / Token / Dept-Member / Perm-Sync)在代码中均看到锁序、乐观锁、CAS、fail-close 的层叠护栏,且 R12 的孤儿绑定修复把最后一条"锁序缺口"也补齐。抽样覆盖的高影响面:
|
|
|
|
|
|
|
|
-同时 `internal/logic/role/deleteRoleLogic.go:41-56` 的 `DeleteRole` 在事务内依次:
|
|
|
|
|
|
|
+- **密码变更链**(`changePasswordLogic` → `UpdatePassword` 带 `expectedUpdateTime`):H-R11-1 TOCTOU 闭环,未见回退。
|
|
|
|
|
+- **Token 轮转**(`authHelper.RotateRefreshToken` + `IncrementTokenVersionIfMatch` CAS):HTTP / gRPC 两条路径收敛到同一 helper,未观测到新的并发 rotate 路径。
|
|
|
|
|
+- **部门生命周期**(`DeleteDept` 锁序:self.X → children.S → users.S)vs(`CreateDept`、`UpdateUser(deptId)`):锁方向无 AB-BA。
|
|
|
|
|
+- **权限同步 × 设置用户权限**:`LockByCodeTx` × `SetUserPerms` 事务内 COUNT 双重把关,TOCTOU 闭环。
|
|
|
|
|
+- **授角色 × 删角色**:R12 的 `LockRolesForShareTx` 已闭合。
|
|
|
|
|
|
|
|
-```
|
|
|
|
|
-FindUserIdsByRoleIdForUpdateTx(roleId) -- 对 sys_user_role WHERE roleId=R FOR UPDATE
|
|
|
|
|
-DeleteByRoleIdTx(sys_role_perm)
|
|
|
|
|
-DeleteByRoleIdTx(sys_user_role) -- DELETE WHERE roleId=R
|
|
|
|
|
-DeleteWithTx(sys_role, id) -- 只在这一步对 sys_role[R] 拿 X 锁
|
|
|
|
|
-```
|
|
|
|
|
|
|
+---
|
|
|
|
|
|
|
|
-即 `DeleteRole` 对 `sys_role[R]` 的 X 锁要到**事务最末**才拿。两个事务交错:
|
|
|
|
|
|
|
+## ⚠️ 健壮性与性能建议 (Medium/Low)
|
|
|
|
|
+
|
|
|
|
|
+### L-R13-1(Low · 信息泄露 · 枚举信号)· `AddMember` 将权限校验放在读产品 / 读用户之后
|
|
|
|
|
+
|
|
|
|
|
+**描述**:`internal/logic/member/addMemberLogic.go:33-56` 的校验顺序是:
|
|
|
|
|
|
|
|
```
|
|
```
|
|
|
-T0: BindRoles 读 FindByIds([R]) → role R 启用、ProductCode 匹配(事务外、无锁)
|
|
|
|
|
-T1: BindRoles 开启事务;锁 member 行;Diff 出 toAdd=[R]
|
|
|
|
|
-T2: DeleteRole 开启事务;FOR UPDATE sys_user_role WHERE roleId=R(索引 idx_role 上的 gap lock)
|
|
|
|
|
-T3: DeleteRole 删除 sys_role_perm / sys_user_role;DELETE FROM sys_role WHERE id=R
|
|
|
|
|
-T4: DeleteRole 提交(sys_role[R] 已不存在)
|
|
|
|
|
-T5: BindRoles BatchInsertWithTx(userId, R) —— 无 FK、无 UNIQUE(sys_role.id) 校验——插入成功
|
|
|
|
|
-T6: BindRoles 提交
|
|
|
|
|
|
|
+① FindOneByCode(productCode) → 400/404 "产品不存在/已禁用"
|
|
|
|
|
+② FindOne(userId) → 400/404 "用户不存在/已冻结"
|
|
|
|
|
+③ memberType 字面校验 → 400
|
|
|
|
|
+④ RequireProductAdminFor → 403 "无权限"
|
|
|
```
|
|
```
|
|
|
|
|
|
|
|
-最终:`sys_user_role` 中存在 `roleId=R` 的行但 `sys_role[R]` 已被删除。由于 schema 未声明外键(`perm.sql` 里 `FOREIGN_KEY_CHECKS=1` 仅作用于 session,未定义任何 FK),该孤儿不会在写入时报错。
|
|
|
|
|
|
|
+任何登录用户(只要持有有效 JWT,不需要是产品 ADMIN)都可以通过响应码差异做两件事:
|
|
|
|
|
+
|
|
|
|
|
+- 枚举**存在且启用**的产品(对比 "产品不存在" / "产品已禁用" / "继续推进到 user 校验")。
|
|
|
|
|
+- 在已知 `productCode` 前提下,枚举**存在且启用**的 `userId`(对比"用户不存在 / 已冻结 / 通过用户校验" 三态)。
|
|
|
|
|
|
|
|
-另一种交错(DeleteRole 先于 BindRoles 取到 sys_user_role 的 gap lock)可能导致 BindRoles 插入时阻塞,而后 `sys_role[R]` 已不在,BindRoles 仍然能成功写入 `sys_user_role(userId, R)`——同样产生孤儿。
|
|
|
|
|
|
|
+步骤 ④ 把"本用户无管理员权限"的 403 放在最后,意味着非 product-ADMIN 的合法登录用户也能消费到 ①②③ 的信号——这比 R10-10 已经封死的 gRPC `GetUserPerms` 枚举面更宽。
|
|
|
|
|
|
|
|
**影响**:
|
|
**影响**:
|
|
|
|
|
|
|
|
-- **功能正确性**:`UserDetailsLoader.loadRoles` 使用 `INNER JOIN sys_role` 过滤掉 `status` 非 Enabled 或已删除的角色,孤儿行**不会**被加载成用户权限——用户行为侧无可见异常。
|
|
|
|
|
-- **数据不变式**:`sys_user_role.roleId` 指向不存在的 `sys_role.id` 违反隐式外键约束。单纯累积孤儿行成本低,但 `UserList` / `RoleList` / 运维排障时会看到"用户绑了一个查不到的角色 id" 的幽灵状态,需要另写清理脚本。
|
|
|
|
|
-- **触发概率**:极低。真实业务里 `DeleteRole` 是罕见操作,必须同产品同角色同时 BindRoles;实际运维每月一次即是顶配。但一旦触发修复成本(人工清洗)远高于事前防御。
|
|
|
|
|
|
|
+- **数据敏感度**:泄露的是"产品 code 集合是否在线" 和 "userId → 是否存在/已冻结"两条事实。前者在中大型组织里属于内部但不敏感,后者在"通过工号推 userId" 的场景下可以被攻击者用来筛选可用账号(配合 H-2 PII 暴露后的 phone / email 泄露链放大)。
|
|
|
|
|
+- **可触发门槛**:任何持有有效 access token 的用户(含仅 MEMBER)。
|
|
|
|
|
+- **概率**:在 JWT 泄露 / 内鬼场景下即时可用,概率非零。
|
|
|
|
|
|
|
|
-**修复方案**:在 `BindRoles` 的事务内对所有目标 `roleIds` 加 `SELECT ... FOR SHARE`(按 `id IN (...)` 排序取锁以避免死锁):
|
|
|
|
|
|
|
+**修复方案**:把 `RequireProductAdminFor(productCode)` 提升到读任何实体之前。由于 `productCode` 来自 `middleware.GetProductCode(ctx)`(权威,不是入参),这一提升零成本:
|
|
|
|
|
|
|
|
```go
|
|
```go
|
|
|
-// internal/logic/user/bindRolesLogic.go
|
|
|
|
|
-if err := l.svcCtx.SysUserRoleModel.TransactCtx(l.ctx, func(ctx, session) error {
|
|
|
|
|
- if _, err := l.svcCtx.SysProductMemberModel.FindOneForUpdateTx(ctx, session, member.Id); err != nil {
|
|
|
|
|
- return err
|
|
|
|
|
|
|
+func (l *AddMemberLogic) AddMember(req *types.AddMemberReq) (*types.IdResp, error) {
|
|
|
|
|
+ // 先把"无权调此接口"的路径一刀切在任何 DB 查询之前
|
|
|
|
|
+ if err := authHelper.RequireProductAdminFor(l.ctx, req.ProductCode); err != nil {
|
|
|
|
|
+ return nil, err
|
|
|
}
|
|
}
|
|
|
- if len(roleIds) > 0 {
|
|
|
|
|
- // 新增:对 toAdd/已有 roleIds 一并加 S 锁,形成与 DeleteRole.DeleteWithTx(sys_role) 的 X 锁链
|
|
|
|
|
- if err := l.svcCtx.SysRoleModel.LockRolesForShareTx(ctx, session, roleIds); err != nil {
|
|
|
|
|
- if errors.Is(err, sqlx.ErrNotFound) {
|
|
|
|
|
- return response.ErrBadRequest("包含已被删除的角色ID")
|
|
|
|
|
- }
|
|
|
|
|
- return err
|
|
|
|
|
- }
|
|
|
|
|
- }
|
|
|
|
|
- // ... 原有 existing / diff / delete / insert 逻辑 ...
|
|
|
|
|
-})
|
|
|
|
|
|
|
+ product, err := l.svcCtx.SysProductModel.FindOneByCode(l.ctx, req.ProductCode)
|
|
|
|
|
+ // ... 其余保持不变
|
|
|
|
|
+}
|
|
|
```
|
|
```
|
|
|
|
|
|
|
|
-`SysRoleModel` 需新增:
|
|
|
|
|
|
|
+同时建议同路径检查两个兄弟接口:
|
|
|
|
|
|
|
|
-```go
|
|
|
|
|
-// LockRolesForShareTx 对一批角色行取 S 锁;DeleteRole 的 DeleteWithTx 会拿 sys_role[R] 的 X 锁
|
|
|
|
|
-// 来阻塞本 S 锁,从而让任一被并发删除的角色在 BindRoles 提交前被感知。
|
|
|
|
|
-LockRolesForShareTx(ctx context.Context, session sqlx.Session, ids []int64) error
|
|
|
|
|
-```
|
|
|
|
|
-
|
|
|
|
|
-对应 SQL `SELECT 1 FROM sys_role WHERE id IN (?,?...) ORDER BY id LOCK IN SHARE MODE`。若命中行数 ≠ `len(ids)`,返回 `sqlx.ErrNotFound` 触发 `ErrBadRequest("包含已被删除的角色ID")`。
|
|
|
|
|
|
|
+- `SetUserPerms`(`setUserPermsLogic.go:38-47`):先 `FindOne(userId)` 再 `RequireProductAdminFor`,同样可枚举 `userId` 存在性。
|
|
|
|
|
+- `BindRoles`(`bindRolesLogic.go:41-49`):先 `FindOne(userId)` 再 `CheckManageAccess`,语义上"管理者才能读 target",但同样走到了未授权调用方的 404 分支。
|
|
|
|
|
|
|
|
-等价做法:`DeleteRole` 把 `FOR UPDATE sys_role[id]` 提前到事务第一步,等价形成 "X lock on sys_role → 所有写入 sys_user_role(roleId=R) 的事务要么先完成要么被阻塞"。两者择一。
|
|
|
|
|
|
|
+**结论**:按 `RequireProductAdminFor → 读其它实体` 的顺序统一重排,Medium 以下的"侧信道枚举面"就能大面积收敛。成本极低。
|
|
|
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
-### L-R12-1(Low · 缓存一致性) · `UpdateProfileWithTx` 族把 sqlc 缓存失效做在事务提交**之前**,存在窗口
|
|
|
|
|
-
|
|
|
|
|
-**描述**:`internal/model/user/sysUserModel.go:147-171` 的 `UpdateProfileWithTx` 把 UPDATE 语句丢给调用方传入的 `session` 执行,外层仍然用 `m.ExecCtx` 包裹以复用"成功后 DelCache"语义:
|
|
|
|
|
-
|
|
|
|
|
-```147:171:internal/model/user/sysUserModel.go
|
|
|
|
|
-func (m *customSysUserModel) UpdateProfileWithTx(ctx context.Context, session sqlx.Session, id int64,
|
|
|
|
|
- username string, nickname, email, phone, remark string, deptId, newStatus int64,
|
|
|
|
|
- statusChanged bool, expectedUpdateTime int64) error {
|
|
|
|
|
- if session == nil {
|
|
|
|
|
- return errors.New("UpdateProfileWithTx requires a non-nil session")
|
|
|
|
|
- }
|
|
|
|
|
- sysUserIdKey := fmt.Sprintf("%s%v", cacheSysUserIdPrefix, id)
|
|
|
|
|
- sysUserUsernameKey := fmt.Sprintf("%s%v", cacheSysUserUsernamePrefix, username)
|
|
|
|
|
- now := time.Now().Unix()
|
|
|
|
|
-
|
|
|
|
|
- res, err := m.ExecCtx(ctx, func(ctx context.Context, _ sqlx.SqlConn) (sql.Result, error) {
|
|
|
|
|
- // 🔴 使用 session 执行(事务内),但 m.ExecCtx 仍在闭包返回后立即 DelCache
|
|
|
|
|
- return session.ExecCtx(ctx, query, ...)
|
|
|
|
|
- }, sysUserIdKey, sysUserUsernameKey)
|
|
|
|
|
-```
|
|
|
|
|
|
|
+### L-R13-2(Low · TOCTOU · 脏写长尾)· `SetUserPerms` 的 memberType 检查点未覆盖到事务内
|
|
|
|
|
|
|
|
-`go-zero` 的 `cachedSqlConn.ExecCtx` 行为:**先执行 exec 闭包,闭包返回成功后调用 `DelCacheCtx`**。这里的"成功"是指 `session.ExecCtx` 写入受影响行数 ≥ 1——而**事务此时尚未 commit**。
|
|
|
|
|
|
|
+**描述**:`internal/logic/user/setUserPermsLogic.go:91-97` 对目标用户的 `memberType == ADMIN / DEVELOPER` 做"禁止写 DENY" 的入口拦截,目的是避免写入"永不生效的 DENY 脏行"(L-R10-8)。该检查读的是事务外的 `targetMember.MemberType` 快照。
|
|
|
|
|
|
|
|
-并发窗口:
|
|
|
|
|
|
|
+时序风险:
|
|
|
|
|
|
|
|
```
|
|
```
|
|
|
-T0: UpdateUserLogic.TransactCtx 入口 → UpdateProfileWithTx 内
|
|
|
|
|
-T1: session.ExecCtx 执行 UPDATE —— 行被锁,undo log 生成,但 tx 尚未 commit
|
|
|
|
|
-T2: m.ExecCtx 包装器 DelCache(idKey, usernameKey) —— sysUser 低层缓存被清
|
|
|
|
|
-T3: UpdateProfileWithTx 返回
|
|
|
|
|
-T4: 另一只请求 R 打进来,走 UserDetailsLoader.Load → cache miss → loadUser → SysUserModel.FindOne(id)
|
|
|
|
|
- FindOne 走独立连接走默认 RC 隔离级别:看到的是 T1 未提交的旧行,写回 sysUser 低层缓存 = 旧值
|
|
|
|
|
- → loadFromDB 得到旧 UserDetails → 5 分钟正缓存
|
|
|
|
|
-T5: UpdateUserLogic 的 TransactCtx 真正 commit —— DB 已是新值
|
|
|
|
|
-T6: UpdateUserLogic 调用 UserDetailsLoader.Clean(userId)
|
|
|
|
|
- → DEL 所有产品下 UserDetails 缓存,解除窗口
|
|
|
|
|
|
|
+T0: caller A 读 targetMember.MemberType = "MEMBER" -- 入口检查通过
|
|
|
|
|
+T1: caller B(产品 ADMIN)调 UpdateMember,把 target 升为 "ADMIN"(并提交)
|
|
|
|
|
+T2: caller A 进入事务;DeleteByUserIdForProductTx + BatchInsertWithTx(DENY 行) 均成功
|
|
|
|
|
+T3: caller A 事务末 COUNT(sys_perm ...) 复核通过,事务提交
|
|
|
```
|
|
```
|
|
|
|
|
|
|
|
-`T4 - T6` 之间用户的 UserDetails 缓存里仍然是 T1 之前的旧值——包括 `DeptId / DeptPath / Status / TokenVersion`。由于 `UpdateUserLogic` 走 `UpdateProfileWithTx` 的唯一分支是"改 deptId 到新部门"(审计 M-R11-3 锁链的 happy path),窗口里被并发 Load 的用户会看到"旧部门 / 旧 path",继续被按原 dept 做管辖决策。
|
|
|
|
|
-
|
|
|
|
|
-窗口长度 ≈ session.ExecCtx 返回 → TransactCtx commit 之间的时长,正常路径下只有毫秒级。
|
|
|
|
|
|
|
+最终:target 现在是 ADMIN(loadPerms 走全权分支),`sys_user_perm` 里留下了永不生效的 DENY 行。这条路径恰好是 L-R10-8 主动防御的语义欺骗("能写、永不生效" 的脏状态污染审计日志 / 权限推理工具)。
|
|
|
|
|
|
|
|
**影响**:
|
|
**影响**:
|
|
|
|
|
|
|
|
-- **功能正确性**:`UpdateUserLogic:202` 在 `TransactCtx` 外调用 `UserDetailsLoader.Clean(req.Id)` 做 post-commit 失效,窗口会被 Clean 关闭。但**窗口内**被 populate 的 UserDetails 正缓存会留到下一次 Clean 时才失效,中间的 5 分钟 TTL 内仍按旧值。
|
|
|
|
|
-- **安全侧**:管辖链(checkDeptHierarchy)以缓存中的 DeptPath 为准;窗口内若 caller 恰好是"旧 deptPath 下的 MEMBER"且目标用户刚被调离,caller 仍可能在短时间内成功下发敏感操作(例如 `UpdateUser nickname`)。对强一致保护的 `checkPermLevel` / `loadFreshMinPermsLevel` 已走 NoCache DB 路径,因此越权决策不受影响;但列表展示、soft 的权限检查会短暂失真。
|
|
|
|
|
-- **概率**:窗口毫秒级,真实业务下几乎观测不到。但该模式被大量使用(任何 `m.ExecCtx` 包装 `session.ExecCtx` 的 `*WithTx` 方法都有同样行为),具备跨文件一致性问题的特征。
|
|
|
|
|
|
|
+- **运行期安全**:零。ADMIN/DEVELOPER 的 loadPerms 分支完全忽略 `sys_user_perm`,DENY 不会意外丢失敏感权限。
|
|
|
|
|
+- **数据卫生**:污染 `sys_user_perm`;未来如果 target 再被 `UpdateMember` 降回 MEMBER 或调 `RemoveMember → AddMember`(后者不清 user_perm),这些 DENY 会"诈尸"生效,运维排查会踩到"为什么这个用户突然少了一条权限"。
|
|
|
|
|
+- **概率**:极低。需要 caller A 与 caller B 在同一产品同一 target 上毫秒级交错。
|
|
|
|
|
|
|
|
-**修复方案**:
|
|
|
|
|
|
|
+**修复方案**:把 memberType 检查纳入事务、与 `sys_product_member` 行 `FOR SHARE` 闭环:
|
|
|
|
|
|
|
|
-- **方案 A(推荐)**:在 `UpdateProfileWithTx` 这类事务性方法中**不要**复用 `m.ExecCtx` 的 DelCache 语义。把缓存失效挪到**事务提交之后**由调用方(Logic 层)执行。具体做法:
|
|
|
|
|
- ```go
|
|
|
|
|
- // model 层只做 session.ExecCtx;失效由调用方显式 DelCache(idKey, usernameKey)
|
|
|
|
|
- func (m *customSysUserModel) UpdateProfileWithTx(...) error {
|
|
|
|
|
- if session == nil { return errors.New(...) }
|
|
|
|
|
- res, err := session.ExecCtx(ctx, query, ...)
|
|
|
|
|
- if err != nil { return err }
|
|
|
|
|
- if affected, _ := res.RowsAffected(); affected == 0 {
|
|
|
|
|
- return ErrUpdateConflict
|
|
|
|
|
- }
|
|
|
|
|
- return nil
|
|
|
|
|
- }
|
|
|
|
|
- // model 层额外暴露 "缓存失效" helper,上游 TransactCtx commit 成功后显式调用
|
|
|
|
|
- func (m *customSysUserModel) InvalidateUser(ctx context.Context, id int64, username string) {
|
|
|
|
|
- idKey := fmt.Sprintf("%s%v", cacheSysUserIdPrefix, id)
|
|
|
|
|
- userKey := fmt.Sprintf("%s%v", cacheSysUserUsernamePrefix, username)
|
|
|
|
|
- _ = m.DelCacheCtx(ctx, idKey, userKey)
|
|
|
|
|
- }
|
|
|
|
|
- ```
|
|
|
|
|
- `UpdateUserLogic` 在 `TransactCtx` 返回后先调 `InvalidateUser`、再 `UserDetailsLoader.Clean`。
|
|
|
|
|
-- **方案 B**:保留当前实现,但在文档里明确"`*WithTx` 的缓存失效是 best-effort 的 pre-commit 清理,强一致读一律走 `...ForUpdateTx / ForShareTx` 事务内查询"。当前代码已经隐式这么做(决策点都走 `loadFresh*`),可视为**已接受的契约**。
|
|
|
|
|
|
|
+```go
|
|
|
|
|
+if err := l.svcCtx.SysUserPermModel.TransactCtx(l.ctx, func(ctx, session) error {
|
|
|
|
|
+ // 事务内复读 member 状态 + 锁
|
|
|
|
|
+ member, err := l.svcCtx.SysProductMemberModel.FindOneForShareTx(ctx, session, targetMember.Id)
|
|
|
|
|
+ if err != nil {
|
|
|
|
|
+ return response.ErrConflict("成员状态已变更,请刷新后重试")
|
|
|
|
|
+ }
|
|
|
|
|
+ if (member.MemberType == ADMIN || member.MemberType == DEVELOPER) && hasDeny {
|
|
|
|
|
+ return response.ErrBadRequest("目标用户是管理员或开发者,DENY 不会生效")
|
|
|
|
|
+ }
|
|
|
|
|
+ // ... 原有 DeleteByUserIdForProductTx + BatchInsertWithTx + COUNT 复核
|
|
|
|
|
+})
|
|
|
|
|
+```
|
|
|
|
|
+
|
|
|
|
|
+需新增 `SysProductMemberModel.FindOneForShareTx`(与现有 `FindOneForUpdateTx` 对称)。`UpdateMember` 走的是 `FOR UPDATE` 路径,会与本 `FOR SHARE` 形成阻塞链。
|
|
|
|
|
|
|
|
-我们倾向方案 A——事务语义一致性比 pre-commit 清掉一次缓存的微小收益更重要。
|
|
|
|
|
|
|
+**优先级**:P3。当前代码的 L-R10-8 "入口拦截 + loadPerms 全权分支忽略脏行"已经把运行期风险压到零,本条只是数据卫生层面的补丁。
|
|
|
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
-### L-R12-2(Low · 逻辑一致性) · `CreateDept` 事务内不复核 `parent.Status`,可在"父部门刚被禁用"时插入新子部门
|
|
|
|
|
|
|
+### L-R13-3(Low · 僵尸代码 / 防御冗余)· `UpdateUserLogic` 中的 `caller.DeptPath != ""` 分支已不可达
|
|
|
|
|
|
|
|
-**描述**:`internal/logic/dept/createDeptLogic.go:46-72`
|
|
|
|
|
|
|
+**描述**:`internal/logic/user/updateUserLogic.go:123-128`
|
|
|
|
|
|
|
|
-```
|
|
|
|
|
-① 事务外 FindOne(parentId) —— 读到 parent.Status=Enabled,捕获 parentPath
|
|
|
|
|
-② 事务内 SELECT id FROM sys_dept WHERE id=? FOR SHARE —— 只校验"父部门存在"
|
|
|
|
|
-③ InsertWithTx(子部门) —— 继承 parentPath、Status=Enabled
|
|
|
|
|
|
|
+```go
|
|
|
|
|
+if !caller.IsSuperAdmin &&
|
|
|
|
|
+ caller.MemberType != consts.MemberTypeAdmin &&
|
|
|
|
|
+ caller.DeptPath != "" && // ← 这条在当前调用链下永远为 true
|
|
|
|
|
+ !strings.HasPrefix(newDept.Path, caller.DeptPath) {
|
|
|
|
|
+ return response.ErrForbidden("无权将用户调入非自己管辖的部门")
|
|
|
|
|
+}
|
|
|
```
|
|
```
|
|
|
|
|
|
|
|
-步骤②只看 `id` 是否仍存在,不读 `Status`。如果两个事务:
|
|
|
|
|
|
|
+走到这段代码有三个前置事实:
|
|
|
|
|
|
|
|
-```
|
|
|
|
|
-T0: UpdateDept 开启事务,持有 sys_dept[P] 的 X 锁,把 Status 改成 Disabled
|
|
|
|
|
-T1: CreateDept 的步骤① 已经在 T0 之前发生,读到 P.Status=Enabled(提交可见版本)
|
|
|
|
|
-T2: T0 commit —— sys_dept[P].Status=Disabled
|
|
|
|
|
-T3: CreateDept 开启事务(T2 之后),步骤② 的 FOR SHARE 看到 P 存在,放行
|
|
|
|
|
-T4: 子部门插入成功:子部门 Status=Enabled,父部门 Status=Disabled
|
|
|
|
|
-```
|
|
|
|
|
|
|
+1. 上方 `caller.UserId == req.Id → 拒绝改 deptId`(line 42-45)已经把"caller == target" 分支 cut 掉;
|
|
|
|
|
+2. `CheckManageAccess`(line 57)对 `caller != target` 分支调 `checkDeptHierarchy`,后者在 `caller.DeptId == 0 || caller.DeptPath == ""` 时 fail-close(`access.go:318-324`);
|
|
|
|
|
+3. 走到第 123 行时 `caller` 必然既不是 super、也不是 ADMIN(同一条判定前几句过滤掉)。
|
|
|
|
|
|
|
|
-> 注:`UpdateDept` 走 `UpdateWithOptLock`,只要它拿到行锁后 CAS 成功即 commit;不与 `CreateDept` 的 FOR SHARE 同处一个锁链外的范围。
|
|
|
|
|
|
|
+综合三条事实:`caller.DeptPath != ""` 恒成立——这是防御冗余,不是主动保护。
|
|
|
|
|
|
|
|
**影响**:
|
|
**影响**:
|
|
|
|
|
|
|
|
-- **loadPerms 语义**:`UserDetailsLoader.loadPerms` 只用 `ud.DeptType / ud.DeptStatus`——即当前用户所在部门的 type / status——不递归向上看父部门,子部门 Enabled 完全可以独立工作,业务语义不破。
|
|
|
|
|
-- **运营观感**:管理员"禁用整个部门子树"的意图被绕过,禁用父部门后若 CreateDept 正在进行,会看到一个 Enabled 子部门挂在 Disabled 父部门下,排障困难。
|
|
|
|
|
-- **概率**:极低(人工操作时序 + 同部门并发)。
|
|
|
|
|
|
|
+- **运行期**:零。
|
|
|
|
|
+- **可读性**:中。新维护者看到这行容易以为"某种分支下 caller.DeptPath 可以为空",进而在 `checkDeptHierarchy` 里放宽约束,把真实的 fail-close 护栏拆掉。R10 ~ R12 对类似误导性注释已经多次浪费工时(L-R12-3)。
|
|
|
|
|
|
|
|
-**修复方案**:在事务内把 `SELECT id` 升级成 `SELECT id, status, path FOR SHARE`,并同时复核 Status 与 Path:
|
|
|
|
|
|
|
+**修复方案**:删除该条件,并在上方添加一行注释说明这段依赖 `CheckManageAccess` 已经 fail-close:
|
|
|
|
|
|
|
|
```go
|
|
```go
|
|
|
-var parent deptModel.SysDept
|
|
|
|
|
-lockQ := fmt.Sprintf("SELECT `id`, `status`, `path` FROM %s WHERE `id`=? FOR SHARE", l.svcCtx.SysDeptModel.TableName())
|
|
|
|
|
-if err := session.QueryRowCtx(ctx, &parent, lockQ, req.ParentId); err != nil {
|
|
|
|
|
- return response.ErrNotFound("父部门已被删除")
|
|
|
|
|
-}
|
|
|
|
|
-if parent.Status != consts.StatusEnabled {
|
|
|
|
|
- return response.ErrBadRequest("父部门已被禁用,无法创建子部门")
|
|
|
|
|
|
|
+// caller.DeptId == 0 / DeptPath == "" 的 fail-close 已经由 CheckManageAccess
|
|
|
|
|
+// → checkDeptHierarchy 在 access.go:318-324 统一兜底,这里不再重复防御。
|
|
|
|
|
+if !caller.IsSuperAdmin &&
|
|
|
|
|
+ caller.MemberType != consts.MemberTypeAdmin &&
|
|
|
|
|
+ !strings.HasPrefix(newDept.Path, caller.DeptPath) {
|
|
|
|
|
+ return response.ErrForbidden("无权将用户调入非自己管辖的部门")
|
|
|
}
|
|
}
|
|
|
-parentPath = parent.Path // 改为使用事务内视图里的 path,理论上与事务外一致(UpdateDept 不改 path)
|
|
|
|
|
```
|
|
```
|
|
|
|
|
|
|
|
-顺手覆盖一个边界:即使未来 `UpdateDept` 扩展为支持"重命名 path",本修复也已经在事务内 snapshot 了最新 path。
|
|
|
|
|
|
|
+**优先级**:P3。纯可读性/防回退修复。
|
|
|
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
-### L-R12-3(Low · 僵尸代码 / 文档一致性) · `updateRoleLogic.go` 的注释"非超管不得降低 PermsLevel"与实际代码相反
|
|
|
|
|
|
|
+### L-R13-4(Low · 边界 · 数据卫生)· `CreateUser` 未拒绝负数 `deptId`
|
|
|
|
|
|
|
|
-**描述**:`internal/logic/role/updateRoleLogic.go` 中非超管的权限级别校验使用 `req.PermsLevel < role.PermsLevel`。在本项目的约定下"数字越小权限越高"(见 `UserDetailsLoader.loadRoles: MinPermsLevel` 计算),因此 `req.PermsLevel < role.PermsLevel` 实际拦截的是**提升权限**,而代码注释却写成"防止降低 PermsLevel"。
|
|
|
|
|
|
|
+**描述**:`internal/logic/user/createUserLogic.go:80-98`
|
|
|
|
|
|
|
|
-本条之前在 R11 审计里已经被口头标记,但未修。核心 bug 不存在——非超管的 `UpdateRole` 只有 product ADMIN 能进入,product ADMIN 在产品内有全权,提升 / 降低都不是越权;真正的边界是 `BindRoles` 的 `GuardRoleLevelAssignable`。但注释与代码反向的现状会让新维护者以为策略写错了,走入方向相反的"修复"最后引入真 bug。
|
|
|
|
|
|
|
+```go
|
|
|
|
|
+if req.DeptId > 0 {
|
|
|
|
|
+ // ... 校验部门存在、启用、hierarchy
|
|
|
|
|
+} else if !caller.IsSuperAdmin {
|
|
|
|
|
+ return nil, response.ErrBadRequest("必须指定部门")
|
|
|
|
|
+}
|
|
|
|
|
+// 落到 Insert 时 deptId 是 req.DeptId 原值;超管可直接 req.DeptId = -1
|
|
|
|
|
+```
|
|
|
|
|
|
|
|
-**影响**:
|
|
|
|
|
|
|
+超级管理员以 `req.DeptId = -1`(或其他负数)调用时会直接 `Insert` 一条 `sys_user.deptId = -1` 的脏数据。后续:
|
|
|
|
|
|
|
|
-- **运行期**:零。
|
|
|
|
|
-- **维护期**:高——误导性注释是 R10 ~ R12 三轮审计里被反复关注的工作量。
|
|
|
|
|
|
|
+- `UserDetailsLoader.loadDept` 有 `if ud.DeptId == 0 { return nil }` 短路,但 `DeptId == -1` 会去 `FindOne(-1)` 命中 `ErrNotFound` → 报错 → `loadOk = false` → 返回 503 Degraded。
|
|
|
|
|
+- `FindIdsByDeptId(-1)` 永远返 nil;这条用户在部门相关接口里彻底隐形。
|
|
|
|
|
|
|
|
-**修复方案**:只改注释,把"防止降低"改成"防止非超管把角色提升到比当前更高的权限(数字更小 = 更高权)",并在同一行把"数字越小 = 权限越高"这条约定显式写出来。
|
|
|
|
|
|
|
+即"超管的输入合法域"未被钉死,能构造出永远无法被部门树管理到的僵尸账号。
|
|
|
|
|
|
|
|
----
|
|
|
|
|
|
|
+**影响**:
|
|
|
|
|
|
|
|
-### L-R12-4(Low · 契约 · 已接受) · `GetUserPerms` 对"合法成员但被冻结"仍返回 `PermissionDenied`,可用于"合法成员 × 冻结态"枚举
|
|
|
|
|
|
|
+- **运行期**:非阻断。被影响的仅是该条记录的用户本人在登录时会踩 503(因 `loadDept` 报错被 degrade)。
|
|
|
|
|
+- **数据完整性**:污染 `sys_user` 的 `deptId` 定义域。
|
|
|
|
|
|
|
|
-**描述**:`internal/server/permserver.go:348-354`
|
|
|
|
|
|
|
+**修复方案**:在入口简单加一条校验:
|
|
|
|
|
|
|
|
```go
|
|
```go
|
|
|
-if ud.Username == "" || (!ud.IsSuperAdmin && ud.MemberType == "") {
|
|
|
|
|
- return nil, status.Error(codes.NotFound, "用户不是该产品的有效成员")
|
|
|
|
|
|
|
+if req.DeptId < 0 {
|
|
|
|
|
+ return nil, response.ErrBadRequest("部门ID必须为非负整数")
|
|
|
}
|
|
}
|
|
|
-if ud.Status != consts.StatusEnabled {
|
|
|
|
|
- return nil, status.Error(codes.PermissionDenied, "用户已被冻结")
|
|
|
|
|
|
|
+if req.DeptId > 0 {
|
|
|
|
|
+ // ... 原有逻辑
|
|
|
|
|
+} else if !caller.IsSuperAdmin {
|
|
|
|
|
+ return nil, response.ErrBadRequest("必须指定部门")
|
|
|
}
|
|
}
|
|
|
```
|
|
```
|
|
|
|
|
|
|
|
-- `NotFound` 同时覆盖"用户全局不存在"和"用户存在但非本产品成员"——这是 L-R10-10 封死的枚举窗口。
|
|
|
|
|
-- `PermissionDenied` 仍显式披露"用户是本产品成员且当前冻结"。
|
|
|
|
|
-
|
|
|
|
|
-持有合法 `appKey + appSecret` 的产品在 `userId` 区间内扫描即可建立"该产品下哪些成员被冻结"的映射。在 `appSecret` 泄露后这是一条信息泄露面。
|
|
|
|
|
-
|
|
|
|
|
-**影响**:
|
|
|
|
|
|
|
+同类检查建议同步到 `UpdateUser`(line 111-138 的 `*req.DeptId > 0` 分支外,没有对 `*req.DeptId < 0` 的显式拒绝——但那条路径会落到"deptId=0 且不是 super/admin 则 403",与 0 等价;超管仍可写入负数)。
|
|
|
|
|
|
|
|
-- 只有合法产品服务端可触发(凭 `bcrypt` 通过 appSecret 校验),攻击前提较高。
|
|
|
|
|
-- 当前版本的安全侧评估(注释中明示):"密码正确才能拿到合法 appKey 这一前提不成立时,这个状态已经是上层业务承诺披露的信息,不构成新增枚举面。"
|
|
|
|
|
-
|
|
|
|
|
-**结论**:保持现状作为已接受契约,不纳入 P0/P1。若未来"冻结状态"被列为 PII 敏感属性(目前不是),改为统一回 `NotFound "用户不是该产品的有效成员"` 即可。
|
|
|
|
|
|
|
+**优先级**:P3。
|
|
|
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
-### L-R12-5(Low · 安全信号最小化) · `AdminLogin` 对"冻结超管"与"成功超管"的时序差可观测
|
|
|
|
|
|
|
+### L-R13-5(Low · 资源管理 · 可观测性)· post-commit 缓存失效在 ctx 被取消时静默降级
|
|
|
|
|
|
|
|
-**描述**:`internal/logic/pub/adminLoginLogic.go:76-86`
|
|
|
|
|
|
|
+**描述**:整套代码库的一种常见模式:
|
|
|
|
|
|
|
|
|
|
+```go
|
|
|
|
|
+if err := transactionBody(...); err != nil { return err }
|
|
|
|
|
+l.svcCtx.UserDetailsLoader.Clean(l.ctx, userId) // 或 BatchDel / CleanByUserIds
|
|
|
|
|
+l.svcCtx.SysUserModel.InvalidateProfileCache(...) // sqlc 低层缓存
|
|
|
```
|
|
```
|
|
|
-① bcrypt.CompareHashAndPassword -- real
|
|
|
|
|
-② if u.Status != Enabled → 401
|
|
|
|
|
-③ UserDetailsLoader.Load(u.Id, "")
|
|
|
|
|
-④ GenerateAccessToken + GenerateRefreshToken
|
|
|
|
|
-```
|
|
|
|
|
|
|
|
|
|
-对冻结超管:①+② 耗时 ≈ 100ms(bcrypt 为主),不走 ③④。
|
|
|
|
|
-对正常超管:①+②+③+④ 耗时 ≈ 100ms + 若干 DB 查询 + 两次 HMAC 签名 ≈ 150 ms+。
|
|
|
|
|
|
|
+当 `l.ctx`(继承自 HTTP 请求 ctx)在 DB 事务提交之后、Clean 执行之前被 client 断连 / server 超时取消时:
|
|
|
|
|
+
|
|
|
|
|
+- `Clean` 内部的 Redis `Smembers + Del + Del` 三步 / `BatchDel` 的 `Del + Pipelined SRem`——只要第一步遇到 `ctx canceled` 就会短路到 logx `Errorf` 并返回,后续步骤不执行。
|
|
|
|
|
+- `InvalidateProfileCache` 对失败是完全静默(`_ = m.DelCacheCtx(ctx, ...)`)。
|
|
|
|
|
|
|
|
-两种路径时序差约 10~50 ms,可被外部远程计时攻击用来区分"存在的超管被冻结" vs "存在的超管密码错误(相同 100ms)" vs "存在的超管成功(更长)"。
|
|
|
|
|
|
|
+最终:**DB 已改、缓存未清**。用户在 5 分钟 TTL 窗口内会继续看到旧的 `UserDetails`(旧 DeptPath / 旧 MinPermsLevel / 旧冻结状态),直到下一次该用户被 Load 命中 TTL 失效或被别的 Clean 触发。
|
|
|
|
|
|
|
|
-但该时序差**必须**已经持有正确超管密码才能被触发——攻击者需要先爆破出密码,再用 1000+ 请求做计时统计推断冻结。实际威胁模型下,拿到密码的人不 care "是否被冻结"。
|
|
|
|
|
|
|
+与 `UserDetailsLoader` 自身文档里承诺的"Clean 的失败是 best-effort,TTL 兜底" 是一致的——但这些失败目前**没有任何可观测的 metric**,只进 Errorf 日志。
|
|
|
|
|
|
|
|
**影响**:
|
|
**影响**:
|
|
|
|
|
|
|
|
-- 实际风险极低;历史审计已归档为"可接受时序差"。
|
|
|
|
|
-- 本条作为 R12 复核条目**重申**——不列 P0/P1。
|
|
|
|
|
|
|
+- **安全敏感变更**(冻结用户、切换部门、降权):TTL 5 分钟内旧 UD 继续生效。
|
|
|
|
|
+- **用户体验**:改完资料立即刷新看到旧头像 / 昵称,属于次要。
|
|
|
|
|
+- **监控侧**:Errorf 在正常业务里也会零星出现(Redis 抖动),没有区分"ctx 取消" vs "Redis 真的挂了"的 tag,运维难以建告警。
|
|
|
|
|
+
|
|
|
|
|
+**修复方案**:
|
|
|
|
|
+
|
|
|
|
|
+- **方案 A(推荐)**:post-commit 阶段用 `context.WithoutCancel(l.ctx)`(Go 1.21+)或手动 detach 出一个独立的 `context.Background()` + timeout(比如 3s)把缓存失效做完。事务已经提交,这几次 Redis 写是后置补偿,不该随请求取消而丢失。
|
|
|
|
|
+
|
|
|
|
|
+ ```go
|
|
|
|
|
+ if err := transactionBody(...); err != nil { return err }
|
|
|
|
|
+
|
|
|
|
|
+ // post-commit 的缓存失效不应随 HTTP 请求一起被取消
|
|
|
|
|
+ cleanCtx, cancel := context.WithTimeout(context.WithoutCancel(l.ctx), 3*time.Second)
|
|
|
|
|
+ defer cancel()
|
|
|
|
|
+ l.svcCtx.UserDetailsLoader.Clean(cleanCtx, userId)
|
|
|
|
|
+ l.svcCtx.SysUserModel.InvalidateProfileCache(cleanCtx, userId, username)
|
|
|
|
|
+ return nil
|
|
|
|
|
+ ```
|
|
|
|
|
+
|
|
|
|
|
+- **方案 B**:保持现状,但在 `UserDetailsLoader.Clean` / `BatchDel` / `CleanByUserIds` 内部识别 `errors.Is(err, context.Canceled)` 并打一条带 `cache_invalidation_skipped_due_to_ctx_cancel` tag 的 WARN 日志,方便运维基于这个 tag 建看板报警;真正的"Redis 挂了"走原来的 Errorf。
|
|
|
|
|
+
|
|
|
|
|
+两种方案可以叠加。方案 A 治本(让敏感变更的缓存失效脱离请求生命周期),方案 B 治可观测性。
|
|
|
|
|
|
|
|
-**结论**:保持现状。如果合规侧(SOC2 / CC)要求所有"已存在账户" vs "不存在账户"分支必须同质化,可把 `③④` 抽到冻结分支里空跑一次(discard 结果),进一步把时序差压到 <5 ms。当前优先级低。
|
|
|
|
|
|
|
+**优先级**:P2(方案 B,日志分级)+ P3(方案 A,detach ctx)。方案 A 的行为改动面较大,建议先做方案 B 收集样本。
|
|
|
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
-## 本轮复核中仍成立的契约(不再修)
|
|
|
|
|
|
|
+## 复核中仍成立的契约(本轮不动)
|
|
|
|
|
|
|
|
-- **H-1 / R10 复核**:`UserDetail` / `MemberList` 同产品成员可见彼此 `email / phone / remark` —— 产品业务需求已确认保留。
|
|
|
|
|
-- **M-4 / R10 复核**:`CreateProduct` 响应体只返回一次性 ticket,真实 `appSecret / adminPassword` 通过 `/fetchInitialCredentials`(超管鉴权 + `GetDelCtx` 原子消费)领取。
|
|
|
|
|
-- **M-3 / H-2 / R10 复核**:授角色、管辖决策点 100% 走 NoCache DB 读(`loadFreshMinPermsLevel`),caller 的 `MinPermsLevel` 缓存不参与决策;TTL 不影响越权闭环。
|
|
|
|
|
-- **L-R10-4 / R11 复核**:RefreshToken 的 `newVersion != predictedVersion` 分支保留 forensic 兜底;R11 通过 `authHelper.RotateRefreshToken` 将 HTTP / gRPC 两条路径收敛到同一 helper,已消除代码重复。
|
|
|
|
|
-- **L-R10-8**:`loadPerms` 对 SUPER / ADMIN / DEVELOPER 忽略 DENY 的语义已在 `SetUserPerms` 入口拦截;`DeptType` 动态变动导致旧 DENY 失效的长尾遗留。
|
|
|
|
|
-- **L-R10-9**:代理层 X-Forwarded-For 链一致性由运维侧在反代/WAF 上硬约束。
|
|
|
|
|
-- **L-R12-4 / L-R12-5**:已在上文明示为接受契约,不纳入本轮修复窗口。
|
|
|
|
|
|
|
+以下条目是历史审计(R5~R12)确认的**业务侧已接受**选择,本轮没有新观测让它们的风险/收益比改变:
|
|
|
|
|
+
|
|
|
|
|
+- **H-1(同产品成员可见彼此 PII)**:业务需求明示保留,不视为漏洞。
|
|
|
|
|
+- **M-4(CreateProduct 的 one-time ticket 机制)**:`internal/logic/product/fetchInitialCredentialsLogic.go` 的 `GetDelCtx` 原子消费 + 超管鉴权仍在位。
|
|
|
|
|
+- **H-2 / M-3(decision-time fresh DB 读)**:`loadFreshMinPermsLevel` 继续覆盖 `GuardRoleLevelAssignable` 与 `checkPermLevel` 两条决策链,TOCTOU 闭环。
|
|
|
|
|
+- **L-R10-8(全权成员的 DENY 入口拦截 + loadPerms 全权分支忽略脏行)**:入口拦截 + loadPerms JOIN status 双层兜底;L-R13-2 仅在数据卫生层进一步收敛。
|
|
|
|
|
+- **L-R10-9(代理 X-Forwarded-For 链一致性)**:依赖部署侧反代 / WAF 硬约束,代码侧 `firstValidIP` + `net.ParseIP` 已尽全力。
|
|
|
|
|
+- **L-R12-4 / L-R12-5**:已接受契约。
|
|
|
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
-## 修复优先级
|
|
|
|
|
|
|
+## 本轮 Findings 汇总与修复优先级
|
|
|
|
|
|
|
|
-| 优先级 | 条目 | 理由 |
|
|
|
|
|
-| ---- | ---- | ---- |
|
|
|
|
|
-| P1 | **M-R12-1** | `BindRoles`+`DeleteRole` 孤儿行;概率低但人工清洗成本高,排入下个迭代 |
|
|
|
|
|
-| P2 | L-R12-1 | `*WithTx` 缓存失效时机;窗口毫秒级但属于"跨文件一致性模式",顺手梳理一批 |
|
|
|
|
|
-| P2 | L-R12-2 | `CreateDept` 父部门 Status 复核;用 `SELECT ... FOR SHARE` 顺带多取两列 |
|
|
|
|
|
-| P3 | L-R12-3 | 注释与代码反向;纯文档修复 |
|
|
|
|
|
-| — | L-R12-4 / L-R12-5 | 已归档为可接受契约 |
|
|
|
|
|
|
|
+| 优先级 | 条目 | 类型 | 一句话总结 |
|
|
|
|
|
+| --- | --- | --- | --- |
|
|
|
|
|
+| P2 | **L-R13-1** | 信息泄露 | `AddMember` / `SetUserPerms` / `BindRoles` 未把 `RequireProductAdminFor` 提到读实体之前,已认证用户可枚举产品 / 用户存在性 |
|
|
|
|
|
+| P2 | L-R13-5 方案 B | 可观测性 | post-commit 缓存失效的 ctx-canceled 失败需要独立 tag,便于告警 |
|
|
|
|
|
+| P3 | L-R13-2 | 数据卫生 | `SetUserPerms` 的 memberType 检查点应纳入事务 + `FOR SHARE` 闭环 |
|
|
|
|
|
+| P3 | L-R13-3 | 僵尸代码 | `UpdateUserLogic` 中冗余的 `caller.DeptPath != ""` 分支删掉 |
|
|
|
|
|
+| P3 | L-R13-4 | 边界 | `CreateUser` 拒绝 `req.DeptId < 0` |
|
|
|
|
|
+| P3 | L-R13-5 方案 A | 资源管理 | post-commit 阶段用 detached ctx 做缓存失效 |
|
|
|
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
## 复核结论
|
|
## 复核结论
|
|
|
|
|
|
|
|
-经过 11 轮迭代 + 本轮(12)深度复核,`perms-system/server` 的核心授权 / 会话 / 数据持久化三条链路已达到一线生产系统的水准:
|
|
|
|
|
|
|
+经过 12 轮迭代 + 本轮(R13)独立复核,`perms-system/server` 的核心授权、会话、数据持久化链路进入**尾部收敛**阶段:
|
|
|
|
|
+
|
|
|
|
|
+- **High Risk 本轮为 0**:连续第二轮无新 High;主链路护栏稳定,未观察到历史修复回退。
|
|
|
|
|
+- **Medium 本轮为 0(本体)**:L-R13-1(枚举面)与 L-R13-5(可观测性)可归为"次 Medium"处理,但其影响面都被既有限流 / TTL 兜住,没有形成直接越权路径。
|
|
|
|
|
+- **Low 本轮 5 条**:全部是**数据卫生**、**僵尸代码**、**防御冗余**三类维护性建议,没有会改变系统安全模型的条目。
|
|
|
|
|
+
|
|
|
|
|
+整体观感:R12 的锁链修复(M-R12-1)+ L-R12-1 的缓存失效时机整改后,这一轮只能在"已认证用户的枚举信号"和"超管输入合法域"这类更小的面上找到可改进点——说明核心逻辑已高度收敛。建议把 L-R13-1(性价比最高,纯 reorder 改动)和 L-R13-5 方案 B(可观测性,零行为变动)合并进最近一次 release;其它 P3 条目随代码重构顺手做即可。
|
|
|
|
|
+
|
|
|
|
|
+若后续要把审计频次从"每轮全量"转成"增量 diff + 主链路定向",建议固化以下 invariant 作为 CI 静态扫描规则:
|
|
|
|
|
|
|
|
-- **High Risk 本轮为 0**:上轮 H-R11-1 的 TOCTOU 已被 `expectedUpdateTime` 显式透传修正;未发现新的可直接越权 / 篡改 / 伪造会话的路径。
|
|
|
|
|
-- **Medium 本轮 1 条**:M-R12-1(`BindRoles`+`DeleteRole` 锁链缺口)属于"罕见但数据无法自愈"的典型,修复成本低,推荐下个迭代处理。
|
|
|
|
|
-- **Low 本轮 5 条**:L-R12-1 ~ L-R12-5 全部落在"缓存一致性 / 文档一致性 / 已接受信号泄露"的维度,均不影响主功能正确性。
|
|
|
|
|
|
|
+1. 所有 `TransactCtx` 的 post-commit cache clean 必须用 detached ctx(L-R13-5)。
|
|
|
|
|
+2. 所有权限 / 管理类 HTTP / gRPC handler 的第一条业务语句必须是 `Require*` / `CheckManageAccess` 之一(L-R13-1)。
|
|
|
|
|
+3. `expectedUpdateTime` 参数必须从"调用方业务读"的快照传入,禁止在 model 层自读自比(H-R11-1 的长期闭环)。
|
|
|
|
|
|
|
|
-R11 闭环的 7 条(H-R11-1、M-R11-1/2/3、L-R11-1/4/5)在代码中均可检验到修复并配有审计注释,未出现历史修复回退。整体代码质量持续收敛,剩余建议多属"工艺与可维护性"级别的打磨。
|
|
|
|
|
|
|
+这三条都可以用 `ast` / `go/analysis` 级别的规则扫出来,成本远低于人工重走全链。
|