fix(sessions): accept a full session resource name again on v1 - #6888
Merged
Conversation
The session id validation ported in #6809 requires a bare id, but Agent Engine passes the full projects/.../sessions/{id} resource name, so get_session, delete_session and create_session now reject it. Upstream hit the same thing and added _extract_short_session_id 13 days after the commit that was ported; v1 took the check without the follow-up. Ports that normalizer verbatim and calls it before validation at the three sites upstream patched. Ids that are not session resource names are unaffected.
xuanyang15
approved these changes
Aug 24, 2026
The resolved session id cannot tell a correctly built resource name from one that embedded the full name and happens to end in the same segment, because the mock resolves on the last path segment.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports
60c55ad74("fix vertex_ai_session_service crashing when Agent Engine passes full resource names instead of short session IDs") tov1, with its regression tests.#6809 brought
_validate_session_idacross, which requires a session id to match^[A-Za-z0-9_-]+$underfullmatch. Upstream added that check inb2916c71and then, 13 days later, had to add_extract_short_session_idin60c55ad74because Agent Engine passes the full resource name rather than the short id.v1took the check and not the follow-up, soget_session,delete_sessionandcreate_sessionnow reject a name of the formprojects/{p}/locations/{l}/reasoningEngines/{e}/sessions/{s}outright.mainhas accepted it since June._extract_short_session_idis ported verbatim from upstream. It returns the last path segment when the id looks like a session resource name, and raises when the embeddedreasoningEngines/{id}does not match the engine the service is configured for.reasoning_engine_idfirst, normalize, then validate, matching upstream's ordering.append_eventstill validatessession.idwithout normalizing, which is whatmaindoes at:396.Behaviour for anything that is not a session resource name is unchanged:
a/b,../fooand''are still rejected, since the normalizer only strips when the second-to-last segment issessions.Tests: the four unit tests from upstream, plus two end-to-end ones covering a full resource name being accepted and a resource name for a different engine being rejected. Deleting the normalizer call makes both of the latter fail while the existing path-traversal test still passes.