diff --git a/oslo/concurrency/processutils.py b/oslo/concurrency/processutils.py index a521e4e8..d95231b0 100644 --- a/oslo/concurrency/processutils.py +++ b/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', - strutils.mask_password(' '.join(cmd))) + LOG.log(loglevel, _('Running cmd (subprocess): %s'), sanitized_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, - stderr=stderr, - cmd=' '.join(cmd)) + stdout=sanitized_stdout, + stderr=sanitized_stderr, + 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: diff --git a/tests/unit/test_processutils.py b/tests/unit/test_processutils.py index 4a6942a2..51460205 100644 --- a/tests/unit/test_processutils.py +++ b/tests/unit/test_processutils.py @@ -24,9 +24,18 @@ 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 @@ def test_with_env_variables(self): 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'