docs: batch G audit fixes record in aibox audit report

Add round-3 audit (section 9): legacy item re-verification (17 items), v3.4 multi-model profile + L4 failover review (P1-P7), architecture compliance matrix. Add batch G fix record (section 10): 17 fixes across 5 waves, 20 new regression tests, full suite 375 passed.
This commit is contained in:
qing committed 2026-08-18 07:21:34 +08:00
1 parent cccc946660
commit acb6ddbb4c
1 file changed
+121 -3
+121 -3
View File
@@ -1,7 +1,7 @@
# aibox 模块安全与质量审计报告
- **日期**:2026-08-17(第一轮)/ 2026-08-18(第二轮复审)
- **审计基线**:第一轮 HEAD `ce142fe`(Batch A–D 后)→ 第二轮 HEAD `177a9ac`(Batch E 修复后)
- **日期**:2026-08-17(第一轮)/ 2026-08-18(第二轮复审)/ 2026-08-18(第三轮:遗留复核 + v3.4 专审)
- **审计基线**:第一轮 HEAD `ce142fe`(Batch A–D 后)→ 第二轮 HEAD `177a9ac`(Batch E 修复后)→ 第三轮 HEAD `aa15841`(L4a/b/c 落地后)
- **审计范围**:`aibox.py`(~3026 行)+ `llm_client.py` + `tools/`(7 文件)+ `subsystems/`(6 文件)+ `common/`(5 文件)
- **方法**:第一轮逐文件代码审读 + PoC 动态验证;第二轮 Batch E 修复复审 + 全量重审 + 6 处关键发现实证抽查
@@ -225,4 +225,122 @@ aibox 全量单测 **264 passed**(零回归)。
- N6(registry 超时路径补清理):两个超时 return(deadline 命中、join(5) 超时)统一改为 `timed_out` 标记 + 出口统一执行 `tool.streaming_output=False` / `tool.output_queue=None`;僵尸 daemon 线程的 `send_line` 落到 None 上不再外写,消除僵尸输出串流路径。`stream_tool_timeout` 提为实例属性(默认 3600s,测试可调)
- N4(reader 线程收尾,**含二次修正**):初版按审计建议"break 后 `stdout.close()` + `reader.join(2)`"实现,PoC 回归实证 **close 会阻塞**——CPython 流对象内部有锁,readline 持锁阻塞时 `close()` 一同挂起直到 readline 返回(PoC-1 从 3s 退化到 15.2s,恰为命令 Start-Sleep 时长)。修正后策略:reader 已正常退出(EOF)→ 显式 close 释放管道 fd;reader 仍阻塞 → 只做 `join(timeout=2)` 后放弃(daemon 兜底,孙进程退出收 EOF 后自然结束,fd 随管道消亡回收),主流程及时返回
- **审计建议修正记录**:8.3 节 N4 与 8.7 节 F2 中"close stdout 可中断阻塞 readline(CPython 行为)"的表述**不成立**,已按实测修正——跨线程中断阻塞中的管道 read 无便携方案,正确做法是控制泄漏窗口而非强行中断
- 测试:新增 `tests/test_f2_fixes.py` 5 例(N6 超时/正常清理 ×2、N4 及时返回 ×2 + EOF 后 close ×1),PoC 回归 5/5(PoC-1/4 恢复 ~5s:超时 3s + join 宽限 2s),全量 272 passed
- **测试**:新增 `tests/test_f2_fixes.py` 5 例(N6 超时/正常清理 ×2、N4 及时返回 ×2 + EOF 后 close ×1),PoC 回归 5/5(PoC-1/4 恢复 ~5s:超时 3s + join 宽限 2s),全量 272 passed
---
## 九、第三轮审计(2026-08-18,HEAD `aa15841`)
方法:前两轮遗留项逐条实证复核 + v3.4 新增代码(多模型档案 / L4a/b/c 失败转移,`llm_client.py`)全量审读 + 测试基线确认(全量 **355 passed**)。
### 9.1 遗留项修复状态复核
| # | 问题 | 状态 | 实证 |
|---|---|---|---|
| L3 | execute() kill 后未 wait() | ✅ **已修** | `executors.py:222-230`:kill/terminate → `wait(timeout=2)` → 超时再 kill 兜底 |
| N17 | InstallSkillTool 任意文件复制 | ✅ **已消解(设计变更)** | 工具重构为纯指引(只返回安装说明文本,不再执行 shell/复制);`CreateSkillTool` 增加 skill_name 正则校验(`^[A-Za-z0-9_-]+$`) |
| N1/N11 | 代理密码脱敏 / 日志打码 | ✅ 在位 | F1 修复无回归 |
| N4/N6 | reader 收尾 / registry 超时清理 | ✅ 在位 | F2 修复无回归 |
| N2 | drain 线程泄漏 | ❌ 未修 | `aibox.py:1280-1303`:循环条件仍只查 cancel 事件,注释"自动退出"与实现不符,每次工具调用泄漏一个常驻线程 |
| N3 | `/new` 阻塞事件循环 | ❌ 未修 | `aibox.py:2325`:2s 轮询竞态窗口后仍 `with self._msg_lock:`(同步 RLock acquire 于事件循环线程) |
| N5 | 子代理 cancel 不传播 | ❌ 未修 | `aibox.py:643-654`:子代理独立 `_cancel_event`/executor/client;`--once` Timer(3110-3113)只 cancel 父代理 |
| N7 | FTS5 引号未翻倍 | ❌ 未修 | `db.py:178`:`f'"{kw}"'` 直接拼,关键词含 `"` 破坏短语边界 |
| N8 | 流式旁路长度上限 | ❌ 未修 | `registry.py:120-129`:`("line", content)` 无限转发;`aibox.py:1326` 的 `MAX_RESULT_LENGTH` 只管回喂 LLM 的最终 result |
| N9 | prompt 注入存储 | ❌ 未修 | `prompt_tools.py`:confirm 默认 True,Agent 可控内容仍直写 00/99/100 文件 |
| N10 | mark_as_triggered TOCTOU | ❌ 未修 | `db.py:509-525`:跨连接先 `get_memo` 后 UPDATE,无 BEGIN IMMEDIATE |
| N12 | _load_persona 路径遍历 | ❌ 未修 | `aibox.py:529-545`:persona LLM 可控,`os.path.join(SKILLS_DIR, name + ".md")` 无边界校验 |
| N13 | broadcast 裸 except | ❌ 未修 | `aibox.py:2131`:`except:` 吞 CancelledError |
| N14 | _processing_tasks 覆盖 | ❌ 未修 | `aibox.py:2788`:按 ws id 覆盖登记;先完成任务的 done_callback 会误 pop 后续任务的登记 |
| N15 | 异常含完整响应体 | ⚠️ 部分改善 | 瞬时路径已截断 `[:200]`(163/213 行);**非瞬时路径仍全量**(`llm_client.py:215-216`) |
| N16 | 历史索引裸 except / 同秒覆盖 | ❌ 未修 | `conversation_history.py:64` 裸 `except:`;136 行归档文件名秒级时间戳 |
| M3/M4 | web scheme 校验 / shlex Windows | ❌ 未修 | 维持原判(Linux 部署不受影响) |
| L1 | LIMIT f-string | ❌ 未修 | `db.py:417, 459` 两处在位(内部 int 直达,防御性待修) |
### 9.2 v3.4 新代码审计(多模型档案 + L4 失败转移)
**设计评价:整体高质量**。免密端点与密钥隔离(`allow_key_file=False` 防跨厂商凭据泄漏)、用户中断绝不转移(cancel 广播全链 + `_cancelled` 门控)、流式仅首事件前重放(`produced_any` 门控)、fallback 链一层不递归 + 自引用/悬空引用硬性报错、构造失败跳过不阻断主档案——关键决策均正确且有测试锚定(`test_models_profile.py` 约 60 个 L4 用例)。
新发现(P 系列):
| # | 级别 | 问题 | 位置 | 说明 |
|---|---|---|---|---|
| P1 | 中 | 非瞬时 HTTP 错误响应体无截断(N15 残留) | `llm_client.py:215-216` | `RuntimeError(f"HTTP {code}: {response.text}")` 全量响应体进异常栈;部分网关 4xx 页回显 Authorization 头 → 密钥进日志。与瞬时路径 `[:200]` 不一致,建议同样截断 |
| P2 | 中 | Retry-After 无上限 | `llm_client.py:290` | `max(1.0, float(v))` 无上界——异常/恶意服务端 `Retry-After: 999999` 使档案冷却 11.5 天。建议 cap(如 `min(v, max(cooldown_seconds, 3600))`) |
| P3 | 低中 | fallback 档案缺 model 不校验 | `llm_client.py:671-673` | 主档案经 `_require_model` 硬性报错,fallback 档案直接 `fb_profile.get("model")` → model=None → API 400 → 表现为神秘 None 而非清晰配置错误。建议构造链时同样校验 |
| P4 | 低 | FailoverClient 非线程安全 | `llm_client.py:321-334` | `_cancelled`/`_call_started`/`_stats` 无锁。当前调用面无实际并发(compressor 在 `_msg_lock` 内同步调用、子代理独立 client),但 cancel(WebSocket 线程)与 generator 启动时 `_cancelled = False` 复位存在理论竞态窗口。建议注释声明"非并发契约"或加锁 |
| P5 | 低 | `LLMClient._cancelled` 死状态 | `llm_client.py:52, 76` | 只写不读:cancel() 置位后无消费方也从不清除,真正判断靠 FailoverClient 自身标志。建议删除或在底层 client 消费 |
| P6 | 低 | fallback 列表重复项不查重 | `llm_client.py:660-673` | `["kimi","kimi"]` 会在链内构造两个同名 client,冷却键同名冲突、重复撞同端点;影响有限 |
| P7 | 信息 | timeout_budget 仅在尝试间检查 | `llm_client.py:363-366, 404, 443` | 预算不截断单档案进行中的阻塞读(单次仍可超时到 timeout)。与文档"限制整条链尝试的总秒数"有细微出入,可接受,建议文档注明"尝试间预算" |
### 9.3 架构合规矩阵(第三轮)
| 硬约束 | 状态 |
|---|---|
| WebSocket :8765 | ✅ |
| 依赖单向 `common←subsystems←tools←aibox` | ✅ |
| Worker 仅 7 执行工具 | ✅ |
| 工具输出 ≤1MB + 超时 + 中断 | ⚠️ executor 链 ✅;残余:N8 流式旁路、N5 子代理超时、N2 drain 泄漏 |
| prompt 三级排序 / sqlite 线程安全 | ⚠️ N7/N10 待修 |
| 多模型档案凭据隔离(v3.4 新增维度) | ✅ 跨厂商 Key 不互发、免密端点显式声明 |
### 9.4 Batch G 修复建议(按优先级)
1. **G1(中)**:P1 非瞬时路径响应体截断 `[:200]` + P2 Retry-After 上限 cap——两处各一行改动
2. **G2(中)**:P3 fallback 档案补 `_require_model` 校验(缺 model 的 fallback 跳过并告警,与缺 Key 行为对齐)
3. **G3(中)**:N5 子代理 cancel 传播(`run_full_subagent` 传递父 `_cancel_event`,每轮 think 前检查)+ N2 drain 线程加 sentinel/空队列退出
4. **G4(中低)**:N3 `/new` 的 `reset_conversation` 移入 `run_in_executor` + N8 registry 转发行截断 + N12 persona 路径边界校验(`os.path.realpath` 前缀检查)
5. **G5(低)**:N7 FTS 引号翻倍 + N10 BEGIN IMMEDIATE + L1 LIMIT 参数化 + N13 `except Exception`/`discard` + N14 任务登记改按 task id 集合 + N16 收窄 except + 归档文件名加毫秒/序号
6. **G6(低/择机)**:P4 并发契约注释或加锁 + P5 死状态清理 + P6 fallback 去重 + N9 prompt 更新确认门槛 + M3/M4 视部署平台决定
### 9.5 测试基线
全量 **355 passed**(21.8s,Windows 开发机 `py -3`),含 L4 失败转移约 60 用例、Batch E PoC 回归 5/5、Batch F1/F2 修复测试 8 例。零回归。
---
## 十、Batch G 修复落地(2026-08-18,第三轮审计修复)
按 §9.4 建议分 5 波实施,每波全量测试验证后进入下一波。
### 10.1 修复清单
| 波次 | # | 修复内容 | 文件/位置 |
|---|---|---|---|
| W1 | P1 | 非瞬时 HTTP 错误响应体截断 `[:200]`(与瞬时路径对齐,密钥/错误页不进异常栈) | `llm_client.py:216` |
| W1 | P2 | Retry-After 解析上限 `min(max(1.0, v), 3600.0)`——恶意/异常头最多冷却 1h | `llm_client.py:284-296` |
| W1 | P3 | fallback 档案构造前补 `_validate_profile` + `_require_model`,配置缺陷跳过并告警(对齐缺 Key 行为) | `llm_client.py:681-695` |
| W1 | P5 | 删除 `LLMClient._cancelled` 死状态(只写不读;真实机制在 FailoverClient),cancel() 注释说明职责边界 | `llm_client.py:48-49, 71-76` |
| W1 | P6 | fallback 列表 `seen` 去重(自引用硬性报错保持不变,测试锚定) | `llm_client.py:669-679` |
| W1 | P4 | FailoverClient 类文档补并发契约声明(非线程安全 + 当前调用面为何安全) | `llm_client.py:324-327` |
| W2 | N8 | registry 流式转发受 `TOOL_MAX_RESULT_LENGTH` 约束:累计预算 + 超限截断 + 停止转发提示(一次性) | `tools/registry.py:113-145` |
| W2 | N12 | persona 路径边界校验:normcase+abspath 前缀检查,越界/绝对路径拒绝 | `aibox.py:536-544` |
| W3 | N2 | drain 线程重构:批次级单线程 + 独立 stop 事件 + try/finally 统一回收(正常/异常/中断路径均 join≤2s);`set_output_queue(None)` 移入 finally | `aibox.py:1254-1386` |
| W3 | N5 | 子代理取消传播:`run_full_subagent` 新增 `cancel_event` 参数→子代理共享父事件(think 前置/工具间/流式事件间既有检查点联动)+ 0.2s 观察者线程转发深度中断(强关在途 HTTP、kill 子进程),finally 通知退出;`SubAgentTool` 构造注入父 `_cancel_event` | `aibox.py:621-624, 669-684, 773-775, 786-790, 897-902, 1145-1147` |
| W4 | N3 | `/new` 的 `reset_conversation` + `prompt_manager.reload()` 移入 `run_in_executor`(不再同步获取 `_msg_lock` 冻结事件循环) | `aibox.py:2386-2400` |
| W4 | N13 | broadcast 裸 except → `except Exception`(不再吞 CancelledError);unregister `remove` → `discard`(幂等) | `aibox.py:2080-2084, 2185-2192` |
| W4 | N14 | `_processing_tasks` 改按 websocket 键的任务集合登记:done_callback 从集合 discard;连接关闭取消该连接全部在途任务(此前仅最后一个) | `aibox.py:2022, 2847-2870` |
| W5 | N7 | FTS5 关键词内双引号翻倍转义(`"` → `""`),MATCH 短语语法合法 | `subsystems/db.py:178-182` |
| W5 | L1 | 两处 `LIMIT {limit}` f-string → `LIMIT ?` 参数化(`get_pending_memos`/`get_immediate_memos`) | `subsystems/db.py:419-421, 462-465` |
| W5 | N10 | `mark_as_triggered` 读写收进单连接 `BEGIN IMMEDIATE` 事务(消灭 check-then-act 竞态:并发触发不再双计数/双推进 repeat) | `subsystems/db.py:515-590` |
| W5 | N16 | 历史索引加载裸 except → `except Exception` + 告警;归档文件名冲突时加 `_N` 序号(同秒归档不覆盖) | `subsystems/conversation_history.py:58-69, 138-146` |
未实施(按 §9.4 G6 择机项):N9(prompt 更新确认门槛,涉及交互设计)、M3/M4(web scheme 校验 / shlex Windows,Linux 部署不受影响)、P7(timeout_budget 语义为文档注记,已在 FailoverClient 文档说明)。
### 10.2 新增测试
`tests/test_g_fixes.py` **20 例**(全部通过):
- P1/P2 ×4:非瞬时错误体截断、Retry-After cap/正常值/非法值
- P3/P5/P6 ×2:fallback 缺 model 跳过 + 重复去重(链 = [main, dupe, ok2])、死状态删除
- N2 ×2:drain 线程批次结束/工具异常路径均回收
- N5 ×3:子代理共享父取消事件 / 未传时保留自身 / SubAgentTool 注册注入链路
- N7 ×2:含引号关键词不破坏检索、常规检索不回归
- N8 ×1:120000 字符流式输出被截断至预算 + 提示标记
- N10 ×2:缺失 id 事务路径返回 None、一次性任务完成语义
- N12 ×3:`../` 越界/绝对路径拒绝、合法 persona 正常加载
- N16 ×1:同秒两次归档文件名不冲突
N3/N13/N14 为 async WebSocket 路径,依赖事件循环与真实连接对象,由现有 WebSocket 测试面回归(全量通过)。
### 10.3 测试基线
全量 **375 passed**(22.5s,Windows 开发机 `py -3`)= 原 355 + Batch G 新增 20。零回归。