From 8ec4b424cbb794ceb5ade552bc641ad710e1bcca Mon Sep 17 00:00:00 2001 From: spaced4ndy <8711996+spaced4ndy@users.noreply.github.com> Date: Wed, 22 Jul 2026 19:39:05 +0000 Subject: [PATCH] core: prohibit to change or assign relay role (#7284) --- src/Simplex/Chat/Library/Commands.hs | 23 ++++++++++++----------- src/Simplex/Chat/Library/Internal.hs | 6 ++---- src/Simplex/Chat/Library/Subscriber.hs | 3 +++ 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/Simplex/Chat/Library/Commands.hs b/src/Simplex/Chat/Library/Commands.hs index 4d1de3bc72..9b36af9327 100644 --- a/src/Simplex/Chat/Library/Commands.hs +++ b/src/Simplex/Chat/Library/Commands.hs @@ -2880,14 +2880,14 @@ processChatCommand cxt nm = \case -- TODO [relays] possible optimization is to read only required members + relays g@(Group gInfo members) <- withFastStore $ \db -> getGroup db cxt user groupId when (selfSelected gInfo) $ throwCmdError "can't change role for self" - let (invitedMems, currentMems, unchangedMems, maxRole, anyAdmin, anyPending, anyPrivilegedTarget, anyRosterChange, finalPrivilegedCount) = selectMembers members + let (invitedMems, currentMems, unchangedMems, maxRole, anyAdmin, anyPending, anyPrivilegedTarget, anyRelay, anyRosterChange, finalPrivilegedCount) = selectMembers members when (length invitedMems + length currentMems + length unchangedMems /= length memberIds) $ throwChatError CEGroupMemberNotFound when (length memberIds > 1 && (anyAdmin || newRole >= GRAdmin)) $ throwCmdError "can't change role of multiple members when admins selected, or new role is admin" when anyPending $ throwCmdError "can't change role of members pending approval" - -- TODO allow moderators (recipients already accept it; needs UI too): the observer..member limit is an - -- TODO `all` over targets - maxRole can't express it, a max hides targets below GRObserver (relay, - -- TODO unknown) that receivers reject. Fold roleRequiredToChange per target, or add allModeratable. + when (anyRelay || newRole == GRRelay) $ throwCmdError "relay role can't be changed" + -- TODO allow moderators (needs UI) - relay is rejected above (anyRelay), so drop the GRAdmin floor: + -- TODO assertUserGroupRole gInfo (roleRequiredToChange maxRole newRole) assertUserGroupRole gInfo $ maximum ([GRAdmin, maxRole, newRole] :: [GroupMemberRole]) -- in relay groups the roster has a single signer, so only the owner may change member/moderator/admin roles when (useRelays' gInfo && (isRosterRole newRole || anyPrivilegedTarget) && memberRole' (membership gInfo) /= GROwner) $ @@ -2908,22 +2908,23 @@ processChatCommand cxt nm = \case -- anyPrivilegedTarget: a target currently member/moderator/admin (gates the owner-only check); anyRosterChange: -- a current member's role change that alters the roster blob - the only case that bumps the version, since a -- bump with no delta reads as a gap to subscribers; finalPrivilegedCount: moderators + admins after the change. - selectMembers :: [GroupMember] -> ([GroupMember], [GroupMember], [GroupMember], GroupMemberRole, Bool, Bool, Bool, Bool, Int) - selectMembers = foldr' addMember ([], [], [], GRObserver, False, False, False, False, 0) + selectMembers :: [GroupMember] -> ([GroupMember], [GroupMember], [GroupMember], GroupMemberRole, Bool, Bool, Bool, Bool, Bool, Int) + selectMembers = foldr' addMember ([], [], [], GRObserver, False, False, False, False, False, 0) where - addMember m@GroupMember {groupMemberId, memberStatus, memberRole} (invited, current, unchanged, maxRole, anyAdmin, anyPending, anyPrivTarget, anyRosterChange, privCount) + addMember m@GroupMember {groupMemberId, memberStatus, memberRole} (invited, current, unchanged, maxRole, anyAdmin, anyPending, anyPrivTarget, anyRelay, anyRosterChange, privCount) | groupMemberId `elem` memberIds = let maxRole' = max maxRole memberRole anyAdmin' = anyAdmin || memberRole >= GRAdmin anyPending' = anyPending || memberPending m anyPrivTarget' = anyPrivTarget || isRosterRole memberRole + anyRelay' = anyRelay || memberRole == GRRelay privCount' = if isRosterRole newRole then privCount + 1 else privCount in if - | memberRole == newRole -> (invited, current, m : unchanged, maxRole', anyAdmin', anyPending', anyPrivTarget', anyRosterChange, privCount') - | memberStatus == GSMemInvited -> (m : invited, current, unchanged, maxRole', anyAdmin', anyPending', anyPrivTarget', anyRosterChange, privCount') + | memberRole == newRole -> (invited, current, m : unchanged, maxRole', anyAdmin', anyPending', anyPrivTarget', anyRelay', anyRosterChange, privCount') + | memberStatus == GSMemInvited -> (m : invited, current, unchanged, maxRole', anyAdmin', anyPending', anyPrivTarget', anyRelay', anyRosterChange, privCount') -- a current member's role actually changes here; it alters the roster iff the old or new role is on it - | otherwise -> (invited, m : current, unchanged, maxRole', anyAdmin', anyPending', anyPrivTarget', anyRosterChange || isRosterRole newRole || isRosterRole memberRole, privCount') - | otherwise = (invited, current, unchanged, maxRole, anyAdmin, anyPending, anyPrivTarget, anyRosterChange, if isRosterRole memberRole then privCount + 1 else privCount) + | otherwise -> (invited, m : current, unchanged, maxRole', anyAdmin', anyPending', anyPrivTarget', anyRelay', anyRosterChange || isRosterRole newRole || isRosterRole memberRole, privCount') + | otherwise = (invited, current, unchanged, maxRole, anyAdmin, anyPending, anyPrivTarget, anyRelay, anyRosterChange, if isRosterRole memberRole then privCount + 1 else privCount) changeRoleInvitedMems :: User -> GroupInfo -> [GroupMember] -> CM ([ChatError], [GroupMember]) changeRoleInvitedMems user gInfo memsToChange = do -- not batched, as we need to send different invitations to different connections anyway diff --git a/src/Simplex/Chat/Library/Internal.hs b/src/Simplex/Chat/Library/Internal.hs index 7bddfcaba5..63866b8a58 100644 --- a/src/Simplex/Chat/Library/Internal.hs +++ b/src/Simplex/Chat/Library/Internal.hs @@ -1288,13 +1288,11 @@ isRosterRole r = r == GRMember || r == GRModerator || r == GRAdmin isPrivilegedRole :: GroupMemberRole -> Bool isPrivilegedRole r = r >= GRMember --- Minimum role allowed to change a member's role from `from` to `to` (moderators only within observer..member). +-- Minimum role allowed to change a member's role from `from` to `to` (moderators only up to member; relay checked separately). roleRequiredToChange :: GroupMemberRole -> GroupMemberRole -> GroupMemberRole roleRequiredToChange from to - | moderatable from && moderatable to = GRModerator + | from <= GRMember && to <= GRMember = GRModerator | otherwise = maximum ([GRAdmin, from, to] :: [GroupMemberRole]) - where - moderatable r = GRObserver <= r && r <= GRMember -- Drop non-privileged-role entries and de-duplicate by memberId, keeping the first. -- Runs on the parsed roster blob. diff --git a/src/Simplex/Chat/Library/Subscriber.hs b/src/Simplex/Chat/Library/Subscriber.hs index 78ff8f85cc..949a9c8dfe 100644 --- a/src/Simplex/Chat/Library/Subscriber.hs +++ b/src/Simplex/Chat/Library/Subscriber.hs @@ -3337,6 +3337,7 @@ processAgentMessageConn cxt user@User {userId} corrId agentConnId agentMessage = xGrpMemRole :: GroupInfo -> Maybe GroupMember -> GroupMember -> MemberId -> GroupMemberRole -> Maybe MemberKey -> Maybe VersionRoster -> RcvMessage -> UTCTime -> CM (Maybe DeliveryJobScope) xGrpMemRole gInfo@GroupInfo {membership} fwdRelay_ m@GroupMember {memberRole = senderRole} memId memRole memberKey_ rosterVer_ msg@RcvMessage {msgSigned} brokerTs + | memRole == GRRelay = messageError "x.grp.mem.role: relay role can't be assigned" $> Nothing | membershipMemId == memId = applyAtRosterVersion gInfo fwdRelay_ m rosterVer_ $ let gInfo' = gInfo {membership = membership {memberRole = memRole}} @@ -3363,6 +3364,8 @@ processAgentMessageConn cxt user@User {userId} corrId agentConnId agentMessage = -- applyMember writes the change (role, or role + pinned key for a freshly TOFU-created member); -- the delivery scope (relay forwarding) is computed on the pre-change role changeMemberRole gInfo' member@GroupMember {memberRole = fromRole} created applyMember gEvent createItem + | fromRole == GRRelay = + messageError "x.grp.mem.role: relay role can't be changed" $> Nothing | senderRole < roleRequiredToChange fromRole memRole = messageError "x.grp.mem.role with insufficient member permissions" $> Nothing | useRelays' gInfo && (isRosterRole memRole || isRosterRole fromRole) && senderRole /= GROwner =