mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
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 <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
co-authored by
Will Pfleger
parent
1e82a573d7
commit
ed274bdc3c
@@ -1453,6 +1453,7 @@ impl Db {
|
||||
limit: u32,
|
||||
cursor: Option<(DateTime<Utc>, Vec<u8>)>,
|
||||
kind_filter: Option<&[u32]>,
|
||||
author_pubkey: Option<&[u8]>,
|
||||
) -> Result<thread::ChannelWindow> {
|
||||
thread::get_channel_window(
|
||||
&self.pool,
|
||||
@@ -1461,6 +1462,7 @@ impl Db {
|
||||
limit,
|
||||
cursor,
|
||||
kind_filter,
|
||||
author_pubkey,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
@@ -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<Utc>, Vec<u8>)>,
|
||||
kind_filter: Option<&[u32]>,
|
||||
author_pubkey: Option<&[u8]>,
|
||||
) -> Result<ChannelWindow> {
|
||||
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<u8>> = Vec::new();
|
||||
let mut cursor: Option<(DateTime<Utc>, Vec<u8>)> = 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");
|
||||
|
||||
|
||||
@@ -374,6 +374,7 @@ async fn handle_channel_window_filter(
|
||||
filter: &nostr::Filter,
|
||||
accessible_channels: &[uuid::Uuid],
|
||||
events: &mut Vec<Value>,
|
||||
pubkey_bytes: &[u8],
|
||||
) -> Result<(), (StatusCode, Json<Value>)> {
|
||||
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);
|
||||
|
||||
@@ -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<Value> {
|
||||
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::<Vec<Value>>()
|
||||
.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<String> = events
|
||||
.iter()
|
||||
.flat_map(|e| {
|
||||
let own_id = e["id"].as_str().map(|s| s.to_string());
|
||||
let tag_values: Vec<String> = 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<u64> = 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<String> = 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<u64> = 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<String> = 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:?}"
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user