🚦 fix: Add Rate Limiting to Conversation Duplicate Endpoint (#12218)

* fix: add rate limiting to conversation duplicate endpoint

* chore: linter

* fix: address review findings for conversation duplicate rate limiting

* refactor: streamline test mocks for conversation routes

- Consolidated mock implementations into a dedicated `convos-route-mocks.js` file to enhance maintainability and readability of test files.
- Updated tests in `convos-duplicate-ratelimit.spec.js` and `convos.spec.js` to utilize the new mock structure, improving clarity and reducing redundancy.
- Enhanced the `duplicateConversation` function to accept an optional title parameter for better flexibility in conversation duplication.

* chore: rename files

api/server/middleware/limiters/forkLimiters.js

@@ -48,7 +48,7 @@ const createForkHandler = (ip = true) => {
     };
 
     await logViolation(req, res, type, errorMessage, forkViolationScore);
-    res.status(429).json({ message: 'Too many conversation fork requests. Try again later' });
+    res.status(429).json({ message: 'Too many requests. Try again later' });
   };
 };
 

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

@@ -0,0 +1,92 @@
+module.exports = {
+  agents: () => ({ sleep: jest.fn() }),
+
+  api: (overrides = {}) => ({
+    isEnabled: jest.fn(),
+    createAxiosInstance: jest.fn(() => ({
+      get: jest.fn(),
+      post: jest.fn(),
+      put: jest.fn(),
+      delete: jest.fn(),
+    })),
+    logAxiosError: jest.fn(),
+    ...overrides,
+  }),
+
+  dataSchemas: () => ({
+    logger: {
+      debug: jest.fn(),
+      info: jest.fn(),
+      warn: jest.fn(),
+      error: jest.fn(),
+    },
+    createModels: jest.fn(() => ({
+      User: {},
+      Conversation: {},
+      Message: {},
+      SharedLink: {},
+    })),
+  }),
+
+  dataProvider: (overrides = {}) => ({
+    CacheKeys: { GEN_TITLE: 'GEN_TITLE' },
+    EModelEndpoint: {
+      azureAssistants: 'azureAssistants',
+      assistants: 'assistants',
+    },
+    ...overrides,
+  }),
+
+  conversationModel: () => ({
+    getConvosByCursor: jest.fn(),
+    getConvo: jest.fn(),
+    deleteConvos: jest.fn(),
+    saveConvo: jest.fn(),
+  }),
+
+  toolCallModel: () => ({ deleteToolCalls: jest.fn() }),
+
+  sharedModels: () => ({
+    deleteAllSharedLinks: jest.fn(),
+    deleteConvoSharedLink: jest.fn(),
+  }),
+
+  requireJwtAuth: () => (req, res, next) => next(),
+
+  middlewarePassthrough: () => ({
+    createImportLimiters: jest.fn(() => ({
+      importIpLimiter: (req, res, next) => next(),
+      importUserLimiter: (req, res, next) => next(),
+    })),
+    createForkLimiters: jest.fn(() => ({
+      forkIpLimiter: (req, res, next) => next(),
+      forkUserLimiter: (req, res, next) => next(),
+    })),
+    configMiddleware: (req, res, next) => next(),
+    validateConvoAccess: (req, res, next) => next(),
+  }),
+
+  forkUtils: () => ({
+    forkConversation: jest.fn(),
+    duplicateConversation: jest.fn(),
+  }),
+
+  importUtils: () => ({ importConversations: jest.fn() }),
+
+  logStores: () => jest.fn(),
+
+  multerSetup: () => ({
+    storage: {},
+    importFileFilter: jest.fn(),
+  }),
+
+  multerLib: () =>
+    jest.fn(() => ({
+      single: jest.fn(() => (req, res, next) => {
+        req.file = { path: '/tmp/test-file.json' };
+        next();
+      }),
+    })),
+
+  assistantEndpoint: () => ({ initializeClient: jest.fn() }),
+};

api/server/routes/__tests__/convos-duplicate-ratelimit.spec.js

@@ -0,0 +1,135 @@
+const express = require('express');
+const request = require('supertest');
+
+const MOCKS = '../__test-utils__/convos-route-mocks';
+
+jest.mock('@librechat/agents', () => require(MOCKS).agents());
+jest.mock('@librechat/api', () => require(MOCKS).api({ limiterCache: jest.fn(() => undefined) }));
+jest.mock('@librechat/data-schemas', () => require(MOCKS).dataSchemas());
+jest.mock('librechat-data-provider', () =>
+  require(MOCKS).dataProvider({ ViolationTypes: { FILE_UPLOAD_LIMIT: 'file_upload_limit' } }),
+);
+
+jest.mock('~/cache/logViolation', () => jest.fn().mockResolvedValue(undefined));
+jest.mock('~/cache/getLogStores', () => require(MOCKS).logStores());
+jest.mock('~/models/Conversation', () => require(MOCKS).conversationModel());
+jest.mock('~/models/ToolCall', () => require(MOCKS).toolCallModel());
+jest.mock('~/models', () => require(MOCKS).sharedModels());
+jest.mock('~/server/middleware/requireJwtAuth', () => require(MOCKS).requireJwtAuth());
+
+jest.mock('~/server/middleware', () => {
+  const { createForkLimiters } = jest.requireActual('~/server/middleware/limiters/forkLimiters');
+  return {
+    createImportLimiters: jest.fn(() => ({
+      importIpLimiter: (req, res, next) => next(),
+      importUserLimiter: (req, res, next) => next(),
+    })),
+    createForkLimiters,
+    configMiddleware: (req, res, next) => next(),
+    validateConvoAccess: (req, res, next) => next(),
+  };
+});
+
+jest.mock('~/server/utils/import/fork', () => require(MOCKS).forkUtils());
+jest.mock('~/server/utils/import', () => require(MOCKS).importUtils());
+jest.mock('~/server/routes/files/multer', () => require(MOCKS).multerSetup());
+jest.mock('multer', () => require(MOCKS).multerLib());
+jest.mock('~/server/services/Endpoints/azureAssistants', () => require(MOCKS).assistantEndpoint());
+jest.mock('~/server/services/Endpoints/assistants', () => require(MOCKS).assistantEndpoint());
+
+describe('POST /api/convos/duplicate - Rate Limiting', () => {
+  let app;
+  let duplicateConversation;
+  const savedEnv = {};
+
+  beforeAll(() => {
+    savedEnv.FORK_USER_MAX = process.env.FORK_USER_MAX;
+    savedEnv.FORK_USER_WINDOW = process.env.FORK_USER_WINDOW;
+    savedEnv.FORK_IP_MAX = process.env.FORK_IP_MAX;
+    savedEnv.FORK_IP_WINDOW = process.env.FORK_IP_WINDOW;
+  });
+
+  afterAll(() => {
+    for (const key of Object.keys(savedEnv)) {
+      if (savedEnv[key] === undefined) {
+        delete process.env[key];
+      } else {
+        process.env[key] = savedEnv[key];
+      }
+    }
+  });
+
+  const setupApp = () => {
+    jest.clearAllMocks();
+    jest.isolateModules(() => {
+      const convosRouter = require('../convos');
+      ({ duplicateConversation } = require('~/server/utils/import/fork'));
+
+      app = express();
+      app.use(express.json());
+      app.use((req, res, next) => {
+        req.user = { id: 'rate-limit-test-user' };
+        next();
+      });
+      app.use('/api/convos', convosRouter);
+    });
+
+    duplicateConversation.mockResolvedValue({
+      conversation: { conversationId: 'duplicated-conv' },
+    });
+  };
+
+  describe('user limit', () => {
+    beforeEach(() => {
+      process.env.FORK_USER_MAX = '2';
+      process.env.FORK_USER_WINDOW = '1';
+      process.env.FORK_IP_MAX = '100';
+      process.env.FORK_IP_WINDOW = '1';
+      setupApp();
+    });
+
+    it('should return 429 after exceeding the user rate limit', async () => {
+      const userMax = parseInt(process.env.FORK_USER_MAX, 10);
+
+      for (let i = 0; i < userMax; i++) {
+        const res = await request(app)
+          .post('/api/convos/duplicate')
+          .send({ conversationId: 'conv-123' });
+        expect(res.status).toBe(201);
+      }
+
+      const res = await request(app)
+        .post('/api/convos/duplicate')
+        .send({ conversationId: 'conv-123' });
+      expect(res.status).toBe(429);
+      expect(res.body.message).toMatch(/too many/i);
+    });
+  });
+
+  describe('IP limit', () => {
+    beforeEach(() => {
+      process.env.FORK_USER_MAX = '100';
+      process.env.FORK_USER_WINDOW = '1';
+      process.env.FORK_IP_MAX = '2';
+      process.env.FORK_IP_WINDOW = '1';
+      setupApp();
+    });
+
+    it('should return 429 after exceeding the IP rate limit', async () => {
+      const ipMax = parseInt(process.env.FORK_IP_MAX, 10);
+
+      for (let i = 0; i < ipMax; i++) {
+        const res = await request(app)
+          .post('/api/convos/duplicate')
+          .send({ conversationId: 'conv-123' });
+        expect(res.status).toBe(201);
+      }
+
+      const res = await request(app)
+        .post('/api/convos/duplicate')
+        .send({ conversationId: 'conv-123' });
+      expect(res.status).toBe(429);
+      expect(res.body.message).toMatch(/too many/i);
+    });
+  });
+});

api/server/routes/__tests__/convos.spec.js

@@ -1,109 +1,24 @@
 const express = require('express');
 const request = require('supertest');
 
-jest.mock('@librechat/agents', () => ({
+const MOCKS = '../__test-utils__/convos-route-mocks';
-  sleep: jest.fn(),
-}));
 
-jest.mock('@librechat/api', () => ({
+jest.mock('@librechat/agents', () => require(MOCKS).agents());
-  isEnabled: jest.fn(),
+jest.mock('@librechat/api', () => require(MOCKS).api());
-  createAxiosInstance: jest.fn(() => ({
+jest.mock('@librechat/data-schemas', () => require(MOCKS).dataSchemas());
-    get: jest.fn(),
+jest.mock('librechat-data-provider', () => require(MOCKS).dataProvider());
-    post: jest.fn(),
+jest.mock('~/models/Conversation', () => require(MOCKS).conversationModel());
-    put: jest.fn(),
+jest.mock('~/models/ToolCall', () => require(MOCKS).toolCallModel());
-    delete: jest.fn(),
+jest.mock('~/models', () => require(MOCKS).sharedModels());
-  })),
+jest.mock('~/server/middleware/requireJwtAuth', () => require(MOCKS).requireJwtAuth());
-  logAxiosError: jest.fn(),
+jest.mock('~/server/middleware', () => require(MOCKS).middlewarePassthrough());
-}));
+jest.mock('~/server/utils/import/fork', () => require(MOCKS).forkUtils());
-
+jest.mock('~/server/utils/import', () => require(MOCKS).importUtils());
-jest.mock('@librechat/data-schemas', () => ({
+jest.mock('~/cache/getLogStores', () => require(MOCKS).logStores());
-  logger: {
+jest.mock('~/server/routes/files/multer', () => require(MOCKS).multerSetup());
-    debug: jest.fn(),
+jest.mock('multer', () => require(MOCKS).multerLib());
-    info: jest.fn(),
+jest.mock('~/server/services/Endpoints/azureAssistants', () => require(MOCKS).assistantEndpoint());
-    warn: jest.fn(),
+jest.mock('~/server/services/Endpoints/assistants', () => require(MOCKS).assistantEndpoint());
-    error: jest.fn(),
-  },
-  createModels: jest.fn(() => ({
-    User: {},
-    Conversation: {},
-    Message: {},
-    SharedLink: {},
-  })),
-}));
-
-jest.mock('~/models/Conversation', () => ({
-  getConvosByCursor: jest.fn(),
-  getConvo: jest.fn(),
-  deleteConvos: jest.fn(),
-  saveConvo: jest.fn(),
-}));
-
-jest.mock('~/models/ToolCall', () => ({
-  deleteToolCalls: jest.fn(),
-}));
-
-jest.mock('~/models', () => ({
-  deleteAllSharedLinks: jest.fn(),
-  deleteConvoSharedLink: jest.fn(),
-}));
-
-jest.mock('~/server/middleware/requireJwtAuth', () => (req, res, next) => next());
-
-jest.mock('~/server/middleware', () => ({
-  createImportLimiters: jest.fn(() => ({
-    importIpLimiter: (req, res, next) => next(),
-    importUserLimiter: (req, res, next) => next(),
-  })),
-  createForkLimiters: jest.fn(() => ({
-    forkIpLimiter: (req, res, next) => next(),
-    forkUserLimiter: (req, res, next) => next(),
-  })),
-  configMiddleware: (req, res, next) => next(),
-  validateConvoAccess: (req, res, next) => next(),
-}));
-
-jest.mock('~/server/utils/import/fork', () => ({
-  forkConversation: jest.fn(),
-  duplicateConversation: jest.fn(),
-}));
-
-jest.mock('~/server/utils/import', () => ({
-  importConversations: jest.fn(),
-}));
-
-jest.mock('~/cache/getLogStores', () => jest.fn());
-
-jest.mock('~/server/routes/files/multer', () => ({
-  storage: {},
-  importFileFilter: jest.fn(),
-}));
-
-jest.mock('multer', () => {
-  return jest.fn(() => ({
-    single: jest.fn(() => (req, res, next) => {
-      req.file = { path: '/tmp/test-file.json' };
-      next();
-    }),
-  }));
-});
-
-jest.mock('librechat-data-provider', () => ({
-  CacheKeys: {
-    GEN_TITLE: 'GEN_TITLE',
-  },
-  EModelEndpoint: {
-    azureAssistants: 'azureAssistants',
-    assistants: 'assistants',
-  },
-}));
-
-jest.mock('~/server/services/Endpoints/azureAssistants', () => ({
-  initializeClient: jest.fn(),
-}));
-
-jest.mock('~/server/services/Endpoints/assistants', () => ({
-  initializeClient: jest.fn(),
-}));
 
 describe('Convos Routes', () => {
   let app;

api/server/routes/convos.js

@@ -224,6 +224,7 @@ router.post('/update', validateConvoAccess, async (req, res) => {
 });
 
 const { importIpLimiter, importUserLimiter } = createImportLimiters();
+/** Fork and duplicate share one rate-limit budget (same "clone" operation class) */
 const { forkIpLimiter, forkUserLimiter } = createForkLimiters();
 const upload = multer({ storage: storage, fileFilter: importFileFilter });
 
@@ -280,7 +281,7 @@ router.post('/fork', forkIpLimiter, forkUserLimiter, async (req, res) => {
   }
 });
 
-router.post('/duplicate', async (req, res) => {
+router.post('/duplicate', forkIpLimiter, forkUserLimiter, async (req, res) => {
   const { conversationId, title } = req.body;
 
   try {

api/server/utils/import/fork.js

@@ -358,16 +358,15 @@ function splitAtTargetLevel(messages, targetMessageId) {
  * @param {object} params - The parameters for duplicating the conversation.
  * @param {string} params.userId - The ID of the user duplicating the conversation.
  * @param {string} params.conversationId - The ID of the conversation to duplicate.
+ * @param {string} [params.title] - Optional title override for the duplicate.
  * @returns {Promise<{ conversation: TConversation, messages: TMessage[] }>} The duplicated conversation and messages.
  */
-async function duplicateConversation({ userId, conversationId }) {
+async function duplicateConversation({ userId, conversationId, title }) {
-  // Get original conversation
   const originalConvo = await getConvo(userId, conversationId);
   if (!originalConvo) {
     throw new Error('Conversation not found');
   }
 
-  // Get original messages
   const originalMessages = await getMessages({
     user: userId,
     conversationId,
@@ -383,14 +382,11 @@ async function duplicateConversation({ userId, conversationId }) {
 
   cloneMessagesWithTimestamps(messagesToClone, importBatchBuilder);
 
-  const result = importBatchBuilder.finishConversation(
+  const duplicateTitle = title || originalConvo.title;
-    originalConvo.title,
+  const result = importBatchBuilder.finishConversation(duplicateTitle, new Date(), originalConvo);
-    new Date(),
-    originalConvo,
-  );
   await importBatchBuilder.saveBatch();
   logger.debug(
-    `user: ${userId} | New conversation "${originalConvo.title}" duplicated from conversation ID ${conversationId}`,
+    `user: ${userId} | New conversation "${duplicateTitle}" duplicated from conversation ID ${conversationId}`,
   );
 
   const conversation = await getConvo(userId, result.conversation.conversationId);