diff --git a/api/server/controllers/agents/client.js b/api/server/controllers/agents/client.js index dbdf5651b54..606d38a24c1 100644 --- a/api/server/controllers/agents/client.js +++ b/api/server/controllers/agents/client.js @@ -725,6 +725,7 @@ class AgentClient extends BaseClient { updateFilesUsage: db.updateFilesUsage, getUserKeyValues: db.getUserKeyValues, getToolFilesByIds: db.getToolFilesByIds, + getRoleByName: db.getRoleByName, getCodeGeneratedFiles: db.getCodeGeneratedFiles, filterFilesByAgentAccess, }, diff --git a/api/server/controllers/agents/openai.js b/api/server/controllers/agents/openai.js index 6c761540d8a..10f1c397797 100644 --- a/api/server/controllers/agents/openai.js +++ b/api/server/controllers/agents/openai.js @@ -262,6 +262,7 @@ const OpenAIChatCompletionController = async (req, res) => { getUserKeyValues: db.getUserKeyValues, getUserCodeFiles: db.getUserCodeFiles, getToolFilesByIds: db.getToolFilesByIds, + getRoleByName: db.getRoleByName, getCodeGeneratedFiles: db.getCodeGeneratedFiles, listSkillsByAccess: skillDbMethods.listSkillsByAccess, listAlwaysApplySkills: skillDbMethods.listAlwaysApplySkills, diff --git a/api/server/controllers/agents/responses.js b/api/server/controllers/agents/responses.js index 48f6b92c9ec..d5ff0557fd5 100644 --- a/api/server/controllers/agents/responses.js +++ b/api/server/controllers/agents/responses.js @@ -384,6 +384,7 @@ const createResponse = async (req, res) => { getUserKeyValues: db.getUserKeyValues, getUserCodeFiles: db.getUserCodeFiles, getToolFilesByIds: db.getToolFilesByIds, + getRoleByName: db.getRoleByName, getCodeGeneratedFiles: db.getCodeGeneratedFiles, listSkillsByAccess: skillDbMethods.listSkillsByAccess, listAlwaysApplySkills: skillDbMethods.listAlwaysApplySkills, diff --git a/api/server/services/Endpoints/agents/addedConvo.js b/api/server/services/Endpoints/agents/addedConvo.js index 847c6c01af6..52c06cb8582 100644 --- a/api/server/services/Endpoints/agents/addedConvo.js +++ b/api/server/services/Endpoints/agents/addedConvo.js @@ -185,6 +185,7 @@ const processAddedConvo = async ({ getUserCodeFiles: db.getUserCodeFiles, getUserKeyValues: db.getUserKeyValues, getToolFilesByIds: db.getToolFilesByIds, + getRoleByName: db.getRoleByName, getCodeGeneratedFiles: db.getCodeGeneratedFiles, filterFilesByAgentAccess, listSkillsByAccess: db.listSkillsByAccess, diff --git a/api/server/services/Endpoints/agents/initialize.js b/api/server/services/Endpoints/agents/initialize.js index 9caee651c41..1de14d1493c 100644 --- a/api/server/services/Endpoints/agents/initialize.js +++ b/api/server/services/Endpoints/agents/initialize.js @@ -396,6 +396,7 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { getUserKeyValues: db.getUserKeyValues, getUserCodeFiles: db.getUserCodeFiles, getToolFilesByIds: db.getToolFilesByIds, + getRoleByName: db.getRoleByName, getCodeGeneratedFiles: db.getCodeGeneratedFiles, filterFilesByAgentAccess, listSkillsByAccess: skillDbMethods.listSkillsByAccess, @@ -472,6 +473,7 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { getUserKeyValues: db.getUserKeyValues, getUserCodeFiles: db.getUserCodeFiles, getToolFilesByIds: db.getToolFilesByIds, + getRoleByName: db.getRoleByName, getCodeGeneratedFiles: db.getCodeGeneratedFiles, filterFilesByAgentAccess, listSkillsByAccess: skillDbMethods.listSkillsByAccess, @@ -679,6 +681,7 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { getUserKeyValues: db.getUserKeyValues, getUserCodeFiles: db.getUserCodeFiles, getToolFilesByIds: db.getToolFilesByIds, + getRoleByName: db.getRoleByName, getCodeGeneratedFiles: db.getCodeGeneratedFiles, filterFilesByAgentAccess, listSkillsByAccess: skillDbMethods.listSkillsByAccess, diff --git a/api/server/services/ToolService.js b/api/server/services/ToolService.js index 2c497a960c8..edba7b67d81 100644 --- a/api/server/services/ToolService.js +++ b/api/server/services/ToolService.js @@ -28,14 +28,12 @@ const { buildMCPAuthRunStepDeltaEvent, buildMCPAuthRunStepCompletedEvent, isFileAuthoringToolDefinition, - checkAccessWithRequestCache, + checkWebSearchGrant, } = require('@librechat/api'); const { Time, Tools, Constants, - Permissions, - PermissionTypes, CacheKeys, ErrorTypes, ContentTypes, @@ -77,33 +75,10 @@ const { recordUsage } = require('~/server/services/Threads'); const { loadTools } = require('~/app/clients/tools/util'); const { findPluginAuthsByKeys, getRoleByName } = require('~/models'); -/** - * Whether the requesting user's role grants `WEB_SEARCH.USE`. - * - * `web_search` is reachable both as an `agent.tools` entry and, on some - * providers, as a model parameter, so the capability switch alone does not - * authorize it — the role has to grant it too. Resolved once per filter pass - * because the tool filters are synchronous. - * - * Fails closed: a missing user or a lookup that throws denies the tool. - */ -const canUseWebSearch = async (req) => { - try { - return await checkAccessWithRequestCache({ - req, - user: req?.user, - permissionType: PermissionTypes.WEB_SEARCH, - permissions: [Permissions.USE], - getRoleByName, - }); - } catch (error) { - logger.error( - `[loadAgentTools][User: ${req?.user?.id}] Failed ${PermissionTypes.WEB_SEARCH} permission check`, - error, - ); - return false; - } -}; +/** Role half of the web-search gate. The tool filters below are synchronous, so + * the grant is resolved once per pass and consulted inline. */ +const canUseWebSearch = (req) => + checkWebSearchGrant({ req, user: req?.user, getRoleByName, context: 'loadAgentTools' }); const { getFlowStateManager, getMCPServersRegistry } = require('~/config'); const { getLogStores } = require('~/cache'); diff --git a/client/src/components/SidePanel/Agents/ModelPanel.test.tsx b/client/src/components/SidePanel/Agents/ModelPanel.test.tsx index 4f748dec382..37f32b738cd 100644 --- a/client/src/components/SidePanel/Agents/ModelPanel.test.tsx +++ b/client/src/components/SidePanel/Agents/ModelPanel.test.tsx @@ -2,8 +2,8 @@ * @jest-environment jsdom */ import React from 'react'; -import { FormProvider, useForm } from 'react-hook-form'; import { render } from '@testing-library/react'; +import { FormProvider, useForm } from 'react-hook-form'; import type { TEndpointsConfig } from 'librechat-data-provider'; import type { AgentForm } from '~/common'; import ModelPanel from './ModelPanel'; @@ -35,7 +35,12 @@ jest.mock('@librechat/client', () => ({ })); jest.mock('~/components/SidePanel/Parameters/components', () => ({ - componentMapping: {}, + componentMapping: { + /** Renders the setting key so a test can assert which parameters the panel + * offered. The other suites pass `componentMapping: {}` semantics through + * `settingKey`-free assertions, so this stays inert for them. */ + switch: ({ settingKey }: { settingKey: string }) =>
, + }, })); jest.mock('~/data-provider', () => ({ @@ -46,8 +51,10 @@ jest.mock('~/Providers', () => ({ useLiveAnnouncer: () => ({ announcePolite: jest.fn() }), })); +const mockHasWebSearchAccess = jest.fn(() => true); jest.mock('~/hooks', () => ({ useLocalize: () => (key: string) => key, + useHasAccess: () => mockHasWebSearchAccess(), })); jest.mock('~/utils', () => ({ @@ -93,3 +100,47 @@ describe('ModelPanel model labels', () => { expect(getByTestId('com_ui_model-custom-model')).toHaveTextContent('Custom Model'); }); }); + +function WebSearchTestForm() { + const methods = useForm({ + defaultValues: { + provider: 'openAI', + model: 'gpt-4o', + model_parameters: {}, + }, + }); + + return ( + + + + ); +} + +describe('ModelPanel web search parameter gating', () => { + beforeEach(() => { + localStorage.clear(); + mockEndpointsConfig = {}; + }); + + afterEach(() => { + mockHasWebSearchAccess.mockReturnValue(true); + }); + + /** `web_search` is a model parameter as well as a tool, so the builder offering + * the switch to a role the server refuses is the same gap one panel over. */ + it('renders the web_search switch when the role grants WEB_SEARCH', () => { + const { queryAllByTestId } = render(); + expect(queryAllByTestId('param-web_search').length).toBeGreaterThan(0); + }); + + it('hides the web_search switch when the role denies WEB_SEARCH', () => { + mockHasWebSearchAccess.mockReturnValue(false); + const { queryAllByTestId } = render(); + expect(queryAllByTestId('param-web_search')).toHaveLength(0); + }); +}); diff --git a/client/src/components/SidePanel/Agents/ModelPanel.tsx b/client/src/components/SidePanel/Agents/ModelPanel.tsx index 2a571c75010..c8d700a411e 100644 --- a/client/src/components/SidePanel/Agents/ModelPanel.tsx +++ b/client/src/components/SidePanel/Agents/ModelPanel.tsx @@ -3,10 +3,11 @@ import keyBy from 'lodash/keyBy'; import { ControlCombobox } from '@librechat/client'; import { ChevronLeft, RotateCcw } from 'lucide-react'; import { useFormContext, useWatch, Controller } from 'react-hook-form'; -import { componentMapping } from '~/components/SidePanel/Parameters/components'; import { + Permissions, alternateName, getSettingsKeys, + PermissionTypes, getEndpointField, LocalStorageKeys, SettingDefinition, @@ -15,11 +16,12 @@ import { } from 'librechat-data-provider'; import type * as t from 'librechat-data-provider'; import type { AgentForm, AgentModelPanelProps, StringOption } from '~/common'; +import { componentMapping } from '~/components/SidePanel/Parameters/components'; import { useGetEndpointsQuery } from '~/data-provider'; +import { useLocalize, useHasAccess } from '~/hooks'; import { useLiveAnnouncer } from '~/Providers'; -import { useLocalize } from '~/hooks'; -import { Panel } from '~/common'; import { cn, getModelLabel } from '~/utils'; +import { Panel } from '~/common'; export default function ModelPanel({ providers, @@ -85,6 +87,13 @@ export default function ModelPanel({ [provider, endpointsConfig], ); + /** `web_search` is a model parameter as well as a tool, so the role has to gate + * it here too — otherwise the builder offers a switch the server will refuse. */ + const hasWebSearchAccess = useHasAccess({ + permissionType: PermissionTypes.WEB_SEARCH, + permission: Permissions.USE, + }); + const parameters = useMemo((): SettingDefinition[] => { const customParams = endpointsConfig[provider]?.customParams ?? {}; const [combinedKey, endpointKey] = getSettingsKeys(endpointType ?? provider, model ?? ''); @@ -98,10 +107,10 @@ export default function ModelPanel({ overriddenEndpointKey, model ?? '', ); - return modelAwareParams.map( - (param) => (overriddenParamsMap[param.key] as SettingDefinition) ?? param, - ); - }, [endpointType, endpointsConfig, model, provider]); + return modelAwareParams + .filter((param) => param.key !== 'web_search' || hasWebSearchAccess) + .map((param) => (overriddenParamsMap[param.key] as SettingDefinition) ?? param); + }, [endpointType, endpointsConfig, model, provider, hasWebSearchAccess]); const setOption = (optionKey: keyof t.AgentModelParameters) => (value: t.AgentParameterValue) => { setValue(`model_parameters.${optionKey}`, value); diff --git a/packages/api/src/agents/__tests__/initialize.test.ts b/packages/api/src/agents/__tests__/initialize.test.ts index 61e1a8afb25..14c3a622d37 100644 --- a/packages/api/src/agents/__tests__/initialize.test.ts +++ b/packages/api/src/agents/__tests__/initialize.test.ts @@ -2223,3 +2223,100 @@ describe('initializeAgent — run-scoped MCP tool definitions', () => { expect(result.mcpAvailableTools).toEqual(mcpAvailableTools); }); }); + +describe('initializeAgent — model_parameters.web_search role gate', () => { + const buildRole = (webSearchUse?: boolean) => ({ + name: 'USER', + permissions: { WEB_SEARCH: { USE: webSearchUse } }, + }); + + const gatedReq = () => + ({ user: { id: 'user-1', role: 'USER' }, config: {} }) as unknown as ServerRequest; + + /** `result.model_parameters` reflects the mocked `llmConfig`, so the only view of + * what `initializeAgent` actually handed the provider is `getOptions`' argument. */ + function modelParametersSentToProvider(): Record { + const results = mockGetProviderConfig.mock.results; + const { getOptions } = results[results.length - 1].value as { getOptions: jest.Mock }; + const [{ model_parameters }] = getOptions.mock.calls[0]; + return model_parameters; + } + + const run = async ( + modelOptions: Record, + role: ReturnType | null, + ) => { + const { agent, res, loadTools, db } = createMocks(); + mockExtractLibreChatParams.mockReturnValueOnce({ + resendFiles: false, + maxContextTokens: undefined, + modelOptions: { model: agent.model, ...modelOptions }, + }); + const getRoleByName = jest.fn().mockResolvedValue(role); + await initializeAgent( + { + req: gatedReq(), + res, + agent, + loadTools, + endpointOption: { endpoint: EModelEndpoint.agents }, + allowedProviders: new Set([agent.provider]), + isInitialAgent: true, + }, + { ...db, getRoleByName } as unknown as InitializeAgentDbMethods, + ); + return { getRoleByName }; + }; + + it('disables an explicitly enabled web_search when the role denies it', async () => { + await run({ web_search: true }, buildRole(false)); + expect(modelParametersSentToProvider()).toEqual(expect.objectContaining({ web_search: false })); + }); + + it('keeps an explicitly enabled web_search when the role grants it', async () => { + await run({ web_search: true }, buildRole(true)); + expect(modelParametersSentToProvider()).toEqual(expect.objectContaining({ web_search: true })); + }); + + /** An absent value is what an endpoint `defaultParams.web_search` fills in, so a + * denied role has to be pinned to `false` rather than left undefined. */ + it('pins web_search false for a denied role when the agent sets no value', async () => { + await run({}, buildRole(false)); + expect(modelParametersSentToProvider()).toEqual(expect.objectContaining({ web_search: false })); + }); + + it('leaves web_search unset for a permitted role when the agent sets no value', async () => { + await run({}, buildRole(true)); + expect(modelParametersSentToProvider()).not.toHaveProperty('web_search'); + }); + + it('skips the role read when the agent explicitly disabled web search', async () => { + const { getRoleByName } = await run({ web_search: false }, buildRole(true)); + expect(getRoleByName).not.toHaveBeenCalled(); + }); + + it('fails closed when the role lookup throws', async () => { + const { agent, res, loadTools, db } = createMocks(); + mockExtractLibreChatParams.mockReturnValueOnce({ + resendFiles: false, + maxContextTokens: undefined, + modelOptions: { model: agent.model, web_search: true }, + }); + await initializeAgent( + { + req: gatedReq(), + res, + agent, + loadTools, + endpointOption: { endpoint: EModelEndpoint.agents }, + allowedProviders: new Set([agent.provider]), + isInitialAgent: true, + }, + { + ...db, + getRoleByName: jest.fn().mockRejectedValue(new Error('mongo down')), + } as unknown as InitializeAgentDbMethods, + ); + expect(modelParametersSentToProvider()).toEqual(expect.objectContaining({ web_search: false })); + }); +}); diff --git a/packages/api/src/agents/initialize.ts b/packages/api/src/agents/initialize.ts index f7f99919512..c1d1b15a5a2 100644 --- a/packages/api/src/agents/initialize.ts +++ b/packages/api/src/agents/initialize.ts @@ -22,7 +22,7 @@ import type { } from 'librechat-data-provider'; import type { GenericTool, LCToolRegistry, ToolMap, LCTool } from '@librechat/agents'; import type { IMongoFile, FileOwnerScope } from '@librechat/data-schemas'; -import type { Response as ServerResponse } from 'express'; +import type { Request, Response as ServerResponse } from 'express'; import type { ServerRequest, EndpointDbMethods, @@ -31,6 +31,7 @@ import type { } from '~/types'; import type { LCAvailableTools, RequestScopedMCPConnectionStore } from '../mcp/types'; import type { ResolvedManualSkill, ResolvedAlwaysApplySkill } from './skills'; +import type { CheckAccessParams } from '~/middleware/access'; import type { TFilterFilesByAgentAccess } from './resources'; import { injectSkillCatalog, @@ -50,6 +51,7 @@ import { registerFileAuthoringTools, isFileAuthoringToolDefinition, } from './tools'; +import { checkWebSearchGrant } from '~/tools/rolePermissions'; import { filterFilesByEndpointConfig } from '~/files'; import { generateArtifactsPrompt } from '~/prompts'; import { getProviderConfig } from '~/endpoints'; @@ -542,6 +544,9 @@ export interface InitializeAgentDbMethods extends EndpointDbMethods { has_more?: boolean; after?: string | null; }>; + /** Resolves a role by name for the `WEB_SEARCH` grant. Optional: when absent + * the provider-native web search gate is not applied. */ + getRoleByName?: CheckAccessParams['getRoleByName']; } /** @@ -605,6 +610,32 @@ export async function initializeAgent( _modelOptions as Record, ); + /** `model_parameters.web_search` turns on the provider's own web search and + * never passes through the agent tool loader, so the tool gate does not cover + * it. The agent builder's model-parameters panel writes this field, so it is + * reachable by any user who can edit an agent. + * + * An absent value is resolved too, not just an explicit `true`: the provider + * builders apply their endpoint `defaultParams.web_search` exactly when the + * field is `undefined`, so leaving it unset for a denied role is the state + * that lets the endpoint grant it. An explicit `false` is already off. */ + if (modelOptions.web_search !== false && db?.getRoleByName != null) { + const allowed = await checkWebSearchGrant({ + req: params.req as Request | undefined, + user: params.req?.user, + getRoleByName: db.getRoleByName, + }); + if (!allowed) { + const wasRequested = modelOptions.web_search === true; + modelOptions.web_search = false; + if (wasRequested) { + logger.warn( + `[initializeAgent][User: ${params.req?.user?.id}][Agent: ${agent.id}] Forbidden: role denies WEB_SEARCH; disabled model_parameters.web_search`, + ); + } + } + } + const provider = agent.provider; agent.endpoint = provider; diff --git a/packages/api/src/tools/index.ts b/packages/api/src/tools/index.ts index 8695d06707b..23e1ab86851 100644 --- a/packages/api/src/tools/index.ts +++ b/packages/api/src/tools/index.ts @@ -1,4 +1,5 @@ export * from './format'; +export * from './rolePermissions'; export * from './registry'; export * from './toolkits'; export * from './definitions'; diff --git a/packages/api/src/tools/rolePermissions.spec.ts b/packages/api/src/tools/rolePermissions.spec.ts new file mode 100644 index 00000000000..b9a44a98c68 --- /dev/null +++ b/packages/api/src/tools/rolePermissions.spec.ts @@ -0,0 +1,67 @@ +import { Permissions, PermissionTypes } from 'librechat-data-provider'; +import type { Request as ServerRequest } from 'express'; +import { checkWebSearchGrant } from './rolePermissions'; + +const buildRole = (use?: boolean) => ({ + name: 'USER', + permissions: { [PermissionTypes.WEB_SEARCH]: { [Permissions.USE]: use } }, +}); + +const buildReq = (user: unknown = { id: 'user-1', role: 'USER' }) => + ({ user }) as unknown as ServerRequest; + +describe('checkWebSearchGrant', () => { + it('grants when the role allows WEB_SEARCH.USE', async () => { + const getRoleByName = jest.fn().mockResolvedValue(buildRole(true)); + await expect(checkWebSearchGrant({ req: buildReq(), getRoleByName })).resolves.toBe(true); + expect(getRoleByName).toHaveBeenCalledWith('USER'); + }); + + it('denies when the role withholds WEB_SEARCH.USE', async () => { + const getRoleByName = jest.fn().mockResolvedValue(buildRole(false)); + await expect(checkWebSearchGrant({ req: buildReq(), getRoleByName })).resolves.toBe(false); + }); + + /** A permission block seeded without an explicit `USE` must not read as a grant. */ + it('denies when the permission block is empty', async () => { + const getRoleByName = jest.fn().mockResolvedValue({ name: 'USER', permissions: {} }); + await expect(checkWebSearchGrant({ req: buildReq(), getRoleByName })).resolves.toBe(false); + }); + + it('denies when the role does not exist', async () => { + const getRoleByName = jest.fn().mockResolvedValue(null); + await expect(checkWebSearchGrant({ req: buildReq(), getRoleByName })).resolves.toBe(false); + }); + + it('denies when the request carries no user', async () => { + const getRoleByName = jest.fn().mockResolvedValue(buildRole(true)); + const req = {} as ServerRequest; + await expect(checkWebSearchGrant({ req, getRoleByName })).resolves.toBe(false); + expect(getRoleByName).not.toHaveBeenCalled(); + }); + + it('denies when the user carries no role', async () => { + const getRoleByName = jest.fn().mockResolvedValue(buildRole(true)); + await expect( + checkWebSearchGrant({ req: buildReq({ id: 'user-1' }), getRoleByName }), + ).resolves.toBe(false); + expect(getRoleByName).not.toHaveBeenCalled(); + }); + + /** Fails closed rather than propagating: a role store outage must not hand a + * denied user provider-native web search. */ + it('denies, without throwing, when the role lookup fails', async () => { + const getRoleByName = jest.fn().mockRejectedValue(new Error('mongo down')); + await expect(checkWebSearchGrant({ req: buildReq(), getRoleByName })).resolves.toBe(false); + }); + + it('authorizes an explicit user over req.user', async () => { + const getRoleByName = jest.fn().mockResolvedValue(buildRole(true)); + await checkWebSearchGrant({ + req: buildReq({ id: 'other', role: 'IGNORED' }), + user: { id: 'user-1', role: 'USER' } as never, + getRoleByName, + }); + expect(getRoleByName).toHaveBeenCalledWith('USER'); + }); +}); diff --git a/packages/api/src/tools/rolePermissions.ts b/packages/api/src/tools/rolePermissions.ts new file mode 100644 index 00000000000..ab632d445a6 --- /dev/null +++ b/packages/api/src/tools/rolePermissions.ts @@ -0,0 +1,49 @@ +import { logger } from '@librechat/data-schemas'; +import { Permissions, PermissionTypes } from 'librechat-data-provider'; +import type { Request as ServerRequest } from 'express'; +import type { CheckAccessParams } from '../middleware/access'; +import { checkAccessWithRequestCache } from '../middleware/access'; + +export interface CheckWebSearchGrantParams { + req?: ServerRequest; + user?: CheckAccessParams['user'] | null; + getRoleByName: CheckAccessParams['getRoleByName']; + /** Prefix for the denial log line, e.g. `loadAgentTools`. */ + context?: string; +} + +/** + * Whether the requesting user's role grants `WEB_SEARCH.USE`. + * + * `web_search` reaches a provider by two routes — an `agent.tools` entry, and a + * `model_parameters` field that turns on the provider's own search without + * passing through the tool loader — so both have to consult this rather than the + * deployment capability alone. + * + * Resolved through the per-request permission cache, so repeat checks within one + * request cost no extra role read. Fails closed: a missing user or a lookup that + * throws denies the tool. + */ +export async function checkWebSearchGrant({ + req, + user, + getRoleByName, + context = 'webSearchGrant', +}: CheckWebSearchGrantParams): Promise { + const subject = (user ?? req?.user) as CheckAccessParams['user'] | undefined; + try { + return await checkAccessWithRequestCache({ + req, + user: subject as CheckAccessParams['user'], + permissionType: PermissionTypes.WEB_SEARCH, + permissions: [Permissions.USE], + getRoleByName, + }); + } catch (error) { + logger.error( + `[${context}][User: ${subject?.id}] Failed ${PermissionTypes.WEB_SEARCH} permission check`, + error, + ); + return false; + } +}