Refuse opening a private channel that has other members

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 "<name>" 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 "<name>" 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 "<name>" 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. |

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<bool> {
+    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<crate::runtime::push_enqueue::PushEnqueue>,
@@ -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,

crates/buzz-relay/src/handlers/edit_metadata_postgres_tests.rs

@@ -829,3 +829,236 @@ 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 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]
+#[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"
+    );
+}