Discord #143
Discord #143
Conversation
WalkthroughВ данном пулл-реквесте внесены изменения, касающиеся интеграции Discord в несколько классов проекта. Основные изменения включают добавление нового класса Changes
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
github-actions
bot
added
Status: Merge Conflict
Changes: C#
Changes: YML
and removed
Status: Merge Conflict
labels
| @@ -22,13 +22,63 @@ public static readonly CVarDef<string> | |||
|
|
|||
| #endregion | |||
|
|
|||
| #region GhostRespawn | |||
There was a problem hiding this comment.
Че за хуйня? Появилась при мердже
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (7)
Content.Server/_White/Discord/DiscordLink.cs (1)
71-72: Рассмотреть ограничениеGatewayIntentsдля повышения безопасностиВ текущей конфигурации Discord-клиента используется
GatewayIntents.All, что запрашивает все доступные разрешения и данные от Discord. Рекомендуется указать только необходимыеGatewayIntents, чтобы повысить безопасность и производительность приложения, избегая обработки лишней информации.Content.Server/Chat/Managers/ChatManager.cs (2)
95-102: Рассмотреть логирование ошибок при неудачном преобразовании Channel IDВ методе
OnOocChannelChanged, при неудачном преобразовании строкиidвulong, метод просто возвращается без уведомления. Рекомендуется добавить логирование или обработку ошибки, чтобы упростить отладку в случае некорректных значенийid.
289-289: Рассмотреть экранирование пользовательского ввода при отправке сообщений в DiscordПри вызове
_discordLink.SendMessage(...)используютсяplayer.Nameиmessage, которые могут содержать специальные символы или разметку. Рекомендуется экранировать эти значения перед отправкой в Discord, чтобы предотвратить ошибки форматирования или потенциальные уязвимости.Content.Server/IoC/ServerContentIoC.cs (1)
65-65: Рекомендуется удалить комментарий WD-EditДля поддержания единообразия кодовой базы рекомендуется удалить комментарий "// WD-Edit", так как другие регистрации сервисов не содержат подобных комментариев.
- IoCManager.Register<DiscordLink>(); // WD-Edit + IoCManager.Register<DiscordLink>();Content.Shared/_White/CVars.cs (3)
56-57: Рекомендации по безопасности для токена DiscordХранение токена в CVar может быть небезопасным. Рекомендуется:
- Использовать переменные окружения или файл конфигурации для хранения токена
- Добавить валидацию токена при загрузке
73-77: Уточнить документацию для AdminAhelpRelayChannelIdВ документации указано, что канал должен быть форумом, но отсутствует информация о последствиях неправильной конфигурации. Рекомендуется:
- Добавить проверку типа канала при инициализации
- Дополнить документацию информацией о том, как правильно настроить форум-канал
88-89: Исправить значение по умолчанию для OocRelayChannelIdТекущее значение по умолчанию - пустая строка (""). Рекомендуется использовать string.Empty для консистентности с другими настройками.
- CVarDef.Create("ooc.relay_channel_id", "", CVar.SERVERONLY); + CVarDef.Create("ooc.relay_channel_id", string.Empty, CVar.SERVERONLY);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
Content.Packaging/ServerPackaging.cs(1 hunks)Content.Server/Administration/Systems/BwoinkSystem.cs(8 hunks)Content.Server/Chat/Managers/ChatManager.cs(5 hunks)Content.Server/Entry/EntryPoint.cs(3 hunks)Content.Server/IoC/ServerContentIoC.cs(1 hunks)Content.Server/_White/Discord/DiscordLink.cs(1 hunks)Content.Shared/Content.Shared.csproj(1 hunks)Content.Shared/_White/CVars.cs(1 hunks)Directory.Packages.props(1 hunks)
🔇 Additional comments (5)
Directory.Packages.props (1)
18-18: Код выглядит корректно
Добавление пакета Discord.Net версии 3.13.0 корректно и соответствует требованиям интеграции с Discord.
Content.Packaging/ServerPackaging.cs (1)
47-47: Добавление Discord.Net в список сборки выглядит корректно!
Добавление библиотеки Discord.Net в список ServerExtraAssemblies необходимо для интеграции с Discord и соответствует целям PR.
Content.Server/Entry/EntryPoint.cs (3)
10-10: Корректное добавление пространства имен для Discord интеграции
Добавление using Content.Server._White.Discord соответствует структуре проекта и необходимо для работы с Discord функционалом.
177-177: Проверьте порядок завершения работы Discord компонента
Завершение работы DiscordLink происходит перед завершением работы ServerApi. Рекомендуется проверить, что все ресурсы Discord корректно освобождаются и нет зависимых компонентов, которые могут обращаться к Discord после его отключения.
✅ Verification successful
Let me check the shutdown sequence and dependencies by examining the EntryPoint.cs file and related components.
Let me check how these components interact with DiscordLink and if there are any potential shutdown-related methods.
На основе анализа кода, я могу сформировать окончательный ответ.
Порядок завершения работы Discord компонента корректен
Анализ показывает, что:
- DiscordLink корректно очищает все ресурсы в методе Shutdown()
- Отключение происходит в правильном порядке:
- Отписка от всех событий
- Выход из Discord (LogoutAsync)
- Остановка клиента (StopAsync)
- Освобождение ресурсов (Dispose)
- Компоненты, использующие DiscordLink (ChatManager и BwoinkSystem), не обращаются к нему после завершения работы
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Поиск использования DiscordLink в других компонентах
rg "DiscordLink" --type cs
Length of output: 1148
Script:
#!/bin/bash
# Check the shutdown sequence in EntryPoint.cs
rg -A 10 "Shutdown\(\)" Content.Server/Entry/EntryPoint.cs
# Check implementations of components using DiscordLink
ast-grep --pattern 'class $_ {
$$$
[Dependency] private readonly DiscordLink $_ = default!;
$$$
}'
Length of output: 678
Script:
#!/bin/bash
# Check ChatManager and BwoinkSystem implementations for Discord usage
cat Content.Server/Chat/Managers/ChatManager.cs Content.Server/Administration/Systems/BwoinkSystem.cs
# Check DiscordLink implementation
cat Content.Server/_White/Discord/DiscordLink.cs
Length of output: 56893
108-108: Проверьте порядок инициализации Discord компонента
Инициализация DiscordLink происходит после ServerInfoManager и перед JoinQueueManager. Убедитесь, что этот порядок корректен и не создает проблем с зависимостями между компонентами.
✅ Verification successful
Let me check the dependencies between these components by looking at their implementations.
Порядок инициализации Discord компонента корректен
Анализ кода показывает, что:
- DiscordLink не имеет прямых зависимостей от ServerInfoManager
- JoinQueueManager не зависит от DiscordLink
- DiscordLink используется в ChatManager и BwoinkSystem, но эти системы инициализируются отдельно
Текущий порядок инициализации (ServerInfoManager → DiscordLink → JoinQueueManager) является безопасным, так как между компонентами нет критических зависимостей, требующих специфического порядка инициализации.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Поиск зависимостей между компонентами
rg -A 5 "IoCManager.Resolve<(ServerInfoManager|JoinQueueManager)" --type cs
Length of output: 793
Script:
#!/bin/bash
# Search for dependencies between DiscordLink, ServerInfoManager and JoinQueueManager
ast-grep --pattern 'class DiscordLink {
$$$
}'
ast-grep --pattern 'class ServerInfoManager {
$$$
}'
ast-grep --pattern 'class JoinQueueManager {
$$$
}'
Length of output: 107
Script:
#!/bin/bash
# Search for dependencies using ripgrep
rg "class (DiscordLink|ServerInfoManager|JoinQueueManager)" -A 10 --type cs
# Also search for any cross-references between these services
rg "(DiscordLink|ServerInfoManager|JoinQueueManager)" --type cs
Length of output: 5979
| _client.LogoutAsync(); | ||
| _client.StopAsync(); | ||
| _client.Dispose(); |
There was a problem hiding this comment.
Исправить: добавить ожидание асинхронных методов для корректного завершения
В методе Shutdown, асинхронные методы _client.LogoutAsync() и _client.StopAsync() вызываются без await, что может привести к незавершённому отключению клиента и потенциальным проблемам при выключении сервера. Рекомендуется добавить await перед этими вызовами и сделать метод Shutdown асинхронным (async), чтобы обеспечить корректное завершение асинхронных операций.
| textChannel.SendMessageAsync(message, isTTS, embed, null, allowedMentions); | ||
| } |
There was a problem hiding this comment.
Исправить: добавить ожидание асинхронного метода SendMessageAsync
В методе SendMessage, вызов textChannel.SendMessageAsync(...) осуществляется без await, что может привести к необработанным исключениям и непредсказуемому поведению приложения. Рекомендуется добавить await перед вызовом и сделать метод SendMessage асинхронным (async), чтобы обеспечить корректное выполнение асинхронной операции.
| var content = "[color=yellow] (d) " + arg.Author.Username + "[/color]: " + arg.Content; | ||
| if (!_showThatTheMessageWasFromDiscord) | ||
| content = "[color=red]" + arg.Author.Username + "[/color]: " + arg.Content; | ||
|
|
||
| var msg = new BwoinkTextMessage(messages.Key, messages.Key, content); |
There was a problem hiding this comment.
Исправить: экранировать пользовательский ввод из Discord перед отправкой игрокам
В методе OnDiscordMessageReceived содержимое arg.Content напрямую вставляется в сообщение, отправляемое игровым клиентам. Это может привести к проблемам безопасности или некорректному отображению сообщений, если пользователь введёт специальные символы. Рекомендуется экранировать arg.Content перед включением в сообщение.
| private string GenerateAHelpMessage(string username, string message, bool admin, bool noReceivers = false, bool playedSound = false) | ||
| { | ||
| var stringbuilder = new StringBuilder(); | ||
|
|
||
| stringbuilder.Append($@"`{_gameTicker.RoundDuration():hh\:mm\:ss}` - "); | ||
| if (admin) | ||
| stringbuilder.Append(":outbox_tray:"); | ||
| else if (noReceivers) | ||
| stringbuilder.Append(":sos:"); | ||
| else | ||
| stringbuilder.Append(":inbox_tray:"); | ||
|
|
||
| if(roundTime != string.Empty && roundState == GameRunLevel.InRound) | ||
| stringbuilder.Append($" **{roundTime}**"); | ||
| if (!playedSound) | ||
| stringbuilder.Append(" **(S)**"); | ||
| stringbuilder.Append(" **(С)**"); | ||
|
|
There was a problem hiding this comment.
Исправить: экранировать сообщение при формировании AHelp сообщения для Discord
В методе GenerateAHelpMessage значение message добавляется в строку без экранирования, что может привести к ошибкам форматирования или потенциальным уязвимостям в Discord. Рекомендуется использовать метод EscapeMarkdown(message) для экранирования специальных символов в сообщении.
Content.Shared/Content.Shared.csproj
Outdated
| @@ -9,6 +9,7 @@ | |||
| <Nullable>enable</Nullable> | |||
| </PropertyGroup> | |||
| <ItemGroup> | |||
| <PackageReference Include="Discord.Net" /> | |||
There was a problem hiding this comment.
Необходимо указать версию пакета Discord.Net
Рекомендуется явно указать версию пакета Discord.Net для обеспечения воспроизводимости сборки и безопасности.
Примените следующие изменения:
- <PackageReference Include="Discord.Net" />
+ <PackageReference Include="Discord.Net" Version="3.13.0" />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <PackageReference Include="Discord.Net" /> | |
| <PackageReference Include="Discord.Net" Version="3.13.0" /> |
Content.Shared/_White/CVars.cs
Outdated
| #region Discord | ||
| /// <summary> | ||
| /// The token used to authenticate with Discord. For the Bot to function set: discord.token, discord.guild_id, and discord.prefix. | ||
| /// If this is empty, the bot will not connect. | ||
| /// </summary> | ||
| public static readonly CVarDef<string> DiscordToken = | ||
| CVarDef.Create("discord.token", string.Empty, CVar.SERVERONLY | CVar.CONFIDENTIAL); | ||
|
|
||
| /// <summary> | ||
| /// The Discord guild ID to use for commands as well as for several other features, like the ahelp relay. | ||
| /// If this is empty, the bot will not connect. | ||
| /// </summary> | ||
| public static readonly CVarDef<string> DiscordGuildId = | ||
| CVarDef.Create("discord.guild_id", string.Empty, CVar.SERVERONLY); | ||
|
|
||
| /// <summary> | ||
| /// Prefix used for commands for the Discord bot. | ||
| /// If this is empty, the bot will not connect. | ||
| /// </summary> | ||
| public static readonly CVarDef<string> DiscordPrefix = | ||
| CVarDef.Create("discord.prefix", "!", CVar.SERVERONLY); | ||
|
|
||
| /// <summary> | ||
| /// The discord **FORUM** channel id that admin messages are sent to. If it's not a forum channel, everything will explode. | ||
| /// </summary> | ||
| public static readonly CVarDef<string> AdminAhelpRelayChannelId = | ||
| CVarDef.Create("admin.ahelp_relay_channel_id", string.Empty, CVar.SERVERONLY); | ||
|
|
||
| /// <summary> | ||
| /// URL of the TTS server API. | ||
| /// </summary> | ||
| public static readonly CVarDef<bool> TTSEnabled = | ||
| CVarDef.Create("tts.enabled", false, CVar.SERVER | CVar.REPLICATED | CVar.ARCHIVE); | ||
|
|
||
| /// <summary> | ||
| /// URL of the TTS server API. | ||
| /// </summary> | ||
| public static readonly CVarDef<string> TTSApiUrl = | ||
| CVarDef.Create("tts.api_url", "", CVar.SERVERONLY | CVar.ARCHIVE); | ||
|
|
||
| /// <summary> | ||
| /// Auth token of the TTS server API. | ||
| /// </summary> | ||
| public static readonly CVarDef<string> TTSApiToken = | ||
| CVarDef.Create("tts.api_token", "", CVar.SERVERONLY | CVar.CONFIDENTIAL); | ||
|
|
||
| /// <summary> | ||
| /// Amount of seconds before timeout for API | ||
| /// </summary> | ||
| public static readonly CVarDef<int> TTSApiTimeout = | ||
| CVarDef.Create("tts.api_timeout", 5, CVar.SERVERONLY | CVar.ARCHIVE); | ||
|
|
||
| /// <summary> | ||
| /// Default volume setting of TTS sound | ||
| /// </summary> | ||
| public static readonly CVarDef<float> TTSVolume = | ||
| CVarDef.Create("tts.volume", 0f, CVar.CLIENTONLY | CVar.ARCHIVE); | ||
|
|
||
| /// <summary> | ||
| /// Count of in-memory cached tts voice lines. | ||
| /// </summary> | ||
| public static readonly CVarDef<int> TTSMaxCache = | ||
| CVarDef.Create("tts.max_cache", 250, CVar.SERVERONLY | CVar.ARCHIVE); | ||
|
|
||
| /// <summary> | ||
| /// VoiceId for Announcement TTS | ||
| /// </summary> | ||
| public static readonly CVarDef<string> TTSAnnounceVoiceId = | ||
| CVarDef.Create("tts.announce_voice", "Announcer", CVar.SERVERONLY | CVar.ARCHIVE); | ||
| /// If this is true, the ahelp relay shows that the response was from discord. If this is false, all messages an admin sends will be shown as if the admin was ingame. | ||
| /// </summary> | ||
| public static readonly CVarDef<bool> AdminAhelpRelayShowDiscord = | ||
| CVarDef.Create("admin.ahelp_relay_show_discord", true, CVar.SERVERONLY); | ||
|
|
||
| /// <summary> | ||
| /// The discord channel id that OOC messages are sent to. | ||
| /// </summary> | ||
| public static readonly CVarDef<string> OocRelayChannelId = | ||
| CVarDef.Create("ooc.relay_channel_id", "", CVar.SERVERONLY); | ||
| #endregion |
There was a problem hiding this comment.
💡 Codebase verification
Требуется улучшить валидацию конфигурации Discord
В текущей реализации присутствует только базовая проверка на пустые значения и простое преобразование Guild ID. Рекомендуется добавить следующие проверки в класс DiscordLink:
- Проверка формата Discord токена (должен соответствовать формату Bot token)
- Валидация Guild ID на соответствие формату Discord Snowflake
- Проверка префикса команд на недопустимые символы и максимальную длину
- Добавить логирование результатов валидации
Текущие проверки находятся в методе Initialize() класса DiscordLink.cs, но они недостаточно строгие для обеспечения корректной работы бота.
🔗 Analysis chain
Добавить валидацию конфигурации Discord
Рекомендуется реализовать валидацию всех Discord-related CVar'ов при запуске:
- Проверка формата Guild ID
- Валидация форматов Channel ID
- Проверка длины и допустимых символов в префиксе команд
Let me search for more context about Discord configuration validation.
Let me check the Discord connection initialization code specifically.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Проверяем наличие валидации конфигурации
# Поиск кода валидации Discord-конфигурации
rg -l "discord\.(token|guild_id|prefix)" --type cs
# Поиск проверок форматов ID
ast-grep --pattern 'if (!ulong.TryParse($_, out _)) { $$ }'
Length of output: 145
Script:
#!/bin/bash
# Search for Discord bot initialization and configuration code
rg -l "DiscordBot|Discord.*Client" --type cs
# Search for any validation patterns in Discord-related files
rg "validate|check|verify" -C 2 $(rg -l "Discord" --type cs)
# Look for Discord connection/startup code
ast-grep --pattern 'async $_ Connect$_() { $$ }'
Length of output: 8582
Script:
#!/bin/bash
# Look for Discord initialization code
rg -A 10 "Initialize|Connect" Content.Server/_White/Discord/DiscordLink.cs
# Check for any existing validation in the Discord connection code
rg -B 5 -A 10 "token|guild_id|prefix" Content.Server/_White/Discord/DiscordLink.cs
Length of output: 6253
|
Simple-Station/Einstein-Engines#1290 |
oke |
Описание PR
Теперь на ахелпы можно отвечать в дискорде, а ООС чат кидается в канал дискордика.
Полностью переписан BwoinkSystem, удачи решать мердж-конфликты с этой хуйней.
Изменения
no cl no fun, admin QoL