fix: update appConfig usage to access allowedDomains from actions instead of registration

This commit is contained in:
Danny Avila 2025-08-18 02:08:34 -04:00
parent 493f60fa54
commit f25e4253d0
No known key found for this signature in database
GPG key ID: BF31EEB2C5CA0956
7 changed files with 23 additions and 59 deletions

View file

@ -87,7 +87,7 @@ router.post(
const appConfig = await getAppConfig({ role: req.user.role }); const appConfig = await getAppConfig({ role: req.user.role });
const isDomainAllowed = await isActionDomainAllowed( const isDomainAllowed = await isActionDomainAllowed(
metadata.domain, metadata.domain,
appConfig?.registration?.allowedDomains, appConfig?.actions?.allowedDomains,
); );
if (!isDomainAllowed) { if (!isDomainAllowed) {
return res.status(400).json({ message: 'Domain not allowed' }); return res.status(400).json({ message: 'Domain not allowed' });

View file

@ -34,7 +34,7 @@ router.post('/:assistant_id', async (req, res) => {
let metadata = await encryptMetadata(removeNullishValues(_metadata, true)); let metadata = await encryptMetadata(removeNullishValues(_metadata, true));
const isDomainAllowed = await isActionDomainAllowed( const isDomainAllowed = await isActionDomainAllowed(
metadata.domain, metadata.domain,
appConfig?.registration?.allowedDomains, appConfig?.actions?.allowedDomains,
); );
if (!isDomainAllowed) { if (!isDomainAllowed) {
return res.status(400).json({ message: 'Domain not allowed' }); return res.status(400).json({ message: 'Domain not allowed' });

View file

@ -8,21 +8,6 @@ const { createMulterInstance, storage, importFileFilter } = require('./multer');
// Mock only the config service that requires external dependencies // Mock only the config service that requires external dependencies
jest.mock('~/server/services/Config', () => ({ jest.mock('~/server/services/Config', () => ({
getCustomConfig: jest.fn(() =>
Promise.resolve({
fileConfig: {
endpoints: {
openAI: {
supportedMimeTypes: ['image/jpeg', 'image/png', 'application/pdf'],
},
default: {
supportedMimeTypes: ['image/jpeg', 'image/png', 'text/plain'],
},
},
serverFileSizeLimit: 10000000, // 10MB
},
}),
),
getAppConfig: jest.fn(), getAppConfig: jest.fn(),
})); }));
@ -546,10 +531,10 @@ describe('Multer Configuration', () => {
describe('Real Configuration Testing', () => { describe('Real Configuration Testing', () => {
it('should handle missing custom config gracefully with real mergeFileConfig', async () => { it('should handle missing custom config gracefully with real mergeFileConfig', async () => {
const { getCustomConfig } = require('~/server/services/Config'); const { getAppConfig } = require('~/server/services/Config');
// Mock getCustomConfig to return undefined // Mock getAppConfig to return undefined
getCustomConfig.mockResolvedValueOnce(undefined); getAppConfig.mockResolvedValueOnce(undefined);
const multerInstance = await createMulterInstance(); const multerInstance = await createMulterInstance();
expect(multerInstance).toBeDefined(); expect(multerInstance).toBeDefined();

View file

@ -1,10 +1,7 @@
const { Constants, EModelEndpoint, actionDomainSeparator } = require('librechat-data-provider'); const { Constants, actionDomainSeparator } = require('librechat-data-provider');
const { domainParser } = require('./ActionService'); const { domainParser } = require('./ActionService');
jest.mock('keyv'); jest.mock('keyv');
jest.mock('~/server/services/Config', () => ({
getCustomConfig: jest.fn(),
}));
const globalCache = {}; const globalCache = {};
jest.mock('~/cache/getLogStores', () => { jest.mock('~/cache/getLogStores', () => {
@ -53,26 +50,6 @@ jest.mock('~/cache/getLogStores', () => {
}); });
describe('domainParser', () => { describe('domainParser', () => {
const req = {
app: {
locals: {
[EModelEndpoint.azureOpenAI]: {
assistants: true,
},
},
},
};
const reqNoAzure = {
app: {
locals: {
[EModelEndpoint.azureOpenAI]: {
assistants: false,
},
},
},
};
const TLD = '.com'; const TLD = '.com';
// Non-azure request // Non-azure request

View file

@ -366,7 +366,7 @@ async function processRequiredActions(client, requiredActions) {
const isDomainAllowed = await isActionDomainAllowed( const isDomainAllowed = await isActionDomainAllowed(
action.metadata.domain, action.metadata.domain,
appConfig?.registration?.allowedDomains, appConfig?.actions?.allowedDomains,
); );
if (!isDomainAllowed) { if (!isDomainAllowed) {
continue; continue;
@ -635,7 +635,7 @@ async function loadAgentTools({ req, res, agent, tool_resources, openAIApiKey })
// Check if domain is allowed (do this once per action set) // Check if domain is allowed (do this once per action set)
const isDomainAllowed = await isActionDomainAllowed( const isDomainAllowed = await isActionDomainAllowed(
action.metadata.domain, action.metadata.domain,
appConfig?.registration?.allowedDomains, appConfig?.actions?.allowedDomains,
); );
if (!isDomainAllowed) { if (!isDomainAllowed) {
continue; continue;

View file

@ -1,8 +1,8 @@
const { isEmailDomainAllowed, isActionDomainAllowed } = require('~/server/services/domains'); const { isEmailDomainAllowed, isActionDomainAllowed } = require('~/server/services/domains');
const { getCustomConfig } = require('~/server/services/Config'); const { getAppConfig } = require('~/server/services/Config');
jest.mock('~/server/services/Config', () => ({ jest.mock('~/server/services/Config', () => ({
getCustomConfig: jest.fn(), getAppConfig: jest.fn(),
})); }));
describe('isEmailDomainAllowed', () => { describe('isEmailDomainAllowed', () => {
@ -24,21 +24,21 @@ describe('isEmailDomainAllowed', () => {
it('should return true if customConfig is not available', async () => { it('should return true if customConfig is not available', async () => {
const email = 'test@domain1.com'; const email = 'test@domain1.com';
getCustomConfig.mockResolvedValue(null); getAppConfig.mockResolvedValue(null);
const result = await isEmailDomainAllowed(email, null); const result = await isEmailDomainAllowed(email, null);
expect(result).toBe(true); expect(result).toBe(true);
}); });
it('should return true if allowedDomains is not defined in customConfig', async () => { it('should return true if allowedDomains is not defined in customConfig', async () => {
const email = 'test@domain1.com'; const email = 'test@domain1.com';
getCustomConfig.mockResolvedValue({}); getAppConfig.mockResolvedValue({});
const result = await isEmailDomainAllowed(email, undefined); const result = await isEmailDomainAllowed(email, undefined);
expect(result).toBe(true); expect(result).toBe(true);
}); });
it('should return true if domain is included in the allowedDomains', async () => { it('should return true if domain is included in the allowedDomains', async () => {
const email = 'user@domain1.com'; const email = 'user@domain1.com';
getCustomConfig.mockResolvedValue({ getAppConfig.mockResolvedValue({
registration: { registration: {
allowedDomains: ['domain1.com', 'domain2.com'], allowedDomains: ['domain1.com', 'domain2.com'],
}, },
@ -49,7 +49,7 @@ describe('isEmailDomainAllowed', () => {
it('should return false if domain is not included in the allowedDomains', async () => { it('should return false if domain is not included in the allowedDomains', async () => {
const email = 'user@domain3.com'; const email = 'user@domain3.com';
getCustomConfig.mockResolvedValue({ getAppConfig.mockResolvedValue({
registration: { registration: {
allowedDomains: ['domain1.com', 'domain2.com'], allowedDomains: ['domain1.com', 'domain2.com'],
}, },
@ -80,7 +80,7 @@ describe('isActionDomainAllowed', () => {
}); });
it('should return false for invalid domain formats', async () => { it('should return false for invalid domain formats', async () => {
getCustomConfig.mockResolvedValue({ getAppConfig.mockResolvedValue({
actions: { allowedDomains: ['http://', 'https://'] }, actions: { allowedDomains: ['http://', 'https://'] },
}); });
expect(await isActionDomainAllowed('http://', ['http://', 'https://'])).toBe(false); expect(await isActionDomainAllowed('http://', ['http://', 'https://'])).toBe(false);
@ -91,17 +91,17 @@ describe('isActionDomainAllowed', () => {
// Configuration Tests // Configuration Tests
describe('configuration handling', () => { describe('configuration handling', () => {
it('should return true if customConfig is null', async () => { it('should return true if customConfig is null', async () => {
getCustomConfig.mockResolvedValue(null); getAppConfig.mockResolvedValue(null);
expect(await isActionDomainAllowed('example.com', null)).toBe(true); expect(await isActionDomainAllowed('example.com', null)).toBe(true);
}); });
it('should return true if actions.allowedDomains is not defined', async () => { it('should return true if actions.allowedDomains is not defined', async () => {
getCustomConfig.mockResolvedValue({}); getAppConfig.mockResolvedValue({});
expect(await isActionDomainAllowed('example.com', undefined)).toBe(true); expect(await isActionDomainAllowed('example.com', undefined)).toBe(true);
}); });
it('should return true if allowedDomains is empty array', async () => { it('should return true if allowedDomains is empty array', async () => {
getCustomConfig.mockResolvedValue({ getAppConfig.mockResolvedValue({
actions: { allowedDomains: [] }, actions: { allowedDomains: [] },
}); });
expect(await isActionDomainAllowed('example.com', [])).toBe(true); expect(await isActionDomainAllowed('example.com', [])).toBe(true);
@ -119,7 +119,7 @@ describe('isActionDomainAllowed', () => {
]; ];
beforeEach(() => { beforeEach(() => {
getCustomConfig.mockResolvedValue({ getAppConfig.mockResolvedValue({
actions: { actions: {
allowedDomains, allowedDomains,
}, },
@ -160,7 +160,7 @@ describe('isActionDomainAllowed', () => {
const edgeAllowedDomains = ['example.com', '*.test.com']; const edgeAllowedDomains = ['example.com', '*.test.com'];
beforeEach(() => { beforeEach(() => {
getCustomConfig.mockResolvedValue({ getAppConfig.mockResolvedValue({
actions: { actions: {
allowedDomains: edgeAllowedDomains, allowedDomains: edgeAllowedDomains,
}, },
@ -186,7 +186,7 @@ describe('isActionDomainAllowed', () => {
it('should handle invalid entries in allowedDomains', async () => { it('should handle invalid entries in allowedDomains', async () => {
const invalidAllowedDomains = ['example.com', null, undefined, '', 'test.com']; const invalidAllowedDomains = ['example.com', null, undefined, '', 'test.com'];
getCustomConfig.mockResolvedValue({ getAppConfig.mockResolvedValue({
actions: { actions: {
allowedDomains: invalidAllowedDomains, allowedDomains: invalidAllowedDomains,
}, },

View file

@ -33,6 +33,8 @@ export interface AppConfig {
fileStrategy: FileSources.local | FileSources.s3 | FileSources.firebase | FileSources.azure_blob; fileStrategy: FileSources.local | FileSources.s3 | FileSources.firebase | FileSources.azure_blob;
/** Registration configurations */ /** Registration configurations */
registration?: TCustomConfig['registration']; registration?: TCustomConfig['registration'];
/** Actions configurations */
actions?: TCustomConfig['actions'];
/** Admin-filtered tools */ /** Admin-filtered tools */
filteredTools?: string[]; filteredTools?: string[];
/** Admin-included tools */ /** Admin-included tools */