restore task arg splatting (#43798)

* restore task arg splatting

* reverts #41804
* supersedes #41295
* fixes #42192
* after lots of discussion amongst the core team, we decided to preserve this feature, clarify the runtime warnings/docs, and prioritize a path toward fixing the underlying behavior that causes this feature to be insecure (un-namespaced facts).

* update faq text

note that warning is disabled when inject_facts_as_vars is

* wordsmithing FAQ entry
This commit is contained in:
Matt Davis 2018-08-14 12:58:00 -07:00 committed by GitHub
parent 8a52f2f1de
commit 6b81c36533
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 38 additions and 2 deletions

View file

@ -446,7 +446,7 @@ In OpenBSD, a similar option is available in the base system called encrypt(1):
encrypt encrypt
.. _commercial_support: .. _dot_or_array_notation:
Ansible supports dot notation and array notation for variables. Which notation should I use? Ansible supports dot notation and array notation for variables. Which notation should I use?
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
@ -464,6 +464,38 @@ safer to use the array notation for variables.
Also array notation allows for dynamic variable composition, see dynamic_variables_. Also array notation allows for dynamic variable composition, see dynamic_variables_.
.. _argsplat_unsafe:
When is it unsafe to bulk-set task arguments from a variable?
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
You can set all of a task's arguments from a dictionary-typed variable. This
technique can be useful in some dynamic execution scenarios. However, it
introduces a security risk. We do not recommend it, so Ansible issues a
warning when you do something like this::
#...
vars:
usermod_args:
name: testuser
state: present
update_password: always
tasks:
- user: '{{ usermod_args }}'
This particular example is safe. However, constructing tasks like this is
risky because the parameters and values passed to ``usermod_args`` could
be overwritten by malicious values in the ``host facts`` on a compromised
target machine. To mitigate this risk:
* set bulk variables at a level of precedence greater than ``host facts`` in the order of precedence found in :ref:`ansible_variable_precedence` (the example above is safe because play vars take precedence over facts)
* disable the :ref:`inject_facts_as_vars` configuration setting to prevent fact values from colliding with variables (this will also disable the original warning)
.. _commercial_support:
Can I get training on Ansible? Can I get training on Ansible?
++++++++++++++++++++++++++++++ ++++++++++++++++++++++++++++++

View file

@ -519,7 +519,11 @@ class TaskExecutor:
if '_variable_params' in self._task.args: if '_variable_params' in self._task.args:
variable_params = self._task.args.pop('_variable_params') variable_params = self._task.args.pop('_variable_params')
if isinstance(variable_params, dict): if isinstance(variable_params, dict):
raise AnsibleError("Using a variable for a task's 'args' is not allowed as it is unsafe, facts can come from untrusted sources.") if C.INJECT_FACTS_AS_VARS:
display.warning("Using a variable for a task's 'args' is unsafe in some situations "
"(see https://docs.ansible.com/ansible/devel/reference_appendices/faq.html#argsplat-unsafe)")
variable_params.update(self._task.args)
self._task.args = variable_params
# get the connection and the handler for this execution # get the connection and the handler for this execution
if (not self._connection or if (not self._connection or