diff --git a/CHANGELOG.md b/CHANGELOG.md index e08f1c463a9..34c0766077c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Slack: limit forwarded-attachment extraction to explicit shared-message attachments and skip non-Slack forwarded image URLs, preventing non-forward unfurls from polluting inbound agent context. Also adds regression tests for forwarded vs non-forward attachment handling. - Cron/Heartbeat: canonicalize session-scoped reminder `sessionKey` routing and preserve explicit flat `sessionKey` cron tool inputs, preventing enqueue/wake namespace drift for session-targeted reminders. (#18637) Thanks @vignesh07. - OpenClawKit/iOS ChatUI: accept canonical session-key completion events for local pending runs and preserve message IDs across history refreshes, preventing stuck "thinking" state and message flicker after gateway replies. (#18165) Thanks @mbelinky. - iOS/Onboarding: add QR-first onboarding wizard with setup-code deep link support, pairing/auth issue guidance, and device-pair QR generation improvements for Telegram/Web/TUI fallback flows. (#18162) Thanks @mbelinky and @Marvae. diff --git a/src/slack/monitor/media.test.ts b/src/slack/monitor/media.test.ts index 7303245bcca..f708f3819d9 100644 --- a/src/slack/monitor/media.test.ts +++ b/src/slack/monitor/media.test.ts @@ -1,7 +1,12 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import * as ssrf from "../../infra/net/ssrf.js"; import * as mediaStore from "../../media/store.js"; -import { fetchWithSlackAuth, resolveSlackMedia, resolveSlackThreadHistory } from "./media.js"; +import { + fetchWithSlackAuth, + resolveSlackAttachmentContent, + resolveSlackMedia, + resolveSlackThreadHistory, +} from "./media.js"; // Store original fetch const originalFetch = globalThis.fetch; @@ -422,6 +427,109 @@ describe("resolveSlackMedia", () => { }); }); +describe("resolveSlackAttachmentContent", () => { + beforeEach(() => { + mockFetch = vi.fn(); + globalThis.fetch = mockFetch as typeof fetch; + vi.spyOn(ssrf, "resolvePinnedHostnameWithPolicy").mockImplementation(async (hostname) => { + const normalized = hostname.trim().toLowerCase().replace(/\.$/, ""); + const addresses = ["93.184.216.34"]; + return { + hostname: normalized, + addresses, + lookup: ssrf.createPinnedLookup({ hostname: normalized, addresses }), + }; + }); + }); + + afterEach(() => { + globalThis.fetch = originalFetch; + vi.restoreAllMocks(); + }); + + it("ignores non-forwarded attachments", async () => { + const result = await resolveSlackAttachmentContent({ + attachments: [ + { + text: "unfurl text", + is_msg_unfurl: true, + image_url: "https://example.com/unfurl.jpg", + }, + ], + token: "xoxb-test-token", + maxBytes: 1024 * 1024, + }); + + expect(result).toBeNull(); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it("extracts text from forwarded shared attachments", async () => { + const result = await resolveSlackAttachmentContent({ + attachments: [ + { + is_share: true, + author_name: "Bob", + text: "Please review this", + }, + ], + token: "xoxb-test-token", + maxBytes: 1024 * 1024, + }); + + expect(result).toEqual({ + text: "[Forwarded message from Bob]\nPlease review this", + media: [], + }); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it("skips forwarded image URLs on non-Slack hosts", async () => { + const saveMediaBufferMock = vi.spyOn(mediaStore, "saveMediaBuffer"); + + const result = await resolveSlackAttachmentContent({ + attachments: [{ is_share: true, image_url: "https://example.com/forwarded.jpg" }], + token: "xoxb-test-token", + maxBytes: 1024 * 1024, + }); + + expect(result).toBeNull(); + expect(saveMediaBufferMock).not.toHaveBeenCalled(); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it("downloads Slack-hosted images from forwarded shared attachments", async () => { + vi.spyOn(mediaStore, "saveMediaBuffer").mockResolvedValue({ + path: "/tmp/forwarded.jpg", + contentType: "image/jpeg", + }); + + mockFetch.mockResolvedValueOnce( + new Response(Buffer.from("forwarded image"), { + status: 200, + headers: { "content-type": "image/jpeg" }, + }), + ); + + const result = await resolveSlackAttachmentContent({ + attachments: [{ is_share: true, image_url: "https://files.slack.com/forwarded.jpg" }], + token: "xoxb-test-token", + maxBytes: 1024 * 1024, + }); + + expect(result).toEqual({ + text: "", + media: [ + { + path: "/tmp/forwarded.jpg", + contentType: "image/jpeg", + placeholder: "[Forwarded image: forwarded.jpg]", + }, + ], + }); + }); +}); + describe("resolveSlackThreadHistory", () => { afterEach(() => { vi.restoreAllMocks(); diff --git a/src/slack/monitor/media.ts b/src/slack/monitor/media.ts index 0e7bdf1bf7b..47729b868c3 100644 --- a/src/slack/monitor/media.ts +++ b/src/slack/monitor/media.ts @@ -1,9 +1,9 @@ import type { WebClient as SlackWebClient } from "@slack/web-api"; -import { normalizeHostname } from "../../infra/net/hostname.js"; import type { FetchLike } from "../../media/fetch.js"; +import type { SlackAttachment, SlackFile } from "../types.js"; +import { normalizeHostname } from "../../infra/net/hostname.js"; import { fetchRemoteMedia } from "../../media/fetch.js"; import { saveMediaBuffer } from "../../media/store.js"; -import type { SlackAttachment, SlackFile } from "../types.js"; function isSlackHostname(hostname: string): boolean { const normalized = normalizeHostname(hostname); @@ -133,6 +133,28 @@ export type SlackMediaResult = { const MAX_SLACK_MEDIA_FILES = 8; const MAX_SLACK_MEDIA_CONCURRENCY = 3; +const MAX_SLACK_FORWARDED_ATTACHMENTS = 8; + +function isForwardedSlackAttachment(attachment: SlackAttachment): boolean { + // Narrow this parser to Slack's explicit "shared/forwarded" attachment payloads. + return attachment.is_share === true; +} + +function resolveForwardedAttachmentImageUrl(attachment: SlackAttachment): string | null { + const rawUrl = attachment.image_url?.trim(); + if (!rawUrl) { + return null; + } + try { + const parsed = new URL(rawUrl); + if (parsed.protocol !== "https:" || !isSlackHostname(parsed.hostname)) { + return null; + } + return parsed.toString(); + } catch { + return null; + } +} async function mapLimit( items: T[], @@ -230,10 +252,17 @@ export async function resolveSlackAttachmentContent(params: { return null; } + const forwardedAttachments = attachments + .filter((attachment) => isForwardedSlackAttachment(attachment)) + .slice(0, MAX_SLACK_FORWARDED_ATTACHMENTS); + if (forwardedAttachments.length === 0) { + return null; + } + const textBlocks: string[] = []; const allMedia: SlackMediaResult[] = []; - for (const att of attachments) { + for (const att of forwardedAttachments) { const text = att.text?.trim() || att.fallback?.trim(); if (text) { const author = att.author_name; @@ -241,7 +270,7 @@ export async function resolveSlackAttachmentContent(params: { textBlocks.push(`${heading}\n${text}`); } - const imageUrl = att.image_url; + const imageUrl = resolveForwardedAttachmentImageUrl(att); if (imageUrl) { try { const fetched = await fetchRemoteMedia({ diff --git a/src/slack/monitor/message-handler/prepare.test.ts b/src/slack/monitor/message-handler/prepare.test.ts index 2c68317223e..3f81c59ed83 100644 --- a/src/slack/monitor/message-handler/prepare.test.ts +++ b/src/slack/monitor/message-handler/prepare.test.ts @@ -1,16 +1,16 @@ +import type { App } from "@slack/bolt"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import type { App } from "@slack/bolt"; import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; -import { expectInboundContextContract } from "../../../../test/helpers/inbound-contract.js"; import type { OpenClawConfig } from "../../../config/config.js"; -import { resolveAgentRoute } from "../../../routing/resolve-route.js"; -import { resolveThreadSessionKeys } from "../../../routing/session-key.js"; import type { RuntimeEnv } from "../../../runtime.js"; import type { ResolvedSlackAccount } from "../../accounts.js"; import type { SlackMessageEvent } from "../../types.js"; import type { SlackMonitorContext } from "../context.js"; +import { expectInboundContextContract } from "../../../../test/helpers/inbound-contract.js"; +import { resolveAgentRoute } from "../../../routing/resolve-route.js"; +import { resolveThreadSessionKeys } from "../../../routing/session-key.js"; import { createSlackMonitorContext } from "../context.js"; import { prepareSlackMessage } from "./prepare.js"; @@ -184,6 +184,30 @@ describe("slack prepareSlackMessage inbound contract", () => { expectInboundContextContract(prepared!.ctxPayload as any); }); + it("includes forwarded shared attachment text in raw body", async () => { + const prepared = await prepareWithDefaultCtx( + createSlackMessage({ + text: "", + attachments: [{ is_share: true, author_name: "Bob", text: "Forwarded hello" }], + }), + ); + + expect(prepared).toBeTruthy(); + expect(prepared!.ctxPayload.RawBody).toContain("[Forwarded message from Bob]\nForwarded hello"); + }); + + it("ignores non-forward attachments when no direct text/files are present", async () => { + const prepared = await prepareWithDefaultCtx( + createSlackMessage({ + text: "", + files: [], + attachments: [{ is_msg_unfurl: true, text: "link unfurl text" }], + }), + ); + + expect(prepared).toBeNull(); + }); + it("keeps channel metadata out of GroupSystemPrompt", async () => { const slackCtx = createInboundSlackCtx({ cfg: {