From fa1c93fa04636efbcd88f0f4f335316b6c4ad888 Mon Sep 17 00:00:00 2001 From: Enjoy Kumawat Date: Wed, 8 Apr 2026 09:05:42 +0530 Subject: [PATCH 1/2] fix: validate user_id and session_id against path traversal in FileArtifactService FileArtifactService validated filenames for path traversal but used user_id and session_id directly in Path() construction without validation. A user_id or session_id containing ../ segments could escape the root_dir. Add _validate_path_segment() to reject path separators (/, \) and traversal sequences (..) in these parameters. Fixes #5110 --- .../adk/artifacts/file_artifact_service.py | 21 ++++++++++++++ .../artifacts/test_artifact_service.py | 28 +++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/src/google/adk/artifacts/file_artifact_service.py b/src/google/adk/artifacts/file_artifact_service.py index b0078e27ce..17138e4960 100644 --- a/src/google/adk/artifacts/file_artifact_service.py +++ b/src/google/adk/artifacts/file_artifact_service.py @@ -62,6 +62,25 @@ def _file_uri_to_path(uri: str) -> Optional[Path]: return Path(unquote(parsed.path)) +def _validate_path_segment(value: str, name: str) -> None: + """Validates that a value is safe for use as a single path segment. + + Args: + value: The string to validate (e.g. a user_id or session_id). + name: A human-readable label used in error messages. + + Raises: + InputValidationError: If the value contains path separators or traversal + sequences. + """ + if not value: + raise InputValidationError(f"{name} must not be empty.") + if any(sep in value for sep in ("/", "\\", "..")): + raise InputValidationError( + f"{name} contains invalid characters: {value!r}" + ) + + _USER_NAMESPACE_PREFIX = "user:" @@ -145,6 +164,7 @@ def _user_artifacts_dir(base_root: Path) -> Path: def _session_artifacts_dir(base_root: Path, session_id: str) -> Path: """Returns the path that stores session-scoped artifacts.""" + _validate_path_segment(session_id, "session_id") return base_root / "sessions" / session_id / "artifacts" @@ -220,6 +240,7 @@ def __init__(self, root_dir: Path | str): def _base_root(self, user_id: str, /) -> Path: """Returns the artifacts root directory for a user.""" + _validate_path_segment(user_id, "user_id") return self.root_dir / "users" / user_id def _scope_root( diff --git a/tests/unittests/artifacts/test_artifact_service.py b/tests/unittests/artifacts/test_artifact_service.py index 25294d4909..32cd3c3e02 100644 --- a/tests/unittests/artifacts/test_artifact_service.py +++ b/tests/unittests/artifacts/test_artifact_service.py @@ -769,6 +769,34 @@ async def test_file_save_artifact_rejects_absolute_path_within_scope(tmp_path): ) +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("user_id", "session_id"), + [ + ("../escape", "sess123"), + ("user/../../etc", "sess123"), + ("user\\\\..\\\\secret", "sess123"), + ("valid_user", "../escape"), + ("valid_user", "sess/../../etc"), + ("valid_user", "sess\\\\..\\\\secret"), + ], +) +async def test_file_save_artifact_rejects_path_traversal_in_ids( + tmp_path, user_id, session_id +): + """FileArtifactService rejects user_id/session_id with path traversal.""" + artifact_service = FileArtifactService(root_dir=tmp_path / "artifacts") + part = types.Part(text="content") + with pytest.raises(InputValidationError): + await artifact_service.save_artifact( + app_name="myapp", + user_id=user_id, + session_id=session_id, + filename="safe.txt", + artifact=part, + ) + + class TestEnsurePart: """Tests for the ensure_part normalization helper.""" From cd9cd6b8a9343281bc3645c9aa86f739b8362710 Mon Sep 17 00:00:00 2001 From: Enjoy Kumawat Date: Thu, 9 Apr 2026 13:13:05 +0530 Subject: [PATCH 2/2] style: apply pyink formatting and add return type annotation --- src/google/adk/artifacts/file_artifact_service.py | 4 +--- tests/unittests/artifacts/test_artifact_service.py | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/google/adk/artifacts/file_artifact_service.py b/src/google/adk/artifacts/file_artifact_service.py index 17138e4960..faf253c97e 100644 --- a/src/google/adk/artifacts/file_artifact_service.py +++ b/src/google/adk/artifacts/file_artifact_service.py @@ -76,9 +76,7 @@ def _validate_path_segment(value: str, name: str) -> None: if not value: raise InputValidationError(f"{name} must not be empty.") if any(sep in value for sep in ("/", "\\", "..")): - raise InputValidationError( - f"{name} contains invalid characters: {value!r}" - ) + raise InputValidationError(f"{name} contains invalid characters: {value!r}") _USER_NAMESPACE_PREFIX = "user:" diff --git a/tests/unittests/artifacts/test_artifact_service.py b/tests/unittests/artifacts/test_artifact_service.py index 32cd3c3e02..8c288d58b4 100644 --- a/tests/unittests/artifacts/test_artifact_service.py +++ b/tests/unittests/artifacts/test_artifact_service.py @@ -783,7 +783,7 @@ async def test_file_save_artifact_rejects_absolute_path_within_scope(tmp_path): ) async def test_file_save_artifact_rejects_path_traversal_in_ids( tmp_path, user_id, session_id -): +) -> None: """FileArtifactService rejects user_id/session_id with path traversal.""" artifact_service = FileArtifactService(root_dir=tmp_path / "artifacts") part = types.Part(text="content")