fix(slack): scope download-file to channel and thread context

This commit is contained in:
Peter Steinberger
2026-03-02 02:23:12 +00:00
parent 17bae93680
commit 40fda40aa7
6 changed files with 298 additions and 12 deletions

View File

@@ -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",

View File

@@ -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({

View File

@@ -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<string, unknown>) => ({
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),
);

View File

@@ -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}.`);

View File

@@ -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);
});
});

View File

@@ -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<string> {
const ids = new Set<string>();
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<Record<string, unknown>> {
if (!file.shares || typeof file.shares !== "object" || Array.isArray(file.shares)) {
return [];
}
const shares = file.shares as Record<string, unknown>;
return [shares.public, shares.private].filter(
(value): value is Record<string, unknown> =>
Boolean(value) && typeof value === "object" && !Array.isArray(value),
);
}
function collectSlackSharedChannelIds(file: SlackFileInfoSummary): Set<string> {
const ids = new Set<string>();
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<string, unknown>;
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<SlackMediaResult | null> {
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: [