Auto-merge upstream openclaw/openclaw

Author: friuns2

extensions/slack/src/action-runtime.test.ts

@@ -729,7 +729,10 @@ describe("handleSlackAction", () => {
     expect(requireRecordArg(downloadSlackFile, "downloadSlackFile", 0, 1).maxBytes).toBe(
       20 * 1024 * 1024,
     );
-    expect(requireDetails(result).ok).toBe(false);
+    expect(requireDetails(result)).toMatchObject({
+      ok: false,
+      error: expect.stringMatching(/requested Slack channel or explicit thread/i),
+    });
   });
 
   it("fails closed for downloadFile when no channel target can be authorized", async () => {

extensions/slack/src/action-runtime.ts

@@ -925,7 +925,8 @@ export async function handleSlackAction(
         if (!downloaded) {
           return jsonResult({
             ok: false,
-            error: "File could not be downloaded (not found, too large, or inaccessible).",
+            error:
+              "File could not be downloaded. Confirm the fileId came from the requested Slack channel or explicit thread and that the file is accessible and within the size limit.",
           });
         }
         if (!isImageContentType(downloaded.contentType)) {

extensions/slack/src/actions.download-file.test.ts

@@ -3,7 +3,7 @@ import type { WebClient } from "@slack/web-api";
 import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts";
 import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
 
-const resolveSlackMedia = vi.fn();
+const resolveSlackMedia = vi.fn<typeof import("./monitor/media.js").resolveSlackMedia>();
 const createSlackLookupClientMock = vi.hoisted(() => vi.fn());
 
 vi.mock("./monitor/media.js", () => ({
@@ -35,6 +35,7 @@ function makeSlackFileInfo(overrides?: Record<string, unknown>) {
     name: "image.png",
     mimetype: "image/png",
     url_private_download: "https://files.slack.com/files-pri/T1-F123/image.png",
+    channels: ["C123"],
     ...overrides,
   };
 }
@@ -53,6 +54,14 @@ function expectNoMediaDownload(result: Awaited<ReturnType<typeof downloadSlackFi
   expect(resolveSlackMedia).not.toHaveBeenCalled();
 }
 
+function requireRefreshedFileAdmission() {
+  const admission = resolveSlackMedia.mock.calls[0]?.[0].isRefreshedFileAllowed;
+  if (!admission) {
+    throw new Error("Expected refreshed Slack file admission");
+  }
+  return admission;
+}
+
 function expectResolveSlackMediaCalledWithDefaults(client: ReturnType<typeof createClient>) {
   expect(resolveSlackMedia).toHaveBeenCalledWith({
     files: [
@@ -65,6 +74,7 @@ function expectResolveSlackMediaCalledWithDefaults(client: ReturnType<typeof cre
       },
     ],
     client,
+    isRefreshedFileAllowed: expect.any(Function),
     token: "xoxb-test",
     maxBytes: 1024,
   });
@@ -100,6 +110,7 @@ describe("downloadSlackFile", () => {
       client,
       token: "xoxb-test",
       maxBytes: 1024,
+      channelId: "C123",
     });
 
     expect(result).toBeNull();
@@ -114,6 +125,7 @@ describe("downloadSlackFile", () => {
       client,
       token: "xoxb-test",
       maxBytes: 1024,
+      channelId: "C123",
     });
 
     expect(client.files.info).toHaveBeenCalledWith({ file: "F123" });
@@ -134,6 +146,7 @@ describe("downloadSlackFile", () => {
       client,
       token: "xoxb-test",
       maxBytes: 1024,
+      channelId: "C123",
     });
 
     expect(resolveSlackMedia).toHaveBeenCalledWith(expect.objectContaining({ client }));
@@ -160,6 +173,7 @@ describe("downloadSlackFile", () => {
       client,
       token: "xoxb-test",
       maxBytes: 1024,
+      channelId: "C123",
     });
 
     expect(resolveSlackMedia).toHaveBeenCalledWith({
@@ -173,6 +187,7 @@ describe("downloadSlackFile", () => {
         },
       ],
       client,
+      isRefreshedFileAllowed: expect.any(Function),
       token: "xoxb-test",
       maxBytes: 1024,
     });
@@ -201,6 +216,55 @@ describe("downloadSlackFile", () => {
     expectNoMediaDownload(result);
   });
 
+  it.each([
+    { name: "public channel metadata", file: { channels: ["C123"] } },
+    { name: "private channel metadata", file: { groups: ["C123"] } },
+    { name: "DM metadata", file: { ims: ["C123"] } },
+    {
+      name: "share metadata",
+      file: {
+        channels: undefined,
+        shares: { private: { C123: [{ ts: "111.111" }] } },
+      },
+    },
+  ])("downloads when $name proves the requested channel", async ({ file }) => {
+    const client = createClient();
+    client.files.info.mockResolvedValueOnce({
+      file: makeSlackFileInfo(file),
+    });
+    resolveSlackMedia.mockResolvedValueOnce([makeResolvedSlackMedia()]);
+
+    const result = await downloadSlackFile("F123", {
+      client,
+      token: "xoxb-test",
+      maxBytes: 1024,
+      channelId: "C123",
+    });
+
+    expect(result).toEqual(makeResolvedSlackMedia());
+  });
+
+  it("accepts positive channel proof even when Slack reports additional shares", async () => {
+    const client = createClient();
+    client.files.info.mockResolvedValueOnce({
+      file: makeSlackFileInfo({
+        channels: ["C123"],
+        has_more_shares: true,
+        skipped_shares: true,
+      }),
+    });
+    resolveSlackMedia.mockResolvedValueOnce([makeResolvedSlackMedia()]);
+
+    const result = await downloadSlackFile("F123", {
+      client,
+      token: "xoxb-test",
+      maxBytes: 1024,
+      channelId: "C123",
+    });
+
+    expect(result).toEqual(makeResolvedSlackMedia());
+  });
+
   it("returns null when thread scope definitely mismatches file share thread", async () => {
     const client = createClient();
     client.files.info.mockResolvedValueOnce({
@@ -224,9 +288,11 @@ describe("downloadSlackFile", () => {
     expectNoMediaDownload(result);
   });
 
-  it("keeps legacy behavior when file metadata does not expose channel/thread shares", async () => {
+  it("returns null when file metadata proves the channel but not the requested thread", async () => {
     const client = createClient();
-    mockSuccessfulMediaDownload(client);
+    client.files.info.mockResolvedValueOnce({
+      file: makeSlackFileInfo({ channels: ["C123"] }),
+    });
 
     const result = await downloadSlackFile("F123", {
       client,
@@ -236,9 +302,105 @@ describe("downloadSlackFile", () => {
       threadId: "222.222",
     });
 
+    expectNoMediaDownload(result);
+  });
+
+  it.each([
+    { name: "share message timestamp", share: { ts: "111.111" } },
+    { name: "thread timestamp", share: { ts: "222.222", thread_ts: "111.111" } },
+  ])("downloads when $name proves the requested thread", async ({ share }) => {
+    const client = createClient();
+    client.files.info.mockResolvedValueOnce({
+      file: makeSlackFileInfo({
+        shares: { private: { C123: [share] } },
+      }),
+    });
+    resolveSlackMedia.mockResolvedValueOnce([makeResolvedSlackMedia()]);
+
+    const result = await downloadSlackFile("F123", {
+      client,
+      token: "xoxb-test",
+      maxBytes: 1024,
+      channelId: "C123",
+      threadId: "111.111",
+    });
+
     expect(result).toEqual(makeResolvedSlackMedia());
-    expect(resolveSlackMedia).toHaveBeenCalledTimes(1);
-    expectResolveSlackMediaCalledWithDefaults(client);
+  });
+
+  it("reapplies the requested channel and thread scope to refreshed metadata", async () => {
+    const client = createClient();
+    client.files.info.mockResolvedValueOnce({
+      file: makeSlackFileInfo({
+        shares: { private: { C123: [{ ts: "111.111" }] } },
+      }),
+    });
+    resolveSlackMedia.mockResolvedValueOnce([makeResolvedSlackMedia()]);
+
+    await downloadSlackFile("F123", {
+      client,
+      token: "xoxb-test",
+      maxBytes: 1024,
+      channelId: "C123",
+      threadId: "111.111",
+    });
+
+    const isAllowed = requireRefreshedFileAdmission();
+    expect(
+      isAllowed(makeSlackFileInfo({ shares: { private: { C123: [{ ts: "111.111" }] } } })),
+    ).toBe(true);
+    expect(
+      isAllowed(makeSlackFileInfo({ shares: { private: { C999: [{ ts: "111.111" }] } } })),
+    ).toBe(false);
+    expect(
+      isAllowed(makeSlackFileInfo({ shares: { private: { C123: [{ ts: "222.222" }] } } })),
+    ).toBe(false);
+  });
+
+  it.each([
+    { name: "absent channel/share evidence", file: { channels: undefined } },
+    {
+      name: "malformed shares container",
+      file: { channels: undefined, shares: "invalid" },
+    },
+    {
+      name: "requested channel with a non-array share value",
+      file: { channels: undefined, shares: { private: { C123: {} } } },
+    },
+    {
+      name: "requested channel with an empty share array",
+      file: { channels: undefined, shares: { private: { C123: [] } } },
+    },
+    {
+      name: "requested channel with a share entry lacking timestamps",
+      file: { channels: undefined, shares: { private: { C123: [{}] } } },
+    },
+  ])("returns null for $name", async ({ file }) => {
+    const client = createClient();
+    client.files.info.mockResolvedValueOnce({ file: makeSlackFileInfo(file) });
+
+    const result = await downloadSlackFile("F123", {
+      client,
+      token: "xoxb-test",
+      maxBytes: 1024,
+      channelId: "C123",
+    });
+
+    expectNoMediaDownload(result);
+  });
+
+  it("returns null when the requested channel is empty after normalization", async () => {
+    const client = createClient();
+    mockSuccessfulMediaDownload(client);
+
+    const result = await downloadSlackFile("F123", {
+      client,
+      token: "xoxb-test",
+      maxBytes: 1024,
+      channelId: "   ",
+    });
+
+    expectNoMediaDownload(result);
   });
 
   it("resolves the bot token from cfg when no explicit token or client is provided", async () => {
@@ -265,6 +427,7 @@ describe("downloadSlackFile", () => {
       cfg,
       accountId: "default",
       maxBytes: 1024,
+      channelId: "C123",
     });
 
     expect(createSlackLookupClientMock).toHaveBeenCalledWith("xoxb-from-cfg", {
@@ -281,6 +444,7 @@ describe("downloadSlackFile", () => {
         },
       ],
       client,
+      isRefreshedFileAllowed: expect.any(Function),
       token: "xoxb-from-cfg",
       maxBytes: 1024,
     });