Repository: scoutapp/scout_apm_python (python)

Issue:
Don't log scout key
Currently we log the scout key from configuration and in the register method at startup. Security conscious companies don't want license keys in their log files. We should avoid logging the key, though perhaps a hash or a substring could be useful.

Pull request:
Don't log Scout key during startup
Fixes #297.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5f5e8a6..24d9239 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,12 @@
 # Changelog
 
+## Pending Release
+
+### Fixed
+
+- Don't log the Scout key during startup.
+  ([Issue #297](https://github.com/scoutapp/scout_apm_python/issues/297))
+
 ## [2.14.2] 2020-05-22
 
 ### Fixed
diff --git a/src/scout_apm/core/commands.py b/src/scout_apm/core/commands.py
index cb93ba0..7e86f42 100644
--- a/src/scout_apm/core/commands.py
+++ b/src/scout_apm/core/commands.py
@@ -2,11 +2,14 @@
 from __future__ import absolute_import, division, print_function, unicode_literals
 
 import logging
+import re
 
 from scout_apm.compat import iteritems
 
 logger = logging.getLogger(__name__)
 
+key_regex = re.compile(r"^[a-zA-Z0-9]{16}$")
+
 
 class Register(object):
     __slots__ = ("app", "key", "hostname")
@@ -17,9 +20,11 @@ class Register(object):
         self.hostname = hostname
 
     def message(self):
+        key_prefix = self.key[:3]
+        key_matches_regex = bool(key_regex.match(self.key))
         logger.info(
-            "Registering with app=%s key=%s host=%s"
-            % (self.app, self.key, self.hostname)
+            "Registering with app=%s key_prefix=%s key_format_validated=%s host=%s"
+            % (self.app, key_prefix, key_matches_regex, self.hostname)
         )
         return {
             "Register": {
diff --git a/src/scout_apm/core/config.py b/src/scout_apm/core/config.py
index c6c8f53..27cb482 100644
--- a/src/scout_apm/core/config.py
+++ b/src/scout_apm/core/config.py
@@ -3,6 +3,7 @@ from __future__ import absolute_import, division, print_function, unicode_litera
 
 import logging
 import os
+import re
 import warnings
 
 from scout_apm.compat import string_type
@@ -10,6 +11,8 @@ from scout_apm.core import platform_detection
 
 logger = logging.getLogger(__name__)
 
+key_regex = re.compile(r"[a-zA-Z0-9]{16}")
+
 
 class ScoutConfig(object):
     """
@@ -45,40 +48,44 @@ class ScoutConfig(object):
 
     def log(self):
         logger.debug("Configuration Loaded:")
-        for key in self.known_keys():
+        for key in self.known_keys:
+            if key in self.secret_keys:
+                continue
+
             layer = self.locate_layer_for_key(key)
             logger.debug(
-                "%-9s: %s = %s", layer.__class__.__name__, key, layer.value(key)
+                "%-9s: %s = %s", layer.__class__.__name__, key, layer.value(key),
             )
 
-    def known_keys(self):
-        return [
-            "app_server",
-            "application_root",
-            "core_agent_config_file",
-            "core_agent_dir",
-            "core_agent_download",
-            "core_agent_launch",
-            "core_agent_log_file",
-            "core_agent_log_level",
-            "core_agent_permissions",
-            "core_agent_socket_path",
-            "core_agent_version",
-            "disabled_instruments",
-            "download_url",
-            "framework",
-            "framework_version",
-            "hostname",
-            "ignore",
-            "key",
-            "log_level",
-            "monitor",
-            "name",
-            "revision_sha",
-            "scm_subdirectory",
-            "shutdown_message_enabled",
-            "shutdown_timeout_seconds",
-        ]
+    known_keys = [
+        "app_server",
+        "application_root",
+        "core_agent_config_file",
+        "core_agent_dir",
+        "core_agent_download",
+        "core_agent_launch",
+        "core_agent_log_file",
+        "core_agent_log_level",
+        "core_agent_permissions",
+        "core_agent_socket_path",
+        "core_agent_version",
+        "disabled_instruments",
+        "download_url",
+        "framework",
+        "framework_version",
+        "hostname",
+        "ignore",
+        "key",
+        "log_level",
+        "monitor",
+        "name",
+        "revision_sha",
+        "scm_subdirectory",
+        "shutdown_message_enabled",
+        "shutdown_timeout_seconds",
+    ]
+
+    secret_keys = {"key"}
 
     def core_agent_permissions(self):
         try:


Test patch:
diff --git a/tests/unit/core/test_commands.py b/tests/unit/core/test_commands.py
index c3e8f63..ad95c8c 100644
--- a/tests/unit/core/test_commands.py
+++ b/tests/unit/core/test_commands.py
@@ -2,6 +2,7 @@
 from __future__ import absolute_import, division, print_function, unicode_literals
 
 import datetime as dt
+import logging
 
 import pytest
 
@@ -23,21 +24,61 @@ END_TIME = dt.datetime(2018, 12, 1, 17, 4, 34, 641403)
 END_TIME_STR = "2018-12-01T17:04:34.641403Z"
 
 
+def test_register_message_good_key(caplog):
+    command = commands.Register(
+        app="test_app", key="abcdefghijabcdef", hostname="test_host"
+    )
+
+    assert command.message() == {
+        "Register": {
+            "app": "test_app",
+            "key": "abcdefghijabcdef",
+            "host": "test_host",
+            "language": "python",
+            "api_version": "1.0",
+        }
+    }
+    assert caplog.record_tuples == [
+        (
+            "scout_apm.core.commands",
+            logging.INFO,
+            (
+                "Registering with app=test_app key_prefix=abc"
+                + " key_format_validated=True host=test_host"
+            ),
+        )
+    ]
+
+
+def test_register_message_bad_key(caplog):
+    command = commands.Register(
+        app="test_app", key="whoops-pasted-it-in-wrong", hostname="test_host"
+    )
+
+    assert command.message() == {
+        "Register": {
+            "app": "test_app",
+            "key": "whoops-pasted-it-in-wrong",
+            "host": "test_host",
+            "language": "python",
+            "api_version": "1.0",
+        }
+    }
+    assert caplog.record_tuples == [
+        (
+            "scout_apm.core.commands",
+            logging.INFO,
+            (
+                "Registering with app=test_app key_prefix=who"
+                + " key_format_validated=False host=test_host"
+            ),
+        )
+    ]
+
+
 @pytest.mark.parametrize(
     "command, message",
     [
-        (
-            commands.Register(app="test_app", key="test_key", hostname="test_host"),
-            {
-                "Register": {
-                    "app": "test_app",
-                    "key": "test_key",
-                    "host": "test_host",
-                    "language": "python",
-                    "api_version": "1.0",
-                }
-            },
-        ),
         (
             commands.StartSpan(
                 timestamp=TIMESTAMP,
diff --git a/tests/unit/core/test_config.py b/tests/unit/core/test_config.py
index b8a1e0d..f7ac37a 100644
--- a/tests/unit/core/test_config.py
+++ b/tests/unit/core/test_config.py
@@ -1,7 +1,9 @@
 # coding=utf-8
 from __future__ import absolute_import, division, print_function, unicode_literals
 
+import logging
 import os
+import pprint
 import re
 import sys
 
@@ -116,10 +118,10 @@ def test_env_outranks_python():
         ScoutConfig.reset_all()
 
 
-def test_log_config():
+def test_log_config(caplog):
     # Include configs in various layers to exercise all code paths.
     os.environ["SCOUT_CORE_AGENT_DOWNLOAD"] = "False"
-    ScoutConfig.set(core_agent_launch=False)
+    ScoutConfig.set(core_agent_launch=False, key="abcdefghij")
     config = ScoutConfig()
     try:
         # Logging the config doesn't crash.
@@ -128,6 +130,18 @@ def test_log_config():
         del os.environ["SCOUT_CORE_AGENT_DOWNLOAD"]
         ScoutConfig.reset_all()
 
+    assert caplog.record_tuples[0] == (
+        "scout_apm.core.config",
+        logging.DEBUG,
+        "Configuration Loaded:",
+    )
+    assert (
+        "scout_apm.core.config",
+        logging.DEBUG,
+        "Python   : core_agent_launch = False",
+    ) in caplog.record_tuples
+    assert "abcdefghij" not in pprint.pformat(caplog.record_tuples)
+
 
 def test_core_agent_permissions_default():
     assert scout_config.core_agent_permissions() == 0o700