Andreas/LibreChat
1
0
Fork 0
mirror of https://github.com/danny-avila/LibreChat.git synced 2025-12-16 16:30:15 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions 6

👤 feat: Agent Avatar Removal and Decouple upload/reset from Agent Updates (#10527)

Browse source 
* ✨ feat: Enhance agent avatar management with upload and reset functionality

* ✨ feat: Refactor AvatarMenu to use DropdownPopup for improved UI and functionality

* ✨ feat: Improve avatar upload handling in AgentPanel to suppress misleading "no changes" toast

* ✨ feat: Refactor toast message handling and payload composition in AgentPanel for improved clarity and functionality

* ✨ feat: Enhance agent avatar functionality with upload, reset, and validation improvements

* ✨ feat: Refactor agent avatar upload handling and enhance related components for improved functionality and user experience

* feat(agents): tighten ACL, harden GETs/search, and sanitize action metadata
stop persisting refreshed S3 URLs on GET; compute per-response only
enforce ACL EDIT on revert route; remove legacy admin/author/collab checks
sanitize action metadata before persisting during duplication (api_key, oauth_client_id, oauth_client_secret)
escape user search input, cap length (100), and use Set for public flag mapping
add explicit req.file guard in avatar upload; fix empty catch lint; remove unused imports

* feat: Remove outdated avatar-related translation keys

* feat: Improve error logging for avatar updates and streamline file input handling

* feat(agents): implement caching for S3 avatar refresh in agent list responses

* fix: replace unconventional 'void e' with explicit comment to clarify intentionally ignored error

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* feat(agents): enhance avatar handling and improve search functionality

* fix: clarify intentionally ignored error in agent list handler

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
Marco Beretta 2025-11-17 23:04:01 +01:00 • committed by GitHub
parent c0cb48256e
commit 8907bd5d7c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
17 changed files with 931 additions and 398 deletions
Download patch file Download diff file Expand all files Collapse all files

161
api/server/controllers/agents/v1.js
View file

@ -11,7 +11,6 @@ const {
const {
Tools,
Constants,
SystemRoles,
FileSources,
ResourceType,
AccessRoleIds,
@ -20,6 +19,8 @@ const {
PermissionBits,
actionDelimiter,
removeNullishValues,
CacheKeys,
Time,
} = require('librechat-data-provider');
const {
getListAgentsByAccess,
@ -45,6 +46,7 @@ const { updateAction, getActions } = require('~/models/Action');
const { getCachedTools } = require('~/server/services/Config');
const { deleteFileByFilter } = require('~/models/File');
const { getCategoriesWithCounts } = require('~/models');
const { getLogStores } = require('~/cache');
const systemTools = {
[Tools.execute_code]: true,
@ -52,6 +54,49 @@ const systemTools = {
[Tools.web_search]: true,
};
const MAX_SEARCH_LEN = 100;
const escapeRegex = (str = '') => str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
/**
* Opportunistically refreshes S3-backed avatars for agent list responses.
* Only list responses are refreshed because they're the highest-traffic surface and
* the avatar URLs have a short-lived TTL. The refresh is cached per-user for 30 minutes
* via {@link CacheKeys.S3_EXPIRY_INTERVAL} so we refresh once per interval at most.
* @param {Array} agents - Agents being enriched with S3-backed avatars
* @param {string} userId - User identifier used for the cache refresh key
*/
const refreshListAvatars = async (agents, userId) => {
if (!agents?.length) {
return;
}
const cache = getLogStores(CacheKeys.S3_EXPIRY_INTERVAL);
const refreshKey = `${userId}:agents_list`;
const alreadyChecked = await cache.get(refreshKey);
if (alreadyChecked) {
return;
}
await Promise.all(
agents.map(async (agent) => {
if (agent?.avatar?.source !== FileSources.s3 || !agent?.avatar?.filepath) {
return;
}
try {
const newPath = await refreshS3Url(agent.avatar);
if (newPath && newPath !== agent.avatar.filepath) {
agent.avatar = { ...agent.avatar, filepath: newPath };
}
} catch (err) {
logger.debug('[/Agents] Avatar refresh error for list item', err);
}
}),
);
await cache.set(refreshKey, true, Time.THIRTY_MINUTES);
};
/**
* Creates an Agent.
* @route POST /Agents
@ -142,10 +187,13 @@ const getAgentHandler = async (req, res, expandProperties = false) => {
agent.version = agent.versions ? agent.versions.length : 0;
if (agent.avatar && agent.avatar?.source === FileSources.s3) {
const originalUrl = agent.avatar.filepath;
agent.avatar.filepath = await refreshS3Url(agent.avatar);
if (originalUrl !== agent.avatar.filepath) {
await updateAgent({ id }, { avatar: agent.avatar }, { updatingUserId: req.user.id });
try {
agent.avatar = {
...agent.avatar,
filepath: await refreshS3Url(agent.avatar),
};
} catch (e) {
logger.warn('[/Agents/:id] Failed to refresh S3 URL', e);
}
}
@ -209,7 +257,12 @@ const updateAgentHandler = async (req, res) => {
try {
const id = req.params.id;
const validatedData = agentUpdateSchema.parse(req.body);
const { _id, ...updateData } = removeNullishValues(validatedData);
// Preserve explicit null for avatar to allow resetting the avatar
const { avatar: avatarField, _id, ...rest } = validatedData;
const updateData = removeNullishValues(rest);
if (avatarField === null) {
updateData.avatar = avatarField;
}
// Convert OCR to context in incoming updateData
convertOcrToContextInPlace(updateData);
@ -342,21 +395,21 @@ const duplicateAgentHandler = async (req, res) => {
const [domain] = action.action_id.split(actionDelimiter);
const fullActionId = `${domain}${actionDelimiter}${newActionId}`;
// Sanitize sensitive metadata before persisting
const filteredMetadata = { ...(action.metadata || {}) };
for (const field of sensitiveFields) {
delete filteredMetadata[field];
}
const newAction = await updateAction(
{ action_id: newActionId },
{
metadata: action.metadata,
metadata: filteredMetadata,
agent_id: newAgentId,
user: userId,
},
);
const filteredMetadata = { ...newAction.metadata };
for (const field of sensitiveFields) {
delete filteredMetadata[field];
}
newAction.metadata = filteredMetadata;
newActionsList.push(newAction);
return fullActionId;
};
@ -463,13 +516,13 @@ const getListAgentsHandler = async (req, res) => {
filter.is_promoted = { $ne: true };
}
// Handle search filter
// Handle search filter (escape regex and cap length)
if (search && search.trim() !== '') {
filter.$or = [
{ name: { $regex: search.trim(), $options: 'i' } },
{ description: { $regex: search.trim(), $options: 'i' } },
];
const safeSearch = escapeRegex(search.trim().slice(0, MAX_SEARCH_LEN));
const regex = new RegExp(safeSearch, 'i');
filter.$or = [{ name: regex }, { description: regex }];
}
// Get agent IDs the user has VIEW access to via ACL
const accessibleIds = await findAccessibleResources({
userId,
@ -477,10 +530,12 @@ const getListAgentsHandler = async (req, res) => {
resourceType: ResourceType.AGENT,
requiredPermissions: requiredPermission,
});
const publiclyAccessibleIds = await findPubliclyAccessibleResources({
resourceType: ResourceType.AGENT,
requiredPermissions: PermissionBits.VIEW,
});
// Use the new ACL-aware function
const data = await getListAgentsByAccess({
accessibleIds,
@ -488,13 +543,31 @@ const getListAgentsHandler = async (req, res) => {
limit,
after: cursor,
});
if (data?.data?.length) {
data.data = data.data.map((agent) => {
if (publiclyAccessibleIds.some((id) => id.equals(agent._id))) {
const agents = data?.data ?? [];
if (!agents.length) {
return res.json(data);
}
const publicSet = new Set(publiclyAccessibleIds.map((oid) => oid.toString()));
data.data = agents.map((agent) => {
try {
if (agent?._id && publicSet.has(agent._id.toString())) {
agent.isPublic = true;
}
return agent;
});
} catch (e) {
// Silently ignore mapping errors
void e;
}
return agent;
});
// Opportunistically refresh S3 avatar URLs for list results with caching
try {
await refreshListAvatars(data.data, req.user.id);
} catch (err) {
logger.debug('[/Agents] Skipping avatar refresh for list', err);
}
return res.json(data);
} catch (error) {
@ -517,28 +590,21 @@ const getListAgentsHandler = async (req, res) => {
const uploadAgentAvatarHandler = async (req, res) => {
try {
const appConfig = req.config;
if (!req.file) {
return res.status(400).json({ message: 'No file uploaded' });
}
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 isAdmin = req.user.role === SystemRoles.ADMIN;
const existingAgent = await getAgent({ id: agent_id });
if (!existingAgent) {
return res.status(404).json({ error: 'Agent not found' });
}
const isAuthor = existingAgent.author.toString() === req.user.id.toString();
const hasEditPermission = existingAgent.isCollaborative || isAdmin || isAuthor;
if (!hasEditPermission) {
return res.status(403).json({
error: 'You do not have permission to modify this non-collaborative agent',
});
}
const buffer = await fs.readFile(req.file.path);
const fileStrategy = getFileStrategy(appConfig, { isAvatar: true });
const resizedBuffer = await resizeAvatar({
@ -571,8 +637,6 @@ const uploadAgentAvatarHandler = async (req, res) => {
}
}
const promises = [];
const data = {
avatar: {
filepath: image.filepath,
@ -580,17 +644,16 @@ const uploadAgentAvatarHandler = async (req, res) => {
},
};
promises.push(
await updateAgent({ id: agent_id }, data, {
updatingUserId: req.user.id,
}),
);
const resolved = await Promise.all(promises);
res.status(201).json(resolved[0]);
const updatedAgent = await updateAgent({ id: agent_id }, data, {
updatingUserId: req.user.id,
});
res.status(201).json(updatedAgent);
} catch (error) {
const message = 'An error occurred while updating the Agent Avatar';
logger.error(message, error);
logger.error(
`[/:agent_id/avatar] ${message} (${req.params?.agent_id ?? 'unknown agent'})`,
error,
);
res.status(500).json({ message });
} finally {
try {
@ -629,21 +692,13 @@ const revertAgentVersionHandler = async (req, res) => {
return res.status(400).json({ error: 'version_index is required' });
}
const isAdmin = req.user.role === SystemRoles.ADMIN;
const existingAgent = await getAgent({ id });
if (!existingAgent) {
return res.status(404).json({ error: 'Agent not found' });
}
const isAuthor = existingAgent.author.toString() === req.user.id.toString();
const hasEditPermission = existingAgent.isCollaborative || isAdmin || isAuthor;
if (!hasEditPermission) {
return res.status(403).json({
error: 'You do not have permission to modify this non-collaborative agent',
});
}
// Permissions are enforced via route middleware (ACL EDIT)
const updatedAgent = await revertAgentVersion({ id }, version_index);

63
api/server/controllers/agents/v1.spec.js
View file

@ -47,6 +47,7 @@ jest.mock('~/server/services/PermissionService', () => ({
findPubliclyAccessibleResources: jest.fn().mockResolvedValue([]),
grantPermission: jest.fn(),
hasPublicPermission: jest.fn().mockResolvedValue(false),
checkPermission: jest.fn().mockResolvedValue(true),
}));
jest.mock('~/models', () => ({
@ -573,6 +574,68 @@ describe('Agent Controllers - Mass Assignment Protection', () => {
expect(updatedAgent.version).toBe(agentInDb.versions.length);
});
test('should allow resetting avatar when value is explicitly null', async () => {
await Agent.updateOne(
{ id: existingAgentId },
{
avatar: {
filepath: 'https://example.com/avatar.png',
source: 's3',
},
},
);
mockReq.user.id = existingAgentAuthorId.toString();
mockReq.params.id = existingAgentId;
mockReq.body = {
avatar: null,
};
await updateAgentHandler(mockReq, mockRes);
const updatedAgent = mockRes.json.mock.calls[0][0];
expect(updatedAgent.avatar).toBeNull();
const agentInDb = await Agent.findOne({ id: existingAgentId });
expect(agentInDb.avatar).toBeNull();
});
test('should ignore avatar field when value is undefined', async () => {
const originalAvatar = {
filepath: 'https://example.com/original.png',
source: 's3',
};
await Agent.updateOne({ id: existingAgentId }, { avatar: originalAvatar });
mockReq.user.id = existingAgentAuthorId.toString();
mockReq.params.id = existingAgentId;
mockReq.body = {
avatar: undefined,
};
await updateAgentHandler(mockReq, mockRes);
const agentInDb = await Agent.findOne({ id: existingAgentId });
expect(agentInDb.avatar.filepath).toBe(originalAvatar.filepath);
expect(agentInDb.avatar.source).toBe(originalAvatar.source);
});
test('should not bump version when no mutable fields change', async () => {
const existingAgent = await Agent.findOne({ id: existingAgentId });
const originalVersionCount = existingAgent.versions.length;
mockReq.user.id = existingAgentAuthorId.toString();
mockReq.params.id = existingAgentId;
mockReq.body = {
avatar: undefined,
};
await updateAgentHandler(mockReq, mockRes);
const agentInDb = await Agent.findOne({ id: existingAgentId });
expect(agentInDb.versions.length).toBe(originalVersionCount);
});
test('should handle validation errors properly', async () => {
mockReq.user.id = existingAgentAuthorId.toString();
mockReq.params.id = existingAgentId;

10
api/server/routes/agents/v1.js
View file

@ -146,7 +146,15 @@ router.delete(
* @param {number} req.body.version_index - Index of the version to revert to.
* @returns {Agent} 200 - success response - application/json
*/
router.post('/:id/revert', checkGlobalAgentShare, v1.revertAgentVersion);
router.post(
'/:id/revert',
checkGlobalAgentShare,
canAccessAgentResource({
requiredPermission: PermissionBits.EDIT,
resourceIdParam: 'id',
}),
v1.revertAgentVersion,
);
/**
* Returns a list of agents.