mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-03-21 15:16:33 +01:00
🛂 fix: Gate MCP Queries Behind USE Permission to Prevent 403 Spam (#12345)
* 🐛 fix: Gate MCP queries behind USE permission to prevent 403 spam Closes #12342 When `interface.mcpServers.use` is set to `false` in `librechat.yaml`, the frontend was still unconditionally fetching `/api/mcp/servers` on every app startup, window focus, and stale interval — producing continuous 403 "Insufficient permissions" log entries. Add `useHasAccess` permission checks to both `useMCPServersQuery` call sites (`useAppStartup` and `useMCPServerManager`) so the query is disabled when the user lacks `MCP_SERVERS.USE`, matching the guard pattern already used by MCP UI components. * fix: Lint and import order corrections * fix: Address review findings — gate permissions query, add tests - Gate `useGetAllEffectivePermissionsQuery` behind `canUseMcp` in `useMCPServerManager` for consistency (wasted request when MCP disabled, even though this endpoint doesn't 403) - Sort multi-line `librechat-data-provider` import shortest to longest - Restore intent comment on `useGetStartupConfig` call - Add `useAppStartup` test suite covering MCP permission gating: query suppression when USE denied, compound `enabled` conditions for tools query (servers loading, empty, no user)
This commit is contained in:
parent
365a0dc0f6
commit
01f19b503a
3 changed files with 158 additions and 9 deletions
123
client/src/hooks/Config/__tests__/useAppStartup.spec.tsx
Normal file
123
client/src/hooks/Config/__tests__/useAppStartup.spec.tsx
Normal file
|
|
@ -0,0 +1,123 @@
|
|||
import React from 'react';
|
||||
import { RecoilRoot } from 'recoil';
|
||||
import { renderHook } from '@testing-library/react';
|
||||
import { PermissionTypes, Permissions } from 'librechat-data-provider';
|
||||
import type { TUser } from 'librechat-data-provider';
|
||||
|
||||
const mockUseHasAccess = jest.fn();
|
||||
const mockUseMCPServersQuery = jest.fn();
|
||||
const mockUseMCPToolsQuery = jest.fn();
|
||||
|
||||
jest.mock('~/hooks', () => ({
|
||||
useHasAccess: (args: unknown) => mockUseHasAccess(args),
|
||||
}));
|
||||
|
||||
jest.mock('~/data-provider', () => ({
|
||||
useMCPServersQuery: (config: unknown) => mockUseMCPServersQuery(config),
|
||||
useMCPToolsQuery: (config: unknown) => mockUseMCPToolsQuery(config),
|
||||
}));
|
||||
|
||||
jest.mock('../useSpeechSettingsInit', () => ({
|
||||
__esModule: true,
|
||||
default: jest.fn(),
|
||||
}));
|
||||
|
||||
jest.mock('~/utils/timestamps', () => ({
|
||||
cleanupTimestampedStorage: jest.fn(),
|
||||
}));
|
||||
|
||||
jest.mock('react-gtm-module', () => ({
|
||||
__esModule: true,
|
||||
default: { initialize: jest.fn() },
|
||||
}));
|
||||
|
||||
import useAppStartup from '../useAppStartup';
|
||||
|
||||
const mockUser = {
|
||||
id: 'user-123',
|
||||
username: 'testuser',
|
||||
email: 'test@example.com',
|
||||
name: 'Test User',
|
||||
avatar: '',
|
||||
role: 'USER',
|
||||
provider: 'local',
|
||||
emailVerified: true,
|
||||
createdAt: '2023-01-01T00:00:00.000Z',
|
||||
updatedAt: '2023-01-01T00:00:00.000Z',
|
||||
} as TUser;
|
||||
|
||||
const wrapper: React.FC<{ children: React.ReactNode }> = ({ children }) => (
|
||||
<RecoilRoot>{children}</RecoilRoot>
|
||||
);
|
||||
|
||||
describe('useAppStartup — MCP permission gating', () => {
|
||||
beforeEach(() => {
|
||||
mockUseMCPServersQuery.mockReturnValue({ data: undefined, isLoading: false });
|
||||
mockUseMCPToolsQuery.mockReturnValue({ data: undefined, isLoading: false });
|
||||
});
|
||||
|
||||
it('checks the MCP_SERVERS.USE permission via useHasAccess', () => {
|
||||
mockUseHasAccess.mockReturnValue(false);
|
||||
|
||||
renderHook(() => useAppStartup({ startupConfig: undefined, user: mockUser }), { wrapper });
|
||||
|
||||
expect(mockUseHasAccess).toHaveBeenCalledWith({
|
||||
permissionType: PermissionTypes.MCP_SERVERS,
|
||||
permission: Permissions.USE,
|
||||
});
|
||||
});
|
||||
|
||||
it('suppresses all MCP queries when user lacks MCP_SERVERS.USE', () => {
|
||||
mockUseHasAccess.mockReturnValue(false);
|
||||
|
||||
renderHook(() => useAppStartup({ startupConfig: undefined, user: mockUser }), { wrapper });
|
||||
|
||||
expect(mockUseMCPServersQuery).toHaveBeenCalledWith({ enabled: false });
|
||||
expect(mockUseMCPToolsQuery).toHaveBeenCalledWith({ enabled: false });
|
||||
});
|
||||
|
||||
it('enables servers query and tools query when permission granted, servers loaded, and user present', () => {
|
||||
mockUseHasAccess.mockReturnValue(true);
|
||||
mockUseMCPServersQuery.mockReturnValue({
|
||||
data: { 'test-server': { url: 'http://test' } },
|
||||
isLoading: false,
|
||||
});
|
||||
|
||||
renderHook(() => useAppStartup({ startupConfig: undefined, user: mockUser }), { wrapper });
|
||||
|
||||
expect(mockUseMCPServersQuery).toHaveBeenCalledWith({ enabled: true });
|
||||
expect(mockUseMCPToolsQuery).toHaveBeenCalledWith({ enabled: true });
|
||||
});
|
||||
|
||||
it('suppresses tools query when permission granted but user prop is undefined', () => {
|
||||
mockUseHasAccess.mockReturnValue(true);
|
||||
mockUseMCPServersQuery.mockReturnValue({
|
||||
data: { 'test-server': { url: 'http://test' } },
|
||||
isLoading: false,
|
||||
});
|
||||
|
||||
renderHook(() => useAppStartup({ startupConfig: undefined, user: undefined }), { wrapper });
|
||||
|
||||
expect(mockUseMCPServersQuery).toHaveBeenCalledWith({ enabled: true });
|
||||
expect(mockUseMCPToolsQuery).toHaveBeenCalledWith({ enabled: false });
|
||||
});
|
||||
|
||||
it('suppresses tools query when permission granted but no servers loaded', () => {
|
||||
mockUseHasAccess.mockReturnValue(true);
|
||||
mockUseMCPServersQuery.mockReturnValue({ data: {}, isLoading: false });
|
||||
|
||||
renderHook(() => useAppStartup({ startupConfig: undefined, user: mockUser }), { wrapper });
|
||||
|
||||
expect(mockUseMCPServersQuery).toHaveBeenCalledWith({ enabled: true });
|
||||
expect(mockUseMCPToolsQuery).toHaveBeenCalledWith({ enabled: false });
|
||||
});
|
||||
|
||||
it('suppresses tools query while servers are still loading', () => {
|
||||
mockUseHasAccess.mockReturnValue(true);
|
||||
mockUseMCPServersQuery.mockReturnValue({ data: undefined, isLoading: true });
|
||||
|
||||
renderHook(() => useAppStartup({ startupConfig: undefined, user: mockUser }), { wrapper });
|
||||
|
||||
expect(mockUseMCPToolsQuery).toHaveBeenCalledWith({ enabled: false });
|
||||
});
|
||||
});
|
||||
|
|
@ -1,11 +1,12 @@
|
|||
import { useEffect } from 'react';
|
||||
import { useRecoilState } from 'recoil';
|
||||
import TagManager from 'react-gtm-module';
|
||||
import { LocalStorageKeys } from 'librechat-data-provider';
|
||||
import { LocalStorageKeys, PermissionTypes, Permissions } from 'librechat-data-provider';
|
||||
import type { TStartupConfig, TUser } from 'librechat-data-provider';
|
||||
import { useMCPToolsQuery, useMCPServersQuery } from '~/data-provider';
|
||||
import { cleanupTimestampedStorage } from '~/utils/timestamps';
|
||||
import useSpeechSettingsInit from './useSpeechSettingsInit';
|
||||
import { useMCPToolsQuery, useMCPServersQuery } from '~/data-provider';
|
||||
import { useHasAccess } from '~/hooks';
|
||||
import store from '~/store';
|
||||
|
||||
export default function useAppStartup({
|
||||
|
|
@ -16,12 +17,23 @@ export default function useAppStartup({
|
|||
user?: TUser;
|
||||
}) {
|
||||
const [defaultPreset, setDefaultPreset] = useRecoilState(store.defaultPreset);
|
||||
const canUseMcp = useHasAccess({
|
||||
permissionType: PermissionTypes.MCP_SERVERS,
|
||||
permission: Permissions.USE,
|
||||
});
|
||||
|
||||
useSpeechSettingsInit(!!user);
|
||||
const { data: loadedServers, isLoading: serversLoading } = useMCPServersQuery();
|
||||
const { data: loadedServers, isLoading: serversLoading } = useMCPServersQuery({
|
||||
enabled: canUseMcp,
|
||||
});
|
||||
|
||||
useMCPToolsQuery({
|
||||
enabled: !serversLoading && !!loadedServers && Object.keys(loadedServers).length > 0 && !!user,
|
||||
enabled:
|
||||
canUseMcp &&
|
||||
!serversLoading &&
|
||||
!!loadedServers &&
|
||||
Object.keys(loadedServers).length > 0 &&
|
||||
!!user,
|
||||
});
|
||||
|
||||
/** Clean up old localStorage entries on startup */
|
||||
|
|
|
|||
|
|
@ -2,7 +2,14 @@ import { useCallback, useState, useMemo, useRef, useEffect } from 'react';
|
|||
import { useAtom } from 'jotai';
|
||||
import { useToastContext } from '@librechat/client';
|
||||
import { useQueryClient } from '@tanstack/react-query';
|
||||
import { Constants, QueryKeys, MCPOptions, ResourceType } from 'librechat-data-provider';
|
||||
import {
|
||||
Constants,
|
||||
QueryKeys,
|
||||
MCPOptions,
|
||||
Permissions,
|
||||
ResourceType,
|
||||
PermissionTypes,
|
||||
} from 'librechat-data-provider';
|
||||
import {
|
||||
useCancelMCPOAuthMutation,
|
||||
useUpdateUserPluginsMutation,
|
||||
|
|
@ -11,7 +18,7 @@ import {
|
|||
} from 'librechat-data-provider/react-query';
|
||||
import type { TUpdateUserPlugins, TPlugin, MCPServersResponse } from 'librechat-data-provider';
|
||||
import type { ConfigFieldDetail } from '~/common';
|
||||
import { useLocalize, useMCPSelect, useMCPConnectionStatus } from '~/hooks';
|
||||
import { useLocalize, useHasAccess, useMCPSelect, useMCPConnectionStatus } from '~/hooks';
|
||||
import { useGetStartupConfig, useMCPServersQuery } from '~/data-provider';
|
||||
import { mcpServerInitStatesAtom, getServerInitState } from '~/store/mcp';
|
||||
import type { MCPServerInitState } from '~/store/mcp';
|
||||
|
|
@ -35,12 +42,19 @@ export function useMCPServerManager({
|
|||
const localize = useLocalize();
|
||||
const queryClient = useQueryClient();
|
||||
const { showToast } = useToastContext();
|
||||
const { data: startupConfig } = useGetStartupConfig(); // Keep for UI config only
|
||||
/** Retained for `interface.mcpServers.placeholder` used by `placeholderText` below */
|
||||
const { data: startupConfig } = useGetStartupConfig();
|
||||
const canUseMcp = useHasAccess({
|
||||
permissionType: PermissionTypes.MCP_SERVERS,
|
||||
permission: Permissions.USE,
|
||||
});
|
||||
|
||||
const { data: loadedServers, isLoading } = useMCPServersQuery();
|
||||
const { data: loadedServers, isLoading } = useMCPServersQuery({ enabled: canUseMcp });
|
||||
|
||||
// Fetch effective permissions for all MCP servers
|
||||
const { data: permissionsMap } = useGetAllEffectivePermissionsQuery(ResourceType.MCPSERVER);
|
||||
const { data: permissionsMap } = useGetAllEffectivePermissionsQuery(ResourceType.MCPSERVER, {
|
||||
enabled: canUseMcp,
|
||||
});
|
||||
|
||||
const [isConfigModalOpen, setIsConfigModalOpen] = useState(false);
|
||||
const [selectedToolForConfig, setSelectedToolForConfig] = useState<TPlugin | null>(null);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue