fix(artifacts): validate user_id and session_id in FileArtifactService path construction#5111
Open
vnykmshr wants to merge 1 commit intogoogle:mainfrom
Open
fix(artifacts): validate user_id and session_id in FileArtifactService path construction#5111vnykmshr wants to merge 1 commit intogoogle:mainfrom
vnykmshr wants to merge 1 commit intogoogle:mainfrom
Conversation
…e path construction _base_root and _session_artifacts_dir used user_id and session_id directly in Path construction without checking for traversal segments. A crafted value (e.g. "../../x") could escape the storage root. Add _validate_path_segment using the same resolve()+relative_to() guard already applied to filenames, and wire it into both functions.
adk-bot
added
the
services
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
2 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.
Fixes #5110.
_base_rootand_session_artifacts_dirpassuser_idandsession_iddirectly intoPath()construction. The existing_resolve_scoped_artifact_pathguards the filename, but the scope root itself is built from these unvalidated values - so traversal segments escaperoot_dirbefore the filename check runs.Adds
_validate_path_segment, using the sameresolve(strict=False)+relative_to()pattern from_resolve_scoped_artifact_path, and applies it in both functions.Testing plan
7 new test cases: user_id traversal (3 parametrized), session_id traversal (3 parametrized), delete-path traversal (confirms
shutil.rmtreeunreachable with crafted session_id). FILE service only - GCS and InMemory are not affected.