diff --git a/api/server/routes/files/multer.js b/api/server/routes/files/multer.js index da17ce8008..7b044f0b51 100644 --- a/api/server/routes/files/multer.js +++ b/api/server/routes/files/multer.js @@ -2,7 +2,7 @@ const fs = require('fs'); const path = require('path'); const crypto = require('crypto'); const multer = require('multer'); -const { sanitizeFilename } = require('@librechat/api'); +const { sanitizeFilename, createCustomError } = require('@librechat/api'); const { mergeFileConfig, inferMimeType, @@ -34,7 +34,7 @@ const importFileFilter = (req, file, cb) => { } else if (path.extname(file.originalname).toLowerCase() === '.json') { cb(null, true); } else { - cb(new Error('Only JSON files are allowed'), false); + cb(createCustomError(415, 'Only JSON files are allowed'), false); } }; @@ -58,7 +58,7 @@ const createFileFilter = (customFileConfig) => { */ const fileFilter = (req, file, cb) => { if (!file) { - return cb(new Error('No file provided'), false); + return cb(createCustomError(400, 'No file provided'), false); } const mimeType = normalizeUploadMimeType(file); @@ -76,7 +76,10 @@ const createFileFilter = (customFileConfig) => { }); if (!defaultFileConfig.checkType(mimeType, endpointFileConfig.supportedMimeTypes)) { - return cb(new Error('Unsupported file type: ' + (file.mimetype || mimeType)), false); + return cb( + createCustomError(415, 'Unsupported file type: ' + (file.mimetype || mimeType)), + false, + ); } cb(null, true); diff --git a/api/server/routes/files/multer.spec.js b/api/server/routes/files/multer.spec.js index 23b7a2458a..a8b9e386b6 100644 --- a/api/server/routes/files/multer.spec.js +++ b/api/server/routes/files/multer.spec.js @@ -215,6 +215,8 @@ describe('Multer Configuration', () => { const cb = jest.fn((err, result) => { expect(err).toBeInstanceOf(Error); expect(err.message).toBe('Only JSON files are allowed'); + expect(err.statusCode).toBe(415); + expect(err.body).toEqual({ message: 'Only JSON files are allowed' }); expect(result).toBe(false); done(); }); @@ -300,6 +302,72 @@ describe('Multer Configuration', () => { fileFilter(mockReq, zipFile, cb); }); + it.each(['application/x-shellscript', 'text/x-shellscript'])( + 'should normalize %s to application/x-sh and accept the upload', + (reportedType) => { + const { mergeFileConfig } = require('librechat-data-provider'); + const fileFilter = createFileFilter(mergeFileConfig()); + const shellFile = { + ...mockFile, + originalname: 'script.sh', + mimetype: reportedType, + }; + + const cb = jest.fn(); + fileFilter(mockReq, shellFile, cb); + + expect(cb).toHaveBeenCalledWith(null, true); + expect(shellFile.mimetype).toBe('application/x-sh'); + }, + ); + + /** Normalization runs before the allowlist check, so an admin who applied one of the documented + * `.sh` workarounds must not be broken by it. Both recipes target `application/x-sh`, which is + * exactly what the alias now produces. */ + it.each([ + ['the canonical type (#4660, #5689, #6297)', ['application/x-sh']], + ['broad patterns (#14804)', ['image/.*', 'text/.*', 'application/.*']], + ])( + 'should keep accepting .sh for an existing workaround config allowing %s', + (_label, supportedMimeTypes) => { + const { mergeFileConfig } = require('librechat-data-provider'); + const fileFilter = createFileFilter( + mergeFileConfig({ endpoints: { agents: { supportedMimeTypes } } }), + ); + mockReq.body.endpoint = 'agents'; + const shellFile = { + ...mockFile, + originalname: 'script.sh', + mimetype: 'application/x-shellscript', + }; + + const cb = jest.fn(); + fileFilter(mockReq, shellFile, cb); + + expect(cb).toHaveBeenCalledWith(null, true); + }, + ); + + it('should reject an unsupported type with a 415 the client can surface', () => { + const { mergeFileConfig } = require('librechat-data-provider'); + const fileFilter = createFileFilter(mergeFileConfig()); + const binaryFile = { + ...mockFile, + originalname: 'program.exe', + mimetype: 'application/x-msdownload', + }; + + const cb = jest.fn(); + fileFilter(mockReq, binaryFile, cb); + + expect(cb).toHaveBeenCalledTimes(1); + const [error, result] = cb.mock.calls[0]; + expect(result).toBe(false); + expect(error).toBeInstanceOf(Error); + expect(error.statusCode).toBe(415); + expect(error.body).toEqual({ message: 'Unsupported file type: application/x-msdownload' }); + }); + it('should use real mergeFileConfig function', async () => { const { mergeFileConfig, mbToBytes } = require('librechat-data-provider'); diff --git a/packages/api/src/middleware/error.spec.ts b/packages/api/src/middleware/error.spec.ts index 90705c8e28..eb636007db 100644 --- a/packages/api/src/middleware/error.spec.ts +++ b/packages/api/src/middleware/error.spec.ts @@ -1,7 +1,7 @@ import { logger, tenantStorage } from '@librechat/data-schemas'; import type { Request, Response } from 'express'; import type { ValidationError, MongoServerError, CustomError } from '~/types'; -import { ErrorController } from './error'; +import { ErrorController, createCustomError } from './error'; // Mock the logger jest.mock('@librechat/data-schemas', () => ({ @@ -198,6 +198,31 @@ describe('ErrorController', () => { }); }); + describe('createCustomError', () => { + it('should build an Error carrying the status and a message body', () => { + const error = createCustomError(415, 'Unsupported file type: application/x-shellscript'); + + expect(error).toBeInstanceOf(Error); + expect(error.message).toBe('Unsupported file type: application/x-shellscript'); + expect(error.statusCode).toBe(415); + expect(error.body).toEqual({ message: 'Unsupported file type: application/x-shellscript' }); + }); + + it('should reach the client through ErrorController instead of a bare 500', () => { + ErrorController(createCustomError(415, 'Unsupported file type'), mockReq, mockRes, mockNext); + + expect(mockRes.status).toHaveBeenCalledWith(415); + expect(mockRes.send).toHaveBeenCalledWith({ message: 'Unsupported file type' }); + }); + + it('should be the piece a plain Error lacks, which falls through to 500', () => { + ErrorController(new Error('Unsupported file type'), mockReq, mockRes, mockNext); + + expect(mockRes.status).toHaveBeenCalledWith(500); + expect(mockRes.send).toHaveBeenCalledWith('An unknown error occurred.'); + }); + }); + describe('Unknown error handling', () => { it('should handle unknown errors', () => { const unknownError = new Error('Some unknown error'); diff --git a/packages/api/src/middleware/error.ts b/packages/api/src/middleware/error.ts index 5832290fcd..b39e5e8a12 100644 --- a/packages/api/src/middleware/error.ts +++ b/packages/api/src/middleware/error.ts @@ -41,6 +41,18 @@ function isCustomError(err: unknown): err is CustomError { return err !== null && typeof err === 'object' && 'statusCode' in err && 'body' in err; } +/** + * Builds an error that `ErrorController` relays to the client verbatim. `isCustomError` matches only + * when both `statusCode` and `body` are present, so a plain `Error` falls through to a bare 500 and + * its message never leaves the server log. Use this wherever the caller needs to see the reason. + */ +export const createCustomError = (statusCode: number, message: string): CustomError => { + const error = new Error(message) as CustomError; + error.statusCode = statusCode; + error.body = { message }; + return error; +}; + export const ErrorController = ( err: Error | CustomError, req: Request, diff --git a/packages/data-provider/src/file-config.spec.ts b/packages/data-provider/src/file-config.spec.ts index e7f895b6b1..84f2a9dc15 100644 --- a/packages/data-provider/src/file-config.spec.ts +++ b/packages/data-provider/src/file-config.spec.ts @@ -33,6 +33,14 @@ describe('inferMimeType', () => { expect(inferMimeType('archive.zip', 'application/x-zip-compressed')).toBe('application/zip'); }); + it('should normalize application/x-shellscript to application/x-sh', () => { + expect(inferMimeType('test.sh', 'application/x-shellscript')).toBe('application/x-sh'); + }); + + it('should normalize text/x-shellscript to application/x-sh', () => { + expect(inferMimeType('test.sh', 'text/x-shellscript')).toBe('application/x-sh'); + }); + it('should return a type that matches textMimeTypes after normalization', () => { const normalized = inferMimeType('test.py', 'text/x-python-script'); expect(textMimeTypes.test(normalized)).toBe(true); @@ -65,6 +73,20 @@ describe('inferMimeType', () => { expect(baseFileConfig.checkType(normalized)).toBe(true); }); + it.each(['application/x-shellscript', 'text/x-shellscript'])( + 'should produce a type accepted by checkType after normalizing %s', + (browserType) => { + expect(baseFileConfig.checkType(inferMimeType('test.sh', browserType))).toBe(true); + }, + ); + + it.each(['application/x-shellscript', 'text/x-shellscript'])( + 'should reject raw %s without normalization', + (browserType) => { + expect(baseFileConfig.checkType(browserType)).toBe(false); + }, + ); + it('should reject raw text/x-python-script without normalization', () => { expect(baseFileConfig.checkType('text/x-python-script')).toBe(false); }); diff --git a/packages/data-provider/src/file-config.ts b/packages/data-provider/src/file-config.ts index 9a0983bf90..5cfbe6b2b3 100644 --- a/packages/data-provider/src/file-config.ts +++ b/packages/data-provider/src/file-config.ts @@ -416,6 +416,10 @@ export const mimeTypeAliases: Readonly> = { 'application/x-zip-compressed': 'application/zip', 'text/x-python-script': 'text/x-python', 'text/x-markdown': 'text/markdown', + /** freedesktop shared-mime-info (Chrome on Linux) */ + 'application/x-shellscript': 'application/x-sh', + /** libmagic, i.e. `file --mime-type` */ + 'text/x-shellscript': 'application/x-sh', }; /**