Merge pull request #1397 from ryanpetrello/unsafe-extra-vars-324

mark launch-time extra vars as !unsafe and sanitize Jinja from certain CLI flags

awx/main/conf.py

@@ -132,6 +132,27 @@ register(
     required=False,
 )
 
+register(
+    'ALLOW_JINJA_IN_EXTRA_VARS',
+    field_class=fields.ChoiceField,
+    choices=[
+        ('always', _('Always')),
+        ('never', _('Never')),
+        ('template', _('Only On Job Template Definitions')),
+    ],
+    required=True,
+    label=_('When can extra variables contain Jinja templates?'),
+    help_text=_(
+        'Ansible allows variable substitution via the Jinja2 templating '
+        'language for --extra-vars. This poses a potential security '
+        'risk where Tower users with the ability to specify extra vars at job '
+        'launch time can use Jinja2 templates to run arbitrary Python.  It is '
+        'recommended that this value be set to "template" or "never".'
+    ),
+    category=_('Jobs'),
+    category_slug='jobs',
+)
+
 register(
     'AWX_PROOT_ENABLED',
     field_class=fields.BooleanField,

awx/main/models/credential.py

@@ -2,7 +2,6 @@
 # All Rights Reserved.
 from collections import OrderedDict
 import functools
-import json
 import logging
 import operator
 import os
@@ -25,6 +24,7 @@ from awx.main.fields import (ImplicitRoleField, CredentialInputField,
                              CredentialTypeInputField,
                              CredentialTypeInjectorField)
 from awx.main.utils import decrypt_field
+from awx.main.utils.safe_yaml import safe_dump
 from awx.main.validators import validate_ssh_private_key
 from awx.main.models.base import * # noqa
 from awx.main.models.mixins import ResourceMixin
@@ -581,16 +581,21 @@ class CredentialType(CommonModelNameNotUnique):
             safe_env[env_var] = Template(tmpl).render(**safe_namespace)
 
         extra_vars = {}
-        safe_extra_vars = {}
         for var_name, tmpl in self.injectors.get('extra_vars', {}).items():
             extra_vars[var_name] = Template(tmpl).render(**namespace)
-            safe_extra_vars[var_name] = Template(tmpl).render(**safe_namespace)
 
+        def build_extra_vars_file(vars, private_dir):
+            handle, path = tempfile.mkstemp(dir = private_dir)
+            f = os.fdopen(handle, 'w')
+            f.write(safe_dump(vars))
+            f.close()
+            os.chmod(path, stat.S_IRUSR)
+            return path
+
+        path = build_extra_vars_file(extra_vars, private_data_dir)
         if extra_vars:
-            args.extend(['-e', json.dumps(extra_vars)])
-
-        if safe_extra_vars:
-            safe_args.extend(['-e', json.dumps(safe_extra_vars)])
+            args.extend(['-e', '@%s' % path])
+            safe_args.extend(['-e', '@%s' % path])
 
 
 @CredentialType.default

awx/main/tasks.py

@@ -58,6 +58,7 @@ from awx.main.utils import (get_ansible_version, get_ssh_version, decrypt_field,
                             wrap_args_with_proot, get_system_task_capacity, OutputEventFilter,
                             parse_yaml_or_json, ignore_inventory_computed_fields, ignore_inventory_group_removal,
                             get_type_for_model, extract_ansible_vars)
+from awx.main.utils.safe_yaml import safe_dump, sanitize_jinja
 from awx.main.utils.reload import restart_local_services, stop_local_services
 from awx.main.utils.handlers import configure_external_logger
 from awx.main.consumers import emit_channel_notification
@@ -622,6 +623,17 @@ class BaseTask(LogErrorsTask):
             '': '',
         }
 
+    def build_extra_vars_file(self, vars, **kwargs):
+        handle, path = tempfile.mkstemp(dir=kwargs.get('private_data_dir', None))
+        f = os.fdopen(handle, 'w')
+        if settings.ALLOW_JINJA_IN_EXTRA_VARS == 'always':
+            f.write(yaml.safe_dump(vars))
+        else:
+            f.write(safe_dump(vars, kwargs.get('safe_dict', {}) or None))
+        f.close()
+        os.chmod(path, stat.S_IRUSR)
+        return path
+
     def add_ansible_venv(self, env, add_awx_lib=True):
         env['VIRTUAL_ENV'] = settings.ANSIBLE_VENV_PATH
         env['PATH'] = os.path.join(settings.ANSIBLE_VENV_PATH, "bin") + ":" + env['PATH']
@@ -900,8 +912,7 @@ class BaseTask(LogErrorsTask):
         except Exception:
             if status != 'canceled':
                 tb = traceback.format_exc()
-                if settings.DEBUG:
-                    logger.exception('%s Exception occurred while running task', instance.log_format)
+                logger.exception('%s Exception occurred while running task', instance.log_format)
         finally:
             try:
                 stdout_handle.flush()
@@ -1142,7 +1153,7 @@ class RunJob(BaseTask):
         args = ['ansible-playbook', '-i', self.build_inventory(job, **kwargs)]
         if job.job_type == 'check':
             args.append('--check')
-        args.extend(['-u', ssh_username])
+        args.extend(['-u', sanitize_jinja(ssh_username)])
         if 'ssh_password' in kwargs.get('passwords', {}):
             args.append('--ask-pass')
         if job.become_enabled:
@@ -1150,9 +1161,9 @@ class RunJob(BaseTask):
         if job.diff_mode:
             args.append('--diff')
         if become_method:
-            args.extend(['--become-method', become_method])
+            args.extend(['--become-method', sanitize_jinja(become_method)])
         if become_username:
-            args.extend(['--become-user', become_username])
+            args.extend(['--become-user', sanitize_jinja(become_username)])
         if 'become_password' in kwargs.get('passwords', {}):
             args.append('--ask-become-pass')
         # Support prompting for a vault password.
@@ -1205,7 +1216,21 @@ class RunJob(BaseTask):
                 extra_vars.update(json.loads(job.display_extra_vars()))
             else:
                 extra_vars.update(json.loads(job.decrypted_extra_vars()))
-        args.extend(['-e', json.dumps(extra_vars)])
+
+        # By default, all extra vars disallow Jinja2 template usage for
+        # security reasons; top level key-values defined in JT.extra_vars, however,
+        # are whitelisted as "safe" (because they can only be set by users with
+        # higher levels of privilege - those that have the ability create and
+        # edit Job Templates)
+        safe_dict = {}
+        if job.job_template and settings.ALLOW_JINJA_IN_EXTRA_VARS == 'template':
+            safe_dict = job.job_template.extra_vars_dict
+        extra_vars_path = self.build_extra_vars_file(
+            vars=extra_vars,
+            safe_dict=safe_dict,
+            **kwargs
+        )
+        args.extend(['-e', '@%s' % (extra_vars_path)])
 
         # Add path to playbook (relative to project.local_path).
         args.append(job.playbook)
@@ -1460,7 +1485,8 @@ class RunProjectUpdate(BaseTask):
             'scm_revision_output': self.revision_path,
             'scm_revision': project_update.project.scm_revision,
         })
-        args.extend(['-e', json.dumps(extra_vars)])
+        extra_vars_path = self.build_extra_vars_file(vars=extra_vars, **kwargs)
+        args.extend(['-e', '@%s' % (extra_vars_path)])
         args.append('project_update.yml')
         return args
 
@@ -2179,7 +2205,7 @@ class RunAdHocCommand(BaseTask):
         args = ['ansible', '-i', self.build_inventory(ad_hoc_command, **kwargs)]
         if ad_hoc_command.job_type == 'check':
             args.append('--check')
-        args.extend(['-u', ssh_username])
+        args.extend(['-u', sanitize_jinja(ssh_username)])
         if 'ssh_password' in kwargs.get('passwords', {}):
             args.append('--ask-pass')
         # We only specify sudo/su user and password if explicitly given by the
@@ -2187,9 +2213,9 @@ class RunAdHocCommand(BaseTask):
         if ad_hoc_command.become_enabled:
             args.append('--become')
         if become_method:
-            args.extend(['--become-method', become_method])
+            args.extend(['--become-method', sanitize_jinja(become_method)])
         if become_username:
-            args.extend(['--become-user', become_username])
+            args.extend(['--become-user', sanitize_jinja(become_username)])
         if 'become_password' in kwargs.get('passwords', {}):
             args.append('--ask-become-pass')
 
@@ -2220,10 +2246,11 @@ class RunAdHocCommand(BaseTask):
                     "{} are prohibited from use in ad hoc commands."
                 ).format(", ".join(removed_vars)))
             extra_vars.update(ad_hoc_command.extra_vars_dict)
-        args.extend(['-e', json.dumps(extra_vars)])
+        extra_vars_path = self.build_extra_vars_file(vars=extra_vars, **kwargs)
+        args.extend(['-e', '@%s' % (extra_vars_path)])
 
         args.extend(['-m', ad_hoc_command.module_name])
-        args.extend(['-a', ad_hoc_command.module_args])
+        args.extend(['-a', sanitize_jinja(ad_hoc_command.module_args)])
 
         if ad_hoc_command.limit:
             args.append(ad_hoc_command.limit)

awx/main/tests/unit/models/test_survey_models.py

@@ -1,6 +1,7 @@
 import tempfile
 import pytest
 import json
+import yaml
 
 from awx.main.utils.encryption import encrypt_value
 from awx.main.tasks import RunJob
@@ -9,6 +10,7 @@ from awx.main.models import (
     JobTemplate,
     WorkflowJobTemplate
 )
+from awx.main.utils.safe_yaml import SafeLoader
 
 ENCRYPTED_SECRET = encrypt_value('secret')
 
@@ -71,7 +73,9 @@ def test_job_safe_args_redacted_passwords(job):
     run_job = RunJob()
     safe_args = run_job.build_safe_args(job, **kwargs)
     ev_index = safe_args.index('-e') + 1
-    extra_vars = json.loads(safe_args[ev_index])
+    extra_var_file = open(safe_args[ev_index][1:], 'r')
+    extra_vars = yaml.load(extra_var_file, SafeLoader)
+    extra_var_file.close()
     assert extra_vars['secret_key'] == '$encrypted$'
 
 
@@ -80,7 +84,9 @@ def test_job_args_unredacted_passwords(job, tmpdir_factory):
     run_job = RunJob()
     args = run_job.build_args(job, **kwargs)
     ev_index = args.index('-e') + 1
-    extra_vars = json.loads(args[ev_index])
+    extra_var_file = open(args[ev_index][1:], 'r')
+    extra_vars = yaml.load(extra_var_file, SafeLoader)
+    extra_var_file.close()
     assert extra_vars['secret_key'] == 'my_password'
 
 

awx/main/tests/unit/test_tasks.py

@@ -23,6 +23,7 @@ from awx.main.models import (
     InventorySource,
     InventoryUpdate,
     Job,
+    JobTemplate,
     Notification,
     Project,
     ProjectUpdate,
@@ -32,7 +33,7 @@ from awx.main.models import (
 
 from awx.main import tasks
 from awx.main.utils import encrypt_field, encrypt_value
-
+from awx.main.utils.safe_yaml import SafeLoader
 
 
 @contextmanager
@@ -172,6 +173,15 @@ def pytest_generate_tests(metafunc):
             )
 
 
+def parse_extra_vars(args):
+    extra_vars = {}
+    for chunk in args:
+        if chunk.startswith('@/tmp/'):
+            with open(chunk.strip('@'), 'r') as f:
+                extra_vars.update(yaml.load(f, SafeLoader))
+    return extra_vars
+
+
 class TestJobExecution:
     """
     For job runs, test that `ansible-playbook` is invoked with the proper
@@ -234,7 +244,8 @@ class TestJobExecution:
             cancel_flag=False,
             project=Project(),
             playbook='helloworld.yml',
-            verbosity=3
+            verbosity=3,
+            job_template=JobTemplate(extra_vars='')
         )
 
         # mock the job.extra_credentials M2M relation so we can avoid DB access
@@ -254,6 +265,131 @@ class TestJobExecution:
         return self.instance.pk
 
 
+class TestExtraVarSanitation(TestJobExecution):
+    # By default, extra vars are marked as `!unsafe` in the generated yaml
+    # _unless_ they've been specified on the JobTemplate's extra_vars (which
+    # are deemed trustable, because they can only be added by users w/ enough
+    # privilege to add/modify a Job Template)
+
+    UNSAFE = '{{ lookup(''pipe'',''ls -la'') }}'
+
+    def test_vars_unsafe_by_default(self):
+        self.instance.created_by = User(pk=123, username='angry-spud')
+
+        def run_pexpect_side_effect(*args, **kwargs):
+            args, cwd, env, stdout = args
+            extra_vars = parse_extra_vars(args)
+
+            # ensure that strings are marked as unsafe
+            for unsafe in ['awx_job_template_name', 'tower_job_template_name',
+                           'awx_user_name', 'tower_job_launch_type',
+                           'awx_project_revision',
+                           'tower_project_revision', 'tower_user_name',
+                           'awx_job_launch_type']:
+                assert hasattr(extra_vars[unsafe], '__UNSAFE__')
+
+            # ensure that non-strings are marked as safe
+            for safe in ['awx_job_template_id', 'awx_job_id', 'awx_user_id',
+                         'tower_user_id', 'tower_job_template_id',
+                         'tower_job_id']:
+                assert not hasattr(extra_vars[safe], '__UNSAFE__')
+            return ['successful', 0]
+
+        self.run_pexpect.side_effect = run_pexpect_side_effect
+        self.task.run(self.pk)
+
+    def test_launchtime_vars_unsafe(self):
+        self.instance.extra_vars = json.dumps({'msg': self.UNSAFE})
+
+        def run_pexpect_side_effect(*args, **kwargs):
+            args, cwd, env, stdout = args
+            extra_vars = parse_extra_vars(args)
+            assert extra_vars['msg'] == self.UNSAFE
+            assert hasattr(extra_vars['msg'], '__UNSAFE__')
+            return ['successful', 0]
+
+        self.run_pexpect.side_effect = run_pexpect_side_effect
+        self.task.run(self.pk)
+
+    def test_nested_launchtime_vars_unsafe(self):
+        self.instance.extra_vars = json.dumps({'msg': {'a': [self.UNSAFE]}})
+
+        def run_pexpect_side_effect(*args, **kwargs):
+            args, cwd, env, stdout = args
+            extra_vars = parse_extra_vars(args)
+            assert extra_vars['msg'] == {'a': [self.UNSAFE]}
+            assert hasattr(extra_vars['msg']['a'][0], '__UNSAFE__')
+            return ['successful', 0]
+
+        self.run_pexpect.side_effect = run_pexpect_side_effect
+        self.task.run(self.pk)
+
+    def test_whitelisted_jt_extra_vars(self):
+        self.instance.job_template.extra_vars = self.instance.extra_vars = json.dumps({'msg': self.UNSAFE})
+
+        def run_pexpect_side_effect(*args, **kwargs):
+            args, cwd, env, stdout = args
+            extra_vars = parse_extra_vars(args)
+            assert extra_vars['msg'] == self.UNSAFE
+            assert not hasattr(extra_vars['msg'], '__UNSAFE__')
+            return ['successful', 0]
+
+        self.run_pexpect.side_effect = run_pexpect_side_effect
+        self.task.run(self.pk)
+
+    def test_nested_whitelisted_vars(self):
+        self.instance.extra_vars = json.dumps({'msg': {'a': {'b': [self.UNSAFE]}}})
+        self.instance.job_template.extra_vars = self.instance.extra_vars
+
+        def run_pexpect_side_effect(*args, **kwargs):
+            args, cwd, env, stdout = args
+            extra_vars = parse_extra_vars(args)
+            assert extra_vars['msg'] == {'a': {'b': [self.UNSAFE]}}
+            assert not hasattr(extra_vars['msg']['a']['b'][0], '__UNSAFE__')
+            return ['successful', 0]
+
+        self.run_pexpect.side_effect = run_pexpect_side_effect
+        self.task.run(self.pk)
+
+    def test_sensitive_values_dont_leak(self):
+        # JT defines `msg=SENSITIVE`, the job *should not* be able to do
+        # `other_var=SENSITIVE`
+        self.instance.job_template.extra_vars = json.dumps({'msg': self.UNSAFE})
+        self.instance.extra_vars = json.dumps({
+            'msg': 'other-value',
+            'other_var': self.UNSAFE
+        })
+
+        def run_pexpect_side_effect(*args, **kwargs):
+            args, cwd, env, stdout = args
+            extra_vars = parse_extra_vars(args)
+
+            assert extra_vars['msg'] == 'other-value'
+            assert hasattr(extra_vars['msg'], '__UNSAFE__')
+
+            assert extra_vars['other_var'] == self.UNSAFE
+            assert hasattr(extra_vars['other_var'], '__UNSAFE__')
+
+            return ['successful', 0]
+
+        self.run_pexpect.side_effect = run_pexpect_side_effect
+        self.task.run(self.pk)
+
+    def test_overwritten_jt_extra_vars(self):
+        self.instance.job_template.extra_vars = json.dumps({'msg': 'SAFE'})
+        self.instance.extra_vars = json.dumps({'msg': self.UNSAFE})
+
+        def run_pexpect_side_effect(*args, **kwargs):
+            args, cwd, env, stdout = args
+            extra_vars = parse_extra_vars(args)
+            assert extra_vars['msg'] == self.UNSAFE
+            assert hasattr(extra_vars['msg'], '__UNSAFE__')
+            return ['successful', 0]
+
+        self.run_pexpect.side_effect = run_pexpect_side_effect
+        self.task.run(self.pk)
+
+
 class TestGenericRun(TestJobExecution):
 
     def test_cancel_flag(self):
@@ -296,15 +432,18 @@ class TestGenericRun(TestJobExecution):
 
     def test_created_by_extra_vars(self):
         self.instance.created_by = User(pk=123, username='angry-spud')
-        self.task.run(self.pk)
 
-        assert self.run_pexpect.call_count == 1
-        call_args, _ = self.run_pexpect.call_args_list[0]
-        args, cwd, env, stdout = call_args
-        assert '"tower_user_id": 123,' in ' '.join(args)
-        assert '"tower_user_name": "angry-spud"' in ' '.join(args)
-        assert '"awx_user_id": 123,' in ' '.join(args)
-        assert '"awx_user_name": "angry-spud"' in ' '.join(args)
+        def run_pexpect_side_effect(*args, **kwargs):
+            args, cwd, env, stdout = args
+            extra_vars = parse_extra_vars(args)
+            assert extra_vars['tower_user_id'] == 123
+            assert extra_vars['tower_user_name'] == "angry-spud"
+            assert extra_vars['awx_user_id'] == 123
+            assert extra_vars['awx_user_name'] == "angry-spud"
+            return ['successful', 0]
+
+        self.run_pexpect.side_effect = run_pexpect_side_effect
+        self.task.run(self.pk)
 
     def test_survey_extra_vars(self):
         self.instance.extra_vars = json.dumps({
@@ -313,12 +452,15 @@ class TestGenericRun(TestJobExecution):
         self.instance.survey_passwords = {
             'super_secret': '$encrypted$'
         }
-        self.task.run(self.pk)
 
-        assert self.run_pexpect.call_count == 1
-        call_args, _ = self.run_pexpect.call_args_list[0]
-        args, cwd, env, stdout = call_args
-        assert '"super_secret": "CLASSIFIED"' in ' '.join(args)
+        def run_pexpect_side_effect(*args, **kwargs):
+            args, cwd, env, stdout = args
+            extra_vars = parse_extra_vars(args)
+            assert extra_vars['super_secret'] == "CLASSIFIED"
+            return ['successful', 0]
+
+        self.run_pexpect.side_effect = run_pexpect_side_effect
+        self.task.run(self.pk)
 
     def test_awx_task_env(self):
         patch = mock.patch('awx.main.tasks.settings.AWX_TASK_ENV', {'FOO': 'BAR'})
@@ -383,18 +525,28 @@ class TestAdhocRun(TestJobExecution):
             extra_vars={'awx_foo': 'awx-bar'}
         )
 
+    def test_options_jinja_usage(self):
+        self.instance.module_args = '{{ ansible_ssh_pass }}'
+        with pytest.raises(Exception):
+            self.task.run(self.pk)
+        update_model_call = self.task.update_model.call_args[1]
+        assert 'Jinja variables are not allowed' in update_model_call['result_traceback']
+
     def test_created_by_extra_vars(self):
         self.instance.created_by = User(pk=123, username='angry-spud')
-        self.task.run(self.pk)
 
-        assert self.run_pexpect.call_count == 1
-        call_args, _ = self.run_pexpect.call_args_list[0]
-        args, cwd, env, stdout = call_args
-        assert '"tower_user_id": 123,' in ' '.join(args)
-        assert '"tower_user_name": "angry-spud"' in ' '.join(args)
-        assert '"awx_user_id": 123,' in ' '.join(args)
-        assert '"awx_user_name": "angry-spud"' in ' '.join(args)
-        assert '"awx_foo": "awx-bar' in ' '.join(args)
+        def run_pexpect_side_effect(*args, **kwargs):
+            args, cwd, env, stdout = args
+            extra_vars = parse_extra_vars(args)
+            assert extra_vars['tower_user_id'] == 123
+            assert extra_vars['tower_user_name'] == "angry-spud"
+            assert extra_vars['awx_user_id'] == 123
+            assert extra_vars['awx_user_name'] == "angry-spud"
+            assert extra_vars['awx_foo'] == "awx-bar"
+            return ['successful', 0]
+
+        self.run_pexpect.side_effect = run_pexpect_side_effect
+        self.task.run(self.pk)
 
 
 class TestIsolatedExecution(TestJobExecution):
@@ -502,6 +654,33 @@ class TestJobCredentials(TestJobExecution):
         ]
     }
 
+    def test_username_jinja_usage(self):
+        ssh = CredentialType.defaults['ssh']()
+        credential = Credential(
+            pk=1,
+            credential_type=ssh,
+            inputs = {'username': '{{ ansible_ssh_pass }}'}
+        )
+        self.instance.credential = credential
+        with pytest.raises(Exception):
+            self.task.run(self.pk)
+        update_model_call = self.task.update_model.call_args[1]
+        assert 'Jinja variables are not allowed' in update_model_call['result_traceback']
+
+    @pytest.mark.parametrize("flag", ['become_username', 'become_method'])
+    def test_become_jinja_usage(self, flag):
+        ssh = CredentialType.defaults['ssh']()
+        credential = Credential(
+            pk=1,
+            credential_type=ssh,
+            inputs = {'username': 'joe', flag: '{{ ansible_ssh_pass }}'}
+        )
+        self.instance.credential = credential
+        with pytest.raises(Exception):
+            self.task.run(self.pk)
+        update_model_call = self.task.update_model.call_args[1]
+        assert 'Jinja variables are not allowed' in update_model_call['result_traceback']
+
     def test_ssh_passwords(self, field, password_name, expected_flag):
         ssh = CredentialType.defaults['ssh']()
         credential = Credential(
@@ -986,14 +1165,17 @@ class TestJobCredentials(TestJobExecution):
             inputs = {'api_token': 'ABC123'}
         )
         self.instance.extra_credentials.add(credential)
+
+        def run_pexpect_side_effect(*args, **kwargs):
+            args, cwd, env, stdout = args
+            extra_vars = parse_extra_vars(args)
+            assert extra_vars["api_token"] == "ABC123"
+            assert hasattr(extra_vars["api_token"], '__UNSAFE__')
+            return ['successful', 0]
+
+        self.run_pexpect.side_effect = run_pexpect_side_effect
         self.task.run(self.pk)
 
-        assert self.run_pexpect.call_count == 1
-        call_args, _ = self.run_pexpect.call_args_list[0]
-        args, cwd, env, stdout = call_args
-
-        assert '-e {"api_token": "ABC123"}' in ' '.join(args)
-
     def test_custom_environment_injectors_with_boolean_extra_vars(self):
         some_cloud = CredentialType(
             kind='cloud',
@@ -1018,12 +1200,15 @@ class TestJobCredentials(TestJobExecution):
             inputs={'turbo_button': True}
         )
         self.instance.extra_credentials.add(credential)
-        self.task.run(self.pk)
 
-        assert self.run_pexpect.call_count == 1
-        call_args, _ = self.run_pexpect.call_args_list[0]
-        args, cwd, env, stdout = call_args
-        assert '-e {"turbo_button": "True"}' in ' '.join(args)
+        def run_pexpect_side_effect(*args, **kwargs):
+            args, cwd, env, stdout = args
+            extra_vars = parse_extra_vars(args)
+            assert extra_vars["turbo_button"] == "True"
+            return ['successful', 0]
+
+        self.run_pexpect.side_effect = run_pexpect_side_effect
+        self.task.run(self.pk)
 
     def test_custom_environment_injectors_with_complicated_boolean_template(self):
         some_cloud = CredentialType(
@@ -1049,12 +1234,15 @@ class TestJobCredentials(TestJobExecution):
             inputs={'turbo_button': True}
         )
         self.instance.extra_credentials.add(credential)
-        self.task.run(self.pk)
 
-        assert self.run_pexpect.call_count == 1
-        call_args, _ = self.run_pexpect.call_args_list[0]
-        args, cwd, env, stdout = call_args
-        assert '-e {"turbo_button": "FAST!"}' in ' '.join(args)
+        def run_pexpect_side_effect(*args, **kwargs):
+            args, cwd, env, stdout = args
+            extra_vars = parse_extra_vars(args)
+            assert extra_vars["turbo_button"] == "FAST!"
+            return ['successful', 0]
+
+        self.run_pexpect.side_effect = run_pexpect_side_effect
+        self.task.run(self.pk)
 
     def test_custom_environment_injectors_with_secret_extra_vars(self):
         """
@@ -1085,13 +1273,16 @@ class TestJobCredentials(TestJobExecution):
         )
         credential.inputs['password'] = encrypt_field(credential, 'password')
         self.instance.extra_credentials.add(credential)
+
+        def run_pexpect_side_effect(*args, **kwargs):
+            args, cwd, env, stdout = args
+            extra_vars = parse_extra_vars(args)
+            assert extra_vars["password"] == "SUPER-SECRET-123"
+            return ['successful', 0]
+
+        self.run_pexpect.side_effect = run_pexpect_side_effect
         self.task.run(self.pk)
 
-        assert self.run_pexpect.call_count == 1
-        call_args, _ = self.run_pexpect.call_args_list[0]
-        args, cwd, env, stdout = call_args
-
-        assert '-e {"password": "SUPER-SECRET-123"}' in ' '.join(args)
         assert 'SUPER-SECRET-123' not in json.dumps(self.task.update_model.call_args_list)
 
     def test_custom_environment_injectors_with_file(self):
@@ -1217,20 +1408,22 @@ class TestProjectUpdateCredentials(TestJobExecution):
             pk=1,
             credential_type=ssh,
         )
+
+        def run_pexpect_side_effect(*args, **kwargs):
+            args, cwd, env, stdout = args
+            extra_vars = parse_extra_vars(args)
+            assert ' '.join(args).startswith('bwrap')
+            assert ' '.join([
+                '--bind',
+                os.path.realpath(settings.PROJECTS_ROOT),
+                os.path.realpath(settings.PROJECTS_ROOT)
+            ]) in ' '.join(args)
+            assert extra_vars["scm_revision_output"].startswith(settings.PROJECTS_ROOT)
+            return ['successful', 0]
+
+        self.run_pexpect.side_effect = run_pexpect_side_effect
         self.task.run(self.pk)
 
-        assert self.run_pexpect.call_count == 1
-        call_args, call_kwargs = self.run_pexpect.call_args_list[0]
-        args, cwd, env, stdout = call_args
-
-        assert ' '.join(args).startswith('bwrap')
-        ' '.join([
-            '--bind',
-            settings.PROJECTS_ROOT,
-            settings.PROJECTS_ROOT,
-        ]) in ' '.join(args)
-        assert '"scm_revision_output": "/projects/tmp' in ' '.join(args)
-
     def test_username_and_password_auth(self, scm_type):
         ssh = CredentialType.defaults['ssh']()
         self.instance.scm_type = scm_type

awx/main/tests/unit/utils/test_safe_yaml.py

@@ -0,0 +1,85 @@
+# -*- coding: utf-8 -*-
+
+from copy import deepcopy
+import pytest
+import yaml
+from awx.main.utils.safe_yaml import safe_dump
+
+
+@pytest.mark.parametrize('value', [None, 1, 1.5, []])
+def test_native_types(value):
+    # Native non-string types should dump the same way that `yaml.safe_dump` does
+    assert safe_dump(value) == yaml.safe_dump(value)
+
+
+def test_empty():
+    assert safe_dump({}) == ''
+
+
+def test_raw_string():
+    assert safe_dump('foo') == "!unsafe 'foo'\n"
+
+
+def test_kv_int():
+    assert safe_dump({'a': 1}) == "!unsafe 'a': 1\n"
+
+
+def test_kv_float():
+    assert safe_dump({'a': 1.5}) == "!unsafe 'a': 1.5\n"
+
+
+def test_kv_unsafe():
+    assert safe_dump({'a': 'b'}) == "!unsafe 'a': !unsafe 'b'\n"
+
+
+def test_kv_unsafe_unicode():
+    assert safe_dump({'a': u'🐉'}) == '!unsafe \'a\': !unsafe "\\U0001F409"\n'
+
+
+def test_kv_unsafe_in_list():
+    assert safe_dump({'a': ['b']}) == "!unsafe 'a':\n- !unsafe 'b'\n"
+
+
+def test_kv_unsafe_in_mixed_list():
+    assert safe_dump({'a': [1, 'b']}) == "!unsafe 'a':\n- 1\n- !unsafe 'b'\n"
+
+
+def test_kv_unsafe_deep_nesting():
+    yaml = safe_dump({'a': [1, [{'b': {'c': [{'d': 'e'}]}}]]})
+    for x in ('a', 'b', 'c', 'd', 'e'):
+        assert "!unsafe '{}'".format(x) in yaml
+
+
+def test_kv_unsafe_multiple():
+    assert safe_dump({'a': 'b', 'c': 'd'}) == '\n'.join([
+        "!unsafe 'a': !unsafe 'b'",
+        "!unsafe 'c': !unsafe 'd'",
+        ""
+    ])
+
+
+def test_safe_marking():
+    assert safe_dump({'a': 'b'}, safe_dict={'a': 'b'}) == "a: b\n"
+
+
+def test_safe_marking_mixed():
+    assert safe_dump({'a': 'b', 'c': 'd'}, safe_dict={'a': 'b'}) == '\n'.join([
+        "a: b",
+        "!unsafe 'c': !unsafe 'd'",
+        ""
+    ])
+
+
+def test_safe_marking_deep_nesting():
+    deep = {'a': [1, [{'b': {'c': [{'d': 'e'}]}}]]}
+    yaml = safe_dump(deep, deepcopy(deep))
+    for x in ('a', 'b', 'c', 'd', 'e'):
+        assert "!unsafe '{}'".format(x) not in yaml
+
+
+def test_deep_diff_unsafe_marking():
+    deep = {'a': [1, [{'b': {'c': [{'d': 'e'}]}}]]}
+    jt_vars = deepcopy(deep)
+    deep['a'][1][0]['b']['z'] = 'not safe'
+    yaml = safe_dump(deep, jt_vars)
+    assert "!unsafe 'z'" in yaml

awx/main/utils/safe_yaml.py

@@ -0,0 +1,87 @@
+import re
+import six
+import yaml
+
+
+__all__ = ['safe_dump', 'SafeLoader']
+
+
+class SafeStringDumper(yaml.SafeDumper):
+
+    def represent_data(self, value):
+        if isinstance(value, six.string_types):
+            return self.represent_scalar('!unsafe', value)
+        return super(SafeStringDumper, self).represent_data(value)
+
+
+class SafeLoader(yaml.Loader):
+
+    def construct_yaml_unsafe(self, node):
+        class UnsafeText(six.text_type):
+            __UNSAFE__ = True
+        node = UnsafeText(self.construct_scalar(node))
+        return node
+
+
+SafeLoader.add_constructor(
+    u'!unsafe',
+    SafeLoader.construct_yaml_unsafe
+)
+
+
+def safe_dump(x, safe_dict=None):
+    """
+    Used to serialize an extra_vars dict to YAML
+
+    By default, extra vars are marked as `!unsafe` in the generated yaml
+    _unless_ they've been deemed "trusted" (meaning, they likely were set/added
+    by a user with a high level of privilege).
+
+    This function allows you to pass in a trusted `safe_dict` to whitelist 
+    certain extra vars so that they are _not_ marked as `!unsafe` in the
+    resulting YAML.  Anything _not_ in this dict will automatically be
+    `!unsafe`.
+
+    safe_dump({'a': 'b', 'c': 'd'}) -> 
+    !unsafe 'a': !unsafe 'b'
+    !unsafe 'c': !unsafe 'd'
+
+    safe_dump({'a': 'b', 'c': 'd'}, safe_dict={'a': 'b'})
+    a: b
+    !unsafe 'c': !unsafe 'd'
+    """
+    if isinstance(x, dict):
+        yamls = []
+        safe_dict = safe_dict or {}
+
+        # Compare the top level keys so that we can find values that have
+        # equality matches (and consider those branches safe)
+        for k, v in x.items():
+            dumper = yaml.SafeDumper
+            if safe_dict.get(k) != v:
+                dumper = SafeStringDumper
+            yamls.append(yaml.dump_all(
+                [{k: v}],
+                None,
+                Dumper=dumper,
+                default_flow_style=False,
+            ))
+        return ''.join(yamls)
+    else:
+        return yaml.dump_all([x], None, Dumper=SafeStringDumper, default_flow_style=False)
+
+
+def sanitize_jinja(arg):
+    """
+    For some string, prevent usage of Jinja-like flags
+    """
+    if isinstance(arg, six.string_types):
+        # If the argument looks like it contains Jinja expressions
+        # {{ x }} ...
+        if re.search('\{\{[^}]+}}', arg) is not None:
+            raise ValueError('Inline Jinja variables are not allowed.')
+        # If the argument looks like it contains Jinja statements/control flow...
+        # {% if x.foo() %} ...
+        if re.search('\{%[^%]+%}', arg) is not None:
+            raise ValueError('Inline Jinja variables are not allowed.')
+    return arg

awx/settings/defaults.py

@@ -585,6 +585,9 @@ CAPTURE_JOB_EVENT_HOSTS = False
 # Rebuild Host Smart Inventory memberships.
 AWX_REBUILD_SMART_MEMBERSHIP = False
 
+# By default, allow arbitrary Jinja templating in extra_vars defined on a Job Template
+ALLOW_JINJA_IN_EXTRA_VARS = 'template'
+
 # Enable bubblewrap support for running jobs (playbook runs only).
 # Note: This setting may be overridden by database settings.
 AWX_PROOT_ENABLED = True