From d23bb857a74229af9a4c210fd8a26cf0a37e3b33 Mon Sep 17 00:00:00 2001 From: Dotta Date: Tue, 22 Sep 2026 13:17:06 -0500 Subject: [PATCH] fix: tolerate revoked Slack channel visibility during discovery Co-Authored-By: Paperclip --- packages/shared/src/slack-tools.ts | 2 +- .../chat-channels.integration.test.ts | 13 ++++++++++++ .../services/connectors/slack-access.test.ts | 13 ++++++++++++ .../src/services/connectors/slack-access.ts | 20 +++++++++++++++---- skills/slack/SKILL.md | 7 +++++++ 5 files changed, 50 insertions(+), 5 deletions(-) diff --git a/packages/shared/src/slack-tools.ts b/packages/shared/src/slack-tools.ts index 94348a4718b..65345b5b573 100644 --- a/packages/shared/src/slack-tools.ts +++ b/packages/shared/src/slack-tools.ts @@ -145,7 +145,7 @@ export const SLACK_TOOLS = [ "assistant.search.context", ["search:read.public", "search:read.files"], "read", - "Search authorized channels by text, author and time. Reports native search or bounded history scan and its coverage. Never claim a partial scan is exhaustive.", + "Search authorized channel history by text, author and time. Pass channels as an array of channel IDs, query as plain text, and limit from 1 to 20 (matches per channel, not messages scanned). Reports bounded scan coverage; never claim it is exhaustive.", { channels: z.array(channel).min(1).max(10), query: text, diff --git a/server/src/__tests__/chat-channels.integration.test.ts b/server/src/__tests__/chat-channels.integration.test.ts index 72a09589ea3..0b580154069 100644 --- a/server/src/__tests__/chat-channels.integration.test.ts +++ b/server/src/__tests__/chat-channels.integration.test.ts @@ -4002,6 +4002,19 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { const read = await executeSlackTool(db, binding, "slack_history", { channel: "COTHER" }, fetched as typeof fetch); expect(read).toMatchObject({ sourceTrust: "untrusted", nextCursor: "older", hasMore: true }); expect(JSON.stringify(read)).toContain("UUNLINKED"); + const directoryFetch = vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + const method = String(input).split("/").at(-1); + const args = Object.fromEntries(new URLSearchParams(String(init?.body ?? ""))); + if (method === "conversations.list") return Response.json({ ok: true, channels: [{ id: "CGONE", is_member: true }, { id: "COTHER", is_member: true }], response_metadata: { next_cursor: "directory-next" } }); + if (method === "conversations.info" && args.channel === "CGONE") return Response.json({ ok: false, error: "channel_not_found" }); + return fetched(input, init); + }); + await expect(executeSlackTool(db, binding, "slack_channels", {}, directoryFetch as typeof fetch)).resolves.toMatchObject({ channels: [{ id: "COTHER" }], nextCursor: "directory-next" }); + await expect(executeSlackTool(db, binding, "slack_search", { channels: ["COTHER"], query: "ship Tuesday", limit: 10 }, fetched as typeof fetch)).resolves.toMatchObject({ + mode: "bounded_history", exhaustive: false, + matches: [{ user: "UUNLINKED", ts: "7000.1" }], + inspected: [{ channel: "COTHER", count: 1, nextCursor: "older", hasMore: true }], + }); await expect(executeSlackTool(db, binding, "slack_history", { channel: "GPRIVATE" }, fetched as typeof fetch)).rejects.toThrow("in a DM"); vi.stubEnv("PAPERCLIP_TOOL_ACTION_SIGNING_SECRET", "slack-tools-integration-signing-secret"); onTestFinished(() => vi.unstubAllEnvs()); diff --git a/server/src/services/connectors/slack-access.test.ts b/server/src/services/connectors/slack-access.test.ts index 070f177f593..7bf51db9f47 100644 --- a/server/src/services/connectors/slack-access.test.ts +++ b/server/src/services/connectors/slack-access.test.ts @@ -9,6 +9,7 @@ import { slackClient } from "./slack-client.js"; import type { SlackTaskAuthority } from "./slack-authority.js"; import type { Db } from "@paperclipai/db"; import { SLACK_TOOLS } from "@paperclipai/shared"; +import { tooManyRequests, unprocessable } from "../../errors.js"; const authority = { endpoint: { id: "endpoint", @@ -48,6 +49,18 @@ const fakeDb = (rows: unknown[]) => select: () => ({ from: () => ({ where: async () => rows }) }), }) as unknown as Db; describe("Slack access and publication boundaries", () => { + it("treats missing channels as access denials without hiding provider failures", async () => { + for (const code of ["slack_channel_not_found", "slack_not_in_channel"]) { + const api = apiFor({}); + api.mockRejectedValue(unprocessable("Slack rejected the lookup", { code })); + await expect(authorizeSlackChannel(authority, api, "C1")).rejects.toMatchObject({ status: 403 }); + } + for (const error of [tooManyRequests("Rate limited"), unprocessable("Missing scope", { code: "slack_missing_scope" })]) { + const api = apiFor({}); + api.mockRejectedValue(error); + await expect(authorizeSlackChannel(authority, api, "C1")).rejects.toBe(error); + } + }); it("reads a public shared channel without checking the enabled response destination", async () => { expect((await authorizeSlackChannel(authority, apiFor({}), "C1")).id).toBe( "C1", diff --git a/server/src/services/connectors/slack-access.ts b/server/src/services/connectors/slack-access.ts index 8004b576552..e6c6b84c628 100644 --- a/server/src/services/connectors/slack-access.ts +++ b/server/src/services/connectors/slack-access.ts @@ -1,6 +1,6 @@ import { and, eq, sql } from "drizzle-orm"; import { chatActions, chatEndpointResources, type Db } from "@paperclipai/db"; -import { forbidden, unprocessable } from "../../errors.js"; +import { HttpError, forbidden, unprocessable } from "../../errors.js"; import type { SlackTaskAuthority } from "./slack-authority.js"; import { nextCursor, @@ -15,9 +15,21 @@ export async function authorizeSlackChannel( api: Api, channelId: string, ) { - const channel = object( - (await api("conversations.info", { channel: channelId })).channel, - ); + let channel: SlackObject; + try { + channel = object( + (await api("conversations.info", { channel: channelId })).channel, + ); + } catch (error) { + // Membership can disappear between listing and inspection. Slack also + // hides inaccessible conversations with channel_not_found. Treat only + // these definite access failures as denials; outages must still surface. + if (error instanceof HttpError && error.status === 422 && + ["slack_channel_not_found", "slack_not_in_channel"].includes(String(object(error.details).code))) { + throw forbidden("This Slack channel is no longer accessible to the bot"); + } + throw error; + } if ( channel.id !== channelId || (channel.is_member !== true && channel.is_im !== true) diff --git a/skills/slack/SKILL.md b/skills/slack/SKILL.md index 363630381c8..15e35446b4c 100644 --- a/skills/slack/SKILL.md +++ b/skills/slack/SKILL.md @@ -38,6 +38,13 @@ history scan is not workspace-wide search and does not automatically inspect thread replies. Fetch further history/thread pages when needed. Report omitted history, rate limits, missing scopes and unavailable Slack features accurately. +For example, to search the assigned channel, call `slack_search` with +`{"channels":["C012AB3CD"],"query":"launch decision","limit":10}`, substituting +the supplied channel ID. `channels` is an array; `limit` is at most 20 matches, +not the history page size. Do not add Slack search syntax to a channel ID or +pass unsupported fields. A schema rejection means the arguments need correcting; +it does not mean another Slack connection is needed. + ## Collaboration and delivery Use Slack messages, uploads, reactions, pins, bookmarks, topics, canvases and lists