fix(transfer_manager): Prevent path traversal in download_many_to_path#1768
fix(transfer_manager): Prevent path traversal in download_many_to_path#1768chandra-siri merged 14 commits intomainfrom
download_many_to_path#1768Conversation
user provided destination_dir
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a security issue in the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
The pull request introduces a Path object and a _resolve_path function to resolve the destination path for downloaded blobs in the download_many_to_path function, preventing path traversal vulnerabilities. The tests were updated to reflect the change in destination path. Review feedback indicates that the implementation of _resolve_path is incorrect for relative paths and will raise a ValueError when blob_path is a relative path. Also, the pathlib.Path.is_relative_to() method was introduced in Python 3.9, and since this library supports Python 3.7+, using this method will cause an AttributeError on Python versions 3.7 and 3.8. The traceback module is imported but never used in the file. The TODO comment should be replaced with a concrete example before merging. The docstring for destination_directory contains a TODO comment, a reference to the old os.path.join implementation, and a redundant parenthetical note. The expression Path(destination_directory).resolve() is evaluated on every iteration of the loop.
download_many_to_path
PR created by the Librarian CLI to initialize a release. Merging this PR will auto trigger a release. Librarian Version: v1.0.2-0.20251119154421-36c3e21ad3ac Language Image: us-central1-docker.pkg.dev/cloud-sdk-librarian-prod/images-prod/python-librarian-generator@sha256:8e2c32496077054105bd06c54a59d6a6694287bc053588e24debe6da6920ad91 <details><summary>google-cloud-storage: 3.10.0</summary> ## [3.10.0](v3.9.0...v3.10.0) (2026-03-18) ### Features * [Bucket Encryption Enforcement] add support for bucket encryption enforcement config (#1742) ([2a6e8b0](2a6e8b0)) ### Perf Improvments * [Rapid Buckets Reads] Use raw proto access for read resumption strategy (#1764) ([14cfd61](14cfd61)) * [Rapid Buckets Benchmarks] init mp pool & grpc client once, use os.sched_setaffinity (#1751) ([a9eb82c](a9eb82c)) * [Rapid Buckets Writes] don't flush at every append, results in bad perf (#1746) ([ab62d72](ab62d72)) ### Bug Fixes * [Windows] skip downloading blobs whose name contain `":" ` eg: `C:` `D:` etc when application runs in Windows. (#1774) ([5581988](5581988)) * [Path Traversal] Prevent path traversal in `download_many_to_path` (#1768) ([700fec3](700fec3)) * [Rapid Buckets] pass token correctly, '&' instead of ',' (#1756) ([d8dd1e0](d8dd1e0)) </details>
fix(transfer_manager): Prevent path traversal in download_many_to_path
This PR addresses a security vulnerability where
download_many_to_pathcould be exploited to write files outside the intended destination directory.The fix ensures that the resolved path for each blob download remains within the bounds of the user-provided
destination_directory. If a blob name would result in a path outside this directory (e.g., by using../), a warning is issued, and that specific blob download is skipped. This prevents directory traversal attacks.Absolute paths in blob names (e.g.,
/etc/passwd) are now treated as relative to thedestination_directory, so/etc/passwdwill be downloaded todestination_directory/etc/passwd.See b/449616593 for more details.
BREAKING CHANGE: Blobs that would resolve to a path outside the
destination_directoryare no longer downloaded. While this is a security fix, users relying on the previous behavior to write files outside the target directory will see a change.