fix(datatrak): RN-1715: enforce allowlist of supported file types for Photo questions #6502
Conversation
Summary of ChangesHello @jaskfla, 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 enhances the robustness and user experience of image uploads by implementing a comprehensive allowlist for supported image file types. It prevents users from selecting unsupported files at the frontend and ensures that the backend strictly validates and rejects any unsupported media types, improving data integrity and system stability. Highlights
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
|
jaskfla
force-pushed
the
rn-1715-img-upload
branch
from
135053b to
9beb90b
Compare
![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 introduces a strict allowlist for image uploads, enhancing security and robustness. The changes are well-implemented across both backend and frontend. On the backend, S3Client.ts is updated to validate image MIME types against a defined list, throwing a new UnsupportedMediaTypeError for invalid types. On the frontend, the ImageUploadField.tsx component now uses the accept attribute to guide users to select supported file types. My review includes a few suggestions to improve maintainability, type safety, and API consistency.
| } | ||
| } | ||
|
|
||
| export class UnsupportedMediaTypeError extends RespondingError { |
There was a problem hiding this comment.
The current usage of this actually gets swallowed and rethrown as a 500 Internal Server Error. Not fixing that for now, but I’d like this error to be ready anyway
There was a problem hiding this comment.
Yeah I like this call, good to have it in place 👍
|
bugbot run |
jaskfla
force-pushed
the
rn-1715-img-upload
branch
from
4b8c3f1 to
1ac1ade
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
jaskfla
force-pushed
the
rn-1715-img-upload
branch
from
1ac1ade to
a43d10e
Compare
jaskfla
force-pushed
the
rn-1715-img-upload
branch
from
d009469 to
ccbbd06
Compare
There was a problem hiding this comment.
Looks good! Thanks for cleaning up the code while you're at it 🙏
I was looking around for some unit tests around this but couldn't find any. Any chance you'd be happy to put something basic in place around the S3Client? Seems like we've got a bit of business logic growing in here.
| function isBase64DataUri(val: string): val is `data:${string};base64,${string}` { | ||
| return val.startsWith('data:') && val.includes(';base64,'); | ||
| } | ||
|
|
||
| function isImageMediaTypeString(val: string): val is `image/${string}` { | ||
| // Check length because 'image/' alone is invalid | ||
| return val.length > 'image/'.length && val.startsWith('image/'); | ||
| } |
There was a problem hiding this comment.
Oooh nice type literal typeguards!!
| } | ||
| } | ||
|
|
||
| export class UnsupportedMediaTypeError extends RespondingError { |
There was a problem hiding this comment.
Yeah I like this call, good to have it in place 👍
2 participants
RN-1715
Stack
This is PR 1 of 2 in a stack:
Note
Restricts uploads to specific web-safe image types end-to-end, adds strict base64/content-type validation with 415 errors, and updates the file input to only allow those types.
contentTypeprecisely; improved error on invalid data URIs.avif,gif,jpeg/jpg,png,svg+xml,webp); throwUnsupportedMediaTypeError(415) with human-readable list.accept.aria-label, concise size error messages, safer refs/file extraction).UnsupportedMediaTypeError(415).Written by Cursor Bugbot for commit 1ac1ade. This will update automatically on new commits. Configure here.