Harden brainstorm companion auth regressions

2026-06-15 23:29:04 +08:00 · 2026-06-10 14:58:16 -07:00
parent f656a28e6e
commit 7b757c6de6
9 changed files with 320 additions and 34 deletions
--- 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(/<script>\n([\s\S]*?)\n<\/script>/);
+  assert(match, 'bootstrap response should contain a script block');
+  const replacements = [];
+  const location = { replace(url) { replacements.push(url); } };
+  new Function('sessionStorage', 'location', match[1])(sessionStorage, location);
+  return replacements;
+}
+
 async function waitForServer(server) {
  let stdout = '', stderr = '';
  return new Promise((resolve, reject) => {
@@ -120,15 +145,46 @@ async function runTests() {
      assert(/key/i.test(res.body), '403 body should mention the key');
    });

+    await test('403 responses include leak-reduction and anti-framing headers', async () => {
+      const res = await get('/');
+      assert.strictEqual(res.status, 403);
+      assertSecurityHeaders(res.headers);
+    });
+
    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 () => {
+    await test('GET / with wrong key and valid cookie is rejected with 403', async () => {
+      const res = await get('/', { key: 'wrong-token', cookie: `${COOKIE_NAME}=${TOKEN}` });
+      assert.strictEqual(res.status, 403, 'explicit wrong query key must not fall back to cookie auth');
+    });
+
+    await test('GET / with valid query returns bootstrap instead of screen content', async () => {
      const res = await get('/', { key: TOKEN });
      assert.strictEqual(res.status, 200);
-      assert(res.body.includes('Secret screen'), 'should serve the screen content');
+      assert(res.body.includes('sessionStorage'), 'bootstrap should store the session key in tab storage');
+      assert(res.body.includes('location.replace'), 'bootstrap should navigate to the bare root URL');
+      assert(!res.body.includes('Secret screen'), 'bootstrap must not serve screen HTML at the keyed URL');
+    });
+
+    await test('bootstrap strips the key URL even when sessionStorage write fails', async () => {
+      const res = await get('/', { key: TOKEN });
+      assert.strictEqual(res.status, 200);
+      let replacements;
+      assert.doesNotThrow(() => {
+        replacements = runBootstrapScript(res.body, {
+        setItem() { throw new Error('storage blocked'); }
+        });
+      });
+      assert.deepStrictEqual(replacements, ['/']);
+    });
+
+    await test('HTML responses include leak-reduction and anti-framing headers', async () => {
+      const res = await get('/', { key: TOKEN });
+      assert.strictEqual(res.status, 200);
+      assertSecurityHeaders(res.headers);
    });

    await test('valid key load sets an HttpOnly SameSite=Strict cookie', async () => {
@@ -142,7 +198,8 @@ async function runTests() {
    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'));
+      assert(res.body.includes('Secret screen'), 'cookie-authenticated bare root should serve the screen');
+      assert(!res.body.includes("location.replace('/');"), 'bare screen response should not be the bootstrap page');
    });

    console.log('\n--- HTTP /files gate ---');
@@ -158,6 +215,12 @@ async function runTests() {
      assert(res.body.includes('secret asset'));
    });

+    await test('/files responses include leak-reduction and anti-framing headers', async () => {
+      const res = await get('/files/asset.txt', { key: TOKEN });
+      assert.strictEqual(res.status, 200);
+      assertSecurityHeaders(res.headers);
+    });
+
    console.log('\n--- WebSocket gate ---');

    await test('WS upgrade without key is rejected', async () => {
@@ -178,6 +241,33 @@ async function runTests() {
      assert.strictEqual(outcome, 'opened');
    });

+    await test('WS upgrade with valid cookie and same-origin Origin opens', async () => {
+      const { outcome, ws } = await wsConnect({
+        cookie: `${COOKIE_NAME}=${TOKEN}`,
+        origin: `http://localhost:${TEST_PORT}`
+      });
+      ws.close();
+      assert.strictEqual(outcome, 'opened');
+    });
+
+    await test('WS upgrade with valid cookie but cross-origin Origin is rejected', async () => {
+      const eventsFile = path.join(TEST_DIR, 'state', 'events');
+      if (fs.existsSync(eventsFile)) fs.unlinkSync(eventsFile);
+
+      const { outcome, ws } = await wsConnect({
+        cookie: `${COOKIE_NAME}=${TOKEN}`,
+        origin: 'http://localhost:9999'
+      });
+      if (outcome === 'opened') {
+        ws.send(JSON.stringify({ type: 'choice', choice: 'attacker-injected', text: 'local attacker probe' }));
+        await sleep(300);
+      }
+      ws.close();
+
+      assert.strictEqual(outcome, 'rejected', 'cross-origin browser WS must not open even with cookie');
+      assert(!fs.existsSync(eventsFile), 'cross-origin WS must not write state/events');
+    });
+
    console.log('\n--- Robustness (A3) ---');

    await test('null payload over an authed WS does not crash the server', async () => {
--- a/tests/brainstorm-server/helper.test.js
+++ b/tests/brainstorm-server/helper.test.js
@@ -85,7 +85,7 @@ console.log('\n--- Reconnect state machine (mocked browser) ---');
 // Drive helper.js's browser code against mocked DOM/WebSocket/timers/clock so we
 // can exercise the actual reconnect/status/tombstone behaviour, not just grep it.
 function makeEnv() {
-  const state = { now: 1000, timers: [], reloads: 0, appended: [] };
+  const state = { now: 1000, timers: [], reloads: 0, replacements: [], appended: [], sessionKey: 'stored-key-abc' };
  const sockets = [];
  const statusEl = { textContent: '', style: { setProperty() {} } };
  class FakeWS {
@@ -97,7 +97,14 @@ function makeEnv() {
  FakeWS.OPEN = 1;
  const env = {
    module: { exports: {} },
-    window: { location: { host: 'localhost:7777', reload() { state.reloads++; } } },
+    window: {
+      location: {
+        host: 'localhost:7777',
+        reload() { state.reloads++; },
+        replace(url) { state.replacements.push(url); }
+      },
+      sessionStorage: { getItem: (key) => key === 'brainstorm-session-key' ? state.sessionKey : null }
+    },
    document: {
      querySelector: (s) => s === '.status' ? statusEl : null,
      getElementById: () => null,
@@ -124,6 +131,20 @@ function makeEnv() {
  };
 }

+test('uses sessionStorage key in the WebSocket URL when present', () => {
+  const e = makeEnv();
+  e.state.sessionKey = 'stored-key-abc';
+  e.boot();
+  assert.strictEqual(e.sockets[0].url, 'ws://localhost:7777/?key=stored-key-abc');
+});
+
+test('uses cookie-only WebSocket URL when no sessionStorage key is present', () => {
+  const e = makeEnv();
+  e.state.sessionKey = null;
+  e.boot();
+  assert.strictEqual(e.sockets[0].url, 'ws://localhost:7777');
+});
+
 test('on disconnect shows Reconnecting and schedules a 500ms reconnect', () => {
  const e = makeEnv(); e.boot();
  e.last().open();
@@ -150,13 +171,26 @@ test('shows the tombstone and Disconnected after the grace period', () => {
  assert.strictEqual(e.state.appended.length, 1, 'tombstone appended exactly once');
 });

-test('reloads to recover when a tombstoned connection comes back', () => {
+test('rebootstraps with stored key when a tombstoned connection comes back', () => {
  const e = makeEnv(); e.boot();
  e.last().open(); e.last().close();
  e.advance(20000); e.fireReconnect(); e.last().close(); // tombstone now shown
-  assert.strictEqual(e.state.reloads, 0);
+  assert.deepStrictEqual(e.state.replacements, []);
  e.fireReconnect(); e.last().open();                    // server back (e.g. same-port restart)
+  assert.strictEqual(e.state.reloads, 0, 'stored-key recovery should not reload bare /');
+  assert.deepStrictEqual(e.state.replacements, ['/?key=stored-key-abc']);
+});
+
+test('reloads to recover when tombstoned and no sessionStorage key is present', () => {
+  const e = makeEnv();
+  e.state.sessionKey = null;
+  e.boot();
+  e.last().open(); e.last().close();
+  e.advance(20000); e.fireReconnect(); e.last().close(); // tombstone now shown
+  assert.strictEqual(e.state.reloads, 0);
+  e.fireReconnect(); e.last().open();                    // server back (e.g. cookie-only page)
  assert.strictEqual(e.state.reloads, 1, 'reloads once on recovery');
+  assert.deepStrictEqual(e.state.replacements, []);
 });

 console.log(`\n--- Results: ${passed} passed, ${failed} failed ---`);
--- a/tests/brainstorm-server/lifecycle.test.js
+++ b/tests/brainstorm-server/lifecycle.test.js
@@ -20,6 +20,19 @@ const START = path.join(__dirname, '../../skills/brainstorming/scripts/start-ser
 const STOP = path.join(__dirname, '../../skills/brainstorming/scripts/stop-server.sh');
 const sleep = ms => new Promise(r => setTimeout(r, ms));

+function waitForExit(child, timeoutMs = 2000) {
+  return new Promise(resolve => {
+    let settled = false;
+    const finish = (exited) => {
+      if (settled) return;
+      settled = true;
+      resolve(exited);
+    };
+    child.once('exit', () => finish(true));
+    setTimeout(() => finish(false), timeoutMs);
+  });
+}
+
 function firstServerStarted(out) {
  return JSON.parse(out.trim().split('\n').find(l => l.includes('server-started')));
 }
@@ -72,7 +85,7 @@ async function runTests() {
  await test('start-server.sh --idle-timeout-minutes sets the timeout', async () => {
    const dir = fs.mkdtempSync('/tmp/bs-life-');
    let info;
-    const out = execFileSync('bash', [START, '--project-dir', dir, '--idle-timeout-minutes', '5'], { encoding: 'utf8' });
+    const out = execFileSync('bash', [START, '--project-dir', dir, '--idle-timeout-minutes', '5', '--background'], { encoding: 'utf8' });
    info = firstServerStarted(out);
    try {
      assert.strictEqual(info.idle_timeout_ms, 5 * 60 * 1000, '5 minutes -> 300000 ms');
@@ -94,7 +107,9 @@ async function runTests() {
    const infoA = firstServerStarted(outA);
    const keyA = new URL(infoA.url).searchParams.get('key');
    assert(fs.existsSync(portFile) && fs.existsSync(tokenFile), 'should write the port and token files');
-    a.kill(); await sleep(400); // free the port
+    const exitedA = waitForExit(a);
+    a.kill();
+    assert(await exitedA, 'first server should exit before restart binds its port');

    const b = spawn('node', [SERVER], { env: { ...env, BRAINSTORM_DIR: path.join(dir, 's2') } });
    let outB = ''; b.stdout.on('data', d => outB += d.toString());
@@ -108,6 +123,49 @@ async function runTests() {
    assert.strictEqual(keyB, keyA, 'restart should reuse the same session key');
  });

+  await test('stored key can authenticate WebSocket after same-port restart', async () => {
+    const dir = fs.mkdtempSync('/tmp/bs-reconnect-');
+    const portFile = path.join(dir, '.last-port');
+    const tokenFile = path.join(dir, '.last-token');
+    const env = { ...process.env, BRAINSTORM_PORT_FILE: portFile, BRAINSTORM_TOKEN_FILE: tokenFile, BRAINSTORM_LIFECYCLE_CHECK_MS: 100000 };
+    let a = null, b = null, ws = null;
+
+    try {
+      a = spawn('node', [SERVER], { env: { ...env, BRAINSTORM_DIR: path.join(dir, 's1') } });
+      let outA = ''; a.stdout.on('data', d => outA += d.toString());
+      for (let i = 0; i < 60 && !outA.includes('server-started'); i++) await sleep(50);
+      const infoA = firstServerStarted(outA);
+      const keyA = new URL(infoA.url).searchParams.get('key');
+      const exitedA = waitForExit(a);
+      a.kill();
+      assert(await exitedA, 'first server should exit before restart binds its port');
+      a = null;
+
+      b = spawn('node', [SERVER], { env: { ...env, BRAINSTORM_DIR: path.join(dir, 's2') } });
+      let outB = ''; b.stdout.on('data', d => outB += d.toString());
+      for (let i = 0; i < 60 && !outB.includes('server-started'); i++) await sleep(50);
+      const infoB = firstServerStarted(outB);
+
+      ws = new WebSocket(`ws://localhost:${infoB.port}/?key=${keyA}`, {
+        headers: { Origin: `http://localhost:${infoB.port}` }
+      });
+      const opened = await new Promise(resolve => {
+        ws.on('open', () => resolve(true));
+        ws.on('error', () => resolve(false));
+        setTimeout(() => resolve(false), 1500);
+      });
+
+      assert.strictEqual(infoB.port, infoA.port, 'restart should reuse same port');
+      assert(opened, 'stored key should authenticate WS after restart');
+    } finally {
+      try { if (ws) ws.close(); } catch (e) {}
+      try { if (a) a.kill(); } catch (e) {}
+      try { if (b) b.kill(); } catch (e) {}
+      await sleep(100);
+      fs.rmSync(dir, { recursive: true, force: true });
+    }
+  });
+
  await test('falls back to a random port when the preferred port is taken', async () => {
    const dir = fs.mkdtempSync('/tmp/bs-port-');
    const portFile = path.join(dir, '.last-port');
--- a/tests/brainstorm-server/server.test.js
+++ b/tests/brainstorm-server/server.test.js
@@ -35,9 +35,9 @@ async function sleep(ms) {
 }

 async function fetch(url) {
-  const authed = url + (url.includes('?') ? '&' : '?') + 'key=' + TOKEN;
  return new Promise((resolve, reject) => {
-    http.get(authed, (res) => {
+    const headers = { Cookie: `brainstorm-key-${TEST_PORT}=${TOKEN}` };
+    http.get(url, { headers }, (res) => {
      let data = '';
      res.on('data', chunk => data += chunk);
      res.on('end', () => resolve({
@@ -210,6 +210,28 @@ async function runTests() {
      assert.strictEqual(alive.status, 200, 'server must survive a /files/ request');
    });

+    await test('does not serve symlinks that escape content dir via /files/', async () => {
+      const target = path.join(STATE_DIR, 'server-info');
+      const link = path.join(CONTENT_DIR, 'linked-server-info.txt');
+      try { fs.unlinkSync(link); } catch (e) {}
+      fs.symlinkSync(target, link);
+
+      const res = await fetch(`http://localhost:${TEST_PORT}/files/linked-server-info.txt`);
+      assert.strictEqual(res.status, 404, 'symlink to state/server-info must not be served');
+      assert(!res.body.includes('server-started'), 'response must not include server-info body');
+    });
+
+    await test('does not serve hard links to files outside content dir via /files/', async () => {
+      const target = path.join(STATE_DIR, 'server-info');
+      const link = path.join(CONTENT_DIR, 'hard-linked-server-info.txt');
+      try { fs.unlinkSync(link); } catch (e) {}
+      fs.linkSync(target, link);
+
+      const res = await fetch(`http://localhost:${TEST_PORT}/files/hard-linked-server-info.txt`);
+      assert.strictEqual(res.status, 404, 'hard link to state/server-info must not be served');
+      assert(!res.body.includes('server-started'), 'response must not include server-info body');
+    });
+
    await test('returns 404 for non-root paths', async () => {
      const res = await fetch(`http://localhost:${TEST_PORT}/other`);
      assert.strictEqual(res.status, 404);