fix: harden Mattermost plugin gating (#1428) (thanks @damoahdominic)

This commit is contained in:
Peter Steinberger
2026-01-23 00:19:23 +00:00
parent 1d658109a8
commit 279f799388
55 changed files with 403 additions and 413 deletions

View File

@@ -0,0 +1,43 @@
import { describe, expect, it } from "vitest";
import { mattermostPlugin } from "./channel.js";
describe("mattermostPlugin", () => {
describe("messaging", () => {
it("keeps @username targets", () => {
const normalize = mattermostPlugin.messaging?.normalizeTarget;
if (!normalize) return;
expect(normalize("@Alice")).toBe("@Alice");
expect(normalize("@alice")).toBe("@alice");
});
it("normalizes mattermost: prefix to user:", () => {
const normalize = mattermostPlugin.messaging?.normalizeTarget;
if (!normalize) return;
expect(normalize("mattermost:USER123")).toBe("user:USER123");
});
});
describe("pairing", () => {
it("normalizes allowlist entries", () => {
const normalize = mattermostPlugin.pairing?.normalizeAllowEntry;
if (!normalize) return;
expect(normalize("@Alice")).toBe("alice");
expect(normalize("user:USER123")).toBe("user123");
});
});
describe("config", () => {
it("formats allowFrom entries", () => {
const formatAllowFrom = mattermostPlugin.config.formatAllowFrom;
const formatted = formatAllowFrom({
allowFrom: ["@Alice", "user:USER123", "mattermost:BOT999"],
});
expect(formatted).toEqual(["@alice", "user123", "bot999"]);
});
});
});

View File

@@ -3,6 +3,7 @@ import {
buildChannelConfigSchema,
DEFAULT_ACCOUNT_ID,
deleteAccountFromConfigSection,
formatPairingApproveHint,
migrateBaseNameToDefaultAccount,
normalizeAccountId,
setAccountEnabledInConfigSection,
@@ -38,14 +39,40 @@ const meta = {
blurb: "self-hosted Slack-style chat; install the plugin to enable.",
systemImage: "bubble.left.and.bubble.right",
order: 65,
quickstartAllowFrom: true,
} as const;
function normalizeAllowEntry(entry: string): string {
return entry
.trim()
.replace(/^(mattermost|user):/i, "")
.replace(/^@/, "")
.toLowerCase();
}
function formatAllowEntry(entry: string): string {
const trimmed = entry.trim();
if (!trimmed) return "";
if (trimmed.startsWith("@")) {
const username = trimmed.slice(1).trim();
return username ? `@${username.toLowerCase()}` : "";
}
return trimmed.replace(/^(mattermost|user):/i, "").toLowerCase();
}
export const mattermostPlugin: ChannelPlugin<ResolvedMattermostAccount> = {
id: "mattermost",
meta: {
...meta,
},
onboarding: mattermostOnboardingAdapter,
pairing: {
idLabel: "mattermostUserId",
normalizeAllowEntry: (entry) => normalizeAllowEntry(entry),
notifyApproval: async ({ id }) => {
console.log(`[mattermost] User ${id} approved for pairing`);
},
},
capabilities: {
chatTypes: ["direct", "channel", "group", "thread"],
threads: true,
@@ -84,6 +111,39 @@ export const mattermostPlugin: ChannelPlugin<ResolvedMattermostAccount> = {
botTokenSource: account.botTokenSource,
baseUrl: account.baseUrl,
}),
resolveAllowFrom: ({ cfg, accountId }) =>
(resolveMattermostAccount({ cfg, accountId }).config.allowFrom ?? []).map((entry) =>
String(entry),
),
formatAllowFrom: ({ allowFrom }) =>
allowFrom
.map((entry) => formatAllowEntry(String(entry)))
.filter(Boolean),
},
security: {
resolveDmPolicy: ({ cfg, accountId, account }) => {
const resolvedAccountId = accountId ?? account.accountId ?? DEFAULT_ACCOUNT_ID;
const useAccountPath = Boolean(cfg.channels?.mattermost?.accounts?.[resolvedAccountId]);
const basePath = useAccountPath
? `channels.mattermost.accounts.${resolvedAccountId}.`
: "channels.mattermost.";
return {
policy: account.config.dmPolicy ?? "pairing",
allowFrom: account.config.allowFrom ?? [],
policyPath: `${basePath}dmPolicy`,
allowFromPath: basePath,
approveHint: formatPairingApproveHint("mattermost"),
normalizeEntry: (raw) => normalizeAllowEntry(raw),
};
},
collectWarnings: ({ account, cfg }) => {
const defaultGroupPolicy = cfg.channels?.defaults?.groupPolicy;
const groupPolicy = account.config.groupPolicy ?? defaultGroupPolicy ?? "allowlist";
if (groupPolicy !== "open") return [];
return [
`- Mattermost channels: groupPolicy="open" allows any member to trigger (mention-gated). Set channels.mattermost.groupPolicy="allowlist" + channels.mattermost.groupAllowFrom to restrict senders.`,
];
},
},
groups: {
resolveRequireMention: resolveMattermostGroupRequireMention,
@@ -105,23 +165,21 @@ export const mattermostPlugin: ChannelPlugin<ResolvedMattermostAccount> = {
return {
ok: false,
error: new Error(
"Delivering to Mattermost requires --to <channelId|user:ID|channel:ID>",
"Delivering to Mattermost requires --to <channelId|@username|user:ID|channel:ID>",
),
};
}
return { ok: true, to: trimmed };
},
sendText: async ({ to, text, accountId, deps, replyToId }) => {
const send = deps?.sendMattermost ?? sendMessageMattermost;
const result = await send(to, text, {
sendText: async ({ to, text, accountId, replyToId }) => {
const result = await sendMessageMattermost(to, text, {
accountId: accountId ?? undefined,
replyToId: replyToId ?? undefined,
});
return { channel: "mattermost", ...result };
},
sendMedia: async ({ to, text, mediaUrl, accountId, deps, replyToId }) => {
const send = deps?.sendMattermost ?? sendMessageMattermost;
const result = await send(to, text, {
sendMedia: async ({ to, text, mediaUrl, accountId, replyToId }) => {
const result = await sendMessageMattermost(to, text, {
accountId: accountId ?? undefined,
mediaUrl,
replyToId: replyToId ?? undefined,

View File

@@ -1,8 +1,13 @@
import { z } from "zod";
import { BlockStreamingCoalesceSchema } from "clawdbot/plugin-sdk";
import {
BlockStreamingCoalesceSchema,
DmPolicySchema,
GroupPolicySchema,
requireOpenAllowFrom,
} from "clawdbot/plugin-sdk";
const MattermostAccountSchema = z
const MattermostAccountSchemaBase = z
.object({
name: z.string().optional(),
capabilities: z.array(z.string()).optional(),
@@ -13,12 +18,36 @@ const MattermostAccountSchema = z
chatmode: z.enum(["oncall", "onmessage", "onchar"]).optional(),
oncharPrefixes: z.array(z.string()).optional(),
requireMention: z.boolean().optional(),
dmPolicy: DmPolicySchema.optional().default("pairing"),
allowFrom: z.array(z.union([z.string(), z.number()])).optional(),
groupAllowFrom: z.array(z.union([z.string(), z.number()])).optional(),
groupPolicy: GroupPolicySchema.optional().default("allowlist"),
textChunkLimit: z.number().int().positive().optional(),
blockStreaming: z.boolean().optional(),
blockStreamingCoalesce: BlockStreamingCoalesceSchema.optional(),
})
.strict();
export const MattermostConfigSchema = MattermostAccountSchema.extend({
accounts: z.record(z.string(), MattermostAccountSchema.optional()).optional(),
const MattermostAccountSchema = MattermostAccountSchemaBase.superRefine((value, ctx) => {
requireOpenAllowFrom({
policy: value.dmPolicy,
allowFrom: value.allowFrom,
ctx,
path: ["allowFrom"],
message:
'channels.mattermost.dmPolicy="open" requires channels.mattermost.allowFrom to include "*"',
});
});
export const MattermostConfigSchema = MattermostAccountSchemaBase.extend({
accounts: z.record(z.string(), MattermostAccountSchema.optional()).optional(),
}).superRefine((value, ctx) => {
requireOpenAllowFrom({
policy: value.dmPolicy,
allowFrom: value.allowFrom,
ctx,
path: ["allowFrom"],
message:
'channels.mattermost.dmPolicy="open" requires channels.mattermost.allowFrom to include "*"',
});
});

View File

@@ -11,4 +11,4 @@ export function resolveMattermostGroupRequireMention(
});
if (typeof account.requireMention === "boolean") return account.requireMention;
return true;
}
}

View File

@@ -112,4 +112,4 @@ export function listEnabledMattermostAccounts(cfg: ClawdbotConfig): ResolvedMatt
return listMattermostAccountIds(cfg)
.map((accountId) => resolveMattermostAccount({ cfg, accountId }))
.filter((account) => account.enabled);
}
}

View File

@@ -205,4 +205,4 @@ export async function uploadMattermostFile(
throw new Error("Mattermost file upload failed");
}
return info;
}
}

View File

@@ -147,4 +147,4 @@ export function resolveThreadSessionKeys(params: {
? `${params.baseSessionKey}:thread:${threadId}`
: params.baseSessionKey;
return { sessionKey, parentSessionKey: params.parentSessionKey };
}
}

View File

@@ -141,6 +141,39 @@ function channelChatType(kind: "dm" | "group" | "channel"): "direct" | "group" |
return "channel";
}
function normalizeAllowEntry(entry: string): string {
const trimmed = entry.trim();
if (!trimmed) return "";
if (trimmed === "*") return "*";
return trimmed
.replace(/^(mattermost|user):/i, "")
.replace(/^@/, "")
.toLowerCase();
}
function normalizeAllowList(entries: Array<string | number>): string[] {
const normalized = entries
.map((entry) => normalizeAllowEntry(String(entry)))
.filter(Boolean);
return Array.from(new Set(normalized));
}
function isSenderAllowed(params: {
senderId: string;
senderName?: string;
allowFrom: string[];
}): boolean {
const allowFrom = params.allowFrom;
if (allowFrom.length === 0) return false;
if (allowFrom.includes("*")) return true;
const normalizedSenderId = normalizeAllowEntry(params.senderId);
const normalizedSenderName = params.senderName ? normalizeAllowEntry(params.senderName) : "";
return allowFrom.some(
(entry) =>
entry === normalizedSenderId || (normalizedSenderName && entry === normalizedSenderName),
);
}
type MattermostMediaInfo = {
path: string;
contentType?: string;
@@ -346,6 +379,122 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
const kind = channelKind(channelType);
const chatType = channelChatType(kind);
const senderName =
payload.data?.sender_name?.trim() ||
(await resolveUserInfo(senderId))?.username?.trim() ||
senderId;
const rawText = post.message?.trim() || "";
const dmPolicy = account.config.dmPolicy ?? "pairing";
const defaultGroupPolicy = cfg.channels?.defaults?.groupPolicy;
const groupPolicy = account.config.groupPolicy ?? defaultGroupPolicy ?? "allowlist";
const configAllowFrom = normalizeAllowList(account.config.allowFrom ?? []);
const configGroupAllowFrom = normalizeAllowList(account.config.groupAllowFrom ?? []);
const storeAllowFrom = normalizeAllowList(
await core.channel.pairing.readAllowFromStore("mattermost").catch(() => []),
);
const effectiveAllowFrom = Array.from(new Set([...configAllowFrom, ...storeAllowFrom]));
const effectiveGroupAllowFrom = Array.from(
new Set([
...(configGroupAllowFrom.length > 0 ? configGroupAllowFrom : configAllowFrom),
...storeAllowFrom,
]),
);
const allowTextCommands = core.channel.commands.shouldHandleTextCommands({
cfg,
surface: "mattermost",
});
const isControlCommand = allowTextCommands && core.channel.text.hasControlCommand(rawText, cfg);
const useAccessGroups = cfg.commands?.useAccessGroups !== false;
const senderAllowedForCommands = isSenderAllowed({
senderId,
senderName,
allowFrom: effectiveAllowFrom,
});
const groupAllowedForCommands = isSenderAllowed({
senderId,
senderName,
allowFrom: effectiveGroupAllowFrom,
});
const commandAuthorized =
kind === "dm"
? dmPolicy === "open" || senderAllowedForCommands
: core.channel.commands.resolveCommandAuthorizedFromAuthorizers({
useAccessGroups,
authorizers: [
{ configured: effectiveAllowFrom.length > 0, allowed: senderAllowedForCommands },
{
configured: effectiveGroupAllowFrom.length > 0,
allowed: groupAllowedForCommands,
},
],
});
if (kind === "dm") {
if (dmPolicy === "disabled") {
logVerboseMessage(`mattermost: drop dm (dmPolicy=disabled sender=${senderId})`);
return;
}
if (dmPolicy !== "open" && !senderAllowedForCommands) {
if (dmPolicy === "pairing") {
const { code, created } = await core.channel.pairing.upsertPairingRequest({
channel: "mattermost",
id: senderId,
meta: { name: senderName },
});
logVerboseMessage(
`mattermost: pairing request sender=${senderId} created=${created}`,
);
if (created) {
try {
await sendMessageMattermost(
`user:${senderId}`,
core.channel.pairing.buildPairingReply({
channel: "mattermost",
idLine: `Your Mattermost user id: ${senderId}`,
code,
}),
{ accountId: account.accountId },
);
opts.statusSink?.({ lastOutboundAt: Date.now() });
} catch (err) {
logVerboseMessage(
`mattermost: pairing reply failed for ${senderId}: ${String(err)}`,
);
}
}
} else {
logVerboseMessage(
`mattermost: drop dm sender=${senderId} (dmPolicy=${dmPolicy})`,
);
}
return;
}
} else {
if (groupPolicy === "disabled") {
logVerboseMessage("mattermost: drop group message (groupPolicy=disabled)");
return;
}
if (groupPolicy === "allowlist") {
if (effectiveGroupAllowFrom.length === 0) {
logVerboseMessage("mattermost: drop group message (no group allowlist)");
return;
}
if (!groupAllowedForCommands) {
logVerboseMessage(
`mattermost: drop group sender=${senderId} (not in groupAllowFrom)`,
);
return;
}
}
}
if (kind !== "dm" && isControlCommand && !commandAuthorized) {
logVerboseMessage(
`mattermost: drop control command from unauthorized sender ${senderId}`,
);
return;
}
const teamId = payload.data?.team_id ?? channelInfo?.team_id ?? undefined;
const channelName = payload.data?.channel_name ?? channelInfo?.name ?? "";
const channelDisplay =
@@ -374,7 +523,6 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
const historyKey = kind === "dm" ? null : sessionKey;
const mentionRegexes = core.channel.mentions.buildMentionRegexes(cfg, route.agentId);
const rawText = post.message?.trim() || "";
const wasMentioned =
kind !== "dm" &&
((botUsername ? rawText.toLowerCase().includes(`@${botUsername.toLowerCase()}`) : false) ||
@@ -384,7 +532,7 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
(post.file_ids?.length
? `[Mattermost ${post.file_ids.length === 1 ? "file" : "files"}]`
: "");
const pendingSender = payload.data?.sender_name?.trim() || senderId;
const pendingSender = senderName;
const recordPendingHistory = () => {
if (!historyKey || historyLimit <= 0) return;
const trimmed = pendingBody.trim();
@@ -402,11 +550,6 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
});
};
const allowTextCommands = core.channel.commands.shouldHandleTextCommands({
cfg,
surface: "mattermost",
});
const isControlCommand = allowTextCommands && core.channel.text.hasControlCommand(rawText, cfg);
const oncharEnabled = account.chatmode === "onchar" && kind !== "dm";
const oncharPrefixes = oncharEnabled ? resolveOncharPrefixes(account.oncharPrefixes) : [];
const oncharResult = oncharEnabled
@@ -414,8 +557,16 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
: { triggered: false, stripped: rawText };
const oncharTriggered = oncharResult.triggered;
const shouldRequireMention = kind === "channel" && (account.requireMention ?? true);
const shouldBypassMention = isControlCommand && shouldRequireMention && !wasMentioned;
const shouldRequireMention =
kind !== "dm" &&
core.channel.groups.resolveRequireMention({
cfg,
channel: "mattermost",
accountId: account.accountId,
groupId: channelId,
}) !== false;
const shouldBypassMention =
isControlCommand && shouldRequireMention && !wasMentioned && commandAuthorized;
const effectiveWasMentioned = wasMentioned || shouldBypassMention || oncharTriggered;
const canDetectMention = Boolean(botUsername) || mentionRegexes.length > 0;
@@ -424,17 +575,12 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
return;
}
if (kind === "channel" && shouldRequireMention && canDetectMention) {
if (kind !== "dm" && shouldRequireMention && canDetectMention) {
if (!effectiveWasMentioned) {
recordPendingHistory();
return;
}
}
const senderName =
payload.data?.sender_name?.trim() ||
(await resolveUserInfo(senderId))?.username?.trim() ||
senderId;
const mediaList = await resolveMattermostMedia(post.file_ids);
const mediaPlaceholder = buildMattermostAttachmentPlaceholder(mediaList);
const bodySource = oncharTriggered ? oncharResult.stripped : rawText;
@@ -499,10 +645,6 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
const to = kind === "dm" ? `user:${senderId}` : `channel:${channelId}`;
const mediaPayload = buildMattermostMediaPayload(mediaList);
const commandAuthorized = core.channel.commands.resolveCommandAuthorizedFromAuthorizers({
useAccessGroups: cfg.commands?.useAccessGroups ?? false,
authorizers: [],
});
const ctxPayload = core.channel.reply.finalizeInboundContext({
Body: combinedBody,
RawBody: bodyText,

View File

@@ -67,4 +67,4 @@ export async function probeMattermost(
} finally {
if (timer) clearTimeout(timer);
}
}
}

View File

@@ -205,4 +205,4 @@ export async function sendMessageMattermost(
messageId: post.id ?? "unknown",
channelId,
};
}
}

View File

@@ -20,7 +20,7 @@ export function normalizeMattermostMessagingTarget(raw: string): string | undefi
}
if (trimmed.startsWith("@")) {
const id = trimmed.slice(1).trim();
return id ? `user:${id}` : undefined;
return id ? `@${id}` : undefined;
}
if (trimmed.startsWith("#")) {
const id = trimmed.slice(1).trim();

View File

@@ -39,4 +39,4 @@ export async function promptAccountId(params: PromptAccountIdParams): Promise<st
);
}
return normalized;
}
}

View File

@@ -184,4 +184,4 @@ export const mattermostOnboardingAdapter: ChannelOnboardingAdapter = {
mattermost: { ...cfg.channels?.mattermost, enabled: false },
},
}),
};
};

View File

@@ -1,4 +1,4 @@
import type { BlockStreamingCoalesceConfig } from "clawdbot/plugin-sdk";
import type { BlockStreamingCoalesceConfig, DmPolicy, GroupPolicy } from "clawdbot/plugin-sdk";
export type MattermostChatMode = "oncall" | "onmessage" | "onchar";
@@ -26,6 +26,14 @@ export type MattermostAccountConfig = {
oncharPrefixes?: string[];
/** Require @mention to respond in channels. Default: true. */
requireMention?: boolean;
/** Direct message policy (pairing/allowlist/open/disabled). */
dmPolicy?: DmPolicy;
/** Allowlist for direct messages (user ids or @usernames). */
allowFrom?: Array<string | number>;
/** Allowlist for group messages (user ids or @usernames). */
groupAllowFrom?: Array<string | number>;
/** Group message policy (allowlist/open/disabled). */
groupPolicy?: GroupPolicy;
/** Outbound text chunk size (chars). Default: 4000. */
textChunkLimit?: number;
/** Disable block streaming for this account. */