code review 2026-07-25: 安全/正确性修复 (A1–A7)

docs/improvement-plan-2026-07.md → docs/archive/improvement-plan-2026-07.md

@@ -13,7 +13,7 @@ next_item_id: "IMP-010"
 
 # 站点改进计划（2026-07）
 
-> 本文是 ohmywod 应用安全、正确性、可发现性、性能和工程质量的工作计划。维护成本支持与 AdSense 去留以 [站点维护成本支持计划](maintenance-support-plan.md) 为准；备份、恢复和节点替换以 [单机 DR 与节点替换计划](ha-plan.md) 为准。
+> 本文是 ohmywod 应用安全、正确性、可发现性、性能和工程质量的工作计划。维护成本支持与 AdSense 去留以 [站点维护成本支持计划](../maintenance-support-plan.md) 为准；备份、恢复和节点替换以 [单机 DR 与节点替换计划](ha-plan.md) 为准。
 >
 > **当前结论：IMP-001 至 IMP-009 已全部完成。应用安全、正确性、SEO、健康检查、SQLite 锁等待、防滥用和最小 CI 都已有实现与验证；生产已启用共享 Redis sitemap cache，并为战报 HTML 增加安全的条件请求。搜索在 11,515 条公开战报下实测 p95 约 11 ms，因此继续使用简单 LIKE，不引入 FTS；后续只在达到已记录阈值时重新评估性能设计。跨仓主线继续由 HA/DR 计划维护。**
 

docs/cdn-improvement-plan.md

@@ -7,13 +7,13 @@ language: zh-CN
 created_at: "2026-07-24"
 last_updated: "2026-07-24"
 review_commit: "907babd"
-review_worktree: "dirty: existing docs/ha-plan.md changes preserved"
+review_worktree: "dirty: existing docs/archive/ha-plan.md changes preserved"
 next_item_id: "CDN-003"
 ---
 
 # 国内访问与 CDN 改进计划（未来）
 
-> 本文是战报网未来 CDN 与国内访问体验优化的工作计划，不是多地域部署方案。节点替换与恢复以 [单机 DR 与节点替换计划](ha-plan.md) 为准，已经完成的应用缓存与安全基线以 [站点改进计划](improvement-plan-2026-07.md) 为准。
+> 本文是战报网未来 CDN 与国内访问体验优化的工作计划，不是多地域部署方案。节点替换与恢复以 [单机 DR 与节点替换计划](archive/ha-plan.md) 为准，已经完成的应用缓存与安全基线以 [站点改进计划](archive/improvement-plan-2026-07.md) 为准。
 >
 > **当前结论：继续使用 Cloudflare 橙云和东京源站，不增加国内节点或更换 CDN。下一步最有价值的两项小改进是：让公开且基本不可变的 `/r/raw/*` 战报 HTML 在 Cloudflare 边缘缓存 1 天；缩小每页都会加载的 1024×1024、约 956 KB logo。两项可以并行。metadata 页面、登录态页面和互动接口继续不缓存。**
 

docs/codereview/2026-07-25-cross-review-antigravity.md

@@ -0,0 +1,114 @@
+# `ohmywod` 代码评审与三方交叉评估
+
+- **日期**：2026-07-25
+- **评审者**：Antigravity
+- **评审基线**：`ohmywod` 仓库代码
+- **交叉评审对象**：
+  - `2026-07-25-review.md`（Claude / Opus 4.8）
+  - `2026-07-25-cross-review-codex.md`（Codex）
+- **定位与标准**：个人兴趣项目，以**实用、清晰、高性价比（投入产出比）**为唯一衡量标准，不搞工业级形式主义。
+
+---
+
+## 总体评估与结论
+
+针对 `ohmywod` 应用仓，综合分析 Claude 与 Codex 的评审报告并核对源码后，**综合结论如下**：
+
+1. **Claude 的初评**抓住了基础安全与运维隐患（Open Redirect、Zip 暂存堆积、用户名大小写），但**漏掉了两个极其关键的路径逃逸与数据逻辑缺陷**。
+2. **Codex 的二次交叉评审非常精准**，通过深入源码发现了用户名/分类名路径逃逸、以及软删除对 raw/ID 视图失效的严重问题，并指出了 Claude 推荐修复函数在特定依赖版本不存在的问题。
+3. **Antigravity 在源码层面的验证结果**：完全证实 Codex 提出的两大核心风险。针对个人兴趣项目，无需引入复杂校验库，仅需增加几处极简的字符限制与条件过滤即可解决 95% 以上的隐患。
+
+目前测试集中为 **`67 passed, 180 warnings`**（180 项均为 Python/SQLAlchemy 废弃告警）。项目整体架构简洁清晰，只需针对以下几点做极小成本的收尾。
+
+---
+
+## 三方交叉对比与逐项裁决
+
+### 1. [高风险] 用户名与分类名可能导致文件路径逃逸出 `DATA_DIR` / `UPLOAD_DIR`
+
+- **代码位置**：
+  - `ohmywod/views/frontend.py:103-109`（注册用户名校验）
+  - `ohmywod/views/report.py:511-518`（新建分类名校验）
+  - `ohmywod/views/upload.py:75-103`（解压与上传落盘路径 `Path(DATA_DIR) / category.owner / category.name / filename`）
+- **问题分析**：
+  - **Claude 判定**：漏报。仅关注了 Zip 内部成员的 Zip-Slip 逃逸。
+  - **Codex 判定**：**正确捕获**。在 Python 中，`Path('/var/data') / owner / '/tmp/abc'` 遇上以 `/` 开头的绝对路径时，前面的根目录会被直接丢弃；包含 `..` 时会向上级目录逃逸。加上生产环境 Gunicorn 以 root 运行，风险极其显著。
+  - **Antigravity 评估**：**完全同意 Codex**。即使 Zip 成员文件名安全，根目录逃逸也能让文件落到系统任意位置。
+- **实用修法（最简且有效）**：
+  - 在注册 `validate_username` 和创建分类 `validate_name` 表单校验中，加入正则限制：仅允许字母、数字、中文、下划线、中划线和空格，拒绝包含 `/`、`\`、`..` 或绝对路径。
+  - 在 `upload.py` 写入前加一行兜底断言：`tpath.resolve().is_relative_to(Path(data_dir).resolve())`。
+
+---
+
+### 2. [中高风险] 战报/分类“删除”后，已有链接与 `/raw/` 路径仍可被公开访问
+
+- **代码位置**：
+  - `ohmywod/controllers/report.py:50-75`（删除仅置 `status = 1`）
+  - `ohmywod/views/report.py:145-189`（`/raw/...` 完全不查数据库，只看磁盘文件）
+  - `ohmywod/views/report.py:192-210`, `:325-342`（按 id 获取分类/战报时不过滤 `status`）
+- **问题分析**：
+  - **Claude 判定**：漏报。
+  - **Codex 判定**：**正确捕获**。用户界面显示“删除”，但拿着旧 ID 链接或 raw 路径依然能看内容，属于符合直觉的逻辑 BUG。
+  - **Antigravity 评估**：**完全同意 Codex**。个人分享站的删除诉求通常就是“让分享链接失效”。
+- **实用修法**：
+  - `ReportController.get_report` / `get_category` 默认增加 `status == None` 过滤。
+  - `/raw/<username>/<category>/<name>/` 路由增加一次极轻量的 DB 状态查询，若分类或战报已置为已删除（`status == 1`），直接 `abort(404)`。
+
+---
+
+### 3. [中风险] 上传 Zip 暂存文件成功后未删除，坏 Zip 抛 500 且遗留垃圾
+
+- **代码位置**：
+  - `ohmywod/views/upload.py:85-128`
+  - `tests/test_views.py:403-408`, `:438-442`
+- **问题分析**：
+  - **Claude 判定**：正确发现成功路径没有删除 `UPLOAD_DIR` 中的 zip，磁盘满会导致全站无法上传。
+  - **Codex 判定**：同意 Claude，并补充指出：现有 `test_views.py` 测试断言中错误地将“Zip 必须保留”写成了预期；且未捕获 `BadZipFile` 异常。
+  - **Antigravity 评估**：**三方达成一致**。
+- **实用修法**：
+  - 使用 `try...finally` 结构，确保解压流程结束后无条件执行 `fpath.unlink(missing_ok=True)`。
+  - 显式捕获 `zipfile.BadZipFile`，向前端返回 400 错误提示，而不是触发 500 服务器崩溃。
+  - 同步修改 `test_views.py` 中对应的断言，使其符合删除 Zip 的正确预期。
+
+---
+
+### 4. [低风险] 登录 `next` 参数存在开放重定向（Open Redirect）
+
+- **代码位置**：
+  - `ohmywod/views/frontend.py:67-69`
+- **问题分析**：
+  - **Claude 判定**：正确指出 `/login?next=https://evil.com` 风险，但建议使用 `flask_login.utils.url_has_allowed_host_and_scheme`。
+  - **Codex 判定**：指出 **Flask-Login 0.6.3 并没有该函数**，建议直接判断 `next.startswith('/') and not next.startswith('//')`。
+  - **Antigravity 评估**：**同意 Codex 的纠错**。单行字符串判断最干净可靠，无需依赖第三方 API。
+- **实用修法**：
+  - 跳转前检查 `if next_url and next_url.startswith('/') and not next_url.startswith('//'):` 允许跳转，否则回首页。
+
+---
+
+### 5. [低风险] 用户名大小写重名导致登录歧义
+
+- **代码位置**：
+  - `ohmywod/controllers/user.py`（注册精确匹配，登录 `lower().first()`）
+- **三方共识**：
+  - 属于极边角逻辑矛盾。按 Claude/Codex 共同建议：注册查重时改为不区分大小写的 `get_by_login` 逻辑即可，堵住新数据，不需去动 SQLite 存量数据库。
+
+---
+
+### 6. 其余低优先级项目总结
+
+| 事项 | 三方评估结论 | 建议处理方式 |
+|---|---|---|
+| `per_page` 无上限 | 三方一致认定为小瑕疵 | 在 `utils.py` 中加上 `min(per_page, 100)` |
+| Zip 解压展开体积限制 | Codex 提出解压配额防护 | 属于防御性加固，有空可加，优先级不高 |
+| Admin HTTP Basic & CSRF 豁免 | 三方一致认定：个人单人运维，**不必管** | 维持现状 |
+| `datetime.utcnow()` 警告 | 仅属于 Python 升级告警，**不必管** | 维持现状 |
+
+---
+
+## 推荐的最小动作清单（按优先级排序）
+
+1. **[必做] 堵住路径逃逸**：对用户名和分类名格式做表单字符限制（禁止 `/`, `\`, `..`）。
+2. **[必做] Zip 清理与坏包处理**：`upload.py` 用 `finally` 清理 zip，捕获 `BadZipFile` 返回 400，更新单元测试。
+3. **[必做] 修复删除生效逻辑**：`get_report`/`get_category` 和 `/raw/` 增加 `status` 过滤。
+4. **[顺手做] 修复 `next` 重定向**：使用 `startswith('/') and not startswith('//')`。
+5. **[顺手做] 修复注册用户名大小写查重**。

docs/codereview/2026-07-25-cross-review-codex.md

@@ -0,0 +1,129 @@
+# `ohmywod` 代码评审与交叉评估
+
+- 日期：2026-07-25
+- 评审者：Codex
+- 基线：`3cf35a4`
+- 交叉评审对象：`2026-07-25-review.md`（Claude / Opus 4.8）
+- 标准：个人兴趣项目，优先实际正确性、简单性和清晰度；不按工业级规范堆加固项。
+
+## 结论
+
+项目整体不差：认证已迁到 SQLite，密码方案稳妥，CSRF、限流、上传 HTML 的 CSP sandbox、缓存降级和测试覆盖都比较实用。现有测试在本次评审中实测为 **67 passed**。
+
+不过，已有评审漏掉了两个比代码风格更实际的问题：
+
+1. 用户名和目录名直接进入文件系统路径，可让上传落到配置目录之外；
+2. UI 所说的“删除”只从列表隐藏，拿着旧链接仍能访问战报和文件。
+
+建议先修下面前四项；其余可以按空闲时间处理。
+
+## 建议优先修
+
+### 1. [高] 用户名/目录名可把上传路径带出 `UPLOAD_DIR` 和 `DATA_DIR`
+
+相关代码：
+
+- `ohmywod/views/frontend.py:103-109`：用户名只做重复校验；
+- `ohmywod/views/report.py:511-518`：目录名只做重复校验；
+- `ohmywod/views/upload.py:75-103`：二者未经路径约束便参与 `mkdir`、zip 保存和解压路径。
+
+例如目录名可以是绝对路径或包含 `../`。`Path(DATA_DIR) / owner / category.name` 遇到绝对的 `category.name` 时会直接丢掉前面的根目录；包含 `..` 时也会向上逃逸。ops 仓还明确说明 Gunicorn 以 root 运行，因此影响不只是“某个用户目录写乱”，而是已注册用户能在配置根目录之外创建目录、保存 zip 和解压文件。
+
+这与 zip 成员的 Zip Slip 检查是两件事：成员名可能安全，但解压目标目录本身已经不安全。
+
+实用修法：
+
+- 对用户名和目录名做统一的“单一路径段”校验：允许中文和空格，但拒绝 `/`、`\`、绝对路径、`.`、`..` 和控制字符；
+- 写文件前再做一次兜底：对目标 `resolve()` 后确认仍位于配置根目录下；
+- 为 `/tmp/x`、`../x`、`a/b`、`a\b` 各补一个回归测试。
+
+不要悄悄替换这些字符，因为数据库中的名字、URL 和磁盘路径会因此不一致；表单直接提示“不允许包含路径分隔符”更清楚。
+
+### 2. [中] “删除”后旧链接仍能访问
+
+相关代码：
+
+- `ohmywod/controllers/report.py:50-75`：删除仅设置 `status = 1`；
+- `ohmywod/views/report.py:192-210`、`:325-342`、`:444-491`：按 id 读取时只检查对象是否存在，不检查 `status`；
+- `ohmywod/views/report.py:145-189`：`/raw/...` 完全不查询数据库，只要文件还在就能访问。
+
+因此删除战报或目录后：
+
+- 列表和 sitemap 中看不到；
+- `/r/report/<id>`、reader 和已知 raw URL 仍然可用；
+- 删除目录时，目录下所有战报也有同样情况。
+
+UI 明确写的是“删除”，404 页面也写“或者已经被删除”，所以这更像行为错误，而不是有意做“仅隐藏”。对分享站尤其实际：用户通常会认为删除后旧分享链接失效。
+
+简单方案是让所有公开读取路径只接受 active 的 category/report，并让 raw 路径也校验对应的 active 数据库记录。文件可以先保留，方便误删恢复；若本意确实只是隐藏，则应把按钮文案改为“从列表隐藏”，避免误解。
+
+### 3. [中] 上传 zip 被当作“临时文件”，但成功后永久保留；坏 zip 还会绕过清理
+
+相关代码：
+
+- `ohmywod/config.py:27-31` 明确说 `UPLOAD_DIR` 只是本地 staging；
+- `ohmywod/views/upload.py:85-128` 成功路径不删除 `fpath`；
+- 异常只捕获 `OSError`，`BadZipFile` 等解析错误会返回 500，并留下 zip 或部分目标目录；
+- `tests/test_views.py:403-408`、`:438-442` 反而把“zip 必须保留”写进了测试。
+
+所以 Claude 提出的“zip 会越积越多”判断成立，但修复时不能只加一行 `unlink()`：现有测试也要改，而且清理应覆盖成功、坏 zip 和数据库写入失败等路径。
+
+够用的修法：
+
+- 用 `finally` 清理 staging zip；
+- 显式捕获 `BadZipFile`，给用户 400，而不是 500；
+- 解压失败时清理本次创建的目标目录；
+- 如果确实想保留原始 zip 作为备份，就别再称它为 staging，并补一个保留周期/清理脚本。以当前注释和磁盘阈值看，删除更符合原设计。
+
+### 4. [中] 登录后的 `next` 是开放重定向
+
+`ohmywod/views/frontend.py:67-69` 直接把查询参数传给 `redirect()`，`/login?next=https://example.invalid` 登录成功后会跳到站外。
+
+Claude 的问题判断正确，但给出的修复 API 不适用于当前依赖：本项目的 Flask-Login 0.6.3 **没有** `flask_login.utils.url_has_allowed_host_and_scheme`。
+
+个人项目无需引入复杂依赖，接受“以单个 `/` 开头且不以 `//` 开头”的本站相对路径即可；不满足就回首页。顺便给 `https://...`、`//example...` 和 `/r/` 补三个测试。
+
+## 建议顺手修
+
+### 5. zip 解压没有限制展开后总大小
+
+`ohmywod/views/upload.py:88-103` 只受上传 zip 本身大小和磁盘占用阈值约束，没有检查 `ZipInfo.file_size` 总和、成员数或单文件大小。很小的高压缩比 zip 仍可展开成很大的 JuiceFS 数据；站点允许公开注册，这比单纯设置 nginx `client_max_body_size` 更值得防。
+
+不必做复杂流式配额。解压前对成员数和 `sum(info.file_size)` 设一个符合真实战报大小的上限，超出返回 400，已经足够。
+
+### 6. 用户名大小写规则前后不一致
+
+`User.username` 的 SQLite 唯一约束默认区分大小写；注册查重用精确匹配，而登录在 `ohmywod/controllers/user.py:25-30` 用 `lower(...).first()`。于是 `Bob` 和 `bob` 可同时注册，之后登录命中哪个取决于查询结果。
+
+Claude 的判断和“先堵新数据，不急着迁移”的建议都合理：注册校验改用不区分大小写的查询，并先用一次离线查询确认存量没有冲突即可。
+
+### 7. `per_page` 没有上限
+
+`ohmywod/utils.py:11-15` 和 search 都接受用户传入的 `per_page`。加一个 100 左右的上限是便宜且清楚的保护。数据量不大时不紧急。
+
+## 对已有 Claude 评审的逐项评估
+
+| 原结论 | 我的评估 |
+|---|---|
+| Open Redirect | **同意问题，修复函数需更正**：当前 Flask-Login 没有所建议的 helper。 |
+| 成功上传后 zip 不删除 | **同意**，并补充：测试当前固化了错误行为，坏 zip 也不会进入现有清理分支。 |
+| 用户名大小写重名 | **同意**，建议按其“只堵新增”方案处理。 |
+| `per_page`、死代码、整串替换 `http:` | **同意优先级较低**。 |
+| 当前 Zip Slip 检查无需处理 | 对“zip 成员名”本身基本同意；但原评审漏掉了更外层的 username/category 目标路径逃逸，这是本次最高优先级问题。 |
+| Admin Basic、CSRF 豁免、非 ASCII Basic Auth、弃用告警 | **同意暂不投入**，符合个人项目尺度。 |
+| BBCode、CSP sandbox、密码迁移等做得好 | **基本同意**；本次未发现需要推翻这些结论的证据。 |
+| “真正建议动手的只有三条” | **不同意**：路径逃逸和删除后仍可访问至少应进入同一优先级。 |
+
+## 验证记录
+
+- `.venv/bin/python -m pytest -q -s`：`67 passed, 180 warnings`；
+- 180 条均为现有弃用警告，未发现测试失败；
+- 直接运行 `.venv/bin/pytest` 在当前执行环境遇到项目导入/pytest 临时捕获文件问题；改用虚拟环境 Python 并关闭捕获后测试全部通过。这是本次工具环境差异，不判为应用缺陷。
+
+## 建议的最小处理顺序
+
+1. 限制 username/category 为单一路径段，并加最终 containment 校验；
+2. 统一上传清理，顺便处理坏 zip 和展开大小上限；
+3. 明确“删除”的语义，让旧详情、reader、raw 链接失效；
+4. 修 `next` 与用户名大小写查重；
+5. 有空再加分页上限、删死代码。