From 195f2ca3783bd84248d4ce24c5a92b1d557f7e3b Mon Sep 17 00:00:00 2001 From: Taksh Date: Sat, 15 Aug 2026 08:18:53 +0530 Subject: [PATCH 1/4] fix(cli): fail a thread read whose event is in another channel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `messages thread` ORs a reply filter scoped by `#h` with a root filter that selects by id alone. A `--channel` that does not match the event therefore returns the root and none of its replies, and exits 0 — indistinguishable from a thread nobody answered, which is a conclusion an operator will act on. Reject the mismatch with the channel the event is actually in. Writes already refuse the same thing ("parent event belongs to a different channel"); this makes the read agree. A root the relay did not return is left alone, so a genuinely missing event still prints nothing. Signed-off-by: Taksh --- crates/buzz-cli/src/commands/messages.rs | 41 ++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/crates/buzz-cli/src/commands/messages.rs b/crates/buzz-cli/src/commands/messages.rs index 40a9ae80b56..0145e669e44 100644 --- a/crates/buzz-cli/src/commands/messages.rs +++ b/crates/buzz-cli/src/commands/messages.rs @@ -391,6 +391,46 @@ pub async fn cmd_get_messages( Ok(()) } +/// The channel an event says it belongs to, from its `h` tag. +fn event_channel_id(event: &serde_json::Value) -> Option<&str> { + event + .get("tags")? + .as_array()? + .iter() + .find_map(|tag| { + let tag = tag.as_array()?; + (tag.first()?.as_str()? == "h").then(|| tag.get(1)?.as_str()) + }) + .flatten() +} + +/// Fail when the thread root does not live in the requested channel. +/// +/// Only the reply filter is scoped by `#h`; the root is fetched by id alone, +/// so a `--channel` that does not match the event returns the root with none +/// of its replies and exits 0. That is indistinguishable from a real thread +/// with no replies, and reads as "nobody answered". Writes already reject the +/// same mismatch ("parent event belongs to a different channel"); this makes +/// the read say so too. +fn check_thread_channel( + events: &[serde_json::Value], + event_id: &str, + channel_id: &str, +) -> Result<(), CliError> { + let root = events + .iter() + .find(|e| e.get("id").and_then(|v| v.as_str()) == Some(event_id)); + let Some(actual) = root.and_then(event_channel_id) else { + return Ok(()); + }; + if actual == channel_id { + return Ok(()); + } + Err(CliError::Usage(format!( + "event belongs to a different channel: --channel is {channel_id}, event {event_id} is in {actual}" + ))) +} + pub async fn cmd_get_thread( client: &BuzzClient, channel_id: &str, @@ -421,6 +461,7 @@ pub async fn cmd_get_thread( }); let resp = client.query_multi(&[reply_filter, root_filter]).await?; let mut events: Vec = serde_json::from_str(&resp).unwrap_or_default(); + check_thread_channel(&events, event_id, channel_id)?; events.sort_by_key(|e| e.get("created_at").and_then(|v| v.as_u64()).unwrap_or(0)); let normalized = normalize_events(&events); println!("{}", format_events(&normalized, format)); From dcb8f688a525b168f050a5408dbf59eb7ed97282 Mon Sep 17 00:00:00 2001 From: Taksh Date: Sat, 15 Aug 2026 08:20:56 +0530 Subject: [PATCH 2/4] test(cli): cover the thread-read channel check A root in the requested channel passes, a root from another channel is rejected with both UUIDs named, a reply's own h tag does not decide the outcome, and a missing or untagged root still passes through. Signed-off-by: Taksh --- crates/buzz-cli/src/commands/messages.rs | 57 ++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/crates/buzz-cli/src/commands/messages.rs b/crates/buzz-cli/src/commands/messages.rs index 0145e669e44..04c4dc6a541 100644 --- a/crates/buzz-cli/src/commands/messages.rs +++ b/crates/buzz-cli/src/commands/messages.rs @@ -1414,3 +1414,60 @@ mod tests { assert_eq!(match_profiles_by_name(&events, "Aaron").len(), 1); } } + +#[cfg(test)] +mod thread_channel_tests { + use super::check_thread_channel; + + const ROOT: &str = "0a9882747d0029df3fc9742b0755068a4ae24426b7e1f18cf44800409fdb437f"; + const CHANNEL_A: &str = "2cf6cfd0-b917-4ea0-b2d8-a29dea949b77"; + const CHANNEL_B: &str = "17c553b9-363f-461a-8f46-ae11f765ea3b"; + + fn root_event(channel: &str) -> serde_json::Value { + serde_json::json!({ + "id": ROOT, + "tags": [["h", channel], ["p", "abc"]], + }) + } + + #[test] + fn accepts_a_root_in_the_requested_channel() { + assert!(check_thread_channel(&[root_event(CHANNEL_A)], ROOT, CHANNEL_A).is_ok()); + } + + #[test] + fn rejects_a_root_from_another_channel() { + let err = check_thread_channel(&[root_event(CHANNEL_A)], ROOT, CHANNEL_B).unwrap_err(); + let msg = err.to_string(); + assert!( + msg.contains(CHANNEL_A), + "error must name the real channel: {msg}" + ); + assert!( + msg.contains(CHANNEL_B), + "error must name the requested channel: {msg}" + ); + } + + #[test] + fn ignores_replies_when_locating_the_root() { + // Only the event whose id matches is checked; a reply carrying some + // other channel tag must not decide the outcome. + let events = vec![ + serde_json::json!({"id": "deadbeef", "tags": [["h", CHANNEL_B]]}), + root_event(CHANNEL_A), + ]; + assert!(check_thread_channel(&events, ROOT, CHANNEL_A).is_ok()); + } + + #[test] + fn passes_through_when_the_root_was_not_returned() { + assert!(check_thread_channel(&[], ROOT, CHANNEL_A).is_ok()); + } + + #[test] + fn passes_through_when_the_root_has_no_h_tag() { + let event = serde_json::json!({"id": ROOT, "tags": [["p", "abc"]]}); + assert!(check_thread_channel(&[event], ROOT, CHANNEL_A).is_ok()); + } +} From cc08901141183e7a019f2199fd83927cb5ddc0ab Mon Sep 17 00:00:00 2001 From: Taksh Date: Sun, 16 Aug 2026 21:02:21 +0530 Subject: [PATCH 3/4] fix(cli): compare thread channels as UUID values, not text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The review points out the check compared UUID text byte-for-byte, so an uppercase but valid --channel falsely mismatched the lowercase UUID in the event's h tag. `event_channel_id` now returns a parsed `Uuid`, mirroring `buzz_sdk::extract_channel_id` — first *parseable* h tag, so a junk tag ahead of the real one does not hide it — and the comparison is by value. The argument is also canonicalized before it reaches the relay. Comparing by value alone would have been half a fix: `Uuid::parse_str` accepts uppercase, braced and unhyphenated spellings, but h tags are lowercase hyphenated on the wire (NIP-PL `h_grammar: uuid-v4-lowercase`), so a non-canonical --channel still matched no replies in the `#h` filter — turning the false error back into the silent "nobody answered" thread this PR exists to remove. 357 buzz-cli lib tests pass. Signed-off-by: Taksh --- crates/buzz-cli/src/commands/messages.rs | 113 ++++++++++++++++++----- 1 file changed, 88 insertions(+), 25 deletions(-) diff --git a/crates/buzz-cli/src/commands/messages.rs b/crates/buzz-cli/src/commands/messages.rs index 04c4dc6a541..f9cedb1c068 100644 --- a/crates/buzz-cli/src/commands/messages.rs +++ b/crates/buzz-cli/src/commands/messages.rs @@ -391,17 +391,21 @@ pub async fn cmd_get_messages( Ok(()) } -/// The channel an event says it belongs to, from its `h` tag. -fn event_channel_id(event: &serde_json::Value) -> Option<&str> { - event - .get("tags")? - .as_array()? - .iter() - .find_map(|tag| { - let tag = tag.as_array()?; - (tag.first()?.as_str()? == "h").then(|| tag.get(1)?.as_str()) - }) - .flatten() +/// The channel an event says it belongs to: the first `h` tag whose value +/// parses as a UUID. +/// +/// Mirrors `buzz_sdk::extract_channel_id`, which is what the rest of the +/// codebase reads an event's channel with — an `h` tag that is not a UUID is +/// skipped rather than ending the search, so a garbage tag ahead of the real +/// one does not hide it. (This path holds `serde_json::Value`, not +/// `nostr::Event`, so the walk cannot be shared outright.) +fn event_channel_id(event: &serde_json::Value) -> Option { + event.get("tags")?.as_array()?.iter().find_map(|tag| { + let tag = tag.as_array()?; + (tag.first()?.as_str()? == "h") + .then(|| Uuid::parse_str(tag.get(1)?.as_str()?).ok()) + .flatten() + }) } /// Fail when the thread root does not live in the requested channel. @@ -412,22 +416,32 @@ fn event_channel_id(event: &serde_json::Value) -> Option<&str> { /// with no replies, and reads as "nobody answered". Writes already reject the /// same mismatch ("parent event belongs to a different channel"); this makes /// the read say so too. +/// +/// Compares UUID values, not their text: `--channel` is parsed by +/// `Uuid::parse_str`, which accepts uppercase, braced and unhyphenated forms, +/// while `h` tags are `uuid-v4-lowercase` on the wire (NIP-PL `h_grammar`). +/// +/// A root the relay did not return is left alone, so a genuinely missing event +/// still prints an empty array. fn check_thread_channel( events: &[serde_json::Value], event_id: &str, - channel_id: &str, + channel: Uuid, ) -> Result<(), CliError> { - let root = events + let Some(root) = events .iter() - .find(|e| e.get("id").and_then(|v| v.as_str()) == Some(event_id)); - let Some(actual) = root.and_then(event_channel_id) else { + .find(|e| e.get("id").and_then(|v| v.as_str()) == Some(event_id)) + else { + return Ok(()); + }; + let Some(actual) = event_channel_id(root) else { return Ok(()); }; - if actual == channel_id { + if actual == channel { return Ok(()); } Err(CliError::Usage(format!( - "event belongs to a different channel: --channel is {channel_id}, event {event_id} is in {actual}" + "event belongs to a different channel: --channel is {channel}, event {event_id} is in {actual}" ))) } @@ -439,7 +453,14 @@ pub async fn cmd_get_thread( depth_limit: Option, format: &crate::OutputFormat, ) -> Result<(), CliError> { - validate_uuid(channel_id)?; + // Canonicalize once. `Uuid::parse_str` accepts uppercase, braced and + // unhyphenated spellings, but `h` tags on the wire are lowercase + // hyphenated (NIP-PL `h_grammar: uuid-v4-lowercase`) — so the raw argument + // is the wrong thing to put in the filter as well as the wrong thing to + // compare with. Sending a non-canonical spelling matches no replies, which + // is the same "nobody answered" silence this command is fixing. + let channel = parse_uuid(channel_id)?; + let channel_id = channel.hyphenated().to_string(); validate_hex64(event_id)?; let limit = limit.unwrap_or(100).min(500); @@ -461,7 +482,7 @@ pub async fn cmd_get_thread( }); let resp = client.query_multi(&[reply_filter, root_filter]).await?; let mut events: Vec = serde_json::from_str(&resp).unwrap_or_default(); - check_thread_channel(&events, event_id, channel_id)?; + check_thread_channel(&events, event_id, channel)?; events.sort_by_key(|e| e.get("created_at").and_then(|v| v.as_u64()).unwrap_or(0)); let normalized = normalize_events(&events); println!("{}", format_events(&normalized, format)); @@ -1417,12 +1438,17 @@ mod tests { #[cfg(test)] mod thread_channel_tests { - use super::check_thread_channel; + use super::{check_thread_channel, event_channel_id}; + use uuid::Uuid; const ROOT: &str = "0a9882747d0029df3fc9742b0755068a4ae24426b7e1f18cf44800409fdb437f"; const CHANNEL_A: &str = "2cf6cfd0-b917-4ea0-b2d8-a29dea949b77"; const CHANNEL_B: &str = "17c553b9-363f-461a-8f46-ae11f765ea3b"; + fn uuid(s: &str) -> Uuid { + Uuid::parse_str(s).expect("test constant must be a UUID") + } + fn root_event(channel: &str) -> serde_json::Value { serde_json::json!({ "id": ROOT, @@ -1432,12 +1458,13 @@ mod thread_channel_tests { #[test] fn accepts_a_root_in_the_requested_channel() { - assert!(check_thread_channel(&[root_event(CHANNEL_A)], ROOT, CHANNEL_A).is_ok()); + assert!(check_thread_channel(&[root_event(CHANNEL_A)], ROOT, uuid(CHANNEL_A)).is_ok()); } #[test] fn rejects_a_root_from_another_channel() { - let err = check_thread_channel(&[root_event(CHANNEL_A)], ROOT, CHANNEL_B).unwrap_err(); + let err = + check_thread_channel(&[root_event(CHANNEL_A)], ROOT, uuid(CHANNEL_B)).unwrap_err(); let msg = err.to_string(); assert!( msg.contains(CHANNEL_A), @@ -1449,6 +1476,22 @@ mod thread_channel_tests { ); } + #[test] + fn accepts_a_channel_argument_that_differs_only_in_spelling() { + // `Uuid::parse_str` accepts these; the `h` tag is always lowercase + // hyphenated. Comparing the text would reject all three. + for spelling in [ + CHANNEL_A.to_uppercase(), + CHANNEL_A.replace('-', ""), + format!("{{{CHANNEL_A}}}"), + ] { + assert!( + check_thread_channel(&[root_event(CHANNEL_A)], ROOT, uuid(&spelling)).is_ok(), + "spelling {spelling} must be accepted" + ); + } + } + #[test] fn ignores_replies_when_locating_the_root() { // Only the event whose id matches is checked; a reply carrying some @@ -1457,17 +1500,37 @@ mod thread_channel_tests { serde_json::json!({"id": "deadbeef", "tags": [["h", CHANNEL_B]]}), root_event(CHANNEL_A), ]; - assert!(check_thread_channel(&events, ROOT, CHANNEL_A).is_ok()); + assert!(check_thread_channel(&events, ROOT, uuid(CHANNEL_A)).is_ok()); } #[test] fn passes_through_when_the_root_was_not_returned() { - assert!(check_thread_channel(&[], ROOT, CHANNEL_A).is_ok()); + assert!(check_thread_channel(&[], ROOT, uuid(CHANNEL_A)).is_ok()); } #[test] fn passes_through_when_the_root_has_no_h_tag() { let event = serde_json::json!({"id": ROOT, "tags": [["p", "abc"]]}); - assert!(check_thread_channel(&[event], ROOT, CHANNEL_A).is_ok()); + assert!(check_thread_channel(&[event], ROOT, uuid(CHANNEL_A)).is_ok()); + } + + #[test] + fn skips_an_unparseable_h_tag_ahead_of_the_real_one() { + // `buzz_sdk::extract_channel_id` takes the first *parseable* `h`, not + // the first `h`; a junk tag ahead of the real one must not win. + let event = serde_json::json!({ + "id": ROOT, + "tags": [["h", "not-a-uuid"], ["h", CHANNEL_A]], + }); + assert_eq!(event_channel_id(&event), Some(uuid(CHANNEL_A))); + assert!(check_thread_channel(&[event], ROOT, uuid(CHANNEL_A)).is_ok()); + } + + #[test] + fn reads_an_uppercase_h_tag_by_value() { + // Nothing on the wire should produce this, but an `h` tag written by a + // client that skipped canonicalization must not read as a mismatch. + let event = root_event(&CHANNEL_A.to_uppercase()); + assert!(check_thread_channel(&[event], ROOT, uuid(CHANNEL_A)).is_ok()); } } From 7300e69c2b4d9265e273b0fdd36ea328d0ed7882 Mon Sep 17 00:00:00 2001 From: Taksh Date: Sun, 16 Aug 2026 21:02:27 +0530 Subject: [PATCH 4/4] fix(cli): fail closed on a thread root with no readable channel tag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The review points out a returned root with no valid h tag passed through, even though the command cannot then establish that the root belongs to the requested channel — printing it with no replies is the same silence the check exists to remove. It is now a usage error naming the event. A root the relay did not return at all still passes through, so a genuinely missing event keeps printing an empty array rather than turning into an error. 358 buzz-cli lib tests pass; clippy --all-targets -D warnings and fmt clean. Signed-off-by: Taksh --- crates/buzz-cli/src/commands/messages.rs | 25 ++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/crates/buzz-cli/src/commands/messages.rs b/crates/buzz-cli/src/commands/messages.rs index f9cedb1c068..e318fb8c9af 100644 --- a/crates/buzz-cli/src/commands/messages.rs +++ b/crates/buzz-cli/src/commands/messages.rs @@ -421,7 +421,10 @@ fn event_channel_id(event: &serde_json::Value) -> Option { /// `Uuid::parse_str`, which accepts uppercase, braced and unhyphenated forms, /// while `h` tags are `uuid-v4-lowercase` on the wire (NIP-PL `h_grammar`). /// -/// A root the relay did not return is left alone, so a genuinely missing event +/// A root that came back without a readable `h` tag fails closed: the command +/// cannot show that it belongs to the requested channel, and printing it with +/// no replies is the exact silence this check exists to remove. A root the +/// relay did not return at all is left alone, so a genuinely missing event /// still prints an empty array. fn check_thread_channel( events: &[serde_json::Value], @@ -435,7 +438,9 @@ fn check_thread_channel( return Ok(()); }; let Some(actual) = event_channel_id(root) else { - return Ok(()); + return Err(CliError::Usage(format!( + "event {event_id} carries no readable channel (h) tag, so it cannot be shown as a thread in {channel}" + ))); }; if actual == channel { return Ok(()); @@ -1509,9 +1514,21 @@ mod thread_channel_tests { } #[test] - fn passes_through_when_the_root_has_no_h_tag() { + fn rejects_a_root_with_no_h_tag() { + // The command cannot establish that this root is in the requested + // channel, so it must not print it as that channel's thread. let event = serde_json::json!({"id": ROOT, "tags": [["p", "abc"]]}); - assert!(check_thread_channel(&[event], ROOT, uuid(CHANNEL_A)).is_ok()); + let err = check_thread_channel(&[event], ROOT, uuid(CHANNEL_A)).unwrap_err(); + assert!( + err.to_string().contains("no readable channel"), + "unexpected error: {err}" + ); + } + + #[test] + fn rejects_a_root_whose_h_tag_is_not_a_uuid() { + let event = serde_json::json!({"id": ROOT, "tags": [["h", "not-a-uuid"]]}); + assert!(check_thread_channel(&[event], ROOT, uuid(CHANNEL_A)).is_err()); } #[test]