From ed274bdc3c566dc01405d649387418f0405877b3 Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Sun, 12 Jul 2026 13:09:46 -0400 Subject: [PATCH] fix(relay): exclude author-only kinds from channel-window path The bridge top_level channel-window filter (handle_channel_window_filter) passed all window rows to the response without an author-only guard. Since next_cursor and has_more are computed at the DB layer before any in-memory filtering, a bridge-level skip would still expose draft ids via the 39006 bounds overlay and leave has_more counts inflated by draft rows. Fix: add an author_pubkey param to get_channel_window. When Some, the query appends AND (e.kind NOT IN (30300, 31234) OR e.pubkey = $N), excluding author-only kinds (KIND_DRAFT=31234, KIND_EVENT_REMINDER=30300) for rows whose pubkey does not match the requester. This keeps the cursor, has_more, and all overlay values computed against the already-restricted row set. Pass Some(&pubkey_bytes) from handle_channel_window_filter via the new pubkey_bytes parameter; internal / test callers pass None. Tests: two new e2e tests in e2e_nip37_draft.rs: - test_channel_window_draft_excluded_for_non_author: mixed kinds:[9,31234] query by a channel member who is not the author must return zero draft rows and zero draft ids anywhere in the response (rows, aux, overlays). kind:9 positive control row must still be present. - test_channel_window_draft_visible_to_author: the author sees their own kind:31234 draft in the window, consistent with all other author-only read paths. Closes the last unguarded author-only read surface identified in code review of PR #1757. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- crates/buzz-db/src/lib.rs | 2 + crates/buzz-db/src/thread.rs | 35 +++- crates/buzz-relay/src/api/bridge.rs | 3 + .../buzz-test-client/tests/e2e_nip37_draft.rs | 197 ++++++++++++++++++ 4 files changed, 232 insertions(+), 5 deletions(-) diff --git a/crates/buzz-db/src/lib.rs b/crates/buzz-db/src/lib.rs index de2efaa9f..be26efb02 100644 --- a/crates/buzz-db/src/lib.rs +++ b/crates/buzz-db/src/lib.rs @@ -1453,6 +1453,7 @@ impl Db { limit: u32, cursor: Option<(DateTime, Vec)>, kind_filter: Option<&[u32]>, + author_pubkey: Option<&[u8]>, ) -> Result { thread::get_channel_window( &self.pool, @@ -1461,6 +1462,7 @@ impl Db { limit, cursor, kind_filter, + author_pubkey, ) .await } diff --git a/crates/buzz-db/src/thread.rs b/crates/buzz-db/src/thread.rs index 3f92212dd..aae37987b 100644 --- a/crates/buzz-db/src/thread.rs +++ b/crates/buzz-db/src/thread.rs @@ -562,6 +562,12 @@ pub async fn get_thread_summary( /// predicates (deletion, top-level, kinds). The sentinel row is dropped here /// and never reaches the wire; callers must not re-derive exhaustion from row /// counts (`rows < limit` proves nothing on an exact-multiple final page). +/// +/// `author_pubkey`: when `Some`, author-only kinds (kind:31234 drafts, +/// kind:30300 reminders) are excluded for rows whose `pubkey` does not match. +/// Pass the authenticated requester's pubkey bytes on all bridge read paths. +/// `None` skips the filter (internal/test callers that don't need author-only +/// visibility control). pub async fn get_channel_window( pool: &PgPool, community_id: CommunityId, @@ -569,6 +575,7 @@ pub async fn get_channel_window( limit: u32, cursor: Option<(DateTime, Vec)>, kind_filter: Option<&[u32]>, + author_pubkey: Option<&[u8]>, ) -> Result { let mut param_idx = 3u32; // $1 is community_id, $2 is channel_id let mut sql = String::from( @@ -624,6 +631,21 @@ pub async fn get_channel_window( } } + // Author-only kinds (kind:31234 drafts, kind:30300 reminders) must never + // be returned for a requester who is not their author. The filter is + // applied at the SQL layer so that `has_more` and `next_cursor` are also + // computed over the already-restricted row set — a bridge-level skip would + // leave those values pointing at or counting draft rows for non-authors. + if author_pubkey.is_some() { + let pk_idx = param_idx; + // KIND_AUTHOR_ONLY list kept in sync with buzz_core::kind::AUTHOR_ONLY_KINDS + // (30300 = KIND_EVENT_REMINDER, 31234 = KIND_DRAFT). + sql.push_str(&format!( + " AND (e.kind NOT IN (30300, 31234) OR e.pubkey = ${pk_idx})" + )); + param_idx += 1; + } + sql.push_str(&format!( " ORDER BY e.created_at DESC, e.id ASC LIMIT ${param_idx}" )); @@ -634,6 +656,9 @@ pub async fn get_channel_window( if let Some((ts, id)) = &cursor { q = q.bind(*ts).bind(id.clone()); } + if let Some(pk) = author_pubkey { + q = q.bind(pk); + } // The +1 probe row is the server-internal has_more evidence. q = q.bind(limit as i64 + 1); @@ -1531,7 +1556,7 @@ mod tests { let quiet_reply = make_stream_event(&author, "quiet reply"); insert_reply(&pool, community, channel.id, &root, &quiet_reply, false).await; - let window = get_channel_window(&pool, community, channel.id, 50, None, None) + let window = get_channel_window(&pool, community, channel.id, 50, None, None, None) .await .expect("fetch window"); @@ -1596,7 +1621,7 @@ mod tests { let mut collected: Vec> = Vec::new(); let mut cursor: Option<(DateTime, Vec)> = None; loop { - let window = get_channel_window(&pool, community, channel.id, 2, cursor, None) + let window = get_channel_window(&pool, community, channel.id, 2, cursor, None, None) .await .expect("fetch window page"); for row in &window.rows { @@ -1646,14 +1671,14 @@ mod tests { insert_root(&pool, community, channel.id, &event).await; } - let page1 = get_channel_window(&pool, community, channel.id, 2, None, None) + let page1 = get_channel_window(&pool, community, channel.id, 2, None, None, None) .await .expect("fetch page 1"); assert_eq!(page1.rows.len(), 2); assert!(page1.has_more, "two more rows exist past page 1"); let cursor = page1.next_cursor.expect("has_more implies next_cursor"); - let page2 = get_channel_window(&pool, community, channel.id, 2, Some(cursor), None) + let page2 = get_channel_window(&pool, community, channel.id, 2, Some(cursor), None, None) .await .expect("fetch page 2"); assert_eq!(page2.rows.len(), 2, "final page is exactly full"); @@ -1695,7 +1720,7 @@ mod tests { insert_reply(&pool, community, channel.id, &discussed, &reply, false).await; } - let window = get_channel_window(&pool, community, channel.id, 50, None, None) + let window = get_channel_window(&pool, community, channel.id, 50, None, None, None) .await .expect("fetch window"); diff --git a/crates/buzz-relay/src/api/bridge.rs b/crates/buzz-relay/src/api/bridge.rs index 58cb2d09e..f20e51a00 100644 --- a/crates/buzz-relay/src/api/bridge.rs +++ b/crates/buzz-relay/src/api/bridge.rs @@ -374,6 +374,7 @@ async fn handle_channel_window_filter( filter: &nostr::Filter, accessible_channels: &[uuid::Uuid], events: &mut Vec, + pubkey_bytes: &[u8], ) -> Result<(), (StatusCode, Json)> { use buzz_core::kind::{KIND_THREAD_SUMMARY, KIND_WINDOW_BOUNDS}; @@ -436,6 +437,7 @@ async fn handle_channel_window_filter( limit, cursor.clone(), kind_filter.as_deref(), + Some(pubkey_bytes), ) .await .map_err(|e| internal_error(&format!("channel window error: {e}")))?; @@ -768,6 +770,7 @@ pub async fn query_events( filter, &accessible_channels, &mut events, + &pubkey_bytes, ) .await?; handled.insert(idx); diff --git a/crates/buzz-test-client/tests/e2e_nip37_draft.rs b/crates/buzz-test-client/tests/e2e_nip37_draft.rs index c62db7bfc..6cfa45b34 100644 --- a/crates/buzz-test-client/tests/e2e_nip37_draft.rs +++ b/crates/buzz-test-client/tests/e2e_nip37_draft.rs @@ -2382,3 +2382,200 @@ async fn test_removed_member_cannot_read_drafts_after_removal() { } removed_client.disconnect().await.expect("disconnect"); } + +// ─── Channel-window (top_level) draft privacy ──────────────────────────────── + +/// POST /query with `top_level: true`, mixed `kinds:[9,31234]`, as `as_keys`. +/// Returns the raw event array (rows + overlays + aux). +async fn query_channel_window_mixed( + as_keys: &Keys, + channel_id: &str, + include_aux: bool, + include_summaries: bool, +) -> Vec { + let client = http_client(); + let filter = serde_json::json!({ + "kinds": [9, KIND_DRAFT], + "#h": [channel_id], + "top_level": true, + "include_aux": include_aux, + "include_summaries": include_summaries, + }); + let resp = client + .post(format!("{}/query", relay_http_url())) + .header("X-Pubkey", &as_keys.public_key().to_hex()) + .header("Content-Type", "application/json") + .body(serde_json::to_string(&serde_json::json!([filter])).unwrap()) + .send() + .await + .expect("channel window mixed query"); + assert!( + resp.status().is_success(), + "window query failed: {}", + resp.status() + ); + resp.json::>() + .await + .expect("parse window response") +} + +/// `top_level: true` channel-window path with mixed `kinds:[9,31234]`: +/// a non-author channel member must receive zero kind:31234 draft rows and +/// zero draft event-ids anywhere in the response (rows, aux, overlays). +/// +/// Contract: the channel-window path applies the same author-only visibility +/// rule as every other read path — kind:31234 drafts are excluded for any +/// requester who is not their author. The SQL-level guard ensures that +/// `next_cursor` and `has_more` are also computed from the draft-excluded row +/// set, so no draft id can leak via the 39006 bounds overlay or aux closure. +#[tokio::test] +#[ignore] +async fn test_channel_window_draft_excluded_for_non_author() { + let client = http_client(); + let author = Keys::generate(); + let attacker = Keys::generate(); + let ch_id = create_open_channel(&author).await; + + // Attacker joins the channel so they can issue a valid top_level query. + add_member_http(&client, &author, &ch_id, &attacker).await; + + // Author posts a visible kind:9 message — provides a positive control row. + let msg = EventBuilder::new(nostr::Kind::Custom(9), "hello channel") + .tags([nostr::Tag::parse(["h", &ch_id]).unwrap()]) + .sign_with_keys(&author) + .unwrap(); + let msg_id = msg.id.to_hex(); + let (ok_m, reason_m) = submit_event_http(&client, &author, &msg).await; + assert!(ok_m, "kind:9 message must be accepted: {reason_m}"); + + // Author posts a kind:31234 draft in the same channel. + let d = uuid::Uuid::new_v4().to_string(); + let draft = build_draft(&author, &d, "9", &ch_id, &fake_nip44_v2()); + let draft_id = draft.id.to_hex(); + let (ok_d, reason_d) = submit_event_http(&client, &author, &draft).await; + assert!(ok_d, "draft must be accepted: {reason_d}"); + + // Attacker issues a top_level window query with kinds:[9,31234] — must + // not receive the draft in rows, aux, or overlays. + let events = query_channel_window_mixed(&attacker, &ch_id, true, true).await; + + // Collect all event-ids that appear anywhere in the response (own id + + // any id referenced in tags — covers bounds `d` tag, summary `e`/`d` tags). + let all_ids: Vec = events + .iter() + .flat_map(|e| { + let own_id = e["id"].as_str().map(|s| s.to_string()); + let tag_values: Vec = e["tags"] + .as_array() + .cloned() + .unwrap_or_default() + .into_iter() + .flat_map(|t| { + t.as_array() + .cloned() + .unwrap_or_default() + .into_iter() + .filter_map(|v| v.as_str().map(|s| s.to_string())) + }) + .collect(); + own_id.into_iter().chain(tag_values) + }) + .collect(); + + assert!( + !all_ids.iter().any(|id| id == &draft_id), + "draft id must not appear anywhere in the channel-window response for a non-author: \ + draft_id={draft_id}, response={events:?}" + ); + + // Positive control: the kind:9 message MUST be present as a row. + let row_kinds: Vec = events.iter().filter_map(|e| e["kind"].as_u64()).collect(); + assert!( + row_kinds.contains(&9), + "kind:9 row must appear in window response for attacker: {events:?}" + ); + // msg_id must specifically be in the rows. + let ids_in_response: Vec = events + .iter() + .filter_map(|e| e["id"].as_str().map(|s| s.to_string())) + .collect(); + assert!( + ids_in_response.contains(&msg_id), + "kind:9 message id must appear in window rows: msg_id={msg_id}, response={events:?}" + ); + + assert!( + !row_kinds.contains(&(KIND_DRAFT as u64)), + "no kind:31234 event may appear in window response for non-author: {events:?}" + ); + + // Exactly one 39006 bounds overlay must be present (window invariant). + let bounds_count = events + .iter() + .filter(|e| e["kind"].as_u64() == Some(39006)) + .count(); + assert_eq!( + bounds_count, 1, + "exactly one 39006 bounds overlay required: {events:?}" + ); +} + +/// `top_level: true` channel-window with kinds:[9,31234]: the author themselves +/// CAN see their own draft in the window — consistent with all other read paths +/// which allow authors to retrieve their own author-only events. +#[tokio::test] +#[ignore] +async fn test_channel_window_draft_visible_to_author() { + let client = http_client(); + let author = Keys::generate(); + let ch_id = create_open_channel(&author).await; + + // Post a kind:9 message and a draft in the same channel. + let msg = EventBuilder::new(nostr::Kind::Custom(9), "public message") + .tags([nostr::Tag::parse(["h", &ch_id]).unwrap()]) + .sign_with_keys(&author) + .unwrap(); + let (ok_m, reason_m) = submit_event_http(&client, &author, &msg).await; + assert!(ok_m, "kind:9 message must be accepted: {reason_m}"); + + let d = uuid::Uuid::new_v4().to_string(); + let draft = build_draft(&author, &d, "9", &ch_id, &fake_nip44_v2()); + let draft_id = draft.id.to_hex(); + let (ok_d, reason_d) = submit_event_http(&client, &author, &draft).await; + assert!(ok_d, "draft must be accepted: {reason_d}"); + + // Author queries the window — their own draft must appear. + let events = query_channel_window_mixed(&author, &ch_id, false, false).await; + + let row_kinds: Vec = events.iter().filter_map(|e| e["kind"].as_u64()).collect(); + + // Both kind:9 and kind:31234 (own draft) must be present. + assert!( + row_kinds.contains(&9), + "kind:9 row must appear for author: {events:?}" + ); + assert!( + row_kinds.contains(&(KIND_DRAFT as u64)), + "author must see their own kind:31234 draft in the window: {events:?}" + ); + + // Verify the specific draft id is present. + let ids_in_response: Vec = events + .iter() + .filter_map(|e| e["id"].as_str().map(|s| s.to_string())) + .collect(); + assert!( + ids_in_response.contains(&draft_id), + "author's draft id must appear in window rows: draft_id={draft_id}, response={events:?}" + ); + + // 39006 bounds overlay invariant. + let bounds_count = events + .iter() + .filter(|e| e["kind"].as_u64() == Some(39006)) + .count(); + assert_eq!( + bounds_count, 1, + "exactly one 39006 bounds overlay required: {events:?}" + ); +}