redact developer API key from request URL logs

googleapiclient/_helpers.py

@@ -17,6 +17,7 @@
 import functools
 import inspect
 import logging
+import re
 import urllib.parse
 
 logger = logging.getLogger(__name__)
@@ -205,3 +206,25 @@ def _add_query_parameter(url, name, value):
         return url
     else:
         return update_query_params(url, {name: value})
+
+
+# The developer API key travels on every request as the ``key`` query
+# parameter. It is a credential and must not be written to logs, even at
+# DEBUG level (CWE-532).
+_SENSITIVE_QUERY_PARAM_RE = re.compile(r"([?&]key=)[^&#]*", re.IGNORECASE)
+
+
+def _redact_sensitive_url(url):
+    """Redact credential query parameters from a URL before logging it.
+
+    Args:
+        url: string, the request URL that may carry an API key.
+
+    Returns:
+        A copy of the URL with the ``key`` query parameter value replaced by
+        ``REDACTED``. Non-string input is returned unchanged.
+    """
+    try:
+        return _SENSITIVE_QUERY_PARAM_RE.sub(r"\1REDACTED", url)
+    except TypeError:
+        return url

googleapiclient/discovery.py

@@ -62,7 +62,11 @@
 
 # Local imports
 from googleapiclient import _auth, mimeparse
-from googleapiclient._helpers import _add_query_parameter, positional
+from googleapiclient._helpers import (
+    _add_query_parameter,
+    _redact_sensitive_url,
+    positional,
+)
 from googleapiclient.errors import (
     HttpError,
     InvalidJsonError,
@@ -432,7 +436,7 @@ def _retrieve_discovery_doc(
         actual_url = _add_query_parameter(url, "userIp", os.environ["REMOTE_ADDR"])
     if developerKey:
         actual_url = _add_query_parameter(url, "key", developerKey)
-    logger.debug("URL being requested: GET %s", actual_url)
+    logger.debug("URL being requested: GET %s", _redact_sensitive_url(actual_url))
 
     # Execute this request with retries build into HttpRequest
     # Note that it will already raise an error if we don't get a 2xx response
@@ -1263,7 +1267,9 @@ def method(self, **kwargs):
                     ) % multipart_boundary
                     url = _add_query_parameter(url, "uploadType", "multipart")
 
-        logger.debug("URL being requested: %s %s" % (httpMethod, url))
+        logger.debug(
+            "URL being requested: %s %s" % (httpMethod, _redact_sensitive_url(url))
+        )
         return self._requestBuilder(
             self._http,
             model.response,
@@ -1383,7 +1389,10 @@ def methodNext(self, previous_request, previous_response):
             request.uri = _add_query_parameter(
                 request.uri, pageTokenName, nextPageToken
             )
-            logger.debug("Next page request URL: %s %s" % (methodName, request.uri))
+            logger.debug(
+                "Next page request URL: %s %s"
+                % (methodName, _redact_sensitive_url(request.uri))
+            )
         else:
             # Replace pageToken value in request body
             model = self._model

googleapiclient/http.py

@@ -181,7 +181,7 @@ def _retry_request(
                 num_retries,
                 req_type,
                 method,
-                uri,
+                util._redact_sensitive_url(uri),
                 resp.status if resp else exception,
             )
             sleep(sleep_time)
@@ -1157,7 +1157,12 @@ def next_chunk(self, http=None, num_retries=0):
                 self._sleep(self._rand() * 2**retry_num)
                 LOGGER.warning(
                     "Retry #%d for media upload: %s %s, following status: %d"
-                    % (retry_num, self.method, self.uri, resp.status)
+                    % (
+                        retry_num,
+                        self.method,
+                        util._redact_sensitive_url(self.uri),
+                        resp.status,
+                    )
                 )
 
             try:

tests/test__helpers.py

@@ -149,3 +149,44 @@ def test_with_repeats(self):
         content = "a=b&a=d"
         with self.assertRaises(ValueError):
             _helpers.parse_unique_urlencoded(content)
+
+
+class Test_redact_sensitive_url(unittest.TestCase):
+    def test_redacts_key(self):
+        url = "https://www.googleapis.com/foo?key=SECRET&alt=json"
+        self.assertEqual(
+            _helpers._redact_sensitive_url(url),
+            "https://www.googleapis.com/foo?key=REDACTED&alt=json",
+        )
+
+    def test_redacts_key_at_end(self):
+        url = "https://www.googleapis.com/foo?alt=json&key=SECRET"
+        self.assertEqual(
+            _helpers._redact_sensitive_url(url),
+            "https://www.googleapis.com/foo?alt=json&key=REDACTED",
+        )
+
+    def test_redacts_key_before_fragment(self):
+        url = "https://www.googleapis.com/foo?key=SECRET#frag"
+        self.assertEqual(
+            _helpers._redact_sensitive_url(url),
+            "https://www.googleapis.com/foo?key=REDACTED#frag",
+        )
+
+    def test_case_insensitive(self):
+        url = "https://www.googleapis.com/foo?Key=SECRET"
+        self.assertEqual(
+            _helpers._redact_sensitive_url(url),
+            "https://www.googleapis.com/foo?Key=REDACTED",
+        )
+
+    def test_does_not_touch_similar_param_names(self):
+        url = "https://www.googleapis.com/foo?apikey=KEEP&key2=KEEP"
+        self.assertEqual(_helpers._redact_sensitive_url(url), url)
+
+    def test_no_key(self):
+        url = "https://www.googleapis.com/foo?alt=json"
+        self.assertEqual(_helpers._redact_sensitive_url(url), url)
+
+    def test_non_string_returned_unchanged(self):
+        self.assertIsNone(_helpers._redact_sensitive_url(None))

tests/test_discovery.py

@@ -652,6 +652,22 @@ def test_building_with_developer_key_skips_adc(self):
         # application default credentials were used.
         self.assertNotIsInstance(plus._http, google_auth_httplib2.AuthorizedHttp)
 
+    def test_developer_key_not_logged(self):
+        discovery = read_datafile("plus.json")
+        plus = build_from_document(
+            discovery,
+            base="https://www.googleapis.com/",
+            developerKey="SECRET_DEVELOPER_KEY",
+        )
+        with self.assertLogs("googleapiclient.discovery", level="DEBUG") as cm:
+            request = plus.activities().list(collection="public", userId="me")
+        logged = "\n".join(cm.output)
+        self.assertIn("URL being requested", logged)
+        self.assertNotIn("SECRET_DEVELOPER_KEY", logged)
+        self.assertIn("key=REDACTED", logged)
+        # The real request still carries the key; only the log line is redacted.
+        self.assertIn("key=SECRET_DEVELOPER_KEY", request.uri)
+
     def test_building_with_context_manager(self):
         discovery = read_datafile("plus.json")
         with mock.patch("httplib2.Http") as http: