Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🛡️ fix: Enhance File Upload Security & Error Handling #4705

Merged
merged 17 commits into from
Nov 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
00d204e
fix: sanitize filename in multer storage callback
danny-avila Nov 12, 2024
a40ba9b
fix: ensure temporary image upload file is deleted after processing
danny-avila Nov 12, 2024
c196ae6
fix: prevent cleanup flag from being set to false before actually del…
danny-avila Nov 12, 2024
5859b79
refactor: user avatar, typing, use 'file' for formData instead of 'in…
danny-avila Nov 12, 2024
991c83e
fix: update Avatar component to include image dimensions in formData …
danny-avila Nov 12, 2024
bb58a2d
fix: refactor avatar upload handling to use fs for file reading and e…
danny-avila Nov 12, 2024
ead61c9
fix: ensure temporary image upload file is deleted after processing
danny-avila Nov 12, 2024
1ea3650
fix: refactor avatar upload routes and handlers for agents and assist…
danny-avila Nov 12, 2024
a23809f
fix: improve audio file validation and cleanup
danny-avila Nov 12, 2024
629be5c
fix: add filename sanitization utility and integrate it into multer s…
danny-avila Nov 12, 2024
0d9cd07
fix: update group project ID check for null and refactor delete promp…
danny-avila Nov 12, 2024
5071bdb
fix: invalid access control for deleting prompt groups
danny-avila Nov 12, 2024
976784c
fix: add error handling and logging to checkBan middleware
danny-avila Nov 12, 2024
95a2125
fix: catch conversation parsing errors
danny-avila Nov 12, 2024
5953909
chore: revert unnecessary height and width parameters from avatar upload
danny-avila Nov 12, 2024
fc8b3cf
chore: update librechat-data-provider version to 0.7.55
danny-avila Nov 12, 2024
388f3bf
style: ensure KaTeX can spread across visible space
danny-avila Nov 12, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 28 additions & 18 deletions api/models/Prompt.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ const createAllGroupsPipeline = (

/**
* Get all prompt groups with filters
* @param {Object} req
* @param {ServerRequest} req
* @param {TPromptGroupsWithFilterRequest} filter
* @returns {Promise<PromptGroupListResponse>}
*/
Expand Down Expand Up @@ -142,7 +142,7 @@ const getAllPromptGroups = async (req, filter) => {

/**
* Get prompt groups with filters
* @param {Object} req
* @param {ServerRequest} req
* @param {TPromptGroupsWithFilterRequest} filter
* @returns {Promise<PromptGroupListResponse>}
*/
Expand Down Expand Up @@ -213,8 +213,34 @@ const getPromptGroups = async (req, filter) => {
}
};

/**
* @param {Object} fields
* @param {string} fields._id
* @param {string} fields.author
* @param {string} fields.role
* @returns {Promise<TDeletePromptGroupResponse>}
*/
const deletePromptGroup = async ({ _id, author, role }) => {
const query = { _id, author };
const groupQuery = { groupId: new ObjectId(_id), author };
if (role === SystemRoles.ADMIN) {
delete query.author;
delete groupQuery.author;
}
const response = await PromptGroup.deleteOne(query);

if (!response || response.deletedCount === 0) {
throw new Error('Prompt group not found');
}

await Prompt.deleteMany(groupQuery);
await removeGroupFromAllProjects(_id);
return { message: 'Prompt group deleted successfully' };
};

module.exports = {
getPromptGroups,
deletePromptGroup,
getAllPromptGroups,
/**
* Create a prompt and its respective group
Expand Down Expand Up @@ -510,20 +536,4 @@ module.exports = {
return { message: 'Error updating prompt labels' };
}
},
deletePromptGroup: async (_id) => {
try {
const response = await PromptGroup.deleteOne({ _id });

if (response.deletedCount === 0) {
return { promptGroup: 'Prompt group not found' };
}

await Prompt.deleteMany({ groupId: new ObjectId(_id) });
await removeGroupFromAllProjects(_id);
return { promptGroup: 'Prompt group deleted successfully' };
} catch (error) {
logger.error('Error deleting prompt group', error);
return { message: 'Error deleting prompt group' };
}
},
};
22 changes: 15 additions & 7 deletions api/server/controllers/agents/v1.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
const fs = require('fs').promises;
const { nanoid } = require('nanoid');
const { FileContext, Constants, Tools, SystemRoles } = require('librechat-data-provider');
const {
Expand All @@ -7,8 +8,8 @@ const {
deleteAgent,
getListAgents,
} = require('~/models/Agent');
const { uploadImageBuffer, filterFile } = require('~/server/services/Files/process');
const { getStrategyFunctions } = require('~/server/services/Files/strategies');
const { uploadImageBuffer } = require('~/server/services/Files/process');
const { getProjectByName } = require('~/models/Project');
const { updateAgentProjects } = require('~/models/Agent');
const { deleteFileByFilter } = require('~/models/File');
Expand Down Expand Up @@ -210,7 +211,7 @@ const getListAgentsHandler = async (req, res) => {

/**
* Uploads and updates an avatar for a specific agent.
* @route POST /avatar/:agent_id
* @route POST /:agent_id/avatar
* @param {object} req - Express Request
* @param {object} req.params - Request params
* @param {string} req.params.agent_id - The ID of the agent.
Expand All @@ -221,25 +222,25 @@ const getListAgentsHandler = async (req, res) => {
*/
const uploadAgentAvatarHandler = async (req, res) => {
try {
filterFile({ req, file: req.file, image: true, isAvatar: true });
const { agent_id } = req.params;
if (!agent_id) {
return res.status(400).json({ message: 'Agent ID is required' });
}

const buffer = await fs.readFile(req.file.path);
const image = await uploadImageBuffer({
req,
context: FileContext.avatar,
metadata: {
buffer: req.file.buffer,
},
metadata: { buffer },
});

let _avatar;
try {
const agent = await getAgent({ id: agent_id });
_avatar = agent.avatar;
} catch (error) {
logger.error('[/avatar/:agent_id] Error fetching agent', error);
logger.error('[/:agent_id/avatar] Error fetching agent', error);
_avatar = {};
}

Expand All @@ -249,7 +250,7 @@ const uploadAgentAvatarHandler = async (req, res) => {
await deleteFile(req, { filepath: _avatar.filepath });
await deleteFileByFilter({ user: req.user.id, filepath: _avatar.filepath });
} catch (error) {
logger.error('[/avatar/:agent_id] Error deleting old avatar', error);
logger.error('[/:agent_id/avatar] Error deleting old avatar', error);
}
}

Expand All @@ -270,6 +271,13 @@ const uploadAgentAvatarHandler = async (req, res) => {
const message = 'An error occurred while updating the Agent Avatar';
logger.error(message, error);
res.status(500).json({ message });
} finally {
try {
await fs.unlink(req.file.path);
logger.debug('[/:agent_id/avatar] Temp. image upload file deleted');
} catch (error) {
logger.debug('[/:agent_id/avatar] Temp. image upload file already deleted');
}
}
};

Expand Down
22 changes: 15 additions & 7 deletions api/server/controllers/assistants/v1.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
const fs = require('fs').promises;
const { FileContext } = require('librechat-data-provider');
const { uploadImageBuffer, filterFile } = require('~/server/services/Files/process');
const validateAuthor = require('~/server/middleware/assistants/validateAuthor');
const { getStrategyFunctions } = require('~/server/services/Files/strategies');
const { deleteAssistantActions } = require('~/server/services/ActionService');
const { updateAssistantDoc, getAssistants } = require('~/models/Assistant');
const { uploadImageBuffer } = require('~/server/services/Files/process');
const { getOpenAIClient, fetchAssistants } = require('./helpers');
const { deleteFileByFilter } = require('~/models/File');
const { logger } = require('~/config');
Expand Down Expand Up @@ -235,7 +236,7 @@ const getAssistantDocuments = async (req, res) => {

/**
* Uploads and updates an avatar for a specific assistant.
* @route POST /avatar/:assistant_id
* @route POST /:assistant_id/avatar
* @param {object} req - Express Request
* @param {object} req.params - Request params
* @param {string} req.params.assistant_id - The ID of the assistant.
Expand All @@ -245,6 +246,7 @@ const getAssistantDocuments = async (req, res) => {
*/
const uploadAssistantAvatar = async (req, res) => {
try {
filterFile({ req, file: req.file, image: true, isAvatar: true });
const { assistant_id } = req.params;
if (!assistant_id) {
return res.status(400).json({ message: 'Assistant ID is required' });
Expand All @@ -253,12 +255,11 @@ const uploadAssistantAvatar = async (req, res) => {
const { openai } = await getOpenAIClient({ req, res });
await validateAuthor({ req, openai });

const buffer = await fs.readFile(req.file.path);
const image = await uploadImageBuffer({
req,
context: FileContext.avatar,
metadata: {
buffer: req.file.buffer,
},
metadata: { buffer },
});

let _metadata;
Expand All @@ -269,7 +270,7 @@ const uploadAssistantAvatar = async (req, res) => {
_metadata = assistant.metadata;
}
} catch (error) {
logger.error('[/avatar/:assistant_id] Error fetching assistant', error);
logger.error('[/:assistant_id/avatar] Error fetching assistant', error);
_metadata = {};
}

Expand All @@ -279,7 +280,7 @@ const uploadAssistantAvatar = async (req, res) => {
await deleteFile(req, { filepath: _metadata.avatar });
await deleteFileByFilter({ user: req.user.id, filepath: _metadata.avatar });
} catch (error) {
logger.error('[/avatar/:assistant_id] Error deleting old avatar', error);
logger.error('[/:assistant_id/avatar] Error deleting old avatar', error);
}
}

Expand Down Expand Up @@ -310,6 +311,13 @@ const uploadAssistantAvatar = async (req, res) => {
const message = 'An error occurred while updating the Assistant Avatar';
logger.error(message, error);
res.status(500).json({ message });
} finally {
try {
await fs.unlink(req.file.path);
logger.debug('[/:agent_id/avatar] Temp. image upload file deleted');
} catch (error) {
logger.debug('[/:agent_id/avatar] Temp. image upload file already deleted');
}
}
};

Expand Down
21 changes: 15 additions & 6 deletions api/server/middleware/buildEndpointOption.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,12 @@ const buildFunction = {

async function buildEndpointOption(req, res, next) {
const { endpoint, endpointType } = req.body;
let parsedBody = parseCompactConvo({ endpoint, endpointType, conversation: req.body });
let parsedBody;
try {
parsedBody = parseCompactConvo({ endpoint, endpointType, conversation: req.body });
} catch (error) {
return handleError(res, { text: 'Error parsing conversation' });
}

if (req.app.locals.modelSpecs?.list && req.app.locals.modelSpecs?.enforce) {
/** @type {{ list: TModelSpec[] }}*/
Expand Down Expand Up @@ -56,11 +61,15 @@ async function buildEndpointOption(req, res, next) {
});
}

parsedBody = parseCompactConvo({
endpoint,
endpointType,
conversation: currentModelSpec.preset,
});
try {
parsedBody = parseCompactConvo({
endpoint,
endpointType,
conversation: currentModelSpec.preset,
});
} catch (error) {
return handleError(res, { text: 'Error parsing model spec' });
}
}

const endpointFn = buildFunction[endpointType ?? endpoint];
Expand Down
Loading
Loading