From 2555b10d49371f1b408fc3701acfcf106e0dfe73 Mon Sep 17 00:00:00 2001 From: Erik Montnemery Date: Wed, 7 Apr 2021 12:15:56 +0200 Subject: [PATCH] Remove login details before logging SQL errors (#48758) --- homeassistant/components/sql/sensor.py | 24 ++++++++++++++-- tests/components/sql/test_sensor.py | 40 ++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/homeassistant/components/sql/sensor.py b/homeassistant/components/sql/sensor.py index a537b160d0b..b90ce2f8e59 100644 --- a/homeassistant/components/sql/sensor.py +++ b/homeassistant/components/sql/sensor.py @@ -2,6 +2,7 @@ import datetime import decimal import logging +import re import sqlalchemy from sqlalchemy.orm import scoped_session, sessionmaker @@ -18,6 +19,13 @@ CONF_COLUMN_NAME = "column" CONF_QUERIES = "queries" CONF_QUERY = "query" +DB_URL_RE = re.compile("//.*:.*@") + + +def redact_credentials(data): + """Redact credentials from string data.""" + return DB_URL_RE.sub("//****:****@", data) + def validate_sql_select(value): """Validate that value is a SQL SELECT query.""" @@ -47,6 +55,7 @@ def setup_platform(hass, config, add_entities, discovery_info=None): if not db_url: db_url = DEFAULT_URL.format(hass_config_path=hass.config.path(DEFAULT_DB_FILE)) + sess = None try: engine = sqlalchemy.create_engine(db_url) sessmaker = scoped_session(sessionmaker(bind=engine)) @@ -56,10 +65,15 @@ def setup_platform(hass, config, add_entities, discovery_info=None): sess.execute("SELECT 1;") except sqlalchemy.exc.SQLAlchemyError as err: - _LOGGER.error("Couldn't connect using %s DB_URL: %s", db_url, err) + _LOGGER.error( + "Couldn't connect using %s DB_URL: %s", + redact_credentials(db_url), + redact_credentials(str(err)), + ) return finally: - sess.close() + if sess: + sess.close() queries = [] @@ -147,7 +161,11 @@ class SQLSensor(SensorEntity): value = str(value) self._attributes[key] = value except sqlalchemy.exc.SQLAlchemyError as err: - _LOGGER.error("Error executing query %s: %s", self._query, err) + _LOGGER.error( + "Error executing query %s: %s", + self._query, + redact_credentials(str(err)), + ) return finally: sess.close() diff --git a/tests/components/sql/test_sensor.py b/tests/components/sql/test_sensor.py index ddab7b1ba36..11f59444c2c 100644 --- a/tests/components/sql/test_sensor.py +++ b/tests/components/sql/test_sensor.py @@ -55,3 +55,43 @@ async def test_invalid_query(hass): state = hass.states.get("sensor.count_tables") assert state.state == STATE_UNKNOWN + + +@pytest.mark.parametrize( + "url,expected_patterns,not_expected_patterns", + [ + ( + "sqlite://homeassistant:hunter2@homeassistant.local", + ["sqlite://****:****@homeassistant.local"], + ["sqlite://homeassistant:hunter2@homeassistant.local"], + ), + ( + "sqlite://homeassistant.local", + ["sqlite://homeassistant.local"], + [], + ), + ], +) +async def test_invalid_url(hass, caplog, url, expected_patterns, not_expected_patterns): + """Test credentials in url is not logged.""" + config = { + "sensor": { + "platform": "sql", + "db_url": url, + "queries": [ + { + "name": "count_tables", + "query": "SELECT 5 as value", + "column": "value", + } + ], + } + } + + assert await async_setup_component(hass, "sensor", config) + await hass.async_block_till_done() + + for pattern in not_expected_patterns: + assert pattern not in caplog.text + for pattern in expected_patterns: + assert pattern in caplog.text