fix(https_fn): stop logging callable request contents on invalid bodies

src/firebase_functions/private/util.py

@@ -121,15 +121,14 @@ def _on_call_valid_body(request: _Request) -> bool:
 
     # The body must have data.
     if "data" not in request.json:
-        _logging.warning("Request body is missing data.", request.json)
+        _logging.warning("Request body is missing data.")
         return False
 
-    extra_keys = {key: request.json[key] for key in request.json.keys() if key != "data"}
-    if len(extra_keys) != 0:
-        _logging.warning(
-            "Request body has extra fields: %s",
-            "".join(f"{key}: {value}," for (key, value) in extra_keys.items()),
-        )
+    # Logged before auth is checked: key names only, never values. %r escapes
+    # newlines so a key cannot forge extra log lines.
+    extra_keys = [key for key in request.json.keys() if key != "data"]
+    if extra_keys:
+        _logging.warning("Request body has extra fields: %r", extra_keys)
         return False
     return True
 

tests/test_util.py

@@ -16,9 +16,13 @@
 """
 
 import datetime as _dt
+import logging
 from os import environ, path
 
+from flask import Flask
+
 from firebase_functions.private.util import (
+    _on_call_valid_body,
     _unsafe_decode_id_token,
     deep_merge,
     firebase_config,
@@ -165,3 +169,32 @@ def test_unsafe_decode_token():
     result = _unsafe_decode_id_token(test_token)
     assert result["sub"] == "firebase"
     assert result["name"] == "John Doe"
+
+
+def _valid_body(body: str) -> bool:
+    with Flask(__name__).test_request_context(
+        method="POST", data=body, content_type="application/json"
+    ) as ctx:
+        return _on_call_valid_body(ctx.request)
+
+
+def test_on_call_valid_body_logs_extra_field_keys_without_values(caplog):
+    with caplog.at_level(logging.WARNING):
+        assert not _valid_body('{"data": {}, "extraKey": "secretValue"}')
+    assert "Request body has extra fields: ['extraKey']" in caplog.text
+    assert "secretValue" not in caplog.text
+
+
+def test_on_call_valid_body_escapes_newlines_in_extra_field_keys(caplog):
+    with caplog.at_level(logging.WARNING):
+        assert not _valid_body('{"data": {}, "a\\nFAKE LOG LINE": 1}')
+    assert "\n" not in caplog.text.rstrip("\n")
+
+
+def test_on_call_valid_body_does_not_log_body_when_data_is_missing(caplog):
+    for body in ('{"secretKey": "secretValue"}', '["secretValue"]'):
+        caplog.clear()
+        with caplog.at_level(logging.WARNING):
+            assert not _valid_body(body)
+        assert "Request body is missing data." in caplog.text
+        assert "secretValue" not in caplog.text