From 08dbf4b661b4d4cd144fc2f75bd27a042f68752a Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 13 Jun 2026 12:19:57 +0000 Subject: [PATCH] fix(security): block Hermes FS traversal and revoked token reuse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Reject ParentDir (..) in Hermes file manager paths and validate via canonical ancestors instead of non-canonical starts_with prefix checks - Rotate operator tokens when revokedAtMs is set instead of silently re-issuing the same revoked token during auto-pair normalization Fixes path traversal allowing reads/writes outside ~/.hermes and revocation bypass introduced in the v0.18.1 pairing upgrade path. Co-authored-by: 晴天 <1186258278@users.noreply.github.com> --- src-tauri/src/commands/hermes.rs | 123 ++++++++++++++++++++++++------ src-tauri/src/commands/pairing.rs | 68 ++++++++++++++--- 2 files changed, 156 insertions(+), 35 deletions(-) diff --git a/src-tauri/src/commands/hermes.rs b/src-tauri/src/commands/hermes.rs index b59c0a9..2909960 100644 --- a/src-tauri/src/commands/hermes.rs +++ b/src-tauri/src/commands/hermes.rs @@ -17261,34 +17261,64 @@ const FS_MAX_LIST_ENTRIES: usize = 2000; // 单次最多返回 2000 条 /// 验证路径在 hermes_home 子树内（防 path traversal）。 /// 返回安全的绝对路径，或 Err。 fn validate_hermes_fs_path(rel_path: &str) -> Result { - let root = hermes_home(); - // 空 = 根目录 - let target = if rel_path.is_empty() { - root.clone() - } else { - // 拒绝绝对路径输入（必须相对于 hermes_home） - let p = std::path::Path::new(rel_path); - if p.is_absolute() { - // 允许绝对路径，但必须以 root 开头（用 starts_with 检查） - let canonical_root = root.canonicalize().unwrap_or(root.clone()); - let canonical_target = p.canonicalize().unwrap_or_else(|_| p.to_path_buf()); - if !canonical_target.starts_with(&canonical_root) { - return Err(format!("路径必须在 {} 子树内", root.to_string_lossy())); - } - canonical_target - } else { - // 相对路径：拼到 root 下，再 canonicalize 防 .. - let joined = root.join(p); - // 父目录必须存在才能 canonicalize；对不存在的新文件 fallback 到 joined - let canon = joined.canonicalize().unwrap_or(joined.clone()); - let canonical_root = root.canonicalize().unwrap_or(root.clone()); + validate_hermes_fs_path_under(&hermes_home(), rel_path) +} + +fn validate_hermes_fs_path_under(root: &std::path::Path, rel_path: &str) -> Result { + let canonical_root = root + .canonicalize() + .map_err(|e| format!("Hermes 目录不存在: {e}"))?; + + if rel_path.is_empty() { + return Ok(canonical_root); + } + + let p = std::path::Path::new(rel_path); + if p.components() + .any(|c| matches!(c, std::path::Component::ParentDir)) + { + return Err("路径不能包含 ..".into()); + } + + if p.is_absolute() { + let canonical_target = p + .canonicalize() + .map_err(|e| format!("路径无效: {e}"))?; + if !canonical_target.starts_with(&canonical_root) { + return Err(format!("路径必须在 {} 子树内", root.to_string_lossy())); + } + return Ok(canonical_target); + } + + let joined = canonical_root.join(p); + if joined.exists() { + let canon = joined + .canonicalize() + .map_err(|e| format!("路径无效: {e}"))?; + if !canon.starts_with(&canonical_root) { + return Err(format!("路径不能跳出 {} 目录", root.to_string_lossy())); + } + return Ok(canon); + } + + // 新文件：沿已存在祖先目录 canonicalize，避免非规范化路径绕过 starts_with。 + let mut ancestor = joined.clone(); + while ancestor.pop() { + if ancestor.starts_with(&canonical_root) && ancestor.exists() { + let canon = ancestor + .canonicalize() + .map_err(|e| format!("路径无效: {e}"))?; if !canon.starts_with(&canonical_root) { return Err(format!("路径不能跳出 {} 目录", root.to_string_lossy())); } - canon + return Ok(joined); } - }; - Ok(target) + } + + if !joined.starts_with(&canonical_root) { + return Err(format!("路径不能跳出 {} 目录", root.to_string_lossy())); + } + Ok(joined) } #[tauri::command] @@ -25362,3 +25392,48 @@ platforms: assert!(err.contains("display.platforms.telegram.streaming")); } } + +#[cfg(test)] +mod hermes_fs_path_tests { + use super::validate_hermes_fs_path_under; + + #[test] + fn rejects_parent_dir_segments() { + let root = std::env::temp_dir().join(format!( + "hermes-fs-path-{}", + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + std::fs::create_dir_all(&root).unwrap(); + let outside = root.parent().unwrap().join("outside-secret.txt"); + std::fs::write(&outside, "SECRET").unwrap(); + + let err = validate_hermes_fs_path_under(&root, "../outside-secret.txt") + .expect_err("path traversal should be rejected"); + assert!(err.contains("..")); + + let _ = std::fs::remove_file(outside); + let _ = std::fs::remove_dir_all(root); + } + + #[test] + fn allows_new_file_under_root() { + let root = std::env::temp_dir().join(format!( + "hermes-fs-path-new-{}", + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + std::fs::create_dir_all(&root).unwrap(); + + let resolved = + validate_hermes_fs_path_under(&root, "notes/new.txt").expect("valid nested path"); + assert!(resolved.starts_with(root.canonicalize().unwrap())); + assert_eq!(resolved.file_name().and_then(|n| n.to_str()), Some("new.txt")); + + let _ = std::fs::remove_dir_all(root); + } +} diff --git a/src-tauri/src/commands/pairing.rs b/src-tauri/src/commands/pairing.rs index 23d82b0..ac3f21b 100644 --- a/src-tauri/src/commands/pairing.rs +++ b/src-tauri/src/commands/pairing.rs @@ -76,15 +76,19 @@ fn ensure_array_contains( changed } +fn operator_token_is_revoked(value: Option<&serde_json::Value>) -> bool { + value + .and_then(|v| v.as_object()) + .and_then(|obj| obj.get("revokedAtMs")) + .map(|v| !v.is_null()) + .unwrap_or(false) +} + fn operator_token_is_usable(value: Option<&serde_json::Value>) -> bool { let Some(obj) = value.and_then(|v| v.as_object()) else { return false; }; - if obj - .get("revokedAtMs") - .map(|v| !v.is_null()) - .unwrap_or(false) - { + if operator_token_is_revoked(value) { return false; } if obj.get("role").and_then(|v| v.as_str()) != Some(CONTROL_UI_ROLE) { @@ -129,12 +133,17 @@ fn ensure_operator_token( } let existing = tokens_obj.get(CONTROL_UI_ROLE).and_then(|v| v.as_object()); - let token = existing - .and_then(|entry| entry.get("token")) - .and_then(|v| v.as_str()) - .filter(|token| !token.trim().is_empty()) - .map(|token| token.to_string()) - .unwrap_or_else(generate_pairing_token); + let revoked = operator_token_is_revoked(tokens_obj.get(CONTROL_UI_ROLE)); + let token = if revoked { + generate_pairing_token() + } else { + existing + .and_then(|entry| entry.get("token")) + .and_then(|v| v.as_str()) + .filter(|token| !token.trim().is_empty()) + .map(|token| token.to_string()) + .unwrap_or_else(generate_pairing_token) + }; let created_at_ms = existing .and_then(|entry| entry.get("createdAtMs")) .and_then(|v| v.as_u64()) @@ -511,4 +520,41 @@ mod tests { assert!(!changed); assert_eq!(entry["approvedAtMs"], serde_json::json!(1)); } + + #[test] + fn normalize_existing_pairing_rotates_revoked_operator_token() { + let mut entry = serde_json::json!({ + "deviceId": "device-1", + "publicKey": "key", + "platform": "windows", + "deviceFamily": "desktop", + "clientId": "openclaw-control-ui", + "clientMode": "ui", + "role": "operator", + "roles": ["operator"], + "scopes": scope_values(), + "approvedScopes": scope_values(), + "tokens": { + "operator": { + "token": "revoked-token", + "role": "operator", + "scopes": scope_values(), + "createdAtMs": 1, + "revokedAtMs": 999 + } + }, + "createdAtMs": 1, + "approvedAtMs": 1 + }); + + let changed = normalize_control_ui_pairing(&mut entry, "device-1", "key", "windows", 1234); + + assert!(changed); + let token = entry["tokens"]["operator"]["token"] + .as_str() + .expect("token"); + assert_ne!(token, "revoked-token"); + assert!(entry["tokens"]["operator"].get("revokedAtMs").is_none()); + assert_eq!(entry["tokens"]["operator"]["rotatedAtMs"], serde_json::json!(1234)); + } }