diff --git a/docs/plugin-authoring.md b/docs/plugin-authoring.md index 42dc97c..b683b8c 100644 --- a/docs/plugin-authoring.md +++ b/docs/plugin-authoring.md @@ -49,6 +49,61 @@ Use `host.routes`, `host.tools`, `host.events`, and `host.jobs` to contribute behaviour. Do not import internal harness paths; only the package root and its `/wire` export are stable public APIs. +## Install a declared remote MCP connector + +An external product plugin can install a remote connector through the MCP +plugin's public port. Declare `uses: ['mcpConnectors']`; do not import MCP +storage, routes, OAuth helpers, or call the machine's own HTTP API. + +```ts +import { definePlugin, type McpConnectorPort } from '@hoshi/harness' + +export default [ + definePlugin({ + name: 'example-packs', + description: 'Installs example integrations.', + uses: ['mcpConnectors'], + setup(host) { + const connectors: McpConnectorPort | undefined = host.ports.mcpConnectors + if (!connectors) return + + void connectors.install({ + name: 'example', + transport: 'http', + url: 'https://mcp.example.com/', + vaultHeaders: { + Authorization: { key: 'EXAMPLE_TOKEN', template: 'Bearer {{secret}}' }, + }, + }) + }, + }), +] +``` + +The port accepts remote `http` or `sse` transports at `http:`/`https:` URLs; +it has no stdio or literal-header path. A vault binding names a machine vault +key and one of two templates: `{{secret}}` or `Bearer {{secret}}`. The binding, +not the secret value, is stored; its value is read only immediately before the MCP +plugin dials the server. If your pack format calls its placeholder +`{{credential}}`, translate it to `{{secret}}` and map the declared credential +to its machine vault key before calling the port. + +To begin machine-owned OAuth DCR from one of your authenticated routes, pass +that incoming request's headers. Discovery, registration, PKCE, callback, +refresh and tokens remain inside the MCP plugin: + +```ts +const result = await host.ports.mcpConnectors?.beginDcrAuthorization({ + name: 'example', + request: { headers: event.node.req.headers }, + scopes: ['read:documents'], +}) +// return or redirect to result?.url; never handle a token in this plugin. +``` + +`status(name)` returns only name, connection state, OAuth state and tool count. +It never returns an endpoint, vault binding, token, or secret value. + ## Describe what the machine can do `capability` is the public, stable identity of the unit your plugin adds. Its diff --git a/src/index.ts b/src/index.ts index 09c43f5..8ec72da 100644 --- a/src/index.ts +++ b/src/index.ts @@ -266,6 +266,15 @@ export type { export { RouteTable, RouteConflictError, ownedBy } from './http/router.js' export type { RouteRecord, Method } from './http/router.js' +export type { + McpConnectorDeclaration, + McpConnectorPort, + McpConnectorSnapshot, + McpConnectorStatus, + McpConnectorTransport, + McpConnectorVaultHeader, + McpDcrAuthorizationRequest, +} from './mcp-connectors.js' /** * diff --git a/src/kernel/host-ports.ts b/src/kernel/host-ports.ts index 7c8e552..7eae27a 100644 --- a/src/kernel/host-ports.ts +++ b/src/kernel/host-ports.ts @@ -3,6 +3,7 @@ import type { MachineEvent } from './events.js' import type { PendingAsk, PermissionResolution } from './permissions.js' import type { Provider } from './providers.js' import type { ShellResult } from '@openharness/core' +import type { McpConnectorPort } from '../mcp-connectors.js' /** * ── What the kernel asks of its host ───────────────────────────────────────── @@ -123,6 +124,9 @@ export interface UnattendedContext { } export interface KernelPorts extends UnattendedContext { + /** Declared remote MCP connectors. Answered by the MCP plugin so product + * plugins never import its storage or route internals. */ + mcpConnectors?: McpConnectorPort /** Extra standing instructions supplied by installed plugins. * * The harness owns the order in which a turn is assembled, but not every diff --git a/src/mcp-connectors.ts b/src/mcp-connectors.ts new file mode 100644 index 0000000..734dd33 --- /dev/null +++ b/src/mcp-connectors.ts @@ -0,0 +1,50 @@ +/** + * Public, tokenless contract for a plugin that installs a declared remote MCP + * connector. The MCP plugin owns storage, discovery and credentials; a product + * plugin only names an endpoint and vault bindings. + */ +export type McpConnectorTransport = 'http' | 'sse' + +/** One request header whose value is assembled on the machine when it dials. + * `template` is either `{{secret}}` or `Bearer {{secret}}`. For example: + * `{ key: 'GITHUB_TOKEN', template: 'Bearer {{secret}}' }`. + */ +export interface McpConnectorVaultHeader { + /** Name of a machine-vault key. Its value never crosses this public seam. */ + key: string + /** `{{secret}}` or `Bearer {{secret}}`. */ + template: string +} + +export interface McpConnectorDeclaration { + name: string + url: string + transport: McpConnectorTransport + /** Header name -> vault binding. Literal credentials are intentionally absent. */ + vaultHeaders?: Record +} + +export type McpConnectorAuthStatus = 'authenticated' | 'needs-auth' | 'expired' | null +export type McpConnectorStatus = 'connected' | 'unreachable' | 'disabled' + +/** Deliberately excludes URL, header bindings and all credential material. */ +export interface McpConnectorSnapshot { + name: string + status: McpConnectorStatus + auth: McpConnectorAuthStatus + toolCount: number +} + +export interface McpDcrAuthorizationRequest { + name: string + /** The request that initiated authorization, used only to derive the machine callback origin. */ + request: { headers: Record } + scopes?: string[] +} + +/** The MCP plugin's narrow service for external product plugins. */ +export interface McpConnectorPort { + install(input: McpConnectorDeclaration): Promise + beginDcrAuthorization(input: McpDcrAuthorizationRequest): Promise<{ url: string }> + status(name: string): Promise +} diff --git a/src/plugins/mcp/index.test.ts b/src/plugins/mcp/index.test.ts new file mode 100644 index 0000000..c440da1 --- /dev/null +++ b/src/plugins/mcp/index.test.ts @@ -0,0 +1,33 @@ +import { describe, expect, it, vi } from 'vitest' +import mcp from './index.js' +import type { KernelPorts } from '../../kernel/host-ports.js' +import type { PluginHost } from '../define.js' + +describe('the public MCP connector port', () => { + it('is provided by the MCP plugin without exposing its routes or storage', () => { + let provided: KernelPorts | undefined + const routes = { get: vi.fn(), post: vi.fn(), patch: vi.fn(), put: vi.fn(), delete: vi.fn(), all: vi.fn() } + const host: PluginHost = { + tools: { add: vi.fn() }, + routes, + events: { publish: vi.fn(), onConnect: vi.fn() }, + jobs: { every: vi.fn(), once: vi.fn() }, + log: { info: vi.fn(), error: vi.fn() }, + unattended: {}, + platform: () => null, + ports: {}, + provide: (extension) => { + provided = typeof extension === 'function' ? extension({}) : extension + }, + } + + mcp.setup(host, undefined) + + expect(provided?.mcpConnectors).toEqual({ + install: expect.any(Function), + beginDcrAuthorization: expect.any(Function), + status: expect.any(Function), + }) + expect(routes.post).toHaveBeenCalledWith('/mcp/:name/oauth', expect.any(Function)) + }) +}) diff --git a/src/plugins/mcp/index.ts b/src/plugins/mcp/index.ts index 02bdb5e..38af6e0 100644 --- a/src/plugins/mcp/index.ts +++ b/src/plugins/mcp/index.ts @@ -10,6 +10,8 @@ import oauthCallback from './mcp.oauth.callback.get.js' import startOauth from './mcp.name.oauth.post.js' import disconnectOauth from './mcp.name.oauth.delete.js' import { refreshExpiringTokens } from './oauth-connect.js' +import { connectorStatus, installRemoteServer } from './servers.js' +import { beginDcrAuthorization } from './mcp.name.oauth.post.js' /** * ── MCP connectors ─────────────────────────────────────────────────────────── @@ -34,6 +36,17 @@ export default definePlugin({ capability: { id: 'mcp.connectors', title: 'MCP connectors', description: 'Third-party MCP servers as tools' }, setup(host) { + /** The product-facing connector service. It is a port, not routes called + * back through localhost, so an external plugin stays independent of this + * plugin's storage and keeps its request context for OAuth redirects. */ + host.provide((current) => ({ + ...current, + mcpConnectors: { + install: installRemoteServer, + beginDcrAuthorization, + status: connectorStatus, + }, + })) /** * * Built per turn rather than cached: a connector added a moment ago has to diff --git a/src/plugins/mcp/mcp.name.oauth.post.ts b/src/plugins/mcp/mcp.name.oauth.post.ts index 6def866..0cb3bd9 100644 --- a/src/plugins/mcp/mcp.name.oauth.post.ts +++ b/src/plugins/mcp/mcp.name.oauth.post.ts @@ -2,6 +2,7 @@ import { defineEventHandler, getRouterParam } from 'h3' import { randomBytes } from 'node:crypto' import { apiError, requireAuth } from '../../kernel/index.js' import { listServers } from './servers.js' +import type { McpDcrAuthorizationRequest } from '../../mcp-connectors.js' import { authorizationUrl, createPkce, @@ -25,14 +26,12 @@ import { beginFlow, redirectUri } from './oauth-flow.js' * business holding one. * **/ -export default defineEventHandler(async (event) => { - await requireAuth(event) - const name = getRouterParam(event, 'name')! - const server = (await listServers()).find((entry) => entry.name === name) - if (!server) apiError(404, 'mcp.notFound', 'No connector by that name.') +export async function beginDcrAuthorization(input: McpDcrAuthorizationRequest): Promise<{ url: string }> { + const server = (await listServers()).find((entry) => entry.name === input.name) + if (!server) throw new McpAuthorizationError('notFound', 'No connector by that name.') const config = server.config if (config.type === 'stdio') { - apiError(400, 'mcp.oauthLocal', 'A local connector runs as a command and has nothing to authorize.') + throw new McpAuthorizationError('local', 'A local connector runs as a command and has nothing to authorize.') } try { @@ -50,32 +49,60 @@ export default defineEventHandler(async (event) => { **/ let registration = await registrationFor(metadata.issuer) if (!registration) { - registration = await registerClient(metadata, redirectUri(event), 'Hoshi machine') + registration = await registerClient(metadata, redirectUri(input.request.headers), 'Hoshi machine') await rememberRegistration(metadata.issuer, registration) } const pkce = createPkce() const state = randomBytes(24).toString('base64url') beginFlow(state, { - name, + name: input.name, issuer: metadata.issuer, resource: resource.resource, metadata, verifier: pkce.verifier, }) + const scopes = input.scopes?.filter((scope) => typeof scope === 'string' && scope.trim()).join(' ') return { url: authorizationUrl({ metadata, clientId: registration.clientId, - redirectUri: redirectUri(event), + redirectUri: redirectUri(input.request.headers), state, challenge: pkce.challenge, resource: resource.resource, + ...(scopes ? { scope: scopes } : {}), }), } } catch (error) { - if (error instanceof OAuthError) apiError(400, 'mcp.oauthFailed', error.message) + if (error instanceof OAuthError) throw new McpAuthorizationError('failed', error.message) + throw error + } +} + +export class McpAuthorizationError extends Error { + constructor( + readonly kind: 'notFound' | 'local' | 'failed', + message: string, + ) { + super(message) + } +} + +export default defineEventHandler(async (event) => { + await requireAuth(event) + try { + return await beginDcrAuthorization({ + name: getRouterParam(event, 'name')!, + request: { headers: event.node.req.headers as Record }, + }) + } catch (error) { + if (error instanceof McpAuthorizationError) { + const status = error.kind === 'notFound' ? 404 : 400 + const code = error.kind === 'notFound' ? 'mcp.notFound' : error.kind === 'local' ? 'mcp.oauthLocal' : 'mcp.oauthFailed' + apiError(status, code, error.message) + } throw error } }) diff --git a/src/plugins/mcp/mcp.oauth.callback.get.ts b/src/plugins/mcp/mcp.oauth.callback.get.ts index bba8060..d8dbb31 100644 --- a/src/plugins/mcp/mcp.oauth.callback.get.ts +++ b/src/plugins/mcp/mcp.oauth.callback.get.ts @@ -63,7 +63,7 @@ export default defineEventHandler(async (event) => { metadata: flow.metadata, clientId: registration.clientId, clientSecret: registration.clientSecret, - redirectUri: redirectUri(event), + redirectUri: redirectUri(event.node.req.headers as Record), code, verifier: flow.verifier, resource: flow.resource, diff --git a/src/plugins/mcp/oauth-flow.ts b/src/plugins/mcp/oauth-flow.ts index b47bf09..288f6ff 100644 --- a/src/plugins/mcp/oauth-flow.ts +++ b/src/plugins/mcp/oauth-flow.ts @@ -51,11 +51,16 @@ export function claimFlow(state: string): PendingFlow | null { return flow } +/** A declaration changed while consent was open. Its callback must not attach + * the old endpoint's authorization to the replacement connector. */ +export function forgetFlowsFor(name: string): void { + for (const [state, flow] of pending) if (flow.name === name) pending.delete(state) +} + /** The machine's own origin, taken from the request the person's browser is * making — not from configuration, which is how a redirect URI ends up * pointing at the wrong host on a machine reached through an ingress. */ -export function redirectUri(event: { node: { req: { headers: Record } } }): string { - const headers = event.node.req.headers +export function redirectUri(headers: Record): string { const forwardedProto = String(headers['x-forwarded-proto'] ?? '') .split(',')[0] ?.trim() diff --git a/src/plugins/mcp/oauth.test.ts b/src/plugins/mcp/oauth.test.ts index 883ec1a..2054f67 100644 --- a/src/plugins/mcp/oauth.test.ts +++ b/src/plugins/mcp/oauth.test.ts @@ -14,6 +14,7 @@ import { registerClient, revokeToken, } from './oauth.js' +import { redirectUri } from './oauth-flow.js' /** * @@ -105,6 +106,13 @@ describe('the well-known paths', () => { ) expect(protectedResourceMetadataUrl('https://x.test/')).toBe('https://x.test/.well-known/oauth-protected-resource') }) + + it('derives the callback from the initiating request headers, not a platform URL', () => { + expect(redirectUri({ host: 'machine.test' })).toBe('https://machine.test/mcp/oauth/callback') + expect(redirectUri({ host: 'internal:4200', 'x-forwarded-host': 'machine.test', 'x-forwarded-proto': 'https' })).toBe( + 'https://machine.test/mcp/oauth/callback', + ) + }) }) describe('a full authorization, against a server', () => { diff --git a/src/plugins/mcp/servers.test.ts b/src/plugins/mcp/servers.test.ts index cea1737..526e462 100644 --- a/src/plugins/mcp/servers.test.ts +++ b/src/plugins/mcp/servers.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest' -import { InvalidServerError, parseConfig } from './servers.js' +import { InvalidServerError, parseConfig, parseRemoteConnector } from './servers.js' /** * ── What a connector definition may be ─────────────────────────────────────── @@ -93,3 +93,53 @@ describe('what a connector definition must carry', () => { }) }) }) + +describe('the public declared-connector boundary', () => { + it('accepts only remote transports and stores a vault binding rather than a credential', () => { + expect( + parseRemoteConnector({ + name: 'github', + transport: 'http', + url: 'https://api.githubcopilot.com/mcp/', + vaultHeaders: { Authorization: { key: 'HOSHI_GITHUB_TOKEN', template: 'Bearer {{secret}}' } }, + }), + ).toEqual({ + type: 'http', + url: 'https://api.githubcopilot.com/mcp/', + vaultHeaders: { Authorization: { key: 'HOSHI_GITHUB_TOKEN', template: 'Bearer {{secret}}' } }, + }) + }) + + it('refuses a local transport, non-http endpoint, and an unsafe vault binding', () => { + expect(() => + parseRemoteConnector({ name: 'local', transport: 'stdio' as never, url: 'https://example.test/mcp' }), + ).toThrow(InvalidServerError) + expect(() => parseRemoteConnector({ name: 'ftp', transport: 'http', url: 'ftp://example.test/mcp' })).toThrow( + InvalidServerError, + ) + expect(() => + parseRemoteConnector({ + name: 'unsafe', + transport: 'sse', + url: 'https://example.test/sse', + vaultHeaders: { Authorization: { key: 'token', template: 'Bearer {{secret}}' } }, + }), + ).toThrow(InvalidServerError) + expect(() => + parseRemoteConnector({ + name: 'unsafe-template', + transport: 'sse', + url: 'https://example.test/sse', + vaultHeaders: { Authorization: { key: 'TOKEN', template: 'Bearer token' } }, + }), + ).toThrow(InvalidServerError) + expect(() => + parseRemoteConnector({ + name: 'unsafe-template', + transport: 'sse', + url: 'https://example.test/sse', + vaultHeaders: { Authorization: { key: 'TOKEN', template: 'Token {{secret}}' } }, + }), + ).toThrow(InvalidServerError) + }) +}) diff --git a/src/plugins/mcp/servers.ts b/src/plugins/mcp/servers.ts index 9e918bd..cfbcca6 100644 --- a/src/plugins/mcp/servers.ts +++ b/src/plugins/mcp/servers.ts @@ -2,8 +2,11 @@ import { closeMCPClients, connectMCPServers, type MCPServerConfig } from '@openh import type { ToolSet } from 'ai' import { hoshiFile, readHoshiJson, writeHoshiJson } from '../../kernel/store.js' import { authorizationHeader } from './oauth-connect.js' -import { authorizedConnectors, authStatus, type AuthStatus } from './oauth-store.js' +import { authorizedConnectors, authStatus, forgetAuth, type AuthStatus } from './oauth-store.js' import { publishMachineEvent } from '../../kernel/events.js' +import { isValidSecretKey, readSecretValue } from '../../kernel/secrets.js' +import type { McpConnectorDeclaration, McpConnectorSnapshot, McpConnectorVaultHeader } from '../../mcp-connectors.js' +import { forgetFlowsFor } from './oauth-flow.js' /** * ── MCP connectors ─────────────────────────────────────────────────────────── @@ -34,7 +37,11 @@ export type ServerStatus = 'connected' | 'unreachable' | 'disabled' * command, the url and the credentials stay exactly as they were, so switching * it back on is one click rather than setting it up again. The machine had no * notion of this at all while every client offered the toggle. */ -export type StoredServer = MCPServerConfig & { enabled?: boolean } +export type StoredServer = MCPServerConfig & { + enabled?: boolean + /** Credential references only. Resolved immediately before connecting. */ + vaultHeaders?: Record +} export interface McpServer { name: string @@ -73,15 +80,15 @@ const FILE = () => hoshiFile('mcp.json') const CONNECT_TIMEOUT_MS = 5_000 interface McpFile { - servers?: Record + servers?: Record } -async function readDefinitions(): Promise> { +async function readDefinitions(): Promise> { const data = await readHoshiJson(FILE()) return data?.servers ?? {} } -async function writeDefinitions(servers: Record): Promise { +async function writeDefinitions(servers: Record): Promise { await writeHoshiJson(FILE(), { servers }) invalidate() publishMachineEvent('mcp.changed', {}) @@ -120,8 +127,23 @@ function invalidate(): void { * the config untouched for stdio, and for anything with no authorization. */ async function withAuthorization(name: string, config: MCPServerConfig): Promise { if (config.type === 'stdio') return config + const stored = config as StoredServer + const vaultHeaders = await resolveVaultHeaders(stored.vaultHeaders) const header = await authorizationHeader(name) - return header ? { ...config, headers: { ...config.headers, ...header } } : config + return { ...config, headers: { ...config.headers, ...vaultHeaders, ...header } } +} + +/** Resolve only while connecting. Stored declarations carry vault key NAMES and + * templates, never a value that a config/status read could reveal. */ +async function resolveVaultHeaders( + bindings: Record | undefined, +): Promise> { + const headers: Record = {} + for (const [name, binding] of Object.entries(bindings ?? {})) { + const secret = await readSecretValue(binding.key) + if (secret !== null) headers[name] = binding.template.replaceAll('{{secret}}', secret) + } + return headers } async function connectAll(): Promise { @@ -222,6 +244,15 @@ export async function mcpTools(): Promise { return (await ensureConnected()).tools } +/** A projection safe for another plugin to render or return. It intentionally + * omits the endpoint and header bindings alongside every credential value. */ +export async function connectorStatus(name: string): Promise { + const server = (await listServers()).find((entry) => entry.name === name) + return server + ? { name: server.name, status: server.status, auth: server.auth, toolCount: server.toolCount } + : null +} + export class InvalidServerError extends Error {} const NAME = /^[a-zA-Z0-9][a-zA-Z0-9_-]{0,63}$/ @@ -240,6 +271,40 @@ function isStringRecord(value: unknown): value is Record { ) } +function vaultHeaderBindings(value: unknown): Record | undefined { + if (value === undefined) return undefined + if (typeof value !== 'object' || value === null || Array.isArray(value)) { + throw new InvalidServerError('vaultHeaders must be a map of header names to vault bindings.') + } + const parsed: Record = {} + for (const [header, binding] of Object.entries(value)) { + if (!header || /[\r\n]/.test(header)) throw new InvalidServerError('A vault header name cannot be empty or contain a line break.') + if (typeof binding !== 'object' || binding === null || Array.isArray(binding)) { + throw new InvalidServerError(`Vault header ${header} must name a key and template.`) + } + const { key, template } = binding as Record + if (typeof key !== 'string' || !isValidSecretKey(key)) { + throw new InvalidServerError(`Vault header ${header} needs a valid uppercase vault key.`) + } + if (template !== '{{secret}}' && template !== 'Bearer {{secret}}') { + throw new InvalidServerError(`Vault header ${header} must use {{secret}} or Bearer {{secret}}.`) + } + parsed[header] = { key, template } + } + return parsed +} + +export function parseRemoteConnector(input: McpConnectorDeclaration): StoredServer { + const config = parseConfig({ type: input.transport, url: input.url }) + if (config.type === 'stdio') throw new InvalidServerError('A declared connector must use an HTTP or SSE transport.') + const protocol = new URL(config.url).protocol + if (protocol !== 'http:' && protocol !== 'https:') { + throw new InvalidServerError('A declared connector needs an http or https endpoint.') + } + const vaultHeaders = vaultHeaderBindings(input.vaultHeaders) + return { ...config, ...(vaultHeaders ? { vaultHeaders } : {}) } +} + export function parseConfig(raw: unknown): MCPServerConfig { const value = (raw ?? {}) as Record const type = value.type @@ -290,6 +355,28 @@ export async function addServer(name: string, config: MCPServerConfig): Promise< await writeDefinitions(servers) } +/** Install a remote declaration from a product plugin. This is deliberately + * separate from the owner-facing add route: product code gets no stdio or + * literal-header escape hatch. Changing the remote identity makes all prior + * OAuth state unusable, so it is discarded along with any in-flight flow. */ +export async function installRemoteServer(input: McpConnectorDeclaration): Promise { + if (!NAME.test(input.name)) { + throw new InvalidServerError('A connector name must be letters, digits, dashes or underscores.') + } + const definition = parseRemoteConnector(input) + const servers = await readDefinitions() + const previous = servers[input.name] + const changedEndpoint = + !previous || previous.type === 'stdio' || previous.type !== definition.type || previous.url !== definition.url + servers[input.name] = definition + await writeDefinitions(servers) + if (changedEndpoint) { + forgetFlowsFor(input.name) + await forgetAuth(input.name) + publishMachineEvent('mcp.changed', { name: input.name, auth: null }) + } +} + /** Merge a change into an existing connector. False when there is nothing by * that name — reported as a 404 rather than quietly creating one, since an * edit that lands as a create is how a typo becomes a second connector nobody