diff --git a/extensions/mattermost/src/channel.ts b/extensions/mattermost/src/channel.ts index 52f1de4242ca..922063252d2a 100644 --- a/extensions/mattermost/src/channel.ts +++ b/extensions/mattermost/src/channel.ts @@ -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 = create lastConnectedAt: null, lastDisconnect: null, }), + collectStatusIssues: collectMattermostStatusIssues, buildChannelSummary: ({ snapshot }) => buildPassiveProbedChannelStatusSummary(snapshot, { botTokenSource: snapshot.botTokenSource ?? "none", @@ -858,6 +860,8 @@ export const mattermostPlugin: ChannelPlugin = 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, diff --git a/extensions/mattermost/src/config-schema.test.ts b/extensions/mattermost/src/config-schema.test.ts index fc4f4a648c36..30512cf9ef19 100644 --- a/extensions/mattermost/src/config-schema.test.ts +++ b/extensions/mattermost/src/config-schema.test.ts @@ -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: { diff --git a/extensions/mattermost/src/mattermost/monitor-auth.test.ts b/extensions/mattermost/src/mattermost/monitor-auth.test.ts index 3f685ffc936a..4cce0e3de643 100644 --- a/extensions/mattermost/src/mattermost/monitor-auth.test.ts +++ b/extensions/mattermost/src/mattermost/monitor-auth.test.ts @@ -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 }); diff --git a/extensions/mattermost/src/mattermost/monitor-auth.ts b/extensions/mattermost/src/mattermost/monitor-auth.ts index f642b91190ce..35af70771c51 100644 --- a/extensions/mattermost/src/mattermost/monitor-auth.ts +++ b/extensions/mattermost/src/mattermost/monitor-auth.ts @@ -61,6 +61,19 @@ export function normalizeMattermostAllowList(entries: Array): 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; diff --git a/extensions/mattermost/src/mattermost/monitor.ts b/extensions/mattermost/src/mattermost/monitor.ts index 28f25852449b..7fe93a4264a9 100644 --- a/extensions/mattermost/src/mattermost/monitor.ts +++ b/extensions/mattermost/src/mattermost/monitor.ts @@ -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") { diff --git a/extensions/mattermost/src/status-issues.test.ts b/extensions/mattermost/src/status-issues.test.ts new file mode 100644 index 000000000000..b2d915a7e38b --- /dev/null +++ b/extensions/mattermost/src/status-issues.test.ts @@ -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([]); + }); +}); diff --git a/extensions/mattermost/src/status-issues.ts b/extensions/mattermost/src/status-issues.ts new file mode 100644 index 000000000000..4a219765770a --- /dev/null +++ b/extensions/mattermost/src/status-issues.ts @@ -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; +} diff --git a/src/config/validation.channel-metadata.test.ts b/src/config/validation.channel-metadata.test.ts index 9b87b78026d5..58b201d23ee5 100644 --- a/src/config/validation.channel-metadata.test.ts +++ b/src/config/validation.channel-metadata.test.ts @@ -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", () => { diff --git a/src/config/validation.ts b/src/config/validation.ts index 2fdcc6f7df39..b27b2179a8f0 100644 --- a/src/config/validation.ts +++ b/src/config/validation.ts @@ -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, cacheKey: `raw-channel:${channelId}`, @@ -1569,6 +1622,7 @@ function validateConfigObjectWithPluginsBase( } continue; } + issues.push(...collectBundledChannelConfigDependencyIssues(trimmed, result.value)); replaceChannelConfig(trimmed, result.value); } }