From 7b7aba4e30366f94dd066bbd8c05f828ff619efe Mon Sep 17 00:00:00 2001 From: "Anthony Fu (via agent)" Date: Tue, 1 Sep 2026 06:30:22 +0000 Subject: [PATCH 1/4] fix(devframe): authenticate HTTP MCP requests The route-based MCP endpoint treated a caller-provided Origin as authorization, so any local process (or a native client spoofing an Origin) could invoke privileged agent tools. Origin is DNS-rebinding hardening, not identity. Add an independent identity gate to the MCP route, checked after the origin gate: - McpRouteOptions.authorization: a bearer token string (constant-time compared), a (request) => boolean callback, or false for an origin-only local opt-out. - mcp: true is shorthand for the bearer read from DEVFRAME_MCP_AUTH_TOKEN; a missing token or an object without authorization fails startup with new diagnostic DF0077 rather than mounting an unauthenticated route. - Missing/invalid bearer -> 401 + WWW-Authenticate: Bearer; disallowed origin stays 403. A callback governs identity only and cannot relax the origin gate. - @devframes/next/hub now defaults MCP to disabled; callers opt in with an explicit policy. - devframe connect reads DEVFRAME_MCP_AUTH_TOKEN and presents it as the bearer; ConnectServerOptions.authToken accepts one token or a per-instance resolver. Credentials live only in configuration and the Authorization header. Created with the help of an agent. --- docs/content/1.guide/14.security.md | 2 +- docs/content/1.guide/18.hub-initiate.md | 2 + docs/content/2.adapters/7.mcp.md | 32 ++++- docs/content/3.frameworks/1.vite.md | 2 +- docs/content/3.frameworks/3.next.md | 3 + docs/content/6.errors/DF0077.md | 49 +++++++ examples/files-inspector/src/devframe.ts | 9 +- .../src/client/devframe/next-devframe-hub.ts | 6 +- .../hub-next/tests/next-devframe-hub.test.ts | 13 +- .../src/adapters/__tests__/dev.test.ts | 4 +- .../src/adapters/__tests__/initiate.test.ts | 11 +- packages/devframe/src/adapters/_shared.ts | 46 ++++++- packages/devframe/src/adapters/initiate.ts | 25 ++-- .../adapters/mcp/__tests__/mcp-http.test.ts | 122 ++++++++++++----- packages/devframe/src/adapters/mcp/fetch.ts | 63 ++++++++- packages/devframe/src/cli/connect.test.ts | 126 ++++++++++++++++++ packages/devframe/src/cli/connect.ts | 59 ++++++-- packages/devframe/src/cli/main.ts | 5 + packages/devframe/src/internal/index.ts | 3 +- packages/devframe/src/node/diagnostics.ts | 4 + packages/devframe/src/types/devframe.ts | 43 +++++- .../hub/src/node/__tests__/initiate.test.ts | 11 +- packages/hub/src/node/initiate.ts | 15 ++- packages/next/src/host.ts | 19 ++- packages/next/src/hub.ts | 16 ++- packages/next/test/handler.test.ts | 5 +- packages/vite/test/single.test.ts | 5 +- plans/README.md | 2 +- .../tsnapi/@devframes/next/hub.snapshot.d.ts | 3 +- .../devframe/adapters/mcp.snapshot.d.ts | 1 + .../tsnapi/devframe/index.snapshot.d.ts | 2 + .../tsnapi/devframe/internal.snapshot.d.ts | 6 + .../tsnapi/devframe/internal.snapshot.js | 1 + .../tsnapi/devframe/types.snapshot.d.ts | 1 + tests/optional-mcp-bundles.test.ts | 4 +- 35 files changed, 627 insertions(+), 93 deletions(-) create mode 100644 docs/content/6.errors/DF0077.md create mode 100644 packages/devframe/src/cli/connect.test.ts diff --git a/docs/content/1.guide/14.security.md b/docs/content/1.guide/14.security.md index e953731da..4b34a9270 100644 --- a/docs/content/1.guide/14.security.md +++ b/docs/content/1.guide/14.security.md @@ -73,7 +73,7 @@ For your own auth UI, disable built-in handling with `otpParam: false`, then cal - **Stay on loopback.** Bind to a routable address only intentionally, and require authentication when you do. - **Keep `auth: false` local.** The hosted bridges (`devframeViteBridge`, `@devframes/next`'s handler) gate their side-car by default; opt out with an explicit `auth: false` only when the host framework owns the trust boundary another way. -- **The MCP route requires an origin.** The route-based MCP server rejects requests without a loopback or allow-listed `Origin`, so an arbitrary local process can't reach it — see [MCP](/adapters/mcp). +- **The MCP route authenticates the caller.** `Origin` hardens the route-based MCP server against DNS-rebinding, but proves nothing about identity — a native client can send any `Origin`. So the route also requires a bearer: `mcp: true` reads it from `DEVFRAME_MCP_AUTH_TOKEN`, and the route refuses to mount ([`DF0077`](/errors/DF0077)) without a policy. Treat the two checks as separate defenses — see [MCP](/adapters/mcp). - **Treat tokens as secrets.** Never log the bearer token or the one-time code, or bake either into build output. - **Authorize every handler.** Validate inputs, and mark state-changing functions `type: 'destructive'` so MCP and agent clients prompt before invoking them. - **Origin-lock remote docks.** When a hub embeds a remote-UI dock, keep `originLock` on (the default) so its session token is only honored on a connection whose `Origin` matches the dock's own. diff --git a/docs/content/1.guide/18.hub-initiate.md b/docs/content/1.guide/18.hub-initiate.md index 29095a713..b83247f6c 100644 --- a/docs/content/1.guide/18.hub-initiate.md +++ b/docs/content/1.guide/18.hub-initiate.md @@ -82,6 +82,8 @@ Registrations are validated fail-fast: one module per type (`DF8108`), an existi The hub's **single Auth** is one gate at the shared transport for every mounted devframe, built-ins, and the MCP route; one handshake (OTP, magic link, or pre-shared token) unlocks the namespace; `auth: false` disables it for localhost. +The aggregate MCP route carries its **own** identity gate independent of this RPC Auth, since it grants agent clients privileged tool access: `mcp: true` requires the `DEVFRAME_MCP_AUTH_TOKEN` bearer (startup fails with [`DF0077`](/errors/DF0077) without it), or pass `mcp: { authorization }` explicitly. `Origin` remains request hardening, not identity. + ## Singular vs hub mounting A devframe's SPA and RPC client are byte-identical in both cases; only the environment differs: diff --git a/docs/content/2.adapters/7.mcp.md b/docs/content/2.adapters/7.mcp.md index 17c2edfdf..5cebbef62 100644 --- a/docs/content/2.adapters/7.mcp.md +++ b/docs/content/2.adapters/7.mcp.md @@ -26,6 +26,7 @@ import { defineDevframe } from 'devframe' export default defineDevframe({ // … cli: { + // Reads the bearer from DEVFRAME_MCP_AUTH_TOKEN. mcp: true, }, }) @@ -33,7 +34,30 @@ export default defineDevframe({ The endpoint speaks Streamable-HTTP at `/__mcp` (`/__/__mcp` under a host framework), sharing its origin/port. `--mcp` / `--no-mcp` override; `__connection.json` advertises it. -The endpoint is **stateless**: it serves the [2026-07-28 revision](https://modelcontextprotocol.io/specification/2026-07-28) per request through the SDK's `createMcpHandler`, building a fresh MCP server for each request — every HTTP request stands alone, with no `Mcp-Session-Id` to correlate. 2025-era clients are still served through the SDK's stateless legacy path. An origin gate requires `Origin` be loopback (or allow-listed) and rejects `Origin`-less requests. Widen for a tunnel/LAN origin with `cli: { mcp: { allowedOrigins: ['https://tunnel.example.com'] } }`. +The endpoint is **stateless**: it serves the [2026-07-28 revision](https://modelcontextprotocol.io/specification/2026-07-28) per request through the SDK's `createMcpHandler`, building a fresh MCP server for each request — every HTTP request stands alone, with no `Mcp-Session-Id` to correlate. 2025-era clients are still served through the SDK's stateless legacy path. + +### Two gates: origin hardening and identity + +The route exposes privileged agent tools, so every request clears two independent gates. The **origin gate** requires `Origin` be loopback (or allow-listed) and rejects `Origin`-less requests — DNS-rebinding hardening that proves nothing about *who* is calling, since a native client can send any `Origin`. Widen it for a tunnel/LAN origin with `cli: { mcp: { authorization: process.env.MY_TOKEN, allowedOrigins: ['https://tunnel.example.com'] } }`. + +The **identity gate** then proves the caller. `mcp: true` reads its bearer from the `DEVFRAME_MCP_AUTH_TOKEN` environment variable; a request presents it as `Authorization: Bearer ` and it is matched in constant time. A missing or wrong bearer gets `401` with a `WWW-Authenticate: Bearer` challenge; a disallowed origin gets `403`. Startup fails with [`DF0077`](/errors/DF0077) — the route is never mounted — when `mcp: true` finds no environment token, or an object config omits `authorization`. + +An object config sets the policy explicitly: + +```ts +export default defineDevframe({ + cli: { + // A bearer from your own environment variable: + mcp: { authorization: process.env.MY_TOKEN }, + // — or a callback identity check (governs identity only; it cannot relax the origin gate): + // mcp: { authorization: request => isTrusted(request) }, + // — or an origin-only opt-out for a loopback-bound local tool that owns its trust boundary another way: + // mcp: { authorization: false }, + }, +}) +``` + +Never place the token in a URL, in `__connection.json`, in the instance registry, in logs, or on the command line — it belongs only in configuration and the `Authorization` header. ### Hosted bridges @@ -47,6 +71,8 @@ devframeViteBridge(myDevframe, { mcp: true }) createDevframeNextHandler(myDevframe, { mcp: true }) ``` +Both honor the same authorization contract: `mcp: true` requires the `DEVFRAME_MCP_AUTH_TOKEN` bearer, or pass `mcp: { authorization }` explicitly. + ## Custom host frameworks `createMcpFetchHandler(ctx, options)` returns the endpoint as a `Request → Response` handler plus a `dispose()` — mount on any fetch server. @@ -58,6 +84,8 @@ const mcp = createMcpFetchHandler(ctx, { serverName: 'my-tool (devframe)', serverVersion: '1.0.0', exposeSharedState: true, + // Required: the identity policy — a bearer token, a callback, or `false`. + authorization: process.env.DEVFRAME_MCP_AUTH_TOKEN!, }) // route every method on /__mcp to mcp.fetch(request) ``` @@ -81,4 +109,6 @@ Two gateway tools (`devframe:connect:*` ids — see [tool ids and wire names](/g Discovery reads the **instance registry**: every `createDevServer` writes `~/.devframe/instances/-.json`, dialed with a loopback origin. In-process host frameworks register via `registerDevframeInstance` (`devframe/node`). `--port ` probes a port; `DEVFRAME_INSTANCES_DIR` relocates the registry, `DEVFRAME_DISABLE_INSTANCE_REGISTRY=1` opts out. +The connector reads `DEVFRAME_MCP_AUTH_TOKEN` and presents it as the bearer to each instance's authenticated route (never a CLI flag — command-line arguments are visible to other processes). An instance whose route requires a different bearer reports auth-required rather than being reached; connect to a fleet with distinct credentials by driving `startConnectServer` with a per-instance `authToken` resolver. + See [Agent-Native](/guide/agent-native) for the API and safety model. diff --git a/docs/content/3.frameworks/1.vite.md b/docs/content/3.frameworks/1.vite.md index 8b57bb4cf..daccb61c9 100644 --- a/docs/content/3.frameworks/1.vite.md +++ b/docs/content/3.frameworks/1.vite.md @@ -42,7 +42,7 @@ Devframe spawns a separate RPC + WS server and registers Vite middleware at `__mcp`. | +| `mcp` | `def.cli?.mcp` | Expose the MCP route at `__mcp`. `true` requires the `DEVFRAME_MCP_AUTH_TOKEN` bearer; `McpRouteOptions` carries an explicit `authorization`. | ## `devframeVite` — convenience wrapper diff --git a/docs/content/3.frameworks/3.next.md b/docs/content/3.frameworks/3.next.md index 23f11e436..826412709 100644 --- a/docs/content/3.frameworks/3.next.md +++ b/docs/content/3.frameworks/3.next.md @@ -48,6 +48,7 @@ export const GET = handler.fetch | `port` | from `def.cli?.port` | Side-car port. | | `flags` | — | Passed to `def.setup(ctx, { flags })`. | | `auth` | `false` | `true` for the OTP gate, or a handler. | +| `mcp` | `def.cli?.mcp` | Expose the MCP route. `true` requires the `DEVFRAME_MCP_AUTH_TOKEN` bearer; `McpRouteOptions` carries an explicit `authorization`. | | `key` | `@devframes/next::` | `globalThis` memoization key. | ## Hosting a hub @@ -124,6 +125,8 @@ export const POST = (req: Request) => hub.handler(req) export const DELETE = (req: Request) => hub.handler(req) ``` +The aggregate MCP route is off by default — it exposes privileged agent tools. Opt in with an authorization policy: `mcp: true` (requiring the `DEVFRAME_MCP_AUTH_TOKEN` bearer) or `mcp: { authorization }`. + No native hub UI provider here, so this scope stays quiet; `createDevframeNextHost()` is the low-level `DevframeHost`. ## See also diff --git a/docs/content/6.errors/DF0077.md b/docs/content/6.errors/DF0077.md new file mode 100644 index 000000000..905a1325b --- /dev/null +++ b/docs/content/6.errors/DF0077.md @@ -0,0 +1,49 @@ +--- +title: 'DF0077: MCP Authorization Required' +description: 'The route-based MCP server needs an authorization policy, but none is configured.' +--- + +## Message + +> The route-based MCP server needs an authorization policy, but none is configured — refusing to mount an unauthenticated agent endpoint. + +## Cause + +The route-based MCP endpoint exposes privileged agent tools to any process that can reach it. `Origin` hardens the request against DNS-rebinding but proves nothing about *who* is calling, so the route also requires an identity policy. This diagnostic fires when that policy is absent: + +- `mcp: true` (the shorthand) reads its bearer from the `DEVFRAME_MCP_AUTH_TOKEN` environment variable, and the variable is missing or empty. +- An object MCP config omits the required `authorization` field (or sets it to an empty string). + +Startup fails and the route is never mounted, rather than exposing the endpoint unauthenticated. + +## Example + +```ts +// ✗ throws DF0077 when DEVFRAME_MCP_AUTH_TOKEN is unset +await createDevServer(def, { mcp: true }) + +// ✗ throws DF0077 — object config with no authorization +await createDevServer(def, { mcp: { path: '__mcp' } }) + +// ✓ shorthand, with the environment token set +process.env.DEVFRAME_MCP_AUTH_TOKEN = 'a-high-entropy-secret' +await createDevServer(def, { mcp: true }) + +// ✓ explicit bearer token +await createDevServer(def, { mcp: { authorization: process.env.MY_TOKEN! } }) + +// ✓ callback identity check +await createDevServer(def, { mcp: { authorization: req => isTrusted(req) } }) + +// ✓ origin-only opt-out for a loopback-bound local tool +await createDevServer(def, { mcp: { authorization: false } }) +``` + +## Fix + +- Set the `DEVFRAME_MCP_AUTH_TOKEN` environment variable to the bearer the `mcp: true` shorthand requires. +- Or pass an explicit `authorization` on the MCP options — a non-empty bearer token string, a `(request) => boolean` callback, or `false` for an origin-only local opt-out. + +## Source + +- [`packages/devframe/src/adapters/_shared.ts`](https://github.com/devframes/devframe/blob/main/packages/devframe/src/adapters/_shared.ts) — `resolveMcpConfig()` throws this when the `mcp: true` shorthand has no environment token, or an object config omits `authorization`. diff --git a/examples/files-inspector/src/devframe.ts b/examples/files-inspector/src/devframe.ts index 703d9696e..5fcbf4c02 100644 --- a/examples/files-inspector/src/devframe.ts +++ b/examples/files-inspector/src/devframe.ts @@ -24,8 +24,13 @@ export default defineDevframe({ // SPA can call RPC without an OTP round-trip. auth: false, // Serve the agent surface over the dev server's `/__mcp` route and - // register the instance for `devframe connect` discovery. - mcp: true, + // register the instance for `devframe connect` discovery. This demo binds + // to loopback (`localhost:9876`), so it takes the origin-only opt-out + // (`authorization: false`) rather than requiring a bearer - the MCP route + // stays reachable to `devframe connect` on the same machine without token + // plumbing. A network-reachable tool would set a real bearer instead + // (e.g. `authorization: process.env.DEVFRAME_MCP_AUTH_TOKEN`). + mcp: { authorization: false }, }, setup(ctx) { // A scoped context auto-namespaces every registered id with `NAMESPACE:`. diff --git a/examples/hub-next/src/client/devframe/next-devframe-hub.ts b/examples/hub-next/src/client/devframe/next-devframe-hub.ts index 0b8761b50..15394be21 100644 --- a/examples/hub-next/src/client/devframe/next-devframe-hub.ts +++ b/examples/hub-next/src/client/devframe/next-devframe-hub.ts @@ -217,7 +217,11 @@ export async function nextDevframeHub( // for a bearer token. See `docs/content/1.guide/13.security.md`. // The aggregate MCP endpoint at `/__devframes/__mcp` - the hub's agent // surface (agent-flagged commands, plugin tools, `devframe:state:read`) - // over the same catch-all route as the SPAs. + // over the same catch-all route as the SPAs. `mcp: true` is the + // environment-backed policy: it reads the required bearer from + // `DEVFRAME_MCP_AUTH_TOKEN`, so startup fails (DF0077) unless that is set - + // the route is never mounted unauthenticated. An MCP client presents that + // token as `Authorization: Bearer ` alongside a loopback Origin. mcp: true, // This host renders its own React UI in `app/page.tsx`, so skip the // default `@devframes/hub-ui` standalone/embedded slot. diff --git a/examples/hub-next/tests/next-devframe-hub.test.ts b/examples/hub-next/tests/next-devframe-hub.test.ts index 289903250..c958deb27 100644 --- a/examples/hub-next/tests/next-devframe-hub.test.ts +++ b/examples/hub-next/tests/next-devframe-hub.test.ts @@ -3,12 +3,23 @@ import { getTempAuthCode } from 'devframe/node/auth' import { createRpcClient } from 'devframe/rpc/client' import { createWsRpcChannel } from 'devframe/rpc/transports/ws-client' import { getPort } from 'get-port-please' -import { afterEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { WebSocket } from 'ws' import { nextDevframeHub } from '../src/client/devframe/next-devframe-hub' vi.stubGlobal('WebSocket', WebSocket) +// The example enables its aggregate MCP route with the environment-backed +// `mcp: true` policy, so a bearer must be configured or the hub refuses to +// start (DF0077). Provide it for the duration of each test. +beforeEach(() => { + vi.stubEnv('DEVFRAME_MCP_AUTH_TOKEN', 'a-high-entropy-example-test-token') +}) + +afterEach(() => { + vi.unstubAllEnvs() +}) + /** The side-car WS port advertised by the hub's connection meta. */ function wsPortOf(hub: HubInstance): number { const ws = hub.connectionMeta().websocket diff --git a/packages/devframe/src/adapters/__tests__/dev.test.ts b/packages/devframe/src/adapters/__tests__/dev.test.ts index 21e5acd99..b476e21cf 100644 --- a/packages/devframe/src/adapters/__tests__/dev.test.ts +++ b/packages/devframe/src/adapters/__tests__/dev.test.ts @@ -782,7 +782,9 @@ describe('adapters/dev', () => { host: '127.0.0.1', port: 0, auth: false, - mcp: true, + // Origin-only opt-out keeps this loopback-bound registry test free of + // bearer plumbing; the identity gate is covered in mcp-http.test.ts. + mcp: { authorization: false }, }) const { readDevframeInstances } = await import('../../node/instance-registry') diff --git a/packages/devframe/src/adapters/__tests__/initiate.test.ts b/packages/devframe/src/adapters/__tests__/initiate.test.ts index b3c1075ee..fae6d3c32 100644 --- a/packages/devframe/src/adapters/__tests__/initiate.test.ts +++ b/packages/devframe/src/adapters/__tests__/initiate.test.ts @@ -245,7 +245,9 @@ describe('adapters/handler', () => { it('mcp: mounts __mcp and advertises it in the meta', async () => { const wsPort = await getPort({ port: 18140, host: '127.0.0.1' }) - const devtools = initDevframe(defineTestDef('handler-mcp'), { base: '/__handler-mcp/', auth: false, mcp: true, ws: { port: wsPort } }) + // An explicit origin-only opt-out keeps this loopback-bound fixture free of + // bearer plumbing; the identity gate itself is covered in mcp-http.test.ts. + const devtools = initDevframe(defineTestDef('handler-mcp'), { base: '/__handler-mcp/', auth: false, mcp: { authorization: false }, ws: { port: wsPort } }) try { await devtools.ready @@ -262,6 +264,13 @@ describe('adapters/handler', () => { } }) + it('mcp: true without DEVFRAME_MCP_AUTH_TOKEN fails startup (DF0077), route absent', async () => { + const wsPort = await getPort({ port: 18145, host: '127.0.0.1' }) + const devtools = initDevframe(defineTestDef('handler-mcp-noauth'), { base: '/__handler-mcp-noauth/', auth: false, mcp: true, ws: { port: wsPort } }) + await expect(devtools.ready).rejects.toThrow(/DF0077|authorization policy/) + await devtools.close() + }) + it('default tier: binds nothing until the host attaches its own server', async () => { const host = '127.0.0.1' const port = await getPort({ port: 18150, host }) diff --git a/packages/devframe/src/adapters/_shared.ts b/packages/devframe/src/adapters/_shared.ts index 1619116a2..c00272cdb 100644 --- a/packages/devframe/src/adapters/_shared.ts +++ b/packages/devframe/src/adapters/_shared.ts @@ -1,8 +1,10 @@ import type { ConnectionMeta } from '../types/context' -import type { DevframeDefinition, DevframeDeploymentKind, McpRouteOptions } from '../types/devframe' +import type { DevframeDefinition, DevframeDeploymentKind, McpAuthorization, McpRouteOptions } from '../types/devframe' +import process from 'node:process' import { getPort } from 'get-port-please' import { cleanDoubleSlashes, withLeadingSlash, withoutLeadingSlash, withTrailingSlash } from 'ufo' import { DEVFRAME_MCP_ROUTE } from '../constants' +import { diagnostics } from '../node/diagnostics' const DEFAULT_PORT = 9999 @@ -56,13 +58,47 @@ export async function resolveDevServerPort( } /** - * Normalize the `cli.mcp` / `mcp` option (`boolean | McpRouteOptions`) into - * concrete options, or `undefined` when the MCP route is disabled. + * A fully-resolved MCP route configuration: the concrete authorization policy + * (never the `mcp: true` shorthand), plus the optional route path and origin + * allow-list. Every route mount consumes this shape. */ -function resolveMcpConfig(mcp: boolean | McpRouteOptions | undefined): McpRouteOptions | undefined { +export interface ResolvedMcpConfig { + /** Route segment, relative to the base. Default resolved by the caller. */ + path?: string + /** Origin allow-list, or `false` to disable the origin gate. */ + allowedOrigins?: readonly string[] | false + /** The resolved identity policy — a bearer token, callback, or `false`. */ + authorization: McpAuthorization +} + +/** + * Normalize the `cli.mcp` / `mcp` option (`boolean | McpRouteOptions`) into a + * fully-resolved config, or `undefined` when the MCP route is disabled. + * + * The route grants access to privileged agent tools, so an enabled route + * always resolves to a concrete authorization policy. `mcp: true` is shorthand + * for the bearer read from `DEVFRAME_MCP_AUTH_TOKEN`; an object config must + * carry an explicit `authorization`. A missing/empty token or absent + * `authorization` throws {@link diagnostics.DF0077} so the route is never + * mounted unauthenticated. + */ +export function resolveMcpConfig(mcp: boolean | McpRouteOptions | undefined): ResolvedMcpConfig | undefined { if (!mcp) return undefined - return mcp === true ? {} : mcp + if (mcp === true) { + const token = process.env.DEVFRAME_MCP_AUTH_TOKEN + if (!token) + throw diagnostics.DF0077() + return { authorization: token } + } + const { authorization } = mcp + if (authorization === undefined || (typeof authorization === 'string' && authorization.length === 0)) + throw diagnostics.DF0077() + return { + ...(mcp.path !== undefined ? { path: mcp.path } : {}), + ...(mcp.allowedOrigins !== undefined ? { allowedOrigins: mcp.allowedOrigins } : {}), + authorization, + } } /** diff --git a/packages/devframe/src/adapters/initiate.ts b/packages/devframe/src/adapters/initiate.ts index 10b40acff..bb1beeff3 100644 --- a/packages/devframe/src/adapters/initiate.ts +++ b/packages/devframe/src/adapters/initiate.ts @@ -13,16 +13,16 @@ import { resolveStaticAssetsSource } from 'devframe/utils/remote-assets' import { mountStaticHandler } from 'devframe/utils/serve-static' import { H3 } from 'h3' import { resolve } from 'pathe' -import { joinURL } from 'ufo' +import { joinURL, withoutLeadingSlash } from 'ufo' import { resolveClientAssets } from '../client-assets' -import { DEVFRAME_CONNECTION_META_FILENAME } from '../constants' +import { DEVFRAME_CONNECTION_META_FILENAME, DEVFRAME_MCP_ROUTE } from '../constants' import { createHostContext } from '../node/context' import { diagnostics } from '../node/diagnostics' import { createH3DevframeHost } from '../node/host-h3' import { importRuntimeModule } from '../node/import-runtime-module' import { createInstanceShell, resolveInstanceRegister } from '../node/instance-shell' -import { normalizeBasePath } from './_shared' -import { resolveDevServerPort, resolveMcpConnectionMeta } from './dev' +import { normalizeBasePath, resolveMcpConfig } from './_shared' +import { resolveDevServerPort } from './dev' export interface InitDevframeOptions { /** @@ -303,13 +303,17 @@ export function initDevframe( // Route-based MCP server (opt-in). Mounted before the SPA static // catch-all so the exact `__mcp` route wins, and advertised in // `__connection.json`. The MCP SDK stays an optional peer — its code is - // only pulled in (dynamically) when the route is enabled. - const mcpOption = options.mcp ?? def.cli?.mcp - const mcpMeta = resolveMcpConnectionMeta(def, mcpOption) + // only pulled in (dynamically) when the route is enabled. Resolving the + // config validates the authorization policy up front: a `mcp: true` + // shorthand with no `DEVFRAME_MCP_AUTH_TOKEN`, or an object with no + // `authorization`, throws `DF0077` here rather than mounting the route. + const mcpConfig = resolveMcpConfig(options.mcp ?? def.cli?.mcp) + let mcpMeta: ConnectionMeta['mcp'] let mcpDispose: (() => Promise) | undefined - if (mcpMeta) { - const mcpConfig = mcpOption === true || mcpOption === undefined ? {} : mcpOption as McpRouteOptions - const mcpPath = joinURL(base, mcpMeta.path) + if (mcpConfig) { + const mcpRoute = withoutLeadingSlash(mcpConfig.path ?? DEVFRAME_MCP_ROUTE) + mcpMeta = { path: mcpRoute } + const mcpPath = joinURL(base, mcpRoute) let mountMcpHttp: typeof import('./mcp/http').mountMcpHttp try { ;({ mountMcpHttp } = await importRuntimeModule('devframe/adapters/mcp')) @@ -322,6 +326,7 @@ export function initDevframe( serverName: `${def.id} (devframe)`, serverVersion: def.version ?? '0.0.0', exposeSharedState: true, + authorization: mcpConfig.authorization, allowedOrigins: mcpConfig.allowedOrigins, }) mcpDispose = mounted.dispose diff --git a/packages/devframe/src/adapters/mcp/__tests__/mcp-http.test.ts b/packages/devframe/src/adapters/mcp/__tests__/mcp-http.test.ts index 4f4150834..21786954a 100644 --- a/packages/devframe/src/adapters/mcp/__tests__/mcp-http.test.ts +++ b/packages/devframe/src/adapters/mcp/__tests__/mcp-http.test.ts @@ -1,9 +1,11 @@ import type { StartedServer } from '../../../node/instance-shell' -import type { DevframeDefinition } from '../../../types/devframe' +import type { DevframeDefinition, McpAuthorization } from '../../../types/devframe' import { Client, StreamableHTTPClientTransport } from '@modelcontextprotocol/client' import { afterEach, describe, expect, it } from 'vitest' import { createDevServer } from '../../dev' +const TOKEN = 'a-high-entropy-test-bearer-token' + function defineTestDef(overrides?: Partial): DevframeDefinition { return { id: 'mcp-http-test', @@ -32,7 +34,7 @@ describe('mcp adapter (streamable http route)', () => { server = undefined }) - async function boot(def = defineTestDef()): Promise { + async function boot(authorization: McpAuthorization = TOKEN, def = defineTestDef()): Promise { // `port: 0` lets the OS assign a fresh ephemeral port per test. Without // it every test binds the same default port, and since they all share // one process, Node's global `fetch()` (undici) pools keep-alive @@ -41,7 +43,7 @@ describe('mcp adapter (streamable http route)', () => { // test's (torn-down) server, failing instantly with a socket error, or // making that earlier server's `close()` hang until undici's // keep-alive timeout releases it. - server = await createDevServer(def, { host: '127.0.0.1', port: 0, mcp: true }) + server = await createDevServer(def, { host: '127.0.0.1', port: 0, mcp: { authorization } }) return server } @@ -60,17 +62,24 @@ describe('mcp adapter (streamable http route)', () => { expect(meta.mcp).toBeUndefined() }) + it('fails startup when the mcp: true shorthand has no environment token', async () => { + await expect( + createDevServer(defineTestDef(), { host: '127.0.0.1', port: 0, mcp: true }), + ).rejects.toThrow(/DF0077|authorization policy/) + }) + // A native MCP client must send a (loopback) Origin so the route's gate — - // which rejects Origin-less requests — accepts it. - function originTransport(started: StartedServer): StreamableHTTPClientTransport { + // which rejects Origin-less requests — accepts it, and the configured + // bearer so the identity gate accepts it. + function authedTransport(started: StartedServer): StreamableHTTPClientTransport { return new StreamableHTTPClientTransport(new URL(`${started.origin}/__mcp`), { - requestInit: { headers: { origin: started.origin } }, + requestInit: { headers: { origin: started.origin, authorization: `Bearer ${TOKEN}` } }, }) } - it('serves the modern era statelessly and lists agent tools', async () => { + it('serves the modern era statelessly and lists agent tools with a valid bearer', async () => { const started = await boot() - const transport = originTransport(started) + const transport = authedTransport(started) // Negotiate the 2026-07-28 era via `server/discover`. const client = new Client( { name: 'test-client', version: '0.0.0' }, @@ -95,29 +104,27 @@ describe('mcp adapter (streamable http route)', () => { } }) - it('answers a bare GET with 405 (no session lifecycle)', async () => { + it('answers a bare GET with 405 (no session lifecycle) once both gates pass', async () => { const started = await boot() // Stateless serving has no session stream to open — the SDK answers a // GET (a 2025 session operation) with `405 Method Not Allowed` rather // than falling through to the SPA static catch-all. const res = await fetch(`${started.origin}/__mcp`, { method: 'GET', - headers: { accept: 'text/event-stream', origin: started.origin }, + headers: { accept: 'text/event-stream', origin: started.origin, authorization: `Bearer ${TOKEN}` }, }) await res.body?.cancel() expect(res.status).toBe(405) }) - it('rejects an Origin-less request', async () => { - const started = await boot() - // Unlike the WS transport, the MCP route does not allow Origin-less - // requests — a route-based endpoint would otherwise be reachable by any - // local process. - const res = await fetch(`${started.origin}/__mcp`, { + /** An `initialize` POST body for the origin/identity gate tests. */ + function initRequest(started: StartedServer, headers: Record): Promise { + return fetch(`${started.origin}/__mcp`, { method: 'POST', headers: { 'content-type': 'application/json', 'accept': 'application/json, text/event-stream', + ...headers, }, body: JSON.stringify({ jsonrpc: '2.0', @@ -126,26 +133,79 @@ describe('mcp adapter (streamable http route)', () => { params: { protocolVersion: '2025-03-26', capabilities: {}, clientInfo: { name: 'x', version: '0' } }, }), }) + } + + it('rejects an Origin-less request with 403 (origin gate runs first)', async () => { + const started = await boot() + // Unlike the WS transport, the MCP route does not allow Origin-less + // requests — a route-based endpoint would otherwise be reachable by any + // local process. The origin gate runs before the identity gate, so even + // a valid bearer is rejected 403 without an Origin. + const res = await initRequest(started, { authorization: `Bearer ${TOKEN}` }) await res.body?.cancel() expect(res.status).toBe(403) }) - it('rejects a disallowed cross-origin request', async () => { + it('rejects a disallowed cross-origin request with 403 even with a valid bearer', async () => { const started = await boot() - const res = await fetch(`${started.origin}/__mcp`, { - method: 'POST', - headers: { - 'content-type': 'application/json', - 'accept': 'application/json, text/event-stream', - 'origin': 'http://evil.example.com', - }, - body: JSON.stringify({ - jsonrpc: '2.0', - id: 1, - method: 'initialize', - params: { protocolVersion: '2025-03-26', capabilities: {}, clientInfo: { name: 'x', version: '0' } }, - }), - }) + const res = await initRequest(started, { origin: 'http://evil.example.com', authorization: `Bearer ${TOKEN}` }) + await res.body?.cancel() expect(res.status).toBe(403) }) + + it('rejects an allowed-origin request with no bearer as 401 + WWW-Authenticate', async () => { + const started = await boot() + const res = await initRequest(started, { origin: started.origin }) + await res.body?.cancel() + expect(res.status).toBe(401) + expect(res.headers.get('www-authenticate')).toBe('Bearer') + }) + + it('rejects an allowed-origin request with the wrong bearer as 401', async () => { + const started = await boot() + const res = await initRequest(started, { origin: started.origin, authorization: 'Bearer not-the-token' }) + await res.body?.cancel() + expect(res.status).toBe(401) + }) + + it('rejects a malformed / multi-credential Authorization header as 401', async () => { + const started = await boot() + const res = await initRequest(started, { origin: started.origin, authorization: `Bearer ${TOKEN}, Bearer other` }) + await res.body?.cancel() + expect(res.status).toBe(401) + }) + + it('delegates identity to a callback policy: allow', async () => { + const started = await boot(req => req.headers.get('x-secret') === 'open-sesame') + const res = await initRequest(started, { 'origin': started.origin, 'x-secret': 'open-sesame' }) + expect(res.status).toBe(200) + await res.body?.cancel() + }) + + it('delegates identity to a callback policy: deny → 401', async () => { + const started = await boot(req => req.headers.get('x-secret') === 'open-sesame') + const res = await initRequest(started, { 'origin': started.origin, 'x-secret': 'wrong' }) + await res.body?.cancel() + expect(res.status).toBe(401) + }) + + it('a callback cannot relax the origin gate (disallowed origin still 403)', async () => { + const started = await boot(() => true) + const res = await initRequest(started, { origin: 'http://evil.example.com' }) + await res.body?.cancel() + expect(res.status).toBe(403) + }) + + it('explicit authorization: false is an origin-only opt-out', async () => { + const started = await boot(false) + // Allowed origin, no bearer at all — accepted, since identity is opted out. + const res = await initRequest(started, { origin: started.origin }) + expect(res.status).toBe(200) + await res.body?.cancel() + + // …but the origin gate still stands. + const cross = await initRequest(started, { origin: 'http://evil.example.com' }) + await cross.body?.cancel() + expect(cross.status).toBe(403) + }) }) diff --git a/packages/devframe/src/adapters/mcp/fetch.ts b/packages/devframe/src/adapters/mcp/fetch.ts index 384e6deff..9082c129e 100644 --- a/packages/devframe/src/adapters/mcp/fetch.ts +++ b/packages/devframe/src/adapters/mcp/fetch.ts @@ -1,6 +1,7 @@ -import type { DevframeNodeContext } from 'devframe/types' +import type { DevframeNodeContext, McpAuthorization } from 'devframe/types' import { createMcpHandler } from '@modelcontextprotocol/server' import { isAllowedOrigin } from 'devframe/rpc/transports/ws-server' +import { timingSafeEqual } from 'devframe/utils/crypto-token' import { bridgeListChanged, buildMcpServerFromContext } from './build-server' export interface CreateMcpFetchHandlerOptions { @@ -10,6 +11,14 @@ export interface CreateMcpFetchHandlerOptions { serverVersion: string /** Expose shared-state keys as MCP resources — see `buildMcpServerFromContext`. */ exposeSharedState: boolean | ((key: string) => boolean) + /** + * The endpoint's identity policy, checked **after** the origin gate: a + * bearer token string (matched in constant time against + * `Authorization: Bearer `), a `(request) => boolean` callback, or + * `false` for an origin-only opt-out. A callback governs identity only and + * cannot relax the origin gate. See {@link McpAuthorization}. + */ + authorization: McpAuthorization /** * Origin allow-list beyond the loopback default. `false` disables the * origin gate entirely. Default: loopback-only. @@ -22,6 +31,36 @@ export interface CreateMcpFetchHandlerOptions { allowedOrigins?: readonly string[] | false } +/** + * Parse exactly one `Authorization: Bearer ` credential, returning the + * token or `undefined` for a missing, malformed, empty, or multi-credential + * header. The token itself is never logged. `\S+` rejects the whitespace that + * a second credential (fetch merges duplicate headers as `a, b`) or an empty + * value would introduce. + */ +function parseBearerToken(header: string | null): string | undefined { + if (!header) + return undefined + const match = /^Bearer (\S+)$/i.exec(header.trim()) + return match ? match[1] : undefined +} + +/** + * Resolve the identity gate for one request. `false` is the origin-only + * opt-out; a callback delegates identity; a string requires a constant-time + * bearer match. Never reveals whether a supplied token was close to correct. + */ +async function isAuthorized(req: Request, authorization: McpAuthorization): Promise { + if (authorization === false) + return true + if (typeof authorization === 'function') + return await authorization(req) === true + const token = parseBearerToken(req.headers.get('authorization')) + if (token === undefined) + return false + return timingSafeEqual(token, authorization) +} + export interface McpFetchHandler { /** * WHATWG-`fetch` handler for the MCP endpoint. Hand every method @@ -47,16 +86,21 @@ export interface McpFetchHandler { * SDK's default stateless legacy path. `list_changed` events reach modern * `subscriptions/listen` streams through the handler's `notify` bus. * - * The origin gate guards every request: loopback-default DNS-rebinding - * protection that — unlike the WS upgrade's `isAllowedOrigin` — also rejects - * `Origin`-less requests, so a route-based endpoint isn't reachable by an - * arbitrary local process. + * Two independent gates guard every request. First the origin gate: + * loopback-default DNS-rebinding protection that — unlike the WS upgrade's + * `isAllowedOrigin` — also rejects `Origin`-less requests, so a route-based + * endpoint isn't reachable by an arbitrary local process (a disallowed origin + * gets `403`). Then the identity gate ({@link CreateMcpFetchHandlerOptions.authorization}): + * a bearer/callback check that proves *who* is calling, since a native client + * can spoof any `Origin` (a missing/invalid credential gets `401` with a + * `WWW-Authenticate: Bearer` challenge). */ export function createMcpFetchHandler( ctx: DevframeNodeContext, options: CreateMcpFetchHandlerOptions, ): McpFetchHandler { const allowedOrigins = options.allowedOrigins + const authorization = options.authorization const handler = createMcpHandler(() => buildMcpServerFromContext(ctx, { serverName: options.serverName, @@ -80,7 +124,14 @@ export function createMcpFetchHandler( // `Origin` that is loopback or on the configured allow-list. const origin = req.headers.get('origin') ?? undefined if (allowedOrigins !== false && (origin === undefined || !isAllowedOrigin(origin, allowedOrigins ?? []))) - return new Response('Forbidden: origin required', { status: 403 }) + return new Response('Forbidden', { status: 403 }) + + // Identity gate — a request that cleared the origin check still has to + // prove *who* it is. A generic 401 (with the `WWW-Authenticate` challenge) + // whether the bearer is absent, malformed, or wrong: no response reveals + // whether a supplied token was close to correct. + if (!await isAuthorized(req, authorization)) + return new Response('Unauthorized', { status: 401, headers: { 'WWW-Authenticate': 'Bearer' } }) return handler.fetch(req) } diff --git a/packages/devframe/src/cli/connect.test.ts b/packages/devframe/src/cli/connect.test.ts new file mode 100644 index 000000000..9644247d0 --- /dev/null +++ b/packages/devframe/src/cli/connect.test.ts @@ -0,0 +1,126 @@ +import type { DevframeInstanceRecord } from '../node/instance-registry' +import type { StartedServer } from '../node/instance-shell' +import type { DevframeDefinition } from '../types/devframe' +import { Client, StreamableHTTPClientTransport } from '@modelcontextprotocol/client' +import { afterEach, describe, expect, it } from 'vitest' +import { createDevServer } from '../adapters/dev' +import { buildInstanceRequestHeaders, resolveAuthToken } from './connect' + +const TOKEN = 'a-high-entropy-connect-test-token' + +function makeRecord(overrides?: Partial): DevframeInstanceRecord { + return { + pid: 123, + port: 9999, + origin: 'http://localhost:9999', + basePath: '/', + id: 'demo', + rootDir: '/tmp/demo', + mcp: { path: '/__mcp' }, + startedAt: 0, + ...overrides, + } +} + +describe('resolveAuthToken', () => { + it('returns a shared string token for any record', () => { + expect(resolveAuthToken(TOKEN, makeRecord())).toBe(TOKEN) + }) + + it('delegates to a per-record resolver', () => { + const record = makeRecord({ port: 4242 }) + const resolved = resolveAuthToken(r => (r.port === 4242 ? 'match' : undefined), record) + expect(resolved).toBe('match') + }) + + it('is undefined when no policy is configured', () => { + expect(resolveAuthToken(undefined, makeRecord())).toBeUndefined() + }) +}) + +describe('buildInstanceRequestHeaders', () => { + it('always sends the instance origin so the origin gate accepts the native client', () => { + const headers = buildInstanceRequestHeaders('http://localhost:9999/__mcp', undefined) + expect(headers.origin).toBe('http://localhost:9999') + expect(headers.authorization).toBeUndefined() + }) + + it('adds the bearer as an Authorization header when a token is configured', () => { + const headers = buildInstanceRequestHeaders('http://localhost:9999/__mcp', TOKEN) + expect(headers.authorization).toBe(`Bearer ${TOKEN}`) + }) + + it('keeps the token out of the connection URL and the registry record', () => { + const url = 'http://localhost:9999/__mcp' + buildInstanceRequestHeaders(url, TOKEN) + // The token is never written back onto the URL… + expect(url).not.toContain(TOKEN) + // …nor does a registry record carry any credential field to leak. + expect(JSON.stringify(makeRecord())).not.toContain(TOKEN) + }) +}) + +describe('connector bearer against a live authenticated MCP route', () => { + let server: StartedServer | undefined + + afterEach(async () => { + await server?.close() + server = undefined + }) + + function defineDef(): DevframeDefinition { + return { + id: 'connect-test', + name: 'Connect Test', + version: '0.0.0', + packageName: '@devframe/connect-test', + homepage: 'https://example.com', + description: 'Fixture for the connect bearer test.', + setup(ctx) { + ctx.agent.registerTool({ + id: 'greet', + description: 'Say hello.', + safety: 'read', + handler: () => ({ greeting: 'hi' }), + }) + }, + } + } + + it('authenticates with the resolved bearer in the request headers', async () => { + server = await createDevServer(defineDef(), { host: '127.0.0.1', port: 0, auth: false, mcp: { authorization: TOKEN } }) + const url = `${server.origin}/__mcp` + const record = makeRecord({ origin: server.origin, port: server.port }) + + // Exactly what the connector does: resolve the token, build the headers, + // and dial the route through a StreamableHTTP transport. + const token = resolveAuthToken(TOKEN, record) + const client = new Client({ name: 'connect-test', version: '0.0.0' }, { versionNegotiation: { mode: 'auto' } }) + const transport = new StreamableHTTPClientTransport(new URL(url), { + requestInit: { headers: buildInstanceRequestHeaders(url, token) }, + }) + try { + await client.connect(transport) + const tools = await client.listTools() + expect(tools.tools.map(t => t.name)).toContain('greet') + } + finally { + await client.close() + } + }) + + it('is refused (never falls through unauthenticated) when no bearer is resolved', async () => { + server = await createDevServer(defineDef(), { host: '127.0.0.1', port: 0, auth: false, mcp: { authorization: TOKEN } }) + const url = `${server.origin}/__mcp` + const record = makeRecord({ origin: server.origin, port: server.port }) + + // No configured policy → no Authorization header → the route's identity + // gate rejects the request rather than serving it. + const headers = buildInstanceRequestHeaders(url, resolveAuthToken(undefined, record)) + expect(headers.authorization).toBeUndefined() + const client = new Client({ name: 'connect-test', version: '0.0.0' }, { versionNegotiation: { mode: 'auto' } }) + const transport = new StreamableHTTPClientTransport(new URL(url), { requestInit: { headers } }) + await expect(client.connect(transport)).rejects.toThrow() + await client.close().catch(() => {}) + }) +}) diff --git a/packages/devframe/src/cli/connect.ts b/packages/devframe/src/cli/connect.ts index 83296e543..f2afe86d5 100644 --- a/packages/devframe/src/cli/connect.ts +++ b/packages/devframe/src/cli/connect.ts @@ -18,6 +18,47 @@ export interface ConnectServerOptions { instancesDir?: string /** Probe timeout per instance, ms. Default 1000. */ timeoutMs?: number + /** + * The bearer credential the connector presents to each instance's + * authenticated MCP route, sent as `Authorization: Bearer `. + * + * - a **string** — one shared token for every instance; + * - a **resolver** `(record) => string | undefined` — a per-instance token, + * for connecting to a fleet with distinct credentials (return `undefined` + * to send none for that instance). + * + * The token is only ever placed in a request header: it never enters the + * instance registry records, the indexed results, connection URLs, or + * formatted errors. Left unset, no `Authorization` header is sent — only an + * instance whose route opted out of identity (`authorization: false`) will + * answer. + */ + authToken?: string | ((record: DevframeInstanceRecord) => string | undefined) +} + +/** + * Resolve the per-record bearer from the {@link ConnectServerOptions.authToken} + * option. Exported for focused tests of the credential resolution. + */ +export function resolveAuthToken( + authToken: ConnectServerOptions['authToken'], + record: DevframeInstanceRecord, +): string | undefined { + return typeof authToken === 'function' ? authToken(record) : authToken +} + +/** + * Build the request headers the connector sends to one instance's MCP route: + * the instance's own (loopback) `origin` so the route's origin gate accepts + * this native client, plus `Authorization: Bearer ` when a bearer is + * configured. The bearer appears **only** here — never in the connection URL, + * the registry records, or the indexed results. Exported for focused tests. + */ +export function buildInstanceRequestHeaders(url: string, token: string | undefined): Record { + const headers: Record = { origin: new URL(url).origin } + if (token) + headers.authorization = `Bearer ${token}` + return headers } export interface ConnectServerHandle { @@ -164,7 +205,7 @@ async function index(sdk: ConnectSdk, options: ConnectServerOptions): Promise { - return withInstanceClient(sdk, url, async (client) => { +async function listInstanceTools(sdk: ConnectSdk, url: string, token: string | undefined): Promise<{ name: string, description?: string }[]> { + return withInstanceClient(sdk, url, token, async (client) => { const listed = await client.listTools() return listed.tools.map((tool: { name: string, description?: string }) => ({ name: tool.name, @@ -231,7 +272,7 @@ async function call( throw diagnostics.DF0051({ port: args.port }) const url = `${record.origin}${record.mcp.path}` - return withInstanceClient(sdk, url, async (client) => { + return withInstanceClient(sdk, url, resolveAuthToken(options.authToken, record), async (client) => { const result = await client.callTool({ name: args.tool!, arguments: args.args ?? {} }) return { instance: { id: record.id, port: record.port }, @@ -246,14 +287,16 @@ async function call( async function withInstanceClient( sdk: ConnectSdk, url: string, + token: string | undefined, fn: (client: InstanceType) => Promise, ): Promise { - // Send the instance's own (loopback) origin so the MCP route's origin gate, - // which rejects `Origin`-less requests, accepts this native client. - const origin = new URL(url).origin + // The instance's own (loopback) origin (so the route's origin gate, which + // rejects `Origin`-less requests, accepts this native client) plus the + // bearer (when configured) — the `Authorization` header being the only place + // the credential ever appears. const transport = new sdk.StreamableHTTPClientTransport( new URL(url), - { requestInit: { headers: { origin } } }, + { requestInit: { headers: buildInstanceRequestHeaders(url, token) } }, ) // Negotiate the era with `server/discover`, falling back to the 2025 // `initialize` handshake for a 2025-only instance. Devframe's own route is diff --git a/packages/devframe/src/cli/main.ts b/packages/devframe/src/cli/main.ts index 340f99478..1c57e9063 100644 --- a/packages/devframe/src/cli/main.ts +++ b/packages/devframe/src/cli/main.ts @@ -20,6 +20,11 @@ export async function runDevframeCli(argv: string[] = process.argv): Promise boolean` callback, or `false` for an origin-only local opt-out.', + }, }, }) diff --git a/packages/devframe/src/types/devframe.ts b/packages/devframe/src/types/devframe.ts index 95fed9186..3a4990d7e 100644 --- a/packages/devframe/src/types/devframe.ts +++ b/packages/devframe/src/types/devframe.ts @@ -84,6 +84,25 @@ export interface DevframeSseOptions { route?: string } +/** + * The identity policy the route-based MCP endpoint enforces once a request + * clears the origin gate. `Origin` proves nothing about *who* is calling — any + * native client can send any `Origin` — so this is the endpoint's actual + * authentication, kept independent of the origin/DNS-rebinding check: + * + * - a non-empty **bearer token string** — the request must carry + * `Authorization: Bearer `, compared in constant time; + * - a **callback** `(request) => boolean | Promise` — a custom + * identity check (validate a signed header, an mTLS-derived claim, …). It + * only governs identity and cannot relax the origin gate; + * - `false` — an explicit **origin-only opt-out** for a loopback-bound local + * tool that owns its trust boundary another way. Use it sparingly. + */ +export type McpAuthorization + = | string + | ((request: Request) => boolean | Promise) + | false + /** * Configuration for the route-based MCP server mounted alongside the dev * server (opt-in via {@link DevframeCliOptions.mcp}). The endpoint speaks @@ -97,6 +116,18 @@ export interface McpRouteOptions { * Default: `__mcp` (i.e. `/__mcp` standalone, `/__/__mcp` hosted). */ path?: string + /** + * The endpoint's identity policy — **required** for an object config, since + * the route grants access to privileged agent tools. See + * {@link McpAuthorization} for the accepted forms (bearer token, callback, + * or explicit `false` for an origin-only local opt-out). The `mcp: true` + * shorthand reads its bearer from the `DEVFRAME_MCP_AUTH_TOKEN` environment + * variable instead of this field. + * + * This is checked **after** the origin gate below — identity and + * origin/DNS-rebinding protection are separate defenses. + */ + authorization: McpAuthorization /** * Extra `Origin` header values to accept beyond the loopback default * (`localhost`/`127.0.0.1`/`::1` and any `Origin`-less native client). @@ -107,7 +138,8 @@ export interface McpRouteOptions { * This is the endpoint's DNS-rebinding protection — the shared * `isAllowedOrigin` gate the WS upgrade already uses, applied as external * middleware (the approach the MCP SDK now recommends over its own - * deprecated `allowedHosts`/`allowedOrigins` transport flags). + * deprecated `allowedHosts`/`allowedOrigins` transport flags). It hardens + * the request; {@link McpRouteOptions.authorization} proves identity. */ allowedOrigins?: readonly string[] | false } @@ -158,9 +190,12 @@ export interface DevframeCliOptions { * stdio `mcp` command, but against the live, running server. * * - `false` / omitted (default) — no MCP route is mounted. - * - `true` — mount at the default `__mcp` route with the loopback-only - * origin gate. - * - {@link McpRouteOptions} — customise the route path / allowed origins. + * - `true` — mount at the default `__mcp` route, requiring the bearer token + * read from the `DEVFRAME_MCP_AUTH_TOKEN` environment variable (startup + * fails with `DF0077` when it is missing/empty, so the route is never + * mounted unauthenticated). + * - {@link McpRouteOptions} — customise the route path / allowed origins, + * with an explicit {@link McpAuthorization} policy (required). * * The `--mcp` / `--no-mcp` CLI flags override this per run. */ diff --git a/packages/hub/src/node/__tests__/initiate.test.ts b/packages/hub/src/node/__tests__/initiate.test.ts index e1dd1032c..d4474e012 100644 --- a/packages/hub/src/node/__tests__/initiate.test.ts +++ b/packages/hub/src/node/__tests__/initiate.test.ts @@ -291,7 +291,9 @@ describe('initHub', () => { it('aggregate MCP: one endpoint lists tools from every mounted frame', async () => { const wsPort = await getPort({ port: 18230, host: '127.0.0.1' }) - const hub = initHub({ base: DEVFRAMES_HUB_BASE, auth: false, host: '127.0.0.1', ws: { port: wsPort }, mcp: true, devframes: [makeFrame('alpha'), makeFrame('beta')] }) + // An origin-only opt-out keeps this loopback-bound fixture free of bearer + // plumbing; the identity gate itself is covered in mcp-http.test.ts. + const hub = initHub({ base: DEVFRAMES_HUB_BASE, auth: false, host: '127.0.0.1', ws: { port: wsPort }, mcp: { authorization: false }, devframes: [makeFrame('alpha'), makeFrame('beta')] }) try { await hub.ready @@ -321,6 +323,13 @@ describe('initHub', () => { } }) + it('aggregate MCP: mcp: true without DEVFRAME_MCP_AUTH_TOKEN fails startup (DF0077)', async () => { + const wsPort = await getPort({ port: 18235, host: '127.0.0.1' }) + const hub = initHub({ base: DEVFRAMES_HUB_BASE, auth: false, host: '127.0.0.1', ws: { port: wsPort }, mcp: true, devframes: [makeFrame('alpha')] }) + await expect(hub.ready).rejects.toThrow(/DF0077|authorization policy/) + await hub.close() + }) + it('single hub Auth: one gate covers every frame on the shared socket', async () => { const wsPort = await getPort({ port: 18240, host: '127.0.0.1' }) const hub = initHub({ base: DEVFRAMES_HUB_BASE, host: '127.0.0.1', ws: { port: wsPort }, devframes: [makeFrame('alpha')] }) diff --git a/packages/hub/src/node/initiate.ts b/packages/hub/src/node/initiate.ts index 1905ec377..ac796faf1 100644 --- a/packages/hub/src/node/initiate.ts +++ b/packages/hub/src/node/initiate.ts @@ -12,7 +12,7 @@ import { existsSync } from 'node:fs' import { readFile } from 'node:fs/promises' import process from 'node:process' import { DEVFRAME_CONNECTION_META_FILENAME, DEVFRAME_DOCK_IMPORTS_FILENAME, DEVFRAME_MCP_ROUTE, DEVFRAME_WS_ROUTE } from 'devframe/constants' -import { createH3DevframeHost, createInstanceShell, importRuntimeModule, resolveInstanceRegister } from 'devframe/internal' +import { createH3DevframeHost, createInstanceShell, importRuntimeModule, resolveInstanceRegister, resolveMcpConfig } from 'devframe/internal' import { mountStaticHandler } from 'devframe/utils/serve-static' import { H3 } from 'h3' import { resolve } from 'pathe' @@ -255,7 +255,10 @@ export interface InitHubOptions { /** * Expose the **aggregate** MCP endpoint at `__mcp` — one * Streamable-HTTP server over the shared context's whole tool registry - * (ids are already namespaced per plugin). Disabled by default. + * (ids are already namespaced per plugin). Disabled by default. When + * enabled it requires an authorization policy: `true` reads the bearer from + * `DEVFRAME_MCP_AUTH_TOKEN` (startup fails with `DF0077` when it is + * missing/empty), an object carries an explicit {@link McpRouteOptions.authorization}. */ mcp?: boolean | McpRouteOptions /** @@ -565,8 +568,11 @@ export function initHub(options: InitHubOptions): HubInstance { // Aggregate MCP — one Streamable-HTTP endpoint over the shared // context's whole registry (tool ids are namespaced per plugin, and the - // wire-name collision policy is `createMcpFetchHandler`'s own). - const mcpConfig = options.mcp === true ? {} : options.mcp + // wire-name collision policy is `createMcpFetchHandler`'s own). Resolving + // the config validates the authorization policy up front: `mcp: true` + // with no `DEVFRAME_MCP_AUTH_TOKEN`, or an object with no + // `authorization`, throws `DF0077` here rather than mounting the route. + const mcpConfig = resolveMcpConfig(options.mcp) if (!mcpConfig) return { context: ctx } @@ -576,6 +582,7 @@ export function initHub(options: InitHubOptions): HubInstance { serverName: options.name ?? 'devframes-hub', serverVersion: options.version ?? '0.0.0', exposeSharedState: true, + authorization: mcpConfig.authorization, allowedOrigins: mcpConfig.allowedOrigins, }) return { context: ctx, mcp: { path: mcpRoute }, dispose: mounted.dispose } diff --git a/packages/next/src/host.ts b/packages/next/src/host.ts index b0e4fa3ba..0ac57ce3a 100644 --- a/packages/next/src/host.ts +++ b/packages/next/src/host.ts @@ -1,4 +1,4 @@ -import type { ConnectionMeta, DevframeHost, DevframeNodeContext, DevframeStorageScope } from 'devframe' +import type { ConnectionMeta, DevframeHost, DevframeNodeContext, DevframeStorageScope, McpAuthorization } from 'devframe' import { DEVFRAME_CONNECTION_META_FILENAME } from 'devframe/constants' import { importRuntimeModule } from 'devframe/internal' import { serveStaticHandler } from 'devframe/utils/serve-static' @@ -26,6 +26,15 @@ export interface CreateDevframeNextHostOptions { } export interface DevframeNextHostMcpOptions { + /** + * The endpoint's identity policy — **required**, since the route grants + * access to privileged agent tools. A bearer token string (matched in + * constant time), a `(request) => boolean` callback, or `false` for an + * origin-only local opt-out. Checked after the origin gate; see + * {@link McpAuthorization}. Back a bearer with an environment variable + * (e.g. `process.env.DEVFRAME_MCP_AUTH_TOKEN`) — never a literal. + */ + authorization: McpAuthorization /** Name reported in the MCP handshake. Default: `'devframe (next)'`. */ serverName?: string /** Version reported in the MCP handshake. Default: `'0.0.0'`. */ @@ -35,7 +44,8 @@ export interface DevframeNextHostMcpOptions { /** * Origin allow-list beyond the loopback default. `false` disables the * origin gate entirely. Note the MCP route rejects `Origin`-less requests - * (see `createMcpFetchHandler`). + * (see `createMcpFetchHandler`). This hardens the request; `authorization` + * proves identity. */ allowedOrigins?: readonly string[] | false } @@ -79,7 +89,7 @@ export interface DevframeNextHost { mountMcp: ( ctx: DevframeNodeContext, path: string, - options?: DevframeNextHostMcpOptions, + options: DevframeNextHostMcpOptions, ) => Promise<{ dispose: () => Promise }> } @@ -163,12 +173,13 @@ export function createDevframeNextHost( setConnectionMeta(meta) { connectionMeta = meta }, - async mountMcp(ctx, path, mcpOptions = {}) { + async mountMcp(ctx, path, mcpOptions) { const { createMcpFetchHandler } = await importRuntimeModule('devframe/adapters/mcp') const handler = createMcpFetchHandler(ctx, { serverName: mcpOptions.serverName ?? 'devframe (next)', serverVersion: mcpOptions.serverVersion ?? '0.0.0', exposeSharedState: mcpOptions.exposeSharedState ?? true, + authorization: mcpOptions.authorization, allowedOrigins: mcpOptions.allowedOrigins, }) const key = stripTrailingSlash(path) diff --git a/packages/next/src/hub.ts b/packages/next/src/hub.ts index ce2c5d2cc..08b2980fc 100644 --- a/packages/next/src/hub.ts +++ b/packages/next/src/hub.ts @@ -50,8 +50,11 @@ export interface NextDevframeHubOptions { /** The hub's single auth gate. Gates by default; `false` opts out. */ auth?: InitHubOptions['auth'] /** - * Expose the aggregate MCP endpoint at `__mcp`. Default: `true` - * (the Next hub's agent surface rides the same catch-all route). + * Expose the aggregate MCP endpoint at `__mcp`. Disabled by default — + * the endpoint exposes privileged agent tools, so opt in with an explicit + * authorization policy: `mcp: true` reads the bearer from + * `DEVFRAME_MCP_AUTH_TOKEN`, or pass an object with an explicit + * `authorization`. */ mcp?: InitHubOptions['mcp'] /** Public origin the Next app is reachable at. Default: derived from `PORT`. */ @@ -70,7 +73,8 @@ export interface NextDevframeHubOptions { * Build a devframes-hub for a Next.js App Router app: one `initHub()` call * mounting every devframe under `/` behind one web-standard * `handler`, with the RPC socket on a side-car (Next routes can't accept WS - * upgrades) and the aggregate MCP route on by default. The UI defaults to + * upgrades) and the aggregate MCP route opt-in (pass `mcp` with an explicit + * authorization policy to enable it). The UI defaults to * `@devframes/hub-ui`'s `createUi()`, loaded lazily via a bundler-ignored * dynamic `import()` so its asset lookups resolve at request time; pass `ui` * to swap it or `ui: false` for a headless hub. @@ -94,8 +98,10 @@ export async function createNextDevframeHub(options: NextDevframeHubOptions = {} auth: options.auth, // Next route handlers can't accept WS upgrades — always a side-car socket. ws: options.port != null ? { port: options.port } : { sidecar: true }, - // The Next hub's agent surface rides the same catch-all route by default. - mcp: options.mcp ?? true, + // MCP is opt-in: the aggregate endpoint exposes privileged agent tools, so + // the caller supplies an explicit authorization policy (undefined leaves + // the route unmounted). + ...(options.mcp !== undefined ? { mcp: options.mcp } : {}), ...(ui ? { ui } : {}), ...(options.renderers ? { renderers: options.renderers } : {}), ...(options.rpcDeclarations ? { rpcDeclarations: options.rpcDeclarations } : {}), diff --git a/packages/next/test/handler.test.ts b/packages/next/test/handler.test.ts index c3c3198de..5f8461373 100644 --- a/packages/next/test/handler.test.ts +++ b/packages/next/test/handler.test.ts @@ -83,7 +83,10 @@ describe('createDevframeNextHandler', () => { const dist = mkdtempSync(join(tmpdir(), 'df-next-mcp-')) writeFileSync(join(dist, 'index.html'), 'ok') - handler = createDevframeNextHandler(makeDef(dist), { host: '127.0.0.1', mcp: true }) + // Origin-only opt-out keeps this loopback-bound handler test free of + // bearer plumbing; the identity gate is covered in devframe's + // mcp-http.test.ts. + handler = createDevframeNextHandler(makeDef(dist), { host: '127.0.0.1', mcp: { authorization: false } }) await handler.ready const meta = await handler.fetch(new Request('http://localhost:3000/__test-next/__connection.json')) diff --git a/packages/vite/test/single.test.ts b/packages/vite/test/single.test.ts index e485a1a38..184a834f3 100644 --- a/packages/vite/test/single.test.ts +++ b/packages/vite/test/single.test.ts @@ -130,7 +130,10 @@ describe('devframeViteBridge (bridge mode mcp)', () => { bridge = devframeViteBridge(defineTestDef(), { port: wsPort, host, - mcp: true, + // Origin-only opt-out: this loopback-bound test dials the MCP route + // directly with a loopback Origin; the identity gate is covered in + // devframe's own mcp-http.test.ts. + mcp: { authorization: false }, // The bridge gates by default; opt out here so this test can dial // the WS side-car and MCP route directly. auth: false, diff --git a/plans/README.md b/plans/README.md index c263bd731..1b08ecf42 100644 --- a/plans/README.md +++ b/plans/README.md @@ -7,7 +7,7 @@ Generated by the improve skill on 2026-09-01 at commit `2d978f84`. Execute in th | Plan | Title | Priority | Effort | Depends on | Status | |---|---|---|---|---|---| | 001 | Pin privileged GitHub Actions dependencies | P1 | S | - | TODO | -| 002 | Require authentication on route-based MCP | P1 | M | 001 | TODO | +| 002 | Require authentication on route-based MCP | P1 | M | 001 | DONE | | 003 | Enforce shared-state exposure policy on direct MCP reads | P1 | S | 002 | TODO | | 004 | Contain remote asset materialization | P1 | S | - | TODO | | 005 | Block Data Inspector prototype-chain writes | P1 | S | - | TODO | diff --git a/tests/__snapshots__/tsnapi/@devframes/next/hub.snapshot.d.ts b/tests/__snapshots__/tsnapi/@devframes/next/hub.snapshot.d.ts index b44dd0472..c8e679145 100644 --- a/tests/__snapshots__/tsnapi/@devframes/next/hub.snapshot.d.ts +++ b/tests/__snapshots__/tsnapi/@devframes/next/hub.snapshot.d.ts @@ -11,11 +11,12 @@ export interface DevframeNextHost { host: DevframeHost; fetch: (_: Request) => Promise; setConnectionMeta: (_: ConnectionMeta) => void; - mountMcp: (_: DevframeNodeContext, _: string, _?: DevframeNextHostMcpOptions) => Promise<{ + mountMcp: (_: DevframeNodeContext, _: string, _: DevframeNextHostMcpOptions) => Promise<{ dispose: () => Promise; }>; } export interface DevframeNextHostMcpOptions { + authorization: McpAuthorization; serverName?: string; serverVersion?: string; exposeSharedState?: boolean | ((_: string) => boolean); diff --git a/tests/__snapshots__/tsnapi/devframe/adapters/mcp.snapshot.d.ts b/tests/__snapshots__/tsnapi/devframe/adapters/mcp.snapshot.d.ts index a55bdd25c..0ea25dcf2 100644 --- a/tests/__snapshots__/tsnapi/devframe/adapters/mcp.snapshot.d.ts +++ b/tests/__snapshots__/tsnapi/devframe/adapters/mcp.snapshot.d.ts @@ -6,6 +6,7 @@ export interface CreateMcpFetchHandlerOptions { serverName: string; serverVersion: string; exposeSharedState: boolean | ((_: string) => boolean); + authorization: McpAuthorization; allowedOrigins?: readonly string[] | false; } export interface CreateMcpServerOptions { diff --git a/tests/__snapshots__/tsnapi/devframe/index.snapshot.d.ts b/tests/__snapshots__/tsnapi/devframe/index.snapshot.d.ts index 57c16eef3..3ba1a55ac 100644 --- a/tests/__snapshots__/tsnapi/devframe/index.snapshot.d.ts +++ b/tests/__snapshots__/tsnapi/devframe/index.snapshot.d.ts @@ -390,6 +390,7 @@ export interface EventUnsubscribe { } export interface McpRouteOptions { path?: string; + authorization: McpAuthorization; allowedOrigins?: readonly string[] | false; } export interface RemoteAssets { @@ -493,6 +494,7 @@ export type DevframeSnapshotRpcEntry = string | { }; export type DevframeSnapshotRpcInputs = readonly (readonly unknown[])[] | ((_: DevframeNodeContext) => readonly (readonly unknown[])[] | Promise); export type DevframeStorageScope = 'workspace' | 'project' | 'global'; +export type McpAuthorization = string | ((_: Request) => boolean | Promise) | false; export type RemoteAssetsProvider = 'jsdelivr' | 'unpkg' | RemoteAssetsProviderCustom; export type RpcFunctionsHost = RpcFunctionsCollectorBase & { invokeLocal: >(_: T, ..._: Args) => Promise>>; diff --git a/tests/__snapshots__/tsnapi/devframe/internal.snapshot.d.ts b/tests/__snapshots__/tsnapi/devframe/internal.snapshot.d.ts index 1915953ec..80175bae6 100644 --- a/tests/__snapshots__/tsnapi/devframe/internal.snapshot.d.ts +++ b/tests/__snapshots__/tsnapi/devframe/internal.snapshot.d.ts @@ -361,6 +361,10 @@ export declare const diagnostics: import("nostics").Diagnostics<{ }) => string; readonly fix: "On Bun/Deno, serve the advertised `__ws` route from `Bun.serve` / `Deno.serve` with `attachBunWsTransport` / `attachDenoWsTransport` (see the hub-deno-minimal example), or connect over the SSE endpoint instead."; }; + readonly DF0077: { + readonly why: "The route-based MCP server needs an authorization policy, but none is configured — refusing to mount an unauthenticated agent endpoint."; + readonly fix: "Set the `DEVFRAME_MCP_AUTH_TOKEN` environment variable (the bearer `mcp: true` requires), or pass an explicit `authorization` on the MCP options — a non-empty bearer token string, a `(request) => boolean` callback, or `false` for an origin-only local opt-out."; + }; }, readonly [(d: import("nostics").Diagnostic, { method }?: { method?: "log" | "warn" | "error"; }) => void]>; @@ -384,7 +388,9 @@ export { listLiveDevframeInstances } export { normalizeBasePath } export { registerDevframeInstance } export { resolveBasePath } +export { ResolvedMcpConfig } export { resolveInstanceRegister } +export { resolveMcpConfig } export { samePath } export { StartedServer } // #endregion \ No newline at end of file diff --git a/tests/__snapshots__/tsnapi/devframe/internal.snapshot.js b/tests/__snapshots__/tsnapi/devframe/internal.snapshot.js index 6dc2fe5e9..9b3a95de9 100644 --- a/tests/__snapshots__/tsnapi/devframe/internal.snapshot.js +++ b/tests/__snapshots__/tsnapi/devframe/internal.snapshot.js @@ -18,5 +18,6 @@ export { registerDevframeInstance } export { resolveBasePath } export { resolveClientAssets } export { resolveInstanceRegister } +export { resolveMcpConfig } export { samePath } // #endregion \ No newline at end of file diff --git a/tests/__snapshots__/tsnapi/devframe/types.snapshot.d.ts b/tests/__snapshots__/tsnapi/devframe/types.snapshot.d.ts index 99be36470..9ad670e3c 100644 --- a/tests/__snapshots__/tsnapi/devframe/types.snapshot.d.ts +++ b/tests/__snapshots__/tsnapi/devframe/types.snapshot.d.ts @@ -64,6 +64,7 @@ export { DevframeWsOptions } export { EventEmitter } export { EventsMap } export { EventUnsubscribe } +export { McpAuthorization } export { McpRouteOptions } export { RemoteAssets } export { RemoteAssetsErrorMessage } diff --git a/tests/optional-mcp-bundles.test.ts b/tests/optional-mcp-bundles.test.ts index e2a9c1ce5..8d7ac6150 100644 --- a/tests/optional-mcp-bundles.test.ts +++ b/tests/optional-mcp-bundles.test.ts @@ -63,7 +63,9 @@ describe('optional MCP peers in consumer bundles', () => { const hub = bundled.initHub({ auth: false, base: bundled.DEVFRAMES_HUB_BASE, - mcp: true, + // Origin-only opt-out: this loopback-bound bundle-load smoke test only + // needs the route mounted; the identity gate is covered elsewhere. + mcp: { authorization: false }, ws: false, }) From f7b7c1e42d38a02f7f90913cbf5aa55f29152b0e Mon Sep 17 00:00:00 2001 From: "Anthony Fu (via agent)" Date: Tue, 1 Sep 2026 07:02:01 +0000 Subject: [PATCH 2/4] test(e2e): supply DEVFRAME_MCP_AUTH_TOKEN to the next-devframe-hub connect flow The hub-next example now enables its aggregate MCP route with the env-backed mcp: true policy, so the Next server needs the bearer at boot and the connect spec's spawned connector needs the same one. Share one token between playwright.config's hub-next webServer env and the withConnectClient spawn env. --- playwright.config.ts | 5 ++++- tests/e2e/_support/mcp-auth.ts | 8 ++++++++ tests/e2e/_support/mcp-connect.ts | 7 ++++++- 3 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 tests/e2e/_support/mcp-auth.ts diff --git a/playwright.config.ts b/playwright.config.ts index 74ad64281..4b3022309 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -1,6 +1,7 @@ import process from 'node:process' import { fileURLToPath } from 'node:url' import { defineConfig, devices } from '@playwright/test' +import { MCP_AUTH_TOKEN } from './tests/e2e/_support/mcp-auth' const fixtureCwd = fileURLToPath(new URL('./tests/e2e/fixtures', import.meta.url)) const serveStatic = fileURLToPath(new URL('./tests/e2e/_support/serve-static.mjs', import.meta.url)) @@ -83,7 +84,9 @@ export default defineConfig({ { command: 'pnpm exec next dev src/client -p 9878', cwd: 'examples/hub-next', - env: { PORT: '9878', DEVFRAME_INSTANCES_DIR: nextHubRegistry }, + // The example's aggregate MCP route uses the env-backed `mcp: true` + // policy, so the server needs the same bearer the connect spec presents. + env: { PORT: '9878', DEVFRAME_INSTANCES_DIR: nextHubRegistry, DEVFRAME_MCP_AUTH_TOKEN: MCP_AUTH_TOKEN }, url: 'http://localhost:9878/', timeout: 120_000, reuseExistingServer: !process.env.CI, diff --git a/tests/e2e/_support/mcp-auth.ts b/tests/e2e/_support/mcp-auth.ts new file mode 100644 index 000000000..2d7919774 --- /dev/null +++ b/tests/e2e/_support/mcp-auth.ts @@ -0,0 +1,8 @@ +/** + * The bearer `devframe connect` presents to an authenticated instance MCP + * route (via `DEVFRAME_MCP_AUTH_TOKEN`). Shared between `playwright.config.ts` + * (which starts the `hub-next` server with it) and the connect support helper + * (which spawns the connector with it), so the two agree on the credential. + * Kept dependency-free so the Playwright config can import it cheaply. + */ +export const MCP_AUTH_TOKEN = 'devframe-e2e-mcp-auth-token' diff --git a/tests/e2e/_support/mcp-connect.ts b/tests/e2e/_support/mcp-connect.ts index 4c4a93d3a..1aafca415 100644 --- a/tests/e2e/_support/mcp-connect.ts +++ b/tests/e2e/_support/mcp-connect.ts @@ -1,12 +1,15 @@ import { fileURLToPath } from 'node:url' import { Client } from '@modelcontextprotocol/client' import { StdioClientTransport } from '@modelcontextprotocol/client/stdio' +import { MCP_AUTH_TOKEN } from './mcp-auth' const BIN = fileURLToPath(new URL('../../../packages/devframe/bin/devframe.mjs', import.meta.url)) /** * Spawn `devframe connect` over stdio against a hermetic registry dir and - * hand a connected MCP client to `fn`, tearing the process down after. + * hand a connected MCP client to `fn`, tearing the process down after. The + * connector reads `DEVFRAME_MCP_AUTH_TOKEN` to authenticate against instances + * whose MCP route requires a bearer (harmless for origin-only routes). */ export async function withConnectClient( instancesDir: string, @@ -15,6 +18,8 @@ export async function withConnectClient( const transport = new StdioClientTransport({ command: 'node', args: [BIN, 'connect', '--instances-dir', instancesDir], + // Merged with the SDK's safe default env (which carries PATH etc.). + env: { DEVFRAME_MCP_AUTH_TOKEN: MCP_AUTH_TOKEN }, }) const client = new Client({ name: 'devframe-e2e', version: '0.0.0' }) await client.connect(transport) From e6792d2ccf11f9aeb5746737f92455e2950e3275 Mon Sep 17 00:00:00 2001 From: "Anthony Fu (via agent)" Date: Thu, 3 Sep 2026 02:09:32 +0000 Subject: [PATCH 3/4] refactor(devframe): trust same-machine MCP callers, make identity opt-in Reshape the route-based MCP auth model per review: the origin gate already keeps browsers and remote hosts out, so a same-machine caller is trusted by default. mcp: true is now origin-only (no bearer, no env var), and authorization (bearer or callback) is opt-in hardening for when a same-machine process is not the trust boundary (LAN/tunnel origin, shared/CI host, destructive tools). - McpRouteOptions.authorization is optional, defaulting to origin-only; drop the DEVFRAME_MCP_AUTH_TOKEN server shorthand and the DF0077 startup failure. - Promote mcp to a top-level DevframeDefinition.mcp option; cli.mcp stays as a deprecated fallback (mirrors clientAssets/cli.distDir). - createMcpFetchHandler / DevframeNextHost.mountMcp authorization is optional again (reverts the breaking narrowing). - Hub: warn (DF8005) when a mounted devframe requests MCP but the hub's aggregate MCP is off, since the hub's single route governs it. - Revert the now-unneeded e2e/example bearer plumbing; the connector keeps authToken as an opt-in for hardened instances. Created with the help of an agent. --- docs/content/1.guide/14.security.md | 2 +- docs/content/1.guide/18.hub-initiate.md | 2 +- docs/content/2.adapters/7.mcp.md | 39 +++-- docs/content/3.frameworks/1.vite.md | 2 +- docs/content/3.frameworks/3.next.md | 4 +- docs/content/6.errors/DF0077.md | 49 ------ docs/content/6.errors/DF8005.md | 33 ++++ docs/content/6.errors/index.md | 1 + examples/files-inspector/src/devframe.ts | 14 +- .../src/client/devframe/next-devframe-hub.ts | 8 +- .../hub-next/tests/next-devframe-hub.test.ts | 13 +- .../src/adapters/__tests__/dev.test.ts | 4 +- .../src/adapters/__tests__/initiate.test.ts | 11 +- packages/devframe/src/adapters/_shared.ts | 36 ++--- packages/devframe/src/adapters/cac.ts | 7 +- packages/devframe/src/adapters/dev.ts | 6 +- packages/devframe/src/adapters/initiate.ts | 14 +- .../adapters/mcp/__tests__/mcp-http.test.ts | 144 ++++++++++-------- packages/devframe/src/adapters/mcp/fetch.ts | 30 ++-- packages/devframe/src/node/diagnostics.ts | 4 - packages/devframe/src/types/devframe.ts | 73 +++++---- .../hub/src/node/__tests__/initiate.test.ts | 35 ++++- packages/hub/src/node/diagnostics.ts | 4 + packages/hub/src/node/initiate.ts | 23 ++- packages/next/src/handler.ts | 4 +- packages/next/src/host.ts | 18 +-- packages/next/src/hub.ts | 12 +- packages/next/test/handler.test.ts | 5 +- packages/vite/src/single.ts | 5 +- packages/vite/test/single.test.ts | 5 +- playwright.config.ts | 5 +- .../tsnapi/@devframes/next/hub.snapshot.d.ts | 4 +- .../devframe/adapters/mcp.snapshot.d.ts | 2 +- .../tsnapi/devframe/index.snapshot.d.ts | 3 +- .../tsnapi/devframe/internal.snapshot.d.ts | 4 - tests/e2e/_support/mcp-auth.ts | 8 - tests/e2e/_support/mcp-connect.ts | 7 +- tests/optional-mcp-bundles.test.ts | 4 +- 38 files changed, 306 insertions(+), 338 deletions(-) delete mode 100644 docs/content/6.errors/DF0077.md create mode 100644 docs/content/6.errors/DF8005.md delete mode 100644 tests/e2e/_support/mcp-auth.ts diff --git a/docs/content/1.guide/14.security.md b/docs/content/1.guide/14.security.md index 7c3d443b9..0dc454d29 100644 --- a/docs/content/1.guide/14.security.md +++ b/docs/content/1.guide/14.security.md @@ -75,7 +75,7 @@ For your own auth UI, disable built-in handling with `otpParam: false`, then cal - **Stay on loopback.** Bind to a routable address only intentionally, and require authentication when you do. - **Keep `auth: false` local.** The hosted bridges (`devframeViteBridge`, `@devframes/next`'s handler) gate their side-car by default; opt out with an explicit `auth: false` only when the host framework owns the trust boundary another way. -- **The MCP route authenticates the caller.** `Origin` hardens the route-based MCP server against DNS-rebinding, but proves nothing about identity — a native client can send any `Origin`. So the route also requires a bearer: `mcp: true` reads it from `DEVFRAME_MCP_AUTH_TOKEN`, and the route refuses to mount ([`DF0077`](/errors/DF0077)) without a policy. Treat the two checks as separate defenses — see [MCP](/adapters/mcp). +- **The MCP route trusts same-machine callers, harden it when that's not your boundary.** The origin gate keeps browsers and remote hosts out (loopback-only, `Origin`-less rejected), so `mcp: true` is enough for a local dev tool. `Origin` proves nothing about *which* local process is calling, though — so when the route is reachable beyond loopback (a widened `allowedOrigins`, a hosted app) or exposes destructive tools, add an identity check with `mcp: { authorization }` (a bearer from an env var, or a callback). See [MCP](/adapters/mcp). - **Treat tokens as secrets.** Never log the bearer token or the one-time code, or bake either into build output. - **Authorize every handler.** Validate inputs, and mark state-changing functions `type: 'destructive'` so MCP and agent clients prompt before invoking them. - **Origin-lock remote docks.** When a hub embeds a remote-UI dock, keep `originLock` on (the default) so its session token is only honored on a connection whose `Origin` matches the dock's own. diff --git a/docs/content/1.guide/18.hub-initiate.md b/docs/content/1.guide/18.hub-initiate.md index b83247f6c..7c164276e 100644 --- a/docs/content/1.guide/18.hub-initiate.md +++ b/docs/content/1.guide/18.hub-initiate.md @@ -82,7 +82,7 @@ Registrations are validated fail-fast: one module per type (`DF8108`), an existi The hub's **single Auth** is one gate at the shared transport for every mounted devframe, built-ins, and the MCP route; one handshake (OTP, magic link, or pre-shared token) unlocks the namespace; `auth: false` disables it for localhost. -The aggregate MCP route carries its **own** identity gate independent of this RPC Auth, since it grants agent clients privileged tool access: `mcp: true` requires the `DEVFRAME_MCP_AUTH_TOKEN` bearer (startup fails with [`DF0077`](/errors/DF0077) without it), or pass `mcp: { authorization }` explicitly. `Origin` remains request hardening, not identity. +The aggregate MCP route has its own origin gate, independent of this RPC Auth: `mcp: true` trusts same-machine callers, and `mcp: { authorization }` adds an identity check when the hub is reachable beyond loopback. A mounted devframe's own `mcp` setting is ignored — the hub exposes one aggregate route over them all, and warns ([`DF8005`](/errors/DF8005)) when a devframe asks for MCP while the hub's is off. ## Singular vs hub mounting diff --git a/docs/content/2.adapters/7.mcp.md b/docs/content/2.adapters/7.mcp.md index 5cebbef62..a35b6ad25 100644 --- a/docs/content/2.adapters/7.mcp.md +++ b/docs/content/2.adapters/7.mcp.md @@ -18,17 +18,14 @@ await createMcpServer(myDevframe, { transport: 'stdio' }) ## Route-based server -The dev server exposes the same MCP API over HTTP, live. Enable with `cli.mcp`: +The dev server exposes the same MCP API over HTTP, live. Enable with the top-level `mcp`: ```ts import { defineDevframe } from 'devframe' export default defineDevframe({ // … - cli: { - // Reads the bearer from DEVFRAME_MCP_AUTH_TOKEN. - mcp: true, - }, + mcp: true, }) ``` @@ -36,27 +33,25 @@ The endpoint speaks Streamable-HTTP at `/__mcp` (`/__/__mcp` under a host fr The endpoint is **stateless**: it serves the [2026-07-28 revision](https://modelcontextprotocol.io/specification/2026-07-28) per request through the SDK's `createMcpHandler`, building a fresh MCP server for each request — every HTTP request stands alone, with no `Mcp-Session-Id` to correlate. 2025-era clients are still served through the SDK's stateless legacy path. -### Two gates: origin hardening and identity +### Origin gate, and opt-in identity -The route exposes privileged agent tools, so every request clears two independent gates. The **origin gate** requires `Origin` be loopback (or allow-listed) and rejects `Origin`-less requests — DNS-rebinding hardening that proves nothing about *who* is calling, since a native client can send any `Origin`. Widen it for a tunnel/LAN origin with `cli: { mcp: { authorization: process.env.MY_TOKEN, allowedOrigins: ['https://tunnel.example.com'] } }`. +The **origin gate** guards every request: `Origin` must be loopback (or allow-listed), and `Origin`-less requests are rejected (a disallowed origin gets `403`). This is DNS-rebinding hardening that keeps browsers and remote hosts out, and it trusts same-machine callers — so `mcp: true` is all a local dev tool needs. -The **identity gate** then proves the caller. `mcp: true` reads its bearer from the `DEVFRAME_MCP_AUTH_TOKEN` environment variable; a request presents it as `Authorization: Bearer ` and it is matched in constant time. A missing or wrong bearer gets `401` with a `WWW-Authenticate: Bearer` challenge; a disallowed origin gets `403`. Startup fails with [`DF0077`](/errors/DF0077) — the route is never mounted — when `mcp: true` finds no environment token, or an object config omits `authorization`. - -An object config sets the policy explicitly: +`Origin` proves nothing about *who* is calling, though: a native process on the same box can send any `Origin`. When a same-machine process isn't your trust boundary — a LAN/tunnel origin, a shared/CI host, a destructive tool surface — layer on an **identity check** with `authorization`: ```ts export default defineDevframe({ - cli: { - // A bearer from your own environment variable: - mcp: { authorization: process.env.MY_TOKEN }, - // — or a callback identity check (governs identity only; it cannot relax the origin gate): - // mcp: { authorization: request => isTrusted(request) }, - // — or an origin-only opt-out for a loopback-bound local tool that owns its trust boundary another way: - // mcp: { authorization: false }, - }, + // A bearer token, backed by your own environment variable (never a literal): + mcp: { authorization: process.env.MY_TOKEN }, + // — or a callback identity check (governs identity only; it cannot relax the origin gate): + // mcp: { authorization: request => isTrusted(request) }, + // — or the explicit default, origin-only: + // mcp: { authorization: false }, }) ``` +A request presents the bearer as `Authorization: Bearer `, matched in constant time; a missing or wrong bearer gets `401` with a `WWW-Authenticate: Bearer` challenge. The origin gate always runs first, so a disallowed origin is `403` regardless of the credential. Widen the origin allow-list for a tunnel/LAN reach with `mcp: { authorization: process.env.MY_TOKEN, allowedOrigins: ['https://tunnel.example.com'] }`. + Never place the token in a URL, in `__connection.json`, in the instance registry, in logs, or on the command line — it belongs only in configuration and the `Authorization` header. ### Hosted bridges @@ -71,7 +66,7 @@ devframeViteBridge(myDevframe, { mcp: true }) createDevframeNextHandler(myDevframe, { mcp: true }) ``` -Both honor the same authorization contract: `mcp: true` requires the `DEVFRAME_MCP_AUTH_TOKEN` bearer, or pass `mcp: { authorization }` explicitly. +Both honor the same contract: `mcp: true` is origin-only; add `mcp: { authorization }` to harden. ## Custom host frameworks @@ -84,8 +79,8 @@ const mcp = createMcpFetchHandler(ctx, { serverName: 'my-tool (devframe)', serverVersion: '1.0.0', exposeSharedState: true, - // Required: the identity policy — a bearer token, a callback, or `false`. - authorization: process.env.DEVFRAME_MCP_AUTH_TOKEN!, + // Optional identity check on top of the origin gate; omit for origin-only. + // authorization: process.env.MY_TOKEN, }) // route every method on /__mcp to mcp.fetch(request) ``` @@ -109,6 +104,6 @@ Two gateway tools (`devframe:connect:*` ids — see [tool ids and wire names](/g Discovery reads the **instance registry**: every `createDevServer` writes `~/.devframe/instances/-.json`, dialed with a loopback origin. In-process host frameworks register via `registerDevframeInstance` (`devframe/node`). `--port ` probes a port; `DEVFRAME_INSTANCES_DIR` relocates the registry, `DEVFRAME_DISABLE_INSTANCE_REGISTRY=1` opts out. -The connector reads `DEVFRAME_MCP_AUTH_TOKEN` and presents it as the bearer to each instance's authenticated route (never a CLI flag — command-line arguments are visible to other processes). An instance whose route requires a different bearer reports auth-required rather than being reached; connect to a fleet with distinct credentials by driving `startConnectServer` with a per-instance `authToken` resolver. +Most instances trust same-machine callers, so the connector reaches them with no credential. For an instance you *hardened* with a bearer, the connector reads `DEVFRAME_MCP_AUTH_TOKEN` and presents it (never a CLI flag — command-line arguments are visible to other processes). Connect to a fleet with distinct credentials by driving `startConnectServer` with a per-instance `authToken` resolver. See [Agent-Native](/guide/agent-native) for the API and safety model. diff --git a/docs/content/3.frameworks/1.vite.md b/docs/content/3.frameworks/1.vite.md index daccb61c9..4fb1ce6a5 100644 --- a/docs/content/3.frameworks/1.vite.md +++ b/docs/content/3.frameworks/1.vite.md @@ -42,7 +42,7 @@ Devframe spawns a separate RPC + WS server and registers Vite middleware at `__mcp`. `true` requires the `DEVFRAME_MCP_AUTH_TOKEN` bearer; `McpRouteOptions` carries an explicit `authorization`. | +| `mcp` | `def.mcp` | Expose the MCP route at `__mcp`. `true` is origin-only (trusts same-machine callers); `McpRouteOptions` can add an `authorization` identity check. | ## `devframeVite` — convenience wrapper diff --git a/docs/content/3.frameworks/3.next.md b/docs/content/3.frameworks/3.next.md index 826412709..4c5c6ef0d 100644 --- a/docs/content/3.frameworks/3.next.md +++ b/docs/content/3.frameworks/3.next.md @@ -48,7 +48,7 @@ export const GET = handler.fetch | `port` | from `def.cli?.port` | Side-car port. | | `flags` | — | Passed to `def.setup(ctx, { flags })`. | | `auth` | `false` | `true` for the OTP gate, or a handler. | -| `mcp` | `def.cli?.mcp` | Expose the MCP route. `true` requires the `DEVFRAME_MCP_AUTH_TOKEN` bearer; `McpRouteOptions` carries an explicit `authorization`. | +| `mcp` | `def.mcp` | Expose the MCP route. `true` is origin-only (trusts same-machine callers); `McpRouteOptions` can add an `authorization` identity check. | | `key` | `@devframes/next::` | `globalThis` memoization key. | ## Hosting a hub @@ -125,7 +125,7 @@ export const POST = (req: Request) => hub.handler(req) export const DELETE = (req: Request) => hub.handler(req) ``` -The aggregate MCP route is off by default — it exposes privileged agent tools. Opt in with an authorization policy: `mcp: true` (requiring the `DEVFRAME_MCP_AUTH_TOKEN` bearer) or `mcp: { authorization }`. +The aggregate MCP route is off by default. Opt in with `mcp: true` (origin-only, trusting same-machine callers), or `mcp: { authorization }` to add an identity check when the app is reachable beyond localhost. No native hub UI provider here, so this scope stays quiet; `createDevframeNextHost()` is the low-level `DevframeHost`. diff --git a/docs/content/6.errors/DF0077.md b/docs/content/6.errors/DF0077.md deleted file mode 100644 index 905a1325b..000000000 --- a/docs/content/6.errors/DF0077.md +++ /dev/null @@ -1,49 +0,0 @@ ---- -title: 'DF0077: MCP Authorization Required' -description: 'The route-based MCP server needs an authorization policy, but none is configured.' ---- - -## Message - -> The route-based MCP server needs an authorization policy, but none is configured — refusing to mount an unauthenticated agent endpoint. - -## Cause - -The route-based MCP endpoint exposes privileged agent tools to any process that can reach it. `Origin` hardens the request against DNS-rebinding but proves nothing about *who* is calling, so the route also requires an identity policy. This diagnostic fires when that policy is absent: - -- `mcp: true` (the shorthand) reads its bearer from the `DEVFRAME_MCP_AUTH_TOKEN` environment variable, and the variable is missing or empty. -- An object MCP config omits the required `authorization` field (or sets it to an empty string). - -Startup fails and the route is never mounted, rather than exposing the endpoint unauthenticated. - -## Example - -```ts -// ✗ throws DF0077 when DEVFRAME_MCP_AUTH_TOKEN is unset -await createDevServer(def, { mcp: true }) - -// ✗ throws DF0077 — object config with no authorization -await createDevServer(def, { mcp: { path: '__mcp' } }) - -// ✓ shorthand, with the environment token set -process.env.DEVFRAME_MCP_AUTH_TOKEN = 'a-high-entropy-secret' -await createDevServer(def, { mcp: true }) - -// ✓ explicit bearer token -await createDevServer(def, { mcp: { authorization: process.env.MY_TOKEN! } }) - -// ✓ callback identity check -await createDevServer(def, { mcp: { authorization: req => isTrusted(req) } }) - -// ✓ origin-only opt-out for a loopback-bound local tool -await createDevServer(def, { mcp: { authorization: false } }) -``` - -## Fix - -- Set the `DEVFRAME_MCP_AUTH_TOKEN` environment variable to the bearer the `mcp: true` shorthand requires. -- Or pass an explicit `authorization` on the MCP options — a non-empty bearer token string, a `(request) => boolean` callback, or `false` for an origin-only local opt-out. - -## Source - -- [`packages/devframe/src/adapters/_shared.ts`](https://github.com/devframes/devframe/blob/main/packages/devframe/src/adapters/_shared.ts) — `resolveMcpConfig()` throws this when the `mcp: true` shorthand has no environment token, or an object config omits `authorization`. diff --git a/docs/content/6.errors/DF8005.md b/docs/content/6.errors/DF8005.md new file mode 100644 index 000000000..fba479565 --- /dev/null +++ b/docs/content/6.errors/DF8005.md @@ -0,0 +1,33 @@ +--- +title: 'DF8005: Devframe MCP Ignored While Hub MCP Is Off' +description: 'Devframe "{id}" requests an MCP route, but the hub''s aggregate MCP is off — its tools are not exposed over MCP.' +--- + +## Message + +> Devframe "`{id}`" requests an MCP route, but the hub's aggregate MCP is off — its tools are not exposed over MCP. + +## Cause + +A hub exposes **one aggregate MCP endpoint** over every mounted devframe (tool ids are already namespaced per plugin), so a mounted devframe's own `mcp` setting is ignored — the hub's own `mcp` governs the route. This warning fires when a devframe is mounted with `mcp` enabled (top-level `mcp`, or the deprecated `cli.mcp`) while the hub itself has no `mcp` configured, so that devframe's tools are not reachable over MCP. + +## Example + +```ts +initHub({ + base: DEVFRAMES_HUB_BASE, + // No `mcp` here → no aggregate route… + devframes: [ + myDevframe, // …but this devframe declares `mcp: true` → DF8005 + ], +}) +``` + +## Fix + +- Enable the hub's own aggregate MCP so the devframe's tools are surfaced: pass `mcp` to `initHub` (`mcp: true` for the loopback origin gate, or `mcp: { authorization }` to add an identity check). +- Or drop `mcp` from the mounted devframe to silence the warning — it has no effect inside a hub. + +## Source + +- [`packages/hub/src/node/initiate.ts`](https://github.com/devframes/devframe/blob/main/packages/hub/src/node/initiate.ts) — `initHub()` emits this while mounting each devframe when the hub has no MCP but the devframe requests one. diff --git a/docs/content/6.errors/index.md b/docs/content/6.errors/index.md index 12d871834..752ba581d 100644 --- a/docs/content/6.errors/index.md +++ b/docs/content/6.errors/index.md @@ -92,6 +92,7 @@ Emitted by `@devframes/hub` while assembling and mounting the unified surface. | [DF8002](/errors/DF8002) | error | Both devframes and context Passed to initHub | | [DF8003](/errors/DF8003) | error | connectionMeta() Before Hub Instance Ready | | [DF8004](/errors/DF8004) | error | Devframe Id Is Not a Mountable URL Segment | +| [DF8005](/errors/DF8005) | warning | Devframe MCP Ignored While Hub MCP Is Off | ## Hub — docks & mounting (DF81xx) diff --git a/examples/files-inspector/src/devframe.ts b/examples/files-inspector/src/devframe.ts index 5fcbf4c02..539f29f45 100644 --- a/examples/files-inspector/src/devframe.ts +++ b/examples/files-inspector/src/devframe.ts @@ -17,20 +17,18 @@ export default defineDevframe({ icon: 'ph:folder-open-duotone', basePath: BASE_PATH, clientAssets: distDir, + // Serve the agent surface over the dev server's `/__mcp` route and register + // the instance for `devframe connect` discovery. This demo binds to loopback + // (`localhost:9876`), so `mcp: true` (the loopback origin gate, trusting + // same-machine callers) is enough - no bearer plumbing. A network-reachable + // tool would harden it with `mcp: { authorization: process.env.MY_TOKEN }`. + mcp: true, cli: { command: 'devframe-files-inspector', port: 9876, // Single-user localhost demo - skip the trust handshake so the served // SPA can call RPC without an OTP round-trip. auth: false, - // Serve the agent surface over the dev server's `/__mcp` route and - // register the instance for `devframe connect` discovery. This demo binds - // to loopback (`localhost:9876`), so it takes the origin-only opt-out - // (`authorization: false`) rather than requiring a bearer - the MCP route - // stays reachable to `devframe connect` on the same machine without token - // plumbing. A network-reachable tool would set a real bearer instead - // (e.g. `authorization: process.env.DEVFRAME_MCP_AUTH_TOKEN`). - mcp: { authorization: false }, }, setup(ctx) { // A scoped context auto-namespaces every registered id with `NAMESPACE:`. diff --git a/examples/hub-next/src/client/devframe/next-devframe-hub.ts b/examples/hub-next/src/client/devframe/next-devframe-hub.ts index 15394be21..638002ece 100644 --- a/examples/hub-next/src/client/devframe/next-devframe-hub.ts +++ b/examples/hub-next/src/client/devframe/next-devframe-hub.ts @@ -217,11 +217,9 @@ export async function nextDevframeHub( // for a bearer token. See `docs/content/1.guide/13.security.md`. // The aggregate MCP endpoint at `/__devframes/__mcp` - the hub's agent // surface (agent-flagged commands, plugin tools, `devframe:state:read`) - // over the same catch-all route as the SPAs. `mcp: true` is the - // environment-backed policy: it reads the required bearer from - // `DEVFRAME_MCP_AUTH_TOKEN`, so startup fails (DF0077) unless that is set - - // the route is never mounted unauthenticated. An MCP client presents that - // token as `Authorization: Bearer ` alongside a loopback Origin. + // over the same catch-all route as the SPAs. `mcp: true` mounts it with + // the loopback origin gate (trusting same-machine callers); harden it with + // `mcp: { authorization }` when the app is reachable beyond localhost. mcp: true, // This host renders its own React UI in `app/page.tsx`, so skip the // default `@devframes/hub-ui` standalone/embedded slot. diff --git a/examples/hub-next/tests/next-devframe-hub.test.ts b/examples/hub-next/tests/next-devframe-hub.test.ts index c958deb27..289903250 100644 --- a/examples/hub-next/tests/next-devframe-hub.test.ts +++ b/examples/hub-next/tests/next-devframe-hub.test.ts @@ -3,23 +3,12 @@ import { getTempAuthCode } from 'devframe/node/auth' import { createRpcClient } from 'devframe/rpc/client' import { createWsRpcChannel } from 'devframe/rpc/transports/ws-client' import { getPort } from 'get-port-please' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import { WebSocket } from 'ws' import { nextDevframeHub } from '../src/client/devframe/next-devframe-hub' vi.stubGlobal('WebSocket', WebSocket) -// The example enables its aggregate MCP route with the environment-backed -// `mcp: true` policy, so a bearer must be configured or the hub refuses to -// start (DF0077). Provide it for the duration of each test. -beforeEach(() => { - vi.stubEnv('DEVFRAME_MCP_AUTH_TOKEN', 'a-high-entropy-example-test-token') -}) - -afterEach(() => { - vi.unstubAllEnvs() -}) - /** The side-car WS port advertised by the hub's connection meta. */ function wsPortOf(hub: HubInstance): number { const ws = hub.connectionMeta().websocket diff --git a/packages/devframe/src/adapters/__tests__/dev.test.ts b/packages/devframe/src/adapters/__tests__/dev.test.ts index b476e21cf..21e5acd99 100644 --- a/packages/devframe/src/adapters/__tests__/dev.test.ts +++ b/packages/devframe/src/adapters/__tests__/dev.test.ts @@ -782,9 +782,7 @@ describe('adapters/dev', () => { host: '127.0.0.1', port: 0, auth: false, - // Origin-only opt-out keeps this loopback-bound registry test free of - // bearer plumbing; the identity gate is covered in mcp-http.test.ts. - mcp: { authorization: false }, + mcp: true, }) const { readDevframeInstances } = await import('../../node/instance-registry') diff --git a/packages/devframe/src/adapters/__tests__/initiate.test.ts b/packages/devframe/src/adapters/__tests__/initiate.test.ts index b1b9b3147..0da1e109e 100644 --- a/packages/devframe/src/adapters/__tests__/initiate.test.ts +++ b/packages/devframe/src/adapters/__tests__/initiate.test.ts @@ -245,9 +245,7 @@ describe('adapters/handler', () => { it('mcp: mounts __mcp and advertises it in the meta', async () => { const wsPort = await getPort({ port: 18140, host: '127.0.0.1' }) - // An explicit origin-only opt-out keeps this loopback-bound fixture free of - // bearer plumbing; the identity gate itself is covered in mcp-http.test.ts. - const devtools = initDevframe(defineTestDef('handler-mcp'), { base: '/__handler-mcp/', auth: false, mcp: { authorization: false }, ws: { port: wsPort } }) + const devtools = initDevframe(defineTestDef('handler-mcp'), { base: '/__handler-mcp/', auth: false, mcp: true, ws: { port: wsPort } }) try { await devtools.ready @@ -264,13 +262,6 @@ describe('adapters/handler', () => { } }) - it('mcp: true without DEVFRAME_MCP_AUTH_TOKEN fails startup (DF0077), route absent', async () => { - const wsPort = await getPort({ port: 18145, host: '127.0.0.1' }) - const devtools = initDevframe(defineTestDef('handler-mcp-noauth'), { base: '/__handler-mcp-noauth/', auth: false, mcp: true, ws: { port: wsPort } }) - await expect(devtools.ready).rejects.toThrow(/DF0077|authorization policy/) - await devtools.close() - }) - it('default tier: binds nothing until the host attaches its own server', async () => { const host = '127.0.0.1' const port = await getPort({ port: 18150, host }) diff --git a/packages/devframe/src/adapters/_shared.ts b/packages/devframe/src/adapters/_shared.ts index c00272cdb..a22b734a6 100644 --- a/packages/devframe/src/adapters/_shared.ts +++ b/packages/devframe/src/adapters/_shared.ts @@ -1,10 +1,8 @@ import type { ConnectionMeta } from '../types/context' import type { DevframeDefinition, DevframeDeploymentKind, McpAuthorization, McpRouteOptions } from '../types/devframe' -import process from 'node:process' import { getPort } from 'get-port-please' import { cleanDoubleSlashes, withLeadingSlash, withoutLeadingSlash, withTrailingSlash } from 'ufo' import { DEVFRAME_MCP_ROUTE } from '../constants' -import { diagnostics } from '../node/diagnostics' const DEFAULT_PORT = 9999 @@ -72,28 +70,22 @@ export interface ResolvedMcpConfig { } /** - * Normalize the `cli.mcp` / `mcp` option (`boolean | McpRouteOptions`) into a + * Normalize the `mcp` option (`boolean | McpRouteOptions`) into a * fully-resolved config, or `undefined` when the MCP route is disabled. * - * The route grants access to privileged agent tools, so an enabled route - * always resolves to a concrete authorization policy. `mcp: true` is shorthand - * for the bearer read from `DEVFRAME_MCP_AUTH_TOKEN`; an object config must - * carry an explicit `authorization`. A missing/empty token or absent - * `authorization` throws {@link diagnostics.DF0077} so the route is never - * mounted unauthenticated. + * An enabled route trusts same-machine callers by default: the authorization + * resolves to origin-only (`false`) unless the object config opts into a + * bearer/callback identity check. An empty-string bearer is treated as no + * bearer (origin-only) rather than a usable credential. */ export function resolveMcpConfig(mcp: boolean | McpRouteOptions | undefined): ResolvedMcpConfig | undefined { if (!mcp) return undefined - if (mcp === true) { - const token = process.env.DEVFRAME_MCP_AUTH_TOKEN - if (!token) - throw diagnostics.DF0077() - return { authorization: token } - } - const { authorization } = mcp - if (authorization === undefined || (typeof authorization === 'string' && authorization.length === 0)) - throw diagnostics.DF0077() + if (mcp === true) + return { authorization: false } + const authorization = typeof mcp.authorization === 'string' && mcp.authorization.length === 0 + ? false + : mcp.authorization ?? false return { ...(mcp.path !== undefined ? { path: mcp.path } : {}), ...(mcp.allowedOrigins !== undefined ? { allowedOrigins: mcp.allowedOrigins } : {}), @@ -103,9 +95,9 @@ export function resolveMcpConfig(mcp: boolean | McpRouteOptions | undefined): Re /** * Resolve the `mcp` entry a `__connection.json` should advertise for a dev - * server started with the given `mcp` option (falling back to `def.cli?.mcp`, - * exactly like `createDevServer`), or `undefined` when the route is - * disabled. + * server started with the given `mcp` option (falling back to `def.mcp`, then + * the deprecated `def.cli?.mcp`, exactly like `createDevServer`), or + * `undefined` when the route is disabled. * * Hosted bridges that hand-roll their connection meta pass the side-car * `port`: the advertised path becomes absolute (the side-car mounts at `/`) @@ -118,7 +110,7 @@ export function resolveMcpConnectionMeta( mcp: boolean | McpRouteOptions | undefined, port?: number, ): ConnectionMeta['mcp'] { - const config = resolveMcpConfig(mcp ?? def.cli?.mcp) + const config = resolveMcpConfig(mcp ?? def.mcp ?? def.cli?.mcp) if (!config) return undefined const route = withoutLeadingSlash(config.path ?? DEVFRAME_MCP_ROUTE) diff --git a/packages/devframe/src/adapters/cac.ts b/packages/devframe/src/adapters/cac.ts index 2a0290722..9d3e4d3fb 100644 --- a/packages/devframe/src/adapters/cac.ts +++ b/packages/devframe/src/adapters/cac.ts @@ -70,8 +70,9 @@ export function createCac(d: DevframeDefinition, options: CreateCacOptions = {}) .option('--no-auth', 'Disable the interactive authentication gate') // Only `--mcp` is declared: CAC's `--no-*` auto-negation would inject a // `true` default, silently enabling MCP. Declaring just `--mcp` yields the - // opt-in tri-state — absent → `undefined` (falls through to `cli.mcp`), - // `--mcp` → `true`, `--no-mcp` → `false` (handled by CAC's `--no-` prefix). + // opt-in tri-state — absent → `undefined` (falls through to the + // definition's `mcp`), `--mcp` → `true`, `--no-mcp` → `false` (handled by + // CAC's `--no-` prefix). .option('--mcp', 'Expose an MCP server over HTTP at /__mcp (use --no-mcp to disable)') // Register typed flags from the definition ahead of `cli.configure` @@ -95,7 +96,7 @@ export function createCac(d: DevframeDefinition, options: CreateCacOptions = {}) const port = (flags.port as number | undefined) ?? await resolveDevServerPort(d, { host, defaultPort }) // `--mcp` / `--no-mcp` map to a boolean override; when neither is // passed CAC leaves `mcp` undefined so `createDevServer` falls through - // to `def.cli?.mcp`. + // to the definition's `mcp` (top-level, then the deprecated `cli.mcp`). const mcp = flags.mcp as boolean | undefined await createDevServer(d, { host, diff --git a/packages/devframe/src/adapters/dev.ts b/packages/devframe/src/adapters/dev.ts index 9b495dddb..c93f62217 100644 --- a/packages/devframe/src/adapters/dev.ts +++ b/packages/devframe/src/adapters/dev.ts @@ -92,9 +92,9 @@ export interface CreateDevServerOptions { auth?: boolean | DevframeAuthHandler /** * Expose a route-based MCP server on the dev server (Streamable-HTTP). - * Overrides `def.cli?.mcp`; `undefined` falls through to it. `false` - * disables the route regardless of the definition default. See - * {@link McpRouteOptions}. + * Overrides the definition's `mcp` (top-level, then the deprecated + * `cli.mcp`); `undefined` falls through to it. `false` disables the route + * regardless of the definition default. See {@link McpRouteOptions}. */ mcp?: boolean | McpRouteOptions /** diff --git a/packages/devframe/src/adapters/initiate.ts b/packages/devframe/src/adapters/initiate.ts index e0fa398f9..705ddb813 100644 --- a/packages/devframe/src/adapters/initiate.ts +++ b/packages/devframe/src/adapters/initiate.ts @@ -89,8 +89,9 @@ export interface InitDevframeOptions { auth?: boolean | DevframeAuthHandler /** * Expose a route-based MCP server (Streamable-HTTP) at `__mcp` and - * advertise it in `__connection.json`. Overrides `def.cli?.mcp`; - * `undefined` falls through to it. See {@link McpRouteOptions}. + * advertise it in `__connection.json`. Overrides the definition's `mcp` + * (top-level, then the deprecated `cli.mcp`); `undefined` falls through to + * it. See {@link McpRouteOptions}. */ mcp?: boolean | McpRouteOptions /** @@ -305,11 +306,10 @@ export function initDevframe( // Route-based MCP server (opt-in). Mounted before the SPA static // catch-all so the exact `__mcp` route wins, and advertised in // `__connection.json`. The MCP SDK stays an optional peer — its code is - // only pulled in (dynamically) when the route is enabled. Resolving the - // config validates the authorization policy up front: a `mcp: true` - // shorthand with no `DEVFRAME_MCP_AUTH_TOKEN`, or an object with no - // `authorization`, throws `DF0077` here rather than mounting the route. - const mcpConfig = resolveMcpConfig(options.mcp ?? def.cli?.mcp) + // only pulled in (dynamically) when the route is enabled. The resolved + // config trusts same-machine callers by default (origin-only); an object + // config can opt into a bearer/callback identity check. + const mcpConfig = resolveMcpConfig(options.mcp ?? def.mcp ?? def.cli?.mcp) let mcpMeta: ConnectionMeta['mcp'] let mcpDispose: (() => Promise) | undefined if (mcpConfig) { diff --git a/packages/devframe/src/adapters/mcp/__tests__/mcp-http.test.ts b/packages/devframe/src/adapters/mcp/__tests__/mcp-http.test.ts index 21786954a..21b1d478e 100644 --- a/packages/devframe/src/adapters/mcp/__tests__/mcp-http.test.ts +++ b/packages/devframe/src/adapters/mcp/__tests__/mcp-http.test.ts @@ -1,5 +1,5 @@ import type { StartedServer } from '../../../node/instance-shell' -import type { DevframeDefinition, McpAuthorization } from '../../../types/devframe' +import type { DevframeDefinition, McpRouteOptions } from '../../../types/devframe' import { Client, StreamableHTTPClientTransport } from '@modelcontextprotocol/client' import { afterEach, describe, expect, it } from 'vitest' import { createDevServer } from '../../dev' @@ -34,7 +34,7 @@ describe('mcp adapter (streamable http route)', () => { server = undefined }) - async function boot(authorization: McpAuthorization = TOKEN, def = defineTestDef()): Promise { + async function boot(mcp: boolean | McpRouteOptions = true, def = defineTestDef()): Promise { // `port: 0` lets the OS assign a fresh ephemeral port per test. Without // it every test binds the same default port, and since they all share // one process, Node's global `fetch()` (undici) pools keep-alive @@ -43,7 +43,7 @@ describe('mcp adapter (streamable http route)', () => { // test's (torn-down) server, failing instantly with a socket error, or // making that earlier server's `close()` hang until undici's // keep-alive timeout releases it. - server = await createDevServer(def, { host: '127.0.0.1', port: 0, mcp: { authorization } }) + server = await createDevServer(def, { host: '127.0.0.1', port: 0, mcp }) return server } @@ -62,24 +62,18 @@ describe('mcp adapter (streamable http route)', () => { expect(meta.mcp).toBeUndefined() }) - it('fails startup when the mcp: true shorthand has no environment token', async () => { - await expect( - createDevServer(defineTestDef(), { host: '127.0.0.1', port: 0, mcp: true }), - ).rejects.toThrow(/DF0077|authorization policy/) - }) - // A native MCP client must send a (loopback) Origin so the route's gate — - // which rejects Origin-less requests — accepts it, and the configured - // bearer so the identity gate accepts it. - function authedTransport(started: StartedServer): StreamableHTTPClientTransport { + // which rejects Origin-less requests — accepts it. `mcp: true` trusts + // same-machine callers, so no bearer is needed. + function originTransport(started: StartedServer, headers: Record = {}): StreamableHTTPClientTransport { return new StreamableHTTPClientTransport(new URL(`${started.origin}/__mcp`), { - requestInit: { headers: { origin: started.origin, authorization: `Bearer ${TOKEN}` } }, + requestInit: { headers: { origin: started.origin, ...headers } }, }) } - it('serves the modern era statelessly and lists agent tools with a valid bearer', async () => { + it('trusts same-machine callers by default (origin only) and lists agent tools', async () => { const started = await boot() - const transport = authedTransport(started) + const transport = originTransport(started) // Negotiate the 2026-07-28 era via `server/discover`. const client = new Client( { name: 'test-client', version: '0.0.0' }, @@ -104,14 +98,14 @@ describe('mcp adapter (streamable http route)', () => { } }) - it('answers a bare GET with 405 (no session lifecycle) once both gates pass', async () => { + it('answers a bare GET with 405 (no session lifecycle)', async () => { const started = await boot() // Stateless serving has no session stream to open — the SDK answers a // GET (a 2025 session operation) with `405 Method Not Allowed` rather // than falling through to the SPA static catch-all. const res = await fetch(`${started.origin}/__mcp`, { method: 'GET', - headers: { accept: 'text/event-stream', origin: started.origin, authorization: `Bearer ${TOKEN}` }, + headers: { accept: 'text/event-stream', origin: started.origin }, }) await res.body?.cancel() expect(res.status).toBe(405) @@ -135,77 +129,95 @@ describe('mcp adapter (streamable http route)', () => { }) } - it('rejects an Origin-less request with 403 (origin gate runs first)', async () => { + it('rejects an Origin-less request with 403', async () => { const started = await boot() // Unlike the WS transport, the MCP route does not allow Origin-less // requests — a route-based endpoint would otherwise be reachable by any - // local process. The origin gate runs before the identity gate, so even - // a valid bearer is rejected 403 without an Origin. - const res = await initRequest(started, { authorization: `Bearer ${TOKEN}` }) + // local process. + const res = await initRequest(started, {}) await res.body?.cancel() expect(res.status).toBe(403) }) - it('rejects a disallowed cross-origin request with 403 even with a valid bearer', async () => { + it('rejects a disallowed cross-origin request with 403', async () => { const started = await boot() - const res = await initRequest(started, { origin: 'http://evil.example.com', authorization: `Bearer ${TOKEN}` }) + const res = await initRequest(started, { origin: 'http://evil.example.com' }) await res.body?.cancel() expect(res.status).toBe(403) }) - it('rejects an allowed-origin request with no bearer as 401 + WWW-Authenticate', async () => { - const started = await boot() - const res = await initRequest(started, { origin: started.origin }) - await res.body?.cancel() - expect(res.status).toBe(401) - expect(res.headers.get('www-authenticate')).toBe('Bearer') - }) + describe('opt-in bearer (authorization: token)', () => { + it('accepts a request carrying the correct bearer', async () => { + const started = await boot({ authorization: TOKEN }) + const client = new Client({ name: 'test-client', version: '0.0.0' }, { versionNegotiation: { mode: 'auto' } }) + const transport = originTransport(started, { authorization: `Bearer ${TOKEN}` }) + try { + await client.connect(transport) + const tools = await client.listTools() + expect(tools.tools.map(t => t.name)).toContain('greet') + } + finally { + await client.close() + } + }) - it('rejects an allowed-origin request with the wrong bearer as 401', async () => { - const started = await boot() - const res = await initRequest(started, { origin: started.origin, authorization: 'Bearer not-the-token' }) - await res.body?.cancel() - expect(res.status).toBe(401) - }) + it('rejects an allowed-origin request with no bearer as 401 + WWW-Authenticate', async () => { + const started = await boot({ authorization: TOKEN }) + const res = await initRequest(started, { origin: started.origin }) + await res.body?.cancel() + expect(res.status).toBe(401) + expect(res.headers.get('www-authenticate')).toBe('Bearer') + }) - it('rejects a malformed / multi-credential Authorization header as 401', async () => { - const started = await boot() - const res = await initRequest(started, { origin: started.origin, authorization: `Bearer ${TOKEN}, Bearer other` }) - await res.body?.cancel() - expect(res.status).toBe(401) - }) + it('rejects the wrong bearer as 401', async () => { + const started = await boot({ authorization: TOKEN }) + const res = await initRequest(started, { origin: started.origin, authorization: 'Bearer not-the-token' }) + await res.body?.cancel() + expect(res.status).toBe(401) + }) - it('delegates identity to a callback policy: allow', async () => { - const started = await boot(req => req.headers.get('x-secret') === 'open-sesame') - const res = await initRequest(started, { 'origin': started.origin, 'x-secret': 'open-sesame' }) - expect(res.status).toBe(200) - await res.body?.cancel() - }) + it('rejects a malformed / multi-credential Authorization header as 401', async () => { + const started = await boot({ authorization: TOKEN }) + const res = await initRequest(started, { origin: started.origin, authorization: `Bearer ${TOKEN}, Bearer other` }) + await res.body?.cancel() + expect(res.status).toBe(401) + }) - it('delegates identity to a callback policy: deny → 401', async () => { - const started = await boot(req => req.headers.get('x-secret') === 'open-sesame') - const res = await initRequest(started, { 'origin': started.origin, 'x-secret': 'wrong' }) - await res.body?.cancel() - expect(res.status).toBe(401) + it('runs the origin gate before the bearer (disallowed origin still 403)', async () => { + const started = await boot({ authorization: TOKEN }) + const res = await initRequest(started, { origin: 'http://evil.example.com', authorization: `Bearer ${TOKEN}` }) + await res.body?.cancel() + expect(res.status).toBe(403) + }) }) - it('a callback cannot relax the origin gate (disallowed origin still 403)', async () => { - const started = await boot(() => true) - const res = await initRequest(started, { origin: 'http://evil.example.com' }) - await res.body?.cancel() - expect(res.status).toBe(403) + describe('opt-in callback (authorization: fn)', () => { + it('allows when the callback returns true', async () => { + const started = await boot({ authorization: req => req.headers.get('x-secret') === 'open-sesame' }) + const res = await initRequest(started, { 'origin': started.origin, 'x-secret': 'open-sesame' }) + expect(res.status).toBe(200) + await res.body?.cancel() + }) + + it('denies with 401 when the callback returns false', async () => { + const started = await boot({ authorization: req => req.headers.get('x-secret') === 'open-sesame' }) + const res = await initRequest(started, { 'origin': started.origin, 'x-secret': 'wrong' }) + await res.body?.cancel() + expect(res.status).toBe(401) + }) + + it('cannot relax the origin gate (disallowed origin still 403)', async () => { + const started = await boot({ authorization: () => true }) + const res = await initRequest(started, { origin: 'http://evil.example.com' }) + await res.body?.cancel() + expect(res.status).toBe(403) + }) }) - it('explicit authorization: false is an origin-only opt-out', async () => { - const started = await boot(false) - // Allowed origin, no bearer at all — accepted, since identity is opted out. + it('explicit authorization: false behaves like the origin-only default', async () => { + const started = await boot({ authorization: false }) const res = await initRequest(started, { origin: started.origin }) expect(res.status).toBe(200) await res.body?.cancel() - - // …but the origin gate still stands. - const cross = await initRequest(started, { origin: 'http://evil.example.com' }) - await cross.body?.cancel() - expect(cross.status).toBe(403) }) }) diff --git a/packages/devframe/src/adapters/mcp/fetch.ts b/packages/devframe/src/adapters/mcp/fetch.ts index 0c2a58cfa..d65d85aec 100644 --- a/packages/devframe/src/adapters/mcp/fetch.ts +++ b/packages/devframe/src/adapters/mcp/fetch.ts @@ -12,13 +12,14 @@ export interface CreateMcpFetchHandlerOptions { /** Expose shared-state keys as MCP resources — see `buildMcpServerFromContext`. */ exposeSharedState: boolean | ((key: string) => boolean) /** - * The endpoint's identity policy, checked **after** the origin gate: a - * bearer token string (matched in constant time against + * Optional identity check, layered on top of the origin gate and checked + * **after** it: a bearer token string (matched in constant time against * `Authorization: Bearer `), a `(request) => boolean` callback, or - * `false` for an origin-only opt-out. A callback governs identity only and - * cannot relax the origin gate. See {@link McpAuthorization}. + * `false` (the default) for origin-only, trusting same-machine callers. A + * callback governs identity only and cannot relax the origin gate. See + * {@link McpAuthorization}. */ - authorization: McpAuthorization + authorization?: McpAuthorization /** * Origin allow-list beyond the loopback default. `false` disables the * origin gate entirely. Default: loopback-only. @@ -86,13 +87,14 @@ export interface McpFetchHandler { * SDK's default stateless legacy path. `list_changed` events reach modern * `subscriptions/listen` streams through the handler's `notify` bus. * - * Two independent gates guard every request. First the origin gate: - * loopback-default DNS-rebinding protection that — unlike the WS upgrade's - * `isAllowedOrigin` — also rejects `Origin`-less requests, so a route-based - * endpoint isn't reachable by an arbitrary local process (a disallowed origin - * gets `403`). Then the identity gate ({@link CreateMcpFetchHandlerOptions.authorization}): - * a bearer/callback check that proves *who* is calling, since a native client - * can spoof any `Origin` (a missing/invalid credential gets `401` with a + * The origin gate guards every request: loopback-default DNS-rebinding + * protection that — unlike the WS upgrade's `isAllowedOrigin` — also rejects + * `Origin`-less requests, so a route-based endpoint isn't reachable by a + * browser or a remote host (a disallowed origin gets `403`). It trusts + * same-machine callers by default. When that isn't your trust boundary, add + * an optional identity gate ({@link CreateMcpFetchHandlerOptions.authorization}), + * checked after the origin gate: a bearer/callback check that proves *who* is + * calling (a missing/invalid credential gets `401` with a * `WWW-Authenticate: Bearer` challenge). */ export function createMcpFetchHandler( @@ -100,7 +102,9 @@ export function createMcpFetchHandler( options: CreateMcpFetchHandlerOptions, ): McpFetchHandler { const allowedOrigins = options.allowedOrigins - const authorization = options.authorization + // Origin-only by default: trust same-machine callers unless a bearer/callback + // identity check is configured. + const authorization = options.authorization ?? false const handler = createMcpHandler(() => buildMcpServerFromContext(ctx, { serverName: options.serverName, diff --git a/packages/devframe/src/node/diagnostics.ts b/packages/devframe/src/node/diagnostics.ts index 3ba20412d..1991132e5 100644 --- a/packages/devframe/src/node/diagnostics.ts +++ b/packages/devframe/src/node/diagnostics.ts @@ -209,9 +209,5 @@ export const diagnostics = defineDiagnostics({ `\`attach\` / \`handleUpgrade\` drive a raw \`node:http\` upgrade into crossws's Node adapter, which refuses to run on ${p.runtime}.`, fix: 'On Bun/Deno, serve the advertised `__ws` route from `Bun.serve` / `Deno.serve` with `attachBunWsTransport` / `attachDenoWsTransport` (see the hub-deno-minimal example), or connect over the SSE endpoint instead.', }, - DF0077: { - why: 'The route-based MCP server needs an authorization policy, but none is configured — refusing to mount an unauthenticated agent endpoint.', - fix: 'Set the `DEVFRAME_MCP_AUTH_TOKEN` environment variable (the bearer `mcp: true` requires), or pass an explicit `authorization` on the MCP options — a non-empty bearer token string, a `(request) => boolean` callback, or `false` for an origin-only local opt-out.', - }, }, }) diff --git a/packages/devframe/src/types/devframe.ts b/packages/devframe/src/types/devframe.ts index 3a4990d7e..c2350d50f 100644 --- a/packages/devframe/src/types/devframe.ts +++ b/packages/devframe/src/types/devframe.ts @@ -85,18 +85,19 @@ export interface DevframeSseOptions { } /** - * The identity policy the route-based MCP endpoint enforces once a request - * clears the origin gate. `Origin` proves nothing about *who* is calling — any - * native client can send any `Origin` — so this is the endpoint's actual - * authentication, kept independent of the origin/DNS-rebinding check: + * An **optional** identity check layered on top of the origin gate. The + * route-based MCP endpoint trusts same-machine callers by default (the + * loopback origin gate is enough), so this is opt-in hardening for the cases + * where a same-machine process is not a trust boundary — a LAN/tunnel origin, + * a shared/CI box, or a destructive tool surface: * * - a non-empty **bearer token string** — the request must carry - * `Authorization: Bearer `, compared in constant time; + * `Authorization: Bearer `, compared in constant time. Back it with + * an environment variable rather than a literal; * - a **callback** `(request) => boolean | Promise` — a custom * identity check (validate a signed header, an mTLS-derived claim, …). It * only governs identity and cannot relax the origin gate; - * - `false` — an explicit **origin-only opt-out** for a loopback-bound local - * tool that owns its trust boundary another way. Use it sparingly. + * - `false` — the default: **origin-only**, trusting same-machine callers. */ export type McpAuthorization = | string @@ -105,7 +106,7 @@ export type McpAuthorization /** * Configuration for the route-based MCP server mounted alongside the dev - * server (opt-in via {@link DevframeCliOptions.mcp}). The endpoint speaks + * server (opt-in via {@link DevframeDefinition.mcp}). The endpoint speaks * the MCP Streamable-HTTP transport over the same origin as the SPA, * exposing the definition's `ctx.agent` tools + shared-state resources to * external MCP clients connected to the *running* server. @@ -117,17 +118,15 @@ export interface McpRouteOptions { */ path?: string /** - * The endpoint's identity policy — **required** for an object config, since - * the route grants access to privileged agent tools. See - * {@link McpAuthorization} for the accepted forms (bearer token, callback, - * or explicit `false` for an origin-only local opt-out). The `mcp: true` - * shorthand reads its bearer from the `DEVFRAME_MCP_AUTH_TOKEN` environment - * variable instead of this field. - * - * This is checked **after** the origin gate below — identity and - * origin/DNS-rebinding protection are separate defenses. + * Optional identity check, layered on top of the origin gate and checked + * **after** it. Defaults to origin-only (`false`) — the route trusts + * same-machine callers, since the loopback origin gate already keeps + * arbitrary remote/browser callers out. Set a bearer token or a callback to + * harden the route when a same-machine process is not your trust boundary + * (LAN/tunnel origin, shared/CI host, destructive tools). See + * {@link McpAuthorization}. */ - authorization: McpAuthorization + authorization?: McpAuthorization /** * Extra `Origin` header values to accept beyond the loopback default * (`localhost`/`127.0.0.1`/`::1` and any `Origin`-less native client). @@ -138,8 +137,9 @@ export interface McpRouteOptions { * This is the endpoint's DNS-rebinding protection — the shared * `isAllowedOrigin` gate the WS upgrade already uses, applied as external * middleware (the approach the MCP SDK now recommends over its own - * deprecated `allowedHosts`/`allowedOrigins` transport flags). It hardens - * the request; {@link McpRouteOptions.authorization} proves identity. + * deprecated `allowedHosts`/`allowedOrigins` transport flags). When you + * widen it past loopback, layer on {@link McpRouteOptions.authorization} to + * prove identity too. */ allowedOrigins?: readonly string[] | false } @@ -184,20 +184,11 @@ export interface DevframeCliOptions { */ auth?: boolean | DevframeAuthHandler /** - * Expose a route-based MCP server alongside the dev server, speaking the - * MCP Streamable-HTTP transport at `/__mcp` (relative to the base path). - * It surfaces the same `ctx.agent` tools + shared-state resources as the - * stdio `mcp` command, but against the live, running server. - * - * - `false` / omitted (default) — no MCP route is mounted. - * - `true` — mount at the default `__mcp` route, requiring the bearer token - * read from the `DEVFRAME_MCP_AUTH_TOKEN` environment variable (startup - * fails with `DF0077` when it is missing/empty, so the route is never - * mounted unauthenticated). - * - {@link McpRouteOptions} — customise the route path / allowed origins, - * with an explicit {@link McpAuthorization} policy (required). + * Expose a route-based MCP server alongside the dev server. * - * The `--mcp` / `--no-mcp` CLI flags override this per run. + * @deprecated Moved to the top-level {@link DevframeDefinition.mcp}. Set + * `mcp` on the definition instead. This field is still read as a fallback + * when the top-level `mcp` is unset, so existing definitions keep working. */ mcp?: boolean | McpRouteOptions /** @@ -425,6 +416,22 @@ export interface DevframeDefinition { * {@link DevframeCliOptions.distDir} is read as a fallback. */ clientAssets?: StaticAssetsSource + /** + * Expose a route-based MCP server alongside the running dev server, speaking + * the MCP Streamable-HTTP transport at `/__mcp` (relative to the base path). + * It surfaces the same `ctx.agent` tools + shared-state resources as the + * stdio `mcp` command, but against the live server. + * + * - `false` / omitted (default) — no MCP route is mounted. + * - `true` — mount at the default `__mcp` route with the loopback origin + * gate (trusting same-machine callers). + * - {@link McpRouteOptions} — customise the route path, origin allow-list, + * and opt into an {@link McpAuthorization} identity check. + * + * The `--mcp` / `--no-mcp` CLI flags override this per run. When unset, the + * deprecated {@link DevframeCliOptions.mcp} is read as a fallback. + */ + mcp?: boolean | McpRouteOptions /** RPC-level configuration for this devframe (see {@link DevframeRpcOptions}). */ rpc?: DevframeRpcOptions /** Server-side setup — the primary entrypoint. Runs in every runtime. */ diff --git a/packages/hub/src/node/__tests__/initiate.test.ts b/packages/hub/src/node/__tests__/initiate.test.ts index d4474e012..c841ac5b1 100644 --- a/packages/hub/src/node/__tests__/initiate.test.ts +++ b/packages/hub/src/node/__tests__/initiate.test.ts @@ -6,7 +6,7 @@ import { join } from 'node:path' import { createRpcClient } from 'devframe/rpc/client' import { createWsRpcChannel } from 'devframe/rpc/transports/ws-client' import { getPort } from 'get-port-please' -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import { DOCK_RENDERERS_STATE_KEY } from '../../constants' import { DEVFRAMES_HUB_BASE, initHub } from '../initiate' @@ -291,9 +291,7 @@ describe('initHub', () => { it('aggregate MCP: one endpoint lists tools from every mounted frame', async () => { const wsPort = await getPort({ port: 18230, host: '127.0.0.1' }) - // An origin-only opt-out keeps this loopback-bound fixture free of bearer - // plumbing; the identity gate itself is covered in mcp-http.test.ts. - const hub = initHub({ base: DEVFRAMES_HUB_BASE, auth: false, host: '127.0.0.1', ws: { port: wsPort }, mcp: { authorization: false }, devframes: [makeFrame('alpha'), makeFrame('beta')] }) + const hub = initHub({ base: DEVFRAMES_HUB_BASE, auth: false, host: '127.0.0.1', ws: { port: wsPort }, mcp: true, devframes: [makeFrame('alpha'), makeFrame('beta')] }) try { await hub.ready @@ -323,11 +321,32 @@ describe('initHub', () => { } }) - it('aggregate MCP: mcp: true without DEVFRAME_MCP_AUTH_TOKEN fails startup (DF0077)', async () => { + it('warns (DF8005) when a mounted devframe asks for MCP but the hub MCP is off', async () => { const wsPort = await getPort({ port: 18235, host: '127.0.0.1' }) - const hub = initHub({ base: DEVFRAMES_HUB_BASE, auth: false, host: '127.0.0.1', ws: { port: wsPort }, mcp: true, devframes: [makeFrame('alpha')] }) - await expect(hub.ready).rejects.toThrow(/DF0077|authorization policy/) - await hub.close() + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + // The hub has no `mcp`, but `beta` declares `mcp: true` — the hub's single + // aggregate route governs MCP, so beta's request is a no-op and warns. + const hub = initHub({ + base: DEVFRAMES_HUB_BASE, + auth: false, + host: '127.0.0.1', + ws: { port: wsPort }, + devframes: [makeFrame('alpha'), { ...makeFrame('beta'), mcp: true }], + }) + + try { + await hub.ready + expect(hub.connectionMeta().mcp).toBeUndefined() + const warned = warn.mock.calls.map((args: unknown[]) => String(args[0])).join('\n') + expect(warned).toMatch(/DF8005/) + expect(warned).toContain('beta') + // `alpha` didn't ask for MCP, so it isn't named. + expect(warned).not.toContain('"alpha"') + } + finally { + warn.mockRestore() + await hub.close() + } }) it('single hub Auth: one gate covers every frame on the shared socket', async () => { diff --git a/packages/hub/src/node/diagnostics.ts b/packages/hub/src/node/diagnostics.ts index 8cb540235..7c67ca6c4 100644 --- a/packages/hub/src/node/diagnostics.ts +++ b/packages/hub/src/node/diagnostics.ts @@ -28,6 +28,10 @@ export const diagnostics = defineDiagnostics({ why: (p: { id: string }) => `Devframe id "${p.id}" is not a mountable URL segment — the hub mounts each frame at \`/\`.`, fix: 'Ids become route segments, so they may only contain letters, digits, `_`, `-`, and `.` — `:` and `*` are route-pattern markers to the underlying router, and `/` would escape the segment. Set a route-safe `id` on the definition (e.g. `my_plugin` instead of `my:plugin`).', }, + DF8005: { + why: (p: { id: string }) => `Devframe "${p.id}" requests an MCP route, but the hub's aggregate MCP is off — its tools are not exposed over MCP.`, + fix: 'A hub exposes one aggregate MCP endpoint over every mounted devframe, so per-devframe `mcp` settings are ignored. Enable the hub\'s own MCP (pass `mcp` to `initHub`) to surface this devframe\'s tools, or drop `mcp` from the devframe to silence this warning.', + }, DF8100: { why: (p: { id: string }) => `Dock with id "${p.id}" is already registered`, fix: 'Use the `force` parameter to overwrite an existing registration.', diff --git a/packages/hub/src/node/initiate.ts b/packages/hub/src/node/initiate.ts index ac796faf1..c7232a40b 100644 --- a/packages/hub/src/node/initiate.ts +++ b/packages/hub/src/node/initiate.ts @@ -255,10 +255,12 @@ export interface InitHubOptions { /** * Expose the **aggregate** MCP endpoint at `__mcp` — one * Streamable-HTTP server over the shared context's whole tool registry - * (ids are already namespaced per plugin). Disabled by default. When - * enabled it requires an authorization policy: `true` reads the bearer from - * `DEVFRAME_MCP_AUTH_TOKEN` (startup fails with `DF0077` when it is - * missing/empty), an object carries an explicit {@link McpRouteOptions.authorization}. + * (ids are already namespaced per plugin). Disabled by default; `true` + * mounts it with the loopback origin gate (trusting same-machine callers), + * an object opts into an {@link McpRouteOptions.authorization} identity + * check. A mounted devframe's own `mcp` setting is ignored — the hub's + * aggregate route covers them all (`DF8005` warns when one asks for MCP + * while this is off). */ mcp?: boolean | McpRouteOptions /** @@ -527,6 +529,12 @@ export function initHub(options: InitHubOptions): HubInstance { // segment entirely. if (!/^[\w.-]+$/.test(def.id)) throw diagnostics.DF8004({ id: def.id }) + // A hub exposes one aggregate MCP route over every mounted devframe, + // so a devframe's own `mcp` request is only meaningful when the hub's + // own MCP is enabled. Warn when it isn't, rather than silently + // dropping the devframe's intended agent surface. + if (!options.mcp && (def.mcp ?? def.cli?.mcp)) + diagnostics.DF8005({ id: def.id }) const frameBase = withTrailingSlash(joinURL(base, def.id)) const run = await prepareDevframe(ctx, def, { base: frameBase, ...(dock ? { dock } : {}) }) if (run) @@ -568,10 +576,9 @@ export function initHub(options: InitHubOptions): HubInstance { // Aggregate MCP — one Streamable-HTTP endpoint over the shared // context's whole registry (tool ids are namespaced per plugin, and the - // wire-name collision policy is `createMcpFetchHandler`'s own). Resolving - // the config validates the authorization policy up front: `mcp: true` - // with no `DEVFRAME_MCP_AUTH_TOKEN`, or an object with no - // `authorization`, throws `DF0077` here rather than mounting the route. + // wire-name collision policy is `createMcpFetchHandler`'s own). The + // resolved config trusts same-machine callers by default (origin-only); + // an object config opts into a bearer/callback identity check. const mcpConfig = resolveMcpConfig(options.mcp) if (!mcpConfig) return { context: ctx } diff --git a/packages/next/src/handler.ts b/packages/next/src/handler.ts index 462f76c96..b6c8d3430 100644 --- a/packages/next/src/handler.ts +++ b/packages/next/src/handler.ts @@ -35,8 +35,8 @@ export interface CreateDevframeNextHandlerOptions { * Expose the route-based MCP server (Streamable-HTTP) at `__mcp` — * on the Next app's own origin, through the same catch-all route as the * SPA — and advertise it in the handler's `__connection.json`. Overrides - * `def.cli?.mcp`, `undefined` falls through to it, `false` disables the - * route regardless. + * the definition's `mcp` (top-level, then the deprecated `cli.mcp`); + * `undefined` falls through to it, `false` disables the route regardless. */ mcp?: InitDevframeOptions['mcp'] /** diff --git a/packages/next/src/host.ts b/packages/next/src/host.ts index 0ac57ce3a..d97b4a8a7 100644 --- a/packages/next/src/host.ts +++ b/packages/next/src/host.ts @@ -27,14 +27,14 @@ export interface CreateDevframeNextHostOptions { export interface DevframeNextHostMcpOptions { /** - * The endpoint's identity policy — **required**, since the route grants - * access to privileged agent tools. A bearer token string (matched in - * constant time), a `(request) => boolean` callback, or `false` for an - * origin-only local opt-out. Checked after the origin gate; see - * {@link McpAuthorization}. Back a bearer with an environment variable - * (e.g. `process.env.DEVFRAME_MCP_AUTH_TOKEN`) — never a literal. + * Optional identity check layered on the origin gate. Defaults to + * origin-only (`false`), trusting same-machine callers. A bearer token + * string (matched in constant time) or a `(request) => boolean` callback + * hardens the route when a same-machine process is not your trust boundary; + * see {@link McpAuthorization}. Back a bearer with an environment variable — + * never a literal. */ - authorization: McpAuthorization + authorization?: McpAuthorization /** Name reported in the MCP handshake. Default: `'devframe (next)'`. */ serverName?: string /** Version reported in the MCP handshake. Default: `'0.0.0'`. */ @@ -89,7 +89,7 @@ export interface DevframeNextHost { mountMcp: ( ctx: DevframeNodeContext, path: string, - options: DevframeNextHostMcpOptions, + options?: DevframeNextHostMcpOptions, ) => Promise<{ dispose: () => Promise }> } @@ -173,7 +173,7 @@ export function createDevframeNextHost( setConnectionMeta(meta) { connectionMeta = meta }, - async mountMcp(ctx, path, mcpOptions) { + async mountMcp(ctx, path, mcpOptions = {}) { const { createMcpFetchHandler } = await importRuntimeModule('devframe/adapters/mcp') const handler = createMcpFetchHandler(ctx, { serverName: mcpOptions.serverName ?? 'devframe (next)', diff --git a/packages/next/src/hub.ts b/packages/next/src/hub.ts index 08b2980fc..dbb14e3da 100644 --- a/packages/next/src/hub.ts +++ b/packages/next/src/hub.ts @@ -50,11 +50,9 @@ export interface NextDevframeHubOptions { /** The hub's single auth gate. Gates by default; `false` opts out. */ auth?: InitHubOptions['auth'] /** - * Expose the aggregate MCP endpoint at `__mcp`. Disabled by default — - * the endpoint exposes privileged agent tools, so opt in with an explicit - * authorization policy: `mcp: true` reads the bearer from - * `DEVFRAME_MCP_AUTH_TOKEN`, or pass an object with an explicit - * `authorization`. + * Expose the aggregate MCP endpoint at `__mcp`. Disabled by default; + * `true` mounts it with the loopback origin gate (trusting same-machine + * callers), or pass an object to opt into an `authorization` identity check. */ mcp?: InitHubOptions['mcp'] /** Public origin the Next app is reachable at. Default: derived from `PORT`. */ @@ -73,8 +71,8 @@ export interface NextDevframeHubOptions { * Build a devframes-hub for a Next.js App Router app: one `initHub()` call * mounting every devframe under `/` behind one web-standard * `handler`, with the RPC socket on a side-car (Next routes can't accept WS - * upgrades) and the aggregate MCP route opt-in (pass `mcp` with an explicit - * authorization policy to enable it). The UI defaults to + * upgrades) and the aggregate MCP route opt-in (pass `mcp` to enable it). The + * UI defaults to * `@devframes/hub-ui`'s `createUi()`, loaded lazily via a bundler-ignored * dynamic `import()` so its asset lookups resolve at request time; pass `ui` * to swap it or `ui: false` for a headless hub. diff --git a/packages/next/test/handler.test.ts b/packages/next/test/handler.test.ts index 5f8461373..c3c3198de 100644 --- a/packages/next/test/handler.test.ts +++ b/packages/next/test/handler.test.ts @@ -83,10 +83,7 @@ describe('createDevframeNextHandler', () => { const dist = mkdtempSync(join(tmpdir(), 'df-next-mcp-')) writeFileSync(join(dist, 'index.html'), 'ok') - // Origin-only opt-out keeps this loopback-bound handler test free of - // bearer plumbing; the identity gate is covered in devframe's - // mcp-http.test.ts. - handler = createDevframeNextHandler(makeDef(dist), { host: '127.0.0.1', mcp: { authorization: false } }) + handler = createDevframeNextHandler(makeDef(dist), { host: '127.0.0.1', mcp: true }) await handler.ready const meta = await handler.fetch(new Request('http://localhost:3000/__test-next/__connection.json')) diff --git a/packages/vite/src/single.ts b/packages/vite/src/single.ts index 8932dde5b..9774d44cf 100644 --- a/packages/vite/src/single.ts +++ b/packages/vite/src/single.ts @@ -114,8 +114,9 @@ export interface DevframeViteBridgeOptions { /** * Expose the bridge's route-based MCP server (Streamable-HTTP) at * `__mcp` — on the Vite app's own origin — and advertise it in the - * bridge's `__connection.json`. Overrides `def.cli?.mcp`, `undefined` - * falls through to it, `false` disables the route regardless. + * bridge's `__connection.json`. Overrides the definition's `mcp` (top-level, + * then the deprecated `cli.mcp`); `undefined` falls through to it, `false` + * disables the route regardless. */ mcp?: boolean | McpRouteOptions } diff --git a/packages/vite/test/single.test.ts b/packages/vite/test/single.test.ts index 184a834f3..e485a1a38 100644 --- a/packages/vite/test/single.test.ts +++ b/packages/vite/test/single.test.ts @@ -130,10 +130,7 @@ describe('devframeViteBridge (bridge mode mcp)', () => { bridge = devframeViteBridge(defineTestDef(), { port: wsPort, host, - // Origin-only opt-out: this loopback-bound test dials the MCP route - // directly with a loopback Origin; the identity gate is covered in - // devframe's own mcp-http.test.ts. - mcp: { authorization: false }, + mcp: true, // The bridge gates by default; opt out here so this test can dial // the WS side-car and MCP route directly. auth: false, diff --git a/playwright.config.ts b/playwright.config.ts index 4b3022309..74ad64281 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -1,7 +1,6 @@ import process from 'node:process' import { fileURLToPath } from 'node:url' import { defineConfig, devices } from '@playwright/test' -import { MCP_AUTH_TOKEN } from './tests/e2e/_support/mcp-auth' const fixtureCwd = fileURLToPath(new URL('./tests/e2e/fixtures', import.meta.url)) const serveStatic = fileURLToPath(new URL('./tests/e2e/_support/serve-static.mjs', import.meta.url)) @@ -84,9 +83,7 @@ export default defineConfig({ { command: 'pnpm exec next dev src/client -p 9878', cwd: 'examples/hub-next', - // The example's aggregate MCP route uses the env-backed `mcp: true` - // policy, so the server needs the same bearer the connect spec presents. - env: { PORT: '9878', DEVFRAME_INSTANCES_DIR: nextHubRegistry, DEVFRAME_MCP_AUTH_TOKEN: MCP_AUTH_TOKEN }, + env: { PORT: '9878', DEVFRAME_INSTANCES_DIR: nextHubRegistry }, url: 'http://localhost:9878/', timeout: 120_000, reuseExistingServer: !process.env.CI, diff --git a/tests/__snapshots__/tsnapi/@devframes/next/hub.snapshot.d.ts b/tests/__snapshots__/tsnapi/@devframes/next/hub.snapshot.d.ts index c8e679145..834560c33 100644 --- a/tests/__snapshots__/tsnapi/@devframes/next/hub.snapshot.d.ts +++ b/tests/__snapshots__/tsnapi/@devframes/next/hub.snapshot.d.ts @@ -11,12 +11,12 @@ export interface DevframeNextHost { host: DevframeHost; fetch: (_: Request) => Promise; setConnectionMeta: (_: ConnectionMeta) => void; - mountMcp: (_: DevframeNodeContext, _: string, _: DevframeNextHostMcpOptions) => Promise<{ + mountMcp: (_: DevframeNodeContext, _: string, _?: DevframeNextHostMcpOptions) => Promise<{ dispose: () => Promise; }>; } export interface DevframeNextHostMcpOptions { - authorization: McpAuthorization; + authorization?: McpAuthorization; serverName?: string; serverVersion?: string; exposeSharedState?: boolean | ((_: string) => boolean); diff --git a/tests/__snapshots__/tsnapi/devframe/adapters/mcp.snapshot.d.ts b/tests/__snapshots__/tsnapi/devframe/adapters/mcp.snapshot.d.ts index 0ea25dcf2..41cfaa198 100644 --- a/tests/__snapshots__/tsnapi/devframe/adapters/mcp.snapshot.d.ts +++ b/tests/__snapshots__/tsnapi/devframe/adapters/mcp.snapshot.d.ts @@ -6,7 +6,7 @@ export interface CreateMcpFetchHandlerOptions { serverName: string; serverVersion: string; exposeSharedState: boolean | ((_: string) => boolean); - authorization: McpAuthorization; + authorization?: McpAuthorization; allowedOrigins?: readonly string[] | false; } export interface CreateMcpServerOptions { diff --git a/tests/__snapshots__/tsnapi/devframe/index.snapshot.d.ts b/tests/__snapshots__/tsnapi/devframe/index.snapshot.d.ts index 3ba1a55ac..f975f2858 100644 --- a/tests/__snapshots__/tsnapi/devframe/index.snapshot.d.ts +++ b/tests/__snapshots__/tsnapi/devframe/index.snapshot.d.ts @@ -148,6 +148,7 @@ export interface DevframeDefinition { }; services?: DevframeServiceInput[]; clientAssets?: StaticAssetsSource; + mcp?: boolean | McpRouteOptions; rpc?: DevframeRpcOptions; setup: (_: DevframeNodeContext, _?: DevframeSetupInfo) => void | Promise; cli?: DevframeCliOptions; @@ -390,7 +391,7 @@ export interface EventUnsubscribe { } export interface McpRouteOptions { path?: string; - authorization: McpAuthorization; + authorization?: McpAuthorization; allowedOrigins?: readonly string[] | false; } export interface RemoteAssets { diff --git a/tests/__snapshots__/tsnapi/devframe/internal.snapshot.d.ts b/tests/__snapshots__/tsnapi/devframe/internal.snapshot.d.ts index 80175bae6..6f33b6f4b 100644 --- a/tests/__snapshots__/tsnapi/devframe/internal.snapshot.d.ts +++ b/tests/__snapshots__/tsnapi/devframe/internal.snapshot.d.ts @@ -361,10 +361,6 @@ export declare const diagnostics: import("nostics").Diagnostics<{ }) => string; readonly fix: "On Bun/Deno, serve the advertised `__ws` route from `Bun.serve` / `Deno.serve` with `attachBunWsTransport` / `attachDenoWsTransport` (see the hub-deno-minimal example), or connect over the SSE endpoint instead."; }; - readonly DF0077: { - readonly why: "The route-based MCP server needs an authorization policy, but none is configured — refusing to mount an unauthenticated agent endpoint."; - readonly fix: "Set the `DEVFRAME_MCP_AUTH_TOKEN` environment variable (the bearer `mcp: true` requires), or pass an explicit `authorization` on the MCP options — a non-empty bearer token string, a `(request) => boolean` callback, or `false` for an origin-only local opt-out."; - }; }, readonly [(d: import("nostics").Diagnostic, { method }?: { method?: "log" | "warn" | "error"; }) => void]>; diff --git a/tests/e2e/_support/mcp-auth.ts b/tests/e2e/_support/mcp-auth.ts deleted file mode 100644 index 2d7919774..000000000 --- a/tests/e2e/_support/mcp-auth.ts +++ /dev/null @@ -1,8 +0,0 @@ -/** - * The bearer `devframe connect` presents to an authenticated instance MCP - * route (via `DEVFRAME_MCP_AUTH_TOKEN`). Shared between `playwright.config.ts` - * (which starts the `hub-next` server with it) and the connect support helper - * (which spawns the connector with it), so the two agree on the credential. - * Kept dependency-free so the Playwright config can import it cheaply. - */ -export const MCP_AUTH_TOKEN = 'devframe-e2e-mcp-auth-token' diff --git a/tests/e2e/_support/mcp-connect.ts b/tests/e2e/_support/mcp-connect.ts index 1aafca415..4c4a93d3a 100644 --- a/tests/e2e/_support/mcp-connect.ts +++ b/tests/e2e/_support/mcp-connect.ts @@ -1,15 +1,12 @@ import { fileURLToPath } from 'node:url' import { Client } from '@modelcontextprotocol/client' import { StdioClientTransport } from '@modelcontextprotocol/client/stdio' -import { MCP_AUTH_TOKEN } from './mcp-auth' const BIN = fileURLToPath(new URL('../../../packages/devframe/bin/devframe.mjs', import.meta.url)) /** * Spawn `devframe connect` over stdio against a hermetic registry dir and - * hand a connected MCP client to `fn`, tearing the process down after. The - * connector reads `DEVFRAME_MCP_AUTH_TOKEN` to authenticate against instances - * whose MCP route requires a bearer (harmless for origin-only routes). + * hand a connected MCP client to `fn`, tearing the process down after. */ export async function withConnectClient( instancesDir: string, @@ -18,8 +15,6 @@ export async function withConnectClient( const transport = new StdioClientTransport({ command: 'node', args: [BIN, 'connect', '--instances-dir', instancesDir], - // Merged with the SDK's safe default env (which carries PATH etc.). - env: { DEVFRAME_MCP_AUTH_TOKEN: MCP_AUTH_TOKEN }, }) const client = new Client({ name: 'devframe-e2e', version: '0.0.0' }) await client.connect(transport) diff --git a/tests/optional-mcp-bundles.test.ts b/tests/optional-mcp-bundles.test.ts index 8d7ac6150..e2a9c1ce5 100644 --- a/tests/optional-mcp-bundles.test.ts +++ b/tests/optional-mcp-bundles.test.ts @@ -63,9 +63,7 @@ describe('optional MCP peers in consumer bundles', () => { const hub = bundled.initHub({ auth: false, base: bundled.DEVFRAMES_HUB_BASE, - // Origin-only opt-out: this loopback-bound bundle-load smoke test only - // needs the route mounted; the identity gate is covered elsewhere. - mcp: { authorization: false }, + mcp: true, ws: false, }) From dec5f0e4df7fe745157adca9c91616ae5d77ffb3 Mon Sep 17 00:00:00 2001 From: "Anthony Fu (via agent)" Date: Thu, 3 Sep 2026 02:21:51 +0000 Subject: [PATCH 4/4] refactor(devframe): keep MCP config off the top-level definition MCP exposure is a hosting decision, so it stays on the host entry points (initDevframe / initHub / createDevServer / bridges) and the standalone CLI config (cli.mcp), not a top-level DevframeDefinition field. Reverts the top-level DevframeDefinition.mcp addition; cli.mcp remains the definition-side default that createCac reads. Created with the help of an agent. --- docs/content/2.adapters/7.mcp.md | 20 ++++++----- docs/content/3.frameworks/1.vite.md | 2 +- docs/content/3.frameworks/3.next.md | 2 +- examples/files-inspector/src/devframe.ts | 13 +++---- packages/devframe/src/adapters/_shared.ts | 8 ++--- packages/devframe/src/adapters/cac.ts | 7 ++-- packages/devframe/src/adapters/dev.ts | 6 ++-- packages/devframe/src/adapters/initiate.ts | 7 ++-- packages/devframe/src/types/devframe.ts | 35 ++++++++----------- .../hub/src/node/__tests__/initiate.test.ts | 6 ++-- packages/hub/src/node/initiate.ts | 2 +- packages/next/src/handler.ts | 4 +-- packages/vite/src/single.ts | 5 ++- .../tsnapi/devframe/index.snapshot.d.ts | 1 - 14 files changed, 56 insertions(+), 62 deletions(-) diff --git a/docs/content/2.adapters/7.mcp.md b/docs/content/2.adapters/7.mcp.md index a35b6ad25..e23283703 100644 --- a/docs/content/2.adapters/7.mcp.md +++ b/docs/content/2.adapters/7.mcp.md @@ -18,14 +18,16 @@ await createMcpServer(myDevframe, { transport: 'stdio' }) ## Route-based server -The dev server exposes the same MCP API over HTTP, live. Enable with the top-level `mcp`: +The dev server exposes the same MCP API over HTTP, live. Enable it with `cli.mcp` (or pass `mcp` to `createDevServer` / `initDevframe` / `initHub` when you host it programmatically): ```ts import { defineDevframe } from 'devframe' export default defineDevframe({ // … - mcp: true, + cli: { + mcp: true, + }, }) ``` @@ -41,12 +43,14 @@ The **origin gate** guards every request: `Origin` must be loopback (or allow-li ```ts export default defineDevframe({ - // A bearer token, backed by your own environment variable (never a literal): - mcp: { authorization: process.env.MY_TOKEN }, - // — or a callback identity check (governs identity only; it cannot relax the origin gate): - // mcp: { authorization: request => isTrusted(request) }, - // — or the explicit default, origin-only: - // mcp: { authorization: false }, + cli: { + // A bearer token, backed by your own environment variable (never a literal): + mcp: { authorization: process.env.MY_TOKEN }, + // — or a callback identity check (governs identity only; it cannot relax the origin gate): + // mcp: { authorization: request => isTrusted(request) }, + // — or the explicit default, origin-only: + // mcp: { authorization: false }, + }, }) ``` diff --git a/docs/content/3.frameworks/1.vite.md b/docs/content/3.frameworks/1.vite.md index 4fb1ce6a5..c48eb0934 100644 --- a/docs/content/3.frameworks/1.vite.md +++ b/docs/content/3.frameworks/1.vite.md @@ -42,7 +42,7 @@ Devframe spawns a separate RPC + WS server and registers Vite middleware at `__mcp`. `true` is origin-only (trusts same-machine callers); `McpRouteOptions` can add an `authorization` identity check. | +| `mcp` | `def.cli?.mcp` | Expose the MCP route at `__mcp`. `true` is origin-only (trusts same-machine callers); `McpRouteOptions` can add an `authorization` identity check. | ## `devframeVite` — convenience wrapper diff --git a/docs/content/3.frameworks/3.next.md b/docs/content/3.frameworks/3.next.md index 4c5c6ef0d..0de01aa51 100644 --- a/docs/content/3.frameworks/3.next.md +++ b/docs/content/3.frameworks/3.next.md @@ -48,7 +48,7 @@ export const GET = handler.fetch | `port` | from `def.cli?.port` | Side-car port. | | `flags` | — | Passed to `def.setup(ctx, { flags })`. | | `auth` | `false` | `true` for the OTP gate, or a handler. | -| `mcp` | `def.mcp` | Expose the MCP route. `true` is origin-only (trusts same-machine callers); `McpRouteOptions` can add an `authorization` identity check. | +| `mcp` | `def.cli?.mcp` | Expose the MCP route. `true` is origin-only (trusts same-machine callers); `McpRouteOptions` can add an `authorization` identity check. | | `key` | `@devframes/next::` | `globalThis` memoization key. | ## Hosting a hub diff --git a/examples/files-inspector/src/devframe.ts b/examples/files-inspector/src/devframe.ts index 539f29f45..c0cdcd4d3 100644 --- a/examples/files-inspector/src/devframe.ts +++ b/examples/files-inspector/src/devframe.ts @@ -17,18 +17,19 @@ export default defineDevframe({ icon: 'ph:folder-open-duotone', basePath: BASE_PATH, clientAssets: distDir, - // Serve the agent surface over the dev server's `/__mcp` route and register - // the instance for `devframe connect` discovery. This demo binds to loopback - // (`localhost:9876`), so `mcp: true` (the loopback origin gate, trusting - // same-machine callers) is enough - no bearer plumbing. A network-reachable - // tool would harden it with `mcp: { authorization: process.env.MY_TOKEN }`. - mcp: true, cli: { command: 'devframe-files-inspector', port: 9876, // Single-user localhost demo - skip the trust handshake so the served // SPA can call RPC without an OTP round-trip. auth: false, + // Serve the agent surface over the dev server's `/__mcp` route and + // register the instance for `devframe connect` discovery. This demo binds + // to loopback (`localhost:9876`), so `mcp: true` (the loopback origin + // gate, trusting same-machine callers) is enough - no bearer plumbing. A + // network-reachable tool would harden it with + // `mcp: { authorization: process.env.MY_TOKEN }`. + mcp: true, }, setup(ctx) { // A scoped context auto-namespaces every registered id with `NAMESPACE:`. diff --git a/packages/devframe/src/adapters/_shared.ts b/packages/devframe/src/adapters/_shared.ts index a22b734a6..264b6a2eb 100644 --- a/packages/devframe/src/adapters/_shared.ts +++ b/packages/devframe/src/adapters/_shared.ts @@ -95,9 +95,9 @@ export function resolveMcpConfig(mcp: boolean | McpRouteOptions | undefined): Re /** * Resolve the `mcp` entry a `__connection.json` should advertise for a dev - * server started with the given `mcp` option (falling back to `def.mcp`, then - * the deprecated `def.cli?.mcp`, exactly like `createDevServer`), or - * `undefined` when the route is disabled. + * server started with the given `mcp` option (falling back to `def.cli?.mcp`, + * exactly like `createDevServer`), or `undefined` when the route is + * disabled. * * Hosted bridges that hand-roll their connection meta pass the side-car * `port`: the advertised path becomes absolute (the side-car mounts at `/`) @@ -110,7 +110,7 @@ export function resolveMcpConnectionMeta( mcp: boolean | McpRouteOptions | undefined, port?: number, ): ConnectionMeta['mcp'] { - const config = resolveMcpConfig(mcp ?? def.mcp ?? def.cli?.mcp) + const config = resolveMcpConfig(mcp ?? def.cli?.mcp) if (!config) return undefined const route = withoutLeadingSlash(config.path ?? DEVFRAME_MCP_ROUTE) diff --git a/packages/devframe/src/adapters/cac.ts b/packages/devframe/src/adapters/cac.ts index 9d3e4d3fb..2a0290722 100644 --- a/packages/devframe/src/adapters/cac.ts +++ b/packages/devframe/src/adapters/cac.ts @@ -70,9 +70,8 @@ export function createCac(d: DevframeDefinition, options: CreateCacOptions = {}) .option('--no-auth', 'Disable the interactive authentication gate') // Only `--mcp` is declared: CAC's `--no-*` auto-negation would inject a // `true` default, silently enabling MCP. Declaring just `--mcp` yields the - // opt-in tri-state — absent → `undefined` (falls through to the - // definition's `mcp`), `--mcp` → `true`, `--no-mcp` → `false` (handled by - // CAC's `--no-` prefix). + // opt-in tri-state — absent → `undefined` (falls through to `cli.mcp`), + // `--mcp` → `true`, `--no-mcp` → `false` (handled by CAC's `--no-` prefix). .option('--mcp', 'Expose an MCP server over HTTP at /__mcp (use --no-mcp to disable)') // Register typed flags from the definition ahead of `cli.configure` @@ -96,7 +95,7 @@ export function createCac(d: DevframeDefinition, options: CreateCacOptions = {}) const port = (flags.port as number | undefined) ?? await resolveDevServerPort(d, { host, defaultPort }) // `--mcp` / `--no-mcp` map to a boolean override; when neither is // passed CAC leaves `mcp` undefined so `createDevServer` falls through - // to the definition's `mcp` (top-level, then the deprecated `cli.mcp`). + // to `def.cli?.mcp`. const mcp = flags.mcp as boolean | undefined await createDevServer(d, { host, diff --git a/packages/devframe/src/adapters/dev.ts b/packages/devframe/src/adapters/dev.ts index c93f62217..9b495dddb 100644 --- a/packages/devframe/src/adapters/dev.ts +++ b/packages/devframe/src/adapters/dev.ts @@ -92,9 +92,9 @@ export interface CreateDevServerOptions { auth?: boolean | DevframeAuthHandler /** * Expose a route-based MCP server on the dev server (Streamable-HTTP). - * Overrides the definition's `mcp` (top-level, then the deprecated - * `cli.mcp`); `undefined` falls through to it. `false` disables the route - * regardless of the definition default. See {@link McpRouteOptions}. + * Overrides `def.cli?.mcp`; `undefined` falls through to it. `false` + * disables the route regardless of the definition default. See + * {@link McpRouteOptions}. */ mcp?: boolean | McpRouteOptions /** diff --git a/packages/devframe/src/adapters/initiate.ts b/packages/devframe/src/adapters/initiate.ts index 705ddb813..f4b4b3558 100644 --- a/packages/devframe/src/adapters/initiate.ts +++ b/packages/devframe/src/adapters/initiate.ts @@ -89,9 +89,8 @@ export interface InitDevframeOptions { auth?: boolean | DevframeAuthHandler /** * Expose a route-based MCP server (Streamable-HTTP) at `__mcp` and - * advertise it in `__connection.json`. Overrides the definition's `mcp` - * (top-level, then the deprecated `cli.mcp`); `undefined` falls through to - * it. See {@link McpRouteOptions}. + * advertise it in `__connection.json`. Overrides `def.cli?.mcp`; + * `undefined` falls through to it. See {@link McpRouteOptions}. */ mcp?: boolean | McpRouteOptions /** @@ -309,7 +308,7 @@ export function initDevframe( // only pulled in (dynamically) when the route is enabled. The resolved // config trusts same-machine callers by default (origin-only); an object // config can opt into a bearer/callback identity check. - const mcpConfig = resolveMcpConfig(options.mcp ?? def.mcp ?? def.cli?.mcp) + const mcpConfig = resolveMcpConfig(options.mcp ?? def.cli?.mcp) let mcpMeta: ConnectionMeta['mcp'] let mcpDispose: (() => Promise) | undefined if (mcpConfig) { diff --git a/packages/devframe/src/types/devframe.ts b/packages/devframe/src/types/devframe.ts index c2350d50f..1f665ce6d 100644 --- a/packages/devframe/src/types/devframe.ts +++ b/packages/devframe/src/types/devframe.ts @@ -106,7 +106,7 @@ export type McpAuthorization /** * Configuration for the route-based MCP server mounted alongside the dev - * server (opt-in via {@link DevframeDefinition.mcp}). The endpoint speaks + * server (opt-in via {@link DevframeCliOptions.mcp}). The endpoint speaks * the MCP Streamable-HTTP transport over the same origin as the SPA, * exposing the definition's `ctx.agent` tools + shared-state resources to * external MCP clients connected to the *running* server. @@ -184,11 +184,20 @@ export interface DevframeCliOptions { */ auth?: boolean | DevframeAuthHandler /** - * Expose a route-based MCP server alongside the dev server. + * Expose a route-based MCP server alongside the standalone dev server, + * speaking the MCP Streamable-HTTP transport at `/__mcp` (relative to the + * base path). It surfaces the same `ctx.agent` tools + shared-state + * resources as the stdio `mcp` command, but against the live server. * - * @deprecated Moved to the top-level {@link DevframeDefinition.mcp}. Set - * `mcp` on the definition instead. This field is still read as a fallback - * when the top-level `mcp` is unset, so existing definitions keep working. + * - `false` / omitted (default) — no MCP route is mounted. + * - `true` — mount at the default `__mcp` route with the loopback origin + * gate (trusting same-machine callers). + * - {@link McpRouteOptions} — customise the route path, origin allow-list, + * and opt into an {@link McpAuthorization} identity check. + * + * The `--mcp` / `--no-mcp` CLI flags override this per run. Whether to expose + * MCP is a hosting decision, so programmatic hosts pass it to + * `initDevframe` / `initHub` / `createDevServer` instead. */ mcp?: boolean | McpRouteOptions /** @@ -416,22 +425,6 @@ export interface DevframeDefinition { * {@link DevframeCliOptions.distDir} is read as a fallback. */ clientAssets?: StaticAssetsSource - /** - * Expose a route-based MCP server alongside the running dev server, speaking - * the MCP Streamable-HTTP transport at `/__mcp` (relative to the base path). - * It surfaces the same `ctx.agent` tools + shared-state resources as the - * stdio `mcp` command, but against the live server. - * - * - `false` / omitted (default) — no MCP route is mounted. - * - `true` — mount at the default `__mcp` route with the loopback origin - * gate (trusting same-machine callers). - * - {@link McpRouteOptions} — customise the route path, origin allow-list, - * and opt into an {@link McpAuthorization} identity check. - * - * The `--mcp` / `--no-mcp` CLI flags override this per run. When unset, the - * deprecated {@link DevframeCliOptions.mcp} is read as a fallback. - */ - mcp?: boolean | McpRouteOptions /** RPC-level configuration for this devframe (see {@link DevframeRpcOptions}). */ rpc?: DevframeRpcOptions /** Server-side setup — the primary entrypoint. Runs in every runtime. */ diff --git a/packages/hub/src/node/__tests__/initiate.test.ts b/packages/hub/src/node/__tests__/initiate.test.ts index c841ac5b1..92fba279a 100644 --- a/packages/hub/src/node/__tests__/initiate.test.ts +++ b/packages/hub/src/node/__tests__/initiate.test.ts @@ -324,14 +324,14 @@ describe('initHub', () => { it('warns (DF8005) when a mounted devframe asks for MCP but the hub MCP is off', async () => { const wsPort = await getPort({ port: 18235, host: '127.0.0.1' }) const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) - // The hub has no `mcp`, but `beta` declares `mcp: true` — the hub's single - // aggregate route governs MCP, so beta's request is a no-op and warns. + // The hub has no `mcp`, but `beta` declares `cli.mcp: true` — the hub's + // single aggregate route governs MCP, so beta's request is a no-op and warns. const hub = initHub({ base: DEVFRAMES_HUB_BASE, auth: false, host: '127.0.0.1', ws: { port: wsPort }, - devframes: [makeFrame('alpha'), { ...makeFrame('beta'), mcp: true }], + devframes: [makeFrame('alpha'), { ...makeFrame('beta'), cli: { mcp: true } }], }) try { diff --git a/packages/hub/src/node/initiate.ts b/packages/hub/src/node/initiate.ts index c7232a40b..0ad82bf37 100644 --- a/packages/hub/src/node/initiate.ts +++ b/packages/hub/src/node/initiate.ts @@ -533,7 +533,7 @@ export function initHub(options: InitHubOptions): HubInstance { // so a devframe's own `mcp` request is only meaningful when the hub's // own MCP is enabled. Warn when it isn't, rather than silently // dropping the devframe's intended agent surface. - if (!options.mcp && (def.mcp ?? def.cli?.mcp)) + if (!options.mcp && def.cli?.mcp) diagnostics.DF8005({ id: def.id }) const frameBase = withTrailingSlash(joinURL(base, def.id)) const run = await prepareDevframe(ctx, def, { base: frameBase, ...(dock ? { dock } : {}) }) diff --git a/packages/next/src/handler.ts b/packages/next/src/handler.ts index b6c8d3430..462f76c96 100644 --- a/packages/next/src/handler.ts +++ b/packages/next/src/handler.ts @@ -35,8 +35,8 @@ export interface CreateDevframeNextHandlerOptions { * Expose the route-based MCP server (Streamable-HTTP) at `__mcp` — * on the Next app's own origin, through the same catch-all route as the * SPA — and advertise it in the handler's `__connection.json`. Overrides - * the definition's `mcp` (top-level, then the deprecated `cli.mcp`); - * `undefined` falls through to it, `false` disables the route regardless. + * `def.cli?.mcp`, `undefined` falls through to it, `false` disables the + * route regardless. */ mcp?: InitDevframeOptions['mcp'] /** diff --git a/packages/vite/src/single.ts b/packages/vite/src/single.ts index 9774d44cf..8932dde5b 100644 --- a/packages/vite/src/single.ts +++ b/packages/vite/src/single.ts @@ -114,9 +114,8 @@ export interface DevframeViteBridgeOptions { /** * Expose the bridge's route-based MCP server (Streamable-HTTP) at * `__mcp` — on the Vite app's own origin — and advertise it in the - * bridge's `__connection.json`. Overrides the definition's `mcp` (top-level, - * then the deprecated `cli.mcp`); `undefined` falls through to it, `false` - * disables the route regardless. + * bridge's `__connection.json`. Overrides `def.cli?.mcp`, `undefined` + * falls through to it, `false` disables the route regardless. */ mcp?: boolean | McpRouteOptions } diff --git a/tests/__snapshots__/tsnapi/devframe/index.snapshot.d.ts b/tests/__snapshots__/tsnapi/devframe/index.snapshot.d.ts index f975f2858..1d52f01d9 100644 --- a/tests/__snapshots__/tsnapi/devframe/index.snapshot.d.ts +++ b/tests/__snapshots__/tsnapi/devframe/index.snapshot.d.ts @@ -148,7 +148,6 @@ export interface DevframeDefinition { }; services?: DevframeServiceInput[]; clientAssets?: StaticAssetsSource; - mcp?: boolean | McpRouteOptions; rpc?: DevframeRpcOptions; setup: (_: DevframeNodeContext, _?: DevframeSetupInfo) => void | Promise; cli?: DevframeCliOptions;