From 7de700e17642ad7e10155f9537033168d9249268 Mon Sep 17 00:00:00 2001 From: Wes Date: Mon, 10 Aug 2026 10:47:37 -0600 Subject: [PATCH 1/2] fix(channels): restore private member invitations Allow every active private-channel member to invite ordinary members, guests, and bots while preserving owner/admin authority over elevated roles and active membership changes. Co-authored-by: Carl Signed-off-by: Wes --- VISION.md | 2 +- crates/buzz-db/src/channel.rs | 18 ++- .../buzz-relay/src/handlers/side_effects.rs | 33 +++--- crates/buzz-test-client/tests/e2e_relay.rs | 107 ++++++++++-------- .../lib/channelMemberAdmission.test.mjs | 28 +++-- .../channels/lib/channelMemberAdmission.ts | 13 ++- .../features/channels/ui/MembersSidebar.tsx | 4 +- desktop/tests/e2e/channels.spec.ts | 22 ++-- mobile/lib/features/channels/channel.dart | 9 +- .../channels/compose_bar/helpers.dart | 2 +- .../test/features/channels/channel_test.dart | 11 +- .../features/channels/compose_bar_test.dart | 15 ++- 12 files changed, 136 insertions(+), 128 deletions(-) diff --git a/VISION.md b/VISION.md index 66a106bdebc..900e5a9475b 100644 --- a/VISION.md +++ b/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 an owner/admin | Any member | +| **Private channels** | Hidden, invite-only | Invited by member | Any member | | **DMs** | Participants only | N/A (up to 9) | Any member | | **Guests** | Scoped to specific channels | Invited | N/A | diff --git a/crates/buzz-db/src/channel.rs b/crates/buzz-db/src/channel.rs index 9d15fccfc81..fecb6b0ac98 100644 --- a/crates/buzz-db/src/channel.rs +++ b/crates/buzz-db/src/channel.rs @@ -371,9 +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, 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). +/// - Private channels: requires an `invited_by` who is an active member, or the channel +/// creator bootstrapping their own first membership. Any active member may add an +/// ordinary member, guest, or bot; only owners/admins may grant elevated roles. /// - Elevated roles (`Owner`, `Admin`) may only be granted by an existing owner/admin, /// even on open channels. /// @@ -421,14 +421,12 @@ pub async fn add_member( DbError::InvalidData(format!("invalid role in database: {inviter_role_str}")) })?; - // 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 { + // Any active member may extend private-channel access with an + // ordinary role. Granting owner/admin remains reserved for an + // existing owner/admin. + if role.is_elevated() && !inviter_role.is_elevated() { return Err(DbError::AccessDenied( - "only owners/admins may add private-channel members".to_string(), + "only owners/admins may grant elevated roles".to_string(), )); } } diff --git a/crates/buzz-relay/src/handlers/side_effects.rs b/crates/buzz-relay/src/handlers/side_effects.rs index 98f8a9aa847..88a9f0c731c 100644 --- a/crates/buzz-relay/src/handlers/side_effects.rs +++ b/crates/buzz-relay/src/handlers/side_effects.rs @@ -358,23 +358,22 @@ pub async fn validate_admin_event( 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" - )); + // PUT_USER: open channels allow any authenticated user; private channels + // require the actor to be an existing active member. Any active member may + // add an ordinary member, guest, or bot, but only owners/admins may grant + // an elevated role. + if channel.visibility == "private" { + if actor_role.is_none() { + return Err(anyhow::anyhow!("actor not authorized")); + } + + if requested_role.is_some_and(|role| role.is_elevated()) + && !actor_role.is_some_and(|role| role.is_elevated()) + { + return Err(anyhow::anyhow!( + "only owners/admins may grant elevated roles" + )); + } } // Changing an ACTIVE existing member's role is privileged in both diff --git a/crates/buzz-test-client/tests/e2e_relay.rs b/crates/buzz-test-client/tests/e2e_relay.rs index 5b9a50b5b17..ff7c831eb8d 100644 --- a/crates/buzz-test-client/tests/e2e_relay.rs +++ b/crates/buzz-test-client/tests/e2e_relay.rs @@ -2249,14 +2249,17 @@ async fn add_member_with_role_ws( (ok.accepted, ok.message) } -/// Only owners/admins can add another identity to a private channel. +/// Any active member can add any ordinary role to a private channel. #[tokio::test] #[ignore] -async fn test_private_channel_member_cannot_invite() { +async fn test_private_channel_any_member_can_invite() { let url = relay_url(); let owner_keys = Keys::generate(); - let member_keys = Keys::generate(); - let invitee_keys = Keys::generate(); + let actors = [ + ("member", Keys::generate()), + ("guest", Keys::generate()), + ("bot", Keys::generate()), + ]; // Connect as owner and create a private channel. let mut owner_client = BuzzTestClient::connect(&url, &owner_keys) @@ -2264,54 +2267,64 @@ async fn test_private_channel_member_cannot_invite() { .expect("connect as owner"); let channel_id = create_private_channel_ws(&mut owner_client, &owner_keys).await; - // Owner adds member_keys as a regular member. - let (accepted, msg) = add_member_ws( - &mut owner_client, - &channel_id, - &member_keys.public_key().to_hex(), - &owner_keys, - ) - .await; - assert!(accepted, "owner should add member, got: {msg}"); + // Seed one actor for each ordinary active role. + for (role, keys) in &actors { + let (accepted, msg) = add_member_with_role_ws( + &mut owner_client, + &channel_id, + &keys.public_key().to_hex(), + role, + &owner_keys, + ) + .await; + assert!(accepted, "owner should add {role} actor, got: {msg}"); + } - // Connect as the regular member. - let mut member_client = BuzzTestClient::connect(&url, &member_keys) - .await - .expect("connect as member"); + // Exercise the full ordinary-role target matrix. Relay and DB authorization + // both run here, unlike the Desktop/mobile policy-unit-test mirrors. + for (actor_role, actor_keys) in &actors { + let mut actor_client = BuzzTestClient::connect(&url, actor_keys) + .await + .unwrap_or_else(|err| panic!("connect as {actor_role}: {err}")); + + for target_role in ["member", "guest", "bot"] { + let target_keys = Keys::generate(); + let (accepted, msg) = add_member_with_role_ws( + &mut actor_client, + &channel_id, + &target_keys.public_key().to_hex(), + target_role, + actor_keys, + ) + .await; + assert!( + accepted, + "private-channel {actor_role} should add {target_role}, got: {msg}" + ); + } - // 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}" - ); + // Re-adding oneself stays idempotent — the huddle bot-add and kind:9021 + // paths depend on a self-targeted PUT_USER working. + let (accepted, msg) = add_member_with_role_ws( + &mut actor_client, + &channel_id, + &actor_keys.public_key().to_hex(), + actor_role, + actor_keys, + ) + .await; + assert!( + accepted, + "self-targeted {actor_role} re-add must stay idempotent, 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, - "self-targeted re-add must stay idempotent, got: {msg}" - ); + actor_client + .disconnect() + .await + .unwrap_or_else(|err| panic!("disconnect {actor_role}: {err}")); + } 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. diff --git a/desktop/src/features/channels/lib/channelMemberAdmission.test.mjs b/desktop/src/features/channels/lib/channelMemberAdmission.test.mjs index 0af22459f44..4f705d0f405 100644 --- a/desktop/src/features/channels/lib/channelMemberAdmission.test.mjs +++ b/desktop/src/features/channels/lib/channelMemberAdmission.test.mjs @@ -22,8 +22,8 @@ test("open channels accept adds from anyone, member or not", () => { ); }); -test("private channels accept adds only from owners/admins", () => { - for (const selfRole of ["owner", "admin"]) { +test("private channels accept adds from every active member role", () => { + for (const selfRole of ["owner", "admin", "member", "bot", "guest"]) { assert.equal( canAddChannelMembers({ channelType: "stream", @@ -35,17 +35,15 @@ test("private channels accept adds only from owners/admins", () => { ); } - for (const selfRole of ["member", "bot", "guest", null]) { - assert.equal( - canAddChannelMembers({ - channelType: "stream", - visibility: "private", - selfRole, - }), - false, - `${selfRole} must not be able to add`, - ); - } + assert.equal( + canAddChannelMembers({ + channelType: "stream", + visibility: "private", + selfRole: null, + }), + false, + "a non-member must not be able to add", + ); }); test("DMs never accept adds, even from an owner", () => { @@ -67,13 +65,13 @@ test("DMs never accept adds, even from an owner", () => { ); }); -test("unknown visibility fails closed for non-elevated callers", () => { +test("unknown visibility fails closed", () => { assert.equal( canAddChannelMembers({ channelType: "stream", selfRole: "member" }), false, ); assert.equal( canAddChannelMembers({ channelType: "stream", selfRole: "owner" }), - true, + false, ); }); diff --git a/desktop/src/features/channels/lib/channelMemberAdmission.ts b/desktop/src/features/channels/lib/channelMemberAdmission.ts index b01c6f52163..7ef15c6b68c 100644 --- a/desktop/src/features/channels/lib/channelMemberAdmission.ts +++ b/desktop/src/features/channels/lib/channelMemberAdmission.ts @@ -4,9 +4,8 @@ * * - 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. + * - Private channels: any active member. The relay separately reserves elevated + * role grants and active-role changes for owners/admins. * * Unknown visibility fails closed — the relay is the authority and a hidden * button is cheaper than an opaque rejection. @@ -28,9 +27,13 @@ export function canAddChannelMembers({ return true; } - return selfRole === "owner" || selfRole === "admin"; + if (visibility === "private") { + return selfRole != null; + } + + return false; } /** 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."; + "Only channel members can add people to a private channel."; diff --git a/desktop/src/features/channels/ui/MembersSidebar.tsx b/desktop/src/features/channels/ui/MembersSidebar.tsx index 8f431fb0afe..9ec3151bb09 100644 --- a/desktop/src/features/channels/ui/MembersSidebar.tsx +++ b/desktop/src/features/channels/ui/MembersSidebar.tsx @@ -251,8 +251,8 @@ export function MembersSidebar({ visibility: channel?.visibility, selfRole: selfMember?.role, }); - // Distinguish "you can't add here" from "nothing to add" so a plain member of - // a private channel gets the reason instead of a silently missing affordance. + // Distinguish "you can't add here" from "nothing to add" so a non-member + // viewing a private channel gets the reason instead of a silently missing affordance. const showPrivateAddDeniedNotice = !canAddMembers && selfMember !== null && diff --git a/desktop/tests/e2e/channels.spec.ts b/desktop/tests/e2e/channels.spec.ts index 13c6d5504a8..ff410205cd3 100644 --- a/desktop/tests/e2e/channels.spec.ts +++ b/desktop/tests/e2e/channels.spec.ts @@ -3828,7 +3828,7 @@ test("members sidebar retains distinct same-persona managed agents", async ({ await expect(page.getByText("Pinky", { exact: true })).toHaveCount(2); }); -test("private-channel members cannot add people without owner/admin", async ({ +test("private-channel members can add people and managed agents without admin", async ({ page, }) => { await installMockBridge(page, { @@ -3842,26 +3842,22 @@ test("private-channel members cannot add people without owner/admin", async ({ }); await page.goto("/"); // secret-projects is a private (non-DM) channel where the current user is a - // plain member. The relay rejects their kind:9000, so the affordance is - // withheld and the reason shown instead of failing after the fact. + // plain member. Active members may add ordinary members and bots; only + // elevated-role grants and role changes require owner/admin authority. await openMembersSidebar(page, "secret-projects"); - await expect(page.getByTestId("members-sidebar-add-denied")).toBeVisible(); - // The field stays, but only as a filter over existing members. + await expect(page.getByTestId("members-sidebar-add-denied")).toHaveCount(0); await expect( page.getByTestId("channel-management-search-users"), - ).toHaveAttribute("placeholder", "Search people and agents"); + ).toHaveAttribute("placeholder", "Add people and agents"); await page.getByTestId("channel-management-search-users").fill("char"); - await expect(page.getByText("Not in this channel")).toHaveCount(0); - await expect( - page.getByTestId( - `channel-user-search-result-${TEST_IDENTITIES.charlie.pubkey}`, - ), - ).toHaveCount(0); + await page + .getByTestId(`channel-user-search-result-${TEST_IDENTITIES.charlie.pubkey}`) + .click(); await expect( page.getByTestId(`sidebar-member-${TEST_IDENTITIES.charlie.pubkey}`), - ).toHaveCount(0); + ).toContainText("charlie"); }); test("open-channel members can add people and managed agents without admin", async ({ diff --git a/mobile/lib/features/channels/channel.dart b/mobile/lib/features/channels/channel.dart index 29db1f96c57..30b3f2b48ff 100644 --- a/mobile/lib/features/channels/channel.dart +++ b/mobile/lib/features/channels/channel.dart @@ -5,7 +5,7 @@ const Object _sentinel = Object(); /// Shown when a private-channel add is refused, so a missing Invite action /// reads as a rule rather than a bug. const privateChannelAddDeniedMessage = - 'Only channel owners and admins can add people to a private channel.'; + 'Only channel members can add people to a private channel.'; @immutable class Channel { @@ -85,12 +85,13 @@ class Channel { /// Whether [selfRole] may add *another* identity here, mirroring the relay's /// kind:9000 authority (`validate_admin_event` + `add_member`): DMs never, - /// open channels always, private channels owners/admins only. An unknown - /// visibility fails closed — the relay is the authority. + /// open channels always, private channels for any active member. Elevated + /// grants remain reserved for owners/admins. Unknown visibility fails closed. bool canAddMembers(String? selfRole) { if (isDm) return false; if (visibility == 'open') return true; - return selfRole == 'owner' || selfRole == 'admin'; + if (visibility == 'private') return selfRole != null; + return false; } bool get isArchived => archivedAt != null; diff --git a/mobile/lib/features/channels/compose_bar/helpers.dart b/mobile/lib/features/channels/compose_bar/helpers.dart index f8cff238e5f..d5592a13e53 100644 --- a/mobile/lib/features/channels/compose_bar/helpers.dart +++ b/mobile/lib/features/channels/compose_bar/helpers.dart @@ -279,7 +279,7 @@ Future<_NonMemberAddOutcome> _addMentionedNonMembers( ]; if (pending.isEmpty) return _NonMemberAddOutcome.empty; - // A plain member of a private channel cannot add anyone: skip the doomed + // A non-member cannot add anyone to a private channel: skip the doomed // kind:9000 rather than trading it for a relay rejection. if (!canAddMembers) { return _NonMemberAddOutcome( diff --git a/mobile/test/features/channels/channel_test.dart b/mobile/test/features/channels/channel_test.dart index 9673eda28e1..2fce3b94870 100644 --- a/mobile/test/features/channels/channel_test.dart +++ b/mobile/test/features/channels/channel_test.dart @@ -215,12 +215,13 @@ void main() { expect(channel.canAddMembers('member'), isTrue); }); - test('private channels accept adds only from owners/admins', () { + test('private channels accept adds from any active member role', () { final channel = make(channelType: 'stream', visibility: 'private'); expect(channel.canAddMembers('owner'), isTrue); expect(channel.canAddMembers('admin'), isTrue); - expect(channel.canAddMembers('member'), isFalse); - expect(channel.canAddMembers('bot'), isFalse); + expect(channel.canAddMembers('member'), isTrue); + expect(channel.canAddMembers('bot'), isTrue); + expect(channel.canAddMembers('guest'), isTrue); expect(channel.canAddMembers(null), isFalse); }); @@ -235,10 +236,10 @@ void main() { ); }); - test('unknown visibility fails closed for non-elevated callers', () { + test('unknown visibility fails closed', () { final channel = make(channelType: 'stream', visibility: 'mystery'); expect(channel.canAddMembers('member'), isFalse); - expect(channel.canAddMembers('owner'), isTrue); + expect(channel.canAddMembers('owner'), isFalse); }); }); } diff --git a/mobile/test/features/channels/compose_bar_test.dart b/mobile/test/features/channels/compose_bar_test.dart index d58555f45ac..1d6d4518c62 100644 --- a/mobile/test/features/channels/compose_bar_test.dart +++ b/mobile/test/features/channels/compose_bar_test.dart @@ -3139,7 +3139,7 @@ void main() { }); testWidgets( - 'skips the agent add in a private channel when not owner/admin', + 'adds the agent in a private channel when the sender is a plain member', (tester) async { final agentPubkey = 'a' * 64; final signer = nostr.Keys.generate(); @@ -3152,8 +3152,8 @@ void main() { _buildComposeBar( uploadService: _testUploadService(signer.nsec), currentPubkey: signer.public, - // Plain member of a private channel: the relay rejects any add, so - // the composer must not attempt one — and must still send. + // Plain member of a private channel: ordinary member and bot + // additions are permitted; elevated-role grants still are not. members: [ ChannelMember( pubkey: signer.public, @@ -3201,15 +3201,14 @@ void main() { expect(didSend, isTrue); expect( publishedEvents.where((event) => event['kind'] == 9000), - isEmpty, + hasLength(1), ); - // The un-added agent is demoted from p-tag to a reference mention. - expect(sentMentionPubkeys, isEmpty); + expect(sentMentionPubkeys, contains(agentPubkey)); expect( sentMediaTags, - contains(orderedEquals(['mention', agentPubkey])), + isNot(contains(orderedEquals(['mention', agentPubkey]))), ); - expect(find.text(privateChannelAddDeniedMessage), findsOneWidget); + expect(find.text(privateChannelAddDeniedMessage), findsNothing); }, ); From 87fb7db53d436c3cd1f567a225d4a088f58b6fe8 Mon Sep 17 00:00:00 2001 From: Wes Date: Mon, 10 Aug 2026 11:06:23 -0600 Subject: [PATCH 2/2] test(channels): verify invitation membership writes Assert the relay-signed authoritative member list after every ordinary-role matrix invitation so accepted events cannot hide DB side-effect failures. Co-authored-by: Carl Signed-off-by: Wes --- crates/buzz-test-client/tests/e2e_relay.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/crates/buzz-test-client/tests/e2e_relay.rs b/crates/buzz-test-client/tests/e2e_relay.rs index ff7c831eb8d..2013875d9ab 100644 --- a/crates/buzz-test-client/tests/e2e_relay.rs +++ b/crates/buzz-test-client/tests/e2e_relay.rs @@ -2289,10 +2289,11 @@ async fn test_private_channel_any_member_can_invite() { for target_role in ["member", "guest", "bot"] { let target_keys = Keys::generate(); + let target_pubkey_hex = target_keys.public_key().to_hex(); let (accepted, msg) = add_member_with_role_ws( &mut actor_client, &channel_id, - &target_keys.public_key().to_hex(), + &target_pubkey_hex, target_role, actor_keys, ) @@ -2301,6 +2302,11 @@ async fn test_private_channel_any_member_can_invite() { accepted, "private-channel {actor_role} should add {target_role}, got: {msg}" ); + assert_eq!( + member_role(&url, &owner_keys, &channel_id, &target_pubkey_hex).await, + Some(target_role.to_string()), + "private-channel {actor_role} add must persist the {target_role} role" + ); } // Re-adding oneself stays idempotent — the huddle bot-add and kind:9021