# 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（引用了不存在的依赖）

```go
// 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`  
**违反原则**: 最佳安全实践

```go
// 封禁
if user.Status == model.StatusBanned {
    return nil, common.ErrUserBanned  // ← 没有 bcrypt dummy hash 比对
}
```

**问题**: 其他分支（维护模式、用户不存在、密码错误、StatusLocked）都有 `_ = bcrypt.CompareHashAndPassword(dummyHash, ...)` 防时序攻击，唯独 StatusBanned 分支缺失。攻击者可以通过响应时间差异判断被封禁的账号是否存在。

**建议**: 在返回 `ErrUserBanned` 前增加 dummy hash 比对：
```go
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` 调用）  
**违反原则**: 安全最佳实践

```go
// 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.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）

```go
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（同一逻辑用了两种实现）

```go
// StatusDeleted 分支:
if !common.CheckPassword(req.Password, user.PasswordHash) { ... }

// StatusBanned 之后的主路径:
if !common.CheckPassword(req.Password, user.PasswordHash) { ... }
```

而 `CheckPassword` 内部只是封装了单行：
```go
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

```go
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

```go
// 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`

```go
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`

```go
// 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`，无日志级别、无结构化字段、无上下文追踪。在生产环境中排查问题困难，无法按级别过滤日志。

**建议**: 引入轻量结构化日志库（如 `slog`，Go 1.21+ 标准库），按级别区分 Info/Warn/Error，关键路径添加 trace/request ID。

---

### 问题 #15: 【低】err != nil 返回时上下文信息丢失

**类型**: 可调试性  
**位置**: 多处，例如 `internal/repository/user_repo.go`

```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`

```go
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`

```go
// 启动后台过清理 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)` |
