diff --git a/skills/brainstorming/scripts/server.cjs b/skills/brainstorming/scripts/server.cjs index fc9f359b..22af144d 100644 --- a/skills/brainstorming/scripts/server.cjs +++ b/skills/brainstorming/scripts/server.cjs @@ -104,6 +104,15 @@ 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; +// Per-session secret key. The companion is reachable by any local browser tab +// and, when bound to a non-loopback host, by any host that can route to it. +// The key authenticates the real client uniformly across loopback, tunnel, and +// remote binds — and defeats DNS rebinding — where a Host/Origin allowlist +// cannot. It rides the served URL as ?key= and is mirrored into a cookie on +// first load so same-origin subresources and the WebSocket carry it for free. +const TOKEN = process.env.BRAINSTORM_TOKEN || crypto.randomBytes(32).toString('hex'); +const COOKIE_NAME = 'brainstorm-key-' + PORT; + const MIME_TYPES = { '.html': 'text/html', '.css': 'text/css', '.js': 'application/javascript', '.json': 'application/json', '.png': 'image/png', '.jpg': 'image/jpeg', @@ -121,6 +130,16 @@ h1 { color: #333; } p { color: #666; }

Brainstorm Companion

Waiting for the agent to push a screen...

`; +const FORBIDDEN_PAGE = ` + +Session key required + + +

Session key required

+

This page needs the full URL your coding agent gave you, including the +?key=… part. Copy the complete URL and open it again.

`; + const frameTemplate = fs.readFileSync(path.join(__dirname, 'frame-template.html'), 'utf-8'); const helperScript = fs.readFileSync(path.join(__dirname, 'helper.js'), 'utf-8'); const helperInjection = ''; @@ -147,11 +166,64 @@ function getNewestScreen() { return files.length > 0 ? files[0].path : null; } +// ========== Authentication ========== + +function timingSafeEqualStr(a, b) { + const ab = Buffer.from(String(a)); + const bb = Buffer.from(String(b)); + if (ab.length !== bb.length) return false; + return crypto.timingSafeEqual(ab, bb); +} + +function parseCookies(header) { + const out = {}; + if (!header) return out; + for (const part of header.split(';')) { + const eq = part.indexOf('='); + if (eq < 0) continue; + out[part.slice(0, eq).trim()] = part.slice(eq + 1).trim(); + } + return out; +} + +// A request is authorized if it carries the session key as ?key= or as the +// session cookie. Both are compared in constant time. +function isAuthorized(req) { + const q = req.url.indexOf('?'); + if (q >= 0) { + const key = new URLSearchParams(req.url.slice(q + 1)).get('key'); + if (key && timingSafeEqualStr(key, TOKEN)) return true; + } + const cookie = parseCookies(req.headers['cookie'])[COOKIE_NAME]; + if (cookie && timingSafeEqualStr(cookie, TOKEN)) return true; + return false; +} + +function pathnameOf(url) { + const q = url.indexOf('?'); + return q >= 0 ? url.slice(0, q) : url; +} + // ========== HTTP Request Handler ========== function handleRequest(req, res) { touchActivity(); - if (req.method === 'GET' && req.url === '/') { + + if (!isAuthorized(req)) { + res.writeHead(403, { 'Content-Type': 'text/html; charset=utf-8' }); + res.end(FORBIDDEN_PAGE); + return; + } + + // Mirror the key into a cookie so same-origin subresources (/files/*) and the + // WebSocket handshake carry it automatically, whatever URL style the agent + // writes. SameSite=Strict: a cross-site page can neither read the key nor ride + // the cookie; HttpOnly: page scripts can't exfiltrate it. + res.setHeader('Set-Cookie', + COOKIE_NAME + '=' + TOKEN + '; HttpOnly; SameSite=Strict; Path=/'); + + const pathname = pathnameOf(req.url); + if (req.method === 'GET' && pathname === '/') { const screenFile = getNewestScreen(); let html = screenFile ? (raw => isFullDocument(raw) ? raw : wrapInFrame(raw))(fs.readFileSync(screenFile, 'utf-8')) @@ -165,8 +237,8 @@ function handleRequest(req, res) { res.writeHead(200, { 'Content-Type': 'text/html; charset=utf-8' }); res.end(html); - } else if (req.method === 'GET' && req.url.startsWith('/files/')) { - const fileName = path.basename(req.url.slice(7)); + } else if (req.method === 'GET' && pathname.startsWith('/files/')) { + const fileName = path.basename(pathname.slice(7)); const filePath = path.join(CONTENT_DIR, fileName); if (fileName.startsWith('.') || !fs.existsSync(filePath)) { res.writeHead(404); @@ -188,6 +260,8 @@ function handleRequest(req, res) { const clients = new Set(); function handleUpgrade(req, socket) { + if (!isAuthorized(req)) { socket.destroy(); return; } + const key = req.headers['sec-websocket-key']; if (!key) { socket.destroy(); return; } @@ -254,7 +328,7 @@ function handleMessage(text) { } touchActivity(); console.log(JSON.stringify({ source: 'user-event', ...event })); - if (event.choice) { + if (event && event.choice) { const eventsFile = path.join(STATE_DIR, 'events'); fs.appendFileSync(eventsFile, JSON.stringify(event) + '\n'); } @@ -418,7 +492,7 @@ function startServer() { } const info = JSON.stringify({ type: 'server-started', port: Number(PORT), host: HOST, - url_host: URL_HOST, url: 'http://' + URL_HOST + ':' + PORT, + url_host: URL_HOST, url: 'http://' + URL_HOST + ':' + PORT + '/?key=' + TOKEN, screen_dir: CONTENT_DIR, state_dir: STATE_DIR, idle_timeout_ms: IDLE_TIMEOUT_MS }); console.log(info); diff --git a/skills/brainstorming/visual-companion.md b/skills/brainstorming/visual-companion.md index b3a1783a..7072b728 100644 --- a/skills/brainstorming/visual-companion.md +++ b/skills/brainstorming/visual-companion.md @@ -37,13 +37,22 @@ The server watches a directory for HTML files and serves the newest one to the b # the first screen; --project-dir persists mockups and enables same-port restart. scripts/start-server.sh --project-dir /path/to/project --open -# Returns: {"type":"server-started","port":52341,"url":"http://localhost:52341", +# Returns: {"type":"server-started","port":52341, +# "url":"http://localhost:52341/?key=ab12…", # "screen_dir":"/path/to/project/.superpowers/brainstorm/12345-1706000000/content", # "state_dir":"/path/to/project/.superpowers/brainstorm/12345-1706000000/state"} ``` Save `screen_dir` and `state_dir` from the response. With `--open`, the browser opens itself when you push the first screen — you don't need to ask the user to open it, but still share the URL as a fallback (headless/remote setups won't auto-open). +**The URL contains a session key (`?key=…`).** The server rejects any request +without it, so always give the user the **complete** URL from the `url` field — +never strip the query string, and never hand out a bare `http://host:port`. The +key gates HTTP and WebSocket access so a stray browser tab or another machine on +the network can't read the screens or inject events. After the first load the +browser remembers the key via a cookie, so reloads and `/files/*` assets work +without repeating it. + **Finding connection info:** The server writes its startup JSON to `$STATE_DIR/server-info`. If you launched the server in the background and didn't capture stdout, read that file to get the URL and port. When using `--project-dir`, check `/.superpowers/brainstorm/` for the session directory. **Note:** Pass the project root as `--project-dir` so mockups persist in `.superpowers/brainstorm/` and survive server restarts. Without it, files go to `/tmp` and get cleaned up. Remind the user to add `.superpowers/` to `.gitignore` if it's not already there. diff --git a/tests/brainstorm-server/auth.test.js b/tests/brainstorm-server/auth.test.js new file mode 100644 index 00000000..9a6fb002 --- /dev/null +++ b/tests/brainstorm-server/auth.test.js @@ -0,0 +1,201 @@ +/** + * Security tests for the brainstorm server's per-session key. + * + * The companion server is reachable by any local browser tab (default loopback + * bind) and by any host that can route to it (remote `--host 0.0.0.0` bind). + * A per-session secret key gates every endpoint so that neither a browser + * confused-deputy nor a direct remote client can read screens/files or inject + * events into state/events (prompt injection into a live agent session). + * + * Auth = a valid `?key=` query param OR a valid session cookie. + * + * Uses the `ws` npm package as a test client (test-only dependency). + */ + +const { spawn } = require('child_process'); +const http = require('http'); +const WebSocket = require('ws'); +const fs = require('fs'); +const path = require('path'); +const assert = require('assert'); + +const SERVER_PATH = path.join(__dirname, '../../skills/brainstorming/scripts/server.cjs'); +const TEST_PORT = 3335; +const TEST_DIR = '/tmp/brainstorm-auth-test'; +const CONTENT_DIR = path.join(TEST_DIR, 'content'); +const TOKEN = 'testtoken-0123456789abcdef0123456789abcdef'; +const COOKIE_NAME = `brainstorm-key-${TEST_PORT}`; + +function cleanup() { + if (fs.existsSync(TEST_DIR)) fs.rmSync(TEST_DIR, { recursive: true }); +} + +async function sleep(ms) { + return new Promise(resolve => setTimeout(resolve, ms)); +} + +// Raw HTTP GET with optional key query and Cookie header. +function get(pathname, { key, cookie } = {}) { + const url = `http://localhost:${TEST_PORT}${pathname}` + (key !== undefined ? `?key=${key}` : ''); + const headers = {}; + if (cookie) headers['Cookie'] = cookie; + return new Promise((resolve, reject) => { + http.get(url, { headers }, (res) => { + let data = ''; + res.on('data', chunk => data += chunk); + res.on('end', () => resolve({ status: res.statusCode, headers: res.headers, body: data })); + }).on('error', reject); + }); +} + +// Try to open a WebSocket; resolve 'opened' or 'rejected'. +function wsConnect({ key, cookie } = {}) { + const url = `ws://localhost:${TEST_PORT}/` + (key !== undefined ? `?key=${key}` : ''); + const opts = cookie ? { headers: { Cookie: cookie } } : {}; + const ws = new WebSocket(url, opts); + return new Promise((resolve) => { + let settled = false; + const done = (outcome) => { if (!settled) { settled = true; resolve({ outcome, ws }); } }; + ws.on('open', () => done('opened')); + ws.on('error', () => done('rejected')); + ws.on('close', () => done('rejected')); + setTimeout(() => done('rejected'), 1500); + }); +} + +function startServer() { + return spawn('node', [SERVER_PATH], { + env: { ...process.env, BRAINSTORM_PORT: TEST_PORT, BRAINSTORM_DIR: TEST_DIR, BRAINSTORM_TOKEN: TOKEN } + }); +} + +async function waitForServer(server) { + let stdout = '', stderr = ''; + return new Promise((resolve, reject) => { + server.stdout.on('data', (d) => { + stdout += d.toString(); + if (stdout.includes('server-started')) resolve({ stdout }); + }); + server.stderr.on('data', (d) => { stderr += d.toString(); }); + server.on('error', reject); + setTimeout(() => reject(new Error(`Server didn't start. stderr: ${stderr}`)), 5000); + }); +} + +async function runTests() { + cleanup(); + fs.mkdirSync(CONTENT_DIR, { recursive: true }); + fs.writeFileSync(path.join(CONTENT_DIR, 'screen.html'), '

Secret screen

'); + fs.writeFileSync(path.join(CONTENT_DIR, 'asset.txt'), 'secret asset'); + + const server = startServer(); + let stdoutAccum = ''; + server.stdout.on('data', (d) => { stdoutAccum += d.toString(); }); + const { stdout: initialStdout } = await waitForServer(server); + + let passed = 0, failed = 0; + async function test(name, fn) { + try { await fn(); console.log(` PASS: ${name}`); passed++; } + catch (e) { console.log(` FAIL: ${name}`); console.log(` ${e.message}`); failed++; } + } + + try { + console.log('\n--- Startup URL ---'); + + await test('server-started url includes the session key', () => { + const msg = JSON.parse(initialStdout.trim()); + assert(msg.url.includes(`key=${TOKEN}`), `url should carry the key, got: ${msg.url}`); + }); + + console.log('\n--- HTTP / gate ---'); + + await test('GET / without key is rejected with 403', async () => { + const res = await get('/'); + assert.strictEqual(res.status, 403, 'no-key request must be 403'); + }); + + await test('403 page names "coding agent" and the key', async () => { + const res = await get('/'); + assert(/coding agent/i.test(res.body), '403 body should reference the coding agent'); + assert(/key/i.test(res.body), '403 body should mention the key'); + }); + + await test('GET / with wrong key is rejected with 403', async () => { + const res = await get('/', { key: 'wrong-token' }); + assert.strictEqual(res.status, 403); + }); + + await test('GET / with valid key serves the screen', async () => { + const res = await get('/', { key: TOKEN }); + assert.strictEqual(res.status, 200); + assert(res.body.includes('Secret screen'), 'should serve the screen content'); + }); + + await test('valid key load sets an HttpOnly SameSite=Strict cookie', async () => { + const res = await get('/', { key: TOKEN }); + const setCookie = (res.headers['set-cookie'] || []).join('; '); + assert(setCookie.includes(`${COOKIE_NAME}=${TOKEN}`), `should set ${COOKIE_NAME}`); + assert(/HttpOnly/i.test(setCookie), 'cookie should be HttpOnly'); + assert(/SameSite=Strict/i.test(setCookie), 'cookie should be SameSite=Strict'); + }); + + await test('GET / with valid cookie (no query key) serves the screen', async () => { + const res = await get('/', { cookie: `${COOKIE_NAME}=${TOKEN}` }); + assert.strictEqual(res.status, 200); + assert(res.body.includes('Secret screen')); + }); + + console.log('\n--- HTTP /files gate ---'); + + await test('GET /files without key is rejected with 403', async () => { + const res = await get('/files/asset.txt'); + assert.strictEqual(res.status, 403); + }); + + await test('GET /files with valid key serves the file', async () => { + const res = await get('/files/asset.txt', { key: TOKEN }); + assert.strictEqual(res.status, 200); + assert(res.body.includes('secret asset')); + }); + + console.log('\n--- WebSocket gate ---'); + + await test('WS upgrade without key is rejected', async () => { + const { outcome, ws } = await wsConnect(); + ws.close(); + assert.strictEqual(outcome, 'rejected', 'unauthenticated WS must not open'); + }); + + await test('WS upgrade with valid key opens', async () => { + const { outcome, ws } = await wsConnect({ key: TOKEN }); + ws.close(); + assert.strictEqual(outcome, 'opened'); + }); + + await test('WS upgrade with valid cookie opens', async () => { + const { outcome, ws } = await wsConnect({ cookie: `${COOKIE_NAME}=${TOKEN}` }); + ws.close(); + assert.strictEqual(outcome, 'opened'); + }); + + console.log('\n--- Robustness (A3) ---'); + + await test('null payload over an authed WS does not crash the server', async () => { + const { ws } = await wsConnect({ key: TOKEN }); + ws.send('null'); + await sleep(300); + const res = await get('/', { key: TOKEN }); + assert.strictEqual(res.status, 200, 'server must still respond after null payload'); + ws.close(); + }); + + console.log(`\n--- Results: ${passed} passed, ${failed} failed ---`); + if (failed > 0) process.exit(1); + } finally { + server.kill(); + await sleep(100); + cleanup(); + } +} + +runTests().catch(err => { console.error('Test failed:', err); process.exit(1); }); diff --git a/tests/brainstorm-server/package.json b/tests/brainstorm-server/package.json index 52ff3aed..70bf5254 100644 --- a/tests/brainstorm-server/package.json +++ b/tests/brainstorm-server/package.json @@ -2,7 +2,7 @@ "name": "brainstorm-server-tests", "version": "1.0.0", "scripts": { - "test": "node ws-protocol.test.js && node helper.test.js && node server.test.js && node lifecycle.test.js && bash stop-server.test.sh" + "test": "node ws-protocol.test.js && node helper.test.js && node auth.test.js && node server.test.js && node lifecycle.test.js && bash stop-server.test.sh" }, "dependencies": { "ws": "^8.19.0" diff --git a/tests/brainstorm-server/server.test.js b/tests/brainstorm-server/server.test.js index 52b3ffad..632710fa 100644 --- a/tests/brainstorm-server/server.test.js +++ b/tests/brainstorm-server/server.test.js @@ -20,6 +20,9 @@ const TEST_PORT = 3334; const TEST_DIR = '/tmp/brainstorm-test'; const CONTENT_DIR = path.join(TEST_DIR, 'content'); const STATE_DIR = path.join(TEST_DIR, 'state'); +// Fixed session key so the test client can authenticate (see auth.test.js for +// the security behavior itself; here we just need authorized requests). +const TOKEN = 'testtoken-server-0123456789abcdef'; function cleanup() { if (fs.existsSync(TEST_DIR)) { @@ -32,8 +35,9 @@ async function sleep(ms) { } async function fetch(url) { + const authed = url + (url.includes('?') ? '&' : '?') + 'key=' + TOKEN; return new Promise((resolve, reject) => { - http.get(url, (res) => { + http.get(authed, (res) => { let data = ''; res.on('data', chunk => data += chunk); res.on('end', () => resolve({ @@ -47,7 +51,7 @@ async function fetch(url) { function startServer() { return spawn('node', [SERVER_PATH], { - env: { ...process.env, BRAINSTORM_PORT: TEST_PORT, BRAINSTORM_DIR: TEST_DIR } + env: { ...process.env, BRAINSTORM_PORT: TEST_PORT, BRAINSTORM_DIR: TEST_DIR, BRAINSTORM_TOKEN: TOKEN } }); } @@ -207,7 +211,7 @@ async function runTests() { console.log('\n--- WebSocket Communication ---'); await test('accepts WebSocket upgrade on /', async () => { - const ws = new WebSocket(`ws://localhost:${TEST_PORT}`); + const ws = new WebSocket(`ws://localhost:${TEST_PORT}/?key=${TOKEN}`); await new Promise((resolve, reject) => { ws.on('open', resolve); ws.on('error', reject); @@ -217,7 +221,7 @@ async function runTests() { await test('relays user events to stdout with source field', async () => { stdoutAccum = ''; - const ws = new WebSocket(`ws://localhost:${TEST_PORT}`); + const ws = new WebSocket(`ws://localhost:${TEST_PORT}/?key=${TOKEN}`); await new Promise(resolve => ws.on('open', resolve)); ws.send(JSON.stringify({ type: 'click', text: 'Test Button' })); @@ -233,7 +237,7 @@ async function runTests() { const eventsFile = path.join(STATE_DIR, 'events'); if (fs.existsSync(eventsFile)) fs.unlinkSync(eventsFile); - const ws = new WebSocket(`ws://localhost:${TEST_PORT}`); + const ws = new WebSocket(`ws://localhost:${TEST_PORT}/?key=${TOKEN}`); await new Promise(resolve => ws.on('open', resolve)); ws.send(JSON.stringify({ type: 'click', choice: 'b', text: 'Option B' })); @@ -251,7 +255,7 @@ async function runTests() { const eventsFile = path.join(STATE_DIR, 'events'); if (fs.existsSync(eventsFile)) fs.unlinkSync(eventsFile); - const ws = new WebSocket(`ws://localhost:${TEST_PORT}`); + const ws = new WebSocket(`ws://localhost:${TEST_PORT}/?key=${TOKEN}`); await new Promise(resolve => ws.on('open', resolve)); ws.send(JSON.stringify({ type: 'hover', text: 'Something' })); @@ -263,8 +267,8 @@ async function runTests() { }); await test('handles multiple concurrent WebSocket clients', async () => { - const ws1 = new WebSocket(`ws://localhost:${TEST_PORT}`); - const ws2 = new WebSocket(`ws://localhost:${TEST_PORT}`); + const ws1 = new WebSocket(`ws://localhost:${TEST_PORT}/?key=${TOKEN}`); + const ws2 = new WebSocket(`ws://localhost:${TEST_PORT}/?key=${TOKEN}`); await Promise.all([ new Promise(resolve => ws1.on('open', resolve)), new Promise(resolve => ws2.on('open', resolve)) @@ -289,7 +293,7 @@ async function runTests() { }); await test('cleans up closed clients from broadcast list', async () => { - const ws1 = new WebSocket(`ws://localhost:${TEST_PORT}`); + const ws1 = new WebSocket(`ws://localhost:${TEST_PORT}/?key=${TOKEN}`); await new Promise(resolve => ws1.on('open', resolve)); ws1.close(); await sleep(100); @@ -301,7 +305,7 @@ async function runTests() { }); await test('handles malformed JSON from client gracefully', async () => { - const ws = new WebSocket(`ws://localhost:${TEST_PORT}`); + const ws = new WebSocket(`ws://localhost:${TEST_PORT}/?key=${TOKEN}`); await new Promise(resolve => ws.on('open', resolve)); // Send invalid JSON — server should not crash @@ -318,7 +322,7 @@ async function runTests() { console.log('\n--- File Watching ---'); await test('sends reload on new .html file', async () => { - const ws = new WebSocket(`ws://localhost:${TEST_PORT}`); + const ws = new WebSocket(`ws://localhost:${TEST_PORT}/?key=${TOKEN}`); await new Promise(resolve => ws.on('open', resolve)); let gotReload = false; @@ -338,7 +342,7 @@ async function runTests() { fs.writeFileSync(filePath, '

Original

'); await sleep(500); - const ws = new WebSocket(`ws://localhost:${TEST_PORT}`); + const ws = new WebSocket(`ws://localhost:${TEST_PORT}/?key=${TOKEN}`); await new Promise(resolve => ws.on('open', resolve)); let gotReload = false; @@ -354,7 +358,7 @@ async function runTests() { }); await test('does NOT send reload for non-.html files', async () => { - const ws = new WebSocket(`ws://localhost:${TEST_PORT}`); + const ws = new WebSocket(`ws://localhost:${TEST_PORT}/?key=${TOKEN}`); await new Promise(resolve => ws.on('open', resolve)); let gotReload = false;