diff --git a/skills/brainstorming/scripts/server.cjs b/skills/brainstorming/scripts/server.cjs index 562c17f893..e03d0184ea 100644 --- a/skills/brainstorming/scripts/server.cjs +++ b/skills/brainstorming/scripts/server.cjs @@ -81,6 +81,51 @@ const CONTENT_DIR = path.join(SESSION_DIR, 'content'); const STATE_DIR = path.join(SESSION_DIR, 'state'); let ownerPid = process.env.BRAINSTORM_OWNER_PID ? Number(process.env.BRAINSTORM_OWNER_PID) : null; +// ========== Host Header Allowlist (DNS-rebinding defense) ========== +// +// Without validating the HTTP Host header, a page on another origin can +// DNS-rebind its own hostname to 127.0.0.1 and then read this server's +// `/` and `/files/*` responses as if same-origin. The browser sends the +// rebound name in `Host`, so allowlisting Host on the loopback path +// closes the rebinding surface even when the TCP connection itself is +// genuinely local. +// +// Default allowlist: the configured URL_HOST (what the agent prints in +// the start-server.sh URL) plus the literal loopback names, both bare +// and with the listening port appended. Operators serving the companion +// behind a tunnel or container hostname can extend the list with the +// BRAINSTORM_ALLOWED_HOSTS env var (comma-separated, case-insensitive). +function buildAllowedHosts() { + const set = new Set(); + const portStr = String(PORT); + const add = (h) => { + if (!h) return; + const hl = String(h).trim().toLowerCase(); + if (!hl) return; + set.add(hl); + set.add(hl + ':' + portStr); + }; + // Always allow the loopback names: every legitimate browser session + // resolves the server via one of these even when URL_HOST differs. + add('localhost'); + add('127.0.0.1'); + add('[::1]'); + // Allow the printed/displayed URL_HOST and the bind HOST verbatim. + add(URL_HOST); + add(HOST); + // Operator-provided extras for tunneled / containerized setups. + const extra = process.env.BRAINSTORM_ALLOWED_HOSTS || ''; + for (const h of extra.split(',')) add(h); + return set; +} +const ALLOWED_HOSTS = buildAllowedHosts(); + +function isHostAllowed(req) { + const host = req.headers && req.headers.host; + if (typeof host !== 'string' || host.length === 0) return false; + return ALLOWED_HOSTS.has(host.toLowerCase()); +} + const MIME_TYPES = { '.html': 'text/html', '.css': 'text/css', '.js': 'application/javascript', '.json': 'application/json', '.png': 'image/png', '.jpg': 'image/jpeg', @@ -127,6 +172,14 @@ function getNewestScreen() { // ========== HTTP Request Handler ========== function handleRequest(req, res) { + if (!isHostAllowed(req)) { + // Reject DNS-rebound / cross-origin Host headers before touching state + // or returning any content. 421 Misdirected Request is the canonical + // status for "this connection is not authoritative for this Host". + res.writeHead(421, { 'Content-Type': 'text/plain' }); + res.end('Misdirected Request'); + return; + } touchActivity(); if (req.method === 'GET' && req.url === '/') { const screenFile = getNewestScreen(); @@ -168,6 +221,12 @@ function handleUpgrade(req, socket) { const key = req.headers['sec-websocket-key']; if (!key) { socket.destroy(); return; } + // Same DNS-rebinding gate as handleRequest, applied before completing + // the WebSocket handshake. Without this, a DNS-rebound page could + // open a WS connection and inject events into state_dir/events even + // though the TCP socket is loopback-bound. + if (!isHostAllowed(req)) { socket.destroy(); return; } + const accept = computeAcceptKey(key); socket.write( 'HTTP/1.1 101 Switching Protocols\r\n' + diff --git a/tests/brainstorm-server/server.test.js b/tests/brainstorm-server/server.test.js index 2cccf09541..49f241d4a5 100644 --- a/tests/brainstorm-server/server.test.js +++ b/tests/brainstorm-server/server.test.js @@ -45,6 +45,31 @@ async function fetch(url) { }); } +// Like fetch() but lets the caller override the Host header so we can +// exercise the DNS-rebinding allowlist directly. The TCP target stays +// 127.0.0.1:PORT; only the HTTP/1.1 Host line is forged. +async function fetchWithHost(path, hostHeader) { + return new Promise((resolve, reject) => { + const req = http.request({ + host: '127.0.0.1', + port: TEST_PORT, + path, + method: 'GET', + headers: { Host: hostHeader } + }, (res) => { + let data = ''; + res.on('data', chunk => data += chunk); + res.on('end', () => resolve({ + status: res.statusCode, + headers: res.headers, + body: data + })); + }); + req.on('error', reject); + req.end(); + }); +} + function startServer() { return spawn('node', [SERVER_PATH], { env: { ...process.env, BRAINSTORM_PORT: TEST_PORT, BRAINSTORM_DIR: TEST_DIR } @@ -184,6 +209,57 @@ async function runTests() { assert.strictEqual(res.status, 404); }); + // ========== Host Header Validation (DNS-rebinding defense) ========== + console.log('\n--- Host Header Validation ---'); + + await test('accepts requests with Host: localhost:PORT', async () => { + const res = await fetchWithHost('/', `localhost:${TEST_PORT}`); + assert.strictEqual(res.status, 200); + }); + + await test('accepts requests with Host: 127.0.0.1:PORT', async () => { + const res = await fetchWithHost('/', `127.0.0.1:${TEST_PORT}`); + assert.strictEqual(res.status, 200); + }); + + await test('accepts bare loopback Host without port', async () => { + const res = await fetchWithHost('/', 'localhost'); + assert.strictEqual(res.status, 200); + }); + + await test('rejects DNS-rebound Host with 421', async () => { + const res = await fetchWithHost('/', `evil.example:${TEST_PORT}`); + assert.strictEqual(res.status, 421, 'foreign Host should be rejected'); + assert(!res.body.includes('Waiting for the agent'), 'should not leak screen content'); + }); + + await test('rejects DNS-rebound Host on /files/ endpoint', async () => { + const res = await fetchWithHost('/files/anything', 'attacker.example'); + assert.strictEqual(res.status, 421, 'foreign Host should be rejected before file lookup'); + }); + + await test('rejects WebSocket upgrade from foreign Host', async () => { + // Connect directly to the loopback address but pretend the Host is + // a rebound name. The ws client sends `Host: :` by + // default, so we point its URL at evil.example and route the TCP + // connection back to the loopback listener. + const ws = new WebSocket(`ws://evil.example:${TEST_PORT}`, { + lookup: (_hostname, _opts, cb) => cb(null, '127.0.0.1', 4) + }); + let opened = false; + let errored = false; + await new Promise((resolve) => { + ws.on('open', () => { opened = true; resolve(); }); + ws.on('error', () => { errored = true; resolve(); }); + ws.on('unexpected-response', () => resolve()); + ws.on('close', resolve); + setTimeout(resolve, 1500); + }); + try { ws.terminate(); } catch (_) { /* ignore */ } + assert(!opened, 'WS upgrade with foreign Host must not complete'); + assert(errored, 'WS client should observe a connection error'); + }); + // ========== WebSocket Communication ========== console.log('\n--- WebSocket Communication ---');