diff --git a/extensions/mattermost/src/mattermost/monitor-resources.test.ts b/extensions/mattermost/src/mattermost/monitor-resources.test.ts index dcc0034ad59a..d680e0c5e6ec 100644 --- a/extensions/mattermost/src/mattermost/monitor-resources.test.ts +++ b/extensions/mattermost/src/mattermost/monitor-resources.test.ts @@ -51,11 +51,42 @@ describe("mattermost monitor resources", () => { body: "quarterly files", nativeMedia: [{}, {}], materializedMedia: [ - { path: "/tmp/q1.pdf", contentType: "application/pdf" }, - { kind: "audio" }, + { path: "/tmp/q1.pdf", contentType: "application/pdf", fileName: "available.pdf" }, + { kind: "audio", fileName: "quarterly recording.mp3" }, ], }), - ).toBe("quarterly files\n\n[mattermost attachment unavailable]"); + ).toBe('quarterly files\n\n[mattermost attachment unavailable] "quarterly recording.mp3"'); + }); + + it.each([ + { + fileName: '../../private/report]\u0000\n".pdf', + expected: '[mattermost attachment unavailable] "report].pdf"', + }, + { fileName: " ", expected: "[mattermost attachment unavailable]" }, + ])("safely formats unavailable attachment names: $fileName", ({ fileName, expected }) => { + expect( + formatMattermostInboundMediaText({ + body: "", + nativeMedia: [{}], + materializedMedia: [{ kind: "document", fileName }], + }), + ).toBe(expected); + }); + + it("bounds multiple unavailable attachment names without losing their count", () => { + const materializedMedia = Array.from({ length: 4 }, (_, index) => ({ + kind: "document" as const, + fileName: `${index}-${"a".repeat(250)}.pdf`, + })); + const result = formatMattermostInboundMediaText({ + body: "", + nativeMedia: materializedMedia.map(() => ({})), + materializedMedia, + }); + + expect(result).toContain("[mattermost 4 attachments unavailable]"); + expect(result.length).toBeLessThanOrEqual(560); }); it("keeps successfully materialized media-only text empty", () => { @@ -186,7 +217,11 @@ describe("mattermost monitor resources", () => { await expect(resources.resolveMattermostMedia(["file-image", "file-audio"])).resolves.toEqual([ { path: "/tmp/file.png", contentType: "image/png", kind: "image" }, - { contentType: "audio/mpeg", kind: "audio" }, + { + contentType: "audio/mpeg", + fileName: "private-unavailable-recording.mp3", + kind: "audio", + }, ]); expect(request).toHaveBeenCalledTimes(1); }); @@ -195,6 +230,8 @@ describe("mattermost monitor resources", () => { const saveRemoteMedia = vi.fn().mockRejectedValue(new Error("download failed")); const request = vi.fn(async (requestPath: string) => ({ mime_type: requestPath.includes("video") ? "video/mp4" : "application/pdf", + // Blank server-side names must be omitted, not forwarded as empty strings. + name: requestPath.includes("video") ? " " : undefined, })); const resources = createMattermostMonitorResources({ accountId: "default", diff --git a/extensions/mattermost/src/mattermost/monitor-resources.ts b/extensions/mattermost/src/mattermost/monitor-resources.ts index e628291fd981..e2ca0f365fa0 100644 --- a/extensions/mattermost/src/mattermost/monitor-resources.ts +++ b/extensions/mattermost/src/mattermost/monitor-resources.ts @@ -15,7 +15,9 @@ import { asDateTimestampMs, resolveExpiresAtMsFromDurationMs, } from "openclaw/plugin-sdk/number-runtime"; +import { sanitizeUntrustedFileName } from "openclaw/plugin-sdk/security-runtime"; import { normalizeStringEntries } from "openclaw/plugin-sdk/string-coerce-runtime"; +import { truncateUtf16Safe } from "openclaw/plugin-sdk/text-utility-runtime"; import { buildMattermostApiUrl, fetchMattermostChannel, @@ -49,7 +51,7 @@ export function formatMattermostPendingMediaText(params: { export function formatMattermostInboundMediaText(params: { body: string; nativeMedia: readonly MediaPlaceholderTextFact[]; - materializedMedia: readonly MediaPlaceholderTextFact[]; + materializedMedia: readonly ChannelInboundMediaInput[]; }): string { const materializedCount = params.materializedMedia.filter( (media) => Boolean(media.path) || Boolean(media.url), @@ -58,9 +60,17 @@ export function formatMattermostInboundMediaText(params: { if (unavailableCount === 0) { return params.body; } + const unavailableFileNames = params.materializedMedia + .filter((media) => !media.path && !media.url && media.fileName) + .map((media) => sanitizeUntrustedFileName(media.fileName ?? "", "")) + .filter(Boolean) + .join(", "); + const fileNameNotice = unavailableFileNames + ? ` ${JSON.stringify(truncateUtf16Safe(unavailableFileNames, 512))}` + : ""; return formatInboundMediaUnavailableText({ body: params.body, - notice: `[mattermost ${unavailableCount > 1 ? `${unavailableCount} attachments` : "attachment"} unavailable]`, + notice: `[mattermost ${unavailableCount > 1 ? `${unavailableCount} attachments` : "attachment"} unavailable]${fileNameNotice}`, }); } @@ -179,17 +189,19 @@ export function createMattermostMonitorResources(params: { }); } catch (err) { logger.debug?.(`mattermost: failed to download file ${fileId}: ${String(err)}`); - let contentType: string | undefined; + let info: { mime_type?: string | null; name?: string | null } | undefined; try { - const info = await client.request<{ mime_type?: string | null }>(`/files/${fileId}/info`); - contentType = info.mime_type?.trim() || undefined; + info = await client.request(`/files/${fileId}/info`); } catch (infoErr) { logger.debug?.( `mattermost: failed to resolve metadata for file ${fileId}: ${String(infoErr)}`, ); } + const contentType = info?.mime_type?.trim() || undefined; + const fileName = info?.name?.trim(); out.push({ contentType, + ...(fileName ? { fileName } : {}), kind: mediaKindFromMime(contentType) ?? "unknown", }); } diff --git a/extensions/mattermost/src/mattermost/monitor.inbound-system-event.test.ts b/extensions/mattermost/src/mattermost/monitor.inbound-system-event.test.ts index 65686110db31..1e3d991c0fd8 100644 --- a/extensions/mattermost/src/mattermost/monitor.inbound-system-event.test.ts +++ b/extensions/mattermost/src/mattermost/monitor.inbound-system-event.test.ts @@ -815,61 +815,96 @@ describe("mattermost inbound user posts", () => { expect(statusSink).not.toHaveBeenCalledWith(expect.objectContaining({ lifecycle: "ready" })); }); - it("does not enqueue regular user posts as system events", async () => { - const socket = new FakeWebSocket(); - const abortController = new AbortController(); - mockState.abortController = abortController; + it.each([ + { + label: "plain text", + fileIds: [], + failedMedia: [], + expectedBody: "hello from mattermost", + }, + { + label: "an unavailable named attachment", + fileIds: ["file-1"], + failedMedia: [ + { + contentType: "application/pdf", + fileName: "quarterly report.pdf", + kind: "document", + }, + ], + expectedBody: + 'hello from mattermost\n\n[mattermost attachment unavailable] "quarterly report.pdf"', + }, + ])( + "does not enqueue regular posts with $label as system events", + async ({ fileIds, failedMedia, expectedBody }) => { + const socket = new FakeWebSocket(); + const abortController = new AbortController(); + mockState.abortController = abortController; + mockState.resolveMattermostMedia.mockResolvedValueOnce(failedMedia); - const monitor = monitorMattermostProvider({ - config: testConfig, - runtime: testRuntime(), - abortSignal: abortController.signal, - webSocketFactory: () => socket, - }); + const monitor = monitorMattermostProvider({ + config: testConfig, + runtime: testRuntime(), + abortSignal: abortController.signal, + webSocketFactory: () => socket, + }); - await vi.waitFor(() => { - expect(socket.openListenerCount).toBeGreaterThan(0); - }); - socket.emitOpen(); + await vi.waitFor(() => { + expect(socket.openListenerCount).toBeGreaterThan(0); + }); + socket.emitOpen(); - await socket.emitMessage({ - event: "posted", - data: { - channel_id: "chan-1", - channel_name: "town-square", - channel_display_name: "Town Square", - sender_name: "alice", - post: JSON.stringify({ - id: "post-inbound-system-event-regular", + await socket.emitMessage({ + event: "posted", + data: { + channel_id: "chan-1", + channel_name: "town-square", + channel_display_name: "Town Square", + sender_name: "alice", + post: JSON.stringify({ + id: "post-inbound-system-event-regular", + channel_id: "chan-1", + user_id: "user-1", + message: "hello from mattermost", + ...(fileIds.length > 0 ? { file_ids: fileIds } : {}), + create_at: 1_714_000_000_000, + }), + }, + broadcast: { channel_id: "chan-1", user_id: "user-1", - message: "hello from mattermost", - create_at: 1_714_000_000_000, - }), - }, - broadcast: { - channel_id: "chan-1", - user_id: "user-1", - }, - }); - socket.emitClose(1000); - await monitor; + }, + }); + socket.emitClose(1000); + await monitor; - expect(mockState.enqueueSystemEvent).not.toHaveBeenCalled(); - expect(mockState.dispatchInboundMessage).toHaveBeenCalledTimes(1); - expect(mockState.deliveryPlanObserver).toHaveBeenCalledExactlyOnceWith(true); - const ctx = mockState.dispatchInboundMessage.mock.calls.at(0)?.[0].ctx; - expect(ctx?.BodyForAgent).toBe("hello from mattermost"); - expect(ctx?.ConversationLabel).toBe("Town Square id:chan-1"); - expect(ctx?.MessageSid).toBe("post-inbound-system-event-regular"); - expect(ctx?.ConversationRouteContextObserved).toBe(true); - expect(ctx?.ConversationRoutePeerId).toBe("chan-1"); - expect(ctx?.GroupSpace).toBe("team-1"); - expect(ctx?.NativeChannelId).toBe("chan-1"); - expect(ctx?.InboundAccessAuthorized).toBe(true); - expect(ctx?.OriginatingChannel).toBe("mattermost"); - expect(ctx?.Provider).toBe("mattermost"); - }); + expect(mockState.enqueueSystemEvent).not.toHaveBeenCalled(); + expect(mockState.dispatchInboundMessage).toHaveBeenCalledTimes(1); + expect(mockState.deliveryPlanObserver).toHaveBeenCalledExactlyOnceWith(true); + const ctx = mockState.dispatchInboundMessage.mock.calls.at(0)?.[0].ctx; + expect(ctx?.BodyForAgent).toBe(expectedBody); + if (failedMedia.length > 0) { + expect(ctx?.media).toEqual([ + expect.objectContaining({ + contentType: "application/pdf", + fileName: "quarterly report.pdf", + }), + ]); + expect(ctx?.media?.[0]?.path).toBeUndefined(); + expect(ctx?.media?.[0]?.url).toBeUndefined(); + } + expect(ctx?.ConversationLabel).toBe("Town Square id:chan-1"); + expect(ctx?.MessageSid).toBe("post-inbound-system-event-regular"); + expect(ctx?.ConversationRouteContextObserved).toBe(true); + expect(ctx?.ConversationRoutePeerId).toBe("chan-1"); + expect(ctx?.GroupSpace).toBe("team-1"); + expect(ctx?.NativeChannelId).toBe("chan-1"); + expect(ctx?.InboundAccessAuthorized).toBe(true); + expect(ctx?.OriginatingChannel).toBe("mattermost"); + expect(ctx?.Provider).toBe("mattermost"); + }, + ); it("formats current and pending-history timestamps in the configured user timezone", async () => { const socket = new FakeWebSocket();