Merge remote-tracking branch 'origin/sam/avoid-logging-passwords-from-env-v2'

Reviewed-By: Pedro Alvarez <pedro.alvarez@codethink.co.uk>
Reviewed-By: Richard Maw <richard.maw@codethink.co.uk>

4 files changed, 42 insertions, 21 deletions

diff --git a/morphlib/app.py b/morphlib/app.py
index 88eb58a4..48de6aba 100644
--- a/morphlib/app.py
+++ b/morphlib/app.py
@@ -203,6 +203,10 @@ class Morph(cliapp.Application):
 
         self.add_subcommand('help-extensions', self.help_extensions)
 
+    def log_config(self):
+        with morphlib.util.hide_password_environment_variables(os.environ):
+            cliapp.Application.log_config(self)
+
     def process_args(self, args):
         self.check_time()
 
@@ -493,7 +497,7 @@ class Morph(cliapp.Application):
 
         # Log the environment.
         prev = getattr(self, 'prev_env', {})
-        morphlib.util.log_dict_diff(self, kwargs['env'], prev)
+        morphlib.util.log_environment_changes(self, kwargs['env'], prev)
         self.prev_env = kwargs['env']
 
         # run the command line
diff --git a/morphlib/plugins/deploy_plugin.py b/morphlib/plugins/deploy_plugin.py
index a80079fa..2bc53a0d 100644
--- a/morphlib/plugins/deploy_plugin.py
+++ b/morphlib/plugins/deploy_plugin.py
@@ -591,8 +591,7 @@ class DeployPlugin(cliapp.Plugin):
         '''
 
         def remove_passwords(env):
-            def is_password(key):
-                return 'PASSWORD' in key
+            is_password = morphlib.util.env_variable_is_password
             return { k:v for k, v in env.iteritems() if not is_password(k) }
 
         meta = {
diff --git a/morphlib/util.py b/morphlib/util.py
index 36ab4e21..ae1df56a 100644
--- a/morphlib/util.py
+++ b/morphlib/util.py
@@ -13,6 +13,7 @@
 # with this program; if not, write to the Free Software Foundation, Inc.,
 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 
+import contextlib
 import itertools
 import os
 import re
@@ -207,24 +208,37 @@ def new_repo_caches(app):  # pragma: no cover
 
     return lrc, rrc
 
-
-def log_dict_diff(app, cur, pre): # pragma: no cover
-    '''Log the differences between two dicts to debug log'''
-    dictA = cur
-    dictB = pre
-    for key in dictA.keys():
-        if key not in dictB:
-            app.status(msg="New environment: %(key)s = %(value)s",
-                       key=key, value=dictA[key], chatty=True)
-        elif dictA[key] != dictB[key]:
-            app.status(msg="Environment changed: \
-                %(key)s = %(valA)s to %(key)s = %(valB)s",
-                key=key, valA=dictA[key], valB=dictB[key], chatty=True)
-    for key in dictB.keys():
-        if key not in dictA:
-            app.status(msg="Environment removed:  %(key)s = %(value)s",
-                       key=key, value=dictB[key], chatty=True)
-
+def env_variable_is_password(key):  # pragma: no cover
+    return 'PASSWORD' in key
+
+@contextlib.contextmanager
+def hide_password_environment_variables(env):  # pragma: no cover
+    is_password = env_variable_is_password
+    password_env = { k:v for k,v in env.iteritems() if is_password(k) }
+    for k in password_env:
+        env[k] = '(value hidden)'
+    yield
+    for k, v in password_env.iteritems():
+        env[k] = v
+
+def log_environment_changes(app, current_env, previous_env): # pragma: no cover
+    '''Log the differences between two environments to debug log.'''
+    def log_event(key, value, event):
+        if env_variable_is_password(key):
+            value_msg = '(value hidden)'
+        else:
+            value_msg = '= "%s"' % value
+        msg = '%s environment variable %s %s' % (event, key, value_msg)
+        app.status(msg=msg, chatty=True)
+
+    for key in current_env.keys():
+        if key not in previous_env:
+            log_event(key, current_env[key], 'new')
+        elif current_env[key] != previous_env[key]:
+            log_event(key, current_env[key], 'changed')
+    for key in previous_env.keys():
+        if key not in current_env:
+            log_event(key, previous_env[key], 'unset')
 
 # This acquired from rdiff-backup which is GPLv2+ and a patch from 2011
 # which has not yet been merged, combined with a tad of tidying from us.
diff --git a/morphlib/writeexts.py b/morphlib/writeexts.py
index 5102bfdc..0fd0ad7b 100644
--- a/morphlib/writeexts.py
+++ b/morphlib/writeexts.py
@@ -113,6 +113,10 @@ class WriteExtension(cliapp.Application):
         logger.addHandler(handler)
         logger.setLevel(logging.DEBUG)
 
+    def log_config(self):
+        with morphlib.util.hide_password_environment_variables(os.environ):
+            cliapp.Application.log_config(self)
+
     def process_args(self, args):
         raise NotImplementedError()