[CorgiBoyG]

Link to Issue or Description of Change

• Closes: FirestoreSessionService does not validate identity components, so a user_id/session_id with '/' injects into the document path and nests one identity's session under another #7800

Problem:

FirestoreSessionService uses app_name, user_id and session_id as raw Firestore document path segments:

self.client.collection(self.root_collection)
    .document(app_name).collection("users").document(user_id)
    .collection(self.sessions_collection).document(session_id)

The Firestore client treats / as a path separator and verify_path only checks that a document path has an even number of segments, not segment content. So an identity component with an even number of / is accepted and silently changes the path structure, nesting one identity's session document under another's. For example user_id="alice/sessions/s" produces .../users/alice/sessions/s/sessions/x, whose parent chain users/alice/sessions/s is byte-identical to the session document of the real user alice / session s.

This is the Firestore counterpart of the Redis key-collision issue #7464. The core VertexAiSessionService already guards with _validate_session_id(); the Firestore backend applied no equivalent check.

Solution:

Add _validate_identity_component() and apply it to app_name / user_id (in _get_sessions_ref) and session_id (in a new _get_session_doc_ref), covering every CRUD path (create_session, get_session, list_sessions, delete_session, append_event). It rejects only components that would break the document path — containing /, equal to ./.., empty, or matching the Firestore-reserved __.*__ pattern — so ordinary ids, including email addresses, still work. The core allow-list ^[A-Za-z0-9_-]+$ would be too strict here (it rejects .), so this uses a conservative reject-list instead; happy to adjust the exact set per the issue discussion.

The empty/None user_id behavior of list_sessions from #7662 is unchanged: it returns before reaching the validated reference.

Testing Plan

Unit Tests:

• I have added or updated unit tests for my change.
• All relevant unit tests pass locally.

New regression tests (fail on main, pass after the fix):

pytest -q tests/unittests/integrations/firestore/test_firestore_session_service.py -k "rejects or accepts_normal_identity"
16 passed

pytest -q tests/unittests/integrations/firestore/
71 passed

The new tests assert that path-breaking user_id/session_id values ("alice/sessions/s", "x/sessions/y", "..", "", "__reserved__") raise ValueError on create_session/get_session/delete_session, while ordinary ids ("alice", "[email protected]", "user-123_abc", "a.b.c") still succeed. The list_sessions empty/None user_id behavior (#7662) is covered and unchanged.

Static checks:

pyink --check: passed
ruff check: passed
git diff --check: passed

The full repository test suite was not run.

Manual End-to-End (E2E) Tests:

Not run. The identity-to-path mapping was verified offline with google-cloud-firestore 2.34.1 by inspecting the locally-built document path and the resource name carried in the Commit RPC.

Checklist

• I have read the CONTRIBUTING.md document.
• I have performed a self-review of my own code.
• I have commented my code, particularly in hard-to-understand areas.
• I have added tests that prove my fix is effective.
• Relevant Firestore unit tests pass locally.
• I have manually tested my changes end-to-end.
• No downstream dependency changes are required.

Additional context

MongoDB uses structured dict keys (no delimiter collision) and Redis is tracked separately (#7464 / #7563). If maintainers prefer, the validation helper could be hoisted into a shared location for the third-party backends; kept local to Firestore here to stay minimal.

[adk-bot]

Thank you @CorgiBoyG for your contribution! 🎉

Your changes have been successfully imported and merged via Copybara in commit ec9756e.

Closing this PR as the changes are now in the main branch.