diff --git a/crates/buzz-cli/src/commands/messages.rs b/crates/buzz-cli/src/commands/messages.rs index ea273336e38..2b41020078a 100644 --- a/crates/buzz-cli/src/commands/messages.rs +++ b/crates/buzz-cli/src/commands/messages.rs @@ -5,8 +5,8 @@ use uuid::Uuid; use crate::client::{normalize_events, normalize_write_response, BuzzClient}; use crate::error::CliError; use crate::validate::{ - infer_language, parse_event_id, parse_uuid, read_or_stdin, truncate_diff, - validate_content_size, validate_hex64, validate_uuid, MAX_DIFF_BYTES, + infer_language, parse_event_id, parse_uuid, read_message_or_stdin, read_or_stdin, + truncate_diff, validate_content_size, validate_hex64, validate_uuid, MAX_DIFF_BYTES, }; use buzz_sdk::mentions::{ extract_at_mentions_with_known, extract_nostr_uris, strip_code_regions, MENTION_CAP, @@ -616,8 +616,13 @@ pub async fn cmd_send_message( // jam shell-metacharacter-heavy text (backticks, $vars, etc.) through argv // quoting — the source of countless self-inflicted command-substitution // bugs for agent and human users alike. - p.content = read_or_stdin(&p.content)?; + p.content = read_message_or_stdin(&p.content)?; validate_content_size(&p.content)?; + if p.content.trim().is_empty() && p.files.is_empty() { + return Err(CliError::Usage( + "message must have content or attachments".into(), + )); + } if let Some(ref r) = p.reply_to { validate_hex64(r)?; } diff --git a/crates/buzz-cli/src/validate.rs b/crates/buzz-cli/src/validate.rs index 4985b441417..16bb85461b4 100644 --- a/crates/buzz-cli/src/validate.rs +++ b/crates/buzz-cli/src/validate.rs @@ -178,6 +178,76 @@ pub fn read_or_stdin(value: &str) -> Result { } } +/// Read message content, bounding how long non-interactive stdin may remain +/// completely idle so an inherited open pipe cannot block an agent forever. +/// +/// A terminal remains interactive and may wait for a human to finish input. +/// Once a pipe produces its first byte, the complete payload may take as long +/// as its producer needs. +pub fn read_message_or_stdin(value: &str) -> Result { + if value != "-" { + return Ok(value.to_string()); + } + + use std::io::{IsTerminal, Read}; + + if std::io::stdin().is_terminal() { + let mut buf = String::new(); + std::io::stdin() + .read_to_string(&mut buf) + .map_err(|e| CliError::Other(format!("failed to read stdin: {e}")))?; + return Ok(buf); + } + + const FIRST_BYTE_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(5); + let (ready_tx, ready_rx) = std::sync::mpsc::sync_channel(1); + let (result_tx, result_rx) = std::sync::mpsc::sync_channel(1); + std::thread::spawn(move || { + let mut stdin = std::io::stdin(); + let mut first = [0_u8; 1]; + match stdin.read(&mut first) { + Ok(0) => { + let _ = ready_tx.send(Ok(())); + let _ = result_tx.send(Ok(String::new())); + } + Ok(_) => { + if ready_tx.send(Ok(())).is_err() { + return; + } + let mut bytes = vec![first[0]]; + let result = stdin + .read_to_end(&mut bytes) + .map_err(|e| CliError::Other(format!("failed to read stdin: {e}"))) + .and_then(|_| { + String::from_utf8(bytes).map_err(|e| { + CliError::Other(format!("failed to read stdin as UTF-8: {e}")) + }) + }); + let _ = result_tx.send(result); + } + Err(error) => { + let _ = ready_tx.send(Err(CliError::Other(format!( + "failed to read stdin: {error}" + )))); + } + } + }); + + match ready_rx.recv_timeout(FIRST_BYTE_TIMEOUT) { + Ok(Ok(())) => result_rx.recv().map_err(|_| { + CliError::Other("failed to read stdin: reader stopped unexpectedly".into()) + })?, + Ok(Err(error)) => Err(error), + Err(std::sync::mpsc::RecvTimeoutError::Timeout) => Err(CliError::Usage( + "stdin produced no data within 5 seconds; pipe content explicitly or redirect a file" + .into(), + )), + Err(std::sync::mpsc::RecvTimeoutError::Disconnected) => Err(CliError::Other( + "failed to read stdin: reader stopped unexpectedly".into(), + )), + } +} + /// Read content from a file path, or stdin if the value is "-". /// /// Unlike [`read_or_stdin`], `value` is never treated as literal content — @@ -476,6 +546,12 @@ mod tests { assert_eq!(super::read_or_stdin("").unwrap(), ""); } + #[test] + fn read_message_or_stdin_passthrough_returns_value() { + let raw = "literal `backticks` and $vars\nwith newline"; + assert_eq!(super::read_message_or_stdin(raw).unwrap(), raw); + } + // --- read_file_or_stdin --- #[test] diff --git a/crates/buzz-cli/tests/message_input.rs b/crates/buzz-cli/tests/message_input.rs new file mode 100644 index 00000000000..a848f4fbbaf --- /dev/null +++ b/crates/buzz-cli/tests/message_input.rs @@ -0,0 +1,136 @@ +use std::io::Write; +use std::process::{Command, Stdio}; +use std::time::{Duration, Instant}; + +const TEST_SECRET_KEY: &str = "0000000000000000000000000000000000000000000000000000000000000001"; +const TEST_CHANNEL: &str = "11111111-1111-4111-8111-111111111111"; + +fn base_command() -> Command { + let mut command = Command::new(env!("CARGO_BIN_EXE_buzz")); + command + .env("BUZZ_PRIVATE_KEY", TEST_SECRET_KEY) + .env_remove("BUZZ_AUTH_TAG") + .arg("--relay") + .arg("http://127.0.0.1:9") + .arg("messages") + .arg("send") + .arg("--channel") + .arg(TEST_CHANNEL); + command +} + +#[test] +fn empty_literal_fails_before_contacting_relay() { + for content in ["", " \n\t"] { + let output = base_command() + .arg("--content") + .arg(content) + .output() + .expect("run buzz"); + assert_eq!(output.status.code(), Some(1), "{output:?}"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("message must have content or attachments")); + assert!(!stderr.contains("network_error")); + } +} + +#[test] +fn empty_stdin_at_eof_fails_before_contacting_relay() { + let output = base_command() + .arg("--content") + .arg("-") + .stdin(Stdio::null()) + .output() + .expect("run buzz"); + assert_eq!(output.status.code(), Some(1), "{output:?}"); + assert!(String::from_utf8_lossy(&output.stderr) + .contains("message must have content or attachments")); +} + +#[test] +fn open_non_tty_stdin_without_data_times_out() { + let started = Instant::now(); + let mut child = base_command() + .arg("--content") + .arg("-") + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn buzz"); + let writer = child.stdin.take().expect("piped stdin"); + + let deadline = started + Duration::from_secs(7); + loop { + if child.try_wait().expect("poll child").is_some() { + break; + } + if Instant::now() >= deadline { + child.kill().expect("kill timed-out child"); + panic!("buzz remained blocked on an open, empty stdin pipe"); + } + std::thread::sleep(Duration::from_millis(25)); + } + + drop(writer); + let output = child.wait_with_output().expect("collect output"); + assert_eq!(output.status.code(), Some(1), "{output:?}"); + assert!(started.elapsed() < Duration::from_secs(7)); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("stdin produced no data within 5 seconds")); + assert!(!stderr.contains("network_error")); +} + +#[test] +fn slow_producer_that_eventually_writes_is_not_timed_out() { + let started = Instant::now(); + let mut child = base_command() + .arg("--content") + .arg("-") + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn buzz"); + let mut writer = child.stdin.take().expect("piped stdin"); + let producer = std::thread::spawn(move || { + std::thread::sleep(Duration::from_secs(3)); + writer.write_all(b"hello after a slow start\n") + }); + + let output = child.wait_with_output().expect("collect output"); + producer + .join() + .expect("join producer") + .expect("write stdin"); + assert!(started.elapsed() >= Duration::from_secs(3)); + assert_eq!(output.status.code(), Some(2), "{output:?}"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("network_error"), "{stderr}"); + assert!(!stderr.contains("stdin produced no data")); +} + +#[test] +fn nonempty_stdin_reaches_the_normal_send_path() { + let mut child = base_command() + .arg("--content") + .arg("-") + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn buzz"); + child + .stdin + .as_mut() + .expect("piped stdin") + .write_all(b"hello from stdin") + .expect("write stdin"); + drop(child.stdin.take()); + + let output = child.wait_with_output().expect("collect output"); + assert_eq!(output.status.code(), Some(2), "{output:?}"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("network_error"), "{stderr}"); + assert!(!stderr.contains("message must have content or attachments")); +} diff --git a/crates/buzz-sdk/src/builders.rs b/crates/buzz-sdk/src/builders.rs index 71c0f1e73db..5c19c40eb7e 100644 --- a/crates/buzz-sdk/src/builders.rs +++ b/crates/buzz-sdk/src/builders.rs @@ -40,6 +40,16 @@ fn check_content(content: &str, max: usize) -> Result<(), SdkError> { Ok(()) } +fn check_message_content(content: &str, media_tags: &[Vec]) -> Result<(), SdkError> { + check_content(content, 64 * 1024)?; + if content.trim().is_empty() && media_tags.is_empty() { + return Err(SdkError::InvalidInput( + "message must have content or attachments".into(), + )); + } + Ok(()) +} + /// Validate hex string has at least `min_len` hex characters. fn check_hex_len(s: &str, min_len: usize, field: &str) -> Result<(), SdkError> { if s.len() < min_len || !s.chars().all(|c| c.is_ascii_hexdigit()) { @@ -229,7 +239,7 @@ pub fn build_message( broadcast: bool, media_tags: &[Vec], ) -> Result { - check_content(content, 64 * 1024)?; + check_message_content(content, media_tags)?; let mut tags = vec![tag(&["h", &channel_id.to_string()])?]; if let Some(tr) = thread_ref { thread_tags(tr, &mut tags)?; @@ -288,7 +298,7 @@ pub fn build_forum_post( mentions: &[&str], media_tags: &[Vec], ) -> Result { - check_content(content, 64 * 1024)?; + check_message_content(content, media_tags)?; let mut tags = vec![tag(&["h", &channel_id.to_string()])?]; mention_tags(mentions, &mut tags)?; imeta_tags(media_tags, &mut tags)?; @@ -305,7 +315,7 @@ pub fn build_forum_comment( mentions: &[&str], media_tags: &[Vec], ) -> Result { - check_content(content, 64 * 1024)?; + check_message_content(content, media_tags)?; let mut tags = vec![tag(&["h", &channel_id.to_string()])?]; thread_tags(thread_ref, &mut tags)?; mention_tags(mentions, &mut tags)?; @@ -2386,6 +2396,33 @@ mod tests { assert!(has_tag(&ev, "h", &cid.to_string())); } + #[test] + fn message_rejects_empty_or_whitespace_only_content_without_media() { + let cid = uuid(); + for content in ["", " \n\t"] { + let err = build_message(cid, content, None, &[], false, &[]).unwrap_err(); + assert!(matches!(err, SdkError::InvalidInput(_))); + } + } + + #[test] + fn message_allows_media_without_text() { + let cid = uuid(); + let media = vec![vec![ + "imeta".to_string(), + "url https://cdn.example/image.png".to_string(), + ]]; + assert!(build_message(cid, "", None, &[], false, &media).is_ok()); + assert!(build_forum_post(cid, " \n", &[], &media).is_ok()); + + let root = event_id(); + let thread_ref = ThreadRef { + root_event_id: root, + parent_event_id: root, + }; + assert!(build_forum_comment(cid, "", &thread_ref, &[], &media).is_ok()); + } + #[test] fn message_preserves_self_mention_p_tag() { // nostr 0.44 strips p tags matching the signer by default. diff --git a/desktop/src-tauri/src/events.rs b/desktop/src-tauri/src/events.rs index 1828b3f5605..79ad0faa795 100644 --- a/desktop/src-tauri/src/events.rs +++ b/desktop/src-tauri/src/events.rs @@ -45,6 +45,14 @@ fn check_content(content: &str) -> Result<(), String> { Ok(()) } +fn check_message_content(content: &str, media_tags: &[Vec]) -> Result<(), String> { + check_content(content)?; + if content.trim().is_empty() && media_tags.is_empty() { + return Err("message must have content or attachments".into()); + } + Ok(()) +} + /// NIP-10 thread reference. pub struct ThreadRef { pub root_event_id: EventId, @@ -298,7 +306,7 @@ pub fn build_message_with_client_tags( if sent_from_thread_tag.is_some() && thread_ref.is_some() { return Err("sent-from-thread provenance requires a top-level message".into()); } - check_content(content)?; + check_message_content(content, media_tags)?; let mut tags = vec![tag(vec!["h", &channel_id.to_string()])?]; if let Some(tr) = thread_ref { tags.extend(thread_tags(tr)?); @@ -321,7 +329,7 @@ pub fn build_forum_post( media_tags: &[Vec], mention_ref_tags: &[Vec], ) -> Result { - check_content(content)?; + check_message_content(content, media_tags)?; let mut tags = vec![tag(vec!["h", &channel_id.to_string()])?]; tags.extend(mention_tags(mentions)?); imeta_tags(media_tags, &mut tags)?; @@ -338,7 +346,7 @@ pub fn build_forum_comment( media_tags: &[Vec], mention_ref_tags: &[Vec], ) -> Result { - check_content(content)?; + check_message_content(content, media_tags)?; let mut tags = vec![tag(vec!["h", &channel_id.to_string()])?]; tags.extend(thread_tags(thread_ref)?); tags.extend(mention_tags(mentions)?); @@ -763,6 +771,10 @@ pub use workflows::{ build_workflow_trigger, }; +#[cfg(test)] +#[path = "events_message_content_tests.rs"] +mod message_content_tests; + // ── Transport ──────────────────────────────────────────────────────────────── #[cfg(test)] diff --git a/desktop/src-tauri/src/events_message_content_tests.rs b/desktop/src-tauri/src/events_message_content_tests.rs new file mode 100644 index 00000000000..a9ace8244b9 --- /dev/null +++ b/desktop/src-tauri/src/events_message_content_tests.rs @@ -0,0 +1,37 @@ +use super::*; + +#[test] +fn channel_messages_require_text_or_media() { + let channel_id = Uuid::new_v4(); + assert!(build_message( + channel_id, + " \n\t", + None, + &[], + &[], + &[], + &[], + &[], + None, + "https://relay.example", + ) + .is_err()); + + let media = vec![vec![ + "imeta".to_string(), + "url https://cdn.example/image.png".to_string(), + ]]; + assert!(build_message( + channel_id, + "", + None, + &[], + &media, + &[], + &[], + &[], + None, + "https://relay.example", + ) + .is_ok()); +}