Fix Mattermost open DM validation

This commit is contained in:
Alex Knight
2026-06-15 19:34:42 +10:00
committed by Alex Knight
parent c12d921291
commit ba91eb7acf
9 changed files with 245 additions and 1 deletions

View File

@@ -63,6 +63,7 @@ import { collectRuntimeConfigAssignments, secretTargetRegistryEntries } from "./
import { resolveMattermostOutboundSessionRoute } from "./session-route.js";
import { mattermostSetupAdapter } from "./setup-core.js";
import { mattermostSetupWizard } from "./setup-surface.js";
import { collectMattermostStatusIssues } from "./status-issues.js";
import type { MattermostConfig } from "./types.js";
const loadMattermostChannelRuntime = createLazyRuntimeModule(() => import("./channel.runtime.js"));
@@ -834,6 +835,7 @@ export const mattermostPlugin: ChannelPlugin<ResolvedMattermostAccount> = create
lastConnectedAt: null,
lastDisconnect: null,
}),
collectStatusIssues: collectMattermostStatusIssues,
buildChannelSummary: ({ snapshot }) =>
buildPassiveProbedChannelStatusSummary(snapshot, {
botTokenSource: snapshot.botTokenSource ?? "none",
@@ -858,6 +860,8 @@ export const mattermostPlugin: ChannelPlugin<ResolvedMattermostAccount> = create
extra: {
botTokenSource: account.botTokenSource,
baseUrl: account.baseUrl,
dmPolicy: account.config.dmPolicy ?? "pairing",
allowFrom: account.config.allowFrom ?? [],
connected: runtime?.connected ?? false,
lastConnectedAt: runtime?.lastConnectedAt ?? null,
lastDisconnect: runtime?.lastDisconnect ?? null,

View File

@@ -30,6 +30,21 @@ describe("MattermostConfigSchema", () => {
expect(result.success).toBe(true);
});
it('rejects dmPolicy="open" without wildcard allowFrom', () => {
const result = MattermostConfigSchema.safeParse({
dmPolicy: "open",
});
expect(result.success).toBe(false);
});
it('accepts dmPolicy="open" with wildcard allowFrom', () => {
const result = MattermostConfigSchema.safeParse({
dmPolicy: "open",
allowFrom: ["*"],
});
expect(result.success).toBe(true);
});
it("accepts documented streaming modes and progress config", () => {
const result = MattermostConfigSchema.safeParse({
streaming: {

View File

@@ -11,6 +11,7 @@ vi.mock("./runtime-api.js", () => ({
describe("mattermost monitor auth", () => {
let authorizeMattermostCommandInvocation: typeof import("./monitor-auth.js").authorizeMattermostCommandInvocation;
let formatMattermostDirectMessageDropLog: typeof import("./monitor-auth.js").formatMattermostDirectMessageDropLog;
let isMattermostSenderAllowed: typeof import("./monitor-auth.js").isMattermostSenderAllowed;
let normalizeMattermostAllowEntry: typeof import("./monitor-auth.js").normalizeMattermostAllowEntry;
let normalizeMattermostAllowList: typeof import("./monitor-auth.js").normalizeMattermostAllowList;
@@ -18,6 +19,7 @@ describe("mattermost monitor auth", () => {
beforeAll(async () => {
({
authorizeMattermostCommandInvocation,
formatMattermostDirectMessageDropLog,
isMattermostSenderAllowed,
normalizeMattermostAllowEntry,
normalizeMattermostAllowList,
@@ -58,6 +60,18 @@ describe("mattermost monitor auth", () => {
});
});
it("formats direct-message drops with the ingress reason and open-policy hint", () => {
expect(
formatMattermostDirectMessageDropLog({
senderId: "alice-id",
dmPolicy: "open",
reasonCode: "dm_policy_not_allowlisted",
}),
).toBe(
"mattermost: drop dm sender=alice-id (dmPolicy=open reason=dm_policy_not_allowlisted hint=add-allowFrom-wildcard)",
);
});
it("resolves direct command authorization from shared ingress", async () => {
isDangerousNameMatchingEnabled.mockReturnValue(false);
resolveAllowlistMatchSimple.mockReturnValue({ allowed: false });

View File

@@ -61,6 +61,19 @@ export function normalizeMattermostAllowList(entries: Array<string | number>): s
return uniqueStrings(normalized);
}
export function formatMattermostDirectMessageDropLog(params: {
senderId: string;
dmPolicy: string;
reasonCode?: string;
}): string {
const reason = params.reasonCode ? ` reason=${params.reasonCode}` : "";
const hint =
params.dmPolicy === "open" && params.reasonCode === "dm_policy_not_allowlisted"
? " hint=add-allowFrom-wildcard"
: "";
return `mattermost: drop dm sender=${params.senderId} (dmPolicy=${params.dmPolicy}${reason}${hint})`;
}
export function isMattermostSenderAllowed(params: {
senderId: string;
senderName?: string;

View File

@@ -57,6 +57,7 @@ import {
} from "./model-picker.js";
import {
authorizeMattermostCommandInvocation,
formatMattermostDirectMessageDropLog,
normalizeMattermostAllowEntry,
resolveMattermostMonitorInboundAccess,
} from "./monitor-auth.js";
@@ -1391,7 +1392,13 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
}
return;
}
logVerboseMessage(`mattermost: drop dm sender=${senderId} (dmPolicy=${dmPolicy})`);
logVerboseMessage(
formatMattermostDirectMessageDropLog({
senderId,
dmPolicy,
reasonCode: accessDecision.senderAccess.reasonCode,
}),
);
return;
}
if (accessDecision.ingress.reasonCode === "group_policy_disabled") {

View File

@@ -0,0 +1,32 @@
// Mattermost tests cover status issues plugin behavior.
import { expectOpenDmPolicyConfigIssue } from "openclaw/plugin-sdk/channel-test-helpers";
import { describe, expect, it } from "vitest";
import { collectMattermostStatusIssues } from "./status-issues.js";
describe("collectMattermostStatusIssues", () => {
it("warns when dmPolicy is open without a wildcard allowlist", () => {
expectOpenDmPolicyConfigIssue({
collectIssues: collectMattermostStatusIssues,
account: {
accountId: "default",
enabled: true,
configured: true,
dmPolicy: "open",
},
});
});
it("allows open dmPolicy when allowFrom includes the wildcard", () => {
expect(
collectMattermostStatusIssues([
{
accountId: "default",
enabled: true,
configured: true,
dmPolicy: "open",
allowFrom: ["*"],
},
]),
).toEqual([]);
});
});

View File

@@ -0,0 +1,51 @@
// Mattermost plugin module implements status issue collection.
import type {
ChannelAccountSnapshot,
ChannelStatusIssue,
} from "openclaw/plugin-sdk/channel-contract";
import {
coerceStatusIssueAccountId,
readStatusIssueFields,
} from "openclaw/plugin-sdk/extension-shared";
const MATTERMOST_STATUS_FIELDS = [
"accountId",
"enabled",
"configured",
"dmPolicy",
"allowFrom",
] as const;
function hasWildcardAllowFrom(value: unknown): boolean {
return Array.isArray(value) && value.some((entry) => String(entry).trim() === "*");
}
export function collectMattermostStatusIssues(
accounts: ChannelAccountSnapshot[],
): ChannelStatusIssue[] {
const issues: ChannelStatusIssue[] = [];
for (const entry of accounts) {
const account = readStatusIssueFields(entry, MATTERMOST_STATUS_FIELDS);
if (!account) {
continue;
}
const accountId = coerceStatusIssueAccountId(account.accountId) ?? "default";
const enabled = account.enabled !== false;
const configured = account.configured === true;
if (!enabled || !configured) {
continue;
}
if (account.dmPolicy === "open" && !hasWildcardAllowFrom(account.allowFrom)) {
issues.push({
channel: "mattermost",
accountId,
kind: "config",
message:
'Mattermost dmPolicy is "open" but allowFrom does not include "*"; public DMs will be dropped.',
fix: 'Add "*" to channels.mattermost.allowFrom (or the account-specific allowFrom) or set dmPolicy to "pairing"/"allowlist".',
});
}
}
return issues;
}

View File

@@ -284,6 +284,60 @@ describe("validateConfigObjectWithPlugins channel metadata (applyDefaults: true)
expect(result.config.channels?.discord?.accounts?.work?.agentComponents?.ttlMs).toBe(60_000);
}
});
it('rejects Mattermost dmPolicy="open" without wildcard allowFrom', () => {
const result = validateConfigObjectWithPlugins({
channels: {
mattermost: {
enabled: true,
baseUrl: "https://chat.example.com",
botToken: "test-token",
dmPolicy: "open",
},
},
});
expect(result.ok).toBe(false);
if (!result.ok) {
expect(result.issues).toContainEqual(
expect.objectContaining({
path: "channels.mattermost.allowFrom",
message: expect.stringContaining(
'channels.mattermost.dmPolicy="open" requires channels.mattermost.allowFrom to include "*"',
),
}),
);
}
});
it('rejects account-scoped Mattermost dmPolicy="open" without wildcard allowFrom', () => {
const result = validateConfigObjectWithPlugins({
channels: {
mattermost: {
accounts: {
work: {
enabled: true,
baseUrl: "https://chat.example.com",
botToken: "test-token",
dmPolicy: "open",
},
},
},
},
});
expect(result.ok).toBe(false);
if (!result.ok) {
expect(result.issues).toContainEqual(
expect.objectContaining({
path: "channels.mattermost.accounts.work.allowFrom",
message: expect.stringContaining(
'channels.mattermost.accounts.work.dmPolicy="open" requires channels.mattermost.accounts.work.allowFrom to include "*"',
),
}),
);
}
});
});
describe("validateConfigObjectRawWithPlugins channel metadata", () => {

View File

@@ -311,6 +311,56 @@ function collectAllowedValuesFromBundledChannelSchemaPath(
function formatRawChannelConfigIssueMessage(message: string): string {
return `invalid config: ${message}`;
}
function hasWildcardAllowFrom(value: unknown): boolean {
return Array.isArray(value) && value.some((entry) => String(entry).trim() === "*");
}
function collectMattermostOpenDmAllowFromIssues(
value: unknown,
pathPrefix: string,
): ConfigValidationIssue[] {
if (!isRecord(value)) {
return [];
}
const issues: ConfigValidationIssue[] = [];
if (value.dmPolicy === "open" && !hasWildcardAllowFrom(value.allowFrom)) {
issues.push({
path: `${pathPrefix}.allowFrom`,
message: formatRawChannelConfigIssueMessage(
`${pathPrefix}.dmPolicy="open" requires ${pathPrefix}.allowFrom to include "*"`,
),
});
}
if (isRecord(value.accounts)) {
for (const [accountId, accountConfig] of Object.entries(value.accounts)) {
if (!isRecord(accountConfig)) {
continue;
}
const accountPath = `${pathPrefix}.accounts.${accountId}`;
if (accountConfig.dmPolicy === "open" && !hasWildcardAllowFrom(accountConfig.allowFrom)) {
issues.push({
path: `${accountPath}.allowFrom`,
message: formatRawChannelConfigIssueMessage(
`${accountPath}.dmPolicy="open" requires ${accountPath}.allowFrom to include "*"`,
),
});
}
}
}
return issues;
}
function collectBundledChannelConfigDependencyIssues(
channelId: string,
value: unknown,
): ConfigValidationIssue[] {
if (channelId === "mattermost") {
return collectMattermostOpenDmAllowFromIssues(value, "channels.mattermost");
}
return [];
}
function collectRawBundledChannelConfigIssues(config: OpenClawConfig): ConfigValidationIssue[] {
if (!config.channels || !isRecord(config.channels)) {
return [];
@@ -320,6 +370,9 @@ function collectRawBundledChannelConfigIssues(config: OpenClawConfig): ConfigVal
if (!Object.hasOwn(config.channels, channelId)) {
continue;
}
issues.push(
...collectBundledChannelConfigDependencyIssues(channelId, config.channels[channelId]),
);
const result = validateJsonSchemaValue({
schema: schema as Record<string, unknown>,
cacheKey: `raw-channel:${channelId}`,
@@ -1569,6 +1622,7 @@ function validateConfigObjectWithPluginsBase(
}
continue;
}
issues.push(...collectBundledChannelConfigDependencyIssues(trimmed, result.value));
replaceChannelConfig(trimmed, result.value);
}
}