AC-132. Mask passwords in project update args and stdout.

awx/main/tasks.py

@@ -44,10 +44,14 @@ class BaseTask(Task):
         '''
         Reload model from database and update the given fields.
         '''
+        output_replacements = updates.pop('output_replacements', None) or []
         instance = self.model.objects.get(pk=pk)
         if updates:
             update_fields = []
             for field, value in updates.items():
+                if field in ('result_stdout', 'result_traceback'):
+                    for srch, repl in output_replacements:
+                        value = value.replace(srch, repl)
                 setattr(instance, field, value)
                 update_fields.append(field)
                 if field == 'status':
@@ -114,9 +118,15 @@ class BaseTask(Task):
     def build_args(self, instance, **kwargs):
         raise NotImplementedError
 
+    def build_safe_args(self, instance, **kwargs):
+        return self.build_args(instance, **kwargs)
+
     def build_cwd(self, instance, **kwargs):
         raise NotImplementedError
 
+    def build_output_replacements(self, instance, **kwargs):
+        return []
+
     def get_password_prompts(self):
         '''
         Return a dictionary of prompt regular expressions and password lookup
@@ -127,7 +137,8 @@ class BaseTask(Task):
             r'Bad passphrase, try again for .*:': '',
         }
 
-    def run_pexpect(self, instance, args, cwd, env, passwords):
+    def run_pexpect(self, instance, args, cwd, env, passwords,
+                    output_replacements=None):
         '''
         Run the given command using pexpect to capture output and provide
         passwords when requested.
@@ -149,7 +160,8 @@ class BaseTask(Task):
             result_id = child.expect(expect_list, timeout=5)
             if result_id in expect_passwords:
                 child.sendline(expect_passwords[result_id])
-            updates = {'status': 'running'}
+            updates = {'status': 'running',
+                       'output_replacements': output_replacements}
             if logfile_pos != logfile.tell():
                 logfile_pos = logfile.tell()
                 updates['result_stdout'] = logfile.getvalue()
@@ -193,9 +205,11 @@ class BaseTask(Task):
             kwargs['ssh_key_path'] = self.build_ssh_key_path(instance, **kwargs)
             kwargs['passwords'] = self.build_passwords(instance, **kwargs)
             args = self.build_args(instance, **kwargs)
+            safe_args = self.build_safe_args(instance, **kwargs)
+            output_replacements = self.build_output_replacements(instance, **kwargs)
             cwd = self.build_cwd(instance, **kwargs)
             env = self.build_env(instance, **kwargs)
-            instance = self.update_model(pk, job_args=json.dumps(args),
+            instance = self.update_model(pk, job_args=json.dumps(safe_args),
                                          job_cwd=cwd, job_env=env)
             status, stdout = self.run_pexpect(instance, args, cwd, env,
                                               kwargs['passwords'])
@@ -208,7 +222,8 @@ class BaseTask(Task):
                 except IOError:
                     pass
         instance = self.update_model(pk, status=status, result_stdout=stdout,
-                                     result_traceback=tb)
+                                     result_traceback=tb,
+                                     output_replacements=output_replacements)
 
 class RunJob(BaseTask):
     '''
@@ -422,13 +437,11 @@ class RunProjectUpdate(BaseTask):
         return urlparse.urlunsplit([parts.scheme, netloc, parts.path,
                                     parts.query, parts.fragment])
 
-    def build_args(self, project_update, **kwargs):
+    def _build_scm_url_extra_vars(self, project_update, **kwargs):
         '''
-        Build command line argument list for running ansible-playbook,
+        Helper method to build SCM url and extra vars with parameters needed
-        optionally using ssh-agent for public/private key authentication.
+        for authentication.
         '''
-        args = ['ansible-playbook', '-i', 'localhost,']
-        args.append('-%s' % ('v' * 3))
         extra_vars = {}
         project = project_update.project
         scm_url = project.scm_url
@@ -446,7 +459,18 @@ class RunProjectUpdate(BaseTask):
                 extra_vars['scm_username'] = scm_username
             else:  
                 scm_url = self.update_url_auth(scm_url, scm_username)
-        # FIXME: Need to hide password in saved job_args and result_stdout!
+        return scm_url, extra_vars
+
+    def build_args(self, project_update, **kwargs):
+        '''
+        Build command line argument list for running ansible-playbook,
+        optionally using ssh-agent for public/private key authentication.
+        '''
+        args = ['ansible-playbook', '-i', 'localhost,']
+        args.append('-%s' % ('v' * 3))
+        project = project_update.project
+        scm_url, extra_vars = self._build_scm_url_extra_vars(project_update,
+                                                             **kwargs)
         scm_branch = project.scm_branch or {'hg': 'tip'}.get(project.scm_type, 'HEAD')
         scm_delete_on_update = project.scm_delete_on_update or project.scm_delete_on_next_update
         extra_vars.update({
@@ -467,9 +491,53 @@ class RunProjectUpdate(BaseTask):
             args = ['ssh-agent', 'sh', '-c', cmd]
         return args
 
+    def build_safe_args(self, project_update, **kwargs):
+        pwdict = dict(kwargs.get('passwords', {}).items())
+        for pw_name, pw_val in pwdict.items():
+            if pw_name in ('', 'yes', 'no', 'scm_username'):
+                continue
+            pwdict[pw_name] = '*'*len(pw_val)
+        kwargs['passwords'] = pwdict
+        return self.build_args(project_update, **kwargs)
+
     def build_cwd(self, project_update, **kwargs):
         return self.get_path_to('..', 'playbooks')
 
+    def build_output_replacements(self, project_update, **kwargs):
+        '''
+        Return search/replace strings to prevent output URLs from showing
+        sensitive passwords.
+        '''
+        output_replacements = []
+        before_url = self._build_scm_url_extra_vars(project_update,
+                                                    **kwargs)[0]
+        scm_password = kwargs.get('passwords', {}).get('scm_password', '')
+        pwdict = dict(kwargs.get('passwords', {}).items())
+        for pw_name, pw_val in pwdict.items():
+            if pw_name in ('', 'yes', 'no', 'scm_username'):
+                continue
+            pwdict[pw_name] = '*'*len(pw_val)
+        kwargs['passwords'] = pwdict
+        after_url = self._build_scm_url_extra_vars(project_update,
+                                                   **kwargs)[0]
+        if after_url != before_url:
+            output_replacements.append((before_url, after_url))
+        project = project_update.project
+        if project.scm_type == 'svn' and project.scm_username and scm_password:
+            d_before = {
+                'username': project.scm_username,
+                'password': scm_password,
+            }
+            d_after = {
+                'username': project.scm_username,
+                'password': '*'*len(scm_password),
+            }
+            pattern1 = "username=\"%(username)s\" password=\"%(password)s\""
+            pattern2 = "--username '%(username)s' --password '%(password)s'"
+            output_replacements.append((pattern1 % d_before, pattern1 % d_after))
+            output_replacements.append((pattern2 % d_before, pattern2 % d_after))
+        return output_replacements
+
     def get_password_prompts(self):
         d = super(RunProjectUpdate, self).get_password_prompts()
         d.update({

awx/main/tests/projects.py

@@ -21,6 +21,7 @@ from django.utils.timezone import now
 from awx.main.models import *
 from awx.main.tests.base import BaseTest, BaseTransactionTest
 from awx.main.tests.tasks import TEST_SSH_KEY_DATA_LOCKED, TEST_SSH_KEY_DATA_UNLOCK
+from awx.main.utils import decrypt_field
 
 TEST_PLAYBOOK = '''- hosts: mygroup
   gather_facts: false
@@ -631,6 +632,8 @@ class ProjectUpdatesTest(BaseTransactionTest):
         return project
 
     def check_project_update(self, project, should_fail=False, **kwargs):
+        pu = kwargs.pop('project_update', None)
+        if not pu:
             pu = project.update(**kwargs)
             self.assertTrue(pu)
         pu = ProjectUpdate.objects.get(pk=pu.pk)
@@ -638,6 +641,26 @@ class ProjectUpdatesTest(BaseTransactionTest):
             self.assertEqual(pu.status, 'failed', pu.result_stdout)
         else:
             self.assertEqual(pu.status, 'successful', pu.result_stdout)
+        scm_password = kwargs.get('scm_password',
+                                  decrypt_field(project, 'scm_password'))
+        if scm_password not in ('', 'ASK'):
+            self.assertFalse(scm_password in pu.job_args, pu.job_args)
+            self.assertFalse(scm_password in json.dumps(pu.job_env),
+                             json.dumps(pu.job_env))
+            self.assertFalse(scm_password in pu.result_stdout,
+                             pu.result_stdout)
+            self.assertFalse(scm_password in pu.result_traceback,
+                             pu.result_traceback)
+        scm_key_unlock = kwargs.get('scm_key_unlock',
+                                    decrypt_field(project, 'scm_key_unlock'))
+        if scm_key_unlock not in ('', 'ASK'):
+            self.assertFalse(scm_key_unlock in pu.job_args, pu.job_args)
+            self.assertFalse(scm_key_unlock in json.dumps(pu.job_env),
+                             json.dumps(pu.job_env))
+            self.assertFalse(scm_key_unlock in pu.result_stdout,
+                             pu.result_stdout)
+            self.assertFalse(scm_key_unlock in pu.result_traceback,
+                             pu.result_traceback)
         project = Project.objects.get(pk=project.pk)
         self.assertEqual(project.last_update, pu)
         self.assertEqual(project.last_update_failed, pu.failed)
@@ -737,7 +760,7 @@ class ProjectUpdatesTest(BaseTransactionTest):
             self.check_project_update(project, should_fail=True)
             # Try invalid username.
             project = Project.objects.get(pk=project.pk)
-            project.scm_username = 'notavalidusername'
+            project.scm_username = 'not a\\ valid\' user" name'
             project.save()
             self.check_project_update(project, should_fail=True)
             # Clear username and password.
@@ -753,7 +776,7 @@ class ProjectUpdatesTest(BaseTransactionTest):
             self.check_project_update(project, should_fail=True)
             # Set username, with invalid password.
             project = Project.objects.get(pk=project.pk)
-            project.scm_password = 'notavalidpassword'
+            project.scm_password = 'not a\\ valid\' "password'
             project.save()
             self.check_project_update(project, should_fail=True)
 
@@ -885,7 +908,11 @@ class ProjectUpdatesTest(BaseTransactionTest):
             response = self.post(url, {}, expect=400)
         self.assertTrue('scm_password' in response['passwords_needed_to_update'])
         with self.current_user(self.super_django_user):
-            response = self.post(url, {'scm_password': 'blah'}, expect=202)
+            response = self.post(url, {'scm_password': 'blah1234'}, expect=202)
+        project_update = project.project_updates.order_by('-pk')[0]
+        self.check_project_update(project, should_fail=False,
+                                  scm_password='blah1234',
+                                  project_update=project_update)
 
     def test_prompt_for_scm_key_unlock_on_update(self):
         scm_url = 'git@github.com:ansible/ansible.github.com.git'
@@ -906,6 +933,10 @@ class ProjectUpdatesTest(BaseTransactionTest):
         self.assertTrue('scm_key_unlock' in response['passwords_needed_to_update'])
         with self.current_user(self.super_django_user):
             response = self.post(url, {'scm_key_unlock': TEST_SSH_KEY_DATA_UNLOCK}, expect=202)
+        project_update = project.project_updates.order_by('-pk')[0]
+        self.check_project_update(project, should_fail=False,
+                                  scm_key_unlock=TEST_SSH_KEY_DATA_UNLOCK,
+                                  project_update=project_update)
 
     def create_test_job_template(self, **kwargs):
         opts = {

awx/playbooks/project_update.yml

@@ -16,22 +16,21 @@
   tasks:
 
     - name: delete project directory before update
-      file: path={{project_path}} state=absent
+      file: path="{{project_path}}" state=absent
       when: scm_delete_on_update|default('')
 
     - name: update project using git
-      git: dest={{project_path}} repo={{scm_url}} version={{scm_branch}} force={{scm_clean}}
+      git: dest="{{project_path}}" repo="{{scm_url}}" version="{{scm_branch}}" force={{scm_clean}}
       when: scm_type == 'git'
 
     - name: update project using hg
-      hg: dest={{project_path}} repo={{scm_url}} revision={{scm_branch}} force={{scm_clean}}
+      hg: dest="{{project_path}}" repo="{{scm_url}}" revision="{{scm_branch}}" force={{scm_clean}}
       when: scm_type == 'hg'
 
     - name: update project using svn
-      subversion: dest={{project_path}} repo={{scm_url}} revision={{scm_branch}} force={{scm_clean}}
+      subversion: dest="{{project_path}}" repo="{{scm_url}}" revision="{{scm_branch}}" force={{scm_clean}}
       when: scm_type == 'svn' and not scm_username|default('')
 
     - name: update project using svn with auth
-      subversion: dest={{project_path}} repo={{scm_url}} revision={{scm_branch}} force={{scm_clean}} username={{scm_username}} password={{scm_password}}
+      subversion: dest="{{project_path}}" repo="{{scm_url}}" revision="{{scm_branch}}" force={{scm_clean}} username="{{scm_username}}" password="{{scm_password}}"
       when: scm_type == 'svn' and scm_username|default('')
-