fix(agent): scope MCP tools to explicit per-agent selection (#117)

This commit is contained in:
matevip 2026-05-17 09:24:56 +08:00
parent e97433cfc5
commit 18df97a8e6
3 changed files with 89 additions and 25 deletions

View File

@ -312,13 +312,17 @@ public class AgentBindingService {
* tools and skill-expanded tools: * tools and skill-expanded tools:
* <ul> * <ul>
* <li>{@link #SYSTEM_LEVEL_TOOLS} agent-wide primitives.</li> * <li>{@link #SYSTEM_LEVEL_TOOLS} agent-wide primitives.</li>
* <li>Every currently-bindable MCP tool (any tool with * <li>Every currently-bindable MCP tool ({@code source="mcp"},
* {@code source="mcp"} and {@code available=true} in the picker). * {@code available=true} in the picker) but only when the agent
* MCP servers are administrator-level capabilities; once enabled * has not ticked any MCP tool itself. MCP servers are
* globally they should not be silently hidden from an agent that * administrator-level capabilities, so an agent that bound merely
* happens to have any other binding. To deny a specific MCP tool * a skill or a built-in tool keeps full MCP access. Once the
* to a specific agent, use the tool-guard deny path applied * operator ticks specific MCP rows, that is read as a deliberate
* upstream in {@code AgentGraphBuilder}.</li> * per-agent scope: only the ticked MCP tools stay and the rest
* are not auto-joined, so a role can be limited to a fixed MCP
* tool set. To hide a single MCP tool from an agent that ticked
* no MCP row, use the tool-guard deny path applied upstream in
* {@code AgentGraphBuilder}.</li>
* </ul> * </ul>
*/ */
public Set<String> getEffectiveToolNames(Long agentId) { public Set<String> getEffectiveToolNames(Long agentId) {
@ -361,16 +365,22 @@ public class AgentBindingService {
// (the LLM stops being able to write to LESSONS.md / MEMORY.md). // (the LLM stops being able to write to LESSONS.md / MEMORY.md).
merged.addAll(SYSTEM_LEVEL_TOOLS); merged.addAll(SYSTEM_LEVEL_TOOLS);
// Enabled MCP server tools auto-join the allowlist for the same // MCP tools. An agent that bound only a skill or a built-in tool
// reason SYSTEM_LEVEL_TOOLS does: MCP servers are an // and ticked no MCP row keeps full access to every enabled MCP
// administrator-enabled capability, not a per-agent opt-in. Without // tool: MCP servers are an administrator-enabled capability and
// this union, an agent with any skill or built-in tool bound would // must not silently vanish just because some unrelated binding
// silently lose every MCP tool users hit this when they bound one // exists. But once the operator ticks specific MCP rows, that is a
// built-in tool, didn't tick the MCP rows, and observed "only // deliberate per-agent scope only those MCP tools (already merged
// built-in tools work". Operators who need to hide a specific MCP // via directTools above) stay, and the rest are not auto-joined, so
// tool from a specific agent still have the tool-guard deny path // a role can be limited to a fixed MCP tool set. To instead hide a
// (AgentGraphBuilder applies withDeniedToolsFiltered before this). // single MCP tool from an agent that ticked no MCP row, use the
merged.addAll(getEnabledMcpToolNames()); // tool-guard deny path applied upstream in AgentGraphBuilder.
Set<String> enabledMcpTools = getEnabledMcpToolNames();
boolean agentScopedMcpExplicitly =
directTools != null && !Collections.disjoint(directTools, enabledMcpTools);
if (!agentScopedMcpExplicitly) {
merged.addAll(enabledMcpTools);
}
return merged; return merged;
} }

View File

@ -12,6 +12,8 @@ import vip.mate.MateClawApplication;
import vip.mate.agent.binding.model.AgentToolBinding; import vip.mate.agent.binding.model.AgentToolBinding;
import vip.mate.agent.binding.service.AgentBindingService; import vip.mate.agent.binding.service.AgentBindingService;
import vip.mate.exception.MateClawException; import vip.mate.exception.MateClawException;
import vip.mate.tool.model.AvailableToolDTO;
import vip.mate.tool.service.AvailableToolService;
import java.util.List; import java.util.List;
import java.util.Set; import java.util.Set;
@ -41,6 +43,9 @@ class AgentBindingServiceTest {
@Autowired @Autowired
private AgentBindingService bindingService; private AgentBindingService bindingService;
@Autowired
private AvailableToolService availableToolService;
@Autowired @Autowired
private JdbcTemplate jdbcTemplate; private JdbcTemplate jdbcTemplate;
@ -357,6 +362,41 @@ class AgentBindingServiceTest {
+ "否则 AgentToolSet.withAllowedToolsOnly 会变成空集禁掉所有工具"); + "否则 AgentToolSet.withAllowedToolsOnly 会变成空集禁掉所有工具");
} }
@Test
@DisplayName("Issue #117: agent 显式勾选某个 MCP 工具后,只有该工具进入 allowlist其它 MCP 工具不再自动并入")
void mcpToolsScopedWhenAgentPicksSpecificMcpTool() {
// Enterprise scenario: a role should be limited to a fixed subset
// of MCP tools. Two enabled MCP servers exist; the operator ticks
// only server A's tool. Server B's tool must NOT leak into the
// allowlist just because its server is enabled at the system level.
seedMcpServerWithOneTool(8_888_101L, "issue117-server-a", "alpha_probe");
seedMcpServerWithOneTool(8_888_102L, "issue117-server-b", "beta_probe");
String mcpA = mcpToolNameForServer(8_888_101L);
String mcpB = mcpToolNameForServer(8_888_102L);
assertNotNull(mcpA, "server A 的 MCP 工具应出现在 picker 中");
assertNotNull(mcpB, "server B 的 MCP 工具应出现在 picker 中");
bindingService.setToolBindings(agentId, List.of(mcpA));
Set<String> effective = bindingService.getEffectiveToolNames(agentId);
assertNotNull(effective, "binding 非空时应返回 allowlist非 null");
assertTrue(effective.contains(mcpA), "显式勾选的 MCP 工具必须在 allowlist 中");
assertFalse(effective.contains(mcpB),
"未勾选的其它 MCP 工具不得自动并入 —— 这正是 issue #117 要求的按岗位限定 MCP 范围。"
+ "实际 allowlist: " + effective);
}
/** Picker name the UI would save for the (only) MCP tool of {@code serverId}. */
private String mcpToolNameForServer(long serverId) {
return availableToolService.listAvailable().stream()
.filter(t -> "mcp".equals(t.getSource()))
.filter(t -> t.getProviderId() != null && serverId == t.getProviderId())
.map(AvailableToolDTO::getName)
.findFirst()
.orElse(null);
}
@Test @Test
@DisplayName("Issue #143: 绑定任意工具后wiki 知识库工具仍留在 effective allowlist可读写知识库") @DisplayName("Issue #143: 绑定任意工具后wiki 知识库工具仍留在 effective allowlist可读写知识库")
void wikiToolsSurviveSkillBindingAllowlist() { void wikiToolsSurviveSkillBindingAllowlist() {

View File

@ -395,18 +395,22 @@
<template v-for="group in filteredAvailableToolGroups" :key="group.groupId"> <template v-for="group in filteredAvailableToolGroups" :key="group.groupId">
<div class="binding-group-header"> <div class="binding-group-header">
<span>{{ group.label }}</span> <span>{{ group.label }}</span>
<!-- MCP groups: ticking is now record-only enabled MCP <!-- MCP group badge reflects the agent's MCP scope:
tools auto-join the agent's effective allowlist with no MCP tool ticked, every enabled MCP tool is
(server is admin-enabled at the system level). Tell available by default; once any MCP tool is ticked,
the user that here so they don't think unchecked the agent is restricted to the ticked set. To deny
MCP rows are disabled. To deny a specific MCP tool, an MCP tool when none are ticked, users still have
users still have Security Tool Guard. --> Security Tool Guard. -->
<span <span
v-if="group.groupId && group.groupId.startsWith('mcp:')" v-if="group.groupId && group.groupId.startsWith('mcp:')"
class="binding-group-note" class="binding-group-note"
:title="t('agents.binding.mcpAutoIncludedTooltip')" :title="anyMcpToolSelected
? t('agents.binding.mcpScopedTooltip')
: t('agents.binding.mcpAutoIncludedTooltip')"
> >
{{ t('agents.binding.mcpAutoIncludedBadge') }} {{ anyMcpToolSelected
? t('agents.binding.mcpScopedBadge')
: t('agents.binding.mcpAutoIncludedBadge') }}
</span> </span>
</div> </div>
<label <label
@ -638,6 +642,16 @@ const availableToolGroups = computed(() => {
const filteredAvailableToolGroups = computed(() => filterAgentToolGroups(availableToolGroups.value, toolBindingSearch.value)) const filteredAvailableToolGroups = computed(() => filterAgentToolGroups(availableToolGroups.value, toolBindingSearch.value))
/**
* True when at least one ticked tool is an MCP tool. The backend reads
* this as a deliberate per-agent MCP scope: only ticked MCP tools stay
* in the effective allowlist. When false, every enabled MCP tool is
* auto-included instead. Drives the MCP group badge in the picker.
*/
const anyMcpToolSelected = computed(() =>
availableTools.value.some((tool) => tool.source === 'mcp' && selectedToolNames.value.includes(tool.name)),
)
/** /**
* Manual checkbox handler replaces v-model on the picker row so that * Manual checkbox handler replaces v-model on the picker row so that
* two rows sharing a tool name (collision/duplicate twins) don't drag * two rows sharing a tool name (collision/duplicate twins) don't drag