Skip to content

Commit 08d1b80

Browse files
committed
fix(devframe): validate authentication link origins
1 parent 1c9f789 commit 08d1b80

6 files changed

Lines changed: 210 additions & 12 deletions

File tree

‎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/src/adapters/__tests__/initiate.test.ts‎

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

355+
it('a hostile first request never becomes the OTP-link origin; a later loopback one does', async () => {
356+
const wsPort = await getPort({ port: 18180, host: '127.0.0.1' })
357+
const spy = vi.spyOn(console, 'log').mockImplementation(() => {})
358+
const devtools = initDevframe(defineTestDef('handler-poison'), { base: '/__handler-poison/', host: '127.0.0.1', ws: { port: wsPort } })
359+
360+
try {
361+
await devtools.ready
362+
// A first request forging a non-loopback Host must not print, adopt, or
363+
// register that authority as the magic-link origin.
364+
await devtools.handler(new Request('http://evil.example.com/__handler-poison/__connection.json', {
365+
headers: { host: 'evil.example.com' },
366+
}))
367+
expect(spy).not.toHaveBeenCalled()
368+
369+
// A later loopback request is trusted, adopted, and prints exactly one
370+
// link pointing at that origin — the rejected candidate never locked it
371+
// out.
372+
await devtools.handler(new Request('http://localhost:4321/__handler-poison/__connection.json'))
373+
expect(spy).toHaveBeenCalledTimes(1)
374+
const link = String(spy.mock.calls[0])
375+
expect(link).toContain('http://localhost:4321/#')
376+
expect(link).not.toContain('evil.example.com')
377+
// The credential rides the fragment; assert only its presence.
378+
expect(link).toContain('#devframe_otp=')
379+
380+
// The first-valid origin is pinned: a second loopback request neither
381+
// re-prints nor moves it.
382+
await devtools.handler(new Request('http://127.0.0.1:9999/__handler-poison/__connection.json'))
383+
expect(spy).toHaveBeenCalledTimes(1)
384+
}
385+
finally {
386+
spy.mockRestore()
387+
await devtools.close()
388+
}
389+
})
390+
391+
it('adopts an exactly allow-listed non-loopback origin, but rejects a prefix/suffix near-match', async () => {
392+
const wsPort = await getPort({ port: 18181, host: '127.0.0.1' })
393+
const spy = vi.spyOn(console, 'log').mockImplementation(() => {})
394+
const devtools = initDevframe(defineTestDef('handler-allow'), {
395+
base: '/__handler-allow/',
396+
host: '127.0.0.1',
397+
ws: { port: wsPort },
398+
allowedOrigins: ['https://tools.example.com'],
399+
})
400+
401+
try {
402+
await devtools.ready
403+
// Only prefix/suffix-matches the allow-list entry — never adopted.
404+
await devtools.handler(new Request('https://tools.example.com.evil.com/__handler-allow/__connection.json', {
405+
headers: { host: 'tools.example.com.evil.com' },
406+
}))
407+
await devtools.handler(new Request('https://evil.tools.example.com/__handler-allow/__connection.json', {
408+
headers: { host: 'evil.tools.example.com' },
409+
}))
410+
expect(spy).not.toHaveBeenCalled()
411+
412+
// The exact allow-listed origin is adopted.
413+
await devtools.handler(new Request('https://tools.example.com/__handler-allow/__connection.json', {
414+
headers: { host: 'tools.example.com' },
415+
}))
416+
expect(spy).toHaveBeenCalledTimes(1)
417+
expect(String(spy.mock.calls[0])).toContain('https://tools.example.com/#')
418+
}
419+
finally {
420+
spy.mockRestore()
421+
await devtools.close()
422+
}
423+
})
424+
425+
it('an explicit origin wins regardless of the inbound Host', async () => {
426+
const wsPort = await getPort({ port: 18182, host: '127.0.0.1' })
427+
const spy = vi.spyOn(console, 'log').mockImplementation(() => {})
428+
const devtools = initDevframe(defineTestDef('handler-pinned'), {
429+
base: '/__handler-pinned/',
430+
host: '127.0.0.1',
431+
ws: { port: wsPort },
432+
origin: 'https://pinned.example.com',
433+
})
434+
435+
try {
436+
await devtools.ready
437+
// A pinned origin needs no request: the banner points at it from the
438+
// start, ignoring whatever Host a request forges.
439+
expect(spy).toHaveBeenCalledTimes(1)
440+
expect(String(spy.mock.calls[0])).toContain('https://pinned.example.com/#')
441+
442+
await devtools.handler(new Request('http://evil.example.com/__handler-pinned/__connection.json', {
443+
headers: { host: 'evil.example.com' },
444+
}))
445+
expect(spy).toHaveBeenCalledTimes(1)
446+
expect(String(spy.mock.calls[0])).toContain('https://pinned.example.com/#')
447+
expect(String(spy.mock.calls[0])).not.toContain('evil.example.com')
448+
}
449+
finally {
450+
spy.mockRestore()
451+
await devtools.close()
452+
}
453+
})
454+
455+
it('canonicalizes the protocol and default port of an adopted origin', async () => {
456+
const wsPort = await getPort({ port: 18183, host: '127.0.0.1' })
457+
const spy = vi.spyOn(console, 'log').mockImplementation(() => {})
458+
const devtools = initDevframe(defineTestDef('handler-canon'), { base: '/__handler-canon/', host: '127.0.0.1', ws: { port: wsPort } })
459+
460+
try {
461+
await devtools.ready
462+
// An explicit :80 default port canonicalizes away in the advertised
463+
// origin, so the link carries no redundant port.
464+
await devtools.handler(new Request('http://localhost:80/__handler-canon/__connection.json', {
465+
headers: { host: 'localhost:80' },
466+
}))
467+
expect(spy).toHaveBeenCalledTimes(1)
468+
expect(String(spy.mock.calls[0])).toContain('http://localhost/#')
469+
expect(String(spy.mock.calls[0])).not.toContain('localhost:80')
470+
}
471+
finally {
472+
spy.mockRestore()
473+
await devtools.close()
474+
}
475+
})
476+
355477
it('bridge mode: without a distDir only meta + WS are served', async () => {
356478
const wsPort = await getPort({ port: 18160, host: '127.0.0.1' })
357479
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/node/instance-shell.ts‎

Lines changed: 78 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -552,9 +552,11 @@ export function createInstanceShell<TContext extends DevframeNodeContext>(
552552
// listener) — derive it from the first request and let the auth banner
553553
// wait for it, unless the caller pinned one (as a string or a getter).
554554
let derivedOrigin: string | undefined
555+
function explicitOrigin(): string | undefined {
556+
return typeof options.origin === 'function' ? options.origin() : options.origin
557+
}
555558
function currentOrigin(): string | undefined {
556-
const explicit = typeof options.origin === 'function' ? options.origin() : options.origin
557-
return explicit || derivedOrigin
559+
return explicitOrigin() || derivedOrigin
558560
}
559561
let authHandler: DevframeAuthHandler | undefined
560562
let bannerPrinted = false
@@ -602,8 +604,78 @@ export function createInstanceShell<TContext extends DevframeNodeContext>(
602604
}).catch(() => {})
603605
}
604606

605-
function noteOrigin(origin: string): void {
606-
derivedOrigin ??= origin
607+
// `isLoopbackHostname` lives in the WS transport module (whose top-level
608+
// `crossws` import instance-shell keeps out of its own static graph), so it
609+
// is pulled in lazily and cached the first time a candidate needs checking.
610+
// An explicit or already-derived origin short-circuits before this loads, so
611+
// the common cases (a pinned dev-server origin, every request after the
612+
// first valid one) never touch the transport module.
613+
let loopbackCheck: ((hostname: string) => boolean) | undefined
614+
async function ensureLoopbackCheck(): Promise<(hostname: string) => boolean> {
615+
if (!loopbackCheck) {
616+
const mod = await import('devframe/rpc/transports/ws-server')
617+
loopbackCheck = mod.isLoopbackHostname
618+
}
619+
return loopbackCheck
620+
}
621+
622+
/**
623+
* Canonicalize a request-derived origin candidate and decide whether it may
624+
* back the advertised origin. That origin becomes the destination of the OTP
625+
* magic link, so a raw inbound authority is never trusted: a candidate is
626+
* adopted only when its parsed hostname is loopback, or when its canonical
627+
* origin exactly matches a configured `allowedOrigins` entry. A dynamic
628+
* `WsOriginRegistry` or a disabled gate (`false`) offers no static list to
629+
* match, so non-loopback adoption stays off there — those deployments supply
630+
* an explicit `origin`. Returns the canonical origin, or `undefined` to
631+
* reject (credentials, a path, a query, a fragment, a malformed port, a
632+
* non-HTTP(S) scheme, or an untrusted host). Forwarded headers are never
633+
* consulted.
634+
*/
635+
function validateOriginCandidate(
636+
candidate: string,
637+
isLoopback: (hostname: string) => boolean,
638+
): string | undefined {
639+
let url: URL
640+
try {
641+
url = new URL(candidate)
642+
}
643+
catch {
644+
return undefined
645+
}
646+
if (url.protocol !== 'http:' && url.protocol !== 'https:')
647+
return undefined
648+
// A canonical origin carries no credentials, path, query, or fragment; any
649+
// of these means the candidate was a full or poisoned URL, not a bare
650+
// authority safe to advertise.
651+
if (url.username || url.password || url.search || url.hash)
652+
return undefined
653+
if (url.pathname !== '/' && url.pathname !== '')
654+
return undefined
655+
const canonical = url.origin
656+
if (canonical === 'null')
657+
return undefined
658+
if (isLoopback(url.hostname))
659+
return canonical
660+
const allowed = options.allowedOrigins
661+
if (Array.isArray(allowed) && allowed.includes(canonical))
662+
return canonical
663+
return undefined
664+
}
665+
666+
/**
667+
* Consider a request-derived origin candidate. Keeps the first-valid-origin
668+
* behavior: an invalid candidate is ignored without setting `derivedOrigin`,
669+
* so it neither prints a banner nor registers a poisoned origin, and a later
670+
* valid candidate can still be adopted. Silent by design — a diagnostic here
671+
* would let an unauthenticated request amplify log noise.
672+
*/
673+
async function noteOrigin(candidate: string): Promise<void> {
674+
if (derivedOrigin === undefined && !explicitOrigin()) {
675+
const accepted = validateOriginCandidate(candidate, await ensureLoopbackCheck())
676+
if (accepted !== undefined)
677+
derivedOrigin = accepted
678+
}
607679
maybePrintBanner()
608680
maybeRegister()
609681
}
@@ -854,7 +926,7 @@ export function createInstanceShell<TContext extends DevframeNodeContext>(
854926

855927
async function handleRequest(request: Request): Promise<Response> {
856928
await initPromise
857-
noteOrigin(new URL(request.url).origin)
929+
await noteOrigin(new URL(request.url).origin)
858930
const response = await app.fetch(request)
859931
// Normalize a miss to a bare 404: an unmounted path falls through to
860932
// h3's default JSON-error handler, but for an asset host a body-less
@@ -885,7 +957,7 @@ export function createInstanceShell<TContext extends DevframeNodeContext>(
885957
const host = req.headers.host
886958
if (host) {
887959
const encrypted = (req.socket as { encrypted?: boolean }).encrypted
888-
noteOrigin(`${encrypted ? 'https' : 'http'}://${host}`)
960+
await noteOrigin(`${encrypted ? 'https' : 'http'}://${host}`)
889961
}
890962
if (!nodeHandler) {
891963
const { toNodeHandler } = await import('h3/node')

‎plans/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ Generated by the improve skill on 2026-09-01 at commit `2d978f84`. Execute in th
1111
| 003 | Enforce shared-state exposure policy on direct MCP reads | P1 | S | 002 | TODO |
1212
| 004 | Contain remote asset materialization | P1 | S | - | TODO |
1313
| 005 | Block Data Inspector prototype-chain writes | P1 | S | - | TODO |
14-
| 006 | Validate request-derived authentication-link origins | P1 | M | - | TODO |
14+
| 006 | Validate request-derived authentication-link origins | P1 | M | - | DONE |
1515
| 007 | Reject pre-existing symlink escapes from filesystem roots | P2 | M | - | TODO |
1616

1717
Status values: TODO | IN PROGRESS | DONE | BLOCKED (with reason) | REJECTED (with rationale)

0 commit comments

Comments
 (0)