SUPER-752 host auth refresh

apps/desktop/src/main/lib/host-service-coordinator.ts

@@ -467,6 +467,7 @@ export class HostServiceCoordinator extends EventEmitter {
 			SUPERSET_AGENT_HOOK_PORT: String(sharedEnv.DESKTOP_NOTIFICATIONS_PORT),
 			SUPERSET_AGENT_HOOK_VERSION: HOOK_PROTOCOL_VERSION,
 			AUTH_TOKEN: config.authToken,
+			SUPERSET_AUTH_CONFIG_PATH: path.join(SUPERSET_HOME_DIR, "config.json"),
 			SUPERSET_API_URL: config.cloudApiUrl,
 			// Read by the child's parent watchdog so it can self-exit if
 			// Electron crashes without sending SIGTERM (orphan reparenting).

packages/cli/src/commands/start/command.ts

@@ -1,6 +1,7 @@
 import * as p from "@clack/prompts";
 import { boolean, CLIError, number } from "@superset/cli-framework";
 import { command } from "../../lib/command";
+import { SUPERSET_CONFIG_PATH } from "../../lib/config";
 import { isProcessAlive, readManifest } from "../../lib/host/manifest";
 import { spawnHostService } from "../../lib/host/spawn";
 
@@ -31,6 +32,8 @@ export default command({
 			const result = await spawnHostService({
 				organizationId: organization.id,
 				sessionToken: ctx.bearer,
+				authConfigPath:
+					ctx.authSource === "oauth" ? SUPERSET_CONFIG_PATH : undefined,
 				api: ctx.api,
 				port: options.port,
 				daemon: options.daemon ?? false,

packages/cli/src/lib/auth.test.ts

@@ -0,0 +1,44 @@
+import { afterEach, describe, expect, mock, test } from "bun:test";
+import { CLIError } from "@superset/cli-framework";
+import { refreshAccessToken } from "./auth";
+
+const originalFetch = globalThis.fetch;
+
+afterEach(() => {
+	globalThis.fetch = originalFetch;
+});
+
+describe("refreshAccessToken", () => {
+	test("sanitizes OAuth refresh failure details", async () => {
+		globalThis.fetch = mock(
+			async () =>
+				new Response(
+					JSON.stringify({
+						error: "invalid_grant",
+						access_token: "access-secret",
+						refresh_token: "refresh-secret",
+						redirect: "https://app.superset.test/callback?code=code-secret",
+						cookie: "session=session-secret",
+					}),
+					{ status: 400 },
+				),
+		) as unknown as typeof fetch;
+
+		let thrown: unknown;
+		try {
+			await refreshAccessToken("refresh-secret");
+		} catch (error) {
+			thrown = error;
+		}
+
+		expect(thrown).toBeInstanceOf(CLIError);
+		const error = thrown as CLIError;
+		const visibleText = `${error.message} ${error.suggestion ?? ""}`;
+		expect(visibleText).toContain("Token refresh failed: 400");
+		expect(visibleText).toContain("superset auth login");
+		expect(visibleText).not.toContain("access-secret");
+		expect(visibleText).not.toContain("refresh-secret");
+		expect(visibleText).not.toContain("session-secret");
+		expect(visibleText).not.toContain("code-secret");
+	});
+});

packages/cli/src/lib/auth.ts

@@ -8,6 +8,7 @@ const PASTE_REDIRECT_PATH = "/cli/auth/code";
 const SCOPE = "openid profile email offline_access";
 const LOOPBACK_PORTS = [51789, 51790, 51791, 51792, 51793];
 const CALLBACK_TIMEOUT_MS = 5 * 60 * 1000;
+const LOGIN_AGAIN_SUGGESTION = "Run `superset auth login` again.";
 
 export interface LoginResult {
 	accessToken: string;
@@ -222,10 +223,9 @@ async function exchangeCodeForToken({
 	});
 
 	if (!response.ok) {
-		const body = await response.text();
 		throw new CLIError(
 			`Token exchange failed: ${response.status}`,
-			body || "Run `superset auth login` again.",
+			LOGIN_AGAIN_SUGGESTION,
 		);
 	}
 
@@ -260,10 +260,9 @@ export async function refreshAccessToken(
 	});
 
 	if (!response.ok) {
-		const body = await response.text();
 		throw new CLIError(
 			`Token refresh failed: ${response.status}`,
-			body || "Run `superset auth login` again.",
+			LOGIN_AGAIN_SUGGESTION,
 		);
 	}
 

packages/cli/src/lib/config.test.ts

@@ -0,0 +1,115 @@
+import { afterAll, beforeEach, describe, expect, mock, test } from "bun:test";
+import * as nodeFs from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+
+// Snapshot real fs functions BEFORE `mock.module` so test setup + assertions
+// keep working even though `node:fs` is about to be replaced for the SUT.
+const realFs = {
+	chmodSync: nodeFs.chmodSync,
+	existsSync: nodeFs.existsSync,
+	mkdirSync: nodeFs.mkdirSync,
+	mkdtempSync: nodeFs.mkdtempSync,
+	readFileSync: nodeFs.readFileSync,
+	renameSync: nodeFs.renameSync,
+	rmSync: nodeFs.rmSync,
+	statSync: nodeFs.statSync,
+	unlinkSync: nodeFs.unlinkSync,
+	writeFileSync: nodeFs.writeFileSync,
+};
+
+const originalSupersetHomeDir = process.env.SUPERSET_HOME_DIR;
+const tempHome = realFs.mkdtempSync(join(tmpdir(), "superset-cli-config-"));
+process.env.SUPERSET_HOME_DIR = tempHome;
+
+// Per-test mutable state for the mocked fs.
+let renameShouldFail = false;
+const writtenPaths: string[] = [];
+const unlinkedPaths: string[] = [];
+
+mock.module("node:fs", () => ({
+	...nodeFs,
+	writeFileSync: (
+		path: nodeFs.PathOrFileDescriptor,
+		data: string | NodeJS.ArrayBufferView,
+		options?: nodeFs.WriteFileOptions,
+	) => {
+		writtenPaths.push(String(path));
+		return realFs.writeFileSync(path, data, options);
+	},
+	renameSync: (oldPath: nodeFs.PathLike, newPath: nodeFs.PathLike) => {
+		if (renameShouldFail) throw new Error("rename failed");
+		return realFs.renameSync(oldPath, newPath);
+	},
+	unlinkSync: (path: nodeFs.PathLike) => {
+		unlinkedPaths.push(String(path));
+		return realFs.unlinkSync(path);
+	},
+}));
+
+const { SUPERSET_CONFIG_PATH, writeConfig } = await import("./config");
+
+beforeEach(() => {
+	writtenPaths.length = 0;
+	unlinkedPaths.length = 0;
+	renameShouldFail = false;
+	if (realFs.existsSync(SUPERSET_CONFIG_PATH)) {
+		realFs.unlinkSync(SUPERSET_CONFIG_PATH);
+	}
+});
+
+afterAll(() => {
+	realFs.rmSync(tempHome, { recursive: true, force: true });
+	if (originalSupersetHomeDir === undefined) {
+		delete process.env.SUPERSET_HOME_DIR;
+	} else {
+		process.env.SUPERSET_HOME_DIR = originalSupersetHomeDir;
+	}
+});
+
+describe("config writes", () => {
+	test("writeConfig uses unique temp files", () => {
+		writeConfig({ apiKey: "sk_live_one" });
+		writeConfig({ apiKey: "sk_live_two" });
+
+		const tempWrites = writtenPaths.filter((p) => p.endsWith(".config.tmp"));
+		expect(tempWrites).toHaveLength(2);
+		expect(tempWrites[0]).not.toBe(tempWrites[1]);
+		expect(
+			JSON.parse(realFs.readFileSync(SUPERSET_CONFIG_PATH, "utf-8")),
+		).toEqual({
+			apiKey: "sk_live_two",
+		});
+	});
+
+	test("writeConfig preserves old config if rename fails", () => {
+		realFs.writeFileSync(
+			SUPERSET_CONFIG_PATH,
+			JSON.stringify({ apiKey: "sk_live_old" }),
+		);
+
+		renameShouldFail = true;
+
+		expect(() => writeConfig({ apiKey: "sk_live_new" })).toThrow(
+			/rename failed/,
+		);
+
+		expect(
+			JSON.parse(realFs.readFileSync(SUPERSET_CONFIG_PATH, "utf-8")),
+		).toEqual({
+			apiKey: "sk_live_old",
+		});
+		expect(unlinkedPaths).toHaveLength(1);
+		expect(realFs.existsSync(unlinkedPaths[0] ?? "")).toBe(false);
+	});
+
+	test("writeConfig writes the exported Superset config path", () => {
+		writeConfig({ organizationId: "org_123" });
+
+		expect(
+			JSON.parse(realFs.readFileSync(SUPERSET_CONFIG_PATH, "utf-8")),
+		).toEqual({
+			organizationId: "org_123",
+		});
+	});
+});

packages/cli/src/lib/config.ts

@@ -1,9 +1,12 @@
+import { randomUUID } from "node:crypto";
 import {
 	chmodSync,
 	existsSync,
 	mkdirSync,
 	readFileSync,
+	renameSync,
 	statSync,
+	unlinkSync,
 	writeFileSync,
 } from "node:fs";
 import { homedir } from "node:os";
@@ -22,7 +25,7 @@ export type SupersetConfig = {
 
 export const SUPERSET_HOME_DIR =
 	process.env.SUPERSET_HOME_DIR ?? join(homedir(), ".superset");
-const CONFIG_PATH = join(SUPERSET_HOME_DIR, "config.json");
+export const SUPERSET_CONFIG_PATH = join(SUPERSET_HOME_DIR, "config.json");
 
 function ensureDir() {
 	if (!existsSync(SUPERSET_HOME_DIR)) {
@@ -35,21 +38,34 @@ function ensureDir() {
 }
 
 export function readConfig(): SupersetConfig {
-	if (!existsSync(CONFIG_PATH)) return {};
+	if (!existsSync(SUPERSET_CONFIG_PATH)) return {};
 	try {
-		const stat = statSync(CONFIG_PATH);
-		if ((stat.mode & 0o077) !== 0) chmodSync(CONFIG_PATH, 0o600);
+		const stat = statSync(SUPERSET_CONFIG_PATH);
+		if ((stat.mode & 0o077) !== 0) chmodSync(SUPERSET_CONFIG_PATH, 0o600);
 	} catch {}
-	return JSON.parse(readFileSync(CONFIG_PATH, "utf-8"));
+	return JSON.parse(readFileSync(SUPERSET_CONFIG_PATH, "utf-8"));
 }
 
 export function writeConfig(config: SupersetConfig): void {
 	ensureDir();
-	writeFileSync(CONFIG_PATH, JSON.stringify(config, null, 2), {
-		mode: 0o600,
-	});
+	const tempPath = join(
+		SUPERSET_HOME_DIR,
+		`.${randomUUID()}.${process.pid}.config.tmp`,
+	);
+	writeFileSync(tempPath, JSON.stringify(config, null, 2), { mode: 0o600 });
 	try {
-		chmodSync(CONFIG_PATH, 0o600);
+		chmodSync(tempPath, 0o600);
+	} catch {}
+	try {
+		renameSync(tempPath, SUPERSET_CONFIG_PATH);
+	} catch (error) {
+		try {
+			unlinkSync(tempPath);
+		} catch {}
+		throw error;
+	}
+	try {
+		chmodSync(SUPERSET_CONFIG_PATH, 0o600);
 	} catch {}
 }
 

packages/cli/src/lib/host/spawn.test.ts

@@ -0,0 +1,98 @@
+import { afterAll, afterEach, describe, expect, mock, test } from "bun:test";
+import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import type { ApiClient } from "../api-client";
+
+const originalFetch = globalThis.fetch;
+const originalSupersetHomeDir = process.env.SUPERSET_HOME_DIR;
+const originalHostBin = process.env.SUPERSET_HOST_BIN;
+const tempHome = mkdtempSync(join(tmpdir(), "superset-cli-spawn-"));
+const hostBin = join(tempHome, "superset-host");
+
+process.env.SUPERSET_HOME_DIR = tempHome;
+process.env.SUPERSET_HOST_BIN = hostBin;
+writeFileSync(hostBin, "");
+
+type SpawnOptions = {
+	env?: NodeJS.ProcessEnv;
+	detached?: boolean;
+	stdio?: unknown;
+};
+
+const spawnCalls: Array<{
+	command: string;
+	args: string[];
+	options: SpawnOptions;
+}> = [];
+
+const spawnMock = mock(
+	(command: string, args: string[], options: SpawnOptions) => {
+		spawnCalls.push({ command, args, options });
+		return {
+			pid: 12345,
+			kill: mock(() => undefined),
+			unref: mock(() => undefined),
+		};
+	},
+);
+
+mock.module("node:child_process", () => ({
+	spawn: spawnMock,
+}));
+
+const { SUPERSET_CONFIG_PATH } = await import("../config");
+const { spawnHostService } = await import("./spawn");
+
+function createApi(): ApiClient {
+	return {
+		analytics: {
+			featureFlagPayload: {
+				query: async () => null,
+			},
+		},
+	} as unknown as ApiClient;
+}
+
+afterEach(() => {
+	spawnCalls.length = 0;
+	spawnMock.mockClear();
+	globalThis.fetch = originalFetch;
+});
+
+afterAll(() => {
+	rmSync(tempHome, { recursive: true, force: true });
+	if (originalSupersetHomeDir === undefined) {
+		delete process.env.SUPERSET_HOME_DIR;
+	} else {
+		process.env.SUPERSET_HOME_DIR = originalSupersetHomeDir;
+	}
+	if (originalHostBin === undefined) {
+		delete process.env.SUPERSET_HOST_BIN;
+	} else {
+		process.env.SUPERSET_HOST_BIN = originalHostBin;
+	}
+});
+
+describe("spawnHostService", () => {
+	test("passes SUPERSET_AUTH_CONFIG_PATH when provided", async () => {
+		globalThis.fetch = mock(
+			async () => new Response("ok", { status: 200 }),
+		) as unknown as typeof fetch;
+
+		await spawnHostService({
+			organizationId: "00000000-0000-0000-0000-000000000001",
+			sessionToken: "session-token",
+			authConfigPath: SUPERSET_CONFIG_PATH,
+			api: createApi(),
+			port: 54879,
+			daemon: true,
+		});
+
+		expect(spawnMock).toHaveBeenCalledTimes(1);
+		expect(spawnCalls[0]?.options.env?.SUPERSET_AUTH_CONFIG_PATH).toBe(
+			SUPERSET_CONFIG_PATH,
+		);
+		expect(spawnCalls[0]?.options.env?.AUTH_TOKEN).toBe("session-token");
+	});
+});

packages/cli/src/lib/host/spawn.ts

@@ -18,6 +18,7 @@ const HEALTH_POLL_TIMEOUT_MS = 10_000;
 export interface SpawnHostOptions {
 	organizationId: string;
 	sessionToken: string;
+	authConfigPath?: string;
 	api: ApiClient;
 	port?: number;
 	daemon: boolean;
@@ -110,6 +111,9 @@ export async function spawnHostService(
 			...process.env,
 			ORGANIZATION_ID: options.organizationId,
 			AUTH_TOKEN: options.sessionToken,
+			...(options.authConfigPath
+				? { SUPERSET_AUTH_CONFIG_PATH: options.authConfigPath }
+				: {}),
 			SUPERSET_API_URL: env.SUPERSET_API_URL,
 			RELAY_URL: relayUrl,
 			PORT: String(port),