# 第 12 轮深度审计报告

审计对象：`perms-system/server`（不含测试代码）
审计维度：逻辑一致性、并发/竞态、资源管理、数据完整性、安全漏洞、边界、DB 性能、僵尸代码、接口契约
说明：本轮基于真实业务量级（数千用户 / 数十产品 / 单产品 <100 role / 一次 SyncPerms < 1k perm / 单 user 10~30 role）做判定。

## 🚩 核心逻辑漏洞 (High Risk)

本轮未发现新的 High Risk 漏洞。R11 的 H-R11-1 是上一轮可复现的密码 TOCTOU，本轮密码链路已经收敛；auth / token / dept / member / sync 四条主链已在锁序、乐观锁、CAS 三层护栏兜底，暂未看到可直接越权/篡改数据的 High 条目。

---

## ⚠️ 健壮性与性能建议 (Medium/Low)

### M-R12-1（Medium · 并发/数据完整性） · `BindRoles` 与 `DeleteRole` 之间无锁链，会留下孤儿 `sys_user_role`

**描述**：`internal/logic/user/bindRolesLogic.go` 在 `TransactCtx` 里只锁 `sys_product_member` 行（`FindOneForUpdateTx(member.Id)`），对即将绑定的 `sys_role` 行**完全不持锁**——连事务外的 `FindByIds` 也不 `FOR SHARE`。

同时 `internal/logic/role/deleteRoleLogic.go:41-56` 的 `DeleteRole` 在事务内依次：

```
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 锁要到**事务最末**才拿。两个事务交错：

```
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 提交
```

最终：`sys_user_role` 中存在 `roleId=R` 的行但 `sys_role[R]` 已被删除。由于 schema 未声明外键（`perm.sql` 里 `FOREIGN_KEY_CHECKS=1` 仅作用于 session，未定义任何 FK），该孤儿不会在写入时报错。

另一种交错（DeleteRole 先于 BindRoles 取到 sys_user_role 的 gap lock）可能导致 BindRoles 插入时阻塞，而后 `sys_role[R]` 已不在，BindRoles 仍然能成功写入 `sys_user_role(userId, R)`——同样产生孤儿。

**影响**：

- **功能正确性**：`UserDetailsLoader.loadRoles` 使用 `INNER JOIN sys_role` 过滤掉 `status` 非 Enabled 或已删除的角色，孤儿行**不会**被加载成用户权限——用户行为侧无可见异常。
- **数据不变式**：`sys_user_role.roleId` 指向不存在的 `sys_role.id` 违反隐式外键约束。单纯累积孤儿行成本低，但 `UserList` / `RoleList` / 运维排障时会看到"用户绑了一个查不到的角色 id" 的幽灵状态，需要另写清理脚本。
- **触发概率**：极低。真实业务里 `DeleteRole` 是罕见操作，必须同产品同角色同时 BindRoles；实际运维每月一次即是顶配。但一旦触发修复成本（人工清洗）远高于事前防御。

**修复方案**：在 `BindRoles` 的事务内对所有目标 `roleIds` 加 `SELECT ... FOR SHARE`（按 `id IN (...)` 排序取锁以避免死锁）：

```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
    }
    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 逻辑 ...
})
```

`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")`。

等价做法：`DeleteRole` 把 `FOR UPDATE sys_role[id]` 提前到事务第一步，等价形成 "X lock on sys_role → 所有写入 sys_user_role(roleId=R) 的事务要么先完成要么被阻塞"。两者择一。

---

### 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)
```

`go-zero` 的 `cachedSqlConn.ExecCtx` 行为：**先执行 exec 闭包，闭包返回成功后调用 `DelCacheCtx`**。这里的"成功"是指 `session.ExecCtx` 写入受影响行数 ≥ 1——而**事务此时尚未 commit**。

并发窗口：

```
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 缓存，解除窗口
```

`T4 - T6` 之间用户的 UserDetails 缓存里仍然是 T1 之前的旧值——包括 `DeptId / DeptPath / Status / TokenVersion`。由于 `UpdateUserLogic` 走 `UpdateProfileWithTx` 的唯一分支是"改 deptId 到新部门"（审计 M-R11-3 锁链的 happy path），窗口里被并发 Load 的用户会看到"旧部门 / 旧 path"，继续被按原 dept 做管辖决策。

窗口长度 ≈ session.ExecCtx 返回 → TransactCtx commit 之间的时长，正常路径下只有毫秒级。

**影响**：

- **功能正确性**：`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` 方法都有同样行为），具备跨文件一致性问题的特征。

**修复方案**：

- **方案 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*`），可视为**已接受的契约**。

我们倾向方案 A——事务语义一致性比 pre-commit 清掉一次缓存的微小收益更重要。

---

### L-R12-2（Low · 逻辑一致性） · `CreateDept` 事务内不复核 `parent.Status`，可在"父部门刚被禁用"时插入新子部门

**描述**：`internal/logic/dept/createDeptLogic.go:46-72`

```
① 事务外 FindOne(parentId) —— 读到 parent.Status=Enabled，捕获 parentPath
② 事务内 SELECT id FROM sys_dept WHERE id=? FOR SHARE —— 只校验"父部门存在"
③ InsertWithTx(子部门) —— 继承 parentPath、Status=Enabled
```

步骤②只看 `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
```

> 注：`UpdateDept` 走 `UpdateWithOptLock`，只要它拿到行锁后 CAS 成功即 commit；不与 `CreateDept` 的 FOR SHARE 同处一个锁链外的范围。

**影响**：

- **loadPerms 语义**：`UserDetailsLoader.loadPerms` 只用 `ud.DeptType / ud.DeptStatus`——即当前用户所在部门的 type / status——不递归向上看父部门，子部门 Enabled 完全可以独立工作，业务语义不破。
- **运营观感**：管理员"禁用整个部门子树"的意图被绕过，禁用父部门后若 CreateDept 正在进行，会看到一个 Enabled 子部门挂在 Disabled 父部门下，排障困难。
- **概率**：极低（人工操作时序 + 同部门并发）。

**修复方案**：在事务内把 `SELECT id` 升级成 `SELECT id, status, path FOR SHARE`，并同时复核 Status 与 Path：

```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("父部门已被禁用，无法创建子部门")
}
parentPath = parent.Path  // 改为使用事务内视图里的 path，理论上与事务外一致（UpdateDept 不改 path）
```

顺手覆盖一个边界：即使未来 `UpdateDept` 扩展为支持"重命名 path"，本修复也已经在事务内 snapshot 了最新 path。

---

### L-R12-3（Low · 僵尸代码 / 文档一致性） · `updateRoleLogic.go` 的注释"非超管不得降低 PermsLevel"与实际代码相反

**描述**：`internal/logic/role/updateRoleLogic.go` 中非超管的权限级别校验使用 `req.PermsLevel < role.PermsLevel`。在本项目的约定下"数字越小权限越高"（见 `UserDetailsLoader.loadRoles: MinPermsLevel` 计算），因此 `req.PermsLevel < role.PermsLevel` 实际拦截的是**提升权限**，而代码注释却写成"防止降低 PermsLevel"。

本条之前在 R11 审计里已经被口头标记，但未修。核心 bug 不存在——非超管的 `UpdateRole` 只有 product ADMIN 能进入，product ADMIN 在产品内有全权，提升 / 降低都不是越权；真正的边界是 `BindRoles` 的 `GuardRoleLevelAssignable`。但注释与代码反向的现状会让新维护者以为策略写错了，走入方向相反的"修复"最后引入真 bug。

**影响**：

- **运行期**：零。
- **维护期**：高——误导性注释是 R10 ~ R12 三轮审计里被反复关注的工作量。

**修复方案**：只改注释，把"防止降低"改成"防止非超管把角色提升到比当前更高的权限（数字更小 = 更高权）"，并在同一行把"数字越小 = 权限越高"这条约定显式写出来。

---

### L-R12-4（Low · 契约 · 已接受） · `GetUserPerms` 对"合法成员但被冻结"仍返回 `PermissionDenied`，可用于"合法成员 × 冻结态"枚举

**描述**：`internal/server/permserver.go:348-354`

```go
if ud.Username == "" || (!ud.IsSuperAdmin && ud.MemberType == "") {
    return nil, status.Error(codes.NotFound, "用户不是该产品的有效成员")
}
if ud.Status != consts.StatusEnabled {
    return nil, status.Error(codes.PermissionDenied, "用户已被冻结")
}
```

- `NotFound` 同时覆盖"用户全局不存在"和"用户存在但非本产品成员"——这是 L-R10-10 封死的枚举窗口。
- `PermissionDenied` 仍显式披露"用户是本产品成员且当前冻结"。

持有合法 `appKey + appSecret` 的产品在 `userId` 区间内扫描即可建立"该产品下哪些成员被冻结"的映射。在 `appSecret` 泄露后这是一条信息泄露面。

**影响**：

- 只有合法产品服务端可触发（凭 `bcrypt` 通过 appSecret 校验），攻击前提较高。
- 当前版本的安全侧评估（注释中明示）："密码正确才能拿到合法 appKey 这一前提不成立时，这个状态已经是上层业务承诺披露的信息，不构成新增枚举面。"

**结论**：保持现状作为已接受契约，不纳入 P0/P1。若未来"冻结状态"被列为 PII 敏感属性（目前不是），改为统一回 `NotFound "用户不是该产品的有效成员"` 即可。

---

### L-R12-5（Low · 安全信号最小化） · `AdminLogin` 对"冻结超管"与"成功超管"的时序差可观测

**描述**：`internal/logic/pub/adminLoginLogic.go:76-86`

```
① bcrypt.CompareHashAndPassword  -- real
② if u.Status != Enabled → 401
③ UserDetailsLoader.Load(u.Id, "")
④ GenerateAccessToken + GenerateRefreshToken
```

对冻结超管：①+② 耗时 ≈ 100ms（bcrypt 为主），不走 ③④。
对正常超管：①+②+③+④ 耗时 ≈ 100ms + 若干 DB 查询 + 两次 HMAC 签名 ≈ 150 ms+。

两种路径时序差约 10~50 ms，可被外部远程计时攻击用来区分"存在的超管被冻结" vs "存在的超管密码错误（相同 100ms）" vs "存在的超管成功（更长）"。

但该时序差**必须**已经持有正确超管密码才能被触发——攻击者需要先爆破出密码，再用 1000+ 请求做计时统计推断冻结。实际威胁模型下，拿到密码的人不 care "是否被冻结"。

**影响**：

- 实际风险极低；历史审计已归档为"可接受时序差"。
- 本条作为 R12 复核条目**重申**——不列 P0/P1。

**结论**：保持现状。如果合规侧（SOC2 / CC）要求所有"已存在账户" vs "不存在账户"分支必须同质化，可把 `③④` 抽到冻结分支里空跑一次（discard 结果），进一步把时序差压到 <5 ms。当前优先级低。

---

## 本轮复核中仍成立的契约（不再修）

- **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**：已在上文明示为接受契约，不纳入本轮修复窗口。

---

## 修复优先级

| 优先级 | 条目 | 理由 |
| ---- | ---- | ---- |
| 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 | 已归档为可接受契约 |

---

## 复核结论

经过 11 轮迭代 + 本轮（12）深度复核，`perms-system/server` 的核心授权 / 会话 / 数据持久化三条链路已达到一线生产系统的水准：

- **High Risk 本轮为 0**：上轮 H-R11-1 的 TOCTOU 已被 `expectedUpdateTime` 显式透传修正；未发现新的可直接越权 / 篡改 / 伪造会话的路径。
- **Medium 本轮 1 条**：M-R12-1（`BindRoles`+`DeleteRole` 锁链缺口）属于"罕见但数据无法自愈"的典型，修复成本低，推荐下个迭代处理。
- **Low 本轮 5 条**：L-R12-1 ~ L-R12-5 全部落在"缓存一致性 / 文档一致性 / 已接受信号泄露"的维度，均不影响主功能正确性。

R11 闭环的 7 条（H-R11-1、M-R11-1/2/3、L-R11-1/4/5）在代码中均可检验到修复并配有审计注释，未出现历史修复回退。整体代码质量持续收敛，剩余建议多属"工艺与可维护性"级别的打磨。