From b37140c26a0e5b01cbdabb3e5e52850d376f641f Mon Sep 17 00:00:00 2001 From: Izaak Gough Date: Fri, 30 Oct 2026 11:06:28 +0000 Subject: [PATCH 1/7] 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/7] 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() From 67debc9570fccc6f6eddf4421eb645bd30fdeb52 Mon Sep 17 00:00:00 2001 From: Izaak Gough Date: Wed, 4 Nov 2026 13:14:44 +0000 Subject: [PATCH 3/7] fix(https_fn): reject non-object and malformed callable bodies with 400 A JSON list, string or number body, an empty body, or malformed JSON returned 500 INTERNAL instead of 400 INVALID_ARGUMENT. --- src/firebase_functions/private/util.py | 7 ++++--- tests/test_util.py | 16 +++++++++++++++- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/firebase_functions/private/util.py b/src/firebase_functions/private/util.py index 1f24d22..658f459 100644 --- a/src/firebase_functions/private/util.py +++ b/src/firebase_functions/private/util.py @@ -115,18 +115,19 @@ def snake_to_camel(word: str) -> str: def _on_call_valid_body(request: _Request) -> bool: """The body must not be empty.""" - if request.json is None: + body = request.get_json(silent=True) + if body is None: _logging.warning("Request is missing body.") return False # The body must have data. - if "data" not in request.json: + if not isinstance(body, dict) or "data" not in body: _logging.warning("Request body is missing data.") return False # 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"] + extra_keys = [key for key in body.keys() if key != "data"] if extra_keys: _logging.warning("Request body has extra fields: %r", extra_keys) return False diff --git a/tests/test_util.py b/tests/test_util.py index 310d14c..fb98030 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -192,9 +192,23 @@ def test_on_call_valid_body_escapes_newlines_in_extra_field_keys(caplog): def test_on_call_valid_body_does_not_log_body_when_data_is_missing(caplog): - for body in ('{"secretKey": "secretValue"}', '["secretValue"]'): + for body in ( + '{"secretKey": "secretValue"}', + '["secretValue"]', + '["data", "secretValue"]', + '"data secretValue"', + "123", + ): 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 + + +def test_on_call_valid_body_rejects_empty_or_malformed_json(caplog): + for body in ("", "{bad"): + caplog.clear() + with caplog.at_level(logging.WARNING): + assert not _valid_body(body) + assert "Request is missing body." in caplog.text From fc7c7568f793aa9a9dcf8db5d43abe35784e3262 Mon Sep 17 00:00:00 2001 From: Izaak Gough Date: Wed, 4 Nov 2026 19:57:42 +0000 Subject: [PATCH 4/7] fix(https_fn): log malformed callable JSON separately from a missing body Also removes the unreachable body checks in the auth blocking handler, one of which still passed the request body to the logger. --- src/firebase_functions/private/_identity_fn.py | 6 ------ src/firebase_functions/private/util.py | 5 ++++- tests/test_util.py | 18 ++++++++++++------ 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/firebase_functions/private/_identity_fn.py b/src/firebase_functions/private/_identity_fn.py index 99e3e41..ec8ebca 100644 --- a/src/firebase_functions/private/_identity_fn.py +++ b/src/firebase_functions/private/_identity_fn.py @@ -361,12 +361,6 @@ def before_operation_handler( if not _util.valid_on_call_request(request): _logging.error("Invalid request, unable to process.") raise HttpsError(FunctionsErrorCode.INVALID_ARGUMENT, "Bad Request") - if request.json is None: - _logging.error("Request is missing body.") - raise HttpsError(FunctionsErrorCode.INVALID_ARGUMENT, "Bad Request") - if request.json is None or "data" not in request.json: - _logging.error("Request body is missing data.", request.json) - raise HttpsError(FunctionsErrorCode.INVALID_ARGUMENT, "Bad Request") jwt_token = request.json["data"]["jwt"] decoded_token = _token_verifier.verify_auth_blocking_token(jwt_token) event = _auth_blocking_event_from_token_data(event_type, decoded_token) diff --git a/src/firebase_functions/private/util.py b/src/firebase_functions/private/util.py index 658f459..4a7388d 100644 --- a/src/firebase_functions/private/util.py +++ b/src/firebase_functions/private/util.py @@ -117,7 +117,10 @@ def _on_call_valid_body(request: _Request) -> bool: """The body must not be empty.""" body = request.get_json(silent=True) if body is None: - _logging.warning("Request is missing body.") + if request.get_data(): + _logging.warning("Request body is not valid JSON.") + else: + _logging.warning("Request is missing body.") return False # The body must have data. diff --git a/tests/test_util.py b/tests/test_util.py index fb98030..52cf843 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -206,9 +206,15 @@ def test_on_call_valid_body_does_not_log_body_when_data_is_missing(caplog): assert "secretValue" not in caplog.text -def test_on_call_valid_body_rejects_empty_or_malformed_json(caplog): - for body in ("", "{bad"): - caplog.clear() - with caplog.at_level(logging.WARNING): - assert not _valid_body(body) - assert "Request is missing body." in caplog.text +def test_on_call_valid_body_rejects_empty_body(caplog): + with caplog.at_level(logging.WARNING): + assert not _valid_body("") + assert "Request is missing body." in caplog.text + + +def test_on_call_valid_body_rejects_malformed_json_without_logging_it(caplog): + with caplog.at_level(logging.WARNING): + assert not _valid_body("{secretValue") + assert "Request body is not valid JSON." in caplog.text + assert "Request is missing body." not in caplog.text + assert "secretValue" not in caplog.text From e940a9277ab141209139f33eb2ab88f271dc5319 Mon Sep 17 00:00:00 2001 From: Izaak Gough Date: Wed, 4 Nov 2026 20:04:59 +0000 Subject: [PATCH 5/7] fix(https_fn): log key names when callable data is missing --- src/firebase_functions/private/util.py | 10 ++++++---- tests/test_util.py | 10 ++++++++-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/firebase_functions/private/util.py b/src/firebase_functions/private/util.py index 4a7388d..b596ce9 100644 --- a/src/firebase_functions/private/util.py +++ b/src/firebase_functions/private/util.py @@ -123,13 +123,15 @@ def _on_call_valid_body(request: _Request) -> bool: _logging.warning("Request is missing body.") return False - # The body must have data. - if not isinstance(body, dict) or "data" not in body: + # Logged before auth is checked: key names only, never values. %r escapes + # newlines so a key cannot forge extra log lines. + if not isinstance(body, dict): _logging.warning("Request body is missing data.") return False + if "data" not in body: + _logging.warning("Request body is missing data. Fields: %r", list(body.keys())) + return False - # 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 body.keys() if key != "data"] if extra_keys: _logging.warning("Request body has extra fields: %r", extra_keys) diff --git a/tests/test_util.py b/tests/test_util.py index 52cf843..3688414 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -191,9 +191,15 @@ def test_on_call_valid_body_escapes_newlines_in_extra_field_keys(caplog): assert "\n" not in caplog.text.rstrip("\n") -def test_on_call_valid_body_does_not_log_body_when_data_is_missing(caplog): +def test_on_call_valid_body_logs_only_key_names_when_data_is_missing(caplog): + with caplog.at_level(logging.WARNING): + assert not _valid_body('{"secretKey": "secretValue"}') + assert "Request body is missing data. Fields: ['secretKey']" in caplog.text + assert "secretValue" not in caplog.text + + +def test_on_call_valid_body_does_not_log_non_object_body_when_data_is_missing(caplog): for body in ( - '{"secretKey": "secretValue"}', '["secretValue"]', '["data", "secretValue"]', '"data secretValue"', From 14a9c2d565aae368e37fd66cfd2957afa4359772 Mon Sep 17 00:00:00 2001 From: Izaak Gough Date: Thu, 5 Nov 2026 10:21:34 +0000 Subject: [PATCH 6/7] test(https_fn): cover valid callable bodies --- tests/test_util.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/test_util.py b/tests/test_util.py index 3688414..1b0a51d 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -224,3 +224,17 @@ def test_on_call_valid_body_rejects_malformed_json_without_logging_it(caplog): assert "Request body is not valid JSON." in caplog.text assert "Request is missing body." not in caplog.text assert "secretValue" not in caplog.text + + +def test_on_call_valid_body_accepts_any_data_value_without_logging(caplog): + for body in ( + '{"data": {}}', + '{"data": "value"}', + '{"data": 123}', + '{"data": null}', + '{"data": [1, 2, 3]}', + ): + caplog.clear() + with caplog.at_level(logging.WARNING): + assert _valid_body(body), body + assert caplog.text == "" From ac74442129fc852490fcd05f249e39f87d6ec24a Mon Sep 17 00:00:00 2001 From: Izaak Gough Date: Thu, 5 Nov 2026 10:27:29 +0000 Subject: [PATCH 7/7] fix(https_fn): skip building the extra-keys list for valid callable bodies --- src/firebase_functions/private/util.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/firebase_functions/private/util.py b/src/firebase_functions/private/util.py index b596ce9..ecefb49 100644 --- a/src/firebase_functions/private/util.py +++ b/src/firebase_functions/private/util.py @@ -132,8 +132,8 @@ def _on_call_valid_body(request: _Request) -> bool: _logging.warning("Request body is missing data. Fields: %r", list(body.keys())) return False - extra_keys = [key for key in body.keys() if key != "data"] - if extra_keys: + if len(body) > 1: + extra_keys = [key for key in body.keys() if key != "data"] _logging.warning("Request body has extra fields: %r", extra_keys) return False return True