Don't print MQTT credentials to log #37364
Conversation
probot-home-assistant
bot
added
bugfix
core
integration: mqtt
small-pr
|
Hey there @home-assistant/core, mind taking a look at this pull request as its been labeled with an integration ( |
| tmp_data.pop(CONF_PASSWORD, None) | ||
| tmp_data.pop(CONF_USERNAME, None) |
There was a problem hiding this comment.
Instead of popping it, should we replace it with ******** ? And maybe only password ?
There was a problem hiding this comment.
Also wonder if we can actually specify which keys are going to be overridden, not just show all the data ?
There was a problem hiding this comment.
OK, improved to replace the password with ********.
It will now print all keys present in both the config entry, and in configuration.yaml, including those which are implicitly present because they have default values.
It could print only keys which actually differ?
|
Clearing from current milestone, as I am prepping it now. Will target 112.2. |
| tmp_data = dict(entry.data) | ||
| tmp_data.pop(CONF_PASSWORD, None) | ||
| tmp_data.pop(CONF_USERNAME, None) | ||
| shared_keys = conf.keys() & entry.data.keys() |
There was a problem hiding this comment.
I didn't know this was a thing. Nice.
|
Maybe somethings like: https://github.com/home-assistant/core/blob/dev/homeassistant/util/logging.py#L15 ? |
Proposed change
Don't print MQTT credentials to log
Type of change
Additional information
Checklist
black --fast homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all..coveragerc.The integration reached or maintains the following Integration Quality Scale: