nyrn
/
openclaw
mirror of https://github.com/openclaw/openclaw.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Mattermost: address slash command ownership and auth review findings

This commit is contained in:
Muhammed Mukhthar CM 2026-03-03 05:50:24 +00:00
parent 67f20c6609
commit 989126574e
4 changed files with 164 additions and 99 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

70
extensions/mattermost/src/mattermost/monitor.ts
View File

@ -299,49 +299,59 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
});
const allRegistered: import("./slash-commands.js").MattermostRegisteredCommand[] = [];
let teamRegistrationFailures = 0;
try {
for (const team of teams) {
for (const team of teams) {
try {
const registered = await registerSlashCommands({
client,
teamId: team.id,
creatorUserId: botUserId,
callbackUrl,
commands: dedupedCommands,
log: (msg) => runtime.log?.(msg),
});
allRegistered.push(...registered);
} catch (err) {
teamRegistrationFailures += 1;
runtime.error?.(
`mattermost: failed to register slash commands for team ${team.id}: ${String(err)}`,
);
}
} catch (err) {
// If we partially succeeded (some teams had commands created) but later failed,
// clean up the created commands so we don't strand registrations that will 503.
await cleanupSlashCommands({
client,
commands: allRegistered,
}
if (allRegistered.length === 0) {
runtime.error?.(
"mattermost: native slash commands enabled but no commands could be registered; keeping slash callbacks inactive",
);
} else {
if (teamRegistrationFailures > 0) {
runtime.error?.(
`mattermost: slash command registration completed with ${teamRegistrationFailures} team error(s)`,
);
}
// Build trigger→originalName map for accurate command name resolution
const triggerMap = new Map<string, string>();
for (const cmd of dedupedCommands) {
if (cmd.originalName) {
triggerMap.set(cmd.trigger, cmd.originalName);
}
}
activateSlashCommands({
account,
commandTokens: allRegistered.map((cmd) => cmd.token).filter(Boolean),
registeredCommands: allRegistered,
triggerMap,
api: { cfg, runtime },
log: (msg) => runtime.log?.(msg),
});
throw err;
runtime.log?.(
`mattermost: slash commands registered (${allRegistered.length} commands across ${teams.length} teams, callback=${callbackUrl})`,
);
}
// Build trigger→originalName map for accurate command name resolution
const triggerMap = new Map<string, string>();
for (const cmd of dedupedCommands) {
if (cmd.originalName) {
triggerMap.set(cmd.trigger, cmd.originalName);
}
}
activateSlashCommands({
account,
commandTokens: allRegistered.map((cmd) => cmd.token).filter(Boolean),
registeredCommands: allRegistered,
triggerMap,
api: { cfg, runtime },
log: (msg) => runtime.log?.(msg),
});
runtime.log?.(
`mattermost: slash commands registered (${allRegistered.length} commands across ${teams.length} teams, callback=${callbackUrl})`,
);
} catch (err) {
runtime.error?.(`mattermost: failed to register slash commands: ${String(err)}`);
}

43
extensions/mattermost/src/mattermost/slash-commands.test.ts
View File

@ -81,6 +81,7 @@ describe("slash-commands", () => {
id: "cmd-1",
token: "tok-1",
team_id: "team-1",
creator_id: "bot-user",
trigger: "oc_status",
method: "P",
url: "http://gateway/callback",
@ -95,6 +96,7 @@ describe("slash-commands", () => {
const result = await registerSlashCommands({
client,
teamId: "team-1",
creatorUserId: "bot-user",
callbackUrl: "http://gateway/callback",
commands: [
{
@ -110,4 +112,45 @@ describe("slash-commands", () => {
expect(result[0]?.id).toBe("cmd-1");
expect(request).toHaveBeenCalledTimes(1);
});
it("skips foreign command trigger collisions instead of mutating non-owned commands", async () => {
const request = vi.fn(async (path: string, init?: { method?: string }) => {
if (path.startsWith("/commands?team_id=")) {
return [
{
id: "cmd-foreign-1",
token: "tok-foreign-1",
team_id: "team-1",
creator_id: "another-bot-user",
trigger: "oc_status",
method: "P",
url: "http://foreign/callback",
auto_complete: true,
},
];
}
if (init?.method === "POST" || init?.method === "PUT" || init?.method === "DELETE") {
throw new Error("should not mutate foreign commands");
}
throw new Error(`unexpected request path: ${path}`);
});
const client = { request } as unknown as MattermostClient;
const result = await registerSlashCommands({
client,
teamId: "team-1",
creatorUserId: "bot-user",
callbackUrl: "http://gateway/callback",
commands: [
{
trigger: "oc_status",
description: "status",
autoComplete: true,
},
],
});
expect(result).toHaveLength(0);
expect(request).toHaveBeenCalledTimes(1);
});
});

39
extensions/mattermost/src/mattermost/slash-commands.ts
View File

@ -239,11 +239,16 @@ export async function updateMattermostCommand(
export async function registerSlashCommands(params: {
client: MattermostClient;
teamId: string;
creatorUserId: string;
callbackUrl: string;
commands: MattermostCommandSpec[];
log?: (msg: string) => void;
}): Promise<MattermostRegisteredCommand[]> {
const { client, teamId, callbackUrl, commands, log } = params;
const { client, teamId, creatorUserId, callbackUrl, commands, log } = params;
const normalizedCreatorUserId = creatorUserId.trim();
if (!normalizedCreatorUserId) {
throw new Error("creatorUserId is required for slash command reconciliation");
}
// Fetch existing commands to avoid duplicates
let existing: MattermostCommandResponse[] = [];
@ -257,12 +262,38 @@ export async function registerSlashCommands(params: {
throw err;
}
const existingByTrigger = new Map(existing.map((cmd) => [cmd.trigger, cmd]));
const existingByTrigger = new Map<string, MattermostCommandResponse[]>();
for (const cmd of existing) {
const list = existingByTrigger.get(cmd.trigger) ?? [];
list.push(cmd);
existingByTrigger.set(cmd.trigger, list);
}
const registered: MattermostRegisteredCommand[] = [];
for (const spec of commands) {
const existingCmd = existingByTrigger.get(spec.trigger);
const existingForTrigger = existingByTrigger.get(spec.trigger) ?? [];
const ownedCommands = existingForTrigger.filter(
(cmd) => cmd.creator_id?.trim() === normalizedCreatorUserId,
);
const foreignCommands = existingForTrigger.filter(
(cmd) => cmd.creator_id?.trim() !== normalizedCreatorUserId,
);
if (ownedCommands.length === 0 && foreignCommands.length > 0) {
log?.(
`mattermost: trigger /${spec.trigger} already used by non-OpenClaw command(s); skipping to avoid mutating external integrations`,
);
continue;
}
if (ownedCommands.length > 1) {
log?.(
`mattermost: multiple owned commands found for /${spec.trigger}; using the first and leaving extras untouched`,
);
}
const existingCmd = ownedCommands[0];
// Already registered with the correct callback URL
if (existingCmd && existingCmd.url === callbackUrl) {
@ -307,7 +338,7 @@ export async function registerSlashCommands(params: {
log?.(
`mattermost: failed to update command /${spec.trigger} (id=${existingCmd.id}): ${String(err)}`,
);
// Fallback: try delete+recreate (safe for the oc_* namespace).
// Fallback: try delete+recreate for commands owned by this bot user.
try {
await deleteMattermostCommand(client, existingCmd.id);
log?.(`mattermost: deleted stale command /${spec.trigger} (id=${existingCmd.id})`);

111
extensions/mattermost/src/mattermost/slash-http.ts
View File

@ -10,6 +10,7 @@ import type { OpenClawConfig, ReplyPayload, RuntimeEnv } from "openclaw/plugin-s
import {
createReplyPrefixOptions,
createTypingCallbacks,
isDangerousNameMatchingEnabled,
logTypingFailure,
resolveControlCommandGate,
} from "openclaw/plugin-sdk";
@ -23,6 +24,11 @@ import {
sendMattermostTyping,
type MattermostChannel,
} from "./client.js";
import {
isMattermostSenderAllowed,
normalizeMattermostAllowList,
resolveMattermostEffectiveAllowFromLists,
} from "./monitor-auth.js";
import { sendMessageMattermost } from "./send.js";
import {
parseSlashCommandPayload,
@ -72,46 +78,6 @@ function sendJsonResponse(
res.end(JSON.stringify(body));
}
/**
* Normalize a single allowlist entry, matching the websocket monitor behaviour.
* Strips `mattermost:`, `user:`, and `@` prefixes, and preserves the `*` wildcard.
*/
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[] }) {
const { senderId, senderName, allowFrom } = params;
if (allowFrom.length === 0) {
return false;
}
if (allowFrom.includes("*")) {
return true;
}
const normalizedId = normalizeAllowEntry(senderId);
const normalizedName = senderName ? normalizeAllowEntry(senderName) : "";
return allowFrom.some(
(entry) => entry === normalizedId || (normalizedName && entry === normalizedName),
);
}
type SlashInvocationAuth = {
ok: boolean;
denyResponse?: MattermostSlashCommandResponse;
@ -181,10 +147,11 @@ async function authorizeSlashInvocation(params: {
const dmPolicy = account.config.dmPolicy ?? "pairing";
const defaultGroupPolicy = cfg.channels?.defaults?.groupPolicy;
const groupPolicy = account.config.groupPolicy ?? defaultGroupPolicy ?? "allowlist";
const allowNameMatching = isDangerousNameMatchingEnabled(account.config);
const configAllowFrom = normalizeAllowList(account.config.allowFrom ?? []);
const configGroupAllowFrom = normalizeAllowList(account.config.groupAllowFrom ?? []);
const storeAllowFrom = normalizeAllowList(
const configAllowFrom = normalizeMattermostAllowList(account.config.allowFrom ?? []);
const configGroupAllowFrom = normalizeMattermostAllowList(account.config.groupAllowFrom ?? []);
const storeAllowFrom = normalizeMattermostAllowList(
await core.channel.pairing
.readAllowFromStore({
channel: "mattermost",
@ -192,13 +159,12 @@ async function authorizeSlashInvocation(params: {
})
.catch(() => []),
);
const effectiveAllowFrom = Array.from(new Set([...configAllowFrom, ...storeAllowFrom]));
const effectiveGroupAllowFrom = Array.from(
new Set([
...(configGroupAllowFrom.length > 0 ? configGroupAllowFrom : configAllowFrom),
...storeAllowFrom,
]),
);
const { effectiveAllowFrom, effectiveGroupAllowFrom } = resolveMattermostEffectiveAllowFromLists({
allowFrom: configAllowFrom,
groupAllowFrom: configGroupAllowFrom,
storeAllowFrom,
dmPolicy,
});
const allowTextCommands = core.channel.commands.shouldHandleTextCommands({
cfg,
@ -206,24 +172,33 @@ async function authorizeSlashInvocation(params: {
});
const hasControlCommand = core.channel.text.hasControlCommand(commandText, cfg);
const useAccessGroups = cfg.commands?.useAccessGroups !== false;
const commandDmAllowFrom = kind === "direct" ? effectiveAllowFrom : configAllowFrom;
const commandGroupAllowFrom =
kind === "direct"
? effectiveGroupAllowFrom
: configGroupAllowFrom.length > 0
? configGroupAllowFrom
: configAllowFrom;
const senderAllowedForCommands = isSenderAllowed({
const senderAllowedForCommands = isMattermostSenderAllowed({
senderId,
senderName,
allowFrom: effectiveAllowFrom,
allowFrom: commandDmAllowFrom,
allowNameMatching,
});
const groupAllowedForCommands = isSenderAllowed({
const groupAllowedForCommands = isMattermostSenderAllowed({
senderId,
senderName,
allowFrom: effectiveGroupAllowFrom,
allowFrom: commandGroupAllowFrom,
allowNameMatching,
});
const commandGate = resolveControlCommandGate({
useAccessGroups,
authorizers: [
{ configured: effectiveAllowFrom.length > 0, allowed: senderAllowedForCommands },
{ configured: commandDmAllowFrom.length > 0, allowed: senderAllowedForCommands },
{
configured: effectiveGroupAllowFrom.length > 0,
configured: commandGroupAllowFrom.length > 0,
allowed: groupAllowedForCommands,
},
],
@ -661,16 +636,22 @@ async function handleSlashCommandAsync(params: {
onReplyStart: typingCallbacks.onReplyStart,
});
await core.channel.reply.dispatchReplyFromConfig({
ctx: ctxPayload,
cfg,
await core.channel.reply.withReplyDispatcher({
dispatcher,
replyOptions: {
...replyOptions,
disableBlockStreaming:
typeof account.blockStreaming === "boolean" ? !account.blockStreaming : undefined,
onModelSelected,
onSettled: () => {
markDispatchIdle();
},
run: () =>
core.channel.reply.dispatchReplyFromConfig({
ctx: ctxPayload,
cfg,
dispatcher,
replyOptions: {
...replyOptions,
disableBlockStreaming:
typeof account.blockStreaming === "boolean" ? !account.blockStreaming : undefined,
onModelSelected,
},
}),
});
markDispatchIdle();
}