From f6ec66b96ec96f9df8a164feb99f5200cf9b33be Mon Sep 17 00:00:00 2001 From: taeha Date: Thu, 23 Jul 2026 10:53:42 +0900 Subject: [PATCH] fix(relay): stop collapsing multi-value #h filters to a single channel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit extract_channel_id_from_filter returned the first parseable #h value even when a filter listed several distinct channels. Filter tag values live in a BTreeSet, so /query and the COUNT fallback silently pinned the SQL query to the lexicographically-first channel UUID and dropped every other listed channel's events — the desktop Workflows overview rendered empty because get_channels_workflows batches all member channels into one #h filter. Return None for multi-#h filters instead, mirroring the multi-filter rule in extract_channel_id_from_filters: the access-scope path then widens the SQL query to the caller's accessible channels and the filters_match post-filter enforces the per-event #h OR. Fixes #2385 Signed-off-by: taeha --- crates/buzz-relay/src/handlers/req.rs | 63 +++++++++++++- crates/buzz-test-client/tests/e2e_relay.rs | 97 ++++++++++++++++++++++ 2 files changed, 158 insertions(+), 2 deletions(-) diff --git a/crates/buzz-relay/src/handlers/req.rs b/crates/buzz-relay/src/handlers/req.rs index ecc3ba6777..bb1354da68 100644 --- a/crates/buzz-relay/src/handlers/req.rs +++ b/crates/buzz-relay/src/handlers/req.rs @@ -839,18 +839,32 @@ fn filters_are_nip43_membership_only(filters: &[Filter]) -> bool { } /// Extract a channel UUID from a single filter's `#h` tag. +/// +/// Returns `None` when the filter carries multiple distinct parseable channel +/// UUIDs — NIP-01 `#h` values are OR'd, so pinning the query to any single one +/// would silently drop the other channels' events. Callers then fall back to +/// the access-scope path (`apply_access_scope_to_query`) and the in-memory +/// `filters_match` post-filter enforces the per-event `#h` OR. Mirrors the +/// multi-filter rule in [`extract_channel_id_from_filters`]. fn extract_channel_id_from_filter(filter: &Filter) -> Option { + let mut found_id: Option = None; for (tag_key, tag_values) in filter.generic_tags.iter() { let key = tag_key.to_string(); if key == "h" { for val in tag_values { if let Ok(id) = val.parse::() { - return Some(id); + match found_id { + Some(existing) if existing != id => { + // Multiple distinct channel IDs — not single-channel. + return None; + } + _ => found_id = Some(id), + } } } } } - None + found_id } /// Convert a single NIP-01 filter into an [`EventQuery`] for the database. @@ -1234,6 +1248,51 @@ mod tests { use super::*; use nostr::{Alphabet, Filter, SingleLetterTag}; + fn filter_with_h_values(values: &[&str]) -> Filter { + Filter::new().custom_tags( + SingleLetterTag::lowercase(Alphabet::H), + values.iter().map(|v| v.to_string()), + ) + } + + #[test] + fn single_h_value_extracts_channel_id() { + let channel = uuid::Uuid::new_v4(); + let filter = filter_with_h_values(&[&channel.to_string()]); + assert_eq!(extract_channel_id_from_filter(&filter), Some(channel)); + } + + /// Regression test for #2385: a filter listing multiple distinct channels + /// must not be pinned to any single one of them. Before the fix the + /// extractor returned the lexicographically-first value from the BTreeSet, + /// so `/query` and COUNT silently dropped every other listed channel. + #[test] + fn multiple_distinct_h_values_do_not_pin_a_channel() { + let a = uuid::Uuid::new_v4(); + let b = uuid::Uuid::new_v4(); + let filter = filter_with_h_values(&[&a.to_string(), &b.to_string()]); + assert_eq!(extract_channel_id_from_filter(&filter), None); + } + + #[test] + fn duplicate_h_values_for_same_channel_still_pin() { + let channel = uuid::Uuid::new_v4(); + // Same UUID in two spellings — parses to one channel, safe to pin. + let upper = channel.to_string().to_uppercase(); + let filter = filter_with_h_values(&[&channel.to_string(), &upper]); + assert_eq!(extract_channel_id_from_filter(&filter), Some(channel)); + } + + #[test] + fn unparseable_h_values_are_ignored_for_pinning() { + let channel = uuid::Uuid::new_v4(); + let filter = filter_with_h_values(&["not-a-uuid", &channel.to_string()]); + assert_eq!(extract_channel_id_from_filter(&filter), Some(channel)); + + let no_uuid = filter_with_h_values(&["not-a-uuid"]); + assert_eq!(extract_channel_id_from_filter(&no_uuid), None); + } + #[test] fn global_queries_push_access_scope_before_limit() { let accessible = vec![uuid::Uuid::new_v4(), uuid::Uuid::new_v4()]; diff --git a/crates/buzz-test-client/tests/e2e_relay.rs b/crates/buzz-test-client/tests/e2e_relay.rs index 7a7f7e19fa..fa829997f4 100644 --- a/crates/buzz-test-client/tests/e2e_relay.rs +++ b/crates/buzz-test-client/tests/e2e_relay.rs @@ -1118,6 +1118,103 @@ async fn test_kind0_nip05_sync() { client.disconnect().await.expect("disconnect"); } +/// Regression test for #2385: a `/query` filter listing multiple `#h` channels +/// must return events from every listed accessible channel, not just the +/// lexicographically-first one. Also covers the `/count` fallback path, which +/// shared the same single-channel pinning. +#[tokio::test] +#[ignore] +async fn test_query_multi_h_filter_returns_all_listed_channels() { + let url = relay_url(); + let http = relay_http_url(); + let keys = Keys::generate(); + let pubkey_hex = keys.public_key().to_hex(); + + let channel_a = create_test_channel(&keys).await; + let channel_b = create_test_channel(&keys).await; + + let mut client = BuzzTestClient::connect(&url, &keys).await.expect("connect"); + + let content_a = format!("multi-h A {}", uuid::Uuid::new_v4()); + let content_b = format!("multi-h B {}", uuid::Uuid::new_v4()); + let ok_a = client + .send_text_message(&keys, &channel_a, &content_a, 9) + .await + .expect("send to channel A"); + assert!( + ok_a.accepted, + "channel A message rejected: {}", + ok_a.message + ); + let ok_b = client + .send_text_message(&keys, &channel_b, &content_b, 9) + .await + .expect("send to channel B"); + assert!( + ok_b.accepted, + "channel B message rejected: {}", + ok_b.message + ); + + tokio::time::sleep(Duration::from_millis(300)).await; + + let http_client = reqwest::Client::new(); + let filters = serde_json::json!([{ + "kinds": [9], + "#h": [&channel_a, &channel_b], + }]); + let body = serde_json::to_string(&filters).unwrap(); + + let query_resp = http_client + .post(format!("{}/query", http)) + .header("X-Pubkey", &pubkey_hex) + .header("Content-Type", "application/json") + .body(body.clone()) + .send() + .await + .expect("multi-#h query"); + assert!( + query_resp.status().is_success(), + "multi-#h query failed: {}", + query_resp.status() + ); + let events: Vec = query_resp.json().await.expect("query json"); + let contents: Vec<&str> = events + .iter() + .filter_map(|e| e["content"].as_str()) + .collect(); + assert!( + contents.contains(&content_a.as_str()), + "channel A event missing from multi-#h query result: {contents:?}" + ); + assert!( + contents.contains(&content_b.as_str()), + "channel B event missing from multi-#h query result: {contents:?}" + ); + + let count_resp = http_client + .post(format!("{}/count", http)) + .header("X-Pubkey", &pubkey_hex) + .header("Content-Type", "application/json") + .body(body) + .send() + .await + .expect("multi-#h count"); + assert!( + count_resp.status().is_success(), + "multi-#h count failed: {}", + count_resp.status() + ); + let count_body: serde_json::Value = count_resp.json().await.expect("count json"); + let count = count_body["count"].as_u64().unwrap_or(0); + assert!( + count >= 2, + "multi-#h count should include both channels' events, got {count}" + ); + + client.disconnect().await.expect("disconnect"); +} + /// NIP-29 kind 9000 (PUT_USER): default policy ("anyone") allows a third party to add an agent. #[tokio::test] #[ignore]