fix(channels): restrict private-channel invitations

VISION.md

@@ -39,7 +39,7 @@ The relay enforces all access control. Channel membership is the only gate.
 | Type | Visibility | Join | Create |
 |------|-----------|------|--------|
 | **Open channels** | Searchable by all members | Self-join | Any member |
-| **Private channels** | Hidden, invite-only | Invited by member | Any member |
+| **Private channels** | Hidden, invite-only | Invited by an owner/admin | Any member |
 | **DMs** | Participants only | N/A (up to 9) | Any member |
 | **Guests** | Scoped to specific channels | Invited | N/A |
 

crates/buzz-db/src/channel.rs

@@ -371,7 +371,9 @@ async fn acquire_channel_membership_lock(
 /// Role enforcement:
 /// - Open channels: `invited_by` is optional; role is forced to `Member` regardless of
 ///   what the caller passes — callers cannot self-assign elevated roles.
-/// - Private channels: requires an `invited_by` who is an active owner/admin.
+/// - Private channels: requires an `invited_by` who is an active owner/admin, the channel
+///   creator bootstrapping their own first membership, or the target adding themselves
+///   (idempotent re-add — an active member's *role* still cannot change this way).
 /// - Elevated roles (`Owner`, `Admin`) may only be granted by an existing owner/admin,
 ///   even on open channels.
 ///
@@ -419,10 +421,14 @@ pub async fn add_member(
                 DbError::InvalidData(format!("invalid role in database: {inviter_role_str}"))
             })?;
 
-            // Any member can invite others, but only owners/admins may grant elevated roles.
-            if role.is_elevated() && !inviter_role.is_elevated() {
+            // Only owners/admins may extend private-channel access to another
+            // identity. `inviter == pubkey` keeps a member's own idempotent
+            // re-add working; it is not a role-escalation hole, because the
+            // active-role-change guard below still rejects a self-targeted
+            // promotion from any non-elevated caller.
+            if !inviter_role.is_elevated() && inviter != pubkey {
                 return Err(DbError::AccessDenied(
-                    "only owners/admins may grant elevated roles".to_string(),
+                    "only owners/admins may add private-channel members".to_string(),
                 ));
             }
         }

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

@@ -355,28 +355,28 @@ pub async fn validate_admin_event(
                 .iter()
                 .find(|m| m.pubkey == actor_bytes)
                 .and_then(|m| m.role.parse().ok());
-
-            // PUT_USER: open channels allow any authenticated user; private channels
-            // require the actor to be an existing member (any role can invite).
-            if channel.visibility == "private" {
-                if actor_role.is_none() {
-                    return Err(anyhow::anyhow!("actor not authorized"));
-                }
-
-                // Only owners/admins may grant elevated roles.
-                if requested_role.is_some_and(|r| r.is_elevated())
-                    && !actor_role.is_some_and(|r| r.is_elevated())
-                {
-                    return Err(anyhow::anyhow!(
-                        "only owners/admins may grant elevated roles"
-                    ));
-                }
-            }
-
-            // Extract target pubkey from p tag
             let target_pubkey =
                 extract_p_tag(event).ok_or_else(|| anyhow::anyhow!("missing p tag"))?;
 
+            // PUT_USER: open channels allow any authenticated user. Private
+            // channels only let owners/admins add another identity; otherwise
+            // any compromised member could extend access to channel history.
+            //
+            // A self-targeted add skips this check so an idempotent re-add
+            // still works. That is not a way into a private channel: ingest's
+            // `check_channel_membership` rejects a non-member (and a
+            // soft-removed member) before this validator runs, and `add_member`
+            // independently requires the self-inviter to hold an active role.
+            // Self-promotion is caught by the role-change guard below.
+            if channel.visibility == "private"
+                && target_pubkey != actor_bytes
+                && !actor_role.is_some_and(|r| r.is_elevated())
+            {
+                return Err(anyhow::anyhow!(
+                    "only owners/admins may add private-channel members"
+                ));
+            }
+
             // Changing an ACTIVE existing member's role is privileged in both
             // directions, on every visibility. `get_members` filters
             // `removed_at IS NULL`, so a soft-removed row is deliberately not an

crates/buzz-test-client/tests/e2e_relay.rs

@@ -2201,6 +2201,10 @@ async fn create_private_channel_ws(client: &mut BuzzTestClient, keys: &Keys) ->
 }
 
 /// Submit a kind:9000 PUT_USER event over WebSocket.
+///
+/// `allow_self_tagging` keeps self-targeted adds working: EventBuilder otherwise
+/// drops a `p` tag matching the signer (nostr-0.44.3 builder.rs:435-449) and the
+/// event fails as "missing p tag" instead of exercising the authority check.
 async fn add_member_ws(
     client: &mut BuzzTestClient,
     channel_id: &str,
@@ -2210,6 +2214,7 @@ async fn add_member_ws(
     let h_tag = Tag::parse(["h", channel_id]).unwrap();
     let p_tag = Tag::parse(["p", target_pubkey_hex]).unwrap();
     let event = EventBuilder::new(Kind::Custom(9000), "")
+        .allow_self_tagging()
         .tags([h_tag, p_tag])
         .sign_with_keys(signer)
         .unwrap();
@@ -2219,6 +2224,8 @@ async fn add_member_ws(
 }
 
 /// Submit a kind:9000 PUT_USER event with a role tag over WebSocket.
+///
+/// See [`add_member_ws`] for why `allow_self_tagging` is required.
 async fn add_member_with_role_ws(
     client: &mut BuzzTestClient,
     channel_id: &str,
@@ -2230,6 +2237,7 @@ async fn add_member_with_role_ws(
     let p_tag = Tag::parse(["p", target_pubkey_hex]).unwrap();
     let role_tag = Tag::parse(["role", role]).unwrap();
     let event = EventBuilder::new(Kind::Custom(9000), "")
+        .allow_self_tagging()
         .tags([h_tag, p_tag, role_tag])
         .sign_with_keys(signer)
         .unwrap();
@@ -2241,10 +2249,10 @@ async fn add_member_with_role_ws(
     (ok.accepted, ok.message)
 }
 
-/// Any member of a private channel can invite another user (Slack model).
+/// Only owners/admins can add another identity to a private channel.
 #[tokio::test]
 #[ignore]
-async fn test_private_channel_any_member_can_invite() {
+async fn test_private_channel_member_cannot_invite() {
     let url = relay_url();
     let owner_keys = Keys::generate();
     let member_keys = Keys::generate();
@@ -2271,23 +2279,85 @@ async fn test_private_channel_any_member_can_invite() {
         .await
         .expect("connect as member");
 
-    // Regular member invites a third user — this should succeed.
+    // Regular member tries to invite a third user.
     let (accepted, msg) = add_member_ws(
         &mut member_client,
         &channel_id,
         &invitee_keys.public_key().to_hex(),
         &member_keys,
     )
     .await;
+    assert!(
+        !accepted,
+        "regular member must not add another private-channel identity: {msg}"
+    );
+    assert!(
+        msg.contains("owners/admins"),
+        "rejection should name the owner/admin requirement, got: {msg}"
+    );
+
+    // The same member re-adding *themselves* stays idempotent — the huddle
+    // bot-add and kind:9021 paths depend on a self-targeted PUT_USER working.
+    let (accepted, msg) = add_member_ws(
+        &mut member_client,
+        &channel_id,
+        &member_keys.public_key().to_hex(),
+        &member_keys,
+    )
+    .await;
     assert!(
         accepted,
-        "regular member should be able to invite to private channel, got: {msg}"
+        "self-targeted re-add must stay idempotent, got: {msg}"
     );
 
     owner_client.disconnect().await.expect("disconnect owner");
     member_client.disconnect().await.expect("disconnect member");
 }
 
+/// An admin — not just the owner — can still add to a private channel.
+#[tokio::test]
+#[ignore]
+async fn test_private_channel_admin_can_invite() {
+    let url = relay_url();
+    let owner_keys = Keys::generate();
+    let admin_keys = Keys::generate();
+    let invitee_keys = Keys::generate();
+
+    let mut owner_client = BuzzTestClient::connect(&url, &owner_keys)
+        .await
+        .expect("connect as owner");
+    let channel_id = create_private_channel_ws(&mut owner_client, &owner_keys).await;
+
+    let (accepted, msg) = add_member_with_role_ws(
+        &mut owner_client,
+        &channel_id,
+        &admin_keys.public_key().to_hex(),
+        "admin",
+        &owner_keys,
+    )
+    .await;
+    assert!(accepted, "owner should add an admin, got: {msg}");
+
+    let mut admin_client = BuzzTestClient::connect(&url, &admin_keys)
+        .await
+        .expect("connect as admin");
+
+    let (accepted, msg) = add_member_ws(
+        &mut admin_client,
+        &channel_id,
+        &invitee_keys.public_key().to_hex(),
+        &admin_keys,
+    )
+    .await;
+    assert!(
+        accepted,
+        "admin should be able to add to a private channel, got: {msg}"
+    );
+
+    owner_client.disconnect().await.expect("disconnect owner");
+    admin_client.disconnect().await.expect("disconnect admin");
+}
+
 /// A non-member cannot invite someone to a private channel.
 #[tokio::test]
 #[ignore]

desktop/src/features/channels/hooks.ts

@@ -32,7 +32,9 @@ import type {
   SetChannelTopicInput,
   UpdateChannelInput,
 } from "@/shared/api/types";
+import { useIdentityQuery } from "@/shared/api/hooks";
 import { useCommunities } from "@/features/communities/useCommunities";
+import { canAddChannelMembers } from "@/features/channels/lib/channelMemberAdmission";
 import {
   readChannelSnapshot,
   writeChannelSnapshot,
@@ -501,6 +503,32 @@ export function useDeleteChannelMutation(channelId: string | null) {
   });
 }
 
+/**
+ * Whether the signed-in identity may add *another* identity to this channel,
+ * per {@link canAddChannelMembers}. Both queries are the ones the channel UI
+ * already holds, so this shares their cache rather than fetching again.
+ */
+export function useCanAddChannelMembers(channelId: string | null) {
+  const channelsQuery = useChannelsQuery();
+  const membersQuery = useChannelMembersQuery(channelId);
+  const identityQuery = useIdentityQuery();
+
+  const channel =
+    channelsQuery.data?.find((candidate) => candidate.id === channelId) ?? null;
+  const selfPubkey = identityQuery.data?.pubkey ?? null;
+  const selfRole = selfPubkey
+    ? (membersQuery.data?.find(
+        (member) => member.pubkey.toLowerCase() === selfPubkey.toLowerCase(),
+      )?.role ?? null)
+    : null;
+
+  return canAddChannelMembers({
+    channelType: channel?.channelType,
+    visibility: channel?.visibility,
+    selfRole,
+  });
+}
+
 export function useAddChannelMembersMutation(channelId: string | null) {
   const queryClient = useQueryClient();
 

desktop/src/features/channels/lib/channelMemberAdmission.test.mjs

@@ -0,0 +1,79 @@
+import { strict as assert } from "node:assert";
+import test from "node:test";
+
+import { canAddChannelMembers } from "./channelMemberAdmission.ts";
+
+test("open channels accept adds from anyone, member or not", () => {
+  assert.equal(
+    canAddChannelMembers({
+      channelType: "stream",
+      visibility: "open",
+      selfRole: null,
+    }),
+    true,
+  );
+  assert.equal(
+    canAddChannelMembers({
+      channelType: "stream",
+      visibility: "open",
+      selfRole: "member",
+    }),
+    true,
+  );
+});
+
+test("private channels accept adds only from owners/admins", () => {
+  for (const selfRole of ["owner", "admin"]) {
+    assert.equal(
+      canAddChannelMembers({
+        channelType: "stream",
+        visibility: "private",
+        selfRole,
+      }),
+      true,
+      `${selfRole} should be able to add`,
+    );
+  }
+
+  for (const selfRole of ["member", "bot", "guest", null]) {
+    assert.equal(
+      canAddChannelMembers({
+        channelType: "stream",
+        visibility: "private",
+        selfRole,
+      }),
+      false,
+      `${selfRole} must not be able to add`,
+    );
+  }
+});
+
+test("DMs never accept adds, even from an owner", () => {
+  assert.equal(
+    canAddChannelMembers({
+      channelType: "dm",
+      visibility: "private",
+      selfRole: "owner",
+    }),
+    false,
+  );
+  assert.equal(
+    canAddChannelMembers({
+      channelType: "dm",
+      visibility: "open",
+      selfRole: "owner",
+    }),
+    false,
+  );
+});
+
+test("unknown visibility fails closed for non-elevated callers", () => {
+  assert.equal(
+    canAddChannelMembers({ channelType: "stream", selfRole: "member" }),
+    false,
+  );
+  assert.equal(
+    canAddChannelMembers({ channelType: "stream", selfRole: "owner" }),
+    true,
+  );
+});

desktop/src/features/channels/lib/channelMemberAdmission.ts

@@ -0,0 +1,36 @@
+/**
+ * Client mirror of the relay's kind:9000 authority for adding *another*
+ * identity to a channel (`validate_admin_event` + `buzz_db::channel::add_member`):
+ *
+ * - DMs: nobody — membership is fixed at creation.
+ * - Open channels: anyone, member or not.
+ * - Private channels: owners/admins only. A plain member extending access to
+ *   channel history is exactly what the relay now rejects, so the affordance
+ *   must not be offered.
+ *
+ * Unknown visibility fails closed — the relay is the authority and a hidden
+ * button is cheaper than an opaque rejection.
+ */
+export function canAddChannelMembers({
+  channelType,
+  visibility,
+  selfRole,
+}: {
+  channelType?: string | null;
+  visibility?: string | null;
+  selfRole?: string | null;
+}): boolean {
+  if (channelType === "dm") {
+    return false;
+  }
+
+  if (visibility === "open") {
+    return true;
+  }
+
+  return selfRole === "owner" || selfRole === "admin";
+}
+
+/** Explains a denied add so the user isn't left guessing at a missing button. */
+export const PRIVATE_CHANNEL_ADD_DENIED_MESSAGE =
+  "Only channel owners and admins can add people to a private channel.";