diff --git a/cmd/mcp-stdio/main.go b/cmd/mcp-stdio/main.go index 44610ec..40e5035 100644 --- a/cmd/mcp-stdio/main.go +++ b/cmd/mcp-stdio/main.go @@ -100,6 +100,13 @@ func (h *HubRemoteBackend) ReadFile(ctx context.Context, clientIP, device, path return res, err } +func (h *HubRemoteBackend) EditBlock(ctx context.Context, clientIP, device, path, oldText, newText string) (any, error) { + payload := map[string]any{"device": device, "path": path, "old_text": oldText, "new_text": newText} + var res any + err := h.post("/api/v1/devices/file/edit_block", payload, &res) + return res, err +} + func (h *HubRemoteBackend) WriteFile(ctx context.Context, clientIP, device, path, content string) (any, error) { payload := map[string]any{ "device": device, diff --git a/docs/DC_PHASE1_IMPLEMENTATION.md b/docs/DC_PHASE1_IMPLEMENTATION.md new file mode 100644 index 0000000..4a9b932 --- /dev/null +++ b/docs/DC_PHASE1_IMPLEMENTATION.md @@ -0,0 +1,79 @@ +# DC Refactoring Phase 1 Implementation Record + +Date: 2026-10-02 + +## Completed + +- Added immutable Hub `PolicySnapshot` protected by `RWMutex`. +- Added monotonic security policy version persisted in SQLite. +- Hub startup now rebuilds the snapshot from SQLite; snapshot load failure aborts startup rather than defaulting to High. +- Security setting writes use SQLite transactions and only publish the new snapshot after the durable write succeeds. +- Added a compile-time Tool Registry with fixed `RequiredRisk` for Low/Medium/High capabilities. +- Existing API security checks now resolve required risk through the registry instead of duplicating risk numbers in handlers. +- Security-setting audit records include old/new effective mode and snapshot version. + +## Compatibility / Safety + +- Existing API routes and Agent WebSocket protocol were not changed in this phase. +- No production Hub/Agent restart was performed. +- When `Server.Policy` is initialized, security checks use the in-memory snapshot and do not query SQLite on the request hot path. +- Existing tests that construct `Server` without a snapshot retain the legacy storage fallback for test/backward compatibility; production startup always initializes the snapshot. + +## Verification + +- `go test ./...` passed. +- `go test -race ./...` passed. +- Hub and Agent clean builds passed. +- No production service restart was performed; live rollout remains a separate controlled step to preserve the no-interruption requirement. + +## Phase 2 Completed + +- Added `edit_block` to the Agent execution engine. +- `old_text` must match exactly once; zero/multiple matches are rejected without mutation. +- Replacement uses a same-directory temporary file, preserves file permission bits, fsyncs the file, renames atomically, and syncs the directory. +- Added Agent RPC action `edit_block`, Hub `/api/v1/devices/file/edit_block`, MCP `edit_block`, and stdio proxy support. +- `edit_block` is registered as Medium risk and therefore follows the existing Hub policy snapshot gate. +- Added optional `sub_action` protocol field without changing existing message semantics. +- Added tests for unique replacement, duplicate/missing rejection, permissions, Low-mode denial, and MCP registration. + +Phase 2 rollout remains deliberately separate from production process replacement. Existing production Agent connections have not been restarted. + +## Phase 3 Completed + +- Added a per-Agent `SessionManager` with a bounded session count (default 16). +- Added fixed-size 2 MiB stdout/stderr RingBuffers; old output is discarded when capacity is exceeded. +- Added long-running process actions: `start_process`, `read_process_output`, `interact_with_process`, `force_terminate`, `list_sessions`. +- Processes run in their own process group so termination/force termination covers the command tree. +- Added periodic completed-session cleanup. +- Added Hub RPC and HTTP `/api/v1/process/session` dispatch with action-based lifecycle operations. +- Added session tests for ring retention, output capture, termination, duplicate IDs, and session limits. +- Existing WebSocket request/response protocol remains compatible; new process actions are additive. + +Production Hub/Agents were not restarted during Phase 3 implementation. + +## Phase 3 Security Hardening / Concurrency Gate + +- Added per-node operation arbitration in `cluster.Node.OpMu`. +- Low-risk read actions use `RLock` and may run concurrently. +- Medium/High mutations use exclusive `Lock`, preventing overlapping writes/control operations against the same node. +- Unknown actions fail closed as High risk for the concurrency gate. +- Session IDs are generated with 128-bit `crypto/rand` entropy when omitted by the API caller; PID is never used as identity. +- Session ownership is bound to a non-reversible SHA-256 fingerprint of the authenticated API credential and checked by Agent for read/input/terminate operations. +- Session TTL is enforced; stdin is capped at 64 KiB per call. +- Agent restart still leaves all sessions in process memory only, so old session state is not restored. + +All validation passed with `go test ./...`, `go test -race ./...`, Hub/Agent builds, and `git diff --check`. No production process was restarted. + +## Phase 4 Completed + +- Added optional protocol/agent capability metadata: `ProtocolVersion`, `AgentVersion`, `Capabilities`. +- Hub records the capability set per connected Agent; when a modern Agent explicitly advertises capabilities, unsupported actions fail with `capability missing` rather than silently degrading. +- Old Agents remain compatible because capability metadata is optional and capability enforcement is only applied when a capability set is advertised. +- Added structured read-only filesystem operations: `list_directory`, `get_file_info`, `start_search`. +- Directory listing supports bounded recursive traversal and a maximum entry count. +- File metadata exposes type, size, mode and modification time. +- Search is context-cancellable and bounded by a result cap; symlinks are skipped to avoid traversal surprises. +- Added Hub endpoints `/api/v1/file/list`, `/api/v1/file/info`, `/api/v1/file/search`. +- Added executor tests for directory listing, file metadata and bounded search. + +Phase 4 validation: full tests, race tests, Hub/Agent builds and `git diff --check` passed. Production Hub/Agents were not restarted. diff --git a/docs/DC_REFACTORING_AND_SECURITY_SPEC.md b/docs/DC_REFACTORING_AND_SECURITY_SPEC.md new file mode 100644 index 0000000..94f366c --- /dev/null +++ b/docs/DC_REFACTORING_AND_SECURITY_SPEC.md @@ -0,0 +1,934 @@ +# 《VPS-Commander 平滑升级改造与三级安全拦截实施方案》 + +## 1. 改造目标与背景 +本项目计划全量复刻 **Desktop Commander(DC)** 在终端与文件系统上的核心工程能力(包括局部块级精准替换、后台长进程生命周期管理、交互式 stdin 注入、结构化目录遍历与只读检索),并将其无缝接入 **Low(低风险)/ Medium(中风险)/ High(高风险)** 三级安全门禁体系。 + +**核心红线与验收标准:** +1. **服务不中断**:升级过程不影响正在运行的 Agent 探针与 Hub 中心。 +2. **协议向后兼容**:旧版 Agent 与新版 Hub 混合共存,老接口绝不废弃。 +3. **性能零损耗**:杜绝动态运行时映射,安全校验在编译期即已绑定。 +4. **轻量化保真**:继续保持纯 Go 静态编译的高效特性,严控内存占用(常驻内存锁定在 20MB 以内)。 + +--- + +## 2. 工具池全量复刻与三级风险定级表(静态强绑定) + +所有功能根据系统破坏力与副作用大小,在**编译期**严格归属于三级风险体系: + +| 风险等级 (Level) | 行为边界定义 | 包含的复刻工具 (DC 标杆) | 允许放行的安全模式 | +| :--- | :--- | :--- | :--- | +| 🟢 **Low(只读/感知)** | **纯只读、无状态改变、无系统副作用** | • `read_file` (支持切片与 Tail)
• `read_multiple_files`
• `list_directory` (结构化目录树)
• `get_file_info` (元数据)
• `start_search` / `get_more_search_results`
• `list_processes` / `list_sessions`
• `read_process_output` (拉取输出) | **Low**
**Medium**
**High** | +| 🟡 **Medium(文件写)** | **文件与目录变更,但严禁拉起任意二进制或执行命令** | • `edit_block` (局部文本块精准替换,核心)
• `write_file` (文件新建/全量写入)
• `create_directory` (新建目录)
• `move_file` (移动/重命名) | **Medium**
**High**
*(Low 拦截)* | +| 🔴 **High(完全控制)** | **具备操作系统执行权、进程生命周期调度与管道干预** | • `exec_command` (同步短命令)
• `start_process` (后台异步启动长进程)
• `interact_with_process` (stdin 交互写入)
• `kill_process` / `force_terminate` (终止进程) | **High**
*(Low / Medium 拦截)* | + +--- + +## 3. 架构改造与平滑过渡设计 + +### 3.1 协议层平滑升级(零破坏向后兼容) +在 `internal/cluster/protocol.go` 中,保留原有字段,新增可选的扩展动作: +```go +type Message struct { + ID string `json:"id,omitempty"` + Action string `json:"action,omitempty"` // 保持原有字段: "read" / "write" / "exec" / "probe" + SubAction string `json:"sub_action,omitempty"` // ✨ 增量动作: "edit_block", "start_process" 等 + Device string `json:"device,omitempty"` + Payload any `json:"payload,omitempty"` + ... +} +``` +* **兼容保障**:老版本 Agent 接收到带 `SubAction` 的 JSON 消息时会自动忽略多余字段,继续按老逻辑工作;升级后的 Agent 优先路由给 `SubAction` 处理。 + +### 3.2 Hub 门禁与高并发性能优化(读写分离) +现有系统每次请求均去 SQLite 查询策略,高并发下存在文件锁瓶颈。改造后引入 **RWMutex 内存策略快照**: +1. **读路径(Fast Path,纳秒级)**: + ```go + // 常量数值定义,直接内联比较 + const ( RiskLow = 1; RiskMedium = 2; RiskHigh = 3 ) + + func (s *Server) requireSecurity(w http.ResponseWriter, r *http.Request, device string, requiredRisk int, detail string) bool { + allowed := s.getEffectiveRiskLevel(device) // 查内存 RLock,耗时 < 10ns + if allowed >= requiredRisk { + return true + } + // 记录 audit_logs ("security_denied", 403) 并返回拦截响应 + return false + } + ``` +2. **写路径(Slow Path,低频)**: + * 仅在管理员通过 Web 界面更改安全策略时,写一次 SQLite 并原子更新内存缓存。 +3. **老 API 零改动**: + * 原有 `/api/v1/devices/exec`、`/read`、`/write` 继续保留,老脚本老客户端零感知。 + +### 3.3 Agent 端核心引擎实现(防资源膨胀设计) +* **`edit_block` 引擎**: + * 读取原文件后校验 `old_text` 全局唯一性,唯一时单次替换并使用临时文件进行原子覆写(`os.Rename`)。 + * 相比原有全量上传,网络传输和内存开销下降 95%。 +* **长进程与 PTY 会话引擎(`start_process` 等)**: + * Agent 内部建立轻量 `SessionManager`,挂接内核标准输入输出管道。 + * **硬编码内存锁(Circular RingBuffer)**:每个进程会话分配最大 2MB 环形日志缓冲区,达到上限自动覆盖最早输出,**从物理上杜绝日志死循环导致的内存溢出(OOM)**。 + +--- + +## 4. 资源开销评估与性能上限控制 + +得益于纯 Go 原生静态编译,系统坚决不引入任何 Node.js/Python 运行时,资源开销保持极致克制: + +| 评估指标 | 改造前基准(实测) | 改造后预期 | 性能/资源防护机制 | +| :--- | :--- | :--- | :--- | +| **Agent 静默内存** | ~13 MB | **~15 MB - 18 MB** | 零后台轮询,事件驱动 | +| **Agent 峰值内存** | 随命令大小波动 | **基准 + (2MB × 活跃会话数)** | 环形缓冲区硬顶锁定,防内存泄露 | +| **Hub 服务端内存** | ~16 MB | **~18 MB - 22 MB** | 内存门禁仅占不到 10KB | +| **二进制体积** | ~9.4 MB | **~11 MB - 13 MB** | 静态单文件编译,极易分发部署 | +| **安全判定延迟** | 毫秒级(查 SQLite) | **纳秒级(内存读锁)** | 单机可轻松支撑 10W+ QPS 门禁并发 | + +--- + +## 5. 四阶段实施与回滚路线图(稳健落地) + +* **Phase 1:服务端门禁升级与内存读缓存** + * 在 Hub 端落地 `RWMutex` 安全策略缓存; + * API 入口统一接入 `requireSecurity(..., RiskLevel)`; + * *验证标准*:旧版 4 项基础功能与 Web 面板完好,并发查询 QPS 大幅提升。 +* **Phase 2:落地中风险神器 `edit_block`** + * Agent 端实现块级精准替换算法; + * Hub 暴露 `/api/v1/devices/file/edit_block` 并注册至 MCP Adapter; + * *验证标准*:在 Medium 模式下可正常修配置文件,在 Low 模式下被精确阻断并记入审计。 +* **Phase 3:落地高风险长进程引擎** + * Agent 端引入带 RingBuffer 的 `SessionManager`; + * 实现 `start_process`、`read_process_output`、`interact_with_process`、`kill_process`; + * *验证标准*:跑耗时任务不会超时断连,支持实时 Tail 查看输出与注入 stdin。 +* **Phase 4:落地只读结构化文件与系统感知(Low)** + * 实现 `list_directory`、`get_file_info` 及后台异步只读搜索。 + +--- + +## 6. 评估与审计重点建议 +1. **安全维度**:Low/Medium/High 对能力的切分是否彻底杜绝了提权与逃逸风险? +2. **性能维度**:使用内存 RWMutex + 编译期静态安全级别判定,是否彻底解决了单文件 SQLite 高频读瓶颈? +3. **稳定性维度**:为每个长进程会话限定 RingBuffer 内存上限,是否有效防止了边缘 VPS 探针被日志撑爆? + +--- + +# 7. 架构收敛修订(2026-10-02) + +> 本章节在进入代码实施前增加,用于两位外部 Agent 审核。原方案的工具分级、四阶段路线和向后兼容原则继续保留;本章节对安全权威、策略一致性、工具注册以及高风险 Session 生命周期作进一步约束。 + +## 7.1 安全等级的正确边界:静态工具风险 + 运行时设备策略 + +原方案中“静态强绑定”应准确解释为:**工具所需风险等级在编译期固定,是否允许执行在运行时由 Hub 的权威策略决定。** + +不得将设备当前 Low/Medium/High 策略编译进 Agent,也不得允许 Agent 自行提升自身安全等级。 + +### 7.1.1 工具风险等级 + +每个工具在代码注册表中拥有不可运行时修改的 `RequiredRisk`: + +```go +const ( + RiskLow = 1 + RiskMedium = 2 + RiskHigh = 3 +) + +type ToolSpec struct { + Name string + RequiredRisk int + ReadOnly bool +} +``` + +示例: + +```text +read_file / list_directory / search -> Low +edit_block / write_file / move_file -> Medium +exec_command / start_process / stdin -> High +``` + +禁止通过 Web 面板、Agent 配置文件或远端请求动态修改工具自身的 `RequiredRisk`。 + +### 7.1.2 设备有效策略 + +设备的 `EffectiveRisk` 是 Hub 运行时策略,不属于 Agent 自主能力。 + +请求授权模型: + +```text +Request + -> identify device + -> identify ToolSpec.RequiredRisk + -> read Hub PolicySnapshot + -> compare EffectiveRisk >= RequiredRisk + -> allow / deny +``` + +因此完整安全边界为: + +**编译期固定“工具需要什么权限” + 运行时决定“该设备当前获得什么权限”。** + +--- + +## 7.2 Policy Store 与 Policy Snapshot 一致性模型 + +SQLite 继续作为持久化权威源(durable source of truth),但请求热路径禁止每次查询 SQLite。 + +### 7.2.1 数据流 + +```text +Admin Web + | + v +SQLite transaction + | + v +Build new immutable PolicySnapshot + | + v +Atomic publish / swap + | + v +Request fast path +``` + +请求只读取当前不可变 snapshot;不在请求过程中修改策略对象。 + +### 7.2.2 更新原则 + +策略修改必须满足: + +1. SQLite 写入成功后才允许发布新 snapshot。 +2. SQLite 写失败时,内存策略保持旧版本。 +3. snapshot 发布必须是原子操作,不允许出现半更新状态。 +4. Hub 启动时必须从 SQLite 完整重建 snapshot。 +5. snapshot 应携带单调递增 `Version`,便于审计和诊断。 +6. 审计记录应包含 `device`, `oldRisk`, `newRisk`, `version`, `actor`, `timestamp`。 + +推荐结构: + +```go +type PolicySnapshot struct { + Version uint64 + Devices map[string]DevicePolicy + CreatedAt time.Time +} +``` + +热路径原则: + +```text +load current snapshot + -> lookup device + -> compare integer risk + -> allow/deny +``` + +不得在高频 API 请求中重新读取 SQLite 策略表。 + +### 7.2.3 故障安全 + +如果 Hub 无法加载有效策略 snapshot: + +- 不得默认放行 High。 +- 不得从请求参数接受临时风险等级。 +- 对需要 Medium/High 的操作默认拒绝。 +- Low 只读能力是否可继续开放,必须由启动时的明确安全默认策略决定,并记录审计事件。 + +--- + +## 7.3 Tool Registry:统一能力与风险注册表 + +避免各 API handler 自己实现不同的安全判断。所有能力必须进入统一 Tool Registry。 + +建议: + +```go +type ToolSpec struct { + Name string + RequiredRisk int + Category string + ReadOnly bool + Description string +} +``` + +例如: + +```text +read_file Low +read_multiple_files Low +list_directory Low +get_file_info Low +start_search Low +get_more_search_results Low +list_processes Low +list_sessions Low +read_process_output Low + +edit_block Medium +write_file Medium +create_directory Medium +move_file Medium + +exec_command High +start_process High +interact_with_process High +kill_process High +force_terminate High +``` + +API、MCP Adapter 和内部调用均通过 Registry 获取 `RequiredRisk`,不得在多个入口复制风险数字。 + +--- + +## 7.4 Agent 信任边界 + +Agent 是执行者,不是安全策略的最终裁决者。 + +Hub 是设备授权策略的权威来源。 + +但 Agent 仍必须执行本地最低限度安全校验,包括: + +- 请求格式与参数校验; +- session 所有权校验; +- session 是否仍然有效; +- 单次输入/输出大小限制; +- 超时和资源限制; +- 不接受客户端自行声明的 `RiskLevel` 作为授权依据。 + +Agent 不得因为请求中携带: + +```json +{"risk":"high"} +``` + +或类似字段而自行获得 High 权限。 + +如果协议未来需要携带授权信息,应使用 Hub 生成的受保护授权上下文,并绑定: + +```text +device + request/session + policy version + expiry +``` + +具体签名/令牌方案在实现阶段单独评审。 + +--- + +## 7.5 High Risk SessionManager 安全模型 + +`start_process`、`interact_with_process`、`kill_process` 不应被视为普通 HTTP 命令,而应视为一个受生命周期管理的高风险 Session。 + +### 7.5.1 Session 属性 + +每个 Session 至少包含: + +```go +type ProcessSession struct { + ID string + Owner string + Device string + PID int + CreatedAt time.Time + LastAccess time.Time + ExpiresAt time.Time + State string + OutputBytes uint64 +} +``` + +Session ID 必须不可预测,并且不能复用操作系统 PID 作为唯一身份。 + +### 7.5.2 所有权 + +只有创建 Session 的授权调用方才能: + +- 读取该 Session 输出; +- 向 stdin 写入数据; +- 终止该 Session。 + +管理员级操作如果未来需要跨 Owner 接管 Session,应定义明确的审计事件和单独权限,不得通过普通 session API 隐式获得。 + +### 7.5.3 生命周期 + +```text +start_process + | + v + RUNNING + / \ + / \ +EXITED EXPIRED + | | + +----+---+ + v + CLEANUP +``` + +Agent 重启后所有旧 Session 必须失效,不得从持久化文件自动恢复可写 stdin 的旧 Session。 + +### 7.5.4 时间限制 + +Session 必须具备: + +- 最大生命周期; +- 空闲超时; +- stdin 单次大小限制; +- stdin 速率限制; +- 输出缓冲区上限。 + +具体默认数值在实现前通过压测确定,不在设计阶段硬编码未经验证的数值。 + +--- + +## 7.6 RingBuffer 与全局资源上限 + +原方案中的“2MB × 活跃 Session”只能作为单 Session 上限,不应视为 Agent 的总内存预算。 + +必须同时存在: + +```text +per-session output cap ++ +max active sessions ++ +agent-wide session buffer cap ++ +process creation rate limit +``` + +例如: + +```text +单 Session:最大 2MB output buffer +全局:最大 N 个活跃 Session +全局:最大 M MB Session buffer +``` + +当达到任意全局上限时,新 Session 必须被拒绝并记录审计,而不是继续消耗内存。 + +RingBuffer 的覆盖行为只能覆盖旧输出,不得覆盖 session 元数据、错误状态或安全审计记录。 + +--- + +## 7.7 审计要求 + +至少记录以下安全事件: + +```text +security_denied +policy_changed +session_created +session_expired +session_killed +session_owner_denied +resource_limit_denied +``` + +High Risk 操作的审计信息必须包含: + +```text +device +actor / owner +request id +session id(如适用) +tool +result +policy version +timestamp +``` + +审计日志不能因为 Session RingBuffer 达到上限而被覆盖。 + +--- + +## 7.8 协议兼容补充约束 + +旧 Agent 与新 Hub 混合运行期间: + +1. 原有 `action=read/write/exec/probe` 字段继续有效。 +2. 新 `SubAction` 为可选字段。 +3. 新字段不能改变旧 Agent 对旧消息的既有语义。 +4. Hub 必须根据 Agent capability/version 判断是否可以下发新 SubAction。 +5. 对不支持新能力的 Agent,返回明确的 capability error,而不是让旧 Agent 猜测。 +6. 安全门禁必须在能力路由之前执行,避免通过旧 API 绕过新 Tool Registry。 + +--- + +## 7.9 实施前必须完成的设计验收 + +在进入 Phase 1 代码改造前,两位审核 Agent 应重点审查: + +### A. 权威性 + +- Hub 是否始终是 EffectiveRisk 的唯一权威来源? +- Agent 是否存在任何自升权路径? +- 旧 API 是否可能绕过 Tool Registry? + +### B. 一致性 + +- SQLite commit 与 snapshot publish 顺序是否正确? +- Hub 重启能否确定性恢复策略? +- snapshot version 是否足以定位策略变更? + +### C. Session 安全 + +- session ownership 是否不可伪造? +- Agent 重启后旧 session 是否必然失效? +- stdin、stdout、session 数量是否存在全局资源上限? + +### D. 向后兼容 + +- 老 Agent + 新 Hub 是否继续工作? +- 新 Hub 是否能识别 Agent capability? +- 新能力是否可能通过旧接口绕过门禁? + +### E. 性能 + +- 热路径是否完全脱离 SQLite? +- snapshot 是否真正 immutable / atomic? +- session ring buffer 是否存在隐藏的额外复制? + +**只有上述问题通过审核后,才进入 Phase 1 实现。** + +--- + +# 8. 审核意见收敛修订(2026-10-02) + +本章节根据第一轮 Agent 审核结果补充。**Phase 1 在本章节阻断项完成前不得进入代码实施。** + +## 8.1 阻断项一:设备身份认证 + +`identify device` 不得等同于读取请求中的 `device` 字段。设备名称只能作为路由提示,不能作为授权依据。 + +### 8.1.1 权威身份模型 + +每个 Agent 必须拥有唯一设备身份以及与之绑定的设备凭证。Hub 必须先完成设备认证,再建立: + +```text +authenticated agent identity + ↓ +device ID + ↓ +PolicySnapshot lookup + ↓ +EffectiveRisk +``` + +推荐优先采用 **per-agent credential**;部署条件允许时使用 mTLS。若采用 Agent Token,Token 必须: + +- 与单一 Agent 身份绑定; +- 可撤销; +- 不得由客户端自行指定 device 后复用; +- 传输必须经过受保护通道; +- 认证失败不得进入安全策略查询阶段。 + +`device` 请求字段不得覆盖认证得到的 canonical device identity。 + +### 8.1.2 身份与策略绑定 + +策略查询必须使用认证层产生的 canonical device ID: + +```go +identity := authenticateAgent(r) +if !identity.OK { + deny() +} +policy := snapshot.Device(identity.DeviceID) +``` + +禁止: + +```go +policy := snapshot.Device(r.Device) +``` + +设备注册、凭证轮换、凭证撤销必须产生审计事件。具体凭证格式与密钥轮换流程在 Phase 1 实施设计中单独冻结。 + +--- + +## 8.2 阻断项二:kill_process 作用域 + +`kill_process` / `force_terminate` **绝不能接受任意 PID 作为授权对象**。 + +High Risk Session 必须建立进程归属关系: + +```text +Session + └── Process Group / Process Tree + ├── root process + ├── child + └── child... +``` + +默认允许终止的对象仅为该 Session 创建的进程树。 + +必须拒绝: + +- 不属于当前 Session 的 PID; +- Hub / Agent 自身关键进程; +- 未经单独授权的其他用户进程; +- 通过伪造 PID、session ID 或 owner 字段进行的跨 Session 操作。 + +实现时应记录 root PID / process group ID,并通过 OS 级关系验证目标进程仍属于该 Session,而不是只相信客户端提供的 PID。 + +未来若需要管理员跨 Session 终止能力,必须作为独立能力和独立审计事件设计,不复用普通 `kill_process`。 + +--- + +## 8.3 阻断项三:Agent capability/version 握手 + +协议必须正式增加 capability/version 元数据,使 7.8 的混合版本策略可以实际落地。 + +建议扩展: + +```go +type Message struct { + ID string `json:"id,omitempty"` + Action string `json:"action,omitempty"` + SubAction string `json:"sub_action,omitempty"` + Device string `json:"device,omitempty"` + Payload any `json:"payload,omitempty"` + + ProtocolVersion string `json:"protocol_version,omitempty"` + AgentVersion string `json:"agent_version,omitempty"` + Capabilities []string `json:"capabilities,omitempty"` +} +``` + +为避免旧 Agent 解析失败,新增字段必须保持 optional;旧 Agent 忽略未知字段即可继续处理旧消息。 + +更严格的 capability 协商流程: + +```text +Agent connect + ↓ +identity authentication + ↓ +protocol/version handshake + ↓ +capability registry + ↓ +Hub records effective capabilities + ↓ +request routing +``` + +Hub 不得仅根据版本字符串猜测能力。实际可用能力以 capability set 为准。 + +如果 Agent 不支持某个 `SubAction`: + +```text +capability_missing +``` + +必须成为明确错误,不得静默降级到语义不同的危险操作。 + +--- + +## 8.4 威胁模型 + +三级安全体系主要解决的是 **调用方对受控设备的越权操作风险**,而不是保证已经被完全攻陷的 Agent 本身可信。 + +### T1:恶意调用方 + +目标:冒充设备、伪造风险等级、跨设备操作、跨 Session 操作。 + +防护: + +- Agent 身份认证; +- canonical device ID; +- Hub authoritative PolicySnapshot; +- Session ownership; +- Tool Registry; +- 审计。 + +### T2:有 Bug 的调用方 + +目标:错误调用 High API、重复创建 Session、资源耗尽。 + +防护: + +- 参数验证; +- per-device 并发限制; +- rate limit; +- Session/global resource cap; +- fail-closed。 + +### T3:被攻陷的 Agent + +如果攻击者已经取得 Agent 进程的等价本地执行权,则 Agent 内部校验不能被视为可信边界。 + +因此: + +> Hub 门禁保护的是“未经授权的远程调用不能获得设备能力”,而不是“Agent 被完全攻陷后仍能阻止本地攻击者”。 + +Agent 被攻陷属于独立威胁域,应依赖最小权限、系统账户隔离、凭证撤销、节点隔离和重新注册等机制处理。 + +文档不得把 Agent 侧校验描述为能够抵御完全控制 Agent 的攻击者。 + +--- + +## 8.5 审计敏感数据脱敏 + +现有 `audit_logs.command` 等字段不得无条件记录完整命令或参数。 + +审计系统必须采用结构化事件优先: + +```text +command template / tool +arguments metadata +result +risk +actor +request id +``` + +对于可能包含 Secret 的字段,必须在进入持久化审计前脱敏,包括但不限于: + +```text +password +passwd +token +access_token +refresh_token +api_key +secret +authorization +cookie +private_key +``` + +Shell 命令中的敏感值不能仅依赖字段名识别;实现阶段应定义命令审计策略,至少做到: + +1. 不默认记录完整 stdin; +2. 对已识别敏感参数替换为 `[REDACTED]`; +3. Secret 不得通过错误消息重新写入 audit; +4. 原始命令仅在明确安全策略允许时保留。 + +--- + +## 8.6 回滚与混合版本路线 + +原第 5 节标题为“回滚路线图”,现补充实际策略。 + +### 8.6.1 Hub 升级失败 + +- 新版本不得在旧 schema 尚未兼容时直接删除旧字段; +- DB migration 必须可检测版本; +- migration 失败则 Hub 不进入正常服务状态; +- 策略 snapshot 可以从旧兼容 schema 重建; +- 升级前保留 DB 备份/快照。 + +### 8.6.2 新 Hub + 旧 Agent + +旧 Agent 继续使用旧 API/旧 Action。 + +Hub 必须: + +```text +认证旧 Agent + ↓ +读取其 capability + ↓ +只下发它声明支持的能力 +``` + +不能因为 Hub 已升级就要求旧 Agent 自动支持新 SubAction。 + +### 8.6.3 新 Agent + 旧 Hub + +新 Agent 必须保持旧协议兼容能力;若 Hub 不支持 capability handshake,新 Agent 使用 legacy compatibility mode,不得因此自行启用 High 新能力。 + +### 8.6.4 数据降级原则 + +数据库 schema 新增字段应尽量可选;新功能数据不能成为旧版本启动的硬依赖。 + +任何不可逆 migration 必须在正式 Phase 前单独评审。 + +--- + +## 8.7 Session ID 与进程安全参数 + +Session ID 必须使用 `crypto/rand` 生成,最低 **128-bit entropy**,并以不暴露内部 PID 的方式编码。 + +PID 仅作为内部 process identity 的组成部分,不作为 session credential。 + +Session authorization 必须同时验证: + +```text +authenticated device ++ session ID ++ owner ++ session state ++ expiry +``` + +--- + +## 8.8 edit_block 的 TOCTOU 防护 + +`edit_block` 不能只做: + +```text +read -> verify -> rename +``` + +因为读取后到替换前存在 TOCTOU 窗口。 + +实现至少需要以下一种一致性保护: + +- 文件锁;或 +- generation / mtime / size / hash 校验后再提交;或 +- OS 支持的原子条件更新机制。 + +推荐:读取时计算 generation fingerprint,写回前重新验证;若文件已经发生变化则拒绝修改并返回 conflict,而不是覆盖第三方更新。 + +--- + +## 8.9 内存预算重新约束 + +原方案“Agent 常驻 20MB 以内”与 `2MB × N sessions` 必须统一解释。 + +新的约束是: + +> **20MB 是 Agent 基础常驻预算;Session buffer 必须从剩余预算反推最大并发数,而不是无限叠加。** + +因此实现前必须定义: + +```text +BaseMemoryBudget +SessionBufferBudget +MaxActiveSessions +PerSessionBuffer +``` + +满足: + +```text +BaseMemory + MaxSessionBufferBudget + safety margin + <= approved Agent memory ceiling +``` + +如果单 Session 仍采用 2MB,则 `MaxActiveSessions` 必须由压测和实际内存预算计算得出。 + +--- + +## 8.10 性能指标修正 + +删除或修正“整机 10W+ QPS”可能造成的误导。 + +准确表述应为: + +> **安全门禁自身的策略读取路径不以 SQLite 为热路径,单次判定成本应保持在内存级别;端到端 API 吞吐仍受网络、Agent、磁盘、进程创建和实际 IO 限制。** + +性能验收应分别测量: + +1. PolicySnapshot lookup latency; +2. authorization decision latency; +3. API end-to-end latency; +4. Agent execution throughput; +5. process/session creation rate。 + +--- + +## 8.11 进程交互传输模型 + +`interact_with_process` 的实时 stdin/stdout 通道必须在实现前冻结。 + +设计目标: + +```text +Hub / MCP + ↓ stdin +Agent Session + ↑ stdout/stderr stream +``` + +传输协议必须明确: + +- session binding; +- reconnect 行为; +- backpressure; +- output cursor / offset; +- heartbeat / timeout; +- disconnect 后 Session 是否继续运行; +- reconnect 后 owner 如何重新认证。 + +如果采用 HTTP/SSE、WebSocket 或轮询,必须在 Phase 3 实施设计中明确其安全和资源模型,不能由 handler 临时决定。 + +--- + +## 8.12 非 Session 类资源限制 + +全局资源控制不仅适用于 `start_process`。 + +### exec_command + +必须增加: + +- per-device concurrent exec limit; +- 单命令执行时间上限; +- 输出大小上限; +- 创建速率限制。 + +防止通过大量同步 `exec_command` 造成 fork/process exhaustion。 + +### list_processes + +进程命令行可能包含密码、Token、Cookie 等敏感信息。返回结果必须执行敏感信息处理,并限制返回规模。 + +### start_search / get_more_search_results + +后台搜索属于全盘 IO 风险,应具备: + +- per-device search concurrency; +- 最大搜索范围/深度; +- 最大结果数; +- 超时; +- 可取消任务; +- 全局 IO 预算。 + +--- + +## 8.13 Phase 1 红队验收 + +除原 A–E 正向验收外,Phase 1 必须增加以下对抗用例: + +1. 伪造 `{"risk":"high"}` 是否被拒绝; +2. 使用合法 Agent A 凭证请求 device B 是否被拒绝; +3. 旧 API 是否能绕过 Tool Registry; +4. Session A 的 ID 是否能访问 Session B; +5. Session A 是否能 kill 非自身进程; +6. Agent 重启后旧 Session 是否失效; +7. Policy snapshot 更新过程中是否出现半更新状态; +8. SQLite 更新失败时是否错误发布新策略; +9. 旧 Agent + 新 Hub 是否保持旧功能; +10. 新能力是否在 capability 缺失时被错误下发; +11. audit 是否泄露 password/token/cookie/private key; +12. 并发 exec 是否能够超过 per-device 上限; +13. search 是否能够无限占用 IO; +14. RingBuffer / session 上限是否能防止内存线性增长。 + +--- + +## 8.14 Phase 1 开工闸门 + +Phase 1 只有在以下条件全部满足后才能开始: + +```text +[ ] Agent identity authentication model frozen +[ ] Canonical device identity frozen +[ ] kill_process process-tree scope frozen +[ ] protocol capability/version handshake frozen +[ ] Threat model accepted +[ ] Audit redaction policy accepted +[ ] DB migration / rollback strategy accepted +[ ] Session ID generation frozen +[ ] edit_block consistency strategy frozen +[ ] Agent memory budget reconciled +[ ] Streaming transport model frozen or explicitly deferred to Phase 3 +[ ] exec/search resource limits defined +[ ] Red-team test plan accepted +``` + +**本章节是 Phase 1 的设计闸门,不是可选建议。任何阻断项未关闭时,不得以“先实现再补安全”为理由进入代码改造。** diff --git a/docs/DC_REFACTORING_AND_SECURITY_SPEC.md.bak-20261002 b/docs/DC_REFACTORING_AND_SECURITY_SPEC.md.bak-20261002 new file mode 100644 index 0000000..af60493 --- /dev/null +++ b/docs/DC_REFACTORING_AND_SECURITY_SPEC.md.bak-20261002 @@ -0,0 +1,109 @@ +# 《VPS-Commander 平滑升级改造与三级安全拦截实施方案》 + +## 1. 改造目标与背景 +本项目计划全量复刻 **Desktop Commander(DC)** 在终端与文件系统上的核心工程能力(包括局部块级精准替换、后台长进程生命周期管理、交互式 stdin 注入、结构化目录遍历与只读检索),并将其无缝接入 **Low(低风险)/ Medium(中风险)/ High(高风险)** 三级安全门禁体系。 + +**核心红线与验收标准:** +1. **服务不中断**:升级过程不影响正在运行的 Agent 探针与 Hub 中心。 +2. **协议向后兼容**:旧版 Agent 与新版 Hub 混合共存,老接口绝不废弃。 +3. **性能零损耗**:杜绝动态运行时映射,安全校验在编译期即已绑定。 +4. **轻量化保真**:继续保持纯 Go 静态编译的高效特性,严控内存占用(常驻内存锁定在 20MB 以内)。 + +--- + +## 2. 工具池全量复刻与三级风险定级表(静态强绑定) + +所有功能根据系统破坏力与副作用大小,在**编译期**严格归属于三级风险体系: + +| 风险等级 (Level) | 行为边界定义 | 包含的复刻工具 (DC 标杆) | 允许放行的安全模式 | +| :--- | :--- | :--- | :--- | +| 🟢 **Low(只读/感知)** | **纯只读、无状态改变、无系统副作用** | • `read_file` (支持切片与 Tail)
• `read_multiple_files`
• `list_directory` (结构化目录树)
• `get_file_info` (元数据)
• `start_search` / `get_more_search_results`
• `list_processes` / `list_sessions`
• `read_process_output` (拉取输出) | **Low**
**Medium**
**High** | +| 🟡 **Medium(文件写)** | **文件与目录变更,但严禁拉起任意二进制或执行命令** | • `edit_block` (局部文本块精准替换,核心)
• `write_file` (文件新建/全量写入)
• `create_directory` (新建目录)
• `move_file` (移动/重命名) | **Medium**
**High**
*(Low 拦截)* | +| 🔴 **High(完全控制)** | **具备操作系统执行权、进程生命周期调度与管道干预** | • `exec_command` (同步短命令)
• `start_process` (后台异步启动长进程)
• `interact_with_process` (stdin 交互写入)
• `kill_process` / `force_terminate` (终止进程) | **High**
*(Low / Medium 拦截)* | + +--- + +## 3. 架构改造与平滑过渡设计 + +### 3.1 协议层平滑升级(零破坏向后兼容) +在 `internal/cluster/protocol.go` 中,保留原有字段,新增可选的扩展动作: +```go +type Message struct { + ID string `json:"id,omitempty"` + Action string `json:"action,omitempty"` // 保持原有字段: "read" / "write" / "exec" / "probe" + SubAction string `json:"sub_action,omitempty"` // ✨ 增量动作: "edit_block", "start_process" 等 + Device string `json:"device,omitempty"` + Payload any `json:"payload,omitempty"` + ... +} +``` +* **兼容保障**:老版本 Agent 接收到带 `SubAction` 的 JSON 消息时会自动忽略多余字段,继续按老逻辑工作;升级后的 Agent 优先路由给 `SubAction` 处理。 + +### 3.2 Hub 门禁与高并发性能优化(读写分离) +现有系统每次请求均去 SQLite 查询策略,高并发下存在文件锁瓶颈。改造后引入 **RWMutex 内存策略快照**: +1. **读路径(Fast Path,纳秒级)**: + ```go + // 常量数值定义,直接内联比较 + const ( RiskLow = 1; RiskMedium = 2; RiskHigh = 3 ) + + func (s *Server) requireSecurity(w http.ResponseWriter, r *http.Request, device string, requiredRisk int, detail string) bool { + allowed := s.getEffectiveRiskLevel(device) // 查内存 RLock,耗时 < 10ns + if allowed >= requiredRisk { + return true + } + // 记录 audit_logs ("security_denied", 403) 并返回拦截响应 + return false + } + ``` +2. **写路径(Slow Path,低频)**: + * 仅在管理员通过 Web 界面更改安全策略时,写一次 SQLite 并原子更新内存缓存。 +3. **老 API 零改动**: + * 原有 `/api/v1/devices/exec`、`/read`、`/write` 继续保留,老脚本老客户端零感知。 + +### 3.3 Agent 端核心引擎实现(防资源膨胀设计) +* **`edit_block` 引擎**: + * 读取原文件后校验 `old_text` 全局唯一性,唯一时单次替换并使用临时文件进行原子覆写(`os.Rename`)。 + * 相比原有全量上传,网络传输和内存开销下降 95%。 +* **长进程与 PTY 会话引擎(`start_process` 等)**: + * Agent 内部建立轻量 `SessionManager`,挂接内核标准输入输出管道。 + * **硬编码内存锁(Circular RingBuffer)**:每个进程会话分配最大 2MB 环形日志缓冲区,达到上限自动覆盖最早输出,**从物理上杜绝日志死循环导致的内存溢出(OOM)**。 + +--- + +## 4. 资源开销评估与性能上限控制 + +得益于纯 Go 原生静态编译,系统坚决不引入任何 Node.js/Python 运行时,资源开销保持极致克制: + +| 评估指标 | 改造前基准(实测) | 改造后预期 | 性能/资源防护机制 | +| :--- | :--- | :--- | :--- | +| **Agent 静默内存** | ~13 MB | **~15 MB - 18 MB** | 零后台轮询,事件驱动 | +| **Agent 峰值内存** | 随命令大小波动 | **基准 + (2MB × 活跃会话数)** | 环形缓冲区硬顶锁定,防内存泄露 | +| **Hub 服务端内存** | ~16 MB | **~18 MB - 22 MB** | 内存门禁仅占不到 10KB | +| **二进制体积** | ~9.4 MB | **~11 MB - 13 MB** | 静态单文件编译,极易分发部署 | +| **安全判定延迟** | 毫秒级(查 SQLite) | **纳秒级(内存读锁)** | 单机可轻松支撑 10W+ QPS 门禁并发 | + +--- + +## 5. 四阶段实施与回滚路线图(稳健落地) + +* **Phase 1:服务端门禁升级与内存读缓存** + * 在 Hub 端落地 `RWMutex` 安全策略缓存; + * API 入口统一接入 `requireSecurity(..., RiskLevel)`; + * *验证标准*:旧版 4 项基础功能与 Web 面板完好,并发查询 QPS 大幅提升。 +* **Phase 2:落地中风险神器 `edit_block`** + * Agent 端实现块级精准替换算法; + * Hub 暴露 `/api/v1/devices/file/edit_block` 并注册至 MCP Adapter; + * *验证标准*:在 Medium 模式下可正常修配置文件,在 Low 模式下被精确阻断并记入审计。 +* **Phase 3:落地高风险长进程引擎** + * Agent 端引入带 RingBuffer 的 `SessionManager`; + * 实现 `start_process`、`read_process_output`、`interact_with_process`、`kill_process`; + * *验证标准*:跑耗时任务不会超时断连,支持实时 Tail 查看输出与注入 stdin。 +* **Phase 4:落地只读结构化文件与系统感知(Low)** + * 实现 `list_directory`、`get_file_info` 及后台异步只读搜索。 + +--- + +## 6. 评估与审计重点建议 +1. **安全维度**:Low/Medium/High 对能力的切分是否彻底杜绝了提权与逃逸风险? +2. **性能维度**:使用内存 RWMutex + 编译期静态安全级别判定,是否彻底解决了单文件 SQLite 高频读瓶颈? +3. **稳定性维度**:为每个长进程会话限定 RingBuffer 内存上限,是否有效防止了边缘 VPS 探针被日志撑爆? diff --git a/docs/DC_REFACTORING_AND_SECURITY_SPEC.md.pre-review-fix-20261002 b/docs/DC_REFACTORING_AND_SECURITY_SPEC.md.pre-review-fix-20261002 new file mode 100644 index 0000000..933e6a9 --- /dev/null +++ b/docs/DC_REFACTORING_AND_SECURITY_SPEC.md.pre-review-fix-20261002 @@ -0,0 +1,491 @@ +# 《VPS-Commander 平滑升级改造与三级安全拦截实施方案》 + +## 1. 改造目标与背景 +本项目计划全量复刻 **Desktop Commander(DC)** 在终端与文件系统上的核心工程能力(包括局部块级精准替换、后台长进程生命周期管理、交互式 stdin 注入、结构化目录遍历与只读检索),并将其无缝接入 **Low(低风险)/ Medium(中风险)/ High(高风险)** 三级安全门禁体系。 + +**核心红线与验收标准:** +1. **服务不中断**:升级过程不影响正在运行的 Agent 探针与 Hub 中心。 +2. **协议向后兼容**:旧版 Agent 与新版 Hub 混合共存,老接口绝不废弃。 +3. **性能零损耗**:杜绝动态运行时映射,安全校验在编译期即已绑定。 +4. **轻量化保真**:继续保持纯 Go 静态编译的高效特性,严控内存占用(常驻内存锁定在 20MB 以内)。 + +--- + +## 2. 工具池全量复刻与三级风险定级表(静态强绑定) + +所有功能根据系统破坏力与副作用大小,在**编译期**严格归属于三级风险体系: + +| 风险等级 (Level) | 行为边界定义 | 包含的复刻工具 (DC 标杆) | 允许放行的安全模式 | +| :--- | :--- | :--- | :--- | +| 🟢 **Low(只读/感知)** | **纯只读、无状态改变、无系统副作用** | • `read_file` (支持切片与 Tail)
• `read_multiple_files`
• `list_directory` (结构化目录树)
• `get_file_info` (元数据)
• `start_search` / `get_more_search_results`
• `list_processes` / `list_sessions`
• `read_process_output` (拉取输出) | **Low**
**Medium**
**High** | +| 🟡 **Medium(文件写)** | **文件与目录变更,但严禁拉起任意二进制或执行命令** | • `edit_block` (局部文本块精准替换,核心)
• `write_file` (文件新建/全量写入)
• `create_directory` (新建目录)
• `move_file` (移动/重命名) | **Medium**
**High**
*(Low 拦截)* | +| 🔴 **High(完全控制)** | **具备操作系统执行权、进程生命周期调度与管道干预** | • `exec_command` (同步短命令)
• `start_process` (后台异步启动长进程)
• `interact_with_process` (stdin 交互写入)
• `kill_process` / `force_terminate` (终止进程) | **High**
*(Low / Medium 拦截)* | + +--- + +## 3. 架构改造与平滑过渡设计 + +### 3.1 协议层平滑升级(零破坏向后兼容) +在 `internal/cluster/protocol.go` 中,保留原有字段,新增可选的扩展动作: +```go +type Message struct { + ID string `json:"id,omitempty"` + Action string `json:"action,omitempty"` // 保持原有字段: "read" / "write" / "exec" / "probe" + SubAction string `json:"sub_action,omitempty"` // ✨ 增量动作: "edit_block", "start_process" 等 + Device string `json:"device,omitempty"` + Payload any `json:"payload,omitempty"` + ... +} +``` +* **兼容保障**:老版本 Agent 接收到带 `SubAction` 的 JSON 消息时会自动忽略多余字段,继续按老逻辑工作;升级后的 Agent 优先路由给 `SubAction` 处理。 + +### 3.2 Hub 门禁与高并发性能优化(读写分离) +现有系统每次请求均去 SQLite 查询策略,高并发下存在文件锁瓶颈。改造后引入 **RWMutex 内存策略快照**: +1. **读路径(Fast Path,纳秒级)**: + ```go + // 常量数值定义,直接内联比较 + const ( RiskLow = 1; RiskMedium = 2; RiskHigh = 3 ) + + func (s *Server) requireSecurity(w http.ResponseWriter, r *http.Request, device string, requiredRisk int, detail string) bool { + allowed := s.getEffectiveRiskLevel(device) // 查内存 RLock,耗时 < 10ns + if allowed >= requiredRisk { + return true + } + // 记录 audit_logs ("security_denied", 403) 并返回拦截响应 + return false + } + ``` +2. **写路径(Slow Path,低频)**: + * 仅在管理员通过 Web 界面更改安全策略时,写一次 SQLite 并原子更新内存缓存。 +3. **老 API 零改动**: + * 原有 `/api/v1/devices/exec`、`/read`、`/write` 继续保留,老脚本老客户端零感知。 + +### 3.3 Agent 端核心引擎实现(防资源膨胀设计) +* **`edit_block` 引擎**: + * 读取原文件后校验 `old_text` 全局唯一性,唯一时单次替换并使用临时文件进行原子覆写(`os.Rename`)。 + * 相比原有全量上传,网络传输和内存开销下降 95%。 +* **长进程与 PTY 会话引擎(`start_process` 等)**: + * Agent 内部建立轻量 `SessionManager`,挂接内核标准输入输出管道。 + * **硬编码内存锁(Circular RingBuffer)**:每个进程会话分配最大 2MB 环形日志缓冲区,达到上限自动覆盖最早输出,**从物理上杜绝日志死循环导致的内存溢出(OOM)**。 + +--- + +## 4. 资源开销评估与性能上限控制 + +得益于纯 Go 原生静态编译,系统坚决不引入任何 Node.js/Python 运行时,资源开销保持极致克制: + +| 评估指标 | 改造前基准(实测) | 改造后预期 | 性能/资源防护机制 | +| :--- | :--- | :--- | :--- | +| **Agent 静默内存** | ~13 MB | **~15 MB - 18 MB** | 零后台轮询,事件驱动 | +| **Agent 峰值内存** | 随命令大小波动 | **基准 + (2MB × 活跃会话数)** | 环形缓冲区硬顶锁定,防内存泄露 | +| **Hub 服务端内存** | ~16 MB | **~18 MB - 22 MB** | 内存门禁仅占不到 10KB | +| **二进制体积** | ~9.4 MB | **~11 MB - 13 MB** | 静态单文件编译,极易分发部署 | +| **安全判定延迟** | 毫秒级(查 SQLite) | **纳秒级(内存读锁)** | 单机可轻松支撑 10W+ QPS 门禁并发 | + +--- + +## 5. 四阶段实施与回滚路线图(稳健落地) + +* **Phase 1:服务端门禁升级与内存读缓存** + * 在 Hub 端落地 `RWMutex` 安全策略缓存; + * API 入口统一接入 `requireSecurity(..., RiskLevel)`; + * *验证标准*:旧版 4 项基础功能与 Web 面板完好,并发查询 QPS 大幅提升。 +* **Phase 2:落地中风险神器 `edit_block`** + * Agent 端实现块级精准替换算法; + * Hub 暴露 `/api/v1/devices/file/edit_block` 并注册至 MCP Adapter; + * *验证标准*:在 Medium 模式下可正常修配置文件,在 Low 模式下被精确阻断并记入审计。 +* **Phase 3:落地高风险长进程引擎** + * Agent 端引入带 RingBuffer 的 `SessionManager`; + * 实现 `start_process`、`read_process_output`、`interact_with_process`、`kill_process`; + * *验证标准*:跑耗时任务不会超时断连,支持实时 Tail 查看输出与注入 stdin。 +* **Phase 4:落地只读结构化文件与系统感知(Low)** + * 实现 `list_directory`、`get_file_info` 及后台异步只读搜索。 + +--- + +## 6. 评估与审计重点建议 +1. **安全维度**:Low/Medium/High 对能力的切分是否彻底杜绝了提权与逃逸风险? +2. **性能维度**:使用内存 RWMutex + 编译期静态安全级别判定,是否彻底解决了单文件 SQLite 高频读瓶颈? +3. **稳定性维度**:为每个长进程会话限定 RingBuffer 内存上限,是否有效防止了边缘 VPS 探针被日志撑爆? + +--- + +# 7. 架构收敛修订(2026-10-02) + +> 本章节在进入代码实施前增加,用于两位外部 Agent 审核。原方案的工具分级、四阶段路线和向后兼容原则继续保留;本章节对安全权威、策略一致性、工具注册以及高风险 Session 生命周期作进一步约束。 + +## 7.1 安全等级的正确边界:静态工具风险 + 运行时设备策略 + +原方案中“静态强绑定”应准确解释为:**工具所需风险等级在编译期固定,是否允许执行在运行时由 Hub 的权威策略决定。** + +不得将设备当前 Low/Medium/High 策略编译进 Agent,也不得允许 Agent 自行提升自身安全等级。 + +### 7.1.1 工具风险等级 + +每个工具在代码注册表中拥有不可运行时修改的 `RequiredRisk`: + +```go +const ( + RiskLow = 1 + RiskMedium = 2 + RiskHigh = 3 +) + +type ToolSpec struct { + Name string + RequiredRisk int + ReadOnly bool +} +``` + +示例: + +```text +read_file / list_directory / search -> Low +edit_block / write_file / move_file -> Medium +exec_command / start_process / stdin -> High +``` + +禁止通过 Web 面板、Agent 配置文件或远端请求动态修改工具自身的 `RequiredRisk`。 + +### 7.1.2 设备有效策略 + +设备的 `EffectiveRisk` 是 Hub 运行时策略,不属于 Agent 自主能力。 + +请求授权模型: + +```text +Request + -> identify device + -> identify ToolSpec.RequiredRisk + -> read Hub PolicySnapshot + -> compare EffectiveRisk >= RequiredRisk + -> allow / deny +``` + +因此完整安全边界为: + +**编译期固定“工具需要什么权限” + 运行时决定“该设备当前获得什么权限”。** + +--- + +## 7.2 Policy Store 与 Policy Snapshot 一致性模型 + +SQLite 继续作为持久化权威源(durable source of truth),但请求热路径禁止每次查询 SQLite。 + +### 7.2.1 数据流 + +```text +Admin Web + | + v +SQLite transaction + | + v +Build new immutable PolicySnapshot + | + v +Atomic publish / swap + | + v +Request fast path +``` + +请求只读取当前不可变 snapshot;不在请求过程中修改策略对象。 + +### 7.2.2 更新原则 + +策略修改必须满足: + +1. SQLite 写入成功后才允许发布新 snapshot。 +2. SQLite 写失败时,内存策略保持旧版本。 +3. snapshot 发布必须是原子操作,不允许出现半更新状态。 +4. Hub 启动时必须从 SQLite 完整重建 snapshot。 +5. snapshot 应携带单调递增 `Version`,便于审计和诊断。 +6. 审计记录应包含 `device`, `oldRisk`, `newRisk`, `version`, `actor`, `timestamp`。 + +推荐结构: + +```go +type PolicySnapshot struct { + Version uint64 + Devices map[string]DevicePolicy + CreatedAt time.Time +} +``` + +热路径原则: + +```text +load current snapshot + -> lookup device + -> compare integer risk + -> allow/deny +``` + +不得在高频 API 请求中重新读取 SQLite 策略表。 + +### 7.2.3 故障安全 + +如果 Hub 无法加载有效策略 snapshot: + +- 不得默认放行 High。 +- 不得从请求参数接受临时风险等级。 +- 对需要 Medium/High 的操作默认拒绝。 +- Low 只读能力是否可继续开放,必须由启动时的明确安全默认策略决定,并记录审计事件。 + +--- + +## 7.3 Tool Registry:统一能力与风险注册表 + +避免各 API handler 自己实现不同的安全判断。所有能力必须进入统一 Tool Registry。 + +建议: + +```go +type ToolSpec struct { + Name string + RequiredRisk int + Category string + ReadOnly bool + Description string +} +``` + +例如: + +```text +read_file Low +read_multiple_files Low +list_directory Low +get_file_info Low +start_search Low +get_more_search_results Low +list_processes Low +list_sessions Low +read_process_output Low + +edit_block Medium +write_file Medium +create_directory Medium +move_file Medium + +exec_command High +start_process High +interact_with_process High +kill_process High +force_terminate High +``` + +API、MCP Adapter 和内部调用均通过 Registry 获取 `RequiredRisk`,不得在多个入口复制风险数字。 + +--- + +## 7.4 Agent 信任边界 + +Agent 是执行者,不是安全策略的最终裁决者。 + +Hub 是设备授权策略的权威来源。 + +但 Agent 仍必须执行本地最低限度安全校验,包括: + +- 请求格式与参数校验; +- session 所有权校验; +- session 是否仍然有效; +- 单次输入/输出大小限制; +- 超时和资源限制; +- 不接受客户端自行声明的 `RiskLevel` 作为授权依据。 + +Agent 不得因为请求中携带: + +```json +{"risk":"high"} +``` + +或类似字段而自行获得 High 权限。 + +如果协议未来需要携带授权信息,应使用 Hub 生成的受保护授权上下文,并绑定: + +```text +device + request/session + policy version + expiry +``` + +具体签名/令牌方案在实现阶段单独评审。 + +--- + +## 7.5 High Risk SessionManager 安全模型 + +`start_process`、`interact_with_process`、`kill_process` 不应被视为普通 HTTP 命令,而应视为一个受生命周期管理的高风险 Session。 + +### 7.5.1 Session 属性 + +每个 Session 至少包含: + +```go +type ProcessSession struct { + ID string + Owner string + Device string + PID int + CreatedAt time.Time + LastAccess time.Time + ExpiresAt time.Time + State string + OutputBytes uint64 +} +``` + +Session ID 必须不可预测,并且不能复用操作系统 PID 作为唯一身份。 + +### 7.5.2 所有权 + +只有创建 Session 的授权调用方才能: + +- 读取该 Session 输出; +- 向 stdin 写入数据; +- 终止该 Session。 + +管理员级操作如果未来需要跨 Owner 接管 Session,应定义明确的审计事件和单独权限,不得通过普通 session API 隐式获得。 + +### 7.5.3 生命周期 + +```text +start_process + | + v + RUNNING + / \ + / \ +EXITED EXPIRED + | | + +----+---+ + v + CLEANUP +``` + +Agent 重启后所有旧 Session 必须失效,不得从持久化文件自动恢复可写 stdin 的旧 Session。 + +### 7.5.4 时间限制 + +Session 必须具备: + +- 最大生命周期; +- 空闲超时; +- stdin 单次大小限制; +- stdin 速率限制; +- 输出缓冲区上限。 + +具体默认数值在实现前通过压测确定,不在设计阶段硬编码未经验证的数值。 + +--- + +## 7.6 RingBuffer 与全局资源上限 + +原方案中的“2MB × 活跃 Session”只能作为单 Session 上限,不应视为 Agent 的总内存预算。 + +必须同时存在: + +```text +per-session output cap ++ +max active sessions ++ +agent-wide session buffer cap ++ +process creation rate limit +``` + +例如: + +```text +单 Session:最大 2MB output buffer +全局:最大 N 个活跃 Session +全局:最大 M MB Session buffer +``` + +当达到任意全局上限时,新 Session 必须被拒绝并记录审计,而不是继续消耗内存。 + +RingBuffer 的覆盖行为只能覆盖旧输出,不得覆盖 session 元数据、错误状态或安全审计记录。 + +--- + +## 7.7 审计要求 + +至少记录以下安全事件: + +```text +security_denied +policy_changed +session_created +session_expired +session_killed +session_owner_denied +resource_limit_denied +``` + +High Risk 操作的审计信息必须包含: + +```text +device +actor / owner +request id +session id(如适用) +tool +result +policy version +timestamp +``` + +审计日志不能因为 Session RingBuffer 达到上限而被覆盖。 + +--- + +## 7.8 协议兼容补充约束 + +旧 Agent 与新 Hub 混合运行期间: + +1. 原有 `action=read/write/exec/probe` 字段继续有效。 +2. 新 `SubAction` 为可选字段。 +3. 新字段不能改变旧 Agent 对旧消息的既有语义。 +4. Hub 必须根据 Agent capability/version 判断是否可以下发新 SubAction。 +5. 对不支持新能力的 Agent,返回明确的 capability error,而不是让旧 Agent 猜测。 +6. 安全门禁必须在能力路由之前执行,避免通过旧 API 绕过新 Tool Registry。 + +--- + +## 7.9 实施前必须完成的设计验收 + +在进入 Phase 1 代码改造前,两位审核 Agent 应重点审查: + +### A. 权威性 + +- Hub 是否始终是 EffectiveRisk 的唯一权威来源? +- Agent 是否存在任何自升权路径? +- 旧 API 是否可能绕过 Tool Registry? + +### B. 一致性 + +- SQLite commit 与 snapshot publish 顺序是否正确? +- Hub 重启能否确定性恢复策略? +- snapshot version 是否足以定位策略变更? + +### C. Session 安全 + +- session ownership 是否不可伪造? +- Agent 重启后旧 session 是否必然失效? +- stdin、stdout、session 数量是否存在全局资源上限? + +### D. 向后兼容 + +- 老 Agent + 新 Hub 是否继续工作? +- 新 Hub 是否能识别 Agent capability? +- 新能力是否可能通过旧接口绕过门禁? + +### E. 性能 + +- 热路径是否完全脱离 SQLite? +- snapshot 是否真正 immutable / atomic? +- session ring buffer 是否存在隐藏的额外复制? + +**只有上述问题通过审核后,才进入 Phase 1 实现。** diff --git a/docs/REVIEW_A_CODE_AUDIT_20261002.md b/docs/REVIEW_A_CODE_AUDIT_20261002.md new file mode 100644 index 0000000..8bebe97 --- /dev/null +++ b/docs/REVIEW_A_CODE_AUDIT_20261002.md @@ -0,0 +1,149 @@ +# 审核专家 A:代码实现与架构安全穿透审计报告 + +- **审核人**:审核专家 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` 等) + ```go + 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.go` + ```go + writeJSON := 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.go` + ```go + if 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.go` + ```go + data, 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()` 彻底回收僵尸进程。 + +--- + +## 四、值得肯定的实现亮点 + +1. **能力握手规范已落地**:`cluster.Message` 成功扩充了 `ProtocolVersion`、`AgentVersion` 与 `Capabilities` 列表,为后续功能向后兼容奠定了坚实基础; +2. **Session ID 强随机性**:统一采用 `crypto/rand` 生成 128-bit 随机 ID,彻底杜绝了伪随机与 ID 碰撞预测; +3. **文件写入原子性良好**:`edit_block` 与 `write_file` 均规范采用了临时文件写入、`Sync()` 刷盘及 `os.Rename` 替换机制。 + +--- + +## 五、专家 A 评审总结与准入判定 + +当前工作区代码虽然跑通了单元测试(`go test ./...` PASS),但**表面的功能实现严重脱离了 Spec 的安全承诺**: +- **MCP 入口裸奔(A2)** 与 **设备自报伪造(A1)** 共同导致三级风控成为空壳; +- **Agent 并发写 WS(A4)** 与 **报错自杀式断连(A5)** 将在生产环境导致大面积崩溃与失联; +- **缺失 CAS 校验(A6)** 存在高概率的并发配置丢失风险。 + +**准入结论**:**不予准入上线。必须依照上述清单完成 A1~A6 的靶向修复并通过覆盖性验收测试后,方可推进合并。** diff --git a/docs/REVIEW_B_CODE_AUDIT_20261002.md b/docs/REVIEW_B_CODE_AUDIT_20261002.md new file mode 100644 index 0000000..f7dbf55 --- /dev/null +++ b/docs/REVIEW_B_CODE_AUDIT_20261002.md @@ -0,0 +1,106 @@ +# 审核专家 B:代码实现审计报告 + +- **审核人**:审核专家 B(外部 Agent) +- **日期**:2026-10-02 +- **对象**:`/root/.openclaw/workspace/vps-commander` 全量 Go 实现 +- **对照文档**:《VPS-Commander 平滑升级改造与三级安全拦截实施方案》第 7 章 +- **方法**:逐行审读安全、协议、执行器、API 四大核心包,非风格审查 +- **结论**:**有条件不通过 —— 5 个阻断项未修复前不得进入 Phase 1** + +--- + +## 一、阻断项(Blockers) + +### B1. MCP 接口完全绕过三级安全门禁【严重】 + +- **位置**:`internal/api/mcp_adapter.go`、`internal/mcp/mcp.go` +- **事实**:两文件**零处**引用安全策略。`MCPBackend.ExecCommand/ReadFile/WriteFile/EditBlock/ListAgentMCP/CallAgentMCP` + 直接执行,不调用 `requireSecurity`。 +- **影响**:`/mcp/sse` 是 AI Agent 实际使用的主接口——安全模型只管住了 REST, + 管 Agent 的通道完全敞开。审计过程中以普通 MCP 客户端身份调用 `exec_command` 未触发任何策略检查。 +- **判定**:阻断。修复前不得向任何不可信调用方暴露 token。 + +### B2. 单一全局 token,可自行提权【严重】 + +- **位置**:`internal/api/api.go` → `UpdateSecuritySettingsJSON`(`/api/v1/security/settings/update`) +- **事实**:仅凭全局 API token 即可修改全局及任意设备的风险等级。实现中无 admin/user 角色区分, + Spec 7.2 假设的 "Admin Web" 不存在。 +- **影响**:拿到 token 即拿到 admin,可把任意设备设为 High,三级门禁形同虚设。 +- **判定**:阻断。策略变更必须独立鉴权(独立 admin 凭证或 mTLS 管理身份)。 + +### B3. 设备身份自报,per-device 策略不可执行【严重】 + +- **位置**:`internal/api/api.go:467`(`device := r.URL.Query().Get("device")`)、 + `internal/cluster/manager.go:108`(`device_name` 取自 query) +- **事实**:设备名来自请求参数自报;Agent WS 仅以共享 cluster secret 认证,device_name 自报。 +- **影响**:任意 token 持有者可冒充任意设备,挑选策略最松的设备执行。 +- **判定**:阻断。需 per-device 凭证(mTLS 或 per-agent token)后 per-device 策略才有意义。 + +### B4. Session 所有权形同虚设【严重】 + +- **位置**:`internal/api/api.go:687` `sessionOwner(r)` +- **事实**:owner = 全局 token 的 SHA256。单 token 体系下所有调用者 owner 哈希相同, + `Session.Authorized()` 对任何已认证调用者恒为真。 +- **影响**:Spec 7.5.2 的所有权模型空转,任意调用者可读写/注入/终止他人 Session。 +- **判定**:阻断。与 B2 同源,需先解决调用者身份体系。 + +### B5. 未知设备 fail-open【严重】 + +- **位置**:`internal/security/policy.go` → `EffectiveRisk()` +- **事实**:设备不在策略表中 → 回退 `GlobalRisk`。若 GlobalRisk 为 High,拼错设备名反而获得最高权限。 +- **违反**:Spec 7.2.3 明确要求 fail-closed(不得默认放行 High)。 +- **判定**:阻断。未知设备应默认拒绝 Medium/High,或拒绝一切非常读操作。 + +--- + +## 二、高优先级(Phase 3 前修复) + +### H1. Session 清理逻辑错误 +- **位置**:`internal/executor/session_manager.go` → `Cleanup()` +- **事实**:以 `StartedAt` 计算所谓“空闲超时”(实为最大生命期);已过期但仍在运行的 session 永不清理(只删已结束的)→ 资源泄漏。 +- **违反**:Spec 7.5.4 要求的空闲超时 + 最大生命期双轨制。 + +### H2. 无 agent 全局 Session 上限与进程创建速率限制 +- **位置**:`internal/executor/session_manager.go` +- **事实**:`MaxSessionsPerManager = 16` 仅为单 manager 上限;`Start()` 存在 TOCTOU(解锁→启动进程→加锁复查),并发可短暂超限。 +- **违反**:Spec 7.6。 + +### H3. 审计日志缺失关键字段 +- **位置**:`internal/storage/storage.go:279` `Audit()` +- **事实**:无 `actor`、无 `policy_version`、无 `session_id`;命令明文入库,参数中的 secret 直接进 DB。 +- **违反**:Spec 7.7。另需敏感参数脱敏。 + +### H4. Token 允许经 URL query 传递 +- **位置**:`internal/auth/auth.go` → `Middleware.Handler` +- **事实**:接受 `?token=` / `?key=` / `?api_key=`。Token 将进入 access log、代理日志。 +- **建议**:仅允许 Authorization header。 + +### H5. Policy version 无单调性校验 +- **位置**:`internal/security/policy.go` → `Publish()` +- **事实**:接受任意版本(含回退)。 +- **违反**:Spec 7.2.2。 + +--- + +## 三、值得肯定的实现 + +- **Capability 握手已实现**:`Message` 含 `ProtocolVersion`/`AgentVersion`/`Capabilities`, + `manager.go:333` 按能力门控——上一轮 spec 评审的阻断项在代码中已解决。 +- **Session ID 使用 `crypto/rand`**(`api.go:698`),不可预测。 +- **`edit_block` 使用临时文件 + 原子 rename**(`edit.go:41-62`),含目录 fsync。 +- **REST 路径 `requireSecurity` 覆盖完整**:exec/read/write/mcp/edit_block 均接入。 + +--- + +## 四、总体评价 + +实现层面的信任模型实质是“持有 token 即上帝”:单 token 无角色分离(B2)、 +设备身份自报(B3)、MCP 主通道零门禁(B1)、Session 所有权空转(B4)。 +B1–B4 任一单独成立即足以让三级门禁失效,四者并存。 + +**在 B1(MCP 加门禁)、B2(策略变更独立鉴权)、B3(设备身份认证)修复并复审前, +不得向不可信调用方暴露 token,不得开放 `start_process` 等新能力给 Agent。** + +--- + +*审核专家 B,2026-10-02* diff --git a/internal/api/api_test.go b/internal/api/api_test.go index d3cb302..f58eb08 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -1,7 +1,10 @@ package api import ( + "fmt" + "net/http" "net/http/httptest" + "os" "path/filepath" "strings" "sync" @@ -59,8 +62,8 @@ func TestSecurityEffectiveModeAndExecGate(t *testing.T) { } defer store.DB.Close() s := &Server{Exec: executor.Local{MaxOutput: 1024 * 1024}, Store: store, LocalName: "local"} - if got := s.GetEffectiveSecurityMode("node-a"); got != "medium" { - t.Fatalf("default mode=%s", got) + if got := s.GetEffectiveSecurityMode("node-a"); got != "unknown" { + t.Fatalf("unknown mode=%s", got) } if err := store.SetNodeSecurityMode("node-a", "high"); err != nil { t.Fatal(err) @@ -88,3 +91,46 @@ func TestSecurityEffectiveModeAndExecGate(t *testing.T) { t.Fatalf("missing denial audit: %+v", logs) } } + +func TestEditBlockSecurityAndReplacement(t *testing.T) { + dir := t.TempDir() + store, err := storage.Open(filepath.Join(dir, "db.sqlite")) + if err != nil { + t.Fatal(err) + } + defer store.DB.Close() + if err := store.SetGlobalSecurityMode("medium"); err != nil { + t.Fatal(err) + } + s := &Server{Exec: executor.Local{MaxOutput: 1024 * 1024}, Store: store, LocalName: "local"} + if err := s.LoadSecurityPolicySnapshot(); err != nil { + t.Fatal(err) + } + path := filepath.Join(dir, "config.txt") + if err := os.WriteFile(path, []byte("alpha\nTARGET\nomega\n"), 0644); err != nil { + t.Fatal(err) + } + r := httptest.NewRequest(http.MethodPost, "/api/v1/devices/file/edit_block", strings.NewReader(fmt.Sprintf(`{"path":%q,"old_text":"TARGET","new_text":"CHANGED"}`, path))) + w := httptest.NewRecorder() + s.editBlock(w, r) + if w.Code != 200 { + t.Fatalf("edit status=%d body=%s", w.Code, w.Body.String()) + } + b, _ := os.ReadFile(path) + if string(b) != "alpha\nCHANGED\nomega\n" { + t.Fatalf("unexpected content: %q", b) + } + + if err := store.SetGlobalSecurityMode("low"); err != nil { + t.Fatal(err) + } + if err := s.LoadSecurityPolicySnapshot(); err != nil { + t.Fatal(err) + } + r = httptest.NewRequest(http.MethodPost, "/api/v1/devices/file/edit_block", strings.NewReader(fmt.Sprintf(`{"path":%q,"old_text":"CHANGED","new_text":"NO"}`, path))) + w = httptest.NewRecorder() + s.editBlock(w, r) + if w.Code != 403 { + t.Fatalf("low mode status=%d body=%s", w.Code, w.Body.String()) + } +} diff --git a/internal/api/mcp_security_test.go b/internal/api/mcp_security_test.go new file mode 100644 index 0000000..b7be594 --- /dev/null +++ b/internal/api/mcp_security_test.go @@ -0,0 +1,54 @@ +package api + +import ( + "context" + "path/filepath" + "testing" + + "github.com/wjyhk/vps-commander/internal/executor" + "github.com/wjyhk/vps-commander/internal/storage" +) + +func TestMCPBackendHonorsSecuritySnapshot(t *testing.T) { + store, err := storage.Open(filepath.Join(t.TempDir(), "commander.db")) + if err != nil { + t.Fatal(err) + } + defer store.DB.Close() + if err := store.SetGlobalSecurityMode("low"); err != nil { + t.Fatal(err) + } + s := &Server{Store: store, LocalName: "local", Exec: executor.Local{MaxOutput: 1024 * 1024}} + if err := s.LoadSecurityPolicySnapshot(); err != nil { + t.Fatal(err) + } + b := s.NewMCPBackend() + if _, err := b.ExecCommand(context.Background(), "test", "local", "true", "", 5); err == nil { + t.Fatal("MCP exec must be denied in low mode") + } + if _, err := b.ReadFile(context.Background(), "test", "local", "/etc/hosts", 0, 128); err != nil { + t.Fatalf("MCP read should remain available in low mode: %v", err) + } +} + +func TestUnknownDeviceFailsClosed(t *testing.T) { + store, err := storage.Open(filepath.Join(t.TempDir(), "commander.db")) + if err != nil { + t.Fatal(err) + } + defer store.DB.Close() + if err := store.SetGlobalSecurityMode("high"); err != nil { + t.Fatal(err) + } + s := &Server{Store: store, LocalName: "local", Exec: executor.Local{MaxOutput: 1024 * 1024}} + if err := s.LoadSecurityPolicySnapshot(); err != nil { + t.Fatal(err) + } + if got := s.GetEffectiveSecurityMode("not-registered"); got != "unknown" { + t.Fatalf("unknown device mode=%q", got) + } + b := s.NewMCPBackend() + if _, err := b.ExecCommand(context.Background(), "test", "not-registered", "true", "", 5); err == nil { + t.Fatal("unknown device must not inherit high risk") + } +} diff --git a/internal/api/openapi.json b/internal/api/openapi.json index 01b90ea..6618643 100644 --- a/internal/api/openapi.json +++ b/internal/api/openapi.json @@ -616,6 +616,136 @@ } } } + }, + "/api/v1/devices/file/edit_block": { + "post": { + "summary": "Atomically replace a unique text block on a device", + "security": [ + { + "bearerAuth": [] + } + ], + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "path", + "old_text", + "new_text" + ], + "properties": { + "device": { + "type": "string" + }, + "path": { + "type": "string" + }, + "old_text": { + "type": "string" + }, + "new_text": { + "type": "string" + } + } + } + } + } + }, + "responses": { + "200": { + "description": "Edited" + }, + "403": { + "description": "Security policy denied" + }, + "409": { + "description": "Device offline or edit conflict" + }, + "413": { + "description": "Edit block too large" + } + } + } + }, + "/api/v1/process/session": { + "post": { + "summary": "Manage long-running process sessions", + "security": [ + { + "bearerAuth": [] + } + ], + "parameters": [ + { + "name": "action", + "in": "query", + "required": true, + "schema": { + "type": "string", + "enum": [ + "start", + "read", + "input", + "stop", + "list" + ] + } + } + ], + "responses": { + "200": { + "description": "Session operation result" + } + } + } + }, + "/api/v1/file/list": { + "post": { + "summary": "List directory", + "security": [ + { + "bearerAuth": [] + } + ], + "responses": { + "200": { + "description": "Successful result" + } + } + } + }, + "/api/v1/file/info": { + "post": { + "summary": "Get file metadata", + "security": [ + { + "bearerAuth": [] + } + ], + "responses": { + "200": { + "description": "Successful result" + } + } + } + }, + "/api/v1/file/search": { + "post": { + "summary": "Search files", + "security": [ + { + "bearerAuth": [] + } + ], + "responses": { + "200": { + "description": "Successful result" + } + } + } } } } diff --git a/internal/auth/auth.go b/internal/auth/auth.go index d7af3e3..f68bb4a 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -42,12 +42,7 @@ func (m Middleware) Handler(next http.Handler) http.Handler { h := r.Header.Get("Authorization") if strings.HasPrefix(h, "Bearer ") { got = strings.TrimSpace(strings.TrimPrefix(h, "Bearer ")) - } else if q := r.URL.Query().Get("token"); q != "" { - got = q - } else if q := r.URL.Query().Get("key"); q != "" { - got = q - } else if q := r.URL.Query().Get("api_key"); q != "" { - got = q + } expected := m.Token diff --git a/internal/cluster/manager_test.go b/internal/cluster/manager_test.go new file mode 100644 index 0000000..757ce02 --- /dev/null +++ b/internal/cluster/manager_test.go @@ -0,0 +1,52 @@ +package cluster + +import ( + "context" + "sync" + "testing" + "time" +) + +func TestNodeOperationGateAllowsReadsAndSerializesWrites(t *testing.T) { + n := &Node{} + release1 := n.acquireOperation("file_read") + done := make(chan struct{}) + go func() { release2 := n.acquireOperation("edit_block"); close(done); release2() }() + select { + case <-done: + t.Fatal("write entered while read active") + case <-time.After(50 * time.Millisecond): + } + release1() + select { + case <-done: + case <-time.After(time.Second): + t.Fatal("write did not proceed") + } +} +func TestNodeOperationGateAllowsConcurrentReads(t *testing.T) { + n := &Node{} + var wg sync.WaitGroup + wg.Add(2) + for i := 0; i < 2; i++ { + go func() { + defer wg.Done() + release := n.acquireOperation("file_read") + time.Sleep(30 * time.Millisecond) + release() + }() + } + done := make(chan struct{}) + go func() { wg.Wait(); close(done) }() + select { + case <-done: + case <-time.After(time.Second): + t.Fatal("reads did not complete") + } +} +func TestOperationRiskUnknownFailsClosed(t *testing.T) { + if operationRisk("unknown-danger") != 3 { + t.Fatal("unknown action must be high risk") + } + _ = context.Background() +} diff --git a/internal/executor/edit.go b/internal/executor/edit.go new file mode 100644 index 0000000..ad6230a --- /dev/null +++ b/internal/executor/edit.go @@ -0,0 +1,130 @@ +package executor + +import ( + "crypto/sha256" + "encoding/hex" + "errors" + "fmt" + "os" + "path/filepath" + "sync" +) + +var ErrBlockNotUnique = errors.New("old_text must occur exactly once") +var ErrEditConflict = errors.New("file changed during edit") + +var editMu sync.Mutex + +type EditBlockResult struct { + BytesWritten int64 +} + +func EditBlock(path, oldText, newText string, expectedHash ...string) (EditBlockResult, error) { + editMu.Lock() + defer editMu.Unlock() + + if path == "" { + return EditBlockResult{}, errors.New("path required") + } + if oldText == "" { + return EditBlockResult{}, errors.New("old_text required") + } + data, err := os.ReadFile(path) + if err != nil { + return EditBlockResult{}, err + } + initial := sha256.Sum256(data) + initialHash := hex.EncodeToString(initial[:]) + if len(expectedHash) > 0 && expectedHash[0] != "" && expectedHash[0] != initialHash { + return EditBlockResult{}, ErrEditConflict + } + + src := string(data) + count := countNonOverlapping(src, oldText) + if count != 1 { + if count == 0 { + return EditBlockResult{}, fmt.Errorf("%w: found 0 matches", ErrBlockNotUnique) + } + return EditBlockResult{}, fmt.Errorf("%w: found %d matches", ErrBlockNotUnique, count) + } + updated := replaceOnce(src, oldText, newText) + + info, err := os.Stat(path) + if err != nil { + return EditBlockResult{}, err + } + + // Re-read immediately before commit. This is the CAS check against + // writers outside this process; editMu serializes our own writers. + latest, err := os.ReadFile(path) + if err != nil { + return EditBlockResult{}, err + } + latestSum := sha256.Sum256(latest) + if hex.EncodeToString(latestSum[:]) != initialHash { + return EditBlockResult{}, ErrEditConflict + } + + dir := filepath.Dir(path) + tmp, err := os.CreateTemp(dir, ".vps-commander-edit-*") + if err != nil { + return EditBlockResult{}, err + } + tmpName := tmp.Name() + defer os.Remove(tmpName) + if err := tmp.Chmod(info.Mode().Perm()); err != nil { + _ = tmp.Close() + return EditBlockResult{}, err + } + if _, err := tmp.WriteString(updated); err != nil { + _ = tmp.Close() + return EditBlockResult{}, err + } + if err := tmp.Sync(); err != nil { + _ = tmp.Close() + return EditBlockResult{}, err + } + if err := tmp.Close(); err != nil { + return EditBlockResult{}, err + } + if err := os.Rename(tmpName, path); err != nil { + return EditBlockResult{}, err + } + if d, err := os.Open(dir); err == nil { + _ = d.Sync() + _ = d.Close() + } + return EditBlockResult{BytesWritten: int64(len(updated))}, nil +} + +func countNonOverlapping(src, old string) int { + n, count := 0, 0 + for { + i := indexString(src[n:], old) + if i < 0 { + return count + } + count++ + n += i + len(old) + if n > len(src) { + return count + } + } +} + +func replaceOnce(src, old, new string) string { + i := indexString(src, old) + return src[:i] + new + src[i+len(old):] +} + +func indexString(src, needle string) int { + if len(needle) == 0 { + return 0 + } + for i := 0; i+len(needle) <= len(src); i++ { + if src[i:i+len(needle)] == needle { + return i + } + } + return -1 +} diff --git a/internal/executor/edit_test.go b/internal/executor/edit_test.go new file mode 100644 index 0000000..f03fdf3 --- /dev/null +++ b/internal/executor/edit_test.go @@ -0,0 +1,52 @@ +package executor + +import ( + "errors" + "os" + "path/filepath" + "testing" +) + +func TestEditBlockUniqueAndAtomic(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.txt") + if err := os.WriteFile(path, []byte("a\nTARGET\nz\n"), 0640); err != nil { + t.Fatal(err) + } + res, err := EditBlock(path, "TARGET", "CHANGED") + if err != nil { + t.Fatal(err) + } + if res.BytesWritten != int64(len("a\nCHANGED\nz\n")) { + t.Fatalf("bytes=%d", res.BytesWritten) + } + got, _ := os.ReadFile(path) + if string(got) != "a\nCHANGED\nz\n" { + t.Fatalf("content=%q", got) + } + info, _ := os.Stat(path) + if info.Mode().Perm() != 0640 { + t.Fatalf("mode=%o, want 0640", info.Mode().Perm()) + } +} + +func TestEditBlockRejectsZeroAndMultipleMatchesWithoutMutation(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.txt") + original := "TARGET\nother\nTARGET\n" + if err := os.WriteFile(path, []byte(original), 0644); err != nil { + t.Fatal(err) + } + _, err := EditBlock(path, "MISSING", "x") + if !errors.Is(err, ErrBlockNotUnique) { + t.Fatalf("missing match error=%v", err) + } + _, err = EditBlock(path, "TARGET", "x") + if !errors.Is(err, ErrBlockNotUnique) { + t.Fatalf("multiple match error=%v", err) + } + got, _ := os.ReadFile(path) + if string(got) != original { + t.Fatalf("file mutated after rejected edit: %q", got) + } +} diff --git a/internal/executor/fs_test.go b/internal/executor/fs_test.go new file mode 100644 index 0000000..bb1f1fd --- /dev/null +++ b/internal/executor/fs_test.go @@ -0,0 +1,36 @@ +package executor + +import ( + "context" + "os" + "path/filepath" + "testing" +) + +func TestListDirectoryAndInfo(t *testing.T) { + d := t.TempDir() + if err := os.WriteFile(filepath.Join(d, "a.txt"), []byte("hello"), 0600); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(filepath.Join(d, "sub"), 0700); err != nil { + t.Fatal(err) + } + es, err := ListDirectory(d, false, 10) + if err != nil || len(es) != 2 { + t.Fatalf("entries=%v err=%v", es, err) + } + fi, err := GetFileInfo(filepath.Join(d, "a.txt")) + if err != nil || fi.Size != 5 { + t.Fatalf("info=%+v err=%v", fi, err) + } +} +func TestSearchHonorsContextAndLimit(t *testing.T) { + d := t.TempDir() + for i := 0; i < 3; i++ { + _ = os.WriteFile(filepath.Join(d, string(rune('a'+i))+".txt"), []byte("needle\n"), 0600) + } + r, err := Search(context.Background(), d, "needle", 2) + if err != nil || len(r) != 2 { + t.Fatalf("results=%v err=%v", r, err) + } +} diff --git a/internal/executor/session.go b/internal/executor/session.go new file mode 100644 index 0000000..00c2f18 --- /dev/null +++ b/internal/executor/session.go @@ -0,0 +1,248 @@ +package executor + +import ( + "context" + "errors" + "io" + "os/exec" + "sync" + "syscall" + "time" +) + +const ( + DefaultRingCapacity = 2 * 1024 * 1024 + MaxSessionInput = 64 * 1024 + DefaultSessionTTL = 30 * time.Minute + SessionIdleTTL = 10 * time.Minute + MaxSessionBufferTotal = 8 * 1024 * 1024 +) + +type RingBuffer struct { + mu sync.RWMutex + buf []byte + head int + size int +} + +func NewRingBuffer(capacity int) *RingBuffer { + if capacity <= 0 { + capacity = DefaultRingCapacity + } + return &RingBuffer{buf: make([]byte, capacity)} +} + +func (r *RingBuffer) Write(p []byte) (int, error) { + total := len(p) + r.mu.Lock() + defer r.mu.Unlock() + for len(p) > 0 { + n := len(p) + if n > len(r.buf) { + p = p[n-len(r.buf):] + n = len(p) + } + first := len(r.buf) - r.head + if first > n { + first = n + } + copy(r.buf[r.head:r.head+first], p[:first]) + if first < n { + copy(r.buf[:n-first], p[first:n]) + } + r.head = (r.head + n) % len(r.buf) + r.size += n + if r.size > len(r.buf) { + r.size = len(r.buf) + } + p = p[n:] + } + return total, nil +} + +func (r *RingBuffer) Snapshot() []byte { + r.mu.RLock() + defer r.mu.RUnlock() + out := make([]byte, r.size) + start := (r.head - r.size + len(r.buf)) % len(r.buf) + first := len(r.buf) - start + if first > r.size { + first = r.size + } + copy(out, r.buf[start:start+first]) + if first < r.size { + copy(out[first:], r.buf[:r.size-first]) + } + return out +} + +type Session struct { + ID string + Command string + Owner string + StartedAt time.Time + ExpiresAt time.Time + LastAccess time.Time + cmd *exec.Cmd + stdin io.WriteCloser + stdout *RingBuffer + stderr *RingBuffer + done chan struct{} + mu sync.RWMutex + err error + exitCode int +} + +func StartSession(ctx context.Context, id, command, workdir, owner string, capacity int) (*Session, error) { + if id == "" || command == "" { + return nil, errors.New("session id and command required") + } + cmd := exec.Command("/bin/sh", "-c", command) + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + if workdir != "" { + cmd.Dir = workdir + } + stdout := NewRingBuffer(capacity) + stderr := NewRingBuffer(capacity) + stdin, err := cmd.StdinPipe() + if err != nil { + return nil, err + } + out, err := cmd.StdoutPipe() + if err != nil { + return nil, err + } + errOut, err := cmd.StderrPipe() + if err != nil { + return nil, err + } + + now := time.Now() + s := &Session{ + ID: id, Command: command, Owner: owner, StartedAt: now, LastAccess: now, ExpiresAt: now.Add(DefaultSessionTTL), cmd: cmd, + stdin: stdin, stdout: stdout, stderr: stderr, done: make(chan struct{}), exitCode: -1, + } + + if err := cmd.Start(); err != nil { + _ = stdin.Close() + return nil, err + } + go s.copy(out, stdout) + go s.copy(errOut, stderr) + go s.wait() + go func() { + select { + case <-ctx.Done(): + s.Terminate() + case <-s.done: + } + }() + return s, nil +} + +func (s *Session) copy(src io.Reader, dst io.Writer) { + _, _ = io.Copy(dst, src) +} + +func (s *Session) wait() { + err := s.cmd.Wait() + s.mu.Lock() + s.err = err + if err == nil { + s.exitCode = 0 + } else if ee, ok := err.(*exec.ExitError); ok { + s.exitCode = ee.ExitCode() + } else { + s.exitCode = 1 + } + s.mu.Unlock() + close(s.done) +} + +func (s *Session) Done() <-chan struct{} { return s.done } + +func (s *Session) Status() (int, bool, error) { + select { + case <-s.done: + s.mu.RLock() + defer s.mu.RUnlock() + return s.exitCode, true, s.err + default: + return -1, false, nil + } +} + +func (s *Session) Output() (stdout, stderr string) { + stdout, stderr = string(s.stdout.Snapshot()), string(s.stderr.Snapshot()) + s.mu.Lock() + if time.Now().Before(s.ExpiresAt) { + s.LastAccess = time.Now() + } + s.mu.Unlock() + return stdout, stderr +} + +func (s *Session) Stdin(data []byte) error { + if len(data) > MaxSessionInput { + return errors.New("session input too large") + } + s.mu.RLock() + stdin := s.stdin + s.mu.RUnlock() + if stdin == nil { + return errors.New("session stdin unavailable") + } + _, err := stdin.Write(data) + if err == nil { + s.mu.Lock() + s.LastAccess = time.Now() + s.mu.Unlock() + } + return err +} + +func (s *Session) Authorized(owner string) bool { + s.mu.Lock() + defer s.mu.Unlock() + now := time.Now() + if owner == "" || s.Owner != owner || now.After(s.ExpiresAt) || now.Sub(s.LastAccess) > SessionIdleTTL { + return false + } + s.LastAccess = now + return true +} +func (s *Session) Expired() bool { + s.mu.RLock() + defer s.mu.RUnlock() + now := time.Now() + return now.After(s.ExpiresAt) || now.Sub(s.LastAccess) > SessionIdleTTL +} + +func (s *Session) Terminate() error { + s.mu.RLock() + pid := 0 + if s.cmd != nil && s.cmd.Process != nil { + pid = s.cmd.Process.Pid + } + s.mu.RUnlock() + if pid == 0 { + return nil + } + return syscall.Kill(-pid, syscall.SIGTERM) +} + +func (s *Session) Kill() error { + s.mu.RLock() + pid := 0 + if s.cmd != nil && s.cmd.Process != nil { + pid = s.cmd.Process.Pid + } + s.mu.RUnlock() + if pid == 0 { + return nil + } + return syscall.Kill(-pid, syscall.SIGKILL) +} + +// Compile-time sanity for the output writer contract. +var _ io.Writer = (*RingBuffer)(nil) diff --git a/internal/executor/session_manager.go b/internal/executor/session_manager.go new file mode 100644 index 0000000..954df2d --- /dev/null +++ b/internal/executor/session_manager.go @@ -0,0 +1,104 @@ +package executor + +import ( + "context" + "errors" + "sync" + "time" +) + +const MaxSessionsPerManager = MaxSessionBufferTotal / DefaultRingCapacity + +type SessionInfo struct { + ID string `json:"id"` + Command string `json:"command"` + StartedAt time.Time `json:"started_at"` + Running bool `json:"running"` + ExitCode int `json:"exit_code"` +} + +type SessionManager struct { + mu sync.RWMutex + sessions map[string]*Session + limit int +} + +func NewSessionManager(limit int) *SessionManager { + if limit <= 0 || limit > MaxSessionsPerManager { + limit = MaxSessionsPerManager + } + return &SessionManager{sessions: make(map[string]*Session), limit: limit} +} +func (m *SessionManager) Start(ctx context.Context, id, command, workdir, owner string, capacity int) (*Session, error) { + m.mu.Lock() + if _, ok := m.sessions[id]; ok { + m.mu.Unlock() + return nil, errors.New("session already exists") + } + if len(m.sessions) >= m.limit { + m.mu.Unlock() + return nil, errors.New("session limit reached") + } + s, err := StartSession(ctx, id, command, workdir, owner, capacity) + if err != nil { + m.mu.Unlock() + return nil, err + } + if _, exists := m.sessions[id]; exists { + m.mu.Unlock() + _ = s.Kill() + return nil, errors.New("session already exists") + } + m.sessions[id] = s + m.mu.Unlock() + return s, nil +} +func (m *SessionManager) Get(id string) (*Session, bool) { + m.mu.RLock() + defer m.mu.RUnlock() + s, ok := m.sessions[id] + return s, ok +} +func (m *SessionManager) Remove(id string) bool { + m.mu.Lock() + defer m.mu.Unlock() + if _, ok := m.sessions[id]; !ok { + return false + } + delete(m.sessions, id) + return true +} +func (m *SessionManager) List() []SessionInfo { + m.mu.RLock() + defer m.mu.RUnlock() + out := make([]SessionInfo, 0, len(m.sessions)) + for _, s := range m.sessions { + code, done, _ := s.Status() + out = append(out, SessionInfo{ID: s.ID, Command: s.Command, StartedAt: s.StartedAt, Running: !done, ExitCode: code}) + } + return out +} +func (m *SessionManager) Stop(id string, force bool) error { + s, ok := m.Get(id) + if !ok { + return errors.New("session not found") + } + if force { + return s.Kill() + } + return s.Terminate() +} +func (m *SessionManager) Cleanup() { + m.mu.Lock() + defer m.mu.Unlock() + for id, s := range m.sessions { + if s.Expired() { + _, done, _ := s.Status() + if !done { + _ = s.Kill() + continue + } + delete(m.sessions, id) + } + } +} diff --git a/internal/executor/session_test.go b/internal/executor/session_test.go new file mode 100644 index 0000000..53b8d71 --- /dev/null +++ b/internal/executor/session_test.go @@ -0,0 +1,72 @@ +package executor + +import ( + "context" + "strings" + "testing" + "time" +) + +func TestRingBufferKeepsNewestData(t *testing.T) { + r := NewRingBuffer(8) + _, _ = r.Write([]byte("12345678")) + _, _ = r.Write([]byte("90")) + if got := string(r.Snapshot()); got != "34567890" { + t.Fatalf("got %q", got) + } +} +func TestSessionCapturesAndTerminates(t *testing.T) { + s, err := StartSession(context.Background(), "t1", "printf hello; sleep 5", "", "owner", 1024) + if err != nil { + t.Fatal(err) + } + time.Sleep(100 * time.Millisecond) + out, _ := s.Output() + if !strings.Contains(out, "hello") { + t.Fatalf("out=%q", out) + } + if err := s.Terminate(); err != nil { + t.Fatal(err) + } + select { + case <-s.Done(): + case <-time.After(2 * time.Second): + t.Fatal("session did not terminate") + } +} +func TestSessionManagerDuplicateAndLimit(t *testing.T) { + m := NewSessionManager(1) + s, err := m.Start(context.Background(), "a", "sleep 1", "", "owner", 128) + if err != nil { + t.Fatal(err) + } + defer s.Kill() + if _, err := m.Start(context.Background(), "b", "true", "", "owner", 128); err == nil { + t.Fatal("expected limit") + } + if _, err := m.Start(context.Background(), "a", "true", "", "owner", 128); err == nil { + t.Fatal("expected duplicate") + } +} + +func TestSessionIdleAuthorizationTouch(t *testing.T) { + s, err := StartSession(context.Background(), "idle-test", "sleep 2", "", "owner", 128) + if err != nil { + t.Fatal(err) + } + defer s.Kill() + + s.mu.Lock() + s.LastAccess = time.Now().Add(-(SessionIdleTTL + time.Second)) + s.mu.Unlock() + if s.Authorized("owner") { + t.Fatal("idle-expired session must not authorize") + } + + s.mu.Lock() + s.LastAccess = time.Now() + s.mu.Unlock() + if !s.Authorized("owner") { + t.Fatal("active session should authorize") + } +} diff --git a/internal/security/policy.go b/internal/security/policy.go new file mode 100644 index 0000000..5600d7f --- /dev/null +++ b/internal/security/policy.go @@ -0,0 +1,98 @@ +package security + +import ( + "errors" + "sync" + "time" +) + +const ( + RiskLow = 1 + RiskMedium = 2 + RiskHigh = 3 +) + +type DevicePolicy struct { + Mode string +} + +type PolicySnapshot struct { + Version uint64 + GlobalRisk int + Devices map[string]DevicePolicy + CreatedAt time.Time +} + +type Snapshot struct { + mu sync.RWMutex + data PolicySnapshot +} + +func New(globalRisk int, devices map[string]DevicePolicy, version uint64) *Snapshot { + if globalRisk < RiskLow || globalRisk > RiskHigh { + globalRisk = RiskMedium + } + cp := make(map[string]DevicePolicy, len(devices)) + for k, v := range devices { + cp[k] = v + } + return &Snapshot{data: PolicySnapshot{ + Version: version, GlobalRisk: globalRisk, Devices: cp, CreatedAt: time.Now(), + }} +} + +func (s *Snapshot) Current() PolicySnapshot { + s.mu.RLock() + defer s.mu.RUnlock() + cp := make(map[string]DevicePolicy, len(s.data.Devices)) + for k, v := range s.data.Devices { + cp[k] = v + } + snap := s.data + snap.Devices = cp + return snap +} + +func (s *Snapshot) Publish(globalRisk int, devices map[string]DevicePolicy, version uint64) error { + if globalRisk < RiskLow || globalRisk > RiskHigh { + return errors.New("invalid global risk") + } + cp := make(map[string]DevicePolicy, len(devices)) + for k, v := range devices { + cp[k] = v + } + s.mu.Lock() + if version < s.data.Version { + s.mu.Unlock() + return errors.New("policy version regression") + } + s.data = PolicySnapshot{ + Version: version, GlobalRisk: globalRisk, Devices: cp, CreatedAt: time.Now(), + } + s.mu.Unlock() + return nil +} + +func (s *Snapshot) EffectiveRisk(device string) int { + s.mu.RLock() + defer s.mu.RUnlock() + if p, ok := s.data.Devices[device]; ok { + if risk := ModeRisk(p.Mode); risk != 0 { + return risk + } + } + return s.data.GlobalRisk +} + +func ModeRisk(mode string) int { + switch mode { + case "low": + return RiskLow + case "medium": + return RiskMedium + case "high": + return RiskHigh + default: + return 0 + } +} diff --git a/internal/security/policy_test.go b/internal/security/policy_test.go new file mode 100644 index 0000000..44ed3dc --- /dev/null +++ b/internal/security/policy_test.go @@ -0,0 +1,35 @@ +package security + +import "testing" + +func TestSnapshotEffectiveRiskAndPublish(t *testing.T) { + s := New(RiskMedium, map[string]DevicePolicy{"hwsg": {Mode: "high"}}, 7) + if got := s.EffectiveRisk("hwsg"); got != RiskHigh { + t.Fatalf("hwsg risk=%d, want %d", got, RiskHigh) + } + if got := s.EffectiveRisk("n3450"); got != RiskMedium { + t.Fatalf("default risk=%d, want %d", got, RiskMedium) + } + if err := s.Publish(RiskLow, map[string]DevicePolicy{"n3450": {Mode: "medium"}}, 8); err != nil { + t.Fatal(err) + } + cur := s.Current() + if cur.Version != 8 { + t.Fatalf("version=%d, want 8", cur.Version) + } + if got := s.EffectiveRisk("hwsg"); got != RiskLow { + t.Fatalf("old node risk=%d, want %d", got, RiskLow) + } + if got := s.EffectiveRisk("n3450"); got != RiskMedium { + t.Fatalf("override risk=%d, want %d", got, RiskMedium) + } +} + +func TestModeRiskRejectsUnknown(t *testing.T) { + if got := ModeRisk("inherit"); got != 0 { + t.Fatalf("inherit risk=%d, want 0", got) + } + if got := ModeRisk("root"); got != 0 { + t.Fatalf("unknown risk=%d, want 0", got) + } +} diff --git a/internal/security/registry.go b/internal/security/registry.go new file mode 100644 index 0000000..0945baa --- /dev/null +++ b/internal/security/registry.go @@ -0,0 +1,52 @@ +package security + +type ToolSpec struct { + Name string + RequiredRisk int + Category string + ReadOnly bool +} + +var registry = map[string]ToolSpec{ + "read_file": {Name: "read_file", RequiredRisk: RiskLow, Category: "file", ReadOnly: true}, + "read_multiple_files": {Name: "read_multiple_files", RequiredRisk: RiskLow, Category: "file", ReadOnly: true}, + "list_directory": {Name: "list_directory", RequiredRisk: RiskLow, Category: "file", ReadOnly: true}, + "get_file_info": {Name: "get_file_info", RequiredRisk: RiskLow, Category: "file", ReadOnly: true}, + "start_search": {Name: "start_search", RequiredRisk: RiskLow, Category: "search", ReadOnly: true}, + "get_more_search_results": {Name: "get_more_search_results", RequiredRisk: RiskLow, Category: "search", ReadOnly: true}, + "list_processes": {Name: "list_processes", RequiredRisk: RiskLow, Category: "process", ReadOnly: true}, + "list_sessions": {Name: "list_sessions", RequiredRisk: RiskLow, Category: "process", ReadOnly: true}, + "read_process_output": {Name: "read_process_output", RequiredRisk: RiskLow, Category: "process", ReadOnly: true}, + "edit_block": {Name: "edit_block", RequiredRisk: RiskMedium, Category: "file", ReadOnly: false}, + "write_file": {Name: "write_file", RequiredRisk: RiskMedium, Category: "file", ReadOnly: false}, + "create_directory": {Name: "create_directory", RequiredRisk: RiskMedium, Category: "file", ReadOnly: false}, + "move_file": {Name: "move_file", RequiredRisk: RiskMedium, Category: "file", ReadOnly: false}, + "exec_command": {Name: "exec_command", RequiredRisk: RiskHigh, Category: "process", ReadOnly: false}, + "start_process": {Name: "start_process", RequiredRisk: RiskHigh, Category: "process", ReadOnly: false}, + "interact_with_process": {Name: "interact_with_process", RequiredRisk: RiskHigh, Category: "process", ReadOnly: false}, + "kill_process": {Name: "kill_process", RequiredRisk: RiskHigh, Category: "process", ReadOnly: false}, + "force_terminate": {Name: "force_terminate", RequiredRisk: RiskHigh, Category: "process", ReadOnly: false}, + "mcp_call": {Name: "mcp_call", RequiredRisk: RiskMedium, Category: "mcp", ReadOnly: false}, +} + +var actionTools = map[string]string{ + "read": "read_file", "probe": "read_file", "write": "write_file", + "mcp": "mcp_call", "exec": "exec_command", +} + +func Lookup(name string) (ToolSpec, bool) { + v, ok := registry[name] + return v, ok +} + +func RequiredRiskForAction(action string) (int, bool) { + name, ok := actionTools[action] + if !ok { + return 0, false + } + spec, ok := registry[name] + if !ok { + return 0, false + } + return spec.RequiredRisk, true +} diff --git a/internal/storage/storage_test.go b/internal/storage/storage_test.go index a5383c8..0f12b13 100644 --- a/internal/storage/storage_test.go +++ b/internal/storage/storage_test.go @@ -58,3 +58,38 @@ func TestAuditConcurrent100NoLoss(t *testing.T) { t.Fatalf("busy_timeout=%d, want >=5000", busy) } } + +func TestDeviceCredentialHashAndRotation(t *testing.T) { + s, err := Open(filepath.Join(t.TempDir(), "commander.db")) + if err != nil { + t.Fatal(err) + } + defer s.DB.Close() + + if err := s.SetDeviceCredential("node-1", "secret-one"); err != nil { + t.Fatal(err) + } + configured, err := s.HasDeviceCredential("node-1") + if err != nil || !configured { + t.Fatalf("credential should be configured: %v", err) + } + valid, err := s.VerifyDeviceCredential("node-1", "secret-one") + if err != nil || !valid { + t.Fatalf("valid credential rejected: %v", err) + } + valid, err = s.VerifyDeviceCredential("node-1", "wrong") + if err != nil || valid { + t.Fatal("wrong credential accepted") + } + if err := s.SetDeviceCredential("node-1", "secret-two"); err != nil { + t.Fatal(err) + } + valid, err = s.VerifyDeviceCredential("node-1", "secret-one") + if err != nil || valid { + t.Fatal("old credential accepted after rotation") + } + valid, err = s.VerifyDeviceCredential("node-1", "secret-two") + if err != nil || !valid { + t.Fatal("rotated credential rejected") + } +}