diff --git a/docs/adapters/mcp.md b/docs/adapters/mcp.md index 8f146de5..1ff6e65f 100644 --- a/docs/adapters/mcp.md +++ b/docs/adapters/mcp.md @@ -35,7 +35,7 @@ export default defineDevframe({ The endpoint speaks the MCP Streamable-HTTP transport at `/__mcp` (relative to the base path — `/__/__mcp` under a host), sharing the dev server's origin and port. The `--mcp` and `--no-mcp` flags override the definition per run. `__connection.json` advertises the route so in-browser tooling can discover it. -Each client session gets its own MCP server built from the live context, correlated by the `Mcp-Session-Id` header, so `tools/list_changed` and `resources/list_changed` notifications reach connected clients as the tool evolves. The endpoint binds to the same loopback host as the dev server and applies the shared loopback origin gate; widen it for a tunnel or LAN origin: +Each client session gets its own MCP server built from the live context, correlated by the `Mcp-Session-Id` header, so `tools/list_changed` and `resources/list_changed` notifications reach connected clients as the tool evolves. The endpoint binds to the same loopback host as the dev server and applies an origin gate: a request must carry an `Origin` that is loopback (or on the configured allow-list). Unlike the WS transport it rejects `Origin`-less requests, so a route-based endpoint isn't reachable by an arbitrary local process — native clients (like `devframe connect`) send their loopback origin explicitly. Widen the gate for a tunnel or LAN origin: ```ts defineDevframe({ @@ -90,6 +90,6 @@ It exposes two gateway tools (the wire names of the `devframe:connect:*` ids — - **`devframe_connect_list-instances`** — discover running devframe dev servers and list each one's MCP tools. Instances running without an MCP route are listed with a hint to restart with `--mcp`. - **`devframe_connect_call-tool`** — invoke one tool on one instance (`{ port, tool, args }`) over its Streamable-HTTP endpoint. -Discovery reads the **instance registry**: every `createDevServer` (CLI `dev`, `viteDevBridge`, `@devframes/next`'s handler) writes a record to `~/.devframe/instances/-.json` on boot and removes it on close; readers prune records whose liveness probe fails. In-process hosts register explicitly with `registerDevframeInstance` from `devframe/node` — see `createDevframeNextHost().mountMcp` for serving MCP on a Next app's own origin. `--port ` probes an explicit port besides the registry; `DEVFRAME_INSTANCES_DIR` relocates the registry and `DEVFRAME_DISABLE_INSTANCE_REGISTRY=1` opts a server out. +Discovery reads the **instance registry**: every `createDevServer` (CLI `dev`, `viteDevBridge`, `@devframes/next`'s handler) writes a record to `~/.devframe/instances/-.json` on boot and removes it on close; readers prune records whose liveness probe fails. The connector dials each instance's endpoint with the instance's own loopback origin, so it clears the route's origin gate without any configuration. In-process hosts register explicitly with `registerDevframeInstance` from `devframe/node` — see `createDevframeNextHost().mountMcp` for serving MCP on a Next app's own origin. `--port ` probes an explicit port besides the registry; `DEVFRAME_INSTANCES_DIR` relocates the registry and `DEVFRAME_DISABLE_INSTANCE_REGISTRY=1` opts a server out. See the [Agent-Native](/guide/agent-native) page for the full API, safety model, and Claude Desktop integration example. diff --git a/docs/guide/client.md b/docs/guide/client.md index d1bdaf29..2ad1c278 100644 --- a/docs/guide/client.md +++ b/docs/guide/client.md @@ -125,7 +125,7 @@ const ok = await rpc.requestTrustWithCode('047204') The code is single-use, expires after five minutes, and is rotated after repeated wrong attempts, so re-display the current code if an exchange fails. -To authenticate without typing, a host can print a link embedding the code (`buildOtpAuthUrl(origin)`); `connectDevframe` reads the `devframe_otp` query parameter, exchanges it, and strips it from the URL. Rename it with the `otpParam` option, or set `otpParam: false` and drive authentication yourself with the exposed `authenticateWithUrlOtp(rpc)` / `consumeOtpFromUrl()` utilities. +To authenticate without typing, a host can print a link embedding the code (`buildOtpAuthUrl(origin)`); `connectDevframe` reads the `devframe_otp` fragment parameter (`#devframe_otp=…`, kept out of server logs and `Referer`), exchanges it, and strips it from the URL. Rename it with the `otpParam` option, or set `otpParam: false` and drive authentication yourself with the exposed `authenticateWithUrlOtp(rpc)` / `consumeOtpFromUrl()` utilities. ### Re-using an existing token diff --git a/docs/guide/security.md b/docs/guide/security.md index 6fa44c37..689dd38d 100644 --- a/docs/guide/security.md +++ b/docs/guide/security.md @@ -84,18 +84,19 @@ Client methods (`devframe/client`): `requestTrustWithCode(code)` (exchange a cod To skip typing, a host can print a link that embeds the code and open the browser straight into an authenticated session. The standalone CLI (`createCac` / `createDevServer`) does this automatically for `--open`: when the server is auth-gated, the browser it launches already carries the current code, so the tab lands authenticated with no prompt at all. Build the link yourself from the current code with `buildOtpAuthUrl(origin)` (devframe stays headless, so the host prints its own banner): ``` -Devtools ready — authenticate this browser: http://localhost:3000/?devframe_otp=123456 +Devtools ready — authenticate this browser: http://localhost:3000/#devframe_otp=123456 ``` -`connectDevframe` reads the `devframe_otp` parameter, exchanges it, and removes it from the URL before anything else. Only the short-lived, single-use **code** ever rides the URL — the resulting bearer token is stored, never written back to it. Because the link grants trust to whoever opens it within the code's lifetime, print it only to a trusted channel (the terminal), exactly as you would the bare code. +The code rides the URL **fragment** (`#devframe_otp=…`), which the browser never sends to the server — so the single-use code stays out of access logs and `Referer` headers. `connectDevframe` reads the `devframe_otp` fragment parameter, exchanges it, and removes it from the URL before anything else. Only the short-lived, single-use **code** ever rides the URL — the resulting bearer token is stored, never written back to it. Because the link grants trust to whoever opens it within the code's lifetime, print it only to a trusted channel (the terminal), exactly as you would the bare code. Higher-level integrations can drive their own authentication UI instead: disable the built-in handling with the `otpParam: false` client option, then call the exposed `authenticateWithUrlOtp(rpc)` (consume the code from the URL and exchange it) or `consumeOtpFromUrl()` (read and strip the code) from `devframe/client`. ## Practices for tools built on devframe - **Stay on loopback.** The default bind host is `localhost`. Bind to a routable address only when you intend to, and require authentication when you do. -- **Keep `auth: false` local.** Reach for it only for single-user localhost tools; leave the default in place anywhere a connection could originate elsewhere. +- **Keep `auth: false` local.** Reach for it only for single-user localhost tools; leave the default in place anywhere a connection could originate elsewhere. The hosted bridges (`viteDevBridge`, `@devframes/next`'s handler) gate their side-car by default too — a host that owns the trust boundary another way opts out with `auth: false` explicitly. +- **The MCP route requires an origin.** Unlike the WS transport, the route-based MCP server rejects `Origin`-less requests (a request must carry a loopback or allow-listed `Origin`), so a route-based endpoint isn't reachable by an arbitrary local process — see [MCP](/adapters/mcp). - **Treat tokens as secrets.** Never log the bearer token or the one-time code, and never bake either into build output. - **Authorize every handler.** A registered function is callable by any trusted client. 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, enable `originLock` so a dock token is only honored from its expected origin. +- **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 — the connect-time gate verifies the token against the recorded origin before the connection is trusted. - **Serve encrypted off-machine.** Use `https://`/`wss://` for any surface reachable beyond `localhost`. diff --git a/examples/next-devframe-hub/src/client/devframe/next-devframe-hub.ts b/examples/next-devframe-hub/src/client/devframe/next-devframe-hub.ts index 26b28755..563a8778 100644 --- a/examples/next-devframe-hub/src/client/devframe/next-devframe-hub.ts +++ b/examples/next-devframe-hub/src/client/devframe/next-devframe-hub.ts @@ -264,6 +264,9 @@ export async function nextDevframeHub( }, }) + // Single-user localhost demo: the side-car is reachable only on loopback, so + // it opts out of the gate for a no-friction dev experience. A hub reachable + // beyond localhost should gate (see `docs/guide/security.md`). const started = await startHttpAndWs({ context, host: hostName, diff --git a/examples/vite-devframe-hub/src/vite-devframe-hub.ts b/examples/vite-devframe-hub/src/vite-devframe-hub.ts index 5c1e8750..bfc68590 100644 --- a/examples/vite-devframe-hub/src/vite-devframe-hub.ts +++ b/examples/vite-devframe-hub/src/vite-devframe-hub.ts @@ -175,6 +175,10 @@ export function viteDevframeHub(options: ViteDevframeHubOptions = {}): Plugin { started = await startHttpAndWs({ context, port, + // Single-user localhost demo: the side-car is reachable only on + // loopback, so it opts out of the gate for a no-friction dev + // experience. A hub reachable beyond localhost should gate (see + // `docs/guide/security.md`). auth: false, }) diff --git a/packages/devframe/src/adapters/__tests__/dev.test.ts b/packages/devframe/src/adapters/__tests__/dev.test.ts index ac621278..cd189407 100644 --- a/packages/devframe/src/adapters/__tests__/dev.test.ts +++ b/packages/devframe/src/adapters/__tests__/dev.test.ts @@ -349,7 +349,7 @@ describe('adapters/dev', () => { try { expect(mockedOpen).toHaveBeenCalledTimes(1) const [target] = mockedOpen.mock.calls[0] - expect(target).toBe(`http://localhost:${port}/?devframe_otp=${code}`) + expect(target).toBe(`http://localhost:${port}/#devframe_otp=${code}`) } finally { spy.mockRestore() 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 1480a6af..081f175b 100644 --- a/packages/devframe/src/adapters/mcp/__tests__/mcp-http.test.ts +++ b/packages/devframe/src/adapters/mcp/__tests__/mcp-http.test.ts @@ -60,9 +60,17 @@ describe('mcp adapter (streamable http route)', () => { expect(meta.mcp).toBeUndefined() }) + // 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 { + return new StreamableHTTPClientTransport(new URL(`${started.origin}/__mcp`), { + requestInit: { headers: { origin: started.origin } }, + }) + } + it('establishes a stateful session and lists agent tools', async () => { const started = await boot() - const transport = new StreamableHTTPClientTransport(new URL(`${started.origin}/__mcp`)) + const transport = originTransport(started) const client = new Client({ name: 'test-client', version: '0.0.0' }) try { await client.connect(transport) @@ -88,11 +96,13 @@ describe('mcp adapter (streamable http route)', () => { // Initialize over raw HTTP to capture the issued session id from the // response header (the body is an SSE stream we can discard). + const originHeader = { origin: started.origin } const init = await fetch(url, { method: 'POST', headers: { 'content-type': 'application/json', 'accept': 'application/json, text/event-stream', + ...originHeader, }, body: JSON.stringify({ jsonrpc: '2.0', @@ -108,7 +118,7 @@ describe('mcp adapter (streamable http route)', () => { // DELETE ends the session. const del = await fetch(url, { method: 'DELETE', - headers: { 'mcp-session-id': sessionId! }, + headers: { 'mcp-session-id': sessionId!, ...originHeader }, }) await del.body?.cancel() expect(del.status).toBeLessThan(300) @@ -121,6 +131,7 @@ describe('mcp adapter (streamable http route)', () => { 'content-type': 'application/json', 'accept': 'application/json, text/event-stream', 'mcp-session-id': sessionId!, + ...originHeader, }, body: JSON.stringify({ jsonrpc: '2.0', id: 2, method: 'tools/list' }), }) @@ -128,6 +139,28 @@ describe('mcp adapter (streamable http route)', () => { expect(stale.status).toBe(404) }) + 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`, { + method: 'POST', + headers: { + 'content-type': 'application/json', + 'accept': 'application/json, text/event-stream', + }, + body: JSON.stringify({ + jsonrpc: '2.0', + id: 1, + method: 'initialize', + params: { protocolVersion: '2025-03-26', capabilities: {}, clientInfo: { name: 'x', version: '0' } }, + }), + }) + await res.body?.cancel() + expect(res.status).toBe(403) + }) + it('rejects a disallowed cross-origin request', async () => { const started = await boot() const res = await fetch(`${started.origin}/__mcp`, { diff --git a/packages/devframe/src/adapters/mcp/fetch.ts b/packages/devframe/src/adapters/mcp/fetch.ts index e192c185..c6a6d9de 100644 --- a/packages/devframe/src/adapters/mcp/fetch.ts +++ b/packages/devframe/src/adapters/mcp/fetch.ts @@ -13,7 +13,12 @@ export interface CreateMcpFetchHandlerOptions { exposeSharedState: boolean | ((key: string) => boolean) /** * Origin allow-list beyond the loopback default. `false` disables the - * origin gate entirely. Default: loopback-only (mirrors the WS transport). + * origin gate entirely. Default: loopback-only. + * + * Unlike the WS transport, the MCP route does **not** allow `Origin`-less + * requests: a route-based endpoint is reachable by any local process, so a + * request must carry an `Origin` that passes the gate. Native clients + * (e.g. `devframe connect`) send their loopback origin explicitly. */ allowedOrigins?: readonly string[] | false } @@ -44,9 +49,10 @@ interface McpSession { * and MCP server (built from the shared, live `ctx` via * `buildMcpServerFromContext`), correlated by the `Mcp-Session-Id` header: an * `initialize` POST spins up a session; later requests route to it; a `DELETE` - * (or client disconnect) tears it down. The origin gate applies devframe's - * loopback-default DNS-rebinding protection (identical semantics to the WS - * upgrade's `isAllowedOrigin`). + * (or client disconnect) tears it down. 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. * * @experimental */ @@ -105,12 +111,14 @@ export function createMcpFetchHandler( } async function handle(req: Request): Promise { - // Origin gate — identical semantics to the WS upgrade's `isAllowedOrigin` - // (loopback + `Origin`-less native clients + the configured allow-list). - // This is the endpoint's DNS-rebinding protection. + // 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 + // otherwise be reachable by any local process. A request must carry an + // `Origin` that is loopback or on the configured allow-list. const origin = req.headers.get('origin') ?? undefined - if (allowedOrigins !== false && !isAllowedOrigin(origin, allowedOrigins ?? [])) - return new Response('Forbidden: origin not allowed', { status: 403 }) + if (allowedOrigins !== false && (origin === undefined || !isAllowedOrigin(origin, allowedOrigins ?? []))) + return new Response('Forbidden: origin required', { status: 403 }) const sessionId = req.headers.get('mcp-session-id') ?? undefined let session = sessionId ? sessions.get(sessionId) : undefined diff --git a/packages/devframe/src/cli/connect.ts b/packages/devframe/src/cli/connect.ts index 0bd96a18..bd459c3f 100644 --- a/packages/devframe/src/cli/connect.ts +++ b/packages/devframe/src/cli/connect.ts @@ -250,7 +250,13 @@ async function withInstanceClient( url: string, fn: (client: InstanceType) => Promise, ): Promise { - const transport = new sdk.StreamableHTTPClientTransport(new URL(url)) + // 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 + const transport = new sdk.StreamableHTTPClientTransport( + new URL(url), + { requestInit: { headers: { origin } } }, + ) const client = new sdk.Client({ name: 'devframe-connect', version: '0.0.0' }) await client.connect(transport) try { diff --git a/packages/devframe/src/client/__tests__/otp.test.ts b/packages/devframe/src/client/__tests__/otp.test.ts index f2eba6ad..fcd01e82 100644 --- a/packages/devframe/src/client/__tests__/otp.test.ts +++ b/packages/devframe/src/client/__tests__/otp.test.ts @@ -6,39 +6,54 @@ afterEach(() => { }) describe('otp url helpers', () => { - it('reads the OTP from the page URL query string (default param)', () => { - vi.stubGlobal('location', { search: '?devframe_otp=123456&x=1', href: 'http://localhost:3000/?devframe_otp=123456&x=1' }) + it('reads the OTP from the page URL fragment (default param)', () => { + vi.stubGlobal('location', { hash: '#devframe_otp=123456&x=1', href: 'http://localhost:3000/#devframe_otp=123456&x=1' }) expect(readOtpFromUrl()).toBe('123456') }) it('supports a custom param name', () => { - vi.stubGlobal('location', { search: '?code=999', href: 'http://localhost:3000/?code=999' }) + vi.stubGlobal('location', { hash: '#code=999', href: 'http://localhost:3000/#code=999' }) expect(readOtpFromUrl('code')).toBe('999') }) it('returns undefined when the param is absent and is safe without location', () => { - vi.stubGlobal('location', { search: '?x=1', href: 'http://localhost:3000/?x=1' }) + vi.stubGlobal('location', { hash: '#x=1', href: 'http://localhost:3000/#x=1' }) expect(readOtpFromUrl()).toBeUndefined() vi.stubGlobal('location', undefined) expect(readOtpFromUrl()).toBeUndefined() expect(() => consumeOtpFromUrl()).not.toThrow() }) + it('ignores an OTP left in the query string (fragment-only)', () => { + vi.stubGlobal('location', { hash: '', search: '?devframe_otp=123456', href: 'http://localhost:3000/?devframe_otp=123456' }) + expect(readOtpFromUrl()).toBeUndefined() + }) + it('consume reads then strips the OTP via history.replaceState, keeping other params', () => { const replaceState = vi.fn() - vi.stubGlobal('location', { search: '?devframe_otp=123456&x=1', href: 'http://localhost:3000/?devframe_otp=123456&x=1' }) + vi.stubGlobal('location', { hash: '#devframe_otp=123456&x=1', href: 'http://localhost:3000/#devframe_otp=123456&x=1' }) vi.stubGlobal('history', { state: { a: 1 }, replaceState }) expect(consumeOtpFromUrl()).toBe('123456') expect(replaceState).toHaveBeenCalledTimes(1) const [state, , href] = replaceState.mock.calls[0] expect(state).toEqual({ a: 1 }) - expect(href).toBe('http://localhost:3000/?x=1') + expect(href).toBe('http://localhost:3000/#x=1') + }) + + it('clears the fragment entirely when the OTP was its only param', () => { + const replaceState = vi.fn() + vi.stubGlobal('location', { hash: '#devframe_otp=123456', href: 'http://localhost:3000/#devframe_otp=123456' }) + vi.stubGlobal('history', { state: null, replaceState }) + + expect(consumeOtpFromUrl()).toBe('123456') + const [, , href] = replaceState.mock.calls[0] + expect(href).toBe('http://localhost:3000/') }) it('does not touch the URL when no OTP is present', () => { const replaceState = vi.fn() - vi.stubGlobal('location', { search: '?x=1', href: 'http://localhost:3000/?x=1' }) + vi.stubGlobal('location', { hash: '#x=1', href: 'http://localhost:3000/#x=1' }) vi.stubGlobal('history', { state: null, replaceState }) expect(consumeOtpFromUrl()).toBeUndefined() @@ -48,7 +63,7 @@ describe('otp url helpers', () => { describe('authenticateWithUrlOtp', () => { it('exchanges the OTP via the client and resolves true on success', async () => { - vi.stubGlobal('location', { search: '?devframe_otp=123456', href: 'http://localhost:3000/?devframe_otp=123456' }) + vi.stubGlobal('location', { hash: '#devframe_otp=123456', href: 'http://localhost:3000/#devframe_otp=123456' }) vi.stubGlobal('history', { state: null, replaceState: vi.fn() }) const requestTrustWithCode = vi.fn().mockResolvedValue(true) @@ -59,7 +74,7 @@ describe('authenticateWithUrlOtp', () => { }) it('returns false (and does not exchange) when no OTP is present', async () => { - vi.stubGlobal('location', { search: '', href: 'http://localhost:3000/' }) + vi.stubGlobal('location', { hash: '', href: 'http://localhost:3000/' }) const requestTrustWithCode = vi.fn() const ok = await authenticateWithUrlOtp({ isTrusted: false, requestTrustWithCode }) @@ -70,7 +85,7 @@ describe('authenticateWithUrlOtp', () => { it('skips the exchange but still consumes the OTP when already trusted', async () => { const replaceState = vi.fn() - vi.stubGlobal('location', { search: '?devframe_otp=123456', href: 'http://localhost:3000/?devframe_otp=123456' }) + vi.stubGlobal('location', { hash: '#devframe_otp=123456', href: 'http://localhost:3000/#devframe_otp=123456' }) vi.stubGlobal('history', { state: null, replaceState }) const requestTrustWithCode = vi.fn() diff --git a/packages/devframe/src/client/otp.ts b/packages/devframe/src/client/otp.ts index ee7092fa..a300cac2 100644 --- a/packages/devframe/src/client/otp.ts +++ b/packages/devframe/src/client/otp.ts @@ -2,17 +2,21 @@ import type { DevframeRpcClient } from './rpc' import { DEVFRAME_OTP_URL_PARAM } from 'devframe/constants' // Browser-only helpers for "magic link" authentication: a host prints a URL -// carrying a one-time authentication code (OTP), and the client reads it, -// exchanges it for a token, and removes it from the address bar. Only the -// short-lived, single-use OTP ever rides the URL — never the resulting token. +// carrying a one-time authentication code (OTP) in its fragment, and the client +// reads it, exchanges it for a token, and removes it from the address bar. Only +// the short-lived, single-use OTP ever rides the URL — never the resulting +// token — and it rides the fragment (`#devframe_otp=…`), which the browser +// never sends to the server, so it can't leak into an access log or `Referer`. /** - * Read a one-time authentication code (OTP) from the current page URL's query - * string, without side effects. Returns `undefined` when the parameter is absent. + * Read a one-time authentication code (OTP) from the current page URL's + * fragment, without side effects. Returns `undefined` when the parameter is + * absent. */ export function readOtpFromUrl(param: string = DEVFRAME_OTP_URL_PARAM): string | undefined { try { - return new URLSearchParams(globalThis.location?.search).get(param) || undefined + const hash = globalThis.location?.hash?.replace(/^#/, '') ?? '' + return new URLSearchParams(hash).get(param) || undefined } catch { return undefined @@ -22,19 +26,22 @@ export function readOtpFromUrl(param: string = DEVFRAME_OTP_URL_PARAM): string | function stripParamFromUrl(param: string): void { try { const url = new URL(globalThis.location!.href) - if (!url.searchParams.has(param)) + const fragment = new URLSearchParams(url.hash.replace(/^#/, '')) + if (!fragment.has(param)) return - url.searchParams.delete(param) + fragment.delete(param) + // An empty fragment clears the `#` entirely rather than leaving a bare one. + url.hash = fragment.toString() globalThis.history?.replaceState(globalThis.history.state, '', url.href) } catch {} } /** - * Read the one-time code from the page URL and remove it from the address bar - * (and the current history entry), so the single-use code isn't left in the - * URL, browser history, or a `Referer`. Returns the code, or `undefined` when - * absent. + * Read the one-time code from the page URL fragment and remove it from the + * address bar (and the current history entry), so the single-use code isn't + * left in the URL, browser history, or a `Referer`. Returns the code, or + * `undefined` when absent. */ export function consumeOtpFromUrl(param: string = DEVFRAME_OTP_URL_PARAM): string | undefined { const code = readOtpFromUrl(param) diff --git a/packages/devframe/src/constants.ts b/packages/devframe/src/constants.ts index 90ef82e0..9fc90d74 100644 --- a/packages/devframe/src/constants.ts +++ b/packages/devframe/src/constants.ts @@ -41,11 +41,14 @@ export const DEVFRAME_RPC_DUMP_DIRNAME = '__rpc-dump' export const REMOTE_CONNECTION_KEY = 'devframe-remote-connection' /** - * Page-URL query parameter carrying a one-time authentication code (OTP) for - * "magic link" auth. A host can print a link like `/?devframe_otp=`; - * the client reads the code, exchanges it for a token, and strips the parameter - * from the URL. See `buildOtpAuthUrl` (node) and the `authenticateWithUrlOtp` / - * `consumeOtpFromUrl` client utilities (or `connectDevframe`'s `otpParam`). + * Page-URL **fragment** parameter carrying a one-time authentication code (OTP) + * for "magic link" auth. A host can print a link like + * `/#devframe_otp=`; the client reads the code, exchanges it for a + * token, and strips the parameter from the URL. The code rides the fragment + * (never the query string) so the browser never transmits it to the server, + * keeping it out of access logs and `Referer` headers. See `buildOtpAuthUrl` + * (node) and the `authenticateWithUrlOtp` / `consumeOtpFromUrl` client utilities + * (or `connectDevframe`'s `otpParam`). */ export const DEVFRAME_OTP_URL_PARAM = 'devframe_otp' diff --git a/packages/devframe/src/helpers/__tests__/vite.test.ts b/packages/devframe/src/helpers/__tests__/vite.test.ts index 42267ab6..dae89fa6 100644 --- a/packages/devframe/src/helpers/__tests__/vite.test.ts +++ b/packages/devframe/src/helpers/__tests__/vite.test.ts @@ -1,5 +1,7 @@ import type { DevframeDefinition } from '../../types/devframe' import { Client, StreamableHTTPClientTransport } from '@modelcontextprotocol/client' +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 } from 'vitest' import { viteDevBridge } from '../vite' @@ -61,6 +63,9 @@ describe('viteDevBridge (bridge mode mcp)', () => { bridge = viteDevBridge(defineTestDef(), { devMiddleware: { port, host: '127.0.0.1' }, mcp: true, + // The bridge now gates by default; opt out here so this test can dial + // the WS/MCP side-car directly. + auth: false, }) const server = fakeViteServer() @@ -73,9 +78,11 @@ describe('viteDevBridge (bridge mode mcp)', () => { expect(meta.websocket).toEqual({ port, path: '/__devframe_ws' }) expect(meta.mcp).toEqual({ port, path: '/__mcp' }) - // The advertised endpoint is live: a real MCP client can connect and - // list the agent tools on the side-car origin. - const transport = new StreamableHTTPClientTransport(new URL(`http://127.0.0.1:${port}/__mcp`)) + // The advertised endpoint is live: a real MCP client presenting a loopback + // Origin (required by the route's gate) can connect and list agent tools. + const transport = new StreamableHTTPClientTransport(new URL(`http://127.0.0.1:${port}/__mcp`), { + requestInit: { headers: { origin: `http://127.0.0.1:${port}` } }, + }) const client = new Client({ name: 'test-client', version: '0.0.0' }) try { await client.connect(transport) @@ -91,6 +98,7 @@ describe('viteDevBridge (bridge mode mcp)', () => { const port = await getPort({ port: 19720, host: '127.0.0.1' }) bridge = viteDevBridge(defineTestDef(), { devMiddleware: { port, host: '127.0.0.1' }, + auth: false, }) const server = fakeViteServer() @@ -100,3 +108,44 @@ describe('viteDevBridge (bridge mode mcp)', () => { expect(meta.mcp).toBeUndefined() }) }) + +describe('viteDevBridge (auth default)', () => { + let bridge: ReturnType | undefined + + afterEach(async () => { + await bridge?.closeBundle?.() + bridge = undefined + }) + + /** Handshake result on a fresh, unauthenticated WS connection. */ + async function handshakeIsTrusted(port: number): Promise { + const rpc = createRpcClient({}, { + channel: createWsRpcChannel({ url: `ws://127.0.0.1:${port}/__devframe_ws` }), + }) + try { + const res = await rpc.$call('anonymous:devframe:auth', { authToken: '', ua: 'test', origin: 'http://localhost' }) as { isTrusted: boolean } + return res.isTrusted + } + finally { + rpc.$close?.() + } + } + + it('gates the side-car by default (unset auth → untrusted handshake)', async () => { + const port = await getPort({ port: 19730, host: '127.0.0.1' }) + bridge = viteDevBridge(defineTestDef(), { devMiddleware: { port, host: '127.0.0.1' } }) + await bridge.configureServer(fakeViteServer()) + + // A gated server answers the handshake with `isTrusted: false` until a + // code is exchanged; an ungated (`auth: false`) server auto-trusts. + expect(await handshakeIsTrusted(port)).toBe(false) + }) + + it('opts out when auth: false is passed explicitly (auto-trust handshake)', async () => { + const port = await getPort({ port: 19740, host: '127.0.0.1' }) + bridge = viteDevBridge(defineTestDef(), { devMiddleware: { port, host: '127.0.0.1' }, auth: false }) + await bridge.configureServer(fakeViteServer()) + + expect(await handshakeIsTrusted(port)).toBe(true) + }) +}) diff --git a/packages/devframe/src/helpers/vite.ts b/packages/devframe/src/helpers/vite.ts index 7c59ed38..68f64f4a 100644 --- a/packages/devframe/src/helpers/vite.ts +++ b/packages/devframe/src/helpers/vite.ts @@ -38,16 +38,17 @@ export interface ViteDevBridgeOptions { flags?: Record } /** - * Whether the bridged devframe runs its own auth gate. This is a **hosted** - * adapter — the devframe shares the host app's origin and the host owns - * authentication — so it defaults to `false`: the plugin's own gate never - * fires and its `cli.auth` default is ignored (matching devframe's - * hosted-deployment contract). Pass `true` to force devframe's interactive - * OTP gate on, or a {@link DevframeAuthHandler} to install a custom scheme. - * Only applies in bridge mode (`devMiddleware`); the static-mount mode + * Whether the bridged devframe runs its own auth gate. The side-car RPC + * server is reachable by anything that can open its socket, so it **gates by + * default**: when unset, authentication resolves through `createDevServer` + * (devframe's interactive OTP gate unless the definition's `cli.auth` opts + * out), and the side-car prints its code/link banner to stdout. Pass a + * {@link DevframeAuthHandler} to install a custom scheme, or `false` to opt + * out for a single-user localhost host that owns the trust boundary another + * way. Only applies in bridge mode (`devMiddleware`); the static-mount mode * starts no RPC server. * - * @default false + * @default gated (devframe's interactive OTP, unless `cli.auth` opts out) */ auth?: boolean | DevframeAuthHandler /** @@ -89,11 +90,11 @@ export interface DevframeVitePlugin { * host-served SPA can discover the WS endpoint via * {@link connectDevframe}. * - * As a hosted adapter the bridge defers authentication to the host: its - * side-car RPC server runs with the plugin's own auth gate **off** by - * default (ignoring `def.cli?.auth`), so a plugin mounted this way never - * triggers its standalone OTP prompt. Opt back in per-mount with - * `options.auth` (`true` for devframe's interactive gate, or a handler). + * The side-car RPC server **gates by default** (devframe's interactive OTP + * unless the definition's `cli.auth` opts out), printing its code/link banner + * to stdout, so a bridged devframe isn't silently reachable by anything that + * can open its socket. Pass `options.auth: false` to opt out for a single-user + * localhost host, or a {@link DevframeAuthHandler} for a custom scheme. * * Use bridge mode when integrating with frameworks that own the SPA * (Nuxt, Astro, SolidStart, plain Vite apps). For the all-in-one @@ -136,9 +137,10 @@ export function viteDevBridge(d: DevframeDefinition, options: ViteDevBridgeOptio port, flags: mw.flags, openBrowser: false, - // Hosted adapter: the host owns auth, so the bridged devframe's own - // gate stays off unless the caller explicitly opts back in. - auth: options.auth ?? false, + // Gate by default: an unset `auth` defers to `createDevServer` + // (devframe's interactive OTP unless `cli.auth` opts out) rather than + // leaving the side-car socket ungated. `false` opts out explicitly. + auth: options.auth, mcp: options.mcp, }) } diff --git a/packages/devframe/src/node/auth/state.ts b/packages/devframe/src/node/auth/state.ts index 00a8e099..823a7ed8 100644 --- a/packages/devframe/src/node/auth/state.ts +++ b/packages/devframe/src/node/auth/state.ts @@ -45,14 +45,21 @@ export function refreshTempAuthCode(): string { } /** - * Build a "magic link" authentication URL that embeds a one-time code (OTP) as - * a query parameter. Opening it authenticates the client without typing — print - * it on startup (devframe stays headless, so the host prints its own banner). - * Defaults to the current code; the link is subject to the same TTL. + * Build a "magic link" authentication URL that embeds a one-time code (OTP) in + * the URL **fragment**. Opening it authenticates the client without typing — + * print it on startup (devframe stays headless, so the host prints its own + * banner). Defaults to the current code; the link is subject to the same TTL. + * + * The code rides the fragment (`#devframe_otp=…`), not the query string, so it + * is never sent to the server, written to an access log, or leaked in a + * `Referer` header — the browser client reads it locally (see + * `consumeOtpFromUrl`). Any existing fragment parameters are preserved. */ export function buildOtpAuthUrl(baseUrl: string, code: string = tempAuthCode): string { const url = new URL(baseUrl) - url.searchParams.set(DEVFRAME_OTP_URL_PARAM, code) + const fragment = new URLSearchParams(url.hash.replace(/^#/, '')) + fragment.set(DEVFRAME_OTP_URL_PARAM, code) + url.hash = fragment.toString() return url.href } diff --git a/packages/devframe/src/recipes/__tests__/interactive-auth.test.ts b/packages/devframe/src/recipes/__tests__/interactive-auth.test.ts index 29ffe0a8..84f6994b 100644 --- a/packages/devframe/src/recipes/__tests__/interactive-auth.test.ts +++ b/packages/devframe/src/recipes/__tests__/interactive-auth.test.ts @@ -8,6 +8,7 @@ import { getPort } from 'get-port-please' import { describe, expect, it } from 'vitest' import { getTempAuthCode } from '../../node/auth/state' import { createHostContext } from '../../node/context' +import { getInternalContext } from '../../node/hub-internals/context' import { startHttpAndWs } from '../../node/server' import { createInteractiveAuth } from '../interactive-auth' @@ -150,6 +151,65 @@ describe('recipes/interactive-auth', () => { } }) + describe('remote-dock tokens (onConnect)', () => { + // Minimal stand-in for the crossws `Peer` — `onConnect` only reads + // `request.url` and `request.headers.get('origin')`. + function fakePeer(token: string, origin?: string): any { + return { + request: { + url: `/?devframe_auth_token=${encodeURIComponent(token)}`, + headers: { get: (name: string) => (name.toLowerCase() === 'origin' ? origin ?? null : null) }, + }, + } + } + + it('trusts a valid remote-dock token when originLock is off, regardless of origin', async () => { + const context = await createTestContext() + const auth = createInteractiveAuth(context) + const token = getInternalContext(context).allocateRemoteToken('dock-1', 'http://localhost:5173', false) + + const session = { meta: {} } as any + auth.onConnect(fakePeer(token, 'http://anywhere.example'), session) + expect(session.meta.isTrusted).toBe(true) + expect(session.meta.clientAuthToken).toBe(token) + }) + + it('honors originLock: trusts only when the request Origin matches the dock origin', async () => { + const context = await createTestContext() + const auth = createInteractiveAuth(context) + const dockOrigin = 'http://localhost:5173' + const token = getInternalContext(context).allocateRemoteToken('dock-2', dockOrigin, true) + + const matching = { meta: {} } as any + auth.onConnect(fakePeer(token, dockOrigin), matching) + expect(matching.meta.isTrusted).toBe(true) + + const mismatched = { meta: {} } as any + auth.onConnect(fakePeer(token, 'http://evil.example'), mismatched) + expect(mismatched.meta.isTrusted).toBeUndefined() + + const noOrigin = { meta: {} } as any + auth.onConnect(fakePeer(token), noOrigin) + expect(noOrigin.meta.isTrusted).toBeUndefined() + }) + + it('does not trust an unknown or revoked remote token', async () => { + const context = await createTestContext() + const auth = createInteractiveAuth(context) + const internal = getInternalContext(context) + const token = internal.allocateRemoteToken('dock-3', 'http://localhost:5173', false) + internal.revokeRemoteTokensForDock('dock-3') + + const session = { meta: {} } as any + auth.onConnect(fakePeer(token, 'http://localhost:5173'), session) + expect(session.meta.isTrusted).toBeUndefined() + + const unknown = { meta: {} } as any + auth.onConnect(fakePeer('deadbeef', 'http://localhost:5173'), unknown) + expect(unknown.meta.isTrusted).toBeUndefined() + }) + }) + it('self-revoke: devframe:auth:revoke drops the caller to untrusted and invalidates the token', async () => { const { server, host, port } = await startAuthenticatedServer() diff --git a/packages/devframe/src/recipes/interactive-auth.ts b/packages/devframe/src/recipes/interactive-auth.ts index 4af42d98..213555f2 100644 --- a/packages/devframe/src/recipes/interactive-auth.ts +++ b/packages/devframe/src/recipes/interactive-auth.ts @@ -149,13 +149,21 @@ export function createInteractiveAuth( return !!session.meta.isTrusted } - function onConnect(peer: { request?: { url: string } }, session: DevframeNodeRpcSession): void { + function onConnect( + peer: { request?: { url?: string, headers?: { get?: (name: string) => string | null } } }, + session: DevframeNodeRpcSession, + ): void { let token: string | undefined + let requestOrigin: string | undefined try { const url = new URL(peer.request?.url ?? '', 'http://localhost') token = url.searchParams.get(DEVFRAME_AUTH_TOKEN_QUERY_PARAM) ?? undefined } catch {} + try { + requestOrigin = peer.request?.headers?.get?.('origin') ?? undefined + } + catch {} if (!token) return if (isStaticToken(token)) { @@ -163,7 +171,17 @@ export function createInteractiveAuth( session.meta.isTrusted = true return } - verifyAuthToken(token, session, storage) + // A persisted bearer minted by the code exchange (returning browser). + if (verifyAuthToken(token, session, storage)) + return + // A session-only remote-UI dock token (see `allocateRemoteToken`). These + // never enter the persisted store, so `verifyAuthToken` can't see them — + // check them here so a remote dock's iframe actually authenticates, and so + // `originLock` binds the token to the dock's recorded origin. + if (internal.isRemoteTokenTrusted(token, requestOrigin)) { + session.meta.clientAuthToken = token + session.meta.isTrusted = true + } } function buildOpenUrl(url: string): string { diff --git a/packages/next/src/handler.ts b/packages/next/src/handler.ts index 7856f4d4..4e07434a 100644 --- a/packages/next/src/handler.ts +++ b/packages/next/src/handler.ts @@ -22,9 +22,11 @@ export interface CreateDevframeNextHandlerOptions { /** Flag bag forwarded to `def.setup(ctx, { flags })`. */ flags?: Record /** - * Whether the side-car runs its own auth gate. Defaults to `false`: this is a - * hosted adapter and the Next app owns authentication. Pass `true` for - * devframe's interactive gate or a handler for a custom scheme. + * Whether the side-car runs its own auth gate. **Gates by default** (defers + * to `createDevServer` — devframe's interactive OTP unless the definition's + * `cli.auth` opts out), so the side-car socket isn't silently reachable by + * anything that can open it. Pass `false` to opt out for a single-user + * localhost host, or a handler for a custom scheme. */ auth?: CreateDevServerOptions['auth'] /** Origin the Next app is reachable at, for docks needing an absolute URL. */ @@ -127,7 +129,9 @@ export function createDevframeNextHandler( port, flags: options.flags, openBrowser: false, - auth: options.auth ?? false, + // Gate by default: an unset `auth` defers to `createDevServer` rather + // than leaving the side-car socket ungated. `false` opts out explicitly. + auth: options.auth, mcp: options.mcp, }) const mcpMeta = resolveMcpConnectionMeta(def, options.mcp, port) diff --git a/packages/next/src/host.ts b/packages/next/src/host.ts index ff771ebf..5d5f710c 100644 --- a/packages/next/src/host.ts +++ b/packages/next/src/host.ts @@ -33,7 +33,8 @@ export interface DevframeNextHostMcpOptions { exposeSharedState?: boolean | ((key: string) => boolean) /** * Origin allow-list beyond the loopback default. `false` disables the - * origin gate entirely. + * origin gate entirely. Note the MCP route rejects `Origin`-less requests + * (see `createMcpFetchHandler`). */ allowedOrigins?: readonly string[] | false } diff --git a/packages/next/test/handler.test.ts b/packages/next/test/handler.test.ts index 00147755..0ee5f47c 100644 --- a/packages/next/test/handler.test.ts +++ b/packages/next/test/handler.test.ts @@ -78,12 +78,15 @@ describe('createDevframeNextHandler', () => { } expect(body.mcp).toEqual({ port: body.websocket.port, path: '/__mcp' }) - // The advertised endpoint answers MCP initialize on the side-car origin. - const init = await fetch(`http://127.0.0.1:${body.mcp!.port}${body.mcp!.path}`, { + // The advertised endpoint answers MCP initialize on the side-car origin + // when a loopback Origin (required by the route's gate) is presented. + const sidecarOrigin = `http://127.0.0.1:${body.mcp!.port}` + const init = await fetch(`${sidecarOrigin}${body.mcp!.path}`, { method: 'POST', headers: { 'content-type': 'application/json', 'accept': 'application/json, text/event-stream', + 'origin': sidecarOrigin, }, body: JSON.stringify({ jsonrpc: '2.0', @@ -95,5 +98,22 @@ describe('createDevframeNextHandler', () => { expect(init.status).toBe(200) expect(init.headers.get('mcp-session-id')).toBeTruthy() await init.body?.cancel() + + // Without an Origin header the same request is rejected. + const unauthed = await fetch(`${sidecarOrigin}${body.mcp!.path}`, { + method: 'POST', + headers: { + 'content-type': 'application/json', + 'accept': 'application/json, text/event-stream', + }, + body: JSON.stringify({ + jsonrpc: '2.0', + id: 1, + method: 'initialize', + params: { protocolVersion: '2025-03-26', capabilities: {}, clientInfo: { name: 'x', version: '0' } }, + }), + }) + await unauthed.body?.cancel() + expect(unauthed.status).toBe(403) }) }) diff --git a/skills/devframe/SKILL.md b/skills/devframe/SKILL.md index 5bbc5c80..a73a908f 100644 --- a/skills/devframe/SKILL.md +++ b/skills/devframe/SKILL.md @@ -569,10 +569,11 @@ RPC handlers run with the full privileges of the host process, so the boundary t - **`auth` defaults to `true`** — dev-mode connections must authenticate before calls are accepted. Devframe ships the node primitives (`exchangeTempAuthCode`, `verifyAuthToken` in `devframe/node/auth`); the host adapter (e.g. Vite DevTools) provides the interactive `devframe:anonymous:auth` + `devframe:auth:exchange` handlers and auth UI. - **`auth: false` trusts every reachable connection.** Use it only for single-user `localhost` tools. Never combine it with a non-loopback bind host, a tunnel, or a shared/CI environment. The default bind host is already `localhost`. - **Authentication** exchanges a 6-digit one-time code (shown in the developer's terminal) for a node-issued bearer token via `requestTrustWithCode(code)`. The code is single-use, expires in 5 min, compared in constant time, and rotates after repeated failures — show it only in the terminal, never over the network. -- **Magic-link (optional):** print `buildOtpAuthUrl(origin)` — `/?devframe_otp=`. `connectDevframe` reads the code, exchanges it, and strips it from the URL. Integrations can opt out (`otpParam: false`) and drive it via the exposed `authenticateWithUrlOtp(rpc)` / `consumeOtpFromUrl()` client utilities. Only the single-use code rides the URL, never the bearer; treat the printed link like the code itself. The standalone CLI's `--open` does this automatically via `DevframeAuthHandler.buildOpenUrl` — the launched tab already carries the OTP, no prompt needed. +- **Magic-link (optional):** print `buildOtpAuthUrl(origin)` — `/#devframe_otp=`. The code rides the URL **fragment**, which the browser never sends to the server, so it stays out of access logs and `Referer`. `connectDevframe` reads the code, exchanges it, and strips it from the URL. Integrations can opt out (`otpParam: false`) and drive it via the exposed `authenticateWithUrlOtp(rpc)` / `consumeOtpFromUrl()` client utilities. Only the single-use code rides the URL, never the bearer; treat the printed link like the code itself. The standalone CLI's `--open` does this automatically via `DevframeAuthHandler.buildOpenUrl` — the launched tab already carries the OTP, no prompt needed. - **Tokens are secrets.** The bearer token rides the WS URL (`?devframe_auth_token=…`) — serve over `wss://`/`https://` beyond loopback. Never log the token or code, never bake them into build output. Revoke via `revokeAuthToken(...)`; clients drop to untrusted on `devframe:auth:revoked`. - **Authorize handlers.** Any trusted client can call any registered function — validate inputs, and mark state-changing functions `type: 'destructive'` so MCP/agent clients prompt first. -- **Origin-lock remote docks** (`originLock`) so a dock token is honored only from its expected origin. +- **Origin-lock remote docks** (`originLock`, on by default) so a dock's session token is honored only on a connection whose `Origin` matches the dock — the connect-time gate enforces it. +- **The MCP route requires an Origin.** The route-based MCP server (`cli.mcp`, `viteDevBridge`/Next handler `mcp`, `createMcpFetchHandler`) rejects `Origin`-less requests — a request must carry a loopback (or allow-listed) `Origin`, so it isn't reachable by an arbitrary local process. `devframe connect` sends each instance's own loopback origin automatically. See [Security](https://devfra.me/security) for the full reference.