diff --git a/crates/buzz-cli/src/commands/issues.rs b/crates/buzz-cli/src/commands/issues.rs index 15284a0d7bd..47c806bd849 100644 --- a/crates/buzz-cli/src/commands/issues.rs +++ b/crates/buzz-cli/src/commands/issues.rs @@ -3,7 +3,9 @@ use std::collections::{HashMap, HashSet}; use crate::client::BuzzClient; use crate::commands::with_git_provenance; use crate::error::CliError; -use crate::validate::{read_or_stdin, sdk_err, validate_hex64, validate_repo_id}; +use crate::validate::{ + parse_hex64, read_or_stdin, repo_coord_a_value, sdk_err, validate_hex64, validate_repo_id, +}; use buzz_sdk::{GitIssueMeta, GitRepoCoord, GitStatusMeta}; use nostr::Timestamp; use serde::Deserialize; @@ -319,7 +321,9 @@ async fn publish_issue_assignment_operation( label: Option<&str>, operation: IssueAssignmentOperation, ) -> Result<(), CliError> { - validate_hex64(issue)?; + // `issue` reaches a `#e` generic tag filter in `issue_assignment_context`, + // which is matched as a raw string against a lowercase tag. + let issue = &parse_hex64(issue)?; validate_hex64(repo_owner)?; validate_repo_id(repo_id)?; for assignee in assignees { @@ -462,10 +466,7 @@ pub async fn cmd_list_issues( label: Option<&str>, limit: Option, ) -> Result<(), CliError> { - validate_hex64(repo_owner)?; - validate_repo_id(repo_id)?; - - let a_value = format!("30617:{repo_owner}:{repo_id}"); + let a_value = repo_coord_a_value(repo_owner, repo_id)?; let mut filter = serde_json::json!({ "kinds": [1621], "#a": [a_value] diff --git a/crates/buzz-cli/src/commands/messages.rs b/crates/buzz-cli/src/commands/messages.rs index 40a9ae80b56..9fae4efd195 100644 --- a/crates/buzz-cli/src/commands/messages.rs +++ b/crates/buzz-cli/src/commands/messages.rs @@ -5,7 +5,7 @@ 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, + infer_language, parse_event_id, parse_hex64, parse_uuid, read_or_stdin, truncate_diff, validate_content_size, validate_hex64, validate_uuid, MAX_DIFF_BYTES, }; use buzz_sdk::mentions::{ @@ -400,12 +400,31 @@ pub async fn cmd_get_thread( format: &crate::OutputFormat, ) -> Result<(), CliError> { validate_uuid(channel_id)?; - validate_hex64(event_id)?; + let event_id = &parse_hex64(event_id)?; let limit = limit.unwrap_or(100).min(500); - // Two filters ORed in a single HTTP call: - // 1. Replies referencing this event via e-tag (no kind restriction) - // 2. The root event itself by ID + let [reply_filter, root_filter] = thread_filters(channel_id, event_id, limit, depth_limit); + let resp = client.query_multi(&[reply_filter, root_filter]).await?; + let mut events: Vec = serde_json::from_str(&resp).unwrap_or_default(); + 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)); + Ok(()) +} + +/// The two filters `buzz messages thread` ORs in a single call: replies that +/// carry this event in an `e` tag, and the root event itself by id. +/// +/// `event_id` must already be lowercased (`parse_hex64`). `ids` is parsed into +/// a typed `EventId` on the relay and so tolerates either case, but `#e` is a +/// generic tag filter compared as a raw string against a lowercase tag — an +/// uppercase id there returns the root and none of its replies. +fn thread_filters( + channel_id: &str, + event_id: &str, + limit: u32, + depth_limit: Option, +) -> [serde_json::Value; 2] { let mut reply_filter = serde_json::json!({ "kinds": [9, 40002, 40003, 40008, 45003], "#h": [channel_id], @@ -419,12 +438,7 @@ pub async fn cmd_get_thread( "ids": [event_id], "limit": 1 }); - let resp = client.query_multi(&[reply_filter, root_filter]).await?; - let mut events: Vec = serde_json::from_str(&resp).unwrap_or_default(); - 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)); - Ok(()) + [reply_filter, root_filter] } pub async fn cmd_search( @@ -1373,3 +1387,39 @@ mod tests { assert_eq!(match_profiles_by_name(&events, "Aaron").len(), 1); } } + +#[cfg(test)] +mod thread_filter_tests { + use super::thread_filters; + use crate::validate::parse_hex64; + + const CHANNEL: &str = "550e8400-e29b-41d4-a716-446655440000"; + + #[test] + fn the_e_tag_filter_carries_the_normalized_id() { + // `#e` is compared as a raw string against a lowercase tag, so an + // uppercase id here would return the root and none of its replies. + let upper = "ABCDEF0123456789".repeat(4); + let id = parse_hex64(&upper).unwrap(); + let [reply, root] = thread_filters(CHANNEL, &id, 100, None); + assert_eq!(reply["#e"][0], upper.to_lowercase().as_str()); + assert_eq!(root["ids"][0], upper.to_lowercase().as_str()); + } + + #[test] + fn the_channel_and_limit_ride_along_unchanged() { + let id = "a".repeat(64); + let [reply, root] = thread_filters(CHANNEL, &id, 25, None); + assert_eq!(reply["#h"][0], CHANNEL); + assert_eq!(reply["limit"], 25); + assert_eq!(root["limit"], 1); + assert!(reply.get("depth_limit").is_none()); + } + + #[test] + fn a_depth_limit_is_added_only_when_given() { + let id = "a".repeat(64); + let [reply, _] = thread_filters(CHANNEL, &id, 100, Some(3)); + assert_eq!(reply["depth_limit"], 3); + } +} diff --git a/crates/buzz-cli/src/commands/patches.rs b/crates/buzz-cli/src/commands/patches.rs index 413934a3c11..d1d16e31a52 100644 --- a/crates/buzz-cli/src/commands/patches.rs +++ b/crates/buzz-cli/src/commands/patches.rs @@ -2,7 +2,8 @@ use crate::client::BuzzClient; use crate::commands::with_git_provenance; use crate::error::CliError; use crate::validate::{ - read_file_or_stdin, read_or_stdin, sdk_err, validate_hex64, validate_repo_id, + read_file_or_stdin, read_or_stdin, repo_coord_a_value, sdk_err, validate_hex64, + validate_repo_id, }; use buzz_sdk::{GitAppliedPatchRef, GitPatchMeta, GitRepoCoord, GitStatus, GitStatusMeta}; @@ -90,10 +91,7 @@ pub async fn cmd_list_patches( author: Option<&str>, limit: Option, ) -> Result<(), CliError> { - validate_hex64(repo_owner)?; - validate_repo_id(repo_id)?; - - let a_value = format!("30617:{repo_owner}:{repo_id}"); + let a_value = repo_coord_a_value(repo_owner, repo_id)?; let mut filter = serde_json::json!({ "kinds": [1617], "#a": [a_value] diff --git a/crates/buzz-cli/src/commands/pr.rs b/crates/buzz-cli/src/commands/pr.rs index 74c580a6d6a..64cbf4603d4 100644 --- a/crates/buzz-cli/src/commands/pr.rs +++ b/crates/buzz-cli/src/commands/pr.rs @@ -2,7 +2,8 @@ use crate::client::BuzzClient; use crate::commands::with_git_provenance; use crate::error::CliError; use crate::validate::{ - read_file_or_stdin, read_or_stdin, sdk_err, validate_hex64, validate_repo_id, + read_file_or_stdin, read_or_stdin, repo_coord_a_value, sdk_err, validate_hex64, + validate_repo_id, }; use buzz_sdk::{GitPrUpdateMeta, GitPullRequestMeta, GitRepoCoord, GitStatusMeta}; @@ -132,10 +133,7 @@ pub async fn cmd_list_prs( label: Option<&str>, limit: Option, ) -> Result<(), CliError> { - validate_hex64(repo_owner)?; - validate_repo_id(repo_id)?; - - let a_value = format!("30617:{repo_owner}:{repo_id}"); + let a_value = repo_coord_a_value(repo_owner, repo_id)?; let mut filter = serde_json::json!({ "kinds": [1618], "#a": [a_value] diff --git a/crates/buzz-cli/src/commands/reactions.rs b/crates/buzz-cli/src/commands/reactions.rs index 9e23d301312..7a6ffc2f1fd 100644 --- a/crates/buzz-cli/src/commands/reactions.rs +++ b/crates/buzz-cli/src/commands/reactions.rs @@ -4,7 +4,23 @@ use nostr::EventId; use crate::client::{normalize_write_response, BuzzClient}; use crate::error::CliError; -use crate::validate::validate_hex64; +use crate::validate::parse_hex64; + +/// The kind:7 query behind `reactions get` and `reactions remove`. +/// +/// `event_id` must already be lowercased (`parse_hex64`): `#e` is a generic +/// tag filter, compared as a raw string against a tag every event writes +/// lowercase. `author`, when given, narrows to the caller's own reactions. +fn reaction_filter(event_id: &str, author: Option<&str>) -> serde_json::Value { + let mut filter = serde_json::json!({ + "kinds": [7], + "#e": [event_id], + }); + if let Some(author) = author { + filter["authors"] = serde_json::json!([author]); + } + filter +} pub async fn cmd_add_reaction( client: &BuzzClient, @@ -12,7 +28,7 @@ pub async fn cmd_add_reaction( emoji: &str, emoji_url: Option<&str>, ) -> Result<(), CliError> { - validate_hex64(event_id)?; + let event_id = &parse_hex64(event_id)?; let target_eid = EventId::parse(event_id).map_err(|e| CliError::Usage(format!("invalid event ID: {e}")))?; @@ -36,16 +52,14 @@ pub async fn cmd_remove_reaction( event_id: &str, emoji: &str, ) -> Result<(), CliError> { - validate_hex64(event_id)?; + // Reaches a `#e` generic tag filter below, matched as a raw string + // against a lowercase tag. + let event_id = &parse_hex64(event_id)?; let keys = client.keys(); // Find our reaction event by querying kind:7 reactions on this event from us let my_pk = keys.public_key().to_hex(); - let filter = serde_json::json!({ - "kinds": [7], - "#e": [event_id], - "authors": [my_pk] - }); + let filter = reaction_filter(event_id, Some(&my_pk)); let raw = client.query(&filter).await?; let events: serde_json::Value = serde_json::from_str(&raw) .map_err(|e| CliError::Other(format!("failed to parse reactions query: {e}")))?; @@ -78,11 +92,8 @@ pub async fn cmd_remove_reaction( } pub async fn cmd_get_reactions(client: &BuzzClient, event_id: &str) -> Result<(), CliError> { - validate_hex64(event_id)?; - let filter = serde_json::json!({ - "kinds": [7], - "#e": [event_id] - }); + let event_id = &parse_hex64(event_id)?; + let filter = reaction_filter(event_id, None); let resp = client.query(&filter).await?; let events: Vec = serde_json::from_str(&resp).unwrap_or_default(); @@ -136,3 +147,27 @@ pub async fn dispatch(cmd: crate::ReactionsCmd, client: &BuzzClient) -> Result<( ReactionsCmd::Get { event } => cmd_get_reactions(client, &event).await, } } + +#[cfg(test)] +mod reaction_filter_tests { + use super::reaction_filter; + use crate::validate::parse_hex64; + + #[test] + fn the_e_tag_filter_carries_the_normalized_id() { + let upper = "ABCDEF0123456789".repeat(4); + let id = parse_hex64(&upper).unwrap(); + let filter = reaction_filter(&id, None); + assert_eq!(filter["#e"][0], upper.to_lowercase().as_str()); + assert_eq!(filter["kinds"][0], 7); + assert!(filter.get("authors").is_none()); + } + + #[test] + fn an_author_narrows_the_query_when_given() { + let id = "a".repeat(64); + let me = "b".repeat(64); + let filter = reaction_filter(&id, Some(&me)); + assert_eq!(filter["authors"][0], me.as_str()); + } +} diff --git a/crates/buzz-cli/src/validate.rs b/crates/buzz-cli/src/validate.rs index 4985b441417..17b1437e6bb 100644 --- a/crates/buzz-cli/src/validate.rs +++ b/crates/buzz-cli/src/validate.rs @@ -25,7 +25,11 @@ pub fn validate_uuid(s: &str) -> Result<(), CliError> { Ok(()) } -/// Validate 64-character lowercase hex string (event_id, pubkey). +/// Validate a 64-character hex string (event_id, pubkey). +/// +/// Accepts either case — `is_ascii_hexdigit` does. Use [`parse_hex64`] when +/// the value goes into a NIP-01 *tag* filter (`#e`, `#p`, `#a`): tag values +/// are compared as strings, so `ABC…` does not match a stored `abc…`. pub fn validate_hex64(s: &str) -> Result<(), CliError> { if s.len() != 64 || !s.chars().all(|c| c.is_ascii_hexdigit()) { return Err(CliError::Usage(format!( @@ -35,6 +39,18 @@ pub fn validate_hex64(s: &str) -> Result<(), CliError> { Ok(()) } +/// Validate a 64-character hex string and return it lowercased. +/// +/// `ids` and `authors` are parsed into typed values on the relay, so case +/// there is harmless. Generic tag filters are not: `filter_match_one` +/// compares `#e`/`#p` values with `==` against the raw tag string, and every +/// event in the tree carries lowercase hex. A query built from an uppercase +/// id therefore matches nothing, silently. +pub fn parse_hex64(s: &str) -> Result { + validate_hex64(s)?; + Ok(s.to_ascii_lowercase()) +} + /// Validate a git repo identifier: `[a-zA-Z0-9._-]{1,64}`, no leading dots, no `..`. pub fn validate_repo_id(s: &str) -> Result<(), CliError> { if s.is_empty() || s.len() > 64 { @@ -60,6 +76,19 @@ pub fn validate_repo_id(s: &str) -> Result<(), CliError> { Ok(()) } +/// Build the `30617::` coordinate used as an `a` tag value. +/// +/// Mirrors `buzz_sdk::GitRepoCoord::to_a_tag_value`, which is what every +/// published event's `a` tag is built from: the owner pubkey is lowercased, +/// the repo `d`-tag is left exactly as given (a `d` tag is case-sensitive). +/// A query that builds this string differently matches nothing — `#a` is a +/// generic tag filter, compared as a raw string. +pub fn repo_coord_a_value(repo_owner: &str, repo_id: &str) -> Result { + let owner = parse_hex64(repo_owner)?; + validate_repo_id(repo_id)?; + Ok(format!("30617:{owner}:{repo_id}")) +} + /// Validate content does not exceed MAX_CONTENT_BYTES (65,536). pub fn validate_content_size(content: &str) -> Result<(), CliError> { if content.len() > MAX_CONTENT_BYTES { @@ -256,6 +285,46 @@ mod tests { assert!(matches!(err, CliError::Usage(_))); } + // --- parse_hex64 --- + + #[test] + fn parse_hex64_lowercases_an_uppercase_id() { + let upper = "ABCDEF0123456789".repeat(4); + assert_eq!(super::parse_hex64(&upper).unwrap(), upper.to_lowercase()); + } + + #[test] + fn parse_hex64_leaves_a_lowercase_id_alone() { + let lower = "abcdef0123456789".repeat(4); + assert_eq!(super::parse_hex64(&lower).unwrap(), lower); + } + + #[test] + fn parse_hex64_rejects_what_validate_hex64_rejects() { + assert!(super::parse_hex64("").is_err()); + assert!(super::parse_hex64(&"a".repeat(63)).is_err()); + assert!(super::parse_hex64(&"a".repeat(65)).is_err()); + assert!(super::parse_hex64(&format!("{}z", "a".repeat(63))).is_err()); + } + + // --- repo_coord_a_value --- + + #[test] + fn repo_coord_lowercases_the_owner_and_keeps_the_dtag() { + // `GitRepoCoord::to_a_tag_value` lowercases the owner and leaves the + // d-tag alone; a query has to agree byte for byte. + let owner = "ABCDEF0123456789".repeat(4); + let value = super::repo_coord_a_value(&owner, "My-Repo").unwrap(); + assert_eq!(value, format!("30617:{}:My-Repo", owner.to_lowercase())); + } + + #[test] + fn repo_coord_rejects_a_bad_owner_or_id() { + assert!(super::repo_coord_a_value("nope", "repo").is_err()); + assert!(super::repo_coord_a_value(&"a".repeat(64), "").is_err()); + assert!(super::repo_coord_a_value(&"a".repeat(64), "..").is_err()); + } + // --- validate_content_size --- #[test]