From d8b1acf4b0e85bb40bf4804156dc9117dce45691 Mon Sep 17 00:00:00 2001 From: ninglinLiu Date: Fri, 30 Oct 2026 12:18:21 +0800 Subject: [PATCH] fix(firestore): only reject ids that fully match __.*__ startswith/endswith("__") also matched "__" and "___" because the prefix and suffix overlap, but Firestore's reserved pattern needs at least four characters. Use re.fullmatch so those valid document ids are accepted again. Fixes #7855 --- .../firestore/firestore_session_service.py | 6 +++- .../test_firestore_session_service.py | 28 +++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/google/adk/integrations/firestore/firestore_session_service.py b/src/google/adk/integrations/firestore/firestore_session_service.py index cf5f402194f..dafdd91fe82 100644 --- a/src/google/adk/integrations/firestore/firestore_session_service.py +++ b/src/google/adk/integrations/firestore/firestore_session_service.py @@ -20,6 +20,7 @@ from datetime import datetime from datetime import timezone import json +import re import logging import os from typing import Any @@ -206,7 +207,10 @@ def _validate_identity_component(name: str, value: str) -> None: ) if value in (".", ".."): raise ValueError(f"{name} must not be '.' or '..', got {value!r}.") - if value.startswith("__") and value.endswith("__"): + # Firestore reserved ids match __.*__ (at least four chars). "__" and + # "___" are valid document ids; startswith/endswith("__") wrongly rejects + # them because the prefix and suffix overlap. + if re.fullmatch(r"__.*__", value): raise ValueError( f"{name} must not match the Firestore-reserved __.*__ pattern, got" f" {value!r}." diff --git a/tests/unittests/integrations/firestore/test_firestore_session_service.py b/tests/unittests/integrations/firestore/test_firestore_session_service.py index 7b4dd44af38..052d30b4652 100644 --- a/tests/unittests/integrations/firestore/test_firestore_session_service.py +++ b/tests/unittests/integrations/firestore/test_firestore_session_service.py @@ -1376,6 +1376,34 @@ async def test_create_session_rejects_path_breaking_session_id( ) +@pytest.mark.parametrize( + "ok_id", + [ + "__", # valid Firestore doc id; prefix/suffix overlap + "___", + "user__", + "__user", + "_x_", + ], +) +def test_validate_identity_allows_non_reserved_underscore_ids(ok_id): + """Only full __.*__ matches are reserved; "__" and "___" must pass.""" + FirestoreSessionService._validate_identity_component("user_id", ok_id) + + +@pytest.mark.parametrize( + "bad_id", + [ + "____", + "__x__", + "__reserved__", + ], +) +def test_validate_identity_rejects_firestore_reserved_pattern(bad_id): + with pytest.raises(ValueError, match="Firestore-reserved"): + FirestoreSessionService._validate_identity_component("user_id", bad_id) + + @pytest.mark.asyncio async def test_get_session_rejects_path_breaking_identity( mock_firestore_client,