Harden RemoteFileSystem path resolution containment and rebuild absolute URLs#22476
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Merging this PR will not alter performance
Comparing Footnotes
|
…ainment Co-authored-by: Alex S. <alex.s@prefect.io> Co-Authored-By: alex.s <ajstreed1@gmail.com>
3178d9d to
8f27321
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f27321c81
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…tests - Reject absolute paths with dot-segment traversal (literal or encoded %2e variants) - Treat backslashes as separators for SMB traversal detection - Strip control characters that urlsplit removes - Rebuild accepted absolute URLs from the configured scheme/netloc - Preserve percent-encoded object keys in returned URLs Co-authored-by: Alex S. <alex.s@prefect.io> Co-Authored-By: alex.s <ajstreed1@gmail.com>
Use the decoded path only for traversal detection. Require the raw candidate URL path to be contained under the raw configured base path before accepting it. This rejects prefix spoofs such as pre%66ix and prefix%2Fchild while preserving safe percent encodings, literal //, /./, and backslashes as object keys. Co-authored-by: Alex S. <alex.s@prefect.io> Co-Authored-By: alex.s <ajstreed1@gmail.com>
Hardens
RemoteFileSystem._resolve_pathso it no longer returns raw user input that downstream fsspec backends may interpret differently.Changes
scheme/netlocand a normalized path...and encoded variants (%2e%2e,.%2e,%2e.).urlsplit(e.g.\r,\n,\t) are not passed through.Tests
Validation
uv run pytest tests/test_filesystems.py -q— 73 passed, 1 skippeduv run ruff check src/prefect/filesystems.py tests/test_filesystems.py— passeduv run ruff format --check src/prefect/filesystems.py tests/test_filesystems.py— formattedgit diff --check— cleanSKIP=check-ui-v2 uv run pre-commit run --all-files— passedLink to Devin session: https://app.devin.ai/sessions/feb3ed2e92b14cfda4a3ad17019bd4a8
Requested by: @desertaxle