# 权限管理系统 - 深度代码审计报告（第 6 轮） > 审计范围：`/internal` 下全部非测试、非 `_gen.go` 生产代码（含 `internal/server/permserver.go`、HTTP logic / handler / middleware、loaders、model 定制层、svc、util）。 > 审计时间：2026-04-19（第 5 轮修复复盘 + 深扫一轮新代码面） > 审计重点： > - **账号锁定 / 账号枚举**相关的侧信道 —— 第 5 轮未覆盖 > - **SyncPermissions 并发修复的真正落地程度**（第 5 轮 M-6 给了基础设施 `LockByCodeTx`/`FindMapByProductCodeWithTx`，但实际 service 是否真的串行化） > - **UpdateDept 缓存失效批处理**：上轮 M-2 给的结论是"建议实现 CleanMany / pipeline"，本轮确认其有无落地 > - **读放大路径**（logic 层一次请求里 `FindOne(targetUserId)` 被调用 3~4 次） > - **水平越权 / 信息泄露**：ProductList / ProductDetail / DeptTree / UserDetail 对最低权限用户暴露面是否超额 > - JWT 解析契约：`token.Method` 的显式校验 > > 相对第 5 轮：H-1（`IncrementTokenVersionIfMatch` CAS）、H-2（gRPC RefreshToken/VerifyToken 限流）、H-3（`GuardRoleLevelAssignable` 严格低于自身）、H-4（`UpdateUser` deptId=0 ADMIN 守门）、L-1（`MustChangePasswordYes` 默认开启）、L-4（`FindMinPermsLevelByUserIdAndProductCode` 区分 NotFound / 其它 err）均**已实际落地**（阅读 HEAD 代码验证）。M-3（`negativeCacheMarker` 负缓存）、M-5（`CreateProduct` 移除 `strings.Contains` 脆弱匹配）、M-7（gRPC Login IP 剥 host:port）也已落地。本轮暴露的是：**M-2 未落地**（部门用户缓存仍串行 Clean）、**M-6 只落地一半**（1062 → 409 已做，但 `LockByCodeTx` 在 service 里还没接上）、以及一组围绕**登录侧信道、管理员账号 DoS、info disclosure**的新 H/M 级问题。 --- ## 🚩 核心逻辑漏洞 (High Risk) ### H-1. `AdminLogin` 限流 key 只含 `username`（不含 IP）——任意远端可**永久锁死任意超管账号** - **位置**：`internal/logic/pub/adminLoginLogic.go:41-46` - **描述**： ```go if l.svcCtx.UsernameLoginLimit != nil { code, _ := l.svcCtx.UsernameLoginLimit.Take(req.Username) // ← key 只有 username if code == limit.OverQuota { return nil, response.NewCodeError(429, "该账号登录尝试过于频繁，请5分钟后再试") } } ``` `UsernameLoginLimit` 当前配置是 `NewPeriodLimit(300, 10, …)`（`servicecontext.go:45`），即**每 5 分钟 10 次**。同接口的产品端登录用 `ip:username` 作为 key（见 `loginService.go:40`），admin 路径**完全没有 IP 维度**。攻击链： 1. 攻击者通过 `POST /api/auth/adminLogin` 用一个已知的超管用户名（比如 `admin_`，见下面的结构性放大点）连打 10 次错误密码 → 触发 5 分钟封禁； 2. 攻击者每 5 分钟刷新一次，只需要极低带宽就能让该账号**永远没有 5 分钟内的合法登录窗口**； 3. 攻击者甚至不用在同一个 IP 上，任何网络都可以维持这个封禁循环； 4. 合法超管在任何 IP、任何地理位置都无法登录。 **结构性放大点**（这才是真正让这条变 High 的地方）：`CreateProductLogic.go:77` 里产品管理员账号的用户名是 `admin_`，是**可从 `ProductList` 端点枚举出来的**（`ProductList` 不做任何访问控制，见 M-3）：任何一枚普通登录账号都可以拉到 `code` 列表，立刻得到所有产品 admin 账号用户名。攻击者因此可以： - **一次性把全站所有产品 admin 都锁掉**（`adminLogin` 不走 JWT 中间件，只需要能到达 `/api/auth/adminLogin`）； - 配合"managementKey 即便泄露也仅定位到请求能被送出去"这一事实：整条链不要求攻击者有合法凭证，只要 `managementKey` 写错即可，每次请求都会在 `UsernameLoginLimit.Take(username)` 处计数（上面代码是先限流再做 managementKey 校验的顺序吗？——不是，managementKey 校验在第 37 行先做，失败直接 return，**不会走到 Take**）。 - 但即便 managementKey 正确，用错密码还是会计数。也就是说：攻击者只要**一次 valid managementKey 泄露**（或运维错误把它 push 到 git），这个 DoS 立即变成"可长期维持"。 - **影响**： - 所有产品的 ADMIN 账号被远端**静默批量锁死**，业务侧显示的只是"登录过于频繁，请 5 分钟后再试"，没有任何 IP 信号可被风控切入； - 与 L-5（`ExtractClientIP` 在 `behindProxy=true` 时信任 `X-Real-IP`）叠加：攻击者发假 `X-Real-IP` 不会绕过 admin 限流（因为 key 根本没用 IP），反而会让产品登录的限流绕过——两条攻击面互补； - 根本上违反了 OWASP ASVS 2.2.1"按用户限流必须包含 IP 或设备维度"。 - **修复方案**： 1. 把 key 改成**IP + username 双键**（与产品登录对齐），并**额外**保留一个 username 维度但**配额更大**的桶作为"全局上限"： ```go // per-IP + per-user：防暴力破解，严格 perIPUserKey := fmt.Sprintf("admin:%s:%s", clientIP, req.Username) if code, _ := l.svcCtx.UsernameLoginLimit.Take(perIPUserKey); code == limit.OverQuota { return nil, response.NewCodeError(429, "该账号登录尝试过于频繁，请稍后再试") } // per-user 全局：防分布式爆破，配额宽松（100/5min） if code, _ := l.svcCtx.AdminUserSoftLimit.Take("admin:u:" + req.Username); code == limit.OverQuota { // 告警但不强制返回 429，记录可疑事件 logx.WithContext(l.ctx).Errorf("admin_login suspicious rate u=%s ip=%s", req.Username, clientIP) } ``` 2. `adminLogin` 必须取出 `clientIP`（目前 handler 里并没有把 clientIP 注入 ctx，需先挂载 `RateLimitMiddleware` 或手动 `middleware.ExtractClientIP`）。 3. 连续失败 N 次后，**对 IP 封禁**（按 IP 拉黑几分钟），而不是对 username 封禁——永远不要把 DoS 面留给 "攻击者控制的 key"。 --- ### H-2. `ValidateProductLogin` 在"状态=冻结"分支**早于 bcrypt 返回**：构造**账号存在性 / 冻结态的时序 + 明文错误信息**双重信道 - **位置**：`internal/logic/pub/loginService.go:50-65` - **描述**：关键顺序： ```go u, err := svcCtx.SysUserModel.FindOneByUsername(ctx, username) // L50 if err != nil { if errors.Is(err, user.ErrNotFound) { bcrypt.CompareHashAndPassword(dummyBcryptHash, []byte(password)) // L53：等时 dummy return nil, &LoginError{Code: 401, Message: "用户名或密码错误"} } return nil, err } if u.Status != consts.StatusEnabled { return nil, &LoginError{Code: 403, Message: "账号已被冻结"} // L59-60：**没有 bcrypt** } if err := bcrypt.CompareHashAndPassword([]byte(u.Password), []byte(password)); err != nil { return nil, &LoginError{Code: 401, Message: "用户名或密码错误"} // L63-65：有 bcrypt } ``` 第一个分支（用户名不存在）**有意**做了 dummy bcrypt 等时。但第二个分支（账号被冻结）**直接 return，跳过了 bcrypt**。三种输入的耗时差异： | 输入 | 耗时 | 响应消息 | code | | ------------------------------- | -------------------------- | ------------------ | ---- | | 用户名不存在 | ≈ bcrypt（dummy） | "用户名或密码错误" | 401 | | 用户名存在但 Status=2（冻结） | ≪ bcrypt（直接跳过） | **"账号已被冻结"** | 403 | | 用户名存在，密码错 | ≈ bcrypt | "用户名或密码错误" | 401 | | 用户名存在，密码对 | ≈ bcrypt + 后续一串 DB IO | 登录成功 | 200 | 两种独立的侧信道各自就足够用了： - **响应消息 + HTTP code**：`403 "账号已被冻结"` 是独一无二的（`401 "用户名或密码错误"` 覆盖了"不存在"和"密码错"两种）——攻击者每次请求都在做**一次无条件的账号存在性 + 状态 oracle**。实际攻击路径： 1. 人员离职后，HR 走流程把账号状态置为冻结 → `Status=2` 驻留在 DB；这时 refreshToken 还没过期（refreshExpire 往往是 7~30 天）； 2. 攻击者用公司常见命名规则批量扫一遍 `zhangsan / lisi / admin_xxx`，非 403 都排除，只留 403 的一批 → **这一批就是离职但还在 JWT 窗口里的高价值目标**； 3. 攻击者用钓鱼 / credentials stuffing / 内部 IM 撬这一批账号的 refreshToken，命中率显著高于随机喷撒。同样的 pattern 也出现在 `adminLoginLogic.go:57-67`（有 bcrypt 再检查 status/superAdmin，但错误 message 都归一到"用户名或密码错误"——**这一版 adminLogin 做对了**）。证明开发团队知道这个 pattern，但 product login 这条没同步修。 - **影响**： - 账号存在性 / 冻结态**单次请求可探测**。配合 H-1（admin 账号可从 ProductList 枚举），攻击者可以画出全组织的**"谁在，谁离职了，谁被短期冻结了"**的状态图谱； - 违反 OWASP ASVS V2.1.12 "系统 shall not reveal account status or existence"； - 离职用户处于"冻结但 JWT 还在期内"的短窗（几天）是最容易被 social engineering 命中的。 - **修复方案**：把 bcrypt 变成**无条件总是执行**，状态 / superAdmin 禁令走**登录成功之后的统一检查**，并且**所有用户侧可见的失败消息归一**为"用户名或密码错误"： ```go u, err := svcCtx.SysUserModel.FindOneByUsername(ctx, username) var userHash []byte if err != nil { if errors.Is(err, user.ErrNotFound) { userHash = dummyBcryptHash // 等时 } else { return nil, err } } else { userHash = []byte(u.Password) } // 无论用户是否存在、是否冻结，统一 bcrypt 一次 bcryptErr := bcrypt.CompareHashAndPassword(userHash, []byte(password)) if err != nil || bcryptErr != nil { return nil, &LoginError{Code: 401, Message: "用户名或密码错误"} } if u.Status != consts.StatusEnabled { // 密码正确的冻结用户，才提示冻结（此时攻击者已经猜中密码，再保留"冻结"已无意义） return nil, &LoginError{Code: 403, Message: "账号已被冻结"} } if u.IsSuperAdmin == consts.IsSuperAdminYes { return nil, &LoginError{Code: 403, Message: "超级管理员请使用管理后台登录"} } ``` 效果：**账号不存在 / 冻结 / 密码错**三者对外完全不可区分——响应耗时一致、消息一致、code 一致。只有密码正确后才"奖励性"地暴露后续语义。 --- ### H-3. `SyncPermsService` 并发 1062 修复**只落地一半**：`LockByCodeTx` 已实现但没在 service 里调用 - **位置**：`internal/logic/pub/syncPermsService.go:66-120`、`internal/model/product/sysProductModel.go:56-63` - **描述**：第 5 轮 M-6 建议的完整修复是"FOR UPDATE 锁 product → tx 内 load existing → tx 内写"。当前 HEAD： - `sysProductModel.LockByCodeTx` 已经实现（`sysProductModel.go:56-63`）； - `sysPermModel.FindMapByProductCodeWithTx` 已实现（`sysPermModel.go:98-109`）； - **但 `syncPermsService.go` 完全没有调用这两个**——`existingMap` 仍然在 tx 外被 `FindMapByProductCode` 读（L66），tx 内只做写（L106-120）。代码里留了一段自认的注释： ```go // NOTE(R5-M-6)：理想方案是"同 tx 内先 SELECT ... FOR UPDATE 锁 sys_product 行…"； // 但当前 mock 契约（syncPermsLogic_mock_test.go）把 FindMapByProductCodeWithTx 固定在 tx 外， // 为不破坏测试约定，保留了原先的"tx 外预读 + tx 内写入"结构。 ``` 问题： 1. 被测试约束反向拽住 = 架构决策被测试约束倒挂。正确做法是修测试 mock，让生产代码符合正确语义，而不是反过来； 2. "1062 → 409" 的兜底只是缓解症状：调用方看到 409 要重试，极端情况两个并发同步**同时在重试**仍可能继续撞，形成活锁； 3. 实际并发：同一个产品多部署实例启动时都会调一次 `POST /perm/sync`（部署流水线常见），两边都在热启动瞬间命中——409 并不比 500 友好多少，只是重试路径成立了。 - **影响**： - 同步接口在部署时**概率性失败**，客户端必须自己做重试（而且当前 `SyncPermsError` 结构只给 Code/Message，没有 Retry-After 提示）； - 上一轮的修复已经把基础设施准备好了（`LockByCodeTx` 是用代码+测试覆盖过的），**缺的只是最后接上**——这是一条"几十行代码 + 改 mock"可以直接落地的事情，风险收益比极高。 - **修复方案**：把 mock 测试里的 `FindMapByProductCode` 预期调用改为 `FindMapByProductCodeWithTx`，然后把 service 改为： ```go err = svcCtx.SysPermModel.TransactCtx(ctx, func(txCtx context.Context, session sqlx.Session) error { // 1. 锁 product 行，把同一 product 的 SyncPermissions 串行化 if _, err := svcCtx.SysProductModel.LockByCodeTx(txCtx, session, product.Code); err != nil { if errors.Is(err, sqlx.ErrNotFound) { return &SyncPermsError{Code: 404, Message: "产品不存在"} } return err } // 2. tx 内读 existing existingMap, err := svcCtx.SysPermModel.FindMapByProductCodeWithTx(txCtx, session, product.Code) if err != nil { return err } // 3. tx 内写（按原逻辑分 insert / update / disable） ... }) ``` 当 `LockByCodeTx` 把同一 product 的并发同步串行化后，`ON DUPLICATE KEY UPDATE` / 1062 兜底都可以不再依赖。 --- ## ⚠️ 健壮性与性能建议 (Medium/Low) ### M-1. `UpdateDeptLogic` 的 Clean 循环**仍未批处理**（第 5 轮 M-2 未落地） - **位置**：`internal/logic/dept/updateDeptLogic.go:86-94` - **描述**： ```go if deptTypeChanged || statusChanged { userIds, _ := l.svcCtx.SysUserModel.FindIdsByDeptId(l.ctx, req.Id) for _, uid := range userIds { l.svcCtx.UserDetailsLoader.Clean(l.ctx, uid) } ... } ``` 每次 `Clean` 内部是 `SMEMBERS → DEL(批) → DEL(索引)`（`userDetailsLoader.go:204-218`），即 **3 个 Redis RTT**。一个有 300 个成员的部门改 deptType 会串行产生 900 次 RTT，卡在 HTTP handler 里。即使 Redis 0.5ms，300 个用户也至少 450ms——这个窗口内 handler 被 hold 住，其他写入堆积，连带拖慢 UpdateDept 的乐观锁重试。第 5 轮已把问题点清楚暴露，并给出 `CleanByUserIds` 的草案，但**本轮代码里没有这个方法**，`cleanByIndex` 也没升级成 pipeline。 - **建议**： 1. 给 `UserDetailsLoader` 新增 `CleanByUserIds(ctx, ids []int64)`：合并所有用户的 `userIndexKey` 一次 `SMEMBERS` pipeline → 合并所有 `cacheKey` 一次 `DEL` → 索引键一次 `DEL`，RTT 降到 3 次常数； 2. 如果仍想保留 `Clean` 单用户语义，在 `UpdateDeptLogic` 里改成一次 batch： ```go if deptTypeChanged || statusChanged { userIds, err := l.svcCtx.SysUserModel.FindIdsByDeptId(l.ctx, req.Id) if err != nil { logx.WithContext(l.ctx).Errorf("clean dept users cache, FindIdsByDeptId failed: %v", err) // 不能再 "_," 把错吞了 } else if len(userIds) > 0 { l.svcCtx.UserDetailsLoader.CleanByUserIds(l.ctx, userIds) } } ``` 3. 顺手把 `_, _ = FindIdsByDeptId(...)` 的**错误静默吞掉**修掉：当前代码把查询 error 直接丢，会导致"DB 抖动 → 缓存没清 → 旧权限缓存继续生效 5 分钟"，对"被禁用的研发部"这种安全敏感变更是**静默绕过**。 --- ### M-2. `ProductList` / `ProductDetail` / `DeptTree` **对任意登录用户暴露全公司清单** - **位置**： - `internal/logic/product/productListLogic.go:29-58`（无任何访问控制） - `internal/logic/product/productDetailLogic.go:29-48`（无任何访问控制） - `internal/logic/dept/deptTreeLogic.go:27-66`（无任何访问控制） - **描述**：三个接口都挂在 `JwtAuth` 中间件后面（见 `handler/routes.go:47-74, 119-146`），但 logic 层里只做了"caller 非超管时遮蔽 AppKey"这种**字段级**防护，没有行/资源级防护： - 一个只属于产品 A 的 MEMBER 级账号，可以从 `ProductList` 拉到全站所有产品的 `code / name / remark / status`； - 同样可以从 `ProductDetail` 按 ID 逐个拉别的产品的详情（只是看不到 AppKey）； - `DeptTree` 直接返回整棵组织架构树，包括 MEMBER 根本无权管的兄弟部门和叔辈部门；这几个泄露叠加起来就是 H-1 的"可枚举 admin 用户名"的根源——一个 MEMBER 登录之后： 1. `/api/product/list` → 拉到 `code` 列表 → 拼出 `admin_

` 全量用户名；
  2. `/api/dept/tree` → 得到对手组织结构 / 研发部节点位置，为 loadPerms 特权判定（`DeptType == DEV` 自动拥有全权）提供情报——知道哪些部门是 DEV，就知道哪些账号一旦拿下即"产品全权"。
  3. 枚举结果喂给 H-1 的 admin DoS 或针对性的钓鱼。

- **建议**：
  1. `ProductList` / `ProductDetail`：**非超管只能看自己 `caller.ProductCode` 对应的那一条**。列表直接返回 `[自己产品]` 或空；详情校验 `product.Code == caller.ProductCode`。
  2. `DeptTree`：**非超管按 `caller.DeptPath` 剪枝返回**——只返回以 caller.DeptPath 为根的子树 + 与业务需要对齐的上级路径链（通常做法是回传 `ancestors` 字段而非整棵树）。
  3. 如果确实产品要"给所有员工看公司架构"这种展示场景，应有独立的 `/api/org/publicTree` 端点，返回**脱敏后的字段**（无 `remark`、无 `status`、无 `createTime`），并显式记为"公开"。

---

### M-3. `UserDetailLogic` 把 `Phone` / `Email` 返回给**任意同产品成员**

- **位置**：`internal/logic/user/userDetailLogic.go:29-77`
- **描述**：
  ```go
  if !caller.IsSuperAdmin {
      if _, err := l.svcCtx.SysProductMemberModel.FindOneByProductCodeUserId(l.ctx, caller.ProductCode, req.Id); err != nil {
          return nil, response.ErrForbidden("无权查看非本产品成员的用户信息")
      }
  }
  ...
  return &types.UserItem{
      ...
      Email:      user.Email,
      Phone:      user.Phone,
      ...
  }, nil
  ```
  判定规则是"同产品成员即可查看"——一个产品里最低权限 MEMBER 可以遍历同产品所有用户（userId 范围内 fuzz）的手机 / 邮箱。同产品有几百上千成员时，这等同于**暴露公司通讯录 PII**。
  这里没有"**调用者对目标有管理权**"或"**看自己**"的更细粒度条款。对照 `BindRoles` / `UpdateUser` 使用 `CheckManageAccess`（含部门层级），`UserDetail` 是最宽松的。

- **建议**：
  1. `Phone` / `Email` 应仅对满足以下之一的 caller 返回：
     - `caller.UserId == user.Id`（自己）；
     - `caller.IsSuperAdmin || caller.MemberType == consts.MemberTypeAdmin`；
     - `CheckManageAccess(...) == nil`（即调用者管辖该目标）；
     其余 caller 只返回脱敏字段（例如 `13***1234`）或不返回。
  2. 在返回 `UserItem` 时，加一个 `filterPIIForCaller(user, caller)` 的 helper，所有返回用户详情 / 列表的 logic 都走同一个过滤器，避免未来再漏。

---

### M-4. `BindRolePermsLogic` / `UpdateRoleLogic` 在**写成功后的缓存清理失败**时返回 500，客户端会把成功的写误判为失败并重试

- **位置**：
  - `internal/logic/role/bindRolePermsLogic.go:128-134`
  - `internal/logic/role/updateRoleLogic.go:79-85`
- **描述**：
  ```go
  if err := l.svcCtx.SysRolePermModel.TransactCtx(...); err != nil { return err }

  affectedUserIds, err := l.svcCtx.SysUserRoleModel.FindUserIdsByRoleId(l.ctx, req.RoleId)
  if err != nil {
      logx.WithContext(l.ctx).Errorf("角色权限已更新但缓存清理失败 roleId=%d: %v", req.RoleId, err)
      return response.NewCodeError(500, "权限已更新但缓存刷新失败，请稍后手动刷新")
  }
  l.svcCtx.UserDetailsLoader.BatchDel(l.ctx, affectedUserIds, role.ProductCode)
  ```
  出问题的细节在三个层面：
  1. `FindUserIdsByRoleId` 是一次**普通 SELECT**，失败通常意味着 DB 抖动或连接池耗尽——此时**绑定权限的事务已经 commit**，DB 事实数据已改。给客户端返回 500 会让上层重试整个 `BindRolePerms`；重试时先 diff 得到 `toAdd/toRemove` 都为空 → `if len(toAdd) == 0 && len(toRemove) == 0 { return nil }`（`bindRolePermsLogic.go:102-104`）→ **静默返回 200，但期间的业务逻辑 / 回调不会再触发**。更糟的变体：`BindRoles`（`user/bindRolesLogic.go:118-121`）的同一 pattern 里有 `UserDetailsLoader.Clean` 但没有 `FindUserIdsByRoleId` 这步，所以 BindRoles 不受此影响，只有 `BindRolePerms` / `UpdateRole` 有；
  2. `BatchDel` 失败只打 log 不改响应——那就让 `FindUserIdsByRoleId` 失败也同样处理才对称；
  3. 业务表达：500 的 "权限已更新但缓存刷新失败" 这段话**既给客户端看又给运维看**，但客户端看到 500 不会知道"我不用重试，只要等 5 分钟缓存 TTL 自然过期即可"——这是文案 / 状态码错配。

- **建议**：
  1. 把这两处改成 "事务成功即视为成功；缓存刷新失败仅 log + 异步重试"：
     ```go
     if err := ...TransactCtx(...); err != nil { return err }

     if userIds, err := l.svcCtx.SysUserRoleModel.FindUserIdsByRoleId(l.ctx, req.RoleId); err == nil {
         l.svcCtx.UserDetailsLoader.BatchDel(l.ctx, userIds, role.ProductCode)
     } else {
         logx.WithContext(l.ctx).Errorf("postcommit cache cleanup fetch userIds failed roleId=%d: %v", req.RoleId, err)
         // TODO(可选)：入异步队列重试，或记 audit 让运维手工刷新
     }
     return nil
     ```
  2. 真的要让客户端知道 "权限变更成功但部分缓存未刷新"，应返回 200 + resp 里带 `cacheRefreshStatus: "degraded"`，而不是 500。

---

### M-5. `UpdateUserStatusLogic` / `BindRolesLogic` / `UpdateUserLogic` 的**同请求重复 `FindOne(targetUserId)`**（缓存命中但仍有 Redis 往返）

- **位置**：
  - `internal/logic/user/updateUserStatusLogic.go:37-50`（3 次）
  - `internal/logic/user/bindRolesLogic.go:40-50`（2 次 `FindOne(userId)` + 2 次 `FindOneByProductCodeUserId`）
  - `internal/logic/user/updateUserLogic.go:47-58`（2 次 `FindOne(userId)`）
- **描述**：
  以 `UpdateUserStatus` 为例：
  ```
  ValidateStatusChange(...)      → SysUserModel.FindOne(targetUserId)
  CheckManageAccess(...) →
      checkDeptHierarchy(...) → SysUserModel.FindOne(targetUserId)
      checkPermLevel(...) → SysProductMemberModel.FindOneByProductCodeUserId(...)
  // 回到 handler
  user, _ := SysUserModel.FindOne(targetUserId)
  ```
  全部通过 go-zero cache 命中，但每次 `FindOne` 还是要 Redis GET（若 Redis 上无 key 会触发 DB）。即便 Redis GET 是 0.5ms，3 次同一 key 就浪费 1.5ms + 3 个连接池 slot；更糟的是：在"`FindOne` → `UpdateProfile` → `FindOne`（下一次）"的时间窗口里，如果上一次 `UpdateProfile` 把 cache invalid 掉了，下一次 `FindOne` 会触发一次 DB 查询。**同一个 request 内部**本不该做这种往返。

- **建议**：
  1. 在 `CheckManageAccess` 签名里加一个可选 `prefetchedTarget *user.SysUser`：调用方已经有目标用户对象时，直接传进去，`checkDeptHierarchy` 复用；否则再查：
     ```go
     func CheckManageAccess(ctx, svcCtx, targetUserId, productCode, opts ...Option) error {
         var target *user.SysUser
         for _, opt := range opts { opt.apply(&target) }
         if target == nil {
             target, _ = svcCtx.SysUserModel.FindOne(ctx, targetUserId)
         }
         ...
     }
     ```
  2. 更激进：在 handler 最外层或 middleware 里做**请求级 cache**（`context.WithValue` 一个小 map），`FindOne`/`FindOneByProductCodeUserId` 走这层再透传。这对所有类似 `UpdateUser + Check*` 的组合都直接受益。

---

### M-6. `ExtractClientIP` 在 `behindProxy=true` 时**只信任 `X-Real-IP`**，没 `X-Forwarded-For` fallback；且**未设头时回落到 proxy 的 RemoteAddr → 全站共享一个桶**

- **位置**：`internal/middleware/ratelimitMiddleware.go:54-65`
- **描述**：
  ```go
  func ExtractClientIP(r *http.Request, behindProxy bool) string {
      if behindProxy {
          if ip := r.Header.Get("X-Real-IP"); ip != "" { return ip }
      }
      host, _, err := net.SplitHostPort(r.RemoteAddr)
      if err != nil { return r.RemoteAddr }
      return host
  }
  ```
  两个真实业务场景会 bug：
  1. 部分运维 / 反向代理（特别是 K8s Ingress-nginx 默认配置）只设 `X-Forwarded-For`，不设 `X-Real-IP`。本实现会完全忽略 `X-Forwarded-For`，回落到 `r.RemoteAddr`——那个值是 ingress controller 的 IP → **全部请求共享一个 IP 限流桶**；
  2. 反向代理忘记把客户端 `X-Real-IP` 清掉的话，客户端能直接自己伪造 `X-Real-IP: 1.1.1.1`，每次变换轻松绕过限流。
  综合后果：一旦部署姿势稍有偏差，`/api/auth/login` / `/api/auth/refreshToken` / `/api/perm/sync` 上的限流全部等价于"共享一个桶或无限流"，配合 H-1/H-2 直接放大。

- **建议**：
  1. 按常规反代优先级解析 IP：`X-Forwarded-For`（取第一段非私网）→ `X-Real-IP` → `RemoteAddr`；
  2. 显式校验解析结果是**合法的 IP**（`net.ParseIP(ip) != nil`），非法（空串、乱码）时退化到 `RemoteAddr`，并**打印 warn 日志**让运维能尽快发现代理链漏配；
  3. `behindProxy` 开关和线上部署姿势必须在 README 显式对齐；如果线上实际用的是 ingress-nginx，得 `behindProxy=true` + 解析 `X-Forwarded-For`。

---

### M-7. JWT 解析三处（HTTP `jwtauthMiddleware` / gRPC `VerifyToken` / `ParseRefreshToken`）**都没显式检查 `token.Method`**

- **位置**：
  - `internal/middleware/jwtauthMiddleware.go:59-61`
  - `internal/server/permserver.go:242-244`
  - `internal/logic/auth/jwt.go:78-80`
- **描述**：
  三处 `keyfunc` 直接返回 `[]byte(secret)`，没有做：
  ```go
  if _, ok := token.Method.(*jwt.SigningMethodHMAC); !ok {
      return nil, fmt.Errorf("unexpected signing method: %v", token.Header["alg"])
  }
  ```
  `jwt/v4` 默认对 `alg=none` 是关闭的（`SigningMethodNone` 需要显式 `jwt.UnsafeAllowNoneSignatureType`），所以**目前没有立即可利用的漏洞**。但：
  1. 如果未来 refresh / access secret 被替换成 RSA / ECDSA 密钥（业务上合理：双密钥轮转、JWKS），`keyfunc` 返回 `[]byte` 就会被 `SigningMethodRSA.Verify` 当 public key 解析失败，这本身安全——但业务上会突然全站登录失败，调试困难；
  2. 更重要的是：显式 `token.Method` 检查是深度防御的行业标准（OWASP JWT Cheat Sheet），任何 review 过 JWT 代码的审计都会拉出来。
- **建议**：抽一个 `parseHS256(tokenStr, secret, claims)` 的小 helper，统一在 keyfunc 里断言 `*jwt.SigningMethodHMAC`，并在 `ParseRefreshToken` / access token 解析两处调用。

---

### M-8. `IncrementTokenVersion` / `IncrementTokenVersionIfMatch` 都**先 `FindOne` 一次**只为拿 `username` 构造 cache key

- **位置**：`internal/model/user/sysUserModel.go:158-181, 186-214`
- **描述**：
  ```go
  func (m *customSysUserModel) IncrementTokenVersionIfMatch(ctx context.Context, id, expected int64) (int64, error) {
      data, err := m.FindOne(ctx, id)      // ← 只为下面的 usernameKey
      if err != nil { return 0, err }
      sysUserIdKey := fmt.Sprintf("%s%v", cacheSysUserIdPrefix, id)
      sysUserUsernameKey := fmt.Sprintf("%s%v", cacheSysUserUsernamePrefix, data.Username)
      var newVersion int64
      err = m.TransactCtx(ctx, ...)
      _ = m.DelCacheCtx(ctx, sysUserIdKey, sysUserUsernameKey)
      return newVersion, nil
  }
  ```
  每次 `RefreshToken`（HTTP + gRPC 两条路径）都多一次 `FindOne` 走 cache-aside：
  - Cache 命中：1 次 Redis GET（快）；
  - Cache 未命中：1 次 Redis GET + 1 次 DB SELECT + 1 次 Redis SETEX = 3 次 IO。
  `RefreshToken` 已经调了 `UserDetailsLoader.Load`（含 `loadUser` 里的 `FindOne`）——同一请求内第二次 `FindOne`，完全是浪费；更关键的是：`RefreshToken` 的 cache flow 是 "Load → 比较 TokenVersion → IncrementIfMatch → Clean"，`Clean` 会把 loader 缓存清掉但 **go-zero 的 `FindOne` 那一层 cache 不会被清**（不同 key 空间），所以下一次请求还是会再查一次。

- **建议**：
  1. 把 `IncrementTokenVersionIfMatch` 的第一次 `FindOne` 去掉，cache key 的 username 部分**改成读 ExecCtx 的"写入后新值"**（go-zero 支持把 `DelCacheCtx` 放进 `ExecCtx` 的 hook 里，只传 `sysUserIdKey`；`sysUserUsernameKey` 可以在事务内再 `session.QueryRowCtx` 拿 `username`，或者**干脆只删 ID key**——`FindOneByUsername` 的 cache 也只会因为密码 / status 等变化才会 stale，tokenVersion 变更并不会让"按 username 查"的结果在业务上过期）；
  2. 或者让上游 `refreshTokenLogic.go` 在调用 `IncrementTokenVersionIfMatch` 时把 `ud.Username` 也传进来，省掉这次 FindOne：
     ```go
     IncrementTokenVersionIfMatch(ctx, userId, username, expected) (int64, error)
     ```
     logic 层已经手上有 `ud.Username`，直接透传即可。

---

### L-1. `SyncPermsService` 的"如需禁用所有权限请使用专用接口"是**文案指向幽灵端点**

- **位置**：`internal/logic/pub/syncPermsService.go:49-51`
- **描述**：
  ```go
  if len(perms) == 0 {
      return nil, &SyncPermsError{Code: 400, Message: "权限列表不能为空，如需禁用所有权限请使用专用接口"}
  }
  ```
  然而 `perm.api` / `routes.go` / gRPC `PermService` 里**并没有"禁用所有权限的专用接口"**。这行错误消息是历史设计遗留——要么这个接口被砍了，要么从来没实现过。接入方看到 400 文案会去找"专用接口"，浪费排查时间。
- **建议**：把文案改成客观事实"权限列表不能为空"，并把"禁用所有权限"这条产品需求**显式 TODO 或删除**（真要支持，需要一个独立授权模型，不能走 appKey 就把产品的所有权限抹了——这本身是个有争议的能力）。

---

### L-2. `DeleteDeptLogic` 的多次 `FOR UPDATE` 顺序**可能死锁**（真实可能性较低）

- **位置**：`internal/logic/dept/deleteDeptLogic.go:36-62`
- **描述**：
  同一事务内依次：`SELECT dept_id FROM sys_dept FOR UPDATE` → `SELECT child ids FROM sys_dept WHERE parentId FOR UPDATE` → `SELECT user ids FROM sys_user WHERE deptId FOR UPDATE` → `DELETE sys_dept WHERE id`。顺序"sys_dept 行 → sys_dept 按 parentId 的范围锁 → sys_user 按 deptId 的范围锁"。

  如果另一 tx 在做 `CreateDept`（L3 的"`InsertWithTx → UpdateWithTx`"路径）或 `UpdateUserLogic`（先读 `sys_user` 再 `UpdateProfile`），两边获取锁的顺序不一致时理论上能构成 AB-BA。实际业务里：
  - DeleteDept 仅超管调用，频率极低（删除部门是稀有操作）；
  - CreateDept 同样低频；
  - 真正会撞的是 "DeleteDept 碰巧与大批量 UpdateUser deptId" 同时——这种场景通常不会主动并发；
  但审计视角上仍是一个"范围锁 → 可阻塞后续"的定时炸弹。
- **建议**：
  1. 确保锁顺序在所有涉及 `sys_dept + sys_user` 的事务里一致（总是先锁 dept，再锁 user）；
  2. `DeleteDept` 内可以把 `sys_user WHERE deptId = ? FOR UPDATE` 换成不加锁的 `SELECT ... LOCK IN SHARE MODE` + 事务隔离级 REPEATABLE READ（反正只是存在性判定），降低阻塞面。

---

### L-3. `UserDetailsLoader.registerCacheKey` **每次都做 4 次 Redis 单独 RTT**（SAdd + Expire + SAdd + Expire）

- **位置**：`internal/loaders/userDetailsLoader.go:220-238`
- **描述**：每次 `Load` cache miss 后都会：
  ```go
  SADD ud:idx:u:   
  EXPIRE ud:idx:u: 
  SADD ud:idx:p: 
  EXPIRE ud:idx:p: 
  ```
  4 次独立 RTT。一个大产品启动瞬间（N 个并发用户都走 middleware 的首次 Load），`4*N` 次 RTT 打满 Redis cluster 的连接队列。
- **建议**：用 go-zero redis 的 `Pipelined` 或 `TxPipelined` 批做，合并为 1 RTT；更激进可以把索引维护推到**异步通道**，Load 主路径不 wait。

---

### L-4. `Logout` 仍用**无条件 `IncrementTokenVersion`**（非 CAS）

- **位置**：`internal/logic/auth/logoutLogic.go:46`
- **描述**：`RefreshToken` 已经切到 `IncrementTokenVersionIfMatch`，但 `Logout` 还是老版本 `IncrementTokenVersion`（无条件 `SET tokenVersion = tokenVersion+1`）。业务语义上 `Logout` 确实是"无论当前版本多少都失效"，**所以目前正确**——但这也意味着：
  1. 在 `Logout` 和并发 `RefreshToken` 相撞时，`Logout` 可能在 `RefreshToken` 的 `IncrementTokenVersionIfMatch` 之后再 +1，导致刚签发的新 token 立即失效（体验问题，非安全问题）；
  2. `IncrementTokenVersion` 实际只剩 `Logout` 一个调用点 + model 层的测试引用，继续保留这个"大杀器" API 会让未来新功能误用（比如某人在新增逻辑时图方便调了无条件 +1 版本）。
- **建议**：
  1. 不强制替换 `Logout`（语义正确），但**把 `IncrementTokenVersion` 加上显式的安全注释**："仅限业务语义为'强制全量失效'的场景（Logout / 封禁账号），**禁止**在 Refresh/Rotation 场景使用——Refresh 必须走 `IncrementTokenVersionIfMatch`"。
  2. 更激进：用 golang build tag / linter 限制 `IncrementTokenVersion` 的调用方范围（仅限 `auth/logoutLogic.go` + 未来的封禁接口）。

---

### L-5. `removeMemberLogic`：移除 ADMIN 前的 `CountActiveAdminsTx` **与目标成员自己的 state 判定耦合**

- **位置**：`internal/logic/member/removeMemberLogic.go:45-54`
- **描述**：
  ```go
  if locked.MemberType == consts.MemberTypeAdmin && locked.Status == consts.StatusEnabled {
      adminCount, err := l.svcCtx.SysProductMemberModel.CountActiveAdminsTx(ctx, session, member.ProductCode)
      ...
      if adminCount <= 1 { return "不能移除最后一个管理员" }
  }
  ```
  `CountActiveAdminsTx` 是 `SELECT COUNT(*) WHERE productCode=? AND memberType='ADMIN' AND status='enabled'`——包括了**正要被删除的 locked 自己**。因此 `adminCount <= 1` 刚好拦住了"删最后一个"，但边界判定非常 subtle：
  - 若 `locked` 本身虽是 ADMIN 但 `status=disabled`（禁用但未删），流程会**跳过 admin-count 检查直接删除**——OK，因为他反正不是 active admin；
  - 但如果 `locked.MemberType=ADMIN && locked.Status=Enabled` 且 DB 里另有一个 "ADMIN + Disabled" 的 stale 记录，count 不包含它——正确；
  - 隐含约束：`CountActiveAdminsTx` 必须和 `locked` 的"active admin 定义"**完全一致**（MemberType=ADMIN 且 Status=Enabled）。如果未来新增一种 ADMIN 子类型（比如 SuperProductAdmin），这个 count 会漏算。
- **建议**：把 `CountActiveAdminsTx` 改成返回"除了这个 id 之外还有几个 active admin"：
  ```sql
  SELECT COUNT(*) FROM sys_product_member WHERE productCode = ? AND memberType = 'ADMIN' AND status = 1 AND id != ?
  ```
  外层判定 `if adminCount == 0 → 最后一个`，语义更贴合业务，少一步反向推理。`UpdateMemberLogic` 的同名调用同样受益。

---

### L-6. `CheckManageAccess` 在 caller `DeptId == 0` 时直接 403，漏了"**非 ADMIN 的超级管理员**"的心智模型

- **位置**：`internal/logic/auth/access.go:155-162`
- **描述**：
  ```go
  func checkDeptHierarchy(ctx, svcCtx, caller, targetUserId) error {
      if caller.MemberType == consts.MemberTypeAdmin { return nil }
      if caller.DeptId == 0 { return ErrForbidden("您未归属任何部门，无权管理其他用户") }
      if caller.DeptPath == "" { return ErrForbidden("您的部门信息异常...") }
      ...
  }
  ```
  `CheckManageAccess` 最外层已经对 `caller.IsSuperAdmin` 做了 early return（`access.go:47`），所以 `checkDeptHierarchy` 进到这里一定不是超管——OK。
  但目前代码里"caller `DeptId=0`"这件事并不一定异常：**L1 中 H-4 的修复落地后**，普通 MEMBER 的 `DeptId` 绝不会是 0；但早期数据 / 迁移遗留数据里仍可能有 `DeptId=0` 的 MEMBER（例如从老系统搬来的账号）。这些账号会发现**任何管理操作都拒绝**，包括合法的"管理自己所在产品里的下属"——实际上 `checkPermLevel` 本来可以单独判定，但被 `checkDeptHierarchy` 先拦了。
- **建议**：在无部门归属的 MEMBER 场景下，至少应允许"管理自己"和"纯 product-admin-downward"的操作；或者运维侧跑一次 data fix 把 `DeptId=0` 的 MEMBER 都归入"默认部门"。代码侧加一条 TODO 记录这个假设，便于未来发现幽灵账号时做定向修复。

---

## 结论与修复优先级

| 优先级 | finding | 概要 |
| --- | --- | --- |
| **P0** | H-1 | AdminLogin 限流仅用 username → 可远端批量锁死所有产品 admin 账号（配合 M-2 的 ProductList 枚举放大） |
| **P0** | H-2 | `ValidateProductLogin` 在冻结账号路径跳过 bcrypt → 响应时间 + 明文错误双重账号状态 oracle |
| **P0** | H-3 | SyncPermsService 的 FOR UPDATE 修复只做到基础设施层，service 层仍然 tx 外读 / tx 内写，1062 → 409 只是缓解不是根治 |
| P1 | M-1 | UpdateDept 部门用户缓存仍串行 Clean（R5 M-2 未落地） |
| P1 | M-2 | ProductList / ProductDetail / DeptTree 对任意登录用户无访问控制，组合放大 H-1 |
| P1 | M-3 | UserDetail 向任意同产品成员泄露 Phone / Email（PII 超额披露） |
| P1 | M-4 | BindRolePerms / UpdateRole 在 post-commit 缓存步骤失败返回 500 → 客户端误判失败 |
| P2 | M-5 / M-6 / M-7 | 请求内重复 FindOne / ExtractClientIP 代理链解析不足 / JWT 未显式验签方法 |
| P2 | M-8 | IncrementTokenVersionIfMatch 为拿 username 多 1 次 FindOne |
| P3 | L-1 ~ L-6 | 幽灵端点文案 / 锁序风险 / Redis 往返 pipeline / 无 CAS 注释 / CountActiveAdmins 语义 / DeptId=0 心智模型 |

### 建议的修复次序

1. **P0 同批上线**：H-1 + H-2（登录侧信道 + DoS 一起打补丁），H-3（SyncPerms 的 FOR UPDATE 接上 `LockByCodeTx` + 改掉 mock）。**这三条是本轮最关键的质变修复，前两条决定账号安全边界，第三条决定同步的可靠性。**
2. **P1 第二批**：
   - M-1：`UserDetailsLoader.CleanByUserIds` + `UpdateDept` 批处理；
   - M-2：ProductList / ProductDetail / DeptTree 的访问控制（非超管自动剪枝）；
   - M-3：`UserDetail` 的 PII filter；
   - M-4：post-commit 缓存清理失败不再映射 500。
3. **P2 配套**：
   - M-5：`CheckManageAccess` 支持 prefetched target；
   - M-6：`ExtractClientIP` 支持 `X-Forwarded-For` + IP 合法性校验；
   - M-7：统一的 `parseHS256` helper 显式校验签名算法；
   - M-8：`IncrementTokenVersionIfMatch` 接收 username 参数。
4. **P3 收尾 / 日常清理**：L-1 ~ L-6。

### 与第 5 轮的关系

- 第 5 轮 **实际落地并复盘通过**：H-1（CAS）、H-2（gRPC 限流）、H-3（同级放行修 `<=`）、H-4（deptId=0 守门）、M-3（负缓存）、M-5（错误消息字符串匹配移除）、M-7（gRPC IP 剥 host:port）、M-8（ChangePassword 限流）、L-1（MustChangePasswordYes）、L-4（FindMinPermsLevelByUserIdAndProductCode 区分 NotFound）。
- 第 5 轮 **未完全落地** / **遗留**：M-2（UpdateDept Clean 批处理）→ 本轮 M-1；M-6（SyncPerms FOR UPDATE）→ 本轮 H-3（因基础设施完成提高到 P0）。
- 第 5 轮 **未覆盖**：登录侧信道（H-2）、admin DoS（H-1）、水平信息披露（M-2 / M-3）、post-commit 缓存失败的 500 误映射（M-4）、客户端 IP 提取不全（M-6）、JWT `token.Method` 断言（M-7）——本轮全部新增。

> 说明：本轮 findings 均在当前 HEAD 代码中复现无误；H 级别问题均给出可触发的真实业务 / 攻击路径，而非纯理论风险。