From 6b14c50f09acf1238b01216f0de2047e31bd610a Mon Sep 17 00:00:00 2001 From: Iris System Date: Wed, 28 Jun 2023 14:38:50 +1200 Subject: [PATCH] fix(bot): only allow proxying in known-supported channel types This is so that new channel types added by Discord (that may or may not support the features we need for proxying to work) don't throw piles of error codes at users when they try to proxy. --- Myriad/Types/Channel.cs | 5 ++- PluralKit.Bot/Commands/Checks.cs | 4 +-- PluralKit.Bot/Proxy/ProxyService.cs | 36 +++++++++++++++---- PluralKit.Bot/Services/WebhookCacheService.cs | 9 ++++- 4 files changed, 44 insertions(+), 10 deletions(-) diff --git a/Myriad/Types/Channel.cs b/Myriad/Types/Channel.cs index 418c3eb2..601ff9f2 100644 --- a/Myriad/Types/Channel.cs +++ b/Myriad/Types/Channel.cs @@ -14,7 +14,10 @@ public record Channel GuildNewsThread = 10, GuildPublicThread = 11, GuildPrivateThread = 12, - GuildStageVoice = 13 + GuildStageVoice = 13, + GuildDirectory = 14, + GuildForum = 15, + GuildMedia = 16, } public ulong Id { get; init; } diff --git a/PluralKit.Bot/Commands/Checks.cs b/PluralKit.Bot/Commands/Checks.cs index 43f2c8a0..a7d61660 100644 --- a/PluralKit.Bot/Commands/Checks.cs +++ b/PluralKit.Bot/Commands/Checks.cs @@ -248,10 +248,10 @@ public class Checks // Run everything through the checks, catch the ProxyCheckFailedException, and reply with the error message. try { - _proxy.ShouldProxy(channel, msg, context); + _proxy.ShouldProxy(channel, rootChannel, msg, context); _matcher.TryMatch(context, autoproxySettings, members, out var match, msg.Content, msg.Attachments.Length > 0, true, ctx.Config.CaseSensitiveProxyTags); - var canProxy = await _proxy.CanProxy(channel, msg, context); + var canProxy = await _proxy.CanProxy(channel, rootChannel, msg, context); if (canProxy != null) { await ctx.Reply(canProxy); diff --git a/PluralKit.Bot/Proxy/ProxyService.cs b/PluralKit.Bot/Proxy/ProxyService.cs index de643ea1..e47881dd 100644 --- a/PluralKit.Bot/Proxy/ProxyService.cs +++ b/PluralKit.Bot/Proxy/ProxyService.cs @@ -57,7 +57,9 @@ public class ProxyService public async Task HandleIncomingMessage(MessageCreateEvent message, MessageContext ctx, Guild guild, Channel channel, bool allowAutoproxy, PermissionSet botPermissions) { - if (!ShouldProxy(channel, message, ctx)) + var rootChannel = await _cache.GetRootChannel(message.ChannelId); + + if (!ShouldProxy(channel, rootChannel, message, ctx)) return false; var autoproxySettings = await _repo.GetAutoproxySettings(ctx.SystemId.Value, guild.Id, null); @@ -72,8 +74,6 @@ public class ProxyService return false; } - var rootChannel = await _cache.GetRootChannel(message.ChannelId); - List members; // Fetch members and try to match to a specific member using (_metrics.Measure.Timer.Time(BotMetrics.ProxyMembersQueryTime)) @@ -82,7 +82,7 @@ public class ProxyService if (!_matcher.TryMatch(ctx, autoproxySettings, members, out var match, message.Content, message.Attachments.Length > 0, allowAutoproxy, ctx.CaseSensitiveProxyTags)) return false; - var canProxy = await CanProxy(channel, message, ctx); + var canProxy = await CanProxy(channel, rootChannel, message, ctx); if (canProxy != null) { if (ctx.ProxyErrorMessageEnabled) @@ -109,8 +109,32 @@ public class ProxyService return true; } - public async Task CanProxy(Channel channel, Message msg, MessageContext ctx) +#pragma warning disable CA1822 // Mark members as static + internal bool CanProxyInChannel(Channel ch, bool isRootChannel = false) +#pragma warning restore CA1822 // Mark members as static { + // this is explicitly selecting known channel types so that when Discord add new + // ones, users don't get flooded with error codes if that new channel type doesn't + // support a feature we need for proxying + return ch.Type switch + { + Channel.ChannelType.GuildText => true, + Channel.ChannelType.GuildPublicThread => true, + Channel.ChannelType.GuildPrivateThread => true, + Channel.ChannelType.GuildNews => true, + Channel.ChannelType.GuildNewsThread => true, + Channel.ChannelType.GuildVoice => true, + Channel.ChannelType.GuildStageVoice => true, + Channel.ChannelType.GuildForum => isRootChannel, + _ => false, + }; + } + + public async Task CanProxy(Channel channel, Channel rootChannel, Message msg, MessageContext ctx) + { + if (!(CanProxyInChannel(channel) && CanProxyInChannel(rootChannel, true))) + return $"PluralKit cannot proxy messages in this type of channel."; + // Check if the message does not go over any Discord Nitro limits if (msg.Content != null && msg.Content.Length > 2000) { @@ -132,7 +156,7 @@ public class ProxyService return null; } - public bool ShouldProxy(Channel channel, Message msg, MessageContext ctx) + public bool ShouldProxy(Channel channel, Channel rootChannel, Message msg, MessageContext ctx) { // Make sure author has a system if (ctx.SystemId == null) diff --git a/PluralKit.Bot/Services/WebhookCacheService.cs b/PluralKit.Bot/Services/WebhookCacheService.cs index 6eb5a236..8900d930 100644 --- a/PluralKit.Bot/Services/WebhookCacheService.cs +++ b/PluralKit.Bot/Services/WebhookCacheService.cs @@ -105,7 +105,14 @@ public class WebhookCacheService { try { - return await _rest.GetChannelWebhooks(channelId); + var webhooks = await _rest.GetChannelWebhooks(channelId); + if (webhooks != null) + return webhooks; + + // Getting a 404 / null response from the above generally means the channel type does + // not support webhooks - this is detected elsewhere for proxying purposes, let's just + // return an empty array here + return new Webhook[0]; } catch (HttpRequestException e) {