Validate uploaded audio files using magic bytes instead of Content-Type header - #111
Conversation
…r (Issue itzzavdhesh#70) File uploads are now validated against actual file contents using the file-type package to detect magic bytes. This prevents clients from spoofing the Content-Type header to bypass audio format validation. Changes: - Import fileTypeFromBuffer from file-type package - Add async magic byte validation in Multer fileFilter - Reject files where detected MIME type does not match audio format - Provide clear error message when file validation fails Security prevents: - Binary files (executables, images, etc.) disguised as audio - Content-Type header spoofing attacks - Invalid audio files reaching ElevenLabs API Fixes itzzavdhesh#70
|
@anshul23102 is attempting to deploy a commit to the itzzavdhesh's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Could the maintainers please add relevant labels? Suggested: type:security, severity:high, area:file-handling, nsoc |
📝 WalkthroughWalkthroughThe upload middleware now validates audio files using actual content inspection rather than relying solely on client-supplied MIME types. The ChangesAudio file content validation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
✍️ DCO Sign-off NeededHey @anshul23102! 👋 One or more commits in this PR are missing a Warning
How to fix: For the latest commit: git commit --amend --signoff
git push --force-with-leaseFor multiple commits, replace git rebase --signoff HEAD~N
git push --force-with-leaseThis comment will update automatically after you push. 🤖 VoiceForge Automation · Updates automatically on edits |
🛠️ PR Needs UpdatesHey @anshul23102! 👋 A few things need fixing before a mentor can review this PR. Warning
How to fix:
Once fixed, the workflow re-runs automatically and pings the right mentor. 🤖 VoiceForge Automation · Updates automatically on edits |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/middleware/upload.js`:
- Around line 12-25: The three plain Error instances thrown in the upload
middleware need HTTP status codes so the global error handler returns 4xx, not
500: when creating the "Please upload an audio recording." error set
error.status = 400 before calling callback(error), when creating the "Uploaded
file is not a valid audio recording..." error set error.status = 415
(Unsupported Media Type) before callback(error), and in the catch block wrap or
set error.status = 400 (or 415 if you prefer to treat detection failures as
media errors) before passing it to callback; locate these changes around the
fileTypeFromBuffer usage and the callback(...) calls in the upload middleware.
- Around line 10-19: The Multer fileFilter currently calls
fileTypeFromBuffer(file.buffer) but Multer invokes fileFilter before the file
buffer is populated, causing valid uploads to be rejected; move the magic-byte
validation out of fileFilter into a post-upload middleware that runs after
upload.single(...) (where req.file.buffer is available) or implement a
stream-based detection, and ensure any validation errors created (e.g., when
detectedType is missing or not audio) set err.status = 400 before passing to
next(err); update references: fileFilter, upload.single, req.file.buffer, and
fileTypeFromBuffer to implement this change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 65bfc890-827d-45d7-aea3-0f21caec86c5
📒 Files selected for processing (1)
server/middleware/upload.js
| fileFilter: async (_request, file, callback) => { | ||
| if (!file.mimetype.startsWith("audio/")) { | ||
| callback(new Error("Please upload an audio recording.")); | ||
| return; | ||
| } | ||
| callback(null, true); | ||
|
|
||
| try { | ||
| const detectedType = await fileTypeFromBuffer(file.buffer); | ||
| if (!detectedType || !detectedType.mime.startsWith("audio/")) { | ||
| callback(new Error("Uploaded file is not a valid audio recording. The file contents do not match an audio format.")); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Multer v2.0.2, what properties are guaranteed on the fileargument insidefileFilter? Specifically, is file.bufferavailable there, or only onreq.fileaftermemoryStorage has finished processing?
💡 Result:
In Multer v2.0.2, the file argument provided to the fileFilter function does not contain the file.buffer property [1][2]. The fileFilter function is designed to run before the file has been processed or fully read into memory/storage [1], meaning it only has access to the file's metadata, such as fieldname, originalname, encoding, and mimetype [3][4]. The file.buffer property is only available when using memoryStorage and is attached to the file object after the processing has been completed and the file is available in the request (e.g., in subsequent middleware or your route handler) [3][1][4]. Because fileFilter is intended to accept or reject a file based on its metadata before the upload process completes, the file contents are not yet available for inspection [1]. If you need to validate a file based on its contents (such as its magic bytes), you cannot do so directly inside fileFilter [1][2]; common workarounds include checking the file in a later middleware or route handler and deleting the file if it fails your criteria [1].
Citations:
- 1: Storage Engines, fileFilter, mixed behavour in v1/v2 and Docs expressjs/multer#1021
- 2: how read a file.buffer from multer file upload expressjs/multer#617
- 3: https://app.unpkg.com/multer@2.0.2/files/README.md
- 4: https://expressjs.com/en/resources/middleware/multer/
Fix: fileTypeFromBuffer(file.buffer) can’t run in Multer fileFilter (rejects valid audio)
In server/middleware/upload.js (fileFilter, around line 17), Multer v2 calls fileFilter before the file is read into memory; the file argument there does not include file.buffer. Move the magic-byte validation to a middleware that runs after upload.single(...) (where req.file.buffer exists with memoryStorage), or validate via a stream-based approach. Also set err.status = 400 for validation failures so clients don’t get 500s.
Suggested direction
const upload = multer({
storage: multer.memoryStorage(),
limits: {
fileSize: 12 * 1024 * 1024
},
- fileFilter: async (_request, file, callback) => {
+ fileFilter: (_request, file, callback) => {
if (!file.mimetype.startsWith("audio/")) {
- callback(new Error("Please upload an audio recording."));
+ const err = new Error("Please upload an audio recording.");
+ err.status = 400;
+ callback(err);
return;
}
-
- try {
- const detectedType = await fileTypeFromBuffer(file.buffer);
- if (!detectedType || !detectedType.mime.startsWith("audio/")) {
- callback(new Error("Uploaded file is not a valid audio recording. The file contents do not match an audio format."));
- return;
- }
- callback(null, true);
- } catch (error) {
- callback(new Error("Failed to validate file type: " + error.message));
- }
+ callback(null, true);
}
});
+
+export const validateAudioMagicBytes = async (request, _response, next) => {
+ try {
+ const detectedType = await fileTypeFromBuffer(request.file?.buffer);
+ if (!detectedType?.mime?.startsWith("audio/")) {
+ const err = new Error("Uploaded file is not a valid audio recording.");
+ err.status = 400;
+ return next(err);
+ }
+ return next();
+ } catch {
+ const err = new Error("Failed to validate file type.");
+ err.status = 400;
+ return next(err);
+ }
+};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fileFilter: async (_request, file, callback) => { | |
| if (!file.mimetype.startsWith("audio/")) { | |
| callback(new Error("Please upload an audio recording.")); | |
| return; | |
| } | |
| callback(null, true); | |
| try { | |
| const detectedType = await fileTypeFromBuffer(file.buffer); | |
| if (!detectedType || !detectedType.mime.startsWith("audio/")) { | |
| callback(new Error("Uploaded file is not a valid audio recording. The file contents do not match an audio format.")); | |
| fileFilter: (_request, file, callback) => { | |
| if (!file.mimetype.startsWith("audio/")) { | |
| const err = new Error("Please upload an audio recording."); | |
| err.status = 400; | |
| callback(err); | |
| return; | |
| } | |
| callback(null, true); | |
| } | |
| }); | |
| export const validateAudioMagicBytes = async (request, _response, next) => { | |
| try { | |
| const detectedType = await fileTypeFromBuffer(request.file?.buffer); | |
| if (!detectedType?.mime?.startsWith("audio/")) { | |
| const err = new Error("Uploaded file is not a valid audio recording."); | |
| err.status = 400; | |
| return next(err); | |
| } | |
| return next(); | |
| } catch { | |
| const err = new Error("Failed to validate file type."); | |
| err.status = 400; | |
| return next(err); | |
| } | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/middleware/upload.js` around lines 10 - 19, The Multer fileFilter
currently calls fileTypeFromBuffer(file.buffer) but Multer invokes fileFilter
before the file buffer is populated, causing valid uploads to be rejected; move
the magic-byte validation out of fileFilter into a post-upload middleware that
runs after upload.single(...) (where req.file.buffer is available) or implement
a stream-based detection, and ensure any validation errors created (e.g., when
detectedType is missing or not audio) set err.status = 400 before passing to
next(err); update references: fileFilter, upload.single, req.file.buffer, and
fileTypeFromBuffer to implement this change.
| callback(new Error("Please upload an audio recording.")); | ||
| return; | ||
| } | ||
| callback(null, true); | ||
|
|
||
| try { | ||
| const detectedType = await fileTypeFromBuffer(file.buffer); | ||
| if (!detectedType || !detectedType.mime.startsWith("audio/")) { | ||
| callback(new Error("Uploaded file is not a valid audio recording. The file contents do not match an audio format.")); | ||
| return; | ||
| } | ||
| callback(null, true); | ||
| } catch (error) { | ||
| callback(new Error("Failed to validate file type: " + error.message)); | ||
| } |
There was a problem hiding this comment.
Validation failures are returned as 500 instead of 4xx
Lines 12, 19, and 24 create plain Error objects without status; with the global error handler, these become HTTP 500 responses for client-side invalid uploads. Set error.status = 400 (or 415) before passing to callback.
Minimal fix
- callback(new Error("Please upload an audio recording."));
+ const err = new Error("Please upload an audio recording.");
+ err.status = 400;
+ callback(err);
return;
@@
- callback(new Error("Uploaded file is not a valid audio recording. The file contents do not match an audio format."));
+ const err = new Error("Uploaded file is not a valid audio recording. The file contents do not match an audio format.");
+ err.status = 400;
+ callback(err);
return;
}
@@
- callback(new Error("Failed to validate file type: " + error.message));
+ const err = new Error("Failed to validate file type.");
+ err.status = 400;
+ callback(err);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/middleware/upload.js` around lines 12 - 25, The three plain Error
instances thrown in the upload middleware need HTTP status codes so the global
error handler returns 4xx, not 500: when creating the "Please upload an audio
recording." error set error.status = 400 before calling callback(error), when
creating the "Uploaded file is not a valid audio recording..." error set
error.status = 415 (Unsupported Media Type) before callback(error), and in the
catch block wrap or set error.status = 400 (or 415 if you prefer to treat
detection failures as media errors) before passing it to callback; locate these
changes around the fileTypeFromBuffer usage and the callback(...) calls in the
upload middleware.
There was a problem hiding this comment.
3 issues found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="server/middleware/upload.js">
<violation number="1" location="server/middleware/upload.js:3">
P1: New `file-type` import is not declared in dependencies, which will break runtime module resolution.</violation>
<violation number="2" location="server/middleware/upload.js:17">
P1: `fileFilter` tries to read `file.buffer` before Multer has produced it, so uploads fail validation.</violation>
<violation number="3" location="server/middleware/upload.js:19">
P2: Validation errors created here (and at the other `callback(new Error(...))` call sites) lack a `status` property. If the global error handler defaults to 500 for errors without an explicit status, clients will receive 500 Internal Server Error for invalid uploads instead of 400 Bad Request. Set `err.status = 400` before passing to the callback.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| callback(null, true); | ||
|
|
||
| try { | ||
| const detectedType = await fileTypeFromBuffer(file.buffer); |
There was a problem hiding this comment.
P1: fileFilter tries to read file.buffer before Multer has produced it, so uploads fail validation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/middleware/upload.js, line 17:
<comment>`fileFilter` tries to read `file.buffer` before Multer has produced it, so uploads fail validation.</comment>
<file context>
@@ -1,17 +1,28 @@
- callback(null, true);
+
+ try {
+ const detectedType = await fileTypeFromBuffer(file.buffer);
+ if (!detectedType || !detectedType.mime.startsWith("audio/")) {
+ callback(new Error("Uploaded file is not a valid audio recording. The file contents do not match an audio format."));
</file context>
| @@ -1,17 +1,28 @@ | |||
| // Configures Multer for in-memory reference audio uploads sent to ElevenLabs. | |||
| import multer from "multer"; | |||
| import { fileTypeFromBuffer } from "file-type"; | |||
There was a problem hiding this comment.
P1: New file-type import is not declared in dependencies, which will break runtime module resolution.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/middleware/upload.js, line 3:
<comment>New `file-type` import is not declared in dependencies, which will break runtime module resolution.</comment>
<file context>
@@ -1,17 +1,28 @@
// Configures Multer for in-memory reference audio uploads sent to ElevenLabs.
import multer from "multer";
+import { fileTypeFromBuffer } from "file-type";
const upload = multer({
</file context>
| try { | ||
| const detectedType = await fileTypeFromBuffer(file.buffer); | ||
| if (!detectedType || !detectedType.mime.startsWith("audio/")) { | ||
| callback(new Error("Uploaded file is not a valid audio recording. The file contents do not match an audio format.")); |
There was a problem hiding this comment.
P2: Validation errors created here (and at the other callback(new Error(...)) call sites) lack a status property. If the global error handler defaults to 500 for errors without an explicit status, clients will receive 500 Internal Server Error for invalid uploads instead of 400 Bad Request. Set err.status = 400 before passing to the callback.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/middleware/upload.js, line 19:
<comment>Validation errors created here (and at the other `callback(new Error(...))` call sites) lack a `status` property. If the global error handler defaults to 500 for errors without an explicit status, clients will receive 500 Internal Server Error for invalid uploads instead of 400 Bad Request. Set `err.status = 400` before passing to the callback.</comment>
<file context>
@@ -1,17 +1,28 @@
+ try {
+ const detectedType = await fileTypeFromBuffer(file.buffer);
+ if (!detectedType || !detectedType.mime.startsWith("audio/")) {
+ callback(new Error("Uploaded file is not a valid audio recording. The file contents do not match an audio format."));
+ return;
+ }
</file context>
| callback(new Error("Uploaded file is not a valid audio recording. The file contents do not match an audio format.")); | |
| const err = new Error("Uploaded file is not a valid audio recording. The file contents do not match an audio format."); | |
| err.status = 400; | |
| callback(err); |
|
Please add relevant labels:
These help with tracking and prioritization. Thank you! |
❌ Merge Policy ViolationCaution Unauthorized Merge — Pull request #111 was merged by @Itzzavdheshh (mentor) without any review on record. VoiceForge guidelines require contributors/mentors to submit at least one review (approval, comment, or changes requested) before merging a pull request to ensure code quality and point-tracking integrity. 📊 Violation Summary
🤖 VoiceForge Automation |
🎊 PR Merged SuccessfullyHey @anshul23102! 👋 Congratulations and thank you for your contribution to VoiceForge! Note 🔗 Linked issue(s): #70 · ✅ Marked as merged and complete Maintainers may still handle final cleanup, release notes, or follow-up tracking after the merge. 🤖 VoiceForge Automation · Updates automatically on edits |
Validate uploaded audio files using magic bytes instead of Content-Type header
Implements file type validation using magic byte signatures to prevent spoofed audio uploads that bypass Content-Type checks. Ensures only valid audio formats (WebM, MP3, WAV, OGG) are accepted.
Changes
server/middleware/upload.jsKey Features
Test Plan
Fixes #70
🤖 Generated with Claude Code