mirror of https://github.com/openclaw/openclaw.git
fix: tighten gateway shared-auth disconnects (#60387) (thanks @mappel-nv)
This commit is contained in:
parent
c742963fd9
commit
7fd9e40960
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
},
|
||||
]);
|
||||
|
||||
|
|
|
|||
|
|
@ -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<typeof import("../../config/config.js")>("../../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<string, never>,
|
||||
};
|
||||
}
|
||||
|
||||
function createOptions(
|
||||
params: unknown,
|
||||
contextOverrides?: Partial<GatewayRequestHandlerOptions["context"]>,
|
||||
): 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<void>((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();
|
||||
});
|
||||
});
|
||||
|
|
@ -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)) {
|
||||
|
|
|
|||
|
|
@ -94,6 +94,7 @@ function makeWsClient(params: {
|
|||
},
|
||||
} as GatewayWsClient["connect"],
|
||||
connId: params.connId,
|
||||
usesSharedGatewayAuth: false,
|
||||
clientIp: params.clientIp,
|
||||
canvasCapability: params.canvasCapability,
|
||||
canvasCapabilityExpiresAtMs: params.canvasCapabilityExpiresAtMs,
|
||||
|
|
|
|||
|
|
@ -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<ReturnType<typeof startGatewayServer>>;
|
||||
|
|
@ -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<string, unknown>;
|
||||
}> {
|
||||
const current = await rpcReq<{
|
||||
hash?: string;
|
||||
config?: Record<string, unknown>;
|
||||
}>(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<string, unknown>;
|
||||
const auth = (gateway.auth ??= {}) as Record<string, unknown>;
|
||||
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", {});
|
||||
|
|
|
|||
Loading…
Reference in New Issue