🪪 fix: Scope Message Conversation Access (#13183)

* fix: Scope message conversation access

* style: Format message route query

api/app/clients/BaseClient.js

@@ -707,7 +707,7 @@ class BaseClient {
   async loadHistory(conversationId, parentMessageId = null) {
     logger.debug('[BaseClient] Loading history:', { conversationId, parentMessageId });
 
-    const messages = (await db.getMessages({ conversationId })) ?? [];
+    const messages = (await db.getMessages({ conversationId, user: this.user })) ?? [];
 
     if (messages.length === 0) {
       return [];

api/app/clients/specs/BaseClient.test.js

@@ -1,5 +1,5 @@
 const { Constants } = require('librechat-data-provider');
-const { initializeFakeClient } = require('./FakeClient');
+const { FakeClient, initializeFakeClient } = require('./FakeClient');
 
 jest.mock('~/db/connect');
 jest.mock('~/server/services/Config', () => ({
@@ -38,7 +38,7 @@ jest.mock('~/models', () => ({
   updateFileUsage: jest.fn(),
 }));
 
-const { getConvo, saveConvo, saveMessage } = require('~/models');
+const { getConvo, getMessages, saveConvo, saveMessage } = require('~/models');
 
 jest.mock('@librechat/agents', () => {
   const actual = jest.requireActual('@librechat/agents');
@@ -622,6 +622,27 @@ describe('BaseClient', () => {
       expect(chatMessages2[chatMessages2.length - 1].text).toEqual("What's up");
     });
 
+    test('loadHistory should scope database reads to the current user', async () => {
+      const user = 'user-123';
+      TestClient = new FakeClient(apiKey, options);
+      TestClient.user = user;
+      getMessages.mockResolvedValueOnce([
+        {
+          role: 'user',
+          isCreatedByUser: true,
+          text: 'Hello',
+          messageId: '1',
+          conversationId,
+        },
+      ]);
+
+      const chatMessages = await TestClient.loadHistory(conversationId, '1');
+
+      expect(getMessages).toHaveBeenCalledWith({ conversationId, user });
+      expect(chatMessages).toHaveLength(1);
+      expect(chatMessages[0].text).toBe('Hello');
+    });
+
     /* Most of the new sendMessage logic revolving around edited/continued AI messages
      *  can be summarized by the following test. The condition will load the entire history up to
      *  the message that is being edited, which will trigger the AI API to 'continue' the response.

api/server/middleware/__tests__/validateMessageReq.spec.js

@@ -0,0 +1,74 @@
+jest.mock('~/models', () => ({
+  getConvo: jest.fn(),
+}));
+
+const validateMessageReq = require('../validateMessageReq');
+const { getConvo } = require('~/models');
+
+function createResponse() {
+  const res = {
+    json: jest.fn(),
+    send: jest.fn(),
+    status: jest.fn(),
+  };
+  res.status.mockReturnValue(res);
+  return res;
+}
+
+describe('validateMessageReq', () => {
+  const userId = 'user-123';
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('should reject requests when URL and body conversationId values differ', async () => {
+    const req = {
+      params: { conversationId: 'convo-owned' },
+      body: { conversationId: 'convo-victim' },
+      user: { id: userId },
+    };
+    const res = createResponse();
+    const next = jest.fn();
+
+    await validateMessageReq(req, res, next);
+
+    expect(res.status).toHaveBeenCalledWith(400);
+    expect(res.json).toHaveBeenCalledWith({ error: 'Conversation ID mismatch' });
+    expect(getConvo).not.toHaveBeenCalled();
+    expect(next).not.toHaveBeenCalled();
+  });
+
+  it('should reject requests when URL and nested message conversationId values differ', async () => {
+    const req = {
+      params: { conversationId: 'convo-owned' },
+      body: { message: { conversationId: 'convo-victim' } },
+      user: { id: userId },
+    };
+    const res = createResponse();
+    const next = jest.fn();
+
+    await validateMessageReq(req, res, next);
+
+    expect(res.status).toHaveBeenCalledWith(400);
+    expect(res.json).toHaveBeenCalledWith({ error: 'Conversation ID mismatch' });
+    expect(getConvo).not.toHaveBeenCalled();
+    expect(next).not.toHaveBeenCalled();
+  });
+
+  it('should validate ownership against the URL conversationId when values match', async () => {
+    const req = {
+      params: { conversationId: 'convo-owned' },
+      body: { conversationId: 'convo-owned' },
+      user: { id: userId },
+    };
+    const res = createResponse();
+    const next = jest.fn();
+    getConvo.mockResolvedValue({ conversationId: 'convo-owned', user: userId });
+
+    await validateMessageReq(req, res, next);
+
+    expect(getConvo).toHaveBeenCalledWith(userId, 'convo-owned');
+    expect(next).toHaveBeenCalledTimes(1);
+  });
+});

api/server/middleware/validateMessageReq.js

@@ -2,16 +2,26 @@ const { getConvo } = require('~/models');
 
 // Middleware to validate conversationId and user relationship
 const validateMessageReq = async (req, res, next) => {
-  let conversationId = req.params.conversationId || req.body.conversationId;
+  const body = req.body ?? {};
+  const paramConversationId = req.params?.conversationId;
+  const bodyConversationId = body.conversationId;
+  const nestedConversationId = body.message?.conversationId;
+
+  if (
+    (paramConversationId &&
+      ((bodyConversationId && paramConversationId !== bodyConversationId) ||
+        (nestedConversationId && paramConversationId !== nestedConversationId))) ||
+    (bodyConversationId && nestedConversationId && bodyConversationId !== nestedConversationId)
+  ) {
+    return res.status(400).json({ error: 'Conversation ID mismatch' });
+  }
+
+  const conversationId = paramConversationId || bodyConversationId || nestedConversationId;
 
   if (conversationId === 'new') {
     return res.status(200).send([]);
   }
 
-  if (!conversationId && req.body.message) {
-    conversationId = req.body.message.conversationId;
-  }
-
   const conversation = await getConvo(req.user.id, conversationId);
 
   if (!conversation) {

api/server/routes/__tests__/messages-delete.spec.js

@@ -197,3 +197,88 @@ describe('DELETE /:conversationId/:messageId – route handler', () => {
     expect(response.body).toEqual({ error: 'Internal server error' });
   });
 });
+
+describe('message route conversation ownership filters', () => {
+  let app;
+  const { getMessages, saveConvo, saveMessage } = require('~/models');
+
+  const authenticatedUserId = 'user-owner-123';
+
+  beforeAll(() => {
+    const messagesRouter = require('../messages');
+
+    app = express();
+    app.use(express.json());
+    app.use((req, res, next) => {
+      req.user = { id: authenticatedUserId };
+      next();
+    });
+    app.use('/api/messages', messagesRouter);
+  });
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('should save POST messages with the validated URL conversationId', async () => {
+    const urlConversationId = '11111111-1111-4111-8111-111111111111';
+    const bodyConversationId = '22222222-2222-4222-8222-222222222222';
+    const savedMessage = {
+      messageId: 'message-1',
+      conversationId: urlConversationId,
+      text: 'hello',
+      user: authenticatedUserId,
+    };
+
+    saveMessage.mockResolvedValue(savedMessage);
+    saveConvo.mockResolvedValue({ conversationId: urlConversationId });
+
+    const response = await request(app).post(`/api/messages/${urlConversationId}`).send({
+      messageId: savedMessage.messageId,
+      conversationId: bodyConversationId,
+      text: savedMessage.text,
+    });
+
+    expect(response.status).toBe(201);
+    expect(saveMessage).toHaveBeenCalledWith(
+      expect.objectContaining({ userId: authenticatedUserId }),
+      expect.objectContaining({
+        messageId: savedMessage.messageId,
+        conversationId: urlConversationId,
+        text: savedMessage.text,
+        user: authenticatedUserId,
+      }),
+      { context: 'POST /api/messages/:conversationId' },
+    );
+    expect(saveMessage.mock.calls[0][1].conversationId).not.toBe(bodyConversationId);
+    expect(saveConvo).toHaveBeenCalledWith(
+      expect.objectContaining({ userId: authenticatedUserId }),
+      savedMessage,
+      { context: 'POST /api/messages/:conversationId' },
+    );
+  });
+
+  it('should filter conversation message reads by authenticated user', async () => {
+    getMessages.mockResolvedValue([{ messageId: 'message-1', conversationId: 'convo-1' }]);
+
+    const response = await request(app).get('/api/messages/convo-1');
+
+    expect(response.status).toBe(200);
+    expect(getMessages).toHaveBeenCalledWith(
+      { conversationId: 'convo-1', user: authenticatedUserId },
+      '-_id -__v -user',
+    );
+  });
+
+  it('should filter single message reads by authenticated user', async () => {
+    getMessages.mockResolvedValue([{ messageId: 'message-1', conversationId: 'convo-1' }]);
+
+    const response = await request(app).get('/api/messages/convo-1/message-1');
+
+    expect(response.status).toBe(200);
+    expect(getMessages).toHaveBeenCalledWith(
+      { conversationId: 'convo-1', messageId: 'message-1', user: authenticatedUserId },
+      '-_id -__v -user',
+    );
+  });
+});

api/server/routes/messages.js

@@ -269,7 +269,7 @@ router.post('/artifact/:messageId', async (req, res) => {
 router.get('/:conversationId', validateMessageReq, async (req, res) => {
   try {
     const { conversationId } = req.params;
-    const messages = await db.getMessages({ conversationId }, '-_id -__v -user');
+    const messages = await db.getMessages({ conversationId, user: req.user.id }, '-_id -__v -user');
     res.status(200).json(messages);
   } catch (error) {
     logger.error('Error fetching messages:', error);
@@ -279,7 +279,7 @@ router.get('/:conversationId', validateMessageReq, async (req, res) => {
 
 router.post('/:conversationId', validateMessageReq, async (req, res) => {
   try {
-    const message = req.body;
+    const message = { ...req.body, conversationId: req.params.conversationId };
     const reqCtx = {
       userId: req?.user?.id,
       isTemporary: req?.body?.isTemporary,
@@ -304,7 +304,10 @@ router.post('/:conversationId', validateMessageReq, async (req, res) => {
 router.get('/:conversationId/:messageId', validateMessageReq, async (req, res) => {
   try {
     const { conversationId, messageId } = req.params;
-    const message = await db.getMessages({ conversationId, messageId }, '-_id -__v -user');
+    const message = await db.getMessages(
+      { conversationId, messageId, user: req.user.id },
+      '-_id -__v -user',
+    );
     if (!message) {
       return res.status(404).json({ error: 'Message not found' });
     }
@@ -331,7 +334,7 @@ router.put('/:conversationId/:messageId', validateMessageReq, async (req, res) =
     }
 
     const message = (
-      await db.getMessages({ conversationId, messageId }, 'content tokenCount')
+      await db.getMessages({ conversationId, messageId, user: req.user.id }, 'content tokenCount')
     )?.[0];
     if (!message) {
       return res.status(404).json({ error: 'Message not found' });

api/server/services/Threads/manage.js

@@ -467,7 +467,7 @@ async function checkMessageGaps({
     apiMessages.push(currentMessage);
   }
 
-  const dbMessages = await getMessages({ conversationId });
+  const dbMessages = await getMessages({ conversationId, user: openai.req.user.id });
   const assistant_id = dbMessages?.[0]?.model;
 
   const syncedMessages = await syncMessages({

packages/data-schemas/src/methods/conversation.spec.ts

@@ -136,7 +136,7 @@ describe('Conversation Operations', () => {
 
       // Verify that getMessages was called with correct parameters
       expect(getMessages).toHaveBeenCalledWith(
-        { conversationId: mockConversationData.conversationId },
+        { conversationId: mockConversationData.conversationId, user: mockCtx.userId },
         '_id',
       );
     });

packages/data-schemas/src/methods/conversation.ts

@@ -178,7 +178,7 @@ export function createConversationMethods(
         logger.debug(`[saveConvo] ${metadata.context}`);
       }
 
-      const messages = await getMessages({ conversationId }, '_id');
+      const messages = await getMessages({ conversationId, user: userId }, '_id');
       const update: Record<string, unknown> = { ...convo, messages, user: userId };
 
       if (newConversationId) {