# 权限管理系统 - 深度代码审计报告（第 3 轮）

> 审计范围：`/internal` 下全部非测试生产代码（logic、model、middleware、handler、loaders、server、svc、consts、response、util）及入口文件 `perm.go`、`perm.api`。
> 审计时间：2026-04-19
> 审计重点：自我越权、并发竞态、接口 DoS、事务内外读写分裂、缓存一致性、接口契约完整性。
> 相对上一轮修复情况：
> - **已修复**：H-1（产品禁用联动）、H-2（JwtAuth MemberType 校验）、H-3（ManagementKey 前置）、H-4（最后一个 ADMIN 保护）、M-1（/auth/logout 路由）、M-2（refreshToken 轮转递增 tokenVersion）、M-9（BindRolePerms 事务外用户集查询已改为事务后）、M-11（`FindByPathPrefix`/`FindByParentId` 已清理）、M-12（UpdateUserStatus 重复校验已清理）、M-14（setUserPerms 校验产品启用）、M-15（BindRoles 无 diff 也清缓存）、M-16（用户 perm 查询已加 `p.status=1`）、L-3（非超管禁止下调 permsLevel）、L-4（CreateRole 校验产品启用）、L-5（AddMember 校验产品启用）、L-6（UserDetail 分支语义已明确）。
> - **未修复且仍存在**：M-3（`generateRandomHex` 截断 bug）、M-4（`DeptTree` 权限过滤）、M-5（`ProductList`/`ProductDetail` 权限过滤）、M-7（`X-Real-IP` 信任 + 无 XFF）、M-8（缓存失效非原子）、L-1/L-2/L-7~L-10。
> - **新增 P0/P1 问题**：H-A（`SetUserPerms` 自我权限越权）、H-B（`IncrementTokenVersion` 读缓存导致返回值错位）、M-A（"最后 ADMIN" 校验存在 TOCTOU）、M-B（`/auth/refreshToken` 无限流，DB 热点写 DoS）、M-C（产品登录用户枚举 + 选择性锁定）、M-D（`DeleteRole` 事务内但用非事务连接读用户集）、M-E（多个 `Delete*ForProductTx` 的 "先 SELECT 再 DELETE" 非原子）、M-F（`CountActiveAdmins` 硬编码 'ADMIN' 字面量）。

---

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

### H-A. `SetUserPerms` 对"自己调用自己"直接放行，普通 MEMBER 可自我授予产品内任意权限（自我越权）

- **位置**：
  - `internal/logic/user/setUserPermsLogic.go:50`
  - `internal/logic/auth/access.go:47`（`CheckManageAccess` 内的 `if caller.UserId == targetUserId { return nil }`）
- **描述**：
  - `SetUserPerms` 使用 `CheckManageAccess(ctx, svcCtx, req.UserId, productCode)` 作为唯一的访问控制。
  - `CheckManageAccess` 中"自己操作自己"是无条件放行的短路：
    ```go
    if caller.UserId == targetUserId {
        return nil
    }
    ```
  - 随后逻辑只校验：
    1. 目标是当前产品的有效成员（调用者自己当然是）；
    2. 传入的 permIds 都属于当前产品、且 `status=1`；
    3. DELETE 现有 `sys_user_perm`（userId+productCode），然后 `BatchInsert` 新的 (permId, effect) 对。
  - 没有校验"调用者本身是否有权授予该 perm"。

  攻击路径（任意 MEMBER 都可完成）：

  ```http
  POST /api/user/setPerms
  Authorization: Bearer <自己的 access token>
  {
    "userId": <自己的 userId>,
    "perms": [
      {"permId": 1, "effect": "ALLOW"},
      {"permId": 2, "effect": "ALLOW"},
      ...所有 permId...
    ]
  }
  ```

  下一次 `loadPerms` 会走"普通成员"分支：`rolePerms ∪ userAllow − userDeny`。用户自己塞进去的 `userAllow` 全部生效（第 427-431 行），直接获得整个产品的全部 `perms` 集合。中间件侧的 `ud.Perms` 也会包含这些 code；下游产品服务通过 `GetUserPerms` gRPC 拿到的权限同样被污染。

  该越权对 ADMIN/DEVELOPER/SUPER_ADMIN 无新增危害（这三类本来就有全产品权限），但对 **MEMBER 类型是完全的权限集逃逸**。

- **影响**：
  - 产品端的"最小权限"模型**彻底失效**：任何通过 `/auth/login` 登录的普通成员都能自我提权到"全权限"（等价于 ADMIN 级 perms，但不改 `memberType`、不触发 `checkDeptHierarchy` 等 ADMIN 后续护栏——也就是说**绕过权限级别校验、绕过部门护栏**，仅对自身 perms 集合生效）。
  - 与 `BindRoles` 形成对比：`BindRoles` 的"自己操作自己"在 `caller.MemberType == MEMBER` 时仍会被 `r.PermsLevel < caller.MinPermsLevel` 拦截（`bindRolesLogic.go:82-88`），避免了自我绑高等级角色；但 `SetUserPerms` **没有任何对应的自我越权护栏**。
  - 由于 `ud.Perms` 会被注入到 access token 的 `claims` 之外的 DB 查询结果中（走的是 loader，不是 JWT 内联 perms），攻击者不需要再刷 token，下一次请求就生效。

- **修复方案**（二选一或叠加）：

  **方案 A（最小侵入，直接禁止自我 set）：**

  ```go
  // internal/logic/user/setUserPermsLogic.go
  caller := middleware.GetUserDetails(l.ctx)
  if caller != nil && caller.UserId == req.UserId && !caller.IsSuperAdmin {
      return response.ErrForbidden("不能为自己设置权限")
  }
  ```

  **方案 B（对齐"高危写操作需 ADMIN"语义，推荐）：**

  ```go
  // 全部调用者限定为超管或该产品 ADMIN
  if err := authHelper.RequireProductAdminFor(l.ctx, productCode); err != nil {
      return err
  }
  ```

  方案 B 更贴近 API 注释"个性化权限"的管理语义（默认只有管理员能做精细化调权）。保留 `CheckManageAccess` 的层级校验作为纵深：

  ```go
  if err := authHelper.RequireProductAdminFor(l.ctx, productCode); err != nil {
      return err
  }
  if err := authHelper.CheckManageAccess(l.ctx, l.svcCtx, req.UserId, productCode); err != nil {
      return err
  }
  ```

  - 同步排查 `BindRoles`：对 MEMBER 目前通过 permsLevel 拦截了"自我绑低等级角色"，这条逻辑是护栏；但 `caller.MemberType == DEVELOPER` 自己绑自己任意角色的路径**仍能过**（DEVELOPER 本身就全权，无新增危害，保留即可）。保留现状是合理的。

- **优先级**：P0（立即修复）

---

### H-B. `SysUserModel.IncrementTokenVersion` 返回基于**缓存读**的新版本号，并发 refresh/logout 会签发"DB 已作废"的新 token

- **位置**：`internal/model/user/sysUserModel.go:140-156`
- **描述**：
  ```go
  func (m *customSysUserModel) IncrementTokenVersion(ctx context.Context, id int64) (int64, error) {
      data, err := m.FindOne(ctx, id)  // ← 从缓存读
      ...
      _, err = m.ExecCtx(ctx, func(ctx context.Context, conn sqlx.SqlConn) (sql.Result, error) {
          query := fmt.Sprintf("UPDATE %s SET `tokenVersion` = `tokenVersion` + 1, `updateTime` = ? WHERE `id` = ?", m.table)
          return conn.ExecCtx(ctx, query, time.Now().Unix(), id)
      }, sysUserIdKey, sysUserUsernameKey)
      if err != nil {
          return 0, err
      }
      return data.TokenVersion + 1, nil  // ← 基于旧缓存 + 1
  }
  ```

  两个严重问题：

  1. **缓存陈旧**：`data.TokenVersion` 来自 `FindOne` 的缓存层（`sqlc.CachedConn.FindOne`）。如果在另一条写链路（`UpdatePassword` / `UpdateStatus` / 另一次 `IncrementTokenVersion`）**DB 已 +1 但 Redis 缓存尚未失效**（M-8 窗口），缓存里读到的 `TokenVersion` 仍是旧值 X；`UPDATE tokenVersion+1` 把 DB 从 X+1 推到 X+2；函数却返回 X+1。随后签发的 access token 里 `tokenVersion=X+1` 与 DB 的 X+2 **不匹配**，**下一次请求直接被 `jwtauthMiddleware` 以 "登录状态已失效" 拒掉**。
  2. **并发自身**：即使不存在外部写入，两条并发 `RefreshToken` 也会命中同一现象——两条流程同时读缓存得 X，`UPDATE x+1` 让 DB 变成 X+2；两条流程都返回 X+1 签发了 access+refresh token 对，两张 refresh token 下一次使用都会因为 `TokenVersion != ud.TokenVersion` 被拒。

  调用方均依赖**返回值**作为新签 token 的 version：
  - `internal/logic/pub/refreshTokenLogic.go:66-75`（生成新 access/refresh）
  - `internal/server/permserver.go:141-148`（gRPC RefreshToken）

  这不只是理论问题——`RefreshToken` 现在每次都轮转 `tokenVersion`（上一轮 M-2 的修复要求），配合前端"多 tab / SW 并发 / 失败重试 / 双请求"等真实场景，用户会规律性遭遇"刷新一次就全家桶失效、需要完全重新登录"。

- **影响**：
  - 正常用户会看到**偶发的"刚刷新完 token 就被登出"**（灰度回溯难度高，属于典型的 TOCTOU 竞态）。
  - `Logout` 本身对此问题免疫（不读 `data.TokenVersion` 作为返回值），但 `RefreshToken` 严重受影响。
  - 间接让 "被盗 refresh token 立即失效" 的安全语义在正常用户身上误伤自己。

- **修复方案**：

  **方案 A（推荐，MySQL 原子自增 + 读取）：**

  ```go
  func (m *customSysUserModel) IncrementTokenVersion(ctx context.Context, id int64) (int64, error) {
      var newVersion int64
      sysUserIdKey := fmt.Sprintf("%s%v", cacheSysUserIdPrefix, id)
      // 先查出 username（仅用于缓存 key）
      data, err := m.FindOne(ctx, id)
      if err != nil {
          return 0, err
      }
      sysUserUsernameKey := fmt.Sprintf("%s%v", cacheSysUserUsernamePrefix, data.Username)
      err = m.TransactCtx(ctx, func(ctx context.Context, session sqlx.Session) error {
          // LAST_INSERT_ID trick：UPDATE 的同时把新值写入 LAST_INSERT_ID()
          upd := fmt.Sprintf("UPDATE %s SET `tokenVersion` = LAST_INSERT_ID(`tokenVersion` + 1), `updateTime` = ? WHERE `id` = ?", m.table)
          if _, err := session.ExecCtx(ctx, upd, time.Now().Unix(), id); err != nil {
              return err
          }
          return session.QueryRowCtx(ctx, &newVersion, "SELECT LAST_INSERT_ID()")
      })
      if err != nil {
          return 0, err
      }
      // 事务外再 purge 两个缓存 key
      _, _ = m.DelCacheCtx(ctx, sysUserIdKey, sysUserUsernameKey)
      return newVersion, nil
  }
  ```

  （`DelCacheCtx` 是 `sqlc.CachedConn` 公开接口；若 go-zero 当前版本未暴露，可在事务成功后通过 `m.ExecCtx` 触发一次空 UPDATE 以复用其自动 invalidation，或直接用 `m.rds.Del` 删 Redis key。）

  **方案 B（最小改动，用 `SELECT ... FOR UPDATE`）：**

  ```go
  err = m.TransactCtx(ctx, func(ctx context.Context, session sqlx.Session) error {
      var cur int64
      if err := session.QueryRowCtx(ctx, &cur,
          fmt.Sprintf("SELECT `tokenVersion` FROM %s WHERE `id` = ? FOR UPDATE", m.table), id); err != nil {
          return err
      }
      newVersion = cur + 1
      _, err := session.ExecCtx(ctx,
          fmt.Sprintf("UPDATE %s SET `tokenVersion` = ?, `updateTime` = ? WHERE `id` = ?", m.table),
          newVersion, time.Now().Unix(), id)
      return err
  })
  ```

  修复后需同时确认 `ChangePassword` / `UpdateStatus` / `UpdateProfile`（status 变更分支）等"递增"路径都走同一实现，避免一致性漂移。

- **优先级**：P0（立即修复，生产将周期性出现自伤）

---

## ⚠️ 健壮性与安全建议 (Medium)

### M-A. `CountActiveAdmins` 校验在事务外，`RemoveMember` / `UpdateMember` "最后 ADMIN" 检查存在 TOCTOU

- **位置**：
  - `internal/logic/member/removeMemberLogic.go:41-49`
  - `internal/logic/member/updateMemberLogic.go:50-58`
  - `internal/model/productmember/sysProductMemberModel.go:49-56`（`CountActiveAdmins`）
- **描述**：新加入的"最后一个 ADMIN 保护"机制是：
  ```go
  if member.MemberType == consts.MemberTypeAdmin {
      adminCount, _ := svcCtx.SysProductMemberModel.CountActiveAdmins(ctx, member.ProductCode)
      if adminCount <= 1 {
          return response.ErrBadRequest("不能移除该产品的最后一个管理员")
      }
  }
  // …然后进入事务 DELETE
  ```

  `CountActiveAdmins` 用 `QueryRowNoCacheCtx` 读 DB，**但不加锁**，且整个"判断 + DELETE"**不在同一个事务里**。

  并发路径：

  | 时序 | 请求 A（移除 admin_P） | 请求 B（移除 admin_Q） |
  |------|------------------------|------------------------|
  | T1   | `CountActiveAdmins` = 2 |                        |
  | T2   |                        | `CountActiveAdmins` = 2 |
  | T3   | 进入 Tx → DELETE admin_P |                        |
  | T4   |                        | 进入 Tx → DELETE admin_Q |
  | T5   | 产品剩余 ADMIN = 0      | 产品剩余 ADMIN = 0      |

  `UpdateMember`（ADMIN 降级为 MEMBER）存在同样的 TOCTOU。真实触发场景：

  - 超管在管理后台"批量移除"两个 ADMIN；
  - 两个 ADMIN 同时在前端点"移除对方"；
  - 自动化脚本一次性下发两条变更。

  虽然概率不高，但"产品永久无 ADMIN 需平台救援"的代价高。

- **影响**：绕过了上一轮 H-4 的"最后一个 ADMIN"保护，导致产品进入无人管理态。

- **修复方案**：把 count 检查挪到事务内、并对要变更/删除的那行加行锁即可；对其他 ADMIN 行不需要锁（因为 count 查询会用到 InnoDB 的当前读规则，配合事务隔离级别足够）。

  ```go
  // RemoveMember 示例（UpdateMember 同理）
  return l.svcCtx.SysProductMemberModel.TransactCtx(l.ctx, func(ctx context.Context, session sqlx.Session) error {
      // 1) 锁定当前要删的行，避免重复删
      var lockedId int64
      lockQ := fmt.Sprintf("SELECT `id` FROM %s WHERE `id` = ? FOR UPDATE", l.svcCtx.SysProductMemberModel.TableName())
      if err := session.QueryRowCtx(ctx, &lockedId, lockQ, req.Id); err != nil {
          return response.ErrNotFound("成员不存在")
      }
      // 2) 最后一个 ADMIN 校验在事务内
      if member.MemberType == consts.MemberTypeAdmin {
          var cnt int64
          cntQ := fmt.Sprintf(
              "SELECT COUNT(*) FROM %s WHERE `productCode`=? AND `memberType`=? AND `status`=? FOR SHARE",
              l.svcCtx.SysProductMemberModel.TableName(),
          )
          if err := session.QueryRowCtx(ctx, &cnt, cntQ, member.ProductCode, consts.MemberTypeAdmin, consts.StatusEnabled); err != nil {
              return err
          }
          if cnt <= 1 {
              return response.ErrBadRequest("不能移除该产品的最后一个管理员")
          }
      }
      // 3) 既有的 DeleteByUserIdForProductTx / DeleteWithTx
      ...
  })
  ```

  备选：保留现有 `CountActiveAdmins` 调用顺序，但在 DELETE 成功**之后**再 count 一次，若 `=0` 则显式 `return fmt.Errorf("rollback: last admin")` 触发回滚。这个实现更简单。

- **优先级**：P1

---

### M-B. `/auth/refreshToken` 路由无任何限流，可对单用户发起 DB 写热点 DoS 并清空其所有缓存

- **位置**：`internal/handler/routes.go:176-185`（`/auth/refreshToken` 无中间件）、`internal/logic/pub/refreshTokenLogic.go:66-70`
- **描述**：
  - 上一轮 M-2 修复后，`RefreshToken` 每次都会：
    1. `IncrementTokenVersion`：`UPDATE sys_user SET tokenVersion=tokenVersion+1` + 清两个 cache key；
    2. `UserDetailsLoader.Clean(userId)`：`SMEMBERS` 该用户 index → `DEL` 所有 cache key → `DEL` index key。
  - 路由挂载处（`routes.go:176-185`）**没有 `JwtAuth`、没有 IP 限流**，只有签名 + `tokenVersion` 校验。
  - 只要攻击者曾经拿到过任意一张有效 refreshToken（或者就是合法用户自己被攻破），就能每秒反复调用：
    - 每次一条 `UPDATE sys_user`（写热点，命中行锁）；
    - 每次一轮 Redis `SMEMBERS` + `DEL`（用户缓存整体失效，后续所有请求都 miss 并穿透到 DB 的 `loadFromDB`，进而扇出 6 条 DB 查询）。
  - 单用户持续被刷，等价于"该用户的每次业务请求都穿透 loader，全部冷路径查 DB"，且对 `sys_user` 表形成写热点。如果该用户是 ADMIN/超管，其 `loadFromDB` 里 `loadPerms` 会扫 `sys_perm` 全表拉"该产品全量 code"，代价更高。

  关键是：**刷新是发出 token 后就能做的，没有任何 IP/账号层面的限流**。`AdminLoginRateLimit` / `ProductLoginRateLimit` 只防登录入口，`SyncRateLimit` 只防 `/perm/sync`。

- **影响**：单个凭据即可对权限系统制造持续的 DB 写 + 缓存穿透；且由于每次刷新都轮转 refresh token，攻击者总能拿到下一张有效凭据继续刷。
- **修复方案**：
  - 在 `/auth/refreshToken` 前挂一条 `refreshLimiter`，key = `user:<userId>` 或 `ip+user`，窗口建议 60s/6 次（用户实际只会在 access 过期前偶发刷新，多端也不太会超过 3 次）。
  - 或者给 `refreshTokenLogic` 内部加一层"Redis 计数器"：以 `rl:refresh:<userId>` 为 key 做自增 + TTL。
  - 超额后不应无条件 429，可选择降级为"返回同一张已经签发的 access token"，但这又引入复杂度，保持 429 最简单。

- **优先级**：P1

---

### M-C. 产品端登录 `ValidateProductLogin`：存在用户名枚举 & 选择性账号锁定

- **位置**：`internal/logic/pub/loginService.go:32-46`
- **描述**：当前顺序是：
  ```go
  u, err := svcCtx.SysUserModel.FindOneByUsername(ctx, username)
  if err != nil { // 不存在 → 401
      return nil, &LoginError{Code: 401, Message: "用户名或密码错误"}
  }
  if svcCtx.UsernameLoginLimit != nil {
      code, _ := svcCtx.UsernameLoginLimit.Take(username)
      if code == limit.OverQuota {
          return nil, &LoginError{Code: 429, Message: ...}
      }
  }
  if u.Status != consts.StatusEnabled { ... }
  if err := bcrypt.CompareHashAndPassword(...); err != nil { return 401 }
  ```

  两个衍生问题：

  1. **用户名枚举**：不存在的 username 直接返回 401、**不消耗任何限流配额**，响应时间也很短（不走 bcrypt）；存在的 username 则可能返回 401（密码错）或 429（锁定）或 403（冻结）。攻击者通过"时间 + 响应码"组合能稳定区分 username 是否存在。
  2. **选择性锁定**：攻击者探测出真实 username 后，对该 username 连打 10 次，该 username 立即被 `UsernameLoginLimit`（5min/10 次，key 纯 username）锁定 5 分钟，真正用户同期无法登录。配合 `AdminLoginRateLimit`（IP 60s/30）毫无阻塞。

  `/auth/adminLogin` 已经把 `UsernameLoginLimit.Take` 移到 `ManagementKey` 校验之后，享受了一定保护（攻击者必须知道 ManagementKey 才能触发）。但 `/auth/login` 没有这种护栏。

- **影响**：外部攻击者可在不触发限流的前提下批量探测有效 username；对已知用户可周期性（每 5 分钟一轮）持续锁定。
- **修复方案**：
  - **限流 key 加 IP 维度**：`UsernameLoginLimit` 的 key 改为 `fmt.Sprintf("%s:%s", ip, username)`，同时保留一个更宽松的"纯 IP 限流"（已有 `ProductLoginRateLimit`），这样攻击者无法通过廉价 IP 锁定目标账号。
  - **`FindOneByUsername` 之前先 `Take`**：让 Take 对无效 username 也消耗配额（这样无效 username 也会得 429，不再区分）；或者在 username 不存在时直接 `time.Sleep` 一个近似 bcrypt 的耗时，避免时间侧信道。
  - **成功登录时重置计数**（go-zero `PeriodLimit` 不支持 reset，可以改用 Redis 自增 + 登录成功 `DEL` key）。

- **优先级**：P1

---

### M-D. `DeleteRoleLogic` 的 `FindUserIdsByRoleId` 写在事务回调内，却**没有用 session**，仍是旧连接读

- **位置**：
  - `internal/logic/role/deleteRoleLogic.go:40-50`
  - `internal/model/userrole/sysUserRoleModel.go:55-62`（`FindUserIdsByRoleId` 用的是 `m.QueryRowsNoCacheCtx`，不接 `session`）
- **描述**：
  ```go
  if err := l.svcCtx.SysRoleModel.TransactCtx(l.ctx, func(ctx context.Context, session sqlx.Session) error {
      affectedUserIds, _ = l.svcCtx.SysUserRoleModel.FindUserIdsByRoleId(ctx, req.Id) // ← 这里没传 session
      ...
      return l.svcCtx.SysRoleModel.DeleteWithTx(ctx, session, req.Id)
  })
  ```

  `FindUserIdsByRoleId` 直接用 `m.QueryRowsNoCacheCtx`（非事务连接、非当前 session）查询。结果：

  - 这一行读位于事务**之外**，本事务内尚未提交的变更看不到；
  - 反之，事务**之外**并发提交的 `BindRoles`（另一 goroutine 给这个 roleId 插入新行）会被这条 SELECT **看到，或看不到**，取决于提交时序；
  - 真正的问题是：**在当前事务 COMMIT 之前**，如果另一条已提交的并发事务给 roleId 加了新用户 U'，本事务的 DELETE（`DeleteByRoleIdTx`）**会把 U' 也删掉**（因为它用 `DELETE ... WHERE roleId=?`），但 `affectedUserIds` 中不包含 U'，所以**U' 的 UserDetails 缓存不会被清理**。

  同样的问题在 `BindRolePermsLogic`（`bindRolePermsLogic.go:127`）已经通过"事务提交后再查"规避；`UpdateRoleLogic`（`updateRoleLogic.go:73`）也是事务外后查。只剩 `DeleteRole` 这一条路径仍然错位。

- **影响**：`DeleteRole` 并发 `BindRoles` 的小概率场景下，漏清一批用户的缓存；这些用户会继续持有"引用已删角色"的权限集合，直到 300s TTL 自然过期。
- **修复方案**：把 `FindUserIdsByRoleId` 挪到事务提交**之后**：

  ```go
  if err := l.svcCtx.SysRoleModel.TransactCtx(l.ctx, func(ctx context.Context, session sqlx.Session) error {
      if err := l.svcCtx.SysRolePermModel.DeleteByRoleIdTx(ctx, session, req.Id); err != nil { return err }
      if err := l.svcCtx.SysUserRoleModel.DeleteByRoleIdTx(ctx, session, req.Id); err != nil { return err }
      return l.svcCtx.SysRoleModel.DeleteWithTx(ctx, session, req.Id)
  }); err != nil {
      return err
  }
  // 提交之后再读（此时该 roleId 已无任何关联行，但仍能查到曾经的 userId——需要在 DELETE 之前保留）
  ```

  **注意**：`DELETE ... WHERE roleId=?` 执行后，`sys_user_role` 里 `roleId=?` 的行已没了，提交后再查 `FindUserIdsByRoleId` 得空集。正确做法是在事务内（用 session）先读一次，然后再 DELETE：

  ```go
  if err := l.svcCtx.SysRoleModel.TransactCtx(l.ctx, func(ctx context.Context, session sqlx.Session) error {
      // 在事务内用 session 读，锁住这些关系行
      query := fmt.Sprintf("SELECT `userId` FROM `sys_user_role` WHERE `roleId` = ? FOR UPDATE")
      if err := session.QueryRowsCtx(ctx, &affectedUserIds, query, req.Id); err != nil {
          return err
      }
      ...DELETE...
      return l.svcCtx.SysRoleModel.DeleteWithTx(ctx, session, req.Id)
  })
  ```

  `FOR UPDATE` 让并发 `BindRoles` 的 `INSERT sys_user_role` 被挂起，直到当前事务提交。这样 affectedUserIds 覆盖所有实际被删的行。

  需要在 `SysUserRoleModel` 接口新增一个 `FindUserIdsByRoleIdForUpdateTx(ctx, session, roleId)`，或者 DeleteRole 内直接拼 SQL（如上）。

- **优先级**：P2（低概率，但修复成本很小）

---

### M-E. 所有 `Delete*ForProductTx` / `DeleteByRoleIdTx` / `DeleteByUserIdForProductTx` 的"先 SELECT 再 DELETE"模式非原子

- **位置**：
  - `internal/model/userrole/sysUserRoleModel.go:75-107,109-136`
  - `internal/model/roleperm/sysRolePermModel.go:73-117`
  - `internal/model/userperm/sysUserPermModel.go:43-63`
  - `internal/model/perm/sysPermModel.go:94-154`（`DisableNotInCodesWithTx`）
- **描述**：统一套路是：
  ```go
  // 1) 先用 QueryRowsNoCacheCtx 读出要删的行，拼 cache keys
  if err := m.QueryRowsNoCacheCtx(ctx, &list, findQuery, ...); err != nil { return err }
  keys := buildCacheKeys(list)
  // 2) 再 m.ExecCtx 包装 session.ExecCtx，借助 ExecCtx 的 cache-aside invalidate
  _, err := m.ExecCtx(ctx, func(ctx, conn) {
      return session.ExecCtx(ctx, deleteQuery, ...)
  }, keys...)
  ```

  问题：
  1. **SELECT 用的是 `m.QueryRowsNoCacheCtx`（主库默认连接，读未提交前的数据）**，它既不是 `session`，也没有加锁。如果此刻有并发 INSERT 提交（例如 BindRoles 插入新 `sys_user_role`），那条新行的 cache key 不会进入 `keys` 列表。
  2. 紧接着 `DELETE ... WHERE roleId=?` 会一并删掉该新行；新行的 `cacheSysUserRoleIdPrefix:<newId>` 和 `cacheSysUserRoleUserIdRoleIdPrefix:<userId>:<roleId>` 缓存**不会被 invalidate**。
  3. 一般情况下，新行刚插入缓存也没写过（只有 `FindOne` 会写缓存，insert 不写），所以大部分时候这些 key 本来就不存在，问题不显现。但一旦有其它读路径（例如 `FindOne`、或 `DeleteByUserIdAndRoleIdsTx` 里的 list 查询本身）在极短窗口内 cache miss + 回填，就会留下**已 DELETE 但仍在 Redis 的幽灵缓存**，直到 TTL 自然过期。

  同时 `perm/sysPermModel.go:DisableNotInCodesWithTx` 走的路径里，SELECT 是非事务读，之后的 UPDATE 是 session.ExecCtx，如果并发 `SyncPerms` 插入了新 perm code（不在 `codes` 列表），第二轮 SELECT 不会看到，但 UPDATE 的 `WHERE NOT IN (codes)` **会禁用**它——该 perm 的缓存键不会被清理。

  这是一类普遍的"缓存 keys ≠ 实际被影响行"的问题。

- **影响**：
  - 罕见但可触发的"幽灵缓存"问题，表现为"某用户/角色/权限在 DB 已删但 Redis 仍返回老值"，最多持续 300s（默认 TTL）+ loader.Load 的 singleflight 覆盖。
  - 实际业务影响通常被 `UserDetailsLoader` 的 300s TTL 吸收——`loader.Load` 是以用户维度缓存 `UserDetails`，不直接依赖 `sys_user_role.id` 级 cache key。所以**实际暴露面主要在直接调 `SysXxxModel.FindOne(id)` 的路径**。
  - 风险较低，但长期看会积累数据不一致。

- **修复方案**：
  - **方案一（推荐）**：把 SELECT 改为在 session 内执行，并且 `FOR UPDATE`：

    ```go
    findQuery := fmt.Sprintf("SELECT ... WHERE `roleId` = ? FOR UPDATE")
    if err := session.QueryRowsCtx(ctx, &list, findQuery, roleId); err != nil { return err }
    ```

    这样在事务提交前，其他事务无法插入新行到同一 roleId 范围内；SELECT 和 DELETE 看到的是完全一致的集合。

  - **方案二**：事务提交后再做一次"宽清理"——直接 `SREM` 掉该 `roleId` / `productCode` 下的全部 cache 键；粒度粗但绝不会漏。
  - **方案三（最小成本）**：既然缓存不一致窗口 ≤ 300s 且 `UserDetailsLoader` 层做主业务隔离，可将这类 cache key 的 TTL 降到 60s，限制风险窗口。

- **优先级**：P2（风险低、影响面小，但建议与 M-D 合并一次性收敛）

---

### M-F. `CountActiveAdmins` SQL 字面量 `'ADMIN'` 与 `consts.MemberTypeAdmin` 解耦，僵尸常量同步风险

- **位置**：`internal/model/productmember/sysProductMemberModel.go:51`
- **描述**：
  ```go
  query := fmt.Sprintf("SELECT COUNT(*) FROM %s WHERE `productCode` = ? AND `memberType` = 'ADMIN' AND `status` = 1", m.table)
  ```

  `'ADMIN'` 和 `1`（StatusEnabled）都是裸字面量，没有引用 `consts.MemberTypeAdmin` / `consts.StatusEnabled`。一旦常量值调整（例如未来新增 `OWNER` 或 `ADMIN` 更名为 `PRODUCT_ADMIN`），Go 调用方的判断和 SQL 查询会**悄悄脱钩**——不会编译报错、不会测试失败（除非有专门的覆盖测试），但"最后一个 ADMIN"保护**逻辑失效**（SQL 找不到任何 ADMIN，count 永远 = 0，所有移除都被拒）。

- **影响**：维护风险（契约隐性约定）；也违反"同一信息不写两遍"原则，与 `FindAllCodesByProductCode`（`perm/sysPermModel.go:56`）等现有做法不一致（那边用了 `consts.StatusEnabled` 拼到 SQL）。
- **修复方案**：

  ```go
  query := fmt.Sprintf(
      "SELECT COUNT(*) FROM %s WHERE `productCode` = ? AND `memberType` = ? AND `status` = ?",
      m.table,
  )
  return m.QueryRowNoCacheCtx(ctx, &count, query, productCode, consts.MemberTypeAdmin, consts.StatusEnabled)
  ```

- **优先级**：P2

---

### M-G. `UserList` 在有 `productCode` 分支时，JOIN 后又多做了一次 `FindMapByProductCodeUserIds`

- **位置**：
  - `internal/logic/user/userListLogic.go:51-76`
  - `internal/model/user/sysUserModel.go:59-76`（`FindListByProductMembers` 已 INNER JOIN `sys_product_member`）
- **描述**：当 `req.ProductCode != ""` 时：
  1. `FindListByProductMembers` 已经 `INNER JOIN sys_product_member pm ON u.id=pm.userId WHERE pm.productCode=?`——此时每行都有对应的 `pm.memberType`；
  2. 代码又发起一次 `FindMapByProductCodeUserIds` 把 `userIds` IN 回 `sys_product_member` 取 `memberType`。

  两次查询做了完全一样的事。这是一个纯粹的冗余读——第一次 JOIN 丢弃了 `pm.*`，第二次再 IN 取回。
- **影响**：产品端用户列表多一次全表/IN 扫描，无功能性问题。
- **修复方案**：扩展 `FindListByProductMembers` 返回 `[]struct{ *SysUser; MemberType string }`（或返回 `memberType` 数组），替掉第二次查询。

- **优先级**：P2

---

### M-3（遗留）. 产品初始管理员随机密码熵仍为 32 bits

- **位置**：`internal/logic/product/createProductLogic.go:80-81, 158-164`
- **状态**：未修复。`generateRandomHex(8)` 依然使用了截断 bug 的路径：
  ```go
  b := make([]byte, 8)              // 8 字节 = 64 bits
  rand.Read(b)
  return hex.EncodeToString(b)[:8]  // 取前 8 hex 字符 = 4 字节 = 32 bits
  ```
- **影响**：见上一轮审计。虽然 `MustChangePassword=Yes`，但一次性密码暴力破解窗口依然存在。
- **修复方案**：同上一轮方案——修正截断边界或直接把 adminPassword 调大到 16 字符（64 bits）。
- **优先级**：P1

---

### M-4（遗留）. `/api/dept/tree` 对所有已登录用户开放，暴露完整组织架构

- **位置**：`internal/logic/dept/deptTreeLogic.go:27-66`
- **状态**：未修复。
- **建议**：仅超管可拉全量；其他用户按 `caller.DeptPath` 过滤（只返回自身部门及其子树）。

---

### M-5（遗留）. `ProductList` / `ProductDetail` 对所有已登录用户返回系统级元数据

- **位置**：`internal/logic/product/productListLogic.go`、`productDetailLogic.go`
- **状态**：未修复。
- **建议**：非超管只能看到自己有有效成员资格的产品。

---

### M-7（遗留）. `ExtractClientIP` 仅识别 `X-Real-IP`，未识别 `X-Forwarded-For`，且无可信代理白名单

- **位置**：`internal/middleware/ratelimitMiddleware.go:41-52`
- **状态**：未修复。
- **建议**：同上一轮——按"可信代理 CIDR 列表 + XFF 取最右可信值 > X-Real-IP > RemoteAddr"。

---

### M-8（遗留）. 缓存失效为 fire-and-forget，"收紧类"安全动作 + Redis 抖动 = 5 分钟内旧视图生效

- **位置**：所有 `Del/Clean/BatchDel/CleanByProduct` 的调用点。
- **状态**：未修复。
- **建议**：
  - 对 `UpdateUserStatus(Disabled)`、`ChangePassword`、`RemoveMember`、`Logout`、`UpdateMember(Status=Disabled)` 等"收紧"类动作，缓存清理失败必须返回 5xx / 重试；
  - 或者在这些路径上同步走 "DB tx 提交 → 消息队列发事件 → 消费方确保删干净"，把缓存失效转为幂等补偿。
  - 对新加入的 `Logout`，`Clean` 失败目前只打日志，用户虽然 DB 的 `tokenVersion+1` 生效、但缓存里旧 ud.TokenVersion 仍在——中间件对比 claim 里的旧 version vs cache 里的旧 version，**会继续放行** 5 分钟，这和 `Logout` 的"立即失效"语义严重冲突。此项建议升到 P1。

---

## 📝 低风险 / 遗留问题 (Low)

### L-A. `CreateUser` 注释写"仅超管可调用"，实际却允许产品 ADMIN 调用

- **位置**：`internal/logic/user/createUserLogic.go:38-43`
- **描述**：
  ```go
  // CreateUser 创建用户。新建系统用户账号，可指定部门归属。仅超管可调用。
  func (l *CreateUserLogic) CreateUser(req *types.CreateUserReq) (...) {
      productCode := middleware.GetProductCode(l.ctx)
      if err := authHelper.RequireProductAdminFor(l.ctx, productCode); err != nil { ... }
  ```
  产品 ADMIN（登录时带 productCode）可以通过该接口创建**系统级用户**（没有任何产品归属）。该用户不会自动加入任何产品，ADMIN 也只能把他加入自己这一个产品；但**文档契约与行为不一致**，容易在后续变更时导致误解。
- **建议**：若意图是"超管专用"，改为 `RequireSuperAdmin`；若意图是"产品 ADMIN 也可以"，改注释并在 `perm.api` 的接口注释中说明。

---

### L-B. `jwtauthMiddleware` 校验顺序：`tokenVersion` 在 `ProductStatus/MemberType` 之后

- **位置**：`internal/middleware/jwtauthMiddleware.go:78-93`
- **描述**：顺序是 `Status → ProductStatus → MemberType → TokenVersion`。当用户已 `Logout` 或 `ChangePassword`（`tokenVersion` 应失效），但产品被同步禁用了，会优先返回 "该产品已被禁用" 而不是 "登录已失效"。这会给前端错误的 UX 分支（比如前端见到 "产品禁用" 可能不会清本地 token，而是提示用户"联系管理员恢复产品"；实际上该 token 已经作废了）。
- **建议**：把 `tokenVersion` 检查提到最前（或仅次于 `Status != Enabled`）。这也符合 "优先拒绝无效凭据，再拒绝业务层禁用" 的错误码语义。

---

### L-C. `Logout` 无并发保护，一条 token 可不断自增 `tokenVersion`

- **位置**：`internal/logic/auth/logoutLogic.go:30-43`
- **描述**：`Logout` 虽然是幂等（多次 +1 不伤害业务），但每次都会：
  1. 发 `UPDATE sys_user tokenVersion+1`（行锁）；
  2. 清所有 UserDetails 缓存。
  攻击者拿到一次有效 access token，就能反复调 `/auth/logout`——每次都是 **DB 写 + 全缓存清**，比 M-B 的 refresh 路径更轻量。
- **建议**：对 `Logout` 同步加一条 "60s / 6 次" 的 userId 级限流；或者在中间件层面对"高写入路径"统一限流。

---

### L-D. `SyncPerms` 通过 appKey 查询，appKey 错误 vs appSecret 错误返回同一文案，但**响应时间**差异明显

- **位置**：`internal/logic/pub/syncPermsService.go:37-43`
- **描述**：appKey 不存在 → 401，不走 bcrypt（快）；appKey 存在但 appSecret 错 → 401，走 bcrypt（慢）。攻击者可据此枚举有效 appKey。
- **影响**：低——appKey 对外不暴露，枚举难度大；但如果将来 `/api/perm/sync` 暴露在公网，这是信息泄露。
- **建议**：appKey 不存在时也跑一次 dummy bcrypt，或对该接口限流。

---

### L-E. 保留的 `FindRoleIdsByUserId`（不按 productCode 过滤）在当前业务中几乎不可达

- **位置**：`internal/model/userrole/sysUserRoleModel.go:37-44`、`internal/logic/user/userDetailLogic.go:53`
- **描述**：仅在 `UserDetail` 的 `productCode == ""` 分支调用。该分支只有超管未带产品上下文时才进入（超管通过 `adminLogin` + 查用户详情页）。虽可达但使用面极窄，且返回**跨全部产品**的 roleIds，对 API 消费方语义模糊（前端只拿到 roleIds 无 productCode 区分，容易误用）。
- **建议**：保留但在接口文档里明确"此分支的 roleIds 是跨产品聚合"，或改为按 "caller 的产品上下文"（不等价，但更可预测）。

---

### L-F. `UpdateUser` 允许他人调用者修改目标的 `deptId`，但未校验新 `deptId` 是否在调用者可管理的子树中

- **位置**：`internal/logic/user/updateUserLogic.go:99-106`
- **描述**：`CheckManageAccess` 只校验"可管理目标用户"，不校验"新目标部门"在 caller 的子树中。实际上 `checkDeptHierarchy` 对 ADMIN 直接放行（第 101-103 行），但对 DEVELOPER 会先走 `callerDeptPath` 前缀匹配；DEVELOPER 如果能管理到跨部门的用户（极少数配置下），就能把他调到任意部门。由于 `CheckManageAccess` 的部门校验只限制"目标当前部门"，不限制"目标的新部门"，DEVELOPER 可以把下属挪出自己的子树。
- **建议**：更新 `deptId` 时，新 deptId 若非空，校验 `caller.IsSuperAdmin || caller.MemberType == ADMIN || strings.HasPrefix(newDept.Path, caller.DeptPath)`。

---

### L-G. `loginService.ValidateProductLogin` 对已冻结用户的密码仍做 bcrypt 比对

- **位置**：`internal/logic/pub/loginService.go:48-54`
- **描述**：Status 检查在 bcrypt 之前，但中间多了一步 `UsernameLoginLimit.Take` → 已冻结用户仍会消耗配额。累积消耗后，**真正解冻后用户 5 分钟内无法登录**。
- **建议**：Status 检查前置到 FindOneByUsername 之后、`UsernameLoginLimit.Take` 之前，冻结即返回 403，不消耗配额。

---

### L-H. `ValidateProductLogin` 成功登录后没有重置 `UsernameLoginLimit`

- **位置**：同上
- **描述**：go-zero 的 `PeriodLimit` 没有 reset API，因此"登录成功"不会归零失败计数。连续多次失败（例如输错 3 次、然后登录成功）后 24 小时内的窗口内若再输错 7 次仍会触发锁定——这对用户体验不友好，也让攻击者可以通过合法登录不打破锁定（因为锁定只和失败计数相关）。
- **建议**：改用 Redis 自增 + 登录成功时 `DEL` key，或只在 `bcrypt.CompareHashAndPassword` 失败时才计数。

---

### L-I. `AddMember` 不校验目标用户 `Status`

- **位置**：`internal/logic/member/addMemberLogic.go:41-43`
- **描述**：只校验用户存在、不校验 `u.Status == Enabled`。可以把已冻结用户加成员；他们被解冻的瞬间就拥有产品访问权。
- **建议**：`if u.Status != consts.StatusEnabled { return ErrBadRequest("用户已被冻结，无法添加为成员") }`。

---

### L-J. `UpdateUser` 允许他人修改**目标用户的 `Status` 值**，与 `UpdateUserStatus` 职责重叠

- **位置**：`internal/logic/user/updateUserLogic.go:108-125`、`internal/logic/user/updateUserStatusLogic.go`
- **描述**：两条接口都能改 `Status`；但 `UpdateUser` 的路径只在"用户维度"`Clean`，而 `UpdateUserStatus` 显式通过 `UpdateStatus`（tokenVersion+1）。`UpdateUser` 里的 `UpdateProfile(statusChanged=true)` 模型代码**也** `tokenVersion+1`，行为一致，所以功能等价；但两条路径分别做校验、很容易在一侧加了新防御一侧漏掉（比如 `UpdateUserStatus` 已经禁止自改自己、不允许改超管；`UpdateUser` 只在 `caller.UserId == req.Id` 时禁 status，漏掉了"不允许把超管冻结"——实际上走 `UpdateUser` 修改超管 Status 会被第 56-60 行的"不能通过此接口修改其他超级管理员的状态和部门"拦下，OK）。
- **建议**：要么把 `UpdateUser` 里处理 `Status` 的分支删掉（转嫁给 `UpdateUserStatus`），要么显式文档化两接口的角色差异，避免未来维护者再添新守则时漏一处。

---

### L-K. `BindRoles` 的 caller 读取在校验之后但使用条件分散，易读性差

- **位置**：`internal/logic/user/bindRolesLogic.go:65-89`
- **描述**：`caller := middleware.GetUserDetails(l.ctx)` 在角色数组校验中段读取，caller 可能为 nil（中间件理论上保证非 nil，但代码仍做 `caller != nil && ...`）。这种"半防御"容易让后续维护者以为 caller 可能为 nil，加上无必要的判空。
- **建议**：在函数开头统一读取 caller，确认非 nil；后续逻辑直接使用。

---

### L-1 / L-2 / L-7 / L-8 / L-9 / L-10（前轮遗留）

- 与上一轮一致，此处不再赘述。保持 P3 跟进。

---

## 📋 审计总结

| 维度 | 评估 |
|------|------|
| **逻辑一致性** | 新增重大发现：`SetUserPerms` 自我越权（H-A）；"最后 ADMIN" 校验 TOCTOU（M-A）；`IncrementTokenVersion` 返回值与 DB 脱钩（H-B）。 |
| **并发与竞态** | `DeleteRole` 仍有事务内外分裂（M-D）；`Delete*ForProductTx` 的 "先 SELECT 再 DELETE" 模式非原子（M-E）。 |
| **资源管理** | 无新增泄漏；`RefreshToken` / `Logout` 无限流造成 DB + Redis 热点（M-B/L-C）。 |
| **数据完整性** | 关键写路径事务化；剩余问题都在"读-用-改"时序上。`UpdateMember` 的 `req.Status=0` 分支保留原值，无覆盖风险。 |
| **安全漏洞** | H-A（MEMBER 自我提权全权限）属于严重权限越权；H-B 导致用户随机被登出（可用性），也降低 refresh rotation 的安全语义；M-C 仍允许账号枚举 + 选择性锁定。 |
| **边界处理** | `SetUserPerms` / `BindRoles` / `RemoveMember` 对空数组、重复值的处理健壮；`UpdateUser` 指针字段的 nil/空区分明确。 |
| **DB 性能** | `UserList` 冗余二次查询（M-G）；`loadPerms` 已加 `p.status=1`；其他列表接口批量 IN 无 N+1。 |
| **僵尸代码** | `FindByPathPrefix`/`FindByParentId` 已清理；残留 `FindRoleIdsByUserId`（L-E）。`CountActiveAdmins` SQL 硬编码常量（M-F）。 |
| **接口契约** | `CreateUser` 注释 vs 实现不一致（L-A）；`UpdateUser` 与 `UpdateUserStatus` 职责重叠（L-J）。 |

### 修复优先级建议

**P0（立即修复）**
- **H-A**：`SetUserPerms` 加 `RequireProductAdminFor(productCode)` 或至少禁止自我 set。——普通 MEMBER 自我提权到产品全权限，属于可利用的权限越权，需优先拦截。
- **H-B**：`IncrementTokenVersion` 改为事务 + `LAST_INSERT_ID` / `FOR UPDATE` 获取真实的新版本号。——否则并发刷新会让正常用户偶发"刷新后即失效"，同时削弱 refresh rotation 安全性。

**P1（短期修复）**
- **M-A**：`RemoveMember` / `UpdateMember` 的"最后 ADMIN"校验挪进事务 + 加锁。
- **M-B**：`/auth/refreshToken` 加 userId 级限流，或在 logic 内部做 Redis 计数。
- **M-C**：`UsernameLoginLimit` key 增加 IP 维度，冻结账号不再消耗配额；可选加入 bcrypt 假耗时。
- **M-3**：`generateRandomHex` 修截断边界 + adminPassword 长度提升到 16。
- **M-4 / M-5**：`DeptTree` / `ProductList` / `ProductDetail` 增加归属过滤。
- **M-8**：对"收紧类"安全动作的缓存失败策略收紧，尤其是新加入的 `Logout`（失败必须返回 5xx）。
- **L-C**：`Logout` 加限流。

**P2（中期修复）**
- M-D：`DeleteRole` 把 `FindUserIdsByRoleId` 改为 session `FOR UPDATE`。
- M-E：所有 `Delete*ForProductTx` 的 SELECT 改为 session 内执行。
- M-F：`CountActiveAdmins` 用常量占位符替换硬编码。
- M-G：`UserList` 合并 JOIN 与 memberType 查询，去掉二次 IN。
- M-7：XFF + 可信代理白名单。
- L-B：JWT 中间件调整校验顺序（tokenVersion 前置）。
- L-A / L-J / L-F / L-G / L-H / L-I / L-K：按各项说明收敛。

**P3（长期）**
- L-1 / L-2（返回码选择、敏感配置明文）、L-7（`loader.Load` error 语义）、L-8（gRPC IP 提取）、L-9（`BindRoles` 空数组默认语义）、L-10（`FindOneByProductCodeUserId` 不按 status 过滤）保持跟进。