mirror of
https://github.com/CopilotKit/CopilotKit.git
synced 2026-09-14 16:26:20 +08:00
fix(runtime): reject unenforceable mcpApps tool policy instead of silently ignoring it (#6292)
## Summary `mcpApps.servers` entries that carry `includeTools` or `excludeTools` are currently accepted even though the pinned `@ag-ui/mcp-apps-middleware` package has no option for them. The runtime then ignores the keys, so tools an operator intended to restrict remain available. This change rejects that configuration instead of allowing a silent no-op. ## What CopilotKit owns - `mcpApps.servers` configuration and `agentId` scoping. - Projection of selected servers into `MCPAppsMiddleware`. - Reporting unsupported configuration before middleware construction. Discovery, model-emitted tool execution, frontend-proxied execution, server identity, and tool provenance belong to `@ag-ui/mcp-apps-middleware`. ## Changes - Extract the server projection into `resolveMcpAppsServers`, which scans all configured entries for defined policy keys, filters by `agentId`, strips only `agentId`, and forwards other fields unchanged. - Return a configuration error naming the unsupported key, server, pinned middleware version, owning package, and issue when a policy key is supplied. - Add tests for agent scoping, field forwarding, malformed and empty values, undefined spread values, constructor avoidance, and the existing HTTP error path. - Document the ownership boundary and add a runtime changeset. ## Why the filter stays external The pinned package is version `0.0.3`. It owns the private server maps, UI-tool discovery, model-emitted execution, and frontend proxy execution. A CopilotKit middleware could observe only one of those paths and would have to duplicate private server identity and tool provenance. The complete `includeTools` and `excludeTools` implementation belongs in the external package, where one predicate can cover discovery and both execution paths. ## Current behavior Plain JavaScript or JSON configuration can supply `excludeTools: ["delete_account"]` without a TypeScript excess-property check. The runtime currently accepts the configuration, constructs `MCPAppsMiddleware`, and leaves the tool available. The new behavior returns an HTTP 500 through the existing runtime error path, names the unsupported key and dependency, and does not construct the middleware. ## Follow-up The counterpart change in `@ag-ui/mcp-apps-middleware` should add the fields to the per-server configuration, preserve absent versus empty include lists, resolve server identity through its existing maps, and apply one predicate after UI-resource discovery and before model-emitted and proxied tool execution. Once that version is released, CopilotKit can remove the rejection and pass the fields through unchanged. ## Related issue Refs #5930. The cross-repository ownership split follows the proposal in https://github.com/CopilotKit/CopilotKit/issues/5930#issuecomment-5128722524. This PR does not close the issue. ## Test plan - [x] `pnpm -C packages/runtime exec vitest run src/v2/runtime/__tests__/mcp-apps-servers.test.ts src/v2/runtime/__tests__/mcp-apps-middleware-integration.test.ts` passed, 2 files and 20 tests - [x] `pnpm -C packages/runtime exec vitest run` passed, 129 files and 1,836 tests - [x] `pnpm exec nx run @copilotkit/runtime:check-types` passed - [x] `pnpm exec oxlint` and `pnpm exec oxfmt --check` passed on changed TypeScript files - [x] `pnpm check:plugin-skills` passed - [ ] `CI green for static / quality and test / unit on Node 20, 22, and 24`
This commit is contained in:
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@copilotkit/runtime": minor
|
||||
---
|
||||
|
||||
fix(runtime): reject unenforceable mcpApps tool policy instead of silently ignoring it
|
||||
|
||||
Reject MCP Apps per-tool policy keys until the external middleware supports them, preventing silent configuration no-ops.
|
||||
@@ -49,6 +49,11 @@ export default { fetch: handler };
|
||||
Each server entry accepts an optional `agentId`. When set, the server's tools are only
|
||||
exposed to that agent. Omit it to expose to all agents.
|
||||
|
||||
Per-tool filtering belongs to `@ag-ui/mcp-apps-middleware`. The runtime currently pins
|
||||
version `0.0.3`, which does not support `includeTools` or `excludeTools`. Supplying either
|
||||
key raises a configuration error instead of silently ignoring it; use a compatible
|
||||
middleware release when that upstream policy contract is available.
|
||||
|
||||
## Gotcha — do NOT put MCP under agents
|
||||
|
||||
```typescript
|
||||
|
||||
@@ -1,13 +1,32 @@
|
||||
import { describe, it, expect, afterEach, vi } from "vitest";
|
||||
import {
|
||||
AbstractAgent,
|
||||
RunAgentInput,
|
||||
BaseEvent,
|
||||
EventType,
|
||||
} from "@ag-ui/client";
|
||||
import { AbstractAgent, EventType } from "@ag-ui/client";
|
||||
import type { BaseEvent, RunAgentInput } from "@ag-ui/client";
|
||||
import { Observable } from "rxjs";
|
||||
import { LLMock, MCPMock } from "@copilotkit/aimock";
|
||||
import { MCPAppsMiddleware, getServerHash } from "@ag-ui/mcp-apps-middleware";
|
||||
import type * as MCPAppsMiddlewareModule from "@ag-ui/mcp-apps-middleware";
|
||||
import type { McpAppsServerConfig } from "../core/runtime";
|
||||
import type { CopilotRuntimeLike } from "../core/runtime";
|
||||
import { CopilotRuntime } from "../core/runtime";
|
||||
import { configureAgentForRequest } from "../handlers/shared/agent-utils";
|
||||
import { handleRunAgent } from "../handlers/handle-run";
|
||||
|
||||
const middlewareConstructor = vi.hoisted(() => vi.fn());
|
||||
|
||||
vi.mock("@ag-ui/mcp-apps-middleware", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof MCPAppsMiddlewareModule>();
|
||||
|
||||
class TrackedMCPAppsMiddleware extends actual.MCPAppsMiddleware {
|
||||
constructor(
|
||||
...args: ConstructorParameters<typeof actual.MCPAppsMiddleware>
|
||||
) {
|
||||
middlewareConstructor(...args);
|
||||
super(...args);
|
||||
}
|
||||
}
|
||||
|
||||
return { ...actual, MCPAppsMiddleware: TrackedMCPAppsMiddleware };
|
||||
});
|
||||
|
||||
/**
|
||||
* A minimal next-agent that emits RUN_STARTED and RUN_FINISHED.
|
||||
@@ -74,6 +93,7 @@ describe("MCPAppsMiddleware integration", () => {
|
||||
if (llm) {
|
||||
await llm.stop().catch(() => {});
|
||||
}
|
||||
middlewareConstructor.mockClear();
|
||||
});
|
||||
|
||||
async function startMcpServer(): Promise<string> {
|
||||
@@ -115,6 +135,140 @@ describe("MCPAppsMiddleware integration", () => {
|
||||
expect(middleware).toBeInstanceOf(MCPAppsMiddleware);
|
||||
});
|
||||
|
||||
it("rejects unsupported policy before the runtime constructs middleware", async () => {
|
||||
const server = {
|
||||
type: "http" as const,
|
||||
url: "https://mcp.example.com/mcp",
|
||||
serverId: "weather",
|
||||
excludeTools: ["delete_account"],
|
||||
} as unknown as McpAppsServerConfig;
|
||||
const runtime = new CopilotRuntime({
|
||||
agents: { default: new MockNextAgent() },
|
||||
mcpApps: { servers: [server] },
|
||||
});
|
||||
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
|
||||
try {
|
||||
const response = await handleRunAgent({
|
||||
runtime,
|
||||
agentId: "default",
|
||||
request: new Request("https://example.com/agent/default/run", {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify(createRunInput()),
|
||||
}),
|
||||
});
|
||||
|
||||
expect(response.status).toBe(500);
|
||||
await expect(response.json()).resolves.toMatchObject({
|
||||
error: "Failed to run agent",
|
||||
message: expect.stringContaining("excludeTools"),
|
||||
});
|
||||
expect(middlewareConstructor).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
errorSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it("attaches valid MCP Apps servers through the production adapter", () => {
|
||||
const agent = new MockNextAgent();
|
||||
const runtime = {
|
||||
mcpApps: {
|
||||
servers: [
|
||||
{
|
||||
type: "http" as const,
|
||||
url: "https://mcp.example.com/mcp",
|
||||
serverId: "weather",
|
||||
agentId: "default",
|
||||
},
|
||||
],
|
||||
},
|
||||
} as unknown as CopilotRuntimeLike;
|
||||
|
||||
configureAgentForRequest({
|
||||
runtime,
|
||||
request: new Request("https://example.com/run"),
|
||||
agentId: "default",
|
||||
agent,
|
||||
});
|
||||
|
||||
expect(middlewareConstructor).toHaveBeenCalledOnce();
|
||||
const [config] = middlewareConstructor.mock.calls[0] as [
|
||||
{ mcpServers: Record<string, unknown>[] },
|
||||
];
|
||||
expect(config.mcpServers).toEqual([
|
||||
{
|
||||
type: "http",
|
||||
url: "https://mcp.example.com/mcp",
|
||||
serverId: "weather",
|
||||
},
|
||||
]);
|
||||
expect(config.mcpServers[0]).not.toHaveProperty("agentId");
|
||||
});
|
||||
|
||||
it("does not attach MCP Apps when no server matches the agent", () => {
|
||||
const agent = new MockNextAgent();
|
||||
const runtime = {
|
||||
mcpApps: {
|
||||
servers: [
|
||||
{
|
||||
type: "http" as const,
|
||||
url: "https://mcp.example.com/mcp",
|
||||
agentId: "other",
|
||||
},
|
||||
],
|
||||
},
|
||||
} as unknown as CopilotRuntimeLike;
|
||||
|
||||
configureAgentForRequest({
|
||||
runtime,
|
||||
request: new Request("https://example.com/run"),
|
||||
agentId: "default",
|
||||
agent,
|
||||
});
|
||||
|
||||
expect(middlewareConstructor).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not attach MCP Apps for an empty server list", () => {
|
||||
const agent = new MockNextAgent();
|
||||
const runtime = {
|
||||
mcpApps: { servers: [] },
|
||||
} as unknown as CopilotRuntimeLike;
|
||||
|
||||
configureAgentForRequest({
|
||||
runtime,
|
||||
request: new Request("https://example.com/run"),
|
||||
agentId: "default",
|
||||
agent,
|
||||
});
|
||||
|
||||
expect(middlewareConstructor).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not attach MCP Apps when the agent has no use method", () => {
|
||||
const agent = { headers: {} } as unknown as AbstractAgent;
|
||||
const runtime = {
|
||||
mcpApps: {
|
||||
servers: [
|
||||
{
|
||||
type: "http" as const,
|
||||
url: "https://mcp.example.com/mcp",
|
||||
},
|
||||
],
|
||||
},
|
||||
} as unknown as CopilotRuntimeLike;
|
||||
|
||||
configureAgentForRequest({
|
||||
runtime,
|
||||
request: new Request("https://example.com/run"),
|
||||
agentId: "default",
|
||||
agent,
|
||||
});
|
||||
|
||||
expect(middlewareConstructor).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("proxies tools/call through to MCPMock and returns results", async () => {
|
||||
const mcpUrl = await startMcpServer();
|
||||
|
||||
|
||||
@@ -0,0 +1,126 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import type { MCPClientConfig } from "@ag-ui/mcp-apps-middleware";
|
||||
import type { McpAppsServerConfig } from "../core/runtime";
|
||||
import { resolveMcpAppsServers } from "../handlers/shared/mcp-apps-servers";
|
||||
|
||||
type Assert<T extends true> = T;
|
||||
type ExternalPolicyKeys = Extract<
|
||||
keyof MCPClientConfig,
|
||||
"includeTools" | "excludeTools"
|
||||
>;
|
||||
type ExternalMiddlewareHasNoPolicyFields = [ExternalPolicyKeys] extends [never]
|
||||
? true
|
||||
: false;
|
||||
type ExternalMiddlewarePolicyTripwire =
|
||||
Assert<ExternalMiddlewareHasNoPolicyFields>;
|
||||
|
||||
void (undefined as unknown as ExternalMiddlewarePolicyTripwire);
|
||||
|
||||
const baseServer = {
|
||||
type: "sse" as const,
|
||||
url: "https://mcp.example.com/sse",
|
||||
headers: { Authorization: "Bearer token" },
|
||||
serverId: "weather",
|
||||
futureField: { retained: true },
|
||||
};
|
||||
|
||||
function asServer(config: Record<string, unknown>): McpAppsServerConfig {
|
||||
return config as unknown as McpAppsServerConfig;
|
||||
}
|
||||
|
||||
describe("resolveMcpAppsServers", () => {
|
||||
it.each([
|
||||
["includeTools", []],
|
||||
["includeTools", null],
|
||||
["includeTools", "get_weather"],
|
||||
["excludeTools", []],
|
||||
["excludeTools", null],
|
||||
["excludeTools", "delete_account"],
|
||||
])("rejects a defined %s value of any shape", (key, value) => {
|
||||
expect(() =>
|
||||
resolveMcpAppsServers(
|
||||
[asServer({ ...baseServer, [key]: value })],
|
||||
"default",
|
||||
),
|
||||
).toThrow(
|
||||
new RegExp(
|
||||
`${key}.*server\\[0\\].*weather.*@ag-ui/mcp-apps-middleware@0\\.0\\.3.*https://github\\.com/CopilotKit/CopilotKit/issues/5930`,
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
it("reports every unsupported key before filtering by agent", () => {
|
||||
expect(() =>
|
||||
resolveMcpAppsServers(
|
||||
[
|
||||
asServer({
|
||||
...baseServer,
|
||||
serverId: "other",
|
||||
agentId: "other",
|
||||
includeTools: [],
|
||||
}),
|
||||
asServer({
|
||||
...baseServer,
|
||||
serverId: "current",
|
||||
agentId: "current",
|
||||
excludeTools: null,
|
||||
}),
|
||||
],
|
||||
"current",
|
||||
),
|
||||
).toThrow(
|
||||
/includeTools at server\[0\] \(other\).*excludeTools at server\[1\] \(current\)/,
|
||||
);
|
||||
});
|
||||
|
||||
it("accepts an explicitly undefined policy value and forwards it unchanged", () => {
|
||||
const server = { ...baseServer, includeTools: undefined };
|
||||
|
||||
expect(resolveMcpAppsServers([asServer(server)], "default")).toEqual([
|
||||
server,
|
||||
]);
|
||||
});
|
||||
|
||||
it("filters by agent, strips only agentId, and preserves order and fields", () => {
|
||||
const globalServer = { ...baseServer, serverId: "global" };
|
||||
const otherServer = {
|
||||
...baseServer,
|
||||
serverId: "other",
|
||||
agentId: "other",
|
||||
};
|
||||
const matchingServer = {
|
||||
...baseServer,
|
||||
serverId: "matching",
|
||||
agentId: "default",
|
||||
};
|
||||
|
||||
const resolved = resolveMcpAppsServers(
|
||||
[asServer(globalServer), asServer(otherServer), asServer(matchingServer)],
|
||||
"default",
|
||||
);
|
||||
|
||||
expect(resolved).toEqual([
|
||||
globalServer,
|
||||
{
|
||||
type: "sse",
|
||||
url: matchingServer.url,
|
||||
headers: matchingServer.headers,
|
||||
serverId: "matching",
|
||||
futureField: matchingServer.futureField,
|
||||
},
|
||||
]);
|
||||
expect(Object.prototype.hasOwnProperty.call(resolved[0], "agentId")).toBe(
|
||||
false,
|
||||
);
|
||||
});
|
||||
|
||||
it("returns no servers for empty and nonmatching configurations", () => {
|
||||
expect(resolveMcpAppsServers([], "default")).toEqual([]);
|
||||
expect(
|
||||
resolveMcpAppsServers(
|
||||
[asServer({ ...baseServer, agentId: "other" })],
|
||||
"default",
|
||||
),
|
||||
).toEqual([]);
|
||||
});
|
||||
});
|
||||
@@ -47,6 +47,7 @@ interface BaseCopilotRuntimeMiddlewareOptions {
|
||||
agents?: string[];
|
||||
}
|
||||
|
||||
/** Per-server tool policy belongs to the external middleware and is unsupported at its pinned 0.0.3 release. */
|
||||
export type McpAppsServerConfig = MCPClientConfig & {
|
||||
/** Agent to bind this server to. If omitted, the server is available to all agents. */
|
||||
agentId?: string;
|
||||
|
||||
@@ -18,6 +18,7 @@ import {
|
||||
mergeForwardableHeaders,
|
||||
resolveForwardHeadersPolicy,
|
||||
} from "../header-utils";
|
||||
import { resolveMcpAppsServers } from "./mcp-apps-servers";
|
||||
import { resolveIntelligenceUser } from "./resolve-intelligence-user";
|
||||
import { resolveWebMemory } from "./memory-policy";
|
||||
import { errorResponse } from "./json-response";
|
||||
@@ -124,13 +125,7 @@ export function configureAgentForRequest(params: {
|
||||
}
|
||||
|
||||
if (runtime.mcpApps?.servers?.length) {
|
||||
const mcpServers = runtime.mcpApps.servers
|
||||
.filter((server) => !server.agentId || server.agentId === agentId)
|
||||
.map((server) => {
|
||||
const mcpServer = { ...server };
|
||||
delete mcpServer.agentId;
|
||||
return mcpServer;
|
||||
});
|
||||
const mcpServers = resolveMcpAppsServers(runtime.mcpApps.servers, agentId);
|
||||
|
||||
if (mcpServers.length > 0 && typeof agent.use === "function") {
|
||||
agent.use(new MCPAppsMiddleware({ mcpServers }));
|
||||
|
||||
@@ -0,0 +1,44 @@
|
||||
import type { MCPClientConfig } from "@ag-ui/mcp-apps-middleware";
|
||||
import type { McpAppsServerConfig } from "../../core/runtime";
|
||||
|
||||
const UNSUPPORTED_POLICY_KEYS = ["includeTools", "excludeTools"] as const;
|
||||
const MCP_APPS_MIDDLEWARE_VERSION = "@ag-ui/mcp-apps-middleware@0.0.3";
|
||||
const MCP_APPS_POLICY_ISSUE =
|
||||
"https://github.com/CopilotKit/CopilotKit/issues/5930";
|
||||
|
||||
/**
|
||||
* Select the servers for an agent and validate the policy boundary owned by
|
||||
* the installed MCP Apps middleware.
|
||||
*/
|
||||
export function resolveMcpAppsServers(
|
||||
servers: readonly McpAppsServerConfig[],
|
||||
agentId: string,
|
||||
): MCPClientConfig[] {
|
||||
const violations: string[] = [];
|
||||
|
||||
for (const [index, server] of servers.entries()) {
|
||||
const config = server as unknown as Record<string, unknown>;
|
||||
for (const key of UNSUPPORTED_POLICY_KEYS) {
|
||||
if (
|
||||
Object.prototype.hasOwnProperty.call(config, key) &&
|
||||
config[key] !== undefined
|
||||
) {
|
||||
violations.push(
|
||||
`${key} at server[${index}] (${server.serverId ?? server.url})`,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (violations.length > 0) {
|
||||
throw new Error(
|
||||
`Unsupported MCP Apps tool policy: ${violations.join(", ")}. ` +
|
||||
`${MCP_APPS_MIDDLEWARE_VERSION} owns per-server tool policy and ` +
|
||||
`does not support these keys; see ${MCP_APPS_POLICY_ISSUE}.`,
|
||||
);
|
||||
}
|
||||
|
||||
return servers
|
||||
.filter((server) => !server.agentId || server.agentId === agentId)
|
||||
.map(({ agentId: _agentId, ...server }) => server as MCPClientConfig);
|
||||
}
|
||||
@@ -182,6 +182,8 @@ const runtime = new CopilotRuntime({
|
||||
|
||||
Each server entry optionally accepts an `agentId` field to scope that server to a single agent. Without it, the server is available to all agents.
|
||||
|
||||
Per-tool filtering belongs to `@ag-ui/mcp-apps-middleware`. The runtime currently pins version `0.0.3`, which does not support `includeTools` or `excludeTools`. Supplying either key raises a configuration error instead of silently ignoring it; use a compatible middleware release when that upstream policy contract is available.
|
||||
|
||||
## Forwarding request headers to your agent
|
||||
|
||||
When a request reaches the runtime, some inbound headers are forwarded onto the outgoing call to your agent (the `/run` path that actually dispatches the agent). This is how a token configured by the frontend provider reaches a self-hosted agent — see [Authentication](/auth).
|
||||
|
||||
@@ -49,6 +49,11 @@ export default { fetch: handler };
|
||||
Each server entry accepts an optional `agentId`. When set, the server's tools are only
|
||||
exposed to that agent. Omit it to expose to all agents.
|
||||
|
||||
Per-tool filtering belongs to `@ag-ui/mcp-apps-middleware`. The runtime currently pins
|
||||
version `0.0.3`, which does not support `includeTools` or `excludeTools`. Supplying either
|
||||
key raises a configuration error instead of silently ignoring it; use a compatible
|
||||
middleware release when that upstream policy contract is available.
|
||||
|
||||
## Gotcha — do NOT put MCP under agents
|
||||
|
||||
```typescript
|
||||
|
||||
Reference in New Issue
Block a user