Update the use of no_log values to cover everything that heuristic_log_sanitize does.

Fixes #12792

lib/ansible/module_utils/basic.py

@@ -364,7 +364,53 @@ def json_dict_bytes_to_unicode(d, encoding='utf-8'):
     else:
         return d
 
-def heuristic_log_sanitize(data):
+def return_values(obj):
+    """ Return stringified values from datastructures. For use with removing
+    sensitive values pre-jsonification."""
+    if isinstance(obj, basestring):
+        if obj:
+            yield obj
+        return
+    elif isinstance(obj, Sequence):
+        for element in obj:
+            for subelement in return_values(element):
+                yield subelement
+    elif isinstance(obj, Mapping):
+        for element in obj.items():
+            for subelement in return_values(element[1]):
+                yield subelement
+    elif isinstance(obj, (bool, NoneType)):
+        # This must come before int because bools are also ints
+        return
+    elif isinstance(obj, NUMBERTYPES):
+        yield str(obj)
+    else:
+        raise TypeError('Unknown parameter type: %s, %s' % (type(obj), obj))
+
+def remove_values(value, no_log_strings):
+    """ Remove strings in no_log_strings from value.  If value is a container
+    type, then remove a lot more"""
+    if isinstance(value, basestring):
+        if value in no_log_strings:
+            return 'VALUE_SPECIFIED_IN_NO_LOG_PARAMETER'
+        for omit_me in no_log_strings:
+            value = value.replace(omit_me, '*' * 8)
+    elif isinstance(value, Sequence):
+        return [remove_values(elem, no_log_strings) for elem in value]
+    elif isinstance(value, Mapping):
+        return dict((k, remove_values(v, no_log_strings)) for k, v in value.items())
+    elif isinstance(value, tuple(chain(NUMBERTYPES, (bool, NoneType)))):
+        stringy_value = str(value)
+        if stringy_value in no_log_strings:
+            return 'VALUE_SPECIFIED_IN_NO_LOG_PARAMETER'
+        for omit_me in no_log_strings:
+            if omit_me in stringy_value:
+                return 'VALUE_SPECIFIED_IN_NO_LOG_PARAMETER'
+    else:
+        raise TypeError('Value of unknown type: %s, %s' % (type(value), value))
+    return value
+
+def heuristic_log_sanitize(data, no_log_values=None):
     ''' Remove strings that look like passwords from log messages '''
     # Currently filters:
     # user:pass@foo/whatever and http://username:pass@wherever/foo
@@ -421,53 +467,10 @@ def heuristic_log_sanitize(data):
             output.insert(0, data[begin:sep + 1])
             prev_begin = begin
 
-    return ''.join(output)
+    output = ''.join(output)
-
+    if no_log_values:
-def _return_values(obj):
+        output = remove_values(output, no_log_values)
-    """ Return stringified values from datastructures. For use with removing
+    return output
-    sensitive values pre-jsonification."""
-    if isinstance(obj, basestring):
-        if obj:
-            yield obj
-        return
-    elif isinstance(obj, Sequence):
-        for element in obj:
-            for subelement in _return_values(element):
-                yield subelement
-    elif isinstance(obj, Mapping):
-        for element in obj.items():
-            for subelement in _return_values(element[1]):
-                yield subelement
-    elif isinstance(obj, (bool, NoneType)):
-        # This must come before int because bools are also ints
-        return
-    elif isinstance(obj, NUMBERTYPES):
-        yield str(obj)
-    else:
-        raise TypeError('Unknown parameter type: %s, %s' % (type(obj), obj))
-
-def _remove_values(value, no_log_strings):
-    """ Remove strings in no_log_strings from value.  If value is a container
-    type, then remove a lot more"""
-    if isinstance(value, basestring):
-        if value in no_log_strings:
-            return 'VALUE_SPECIFIED_IN_NO_LOG_PARAMETER'
-        for omit_me in no_log_strings:
-            value = value.replace(omit_me, '*' * 8)
-    elif isinstance(value, Sequence):
-        return [_remove_values(elem, no_log_strings) for elem in value]
-    elif isinstance(value, Mapping):
-        return dict((k, _remove_values(v, no_log_strings)) for k, v in value.items())
-    elif isinstance(value, tuple(chain(NUMBERTYPES, (bool, NoneType)))):
-        stringy_value = str(value)
-        if stringy_value in no_log_strings:
-            return 'VALUE_SPECIFIED_IN_NO_LOG_PARAMETER'
-        for omit_me in no_log_strings:
-            if omit_me in stringy_value:
-                return 'VALUE_SPECIFIED_IN_NO_LOG_PARAMETER'
-    else:
-        raise TypeError('Value of unknown type: %s, %s' % (type(value), value))
-    return value
 
 def is_executable(path):
     '''is the given path executable?'''
@@ -502,12 +505,22 @@ class AnsibleModule(object):
                 if k not in self.argument_spec:
                     self.argument_spec[k] = v
 
+        self.params = self._load_params()
+
+        # Save parameter values that should never be logged
+        self.no_log_values = set()
+        # Use the argspec to determine which args are no_log
+        for arg_name, arg_opts in self.argument_spec.items():
+            if arg_opts.get('no_log', False):
+                # Find the value for the no_log'd param
+                no_log_object = self.params.get(arg_name, None)
+                if no_log_object:
+                    self.no_log_values.update(return_values(no_log_object))
+
         # check the locale as set by the current environment, and
         # reset to LANG=C if it's an invalid/unavailable locale
         self._check_locale()
 
-        self.params = self._load_params()
-
         self._legal_inputs = ['_ansible_check_mode', '_ansible_no_log', '_ansible_debug']
 
         # append to legal_inputs and then possibly check against them
@@ -540,6 +553,7 @@ class AnsibleModule(object):
             self._check_required_if(required_if)
 
         self._set_defaults(pre=False)
+
         if not self.no_log:
             self._log_invocation()
 
@@ -1337,20 +1351,16 @@ class AnsibleModule(object):
 
             # We want journal to always take text type
             # syslog takes bytes on py2, text type on py3
-            if sys.version_info >= (3,):
             if isinstance(msg, bytes):
-                    journal_msg = msg.decode('utf-8', 'replace')
+                journal_msg = remove_values(msg.decode('utf-8', 'replace'), self.no_log_values)
+            else:
+                # TODO: surrogateescape is a danger here on Py3
+                journal_msg = remove_values(msg, self.no_log_values)
+
+            if sys.version_info >= (3,):
                 syslog_msg = journal_msg
             else:
-                    # TODO: surrogateescape is a danger here
+                syslog_msg = journal_msg.encode('utf-8', 'replace')
-                    journal_msg = syslog_msg = msg
-            else:
-                if isinstance(msg, bytes):
-                    journal_msg = msg.decode('utf-8', 'replace')
-                    syslog_msg = msg
-                else:
-                     journal_msg = msg
-                     syslog_msg = msg.encode('utf-8', 'replace')
 
             if has_journal:
                 journal_args = [("MODULE", os.path.basename(__file__))]
@@ -1386,7 +1396,7 @@ class AnsibleModule(object):
                     param_val = str(param_val)
                 elif isinstance(param_val, unicode):
                     param_val = param_val.encode('utf-8')
-                log_args[param] = heuristic_log_sanitize(param_val)
+                log_args[param] = heuristic_log_sanitize(param_val, self.no_log_values)
 
         msg = []
         for arg in log_args:
@@ -1492,30 +1502,12 @@ class AnsibleModule(object):
         for path in self.cleanup_files:
             self.cleanup(path)
 
-    def remove_no_log_values(self, to_jsonify):
-        """ Strip values associated with no_log parameters from output.
-            Note: does not strip dict keys, only dict values.
-        """
-        no_log_strings = set()
-        # Use the argspec to determine which args are no_log
-        for arg_name, arg_opts in self.argument_spec.items():
-            if arg_opts.get('no_log', False):
-                # Find the value for the no_log'd param
-                no_log_object = self.params.get(arg_name, None)
-                if no_log_object:
-                    no_log_strings.update(_return_values(no_log_object))
-
-        for field, value in to_jsonify.items():
-            to_jsonify[field] = _remove_values(value, no_log_strings)
-
-        return to_jsonify
-
     def exit_json(self, **kwargs):
         ''' return from the module, without error '''
         self.add_path_info(kwargs)
         if not 'changed' in kwargs:
             kwargs['changed'] = False
-        self.remove_no_log_values(kwargs)
+        kwargs = remove_values(kwargs, self.no_log_values)
         self.do_cleanup_files()
         print(self.jsonify(kwargs))
         sys.exit(0)
@@ -1525,7 +1517,7 @@ class AnsibleModule(object):
         self.add_path_info(kwargs)
         assert 'msg' in kwargs, "implementation error -- msg to explain the error is required"
         kwargs['failed'] = True
-        self.remove_no_log_values(kwargs)
+        kwargs = remove_values(kwargs, self.no_log_values)
         self.do_cleanup_files()
         print(self.jsonify(kwargs))
         sys.exit(1)
@@ -1776,7 +1768,8 @@ class AnsibleModule(object):
                     continue
                 else:
                     is_passwd = True
-            clean_args.append(heuristic_log_sanitize(arg))
+            arg = heuristic_log_sanitize(arg, self.no_log_values)
+            clean_args.append(arg)
         clean_args = ' '.join(pipes.quote(arg) for arg in clean_args)
 
         if data:
@@ -1872,7 +1865,7 @@ class AnsibleModule(object):
             self.fail_json(rc=257, msg=traceback.format_exc(), cmd=clean_args)
 
         if rc != 0 and check_rc:
-            msg = heuristic_log_sanitize(stderr.rstrip())
+            msg = heuristic_log_sanitize(stderr.rstrip(), self.no_log_values)
             self.fail_json(cmd=clean_args, rc=rc, stdout=stdout, stderr=stderr, msg=msg)
 
         # reset the pwd

test/units/module_utils/basic/test_exit_json.py

@@ -0,0 +1,140 @@
+# -*- coding: utf-8 -*-
+# (c) 2015, Toshio Kuratomi <tkuratomi@ansible.com>
+#
+# This file is part of Ansible
+#
+# Ansible is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Ansible is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
+
+# Make coding more python3-ish
+from __future__ import (absolute_import, division)
+__metaclass__ = type
+
+import copy
+import json
+import sys
+
+from ansible.compat.tests import unittest
+from ansible.compat.six import StringIO
+
+from ansible.module_utils import basic
+from ansible.module_utils.basic import heuristic_log_sanitize
+from ansible.module_utils.basic import return_values, remove_values
+
+@unittest.skipIf(sys.version_info[0] >= 3, "Python 3 is not supported on targets (yet)")
+class TestAnsibleModuleExitJson(unittest.TestCase):
+    def setUp(self):
+        self.COMPLEX_ARGS = basic.MODULE_COMPLEX_ARGS
+        basic.MODULE_COMPLEX_ARGS = '{}'
+
+        self.old_stdout = sys.stdout
+        self.fake_stream = StringIO()
+        sys.stdout = self.fake_stream
+
+        self.module = basic.AnsibleModule(argument_spec=dict())
+
+    def tearDown(self):
+        basic.MODULE_COMPLEX_ARGS = self.COMPLEX_ARGS
+        sys.stdout = self.old_stdout
+
+    def test_exit_json_no_args_exits(self):
+        with self.assertRaises(SystemExit) as ctx:
+            self.module.exit_json()
+        self.assertEquals(ctx.exception.code, 0)
+        return_val = json.loads(self.fake_stream.getvalue())
+        self.assertEquals(return_val, dict(changed=False))
+
+    def test_exit_json_args_exits(self):
+        with self.assertRaises(SystemExit) as ctx:
+            self.module.exit_json(msg='message')
+        self.assertEquals(ctx.exception.code, 0)
+        return_val = json.loads(self.fake_stream.getvalue())
+        self.assertEquals(return_val, dict(msg="message", changed=False))
+
+    def test_fail_json_exits(self):
+        with self.assertRaises(SystemExit) as ctx:
+            self.module.fail_json(msg='message')
+        self.assertEquals(ctx.exception.code, 1)
+        return_val = json.loads(self.fake_stream.getvalue())
+        self.assertEquals(return_val, dict(msg="message", failed=True))
+
+    def test_exit_json_proper_changed(self):
+        with self.assertRaises(SystemExit) as ctx:
+            self.module.exit_json(changed=True, msg='success')
+        return_val = json.loads(self.fake_stream.getvalue())
+        self.assertEquals(return_val, dict(changed=True, msg='success'))
+
+@unittest.skipIf(sys.version_info[0] >= 3, "Python 3 is not supported on targets (yet)")
+class TestAnsibleModuleExitValuesRemoved(unittest.TestCase):
+    OMIT = 'VALUE_SPECIFIED_IN_NO_LOG_PARAMETER'
+    dataset = (
+            (dict(username='person', password='$ecret k3y'),
+                dict(one=1, pwd='$ecret k3y', url='https://username:password12345@foo.com/login/',
+                    not_secret='following the leader', msg='here'),
+                dict(one=1, pwd=OMIT, url='https://username:password12345@foo.com/login/',
+                    not_secret='following the leader', changed=False, msg='here')
+                ),
+            (dict(username='person', password='password12345'),
+                dict(one=1, pwd='$ecret k3y', url='https://username:password12345@foo.com/login/',
+                    not_secret='following the leader', msg='here'),
+                dict(one=1, pwd='$ecret k3y', url='https://username:********@foo.com/login/',
+                    not_secret='following the leader', changed=False, msg='here')
+                ),
+            (dict(username='person', password='$ecret k3y'),
+                dict(one=1, pwd='$ecret k3y', url='https://username:$ecret k3y@foo.com/login/',
+                    not_secret='following the leader', msg='here'),
+                dict(one=1, pwd=OMIT, url='https://username:********@foo.com/login/',
+                    not_secret='following the leader', changed=False, msg='here')
+                ),
+            )
+
+    def setUp(self):
+        self.COMPLEX_ARGS = basic.MODULE_COMPLEX_ARGS
+        self.old_stdout = sys.stdout
+
+    def tearDown(self):
+        basic.MODULE_COMPLEX_ARGS = self.COMPLEX_ARGS
+        sys.stdout = self.old_stdout
+
+    def test_exit_json_removes_values(self):
+        for args, return_val, expected in self.dataset:
+            sys.stdout = StringIO()
+            basic.MODULE_COMPLEX_ARGS = json.dumps(args)
+            module = basic.AnsibleModule(
+                argument_spec = dict(
+                    username=dict(),
+                    password=dict(no_log=True),
+                    token=dict(no_log=True),
+                    ),
+                )
+            with self.assertRaises(SystemExit) as ctx:
+                self.assertEquals(module.exit_json(**return_val), expected)
+            self.assertEquals(json.loads(sys.stdout.getvalue()), expected)
+
+    def test_fail_json_removes_values(self):
+        for args, return_val, expected in self.dataset:
+            expected = copy.deepcopy(expected)
+            del expected['changed']
+            expected['failed'] = True
+            sys.stdout = StringIO()
+            basic.MODULE_COMPLEX_ARGS = json.dumps(args)
+            module = basic.AnsibleModule(
+                argument_spec = dict(
+                    username=dict(),
+                    password=dict(no_log=True),
+                    token=dict(no_log=True),
+                    ),
+                )
+            with self.assertRaises(SystemExit) as ctx:
+                self.assertEquals(module.fail_json(**return_val), expected)
+            self.assertEquals(json.loads(sys.stdout.getvalue()), expected)

test/units/module_utils/basic/test_heuristic_log_sanitize.py

@@ -85,18 +85,6 @@ class TestHeuristicLogSanitize(unittest.TestCase):
         self.assertTrue(ssh_output.endswith("}"))
         self.assertIn(":********@foo.com/data'", ssh_output)
 
-
+    def test_hides_parameter_secrets(self):
-class TestStripNoLog(unittest.TestCase):
+        output = heuristic_log_sanitize('token="secret", user="person", token_entry="test=secret"', frozenset(['secret']))
-    def setUp(self):
+        self.assertNotIn('secret', output)
-        data = ''
-
-    def test_return_strings(self):
-        pass
-
-    def test_strip_no_log(self):
-        pass
-
-class TestAnsibleModuleStripNoLogValues(unittest.TestCase):
-    pass
-
-

test/units/module_utils/basic/test_log.py

@@ -103,7 +103,7 @@ class TestAnsibleModuleLogSyslog(unittest.TestCase):
             u'Toshio くらとみ non-ascii test': u'Toshio くらとみ non-ascii test'.encode('utf-8'),
             b'Byte string': b'Byte string',
             u'Toshio くらとみ non-ascii test'.encode('utf-8'): u'Toshio くらとみ non-ascii test'.encode('utf-8'),
-            b'non-utf8 :\xff: test': b'non-utf8 :\xff: test'
+            b'non-utf8 :\xff: test': b'non-utf8 :\xff: test'.decode('utf-8', 'replace').encode('utf-8'),
             }
 
     py3_output_data = {

test/units/module_utils/basic/test_no_log.py

@@ -29,7 +29,7 @@ from ansible.compat.tests.mock import patch, MagicMock
 
 from ansible.module_utils import basic
 from ansible.module_utils.basic import heuristic_log_sanitize
-from ansible.module_utils.basic import _return_values, _remove_values
+from ansible.module_utils.basic import return_values, remove_values
 
 
 class TestReturnValues(unittest.TestCase):
@@ -50,10 +50,10 @@ class TestReturnValues(unittest.TestCase):
 
     def test_return_values(self):
         for data, expected in self.dataset:
-            self.assertEquals(frozenset(_return_values(data)), expected)
+            self.assertEquals(frozenset(return_values(data)), expected)
 
     def test_unknown_type(self):
-        self.assertRaises(TypeError, frozenset, _return_values(object()))
+        self.assertRaises(TypeError, frozenset, return_values(object()))
 
 
 class TestRemoveValues(unittest.TestCase):
@@ -98,54 +98,13 @@ class TestRemoveValues(unittest.TestCase):
 
     def test_no_removal(self):
         for value, no_log_strings in self.dataset_no_remove:
-            self.assertEquals(_remove_values(value, no_log_strings), value)
+            self.assertEquals(remove_values(value, no_log_strings), value)
 
     def test_strings_to_remove(self):
         for value, no_log_strings, expected in self.dataset_remove:
-            self.assertEquals(_remove_values(value, no_log_strings), expected)
+            self.assertEquals(remove_values(value, no_log_strings), expected)
 
     def test_unknown_type(self):
-        self.assertRaises(TypeError, _remove_values, object(), frozenset())
+        self.assertRaises(TypeError, remove_values, object(), frozenset())
 
 
-@unittest.skipIf(sys.version_info[0] >= 3, "Python 3 is not supported on targets (yet)")
-class TestAnsibleModuleRemoveNoLogValues(unittest.TestCase):
-    OMIT = 'VALUE_SPECIFIED_IN_NO_LOG_PARAMETER'
-    dataset = (
-            (dict(username='person', password='$ecret k3y'),
-                dict(one=1, pwd='$ecret k3y', url='https://username:password12345@foo.com/login/',
-                    not_secret='following the leader'),
-                dict(one=1, pwd=OMIT, url='https://username:password12345@foo.com/login/',
-                    not_secret='following the leader')
-                ),
-            (dict(username='person', password='password12345'),
-                dict(one=1, pwd='$ecret k3y', url='https://username:password12345@foo.com/login/',
-                    not_secret='following the leader'),
-                dict(one=1, pwd='$ecret k3y', url='https://username:********@foo.com/login/',
-                    not_secret='following the leader')
-                ),
-            (dict(username='person', password='$ecret k3y'),
-                dict(one=1, pwd='$ecret k3y', url='https://username:$ecret k3y@foo.com/login/',
-                    not_secret='following the leader'),
-                dict(one=1, pwd=OMIT, url='https://username:********@foo.com/login/',
-                    not_secret='following the leader')
-                ),
-            )
-
-    def setUp(self):
-        self.COMPLEX_ARGS = basic.MODULE_COMPLEX_ARGS
-
-    def tearDown(self):
-        basic.MODULE_COMPLEX_ARGS = self.COMPLEX_ARGS
-
-    def test_remove_no_log_values(self):
-        for args, return_val, expected in self.dataset:
-            basic.MODULE_COMPLEX_ARGS = json.dumps(args)
-            module = basic.AnsibleModule(
-                argument_spec = dict(
-                    username=dict(),
-                    password=dict(no_log=True),
-                    token=dict(no_log=True),
-                    ),
-                )
-            self.assertEquals(module.remove_no_log_values(return_val), expected)