审计对象:perms-system/server(不含测试代码)
审计维度:逻辑一致性、并发/竞态、资源管理、数据完整性、安全漏洞、边界、DB 性能、僵尸代码、接口契约
说明:本轮基于真实业务量级(数千用户 / 数十产品 / 单产品 <100 role / 一次 SyncPerms < 1k perm / 单 user 10~30 role)做判定。对前 10 轮已闭环条目(H-1~H-4、M-1~M-R10-5、L-1~L-R10-10 等)不重复列举,仅追踪本轮新发现或重新归类的风险点。
UpdatePassword 内部 FindOne 把"外层校验过的状态"洗掉,乐观锁自我对齐 → TOCTOU描述:internal/model/user/sysUserModel.go:128-152 的 UpdatePassword 不接受外部 expectedUpdateTime,而是在内部自己 FindOne 再取 data.UpdateTime 作为乐观锁 WHERE:
func (m *customSysUserModel) UpdatePassword(ctx context.Context, id int64, password string, mustChangePassword int64) error {
data, err := m.FindOne(ctx, id)
if err != nil {
return err
}
sysUserIdKey := fmt.Sprintf("%s%v", cacheSysUserIdPrefix, id)
sysUserUsernameKey := fmt.Sprintf("%s%v", cacheSysUserUsernamePrefix, data.Username)
// 乐观锁:WHERE 叠加 updateTime 与 FindOne 拿到的一致。避免 FindOne → Exec 之间并发改密把
// 本次写盖成"最后一写赢"、或目标行被删除后仍返回成功造成语义欺骗(见审计 M-2)。
expectedUpdateTime := data.UpdateTime
res, err := m.ExecCtx(ctx, func(ctx context.Context, conn sqlx.SqlConn) (sql.Result, error) {
query := fmt.Sprintf("UPDATE %s SET `password` = ?, `mustChangePassword` = ?, `tokenVersion` = `tokenVersion` + 1, `updateTime` = ? WHERE `id` = ? AND `updateTime` = ?", m.table)
return conn.ExecCtx(ctx, query, password, mustChangePassword, time.Now().Unix(), id, expectedUpdateTime)
}, sysUserIdKey, sysUserUsernameKey)
调用方 ChangePasswordLogic(internal/logic/auth/changePasswordLogic.go:50-81)早已经自己 FindOne 读到 user、校验 user.Password + user.Status != Enabled,然后把 userId 透传进来。此处 UpdatePassword 又打一次 FindOne,内层 CAS 对齐的是内层 FindOne 的时间戳——而不是外层校验旧密码所依赖的那个时间戳。
真实并发场景(两个并存会话):
T0: Device A 发起改密 (oldPass=P0, newPass=P1)
ChangePasswordLogic.FindOne → user{password=H(P0), updateTime=T0}
bcrypt.CompareHashAndPassword(H(P0), P0) → OK
bcrypt.GenerateFromPassword(P1) → H(P1)
T1: Device B 独立完成改密到 P2
UpdatePassword: FindOne → user{updateTime=T0} → UPDATE ... WHERE updateTime=T0
提交成功:password=H(P2), updateTime=T1, tokenVersion+1
T2: Device A 的 UpdatePassword 开始执行
内部 FindOne → user{updateTime=T1, password=H(P2)} ← 被 B 的写"刷新"
expectedUpdateTime=T1
UPDATE ... WHERE updateTime=T1 → 匹配,提交成功
最终 DB:password=H(P1), Device B 的新密码 P2 被覆盖
等价结论:内层"自 FindOne-自 Update"的 CAS 等于没有 CAS。调用方看似"带乐观锁",实际语义已退化为 last-write-wins on password,而且连"外层校验的旧密码还是有效旧密码"都不再成立(A 验证的是 P0,应用出去的是把 P2 改回到 P1)。
这条 TOCTOU 并非纸面理论:
此外:UpdatePassword 里 tokenVersion = tokenVersion + 1 是累计的,所以两次成功的 UPDATE 会连续 +2,把刚刚因 B 改密正准备下线的 A 的旧 token(version 已经对不上)再把 B 的所有新会话也踢掉,用户本次密码修改后的新登录也会被强制登出。
影响:
修复方案:把 expectedUpdateTime 改由调用方显式传入,彻底消除内部二次 FindOne 造成的"自对齐":
// sysUserModel.go
UpdatePassword(ctx context.Context, id int64, username, password string,
mustChangePassword, expectedUpdateTime int64) error
func (m *customSysUserModel) UpdatePassword(ctx context.Context, id int64,
username, password string, mustChangePassword, expectedUpdateTime int64) error {
sysUserIdKey := fmt.Sprintf("%s%v", cacheSysUserIdPrefix, id)
sysUserUsernameKey := fmt.Sprintf("%s%v", cacheSysUserUsernamePrefix, username)
res, err := m.ExecCtx(ctx, func(ctx context.Context, conn sqlx.SqlConn) (sql.Result, error) {
query := fmt.Sprintf(
"UPDATE %s SET `password` = ?, `mustChangePassword` = ?, `tokenVersion` = `tokenVersion` + 1, `updateTime` = ? WHERE `id` = ? AND `updateTime` = ?",
m.table)
return conn.ExecCtx(ctx, query, password, mustChangePassword, time.Now().Unix(), id, expectedUpdateTime)
}, sysUserIdKey, sysUserUsernameKey)
...
}
调用方 ChangePasswordLogic 把已经持有的 user.UpdateTime / user.Username 透传(与 IncrementTokenVersionIfMatch(id, username, expected) 的签名风格对齐)。这样 CAS 的 expected 就是"外层用来校验旧密码的那一份快照";只要 DB 里 updateTime 发生过任何变化(并发改密 / 改资料 / 冻结/解冻),都会 CAS 失败返回 ErrUpdateConflict,调用方按 409 映射返回"密码已被其他会话修改,请刷新后重试",并提示用户重新登录确认。
同时收益:
FindOne 不再浪费(以前只用于校验旧密码,然后内层再打一次);AdminResetPassword 之类未来想复用本方法的路径,也必须显式承诺"基于某个观察到的 updateTime 改",语义更清晰。SyncPermissions / GetUserPerms 未挂 gRPC 入口限流描述:HTTP 侧 /api/perm/sync 已挂 serverCtx.SyncRateLimit(internal/handler/routes.go:195-206),而 gRPC 的 PermServer.SyncPermissions(internal/server/permserver.go:60-92)与 PermServer.GetUserPerms(:331-369)既不做 IP 维度限流、也不做按 appKey 维度的限流:
func (s *PermServer) SyncPermissions(ctx context.Context, req *pb.SyncPermissionsReq) (*pb.SyncPermissionsResp, error) {
items := make([]pub.SyncPermItem, len(req.Perms))
for i, p := range req.Perms {
items[i] = pub.SyncPermItem{Code: p.Code, Name: p.Name, Remark: p.Remark}
}
result, err := pub.ExecuteSyncPerms(ctx, s.svcCtx, req.AppKey, req.AppSecret, items)
Login / RefreshToken / VerifyToken 都各自持有 GrpcLoginLimiter / GrpcRefreshLimiter / GrpcVerifyLimiter,口径完整;唯独"产品服务端调用"这两个接口是裸调。
影响:
SyncPermissions 内部要走"tx + LockByCodeTx 的 X 锁";appSecret 一旦泄露,恶意方在没有限流兜底时可持续对同一 product 打高频同步请求,LockByCodeTx 会串行化但前置 bcrypt.Compare(appSecret) 的 CPU 开销(cost=10 默认 ~100ms)与 DB 短事务都会被放大,单点产品同步的尾延迟会被显著拉高。GetUserPerms 会触发 UserDetailsLoader.Load,缓存未命中时回源多张表;同样的泄露凭证 + 枚举 userId 可打爆 DB。本条未能被"HTTP 层 SyncRateLimit"兜住,因为 gRPC 是独立监听端口(不同服务进程入口)。
修复:
servicecontext.go 增设 GrpcSyncLimiter / GrpcGetUserPermsLimiter(配额取决于真实产品数和 QPS,例如单 product 每分钟 60 次同步 / 1k 次 perm 查询),按 fmt.Sprintf("grpc:sync:%s", req.AppKey) / fmt.Sprintf("grpc:perms:%s", req.AppKey) 为 key,避免按 IP(产品后端多实例共享 egress IP 时会误伤);GetUserPerms 可以同时叠加按 IP 维度,防止同一合法产品多个后端实例在 DDoS 场景下集体被耗尽配额。UpdateStatus / IncrementTokenVersion 只为构造 cache key 而多打一次 FindOne描述:与 H-R11-1 同属"内部 FindOne 冗余"族,但这两处只影响性能而不破坏正确性:
func (m *customSysUserModel) UpdateStatus(ctx context.Context, id int64, status int64, expectedUpdateTime int64) error {
data, err := m.FindOne(ctx, id)
if err != nil {
return err
}
sysUserIdKey := fmt.Sprintf("%s%v", cacheSysUserIdPrefix, id)
sysUserUsernameKey := fmt.Sprintf("%s%v", cacheSysUserUsernamePrefix, data.Username)
...
func (m *customSysUserModel) IncrementTokenVersion(ctx context.Context, id int64) (int64, error) {
data, err := m.FindOne(ctx, id)
if err != nil {
return 0, err
}
sysUserIdKey := fmt.Sprintf("%s%v", cacheSysUserIdPrefix, id)
sysUserUsernameKey := fmt.Sprintf("%s%v", cacheSysUserUsernamePrefix, data.Username)
两处 FindOne 的唯一作用都是取 username 构造 cacheSysUserUsernamePrefix 键;真正的并发安全依赖外层 expectedUpdateTime 或 IncrementTokenVersion 自己的 RowsAffected==0 → ErrUpdateConflict 兜底,data 对象其他字段并没有参与逻辑。
UpdateStatus 的调用方 UpdateUserStatusLogic 事前已经从 ValidateStatusChange 拿到 sysUser——sysUser.Username 就在手里;IncrementTokenVersion 的唯一调用方 LogoutLogic 从 middleware 拿到 userId,没有 username,但只要上游 ud, _ := UserDetailsLoader.Load(...) 已经拉过用户详情,一样可以透传。影响:
TokenOpLimiter,调用量不大;修复:把 username 显式提到函数签名,与 IncrementTokenVersionIfMatch(id, username, expected) 口径对齐:
UpdateStatus(ctx, id, username, status, expectedUpdateTime int64) error
IncrementTokenVersion(ctx, id, username int64) (int64, error)
调用方:
UpdateUserStatusLogic 传 user.Username;LogoutLogic 在限流已经通过的前提下,先 ud := UserDetailsLoader.Load(...)(Logout 本就会走到 Load),把 ud.Username 传进来。顺带把"内部 FindOne → ErrUpdateConflict 能正确触发"这条隐性依赖显式化,未来有人重构把内部 FindOne 挪走也不会把 CAS 语义改坏。
DeleteDept 与 UpdateUser.deptId 之间的 write skew描述:internal/logic/dept/deleteDeptLogic.go:36-69 的策略是:
DeleteDept tx:
① SELECT sys_dept WHERE id=? FOR UPDATE -- X 锁目标部门
② SELECT sys_dept WHERE parentId=? FOR SHARE -- S 锁确认无子部门
③ SELECT sys_user WHERE deptId=? FOR SHARE -- S 锁确认无关联用户
④ DELETE sys_dept WHERE id=?
UpdateUser 在修改 deptId 为目标部门时,只对 sys_dept 做不加锁的 FindOne 验证"目标部门存在且 Enabled",然后在自己的事务里对 sys_user 行取 X 锁写新 deptId(internal/logic/user/updateUserLogic.go:110-137、internal/model/user/sysUserModel.go:105-126)。
两个事务交错:
T1: DeleteDept
①②③ 都通过:sys_user.deptId=X 为空
(尚未 commit)
T2: UpdateUser.deptId=X
读 sys_dept[X] 的 RR 快照:Status=Enabled(T1 尚未 commit)
X 锁 sys_user[userY],写 deptId=X
T2 提交
T1: ④ DELETE sys_dept[X],提交
最终:sys_user[userY].deptId=X, sys_dept[X] 已删除 → 悬挂 deptId
T1 的 FOR SHARE ON sys_user WHERE deptId=X 对"将要变成 X 但目前不是 X"的行没有锁效力:InnoDB 的 gap lock 只覆盖 deptId 列现有的索引范围,userY 当前 deptId=Y 不在 T1 的锁范围。反向 T2 对 sys_dept[X] 是无锁读,看不到 T1 的 X 锁。这是典型的 write skew。
影响:
DeptTree / UserList 渲染时,这些用户找不到 dept path,会落回"default / 空 path"分支——所有非超管/非产品 ADMIN 的管辖判定全部对该用户失效(管他们的人发现人没了,被管的人发现管理员找不到自己);修复方案:二选一。
sys_user.deptId 用 FOR UPDATE 并补 gap lock(推荐):把 FOR SHARE 改成 FOR UPDATE,并在 sys_dept.id 列上(以及 deptId 外键索引)依赖 next-key lock。这会把"向这个 deptId 写入的 UpdateUser"阻塞到 DeleteDept 提交或回滚。代价:DeleteDept 持锁时间变长,但 DeleteDept 极其低频。UpdateUser 里对目标 sys_dept 加 FOR SHARE:UpdateUser 事务内先 SELECT sys_dept WHERE id=? FOR SHARE,然后再做 sys_user 的 X 锁写。这样 DeleteDept 的 X 锁会把 UpdateUser 的 S 读阻塞,形成一致锁链。代价:UpdateUser 的一次查询变成锁读,但 dept 查询本来就走缓存,打穿到 DB 的比例很低。两种方案等价化解 write skew。推荐 2(把锁链约束放在"新行写入方"上更自然,也与 CreateDept 在插子部门前对 parentId FOR SHARE 的现有策略口径一致)。
UpdateMember 对 memberType 空串直接 400,丧失"只改 status"语义此条在前轮 M-4 / L-5 系列修复中被反向推导过,R10 未列;本次复核明确为接口契约问题。
描述:UpdateMember 的 types 中 MemberType / Status 都是非指针必传字段。如果 admin 想只改 member 状态(冻结其产品成员资格),必须重传一份完整的 memberType;前端直接传空会被拦 400。这一约束与 HTTP API 的"部分更新"直觉不符。
影响:前端要么自己维护"原 memberType"在内存(多一个状态源),要么多打一次 member.detail 接口;属于接口易用性问题,不涉及安全。
修复:把 memberType / status 改成 *string / *int64,为 nil 时表示不改该字段;logic 侧按 nil/非 nil 分支分别处理校验,和 UpdateUserReq 的可选字段风格对齐。
DisableNotInCodesWithTx 与 DeleteBy...Tx 族把"整行 SELECT"当作"取缓存 key"的手段描述:internal/model/perm/sysPermModel.go:100-164、internal/model/userrole/sysUserRoleModel.go:105-166、internal/model/roleperm/sysRolePermModel.go:86-130、internal/model/userperm/sysUserPermModel.go:45-66 都遵循同一结构:
SELECT <全部列> FROM ... WHERE ... FOR UPDATE
拼 cache keys
UPDATE/DELETE ... WHERE <同样条件>
这里"SELECT 整行"的唯一用途是取 id 与构成缓存 key 的两三个字段(Code / ProductCode / UserId / RoleId / PermId)。真正需要的列最多 3 个,却把 Name / Remark / Status / CreateTime / UpdateTime 等字段全部搬运到应用层再丢弃。
影响:
DeleteByRoleIdTx 场景(DeleteRole 会级联删除所有 sys_user_role WHERE roleId=?,后续会对"受影响用户列表"做批量 cache 失效),被"关联成百上千用户"的角色删除时会显著增加 goroutine 临时内存与网络 I/O;session.QueryRowsCtx(list) + len(list)==0 提前 return 的"先查后写"模式,每次删除都付出一次"完整行读回"的成本,哪怕是单行删。修复:把所有"只是为构 cache key"的 SELECT 精简到只取必要列;或者只取 id / (user, role) / (role, perm) 等 key 组件,省略业务字段。例如:
// 只取 id + (productCode, code)
var rows []struct {
Id int64 `db:"id"`
ProductCode string `db:"productCode"`
Code string `db:"code"`
}
对"单行删除"路径(如 DeleteByRoleIdAndPermIdsTx 只有一个 permId)甚至可以直接由调用方传 key,不再反查。
UpdateProfile 不支持改 username,但签名暴露 username 参数,易被后续误用描述:internal/model/user/sysUserModel.go:105-126
func (m *customSysUserModel) UpdateProfile(ctx context.Context, id int64, username string,
nickname, email, phone, remark string, deptId, newStatus int64,
statusChanged bool, expectedUpdateTime int64) error {
sysUserIdKey := fmt.Sprintf("%s%v", cacheSysUserIdPrefix, id)
sysUserUsernameKey := fmt.Sprintf("%s%v", cacheSysUserUsernamePrefix, username)
...
// SET 语句里没有 `username`=?
入参有 username、但 UPDATE 里不写这个列;它只被用作"构造旧缓存 key"。未来若某人认为"签名已经带了 username,那 UpdateProfile 应该也能顺手改 username"并往 SET 里加上 username=?,会立刻出现:
cacheSysUserUsernamePrefix 删除旧值,stale 缓存残留;sys_user.username UNIQUE 约束违反的 1062 回滚。影响:当前代码功能正确,属于"签名 / 语义鸿沟"。若维护同学出于"方便"扩展此函数支持改 username 就会踩坑。
修复:
UpdateProfile 的 Go doc 里显式说明"本方法不负责修改 username;username 参数仅用于旧缓存键构造";UpdateUsernameTx,内部处理"新/旧 username 两份缓存键清理 + 1062 捕获"。UserDetailsLoader.CleanByProduct 扫描缓存前缀的放大效应描述:SyncPerms 成功后调用 UserDetailsLoader.CleanByProduct(ctx, product.Code)(internal/logic/pub/syncPermsService.go:163)。该方法(见 loaders/userDetailsLoader.go 实现)通常基于前缀批量失效该产品名下所有用户的详情缓存。
在"单 product <1k 活跃成员"的真实业务量下没有问题;但 SyncPerms 属于高频事件(产品每次部署都会触发,一天数十次),且 CleanByProduct 会把该产品所有活跃用户的缓存(含各自的 Perms / Member / Role 计算结果)一次性清掉,使得紧随其后的大量在线请求都会打穿缓存到 DB。
影响:
loadPerms 缓存一并清空;修复:
Added/Updated/Disabled 里对"发生状态变化的 code 集合"构建失效名单(按 role → user 反推),代价是扫描关联表(小量级 OK);CleanByProduct 里做 jitter(比如按 userId 哈希分批清,而非一次性全清),缓解"雪崩清空"。RefreshToken 两条路径的 newVersion != predictedVersion 分支实际不可达(重申 L-R10-4)R10-4 已记录本点。本轮复核确认 HTTP 与 gRPC 两条 RefreshToken 路径(internal/logic/pub/refreshTokenLogic.go:117-135、internal/server/permserver.go:240-251)的 forensic 分支仍然保留,没有被收敛到一个共享 helper。
影响:零运行期影响;纯代码重复 + 维护负担。
修复:把"试签 → CAS → Clean → 对比 newVersion"整段抽成 authHelper.RotateRefreshToken(ctx, svcCtx, claims, ud) (access, refresh string, err error),让 HTTP / gRPC 只负责前置校验与错误映射。这样未来 CAS 语义若要微调(例如把预签下沉到 tx 内),两条路径只改一处。
列出以下事项作为已定档契约,审计不再要求整改:
UserDetail / MemberList 同产品成员可见彼此 email / phone / remark —— 产品业务需求已确认保留。CreateProduct 响应体只返回一次性 ticket,真实 appSecret / adminPassword 通过 /fetchInitialCredentials(超管鉴权 + GetDelCtx 原子消费)领取。loadFreshMinPermsLevel),caller 的 MinPermsLevel 缓存不参与决策;TTL 不影响越权闭环。newVersion != predictedVersion 分支保留 forensic 兜底,本轮新建议(L-R11-5)仅涉及"把两处重复抽象成一处",不改变契约。PermList / RoleList 对同产品成员可见全量定义。属业务默认约定。loadPerms 对 SUPER / ADMIN / DEVELOPER 忽略 DENY 的语义已在 SetUserPerms 入口拦截;DeptType 动态变动导致旧 DENY 失效的长尾遗留。| 优先级 | 条目 | 理由 |
|---|---|---|
| P0 | H-R11-1 | 涉及密码本身的 last-write-wins;修复即放弃一条会话劫持旁路。下一迭代必修。 |
| P1 | M-R11-1 | gRPC 入口限流缺口;产品接入方越多风险敞口越大,建议排入下迭代 |
| P1 | M-R11-3 | write skew 罕见但数据无法自愈,连带修 UpdateUser 的锁链建议一次做完 |
| P2 | M-R11-2 | 性能与观测性改进;顺带提升 UpdateStatus / IncrementTokenVersion 的语义清晰度 |
| P2 | L-R11-1 | 前端易用性;可并到"接口契约梳理"专项 |
| P3 | L-R11-2 ~ L-R11-5 | 代码质量/性能优化;触及相关文件时顺手处理即可 |
整体代码质量在 10 轮迭代后已高度收敛,本轮只发现一条 High(H-R11-1,存在可复现的 TOCTOU)与三条 Medium;核心授权 / 会话 / 数据持久化三条链路的主干逻辑仍然稳健,历史修复契约未发生回退。