Skip to content

Commit 44273e0

Browse files
GWealeKoushik-Salammagari
authored andcommitted
fix: validate user_id and session_id against path traversal
Closes #5110 Co-authored-by: George Weale <gweale@google.com> PiperOrigin-RevId: 899108631
1 parent 8b8bb73 commit 44273e0

2 files changed

Lines changed: 90 additions & 0 deletions

File tree

src/google/adk/artifacts/file_artifact_service.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,13 +138,39 @@ def _is_user_scoped(session_id: Optional[str], filename: str) -> bool:
138138
return session_id is None or _file_has_user_namespace(filename)
139139

140140

141+
def _validate_path_segment(value: str, field_name: str) -> None:
142+
"""Rejects values that could alter the constructed filesystem path.
143+
144+
Args:
145+
value: The caller-supplied identifier (e.g. user_id or session_id).
146+
field_name: Human-readable name used in the error message.
147+
148+
Raises:
149+
InputValidationError: If the value contains path separators, traversal
150+
segments, or null bytes.
151+
"""
152+
if not value:
153+
raise InputValidationError(f"{field_name} must not be empty.")
154+
if "\x00" in value:
155+
raise InputValidationError(f"{field_name} must not contain null bytes.")
156+
if "/" in value or "\\" in value:
157+
raise InputValidationError(
158+
f"{field_name} {value!r} must not contain path separators."
159+
)
160+
if value in (".", "..") or ".." in value.split("/"):
161+
raise InputValidationError(
162+
f"{field_name} {value!r} must not contain traversal segments."
163+
)
164+
165+
141166
def _user_artifacts_dir(base_root: Path) -> Path:
142167
"""Returns the path that stores user-scoped artifacts."""
143168
return base_root / "artifacts"
144169

145170

146171
def _session_artifacts_dir(base_root: Path, session_id: str) -> Path:
147172
"""Returns the path that stores session-scoped artifacts."""
173+
_validate_path_segment(session_id, "session_id")
148174
return base_root / "sessions" / session_id / "artifacts"
149175

150176

@@ -220,6 +246,7 @@ def __init__(self, root_dir: Path | str):
220246

221247
def _base_root(self, user_id: str, /) -> Path:
222248
"""Returns the artifacts root directory for a user."""
249+
_validate_path_segment(user_id, "user_id")
223250
return self.root_dir / "users" / user_id
224251

225252
def _scope_root(

tests/unittests/artifacts/test_artifact_service.py

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -744,6 +744,69 @@ async def test_file_save_artifact_rejects_out_of_scope_paths(
744744
)
745745

746746

747+
@pytest.mark.asyncio
748+
@pytest.mark.parametrize(
749+
"user_id",
750+
[
751+
"../escape",
752+
"../../etc",
753+
"foo/../../bar",
754+
"valid/../..",
755+
"..",
756+
".",
757+
"has/slash",
758+
"back\\slash",
759+
"null\x00byte",
760+
"",
761+
],
762+
)
763+
async def test_file_save_artifact_rejects_traversal_in_user_id(
764+
tmp_path, user_id
765+
):
766+
"""FileArtifactService rejects user_id values that escape root_dir."""
767+
artifact_service = FileArtifactService(root_dir=tmp_path / "artifacts")
768+
part = types.Part(text="content")
769+
with pytest.raises(InputValidationError):
770+
await artifact_service.save_artifact(
771+
app_name="myapp",
772+
user_id=user_id,
773+
session_id="sess123",
774+
filename="safe.txt",
775+
artifact=part,
776+
)
777+
778+
779+
@pytest.mark.asyncio
780+
@pytest.mark.parametrize(
781+
"session_id",
782+
[
783+
"../escape",
784+
"../../tmp",
785+
"foo/../../bar",
786+
"..",
787+
".",
788+
"has/slash",
789+
"back\\slash",
790+
"null\x00byte",
791+
"",
792+
],
793+
)
794+
async def test_file_save_artifact_rejects_traversal_in_session_id(
795+
tmp_path, session_id
796+
):
797+
"""FileArtifactService rejects session_id values that escape root_dir."""
798+
artifact_service = FileArtifactService(root_dir=tmp_path / "artifacts")
799+
part = types.Part(text="content")
800+
with pytest.raises(InputValidationError):
801+
await artifact_service.save_artifact(
802+
app_name="myapp",
803+
user_id="user123",
804+
session_id=session_id,
805+
filename="safe.txt",
806+
artifact=part,
807+
)
808+
809+
747810
@pytest.mark.asyncio
748811
async def test_file_save_artifact_rejects_absolute_path_within_scope(tmp_path):
749812
"""Absolute filenames are rejected even when they point inside the scope."""

0 commit comments

Comments
 (0)