mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-13 16:30:36 +00:00
fix(slack): scope attachment extraction to forwarded shares
This commit is contained in:
@@ -17,6 +17,7 @@ Docs: https://docs.openclaw.ai
|
|||||||
|
|
||||||
### Fixes
|
### 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.
|
- 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.
|
- 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.
|
- 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.
|
||||||
|
|||||||
@@ -1,7 +1,12 @@
|
|||||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||||
import * as ssrf from "../../infra/net/ssrf.js";
|
import * as ssrf from "../../infra/net/ssrf.js";
|
||||||
import * as mediaStore from "../../media/store.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
|
// Store original fetch
|
||||||
const originalFetch = globalThis.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", () => {
|
describe("resolveSlackThreadHistory", () => {
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
vi.restoreAllMocks();
|
vi.restoreAllMocks();
|
||||||
|
|||||||
@@ -1,9 +1,9 @@
|
|||||||
import type { WebClient as SlackWebClient } from "@slack/web-api";
|
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 { 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 { fetchRemoteMedia } from "../../media/fetch.js";
|
||||||
import { saveMediaBuffer } from "../../media/store.js";
|
import { saveMediaBuffer } from "../../media/store.js";
|
||||||
import type { SlackAttachment, SlackFile } from "../types.js";
|
|
||||||
|
|
||||||
function isSlackHostname(hostname: string): boolean {
|
function isSlackHostname(hostname: string): boolean {
|
||||||
const normalized = normalizeHostname(hostname);
|
const normalized = normalizeHostname(hostname);
|
||||||
@@ -133,6 +133,28 @@ export type SlackMediaResult = {
|
|||||||
|
|
||||||
const MAX_SLACK_MEDIA_FILES = 8;
|
const MAX_SLACK_MEDIA_FILES = 8;
|
||||||
const MAX_SLACK_MEDIA_CONCURRENCY = 3;
|
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<T, R>(
|
async function mapLimit<T, R>(
|
||||||
items: T[],
|
items: T[],
|
||||||
@@ -230,10 +252,17 @@ export async function resolveSlackAttachmentContent(params: {
|
|||||||
return null;
|
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 textBlocks: string[] = [];
|
||||||
const allMedia: SlackMediaResult[] = [];
|
const allMedia: SlackMediaResult[] = [];
|
||||||
|
|
||||||
for (const att of attachments) {
|
for (const att of forwardedAttachments) {
|
||||||
const text = att.text?.trim() || att.fallback?.trim();
|
const text = att.text?.trim() || att.fallback?.trim();
|
||||||
if (text) {
|
if (text) {
|
||||||
const author = att.author_name;
|
const author = att.author_name;
|
||||||
@@ -241,7 +270,7 @@ export async function resolveSlackAttachmentContent(params: {
|
|||||||
textBlocks.push(`${heading}\n${text}`);
|
textBlocks.push(`${heading}\n${text}`);
|
||||||
}
|
}
|
||||||
|
|
||||||
const imageUrl = att.image_url;
|
const imageUrl = resolveForwardedAttachmentImageUrl(att);
|
||||||
if (imageUrl) {
|
if (imageUrl) {
|
||||||
try {
|
try {
|
||||||
const fetched = await fetchRemoteMedia({
|
const fetched = await fetchRemoteMedia({
|
||||||
|
|||||||
@@ -1,16 +1,16 @@
|
|||||||
|
import type { App } from "@slack/bolt";
|
||||||
import fs from "node:fs";
|
import fs from "node:fs";
|
||||||
import os from "node:os";
|
import os from "node:os";
|
||||||
import path from "node:path";
|
import path from "node:path";
|
||||||
import type { App } from "@slack/bolt";
|
|
||||||
import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
|
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 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 { RuntimeEnv } from "../../../runtime.js";
|
||||||
import type { ResolvedSlackAccount } from "../../accounts.js";
|
import type { ResolvedSlackAccount } from "../../accounts.js";
|
||||||
import type { SlackMessageEvent } from "../../types.js";
|
import type { SlackMessageEvent } from "../../types.js";
|
||||||
import type { SlackMonitorContext } from "../context.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 { createSlackMonitorContext } from "../context.js";
|
||||||
import { prepareSlackMessage } from "./prepare.js";
|
import { prepareSlackMessage } from "./prepare.js";
|
||||||
|
|
||||||
@@ -184,6 +184,30 @@ describe("slack prepareSlackMessage inbound contract", () => {
|
|||||||
expectInboundContextContract(prepared!.ctxPayload as any);
|
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 () => {
|
it("keeps channel metadata out of GroupSystemPrompt", async () => {
|
||||||
const slackCtx = createInboundSlackCtx({
|
const slackCtx = createInboundSlackCtx({
|
||||||
cfg: {
|
cfg: {
|
||||||
|
|||||||
Reference in New Issue
Block a user