diff --git a/.changeset/29341-connector-credential-literal.md b/.changeset/29341-connector-credential-literal.md index 8a511c345ce..4e4a489fed9 100644 --- a/.changeset/29341-connector-credential-literal.md +++ b/.changeset/29341-connector-credential-literal.md @@ -11,6 +11,6 @@ Metadata-protocol: none A declarative connector entry's four maps — `providerConfig.headers`, `providerConfig.defaultHeaders`, `providerConfig.transport.headers` and `providerConfig.transport.env` — are withheld whole on the metadata read path, but a literal credential in one of them is still stored in metadata. The same `credential-literal` predicate the `flow-credential-literal` advisory asks now judges those four maps and names each credential-shaped entry by its position (never its value), with the `auth.credentialRef` route: - a header entry: move it to the connector's `auth: { type: 'bearer' | 'api-key' | 'basic', credentialRef }`, which the `rest` provider and an `mcp` http transport send themselves; -- a stdio `transport.env` entry: a stdio MCP server takes no `auth`, so the warning says `auth.credentialRef` reaches the server only over an http transport. +- a stdio `transport.env` entry: move it to the connector's `auth: { type: 'bearer' | 'api-key' | 'basic', credentialRef }` and name the variable in `providerConfig.transport.credentialEnv`; the `mcp` provider hands the resolved credential to the stdio server in that variable itself. Always a `warning`; nothing is refused and every save lands. The rule runs on all three CLI commands and on the runtime publish gate for `connector` (`PUT /meta/connector/:name`), where it rides the save response's `advisories`. `os explain connector-credential-literal` carries the reasoning. diff --git a/.changeset/29434-mcp-stdio-credential-env.md b/.changeset/29434-mcp-stdio-credential-env.md new file mode 100644 index 00000000000..4e01a7f05a9 --- /dev/null +++ b/.changeset/29434-mcp-stdio-credential-env.md @@ -0,0 +1,18 @@ +--- +'@objectstack/connector-mcp': minor +'@objectstack/lint': patch +--- + +Clause-②: yes (widening) +Metadata-protocol: added + +`provider: 'mcp'`: a stdio MCP server can now take its credential from the secrets layer instead of a literal in `transport.env`. Declare the instance's `auth` with a `credentialRef` as for an http server, and name the environment variable the server reads in the new `providerConfig.transport.credentialEnv` key: the host resolves the reference (environment, Setup → Connector credentials, the host's resolver) and the provider hands the resolved secret (the `bearer` token, the `api-key` key, or the `basic` password) to the child in that variable. The value is never stored, logged, served on a metadata read or put in a message; a reference that resolves to nothing degrades the instance naming the reference and the server is not started. + +```ts +providerConfig: { transport: { kind: 'stdio', command: 'repo-mcp-server', credentialEnv: 'REPO_MCP_TOKEN' } }, +auth: { type: 'bearer', credentialRef: 'REPO_MCP_TOKEN' }, +``` + +Refused at materialization as configuration faults (a failed boot for the host's own entry): a `credentialEnv` that is not an environment-variable name, one on an `http` transport, one with no `bearer` / `api-key` / `basic` `auth`, and one naming a variable `transport.env` also sets. A stdio transport that names no `credentialEnv` is unchanged: it does not apply `auth`. + +`@objectstack/lint`: the `connector-credential-literal` advisory's finding for a credential literal in `transport.env` now steers to this route (`auth.credentialRef` with `transport.credentialEnv`) instead of telling the author to move to an http transport. diff --git a/content/docs/automation/connectors.mdx b/content/docs/automation/connectors.mdx index 82904afbd4d..c4316a4388e 100644 --- a/content/docs/automation/connectors.mdx +++ b/content/docs/automation/connectors.mdx @@ -228,9 +228,10 @@ allowlist — only the listed tools become actions) and **`toolsSnapshot`** HTTP. Resolved `auth` is folded into the request headers; a grant-bearing `auth` (`oauth2`, `app-credential`) is asked for on every request instead (see *Grant-bearing kinds* below). -- `{ kind: 'stdio', command, args?, env? }` — a **local child process**, - policy-gated (below). Credentials for a stdio server ride `transport.env`, - not `auth`. +- `{ kind: 'stdio', command, args?, env?, credentialEnv? }` — a **local child + process**, policy-gated (below). Resolved `auth` is handed to the server in + its environment, under the variable `credentialEnv` names (see + [*A stdio server's credential*](#a-stdio-servers-credential) below). {/* os:check */} ```typescript @@ -346,6 +347,49 @@ characters fails the boot before connecting, and so does a pinned tool that `include` leaves out — the instance never exposes it, so its pin would never be checked. +#### A stdio server's credential + +A stdio MCP server reads its credential from an environment variable, and the +value stays out of metadata the way an http server's does: declare `auth` with +a `credentialRef`, and name the variable in `transport.credentialEnv`. The +platform resolves the reference like any instance's (the environment, then a +credential set under Setup → Connector credentials, then the host's resolver) +and the provider hands the resolved secret to the child in that variable — the +token of a `bearer`, the key of an `api-key`, the password of a `basic` (whose +username, not a secret, goes in `env` if the server reads it). + +{/* os:check */} +```typescript +import { defineConnector } from '@objectstack/spec/integration'; + +export const RepoToolsConnector = defineConnector({ + name: 'repo_tools', + label: 'Repository Tools (MCP, stdio)', + type: 'api', + provider: 'mcp', + providerConfig: { + transport: { + kind: 'stdio', + command: 'repo-mcp-server', + env: { LOG_LEVEL: 'info' }, + credentialEnv: 'REPO_MCP_TOKEN', + }, + }, + auth: { type: 'bearer', credentialRef: 'REPO_MCP_TOKEN' }, +}); +``` + +The value is never stored, logged, served on a metadata read (which shows +`credentialRef` and `credentialEnv`, two names) or carried in a message; it +exists only in the environment the server is started with. A reference that +resolves to nothing never starts the server: the instance is degraded naming +the reference, and a credential set later starts it. These fail the boot: +a `credentialEnv` that is not a variable name, one on an `http` transport, +one with no `bearer` / `api-key` / `basic` `auth` to place, and one naming a +variable `env` also sets. A stdio transport that names no `credentialEnv` +does not apply `auth`. A literal credential typed into `env` instead is +reported by the `connector-credential-literal` advisory, which points here. + **Declarative stdio transports are denied by default.** Materializing one spawns a local process *from metadata* — which a runtime Studio publish can @@ -581,8 +625,9 @@ platform's to fill, so it is left out of the action's input schema, as `Authorization` is. An **`mcp`** instance carries a grant-bearing credential only over an `http` -transport (a stdio server takes its credentials in `transport.env`; a handle -on a stdio transport, or beside a static credential, fails the boot). Every +transport (a stdio server is handed a static credential once, at spawn, in +`transport.credentialEnv`; a handle on a stdio transport, or beside a static +credential, fails the boot). Every HTTP request of its MCP session asks the handle. Each organization a dispatch runs for opens its own session with its own token, so organizations never share one. The tools are listed at materialization, outside any run, so the diff --git a/packages/connectors/connector-mcp/src/mcp-connector.ts b/packages/connectors/connector-mcp/src/mcp-connector.ts index 8743dd57ec0..0b67fb519db 100644 --- a/packages/connectors/connector-mcp/src/mcp-connector.ts +++ b/packages/connectors/connector-mcp/src/mcp-connector.ts @@ -21,7 +21,10 @@ import type { Connector } from '@objectstack/spec/integration'; * **Credentials live with the MCP server, not in `ConnectorSchema`** (ADR-0024 * §3). The operator supplies `env` (stdio) / `headers` (http) which we pass * straight to the transport; they are never copied into the serialized `def` - * (which is exposed via discovery) and must never be logged. + * (which is exposed via discovery) and must never be logged. A declarative + * instance's resolved `auth` reaches them through the `mcp` provider factory: + * as a header over http, and over stdio in the variable its + * `transport.credentialEnv` names. * * **Trust:** launching a stdio server runs a local process. Sandboxed, * multi-tenant execution and managed secrets are the enterprise tier (ADR-0024 diff --git a/packages/connectors/connector-mcp/src/mcp-provider.ts b/packages/connectors/connector-mcp/src/mcp-provider.ts index f1eea11cb47..5a1d6443a07 100644 --- a/packages/connectors/connector-mcp/src/mcp-provider.ts +++ b/packages/connectors/connector-mcp/src/mcp-provider.ts @@ -79,8 +79,9 @@ function isStringRecord(v: unknown): v is Record { * Fold the resolved instance `auth` into an MCP **http** transport's headers * (ADR-0024 keeps MCP credentials with the transport). `credentialRef` has * already been resolved upstream, so this only maps the static credential to the - * right header. Not applied to stdio transports — a stdio server receives its - * credentials through `transport.env`. + * right header. Not applied to stdio transports — a stdio server receives the + * instance's credential in its environment, under the variable + * `transport.credentialEnv` names (see {@link credentialEnvOf}). */ function applyAuthToHeaders( auth: ResolvedConnectorAuth | undefined, @@ -101,11 +102,85 @@ function applyAuthToHeaders( } } -/** Validate + normalize `providerConfig.transport`, injecting resolved auth for http. */ +/** An environment variable name as POSIX shells spell one. */ +const ENV_NAME_RE = /^[A-Za-z_][A-Za-z0-9_]*$/; + +/** + * The secret of a resolved static credential — what a stdio server is handed + * in its environment: a `bearer` token, an `api-key` key, a `basic` password. + * An `api-key`'s `headerName` / `paramName` place the key on an http request + * and are not read here; a `basic` username is not a secret, and a server that + * needs it takes it from `transport.env`. + */ +function secretOf(auth: Exclude): string { + switch (auth.type) { + case 'bearer': + return auth.token; + case 'api-key': + return auth.key; + case 'basic': + return auth.password; + } +} + +/** + * `transport.credentialEnv` of a stdio transport, judged — the route a stdio + * server's credential takes out of metadata (ADR-0097 §3 at ADR-0024 §3's + * process boundary). The instance's `auth.credentialRef` is resolved by the + * host like any other instance's (the environment, the stored credential, the + * host's resolver; a reference nothing answers degrades the instance naming it + * before this factory runs), and the resolved secret is put into the child's + * environment under the variable `credentialEnv` names. The value is never + * stored, logged, put on the connector `def` or carried in a message: it + * exists only in the environment handed to the spawn. + * + * Refused plain (configuration faults), each naming the key and never a value: + * a `credentialEnv` that is not a variable name; one with no static `auth` to + * place (a grant-bearing kind is refused by the credential-handle check, with + * this route named); one naming a variable `transport.env` also sets, which + * would give that variable two values. `undefined` when the transport names + * none: then `auth` is not applied to the stdio server at all, as before. + */ +function credentialEnvOf( + t: Record, + env: Record | undefined, + connectorName: string, + auth: ResolvedConnectorAuth | undefined, + borrowsCredential: boolean, +): { name: string; value: string } | undefined { + const name = t.credentialEnv; + if (name === undefined) return undefined; + if (typeof name !== 'string' || !ENV_NAME_RE.test(name)) { + throw new Error( + `connector-mcp provider: connector '${connectorName}' providerConfig.transport.credentialEnv must be the name of ` + + `an environment variable (letters, digits and '_', not starting with a digit): the variable the stdio server ` + + `reads the instance's credential from.`, + ); + } + if (borrowsCredential) return undefined; + if (!auth || auth.type === 'none') { + throw new Error( + `connector-mcp provider: connector '${connectorName}' names providerConfig.transport.credentialEnv '${name}' but ` + + `declares no credential to place there. Give the entry auth: { type: 'bearer', credentialRef } (or 'api-key' / ` + + `'basic'): the provider passes the resolved credential to the server in '${name}'.`, + ); + } + if (env && Object.prototype.hasOwnProperty.call(env, name)) { + throw new Error( + `connector-mcp provider: connector '${connectorName}' sets environment variable '${name}' twice: in ` + + `providerConfig.transport.env and as providerConfig.transport.credentialEnv. Drop the env entry: the provider ` + + `passes the resolved credential to the server in '${name}' itself.`, + ); + } + return { name, value: secretOf(auth) }; +} + +/** Validate + normalize `providerConfig.transport`, injecting resolved auth (http headers, stdio `credentialEnv`). */ function normalizeTransport( raw: unknown, connectorName: string, auth: ResolvedConnectorAuth | undefined, + borrowsCredential: boolean, ): McpTransport { if (!raw || typeof raw !== 'object') { throw new Error( @@ -120,14 +195,23 @@ function normalizeTransport( `connector-mcp provider: connector '${connectorName}' stdio transport requires a 'command' string.`, ); } + const env = isStringRecord(t.env) ? t.env : undefined; + const credential = credentialEnvOf(t, env, connectorName, auth, borrowsCredential); return { kind: 'stdio', command: t.command, args: Array.isArray(t.args) ? t.args.map((a) => String(a)) : undefined, - env: isStringRecord(t.env) ? t.env : undefined, + env: credential ? { ...env, [credential.name]: credential.value } : env, }; } if (t.kind === 'http') { + if (t.credentialEnv !== undefined) { + throw new Error( + `connector-mcp provider: connector '${connectorName}' names providerConfig.transport.credentialEnv on an http ` + + `transport. credentialEnv places the instance's credential in a stdio server's environment; over http the ` + + `provider sends the resolved auth as a request header itself. Delete the key.`, + ); + } if (typeof t.url !== 'string' || t.url.length === 0) { throw new Error( `connector-mcp provider: connector '${connectorName}' http transport requires a 'url' string.`, @@ -281,6 +365,13 @@ async function credentialRejectionStatus(err: unknown): Promise { const cfg = (ctx.providerConfig ?? {}) as McpProviderConfig; - let transport = normalizeTransport(cfg.transport, ctx.name, ctx.auth); + let transport = normalizeTransport(cfg.transport, ctx.name, ctx.auth, ctx.credentialHandle !== undefined); if (transport.kind === 'stdio') { assertDeclarativeStdioAllowed(deps.declarativeStdio, transport.command, ctx.name); // One anchor for the app's relative paths (#22423). A resolver that @@ -370,15 +461,17 @@ export function createMcpProviderFactory(deps: McpProviderDeps = {}): ConnectorP } // ADR-0143 D4 — a grant-bearing instance (`oauth2`, `app-credential`) // borrows its credential per request. Only an http transport carries one - // (a stdio server takes its credentials in `transport.env`), and a - // request is authenticated one way. + // (a stdio server is handed a static credential once, at spawn, in the + // variable `transport.credentialEnv` names), and a request is + // authenticated one way. const handle = ctx.credentialHandle; if (handle) { if (transport.kind !== 'http') { throw new Error( `connector-mcp provider: connector '${ctx.name}' borrows its credential per request (a grant-bearing auth kind), ` + - `which only an http transport can carry: a stdio server takes its credentials in transport.env. Declare an ` + - `http transport, or a static credential for the stdio one.`, + `which only an http transport can carry: a stdio server is handed its credential once, at spawn. Declare an ` + + `http transport, or a static credential for the stdio one (auth: { type: 'bearer', credentialRef }) with ` + + `providerConfig.transport.credentialEnv naming the variable the server reads it from.`, ); } const ways = [ @@ -504,7 +597,9 @@ export function createMcpProviderFactory(deps: McpProviderDeps = {}): ConnectorP } // [#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 was given to that credential. + // the server's answer was given to that credential. A stdio server handed + // the credential in `credentialEnv` is not counted: nothing in a stdio + // handshake says the server used, let alone accepted, its environment. // // A grant-bearing instance's borrowed token (`ctx.credentialHandle`) // reaches the server too, and is deliberately not counted here. A 403 the diff --git a/packages/connectors/connector-mcp/src/mcp-stdio-credential-env.test.ts b/packages/connectors/connector-mcp/src/mcp-stdio-credential-env.test.ts new file mode 100644 index 00000000000..6ce14bd5661 --- /dev/null +++ b/packages/connectors/connector-mcp/src/mcp-stdio-credential-env.test.ts @@ -0,0 +1,304 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #29434 — a stdio MCP server's credential, out of metadata: the instance's +// `auth.credentialRef` is resolved by the host, and the `mcp` provider hands +// the resolved secret to the child in the variable +// `providerConfig.transport.credentialEnv` names. +// +// The real-spawn cases run a dependency-free fixture through the SDK's +// `StdioClientTransport`. The fixture reports ONLY whether the variable is set +// and its length, as its one tool's description — never the value — so nothing +// here can copy the credential into a log or a snapshot. Every value below is +// an obvious placeholder. + +import { existsSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterAll, describe, expect, it } from 'vitest'; +import { LiteKernel } from '@objectstack/core'; +import { AutomationServicePlugin, type AutomationEngine } from '@objectstack/service-automation'; +import type { ConnectorCredentialHandle, ConnectorProviderContext } from '@objectstack/spec/integration'; +import { getMetadataTypeRedactor } from '@objectstack/spec/kernel'; +import { ConnectorMcpPlugin } from './connector-mcp-plugin.js'; +import type { McpClientLike, McpTransport } from './mcp-connector.js'; +import { createMcpProviderFactory } from './mcp-provider.js'; + +/** The variable the server reads its credential from. Not set in this process. */ +const CREDENTIAL_ENV = 'MCP_FIXTURE_TOKEN'; +/** An obvious placeholder, never a realistic secret. */ +const PLACEHOLDER = 'placeholder-credential-value'; + +/** + * A minimal MCP stdio server: argv[2] names the variable to report on, argv[3] + * is a file it writes when it starts (so a test can tell it never started). + */ +const FIXTURE_SERVER = ` +import { createInterface } from 'node:readline'; +import { writeFileSync } from 'node:fs'; +const [name, marker] = process.argv.slice(2); +if (marker) writeFileSync(marker, 'started'); +const value = process.env[name]; +const report = (typeof value === 'string' ? 'set' : 'unset') + ':' + (typeof value === 'string' ? value.length : 0); +const send = (m) => process.stdout.write(JSON.stringify(m) + '\\n'); +createInterface({ input: process.stdin }).on('line', (line) => { + if (!line.trim()) return; + const msg = JSON.parse(line); + if (msg.id === undefined) return; + if (msg.method === 'initialize') { + send({ jsonrpc: '2.0', id: msg.id, result: { + protocolVersion: msg.params.protocolVersion, + capabilities: { tools: {} }, + serverInfo: { name: 'env-fixture', version: '1.0.0' }, + } }); + } else if (msg.method === 'tools/list') { + send({ jsonrpc: '2.0', id: msg.id, result: { tools: [ + { name: 'credential_report', description: report, inputSchema: { type: 'object' } }, + ] } }); + } else { + send({ jsonrpc: '2.0', id: msg.id, error: { code: -32601, message: 'method not found' } }); + } +}); +`; + +const scratch = realpathSync(mkdtempSync(join(tmpdir(), 'connector-mcp-credential-env-'))); +const SERVER = join(scratch, 'server.mjs'); +writeFileSync(SERVER, FIXTURE_SERVER); + +afterAll(() => { + rmSync(scratch, { recursive: true, force: true }); +}); + +function ctx(partial: Partial & Pick): ConnectorProviderContext { + return { name: 'repo_tools', label: 'Repo Tools', type: 'api', ...partial }; +} + +function stdio(extra: Record = {}): Record { + return { kind: 'stdio', command: process.execPath, args: [SERVER, CREDENTIAL_ENV], ...extra }; +} + +/** A client factory that records whether anything was spawned, and never spawns. */ +function recordingClientFactory() { + const seen: McpTransport[] = []; + const clientFactory = async (transport: McpTransport): Promise => { + seen.push(transport); + return { listTools: async () => [], callTool: async () => ({}), close: async () => {} }; + }; + return { clientFactory, seen }; +} + +async function thrownBy(fn: () => unknown): Promise { + try { + await fn(); + } catch (err) { + return err as Error; + } + throw new Error('expected a throw'); +} + +describe('stdio credentialEnv — real spawns through the SDK', () => { + it('precondition: this process does not carry the variable, so only the provider can set it', () => { + expect(process.env[CREDENTIAL_ENV]).toBeUndefined(); + }); + + it.each([ + ['bearer', { type: 'bearer', token: PLACEHOLDER }], + ['api-key', { type: 'api-key', key: PLACEHOLDER, headerName: 'X-API-Key' }], + ['basic', { type: 'basic', username: 'service-user', password: PLACEHOLDER }], + ] as const)('a %s credential reaches the child in the variable credentialEnv names, and nowhere else', async (_kind, auth) => { + const factory = createMcpProviderFactory({ declarativeStdio: [process.execPath] }); + const mat = await factory(ctx({ + auth, + providerConfig: { transport: stdio({ env: { LOG_LEVEL: 'info' }, credentialEnv: CREDENTIAL_ENV }) }, + })); + try { + expect(mat.def.actions?.[0]?.description).toBe(`set:${PLACEHOLDER.length}`); + // The materialization the host keeps and serves carries no value. + expect(JSON.stringify(mat).includes(PLACEHOLDER)).toBe(false); + // A stdio handshake proves nothing about the credential. + expect(mat.credentialVerified).toBeUndefined(); + } finally { + await mat.close?.(); + } + }); + + it('control: the same instance with no credentialEnv starts the child without the variable (auth is not applied to stdio)', async () => { + const factory = createMcpProviderFactory({ declarativeStdio: [process.execPath] }); + const mat = await factory(ctx({ + auth: { type: 'bearer', token: PLACEHOLDER }, + providerConfig: { transport: stdio({ env: { LOG_LEVEL: 'info' } }) }, + })); + try { + expect(mat.def.actions?.[0]?.description).toBe('unset:0'); + } finally { + await mat.close?.(); + } + }); + + it('control: a literal in transport.env keeps reaching the child as today', async () => { + const factory = createMcpProviderFactory({ declarativeStdio: [process.execPath] }); + const mat = await factory(ctx({ providerConfig: { transport: stdio({ env: { [CREDENTIAL_ENV]: 'literal-placeholder' } }) } })); + try { + expect(mat.def.actions?.[0]?.description).toBe(`set:${'literal-placeholder'.length}`); + } finally { + await mat.close?.(); + } + }); +}); + +describe('stdio credentialEnv — configuration faults, refused before anything is spawned', () => { + const cases: Array<[string, Partial, RegExp]> = [ + [ + 'a credentialEnv that is not a variable name', + { auth: { type: 'bearer', token: PLACEHOLDER }, providerConfig: { transport: stdio({ credentialEnv: 'NOT A NAME' }) } }, + /transport\.credentialEnv must be the name of an environment variable/, + ], + [ + 'a credentialEnv that is not a string', + { auth: { type: 'bearer', token: PLACEHOLDER }, providerConfig: { transport: stdio({ credentialEnv: 42 }) } }, + /transport\.credentialEnv must be the name of an environment variable/, + ], + [ + 'a credentialEnv on an http transport', + { + auth: { type: 'bearer', token: PLACEHOLDER }, + providerConfig: { transport: { kind: 'http', url: 'https://mcp.example.invalid/mcp', credentialEnv: CREDENTIAL_ENV } }, + }, + /credentialEnv on an http transport/, + ], + [ + 'a credentialEnv with no auth', + { providerConfig: { transport: stdio({ credentialEnv: CREDENTIAL_ENV }) } }, + /declares no credential to place there/, + ], + [ + "a credentialEnv with auth: { type: 'none' }", + { auth: { type: 'none' }, providerConfig: { transport: stdio({ credentialEnv: CREDENTIAL_ENV }) } }, + /declares no credential to place there/, + ], + [ + 'a credentialEnv naming a variable transport.env also sets', + { + auth: { type: 'bearer', token: PLACEHOLDER }, + providerConfig: { transport: stdio({ env: { [CREDENTIAL_ENV]: 'other-placeholder' }, credentialEnv: CREDENTIAL_ENV }) }, + }, + /sets environment variable 'MCP_FIXTURE_TOKEN' twice/, + ], + ]; + + it.each(cases)('refuses %s, plain, naming no value', async (_label, partial, message) => { + const { clientFactory, seen } = recordingClientFactory(); + const factory = createMcpProviderFactory({ clientFactory, declarativeStdio: true }); + const err = await thrownBy(() => factory(ctx({ providerConfig: {}, ...partial }))); + expect(err.constructor).toBe(Error); + expect(err.message).toMatch(message); + expect(err.message.includes(PLACEHOLDER)).toBe(false); + expect(err.message.includes('other-placeholder')).toBe(false); + expect(seen).toEqual([]); + }); + + it('a grant-bearing instance on stdio meets the credential-handle refusal, which names the credentialEnv route', async () => { + const handle: ConnectorCredentialHandle = { + connector: 'repo_tools', + async get() { return { auth: { type: 'bearer', token: PLACEHOLDER } }; }, + async refresh() { return { auth: { type: 'bearer', token: PLACEHOLDER } }; }, + }; + const { clientFactory, seen } = recordingClientFactory(); + const factory = createMcpProviderFactory({ clientFactory, declarativeStdio: true }); + const err = await thrownBy(() => + factory(ctx({ credentialHandle: handle, providerConfig: { transport: stdio({ credentialEnv: CREDENTIAL_ENV }) } })), + ); + expect(err.message).toMatch(/only an http transport can carry/); + expect(err.message).toContain('providerConfig.transport.credentialEnv'); + expect(seen).toEqual([]); + }); +}); + +describe('stdio credentialEnv — the metadata read', () => { + it('serves the reference and the variable name, and withholds only the env map', () => { + const redact = getMetadataTypeRedactor('connector'); + expect(redact).toBeDefined(); + const { item, redactedKeys } = redact!({ + name: 'repo_tools', + label: 'Repo Tools', + type: 'api', + provider: 'mcp', + providerConfig: { + transport: { kind: 'stdio', command: 'repo-mcp-server', env: { LOG_LEVEL: 'info' }, credentialEnv: CREDENTIAL_ENV }, + }, + auth: { type: 'bearer', credentialRef: 'REPO_MCP_TOKEN' }, + }); + const transport = (item.providerConfig as { transport: Record }).transport; + expect(transport.credentialEnv).toBe(CREDENTIAL_ENV); + expect((item.auth as { credentialRef?: unknown }).credentialRef).toBe('REPO_MCP_TOKEN'); + expect(redactedKeys).toEqual(['providerConfig.transport.env']); + }); +}); + +describe('declarative stdio instance through the automation service', () => { + const REF = 'OS_TEST_MCP_STDIO_CREDENTIAL_REF'; + + function boot(credentialResolver: ((ref: string) => string | undefined) | undefined, marker: string) { + const declared = [ + { + name: 'repo_tools', + label: 'Repo Tools', + type: 'api', + provider: 'mcp', + providerConfig: { + transport: { kind: 'stdio', command: process.execPath, args: [SERVER, CREDENTIAL_ENV, marker], credentialEnv: CREDENTIAL_ENV }, + }, + auth: { type: 'bearer', credentialRef: REF }, + }, + ]; + const kernel = new LiteKernel({ logger: { level: 'silent' } } as never); + kernel.use(new AutomationServicePlugin(credentialResolver ? { credentialResolver } : {})); + 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({ declarativeStdio: [process.execPath] })); + return kernel; + } + + it('precondition: the reference is not an environment variable of this process', () => { + expect(process.env[REF]).toBeUndefined(); + }); + + it('a resolved reference reaches the spawned server; the served descriptor carries no value', async () => { + const marker = join(scratch, 'resolved.started'); + const kernel = boot((ref) => (ref === REF ? PLACEHOLDER : undefined), marker); + try { + await kernel.bootstrap(); + const engine = kernel.getService('automation') as AutomationEngine; + expect(engine.getConnectorDegradedReason('repo_tools')).toBeUndefined(); + const descriptor = engine.getConnectorDescriptors().find((d) => d.name === 'repo_tools'); + expect(descriptor?.state).toBe('ready'); + expect(descriptor?.actions[0]?.description).toBe(`set:${PLACEHOLDER.length}`); + expect(JSON.stringify(descriptor).includes(PLACEHOLDER)).toBe(false); + } finally { + await kernel.shutdown(); + } + }); + + it('an unresolvable reference never starts the server: the instance is degraded naming the reference', async () => { + const marker = join(scratch, 'missing.started'); + const kernel = boot(undefined, marker); + try { + await kernel.bootstrap(); + const engine = kernel.getService('automation') as AutomationEngine; + const reason = engine.getConnectorDegradedReason('repo_tools'); + expect(reason).toContain(`credential '${REF}' is not set`); + expect(existsSync(marker)).toBe(false); + } finally { + await kernel.shutdown(); + } + }); +}); diff --git a/packages/lint/src/lint-connector-credential-literals.test.ts b/packages/lint/src/lint-connector-credential-literals.test.ts index d3f8a5f7794..9dbea95e173 100644 --- a/packages/lint/src/lint-connector-credential-literals.test.ts +++ b/packages/lint/src/lint-connector-credential-literals.test.ts @@ -147,7 +147,7 @@ describe('connector-credential-literal — the pin set', () => { for (const sentinel of Object.values(S)) expect(wire.includes(sentinel), sentinel).toBe(false); }); - it('reads as one sentence per map; the stdio env verdict promises `auth` only over http', () => { + it('reads as one sentence per map; the stdio env verdict names `auth.credentialRef` with `transport.credentialEnv`', () => { const byPath = new Map(lintConnectorCredentialLiterals(litStack()).map((f) => [f.path, f])); expect(byPath.get(LIT_PATHS[0])!.message).toBe( "header 'Authorization' in providerConfig.headers holds a credential-shaped literal, kept in metadata, " @@ -155,13 +155,24 @@ describe('connector-credential-literal — the pin set', () => { ); expect(byPath.get(LIT_PATHS[3])!.message).toBe( "env 'MCP_UPSTREAM' in providerConfig.transport.env holds a credential-shaped literal, kept in metadata, " - + 'not the secrets layer; a stdio server takes no `auth`; `auth.credentialRef` reaches it only over http', + + "not the secrets layer; move it to the connector's `auth.credentialRef` with `transport.credentialEnv`", ); for (const f of byPath.values()) { expect(f.message.includes('\n'), f.path).toBe(false); expect(f.message.length, f.path).toBeLessThanOrEqual(210); } }); + + it('the stdio env hint names the credentialEnv route, spelling `auth` as the header hint does', () => { + const byPath = new Map(lintConnectorCredentialLiterals(litStack()).map((f) => [f.path, f])); + const env = byPath.get(LIT_PATHS[3])!.hint; + const header = byPath.get(LIT_PATHS[0])!.hint; + const shared = "auth: { type: 'bearer', credentialRef }"; + expect(header).toContain(shared); + expect(env).toContain(shared); + expect(env).toContain('transport.credentialEnv'); + expect(env).not.toContain("kind: 'http'"); + }); }); describe('connector-credential-literal — what draws nothing', () => { @@ -284,7 +295,8 @@ describe('connector-credential-literal — `os explain`', () => { for (const fact of [ 'withholds each of them whole', 'ADR-0097 §3', - 'A stdio MCP server takes no `auth`', + 'A stdio MCP server takes the same `auth`', + '`providerConfig.transport.credentialEnv`', 'a heuristic', 'A `{variable}` template is not a literal', 'never the value itself', diff --git a/packages/lint/src/lint-connector-credential-literals.ts b/packages/lint/src/lint-connector-credential-literals.ts index bec8d272916..266594afd71 100644 --- a/packages/lint/src/lint-connector-credential-literals.ts +++ b/packages/lint/src/lint-connector-credential-literals.ts @@ -30,12 +30,13 @@ * (ADR-0097 §3). The `rest` provider and an `mcp` http transport both send the * resolved credential themselves, so a header literal moves there by shape. * - * ⛔ A stdio MCP server takes NO `auth`: the provider folds a resolved - * credential into an http transport's headers only, and a stdio server's - * credentials ride `transport.env` (the connector guide says so). So no text - * here may promise that `credentialRef` reaches a stdio server — an author - * cannot follow it. The env finding names the route with its condition: an - * http transport. + * A stdio MCP server takes the same `auth`, and one more key says where it + * goes: `providerConfig.transport.credentialEnv` names the environment + * variable the `mcp` provider hands the resolved credential to the child in. + * ⛔ Without that key a stdio transport does not apply `auth` at all, so the + * env finding names BOTH halves — `auth.credentialRef` and + * `transport.credentialEnv` — never the reference alone, and its hint spells + * the `auth` the way the header hint does, so the two routes read as one. * * The predicate is a heuristic (a key named `token` can hold something that is * not a secret), so the finding is a `warning`, always: a false positive costs @@ -83,15 +84,15 @@ const KEPT = 'kept in metadata, not the secrets layer'; /** The route, short, in the VERDICT as well as in the hint (see the flow rule's note). */ const STEER_HEADER = "move it to the connector's `auth.credentialRef`"; -const STEER_ENV = 'a stdio server takes no `auth`; `auth.credentialRef` reaches it only over http'; +const STEER_ENV = "move it to the connector's `auth.credentialRef` with `transport.credentialEnv`"; const HEADER_ROUTE = "Give the entry `auth: { type: 'bearer', credentialRef }` (or `{ type: 'api-key', headerName, credentialRef }`) " + 'and drop the header: the provider sends the resolved credential itself'; const ENV_ROUTE = - "Reach the MCP server over `transport: { kind: 'http', url }` with `auth: { type: 'bearer', credentialRef }`; " + - "a stdio server's credentials ride `transport.env` only"; + "Give the entry `auth: { type: 'bearer', credentialRef }`, name the variable in `transport.credentialEnv` " + + 'and drop the env entry: the provider passes the resolved credential to the stdio server itself'; /** One judged map: where it sits under the entry, what an entry of it is called, and its route. */ interface JudgedMap { diff --git a/packages/lint/src/rule-explanations.ts b/packages/lint/src/rule-explanations.ts index ca36ea0e103..b5bb4d92e42 100644 --- a/packages/lint/src/rule-explanations.ts +++ b/packages/lint/src/rule-explanations.ts @@ -4867,10 +4867,11 @@ const CONNECTOR_CREDENTIAL_LITERAL_EXPLANATION: RuleExplanation = { 'literal moves by shape: an `Authorization` bearer value to `auth: { type: \'bearer\', credentialRef }`, ' + 'a key header to `{ type: \'api-key\', headerName, credentialRef }`, a Basic pair to ' + '`{ type: \'basic\', username, credentialRef }`.', - 'A stdio MCP server takes no `auth`: the provider folds a resolved credential into an http ' + - 'transport\'s headers only, and a stdio server\'s credentials ride `transport.env`. So the env ' + - 'finding names `auth.credentialRef` with its condition, an http transport, and promises nothing ' + - 'for stdio. The connector guide is content/docs/automation/connectors.mdx.', + 'A stdio MCP server takes the same `auth`, and `providerConfig.transport.credentialEnv` names the ' + + 'environment variable the `mcp` provider hands the resolved credential to the server in, so an env ' + + 'literal moves to `auth: { type: \'bearer\', credentialRef }` plus `credentialEnv: \'VARIABLE_NAME\'`, ' + + 'and the env entry is dropped. Without `credentialEnv` a stdio transport does not apply `auth` at ' + + 'all, so the env finding names both keys. The connector guide is content/docs/automation/connectors.mdx.', 'Always a warning, and nothing is refused: the predicate is a heuristic (a value under a name ' + 'that reads as a credential, or a value that opens with an auth scheme), so a key named ' + '`token` can hold something that is not a secret, and a false positive costs a line of ' +