MAINT: Standardize path handling on pathlib#1815
Merged
romanlutz merged 3 commits intoMay 27, 2026
Merged
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jsong468
reviewed
May 26, 2026
jsong468
reviewed
May 26, 2026
jsong468
reviewed
May 26, 2026
jsong468
reviewed
May 26, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
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.
Summary
Standardizes path handling on
pathlib.Pathacross four files, eliminating strayos.path.*usage. Per the general rule,Pathis used end-to-end and only round-tripped tostrat API boundaries that genuinely require strings (markdown link hrefs, display strings, etc.).Files changed
pyrit/backend/routes/media.pypyrit/output/conversation/markdown.py—_format_link_path,_maybe_blur_image_on_disk,_blurred_destination. Keptimport osbecauseos.PathLike(type hints),os.fspath,os.getpid, andos.replaceare still used.pyrit/backend/mappers/attack_mappers.py— two sites (_resolve_media_url,_build_filename); removedimport os, addedfrom pathlib import Path.pyrit/models/storage_io.py— routedpath_exists,is_file, andcreate_directory_if_not_existsthrough the existing_convert_to_pathhelper and replacedos.path.exists/os.path.isfile/os.makedirswithPathmethods; removedimport os.pyrit/datasets/jailbreak/text_jailbreak.pywas intentionally left alone — thestr(template_path)calls there are deliberate boundary conversions for thetemplate_source: strfield used in logging and error messages.media.py— please review carefullyThe path-traversal protection in
_validate_media_pathwas rewritten to usepathlib. Symlink-resolution semantics are preserved:os.path.realpath(path)→Path(path).resolve(strict=False). Both resolve symlinks and normalize..components;strict=Falsematchesrealpath's behavior for non-existent paths.startswith(allowed_root + os.sep)) was replaced withreal_path.relative_to(allowed_root).relative_toraisesValueErrorwhenreal_pathis not underallowed_root, which is now caught and converted to the sameHTTPException 403as before. This is equivalent to (and arguably stricter than) the old prefix check — it does not allow sibling directory names that happen to share a prefix._, ext = os.path.splitext(real_path)→real_path.suffix.Pathinstead ofstr(private helper; the only caller was updated).A new symlink-escape test was added (
test_rejects_symlink_pointing_outside_results) that creates a symlink under an allowed subdirectory pointing outsideresults_pathand asserts the request is rejected with 403. The test skips on platforms where the user can't create symlinks (Windows without developer mode / admin).The existing
..-escape test (test_rejects_path_traversal) still passes unchanged.Behavior note:
_format_link_pathinmarkdown.pyos.path.relpath(path)was replaced withPath(path).relative_to(Path.cwd())plus aValueErrorfallback tostr(Path(path).resolve()). This is a small behavior change:os.path.relpathproduces../../...-style relative paths when the target is not under cwd, whereasrelative_toraises and we fall back to the absolute path. The output is still a valid path for the markdown link href — just absolute instead of relative-with-dotdots when the image lives outside the current working directory.The
_expected_linkhelper intests/unit/output/test_blur_images.pywas updated to mirror the new behavior, andtest_markdown_format_image_content_handles_cross_drive_pathnow patchesPath.relative_toinstead ofos.path.relpath. Test patches intests/unit/models/test_storage_io.pywere similarly updated to targetpathlib.Pathmethods instead ofos.path.*/os.makedirs.Verification
uv run pytest tests/unit -q -n 4→ 8049 passed, 119 skipped.uv run pre-commit run --files <changed files>→ all hooks pass.grep "os\.path\." pyrit/backend/routes/media.py pyrit/output/conversation/markdown.py pyrit/backend/mappers/attack_mappers.py pyrit/models/storage_io.py→ zero hits.