fix: limit file type in file picker in everything app#526
fix: limit file type in file picker in everything app#526tibisabau wants to merge 1 commit intoalpic-ai:mainfrom
Conversation
| <input | ||
| ref={inputRef} | ||
| type="file" | ||
| accept="image/png,image/jpeg,image/webp" |
There was a problem hiding this comment.
Hardcoded accept string instead of using the exported constant
The accept attribute duplicates the string "image/png,image/jpeg,image/webp" rather than importing and using the SUPPORTED_FILE_TYPES_ACCEPT constant that was specifically created for this purpose. If supported MIME types are updated in the future, this attribute won't automatically reflect the change, and the file picker can silently diverge from the server-side validation.
Since useFiles is already imported from skybridge/web, SUPPORTED_FILE_TYPES_ACCEPT can be imported from the same package:
import { SUPPORTED_FILE_TYPES_ACCEPT, useFiles } from "skybridge/web";
| accept="image/png,image/jpeg,image/webp" | |
| accept={SUPPORTED_FILE_TYPES_ACCEPT} |
Prompt To Fix With AI
This is a comment left during a code review.
Path: examples/everything/web/src/widgets/tabs/use-files-tab.tsx
Line: 58
Comment:
**Hardcoded accept string instead of using the exported constant**
The `accept` attribute duplicates the string `"image/png,image/jpeg,image/webp"` rather than importing and using the `SUPPORTED_FILE_TYPES_ACCEPT` constant that was specifically created for this purpose. If supported MIME types are updated in the future, this attribute won't automatically reflect the change, and the file picker can silently diverge from the server-side validation.
Since `useFiles` is already imported from `skybridge/web`, `SUPPORTED_FILE_TYPES_ACCEPT` can be imported from the same package:
```
import { SUPPORTED_FILE_TYPES_ACCEPT, useFiles } from "skybridge/web";
```
```suggestion
accept={SUPPORTED_FILE_TYPES_ACCEPT}
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
This does not compile with the current package.json configuration in the everything folder.
There was a problem hiding this comment.
You're right. While SUPPORTED_FILE_TYPES_ACCEPT is exported in this PR (see packages/core/src/web/hooks/index.ts), the everything app references the built package at dist/web/index.js, which won't include the new exports until you run pnpm build from the monorepo root.
For an example app, hardcoding the accept attribute is perfectly acceptable — it's just demonstrating the feature. The important validation logic is in use-files.ts where it matters.
If you want to use the exported constant during development, you'd need to rebuild the skybridge package first (pnpm build), but that's probably overkill for this context.
There was a problem hiding this comment.
Actually yes that's a change we should do in the future (when the package will be published) !
|
Thanks for contributing @tibisabau ! Indeed currently there is no safety net if the user upload other files than images. I'm currently having a small after thought on this should live in the API (throwing an error is a bit hard). One ideas I had was :
That being said, @fredericbarthelet was suggesting to emit an error so just wanted to share my thoughts here :) |
|
Hi @tibisabau ! We just noticed that OpenAI remove his file type limitation in the doc : https://developers.openai.com/apps-sdk/build/chatgpt-ui#upload-files-from-the-widget-chatgpt-extension I'm going to try with other files but that would make this PR obsolete ! |
Description
This pull request addresses issue #368 by implementing proper file type validation for the file upload functionality in both the Everything app and the useFiles hook. The changes restrict file uploads to only the 3 image file types supported by ChatGPT's file upload API, preventing cryptic backend errors and improving user experience.
According to the ChatGPT UI documentation,
window.openai.uploadFile()currently supports only:image/pngimage/jpegimage/webpThe implementation adds:
The validation is case-insensitive and covers all image format variations that ChatGPT supports (.jpg, .jpeg, .png, .webp).
Closes #368.
The code in this pull request was generated by GitHub Copilot with the Claude Sonnet 4.5 model.
Checklist if Applicable
pnpm testexecuted successfully in packages/coreGreptile Summary
This PR restricts file uploads to the three image types supported by ChatGPT's
window.openai.uploadFile()API (image/png,image/jpeg,image/webp) by adding avalidateFileTypehelper inuseFilesand anacceptattribute to the Everything-app file picker. The tests are well-structured and cover the full range of validation cases.One maintenance concern:
acceptattribute inuse-files-tab.tsx(line 58) hardcodes"image/png,image/jpeg,image/webp"verbatim instead of importing the newly-exportedSUPPORTED_FILE_TYPES_ACCEPTconstant. If the supported types are extended inuse-files.ts, the file picker's filter won't update automatically, silently allowing the UI restriction to drift from the server-side validation.Positive notes:
beforeEach/afterEachandAppsSdkAdaptor.resetInstance(), eliminating shared state between tests..png) avoiding false-positive suffix matches.uploadwrapper callsadaptor.uploadFile(file)as a method invocation (preservingthiscontext), which is a subtle improvement over the prior bare method reference.Confidence Score: 4/5
use-files.tsis correct, well-tested, and cleanly separated. The sole concern is thatuse-files-tab.tsxhardcodes the accept MIME-type string rather than using the exportedSUPPORTED_FILE_TYPES_ACCEPTconstant, creating a potential drift risk. This is a style-level issue in an example app and does not affect correctness or security today.acceptstring should use the exported constant.Last reviewed commit: bcdae51
(5/5) You can turn off certain types of comments like style here!