mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
fix(slack): route directory cursor pagination through the shared guard (#120445)
listSlackDirectoryPeersLive/listSlackDirectoryGroupsLive hand-rolled do/while loops with no repeated-cursor detection and no page bound, while every other users.list/conversations.list consumer goes through collectSlackCursorPages. A Slack API or proxy edge that keeps returning the same non-empty next_cursor made directory queries paginate forever, growing the member/channel arrays without bound. Both loops now go through collectSlackCursorPages, which throws on a repeated cursor and caps total pages.
This commit is contained in:
@@ -0,0 +1,67 @@
|
||||
// Slack tests cover directory live behavior.
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { listSlackDirectoryGroupsLive, listSlackDirectoryPeersLive } from "./directory-live.js";
|
||||
|
||||
const slackClientMocks = vi.hoisted(() => ({
|
||||
createSlackLookupClient: vi.fn(),
|
||||
usersList: vi.fn(),
|
||||
conversationsList: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("./client.js", () => ({
|
||||
createSlackLookupClient: slackClientMocks.createSlackLookupClient,
|
||||
}));
|
||||
|
||||
const params = { cfg: { channels: { slack: { botToken: "xoxb-test" } } } };
|
||||
|
||||
describe("slack directory live cursor pagination", () => {
|
||||
beforeEach(() => {
|
||||
slackClientMocks.usersList.mockReset();
|
||||
slackClientMocks.conversationsList.mockReset();
|
||||
slackClientMocks.createSlackLookupClient.mockReset().mockReturnValue({
|
||||
users: { list: slackClientMocks.usersList },
|
||||
conversations: { list: slackClientMocks.conversationsList },
|
||||
});
|
||||
});
|
||||
|
||||
it("lists peers across advancing cursors", async () => {
|
||||
slackClientMocks.usersList
|
||||
.mockResolvedValueOnce({
|
||||
members: [{ id: "U1", name: "one" }],
|
||||
response_metadata: { next_cursor: "cursor-1" },
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
members: [{ id: "U2", name: "two" }],
|
||||
response_metadata: { next_cursor: "" },
|
||||
});
|
||||
|
||||
const rows = await listSlackDirectoryPeersLive(params);
|
||||
|
||||
expect(rows.map((row) => row.id)).toEqual(["user:U1", "user:U2"]);
|
||||
expect(slackClientMocks.usersList).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("rejects a repeated users.list cursor instead of paginating forever", async () => {
|
||||
slackClientMocks.usersList.mockResolvedValue({
|
||||
members: [],
|
||||
response_metadata: { next_cursor: "cursor-loop" },
|
||||
});
|
||||
|
||||
await expect(listSlackDirectoryPeersLive(params)).rejects.toThrow(
|
||||
"Slack cursor pagination repeated a cursor",
|
||||
);
|
||||
expect(slackClientMocks.usersList).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("rejects a repeated conversations.list cursor instead of paginating forever", async () => {
|
||||
slackClientMocks.conversationsList.mockResolvedValue({
|
||||
channels: [],
|
||||
response_metadata: { next_cursor: "cursor-loop" },
|
||||
});
|
||||
|
||||
await expect(listSlackDirectoryGroupsLive(params)).rejects.toThrow(
|
||||
"Slack cursor pagination repeated a cursor",
|
||||
);
|
||||
expect(slackClientMocks.conversationsList).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
});
|
||||
@@ -11,6 +11,7 @@ import {
|
||||
} from "openclaw/plugin-sdk/string-coerce-runtime";
|
||||
import { resolveSlackAccount } from "./accounts.js";
|
||||
import { createSlackLookupClient } from "./client.js";
|
||||
import { collectSlackCursorPages } from "./cursor-pages.js";
|
||||
|
||||
type SlackUser = NonNullable<UsersListResponse["members"]>[number];
|
||||
type SlackChannel = NonNullable<ConversationsListResponse["channels"]>[number];
|
||||
@@ -95,20 +96,12 @@ export async function listSlackDirectoryPeersLive(
|
||||
return [];
|
||||
}
|
||||
const query = normalizeQuery(params.query);
|
||||
const members: SlackUser[] = [];
|
||||
let cursor: string | undefined;
|
||||
|
||||
do {
|
||||
const res = await client.users.list({
|
||||
limit: 200,
|
||||
cursor,
|
||||
});
|
||||
if (Array.isArray(res.members)) {
|
||||
members.push(...res.members);
|
||||
}
|
||||
const next = res.response_metadata?.next_cursor?.trim();
|
||||
cursor = next ? next : undefined;
|
||||
} while (cursor);
|
||||
// Route through the shared cursor guard: a repeated or endless next_cursor
|
||||
// (buggy proxy or Slack edge case) must fail instead of paginating forever.
|
||||
const members = await collectSlackCursorPages({
|
||||
fetchPage: (cursor) => client.users.list({ limit: 200, cursor }),
|
||||
collectPageItems: (res) => (Array.isArray(res.members) ? res.members : []),
|
||||
});
|
||||
|
||||
const filtered = members.filter((member) => {
|
||||
const name = member.profile?.display_name || member.profile?.real_name || member.real_name;
|
||||
@@ -141,22 +134,16 @@ export async function listSlackDirectoryGroupsLive(
|
||||
return [];
|
||||
}
|
||||
const query = normalizeQuery(params.query);
|
||||
const channels: SlackChannel[] = [];
|
||||
let cursor: string | undefined;
|
||||
|
||||
do {
|
||||
const res = await client.conversations.list({
|
||||
types: "public_channel,private_channel",
|
||||
exclude_archived: false,
|
||||
limit: 1000,
|
||||
cursor,
|
||||
});
|
||||
if (Array.isArray(res.channels)) {
|
||||
channels.push(...res.channels);
|
||||
}
|
||||
const next = res.response_metadata?.next_cursor?.trim();
|
||||
cursor = next ? next : undefined;
|
||||
} while (cursor);
|
||||
const channels = await collectSlackCursorPages({
|
||||
fetchPage: (cursor) =>
|
||||
client.conversations.list({
|
||||
types: "public_channel,private_channel",
|
||||
exclude_archived: false,
|
||||
limit: 1000,
|
||||
cursor,
|
||||
}),
|
||||
collectPageItems: (res) => (Array.isArray(res.channels) ? res.channels : []),
|
||||
});
|
||||
|
||||
const filtered = channels.filter((channel) => {
|
||||
const name = normalizeOptionalLowercaseString(channel.name);
|
||||
|
||||
Reference in New Issue
Block a user