mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-10-22 01:53:33 +00:00
🚧 fix: Narrow Chat MCP Selections to What the Picker May Offer (#15478)
* 🚧 fix: Narrow Chat MCP Selections to What the Picker May Offer The chat MCP picker offers a server only when the operator has not hidden it (`chatMenu: false`) and the user reaches it directly rather than through an agent (`consumeOnly`). Nothing enforced either condition on the request, so a stale client, a replayed body, or a hand-written one could name any server and get its tools — `consumeOnly` in particular is a permission the registry derives per user, so ignoring it let a user reach in plain chat a server they had only been granted through an agent. Narrow the picker's selection in the two ephemeral loaders, against the same predicate the client's `selectableServers` uses. The resolver is injected, so `packages/api` stays free of the registry and the loaders keep their existing `deps` shape. Provenance is why this lives in the loaders rather than at the tool-resolution chokepoint: only here is a server name known to have come from the picker. Servers a model spec pins are the operator's own choice and are added after the narrowing, so pinning a chat-hidden server to a spec keeps working. `chatMenu` only ever comes from the config tier, so the overlay settles it with no lookup at all. Lookups for the rest run concurrently and are cached per server and user. A name the registry cannot resolve — request-tier, declared on the body and never registered — is kept, as is one whose lookup fails: this narrows an already-authenticated selection and is not the authorization boundary. * 📝 docs: Say What chatMenu Now Bounds The schema still described `chatMenu` as dropdown visibility alone, which is misleading now that it also bars a server from the chat selection a request carries. Spell out both halves, and what it deliberately does not restrict: agents, and servers a model spec pins. * 🛡️ fix: Close Four Gaps in the Chat MCP Narrowing Dedupe before resolving. The selection is request-supplied and the loaders used to build a `Set` immediately; mapping the raw array first let one body repeat a name enough times to launch a resolver — registry, cache, and ACL reads, with no single-flight — per occurrence. Resolve names through the tool-loading aliases. A configured `private server` is addressable as `private_server` downstream, so an exact-key lookup let the normalized spelling walk past a `chatMenu: false` check and still load tools. Names are returned as sent; only the decision is canonicalized. Let the request overlay settle `chatMenu`. The config tier is where the flag lives and is what the picker itself was built from, so an overlay that re-enables a server now outranks the durable config the registry returns, which was silently dropping a valid selection. `consumeOnly` still comes from the registry. Narrow the request body, not just the loader's copy. `applyContextToAgent` reads `req.body.ephemeralAgent.mcp` directly and prefers it over the agent's tools, so a hidden server's `serverInstructions` reached the model context even after its tools were dropped. * 🧭 fix: Ask the Registry Which MCP Servers the Picker May Offer Three rounds of review found the same mistake in three disguises: this narrowing re-derived the registry's tier precedence instead of asking for it. A config entry was treated as authoritative over a user-DB or process-backed server of the same name; the alias map was built from config names only, so a DB server named `private_server` was misrouted to a config `private server`; and the overlay short-circuit ran before any of that could be consulted. Resolve the accessible set through `resolveAllMcpConfigs` instead — the same call behind `GET /api/mcp/servers`, so what the request may use and what the picker was offered come from one place and cannot drift. Precedence, cross-tier name collisions, and the config overlay are the registry's job again, and the selectability test is a single flag check over what it returns. The result is smaller: no overlay branch, no per-server lookups, one catalog read regardless of how long the selection is, and aliases built over the whole accessible set so an exact name always outranks another server's normalized form. Publish the servers actually in play back onto the request body, spec pins included. `applyContextToAgent` prefers that list over the agent's tools, so narrowing to the picker selection alone had started dropping a spec-pinned server's `serverInstructions` while suppressing a hidden one's.
This commit is contained in:
parent
51edb2a660
commit
a3c2277c38
10 changed files with 468 additions and 11 deletions
|
|
@ -170,11 +170,17 @@ const { encodeAndFormat } = require('~/server/services/Files/images/encode');
|
|||
const { createContextHandlers } = require('~/app/clients/prompts');
|
||||
const { resolveConfigServers, getAccessibleMcpServerNames } = require('~/server/services/MCP');
|
||||
const { getMCPServerTools } = require('~/server/services/Config');
|
||||
const { getAccessibleMCPServers } = require('~/server/services/MCP');
|
||||
const BaseClient = require('~/app/clients/BaseClient');
|
||||
const { getMCPManager } = require('~/config');
|
||||
const db = require('~/models');
|
||||
|
||||
const loadAgent = (params) => loadAgentFn(params, { getAgent: db.getAgent, getMCPServerTools });
|
||||
const loadAgent = (params) =>
|
||||
loadAgentFn(params, {
|
||||
getAgent: db.getAgent,
|
||||
getMCPServerTools,
|
||||
getAccessibleMCPServers,
|
||||
});
|
||||
|
||||
const MEMORY_INPUT_CHARS_PER_TOKEN = 8;
|
||||
|
||||
|
|
|
|||
|
|
@ -10,13 +10,17 @@ const {
|
|||
const { isEphemeralAgentId } = require('librechat-data-provider');
|
||||
const { filterFilesByAgentAccess } = require('~/server/services/Files/permissions');
|
||||
const { getMCPServerTools } = require('~/server/services/Config');
|
||||
const { getAccessibleMcpServerNames } = require('~/server/services/MCP');
|
||||
const { getAccessibleMcpServerNames, getAccessibleMCPServers } = require('~/server/services/MCP');
|
||||
const { isFatalAgentInitializationError } = require('~/server/services/ToolService');
|
||||
const { getSkillDbMethods, canAuthorSkillFiles } = require('./skillDeps');
|
||||
const db = require('~/models');
|
||||
|
||||
const loadAddedAgent = (params) =>
|
||||
loadAddedAgentFn(params, { getAgent: db.getAgent, getMCPServerTools });
|
||||
loadAddedAgentFn(params, {
|
||||
getAgent: db.getAgent,
|
||||
getMCPServerTools,
|
||||
getAccessibleMCPServers,
|
||||
});
|
||||
|
||||
/**
|
||||
* Process addedConvo for parallel agent execution.
|
||||
|
|
|
|||
|
|
@ -2,9 +2,15 @@ const { logger } = require('@librechat/data-schemas');
|
|||
const { loadAgent: loadAgentFn } = require('@librechat/api');
|
||||
const { isAgentsEndpoint, removeNullishValues, Constants } = require('librechat-data-provider');
|
||||
const { getMCPServerTools } = require('~/server/services/Config');
|
||||
const { getAccessibleMCPServers } = require('~/server/services/MCP');
|
||||
const db = require('~/models');
|
||||
|
||||
const loadAgent = (params) => loadAgentFn(params, { getAgent: db.getAgent, getMCPServerTools });
|
||||
const loadAgent = (params) =>
|
||||
loadAgentFn(params, {
|
||||
getAgent: db.getAgent,
|
||||
getMCPServerTools,
|
||||
getAccessibleMCPServers,
|
||||
});
|
||||
|
||||
const buildOptions = (req, endpoint, parsedBody, endpointType) => {
|
||||
const { spec, iconURL, agent_id, chatProjectId, ...model_parameters } = parsedBody;
|
||||
|
|
|
|||
|
|
@ -467,6 +467,18 @@ async function resolveCollisionAuditNames({ rawServerNames, accessibleServerName
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The MCP servers a user can reach, keyed by name, with the registry's tier
|
||||
* precedence already applied. This is the resolution behind `GET /api/mcp/servers`,
|
||||
* so anything derived from it agrees with the catalog the client was given.
|
||||
* @param {string} userId
|
||||
* @param {string} [role]
|
||||
* @returns {Promise<Record<string, import('@librechat/api').ParsedServerConfig>>}
|
||||
*/
|
||||
async function getAccessibleMCPServers(userId, role) {
|
||||
return await resolveAllMcpConfigs(userId, role != null ? { role } : undefined);
|
||||
}
|
||||
|
||||
async function resolveAllMcpConfigs(userId, user) {
|
||||
const registry = getMCPServersRegistry();
|
||||
const appConfig = await getAppConfigForUser(userId, user);
|
||||
|
|
@ -1648,6 +1660,7 @@ module.exports = {
|
|||
resolveCollisionAuditNames,
|
||||
resolveMcpConfigNames,
|
||||
resolveAllMcpConfigs,
|
||||
getAccessibleMCPServers,
|
||||
createOAuthStart,
|
||||
checkOAuthFlowStatus,
|
||||
getServerConnectionStatus,
|
||||
|
|
|
|||
|
|
@ -19,10 +19,12 @@ let createAgent: ReturnType<typeof createMethods>['createAgent'];
|
|||
let getAgent: ReturnType<typeof createMethods>['getAgent'];
|
||||
|
||||
const mockGetMCPServerTools = jest.fn();
|
||||
const mockGetAccessibleMCPServers = jest.fn();
|
||||
|
||||
const deps: LoadAgentDeps = {
|
||||
getAgent: (searchParameter) => getAgent(searchParameter) as Promise<LibreChatAgent | null>,
|
||||
getMCPServerTools: mockGetMCPServerTools,
|
||||
getAccessibleMCPServers: mockGetAccessibleMCPServers,
|
||||
};
|
||||
|
||||
describe('loadAgent', () => {
|
||||
|
|
@ -167,6 +169,149 @@ describe('loadAgent', () => {
|
|||
expect(result?.tools).toContain('tool1_mcp_server1');
|
||||
});
|
||||
|
||||
describe('chat-selectable MCP narrowing', () => {
|
||||
const { EPHEMERAL_AGENT_ID } = Constants;
|
||||
|
||||
const loadEphemeral = (
|
||||
config: Record<string, unknown>,
|
||||
body: { ephemeralAgent: { mcp: string[] } },
|
||||
spec?: string,
|
||||
) =>
|
||||
loadAgent(
|
||||
{
|
||||
req: {
|
||||
user: { id: 'user123', role: 'USER' },
|
||||
config: config as unknown as AppConfig,
|
||||
body,
|
||||
},
|
||||
spec,
|
||||
agent_id: EPHEMERAL_AGENT_ID as string,
|
||||
endpoint: 'openai',
|
||||
model_parameters: { model: 'gpt-4' } as unknown as AgentModelParameters,
|
||||
},
|
||||
deps,
|
||||
);
|
||||
|
||||
const selectedServerNames = () => mockGetMCPServerTools.mock.calls.map((call) => call[1]);
|
||||
|
||||
beforeEach(() => {
|
||||
mockGetAccessibleMCPServers.mockResolvedValue({
|
||||
visible: { chatMenu: true },
|
||||
hidden: { chatMenu: false },
|
||||
'agent-only': { consumeOnly: true },
|
||||
});
|
||||
mockGetMCPServerTools.mockImplementation(async (_userId: string, server: string) => ({
|
||||
[`tool_mcp_${server}`]: {},
|
||||
}));
|
||||
});
|
||||
|
||||
test('drops a server the chat menu hides', async () => {
|
||||
const result = await loadEphemeral(
|
||||
{ mcpConfig: {} },
|
||||
{
|
||||
ephemeralAgent: { mcp: ['visible', 'hidden'] },
|
||||
},
|
||||
);
|
||||
|
||||
expect(selectedServerNames()).toEqual(['visible']);
|
||||
expect(result?.tools).toContain('tool_mcp_visible');
|
||||
expect(result?.tools).not.toContain('tool_mcp_hidden');
|
||||
});
|
||||
|
||||
test('drops a server the user only reaches through an agent', async () => {
|
||||
const result = await loadEphemeral(
|
||||
{ mcpConfig: {} },
|
||||
{
|
||||
ephemeralAgent: { mcp: ['visible', 'agent-only'] },
|
||||
},
|
||||
);
|
||||
|
||||
expect(selectedServerNames()).toEqual(['visible']);
|
||||
expect(result?.tools).not.toContain('tool_mcp_agent-only');
|
||||
});
|
||||
|
||||
test('resolves the accessible catalog with the requesting user role', async () => {
|
||||
await loadEphemeral({ mcpConfig: {} }, { ephemeralAgent: { mcp: ['visible'] } });
|
||||
expect(mockGetAccessibleMCPServers).toHaveBeenCalledWith('user123', 'USER');
|
||||
});
|
||||
|
||||
test('keeps a spec-pinned server even when the chat menu hides it', async () => {
|
||||
const result = await loadEphemeral(
|
||||
{
|
||||
mcpConfig: {},
|
||||
modelSpecs: {
|
||||
list: [
|
||||
{
|
||||
name: 'pins-hidden',
|
||||
label: 'Pins Hidden',
|
||||
preset: { endpoint: 'openai', model: 'gpt-4' },
|
||||
mcpServers: ['hidden'],
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
{ ephemeralAgent: { mcp: [] } },
|
||||
'pins-hidden',
|
||||
);
|
||||
|
||||
expect(selectedServerNames()).toEqual(['hidden']);
|
||||
expect(result?.tools).toContain('tool_mcp_hidden');
|
||||
});
|
||||
|
||||
test('keeps a request-tier server the registry cannot resolve', async () => {
|
||||
const result = await loadEphemeral(
|
||||
{ mcpConfig: {} },
|
||||
{
|
||||
ephemeralAgent: { mcp: ['body-scoped'] },
|
||||
},
|
||||
);
|
||||
|
||||
expect(selectedServerNames()).toEqual(['body-scoped']);
|
||||
expect(result?.tools).toContain('tool_mcp_body-scoped');
|
||||
});
|
||||
|
||||
test('keeps the selection when the catalog lookup fails', async () => {
|
||||
mockGetAccessibleMCPServers.mockRejectedValue(new Error('registry unavailable'));
|
||||
|
||||
const result = await loadEphemeral(
|
||||
{ mcpConfig: {} },
|
||||
{
|
||||
ephemeralAgent: { mcp: ['flaky'] },
|
||||
},
|
||||
);
|
||||
|
||||
expect(selectedServerNames()).toEqual(['flaky']);
|
||||
expect(result?.tools).toContain('tool_mcp_flaky');
|
||||
});
|
||||
|
||||
test('publishes the servers actually in play back onto the request body', async () => {
|
||||
const body = { ephemeralAgent: { mcp: ['visible', 'hidden'] } };
|
||||
|
||||
await loadEphemeral(
|
||||
{
|
||||
mcpConfig: {},
|
||||
modelSpecs: {
|
||||
list: [
|
||||
{
|
||||
name: 'pins-hidden',
|
||||
label: 'Pins Hidden',
|
||||
preset: { endpoint: 'openai', model: 'gpt-4' },
|
||||
mcpServers: ['hidden'],
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
body,
|
||||
'pins-hidden',
|
||||
);
|
||||
|
||||
/** `applyContextToAgent` reads this straight off the body and prefers it
|
||||
* over the agent's tools when loading `serverInstructions`: the hidden
|
||||
* pick is gone, the spec's pin stays. */
|
||||
expect(body.ephemeralAgent.mcp).toEqual(['visible', 'hidden']);
|
||||
});
|
||||
});
|
||||
|
||||
test('addresses cached tools with a non-ephemeral request overlay', async () => {
|
||||
const { EPHEMERAL_AGENT_ID } = Constants;
|
||||
const overlayConfig = {
|
||||
|
|
|
|||
|
|
@ -11,7 +11,11 @@ import {
|
|||
import type { Agent, AgentToolOptions, TConversation, TModelSpec } from 'librechat-data-provider';
|
||||
import type { AppConfig } from '@librechat/data-schemas';
|
||||
import type { ParsedServerConfig } from '~/mcp/types';
|
||||
import { requiresEphemeralUserConnection, validateMCPServerConfig } from '~/mcp/utils';
|
||||
import {
|
||||
requiresEphemeralUserConnection,
|
||||
filterChatSelectableMCPServers,
|
||||
validateMCPServerConfig,
|
||||
} from '~/mcp/utils';
|
||||
import { ASK_USER_QUESTION_TOOL_NAME } from '~/agents/hitl/askUserQuestionTool';
|
||||
import { synthesizeBackgroundToolOptions } from '~/agents/background';
|
||||
import { mergeSynthesizedToolOptions } from '~/agents/selection';
|
||||
|
|
@ -57,10 +61,17 @@ export interface LoadAddedAgentDeps {
|
|||
serverName: string,
|
||||
serverConfig?: ParsedServerConfig,
|
||||
) => Promise<Record<string, unknown> | null>;
|
||||
/** The MCP servers this user can reach, with the registry's tier precedence
|
||||
* already applied — the resolution behind the client's catalog. Omitted, the
|
||||
* chat selection is used as sent. */
|
||||
getAccessibleMCPServers?: (
|
||||
userId: string,
|
||||
role?: string,
|
||||
) => Promise<Record<string, ParsedServerConfig>>;
|
||||
}
|
||||
|
||||
interface LoadAddedAgentParams {
|
||||
req: { user?: { id?: string }; config?: Record<string, unknown> };
|
||||
req: { user?: { id?: string; role?: string }; config?: Record<string, unknown> };
|
||||
conversation: TConversation | null;
|
||||
primaryAgent?: Agent | null;
|
||||
}
|
||||
|
|
@ -182,8 +193,16 @@ export async function loadAddedAgent(
|
|||
return result as unknown as Agent;
|
||||
}
|
||||
|
||||
const mcpServers = new Set<string>(ephemeralAgent?.mcp);
|
||||
const userId = req.user?.id ?? '';
|
||||
/** Narrowed like the primary ephemeral loader: picker selection only, spec
|
||||
* servers added below. */
|
||||
const mcpServers = new Set<string>(
|
||||
await filterChatSelectableMCPServers(ephemeralAgent?.mcp, {
|
||||
userId,
|
||||
role: req.user?.role,
|
||||
getAccessibleMCPServers: deps.getAccessibleMCPServers,
|
||||
}),
|
||||
);
|
||||
|
||||
const modelSpecs = (appConfig?.modelSpecs as { list?: TModelSpec[] })?.list;
|
||||
let modelSpec: (typeof modelSpecs extends Array<infer T> | undefined ? T : never) | null = null;
|
||||
|
|
|
|||
|
|
@ -16,7 +16,11 @@ import type {
|
|||
} from 'librechat-data-provider';
|
||||
import type { AppConfig } from '@librechat/data-schemas';
|
||||
import type { ParsedServerConfig } from '~/mcp/types';
|
||||
import { requiresEphemeralUserConnection, validateMCPServerConfig } from '~/mcp/utils';
|
||||
import {
|
||||
requiresEphemeralUserConnection,
|
||||
filterChatSelectableMCPServers,
|
||||
validateMCPServerConfig,
|
||||
} from '~/mcp/utils';
|
||||
import { ASK_USER_QUESTION_TOOL_NAME } from '~/agents/hitl/askUserQuestionTool';
|
||||
import { synthesizeBackgroundToolOptions } from '~/agents/background';
|
||||
import { mergeSynthesizedToolOptions } from '~/agents/selection';
|
||||
|
|
@ -33,11 +37,18 @@ export interface LoadAgentDeps {
|
|||
serverName: string,
|
||||
serverConfig?: ParsedServerConfig,
|
||||
) => Promise<Record<string, unknown> | null>;
|
||||
/** The MCP servers this user can reach, with the registry's tier precedence
|
||||
* already applied — the resolution behind the client's catalog. Omitted, the
|
||||
* chat selection is used as sent. */
|
||||
getAccessibleMCPServers?: (
|
||||
userId: string,
|
||||
role?: string,
|
||||
) => Promise<Record<string, ParsedServerConfig>>;
|
||||
}
|
||||
|
||||
export interface LoadAgentParams {
|
||||
req: {
|
||||
user?: { id?: string };
|
||||
user?: { id?: string; role?: string };
|
||||
config?: AppConfig;
|
||||
body?: {
|
||||
promptPrefix?: string;
|
||||
|
|
@ -64,13 +75,29 @@ export async function loadEphemeralAgent(
|
|||
modelSpec = modelSpecs?.list?.find((s) => s.name === spec) ?? null;
|
||||
}
|
||||
const ephemeralAgent: TEphemeralAgent | undefined = req.body?.ephemeralAgent;
|
||||
const mcpServers = new Set<string>(ephemeralAgent?.mcp);
|
||||
const userId = req.user?.id ?? '';
|
||||
/** The picker's own selection is narrowed to what the picker may offer; a
|
||||
* spec's servers are the operator's choice and are added after, so pinning a
|
||||
* chat-hidden server to a spec keeps working. */
|
||||
const mcpServers = new Set<string>(
|
||||
await filterChatSelectableMCPServers(ephemeralAgent?.mcp, {
|
||||
userId,
|
||||
role: req.user?.role,
|
||||
getAccessibleMCPServers: deps.getAccessibleMCPServers,
|
||||
}),
|
||||
);
|
||||
if (modelSpec?.mcpServers) {
|
||||
for (const mcpServer of modelSpec.mcpServers) {
|
||||
mcpServers.add(mcpServer);
|
||||
}
|
||||
}
|
||||
/** Publish the servers this request will actually use back onto the body. The
|
||||
* instruction path reads `req.body.ephemeralAgent.mcp` directly and prefers
|
||||
* it over the agent's tools, so it would otherwise both inject a hidden
|
||||
* server's `serverInstructions` and omit a spec-pinned server's. */
|
||||
if (ephemeralAgent != null && Array.isArray(ephemeralAgent.mcp)) {
|
||||
ephemeralAgent.mcp = [...mcpServers];
|
||||
}
|
||||
const tools: string[] = [];
|
||||
if (ephemeralAgent?.execute_code === true || modelSpec?.executeCode === true) {
|
||||
tools.push(Tools.execute_code);
|
||||
|
|
|
|||
|
|
@ -20,6 +20,8 @@ import {
|
|||
isUserSourced,
|
||||
validateMCPServerConfig,
|
||||
requiresEphemeralUserConnection,
|
||||
isChatSelectableMCPServer,
|
||||
filterChatSelectableMCPServers,
|
||||
} from '~/mcp/utils';
|
||||
|
||||
describe('normalizeServerName', () => {
|
||||
|
|
@ -1092,3 +1094,138 @@ describe('getMissingCustomUserVars', () => {
|
|||
).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('isChatSelectableMCPServer', () => {
|
||||
it('rejects a server hidden from the chat menu', () => {
|
||||
expect(isChatSelectableMCPServer({ chatMenu: false })).toBe(false);
|
||||
});
|
||||
|
||||
it('rejects a server reachable only through an agent', () => {
|
||||
expect(isChatSelectableMCPServer({ consumeOnly: true })).toBe(false);
|
||||
});
|
||||
|
||||
it('accepts a server with the flags unset or explicitly on', () => {
|
||||
expect(isChatSelectableMCPServer({})).toBe(true);
|
||||
expect(isChatSelectableMCPServer({ chatMenu: true, consumeOnly: false })).toBe(true);
|
||||
});
|
||||
|
||||
it('accepts an unresolved config so request-tier servers are not dropped', () => {
|
||||
expect(isChatSelectableMCPServer(undefined)).toBe(true);
|
||||
expect(isChatSelectableMCPServer(null)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('filterChatSelectableMCPServers', () => {
|
||||
const catalog = {
|
||||
visible: { chatMenu: true },
|
||||
hidden: { chatMenu: false },
|
||||
unset: {},
|
||||
'agent-only': { consumeOnly: true },
|
||||
'private server': { chatMenu: false },
|
||||
};
|
||||
const accessible = jest.fn(async () => catalog);
|
||||
|
||||
beforeEach(() => accessible.mockClear());
|
||||
|
||||
it('drops servers hidden from the chat menu', async () => {
|
||||
await expect(
|
||||
filterChatSelectableMCPServers(['visible', 'hidden'], {
|
||||
userId: 'user123',
|
||||
getAccessibleMCPServers: accessible,
|
||||
}),
|
||||
).resolves.toEqual(['visible']);
|
||||
});
|
||||
|
||||
it('drops servers the user only reaches through an agent', async () => {
|
||||
await expect(
|
||||
filterChatSelectableMCPServers(['visible', 'agent-only'], {
|
||||
userId: 'user123',
|
||||
getAccessibleMCPServers: accessible,
|
||||
}),
|
||||
).resolves.toEqual(['visible']);
|
||||
});
|
||||
|
||||
it('keeps servers with the flags unset', async () => {
|
||||
await expect(
|
||||
filterChatSelectableMCPServers(['unset'], {
|
||||
userId: 'user123',
|
||||
getAccessibleMCPServers: accessible,
|
||||
}),
|
||||
).resolves.toEqual(['unset']);
|
||||
});
|
||||
|
||||
it('resolves the catalog once, however long the selection', async () => {
|
||||
await expect(
|
||||
filterChatSelectableMCPServers(['visible', 'visible', 'unset', 'visible'], {
|
||||
userId: 'user123',
|
||||
getAccessibleMCPServers: accessible,
|
||||
}),
|
||||
).resolves.toEqual(['visible', 'unset']);
|
||||
expect(accessible).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('passes the role through, since it scopes what the user can reach', async () => {
|
||||
await filterChatSelectableMCPServers(['visible'], {
|
||||
userId: 'user123',
|
||||
role: 'ADMIN',
|
||||
getAccessibleMCPServers: accessible,
|
||||
});
|
||||
expect(accessible).toHaveBeenCalledWith('user123', 'ADMIN');
|
||||
});
|
||||
|
||||
it('drops a hidden server named by its normalized spelling', async () => {
|
||||
await expect(
|
||||
filterChatSelectableMCPServers(['private_server'], {
|
||||
userId: 'user123',
|
||||
getAccessibleMCPServers: accessible,
|
||||
}),
|
||||
).resolves.toEqual([]);
|
||||
});
|
||||
|
||||
it('lets an exact accessible name win over another server normalizing onto it', async () => {
|
||||
const crossTier = jest.fn(async () => ({
|
||||
'private server': { chatMenu: false },
|
||||
private_server: { chatMenu: true },
|
||||
}));
|
||||
await expect(
|
||||
filterChatSelectableMCPServers(['private_server'], {
|
||||
userId: 'user123',
|
||||
getAccessibleMCPServers: crossTier,
|
||||
}),
|
||||
).resolves.toEqual(['private_server']);
|
||||
});
|
||||
|
||||
it('keeps a request-tier server the registry does not know', async () => {
|
||||
await expect(
|
||||
filterChatSelectableMCPServers(['body-scoped'], {
|
||||
userId: 'user123',
|
||||
getAccessibleMCPServers: accessible,
|
||||
}),
|
||||
).resolves.toEqual(['body-scoped']);
|
||||
});
|
||||
|
||||
it('keeps the selection when the catalog lookup fails', async () => {
|
||||
const failing = jest.fn(async () => {
|
||||
throw new Error('registry unavailable');
|
||||
});
|
||||
await expect(
|
||||
filterChatSelectableMCPServers(['a', 'b'], {
|
||||
userId: 'user123',
|
||||
getAccessibleMCPServers: failing,
|
||||
}),
|
||||
).resolves.toEqual(['a', 'b']);
|
||||
});
|
||||
|
||||
it('uses the selection as sent when no resolver is supplied', async () => {
|
||||
await expect(
|
||||
filterChatSelectableMCPServers(['a', 'a', 'b'], { userId: 'user123' }),
|
||||
).resolves.toEqual(['a', 'b']);
|
||||
});
|
||||
|
||||
it('returns an empty array for an empty or missing selection', async () => {
|
||||
const opts = { userId: 'u', getAccessibleMCPServers: accessible };
|
||||
await expect(filterChatSelectableMCPServers([], opts)).resolves.toEqual([]);
|
||||
await expect(filterChatSelectableMCPServers(undefined, opts)).resolves.toEqual([]);
|
||||
await expect(filterChatSelectableMCPServers(null, opts)).resolves.toEqual([]);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
import { logger } from '@librechat/data-schemas';
|
||||
import {
|
||||
Constants,
|
||||
MCPOptionsSchema,
|
||||
|
|
@ -403,6 +404,98 @@ export function requiresEphemeralUserConnection(config: UserScopedConnectionConf
|
|||
return getMCPRequestScope(config).requestScoped;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a resolved server config may be reached from the chat MCP picker.
|
||||
*
|
||||
* Mirrors `selectableServers` in the client's `useMCPServerManager`, which is
|
||||
* the list the dropdown offers: `chatMenu: false` is the operator hiding a
|
||||
* server from chat, and `consumeOnly` marks a server the user reaches only
|
||||
* through an agent that references it, never on its own.
|
||||
*
|
||||
* An unresolved config is selectable. A name the registry cannot resolve is
|
||||
* either request-tier (declared on the request body and never registered) or
|
||||
* genuinely unknown, and both already have their own handling downstream —
|
||||
* failing closed here would silently drop request-scoped servers instead.
|
||||
*/
|
||||
export function isChatSelectableMCPServer(
|
||||
config?: Pick<ParsedServerConfig, 'chatMenu' | 'consumeOnly'> | null,
|
||||
): boolean {
|
||||
if (config == null) {
|
||||
return true;
|
||||
}
|
||||
return config.chatMenu !== false && config.consumeOnly !== true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Narrows a chat picker selection to the servers that picker is allowed to
|
||||
* offer, so a stale client, a replayed body, or a hand-written request cannot
|
||||
* reach a server the menu hides.
|
||||
*
|
||||
* Pass only the picker's own selection. Servers a model spec pins
|
||||
* (`modelSpec.mcpServers`) are the operator's choice and stay attached even
|
||||
* when hidden, so they must be added after this call, not through it.
|
||||
*
|
||||
* The accessible set comes from the registry — the same resolution that feeds
|
||||
* the client's catalog, with its own tier precedence already applied — rather
|
||||
* than being re-derived from the request's config overlay. A user-tier or
|
||||
* process-backed server outranks a config entry of the same name, and this must
|
||||
* agree with whatever the picker was actually offered.
|
||||
*
|
||||
* Names are deduplicated before the lookup, since the selection is
|
||||
* request-supplied, and are matched through the same normalized-name aliasing
|
||||
* that tool loading uses, built over the whole accessible set so an exact name
|
||||
* always wins over another server's normalized form. A name the registry does
|
||||
* not know is request-tier — declared on the body, never registered — and is
|
||||
* kept, as is every name if the lookup fails: this narrows an
|
||||
* already-authenticated selection and is not the authorization boundary.
|
||||
*
|
||||
* Returns the names as they were sent, so callers keep addressing servers the
|
||||
* way the rest of the request does.
|
||||
*/
|
||||
export async function filterChatSelectableMCPServers(
|
||||
selectedServers: string[] | null | undefined,
|
||||
{
|
||||
userId,
|
||||
role,
|
||||
getAccessibleMCPServers,
|
||||
}: {
|
||||
userId: string;
|
||||
role?: string;
|
||||
getAccessibleMCPServers?: (
|
||||
userId: string,
|
||||
role?: string,
|
||||
) => Promise<Record<string, Pick<ParsedServerConfig, 'chatMenu' | 'consumeOnly'>>>;
|
||||
},
|
||||
): Promise<string[]> {
|
||||
if (!Array.isArray(selectedServers) || selectedServers.length === 0) {
|
||||
return [];
|
||||
}
|
||||
const uniqueServers = [...new Set(selectedServers)];
|
||||
if (getAccessibleMCPServers == null) {
|
||||
return uniqueServers;
|
||||
}
|
||||
|
||||
let accessible: Record<string, Pick<ParsedServerConfig, 'chatMenu' | 'consumeOnly'>>;
|
||||
try {
|
||||
accessible = await getAccessibleMCPServers(userId, role);
|
||||
} catch (error) {
|
||||
logger.warn('[MCP] Could not resolve accessible servers; keeping the chat selection', error);
|
||||
return uniqueServers;
|
||||
}
|
||||
|
||||
const accessibleNames = Object.keys(accessible ?? {});
|
||||
if (accessibleNames.length === 0) {
|
||||
return uniqueServers;
|
||||
}
|
||||
const aliases = buildServerNameAliases(accessibleNames);
|
||||
|
||||
return uniqueServers.filter((serverName) => {
|
||||
const resolved =
|
||||
accessible[serverName] ?? accessible[aliases.get(normalizeServerName(serverName)) ?? ''];
|
||||
return isChatSelectableMCPServer(resolved);
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns true when a server requires a per-user connection instead of an
|
||||
* app-shared connection.
|
||||
|
|
|
|||
|
|
@ -178,7 +178,14 @@ const BaseOptionsSchema = z.object({
|
|||
/** Timeout (ms) for the long-lived SSE GET stream body before undici aborts it. Default: 300_000 (5 min). */
|
||||
sseReadTimeout: z.number().int().positive().optional(),
|
||||
initTimeout: z.number().int().nonnegative().optional(),
|
||||
/** Controls visibility in chat dropdown menu (MCPSelect) */
|
||||
/**
|
||||
* Whether the server is offered in chat.
|
||||
*
|
||||
* `false` hides it from the chat dropdown (MCPSelect) AND bars it from the
|
||||
* chat selection a request carries, so a stale or hand-written request cannot
|
||||
* reach it either. It does not restrict agents, nor a server a model spec
|
||||
* pins through `mcpServers` — both are the operator's own choice.
|
||||
*/
|
||||
chatMenu: z.boolean().optional(),
|
||||
/**
|
||||
* Controls server instruction behavior:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue