From f602359f0d66cf018713246f3bef4e8ad73da160 Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Wed, 25 Feb 2026 20:21:48 -0500 Subject: [PATCH] Matrix-js: harden account config updates and onboarding --- .../matrix-js/src/channel.directory.test.ts | 56 +++++++++ extensions/matrix-js/src/channel.ts | 53 ++------ .../src/matrix/config-update.test.ts | 50 ++++++++ .../matrix-js/src/matrix/config-update.ts | 100 +++++++++++++++ extensions/matrix-js/src/onboarding.test.ts | 115 ++++++++++++++++++ extensions/matrix-js/src/onboarding.ts | 63 ++-------- 6 files changed, 340 insertions(+), 97 deletions(-) create mode 100644 extensions/matrix-js/src/matrix/config-update.test.ts create mode 100644 extensions/matrix-js/src/matrix/config-update.ts create mode 100644 extensions/matrix-js/src/onboarding.test.ts diff --git a/extensions/matrix-js/src/channel.directory.test.ts b/extensions/matrix-js/src/channel.directory.test.ts index 6126dbb286c..90e4ef2eb7f 100644 --- a/extensions/matrix-js/src/channel.directory.test.ts +++ b/extensions/matrix-js/src/channel.directory.test.ts @@ -248,4 +248,60 @@ describe("matrix directory", () => { }); expect(accountId).toBe("ops"); }); + + it("clears stale access token when switching an account to password auth", () => { + const cfg = { + channels: { + "matrix-js": { + accounts: { + default: { + homeserver: "https://matrix.example.org", + accessToken: "old-token", + }, + }, + }, + }, + } as unknown as CoreConfig; + + const updated = matrixPlugin.setup!.applyAccountConfig({ + cfg, + accountId: "default", + input: { + homeserver: "https://matrix.example.org", + userId: "@bot:example.org", + password: "new-password", + }, + }) as CoreConfig; + + expect(updated.channels?.["matrix-js"]?.accounts?.default?.password).toBe("new-password"); + expect(updated.channels?.["matrix-js"]?.accounts?.default?.accessToken).toBeUndefined(); + }); + + it("clears stale password when switching an account to token auth", () => { + const cfg = { + channels: { + "matrix-js": { + accounts: { + default: { + homeserver: "https://matrix.example.org", + userId: "@bot:example.org", + password: "old-password", + }, + }, + }, + }, + } as unknown as CoreConfig; + + const updated = matrixPlugin.setup!.applyAccountConfig({ + cfg, + accountId: "default", + input: { + homeserver: "https://matrix.example.org", + accessToken: "new-token", + }, + }) as CoreConfig; + + expect(updated.channels?.["matrix-js"]?.accounts?.default?.accessToken).toBe("new-token"); + expect(updated.channels?.["matrix-js"]?.accounts?.default?.password).toBeUndefined(); + }); }); diff --git a/extensions/matrix-js/src/channel.ts b/extensions/matrix-js/src/channel.ts index ad73caf2174..3740395bf5c 100644 --- a/extensions/matrix-js/src/channel.ts +++ b/extensions/matrix-js/src/channel.ts @@ -28,6 +28,7 @@ import { type ResolvedMatrixAccount, } from "./matrix/accounts.js"; import { resolveMatrixAuth } from "./matrix/client.js"; +import { updateMatrixAccountConfig } from "./matrix/config-update.js"; import { normalizeMatrixAllowList, normalizeMatrixUserId } from "./matrix/monitor/allowlist.js"; import { probeMatrix } from "./matrix/probe.js"; import { sendMessageMatrix } from "./matrix/send.js"; @@ -63,47 +64,6 @@ function normalizeMatrixMessagingTarget(raw: string): string | undefined { return stripped || undefined; } -function buildMatrixConfigUpdate( - cfg: CoreConfig, - accountId: string, - input: { - homeserver?: string; - userId?: string; - accessToken?: string; - password?: string; - deviceName?: string; - initialSyncLimit?: number; - }, -): CoreConfig { - const normalizedAccountId = normalizeAccountId(accountId); - const existing = cfg.channels?.["matrix-js"] ?? {}; - return { - ...cfg, - channels: { - ...cfg.channels, - "matrix-js": { - ...existing, - enabled: true, - accounts: { - ...existing.accounts, - [normalizedAccountId]: { - ...existing.accounts?.[normalizedAccountId], - enabled: true, - ...(input.homeserver ? { homeserver: input.homeserver } : {}), - ...(input.userId ? { userId: input.userId } : {}), - ...(input.accessToken ? { accessToken: input.accessToken } : {}), - ...(input.password ? { password: input.password } : {}), - ...(input.deviceName ? { deviceName: input.deviceName } : {}), - ...(typeof input.initialSyncLimit === "number" - ? { initialSyncLimit: input.initialSyncLimit } - : {}), - }, - }, - }, - }, - }; -} - export const matrixPlugin: ChannelPlugin = { id: "matrix-js", meta, @@ -378,11 +338,14 @@ export const matrixPlugin: ChannelPlugin = { enabled: true, }) as CoreConfig; } - return buildMatrixConfigUpdate(next as CoreConfig, accountId, { + const accessToken = input.accessToken?.trim(); + const password = input.password?.trim(); + const userId = input.userId?.trim(); + return updateMatrixAccountConfig(next as CoreConfig, accountId, { homeserver: input.homeserver?.trim(), - userId: input.userId?.trim(), - accessToken: input.accessToken?.trim(), - password: input.password?.trim(), + userId: password && !userId ? null : userId, + accessToken: accessToken || (password ? null : undefined), + password: password || (accessToken ? null : undefined), deviceName: input.deviceName?.trim(), initialSyncLimit: input.initialSyncLimit, }); diff --git a/extensions/matrix-js/src/matrix/config-update.test.ts b/extensions/matrix-js/src/matrix/config-update.test.ts new file mode 100644 index 00000000000..3be4a6dac89 --- /dev/null +++ b/extensions/matrix-js/src/matrix/config-update.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, it } from "vitest"; +import type { CoreConfig } from "../types.js"; +import { updateMatrixAccountConfig } from "./config-update.js"; + +describe("updateMatrixAccountConfig", () => { + it("supports explicit null clears and boolean false values", () => { + const cfg = { + channels: { + "matrix-js": { + accounts: { + default: { + homeserver: "https://matrix.example.org", + userId: "@bot:example.org", + accessToken: "old-token", + password: "old-password", + encryption: true, + }, + }, + }, + }, + } as CoreConfig; + + const updated = updateMatrixAccountConfig(cfg, "default", { + accessToken: "new-token", + password: null, + userId: null, + encryption: false, + }); + + expect(updated.channels?.["matrix-js"]?.accounts?.default).toMatchObject({ + accessToken: "new-token", + encryption: false, + }); + expect(updated.channels?.["matrix-js"]?.accounts?.default?.password).toBeUndefined(); + expect(updated.channels?.["matrix-js"]?.accounts?.default?.userId).toBeUndefined(); + }); + + it("normalizes account id and defaults account enabled=true", () => { + const updated = updateMatrixAccountConfig({} as CoreConfig, "Main Bot", { + name: "Main Bot", + homeserver: "https://matrix.example.org", + }); + + expect(updated.channels?.["matrix-js"]?.accounts?.["main-bot"]).toMatchObject({ + name: "Main Bot", + homeserver: "https://matrix.example.org", + enabled: true, + }); + }); +}); diff --git a/extensions/matrix-js/src/matrix/config-update.ts b/extensions/matrix-js/src/matrix/config-update.ts new file mode 100644 index 00000000000..974c8fff00d --- /dev/null +++ b/extensions/matrix-js/src/matrix/config-update.ts @@ -0,0 +1,100 @@ +import { normalizeAccountId } from "openclaw/plugin-sdk"; +import type { CoreConfig, MatrixConfig } from "../types.js"; + +export type MatrixAccountPatch = { + name?: string | null; + enabled?: boolean; + homeserver?: string | null; + userId?: string | null; + accessToken?: string | null; + password?: string | null; + deviceName?: string | null; + encryption?: boolean | null; + initialSyncLimit?: number | null; +}; + +function applyNullableStringField( + target: Record, + key: keyof MatrixAccountPatch, + value: string | null | undefined, +): void { + if (value === undefined) { + return; + } + if (value === null) { + delete target[key]; + return; + } + const trimmed = value.trim(); + if (!trimmed) { + delete target[key]; + return; + } + target[key] = trimmed; +} + +export function updateMatrixAccountConfig( + cfg: CoreConfig, + accountId: string, + patch: MatrixAccountPatch, +): CoreConfig { + const matrix = cfg.channels?.["matrix-js"] ?? {}; + const normalizedAccountId = normalizeAccountId(accountId); + const existingAccount = (matrix.accounts?.[normalizedAccountId] ?? {}) as MatrixConfig; + const nextAccount: Record = { ...existingAccount }; + + if (patch.name !== undefined) { + if (patch.name === null) { + delete nextAccount.name; + } else { + const trimmed = patch.name.trim(); + if (trimmed) { + nextAccount.name = trimmed; + } else { + delete nextAccount.name; + } + } + } + if (typeof patch.enabled === "boolean") { + nextAccount.enabled = patch.enabled; + } else if (typeof nextAccount.enabled !== "boolean") { + nextAccount.enabled = true; + } + + applyNullableStringField(nextAccount, "homeserver", patch.homeserver); + applyNullableStringField(nextAccount, "userId", patch.userId); + applyNullableStringField(nextAccount, "accessToken", patch.accessToken); + applyNullableStringField(nextAccount, "password", patch.password); + applyNullableStringField(nextAccount, "deviceName", patch.deviceName); + + if (patch.initialSyncLimit !== undefined) { + if (patch.initialSyncLimit === null) { + delete nextAccount.initialSyncLimit; + } else { + nextAccount.initialSyncLimit = Math.max(0, Math.floor(patch.initialSyncLimit)); + } + } + + if (patch.encryption !== undefined) { + if (patch.encryption === null) { + delete nextAccount.encryption; + } else { + nextAccount.encryption = patch.encryption; + } + } + + return { + ...cfg, + channels: { + ...cfg.channels, + "matrix-js": { + ...matrix, + enabled: true, + accounts: { + ...matrix.accounts, + [normalizedAccountId]: nextAccount as MatrixConfig, + }, + }, + }, + }; +} diff --git a/extensions/matrix-js/src/onboarding.test.ts b/extensions/matrix-js/src/onboarding.test.ts new file mode 100644 index 00000000000..00d088e0091 --- /dev/null +++ b/extensions/matrix-js/src/onboarding.test.ts @@ -0,0 +1,115 @@ +import type { RuntimeEnv, WizardPrompter } from "openclaw/plugin-sdk"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { matrixOnboardingAdapter } from "./onboarding.js"; +import { setMatrixRuntime } from "./runtime.js"; +import type { CoreConfig } from "./types.js"; + +vi.mock("./matrix/deps.js", () => ({ + ensureMatrixSdkInstalled: vi.fn(async () => {}), + isMatrixSdkAvailable: vi.fn(() => true), +})); + +describe("matrix onboarding", () => { + const previousEnv = { + MATRIX_HOMESERVER: process.env.MATRIX_HOMESERVER, + MATRIX_USER_ID: process.env.MATRIX_USER_ID, + MATRIX_ACCESS_TOKEN: process.env.MATRIX_ACCESS_TOKEN, + MATRIX_PASSWORD: process.env.MATRIX_PASSWORD, + }; + + afterEach(() => { + process.env.MATRIX_HOMESERVER = previousEnv.MATRIX_HOMESERVER; + process.env.MATRIX_USER_ID = previousEnv.MATRIX_USER_ID; + process.env.MATRIX_ACCESS_TOKEN = previousEnv.MATRIX_ACCESS_TOKEN; + process.env.MATRIX_PASSWORD = previousEnv.MATRIX_PASSWORD; + }); + + it("does not offer env shortcut when adding a non-default account", async () => { + setMatrixRuntime({ + state: { + resolveStateDir: (_env: NodeJS.ProcessEnv, homeDir?: () => string) => + (homeDir ?? (() => "/tmp"))(), + }, + config: { + loadConfig: () => ({}), + }, + } as never); + + process.env.MATRIX_HOMESERVER = "https://matrix.env.example.org"; + process.env.MATRIX_USER_ID = "@env:example.org"; + process.env.MATRIX_PASSWORD = "env-password"; + process.env.MATRIX_ACCESS_TOKEN = ""; + + const confirmMessages: string[] = []; + const prompter = { + note: vi.fn(async () => {}), + select: vi.fn(async ({ message }: { message: string }) => { + if (message === "Matrix-js already configured. What do you want to do?") { + return "add-account"; + } + if (message === "Matrix auth method") { + return "token"; + } + throw new Error(`unexpected select prompt: ${message}`); + }), + text: vi.fn(async ({ message }: { message: string }) => { + if (message === "Matrix account name") { + return "ops"; + } + if (message === "Matrix homeserver URL") { + return "https://matrix.ops.example.org"; + } + if (message === "Matrix access token") { + return "ops-token"; + } + if (message === "Matrix device name (optional)") { + return "Ops Device"; + } + throw new Error(`unexpected text prompt: ${message}`); + }), + confirm: vi.fn(async ({ message }: { message: string }) => { + confirmMessages.push(message); + if (message === "Enable end-to-end encryption (E2EE)?") { + return false; + } + if (message === "Configure Matrix rooms access?") { + return false; + } + return false; + }), + } as unknown as WizardPrompter; + + const result = await matrixOnboardingAdapter.configureInteractive!({ + cfg: { + channels: { + "matrix-js": { + accounts: { + default: { + homeserver: "https://matrix.main.example.org", + accessToken: "main-token", + }, + }, + }, + }, + } as CoreConfig, + runtime: { log: vi.fn(), error: vi.fn(), exit: vi.fn() } as unknown as RuntimeEnv, + prompter, + options: undefined, + accountOverrides: {}, + shouldPromptAccountIds: true, + forceAllowFrom: false, + configured: true, + label: "Matrix-js", + }); + + expect(result).not.toBe("skip"); + if (result !== "skip") { + expect(result.accountId).toBe("ops"); + expect(result.cfg.channels?.["matrix-js"]?.accounts?.ops).toMatchObject({ + homeserver: "https://matrix.ops.example.org", + accessToken: "ops-token", + }); + } + expect(confirmMessages).not.toContain("Matrix env vars detected. Use env values?"); + }); +}); diff --git a/extensions/matrix-js/src/onboarding.ts b/extensions/matrix-js/src/onboarding.ts index dd52b635b7b..9e4ebfe049c 100644 --- a/extensions/matrix-js/src/onboarding.ts +++ b/extensions/matrix-js/src/onboarding.ts @@ -20,6 +20,7 @@ import { resolveMatrixAccount, resolveMatrixAccountConfig, } from "./matrix/accounts.js"; +import { updateMatrixAccountConfig } from "./matrix/config-update.js"; import { ensureMatrixSdkInstalled, isMatrixSdkAvailable } from "./matrix/deps.js"; import { resolveMatrixTargets } from "./resolve-targets.js"; import type { CoreConfig } from "./types.js"; @@ -179,50 +180,6 @@ function setMatrixGroupRooms(cfg: CoreConfig, roomKeys: string[]) { }; } -function upsertMatrixAccountConfig( - cfg: CoreConfig, - accountId: string, - patch: { - name?: string; - enabled?: boolean; - homeserver?: string; - userId?: string; - accessToken?: string; - password?: string; - deviceName?: string; - encryption?: boolean; - }, -): CoreConfig { - const matrix = cfg.channels?.["matrix-js"] ?? {}; - const normalizedAccountId = normalizeAccountId(accountId); - return { - ...cfg, - channels: { - ...cfg.channels, - "matrix-js": { - ...matrix, - enabled: true, - accounts: { - ...matrix.accounts, - [normalizedAccountId]: { - ...matrix.accounts?.[normalizedAccountId], - ...(patch.name?.trim() ? { name: patch.name.trim() } : {}), - ...(typeof patch.enabled === "boolean" - ? { enabled: patch.enabled } - : { enabled: true }), - ...(patch.homeserver ? { homeserver: patch.homeserver } : {}), - ...(patch.userId ? { userId: patch.userId } : {}), - ...(patch.accessToken ? { accessToken: patch.accessToken } : {}), - ...(patch.password ? { password: patch.password } : {}), - ...(patch.deviceName ? { deviceName: patch.deviceName } : {}), - ...(typeof patch.encryption === "boolean" ? { encryption: patch.encryption } : {}), - }, - }, - }, - }, - }; -} - const dmPolicy: ChannelOnboardingDmPolicy = { label: "Matrix", channel, @@ -266,7 +223,7 @@ async function runMatrixConfigure(params: { if (enteredName !== accountId) { await params.prompter.note(`Account id will be "${accountId}".`, "Matrix account"); } - next = upsertMatrixAccountConfig(next, accountId, { name: enteredName, enabled: true }); + next = updateMatrixAccountConfig(next, accountId, { name: enteredName, enabled: true }); } else { const override = params.accountOverrides?.[channel]?.trim(); if (override) { @@ -295,7 +252,9 @@ async function runMatrixConfigure(params: { const envPassword = process.env.MATRIX_PASSWORD?.trim(); const envReady = Boolean(envHomeserver && (envAccessToken || (envUserId && envPassword))); + const canUseEnvShortcut = accountId === DEFAULT_ACCOUNT_ID; if ( + canUseEnvShortcut && envReady && !existing.homeserver && !existing.userId && @@ -307,7 +266,7 @@ async function runMatrixConfigure(params: { initialValue: true, }); if (useEnv) { - next = upsertMatrixAccountConfig(next, accountId, { enabled: true }); + next = updateMatrixAccountConfig(next, accountId, { enabled: true }); if (params.forceAllowFrom) { next = await promptMatrixAllowFrom({ cfg: next, prompter: params.prompter }); } @@ -406,14 +365,14 @@ async function runMatrixConfigure(params: { initialValue: existing.encryption ?? false, }); - next = upsertMatrixAccountConfig(next, accountId, { + next = updateMatrixAccountConfig(next, accountId, { enabled: true, homeserver, - userId: userId || undefined, - accessToken: accessToken || undefined, - password: password || undefined, - deviceName: deviceName || undefined, - encryption: enableEncryption || undefined, + userId: userId || null, + accessToken: accessToken || null, + password: password || null, + deviceName: deviceName || null, + encryption: enableEncryption, }); if (params.forceAllowFrom) {