Uh oh!
There was an error while loading. Please reload this page.
fix(uploads): allow images/video/audio in mothership presigned route - #4534
Conversation
…gned route The mothership branch of the presigned upload route called validateFileType, which only permits SUPPORTED_DOCUMENT_EXTENSIONS — rejecting PNG screenshots and other media users have always been able to attach via the legacy /api/files/upload mothership branch. Introduce validateAttachmentFileType, backed by the union of document, code, image, audio, and video extensions, and wire it into the mothership branch. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Introduces Expands API/unit test coverage to assert the correct validator is used per upload type (attachments vs knowledge-base), validate permission failures, and exercise new attachment validation behavior and error payloads. Reviewed by Cursor Bugbot for commit a70f6f2. Configure here. |
Greptile SummaryThis PR fixes a regression introduced in #4509 where the presigned URL route rejected image, audio, and video files for mothership and execution uploads because it used
Confidence Score: 5/5Safe to merge — the change is a targeted fix that restores the intended attachment-type permissions for mothership and execution uploads with no impact on other upload paths. The change is narrow in scope: two call sites swap to a new validator, the new validator is backed by the same extension sets already used in the legacy route, deduplication is handled at construction time and covered by a test, and the permission gates for both affected branches are untouched. No new execution paths are opened; knowledge-base validation is deliberately unchanged. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[POST /api/files/presigned] --> B{uploadType}
B -->|knowledge-base| C[validateFileType\ndocs only + MIME check]
B -->|mothership| D{permission\nwrite or admin?}
B -->|execution| E{permission\nwrite or admin?}
B -->|workspace-logos| F[isImageFileType\nMIME check]
B -->|profile-pictures| G[isImageFileType\nMIME check]
B -->|copilot| H[CopilotFiles.generateCopilotUploadUrl\ninternal validation]
B -->|chat| I[No file-type validation]
D -->|yes| J[validateAttachmentFileType\nall ext, no MIME]
D -->|no| K[403]
E -->|yes| L[validateAttachmentFileType\nall ext, no MIME]
E -->|no| M[403]
C --> N{error?}
J --> O{error?}
L --> P{error?}
N -->|yes| Q[400 VALIDATION_ERROR]
O -->|yes| Q
P -->|yes| Q
N -->|no| R[generatePresignedUploadUrl]
O -->|no| R
P -->|no| R
F --> R
G --> R
H --> R
I --> R
style J fill:#90EE90
style L fill:#90EE90
Reviews (3): Last reviewed commit: "fix(uploads): restore SUPPORTED_IMAGE_EX..." | Re-trigger Greptile |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
The execution branch of the presigned upload route called validateFileType, which only permits documents — but workflow execution outputs are arbitrary by design (images, audio, video, code). The legacy /api/files/upload execution branch had no docs-only gate, so the staging refactor to presigned PUTs regressed parity. Switch execution to validateAttachmentFileType to match prior behavior. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…sions - Dedupe SUPPORTED_ATTACHMENT_EXTENSIONS (webm appears in both audio and video lists) - Reuse SUPPORTED_ATTACHMENT_EXTENSIONS in /api/files/upload to avoid drift with the presigned route - Add unit tests for validateAttachmentFileType - Add presigned route tests covering mothership/execution/knowledge-base validator selection and permission gating
waleedlatif1
commented
May 9, 2026
waleedlatif1
commented
May 9, 2026
@cursor review |
There was a problem hiding this comment.
✅ 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 93b1565. Configure here.
Build broke because biome auto-fix collapsed my multi-import edit and dropped SUPPORTED_IMAGE_EXTENSIONS, which is still referenced for the generic-MIME image fallback at line 298.
waleedlatif1
commented
May 9, 2026
waleedlatif1
commented
May 9, 2026
@cursor review |
There was a problem hiding this comment.
✅ 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 a70f6f2. Configure here.
Uh oh!
There was an error while loading. Please reload this page.
Summary
/api/files/presignedcallsvalidateFileType, which only permitsSUPPORTED_DOCUMENT_EXTENSIONS. That rejects PNG screenshots (and any image/video/audio) at the new presigned upload path introduced in improvement(uploads): migrate remaining FormData uploads to presigned PUT #4509, even though Mothership has always supported media attachments via the legacy/api/files/uploadroute.validateAttachmentFileTypeinlib/uploads/utils/validation.ts, backed by the union of document, code, image, audio, and video extension sets, and wire it into the mothership branch. Behavior now matches what/api/files/uploadalready enforced.Test plan
.pngscreenshot into Mothership chat input on staging — uploads successfully.mp4/.mp3— uploads successfully.exe— rejected with a clear errorbun run check:api-validationpasses