fix: cover retention deletion and pipeline casting

This commit is contained in:
Marco Beretta 2026-07-26 20:47:42 +02:00
parent 2cdd88133d
commit 744e850feb
No known key found for this signature in database
GPG key ID: D918033D8E74CC11
4 changed files with 102 additions and 18 deletions

View file

@ -36,6 +36,7 @@ jest.mock('~/models', () => ({
getMessages: jest.fn(),
updateMessage: jest.fn(),
deleteMessages: jest.fn(),
applyForcedRetention: jest.fn(),
getConvosQueried: jest.fn(),
searchMessages: jest.fn(),
getMessagesByCursor: jest.fn(),
@ -70,7 +71,10 @@ jest.mock('~/server/middleware', () => {
validateMessageReq,
sendValidationResponse,
prepareMessageRequestValidation,
configMiddleware: (req, res, next) => next(),
configMiddleware: (req, res, next) => {
req.config = { interfaceConfig: { retentionMode: 'ephemeral' } };
next();
},
};
});
@ -165,7 +169,7 @@ describe('deleteMessages model-level IDOR prevention', () => {
describe('DELETE /:conversationId/:messageId route handler', () => {
let app;
const { deleteMessages } = require('~/models');
const { deleteMessages, applyForcedRetention } = require('~/models');
const authenticatedUserId = 'user-owner-123';
@ -209,6 +213,14 @@ describe('DELETE /:conversationId/:messageId route handler', () => {
conversationId: 'convo-1',
user: authenticatedUserId,
});
expect(applyForcedRetention).toHaveBeenCalledWith(
'convo-1',
authenticatedUserId,
expect.objectContaining({ retentionMode: 'ephemeral' }),
);
expect(applyForcedRetention.mock.invocationCallOrder[0]).toBeGreaterThan(
deleteMessages.mock.invocationCallOrder[0],
);
});
it('should return 500 when deleteMessages throws', async () => {
@ -218,5 +230,6 @@ describe('DELETE /:conversationId/:messageId route handler', () => {
expect(response.status).toBe(500);
expect(response.body).toEqual({ error: 'Internal server error' });
expect(applyForcedRetention).not.toHaveBeenCalled();
});
});

View file

@ -489,15 +489,21 @@ router.put(
},
);
router.delete('/:conversationId/:messageId', validateMessageReq, async (req, res) => {
try {
const { conversationId, messageId } = req.params;
await db.deleteMessages({ messageId, conversationId, user: req.user.id });
res.status(204).send();
} catch (error) {
logger.error('Error deleting message:', error);
res.status(500).json({ error: 'Internal server error' });
}
});
router.delete(
'/:conversationId/:messageId',
validateMessageReq,
configMiddleware,
async (req, res) => {
try {
const { conversationId, messageId } = req.params;
await db.deleteMessages({ messageId, conversationId, user: req.user.id });
await enforceForcedRetention(req, conversationId);
res.status(204).send();
} catch (error) {
logger.error('Error deleting message:', error);
res.status(500).json({ error: 'Internal server error' });
}
},
);
module.exports = router;

View file

@ -3,7 +3,7 @@ import { v4 as uuidv4 } from 'uuid';
import { MongoMemoryServer } from 'mongodb-memory-server';
import { EToolResources, FileContext, FileSources } from 'librechat-data-provider';
import { _resetStrictCache } from '~/models/plugins/tenantIsolation';
import { runAsSystem } from '~/config/tenantContext';
import { runAsSystem, tenantStorage } from '~/config/tenantContext';
import { createFileMethods } from './file';
import { createModels } from '~/models';
@ -107,6 +107,60 @@ describe('File Methods', () => {
await expect(File.countDocuments({ file_id: fileId, user: userId })).resolves.toBe(1);
});
it('casts caller-supplied timestamps in atomic pipeline upserts', async () => {
const createdAt = '2026-07-26T10:15:30.000Z';
const file = await fileMethods.createFile({
file_id: uuidv4(),
user: new mongoose.Types.ObjectId(),
filename: 'timestamped.png',
filepath: '/uploads/timestamped.png',
type: 'image/png',
bytes: 100,
createdAt: createdAt as unknown as Date,
});
expect(file?.createdAt).toBeInstanceOf(Date);
expect(file?.createdAt?.toISOString()).toBe(createdAt);
});
it('rejects cross-tenant mutation fields before atomic pipeline upserts', async () => {
const userId = new mongoose.Types.ObjectId();
await expect(
tenantStorage.run({ tenantId: 'tenant-a' }, async () =>
fileMethods.createFile({
file_id: uuidv4(),
user: userId,
tenantId: 'tenant-b',
filename: 'cross-tenant.txt',
filepath: '/uploads/cross-tenant.txt',
type: 'text/plain',
bytes: 100,
}),
),
).rejects.toThrow('Cross-tenant tenantId mutation is not allowed');
});
it('derives matching tenant ids from the tenant-scoped upsert filter', async () => {
const fileId = uuidv4();
const file = await tenantStorage.run({ tenantId: 'tenant-a' }, async () =>
fileMethods.createFile({
file_id: fileId,
user: new mongoose.Types.ObjectId(),
tenantId: 'tenant-a',
filename: 'tenant-owned.txt',
filepath: '/uploads/tenant-owned.txt',
type: 'text/plain',
bytes: 100,
}),
);
expect(file?.tenantId).toBe('tenant-a');
await expect(
runAsSystem(() => File.countDocuments({ file_id: fileId, tenantId: 'tenant-a' })),
).resolves.toBe(1);
});
it('updates expiredAt monotonically with an atomic minimum', async () => {
const fileId = uuidv4();
const userId = new mongoose.Types.ObjectId();

View file

@ -1,6 +1,7 @@
import { EToolResources, FileContext, FileSources } from 'librechat-data-provider';
import type { FilterQuery, SortOrder, Model } from 'mongoose';
import type { IMongoFile } from '~/types/file';
import { getTenantId, SYSTEM_TENANT_ID } from '~/config/tenantContext';
import { tenantSafeBulkWrite } from '~/utils/tenantBulkWrite';
import logger from '../config/winston';
@ -364,13 +365,23 @@ export function createFileMethods(mongoose: typeof import('mongoose')): {
delete fileData.expiresAt;
}
const { expiredAt, ...fields } = fileData;
const definedFields = Object.fromEntries(
Object.entries(fields).filter(([, value]) => value !== undefined),
const uncastFields = Object.fromEntries(
Object.entries(fileData).filter(([, value]) => value !== undefined),
);
if (definedFields.user != null) {
definedFields.user = File.schema.path('user').cast(definedFields.user);
const activeTenantId = getTenantId();
if (activeTenantId !== SYSTEM_TENANT_ID) {
if (
activeTenantId &&
Object.prototype.hasOwnProperty.call(uncastFields, 'tenantId') &&
uncastFields.tenantId !== activeTenantId
) {
throw new Error('[TenantIsolation] Cross-tenant tenantId mutation is not allowed');
}
delete uncastFields.tenantId;
}
const { expiredAt, ...definedFields } = File.castObject(uncastFields);
const insertDefaults = {
object: { $ifNull: ['$object', 'file'] },
usage: { $ifNull: ['$usage', 0] },