kord
kord copied to clipboard
kordlib
Don't log bot and voice tokens in Gateway implementations (trace level)
DefaultGateway logs the JSON payloads it sends when trace logging is enabled. It also hides the bot token in case of an Identify command. But it didn't do this for Resume commands, which also includes the bot token.
Same goes for DefaultVoiceGateway, which didn't hide the voice token for Resume commands.
This PR fixes this and also updates some toString implementations of data classes that contain a bot or voice token.
DefaultVoiceGateway also hides secret_key for SessionDescription events, this is also added for voice tokens in VoiceServerUpdate events that DefaultGateway receives.
The PR also makes the Json instance used by DefaultGateway and DefaultVoiceGateway a property of the companion object since it can be shared safely.
I'm not against it. Might as well be consistent across the entirety of Kord.
Probably sensitive enough, it is a token after all. I believe someone with access to it would be able to hijack the voice connection for a server (within a certain timeframe), though I haven't tried it :p
Other token types this PR didn't touch yet are interaction and webhook tokens, what about them?
Other token types this PR didn't touch yet are interaction and webhook tokens, what about them?
I think that, for consistency, it would be nice to hide them too, because they are sensitive too, because with interaction/webhook tokens you can send whatever message you want.
By the way, wouldn't it be better to filter the sensitive data on the logger end, instead of replacing all entities with a custom toString() implementation? While replacing the toString() implemention works, someone could forget to update it if new variables are added to the entities. Filtering on the logger side would make it easier to filter in other places were the token might get accidentally printed to the console, however you do have the disadvantage that it may filter something that it shouldn't be filtered AND how the tokens are filtered would vary depending on what logger the user is using (here's a logback example: https://www.baeldung.com/logback-mask-sensitive-data)
Hm, how would we filter it on the logger end without a logger implementation (kord doesn't provide one)?
Hm, how would we filter it on the logger end without a logger implementation (kord doesn't provide one)?
Yeah, that's one of the issues with this approach: The user would need to configure their logger implementation (logback, log4j, etc) to filter tokens
Not quite, Kord depends on kotlin-logging for logging, which simply wraps around the SLF4J api. I'm aware of the log4j api having an AbstractLogger class that can be implemented, which would then strip out the token(s) from any outgoing log message, and delegate it to the actual logger. I assume this can similarly be done with the slf4j abstract logger. Might be worth taking a look at.