Repository navigation
FileArtifactService: user_id and session_id not validated for path traversal #5110
Description
_resolve_scoped_artifact_path validates the filename against traversal, but _base_root and _session_artifacts_dir use user_id and session_id directly in Path() construction without the same check.
A user_id or session_id containing ../ segments builds a scope root outside root_dir. The filename guard then validates against the already-escaped scope root, so it doesn't catch the escape.
GcsArtifactService and InMemoryArtifactService use string keys, not filesystem paths - only FileArtifactService is affected.
Activity
fix: validate user_id and session_id against path traversal in FileAr…
fix: validate user_id and session_id against path traversal in FileAr…
surajksharma07 commentedon Apr 11, 2026
@vnykmshr The missing guard in _base_root() and _scope_root() is the root cause here.
While PR #5206 is under review, a workaround worth trying: subclass FileArtifactService and add a resolve() + relative_to() check on user_id and session_id before the path is constructed — same pattern already used for filenames in _resolve_scoped_artifact_path(), so it should slot in cleanly.
Could you give that a try and let us know if it blocks both traversal vectors on your end?
If it holds up, it'd be great to include it as a test case in the PR too!
vnykmshr commentedon Apr 11, 2026
@surajksharma07
_validate_path_segmentin #5111 does exactly that: resolve(strict=False) + relative_to() on both user_id and session_id before the path is built, same guard as_resolve_scoped_artifact_pathuses for filenames. Blocks both vectors.Tests: 7 parametrized cases covering user_id, session_id, and the delete path (confirms
shutil.rmtreeis unreachable with a crafted session_id).PR's been open since Apr 2 if it helps.
surajksharma07 commentedon Apr 12, 2026
@vnykmshr Apologies for the overlap! The _validate_path_segment approach in #5111 is exactly the right fix, and 7 parametrized tests covering both the user_id/session_id vectors and the shutil.rmtree path is really thorough work.
I'll flag both #5111 and #5206 for prioritized review given this has been sitting unpatched since Apr 2.
surajksharma07 commentedon Apr 12, 2026
@DeanChensj Please have a look into it once.
fix: validate user_id and session_id against path traversal
fix: validate user_id and session_id against path traversal
fix: validate user_id and session_id against path traversal
fix: validate user_id and session_id against path traversal
fix: validate user_id and session_id against path traversal