From 7b757c6de626fa25b1a25ca87154d6b13ad4a4ef Mon Sep 17 00:00:00 2001 From: Drew Ritter Date: Wed, 10 Jun 2026 14:58:16 -0700 Subject: [PATCH] Harden brainstorm companion auth regressions --- .gitignore | 1 + skills/brainstorming/scripts/helper.js | 29 +++++- skills/brainstorming/scripts/server.cjs | 88 +++++++++++++--- skills/brainstorming/scripts/start-server.sh | 4 +- skills/brainstorming/scripts/stop-server.sh | 2 +- tests/brainstorm-server/auth.test.js | 100 ++++++++++++++++++- tests/brainstorm-server/helper.test.js | 42 +++++++- tests/brainstorm-server/lifecycle.test.js | 62 +++++++++++- tests/brainstorm-server/server.test.js | 26 ++++- 9 files changed, 320 insertions(+), 34 deletions(-) diff --git a/.gitignore b/.gitignore index 981f4bc3..7acf00f1 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,7 @@ .worktrees/ .private-journal/ .claude/ +.superpowers/ .DS_Store node_modules/ inspo diff --git a/skills/brainstorming/scripts/helper.js b/skills/brainstorming/scripts/helper.js index 41a3aa61..0d8098ad 100644 --- a/skills/brainstorming/scripts/helper.js +++ b/skills/brainstorming/scripts/helper.js @@ -14,7 +14,6 @@ // Everything below is browser-only; bail out when loaded in Node (tests). if (typeof window === 'undefined') return; - const WS_URL = 'ws://' + window.location.host; let ws = null; let eventQueue = []; let reconnectDelay = MIN_RECONNECT_MS; @@ -23,6 +22,27 @@ let everConnected = false; let tombstoneShown = false; + function sessionKey() { + try { + return window.sessionStorage && window.sessionStorage.getItem('brainstorm-session-key'); + } catch (e) {} + return null; + } + + function websocketUrl() { + const key = sessionKey(); + return 'ws://' + window.location.host + (key ? '/?key=' + encodeURIComponent(key) : ''); + } + + function reloadAfterRecovery() { + const key = sessionKey(); + if (key) { + window.location.replace('/?key=' + encodeURIComponent(key)); + } else { + window.location.reload(); + } + } + // Reflect connection state in the frame's status pill (absent on full-doc screens). function setStatus(state) { const el = document.querySelector('.status'); @@ -57,7 +77,7 @@ function connect() { if (reconnectTimer) { clearTimeout(reconnectTimer); reconnectTimer = null; } setStatus(everConnected ? 'reconnecting' : 'connecting'); - ws = new WebSocket(WS_URL); + ws = new WebSocket(websocketUrl()); ws.onopen = () => { const recovered = tombstoneShown; @@ -69,8 +89,9 @@ eventQueue.forEach(e => ws.send(JSON.stringify(e))); eventQueue = []; // Recovered from a tombstoned outage (e.g. the server restarted on the same - // port) — reload to pick up the restarted server's current screen. - if (recovered) window.location.reload(); + // port) — reload through the keyed bootstrap when possible so the cookie is + // refreshed before the visible URL returns to bare /. + if (recovered) reloadAfterRecovery(); }; ws.onmessage = (msg) => { diff --git a/skills/brainstorming/scripts/server.cjs b/skills/brainstorming/scripts/server.cjs index 86024f05..8728cc38 100644 --- a/skills/brainstorming/scripts/server.cjs +++ b/skills/brainstorming/scripts/server.cjs @@ -152,6 +152,20 @@ h1 { color: #333; } p { color: #666; } code { background: #f0f0f0; padding: 0.1e

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

`; +function bootstrapPage(key) { + const jsonKey = JSON.stringify(String(key)); + return ` + +Opening Brainstorm Companion + + + +`; +} + 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 = ''; @@ -178,6 +192,21 @@ function getNewestScreen() { return files.length > 0 ? files[0].path : null; } +function isRegularFileInsideContentDir(filePath) { + let stat, realContentDir, realFilePath; + try { + stat = fs.lstatSync(filePath); + if (stat.isSymbolicLink()) return false; + if (!stat.isFile()) return false; + if (stat.nlink !== 1) return false; + realContentDir = fs.realpathSync(CONTENT_DIR); + realFilePath = fs.realpathSync(filePath); + } catch (e) { + return false; + } + return realFilePath.startsWith(realContentDir + path.sep); +} + // ========== Authentication ========== function timingSafeEqualStr(a, b) { @@ -203,8 +232,11 @@ function parseCookies(header) { 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 params = new URLSearchParams(req.url.slice(q + 1)); + if (params.has('key')) { + const key = params.get('key'); + return Boolean(key && timingSafeEqualStr(key, TOKEN)); + } } const cookie = parseCookies(req.headers['cookie'])[COOKIE_NAME]; if (cookie && timingSafeEqualStr(cookie, TOKEN)) return true; @@ -216,25 +248,53 @@ function pathnameOf(url) { return q >= 0 ? url.slice(0, q) : url; } +function queryKey(url) { + const q = url.indexOf('?'); + if (q < 0) return null; + return new URLSearchParams(url.slice(q + 1)).get('key'); +} + +function securityHeaders(headers = {}) { + return { + 'Referrer-Policy': 'no-referrer', + 'Cache-Control': 'no-store', + 'X-Frame-Options': 'DENY', + 'Content-Security-Policy': "frame-ancestors 'none'", + 'Cross-Origin-Resource-Policy': 'same-origin', + ...headers + }; +} + +function isAllowedWebSocketOrigin(req) { + const origin = req.headers.origin; + if (!origin) return true; + const host = req.headers.host; + if (!host) return false; + return origin === 'http://' + host; +} + // ========== HTTP Request Handler ========== function handleRequest(req, res) { if (!isAuthorized(req)) { - res.writeHead(403, { 'Content-Type': 'text/html; charset=utf-8' }); + res.writeHead(403, securityHeaders({ 'Content-Type': 'text/html; charset=utf-8' })); res.end(FORBIDDEN_PAGE); return; } touchActivity(); // only authorized requests count as activity - // 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. + // Mirror the key into a cookie so same-origin subresources (/files/*) can + // authenticate after bootstrap. HttpOnly keeps it away from page scripts; the + // WebSocket Origin check below is what blocks cross-origin localhost injection. res.setHeader('Set-Cookie', COOKIE_NAME + '=' + TOKEN + '; HttpOnly; SameSite=Strict; Path=/'); const pathname = pathnameOf(req.url); - if (req.method === 'GET' && pathname === '/') { + const keyFromQuery = queryKey(req.url); + if (req.method === 'GET' && pathname === '/' && keyFromQuery && timingSafeEqualStr(keyFromQuery, TOKEN)) { + res.writeHead(200, securityHeaders({ 'Content-Type': 'text/html; charset=utf-8' })); + res.end(bootstrapPage(keyFromQuery)); + } else if (req.method === 'GET' && pathname === '/') { const screenFile = getNewestScreen(); let html = screenFile ? (raw => isFullDocument(raw) ? raw : wrapInFrame(raw))(fs.readFileSync(screenFile, 'utf-8')) @@ -246,24 +306,24 @@ function handleRequest(req, res) { html += helperInjection; } - res.writeHead(200, { 'Content-Type': 'text/html; charset=utf-8' }); + res.writeHead(200, securityHeaders({ 'Content-Type': 'text/html; charset=utf-8' })); res.end(html); } else if (req.method === 'GET' && pathname.startsWith('/files/')) { const fileName = path.basename(pathname.slice(7)); const filePath = path.join(CONTENT_DIR, fileName); // Reject empty/dotfile names and anything that isn't a regular file — // `/files/` would otherwise resolve to CONTENT_DIR and crash readFileSync (EISDIR). - if (!fileName || fileName.startsWith('.') || !fs.existsSync(filePath) || !fs.statSync(filePath).isFile()) { - res.writeHead(404); + if (!fileName || fileName.startsWith('.') || !isRegularFileInsideContentDir(filePath)) { + res.writeHead(404, securityHeaders()); res.end('Not found'); return; } const ext = path.extname(filePath).toLowerCase(); const contentType = MIME_TYPES[ext] || 'application/octet-stream'; - res.writeHead(200, { 'Content-Type': contentType }); + res.writeHead(200, securityHeaders({ 'Content-Type': contentType })); res.end(fs.readFileSync(filePath)); } else { - res.writeHead(404); + res.writeHead(404, securityHeaders()); res.end('Not found'); } } @@ -273,7 +333,7 @@ function handleRequest(req, res) { const clients = new Set(); function handleUpgrade(req, socket) { - if (!isAuthorized(req)) { socket.destroy(); return; } + if (!isAuthorized(req) || !isAllowedWebSocketOrigin(req)) { socket.destroy(); return; } const key = req.headers['sec-websocket-key']; if (!key) { socket.destroy(); return; } diff --git a/skills/brainstorming/scripts/start-server.sh b/skills/brainstorming/scripts/start-server.sh index 5e9a00cb..e56b133b 100755 --- a/skills/brainstorming/scripts/start-server.sh +++ b/skills/brainstorming/scripts/start-server.sh @@ -125,7 +125,7 @@ if [[ -f "$PID_FILE" ]]; then rm -f "$PID_FILE" fi -cd "$SCRIPT_DIR" +cd "$SCRIPT_DIR" || exit 1 # Resolve the harness PID (grandparent of this script). # $PPID is the ephemeral shell the harness spawned to run us — it dies @@ -163,7 +163,7 @@ disown "$SERVER_PID" 2>/dev/null echo "$SERVER_PID" > "$PID_FILE" # Wait for server-started message (check log file) -for i in {1..50}; do +for _ in {1..50}; do if grep -q "server-started" "$LOG_FILE" 2>/dev/null; then # Verify server is still alive after a short window (catches process reapers) alive="true" diff --git a/skills/brainstorming/scripts/stop-server.sh b/skills/brainstorming/scripts/stop-server.sh index 082d065c..1bfcd082 100755 --- a/skills/brainstorming/scripts/stop-server.sh +++ b/skills/brainstorming/scripts/stop-server.sh @@ -54,7 +54,7 @@ if [[ -f "$PID_FILE" ]]; then kill "$pid" 2>/dev/null || true # Wait for graceful shutdown (up to ~2s) - for i in {1..20}; do + for _ in {1..20}; do if ! kill -0 "$pid" 2>/dev/null; then break fi diff --git a/tests/brainstorm-server/auth.test.js b/tests/brainstorm-server/auth.test.js index 9a6fb002..ccda4e39 100644 --- a/tests/brainstorm-server/auth.test.js +++ b/tests/brainstorm-server/auth.test.js @@ -25,6 +25,13 @@ 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}`; +const EXPECTED_SECURITY_HEADERS = { + 'referrer-policy': 'no-referrer', + 'cache-control': 'no-store', + 'x-frame-options': 'DENY', + 'content-security-policy': "frame-ancestors 'none'", + 'cross-origin-resource-policy': 'same-origin' +}; function cleanup() { if (fs.existsSync(TEST_DIR)) fs.rmSync(TEST_DIR, { recursive: true }); @@ -49,9 +56,12 @@ function get(pathname, { key, cookie } = {}) { } // Try to open a WebSocket; resolve 'opened' or 'rejected'. -function wsConnect({ key, cookie } = {}) { +function wsConnect({ key, cookie, origin } = {}) { const url = `ws://localhost:${TEST_PORT}/` + (key !== undefined ? `?key=${key}` : ''); - const opts = cookie ? { headers: { Cookie: cookie } } : {}; + const headers = {}; + if (cookie) headers['Cookie'] = cookie; + if (origin) headers['Origin'] = origin; + const opts = Object.keys(headers).length ? { headers } : {}; const ws = new WebSocket(url, opts); return new Promise((resolve) => { let settled = false; @@ -69,6 +79,21 @@ function startServer() { }); } +function assertSecurityHeaders(headers) { + for (const [name, value] of Object.entries(EXPECTED_SECURITY_HEADERS)) { + assert.strictEqual(headers[name], value, `${name} should be ${value}`); + } +} + +function runBootstrapScript(html, sessionStorage) { + const match = html.match(/