diff --git a/homeassistant/components/config/core.py b/homeassistant/components/config/core.py index d9029dc497f..a6b39e556aa 100644 --- a/homeassistant/components/config/core.py +++ b/homeassistant/components/config/core.py @@ -44,8 +44,8 @@ class CheckConfigView(HomeAssistantView): vol.Optional("unit_system"): cv.unit_system, vol.Optional("location_name"): str, vol.Optional("time_zone"): cv.time_zone, - vol.Optional("external_url"): vol.Any(cv.url, None), - vol.Optional("internal_url"): vol.Any(cv.url, None), + vol.Optional("external_url"): vol.Any(cv.url_no_path, None), + vol.Optional("internal_url"): vol.Any(cv.url_no_path, None), vol.Optional("currency"): cv.currency, } ) diff --git a/homeassistant/config.py b/homeassistant/config.py index 12a39ab291b..cd159dfc8ce 100644 --- a/homeassistant/config.py +++ b/homeassistant/config.py @@ -10,6 +10,7 @@ import re import shutil from types import ModuleType from typing import Any, Callable +from urllib.parse import urlparse from awesomeversion import AwesomeVersion import voluptuous as vol @@ -161,6 +162,19 @@ def _no_duplicate_auth_mfa_module( return configs +def _filter_bad_internal_external_urls(conf: dict) -> dict: + """Filter internal/external URL with a path.""" + for key in CONF_INTERNAL_URL, CONF_EXTERNAL_URL: + if key in conf and urlparse(conf[key]).path not in ("", "/"): + # We warn but do not fix, because if this was incorrectly configured, + # adjusting this value might impact security. + _LOGGER.warning( + "Invalid %s set. It's not allowed to have a path (/bla)", key + ) + + return conf + + PACKAGES_CONFIG_SCHEMA = cv.schema_with_slug_keys( # Package names are slugs vol.Schema({cv.string: vol.Any(dict, list, None)}) # Component config ) @@ -188,59 +202,64 @@ CUSTOMIZE_CONFIG_SCHEMA = vol.Schema( } ) -CORE_CONFIG_SCHEMA = CUSTOMIZE_CONFIG_SCHEMA.extend( - { - CONF_NAME: vol.Coerce(str), - CONF_LATITUDE: cv.latitude, - CONF_LONGITUDE: cv.longitude, - CONF_ELEVATION: vol.Coerce(int), - vol.Optional(CONF_TEMPERATURE_UNIT): cv.temperature_unit, - CONF_UNIT_SYSTEM: cv.unit_system, - CONF_TIME_ZONE: cv.time_zone, - vol.Optional(CONF_INTERNAL_URL): cv.url, - vol.Optional(CONF_EXTERNAL_URL): cv.url, - vol.Optional(CONF_ALLOWLIST_EXTERNAL_DIRS): vol.All( - cv.ensure_list, [vol.IsDir()] # pylint: disable=no-value-for-parameter - ), - vol.Optional(LEGACY_CONF_WHITELIST_EXTERNAL_DIRS): vol.All( - cv.ensure_list, [vol.IsDir()] # pylint: disable=no-value-for-parameter - ), - vol.Optional(CONF_ALLOWLIST_EXTERNAL_URLS): vol.All(cv.ensure_list, [cv.url]), - vol.Optional(CONF_PACKAGES, default={}): PACKAGES_CONFIG_SCHEMA, - vol.Optional(CONF_AUTH_PROVIDERS): vol.All( - cv.ensure_list, - [ - auth_providers.AUTH_PROVIDER_SCHEMA.extend( - { - CONF_TYPE: vol.NotIn( - ["insecure_example"], - "The insecure_example auth provider" - " is for testing only.", - ) - } - ) - ], - _no_duplicate_auth_provider, - ), - vol.Optional(CONF_AUTH_MFA_MODULES): vol.All( - cv.ensure_list, - [ - auth_mfa_modules.MULTI_FACTOR_AUTH_MODULE_SCHEMA.extend( - { - CONF_TYPE: vol.NotIn( - ["insecure_example"], - "The insecure_example mfa module is for testing only.", - ) - } - ) - ], - _no_duplicate_auth_mfa_module, - ), - # pylint: disable=no-value-for-parameter - vol.Optional(CONF_MEDIA_DIRS): cv.schema_with_slug_keys(vol.IsDir()), - vol.Optional(CONF_LEGACY_TEMPLATES): cv.boolean, - vol.Optional(CONF_CURRENCY): cv.currency, - } +CORE_CONFIG_SCHEMA = vol.All( + CUSTOMIZE_CONFIG_SCHEMA.extend( + { + CONF_NAME: vol.Coerce(str), + CONF_LATITUDE: cv.latitude, + CONF_LONGITUDE: cv.longitude, + CONF_ELEVATION: vol.Coerce(int), + vol.Optional(CONF_TEMPERATURE_UNIT): cv.temperature_unit, + CONF_UNIT_SYSTEM: cv.unit_system, + CONF_TIME_ZONE: cv.time_zone, + vol.Optional(CONF_INTERNAL_URL): cv.url, + vol.Optional(CONF_EXTERNAL_URL): cv.url, + vol.Optional(CONF_ALLOWLIST_EXTERNAL_DIRS): vol.All( + cv.ensure_list, [vol.IsDir()] # pylint: disable=no-value-for-parameter + ), + vol.Optional(LEGACY_CONF_WHITELIST_EXTERNAL_DIRS): vol.All( + cv.ensure_list, [vol.IsDir()] # pylint: disable=no-value-for-parameter + ), + vol.Optional(CONF_ALLOWLIST_EXTERNAL_URLS): vol.All( + cv.ensure_list, [cv.url] + ), + vol.Optional(CONF_PACKAGES, default={}): PACKAGES_CONFIG_SCHEMA, + vol.Optional(CONF_AUTH_PROVIDERS): vol.All( + cv.ensure_list, + [ + auth_providers.AUTH_PROVIDER_SCHEMA.extend( + { + CONF_TYPE: vol.NotIn( + ["insecure_example"], + "The insecure_example auth provider" + " is for testing only.", + ) + } + ) + ], + _no_duplicate_auth_provider, + ), + vol.Optional(CONF_AUTH_MFA_MODULES): vol.All( + cv.ensure_list, + [ + auth_mfa_modules.MULTI_FACTOR_AUTH_MODULE_SCHEMA.extend( + { + CONF_TYPE: vol.NotIn( + ["insecure_example"], + "The insecure_example mfa module is for testing only.", + ) + } + ) + ], + _no_duplicate_auth_mfa_module, + ), + # pylint: disable=no-value-for-parameter + vol.Optional(CONF_MEDIA_DIRS): cv.schema_with_slug_keys(vol.IsDir()), + vol.Optional(CONF_LEGACY_TEMPLATES): cv.boolean, + vol.Optional(CONF_CURRENCY): cv.currency, + } + ), + _filter_bad_internal_external_urls, ) diff --git a/homeassistant/core.py b/homeassistant/core.py index e2418321592..d95c25d93e2 100644 --- a/homeassistant/core.py +++ b/homeassistant/core.py @@ -19,6 +19,7 @@ import threading from time import monotonic from types import MappingProxyType from typing import TYPE_CHECKING, Any, Callable, Optional, TypeVar, cast +from urllib.parse import urlparse import attr import voluptuous as vol @@ -1717,19 +1718,35 @@ class Config: ) data = await store.async_load() - if data: - self._update( - source=SOURCE_STORAGE, - latitude=data.get("latitude"), - longitude=data.get("longitude"), - elevation=data.get("elevation"), - unit_system=data.get("unit_system"), - location_name=data.get("location_name"), - time_zone=data.get("time_zone"), - external_url=data.get("external_url", _UNDEF), - internal_url=data.get("internal_url", _UNDEF), - currency=data.get("currency"), - ) + if not data: + return + + # In 2021.9 we fixed validation to disallow a path (because that's never correct) + # but this data still lives in storage, so we print a warning. + if "external_url" in data and urlparse(data["external_url"]).path not in ( + "", + "/", + ): + _LOGGER.warning("Invalid external_url set. It's not allowed to have a path") + + if "internal_url" in data and urlparse(data["internal_url"]).path not in ( + "", + "/", + ): + _LOGGER.warning("Invalid internal_url set. It's not allowed to have a path") + + self._update( + source=SOURCE_STORAGE, + latitude=data.get("latitude"), + longitude=data.get("longitude"), + elevation=data.get("elevation"), + unit_system=data.get("unit_system"), + location_name=data.get("location_name"), + time_zone=data.get("time_zone"), + external_url=data.get("external_url", _UNDEF), + internal_url=data.get("internal_url", _UNDEF), + currency=data.get("currency"), + ) async def async_store(self) -> None: """Store [homeassistant] core config.""" diff --git a/homeassistant/helpers/config_validation.py b/homeassistant/helpers/config_validation.py index 66d1c01d6d3..f8d69a6e49a 100644 --- a/homeassistant/helpers/config_validation.py +++ b/homeassistant/helpers/config_validation.py @@ -649,6 +649,16 @@ def url(value: Any) -> str: raise vol.Invalid("invalid url") +def url_no_path(value: Any) -> str: + """Validate a url without a path.""" + url_in = url(value) + + if urlparse(url_in).path not in ("", "/"): + raise vol.Invalid("url it not allowed to have a path component") + + return url_in + + def x10_address(value: str) -> str: """Validate an x10 address.""" regex = re.compile(r"([A-Pa-p]{1})(?:[2-9]|1[0-6]?)$") diff --git a/tests/helpers/test_config_validation.py b/tests/helpers/test_config_validation.py index c5e9f5880c4..79b558e5083 100644 --- a/tests/helpers/test_config_validation.py +++ b/tests/helpers/test_config_validation.py @@ -120,6 +120,25 @@ def test_url(): assert schema(value) +def test_url_no_path(): + """Test URL.""" + schema = vol.Schema(cv.url_no_path) + + for value in ( + "https://localhost/test/index.html", + "http://home-assistant.io/test/", + ): + with pytest.raises(vol.MultipleInvalid): + schema(value) + + for value in ( + "http://localhost", + "http://home-assistant.io", + "https://community.home-assistant.io/", + ): + assert schema(value) + + def test_platform_config(): """Test platform config validation.""" options = ({}, {"hello": "world"}) diff --git a/tests/test_config.py b/tests/test_config.py index 96196c943aa..441029d27dc 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -215,6 +215,19 @@ def test_core_config_schema(): ) +def test_core_config_schema_internal_external_warning(caplog): + """Test that we warn for internal/external URL with path.""" + config_util.CORE_CONFIG_SCHEMA( + { + "external_url": "https://www.example.com/bla", + "internal_url": "http://example.local/yo", + } + ) + + assert "Invalid external_url set" in caplog.text + assert "Invalid internal_url set" in caplog.text + + def test_customize_dict_schema(): """Test basic customize config validation.""" values = ({ATTR_FRIENDLY_NAME: None}, {ATTR_ASSUMED_STATE: "2"}) diff --git a/tests/test_core.py b/tests/test_core.py index 77ec07e6a63..df66fedd025 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -1374,6 +1374,21 @@ async def test_additional_data_in_core_config(hass, hass_storage): assert config.location_name == "Test Name" +async def test_incorrect_internal_external_url(hass, hass_storage, caplog): + """Test that we warn when detecting invalid internal/extenral url.""" + config = ha.Config(hass) + hass_storage[ha.CORE_STORAGE_KEY] = { + "version": 1, + "data": { + "internal_url": "https://community.home-assistant.io/profile", + "external_url": "https://www.home-assistant.io/blue", + }, + } + await config.async_load() + assert "Invalid external_url set" in caplog.text + assert "Invalid internal_url set" in caplog.text + + async def test_start_events(hass): """Test events fired when starting Home Assistant.""" hass.state = ha.CoreState.not_running