New Filtering System #2390
Merged
New Filtering System #2390
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Tests and dependent functionality in other extensions will be re-added later on.
This commit provides the basis of the new filtering system: - The filtering cog consists of several filter lists loaded from the database (filtering.py). - Each filter list contains a list of filters, which are run in response to events (message posting, reaction, thread creation). Each filter list may choose to respond to different events (the subscribe method in filtering.py). - Each filter has settings (settings.py) which decide when it is going to be run (e.g it might be disabled in a specific channel), and what will happen if it triggers (e.g delete the offending message). - Not every filter has a value for every setting (the _settings_types package) . It will use the default settings specified by its filter list as a fallback. - Since each filter might have a different effect when triggered, we must check all relevant filters even if we found a triggered filter already, unlike in the old system. - Two triggered filters may specify different values for the same setting, therefore each entry has a rule for combining two different values (the __or__ method in each file in _settings_types). To avoid having to prefix each file with an underscore (or the bot will try to load it as a cog), the loading script was changed to ignore packages with names starting with an underscore. Alert sending is done via a webhook so that several embeds can be sent in the same message (will be useful for example for guild invite alerts). Filter lists and setting entries classes are loaded dynamically from their respective packages. In order to be able to test the new features, this commit also includes a migration of the regex-based filtering.
For legacy purposes, separate command groups were re-added for blacklists and whitelists. There's a new command group for filters. Not specifying a list type for the `filter list` command will cause the bot to try to infer whether there's only one kind of list, for example `!filter list tokens` will pull up the blacklist since there's no whitelist. If a required field is missing, the user will be prompted to complete it from a selection. Some level of flexibility was added for the list type in the `filter list` command. For example, a list type can be "DENY", but it will also match "blacklist", "denylist", "deny", "denied", "blacklisted" etc.
The channel scope settings were changed to accomodate strings. That means that if a string is specified, the bot will look whether the context channel's name matches. If it's a number, it will match the ID. Accordingly the same changed was applied to the bypass roles and pings settings: if it's a non-numeric string, it will look for a role with that name.
This commmit migrates the AntiMalware cog to a new filter list which goes over a message's attachments. Some changes were needed to accomodate the new list, primarily what a filter list returns for a given context: Instead of returning a list of filters, it will return the action itself that should be taken. This adds the flexibility of not needing existing filters to dictate the action. For example, in the case of the extensions list, an action should be taken when filters were *not* triggered. Or more precisely, when not all attachment extensions are whitelisted. Therefore, the action in that case is dictated by the filter list (stored as the list's default actions). Additionally each filter list can now return its own message for the alert embed, instead of the cog formatting it according to the filters raised. Because again, an action might be taken without any deny filters being triggered. This is going to be especially relevant for the invites list. Additionally, the infraction_and_notification action now doesn't redirect the notification to the context channel when the DM fails, since this can be incredibly noisy in cases of spam. If we want this functionality, a more suitable solution should be found.
This commit adds the invite filtering implementation to the new system. This also fixes an issue with the implementation of the extension filtering, where there was no way to tell the bot to ignore a user when they posted a non-whitelisted file extension, since there's no relevant filter in this scenario. Instead the extensions and invites filters now use the whitelist validation defaults to dictate when filtering should be done at all. For example, if the list validations are to ignore Helpers, then no invite filtering will occur no matter what. The meaning of this is that the system is somewhat less configurable, because settings overrides in filters belonging to a whitelist are meaningless. Additionally this commit contains the following fixes: - If the user tries to show the filters in a list which doesn't exist, it says it doesn't exist instead of saying there are 0 filters. - The filter context content is `Union[str, set]` instead of `Union[str, set[str]]`. - An empty embed will no longer be created when `dm_embed` is empty.
Previously the completed arg would be appended to the end of the message, which didn't work if the missing argument wasn't the last one (for example, a value was given to a following argument because of a failed conversion for previous ones). The select now employs a different strategy of storing the provided args, and the position where the new are should be inserted.
The domain filtering works very similarly to the token filtering, and the domain matching itself is based on the implementation in the old system. The deletion setting is accessed explicitly in the domain filter in order to allow DMing the user the domain the message got deleted for. This is fine, since practical uses are more important than the theory this system was designed with. Of course, breaking the design should still be avoided whenever possible.
Pydantic is added to the bot's dependencies, allowing easily setting up settings that are specific to a filter type (e.g for domains). The data for those settings is loaded from the "additional_field" JSON field in the Filter table. This commit specifically adds an option to match a domain exactly, without matching subdomains. There are several benefits to using Pydantic: - Built-in serialization to, and de-serialization from JSON, which is useful for reading and writing to the DB. - Type validation on initialization. - By default ignores extra fields when de-serializing, which can prevent things breaking if the extra fields change.
The system is fairly complex, and to avoid having to delve into the source code, this commit adds several commands to show descriptions for various components of the system: - Filters - Filter lists - Settings (both from the DB schema and filter-type-specific) The names that can be described are the ones which are actually being used in the system. So if no tokens list was loaded, it can't be described even if there is an implementation for it. If no name is relayed to the commands, the list of options will be shown instead.
The filterlist describe command is changed to include the default settings it contains. The filters group can now take a filter ID, and it will display embed detailing the filter data and settings. The mechanic is similar to how individual infractions can be displayed with `!infraction <id>`. - To be able to quickly find whether the filter with the provided ID is in a specific list, the data structure was changed to a dictionary. - To be able to mark which settings have the default values and which are overrides, resolving the full actions of a filter is deferred to when the filter is actually triggered. This wasn't possible in the beginning of development, but now that each filterlist can resolve the action to be taken with its own internal logic, it is. - Some attribute names of SettingsEntry subclasses were changed to match the name under which they're stored in the DB. This allows displaying the settings with the names that need to be used to edit them. - Each filterlist now contains all settings, even if they're empty, so that they can be displayed. While this is slightly less efficient, I considered it too negligible to make displaying the settings messier than it already is. - Some additional refactoring in the cog was done to avoid code repetition.
In order to facilitate this change, the init of all setting entries was removed, and as such the base SettingsEntry class doesn't count as abstract anymore, which broke the MUST_SET behavior. It was changed to not raise errors for MUST_SET values of attributes which were set in the current class. Additionally fixed a small bug where filters weren't listed properly in the list command. Despite the pydantic manual not writing it that way, I first made the validators class methods, otherwise it gave linting errors which couldn't be ignored with noqa (because then it complained about blanket noqas).
This commit adds the mechanic to add new filters. The overrides of the settings can be provided in the command itself, but also through a UI made of Discord components. The UI adjusts itself to the data type of the setting, e.g a boolean setting will invoke a select with "True" and "False" options. This commit additionally gets rid of the mechanic to apply a superstar alongside another higher tier infraction for the same message. This is an edge case that isn't worth the complexity at the moment. Includes a small fix to the Ping setting which made the __or__ method malfunction.
This is a remnant after the last rebase.
This is a purely aesthetic choice. Additionally fixes a small bug where a missing entry type would repeatedly invoke a warning on cog load.
Sequences such as sets are not serializable, and it's going to be converted to the right type when loaded to the cog anyway.
Some items might not be stored as strings.
There needs to be a way to only enable a filter in a specific category, so this setting now fulfills that role. Disabled channels can be used to disable a filter in a specific channel within the category. Additionally the validators which maybe convert to int are now removed. As long as the int is the first in the Union, it will happen anyway.
Adds the `CustomIOField` class which can be used as a base for wrappers that store a value with a customized way to process the user input and to present the value in the UI.
wookie184
approved these changes
Mar 28, 2023
Contributor
wookie184
left a comment
wookie184
left a comment
There was a problem hiding this comment.
Played around with this some more and didn't notice anything urgent. Very good!!!
shtlrs
requested changes
Apr 2, 2023
Member
shtlrs
left a comment
shtlrs
left a comment
There was a problem hiding this comment.
Looks & works great!
A couple of comments + small bugs were identified, will retest once addressed
bot/exts/filtering/_settings_types/validations/channel_scope.py
Outdated
Show resolved
Hide resolved
bot/exts/filtering/_settings_types/actions/infraction_and_notification.py
Show resolved
Hide resolved
bot/exts/filtering/_settings_types/actions/infraction_and_notification.py
Show resolved
Hide resolved
This shouldn't happen in production, but just in case
Also changes the alert to use the full domain instead of the filter content.
…ings This is to make sure that the unique constraint always compares between dicts instead of sometimes dealing with nulls.
shtlrs
approved these changes
Apr 6, 2023
Member
There was a problem hiding this comment.
The moderation lead strikes again, extremely beautiful work.
Like wookie said, there isn't urgent/alarming that I think needs to be addressed here.
I have played around with it and tried to test all the features that we rely on, and all seems to work fine after the small bugs we've found.
Thank you !
mbaruh
removed
the
review: do not merge
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request implements the new filtering system as described in #1530.
For more information about the database and API changes made to support this, check out the site PR: python-discord/site#861
closes #1530.
Overview
The main idea is that there is a list of filters each deciding whether they apply to the given content.
For example, there can be a filter that decides it will trigger when the content contains the string "lemon".
There are several types of filters, and two filters of the same type differ by their content.
For example, filters of type "token" search for a specific token (provided as regex) inside the provided string.
One token filter might look for the string "lemon", while another will look for the string "joe".
Each filter has a set of settings that decide when it triggers (e.g. in which channels), and what happens if it does (e.g. delete the message).
Filters of a specific type can have additional settings that are special to them.
A list of filters is contained within a filter list.
The filter list gets content to filter, and dispatches it to each of its filters.
It takes the answers from its filters and returns a unified response (e.g. if at least one of the filters says it should be deleted, then the filter list response will include it).
A filter list has the same set of possible settings, which act as defaults.
If a filter in the list doesn't define a value for a setting (meaning it has a value of None), it will use the value of the containing filter list.
The cog receives "filtering events". For example, a new message is sent.
It creates a "filtering context" with everything a filtering list needs to know to provide an answer for what should be done.
For example, if the event is a new message, then the content to filter is the content of the message, embeds if any exist, etc.
The cog dispatches the event to each filter list, gets the result from each, compiles them, and takes any action dictated by them.
For example, if any of the filter lists want the message to be deleted, then the cog will delete it.
Main Features
bypass_roles- A list of role IDs or role names. Users with these roles will not trigger the filter.enabled- A boolean field. Setting it to False allows disabling the filter without deleting it entirely.filter_dm- A boolean field. If True, the filter can trigger for messages sent to the bot in DMs.disabled_channels- A list of channel IDs or channel names. The filter will not trigger in these channels even if the category is expressly enabled.disabled_categories- A list of category IDs or category names. The filter will not trigger in these categories.enabled_channels- A list of channel IDs or channel names. The filter can trigger in these channels even if the category is disabled or not expressly enabled.enabled_categories- A list of category IDs or category names. If the list is not empty, filters will trigger only in channels of these categories, unless the channel is expressly disabled.send_alert- A boolean. If all filters triggered set this to False, no mod-alert will be created.remove_context- A boolean field. If True, the filter being triggered will cause the offending context to be removed. An offending message will be deleted, while an offending nickname will be superstarified.guild_pings- A list of role IDs/role names/user IDs/user names/here/everyone. If a mod-alert is generated for a filter triggered in a public channel, these will be pinged.dm_pings- A list of role IDs/role names/user IDs/user names/here/everyone. If a mod-alert is generated for a filter triggered in DMs, these will be pinged.infraction_type- The type of infraction to issue when the filter triggers, or 'NONE'. If two infractions are triggered for the same message, the harsher one will be applied (by type or duration).infraction_duration- How long the infraction should last for in seconds. 0 for permanent.infraction_reason- The reason delivered with the infraction.infraction_channel- The channel ID in which to invoke the infraction (and send the confirmation message). If 0, the infraction will be sent in the context channel. If the ID otherwise fails to resolve, it will default to the mod-alerts channel.dm_content- The contents of a message to be DMed to the offending user.dm_embed- The contents of the embed to be DMed to the offending user.Additionally closes #2266 as the formatting of the alert is significantly changed and should address the issue described.
Deferred Features
Some features are purposefully not addressed in this PR, as it is already very big, and they are not directly related to the scope of the PR or are minor:
!filter settingscommand, instead of the current feature which either lists all of them without descriptions, or shows a specific one with its description.