diff --git a/src/agents/tools/slack-actions.test.ts b/src/agents/tools/slack-actions.test.ts index f4ba99488df..8a57602f58e 100644 --- a/src/agents/tools/slack-actions.test.ts +++ b/src/agents/tools/slack-actions.test.ts @@ -216,6 +216,33 @@ describe("handleSlackAction", () => { ); }); + it("passes download scope (channel/thread) to downloadSlackFile", async () => { + downloadSlackFile.mockResolvedValueOnce(null); + + const result = await handleSlackAction( + { + action: "downloadFile", + fileId: "F123", + to: "channel:C1", + replyTo: "123.456", + }, + slackConfig(), + ); + + expect(downloadSlackFile).toHaveBeenCalledWith( + "F123", + expect.objectContaining({ + channelId: "C1", + threadId: "123.456", + }), + ); + expect(result).toEqual( + expect.objectContaining({ + details: expect.objectContaining({ ok: false }), + }), + ); + }); + it.each([ { name: "JSON blocks", diff --git a/src/agents/tools/slack-actions.ts b/src/agents/tools/slack-actions.ts index a56eb2b3686..20a491c350d 100644 --- a/src/agents/tools/slack-actions.ts +++ b/src/agents/tools/slack-actions.ts @@ -290,12 +290,17 @@ export async function handleSlackAction( } case "downloadFile": { const fileId = readStringParam(params, "fileId", { required: true }); + const channelTarget = readStringParam(params, "channelId") ?? readStringParam(params, "to"); + const channelId = channelTarget ? resolveSlackChannelId(channelTarget) : undefined; + const threadId = readStringParam(params, "threadId") ?? readStringParam(params, "replyTo"); const maxBytes = account.config?.mediaMaxMb ? account.config.mediaMaxMb * 1024 * 1024 : 20 * 1024 * 1024; const downloaded = await downloadSlackFile(fileId, { ...readOpts, maxBytes, + channelId, + threadId: threadId ?? undefined, }); if (!downloaded) { return jsonResult({ diff --git a/src/plugin-sdk/slack-message-actions.test.ts b/src/plugin-sdk/slack-message-actions.test.ts index 14f584f9ca2..109b825fab9 100644 --- a/src/plugin-sdk/slack-message-actions.test.ts +++ b/src/plugin-sdk/slack-message-actions.test.ts @@ -16,6 +16,7 @@ describe("handleSlackMessageAction", () => { params: { channelId: "C1", fileId: "F123", + threadId: "111.222", }, } as never, invoke: invoke as never, @@ -25,6 +26,39 @@ describe("handleSlackMessageAction", () => { expect.objectContaining({ action: "downloadFile", fileId: "F123", + channelId: "C1", + threadId: "111.222", + }), + expect.any(Object), + ); + }); + + it("maps download-file target aliases to scope fields", async () => { + const invoke = vi.fn(async (action: Record) => ({ + ok: true, + content: action, + })); + + await handleSlackMessageAction({ + providerId: "slack", + ctx: { + action: "download-file", + cfg: {}, + params: { + to: "channel:C2", + fileId: "F999", + replyTo: "333.444", + }, + } as never, + invoke: invoke as never, + }); + + expect(invoke).toHaveBeenCalledWith( + expect.objectContaining({ + action: "downloadFile", + fileId: "F999", + channelId: "channel:C2", + threadId: "333.444", }), expect.any(Object), ); diff --git a/src/plugin-sdk/slack-message-actions.ts b/src/plugin-sdk/slack-message-actions.ts index 16abb4f9b4f..d9e0fa333a5 100644 --- a/src/plugin-sdk/slack-message-actions.ts +++ b/src/plugin-sdk/slack-message-actions.ts @@ -178,7 +178,20 @@ export async function handleSlackMessageAction(params: { if (action === "download-file") { const fileId = readStringParam(actionParams, "fileId", { required: true }); - return await invoke({ action: "downloadFile", fileId, accountId }, cfg); + const channelId = + readStringParam(actionParams, "channelId") ?? readStringParam(actionParams, "to"); + const threadId = + readStringParam(actionParams, "threadId") ?? readStringParam(actionParams, "replyTo"); + return await invoke( + { + action: "downloadFile", + fileId, + channelId: channelId ?? undefined, + threadId: threadId ?? undefined, + accountId, + }, + cfg, + ); } throw new Error(`Action ${action} is not supported for provider ${providerId}.`); diff --git a/src/slack/actions.download-file.test.ts b/src/slack/actions.download-file.test.ts index 3bc7ae76cab..b7afe84b149 100644 --- a/src/slack/actions.download-file.test.ts +++ b/src/slack/actions.download-file.test.ts @@ -1,5 +1,5 @@ import type { WebClient } from "@slack/web-api"; -import { describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; const resolveSlackMedia = vi.fn(); @@ -22,6 +22,10 @@ function createClient() { } describe("downloadSlackFile", () => { + beforeEach(() => { + resolveSlackMedia.mockReset(); + }); + it("returns null when files.info has no private download URL", async () => { const client = createClient(); client.files.info.mockResolvedValueOnce({ @@ -85,4 +89,89 @@ describe("downloadSlackFile", () => { placeholder: "[Slack file: image.png]", }); }); + + it("returns null when channel scope definitely mismatches file shares", async () => { + const client = createClient(); + client.files.info.mockResolvedValueOnce({ + file: { + id: "F123", + name: "image.png", + mimetype: "image/png", + url_private_download: "https://files.slack.com/files-pri/T1-F123/image.png", + channels: ["C999"], + }, + }); + + const result = await downloadSlackFile("F123", { + client, + token: "xoxb-test", + maxBytes: 1024, + channelId: "C123", + }); + + expect(result).toBeNull(); + expect(resolveSlackMedia).not.toHaveBeenCalled(); + }); + + it("returns null when thread scope definitely mismatches file share thread", async () => { + const client = createClient(); + client.files.info.mockResolvedValueOnce({ + file: { + id: "F123", + name: "image.png", + mimetype: "image/png", + url_private_download: "https://files.slack.com/files-pri/T1-F123/image.png", + shares: { + private: { + C123: [{ ts: "111.111", thread_ts: "111.111" }], + }, + }, + }, + }); + + const result = await downloadSlackFile("F123", { + client, + token: "xoxb-test", + maxBytes: 1024, + channelId: "C123", + threadId: "222.222", + }); + + expect(result).toBeNull(); + expect(resolveSlackMedia).not.toHaveBeenCalled(); + }); + + it("keeps legacy behavior when file metadata does not expose channel/thread shares", async () => { + const client = createClient(); + client.files.info.mockResolvedValueOnce({ + file: { + id: "F123", + name: "image.png", + mimetype: "image/png", + url_private_download: "https://files.slack.com/files-pri/T1-F123/image.png", + }, + }); + resolveSlackMedia.mockResolvedValueOnce([ + { + path: "/tmp/image.png", + contentType: "image/png", + placeholder: "[Slack file: image.png]", + }, + ]); + + const result = await downloadSlackFile("F123", { + client, + token: "xoxb-test", + maxBytes: 1024, + channelId: "C123", + threadId: "222.222", + }); + + expect(result).toEqual({ + path: "/tmp/image.png", + contentType: "image/png", + placeholder: "[Slack file: image.png]", + }); + expect(resolveSlackMedia).toHaveBeenCalledTimes(1); + }); }); diff --git a/src/slack/actions.ts b/src/slack/actions.ts index 3feedcc5314..d2e57959b0e 100644 --- a/src/slack/actions.ts +++ b/src/slack/actions.ts @@ -280,6 +280,129 @@ export async function listSlackPins( return (result.items ?? []) as SlackPin[]; } +type SlackFileInfoSummary = { + id?: string; + name?: string; + mimetype?: string; + url_private?: string; + url_private_download?: string; + channels?: unknown; + groups?: unknown; + ims?: unknown; + shares?: unknown; +}; + +type SlackFileThreadShare = { + channelId: string; + ts?: string; + threadTs?: string; +}; + +function normalizeSlackScopeValue(value: string | undefined): string | undefined { + const trimmed = value?.trim(); + return trimmed ? trimmed : undefined; +} + +function collectSlackDirectShareChannelIds(file: SlackFileInfoSummary): Set { + const ids = new Set(); + for (const group of [file.channels, file.groups, file.ims]) { + if (!Array.isArray(group)) { + continue; + } + for (const entry of group) { + if (typeof entry !== "string") { + continue; + } + const normalized = normalizeSlackScopeValue(entry); + if (normalized) { + ids.add(normalized); + } + } + } + return ids; +} + +function collectSlackShareMaps(file: SlackFileInfoSummary): Array> { + if (!file.shares || typeof file.shares !== "object" || Array.isArray(file.shares)) { + return []; + } + const shares = file.shares as Record; + return [shares.public, shares.private].filter( + (value): value is Record => + Boolean(value) && typeof value === "object" && !Array.isArray(value), + ); +} + +function collectSlackSharedChannelIds(file: SlackFileInfoSummary): Set { + const ids = new Set(); + for (const shareMap of collectSlackShareMaps(file)) { + for (const channelId of Object.keys(shareMap)) { + const normalized = normalizeSlackScopeValue(channelId); + if (normalized) { + ids.add(normalized); + } + } + } + return ids; +} + +function collectSlackThreadShares( + file: SlackFileInfoSummary, + channelId: string, +): SlackFileThreadShare[] { + const matches: SlackFileThreadShare[] = []; + for (const shareMap of collectSlackShareMaps(file)) { + const rawEntries = shareMap[channelId]; + if (!Array.isArray(rawEntries)) { + continue; + } + for (const rawEntry of rawEntries) { + if (!rawEntry || typeof rawEntry !== "object" || Array.isArray(rawEntry)) { + continue; + } + const entry = rawEntry as Record; + const ts = typeof entry.ts === "string" ? normalizeSlackScopeValue(entry.ts) : undefined; + const threadTs = + typeof entry.thread_ts === "string" ? normalizeSlackScopeValue(entry.thread_ts) : undefined; + matches.push({ channelId, ts, threadTs }); + } + } + return matches; +} + +function hasSlackScopeMismatch(params: { + file: SlackFileInfoSummary; + channelId?: string; + threadId?: string; +}): boolean { + const channelId = normalizeSlackScopeValue(params.channelId); + if (!channelId) { + return false; + } + const threadId = normalizeSlackScopeValue(params.threadId); + + const directIds = collectSlackDirectShareChannelIds(params.file); + const sharedIds = collectSlackSharedChannelIds(params.file); + const hasChannelEvidence = directIds.size > 0 || sharedIds.size > 0; + const inChannel = directIds.has(channelId) || sharedIds.has(channelId); + if (hasChannelEvidence && !inChannel) { + return true; + } + + if (!threadId) { + return false; + } + const threadShares = collectSlackThreadShares(params.file, channelId); + if (threadShares.length === 0) { + return false; + } + const threadEvidence = threadShares.filter((entry) => entry.threadTs || entry.ts); + if (threadEvidence.length === 0) { + return false; + } + return !threadEvidence.some((entry) => entry.threadTs === threadId || entry.ts === threadId); +} + /** * Downloads a Slack file by ID and saves it to the local media store. * Fetches a fresh download URL via files.info to avoid using stale private URLs. @@ -287,26 +410,21 @@ export async function listSlackPins( */ export async function downloadSlackFile( fileId: string, - opts: SlackActionClientOpts & { maxBytes: number }, + opts: SlackActionClientOpts & { maxBytes: number; channelId?: string; threadId?: string }, ): Promise { const token = resolveToken(opts.token, opts.accountId); const client = await getClient(opts); // Fetch fresh file metadata (includes a current url_private_download). const info = await client.files.info({ file: fileId }); - const file = info.file as - | { - id?: string; - name?: string; - mimetype?: string; - url_private?: string; - url_private_download?: string; - } - | undefined; + const file = info.file as SlackFileInfoSummary | undefined; if (!file?.url_private_download && !file?.url_private) { return null; } + if (hasSlackScopeMismatch({ file, channelId: opts.channelId, threadId: opts.threadId })) { + return null; + } const results = await resolveSlackMedia({ files: [