diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f4cd1063fc..c969315de09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,7 +78,10 @@ Docs: https://docs.openclaw.ai - Discord/ack reactions: keep automatic ACK reaction auth on the active hydrated Discord account so SecretRef-backed and non-default-account reactions stop falling back to stale default config resolution. (#60081) Thanks @FunJim. - Telegram/model switching: render non-default `/model` callback confirmations with HTML formatting so Telegram shows the selected model in bold instead of raw `**...**` markers. (#60042) Thanks @GitZhangChi. - Plugins/update: allow `openclaw plugins update` to use `--dangerously-force-unsafe-install` for built-in dangerous-code false positives during plugin updates. (#60066) Thanks @huntharo. +<<<<<<< HEAD +- Gateway/auth: disconnect shared-auth websocket sessions only for effective auth rotations on restart-capable config writes, and keep `config.set` auth edits from dropping still-valid live sessions. (#60387) Thanks @mappel-nv. - Control UI/chat: keep the Stop button visible during tool-only execution so abortable runs do not fall back to Send while tools are still running. (#54528) thanks @chziyue. +- Gateway/auth: disconnect shared-auth websocket sessions only for effective auth rotations on restart-capable config writes, and keep `config.set` auth edits from dropping still-valid live sessions. (#60387) Thanks @mappel-nv. ## 2026.4.2 diff --git a/src/gateway/gateway-misc.test.ts b/src/gateway/gateway-misc.test.ts index 0c51339b681..954aa65c623 100644 --- a/src/gateway/gateway-misc.test.ts +++ b/src/gateway/gateway-misc.test.ts @@ -203,16 +203,19 @@ describe("gateway broadcaster", () => { socket: approvalsSocket as unknown as GatewayWsClient["socket"], connect: { role: "operator", scopes: ["operator.approvals"] } as GatewayWsClient["connect"], connId: "c-approvals", + usesSharedGatewayAuth: false, }, { socket: pairingSocket as unknown as GatewayWsClient["socket"], connect: { role: "operator", scopes: ["operator.pairing"] } as GatewayWsClient["connect"], connId: "c-pairing", + usesSharedGatewayAuth: false, }, { socket: readSocket as unknown as GatewayWsClient["socket"], connect: { role: "operator", scopes: ["operator.read"] } as GatewayWsClient["connect"], connId: "c-read", + usesSharedGatewayAuth: false, }, ]); diff --git a/src/gateway/server-methods/config.shared-auth.test.ts b/src/gateway/server-methods/config.shared-auth.test.ts new file mode 100644 index 00000000000..22bc9467661 --- /dev/null +++ b/src/gateway/server-methods/config.shared-auth.test.ts @@ -0,0 +1,181 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { OpenClawConfig } from "../../config/types.openclaw.js"; +import type { GatewayRequestHandlerOptions } from "./types.js"; + +const readConfigFileSnapshotForWriteMock = vi.fn(); +const writeConfigFileMock = vi.fn(); +const validateConfigObjectWithPluginsMock = vi.fn(); +const prepareSecretsRuntimeSnapshotMock = vi.fn(); +const scheduleGatewaySigusr1RestartMock = vi.fn(() => ({ + scheduled: true, + delayMs: 1_000, + coalesced: false, +})); + +vi.mock("../../config/config.js", async () => { + const actual = + await vi.importActual("../../config/config.js"); + return { + ...actual, + createConfigIO: () => ({ configPath: "/tmp/openclaw.json" }), + readConfigFileSnapshotForWrite: readConfigFileSnapshotForWriteMock, + validateConfigObjectWithPlugins: validateConfigObjectWithPluginsMock, + writeConfigFile: writeConfigFileMock, + }; +}); + +vi.mock("../../config/runtime-schema.js", () => ({ + loadGatewayRuntimeConfigSchema: () => ({ uiHints: undefined }), +})); + +vi.mock("../../secrets/runtime.js", () => ({ + prepareSecretsRuntimeSnapshot: prepareSecretsRuntimeSnapshotMock, +})); + +vi.mock("../../infra/restart.js", () => ({ + scheduleGatewaySigusr1Restart: scheduleGatewaySigusr1RestartMock, +})); + +const { configHandlers } = await import("./config.js"); + +function createConfigSnapshot(config: OpenClawConfig) { + return { + snapshot: { + path: "/tmp/openclaw.json", + exists: true, + raw: JSON.stringify(config, null, 2), + parsed: config, + sourceConfig: config, + resolved: config, + valid: true, + runtimeConfig: config, + config, + hash: "base-hash", + issues: [], + warnings: [], + legacyIssues: [], + }, + writeOptions: {} as Record, + }; +} + +function createOptions( + params: unknown, + contextOverrides?: Partial, +): GatewayRequestHandlerOptions { + return { + req: { type: "req", id: "1", method: "config.patch" }, + params, + client: null, + isWebchatConnect: () => false, + respond: vi.fn(), + context: { + logGateway: { + error: vi.fn(), + warn: vi.fn(), + info: vi.fn(), + debug: vi.fn(), + }, + disconnectClientsUsingSharedGatewayAuth: vi.fn(), + ...contextOverrides, + }, + } as unknown as GatewayRequestHandlerOptions; +} + +async function flushMicrotaskQueue() { + await new Promise((resolve) => queueMicrotask(resolve)); +} + +afterEach(() => { + vi.clearAllMocks(); +}); + +beforeEach(() => { + validateConfigObjectWithPluginsMock.mockImplementation((config: OpenClawConfig) => ({ + ok: true, + config, + })); + prepareSecretsRuntimeSnapshotMock.mockResolvedValue(undefined); +}); + +describe("config shared auth disconnects", () => { + it("does not disconnect shared-auth clients for config.set auth writes without restart", async () => { + const prevConfig: OpenClawConfig = { + gateway: { + auth: { + mode: "token", + token: "old-token", + }, + }, + }; + const nextConfig: OpenClawConfig = { + gateway: { + auth: { + mode: "token", + token: "new-token", + }, + }, + }; + readConfigFileSnapshotForWriteMock.mockResolvedValue(createConfigSnapshot(prevConfig)); + + const opts = createOptions({ + raw: JSON.stringify(nextConfig, null, 2), + baseHash: "base-hash", + }); + + await configHandlers["config.set"](opts); + await flushMicrotaskQueue(); + + expect(writeConfigFileMock).toHaveBeenCalledWith(nextConfig, {}); + expect(opts.context.disconnectClientsUsingSharedGatewayAuth).not.toHaveBeenCalled(); + expect(scheduleGatewaySigusr1RestartMock).not.toHaveBeenCalled(); + }); + + it("disconnects shared-auth clients after config.patch rotates the active token", async () => { + const prevConfig: OpenClawConfig = { + gateway: { + auth: { + mode: "token", + token: "old-token", + }, + }, + }; + readConfigFileSnapshotForWriteMock.mockResolvedValue(createConfigSnapshot(prevConfig)); + + const opts = createOptions({ + baseHash: "base-hash", + raw: JSON.stringify({ gateway: { auth: { token: "new-token" } } }), + restartDelayMs: 1_000, + }); + + await configHandlers["config.patch"](opts); + await flushMicrotaskQueue(); + + expect(scheduleGatewaySigusr1RestartMock).toHaveBeenCalledTimes(1); + expect(opts.context.disconnectClientsUsingSharedGatewayAuth).toHaveBeenCalledTimes(1); + }); + + it("does not disconnect shared-auth clients when config.patch changes only inactive password auth", async () => { + const prevConfig: OpenClawConfig = { + gateway: { + auth: { + mode: "token", + token: "old-token", + }, + }, + }; + readConfigFileSnapshotForWriteMock.mockResolvedValue(createConfigSnapshot(prevConfig)); + + const opts = createOptions({ + baseHash: "base-hash", + raw: JSON.stringify({ gateway: { auth: { password: "new-password" } } }), + restartDelayMs: 1_000, + }); + + await configHandlers["config.patch"](opts); + await flushMicrotaskQueue(); + + expect(scheduleGatewaySigusr1RestartMock).toHaveBeenCalledTimes(1); + expect(opts.context.disconnectClientsUsingSharedGatewayAuth).not.toHaveBeenCalled(); + }); +}); diff --git a/src/gateway/server-methods/config.ts b/src/gateway/server-methods/config.ts index dcf9e06579d..0935185878f 100644 --- a/src/gateway/server-methods/config.ts +++ b/src/gateway/server-methods/config.ts @@ -28,6 +28,7 @@ import { } from "../../infra/restart-sentinel.js"; import { scheduleGatewaySigusr1Restart } from "../../infra/restart.js"; import { prepareSecretsRuntimeSnapshot } from "../../secrets/runtime.js"; +import { resolveGatewayAuth } from "../auth.js"; import { diffConfigPaths } from "../config-reload.js"; import { formatControlPlaneActor, @@ -221,14 +222,37 @@ function parseValidateConfigFromRawOrRespond( return { config: validated.config, schema }; } +function getEffectiveSharedGatewayAuth(config: OpenClawConfig): { + mode: "token" | "password"; + secret: string | undefined; +} | null { + const resolvedAuth = resolveGatewayAuth({ + authConfig: config.gateway?.auth, + env: process.env, + tailscaleMode: config.gateway?.tailscale?.mode, + }); + if (resolvedAuth.mode === "token") { + return { + mode: "token", + secret: resolvedAuth.token, + }; + } + if (resolvedAuth.mode === "password") { + return { + mode: "password", + secret: resolvedAuth.password, + }; + } + return null; +} + function didSharedGatewayAuthChange(prev: OpenClawConfig, next: OpenClawConfig): boolean { - const prevAuth = prev.gateway?.auth; - const nextAuth = next.gateway?.auth; - return ( - prevAuth?.mode !== nextAuth?.mode || - !isDeepStrictEqual(prevAuth?.token, nextAuth?.token) || - !isDeepStrictEqual(prevAuth?.password, nextAuth?.password) - ); + const prevAuth = getEffectiveSharedGatewayAuth(prev); + const nextAuth = getEffectiveSharedGatewayAuth(next); + if (prevAuth === null || nextAuth === null) { + return prevAuth !== nextAuth; + } + return prevAuth.mode !== nextAuth.mode || !isDeepStrictEqual(prevAuth.secret, nextAuth.secret); } function queueSharedGatewayAuthDisconnect( @@ -394,7 +418,7 @@ export const configHandlers: GatewayRequestHandlers = { } respond(true, result, undefined); }, - "config.set": async ({ params, respond, context }) => { + "config.set": async ({ params, respond }) => { if (!assertValidParams(params, validateConfigSetParams, "config.set", respond)) { return; } @@ -409,9 +433,6 @@ export const configHandlers: GatewayRequestHandlers = { if (!(await ensureResolvableSecretRefsOrRespond({ config: parsed.config, respond }))) { return; } - // Compare before the write so we invalidate clients authenticated against the - // previous shared secret immediately after the config update succeeds. - const disconnectSharedAuthClients = didSharedGatewayAuthChange(snapshot.config, parsed.config); await writeConfigFile(parsed.config, writeOptions); respond( true, @@ -422,7 +443,6 @@ export const configHandlers: GatewayRequestHandlers = { }, undefined, ); - queueSharedGatewayAuthDisconnect(disconnectSharedAuthClients, context); }, "config.patch": async ({ params, respond, client, context }) => { if (!assertValidParams(params, validateConfigPatchParams, "config.patch", respond)) { diff --git a/src/gateway/server.canvas-auth.test.ts b/src/gateway/server.canvas-auth.test.ts index c0822712ba8..c238f522a19 100644 --- a/src/gateway/server.canvas-auth.test.ts +++ b/src/gateway/server.canvas-auth.test.ts @@ -94,6 +94,7 @@ function makeWsClient(params: { }, } as GatewayWsClient["connect"], connId: params.connId, + usesSharedGatewayAuth: false, clientIp: params.clientIp, canvasCapability: params.canvasCapability, canvasCapabilityExpiresAtMs: params.canvasCapabilityExpiresAtMs, diff --git a/src/gateway/server.shared-auth-rotation.test.ts b/src/gateway/server.shared-auth-rotation.test.ts index 20ae9674274..d825569e198 100644 --- a/src/gateway/server.shared-auth-rotation.test.ts +++ b/src/gateway/server.shared-auth-rotation.test.ts @@ -18,7 +18,8 @@ const ORIGINAL_GATEWAY_AUTH = testState.gatewayAuth; const ORIGINAL_GATEWAY_TOKEN_ENV = process.env.OPENCLAW_GATEWAY_TOKEN; const OLD_TOKEN = "shared-token-old"; const NEW_TOKEN = "shared-token-new"; -const TEST_METHODS = ["config.set", "config.patch", "config.apply"] as const; +const DEFERRED_RESTART_DELAY_MS = 1_000; +const RESTARTING_AUTH_CHANGE_METHODS = ["config.patch", "config.apply"] as const; const SECRET_REF_TOKEN_ID = "OPENCLAW_SHARED_AUTH_ROTATION_SECRET_REF"; let server: Awaited>; @@ -94,42 +95,52 @@ async function waitForClose(ws: WebSocket): Promise<{ code: number; reason: stri }); } -async function sendAuthChange( - ws: WebSocket, - method: (typeof TEST_METHODS)[number], -): Promise<{ ok: boolean }> { +async function loadCurrentConfig(ws: WebSocket): Promise<{ + hash: string; + config: Record; +}> { const current = await rpcReq<{ hash?: string; config?: Record; }>(ws, "config.get", {}); expect(current.ok).toBe(true); expect(typeof current.payload?.hash).toBe("string"); - const currentConfig = structuredClone(current.payload?.config ?? {}); + return { + hash: String(current.payload?.hash), + config: structuredClone(current.payload?.config ?? {}), + }; +} + +async function sendAuthTokenChange( + ws: WebSocket, + method: (typeof RESTARTING_AUTH_CHANGE_METHODS)[number], +): Promise<{ ok: boolean }> { + const current = await loadCurrentConfig(ws); + const currentConfig = current.config; const gateway = (currentConfig.gateway ??= {}) as Record; const auth = (gateway.auth ??= {}) as Record; auth.token = NEW_TOKEN; if (method === "config.patch") { return await rpcReq(ws, "config.patch", { - baseHash: current.payload?.hash, + baseHash: current.hash, raw: JSON.stringify({ gateway: { auth: { token: NEW_TOKEN } } }), - restartDelayMs: 60_000, + restartDelayMs: DEFERRED_RESTART_DELAY_MS, }); } return await rpcReq(ws, method, { raw: JSON.stringify(currentConfig, null, 2), - ...(method === "config.set" ? { baseHash: current.payload?.hash } : {}), }); } describe("gateway shared auth rotation", () => { - for (const method of TEST_METHODS) { + for (const method of RESTARTING_AUTH_CHANGE_METHODS) { it(`disconnects existing shared-token websocket sessions after ${method}`, async () => { const ws = await openAuthenticatedWs(OLD_TOKEN); try { const closed = waitForClose(ws); - const res = await sendAuthChange(ws, method); + const res = await sendAuthTokenChange(ws, method); expect(res.ok).toBe(true); await expect(closed).resolves.toMatchObject({ @@ -145,7 +156,7 @@ describe("gateway shared auth rotation", () => { it("keeps existing device-token websocket sessions connected after shared token rotation", async () => { const ws = await openDeviceTokenWs(); try { - const res = await sendAuthChange(ws, "config.patch"); + const res = await sendAuthTokenChange(ws, "config.patch"); expect(res.ok).toBe(true); const followUp = await rpcReq<{ hash?: string }>(ws, "config.get", {});