- 安全:移除 CSP 中 esm.sh/cdnjs.cloudflare.com,highlight.js 主题已本地化 73 个文件 - 安全:StatusBanned 分支补 dummy hash 防时序攻击 - 安全:手写 constantTimeEq 替换为 crypto/subtle.ConstantTimeCompare - Bug:锁定操作消息已删除→已锁定 - YAGNI:删除 12 个空预留模板目录 - KISS:删除 common.CheckPassword 薄封装,统一用 bcrypt 调用 - KISS:删除 tokenCtrl 别名字段,api.go 统一用 authCtrl - RateLimiter:check()+recordFail 闭包模式重构为 try() 原子操作,消除竞态 - RateLimiter:新增 ClearIP() 方法,注册成功时清除 IP 计数 - 文档:修正 audit_service.go 注释编号跳跃(3→5→4) - 文档:修正 deps_core.go 过清理→过期清理
16 KiB
MetaLab 项目全量代码审计报告
审计日期: 2025-05-31
审计范围: lab.metazone.cc-GO/ 全部 Go 源码、模板、配置
项目状态: 未发布,无需考虑旧版兼容
审查标准: DRY/KISS/YAGNI/LoD/SOLID + 最佳实践
问题清单
问题 #1: 【严重】CSP 安全头引用已废弃的 Tiptap CDN(esm.sh)
类型: 死代码 / 残留配置
位置: internal/middleware/security.go:12-21
违反原则: KISS(引用了不存在的依赖)
// script-src: 本站 + esm.sh CDN (Tiptap ESM 模块) + cdnjs (highlight.js)
"script-src 'self' 'unsafe-inline' https://esm.sh https://cdnjs.cloudflare.com; "+
"style-src 'self' 'unsafe-inline' https://esm.sh https://cdnjs.cloudflare.com; "+
"connect-src 'self' https://esm.sh"
问题: 项目已迁移到 Vditor 编辑器,但 CSP 头仍保留 Tiptap 时代的 esm.sh CDN 白名单。三个指令(script-src、style-src、connect-src)都包含未使用的 https://esm.sh。
风险:
- 扩大了不必要的 CSP 白名单,引入额外信任域
- connect-src 允许到 esm.sh 的连接,可能泄露页面信息
建议: 移除所有 https://esm.sh 引用,highlight.js 主题已本地化为 73 个静态文件(static/vditor/dist/js/highlight.js/styles/),CSP 可完全收紧为 'self'。
问题 #2: 【严重】Login() 中封禁状态检查缺少防时序攻击保护
类型: 安全缺陷
位置: internal/service/auth_service.go:151-154
违反原则: 最佳安全实践
// 封禁
if user.Status == model.StatusBanned {
return nil, common.ErrUserBanned // ← 没有 bcrypt dummy hash 比对
}
问题: 其他分支(维护模式、用户不存在、密码错误、StatusLocked)都有 _ = bcrypt.CompareHashAndPassword(dummyHash, ...) 防时序攻击,唯独 StatusBanned 分支缺失。攻击者可以通过响应时间差异判断被封禁的账号是否存在。
建议: 在返回 ErrUserBanned 前增加 dummy hash 比对:
if user.Status == model.StatusBanned {
_ = bcrypt.CompareHashAndPassword(dummyHash, []byte(req.Password))
return nil, common.ErrUserBanned
}
问题 #3: 【严重】自定义 constantTimeEq 不如 crypto/subtle 安全
类型: 安全缺陷 / 最佳实践
位置: internal/middleware/csrf_token.go:44-53(被 csrf.go:47 调用)
违反原则: 安全最佳实践
// constantTimeEq 恒定时间字符串比较(防时序攻击)
func constantTimeEq(a, b string) bool {
if len(a) != len(b) { // ← 长度不等时提前返回,泄露长度信息
return false
}
var result byte
for i := 0; i < len(a); i++ {
result |= a[i] ^ b[i]
}
return result == 0
}
问题:
constantTimeEq在同包内被csrf.go:47调用,非死代码- 但其手写实现存在两个安全隐患:
- 长度提前返回泄露信息:
len(a) != len(b)时立即返回 false,攻击者可通过响应时间判断 CSRF token 长度 - 无编译器优化防护:
crypto/subtle.ConstantTimeCompare内部使用了特殊的编译器屏障防止被优化,手写版本可能被 Go 编译器优化掉 XOR 结果检查
- 长度提前返回泄露信息:
- 函数定义在
csrf_token.go(token 生成文件)而非csrf.go(校验文件),逻辑归属不当
建议: 删除 constantTimeEq,改用 crypto/subtle.ConstantTimeCompare([]byte(cookieToken), []byte(headerToken)) == 1。这也是 Go 官方推荐的做法。
问题 #4: 【高】Admin 控制器锁定操作消息显示错误
类型: Bug
位置: internal/controller/admin/admin_controller.go:93-96
违反原则: 无(纯 bug)
action := "封禁"
if req.Status == model.StatusActive {
action = "解封"
} else if req.Status == model.StatusLocked {
action = "已删除" // ← BUG: 应该是 "已锁定"
}
问题: 当管理员执行锁定操作时,成功提示消息显示"已删除成功",而不是"已锁定成功"。Locked 与 Deleted 是两个完全不同的状态。
建议: 改为 action = "已锁定"
问题 #5: 【高】Login() 中密码验证方式不一致
类型: 代码一致性问题
位置: internal/service/auth_service.go:138,152
违反原则: KISS(同一逻辑用了两种实现)
// StatusDeleted 分支:
if !common.CheckPassword(req.Password, user.PasswordHash) { ... }
// StatusBanned 之后的主路径:
if !common.CheckPassword(req.Password, user.PasswordHash) { ... }
而 CheckPassword 内部只是封装了单行:
func CheckPassword(password, hash string) bool {
err := bcrypt.CompareHashAndPassword([]byte(hash), []byte(password))
return err == nil
}
问题: 虽然功能上没有 bug,但在同一函数中既有 common.CheckPassword() 调用,也有其他分支使用 _ = bcrypt.CompareHashAndPassword(dummyHash, ...)(直接调用 bcrypt)。风格不统一,且 CheckPassword 的封装价值极低(仅包装一行标准库调用,不如直接使用 bcrypt 调用更直观)。
建议:
- 选项A: 删除
common.CheckPassword,统一使用bcrypt.CompareHashAndPassword - 选项B: 在
CheckPassword中增加防时序的一致性包装,统一入口
问题 #6: 【中】大量空模板目录(YAGNI 违规)
类型: YAGNI 违规
位置:
templates/MetaLab-2026/html/post/(空)templates/MetaLab-2026/html/comment/(空)templates/MetaLab-2026/html/partials/(空)templates/MetaLab-2026/html/search/(空)templates/MetaLab-2026/html/error/(空)templates/MetaLab-2026/html/admin/(空)templates/system/email/(空)
问题: 7 个空目录,标注为"预留"。项目尚未发布,这些目录的创建时机应该和实际功能开发同步,而非提前占位。
建议: 删除所有空目录。需要时随功能一起创建。
问题 #7: 【中】Shortcode 预留类型(poll/resource)无后端实现
类型: YAGNI 违规
位置: internal/model/shortcode.go:37-38, internal/service/shortcode_service.go:126-137
违反原则: YAGNI
ShortcodePoll ShortcodeType = "poll" // ※预留(后端API未实现)
ShortcodeResource ShortcodeType = "resource" // ※预留(后端API未实现)
问题: poll 和 resource 两个 shortcode 类型的后端 API 未实现,前端也未实现(shortcode.js 中可能也未实现对应渲染),但代码中已注册了完整的解析和占位 HTML 生成逻辑。用户实际上可以使用 [zone:poll:xxx] 语法,但会得到一个永远"加载中..."的卡片。
状态: 已排期开发,保留(不删除)。首次审计时误删,已恢复。前端渲染逻辑将随 API 同步实现。
问题 #8: 【中】redis_store.go 全注释的"预留实现"
类型: YAGNI / 死代码
位置: internal/session/redis_store.go
违反原则: YAGNI
问题: 整个文件是注释掉的代码,没有实际可执行逻辑。如果未来需要 Redis 支持,到时再创建即可。
状态: 已排期开发,保留(不删除)。首次审计时误删,已恢复。Redis 支持将在后续迭代中实现。
问题 #9: 【中】tokenCtrl 是 authCtrl 的无意义别名
类型: 不必要的字段重复
位置: internal/router/deps_core.go:30, internal/router/deps_extra.go:54
违反原则: KISS
// deps_core.go
type dependencies struct {
// ...
tokenCtrl *controller.AuthController // ← 与 authCtrl 类型完全相同
}
// deps_extra.go
return &dependencies{
// ...
tokenCtrl: authCtrl, // ← 赋的是同一个对象
}
问题: tokenCtrl 和 authCtrl 指向同一个 *controller.AuthController 实例,tokenCtrl 仅在 api.go 的路由中使用(d.tokenCtrl.CheckEmail/Register/Login/...)。这个别名不带来任何好处,反而增加理解成本。
建议: 删除 tokenCtrl 字段,api.go 中直接使用 d.authCtrl。
问题 #10: 【中】RateLimiter.check() 存在竞态条件
类型: 并发缺陷
位置: internal/middleware/ratelimit_core.go:34-81
func (rl *RateLimiter) check(...) (RateLimitResult, func()) {
rl.mu.Lock()
defer rl.mu.Unlock()
// ... 读取 state ...
recordFail := func() {
rl.mu.Lock() // ← 重新获取锁
defer rl.mu.Unlock()
// ... 修改 state ...
}
return RateLimitResult{Blocked: false}, recordFail
}
问题: check() 持锁检查后释放锁,返回的 recordFail 闭包在锁外执行,重新获取锁后再修改状态。
修复方案: 将 check() + recordFail 闭包模式重构为 try() 原子操作模式——在持锁状态下一次性完成检查+递增,消除竞态窗口。API 改为 AllowAccount() RateLimitResult / AllowIP() RateLimitResult(不再返回闭包)。调用方在失败时不再需要显式调用 recordFail(),成功时仍调用 Clear() 清除计数。
问题 #11: 【低】audit_service.go review() 注释编号跳跃
类型: 文档瑕疵
位置: internal/service/audit_service.go:152-166
// 3. 查审核人信息
reviewer, err := s.userRepo.FindByID(reviewerID)
// ...
// 5. 标记审核结果 ← 跳过了 4
submission.ReviewedBy = &reviewerID
问题: 注释编号从"3."直接跳到"5.",缺少"4."。
建议: 修正编号为连续递增。
问题 #12: 【低】编译产物未纳入 .gitignore(误报,实际不存在)
类型: 仓库整洁性
位置: server(根目录), cmd/server/server
状态: 误报。经核实,.gitignore 已配置 server 规则且从未被 git 跟踪,git rm --cached 实际为空操作。此项已从修复表中移除。
问题 #13: 【低】项目零测试覆盖
类型: 质量保障缺失
位置: 整个项目(*_test.go 搜索结果: 0)
问题: 项目没有任何单元测试或集成测试。对于包含认证、权限、审核、数据持久化等复杂逻辑的系统,零测试意味着每次重构和修改都有回归风险。
建议: 至少为核心模块添加测试:
model/user.go- HasMinRole/CanOperateRole(纯函数,易测)service/auth_service.go- 注册/登录/密码验证逻辑service/admin_service.go- checkAndOperate 权限矩阵middleware/ratelimit_core.go- 限流逻辑
问题 #14: 【低】全项目使用 log.Printf 无结构化日志
类型: 可维护性 / 可观测性
位置: 10 个文件,约 17 处 log.Printf 调用
问题: 项目大量使用标准库 log.Printf,无日志级别、无结构化字段、无上下文追踪。在生产环境中排查问题困难,无法按级别过滤日志。
建议: 引入轻量结构化日志库(如 slog,Go 1.21+ 标准库),按级别区分 Info/Warn/Error,关键路径添加 trace/request ID。
问题 #15: 【低】err != nil 返回时上下文信息丢失
类型: 可调试性
位置: 多处,例如 internal/repository/user_repo.go
func (r *UserRepo) Create(user *model.User) error {
return r.db.Create(user).Error // ← 无上下文
}
问题: 数据库操作失败时,调用方只知道"出错了",无法快速定位是哪个操作、哪个实体、哪个 ID 导致的失败。
建议: 使用 fmt.Errorf("创建用户失败: %w", err) 包装错误,在保持错误链的同时添加操作上下文。
问题 #16: 【低】common.CheckPassword 封装价值极低
类型: KISS 违规
位置: internal/common/crypto.go:12-15
func CheckPassword(password, hash string) bool {
err := bcrypt.CompareHashAndPassword([]byte(hash), []byte(password))
return err == nil
}
问题: 仅包装一行标准库调用,无额外逻辑。与同一文件中封装了 bcrypt 成本参数的 HashPassword 不同,CheckPassword 没有提供抽象价值。反而因为隐藏了 bcrypt.CompareHashAndPassword 的调用,在需要 dummyHash 比对时(如 auth_service.go 的时序攻击防护)不得不绕过它直接调用 bcrypt。
建议:
- 如果保留
CheckPassword,将 dummy hash 比对也内置进去 - 或者删除此函数,直接在各处显式调用
bcrypt.CompareHashAndPassword
问题 #17: 【低】deps_core.go 中文注释错别字
类型: 文档瑕疵
位置: internal/router/deps_core.go:45
// 启动后台过清理 goroutine
问题: "过清理"应为"过期清理",少了一个"期"字。
建议: 修正为 启动后台过期清理 goroutine
汇总统计
| 严重程度 | 数量 | 问题编号 |
|---|---|---|
| 严重 | 3 | #1, #2, #3 |
| 高 | 2 | #4, #5 |
| 中 | 5 | #6, #7, #8, #9, #10 |
| 低 | 7 | #11, #12, #13, #14, #15, #16, #17 |
总计: 17 个问题
按原则分类
| 原则 | 问题编号 |
|---|---|
| 安全 | #1, #2, #3 |
| YAGNI | #6, #7, #8 |
| KISS | #5, #9, #16 |
| Bug | #4, #10 |
| 质量/可维护性 | #12, #13, #14, #15 |
| 文档 | #11, #17 |
整体评价
项目的分层架构设计合理,严格遵循单向依赖(router→controller→service→repository→model),接口隔离原则(ISP)执行到位,每层都通过最小接口依赖下层。依赖注入清晰,无循环依赖。
主要问题集中在三个方面:
- 安全防护需加强 - CSP 配置残留、时序攻击防护不完整
- 代码清理不及时 - 存在死代码、预留目录、未使用函数
- 工程基础设施薄弱 - 零测试、无结构化日志、错误上下文丢失
建议优先处理严重级别问题(#1~#3),然后按批次逐步处理其余问题。
修复记录
修复日期: 2025-05-31
执行方式: 全量修复(commit: fix: 审计问题全量修复(安全/YAGNI/Bug/代码质量))
已修复(13 项)
| # | 修复内容 | 变更文件 |
|---|---|---|
| 1 | 移除 CSP 中 esm.sh (Tiptap 残留) 和 cdnjs.cloudflare.com(主题已本地化),CSP 全面收紧为 'self' | middleware/security.go |
| 2 | StatusBanned 分支增加 dummy hash 防时序攻击 | service/auth_service.go |
| 3 | 删除手写 constantTimeEq,改用 crypto/subtle.ConstantTimeCompare |
middleware/csrf_token.go, middleware/csrf.go |
| 4 | 锁定操作消息修正"已删除"→"已锁定" | controller/admin/admin_controller.go |
| 5 | 删除 common.CheckPassword 薄封装,统一改用 bcrypt.CompareHashAndPassword | common/crypto.go, service/auth_service.go |
| 6 | 删除 12 个空预留目录(含第二轮追加 5 个) | templates/MetaLab-2026/html/{post,comment,partials,search,error,admin,topic,user}, templates/{system,system/email}, templates/MetaLab-2026/static/{img,vendor} |
| 9 | 删除 tokenCtrl 别名字段,统一使用 authCtrl | router/deps_core.go, router/deps_extra.go, router/api.go |
| 10 | 将 check()+recordFail 闭包重构为 try() 原子操作,消除竞态 | middleware/ratelimit_core.go, middleware/ratelimit.go, middleware/ratelimit_cleanup.go, controller/auth_api_login.go, controller/auth_api_register.go |
| 11 | 修正 review() 注释编号 3→5→4 | service/audit_service.go |
| 17 | 修正"过清理"→"过期清理" | router/deps_core.go |
已回滚(误删,已排期开发,保留)
| # | 回滚内容 | 变更文件 |
|---|---|---|
| 7 | 恢复 poll/resource shortcode 类型(误删) | model/shortcode.go, service/shortcode_service.go |
| 8 | 恢复 redis_store.go 预留实现(误删) | session/redis_store.go |
误报(实际不存在)
| # | 说明 |
|---|---|
| 12 | 编译产物从未被 git 跟踪,.gitignore 已生效,git rm --cached 为空操作 |
暂缓修复(3 项)
| # | 暂缓原因 | 后续计划 |
|---|---|---|
| 13 | 零测试 — 需要建立测试框架、mock 策略,工作量大 | 核心模块优先:hasMinRole、checkAndOperate、RateLimiter |
| 14 | 无结构化日志 — 需评估 slog vs zap,全量替换 log.Printf | Go 1.21+ 使用标准库 slog 渐进替换 |
| 15 | 错误上下文丢失 — 涉及全部 repository 层,工作量大 | 按文件逐批添加 fmt.Errorf("...: %w", err) |