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: owner-scope referenced file caps and include thread file ids

Browse source 
Two gaps in the referenced-file-id capping:

The file_id branch matched File rows with no ownership check, and file ids
inside message/conversation documents are caller-supplied (saveMessage spreads
request params into the update). A crafted or imported message referencing
another user's file_id would therefore set that user's File.expiredAt during a
forced-retention cascade or migration, and the storage sweeper would later
delete their object. The referenced-id branch now always filters on File.user
(dropped entirely when the caller's user id is not a castable ObjectId string —
fail closed), in both capConversationFiles and the bulk tag/project scope. The
conversationId branch stays unscoped: those rows are server-created by the
conversation's own processes, and an id collision can only shorten the
colliding owner's files.

Assistants conversations persist thread uploads on Conversation.file_ids
(saveUserMessage/syncMessages), which the parent projections never read, so
files stored only there kept expiredAt: null after conversion. A shared
conversationSeedFileIds helper now seeds the id collection from both files and
file_ids across the cascade, saveConvo, the bulk cascade, and both sweep
passes.
This commit is contained in:
Marco Beretta 2026-07-05 04:21:21 +02:00
parent 866f2f5afc
commit 5a61dfbe03
No known key found for this signature in database
GPG key ID: D918033D8E74CC11
3 changed files with 100 additions and 40 deletions
Download patch file Download diff file Expand all files Collapse all files

9
packages/data-schemas/src/methods/conversation.ts
View file

@ -17,6 +17,7 @@ import {
capForcedRetentionExpiry,
collectConversationFileIds,
conversationNeedsForcedRetention,
conversationSeedFileIds,
createFallbackRetentionDate,
forceConversationMessagesTemporary,
} from '~/utils/retention';
@ -261,16 +262,18 @@ export function createConversationMethods(
isTemporary?: boolean | null;
expiredAt?: Date | null;
files?: string[];
file_ids?: string[];
} | null = null;
if (mayChangeProjectMembership || isForcedRetention) {
const existing = await Conversation.findOne(
{ conversationId, user: userId },
'chatProjectId isTemporary expiredAt files',
'chatProjectId isTemporary expiredAt files file_ids',
).lean<{
chatProjectId?: string | null;
isTemporary?: boolean | null;
expiredAt?: Date | null;
files?: string[];
file_ids?: string[];
} | null>();
previousChatProjectId = existing?.chatProjectId ?? null;
parentRetention = existing;
@ -343,12 +346,12 @@ export function createConversationMethods(
Message,
userId,
[conversationId],
parentRetention.files,
conversationSeedFileIds(parentRetention),
)
: [];
await forceConversationMessagesTemporary(Message, userId, conversationId, forcedExpiredAt);
await capConversationSharedLinks(SharedLink, userId, conversationId, forcedExpiredAt);
await capConversationFiles(File, conversationId, forcedExpiredAt, fileIds);
await capConversationFiles(File, userId, conversationId, forcedExpiredAt, fileIds);
}
const createdAtOnInsert =

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

@ -1896,12 +1896,14 @@ describe('Message Operations', () => {
it('aligns referenced attachment files that carry no conversationId', async () => {
const forcedExpiredAt = new Date(Date.now() + 24 * 60 * 60 * 1000);
const parentDeadline = new Date(Date.now() + 60 * 60 * 1000);
const ownerObjectId = new mongoose.Types.ObjectId();
const owner = ownerObjectId.toString();
const conversationId = uuidv4();
const messageFileId = uuidv4();
await Conversation().create({
conversationId,
user: 'user123',
user: owner,
endpoint: 'openAI',
isTemporary: true,
expiredAt: parentDeadline,
@ -1909,7 +1911,7 @@ describe('Message Operations', () => {
await Message.create({
messageId: uuidv4(),
conversationId,
user: 'user123',
user: owner,
text: 'with attachment',
isTemporary: true,
expiredAt: parentDeadline,
@ -1917,7 +1919,7 @@ describe('Message Operations', () => {
});
await File().collection.insertOne({
file_id: messageFileId,
user: new mongoose.Types.ObjectId(),
user: ownerObjectId,
expiredAt: null,
});
@ -2117,30 +2119,40 @@ describe('Message Operations', () => {
it('caps referenced attachment files that carry no conversationId when converting', async () => {
const forcedExpiredAt = new Date(Date.now() + 24 * 60 * 60 * 1000);
const ownerObjectId = new mongoose.Types.ObjectId();
const owner = ownerObjectId.toString();
const conversationId = uuidv4();
const messageFileId = uuidv4();
const toolOutputFileId = uuidv4();
const convoFileId = uuidv4();
const threadFileId = uuidv4();
const foreignFileId = uuidv4();
await Conversation().create({
conversationId,
user: 'user123',
user: owner,
endpoint: 'openAI',
isTemporary: false,
files: [convoFileId],
file_ids: [threadFileId],
});
await Message.create({
messageId: uuidv4(),
conversationId,
user: 'user123',
user: owner,
text: 'with attachment',
files: [{ file_id: messageFileId, filename: 'doc.pdf' }],
files: [
{ file_id: messageFileId, filename: 'doc.pdf' },
{ file_id: foreignFileId, filename: 'crafted.pdf' },
],
attachments: [{ file_id: toolOutputFileId, filename: 'chart.png' }],
});
await File().collection.insertMany([
{ file_id: messageFileId, user: new mongoose.Types.ObjectId(), expiredAt: null },
{ file_id: toolOutputFileId, user: new mongoose.Types.ObjectId(), expiredAt: null },
{ file_id: convoFileId, user: new mongoose.Types.ObjectId(), expiredAt: null },
{ file_id: messageFileId, user: ownerObjectId, expiredAt: null },
{ file_id: toolOutputFileId, user: ownerObjectId, expiredAt: null },
{ file_id: convoFileId, user: ownerObjectId, expiredAt: null },
{ file_id: threadFileId, user: ownerObjectId, expiredAt: null },
{ file_id: foreignFileId, user: new mongoose.Types.ObjectId(), expiredAt: null },
]);
await cascadeForcedConversationRetention(
@ -2148,7 +2160,7 @@ describe('Message Operations', () => {
Message,
SharedLink(),
File(),
'user123',
owner,
conversationId,
forcedExpiredAt,
);
@ -2161,6 +2173,16 @@ describe('Message Operations', () => {
const convoFile = await File().findOne({ file_id: convoFileId }).lean();
expect(convoFile?.expiredAt?.getTime()).toBe(forcedExpiredAt.getTime());
const threadFile = await File().findOne({ file_id: threadFileId }).lean();
expect(threadFile?.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();
});
it('caps lagging children even when the parent conversation already conforms', async () => {

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

@ -1,6 +1,7 @@
import type { FilterQuery, Model } from 'mongoose';
import type { AppConfig, IConversation, IMessage, IMongoFile, ISharedLink } from '~/types';
import { createTempChatExpirationDate, DEFAULT_RETENTION_HOURS } from './tempChatRetention';
import { isValidObjectIdString } from './objectId';
import logger from '~/config/winston';
export type RetentionFilterDocument = {
@ -215,6 +216,32 @@ export const collectConversationFileIds = async (
return [...fileIds];
};
/**
* Concatenates a conversation's own file references: `files` (regular uploads) and `file_ids`
* (Assistants thread uploads persisted by saveUserMessage/syncMessages). Used to seed
* {@link collectConversationFileIds} so both reference styles are capped.
*/
export const conversationSeedFileIds = (convo: {
files?: string[] | null;
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 = (
userId: string,
fileIds: string[],
): { file_id: { $in: string[] }; user: string } | null =>
fileIds.length > 0 && isValidObjectIdString(userId)
? { file_id: { $in: fileIds }, user: userId }
: null;
/**
* Caps a conversation's uploaded files to the forced deadline. Files use a retention-scoped
* `expiredAt` swept by application code (`getExpiredFiles` only sweeps files whose own `expiredAt`
@ -224,23 +251,22 @@ export const collectConversationFileIds = async (
* 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; unlike the message/share
* caps the `File.user` field is an `ObjectId` rather than the string user id, so filtering by
* user would require a cast the callers cannot guarantee) plus any referenced `fileIds` —
* message-attachment rows carry no `conversationId`, so conversion-time callers collect the ids
* via {@link collectConversationFileIds}. A shared file referenced from several chats is capped
* to the earliest converting chat's deadline, consistent with cap-don't-extend.
* 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.
* A shared file referenced from several chats is capped to the earliest converting chat's
* deadline, consistent with cap-don't-extend.
*/
export const capConversationFiles = async (
File: Model<IMongoFile>,
userId: string,
conversationId: string,
forcedExpiredAt: Date,
fileIds: string[] = [],
): Promise<number> => {
const scope =
fileIds.length > 0
? { $or: [{ conversationId }, { file_id: { $in: fileIds } }] }
: { conversationId };
const fileIdScope = ownedFileIdScope(userId, fileIds);
const scope = fileIdScope ? { $or: [{ conversationId }, fileIdScope] } : { conversationId };
const result = await File.updateMany(
{
$and: [scope, { $or: [{ expiredAt: null }, { expiredAt: { $gt: forcedExpiredAt } }] }],
@ -300,8 +326,8 @@ export const cascadeForcedConversationRetention = async (
): Promise<boolean> => {
const parent = await Conversation.findOne(
{ conversationId, user: userId },
'isTemporary expiredAt files',
).lean<(RetentionFilterDocument & { files?: string[] }) | null>();
'isTemporary expiredAt files file_ids',
).lean<(RetentionFilterDocument & { files?: string[]; file_ids?: string[] }) | null>();
if (parent == null) {
return false;
}
@ -313,7 +339,12 @@ export const cascadeForcedConversationRetention = async (
* scan over the chat's messages is not needed on every conforming-parent write.
*/
const fileIds = needsConversion
? await collectConversationFileIds(Message, userId, [conversationId], parent.files)
? await collectConversationFileIds(
Message,
userId,
[conversationId],
conversationSeedFileIds(parent),
)
: [];
/**
* Cap the dependent messages, shares, and files independently of the parent gap check, and
@ -325,7 +356,7 @@ export const cascadeForcedConversationRetention = async (
*/
await forceConversationMessagesTemporary(Message, userId, conversationId, expiredAt);
await capConversationSharedLinks(SharedLink, userId, conversationId, expiredAt);
await capConversationFiles(File, conversationId, expiredAt, fileIds);
await capConversationFiles(File, userId, conversationId, expiredAt, fileIds);
if (!needsConversion) {
return false;
}
@ -356,8 +387,12 @@ const cascadeForcedRetentionForConversationSet = async (
): Promise<void> => {
const conversations = await Conversation.find(
{ user: userId, ...conversationMatch } as FilterQuery<IConversation>,
'conversationId isTemporary expiredAt files',
).lean<Array<RetentionFilterDocument & { conversationId?: string; files?: string[] }>>();
'conversationId isTemporary expiredAt files file_ids',
).lean<
Array<
RetentionFilterDocument & { conversationId?: string; files?: string[]; file_ids?: string[] }
>
>();
if (conversations.length === 0) {
return;
}
@ -379,7 +414,7 @@ const cascadeForcedRetentionForConversationSet = async (
seedFileIds: [],
};
bucket.conversationIds.push(convo.conversationId);
for (const fileId of convo.files ?? []) {
for (const fileId of conversationSeedFileIds(convo)) {
bucket.seedFileIds.push(fileId);
}
retentionBuckets.set(key, bucket);
@ -418,10 +453,10 @@ const cascadeForcedRetentionForConversationSet = async (
{ $set: { expiredAt } },
);
const fileIds = await collectConversationFileIds(Message, userId, conversationIds, seedFileIds);
const fileScope =
fileIds.length > 0
? { $or: [{ conversationId: { $in: conversationIds } }, { file_id: { $in: fileIds } }] }
: { conversationId: { $in: conversationIds } };
const fileIdScope = ownedFileIdScope(userId, fileIds);
const fileScope = fileIdScope
? { $or: [{ conversationId: { $in: conversationIds } }, fileIdScope] }
: { conversationId: { $in: conversationIds } };
await File.updateMany(
{
$and: [fileScope, { $or: [{ expiredAt: null }, { expiredAt: { $gt: expiredAt } }] }],
@ -528,7 +563,7 @@ export const sweepForcedRetention = async (
isTemporary: true,
expiredAt: { $ne: null, $lte: forcedExpiredAt },
} as FilterQuery<IConversation>)
.select('conversationId user expiredAt files')
.select('conversationId user expiredAt files file_ids')
.lean()
.cursor();
@ -547,12 +582,12 @@ export const sweepForcedRetention = async (
Message,
user,
[conversationId],
convo.files,
conversationSeedFileIds(convo),
);
const changed =
(await forceConversationMessagesTemporary(Message, user, conversationId, expiredAt)) +
(await capConversationSharedLinks(SharedLink, user, conversationId, expiredAt)) +
(await capConversationFiles(File, conversationId, expiredAt, fileIds));
(await capConversationFiles(File, user, conversationId, expiredAt, fileIds));
if (changed > 0) {
result.aligned += 1;
}
@ -562,7 +597,7 @@ export const sweepForcedRetention = async (
}
const cursor = Conversation.find(forcedRetentionGapFilter<IConversation>(forcedExpiredAt))
.select('_id conversationId user expiredAt chatProjectId files')
.select('_id conversationId user expiredAt chatProjectId files file_ids')
.lean()
.cursor();
@ -577,7 +612,7 @@ export const sweepForcedRetention = async (
Message,
user,
[conversationId],
convo.files,
conversationSeedFileIds(convo),
);
/**
* Convert the dependent messages, shares, and files before marking the conversation itself
@ -586,7 +621,7 @@ export const sweepForcedRetention = async (
*/
await forceConversationMessagesTemporary(Message, user, conversationId, expiredAt);
await capConversationSharedLinks(SharedLink, user, conversationId, expiredAt);
await capConversationFiles(File, conversationId, expiredAt, fileIds);
await capConversationFiles(File, user, conversationId, expiredAt, fileIds);
await Conversation.updateOne({ _id: convo._id }, { $set: { isTemporary: true, expiredAt } });
result.conversations += 1;
if (typeof chatProjectId === 'string' && chatProjectId.length > 0) {