mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-09-30 12:52:10 +00:00
💻 fix(agents): require Code Interpreter for programmatic MCP tools (#14977)
* fix(agents): require code interpreter for programmatic MCP tools * test(data-provider): fix tool options fixture type * fix(agents): address programmatic tool review feedback * fix(agents): avoid no-op update on version revert
This commit is contained in:
parent
6daafda86f
commit
da0491d5db
16 changed files with 557 additions and 15 deletions
|
|
@ -37,6 +37,7 @@ const {
|
|||
AgentCapabilities,
|
||||
EModelEndpoint,
|
||||
resolveAllowedStatefulCodeEnvironments,
|
||||
removeCodeExecutionCaller,
|
||||
removeNullishValues,
|
||||
} = require('librechat-data-provider');
|
||||
const {
|
||||
|
|
@ -271,6 +272,12 @@ const isSubagentsCapabilityEnabled = (req) => {
|
|||
return capabilities.includes(AgentCapabilities.subagents);
|
||||
};
|
||||
|
||||
const isCodeInterpreterCapabilityEnabled = (req) => {
|
||||
const capabilities = req.config?.endpoints?.[EModelEndpoint.agents]?.capabilities;
|
||||
if (!Array.isArray(capabilities)) return false;
|
||||
return capabilities.includes(AgentCapabilities.execute_code);
|
||||
};
|
||||
|
||||
/** Reject a newly selected stateful workspace scope that the deployment owner
|
||||
* has excluded. Disabled sessions and unrelated edits remain saveable so an
|
||||
* allowlist tightening never silently rewrites or strands an existing agent. */
|
||||
|
|
@ -533,6 +540,13 @@ const createAgentHandler = async (req, res) => {
|
|||
const validatedData = agentCreateSchema.parse(req.body);
|
||||
const { tools = [], ...agentData } = removeNullishValues(validatedData);
|
||||
|
||||
if (
|
||||
(!isCodeInterpreterCapabilityEnabled(req) || !tools.includes(Tools.execute_code)) &&
|
||||
agentData.tool_options != null
|
||||
) {
|
||||
agentData.tool_options = removeCodeExecutionCaller(agentData.tool_options);
|
||||
}
|
||||
|
||||
if (
|
||||
!validateStatefulCodeEnvironment(
|
||||
req,
|
||||
|
|
@ -818,7 +832,12 @@ const updateAgentHandler = async (req, res) => {
|
|||
updateData.stateful_code_sessions !== undefined ||
|
||||
updateData.stateful_code_environment !== undefined;
|
||||
const includesToolsConfiguration = Array.isArray(updateData.tools);
|
||||
if (includesStatefulConfiguration || includesToolsConfiguration) {
|
||||
const includesToolOptionsConfiguration = updateData.tool_options !== undefined;
|
||||
if (
|
||||
includesStatefulConfiguration ||
|
||||
includesToolsConfiguration ||
|
||||
includesToolOptionsConfiguration
|
||||
) {
|
||||
existingAgent = await db.getAgent({ id });
|
||||
if (!existingAgent) {
|
||||
return res.status(404).json({ error: 'Agent not found' });
|
||||
|
|
@ -851,6 +870,18 @@ const updateAgentHandler = async (req, res) => {
|
|||
return;
|
||||
}
|
||||
}
|
||||
|
||||
if (includesToolsConfiguration || includesToolOptionsConfiguration) {
|
||||
const effectiveTools = updateData.tools ?? existingAgent.tools;
|
||||
const effectiveToolOptions = updateData.tool_options ?? existingAgent.tool_options;
|
||||
if (
|
||||
(!isCodeInterpreterCapabilityEnabled(req) ||
|
||||
!effectiveTools?.includes(Tools.execute_code)) &&
|
||||
effectiveToolOptions != null
|
||||
) {
|
||||
updateData.tool_options = removeCodeExecutionCaller(effectiveToolOptions);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (updateData.model_parameters && typeof updateData.model_parameters === 'object') {
|
||||
|
|
@ -1265,6 +1296,14 @@ const duplicateAgentHandler = async (req, res) => {
|
|||
});
|
||||
}
|
||||
|
||||
if (
|
||||
(!isCodeInterpreterCapabilityEnabled(req) ||
|
||||
!newAgentData.tools?.includes(Tools.execute_code)) &&
|
||||
newAgentData.tool_options != null
|
||||
) {
|
||||
newAgentData.tool_options = removeCodeExecutionCaller(newAgentData.tool_options);
|
||||
}
|
||||
|
||||
const newAgent = await db.createAgent(newAgentData);
|
||||
|
||||
try {
|
||||
|
|
@ -1757,6 +1796,18 @@ const revertAgentVersionHandler = async (req, res) => {
|
|||
}
|
||||
}
|
||||
|
||||
const effectiveRevertTools = revertUpdates.tools ?? updatedAgent.tools;
|
||||
const hasCodeExecutionCaller = Object.values(updatedAgent.tool_options ?? {}).some((options) =>
|
||||
options.allowed_callers?.includes('code_execution'),
|
||||
);
|
||||
if (
|
||||
(!isCodeInterpreterCapabilityEnabled(req) ||
|
||||
!effectiveRevertTools?.includes(Tools.execute_code)) &&
|
||||
hasCodeExecutionCaller
|
||||
) {
|
||||
revertUpdates.tool_options = removeCodeExecutionCaller(updatedAgent.tool_options);
|
||||
}
|
||||
|
||||
if (updatedAgent.tool_resources) {
|
||||
const removedCount = await pruneToolResourceFileIdsForAgent({
|
||||
tool_resources: updatedAgent.tool_resources,
|
||||
|
|
|
|||
|
|
@ -182,6 +182,23 @@ describe('Agent Controllers - Mass Assignment Protection', () => {
|
|||
});
|
||||
|
||||
describe('createAgentHandler', () => {
|
||||
test('removes programmatic tool options when Code Interpreter capability is disabled', async () => {
|
||||
mockReq.body = {
|
||||
name: 'Invalid Programmatic Agent',
|
||||
provider: 'openai',
|
||||
model: 'gpt-4',
|
||||
tools: [Tools.execute_code, 'search_mcp_example'],
|
||||
tool_options: {
|
||||
search_mcp_example: { allowed_callers: ['code_execution'] },
|
||||
},
|
||||
};
|
||||
|
||||
await createAgentHandler(mockReq, mockRes);
|
||||
|
||||
expect(mockRes.status).toHaveBeenCalledWith(201);
|
||||
expect(mockRes.json.mock.calls[0][0].tool_options).toEqual({});
|
||||
});
|
||||
|
||||
test('rejects a stateful environment excluded by deployment policy', async () => {
|
||||
mockReq.config = {
|
||||
endpoints: {
|
||||
|
|
@ -824,6 +841,136 @@ describe('Agent Controllers - Mass Assignment Protection', () => {
|
|||
expect(agentInDb.name).toBe('Updated Agent');
|
||||
});
|
||||
|
||||
test('removes newly added programmatic options when Code Interpreter capability is disabled', async () => {
|
||||
await Agent.updateOne(
|
||||
{ id: existingAgentId },
|
||||
{ tools: [Tools.execute_code, 'search_mcp_example'] },
|
||||
);
|
||||
mockReq.user.id = existingAgentAuthorId.toString();
|
||||
mockReq.params.id = existingAgentId;
|
||||
mockReq.body = {
|
||||
tool_options: {
|
||||
search_mcp_example: { allowed_callers: ['code_execution'] },
|
||||
},
|
||||
};
|
||||
|
||||
await updateAgentHandler(mockReq, mockRes);
|
||||
|
||||
expect(mockRes.status).not.toHaveBeenCalledWith(400);
|
||||
expect(mockRes.json.mock.calls[0][0].tool_options).toEqual({});
|
||||
});
|
||||
|
||||
test('removes programmatic callers when Code Interpreter is disabled', async () => {
|
||||
await Agent.updateOne(
|
||||
{ id: existingAgentId },
|
||||
{
|
||||
tools: [Tools.execute_code, 'search_mcp_example'],
|
||||
tool_options: {
|
||||
search_mcp_example: {
|
||||
allowed_callers: ['code_execution'],
|
||||
defer_loading: true,
|
||||
},
|
||||
},
|
||||
},
|
||||
);
|
||||
mockReq.user.id = existingAgentAuthorId.toString();
|
||||
mockReq.params.id = existingAgentId;
|
||||
mockReq.body = { tools: ['search_mcp_example'] };
|
||||
|
||||
await updateAgentHandler(mockReq, mockRes);
|
||||
|
||||
expect(mockRes.status).not.toHaveBeenCalledWith(400);
|
||||
expect(mockRes.json.mock.calls[0][0].tool_options).toEqual({
|
||||
search_mcp_example: { defer_loading: true },
|
||||
});
|
||||
});
|
||||
|
||||
test('allows unrelated edits to a legacy inconsistent agent', async () => {
|
||||
await Agent.updateOne(
|
||||
{ id: existingAgentId },
|
||||
{
|
||||
tools: ['search_mcp_example'],
|
||||
tool_options: {
|
||||
search_mcp_example: { allowed_callers: ['code_execution'] },
|
||||
},
|
||||
},
|
||||
);
|
||||
mockReq.user.id = existingAgentAuthorId.toString();
|
||||
mockReq.params.id = existingAgentId;
|
||||
mockReq.body = { description: 'Still saveable' };
|
||||
|
||||
await updateAgentHandler(mockReq, mockRes);
|
||||
|
||||
expect(mockRes.status).not.toHaveBeenCalledWith(400);
|
||||
expect(mockRes.json.mock.calls[0][0].description).toBe('Still saveable');
|
||||
});
|
||||
|
||||
test('allows detaching a programmatic tool from a legacy inconsistent agent', async () => {
|
||||
await Agent.updateOne(
|
||||
{ id: existingAgentId },
|
||||
{
|
||||
tools: ['search_mcp_example'],
|
||||
tool_options: {
|
||||
search_mcp_example: { allowed_callers: ['code_execution'] },
|
||||
},
|
||||
},
|
||||
);
|
||||
mockReq.user.id = existingAgentAuthorId.toString();
|
||||
mockReq.params.id = existingAgentId;
|
||||
mockReq.body = { tools: [] };
|
||||
|
||||
await updateAgentHandler(mockReq, mockRes);
|
||||
|
||||
expect(mockRes.status).not.toHaveBeenCalledWith(400);
|
||||
expect(mockRes.json.mock.calls[0][0].tools).toEqual([]);
|
||||
expect(mockRes.json.mock.calls[0][0].tool_options).toEqual({});
|
||||
});
|
||||
|
||||
test('allows clearing programmatic options from a legacy inconsistent agent', async () => {
|
||||
await Agent.updateOne(
|
||||
{ id: existingAgentId },
|
||||
{
|
||||
tools: ['search_mcp_example'],
|
||||
tool_options: {
|
||||
search_mcp_example: { allowed_callers: ['code_execution'] },
|
||||
},
|
||||
},
|
||||
);
|
||||
mockReq.user.id = existingAgentAuthorId.toString();
|
||||
mockReq.params.id = existingAgentId;
|
||||
mockReq.body = { tool_options: {} };
|
||||
|
||||
await updateAgentHandler(mockReq, mockRes);
|
||||
|
||||
expect(mockRes.status).not.toHaveBeenCalledWith(400);
|
||||
expect(mockRes.json.mock.calls[0][0].tool_options).toEqual({});
|
||||
});
|
||||
|
||||
test('removes all newly submitted programmatic options from a legacy agent', async () => {
|
||||
await Agent.updateOne(
|
||||
{ id: existingAgentId },
|
||||
{
|
||||
tools: ['search_mcp_example', 'lookup_mcp_example'],
|
||||
tool_options: {
|
||||
search_mcp_example: { allowed_callers: ['code_execution'] },
|
||||
},
|
||||
},
|
||||
);
|
||||
mockReq.user.id = existingAgentAuthorId.toString();
|
||||
mockReq.params.id = existingAgentId;
|
||||
mockReq.body = {
|
||||
tool_options: {
|
||||
search_mcp_example: { allowed_callers: ['code_execution'] },
|
||||
lookup_mcp_example: { allowed_callers: ['code_execution'] },
|
||||
},
|
||||
};
|
||||
|
||||
await updateAgentHandler(mockReq, mockRes);
|
||||
|
||||
expect(mockRes.status).not.toHaveBeenCalledWith(400);
|
||||
expect(mockRes.json.mock.calls[0][0].tool_options).toEqual({});
|
||||
});
|
||||
|
||||
test('rejects selecting a stateful environment excluded by deployment policy', async () => {
|
||||
mockReq.user.id = existingAgentAuthorId.toString();
|
||||
mockReq.params.id = existingAgentId;
|
||||
|
|
@ -1495,6 +1642,83 @@ describe('Agent Controllers - Mass Assignment Protection', () => {
|
|||
const agentInDb = await Agent.findOne({ id: agent.id }).lean();
|
||||
expect(agentInDb.tool_resources.file_search.file_ids).toEqual([ownedFileId, otherFileId]);
|
||||
});
|
||||
|
||||
test('duplicateAgentHandler removes programmatic options without Code Interpreter', async () => {
|
||||
const sourceAgent = await Agent.create({
|
||||
id: `agent_${uuidv4()}`,
|
||||
name: 'Legacy Programmatic Agent',
|
||||
provider: 'openai',
|
||||
model: 'gpt-4',
|
||||
author: mockReq.user.id,
|
||||
tools: ['search_mcp_example'],
|
||||
tool_options: {
|
||||
search_mcp_example: { allowed_callers: ['code_execution'] },
|
||||
},
|
||||
});
|
||||
const db = require('~/models');
|
||||
jest.spyOn(db, 'getActions').mockResolvedValueOnce([]);
|
||||
mockReq.params.id = sourceAgent.id;
|
||||
|
||||
await duplicateAgentHandler(mockReq, mockRes);
|
||||
|
||||
expect(mockRes.status).toHaveBeenCalledWith(201);
|
||||
expect(mockRes.json.mock.calls[0][0].agent.tool_options).toEqual({});
|
||||
});
|
||||
|
||||
test('revertAgentVersionHandler removes restored programmatic options without Code Interpreter', async () => {
|
||||
const agent = await Agent.create({
|
||||
id: `agent_${uuidv4()}`,
|
||||
name: 'Current Agent',
|
||||
provider: 'openai',
|
||||
model: 'gpt-4',
|
||||
author: mockReq.user.id,
|
||||
versions: [
|
||||
{
|
||||
name: 'Legacy Programmatic Agent',
|
||||
provider: 'openai',
|
||||
model: 'gpt-4',
|
||||
tools: ['search_mcp_example'],
|
||||
tool_options: {
|
||||
search_mcp_example: { allowed_callers: ['code_execution'] },
|
||||
},
|
||||
},
|
||||
],
|
||||
});
|
||||
mockReq.params.id = agent.id;
|
||||
mockReq.body = { version_index: 0 };
|
||||
|
||||
await revertAgentVersionHandler(mockReq, mockRes);
|
||||
|
||||
expect(mockRes.json).toHaveBeenCalled();
|
||||
expect(mockRes.json.mock.calls[0][0].tool_options).toEqual({});
|
||||
});
|
||||
|
||||
test('revertAgentVersionHandler does not update unchanged tool options', async () => {
|
||||
const agent = await Agent.create({
|
||||
id: `agent_${uuidv4()}`,
|
||||
name: 'Current Agent',
|
||||
provider: 'openai',
|
||||
model: 'gpt-4',
|
||||
author: mockReq.user.id,
|
||||
versions: [
|
||||
{
|
||||
name: 'Historical Agent',
|
||||
provider: 'openai',
|
||||
model: 'gpt-4',
|
||||
tool_options: {},
|
||||
},
|
||||
],
|
||||
});
|
||||
const db = require('~/models');
|
||||
const updateAgentSpy = jest.spyOn(db, 'updateAgent');
|
||||
mockReq.params.id = agent.id;
|
||||
mockReq.body = { version_index: 0 };
|
||||
|
||||
await revertAgentVersionHandler(mockReq, mockRes);
|
||||
|
||||
expect(mockRes.json).toHaveBeenCalled();
|
||||
expect(updateAgentSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('Mass Assignment Attack Scenarios', () => {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue