mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-07-10 16:23:44 +00:00
fix: enforce forced retention on message edits, feedback, and error saves
Two more message-write paths bypassed ephemeral enforcement: - The edit and feedback endpoints call updateMessage directly, without loading retention config, so editing an older permanent message after a switch to ephemeral left the message and its conversation non-temporary and visible. Load config on those routes and run a new applyForcedRetention helper after the update, which stamps the message and cascades the conversation/messages. - The sendError and denyRequest middleware save messages with retention config but never call saveConvo, so a validation/model error or denied-request message could outlive its conversation. Pass capExpiryToConversation like the other message-only paths. Extract the conversation cascade into a shared cascadeForcedConversationRetention helper used by both saveMessage and applyForcedRetention.
This commit is contained in:
parent
571de8c3db
commit
84ab681adf
6 changed files with 264 additions and 90 deletions
|
|
@ -49,7 +49,10 @@ const denyRequest = async (req, res, errorMessage) => {
|
|||
interfaceConfig: req?.config?.interfaceConfig,
|
||||
},
|
||||
{ ...userMessage, user: req.user.id },
|
||||
{ context: `api/server/middleware/denyRequest.js - ${responseText}` },
|
||||
{
|
||||
context: `api/server/middleware/denyRequest.js - ${responseText}`,
|
||||
capExpiryToConversation: true,
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -56,6 +56,7 @@ const sendError = async (req, res, options, callback) => {
|
|||
{ ...errorMessage, user },
|
||||
{
|
||||
context: 'api/server/utils/streamResponse.js - sendError',
|
||||
capExpiryToConversation: true,
|
||||
},
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -16,6 +16,17 @@ const db = require('~/models');
|
|||
const router = express.Router();
|
||||
router.use(requireJwtAuth);
|
||||
|
||||
/**
|
||||
* Enforces forced (ephemeral) retention after a message-only update (edit/feedback),
|
||||
* which bypasses the saveMessage/saveConvo enforcement. No-op outside forced retention.
|
||||
*/
|
||||
const enforceForcedRetention = (req, conversationId, messageId, context) =>
|
||||
db.applyForcedRetention(
|
||||
{ userId: req?.user?.id, interfaceConfig: req?.config?.interfaceConfig },
|
||||
{ conversationId, messageId },
|
||||
{ context, capExpiryToConversation: true },
|
||||
);
|
||||
|
||||
router.get('/', async (req, res) => {
|
||||
try {
|
||||
const user = req.user.id ?? '';
|
||||
|
|
@ -324,79 +335,96 @@ router.get('/:conversationId/:messageId', validateMessageReq, async (req, res) =
|
|||
}
|
||||
});
|
||||
|
||||
router.put('/:conversationId/:messageId', validateMessageReq, async (req, res) => {
|
||||
try {
|
||||
const { conversationId, messageId } = req.params;
|
||||
const { text, index, model } = req.body;
|
||||
router.put(
|
||||
'/:conversationId/:messageId',
|
||||
validateMessageReq,
|
||||
configMiddleware,
|
||||
async (req, res) => {
|
||||
try {
|
||||
const { conversationId, messageId } = req.params;
|
||||
const { text, index, model } = req.body;
|
||||
|
||||
if (index === undefined) {
|
||||
/** A user turn's persisted `quotes` are re-prepended into the prompt on
|
||||
* every send, but this edit only changes `text`. Count the merged
|
||||
* text+quotes so the stored `tokenCount` stays authoritative (matching the
|
||||
* send path); a plain text-only count under-reports by the quote block. */
|
||||
const existing = (
|
||||
await db.getMessages(
|
||||
{ conversationId, messageId, user: req.user.id },
|
||||
'quotes isCreatedByUser',
|
||||
)
|
||||
if (index === undefined) {
|
||||
/** A user turn's persisted `quotes` are re-prepended into the prompt on
|
||||
* every send, but this edit only changes `text`. Count the merged
|
||||
* text+quotes so the stored `tokenCount` stays authoritative (matching the
|
||||
* send path); a plain text-only count under-reports by the quote block. */
|
||||
const existing = (
|
||||
await db.getMessages(
|
||||
{ conversationId, messageId, user: req.user.id },
|
||||
'quotes isCreatedByUser',
|
||||
)
|
||||
)?.[0];
|
||||
const textToCount = mergeQuotedTextForCount(
|
||||
text,
|
||||
existing?.quotes,
|
||||
existing?.isCreatedByUser === true,
|
||||
);
|
||||
const tokenCount = await countTokens(textToCount, model);
|
||||
const result = await db.updateMessage(req?.user?.id, { messageId, text, tokenCount });
|
||||
await enforceForcedRetention(
|
||||
req,
|
||||
conversationId,
|
||||
messageId,
|
||||
'PUT /api/messages - edit text',
|
||||
);
|
||||
return res.status(200).json(result);
|
||||
}
|
||||
|
||||
if (typeof index !== 'number' || index < 0) {
|
||||
return res.status(400).json({ error: 'Invalid index' });
|
||||
}
|
||||
|
||||
const message = (
|
||||
await db.getMessages({ conversationId, messageId, user: req.user.id }, 'content tokenCount')
|
||||
)?.[0];
|
||||
const textToCount = mergeQuotedTextForCount(
|
||||
text,
|
||||
existing?.quotes,
|
||||
existing?.isCreatedByUser === true,
|
||||
if (!message) {
|
||||
return res.status(404).json({ error: 'Message not found' });
|
||||
}
|
||||
|
||||
const existingContent = message.content;
|
||||
if (!Array.isArray(existingContent) || index >= existingContent.length) {
|
||||
return res.status(400).json({ error: 'Invalid index' });
|
||||
}
|
||||
|
||||
const updatedContent = [...existingContent];
|
||||
if (!updatedContent[index]) {
|
||||
return res.status(400).json({ error: 'Content part not found' });
|
||||
}
|
||||
|
||||
const currentPartType = updatedContent[index].type;
|
||||
if (currentPartType !== ContentTypes.TEXT && currentPartType !== ContentTypes.THINK) {
|
||||
return res.status(400).json({ error: 'Cannot update non-text content' });
|
||||
}
|
||||
|
||||
const oldText = updatedContent[index][currentPartType];
|
||||
updatedContent[index] = { type: currentPartType, [currentPartType]: text };
|
||||
|
||||
let tokenCount = message.tokenCount;
|
||||
if (tokenCount !== undefined) {
|
||||
const oldTokenCount = await countTokens(oldText, model);
|
||||
const newTokenCount = await countTokens(text, model);
|
||||
tokenCount = Math.max(0, tokenCount - oldTokenCount) + newTokenCount;
|
||||
}
|
||||
|
||||
const result = await db.updateMessage(req?.user?.id, {
|
||||
messageId,
|
||||
content: updatedContent,
|
||||
tokenCount,
|
||||
});
|
||||
await enforceForcedRetention(
|
||||
req,
|
||||
conversationId,
|
||||
messageId,
|
||||
'PUT /api/messages - edit content',
|
||||
);
|
||||
const tokenCount = await countTokens(textToCount, model);
|
||||
const result = await db.updateMessage(req?.user?.id, { messageId, text, tokenCount });
|
||||
return res.status(200).json(result);
|
||||
} catch (error) {
|
||||
logger.error('Error updating message:', error);
|
||||
res.status(500).json({ error: 'Internal server error' });
|
||||
}
|
||||
|
||||
if (typeof index !== 'number' || index < 0) {
|
||||
return res.status(400).json({ error: 'Invalid index' });
|
||||
}
|
||||
|
||||
const message = (
|
||||
await db.getMessages({ conversationId, messageId, user: req.user.id }, 'content tokenCount')
|
||||
)?.[0];
|
||||
if (!message) {
|
||||
return res.status(404).json({ error: 'Message not found' });
|
||||
}
|
||||
|
||||
const existingContent = message.content;
|
||||
if (!Array.isArray(existingContent) || index >= existingContent.length) {
|
||||
return res.status(400).json({ error: 'Invalid index' });
|
||||
}
|
||||
|
||||
const updatedContent = [...existingContent];
|
||||
if (!updatedContent[index]) {
|
||||
return res.status(400).json({ error: 'Content part not found' });
|
||||
}
|
||||
|
||||
const currentPartType = updatedContent[index].type;
|
||||
if (currentPartType !== ContentTypes.TEXT && currentPartType !== ContentTypes.THINK) {
|
||||
return res.status(400).json({ error: 'Cannot update non-text content' });
|
||||
}
|
||||
|
||||
const oldText = updatedContent[index][currentPartType];
|
||||
updatedContent[index] = { type: currentPartType, [currentPartType]: text };
|
||||
|
||||
let tokenCount = message.tokenCount;
|
||||
if (tokenCount !== undefined) {
|
||||
const oldTokenCount = await countTokens(oldText, model);
|
||||
const newTokenCount = await countTokens(text, model);
|
||||
tokenCount = Math.max(0, tokenCount - oldTokenCount) + newTokenCount;
|
||||
}
|
||||
|
||||
const result = await db.updateMessage(req?.user?.id, {
|
||||
messageId,
|
||||
content: updatedContent,
|
||||
tokenCount,
|
||||
});
|
||||
return res.status(200).json(result);
|
||||
} catch (error) {
|
||||
logger.error('Error updating message:', error);
|
||||
res.status(500).json({ error: 'Internal server error' });
|
||||
}
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
router.put(
|
||||
'/:conversationId/:messageId/feedback',
|
||||
|
|
@ -415,6 +443,8 @@ router.put(
|
|||
},
|
||||
{ context: 'updateFeedback' },
|
||||
);
|
||||
await enforceForcedRetention(req, conversationId, messageId, 'PUT /api/messages - feedback');
|
||||
|
||||
|
||||
// Best-effort: Assistants messages do not have deterministic AgentRun traces.
|
||||
if (!isAssistantsEndpoint(updatedMessage.endpoint)) {
|
||||
|
|
|
|||
|
|
@ -22,6 +22,7 @@ let Message: mongoose.Model<IMessage>;
|
|||
let saveMessage: ReturnType<typeof createMessageMethods>['saveMessage'];
|
||||
let getMessages: ReturnType<typeof createMessageMethods>['getMessages'];
|
||||
let updateMessage: ReturnType<typeof createMessageMethods>['updateMessage'];
|
||||
let applyForcedRetention: ReturnType<typeof createMessageMethods>['applyForcedRetention'];
|
||||
let deleteMessages: ReturnType<typeof createMessageMethods>['deleteMessages'];
|
||||
let bulkSaveMessages: ReturnType<typeof createMessageMethods>['bulkSaveMessages'];
|
||||
let updateMessageText: ReturnType<typeof createMessageMethods>['updateMessageText'];
|
||||
|
|
@ -40,6 +41,7 @@ beforeAll(async () => {
|
|||
saveMessage = methods.saveMessage;
|
||||
getMessages = methods.getMessages;
|
||||
updateMessage = methods.updateMessage;
|
||||
applyForcedRetention = methods.applyForcedRetention;
|
||||
deleteMessages = methods.deleteMessages;
|
||||
bulkSaveMessages = methods.bulkSaveMessages;
|
||||
updateMessageText = methods.updateMessageText;
|
||||
|
|
@ -945,6 +947,75 @@ describe('Message Operations', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('applyForcedRetention', () => {
|
||||
const Conversation = () => mongoose.models.Conversation as mongoose.Model<IConversation>;
|
||||
|
||||
beforeEach(async () => {
|
||||
await Conversation().deleteMany({});
|
||||
});
|
||||
|
||||
it('converts a permanent message and its conversation when editing under ephemeral mode', async () => {
|
||||
const conversationId = uuidv4();
|
||||
const editedMessageId = uuidv4();
|
||||
await Conversation().create({
|
||||
conversationId,
|
||||
user: 'user123',
|
||||
endpoint: 'openAI',
|
||||
title: 'Existing permanent chat',
|
||||
});
|
||||
await Message.create([
|
||||
{ messageId: editedMessageId, conversationId, user: 'user123', text: 'first' },
|
||||
{ messageId: uuidv4(), conversationId, user: 'user123', text: 'second' },
|
||||
]);
|
||||
|
||||
await applyForcedRetention(
|
||||
{
|
||||
userId: 'user123',
|
||||
interfaceConfig: { temporaryChatRetention: 24, retentionMode: RetentionMode.EPHEMERAL },
|
||||
},
|
||||
{ conversationId, messageId: editedMessageId },
|
||||
{ context: 'edit', capExpiryToConversation: true },
|
||||
);
|
||||
|
||||
const convo = await Conversation().findOne({ conversationId }).lean();
|
||||
expect(convo?.isTemporary).toBe(true);
|
||||
expect(convo?.expiredAt).toBeInstanceOf(Date);
|
||||
|
||||
const messages = await getMessages({ conversationId, user: 'user123' });
|
||||
expect(messages).toHaveLength(2);
|
||||
for (const message of messages) {
|
||||
expect(message.isTemporary).toBe(true);
|
||||
expect(message.expiredAt).toBeInstanceOf(Date);
|
||||
}
|
||||
});
|
||||
|
||||
it('is a no-op outside forced retention', async () => {
|
||||
const conversationId = uuidv4();
|
||||
const messageId = uuidv4();
|
||||
await Conversation().create({
|
||||
conversationId,
|
||||
user: 'user123',
|
||||
endpoint: 'openAI',
|
||||
title: 'Existing permanent chat',
|
||||
});
|
||||
await Message.create({ messageId, conversationId, user: 'user123', text: 'first' });
|
||||
|
||||
await applyForcedRetention(
|
||||
{
|
||||
userId: 'user123',
|
||||
interfaceConfig: { temporaryChatRetention: 24, retentionMode: RetentionMode.TEMPORARY },
|
||||
},
|
||||
{ conversationId, messageId },
|
||||
{ context: 'edit', capExpiryToConversation: true },
|
||||
);
|
||||
|
||||
const convo = await Conversation().findOne({ conversationId }).lean();
|
||||
expect(convo?.expiredAt ?? null).toBeNull();
|
||||
const message = await Message.findOne({ messageId }).lean();
|
||||
expect(message?.expiredAt ?? null).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('Message cursor pagination', () => {
|
||||
/**
|
||||
* Helper to create messages with specific timestamps
|
||||
|
|
|
|||
|
|
@ -1,11 +1,7 @@
|
|||
import { RetentionMode, isForcedTemporaryRetention } from 'librechat-data-provider';
|
||||
import type { DeleteResult, FilterQuery, Model } from 'mongoose';
|
||||
import type { AppConfig, IConversation, IMessage } from '~/types';
|
||||
import {
|
||||
createFallbackRetentionDate,
|
||||
forceConversationMessagesTemporary,
|
||||
forcedRetentionGapFilter,
|
||||
} from '~/utils/retention';
|
||||
import { cascadeForcedConversationRetention, createFallbackRetentionDate } from '~/utils/retention';
|
||||
import { createTempChatExpirationDate } from '~/utils/tempChatRetention';
|
||||
import { tenantSafeBulkWrite } from '~/utils/tenantBulkWrite';
|
||||
import logger from '~/config/winston';
|
||||
|
|
@ -42,6 +38,11 @@ export interface MessageMethods {
|
|||
message: Partial<IMessage> & { newMessageId?: string },
|
||||
metadata?: { context?: string },
|
||||
): Promise<Partial<IMessage>>;
|
||||
applyForcedRetention(
|
||||
ctx: { userId: string; interfaceConfig?: AppConfig['interfaceConfig'] },
|
||||
params: { conversationId: string; messageId: string },
|
||||
metadata?: { context?: string; capExpiryToConversation?: boolean },
|
||||
): Promise<void>;
|
||||
deleteMessagesSince(
|
||||
userId: string,
|
||||
params: { messageId: string; conversationId: string },
|
||||
|
|
@ -196,22 +197,13 @@ export function createMessageMethods(mongoose: typeof import('mongoose')): Messa
|
|||
|
||||
if (isForcedRetention && forcedExpiredAt instanceof Date) {
|
||||
const Conversation = mongoose.models.Conversation as Model<IConversation>;
|
||||
const convoResult = await Conversation.updateOne(
|
||||
{
|
||||
conversationId,
|
||||
user: userId,
|
||||
...forcedRetentionGapFilter<IConversation>(forcedExpiredAt),
|
||||
},
|
||||
{ $set: { isTemporary: true, expiredAt: forcedExpiredAt } },
|
||||
await cascadeForcedConversationRetention(
|
||||
Conversation,
|
||||
Message,
|
||||
userId,
|
||||
conversationId,
|
||||
forcedExpiredAt,
|
||||
);
|
||||
if (convoResult.modifiedCount > 0) {
|
||||
await forceConversationMessagesTemporary(
|
||||
Message,
|
||||
userId,
|
||||
conversationId,
|
||||
forcedExpiredAt,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
return message.toObject();
|
||||
|
|
@ -366,6 +358,60 @@ export function createMessageMethods(mongoose: typeof import('mongoose')): Messa
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Enforces forced (ephemeral) retention on an existing message and its parent
|
||||
* conversation. Message-write paths that only update a row — edits, feedback — bypass
|
||||
* the `saveMessage`/`saveConvo` enforcement, so an older permanent chat touched after an
|
||||
* install switches to ephemeral would otherwise stay visible and never expire.
|
||||
*/
|
||||
async function applyForcedRetention(
|
||||
{ userId, interfaceConfig }: { userId: string; interfaceConfig?: AppConfig['interfaceConfig'] },
|
||||
{ conversationId, messageId }: { conversationId: string; messageId: string },
|
||||
metadata?: { context?: string; capExpiryToConversation?: boolean },
|
||||
): Promise<void> {
|
||||
if (!isForcedTemporaryRetention(interfaceConfig?.retentionMode)) {
|
||||
return;
|
||||
}
|
||||
|
||||
let forcedExpiredAt: Date;
|
||||
try {
|
||||
forcedExpiredAt = createTempChatExpirationDate(interfaceConfig);
|
||||
} catch (err) {
|
||||
logger.error('Error creating temporary chat expiration date:', err);
|
||||
logger.info(`---\`applyForcedRetention\` context: ${metadata?.context}`);
|
||||
forcedExpiredAt = createFallbackRetentionDate();
|
||||
}
|
||||
|
||||
const Message = mongoose.models.Message as Model<IMessage>;
|
||||
const Conversation = mongoose.models.Conversation as Model<IConversation>;
|
||||
|
||||
if (metadata?.capExpiryToConversation === true) {
|
||||
const parent = await Conversation.findOne(
|
||||
{ conversationId, user: userId },
|
||||
'isTemporary expiredAt',
|
||||
).lean<{ isTemporary?: boolean | null; expiredAt?: Date | null } | null>();
|
||||
if (
|
||||
parent?.isTemporary === true &&
|
||||
parent.expiredAt != null &&
|
||||
parent.expiredAt.getTime() < forcedExpiredAt.getTime()
|
||||
) {
|
||||
forcedExpiredAt = parent.expiredAt;
|
||||
}
|
||||
}
|
||||
|
||||
await Message.updateOne(
|
||||
{ messageId, user: userId },
|
||||
{ $set: { isTemporary: true, expiredAt: forcedExpiredAt } },
|
||||
);
|
||||
await cascadeForcedConversationRetention(
|
||||
Conversation,
|
||||
Message,
|
||||
userId,
|
||||
conversationId,
|
||||
forcedExpiredAt,
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Deletes messages in a conversation since a specific message.
|
||||
*/
|
||||
|
|
@ -502,6 +548,7 @@ export function createMessageMethods(mongoose: typeof import('mongoose')): Messa
|
|||
recordMessage,
|
||||
updateMessageText,
|
||||
updateMessage,
|
||||
applyForcedRetention,
|
||||
deleteMessagesSince,
|
||||
getMessages,
|
||||
getMessage,
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
import type { FilterQuery, Model } from 'mongoose';
|
||||
import type { IMessage } from '~/types';
|
||||
import type { IConversation, IMessage } from '~/types';
|
||||
import { DEFAULT_RETENTION_HOURS } from './tempChatRetention';
|
||||
|
||||
export type RetentionFilterDocument = {
|
||||
|
|
@ -90,3 +90,25 @@ export const forceConversationMessagesTemporary = async (
|
|||
{ $set: { isTemporary: true, expiredAt } },
|
||||
);
|
||||
};
|
||||
|
||||
/**
|
||||
* Converts or re-caps a parent conversation to the forced deadline and, when that first
|
||||
* brings the conversation into the forced window, backfills its lagging messages. Shared
|
||||
* by every forced-retention message-write path so a single conversation/message rule is
|
||||
* enforced regardless of which save touched the chat.
|
||||
*/
|
||||
export const cascadeForcedConversationRetention = async (
|
||||
Conversation: Model<IConversation>,
|
||||
Message: Model<IMessage>,
|
||||
userId: string,
|
||||
conversationId: string,
|
||||
forcedExpiredAt: Date,
|
||||
): Promise<void> => {
|
||||
const convoResult = await Conversation.updateOne(
|
||||
{ conversationId, user: userId, ...forcedRetentionGapFilter<IConversation>(forcedExpiredAt) },
|
||||
{ $set: { isTemporary: true, expiredAt: forcedExpiredAt } },
|
||||
);
|
||||
if (convoResult.modifiedCount > 0) {
|
||||
await forceConversationMessagesTemporary(Message, userId, conversationId, forcedExpiredAt);
|
||||
}
|
||||
};
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue