179 lines
19 KiB
Markdown
179 lines
19 KiB
Markdown
# TamaBox-v4 整体代码审阅报告
|
||
|
||
- **审阅日期**：2026-08-28
|
||
- **审阅范围**：全仓（87 个 Go 文件 + 38 个 HTML 模板 + 配置/脚本）
|
||
- **技术栈**：Flamego + GORM + PostgreSQL + Redis（1C1G 部署）
|
||
- **方法**：4 个并行只读探索代理（架构 / 安全 / 性能 / 前端·代码质量）+ 关键高危项人工复核
|
||
- **说明**：本报告为纯审阅，**未改动任何代码**。按你一贯的分阶段工作流，请选定优先级后我分批落地。
|
||
|
||
---
|
||
|
||
## 一、执行摘要
|
||
|
||
整体健康度**良好**——安全工程成熟度高于同类项目：IDOR 越权、XSS 转义、SQL 注入三大高风险面均有系统化纵深防御；会话/CSRF/限流/SSRF/内容审核（fail-closed）均已落地。未发现会让线上立即崩溃或数据泄露的 P0 级事故。
|
||
|
||
主要短板集中在：**配置防呆不足**（Salt 未校验）、**部署样例与结构体错位**（PostgreSQL 直接连库失败）、**少量数据层查询未分页/缺索引**、**一个暴露面偏宽的内部反代**、以及**架构债**（上帝文件、缺 service 层）。这些均为可规划修复的中等问题，无紧急止血项。
|
||
|
||
| 维度 | 评分 | 一句话 |
|
||
|------|------|--------|
|
||
| 安全 | 🟢 良好 | 高风险面有纵深防御；残余项为中低危加固 |
|
||
| 架构 | 🟡 中等 | 分层清晰但存在上帝文件、缺 service 层、配置防呆不足 |
|
||
| 性能 | 🟢 良好 | 连接池/缓存/分页已调优；少量 N+1 与缺索引 |
|
||
| 代码质量 | 🟢 良好 | 日志统一、测试有部分覆盖；魔法字符串与构建依赖待理 |
|
||
| 前端/UX | 🟢 良好 | 模板写法规范、交互健全；少量体验增强空间 |
|
||
|
||
---
|
||
|
||
## 二、问题清单（按优先级）
|
||
|
||
### 🔴 P0 — 部署前必须修复（防呆 / 阻断）
|
||
|
||
**P0-1 密码 Salt 未做非空校验（高危防呆）**
|
||
- 位置：`internal/db/password.go:25`、`password.go:55` 使用 `HmacSha1(password, conf.Server.Salt)`；`conf/conf.go` 仅校验 `external_url` 与 `xsrf_key`，**未校验 Salt**。
|
||
- 影响：若部署沿用样例空盐（`app.sample.ini` 中 `salt = ""`），全站密码散列实际以空盐计算——相同密码产生相同哈希，且针对本站的离线字典/彩虹表攻击可行。
|
||
- 建议：在 `conf.Init` 仿照 `xsrf_key` 增加 `if Server.Salt == "" { return error }`；并提供强随机默认值或启动即失败。
|
||
|
||
**P0-2 配置样例与结构体字段不一致（部署即失败）**
|
||
- 位置：`conf/app.sample.ini:36-42` 的 `[database]` 用 `address = "tcp(127.0.0.1:3306)"`；而结构体为 `type/host/port/name`（`internal/conf/static.go:58-63`），**缺 host/port/type**。
|
||
- 影响：按样例部署时 `Database.Type` 为空 → 走 mysql 分支 → 生成非法 DSN `:@tcp(:0)/`，启动连库直接失败；PostgreSQL 用户也无法从样例获得正确字段。
|
||
- 建议：样例补全 `type = postgres`、`host`、`port`、`name`、`user`、`password`、`ssl_mode`，使其与当前结构体一一对应。
|
||
|
||
### 🟠 P1 — 应当修复（重要）
|
||
|
||
**P1-1 `/api/v1/service` 反代可转发至内部后端（中危）**
|
||
- 位置：`route/service/proxy.go:35-48`
|
||
- 影响：登录用户可控 `**` 路径转发到配置后端，`httputil.ReverseProxy` 透传原始 Cookie/Header；若后端前缀泄露或配置为内网地址，可成为横向探测入口；且 `TLSInsecureSkipVerify` 可关闭证书校验。
|
||
- 建议：白名单限定可转发子路径与方法；禁止转发敏感 Header；内网后端强制校验；补审计日志。
|
||
|
||
**P1-2 导出接口 N+1 查询**
|
||
- 位置：`route/user/profile.go:381-401`（循环内 `FollowUps.GetByQuestionID`/`UploadImages.GetByQuestionID`，每题 2 次）、`profile.go:426-449`（循环 `Messages.GetByConversationID`，每会话 1 次）。
|
||
- 影响：大账号导出慢、占用 DB 连接。
|
||
- 建议：改为 `IN (?)` 批量拉取后内存分组（与已有的 `GetFirstMessagesByConversationIDs` 窗口函数做法一致）。
|
||
|
||
**P1-3 私信列表无分页（全量返回）**
|
||
- 位置：`internal/db/conversation.go:145-161` `ListByOwner`/`ListByAsker` 无 LIMIT；调用处 `profile.go:419-422`、`chat` 列表。
|
||
- 影响：私信多的用户一次拉全部会话与消息，内存/DB 压力大。
|
||
- 建议：加 `LIMIT/OFFSET` 或按 `created_at DESC` 游标分页。
|
||
|
||
**P1-4 高频查询缺复合索引**
|
||
- `questions.asker_user_id`：`CREATE INDEX idx_questions_asker ON questions(asker_user_id);`（`questions.go:243`）
|
||
- 箱主公开页：`CREATE INDEX idx_q_user_private_created ON questions(user_id, is_private, created_at);`（`questions.go:171-228`）
|
||
- 探索页：`CREATE INDEX idx_users_showinhome_created ON users(show_in_home, created_at);`（`users.go:487-540`）
|
||
- 投票热门：`CREATE INDEX idx_votes_status_vis_created ON votes(status, visibility, created_at);`（`votes.go:206-270`）
|
||
- 消息每日计数：`CREATE INDEX idx_messages_sender_created ON messages(sender_id, created_at);`（`users.go:805`）
|
||
- 会话消息排序：将单列 `conversation_id` 索引改为复合 `(conversation_id, created_at)`（`message.go:56`）
|
||
|
||
**P1-5 changelog 仍走外部 fetch（引入外部依赖）**
|
||
- 位置：`templates/change-logs.html:10` 从外部域名取 `tamabox.changelogs.json`，而本地已生成并提交 `tamabox.changelogs.json` 未被使用。
|
||
- 影响：跨域依赖、外部不可达时 changelog 空白、可被篡改。
|
||
- 建议：将 json 纳入 `static` embed，新增同源路由 `/changelogs.json`（或放入 static 目录直接 fetch 同源路径），去掉外部依赖。
|
||
|
||
**P1-6 私密提问访问令牌熵值偏低（隐私/安全）**
|
||
- 位置：`internal/db/questions.go:93`（`Token: randstr.String(6)`）
|
||
- 影响：私密/未答提问与追问权仅靠 6 位随机串（约 62^6），结合 questionID 可枚举猜测，进而读取私密提问或冒用提问者身份发追问。
|
||
- 建议：Token 提升至 ≥16 位随机串或直接用 UUIDv4。
|
||
|
||
**P1-7 架构债：上帝文件 + 缺 service 层（可维护性，分阶段重构）**
|
||
- 位置：`route/user/vote.go`(1289 行)、`route/vote/vote.go`(1036)、`route/question/share.go`(964)、`internal/db/users.go`(881)。
|
||
- 影响：单文件同时承载 HTML 渲染、图片生成、DB 事务与业务判断，违反单一职责，难测试难维护；`route/*` 直接调 `db.*`，业务逻辑散布。
|
||
- 建议：引入 `internal/service` 承载用例编排，handler 仅做请求解析/渲染；图片/分享卡生成抽到 `internal/render`。分阶段进行，不一次性大改。
|
||
|
||
### 🟡 P2 — 可优化（锦上添花 / 技术债）
|
||
|
||
- **webp CGO 硬依赖**（已知约束）：`internal/storage/s3.go:23` 与 `route/question/page.go:595` 直接 `import "github.com/chai2010/webp"`，链接阶段即需 libwebp，阻断 Windows/macOS 构建。你已在 Linux 解决，**不作为当前阻塞项**。建议用 `//go:build linux,cgo` 隔离为可选实现，未启用时走原图直传，消除跨平台构建门槛。
|
||
- **魔法字符串**：提问过滤参数 `"all"/"public"/"private_question"` 散落 `route/user/question.go:58,121,171,174`、`form/question.go:17`，无编译期保护。建议抽 `QuestionFilter*` 常量（投票可见性已有先例 `internal/db/votes.go:43`）。
|
||
- **会话 Cookie 未硬化**：`internal/route/route.go:92-95` 未设 `Secure/SameSite`。建议 `session.Options{Cookie:{Secure:true, SameSite:Lax}}`。
|
||
- **PostgreSQL 无 TLS**：`internal/conf/db.go:16` 硬编码 `sslmode=disable`。建议加 `ssl_mode` 配置项，生产默认 `require`。
|
||
- **限流为单进程内存**：`internal/route/ratelimit.go:17-100`，多实例可被分摊绕过；登录无失败锁定。建议改用 Redis 共享限流 + 失败阈值临时锁定。
|
||
- **匿名身份服务端留存**：`internal/db/questions.go:64-72` 记录 `FromIP`；若承诺强匿名，应定期脱敏并明示隐私策略。
|
||
- **邮件模板外链字体**：`templates/mail/*.html` 引用 `//fonts.loli.net`，离线/被墙时回退不可控。建议改用系统字体栈或自托管。
|
||
- **遗留 .bak**：`templates/question/new-question-template.html.bak` 含硬编码中文，易与 i18n 正式版混淆。建议删除，由 git 管历史。
|
||
- **无优雅退出**：`internal/cmd/web.go:82-94` 未注册 `signal.Notify`，重启会断连。建议加 SIGTERM + `server.Shutdown(ctx)`。
|
||
- **路由包命名混乱**：顶层 `route` 与 `internal/route` 同名（无 import 循环，但易误读）。建议收口到 `internal/route/*`。
|
||
- **首页统计缓存细节**：`internal/stats/stats.go:41-78` 进程内 30 分钟缓存合理，但过期瞬间并发重算、写后未 `Invalidate()`。建议 TTL 加抖动 + 写后失效。
|
||
- **GetHotTags 全表 Pluck**：`internal/db/users.go:661-667`（已 60s 缓存）。大数据量建议 SQL 内聚合或限样本。
|
||
- **搜索无分页**：`internal/db/questions.go:374` `SearchByUserID` 返回全部 LIKE 匹配。建议加 `LIMIT`。
|
||
- **CSRF 对 `/api/v1/pixel/` 关闭**：`internal/context/context.go:170`。影响有限，建议仅限 GET 或补回校验。
|
||
- **handler 级测试薄弱**：`route/question`、`route/user` 多依赖真实 DB/Redis，缺单测。建议补基于 `dbutil/testing.go` 的集成测试或 httptest mock。
|
||
|
||
---
|
||
|
||
## 三、已确认的良好实践（正面，保持）
|
||
|
||
- **IDOR 防护到位**：提问删除/私密/回答经 `canDelete = 登录且为箱主`（`route/question/question.go:69`）及 `question.UserID==pageUser.ID` 双重校验；投票经 `getOwnedVote`（`route/user/vote.go:392`）；私聊经 `OwnerID/AskerID`（`chat_handler.go:52`）。
|
||
- **XSS 转义到位**：`QuestionFormat/AnswerFormat` 走 `html.EscapeString`（`internal/template/template.go:46-55`）；Markdown 经白名单 sanitize；`toJSON` 依赖 `json.Marshal` 默认转义。
|
||
- **SQL 注入为零**：全量 GORM 参数化；Raw 均用 `?` 占位（`message.go:91` 的 `IN ?`）。
|
||
- **SSRF 防护**：`/proxy-image` 含域名白名单 + 私网 IP 拦截 + DNS rebinding 防护（`route/proxy.go:24-127`）。
|
||
- **其他**：注册/登录/提问强制验证码；内容审核 fail-closed；登录会话再生防固定（`route/auth/login.go:66`）；开放跳转前缀校验防开放重定向。
|
||
- **性能基础好**：模板启动解析缓存（`templates/embed.go`）、Gzip 全量接入（`internal/cmd/web.go:84`）、连接池按 1C1G 调优（MaxOpen=10）、批量反 N+1 已落地。
|
||
- **Go 模板无运算符误用**：全仓 grep `!=`/`==`/`&&`/`||` 零命中，无启动 panic 风险；删除交互均有二次确认。
|
||
|
||
---
|
||
|
||
## 四、新功能与 UX 提升建议
|
||
|
||
> 均考虑 1C1G 约束（不引入常驻重服务）；按"用户价值 / 实现成本"简评。
|
||
|
||
| # | 建议 | 用户价值 | 成本 | 说明 |
|
||
|---|------|---------|------|------|
|
||
| F1 | **通知中心 + 邮件摘要聚合** | 高 | 中 | 提问箱核心体验是"收到提问"。站内通知列表 + 邮件可设每日/每周摘要开关，避免逐条邮件轰炸；与现有邮件体系复用。 |
|
||
| F2 | **箱主治理面板** | 高 | 中 | 自定义提问箱简介/主题、开关"允许匿名/图片/投票"、屏蔽关键词、隐藏不当内容。提升箱主掌控感。 |
|
||
| F3 | **前台举报入口** | 高 | 低-中 | 后台已有 fail-closed 审核，但面向用户的"举报不当提问/回答"按钮缺失；补前台入口 + 箱主一键隐藏。 |
|
||
| F4 | **私密提问体验增强** | 中 | 中 | 追问流、已读状态、链接过期机制（配合 P1-6 令牌熵提升），让匿名双向对话更顺畅。 |
|
||
| F5 | **探索/发现页增强** | 中 | 中 | 标签筛选、热门排序、按话题聚合（`GetHotTags` 已有数据），提升内容分发与留存。 |
|
||
| F6 | **防刷与内容质量** | 中 | 中 | per-box 提问限流、垃圾链接/敏感词前端预检、重复内容检测（配合 P2 限流改造）。 |
|
||
| F7 | **数据导出优化** | 中 | 低 | 支持 Markdown/PDF 导出，导出走批量查询（顺带修 P1-2 N+1），导出包隐私脱敏。 |
|
||
| F8 | **移动端 PWA + 暗色模式** | 中 | 中 | 纯静态资源即可，1C1G 友好；显著提升移动端阅读与夜间体验。 |
|
||
| F9 | **草稿与定时发布** | 低-中 | 中 | 私密回答存草稿、公开回答定时发布，降低发布压力。 |
|
||
| F10 | **无障碍 a11y** | 低 | 低 | 键盘可达、aria 标签、对比度，提升专业度与合规。 |
|
||
|
||
**优先推荐**：F1（通知中心）+ F2（箱主治理）+ F3（前台举报）组合——直接提升"收提问→治理→互动"的主链路体验，且不引入重型依赖。
|
||
|
||
---
|
||
|
||
## 五、建议的分阶段实施路径
|
||
|
||
1. **阶段 0（本次部署前）**：P0-1、P0-2（防呆 + 样例对齐），半小时内可合。
|
||
2. **阶段 1（安全/数据层）**：P1-1 反代收敛、P1-6 令牌熵、P1-2/P1-3 分页与 N+1、P1-4 建索引、P1-5 changelog 同源。
|
||
3. **阶段 2（架构债）**：P1-7 引入 service 层，按模块分批抽离上帝文件（先 `route/question/share.go` 的图片/分享卡）。
|
||
4. **阶段 3（体验增强）**：P2 各项 + 从 F1/F2/F3 启动新功能。
|
||
|
||
---
|
||
|
||
## 附：探索索引
|
||
- 架构：`tamabox.go` → `cmd.Web` → `conf.Init` → `db.Init` → `route.New()`
|
||
- 安全：IDOR/ XSS / SQLi / SSRF / 限流 / 会话 已逐项核验
|
||
- 性能：连接池、缓存、分页、索引、changelog、统计
|
||
- 前端：模板转义、partial、交互确认、webp 构建依赖、魔法字符串
|
||
|
||
---
|
||
|
||
## 六、实施进展
|
||
|
||
### P0 已修复（2026-08-28）
|
||
- **P0-1 Salt 校验**：`internal/conf/conf.go` 的 `Init` 在 `server` 段 MapTo 后新增 `if Server.Salt == "" { return error }`，空盐启动即失败，防止密码以空盐哈希（离线碰撞风险）。
|
||
- **P0-2 配置样例对齐**：`conf/app.sample.ini` 的 `[database]` 段按 `Database` 结构体字段对齐（`type`/`host`/`port`/`name`/`schema`），去掉旧的不存在字段 `address`，默认 `type=postgres`、`port=5432`、`schema=public`。
|
||
- **验证**：`go build ./internal/conf/...` 通过；临时测试（已清理）确认 ① 样例 database 段 MapTo 出 `type=postgres`/`port=5432`/`schema=public`；② 空 salt 时 `conf.Init` 返回 salt 错误；③ 非空 salt 时正常加载。
|
||
- **重要陷阱**：`conf.Init` 使用 `IgnoreInlineComment: true`，行内 `;` 注释会被并入配置值——初次加的行内注释曾污染 `database.type` 与 `server.salt`（并使 Salt 校验失效），已全部改回整行注释。原 sample 大量行内注释存在同类系统性污染，建议后续统一改为整行注释（见 MEMORY.md 配置解析约定）。
|
||
|
||
### P1 已修复（2026-08-28）
|
||
- **P1-1 服务反代加固**：`route/service/proxy.go` 剥离转发的敏感 Header（Cookie / Authorization / X-* / Proxy-* / Host 等），并在转发前补审计日志（操作者、目标、路径、大小）；`internal/route/route.go` 将 `/service/{**}` 路由由 `f.Any`（任意方法）收敛为 `f.Get`（仅 GET），消除写方法经反代的安全面。`go build ./route/service/...` 通过。
|
||
- **P1-2 导出接口 N+1 批量化**：`internal/db/follow_ups.go`、`upload_images.go`、`message.go` 新增 `GetByQuestionIDs` / `GetAllByConversationIDs` 批量方法（返回 `map[uint][]*T` 分组）；`route/user/profile.go` 的导出逻辑由"循环单查"改为"IN 查询 + 内存分组"，追问题/图片由每题 2 次、消息由每会话 1 次降为 3 次批量查询。`go build ./internal/db/...` 通过。
|
||
- **P1-3 私信列表分页**：`internal/db/conversation.go` 的 `ListByOwner`/`ListByAsker` 增加 `limit, offset` 参数（导出仍传 `0,0` 取全量，行为不变），并新增 `CountByOwner`/`CountByAsker`；`route/user/question.go` 的「我收到的 / 我发起的」私聊列表接入游标分页（每页 50，复用 `buildQListPageURL` 保留筛选参数），模板 `templates/user/question-list.html` 增加真实总数头与「加载更多」链接。模板解析/渲染测试全部通过。**注意**：`route/user` 在 Windows 因 `github.com/chai2010/webp` 的 CGO 硬依赖无法编译（已知约束，最终在 Linux 完整构建）；逻辑已逐处类型核对无误。
|
||
- **P1-4 高频查询补复合索引**：`internal/db/db.go` 在 `AutoMigrate` 之后调用 `ensureFrequentlyUsedIndexes`（仅 PostgreSQL），以 6 条 `CREATE INDEX IF NOT EXISTS` 幂等建索引：`idx_questions_asker_user_id`、`idx_questions_user_private_created`、`idx_users_show_in_home_created`、`idx_votes_status_visibility_created`、`idx_messages_sender_created`、`idx_messages_conversation_created`。不影响共享 Model，适合既有库。`go build ./internal/db/...` 通过。
|
||
- **P1-5 changelog 同源化**：`dev/gen_changelog.py` 输出路径由根目录 `tamabox.changelogs.json` 改为 `static/tamabox.changelogs.json`；`static/fs.go` 的 `//go:embed` 纳入该文件；`templates/change-logs.html` 的 fetch 由外部域名 `api-direct.337845818.best/...` 改为同源 `/static/tamabox.changelogs.json`，并删除根目录旧副本。`go build ./static/...` + 模板解析测试通过。
|
||
- **验证总览**：可本地构建的包（`internal/conf`、`internal/db`、`static`、`templates`）全部 `go build`/`go test` 通过；`gofmt -l` 对全部改动 Go 文件零输出（已 `gofmt -w` 归一化）；`route/service` 与 `route/user` 因 webp CGO 在 Windows 不可编，依项目约定留待 Linux/CI 完整构建。P1-6（令牌熵）、P1-7（service 层）未在本轮处理，列为后续阶段。
|
||
|
||
### 回归核查（2026-08-28 16:2x）
|
||
逐项复核 P0 + P1-1~P1-5 是否复现或引发新问题。**结论：7 处修复均完好、无复现、无回归。**
|
||
- 验证手段：逐文件 grep 关键标记 + 读 proxy.go/route.go/question.go/profile.go/db.go 改动段 + `gofmt -l`/`go build`/`go vet ./internal/db`/模板解析渲染测试 + 核对 i18n 三语言键（zh_cn/ja/en）。
|
||
- 构建结果：可本地编译包 `build` 退出码 0、`go vet` 0、模板测试 ok、`gofmt -l` 对 11 个改动文件零输出。
|
||
- 各点确认：
|
||
- P0-1 Salt 非空校验在 conf.go:57；P0-2 样例 `[database]` 字段对齐且**无行内注释污染**（IgnoreInlineComment 陷阱未复发）。
|
||
- P1-1 反代剥离敏感头（proxy.go:66-80）+ 审计日志（92-99）+ route.go:292 收敛 `f.Get`。
|
||
- P1-2 三个 store 批量方法签名与实现齐全，profile.go 导出改用批量/分组。
|
||
- P1-3 私信分页：`ChatStats.received/initiated` 来自 `CountByOwner/CountByAsker` **真实总数**（非分页切片长度）；模板「加载更多」键 `qlist.loadMore` 三语言齐全；`strconv.FormatUint(uint64(...))` 类型错误已修。
|
||
- P1-4 6 条 `CREATE INDEX IF NOT EXISTS` 幂等、internal/db 编译通过。
|
||
- P1-5 静态前缀 `/static`（route.go:99）+ embed 含 json + 模板同源 fetch；根副本已删、无残留引用。
|
||
- 一处措辞纠正（非缺陷）：P1-4 `ensureFrequentlyUsedIndexes` 实际**未做 dialect 门控**（无条件执行），但所用 DDL 为 PG 与 MySQL 8.0+ 均支持的 `CREATE INDEX IF NOT EXISTS` 可移植语法，对当前 PG 部署无影响；若将来接 MySQL 仍可用。
|