From 744e850febdfd6f38f7ca30c2fc1d7a7bba4d133 Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Sun, 26 Jul 2026 20:47:42 +0200 Subject: [PATCH] fix: cover retention deletion and pipeline casting --- .../routes/__tests__/messages-delete.spec.js | 17 +++++- api/server/routes/messages.js | 26 +++++---- .../data-schemas/src/methods/file.spec.ts | 56 ++++++++++++++++++- packages/data-schemas/src/methods/file.ts | 21 +++++-- 4 files changed, 102 insertions(+), 18 deletions(-) diff --git a/api/server/routes/__tests__/messages-delete.spec.js b/api/server/routes/__tests__/messages-delete.spec.js index 44ffaf56d7..f166f1d211 100644 --- a/api/server/routes/__tests__/messages-delete.spec.js +++ b/api/server/routes/__tests__/messages-delete.spec.js @@ -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(); }); }); diff --git a/api/server/routes/messages.js b/api/server/routes/messages.js index 857cca2b19..81ac05a3a9 100644 --- a/api/server/routes/messages.js +++ b/api/server/routes/messages.js @@ -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; diff --git a/packages/data-schemas/src/methods/file.spec.ts b/packages/data-schemas/src/methods/file.spec.ts index 1820a822c9..f31c639d1f 100644 --- a/packages/data-schemas/src/methods/file.spec.ts +++ b/packages/data-schemas/src/methods/file.spec.ts @@ -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(); diff --git a/packages/data-schemas/src/methods/file.ts b/packages/data-schemas/src/methods/file.ts index 1f5b938095..92fb024a41 100644 --- a/packages/data-schemas/src/methods/file.ts +++ b/packages/data-schemas/src/methods/file.ts @@ -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] },