diff --git a/hive-matrix-mcp/src/handlers.rs b/hive-matrix-mcp/src/handlers.rs index 2f789ba4..37277fa3 100644 --- a/hive-matrix-mcp/src/handlers.rs +++ b/hive-matrix-mcp/src/handlers.rs @@ -1128,6 +1128,17 @@ fn media_parts( Some((source.clone(), name.to_owned(), caption.map(str::to_owned))) } +/// The last path segment of a sender-supplied attachment name, so it cannot +/// point outside the temp dir; `matrix-attachment` when nothing usable is +/// left. +fn safe_file_name(name: &str) -> &str { + std::path::Path::new(name) + .file_name() + .and_then(|n| n.to_str()) + .filter(|n| !n.is_empty()) + .unwrap_or("matrix-attachment") +} + /// Download the media attachment carried by `event_id` in `room` and /// write it to a local file, returning the path. Resolves the event, /// extracts the `MediaSource` from its `m.file` / `m.image` / `m.audio` @@ -1180,15 +1191,11 @@ pub async fn download_file( Ok(b) => b, Err(e) => return DaemonResponse::error(format!("download media for {eid}: {e}")), }; + let filename = safe_file_name(&filename); let dest = if let Some(p) = dest_path { std::path::PathBuf::from(p) } else { - let name = std::path::Path::new(&filename) - .file_name() - .and_then(|n| n.to_str()) - .filter(|n| !n.is_empty()) - .unwrap_or("matrix-attachment"); - std::env::temp_dir().join(name) + std::env::temp_dir().join(filename) }; if let Err(e) = tokio::fs::write(&dest, &bytes).await { return DaemonResponse::error(format!("write {}: {e}", dest.display())); @@ -1315,7 +1322,8 @@ mod utd_body_tests { mod unread_tests { use super::{ ParsedEvent, TimelineEvent, attachment_marker, collapse_edits, extract_body_sync, - extract_in_reply_to, is_intentional, media_parts, replacement_of, unread_verdict, + extract_in_reply_to, is_intentional, media_parts, replacement_of, safe_file_name, + unread_verdict, }; use matrix_sdk::ruma::events::AnySyncTimelineEvent; use matrix_sdk::ruma::events::room::message::MessageType; @@ -1705,6 +1713,15 @@ mod unread_tests { assert!(media_parts(&msgtype(&json!({"msgtype": "m.text", "body": "x"}))).is_none()); } + #[test] + fn an_attachment_name_is_reduced_to_its_last_segment() { + assert_eq!(safe_file_name("a.png"), "a.png"); + assert_eq!(safe_file_name("../../etc/x.png"), "x.png"); + assert_eq!(safe_file_name("dir/"), "dir"); + assert_eq!(safe_file_name(""), "matrix-attachment"); + assert_eq!(safe_file_name(".."), "matrix-attachment"); + } + #[test] fn attachments_render_as_typed_markers_and_text_as_itself() { let marker = |kind: &str, extra: serde_json::Value| {