Merge branch 'main' into gnuxie/config-bye-bye

src/Mjolnir.ts

@@ -103,6 +103,15 @@ export class Mjolnir {
     private webapis: WebAPIs;
     private protectedRoomActivityTracker: ProtectedRoomActivityTracker;
     public taskQueue: ThrottlingQueue;
+    /**
+     * Used to provide mutual exclusion when synchronizing rooms with the state of a policy list.
+     * This is because requests operating with rules from an older version of the list that are slow
+     * could race & give the room an inconsistent state. An example is if we add multiple m.policy.rule.server rules,
+     * which would cause several requests to a room to send a new m.room.server_acl event.
+     * These requests could finish in any order, which has left rooms with an inconsistent server_acl event
+     * until Mjolnir synchronises the room with its policy lists again, which can be in the region of hours.
+     */
+    public aclChain: Promise<void> = Promise.resolve();
     /*
      * Config-enabled polling of reports in Synapse, so Mjolnir can react to reports
      */

src/actions/ApplyAcl.ts

@@ -30,6 +30,16 @@ import { ERROR_KIND_FATAL, ERROR_KIND_PERMISSION } from "../ErrorCache";
  * @param {Mjolnir} mjolnir The Mjolnir client to apply the ACLs with.
  */
 export async function applyServerAcls(lists: PolicyList[], roomIds: string[], mjolnir: Mjolnir): Promise<RoomUpdateError[]> {
+    // we need to provide mutual exclusion so that we do not have requests updating the m.room.server_acl event
+    // finish out of order and therefore leave the room out of sync with the policy lists.
+    return new Promise((resolve, reject) => {
+        mjolnir.aclChain = mjolnir.aclChain
+            .then(() => _applyServerAcls(lists, roomIds, mjolnir))
+            .then(resolve, reject);
+    });
+}
+
+async function _applyServerAcls(lists: PolicyList[], roomIds: string[], mjolnir: Mjolnir): Promise<RoomUpdateError[]> {
     const serverName: string = new UserID(await mjolnir.client.getUserId()).domain;
 
     // Construct a server ACL first

test/integration/banListTest.ts

@@ -260,20 +260,23 @@ describe('Test: ACL updates will batch when rules are added in succession.', fun
         mjolnir.client.joinRoom(banListId);
         mjolnir.watchList(Permalinks.forRoom(banListId));
         const acl = new ServerAcl(serverName).denyIpAddresses().allowServer("*");
-        for (let i = 0; i < 200; i++) {
+        const evilServerCount = 200;
+        for (let i = 0; i < evilServerCount; i++) {
             const badServer = `${i}.evil.com`;
             acl.denyServer(badServer);
             await createPolicyRule(moderator, banListId, RULE_SERVER, badServer, `Rule #${i}`);
             // Give them a bit of a spread over time.
             await new Promise(resolve => setTimeout(resolve, 5));
         }
-        // give the events a chance to appear in the response to `/state`, since this is a problem.
-        await new Promise(resolve => setTimeout(resolve, 2000));
-
         // We do this because it should force us to wait until all the ACL events have been applied.
         // Even if that does mean the last few events will not go through batching...
         await mjolnir.syncLists();
 
+        // At this point we check that the state within Mjolnir is internally consistent, this is just because debugging the following
+        // is a pita.
+        const list: PolicyList = this.mjolnir.policyLists[0]!;
+        assert.equal(list.serverRules.length, evilServerCount, `There should be ${evilServerCount} rules in here`);
+
         // Check each of the protected rooms for ACL events and make sure they were batched and are correct.
         await Promise.all(protectedRooms.map(async room => {
             const roomAcl = await mjolnir.client.getRoomStateEvent(room, "m.room.server_acl", "");