Wave 10: deep audits of 5 unaudited plugins, smoke regression set (W13.1-W13.6)
- W13.1 anthropic_plugin (architect-yang, 497 lines): rated C. 6 C ABI functions lack try/catch (§8 violation); my_chat leaks response_body on error path; tool_use response silently dropped. - W13.2 deepseek_plugin (engineer-sun, 486 lines): rated C+. 7 ABI entries unprotected including json::parse paths (malformed JSON terminates); SSE [DONE] sentinel match brittle; ~55% code overlap with anthropic suggests an ai_plugin_base extraction. - W13.3 network_plugin (qa-wang, 322 lines): rated C. CRITICAL: TLS certificate verification fully disabled (set_verify_mode never called, default verify_none accepts any cert) — all AI traffic incl. api_key is MITM-vulnerable. DNS resolve has no timeout; catch lacks (...). - W13.4 lsp_plugin (architect-huang, 749 lines): rated C. CRITICAL: guaranteed deadlock at L519-526 → L547 (g_lsp_impl_start holds mutex then calls g_lsp_impl_stop which re-locks the same non-recursive mutex); 7 vtable funcs unprotected; server→client requests dropped. - W13.5 session+tools (security-cao, 264+251 lines): rated D+/D. Path traversal in builtin_file_read/write (zero validation); global static state in both plugins lacks mutex (UAF risk); 9 vtable funcs lack try/catch. - W13.6 smoke regression (qa-xu, +193 lines): 4 new cases — context max_tokens trim, config dual-store consistency (exposes that W12.2 merge is incomplete: dstalk_config_set→config_service.get returns null), HTTP error path no-crash, repeated init/shutdown cycle. Verified: cmake build 0 error 0 warning, ctest 4/4 pass. Top W14 priorities surfaced: TLS verification (W13.3), LSP deadlock (W13.4), file-tool path traversal (W13.5), config dual-store still broken (W13.6 R2), shared try/catch wrapper across all AI plugins. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
269
agents/audits/W13.4-lsp-audit.md
Normal file
269
agents/audits/W13.4-lsp-audit.md
Normal file
@@ -0,0 +1,269 @@
|
||||
# W13.4 LSP Plugin 深度审计
|
||||
|
||||
**Auditor**: 黄岭 (architect-huang)
|
||||
**Date**: 2026-05-27
|
||||
**File**: plugins/lsp/src/lsp_plugin.cpp (749 行)
|
||||
**Wave Coverage**: W6.1 仅审计 reader_loop (L415-459)，其余 700+ 行首次审计
|
||||
|
||||
---
|
||||
|
||||
## 0. 文件总览
|
||||
|
||||
749 行。模块分布:
|
||||
- Process 子进程 (start/stop/write/read_line/read_bytes): ~220 行 (L54-274)
|
||||
- Service dispatch 实现 (start/stop/open/close/diagnostics/hover/completion): ~280 行 (L461-698)
|
||||
- JSON-RPC 框架 (frame_message/parse_content_length/send_request/send_notification/handle_message): ~110 行 (L299-413)
|
||||
- reader_loop 状态机 (W6.1 已修): ~45 行 (L415-459)
|
||||
- State/Lifecycle/Descriptor: ~50 行 (L276-297 + L700-749)
|
||||
- Headers/Globals: ~44 行 (L1-53)
|
||||
|
||||
## 1. §5 + §8 异常安全——穿越 ABI 边界
|
||||
|
||||
状态: **严重违反** FAIL
|
||||
|
||||
vtable 全部 7 个函数均以 C 函数指针暴露给 host (`dstalk_lsp_service_t`)，但底层 C++ 实现大量使用 `std::string` / `json::object` / `json::parse` / `std::unique_lock` 等可抛异常类型，**零 try/catch 保护**:
|
||||
|
||||
| 函数 | 行号 | 可抛异常操作 |
|
||||
|------|------|-------------|
|
||||
| `g_lsp_impl_start` | L467-537 | json::object, std::string, std::thread, std::unique_lock |
|
||||
| `g_lsp_impl_stop` | L539-566 | json::object, std::unique_lock |
|
||||
| `g_lsp_impl_open_document` | L568-584 | json::object |
|
||||
| `g_lsp_impl_close_document` | L586-598 | json::object |
|
||||
| `g_lsp_impl_get_diagnostics` | L600-612 | std::lock_guard, std::string |
|
||||
| `g_lsp_impl_get_hover` | L614-655 | json::object, json::parse, std::unique_lock, std::string |
|
||||
| `g_lsp_impl_get_completion` | L657-698 | 同上 |
|
||||
|
||||
此外 `reader_loop` (L419-459)、`handle_message` (L375-413)、`on_shutdown` (L724-730) 均无保护。
|
||||
|
||||
**违反代价**: OOM 或任何 `boost::json` 内部异常穿越 C 函数指针边界 → `std::terminate()` → 进程崩溃，零恢复机会。plugin-abi §8 标记此为强制规则 (§8.1 明确覆盖 service vtable 函数指针)，当前代码全线违规。
|
||||
|
||||
对比 W11.1: context_plugin 的 trim_impl (L114-226) 同样违规，但当时仅 1 个 vtable 函数受影响。lsp_plugin 受影响面是其 7 倍。
|
||||
|
||||
评级: **F**
|
||||
|
||||
---
|
||||
|
||||
## 2. §9 字符串返回值生命周期
|
||||
|
||||
状态: **合规** PASS
|
||||
|
||||
三个返回 `char** json_out` 的函数:
|
||||
|
||||
- `g_lsp_impl_get_diagnostics` (L600-612): `g_host->strdup(it->second.c_str())` 或 `g_host->strdup("[]")`，均在 mutex 锁内复制。Mode A (§9.2)，调用方 `host->free` 释放。合规。
|
||||
- `g_lsp_impl_get_hover` (L614-655): `g_host->strdup(json::serialize(resp["result"]).c_str())`，temporary 在 strdup 复制后销毁，指针独立。合规。
|
||||
- `g_lsp_impl_get_completion` (L657-698): 同 get_hover。合规。
|
||||
|
||||
`dstalk_plugin_info_t` 的 name/version/description 均为 static 字面量 → Mode B。合规。
|
||||
|
||||
评级: **A**
|
||||
|
||||
---
|
||||
|
||||
## 3. §2 跨 DLL 堆纪律
|
||||
|
||||
状态: **合规** PASS
|
||||
|
||||
逐行检查所有 malloc/free/strdup/new/delete:
|
||||
|
||||
| 模式 | 结果 |
|
||||
|------|------|
|
||||
| 裸 `malloc`/`free`/`new`/`delete` | 0 处 |
|
||||
| 裸 `strdup` | 1 处 (L143, POSIX 子进程内, fork 后 exec 前, 不跨 DLL) |
|
||||
| `g_host->strdup` | 4 处 (L607/609/653/696), 全部正确 |
|
||||
| `g_host->alloc`/`g_host->free` | 0 处 |
|
||||
|
||||
唯一裸 `strdup` 位于 `fork()` 后的子进程 (L127-151)，该进程随即调用 `execvp()` 替换地址空间，或 `_exit(127)` 退出。不涉及跨 DLL 边界。无合规风险。
|
||||
|
||||
评级: **A**
|
||||
|
||||
---
|
||||
|
||||
## 4. 进程生命周期
|
||||
|
||||
状态: **多个问题** WARN
|
||||
|
||||
### 4.1 Win32: TerminateProcess 无条件调用 (L174-176)
|
||||
|
||||
```cpp
|
||||
WaitForSingleObject(hProcess, 2000); // 返回值被忽略
|
||||
TerminateProcess(hProcess, 1); // 无条件调用
|
||||
```
|
||||
|
||||
即使进程在 2 秒内正常退出，`TerminateProcess` 仍被调用。虽对已退出进程无害（返回 ERROR_ACCESS_DENIED），但语义错误且日志缺失。
|
||||
|
||||
### 4.2 POSIX: SIGKILL 后 waitpid 可能永久阻塞 (L191-192)
|
||||
|
||||
```cpp
|
||||
kill(pid, SIGKILL);
|
||||
waitpid(pid, &status, 0); // 阻塞等待，无超时
|
||||
```
|
||||
|
||||
若子进程处于 D 状态（不可中断睡眠），`waitpid(0)` 永不返回 → 调用线程永久挂起。
|
||||
|
||||
### 4.3 POSIX 子进程: strdup 未检查 null (L143-144)
|
||||
|
||||
`strdup(cmd)` 在 fork 后调用，若返回 NULL → `strtok(NULL, ...)` → 段错误。虽在 exec 前，但仍是未处理失败路径。
|
||||
|
||||
### 4.4 start() 中不必要调用 stop() (L69)
|
||||
|
||||
`Process::start` 开头调用 `stop()`，后者在 Win32 上有 2 秒 `WaitForSingleObject`。若首次启动，句柄均为 INVALID，无实际操作。但语义不清，且 `stop()` 应独立于 `start()` 调用。
|
||||
|
||||
评级: **C**
|
||||
|
||||
---
|
||||
|
||||
## 5. JSON-RPC 协议合规
|
||||
|
||||
状态: **部分缺失** WARN
|
||||
|
||||
### 5.1 Content-Length 解析 (L323-341)
|
||||
|
||||
W6.1 已修。大小写不敏感、空白跳过、异常安全。合规。
|
||||
|
||||
### 5.2 消息分发缺口 (L384-412)
|
||||
|
||||
当前仅处理两种消息:
|
||||
- Response: `id && !method` (L384)
|
||||
- Notification: `method && !id` (L391)
|
||||
|
||||
**缺失**: Server→Client Request (`id && method`) 被静默忽略。LSP 规范中 server 可发送:
|
||||
- `window/showMessageRequest` (需客户端响应)
|
||||
- `workspace/applyEdit` (需客户端响应)
|
||||
- `client/registerCapability`
|
||||
- `workspace/configuration`
|
||||
|
||||
server 请求得不到响应可能导致 LS 功能降级或阻塞。
|
||||
|
||||
### 5.3 Error 对象未处理 (L651, L694)
|
||||
|
||||
`get_hover` / `get_completion` 仅检查 `resp.contains("result")`，不检查 `"error"`。若 server 返回 `{"jsonrpc":"2.0","id":N,"error":{"code":-32601,"message":"..."}}`，函数返回 -1 但 **丢弃错误详情**。调用方无法区分超时和 server 错误。
|
||||
|
||||
### 5.4 通知/响应的 ID 匹配 (L386, L388)
|
||||
|
||||
`as_int64()` → `static_cast<int>`: 若 server 回传 >INT_MAX 的 id 值则截断。不过当前 id 由本端控制，仅 server→client request 场景可能触发。
|
||||
|
||||
评级: **B**
|
||||
|
||||
---
|
||||
|
||||
## 6. 状态机 / 线程同步
|
||||
|
||||
状态: **死锁 + 数据竞争** FAIL
|
||||
|
||||
### 6.1 致命死锁: start 超时路径 (L519-L526 → L547)
|
||||
|
||||
```
|
||||
L519: std::unique_lock<std::mutex> lock(g_lsp.mutex); // 获取 mutex
|
||||
L520: g_lsp.cv.wait_for(lock, 10s, ...); // 超时返回，lock 仍持有
|
||||
L526: g_lsp_impl_stop(); // → 进入 g_lsp_impl_stop
|
||||
L547: std::unique_lock<std::mutex> lock(g_lsp.mutex); // 再次 lock 同一非递归 mutex → 死锁!
|
||||
```
|
||||
|
||||
`std::mutex` 非递归，同线程二次 lock 为未定义行为（Windows/Linux 均死锁）。initialize 超时 (10s) 后触发，调用线程永久挂起。
|
||||
|
||||
注意: `get_hover`/`get_completion` 超时时仅返回 -1，不调 stop，无此问题。
|
||||
|
||||
### 6.2 写管道无同步 (L357, L368)
|
||||
|
||||
`send_request` 和 `send_notification` 均调用 `proc.write()` 而无 mutex 保护。若多线程并发调用 LSP 服务 → JSON 帧在管道上交错 → 协议解析失败。
|
||||
|
||||
### 6.3 读管道关闭的竞态 (L558-562 vs L228)
|
||||
|
||||
`g_lsp_impl_stop` 的时序: L558 `running=false` → L559 `proc.stop()` (关闭 hStdOut) → L562 `join`。reader 线程可能正阻塞于 `ReadFile(hStdOut)` (L228)，句柄关闭触发 FALSE 返回 → `read_line` 返回 false → loop 退出 → join 成功。此竞态**被利用**来唤醒 reader，属有意设计。可工作但脆弱。
|
||||
|
||||
评级: **F**
|
||||
|
||||
---
|
||||
|
||||
## 7. 请求 ID 管理
|
||||
|
||||
状态: **潜在 UB** WARN
|
||||
|
||||
- L284: `std::atomic<int> next_id{1};`
|
||||
- L348: `int id = g_lsp.next_id.fetch_add(1);`
|
||||
- L484: `g_lsp.next_id = 1;` (重启用)
|
||||
|
||||
问题:
|
||||
1. **fetch_add 溢出**: `std::atomic<int>::fetch_add` 在有符号整型溢出时是未定义行为 (C++ 标准 [atomics.types.int]/8)。虽 2^31 次请求极难达到，但长生命期 LS 进程不排除。
|
||||
2. **重启安全**: L484 重置为 1 时旧 pending 已在 L551 清除，安全。
|
||||
3. **窄化转换**: L386 `as_int64()` → `int` 截断（见 §5.4）。
|
||||
|
||||
评级: **B**
|
||||
|
||||
---
|
||||
|
||||
## 8. 资源清理
|
||||
|
||||
状态: **基本正确** OK
|
||||
|
||||
`on_shutdown` (L724-730): 检查 running → 调 `g_lsp_impl_stop` → 置 `g_host=nullptr`。路径正确。
|
||||
|
||||
`g_lsp_impl_stop` (L539-566):
|
||||
1. shutdown request + 2s wait
|
||||
2. exit notification
|
||||
3. `running = false`
|
||||
4. `proc.stop()` (terminate/kill 子进程)
|
||||
5. `reader_thread.join()`
|
||||
6. `diagnostics.clear()`
|
||||
|
||||
时序正确。pending_responses 在 L551 清除后，任何在 `cv.wait_for` 阻塞的调用者因 `!g_lsp.running` 被唤醒并返回 -1。无泄漏。
|
||||
|
||||
`g_host` 在 `on_shutdown` 与 service 函数间无同步 (同 W11.1 的 Race A)。缓解: host 保证 shutdown 前无 in-flight 调用。
|
||||
|
||||
评级: **B**
|
||||
|
||||
---
|
||||
|
||||
## 9. 能力协商
|
||||
|
||||
状态: **不完整** WARN
|
||||
|
||||
- `initialize` 请求中声明 hover/completion/diagnostic 能力 (L491-503)。合规。
|
||||
- `initialized` 通知在 initialize 响应后发送 (L533)。符合 LSP spec。
|
||||
- **ServerCapabilities 被丢弃** (L529): initialize 响应仅取出后删除，server 上报的能力集完全未被读取。若 server 不支持 hover，插件不会知道，仍会发送 hover 请求。
|
||||
- **rootUri 硬编码 nullptr** (L511): 多数 LS 需要 rootUri 提供项目根目录以建立索引。缺少 rootUri 可导致 go-to-definition、workspace symbols 等完全不工作。
|
||||
|
||||
评级: **C**
|
||||
|
||||
---
|
||||
|
||||
## TOP 3 严重问题
|
||||
|
||||
### 1 — [严重] initialize 超时死锁 (L519-526 → L547)
|
||||
`g_lsp_impl_start` 持有 `g_lsp.mutex` (L519) 时调用 `g_lsp_impl_stop()` (L526)，后者在 L547 再次 `unique_lock` 同一非递归 mutex → **自死锁，线程永久挂起**。任何 initialize 超时场景（server 10s 不响应）触发。
|
||||
|
||||
### 2 — [严重] 全 vtable 无异常保护，违反 §8 (L467-698 + L724-730)
|
||||
7 个 service vtable 函数 + `reader_loop` + `handle_message` + `on_shutdown` 均大量使用 `std::string` / `json::object` / `json::parse`，零 `try/catch`。OOM 或 boost::json 异常穿越 C 函数指针边界 → `std::terminate()` 进程崩溃。受影响面是 context_plugin (W11.1) 的 7 倍。
|
||||
|
||||
### 3 — [高] Server→Client Request 被静默丢弃 + error 信息丢失 (L384-412 + L651/L694)
|
||||
`handle_message` 仅分发 response/notification，`id && method` 的 server request 直接丢弃不响应。server 可能因此阻塞（如 `window/showMessageRequest` 等待用户选择）。同时 error response 的 `error` 字段被忽略 (L651/L694)，调用方无法区分超时与 server 错误。
|
||||
|
||||
---
|
||||
|
||||
## 整体评级
|
||||
|
||||
| 维度 | 评级 |
|
||||
|------|------|
|
||||
| §5+§8 异常安全 (ABI) | F (全 vtable 裸奔) |
|
||||
| §9 字符串返回 | A (完全合规) |
|
||||
| §2 跨 DLL 堆纪律 | A (0 处违规) |
|
||||
| 进程生命周期 | C (死锁 + waitpid 阻塞 + strdup 未检查) |
|
||||
| JSON-RPC 协议合规 | B (分发缺口 + error 丢弃) |
|
||||
| 状态机 / 线程同步 | F (死锁 + pipe write 竞态) |
|
||||
| 请求 ID 管理 | B (fetch_add 溢出 UB) |
|
||||
| 资源清理 | B (路径正确，缺同步) |
|
||||
| 能力协商 | C (ServerCapabilities 丢弃，rootUri 硬编码 null) |
|
||||
| **综合** | **C** |
|
||||
|
||||
**总评**: lsp_plugin 在堆纪律和字符串生命周期上完全合规（比 context_plugin 更干净），但 C++ 异常安全全线崩溃（7 个 vtable + 多个内部函数无保护）。最致命的是 initialize 超时自死锁——触发条件明确且结果确定（永久挂起）。JSON-RPC 协议处理不完整 (server request 丢弃) 在真实 LS 场景下会显著降级功能。这些问题集中在 280 行 dispatch 代码中，修复范围可控（约 300 行需加 try/catch 包裹 + 修复死锁 + 补充分发逻辑）。
|
||||
|
||||
---
|
||||
|
||||
## 补充发现 (优先级低)
|
||||
|
||||
- **`#include <boost/json/src.hpp>` (L12)**: 将整个 Boost.JSON 实现编译进插件 DLL，增加编译时间且可能与其他插件/库的 Boost.JSON 符号冲突。应改为仅 `#include <boost/json.hpp>`，通过 `target_link_libraries` 链接 boost::boost (CMakeLists.txt L16 已链接)。
|
||||
- **trim (L303-311) 与 context_plugin 重复**: 相同逻辑出现于 context_plugin 的 trim，两处独立维护。建议提取到 dstalk 核心库。
|
||||
- **MultiByteToWideChar 缓冲区硬编码 4096 (L95)**: 命令行超长时静默截断，进程可能无法正确启动。
|
||||
- **JSON-RPC "id" 同时接受 null**: LSP 规范允许 `"id": null` 表示通知。当前用 `msg.contains("id")` 仅检查 key 存在，未检查 null 值。若 server 误发 null-id response，会被当作有效响应。暂非实际问题（主流 LS 实现均守约）。
|
||||
- **initialize 超时后 g_lsp_impl_stop 仍发 shutdown/exit 给可能未初始化的 LS**: 若 server 启动但未完成 initialize 握手，收到 shutdown 行为未定义。但在实践中，多数 LS 仍能处理。
|
||||
- **Process::read_bytes L249 `buf.resize(count+1)` 后 `buf[count]='\0'`**: 刻意添加 null terminator，虽 `std::string` 不要求 null 终止，但后续 `json::parse(buf)` 需要。为性能优化（避免额外拷贝），可接受。
|
||||
Reference in New Issue
Block a user