Merge "Mask passwords in exceptions and error messages"

oslo/concurrency/processutils.py

@@ -150,12 +150,12 @@ def execute(*cmd, **kwargs):
         cmd = shlex.split(root_helper) + list(cmd)
 
     cmd = map(str, cmd)
+    sanitized_cmd = strutils.mask_password(' '.join(cmd))
 
     while attempts > 0:
         attempts -= 1
         try:
-            LOG.log(loglevel, 'Running cmd (subprocess): %s',
+            LOG.log(loglevel, _('Running cmd (subprocess): %s'), sanitized_cmd)
-                    strutils.mask_password(' '.join(cmd)))
             _PIPE = subprocess.PIPE  # pylint: disable=E1101
 
             if os.name == 'nt':
@@ -192,16 +192,18 @@ def execute(*cmd, **kwargs):
             LOG.log(loglevel, 'Result was %s' % _returncode)
             if not ignore_exit_code and _returncode not in check_exit_code:
                 (stdout, stderr) = result
+                sanitized_stdout = strutils.mask_password(stdout)
+                sanitized_stderr = strutils.mask_password(stderr)
                 raise ProcessExecutionError(exit_code=_returncode,
-                                            stdout=stdout,
+                                            stdout=sanitized_stdout,
-                                            stderr=stderr,
+                                            stderr=sanitized_stderr,
-                                            cmd=' '.join(cmd))
+                                            cmd=sanitized_cmd)
             return result
         except ProcessExecutionError:
             if not attempts:
                 raise
             else:
-                LOG.log(loglevel, '%r failed. Retrying.', cmd)
+                LOG.log(loglevel, _('%r failed. Retrying.'), sanitized_cmd)
                 if delay_on_retry:
                     greenthread.sleep(random.randint(20, 200) / 100.0)
         finally:

tests/unit/test_processutils.py

@@ -24,9 +24,18 @@ import fixtures
 import mock
 from oslotest import base as test_base
 import six
+import stat
 
 from oslo.concurrency import processutils
 
+TEST_EXCEPTION_AND_MASKING_SCRIPT = """#!/bin/bash
+# This is to test stdout and stderr
+# and the command returned in an exception
+# when a non-zero exit code is returned
+echo onstdout --password='"secret"'
+echo onstderr --password='"secret"' 1>&2
+exit 38"""
+
 
 class UtilsTest(test_base.BaseTestCase):
     # NOTE(jkoelker) Moar tests from nova need to be ported. But they
@@ -217,6 +226,30 @@ grep foo
 
         self.assertIn('SUPER_UNIQUE_VAR=The answer is 42', out)
 
+    def test_exception_and_masking(self):
+        tmpfilename = self.create_tempfiles(
+            [["test_exceptions_and_masking",
+              TEST_EXCEPTION_AND_MASKING_SCRIPT]], ext='bash')[0]
+
+        os.chmod(tmpfilename, (stat.S_IRWXU |
+                               stat.S_IRGRP |
+                               stat.S_IXGRP |
+                               stat.S_IROTH |
+                               stat.S_IXOTH))
+
+        err = self.assertRaises(processutils.ProcessExecutionError,
+                                processutils.execute,
+                                tmpfilename, 'password="secret"',
+                                'something')
+
+        self.assertEqual(38, err.exit_code)
+        self.assertEqual(err.stdout, 'onstdout --password="***"\n')
+        self.assertEqual(err.stderr, 'onstderr --password="***"\n')
+        self.assertEqual(err.cmd, ' '.join([tmpfilename,
+                                            'password="***"',
+                                            'something']))
+        self.assertNotIn('secret', str(err))
+
 
 def fake_execute(*cmd, **kwargs):
     return 'stdout', 'stderr'