Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added spam and troll detection #152

Merged
merged 9 commits into from
Feb 6, 2022
Merged

Conversation

Picowchew
Copy link
Collaborator

@Picowchew Picowchew commented Feb 3, 2022

Related Issues

Resolves #138

Summary of Changes

If someone does @ everyone or @ here and includes a link with "http" or "nitro" outside of the PC category or if someone sends a message in the honeypot channel, then they get kicked and their relevant messages get deleted. The kick gets logged.

Steps to Reproduce

Try the above

@Picowchew Picowchew requested a review from a team February 3, 2022 20:50
Copy link
Collaborator

@gzcharleszhang gzcharleszhang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

really cool stuff, left some comments about code style

src/components/messageListener.ts Outdated Show resolved Hide resolved
src/components/messageListener.ts Outdated Show resolved Hide resolved
src/components/messageListener.ts Outdated Show resolved Hide resolved
src/components/messageListener.ts Outdated Show resolved Hide resolved
// Soft bans member and deletes messages from past 1 day
try {
if (await message.member?.ban({ days: 1, reason: 'Spammer/troll/got hacked' })) {
await message.guild?.members.unban(message.author.id);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, is there a reason why we need to wrap this in an if statement, if ban fails, wouldn't it throw an error and skip unban? I.e. is there a case where ban returns false instead of throwing an error.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the if condition, message.member might be null, in which undefined would returned, which would evaluate to false. I guess in this case though, message.member would never be null based on the if statement above it, but I guess this might change in the future?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you test out a code snippet that would trigger spam detection? like @everyone give nitro
Also, plz ban for 1 days and 1 hour in case someone auto spams exactly and every 24 hours.
Do we have protection against an exec or mod or advisor or someone of somewhat higher CSC status accidently getting banned and all msgs deleted?

Copy link
Collaborator Author

@Picowchew Picowchew Feb 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, the inline code block also triggers spam detection.
Done.
The Executive, Discord Mod, and Advisor roles are above the Codey role, so Codey can't possibly ban anyone with these roles.

Copy link
Collaborator

@marko-polo-cheno marko-polo-cheno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also constants -> config if not done already :)
Rest looks good to me - ty Charles u beat me to it.

// Soft bans member and deletes messages from past 1 day
try {
if (await message.member?.ban({ days: 1, reason: 'Spammer/troll/got hacked' })) {
await message.guild?.members.unban(message.author.id);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you test out a code snippet that would trigger spam detection? like @everyone give nitro
Also, plz ban for 1 days and 1 hour in case someone auto spams exactly and every 24 hours.
Do we have protection against an exec or mod or advisor or someone of somewhat higher CSC status accidently getting banned and all msgs deleted?

Copy link
Collaborator

@marko-polo-cheno marko-polo-cheno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there protection for general PC members?
would be good cause not everyone is exec, mod, or advisor

config/config.ts Outdated Show resolved Hide resolved
// Temp ban member for 1 day and 1 hour, and delete messages from past 1 day
try {
if (await message.member?.ban({ days: 1, reason: 'Spammer/troll/got hacked' })) {
setTimeout(async () => await message.guild?.members.unban(message.author.id), 25 * 60 * 60 * 1000); // 1 day and 1 hour in milliseconds
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implementation may not be resilient. Let's say if a user gets banned, and then the bot crashes or a new version gets deployed within 24 hours, then the user will not get unbanned.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative solution that I can think of is that the time of unban would be stored in the database and there would be a cron job to check if it is time to do any unbans? Also, is the current implementation okay or is a more resilient implementation wanted?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A db isn't needed as long as we print out exactly who was banned in # moderation and when they were banned, prior to actually kicking them.

We can manually do unbans upon request, which is probably very rare anyways.
What if we KICKED instead of banning for some amt of time? Right now, I think msgs disappear, which might not always be good. Suppose I stopped being a mod, and became a norm member, and got hacked, and made spam, hence banned - then would my 4000+ msgs all disappear?

@Picowchew
Copy link
Collaborator Author

is there protection for general PC members? would be good cause not everyone is exec, mod, or advisor

Yup, similarly, the PC role is above the Codey role, so Codey can't ban people with the PC role. I guess in the case that the roles get moved around, then the PC category check would help. For the official documentation, according to https://discord.com/developers/docs/topics/permissions, "A bot can only kick, ban, and edit nicknames for users whose highest role is lower than the bot's highest role."

Copy link
Collaborator

@gzcharleszhang gzcharleszhang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 minor comment otherwise LGTM

// Delete the message, and kick the member if they are still in the server
try {
await message.delete();
await message.member?.kick('Spammer/troll/got hacked');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we want to send the mod channel some kind of message? so we know someone was kicked

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes
A db isn't needed as long as we print out exactly who was kicked in # moderation and the msg they were kicked for, and when they were banned, prior to actually kicking them.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also :codey-poggers: to @Picowchew for getting this done so fast!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup done, except that I did the logging after the kick, since I don't think we want there to be logging if the kick doesn't go through (e.g. Codey cannot kick a mod).

Copy link
Collaborator

@marko-polo-cheno marko-polo-cheno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great stuff! :D

README.md Show resolved Hide resolved
@marko-polo-cheno
Copy link
Collaborator

marko-polo-cheno commented Feb 6, 2022

wait
one more thing, maybe it's good to log triggered kicks, even if they don't have an effect on mods

but that's optional ig

Copy link
Collaborator

@marko-polo-cheno marko-polo-cheno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome :) i guess u can merge and tell ppl to pull latest since there are some changes that effect a lot of files

@Picowchew Picowchew merged commit b9efbb0 into master Feb 6, 2022
@Picowchew Picowchew deleted the spam-and-troll-detection branch February 6, 2022 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Spam and troll detection
3 participants