Verify groupId for group changes

Co-authored-by: ayumi-signal <143036029+ayumi-signal@users.noreply.github.com>

package-lock.json

@@ -124,7 +124,7 @@
         "@indutny/rezip-electron": "2.0.1",
         "@indutny/symbolicate-mac": "2.3.0",
         "@napi-rs/canvas": "0.1.61",
-        "@signalapp/mock-server": "10.0.0",
+        "@signalapp/mock-server": "10.0.1",
         "@storybook/addon-a11y": "8.4.4",
         "@storybook/addon-actions": "8.4.4",
         "@storybook/addon-controls": "8.4.4",
@@ -6027,9 +6027,9 @@
       }
     },
     "node_modules/@signalapp/mock-server": {
-      "version": "10.0.0",
+      "version": "10.0.1",
-      "resolved": "https://registry.npmjs.org/@signalapp/mock-server/-/mock-server-10.0.0.tgz",
+      "resolved": "https://registry.npmjs.org/@signalapp/mock-server/-/mock-server-10.0.1.tgz",
-      "integrity": "sha512-gvUnWY4qCR5LUhuNbJWsZTj1Rjnu9pl8QMWSHU91TyBHwjABzgY6fKTTALf2ZCxcGY8FH5sObbP8kH8nq5o3bw==",
+      "integrity": "sha512-g2gOA58phCORrgXMR/rz6kia9HV99onYsSzhAepfBkGSugjBOySku1Z1uPdJ5wNZiOtdjZPcrBRxD0WaSRKZXg==",
       "dev": true,
       "license": "AGPL-3.0-only",
       "dependencies": {
@@ -6064,23 +6064,6 @@
         "uuid": "^8.3.0"
       }
     },
-    "node_modules/@signalapp/mock-server/node_modules/debug": {
-      "version": "4.3.4",
-      "resolved": "https://registry.npmjs.org/debug/-/debug-4.3.4.tgz",
-      "integrity": "sha512-PRWFHuSU3eDtQJPvnNY7Jcket1j0t5OuOsFzPPzsekD52Zl8qUfFIPEiswXqIvHWGVHOgX+7G/vCNNhehwxfkQ==",
-      "dev": true,
-      "dependencies": {
-        "ms": "2.1.2"
-      },
-      "engines": {
-        "node": ">=6.0"
-      },
-      "peerDependenciesMeta": {
-        "supports-color": {
-          "optional": true
-        }
-      }
-    },
     "node_modules/@signalapp/mock-server/node_modules/is-plain-obj": {
       "version": "3.0.0",
       "resolved": "https://registry.npmjs.org/is-plain-obj/-/is-plain-obj-3.0.0.tgz",
@@ -6100,13 +6083,6 @@
       "dev": true,
       "license": "Apache-2.0"
     },
-    "node_modules/@signalapp/mock-server/node_modules/ms": {
-      "version": "2.1.2",
-      "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.2.tgz",
-      "integrity": "sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w==",
-      "dev": true,
-      "license": "MIT"
-    },
     "node_modules/@signalapp/mock-server/node_modules/uuid": {
       "version": "8.3.2",
       "resolved": "https://registry.npmjs.org/uuid/-/uuid-8.3.2.tgz",

package.json

@@ -208,7 +208,7 @@
     "@indutny/rezip-electron": "2.0.1",
     "@indutny/symbolicate-mac": "2.3.0",
     "@napi-rs/canvas": "0.1.61",
-    "@signalapp/mock-server": "10.0.0",
+    "@signalapp/mock-server": "10.0.1",
     "@storybook/addon-a11y": "8.4.4",
     "@storybook/addon-actions": "8.4.4",
     "@storybook/addon-controls": "8.4.4",

protos/Groups.proto

@@ -190,6 +190,9 @@ message GroupChange {
 
              bytes                                      sourceUserId                       = 1; // Who made the change
              uint32                                     version                            = 2; // The change version number
+    // clients should not provide this value; the server will provide it in the response buffer to ensure the signature is binding to a particular group
+    // if clients set it during a request the server will respond with 400.
+    bytes                                               groupId                            = 25;
     repeated AddMemberAction                            addMembers                         = 3; // Members added
     repeated DeleteMemberAction                         deleteMembers                      = 4; // Members deleted
     repeated ModifyMemberRoleAction                     modifyMemberRoles                  = 5; // Modified member roles
@@ -212,7 +215,7 @@ message GroupChange {
     repeated AddMemberBannedAction                      addMembersBanned                   = 22; // change epoch = 4
     repeated DeleteMemberBannedAction                   deleteMembersBanned                = 23; // change epoch = 4
     repeated PromoteMemberPendingPniAciProfileKeyAction promoteMembersPendingPniAciProfileKey = 24;  // change epoch = 5
-    // next: 25
+    // next: 26
   }
 
   bytes  actions         = 1; // The serialized actions

ts/groups.ts

@@ -3536,7 +3536,10 @@ async function getGroupUpdates({
       groupChange.changeEpoch <= SUPPORTED_CHANGE_EPOCH;
 
     if (isChangeSupported) {
-      if (!wrappedGroupChange.isTrusted) {
+      const { isTrusted } = wrappedGroupChange;
+      let isUntrustedChangeVerified = false;
+
+      if (!isTrusted) {
         strictAssert(
           groupChange.serverSignature,
           'Server signature must be present in untrusted group change'
@@ -3563,14 +3566,35 @@ async function getGroupUpdates({
             newProfileKeys: new Map(),
           };
         }
+
+        const { groupId: groupIdBytes } = Proto.GroupChange.Actions.decode(
+          groupChange.actions || new Uint8Array(0)
+        );
+        const actionsGroupId: string | undefined =
+          groupIdBytes && groupIdBytes.length !== 0
+            ? Bytes.toBase64(groupIdBytes)
+            : undefined;
+        if (actionsGroupId && actionsGroupId === group.groupId) {
+          isUntrustedChangeVerified = true;
+        } else if (!actionsGroupId) {
+          log.warn(
+            `getGroupUpdates/${logId}: Missing groupId in group change actions`
+          );
+        } else {
+          log.warn(
+            `getGroupUpdates/${logId}: Incorrect groupId in group change actions`
+          );
+        }
       }
 
+      if (isTrusted || isUntrustedChangeVerified) {
         return updateGroupViaSingleChange({
           group,
           newRevision,
           groupChange,
         });
       }
+    }
 
     log.info(
       `getGroupUpdates/${logId}: Failing over; group change unsupported`