Added template rendering to shell_command component

[partofthething]

Description:
This allows the shell_command component to accept templates. I use it to customize the commands that get sent over LIRC to my A/C based on an input slider value.

Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#

Example entry for configuration.yaml (if applicable):

input_slider:
  ac_temperature:
    name: A/C Setting
    initial: 24
    min: 18
    max: 32
    step: 1

input_select:
  fan_on:
    name: Turn on Fan
    options:
     - LOW
     - MEDIUM
     - HIGH
    initial: MEDIUM
    icon: mdi:weather-snowy

shell_command:
  set_ac_to_slider: 'irsend SEND_ONCE DELONGHI AC_{{ states.input_slider.ac_temperature.state}}_AUTO'
  fan_on: 'irsend SEND_ONCE DELONGHI FAN_{{states.input_select.fan_on.state}}'

Checklist:

If user exposed functionality or configuration variables are added/changed:

• Documentation added/updated in home-assistant.io See: Added example of using template in shell_command. home-assistant.io#547

If the code does not interact with devices:

• Local tests with tox run successfully. Your PR cannot be merged unless tests pass
• Tests have been added to verify that the new code works.

[balloob]

This has a big security vulnerability. If a user has a bad crafted template, an attacker would be able to run any command in the shell by setting a specific state and calling a service.

I wonder if it would be possible to do this safely.

[partofthething]

Good point. What if we sanitized the template values before passing them to the command and only allowed [0-9a-zA-Z_]+? That would at least give basic functionality (allowing for instance this super-awesome A/C controller functionality) but probably limits the damage that could be done.

[kellerza]

The first word in the shell cmd should be fixed, or any word that follows && / || and only inject numbers and letters like you suggest

[balloob]

I think that disabling shell=True when templates are involved would already make it more secure as it disables the ability to use pipes. Also make the template part never be able to specify the actual command.

But even then, I might prefer that we only allow writing dynamic content to STDIN. That's what we do with the command line notify platform. That way if people really want dynamic commands, they can make a bash script to execute STDIN but they will have to go through hoops to do it.

[partofthething]

The latest commit is an attempt to alleviate some of these concerns.

• It uses shell=False when templates are used.
• It only allows templates in the arguments, never the actual program.
• It defaults to regular old shell=True when no templates are present.

For further sanitizing the templates, I looked into the shlex module but in the end the best practice seemed to just use shell=False and pass in the arguments in a list.

I did attempt to get the stdin thing working without custom scripts but a lot of commands just don't accept arguments on the stdin. Yes, I agree that we could push people over to writing their own scripts, but I think this is a good compromise. Thoughts?

[balloob]

Alright. That seems ok. This would still require tests to be written.

I have one more thing that you could add is passing the service variables as template variables to the render method.

        rendered_args = template.render(hass, args, variables=call.data)

That way you can call a service and the service data can be used to render the command.

[balloob]

Looks good! 🐬