From b37140c26a0e5b01cbdabb3e5e52850d376f641f Mon Sep 17 00:00:00 2001 From: Izaak Gough Date: Fri, 30 Oct 2026 11:06:28 +0000 Subject: [PATCH 1/2] fix(https_fn): stop logging callable request contents on invalid bodies Invalid callable bodies are logged before auth is checked, so log only the count of extra fields and never the body. --- src/firebase_functions/private/util.py | 12 +++++------ tests/test_util.py | 28 ++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/src/firebase_functions/private/util.py b/src/firebase_functions/private/util.py index d91d13e..ff59ce7 100644 --- a/src/firebase_functions/private/util.py +++ b/src/firebase_functions/private/util.py @@ -121,15 +121,13 @@ 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()), - ) + # Request content is logged before auth is checked, so log neither keys nor values. + extra_field_count = sum(1 for key in request.json.keys() if key != "data") + if extra_field_count: + _logging.warning("Request body has extra fields (count=%d).", extra_field_count) return False return True diff --git a/tests/test_util.py b/tests/test_util.py index 71ce614..8043133 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,27 @@ 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_does_not_log_extra_field_keys_or_values(caplog): + with caplog.at_level(logging.WARNING): + assert not _valid_body('{"data": {}, "secretKey": "secretValue"}') + assert "Request body has extra fields (count=1)." in caplog.text + assert "secretKey" not in caplog.text + assert "secretValue" not in caplog.text + + +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 From 77ab9671133fa22e270636cdff2ca75fa301e5ff Mon Sep 17 00:00:00 2001 From: Izaak Gough Date: Fri, 30 Oct 2026 11:13:42 +0000 Subject: [PATCH 2/2] fix(https_fn): log extra callable field names instead of a count Matches the Node SDK. Names are logged with %r so a key cannot add log lines. --- src/firebase_functions/private/util.py | 9 +++++---- tests/test_util.py | 13 +++++++++---- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/firebase_functions/private/util.py b/src/firebase_functions/private/util.py index ff59ce7..1f24d22 100644 --- a/src/firebase_functions/private/util.py +++ b/src/firebase_functions/private/util.py @@ -124,10 +124,11 @@ def _on_call_valid_body(request: _Request) -> bool: _logging.warning("Request body is missing data.") return False - # Request content is logged before auth is checked, so log neither keys nor values. - extra_field_count = sum(1 for key in request.json.keys() if key != "data") - if extra_field_count: - _logging.warning("Request body has extra fields (count=%d).", extra_field_count) + # 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 8043133..310d14c 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -178,14 +178,19 @@ def _valid_body(body: str) -> bool: return _on_call_valid_body(ctx.request) -def test_on_call_valid_body_does_not_log_extra_field_keys_or_values(caplog): +def test_on_call_valid_body_logs_extra_field_keys_without_values(caplog): with caplog.at_level(logging.WARNING): - assert not _valid_body('{"data": {}, "secretKey": "secretValue"}') - assert "Request body has extra fields (count=1)." in caplog.text - assert "secretKey" not in caplog.text + 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()