docs: 新增第 2 轮评审报告、D36 决策记录与 serve 旧词收尾
review-round2.md 全量修复清单；decisions.md 澄清 D6、新增 D36；design/acceptance 残留 serve → start（P2-14）。
This commit is contained in:
@@ -0,0 +1,128 @@
|
||||
# 《Pure Note v1.1 实现》审评报告（第 2 轮）
|
||||
|
||||
> 审评对象：docs/design.md v1.1 的完整实现（Go 后端 + React 前端 + 构建/部署链），含二进制/子命令改名（pure-note → pn）后的全量代码
|
||||
> 审评日期：2026-09-08
|
||||
> 审评方法：三路并行代码评审（store/数据层、httpapi/中间件、前端），关键发现全部人工复核源码与文档（decisions.md D6 等）确认后成稿
|
||||
> 基线：docs/review-round1.md（v1.0 设计评审）——其 P0/P1 修复要求已在 v1.1 设计中落地，本轮核对实现是否兑现
|
||||
|
||||
---
|
||||
|
||||
## 一、总体结论
|
||||
|
||||
**无 P0。** 默认配置下未发现远程可利用的越权或注入漏洞：SQL 全部参数化、可见性矩阵（列表/详情/标签/RSS/sitemap/图片/meta 注入）落实到位、CSRF 双保险（token + Origin/Referer 严格模式，覆盖 login/logout）、`__Host-` Cookie、PHC 口令哈希与会话摘要、meta 注入经 html/template 转义——round1 的 4 个 P0 均已在实现层封死。
|
||||
|
||||
但存在 **5 个 P1 功能缺陷**：其中 1 个导致前端功能完全不可用却被测试漏过（图片上传）、1 个邻接导航查询逻辑写反、1 个文档承诺（D6）与实现矛盾（升级/回滚场景备份不可用）。另有 10 项 P2 加固项。
|
||||
|
||||
| 级别 | 定义 | 数量 |
|
||||
| --- | --- | --- |
|
||||
| P0 | 远程可利用的越权/注入/私密内容泄露 | **0** |
|
||||
| P1 | 功能缺陷/契约破坏/文档承诺与实现矛盾 | **5** |
|
||||
| P2 | 加固与健壮性 | **10**（表内合并条目） |
|
||||
|
||||
---
|
||||
|
||||
## 二、发现汇总
|
||||
|
||||
| # | 级别 | 位置 | 问题一句话 |
|
||||
| --- | --- | --- | --- |
|
||||
| 1 | P1 | internal/store/notes.go:188-210 | 上一篇/下一篇邻接查询 tie-break 写反且忽略 pinned，同秒更新漏链/错链，无测试覆盖 |
|
||||
| 2 | P1 | web/src/lib/api.ts:39-41 | api() 对 FormData 强制 `Content-Type: application/json`，前端图片上传完全不可用 |
|
||||
| 3 | P1 | internal/httpapi/server.go:102 | `/api/admin/*` 与 `/api/me` 缺 `Cache-Control: no-store`（中间件注释宣称已覆盖） |
|
||||
| 4 | P1 | web/src/pages/AdminEdit.tsx:113 | 自动保存成功后 invalidate 连详情查询一起失效，refetch 回滚正在输入的内容并抑制下一轮自动保存 |
|
||||
| 5 | P1 | cmd/pn/main.go:241/316/340 | 维护子命令总是迁移且带守卫、`--allow-newer` 被解析但忽略，与 D6 矛盾；回滚场景连备份都无法执行 |
|
||||
| 6 | P2 | store/maint.go:36-98 | GC SELECT 与 DELETE 之间竞态：恢复/新引用的活数据会被物理删除 |
|
||||
| 7 | P2 | store/maint.go:109-119 | 备份先以默认 umask 创建后 chmod 0600，私有内容短暂暴露窗口 |
|
||||
| 8 | P2 | auth/auth.go:57-68 | VerifyPassword 不校验 PHC 参数上下界，畸形哈希可 panic/OOM |
|
||||
| 9 | P2 | httpapi/public.go:161-174 | 图片数据行竞态缺失时 404 携带 public immutable 一年 |
|
||||
| 10 | P2 | httpapi/feed.go:52-75 | 无公开笔记时 lastBuildDate 输出公元 1 年（非法 RSS 日期） |
|
||||
| 11 | P2 | middleware/ratelimit.go:41 | sameOrigin 只比 host 不比 scheme |
|
||||
| 12 | P2 | httpapi/admin.go:344-349 | image.Decode 无尺寸上限，<5MB 解压炸弹可 OOM 整站 |
|
||||
| 13 | P2 | store/settings.go:64-67 | Sscanf("%d") 部分解析（"10abc"→10），脏 page_size 被静默采用 |
|
||||
| 14 | P2 | docs（design:567/590/628、acceptance:53、decisions D6/D28） | 改名后残留 5 处 `serve` 旧词；D6 内容与实现矛盾（见 P1-5） |
|
||||
| 15 | P2 | web/src 多处 | Home 硬编码 page_size 使站点设置失效；auth.tsx 登录竞态覆盖 CSRF；401 无全局跳转；公共缓存失效缺失等 |
|
||||
|
||||
---
|
||||
|
||||
## 三、P1 详细说明
|
||||
|
||||
### P1-1 上一篇/下一篇邻接查询 tie-break 写反（store/notes.go:188-210）
|
||||
|
||||
**问题**：公开列表序为 `ORDER BY pinned DESC, updated_at DESC, id DESC`（notes.go:169）。邻接语义应在 (updated_at, id) 字典序上取紧邻项，但：
|
||||
|
||||
- prev（列表中更早一篇）写 `(updated_at > ? OR (updated_at = ? AND id < ?)) ORDER BY updated_at ASC, id DESC`——`id` 比较方向与列表序（id DESC）相反；同秒组内 `id DESC` 取到的是组内顶端而非紧邻项；
|
||||
- next 同理（`id > ?` + `id ASC`）。
|
||||
|
||||
**后果**：同一秒有两篇以上笔记更新/创建（个人站点连续保存极常见，updated_at 为秒级粒度）时 prev/next 漏链或错链；且查询完全不参与 pinned 排序，置顶笔记附近跳错条目。调用方 public.go:103-104 用 `_` 吞掉错误，静默失效。该函数**无任何测试覆盖**（已确认）。
|
||||
|
||||
**修复**：prev 改为 `(updated_at > ? OR (updated_at = ? AND id > ?)) ORDER BY updated_at ASC, id ASC LIMIT 1`；next 改为 `(updated_at < ? OR (updated_at = ? AND id < ?)) ORDER BY updated_at DESC, id DESC LIMIT 1`；补表驱动测试（含同秒多篇、置顶参与排序的语义确认，必要时把 pinned 纳入邻接定义）。
|
||||
|
||||
### P1-2 前端图片上传完全不可用（web/src/lib/api.ts:39-41）
|
||||
|
||||
**问题**：`api()` 对所有带 body 的请求无条件补 `Content-Type: application/json`。Editor.tsx:22 的 `uploadImage` 用 FormData（注释明确「让浏览器自动设置 Content-Type（含 boundary）」），被此逻辑覆盖为 application/json → 后端 `ParseMultipartForm` 失败 → 400，粘贴/拖拽上传功能不可用。
|
||||
|
||||
**为什么测试全绿**：Go 集成测试直连 API 手工构造正确 multipart 头；验收浏览器走查未覆盖上传动作；前端无该路径的测试。
|
||||
|
||||
**修复**：`if (options.body && !(options.body instanceof FormData) && !headers.has('Content-Type'))`；补一个前端测试或至少把上传动作纳入浏览器走查清单。
|
||||
|
||||
### P1-3 /api/admin/* 与 /api/me 缺 no-store（server.go:102、public.go:21-28）
|
||||
|
||||
**问题**：middleware.go:129-136 注释宣称 NoStore「为 /api/admin/* 与 /api/auth/* 响应统一附加」，实际接线仅 `/api/auth/`（server.go:86）挂了 NoStore，`mux.Handle("/api/admin/", s.requireAdmin(adminMux))`（server.go:102）没有。`/api/me`（已认证时返回 csrf_token）同样无。
|
||||
|
||||
**后果**：登出后经 bfcache/历史回退（共用设备场景）可回看管理数据（笔记全文、回收站列表、settings、CSRF token）；与设计 §9.2 目标直接冲突。
|
||||
|
||||
**修复**：`mux.Handle("/api/admin/", middleware.NoStore(s.requireAdmin(adminMux)))`；`/api/me` 已认证分支响应前补 `Cache-Control: no-store`（或对 /api/me 整路由统一 no-store）。
|
||||
|
||||
### P1-4 自动保存竞态回滚用户输入（AdminEdit.tsx:108-116、62-76）
|
||||
|
||||
**问题**：保存成功 `qc.invalidateQueries({ queryKey: ['admin'] })` 前缀匹配连当前编辑中的详情查询（key 形如 `['admin','note',id]`）一起失效 → refetch 返回服务端快照 → `useEffect([existing])` 执行 `setForm(...)` 把保存期间用户继续输入的内容回滚，并把 `dirtyRef.current = false`，抑制下一轮自动保存。每次自动保存都经历一次「输入被吞」窗口。
|
||||
|
||||
**修复**：onSuccess 仅 invalidate 列表键（如 `['admin','notes']`）与回收站键，不失效当前详情；或 refetch 落地时比较 `updated_at`/内容未变才允许重置 form。
|
||||
|
||||
### P1-5 维护子命令与 D6 矛盾：迁移与守卫始终执行、--allow-newer 被忽略（cmd/pn/main.go:241/316/340）
|
||||
|
||||
**问题**：decisions.md D6 承诺「backup/gc 不执行迁移、不做 user_version 守卫（纯数据操作）」，但实现中 passwd/backup/gc 一律 `store.Open(cfg.DBPath(), false)`，而 `store.Open` 无条件 `migrate()`（store.go:105）且 `false` 关闭 `--allow-newer`。`config.ParseMaint` 解析了 `--allow-newer` 却从未使用。
|
||||
|
||||
**后果**：
|
||||
1. 用户升级到更高 schema 后回滚旧二进制，**`pn backup` 直接被守卫拒绝**——与「升级 SOP 第一步先备份」（review-round1 P1-13 修复）的语义冲突；
|
||||
2. 错误 `--data-dir` 会静默创建并迁移出一个新库（维护命令不再是无副作用的纯数据操作）；
|
||||
3. 传入 `--allow-newer` 无任何效果且无提示。
|
||||
|
||||
**修复**：按 D6 拆分「打开 + 迁移（serve/init）」与「仅数据操作打开（passwd/backup/gc，跳过迁移与守卫）」，或至少让三个子命令真正透传 `cfg.AllowNewer`；同步修订 D6/代码注释，消除文档与实现二选一的漂移。
|
||||
|
||||
---
|
||||
|
||||
## 四、P2 简述
|
||||
|
||||
| # | 位置 | 修复建议 |
|
||||
| --- | --- | --- |
|
||||
| 6 | store/maint.go:36-98 | GC commit 阶段 DELETE 带原条件复查：笔记 `WHERE id=? AND deleted_at < ?`，图片 `DELETE ... WHERE id=? AND NOT EXISTS(SELECT 1 FROM image_refs WHERE image_id=?)`（或整体单事务）——防 SELECT 与 DELETE 之间恢复/新引用导致活数据被物理删除 |
|
||||
| 7 | store/maint.go:109-119 | 备份文件以 0600 创建（先 `os.OpenFile(O_CREATE|O_EXCL, 0600)` 占位或临时目录生成后 rename），消除「先以 umask 创建后 chmod」的暴露窗口与存在性竞态 |
|
||||
| 8 | auth/auth.go:57-68 | 解析 PHC 后校验 `1 ≤ t ≤ 10`、`1 ≤ p ≤ 8`、`m ≤ 1<<20`（KiB）、`len(want) == 32`，不合法直接返回 false——防 t/p=0 触发 argon2 panic、m 巨大 OOM（库被篡改场景） |
|
||||
| 9 | httpapi/public.go:161-174 | 缓存头移到 `GetImageData` 成功之后设置；数据行缺失的 404 不带 public immutable（防恢复后仍被缓存 404 一年） |
|
||||
| 10 | httpapi/feed.go:52-75 | 无公开笔记时 lastBuildDate 用当前时间或省略字段，不输出 `Mon, 01 Jan 0001` |
|
||||
| 11 | middleware/ratelimit.go:41 | `sameOrigin` 同时比较 scheme（或至少把 http/https 视为不同源）；与 HSTS 形成双层防护 |
|
||||
| 12 | httpapi/admin.go:344-349 | 解码校验前先 `image.DecodeConfig` 校验宽高/像素总数上限（如 ≤8192×8192 或总像素 ≤2^26），防 <5MB 高压缩图解压炸弹 OOM |
|
||||
| 13 | store/settings.go:64-67 | `Sscanf("%d")` 后校验「串已完整消费」（如 `%d%c` 探尾或 strconv.Atoi 全文），拒 "10abc" 类脏值 |
|
||||
| 14 | docs 多处 | 改名收尾：design.md:567/590/628、acceptance.md:53 的 `serve` → `start`；decisions.md D6/D28 同步（D28 的 `serve` 改 `start`；D6 待 P1-5 修复后按新行为重写） |
|
||||
| 15 | web/src 多处 | Home.tsx:10 改用 `/api/site` 的 page_size；auth.tsx:26-45 给 refresh 加请求序号/登录态变化判定，丢弃迟到匿名响应（StrictMode 双挂载放大窗口）；api.ts 401 回调接入 AuthContext 触发跳转；写操作成功后同步 invalidate `['notes']`/`['tags']`/`['note']` 公共键；TagPage 补 error 分支；上传路径加前端测试 |
|
||||
|
||||
---
|
||||
|
||||
## 五、核实通过（维持不动）
|
||||
|
||||
1. **可见性单一可信点**：列表/标签/RSS/sitemap 统一走 `PublicNoteFilter`（store/notes.go:152），详情/图片/meta 出口 status + deleted_at 双层裁决，私有与不存在统一 404 防枚举，id 越界/负数均 404；
|
||||
2. **SQL 注入面**：全部 `?` 参数化；仅 `VACUUM INTO` 路径经 `escapeSQLString`（引号加倍，SQLite 语义正确）与内部常量 PRAGMA 两处拼接，均无注入面；
|
||||
3. **认证与 CSRF**：`__Host-` + Secure + HttpOnly + Lax、登录重建会话防固定、Origin/Referer 严格模式（缺头即拒）覆盖 login/logout、token 仅内存、改密校验旧口令且会话保持；
|
||||
4. **口令与令牌**：Argon2id PHC 串（参数随哈希走）、会话库存 SHA-256 摘要、常量时间比较；
|
||||
5. **上传防线**：声明类型白名单 → 魔数（SVG 拒绝）→ PNG/JPEG/GIF 再解码 → ≤5MB 哨兵 → sha256 去重；
|
||||
6. **输出转义**：meta 注入经 html/template、JSON 默认 HTML 转义、RSS 经 encoding/xml + goldmark(非 unsafe) + bluemonday 双层清洗；
|
||||
7. **迁移与并发**：user_version 上界守卫、仅追加式、DDL 与版本号同事务幂等；WAL + busy_timeout(5000) + synchronous(NORMAL) + foreign_keys(1) + `SetMaxOpenConns(1)`；
|
||||
8. **构建/部署链**：Vite `base:'/'` + sync-assets + 指纹缓存 + 冒烟脚本（19 项断言）全链路打通；
|
||||
9. **重命名**：pure-note → pn 的代码/脚本/deploy/README 残留已清理干净，本轮仅剩 docs 中 5 处 `serve` 旧词（P2-14）；
|
||||
10. **测试资产**：可见性矩阵、CSRF、限流、上传、回收站/gc、slug、设置白名单、meta 转义、迁移守卫均有自动化用例；`go vet`/`go test`/vitest/`make smoke` 全绿。
|
||||
|
||||
---
|
||||
|
||||
## 六、结论
|
||||
|
||||
v1.1 实现的骨架与安全设计经受住了第二轮对抗性评审，round1 的全部 P0 修复要求已在实现层兑现，无新增 P0。需优先处理的是 5 个 P1：其中 P1-1（邻接导航）与 P1-2（图片上传）是用户可直接感知的功能性缺陷，P1-3（no-store）与 P1-5（维护命令契约）是安全/运维承诺的缺口，P1-4（自动保存竞态）影响核心编辑体验。P1 修复后建议随本轮把 P2 清单中低成本项（8/9/10/13/14）一并落地，并补上 P1-1 的邻接查询测试。
|
||||
Reference in New Issue
Block a user