mirror of
https://github.com/block/buzz.git
synced 2026-10-27 04:30:52 +00:00
fix(relay): scope push matches with the read path's channel rule (#8507)
🤖 ## Summary This is a consistency and forward-compatibility fix. It does not change what users of current clients see. Mobile push notifications now decide which channels can notify a device with the same rule that reads use, with one exception that the protocol requires: system channels never push. Before this change, push and reads used two separate rules: - **Reads** (subscriptions, counts, search and the bridge) show only the original channel types (stream, forum, DM, workflow) unless the filter names the channel. - **Push** skipped only `system` channels. Today `system` is the only non-original type, so the two rules give the same result. But a channel type added later would be hidden from reads and would still send push notifications to every member. This PR removes that gap: push now calls the same function that reads use, so a future type gets the correct push behavior with no extra work. ## What changes - A channel of an original type pushes as before. Membership is still required. - A system channel never pushes, even when a push filter names it. NIP-CL says: "The relay MUST NOT send push notifications for system channels." This is the same as before. - A channel of a type added later pushes only to a push filter that names that channel with `#h` (the channel tag), as reads do. - A push filter that names one channel does not get pushes from other named-only channels. - Events that are not in a channel (for example gift wraps) match as before. Push filters cannot carry the `#t` or `#P` opt-in that reads accept, so for push, naming the channel means `#h` only. ## Impact on current clients: none - System channels still never push, and no current channel type is affected by the later-type rule. - The mobile app names channels by `#h` only for joined, unarchived DM channels (`mobile/lib/shared/push/push_subscription_provider.dart`). DMs are an original type. - Naming a channel cannot get around membership. A push filter that names a channel still gets nothing unless its owner is a member of that channel. - Database cost is unchanged: one per-batch query replaces another, on the same channel list, with no extra round trip. - The push docs (NIP-PL and the push suppression doc) do not mention channel types, so they need no update. ## Details - The push matcher loads the named-only channels in each batch and checks each lease filter with `filter_channel_scope`, the function REQ, COUNT, search and the bridge already use. Before that check, it rejects system channels outright. - This removes `system_channel_ids_among`, which push was the last caller of. ### Related issue Part of the system channels spec (§2, rule 5). Builds on #8181 and #8469. No duplicate PR found. ### Testing - A `match_job` unit test covers: unnamed filters on an original-type channel, on a channel-less event, on a system channel and on a later-type channel; a later-type channel named by `#h` (1 wake); a system channel named by `#h` (0 wakes); an unrelated `#h` (no widening); and non-members. Removing either the shared scope check or the system exception fails it. - Before this PR, no test covered the push channel filter, not even for `system`. --------- Signed-off-by: Logan Johnson <loganj@squareup.com> Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
This commit is contained in:
@@ -1321,31 +1321,6 @@ pub async fn channel_type_of(
|
|||||||
.await?)
|
.await?)
|
||||||
}
|
}
|
||||||
|
|
||||||
/// The IDs in `channel_ids` that name system channels, deleted or not.
|
|
||||||
/// Push matching uses it to skip them.
|
|
||||||
pub async fn system_channel_ids_among(
|
|
||||||
pool: &PgPool,
|
|
||||||
community_id: CommunityId,
|
|
||||||
channel_ids: &[Uuid],
|
|
||||||
) -> Result<Vec<Uuid>> {
|
|
||||||
if channel_ids.is_empty() {
|
|
||||||
return Ok(Vec::new());
|
|
||||||
}
|
|
||||||
let mut connection = crate::observability::acquire_writer(
|
|
||||||
pool,
|
|
||||||
crate::observability::WriterOperation::Authorization,
|
|
||||||
)
|
|
||||||
.await?;
|
|
||||||
Ok(sqlx::query_scalar(
|
|
||||||
"SELECT id FROM channels \
|
|
||||||
WHERE community_id = $1 AND id = ANY($2) AND channel_type = 'system'",
|
|
||||||
)
|
|
||||||
.bind(community_id.as_uuid())
|
|
||||||
.bind(channel_ids)
|
|
||||||
.fetch_all(&mut *connection)
|
|
||||||
.await?)
|
|
||||||
}
|
|
||||||
|
|
||||||
/// The channels in `channel_ids` whose type is not a legacy type
|
/// The channels in `channel_ids` whose type is not a legacy type
|
||||||
/// ([`buzz_core::channel::LEGACY_CHANNEL_TYPES`]), deleted or not, each with
|
/// ([`buzz_core::channel::LEGACY_CHANNEL_TYPES`]), deleted or not, each with
|
||||||
/// its type: system channels and any type added later. A filter reads them
|
/// its type: system channels and any type added later. A filter reads them
|
||||||
@@ -2029,16 +2004,6 @@ impl Db {
|
|||||||
channel_type_of(&self.pool, community_id, channel_id).await
|
channel_type_of(&self.pool, community_id, channel_id).await
|
||||||
}
|
}
|
||||||
|
|
||||||
/// The IDs in `channel_ids` that name system channels, deleted or not.
|
|
||||||
#[datastore_span(name = "system_channel_ids_among", system = "postgresql")]
|
|
||||||
pub async fn system_channel_ids_among(
|
|
||||||
&self,
|
|
||||||
community_id: CommunityId,
|
|
||||||
channel_ids: &[Uuid],
|
|
||||||
) -> Result<Vec<Uuid>> {
|
|
||||||
system_channel_ids_among(&self.pool, community_id, channel_ids).await
|
|
||||||
}
|
|
||||||
|
|
||||||
/// The channels in `channel_ids` whose type is not a legacy type, deleted
|
/// The channels in `channel_ids` whose type is not a legacy type, deleted
|
||||||
/// or not, each with its type.
|
/// or not, each with its type.
|
||||||
#[datastore_span(name = "named_only_channels_among", system = "postgresql")]
|
#[datastore_span(name = "named_only_channels_among", system = "postgresql")]
|
||||||
|
|||||||
@@ -67,6 +67,11 @@ impl NamedOnlyChannels {
|
|||||||
&self.ids
|
&self.ids
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Whether `id` is a system channel.
|
||||||
|
pub(crate) fn is_system(&self, id: &Uuid) -> bool {
|
||||||
|
self.system.contains(id)
|
||||||
|
}
|
||||||
|
|
||||||
pub(crate) fn is_empty(&self) -> bool {
|
pub(crate) fn is_empty(&self) -> bool {
|
||||||
self.ids.is_empty()
|
self.ids.is_empty()
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -11,7 +11,14 @@ use nostr::Filter;
|
|||||||
use serde::{Deserialize, Serialize};
|
use serde::{Deserialize, Serialize};
|
||||||
use tracing::{debug, error, warn};
|
use tracing::{debug, error, warn};
|
||||||
|
|
||||||
use crate::{handlers::push_lease::Subscription, nip98::nip98_header, state::AppState};
|
use crate::{
|
||||||
|
handlers::{
|
||||||
|
push_lease::Subscription,
|
||||||
|
system_channels::{filter_channel_scope, NamedOnlyChannels},
|
||||||
|
},
|
||||||
|
nip98::nip98_header,
|
||||||
|
state::AppState,
|
||||||
|
};
|
||||||
|
|
||||||
// Dropping a SQLx transaction rolls it back; committed claims remain leased
|
// Dropping a SQLx transaction rolls it back; committed claims remain leased
|
||||||
// until expiry. Dropping HTTP cannot recall requests already accepted by the
|
// until expiry. Dropping HTTP cannot recall requests already accepted by the
|
||||||
@@ -116,11 +123,13 @@ async fn matcher_loop(state: &AppState) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Per-batch state shared by every job: the community's active leases and
|
/// Per-batch state shared by every job: the community's active leases, the
|
||||||
/// the exact (channel, lease author) membership pairs the jobs can consult.
|
/// exact (channel, lease author) membership pairs the jobs can consult, and
|
||||||
|
/// the batch's named-only channels (system channels and any type added later).
|
||||||
struct MatchContext {
|
struct MatchContext {
|
||||||
leases: Vec<buzz_db::push::MatchLease>,
|
leases: Vec<buzz_db::push::MatchLease>,
|
||||||
memberships: std::collections::HashSet<(uuid::Uuid, Vec<u8>)>,
|
memberships: std::collections::HashSet<(uuid::Uuid, Vec<u8>)>,
|
||||||
|
named_only: NamedOnlyChannels,
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn load_match_context(
|
async fn load_match_context(
|
||||||
@@ -138,22 +147,22 @@ async fn load_match_context(
|
|||||||
let mut authors: Vec<Vec<u8>> = leases.iter().map(|lease| lease.author.clone()).collect();
|
let mut authors: Vec<Vec<u8>> = leases.iter().map(|lease| lease.author.clone()).collect();
|
||||||
authors.sort_unstable();
|
authors.sort_unstable();
|
||||||
authors.dedup();
|
authors.dedup();
|
||||||
// System channels never push: drop their membership pairs, so a channel
|
|
||||||
// event there matches no lease (channel events require membership).
|
|
||||||
let system = state
|
|
||||||
.db
|
|
||||||
.system_channel_ids_among(batch.community, &channels)
|
|
||||||
.await?;
|
|
||||||
let memberships = state
|
let memberships = state
|
||||||
.db
|
.db
|
||||||
.membership_pairs(batch.community, &channels, &authors)
|
.membership_pairs(batch.community, &channels, &authors)
|
||||||
.await?
|
.await?
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.filter(|(channel, _)| !system.contains(channel))
|
|
||||||
.collect();
|
.collect();
|
||||||
|
let named_only = NamedOnlyChannels::from_typed(
|
||||||
|
state
|
||||||
|
.db
|
||||||
|
.named_only_channels_among(batch.community, &channels)
|
||||||
|
.await?,
|
||||||
|
);
|
||||||
Ok(MatchContext {
|
Ok(MatchContext {
|
||||||
leases,
|
leases,
|
||||||
memberships,
|
memberships,
|
||||||
|
named_only,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -301,6 +310,7 @@ fn match_job(
|
|||||||
let filter: Filter =
|
let filter: Filter =
|
||||||
serde_json::from_value(serde_json::Value::Object(sub.filter.clone()))?;
|
serde_json::from_value(serde_json::Value::Object(sub.filter.clone()))?;
|
||||||
if !push_filter_authorized_for_event(&filter, &job.event.event, &author_hex)
|
if !push_filter_authorized_for_event(&filter, &job.event.event, &author_hex)
|
||||||
|
|| !push_filter_reads_channel(&filter, job.event.channel_id, &context.named_only)
|
||||||
|| !filters_match(std::slice::from_ref(&filter), &job.event)
|
|| !filters_match(std::slice::from_ref(&filter), &job.event)
|
||||||
{
|
{
|
||||||
continue;
|
continue;
|
||||||
@@ -346,6 +356,29 @@ fn match_job(
|
|||||||
Ok(wakes)
|
Ok(wakes)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Whether a lease filter may push an event in `channel`, by the same rule as
|
||||||
|
/// REQ: a legacy-type channel is in every filter's scope (membership is
|
||||||
|
/// checked separately), and a named-only channel is in scope only when the
|
||||||
|
/// filter names it (see [`crate::handlers::system_channels`]). Lease filters
|
||||||
|
/// cannot carry `#t` or `#P`, so for push "names it" means `#h`. The one
|
||||||
|
/// exception: NIP-CL forbids push for system channels, even when named.
|
||||||
|
fn push_filter_reads_channel(
|
||||||
|
filter: &Filter,
|
||||||
|
channel: Option<uuid::Uuid>,
|
||||||
|
named_only: &NamedOnlyChannels,
|
||||||
|
) -> bool {
|
||||||
|
let Some(channel) = channel else {
|
||||||
|
return true;
|
||||||
|
};
|
||||||
|
if !named_only.ids().contains(&channel) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
if named_only.is_system(&channel) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
filter_channel_scope(filter, &[], named_only).contains(&channel)
|
||||||
|
}
|
||||||
|
|
||||||
/// Match-time counterpart of REQ's filter-level `#p` authorization gate.
|
/// Match-time counterpart of REQ's filter-level `#p` authorization gate.
|
||||||
/// Kind 1059 is globally stored and leaks recipient activity through wake
|
/// Kind 1059 is globally stored and leaks recipient activity through wake
|
||||||
/// timing, so a lease may only match gift wraps addressed to its own author.
|
/// timing, so a lease may only match gift wraps addressed to its own author.
|
||||||
@@ -764,6 +797,90 @@ mod tests {
|
|||||||
));
|
));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn named_only_channel_pushes_only_to_a_filter_that_names_it() {
|
||||||
|
let member = nostr::Keys::generate();
|
||||||
|
let outsider = nostr::Keys::generate();
|
||||||
|
let sender = nostr::Keys::generate();
|
||||||
|
let legacy = uuid::Uuid::new_v4();
|
||||||
|
let system = uuid::Uuid::new_v4();
|
||||||
|
let future = uuid::Uuid::new_v4();
|
||||||
|
let author = |keys: &nostr::Keys| keys.public_key().to_bytes().to_vec();
|
||||||
|
let context = |keys: &nostr::Keys, filter: Value| MatchContext {
|
||||||
|
leases: vec![buzz_db::push::MatchLease {
|
||||||
|
author: author(keys),
|
||||||
|
installation_id: "phone".into(),
|
||||||
|
generation: 1,
|
||||||
|
subscriptions: serde_json::json!([{"filter": filter, "class": "default"}]),
|
||||||
|
expires_at: Utc::now().timestamp() + 3600,
|
||||||
|
}],
|
||||||
|
memberships: [legacy, system, future]
|
||||||
|
.into_iter()
|
||||||
|
.map(|channel| (channel, author(&member)))
|
||||||
|
.collect(),
|
||||||
|
named_only: NamedOnlyChannels::from_typed([
|
||||||
|
(system, "system".to_owned()),
|
||||||
|
(future, "later-type".to_owned()),
|
||||||
|
]),
|
||||||
|
};
|
||||||
|
let job = |channel: Option<uuid::Uuid>| {
|
||||||
|
let mut event = EventBuilder::new(Kind::Custom(9), "hi");
|
||||||
|
if let Some(channel) = channel {
|
||||||
|
event = event.tag(Tag::parse(["h", &channel.to_string()]).unwrap());
|
||||||
|
}
|
||||||
|
buzz_db::push::BatchedMatch {
|
||||||
|
event: buzz_core::StoredEvent::new(event.sign_with_keys(&sender).unwrap(), channel),
|
||||||
|
attempt: 1,
|
||||||
|
}
|
||||||
|
};
|
||||||
|
let wakes = |keys: &nostr::Keys, filter: Value, channel| {
|
||||||
|
match_job(&job(channel), &context(keys, filter))
|
||||||
|
.unwrap()
|
||||||
|
.len()
|
||||||
|
};
|
||||||
|
let names =
|
||||||
|
|channel: uuid::Uuid| serde_json::json!({"kinds": [9], "#h": [channel.to_string()]});
|
||||||
|
let broad = serde_json::json!({"kinds": [9]});
|
||||||
|
|
||||||
|
// A filter that names no channel reads legacy-type channels and
|
||||||
|
// channel-less events, but no named-only channel.
|
||||||
|
assert_eq!(wakes(&member, broad.clone(), Some(legacy)), 1, "legacy");
|
||||||
|
assert_eq!(wakes(&member, broad.clone(), None), 1, "no channel");
|
||||||
|
assert_eq!(
|
||||||
|
wakes(&member, broad.clone(), Some(system)),
|
||||||
|
0,
|
||||||
|
"unnamed system"
|
||||||
|
);
|
||||||
|
assert_eq!(wakes(&member, broad, Some(future)), 0, "unnamed later type");
|
||||||
|
|
||||||
|
// Naming a later-type channel by #h reads it, as in REQ.
|
||||||
|
assert_eq!(
|
||||||
|
wakes(&member, names(future), Some(future)),
|
||||||
|
1,
|
||||||
|
"named later type"
|
||||||
|
);
|
||||||
|
// NIP-CL: a system channel never pushes, even when named.
|
||||||
|
assert_eq!(
|
||||||
|
wakes(&member, names(system), Some(system)),
|
||||||
|
0,
|
||||||
|
"named system"
|
||||||
|
);
|
||||||
|
// Naming one channel does not widen the filter to another.
|
||||||
|
let other = uuid::Uuid::new_v4();
|
||||||
|
assert_eq!(wakes(&member, names(other), Some(future)), 0, "other #h");
|
||||||
|
// Membership is still required.
|
||||||
|
assert_eq!(
|
||||||
|
wakes(&outsider, names(future), Some(future)),
|
||||||
|
0,
|
||||||
|
"non-member"
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
wakes(&outsider, names(legacy), Some(legacy)),
|
||||||
|
0,
|
||||||
|
"non-member legacy"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
async fn capture(
|
async fn capture(
|
||||||
State(seen): State<Arc<Mutex<Vec<Value>>>>,
|
State(seen): State<Arc<Mutex<Vec<Value>>>>,
|
||||||
OriginalUri(uri): OriginalUri,
|
OriginalUri(uri): OriginalUri,
|
||||||
|
|||||||
Reference in New Issue
Block a user