diff --git a/homeassistant/components/automation/config.py b/homeassistant/components/automation/config.py index bd87e359d7a..c127208377f 100644 --- a/homeassistant/components/automation/config.py +++ b/homeassistant/components/automation/config.py @@ -211,6 +211,7 @@ async def _try_async_validate_config_item( async def async_validate_config_item( hass: HomeAssistant, + config_key: str, config: dict[str, Any], ) -> AutomationConfig | None: """Validate config item, called by EditAutomationConfigView.""" diff --git a/homeassistant/components/config/__init__.py b/homeassistant/components/config/__init__.py index 329134e5486..def7edd4950 100644 --- a/homeassistant/components/config/__init__.py +++ b/homeassistant/components/config/__init__.py @@ -130,7 +130,7 @@ class BaseEditConfigView(HomeAssistantView): # We just validate, we don't store that data because # we don't want to store the defaults. if self.data_validator: - await self.data_validator(hass, data) + await self.data_validator(hass, config_key, data) else: self.data_schema(data) except (vol.Invalid, HomeAssistantError) as err: diff --git a/homeassistant/components/script/__init__.py b/homeassistant/components/script/__init__.py index 359486d2687..45786ae984a 100644 --- a/homeassistant/components/script/__init__.py +++ b/homeassistant/components/script/__init__.py @@ -7,10 +7,9 @@ import logging from typing import Any, cast import voluptuous as vol -from voluptuous.humanize import humanize_error from homeassistant.components import websocket_api -from homeassistant.components.blueprint import CONF_USE_BLUEPRINT, BlueprintInputs +from homeassistant.components.blueprint import CONF_USE_BLUEPRINT from homeassistant.const import ( ATTR_ENTITY_ID, ATTR_MODE, @@ -53,7 +52,7 @@ from homeassistant.helpers.typing import ConfigType from homeassistant.loader import bind_hass from homeassistant.util.dt import parse_datetime -from .config import ScriptConfig, async_validate_config_item +from .config import ScriptConfig from .const import ( ATTR_LAST_ACTION, ATTR_LAST_TRIGGERED, @@ -249,32 +248,11 @@ async def _prepare_script_config( """Parse configuration and prepare script entity configuration.""" script_configs: list[ScriptEntityConfig] = [] - conf: dict[str, dict[str, Any] | BlueprintInputs] = config[DOMAIN] + conf: dict[str, ConfigType] = config[DOMAIN] for key, config_block in conf.items(): - raw_blueprint_inputs = None - raw_config = None - - if isinstance(config_block, BlueprintInputs): - blueprint_inputs = config_block - raw_blueprint_inputs = blueprint_inputs.config_with_inputs - - try: - raw_config = blueprint_inputs.async_substitute() - config_block = cast( - dict[str, Any], - await async_validate_config_item(hass, raw_config), - ) - except vol.Invalid as err: - LOGGER.error( - "Blueprint %s generated invalid script with input %s: %s", - blueprint_inputs.blueprint.name, - blueprint_inputs.inputs, - humanize_error(config_block, err), - ) - continue - else: - raw_config = cast(ScriptConfig, config_block).raw_config + raw_config = cast(ScriptConfig, config_block).raw_config + raw_blueprint_inputs = cast(ScriptConfig, config_block).raw_blueprint_inputs script_configs.append( ScriptEntityConfig(config_block, key, raw_blueprint_inputs, raw_config) diff --git a/homeassistant/components/script/config.py b/homeassistant/components/script/config.py index bd7ca1fc791..193a010671e 100644 --- a/homeassistant/components/script/config.py +++ b/homeassistant/components/script/config.py @@ -1,14 +1,19 @@ """Config validation helper for the script integration.""" +from __future__ import annotations + +from collections.abc import Mapping from contextlib import suppress +from typing import Any import voluptuous as vol +from voluptuous.humanize import humanize_error from homeassistant.components.blueprint import ( - BlueprintInputs, + BlueprintException, is_blueprint_instance_config, ) from homeassistant.components.trace import TRACE_CONFIG_SCHEMA -from homeassistant.config import async_log_exception, config_without_domain +from homeassistant.config import config_without_domain from homeassistant.const import ( CONF_ALIAS, CONF_DEFAULT, @@ -19,6 +24,7 @@ from homeassistant.const import ( CONF_SEQUENCE, CONF_VARIABLES, ) +from homeassistant.core import HomeAssistant from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers import config_per_platform, config_validation as cv from homeassistant.helpers.script import ( @@ -27,6 +33,8 @@ from homeassistant.helpers.script import ( make_script_schema, ) from homeassistant.helpers.selector import validate_selector +from homeassistant.helpers.typing import ConfigType +from homeassistant.util.yaml.input import UndefinedSubstitution from .const import ( CONF_ADVANCED, @@ -65,45 +73,132 @@ SCRIPT_ENTITY_SCHEMA = make_script_schema( ) -async def async_validate_config_item(hass, config, full_config=None): +async def _async_validate_config_item( + hass: HomeAssistant, object_id: str, config: ConfigType, warn_on_errors: bool +) -> ScriptConfig: """Validate config item.""" + raw_config = None + raw_blueprint_inputs = None + uses_blueprint = False + with suppress(ValueError): # Invalid config + raw_config = dict(config) + + def _log_invalid_script( + err: Exception, + script_name: str, + problem: str, + data: Any, + ) -> None: + """Log an error about invalid script.""" + if not warn_on_errors: + return + + if uses_blueprint: + LOGGER.error( + "Blueprint '%s' generated invalid script with inputs %s: %s", + blueprint_inputs.blueprint.name, + blueprint_inputs.inputs, + humanize_error(data, err) if isinstance(err, vol.Invalid) else err, + ) + return + + LOGGER.error( + "%s %s and has been disabled: %s", + script_name, + problem, + humanize_error(data, err) if isinstance(err, vol.Invalid) else err, + ) + return + if is_blueprint_instance_config(config): + uses_blueprint = True blueprints = async_get_blueprints(hass) - return await blueprints.async_inputs_from_config(config) + try: + blueprint_inputs = await blueprints.async_inputs_from_config(config) + except BlueprintException as err: + if warn_on_errors: + LOGGER.error( + "Failed to generate script from blueprint: %s", + err, + ) + raise - config = SCRIPT_ENTITY_SCHEMA(config) - config[CONF_SEQUENCE] = await async_validate_actions_config( - hass, config[CONF_SEQUENCE] - ) + raw_blueprint_inputs = blueprint_inputs.config_with_inputs - return config + try: + config = blueprint_inputs.async_substitute() + raw_config = dict(config) + except UndefinedSubstitution as err: + if warn_on_errors: + LOGGER.error( + "Blueprint '%s' failed to generate script with inputs %s: %s", + blueprint_inputs.blueprint.name, + blueprint_inputs.inputs, + err, + ) + raise HomeAssistantError from err + + script_name = f"Script with object id '{object_id}'" + if isinstance(config, Mapping): + if CONF_ALIAS in config: + script_name = f"Script with alias '{config[CONF_ALIAS]}'" + + try: + cv.slug(object_id) + except vol.Invalid as err: + _log_invalid_script(err, script_name, "has invalid object id", object_id) + raise + try: + validated_config = SCRIPT_ENTITY_SCHEMA(config) + except vol.Invalid as err: + _log_invalid_script(err, script_name, "could not be validated", config) + raise + + try: + validated_config[CONF_SEQUENCE] = await async_validate_actions_config( + hass, validated_config[CONF_SEQUENCE] + ) + except ( + vol.Invalid, + HomeAssistantError, + ) as err: + _log_invalid_script( + err, script_name, "failed to setup actions", validated_config + ) + raise + + script_config = ScriptConfig(validated_config) + script_config.raw_blueprint_inputs = raw_blueprint_inputs + script_config.raw_config = raw_config + return script_config class ScriptConfig(dict): """Dummy class to allow adding attributes.""" - raw_config = None + raw_config: ConfigType | None = None + raw_blueprint_inputs: ConfigType | None = None -async def _try_async_validate_config_item(hass, object_id, config, full_config=None): +async def _try_async_validate_config_item( + hass: HomeAssistant, + object_id: str, + config: ConfigType, +) -> ScriptConfig | None: """Validate config item.""" - raw_config = None - with suppress(ValueError): # Invalid config - raw_config = dict(config) - try: - cv.slug(object_id) - config = await async_validate_config_item(hass, config, full_config) - except (vol.Invalid, HomeAssistantError) as ex: - async_log_exception(ex, DOMAIN, full_config or config, hass) + return await _async_validate_config_item(hass, object_id, config, True) + except (vol.Invalid, HomeAssistantError): return None - if isinstance(config, BlueprintInputs): - return config - config = ScriptConfig(config) - config.raw_config = raw_config - return config +async def async_validate_config_item( + hass: HomeAssistant, + object_id: str, + config: dict[str, Any], +) -> ScriptConfig | None: + """Validate config item, called by EditScriptConfigView.""" + return await _async_validate_config_item(hass, object_id, config, False) async def async_validate_config(hass, config): @@ -114,7 +209,7 @@ async def async_validate_config(hass, config): if object_id in scripts: LOGGER.warning("Duplicate script detected with name: '%s'", object_id) continue - cfg = await _try_async_validate_config_item(hass, object_id, cfg, config) + cfg = await _try_async_validate_config_item(hass, object_id, cfg) if cfg is not None: scripts[object_id] = cfg diff --git a/tests/components/config/test_script.py b/tests/components/config/test_script.py index bd79c000027..98757f894cb 100644 --- a/tests/components/config/test_script.py +++ b/tests/components/config/test_script.py @@ -71,6 +71,36 @@ async def test_update_script_config( assert new_data["moon"] == {"alias": "Moon updated", "sequence": []} +@pytest.mark.parametrize("script_config", ({},)) +async def test_update_script_config_with_error( + hass: HomeAssistant, hass_client, hass_config_store, caplog +): + """Test updating script config with errors.""" + with patch.object(config, "SECTIONS", ["script"]): + await async_setup_component(hass, "config", {}) + + assert sorted(hass.states.async_entity_ids("script")) == [] + + client = await hass_client() + + orig_data = {"sun": {}, "moon": {}} + hass_config_store["scripts.yaml"] = orig_data + + resp = await client.post( + "/api/config/script/config/moon", + data=json.dumps({}), + ) + await hass.async_block_till_done() + assert sorted(hass.states.async_entity_ids("script")) == [] + + assert resp.status != HTTPStatus.OK + result = await resp.json() + validation_error = "required key not provided @ data['sequence']" + assert result == {"message": f"Message malformed: {validation_error}"} + # Assert the validation error is not logged + assert validation_error not in caplog.text + + @pytest.mark.parametrize("script_config", ({},)) async def test_update_remove_key_script_config( hass: HomeAssistant, hass_client, hass_config_store diff --git a/tests/components/script/test_init.py b/tests/components/script/test_init.py index 09d2c3c70b1..7fbb1f6bfe4 100644 --- a/tests/components/script/test_init.py +++ b/tests/components/script/test_init.py @@ -38,6 +38,7 @@ from homeassistant.helpers.script import ( ) from homeassistant.helpers.service import async_get_all_descriptions from homeassistant.setup import async_setup_component +from homeassistant.util import yaml import homeassistant.util.dt as dt_util from tests.common import async_fire_time_changed, async_mock_service, mock_restore_cache @@ -166,6 +167,67 @@ async def test_setup_with_invalid_configs(hass, value): assert len(hass.states.async_entity_ids("script")) == 0 +@pytest.mark.parametrize( + "object_id, broken_config, problem, details", + ( + ( + "Bad Script", + {}, + "has invalid object id", + "invalid slug Bad Script", + ), + ( + "bad_script", + {}, + "could not be validated", + "required key not provided @ data['sequence']", + ), + ( + "bad_script", + { + "sequence": { + "condition": "state", + # The UUID will fail being resolved to en entity_id + "entity_id": "abcdabcdabcdabcdabcdabcdabcdabcd", + "state": "blah", + }, + }, + "failed to setup actions", + "Unknown entity registry entry abcdabcdabcdabcdabcdabcdabcdabcd.", + ), + ), +) +async def test_bad_config_validation( + hass: HomeAssistant, caplog, object_id, broken_config, problem, details +): + """Test bad script configuration which can be detected during validation.""" + assert await async_setup_component( + hass, + script.DOMAIN, + { + script.DOMAIN: { + object_id: {"alias": "bad_script", **broken_config}, + "good_script": { + "alias": "good_script", + "sequence": { + "service": "test.automation", + "entity_id": "hello.world", + }, + }, + } + }, + ) + + # Check we get the expected error message + assert ( + f"Script with alias 'bad_script' {problem} and has been disabled: {details}" + in caplog.text + ) + + # Make sure one bad automation does not prevent other automations from setting up + assert hass.states.async_entity_ids("script") == ["script.good_script"] + + @pytest.mark.parametrize("running", ["no", "same", "different"]) async def test_reload_service(hass, running): """Verify the reload service.""" @@ -1248,3 +1310,81 @@ async def test_script_service_changed_entity_id(hass: HomeAssistant) -> None: assert len(calls) == 2 assert calls[1].data["entity_id"] == "script.custom_entity_id_2" + + +@pytest.mark.parametrize( + "blueprint_inputs, problem, details", + ( + ( + # No input + {}, + "Failed to generate script from blueprint", + "Missing input service_to_call", + ), + ( + # Missing input + {"a_number": 5}, + "Failed to generate script from blueprint", + "Missing input service_to_call", + ), + ( + # Wrong input + { + "trigger_event": "blueprint_event", + "service_to_call": {"dict": "not allowed"}, + "a_number": 5, + }, + "Blueprint 'Call service' generated invalid script", + "value should be a string for dictionary value @ data['sequence'][0]['service']", + ), + ), +) +async def test_blueprint_script_bad_config( + hass, caplog, blueprint_inputs, problem, details +): + """Test blueprint script with bad inputs.""" + assert await async_setup_component( + hass, + script.DOMAIN, + { + script.DOMAIN: { + "test_script": { + "use_blueprint": { + "path": "test_service.yaml", + "input": blueprint_inputs, + } + } + } + }, + ) + assert problem in caplog.text + assert details in caplog.text + + +async def test_blueprint_script_fails_substitution(hass, caplog): + """Test blueprint script with bad inputs.""" + with patch( + "homeassistant.components.blueprint.models.BlueprintInputs.async_substitute", + side_effect=yaml.UndefinedSubstitution("blah"), + ): + assert await async_setup_component( + hass, + script.DOMAIN, + { + script.DOMAIN: { + "test_script": { + "use_blueprint": { + "path": "test_service.yaml", + "input": { + "service_to_call": "test.automation", + }, + } + } + } + }, + ) + assert ( + "Blueprint 'Call service' failed to generate script with inputs " + "{'service_to_call': 'test.automation'}: No substitution found for input blah" + in caplog.text + )