diff --git a/crates/buzz-relay/src/api/bridge.rs b/crates/buzz-relay/src/api/bridge.rs index 788ff7acc..c97d95768 100644 --- a/crates/buzz-relay/src/api/bridge.rs +++ b/crates/buzz-relay/src/api/bridge.rs @@ -751,15 +751,23 @@ async fn handle_bridge_search( let since_secs = filter.since.map(|t| t.as_secs() as i64); let until_secs = filter.until.map(|t| t.as_secs() as i64); - let search_query = buzz_search::SearchQuery { - q: search_text, - kinds: kinds_vec, - authors: authors_vec, - channel_ids: filter_channel_scope, - since: since_secs, - until: until_secs, - page: 1, - per_page: limit, + let search_query = match buzz_search::SearchQuery::new(search_text, filter_channel_scope) { + Ok(q) => q + .with_kinds(kinds_vec) + .with_authors(authors_vec) + .with_since(since_secs) + .with_until(until_secs) + .with_page(1) + .with_per_page(limit), + Err(e) => { + // Upstream guards (the per-filter h_tag validity check + // immediately above + the outer accessible_channels gate) + // make this unreachable in normal operation. If a future + // refactor ever lets an empty scope through, fail closed: + // log and skip this filter instead of widening visibility. + tracing::warn!("bridge search rejected empty channel scope: {e}"); + continue; + } }; let search_result = state diff --git a/crates/buzz-relay/src/handlers/req.rs b/crates/buzz-relay/src/handlers/req.rs index 5a1854147..496c1b229 100644 --- a/crates/buzz-relay/src/handlers/req.rs +++ b/crates/buzz-relay/src/handlers/req.rs @@ -384,16 +384,26 @@ async fn handle_search_req( break; } - let search_query = buzz_search::SearchQuery { - q: search_text.clone(), - kinds: kinds_vec.clone(), - authors: authors_vec.clone(), - channel_ids: channel_scope.clone(), - since: since_secs, - until: until_secs, - page, - per_page, - }; + let search_query = + match buzz_search::SearchQuery::new(search_text.clone(), channel_scope.clone()) { + Ok(q) => q + .with_kinds(kinds_vec.clone()) + .with_authors(authors_vec.clone()) + .with_since(since_secs) + .with_until(until_secs) + .with_page(page) + .with_per_page(per_page), + Err(e) => { + // Upstream guards (build_search_channel_scope_filter + + // the per-filter h_tag validity check) make this + // unreachable in normal operation. If a future refactor + // ever lets an empty scope through, fail closed: log + // and stop paginating this filter — no results, never + // a widened search. + warn!(sub_id = %sub_id, "NIP-50 search rejected empty channel scope: {e}"); + break; + } + }; let search_result = match state.search.search(&search_query).await { Ok(r) => r, diff --git a/crates/buzz-relay/src/main.rs b/crates/buzz-relay/src/main.rs index bfeea5f2b..5638fc4a5 100644 --- a/crates/buzz-relay/src/main.rs +++ b/crates/buzz-relay/src/main.rs @@ -197,6 +197,10 @@ async fn main() -> anyhow::Result<()> { collection: std::env::var("TYPESENSE_COLLECTION") .unwrap_or_else(|_| "events".to_string()), }; + info!( + collection = %search_config.collection, + "Search backend: typesense" + ); let service = SearchService::new(search_config); if let Err(e) = service.ensure_collection().await { error!("Typesense collection setup failed (non-fatal): {e}"); diff --git a/crates/buzz-search/src/error.rs b/crates/buzz-search/src/error.rs index fd02f0958..1702d654e 100644 --- a/crates/buzz-search/src/error.rs +++ b/crates/buzz-search/src/error.rs @@ -40,4 +40,14 @@ pub enum SearchError { /// A database error from sqlx (Postgres backend only). #[error("Database error: {0}")] Database(#[from] sqlx::Error), + + /// `SearchQuery::new` was called with an empty `channel_ids` set. + /// + /// `channel_ids` is the access-control boundary: every search must be + /// scoped to an explicit, non-empty set of channel UUIDs (or the + /// [`crate::query::GLOBAL_CHANNEL_SENTINEL`]). Empty means "no scope," + /// which would widen visibility — refused at construction time so the + /// invariant holds by type, not by remembering to filter at the call site. + #[error("SearchQuery requires a non-empty channel_ids scope")] + EmptyChannelScope, } diff --git a/crates/buzz-search/src/lib.rs b/crates/buzz-search/src/lib.rs index f52e404bb..22d31647b 100644 --- a/crates/buzz-search/src/lib.rs +++ b/crates/buzz-search/src/lib.rs @@ -372,10 +372,13 @@ mod integration_tests { tokio::time::sleep(std::time::Duration::from_millis(500)).await; let result = service - .search(&SearchQuery { - q: unique_token.clone(), - ..Default::default() - }) + .search( + &SearchQuery::new( + unique_token.clone(), + vec![GLOBAL_CHANNEL_SENTINEL.to_string()], + ) + .expect("non-empty scope"), + ) .await .unwrap(); @@ -429,11 +432,11 @@ mod integration_tests { tokio::time::sleep(std::time::Duration::from_millis(500)).await; let result = service - .search(&SearchQuery { - q: unique.clone(), - kinds: vec![1], - ..Default::default() - }) + .search( + &SearchQuery::new(unique.clone(), vec![GLOBAL_CHANNEL_SENTINEL.to_string()]) + .expect("non-empty scope") + .with_kinds(vec![1]), + ) .await .unwrap(); @@ -447,7 +450,9 @@ mod integration_tests { #[tokio::test] async fn disabled_backend_returns_empty_results() { let service = SearchService::disabled(); - let result = service.search(&SearchQuery::default()).await.unwrap(); + let query = SearchQuery::new("*", vec![GLOBAL_CHANNEL_SENTINEL.to_string()]) + .expect("non-empty scope"); + let result = service.search(&query).await.unwrap(); assert_eq!(result.found, 0); assert!(result.hits.is_empty()); diff --git a/crates/buzz-search/src/query.rs b/crates/buzz-search/src/query.rs index 0981eaaea..808f9b93a 100644 --- a/crates/buzz-search/src/query.rs +++ b/crates/buzz-search/src/query.rs @@ -17,40 +17,102 @@ pub const GLOBAL_CHANNEL_SENTINEL: &str = "__global__"; /// structured fields into its own filter syntax (Typesense `filter_by` string, /// Postgres `WHERE` clause, …) so the call site doesn't have to know which /// backend is in use. +/// +/// # Access-control invariant +/// +/// `channel_ids` is required to be non-empty by construction: external code +/// can only build a `SearchQuery` through [`SearchQuery::new`], which rejects +/// an empty scope with [`SearchError::EmptyChannelScope`]. The fields are +/// `pub(crate)` so backends inside this crate can read them directly, but +/// struct-literal construction from outside the crate is impossible. This +/// keeps "search cannot widen visibility" true at the type level instead of +/// relying on every caller to remember to pass a channel filter. #[derive(Debug, Clone)] pub struct SearchQuery { /// The full-text query string. Empty string is treated as "match all". - pub q: String, + pub(crate) q: String, /// Nostr kinds to restrict to. Empty = no restriction. - pub kinds: Vec, + pub(crate) kinds: Vec, /// Event author pubkeys (hex). Empty = no restriction. - pub authors: Vec, - /// Channel UUID strings to restrict to. Empty = no restriction. The - /// [`GLOBAL_CHANNEL_SENTINEL`] value (`"__global__"`) selects events that - /// have no `channel_id` set. - pub channel_ids: Vec, + pub(crate) authors: Vec, + /// Channel UUID strings to restrict to. Never empty — the access-control + /// boundary. The [`GLOBAL_CHANNEL_SENTINEL`] value (`"__global__"`) + /// selects events that have no `channel_id` set. + pub(crate) channel_ids: Vec, /// Lower bound on `created_at` (Unix seconds, inclusive). - pub since: Option, + pub(crate) since: Option, /// Upper bound on `created_at` (Unix seconds, inclusive). - pub until: Option, + pub(crate) until: Option, /// Page number (1-indexed). - pub page: u32, + pub(crate) page: u32, /// Number of results per page. - pub per_page: u32, + pub(crate) per_page: u32, } -impl Default for SearchQuery { - fn default() -> Self { - Self { - q: "*".into(), +impl SearchQuery { + /// Build a `SearchQuery` with the required full-text term and channel + /// scope. Returns [`SearchError::EmptyChannelScope`] if `channel_ids` is + /// empty — see the type-level note on the access-control invariant. + /// + /// Optional facets (`kinds`, `authors`, `since`, `until`, `page`, + /// `per_page`) default to "no restriction" / page 1 of 20 results, and + /// can be set with the `with_*` builder methods. + pub fn new(q: impl Into, channel_ids: Vec) -> Result { + if channel_ids.is_empty() { + return Err(SearchError::EmptyChannelScope); + } + Ok(Self { + q: q.into(), kinds: Vec::new(), authors: Vec::new(), - channel_ids: Vec::new(), + channel_ids, since: None, until: None, page: 1, per_page: 20, - } + }) + } + + /// Restrict to the given Nostr kinds. + #[must_use] + pub fn with_kinds(mut self, kinds: Vec) -> Self { + self.kinds = kinds; + self + } + + /// Restrict to the given author pubkeys (hex). + #[must_use] + pub fn with_authors(mut self, authors: Vec) -> Self { + self.authors = authors; + self + } + + /// Set the lower bound on `created_at` (Unix seconds, inclusive). + #[must_use] + pub fn with_since(mut self, since: Option) -> Self { + self.since = since; + self + } + + /// Set the upper bound on `created_at` (Unix seconds, inclusive). + #[must_use] + pub fn with_until(mut self, until: Option) -> Self { + self.until = until; + self + } + + /// Set the 1-indexed page number. + #[must_use] + pub fn with_page(mut self, page: u32) -> Self { + self.page = page; + self + } + + /// Set the page size. + #[must_use] + pub fn with_per_page(mut self, per_page: u32) -> Self { + self.per_page = per_page; + self } } @@ -305,18 +367,31 @@ mod tests { use super::*; use serde_json::json; + /// A non-empty channel scope used by tests that don't otherwise care + /// about which channel is being searched — the constructor requires + /// non-empty `channel_ids`, so tests pick a canonical placeholder. + const TEST_CHANNEL: &str = "11111111-1111-1111-1111-111111111111"; + + fn test_scope() -> Vec { + vec![TEST_CHANNEL.to_string()] + } + + #[test] + fn test_search_query_rejects_empty_channel_scope() { + // The access-control invariant: empty channel_ids must be refused + // at construction time, not silently accepted and then patched up + // by remembering to pass a filter at the call site. + let err = SearchQuery::new("hello", Vec::new()).expect_err("must reject empty scope"); + assert!(matches!(err, SearchError::EmptyChannelScope)); + } + #[test] fn test_search_query_building() { - let q = SearchQuery { - q: "hello world".into(), - kinds: vec![1], - authors: Vec::new(), - channel_ids: Vec::new(), - since: None, - until: None, - page: 2, - per_page: 10, - }; + let q = SearchQuery::new("hello world", test_scope()) + .expect("non-empty scope") + .with_kinds(vec![1]) + .with_page(2) + .with_per_page(10); let params = q.to_query_params(); let get = |key: &str| -> Option { @@ -330,23 +405,18 @@ mod tests { assert_eq!(get("query_by").unwrap(), "content"); assert_eq!(get("page").unwrap(), "2"); assert_eq!(get("per_page").unwrap(), "10"); - assert_eq!(get("filter_by").unwrap(), "kind:=[1]"); + let filter = get("filter_by").unwrap(); + assert!(filter.contains(&format!("channel_id:=[{TEST_CHANNEL}]"))); + assert!(filter.contains("kind:=[1]")); // sort_by is no longer emitted — Typesense default = relevance. assert!(params.iter().all(|(k, _)| k != "sort_by")); } #[test] - fn test_search_query_no_optional_fields() { - let q = SearchQuery { - q: "*".into(), - kinds: Vec::new(), - authors: Vec::new(), - channel_ids: Vec::new(), - since: None, - until: None, - page: 1, - per_page: 20, - }; + fn test_search_query_minimum_filters() { + // Even with no optional facets, channel scope is always rendered — + // the access boundary follows the query through every backend path. + let q = SearchQuery::new("*", test_scope()).expect("non-empty scope"); let params = q.to_query_params(); let has_key = |key: &str| params.iter().any(|(k, _)| k == key); @@ -355,23 +425,31 @@ mod tests { assert!(has_key("query_by")); assert!(has_key("page")); assert!(has_key("per_page")); - assert!(!has_key("filter_by")); + assert!(has_key("filter_by")); assert!(!has_key("sort_by")); + + let get = |key: &str| -> Option { + params + .iter() + .find(|(k, _)| k == key) + .map(|(_, v)| v.clone()) + }; + assert_eq!( + get("filter_by").unwrap(), + format!("channel_id:=[{TEST_CHANNEL}]") + ); } #[test] fn test_typesense_filter_by_renders_structured_fields() { - let q = SearchQuery { - q: "hello".into(), - kinds: vec![1, 42], - authors: vec!["deadbeef".into()], - channel_ids: vec!["11111111-1111-1111-1111-111111111111".into()], - since: Some(1_700_000_000), - until: Some(1_700_000_100), - ..Default::default() - }; + let q = SearchQuery::new("hello", test_scope()) + .expect("non-empty scope") + .with_kinds(vec![1, 42]) + .with_authors(vec!["deadbeef".into()]) + .with_since(Some(1_700_000_000)) + .with_until(Some(1_700_000_100)); let filter = q.typesense_filter_by().expect("non-empty filter"); - assert!(filter.contains("channel_id:=[11111111-1111-1111-1111-111111111111]")); + assert!(filter.contains(&format!("channel_id:=[{TEST_CHANNEL}]"))); assert!(filter.contains("kind:=[1,42]")); assert!(filter.contains("pubkey:=[deadbeef]")); assert!(filter.contains("created_at:>=1700000000")); @@ -380,27 +458,24 @@ mod tests { #[test] fn test_typesense_filter_by_handles_global_sentinel() { - let with_global_only = SearchQuery { - q: "*".into(), - channel_ids: vec![GLOBAL_CHANNEL_SENTINEL.to_string()], - ..Default::default() - }; + let with_global_only = SearchQuery::new("*", vec![GLOBAL_CHANNEL_SENTINEL.to_string()]) + .expect("non-empty scope"); assert_eq!( with_global_only.typesense_filter_by().as_deref(), Some("channel_id:=__global__") ); - let with_mix = SearchQuery { - q: "*".into(), - channel_ids: vec![ - "11111111-1111-1111-1111-111111111111".into(), + let with_mix = SearchQuery::new( + "*", + vec![ + TEST_CHANNEL.to_string(), GLOBAL_CHANNEL_SENTINEL.to_string(), ], - ..Default::default() - }; + ) + .expect("non-empty scope"); let filter = with_mix.typesense_filter_by().expect("non-empty"); assert!(filter.contains("|| channel_id:=__global__")); - assert!(filter.contains("channel_id:=[11111111-1111-1111-1111-111111111111]")); + assert!(filter.contains(&format!("channel_id:=[{TEST_CHANNEL}]"))); } #[test]