-
Notifications
You must be signed in to change notification settings - Fork 3.2k
feat(cli): allow generic file uploads including zip #4880
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 3 commits
1562cc9
5fb4c1a
7e63ca5
8b06342
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,14 +37,26 @@ pub struct BlobDescriptor { | |
| } | ||
|
|
||
| /// Build an `imeta` tag array from a BlobDescriptor (NIP-92 media metadata). | ||
| pub fn build_imeta_tag(d: &BlobDescriptor) -> Vec<String> { | ||
| /// | ||
| /// When `filename` is set (basename of the local path the user uploaded), it is | ||
| /// included as `filename <name>` so Desktop can render FileCards with a real | ||
| /// label for generic attachments. Pass `None` for media that needs no label. | ||
| pub fn build_imeta_tag_with_filename(d: &BlobDescriptor, filename: Option<&str>) -> Vec<String> { | ||
| let mut tag = vec![ | ||
| "imeta".to_string(), | ||
| format!("url {}", d.url), | ||
| format!("m {}", d.mime_type), | ||
| format!("x {}", d.sha256), | ||
| format!("size {}", d.size), | ||
| ]; | ||
| if let Some(name) = filename.map(str::trim).filter(|s| !s.is_empty()) { | ||
| // Relay rejects path-like filenames; basename only. | ||
| let base = std::path::Path::new(name) | ||
| .file_name() | ||
| .and_then(|s| s.to_str()) | ||
| .unwrap_or(name); | ||
| tag.push(format!("filename {base}")); | ||
| } | ||
| if let Some(ref dim) = d.dim { | ||
| tag.push(format!("dim {dim}")); | ||
| } | ||
|
|
@@ -60,13 +72,64 @@ pub fn build_imeta_tag(d: &BlobDescriptor) -> Vec<String> { | |
| tag | ||
| } | ||
|
|
||
| /// MIME types accepted for upload. | ||
| const ALLOWED_MIMES: &[&str] = &[ | ||
| "image/jpeg", | ||
| "image/png", | ||
| "image/gif", | ||
| "image/webp", | ||
| "video/mp4", | ||
| /// Format one uploaded attachment for message body markdown. | ||
| /// | ||
| /// Matches Desktop `formatImetaMediaLine`: | ||
| /// - `video/*` → `` | ||
| /// - `image/*` (except `*.agent.png` / `*.team.png` snapshots) → `` | ||
| /// - everything else (zip, pdf, txt, …) → plain `[filename](url)` so Desktop | ||
| /// FileCard renders a download card instead of a broken image. | ||
| pub fn format_attachment_markdown(file_path: &str, desc: &BlobDescriptor) -> String { | ||
| let mime = desc.mime_type.as_str(); | ||
| let basename = std::path::Path::new(file_path) | ||
| .file_name() | ||
| .and_then(|s| s.to_str()) | ||
| .unwrap_or("file"); | ||
| let lower = basename.to_ascii_lowercase(); | ||
| let is_snapshot_png = lower.ends_with(".agent.png") || lower.ends_with(".team.png"); | ||
|
|
||
| if mime.starts_with("video/") { | ||
| return format!("\n", desc.url); | ||
| } | ||
| if mime.starts_with("image/") && !is_snapshot_png { | ||
| return format!("\n", desc.url); | ||
| } | ||
|
|
||
| // Generic / snapshot: escape `[` `]` `\` in the label so markdown stays valid. | ||
| let escaped = basename | ||
| .replace('\\', "\\\\") | ||
| .replace('[', "\\[") | ||
| .replace(']', "\\]"); | ||
| format!("\n[{escaped}]({})", desc.url) | ||
| } | ||
|
|
||
| /// Image MIME types accepted on the image/thumbnail upload path. | ||
| const ALLOWED_IMAGE_MIMES: &[&str] = &["image/jpeg", "image/png", "image/gif", "image/webp"]; | ||
|
|
||
| /// Video MIME types accepted on the video pipeline path. | ||
| const ALLOWED_VIDEO_MIMES: &[&str] = &["video/mp4"]; | ||
|
|
||
| /// MIME types blocked on the generic file-upload path. | ||
| /// | ||
| /// Mirrors `buzz-media` `BLOCKED_FILE_MIME_TYPES`: active web content (stored | ||
| /// XSS) and native executables/installers. Everything else is allowed for | ||
| /// agent co-lab (zip skill packs, pdf, text, office docs, …) and is enforced | ||
| /// again server-side by `validate_file_content`. | ||
| const BLOCKED_FILE_MIMES: &[&str] = &[ | ||
| "text/html", | ||
| "application/xhtml+xml", | ||
| "image/svg+xml", | ||
| "application/javascript", | ||
| "text/javascript", | ||
| "application/x-msdownload", | ||
| "application/x-executable", | ||
| "application/vnd.microsoft.portable-executable", | ||
| "application/x-mach-binary", | ||
| "application/x-sharedlib", | ||
| "application/x-elf", | ||
| "application/x-msi", | ||
| "application/vnd.android.package-archive", | ||
| "application/x-apple-diskimage", | ||
| ]; | ||
|
|
||
| /// Maximum file size for image uploads (50 MB). | ||
|
|
@@ -75,6 +138,39 @@ const MAX_IMAGE_BYTES: u64 = 50 * 1024 * 1024; | |
| /// Maximum file size for video uploads (500 MB). | ||
| const MAX_VIDEO_BYTES: u64 = 500 * 1024 * 1024; | ||
|
|
||
| /// Maximum file size for generic (non-image/video) uploads (100 MB). | ||
| /// Matches relay default `BUZZ_MAX_FILE_BYTES`. | ||
| const MAX_FILE_BYTES: u64 = 100 * 1024 * 1024; | ||
|
|
||
| /// Whether a sniffed MIME may be uploaded via CLI (`buzz upload` / `--file`). | ||
| /// | ||
| /// - Images: jpeg/png/gif/webp only (image pipeline). | ||
| /// - Video: mp4 only (video pipeline). | ||
| /// - Other audio/video: rejected (no sanitizer yet — same as relay). | ||
| /// - Everything else: allowed unless on the dangerous blocklist (zip, pdf, …). | ||
| fn is_upload_mime_allowed(mime: &str) -> bool { | ||
| if mime.starts_with("image/") { | ||
| return ALLOWED_IMAGE_MIMES.contains(&mime); | ||
| } | ||
| if mime.starts_with("video/") { | ||
| return ALLOWED_VIDEO_MIMES.contains(&mime); | ||
| } | ||
| if mime.starts_with("audio/") { | ||
| return false; | ||
| } | ||
| !BLOCKED_FILE_MIMES.contains(&mime) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Allowing every non-blocklisted MIME here also enables Useful? React with 👍 / 👎. |
||
| } | ||
|
|
||
| fn max_upload_bytes_for_mime(mime: &str) -> u64 { | ||
| if mime.starts_with("video/") { | ||
| MAX_VIDEO_BYTES | ||
| } else if mime.starts_with("image/") { | ||
| MAX_IMAGE_BYTES | ||
| } else { | ||
| MAX_FILE_BYTES | ||
| } | ||
| } | ||
|
|
||
| /// Sign a NIP-98 HTTP auth event (kind:27235) and return the Authorization header value. | ||
| /// | ||
| /// The event includes: | ||
|
|
@@ -1108,21 +1204,17 @@ impl BuzzClient { | |
| let bytes = std::fs::read(file_path) | ||
| .map_err(|e| CliError::Other(format!("failed to read {file_path}: {e}")))?; | ||
|
|
||
| // 2. Detect MIME from magic bytes | ||
| // 2. Detect MIME from magic bytes (no signature → opaque download). | ||
| let mime = infer::get(&bytes) | ||
| .map(|t| t.mime_type().to_string()) | ||
| .unwrap_or_else(|| "application/octet-stream".to_string()); | ||
|
|
||
| if !ALLOWED_MIMES.contains(&mime.as_str()) { | ||
| if !is_upload_mime_allowed(&mime) { | ||
| return Err(CliError::Usage(format!("unsupported file type: {mime}"))); | ||
| } | ||
|
|
||
| // 3. Size check | ||
| let max = if mime.starts_with("video/") { | ||
| MAX_VIDEO_BYTES | ||
| } else { | ||
| MAX_IMAGE_BYTES | ||
| }; | ||
| // 3. Size check (image / video / generic file tiers). | ||
| let max = max_upload_bytes_for_mime(&mime); | ||
| if bytes.len() as u64 > max { | ||
| return Err(CliError::Usage(format!( | ||
| "file too large: {} bytes (max {})", | ||
|
|
@@ -1442,8 +1534,10 @@ mod retry_tests { | |
| use std::time::Duration; | ||
|
|
||
| use super::{ | ||
| env_duration_secs, is_moderation_kind, jitter_delay, parse_retry_hint_text, | ||
| parse_retry_in_secs, RETRY_BASE_SECS, RETRY_IN_MAX_SECS, RETRY_MAX_ATTEMPTS, | ||
| build_imeta_tag_with_filename, env_duration_secs, format_attachment_markdown, | ||
| is_moderation_kind, is_upload_mime_allowed, jitter_delay, max_upload_bytes_for_mime, | ||
| parse_retry_hint_text, parse_retry_in_secs, BlobDescriptor, MAX_FILE_BYTES, | ||
| MAX_IMAGE_BYTES, MAX_VIDEO_BYTES, RETRY_BASE_SECS, RETRY_IN_MAX_SECS, RETRY_MAX_ATTEMPTS, | ||
| }; | ||
|
|
||
| // ---- parse_retry_in_secs ---- | ||
|
|
@@ -1482,6 +1576,122 @@ mod retry_tests { | |
| assert_eq!(parse_retry_in_secs(""), None); | ||
| } | ||
|
|
||
| // ---- is_upload_mime_allowed (M1 media widen) ---- | ||
|
|
||
| #[test] | ||
| fn upload_allows_images_video_and_zip() { | ||
| for mime in [ | ||
| "image/jpeg", | ||
| "image/png", | ||
| "image/gif", | ||
| "image/webp", | ||
| "video/mp4", | ||
| "application/zip", | ||
| "application/pdf", | ||
| "text/plain", | ||
| "application/json", | ||
| "application/octet-stream", | ||
| ] { | ||
| assert!(is_upload_mime_allowed(mime), "expected allowed: {mime}"); | ||
| } | ||
| } | ||
|
|
||
| #[test] | ||
| fn upload_blocks_active_and_executable_types() { | ||
| for mime in [ | ||
| "text/html", | ||
| "application/javascript", | ||
| "image/svg+xml", | ||
| "application/x-msdownload", | ||
| "application/x-executable", | ||
| "application/vnd.android.package-archive", | ||
| "audio/mpeg", | ||
| "video/webm", | ||
| "image/bmp", | ||
| ] { | ||
| assert!(!is_upload_mime_allowed(mime), "expected blocked: {mime}"); | ||
| } | ||
| } | ||
|
|
||
| #[test] | ||
| fn upload_size_tiers() { | ||
| assert_eq!(max_upload_bytes_for_mime("image/png"), MAX_IMAGE_BYTES); | ||
| assert_eq!(max_upload_bytes_for_mime("video/mp4"), MAX_VIDEO_BYTES); | ||
| assert_eq!(max_upload_bytes_for_mime("application/zip"), MAX_FILE_BYTES); | ||
| } | ||
|
|
||
| fn test_desc(mime: &str, url: &str) -> BlobDescriptor { | ||
| BlobDescriptor { | ||
| url: url.to_string(), | ||
| sha256: "aa".repeat(32), | ||
| size: 1, | ||
| mime_type: mime.to_string(), | ||
| uploaded: 0, | ||
| dim: None, | ||
| blurhash: None, | ||
| thumb: None, | ||
| duration: None, | ||
| } | ||
| } | ||
|
|
||
| #[test] | ||
| fn attachment_markdown_image_and_video_inline() { | ||
| let img = test_desc("image/png", "https://relay.example/media/a.png"); | ||
| assert_eq!( | ||
| format_attachment_markdown("/tmp/shot.png", &img), | ||
| "\n" | ||
| ); | ||
| let vid = test_desc("video/mp4", "https://relay.example/media/a.mp4"); | ||
| assert_eq!( | ||
| format_attachment_markdown("/tmp/clip.mp4", &vid), | ||
| "\n" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn attachment_markdown_generic_file_is_plain_link() { | ||
| let zip = test_desc("application/zip", "https://relay.example/media/pack.zip"); | ||
| assert_eq!( | ||
| format_attachment_markdown("/tmp/gcr-skill-pack.zip", &zip), | ||
| "\n[gcr-skill-pack.zip](https://relay.example/media/pack.zip)" | ||
| ); | ||
| let pdf = test_desc("application/pdf", "https://relay.example/media/doc.pdf"); | ||
| assert_eq!( | ||
| format_attachment_markdown("/home/u/notes.pdf", &pdf), | ||
| "\n[notes.pdf](https://relay.example/media/doc.pdf)" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn attachment_markdown_escapes_label_metacharacters() { | ||
| let d = test_desc("application/pdf", "https://relay.example/media/x.pdf"); | ||
| assert_eq!( | ||
| format_attachment_markdown("/tmp/a].pdf", &d), | ||
| "\n[a\\].pdf](https://relay.example/media/x.pdf)" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn attachment_markdown_snapshot_png_uses_file_link() { | ||
| let d = test_desc("image/png", "https://relay.example/media/snap.png"); | ||
| assert_eq!( | ||
| format_attachment_markdown("/tmp/bot.agent.png", &d), | ||
| "\n[bot.agent.png](https://relay.example/media/snap.png)" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn imeta_tag_includes_optional_filename() { | ||
| let d = test_desc("application/zip", "https://relay.example/media/p.zip"); | ||
| let with = build_imeta_tag_with_filename(&d, Some("pack.zip")); | ||
| assert!(with.iter().any(|f| f == "filename pack.zip"), "{with:?}"); | ||
| let without = build_imeta_tag_with_filename(&d, None); | ||
| assert!( | ||
| !without.iter().any(|f| f.starts_with("filename ")), | ||
| "{without:?}" | ||
| ); | ||
| } | ||
|
|
||
| // ---- parse_retry_hint_text ---- | ||
|
|
||
| #[test] | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blocking: This does not actually guarantee a relay-valid filename. On Unix,
Path::file_name()leavesbad\\name.zipunchanged; control characters are also retained, and names longer than 255 are unbounded. The relay rejects all three (crates/buzz-relay/src/handlers/imeta.rs:140-155), while Desktop explicitly strips both separator styles, controls, and caps the label (desktop/src-tauri/src/commands/media.rs:138-153). Becausecmd_send_messagenow attachesfilenameto every upload, even an otherwise valid image with such a local name uploads first and then fails message ingest—a regression from the old filename-free tag. Please centralize a sanitizer matching the relay contract and use its result for both this field and the markdown label. Add cases for backslashes, controls, and oversized/multibyte names.