Skip to content

fix(files): align upload route image extensions with picker#4423

Merged
waleedlatif1 merged 7 commits intostagingfrom
waleedlatif1/files-upload-images
May 3, 2026
Merged

fix(files): align upload route image extensions with picker#4423
waleedlatif1 merged 7 commits intostagingfrom
waleedlatif1/files-upload-images

Conversation

@waleedlatif1
Copy link
Copy Markdown
Collaborator

Summary

  • /api/files/upload had a stale local IMAGE_EXTENSIONS = ['png','jpg','jpeg','gif','webp','svg'] that didn't match the new exported SUPPORTED_IMAGE_EXTENSIONS (which adds bmp/tif/tiff/heic/heif/avif/ico)
  • chat / copilot / knowledge / file-upload-block flows hit this route — users could select the new image types in the picker but uploads would fail with "File type not allowed"
  • replaced the local constant with the imported SUPPORTED_IMAGE_EXTENSIONS so the upload route accepts the same set the picker offers

Type of Change

  • Bug fix

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link
Copy Markdown

vercel Bot commented May 3, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped May 3, 2026 10:28am

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented May 3, 2026

PR Summary

Medium Risk
Touches the server upload route’s file-type validation and Content-Type handling, which can affect what files are accepted/stored and how downstream consumers interpret uploads. Scope is limited to specific upload contexts and MIME/extension mapping changes.

Overview
Fixes /api/files/upload rejecting newly-supported image extensions by replacing a stale local image-extension allowlist with shared SUPPORTED_IMAGE_EXTENSIONS.

Tightens image-only context validation (chat, profile-pictures, workspace-logos) to fall back to extension checks when the browser reports a generic/empty MIME type, and standardizes stored contentType/returned fileInfo.type using resolveFileType.

Extends file-utils MIME/extension mappings to include additional image formats (e.g. bmp, tiff, heic/heif, avif, ico) and prevents non-Claude-supported image MIME types from being converted into Claude message image content.

Reviewed by Cursor Bugbot for commit 6abf6c2. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 3, 2026

Greptile Summary

This PR replaces the stale local IMAGE_EXTENSIONS list in the upload route with the shared SUPPORTED_IMAGE_EXTENSIONS constant, adds a MIME-type fallback for browsers that report application/octet-stream (e.g. HEIC/TIFF on non-Apple platforms), and guards createFileContent from emitting Claude-incompatible image blocks — all of which address bugs confirmed in prior review rounds.

Confidence Score: 5/5

Safe to merge — all previously identified P1 bugs are resolved and no new defects were introduced

The two confirmed P1 issues from prior review rounds are both correctly addressed. The only remaining finding is a P2 style concern about constant declaration order. No security issues, no broken code paths.

No files require special attention beyond the P2 style suggestion in file-utils.ts

Important Files Changed

Filename Overview
apps/sim/lib/uploads/utils/file-utils.ts Adds new image MIME types to MIME_TYPE_MAPPING and EXTENSION_TO_MIME/MIME_TO_EXTENSION, guards createFileContent against non-Claude-supported images; CLAUDE_SUPPORTED_IMAGE_MIME_TYPES is declared after the function that uses it (TDZ risk if called at module init time)
apps/sim/app/api/files/upload/route.ts Replaces stale local IMAGE_EXTENSIONS with imported SUPPORTED_IMAGE_EXTENSIONS, adds MIME fallback for generic octet-stream types, and resolves content-type before storage upload — logic is correct and addresses prior review feedback

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[POST /api/files/upload] --> B{validateFileExtension\nSUPPORTED_IMAGE_EXTENSIONS}
    B -- fails --> Z1[400 File type not allowed]
    B -- passes --> C{context?}
    C -- copilot --> E[Skip MIME check]
    C -- chat / profile-pictures\nworkspace-logos --> D{isGenericMime?}
    D -- yes octet-stream or empty --> F[extensionIsImage check\nvs SUPPORTED_IMAGE_EXTENSIONS]
    D -- no specific MIME --> G[isImageFileType MIME check\nvs MIME_TYPE_MAPPING]
    F -- false --> Z2[400 Only image files allowed]
    G -- false --> Z2
    F -- true --> E
    G -- true --> E
    E --> H[resolveFileType\nbrowser MIME → extension fallback]
    H --> I[storageService.uploadFile\nwith resolvedContentType]
    I --> J[Return fileInfo with resolvedContentType]
Loading

Reviews (6): Last reviewed commit: "fix(files): cover image/jpg in Claude ga..." | Re-trigger Greptile

Comment thread apps/sim/app/api/files/upload/route.ts
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile-apps Good catch on the secondary MIME gate — fixed in 09618ed. Added the new image MIME types (image/bmp, image/tiff, image/heic, image/heif, image/avif, image/x-icon, image/vnd.microsoft.icon) to MIME_TYPE_MAPPING in file-utils.ts, so isImageFileType now accepts them and the chat/profile-pictures/workspace-logos contexts no longer reject the new formats. Also updated the stale error message that hardcoded the old type list.

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 3, 2026

The fix looks correct. The 7 new MIME types are properly added to MIME_TYPE_MAPPING, and since IMAGE_MIME_TYPES is derived dynamically:

const IMAGE_MIME_TYPES = new Set(
  Object.entries(MIME_TYPE_MAPPING)
    .filter(([, v]) => v === 'image')
    .map(([k]) => k)
)

...isImageFileType() automatically picks them all up without any further changes needed. The two-gate fix is now complete and consistent.

One minor gap worth being aware of: EXTENSION_TO_MIME (used by resolveFileType as a fallback when the browser reports application/octet-stream or an empty type) doesn't have entries for bmp, tif, tiff, heic, heif, avif, or ico. Modern browsers reliably report MIME types for these formats, so it's unlikely to be a practical issue — but if you ever want full coverage, adding those extension mappings would close the last edge case.

LGTM overall.

Comment thread apps/sim/app/api/files/upload/route.ts Outdated
…y uploads

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/lib/uploads/utils/file-utils.ts
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Comment thread apps/sim/lib/uploads/utils/file-utils.ts
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 7377716. Configure here.

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/app/api/files/upload/route.ts
… generic

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

Comment thread apps/sim/lib/uploads/utils/file-utils.ts
Comment thread apps/sim/lib/uploads/utils/file-utils.ts
Comment thread apps/sim/app/api/files/upload/route.ts
…etadata

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

@waleedlatif1 waleedlatif1 merged commit 0f09310 into staging May 3, 2026
10 checks passed
@waleedlatif1 waleedlatif1 deleted the waleedlatif1/files-upload-images branch May 3, 2026 10:32
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 6abf6c2. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant