mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-27 04:07:05 +00:00
📎 fix: Alias Shell Script MIME Variants to application/x-sh (#14817)
* 📎 fix: Alias Shell Script MIME Variants to `application/x-sh` Chrome on Linux reports `.sh` files as `application/x-shellscript` (freedesktop shared-mime-info) and libmagic reports `text/x-shellscript`. Neither string appears anywhere in the source, so uploads were rejected even though `application/x-sh` is in the default allowlist and `codeTypeMapping` maps `sh` to it — `inferMimeType` only consults the extension map when the client sends no type at all, so a non-empty browser value passed straight through to the allowlist check. Alias both variants to the canonical `application/x-sh`, matching the existing treatment of `text/x-markdown` and `application/x-zip-compressed`. Also attach `statusCode`/`body` to multer file-filter rejections. Without them the error misses the `isCustomError` branch in `ErrorController` and falls through to a bare `500 An unknown error occurred.`, so the rejection reason was logged server-side but never reached the client. The upload hook already surfaces `error.response.data.message`, so a rejected file now explains itself instead of showing a generic upload failure. * 🔁 refactor: Move Upload Error Contract Into `packages/api` Addresses codex P1 on #14817. The producer of the `statusCode`/`body` pair now sits beside its consumer: `isCustomError` and `ErrorController` are already in `packages/api/src/middleware/error.ts`, and `CustomError` is already in `packages/api/src/types/error.ts` — only the construction of that pair was stranded in legacy JS. `createCustomError` is exported from the same module as the guard that recognizes it, and `multer.js` is back to a thin caller. Also pins the `.sh` back-compat claim with tests: configs from the documented workarounds (`application/x-sh` per #4660/#5689/#6297, and the broad patterns from #14804) still accept a `.sh` upload after the alias rewrites the type. A negative control confirms the endpoint config is genuinely in play rather than falling back to the default allowlist.
This commit is contained in:
parent
6c46fd1252
commit
5e464bc930
6 changed files with 139 additions and 5 deletions
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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');
|
||||
|
||||
|
|
|
|||
|
|
@ -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');
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -416,6 +416,10 @@ export const mimeTypeAliases: Readonly<Record<string, string>> = {
|
|||
'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',
|
||||
};
|
||||
|
||||
/**
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue