error reports: Ensure we filter API keys from query strings.

For some webhook endpoints where the third-party API requires us to do
this, the user's API key might appear in error emails through
appearing in the `QUERY_STRING` parameter.  Fix that by filtering any
actual content from those; what we usually need for debugging is just
what set of parameters were provided.

zerver/filters.py

@@ -1,4 +1,5 @@
 
+import re
 from typing import Any, Dict
 
 from django.http import HttpRequest
@@ -14,3 +15,6 @@ class ZulipExceptionReporterFilter(SafeExceptionReporterFilter):
             if var in filtered_post:
                 filtered_post[var] = '**********'
         return filtered_post
+
+def clean_data_from_query_parameters(val: str) -> str:
+    return re.sub(r"([a-z_-]+=)([^&]+)([&]|$)", r"\1******\3", val)

zerver/lib/error_notify.py

@@ -7,8 +7,9 @@ from django.conf import settings
 from django.core.mail import mail_admins
 from django.http import HttpResponse
 from django.utils.translation import ugettext as _
-from typing import Any, Dict, Optional
+from typing import cast, Any, Dict, Optional
 
+from zerver.filters import clean_data_from_query_parameters
 from zerver.models import get_system_bot
 from zerver.lib.actions import internal_send_message
 from zerver.lib.response import json_success, json_error
@@ -102,7 +103,10 @@ def zulip_server_error(report: Dict[str, Any]) -> None:
             "- path: %(path)s\n"
             "- %(method)s: %(data)s\n") % (report)
         for field in ["REMOTE_ADDR", "QUERY_STRING", "SERVER_NAME"]:
-            request_repr += "- %s: \"%s\"\n" % (field, report.get(field.lower()))
+            val = cast(str, report.get(field.lower()))
+            if field == "QUERY_STRING":
+                val = clean_data_from_query_parameters(val)
+            request_repr += "- %s: \"%s\"\n" % (field, val)
         request_repr += "~~~~"
     else:
         request_repr = "Request info: none"
@@ -127,7 +131,10 @@ def email_server_error(report: Dict[str, Any]) -> None:
             "- path: %(path)s\n"
             "- %(method)s: %(data)s\n") % (report)
         for field in ["REMOTE_ADDR", "QUERY_STRING", "SERVER_NAME"]:
-            request_repr += "- %s: \"%s\"\n" % (field, report.get(field.lower()))
+            val = cast(str, report.get(field.lower()))
+            if field == "QUERY_STRING":
+                val = clean_data_from_query_parameters(val)
+            request_repr += "- %s: \"%s\"\n" % (field, val)
     else:
         request_repr = "Request info: none\n"
 

zerver/tests/test_logging_handlers.py

@@ -204,3 +204,11 @@ class LoggingConfigTest(TestCase):
             # `all_loggers`.
             for handler in logger.handlers:
                 assert not isinstance(handler, AdminEmailHandler)
+
+class ErrorFiltersTest(TestCase):
+    def test_clean_data_from_query_parameters(self) -> None:
+        from zerver.filters import clean_data_from_query_parameters
+        self.assertEqual(clean_data_from_query_parameters("api_key=abcdz&stream=1"),
+                         "api_key=******&stream=******")
+        self.assertEqual(clean_data_from_query_parameters("api_key=abcdz&stream=foo&topic=bar"),
+                         "api_key=******&stream=******&topic=******")