bot
bot copied to clipboard
python-discord
enhancement(filters): use a stricter bot token regex
Approval: https://canary.discord.com/channels/267624335836053506/635950537262759947/919203386660884560
Enhances the regex of the token remover to use the same regex that discord itself uses, with a slight modification. The mfa section was removed, but depending on an updated #1421, may be implemented. Additionally, the sections were grouped to keep working with the current code.
I kept the existing validation to keep false positives at a minimum. The current code checks the user resolves, the timestamp is valid, and the last section has at least 3 different characters.
As per @jb3, to implement:
- #1421
Rejected:
- also check the upper bound of a token timestamp, to ensure it was in the past.
Closes #1421
This has been inactive for a while so I'll put it up for grabs.
What's left to do? Addressing your comment and fixing the merge conflict?
This has been inactive for a while so I'll put it up for grabs.
What's left to do? Addressing your comment and fixing the merge conflict?
Yeah, merge main, match the output embed to the existing one, and fix the bug
As far as I can tell the mfa code is no longer necessary as discord doesn't have mfa tokens like that now.
There's also a bit of the matter that this regex is now out of date. I don't know what the current token length is.
There's also a bit of the matter that this regex is now out of date. I don't know what the current token length is.
I can't recall any problems with false positives, even with the current regex, so we can afford to be very general with what we match. Could just check that the first and last parts are >= 10 and middle is >= 5 characters in length or something.
I agree with wookie, I think having extensive matching for the sake of even future-proofing is okay, considering additional checks are being performed
The only inaccurate part is the last section is too short as it is, but because of how the parsing works, that isn't an issue, i suppose
The only inaccurate part is the last section is too short as it is, but because of how the parsing works, that isn't an issue, i suppose
If that's the case I think we should just have no upper limit on the length of the last section. Otherwise the code is somewhat misleading in how it works.
Hey @onerandomusername, thanks for your work so far. We needed to get this merged so went ahead and implemented the comments. Sorry the MFA part didn't end up being used 😅