fix(mattermost): harden native slash commands

Address Codex follow-up feedback:

- Enforce DM/group allowlist + control-command gating for native slash commands
  (no unconditional CommandAuthorized=true).
- Register callback HTTP routes for all configured callbackPath values
  (top-level + per-account overrides).
- Track whether slash commands were created by this process; only delete
  managed commands on shutdown, leaving pre-existing commands intact.
This commit is contained in:
Echo 2026-02-15 01:27:27 -05:00 committed by Muhammed Mukhthar CM
parent fb720193d9
commit a59b27f2e6
3 changed files with 438 additions and 129 deletions

View File

@ -42,6 +42,8 @@ export type MattermostRegisteredCommand = {
trigger: string;
teamId: string;
token: string;
/** True when this process created the command and should delete it on shutdown. */
managed: boolean;
};
/**
@ -230,6 +232,7 @@ export async function registerSlashCommands(params: {
trigger: spec.trigger,
teamId,
token: existingCmd.token,
managed: false,
});
continue;
}
@ -251,6 +254,7 @@ export async function registerSlashCommands(params: {
trigger: spec.trigger,
teamId,
token: created.token,
managed: true,
});
} catch (err) {
log?.(`mattermost: failed to register command /${spec.trigger}: ${String(err)}`);
@ -270,6 +274,9 @@ export async function cleanupSlashCommands(params: {
}): Promise<void> {
const { client, commands, log } = params;
for (const cmd of commands) {
if (!cmd.managed) {
continue;
}
try {
await deleteMattermostCommand(client, cmd.id);
log?.(`mattermost: deleted command /${cmd.trigger} (id=${cmd.id})`);

View File

@ -11,6 +11,7 @@ import {
createReplyPrefixOptions,
createTypingCallbacks,
logTypingFailure,
resolveControlCommandGate,
} from "openclaw/plugin-sdk";
import type { ResolvedMattermostAccount } from "../mattermost/accounts.js";
import { getMattermostRuntime } from "../runtime.js";
@ -69,6 +70,271 @@ function sendJsonResponse(
res.end(JSON.stringify(body));
}
function normalizeAllowList(entries: Array<string | number>): string[] {
const normalized = entries
.map((entry) => String(entry).trim())
.filter(Boolean)
.map((entry) => entry.toLowerCase());
return Array.from(new Set(normalized));
}
function isSenderAllowed(params: { senderId: string; senderName: string; allowFrom: string[] }) {
const { senderId, senderName, allowFrom } = params;
if (allowFrom.length === 0) {
return false;
}
const allowed = new Set(allowFrom.map((v) => v.toLowerCase()));
const id = senderId.trim().toLowerCase();
const name = senderName.trim().toLowerCase();
return allowed.has(id) || allowed.has(name);
}
type SlashInvocationAuth = {
ok: boolean;
denyResponse?: MattermostSlashCommandResponse;
commandAuthorized: boolean;
channelInfo: MattermostChannel | null;
kind: "direct" | "group" | "channel";
chatType: "direct" | "group" | "channel";
channelName: string;
channelDisplay: string;
roomLabel: string;
};
async function authorizeSlashInvocation(params: {
account: ResolvedMattermostAccount;
cfg: OpenClawConfig;
client: ReturnType<typeof createMattermostClient>;
commandText: string;
channelId: string;
senderId: string;
senderName: string;
}): Promise<SlashInvocationAuth> {
const { account, cfg, client, commandText, channelId, senderId, senderName } = params;
const core = getMattermostRuntime();
// Resolve channel info so we can enforce DM vs group/channel policies.
let channelInfo: MattermostChannel | null = null;
try {
channelInfo = await fetchMattermostChannel(client, channelId);
} catch {
// continue without channel info
}
const channelType = channelInfo?.type ?? undefined;
const isDirectMessage = channelType?.toUpperCase() === "D";
const kind = isDirectMessage
? "direct"
: channelType?.toUpperCase() === "G"
? "group"
: "channel";
const chatType = kind === "direct" ? "direct" : kind === "group" ? "group" : "channel";
const channelName = channelInfo?.name ?? "";
const channelDisplay = channelInfo?.display_name ?? channelName;
const roomLabel = channelName ? `#${channelName}` : channelDisplay || `#${channelId}`;
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 hasControlCommand = core.channel.text.hasControlCommand(commandText, cfg);
const useAccessGroups = cfg.commands?.useAccessGroups !== false;
const senderAllowedForCommands = isSenderAllowed({
senderId,
senderName,
allowFrom: effectiveAllowFrom,
});
const groupAllowedForCommands = isSenderAllowed({
senderId,
senderName,
allowFrom: effectiveGroupAllowFrom,
});
const commandGate = resolveControlCommandGate({
useAccessGroups,
authorizers: [
{ configured: effectiveAllowFrom.length > 0, allowed: senderAllowedForCommands },
{
configured: effectiveGroupAllowFrom.length > 0,
allowed: groupAllowedForCommands,
},
],
allowTextCommands,
hasControlCommand,
});
const commandAuthorized =
kind === "direct"
? dmPolicy === "open" || senderAllowedForCommands
: commandGate.commandAuthorized;
// DM policy enforcement
if (kind === "direct") {
if (dmPolicy === "disabled") {
return {
ok: false,
denyResponse: {
response_type: "ephemeral",
text: "This bot is not accepting direct messages.",
},
commandAuthorized: false,
channelInfo,
kind,
chatType,
channelName,
channelDisplay,
roomLabel,
};
}
if (dmPolicy !== "open" && !senderAllowedForCommands) {
if (dmPolicy === "pairing") {
const { code } = await core.channel.pairing.upsertPairingRequest({
channel: "mattermost",
id: senderId,
meta: { name: senderName },
});
return {
ok: false,
denyResponse: {
response_type: "ephemeral",
text: core.channel.pairing.buildPairingReply({
channel: "mattermost",
idLine: `Your Mattermost user id: ${senderId}`,
code,
}),
},
commandAuthorized: false,
channelInfo,
kind,
chatType,
channelName,
channelDisplay,
roomLabel,
};
}
return {
ok: false,
denyResponse: {
response_type: "ephemeral",
text: "Unauthorized.",
},
commandAuthorized: false,
channelInfo,
kind,
chatType,
channelName,
channelDisplay,
roomLabel,
};
}
} else {
// Group/channel policy enforcement
if (groupPolicy === "disabled") {
return {
ok: false,
denyResponse: {
response_type: "ephemeral",
text: "Slash commands are disabled in channels.",
},
commandAuthorized: false,
channelInfo,
kind,
chatType,
channelName,
channelDisplay,
roomLabel,
};
}
if (groupPolicy === "allowlist") {
if (effectiveGroupAllowFrom.length === 0) {
return {
ok: false,
denyResponse: {
response_type: "ephemeral",
text: "Slash commands are not configured for this channel (no allowlist).",
},
commandAuthorized: false,
channelInfo,
kind,
chatType,
channelName,
channelDisplay,
roomLabel,
};
}
if (!groupAllowedForCommands) {
return {
ok: false,
denyResponse: {
response_type: "ephemeral",
text: "Unauthorized.",
},
commandAuthorized: false,
channelInfo,
kind,
chatType,
channelName,
channelDisplay,
roomLabel,
};
}
}
if (commandGate.shouldBlock) {
return {
ok: false,
denyResponse: {
response_type: "ephemeral",
text: "Unauthorized.",
},
commandAuthorized: false,
channelInfo,
kind,
chatType,
channelName,
channelDisplay,
roomLabel,
};
}
}
return {
ok: true,
commandAuthorized,
channelInfo,
kind,
chatType,
channelName,
channelDisplay,
roomLabel,
};
}
/**
* Create the HTTP request handler for Mattermost slash command callbacks.
*
@ -77,7 +343,6 @@ function sendJsonResponse(
*/
export function createSlashCommandHttpHandler(params: SlashHttpHandlerParams) {
const { account, cfg, runtime, commandTokens, log } = params;
const core = getMattermostRuntime();
const MAX_BODY_BYTES = 64 * 1024; // 64KB
@ -125,6 +390,30 @@ export function createSlashCommandHttpHandler(params: SlashHttpHandlerParams) {
const senderId = payload.user_id;
const senderName = payload.user_name ?? senderId;
const client = createMattermostClient({
baseUrl: account.baseUrl ?? "",
botToken: account.botToken ?? "",
});
const auth = await authorizeSlashInvocation({
account,
cfg,
client,
commandText,
channelId,
senderId,
senderName,
});
if (!auth.ok) {
sendJsonResponse(
res,
200,
auth.denyResponse ?? { response_type: "ephemeral", text: "Unauthorized." },
);
return;
}
log?.(`mattermost: slash command /${trigger} from ${senderName} in ${channelId}`);
// Acknowledge immediately — we'll send the actual reply asynchronously
@ -139,12 +428,19 @@ export function createSlashCommandHttpHandler(params: SlashHttpHandlerParams) {
account,
cfg,
runtime,
client,
commandText,
channelId,
senderId,
senderName,
teamId: payload.team_id,
triggerId: payload.trigger_id,
kind: auth.kind,
chatType: auth.chatType,
channelName: auth.channelName,
channelDisplay: auth.channelDisplay,
roomLabel: auth.roomLabel,
commandAuthorized: auth.commandAuthorized,
log,
});
} catch (err) {
@ -165,49 +461,42 @@ async function handleSlashCommandAsync(params: {
account: ResolvedMattermostAccount;
cfg: OpenClawConfig;
runtime: RuntimeEnv;
client: ReturnType<typeof createMattermostClient>;
commandText: string;
channelId: string;
senderId: string;
senderName: string;
teamId: string;
kind: "direct" | "group" | "channel";
chatType: "direct" | "group" | "channel";
channelName: string;
channelDisplay: string;
roomLabel: string;
commandAuthorized: boolean;
triggerId?: string;
log?: (msg: string) => void;
}) {
const { account, cfg, runtime, commandText, channelId, senderId, senderName, teamId, log } =
params;
const {
account,
cfg,
runtime,
client,
commandText,
channelId,
senderId,
senderName,
teamId,
kind,
chatType,
channelName,
channelDisplay,
roomLabel,
commandAuthorized,
triggerId,
log,
} = params;
const core = getMattermostRuntime();
// Resolve channel info
const client = createMattermostClient({
baseUrl: account.baseUrl ?? "",
botToken: account.botToken ?? "",
});
let channelInfo: MattermostChannel | null = null;
try {
channelInfo = await fetchMattermostChannel(client, channelId);
} catch {
// continue without channel info
}
const channelType = channelInfo?.type ?? undefined;
const isDirectMessage = channelType?.toUpperCase() === "D";
const kind = isDirectMessage
? "direct"
: channelType?.toUpperCase() === "G"
? "group"
: "channel";
const chatType =
kind === "direct"
? ("direct" as const)
: kind === "group"
? ("group" as const)
: ("channel" as const);
const channelName = channelInfo?.name ?? "";
const channelDisplay = channelInfo?.display_name ?? channelName;
const roomLabel = channelName ? `#${channelName}` : channelDisplay || `#${channelId}`;
const route = core.channel.routing.resolveAgentRoute({
cfg,
channel: "mattermost",
@ -248,10 +537,10 @@ async function handleSlashCommandAsync(params: {
SenderId: senderId,
Provider: "mattermost" as const,
Surface: "mattermost" as const,
MessageSid: params.triggerId ?? `slash-${Date.now()}`,
MessageSid: triggerId ?? `slash-${Date.now()}`,
Timestamp: Date.now(),
WasMentioned: true,
CommandAuthorized: true, // Slash commands bypass mention requirements
CommandAuthorized: commandAuthorized,
CommandSource: "native" as const,
OriginatingChannel: "mattermost" as const,
OriginatingTo: to,

View File

@ -116,16 +116,47 @@ export function deactivateSlashCommands(accountId?: string) {
*/
export function registerSlashCommandRoute(api: OpenClawPluginApi) {
const mmConfig = api.config.channels?.mattermost as Record<string, unknown> | undefined;
// Collect callback paths from both top-level and per-account config.
// Command registration uses account.config.commands, so the HTTP route
// registration must include any account-specific callbackPath overrides.
const callbackPaths = new Set<string>();
const commandsRaw = mmConfig?.commands as
| Partial<import("./slash-commands.js").MattermostSlashCommandConfig>
| undefined;
const slashConfig = resolveSlashCommandConfig(commandsRaw);
const callbackPath = slashConfig.callbackPath;
callbackPaths.add(resolveSlashCommandConfig(commandsRaw).callbackPath);
api.registerHttpRoute({
path: callbackPath,
handler: async (req: IncomingMessage, res: ServerResponse) => {
if (accountStates.size === 0) {
const accountsRaw = (mmConfig?.accounts ?? {}) as Record<string, unknown>;
for (const accountId of Object.keys(accountsRaw)) {
const accountCfg = accountsRaw[accountId] as Record<string, unknown> | undefined;
const accountCommandsRaw = accountCfg?.commands as
| Partial<import("./slash-commands.js").MattermostSlashCommandConfig>
| undefined;
callbackPaths.add(resolveSlashCommandConfig(accountCommandsRaw).callbackPath);
}
const routeHandler = async (req: IncomingMessage, res: ServerResponse) => {
if (accountStates.size === 0) {
res.statusCode = 503;
res.setHeader("Content-Type", "application/json; charset=utf-8");
res.end(
JSON.stringify({
response_type: "ephemeral",
text: "Slash commands are not yet initialized. Please try again in a moment.",
}),
);
return;
}
// We need to peek at the token to route to the right account handler.
// Since each account handler also validates the token, we find the
// account whose token set contains the inbound token and delegate.
// If there's only one active account (common case), route directly.
if (accountStates.size === 1) {
const [, state] = [...accountStates.entries()][0]!;
if (!state.handler) {
res.statusCode = 503;
res.setHeader("Content-Type", "application/json; charset=utf-8");
res.end(
@ -136,105 +167,87 @@ export function registerSlashCommandRoute(api: OpenClawPluginApi) {
);
return;
}
await state.handler(req, res);
return;
}
// We need to peek at the token to route to the right account handler.
// Since each account handler also validates the token, we find the
// account whose token set contains the inbound token and delegate.
// If none match, we pick the first handler and let its own validation
// reject the request (fail closed).
// For multi-account routing: the handlers read the body themselves,
// so we can't pre-parse here without buffering. Instead, if there's
// only one active account (common case), route directly.
if (accountStates.size === 1) {
const [, state] = [...accountStates.entries()][0]!;
if (!state.handler) {
res.statusCode = 503;
res.setHeader("Content-Type", "application/json; charset=utf-8");
res.end(
JSON.stringify({
response_type: "ephemeral",
text: "Slash commands are not yet initialized. Please try again in a moment.",
}),
);
return;
}
await state.handler(req, res);
// Multi-account: buffer the body, find the matching account by token,
// then replay the request to the correct handler.
const chunks: Buffer[] = [];
const MAX_BODY = 64 * 1024;
let size = 0;
for await (const chunk of req) {
size += (chunk as Buffer).length;
if (size > MAX_BODY) {
res.statusCode = 413;
res.end("Payload Too Large");
return;
}
chunks.push(chunk as Buffer);
}
const bodyStr = Buffer.concat(chunks).toString("utf8");
// Multi-account: buffer the body, find the matching account by token,
// then replay the request to the correct handler.
const chunks: Buffer[] = [];
const MAX_BODY = 64 * 1024;
let size = 0;
for await (const chunk of req) {
size += (chunk as Buffer).length;
if (size > MAX_BODY) {
res.statusCode = 413;
res.end("Payload Too Large");
return;
}
chunks.push(chunk as Buffer);
// Parse just the token to find the right account
let token: string | null = null;
const ct = req.headers["content-type"] ?? "";
try {
if (ct.includes("application/json")) {
token = (JSON.parse(bodyStr) as { token?: string }).token ?? null;
} else {
token = new URLSearchParams(bodyStr).get("token");
}
const bodyStr = Buffer.concat(chunks).toString("utf8");
} catch {
// parse failed — will be caught by handler
}
// Parse just the token to find the right account
let token: string | null = null;
const ct = req.headers["content-type"] ?? "";
try {
if (ct.includes("application/json")) {
token = (JSON.parse(bodyStr) as { token?: string }).token ?? null;
} else {
token = new URLSearchParams(bodyStr).get("token");
}
} catch {
// parse failed — will be caught by handler
}
// Find the account whose tokens include this one
let matchedHandler: ((req: IncomingMessage, res: ServerResponse) => Promise<void>) | null =
null;
// Find the account whose tokens include this one
let matchedHandler: ((req: IncomingMessage, res: ServerResponse) => Promise<void>) | null =
null;
if (token) {
for (const [, state] of accountStates) {
if (state.commandTokens.has(token) && state.handler) {
matchedHandler = state.handler;
break;
}
if (token) {
for (const [, state] of accountStates) {
if (state.commandTokens.has(token) && state.handler) {
matchedHandler = state.handler;
break;
}
}
}
if (!matchedHandler) {
// No matching account — reject
res.statusCode = 401;
res.setHeader("Content-Type", "application/json; charset=utf-8");
res.end(
JSON.stringify({
response_type: "ephemeral",
text: "Unauthorized: invalid command token.",
}),
);
return;
}
if (!matchedHandler) {
// No matching account — reject
res.statusCode = 401;
res.setHeader("Content-Type", "application/json; charset=utf-8");
res.end(
JSON.stringify({
response_type: "ephemeral",
text: "Unauthorized: invalid command token.",
}),
);
return;
}
// Replay: create a synthetic readable that re-emits the buffered body
const { Readable } = await import("node:stream");
const syntheticReq = new Readable({
read() {
this.push(Buffer.from(bodyStr, "utf8"));
this.push(null);
},
}) as IncomingMessage;
// Replay: create a synthetic readable that re-emits the buffered body
const { Readable } = await import("node:stream");
const syntheticReq = new Readable({
read() {
this.push(Buffer.from(bodyStr, "utf8"));
this.push(null);
},
}) as IncomingMessage;
// Copy necessary IncomingMessage properties
syntheticReq.method = req.method;
syntheticReq.url = req.url;
syntheticReq.headers = req.headers;
// Copy necessary IncomingMessage properties
syntheticReq.method = req.method;
syntheticReq.url = req.url;
syntheticReq.headers = req.headers;
await matchedHandler(syntheticReq, res);
},
});
await matchedHandler(syntheticReq, res);
};
api.logger.info?.(`mattermost: registered slash command callback at ${callbackPath}`);
for (const callbackPath of callbackPaths) {
api.registerHttpRoute({
path: callbackPath,
handler: routeHandler,
});
api.logger.info?.(`mattermost: registered slash command callback at ${callbackPath}`);
}
}