From 93f2ce91b80f83f584b654dda3985e4103a932bf Mon Sep 17 00:00:00 2001 From: Drew Ritter Date: Thu, 11 Jun 2026 10:25:19 -0700 Subject: [PATCH] Fix companion stop metadata and token permissions --- .../2026-06-09-visual-companion-issues.md | 10 +++--- skills/brainstorming/scripts/server.cjs | 14 ++++++-- skills/brainstorming/scripts/stop-server.sh | 8 +++++ tests/brainstorm-server/lifecycle.test.js | 35 +++++++++++++++++++ tests/brainstorm-server/stop-server.test.sh | 31 ++++++++++++++++ 5 files changed, 91 insertions(+), 7 deletions(-) diff --git a/docs/superpowers/plans/2026-06-09-visual-companion-issues.md b/docs/superpowers/plans/2026-06-09-visual-companion-issues.md index f3ca90b9..09d6c1d4 100644 --- a/docs/superpowers/plans/2026-06-09-visual-companion-issues.md +++ b/docs/superpowers/plans/2026-06-09-visual-companion-issues.md @@ -283,11 +283,11 @@ vs both) and where the textarea renders (frame-level vs opt-in per screen). In WSL2 especially, people expect the browser to open. **Change.** Best-effort opener after the `server-started` JSON is parsed: -WSL → `cmd.exe /c start`, macOS → `open`, Linux → `xdg-open` only when -`DISPLAY`/`WAYLAND_DISPLAY` is set. Swallow failures, never block startup, keep -echoing the URL. Document in `visual-companion.md`. (Consider an opt-out for -headless/remote runs where popping a browser is wrong — ties into D1's config -mechanism.) +Windows/WSL → `rundll32.exe url.dll,FileProtocolHandler `, macOS → `open`, +Linux → `xdg-open` only when `DISPLAY`/`WAYLAND_DISPLAY` is set. Swallow +failures, never block startup, keep echoing the URL. Document in +`visual-companion.md`. (Consider an opt-out for headless/remote runs where +popping a browser is wrong — ties into D1's config mechanism.) ### D4 — Light/dark contrast helpers (PR #1683) diff --git a/skills/brainstorming/scripts/server.cjs b/skills/brainstorming/scripts/server.cjs index bce67e32..ad9e7347 100644 --- a/skills/brainstorming/scripts/server.cjs +++ b/skills/brainstorming/scripts/server.cjs @@ -117,6 +117,10 @@ function generateToken() { return crypto.randomBytes(32).toString('hex'); } +function chmodOwnerOnly(file) { + try { fs.chmodSync(file, 0o600); } catch (e) { /* best effort */ } +} + function initialToken() { if (process.env.BRAINSTORM_TOKEN) { return { value: process.env.BRAINSTORM_TOKEN, source: 'env' }; @@ -124,7 +128,10 @@ function initialToken() { if (TOKEN_FILE) { try { const t = fs.readFileSync(TOKEN_FILE, 'utf-8').trim(); - if (/^[0-9a-f]{32,}$/i.test(t)) return { value: t, source: 'file' }; + if (/^[0-9a-f]{32,}$/i.test(t)) { + chmodOwnerOnly(TOKEN_FILE); + return { value: t, source: 'file' }; + } } catch (e) { /* no prior token recorded */ } } return { value: generateToken(), source: 'generated' }; @@ -599,7 +606,10 @@ function startServer() { if (PORT_FILE && !triedFallback) { try { fs.writeFileSync(PORT_FILE, String(PORT)); } catch (e) { /* best effort */ } if (TOKEN_FILE) { - try { fs.writeFileSync(TOKEN_FILE, TOKEN, { mode: 0o600 }); } catch (e) { /* best effort */ } + try { + fs.writeFileSync(TOKEN_FILE, TOKEN, { mode: 0o600 }); + chmodOwnerOnly(TOKEN_FILE); + } catch (e) { /* best effort */ } } } const info = JSON.stringify({ diff --git a/skills/brainstorming/scripts/stop-server.sh b/skills/brainstorming/scripts/stop-server.sh index 355c8e0e..7cacfe94 100755 --- a/skills/brainstorming/scripts/stop-server.sh +++ b/skills/brainstorming/scripts/stop-server.sh @@ -17,6 +17,12 @@ STATE_DIR="${SESSION_DIR}/state" PID_FILE="${STATE_DIR}/server.pid" SERVER_ID_FILE="${STATE_DIR}/server-instance-id" +mark_stopped() { + local reason="$1" + rm -f "${STATE_DIR}/server-info" + printf '{"reason":"%s","timestamp":%s}\n' "$reason" "$(date +%s)" > "${STATE_DIR}/server-stopped" +} + read_expected_server_id() { [[ -f "$SERVER_ID_FILE" ]] || return 1 local id @@ -71,6 +77,7 @@ if [[ -f "$PID_FILE" ]]; then # point at an unrelated process after a reboot/PID wraparound. if ! is_brainstorm_server "$pid"; then rm -f "$PID_FILE" "$SERVER_ID_FILE" + mark_stopped "stale_pid" echo '{"status": "stale_pid"}' exit 0 fi @@ -100,6 +107,7 @@ if [[ -f "$PID_FILE" ]]; then fi rm -f "$PID_FILE" "$SERVER_ID_FILE" "${STATE_DIR}/server.log" + mark_stopped "stop-server.sh" # Only delete ephemeral /tmp directories if [[ "$SESSION_DIR" == /tmp/* ]]; then diff --git a/tests/brainstorm-server/lifecycle.test.js b/tests/brainstorm-server/lifecycle.test.js index 236c51f8..c43d8c0d 100644 --- a/tests/brainstorm-server/lifecycle.test.js +++ b/tests/brainstorm-server/lifecycle.test.js @@ -243,6 +243,41 @@ async function runTests() { assert.strictEqual(keyB, keyA, 'restart should reuse the same session key'); }); + await test('hardens existing persisted token file permissions', async () => { + const dir = fs.mkdtempSync('/tmp/bs-token-mode-'); + const portFile = path.join(dir, '.last-port'); + const tokenFile = path.join(dir, '.last-token'); + const token = 'efefefefefefefefefefefefefefefef'; + let srv = null; + + try { + fs.writeFileSync(tokenFile, token, { mode: 0o644 }); + fs.chmodSync(tokenFile, 0o644); + srv = spawn('node', [SERVER], { + env: { + ...process.env, + BRAINSTORM_DIR: path.join(dir, 's1'), + BRAINSTORM_PORT_FILE: portFile, + BRAINSTORM_TOKEN_FILE: tokenFile, + BRAINSTORM_LIFECYCLE_CHECK_MS: 100000 + } + }); + let out = ''; srv.stdout.on('data', d => out += d.toString()); + for (let i = 0; i < 60 && !out.includes('server-started'); i++) await sleep(50); + assert(out.includes('server-started'), 'server should start with persisted token'); + + if (process.platform !== 'win32') { + const mode = fs.statSync(tokenFile).mode & 0o777; + assert.strictEqual(mode, 0o600, `.last-token mode should be 0600, got ${mode.toString(8)}`); + } else { + assert(fs.existsSync(tokenFile), 'token file should remain present on Windows'); + } + } finally { + await killAndWait(srv); + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + 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'); diff --git a/tests/brainstorm-server/stop-server.test.sh b/tests/brainstorm-server/stop-server.test.sh index 9362ab1f..cbe89837 100755 --- a/tests/brainstorm-server/stop-server.test.sh +++ b/tests/brainstorm-server/stop-server.test.sh @@ -84,6 +84,37 @@ else esac fi +# --- Test 2b: persistent sessions stop with explicit stopped metadata --- +SESS="$(mktemp -d "$SCRIPT_DIR/.stop-persistent.XXXXXX")"; track_dir "$SESS"; mkdir -p "$SESS/content" "$SESS/state" +SERVER_ID="$(new_server_id)" +printf '%s\n' "$SERVER_ID" > "$SESS/state/server-instance-id" +BRAINSTORM_DIR="$SESS" BRAINSTORM_PORT=0 node "$SERVER" "--brainstorm-server-id=$SERVER_ID" > /dev/null 2>&1 & +SRV=$! +track_pid "$SRV" +disown "$SRV" 2>/dev/null || true +for _ in $(seq 1 40); do + [[ -f "$SESS/state/server-info" ]] && break + sleep 0.1 +done +echo "$SRV" > "$SESS/state/server.pid" +OUT="$("$STOP" "$SESS")" +sleep 0.3 +if kill -0 "$SRV" 2>/dev/null; then + bad "persistent brainstorm server still running after stop" "$OUT" +else + wait "$SRV" 2>/dev/null || true + untrack_pid "$SRV" + if [[ -f "$SESS/state/server-info" ]]; then + bad "persistent stop clears server-info" "server-info still exists after: $OUT" + elif [[ ! -f "$SESS/state/server-stopped" ]]; then + bad "persistent stop writes server-stopped" "server-stopped missing after: $OUT" + elif grep -q '"reason":"stop-server.sh"' "$SESS/state/server-stopped"; then + ok "persistent stop clears alive metadata and writes server-stopped" + else + bad "persistent stop writes stop reason" "$(cat "$SESS/state/server-stopped" 2>/dev/null || true)" + fi +fi + # --- Test 3: no pid file --- SESS="$(mktemp -d)"; track_dir "$SESS"; mkdir -p "$SESS/state" OUT="$("$STOP" "$SESS")"