From a876a055203757c455aa781a06e63f5b50c8a07d Mon Sep 17 00:00:00 2001 From: Jae Lo Presti Date: Wed, 15 Jun 2022 14:20:27 +0300 Subject: [PATCH 01/10] Glob kick command (#291) This pull requests adds for glob support in the `!mjolnir kick` command. ## Example ``` !mjolnir kick @*:domain.tld --force ``` This command will kick every user having a mxid matching `domain.tld`. You can also still kick a particular user: ``` !mjolnir kick @user:domain.tld ``` ## Tests: Tested on the Furry Tech room (`vRGLvqJYlFvzpThbxI:matrix.org`) after a spam wave. It kicked over 13k bots in a matter of hours without putting too much strain on the homeserver. For instance, this command was matching `@spam*`: ![image](https://user-images.githubusercontent.com/76598503/167320002-f0575f50-4b54-41d1-8220-f67d72ccaf16.png) Signed-off-by: Jae Lo Presti --- src/commands/CommandHandler.ts | 2 +- src/commands/KickCommand.ts | 54 ++++++++++++++++++++++++++-------- 2 files changed, 42 insertions(+), 14 deletions(-) diff --git a/src/commands/CommandHandler.ts b/src/commands/CommandHandler.ts index 79c7b83..ea12167 100644 --- a/src/commands/CommandHandler.ts +++ b/src/commands/CommandHandler.ts @@ -131,7 +131,7 @@ export async function handleCommand(roomId: string, event: { content: { body: st "!mjolnir unban [apply] - Removes an entity from the ban list. If apply is 'true', the users matching the glob will actually be unbanned\n" + "!mjolnir redact [room alias/ID] [limit] - Redacts messages by the sender in the target room (or all rooms), up to a maximum number of events in the backlog (default 1000)\n" + "!mjolnir redact - Redacts a message by permalink\n" + - "!mjolnir kick [room alias/ID] [reason] - Kicks a user in a particular room or all protected rooms\n" + + "!mjolnir kick [room alias/ID] [reason] - Kicks a user or all of those matching a glob in a particular room or all protected rooms\n" + "!mjolnir rules - Lists the rules currently in use by Mjolnir\n" + "!mjolnir sync - Force updates of all lists and re-apply rules\n" + "!mjolnir verify - Ensures Mjolnir can moderate all your rooms\n" + diff --git a/src/commands/KickCommand.ts b/src/commands/KickCommand.ts index deec02c..786da20 100644 --- a/src/commands/KickCommand.ts +++ b/src/commands/KickCommand.ts @@ -15,14 +15,31 @@ limitations under the License. */ import { Mjolnir } from "../Mjolnir"; -import { LogLevel } from "matrix-bot-sdk"; +import { LogLevel, MatrixGlob, RichReply } from "matrix-bot-sdk"; import config from "../config"; // !mjolnir kick [room] [reason] export async function execKickCommand(roomId: string, event: any, mjolnir: Mjolnir, parts: string[]) { - const userId = parts[2]; + let force = false; + const glob = parts[2]; let rooms = [...Object.keys(mjolnir.protectedRooms)]; + + if (parts[parts.length - 1] === "--force") { + force = true; + parts.pop(); + } + + if (config.commands.confirmWildcardBan && /[*?]/.test(glob) && !force) { + let replyMessage = "Wildcard bans require an addition `--force` argument to confirm"; + const reply = RichReply.createFor(roomId, event, replyMessage, replyMessage); + reply["msgtype"] = "m.notice"; + await mjolnir.client.sendMessage(roomId, reply); + return; + } + + const kickRule = new MatrixGlob(glob); + let reason: string | undefined; if (parts.length > 3) { let reasonIndex = 3; @@ -32,19 +49,30 @@ export async function execKickCommand(roomId: string, event: any, mjolnir: Mjoln } reason = parts.slice(reasonIndex).join(' ') || ''; } - if (!reason) reason = ""; + if (!reason) reason = ''; - for (const targetRoomId of rooms) { - const joinedUsers = await mjolnir.client.getJoinedRoomMembers(targetRoomId); - if (!joinedUsers.includes(userId)) continue; // skip + for (const protectedRoomId of rooms) { + const members = await mjolnir.client.getRoomMembers(protectedRoomId, undefined, ["join"], ["ban", "leave"]); - await mjolnir.logMessage(LogLevel.INFO, "KickCommand", `Kicking ${userId} in ${targetRoomId} for ${reason}`, targetRoomId); - if (!config.noop) { - await mjolnir.taskQueue.push(async () => { - return mjolnir.client.kickUser(userId, targetRoomId, reason); - }); - } else { - await mjolnir.logMessage(LogLevel.WARN, "KickCommand", `Tried to kick ${userId} in ${targetRoomId} but the bot is running in no-op mode.`, targetRoomId); + for (const member of members) { + const victim = member.membershipFor; + + if (kickRule.test(victim)) { + await mjolnir.logMessage(LogLevel.DEBUG, "KickCommand", `Removing ${victim} in ${protectedRoomId}`, protectedRoomId); + + if (!config.noop) { + try { + await mjolnir.taskQueue.push(async () => { + return mjolnir.client.kickUser(victim, protectedRoomId, reason); + }); + await mjolnir.client.kickUser(victim, protectedRoomId, reason); + } catch (e) { + await mjolnir.logMessage(LogLevel.WARN, "KickCommand", `An error happened while trying to kick ${victim}: ${e}`); + } + } else { + await mjolnir.logMessage(LogLevel.WARN, "KickCommand", `Tried to kick ${victim} in ${protectedRoomId} but the bot is running in no-op mode.`, protectedRoomId); + } + } } } From d7b846cdb3f250485f4b19d924c25d115c7dfea3 Mon Sep 17 00:00:00 2001 From: gnuxie Date: Wed, 15 Jun 2022 12:33:45 +0100 Subject: [PATCH 02/10] Remove duplicated kick from kick command (merging cleanup) Accidentally introduced while merging https://github.com/matrix-org/mjolnir/pull/291. --- src/commands/KickCommand.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/commands/KickCommand.ts b/src/commands/KickCommand.ts index 786da20..f841ef4 100644 --- a/src/commands/KickCommand.ts +++ b/src/commands/KickCommand.ts @@ -65,7 +65,6 @@ export async function execKickCommand(roomId: string, event: any, mjolnir: Mjoln await mjolnir.taskQueue.push(async () => { return mjolnir.client.kickUser(victim, protectedRoomId, reason); }); - await mjolnir.client.kickUser(victim, protectedRoomId, reason); } catch (e) { await mjolnir.logMessage(LogLevel.WARN, "KickCommand", `An error happened while trying to kick ${victim}: ${e}`); } From ed68e02c4e34262d68a5ada3ba8afd47e979031a Mon Sep 17 00:00:00 2001 From: Jess Porter Date: Mon, 4 Jul 2022 15:06:36 +0100 Subject: [PATCH 03/10] implement polling reports in synapse (#259) --- config/default.yaml | 5 + src/Mjolnir.ts | 35 ++++++- src/config.ts | 2 + src/report/ReportPoller.ts | 145 ++++++++++++++++++++++++++ test/integration/reportPollingTest.ts | 54 ++++++++++ tsconfig.json | 3 +- 6 files changed, 238 insertions(+), 6 deletions(-) create mode 100644 src/report/ReportPoller.ts create mode 100644 test/integration/reportPollingTest.ts diff --git a/config/default.yaml b/config/default.yaml index dcaae67..66f6566 100644 --- a/config/default.yaml +++ b/config/default.yaml @@ -235,3 +235,8 @@ web: abuseReporting: # Whether to enable this feature. enabled: false + +# Whether or not to actively poll synapse for abuse reports, to be used +# instead of intercepting client calls to synapse's abuse endpoint, when that +# isn't possible/practical. +pollReports: false diff --git a/src/Mjolnir.ts b/src/Mjolnir.ts index ce3dd23..6026072 100644 --- a/src/Mjolnir.ts +++ b/src/Mjolnir.ts @@ -43,6 +43,7 @@ import { Healthz } from "./health/healthz"; import { EventRedactionQueue, RedactUserInRoom } from "./queues/EventRedactionQueue"; import { htmlEscape } from "./utils"; import { ReportManager } from "./report/ReportManager"; +import { ReportPoller } from "./report/ReportPoller"; import { WebAPIs } from "./webapis/WebAPIs"; import { replaceRoomIdsWithPills } from "./utils"; import RuleServer from "./models/RuleServer"; @@ -67,6 +68,11 @@ const ENABLED_PROTECTIONS_EVENT_TYPE = "org.matrix.mjolnir.enabled_protections"; const PROTECTED_ROOMS_EVENT_TYPE = "org.matrix.mjolnir.protected_rooms"; const WARN_UNPROTECTED_ROOM_EVENT_PREFIX = "org.matrix.mjolnir.unprotected_room_warning.for."; const CONSEQUENCE_EVENT_DATA = "org.matrix.mjolnir.consequence"; +/** + * Synapse will tell us where we last got to on polling reports, so we need + * to store that for pagination on further polls + */ +export const REPORT_POLL_EVENT_TYPE = "org.matrix.mjolnir.report_poll"; export class Mjolnir { private displayName: string; @@ -97,7 +103,10 @@ export class Mjolnir { private webapis: WebAPIs; private protectedRoomActivityTracker: ProtectedRoomActivityTracker; public taskQueue: ThrottlingQueue; - + /* + * Config-enabled polling of reports in Synapse, so Mjolnir can react to reports + */ + private reportPoller?: ReportPoller; /** * Adds a listener to the client that will automatically accept invitations. * @param {MatrixClient} client @@ -256,12 +265,13 @@ export class Mjolnir { // Setup Web APIs console.log("Creating Web APIs"); const reportManager = new ReportManager(this); - reportManager.on("report.new", this.handleReport); + reportManager.on("report.new", this.handleReport.bind(this)); this.webapis = new WebAPIs(reportManager, this.ruleServer); - + if (config.pollReports) { + this.reportPoller = new ReportPoller(this, reportManager); + } // Setup join/leave listener this.roomJoins = new RoomMemberManager(this.client); - this.taskQueue = new ThrottlingQueue(this, config.backgroundDelayMS); } @@ -302,6 +312,20 @@ export class Mjolnir { console.log("Starting web server"); await this.webapis.start(); + if (this.reportPoller) { + let reportPollSetting: { from: number } = { from: 0 }; + try { + reportPollSetting = await this.client.getAccountData(REPORT_POLL_EVENT_TYPE); + } catch (err) { + if (err.body?.errcode !== "M_NOT_FOUND") { + throw err; + } else { + this.logMessage(LogLevel.INFO, "Mjolnir@startup", "report poll setting does not exist yet"); + } + } + this.reportPoller.start(reportPollSetting.from); + } + // Load the state. this.currentState = STATE_CHECKING_PERMISSIONS; @@ -358,6 +382,7 @@ export class Mjolnir { LogService.info("Mjolnir", "Stopping Mjolnir..."); this.client.stop(); this.webapis.stop(); + this.reportPoller?.stop(); } public async logMessage(level: LogLevel, module: string, message: string | any, additionalRoomIds: string[] | string | null = null, isRecursive = false): Promise { @@ -1163,7 +1188,7 @@ export class Mjolnir { return await this.eventRedactionQueue.process(this, roomId); } - private async handleReport(roomId: string, reporterId: string, event: any, reason?: string) { + private async handleReport({ roomId, reporterId, event, reason }: { roomId: string, reporterId: string, event: any, reason?: string }) { for (const protection of this.enabledProtections) { await protection.handleReport(this, roomId, reporterId, event, reason); } diff --git a/src/config.ts b/src/config.ts index dcaffb7..dab6a47 100644 --- a/src/config.ts +++ b/src/config.ts @@ -53,6 +53,7 @@ interface IConfig { * of one background task and the start of the next one. */ backgroundDelayMS: number; + pollReports: boolean; admin?: { enableMakeRoomAdminCommand?: boolean; } @@ -122,6 +123,7 @@ const defaultConfig: IConfig = { automaticallyRedactForReasons: ["spam", "advertising"], protectAllJoinedRooms: false, backgroundDelayMS: 500, + pollReports: false, commands: { allowNoPrefix: false, additionalPrefixes: [], diff --git a/src/report/ReportPoller.ts b/src/report/ReportPoller.ts new file mode 100644 index 0000000..2229b50 --- /dev/null +++ b/src/report/ReportPoller.ts @@ -0,0 +1,145 @@ +/* +Copyright 2022 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { Mjolnir, REPORT_POLL_EVENT_TYPE } from "../Mjolnir"; +import { ReportManager } from './ReportManager'; +import { LogLevel } from "matrix-bot-sdk"; + +class InvalidStateError extends Error {} + +/** + * A class to poll synapse's report endpoint, so we can act on new reports + * + * @param mjolnir The running Mjolnir instance + * @param manager The report manager in to which we feed new reports + */ +export class ReportPoller { + /** + * https://matrix-org.github.io/synapse/latest/admin_api/event_reports.html + * "from" is an opaque token that is returned from the API to paginate reports + */ + private from = 0; + /** + * The currently-pending report poll + */ + private timeout: ReturnType | null = null; + + constructor( + private mjolnir: Mjolnir, + private manager: ReportManager, + ) { } + + private schedulePoll() { + if (this.timeout === null) { + /* + * Important that we use `setTimeout` here, not `setInterval`, + * because if there's networking problems and `getAbuseReports` + * hangs for longer thank the interval, it could cause a stampede + * of requests when networking problems resolve + */ + this.timeout = setTimeout( + this.tryGetAbuseReports.bind(this), + 30_000 // a minute in milliseconds + ); + } else { + throw new InvalidStateError("poll already scheduled"); + } + } + + private async getAbuseReports() { + let response_: { + event_reports: { room_id: string, event_id: string, sender: string, reason: string }[], + next_token: number | undefined + } | undefined; + try { + response_ = await this.mjolnir.client.doRequest( + "GET", + "/_synapse/admin/v1/event_reports", + { from: this.from.toString() } + ); + } catch (ex) { + await this.mjolnir.logMessage(LogLevel.ERROR, "getAbuseReports", `failed to poll events: ${ex}`); + return; + } + + const response = response_!; + for (let report of response.event_reports) { + if (!(report.room_id in this.mjolnir.protectedRooms)) { + continue; + } + + let event: any; // `any` because `handleServerAbuseReport` uses `any` + try { + event = (await this.mjolnir.client.doRequest( + "GET", + `/_synapse/admin/v1/rooms/${report.room_id}/context/${report.event_id}?limit=1` + )).event; + } catch (ex) { + this.mjolnir.logMessage(LogLevel.ERROR, "getAbuseReports", `failed to get context: ${ex}`); + continue; + } + + await this.manager.handleServerAbuseReport({ + roomId: report.room_id, + reporterId: report.sender, + event: event, + reason: report.reason, + }); + } + + /* + * This API endpoint returns an opaque `next_token` number that we + * need to give back to subsequent requests for pagination, so here we + * save it in account data + */ + if (response.next_token !== undefined) { + this.from = response.next_token; + try { + await this.mjolnir.client.setAccountData(REPORT_POLL_EVENT_TYPE, { from: response.next_token }); + } catch (ex) { + await this.mjolnir.logMessage(LogLevel.ERROR, "getAbuseReports", `failed to update progress: ${ex}`); + } + } + } + + private async tryGetAbuseReports() { + this.timeout = null; + + try { + await this.getAbuseReports() + } catch (ex) { + await this.mjolnir.logMessage(LogLevel.ERROR, "tryGetAbuseReports", `failed to get abuse reports: ${ex}`); + } + + this.schedulePoll(); + } + public start(startFrom: number) { + if (this.timeout === null) { + this.from = startFrom; + this.schedulePoll(); + } else { + throw new InvalidStateError("cannot start an already started poll"); + } + } + public stop() { + if (this.timeout !== null) { + clearTimeout(this.timeout); + this.timeout = null; + } else { + throw new InvalidStateError("cannot stop a poll that hasn't started"); + } + } +} diff --git a/test/integration/reportPollingTest.ts b/test/integration/reportPollingTest.ts new file mode 100644 index 0000000..c5a6e66 --- /dev/null +++ b/test/integration/reportPollingTest.ts @@ -0,0 +1,54 @@ +import { strict as assert } from "assert"; + +import config from "../../src/config"; +import { Mjolnir } from "../../src/Mjolnir"; +import { IProtection } from "../../src/protections/IProtection"; +import { PROTECTIONS } from "../../src/protections/protections"; +import { ProtectionSettingValidationError } from "../../src/protections/ProtectionSettings"; +import { NumberProtectionSetting, StringProtectionSetting, StringListProtectionSetting } from "../../src/protections/ProtectionSettings"; +import { newTestUser, noticeListener } from "./clientHelper"; +import { matrixClient, mjolnir } from "./mjolnirSetupUtils"; + +describe("Test: Report polling", function() { + let client; + this.beforeEach(async function () { + client = await newTestUser({ name: { contains: "protection-settings" }}); + await client.start(); + }) + this.afterEach(async function () { + await client.stop(); + }) + it("Mjolnir correctly retrieves a report from synapse", async function() { + this.timeout(40000); + + const reportPromise = new Promise(async (resolve, reject) => { + await this.mjolnir.registerProtection(new class implements IProtection { + name = "jYvufI"; + description = "A test protection"; + settings = { }; + handleEvent = async (mjolnir: Mjolnir, roomId: string, event: any) => { }; + handleReport = (mjolnir: Mjolnir, roomId: string, reporterId: string, event: any, reason?: string) => { + if (reason === "x5h1Je") { + resolve(null); + } + }; + }); + }); + await this.mjolnir.enableProtection("jYvufI"); + + let protectedRoomId = await this.mjolnir.client.createRoom({ invite: [await client.getUserId()] }); + await client.joinRoom(protectedRoomId); + await this.mjolnir.addProtectedRoom(protectedRoomId); + + const eventId = await client.sendMessage(protectedRoomId, {msgtype: "m.text", body: "uwNd3q"}); + await client.doRequest( + "POST", + `/_matrix/client/r0/rooms/${encodeURIComponent(protectedRoomId)}/report/${encodeURIComponent(eventId)}`, "", { + reason: "x5h1Je" + } + ); + + await reportPromise; + }); +}); + diff --git a/tsconfig.json b/tsconfig.json index d4c654f..91e41d7 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -22,6 +22,7 @@ "./src/**/*", "./test/integration/manualLaunchScript.ts", "./test/integration/roomMembersTest.ts", - "./test/integration/banListTest.ts" + "./test/integration/banListTest.ts", + "./test/integration/reportPollingTest" ] } From d8aac434f1279b387cef7b59ed5d2b20f3b25f12 Mon Sep 17 00:00:00 2001 From: David Teller Date: Tue, 5 Jul 2022 14:38:53 +0200 Subject: [PATCH 04/10] Fix: roomMemberTest off-by-one error (#319) --- package.json | 1 + test/integration/roomMembersTest.ts | 97 ++++++++++++++++++++--------- yarn.lock | 12 ++++ 3 files changed, 81 insertions(+), 29 deletions(-) diff --git a/package.json b/package.json index 0abada2..f85ceb5 100644 --- a/package.json +++ b/package.json @@ -44,6 +44,7 @@ "js-yaml": "^4.1.0", "jsdom": "^16.6.0", "matrix-bot-sdk": "^0.5.19", + "node": "^18.4.0", "parse-duration": "^1.0.2", "shell-quote": "^1.7.3" }, diff --git a/test/integration/roomMembersTest.ts b/test/integration/roomMembersTest.ts index 792c911..a4017f5 100644 --- a/test/integration/roomMembersTest.ts +++ b/test/integration/roomMembersTest.ts @@ -401,25 +401,37 @@ describe("Test: Testing RoomMemberManager", function() { } // Create and protect rooms. - // - room 0 remains unprotected, as witness; - // - room 1 is protected but won't be targeted directly, also as witness. + // + // We reserve two control rooms: + // - room 0, also known as the "control unprotected room" is unprotected + // (we're not calling `!mjolnir rooms add` for this room), so none + // of the operations of `!mjolnir since` shoud affect it. We are + // using it to control, at the end of each experiment, that none of + // the `!mjolnir since` operations affect it. + // - room 1, also known as the "control protected room" is protected + // (we are calling `!mjolnir rooms add` for this room), but we are + // never directly requesting any `!mjolnir since` action against + // this room. We are using it to control, at the end of each experiment, + // that none of the `!mjolnir since` operations that should target + // one single other room also affect that room. It is, however, affected + // by general operations that are designed to affect all protected rooms. const NUMBER_OF_ROOMS = 18; - const roomIds: string[] = []; - const roomAliases: string[] = []; + const allRoomIds: string[] = []; + const allRoomAliases: string[] = []; const mjolnirUserId = await this.mjolnir.client.getUserId(); for (let i = 0; i < NUMBER_OF_ROOMS; ++i) { const roomId = await this.moderator.createRoom({ invite: [mjolnirUserId, ...goodUserIds, ...badUserIds], }); - roomIds.push(roomId); + allRoomIds.push(roomId); const alias = `#since-test-${randomUUID()}:localhost:9999`; await this.moderator.createRoomAlias(alias, roomId); - roomAliases.push(alias); + allRoomAliases.push(alias); } - for (let i = 1; i < roomIds.length; ++i) { - // Protect all rooms except roomIds[0], as witness. - const roomId = roomIds[i]; + for (let i = 1; i < allRoomIds.length; ++i) { + // Protect all rooms except allRoomIds[0], as control. + const roomId = allRoomIds[i]; await this.mjolnir.client.joinRoom(roomId); await this.moderator.setUserPowerLevel(mjolnirUserId, roomId, 100); await this.moderator.sendMessage(this.mjolnir.managementRoomId, { msgtype: 'm.text', body: `!mjolnir rooms add ${roomId}` }); @@ -429,8 +441,8 @@ describe("Test: Testing RoomMemberManager", function() { do { let protectedRooms = this.mjolnir.protectedRooms; protectedRoomsUpdated = true; - for (let i = 1; i < roomIds.length; ++i) { - const roomId = roomIds[i]; + for (let i = 1; i < allRoomIds.length; ++i) { + const roomId = allRoomIds[i]; if (!(roomId in protectedRooms)) { protectedRoomsUpdated = false; await new Promise(resolve => setTimeout(resolve, 1_000)); @@ -440,7 +452,7 @@ describe("Test: Testing RoomMemberManager", function() { // Good users join before cut date. for (let user of this.goodUsers) { - for (let roomId of roomIds) { + for (let roomId of allRoomIds) { await user.joinRoom(roomId); } } @@ -453,25 +465,30 @@ describe("Test: Testing RoomMemberManager", function() { // Bad users join after cut date. for (let user of this.badUsers) { - for (let roomId of roomIds) { + for (let roomId of allRoomIds) { await user.joinRoom(roomId); } } + // Finally, prepare our control rooms and separate them + // from the regular rooms. + const CONTROL_UNPROTECTED_ROOM_ID = allRoomIds[0]; + const CONTROL_PROTECTED_ID = allRoomIds[1]; + const roomIds = allRoomIds.slice(2); + const roomAliases = allRoomAliases.slice(2); + enum Method { kick, ban, mute, unmute, } - const WITNESS_UNPROTECTED_ROOM_ID = roomIds[0]; - const WITNESS_ROOM_ID = roomIds[1]; class Experiment { // A human-readable name for the command. readonly name: string; - // If `true`, this command should affect room `WITNESS_ROOM_ID`. + // If `true`, this command should affect room `CONTROL_PROTECTED_ID`. // Defaults to `false`. - readonly shouldAffectWitnessRoom: boolean; + readonly shouldAffectControlProtected: boolean; // The actual command-line. readonly command: (roomId: string, roomAlias: string) => string; // The number of responses we expect to this command. @@ -484,17 +501,23 @@ describe("Test: Testing RoomMemberManager", function() { // Defaults to `false`. readonly isSameRoomAsPrevious: boolean; + // The index of the room on which we're acting. + // + // Initialized by `addTo`. roomIndex: number | undefined; - constructor({name, shouldAffectWitnessRoom, command, n, method, sameRoom}: {name: string, command: (roomId: string, roomAlias: string) => string, shouldAffectWitnessRoom?: boolean, n?: number, method: Method, sameRoom?: boolean}) { + constructor({name, shouldAffectControlProtected, command, n, method, sameRoom}: {name: string, command: (roomId: string, roomAlias: string) => string, shouldAffectControlProtected?: boolean, n?: number, method: Method, sameRoom?: boolean}) { this.name = name; - this.shouldAffectWitnessRoom = typeof shouldAffectWitnessRoom === "undefined" ? false : shouldAffectWitnessRoom; + this.shouldAffectControlProtected = typeof shouldAffectControlProtected === "undefined" ? false : shouldAffectControlProtected; this.command = command; this.n = typeof n === "undefined" ? 1 : n; this.method = method; this.isSameRoomAsPrevious = typeof sameRoom === "undefined" ? false : sameRoom; } + // Add an experiment to the list of experiments. + // + // This is how `roomIndex` gets initialized. addTo(experiments: Experiment[]) { if (this.isSameRoomAsPrevious) { this.roomIndex = experiments[experiments.length - 1].roomIndex; @@ -586,7 +609,7 @@ describe("Test: Testing RoomMemberManager", function() { new Experiment({ name: "kick with date and reason", command: (roomId: string) => `!mjolnir since "${cutDate}" kick 100 ${roomId} bad, bad user`, - shouldAffectWitnessRoom: false, + shouldAffectControlProtected: false, n: 1, method: Method.kick, }), @@ -626,19 +649,35 @@ describe("Test: Testing RoomMemberManager", function() { new Experiment({ name: "kick with date everywhere", command: () => `!mjolnir since "${cutDate}" kick 100 * bad, bad user`, - shouldAffectWitnessRoom: true, + shouldAffectControlProtected: true, n: NUMBER_OF_ROOMS - 1, method: Method.kick, }), ]) { experiment.addTo(EXPERIMENTS); } + + // Just-in-case health check, before starting. + { + const usersInUnprotectedControlProtected = await this.mjolnir.client.getJoinedRoomMembers(CONTROL_UNPROTECTED_ROOM_ID); + const usersInControlProtected = await this.mjolnir.client.getJoinedRoomMembers(CONTROL_PROTECTED_ID); + for (let userId of goodUserIds) { + assert.ok(usersInUnprotectedControlProtected.includes(userId), `Initially, good user ${userId} should be in the unprotected control room`); + assert.ok(usersInControlProtected.includes(userId), `Initially, good user ${userId} should be in the control room`); + } + for (let userId of badUserIds) { + assert.ok(usersInUnprotectedControlProtected.includes(userId), `Initially, bad user ${userId} should be in the unprotected control room`); + assert.ok(usersInControlProtected.includes(userId), `Initially, bad user ${userId} should be in the control room`); + } + } + for (let i = 0; i < EXPERIMENTS.length; ++i) { const experiment = EXPERIMENTS[i]; - const index = experiment.roomIndex! + 1; - const roomId = roomIds[index]; + const index = experiment.roomIndex!; + const roomId = roomIds[index]; const roomAlias = roomAliases[index]; const joined = this.mjolnir.roomJoins.getUsersInRoom(roomId, start, 100); + console.debug(`Running experiment ${i} "${experiment.name}" in room index ${index} (${roomId} / ${roomAlias}): \`${experiment.command(roomId, roomAlias)}\``); assert.ok(joined.length >= 2 * SAMPLE_SIZE, `In experiment ${experiment.name}, we should have seen ${2 * SAMPLE_SIZE} users, saw ${joined.length}`); // Run experiment. @@ -650,12 +689,12 @@ describe("Test: Testing RoomMemberManager", function() { // Check post-conditions. const usersInRoom = await this.mjolnir.client.getJoinedRoomMembers(roomId); - const usersInUnprotectedWitnessRoom = await this.mjolnir.client.getJoinedRoomMembers(WITNESS_UNPROTECTED_ROOM_ID); - const usersInWitnessRoom = await this.mjolnir.client.getJoinedRoomMembers(WITNESS_ROOM_ID); + const usersInUnprotectedControlProtected = await this.mjolnir.client.getJoinedRoomMembers(CONTROL_UNPROTECTED_ROOM_ID); + const usersInControlProtected = await this.mjolnir.client.getJoinedRoomMembers(CONTROL_PROTECTED_ID); for (let userId of goodUserIds) { assert.ok(usersInRoom.includes(userId), `After a ${experiment.name}, good user ${userId} should still be in affected room`); - assert.ok(usersInWitnessRoom.includes(userId), `After a ${experiment.name}, good user ${userId} should still be in witness room`); - assert.ok(usersInUnprotectedWitnessRoom.includes(userId), `After a ${experiment.name}, good user ${userId} should still be in unprotected witness room`); + assert.ok(usersInControlProtected.includes(userId), `After a ${experiment.name}, good user ${userId} should still be in control room (${CONTROL_PROTECTED_ID})`); + assert.ok(usersInUnprotectedControlProtected.includes(userId), `After a ${experiment.name}, good user ${userId} should still be in unprotected control room (${CONTROL_UNPROTECTED_ROOM_ID})`); } if (experiment.method === Method.mute) { for (let userId of goodUserIds) { @@ -678,8 +717,8 @@ describe("Test: Testing RoomMemberManager", function() { } else { for (let userId of badUserIds) { assert.ok(!usersInRoom.includes(userId), `After a ${experiment.name}, bad user ${userId} should NOT be in affected room`); - assert.equal(usersInWitnessRoom.includes(userId), !experiment.shouldAffectWitnessRoom, `After a ${experiment.name}, bad user ${userId} should ${experiment.shouldAffectWitnessRoom ? "NOT" : "still"} be in witness room`); - assert.ok(usersInUnprotectedWitnessRoom.includes(userId), `After a ${experiment.name}, bad user ${userId} should still be in unprotected witness room`); + assert.equal(usersInControlProtected.includes(userId), !experiment.shouldAffectControlProtected, `After a ${experiment.name}, bad user ${userId} should ${experiment.shouldAffectControlProtected ? "NOT" : "still"} be in control room`); + assert.ok(usersInUnprotectedControlProtected.includes(userId), `After a ${experiment.name}, bad user ${userId} should still be in unprotected control room`); const leaveEvent = await this.mjolnir.client.getRoomStateEvent(roomId, "m.room.member", userId); switch (experiment.method) { case Method.kick: diff --git a/yarn.lock b/yarn.lock index 21df715..25469bf 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2061,6 +2061,18 @@ negotiator@0.6.3: resolved "https://registry.yarnpkg.com/negotiator/-/negotiator-0.6.3.tgz#58e323a72fedc0d6f9cd4d31fe49f51479590ccd" integrity sha512-+EUsqGPLsM+j/zdChZjsnX51g4XrHFOIXwfnCVPGlQk/k5giakcKsuxCObBRu6DSm9opw/O6slWbJdghQM4bBg== +node-bin-setup@^1.0.0: + version "1.1.0" + resolved "https://registry.yarnpkg.com/node-bin-setup/-/node-bin-setup-1.1.0.tgz#9df94c41335a8f41958a639b2736f860582a209c" + integrity sha512-pTeU6NgUrexiLNtd+AKwvg6cngHMvj5FZ5e2bbv2ogBSIc9yhkXSSaTScfSRZnwHIh5YFmYSYlemLWkiKD7rog== + +node@^18.4.0: + version "18.4.0" + resolved "https://registry.yarnpkg.com/node/-/node-18.4.0.tgz#94e9a09f7303ad5cc64bc60950dba78e9a5664c5" + integrity sha512-uOeU340PxuLdddq9IWLBNrPU1x/ndr+p7UE3C9D+hHeXqjWC7zfe+QY67pfis/1Z5e0WXIImU5eBj7tfOpHIVg== + dependencies: + node-bin-setup "^1.0.0" + normalize-path@^3.0.0, normalize-path@~3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/normalize-path/-/normalize-path-3.0.0.tgz#0dcd69ff23a1c9b11fd0978316644a0388216a65" From cb34af02c628fdf0b6bbd45df2b14dde9319e010 Mon Sep 17 00:00:00 2001 From: David Teller Date: Tue, 5 Jul 2022 15:29:01 +0200 Subject: [PATCH 05/10] Revert "Fix: roomMemberTest off-by-one error (#319)" (#323) This reverts commit d8aac434f1279b387cef7b59ed5d2b20f3b25f12. --- package.json | 1 - test/integration/roomMembersTest.ts | 97 +++++++++-------------------- yarn.lock | 12 ---- 3 files changed, 29 insertions(+), 81 deletions(-) diff --git a/package.json b/package.json index f85ceb5..0abada2 100644 --- a/package.json +++ b/package.json @@ -44,7 +44,6 @@ "js-yaml": "^4.1.0", "jsdom": "^16.6.0", "matrix-bot-sdk": "^0.5.19", - "node": "^18.4.0", "parse-duration": "^1.0.2", "shell-quote": "^1.7.3" }, diff --git a/test/integration/roomMembersTest.ts b/test/integration/roomMembersTest.ts index a4017f5..792c911 100644 --- a/test/integration/roomMembersTest.ts +++ b/test/integration/roomMembersTest.ts @@ -401,37 +401,25 @@ describe("Test: Testing RoomMemberManager", function() { } // Create and protect rooms. - // - // We reserve two control rooms: - // - room 0, also known as the "control unprotected room" is unprotected - // (we're not calling `!mjolnir rooms add` for this room), so none - // of the operations of `!mjolnir since` shoud affect it. We are - // using it to control, at the end of each experiment, that none of - // the `!mjolnir since` operations affect it. - // - room 1, also known as the "control protected room" is protected - // (we are calling `!mjolnir rooms add` for this room), but we are - // never directly requesting any `!mjolnir since` action against - // this room. We are using it to control, at the end of each experiment, - // that none of the `!mjolnir since` operations that should target - // one single other room also affect that room. It is, however, affected - // by general operations that are designed to affect all protected rooms. + // - room 0 remains unprotected, as witness; + // - room 1 is protected but won't be targeted directly, also as witness. const NUMBER_OF_ROOMS = 18; - const allRoomIds: string[] = []; - const allRoomAliases: string[] = []; + const roomIds: string[] = []; + const roomAliases: string[] = []; const mjolnirUserId = await this.mjolnir.client.getUserId(); for (let i = 0; i < NUMBER_OF_ROOMS; ++i) { const roomId = await this.moderator.createRoom({ invite: [mjolnirUserId, ...goodUserIds, ...badUserIds], }); - allRoomIds.push(roomId); + roomIds.push(roomId); const alias = `#since-test-${randomUUID()}:localhost:9999`; await this.moderator.createRoomAlias(alias, roomId); - allRoomAliases.push(alias); + roomAliases.push(alias); } - for (let i = 1; i < allRoomIds.length; ++i) { - // Protect all rooms except allRoomIds[0], as control. - const roomId = allRoomIds[i]; + for (let i = 1; i < roomIds.length; ++i) { + // Protect all rooms except roomIds[0], as witness. + const roomId = roomIds[i]; await this.mjolnir.client.joinRoom(roomId); await this.moderator.setUserPowerLevel(mjolnirUserId, roomId, 100); await this.moderator.sendMessage(this.mjolnir.managementRoomId, { msgtype: 'm.text', body: `!mjolnir rooms add ${roomId}` }); @@ -441,8 +429,8 @@ describe("Test: Testing RoomMemberManager", function() { do { let protectedRooms = this.mjolnir.protectedRooms; protectedRoomsUpdated = true; - for (let i = 1; i < allRoomIds.length; ++i) { - const roomId = allRoomIds[i]; + for (let i = 1; i < roomIds.length; ++i) { + const roomId = roomIds[i]; if (!(roomId in protectedRooms)) { protectedRoomsUpdated = false; await new Promise(resolve => setTimeout(resolve, 1_000)); @@ -452,7 +440,7 @@ describe("Test: Testing RoomMemberManager", function() { // Good users join before cut date. for (let user of this.goodUsers) { - for (let roomId of allRoomIds) { + for (let roomId of roomIds) { await user.joinRoom(roomId); } } @@ -465,30 +453,25 @@ describe("Test: Testing RoomMemberManager", function() { // Bad users join after cut date. for (let user of this.badUsers) { - for (let roomId of allRoomIds) { + for (let roomId of roomIds) { await user.joinRoom(roomId); } } - // Finally, prepare our control rooms and separate them - // from the regular rooms. - const CONTROL_UNPROTECTED_ROOM_ID = allRoomIds[0]; - const CONTROL_PROTECTED_ID = allRoomIds[1]; - const roomIds = allRoomIds.slice(2); - const roomAliases = allRoomAliases.slice(2); - enum Method { kick, ban, mute, unmute, } + const WITNESS_UNPROTECTED_ROOM_ID = roomIds[0]; + const WITNESS_ROOM_ID = roomIds[1]; class Experiment { // A human-readable name for the command. readonly name: string; - // If `true`, this command should affect room `CONTROL_PROTECTED_ID`. + // If `true`, this command should affect room `WITNESS_ROOM_ID`. // Defaults to `false`. - readonly shouldAffectControlProtected: boolean; + readonly shouldAffectWitnessRoom: boolean; // The actual command-line. readonly command: (roomId: string, roomAlias: string) => string; // The number of responses we expect to this command. @@ -501,23 +484,17 @@ describe("Test: Testing RoomMemberManager", function() { // Defaults to `false`. readonly isSameRoomAsPrevious: boolean; - // The index of the room on which we're acting. - // - // Initialized by `addTo`. roomIndex: number | undefined; - constructor({name, shouldAffectControlProtected, command, n, method, sameRoom}: {name: string, command: (roomId: string, roomAlias: string) => string, shouldAffectControlProtected?: boolean, n?: number, method: Method, sameRoom?: boolean}) { + constructor({name, shouldAffectWitnessRoom, command, n, method, sameRoom}: {name: string, command: (roomId: string, roomAlias: string) => string, shouldAffectWitnessRoom?: boolean, n?: number, method: Method, sameRoom?: boolean}) { this.name = name; - this.shouldAffectControlProtected = typeof shouldAffectControlProtected === "undefined" ? false : shouldAffectControlProtected; + this.shouldAffectWitnessRoom = typeof shouldAffectWitnessRoom === "undefined" ? false : shouldAffectWitnessRoom; this.command = command; this.n = typeof n === "undefined" ? 1 : n; this.method = method; this.isSameRoomAsPrevious = typeof sameRoom === "undefined" ? false : sameRoom; } - // Add an experiment to the list of experiments. - // - // This is how `roomIndex` gets initialized. addTo(experiments: Experiment[]) { if (this.isSameRoomAsPrevious) { this.roomIndex = experiments[experiments.length - 1].roomIndex; @@ -609,7 +586,7 @@ describe("Test: Testing RoomMemberManager", function() { new Experiment({ name: "kick with date and reason", command: (roomId: string) => `!mjolnir since "${cutDate}" kick 100 ${roomId} bad, bad user`, - shouldAffectControlProtected: false, + shouldAffectWitnessRoom: false, n: 1, method: Method.kick, }), @@ -649,35 +626,19 @@ describe("Test: Testing RoomMemberManager", function() { new Experiment({ name: "kick with date everywhere", command: () => `!mjolnir since "${cutDate}" kick 100 * bad, bad user`, - shouldAffectControlProtected: true, + shouldAffectWitnessRoom: true, n: NUMBER_OF_ROOMS - 1, method: Method.kick, }), ]) { experiment.addTo(EXPERIMENTS); } - - // Just-in-case health check, before starting. - { - const usersInUnprotectedControlProtected = await this.mjolnir.client.getJoinedRoomMembers(CONTROL_UNPROTECTED_ROOM_ID); - const usersInControlProtected = await this.mjolnir.client.getJoinedRoomMembers(CONTROL_PROTECTED_ID); - for (let userId of goodUserIds) { - assert.ok(usersInUnprotectedControlProtected.includes(userId), `Initially, good user ${userId} should be in the unprotected control room`); - assert.ok(usersInControlProtected.includes(userId), `Initially, good user ${userId} should be in the control room`); - } - for (let userId of badUserIds) { - assert.ok(usersInUnprotectedControlProtected.includes(userId), `Initially, bad user ${userId} should be in the unprotected control room`); - assert.ok(usersInControlProtected.includes(userId), `Initially, bad user ${userId} should be in the control room`); - } - } - for (let i = 0; i < EXPERIMENTS.length; ++i) { const experiment = EXPERIMENTS[i]; - const index = experiment.roomIndex!; - const roomId = roomIds[index]; + const index = experiment.roomIndex! + 1; + const roomId = roomIds[index]; const roomAlias = roomAliases[index]; const joined = this.mjolnir.roomJoins.getUsersInRoom(roomId, start, 100); - console.debug(`Running experiment ${i} "${experiment.name}" in room index ${index} (${roomId} / ${roomAlias}): \`${experiment.command(roomId, roomAlias)}\``); assert.ok(joined.length >= 2 * SAMPLE_SIZE, `In experiment ${experiment.name}, we should have seen ${2 * SAMPLE_SIZE} users, saw ${joined.length}`); // Run experiment. @@ -689,12 +650,12 @@ describe("Test: Testing RoomMemberManager", function() { // Check post-conditions. const usersInRoom = await this.mjolnir.client.getJoinedRoomMembers(roomId); - const usersInUnprotectedControlProtected = await this.mjolnir.client.getJoinedRoomMembers(CONTROL_UNPROTECTED_ROOM_ID); - const usersInControlProtected = await this.mjolnir.client.getJoinedRoomMembers(CONTROL_PROTECTED_ID); + const usersInUnprotectedWitnessRoom = await this.mjolnir.client.getJoinedRoomMembers(WITNESS_UNPROTECTED_ROOM_ID); + const usersInWitnessRoom = await this.mjolnir.client.getJoinedRoomMembers(WITNESS_ROOM_ID); for (let userId of goodUserIds) { assert.ok(usersInRoom.includes(userId), `After a ${experiment.name}, good user ${userId} should still be in affected room`); - assert.ok(usersInControlProtected.includes(userId), `After a ${experiment.name}, good user ${userId} should still be in control room (${CONTROL_PROTECTED_ID})`); - assert.ok(usersInUnprotectedControlProtected.includes(userId), `After a ${experiment.name}, good user ${userId} should still be in unprotected control room (${CONTROL_UNPROTECTED_ROOM_ID})`); + assert.ok(usersInWitnessRoom.includes(userId), `After a ${experiment.name}, good user ${userId} should still be in witness room`); + assert.ok(usersInUnprotectedWitnessRoom.includes(userId), `After a ${experiment.name}, good user ${userId} should still be in unprotected witness room`); } if (experiment.method === Method.mute) { for (let userId of goodUserIds) { @@ -717,8 +678,8 @@ describe("Test: Testing RoomMemberManager", function() { } else { for (let userId of badUserIds) { assert.ok(!usersInRoom.includes(userId), `After a ${experiment.name}, bad user ${userId} should NOT be in affected room`); - assert.equal(usersInControlProtected.includes(userId), !experiment.shouldAffectControlProtected, `After a ${experiment.name}, bad user ${userId} should ${experiment.shouldAffectControlProtected ? "NOT" : "still"} be in control room`); - assert.ok(usersInUnprotectedControlProtected.includes(userId), `After a ${experiment.name}, bad user ${userId} should still be in unprotected control room`); + assert.equal(usersInWitnessRoom.includes(userId), !experiment.shouldAffectWitnessRoom, `After a ${experiment.name}, bad user ${userId} should ${experiment.shouldAffectWitnessRoom ? "NOT" : "still"} be in witness room`); + assert.ok(usersInUnprotectedWitnessRoom.includes(userId), `After a ${experiment.name}, bad user ${userId} should still be in unprotected witness room`); const leaveEvent = await this.mjolnir.client.getRoomStateEvent(roomId, "m.room.member", userId); switch (experiment.method) { case Method.kick: diff --git a/yarn.lock b/yarn.lock index 25469bf..21df715 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2061,18 +2061,6 @@ negotiator@0.6.3: resolved "https://registry.yarnpkg.com/negotiator/-/negotiator-0.6.3.tgz#58e323a72fedc0d6f9cd4d31fe49f51479590ccd" integrity sha512-+EUsqGPLsM+j/zdChZjsnX51g4XrHFOIXwfnCVPGlQk/k5giakcKsuxCObBRu6DSm9opw/O6slWbJdghQM4bBg== -node-bin-setup@^1.0.0: - version "1.1.0" - resolved "https://registry.yarnpkg.com/node-bin-setup/-/node-bin-setup-1.1.0.tgz#9df94c41335a8f41958a639b2736f860582a209c" - integrity sha512-pTeU6NgUrexiLNtd+AKwvg6cngHMvj5FZ5e2bbv2ogBSIc9yhkXSSaTScfSRZnwHIh5YFmYSYlemLWkiKD7rog== - -node@^18.4.0: - version "18.4.0" - resolved "https://registry.yarnpkg.com/node/-/node-18.4.0.tgz#94e9a09f7303ad5cc64bc60950dba78e9a5664c5" - integrity sha512-uOeU340PxuLdddq9IWLBNrPU1x/ndr+p7UE3C9D+hHeXqjWC7zfe+QY67pfis/1Z5e0WXIImU5eBj7tfOpHIVg== - dependencies: - node-bin-setup "^1.0.0" - normalize-path@^3.0.0, normalize-path@~3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/normalize-path/-/normalize-path-3.0.0.tgz#0dcd69ff23a1c9b11fd0978316644a0388216a65" From 6e5d52056657bee9db65ebfe886b26034a18fb04 Mon Sep 17 00:00:00 2001 From: David Teller Date: Tue, 5 Jul 2022 15:33:53 +0200 Subject: [PATCH 06/10] Fix: roomMemberTest off-by-one error (#324) --- test/integration/roomMembersTest.ts | 97 ++++++++++++++++++++--------- 1 file changed, 68 insertions(+), 29 deletions(-) diff --git a/test/integration/roomMembersTest.ts b/test/integration/roomMembersTest.ts index 792c911..a4017f5 100644 --- a/test/integration/roomMembersTest.ts +++ b/test/integration/roomMembersTest.ts @@ -401,25 +401,37 @@ describe("Test: Testing RoomMemberManager", function() { } // Create and protect rooms. - // - room 0 remains unprotected, as witness; - // - room 1 is protected but won't be targeted directly, also as witness. + // + // We reserve two control rooms: + // - room 0, also known as the "control unprotected room" is unprotected + // (we're not calling `!mjolnir rooms add` for this room), so none + // of the operations of `!mjolnir since` shoud affect it. We are + // using it to control, at the end of each experiment, that none of + // the `!mjolnir since` operations affect it. + // - room 1, also known as the "control protected room" is protected + // (we are calling `!mjolnir rooms add` for this room), but we are + // never directly requesting any `!mjolnir since` action against + // this room. We are using it to control, at the end of each experiment, + // that none of the `!mjolnir since` operations that should target + // one single other room also affect that room. It is, however, affected + // by general operations that are designed to affect all protected rooms. const NUMBER_OF_ROOMS = 18; - const roomIds: string[] = []; - const roomAliases: string[] = []; + const allRoomIds: string[] = []; + const allRoomAliases: string[] = []; const mjolnirUserId = await this.mjolnir.client.getUserId(); for (let i = 0; i < NUMBER_OF_ROOMS; ++i) { const roomId = await this.moderator.createRoom({ invite: [mjolnirUserId, ...goodUserIds, ...badUserIds], }); - roomIds.push(roomId); + allRoomIds.push(roomId); const alias = `#since-test-${randomUUID()}:localhost:9999`; await this.moderator.createRoomAlias(alias, roomId); - roomAliases.push(alias); + allRoomAliases.push(alias); } - for (let i = 1; i < roomIds.length; ++i) { - // Protect all rooms except roomIds[0], as witness. - const roomId = roomIds[i]; + for (let i = 1; i < allRoomIds.length; ++i) { + // Protect all rooms except allRoomIds[0], as control. + const roomId = allRoomIds[i]; await this.mjolnir.client.joinRoom(roomId); await this.moderator.setUserPowerLevel(mjolnirUserId, roomId, 100); await this.moderator.sendMessage(this.mjolnir.managementRoomId, { msgtype: 'm.text', body: `!mjolnir rooms add ${roomId}` }); @@ -429,8 +441,8 @@ describe("Test: Testing RoomMemberManager", function() { do { let protectedRooms = this.mjolnir.protectedRooms; protectedRoomsUpdated = true; - for (let i = 1; i < roomIds.length; ++i) { - const roomId = roomIds[i]; + for (let i = 1; i < allRoomIds.length; ++i) { + const roomId = allRoomIds[i]; if (!(roomId in protectedRooms)) { protectedRoomsUpdated = false; await new Promise(resolve => setTimeout(resolve, 1_000)); @@ -440,7 +452,7 @@ describe("Test: Testing RoomMemberManager", function() { // Good users join before cut date. for (let user of this.goodUsers) { - for (let roomId of roomIds) { + for (let roomId of allRoomIds) { await user.joinRoom(roomId); } } @@ -453,25 +465,30 @@ describe("Test: Testing RoomMemberManager", function() { // Bad users join after cut date. for (let user of this.badUsers) { - for (let roomId of roomIds) { + for (let roomId of allRoomIds) { await user.joinRoom(roomId); } } + // Finally, prepare our control rooms and separate them + // from the regular rooms. + const CONTROL_UNPROTECTED_ROOM_ID = allRoomIds[0]; + const CONTROL_PROTECTED_ID = allRoomIds[1]; + const roomIds = allRoomIds.slice(2); + const roomAliases = allRoomAliases.slice(2); + enum Method { kick, ban, mute, unmute, } - const WITNESS_UNPROTECTED_ROOM_ID = roomIds[0]; - const WITNESS_ROOM_ID = roomIds[1]; class Experiment { // A human-readable name for the command. readonly name: string; - // If `true`, this command should affect room `WITNESS_ROOM_ID`. + // If `true`, this command should affect room `CONTROL_PROTECTED_ID`. // Defaults to `false`. - readonly shouldAffectWitnessRoom: boolean; + readonly shouldAffectControlProtected: boolean; // The actual command-line. readonly command: (roomId: string, roomAlias: string) => string; // The number of responses we expect to this command. @@ -484,17 +501,23 @@ describe("Test: Testing RoomMemberManager", function() { // Defaults to `false`. readonly isSameRoomAsPrevious: boolean; + // The index of the room on which we're acting. + // + // Initialized by `addTo`. roomIndex: number | undefined; - constructor({name, shouldAffectWitnessRoom, command, n, method, sameRoom}: {name: string, command: (roomId: string, roomAlias: string) => string, shouldAffectWitnessRoom?: boolean, n?: number, method: Method, sameRoom?: boolean}) { + constructor({name, shouldAffectControlProtected, command, n, method, sameRoom}: {name: string, command: (roomId: string, roomAlias: string) => string, shouldAffectControlProtected?: boolean, n?: number, method: Method, sameRoom?: boolean}) { this.name = name; - this.shouldAffectWitnessRoom = typeof shouldAffectWitnessRoom === "undefined" ? false : shouldAffectWitnessRoom; + this.shouldAffectControlProtected = typeof shouldAffectControlProtected === "undefined" ? false : shouldAffectControlProtected; this.command = command; this.n = typeof n === "undefined" ? 1 : n; this.method = method; this.isSameRoomAsPrevious = typeof sameRoom === "undefined" ? false : sameRoom; } + // Add an experiment to the list of experiments. + // + // This is how `roomIndex` gets initialized. addTo(experiments: Experiment[]) { if (this.isSameRoomAsPrevious) { this.roomIndex = experiments[experiments.length - 1].roomIndex; @@ -586,7 +609,7 @@ describe("Test: Testing RoomMemberManager", function() { new Experiment({ name: "kick with date and reason", command: (roomId: string) => `!mjolnir since "${cutDate}" kick 100 ${roomId} bad, bad user`, - shouldAffectWitnessRoom: false, + shouldAffectControlProtected: false, n: 1, method: Method.kick, }), @@ -626,19 +649,35 @@ describe("Test: Testing RoomMemberManager", function() { new Experiment({ name: "kick with date everywhere", command: () => `!mjolnir since "${cutDate}" kick 100 * bad, bad user`, - shouldAffectWitnessRoom: true, + shouldAffectControlProtected: true, n: NUMBER_OF_ROOMS - 1, method: Method.kick, }), ]) { experiment.addTo(EXPERIMENTS); } + + // Just-in-case health check, before starting. + { + const usersInUnprotectedControlProtected = await this.mjolnir.client.getJoinedRoomMembers(CONTROL_UNPROTECTED_ROOM_ID); + const usersInControlProtected = await this.mjolnir.client.getJoinedRoomMembers(CONTROL_PROTECTED_ID); + for (let userId of goodUserIds) { + assert.ok(usersInUnprotectedControlProtected.includes(userId), `Initially, good user ${userId} should be in the unprotected control room`); + assert.ok(usersInControlProtected.includes(userId), `Initially, good user ${userId} should be in the control room`); + } + for (let userId of badUserIds) { + assert.ok(usersInUnprotectedControlProtected.includes(userId), `Initially, bad user ${userId} should be in the unprotected control room`); + assert.ok(usersInControlProtected.includes(userId), `Initially, bad user ${userId} should be in the control room`); + } + } + for (let i = 0; i < EXPERIMENTS.length; ++i) { const experiment = EXPERIMENTS[i]; - const index = experiment.roomIndex! + 1; - const roomId = roomIds[index]; + const index = experiment.roomIndex!; + const roomId = roomIds[index]; const roomAlias = roomAliases[index]; const joined = this.mjolnir.roomJoins.getUsersInRoom(roomId, start, 100); + console.debug(`Running experiment ${i} "${experiment.name}" in room index ${index} (${roomId} / ${roomAlias}): \`${experiment.command(roomId, roomAlias)}\``); assert.ok(joined.length >= 2 * SAMPLE_SIZE, `In experiment ${experiment.name}, we should have seen ${2 * SAMPLE_SIZE} users, saw ${joined.length}`); // Run experiment. @@ -650,12 +689,12 @@ describe("Test: Testing RoomMemberManager", function() { // Check post-conditions. const usersInRoom = await this.mjolnir.client.getJoinedRoomMembers(roomId); - const usersInUnprotectedWitnessRoom = await this.mjolnir.client.getJoinedRoomMembers(WITNESS_UNPROTECTED_ROOM_ID); - const usersInWitnessRoom = await this.mjolnir.client.getJoinedRoomMembers(WITNESS_ROOM_ID); + const usersInUnprotectedControlProtected = await this.mjolnir.client.getJoinedRoomMembers(CONTROL_UNPROTECTED_ROOM_ID); + const usersInControlProtected = await this.mjolnir.client.getJoinedRoomMembers(CONTROL_PROTECTED_ID); for (let userId of goodUserIds) { assert.ok(usersInRoom.includes(userId), `After a ${experiment.name}, good user ${userId} should still be in affected room`); - assert.ok(usersInWitnessRoom.includes(userId), `After a ${experiment.name}, good user ${userId} should still be in witness room`); - assert.ok(usersInUnprotectedWitnessRoom.includes(userId), `After a ${experiment.name}, good user ${userId} should still be in unprotected witness room`); + assert.ok(usersInControlProtected.includes(userId), `After a ${experiment.name}, good user ${userId} should still be in control room (${CONTROL_PROTECTED_ID})`); + assert.ok(usersInUnprotectedControlProtected.includes(userId), `After a ${experiment.name}, good user ${userId} should still be in unprotected control room (${CONTROL_UNPROTECTED_ROOM_ID})`); } if (experiment.method === Method.mute) { for (let userId of goodUserIds) { @@ -678,8 +717,8 @@ describe("Test: Testing RoomMemberManager", function() { } else { for (let userId of badUserIds) { assert.ok(!usersInRoom.includes(userId), `After a ${experiment.name}, bad user ${userId} should NOT be in affected room`); - assert.equal(usersInWitnessRoom.includes(userId), !experiment.shouldAffectWitnessRoom, `After a ${experiment.name}, bad user ${userId} should ${experiment.shouldAffectWitnessRoom ? "NOT" : "still"} be in witness room`); - assert.ok(usersInUnprotectedWitnessRoom.includes(userId), `After a ${experiment.name}, bad user ${userId} should still be in unprotected witness room`); + assert.equal(usersInControlProtected.includes(userId), !experiment.shouldAffectControlProtected, `After a ${experiment.name}, bad user ${userId} should ${experiment.shouldAffectControlProtected ? "NOT" : "still"} be in control room`); + assert.ok(usersInUnprotectedControlProtected.includes(userId), `After a ${experiment.name}, bad user ${userId} should still be in unprotected control room`); const leaveEvent = await this.mjolnir.client.getRoomStateEvent(roomId, "m.room.member", userId); switch (experiment.method) { case Method.kick: From ac2e736e961206e951ed6dfeec297d13cd205389 Mon Sep 17 00:00:00 2001 From: Gnuxie <50846879+Gnuxie@users.noreply.github.com> Date: Tue, 5 Jul 2022 16:58:29 +0100 Subject: [PATCH 07/10] Report polling test temporality (#325) So the test before sometimes sent the report *before* the protection (that is used to check whether we have received the report) was registered. This of course meant that we didn't ever receive the report from the perspectivee of the test. This PR should now mean we always send the report after registering the protection. --- test/integration/reportPollingTest.ts | 49 ++++++++++++--------------- 1 file changed, 21 insertions(+), 28 deletions(-) diff --git a/test/integration/reportPollingTest.ts b/test/integration/reportPollingTest.ts index c5a6e66..4e604df 100644 --- a/test/integration/reportPollingTest.ts +++ b/test/integration/reportPollingTest.ts @@ -1,27 +1,21 @@ -import { strict as assert } from "assert"; - -import config from "../../src/config"; import { Mjolnir } from "../../src/Mjolnir"; import { IProtection } from "../../src/protections/IProtection"; -import { PROTECTIONS } from "../../src/protections/protections"; -import { ProtectionSettingValidationError } from "../../src/protections/ProtectionSettings"; -import { NumberProtectionSetting, StringProtectionSetting, StringListProtectionSetting } from "../../src/protections/ProtectionSettings"; -import { newTestUser, noticeListener } from "./clientHelper"; -import { matrixClient, mjolnir } from "./mjolnirSetupUtils"; +import { newTestUser } from "./clientHelper"; describe("Test: Report polling", function() { let client; this.beforeEach(async function () { client = await newTestUser({ name: { contains: "protection-settings" }}); - await client.start(); - }) - this.afterEach(async function () { - await client.stop(); }) it("Mjolnir correctly retrieves a report from synapse", async function() { this.timeout(40000); - const reportPromise = new Promise(async (resolve, reject) => { + let protectedRoomId = await this.mjolnir.client.createRoom({ invite: [await client.getUserId()] }); + await client.joinRoom(protectedRoomId); + await this.mjolnir.addProtectedRoom(protectedRoomId); + + const eventId = await client.sendMessage(protectedRoomId, {msgtype: "m.text", body: "uwNd3q"}); + await new Promise(async resolve => { await this.mjolnir.registerProtection(new class implements IProtection { name = "jYvufI"; description = "A test protection"; @@ -33,22 +27,21 @@ describe("Test: Report polling", function() { } }; }); + await this.mjolnir.enableProtection("jYvufI"); + await client.doRequest( + "POST", + `/_matrix/client/r0/rooms/${encodeURIComponent(protectedRoomId)}/report/${encodeURIComponent(eventId)}`, "", { + reason: "x5h1Je" + } + ); }); - await this.mjolnir.enableProtection("jYvufI"); - - let protectedRoomId = await this.mjolnir.client.createRoom({ invite: [await client.getUserId()] }); - await client.joinRoom(protectedRoomId); - await this.mjolnir.addProtectedRoom(protectedRoomId); - - const eventId = await client.sendMessage(protectedRoomId, {msgtype: "m.text", body: "uwNd3q"}); - await client.doRequest( - "POST", - `/_matrix/client/r0/rooms/${encodeURIComponent(protectedRoomId)}/report/${encodeURIComponent(eventId)}`, "", { - reason: "x5h1Je" - } - ); - - await reportPromise; + // So I kid you not, it seems like we can quit before the webserver for reports sends a respond to the client (via L#26) + // because the promise above gets resolved before we finish awaiting the report sending request on L#31, + // then mocha's cleanup code runs (and shuts down the webserver) before the webserver can respond. + // Wait a minute 😲😲🤯 it's not even supposed to be using the webserver if this is testing report polling. + // Ok, well apparently that needs a big refactor to change, but if you change the config before running this test, + // then you can ensure that report polling works. https://github.com/matrix-org/mjolnir/issues/326. + await new Promise(resolve => setTimeout(resolve, 1000)); }); }); From b850e4554c6cbc9456e23ab1a92ede547d044241 Mon Sep 17 00:00:00 2001 From: Gnuxie <50846879+Gnuxie@users.noreply.github.com> Date: Wed, 6 Jul 2022 14:20:25 +0100 Subject: [PATCH 08/10] Remove debug leftovers from a test. (#314) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Remove debug leftovers from a test. This is really terrible and has meant whenever anyone has run `yarn test:integration` they have only been running this test. 💀💀💀 https://www.youtube.com/watch?v=jmX-tzSOFE0 * Set a default timeout for integration tests that is 5 minutes long. Seriously, I don't think there is much to gain by making people guess a reasnoble time for a test to complete in all the time, especially with how much Synapse changes in response time and all of the machines involved in running these tests. * Warn when giving up on being throttled * For some reason it takes longer for events to appear in /state no i am not going to track down why yet. * Rate limiting got a lot more aggresive. https://github.com/matrix-org/synapse/pull/13018 Rate limiting in Synapse used to reset the burst count and remove the backoff when you were spamming continuously, now it doesn't. Ideally we'd rewrite the rate limiting logic to back off for longer than suggested so we could get burst again, but for now lets just unblock CI by reducing the number of events we send in these tests. --- package.json | 2 +- src/utils.ts | 3 +++ test/integration/banListTest.ts | 7 ++++--- test/integration/throttleTest.ts | 16 +++++++--------- 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/package.json b/package.json index 0abada2..6594532 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ "lint": "tslint --project ./tsconfig.json -t stylish", "start:dev": "yarn build && node --async-stack-traces lib/index.js", "test": "ts-mocha --project ./tsconfig.json test/commands/**/*.ts", - "test:integration": "NODE_ENV=harness ts-mocha --async-stack-traces --require test/integration/fixtures.ts --project ./tsconfig.json \"test/integration/**/*Test.ts\"", + "test:integration": "NODE_ENV=harness ts-mocha --async-stack-traces --require test/integration/fixtures.ts --timeout 300000 --project ./tsconfig.json \"test/integration/**/*Test.ts\"", "test:manual": "NODE_ENV=harness ts-node test/integration/manualLaunchScript.ts", "version": "sed -i '/# version automated/s/[0-9][0-9]*\\.[0-9][0-9]*\\.[0-9][0-9]*/'$npm_package_version'/' synapse_antispam/setup.py && git add synapse_antispam/setup.py && cat synapse_antispam/setup.py" }, diff --git a/src/utils.ts b/src/utils.ts index b42038e..d30609f 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -386,6 +386,9 @@ function patchMatrixClientForRetry() { // We need to retry. reject(err); } else { + if (attempt >= MAX_REQUEST_ATTEMPTS) { + LogService.warn('Mjolnir.client', `Retried request ${params.method} ${params.uri} ${attempt} times, giving up.`); + } // No need-to-retry error? Lucky us! // Note that this may very well be an error, just not // one we need to retry. diff --git a/test/integration/banListTest.ts b/test/integration/banListTest.ts index a646373..66e52ef 100644 --- a/test/integration/banListTest.ts +++ b/test/integration/banListTest.ts @@ -231,7 +231,6 @@ describe('Test: We will not be able to ban ourselves via ACL.', function () { describe('Test: ACL updates will batch when rules are added in succession.', function () { it('Will batch ACL updates if we spam rules into a BanList', async function () { - this.timeout(180000) const mjolnir = config.RUNTIME.client! const serverName: string = new UserID(await mjolnir.getUserId()).domain const moderator = await newTestUser({ name: { contains: "moderator" }}); @@ -268,6 +267,8 @@ describe('Test: ACL updates will batch when rules are added in succession.', fun // 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... @@ -364,9 +365,9 @@ describe('Test: unbaning entities via the BanList.', function () { }) }) -describe.only('Test: should apply bans to the most recently active rooms first', function () { +describe('Test: should apply bans to the most recently active rooms first', function () { it('Applies bans to the most recently active rooms first', async function () { - this.timeout(6000000000) + this.timeout(180000) const mjolnir = config.RUNTIME.client! const serverName: string = new UserID(await mjolnir.getUserId()).domain const moderator = await newTestUser({ name: { contains: "moderator" }}); diff --git a/test/integration/throttleTest.ts b/test/integration/throttleTest.ts index 6c097e9..b2070c3 100644 --- a/test/integration/throttleTest.ts +++ b/test/integration/throttleTest.ts @@ -5,17 +5,16 @@ import { getFirstReaction } from "./commands/commandUtils"; describe("Test: throttled users can function with Mjolnir.", function () { it('throttled users survive being throttled by synapse', async function() { - this.timeout(60000); let throttledUser = await newTestUser({ name: { contains: "throttled" }, isThrottled: true }); let throttledUserId = await throttledUser.getUserId(); let targetRoom = await throttledUser.createRoom(); // send enough messages to hit the rate limit. - await Promise.all([...Array(150).keys()].map((i) => throttledUser.sendMessage(targetRoom, {msgtype: 'm.text.', body: `Message #${i}`}))); + await Promise.all([...Array(25).keys()].map((i) => throttledUser.sendMessage(targetRoom, {msgtype: 'm.text.', body: `Message #${i}`}))); let messageCount = 0; - await getMessagesByUserIn(throttledUser, throttledUserId, targetRoom, 150, (events) => { + await getMessagesByUserIn(throttledUser, throttledUserId, targetRoom, 25, (events) => { messageCount += events.length; }); - assert.equal(messageCount, 150, "There should have been 150 messages in this room"); + assert.equal(messageCount, 25, "There should have been 25 messages in this room"); }) }) @@ -31,7 +30,6 @@ describe("Test: Mjolnir can still sync and respond to commands while throttled", }) it('Can still perform and respond to a redaction command', async function () { - this.timeout(60000); // Create a few users and a room. let badUser = await newTestUser({ name: { contains: "spammer-needs-redacting" } }); let badUserId = await badUser.getUserId(); @@ -45,12 +43,12 @@ describe("Test: Mjolnir can still sync and respond to commands while throttled", await badUser.joinRoom(targetRoom); // Give Mjolnir some work to do and some messages to sync through. - await Promise.all([...Array(100).keys()].map((i) => moderator.sendMessage(this.mjolnir.managementRoomId, {msgtype: 'm.text.', body: `Irrelevant Message #${i}`}))); - await Promise.all([...Array(50).keys()].map(_ => moderator.sendMessage(this.mjolnir.managementRoomId, {msgtype: 'm.text', body: '!mjolnir status'}))); + await Promise.all([...Array(25).keys()].map((i) => moderator.sendMessage(this.mjolnir.managementRoomId, {msgtype: 'm.text.', body: `Irrelevant Message #${i}`}))); + await Promise.all([...Array(25).keys()].map(_ => moderator.sendMessage(this.mjolnir.managementRoomId, {msgtype: 'm.text', body: '!mjolnir status'}))); await moderator.sendMessage(this.mjolnir.managementRoomId, {msgtype: 'm.text', body: `!mjolnir rooms add ${targetRoom}`}); - await Promise.all([...Array(50).keys()].map((i) => badUser.sendMessage(targetRoom, {msgtype: 'm.text.', body: `Bad Message #${i}`}))); + await Promise.all([...Array(25).keys()].map((i) => badUser.sendMessage(targetRoom, {msgtype: 'm.text.', body: `Bad Message #${i}`}))); try { await moderator.start(); @@ -72,6 +70,6 @@ describe("Test: Mjolnir can still sync and respond to commands while throttled", } }) }); - assert.equal(count, 51, "There should be exactly 51 events from the spammer in this room."); + assert.equal(count, 26, "There should be exactly 26 events from the spammer in this room."); }) }) From 65f52fef3aac5b53b7ac9b2de53b339dd1907400 Mon Sep 17 00:00:00 2001 From: Gnuxie <50846879+Gnuxie@users.noreply.github.com> Date: Wed, 6 Jul 2022 14:21:33 +0100 Subject: [PATCH 09/10] Timeout integration tests CI after 1 hour. (#317) --- .github/workflows/mjolnir.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/mjolnir.yml b/.github/workflows/mjolnir.yml index 354f38b..ce31095 100644 --- a/.github/workflows/mjolnir.yml +++ b/.github/workflows/mjolnir.yml @@ -13,6 +13,7 @@ jobs: build: name: Integration tests runs-on: ubuntu-latest + timeout-minutes: 60 steps: - uses: actions/checkout@v2 - name: install mx-tester From 84ffb3649447085badab306a3b90e4afbcce5318 Mon Sep 17 00:00:00 2001 From: Gnuxie <50846879+Gnuxie@users.noreply.github.com> Date: Wed, 6 Jul 2022 14:50:33 +0100 Subject: [PATCH 10/10] Bump minimum node version to 16. (#316) * Bump minimum node version to 16. We already made the mistake of using features from 16 without realising Previously: https://github.com/matrix-org/mjolnir/pull/192 * Make sure CI uses specific version of node and we also lint. https://github.com/matrix-org/pipelines/blob/master/mjolnir/pipeline.yml#L13 --- .github/workflows/mjolnir.yml | 29 ++++++++++++++++++++++++++++- Dockerfile | 2 +- docs/setup_selfbuild.md | 4 ++-- package.json | 2 +- 4 files changed, 32 insertions(+), 5 deletions(-) diff --git a/.github/workflows/mjolnir.yml b/.github/workflows/mjolnir.yml index ce31095..e7d0066 100644 --- a/.github/workflows/mjolnir.yml +++ b/.github/workflows/mjolnir.yml @@ -11,11 +11,38 @@ env: jobs: build: + name: Build & Lint + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + + - name: Specifically use node 16 like in the readme. + uses: actions/setup-node@v3 + with: + node-version: '16' + - run: yarn install + - run: yarn build + - run: yarn lint + unit: + name: Unit tests + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - name: Specifically use node 16 like in the readme. + uses: actions/setup-node@v3 + with: + node-version: '16' + - run: yarn install + - run: yarn test + integration: name: Integration tests runs-on: ubuntu-latest timeout-minutes: 60 steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 + - uses: actions/setup-node@v3 + with: + node-version: '16' - name: install mx-tester run: cargo install mx-tester - name: Setup image diff --git a/Dockerfile b/Dockerfile index 08781f0..02b650d 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM node:14-alpine +FROM node:16-alpine COPY . /tmp/src RUN cd /tmp/src \ && yarn install \ diff --git a/docs/setup_selfbuild.md b/docs/setup_selfbuild.md index 34cbb94..908078a 100644 --- a/docs/setup_selfbuild.md +++ b/docs/setup_selfbuild.md @@ -1,4 +1,4 @@ -To build mjolnir, you have to have installed `yarn` 1.x and Node 14. +To build mjolnir, you have to have installed `yarn` 1.x and Node 16. ```bash git clone https://github.com/matrix-org/mjolnir.git @@ -12,4 +12,4 @@ cp config/default.yaml config/development.yaml nano config/development.yaml node lib/index.js -``` \ No newline at end of file +``` diff --git a/package.json b/package.json index 6594532..6d4932a 100644 --- a/package.json +++ b/package.json @@ -48,6 +48,6 @@ "shell-quote": "^1.7.3" }, "engines": { - "node": ">=14.0.0" + "node": ">=16.0.0" } }