fix: misidentified the connected user as a removed group member

Author: highesttt

pkg/connector/sync.go

@@ -1154,8 +1154,13 @@ func (lc *LineClient) cacheGroupMembersFromSystemMessage(msg *line.Message) {
 		lc.addGroupMembersToCache(chatMid, append(midsFromSystemLocArgs(msg.ContentMetadata["LOC_ARGS"]), msg.From)...)
 	case "C_MJ", "A_MJ":
 		lc.addGroupMembersToCache(chatMid, msg.From)
-	case "C_ML", "A_ML", "C_MR", "A_MR":
+	case "C_ML", "A_ML":
 		lc.removeGroupMemberFromCache(chatMid, msg.From)
+	case "C_MR", "A_MR":
+		_, removedMID, ok := memberRemovalMIDs(msg)
+		if ok {
+			lc.removeGroupMemberFromCache(chatMid, removedMID)
+		}
 	case "C_IC":
 		parts := strings.SplitN(msg.ContentMetadata["LOC_ARGS"], "\x1e", 2)
 		if len(parts) == 2 {
@@ -1207,6 +1212,23 @@ func midsFromSystemLocArgs(locArgs string) []string {
 	return mids
 }
 
+// memberRemovalMIDs returns the actor and target encoded by LINE's C_MR/A_MR
+// system records. msg.From is the member who performed the removal, while
+// LOC_ARGS contains the removed member MID (and may also repeat the actor MID).
+func memberRemovalMIDs(msg *line.Message) (removerMID, removedMID string, ok bool) {
+	if msg == nil || !isUserMID(msg.From) || msg.ContentMetadata == nil {
+		return "", "", false
+	}
+	removerMID = msg.From
+	mids := midsFromSystemLocArgs(msg.ContentMetadata["LOC_ARGS"])
+	for i := len(mids) - 1; i >= 0; i-- {
+		if mids[i] != removerMID {
+			return removerMID, mids[i], true
+		}
+	}
+	return "", "", false
+}
+
 func isUserMID(mid string) bool {
 	return len(mid) > 1 && (mid[0] == 'U' || mid[0] == 'u')
 }
@@ -2374,8 +2396,11 @@ func isHandledSystemMessage(msg *line.Message) bool {
 	case "C_PN":
 		parts := strings.SplitN(msg.ContentMetadata["LOC_ARGS"], "\x1e", 2)
 		return len(parts) == 2 && parts[1] != ""
-	case "C_MJ", "A_MJ", "C_ML", "A_ML", "C_MR", "A_MR", "A_MC":
+	case "C_MJ", "A_MJ", "C_ML", "A_ML", "A_MC":
 		return msg.From != ""
+	case "C_MR", "A_MR":
+		_, _, ok := memberRemovalMIDs(msg)
+		return ok
 	case "C_GI", "C_MI", "A_MI", "C_IC":
 		parts := strings.SplitN(msg.ContentMetadata["LOC_ARGS"], "\x1e", 2)
 		return len(parts) == 2 && parts[1] != ""
@@ -2431,12 +2456,18 @@ func (lc *LineClient) makeSystemMessageEvent(op line.Operation) (*simplevent.Cha
 		leaver := lc.eventSenderForMID(msg.From)
 		return makeMemberChangeEvent(portalKey, leaver, leaver, event.MembershipLeave, ts, false), true
 	case "C_MR", "A_MR":
-		lc.UserLogin.Bridge.Log.Debug().Str("loc_key", locKey).Str("chat_mid", msg.To).Str("removed_mid", msg.From).Msg("System message: member removed")
-		lc.removeGroupMemberFromCache(msg.To, msg.From)
+		removerMID, removedMID, _ := memberRemovalMIDs(msg)
+		lc.UserLogin.Bridge.Log.Debug().
+			Str("loc_key", locKey).
+			Str("chat_mid", msg.To).
+			Str("remover_mid", removerMID).
+			Str("removed_mid", removedMID).
+			Msg("System message: member removed")
+		lc.removeGroupMemberFromCache(msg.To, removedMID)
 		return makeMemberChangeEvent(
 			portalKey,
-			lc.eventSenderForMID(msg.From),
-			bridgev2.EventSender{},
+			lc.eventSenderForMID(removedMID),
+			lc.eventSenderForMID(removerMID),
 			event.MembershipLeave,
 			ts,
 			false,

pkg/connector/sync_test.go

@@ -6,6 +6,7 @@ import (
 	"encoding/json"
 	"errors"
 	"io"
+	"slices"
 	"strings"
 	"testing"
 	"time"
@@ -122,6 +123,153 @@ func TestMakeSystemMessageEventForHistoricalMembership(t *testing.T) {
 	}
 }
 
+func TestMemberRemovalSystemMessageAttribution(t *testing.T) {
+	tests := []struct {
+		name           string
+		locKey         string
+		fromMID        string
+		locArgs        string
+		removedMID     string
+		senderIsFromMe bool
+	}{
+		{
+			name:           "connected user removes another member",
+			locKey:         "C_MR",
+			fromMID:        "Uself",
+			locArgs:        "Uself\x1eUremoved",
+			removedMID:     "Uremoved",
+			senderIsFromMe: true,
+		},
+		{
+			name:       "another member removes connected user",
+			locKey:     "A_MR",
+			fromMID:    "Uremover",
+			locArgs:    "Uself",
+			removedMID: "Uself",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			lc := &LineClient{
+				Mid: "Uself",
+				UserLogin: &bridgev2.UserLogin{
+					UserLogin: &database.UserLogin{ID: "Uself"},
+					Bridge:    &bridgev2.Bridge{Log: zerolog.Nop()},
+				},
+				groupMemberCache: map[string][]string{
+					"Cgroup": {"Uself", "Uremover", "Uremoved"},
+				},
+			}
+
+			removal, handled := lc.makeSystemMessageEvent(line.Operation{Message: &line.Message{
+				ID:          "remove-message",
+				From:        test.fromMID,
+				To:          "Cgroup",
+				ContentType: int(ContentSystem),
+				CreatedTime: json.Number("1784930400123"),
+				ContentMetadata: map[string]string{
+					"LOC_KEY":  test.locKey,
+					"LOC_ARGS": test.locArgs,
+				},
+			}})
+			if !handled || removal == nil {
+				t.Fatalf("removal handled/event = %v/%#v, want true/non-nil", handled, removal)
+			}
+			if removal.EventMeta.Sender.Sender != networkid.UserID(test.fromMID) || removal.EventMeta.Sender.IsFromMe != test.senderIsFromMe {
+				t.Fatalf("removal sender = %#v, want MID %q with IsFromMe=%v", removal.EventMeta.Sender, test.fromMID, test.senderIsFromMe)
+			}
+			if changes := removal.ChatInfoChange.MemberChanges.MemberMap; len(changes) != 1 {
+				t.Fatalf("member changes = %#v, want exactly one", changes)
+			} else if change, ok := changes[networkid.UserID(test.removedMID)]; !ok {
+				t.Fatalf("removed member %q missing from %#v", test.removedMID, changes)
+			} else if change.Membership != event.MembershipLeave || change.EventSender.Sender != networkid.UserID(test.removedMID) {
+				t.Fatalf("removed member change = %#v", change)
+			}
+
+			members := lc.getCachedGroupMembers("Cgroup")
+			if slices.Contains(members, test.removedMID) {
+				t.Fatalf("removed member %q remained cached: %v", test.removedMID, members)
+			}
+			if test.fromMID != test.removedMID && !slices.Contains(members, test.fromMID) {
+				t.Fatalf("remover %q was incorrectly evicted: %v", test.fromMID, members)
+			}
+		})
+	}
+}
+
+func TestMemberRemovalSystemMessageCacheTarget(t *testing.T) {
+	for _, locKey := range []string{"C_MR", "A_MR"} {
+		t.Run(locKey, func(t *testing.T) {
+			lc := &LineClient{
+				Mid: "Uself",
+				groupMemberCache: map[string][]string{
+					"Cgroup": {"Uself", "Uremoved"},
+				},
+			}
+			lc.cacheGroupMembersFromSystemMessage(&line.Message{
+				From:        "Uself",
+				To:          "Cgroup",
+				ContentType: int(ContentSystem),
+				ContentMetadata: map[string]string{
+					"LOC_KEY":  locKey,
+					"LOC_ARGS": "Uself\x1eUremoved",
+				},
+			})
+
+			members := lc.getCachedGroupMembers("Cgroup")
+			if slices.Contains(members, "Uremoved") {
+				t.Fatalf("removed member remained cached: %v", members)
+			}
+			if !slices.Contains(members, "Uself") {
+				t.Fatalf("connected remover was incorrectly evicted: %v", members)
+			}
+		})
+	}
+}
+
+func TestMemberRemovalSystemMessageValidation(t *testing.T) {
+	tests := []struct {
+		name    string
+		fromMID string
+		locArgs string
+	}{
+		{name: "missing target", fromMID: "Uremover"},
+		{name: "only repeats remover", fromMID: "Uremover", locArgs: "Uremover"},
+		{name: "target is not a user MID", fromMID: "Uremover", locArgs: "not-a-mid"},
+		{name: "remover is not a user MID", fromMID: "Cgroup", locArgs: "Uremoved"},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			msg := &line.Message{
+				From:        test.fromMID,
+				To:          "Cgroup",
+				ContentType: int(ContentSystem),
+				ContentMetadata: map[string]string{
+					"LOC_KEY":  "C_MR",
+					"LOC_ARGS": test.locArgs,
+				},
+			}
+			if isHandledSystemMessage(msg) {
+				t.Fatal("malformed removal was unexpectedly accepted")
+			}
+		})
+	}
+
+	valid := &line.Message{
+		From:        "Uremover",
+		To:          "Cgroup",
+		ContentType: int(ContentSystem),
+		ContentMetadata: map[string]string{
+			"LOC_KEY":  "A_MR",
+			"LOC_ARGS": "Uremoved",
+		},
+	}
+	if !isHandledSystemMessage(valid) {
+		t.Fatal("valid removal with a distinct target was rejected")
+	}
+}
+
 func TestHandledSystemMessageRejectsUnsupportedRecords(t *testing.T) {
 	tests := []struct {
 		name string