diff --git a/extensions/matrix/src/matrix/client.test.ts b/extensions/matrix/src/matrix/client.test.ts index 133e1293db3..b9800dc79be 100644 --- a/extensions/matrix/src/matrix/client.test.ts +++ b/extensions/matrix/src/matrix/client.test.ts @@ -1,6 +1,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import type { CoreConfig } from "../types.js"; import { + getMatrixScopedEnvVarNames, resolveImplicitMatrixAccountId, resolveMatrixConfig, resolveMatrixConfigForAccount, @@ -97,6 +98,15 @@ describe("resolveMatrixConfig", () => { expect(resolved.deviceName).toBe("Ops Device"); }); + it("uses collision-free scoped env var names for normalized account ids", () => { + expect(getMatrixScopedEnvVarNames("ops-prod").accessToken).toBe( + "MATRIX_OPS_X2D_PROD_ACCESS_TOKEN", + ); + expect(getMatrixScopedEnvVarNames("ops_prod").accessToken).toBe( + "MATRIX_OPS_X5F_PROD_ACCESS_TOKEN", + ); + }); + it("prefers channels.matrix.accounts.default over global env for the default account", () => { const cfg = { channels: { @@ -172,6 +182,65 @@ describe("resolveMatrixConfig", () => { ); }); + it("rejects explicit non-default account ids that are neither configured nor scoped in env", () => { + const cfg = { + channels: { + matrix: { + homeserver: "https://legacy.example.org", + accessToken: "legacy-token", + accounts: { + ops: { + homeserver: "https://ops.example.org", + accessToken: "ops-token", + }, + }, + }, + }, + } as CoreConfig; + + expect(() => + resolveMatrixAuthContext({ cfg, env: {} as NodeJS.ProcessEnv, accountId: "typo" }), + ).toThrow(/Matrix account "typo" is not configured/i); + }); + + it("allows explicit non-default account ids backed only by scoped env vars", () => { + const cfg = { + channels: { + matrix: { + homeserver: "https://legacy.example.org", + accessToken: "legacy-token", + }, + }, + } as CoreConfig; + const env = { + MATRIX_OPS_HOMESERVER: "https://ops.example.org", + MATRIX_OPS_ACCESS_TOKEN: "ops-token", + } as NodeJS.ProcessEnv; + + expect(resolveMatrixAuthContext({ cfg, env, accountId: "ops" }).accountId).toBe("ops"); + }); + + it("does not inherit the base deviceId for non-default accounts", () => { + const cfg = { + channels: { + matrix: { + homeserver: "https://base.example.org", + accessToken: "base-token", + deviceId: "BASEDEVICE", + accounts: { + ops: { + homeserver: "https://ops.example.org", + accessToken: "ops-token", + }, + }, + }, + }, + } as CoreConfig; + + const resolved = resolveMatrixConfigForAccount(cfg, "ops", {} as NodeJS.ProcessEnv); + expect(resolved.deviceId).toBeUndefined(); + }); + it("rejects insecure public http Matrix homeservers", () => { expect(() => validateMatrixHomeserverUrl("http://matrix.example.org")).toThrow( "Matrix homeserver must use https:// unless it targets a private or loopback host", diff --git a/extensions/matrix/src/matrix/client/config.ts b/extensions/matrix/src/matrix/client/config.ts index 55a2f8464f4..3230e82fd77 100644 --- a/extensions/matrix/src/matrix/client/config.ts +++ b/extensions/matrix/src/matrix/client/config.ts @@ -9,7 +9,11 @@ import { } from "openclaw/plugin-sdk/matrix"; import { getMatrixRuntime } from "../../runtime.js"; import type { CoreConfig } from "../../types.js"; -import { findMatrixAccountConfig, resolveMatrixBaseConfig } from "../account-config.js"; +import { + findMatrixAccountConfig, + resolveMatrixBaseConfig, + listNormalizedMatrixAccountIds, +} from "../account-config.js"; import { resolveMatrixConfigFieldPath } from "../config-update.js"; import { MatrixClient } from "../sdk.js"; import { ensureMatrixSdkLoggingConfigured } from "./logging.js"; @@ -88,10 +92,13 @@ function resolveGlobalMatrixEnvConfig(env: NodeJS.ProcessEnv): MatrixEnvConfig { } function resolveMatrixEnvAccountToken(accountId: string): string { - return normalizeAccountId(accountId) - .replace(/[^a-z0-9]+/g, "_") - .replace(/^_+|_+$/g, "") - .toUpperCase(); + return Array.from(normalizeAccountId(accountId)) + .map((char) => + /[a-z0-9]/.test(char) + ? char.toUpperCase() + : `_X${char.codePointAt(0)?.toString(16).toUpperCase() ?? "00"}_`, + ) + .join(""); } export function getMatrixScopedEnvVarNames(accountId: string): { @@ -128,6 +135,18 @@ export function resolveScopedMatrixEnvConfig( }; } +function hasScopedMatrixEnvConfig(accountId: string, env: NodeJS.ProcessEnv): boolean { + const scoped = resolveScopedMatrixEnvConfig(accountId, env); + return Boolean( + scoped.homeserver || + scoped.userId || + scoped.accessToken || + scoped.password || + scoped.deviceId || + scoped.deviceName, + ); +} + export function hasReadyMatrixEnvAuth(config: { homeserver?: string; userId?: string; @@ -278,14 +297,13 @@ export function resolveMatrixConfigForAccount( scopedEnvValue: scopedEnv.password, globalEnvValue: globalEnv.password, }) || undefined; - const deviceId = - resolveMatrixStringField({ - matrix, - field: "deviceId", - accountValue: accountField("deviceId"), - scopedEnvValue: scopedEnv.deviceId, - globalEnvValue: globalEnv.deviceId, - }) || undefined; + const deviceIdSource = + accountField("deviceId") || + scopedEnv.deviceId || + (normalizedAccountId === DEFAULT_ACCOUNT_ID + ? readMatrixBaseConfigField(matrix, "deviceId") || globalEnv.deviceId || "" + : ""); + const deviceId = deviceIdSource || undefined; const deviceName = resolveMatrixStringField({ matrix, @@ -342,6 +360,16 @@ export function resolveMatrixAuthContext(params?: { 'Multiple Matrix accounts are configured and channels.matrix.defaultAccount is not set. Set "channels.matrix.defaultAccount" to the intended account or pass --account .', ); } + if ( + explicitAccountId && + explicitAccountId !== DEFAULT_ACCOUNT_ID && + !listNormalizedMatrixAccountIds(cfg).includes(explicitAccountId) && + !hasScopedMatrixEnvConfig(explicitAccountId, env) + ) { + throw new Error( + `Matrix account "${explicitAccountId}" is not configured. Add channels.matrix.accounts.${explicitAccountId} or define scoped MATRIX_${resolveMatrixEnvAccountToken(explicitAccountId)}_* variables.`, + ); + } const resolved = resolveMatrixConfigForAccount(cfg, effectiveAccountId, env); return { diff --git a/extensions/matrix/src/matrix/monitor/auto-join.test.ts b/extensions/matrix/src/matrix/monitor/auto-join.test.ts index f3a27db8598..07dc83fe2a6 100644 --- a/extensions/matrix/src/matrix/monitor/auto-join.test.ts +++ b/extensions/matrix/src/matrix/monitor/auto-join.test.ts @@ -147,6 +147,33 @@ describe("registerMatrixAutoJoin", () => { expect(joinRoom).toHaveBeenCalledWith("!room:example.org"); }); + it("logs and skips allowlist alias resolution failures", async () => { + const { client, getInviteHandler, joinRoom, resolveRoom } = createClientStub(); + const error = vi.fn(); + resolveRoom.mockRejectedValue(new Error("temporary homeserver failure")); + + registerMatrixAutoJoin({ + client, + accountConfig: { + autoJoin: "allowlist", + autoJoinAllowlist: ["#allowed:example.org"], + }, + runtime: { + log: vi.fn(), + error, + } as unknown as import("openclaw/plugin-sdk/matrix").RuntimeEnv, + }); + + const inviteHandler = getInviteHandler(); + expect(inviteHandler).toBeTruthy(); + await expect(inviteHandler!("!room:example.org", {})).resolves.toBeUndefined(); + + expect(joinRoom).not.toHaveBeenCalled(); + expect(error).toHaveBeenCalledWith( + expect.stringContaining("matrix: failed resolving allowlisted alias #allowed:example.org:"), + ); + }); + it("does not trust room-provided alias claims for allowlist joins", async () => { const { client, getInviteHandler, joinRoom, resolveRoom } = createClientStub(); resolveRoom.mockResolvedValue("!different-room:example.org"); diff --git a/extensions/matrix/src/matrix/monitor/auto-join.ts b/extensions/matrix/src/matrix/monitor/auto-join.ts index f92b568e2bd..79dfc30f976 100644 --- a/extensions/matrix/src/matrix/monitor/auto-join.ts +++ b/extensions/matrix/src/matrix/monitor/auto-join.ts @@ -46,12 +46,24 @@ export function registerMatrixAutoJoin(params: { return resolved; }; + const resolveAllowedAliasRoomIds = async (): Promise => { + const resolved = await Promise.all( + allowedAliases.map(async (alias) => { + try { + return await resolveAllowedAliasRoomId(alias); + } catch (err) { + runtime.error?.(`matrix: failed resolving allowlisted alias ${alias}: ${String(err)}`); + return null; + } + }), + ); + return resolved.filter((roomId): roomId is string => Boolean(roomId)); + }; + // Handle invites directly so both "always" and "allowlist" modes share the same path. client.on("room.invite", async (roomId: string, _inviteEvent: unknown) => { if (autoJoin === "allowlist") { - const allowedAliasRoomIds = await Promise.all( - allowedAliases.map(async (alias) => await resolveAllowedAliasRoomId(alias)), - ); + const allowedAliasRoomIds = await resolveAllowedAliasRoomIds(); const allowed = autoJoinAllowlist.has("*") || allowedRoomIds.has(roomId) ||