9.9 KiB
9.9 KiB
审核专家 A：代码实现与架构安全穿透审计报告
文档属性：历史独立代码审计快照。审计问题的当前处置状态以 B→A handoff、当前架构和进度文档为准。
- 审核人：审核专家 A（系统架构与安全审核组）
- 日期：2026-10-02
- 对象：
/root/.openclaw/workspace/vps-commander工作区最新 Go 源码实现 - 对照规范：
docs/DC_REFACTORING_AND_SECURITY_SPEC.md（VPS-Commander 平滑升级与三级安全拦截实施方案） - 审查维度：架构漏洞、安全绕过、协议兼容性、并发/竞态、生产失联风险、Spec 与代码一致性
- 最终结论：判定阻断（不通过）—— 存在 3 大核心架构与安全阻断项及 3 项生产级崩溃/失联隐患，严禁直接上线生产
一、核心阻断项（Blockers - 违反 Spec 铁律，安全归零）
A1. 【致命安全绕过】Hub 信任客户端自报设备名，Per-Device 策略完全被穿透
- 违反规范：Spec 8.1 明确规定：
“
identify device不得等同于读取请求中的device字段。设备名称只能作为路由提示，不能作为授权依据。禁止：policy := snapshot.Device(r.Device)。” - 代码位置：
internal/api/api.go全量 REST 路由（processSession,listDirectory,fileInfo,editBlock等）if req.Device == "" { req.Device = s.LocalName } if !s.requireSecurity(w, r, req.Device, "exec", req.SessionID) { return } - 漏洞事实：Hub 在调用
s.requireSecurity计算生效风控等级（EffectiveRisk）时，直接使用了未经签名、未经校验的请求体参数req.Device。 - 真实危害：
攻击者或不受信客户端若需在严格受控节点 A（策略为
Low，禁止执行命令）上执行高危动作，只需在调用接口时将device声明为策略宽松的节点 B（策略为High），或者置空使其 fallback 到s.LocalName，即可成功骗过 Hub 的策略判定，造成三级风控彻底被穿透与权限降级逃逸。 - 整改要求： 必须基于认证凭据（如 mTLS 证书主体、Node Token 或已握手隧道的 Canonical Device ID）作为策略计算的唯一权威源。
A2. 【架构旁路后门】MCP 适配器零门禁直通集群
- 违反规范：Spec 7.3 明确规定：
“所有对外暴露能力（REST API、MCP Stdio、MCP SSE）必须收敛于统一的 Security Snapshot 门禁。”
- 代码位置：
internal/api/mcp_adapter.go与internal/mcp/mcp.go - 漏洞事实：
MCPBackend实现的所有底层操作（ExecCommand,ReadFile,WriteFile,EditBlock,CallAgentMCP等）没有一处引用security.Snapshot或调用requireSecurity，直接裸调集群 Manager 执行。 - 真实危害：
/mcp/sse和vps-commander-mcp-stdio是 AI Agent 实际高频使用的核心入口。当前实现相当于REST 侧设了关卡，但 AI 的主通道直接留了后门。持合法 MCP 凭证即可任意下发 High 级命令与文件覆写，安全门禁完全失效。 - 整改要求：
在
mcp_adapter.go每一项 Tool 处理入口前，必须强制注入s.server.Policy.EffectiveRisk校验，未通过直接返回 Tool Error。
A3. 【进程治理漏洞】force_terminate / kill_process 缺失进程树归属校验
- 违反规范：Spec 8.2 明确规定：
“
kill_process必须基于 Session/PGID 树状绑定，禁止盲信裸 PID。不得终止非当前 Agent 创建或非目标 Session 所属的系统进程。” - 代码位置：
internal/executor/session.go、internal/cluster/protocol.go - 漏洞事实：
处理终止进程请求时，底层直接根据传入的
pid调用操作系统信号发送函数，没有校验该 PID 是否归属于SessionManager所纳管的进程树（Process Tree / Process Group）。 - 真实危害： 在 Linux 高并发场景下存在 PID 重用（PID Recycling）。如果被终止的会话子进程恰好刚退出，操作系统将该 PID 重新分配给了系统核心服务（如 Hub 自己、systemd-journald、网络守护进程等），Agent 将直接误杀宿主机关键系统进程。
- 整改要求：
必须以
SessionID为主键查找*Session对象，从内部持有的cmd.Process提取句柄并执行 Kill；拒绝接受客户端直传任意孤立 PID。
二、生产级严重隐患（生产必现崩溃、失联与数据破坏）
A4. 【并发 Panic / 进程崩溃】Agent WebSocket 并发写入缺失互斥保护
- 代码位置：
cmd/agent/main.gowriteJSON := func(msg cluster.Message) error { return c.WriteJSON(msg) // 💥 裸写 WebSocket 连接，无任何锁保护！ } ... go func(msg cluster.Message) { handle(...) // 内部异步执行任务并调用 writeJSON 回传响应 }(msg) - 隐患事实：
Gorilla WebSocket 官方规范明确说明：
*websocket.Conn不支持并发写入。在当前 Agent 实现中，每条消息都会派生一个 goroutine 并发执行writeJSON，同时心跳定时器也在周期性调用writeJSON。 - 后果：
在并发下发任务或心跳与命令响应撞车的瞬间，Go 运行时会直接抛出：
fatal error: concurrent write to websocket connection，导致 Agent 进程当场崩溃退出。 - 整改要求：
必须对 Agent 的
*websocket.Conn封装sync.Mutex，确保所有 JSON 写回与心跳握手互斥串行化。
A5. 【生产自杀失联】Agent 常规任务报错自杀式断开隧道
- 代码位置：
cmd/agent/main.goif err := handle(msg); err != nil { log.Printf("handle error: %v", err) c.Close() // 💥 业务层报错直接杀掉物理 WebSocket 连接！ return } - 隐患事实：
当用户执行一个不存在的命令（如
exec_command("foo_bar")导致exec: "foo_bar": executable file not found in $PATH）、读取权限不足的文件或执行超时，handle返回常规错误。Agent 竟然直接执行了c.Close()撕毁长连接隧道。 - 后果： 一次轻微的业务操作失误或命令打错，直接导致 Agent 离线失联，Hub 显示该节点掉线，必须等待重连甚至人工干预。
- 整改要求：
移除
c.Close()。业务错误必须包装为cluster.Message{Status: "error", Error: err.Error()}原路回传给 Hub，保持底层物理连接健康存活。
A6. 【TOCTOU 丢数据】edit_block 缺失代际指纹（Generation Fingerprint）并发保护
- 违反规范：Spec 8.8 明确规定：
“
edit_block不能只做简单的read -> verify -> rename……必须在读取阶段生成 generation fingerprint (mtime/size/sha256)，写回前必须做 CAS 级比对；一旦发生变动，必须拒绝修改并返回 409 Conflict，防止并发覆写覆盖。” - 代码位置：
internal/executor/edit.godata, err := os.ReadFile(path) ... count := countNonOverlapping(src, oldText) if count != 1 { return ... } newContent := strings.Replace(src, oldText, newText, 1) // 直接写写入临时文件并通过 os.Rename 强制覆盖！ - 隐患事实：
edit_block仅在内存中做了非重叠计数校验，完全缺失目标文件的版本比对机制（如expected_hash或mtime）。 - 后果： 在多会话或多 Agent 并发场景下，A 和 B 几乎同时读取同一配置文件。A 先行写入了第 10 行并落盘；B 随后提交第 50 行的修改，由于 B 基于旧内容替换且单匹配依然成立，B 的原子 Rename 将直接静默抹除 A 刚提交的全部修改，造成不可逆的生产配置丢失。
- 整改要求：
接口必须支持并强制校验
expected_hash，写回落盘前再次哈希比对，哈希不符直接阻断并返回冲突错误。
三、高优先级架构缺陷（Phase 3 前必须收敛）
A7. 【审计链断裂】策略版本与 audit_logs 落库脱节
- 代码位置：
internal/api/api.go的UpdateSecuritySettingsJSON - 问题：策略变更先提交了 SQLite 事务并推进了内存版本号，随后才执行 Audit 写入。若两步之间服务异常断电或重启，会导致版本号已变动，但审计表中无任何迹象与操作人。审计日志必须在同一事务内完成提交。
A8. 【资源泄漏】Session 清理机制单轨制
- 代码位置：
internal/executor/session_manager.go - 问题：
Cleanup()仅清理了已处于Running == false的死会话，对于挂起、死循环但已超过最大生存期的卡死会话毫无治理能力。必须落地 Spec 要求的「空闲超时 + 最大绝对生命期（Hard TTL）」双轨强制收割机制，并调用cmd.Wait()彻底回收僵尸进程。
四、值得肯定的实现亮点
- 能力握手规范已落地：
cluster.Message成功扩充了ProtocolVersion、AgentVersion与Capabilities列表，为后续功能向后兼容奠定了坚实基础； - Session ID 强随机性：统一采用
crypto/rand生成 128-bit 随机 ID，彻底杜绝了伪随机与 ID 碰撞预测； - 文件写入原子性良好：
edit_block与write_file均规范采用了临时文件写入、Sync()刷盘及os.Rename替换机制。
五、专家 A 评审总结与准入判定
当前工作区代码虽然跑通了单元测试（go test ./... PASS），但表面的功能实现严重脱离了 Spec 的安全承诺：
- MCP 入口裸奔（A2） 与 设备自报伪造（A1） 共同导致三级风控成为空壳；
- Agent 并发写 WS（A4） 与 报错自杀式断连（A5） 将在生产环境导致大面积崩溃与失联；
- 缺失 CAS 校验（A6） 存在高概率的并发配置丢失风险。
准入结论：不予准入上线。必须依照上述清单完成 A1~A6 的靶向修复并通过覆盖性验收测试后，方可推进合并。