-
Notifications
You must be signed in to change notification settings - Fork 8
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
MessageTag Manager: Allows server owners to permit specific client-to-client tags #94
base: unreal6
Are you sure you want to change the base?
Conversation
We talked about this in the development team and as you know our current stance is documented under https://www.unrealircd.org/docs/FAQ#How_can_I_allow_other_message_tags.3F
What your module does is allow the admin to configure tags and then allow these blindly through without really much or actually any checking, it checks just the name, not the value. In message tag modules the idea is that you do some value validation, length/syntax but also an example would be if you allow a tag that does reactions, you could apply channel mode +G filtering in the reaction, so you can't evade +G by using react rather than normal messages. And with for example display-name you could have similar thoughts. This module can't do that, it's generic. So we could still allow this module, but it should then be noted that specific modules are very much preferred. The +G filtering for react is an example, but more generally modules tend to limit length and syntax. Almost no message tag needs the full 4KB, which is quite ridiculous in my opinion. In our dev team some say we should not allow this module, because this complicates things to users too much, they would likely and quite easily think that just loading this module and then listing the tags they want to use, like +draft/react and the like, and they are done, why would they install any other module to do with tags? This one would do great, and it fixes them all !!! But... then you loose all the benefits of filtering and sanity checking of those other modules. I think that is a fair point. But I don't know... i am a bit stuck on this. There are pro's and con's but I have trouble deciding. The only thing I know is that I would not allow this in unrealircd itself, but that is not on the table anyway, this is about a third party module, and in third party modules we tend to allow almost everything. But even then, lots of the logic and comments still apply and it's so easy to make users make bad decisions with this module. |
Please note my word of caution about validation in the README (any reader) |
In https://bugs.unrealircd.org/view.php?id=6411 westid suggested adding more value checks. Like max key length, allowed values wildcard/regex, limiting by security group. I have also suggested part of these in the past (though not limiting by the security group, that idea is novel). I'm not sure if that will convince me or the other devs, but it is at least a good thought if you are going this way. What do you think about it? |
No description provided.