From 9a8e8d1d7d547b95aeca8ef2d4227669170b6b73 Mon Sep 17 00:00:00 2001 From: oletillmann Date: Fri, 21 Aug 2026 18:29:37 +0200 Subject: [PATCH 1/2] fix(cli): reject empty channel messages Bound non-interactive stdin reads so inherited open pipes cannot block agent sends indefinitely. Reject empty message envelopes at CLI, SDK, and desktop builder boundaries while preserving media-only messages. Co-authored-by: oletillmann Signed-off-by: oletillmann --- crates/buzz-cli/src/commands/messages.rs | 11 +- crates/buzz-cli/src/validate.rs | 49 ++++++++ crates/buzz-cli/tests/message_input.rs | 107 ++++++++++++++++++ crates/buzz-sdk/src/builders.rs | 43 ++++++- desktop/src-tauri/src/events.rs | 18 ++- .../src/events_message_content_tests.rs | 37 ++++++ 6 files changed, 256 insertions(+), 9 deletions(-) create mode 100644 crates/buzz-cli/tests/message_input.rs create mode 100644 desktop/src-tauri/src/events_message_content_tests.rs 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..ae78eb51d31 100644 --- a/crates/buzz-cli/src/validate.rs +++ b/crates/buzz-cli/src/validate.rs @@ -178,6 +178,49 @@ pub fn read_or_stdin(value: &str) -> Result { } } +/// Read message content, bounding non-interactive stdin so an inherited open +/// pipe cannot leave an agent send blocked forever. +/// +/// A terminal remains interactive and may wait for a human to finish input. +/// Pipes must deliver their complete payload (including EOF) promptly. +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 PIPE_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(2); + let (tx, rx) = std::sync::mpsc::sync_channel(1); + std::thread::spawn(move || { + let mut buf = String::new(); + let result = std::io::stdin() + .read_to_string(&mut buf) + .map(|_| buf) + .map_err(|e| CliError::Other(format!("failed to read stdin: {e}"))); + let _ = tx.send(result); + }); + + match rx.recv_timeout(PIPE_READ_TIMEOUT) { + Ok(result) => result, + Err(std::sync::mpsc::RecvTimeoutError::Timeout) => Err(CliError::Usage( + "stdin did not reach EOF within 2 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 +519,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..4bedca8ad20 --- /dev/null +++ b/crates/buzz-cli/tests/message_input.rs @@ -0,0 +1,107 @@ +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(5); + 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(5)); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("stdin did not reach EOF within 2 seconds")); + assert!(!stderr.contains("network_error")); +} + +#[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()); +} From fcb3e06f6bcb2cd09e998d525394ed1812ca65b5 Mon Sep 17 00:00:00 2001 From: oletillmann Date: Fri, 21 Aug 2026 18:35:18 +0200 Subject: [PATCH 2/2] fix(cli): preserve slow stdin producers Limit the guard to initial stdin inactivity. Once the first byte arrives, allow the producer to take as long as needed to finish, and cover both delayed producers and permanently idle pipes. Co-authored-by: oletillmann Signed-off-by: oletillmann --- crates/buzz-cli/src/validate.rs | 55 +++++++++++++++++++------- crates/buzz-cli/tests/message_input.rs | 35 ++++++++++++++-- 2 files changed, 73 insertions(+), 17 deletions(-) diff --git a/crates/buzz-cli/src/validate.rs b/crates/buzz-cli/src/validate.rs index ae78eb51d31..16bb85461b4 100644 --- a/crates/buzz-cli/src/validate.rs +++ b/crates/buzz-cli/src/validate.rs @@ -178,11 +178,12 @@ pub fn read_or_stdin(value: &str) -> Result { } } -/// Read message content, bounding non-interactive stdin so an inherited open -/// pipe cannot leave an agent send blocked forever. +/// 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. -/// Pipes must deliver their complete payload (including EOF) promptly. +/// 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()); @@ -198,21 +199,47 @@ pub fn read_message_or_stdin(value: &str) -> Result { return Ok(buf); } - const PIPE_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(2); - let (tx, rx) = std::sync::mpsc::sync_channel(1); + 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 buf = String::new(); - let result = std::io::stdin() - .read_to_string(&mut buf) - .map(|_| buf) - .map_err(|e| CliError::Other(format!("failed to read stdin: {e}"))); - let _ = tx.send(result); + 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 rx.recv_timeout(PIPE_READ_TIMEOUT) { - Ok(result) => result, + 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 did not reach EOF within 2 seconds; pipe content explicitly or redirect a file" + "stdin produced no data within 5 seconds; pipe content explicitly or redirect a file" .into(), )), Err(std::sync::mpsc::RecvTimeoutError::Disconnected) => Err(CliError::Other( diff --git a/crates/buzz-cli/tests/message_input.rs b/crates/buzz-cli/tests/message_input.rs index 4bedca8ad20..a848f4fbbaf 100644 --- a/crates/buzz-cli/tests/message_input.rs +++ b/crates/buzz-cli/tests/message_input.rs @@ -60,7 +60,7 @@ fn open_non_tty_stdin_without_data_times_out() { .expect("spawn buzz"); let writer = child.stdin.take().expect("piped stdin"); - let deadline = started + Duration::from_secs(5); + let deadline = started + Duration::from_secs(7); loop { if child.try_wait().expect("poll child").is_some() { break; @@ -75,12 +75,41 @@ fn open_non_tty_stdin_without_data_times_out() { 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(5)); + assert!(started.elapsed() < Duration::from_secs(7)); let stderr = String::from_utf8_lossy(&output.stderr); - assert!(stderr.contains("stdin did not reach EOF within 2 seconds")); + 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()