Conversation
There was a problem hiding this comment.
Pull request overview
Adds GIF support to the project dashboard “Add entry” image upload flow.
Changes:
- Allow
image/gifin the shared allowed image MIME type list. - Update the upload UI copy to mention GIFs.
- Adjust server-side upload handling to bypass
sharpfor GIF buffers.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/routes/dashboard/projects/[id]/config.ts |
Adds image/gif to ALLOWED_IMAGE_TYPES. |
src/routes/dashboard/projects/[id]/+page.svelte |
Updates helper/validation text to include GIFs. |
src/routes/dashboard/projects/[id]/+page.server.ts |
Uses the raw buffer for GIF uploads instead of always processing via sharp. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const imageBody = imageFile.type === 'image/gif' ? imageBuffer : await sharp(imageBuffer).toBuffer(); | ||
|
|
||
| const imageCommand = new PutObjectCommand({ | ||
| Bucket: env.S3_BUCKET_NAME, | ||
| Key: imagePath, | ||
| Body: await sharp(imageBuffer).toBuffer() | ||
| Body: imageBody |
There was a problem hiding this comment.
Skipping Sharp processing for GIFs means the server no longer validates that the uploaded bytes are actually an image; it relies solely on imageFile.type (client-controlled), so a forged MIME type could upload arbitrary content to the images/ bucket. It also bypasses the intended metadata-stripping step mentioned in the comment. Consider performing server-side type sniffing/decoding for GIFs as well (e.g., decode + re-encode with Sharp in animated mode, or validate via magic bytes) so invalid/non-image payloads are rejected and metadata is normalized.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| // Remove Exif metadata and save (we don't want another Hack Club classic PII leak :D) | ||
| const imageBuffer = Buffer.from(await imageFile.arrayBuffer()); | ||
|
|
||
| const imageBody = imageFile.type === 'image/gif' ? imageBuffer : await sharp(imageBuffer).toBuffer(); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
|
||
| let imageBody: Buffer; | ||
| try { | ||
| imageBody = imageFile.type === 'image/gif' ? imageBuffer : await sharp(imageBuffer).toBuffer(); |
There was a problem hiding this comment.
Bug: The image processing logic relies on the unreliable imageFile.type to identify GIFs. A mismatched type will cause sharp to process the GIF, stripping its animation.
Severity: MEDIUM
Suggested Fix
Instead of relying on the client-provided imageFile.type, perform file content validation using magic bytes to reliably determine the true file type on the server. Use this verified file type to decide whether to bypass the sharp processing logic for animated GIFs.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/routes/dashboard/projects/[id]/+page.server.ts#L218
Potential issue: The image processing logic determines whether to process an image with
`sharp` based on the client-provided `imageFile.type`. This property is unreliable and
can be easily spoofed or misreported by the browser. If a user uploads an animated GIF,
but the browser reports a different MIME type (e.g., `image/png`), the code will
incorrectly send the GIF data to `sharp` for processing. This will result in the
animation being stripped, and a static image will be saved instead. This undermines the
goal of supporting animated GIFs, as it can fail silently without triggering the
`try-catch` error handling.
1 participant
No description provided.