diff --git a/src/firebase_functions/private/util.py b/src/firebase_functions/private/util.py index d91d13e..1f24d22 100644 --- a/src/firebase_functions/private/util.py +++ b/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 diff --git a/tests/test_util.py b/tests/test_util.py index 71ce614..310d14c 100644 --- a/tests/test_util.py +++ b/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