Skip to content

Commit f2632b2

Browse files
authored
fix(devframe): validate authentication link origins (#325)
1 parent 79b8760 commit f2632b2

18 files changed

Lines changed: 240 additions & 71 deletions

File tree

alias.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ export const alias = {
3434
'devframe/utils/nanoid': r('devframe/src/utils/nanoid.ts'),
3535
'devframe/utils/nostics': r('devframe/src/utils/nostics.ts'),
3636
'devframe/utils/open': r('devframe/src/utils/open.ts'),
37+
'devframe/utils/origin': r('devframe/src/utils/origin.ts'),
3738
'devframe/utils/remote-assets': r('devframe/src/utils/remote-assets.ts'),
3839
'devframe/utils/simple-schema': r('devframe/src/utils/simple-schema.ts'),
3940
'devframe/utils/serve-static': r('devframe/src/utils/serve-static.ts'),

docs/content/1.guide/14.security.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@ Devtools ready — authenticate this browser: http://localhost:3000/#devframe_ot
6767

6868
The code rides the URL **fragment** (`#devframe_otp=…`), which browsers never send to the server, keeping the single-use code out of access logs and `Referer` headers. `connectDevframe` reads it, exchanges it, and strips it from the URL. Because the link grants trust to whoever opens it within the code's lifetime, print it only to a trusted channel (the terminal).
6969

70+
The link points at the **public origin**. A standalone dev server derives it from its own bound address; an owned listener uses that address regardless of any inbound `Host` header. A handler or middleware without an explicit `origin` derives one from a request only when the request's own origin is loopback or exactly matches an `allowedOrigins` entry — a raw inbound authority and forwarded headers are never trusted. Set `origin` explicitly for non-loopback handler deployments (behind a proxy, on a LAN, or on a public host) so the magic link always resolves to the address you intend.
71+
7072
For your own auth UI, disable built-in handling with `otpParam: false`, then call `authenticateWithUrlOtp(rpc)` or `consumeOtpFromUrl()` from `devframe/client`.
7173

7274
## Practices for tools built on devframe

docs/content/2.adapters/1.initiate.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ Fetch handlers only hand over `Request`s, so the host framework binds the RPC so
129129

130130
## Auth
131131

132-
The running devframe **gates by default**. The interactive OTP handler wires automatically, printing its code/magic-link banner once the public origin is known (the first request, or the `origin` option). Pass `auth: false` for single-user localhost, or a `DevframeAuthHandler` for a custom scheme.
132+
The running devframe **gates by default**. The interactive OTP handler wires automatically, printing its code/magic-link banner once the public origin is known — from the `origin` option, or derived from a request whose own origin is loopback or exactly matches an `allowedOrigins` entry. A non-loopback deployment (behind a proxy, on a LAN, on a public host) sets `origin` explicitly so the magic link resolves to the intended address; a raw inbound `Host` header and forwarded headers are never trusted. Pass `auth: false` for single-user localhost, or a `DevframeAuthHandler` for a custom scheme.
133133

134134
## Relation to the other adapters
135135

packages/devframe/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
"./utils/nanoid": "./dist/utils/nanoid.mjs",
5757
"./utils/nostics": "./dist/utils/nostics.mjs",
5858
"./utils/open": "./dist/utils/open.mjs",
59+
"./utils/origin": "./dist/utils/origin.mjs",
5960
"./utils/remote-assets": "./dist/utils/remote-assets.mjs",
6061
"./utils/simple-schema": "./dist/utils/simple-schema.mjs",
6162
"./utils/serve-static": "./dist/utils/serve-static.mjs",

packages/devframe/src/adapters/__tests__/initiate.test.ts

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,77 @@ describe('adapters/handler', () => {
352352
}
353353
})
354354

355+
// The auth-link origin is derived from the served request's URL (the fetch
356+
// handler ignores the `Host` header — that path is `nodeMiddleware`'s), so
357+
// each case just points a request at the origin under test and inspects the
358+
// one-time banner (`console.log`).
359+
async function withBannerSpy(
360+
id: string,
361+
extra: Partial<Parameters<typeof initDevframe>[1]>,
362+
run: (devtools: ReturnType<typeof initDevframe>, spy: ReturnType<typeof vi.spyOn>) => Promise<void>,
363+
): Promise<void> {
364+
const wsPort = await getPort({ host: '127.0.0.1' })
365+
const spy = vi.spyOn(console, 'log').mockImplementation(() => {})
366+
const devtools = initDevframe(defineTestDef(id), { base: `/__${id}/`, host: '127.0.0.1', ws: { port: wsPort }, ...extra })
367+
try {
368+
await devtools.ready
369+
await run(devtools, spy)
370+
}
371+
finally {
372+
spy.mockRestore()
373+
await devtools.close()
374+
}
375+
}
376+
const hit = (devtools: ReturnType<typeof initDevframe>, origin: string): Promise<Response> =>
377+
devtools.handler(new Request(`${origin}/__connection.json`))
378+
379+
it('a hostile first request never becomes the OTP-link origin; a later loopback one does', () =>
380+
withBannerSpy('h-poison', {}, async (devtools, spy) => {
381+
// A forged non-loopback origin is not adopted and prints nothing.
382+
await hit(devtools, 'http://evil.example.com/__h-poison')
383+
expect(spy).not.toHaveBeenCalled()
384+
// A later loopback origin is adopted and prints exactly one OTP link
385+
// (the credential rides the fragment) — the reject never locked it out.
386+
await hit(devtools, 'http://localhost:4321/__h-poison')
387+
expect(spy).toHaveBeenCalledTimes(1)
388+
expect(String(spy.mock.calls[0])).toContain('http://localhost:4321/#devframe_otp=')
389+
expect(String(spy.mock.calls[0])).not.toContain('evil.example.com')
390+
// First-valid origin is pinned: a second loopback request doesn't move it.
391+
await hit(devtools, 'http://127.0.0.1:9999/__h-poison')
392+
expect(spy).toHaveBeenCalledTimes(1)
393+
}))
394+
395+
it('adopts an exactly allow-listed non-loopback origin, but rejects a near-match', () =>
396+
withBannerSpy('h-allow', { allowedOrigins: ['https://tools.example.com'] }, async (devtools, spy) => {
397+
// Prefix/suffix near-matches of the allow-list entry are never adopted.
398+
await hit(devtools, 'https://tools.example.com.evil.com/__h-allow')
399+
await hit(devtools, 'https://evil.tools.example.com/__h-allow')
400+
expect(spy).not.toHaveBeenCalled()
401+
// The exact allow-listed origin is.
402+
await hit(devtools, 'https://tools.example.com/__h-allow')
403+
expect(spy).toHaveBeenCalledTimes(1)
404+
expect(String(spy.mock.calls[0])).toContain('https://tools.example.com/#')
405+
}))
406+
407+
it('an explicit origin wins over any request', () =>
408+
withBannerSpy('h-pinned', { origin: 'https://pinned.example.com' }, async (devtools, spy) => {
409+
// Pinned: the banner points at it before any request, and a forged
410+
// request can't move it.
411+
expect(spy).toHaveBeenCalledTimes(1)
412+
expect(String(spy.mock.calls[0])).toContain('https://pinned.example.com/#')
413+
await hit(devtools, 'http://evil.example.com/__h-pinned')
414+
expect(spy).toHaveBeenCalledTimes(1)
415+
expect(String(spy.mock.calls[0])).not.toContain('evil.example.com')
416+
}))
417+
418+
it('canonicalizes an adopted origin, dropping the default port', () =>
419+
withBannerSpy('h-canon', {}, async (devtools, spy) => {
420+
await hit(devtools, 'http://localhost:80/__h-canon')
421+
expect(spy).toHaveBeenCalledTimes(1)
422+
expect(String(spy.mock.calls[0])).toContain('http://localhost/#')
423+
expect(String(spy.mock.calls[0])).not.toContain('localhost:80')
424+
}))
425+
355426
it('bridge mode: without a distDir only meta + WS are served', async () => {
356427
const wsPort = await getPort({ port: 18160, host: '127.0.0.1' })
357428
const devtools = initDevframe(defineTestDef('handler-bridge'), { base: '/__handler-bridge/', auth: false, ws: { port: wsPort } })

packages/devframe/src/adapters/initiate.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,10 +95,12 @@ export interface InitDevframeOptions {
9595
mcp?: boolean | McpRouteOptions
9696
/**
9797
* Public origin the host app is reachable at (e.g. `http://localhost:3000`),
98-
* or a getter for hosts that resolve it late. When omitted (or the getter
99-
* returns a falsy value), it is derived lazily from the first request the
100-
* handler serves — used for the auth banner's magic link and absolute dock
101-
* URLs.
98+
* or a getter for hosts that resolve it late. Backs the auth banner's magic
99+
* link and absolute dock URLs. When omitted (or the getter returns a falsy
100+
* value), it is derived from a served request — but only when that request's
101+
* own origin is loopback or exactly matches an `allowedOrigins` entry; a raw
102+
* inbound `Host`/URL authority and forwarded headers are never adopted. Set
103+
* this explicitly for a non-loopback deployment (proxy, LAN, public host).
102104
*/
103105
origin?: string | (() => string)
104106
/**

packages/devframe/src/adapters/mcp/fetch.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { DevframeNodeContext } from 'devframe/types'
22
import { createMcpHandler } from '@modelcontextprotocol/server'
3-
import { isAllowedOrigin } from 'devframe/rpc/transports/ws-server'
3+
import { isAllowedOrigin } from 'devframe/utils/origin'
44
import { bridgeListChanged, buildMcpServerFromContext } from './build-server'
55

66
export interface CreateMcpFetchHandlerOptions {

packages/devframe/src/node/instance-shell.ts

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import type { DevframeInstanceRecord, DevframeInstanceRegistration } from './ins
1414
import type { ContextRpcServer } from './rpc-core'
1515
import { createServer } from 'node:http'
1616
import process from 'node:process'
17+
import { validateOriginCandidate } from 'devframe/utils/origin'
1718
import { defineHandler, H3 as H3App, toNodeHandler } from 'h3'
1819
import { joinURL, withLeadingSlash, withoutLeadingSlash, withoutTrailingSlash } from 'ufo'
1920
import { DEVFRAME_SSE_ROUTE, DEVFRAME_WS_ROUTE } from '../constants'
@@ -552,9 +553,11 @@ export function createInstanceShell<TContext extends DevframeNodeContext>(
552553
// listener) — derive it from the first request and let the auth banner
553554
// wait for it, unless the caller pinned one (as a string or a getter).
554555
let derivedOrigin: string | undefined
556+
function explicitOrigin(): string | undefined {
557+
return typeof options.origin === 'function' ? options.origin() : options.origin
558+
}
555559
function currentOrigin(): string | undefined {
556-
const explicit = typeof options.origin === 'function' ? options.origin() : options.origin
557-
return explicit || derivedOrigin
560+
return explicitOrigin() || derivedOrigin
558561
}
559562
let authHandler: DevframeAuthHandler | undefined
560563
let bannerPrinted = false
@@ -602,8 +605,21 @@ export function createInstanceShell<TContext extends DevframeNodeContext>(
602605
}).catch(() => {})
603606
}
604607

605-
function noteOrigin(origin: string): void {
606-
derivedOrigin ??= origin
608+
/**
609+
* Consider a request-derived origin candidate for the advertised public
610+
* origin (which backs the OTP magic link). {@link validateOriginCandidate}
611+
* adopts only a loopback host or an exact `allowedOrigins` match, so a raw
612+
* inbound `Host`/URL authority never redirects the credential-bearing link.
613+
* First-valid-origin wins: an invalid candidate leaves `derivedOrigin` unset
614+
* — printing/registering nothing — so a later valid one can still be adopted.
615+
*/
616+
function noteOrigin(candidate: string): void {
617+
if (derivedOrigin === undefined && !explicitOrigin()) {
618+
const allowed = options.allowedOrigins
619+
const accepted = validateOriginCandidate(candidate, Array.isArray(allowed) ? allowed : undefined)
620+
if (accepted !== undefined)
621+
derivedOrigin = accepted
622+
}
607623
maybePrintBanner()
608624
maybeRegister()
609625
}

packages/devframe/src/rpc/transports/sse-server.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@ import type { RpcFunctionDefinitionAny } from '../types'
33
import type { DevframeNodeRpcSessionMeta, DevframeRpcConnection } from './session'
44
import type { WsOriginRegistry } from './ws-server'
55
import { DEVFRAME_SSE_SESSION_HEADER } from 'devframe/constants'
6+
import { isAllowedOrigin } from 'devframe/utils/origin'
67
import { createRpcWireCodec, peekRpcWireFrame } from '../wire-codec'
78
import { createRpcSessionMeta } from './session'
8-
import { isAllowedOrigin } from './ws-server'
99

1010
export interface SseRpcTransportOptions {
1111
/**

packages/devframe/src/rpc/transports/ws-server.ts

Lines changed: 8 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { createServer as createHttpsServer } from 'node:https'
1313
import crossws from 'crossws/adapters/node'
1414
import { DEVFRAME_VIEWER_ORIGIN_QUERY_PARAM, DEVFRAME_VIEWER_ORIGIN_TOKEN_QUERY_PARAM } from 'devframe/constants'
1515
import { randomToken, timingSafeEqual } from 'devframe/utils/crypto-token'
16+
import { isAllowedOrigin } from 'devframe/utils/origin'
1617
import { createRpcWireCodec } from '../wire-codec'
1718
import { createRpcSessionMeta } from './session'
1819

@@ -226,62 +227,13 @@ function pathMatches(a: string, b: string): boolean {
226227
return strip(a) === strip(b)
227228
}
228229

229-
/**
230-
* Whether `hostname` names a loopback host: `localhost` (or any `*.localhost`
231-
* subdomain), the IPv6 loopback `::1`, or an IPv4 literal inside the
232-
* `127.0.0.0/8` loopback block.
233-
*
234-
* The IPv4 case is matched **structurally** — the whole hostname must be a
235-
* canonical dotted-decimal IPv4 literal whose first octet is `127`. A bare
236-
* `startsWith('127.')` prefix check would also accept an attacker-controlled
237-
* DNS name that merely *begins* with `127.` (`127.attacker.example`,
238-
* `127.0.0.1.attacker.example`), letting a cross-origin browser page defeat
239-
* the loopback origin gate that guards the RPC/MCP surface (a DNS-rebinding /
240-
* cross-site WebSocket-hijacking bypass). Requiring a real IPv4 literal keeps
241-
* genuine loopback addresses (`127.0.0.1`, `127.5.5.5`) allowed while rejecting
242-
* those DNS names.
243-
*/
244-
export function isLoopbackHostname(hostname: string): boolean {
245-
const h = hostname.replace(/^\[|\]$/g, '') // strip IPv6 brackets
246-
if (h === 'localhost' || h.endsWith('.localhost') || h === '::1')
247-
return true
248-
return isLoopbackIPv4(h)
249-
}
250-
251-
/** A canonical dotted-decimal IPv4 literal in `127.0.0.0/8`. */
252-
function isLoopbackIPv4(hostname: string): boolean {
253-
const octets = hostname.split('.')
254-
if (octets.length !== 4 || !octets.every(isDecimalOctet))
255-
return false
256-
return Number(octets[0]) === 127
257-
}
258-
259-
/** A single canonical IPv4 octet: 1–3 digits, no leading zero, value 0–255. */
260-
function isDecimalOctet(part: string): boolean {
261-
if (!/^\d{1,3}$/.test(part) || (part.length > 1 && part[0] === '0'))
262-
return false
263-
return Number(part) <= 255
264-
}
265-
266-
/**
267-
* Default origin policy for a localhost dev tool: allow requests with no
268-
* `Origin` header (native, non-browser clients), allow any loopback origin
269-
* (so cross-port localhost dev setups keep working), and allow explicitly
270-
* configured origins. Everything else — a real remote page in the dev's
271-
* browser — is rejected.
272-
*/
273-
export function isAllowedOrigin(origin: string | undefined, allowedOrigins: readonly string[]): boolean {
274-
if (!origin)
275-
return true
276-
if (allowedOrigins.includes(origin))
277-
return true
278-
try {
279-
return isLoopbackHostname(new URL(origin).hostname)
280-
}
281-
catch {
282-
return false
283-
}
284-
}
230+
// The loopback / origin predicates live in the dependency-free
231+
// `devframe/utils/origin` module so consumers that only need one check (e.g.
232+
// the instance shell's auth-link origin validation) don't import this whole
233+
// `crossws`-carrying transport. Re-exported here to keep the historical
234+
// `devframe/rpc/transports/ws-server` import path for `isAllowedOrigin` /
235+
// `isLoopbackHostname` intact.
236+
export { isAllowedOrigin, isLoopbackHostname } from 'devframe/utils/origin'
285237

286238
function isWsOriginRegistry(
287239
value: readonly string[] | WsOriginRegistry | false | undefined,

0 commit comments

Comments
 (0)