fix(storage): handle OSS folder marker objects in download_folder#454
fix(storage): handle OSS folder marker objects in download_folder#454rayrayraykk merged 1 commit intoagentscope-ai:mainfrom
Conversation
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 critical bug 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 effectively addresses the issue of OSSStorage.download_folder() incorrectly handling OSS folder marker objects by adding a check for object keys ending with a slash, preventing potential file/directory name conflicts during folder downloads. However, the OSSStorage.download_folder function remains vulnerable to a path traversal attack. It does not sanitize or validate the object keys retrieved from OSS before using them to construct local file paths, which could allow an attacker with control over the OSS bucket to write files to arbitrary locations on the local filesystem.
Summary
Fixes
OSSStorage.download_folder()incorrectly treating OSS "folder marker" objects (keys ending with/, often zero-byte objects) as regular files.When a bucket contains both:
dir/(marker object)dir/file.txt(real file)the previous implementation would download
dir/to a local file nameddir, and then later fail when trying to create the directorydir/fordir/file.txt(file/dir name conflict).Root cause
download_folder()only checkedobj.is_prefix()to decide whether an entry should be created as a local directory.However, folder marker objects like
dir/are real objects inobject_list(not necessarily common-prefix entries), so they may not satisfyis_prefix()and were handled as files.Fix
Treat objects with keys ending in
/as directories:This preserves empty directories represented by marker objects and prevents local path conflicts.
How to reproduce
<prefix>dir/(empty object)<prefix>dir/file.txtdownload_folder(<prefix>, <local_dir>)Before this patch: may fail due to
dirbeing created as a file first.After this patch:
dir/becomes a local directory anddir/file.txtdownloads correctly.Related issue
Fixes #450