refactor(mcp): centralize tool filter policy (#124739)

This commit is contained in:
Peter Steinberger
2026-08-16 11:51:23 -07:00
committed by GitHub
parent a2067ed1bb
commit a741f5b8ae
9 changed files with 125 additions and 141 deletions
@@ -1,22 +0,0 @@
import { describe, expect, it } from "vitest";
import { matchesMcpToolFilterPattern } from "./agent-bundle-mcp-filter.js";
describe("matchesMcpToolFilterPattern", () => {
it.each([
["", "tool", false],
["search_docs", "search_docs", true],
["search_docs", "read_docs", false],
["*_docs", "search_docs", true],
["resources_*", "resources_read", true],
["a**b***c", "axbyc", true],
["a*b*c", "acb", false],
])("matches %j against %j", (pattern, value, expected) => {
expect(matchesMcpToolFilterPattern(pattern, value)).toBe(expected);
});
it("rejects adversarial separated wildcards without regex backtracking", () => {
const pattern = `${"*a".repeat(128)}*b`;
const value = `${"a".repeat(10_000)}c`;
expect(matchesMcpToolFilterPattern(pattern, value)).toBe(false);
});
});
-37
View File
@@ -1,37 +0,0 @@
/** Match the documented MCP tool-filter glob syntax: exact text plus `*`. */
export function matchesMcpToolFilterPattern(pattern: string, value: string): boolean {
const trimmed = pattern.trim();
if (!trimmed) {
return false;
}
if (!trimmed.includes("*")) {
return trimmed === value;
}
const parts = trimmed.split("*");
const first = parts[0] ?? "";
const last = parts.at(-1) ?? "";
let cursor = 0;
if (first) {
if (!value.startsWith(first)) {
return false;
}
cursor = first.length;
}
const endBound = last ? value.length - last.length : value.length;
if (last && (!value.endsWith(last) || endBound < cursor)) {
return false;
}
for (const part of parts.slice(1, -1)) {
if (!part) {
continue;
}
const index = value.indexOf(part, cursor);
if (index === -1 || index + part.length > endBound) {
return false;
}
cursor = index + part.length;
}
return true;
}
+2 -10
View File
@@ -7,7 +7,6 @@ import { normalizeLowercaseStringOrEmpty } from "@openclaw/normalization-core/st
import type { OpenClawConfig } from "../config/types.openclaw.js";
import { logWarn } from "../logger.js";
import { getPluginToolMeta, setPluginToolMeta, type PluginToolMcpMeta } from "../plugins/tools.js";
import { matchesMcpToolFilterPattern } from "./agent-bundle-mcp-filter.js";
import {
buildSafeToolName,
normalizeReservedToolNames,
@@ -22,6 +21,7 @@ import type {
SessionMcpRuntime,
} from "./agent-bundle-mcp-types.js";
import { mcpContentBlockToAgentContent } from "./mcp-content.js";
import { isMcpToolAllowed } from "./mcp-tool-filter.js";
import { buildMcpAppCanvasPayload, fetchMcpAppView } from "./mcp-ui-resource.js";
import type { AgentToolResult } from "./runtime/index.js";
import type { AnyAgentTool } from "./tools/common.js";
@@ -210,15 +210,7 @@ function serverAllowsUtilityTool(
if ((server.deniedToolNames?.includes(operation) === true) !== sessionDeniedOnly) {
return false;
}
const include = server.toolFilter?.include ?? [];
const exclude = server.toolFilter?.exclude ?? [];
if (
include.length > 0 &&
!include.some((pattern) => matchesMcpToolFilterPattern(pattern, operation))
) {
return false;
}
return !exclude.some((pattern) => matchesMcpToolFilterPattern(pattern, operation));
return isMcpToolAllowed(server.toolFilter, operation);
}
function addMcpUtilityTool(params: {
+6 -46
View File
@@ -22,7 +22,6 @@ import { redactToolPayloadText } from "../logging/redact.js";
import type { PluginManifestRegistry } from "../plugins/manifest-registry.js";
import { runTasksWithConcurrency } from "../utils/run-with-concurrency.js";
import { mergeMcpToolCatalogs } from "./agent-bundle-mcp-combined.js";
import { matchesMcpToolFilterPattern } from "./agent-bundle-mcp-filter.js";
import {
completeDeferredSessionMcpRuntimeRetirement,
disposeAllSessionMcpRuntimes,
@@ -69,6 +68,7 @@ import { createMcpJsonSchemaValidator } from "./mcp-json-schema-validator.js";
import { sanitizeMcpMetadataText } from "./mcp-metadata.js";
import { collectMcpPaginatedItems } from "./mcp-pagination.js";
import { OpenClawStdioClientTransport } from "./mcp-stdio-transport.js";
import { isMcpToolAllowed, normalizeMcpToolFilter } from "./mcp-tool-filter.js";
import { resolveMcpTransport } from "./mcp-transport.js";
type BundleMcpSession = {
@@ -114,11 +114,6 @@ function getBundleMcpTestState(): BundleMcpTestState {
return state;
}
type McpToolSelection = {
include?: readonly string[];
exclude?: readonly string[];
};
type McpServerBackoffState = {
session: BundleMcpSession;
failures: number;
@@ -285,14 +280,6 @@ function buildMcpClientOptions(mcpAppsEnabled: boolean): ClientOptions {
return { capabilities: buildMcpClientCapabilities(mcpAppsEnabled) };
}
function normalizeStringList(value: unknown): string[] | undefined {
if (!Array.isArray(value)) {
return undefined;
}
const entries = value.filter((entry): entry is string => typeof entry === "string");
return entries.length > 0 ? entries : undefined;
}
function normalizeToolUiVisibility(value: unknown): Array<"app" | "model"> | undefined {
if (!Array.isArray(value)) {
return undefined;
@@ -303,28 +290,6 @@ function normalizeToolUiVisibility(value: unknown): Array<"app" | "model"> | und
return [...new Set(normalized)].toSorted();
}
function getMcpToolSelection(rawServer: unknown): McpToolSelection {
if (!isRecord(rawServer) || !isRecord(rawServer.toolFilter)) {
return {};
}
return {
include: normalizeStringList(rawServer.toolFilter.include),
exclude: normalizeStringList(rawServer.toolFilter.exclude),
};
}
function shouldExposeMcpTool(selection: McpToolSelection, toolName: string): boolean {
const include = selection.include ?? [];
const exclude = selection.exclude ?? [];
if (
include.length > 0 &&
!include.some((pattern) => matchesMcpToolFilterPattern(pattern, toolName))
) {
return false;
}
return !exclude.some((pattern) => matchesMcpToolFilterPattern(pattern, toolName));
}
function summarizeServerCapabilities(capabilities: ServerCapabilities | undefined) {
return {
resources: capabilities?.resources
@@ -891,13 +856,15 @@ export function createSessionMcpRuntime(params: {
),
});
failIfDisposed();
const selection = getMcpToolSelection(rawServer);
const toolFilter = normalizeMcpToolFilter(
isRecord(rawServer) ? rawServer.toolFilter : undefined,
);
const denialMap = params.toolOverrides?.mcpToolsDeny;
const deniedToolNames = new Set(
denialMap && Object.hasOwn(denialMap, serverName) ? denialMap[serverName] : [],
);
const policyEligibleTools = listedTools.filter((tool) =>
shouldExposeMcpTool(selection, tool.name.trim()),
isMcpToolAllowed(toolFilter, tool.name.trim()),
);
const exposedTools = policyEligibleTools.filter((tool) => {
const toolName = tool.name.trim();
@@ -922,14 +889,7 @@ export function createSessionMcpRuntime(params: {
},
}
: {}),
...(selection.include || selection.exclude
? {
toolFilter: {
...(selection.include ? { include: [...selection.include] } : {}),
...(selection.exclude ? { exclude: [...selection.exclude] } : {}),
},
}
: {}),
...(toolFilter ? { toolFilter } : {}),
...(deniedToolNames.size > 0
? { deniedToolNames: [...deniedToolNames].toSorted() }
: {}),
+2 -5
View File
@@ -6,7 +6,7 @@ import type {
} from "@modelcontextprotocol/sdk/types.js";
import type { TSchema } from "typebox";
import type { SessionToolOverrides } from "../config/sessions/types.js";
import type { McpCodexToolApprovalMode } from "../config/types.mcp.js";
import type { McpCodexToolApprovalMode, McpServerToolFilterConfig } from "../config/types.mcp.js";
import type { OpenClawConfig } from "../config/types.openclaw.js";
import type { PluginManifestRegistry } from "../plugins/manifest-registry.js";
import type { McpCodexToolAnnotations } from "./mcp-codex-tool-approval.js";
@@ -40,10 +40,7 @@ export type McpServerCatalog = {
};
requestTimeoutMs?: number;
supportsParallelToolCalls?: boolean;
toolFilter?: {
include?: string[];
exclude?: string[];
};
toolFilter?: McpServerToolFilterConfig;
deniedToolNames?: string[];
codexApprovalMode?: McpCodexToolApprovalMode;
};
+45
View File
@@ -0,0 +1,45 @@
import { describe, expect, it } from "vitest";
import { isMcpToolAllowed, normalizeMcpToolFilter } from "./mcp-tool-filter.js";
describe("isMcpToolAllowed", () => {
it.each([
["", "tool", false],
["search_docs", "search_docs", true],
["search_docs", "read_docs", false],
["*_docs", "search_docs", true],
["resources_*", "resources_read", true],
["a**b***c", "axbyc", true],
["a*b*c", "acb", false],
])("matches %j against %j", (pattern, value, expected) => {
expect(isMcpToolAllowed({ include: [pattern] }, value)).toBe(expected);
});
it("rejects adversarial separated wildcards without regex backtracking", () => {
const pattern = `${"*a".repeat(128)}*b`;
const value = `${"a".repeat(10_000)}c`;
expect(isMcpToolAllowed({ include: [pattern] }, value)).toBe(false);
});
it.each([
[undefined, undefined],
["malformed", undefined],
[{ include: "search_*", exclude: 42 }, undefined],
[{ include: [] }, undefined],
[{ include: [false, "search_*", null] }, { include: ["search_*"] }],
[{ include: [false, null] }, undefined],
[{ exclude: [false, "search_*", null] }, { exclude: ["search_*"] }],
[
{ include: [" *_docs "], exclude: ["admin_*"] },
{ include: [" *_docs "], exclude: ["admin_*"] },
],
])("normalizes filter %j", (raw, expected) => {
expect(normalizeMcpToolFilter(raw)).toEqual(expected);
});
it("requires an include match and lets exclude win", () => {
const filter = { include: ["*_docs"], exclude: ["search_*"] };
expect(isMcpToolAllowed(filter, "read_docs")).toBe(true);
expect(isMcpToolAllowed(filter, "search_docs")).toBe(false);
expect(isMcpToolAllowed(filter, "read_file")).toBe(false);
});
});
+66
View File
@@ -0,0 +1,66 @@
import { isRecord } from "@openclaw/normalization-core/record-coerce";
import { filterStringEntries } from "@openclaw/normalization-core/string-normalization";
import type { McpServerToolFilterConfig } from "../config/types.mcp.js";
/** Match the documented MCP tool-filter glob syntax: exact text plus `*`. */
function matchesMcpToolFilterPattern(pattern: string, value: string): boolean {
const trimmed = pattern.trim();
if (!trimmed) {
return false;
}
if (!trimmed.includes("*")) {
return trimmed === value;
}
const parts = trimmed.split("*");
const first = parts[0] ?? "";
const last = parts.at(-1) ?? "";
if (first && !value.startsWith(first)) {
return false;
}
let cursor = first.length;
const endBound = last ? value.length - last.length : value.length;
if (last && (!value.endsWith(last) || endBound < cursor)) {
return false;
}
for (const part of parts.slice(1, -1)) {
if (!part) {
continue;
}
const index = value.indexOf(part, cursor);
if (index === -1 || index + part.length > endBound) {
return false;
}
cursor = index + part.length;
}
return true;
}
/** Normalizes open-world MCP tool filters into the runtime policy shape. */
export function normalizeMcpToolFilter(raw: unknown): McpServerToolFilterConfig | undefined {
if (!isRecord(raw)) {
return undefined;
}
const include = filterStringEntries(raw.include);
const exclude = filterStringEntries(raw.exclude);
if (include.length === 0 && exclude.length === 0) {
return undefined;
}
return {
...(include.length > 0 ? { include } : {}),
...(exclude.length > 0 ? { exclude } : {}),
};
}
/** Applies the shared include-then-exclude policy. */
export function isMcpToolAllowed(
toolFilter: McpServerToolFilterConfig | undefined,
toolName: string,
): boolean {
const matches = (pattern: string) => matchesMcpToolFilterPattern(pattern, toolName);
return (
(!toolFilter?.include?.length || toolFilter.include.some(matches)) &&
!toolFilter?.exclude?.some(matches)
);
}
+2 -7
View File
@@ -11,6 +11,7 @@ import { replaceConfigFile } from "./mutate.js";
import { redactSensitiveArgv } from "./redact-argv.js";
import { REDACTED_SENTINEL, restoreRedactedValues } from "./redact-snapshot.js";
import { buildConfigSchemaCore } from "./schema.js";
import type { McpServerToolFilterConfig } from "./types.mcp.js";
import type { OpenClawConfig } from "./types.openclaw.js";
import { validateConfigObjectWithPlugins } from "./validation.js";
@@ -39,12 +40,6 @@ type McpConfigMutation = {
};
type McpConfigMutationHook = (mutation: McpConfigMutation) => Promise<void>;
/** Include/exclude tool selection stored for a configured MCP server. */
type McpServerToolSelection = {
include?: string[];
exclude?: string[];
};
function normalizeToolSelectionList(value: readonly string[] | undefined): string[] | undefined {
if (!value) {
return undefined;
@@ -205,7 +200,7 @@ async function updateConfiguredMcpServerConfig(params: {
async function updateConfiguredMcpServerTools(
params: {
name: string;
tools: McpServerToolSelection | null;
tools: McpServerToolFilterConfig | null;
recordIndependentOwner?: boolean;
},
onCommitted?: McpConfigMutationHook,
+2 -14
View File
@@ -7,10 +7,10 @@ import { clampPositiveTimerTimeoutMs } from "@openclaw/normalization-core/number
import { normalizeLowercaseStringOrEmpty } from "@openclaw/normalization-core/string-coerce";
import { truncateUtf16Safe } from "@openclaw/normalization-core/utf16-slice";
import type { NodePluginToolDescriptor } from "../../packages/gateway-protocol/src/schema/nodes.js";
import { matchesMcpToolFilterPattern } from "../agents/agent-bundle-mcp-filter.js";
import { createMcpJsonSchemaValidator } from "../agents/mcp-json-schema-validator.js";
import { sanitizeMcpMetadataText } from "../agents/mcp-metadata.js";
import { collectMcpPaginatedItems } from "../agents/mcp-pagination.js";
import { isMcpToolAllowed } from "../agents/mcp-tool-filter.js";
import { resolveMcpRequestTimeoutMs } from "../agents/mcp-transport-config.js";
import { resolveMcpTransport } from "../agents/mcp-transport.js";
import { normalizeConfiguredMcpServers } from "../config/mcp-config-normalize.js";
@@ -197,18 +197,6 @@ function buildNodeMcpToolDescriptors(
return descriptors;
}
function shouldExposeTool(config: McpServerConfig, toolName: string): boolean {
const include = config.toolFilter?.include ?? [];
const exclude = config.toolFilter?.exclude ?? [];
if (
include.length > 0 &&
!include.some((pattern) => matchesMcpToolFilterPattern(pattern, toolName))
) {
return false;
}
return !exclude.some((pattern) => matchesMcpToolFilterPattern(pattern, toolName));
}
async function connectWithTimeout(
client: NodeHostMcpClient,
transport: Transport,
@@ -350,7 +338,7 @@ export async function startNodeHostMcpManager(
const tools = await listAllTools(
client,
resolved.requestTimeoutMs,
(toolName) => shouldExposeTool(config, toolName),
(toolName) => isMcpToolAllowed(config.toolFilter, toolName),
deps.signal,
);
if (session.connected) {