mirror of
https://github.com/qingchencloud/clawpanel.git
synced 2026-06-23 16:44:23 +08:00
fix(security): block Hermes FS traversal and revoked token reuse
- 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>
This commit is contained in:
@@ -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<PathBuf, String> {
|
||||
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<PathBuf, String> {
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user