From d1832a782968edf7d9ab126773270a52692bb7d6 Mon Sep 17 00:00:00 2001 From: Aaron Elijah Mars Date: Tue, 26 May 2026 09:05:37 -0400 Subject: [PATCH] fix(vis-server): add DNS-rebinding Host guard (no-token mode) Re-lands the Host-header guard on top of the agent-core protocol rewrite, which reorganized the vis server and dropped the earlier version of this change. The vis server binds to loopback and serves no auth token by default, so without a Host check any page the user visits could rebind its hostname to 127.0.0.1 and read or delete local agent sessions cross-origin. Design: the guard is installed only when no auth token is configured. A non-loopback bind already requires VIS_AUTH_TOKEN (config.ts), and a token defeats rebinding on its own (the attacker's page cannot read it cross-origin), so a Host allow-list in token mode would only break legitimate LAN / wildcard access without adding security. In the no-token loopback case the guard allows loopback names, the configured bind host, and VIS_ALLOWED_HOSTS entries; everything else gets 403 FORBIDDEN_HOST. Hardening folded in from review of the prior iteration: - isLoopbackHost matched any host starting with `127.`, so `127.0.0.1.nip.io` / `127.evil.com` were treated as loopback and bypassed the guard. Now a strict 127.0.0.0/8 dotted-quad match. - hostnameFromAuthority split unbracketed authorities at the first colon, truncating bare IPv6 literals (`2001:db8::10` -> `2001`); and a malformed bracketed authority like `[::1]evil.com` normalized to the loopback `::1`. Both are fixed: bare IPv6 literals are returned whole, and trailing junk after `]` makes the authority fail every allow check. - `*.localhost` (RFC 6761) is now treated as loopback instead of 403'd. Adds test/host-guard.test.ts covering authority parsing, the loopback matcher (including the 127.-prefix and bracket bypasses), the guard's allow/deny behavior, and the no-token-vs-token wiring in createApp. Co-Authored-By: Claude Opus 4.7 (1M context) --- apps/vis/server/src/app.ts | 22 ++++- apps/vis/server/src/config.ts | 9 +- apps/vis/server/src/host-guard.ts | 120 ++++++++++++++++++++++++ apps/vis/server/src/index.ts | 2 +- apps/vis/server/test/host-guard.test.ts | 110 ++++++++++++++++++++++ 5 files changed, 260 insertions(+), 3 deletions(-) create mode 100644 apps/vis/server/src/host-guard.ts create mode 100644 apps/vis/server/test/host-guard.test.ts diff --git a/apps/vis/server/src/app.ts b/apps/vis/server/src/app.ts index e2670b1..d49e88c 100644 --- a/apps/vis/server/src/app.ts +++ b/apps/vis/server/src/app.ts @@ -4,6 +4,8 @@ import { join, resolve } from 'node:path'; import { Hono } from 'hono'; +import { resolveHost } from './config'; +import { hostGuard } from './host-guard'; import { contextRoute } from './routes/context'; import { sessionDetailRoute } from './routes/session-detail'; import { sessionsRoute } from './routes/sessions'; @@ -51,6 +53,11 @@ function mimeFor(path: string): string { export interface CreateAppOptions { readonly authToken?: string; + /** Host the server is bound to; used by the no-token DNS-rebinding guard. + * Defaults to resolveHost(). */ + readonly host?: string; + /** Raw comma-separated VIS_ALLOWED_HOSTS value for the no-token guard. */ + readonly allowedHosts?: string; } function bearerToken(value: string | undefined): string | null { @@ -69,9 +76,22 @@ function tokenMatches(actual: string, expected: string): boolean { export async function createApp(options: CreateAppOptions = {}): Promise { const app = new Hono(); + const authToken = options.authToken; + + // DNS-rebinding guard. Only meaningful in the no-token loopback mode: when a + // token is configured (required for any non-loopback bind), the token is the + // access control and a rebinding attacker cannot read it cross-origin, so a + // Host allow-list would only break legitimate LAN / wildcard access. Mounted + // before the routes so it covers /api/* and the static fallback alike. + if (authToken === undefined || authToken.length === 0) { + app.use( + '*', + hostGuard({ bindHost: options.host ?? resolveHost(), allowedHosts: options.allowedHosts }), + ); + } + // /api/* handlers. const api = new Hono(); - const authToken = options.authToken; if (authToken !== undefined && authToken.length > 0) { api.use('*', async (c, next) => { const token = bearerToken(c.req.header('authorization')); diff --git a/apps/vis/server/src/config.ts b/apps/vis/server/src/config.ts index 1a719f3..c8b151c 100644 --- a/apps/vis/server/src/config.ts +++ b/apps/vis/server/src/config.ts @@ -29,13 +29,20 @@ export function resolveHost(): string { return host !== undefined && host.length > 0 ? host : '127.0.0.1'; } +/** Strict dotted-quad match for the 127.0.0.0/8 loopback range. Anchored so a + * hostname that merely *starts with* `127.` (e.g. `127.0.0.1.nip.io`) is not + * mistaken for a loopback address. */ +const LOOPBACK_IPV4 = /^127\.(?:25[0-5]|2[0-4]\d|1?\d?\d)(?:\.(?:25[0-5]|2[0-4]\d|1?\d?\d)){2}$/u; + export function isLoopbackHost(host: string): boolean { const normalized = host.trim().toLowerCase().replaceAll('[', '').replaceAll(']', ''); return ( normalized === 'localhost' || + // RFC 6761: `localhost.` and any `*.localhost` name resolves to loopback. + normalized.endsWith('.localhost') || normalized === '::1' || normalized === '0:0:0:0:0:0:0:1' || - normalized.startsWith('127.') + LOOPBACK_IPV4.test(normalized) ); } diff --git a/apps/vis/server/src/host-guard.ts b/apps/vis/server/src/host-guard.ts new file mode 100644 index 0000000..2891af3 --- /dev/null +++ b/apps/vis/server/src/host-guard.ts @@ -0,0 +1,120 @@ +import type { Context, Next } from 'hono'; + +import { isLoopbackHost } from './config'; + +/** Extract the hostname (without port) from a Host header or URL authority. + * Handles IPv6 bracket form (`[::1]:3001`), bare IPv6 literals (`2001:db8::1`), + * and `host:port`. Returns null for empty/missing input. + * + * A malformed authority is returned verbatim (lowercased) rather than coerced, + * so it fails every allow check instead of being silently reduced to a value + * that looks allowed (e.g. `[::1]evil.com` must not become `::1`). */ +export function hostnameFromAuthority(authority: string | undefined | null): string | null { + if (authority === undefined || authority === null) return null; + const trimmed = authority.trim(); + if (trimmed.length === 0) return null; + if (trimmed.startsWith('[')) { + const end = trimmed.indexOf(']'); + if (end < 0) return trimmed.toLowerCase(); // unterminated bracket → cannot match + const rest = trimmed.slice(end + 1); + // After the closing bracket only an empty string or `:port` is valid; any + // other trailing text means the authority is malformed — don't let the + // bracketed prefix masquerade as the real host. + if (rest.length > 0 && !/^:\d+$/u.test(rest)) return trimmed.toLowerCase(); + return trimmed.slice(1, end).toLowerCase(); + } + const firstColon = trimmed.indexOf(':'); + if (firstColon < 0) return trimmed.toLowerCase(); + // An unbracketed authority with more than one colon is a bare IPv6 literal + // with no port (a port requires bracket form), so the whole string is the + // host. A single colon is an ordinary `host:port`. + if (trimmed.includes(':', firstColon + 1)) return trimmed.toLowerCase(); + return trimmed.slice(0, firstColon).toLowerCase(); +} + +/** Wildcard bind addresses (`0.0.0.0`, `::`) mean "every interface" and never + * appear as a concrete client's Host, so they cannot serve as a match target. */ +function isWildcardHost(host: string): boolean { + return host === '0.0.0.0' || host === '::' || host === '0:0:0:0:0:0:0:0'; +} + +function parseAllowedHosts(raw: string | undefined): ReadonlySet { + const set = new Set(); + if (raw === undefined) return set; + for (const part of raw.split(',')) { + const h = hostnameFromAuthority(part); + if (h !== null) set.add(h); + } + return set; +} + +/** Decide whether a single request hostname is one we expect to serve. Loopback + * names are always allowed (a DNS-rebinding attacker cannot forge a loopback + * Host from a browser). Otherwise the hostname must match the configured bind + * host or an explicit allow-list entry. */ +function hostnameAllowed( + hostname: string, + normalizedBindHost: string, + allowedHosts: ReadonlySet, +): boolean { + if (isLoopbackHost(hostname)) return true; + if (allowedHosts.has(hostname)) return true; + if (normalizedBindHost.length > 0 && hostname === normalizedBindHost) return true; + return false; +} + +export interface HostGuardOptions { + /** The host the server is bound to (e.g. resolveHost()). */ + readonly bindHost: string; + /** Raw comma-separated VIS_ALLOWED_HOSTS value, if any. */ + readonly allowedHosts?: string; +} + +/** Hono middleware that rejects requests whose Host header / URL authority is + * neither a loopback name nor an explicitly allowed host. + * + * This is a DNS-rebinding defense for the *no-token loopback* mode: the vis + * server binds to loopback by default and serves no auth token there, so + * without this check any web page the user visits could rebind its own + * hostname to 127.0.0.1 and read or delete the user's local agent sessions + * cross-origin. A browser performing that attack still sends the *original* + * attacker hostname in the Host header, which will not match a loopback name + * or the configured bind host. When an auth token is configured the token is + * the access control — a rebinding attacker cannot read it cross-origin — so + * this guard is not installed in that mode (see createApp), which keeps LAN / + * wildcard binds working without listing every reachable host. */ +export function hostGuard(options: HostGuardOptions) { + const allowedHosts = parseAllowedHosts(options.allowedHosts); + const bindHost = hostnameFromAuthority(options.bindHost) ?? ''; + const normalizedBindHost = isWildcardHost(bindHost) ? '' : bindHost; + + return async (c: Context, next: Next): Promise => { + // Collect every hostname the request claims. In @hono/node-server the URL + // authority is built from the client's Host header, so both reflect what a + // rebinding attacker controls; require all present hostnames to be allowed. + const hostnames: string[] = []; + try { + const fromUrl = hostnameFromAuthority(new URL(c.req.url).host); + if (fromUrl !== null) hostnames.push(fromUrl); + } catch { + // ignore unparseable URL; fall back to the Host header below + } + const fromHeader = hostnameFromAuthority(c.req.header('host')); + if (fromHeader !== null) hostnames.push(fromHeader); + + const ok = + hostnames.length > 0 && + hostnames.every((h) => hostnameAllowed(h, normalizedBindHost, allowedHosts)); + if (!ok) { + return c.json( + { + error: + 'forbidden host: request Host is not loopback, the configured bind host, or in VIS_ALLOWED_HOSTS', + code: 'FORBIDDEN_HOST', + }, + 403, + ); + } + await next(); + }; +} diff --git a/apps/vis/server/src/index.ts b/apps/vis/server/src/index.ts index e97d660..90875f0 100644 --- a/apps/vis/server/src/index.ts +++ b/apps/vis/server/src/index.ts @@ -7,7 +7,7 @@ import { formatStartupBanner } from './startup-banner'; async function main(): Promise { const host = resolveHost(); const authToken = resolveVisAuthToken(host); - const app = await createApp({ authToken }); + const app = await createApp({ authToken, host, allowedHosts: process.env['VIS_ALLOWED_HOSTS'] }); const port = resolvePort(); serve({ fetch: app.fetch, hostname: host, port }, (info) => { // Startup banner. diff --git a/apps/vis/server/test/host-guard.test.ts b/apps/vis/server/test/host-guard.test.ts new file mode 100644 index 0000000..ac367d0 --- /dev/null +++ b/apps/vis/server/test/host-guard.test.ts @@ -0,0 +1,110 @@ +// apps/vis/server/test/host-guard.test.ts +import { Hono } from 'hono'; +import { afterEach, describe, expect, it } from 'vitest'; + +import { createApp } from '../src/app'; +import { isLoopbackHost } from '../src/config'; +import { hostGuard, hostnameFromAuthority } from '../src/host-guard'; + +/** Minimal app exercising only the guard, so assertions don't depend on the + * session routes or KIMI_CODE_HOME. */ +function guarded(options: { bindHost: string; allowedHosts?: string }): Hono { + const app = new Hono(); + app.use('*', hostGuard(options)); + app.get('/api/ping', (c) => c.json({ ok: true })); + return app; +} + +describe('hostnameFromAuthority', () => { + it('parses host:port and bare hostnames', () => { + expect(hostnameFromAuthority('example.com:3001')).toBe('example.com'); + expect(hostnameFromAuthority('127.0.0.1')).toBe('127.0.0.1'); + expect(hostnameFromAuthority(' Example.COM ')).toBe('example.com'); + expect(hostnameFromAuthority('')).toBeNull(); + expect(hostnameFromAuthority(undefined)).toBeNull(); + }); + + it('does not truncate bare IPv6 literals at the first colon', () => { + expect(hostnameFromAuthority('2001:db8::10')).toBe('2001:db8::10'); + expect(hostnameFromAuthority('[2001:db8::10]:3001')).toBe('2001:db8::10'); + expect(hostnameFromAuthority('[::1]')).toBe('::1'); + }); + + it('does not let trailing junk after a bracket masquerade as the host', () => { + // `[::1]evil.com` must NOT normalize to the loopback `::1`. + const parsed = hostnameFromAuthority('[::1]evil.com'); + expect(parsed).not.toBe('::1'); + expect(isLoopbackHost(parsed ?? '')).toBe(false); + }); +}); + +describe('isLoopbackHost', () => { + it('accepts loopback names and the 127.0.0.0/8 range', () => { + for (const h of ['localhost', 'tenant.localhost', '127.0.0.1', '127.0.0.2', '::1', '[::1]']) { + expect(isLoopbackHost(h), h).toBe(true); + } + }); + + it('rejects hostnames that merely start with 127.', () => { + for (const h of ['127.0.0.1.nip.io', '127.evil.com', '1270.0.0.1', '127.0.0.256']) { + expect(isLoopbackHost(h), h).toBe(false); + } + }); +}); + +describe('hostGuard (DNS-rebinding defense)', () => { + it('rejects a rebound non-loopback Host', async () => { + const app = guarded({ bindHost: '127.0.0.1' }); + const res = await app.request('http://attacker.example/api/ping'); + expect(res.status).toBe(403); + await expect(res.json()).resolves.toMatchObject({ code: 'FORBIDDEN_HOST' }); + }); + + it('allows loopback and *.localhost Host values', async () => { + const app = guarded({ bindHost: '127.0.0.1' }); + for (const origin of [ + 'http://localhost/api/ping', + 'http://127.0.0.1/api/ping', + 'http://127.0.0.2/api/ping', + 'http://tenant.localhost/api/ping', + ]) { + expect((await app.request(origin)).status, origin).toBe(200); + } + }); + + it('does not allow a domain that only starts with 127. (no prefix bypass)', async () => { + const app = guarded({ bindHost: '127.0.0.1' }); + expect((await app.request('http://127.0.0.1.nip.io/api/ping')).status).toBe(403); + }); + + it('allows hosts listed in VIS_ALLOWED_HOSTS, still blocks others', async () => { + const app = guarded({ bindHost: '127.0.0.1', allowedHosts: 'vis.internal,dev.box' }); + expect((await app.request('http://vis.internal/api/ping')).status).toBe(200); + expect((await app.request('http://attacker.example/api/ping')).status).toBe(403); + }); +}); + +describe('createApp host-guard wiring', () => { + const savedEnv = { ...process.env }; + afterEach(() => { + process.env = { ...savedEnv }; + }); + + it('guards requests when no auth token is configured', async () => { + const app = await createApp({ host: '127.0.0.1' }); + const res = await app.request('http://attacker.example/api/sessions'); + expect(res.status).toBe(403); + await expect(res.json()).resolves.toMatchObject({ code: 'FORBIDDEN_HOST' }); + }); + + it('does not host-guard in token mode, so wildcard LAN access keeps working', async () => { + // Binding to a wildcard requires a token; the token (not the Host) is the + // access control, so a concrete LAN client must not be rejected as a + // forbidden host. + const app = await createApp({ authToken: 'secret-token', host: '0.0.0.0' }); + const res = await app.request('http://192.168.1.10/api/sessions', { + headers: { authorization: 'Bearer secret-token' }, + }); + expect(res.status).not.toBe(403); + }); +});