mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
spike: enforce channel_scope by construction on SearchQuery
SearchQuery::new now requires non-empty channel_ids, returning
SearchError::EmptyChannelScope otherwise. Fields are pub(crate) so
struct-literal construction outside the crate is impossible; optional
facets use #[must_use] builder methods (with_kinds/authors/since/
until/page/per_page).
Closes the type-system gap on Eva's gate-1 "no visibility widening"
invariant: the access boundary is now enforced at construction, not
just at the call sites. Both call sites (req.rs, bridge.rs) wrap
SearchQuery::new in a match — req.rs logs + breaks pagination on the
Err path, bridge.rs continues the filter loop. Upstream guards
(build_search_channel_scope_filter + the per-filter h_tag validity
check) keep the Err path unreachable in normal operation, but if a
future refactor lets an empty scope through, behavior is "no results"
not "widened search".
Also adds the missing `info!("Search backend: typesense", ...)` log
line for symmetry with the postgres/disabled branches — small
operational polish, no behavior change.
Tests: buzz-search 30/30 (+1 rejection test), buzz-relay lib 337/337,
NIP-50 e2e 5/5 on both Postgres and Typesense backends (4 NIP-50 +
test_ws_search_isolation_other_user_cannot_find_reminder).
Co-authored-by: Tyler <109685178+tlongwell-block@users.noreply.github.com>
Signed-off-by: Tyler <109685178+tlongwell-block@users.noreply.github.com>
This commit is contained in:
co-authored by
Tyler
parent
4d8fe9a5e0
commit
a3fbd0288f
@@ -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
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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}");
|
||||
|
||||
@@ -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,
|
||||
}
|
||||
|
||||
@@ -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());
|
||||
|
||||
|
||||
+137
-62
@@ -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<u16>,
|
||||
pub(crate) kinds: Vec<u16>,
|
||||
/// Event author pubkeys (hex). Empty = no restriction.
|
||||
pub authors: Vec<String>,
|
||||
/// 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<String>,
|
||||
pub(crate) authors: Vec<String>,
|
||||
/// 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<String>,
|
||||
/// Lower bound on `created_at` (Unix seconds, inclusive).
|
||||
pub since: Option<i64>,
|
||||
pub(crate) since: Option<i64>,
|
||||
/// Upper bound on `created_at` (Unix seconds, inclusive).
|
||||
pub until: Option<i64>,
|
||||
pub(crate) until: Option<i64>,
|
||||
/// 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<String>, channel_ids: Vec<String>) -> Result<Self, SearchError> {
|
||||
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<u16>) -> Self {
|
||||
self.kinds = kinds;
|
||||
self
|
||||
}
|
||||
|
||||
/// Restrict to the given author pubkeys (hex).
|
||||
#[must_use]
|
||||
pub fn with_authors(mut self, authors: Vec<String>) -> 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<i64>) -> 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<i64>) -> 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<String> {
|
||||
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<String> {
|
||||
@@ -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<String> {
|
||||
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]
|
||||
|
||||
Reference in New Issue
Block a user