From 16b49505064f9e38739f5a33d8621cf4fe97461a Mon Sep 17 00:00:00 2001 From: ixrec Date: Wed, 15 Nov 2023 13:02:21 +0000 Subject: [PATCH 01/11] delete .getmessages command --- Izzy-Moonbot/Modules/SpamModule.cs | 46 ------------- Izzy-MoonbotTests/Tests/SpamModuleTests.cs | 75 ---------------------- 2 files changed, 121 deletions(-) diff --git a/Izzy-Moonbot/Modules/SpamModule.cs b/Izzy-Moonbot/Modules/SpamModule.cs index 0be4651a..b1385cfe 100644 --- a/Izzy-Moonbot/Modules/SpamModule.cs +++ b/Izzy-Moonbot/Modules/SpamModule.cs @@ -58,50 +58,4 @@ public async Task TestableGetPressureAsync( await context.Channel.SendMessageAsync($"Current Pressure for {user.DisplayName} ({user.Username}/{user.Id}): {pressure}"); } } - - [Command("getmessages")] - [Summary("Get a user's previous messages (the messages which would have been deleted if the user spammed).")] - [ModCommand(Group = "Permissions")] - [DevCommand(Group = "Permissions")] - [Parameter("user", ParameterType.UserResolvable, "The user to get the messages of, or yourself if no user is provided.", true)] - public async Task GetPreviousMessagesAsync( - [Remainder] string userName = "") - { - await TestableGetPreviousMessagesAsync( - new SocketCommandContextAdapter(Context), - userName - ); - } - - public async Task TestableGetPreviousMessagesAsync( - IIzzyContext context, - string userName = "") - { - // If no target is specified, target self. - if (userName == "") userName = $"<@{context.User.Id}>"; - - var (userId, userError) = await ParseHelper.TryParseUserResolvable(userName, context.Guild!); - if (userId == null) - { - await context.Channel.SendMessageAsync($"Failed to get user id from \"{userName}\": {userError}"); - return; - } - - var user = context.Guild?.GetUser((ulong)userId); - if (user == null) - { - await context.Channel.SendMessageAsync($"Couldn't find <@{userId}> in this server", allowedMentions: AllowedMentions.None); - } - else - { - var previousMessages = _spamService.GetPreviousMessages(user.Id); - - var messageList = previousMessages.Select(item => - $"https://discord.com/channels/{item.GuildId}/{item.ChannelId}/{item.Id} at ()" - ); - - await context.Channel.SendMessageAsync( - $"I consider the following messages from {user.DisplayName} ({user.Username}/{user.Id}) to be recent: \n{string.Join('\n', messageList)}\n*Note that these messages may not actually be recent as their age is only checked when the user sends more messages.*"); - } - } } diff --git a/Izzy-MoonbotTests/Tests/SpamModuleTests.cs b/Izzy-MoonbotTests/Tests/SpamModuleTests.cs index 5a35a56d..dd4b8c8a 100644 --- a/Izzy-MoonbotTests/Tests/SpamModuleTests.cs +++ b/Izzy-MoonbotTests/Tests/SpamModuleTests.cs @@ -76,81 +76,6 @@ public async Task GetPressure_Tests() Assert.AreEqual($"Current Pressure for Sunny (Sunny/2): {finalPressure}", generalChannel.Messages.Last().Content); } - [TestMethod()] - public async Task GetMessages_Tests() - { - var (cfg, _, (_, sunny), _, (generalChannel, modChat, _), guild, client) = TestUtils.DefaultStubs(); - DiscordHelper.DefaultGuildId = guild.Id; - DiscordHelper.DevUserIds = new List(); - DiscordHelper.PleaseAwaitEvents = true; - DateTimeHelper.FakeUtcNow = TestUtils.FiMEpoch; - - // SpamService assumes that every MessageReceived event it receives is for - // a user who is already in the users Dictionary and has a timestamp - var users = new Dictionary(); - users[sunny.Id] = new User(); - users[sunny.Id].Timestamp = DateTimeHelper.UtcNow; - - var mod = new ModService(cfg, users); - var modLog = new ModLoggingService(cfg); - var logger = new LoggingService(new TestLogger()); - var ss = new SpamService(logger, mod, modLog, cfg, users); - ss.RegisterEvents(client); - var sm = new SpamModule(ss); - - // start out with no previous messages besides .getmessages itself - var context = await client.AddMessageAsync(guild.Id, generalChannel.Id, sunny.Id, ".getmessages"); - await sm.TestableGetPreviousMessagesAsync(context, ""); - - Assert.AreEqual($"I consider the following messages from Sunny (Sunny/2) to be recent: " + - $"\nhttps://discord.com/channels/{guild.Id}/{generalChannel.Id}/0 at ()" + - $"\n*Note that these messages may not actually be recent as their age is only checked when the user sends more messages.*", - generalChannel.Messages.Last().Content); - - // say a few normal messages without any time passing - var message1 = "hi everypony"; - var message2 = "my name is Sunny"; - - await client.AddMessageAsync(guild.Id, generalChannel.Id, sunny.Id, message1); - await client.AddMessageAsync(guild.Id, generalChannel.Id, sunny.Id, message2); - context = await client.AddMessageAsync(guild.Id, generalChannel.Id, sunny.Id, ".getmessages"); - await sm.TestableGetPreviousMessagesAsync(context, ""); - - Assert.AreEqual($"I consider the following messages from Sunny (Sunny/2) to be recent: " + - $"\nhttps://discord.com/channels/{guild.Id}/{generalChannel.Id}/0 at ()" + - $"\nhttps://discord.com/channels/{guild.Id}/{generalChannel.Id}/1 at ()" + - $"\nhttps://discord.com/channels/{guild.Id}/{generalChannel.Id}/2 at ()" + - $"\nhttps://discord.com/channels/{guild.Id}/{generalChannel.Id}/3 at ()" + - $"\n*Note that these messages may not actually be recent as their age is only checked when the user sends more messages.*", - generalChannel.Messages.Last().Content); - - // simulate 10 seconds, which should still count as "recent" - DateTimeHelper.FakeUtcNow = DateTimeHelper.FakeUtcNow?.AddSeconds(10); - - context = await client.AddMessageAsync(guild.Id, generalChannel.Id, sunny.Id, ".getmessages"); - await sm.TestableGetPreviousMessagesAsync(context, ""); - - Assert.AreEqual($"I consider the following messages from Sunny (Sunny/2) to be recent: " + - $"\nhttps://discord.com/channels/{guild.Id}/{generalChannel.Id}/0 at ()" + - $"\nhttps://discord.com/channels/{guild.Id}/{generalChannel.Id}/1 at ()" + - $"\nhttps://discord.com/channels/{guild.Id}/{generalChannel.Id}/2 at ()" + - $"\nhttps://discord.com/channels/{guild.Id}/{generalChannel.Id}/3 at ()" + - $"\nhttps://discord.com/channels/{guild.Id}/{generalChannel.Id}/4 at ()" + // slightly different timestamp - $"\n*Note that these messages may not actually be recent as their age is only checked when the user sends more messages.*", - generalChannel.Messages.Last().Content); - - // simulate enough time that none of the above messages should remain "recent" - DateTimeHelper.FakeUtcNow = DateTimeHelper.FakeUtcNow?.AddMinutes(10); - - context = await client.AddMessageAsync(guild.Id, generalChannel.Id, sunny.Id, ".getmessages"); - await sm.TestableGetPreviousMessagesAsync(context, ""); - - Assert.AreEqual($"I consider the following messages from Sunny (Sunny/2) to be recent: " + - $"\nhttps://discord.com/channels/{guild.Id}/{generalChannel.Id}/5 at ()" + - $"\n*Note that these messages may not actually be recent as their age is only checked when the user sends more messages.*", - generalChannel.Messages.Last().Content); - } - [TestMethod()] public async Task GetPressure_OnRegularUser_InPublicOrPrivateChannels_Tests() { From a1ea5448364a110a751515f08d5e02fb88466a35 Mon Sep 17 00:00:00 2001 From: ixrec Date: Wed, 15 Nov 2023 13:13:36 +0000 Subject: [PATCH 02/11] move RecentMessage caching to SpamService --- .../EventListeners/MessageListener.cs | 25 ------------- Izzy-Moonbot/Service/SpamService.cs | 37 +++++++++++++++---- Izzy-MoonbotTests/Tests/SpamModuleTests.cs | 6 ++- Izzy-MoonbotTests/Tests/SpamServiceTests.cs | 3 +- 4 files changed, 36 insertions(+), 35 deletions(-) diff --git a/Izzy-Moonbot/EventListeners/MessageListener.cs b/Izzy-Moonbot/EventListeners/MessageListener.cs index 87f73b81..c71edd07 100644 --- a/Izzy-Moonbot/EventListeners/MessageListener.cs +++ b/Izzy-Moonbot/EventListeners/MessageListener.cs @@ -37,31 +37,6 @@ private async Task ProcessMessageReceived( IIzzyClient client) { var author = message.Author; - - // RecentMessages updating - - if ((message.Channel.Id != _config.ModChannel) && DiscordHelper.IsInGuild(message)) - { - if (!_state.RecentMessages.ContainsKey(author.Id)) - _state.RecentMessages[author.Id] = new(); - var recentMessages = _state.RecentMessages[author.Id]; - recentMessages.Add((message.GetJumpUrl(), message.Timestamp, message.Content)); - - if (recentMessages.Count > 5) - { - var secondsUntilIrrelevant = _config.SpamPressureDecay * (_config.SpamMaxPressure / _config.SpamBasePressure); - while ( - (DateTimeOffset.UtcNow - recentMessages[0].Item2).TotalSeconds > secondsUntilIrrelevant && - recentMessages.Count > 5 - ) - { - recentMessages.RemoveAt(0); - } - } - } - - // Witty processing - if (author.Id == client.CurrentUser.Id) return; // Don't process self. if (author.IsBot) return; // Don't listen to bots diff --git a/Izzy-Moonbot/Service/SpamService.cs b/Izzy-Moonbot/Service/SpamService.cs index b71d6283..b5b810cb 100644 --- a/Izzy-Moonbot/Service/SpamService.cs +++ b/Izzy-Moonbot/Service/SpamService.cs @@ -31,16 +31,13 @@ namespace Izzy_Moonbot.Service; */ public class SpamService { - // Required services private readonly LoggingService _logger; private readonly ModService _mod; private readonly ModLoggingService _modLogger; - - // Configuation private readonly Config _config; private readonly Dictionary _users; + private readonly TransientState _state; - // Utility parameters private readonly Regex _mention = new("<@&?[0-9]+>"); private readonly Regex _url = new("https?://(.+\\.)*(.+)\\.([A-z]+)(/?.+)*", RegexOptions.IgnoreCase); private readonly Regex _noUnfurlUrl = new("<{1}https?://(.+\\.)*(.+)\\.([A-z]+)(/?.+)*>{1}", RegexOptions.IgnoreCase); @@ -50,17 +47,16 @@ public class SpamService */ public static readonly string _testString = "=+i7B3s+#(-{×jn6Ga3F~lA:IZZY_PRESSURE_TEST:H4fgd3!#!"; - // Pull services from the service system - public SpamService(LoggingService logger, ModService mod, ModLoggingService modLogger, Config config, Dictionary users) + public SpamService(LoggingService logger, ModService mod, ModLoggingService modLogger, Config config, Dictionary users, TransientState state) { _logger = logger; _mod = mod; _modLogger = modLogger; _config = config; _users = users; + _state = state; } - // Register required events public void RegisterEvents(IIzzyClient client) { // Register MessageReceived event @@ -424,6 +420,9 @@ private string PonyReadableBreakdown(List<(double, string)> pressureBreakdown) private async Task MessageReceiveEvent(IIzzyMessage messageParam, IIzzyClient client) { + // the RecentMessages cache needs updating even if we aren't doing spam detection + await UpdateRecentMessages(messageParam, client); + if (!_config.SpamEnabled) return; // anti-spam is off if (messageParam.Author.IsBot) return; // Don't listen to bots if (!DiscordHelper.IsInGuild(messageParam)) return; // Not in guild (in dm/group) @@ -441,4 +440,28 @@ private async Task MessageReceiveEvent(IIzzyMessage messageParam, IIzzyClient cl await ProcessPressure(guildUser.Id, context.Message, guildUser, context); } + + private async Task UpdateRecentMessages(IIzzyMessage message, IIzzyClient client) + { + var author = message.Author; + if ((message.Channel.Id != _config.ModChannel) && DiscordHelper.IsInGuild(message)) + { + if (!_state.RecentMessages.ContainsKey(author.Id)) + _state.RecentMessages[author.Id] = new(); + var recentMessages = _state.RecentMessages[author.Id]; + recentMessages.Add((message.GetJumpUrl(), message.Timestamp, message.Content)); + + if (recentMessages.Count > 5) + { + var secondsUntilIrrelevant = _config.SpamPressureDecay * (_config.SpamMaxPressure / _config.SpamBasePressure); + while ( + (DateTimeOffset.UtcNow - recentMessages[0].Item2).TotalSeconds > secondsUntilIrrelevant && + recentMessages.Count > 5 + ) + { + recentMessages.RemoveAt(0); + } + } + } + } } diff --git a/Izzy-MoonbotTests/Tests/SpamModuleTests.cs b/Izzy-MoonbotTests/Tests/SpamModuleTests.cs index dd4b8c8a..78169146 100644 --- a/Izzy-MoonbotTests/Tests/SpamModuleTests.cs +++ b/Izzy-MoonbotTests/Tests/SpamModuleTests.cs @@ -28,7 +28,8 @@ public async Task GetPressure_Tests() var mod = new ModService(cfg, users); var modLog = new ModLoggingService(cfg); var logger = new LoggingService(new TestLogger()); - var ss = new SpamService(logger, mod, modLog, cfg, users); + var state = new TransientState(); + var ss = new SpamService(logger, mod, modLog, cfg, users, state); ss.RegisterEvents(client); var sm = new SpamModule(ss); @@ -98,7 +99,8 @@ public async Task GetPressure_OnRegularUser_InPublicOrPrivateChannels_Tests() var mod = new ModService(cfg, users); var modLog = new ModLoggingService(cfg); var logger = new LoggingService(new TestLogger()); - var ss = new SpamService(logger, mod, modLog, cfg, users); + var state = new TransientState(); + var ss = new SpamService(logger, mod, modLog, cfg, users, state); ss.RegisterEvents(client); var sm = new SpamModule(ss); diff --git a/Izzy-MoonbotTests/Tests/SpamServiceTests.cs b/Izzy-MoonbotTests/Tests/SpamServiceTests.cs index ea309b46..f40c0597 100644 --- a/Izzy-MoonbotTests/Tests/SpamServiceTests.cs +++ b/Izzy-MoonbotTests/Tests/SpamServiceTests.cs @@ -29,7 +29,8 @@ public void SpamSetup(Config cfg, IIzzyUser spammer, StubChannel modChat, StubGu var mod = new ModService(cfg, users); var modLog = new ModLoggingService(cfg); var logger = new LoggingService(new TestLogger()); - var ss = new SpamService(logger, mod, modLog, cfg, users); + var state = new TransientState(); + var ss = new SpamService(logger, mod, modLog, cfg, users, state); ss.RegisterEvents(client); } From 354ea11377b2246b1970a3fdfc79c6591a43c926 Mon Sep 17 00:00:00 2001 From: ixrec Date: Wed, 15 Nov 2023 15:13:37 +0000 Subject: [PATCH 03/11] add three tests for different spam race conditions, one of which fails today --- Izzy-MoonbotTests/Tests/SpamServiceTests.cs | 114 +++++++++++++++++++- 1 file changed, 113 insertions(+), 1 deletion(-) diff --git a/Izzy-MoonbotTests/Tests/SpamServiceTests.cs b/Izzy-MoonbotTests/Tests/SpamServiceTests.cs index f40c0597..ef7aedda 100644 --- a/Izzy-MoonbotTests/Tests/SpamServiceTests.cs +++ b/Izzy-MoonbotTests/Tests/SpamServiceTests.cs @@ -12,7 +12,7 @@ namespace Izzy_Moonbot_Tests.Services; [TestClass()] public class SpamServiceTests { - public void SpamSetup(Config cfg, IIzzyUser spammer, StubChannel modChat, StubGuild guild, StubClient client) + public Dictionary SpamSetup(Config cfg, IIzzyUser spammer, StubChannel modChat, StubGuild guild, StubClient client) { DiscordHelper.DefaultGuildId = guild.Id; DiscordHelper.DevUserIds = new List(); @@ -33,6 +33,8 @@ public void SpamSetup(Config cfg, IIzzyUser spammer, StubChannel modChat, StubGu var ss = new SpamService(logger, mod, modLog, cfg, users, state); ss.RegisterEvents(client); + + return users; } [TestMethod()] @@ -349,4 +351,114 @@ await client.AddMessageAsync(guild.Id, generalChannel.Id, sunny.Id, $"Length: 2.7 ≈ 432 characters × 0.00625"), }); } + + [TestMethod()] + public async Task ContinueSpammingAfterSpamTrip_Tests() + { + var (cfg, _, (_, sunny), _, (generalChannel, modChat, _), guild, client) = TestUtils.DefaultStubs(); + SpamSetup(cfg, sunny, modChat, guild, client); + + Assert.AreEqual(0, generalChannel.Messages.Count); + Assert.AreEqual(0, modChat.Messages.Count); + + // This simulates the scenario where, after the message that actually trips spam, + // the user simply posts another message before Izzy can silence them. + // Since message receiving and user silencing are network requests, this cannot be prevented, only handled. + var t1 = client.AddMessageAsync(guild.Id, generalChannel.Id, sunny.Id, SpamService._testString); + var t2 = client.AddMessageAsync(guild.Id, generalChannel.Id, sunny.Id, SpamService._testString); + var t3 = client.AddMessageAsync(guild.Id, generalChannel.Id, sunny.Id, SpamService._testString); + + await t1; await t2; await t3; + + // The spam messages have been deleted + Assert.AreEqual(0, generalChannel.Messages.Count); + + // Since they were posted "at the same time", only one task should have deleted all 3 with a single mod message + Assert.AreEqual(1, modChat.Messages.Count); + Assert.AreEqual("<@&0> I've silenced <@2> for spamming and deleted 3 of their message(s)", modChat.Messages.Last().Content); + TestUtils.AssertEmbedFieldsAre(modChat.Messages.Last().Embeds[0].Fields, new List<(string, string)> + { + ("Silenced User", "<@2> (`2`)"), + ("Channel", $"<#{generalChannel.Id}>"), + ("Pressure", "This user's last message raised their pressure from 0 to 60, exceeding 60"), + ("Breakdown of last message", "**Test string**"), + }); + } + + [TestMethod()] + public async Task TripSpamThenSpamAgainLater_Tests() + { + var (cfg, _, (_, sunny), _, (generalChannel, modChat, _), guild, client) = TestUtils.DefaultStubs(); + var users = SpamSetup(cfg, sunny, modChat, guild, client); + + Assert.AreEqual(0, generalChannel.Messages.Count); + Assert.AreEqual(0, modChat.Messages.Count); + + await client.AddMessageAsync(guild.Id, generalChannel.Id, sunny.Id, SpamService._testString); + + Assert.AreEqual(0, generalChannel.Messages.Count); + Assert.AreEqual(1, modChat.Messages.Count); + Assert.AreEqual("<@&0> I've silenced <@2> for spamming and deleted 1 of their message(s)", modChat.Messages.Last().Content); + TestUtils.AssertEmbedFieldsAre(modChat.Messages.Last().Embeds[0].Fields, new List<(string, string)> + { + ("Silenced User", "<@2> (`2`)"), + ("Channel", $"<#{generalChannel.Id}>"), + ("Pressure", "This user's last message raised their pressure from 0 to 60, exceeding 60"), + ("Breakdown of last message", "**Test string**"), + }); + + // Pretend we had a moon talk and let them back in + users[sunny.Id].Silenced = false; + + await client.AddMessageAsync(guild.Id, generalChannel.Id, sunny.Id, SpamService._testString); + + Assert.AreEqual(0, generalChannel.Messages.Count); + Assert.AreEqual(2, modChat.Messages.Count); + Assert.AreEqual("<@&0> I've silenced <@2> for spamming and deleted 1 of their message(s)", modChat.Messages.Last().Content); + TestUtils.AssertEmbedFieldsAre(modChat.Messages.Last().Embeds[0].Fields, new List<(string, string)> + { + ("Silenced User", "<@2> (`2`)"), + ("Channel", $"<#{generalChannel.Id}>"), + // since no fake time has passed, the second spam trip says "60 to 120" + ("Pressure", "This user's last message raised their pressure from 60 to 120, exceeding 60"), + ("Breakdown of last message", "**Test string**"), + }); + } + + [TestMethod()] + public async Task MultipleUsersSpamSimultaneously_Tests() + { + var (cfg, _, (_, sunny), _, (generalChannel, modChat, _), guild, client) = TestUtils.DefaultStubs(); + var users = SpamSetup(cfg, sunny, modChat, guild, client); + + var zippId = guild.Users[2].Id; + users[zippId] = new User(); + users[zippId].Timestamp = DateTimeHelper.UtcNow; + + Assert.AreEqual(0, generalChannel.Messages.Count); + Assert.AreEqual(0, modChat.Messages.Count); + + await client.AddMessageAsync(guild.Id, generalChannel.Id, sunny.Id, SpamService._testString); + await client.AddMessageAsync(guild.Id, generalChannel.Id, zippId, SpamService._testString); + + Assert.AreEqual(0, generalChannel.Messages.Count); + Assert.AreEqual(2, modChat.Messages.Count); + Assert.AreEqual("<@&0> I've silenced <@2> for spamming and deleted 1 of their message(s)", modChat.Messages[0].Content); + TestUtils.AssertEmbedFieldsAre(modChat.Messages[0].Embeds[0].Fields, new List<(string, string)> + { + ("Silenced User", "<@2> (`2`)"), + ("Channel", $"<#{generalChannel.Id}>"), + ("Pressure", "This user's last message raised their pressure from 0 to 60, exceeding 60"), + ("Breakdown of last message", "**Test string**"), + }); + Assert.AreEqual("<@&0> I've silenced <@3> for spamming and deleted 1 of their message(s)", modChat.Messages[1].Content); + TestUtils.AssertEmbedFieldsAre(modChat.Messages[1].Embeds[0].Fields, new List<(string, string)> + { + ("Silenced User", "<@3> (`3`)"), + ("Channel", $"<#{generalChannel.Id}>"), + ("Pressure", "This user's last message raised their pressure from 0 to 60, exceeding 60"), + ("Breakdown of last message", "**Test string**"), + }); + } + } From 98d5195cf3ec2f21ecabb645bd5341dbe219f0ed Mon Sep 17 00:00:00 2001 From: ixrec Date: Wed, 15 Nov 2023 22:59:00 +0000 Subject: [PATCH 04/11] delete dead method --- Izzy-Moonbot/Service/SpamService.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/Izzy-Moonbot/Service/SpamService.cs b/Izzy-Moonbot/Service/SpamService.cs index b5b810cb..b7fcbe0c 100644 --- a/Izzy-Moonbot/Service/SpamService.cs +++ b/Izzy-Moonbot/Service/SpamService.cs @@ -70,8 +70,6 @@ public void RegisterEvents(IIzzyClient client) /// The pressure of the user. public double GetPressure(ulong id) => _users[id].Pressure; // Just return the user's pressure - public List GetPreviousMessages(ulong id) => _users[id].PreviousMessages; // Just return the user's previous messages - private async Task GetAndDecayPressure(ulong id) { // Get current time, calculate pressure loss per second and time difference between now and last pressure task then calculate full pressure loss From 6e62d594d0165a7e9484d3c011af318e6fd02433 Mon Sep 17 00:00:00 2001 From: ixrec Date: Wed, 15 Nov 2023 23:00:03 +0000 Subject: [PATCH 05/11] replace PreviousMessages with RecentMessages in deletion code, including bugfixes --- Izzy-Moonbot/Service/SpamService.cs | 31 +++++++++++++++---------- Izzy-MoonbotTests/Tests/TestAdapters.cs | 5 ++-- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/Izzy-Moonbot/Service/SpamService.cs b/Izzy-Moonbot/Service/SpamService.cs index b7fcbe0c..3f8074da 100644 --- a/Izzy-Moonbot/Service/SpamService.cs +++ b/Izzy-Moonbot/Service/SpamService.cs @@ -309,23 +309,30 @@ private async Task ProcessTrip(ulong id, double oldPressureAfterDecay, double pr var bulkDeletionLog = new List<(DateTimeOffset, string)>(); var bulkDeletionCount = 0; var alreadyDeletedMessages = 0; - + + var secondsUntilIrrelevant = _config.SpamPressureDecay * (_config.SpamMaxPressure / _config.SpamBasePressure); + // Remove all messages considered part of spam. - foreach (var previousMessageItem in _users[id].PreviousMessages) + foreach (var recentMessageItem in _state.RecentMessages[id]) { + if ((DateTimeHelper.UtcNow - recentMessageItem.Item2).TotalSeconds > secondsUntilIrrelevant) continue; + + var ids = recentMessageItem.Item1.Split('/').TakeLast(2).ToArray(); + var channelId = ulong.Parse(ids[0]); + var messageId = ulong.Parse(ids[1]); try { - var channel = context.Guild.GetTextChannel(previousMessageItem.ChannelId); + var channel = context.Guild.GetTextChannel(channelId); if (channel == null) - throw new InvalidOperationException($"{id}'s PreviousMessages are somehow from a non-existent channel"); + throw new InvalidOperationException($"{id}'s RecentMessages are somehow from a non-existent channel"); - var previousMessage = channel is null ? null : await channel.GetMessageAsync(previousMessageItem.Id); - if (previousMessage is not null) + var recentMessage = channel is null ? null : await channel.GetMessageAsync(messageId); + if (recentMessage is not null) { - if (previousMessage.Content != "") - bulkDeletionLog.Add((previousMessageItem.Timestamp, - $"[{previousMessageItem.Timestamp}] in #{channel?.Name}: {previousMessage.Content}")); - await previousMessage.DeleteAsync(); + if (recentMessage.Content != "") + bulkDeletionLog.Add((recentMessageItem.Item2, + $"[{recentMessageItem.Item2}] in #{channel?.Name}: {recentMessage.Content}")); + await recentMessage.DeleteAsync(); bulkDeletionCount++; } else @@ -343,7 +350,7 @@ private async Task ProcessTrip(ulong id, double oldPressureAfterDecay, double pr { // Something funky is going on here _logger.Log($"Exception occured while trying to delete message, assuming deleted.", level: LogLevel.Warning); - _logger.Log($"Message ID: {previousMessageItem.Id}", level: LogLevel.Warning); + _logger.Log($"Message Link: {recentMessageItem.Item1}", level: LogLevel.Warning); _logger.Log($"Message: {ex.Message}", level: LogLevel.Warning); _logger.Log($"Source: {ex.Source}", level: LogLevel.Warning); _logger.Log($"Method: {ex.TargetSite}", level: LogLevel.Warning); @@ -453,7 +460,7 @@ private async Task UpdateRecentMessages(IIzzyMessage message, IIzzyClient client { var secondsUntilIrrelevant = _config.SpamPressureDecay * (_config.SpamMaxPressure / _config.SpamBasePressure); while ( - (DateTimeOffset.UtcNow - recentMessages[0].Item2).TotalSeconds > secondsUntilIrrelevant && + (DateTimeHelper.UtcNow - recentMessages[0].Item2).TotalSeconds > secondsUntilIrrelevant && recentMessages.Count > 5 ) { diff --git a/Izzy-MoonbotTests/Tests/TestAdapters.cs b/Izzy-MoonbotTests/Tests/TestAdapters.cs index f2d139ff..f3732921 100644 --- a/Izzy-MoonbotTests/Tests/TestAdapters.cs +++ b/Izzy-MoonbotTests/Tests/TestAdapters.cs @@ -170,8 +170,8 @@ public class TestMessage : IIzzyUserMessage public IReadOnlyCollection Attachments => (IReadOnlyCollection)_message.Attachments; public IReadOnlyCollection Embeds => (IReadOnlyCollection)_message.Embeds; public IReadOnlyCollection Stickers => (IReadOnlyCollection)_message.Stickers; - public DateTimeOffset CreatedAt => new DateTimeOffset(2010, 10, 10, 0, 0, 0, TimeSpan.Zero); - public DateTimeOffset Timestamp => new DateTimeOffset(2010, 10, 10, 0, 0, 0, TimeSpan.Zero); + public DateTimeOffset CreatedAt { get => Timestamp; } + public DateTimeOffset Timestamp { get; } private readonly StubMessage _message; private readonly StubChannel _channelBackref; @@ -185,6 +185,7 @@ public TestMessage(StubMessage message, IIzzyUser author, StubChannel channel, S _channelBackref = channel; _guildBackref = guild; _clientBackref = client; + Timestamp = DateTimeHelper.UtcNow; } public async Task ReplyAsync(string message) From 16c725ab14f33026af58fd691aed9eebe0a687a3 Mon Sep 17 00:00:00 2001 From: ixrec Date: Wed, 15 Nov 2023 23:42:18 +0000 Subject: [PATCH 06/11] create a proper RecentMessage type, including channel/messageId instead of jumpUrl the EmbedsCount we need to compute spam --- Izzy-Moonbot/Modules/ModMiscModule.cs | 2 +- Izzy-Moonbot/Service/SpamService.cs | 22 +++++++++++----------- Izzy-Moonbot/Settings/TransientState.cs | 24 ++++++++++++++++++++++-- 3 files changed, 34 insertions(+), 14 deletions(-) diff --git a/Izzy-Moonbot/Modules/ModMiscModule.cs b/Izzy-Moonbot/Modules/ModMiscModule.cs index 63efbe84..40a50d29 100644 --- a/Izzy-Moonbot/Modules/ModMiscModule.cs +++ b/Izzy-Moonbot/Modules/ModMiscModule.cs @@ -788,7 +788,7 @@ public async Task RecentMessagesCommandAsync([Remainder] string user = "") await ReplyAsync( $"These are all the recent messages (without edits or deletions) I have cached from <@{userId}>:\n" + "\n" + - String.Join("\n", recentMessages.Select(rm => $"[{rm.Item1} ] {rm.Item3}")), + String.Join("\n", recentMessages.Select(rm => $"[{rm.GetJumpUrl()} ] {rm.Content}")), allowedMentions: AllowedMentions.None ); } diff --git a/Izzy-Moonbot/Service/SpamService.cs b/Izzy-Moonbot/Service/SpamService.cs index 3f8074da..75cfe4e9 100644 --- a/Izzy-Moonbot/Service/SpamService.cs +++ b/Izzy-Moonbot/Service/SpamService.cs @@ -315,23 +315,20 @@ private async Task ProcessTrip(ulong id, double oldPressureAfterDecay, double pr // Remove all messages considered part of spam. foreach (var recentMessageItem in _state.RecentMessages[id]) { - if ((DateTimeHelper.UtcNow - recentMessageItem.Item2).TotalSeconds > secondsUntilIrrelevant) continue; + if ((DateTimeHelper.UtcNow - recentMessageItem.Timestamp).TotalSeconds > secondsUntilIrrelevant) continue; - var ids = recentMessageItem.Item1.Split('/').TakeLast(2).ToArray(); - var channelId = ulong.Parse(ids[0]); - var messageId = ulong.Parse(ids[1]); try { - var channel = context.Guild.GetTextChannel(channelId); + var channel = context.Guild.GetTextChannel(recentMessageItem.ChannelId); if (channel == null) throw new InvalidOperationException($"{id}'s RecentMessages are somehow from a non-existent channel"); - var recentMessage = channel is null ? null : await channel.GetMessageAsync(messageId); + var recentMessage = channel is null ? null : await channel.GetMessageAsync(recentMessageItem.MessageId); if (recentMessage is not null) { if (recentMessage.Content != "") - bulkDeletionLog.Add((recentMessageItem.Item2, - $"[{recentMessageItem.Item2}] in #{channel?.Name}: {recentMessage.Content}")); + bulkDeletionLog.Add((recentMessageItem.Timestamp, + $"[{recentMessageItem.Timestamp}] in #{channel?.Name}: {recentMessage.Content}")); await recentMessage.DeleteAsync(); bulkDeletionCount++; } @@ -350,7 +347,7 @@ private async Task ProcessTrip(ulong id, double oldPressureAfterDecay, double pr { // Something funky is going on here _logger.Log($"Exception occured while trying to delete message, assuming deleted.", level: LogLevel.Warning); - _logger.Log($"Message Link: {recentMessageItem.Item1}", level: LogLevel.Warning); + _logger.Log($"Message Link: {recentMessageItem.GetJumpUrl()}", level: LogLevel.Warning); _logger.Log($"Message: {ex.Message}", level: LogLevel.Warning); _logger.Log($"Source: {ex.Source}", level: LogLevel.Warning); _logger.Log($"Method: {ex.TargetSite}", level: LogLevel.Warning); @@ -451,16 +448,19 @@ private async Task UpdateRecentMessages(IIzzyMessage message, IIzzyClient client var author = message.Author; if ((message.Channel.Id != _config.ModChannel) && DiscordHelper.IsInGuild(message)) { + var embedsCount = message.Attachments.Count + message.Embeds.Count + message.Stickers.Count; + if (!_state.RecentMessages.ContainsKey(author.Id)) _state.RecentMessages[author.Id] = new(); + var recentMessages = _state.RecentMessages[author.Id]; - recentMessages.Add((message.GetJumpUrl(), message.Timestamp, message.Content)); + recentMessages.Add(new TransientState.RecentMessage(message.Id, message.Channel.Id, message.Timestamp, message.Content, embedsCount)); if (recentMessages.Count > 5) { var secondsUntilIrrelevant = _config.SpamPressureDecay * (_config.SpamMaxPressure / _config.SpamBasePressure); while ( - (DateTimeHelper.UtcNow - recentMessages[0].Item2).TotalSeconds > secondsUntilIrrelevant && + (DateTimeHelper.UtcNow - recentMessages[0].Timestamp).TotalSeconds > secondsUntilIrrelevant && recentMessages.Count > 5 ) { diff --git a/Izzy-Moonbot/Settings/TransientState.cs b/Izzy-Moonbot/Settings/TransientState.cs index 584e1dd7..078e13e4 100644 --- a/Izzy-Moonbot/Settings/TransientState.cs +++ b/Izzy-Moonbot/Settings/TransientState.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using Izzy_Moonbot.Helpers; namespace Izzy_Moonbot.Settings; @@ -18,6 +19,25 @@ public class TransientState // RaidService public List RecentJoins = new(); - // (string, DateTimeOffset, string) = (jump URL, timestamp, content) - public Dictionary> RecentMessages = new(); + public class RecentMessage + { + public ulong MessageId; + public ulong ChannelId; + public DateTimeOffset Timestamp; + public string Content; + public int EmbedsCount; + + public RecentMessage(ulong messageId, ulong channelId, DateTimeOffset timestamp, string content, int embedsCount) + { + MessageId = messageId; + ChannelId = channelId; + Timestamp = timestamp; + Content = content; + EmbedsCount = embedsCount; + } + + public string GetJumpUrl() => $"https://discord.com/channels/{DiscordHelper.DefaultGuild()}/{ChannelId}/{MessageId}"; + } + + public Dictionary> RecentMessages = new(); } From 192997872609e537da97d03d3a7d0dd1f2699428 Mon Sep 17 00:00:00 2001 From: ixrec Date: Thu, 16 Nov 2023 00:24:10 +0000 Subject: [PATCH 07/11] reimplement spam pressure calculation using RecentMessages as the source of truth, with no caching of running pressure totals --- Izzy-Moonbot/Modules/DevModule.cs | 4 - Izzy-Moonbot/Service/SpamService.cs | 153 +++++++++--------------- Izzy-Moonbot/Settings/TransientState.cs | 40 +++---- 3 files changed, 79 insertions(+), 118 deletions(-) diff --git a/Izzy-Moonbot/Modules/DevModule.cs b/Izzy-Moonbot/Modules/DevModule.cs index 0f2e08b6..2561d284 100644 --- a/Izzy-Moonbot/Modules/DevModule.cs +++ b/Izzy-Moonbot/Modules/DevModule.cs @@ -122,10 +122,6 @@ [Remainder] [Summary("Test arguments")] var paginationHelper = new PaginationHelper(Context, pages, staticParts); break; - case "pressure-hook": - await Context.Message.ReplyAsync( - $"**Test utility** - Pressure hookin test.\n*Other services or modules can hook into the pressure service to do specific things.*\n*An example of this is getting pressure for a user.*\n*Like, your current pressure is `{_pressureService.GetPressure(Context.User.Id)}`*"); - break; case "dump-users-size": await Context.Message.ReplyAsync($"UserStore size: {_users.Count}"); break; diff --git a/Izzy-Moonbot/Service/SpamService.cs b/Izzy-Moonbot/Service/SpamService.cs index 75cfe4e9..a191b84f 100644 --- a/Izzy-Moonbot/Service/SpamService.cs +++ b/Izzy-Moonbot/Service/SpamService.cs @@ -15,20 +15,6 @@ namespace Izzy_Moonbot.Service; -/* - * This service handles anti-spam routines. - * The anti-spam works like this: - * - Each user has "pressure", this is a number based on several factors including but not limitied to: - * - Message length - * - Message attachments - * - Whether their last message is the same as this one - * - This pressure decays by Config.SpamBasePressure every Config.SpamPressureDecay seconds - * - If a users pressure reaches or exceeds Config.SpamMaxPressure, the bot will automatically silence them and inform the mods of this action - * - Users will stay silenced until either banned, kicked, or unsilenced by the mods. - * - * Other modules/services are capable of reading and adding pressure to users. This can be useful for increasing pressure due to a filter violation. - * (however this is mainly implemented so that SpamModule can output the pressure of a user via command) - */ public class SpamService { private readonly LoggingService _logger; @@ -63,68 +49,38 @@ public void RegisterEvents(IIzzyClient client) client.MessageReceived += async (message) => await DiscordHelper.LeakOrAwaitTask(MessageReceiveEvent(message, client)); } - /// - /// Get the last known pressure of a given user by their Discord id. - /// - /// User ID - /// The pressure of the user. - public double GetPressure(ulong id) => _users[id].Pressure; // Just return the user's pressure - - private async Task GetAndDecayPressure(ulong id) + public double GetPressure(ulong id) { - // Get current time, calculate pressure loss per second and time difference between now and last pressure task then calculate full pressure loss - var now = DateTimeHelper.UtcNow; - var pressureLossPerSecond = _config.SpamBasePressure / _config.SpamPressureDecay; - var pressure = _users[id].Pressure; - var difference = now - _users[id].Timestamp; - var pressureLoss = difference.TotalSeconds * pressureLossPerSecond; - - // Execute pressure loss - pressure -= pressureLoss; - if (pressure <= 0) pressure = 0; // Pressure cannot be negative - - // Save pressure loss - _users[id].Pressure = pressure; - _users[id].Timestamp = now; - - // Remove out of date message cache. - // TODO: Move to it's own method. Not sure how to without saving the users file again... - var messages = _users[id].PreviousMessages.ToArray().ToList(); // .NET gets angry if we modify the iterator while iterating - - foreach (var previousMessageItem in messages) + if (!_state.RecentMessages.ContainsKey(id)) return 0.0; + + var recentMessages = _state.RecentMessages[id]; + RecentMessage? previousRecentMessage = null; + + var pressureDecayPerSecond = _config.SpamBasePressure / _config.SpamPressureDecay; + double totalPressure = 0.0; + + foreach (var rm in recentMessages) { - if ((previousMessageItem.Timestamp.ToUniversalTime().ToUnixTimeMilliseconds() + (_config.SpamMessageDeleteLookback * 1000)) <= - DateTimeHelper.UtcNow.ToUnixTimeMilliseconds()) - { - // Message is out of date, remove it - _users[id].PreviousMessages.Remove(previousMessageItem); - } - } - - await FileHelper.SaveUsersAsync(_users); + calculateMessagePressureWithoutDecay(rm, previousRecentMessage, out var pressureForMessage, out _); - // Return pressure - return pressure; - } + var pressureDecay = 0.0; + if (previousRecentMessage is not null) + pressureDecay = (rm.Timestamp - previousRecentMessage.Timestamp).TotalSeconds * pressureDecayPerSecond; + totalPressure = pressureForMessage + Math.Max(totalPressure - pressureDecay, 0); - public async Task IncreasePressure(ulong id, double pressure) - { - // Increase pressure - _users[id].Pressure += pressure; + previousRecentMessage = rm; + } - // Save user - _users[id].Timestamp = DateTimeHelper.UtcNow; - await FileHelper.SaveUsersAsync(_users); + var finalPressureDecay = (DateTimeHelper.UtcNow - recentMessages.Last().Timestamp).TotalSeconds * pressureDecayPerSecond; + totalPressure -= finalPressureDecay; - // Return new pressure - return _users[id].Pressure; + return totalPressure; } - private async Task ProcessPressure(ulong id, IIzzyUserMessage message, IIzzyGuildUser user, - IIzzyContext context) + private void calculateMessagePressureWithoutDecay(RecentMessage message, RecentMessage? previousMessage, out double pressure, out List<(double, string)> pressureBreakdown) { - var pressure = 0.0; - var pressureBreakdown = new List<(double, string)>{}; + pressure = 0.0; + pressureBreakdown = new List<(double, string)> { }; var lengthPressure = Math.Round(_config.SpamLengthPressure * message.Content.Length, 2); if (lengthPressure > 0) @@ -143,20 +99,20 @@ private async Task ProcessPressure(ulong id, IIzzyUserMessage message, IIzzyGuil // Attachments, embeds, and stickers count as Image pressure // TODO: figure out better names for this - var embedsCount = message.Attachments.Count + message.Embeds.Count + message.Stickers.Count; + var embedsCount = message.EmbedsCount; if (embedsCount >= 1) { var embedPressure = Math.Round(_config.SpamImagePressure * embedsCount, 2); pressure += embedPressure; - pressureBreakdown.Add((embedPressure , $"Embeds: {embedPressure} ≈ {embedsCount} embeds × {_config.SpamImagePressure}")); + pressureBreakdown.Add((embedPressure, $"Embeds: {embedPressure} ≈ {embedsCount} embeds × {_config.SpamImagePressure}")); } // Check if there's at least one url in the message (and there's no embeds) - if (_url.IsMatch(message.Content) && message.Embeds.Count == 0) + if (_url.IsMatch(message.Content) && message.EmbedsCount == 0) { // Because url pressure can occur multiple times, we store the pressure to add here var totalMatches = 0; - + // Go through each "word" because the URL regex is funky foreach (var content in message.Content.Split(" ")) { @@ -167,8 +123,8 @@ private async Task ProcessPressure(ulong id, IIzzyUserMessage message, IIzzyGuil // Check if url is in fact set to not unfurl var matchToRemove = matches.Find(urlMatch => match.Value.Contains(urlMatch.Value)); // If not, just continue - if(matchToRemove == null) continue; - + if (matchToRemove == null) continue; + // If it is, remove the match. matches.Remove(matchToRemove); } @@ -195,7 +151,7 @@ private async Task ProcessPressure(ulong id, IIzzyUserMessage message, IIzzyGuil } // Repeat pressure - if (message.CleanContent.ToLower() == _users[id].PreviousMessage.ToLower() && message.CleanContent != "") + if (previousMessage is not null && message.Content.ToLower() == previousMessage.Content.ToLower() && message.Content != "") { pressure += _config.SpamRepeatPressure; pressureBreakdown.Add((_config.SpamRepeatPressure, $"Repeat of Previous Message: {_config.SpamRepeatPressure}")); @@ -233,29 +189,38 @@ private async Task ProcessPressure(ulong id, IIzzyUserMessage message, IIzzyGuil pressure = _config.SpamMaxPressure; pressureBreakdown = new List<(double, string)> { (_config.SpamMaxPressure, "Test string") }; } + } - _users[id].PreviousMessage = context.Message.CleanContent; - + private async Task ProcessPressure(ulong id, IIzzyUserMessage message, IIzzyGuildUser user, IIzzyContext context) + { if (context.Guild == null) throw new InvalidOperationException("ProcessPressure was somehow called with a non-guild context"); - var messageItem = - new PreviousMessageItem(message.Id, context.Channel.Id, context.Guild.Id, DateTimeHelper.UtcNow); - - _users[id].PreviousMessages.Add(messageItem); - - await FileHelper.SaveUsersAsync(_users); + var recentMessages = _state.RecentMessages[id]; + RecentMessage? previousRecentMessage = null; - var oldPressureBeforeDecay = _users[id].Pressure * 1; // seperate it from the thingy + var pressureDecayPerSecond = _config.SpamBasePressure / _config.SpamPressureDecay; + List<(double, string)> lastPressureBreakdown = new(); + double oldPressureAfterDecay = 0.0; + double totalPressure = 0.0; - var oldPressureAfterDecay = await GetAndDecayPressure(id); + foreach (var rm in recentMessages) + { + calculateMessagePressureWithoutDecay(rm, previousRecentMessage, out var pressureForMessage, out lastPressureBreakdown); - var newPressure = await IncreasePressure(id, pressure); + var pressureDecay = 0.0; + if (previousRecentMessage is not null) + pressureDecay = (rm.Timestamp - previousRecentMessage.Timestamp).TotalSeconds * pressureDecayPerSecond; + oldPressureAfterDecay = Math.Max(totalPressure - pressureDecay, 0); + totalPressure = pressureForMessage + oldPressureAfterDecay; + + previousRecentMessage = rm; + } - // Logging on every single server message proved too spammy, but this is indispensable for testing spam changes, so leaving as a comment for us to uncomment during manual testing. - // _logger.Log($"\nPressure channge: {oldPressureAfterDecay} + {pressure} = {newPressure} out of {_config.SpamMaxPressure}\n{string.Join('\n', pressureBreakdown)}", context, level: LogLevel.Debug); + var finalPressureDecay = (DateTimeHelper.UtcNow - recentMessages.Last().Timestamp).TotalSeconds * pressureDecayPerSecond; + totalPressure -= finalPressureDecay; - if (newPressure >= _config.SpamMaxPressure) + if (totalPressure >= _config.SpamMaxPressure) { _logger.Log("Spam pressure trip, checking whether user should be silenced or not...", context, level: LogLevel.Debug); var roleIds = user.Roles.Select(roles => roles.Id).ToList(); @@ -271,15 +236,15 @@ private async Task ProcessPressure(ulong id, IIzzyUserMessage message, IIzzyGuil .WithColor(3355443) .AddField("User", $"<@{context.User.Id}> (`{context.User.Id}`)", true) .AddField("Channel", $"<#{context.Channel.Id}>", true) - .AddField("Pressure", $"This user's last message raised their pressure from {oldPressureAfterDecay} to {newPressure}, exceeding {_config.SpamMaxPressure}") - .AddField("Breakdown of last message", PonyReadableBreakdown(pressureBreakdown)); + .AddField("Pressure", $"This user's last message raised their pressure from {oldPressureAfterDecay} to {totalPressure}, exceeding {_config.SpamMaxPressure}") + .AddField("Breakdown of last message", PonyReadableBreakdown(lastPressureBreakdown)); await _modLogger.CreateModLog(context.Guild) .SetContent($"Spam detected by <@{user.Id}>") .SetEmbed(embedBuilder.Build()) .SetFileLogContent( - $"{user.DisplayName} (`{user.Username}`/`{user.Id}`) exceeded pressure max ({newPressure}/{_config.SpamMaxPressure}) in #{message.Channel.Name} (`{message.Channel.Id}`).\n" + - $"Pressure breakdown: {PonyReadableBreakdown(pressureBreakdown)}\n" + + $"{user.DisplayName} (`{user.Username}`/`{user.Id}`) exceeded pressure max ({totalPressure}/{_config.SpamMaxPressure}) in #{message.Channel.Name} (`{message.Channel.Id}`).\n" + + $"Pressure breakdown: {PonyReadableBreakdown(lastPressureBreakdown)}\n" + $"Did nothing: User has a role which bypasses punishment or has dev bypass.") .Send(); } @@ -287,7 +252,7 @@ await _modLogger.CreateModLog(context.Guild) { // User is not immune to spam punishments, process trip. _logger.Log("Silence, executing trip method.", context, level: LogLevel.Debug); - await ProcessTrip(id, oldPressureAfterDecay, newPressure, pressureBreakdown, message, user, context); + await ProcessTrip(id, oldPressureAfterDecay, totalPressure, lastPressureBreakdown, message, user, context); } } } @@ -454,7 +419,7 @@ private async Task UpdateRecentMessages(IIzzyMessage message, IIzzyClient client _state.RecentMessages[author.Id] = new(); var recentMessages = _state.RecentMessages[author.Id]; - recentMessages.Add(new TransientState.RecentMessage(message.Id, message.Channel.Id, message.Timestamp, message.Content, embedsCount)); + recentMessages.Add(new RecentMessage(message.Id, message.Channel.Id, message.Timestamp, message.Content, embedsCount)); if (recentMessages.Count > 5) { diff --git a/Izzy-Moonbot/Settings/TransientState.cs b/Izzy-Moonbot/Settings/TransientState.cs index 078e13e4..e2952085 100644 --- a/Izzy-Moonbot/Settings/TransientState.cs +++ b/Izzy-Moonbot/Settings/TransientState.cs @@ -4,6 +4,26 @@ namespace Izzy_Moonbot.Settings; +public class RecentMessage +{ + public ulong MessageId; + public ulong ChannelId; + public DateTimeOffset Timestamp; + public string Content; + public int EmbedsCount; + + public RecentMessage(ulong messageId, ulong channelId, DateTimeOffset timestamp, string content, int embedsCount) + { + MessageId = messageId; + ChannelId = channelId; + Timestamp = timestamp; + Content = content; + EmbedsCount = embedsCount; + } + + public string GetJumpUrl() => $"https://discord.com/channels/{DiscordHelper.DefaultGuild()}/{ChannelId}/{MessageId}"; +} + // Storage for Izzy's transient shared state. // This is used for volatile data that needs to be used by multiple services and modules. public class TransientState @@ -19,25 +39,5 @@ public class TransientState // RaidService public List RecentJoins = new(); - public class RecentMessage - { - public ulong MessageId; - public ulong ChannelId; - public DateTimeOffset Timestamp; - public string Content; - public int EmbedsCount; - - public RecentMessage(ulong messageId, ulong channelId, DateTimeOffset timestamp, string content, int embedsCount) - { - MessageId = messageId; - ChannelId = channelId; - Timestamp = timestamp; - Content = content; - EmbedsCount = embedsCount; - } - - public string GetJumpUrl() => $"https://discord.com/channels/{DiscordHelper.DefaultGuild()}/{ChannelId}/{MessageId}"; - } - public Dictionary> RecentMessages = new(); } From 246fbaac832162397a0285ba973708d3038c2430 Mon Sep 17 00:00:00 2001 From: ixrec Date: Thu, 16 Nov 2023 01:15:27 +0000 Subject: [PATCH 08/11] maintain a list of userIds currently tripping spam so we can lock() access to it, preventing multiple tasks from trying to clean up the same messages --- Izzy-Moonbot/Service/SpamService.cs | 33 +++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/Izzy-Moonbot/Service/SpamService.cs b/Izzy-Moonbot/Service/SpamService.cs index a191b84f..f810c546 100644 --- a/Izzy-Moonbot/Service/SpamService.cs +++ b/Izzy-Moonbot/Service/SpamService.cs @@ -32,7 +32,9 @@ public class SpamService * The test string is programmed to immediately set pressure to Config.SpamMaxPressure. */ public static readonly string _testString = "=+i7B3s+#(-{×jn6Ga3F~lA:IZZY_PRESSURE_TEST:H4fgd3!#!"; - + + private List usersCurrentlyTrippingSpam = new(); + public SpamService(LoggingService logger, ModService mod, ModLoggingService modLogger, Config config, Dictionary users, TransientState state) { _logger = logger; @@ -251,7 +253,21 @@ await _modLogger.CreateModLog(context.Guild) else { // User is not immune to spam punishments, process trip. - _logger.Log("Silence, executing trip method.", context, level: LogLevel.Debug); + _logger.Log($"Message {message.Id} task attempting to acquire lock on usersCurrentlyTrippingSpam.", context); + lock (usersCurrentlyTrippingSpam) + { + if (usersCurrentlyTrippingSpam.Contains(id)) + { + _logger.Log($"User {id} already has a spam trip being processed. Message {message.Id} task returning early.", context); + return; + } + else + { + _logger.Log($"No spam trip in progress for user {id}. Message {message.Id} task proceeding with ProcessTrip() call.", context); + usersCurrentlyTrippingSpam.Add(id); + } + } + await ProcessTrip(id, oldPressureAfterDecay, totalPressure, lastPressureBreakdown, message, user, context); } } @@ -322,6 +338,19 @@ private async Task ProcessTrip(ulong id, double oldPressureAfterDecay, double pr } } + // We're done asking Discord to clean up this user's spam, so before we post mod logs + // mark the user as no longer having an in-progress spam trip. + lock (usersCurrentlyTrippingSpam) + { + if (!usersCurrentlyTrippingSpam.Contains(id)) + _logger.Log($"User {id} is somehow missing from usersCurrentlyTrippingSpam in the ProcessTrip() call for them. This should be impossible.", level: LogLevel.Error); + else + { + _logger.Log($"ProcessTrip() call for message {message.Id} by user {id} is done cleaning up. Removing them from usersCurrentlyTrippingSpam."); + usersCurrentlyTrippingSpam.Remove(id); + } + } + string? bulkLogJumpUrl = null; if (bulkDeletionLog.Count > 0) { From 6e0c1650ee3df8c084dcb8a93917ca55219c65ab Mon Sep 17 00:00:00 2001 From: ixrec Date: Thu, 16 Nov 2023 01:26:11 +0000 Subject: [PATCH 09/11] for additional safety, use Concurrent* collections for RecentMessages --- Izzy-Moonbot/Service/SpamService.cs | 6 +++--- Izzy-Moonbot/Settings/TransientState.cs | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Izzy-Moonbot/Service/SpamService.cs b/Izzy-Moonbot/Service/SpamService.cs index f810c546..2b723ab2 100644 --- a/Izzy-Moonbot/Service/SpamService.cs +++ b/Izzy-Moonbot/Service/SpamService.cs @@ -448,17 +448,17 @@ private async Task UpdateRecentMessages(IIzzyMessage message, IIzzyClient client _state.RecentMessages[author.Id] = new(); var recentMessages = _state.RecentMessages[author.Id]; - recentMessages.Add(new RecentMessage(message.Id, message.Channel.Id, message.Timestamp, message.Content, embedsCount)); + recentMessages.Enqueue(new RecentMessage(message.Id, message.Channel.Id, message.Timestamp, message.Content, embedsCount)); if (recentMessages.Count > 5) { var secondsUntilIrrelevant = _config.SpamPressureDecay * (_config.SpamMaxPressure / _config.SpamBasePressure); while ( - (DateTimeHelper.UtcNow - recentMessages[0].Timestamp).TotalSeconds > secondsUntilIrrelevant && + (DateTimeHelper.UtcNow - recentMessages.First().Timestamp).TotalSeconds > secondsUntilIrrelevant && recentMessages.Count > 5 ) { - recentMessages.RemoveAt(0); + recentMessages.TryDequeue(out _); } } } diff --git a/Izzy-Moonbot/Settings/TransientState.cs b/Izzy-Moonbot/Settings/TransientState.cs index e2952085..9710e31d 100644 --- a/Izzy-Moonbot/Settings/TransientState.cs +++ b/Izzy-Moonbot/Settings/TransientState.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Concurrent; using System.Collections.Generic; using Izzy_Moonbot.Helpers; @@ -39,5 +40,5 @@ public class TransientState // RaidService public List RecentJoins = new(); - public Dictionary> RecentMessages = new(); + public ConcurrentDictionary> RecentMessages = new(); } From 512162a5af90999e7357884128407add8d6a3dc5 Mon Sep 17 00:00:00 2001 From: ixrec Date: Thu, 16 Nov 2023 01:41:11 +0000 Subject: [PATCH 10/11] delete sadly non-deterministic test --- Izzy-MoonbotTests/Tests/SpamServiceTests.cs | 33 --------------------- 1 file changed, 33 deletions(-) diff --git a/Izzy-MoonbotTests/Tests/SpamServiceTests.cs b/Izzy-MoonbotTests/Tests/SpamServiceTests.cs index ef7aedda..de43b06f 100644 --- a/Izzy-MoonbotTests/Tests/SpamServiceTests.cs +++ b/Izzy-MoonbotTests/Tests/SpamServiceTests.cs @@ -352,39 +352,6 @@ await client.AddMessageAsync(guild.Id, generalChannel.Id, sunny.Id, }); } - [TestMethod()] - public async Task ContinueSpammingAfterSpamTrip_Tests() - { - var (cfg, _, (_, sunny), _, (generalChannel, modChat, _), guild, client) = TestUtils.DefaultStubs(); - SpamSetup(cfg, sunny, modChat, guild, client); - - Assert.AreEqual(0, generalChannel.Messages.Count); - Assert.AreEqual(0, modChat.Messages.Count); - - // This simulates the scenario where, after the message that actually trips spam, - // the user simply posts another message before Izzy can silence them. - // Since message receiving and user silencing are network requests, this cannot be prevented, only handled. - var t1 = client.AddMessageAsync(guild.Id, generalChannel.Id, sunny.Id, SpamService._testString); - var t2 = client.AddMessageAsync(guild.Id, generalChannel.Id, sunny.Id, SpamService._testString); - var t3 = client.AddMessageAsync(guild.Id, generalChannel.Id, sunny.Id, SpamService._testString); - - await t1; await t2; await t3; - - // The spam messages have been deleted - Assert.AreEqual(0, generalChannel.Messages.Count); - - // Since they were posted "at the same time", only one task should have deleted all 3 with a single mod message - Assert.AreEqual(1, modChat.Messages.Count); - Assert.AreEqual("<@&0> I've silenced <@2> for spamming and deleted 3 of their message(s)", modChat.Messages.Last().Content); - TestUtils.AssertEmbedFieldsAre(modChat.Messages.Last().Embeds[0].Fields, new List<(string, string)> - { - ("Silenced User", "<@2> (`2`)"), - ("Channel", $"<#{generalChannel.Id}>"), - ("Pressure", "This user's last message raised their pressure from 0 to 60, exceeding 60"), - ("Breakdown of last message", "**Test string**"), - }); - } - [TestMethod()] public async Task TripSpamThenSpamAgainLater_Tests() { From 29b1990c3bb18b191e1fe894c43a87103340ccc7 Mon Sep 17 00:00:00 2001 From: ixrec Date: Thu, 16 Nov 2023 01:44:05 +0000 Subject: [PATCH 11/11] codeql --- Izzy-Moonbot/Service/SpamService.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Izzy-Moonbot/Service/SpamService.cs b/Izzy-Moonbot/Service/SpamService.cs index 2b723ab2..62f7fcdc 100644 --- a/Izzy-Moonbot/Service/SpamService.cs +++ b/Izzy-Moonbot/Service/SpamService.cs @@ -33,7 +33,7 @@ public class SpamService */ public static readonly string _testString = "=+i7B3s+#(-{×jn6Ga3F~lA:IZZY_PRESSURE_TEST:H4fgd3!#!"; - private List usersCurrentlyTrippingSpam = new(); + private readonly List usersCurrentlyTrippingSpam = new(); public SpamService(LoggingService logger, ModService mod, ModLoggingService modLogger, Config config, Dictionary users, TransientState state) { @@ -53,9 +53,8 @@ public void RegisterEvents(IIzzyClient client) public double GetPressure(ulong id) { - if (!_state.RecentMessages.ContainsKey(id)) return 0.0; + if (!_state.RecentMessages.TryGetValue(id, out var recentMessages)) return 0.0; - var recentMessages = _state.RecentMessages[id]; RecentMessage? previousRecentMessage = null; var pressureDecayPerSecond = _config.SpamBasePressure / _config.SpamPressureDecay;