From 586d2ceff66480faea4ee1e9025172239b1aa346 Mon Sep 17 00:00:00 2001 From: potelux Date: Tue, 31 Mar 2026 16:07:26 -0500 Subject: [PATCH] Add reload service to shell_command (#166557) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Joostlek --- .../components/shell_command/__init__.py | 101 +++++++++++++++--- .../components/shell_command/icons.json | 7 ++ .../components/shell_command/services.yaml | 2 +- .../components/shell_command/strings.json | 12 +++ tests/components/shell_command/test_init.py | 83 +++++++++++++- 5 files changed, 190 insertions(+), 15 deletions(-) create mode 100644 homeassistant/components/shell_command/icons.json diff --git a/homeassistant/components/shell_command/__init__.py b/homeassistant/components/shell_command/__init__.py index 842dc74ea5a..8dbd867a389 100644 --- a/homeassistant/components/shell_command/__init__.py +++ b/homeassistant/components/shell_command/__init__.py @@ -3,12 +3,16 @@ from __future__ import annotations import asyncio +from collections.abc import Callable, Coroutine from contextlib import suppress import logging import shlex +from typing import Any import voluptuous as vol +import homeassistant.config as conf_util +from homeassistant.const import SERVICE_RELOAD from homeassistant.core import ( HomeAssistant, ServiceCall, @@ -16,7 +20,12 @@ from homeassistant.core import ( SupportsResponse, ) from homeassistant.exceptions import HomeAssistantError, TemplateError -from homeassistant.helpers import config_validation as cv, template +from homeassistant.helpers import ( + config_validation as cv, + issue_registry as ir, + service as service_helper, + template, +) from homeassistant.helpers.typing import ConfigType from homeassistant.util.json import JsonObjectType @@ -31,16 +40,14 @@ CONFIG_SCHEMA = vol.Schema( ) -async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: - """Set up the shell_command component.""" - conf = config.get(DOMAIN, {}) - - cache: dict[str, tuple[str, str | None, template.Template | None]] = {} +def _make_handler( + cmd: str, + hass: HomeAssistant, + cache: dict[str, tuple[str, str | None, template.Template | None]], +) -> Callable[[ServiceCall], Coroutine[Any, Any, ServiceResponse]]: + """Return a service handler that executes the given shell command.""" async def async_service_handler(service: ServiceCall) -> ServiceResponse: - """Execute a shell command service.""" - cmd = conf[service.service] - if cmd in cache: prog, args, args_compiled = cache[cmd] elif " " not in cmd: @@ -66,7 +73,6 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: if rendered_args == args: # No template used. default behavior - create_process = asyncio.create_subprocess_shell( cmd, stdin=None, @@ -78,7 +84,6 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: # Template used. Break into list and use create_subprocess_exec # (which uses shell=False) for security shlexed_cmd = [prog, *shlex.split(rendered_args)] - create_process = asyncio.create_subprocess_exec( *shlexed_cmd, stdin=None, @@ -153,11 +158,81 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: return service_response return None - for name in conf: + return async_service_handler + + +async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: + """Set up the shell_command component.""" + conf = config.get(DOMAIN, {}) + + cache: dict[str, tuple[str, str | None, template.Template | None]] = {} + + for name, command in conf.items(): + if name == SERVICE_RELOAD: + ir.async_create_issue( + hass, + DOMAIN, + f"reserved_{SERVICE_RELOAD}", + is_fixable=False, + severity=ir.IssueSeverity.ERROR, + translation_key="reserved_reload_name", + translation_placeholders={"name": name}, + ) + _LOGGER.warning("Skipping shell_command entry '%s': name is reserved", name) + continue hass.services.async_register( DOMAIN, name, - async_service_handler, + _make_handler(command, hass, cache), supports_response=SupportsResponse.OPTIONAL, ) + + async def reload_service_handler(service_call: ServiceCall) -> None: + """Reload shell_command from YAML configuration.""" + try: + raw_config = await conf_util.async_hass_config_yaml(hass) + except HomeAssistantError as err: + _LOGGER.error("Error loading configuration.yaml: %s", err) + return + + try: + new_conf = CONFIG_SCHEMA(raw_config).get(DOMAIN, {}) + except vol.Invalid as err: + _LOGGER.error("Invalid shell_command configuration: %s", err) + return + + for svc in list(hass.services.async_services_for_domain(DOMAIN)): + if svc != SERVICE_RELOAD: + hass.services.async_remove(DOMAIN, svc) + cache.clear() + ir.async_delete_issue(hass, DOMAIN, f"reserved_{SERVICE_RELOAD}") + for name, command in new_conf.items(): + if name == SERVICE_RELOAD: + ir.async_create_issue( + hass, + DOMAIN, + f"reserved_{SERVICE_RELOAD}", + is_fixable=False, + severity=ir.IssueSeverity.ERROR, + translation_key="reserved_reload_name", + translation_placeholders={"name": name}, + ) + _LOGGER.warning( + "Skipping shell_command entry '%s': name is reserved", name + ) + continue + hass.services.async_register( + DOMAIN, + name, + _make_handler(command, hass, cache), + supports_response=SupportsResponse.OPTIONAL, + ) + + service_helper.async_register_admin_service( + hass, + DOMAIN, + SERVICE_RELOAD, + reload_service_handler, + ) + return True diff --git a/homeassistant/components/shell_command/icons.json b/homeassistant/components/shell_command/icons.json new file mode 100644 index 00000000000..a9829425570 --- /dev/null +++ b/homeassistant/components/shell_command/icons.json @@ -0,0 +1,7 @@ +{ + "services": { + "reload": { + "service": "mdi:reload" + } + } +} diff --git a/homeassistant/components/shell_command/services.yaml b/homeassistant/components/shell_command/services.yaml index df056f94e85..c983a105c93 100644 --- a/homeassistant/components/shell_command/services.yaml +++ b/homeassistant/components/shell_command/services.yaml @@ -1 +1 @@ -# Empty file, shell_command services are dynamically created +reload: diff --git a/homeassistant/components/shell_command/strings.json b/homeassistant/components/shell_command/strings.json index f2f2dc1b819..a395ef9bd52 100644 --- a/homeassistant/components/shell_command/strings.json +++ b/homeassistant/components/shell_command/strings.json @@ -6,5 +6,17 @@ "timeout": { "message": "Timed out running command: `{command}`, after: {timeout} seconds" } + }, + "issues": { + "reserved_reload_name": { + "description": "The shell command name {name} is a reserved for the reload action and cannot be used for user-defined commands. Please rename or remove this entry from your configuration.", + "title": "Reserved shell command action name" + } + }, + "services": { + "reload": { + "description": "Reloads shell command configuration.", + "name": "[%key:common::action::reload%]" + } } } diff --git a/tests/components/shell_command/test_init.py b/tests/components/shell_command/test_init.py index 526ac1643ec..cfc36d297e1 100644 --- a/tests/components/shell_command/test_init.py +++ b/tests/components/shell_command/test_init.py @@ -10,10 +10,14 @@ from unittest.mock import AsyncMock, MagicMock, Mock, patch import pytest from homeassistant.components import shell_command -from homeassistant.core import HomeAssistant +from homeassistant.const import SERVICE_RELOAD +from homeassistant.core import Context, HomeAssistant from homeassistant.exceptions import HomeAssistantError, TemplateError +from homeassistant.helpers import issue_registry as ir from homeassistant.setup import async_setup_component +from tests.common import MockUser + def mock_process_creator(error: bool = False): """Mock a coroutine that creates a process when yielded.""" @@ -276,3 +280,80 @@ async def test_do_not_run_forever( mock_process.kill.assert_called_once() assert "Timed out" in caplog.text assert "mock_sleep 10000" in caplog.text + + +async def test_reload_service(hass: HomeAssistant, hass_admin_user: MockUser) -> None: + """Test that the reload service re-registers commands from YAML.""" + assert await async_setup_component( + hass, + shell_command.DOMAIN, + {shell_command.DOMAIN: {"initial_cmd": "echo initial"}}, + ) + await hass.async_block_till_done() + + assert hass.services.has_service(shell_command.DOMAIN, "initial_cmd") + + with patch( + "homeassistant.config.load_yaml_config_file", + autospec=True, + return_value={shell_command.DOMAIN: {"reloaded_cmd": "echo reloaded"}}, + ): + await hass.services.async_call( + shell_command.DOMAIN, + SERVICE_RELOAD, + blocking=True, + context=Context(user_id=hass_admin_user.id), + ) + + assert not hass.services.has_service(shell_command.DOMAIN, "initial_cmd") + assert hass.services.has_service(shell_command.DOMAIN, "reloaded_cmd") + + +async def test_repair_issue_on_reserved_reload_name( + hass: HomeAssistant, issue_registry: ir.IssueRegistry, hass_admin_user: MockUser +) -> None: + """Test repair issue is created if 'reload' is used as a shell_command name.""" + config = {shell_command.DOMAIN: {"reload": "echo should not work"}} + await async_setup_component(hass, shell_command.DOMAIN, config) + await hass.async_block_till_done() + issue = issue_registry.async_get_issue(shell_command.DOMAIN, "reserved_reload") + assert issue is not None + assert issue.translation_key == "reserved_reload_name" + assert issue.severity == ir.IssueSeverity.ERROR + assert issue.translation_placeholders["name"] == "reload" + with patch( + "homeassistant.config.load_yaml_config_file", + autospec=True, + return_value={shell_command.DOMAIN: {"reloaded_cmd": "echo reloaded"}}, + ): + await hass.services.async_call( + shell_command.DOMAIN, + SERVICE_RELOAD, + blocking=True, + context=Context(user_id=hass_admin_user.id), + ) + issue = issue_registry.async_get_issue(shell_command.DOMAIN, "reserved_reload") + assert issue is None + + +async def test_repair_issue_on_reload_service_reload( + hass: HomeAssistant, issue_registry: ir.IssueRegistry, hass_admin_user: MockUser +) -> None: + """Test repair issue is created if 'reload' is used in YAML and reload service is called.""" + config = {shell_command.DOMAIN: {"test": "echo ok"}} + await async_setup_component(hass, shell_command.DOMAIN, config) + await hass.async_block_till_done() + + with patch( + "homeassistant.config.load_yaml_config_file", + autospec=True, + return_value={shell_command.DOMAIN: {"reload": "echo reloaded"}}, + ): + await hass.services.async_call( + shell_command.DOMAIN, + SERVICE_RELOAD, + blocking=True, + context=Context(user_id=hass_admin_user.id), + ) + issue = issue_registry.async_get_issue(shell_command.DOMAIN, "reserved_reload") + assert issue is not None