Matrix: harden alias trust and log redaction

This commit is contained in:
Gustavo Madeira Santana
2026-03-12 04:23:09 +00:00
parent 0f14b359a8
commit 63fc3c780b
9 changed files with 163 additions and 51 deletions

View File

@@ -258,6 +258,7 @@ describe("matrix directory", () => {
}),
).toEqual([
'- Matrix rooms: groupPolicy="open" allows any room to trigger (mention-gated). Set channels.matrix.groupPolicy="allowlist" + channels.matrix.groups (and optionally channels.matrix.groupAllowFrom) to restrict rooms.',
'- Matrix invites: autoJoin="always" joins any invited room before message policy applies. Set channels.matrix.autoJoin="allowlist" + channels.matrix.autoJoinAllowlist (or channels.matrix.autoJoin="off") to restrict joins.',
]);
expect(
@@ -292,6 +293,33 @@ describe("matrix directory", () => {
}),
).toEqual([
'- Matrix rooms: groupPolicy="open" allows any room to trigger (mention-gated). Set channels.matrix.accounts.assistant.groupPolicy="allowlist" + channels.matrix.accounts.assistant.groups (and optionally channels.matrix.accounts.assistant.groupAllowFrom) to restrict rooms.',
'- Matrix invites: autoJoin="always" joins any invited room before message policy applies. Set channels.matrix.accounts.assistant.autoJoin="allowlist" + channels.matrix.accounts.assistant.autoJoinAllowlist (or channels.matrix.accounts.assistant.autoJoin="off") to restrict joins.',
]);
});
it("reports invite auto-join warnings even when room policy is restricted", () => {
expect(
matrixPlugin.security?.collectWarnings?.({
cfg: {
channels: {
matrix: {
groupPolicy: "allowlist",
},
},
} as CoreConfig,
account: resolveMatrixAccount({
cfg: {
channels: {
matrix: {
groupPolicy: "allowlist",
},
},
} as CoreConfig,
accountId: "default",
}),
}),
).toEqual([
'- Matrix invites: autoJoin="always" joins any invited room before message policy applies. Set channels.matrix.autoJoin="allowlist" + channels.matrix.autoJoinAllowlist (or channels.matrix.autoJoin="off") to restrict joins.',
]);
});

View File

@@ -158,13 +158,19 @@ export const matrixPlugin: ChannelPlugin<ResolvedMatrixAccount> = {
groupPolicy: account.config.groupPolicy,
defaultGroupPolicy,
});
if (groupPolicy !== "open") {
return [];
}
const configPath = resolveMatrixConfigPath(cfg as CoreConfig, account.accountId);
return [
`- Matrix rooms: groupPolicy="open" allows any room to trigger (mention-gated). Set ${configPath}.groupPolicy="allowlist" + ${configPath}.groups (and optionally ${configPath}.groupAllowFrom) to restrict rooms.`,
];
const warnings: string[] = [];
if (groupPolicy === "open") {
warnings.push(
`- Matrix rooms: groupPolicy="open" allows any room to trigger (mention-gated). Set ${configPath}.groupPolicy="allowlist" + ${configPath}.groups (and optionally ${configPath}.groupAllowFrom) to restrict rooms.`,
);
}
if ((account.config.autoJoin ?? "always") === "always") {
warnings.push(
`- Matrix invites: autoJoin="always" joins any invited room before message policy applies. Set ${configPath}.autoJoin="allowlist" + ${configPath}.autoJoinAllowlist (or ${configPath}.autoJoin="off") to restrict joins.`,
);
}
return warnings;
},
},
groups: {

View File

@@ -16,15 +16,14 @@ function createClientStub() {
return client;
}),
joinRoom: vi.fn(async () => {}),
getRoomStateEvent: vi.fn(async () => ({})),
resolveRoom: vi.fn(async () => null),
} as unknown as import("../sdk.js").MatrixClient;
return {
client,
getInviteHandler: () => inviteHandler,
joinRoom: (client as unknown as { joinRoom: ReturnType<typeof vi.fn> }).joinRoom,
getRoomStateEvent: (client as unknown as { getRoomStateEvent: ReturnType<typeof vi.fn> })
.getRoomStateEvent,
resolveRoom: (client as unknown as { resolveRoom: ReturnType<typeof vi.fn> }).resolveRoom,
};
}
@@ -60,11 +59,8 @@ describe("registerMatrixAutoJoin", () => {
});
it("ignores invites outside allowlist when autoJoin=allowlist", async () => {
const { client, getInviteHandler, joinRoom, getRoomStateEvent } = createClientStub();
getRoomStateEvent.mockResolvedValue({
alias: "#other:example.org",
alt_aliases: ["#else:example.org"],
});
const { client, getInviteHandler, joinRoom, resolveRoom } = createClientStub();
resolveRoom.mockResolvedValue(null);
const accountConfig: MatrixConfig = {
autoJoin: "allowlist",
autoJoinAllowlist: ["#allowed:example.org"],
@@ -86,12 +82,9 @@ describe("registerMatrixAutoJoin", () => {
expect(joinRoom).not.toHaveBeenCalled();
});
it("joins invite when alias matches allowlist", async () => {
const { client, getInviteHandler, joinRoom, getRoomStateEvent } = createClientStub();
getRoomStateEvent.mockResolvedValue({
alias: "#allowed:example.org",
alt_aliases: ["#backup:example.org"],
});
it("joins invite when allowlisted alias resolves to the invited room", async () => {
const { client, getInviteHandler, joinRoom, resolveRoom } = createClientStub();
resolveRoom.mockResolvedValue("!room:example.org");
const accountConfig: MatrixConfig = {
autoJoin: "allowlist",
autoJoinAllowlist: [" #allowed:example.org "],
@@ -113,13 +106,33 @@ describe("registerMatrixAutoJoin", () => {
expect(joinRoom).toHaveBeenCalledWith("!room:example.org");
});
it("uses account-scoped auto-join settings for non-default accounts", async () => {
const { client, getInviteHandler, joinRoom, getRoomStateEvent } = createClientStub();
getRoomStateEvent.mockResolvedValue({
alias: "#ops-allowed:example.org",
alt_aliases: [],
it("does not trust room-provided alias claims for allowlist joins", async () => {
const { client, getInviteHandler, joinRoom, resolveRoom } = createClientStub();
resolveRoom.mockResolvedValue("!different-room:example.org");
registerMatrixAutoJoin({
client,
accountConfig: {
autoJoin: "allowlist",
autoJoinAllowlist: ["#allowed:example.org"],
},
runtime: {
log: vi.fn(),
error: vi.fn(),
} as unknown as import("openclaw/plugin-sdk/matrix").RuntimeEnv,
});
const inviteHandler = getInviteHandler();
expect(inviteHandler).toBeTruthy();
await inviteHandler!("!room:example.org", {});
expect(joinRoom).not.toHaveBeenCalled();
});
it("uses account-scoped auto-join settings for non-default accounts", async () => {
const { client, getInviteHandler, joinRoom, resolveRoom } = createClientStub();
resolveRoom.mockResolvedValue("!room:example.org");
registerMatrixAutoJoin({
client,
accountConfig: {

View File

@@ -17,9 +17,13 @@ export function registerMatrixAutoJoin(params: {
runtime.log?.(message);
};
const autoJoin = accountConfig.autoJoin ?? "always";
const autoJoinAllowlist = new Set(
(accountConfig.autoJoinAllowlist ?? []).map((entry) => String(entry).trim()).filter(Boolean),
);
const rawAllowlist = (accountConfig.autoJoinAllowlist ?? [])
.map((entry) => String(entry).trim())
.filter(Boolean);
const autoJoinAllowlist = new Set(rawAllowlist);
const allowedRoomIds = new Set(rawAllowlist.filter((entry) => entry.startsWith("!")));
const allowedAliases = rawAllowlist.filter((entry) => entry.startsWith("#"));
const resolvedAliasRoomIds = new Map<string, string | null>();
if (autoJoin === "off") {
return;
@@ -31,31 +35,25 @@ export function registerMatrixAutoJoin(params: {
logVerbose("matrix: auto-join enabled for allowlist invites");
}
const resolveAllowedAliasRoomId = async (alias: string): Promise<string | null> => {
if (resolvedAliasRoomIds.has(alias)) {
return resolvedAliasRoomIds.get(alias) ?? null;
}
const resolved = await params.client.resolveRoom(alias);
resolvedAliasRoomIds.set(alias, resolved);
return resolved;
};
// 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") {
let alias: string | undefined;
let altAliases: string[] = [];
try {
const aliasState = await client
.getRoomStateEvent(roomId, "m.room.canonical_alias", "")
.catch(() => null);
alias = aliasState && typeof aliasState.alias === "string" ? aliasState.alias : undefined;
altAliases =
aliasState && Array.isArray(aliasState.alt_aliases)
? aliasState.alt_aliases
.map((entry) => (typeof entry === "string" ? entry.trim() : ""))
.filter(Boolean)
: [];
} catch {
// Ignore errors
}
const allowedAliasRoomIds = await Promise.all(
allowedAliases.map(async (alias) => await resolveAllowedAliasRoomId(alias)),
);
const allowed =
autoJoinAllowlist.has("*") ||
autoJoinAllowlist.has(roomId) ||
(alias ? autoJoinAllowlist.has(alias) : false) ||
altAliases.some((value) => autoJoinAllowlist.has(value));
allowedRoomIds.has(roomId) ||
allowedAliasRoomIds.some((resolvedRoomId) => resolvedRoomId === roomId);
if (!allowed) {
logVerbose(`matrix: invite ignored (not in allowlist) room=${roomId}`);

View File

@@ -153,4 +153,45 @@ describe("resolveMatrixMonitorConfig", () => {
"matrix rooms must be room IDs or aliases (example: !room:server or #alias:server). Unresolved entries are ignored.",
);
});
it("resolves exact room aliases to canonical room ids instead of trusting alias keys directly", async () => {
const runtime = createRuntime();
const resolveTargets = vi.fn(
async ({ kind, inputs }: { inputs: string[]; kind: "user" | "group" }) => {
if (kind === "group") {
return inputs.map((input) =>
input === "#allowed:example.org"
? { input, resolved: true, id: "!allowed-room:example.org" }
: { input, resolved: false },
);
}
return [];
},
);
const result = await resolveMatrixMonitorConfig({
cfg: {} as CoreConfig,
accountId: "ops",
roomsConfig: {
"#allowed:example.org": {
allow: true,
},
},
runtime,
resolveTargets,
});
expect(result.roomsConfig).toEqual({
"!allowed-room:example.org": {
allow: true,
},
});
expect(resolveTargets).toHaveBeenCalledWith(
expect.objectContaining({
accountId: "ops",
kind: "group",
inputs: ["#allowed:example.org"],
}),
);
});
});

View File

@@ -183,7 +183,7 @@ async function resolveMatrixMonitorRoomsConfig(params: {
unresolved.push(entry);
continue;
}
if ((cleaned.startsWith("!") || cleaned.startsWith("#")) && cleaned.includes(":")) {
if (cleaned.startsWith("!") && cleaned.includes(":")) {
if (!nextRooms[cleaned]) {
nextRooms[cleaned] = roomConfig;
}

View File

@@ -0,0 +1,25 @@
import { afterEach, describe, expect, it, vi } from "vitest";
import { ConsoleLogger, setMatrixConsoleLogging } from "./logger.js";
describe("ConsoleLogger", () => {
afterEach(() => {
setMatrixConsoleLogging(false);
vi.restoreAllMocks();
});
it("redacts sensitive tokens in emitted log messages", () => {
setMatrixConsoleLogging(true);
const spy = vi.spyOn(console, "error").mockImplementation(() => {});
new ConsoleLogger().error(
"MatrixHttpClient",
"Authorization: Bearer 123456:abcdefghijklmnopqrstuvwxyzABCDEFG",
);
const message = spy.mock.calls[0]?.[0];
expect(typeof message).toBe("string");
expect(message).toContain("Authorization: Bearer");
expect(message).not.toContain("123456:abcdefghijklmnopqrstuvwxyzABCDEFG");
expect(message).toContain("***");
});
});

View File

@@ -1,5 +1,5 @@
import { format } from "node:util";
import type { RuntimeLogger } from "openclaw/plugin-sdk/matrix";
import { redactSensitiveText, type RuntimeLogger } from "openclaw/plugin-sdk/matrix";
import { getMatrixRuntime } from "../../runtime.js";
export type Logger = {
@@ -35,7 +35,7 @@ function formatMessage(module: string, messageOrObject: unknown[]): string {
if (messageOrObject.length === 0) {
return `[${module}]`;
}
return `[${module}] ${format(...messageOrObject)}`;
return redactSensitiveText(`[${module}] ${format(...messageOrObject)}`);
}
export class ConsoleLogger {

View File

@@ -154,6 +154,7 @@ export {
} from "../security/dm-policy-shared.js";
export { normalizeStringEntries } from "../shared/string-normalization.js";
export { formatDocsLink } from "../terminal/links.js";
export { redactSensitiveText } from "../logging/redact.js";
export type { WizardPrompter } from "../wizard/prompts.js";
export {
evaluateGroupRouteAccessForPolicy,