Don't log environment variables with 'PASSWORD' in their name.

This involved rewriting the util.log_dict_diff() function. It has been
renamed to log_environment_changes() to better reflect its purpose.

It no longer logs both the old and new values in the event of an
environment variable changing. It now just logs the new value. This makes
the code simpler and seems like it should not be a big problem.

Some projects recommend passing credentials through the environment.

OpenStack does this, for example, see:
<http://docs.openstack.org/user-guide/content/cli_openrc.html>

It's unlikely that users would be happy about applications saving
these passwords in log files all over their system.

I do not recommend ever storing valuable passwords in the environment.

3 files changed, 23 insertions, 21 deletions

diff --git a/morphlib/app.py b/morphlib/app.py
index 88eb58a4..25f705f7 100644
--- a/morphlib/app.py
+++ b/morphlib/app.py
@@ -493,7 +493,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..0d4e25dc 100644
--- a/morphlib/util.py
+++ b/morphlib/util.py
@@ -207,24 +207,27 @@ 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
+
+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.