253 lines
16 KiB
Markdown
253 lines
16 KiB
Markdown
# 2026-09-15 工作日志
|
||
|
||
## 落地 P2 推荐批次 #1–3（审核链路一致性 / 文档）
|
||
|
||
承接 09-11 的全库审阅（无 P0/P1，8 项 P2）。上次征询修复批次未获选择，本次按「继续 +
|
||
我标注的建议先做」自主推进**最低风险的一致性/文档类 #1–3**，未动 #4–8（需另行确认）。
|
||
|
||
### 已改（`internal/security/censor/`）
|
||
|
||
1. **#1 缓存/送审归一化统一**（censor.go:66-68,76,122,131）
|
||
- 原先：缓存按**原文** `text` 查 `GetByText`，实际送审 `RemoveTrustedURL(text)`，落库 `Input:text`。
|
||
→ 审计记录与实际送审内容不符；归一化后相同的原文会各占一条缓存、重复调 API。
|
||
- 现在：循环外算一次 `normalized := RemoveTrustedURL(text)`，缓存查找 / 送审 / `Input` 三者统一。
|
||
- 安全性论证：归一化是纯函数确定性映射；`TrustedHosts` 增减只会让旧条目 miss（重新送审），
|
||
不会让「已通过」的旧缓存被错配到新文本上 → 无漏放风险。
|
||
2. **#2 阿里云缓存回放分支与七牛对齐**（censor.go:99-110）
|
||
- 原先：aliyun 命中缓存**不看 Pass 直接 return**，依赖「aliyun 恒为链尾」这一隐式前提。
|
||
- 现在：与 qiniu 一致 —— 仅 `Pass` 才 return，否则 append + continue 交给下一通道复核。
|
||
- 等价性：aliyun 现为链尾，continue 后循环结束仍由 `responses[len-1]` 返回**同一份响应**，行为不变；
|
||
但消除了顺序耦合，将来若前置 aliyun 也不会短路后续通道。
|
||
3. **#3 修正失真注释**（text.go:66）
|
||
- 原注释称 `Details`「回放缓存结果时为 nil」**是错的**：
|
||
`AliyunTextCensorParser`(aliyun.go:433-461) 与 `QiniuTextCensorParser`(qiniu.go:133-170)
|
||
都会从 `RawResponse` 重建 `Details` → 缓存回放同样受严格度策略约束，改档位对历史缓存也生效。
|
||
- 已改为准确描述。**结论：审核缓存回放无 fail-open**（09-11 的怀疑已排除）。
|
||
|
||
### 验证
|
||
|
||
- `gofmt_keep_crlf.py` 两个文件 exit=0，CRLF 保留（censor 147 / text 141，loneLF=0）。
|
||
- `go build ./internal/security/censor/...` **EXIT=0**（定向构建，~秒级；用 goproxy.cn + overlay）。
|
||
- `go test ./internal/security/censor/` **ok 2.467s** —— 确认严格度策略与回放行为未被破坏。
|
||
|
||
### 顺带已完成 #6
|
||
|
||
- **#6 `User.Email` 改 `json:"-"`**（users.go:89）。
|
||
查证：`route/` 与 `internal/` 中**无任何非测试代码读取序列化后的 `.Email`**（模板里的 email 均为表单字段名 /
|
||
邮件模板，属 Go template 服务端渲染，不受 json tag 影响；输入走 `CreateUserOptions`/`UpdateUserOptions`，
|
||
不经 `User` 模型反序列化）→ 无消费方受损。
|
||
验证：gofmt exit=0，`go build ./internal/db/` **EXIT=0**。
|
||
|
||
### 待确认（P2 #4/#5/#7/#8）
|
||
|
||
| # | 项 | 我的建议 |
|
||
|---|---|---|
|
||
| 4 | 投票配额「插入后计数」非原子（route/vote/vote.go:814-822） | 暂缓：需 DB 层条件插入，超额有界且自动结束自愈 |
|
||
| 5 | ~~profile Name/Intro/link 无长度校验~~ | **误报，已关闭**：表单层已有校验，见下方更正 |
|
||
| 7 | `conf.Database.Schema` 拼 CREATE SEQUENCE（db.go:215-222） | 可选：配置来源非远端输入，纵深防御 |
|
||
| 8 | `RemoveTrustedURL` 24 次整串重建（text.go:114-141） | 可跳过：短文本开销可忽略（#1 已顺带把调用从循环内降到 1 次） |
|
||
|
||
#### ✅ #5 更正：是**误报**，已按「报错式校验」实现后**全部回滚**
|
||
|
||
- 先按用户选择实现了一版（Name 50 / Intro 500 / link 500 rune，4 语 i18n 词条），随后核对
|
||
`internal/form/user.go` 时发现**表单层早已有校验**：
|
||
- `Name` → `valid:"required;maxlen:20"`
|
||
- `Intro` → `valid:"required;maxlen:100"`
|
||
- 各 link → `valid:"maxlen:30"`
|
||
- 且确实生效：`internal/form/form.go:16` 引入 `govalid`，`:131` 在 `Bind()` 中 `govalid.Check()`；
|
||
handler 顶部 `ctx.HasError()` 承接；报错文案走标签 `msg:` 对应的 i18n 键
|
||
（`form.profile.name`="不超过 20 个字符"、`form.profile.intro`=100、`form.profile.linkMax`=30）。
|
||
- 结论：我那版是**永不触发的死代码**（超限在 Bind 阶段就被拒），且上限与真实值冲突 → 已完整回滚
|
||
（profile.go 的 utf8 导入 / 常量 / 校验块 + 4 份字典词条），`git status` 确认 5 个文件零 diff。
|
||
- 教训已写入 MEMORY.md「表单校验（勿重复造轮子）」：查某字段有无校验，**先看 `internal/form/*.go` 的 struct 标签**。
|
||
- 附带修正：Postgres 下 `string` 映射为 `text`（无 DB 层长度上限）这一点本身**没错**，
|
||
但不构成漏洞 —— 因为表单层已经挡住了。
|
||
|
||
### 环境
|
||
|
||
- ~~重包 `./route/user/ ./internal/route/ ./internal/db/` 仍会 30 分钟超时 → 判定为 1C1G 沙箱编译整图慢~~
|
||
**❌ 该结论已被推翻，见文末「编译卡死」误判更正。全库 `go build ./...` 实测 EXIT=0。**
|
||
|
||
---
|
||
|
||
## 「编译卡死」误判更正（用户指出：1C1G 是正式环境，不是沙箱）
|
||
|
||
用户明确纠正：**1C1G 是正式生产环境**，我不该拿「沙箱慢」当挡箭牌解释构建超时。重新取证后确认：
|
||
**根本没有编译问题，是我把代理挂起误判成了编译慢。**
|
||
|
||
### 取证过程
|
||
|
||
| 步骤 | 命令 | 结果 | 结论 |
|
||
|---|---|---|---|
|
||
| 隔离模块解析 | `go list -deps ./route/user/` | **2.3s EXIT=0** | 模块全在本地缓存，网络不是瓶颈 |
|
||
| 编译目标包 | `go build -overlay=... ./route/user/` | **7s EXIT=0** | 之前 240s 超时是伪命题 |
|
||
| 关键包逐个 | route/ internal/route/ question/ auth/ security/... db/ mail/ | **全 OK** | |
|
||
| 全量 | `go build -overlay=... ./...` | **EXIT=0** | |
|
||
| 静态检查 | `go vet -overlay=...` 7 个包 | **EXIT=0** | |
|
||
| 测试 | censor/autoban/ipinfo/route/conf/form | **全部 ok**，EXIT=0 | |
|
||
|
||
**真因**：前几次超时是 `GOPROXY` 未导出 → 卡在 `proxy.golang.org` 连接超时（HTTP=000）。
|
||
与机器性能、与 1C1G 都无关。**只要每条 go 命令前 `export GOPROXY=https://goproxy.cn,direct
|
||
GOSUMDB=off GOFLAGS=-mod=mod` 就秒级完成。**
|
||
|
||
⚠️ 方法论教训：**超时 ≠ 慢**。要用 `go list -deps`（纯元数据、不编译）先把「模块解析」和
|
||
「编译」两层拆开，否则会把网络挂起误判成性能问题，进而拿「环境」当借口。
|
||
|
||
---
|
||
|
||
## 复核：追问邮件 token + 其他严重错误
|
||
|
||
### 1. 追问邮件确实没带 token（用户怀疑成立）
|
||
|
||
- `internal/mail/mail.go:58-65` `SendNewFollowUpMail` 链接 `%s/_/%s/%d`，**无 `?t=`**。
|
||
- 收件人是**箱主** `pageUser.Email`（follow_up.go:131），箱主靠登录态查看 → **不是权限漏洞**。
|
||
- 但有两处真实问题：
|
||
1. **不一致**：同样发给箱主的 `SendNewQuestionMail`（mail.go:25）**带** `?t=`。
|
||
2. **具体失效场景**：若该提问被箱主设为私密（`IsPrivate=true`），箱主在**未登录设备**点邮件链接
|
||
→ `Questioner` 中间件（question.go:61-66）判定 `IsPrivate && !IsLogged && token 不匹配`
|
||
→ **302 回首页，看不到追问**。新提问邮件因带 token 则不会。
|
||
- 建议：给 `SendNewFollowUpMail` 补 token 参数，与 `SendNewQuestionMail` 对齐。
|
||
- 关键的那封（**发给匿名提问者的回答通知**）`question.ReceiveReplyEmail` 是**带 token 的**：
|
||
follow_up.go:185、question.go:214 → `SendNewAnswerMail(..., question.Token)` ✓ 无问题。
|
||
|
||
### 2. 新发现 P2：提问 token 仅 6 位且爆破无限流
|
||
|
||
- `internal/db/questions.go:95` `Token: randstr.String(6)`；`randstr` 用 `crypto/rand` +
|
||
base62 → 62⁶ ≈ 5.68×10¹⁰，**仅 35.7 bit**（随机源没问题，是长度短）。
|
||
- `GET /_/{domain}/{questionID}?t=` **无 rateLimitMiddleware**；猜错由 `Questioner` 返回
|
||
**302 → /**，而 `autoban` 只统计「敏感路径」与「404」（404 默认关闭）→ **完全不计入封禁**。
|
||
- token 仅在「未回答 或 私密」且非箱主时才需要，攻破即=读私密/未公开提问。
|
||
- 可行性：单条定向 ~2.8×10¹⁰ 次不可行；但 questionID 可枚举、只需命中**任意一条**，
|
||
受保护提问 N 条时期望 5.68×10¹⁰/N 次 → 量大时理论可行。
|
||
- 低成本修复：`randstr.String(6)` → `randstr.String(12)`（≈71 bit）。存量 6 位 token 不受影响
|
||
（列是 string，无长度校验）。或对该 GET 路径加限流。
|
||
|
||
### 3. 已复核无问题（勿再重复查）
|
||
|
||
- **密码**（`internal/db/password.go`）：bcrypt cost 10；旧 HMAC-SHA1 用 `subtle.ConstantTimeCompare`
|
||
常量时间比较并透明升级；哈希失败**不降级**为弱哈希（返回空串=锁定）；bcrypt 72 字节截断 ✓
|
||
- **越权**：`Delete/SetPrivate/SetPublic/TogglePin/SetRead/SetUnread` 全部由 `canDelete`
|
||
（question.go:72 `ctx.IsLogged && ctx.User.ID == pageUser.ID`）把关；
|
||
`AnswerFollowUp`(follow_up.go:143) 与 `PublishAnswer`(question.go:154,161) 双重校验 ✓
|
||
（`ctx.Redirect` 会写响应从而终止后续 handler，早退有效）
|
||
- **CSRF**：全局挂载，SessionKey=uid 正确，有回归测试 ✓
|
||
- **会话**：登录 RegenerateID、pwd_ver 失效、HTTPOnly + SameSite=Lax ✓
|
||
- **密码找回**：验证码 24h TTL + 用后即删 ✓
|
||
- **SQL 注入**：全参数化 ✓
|
||
- **token/邮箱泄露**：`Question.Token` 与 `User.Email` 均为 `json:"-"` ✓
|
||
- **审核**：提问/回答/追问/投票/私聊统一走 `moderation.Check` ✓
|
||
|
||
---
|
||
|
||
## 落地 A+B（追问邮件补 token + 提问 token 6→12）
|
||
|
||
按「继续未完成的任务」指示，执行上一条报告的 A+B 两项变更。已用 gofmt_keep_crlf 保 CRLF。
|
||
|
||
### A. 追问邮件（发给箱主）补 token，与 `SendNewQuestionMail` 对齐
|
||
|
||
- `internal/mail/mail.go:58` `SendNewFollowUpMail` 签名加 `token string`，链接改为 `%s/_/%s/%d?t=%s`。
|
||
- `route/question/follow_up.go:131` 调用处补传 `question.Token`。
|
||
- 目的：箱主在**未登录设备**点开**私密**提问的追问邮件时也能凭 token 查看（否则被 Questioner 重定向首页）。
|
||
- 仅一个调用方，构建已验证签名一致。
|
||
|
||
### B. 提问 token 长度 6 → 12
|
||
|
||
- `internal/db/questions.go:95` `randstr.String(6)` → `randstr.String(12)`（≈35.7→71 bit）。
|
||
- 存量 6 位 token 不受影响（列是 string，无长度校验，旧 token 仍有效）。
|
||
|
||
### 验证
|
||
|
||
- `go build ./route/question/ ./internal/mail/ ./internal/db/` → EXIT=0
|
||
- `go build ./...`（overlay）→ EXIT=0
|
||
- `go vet` 三包 → EXIT=0
|
||
- `internal/db` 单元测试 FAIL 为已知环境限制（需真实 PostgreSQL/MySQL，报 `Unknown database type: ""`），
|
||
与本次改动无关，可跳过；security/autoban/censor/ipinfo、route 测试均 ok。
|
||
|
||
### 待定（未做，按需）
|
||
|
||
- CHANGELOG.json 是否补一条 entry（未做：需确认版本号与条目格式）。
|
||
|
||
---
|
||
|
||
## 用本地 Postgres 跑通 db 测试并修真实缺陷（用户提供 127.0.0.1:5432）
|
||
|
||
### 测试库环境变量（`internal/dbutil/testing.go:NewTestDB`）
|
||
- `DB_TYPE=postgres DB_USER=postgres DB_PASSWORD=<pwd> DB_HOST=127.0.0.1 DB_PORT=5432`
|
||
DSN = `postgres://$DB_USER:$DB_PASSWORD@$DB_HOST:$DB_PORT?sslmode=disable`（不带库名，连上后
|
||
自动 `CREATE DATABASE tamabox-test-<rand>`，测试结束（未失败时）DROP）。
|
||
- 仍需 `GOPROXY=https://goproxy.cn,direct GOSUMDB=off GOFLAGS=-mod=mod` + webp overlay。
|
||
|
||
### 修的真实缺陷（生产代码，2 处）
|
||
|
||
- `internal/db/questions.go:DeleteByID`：级联删追问/上传图 + 回退提问配额时**直接调包级全局单例**
|
||
`FollowUps`/`UploadImages`/`FeatureControls`。这些单例只在 `db.Init` 装配 → 测试/部分初始化场景 nil → **panic**。
|
||
改为 `NewFollowUpsStore(db.DB)` / `NewUploadImagesStore(db.DB)` / `NewFeatureControlsStore(db.DB)`，
|
||
用自身 store 的 `*gorm.DB`。生产行为不变（`Init` 里所有 store 共用同一 `*gorm.DB`），但去掉了隐藏全局依赖。
|
||
- `internal/db/conversation.go:MarkDeletedByOwner`：同类问题（`FeatureControls.IncrementQuota`），同样修。
|
||
- 同包 `feature_control.go:355` 的 `FeatureControls.Get` **已有** `if FeatureControls == nil` 守卫，故未动。
|
||
|
||
### 修的测试缺陷（全在 internal/db，非生产 bug）
|
||
|
||
1. `users_test.go`：5 处 `want.EncodePassword(); require.Equal(t, want, got)` —— bcrypt 随机盐，
|
||
哈希**永不相等**。新建 helper `requireUserEqual`：先 `got.Authenticate(明文)` 校验密码，再对齐
|
||
`Password` 与 `ChatMaxMessages`（= `conf.Limits.ChatMaxMessages`，默认 200）后比较其余字段。
|
||
2. `testUsersDeactivate`：复用已被 `testUsersCreate` 占用的 `[email protected]`/`e99` → 改唯一值。
|
||
3. `users_test.go`/`upload_images_test.go` 去掉顶层 `t.Parallel()`：子测试共用一张表、cleanup 会
|
||
`TRUNCATE ... RESTART IDENTITY`，并行时多个子测试拿到相同 id/邮箱 → 撞唯一键。
|
||
4. `upload_images_test.go`：cleanup 只 truncate `upload_images`，而链接表 `upload_image_questions`
|
||
**无 FK 级联**（复合主键 `upload_image_id+question_id`），残留行让下一子测试撞 `_pkey`
|
||
→ 补 `cleanup("upload_images", "upload_image_questions")`。
|
||
5. `UpdateBlockWords/normal`：期望 `HarassmentSettingNone`（"none"）但新用户实际 `""`
|
||
（模型字段无 `gorm:"default:"` 标签）。`""` 与 `"none"` 生产行为等价（只有 `==register_only` 才限制；
|
||
模板 profile.html:851 也只在 `register_only` 时勾选）→ **非生产 bug**。改成先显式
|
||
`UpdateHarassmentType(RegisterOnly)` 再断言未被误改（更强的测试，且不把 `""` 奉为期望）。
|
||
|
||
### 验证
|
||
|
||
- `go build ./...` **EXIT=0**、`go vet ./...` **EXIT=0**
|
||
- `go test ./...`（含 DB）**全绿 EXIT=0**：internal/db 57s、route/vote、internal/route、templates、
|
||
i18n、conf、form、cache、imagecheck、mail、shared、template、security/* 均 ok。
|
||
- 行尾：`internal/db/*` 与 `internal/mail/mail.go` 原本就是 LF，`route/question/*`、`route/vote/vote.go`、
|
||
`internal/route/route.go` 是 CRLF —— **仓库本来就是混合行尾**；本次编辑未改变任何文件原行尾（无 diff 污染）。
|
||
|
||
### 结论
|
||
|
||
- 除上述「全局单例 nil 依赖」（健壮性隐患：测试与部分初始化场景必崩）外，**未发现新的生产逻辑 bug**。
|
||
- db 测试套件此前的一堆失败**全部是测试自身缺陷**：bcrypt 直接比哈希、测试数据撞车、并行共享表、漏清链接表。
|
||
|
||
---
|
||
|
||
## 修复 HarassmentSetting 潜在隐患（schema 默认值 + 启动期归一）
|
||
|
||
用户确认「影响不大就做」。改动面很小（3 处源码 + 1 处测试期望 + 1 个新回归测试）。
|
||
|
||
### 线索
|
||
- 模型 `HarassmentSetting HarassmentSettingType` **无 `gorm:"default:"` 标签** → 新用户该列为空串；
|
||
而规范取值是 none / register_only。全仓库**没有任何代码比较 `== HarassmentSettingNone`**
|
||
（production 只有 `== register_only`，模板 profile.html:851 也只看 register_only）→ **运行期无 bug**，
|
||
但将来若有人写 `== None` 判断就会漏判。
|
||
|
||
### 改动
|
||
1. `internal/db/users.go` 模型：`HarassmentSetting` 补 `gorm:"default:'none'"`。
|
||
⚠️ **字符串默认值必须带单引号**（`default:'none'`），参考既有 `ModerationStrictness` 的 `default:''`；
|
||
裸写 `default:none` 会生成非法 SQL。
|
||
2. `internal/db/users.go:Create`：显式 `HarassmentSetting: HarassmentSettingNone`
|
||
（确定写库，不依赖 DB 默认值回读）。
|
||
3. `internal/db/users.go`：新增 `normalizeHarassmentSetting(db)`，把历史空值（''/NULL）归一为 none，幂等。
|
||
4. `internal/db/db.go:Init`：在 `normalizeChatQuota` 之后调用，失败仅告警不阻断启动
|
||
（与 `backfillFeatureControls`/`reconcileLegacyChatEnabled` 等既有惯例一致）。
|
||
|
||
### 测试
|
||
- `users_test.go` helper `requireUserEqual` 补 `want.HarassmentSetting = HarassmentSettingNone`。
|
||
- 新增 `TestHarassmentSettingNormalize`：新建用户即 none + 历史空值经归一后为 none。
|
||
|
||
### 实测证据
|
||
- 临时探针查 `information_schema`：`harassment_setting` 的 `column_default = "'none'::text"` ✅
|
||
（AutoMigrate 确实把列默认值改成了 none；探针文件跑完即删）。
|
||
- `go build ./...` / `go vet ./...` EXIT=0；`go test ./...`（含 DB）**全绿**；gofmt 无 diff；
|
||
行尾未变（users.go CRLF、db.go/users_test.go LF）。
|
||
|
||
### ⚠️ 新踩到的 gofmt 陷阱
|
||
在**文档注释**里写 `''` 会被 gofmt 重排成 `”`（Go 1.19+ 的 doc comment 规范化：`` `` ``→`“`、`''`→`”`），
|
||
导致 `gofmt -l` 反复报未格式化。**注释里避免使用 `''`**（改写为「空字符串」等文字）。
|