diff --git a/.changeset/adr-0097-stdio-policy-gate.md b/.changeset/adr-0097-stdio-policy-gate.md new file mode 100644 index 0000000000..25e54ec108 --- /dev/null +++ b/.changeset/adr-0097-stdio-policy-gate.md @@ -0,0 +1,23 @@ +--- +'@objectstack/connector-mcp': minor +--- + +feat(connector-mcp)!: policy-gate declarative stdio transports — default-deny + host allowlist (#3055) + +A declarative `provider: 'mcp'` entry with a stdio transport spawns a local +child process **from metadata** — which a runtime Studio publish can introduce. +Declarative stdio is now **denied by default**; hosts opt in deliberately: + +```ts +new ConnectorMcpPlugin({ declarativeStdio: ['my-mcp-server'] }) // command allowlist +new ConnectorMcpPlugin({ declarativeStdio: true }) // allow any (full trust) +``` + +Behavior change: a declarative stdio instance that materialized before now +fails as a **configuration fault** (fatal at boot / skipped on reload) with an +actionable opt-in message, and is never classified upstream-unavailable — a +security rejection must not be retried into existence. `http` transports and +**hand-wired** connectors (plugin instance options / `createMcpConnector`) are +unaffected. This is the security precondition for shipping `connector-mcp` in +default presets (#3056); see ADR-0097 §"Declarative stdio policy" and +ADR-0024 §4. diff --git a/docs/adr/0097-declarative-connector-instances.md b/docs/adr/0097-declarative-connector-instances.md index 89a7039289..e44926051e 100644 --- a/docs/adr/0097-declarative-connector-instances.md +++ b/docs/adr/0097-declarative-connector-instances.md @@ -167,6 +167,31 @@ dispatch was rejected: MCP actions ARE the server's `tools/list`, so a connector materialized without connecting would register a zero-action def — exactly the plausible-but-dead shape this ADR exists to kill. +### Declarative stdio policy: default-deny (follow-up, landed — #3055) + +A declarative `provider: 'mcp'` entry may name a **stdio transport**, and +materializing one **spawns a local child process** — from *metadata*, which a +runtime Studio publish can introduce (the reload reconcile materializes new +instances; soft mode skips failures, not successes). Anyone who can publish +metadata could otherwise execute commands on the server. Stdio on declarative +instances is therefore **denied by default** and opt-in per host: + +```ts +new ConnectorMcpPlugin({ declarativeStdio: ['my-mcp-server'] }) // allowlist +new ConnectorMcpPlugin({ declarativeStdio: true }) // full trust +``` + +A policy violation is a **configuration fault** — plain throw (fatal at boot, +skipped on reload), never `CONNECTOR_UPSTREAM_UNAVAILABLE`: a security +rejection must not be retried into existence. `http` transports are unaffected +(same trust class as the `http` node), and so are **hand-wired** connectors +(plugin instance options / `createMcpConnector`) — their command lives in host +code, a different trust anchor than metadata. The allowlist is deliberately +honest about being a *coarse* boundary (allowlisting `npx` ≈ allowing any +package it can run); sandboxed execution remains the enterprise tier +(ADR-0024 §4). This gate is the precondition for shipping `connector-mcp` in +any default preset (#3056). + ### Deliberate scope boundaries - **`providerConfig.spec` (openapi)** accepts an inline document, an http(s) URL, **or a package-relative file path** (#3016 follow-up). The connector still owns no filesystem access: the automation service injects a `loadPackageFile` capability into `ConnectorProviderContext` that resolves the ref against the declaring stack/package root (`packageRoot`, CLI default: the `objectstack.config.ts` directory) and **confines reads to that root** — absolute and `..`-escaping paths are rejected. Read/parse failures follow the reconcile policy above: fatal at boot, skipped on reload. diff --git a/packages/connectors/connector-mcp/src/connector-mcp-plugin.test.ts b/packages/connectors/connector-mcp/src/connector-mcp-plugin.test.ts index f8a1029375..3102163601 100644 --- a/packages/connectors/connector-mcp/src/connector-mcp-plugin.test.ts +++ b/packages/connectors/connector-mcp/src/connector-mcp-plugin.test.ts @@ -41,6 +41,9 @@ describe('ConnectorMcpPlugin — end to end with the automation engine', () => { kernel.use(new AutomationServicePlugin()); kernel.use( new ConnectorMcpPlugin({ + // NOTE deliberately no `declarativeStdio`: the #3055 default-deny + // policy gates DECLARATIVE instances only — this hand-wired stdio + // transport (host code, not metadata) must keep working un-gated. name: 'github_mcp', label: 'GitHub MCP', transport: { kind: 'stdio', command: 'noop' }, @@ -93,4 +96,32 @@ describe('ConnectorMcpPlugin — end to end with the automation engine', () => { await kernel.shutdown(); expect(isClosed()).toBe(true); }); + + it('plumbs declarativeStdio through to the registered provider factory (#3055)', async () => { + const { client } = fakeClient(); + let registered: ((ctx: unknown) => Promise) | undefined; + const automationStub = { + registerConnector: () => {}, + unregisterConnector: () => {}, + registerConnectorProvider: (_key: string, factory: never) => { registered = factory; }, + }; + const plugin = new ConnectorMcpPlugin({ + clientFactory: async () => client, + declarativeStdio: ['trusted-mcp'], + }); + await plugin.init({ + getService: () => automationStub, + logger: { info: () => {}, warn: () => {} }, + } as never); + expect(registered).toBeDefined(); + + const provider = registered!; + const declarativeCtx = (command: string) => ({ + name: 'x', label: 'X', type: 'api', + providerConfig: { transport: { kind: 'stdio', command } }, + }); + // Allowlisted command materializes; anything else is denied by policy. + await expect(provider(declarativeCtx('trusted-mcp'))).resolves.toBeDefined(); + await expect(provider(declarativeCtx('bash'))).rejects.toThrow(/declarativeStdio allowlist/); + }); }); diff --git a/packages/connectors/connector-mcp/src/connector-mcp-plugin.ts b/packages/connectors/connector-mcp/src/connector-mcp-plugin.ts index a2b31d1cbd..a4cced126c 100644 --- a/packages/connectors/connector-mcp/src/connector-mcp-plugin.ts +++ b/packages/connectors/connector-mcp/src/connector-mcp-plugin.ts @@ -3,7 +3,11 @@ import type { Plugin, PluginContext } from '@objectstack/core'; import type { Connector, ConnectorProviderFactory } from '@objectstack/spec/integration'; import { createMcpConnector, type McpConnectorOptions } from './mcp-connector.js'; -import { createMcpProviderFactory, MCP_PROVIDER_KEY } from './mcp-provider.js'; +import { + createMcpProviderFactory, + MCP_PROVIDER_KEY, + type McpDeclarativeStdioPolicy, +} from './mcp-provider.js'; /** * Minimal surface of the automation engine this plugin depends on — the @@ -29,7 +33,17 @@ export interface ConnectorRegistrySurface { * stack can declare `provider: 'mcp'` instances as pure metadata. Supply a * `transport` to ALSO connect one hand-wired MCP server at `start()`. */ -export interface ConnectorMcpPluginOptions extends Partial {} +export interface ConnectorMcpPluginOptions extends Partial { + /** + * Policy for stdio transports on **declarative** `provider: 'mcp'` + * instances (#3055). Default **deny**: metadata (including a runtime Studio + * publish) must not spawn local processes unless the host opts in. + * `string[]` allowlists specific commands; `true` allows any. Hand-wired + * connectors configured via these plugin options are not subject to it — + * their command lives in host code, not metadata. + */ + declarativeStdio?: McpDeclarativeStdioPolicy; +} /** * ConnectorMcpPlugin — contributes the generic MCP adapter (ADR-0024) in two forms: @@ -71,7 +85,10 @@ export class ConnectorMcpPlugin implements Plugin { if (automation && typeof automation.registerConnectorProvider === 'function') { automation.registerConnectorProvider( MCP_PROVIDER_KEY, - createMcpProviderFactory({ clientFactory: this.options.clientFactory }), + createMcpProviderFactory({ + clientFactory: this.options.clientFactory, + declarativeStdio: this.options.declarativeStdio, + }), ); ctx.logger.info("ConnectorMcpPlugin: registered 'mcp' connector provider"); } diff --git a/packages/connectors/connector-mcp/src/index.ts b/packages/connectors/connector-mcp/src/index.ts index d9919e4f2e..2999138e24 100644 --- a/packages/connectors/connector-mcp/src/index.ts +++ b/packages/connectors/connector-mcp/src/index.ts @@ -35,4 +35,5 @@ export { createMcpProviderFactory, MCP_PROVIDER_KEY, type McpProviderDeps, + type McpDeclarativeStdioPolicy, } from './mcp-provider.js'; diff --git a/packages/connectors/connector-mcp/src/mcp-provider.test.ts b/packages/connectors/connector-mcp/src/mcp-provider.test.ts index 639ebc7c7d..ccb27bca92 100644 --- a/packages/connectors/connector-mcp/src/mcp-provider.test.ts +++ b/packages/connectors/connector-mcp/src/mcp-provider.test.ts @@ -41,7 +41,8 @@ describe('mcp provider factory (ADR-0097)', () => { it('connects, lists tools, and maps them to actions', async () => { const { factory: clientFactory } = fakeClientFactory(); - const factory = createMcpProviderFactory({ clientFactory }); + // stdio on a declarative instance requires the host opt-in (#3055). + const factory = createMcpProviderFactory({ clientFactory, declarativeStdio: ['my-mcp'] }); const mat = await factory(ctx({ providerConfig: { transport: { kind: 'stdio', command: 'my-mcp' } } })); expect(mat.def.name).toBe('github'); expect(Object.keys(mat.handlers).sort()).toEqual(['create_issue', 'list_issues']); @@ -50,7 +51,7 @@ describe('mcp provider factory (ADR-0097)', () => { it('applies the tool allowlist from providerConfig.include', async () => { const { factory: clientFactory } = fakeClientFactory(); - const factory = createMcpProviderFactory({ clientFactory }); + const factory = createMcpProviderFactory({ clientFactory, declarativeStdio: ['my-mcp'] }); const mat = await factory( ctx({ providerConfig: { transport: { kind: 'stdio', command: 'my-mcp' }, include: ['create_issue'] } }), ); @@ -88,11 +89,12 @@ describe('mcp provider factory (ADR-0097)', () => { describe('mcp provider fault classification (#3017)', () => { const stdio = { transport: { kind: 'stdio', command: 'my-mcp' } }; + const allowMyMcp = { declarativeStdio: ['my-mcp'] }; it('classifies a connect failure as upstream-unavailable (retryable), keeping the cause', async () => { const boom = new Error('connect ECONNREFUSED 127.0.0.1:9999'); const clientFactory = async (): Promise => { throw boom; }; - const factory = createMcpProviderFactory({ clientFactory }); + const factory = createMcpProviderFactory({ clientFactory, ...allowMyMcp }); const err = await factory(ctx({ providerConfig: stdio })).then( () => { throw new Error('expected rejection'); }, @@ -111,7 +113,7 @@ describe('mcp provider fault classification (#3017)', () => { callTool: async () => ({}), close: async () => { closed = true; }, }); - const factory = createMcpProviderFactory({ clientFactory }); + const factory = createMcpProviderFactory({ clientFactory, ...allowMyMcp }); const err = await factory(ctx({ providerConfig: stdio })).then( () => { throw new Error('expected rejection'); }, @@ -130,3 +132,62 @@ describe('mcp provider fault classification (#3017)', () => { expect(isConnectorUpstreamUnavailable(err)).toBe(false); }); }); + +// ── #3055 — declarative stdio policy: default-deny + host allowlist ───────── +// +// A declarative stdio transport spawns a local process from metadata (a Studio +// publish reaches materialization at runtime), so it is gated OFF unless the +// host opts in. Violations are CONFIGURATION faults: plain throw (fatal at +// boot, skipped on reload) — never upstream-unavailable, which would retry a +// security rejection into existence. + +describe('mcp provider declarative stdio policy (#3055)', () => { + const stdioCfg = { transport: { kind: 'stdio', command: 'my-mcp' } }; + + it('DENIES a declarative stdio transport by default, as a plain (non-retryable) fault', async () => { + const { factory: clientFactory, seen } = fakeClientFactory(); + const factory = createMcpProviderFactory({ clientFactory }); // no policy + const err = await factory(ctx({ providerConfig: stdioCfg })).then( + () => { throw new Error('expected rejection'); }, + (e: unknown) => e, + ); + expect((err as Error).message).toMatch(/stdio transports are disabled by default/); + expect((err as Error).message).toContain("declarativeStdio: ['my-mcp']"); // actionable opt-in hint + expect(isConnectorUpstreamUnavailable(err)).toBe(false); + expect(seen.transport).toBeUndefined(); // rejected before any connection attempt + }); + + it('allowlist admits exactly the listed command and rejects others', async () => { + const { factory: clientFactory } = fakeClientFactory(); + const factory = createMcpProviderFactory({ clientFactory, declarativeStdio: ['npx', 'my-mcp'] }); + const mat = await factory(ctx({ providerConfig: stdioCfg })); + expect(mat.def.name).toBe('github'); + + const err = await factory( + ctx({ providerConfig: { transport: { kind: 'stdio', command: 'bash' } } }), + ).then( + () => { throw new Error('expected rejection'); }, + (e: unknown) => e, + ); + expect((err as Error).message).toMatch(/not in the host's declarativeStdio allowlist \[npx, my-mcp\]/); + expect(isConnectorUpstreamUnavailable(err)).toBe(false); + }); + + it('declarativeStdio: true allows any command (explicit full trust)', async () => { + const { factory: clientFactory } = fakeClientFactory(); + const factory = createMcpProviderFactory({ clientFactory, declarativeStdio: true }); + const mat = await factory( + ctx({ providerConfig: { transport: { kind: 'stdio', command: 'anything' } } }), + ); + expect(Object.keys(mat.handlers).length).toBeGreaterThan(0); + }); + + it('http transports are NOT subject to the policy', async () => { + const { factory: clientFactory } = fakeClientFactory(); + const factory = createMcpProviderFactory({ clientFactory }); // default-deny policy in force + const mat = await factory( + ctx({ providerConfig: { transport: { kind: 'http', url: 'https://mcp.example.com' } } }), + ); + expect(mat.def.name).toBe('github'); + }); +}); diff --git a/packages/connectors/connector-mcp/src/mcp-provider.ts b/packages/connectors/connector-mcp/src/mcp-provider.ts index 04924e02b9..0f8484abe8 100644 --- a/packages/connectors/connector-mcp/src/mcp-provider.ts +++ b/packages/connectors/connector-mcp/src/mcp-provider.ts @@ -10,10 +10,34 @@ import { createMcpConnector, type McpConnectorOptions, type McpTransport } from */ export const MCP_PROVIDER_KEY = 'mcp'; +/** + * Host policy for **declarative** stdio transports (#3055). A stdio transport + * launches a local child process, and declarative entries arrive through + * metadata — including a runtime Studio publish — so spawning from them is + * gated OFF by default: + * + * - `undefined` / `false` — deny (default): a `provider: 'mcp'` entry with a + * stdio transport is rejected as a configuration fault. + * - `string[]` — allowlist: the transport's `command` must strictly equal one + * of the listed commands. NOTE this is a coarse trust boundary — listing a + * launcher like `npx` effectively allows any package it can run; list the + * specific server binaries you trust. Sandboxed execution is the enterprise + * tier (ADR-0024 §4). + * - `true` — allow any command (explicit full trust; hosts that treat every + * metadata author as an operator). + * + * Hand-wired connectors (plugin instance options / `createMcpConnector`) are + * NOT subject to this policy: their command was written in host code, a + * different trust anchor than metadata. + */ +export type McpDeclarativeStdioPolicy = boolean | string[]; + /** Injectable dependencies for {@link createMcpProviderFactory} (tests). */ export interface McpProviderDeps { /** Injected MCP client factory; defaults to the SDK-backed client. */ clientFactory?: McpConnectorOptions['clientFactory']; + /** Policy for declarative stdio transports (#3055). Default: deny. */ + declarativeStdio?: McpDeclarativeStdioPolicy; } /** Shape of `providerConfig` for a `provider: 'mcp'` declarative instance. */ @@ -96,6 +120,35 @@ function normalizeTransport( ); } +/** + * Enforce the {@link McpDeclarativeStdioPolicy} for one declarative instance + * (#3055). Throws a **plain** Error on violation: a security-policy rejection + * is a configuration fault — fatal at boot, skipped+logged on reload — and must + * never be classified upstream-unavailable (it cannot be retried into + * existence). + */ +function assertDeclarativeStdioAllowed( + policy: McpDeclarativeStdioPolicy | undefined, + command: string, + connectorName: string, +): void { + if (policy === true) return; + if (Array.isArray(policy)) { + if (policy.includes(command)) return; + throw new Error( + `connector-mcp provider: connector '${connectorName}' declares a stdio transport with command '${command}', ` + + `which is not in the host's declarativeStdio allowlist [${policy.join(', ')}]. ` + + `Add the command to new ConnectorMcpPlugin({ declarativeStdio: [...] }) if this server is trusted (#3055).`, + ); + } + throw new Error( + `connector-mcp provider: connector '${connectorName}' declares a stdio transport (command '${command}'), ` + + `but declarative stdio transports are disabled by default — a stdio transport launches a local process ` + + `from stack metadata (including runtime Studio publishes). If this server is trusted, opt in deliberately: ` + + `new ConnectorMcpPlugin({ declarativeStdio: ['${command}'] }) — or use an http transport (#3055, ADR-0024 §4).`, + ); +} + /** * Build the `mcp` {@link ConnectorProviderFactory} (ADR-0097 / ADR-0024). At boot * the automation service invokes it for each `provider: 'mcp'` declarative @@ -104,6 +157,9 @@ function normalizeTransport( * {@link createMcpConnector} builds for a hand-wired MCP connector — one action * per tool, dispatched to the server's `tools/call`. * + * Stdio transports on declarative instances are policy-gated (default deny) — + * see {@link McpDeclarativeStdioPolicy} (#3055). + * * The connection is opened at materialization. Faults are classified (#3017): * an invalid transport shape is a *configuration* fault and throws plain — * fatal at boot per the ADR-0097 fail-loud contract — while a connect / @@ -116,6 +172,9 @@ export function createMcpProviderFactory(deps: McpProviderDeps = {}): ConnectorP return async (ctx) => { const cfg = (ctx.providerConfig ?? {}) as McpProviderConfig; const transport = normalizeTransport(cfg.transport, ctx.name, ctx.auth); + if (transport.kind === 'stdio') { + assertDeclarativeStdioAllowed(deps.declarativeStdio, transport.command, ctx.name); + } const includeList = Array.isArray(cfg.include) ? cfg.include.filter((x): x is string => typeof x === 'string') : undefined;