This should hopefully fix some startup woes (#462)

Splitting PolicyListManager from Mjolnir, making it more resilient to startup errors

src/Mjolnir.ts

@@ -19,7 +19,6 @@ import {
     LogLevel,
     LogService,
     MembershipEvent,
-    Permalinks,
 } from "matrix-bot-sdk";
 
 import { ALL_RULE_TYPES as ALL_BAN_LIST_RULE_TYPES } from "./models/ListRule";
@@ -32,7 +31,7 @@ import { WebAPIs } from "./webapis/WebAPIs";
 import RuleServer from "./models/RuleServer";
 import { ThrottlingQueue } from "./queues/ThrottlingQueue";
 import { getDefaultConfig, IConfig } from "./config";
-import PolicyList from "./models/PolicyList";
+import { PolicyListManager } from "./models/PolicyList";
 import { ProtectedRoomsSet } from "./ProtectedRoomsSet";
 import ManagementRoomOutput from "./ManagementRoomOutput";
 import { ProtectionManager } from "./protections/ProtectionManager";
@@ -45,9 +44,6 @@ export const STATE_CHECKING_PERMISSIONS = "checking_permissions";
 export const STATE_SYNCING = "syncing";
 export const STATE_RUNNING = "running";
 
-const WATCHED_LISTS_EVENT_TYPE = "org.matrix.mjolnir.watched_lists";
-const WARN_UNPROTECTED_ROOM_EVENT_PREFIX = "org.matrix.mjolnir.unprotected_room_warning.for.";
-
 /**
  * Synapse will tell us where we last got to on polling reports, so we need
  * to store that for pagination on further polls
@@ -86,6 +82,8 @@ export class Mjolnir {
      */
     public readonly reportManager: ReportManager;
 
+    public readonly policyListManager: PolicyListManager;
+
     /**
      * Adds a listener to the client that will automatically accept invitations.
      * @param {MatrixSendClient} client
@@ -143,7 +141,6 @@ export class Mjolnir {
         if (!config.autojoinOnlyIfManager && config.acceptInvitesFromSpace === getDefaultConfig().acceptInvitesFromSpace) {
             throw new TypeError("`autojoinOnlyIfManager` has been disabled, yet no space has been provided for `acceptInvitesFromSpace`.");
         }
-        const policyLists: PolicyList[] = [];
         const joinedRooms = await client.getJoinedRooms();
 
         // Ensure we're also in the management room
@@ -154,7 +151,7 @@ export class Mjolnir {
         }
 
         const ruleServer = config.web.ruleServer ? new RuleServer() : null;
-        const mjolnir = new Mjolnir(client, await client.getUserId(), matrixEmitter, managementRoomId, config, policyLists, ruleServer);
+        const mjolnir = new Mjolnir(client, await client.getUserId(), matrixEmitter, managementRoomId, config, ruleServer);
         await mjolnir.managementRoomOutput.logMessage(LogLevel.INFO, "index", "Mjolnir is starting up. Use !mjolnir to query status.");
         Mjolnir.addJoinOnInviteListener(mjolnir, client, config);
         return mjolnir;
@@ -166,11 +163,11 @@ export class Mjolnir {
         public readonly matrixEmitter: MatrixEmitter,
         public readonly managementRoomId: string,
         public readonly config: IConfig,
-        private policyLists: PolicyList[],
         // Combines the rules from ban lists so they can be served to a homeserver module or another consumer.
         public readonly ruleServer: RuleServer | null,
     ) {
         this.protectedRoomsConfig = new ProtectedRoomsConfig(client);
+        this.policyListManager = new PolicyListManager(this);
 
         // Setup bot.
 
@@ -247,10 +244,6 @@ export class Mjolnir {
         this.protectedRoomsTracker = new ProtectedRoomsSet(client, clientUserId, managementRoomId, this.managementRoomOutput, protections, config);
     }
 
-    public get lists(): PolicyList[] {
-        return this.policyLists;
-    }
-
     public get state(): string {
         return this.currentState;
     }
@@ -296,7 +289,7 @@ export class Mjolnir {
             this.protectedRoomsConfig.getExplicitlyProtectedRooms().forEach(this.protectRoom, this);
             // We have to build the policy lists before calling `resyncJoinedRooms` otherwise mjolnir will try to protect
             // every policy list we are already joined to, as mjolnir will not be able to distinguish them from normal rooms.
-            await this.buildWatchedPolicyLists();
+            await this.policyListManager.init();
             await this.resyncJoinedRooms(false);
             await this.protectionManager.start();
 
@@ -399,7 +392,7 @@ export class Mjolnir {
         // We filter out all policy rooms so that we only protect ones that are
         // explicitly protected, so that we don't try to protect lists that we are just watching.
         const filterOutManagementAndPolicyRooms = (roomId: string) => {
-            const policyListIds = this.policyLists.map(list => list.roomId);
+            const policyListIds = this.policyListManager.lists.map(list => list.roomId);
             return roomId !== this.managementRoomId && !policyListIds.includes(roomId);
         };
 
@@ -430,109 +423,6 @@ export class Mjolnir {
         }
     }
 
-
-    /**
-     * Helper for constructing `PolicyList`s and making sure they have the right listeners set up.
-     * @param roomId The room id for the `PolicyList`.
-     * @param roomRef A reference (matrix.to URL) for the `PolicyList`.
-     */
-    private async addPolicyList(roomId: string, roomRef: string): Promise<PolicyList> {
-        const list = new PolicyList(roomId, roomRef, this.client);
-        this.ruleServer?.watch(list);
-        await list.updateList();
-        this.policyLists.push(list);
-        this.protectedRoomsTracker.watchList(list);
-        return list;
-    }
-
-    public async watchList(roomRef: string): Promise<PolicyList | null> {
-        const joinedRooms = await this.client.getJoinedRooms();
-        const permalink = Permalinks.parseUrl(roomRef);
-        if (!permalink.roomIdOrAlias) return null;
-
-        const roomId = await this.client.resolveRoom(permalink.roomIdOrAlias);
-        if (!joinedRooms.includes(roomId)) {
-            await this.client.joinRoom(permalink.roomIdOrAlias, permalink.viaServers);
-        }
-
-        if (this.policyLists.find(b => b.roomId === roomId)) return null;
-
-        const list = await this.addPolicyList(roomId, roomRef);
-
-        await this.client.setAccountData(WATCHED_LISTS_EVENT_TYPE, {
-            references: this.policyLists.map(b => b.roomRef),
-        });
-
-        await this.warnAboutUnprotectedPolicyListRoom(roomId);
-
-        return list;
-    }
-
-    public async unwatchList(roomRef: string): Promise<PolicyList | null> {
-        const permalink = Permalinks.parseUrl(roomRef);
-        if (!permalink.roomIdOrAlias) return null;
-
-        const roomId = await this.client.resolveRoom(permalink.roomIdOrAlias);
-        const list = this.policyLists.find(b => b.roomId === roomId) || null;
-        if (list) {
-            this.policyLists.splice(this.policyLists.indexOf(list), 1);
-            this.ruleServer?.unwatch(list);
-            this.protectedRoomsTracker.unwatchList(list);
-        }
-
-        await this.client.setAccountData(WATCHED_LISTS_EVENT_TYPE, {
-            references: this.policyLists.map(b => b.roomRef),
-        });
-        return list;
-    }
-
-    public async warnAboutUnprotectedPolicyListRoom(roomId: string) {
-        if (!this.config.protectAllJoinedRooms) return; // doesn't matter
-        if (this.protectedRoomsConfig.getExplicitlyProtectedRooms().includes(roomId)) return; // explicitly protected
-
-        try {
-            const accountData: { warned: boolean } | null = await this.client.getAccountData(WARN_UNPROTECTED_ROOM_EVENT_PREFIX + roomId);
-            if (accountData && accountData.warned) return; // already warned
-        } catch (e) {
-            // Ignore - probably haven't warned about it yet
-        }
-
-        await this.managementRoomOutput.logMessage(LogLevel.WARN, "Mjolnir", `Not protecting ${roomId} - it is a ban list that this bot did not create. Add the room as protected if it is supposed to be protected. This warning will not appear again.`, roomId);
-        await this.client.setAccountData(WARN_UNPROTECTED_ROOM_EVENT_PREFIX + roomId, { warned: true });
-    }
-
-    /**
-     * Load the watched policy lists from account data, only used when Mjolnir is initialized.
-     */
-    private async buildWatchedPolicyLists() {
-        this.policyLists = [];
-        const joinedRooms = await this.client.getJoinedRooms();
-
-        let watchedListsEvent: { references?: string[] } | null = null;
-        try {
-            watchedListsEvent = await this.client.getAccountData(WATCHED_LISTS_EVENT_TYPE);
-        } catch (e) {
-            if (e.statusCode === 404) {
-                LogService.warn('Mjolnir', "Couldn't find account data for Mjolnir's watched lists, assuming first start.", extractRequestError(e));
-            } else {
-                throw e;
-            }
-        }
-
-        for (const roomRef of (watchedListsEvent?.references || [])) {
-            const permalink = Permalinks.parseUrl(roomRef);
-            if (!permalink.roomIdOrAlias) continue;
-
-            const roomId = await this.client.resolveRoom(permalink.roomIdOrAlias);
-            if (!joinedRooms.includes(roomId)) {
-                await this.client.joinRoom(permalink.roomIdOrAlias, permalink.viaServers);
-            }
-
-            await this.warnAboutUnprotectedPolicyListRoom(roomId);
-            await this.addPolicyList(roomId, roomRef);
-        }
-    }
-
     private async handleEvent(roomId: string, event: any) {
         // Check for UISI errors
         if (roomId === this.managementRoomId) {
@@ -548,7 +438,7 @@ export class Mjolnir {
 
         // Check for updated ban lists before checking protected rooms - the ban lists might be protected
         // themselves.
-        const policyList = this.policyLists.find(list => list.roomId === roomId);
+        const policyList = this.policyListManager.lists.find(list => list.roomId === roomId);
         if (policyList !== undefined) {
             if (ALL_BAN_LIST_RULE_TYPES.includes(event['type']) || event['type'] === 'm.room.redaction') {
                 policyList.updateForEvent(event.event_id)

src/appservice/MjolnirManager.ts

@@ -206,7 +206,7 @@ export class ManagedMjolnir {
         );
         const roomRef = Permalinks.forRoom(listRoomId);
         await this.mjolnir.addProtectedRoom(listRoomId);
-        return await this.mjolnir.watchList(roomRef);
+        return await this.mjolnir.policyListManager.watchList(roomRef);
     }
 
     public get managementRoomId(): string {

src/commands/CreateBanListCommand.ts

@@ -31,7 +31,7 @@ export async function execCreateListCommand(roomId: string, event: any, mjolnir:
     );
 
     const roomRef = Permalinks.forRoom(listRoomId);
-    await mjolnir.watchList(roomRef);
+    await mjolnir.policyListManager.watchList(roomRef);
     await mjolnir.addProtectedRoom(listRoomId);
 
     const html = `Created new list (<a href="${roomRef}">${listRoomId}</a>). This list is now being watched.`;

src/commands/DumpRulesCommand.ts

@@ -32,7 +32,7 @@ import { htmlEscape } from "../utils";
 export async function execRulesMatchingCommand(roomId: string, event: any, mjolnir: Mjolnir, entity: string) {
     let html = "";
     let text = "";
-    for (const list of mjolnir.lists) {
+    for (const list of mjolnir.policyListManager.lists) {
         const matches = list.rulesMatchingEntity(entity)
 
         if (matches.length === 0) {
@@ -81,7 +81,7 @@ export async function execDumpRulesCommand(roomId: string, event: any, mjolnir:
     let text = "Rules currently in use:\n";
 
     let hasLists = false;
-    for (const list of mjolnir.lists) {
+    for (const list of mjolnir.policyListManager.lists) {
         hasLists = true;
         let hasRules = false;
 

src/commands/ImportCommand.ts

@@ -22,7 +22,7 @@ import PolicyList from "../models/PolicyList";
 // !mjolnir import <room ID> <shortcode>
 export async function execImportCommand(roomId: string, event: any, mjolnir: Mjolnir, parts: string[]) {
     const importRoomId = await mjolnir.client.resolveRoom(parts[2]);
-    const list = mjolnir.lists.find(b => b.listShortcode === parts[3]) as PolicyList;
+    const list = mjolnir.policyListManager.lists.find(b => b.listShortcode === parts[3]) as PolicyList;
     if (!list) {
         const errMessage = "Unable to find list - check your shortcode.";
         const errReply = RichReply.createFor(roomId, event, errMessage, errMessage);

src/commands/SetDefaultBanListCommand.ts

@@ -22,7 +22,7 @@ export const DEFAULT_LIST_EVENT_TYPE = "org.matrix.mjolnir.default_list";
 // !mjolnir default <shortcode>
 export async function execSetDefaultListCommand(roomId: string, event: any, mjolnir: Mjolnir, parts: string[]) {
     const shortcode = parts[2];
-    const list = mjolnir.lists.find(b => b.listShortcode === shortcode);
+    const list = mjolnir.policyListManager.lists.find(b => b.listShortcode === shortcode);
     if (!list) {
         const replyText = "No ban list with that shortcode was found.";
         const reply = RichReply.createFor(roomId, event, replyText, replyText);

src/commands/StatusCommand.ts

@@ -86,9 +86,9 @@ async function showMjolnirStatus(roomId: string, event: any, mjolnir: Mjolnir) {
         }
         html += "</ul>";
     }
-    const subscribedLists = mjolnir.lists.filter(list => !mjolnir.explicitlyProtectedRooms.includes(list.roomId));
+    const subscribedLists = mjolnir.policyListManager.lists.filter(list => !mjolnir.explicitlyProtectedRooms.includes(list.roomId));
     renderPolicyLists("Subscribed policy lists", subscribedLists);
-    const subscribedAndProtectedLists = mjolnir.lists.filter(list => mjolnir.explicitlyProtectedRooms.includes(list.roomId));
+    const subscribedAndProtectedLists = mjolnir.policyListManager.lists.filter(list => mjolnir.explicitlyProtectedRooms.includes(list.roomId));
     renderPolicyLists("Subscribed and protected policy lists", subscribedAndProtectedLists);
 
     const reply = RichReply.createFor(roomId, event, text, html);

src/commands/UnbanBanCommand.ts

@@ -59,7 +59,7 @@ export async function parseArguments(roomId: string, event: any, mjolnir: Mjolni
             else if (arg.startsWith("!") && !ruleType) ruleType = RULE_ROOM;
             else if (!ruleType) ruleType = RULE_SERVER;
         } else if (!list) {
-            const foundList = mjolnir.lists.find(b => b.listShortcode.toLowerCase() === arg.toLowerCase());
+            const foundList = mjolnir.policyListManager.lists.find(b => b.listShortcode.toLowerCase() === arg.toLowerCase());
             if (foundList !== undefined) {
                 list = foundList;
             }
@@ -86,7 +86,7 @@ export async function parseArguments(roomId: string, event: any, mjolnir: Mjolni
     }
 
     if (!list) {
-        list = mjolnir.lists.find(b => b.listShortcode.toLowerCase() === defaultShortcode) || null;
+        list = mjolnir.policyListManager.lists.find(b => b.listShortcode.toLowerCase() === defaultShortcode) || null;
     }
 
     let replyMessage: string | null = null;

src/commands/WatchUnwatchCommand.ts

@@ -19,7 +19,7 @@ import { Permalinks, RichReply } from "matrix-bot-sdk";
 
 // !mjolnir watch <room alias or ID>
 export async function execWatchCommand(roomId: string, event: any, mjolnir: Mjolnir, parts: string[]) {
-    const list = await mjolnir.watchList(Permalinks.forRoom(parts[2]));
+    const list = await mjolnir.policyListManager.watchList(Permalinks.forRoom(parts[2]));
     if (!list) {
         const replyText = "Cannot watch list due to error - is that a valid room alias?";
         const reply = RichReply.createFor(roomId, event, replyText, replyText);
@@ -32,7 +32,7 @@ export async function execWatchCommand(roomId: string, event: any, mjolnir: Mjol
 
 // !mjolnir unwatch <room alias or ID>
 export async function execUnwatchCommand(roomId: string, event: any, mjolnir: Mjolnir, parts: string[]) {
-    const list = await mjolnir.unwatchList(Permalinks.forRoom(parts[2]));
+    const list = await mjolnir.policyListManager.unwatchList(Permalinks.forRoom(parts[2]));
     if (!list) {
         const replyText = "Cannot unwatch list due to error - is that a valid room alias?";
         const reply = RichReply.createFor(roomId, event, replyText, replyText);

src/models/PolicyList.ts

@@ -14,13 +14,30 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import { extractRequestError, LogService, RoomCreateOptions, UserID } from "matrix-bot-sdk";
+import { extractRequestError, LogLevel, LogService, Permalinks, RoomCreateOptions, UserID } from "matrix-bot-sdk";
 import { EventEmitter } from "events";
 import { ALL_RULE_TYPES, EntityType, ListRule, Recommendation, ROOM_RULE_TYPES, RULE_ROOM, RULE_SERVER, RULE_USER, SERVER_RULE_TYPES, USER_RULE_TYPES } from "./ListRule";
 import { MatrixSendClient } from "../MatrixEmitter";
 import AwaitLock from "await-lock";
 import { monotonicFactory } from "ulidx";
+import { Mjolnir } from "../Mjolnir";
 
+/**
+ * Account data event type used to store the permalinks to each of the policylists.
+ *
+ * Content:
+ * ```jsonc
+ * {
+ *   references: string[], // Each entry is a `matrix.to` permalink.
+ * }
+ * ```
+ */
+export const WATCHED_LISTS_EVENT_TYPE = "org.matrix.mjolnir.watched_lists";
+
+/**
+ * A prefix used to record that we have already warned at least once that a PolicyList room is unprotected.
+ */
+const WARN_UNPROTECTED_ROOM_EVENT_PREFIX = "org.matrix.mjolnir.unprotected_room_warning.for.";
 export const SHORTCODE_EVENT_TYPE = "org.matrix.mjolnir.shortcode";
 
 export enum ChangeType {
@@ -596,3 +613,176 @@ export class Revision {
         return this.ulid > revision.ulid;
     }
 }
+
+/**
+ * A manager for all the policy lists for this Mjölnir
+ */
+export class PolicyListManager {
+    private policyLists: PolicyList[];
+
+    /**
+     * A list of references (matrix.to URLs) to policy lists that
+     * we could not resolve during startup. We store them to make
+     * sure that they're written back whenever we rewrite the references
+     * to account data.
+     */
+    private readonly failedStartupWatchListRefs: Set<string> = new Set();
+
+    constructor(private readonly mjolnir: Mjolnir) {
+        // Nothing to do.
+    }
+
+    public get lists(): PolicyList[] {
+        return this.policyLists;
+    }
+
+    /**
+     * Helper for constructing `PolicyList`s and making sure they have the right listeners set up.
+     * @param roomId The room id for the `PolicyList`.
+     * @param roomRef A reference (matrix.to URL) for the `PolicyList`.
+     */
+    private async addPolicyList(roomId: string, roomRef: string): Promise<PolicyList> {
+        const list = new PolicyList(roomId, roomRef, this.mjolnir.client);
+        this.mjolnir.ruleServer?.watch(list);
+        await list.updateList();
+        this.policyLists.push(list);
+        this.mjolnir.protectedRoomsTracker.watchList(list);
+
+        // If we have succeeded, let's remove this from the list of failed policy rooms.
+        this.failedStartupWatchListRefs.delete(roomRef);
+        return list;
+    }
+
+    public async watchList(roomRef: string): Promise<PolicyList | null> {
+        const joinedRooms = await this.mjolnir.client.getJoinedRooms();
+        const permalink = Permalinks.parseUrl(roomRef);
+        if (!permalink.roomIdOrAlias) return null;
+
+        const roomId = await this.mjolnir.client.resolveRoom(permalink.roomIdOrAlias);
+        if (!joinedRooms.includes(roomId)) {
+            await this.mjolnir.client.joinRoom(roomId, permalink.viaServers);
+        }
+
+        if (this.policyLists.find(b => b.roomId === roomId)) {
+            // This room was already in our list of policy rooms, nothing else to do.
+            // Note that we bailout *after* the call to `joinRoom`, in case a user
+            // calls `watchList` in an attempt to repair something that was broken,
+            // e.g. a Mjölnir who could not join the room because of alias resolution
+            // or server being down, etc.
+            return null;
+        }
+
+        const list = await this.addPolicyList(roomId, roomRef);
+
+        await this.storeWatchedPolicyLists();
+
+        await this.warnAboutUnprotectedPolicyListRoom(roomId);
+
+        return list;
+    }
+
+    public async unwatchList(roomRef: string): Promise<PolicyList | null> {
+        const permalink = Permalinks.parseUrl(roomRef);
+        if (!permalink.roomIdOrAlias) return null;
+
+        const roomId = await this.mjolnir.client.resolveRoom(permalink.roomIdOrAlias);
+        const list = this.policyLists.find(b => b.roomId === roomId) || null;
+        if (list) {
+            this.policyLists.splice(this.policyLists.indexOf(list), 1);
+            this.mjolnir.ruleServer?.unwatch(list);
+            this.mjolnir.protectedRoomsTracker.unwatchList(list);
+        }
+
+        await this.storeWatchedPolicyLists();
+        return list;
+    }
+
+    /**
+     * Load the watched policy lists from account data, only used when Mjolnir is initialized.
+     */
+    public async init() {
+        this.policyLists = [];
+        const joinedRooms = await this.mjolnir.client.getJoinedRooms();
+
+        let watchedListsEvent: { references?: string[] } | null = null;
+        try {
+            watchedListsEvent = await this.mjolnir.client.getAccountData(WATCHED_LISTS_EVENT_TYPE);
+        } catch (e) {
+            if (e.statusCode === 404) {
+                LogService.warn('Mjolnir', "Couldn't find account data for Mjolnir's watched lists, assuming first start.", extractRequestError(e));
+            } else {
+                throw e;
+            }
+        }
+
+        for (const roomRef of (watchedListsEvent?.references || [])) {
+            const permalink = Permalinks.parseUrl(roomRef);
+            if (!permalink.roomIdOrAlias) continue;
+
+            let roomId;
+            try {
+                roomId = await this.mjolnir.client.resolveRoom(permalink.roomIdOrAlias);
+            } catch (ex) {
+                // Let's not fail startup because of a problem resolving a room id or an alias.
+                LogService.warn('Mjolnir', 'Could not resolve policy list room, skipping for this run', permalink.roomIdOrAlias)
+                await this.mjolnir.managementRoomOutput.logMessage(LogLevel.WARN, "Mjolnir", `Room ${permalink.roomIdOrAlias} could **not** be resolved, perhaps a server is down? Skipping this room. If this is a recurring problem, please consider removing this room.`);
+                this.failedStartupWatchListRefs.add(roomRef);
+                continue;
+            }
+            if (!joinedRooms.includes(roomId)) {
+                await this.mjolnir.client.joinRoom(permalink.roomIdOrAlias, permalink.viaServers);
+            }
+
+            await this.warnAboutUnprotectedPolicyListRoom(roomId);
+            await this.addPolicyList(roomId, roomRef);
+        }
+    }
+
+    /**
+     * Store to account the list of policy rooms.
+     *
+     * We store both rooms that we are currently monitoring and rooms for which
+     * we could not setup monitoring, assuming that the setup is a transient issue
+     * that the user (or someone else) will eventually resolve.
+     */
+    private async storeWatchedPolicyLists() {
+        let list = this.policyLists.map(b => b.roomRef);
+        for (let entry of this.failedStartupWatchListRefs) {
+            list.push(entry);
+        }
+        await this.mjolnir.client.setAccountData(WATCHED_LISTS_EVENT_TYPE, {
+            references: list,
+        });
+    }
+
+    /**
+     * Check whether a policy list room is protected. If not, display
+     * a user-readable warning.
+     *
+     * We store as account data the list of room ids for which we have
+     * already displayed the warning, to avoid bothering users at every
+     * single startup.
+     *
+     * @param roomId The id of the room to check/warn.
+     */
+    private async warnAboutUnprotectedPolicyListRoom(roomId: string) {
+        if (!this.mjolnir.config.protectAllJoinedRooms) {
+            return; // doesn't matter
+        }
+        if (this.mjolnir.explicitlyProtectedRooms.includes(roomId)) {
+            return; // explicitly protected
+        }
+
+        try {
+            const accountData: { warned: boolean } | null = await this.mjolnir.client.getAccountData(WARN_UNPROTECTED_ROOM_EVENT_PREFIX + roomId);
+            if (accountData && accountData.warned) {
+                return; // already warned
+            }
+        } catch (e) {
+            // Expect that we haven't warned yet.
+        }
+
+        await this.mjolnir.managementRoomOutput.logMessage(LogLevel.WARN, "Mjolnir", `Not protecting ${roomId} - it is a ban list that this bot did not create. Add the room as protected if it is supposed to be protected. This warning will not appear again.`, roomId);
+        await this.mjolnir.client.setAccountData(WARN_UNPROTECTED_ROOM_EVENT_PREFIX + roomId, { warned: true });
+    }
+}

test/commands/UnbanBanCommandTest.ts

@@ -35,6 +35,7 @@ function createTestMjolnir(defaultShortcode: string|null = null): Mjolnir {
     return <Mjolnir>{
         client,
         config,
+        policyListManager: {}
     };
 }
 
@@ -53,7 +54,7 @@ describe("UnbanBanCommand", () => {
     describe("parseArguments", () => {
         it("should be able to detect servers", async () => {
             const mjolnir = createTestMjolnir();
-            (<any>mjolnir).lists = [{listShortcode: "test"}];
+            (<any>mjolnir).policyListManager.lists = [{listShortcode: "test"}];
             mjolnir.client.sendMessage = (roomId: string, content: any): Promise<string> => {
                 throw new Error("sendMessage should not have been called: " + JSON.stringify(content));
             };
@@ -70,7 +71,7 @@ describe("UnbanBanCommand", () => {
 
         it("should be able to detect servers with ban reasons", async () => {
             const mjolnir = createTestMjolnir();
-            (<any>mjolnir).lists = [{listShortcode: "test"}];
+            (<any>mjolnir).policyListManager.lists = [{listShortcode: "test"}];
             mjolnir.client.sendMessage = (roomId: string, content: any): Promise<string> => {
                 throw new Error("sendMessage should not have been called: " + JSON.stringify(content));
             };
@@ -87,7 +88,7 @@ describe("UnbanBanCommand", () => {
 
         it("should be able to detect servers with globs", async () => {
             const mjolnir = createTestMjolnir();
-            (<any>mjolnir).lists = [{listShortcode: "test"}];
+            (<any>mjolnir).policyListManager.lists = [{listShortcode: "test"}];
             mjolnir.client.sendMessage = (roomId: string, content: any): Promise<string> => {
                 throw new Error("sendMessage should not have been called: " + JSON.stringify(content));
             };
@@ -104,7 +105,7 @@ describe("UnbanBanCommand", () => {
 
         it("should be able to detect servers with the type specified", async () => {
             const mjolnir = createTestMjolnir();
-            (<any>mjolnir).lists = [{listShortcode: "test"}];
+            (<any>mjolnir).policyListManager.lists = [{listShortcode: "test"}];
             mjolnir.client.sendMessage = (roomId: string, content: any): Promise<string> => {
                 throw new Error("sendMessage should not have been called: " + JSON.stringify(content));
             };
@@ -121,7 +122,7 @@ describe("UnbanBanCommand", () => {
 
         it("should be able to detect room IDs", async () => {
             const mjolnir = createTestMjolnir();
-            (<any>mjolnir).lists = [{listShortcode: "test"}];
+            (<any>mjolnir).policyListManager.lists = [{listShortcode: "test"}];
             mjolnir.client.sendMessage = (roomId: string, content: any): Promise<string> => {
                 throw new Error("sendMessage should not have been called: " + JSON.stringify(content));
             };
@@ -138,7 +139,7 @@ describe("UnbanBanCommand", () => {
 
         it("should be able to detect room IDs with ban reasons", async () => {
             const mjolnir = createTestMjolnir();
-            (<any>mjolnir).lists = [{listShortcode: "test"}];
+            (<any>mjolnir).policyListManager.lists = [{listShortcode: "test"}];
             mjolnir.client.sendMessage = (roomId: string, content: any): Promise<string> => {
                 throw new Error("sendMessage should not have been called: " + JSON.stringify(content));
             };
@@ -155,7 +156,7 @@ describe("UnbanBanCommand", () => {
 
         it("should be able to detect room IDs with globs", async () => {
             const mjolnir = createTestMjolnir();
-            (<any>mjolnir).lists = [{listShortcode: "test"}];
+            (<any>mjolnir).policyListManager.lists = [{listShortcode: "test"}];
             mjolnir.client.sendMessage = (roomId: string, content: any): Promise<string> => {
                 throw new Error("sendMessage should not have been called: " + JSON.stringify(content));
             };
@@ -172,7 +173,7 @@ describe("UnbanBanCommand", () => {
 
         it("should be able to detect room aliases", async () => {
             const mjolnir = createTestMjolnir();
-            (<any>mjolnir).lists = [{listShortcode: "test"}];
+            (<any>mjolnir).policyListManager.lists = [{listShortcode: "test"}];
             mjolnir.client.sendMessage = (roomId: string, content: any): Promise<string> => {
                 throw new Error("sendMessage should not have been called: " + JSON.stringify(content));
             };
@@ -189,7 +190,7 @@ describe("UnbanBanCommand", () => {
 
         it("should be able to detect room aliases with ban reasons", async () => {
             const mjolnir = createTestMjolnir();
-            (<any>mjolnir).lists = [{listShortcode: "test"}];
+            (<any>mjolnir).policyListManager.lists = [{listShortcode: "test"}];
             mjolnir.client.sendMessage = (roomId: string, content: any): Promise<string> => {
                 throw new Error("sendMessage should not have been called: " + JSON.stringify(content));
             };
@@ -206,7 +207,7 @@ describe("UnbanBanCommand", () => {
 
         it("should be able to detect room aliases with globs", async () => {
             const mjolnir = createTestMjolnir();
-            (<any>mjolnir).lists = [{listShortcode: "test"}];
+            (<any>mjolnir).policyListManager.lists = [{listShortcode: "test"}];
             mjolnir.client.sendMessage = (roomId: string, content: any): Promise<string> => {
                 throw new Error("sendMessage should not have been called: " + JSON.stringify(content));
             };
@@ -223,7 +224,7 @@ describe("UnbanBanCommand", () => {
 
         it("should be able to detect rooms with the type specified", async () => {
             const mjolnir = createTestMjolnir();
-            (<any>mjolnir).lists = [{listShortcode: "test"}];
+            (<any>mjolnir).policyListManager.lists = [{listShortcode: "test"}];
             mjolnir.client.sendMessage = (roomId: string, content: any): Promise<string> => {
                 throw new Error("sendMessage should not have been called: " + JSON.stringify(content));
             };
@@ -240,7 +241,7 @@ describe("UnbanBanCommand", () => {
 
         it("should be able to detect user IDs", async () => {
             const mjolnir = createTestMjolnir();
-            (<any>mjolnir).lists = [{listShortcode: "test"}];
+            (<any>mjolnir).policyListManager.lists = [{listShortcode: "test"}];
             mjolnir.client.sendMessage = (roomId: string, content: any): Promise<string> => {
                 throw new Error("sendMessage should not have been called: " + JSON.stringify(content));
             };
@@ -257,7 +258,7 @@ describe("UnbanBanCommand", () => {
 
         it("should be able to detect user IDs with ban reasons", async () => {
             const mjolnir = createTestMjolnir();
-            (<any>mjolnir).lists = [{listShortcode: "test"}];
+            (<any>mjolnir).policyListManager.lists = [{listShortcode: "test"}];
             mjolnir.client.sendMessage = (roomId: string, content: any): Promise<string> => {
                 throw new Error("sendMessage should not have been called: " + JSON.stringify(content));
             };
@@ -274,7 +275,7 @@ describe("UnbanBanCommand", () => {
 
         it("should be able to detect user IDs with globs", async () => {
             const mjolnir = createTestMjolnir();
-            (<any>mjolnir).lists = [{listShortcode: "test"}];
+            (<any>mjolnir).policyListManager.lists = [{listShortcode: "test"}];
             mjolnir.client.sendMessage = (roomId: string, content: any): Promise<string> => {
                 throw new Error("sendMessage should not have been called: " + JSON.stringify(content));
             };
@@ -291,7 +292,7 @@ describe("UnbanBanCommand", () => {
 
         it("should be able to detect user IDs with the type specified", async () => {
             const mjolnir = createTestMjolnir();
-            (<any>mjolnir).lists = [{listShortcode: "test"}];
+            (<any>mjolnir).policyListManager.lists = [{listShortcode: "test"}];
             mjolnir.client.sendMessage = (roomId: string, content: any): Promise<string> => {
                 throw new Error("sendMessage should not have been called: " + JSON.stringify(content));
             };
@@ -308,7 +309,7 @@ describe("UnbanBanCommand", () => {
 
         it("should error if wildcards used without --force", async () => {
             const mjolnir = createTestMjolnir();
-            (<any>mjolnir).lists = [{listShortcode: "test"}];
+            (<any>mjolnir).policyListManager.lists = [{listShortcode: "test"}];
             mjolnir.client.sendMessage = (roomId: string, content: any): Promise<string> => {
                 expect(content).toBeDefined();
                 expect(content['body']).toContain("Wildcard bans require an additional `--force` argument to confirm");
@@ -322,7 +323,7 @@ describe("UnbanBanCommand", () => {
 
         it("should have correct ban reason with --force after", async () => {
             const mjolnir = createTestMjolnir();
-            (<any>mjolnir).lists = [{listShortcode: "test"}];
+            (<any>mjolnir).policyListManager.lists = [{listShortcode: "test"}];
             mjolnir.client.sendMessage = (roomId: string, content: any): Promise<string> => {
                 throw new Error("sendMessage should not have been called: " + JSON.stringify(content));
             };
@@ -340,7 +341,7 @@ describe("UnbanBanCommand", () => {
         describe("[without default list]", () => {
             it("should error if no list (with type) is specified", async () => {
                 const mjolnir = createTestMjolnir();
-                (<any>mjolnir).lists = [{listShortcode: "test"}];
+                (<any>mjolnir).policyListManager.lists = [{listShortcode: "test"}];
                 mjolnir.client.sendMessage = (roomId: string, content: any): Promise<string> => {
                     expect(content).toBeDefined();
                     expect(content['body']).toContain("No ban list matching that shortcode was found");
@@ -354,7 +355,7 @@ describe("UnbanBanCommand", () => {
 
             it("should error if no list (without type) is specified", async () => {
                 const mjolnir = createTestMjolnir();
-                (<any>mjolnir).lists = [{listShortcode: "test"}];
+                (<any>mjolnir).policyListManager.lists = [{listShortcode: "test"}];
                 mjolnir.client.sendMessage = (roomId: string, content: any): Promise<string> => {
                     expect(content).toBeDefined();
                     expect(content['body']).toContain("No ban list matching that shortcode was found");
@@ -368,7 +369,7 @@ describe("UnbanBanCommand", () => {
 
             it("should not error if a list (with type) is specified", async () => {
                 const mjolnir = createTestMjolnir();
-                (<any>mjolnir).lists = [{listShortcode: "test"}];
+                (<any>mjolnir).policyListManager.lists = [{listShortcode: "test"}];
                 mjolnir.client.sendMessage = (roomId: string, content: any): Promise<string> => {
                     throw new Error("sendMessage should not have been called: " + JSON.stringify(content));
                 };
@@ -385,7 +386,7 @@ describe("UnbanBanCommand", () => {
 
             it("should not error if a list (without type) is specified", async () => {
                 const mjolnir = createTestMjolnir();
-                (<any>mjolnir).lists = [{listShortcode: "test"}];
+                (<any>mjolnir).policyListManager.lists = [{listShortcode: "test"}];
                 mjolnir.client.sendMessage = (roomId: string, content: any): Promise<string> => {
                     throw new Error("sendMessage should not have been called: " + JSON.stringify(content));
                 };
@@ -402,7 +403,7 @@ describe("UnbanBanCommand", () => {
 
             it("should not error if a list (with type reversed) is specified", async () => {
                 const mjolnir = createTestMjolnir();
-                (<any>mjolnir).lists = [{listShortcode: "test"}];
+                (<any>mjolnir).policyListManager.lists = [{listShortcode: "test"}];
                 mjolnir.client.sendMessage = (roomId: string, content: any): Promise<string> => {
                     throw new Error("sendMessage should not have been called: " + JSON.stringify(content));
                 };
@@ -421,7 +422,7 @@ describe("UnbanBanCommand", () => {
         describe("[with default list]", () => {
             it("should use the default list if no list (with type) is specified", async () => {
                 const mjolnir = createTestMjolnir("test");
-                (<any>mjolnir).lists = [{listShortcode: "test"}, {listShortcode: "other"}];
+                (<any>mjolnir).policyListManager.lists = [{listShortcode: "test"}, {listShortcode: "other"}];
                 mjolnir.client.sendMessage = (roomId: string, content: any): Promise<string> => {
                     throw new Error("sendMessage should not have been called: " + JSON.stringify(content));
                 };
@@ -438,7 +439,7 @@ describe("UnbanBanCommand", () => {
 
             it("should use the default list if no list (without type) is specified", async () => {
                 const mjolnir = createTestMjolnir("test");
-                (<any>mjolnir).lists = [{listShortcode: "test"}, {listShortcode: "other"}];
+                (<any>mjolnir).policyListManager.lists = [{listShortcode: "test"}, {listShortcode: "other"}];
                 mjolnir.client.sendMessage = (roomId: string, content: any): Promise<string> => {
                     throw new Error("sendMessage should not have been called: " + JSON.stringify(content));
                 };
@@ -455,7 +456,7 @@ describe("UnbanBanCommand", () => {
 
             it("should use the specified list if a list (with type) is specified", async () => {
                 const mjolnir = createTestMjolnir("test");
-                (<any>mjolnir).lists = [{listShortcode: "test"}, {listShortcode: "other"}];
+                (<any>mjolnir).policyListManager.lists = [{listShortcode: "test"}, {listShortcode: "other"}];
                 mjolnir.client.sendMessage = (roomId: string, content: any): Promise<string> => {
                     throw new Error("sendMessage should not have been called: " + JSON.stringify(content));
                 };
@@ -472,7 +473,7 @@ describe("UnbanBanCommand", () => {
 
             it("should use the specified list if a list (without type) is specified", async () => {
                 const mjolnir = createTestMjolnir("test");
-                (<any>mjolnir).lists = [{listShortcode: "test"}, {listShortcode: "other"}];
+                (<any>mjolnir).policyListManager.lists = [{listShortcode: "test"}, {listShortcode: "other"}];
                 mjolnir.client.sendMessage = (roomId: string, content: any): Promise<string> => {
                     throw new Error("sendMessage should not have been called: " + JSON.stringify(content));
                 };
@@ -489,7 +490,7 @@ describe("UnbanBanCommand", () => {
 
             it("should not error if a list (with type reversed) is specified", async () => {
                 const mjolnir = createTestMjolnir("test");
-                (<any>mjolnir).lists = [{listShortcode: "test"}, {listShortcode: "other"}];
+                (<any>mjolnir).policyListManager.lists = [{listShortcode: "test"}, {listShortcode: "other"}];
                 mjolnir.client.sendMessage = (roomId: string, content: any): Promise<string> => {
                     throw new Error("sendMessage should not have been called: " + JSON.stringify(content));
                 };

test/integration/banListTest.ts

@@ -262,7 +262,7 @@ describe('Test: ACL updates will batch when rules are added in succession.', fun
         // Flood the watched list with banned servers, which should prompt Mjolnir to update server ACL in protected rooms.
         const banListId = await moderator.createRoom({ invite: [mjolnirId] });
         await mjolnir.client.joinRoom(banListId);
-        await mjolnir.watchList(Permalinks.forRoom(banListId));
+        await mjolnir.policyListManager.watchList(Permalinks.forRoom(banListId));
         const acl = new ServerAcl(serverName).denyIpAddresses().allowServer("*");
         const evilServerCount = 200;
         for (let i = 0; i < evilServerCount; i++) {
@@ -278,7 +278,7 @@ describe('Test: ACL updates will batch when rules are added in succession.', fun
 
         // 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]!;
+        const list: PolicyList = this.mjolnir.policyListManager.lists[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.
@@ -327,7 +327,7 @@ describe('Test: unbaning entities via the PolicyList.', function() {
         await moderator.setUserPowerLevel(await mjolnir.client.getUserId(), banListId, 100);
         await moderator.sendStateEvent(banListId, 'org.matrix.mjolnir.shortcode', '', { shortcode: "unban-test" });
         await mjolnir.client.joinRoom(banListId);
-        await mjolnir.watchList(Permalinks.forRoom(banListId));
+        await mjolnir.policyListManager.watchList(Permalinks.forRoom(banListId));
         // we use this to compare changes.
         const banList = new PolicyList(banListId, banListId, moderator);
         // we need two because we need to test the case where an entity has all rule types in the list
@@ -402,7 +402,7 @@ describe('Test: should apply bans to the most recently active rooms first', func
         // Flood the watched list with banned servers, which should prompt Mjolnir to update server ACL in protected rooms.
         const banListId = await moderator.createRoom({ invite: [mjolnirId] });
         await mjolnir.client.joinRoom(banListId);
-        await mjolnir.watchList(Permalinks.forRoom(banListId));
+        await mjolnir.policyListManager.watchList(Permalinks.forRoom(banListId));
 
         await mjolnir.protectedRoomsTracker.syncLists();
 
@@ -506,7 +506,7 @@ describe('Test: AccessControlUnit interaction with policy lists.', function() {
         assertAccess(Access.Allowed, aclUnit.getAccessForUser('@someone:matrix.org', "CHECK_SERVER"));
 
         // protect a room and check that only bad.example.com, *.ddns.example.com are in the deny ACL and not matrix.org
-        await mjolnir.watchList(policyList.roomRef);
+        await mjolnir.policyListManager.watchList(policyList.roomRef);
         const protectedRoom = await mjolnir.client.createRoom();
         await mjolnir.protectedRoomsTracker.addProtectedRoom(protectedRoom);
         await mjolnir.protectedRoomsTracker.syncLists();

test/integration/commands/commandUtils.ts

@@ -125,7 +125,7 @@ export async function getFirstReaction(matrix: MatrixEmitter, targetRoom: string
  * @param client A client that isn't mjolnir to send the message with, as you will be invited to the room.
  * @returns The shortcode for the list that can be used to refer to the list in future commands.
  */
-export async function createBanList(managementRoom: string, mjolnir: MatrixClient, client: MatrixClient): Promise<string> {
+export async function createBanList(managementRoom: string, mjolnir: MatrixEmitter, client: MatrixClient): Promise<string> {
     const listName = crypto.randomUUID();
     const listCreationResponse = await getFirstReply(mjolnir, managementRoom, async () => {
         return await client.sendMessage(managementRoom, { msgtype: 'm.text', body: `!mjolnir list create ${listName} ${listName}`});

test/integration/protectedRoomsConfigTest.ts

@@ -1,6 +1,7 @@
 
 import { strict as assert } from "assert";
 import { MatrixClient, Permalinks, UserID } from "matrix-bot-sdk";
+import { MatrixSendClient } from "../../src/MatrixEmitter";
 import { Mjolnir } from "../../src/Mjolnir";
 import PolicyList from "../../src/models/PolicyList";
 import { newTestUser } from "./clientHelper";
@@ -12,7 +13,7 @@ async function createPolicyList(client: MatrixClient): Promise<PolicyList> {
     return new PolicyList(policyListId, Permalinks.forRoom(policyListId), client);
 }
 
-async function getProtectedRoomsFromAccountData(client: MatrixClient): Promise<string[]> {
+async function getProtectedRoomsFromAccountData(client: MatrixSendClient): Promise<string[]> {
     const rooms: { rooms?: string[] } = await client.getAccountData("org.matrix.mjolnir.protected_rooms");
     return rooms.rooms!;
 }
@@ -42,9 +43,9 @@ describe('Test: config.protectAllJoinedRooms behaves correctly.', function() {
             .forEach(roomId => assert.equal(implicitlyProtectedRooms.includes(roomId), true));
 
         // We create one policy list with Mjolnir, and we watch another that is maintained by someone else.
-        const policyListShortcode = await createBanList(mjolnir.managementRoomId, mjolnir.client, moderator);
+        const policyListShortcode = await createBanList(mjolnir.managementRoomId, mjolnir.matrixEmitter, moderator);
         const unprotectedWatchedList = await createPolicyList(moderator);
-        await mjolnir.watchList(unprotectedWatchedList.roomRef);
+        await mjolnir.policyListManager.watchList(unprotectedWatchedList.roomRef);
         await mjolnir.protectedRoomsTracker.syncLists();
 
         // We expect that the watched list will not be protected, despite config.protectAllJoinedRooms being true