Files
DevFlow/docs/05-代码审查/01-df-nodes-走查-2026-08-02.md
T

311 lines
10 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.
# df-nodes 代码走查报告
> 走查日期: 2026-08-02
> 范围: crates/df-nodes/src/ (17 文件, ~280KB)
> 走查方式: 逐文件全量阅读 + 交叉引用
---
## 问题汇总
| # | 等级 | 文件 | 类型 | 简述 |
|---|---|---|---|---|
| 1 | 🔴 P0 | ai_node.rs | bug | schema `required=[]` 但 execute 时 prompt 必填,schema/运行时不一致 |
| 2 | 🔴 P0 | ai_self_review_node.rs | bug | schema `required=["task_id","provider_id"]` 但 execute 时 provider_id 可空 |
| 3 | 🔴 P0 | docker_node.rs | risk | shell_quote 不处理 `;` `|` `&` 等 shell 元字符,存在命令注入 |
| 4 | 🟡 P1 | docker_node.rs | risk | 命令注入:volumes.host 用户可控,shell_quote 不完整 |
| 5 | 🟡 P1 | human_node.rs | risk | timeout_secs 默认 3600s(1小时),前端崩溃时用户等 1 小时 |
| 6 | 🟡 P1 | http_node.rs | smell | 每次请求新建 reqwest::Client,无连接池复用 |
| 7 | 🟡 P1 | docker_node.rs | smell | 每次执行都 `docker --version` 探测,浪费 |
| 8 | 🟡 P1 | subflow_node.rs | bug | 只返回子 DAG 元数据,不实际执行子 DAG |
| 9 | 🟡 P1 | notify_node.rs | tech-debt | desktop 类型只是 tracing 日志占位,不发送桌面通知 |
| 10 | 🟡 P1 | human_node.rs | smell | 833 行单文件,impl + 测试混在一起 |
| 11 | 🟡 P1 | conditions.rs | smell | 934 行单文件,解析器 + JSON Path + 测试未拆分 |
| 12 | 🟡 P1 | task_advance_node.rs | smell | 528 行含大量测试,测试应拆到独立模块 |
| 13 | 🟢 P2 | docker_node.rs | smell | shell_quote 与 git_node.rs 重复定义(DRY 漂移) |
| 14 | 🟢 P2 | docker_node.rs | smell | volumes 解析跳过坏项但不 warn,用户不知配置被忽略 |
| 15 | 🟢 P2 | script_node.rs | risk | dangerous_keywords 硬编码且仅告警不阻止 |
| 16 | 🟢 P2 | script_node.rs | smell | 白名单/黑名单用 OnceLock,设置后不可重置 |
| 17 | 🟢 P2 | human_node_helpers.rs | risk | REJECT_KEYWORDS 含 "no""no problem" 等文本可能误判 |
| 18 | 🟢 P2 | conditions.rs | tech-debt | 934 行条件引擎写了但默认 feature 关闭 |
| 19 | 🟢 P2 | ai_node_helpers.rs | smell | resolve_provider 三路径逻辑清晰但函数过长(80行) |
| 20 | 🟢 P2 | ai_self_review_node.rs | smell | build_review_prompt 用 format! 拼接 JSON 模板,可读性差 |
---
## 🔴 P0 问题详述
### #1 AiNode schema/运行时不一致
**文件**: `ai_node.rs:118-127`
**严重**: P0 — schema 是前端校验依据,不一致导致前端误拒合法配置
**现状**:
```rust
// schema 声明 required=[]
"required": []
```
**运行时**:
```rust
// ai_node_helpers.rs:156-164
let prompt = inputs.get("prompt")
.or_else(|| config.get("prompt"))
.ok_or_else(|| anyhow!("缺少必填参数: prompt"))?;
```
**影响**: 前端按 schema 校验认为 prompt 可选,用户不填时前端放行但后端报错。
**建议**: schema `required``"prompt"`,或在 schema 描述中注明"config.prompt 或上游 inputs.prompt 至少一个必填"。
---
### #2 AiSelfReviewNode schema/运行时不一致
**文件**: `ai_self_review_node.rs:276-280`
**严重**: P0 — 同上,schema 说 provider_id 必填但运行时可空
**现状**:
```rust
"required": ["task_id", "provider_id"]
```
**运行时**: `resolve_and_parse` 三路径兜底,provider_id 可空(走默认 provider)。
**影响**: 前端按 schema 强制要求 provider_id,用户不填被前端拒绝,但实际后端能兜底。
**建议**: schema `required` 改为 `["task_id"]`provider_id 描述注明"留空走默认 provider"。
---
### #3 DockerNode shell_quote 命令注入
**文件**: `docker_node.rs:105-111`
**严重**: P0 — 用户可控输入经不完整的 shell_quote 进入 shell 命令
**现状**:
```rust
fn shell_quote(s: &str) -> String {
if s.chars().any(|c| c.is_whitespace() || c == '"' || c == '$' || c == '`') {
format!("\"{}\"", s.replace('"', "\\\""))
} else {
s.to_string()
}
}
```
**漏洞**: 只处理空格/双引号/$/反引号,不处理 `;` `|` `&` `` ` `` (反引号在条件中但替换时未转义)。
**攻击场景**: 用户传入 `host: "/workspace; rm -rf /"` → shell_quote 检测到空格加引号 → `"\"/workspace; rm -rf /\""` → 引号内的 `;` 被 shell 解释为命令分隔符。
**建议**: 使用 `shell-escape` crate 或手动转义所有 shell 元字符(`;` `|` `&` `` ` `` `$` `(` `)` `<` `>` `{` `}` `!`)。
---
## 🟡 P1 问题详述
### #4 DockerNode volumes.host 用户可控
**文件**: `docker_node.rs:105-111`
**严重**: P1 — 与 #3 关联,volumes.host 是用户直接传入的字符串
**影响**: 攻击者通过 volumes 配置注入 shell 命令。
---
### #5 HumanNode timeout 默认值过长
**文件**: `human_node.rs:43`
**严重**: P1 — 用户体验问题
**现状**:
```rust
let timeout_secs = config.get("timeout_secs")
.and_then(|v| v.as_u64())
.unwrap_or(3600); // 1 小时
```
**影响**: 前端崩溃/用户离开时,审批节点等 1 小时才超时。
**建议**: 默认值改为 1800s(30分钟)或 3600s 但加 max 上限。
---
### #6 HttpNode 每次新建 Client
**文件**: `http_node.rs:105-110`
**严重**: P1 — 性能问题
**现状**:
```rust
let client = reqwest::Client::builder()
.timeout(Duration::from_secs(params.timeout_secs))
.build()?;
```
**影响**: 每次节点执行都新建 HTTP client,无法复用连接池,高并发工作流时性能差。
**建议**: 用 `OnceLock<reqwest::Client>` 或 `Arc<reqwest::Client>` 共享。
---
### #7 DockerNode 每次 docker --version
**文件**: `docker_node.rs:88-96`
**严重**: P1 — 性能浪费
**现状**: 每次节点执行都跑一次 `docker --version` 探测。
**建议**: 用 `OnceLock<bool>` 缓存探测结果。
---
### #8 SubflowNode 不实际执行子 DAG
**文件**: `subflow_node.rs:45-67`
**严重**: P1 — 功能不完整
**现状**:
```rust
Ok(NodeOutput::from_value(serde_json::json!({
"subflow": true,
"node_count": sub_dag.nodes.len(),
...
"dag": sub_dag, // 只返回元数据
})))
```
**影响**: SubflowNode 只返回子 DAG 的 JSON 快照,不递归执行。注释说"供 DagExecutor 消费",但 executor 不会自动执行返回的 subflow。
**建议**: 要么在 execute 内递归调 `DagExecutor::run`,要么明确文档说明"需要调用方自行执行返回的 DAG"。
---
### #9 NotifyNode desktop 只是日志
**文件**: `notify_node.rs:108-117`
**严重**: P1 — 功能缺失
**现状**:
```rust
NotifyType::Desktop => {
tracing::info!(title = %params.title, message = %params.message,
"NotifyNode desktop 通知(日志占位,集成待后续 Sprint)");
Ok(NodeOutput::from_value(...))
}
```
**影响**: 用户配置 desktop 通知类型,实际只写日志,不发送桌面通知。
**建议**: 要么移除 desktop 类型,要么集成 tauri-plugin-notification。
---
### #10-12 大文件拆分
**文件**: human_node.rs(833行) / conditions.rs(934行) / task_advance_node.rs(528行)
**严重**: P1 — 可维护性
**建议**:
- human_node.rs: 测试拆到 `human_node_tests.rs`
- conditions.rs: 拆为 `tokenizer.rs` / `parser.rs` / `jsonpath.rs` / `tests.rs`
- task_advance_node.rs: 测试拆到独立模块
---
## 🟢 P2 问题详述
### #13 shell_quote DRY 漂移
**文件**: `docker_node.rs:105-111` 与 `git_node.rs:137-142`
**严重**: P2 — 两处逐字相同
**建议**: 抽到 `df-nodes/src/shell_quote.rs` 共享。
---
### #14 DockerNode volumes 跳过坏项不 warn
**文件**: `docker_node.rs:62-70`
**严重**: P2
**建议**: 跳过时 `tracing::warn!` 记录被跳过的配置项。
---
### #15 ScriptNode dangerous_keywords 硬编码
**文件**: `script_node.rs:58-65`
**严重**: P2 — 仅告警不阻止,关键词列表不完整
---
### #16 ScriptNode OnceLock 不可重置
**文件**: `script_node.rs:13-18`
**严重**: P2 — 设置后不可重置,需重启应用
---
### #17 REJECT_KEYWORDS 含 "no" 过宽
**文件**: `human_node_helpers.rs:14-18`
**严重**: P2 — "no" 作为拒绝关键字太宽泛
**建议**: 改为 "no" 仅当 options 含 "no" 时匹配,或从关键字列表移除。
---
### #18 conditions.rs 默认 feature 关闭
**文件**: `conditions.rs` (934行)
**严重**: P2 — 写了大量代码但默认不启用
**建议**: 要么默认开启,要么在 README 中说明如何启用。
---
## 正面评价(值得保留的设计)
1. **状态机设计优秀**: `task_state_machine.rs` 从 `TaskStatus::as_str()` 派生常量,消除双源问题,有双源一致性测试锁定。
2. **advance_task_atomic 原子写**: CAS `WHERE status=expected` 防 TOCTOU,退回转换一并 `review_rounds+=1`,设计严谨。
3. **HumanNode 拒绝语义化**: 审批拒绝从 Ok→Err,触发工作流 failed→退回,语义正确。
4. **AiSelfReviewNode 兜底设计**: LLM 输出不可靠时 verdict=unknown 不阻断,保持人定权。
5. **executor 取消处理**: Ok/Err 分支对称处理已取消节点,状态机与事件类型一致。
6. **EventBus broadcast 容量 256**: 审批低频场景下漏自身 Response 概率极低,设计合理。
7. **conditions.rs 求值失败保守 false**: 任何解析错误/JSON Path 缺失/类型不兼容均返回 false,安全优先。
8. **StateMachine 锁中毒降级**: 不 panic,返回保守默认值或 Err,符合"无 panic"铁律。
9. **测试覆盖率高**: 每个节点文件都有配套测试,advance_task_atomic 有 20+ 测试覆盖各种状态转换。
10. **代码注释详尽**: 每处设计决策都有注释说明理由、替代方案和选型依据。
---
## 改进优先级建议
```
立即修复(P0):
#1 AiNode schema/运行时不一致
#2 AiSelfReviewNode schema/运行时不一致
#3 DockerNode shell_quote 命令注入
尽快修复(P1):
#4 DockerNode volumes 注入(与 #3 一起修)
#5 HumanNode timeout 默认值
#6 HttpNode Client 复用
#8 SubflowNode 功能不完整
后续迭代(P2):
#10-12 大文件拆分
#13 shell_quote DRY
#17 REJECT_KEYWORDS "no" 过宽
```