Conversation
Summary of ChangesHello @gantoine, 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 introduces a significant architectural improvement by standardizing how form data and JSON payloads are handled in the backend API. By moving from manual request parsing to explicit FastAPI dependencies and Pydantic models, the API becomes more robust, type-safe, and self-documenting. The frontend has been comprehensively updated to consume these refined API definitions, ensuring seamless integration and leveraging the benefits of strong typing for API interactions. 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
|
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly refactors the backend API by replacing manual parsing with typed fields using FastAPI's Form and Pydantic models, which enhances type safety, validation, and API documentation. The frontend has been updated accordingly, including a new buildFormInput utility. However, critical security vulnerabilities were identified: Path Traversal due to a lack of sanitization for user-provided filenames in upload endpoints, and Server-Side Request Forgery (SSRF) from insufficient validation of remote URLs used to fetch covers and manuals. These could allow arbitrary file writes or access to internal network resources. It is recommended to apply filename sanitization and URL validation across all affected endpoints. Additionally, an issue was found in the frontend's updateUser API call where multipart/form-data is likely being sent with an incorrect body format.
| url_cover: str = Form( | ||
| default="", description="Remote URL to fetch and use as cover artwork." | ||
| ), |
There was a problem hiding this comment.
The url_cover parameter is used to fetch remote content without validation in the add_collection endpoint. This can be exploited to perform Server-Side Request Forgery (SSRF). It is recommended to validate the URL scheme (e.g., allow only http/https) and ensure the host is not an internal IP address or a local hostname.
| url_cover: str | None = None | ||
| url_manual: str | None = None |
There was a problem hiding this comment.
The url_cover and url_manual parameters are used to fetch remote content without proper validation, leading to potential SSRF. An attacker could provide URLs pointing to internal services or metadata endpoints. Ensure that provided URLs are validated against an allow-list of schemes and that internal network addresses are blocked.
| saveFile: UploadFile | None = File( | ||
| default=None, description="Save file to upload." | ||
| ), | ||
| screenshotFile: UploadFile | None = File( | ||
| default=None, description="Screenshot file associated with this save." | ||
| ), |
There was a problem hiding this comment.
The saveFile and screenshotFile parameters are introduced as UploadFile objects. Their filename attribute is used directly to construct file paths on the server without sanitization (e.g., in line 189 and 260 of the full file). An attacker can provide a filename containing .. sequences to perform a path traversal attack and potentially write files to unauthorized locations. Use a sanitization function like sanitize_filename before using the filename in path construction.
| screenshotFile: UploadFile | None = File( | ||
| default=None, description="Screenshot file to upload." | ||
| ), |
There was a problem hiding this comment.
The screenshotFile parameter is used without filename sanitization, leading to a potential path traversal vulnerability when the file is written to disk. Always sanitize user-provided filenames to prevent directory traversal attacks.
| stateFile: UploadFile | None = File( | ||
| default=None, description="State file to upload." | ||
| ), | ||
| screenshotFile: UploadFile | None = File( | ||
| default=None, description="Screenshot file associated with this state." | ||
| ), |
There was a problem hiding this comment.
The stateFile and screenshotFile parameters are used without filename sanitization, allowing for path traversal attacks during file upload. Ensure that all uploaded filenames are sanitized to remove directory traversal sequences like ...
1 participant
Description
Explain the changes or enhancements you are proposing with this pull request.
Checklist
Please check all that apply.
Screenshots (if applicable)