Ensure internal/external URL have no path (#54304)

* Ensure internal/external URL have no path

* Fix comment typo

Co-authored-by: Martin Hjelmare <marhje52@gmail.com>

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("external_url"): vol.Any(cv.url_no_path, None),
-        vol.Optional("internal_url"): vol.Any(cv.url, None),
+        vol.Optional("internal_url"): vol.Any(cv.url_no_path, None),
         vol.Optional("currency"): cv.currency,
     }
 )

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,7 +202,8 @@ CUSTOMIZE_CONFIG_SCHEMA = vol.Schema(
     }
 )
 
-CORE_CONFIG_SCHEMA = CUSTOMIZE_CONFIG_SCHEMA.extend(
+CORE_CONFIG_SCHEMA = vol.All(
+    CUSTOMIZE_CONFIG_SCHEMA.extend(
         {
             CONF_NAME: vol.Coerce(str),
             CONF_LATITUDE: cv.latitude,
@@ -205,7 +220,9 @@ CORE_CONFIG_SCHEMA = CUSTOMIZE_CONFIG_SCHEMA.extend(
             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_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,
@@ -241,6 +258,8 @@ CORE_CONFIG_SCHEMA = CUSTOMIZE_CONFIG_SCHEMA.extend(
             vol.Optional(CONF_LEGACY_TEMPLATES): cv.boolean,
             vol.Optional(CONF_CURRENCY): cv.currency,
         }
+    ),
+    _filter_bad_internal_external_urls,
 )
 
 

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,7 +1718,23 @@ class Config:
         )
         data = await store.async_load()
 
-        if data:
+        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"),

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]?)$")

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"})

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"})

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