文档: 全库走查报告(docs/05-代码审查 00-05)+ 规范更新
This commit is contained in:
@@ -0,0 +1,310 @@
|
||||
# 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" 过宽
|
||||
```
|
||||
Reference in New Issue
Block a user