Skip to content

Commit 9957c52

Browse files
committed
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
1 parent 114deef commit 9957c52

File tree

2 files changed

+49
-0
lines changed

2 files changed

+49
-0
lines changed

src/google/adk/artifacts/file_artifact_service.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,25 @@ def _file_uri_to_path(uri: str) -> Optional[Path]:
6262
return Path(unquote(parsed.path))
6363

6464

65+
def _validate_path_segment(value: str, name: str) -> None:
66+
"""Validates that a value is safe for use as a single path segment.
67+
68+
Args:
69+
value: The string to validate (e.g. a user_id or session_id).
70+
name: A human-readable label used in error messages.
71+
72+
Raises:
73+
InputValidationError: If the value contains path separators or traversal
74+
sequences.
75+
"""
76+
if not value:
77+
raise InputValidationError(f"{name} must not be empty.")
78+
if any(sep in value for sep in ("/", "\\", "..")):
79+
raise InputValidationError(
80+
f"{name} contains invalid characters: {value!r}"
81+
)
82+
83+
6584
_USER_NAMESPACE_PREFIX = "user:"
6685

6786

@@ -145,6 +164,7 @@ def _user_artifacts_dir(base_root: Path) -> Path:
145164

146165
def _session_artifacts_dir(base_root: Path, session_id: str) -> Path:
147166
"""Returns the path that stores session-scoped artifacts."""
167+
_validate_path_segment(session_id, "session_id")
148168
return base_root / "sessions" / session_id / "artifacts"
149169

150170

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

221241
def _base_root(self, user_id: str, /) -> Path:
222242
"""Returns the artifacts root directory for a user."""
243+
_validate_path_segment(user_id, "user_id")
223244
return self.root_dir / "users" / user_id
224245

225246
def _scope_root(

tests/unittests/artifacts/test_artifact_service.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -769,6 +769,34 @@ async def test_file_save_artifact_rejects_absolute_path_within_scope(tmp_path):
769769
)
770770

771771

772+
@pytest.mark.asyncio
773+
@pytest.mark.parametrize(
774+
("user_id", "session_id"),
775+
[
776+
("../escape", "sess123"),
777+
("user/../../etc", "sess123"),
778+
("user\\\\..\\\\secret", "sess123"),
779+
("valid_user", "../escape"),
780+
("valid_user", "sess/../../etc"),
781+
("valid_user", "sess\\\\..\\\\secret"),
782+
],
783+
)
784+
async def test_file_save_artifact_rejects_path_traversal_in_ids(
785+
tmp_path, user_id, session_id
786+
):
787+
"""FileArtifactService rejects user_id/session_id with path traversal."""
788+
artifact_service = FileArtifactService(root_dir=tmp_path / "artifacts")
789+
part = types.Part(text="content")
790+
with pytest.raises(InputValidationError):
791+
await artifact_service.save_artifact(
792+
app_name="myapp",
793+
user_id=user_id,
794+
session_id=session_id,
795+
filename="safe.txt",
796+
artifact=part,
797+
)
798+
799+
772800
class TestEnsurePart:
773801
"""Tests for the ensure_part normalization helper."""
774802

0 commit comments

Comments
 (0)