From 9e28cfbdadb83fb03ecb054ac053766b442ec03e Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Mon, 9 Nov 2026 16:51:05 -0500 Subject: [PATCH 1/2] Refuse opening a private channel that has other members A kind:9002 that changes a private channel to open changes the admission rules under everyone who joined it while it was private. The relay now refuses that change unless every member, current or removed, is the sender or an agent whose NIP-OA owner is the sender, for example a Solo chat that holds only its owner and the owner's agents. Removed members count because their posts stay in the history that opening would publish. Open to private stays allowed. The check runs under the membership lock that a visibility change already takes, so the member set it reads stays the same until commit. A refused edit stores nothing. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: Logan Johnson --- NOSTR.md | 2 +- crates/buzz-db/src/store/channel.rs | 46 +++- .../handlers/edit_metadata_postgres_tests.rs | 232 ++++++++++++++++++ 3 files changed, 278 insertions(+), 2 deletions(-) diff --git a/NOSTR.md b/NOSTR.md index 629c4f808a2..1c3c0a9076b 100644 --- a/NOSTR.md +++ b/NOSTR.md @@ -53,7 +53,7 @@ PGPASSWORD=buzz_dev psql -h localhost -U buzz -d buzz -c \ | **Group creation (kind:9007)** | ✅ | NIP-29; include an `h` tag with the new channel id and a `name` tag, optional `visibility`, `channel_type`, label `t` and `unlisted` tags. The relay refuses a 9007 without `h`. It refuses a listed channel (open, not unlisted, stream or forum) whose name another listed channel already has, ignoring case and including archived ones: `invalid: an open channel named "" already exists`. | | **Add user (kind:9000)** | ✅ | Open: any user, subject to target's `channel_add_policy` (`owner_only`/`nobody` can block). Private: owner/admin only. Self-add bypasses agent policy but not private-channel auth. A guest cannot add anyone. A new member with no role tag gets the channel's `join_role`; a non-member adding themselves may ask for `join_role` or a lower role. One event may name up to 100 people: one `p` tag each, with an optional role as the `p` tag's third element (NIP-29), falling back to the event's `role` tag. Each person is checked and added on their own. When the event names several people, the OK message is `members: ` followed by JSON listing the non-empty `added`, `updated` (role changed), `already_member` and `rejected` (`[{"reason", "pubkeys"}]`) results. The event is refused only when it is malformed or nobody it names may be added; then one person keeps the single-add error, and several get `invalid: members: {…}`. The event and its membership changes are stored together, so an `error:` rejection stores nothing and the same event can be resent; a resent stored event returns `duplicate:` without results. Re-adding someone at their current role posts no second `member_joined` notice. | | **Remove user (kind:9001)** | ✅ | Self-remove allowed (with last-owner guard). Removing others: owner/admin only. | -| **Edit group metadata (kind:9002)** | ✅ | `name`/`about`/label `t`/`visibility`/`ttl`/`archived`/`join_role`/`unlisted` tags: owner/admin. `topic`/`purpose` tags: any member except a guest. `auto_join` tag: relay owner/admin (see [Default channels](#default-channels)). A relay operator may also send a 9002 that changes only `name` on a listed channel, archived ones included, whatever its channel role, so that duplicate names can be repaired; the channel stays archived. The relay refuses a `name`, `visibility` or `unlisted` change that leaves a listed channel with a name another listed channel already has: `invalid: an open channel named "" already exists`. Buzz applies 9002 as a partial update: an absent tag leaves that setting unchanged. | +| **Edit group metadata (kind:9002)** | ✅ | `name`/`about`/label `t`/`visibility`/`ttl`/`archived`/`join_role`/`unlisted` tags: owner/admin. `topic`/`purpose` tags: any member except a guest. `auto_join` tag: relay owner/admin (see [Default channels](#default-channels)). A relay operator may also send a 9002 that changes only `name` on a listed channel, archived ones included, whatever its channel role, so that duplicate names can be repaired; the channel stays archived. The relay refuses a `name`, `visibility` or `unlisted` change that leaves a listed channel with a name another listed channel already has: `invalid: an open channel named "" already exists`. The relay refuses a private-to-open `visibility` change while the channel has, or once had, a member who is neither the sender nor an agent the sender owns (NIP-OA); removed members count, because their posts stay in its history: `invalid: a private channel with other members cannot be made open; create a new open channel instead`. Buzz applies 9002 as a partial update: an absent tag leaves that setting unchanged. | | **Admin delete event (kind:9005)** | ✅ | Event author can always delete own. Otherwise owner/admin required. Target must be in same channel. | | **Group deletion (kind:9008)** | ✅ | Owner only. | | **Leave group (kind:9022)** | ✅ | Any member. Last-owner guard prevents orphaned groups. | diff --git a/crates/buzz-db/src/store/channel.rs b/crates/buzz-db/src/store/channel.rs index b868ed8b612..eded1c0b32f 100644 --- a/crates/buzz-db/src/store/channel.rs +++ b/crates/buzz-db/src/store/channel.rs @@ -1102,6 +1102,9 @@ impl ChannelUpdate { /// /// At least one field must be provided; returns `InvalidData` otherwise. /// Returns the updated `ChannelRecord` on success. +/// +/// It has no sender, so it does not apply the private-to-open rule that +/// kind:9002 enforces (`apply_channel_metadata_event`); user edits go there. pub async fn update_channel( pool: &PgPool, community_id: CommunityId, @@ -1636,6 +1639,34 @@ pub struct ChannelMetadataWrite { pub previous_visibility: String, } +/// The refusal of a private-to-open change on a channel that holds, or once +/// held, someone besides the sender and the sender's own agents. +const PRIVATE_TO_OPEN_REFUSAL: &str = + "a private channel with other members cannot be made open; create a new open channel instead"; + +/// Whether the locked channel has a member, current or removed, that is neither +/// `sender` nor an agent whose NIP-OA owner is `sender`. Removed members count: +/// their posts stay in the history that opening would publish. +async fn has_member_besides_sender_and_agents( + w: &mut ChannelSettingsWriter<'_>, + sender: &[u8], +) -> Result { + let channel_id = w.channel_id; + let tx = w.tx(); + let community_id = tx.community(); + Ok(sqlx::query_scalar::<_, bool>( + "SELECT EXISTS (SELECT 1 FROM channel_members m \ + LEFT JOIN users u ON u.community_id = m.community_id AND u.pubkey = m.pubkey \ + WHERE m.community_id = $1 AND m.channel_id = $2 \ + AND m.pubkey <> $3 AND u.agent_owner_pubkey IS DISTINCT FROM $3)", + ) + .bind(community_id.as_uuid()) + .bind(channel_id) + .bind(sender) + .fetch_one(tx.conn()) + .await?) +} + pub(crate) async fn apply_channel_metadata_event( pool: &PgPool, push_enqueue: Option, @@ -1693,6 +1724,18 @@ pub(crate) async fn apply_channel_metadata_event( return Err(DbError::AccessDenied("channel is not archived".to_string())); } } + // Opening a private channel changes the admission rules under everyone + // who joined it, so only a channel that holds nobody but the sender and + // the sender's own agents (NIP-OA) may be opened, for example a Solo + // chat. A visibility change holds the membership lock + // (`lock_channel_for_settings_write`), so the member set read here stays + // the same until commit. + if edit.columns.visibility.as_deref() == Some(ChannelVisibility::Open.as_str()) + && locked.previous_visibility() == ChannelVisibility::Private.as_str() + && has_member_besides_sender_and_agents(&mut locked, &event.pubkey.to_bytes()).await? + { + return Err(DbError::AccessDenied(PRIVATE_TO_OPEN_REFUSAL.to_string())); + } // `auto_join` only makes sense where a kind:9021 self-join would work: // an open, unarchived channel. Decide it here, under the row lock and // after the archive decision, on the state this event leaves behind, so @@ -2167,7 +2210,8 @@ impl Db { .await } - /// Updates a channel's name and/or description. + /// Updates a channel's name and/or description. It does not apply the + /// private-to-open rule; user edits go through kind:9002. #[datastore_span(name = "update_channel", system = "postgresql")] pub async fn update_channel( &self, diff --git a/crates/buzz-relay/src/handlers/edit_metadata_postgres_tests.rs b/crates/buzz-relay/src/handlers/edit_metadata_postgres_tests.rs index 54283d5f86c..a133d1f38a1 100644 --- a/crates/buzz-relay/src/handlers/edit_metadata_postgres_tests.rs +++ b/crates/buzz-relay/src/handlers/edit_metadata_postgres_tests.rs @@ -829,3 +829,235 @@ async fn concurrent_copies_of_a_restore_apply_once() { notices + 1 ); } + +/// The refusal NOSTR.md documents for a private-to-open edit. +const REFUSAL: &str = "invalid: a private channel with other members cannot be made open; create a new open channel instead"; + +/// What `edit_channel` returns for that refusal: ingest rejects the event +/// with exactly the documented OK message. +fn refused() -> Result<(), String> { + Err(format!("Rejected({REFUSAL:?})")) +} + +/// Seed `agent` as a user whose NIP-OA owner is `owner`. +async fn seed_agent( + state: &crate::state::AppState, + tenant: &buzz_core::TenantContext, + agent: &Keys, + owner: &Keys, +) { + for user in [agent, owner] { + state + .db + .ensure_user(tenant.community(), user.public_key().to_bytes().as_slice()) + .await + .expect("ensure user"); + } + assert!(state + .db + .set_agent_owner( + tenant.community(), + agent.public_key().to_bytes().as_slice(), + owner.public_key().to_bytes().as_slice(), + ) + .await + .expect("set agent owner")); +} + +/// A private channel can be made open only when nobody but the sender and +/// the sender's own agents is a member (Channels for IFC): opening it would +/// change the admission rules under anyone else who joined. Someone else's +/// agent counts as someone else, and so does the sender's owner when an +/// agent sends. Removed members do not count. A refused change stores +/// nothing and leaves the channel private. Mutation: drop the check in +/// `apply_channel_metadata_event` → the refused rows open → RED. +#[tokio::test] +#[ignore = "requires Postgres"] +async fn private_to_open_needs_no_other_members() { + use buzz_db::channel::MemberRole; + + let state = state().await; + let tenant = community(&state, "private-to-open").await; + let owner = Keys::generate(); + let own_agent = Keys::generate(); + let stranger = Keys::generate(); + let strangers_agent = Keys::generate(); + // A member with no users row, as `put_members_on` can leave one. + let unknown = Keys::generate(); + seed_agent(&state, &tenant, &own_agent, &owner).await; + seed_agent(&state, &tenant, &strangers_agent, &stranger).await; + + struct Row<'a> { + name: &'a str, + members: Vec<(&'a Keys, MemberRole)>, + removed: Vec<&'a Keys>, + sender: &'a Keys, + opens: bool, + } + let rows = [ + Row { + name: "only the owner", + members: vec![], + removed: vec![], + sender: &owner, + opens: true, + }, + Row { + name: "the owner and their agent (a Solo chat)", + members: vec![(&own_agent, MemberRole::Member)], + removed: vec![], + sender: &owner, + opens: true, + }, + Row { + name: "a removed person still counts: their posts stay", + members: vec![(&stranger, MemberRole::Member)], + removed: vec![&stranger], + sender: &owner, + opens: false, + }, + Row { + name: "the owner's removed agent", + members: vec![(&own_agent, MemberRole::Member)], + removed: vec![&own_agent], + sender: &owner, + opens: true, + }, + Row { + name: "another person", + members: vec![ + (&own_agent, MemberRole::Member), + (&stranger, MemberRole::Guest), + ], + removed: vec![], + sender: &owner, + opens: false, + }, + Row { + name: "a member with no user row", + members: vec![(&unknown, MemberRole::Member)], + removed: vec![], + sender: &owner, + opens: false, + }, + Row { + name: "someone else's agent", + members: vec![(&strangers_agent, MemberRole::Bot)], + removed: vec![], + sender: &owner, + opens: false, + }, + Row { + name: "an agent sending, with its owner a member", + members: vec![(&own_agent, MemberRole::Admin)], + removed: vec![], + sender: &own_agent, + opens: false, + }, + ]; + + for row in rows { + let channel_id = + create_channel(&state, &tenant, &owner, &[&["visibility", "private"]]).await; + for (member, role) in &row.members { + state + .db + .add_member( + tenant.community(), + channel_id, + member.public_key().to_bytes().as_slice(), + *role, + Some(owner.public_key().to_bytes().as_slice()), + ) + .await + .expect("add member"); + } + for member in &row.removed { + state + .db + .remove_member( + tenant.community(), + channel_id, + member.public_key().to_bytes().as_slice(), + owner.public_key().to_bytes().as_slice(), + ) + .await + .expect("remove member"); + } + let events_before = count_kind(&state, &tenant, channel_id, 9002).await; + + let result = edit_channel( + &state, + &tenant, + row.sender, + channel_id, + &[&["visibility", "open"]], + ) + .await; + let visibility = state + .db + .get_channel(tenant.community(), channel_id) + .await + .expect("channel") + .visibility; + if row.opens { + assert_eq!(result, Ok(()), "{}", row.name); + assert_eq!(visibility, "open", "{}", row.name); + // Open to private stays allowed, whoever is in the channel. + edit_channel( + &state, + &tenant, + row.sender, + channel_id, + &[&["visibility", "private"]], + ) + .await + .unwrap_or_else(|e| panic!("{}: make private again: {e}", row.name)); + } else { + assert_eq!(result, refused(), "{}", row.name); + assert_eq!(visibility, "private", "{}", row.name); + assert_eq!( + count_kind(&state, &tenant, channel_id, 9002).await, + events_before, + "{}: a refused edit is not stored", + row.name + ); + } + } + + // A channel that someone else joined while it was open, then made + // private again, cannot be reopened. + let channel_id = create_channel(&state, &tenant, &owner, &[&["visibility", "open"]]).await; + state + .db + .add_member( + tenant.community(), + channel_id, + stranger.public_key().to_bytes().as_slice(), + MemberRole::Member, + None, + ) + .await + .expect("stranger joins"); + edit_channel( + &state, + &tenant, + &owner, + channel_id, + &[&["visibility", "private"]], + ) + .await + .expect("make private"); + assert_eq!( + edit_channel( + &state, + &tenant, + &owner, + channel_id, + &[&["visibility", "open"]], + ) + .await, + refused(), + "reopen with a stranger in it" + ); +} From 2482ad871418b8475dd899b7e55939cb5eb8f727 Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Tue, 10 Nov 2026 08:03:52 -0500 Subject: [PATCH 2/2] Say removed members count in the private-to-open test doc The test asserts that removed members block opening; its doc comment said the opposite. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: Logan Johnson --- crates/buzz-relay/src/handlers/edit_metadata_postgres_tests.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/buzz-relay/src/handlers/edit_metadata_postgres_tests.rs b/crates/buzz-relay/src/handlers/edit_metadata_postgres_tests.rs index a133d1f38a1..8acfed3057a 100644 --- a/crates/buzz-relay/src/handlers/edit_metadata_postgres_tests.rs +++ b/crates/buzz-relay/src/handlers/edit_metadata_postgres_tests.rs @@ -868,7 +868,8 @@ async fn seed_agent( /// the sender's own agents is a member (Channels for IFC): opening it would /// change the admission rules under anyone else who joined. Someone else's /// agent counts as someone else, and so does the sender's owner when an -/// agent sends. Removed members do not count. A refused change stores +/// agent sends. Removed members count too: their posts stay in the +/// history that opening would publish. A refused change stores /// nothing and leaves the channel private. Mutation: drop the check in /// `apply_channel_metadata_event` → the refused rows open → RED. #[tokio::test]