From 58f461c9533f128c259b3139522164588b7143ff Mon Sep 17 00:00:00 2001 From: oudecheng <13802883547@139.com> Date: Wed, 8 Jul 2026 16:23:09 +0800 Subject: [PATCH] =?UTF-8?q?fix:=20=E4=BF=AE=E5=A4=8D=20SSRF=20=E9=87=8D?= =?UTF-8?q?=E5=AE=9A=E5=90=91=E7=BB=95=E8=BF=87=20+=20=E7=AC=A6=E5=8F=B7?= =?UTF-8?q?=E9=93=BE=E6=8E=A5=E8=B7=AF=E5=BE=84=E9=81=8D=E5=8E=86=20+=20To?= =?UTF-8?q?doItemSummary=20=E5=AD=97=E6=AE=B5=E7=BC=BA=E5=A4=B1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P0: src/tools/http_request.rs SSRF 重定向绕过 - reqwest::Client 默认跟随最多 10 次重定向，is_private_host 仅检查初始 URL - 攻击者可用公网 URL 返回 302 → http://127.0.0.1/ 或 http://169.254.169.254/（云元数据端点）绕过防护访问内网 - 修复：.redirect(reqwest::redirect::Policy::none()) 完全禁用重定向 P1: src/tools/file_read/write/edit.rs 符号链接路径遍历 - resolve_path 用 starts_with 检查但未 canonicalize - 攻击者可在 allowed_dir 内创建指向 /etc/passwd 的符号链接绕过限制 - 修复：对 resolved 和 allowed 均执行 canonicalize 后比较 - file_read: 文件必须存在，canonicalize 失败直接报错 - file_write/edit: 文件可能不存在，降级到父目录 canonicalize P1: src/protocol/mod.rs + list_todos.rs TodoItemSummary 字段缺失 - 后端 TodoItemSummary 仅返回 4 字段，前端期望 7 字段 - 缺失 priority, created_at, updated_at，前端 TodoPanel 无法显示 优先级和时间戳 - 修复：struct 补齐 3 字段，list_todos 构造时传递完整字段 --- src/command/handlers/list_todos.rs | 3 +++ src/protocol/mod.rs | 3 +++ src/tools/file_edit.rs | 28 ++++++++++++++++++++++++---- src/tools/file_read.rs | 15 +++++++++++---- src/tools/file_write.rs | 28 ++++++++++++++++++++++++---- src/tools/http_request.rs | 1 + 6 files changed, 66 insertions(+), 12 deletions(-) diff --git a/src/command/handlers/list_todos.rs b/src/command/handlers/list_todos.rs index 07e3dda..3b837ca 100644 --- a/src/command/handlers/list_todos.rs +++ b/src/command/handlers/list_todos.rs @@ -76,6 +76,9 @@ impl CommandHandler for ListTodosCommandHandler { id: r.id, content: r.content, status: r.status, + priority: r.priority, + created_at: r.created_at, + updated_at: r.updated_at, created_by_message_id: r.created_by_message_id, }) .collect(); diff --git a/src/protocol/mod.rs b/src/protocol/mod.rs index c6073c5..a15c496 100644 --- a/src/protocol/mod.rs +++ b/src/protocol/mod.rs @@ -88,6 +88,9 @@ pub struct TodoItemSummary { pub id: String, pub content: String, pub status: String, + pub priority: String, + pub created_at: i64, + pub updated_at: i64, pub created_by_message_id: Option, } diff --git a/src/tools/file_edit.rs b/src/tools/file_edit.rs index b9f2e52..c751235 100644 --- a/src/tools/file_edit.rs +++ b/src/tools/file_edit.rs @@ -33,11 +33,31 @@ impl FileEditTool { // Check directory restriction if let Some(ref allowed) = self.allowed_dir { - let allowed_path = Path::new(allowed); - if !resolved.starts_with(allowed_path) { + // canonicalize both paths to resolve symlinks and prevent path traversal + // via symlinks inside allowed_dir pointing outside. + // For edit tool the target file may not exist yet; fall back to + // canonicalizing the parent directory. + let canonical_allowed = std::fs::canonicalize(allowed) + .map_err(|e| format!("Failed to canonicalize allowed dir '{}': {}", allowed, e))?; + let canonical_resolved = match std::fs::canonicalize(&resolved) { + Ok(c) => c, + Err(_) => { + // File doesn't exist yet; canonicalize parent directory + let parent = resolved.parent().ok_or_else(|| { + format!("Path '{}' has no parent directory", path) + })?; + let canonical_parent = std::fs::canonicalize(parent).map_err(|e| { + format!("Failed to canonicalize parent directory of '{}': {}", path, e) + })?; + canonical_parent.join(resolved.file_name().ok_or_else(|| { + format!("Path '{}' has no file name component", path) + })?) + } + }; + if !canonical_resolved.starts_with(&canonical_allowed) { return Err(format!( - "Path '{}' is outside allowed directory '{}'", - path, allowed + "Path '{}' (resolves to '{}') is outside allowed directory '{}'", + path, canonical_resolved.display(), canonical_allowed.display() )); } } diff --git a/src/tools/file_read.rs b/src/tools/file_read.rs index 6a8a8a8..5ba4074 100644 --- a/src/tools/file_read.rs +++ b/src/tools/file_read.rs @@ -37,11 +37,18 @@ impl FileReadTool { // Check directory restriction if let Some(ref allowed) = self.allowed_dir { - let allowed_path = Path::new(allowed); - if !resolved.starts_with(allowed_path) { + // canonicalize both paths to resolve symlinks and prevent path traversal + // via symlinks inside allowed_dir pointing outside + let canonical_allowed = std::fs::canonicalize(allowed) + .map_err(|e| format!("Failed to canonicalize allowed dir '{}': {}", allowed, e))?; + // For read tool, file must exist; canonicalize will fail for non-existent paths + // which is acceptable (returns error) + let canonical_resolved = std::fs::canonicalize(&resolved) + .map_err(|e| format!("Failed to canonicalize path '{}': {}", path, e))?; + if !canonical_resolved.starts_with(&canonical_allowed) { return Err(format!( - "Path '{}' is outside allowed directory '{}'", - path, allowed + "Path '{}' (resolves to '{}') is outside allowed directory '{}'", + path, canonical_resolved.display(), canonical_allowed.display() )); } } diff --git a/src/tools/file_write.rs b/src/tools/file_write.rs index 15ca360..ef07ea5 100644 --- a/src/tools/file_write.rs +++ b/src/tools/file_write.rs @@ -32,11 +32,31 @@ impl FileWriteTool { // Check directory restriction if let Some(ref allowed) = self.allowed_dir { - let allowed_path = Path::new(allowed); - if !resolved.starts_with(allowed_path) { + // canonicalize both paths to resolve symlinks and prevent path traversal + // via symlinks inside allowed_dir pointing outside. + // For write tool the target file may not exist yet; fall back to + // canonicalizing the parent directory. + let canonical_allowed = std::fs::canonicalize(allowed) + .map_err(|e| format!("Failed to canonicalize allowed dir '{}': {}", allowed, e))?; + let canonical_resolved = match std::fs::canonicalize(&resolved) { + Ok(c) => c, + Err(_) => { + // File doesn't exist yet; canonicalize parent directory + let parent = resolved.parent().ok_or_else(|| { + format!("Path '{}' has no parent directory", path) + })?; + let canonical_parent = std::fs::canonicalize(parent).map_err(|e| { + format!("Failed to canonicalize parent directory of '{}': {}", path, e) + })?; + canonical_parent.join(resolved.file_name().ok_or_else(|| { + format!("Path '{}' has no file name component", path) + })?) + } + }; + if !canonical_resolved.starts_with(&canonical_allowed) { return Err(format!( - "Path '{}' is outside allowed directory '{}'", - path, allowed + "Path '{}' (resolves to '{}') is outside allowed directory '{}'", + path, canonical_resolved.display(), canonical_allowed.display() )); } } diff --git a/src/tools/http_request.rs b/src/tools/http_request.rs index be6f9ce..f484b8f 100644 --- a/src/tools/http_request.rs +++ b/src/tools/http_request.rs @@ -311,6 +311,7 @@ impl Tool for HttpRequestTool { let client = match reqwest::Client::builder() .timeout(Duration::from_secs(self.timeout_secs)) + .redirect(reqwest::redirect::Policy::none()) .build() { Ok(c) => c,