CERBERUS/LibreChat
1
0
Fork 0
mirror of https://github.com/danny-avila/LibreChat.git synced 2026-08-04 14:57:42 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 6

fix: harden forced retention ownership and races

Browse source
This commit is contained in:
Marco Beretta 2026-07-25 14:12:09 +02:00
parent 5a61dfbe03
commit e10c9ba834
No known key found for this signature in database
GPG key ID: D918033D8E74CC11
6 changed files with 228 additions and 63 deletions
Download patch file Download diff file Expand all files Collapse all files

4
api/server/routes/__test-utils__/convos-route-mocks.js
View file

@ -72,8 +72,8 @@ module.exports = {
forkIpLimiter: (req, res, next) => next(),
forkUserLimiter: (req, res, next) => next(),
})),
configMiddleware: (req, res, next) => next(),
validateConvoAccess: (req, res, next) => next(),
configMiddleware: jest.fn((req, res, next) => next()),
validateConvoAccess: jest.fn((req, res, next) => next()),
}),
forkUtils: () => ({

25
api/server/routes/__tests__/convos.spec.js
View file

@ -27,6 +27,7 @@ describe('Convos Routes', () => {
deleteAllSharedLinksWithCleanup,
deleteConvoSharedLinksWithCleanup,
} = require('@librechat/api');
const { configMiddleware, validateConvoAccess } = require('~/server/middleware');
beforeAll(() => {
convosRouter = require('../convos');
@ -47,6 +48,30 @@ describe('Convos Routes', () => {
jest.clearAllMocks();
});
describe('conversation mutation middleware', () => {
it.each([
['/api/convos/archive', { arg: { conversationId: 'conv-123', isArchived: true } }],
['/api/convos/pin', { arg: { conversationId: 'conv-123', pinned: true } }],
['/api/convos/update', { arg: { conversationId: 'conv-123', title: 'Updated' } }],
])('loads config before an access denial on %s', async (path, body) => {
configMiddleware.mockImplementationOnce((req, res, next) => {
req.config = { interfaceConfig: { retentionMode: 'ephemeral' } };
next();
});
validateConvoAccess.mockImplementationOnce((req, res) => {
res.status(403).json({ configLoaded: req.config != null });
});
const response = await request(app).post(path).send(body);
expect(response.status).toBe(403);
expect(response.body).toEqual({ configLoaded: true });
expect(configMiddleware).toHaveBeenCalledTimes(1);
expect(validateConvoAccess).toHaveBeenCalledTimes(1);
expect(saveConvo).not.toHaveBeenCalled();
});
});
describe('DELETE /all', () => {
it('prunes the deleted conversations’ agent checkpoints (bulk, ids from deleteConvos)', async () => {
// HITL: a paused conversation's durable checkpoint must not outlive the conversation.

6
api/server/routes/convos.js
View file

@ -186,7 +186,7 @@ router.delete('/all', configMiddleware, async (req, res) => {
* @param {boolean} req.body.arg.isArchived - Whether to archive (true) or unarchive (false).
* @returns {object} 200 - The updated conversation object.
*/
router.post('/archive', validateConvoAccess, configMiddleware, async (req, res) => {
router.post('/archive', configMiddleware, validateConvoAccess, async (req, res) => {
const { conversationId, isArchived } = req.body?.arg ?? {};
if (!conversationId) {
@ -214,7 +214,7 @@ router.post('/archive', validateConvoAccess, configMiddleware, async (req, res)
}
});
router.post('/pin', validateConvoAccess, configMiddleware, async (req, res) => {
router.post('/pin', configMiddleware, validateConvoAccess, async (req, res) => {
const { conversationId, pinned } = req.body?.arg ?? {};
if (!conversationId) {
@ -256,7 +256,7 @@ const MAX_CONVO_TITLE_LENGTH = 1024;
* @param {string} req.body.arg.title - The new title for the conversation.
* @returns {object} 201 - The updated conversation object.
*/
router.post('/update', validateConvoAccess, configMiddleware, async (req, res) => {
router.post('/update', configMiddleware, validateConvoAccess, async (req, res) => {
const { conversationId, title } = req.body?.arg ?? {};
if (!conversationId) {

58
packages/data-schemas/src/methods/chatProject.spec.ts
View file

@ -308,23 +308,25 @@ describe('ChatProject methods', () => {
});
it('forces ephemeral retention on a permanent chat, its messages, shares, and files when assigning', async () => {
const project = await methods.createChatProject(user, { name: 'Ephemeral' });
await createConversation(user, 'convo-1', 'Permanent');
const ownerObjectId = new mongoose.Types.ObjectId();
const owner = ownerObjectId.toString();
const project = await methods.createChatProject(owner, { name: 'Ephemeral' });
await createConversation(owner, 'convo-1', 'Permanent');
await Message.create([
{ messageId: uuidv4(), conversationId: 'convo-1', user, text: 'first' },
{ messageId: uuidv4(), conversationId: 'convo-1', user, text: 'second' },
{ messageId: uuidv4(), conversationId: 'convo-1', user: owner, text: 'first' },
{ messageId: uuidv4(), conversationId: 'convo-1', user: owner, text: 'second' },
]);
await SharedLink.create({ conversationId: 'convo-1', user, shareId: uuidv4() });
await SharedLink.create({ conversationId: 'convo-1', user: owner, shareId: uuidv4() });
const fileId = uuidv4();
await File.collection.insertOne({
file_id: fileId,
conversationId: 'convo-1',
user: new mongoose.Types.ObjectId(),
user: ownerObjectId,
expiredAt: null,
});
const result = await methods.assignConversationToProject(
user,
owner,
'convo-1',
project._id!.toString(),
ephemeralConfig,
@ -335,20 +337,26 @@ describe('ChatProject methods', () => {
expect(result?.conversation.chatProjectId).toBe(project._id!.toString());
const conversation = await Conversation.findOne({
user,
user: owner,
conversationId: 'convo-1',
}).lean<IConversation>();
expect(conversation?.isTemporary).toBe(true);
expect(conversation?.expiredAt).toBeInstanceOf(Date);
const messages = await Message.find({ user, conversationId: 'convo-1' }).lean<IMessage[]>();
const messages = await Message.find({
user: owner,
conversationId: 'convo-1',
}).lean<IMessage[]>();
expect(messages).toHaveLength(2);
for (const message of messages) {
expect(message.isTemporary).toBe(true);
expect(message.expiredAt).toBeInstanceOf(Date);
}
const share = await SharedLink.findOne({ user, conversationId: 'convo-1' }).lean<ISharedLink>();
const share = await SharedLink.findOne({
user: owner,
conversationId: 'convo-1',
}).lean<ISharedLink>();
expect(share?.expiredAt).toBeInstanceOf(Date);
const file = await File.findOne({ file_id: fileId }).lean<IMongoFile>();
@ -386,25 +394,32 @@ describe('ChatProject methods', () => {
});
it('forces ephemeral retention on member chats when deleting a project', async () => {
const project = await methods.createChatProject(user, { name: 'Ephemeral' });
const ownerObjectId = new mongoose.Types.ObjectId();
const owner = ownerObjectId.toString();
const project = await methods.createChatProject(owner, { name: 'Ephemeral' });
const projectId = project._id!.toString();
await createConversation(user, 'convo-1', 'First');
await createConversation(user, 'convo-2', 'Second');
await methods.assignConversationToProject(user, 'convo-1', projectId);
await methods.assignConversationToProject(user, 'convo-2', projectId);
await Message.create({ messageId: uuidv4(), conversationId: 'convo-1', user, text: 'hi' });
await createConversation(owner, 'convo-1', 'First');
await createConversation(owner, 'convo-2', 'Second');
await methods.assignConversationToProject(owner, 'convo-1', projectId);
await methods.assignConversationToProject(owner, 'convo-2', projectId);
await Message.create({
messageId: uuidv4(),
conversationId: 'convo-1',
user: owner,
text: 'hi',
});
const fileId = uuidv4();
await File.collection.insertOne({
file_id: fileId,
conversationId: 'convo-1',
user: new mongoose.Types.ObjectId(),
user: ownerObjectId,
expiredAt: null,
});
await methods.deleteChatProject(user, projectId, ephemeralConfig);
await methods.deleteChatProject(owner, projectId, ephemeralConfig);
const conversations = await Conversation.find({
user,
user: owner,
conversationId: { $in: ['convo-1', 'convo-2'] },
}).lean<IConversation[]>();
expect(conversations).toHaveLength(2);
@ -414,7 +429,10 @@ describe('ChatProject methods', () => {
expect(conversation.expiredAt).toBeInstanceOf(Date);
}
const message = await Message.findOne({ user, conversationId: 'convo-1' }).lean<IMessage>();
const message = await Message.findOne({
user: owner,
conversationId: 'convo-1',
}).lean<IMessage>();
expect(message?.isTemporary).toBe(true);
expect(message?.expiredAt).toBeInstanceOf(Date);

141
packages/data-schemas/src/methods/message.spec.ts
View file

@ -1554,10 +1554,12 @@ describe('Message Operations', () => {
describe('applyForcedRetentionToTag', () => {
const Conversation = () => mongoose.models.Conversation as mongoose.Model<IConversation>;
const SharedLink = () => mongoose.models.SharedLink as mongoose.Model<ISharedLink>;
const File = () => mongoose.models.File as mongoose.Model<IMongoFile>;
beforeEach(async () => {
await Conversation().deleteMany({});
await SharedLink().deleteMany({});
await File().deleteMany({});
});
it('converts every permanent conversation carrying the tag under ephemeral mode', async () => {
@ -1689,6 +1691,45 @@ describe('Message Operations', () => {
expect(permanentShare?.expiredAt?.getTime()).toBeGreaterThan(soonerExpiry.getTime());
});
it('owner-scopes bulk conversation file caps when conversation ids collide', async () => {
const ownerObjectId = new mongoose.Types.ObjectId();
const foreignOwnerObjectId = new mongoose.Types.ObjectId();
const owner = ownerObjectId.toString();
const foreignOwner = foreignOwnerObjectId.toString();
const conversationId = uuidv4();
const ownerFileId = uuidv4();
const foreignFileId = uuidv4();
await Conversation().create([
{ conversationId, user: owner, endpoint: 'openAI', tags: ['work'] },
{ conversationId, user: foreignOwner, endpoint: 'openAI', tags: ['personal'] },
]);
await File().collection.insertMany([
{ file_id: ownerFileId, conversationId, user: ownerObjectId, expiredAt: null },
{
file_id: foreignFileId,
conversationId,
user: foreignOwnerObjectId,
expiredAt: null,
},
]);
await applyForcedRetentionToTag(
{
userId: owner,
interfaceConfig: { temporaryChatRetention: 24, retentionMode: RetentionMode.EPHEMERAL },
},
{ tag: 'work' },
{ context: 'PUT /api/tags/:tag' },
);
const ownerFile = await File().findOne({ file_id: ownerFileId }).lean();
expect(ownerFile?.expiredAt).toBeInstanceOf(Date);
const foreignFile = await File().findOne({ file_id: foreignFileId }).lean();
expect(foreignFile?.expiredAt ?? null).toBeNull();
});
it('is a no-op outside forced retention', async () => {
const conversationId = uuidv4();
await Conversation().create({
@ -1813,10 +1854,12 @@ describe('Message Operations', () => {
const permanentFileId = uuidv4();
const soonerFileId = uuidv4();
const unrelatedFileId = uuidv4();
const ownerObjectId = new mongoose.Types.ObjectId();
const owner = ownerObjectId.toString();
await Conversation().create({
conversationId,
user: 'user123',
user: owner,
endpoint: 'openAI',
isTemporary: false,
});
@ -1824,19 +1867,19 @@ describe('Message Operations', () => {
{
file_id: permanentFileId,
conversationId,
user: new mongoose.Types.ObjectId(),
user: ownerObjectId,
expiredAt: null,
},
{
file_id: soonerFileId,
conversationId,
user: new mongoose.Types.ObjectId(),
user: ownerObjectId,
expiredAt: soonerExpiry,
},
{
file_id: unrelatedFileId,
conversationId: unrelatedConversationId,
user: new mongoose.Types.ObjectId(),
user: ownerObjectId,
expiredAt: null,
},
]);
@ -1858,19 +1901,21 @@ describe('Message Operations', () => {
const parentDeadline = new Date(Date.now() + 60 * 60 * 1000);
const conversationId = uuidv4();
const fileId = uuidv4();
const ownerObjectId = new mongoose.Types.ObjectId();
const owner = ownerObjectId.toString();
await Conversation().create({
conversationId,
user: 'user123',
user: owner,
endpoint: 'openAI',
isTemporary: true,
expiredAt: parentDeadline,
});
await SharedLink().create({ conversationId, user: 'user123', shareId: uuidv4() });
await SharedLink().create({ conversationId, user: owner, shareId: uuidv4() });
await File().collection.insertOne({
file_id: fileId,
conversationId,
user: new mongoose.Types.ObjectId(),
user: ownerObjectId,
expiredAt: null,
});
@ -2067,6 +2112,53 @@ describe('Message Operations', () => {
expect(converted?.expiredAt?.getTime()).toBe(forcedExpiredAt.getTime());
});
it('does not extend a parent shortened after the sweep cursor read', async () => {
const forcedExpiredAt = new Date(Date.now() + 24 * 60 * 60 * 1000);
const soonerExpiry = new Date(Date.now() + 30 * 60 * 1000);
const owner = new mongoose.Types.ObjectId().toString();
const conversationId = uuidv4();
const convo = await Conversation().create({
conversationId,
user: owner,
endpoint: 'openAI',
isTemporary: false,
});
const FileModel = File();
const updateMany = FileModel.updateMany.bind(FileModel);
const racingUpdateMany = jest.fn(
async (
filter: mongoose.FilterQuery<IMongoFile>,
update: mongoose.UpdateQuery<IMongoFile>,
) => {
await Conversation().collection.updateOne(
{ _id: convo._id },
{ $set: { isTemporary: true, expiredAt: soonerExpiry } },
);
return updateMany(filter, update);
},
);
Object.assign(FileModel, { updateMany: racingUpdateMany });
const result = await (async () => {
try {
return await sweepForcedRetention(
Conversation(),
Message,
SharedLink(),
FileModel,
forcedExpiredAt,
);
} finally {
Object.assign(FileModel, { updateMany });
}
})();
expect(racingUpdateMany).toHaveBeenCalledTimes(1);
expect(result).toEqual({ conversations: 0, aligned: 0, errors: 0, projects: [] });
const parent = await Conversation().findById(convo._id).lean();
expect(parent?.isTemporary).toBe(true);
expect(parent?.expiredAt?.getTime()).toBe(soonerExpiry.getTime());
});
it('scopes the sweep to the active tenant context, leaving other tenants untouched', async () => {
const forcedExpiredAt = new Date(Date.now() + 24 * 60 * 60 * 1000);
const tenantAConversationId = uuidv4();
@ -2127,6 +2219,9 @@ describe('Message Operations', () => {
const convoFileId = uuidv4();
const threadFileId = uuidv4();
const foreignFileId = uuidv4();
const conversationFileId = uuidv4();
const foreignConversationFileId = uuidv4();
const foreignOwnerObjectId = new mongoose.Types.ObjectId();
await Conversation().create({
conversationId,
@ -2153,6 +2248,13 @@ describe('Message Operations', () => {
{ file_id: convoFileId, user: ownerObjectId, expiredAt: null },
{ file_id: threadFileId, user: ownerObjectId, expiredAt: null },
{ file_id: foreignFileId, user: new mongoose.Types.ObjectId(), expiredAt: null },
{ file_id: conversationFileId, conversationId, user: ownerObjectId, expiredAt: null },
{
file_id: foreignConversationFileId,
conversationId,
user: foreignOwnerObjectId,
expiredAt: null,
},
]);
await cascadeForcedConversationRetention(
@ -2177,12 +2279,20 @@ describe('Message Operations', () => {
const threadFile = await File().findOne({ file_id: threadFileId }).lean();
expect(threadFile?.expiredAt?.getTime()).toBe(forcedExpiredAt.getTime());
const conversationFile = await File().findOne({ file_id: conversationFileId }).lean();
expect(conversationFile?.expiredAt?.getTime()).toBe(forcedExpiredAt.getTime());
/**
* A crafted reference to another user's file must never shorten that file's retention:
* the referenced-id cap is owner-scoped, so the foreign row stays permanent.
*/
const foreignFile = await File().findOne({ file_id: foreignFileId }).lean();
expect(foreignFile?.expiredAt ?? null).toBeNull();
const foreignConversationFile = await File()
.findOne({ file_id: foreignConversationFileId })
.lean();
expect(foreignConversationFile?.expiredAt ?? null).toBeNull();
});
it('caps lagging children even when the parent conversation already conforms', async () => {
@ -2190,19 +2300,21 @@ describe('Message Operations', () => {
const parentDeadline = new Date(Date.now() + 60 * 60 * 1000);
const conversationId = uuidv4();
const fileId = uuidv4();
const ownerObjectId = new mongoose.Types.ObjectId();
const owner = ownerObjectId.toString();
await Conversation().create({
conversationId,
user: 'user123',
user: owner,
endpoint: 'openAI',
isTemporary: true,
expiredAt: parentDeadline,
});
await SharedLink().create({ conversationId, user: 'user123', shareId: uuidv4() });
await SharedLink().create({ conversationId, user: owner, shareId: uuidv4() });
await File().collection.insertOne({
file_id: fileId,
conversationId,
user: new mongoose.Types.ObjectId(),
user: ownerObjectId,
expiredAt: null,
});
@ -2211,7 +2323,7 @@ describe('Message Operations', () => {
Message,
SharedLink(),
File(),
'user123',
owner,
conversationId,
forcedExpiredAt,
);
@ -2229,9 +2341,10 @@ describe('Message Operations', () => {
it('leaves the conversation non-conforming when a child backfill fails so a re-run retries it', async () => {
const forcedExpiredAt = new Date(Date.now() + 24 * 60 * 60 * 1000);
const conversationId = uuidv4();
const owner = new mongoose.Types.ObjectId().toString();
await Conversation().create({
conversationId,
user: 'user123',
user: owner,
endpoint: 'openAI',
isTemporary: false,
});
@ -2246,7 +2359,7 @@ describe('Message Operations', () => {
Message,
SharedLink(),
throwingFile,
'user123',
owner,
conversationId,
forcedExpiredAt,
),
@ -2261,7 +2374,7 @@ describe('Message Operations', () => {
Message,
SharedLink(),
File(),
'user123',
owner,
conversationId,
forcedExpiredAt,
);

57
packages/data-schemas/src/utils/retention.ts
View file

@ -226,21 +226,23 @@ export const conversationSeedFileIds = (convo: {
file_ids?: string[] | null;
}): string[] => [...(convo.files ?? []), ...(convo.file_ids ?? [])];
/**
* Builds the owner-scoped referenced-file-id branch of a file-cap filter. File ids inside
* message/conversation documents are caller-supplied, so a crafted reference to another user's
* file must never shorten that file's retention: the branch always filters on `File.user`.
* `File.user` is an ObjectId, so when the caller's user id is not a castable ObjectId string
* (legacy/test data) the branch is dropped entirely — fail closed rather than cap unverified
* rows.
*/
const ownedFileIdScope = (
/** Builds an owner-scoped file-cap filter for conversation and referenced-file matches. */
const ownedFileScope = (
userId: string,
conversationId: string | { $in: string[] },
fileIds: string[],
): { file_id: { $in: string[] }; user: string } | null =>
fileIds.length > 0 && isValidObjectIdString(userId)
? { file_id: { $in: fileIds }, user: userId }
: null;
): FilterQuery<IMongoFile> | null => {
if (!isValidObjectIdString(userId)) {
return null;
}
const conversationScope = { conversationId, user: userId };
if (fileIds.length === 0) {
return conversationScope;
}
return {
$or: [conversationScope, { file_id: { $in: fileIds }, user: userId }],
};
};
/**
* Caps a conversation's uploaded files to the forced deadline. Files use a retention-scoped
@ -251,10 +253,9 @@ const ownedFileIdScope = (
* already expires sooner. Under ephemeral retention every conversation-scoped file is meant to
* expire (persistent agent files are not retained), so no agent-file exclusion is needed here.
*
* Matches by `conversationId` (a globally unique per-conversation id whose File rows are
* server-created by this conversation's own processes; a colliding id can only shorten the
* colliding owner's files, never extend) plus any referenced `fileIds`, which are
* owner-scoped via {@link ownedFileIdScope} because references are caller-supplied.
* Matches by `conversationId` plus any referenced `fileIds`. Both branches are scoped by
* `File.user` because conversation ids are unique only together with their owner and tenant,
* while referenced ids can be caller-supplied.
* A shared file referenced from several chats is capped to the earliest converting chat's
* deadline, consistent with cap-don't-extend.
*/
@ -265,8 +266,10 @@ export const capConversationFiles = async (
forcedExpiredAt: Date,
fileIds: string[] = [],
): Promise<number> => {
const fileIdScope = ownedFileIdScope(userId, fileIds);
const scope = fileIdScope ? { $or: [{ conversationId }, fileIdScope] } : { conversationId };
const scope = ownedFileScope(userId, conversationId, fileIds);
if (scope == null) {
return 0;
}
const result = await File.updateMany(
{
$and: [scope, { $or: [{ expiredAt: null }, { expiredAt: { $gt: forcedExpiredAt } }] }],
@ -453,10 +456,10 @@ const cascadeForcedRetentionForConversationSet = async (
{ $set: { expiredAt } },
);
const fileIds = await collectConversationFileIds(Message, userId, conversationIds, seedFileIds);
const fileIdScope = ownedFileIdScope(userId, fileIds);
const fileScope = fileIdScope
? { $or: [{ conversationId: { $in: conversationIds } }, fileIdScope] }
: { conversationId: { $in: conversationIds } };
const fileScope = ownedFileScope(userId, { $in: conversationIds }, fileIds);
if (fileScope == null) {
continue;
}
await File.updateMany(
{
$and: [fileScope, { $or: [{ expiredAt: null }, { expiredAt: { $gt: expiredAt } }] }],
@ -622,7 +625,13 @@ export const sweepForcedRetention = async (
await forceConversationMessagesTemporary(Message, user, conversationId, expiredAt);
await capConversationSharedLinks(SharedLink, user, conversationId, expiredAt);
await capConversationFiles(File, user, conversationId, expiredAt, fileIds);
await Conversation.updateOne({ _id: convo._id }, { $set: { isTemporary: true, expiredAt } });
const convoResult = await Conversation.updateOne(
{ _id: convo._id, ...forcedRetentionGapFilter<IConversation>(expiredAt) },
{ $set: { isTemporary: true, expiredAt } },
);
if ((convoResult.modifiedCount ?? 0) === 0) {
continue;
}
result.conversations += 1;
if (typeof chatProjectId === 'string' && chatProjectId.length > 0) {
const key = `${user}|${chatProjectId}`;