fix(channels): restrict private-channel invitations (#4612)

This change requires an active owner or administrator for third-party additions to private channels. The relay validator and transactional database authority enforce the same rule, including removed-member reactivation and role-change paths. Idempotent self-target behavior remains available, while ordinary members can no longer extend private-channel access to another identity. ## Testing - `git diff --check origin/main...codex/security-private-channel-invite-authority` - Rebased onto `origin/main` at `5c98932` - Full CI pending Originating Buzz thread: `buzz://message?channel=3928fe05-df61-4b5d-b9c7-d623b9b10ea1&id=3c6c02312f763fbe0d2bfc33a6c1a362f91d0354f3d18b039cf7a0558c1439d1` --------- Signed-off-by: Jordan Mecom <jm@squareup.com> Signed-off-by: Eli Foster <efoster@squareup.com> Co-authored-by: Eli Foster <efoster@squareup.com>
2026-08-18 06:50:31 +02:00 · 2026-08-05 10:47:00 -07:00
parent ad538bfb1e
commit efe1893dd3
19 changed files with 858 additions and 190 deletions
@@ -2,6 +2,11 @@ import 'package:flutter/foundation.dart';

 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.';
+
@immutable
 class Channel {
  final String id;
@@ -77,6 +82,17 @@ class Channel {
  bool get isForum => channelType == 'forum';
  bool get isDm => channelType == 'dm';
  bool get isPrivate => visibility == 'private';
+
+  /// 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.
+  bool canAddMembers(String? selfRole) {
+    if (isDm) return false;
+    if (visibility == 'open') return true;
+    return selfRole == 'owner' || selfRole == 'admin';
+  }
+
  bool get isArchived => archivedAt != null;

  String displayLabel({String? currentPubkey}) {
@@ -13,6 +13,31 @@ import '../profile/profile_provider.dart';
 import 'channel.dart';
 import 'channels_provider.dart';

+String _relayErrorMessage(Object error) =>
+    error.toString().replaceFirst('Exception: ', '');
+
+/// Raised when one or more kind:9000 adds were rejected, keyed by pubkey.
+///
+/// Callers surface [message] to the user — a relay rejection here (e.g. a plain
+/// member trying to add someone to a private channel) is a real outcome, not a
+/// crash to swallow.
+@immutable
+class AddMembersException implements Exception {
+  final Map<String, String> failures;
+
+  const AddMembersException(this.failures);
+
+  String get message => failures.entries
+      .map(
+        (entry) =>
+            '${entry.key.length > 8 ? '${entry.key.substring(0, 8)}…' : entry.key}: ${entry.value}',
+      )
+      .join('; ');
+
+  @override
+  String toString() => 'AddMembersException($message)';
+}
+
@immutable
 class ChannelMember {
  final String pubkey;
@@ -565,21 +590,34 @@ class ChannelActions {
        if (pubkey.trim().isNotEmpty) pubkey.trim().toLowerCase(),
    };
    _ensureCommunityValid();
+    // Per-pubkey failures are collected rather than thrown on the spot: one
+    // relay rejection must not skip the remaining adds or the invalidation
+    // below, which would leave the members list stale for the adds that landed.
+    final failures = <String, String>{};
    for (final pubkey in normalizedPubkeys) {
+      // Outside the catch: a community switch mid-loop must abort the whole
+      // add, not be recorded as this pubkey's rejection.
      _ensureCommunityValid();
-      await _signedEventRelay.submit(
-        kind: 9000,
-        content: '',
-        tags: [
-          ['h', channelId],
-          ['p', pubkey],
-          ['role', normalizedRole],
-        ],
-      );
+      try {
+        await _signedEventRelay.submit(
+          kind: 9000,
+          content: '',
+          tags: [
+            ['h', channelId],
+            ['p', pubkey],
+            ['role', normalizedRole],
+          ],
+        );
+      } catch (error) {
+        failures[pubkey] = _relayErrorMessage(error);
+      }
    }
    _ensureCommunityValid();
    _ref.invalidate(channelMembersProvider(channelId));
    _ref.invalidate(channelBotPubkeysProvider(channelId));
+    if (failures.isNotEmpty) {
+      throw AddMembersException(failures);
+    }
  }

  void _ensureCommunityValid() {
@@ -416,79 +416,37 @@ class ComposeBar extends HookConsumerWidget {
        for (final entry in mentionMap.value.entries)
          if (hasMention(text, entry.key)) entry.value,
      ];
-      final pubkeys = LinkedHashSet<String>.from(
-        selectedMentions.map((candidate) => candidate.pubkey.toLowerCase()),
-      ).toList();
-      final nonMemberAgentPubkeys = <String>[];
-      final nonMemberHumans = <MentionCandidate>[];
-      if (selectedMentions.isNotEmpty) {
-        final currentChannel = (await ref.read(
-          channelsProvider.future,
-        )).firstWhere((channel) => channel.id == channelId);
-        if (!currentChannel.isDm) {
-          final memberPubkeys = (await ref.read(
-            channelMembersProvider(channelId).future,
-          )).map((member) => member.pubkey.toLowerCase()).toSet();
-          final seenNonMembers = <String>{};
-          for (final candidate in selectedMentions) {
-            final pk = candidate.pubkey.toLowerCase();
-            if (memberPubkeys.contains(pk)) continue;
-            if (!seenNonMembers.add(pk)) continue;
-            if (candidate.isAgent) {
-              nonMemberAgentPubkeys.add(pk);
-            } else {
-              nonMemberHumans.add(candidate);
-            }
-          }
-        }
-      }
+      final outgoing = _OutgoingMentions(selectedMentions);
+      final scan = await _scanNonMemberMentions(
+        ref,
+        channelId: channelId,
+        selectedMentions: selectedMentions,
+        currentPubkey: currentPubkey,
+      );

      // Mentioning humans outside the channel prompts "Invite" / "Do
      // nothing" (send without inviting) — mirrors desktop's
      // NonMemberMentionDialog. Agents keep the existing silent auto-add.
-      var mentionPubkeys = pubkeys;
-      final referenceMentionTags = <List<String>>[];
-      var inviteHumanPubkeys = const <String>[];
-      if (nonMemberHumans.isNotEmpty) {
+      if (scan.humans.isNotEmpty) {
        if (!context.mounted) return;
        final choice = await _promptNonMemberMention(
          context,
-          names: [for (final candidate in nonMemberHumans) candidate.label],
+          names: [for (final candidate in scan.humans) candidate.label],
+          canInvite: scan.canAddMembers,
        );
-        switch (choice) {
-          case null:
-            return; // Dismissed — keep the draft, send nothing.
-          case _NonMemberMentionChoice.invite:
-            inviteHumanPubkeys = [
-              for (final candidate in nonMemberHumans)
-                candidate.pubkey.toLowerCase(),
-            ];
-          case _NonMemberMentionChoice.sendWithoutInviting:
-            // Strip their p-tags (no channel notification) but keep a
-            // `mention` reference tag so their name still renders —
-            // mirrors desktop's mergeOutgoingTagsWithReferenceMentions.
-            final excluded = {
-              for (final candidate in nonMemberHumans)
-                candidate.pubkey.toLowerCase(),
-            };
-            mentionPubkeys = [
-              for (final pk in pubkeys)
-                if (!excluded.contains(pk)) pk,
-            ];
-            referenceMentionTags.addAll([
-              for (final pk in excluded) ['mention', pk],
-            ]);
-        }
+        if (choice == null) return; // Dismissed — keep the draft, send nothing.
+        outgoing.resolveHumanChoice(choice, scan.humans);
      }

      final queuedAttachments = List<_PendingAttachment>.of(attachments.value);
      final channelActions = ref.read(channelActionsProvider);

-      Future<void> addMentionedNonMembers() => _addMentionedNonMembers(
+      // An add that was refused doesn't block the message: it is reported and
+      // the un-added mentions are demoted to reference tags so the send lands.
+      Future<void> addMentionedNonMembers() => outgoing.addNonMembers(
        channelActions,
-        channelId: channelId,
-        agentPubkeys: nonMemberAgentPubkeys,
-        humanPubkeys: inviteHumanPubkeys,
+        scan: scan,
+        messenger: messenger,
      );

      isSending.value = true;
@@ -503,12 +461,19 @@ class ComposeBar extends HookConsumerWidget {
            );
            await onSend(
              payload.content,
-              mentionPubkeys,
-              mediaTags: [...payload.mediaTags, ...referenceMentionTags],
+              outgoing.pubkeys,
+              mediaTags: [...payload.mediaTags, ...outgoing.referenceTags],
            );
            if (context.mounted) clearComposer();
          } on StateError {
            _reportSendCancelledByCommunitySwitch(messenger);
+          } catch (error) {
+            // send() runs unawaited, so a relay rejection or publish timeout
+            // would otherwise vanish with the composer looking idle. The draft
+            // is kept (clearComposer never ran) so the user can retry.
+            messenger?.showSnackBar(
+              SnackBar(content: Text(_composeSendErrorMessage(error))),
+            );
          }
          return;
        }
@@ -561,8 +526,8 @@ class ComposeBar extends HookConsumerWidget {
            if (queueGeneration != uploadGeneration.value) return;
            await delivery(
              payload.content,
-              mentionPubkeys,
-              mediaTags: [...payload.mediaTags, ...referenceMentionTags],
+              outgoing.pubkeys,
+              mediaTags: [...payload.mediaTags, ...outgoing.referenceTags],
            );
          } catch (error) {
            if (cancellation.isCancelled) return;
@@ -143,11 +143,21 @@ bool hasMention(String text, String name) {
 /// cancels the send and keeps the draft.
 enum _NonMemberMentionChoice { invite, sendWithoutInviting }

+/// User-facing text for a failed add or send.
+String _composeSendErrorMessage(Object error) {
+  if (error is AddMembersException) return error.message;
+  return error.toString().replaceFirst('Exception: ', '');
+}
+
 /// Ask whether to invite mentioned humans who aren't channel members, or
 /// send without inviting them. Mirrors desktop's `NonMemberMentionDialog`.
+/// [canInvite] false (a private channel the sender doesn't own/administer)
+/// drops the Invite action — the relay rejects that add, so offering it would
+/// only produce an error.
 Future<_NonMemberMentionChoice?> _promptNonMemberMention(
  BuildContext context, {
  required List<String> names,
+  required bool canInvite,
 }) {
  final verb = names.length == 1 ? 'is' : 'are';
  return showDialog<_NonMemberMentionChoice>(
@@ -155,21 +165,26 @@ Future<_NonMemberMentionChoice?> _promptNonMemberMention(
    builder: (dialogContext) => AlertDialog(
      title: const Text('Mention people outside this channel?'),
      content: Text(
-        '${names.join(', ')} $verb not in this channel. Invite them to '
-        'the channel, or send without inviting them.',
+        canInvite
+            ? '${names.join(', ')} $verb not in this channel. Invite them to '
+                  'the channel, or send without inviting them.'
+            : '${names.join(', ')} $verb not in this channel. '
+                  '$privateChannelAddDeniedMessage You can still send without '
+                  'inviting them.',
      ),
      actions: [
        TextButton(
          onPressed: () => Navigator.of(
            dialogContext,
          ).pop(_NonMemberMentionChoice.sendWithoutInviting),
-          child: const Text('Do nothing'),
-        ),
-        TextButton(
-          onPressed: () =>
-              Navigator.of(dialogContext).pop(_NonMemberMentionChoice.invite),
-          child: const Text('Invite'),
+          child: Text(canInvite ? 'Do nothing' : 'Send anyway'),
        ),
+        if (canInvite)
+          TextButton(
+            onPressed: () =>
+                Navigator.of(dialogContext).pop(_NonMemberMentionChoice.invite),
+            child: const Text('Invite'),
+          ),
      ],
    ),
  );
@@ -232,27 +247,204 @@ void _reportSendCancelledByCommunitySwitch(ScaffoldMessengerState? messenger) {
  );
 }

+/// What an add attempt left undone: who is still a non-member, and why.
+@immutable
+class _NonMemberAddOutcome {
+  final List<String> notAdded;
+  final List<String> errors;
+
+  const _NonMemberAddOutcome({required this.notAdded, required this.errors});
+
+  static const empty = _NonMemberAddOutcome(notAdded: [], errors: []);
+}
+
 /// Adds mentioned non-members to the channel before a send.
 ///
 /// Agents are added silently with the `bot` role; humans are only passed here
 /// after they have been explicitly invited from the mention prompt.
-Future<void> _addMentionedNonMembers(
+///
+/// A rejection is reported, never thrown: the send is fire-and-forget, so an
+/// escaping error would drop the message with nothing shown. [StateError] still
+/// propagates — a community switch must cancel the whole send.
+Future<_NonMemberAddOutcome> _addMentionedNonMembers(
  ChannelActions channelActions, {
  required String channelId,
  required List<String> agentPubkeys,
  required List<String> humanPubkeys,
+  required bool canAddMembers,
 }) async {
-  if (agentPubkeys.isNotEmpty) {
-    await channelActions.addMembers(
-      channelId: channelId,
-      pubkeys: agentPubkeys,
-      role: 'bot',
+  final pending = [
+    if (agentPubkeys.isNotEmpty) (agentPubkeys, 'bot'),
+    if (humanPubkeys.isNotEmpty) (humanPubkeys, 'member'),
+  ];
+  if (pending.isEmpty) return _NonMemberAddOutcome.empty;
+
+  // A plain member of a private channel cannot add anyone: skip the doomed
+  // kind:9000 rather than trading it for a relay rejection.
+  if (!canAddMembers) {
+    return _NonMemberAddOutcome(
+      notAdded: [for (final (pubkeys, _) in pending) ...pubkeys],
+      errors: const [privateChannelAddDeniedMessage],
    );
  }
-  if (humanPubkeys.isNotEmpty) {
-    await channelActions.addMembers(
-      channelId: channelId,
-      pubkeys: humanPubkeys,
+
+  final notAdded = <String>[];
+  final errors = <String>[];
+  for (final (pubkeys, role) in pending) {
+    try {
+      await channelActions.addMembers(
+        channelId: channelId,
+        pubkeys: pubkeys,
+        role: role,
+      );
+    } on StateError {
+      rethrow;
+    } catch (error) {
+      notAdded.addAll(
+        error is AddMembersException ? error.failures.keys : pubkeys,
+      );
+      errors.add(_composeSendErrorMessage(error));
+    }
+  }
+  return _NonMemberAddOutcome(notAdded: notAdded, errors: errors);
+}
+
+/// Mentioned identities that aren't in the channel yet, plus whether the sender
+/// is allowed to add them at all.
+@immutable
+class _NonMemberMentionScan {
+  final String channelId;
+  final List<String> agentPubkeys;
+  final List<MentionCandidate> humans;
+  final bool canAddMembers;
+
+  const _NonMemberMentionScan({
+    required this.channelId,
+    required this.agentPubkeys,
+    required this.humans,
+    required this.canAddMembers,
+  });
+}
+
+/// Resolves which mentioned identities are non-members, and whether this
+/// identity may add them (see [Channel.canAddMembers]). DMs are skipped: their
+/// participant set is fixed at creation.
+Future<_NonMemberMentionScan> _scanNonMemberMentions(
+  WidgetRef ref, {
+  required String channelId,
+  required List<MentionCandidate> selectedMentions,
+  required String? currentPubkey,
+}) async {
+  final none = _NonMemberMentionScan(
+    channelId: channelId,
+    agentPubkeys: const [],
+    humans: const [],
+    canAddMembers: true,
+  );
+  if (selectedMentions.isEmpty) return none;
+
+  final channel = (await ref.read(
+    channelsProvider.future,
+  )).firstWhere((candidate) => candidate.id == channelId);
+  if (channel.isDm) return none;
+
+  final members = await ref.read(channelMembersProvider(channelId).future);
+  final memberPubkeys = {
+    for (final member in members) member.pubkey.toLowerCase(),
+  };
+  String? selfRole;
+  if (currentPubkey != null) {
+    final self = currentPubkey.toLowerCase();
+    for (final member in members) {
+      if (member.pubkey.toLowerCase() == self) {
+        selfRole = member.role;
+        break;
+      }
+    }
+  }
+
+  final agentPubkeys = <String>[];
+  final humans = <MentionCandidate>[];
+  final seen = <String>{};
+  for (final candidate in selectedMentions) {
+    final pubkey = candidate.pubkey.toLowerCase();
+    if (memberPubkeys.contains(pubkey) || !seen.add(pubkey)) continue;
+    if (candidate.isAgent) {
+      agentPubkeys.add(pubkey);
+    } else {
+      humans.add(candidate);
+    }
+  }
+
+  return _NonMemberMentionScan(
+    channelId: channelId,
+    agentPubkeys: agentPubkeys,
+    humans: humans,
+    canAddMembers: channel.canAddMembers(selfRole),
+  );
+}
+
+/// The p-tags and `mention` reference tags an outgoing message should carry.
+///
+/// Anyone who ends up *not* added is demoted from a p-tag to a reference tag so
+/// their name still renders without notifying a non-member — mirrors desktop's
+/// `mergeOutgoingTagsWithReferenceMentions`.
+class _OutgoingMentions {
+  List<String> pubkeys;
+  final List<List<String>> referenceTags = [];
+  List<String> _invitedHumanPubkeys = const [];
+
+  _OutgoingMentions(List<MentionCandidate> selectedMentions)
+    : pubkeys = LinkedHashSet<String>.from(
+        selectedMentions.map((candidate) => candidate.pubkey.toLowerCase()),
+      ).toList();
+
+  void demote(Iterable<String> demoted) {
+    final excluded = {for (final pubkey in demoted) pubkey.toLowerCase()};
+    if (excluded.isEmpty) return;
+    pubkeys = [
+      for (final pubkey in pubkeys)
+        if (!excluded.contains(pubkey)) pubkey,
+    ];
+    referenceTags.addAll([
+      for (final pubkey in excluded) ['mention', pubkey],
+    ]);
+  }
+
+  /// Applies the mention prompt's outcome: invite them, or send without.
+  void resolveHumanChoice(
+    _NonMemberMentionChoice choice,
+    List<MentionCandidate> humans,
+  ) {
+    final humanPubkeys = [
+      for (final candidate in humans) candidate.pubkey.toLowerCase(),
+    ];
+    switch (choice) {
+      case _NonMemberMentionChoice.invite:
+        _invitedHumanPubkeys = humanPubkeys;
+      case _NonMemberMentionChoice.sendWithoutInviting:
+        demote(humanPubkeys);
+    }
+  }
+
+  /// Adds the scanned non-members, demoting and reporting whatever didn't land.
+  Future<void> addNonMembers(
+    ChannelActions channelActions, {
+    required _NonMemberMentionScan scan,
+    required ScaffoldMessengerState? messenger,
+  }) async {
+    final outcome = await _addMentionedNonMembers(
+      channelActions,
+      channelId: scan.channelId,
+      agentPubkeys: scan.agentPubkeys,
+      humanPubkeys: _invitedHumanPubkeys,
+      canAddMembers: scan.canAddMembers,
    );
+    demote(outcome.notAdded);
+    if (outcome.errors.isNotEmpty) {
+      messenger?.showSnackBar(
+        SnackBar(content: Text(outcome.errors.join(' '))),
+      );
+    }
  }
 }
@@ -195,4 +195,50 @@ void main() {
      expect(updated.archivedAt, newDate);
    });
  });
+
+  group('Channel.canAddMembers', () {
+    Channel make({required String channelType, required String visibility}) =>
+        Channel(
+          id: '1',
+          name: 'c',
+          channelType: channelType,
+          visibility: visibility,
+          description: '',
+          createdBy: 'x',
+          createdAt: DateTime(2025),
+          memberCount: 2,
+        );
+
+    test('open channels accept adds from anyone', () {
+      final channel = make(channelType: 'stream', visibility: 'open');
+      expect(channel.canAddMembers(null), isTrue);
+      expect(channel.canAddMembers('member'), isTrue);
+    });
+
+    test('private channels accept adds only from owners/admins', () {
+      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(null), isFalse);
+    });
+
+    test('DMs never accept adds', () {
+      expect(
+        make(channelType: 'dm', visibility: 'open').canAddMembers('owner'),
+        isFalse,
+      );
+      expect(
+        make(channelType: 'dm', visibility: 'private').canAddMembers('owner'),
+        isFalse,
+      );
+    });
+
+    test('unknown visibility fails closed for non-elevated callers', () {
+      final channel = make(channelType: 'stream', visibility: 'mystery');
+      expect(channel.canAddMembers('member'), isFalse);
+      expect(channel.canAddMembers('owner'), isTrue);
+    });
+  });
 }
@@ -3138,6 +3138,81 @@ void main() {
      expect(publishedEvents.where((event) => event['kind'] == 9000), isEmpty);
    });

+    testWidgets(
+      'skips the agent add in a private channel when not owner/admin',
+      (tester) async {
+        final agentPubkey = 'a' * 64;
+        final signer = nostr.Keys.generate();
+        final publishedEvents = <Map<String, dynamic>>[];
+        var didSend = false;
+        List<String> sentMentionPubkeys = const <String>[];
+        List<List<String>> sentMediaTags = const <List<String>>[];
+
+        await tester.pumpWidget(
+          _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.
+            members: [
+              ChannelMember(
+                pubkey: signer.public,
+                role: 'member',
+                joinedAt: DateTime(2024),
+              ),
+            ],
+            relayAgents: [_testAgent(agentPubkey)],
+            channels: [
+              _makeCurrentChannel(visibility: 'private'),
+              _makeSharedMemberChannel(),
+            ],
+            onSend:
+                (
+                  content,
+                  mentionPubkeys, {
+                  mediaTags = const <List<String>>[],
+                }) async {
+                  didSend = true;
+                  sentMentionPubkeys = mentionPubkeys;
+                  sentMediaTags = mediaTags;
+                },
+          ),
+        );
+
+        final container = ProviderScope.containerOf(
+          tester.element(find.byType(ComposeBar)),
+        );
+        final session = container.read(relaySessionProvider.notifier);
+        final socket = _RecordingRelaySocket(
+          publishedEvents,
+          session.debugHandleSocketMessageForTest,
+        );
+        session.debugAttachSocketForTest(socket);
+
+        await _expandComposer(tester);
+        await tester.enterText(find.byType(TextField), '@hel');
+        await tester.pumpAndSettle();
+        await tester.tap(find.text('Helper Bot'));
+        await tester.pumpAndSettle();
+        await tester.enterText(find.byType(TextField), 'hello @Helper Bot');
+        await tester.tap(find.byIcon(LucideIcons.arrowUp));
+        await tester.pumpAndSettle();
+
+        expect(didSend, isTrue);
+        expect(
+          publishedEvents.where((event) => event['kind'] == 9000),
+          isEmpty,
+        );
+        // The un-added agent is demoted from p-tag to a reference mention.
+        expect(sentMentionPubkeys, isEmpty);
+        expect(
+          sentMediaTags,
+          contains(orderedEquals(['mention', agentPubkey])),
+        );
+        expect(find.text(privateChannelAddDeniedMessage), findsOneWidget);
+      },
+    );
+
    testWidgets('adds a sanitized animated PNG attachment', (tester) async {
      final keychain = nostr.Keys.generate();
      final nsec = keychain.nsec;
@@ -3489,12 +3564,15 @@ List<({String text, TextStyle style})> _flattenStyledTextSpans(
  return result;
 }

-Channel _makeCurrentChannel({String channelType = 'stream'}) {
+Channel _makeCurrentChannel({
+  String channelType = 'stream',
+  String visibility = 'open',
+}) {
  return Channel(
    id: 'channel-1',
    name: 'current',
    channelType: channelType,
-    visibility: 'open',
+    visibility: visibility,
    description: '',
    createdBy: 'pubkey123',
    createdAt: DateTime(2024),