From 01f19b503a993efbf956b42a2e5e0029d040812f Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Fri, 20 Mar 2026 17:10:39 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=82=20fix:=20Gate=20MCP=20Queries=20Be?= =?UTF-8?q?hind=20USE=20Permission=20to=20Prevent=20403=20Spam=20(#12345)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 🐛 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) --- .../Config/__tests__/useAppStartup.spec.tsx | 123 ++++++++++++++++++ client/src/hooks/Config/useAppStartup.ts | 20 ++- client/src/hooks/MCP/useMCPServerManager.ts | 24 +++- 3 files changed, 158 insertions(+), 9 deletions(-) create mode 100644 client/src/hooks/Config/__tests__/useAppStartup.spec.tsx diff --git a/client/src/hooks/Config/__tests__/useAppStartup.spec.tsx b/client/src/hooks/Config/__tests__/useAppStartup.spec.tsx new file mode 100644 index 0000000000..eef2795a76 --- /dev/null +++ b/client/src/hooks/Config/__tests__/useAppStartup.spec.tsx @@ -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 }) => ( + {children} +); + +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 }); + }); +}); diff --git a/client/src/hooks/Config/useAppStartup.ts b/client/src/hooks/Config/useAppStartup.ts index 52b4325eea..f40b283ee2 100644 --- a/client/src/hooks/Config/useAppStartup.ts +++ b/client/src/hooks/Config/useAppStartup.ts @@ -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 */ diff --git a/client/src/hooks/MCP/useMCPServerManager.ts b/client/src/hooks/MCP/useMCPServerManager.ts index af65ba4507..4ba1ff6278 100644 --- a/client/src/hooks/MCP/useMCPServerManager.ts +++ b/client/src/hooks/MCP/useMCPServerManager.ts @@ -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(null);