From ea15fd77a9f7d15b0704cb56643112d1c5aa977c Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Wed, 18 Mar 2026 08:16:34 +0000 Subject: [PATCH] fix(desktop): restart terminal-host daemon on app startup to fix stale macOS security session On macOS with Fast User Switching, the terminal-host daemon can be spawned while the user is a background user, inheriting a degraded security context. When the user becomes the console owner and relaunches Superset, the app reuses the stale daemon, causing all Go binaries (gh, etc.) and Security.framework calls to fail with TLS/Keychain errors. Fix: always restart the daemon on app startup via reconcileOnStartup() so it inherits the current user's security session. The cold restore mechanism recovers terminal scrollback from disk history. Closes #2570 --- .../src/main/lib/terminal-host/client.test.ts | 303 ++++++++++++++++++ .../src/main/lib/terminal-host/client.ts | 59 ++++ .../terminal/daemon/daemon-manager.test.ts | 17 + .../lib/terminal/daemon/daemon-manager.ts | 7 + 4 files changed, 386 insertions(+) create mode 100644 apps/desktop/src/main/lib/terminal-host/client.test.ts diff --git a/apps/desktop/src/main/lib/terminal-host/client.test.ts b/apps/desktop/src/main/lib/terminal-host/client.test.ts new file mode 100644 index 00000000000..76015fd57ec --- /dev/null +++ b/apps/desktop/src/main/lib/terminal-host/client.test.ts @@ -0,0 +1,303 @@ +import { beforeEach, describe, expect, it, mock, spyOn } from "bun:test"; +import { EventEmitter } from "node:events"; + +/** + * Tests for TerminalHostClient.restartDaemon() + * + * Verifies that on app restart, the client shuts down any existing daemon + * and spawns a fresh one. This ensures the new daemon inherits the current + * user's security session context — critical on macOS where Fast User + * Switching can leave a stale daemon with a degraded security context + * (causing TLS/Keychain failures for Go binaries, `security` CLI, etc.). + * + * See: https://github.com/anthropics/superset/issues/2570 + */ + +// --------------------------------------------------------------------------- +// Mocks +// --------------------------------------------------------------------------- + +const fsState = { + existingPaths: new Set(), + writtenFiles: new Map(), + unlinkedPaths: new Set(), + readFiles: new Map(), +}; + +mock.module("node:fs", () => { + const realFs = require("node:fs"); + const overrides = { + existsSync: (path: string) => fsState.existingPaths.has(path), + readFileSync: (path: string, ...args: unknown[]) => { + const content = fsState.readFiles.get(path); + if (content !== undefined) return content; + return realFs.readFileSync(path, ...args); + }, + writeFileSync: (path: string, data: string) => { + fsState.writtenFiles.set(path, data); + fsState.existingPaths.add(path); + }, + unlinkSync: (path: string) => { + fsState.unlinkedPaths.add(path); + fsState.existingPaths.delete(path); + }, + mkdirSync: () => {}, + chmodSync: () => {}, + statSync: () => ({ mtimeMs: 0 }), + openSync: () => 3, + closeSync: () => {}, + }; + return { + ...realFs, + ...overrides, + default: { ...realFs, ...overrides }, + }; +}); + +let connectFn: (path: string) => EventEmitter; + +mock.module("node:net", () => ({ + connect: (path: string) => connectFn(path), +})); + +mock.module("electron", () => ({ + app: { + isPackaged: false, + getAppPath: () => "/mock/app", + }, +})); + +mock.module("shared/constants", () => ({ + SUPERSET_DIR_NAME: ".superset-test", +})); + +const spawnCalls: Array<{ cmd: string; args: string[] }> = []; + +/** Called by spawn mock to simulate daemon creating its socket file. */ +let onSpawn: (() => void) | null = null; + +const mockChildProcess = new EventEmitter(); +Object.assign(mockChildProcess, { + pid: 99999, + unref: () => {}, +}); + +mock.module("node:child_process", () => { + const realCp = require("node:child_process"); + const overrides = { + spawn: (cmd: string, args: string[]) => { + spawnCalls.push({ cmd, args }); + onSpawn?.(); + return mockChildProcess; + }, + }; + return { ...realCp, ...overrides, default: { ...realCp, ...overrides } }; +}); + +const { TerminalHostClient } = await import("./client"); + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +const HOME = require("node:os").homedir(); +const SUPERSET_HOME = require("node:path").join(HOME, ".superset-test"); +const SOCKET_PATH = require("node:path").join( + SUPERSET_HOME, + "terminal-host.sock", +); +const TOKEN_PATH = require("node:path").join( + SUPERSET_HOME, + "terminal-host.token", +); +const PID_PATH = require("node:path").join(SUPERSET_HOME, "terminal-host.pid"); +const DAEMON_SCRIPT = "/mock/app/dist/main/terminal-host.js"; + +function resetState() { + fsState.existingPaths.clear(); + fsState.writtenFiles.clear(); + fsState.unlinkedPaths.clear(); + fsState.readFiles.clear(); + spawnCalls.length = 0; + onSpawn = null; +} + +function createMockSocket( + responseMap: Record = {}, +): EventEmitter { + const socket = new EventEmitter() as EventEmitter & { + write: (data: string) => boolean; + destroy: () => void; + unref: () => void; + setEncoding: (enc: string) => void; + remoteAddress: string; + }; + socket.remoteAddress = "mock"; + socket.unref = () => {}; + socket.setEncoding = () => {}; + socket.destroy = () => {}; + + socket.write = (data: string) => { + try { + const req = JSON.parse(data.trim()); + const payload = responseMap[req.type]; + if (payload !== undefined) { + const response = JSON.stringify({ + id: req.id, + ok: true, + payload, + }); + setTimeout(() => socket.emit("data", `${response}\n`), 0); + } + } catch { + // Not NDJSON + } + return true; + }; + + setTimeout(() => socket.emit("connect"), 0); + return socket; +} + +function createFailingSocket(): EventEmitter { + const socket = new EventEmitter() as EventEmitter & { + write: (data: string) => boolean; + destroy: () => void; + unref: () => void; + setEncoding: (enc: string) => void; + }; + socket.write = () => true; + socket.destroy = () => {}; + socket.unref = () => {}; + socket.setEncoding = () => {}; + setTimeout(() => socket.emit("error", new Error("ECONNREFUSED")), 0); + return socket; +} + +const HELLO_RESPONSE = { + protocolVersion: 2, + daemonVersion: "1.0.0", + daemonPid: 99999, +}; + +/** Set up onSpawn to simulate the daemon creating its socket + token files. */ +function simulateDaemonSpawn(token = "new-token-xyz") { + onSpawn = () => { + fsState.existingPaths.add(SOCKET_PATH); + fsState.existingPaths.add(TOKEN_PATH); + fsState.readFiles.set(TOKEN_PATH, token); + }; +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe("TerminalHostClient.restartDaemon", () => { + beforeEach(() => { + resetState(); + }); + + it("shuts down a running daemon and removes stale files before spawning fresh", async () => { + // Existing daemon files + fsState.existingPaths.add(SOCKET_PATH); + fsState.existingPaths.add(TOKEN_PATH); + fsState.existingPaths.add(PID_PATH); + fsState.existingPaths.add(DAEMON_SCRIPT); + fsState.readFiles.set(TOKEN_PATH, "test-token-abc"); + fsState.readFiles.set(PID_PATH, "12345"); + + simulateDaemonSpawn(); + + let connectCount = 0; + connectFn = () => { + connectCount++; + + // 1st connect: tryConnectControl for graceful shutdown + if (connectCount === 1) { + return createMockSocket({ + hello: HELLO_RESPONSE, + shutdown: { success: true }, + }); + } + + // 2nd: waitForDaemonShutdown isSocketLive check — daemon gone + if (connectCount === 2) { + fsState.existingPaths.delete(SOCKET_PATH); + return createFailingSocket(); + } + + // 3rd+: new daemon sockets (control + stream after spawn) + return createMockSocket({ hello: HELLO_RESPONSE }); + }; + + const client = new TerminalHostClient(); + await client.restartDaemon(); + + // Graceful shutdown was sent (socket accepted the shutdown request) + // and old token file was cleaned up (removed either by daemon or cleanup) + expect(fsState.unlinkedPaths.has(TOKEN_PATH)).toBe(true); + + // New daemon was spawned + expect(spawnCalls.length).toBeGreaterThanOrEqual(1); + expect(spawnCalls[0]?.args[0]).toContain("terminal-host.js"); + + client.dispose(); + }); + + it("falls back to SIGTERM via PID file when socket is not connectable", async () => { + fsState.existingPaths.add(SOCKET_PATH); + fsState.existingPaths.add(PID_PATH); + fsState.existingPaths.add(TOKEN_PATH); + fsState.existingPaths.add(DAEMON_SCRIPT); + fsState.readFiles.set(PID_PATH, "12345"); + + const killSpy = spyOn(process, "kill").mockImplementation(() => true); + simulateDaemonSpawn(); + + let connectCount = 0; + connectFn = () => { + connectCount++; + + // 1st: tryConnectControl fails (daemon unresponsive) + if (connectCount === 1) { + return createFailingSocket(); + } + + // 2nd: waitForDaemonShutdown — socket gone + if (connectCount === 2) { + fsState.existingPaths.delete(SOCKET_PATH); + return createFailingSocket(); + } + + // 3rd+: new daemon + return createMockSocket({ hello: HELLO_RESPONSE }); + }; + + const client = new TerminalHostClient(); + await client.restartDaemon(); + + expect(killSpy).toHaveBeenCalledWith(12345, "SIGTERM"); + expect(spawnCalls.length).toBeGreaterThanOrEqual(1); + + killSpy.mockRestore(); + client.dispose(); + }); + + it("spawns fresh daemon when no existing daemon is running", async () => { + // No socket/pid/token files — clean state + fsState.existingPaths.add(DAEMON_SCRIPT); + simulateDaemonSpawn("fresh-token"); + + connectFn = () => { + // All connects go to the freshly spawned daemon + return createMockSocket({ hello: HELLO_RESPONSE }); + }; + + const client = new TerminalHostClient(); + await client.restartDaemon(); + + expect(spawnCalls.length).toBeGreaterThanOrEqual(1); + client.dispose(); + }); +}); diff --git a/apps/desktop/src/main/lib/terminal-host/client.ts b/apps/desktop/src/main/lib/terminal-host/client.ts index d69e946f9f4..0ce0b222163 100644 --- a/apps/desktop/src/main/lib/terminal-host/client.ts +++ b/apps/desktop/src/main/lib/terminal-host/client.ts @@ -1411,6 +1411,65 @@ export class TerminalHostClient extends EventEmitter { return response; } + /** + * Restart the daemon by shutting down any existing instance and spawning a fresh one. + * This ensures the new daemon inherits the current user's security session context, + * which is critical on macOS when Fast User Switching changes the console owner. + * + * Sessions in the old daemon are lost, but the cold restore mechanism recovers + * terminal scrollback from disk history. + */ + async restartDaemon(): Promise { + // Disconnect any existing sockets first + this.resetConnectionState({ emitDisconnected: false }); + + // Try graceful shutdown first + const connected = await this.tryConnectControl(); + if (connected) { + try { + const token = this.readAuthToken(); + try { + await this.authenticateControl({ token }); + await this.sendRequest("shutdown", { + killSessions: false, + }); + } catch (error) { + if (this.isProtocolMismatchError(error)) { + this.resetConnectionState({ emitDisconnected: false }); + await this.shutdownLegacyDaemon({ killSessions: false }); + } else { + // Auth or send failed - fall back to SIGTERM + this.killDaemonFromPidFile(); + } + } + } catch { + // Token missing - fall back to SIGTERM + this.killDaemonFromPidFile(); + } finally { + this.resetConnectionState({ emitDisconnected: false }); + } + } else { + // Socket not connectable but may still exist - try SIGTERM + this.killDaemonFromPidFile(); + } + + await this.waitForDaemonShutdown(); + + // Clean up stale files + for (const path of [SOCKET_PATH, PID_PATH, TOKEN_PATH]) { + try { + if (existsSync(path)) unlinkSync(path); + } catch { + // Best effort + } + } + + // Spawn fresh daemon and connect + await this.connectAndAuthenticate(); + this.connectionState = ConnectionState.CONNECTED; + this.emit("connected"); + } + /** * Shutdown the daemon if it's currently running, without spawning a new one. * Returns true if daemon was running and shutdown was sent, false if no daemon was running. diff --git a/apps/desktop/src/main/lib/terminal/daemon/daemon-manager.test.ts b/apps/desktop/src/main/lib/terminal/daemon/daemon-manager.test.ts index 23ac8963cad..69bb49fb945 100644 --- a/apps/desktop/src/main/lib/terminal/daemon/daemon-manager.test.ts +++ b/apps/desktop/src/main/lib/terminal/daemon/daemon-manager.test.ts @@ -4,6 +4,7 @@ import type { SessionInfo } from "./types"; class MockTerminalHostClient extends EventEmitter { killCalls: Array<{ sessionId: string; deleteHistory?: boolean }> = []; + restartDaemonCalls = 0; async kill(params: { sessionId: string; deleteHistory?: boolean }) { this.killCalls.push(params); @@ -13,6 +14,10 @@ class MockTerminalHostClient extends EventEmitter { return { sessions: [] }; } + async restartDaemon() { + this.restartDaemonCalls++; + } + writeNoAck() {} resize() { return Promise.resolve(); @@ -60,6 +65,18 @@ mock.module("@superset/local-db", () => ({ const { DaemonTerminalManager } = await import("./daemon-manager"); +describe("DaemonTerminalManager.reconcileOnStartup", () => { + beforeEach(() => { + mockClient = new MockTerminalHostClient(); + }); + + it("restarts the daemon to inherit current security session context", async () => { + const manager = new DaemonTerminalManager(); + await manager.reconcileOnStartup(); + expect(mockClient.restartDaemonCalls).toBe(1); + }); +}); + describe("DaemonTerminalManager kill tracking", () => { beforeEach(() => { mockClient = new MockTerminalHostClient(); diff --git a/apps/desktop/src/main/lib/terminal/daemon/daemon-manager.ts b/apps/desktop/src/main/lib/terminal/daemon/daemon-manager.ts index 630980c5519..6d366607fcf 100644 --- a/apps/desktop/src/main/lib/terminal/daemon/daemon-manager.ts +++ b/apps/desktop/src/main/lib/terminal/daemon/daemon-manager.ts @@ -78,6 +78,13 @@ export class DaemonTerminalManager extends EventEmitter { async reconcileOnStartup(): Promise { try { + // Always restart the daemon on app startup to ensure it inherits + // the current user's security session context. This is critical on + // macOS where Fast User Switching can leave a stale daemon with a + // degraded security context (causing TLS/Keychain failures). + // Sessions are recovered via cold restore from disk history. + await this.client.restartDaemon(); + const response = await this.client.listSessions(); if (response.sessions.length === 0) { this.daemonAliveSessionIds.clear();