Files
mce/docs/audit-report-2025-05-31.md
Victor_Jay 55c408d86c fix: 审计问题全量修复 + RateLimiter 原子化重构 + CDN 本地化收紧
- 安全:移除 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 过清理→过期清理
2026-05-31 11:13:32 +08:00

16 KiB
Raw Blame History

MetaLab 项目全量代码审计报告

审计日期: 2025-05-31
审计范围: lab.metazone.cc-GO/ 全部 Go 源码、模板、配置
项目状态: 未发布,无需考虑旧版兼容
审查标准: DRY/KISS/YAGNI/LoD/SOLID + 最佳实践


问题清单


问题 #1: 【严重】CSP 安全头引用已废弃的 Tiptap CDNesm.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
}

问题:

  1. constantTimeEq 在同包内被 csrf.go:47 调用,非死代码
  2. 但其手写实现存在两个安全隐患:
    • 长度提前返回泄露信息len(a) != len(b) 时立即返回 false攻击者可通过响应时间判断 CSRF token 长度
    • 无编译器优化防护crypto/subtle.ConstantTimeCompare 内部使用了特殊的编译器屏障防止被优化,手写版本可能被 Go 编译器优化掉 XOR 结果检查
  3. 函数定义在 csrf_token.gotoken 生成文件)而非 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,  // ← 赋的是同一个对象
}

问题: tokenCtrlauthCtrl 指向同一个 *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

问题: 项目没有任何单元测试或集成测试。对于包含认证、权限、审核、数据持久化等复杂逻辑的系统,零测试意味着每次重构和修改都有回归风险。

建议: 至少为核心模块添加测试:

  1. model/user.go - HasMinRole/CanOperateRole纯函数易测
  2. service/auth_service.go - 注册/登录/密码验证逻辑
  3. service/admin_service.go - checkAndOperate 权限矩阵
  4. middleware/ratelimit_core.go - 限流逻辑

问题 #14: 【低】全项目使用 log.Printf 无结构化日志

类型: 可维护性 / 可观测性
位置: 10 个文件,约 17 处 log.Printf 调用

问题: 项目大量使用标准库 log.Printf,无日志级别、无结构化字段、无上下文追踪。在生产环境中排查问题困难,无法按级别过滤日志。

建议: 引入轻量结构化日志库(如 slogGo 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执行到位每层都通过最小接口依赖下层。依赖注入清晰无循环依赖。

主要问题集中在三个方面:

  1. 安全防护需加强 - CSP 配置残留、时序攻击防护不完整
  2. 代码清理不及时 - 存在死代码、预留目录、未使用函数
  3. 工程基础设施薄弱 - 零测试、无结构化日志、错误上下文丢失

建议优先处理严重级别问题(#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)