Add MIME type and extension validation for file uploads #5596
+273
−9
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Updated jest.config.js to include the 'src' directory in test roots. - Introduced a new validator for checking MIME type and file extension matches, addressing security vulnerabilities. - Implemented validation in multiple server controllers and utilities to prevent MIME type spoofing attacks. - Added comprehensive tests for the new validation logic in validator.test.ts.
Contributor
Summary of ChangesHello @jocelynlin-wd, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the security of file uploads by introducing a comprehensive validation mechanism. It directly addresses a reported vulnerability (GHSA-j8g8-j7fc-43v6) by preventing MIME type spoofing and path traversal attacks. The new validation ensures that uploaded files have appropriate extensions matching their declared MIME types and are free from malicious path components. This logic is integrated across several file handling services, and a cleanup process is added to remove invalid files from storage, bolstering the system's integrity against malicious uploads. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request effectively addresses a security vulnerability by introducing robust MIME type and file extension validation for file uploads. The new validator is well-implemented and comes with a comprehensive suite of tests, covering many edge cases. The validation has been consistently applied across all relevant parts of the application that handle file uploads. Additionally, the new logic to clean up files from storage when a loader fails is a great improvement for system hygiene. I have a few minor suggestions to improve code consistency and error message accuracy.
isUnsafeFilePath function checks for various security risks, including absolute paths, null bytes, and control characters, not just path traversal. A more general error message would be more appropriate to cover all cases. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
jocelynlin-wd
commented
Dec 18, 2025
| preset: 'ts-jest', | ||
| testEnvironment: 'node', | ||
| roots: ['<rootDir>/nodes'], | ||
| roots: ['<rootDir>/nodes', '<rootDir>/src'], |
Author
There was a problem hiding this comment.
tests in components/src was never run, need to expose the directory as roots
| jest.mock('@opentelemetry/exporter-trace-otlp-proto', () => { | ||
| return { | ||
| ProtoOTLPTraceExporter: jest.fn().mockImplementation((args) => { | ||
| OTLPTraceExporter: jest.fn().mockImplementation((args) => { |
Author
There was a problem hiding this comment.
the exported function from the library is OTLPTraceExporter not the renamed version import { OTLPTraceExporter as ProtoOTLPTraceExporter }.
| } | ||
| }) | ||
|
|
||
| import { OTLPTraceExporter as ProtoOTLPTraceExporter } from '@opentelemetry/exporter-trace-otlp-proto' |
Author
There was a problem hiding this comment.
move import to the top of the file
yau-wd
reviewed
Dec 19, 2025
There was a problem hiding this comment.
Environment
OS: WIN
Node: v20.19.1
Result
FAIL src/validator.test.ts (15.632 s)
validateMimeTypeAndExtensionMatch
valid cases
√ should pass validation for matching MIME type and extension - document.txt with text/plain (9 ms)
√ should pass validation for matching MIME type and extension - page.html with text/html
√ should pass validation for matching MIME type and extension - data.json with application/json
√ should pass validation for matching MIME type and extension - document.pdf with application/pdf (1 ms)
√ should pass validation for matching MIME type and extension - script.js with text/javascript
√ should pass validation for matching MIME type and extension - script.js with application/javascript (1 ms)
√ should pass validation for matching MIME type and extension - readme.md with text/markdown
√ should pass validation for matching MIME type and extension - readme.md with text/x-markdown
√ should pass validation for matching MIME type and extension - DOCUMENT.TXT with text/plain
√ should pass validation for matching MIME type and extension - Document.TxT with text/plain
√ should pass validation for matching MIME type and extension - my.document.txt with text/plain
invalid filename
√ should throw error for empty filename (11 ms)
√ should throw error for null filename (1 ms)
√ should throw error for undefined filename (1 ms)
√ should throw error for non-string filename (number)
√ should throw error for object filename (1 ms)
invalid MIME type
√ should throw error for empty MIME type
√ should throw error for null MIME type
√ should throw error for undefined MIME type
√ should throw error for non-string MIME type (number) (1 ms)
path traversal detection
× should throw error for filename with .. (13 ms)
× should throw error for filename with .. in middle (3 ms)
× should throw error for filename with multle levels of .. (2 ms)
× should throw error for filename with ..\..\.. (2 ms)
× should throw error for filename with ....//....// (1 ms)
× should throw error for filename starting with / (1 ms)
× should throw error for Windows absolute path (2 ms)
× should throw error for URL encoded path traversal (1 ms)
× should throw error for URL encoded path traversal multiple levels (2 ms)
× should throw error for null byte (2 ms)
files without extensions
√ should throw error for filename without extension
√ should throw error for filename ending with dot (1 ms)
unsupported MIME types
√ should throw error for unsupported MIME type application/octet-stream with file.txt
√ should throw error for unsupported MIME type invalid-mime-type with file.txt (1 ms)
√ should throw error for unsupported MIME type application/x-msdownload with malware.exe
√ should throw error for unsupported MIME type application/x-executable with script.exe
√ should throw error for unsupported MIME type application/x-msdownload with program.EXE (1 ms)
√ should throw error for unsupported MIME type application/octet-stream with script.js
MIME type and extension mismatches
√ should throw error when extension does not match MIME type - file.txt with application/json
√ should throw error when extension does not match MIME type - script.js with application/pdf (1 ms)
√ should throw error when extension does not match MIME type - page.html with text/plain
√ should throw error when extension does not match MIME type - document.pdf with application/json (1 ms)
√ should throw error when extension does not match MIME type - data.json with text/plain
√ should throw error when extension does not match MIME type - malware.exe with text/plain (1 ms)
√ should throw error when extension does not match MIME type - script.js with application/json
● validateMimeTypeAndExtensionMatch › path traversal detection › should throw error for filename with ..
expect(received).toThrow(expected)
Expected substring: "Invalid filename: path traversal detected in filename \"../file.txt\""
Received message: "Invalid filename: unsafe characters or path traversal attempt detected in filename \"../file.txt\""
83 | }
84 | if (isUnsafeFilePath(filename)) {
> 85 | throw new Error(`Invalid filename: unsafe characters or path traversal attempt detected in filename "${filename}"`)
| ^
86 | }
87 | }
88 |
at validateFilename (src/validator.ts:85:15)
at validateMimeTypeAndExtensionMatch (src/validator.ts:111:5)
at src/validator.test.ts:65:50
at Object.<anonymous> (../../node_modules/.pnpm/[email protected]/node_modules/expect/build/toThrowMatchers.js:74:11)
at Object.throwingMatcher [as toThrow] (../../node_modules/.pnpm/[email protected]/node_modules/expect/build/index.js:320:21)
at src/validator.test.ts:66:16
64 | expect(() => {
65 | validateMimeTypeAndExtensionMatch(filename, 'text/plain')
> 66 | }).toThrow(`Invalid filename: path traversal detected in filename "${filename}"`)
| ^
67 | })
68 | })
69 |
at src/validator.test.ts:66:16
● validateMimeTypeAndExtensionMatch › path traversal detection › should throw error for filename with .. in middle
expect(received).toThrow(expected)
Expected substring: "Invalid filename: path traversal detected in filename \"path/../file.txt\""
Received message: "Invalid filename: unsafe characters or path traversal attempt detected in filename \"path/../file.txt\""
83 | }
84 | if (isUnsafeFilePath(filename)) {
> 85 | throw new Error(`Invalid filename: unsafe characters or path traversal attempt detected in filename "${filename}"`)
| ^
86 | }
87 | }
88 |
at validateFilename (src/validator.ts:85:15)
at validateMimeTypeAndExtensionMatch (src/validator.ts:111:5)
at src/validator.test.ts:65:50
at Object.<anonymous> (../../node_modules/.pnpm/[email protected]/node_modules/expect/build/toThrowMatchers.js:74:11)
at Object.throwingMatcher [as toThrow] (../../node_modules/.pnpm/[email protected]/node_modules/expect/build/index.js:320:21)
at src/validator.test.ts:66:16
64 | expect(() => {
65 | validateMimeTypeAndExtensionMatch(filename, 'text/plain')
> 66 | }).toThrow(`Invalid filename: path traversal detected in filename "${filename}"`)
| ^
67 | })
68 | })
69 |
at src/validator.test.ts:66:16
● validateMimeTypeAndExtensionMatch › path traversal detection › should throw error for filename with multle levels of ..
expect(received).toThrow(expected)
Expected substring: "Invalid filename: path traversal detected in filename \"../../../etc/passwd.txt\""
Received message: "Invalid filename: unsafe characters or path traversal attempt detected in filename \"../../../etc/passwd.txt\""
83 | }
84 | if (isUnsafeFilePath(filename)) {
> 85 | throw new Error(`Invalid filename: unsafe characters or path traversal attempt detected in filename "${filename}"`)
| ^
86 | }
87 | }
88 |
at validateFilename (src/validator.ts:85:15)
at validateMimeTypeAndExtensionMatch (src/validator.ts:111:5)
at src/validator.test.ts:65:50
at Object.<anonymous> (../../node_modules/.pnpm/[email protected]/node_modules/expect/build/toThrowMatchers.js:74:11)
at Object.throwingMatcher [as toThrow] (../../node_modules/.pnpm/[email protected]/node_modules/expect/build/index.js:320:21)
at src/validator.test.ts:66:16
64 | expect(() => {
65 | validateMimeTypeAndExtensionMatch(filename, 'text/plain')
> 66 | }).toThrow(`Invalid filename: path traversal detected in filename "${filename}"`)
| ^
67 | })
68 | })
69 |
at src/validator.test.ts:66:16
● validateMimeTypeAndExtensionMatch › path traversal detection › should throw error for filename with ..\..\..
expect(received).toThrow(expected)
Expected substring: "Invalid filename: path traversal detected in filename \"..\\..\\..\\windows\\system32\\file.txt\""
Received message: "Invalid filename: unsafe characters or path traversal attempt detected in filename \"..\\..\\..\\windows\\system32\\file.txt\""
83 | }
84 | if (isUnsafeFilePath(filename)) {
> 85 | throw new Error(`Invalid filename: unsafe characters or path traversal attempt detected in filename "${filename}"`)
| ^
86 | }
87 | }
88 |
at validateFilename (src/validator.ts:85:15)
at validateMimeTypeAndExtensionMatch (src/validator.ts:111:5)
at src/validator.test.ts:65:50
at Object.<anonymous> (../../node_modules/.pnpm/[email protected]/node_modules/expect/build/toThrowMatchers.js:74:11)
at Object.throwingMatcher [as toThrow] (../../node_modules/.pnpm/[email protected]/node_modules/expect/build/index.js:320:21)
at src/validator.test.ts:66:16
64 | expect(() => {
65 | validateMimeTypeAndExtensionMatch(filename, 'text/plain')
> 66 | }).toThrow(`Invalid filename: path traversal detected in filename "${filename}"`)
| ^
67 | })
68 | })
69 |
at src/validator.test.ts:66:16
● validateMimeTypeAndExtensionMatch › path traversal detection › should throw error for filename with ....//....//
expect(received).toThrow(expected)
Expected substring: "Invalid filename: path traversal detected in filename \"....//....//etc/passwd.txt\""
Received message: "Invalid filename: unsafe characters or path traversal attempt detected in filename \"....//....//etc/passwd.txt\""
83 | }
84 | if (isUnsafeFilePath(filename)) {
> 85 | throw new Error(`Invalid filename: unsafe characters or path traversal attempt detected in filename "${filename}"`)
| ^
86 | }
87 | }
88 |
at validateFilename (src/validator.ts:85:15)
at validateMimeTypeAndExtensionMatch (src/validator.ts:111:5)
at src/validator.test.ts:65:50
at Object.<anonymous> (../../node_modules/.pnpm/[email protected]/node_modules/expect/build/toThrowMatchers.js:74:11)
at Object.throwingMatcher [as toThrow] (../../node_modules/.pnpm/[email protected]/node_modules/expect/build/index.js:320:21)
at src/validator.test.ts:66:16
64 | expect(() => {
65 | validateMimeTypeAndExtensionMatch(filename, 'text/plain')
> 66 | }).toThrow(`Invalid filename: path traversal detected in filename "${filename}"`)
| ^
67 | })
68 | })
69 |
at src/validator.test.ts:66:16
● validateMimeTypeAndExtensionMatch › path traversal detection › should throw error for filename starting with /
expect(received).toThrow(expected)
Expected substring: "Invalid filename: path traversal detected in filename \"/etc/passwd.txt\""
Received message: "Invalid filename: unsafe characters or path traversal attempt detected in filename \"/etc/passwd.txt\""
83 | }
84 | if (isUnsafeFilePath(filename)) {
> 85 | throw new Error(`Invalid filename: unsafe characters or path traversal attempt detected in filename "${filename}"`)
| ^
86 | }
87 | }
88 |
at validateFilename (src/validator.ts:85:15)
at validateMimeTypeAndExtensionMatch (src/validator.ts:111:5)
at src/validator.test.ts:65:50
at Object.<anonymous> (../../node_modules/.pnpm/[email protected]/node_modules/expect/build/toThrowMatchers.js:74:11)
at Object.throwingMatcher [as toThrow] (../../node_modules/.pnpm/[email protected]/node_modules/expect/build/index.js:320:21)
at src/validator.test.ts:66:16
64 | expect(() => {
65 | validateMimeTypeAndExtensionMatch(filename, 'text/plain')
> 66 | }).toThrow(`Invalid filename: path traversal detected in filename "${filename}"`)
| ^
67 | })
68 | })
69 |
at src/validator.test.ts:66:16
● validateMimeTypeAndExtensionMatch › path traversal detection › should throw error for Windows absolute path
expect(received).toThrow(expected)
Expected substring: "Invalid filename: path traversal detected in filename \"C:\\file.txt\""
Received message: "Invalid filename: unsafe characters or path traversal attempt detected in filename \"C:\\file.txt\""
83 | }
84 | if (isUnsafeFilePath(filename)) {
> 85 | throw new Error(`Invalid filename: unsafe characters or path traversal attempt detected in filename "${filename}"`)
| ^
86 | }
87 | }
88 |
at validateFilename (src/validator.ts:85:15)
at validateMimeTypeAndExtensionMatch (src/validator.ts:111:5)
at src/validator.test.ts:65:50
at Object.<anonymous> (../../node_modules/.pnpm/[email protected]/node_modules/expect/build/toThrowMatchers.js:74:11)
at Object.throwingMatcher [as toThrow] (../../node_modules/.pnpm/[email protected]/node_modules/expect/build/index.js:320:21)
at src/validator.test.ts:66:16
64 | expect(() => {
65 | validateMimeTypeAndExtensionMatch(filename, 'text/plain')
> 66 | }).toThrow(`Invalid filename: path traversal detected in filename "${filename}"`)
| ^
67 | })
68 | })
69 |
at src/validator.test.ts:66:16
● validateMimeTypeAndExtensionMatch › path traversal detection › should throw error for URL encoded path traversal
expect(received).toThrow(expected)
Expected substring: "Invalid filename: path traversal detected in filename \"%2e%2e/file.txt\""
Received message: "Invalid filename: unsafe characters or path traversal attempt detected in filename \"%2e%2e/file.txt\""
83 | }
84 | if (isUnsafeFilePath(filename)) {
> 85 | throw new Error(`Invalid filename: unsafe characters or path traversal attempt detected in filename "${filename}"`)
| ^
86 | }
87 | }
88 |
at validateFilename (src/validator.ts:85:15)
at validateMimeTypeAndExtensionMatch (src/validator.ts:111:5)
at src/validator.test.ts:65:50
at Object.<anonymous> (../../node_modules/.pnpm/[email protected]/node_modules/expect/build/toThrowMatchers.js:74:11)
at Object.throwingMatcher [as toThrow] (../../node_modules/.pnpm/[email protected]/node_modules/expect/build/index.js:320:21)
at src/validator.test.ts:66:16
64 | expect(() => {
65 | validateMimeTypeAndExtensionMatch(filename, 'text/plain')
> 66 | }).toThrow(`Invalid filename: path traversal detected in filename "${filename}"`)
| ^
67 | })
68 | })
69 |
at src/validator.test.ts:66:16
● validateMimeTypeAndExtensionMatch › path traversal detection › should throw error for URL encoded path traversal multiple levels
expect(received).toThrow(expected)
Expected substring: "Invalid filename: path traversal detected in filename \"%2e%2e%2f%2e%2e%2f%2e%2e%2ffile.txt\""
Received message: "Invalid filename: unsafe characters or path traversal attempt detected in filename \"%2e%2e%2f%2e%2e%2f%2e%2e%2ffile.txt\""
83 | }
84 | if (isUnsafeFilePath(filename)) {
> 85 | throw new Error(`Invalid filename: unsafe characters or path traversal attempt detected in filename "${filename}"`)
| ^
86 | }
87 | }
88 |
at validateFilename (src/validator.ts:85:15)
at validateMimeTypeAndExtensionMatch (src/validator.ts:111:5)
at src/validator.test.ts:65:50
at Object.<anonymous> (../../node_modules/.pnpm/[email protected]/node_modules/expect/build/toThrowMatchers.js:74:11)
at Object.throwingMatcher [as toThrow] (../../node_modules/.pnpm/[email protected]/node_modules/expect/build/index.js:320:21)
at src/validator.test.ts:66:16
64 | expect(() => {
65 | validateMimeTypeAndExtensionMatch(filename, 'text/plain')
> 66 | }).toThrow(`Invalid filename: path traversal detected in filename "${filename}"`)
| ^
67 | })
68 | })
69 |
at src/validator.test.ts:66:16
● validateMimeTypeAndExtensionMatch › path traversal detection › should throw error for null byte
expect(received).toThrow(expected)
Expected substring: "Invalid filename: path traversal detected in filename \"file.txt\""
Received message: "Invalid filename: unsafe characters or path traversal attempt detected in filename \"file.txt\""
83 | }
84 | if (isUnsafeFilePath(filename)) {
> 85 | throw new Error(`Invalid filename: unsafe characters or path traversal attempt detected in filename "${filename}"`)
| ^
86 | }
87 | }
88 |
at validateFilename (src/validator.ts:85:15)
at validateMimeTypeAndExtensionMatch (src/validator.ts:111:5)
at src/validator.test.ts:65:50
at Object.<anonymous> (../../node_modules/.pnpm/[email protected]/node_modules/expect/build/toThrowMatchers.js:74:11)
at Object.throwingMatcher [as toThrow] (../../node_modules/.pnpm/[email protected]/node_modules/expect/build/index.js:320:21)
at src/validator.test.ts:66:16
64 | expect(() => {
65 | validateMimeTypeAndExtensionMatch(filename, 'text/plain')
> 66 | }).toThrow(`Invalid filename: path traversal detected in filename "${filename}"`)
| ^
67 | })
68 | })
69 |
at src/validator.test.ts:66:16
PASS src/handler.test.ts (19.855 s)
URL Handling For Phoenix Tracer
√ baseUrl http://localhost:6006 - exporterUrl http://localhost:6006/v1/traces (7 ms)
√ baseUrl http://localhost:6006/v1/traces - exporterUrl http://localhost:6006/v1/traces (1 ms)
√ baseUrl https://app.phoenix.arize.com - exporterUrl https://app.phoenix.arize.com/v1/traces
√ baseUrl https://app.phoenix.arize.com/v1/traces - exporterUrl https://app.phoenix.arize.com/v1/traces (1 ms)
√ baseUrl https://app.phoenix.arize.com/s/my-space - exporterUrl https://app.phoenix.arize.com/s/my-space/v1/traces (1 ms)
√ baseUrl https://app.phoenix.arize.com/s/my-space/v1/traces - exporterUrl https://app.phoenix.arize.com/s/my-space/v1/traces (1 ms)
√ baseUrl https://my-phoenix.com/my-slug - exporterUrl https://my-phoenix.com/my-slug/v1/traces (1 ms)
√ baseUrl https://my-phoenix.com/my-slug/v1/traces - exporterUrl https://my-phoenix.com/my-slug/v1/traces
Test Suites: 1 failed, 3 passed, 4 total
Tests: 10 failed, 103 passed, 113 total
Snapshots: 0 total
Time: 20.62 s, estimated 57 s
Ran all test suites.
ELIFECYCLE Test failed. See above for more details.
```
Author
There was a problem hiding this comment.
thanks for catching this. was using github to apply suggested changes from gemini but forgot to update test (should be reflected now)
yau-wd
reviewed
Dec 19, 2025
…e changes in source
Author
|
/gemini review |
Contributor
There was a problem hiding this comment.
Code Review
This pull request effectively addresses a security vulnerability by introducing MIME type and file extension validation for file uploads. The new validation logic in packages/components/src/validator.ts is robust and is accompanied by a comprehensive set of unit tests in validator.test.ts, covering numerous edge cases. The validation has been correctly integrated into all relevant file upload points across the application. Additionally, the new functionality to clean up failed uploads from storage is a great improvement for system stability and resource management. I have one suggestion to improve maintainability by reducing code duplication.
Comment on lines
+149
to
+153
| try { | ||
| validateMimeTypeAndExtensionMatch(file.originalname, file.mimetype) | ||
| } catch (error) { | ||
| throw new InternalFlowiseError(StatusCodes.BAD_REQUEST, getErrorMessage(error)) | ||
| } |
Contributor
There was a problem hiding this comment.
This try...catch block for MIME type validation is repeated across multiple files (openai-assistants/index.ts, documentstore/index.ts, buildChatflow.ts, createAttachment.ts, upsertVector.ts). To improve maintainability and reduce code duplication, consider abstracting this logic into a shared helper function.
For example, you could create a function like this in a utility file:
import { StatusCodes } from 'http-status-codes';
import { validateMimeTypeAndExtensionMatch } from 'flowise-components';
import { InternalFlowiseError } from '../../errors/internalFlowiseError';
import { getErrorMessage } from '../../errors/utils';
export const validateFileOrThrow = (filename: string, mimetype: string): void => {
try {
validateMimeTypeAndExtensionMatch(filename, mimetype);
} catch (error) {
throw new InternalFlowiseError(StatusCodes.BAD_REQUEST, getErrorMessage(error));
}
};Then, you can replace this block with a single call: validateFileOrThrow(file.originalname, file.mimetype);
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Address security vulnerability reported in https://github.com/FlowiseAI/Flowise/security/advisories/GHSA-j8g8-j7fc-43v6 (also in issues #5579)
Add MIME type and extension validation for file uploads:
Unit Tests
Manual tests