diff --git a/docs/content/1.guide/14.security.md b/docs/content/1.guide/14.security.md index 68e7f84a7..cf9973e98 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 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 the `'auto'` default - which mounts the route once agent tools exist - and `mcp: true` are 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), or turn the route off with `mcp: false`. See [MCP](/adapters/mcp). +- **The MCP route trusts same-machine callers, harden it when that's not your boundary.** Two gates enforce that default: an origin gate (loopback-only, `Origin`-less rejected) is browser DNS-rebinding hardening, and a peer-address gate rejects a non-loopback caller even with a forged loopback `Origin` (the socket address can't be forged the way a header can). So the `'auto'` default - which mounts the route once agent tools exist - and `mcp: true` are enough for a local dev tool. Neither gate proves *which* caller it is, though, so to intentionally reach the route beyond loopback (a widened `allowedOrigins`, a hosted app) or to expose destructive tools, add an identity check with `mcp: { authorization }` (a bearer from an env var, or a callback), which also lifts the loopback-peer restriction - or turn the route off with `mcp: false`. 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/2.adapters/7.mcp.md b/docs/content/2.adapters/7.mcp.md index 8d52b6727..a39cda8e2 100644 --- a/docs/content/2.adapters/7.mcp.md +++ b/docs/content/2.adapters/7.mcp.md @@ -35,9 +35,11 @@ The endpoint is **stateless**: it serves the [2026-07-28 revision](https://model ### Origin gate, and opt-in identity -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 - the `'auto'` default and `mcp: true` both mount origin-only, all a local dev tool needs. +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 a browser from reaching the route across origins. It is **not** a network-locality check: `Origin` is a request header, so a non-browser client (curl, a script) sends any value it likes. -`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`: +Same-machine locality is instead proven from the **connected peer**: on the origin-only default (the `'auto'` default and `mcp: true`, with no widened `allowedOrigins` and no identity check), a request whose peer address is not loopback gets `403` even with a loopback `Origin`. The peer address comes from the socket, not a header, so a remote client cannot forge it. This is what makes "trusts same-machine callers" hold, and it's all a local dev tool needs. (A host that can't resolve a peer address, such as a serverless route, keeps the origin-only behavior; harden it with `authorization`.) + +`Origin` still proves nothing about *who* is calling, and the peer check only proves *where from*. When same-machine isn't your trust boundary (a LAN/tunnel origin, a shared/CI host, a destructive tool surface), layer on an **identity check** with `authorization` - which also lifts the loopback-peer restriction, so authenticated callers may be remote: ```ts createCac(myDevframe, { diff --git a/packages/devframe/src/adapters/mcp/__tests__/mcp-locality.test.ts b/packages/devframe/src/adapters/mcp/__tests__/mcp-locality.test.ts new file mode 100644 index 000000000..183e61f52 --- /dev/null +++ b/packages/devframe/src/adapters/mcp/__tests__/mcp-locality.test.ts @@ -0,0 +1,112 @@ +import type { McpAuthorization } from '../../../types/devframe' +import type { DevframeHost } from '../../../types/host' +import type { McpConnectionInfo } from '../fetch' +import { createHostContext } from 'devframe/node' +import { isLoopbackAddress } from 'devframe/utils/origin' +import { afterEach, describe, expect, it } from 'vitest' +import { createMcpFetchHandler } from '../fetch' + +function nullHost(): DevframeHost { + return { + mountStatic: () => { /* no-op */ }, + resolveOrigin: () => 'http://localhost', + getStorageDir: () => '/tmp/devframe-test-storage', + } +} + +const disposers: Array<() => Promise> = [] + +afterEach(async () => { + await Promise.all(disposers.splice(0).map(d => d())) +}) + +async function handlerWith(options: { authorization?: McpAuthorization, allowedOrigins?: readonly string[] | false } = {}) { + const ctx = await createHostContext({ cwd: process.cwd(), mode: 'dev', host: nullHost() }) + ctx.agent.registerTool({ id: 'greet', description: 'Say hello.', safety: 'read', handler: () => ({ greeting: 'hi' }) }) + const handler = createMcpFetchHandler(ctx, { + serverName: 'test', + serverVersion: '0.0.0-test', + exposeSharedState: true, + ...options, + }) + disposers.push(handler.dispose) + return handler +} + +/** A well-formed `initialize` request carrying a (forgeable) loopback Origin. */ +function initRequest(headers: Record = {}): Request { + return new Request('http://localhost/__mcp', { + method: 'POST', + headers: { + 'content-type': 'application/json', + 'accept': 'application/json, text/event-stream', + 'origin': 'http://localhost', + ...headers, + }, + body: JSON.stringify({ + jsonrpc: '2.0', + id: 1, + method: 'initialize', + params: { protocolVersion: '2025-03-26', capabilities: {}, clientInfo: { name: 'x', version: '0' } }, + }), + }) +} + +async function status(handler: Awaited>, connection?: McpConnectionInfo): Promise { + const res = await handler.fetch(initRequest(), connection) + await res.body?.cancel() + return res.status +} + +describe('mcp locality gate (origin-only default)', () => { + it('rejects a forged loopback Origin from a non-loopback peer with 403', async () => { + // The reported RCE: a raw network client sends `Origin: http://localhost` + // to pass the origin gate. The peer address it cannot forge gives it away. + const handler = await handlerWith() + expect(await status(handler, { remoteAddress: '203.0.113.7' })).toBe(403) + }) + + it('allows a loopback peer (local dev keeps working with zero config)', async () => { + const handler = await handlerWith() + expect(await status(handler, { remoteAddress: '127.0.0.1' })).toBe(200) + }) + + it('allows an IPv4-mapped IPv6 loopback peer from a dual-stack listener', async () => { + const handler = await handlerWith() + expect(await status(handler, { remoteAddress: '::ffff:127.0.0.1' })).toBe(200) + }) + + it('falls back to origin-only when the host cannot resolve a peer address', async () => { + const handler = await handlerWith() + expect(await status(handler, {})).toBe(200) + expect(await status(handler, undefined)).toBe(200) + }) + + it('an identity check lifts the loopback-peer restriction', async () => { + const handler = await handlerWith({ authorization: 'a-high-entropy-test-bearer-token' }) + expect(await status(handler, { remoteAddress: '203.0.113.7' })).toBe(401) + const ok = await handler.fetch( + initRequest({ authorization: 'Bearer a-high-entropy-test-bearer-token' }), + { remoteAddress: '203.0.113.7' }, + ) + await ok.body?.cancel() + expect(ok.status).toBe(200) + }) + + it('allowedOrigins: false opts out of both origin and locality gates', async () => { + const handler = await handlerWith({ allowedOrigins: false }) + expect(await status(handler, { remoteAddress: '203.0.113.7' })).toBe(200) + }) +}) + +describe('isLoopbackAddress', () => { + it('accepts loopback literals a socket reports', () => { + for (const a of ['127.0.0.1', '127.5.5.5', '::1', '::ffff:127.0.0.1', '[::1]']) + expect(isLoopbackAddress(a)).toBe(true) + }) + + it('rejects routable and mapped-routable addresses', () => { + for (const a of ['203.0.113.7', '10.0.0.5', '192.168.1.9', '::ffff:203.0.113.7', '0.0.0.0']) + expect(isLoopbackAddress(a)).toBe(false) + }) +}) diff --git a/packages/devframe/src/adapters/mcp/fetch.ts b/packages/devframe/src/adapters/mcp/fetch.ts index 3924cdb4d..8ff63c9c8 100644 --- a/packages/devframe/src/adapters/mcp/fetch.ts +++ b/packages/devframe/src/adapters/mcp/fetch.ts @@ -1,7 +1,7 @@ import type { DevframeNodeContext, McpAuthorization } from 'devframe/types' import { createMcpHandler } from '@modelcontextprotocol/server' import { timingSafeEqual } from 'devframe/utils/crypto-token' -import { isAllowedOrigin } from 'devframe/utils/origin' +import { isAllowedOrigin, isLoopbackAddress } from 'devframe/utils/origin' import { bridgeListChanged, buildMcpServerFromContext } from './build-server' export interface CreateMcpFetchHandlerOptions { @@ -62,13 +62,27 @@ async function isAuthorized(req: Request, authorization: McpAuthorization): Prom return timingSafeEqual(token, authorization) } +/** Connection facts a host knows about a request beyond the `Request` itself. */ +export interface McpConnectionInfo { + /** + * The connecting peer's remote address (a node socket's `remoteAddress`), + * used to prove a same-machine caller when the endpoint relies on the + * loopback origin default with no identity check. A host that can resolve a + * trustworthy peer address (the h3/node mount) supplies it; when it's + * omitted the origin gate stays the only locality signal. + */ + remoteAddress?: string +} + export interface McpFetchHandler { /** * WHATWG-`fetch` handler for the MCP endpoint. Hand every method * (POST/GET/DELETE) on the endpoint's path to it; routing by path is the - * host's job. + * host's job. Pass {@link McpConnectionInfo} when the host can resolve the + * peer address so the default trust boundary can enforce same-machine + * locality. */ - fetch: (request: Request) => Promise + fetch: (request: Request, connection?: McpConnectionInfo) => Promise /** Tear down the handler (aborts in-flight exchanges, drops the change bridge). */ dispose: () => Promise } @@ -89,13 +103,19 @@ export interface McpFetchHandler { * * 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). + * `Origin`-less requests, so a browser can't reach the route across origins (a + * disallowed origin gets `403`). The `Origin` header is only browser hardening: + * a non-browser client forges it. So on the zero-config default (no widened + * `allowedOrigins`, no identity check) a second locality gate requires the + * connected peer to be loopback, proven from the host-supplied + * {@link McpConnectionInfo.remoteAddress} (which a client cannot forge), so the + * "trusts same-machine callers" default holds against a remote raw client. + * When same-machine isn't your trust boundary, add an 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), which also lifts the loopback-peer restriction for authenticated + * callers. */ export function createMcpFetchHandler( ctx: DevframeNodeContext, @@ -120,7 +140,13 @@ export function createMcpFetchHandler( resources: () => { handler.notify.resourcesChanged() }, }) - async function handle(req: Request): Promise { + // The zero-config trust boundary: no widened origin allow-list and no + // identity check, so the endpoint trusts same-machine callers alone. A + // loopback `Origin` is only browser hardening (a raw client forges it), so + // here locality is proven from the connected peer instead. + const originOnlyDefault = allowedOrigins === undefined && authorization === false + + async function handle(req: Request, connection?: McpConnectionInfo): Promise { // Origin gate: the endpoint's DNS-rebinding protection and its guard // against arbitrary local processes. Unlike the WS transport, an // `Origin`-less request is rejected: a route-based MCP endpoint would @@ -130,6 +156,14 @@ export function createMcpFetchHandler( if (allowedOrigins !== false && (origin === undefined || !isAllowedOrigin(origin, allowedOrigins ?? []))) return new Response('Forbidden', { status: 403 }) + // Locality gate: a raw client forges `Origin: http://localhost`, so when + // that default is the only trust boundary, require the connected peer to be + // loopback (an address it cannot forge). Opt out with `authorization` or + // `allowedOrigins: false`; a host that can't resolve a peer stays + // origin-only. + if (originOnlyDefault && connection?.remoteAddress !== undefined && !isLoopbackAddress(connection.remoteAddress)) + 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 diff --git a/packages/devframe/src/adapters/mcp/http.ts b/packages/devframe/src/adapters/mcp/http.ts index ff9ec014c..e6e150026 100644 --- a/packages/devframe/src/adapters/mcp/http.ts +++ b/packages/devframe/src/adapters/mcp/http.ts @@ -1,7 +1,7 @@ import type { DevframeNodeContext } from 'devframe/types' import type { H3, H3Event } from 'h3' import type { CreateMcpFetchHandlerOptions } from './fetch' -import { defineHandler } from 'h3' +import { defineHandler, getRequestIP } from 'h3' import { createMcpFetchHandler } from './fetch' export interface MountMcpHttpOptions extends CreateMcpFetchHandlerOptions {} @@ -32,7 +32,14 @@ export function mountMcpHttp( ): MountedMcpHttp { const handler = createMcpFetchHandler(ctx, options) - app.use(path, defineHandler(async event => respond(event, await handler.fetch(event.req)))) + // `getRequestIP` (default, `xForwardedFor: false`) returns the connected + // socket's own address, never a client-supplied `X-Forwarded-For`, so the + // handler's locality gate proves a same-machine caller from an address the + // client cannot forge (a widened `allowedOrigins` or a proxy deployment opts + // out via `authorization` / `allowedOrigins: false`). + app.use(path, defineHandler(async event => + respond(event, await handler.fetch(event.req, { remoteAddress: getRequestIP(event) })), + )) return { dispose: handler.dispose, diff --git a/packages/devframe/src/adapters/mcp/index.ts b/packages/devframe/src/adapters/mcp/index.ts index 7e7937903..0482cf1e8 100644 --- a/packages/devframe/src/adapters/mcp/index.ts +++ b/packages/devframe/src/adapters/mcp/index.ts @@ -18,6 +18,7 @@ export { export { createMcpFetchHandler, type CreateMcpFetchHandlerOptions, + type McpConnectionInfo, type McpFetchHandler, } from './fetch' diff --git a/packages/devframe/src/utils/origin.ts b/packages/devframe/src/utils/origin.ts index d71646aab..ea9b42ca5 100644 --- a/packages/devframe/src/utils/origin.ts +++ b/packages/devframe/src/utils/origin.ts @@ -28,6 +28,29 @@ export function isLoopbackHostname(hostname: string): boolean { return isLoopbackIPv4(h) } +/** + * Whether `address` is a loopback peer address as reported by a socket + * (`net.Socket.remoteAddress`): the IPv6 loopback `::1`, an IPv4 literal in + * `127.0.0.0/8`, or an IPv4-mapped IPv6 form of one (`::ffff:127.0.0.1`). + * + * Unlike {@link isLoopbackHostname} this takes a raw address, not a hostname: + * it never accepts a `localhost`-style name (a socket peer is always a literal + * address) and understands the IPv4-mapped IPv6 form the OS hands back on a + * dual-stack listener. Used to prove a same-machine caller from the connected + * peer, which a client cannot forge, rather than from the `Origin` header, + * which it can. + */ +export function isLoopbackAddress(address: string): boolean { + let h = address.trim().replace(/^\[|\]$/g, '') // strip IPv6 brackets + const zone = h.indexOf('%') // drop an IPv6 zone id (fe80::1%eth0) + if (zone !== -1) + h = h.slice(0, zone) + if (h === '::1') + return true + const mapped = /^::ffff:(.+)$/i.exec(h) + return isLoopbackIPv4(mapped ? mapped[1] : h) +} + /** A canonical dotted-decimal IPv4 literal in `127.0.0.0/8`. */ function isLoopbackIPv4(hostname: string): boolean { const octets = hostname.split('.') diff --git a/tests/__snapshots__/tsnapi/devframe/adapters/mcp.snapshot.d.ts b/tests/__snapshots__/tsnapi/devframe/adapters/mcp.snapshot.d.ts index 41cfaa198..a69263e97 100644 --- a/tests/__snapshots__/tsnapi/devframe/adapters/mcp.snapshot.d.ts +++ b/tests/__snapshots__/tsnapi/devframe/adapters/mcp.snapshot.d.ts @@ -18,8 +18,11 @@ export interface CreateMcpServerOptions { transport: 'stdio'; }) => void; } +export interface McpConnectionInfo { + remoteAddress?: string; +} export interface McpFetchHandler { - fetch: (_: Request) => Promise; + fetch: (_: Request, _?: McpConnectionInfo) => Promise; dispose: () => Promise; } export interface McpServerHandle { diff --git a/tests/__snapshots__/tsnapi/devframe/utils/origin.snapshot.d.ts b/tests/__snapshots__/tsnapi/devframe/utils/origin.snapshot.d.ts index 24f8556d1..5c7679e8d 100644 --- a/tests/__snapshots__/tsnapi/devframe/utils/origin.snapshot.d.ts +++ b/tests/__snapshots__/tsnapi/devframe/utils/origin.snapshot.d.ts @@ -3,6 +3,7 @@ */ // #region Other export { isAllowedOrigin } +export { isLoopbackAddress } export { isLoopbackHostname } export { validateOriginCandidate } // #endregion \ No newline at end of file diff --git a/tests/__snapshots__/tsnapi/devframe/utils/origin.snapshot.js b/tests/__snapshots__/tsnapi/devframe/utils/origin.snapshot.js index aa340eb89..d91794f63 100644 --- a/tests/__snapshots__/tsnapi/devframe/utils/origin.snapshot.js +++ b/tests/__snapshots__/tsnapi/devframe/utils/origin.snapshot.js @@ -3,6 +3,7 @@ */ // #region Functions export function isAllowedOrigin(_, _) {} +export function isLoopbackAddress(_) {} export function isLoopbackHostname(_) {} export function validateOriginCandidate(_, _) {} // #endregion \ No newline at end of file