fix(connector-mcp)!: refuse an unreadable or unknown providerConfig.include instead of exposing every tool

.changeset/27402-mcp-include-refuses.md

@@ -0,0 +1,21 @@
+---
+'@objectstack/connector-mcp': minor
+---
+
+fix(connector-mcp)!: a declarative `mcp` instance refuses a `providerConfig.include` it cannot read, and an entry naming a tool the server does not list, instead of exposing every tool
+
+Clause-②: no (narrowing)
+Metadata-protocol: narrowed
+
+<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped: `providerConfig` is an open record the `mcp` provider reads itself, and `include` keeps its name and its allowlist meaning, now the same as the `openapi` provider's. What changes is that two values the provider used to drop silently are refused at materialization, each with a message naming the entry and the fix, so there is nothing for a metadata migration to rewrite and no tombstone. -->
+
+**BREAKING** accept-set narrowing of `providerConfig.include` on `provider: 'mcp'` connector instances (`@objectstack/connector-mcp`).
+
+- **Before.** An `include` that was not an array (a string such as `include: 'web_search'`, an object, `null`) was dropped and read as "no allowlist": the instance exposed **every** tool the server lists. Non-string entries were filtered out silently (`['web_search', 7]` exposed `web_search`). An entry naming a tool the server does not list was ignored: `include: ['nope']` brought the instance up `ready` with no actions and no error.
+- **Now.** `include` keeps the `openapi` provider's meaning: absent means every tool; otherwise it must be an array of non-empty tool names.
+  - Any other shape is refused before connecting, with `connector-mcp provider: connector 'NAME' providerConfig.include must be an array of tool names (strings): ...`.
+  - An entry the server's `tools/list` does not carry is refused after listing, naming the entry and the tools the server lists (`... providerConfig.include names 'nope', which the MCP server does not list. ...`); the connection is closed first.
+  - Both are configuration faults, thrown plain: fatal at boot, a skipped entry on a reload. The instance never falls back to exposing every tool.
+- **Who sees it.** An app whose `mcp` connector instance writes `include` as anything but an array of tool names, or lists a tool its server does not (a typo, or a tool the server renamed or dropped).
+
+**The remedy.** Write `include` as an array of the server's tool names (`include: ['web_search']`), or remove it to expose every tool. For an entry the refusal names, use a name from the tool list the refusal prints, or delete the entry.

content/docs/automation/connectors.mdx

@@ -231,6 +231,16 @@ connect, handshake or `tools/list`) is a configuration fault instead: the
 instance is degraded naming the refusal, as described under *A credential the
 upstream refuses* below.
 
+`include` has the same allowlist meaning as the `openapi` provider's: list the
+tools the instance should expose and every other tool stays out — the way to
+keep a destructive tool away from your flows and agents. It must be an array of
+tool names; any other shape (a string such as `include: 'web_search'`, an
+object) fails the boot before connecting, and never falls back to exposing
+every tool. An entry the server's `tools/list` does not carry — a typo, or a
+tool the server renamed or dropped — fails the boot with an error naming the
+entry and listing the server's tools; it is never ignored. Leave `include` out
+to expose every tool.
+
 <Callout type="warn">
 **Declarative stdio transports are denied by default.** Materializing one
 spawns a local process *from metadata* — which a runtime Studio publish can

packages/connectors/connector-mcp/src/mcp-provider.test.ts

@@ -5,12 +5,15 @@
 // fake client here), listing its tools, and mapping them to actions.
 
 import { describe, it, expect } from 'vitest';
+import { LiteKernel } from '@objectstack/core';
+import { AutomationServicePlugin, type AutomationEngine } from '@objectstack/service-automation';
 import { UnauthorizedError } from '@modelcontextprotocol/sdk/client/auth.js';
 import { StreamableHTTPError } from '@modelcontextprotocol/sdk/client/streamableHttp.js';
 import { McpError } from '@modelcontextprotocol/sdk/types.js';
 import type { ConnectorProviderContext } from '@objectstack/spec/integration';
 import { isConnectorCredentialRejected, isConnectorUpstreamUnavailable } from '@objectstack/spec/integration';
 import type { McpClientLike, McpToolDescriptor, McpTransport } from './mcp-connector.js';
+import { ConnectorMcpPlugin } from './connector-mcp-plugin.js';
 import { createMcpProviderFactory, MCP_PROVIDER_KEY } from './mcp-provider.js';
 
 const TOOLS: McpToolDescriptor[] = [
@@ -364,3 +367,143 @@ describe('mcp provider: declarative stdio working directory (#22423)', () => {
         expect(seen.transport).toBeUndefined();
     });
 });
+
+// #27402 — `providerConfig.include` is the allowlist that keeps destructive
+// tools away from flows and agents, with the `openapi` provider's one meaning
+// (#26709, #27387): a shape it cannot read is refused before connecting, an
+// entry naming a tool the server does not list is refused after listing, and
+// the instance never falls back to exposing every tool.
+describe('mcp provider: providerConfig.include refuses loudly (#27402)', () => {
+    const SERVER_TOOLS: McpToolDescriptor[] = [
+        { name: 'web_search', description: 'Search the web' },
+        { name: 'delete_everything', description: 'Destructive' },
+    ];
+    const http = { kind: 'http', url: 'https://mcp.vendor.example/mcp' };
+
+    /** A fake server listing SERVER_TOOLS; records whether it was connected and closed. */
+    function server() {
+        const state = { connected: false, closed: false };
+        const clientFactory = async (): Promise<McpClientLike> => {
+            state.connected = true;
+            return {
+                listTools: async () => SERVER_TOOLS,
+                callTool: async () => ({ content: [] }),
+                close: async () => { state.closed = true; },
+            };
+        };
+        return { clientFactory, state };
+    }
+
+    async function refusal(include: unknown) {
+        const { clientFactory, state } = server();
+        const factory = createMcpProviderFactory({ clientFactory });
+        const err: unknown = await Promise.resolve(
+            factory(ctx({ name: 'vendor', providerConfig: { transport: http, include } })),
+        ).then(
+            (mat) => { throw new Error(`expected a refusal, got actions ${Object.keys(mat.handlers).join(', ')}`); },
+            (e: unknown) => e,
+        );
+        return { err, state };
+    }
+
+    /** A configuration fault: plain, neither retryable nor a credential refusal. */
+    function expectConfigurationFault(err: unknown) {
+        expect(err).toBeInstanceOf(Error);
+        expect(isConnectorUpstreamUnavailable(err)).toBe(false);
+        expect(isConnectorCredentialRejected(err)).toBe(false);
+    }
+
+    it.each([
+        ['a string', 'web_search'],
+        ['an array with a non-string entry', ['web_search', 7]],
+        ['an array with an empty name', ['web_search', '']],
+        ['an object', { web_search: true }],
+        ['null', null],
+    ])('refuses an include that is %s, before connecting', async (_shape, include) => {
+        const { err, state } = await refusal(include);
+        expectConfigurationFault(err);
+        expect((err as Error).message).toMatch(
+            /^connector-mcp provider: connector 'vendor' providerConfig\.include must be an array of tool names \(strings\)/,
+        );
+        expect(state.connected).toBe(false);
+    });
+
+    it('refuses an entry naming a tool the server does not list, naming it and the listed tools, and closes the client', async () => {
+        const { err, state } = await refusal(['nope']);
+        expectConfigurationFault(err);
+        expect((err as Error).message).toMatch(
+            /^connector-mcp provider: connector 'vendor' providerConfig\.include names 'nope', which the MCP server does not list\./,
+        );
+        expect((err as Error).message).toContain("the server lists 2 tool(s): 'web_search', 'delete_everything'.");
+        expect(state.closed).toBe(true);
+    });
+
+    it('refuses when one entry is listed and another is not: no partial instance', async () => {
+        const { err } = await refusal(['web_search', 'nope', 'nope']);
+        expectConfigurationFault(err);
+        expect((err as Error).message).toContain("providerConfig.include names 'nope', which");
+    });
+
+    it('an include of listed tools exposes exactly those tools', async () => {
+        const { clientFactory, state } = server();
+        const mat = await createMcpProviderFactory({ clientFactory })(
+            ctx({ name: 'vendor', providerConfig: { transport: http, include: ['web_search'] } }),
+        );
+        expect(Object.keys(mat.handlers)).toEqual(['web_search']);
+        expect(mat.def.actions?.map((a) => a.key)).toEqual(['web_search']);
+        expect(state.closed).toBe(false);
+    });
+
+    it('control: no include exposes every tool the server lists', async () => {
+        const { clientFactory } = server();
+        const mat = await createMcpProviderFactory({ clientFactory })(
+            ctx({ name: 'vendor', providerConfig: { transport: http } }),
+        );
+        expect(Object.keys(mat.handlers)).toEqual(['web_search', 'delete_everything']);
+    });
+
+    // Through the materializer: a refused include fails the boot naming the
+    // instance and the fault, and no connector exposing the server's tools is
+    // ever registered under the declared name.
+    it('a declarative instance with a string include fails boot, never registering every tool', async () => {
+        const { clientFactory, state } = server();
+        const declared = [
+            {
+                name: 'vendor_mcp',
+                label: 'Vendor MCP',
+                type: 'api',
+                provider: 'mcp',
+                providerConfig: { transport: http, include: 'web_search' },
+            },
+        ];
+        const kernel = new LiteKernel({ logger: { level: 'silent' } } as never);
+        kernel.use(new AutomationServicePlugin());
+        kernel.use({
+            name: 'test.connector-metadata',
+            type: 'standard',
+            version: '1.0.0',
+            dependencies: ['com.objectstack.service-automation'],
+            async init(c: { registerService(name: string, service: unknown): void }) {
+                c.registerService('objectql', {
+                    registry: { listItems: (type: string) => (type === 'connector' ? declared : []) },
+                });
+            },
+            async start() {},
+        } as never);
+        kernel.use(new ConnectorMcpPlugin({ clientFactory }));
+        try {
+            const booted: unknown = await kernel.bootstrap().then(() => undefined, (e: unknown) => e);
+            expect(booted).toBeInstanceOf(Error);
+            expect((booted as Error).message).toContain(
+                "failed to materialize connector instance 'vendor_mcp' via provider 'mcp'",
+            );
+            expect((booted as Error).message).toContain('providerConfig.include must be an array of tool names');
+            expect(state.connected).toBe(false);
+            const engine = kernel.getService('automation') as AutomationEngine | undefined;
+            const descriptor = engine?.getConnectorDescriptors().find((d) => d.name === 'vendor_mcp');
+            expect(descriptor?.actions ?? []).toEqual([]);
+        } finally {
+            await kernel.shutdown().catch(() => {});
+        }
+    });
+});

packages/connectors/connector-mcp/src/mcp-provider.ts

@@ -208,6 +208,13 @@ async function credentialRejectionStatus(err: unknown): Promise<number | undefin
  * a configuration fault the materializer degrades the instance for (see
  * {@link credentialRejectionStatus}, #27237). A handshake that carried the
  * instance's credential answers `credentialVerified`.
+ *
+ * `providerConfig.include` (optional) narrows the actions to an allowlist of
+ * tool names, with the `openapi` provider's meaning. An `include` that is not
+ * an array of non-empty strings is refused before connecting, and an entry
+ * naming a tool the server's `tools/list` does not carry is refused after
+ * listing (the connection is closed first); both are configuration faults,
+ * thrown plain. The instance never falls back to exposing every tool.
  */
 export function createMcpProviderFactory(deps: McpProviderDeps = {}): ConnectorProviderFactory {
   return async (ctx) => {
@@ -222,10 +229,33 @@ export function createMcpProviderFactory(deps: McpProviderDeps = {}): ConnectorP
         transport = { ...transport, cwd: await ctx.resolvePackagePath('.') };
       }
     }
-    const includeList = Array.isArray(cfg.include)
-      ? cfg.include.filter((x): x is string => typeof x === 'string')
+    // `include` keeps the `openapi` provider's meaning (one allowlist, two
+    // providers): absent ⇒ every tool; otherwise an array of non-empty tool
+    // names. Any other shape is a configuration fault, thrown plain before
+    // connecting — fatal at boot, a skipped entry on reload. An allowlist that
+    // dropped a shape it could not read would expose every tool the server
+    // has, the very ones the author meant to keep out of flows and agents.
+    if (
+      cfg.include !== undefined &&
+      (!Array.isArray(cfg.include) ||
+        cfg.include.some((entry) => typeof entry !== 'string' || entry.length === 0))
+    ) {
+      throw new Error(
+        `connector-mcp provider: connector '${ctx.name}' providerConfig.include must be an array of ` +
+          `tool names (strings): each the name of a tool the MCP server lists in tools/list.`,
+      );
+    }
+    const includeList = cfg.include as readonly string[] | undefined;
+    // The allowlist rides the connector's own `include` predicate; every tool
+    // name the server lists is recorded on the way, so an entry nothing
+    // matched can be named in the refusal below.
+    const listedTools: string[] = [];
+    const include = includeList
+      ? (toolName: string) => {
+          listedTools.push(toolName);
+          return includeList.includes(toolName);
+        }
       : undefined;
-    const include = includeList ? (toolName: string) => includeList.includes(toolName) : undefined;
 
     let bundle;
     try {
@@ -270,6 +300,29 @@ export function createMcpProviderFactory(deps: McpProviderDeps = {}): ConnectorP
         { cause: err },
       );
     }
+    if (includeList) {
+      // An entry naming a tool the server does not list is a configuration
+      // fault, thrown plain — fatal at boot, a skipped entry on reload — after
+      // the connection is released. Ignoring it would bring the instance up
+      // without the tool the author listed (a typo, or a tool the server
+      // renamed or dropped), and the first sign would be a flow failing at
+      // run time.
+      const offered = new Set(listedTools);
+      const unknown = [...new Set(includeList)].filter((entry) => !offered.has(entry));
+      if (unknown.length > 0) {
+        await bundle.close().catch(() => {});
+        // At most 20 of the server's tool names, as the openapi refusal lists.
+        const sample = [...offered].slice(0, 20);
+        const more = offered.size > sample.length ? `, … ${offered.size - sample.length} more` : '';
+        throw new Error(
+          `connector-mcp provider: connector '${ctx.name}' providerConfig.include names ` +
+            `${unknown.map((entry) => `'${entry}'`).join(', ')}, which the MCP server does not list. ` +
+            `Each entry must be the name of a tool from the server's tools/list; ` +
+            `the server lists ${offered.size} tool(s): ` +
+            `${sample.map((name) => `'${name}'`).join(', ')}${more}.`,
+        );
+      }
+    }
     // [#27237] The handshake carried the instance's credential only over
     // http, and only as a header (an api-key `paramName` is not sent): then
     // the server's answer proved it.