Files
DevFlow/docs/05-代码审查/近期改动代码审查-2026-06-15.md
绝尘 04032a2a8d 重构: 文档汇总+进度看板+孤儿任务清理脚本+gitignore 噪音排除
- docs/02 架构设计: 新增 aichat审查/异步审批构想/流式渲染调研/generating状态机/密钥迁移健壮性/工作流脚本执行边界/条件表达式引擎/F-07 trait下沉/Agent架构说明/任务推进构想/功能创意池;更新功能决策记录+归档/对抗论证/文档记录规范/经验记录
- docs/03 模块文档: 新增 AI对话引擎/DAG引擎详解;更新 df-knowledge/df-nodes/df-storage/df-workflow/df-ai
- docs/05 代码审查: 新增 全栈审查/全局review/架构审查/近期改动审查/工作区多角度走查/自研memo流式渲染审查
- docs/09 问题排查: 新增 aichat-apikey-401
- docs/INDEX+README 索引同步;docs/todo 待办看板(2026-06-15 汇总)
- PROGRESS.md Sprint 22-25;URGENT.md 加急清单快照(5 项 P0 已全修)
- scripts/cleanup_orphan_tasks.{py,sh} 孤儿任务清理工具
- .gitignore 补 *.broken.bak + tmp/ 噪音排除
2026-06-15 05:14:21 +08:00

202 lines
12 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# 近期改动代码审查报告2026-06-15
> 范围:工作区未提交 RustFR-S1 密钥管理 / FR-S7 写入防护 / FR-S8 symlink 防护)+ 近 5 个提交Rust 后端 7 文件 + 前端 12 文件),约 770 行。
> 方法主代理深读工作区安全改动secret.rs / tool_registry.rs / commands.rs / lib.rs含 resolve_workspace_path 双层校验、write_file 原子写、list_dir symlink 处理)+ 2 个 general-purpose 子代理并行审「近 5 提交 Rust 后端」「近 5 提交前端」,主代理对三路结果核实去重并补查 keyring 闭环。
> 去重:与 [全栈代码审查报告-2026-06-14.md](全栈代码审查报告-2026-06-14.md) / [架构与缺陷复核报告-2026-06-14.md](架构与缺陷复核报告-2026-06-14.md) 范围互补——前两份是 06-14 全栈基线审查,本轮针对 **06-14 之后的提交FR-S 安全修复、confirm 抽取、dag 拓扑 O(V+E)、流式健壮性)+ 工作区未提交**。aichat AR 系列、FR-C/S/R 已修项不重复,仅审本轮代码本身引入或暴露的新问题。
> 互斥:本报告 §①FR-S1 keyring 清理闭环)与 [aichat审查报告](../02-架构设计/aichat审查报告-2026-06-14.md) 的 FR-S1 条目同源(密钥迁移至 keyring本轮发现其删除路径闭环缺失。
---
## 审查范围
**工作区未提交Rust**
- `src-tauri/src/commands/ai/secret.rs`新文件FR-S1 keyring 管理)
- `src-tauri/src/commands/ai/tool_registry.rs`write_file 原子写+.bak / read_file 二进制降级+limit 上限 / list_dir symlink 防护)
- `src-tauri/src/commands/ai/commands.rs`ai_list_providers mask / ai_save_provider 密钥转 keyring
- `src-tauri/src/commands/ai/{agentic,knowledge_inject,title,mod}.rs``commands/project.rs``lib.rs`resolve_provider_secret 接入 + 启动迁移)
**近 5 提交commit 4b5f096 → f58743e**
- Rust`df-ai/{anthropic_compat,openai_compat,context}.rs``df-workflow/{dag,executor}.rs``df-storage/crud.rs``audit.rs`
- 前端:`ToolCard.vue``AiChat.vue``useAiEvents.ts``useConfirm.ts`、4 个 view、2 个 i18n
---
## 🔴 必须修复1
### ① [commands.rs:390-401] 删 provider 不清理 keyring — FR-S1 密钥残留泄漏
**现状**FR-S1 把密钥迁到 OS keyring`devflow-ai-provider/<id>`DB `api_key` 列置空。`secret.rs:44` 已定义 `delete_provider_secret`,但 `ai_delete_provider` 只调 `state.ai_providers.delete`**keyring entry 永久残留**。
**问题**:用户「删除 provider」本意含撤销密钥残留密钥仍可被同用户下任意进程读取迁移后若同 id 被复用new_id 为 uuid 概率极低,但外部指定 id 场景存在旧密钥复活。FR-S1 安全特性闭环缺一环。
**修法**:删 DB 后清理 keyring失败仅日志不阻断——DB 已删,残留 keyring 无消费方)。
```diff
pub async fn ai_delete_provider(
state: State<'_, AppState>,
provider_id: String,
) -> Result<(), String> {
state.ai_providers.delete(&provider_id).await.map_err(|e| e.to_string())?;
+ // FR-S1:清理 keyring 残留密钥(失败仅日志,不阻断删除——DB 已删,残留 keyring 无消费方)
+ if let Err(e) = super::secret::delete_provider_secret(&provider_id) {
+ tracing::warn!("[FR-S1] 删除 provider 后清理 keyring 失败 {}: {}", provider_id, e);
+ }
let mut session = state.ai_session.lock().await;
```
---
## 🟡 建议改进7
### ② [audit.rs:50] `_` 把 DB Err 误报为「项目已不存在」
**现状**`get_by_id``Err`DB 故障/锁/连接断)与 `Ok(None)`(真不存在)被 `_` 合并DB 出错时用户看到误导性「项目没了」而非错误。
**修法**:分三臂,`Err` 打日志回退裸 id。
```diff
- match repo.get_by_id(id).await {
- Ok(Some(p)) => format!("「{}」(id={})", p.name, id),
- _ => format!("(项目已不存在, id={})", id),
- }
+ match repo.get_by_id(id).await {
+ Ok(Some(p)) => format!("「{}」(id={})", p.name, id),
+ Ok(None) => format!("(项目已不存在, id={})", id),
+ Err(e) => { tracing::warn!(%id, error=%e, "查项目标签失败"); format!("(id={})", id) }
+ }
```
### ③ [openai_compat.rs:414] `anyhow::anyhow!(e)` 丢 error source 链
**现状**:非超时分支用 `anyhow!(e)``reqwest::Error` 整体塞进 message丢失 `#[source]` 因果链(`?` 本会保留)。注释强调「不静默挂」,此处反而降低可追溯性。
**修法**:保留 source。
```diff
- } else {
- anyhow::anyhow!(e)
- }
+ } else {
+ anyhow::Error::from(e)
+ }
```
### ④ [AiChat.vue:371-387] 第五份 confirm 逻辑未迁移 useConfirm
**现状**本次「confirmDialog 抽 composable」收敛了 Projects/ProjectDetail/Ideas/Settings 四处 `{visible,msg,resolve}+Promise` 样板,但 `AiChat.vue` 是同模式第五处reactive 版,删对话/清空消息),被遗漏。抽取做了一半,下次改 confirm 语义这里会脱节。
**修法**:直接复用 composable。
```diff
- const confirmState = reactive({ visible:false, msg:'', resolve:null as null|((v:boolean)=>void) })
- function confirmDialog(msg:string):Promise<boolean> { /* Promise 样板 */ }
- function answerConfirm(ok:boolean) { /* ... */ }
+ const { confirmState, confirmDialog, answerConfirm } = useConfirm()
```
### ⑤ [ToolCard.vue:313] displayArgValue 对非 string id 降级为裸值
**现状**:项目名回显前提是 `typeof arg.val === 'string'`。若后端把 id 序列化成 numberJSON 无引号整数),`id` 变空串、分支不进、落到裸数字显示——恰是本次「可读化」要消灭的形态,且无告警。既然已为「查不到项目名」做 `projectIdNotFound` 兜底,类型不一致这一更基本的不可靠也应收口。
**修法**:归一为 string。
```diff
- const id = typeof arg.val === 'string' ? arg.val : ''
+ const id = typeof arg.val === 'string' || typeof arg.val === 'number' ? String(arg.val) : ''
```
> 若确认后端 id 恒为 string uuid此项可降为 ⚪。
### ⑥ [tool_registry.rs:469-484] `.bak`/`.tmp-write` 残留污染目录视图
**现状**FR-S7 覆盖前备份 `path.bak` + 原子写用 `path.tmp-write`,两者留在 workspace 内且**永不清理**,多次写入堆积;`list_directory` 不过滤它们(`is_noise_dir` 只过滤目录LLM/用户看到的目录混入噪音文件LLM 可能误读 `.bak` 当真实文件。
**修法**`.bak` 是给用户恢复用的安全网不能删;改为 `list_dir_recursive``.bak`/`.tmp-write` 后缀并入跳过,或写入前清理同 path 旧 `.bak`(只留最新一份)。
```diff
+fn is_noise_file(name: &str) -> bool {
+ name.ends_with(".bak") || name.ends_with(".tmp-write")
+}
// list_dir_recursive 推 entry 前过滤
+if is_noise_file(&name) { continue; }
```
### ⑦ [crud.rs:1187] COLS 手写列串易与表结构漂移
**现状**`COLS` 硬编码列名,表加列/`KnowledgeRecord` 改字段时编译期不报错query_map 按列名读列少才运行时崩。「列限定」目标未真正达成——隐式依赖从「表全列」挪到「手写列串」。注释偏长10 行 TODO
**修法**:加一行测试断言列数 == 14 防漂移,或注释压缩并点明「改表结构需同步本常量」。
### ⑧ [ToolCard.vue:288-293] projectNameById computed 过度结构化
**现状**computed 每次重建覆盖全部项目(含回收站)的 Map而审批卡通常仅 1-2 行用项目 id。为单点查询做的全量索引收益成本不匹配。
**修法**:直接 find命中即返审批场景项目数有限、find 提前退出,响应式仍由 store 数组保证)。
```diff
- const projectNameById = computed<Record<string,string>>(() => { /* 遍历构建 map */ })
- const name = projectNameById.value[id]
+ const name = projectStore.projects.find(p => p.id === id)?.name
+ ?? projectStore.deletedProjects.find(p => p.id === id)?.name
```
---
## ⚪ 可选优化5
### ⑨ [dag.rs:126 / executor.rs:91] 邻接表两处镜像重复
`topological_layers` 与 executor 各自内联重建邻接表,旧 `successors`/`predecessors`O(E) 全扫)成死代码候选。建议抽 `Dag::adjacency_out()/adjacency_in()` 供两处复用,或确认无调用方后删。
### ⑩ [anthropic_compat.rs:167] 占位 id `tool_missing_{idx}` 理论可撞
流式缺 id 用 index 兜底,同 index 复用理论可撞。实际 index 唯一、低概率,注释已说明。可加计数器求稳。
### ⑪ [useConfirm.ts:1] `//!` 注释风格
Rust doc 风格,但本仓 composable 既有此约定,保持一致即可,不必动。
### ⑫ [ToolCard.vue:61+] `parsed?.` 冗余可选链
这些引用都在 `v-if/parsed` 链内,到达时必非空。非本次引入。
### ⑬ [useAiEvents.ts:79] 注释「沿用正向扫描」表述含糊
改为「消息内 toolCalls 通常 1-2 条,正序即可」更清晰。
---
## ✅ 亮点
- **FR-S7 原子写 + .bak 双保险**tool_registry.rs:467-498write 崩溃不留半成品tmp→rename误覆盖有 .bak 兜底 + 缩减>90% warn。堵住「LLM 把 write_file 当 edit 致数据彻底丢失」的真实事故(会话 3473fcb7PROGRESS.md 762 行/72KB 被覆盖成 248 字节)。
- **FR-S8 symlink 防护**tool_registry.rs:528-540`file_type()` 不跟随 symlinksymlink 标记但不递归——精准堵住「workspace 内 symlink 指向外部」的逃逸,且不取目标 metadata。防御点选在正确边界非中间层冗余校验。
- **findToolCall 反向扫描权衡留痕**useAiEvents.ts:75-85注释讲透「为何不建 Map 索引」messages 多处整体替换、独立索引易陈旧),行为等价、最坏复杂度明确,务实画像下优选。
- **renderMd 流式纯文本短路**AiChat.vue:347-360streaming 时跳过 marked+sanitize 避掉帧刻意不缓存流式文本防污染false 翻转自动走 markdown 重渲染,决策清晰无副作用。
- **resolve_workspace_path 双层校验**tool_registry.rs:50-65词法层 `starts_with`(兜底不存在路径)+ canonicalize 层(解析存在路径的 symlink返回词法 resolved 保证前端友好。设计扎实。
---
## 📊 摘要
| # | 等级 | 文件:行 | 修改内容 |
|---|------|---------|----------|
| ① | 🔴 | commands.rs:390 | 删 provider 补 keyring 清理FR-S1 闭环) |
| ② | 🟡 | audit.rs:50 | Err 与 None 分臂,防误报「项目不存在」 |
| ③ | 🟡 | openai_compat.rs:414 | 保留 error source 链 |
| ④ | 🟡 | AiChat.vue:371 | 第五份 confirm 迁移 useConfirm |
| ⑤ | 🟡 | ToolCard.vue:313 | id 归一 string 覆盖 number |
| ⑥ | 🟡 | tool_registry.rs:469 | .bak/.tmp-write 列表过滤防噪音 |
| ⑦ | 🟡 | crud.rs:1187 | COLS 列数断言防漂移 |
| ⑧ | 🟡 | ToolCard.vue:288 | projectNameById 改 find |
| ⑨ | ⚪ | dag.rs/executor.rs | 邻接表抽公共方法消除重复 |
| ⑩ | ⚪ | anthropic_compat.rs:167 | 占位 id 加计数器 |
| ⑪ | ⚪ | useConfirm.ts:1 | 注释风格(保持现状) |
| ⑫ | ⚪ | ToolCard.vue:61+ | 去冗余可选链 |
| ⑬ | ⚪ | useAiEvents.ts:79 | 注释表述 |
**总计**🔴1 🟡7 ⚪5
**总体评价**改动方向扎实——FR-S7/S8 安全防护、拓扑 O(V+E)、流式健壮性、confirm 抽取都对路,注释质量高(权衡/根因都留痕)。唯一真实闭环漏洞是 ①FR-S1 删 provider 漏清 keyring。其余为 DRY 收口(④抽取只完成 4/5与可维护性。
**质量评级**:良(修 ①④ 可达优)