mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-19 11:48:38 +00:00
Matrix: harden multi-account auth resolution
This commit is contained in:
@@ -1,6 +1,7 @@
|
|||||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||||
import type { CoreConfig } from "../types.js";
|
import type { CoreConfig } from "../types.js";
|
||||||
import {
|
import {
|
||||||
|
getMatrixScopedEnvVarNames,
|
||||||
resolveImplicitMatrixAccountId,
|
resolveImplicitMatrixAccountId,
|
||||||
resolveMatrixConfig,
|
resolveMatrixConfig,
|
||||||
resolveMatrixConfigForAccount,
|
resolveMatrixConfigForAccount,
|
||||||
@@ -97,6 +98,15 @@ describe("resolveMatrixConfig", () => {
|
|||||||
expect(resolved.deviceName).toBe("Ops Device");
|
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", () => {
|
it("prefers channels.matrix.accounts.default over global env for the default account", () => {
|
||||||
const cfg = {
|
const cfg = {
|
||||||
channels: {
|
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", () => {
|
it("rejects insecure public http Matrix homeservers", () => {
|
||||||
expect(() => validateMatrixHomeserverUrl("http://matrix.example.org")).toThrow(
|
expect(() => validateMatrixHomeserverUrl("http://matrix.example.org")).toThrow(
|
||||||
"Matrix homeserver must use https:// unless it targets a private or loopback host",
|
"Matrix homeserver must use https:// unless it targets a private or loopback host",
|
||||||
|
|||||||
@@ -9,7 +9,11 @@ import {
|
|||||||
} from "openclaw/plugin-sdk/matrix";
|
} from "openclaw/plugin-sdk/matrix";
|
||||||
import { getMatrixRuntime } from "../../runtime.js";
|
import { getMatrixRuntime } from "../../runtime.js";
|
||||||
import type { CoreConfig } from "../../types.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 { resolveMatrixConfigFieldPath } from "../config-update.js";
|
||||||
import { MatrixClient } from "../sdk.js";
|
import { MatrixClient } from "../sdk.js";
|
||||||
import { ensureMatrixSdkLoggingConfigured } from "./logging.js";
|
import { ensureMatrixSdkLoggingConfigured } from "./logging.js";
|
||||||
@@ -88,10 +92,13 @@ function resolveGlobalMatrixEnvConfig(env: NodeJS.ProcessEnv): MatrixEnvConfig {
|
|||||||
}
|
}
|
||||||
|
|
||||||
function resolveMatrixEnvAccountToken(accountId: string): string {
|
function resolveMatrixEnvAccountToken(accountId: string): string {
|
||||||
return normalizeAccountId(accountId)
|
return Array.from(normalizeAccountId(accountId))
|
||||||
.replace(/[^a-z0-9]+/g, "_")
|
.map((char) =>
|
||||||
.replace(/^_+|_+$/g, "")
|
/[a-z0-9]/.test(char)
|
||||||
.toUpperCase();
|
? char.toUpperCase()
|
||||||
|
: `_X${char.codePointAt(0)?.toString(16).toUpperCase() ?? "00"}_`,
|
||||||
|
)
|
||||||
|
.join("");
|
||||||
}
|
}
|
||||||
|
|
||||||
export function getMatrixScopedEnvVarNames(accountId: string): {
|
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: {
|
export function hasReadyMatrixEnvAuth(config: {
|
||||||
homeserver?: string;
|
homeserver?: string;
|
||||||
userId?: string;
|
userId?: string;
|
||||||
@@ -278,14 +297,13 @@ export function resolveMatrixConfigForAccount(
|
|||||||
scopedEnvValue: scopedEnv.password,
|
scopedEnvValue: scopedEnv.password,
|
||||||
globalEnvValue: globalEnv.password,
|
globalEnvValue: globalEnv.password,
|
||||||
}) || undefined;
|
}) || undefined;
|
||||||
const deviceId =
|
const deviceIdSource =
|
||||||
resolveMatrixStringField({
|
accountField("deviceId") ||
|
||||||
matrix,
|
scopedEnv.deviceId ||
|
||||||
field: "deviceId",
|
(normalizedAccountId === DEFAULT_ACCOUNT_ID
|
||||||
accountValue: accountField("deviceId"),
|
? readMatrixBaseConfigField(matrix, "deviceId") || globalEnv.deviceId || ""
|
||||||
scopedEnvValue: scopedEnv.deviceId,
|
: "");
|
||||||
globalEnvValue: globalEnv.deviceId,
|
const deviceId = deviceIdSource || undefined;
|
||||||
}) || undefined;
|
|
||||||
const deviceName =
|
const deviceName =
|
||||||
resolveMatrixStringField({
|
resolveMatrixStringField({
|
||||||
matrix,
|
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 <id>.',
|
'Multiple Matrix accounts are configured and channels.matrix.defaultAccount is not set. Set "channels.matrix.defaultAccount" to the intended account or pass --account <id>.',
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
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);
|
const resolved = resolveMatrixConfigForAccount(cfg, effectiveAccountId, env);
|
||||||
|
|
||||||
return {
|
return {
|
||||||
|
|||||||
@@ -147,6 +147,33 @@ describe("registerMatrixAutoJoin", () => {
|
|||||||
expect(joinRoom).toHaveBeenCalledWith("!room:example.org");
|
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 () => {
|
it("does not trust room-provided alias claims for allowlist joins", async () => {
|
||||||
const { client, getInviteHandler, joinRoom, resolveRoom } = createClientStub();
|
const { client, getInviteHandler, joinRoom, resolveRoom } = createClientStub();
|
||||||
resolveRoom.mockResolvedValue("!different-room:example.org");
|
resolveRoom.mockResolvedValue("!different-room:example.org");
|
||||||
|
|||||||
@@ -46,12 +46,24 @@ export function registerMatrixAutoJoin(params: {
|
|||||||
return resolved;
|
return resolved;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
const resolveAllowedAliasRoomIds = async (): Promise<string[]> => {
|
||||||
|
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.
|
// Handle invites directly so both "always" and "allowlist" modes share the same path.
|
||||||
client.on("room.invite", async (roomId: string, _inviteEvent: unknown) => {
|
client.on("room.invite", async (roomId: string, _inviteEvent: unknown) => {
|
||||||
if (autoJoin === "allowlist") {
|
if (autoJoin === "allowlist") {
|
||||||
const allowedAliasRoomIds = await Promise.all(
|
const allowedAliasRoomIds = await resolveAllowedAliasRoomIds();
|
||||||
allowedAliases.map(async (alias) => await resolveAllowedAliasRoomId(alias)),
|
|
||||||
);
|
|
||||||
const allowed =
|
const allowed =
|
||||||
autoJoinAllowlist.has("*") ||
|
autoJoinAllowlist.has("*") ||
|
||||||
allowedRoomIds.has(roomId) ||
|
allowedRoomIds.has(roomId) ||
|
||||||
|
|||||||
Reference in New Issue
Block a user