mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-07-11 00:33:40 +00:00
⚙️ perf: reduce first-load MongoDB round trips (#14101)
* perf(api): reduce first-load database round trips * docs: move agent guidance to claude docs * refactor(api): move message validation into api package * fix(api): narrow active generation job lookup * fix(api): preserve omitted source identity
This commit is contained in:
parent
a0aa1f2b9d
commit
44d1275f36
25 changed files with 895 additions and 242 deletions
|
|
@ -1 +1 @@
|
|||
CLAUDE.md
|
||||
See CLAUDE.md.
|
||||
|
|
|
|||
13
CLAUDE.md
13
CLAUDE.md
|
|
@ -59,6 +59,19 @@ The source code for `@librechat/agents` (major backend dependency, same team) is
|
|||
- Avoid unnecessary object creation; consider space-time tradeoffs.
|
||||
- Prevent memory leaks: careful with closures, dispose resources/event listeners, no circular references.
|
||||
|
||||
### Backend Database Performance
|
||||
|
||||
- On request startup and first page load paths, watch for serial database reads.
|
||||
Multiple round trips to MongoDB can add significant latency when the database
|
||||
is far from the app server.
|
||||
- Prefer passing already-loaded request/user/config data through helper
|
||||
functions instead of re-reading the same user, role, tenant, or principal data.
|
||||
- When two reads are independent, start them in parallel and gate the response
|
||||
on the authorization or validation result before returning data.
|
||||
- Keep authorization, permission, and tenant checks semantically identical when
|
||||
parallelizing reads. Speculative reads must remain scoped to the authenticated
|
||||
user or tenant and must not write to the response before validation succeeds.
|
||||
|
||||
### Type Safety
|
||||
|
||||
- **Never use `any`**. Explicit types for all parameters, return values, and variables.
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ const {
|
|||
needsRefresh,
|
||||
MCPOAuthHandler,
|
||||
MCPTokenStorage,
|
||||
getAppConfigOptionsFromUser,
|
||||
normalizeHttpError,
|
||||
extractWebSearchEnvVars,
|
||||
deleteAgentCheckpoints,
|
||||
|
|
@ -59,13 +60,7 @@ const sanitizeUserForResponse = (user) => {
|
|||
};
|
||||
|
||||
const getUserController = async (req, res) => {
|
||||
const appConfig =
|
||||
req.config ??
|
||||
(await getAppConfig({
|
||||
role: req.user?.role,
|
||||
userId: req.user?.id,
|
||||
tenantId: req.user?.tenantId,
|
||||
}));
|
||||
const appConfig = req.config ?? (await getAppConfig(getAppConfigOptionsFromUser(req.user)));
|
||||
/** @type {IUser} */
|
||||
const userData = sanitizeUserForResponse(req.user);
|
||||
if (appConfig.fileStrategy === FileSources.s3 && userData.avatar) {
|
||||
|
|
@ -203,13 +198,7 @@ const deleteUserMcpServers = async (userId) => {
|
|||
};
|
||||
|
||||
const updateUserPluginsController = async (req, res) => {
|
||||
const appConfig =
|
||||
req.config ??
|
||||
(await getAppConfig({
|
||||
role: req.user?.role,
|
||||
userId: req.user?.id,
|
||||
tenantId: req.user?.tenantId,
|
||||
}));
|
||||
const appConfig = req.config ?? (await getAppConfig(getAppConfigOptionsFromUser(req.user)));
|
||||
const { user } = req;
|
||||
const { pluginKey, action, auth, isEntityTool } = req.body;
|
||||
try {
|
||||
|
|
|
|||
|
|
@ -40,6 +40,16 @@ jest.mock('@librechat/api', () => ({
|
|||
},
|
||||
normalizeHttpError: jest.fn((error) => error),
|
||||
extractWebSearchEnvVars: jest.fn((params) => params.keys),
|
||||
getAppConfigOptionsFromUser: jest.fn((user) => {
|
||||
const hasSourceIdentity =
|
||||
user != null && Object.prototype.hasOwnProperty.call(user, 'idOnTheSource');
|
||||
return {
|
||||
role: user?.role,
|
||||
userId: user?.id,
|
||||
idOnTheSource: user?.id && hasSourceIdentity ? (user.idOnTheSource ?? null) : undefined,
|
||||
tenantId: user?.tenantId,
|
||||
};
|
||||
}),
|
||||
needsRefresh: jest.fn(),
|
||||
getNewS3URL: jest.fn(),
|
||||
}));
|
||||
|
|
|
|||
|
|
@ -3,9 +3,12 @@ jest.mock('~/models', () => ({
|
|||
}));
|
||||
|
||||
jest.mock('@librechat/api', () => ({
|
||||
createMessageRequestMiddleware:
|
||||
jest.requireActual('@librechat/api').createMessageRequestMiddleware,
|
||||
GenerationJobManager: {
|
||||
getJob: jest.fn(),
|
||||
},
|
||||
isPendingActionStale: jest.fn(() => false),
|
||||
}));
|
||||
|
||||
jest.mock('@librechat/data-schemas', () => ({
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
const { logger } = require('@librechat/data-schemas');
|
||||
const { isEmailDomainAllowed } = require('@librechat/api');
|
||||
const { getAppConfigOptionsFromUser, isEmailDomainAllowed } = require('@librechat/api');
|
||||
const { getAppConfig } = require('~/server/services/Config');
|
||||
|
||||
/**
|
||||
|
|
@ -16,11 +16,7 @@ const { getAppConfig } = require('~/server/services/Config');
|
|||
const checkDomainAllowed = async (req, res, next) => {
|
||||
try {
|
||||
const email = req?.user?.email;
|
||||
const appConfig = await getAppConfig({
|
||||
role: req?.user?.role,
|
||||
userId: req?.user?.id,
|
||||
tenantId: req?.user?.tenantId,
|
||||
});
|
||||
const appConfig = await getAppConfig(getAppConfigOptionsFromUser(req?.user));
|
||||
|
||||
if (email && !isEmailDomainAllowed(email, appConfig?.registration?.allowedDomains)) {
|
||||
logger.error(`[Social Login] [Social Login not allowed] [Email: ${email}]`);
|
||||
|
|
|
|||
|
|
@ -1,12 +1,10 @@
|
|||
const { logger } = require('@librechat/data-schemas');
|
||||
const { getAppConfigOptionsFromUser } = require('@librechat/api');
|
||||
const { getAppConfig } = require('~/server/services/Config');
|
||||
|
||||
const configMiddleware = async (req, res, next) => {
|
||||
try {
|
||||
const userRole = req.user?.role;
|
||||
const userId = req.user?.id;
|
||||
const tenantId = req.user?.tenantId;
|
||||
req.config = await getAppConfig({ role: userRole, userId, tenantId });
|
||||
req.config = await getAppConfig(getAppConfigOptionsFromUser(req.user));
|
||||
|
||||
next();
|
||||
} catch (error) {
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ const setTwoFactorTempUser = require('./setTwoFactorTempUser');
|
|||
const validateRegistration = require('./validateRegistration');
|
||||
const buildEndpointOption = require('./buildEndpointOption');
|
||||
const validateMessageReq = require('./validateMessageReq');
|
||||
const { prepareMessageRequestValidation, sendValidationResponse } = require('./messageValidation');
|
||||
const checkDomainAllowed = require('./checkDomainAllowed');
|
||||
const requireLocalAuth = require('./requireLocalAuth');
|
||||
const canDeleteAccount = require('./canDeleteAccount');
|
||||
|
|
@ -47,6 +48,8 @@ module.exports = {
|
|||
configMiddleware,
|
||||
checkDomainAllowed,
|
||||
validateMessageReq,
|
||||
sendValidationResponse,
|
||||
prepareMessageRequestValidation,
|
||||
buildEndpointOption,
|
||||
validateRegistration,
|
||||
validatePasswordReset,
|
||||
|
|
|
|||
14
api/server/middleware/messageValidation.js
Normal file
14
api/server/middleware/messageValidation.js
Normal file
|
|
@ -0,0 +1,14 @@
|
|||
const {
|
||||
GenerationJobManager,
|
||||
createMessageRequestMiddleware,
|
||||
isPendingActionStale,
|
||||
} = require('@librechat/api');
|
||||
const { logger } = require('@librechat/data-schemas');
|
||||
const { getConvo } = require('~/models');
|
||||
|
||||
module.exports = createMessageRequestMiddleware({
|
||||
getConvo,
|
||||
getJob: (conversationId) => GenerationJobManager.getJob(conversationId),
|
||||
isPendingActionStale,
|
||||
logger,
|
||||
});
|
||||
|
|
@ -1,78 +1,3 @@
|
|||
const { GenerationJobManager, isPendingActionStale } = require('@librechat/api');
|
||||
const { logger } = require('@librechat/data-schemas');
|
||||
const { getConvo } = require('~/models');
|
||||
|
||||
function hasTenantMismatch(job, user) {
|
||||
// Untenanted jobs remain readable by their owner for pre-multi-tenancy deployments.
|
||||
return job.metadata?.tenantId != null && job.metadata.tenantId !== user.tenantId;
|
||||
}
|
||||
|
||||
async function canReadActiveJobConversation(req, conversationId) {
|
||||
if (req.method !== 'GET' || req.params?.messageId) {
|
||||
return false;
|
||||
}
|
||||
|
||||
let job;
|
||||
try {
|
||||
job = await GenerationJobManager.getJob(conversationId);
|
||||
} catch (error) {
|
||||
logger.warn(`[validateMessageReq] Active job lookup failed for ${conversationId}:`, error);
|
||||
return false;
|
||||
}
|
||||
|
||||
// A job paused for human review is still active (consistent with /chat/status
|
||||
// and /chat/active), so a new-conversation run that pauses before its final
|
||||
// save can still recover the prompt — but only while it has a live,
|
||||
// resolvable prompt (missing/malformed or past-expiry reads as inactive).
|
||||
const isActive =
|
||||
!!job &&
|
||||
(job.status === 'running' ||
|
||||
(job.status === 'requires_action' &&
|
||||
!isPendingActionStale({ pendingAction: job.metadata?.pendingAction })));
|
||||
if (!isActive) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return job.metadata?.userId === req.user.id && !hasTenantMismatch(job, req.user);
|
||||
}
|
||||
|
||||
// Middleware to validate conversationId and user relationship
|
||||
const validateMessageReq = async (req, res, next) => {
|
||||
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([]);
|
||||
}
|
||||
|
||||
const conversation = await getConvo(req.user.id, conversationId);
|
||||
|
||||
if (!conversation) {
|
||||
if (await canReadActiveJobConversation(req, conversationId)) {
|
||||
return next();
|
||||
}
|
||||
|
||||
return res.status(404).json({ error: 'Conversation not found' });
|
||||
}
|
||||
|
||||
if (conversation.user !== req.user.id) {
|
||||
return res.status(403).json({ error: 'User not authorized for this conversation' });
|
||||
}
|
||||
|
||||
next();
|
||||
};
|
||||
const { validateMessageReq } = require('./messageValidation');
|
||||
|
||||
module.exports = validateMessageReq;
|
||||
|
|
|
|||
|
|
@ -290,6 +290,7 @@ describe('GET /api/config', () => {
|
|||
expect(mockGetAppConfig).toHaveBeenCalledWith({
|
||||
role: 'USER',
|
||||
userId: 'user123',
|
||||
idOnTheSource: undefined,
|
||||
tenantId: 'fallback-tenant',
|
||||
});
|
||||
});
|
||||
|
|
@ -304,6 +305,7 @@ describe('GET /api/config', () => {
|
|||
expect(mockGetAppConfig).toHaveBeenCalledWith({
|
||||
role: 'USER',
|
||||
userId: 'user123',
|
||||
idOnTheSource: undefined,
|
||||
tenantId: 'user-tenant',
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -48,11 +48,31 @@ jest.mock('~/server/services/Artifacts/update', () => ({
|
|||
|
||||
jest.mock('~/server/middleware/requireJwtAuth', () => (req, res, next) => next());
|
||||
|
||||
jest.mock('~/server/middleware', () => ({
|
||||
requireJwtAuth: (req, res, next) => next(),
|
||||
validateMessageReq: (req, res, next) => next(),
|
||||
configMiddleware: (req, res, next) => next(),
|
||||
}));
|
||||
jest.mock('~/server/middleware', () => {
|
||||
const validateMessageReq = jest.fn((req, res, next) => next());
|
||||
const prepareMessageRequestValidation = jest.fn((req, res, next) => {
|
||||
req.messageRequestValidation = {
|
||||
conversationId: 'convo-1',
|
||||
shouldFetchMessages: true,
|
||||
promise: Promise.resolve({ ok: true }),
|
||||
};
|
||||
next();
|
||||
});
|
||||
const sendValidationResponse = jest.fn((res, result) => {
|
||||
if (result.send) {
|
||||
return res.status(result.status).send(result.body);
|
||||
}
|
||||
return res.status(result.status).json(result.body);
|
||||
});
|
||||
|
||||
return {
|
||||
requireJwtAuth: (req, res, next) => next(),
|
||||
validateMessageReq,
|
||||
sendValidationResponse,
|
||||
prepareMessageRequestValidation,
|
||||
configMiddleware: (req, res, next) => next(),
|
||||
};
|
||||
});
|
||||
|
||||
jest.mock('~/db/models', () => ({
|
||||
Message: {
|
||||
|
|
@ -200,88 +220,3 @@ 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',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
167
api/server/routes/__tests__/messages-get-real-validation.spec.js
Normal file
167
api/server/routes/__tests__/messages-get-real-validation.spec.js
Normal file
|
|
@ -0,0 +1,167 @@
|
|||
const express = require('express');
|
||||
const request = require('supertest');
|
||||
|
||||
jest.mock('@librechat/agents', () => ({
|
||||
...jest.requireActual('@librechat/agents'),
|
||||
CODE_EXECUTION_TOOLS: new Set(['execute_code', 'bash_tool']),
|
||||
BashExecutionToolDefinition: {
|
||||
name: 'bash_tool',
|
||||
description: 'bash',
|
||||
schema: { type: 'object', properties: {} },
|
||||
},
|
||||
ReadFileToolDefinition: {
|
||||
name: 'read_file',
|
||||
description: 'Read a file',
|
||||
parameters: { type: 'object', properties: {} },
|
||||
responseFormat: 'content',
|
||||
},
|
||||
buildBashExecutionToolDescription: () => 'bash',
|
||||
sleep: jest.fn(),
|
||||
}));
|
||||
|
||||
jest.mock('@librechat/api', () => ({
|
||||
createMessageRequestMiddleware:
|
||||
jest.requireActual('@librechat/api').createMessageRequestMiddleware,
|
||||
unescapeLaTeX: jest.fn((x) => x),
|
||||
countTokens: jest.fn().mockResolvedValue(10),
|
||||
sendFeedbackScore: jest.fn().mockResolvedValue(undefined),
|
||||
traceIdForMessage: jest.fn((messageId) => `trace-${messageId}`),
|
||||
mergeQuotedTextForCount: jest.fn((text) => text),
|
||||
GenerationJobManager: {
|
||||
getJob: jest.fn(),
|
||||
},
|
||||
isPendingActionStale: jest.fn(() => false),
|
||||
}));
|
||||
|
||||
jest.mock('@librechat/data-schemas', () => ({
|
||||
...jest.requireActual('@librechat/data-schemas'),
|
||||
logger: {
|
||||
debug: jest.fn(),
|
||||
info: jest.fn(),
|
||||
warn: jest.fn(),
|
||||
error: jest.fn(),
|
||||
},
|
||||
}));
|
||||
|
||||
jest.mock('librechat-data-provider', () => ({
|
||||
...jest.requireActual('librechat-data-provider'),
|
||||
}));
|
||||
|
||||
jest.mock('~/models', () => ({
|
||||
saveConvo: jest.fn(),
|
||||
getConvo: jest.fn(),
|
||||
getMessage: jest.fn(),
|
||||
saveMessage: jest.fn(),
|
||||
getMessages: jest.fn(),
|
||||
updateMessage: jest.fn(),
|
||||
deleteMessages: jest.fn(),
|
||||
getConvosQueried: jest.fn(),
|
||||
searchMessages: jest.fn(),
|
||||
getMessagesByCursor: jest.fn(),
|
||||
}));
|
||||
|
||||
jest.mock('~/server/services/Artifacts/update', () => ({
|
||||
findAllArtifacts: jest.fn(),
|
||||
replaceArtifactContent: jest.fn(),
|
||||
}));
|
||||
|
||||
jest.mock('~/server/middleware/requireJwtAuth', () => (req, res, next) => next());
|
||||
|
||||
jest.mock('~/server/middleware', () => {
|
||||
const { sendValidationResponse, validateMessageReq, prepareMessageRequestValidation } =
|
||||
jest.requireActual('~/server/middleware/messageValidation');
|
||||
|
||||
return {
|
||||
requireJwtAuth: (req, res, next) => next(),
|
||||
validateMessageReq,
|
||||
sendValidationResponse,
|
||||
prepareMessageRequestValidation,
|
||||
configMiddleware: (req, res, next) => next(),
|
||||
};
|
||||
});
|
||||
|
||||
jest.mock('~/db/models', () => ({
|
||||
Message: {
|
||||
findOne: jest.fn(),
|
||||
find: jest.fn(),
|
||||
meiliSearch: jest.fn(),
|
||||
},
|
||||
}));
|
||||
|
||||
describe('GET /api/messages/:conversationId with real validation middleware', () => {
|
||||
let app;
|
||||
const { getConvo, getMessages } = 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('returns the existing empty response for new conversations without fetching messages', async () => {
|
||||
const response = await request(app).get('/api/messages/new');
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.body).toEqual([]);
|
||||
expect(getConvo).not.toHaveBeenCalled();
|
||||
expect(getMessages).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('starts user-scoped message reads before real conversation validation resolves', async () => {
|
||||
const events = [];
|
||||
let resolveConvo;
|
||||
const convoPromise = new Promise((resolve) => {
|
||||
resolveConvo = resolve;
|
||||
});
|
||||
|
||||
getConvo.mockImplementation(() => {
|
||||
events.push('convo-started');
|
||||
return convoPromise;
|
||||
});
|
||||
|
||||
let resolveMessagesStarted;
|
||||
const messagesStartedPromise = new Promise((resolve) => {
|
||||
resolveMessagesStarted = resolve;
|
||||
});
|
||||
getMessages.mockImplementation(() => {
|
||||
events.push('messages-started');
|
||||
resolveMessagesStarted();
|
||||
return Promise.resolve([{ messageId: 'message-1', conversationId: 'convo-1' }]);
|
||||
});
|
||||
|
||||
const responsePromise = new Promise((resolve, reject) => {
|
||||
request(app)
|
||||
.get('/api/messages/convo-1')
|
||||
.end((error, response) => (error ? reject(error) : resolve(response)));
|
||||
});
|
||||
|
||||
await Promise.race([
|
||||
messagesStartedPromise,
|
||||
new Promise((resolve) => setTimeout(resolve, 100)),
|
||||
]);
|
||||
const eventsBeforeValidation = [...events];
|
||||
|
||||
resolveConvo({ conversationId: 'convo-1', user: authenticatedUserId });
|
||||
const response = await responsePromise;
|
||||
|
||||
expect(eventsBeforeValidation).toEqual(['convo-started', 'messages-started']);
|
||||
expect(getConvo).toHaveBeenCalledWith(authenticatedUserId, 'convo-1');
|
||||
expect(getMessages).toHaveBeenCalledWith(
|
||||
{ conversationId: 'convo-1', user: authenticatedUserId },
|
||||
'-_id -__v -user',
|
||||
);
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.body).toEqual([{ messageId: 'message-1', conversationId: 'convo-1' }]);
|
||||
});
|
||||
});
|
||||
247
api/server/routes/__tests__/messages-get.spec.js
Normal file
247
api/server/routes/__tests__/messages-get.spec.js
Normal file
|
|
@ -0,0 +1,247 @@
|
|||
const express = require('express');
|
||||
const request = require('supertest');
|
||||
|
||||
jest.mock('@librechat/agents', () => ({
|
||||
sleep: jest.fn(),
|
||||
}));
|
||||
|
||||
jest.mock('@librechat/api', () => ({
|
||||
unescapeLaTeX: jest.fn((x) => x),
|
||||
countTokens: jest.fn().mockResolvedValue(10),
|
||||
sendFeedbackScore: jest.fn().mockResolvedValue(undefined),
|
||||
traceIdForMessage: jest.fn((messageId) => `trace-${messageId}`),
|
||||
}));
|
||||
|
||||
jest.mock('@librechat/data-schemas', () => ({
|
||||
...jest.requireActual('@librechat/data-schemas'),
|
||||
logger: {
|
||||
debug: jest.fn(),
|
||||
info: jest.fn(),
|
||||
warn: jest.fn(),
|
||||
error: jest.fn(),
|
||||
},
|
||||
}));
|
||||
|
||||
jest.mock('librechat-data-provider', () => ({
|
||||
...jest.requireActual('librechat-data-provider'),
|
||||
}));
|
||||
|
||||
jest.mock('~/models', () => ({
|
||||
saveConvo: jest.fn(),
|
||||
getMessage: jest.fn(),
|
||||
saveMessage: jest.fn(),
|
||||
getMessages: jest.fn(),
|
||||
updateMessage: jest.fn(),
|
||||
deleteMessages: jest.fn(),
|
||||
getConvosQueried: jest.fn(),
|
||||
searchMessages: jest.fn(),
|
||||
getMessagesByCursor: jest.fn(),
|
||||
}));
|
||||
|
||||
jest.mock('~/server/services/Artifacts/update', () => ({
|
||||
findAllArtifacts: jest.fn(),
|
||||
replaceArtifactContent: jest.fn(),
|
||||
}));
|
||||
|
||||
jest.mock('~/server/middleware/requireJwtAuth', () => (req, res, next) => next());
|
||||
|
||||
jest.mock('~/server/middleware', () => {
|
||||
const validateMessageReq = jest.fn((req, res, next) => next());
|
||||
const prepareMessageRequestValidation = jest.fn((req, res, next) => {
|
||||
req.messageRequestValidation = {
|
||||
conversationId: 'convo-1',
|
||||
shouldFetchMessages: true,
|
||||
promise: Promise.resolve({ ok: true }),
|
||||
};
|
||||
next();
|
||||
});
|
||||
const sendValidationResponse = jest.fn((res, result) => {
|
||||
if (result.send) {
|
||||
return res.status(result.status).send(result.body);
|
||||
}
|
||||
return res.status(result.status).json(result.body);
|
||||
});
|
||||
|
||||
return {
|
||||
requireJwtAuth: (req, res, next) => next(),
|
||||
validateMessageReq,
|
||||
sendValidationResponse,
|
||||
prepareMessageRequestValidation,
|
||||
configMiddleware: (req, res, next) => next(),
|
||||
};
|
||||
});
|
||||
|
||||
jest.mock('~/db/models', () => ({
|
||||
Message: {
|
||||
findOne: jest.fn(),
|
||||
find: jest.fn(),
|
||||
meiliSearch: jest.fn(),
|
||||
},
|
||||
}));
|
||||
|
||||
describe('message route conversation ownership filters', () => {
|
||||
let app;
|
||||
const { getMessages, saveConvo, saveMessage } = require('~/models');
|
||||
const { prepareMessageRequestValidation } = require('~/server/middleware');
|
||||
|
||||
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();
|
||||
prepareMessageRequestValidation.mockImplementation((req, res, next) => {
|
||||
req.messageRequestValidation = {
|
||||
conversationId: 'convo-1',
|
||||
shouldFetchMessages: true,
|
||||
promise: Promise.resolve({ ok: true }),
|
||||
};
|
||||
next();
|
||||
});
|
||||
});
|
||||
|
||||
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 start conversation message reads before validation resolves', async () => {
|
||||
const events = [];
|
||||
let resolveValidation;
|
||||
const validationPromise = new Promise((resolve) => {
|
||||
resolveValidation = resolve;
|
||||
});
|
||||
prepareMessageRequestValidation.mockImplementationOnce((req, res, next) => {
|
||||
req.messageRequestValidation = {
|
||||
conversationId: 'convo-1',
|
||||
shouldFetchMessages: true,
|
||||
promise: validationPromise,
|
||||
};
|
||||
next();
|
||||
});
|
||||
let resolveMessagesStarted;
|
||||
const messagesStartedPromise = new Promise((resolve) => {
|
||||
resolveMessagesStarted = resolve;
|
||||
});
|
||||
getMessages.mockImplementation(() => {
|
||||
events.push('messages-started');
|
||||
resolveMessagesStarted();
|
||||
return Promise.resolve([{ messageId: 'message-1', conversationId: 'convo-1' }]);
|
||||
});
|
||||
|
||||
const responsePromise = new Promise((resolve, reject) => {
|
||||
request(app)
|
||||
.get('/api/messages/convo-1')
|
||||
.end((error, response) => (error ? reject(error) : resolve(response)));
|
||||
});
|
||||
await Promise.race([
|
||||
messagesStartedPromise,
|
||||
new Promise((resolve) => setTimeout(resolve, 100)),
|
||||
]);
|
||||
|
||||
const eventsBeforeValidation = [...events];
|
||||
resolveValidation({ ok: true });
|
||||
const response = await responsePromise;
|
||||
|
||||
expect(eventsBeforeValidation).toEqual(['messages-started']);
|
||||
expect(getMessages).toHaveBeenCalledWith(
|
||||
{ conversationId: 'convo-1', user: authenticatedUserId },
|
||||
'-_id -__v -user',
|
||||
);
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.body).toEqual([{ messageId: 'message-1', conversationId: 'convo-1' }]);
|
||||
});
|
||||
|
||||
it('should not return fetched messages when conversation validation fails', async () => {
|
||||
prepareMessageRequestValidation.mockImplementationOnce((req, res, next) => {
|
||||
req.messageRequestValidation = {
|
||||
conversationId: 'convo-1',
|
||||
shouldFetchMessages: true,
|
||||
promise: Promise.resolve({
|
||||
ok: false,
|
||||
status: 404,
|
||||
body: { error: 'Conversation not found' },
|
||||
}),
|
||||
};
|
||||
next();
|
||||
});
|
||||
getMessages.mockResolvedValue([{ messageId: 'secret-message', conversationId: 'convo-1' }]);
|
||||
|
||||
const response = await request(app).get('/api/messages/convo-1');
|
||||
|
||||
expect(getMessages).toHaveBeenCalledWith(
|
||||
{ conversationId: 'convo-1', user: authenticatedUserId },
|
||||
'-_id -__v -user',
|
||||
);
|
||||
expect(response.status).toBe(404);
|
||||
expect(response.body).toEqual({ error: 'Conversation not found' });
|
||||
});
|
||||
|
||||
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',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
@ -3,6 +3,7 @@ const {
|
|||
isEnabled,
|
||||
getBalanceConfig,
|
||||
getCloudFrontConfig,
|
||||
getAppConfigOptionsFromUser,
|
||||
resolveBuildInfo,
|
||||
resolveTitleTiming,
|
||||
sanitizeModelSpecs,
|
||||
|
|
@ -244,11 +245,7 @@ router.get('/', async function (req, res) {
|
|||
return res.status(200).send(payload);
|
||||
}
|
||||
|
||||
const appConfig = await getAppConfig({
|
||||
role: req.user.role,
|
||||
userId: req.user.id,
|
||||
tenantId: req.user.tenantId || getTenantId(),
|
||||
});
|
||||
const appConfig = await getAppConfig(getAppConfigOptionsFromUser(req.user));
|
||||
|
||||
const balanceConfig = getBalanceConfig(appConfig);
|
||||
const cloudFront = buildCloudFrontStartupConfig();
|
||||
|
|
|
|||
|
|
@ -10,7 +10,13 @@ const {
|
|||
mergeQuotedTextForCount,
|
||||
} = require('@librechat/api');
|
||||
const { findAllArtifacts, replaceArtifactContent } = require('~/server/services/Artifacts/update');
|
||||
const { requireJwtAuth, validateMessageReq, configMiddleware } = require('~/server/middleware');
|
||||
const {
|
||||
requireJwtAuth,
|
||||
validateMessageReq,
|
||||
configMiddleware,
|
||||
sendValidationResponse,
|
||||
prepareMessageRequestValidation,
|
||||
} = require('~/server/middleware');
|
||||
const db = require('~/models');
|
||||
|
||||
const router = express.Router();
|
||||
|
|
@ -271,11 +277,30 @@ router.post('/artifact/:messageId', async (req, res) => {
|
|||
}
|
||||
});
|
||||
|
||||
/* Note: It's necessary to add `validateMessageReq` within route definition for correct params */
|
||||
router.get('/:conversationId', validateMessageReq, async (req, res) => {
|
||||
router.get('/:conversationId', prepareMessageRequestValidation, async (req, res) => {
|
||||
try {
|
||||
const { conversationId } = req.params;
|
||||
const messages = await db.getMessages({ conversationId, user: req.user.id }, '-_id -__v -user');
|
||||
const validation = req.messageRequestValidation;
|
||||
// This intentionally starts a user-scoped read before validation resolves;
|
||||
// the response remains gated on validation success below.
|
||||
const messagesPromise = validation.shouldFetchMessages
|
||||
? db.getMessages({ conversationId, user: req.user.id }, '-_id -__v -user').then(
|
||||
(messages) => ({ messages }),
|
||||
(error) => ({ error }),
|
||||
)
|
||||
: null;
|
||||
|
||||
const validationResult = await validation.promise;
|
||||
if (!validationResult.ok) {
|
||||
return sendValidationResponse(res, validationResult);
|
||||
}
|
||||
|
||||
const messagesResult = await messagesPromise;
|
||||
if (messagesResult?.error) {
|
||||
throw messagesResult.error;
|
||||
}
|
||||
|
||||
const messages = messagesResult?.messages ?? [];
|
||||
res.status(200).json(messages);
|
||||
} catch (error) {
|
||||
logger.error('Error fetching messages:', error);
|
||||
|
|
|
|||
|
|
@ -104,6 +104,7 @@ describe('loadConfigModels', () => {
|
|||
role: 'USER',
|
||||
userId: 'testUserId',
|
||||
tenantId: 'tenant-a',
|
||||
idOnTheSource: undefined,
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ const {
|
|||
mergeHeaders,
|
||||
getAnthropicModels,
|
||||
getBedrockModels,
|
||||
getAppConfigOptionsFromUser,
|
||||
getOpenAIModels,
|
||||
getGoogleModels,
|
||||
} = require('@librechat/api');
|
||||
|
|
@ -17,13 +18,7 @@ const { getAppConfig } = require('./app');
|
|||
*/
|
||||
async function loadDefaultModels(req) {
|
||||
try {
|
||||
const appConfig =
|
||||
req.config ??
|
||||
(await getAppConfig({
|
||||
role: req.user?.role,
|
||||
userId: req.user?.id,
|
||||
tenantId: req.user?.tenantId,
|
||||
}));
|
||||
const appConfig = req.config ?? (await getAppConfig(getAppConfigOptionsFromUser(req.user)));
|
||||
const vertexConfig = appConfig?.endpoints?.[EModelEndpoint.anthropic]?.vertexConfig;
|
||||
|
||||
/** Forward configured custom headers (endpoint over global `all`) so model
|
||||
|
|
|
|||
|
|
@ -1,5 +1,9 @@
|
|||
import type { AppConfig } from '@librechat/data-schemas';
|
||||
import { createAppConfigService, _resetOverrideStrictCache } from './service';
|
||||
import {
|
||||
createAppConfigService,
|
||||
_resetOverrideStrictCache,
|
||||
getAppConfigOptionsFromUser,
|
||||
} from './service';
|
||||
|
||||
/** Extends AppConfig with mock fields used by merge behavior tests. */
|
||||
interface TestConfig extends AppConfig {
|
||||
|
|
@ -415,6 +419,33 @@ describe('createAppConfigService', () => {
|
|||
});
|
||||
});
|
||||
|
||||
it('passes local identity through to getUserPrincipals when provided', async () => {
|
||||
const deps = createDeps();
|
||||
const { getAppConfig } = createAppConfigService(deps);
|
||||
|
||||
await getAppConfig({ role: 'USER', userId: 'uid1', idOnTheSource: null });
|
||||
|
||||
expect(deps.getUserPrincipals).toHaveBeenCalledWith({
|
||||
userId: 'uid1',
|
||||
role: 'USER',
|
||||
idOnTheSource: null,
|
||||
});
|
||||
});
|
||||
|
||||
it('uses the same override cache entry when source identity changes for a user', async () => {
|
||||
const deps = createDeps();
|
||||
const { getAppConfig } = createAppConfigService(deps);
|
||||
|
||||
await getAppConfig({ role: 'USER', userId: 'uid1', idOnTheSource: null });
|
||||
await getAppConfig({ role: 'USER', userId: 'uid1', idOnTheSource: 'source-user-1' });
|
||||
|
||||
expect(deps.getUserPrincipals).toHaveBeenCalledTimes(1);
|
||||
expect(deps.getApplicableConfigs).toHaveBeenCalledTimes(1);
|
||||
expect([...deps._cache._store.keys()]).toEqual(
|
||||
expect.arrayContaining(['app_config:_OVERRIDE_:__default__:USER:uid1']),
|
||||
);
|
||||
});
|
||||
|
||||
it('does not call getUserPrincipals when only role is provided', async () => {
|
||||
const deps = createDeps();
|
||||
const { getAppConfig } = createAppConfigService(deps);
|
||||
|
|
@ -505,3 +536,48 @@ describe('createAppConfigService', () => {
|
|||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('getAppConfigOptionsFromUser', () => {
|
||||
it('maps resolved request users to app config principal options', () => {
|
||||
expect(
|
||||
getAppConfigOptionsFromUser({
|
||||
id: 'uid1',
|
||||
role: 'USER',
|
||||
tenantId: 'tenant-a',
|
||||
idOnTheSource: 'source-user-1',
|
||||
}),
|
||||
).toEqual({
|
||||
role: 'USER',
|
||||
userId: 'uid1',
|
||||
idOnTheSource: 'source-user-1',
|
||||
tenantId: 'tenant-a',
|
||||
});
|
||||
});
|
||||
|
||||
it('preserves omitted source identity for partial users so fallback lookup can run', () => {
|
||||
expect(getAppConfigOptionsFromUser({ id: 'uid1', role: 'USER' })).toEqual({
|
||||
role: 'USER',
|
||||
userId: 'uid1',
|
||||
idOnTheSource: undefined,
|
||||
tenantId: undefined,
|
||||
});
|
||||
});
|
||||
|
||||
it('marks explicitly normalized local users with null idOnTheSource', () => {
|
||||
expect(getAppConfigOptionsFromUser({ id: 'uid1', role: 'USER', idOnTheSource: null })).toEqual({
|
||||
role: 'USER',
|
||||
userId: 'uid1',
|
||||
idOnTheSource: null,
|
||||
tenantId: undefined,
|
||||
});
|
||||
});
|
||||
|
||||
it('omits source identity when no user id is available', () => {
|
||||
expect(getAppConfigOptionsFromUser({ role: 'USER', tenantId: 'tenant-a' })).toEqual({
|
||||
role: 'USER',
|
||||
userId: undefined,
|
||||
idOnTheSource: undefined,
|
||||
tenantId: 'tenant-a',
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -43,6 +43,7 @@ export interface AppConfigServiceDeps {
|
|||
getUserPrincipals: (params: {
|
||||
userId: string | Types.ObjectId;
|
||||
role?: string | null;
|
||||
idOnTheSource?: string | null;
|
||||
}) => Promise<Array<{ principalType: string; principalId?: string | Types.ObjectId }>>;
|
||||
/** TTL in ms for per-user/role merged config caches. Defaults to 60 000. */
|
||||
overrideCacheTtl?: number;
|
||||
|
|
@ -51,12 +52,36 @@ export interface AppConfigServiceDeps {
|
|||
export interface GetAppConfigOptions {
|
||||
role?: string;
|
||||
userId?: string;
|
||||
idOnTheSource?: string | null;
|
||||
tenantId?: string;
|
||||
refresh?: boolean;
|
||||
/** When true, return only the YAML-derived base config — no DB override queries. */
|
||||
baseOnly?: boolean;
|
||||
}
|
||||
|
||||
export interface AppConfigUserLike {
|
||||
/** Resolved app user id. */
|
||||
id?: string;
|
||||
role?: string;
|
||||
tenantId?: string;
|
||||
idOnTheSource?: string | null;
|
||||
}
|
||||
|
||||
export function getAppConfigOptionsFromUser(
|
||||
user?: AppConfigUserLike | null,
|
||||
tenantId?: string,
|
||||
): GetAppConfigOptions {
|
||||
const userId = user?.id;
|
||||
const hasSourceIdentity =
|
||||
user != null && Object.prototype.hasOwnProperty.call(user, 'idOnTheSource');
|
||||
return {
|
||||
role: user?.role,
|
||||
userId,
|
||||
idOnTheSource: userId && hasSourceIdentity ? (user.idOnTheSource ?? null) : undefined,
|
||||
tenantId: tenantId ?? user?.tenantId ?? getTenantId(),
|
||||
};
|
||||
}
|
||||
|
||||
// ── Helpers ──────────────────────────────────────────────────────────
|
||||
|
||||
let _strictOverride: boolean | undefined;
|
||||
|
|
@ -77,10 +102,10 @@ function overrideCacheKey(role?: string, userId?: string, tenantId?: string): st
|
|||
// tenant middleware (the common path) pass no explicit tenantId, so without this the
|
||||
// entry is keyed under the shared `__default__` bucket and leaks across tenants.
|
||||
const tenant = tenantId || getTenantId() || '__default__';
|
||||
if (userId && role) {
|
||||
return `_OVERRIDE_:${tenant}:${role}:${userId}`;
|
||||
}
|
||||
if (userId) {
|
||||
if (role) {
|
||||
return `_OVERRIDE_:${tenant}:${role}:${userId}`;
|
||||
}
|
||||
return `_OVERRIDE_:${tenant}:${userId}`;
|
||||
}
|
||||
if (role) {
|
||||
|
|
@ -111,9 +136,17 @@ export function createAppConfigService(deps: AppConfigServiceDeps): {
|
|||
async function buildPrincipals(
|
||||
role?: string,
|
||||
userId?: string,
|
||||
idOnTheSource?: string | null,
|
||||
): Promise<Array<{ principalType: string; principalId?: string | Types.ObjectId }>> {
|
||||
if (userId) {
|
||||
return getUserPrincipals({ userId, role });
|
||||
const params: { userId: string; role?: string | null; idOnTheSource?: string | null } = {
|
||||
userId,
|
||||
role,
|
||||
};
|
||||
if (idOnTheSource !== undefined) {
|
||||
params.idOnTheSource = idOnTheSource;
|
||||
}
|
||||
return getUserPrincipals(params);
|
||||
}
|
||||
const principals: Array<{ principalType: string; principalId?: string | Types.ObjectId }> = [];
|
||||
if (role) {
|
||||
|
|
@ -157,7 +190,7 @@ export function createAppConfigService(deps: AppConfigServiceDeps): {
|
|||
* Use this for startup, auth strategies, and other pre-tenant code paths.
|
||||
*/
|
||||
async function getAppConfig(options: GetAppConfigOptions = {}): Promise<AppConfig> {
|
||||
const { role, userId, tenantId, refresh, baseOnly } = options;
|
||||
const { role, userId, idOnTheSource, tenantId, refresh, baseOnly } = options;
|
||||
|
||||
const baseConfig = await ensureBaseConfig(refresh);
|
||||
|
||||
|
|
@ -173,10 +206,12 @@ export function createAppConfigService(deps: AppConfigServiceDeps): {
|
|||
}
|
||||
}
|
||||
|
||||
const principals = await buildPrincipals(role, userId).catch((error: unknown) => {
|
||||
logger.error('[getAppConfig] Error building principals, falling back to base:', error);
|
||||
return null;
|
||||
});
|
||||
const principals = await buildPrincipals(role, userId, idOnTheSource).catch(
|
||||
(error: unknown) => {
|
||||
logger.error('[getAppConfig] Error building principals, falling back to base:', error);
|
||||
return null;
|
||||
},
|
||||
);
|
||||
if (principals === null) {
|
||||
return baseConfig;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -258,6 +258,7 @@ describe('createEndpointsConfigService', () => {
|
|||
expect(mockGetAppConfig).toHaveBeenCalledWith({
|
||||
role: 'USER',
|
||||
userId: 'u1',
|
||||
idOnTheSource: undefined,
|
||||
tenantId: 'tenant-a',
|
||||
});
|
||||
});
|
||||
|
|
@ -314,7 +315,10 @@ describe('createEndpointsConfigService', () => {
|
|||
|
||||
const result = await getEndpointsConfig(fakeReq({ user: { id: 'u1', role: 'USER' } }));
|
||||
|
||||
expect(getUserPrincipals).toHaveBeenCalledWith({ userId: 'u1', role: 'USER' });
|
||||
expect(getUserPrincipals).toHaveBeenCalledWith({
|
||||
userId: 'u1',
|
||||
role: 'USER',
|
||||
});
|
||||
expect(getApplicableConfigs).toHaveBeenCalledWith(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({ principalType: PrincipalType.GROUP, principalId: groupId }),
|
||||
|
|
|
|||
|
|
@ -8,18 +8,16 @@ import {
|
|||
import type { AgentCapabilities, TEndpointsConfig, TConfig } from 'librechat-data-provider';
|
||||
import type { AppConfig } from '@librechat/data-schemas';
|
||||
import type { ServerRequest, TCustomEndpointsConfig } from '~/types';
|
||||
import type { GetAppConfigOptions } from '~/app/service';
|
||||
import { loadCustomEndpointsConfig as defaultLoadCustomEndpoints } from '~/endpoints/custom';
|
||||
import { getAppConfigOptionsFromUser } from '~/app/service';
|
||||
|
||||
type PartialEndpointEntry = Partial<TConfig> & Record<string, unknown>;
|
||||
type DefaultEndpointsResult = Record<string, PartialEndpointEntry | false | null>;
|
||||
type MutableEndpointsConfig = Record<string, PartialEndpointEntry | false | null | undefined>;
|
||||
|
||||
export interface EndpointsConfigDeps {
|
||||
getAppConfig: (params: {
|
||||
role?: string;
|
||||
userId?: string;
|
||||
tenantId?: string;
|
||||
}) => Promise<AppConfig>;
|
||||
getAppConfig: (params: GetAppConfigOptions) => Promise<AppConfig>;
|
||||
loadDefaultEndpointsConfig: (appConfig: AppConfig) => Promise<DefaultEndpointsResult>;
|
||||
loadCustomEndpointsConfig?: (custom: unknown) => TCustomEndpointsConfig | undefined;
|
||||
}
|
||||
|
|
@ -35,13 +33,7 @@ export function createEndpointsConfigService(deps: EndpointsConfigDeps): {
|
|||
} = deps;
|
||||
|
||||
async function getEndpointsConfig(req: ServerRequest): Promise<TEndpointsConfig> {
|
||||
const appConfig =
|
||||
req.config ??
|
||||
(await getAppConfig({
|
||||
role: req.user?.role,
|
||||
userId: req.user?.id,
|
||||
tenantId: req.user?.tenantId,
|
||||
}));
|
||||
const appConfig = req.config ?? (await getAppConfig(getAppConfigOptionsFromUser(req.user)));
|
||||
const defaultEndpointsConfig = await loadDefaultEndpointsConfig(appConfig);
|
||||
const customEndpointsConfig = loadCustomEndpointsConfig(appConfig?.endpoints?.custom);
|
||||
|
||||
|
|
|
|||
|
|
@ -10,8 +10,10 @@ import type { TModelsConfig, TEndpoint } from 'librechat-data-provider';
|
|||
import type { AppConfig } from '@librechat/data-schemas';
|
||||
import type { ServerRequest, GetUserKeyValuesFunction, UserKeyValues } from '~/types';
|
||||
import type { FetchModelsParams } from '~/endpoints/models';
|
||||
import type { GetAppConfigOptions } from '~/app/service';
|
||||
import { fetchModels as defaultFetchModels } from '~/endpoints/models';
|
||||
import { getTokenConfigKey } from '~/endpoints/custom/initialize';
|
||||
import { getAppConfigOptionsFromUser } from '~/app/service';
|
||||
import { validateEndpointURL } from '~/auth';
|
||||
import { tokenConfigCache } from '~/cache';
|
||||
import { isUserProvided } from '~/utils';
|
||||
|
|
@ -43,11 +45,7 @@ interface ResolvedEndpoint {
|
|||
}
|
||||
|
||||
export interface LoadConfigModelsDeps {
|
||||
getAppConfig: (params: {
|
||||
role?: string;
|
||||
userId?: string;
|
||||
tenantId?: string;
|
||||
}) => Promise<AppConfig>;
|
||||
getAppConfig: (params: GetAppConfigOptions) => Promise<AppConfig>;
|
||||
getUserKeyValues: GetUserKeyValuesFunction;
|
||||
fetchModels?: (params: FetchModelsParams) => Promise<string[]>;
|
||||
}
|
||||
|
|
@ -56,13 +54,7 @@ export function createLoadConfigModels(deps: LoadConfigModelsDeps) {
|
|||
const { getAppConfig, getUserKeyValues, fetchModels = defaultFetchModels } = deps;
|
||||
|
||||
return async function loadConfigModels(req: ServerRequest): Promise<TModelsConfig> {
|
||||
const appConfig =
|
||||
req.config ??
|
||||
(await getAppConfig({
|
||||
role: req.user?.role,
|
||||
userId: req.user?.id,
|
||||
tenantId: req.user?.tenantId,
|
||||
}));
|
||||
const appConfig = req.config ?? (await getAppConfig(getAppConfigOptionsFromUser(req.user)));
|
||||
if (!appConfig) {
|
||||
return {};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -17,3 +17,4 @@ export * from './checkBalance';
|
|||
export * from './remoteAgentAuth';
|
||||
export * from './share';
|
||||
export * from './messageFilterPii';
|
||||
export * from './messageValidation';
|
||||
|
|
|
|||
233
packages/api/src/middleware/messageValidation.ts
Normal file
233
packages/api/src/middleware/messageValidation.ts
Normal file
|
|
@ -0,0 +1,233 @@
|
|||
import type { NextFunction, Response } from 'express';
|
||||
|
||||
type MessageValidationUser = {
|
||||
id: string;
|
||||
tenantId?: string | null;
|
||||
};
|
||||
|
||||
type MessageValidationBody = {
|
||||
conversationId?: string;
|
||||
message?: {
|
||||
conversationId?: string;
|
||||
};
|
||||
};
|
||||
|
||||
type MessageValidationParams = {
|
||||
conversationId?: string;
|
||||
messageId?: string;
|
||||
};
|
||||
|
||||
export type MessageValidationRequest = {
|
||||
method?: string;
|
||||
params?: MessageValidationParams;
|
||||
body?: MessageValidationBody;
|
||||
user: MessageValidationUser;
|
||||
messageRequestValidation?: MessageRequestValidation;
|
||||
};
|
||||
|
||||
type ConversationRecord = {
|
||||
user?: string;
|
||||
} | null;
|
||||
|
||||
type PendingActionRecord = unknown;
|
||||
|
||||
type GenerationJobRecord = {
|
||||
status?: string;
|
||||
metadata?: {
|
||||
userId?: string;
|
||||
tenantId?: string | null;
|
||||
pendingAction?: PendingActionRecord;
|
||||
};
|
||||
} | null;
|
||||
|
||||
export type MessageValidationResult =
|
||||
| {
|
||||
ok: true;
|
||||
}
|
||||
| {
|
||||
ok: false;
|
||||
status: number;
|
||||
body: unknown;
|
||||
send?: boolean;
|
||||
};
|
||||
|
||||
export type FailedMessageValidationResult = Extract<MessageValidationResult, { ok: false }>;
|
||||
|
||||
export type MessageRequestValidation = {
|
||||
conversationId?: string;
|
||||
shouldFetchMessages: boolean;
|
||||
promise: Promise<MessageValidationResult>;
|
||||
};
|
||||
|
||||
type MessageValidationLogger = {
|
||||
warn: (message: string, error: unknown) => void;
|
||||
};
|
||||
|
||||
export type MessageValidationDeps = {
|
||||
getConvo: (userId: string, conversationId?: string) => Promise<ConversationRecord>;
|
||||
getJob: (conversationId?: string) => Promise<GenerationJobRecord>;
|
||||
isPendingActionStale: (job: { pendingAction?: PendingActionRecord }) => boolean;
|
||||
logger: MessageValidationLogger;
|
||||
};
|
||||
|
||||
export type MessageRequestMiddleware = {
|
||||
createMessageRequestValidation: (req: MessageValidationRequest) => MessageRequestValidation;
|
||||
prepareMessageRequestValidation: (
|
||||
req: MessageValidationRequest,
|
||||
res: Response,
|
||||
next: NextFunction,
|
||||
) => void;
|
||||
sendValidationResponse: (res: Response, result: FailedMessageValidationResult) => Response;
|
||||
validateMessageReq: (
|
||||
req: MessageValidationRequest,
|
||||
res: Response,
|
||||
next: NextFunction,
|
||||
) => Promise<Response | void>;
|
||||
};
|
||||
|
||||
function hasTenantMismatch(job: GenerationJobRecord, user: MessageValidationUser): boolean {
|
||||
// Untenanted jobs remain readable by their owner for pre-multi-tenancy deployments.
|
||||
return job?.metadata?.tenantId != null && job.metadata.tenantId !== user.tenantId;
|
||||
}
|
||||
|
||||
export function createMessageRequestMiddleware(
|
||||
deps: MessageValidationDeps,
|
||||
): MessageRequestMiddleware {
|
||||
async function canReadActiveJobConversation(
|
||||
req: MessageValidationRequest,
|
||||
conversationId?: string,
|
||||
): Promise<boolean> {
|
||||
if (req.method !== 'GET' || req.params?.messageId) {
|
||||
return false;
|
||||
}
|
||||
|
||||
let job: GenerationJobRecord;
|
||||
try {
|
||||
job = await deps.getJob(conversationId);
|
||||
} catch (error) {
|
||||
deps.logger.warn(
|
||||
`[validateMessageReq] Active job lookup failed for ${conversationId}:`,
|
||||
error,
|
||||
);
|
||||
return false;
|
||||
}
|
||||
|
||||
if (!job) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// A job paused for human review is still active (consistent with /chat/status
|
||||
// and /chat/active), so a new-conversation run that pauses before its final
|
||||
// save can still recover the prompt, but only while it has a live,
|
||||
// resolvable prompt (missing/malformed or past-expiry reads as inactive).
|
||||
const isActive =
|
||||
job.status === 'running' ||
|
||||
(job.status === 'requires_action' &&
|
||||
!deps.isPendingActionStale({ pendingAction: job.metadata?.pendingAction }));
|
||||
if (!isActive) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return job.metadata?.userId === req.user.id && !hasTenantMismatch(job, req.user);
|
||||
}
|
||||
|
||||
async function validateConversationAccess(
|
||||
req: MessageValidationRequest,
|
||||
conversationId?: string,
|
||||
): Promise<MessageValidationResult> {
|
||||
const conversation = await deps.getConvo(req.user.id, conversationId);
|
||||
|
||||
if (!conversation) {
|
||||
if (await canReadActiveJobConversation(req, conversationId)) {
|
||||
return { ok: true };
|
||||
}
|
||||
|
||||
return { ok: false, status: 404, body: { error: 'Conversation not found' } };
|
||||
}
|
||||
|
||||
if (conversation.user !== req.user.id) {
|
||||
return {
|
||||
ok: false,
|
||||
status: 403,
|
||||
body: { error: 'User not authorized for this conversation' },
|
||||
};
|
||||
}
|
||||
|
||||
return { ok: true };
|
||||
}
|
||||
|
||||
function createMessageRequestValidation(req: MessageValidationRequest): MessageRequestValidation {
|
||||
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 {
|
||||
shouldFetchMessages: false,
|
||||
promise: Promise.resolve({
|
||||
ok: false,
|
||||
status: 400,
|
||||
body: { error: 'Conversation ID mismatch' },
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
const conversationId = paramConversationId || bodyConversationId || nestedConversationId;
|
||||
|
||||
if (conversationId === 'new') {
|
||||
return {
|
||||
conversationId,
|
||||
shouldFetchMessages: false,
|
||||
promise: Promise.resolve({ ok: false, status: 200, body: [], send: true }),
|
||||
};
|
||||
}
|
||||
|
||||
return {
|
||||
conversationId,
|
||||
shouldFetchMessages: true,
|
||||
promise: validateConversationAccess(req, conversationId),
|
||||
};
|
||||
}
|
||||
|
||||
function sendValidationResponse(res: Response, result: FailedMessageValidationResult): Response {
|
||||
if (result.send) {
|
||||
return res.status(result.status).send(result.body);
|
||||
}
|
||||
return res.status(result.status).json(result.body);
|
||||
}
|
||||
|
||||
function prepareMessageRequestValidation(
|
||||
req: MessageValidationRequest,
|
||||
_res: Response,
|
||||
next: NextFunction,
|
||||
): void {
|
||||
req.messageRequestValidation = createMessageRequestValidation(req);
|
||||
next();
|
||||
}
|
||||
|
||||
async function validateMessageReq(
|
||||
req: MessageValidationRequest,
|
||||
res: Response,
|
||||
next: NextFunction,
|
||||
): Promise<Response | void> {
|
||||
const validation = createMessageRequestValidation(req);
|
||||
const result = await validation.promise;
|
||||
if (!result.ok) {
|
||||
return sendValidationResponse(res, result);
|
||||
}
|
||||
next();
|
||||
}
|
||||
|
||||
return {
|
||||
createMessageRequestValidation: createMessageRequestValidation,
|
||||
prepareMessageRequestValidation: prepareMessageRequestValidation,
|
||||
sendValidationResponse: sendValidationResponse,
|
||||
validateMessageReq: validateMessageReq,
|
||||
};
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue