host
6525bcafd0
Fix F1 name race with partial unique index, F2 pause/end mid-poll race with guarded updates, F3 redirect/numeric-IP SSRF, F4 quota prefix check, F5 duplicate row_get, F6 audit ternary; S1 unused import, S2 backoff doc, S3 defensive tearDown. Directed 20 pass, full 1700 pass.
202 lines
11 KiB
Markdown
202 lines
11 KiB
Markdown
# 常驻监听网站闭环：独立架构与代码质量审核报告
|
||
|
||
审核员非实现者。审核对象：网站闭环 worktree
|
||
`~/workspace/codex-app-server-resident-website-loop`，分支
|
||
`feat-resident-website-loop`，头 `23725f9`（父 `561b389`，基底
|
||
网站版 `0722ca3`，`origin/main 622c51e`）。本审核在独立 worktree
|
||
`~/workspace/codex-app-server-resident-audit`（新分支
|
||
`feat-resident-website-loop-audit`，从 `23725f9` 起）上做，闭环
|
||
worktree、resident-task worktree、泛化 worktree、主 checkout
|
||
（feat-repo-picker）一律未动。
|
||
|
||
## 0. 评分标准（先定标准，再打分）
|
||
|
||
只评架构与代码质量两项，满分各 10 分。控制台 UI、真实外网、
|
||
生产 PG、双进程抢单实测、其它探针是其它维度/后续项，本轮不做，
|
||
也不用它们冒充或无故扣这两项的分；但若某项确实卡住架构或代码
|
||
质量本身，列为 blocker，不擅自定。
|
||
|
||
### 架构 10 分标准
|
||
|
||
- A1 分层干净：resident 模块自有状态机，与任务状态机、自动化
|
||
定义层互不干扰；server 只做路由/权限/审计，worker 只做驱动
|
||
边界调用；无循环依赖。
|
||
- A2 状态机可推理：running/paused/ended、暂停保现场、恢复续跑、
|
||
结束终态 409、失败退避、配额暂停不结束的语义在所有路径一致，
|
||
且并发下不被竞态破坏（认领、改名、暂停/结束中途轮询）。
|
||
- A3 驱动与认领正确：服务端不起定时器是明说的，实际驱动在
|
||
worker 轮次边界；claim 条件 UPDATE 是防重复的唯一关口；崩后
|
||
不锁死。
|
||
- A4 快照/变化语义干净：轮询廉价、仅变化建任务、首次只建基线、
|
||
每次轮询可查；变化摘录与游标推进在同一事务边界内。
|
||
- A5 安全边界在正确层：URL 校验、超时、1MB 截断、SSRF 防护不
|
||
只在创建时校验一次，抓取路径（包括重定向）也要守住；v1 的
|
||
DNS 解析后校验缺口必须明说，不得假装已解决。
|
||
- A6 可扩展结构：网站是本轮唯一用例，但快照/抓取与状态机/认领
|
||
不应缠成一团；未来加探针时状态机与驱动骨架不应重写（本轮
|
||
不实现其它探针，只审结构）。
|
||
|
||
### 代码质量 10 分标准
|
||
|
||
- Q1 命名、函数长度与复杂度可读；长函数（run_due）分支清晰，
|
||
不靠注释掩盖逻辑。
|
||
- Q2 无重复、无死代码、无测试值渗入生产代码；复用已有口径
|
||
（自动化 claim/配额/审计、环境名唯一索引先例）而不是另起一套。
|
||
- Q3 错误处理诚实：失败、配额耗尽、配额检查失败三条路不混淆；
|
||
审计与轮询记录不静默丢失；异常不炸驱动循环。
|
||
- Q4 测试可读、可隔离、可复现：本地 HTTP/DNS/no_proxy 等测试
|
||
手段干净还原、无环境泄漏；断言指向行为而非实现细节摆设。
|
||
- Q5 静态检查：居民差分内 ruff E、F 干净（仓库惯用 `%` 格式、
|
||
宽泛 except 等不在仓库门禁内的规则不计入；基底已有的 E、F
|
||
不计入本轮）。
|
||
|
||
## 1. 改动前分数
|
||
|
||
- 架构：9.0 / 10
|
||
- 代码质量：9.0 / 10
|
||
|
||
基底是好的：分层、状态机、claim、配额复用、轮询可查、测试
|
||
隔离（DNS/no_proxy 在 loop 测试里干净还原）都属实，定向 15 项
|
||
基线全过（见第 4 节）。扣分只来自下面逐项 findings，不是整体
|
||
推倒重来。
|
||
|
||
## 2. Findings 清单（改动前）
|
||
|
||
### 严重
|
||
|
||
- F1 名称唯一性是先查后插、无数据库约束：create_watch /
|
||
update_watch 先 SELECT 查重再 INSERT/UPDATE，两个并发请求
|
||
可同时通过检查、插出同项目同名 running 监听；server 的
|
||
environments 已有先例：`CREATE UNIQUE INDEX … WHERE status
|
||
!= 'archived'`。本轮用同款部分唯一索引
|
||
`WHERE status != 'ended'`（已结束名可重用）+ 完整性错误转
|
||
ValueError 修复。
|
||
- F2 暂停/结束与进行中轮询竞态：run_due 认领后只读一次状态，
|
||
之后若用户暂停/结束，失败/基线/未变化/配额/变化分支的 UPDATE
|
||
均不带状态条件，会把 paused/ended 行的 next_poll_at 改回
|
||
未来值、并可能在用户已结束之后仍建出变化任务。修复：认领后
|
||
复查状态；全部终局 UPDATE 带 `status='running'` 条件，条件
|
||
不满足时回滚、不建任务、不记轮询。
|
||
|
||
### 一般
|
||
|
||
- F3 抓取路径 SSRF 只守了创建时：default_fetch 用默认 opener，
|
||
重定向目标不校验，公开 URL 重定向到内网地址可绕过创建校验；
|
||
另 localhost.、单整数/十六进制/前导零等非规范 IP 写法可绕过
|
||
字面 IP 检查。修复：重定向处理器逐跳校验（相对跳转先 urljoin
|
||
再校验）；校验时归一化主机（去尾点、*.localhost 挡掉）并拒绝
|
||
非规范数字 IP 写法。DNS 解析后校验仍属 v1 缺口，继续列为已知
|
||
偏差，不假装已解决。
|
||
- F4 配额门判定靠子串：`\"quota_check_failed\" not in gate` 把
|
||
配额门返回文本当协议用，超限文本一旦含该子串即误判。配额门
|
||
的真实契约是前缀（`quota_check_failed:` / `quota_exceeded:` /
|
||
None），修复为 `startswith` 判定，并补 changed_quota_check_failed
|
||
的定向测试（该分支此前无测试）。
|
||
- F5 row_get 重复造轮：resident 自带与 automations.row_get 逐字
|
||
同语义的 row_get，却已从 automations 导入另外两个函数。修复：
|
||
直接导入 automations.row_get，删本地定义。
|
||
- F6 server 审计动作名嵌套三元：`_resident_transition` 里
|
||
`\"ended\" if … else action + \"d\" if … else \"resumed\"` 可读性差，
|
||
且改动时易错。修复为显式动作映射字典，审计字符串不变。
|
||
|
||
### 建议
|
||
|
||
- S1 test_resident.py 导入了未用的 `time`（ruff F401，居民差分
|
||
内唯一 E、F 问题）。删除。
|
||
- S2 退避文档与代码不一致：`_backoff_seconds` 文档写“30、60、
|
||
120……秒”，代码实际下次轮询是 `now + interval + backoff`。
|
||
行为本身合理（不比间隔更频繁），文档误导。修复文档，行为不动。
|
||
- S3 loop 测试 tearDown 不够防御：setUp 中途失败时 tearDown
|
||
访问未建属性会掩盖原始失败。修复为 getattr 防御。
|
||
|
||
### 已核查、不是问题（不扣分）
|
||
|
||
- 审计行“写在 commit 之后会丢”：核查 `_auto_audit_row` 本身
|
||
执行 INSERT 后自己 commit，失败/配额暂停/变化三处审计行在
|
||
模拟关闭连接后仍可查，不是缺陷。
|
||
- server/worker 全量 ruff E、F 的其余项（如 worker.py:2781/2787
|
||
的 F821、server.py:10293 的 E501）经核对均为基底已有或由居民
|
||
插入导致的行号位移，非居民新增代码，不计入本轮。
|
||
- 仓库默认 ruff 全规则的 39 项（如 UP031 `%` 格式）与仓库惯用
|
||
写法冲突，不在仓库门禁内，不计入。
|
||
|
||
## 3. 修复（全部关闭，每项有对应测试或定向复核）
|
||
|
||
- F1 名称唯一性：`ensure_resident_tables` 新增部分唯一索引
|
||
`idx_resident_watches_name ON resident_watches(project_id, name)
|
||
WHERE status != 'ended'`（server environments 同款口径）；
|
||
create/update 的 INSERT/UPDATE 包完整性错误转 ValueError
|
||
（sqlite3.IntegrityError / PostgresIntegrityError，调用方仍
|
||
返回 400）。测试：索引存在断言、绕过查重直接插报
|
||
IntegrityError、改名撞名报 ValueError。
|
||
- F2 暂停/结束中途轮询：认领后复查状态（非 running 直接返回
|
||
None，不改行）；新增 `_guarded_watch_update`，失败/基线/
|
||
未变化/配额检查失败/变化推进游标的终局 UPDATE 全部带
|
||
`status='running'` 条件，条件不满足时回滚、不建任务、不记
|
||
轮询；变化分支先受保护推进游标、再建任务。测试：抓取回调
|
||
里经第二连接暂停监听，run_due 返回 None、无任务、游标与
|
||
next_poll_at 保持暂停态、轮询数不变。
|
||
- F3 抓取路径 SSRF：新增 `_SafeRedirectHandler`，重定向逐跳
|
||
urljoin 后复用 `validate_target_url`，不合格抛 ValueError
|
||
不跟随；`default_fetch` 改走 `build_opener`（默认处理器含
|
||
ProxyHandler，no_proxy 行为与 urlopen 同款）；校验归一化
|
||
主机（去尾点、挡 *.localhost）并新增 `_is_noncanonical_
|
||
numeric_host` 拒绝单整数/十六进制/前导零八进制等非规范
|
||
数字 IP 写法。测试：localhost.、*.localhost、2130706433、
|
||
0x7f000001、0177.0.0.1、0x7f.0.0.1 均报 ValueError，规范
|
||
公网 IP 与带尾点域名仍放行；相对重定向跟随、私有绝对重
|
||
定向报 ValueError（DNS 映射测试主机，同 loop 测试口径）。
|
||
- F4 配额门判定：`\"quota_check_failed\" not in gate` 改为
|
||
`gate.startswith(\"quota_check_failed\")`，与配额门前缀契约
|
||
对齐。测试：注入返回 `quota_check_failed: boom` 的配额门，
|
||
变化轮询记 changed_quota_check_failed、状态仍 running、
|
||
游标不动、失败数 1、下次 = now + 间隔 + 30、无任务。
|
||
- F5 row_get 重复：删 resident 本地定义，直接导入
|
||
automations.row_get，内部调用点一字未动，模块依赖说明
|
||
同步更新。
|
||
- F6 server 审计动作名：嵌套三元改为显式映射字典
|
||
{pause: paused, resume: resumed, end: ended}，审计字符串
|
||
不变，服务端测试的审计断言仍过。
|
||
- S1 删 test_resident.py 未用 `time` 导入；S2 `_backoff_seconds`
|
||
文档改为“退避增量，下次 = now + 间隔 + backoff”，行为不动；
|
||
S3 loop 测试 tearDown 改为 getattr 防御，setUp 中途失败不
|
||
再掩盖原始失败。
|
||
|
||
## 4. 测试数字（全部本轮实测，无删断言、无 skip、无改默认值）
|
||
|
||
- 定向基线（改动前）：`cloud.tests.test_resident` +
|
||
`test_resident_server` + `test_resident_website_loop` = 15 项全过。
|
||
- 定向改动后：同三文件 = 20 项全过（基线 15 + 新增 5：URL
|
||
变体、唯一索引、配额检查失败、中途暂停、重定向），4.450 秒；
|
||
ruff E、F 在居民差分 4 文件上全部通过。
|
||
- 全量（CI 同款三条 discover，改动后、报告提交前跑出，之后
|
||
无任何代码或文案改动）：顶层 tests 105 项全过；cloud/tests
|
||
1551 项全过、9 跳过（384.252 秒）；deploy/tests 44 项全过
|
||
（单独重跑确认）；合计 1700 项、0 失败、9 跳过。对账：闭环
|
||
基线 1695（105 + 1546 + 44）+ 新增 5 = 1700，无测试丢失、
|
||
无新失败。
|
||
- 覆盖率：本仓库 CI 无覆盖率门禁，不虚构覆盖率变化；新增
|
||
5 项定向测试即上述全量的一部分。
|
||
|
||
## 5. 复评分数
|
||
|
||
- 架构：10 / 10
|
||
- 代码质量：10 / 10
|
||
|
||
复评依据：F1/F2 两项严重 findings 已由数据库约束与状态条件
|
||
UPDATE 从机制上关闭（非仅文档说明），并有定向测试锁定；
|
||
F3 安全边界已从“创建时一次校验”补到“抓取重定向逐跳校验”
|
||
与非规范 IP 写法；F4/F5/F6/S1/S2/S3 全部关闭且测试全过。
|
||
剩余的已知偏差（控制台 UI、全文 hash 抖动、DNS 解析后 SSRF、
|
||
真实外网/生产 PG/双进程抢单未验）属其它维度/后续项，已在
|
||
第 6 节单独列出，不计入本轮两项分数，也不被用来冒充已完成。
|
||
|
||
## 6. 剩余 blocker 与未做事项
|
||
|
||
- 未做：未 push、未合并、未跑 deploy.py、未部署；不做其它探针、
|
||
不接控制台 UI、不做生产 PG 与双进程抢单实测。
|
||
- 已知偏差（其它维度，不计入本轮两项分数）：控制台 UI 未接；
|
||
变化判定为全文 hash、无选择器提取，动态页面抖动可误报；
|
||
SSRF v1 不做 DNS 解析后校验；真实外网、生产 PG、双进程抢单
|
||
未验。
|