mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-21 00:05:49 +02:00
Explain disabled Slack MCP app access during discovery
Recognize Slack's exact disabled-app response and return actionable setup instructions from catalog and health routes. Bound response parsing and keep unknown upstream errors reportable without exposing provider settings links. Verified 361 focused tests, server typecheck, and authenticated discovery. The three route regressions fail before this change and pass afterward. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
3c2bc4f546
commit
a7d3b17a97
6 files changed
+187
No files matched your search
@@ -62,6 +62,14 @@ back that step while retaining the established draft identity.
|
||||
Later health checks still require the user's authorization and return an
|
||||
actionable `422` error when it is missing.
|
||||
|
||||
### Slack app access
|
||||
|
||||
If Slack reports that MCP access is disabled for the app, ask the Slack app
|
||||
owner to enable MCP access in the app's Slack settings, then refresh the
|
||||
connection. Signing in alone does not enable that app setting. Paperclip shows
|
||||
this as a setup error and keeps the connection available for retry. See
|
||||
[Slack's MCP app requirements](https://docs.slack.dev/ai/slack-mcp-server/#app-identity).
|
||||
|
||||
### Advanced authentication
|
||||
|
||||
Collapsed by default. Open it when the server's docs are specific:
|
||||
|
||||
@@ -483,6 +483,10 @@ These fields contain build identifiers; they add no tenant or user identity.
|
||||
refreshing its catalog returns 422 with `oauth_challenge` and the existing
|
||||
setup/reconnect links. Other upstream failures still return 502 and are
|
||||
reported, including an unexplained upstream HTTP 400.
|
||||
- Slack's explicit response that its app has not enabled MCP access. Discovery
|
||||
and health checks return 422 with `slack_mcp_access_disabled` and setup
|
||||
instructions. This requires Slack's exact MCP endpoint and known error;
|
||||
other HTTP 400 responses remain reportable.
|
||||
- A performance trace and a profile, because `tracesSampleRate` is 0.
|
||||
|
||||
### Operator responsibilities
|
||||
|
||||
@@ -13768,6 +13768,55 @@ describeEmbeddedPostgres("tool access service", () => {
|
||||
await expect(db.select().from(toolConnections)).resolves.toHaveLength(0);
|
||||
});
|
||||
|
||||
it.each(["catalog", "catalog/refresh", "health-check"])(
|
||||
"explains disabled Slack MCP access on %s without reporting a server error",
|
||||
async (path) => {
|
||||
const company = await createCompany(db);
|
||||
const [application] = await db.insert(toolApplications).values({
|
||||
companyId: company.id,
|
||||
applicationKey: `slack-setup-${randomUUID()}`,
|
||||
name: "Slack setup fixture",
|
||||
type: "mcp_http",
|
||||
status: "active",
|
||||
}).returning();
|
||||
const [connection] = await db.insert(toolConnections).values({
|
||||
companyId: company.id,
|
||||
applicationId: application!.id,
|
||||
name: "Slack setup fixture",
|
||||
uid: `test/${randomUUID()}`,
|
||||
transport: "mcp_remote",
|
||||
status: "draft",
|
||||
enabled: false,
|
||||
config: { url: "https://mcp.slack.com/mcp" },
|
||||
transportConfig: { url: "https://mcp.slack.com/mcp" },
|
||||
}).returning();
|
||||
vi.spyOn(globalThis, "fetch").mockImplementation(async () => new Response(
|
||||
JSON.stringify({ jsonrpc: "2.0", id: null, error: {
|
||||
code: -32600,
|
||||
message: "App is not enabled for Slack MCP server access. Please enable it here: https://api.slack.com/apps/fixture/mcp",
|
||||
} }),
|
||||
{ status: 400, headers: { "content-type": "application/json" } },
|
||||
));
|
||||
const capture = vi.spyOn(sentry, "captureException").mockImplementation(() => {});
|
||||
const app = createRouteApp(db);
|
||||
const url = `/api/tool-connections/${connection!.id}/${path}`;
|
||||
const response = await (path === "catalog" ? request(app).get(url) : request(app).post(url));
|
||||
expect(response.status).toBe(422);
|
||||
expect(response.body).toMatchObject({
|
||||
code: "slack_mcp_access_disabled",
|
||||
error: "Slack MCP access is disabled for this app. Ask the Slack app owner to enable MCP access, then refresh this connection.",
|
||||
details: { code: "slack_mcp_access_disabled", setupUrl: expect.any(String) },
|
||||
});
|
||||
expect(JSON.stringify(response.body)).not.toContain("api.slack.com/apps/fixture");
|
||||
expect(capture).not.toHaveBeenCalled();
|
||||
const [updated] = await db.select().from(toolConnections).where(eq(toolConnections.id, connection!.id));
|
||||
expect(updated?.healthStatus).toBe("error");
|
||||
expect(updated?.healthMessage).toBe(response.body.error);
|
||||
await expect(db.select().from(toolCatalogEntries).where(eq(toolCatalogEntries.connectionId, connection!.id)))
|
||||
.resolves.toHaveLength(0);
|
||||
},
|
||||
);
|
||||
|
||||
it.each([
|
||||
["catalog", 401, 'Bearer realm="app"', 422, false],
|
||||
["catalog/refresh", 401, 'Bearer realm="app"', 422, false],
|
||||
|
||||
@@ -0,0 +1,69 @@
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { isSlackMcpAccessDisabledResponse } from "./slack-mcp-error.js";
|
||||
|
||||
const endpoint = "https://mcp.slack.com/mcp";
|
||||
const message = "App is not enabled for Slack MCP server access. Please enable it here: https://api.slack.com/apps/fixture/mcp";
|
||||
const payload = { jsonrpc: "2.0", id: null, error: { code: -32600, message } };
|
||||
const response = (body: unknown = payload, status = 400, contentType = "application/json") =>
|
||||
new Response(JSON.stringify(body), { status, headers: { "content-type": contentType } });
|
||||
|
||||
afterEach(() => vi.useRealTimers());
|
||||
|
||||
describe("Slack MCP app setup failure", () => {
|
||||
it("recognizes Slack's explicit disabled-app response, including its null RPC id", async () => {
|
||||
expect(await isSlackMcpAccessDisabledResponse(endpoint, response())).toBe(true);
|
||||
});
|
||||
|
||||
it.each([
|
||||
"https://other.example.test/mcp",
|
||||
"https://mcp.slack.com.example.test/mcp",
|
||||
"http://mcp.slack.com/mcp",
|
||||
"https://mcp.slack.com:8443/mcp",
|
||||
"https://mcp.slack.com/other",
|
||||
])("does not classify matching text from %s", async (url) => {
|
||||
const upstream = response();
|
||||
expect(await isSlackMcpAccessDisabledResponse(url, upstream)).toBe(false);
|
||||
expect(upstream.bodyUsed).toBe(false);
|
||||
});
|
||||
|
||||
it.each([401, 403, 429, 500, 503])("does not hide HTTP %i behind a setup error", async (status) => {
|
||||
expect(await isSlackMcpAccessDisabledResponse(endpoint, response(payload, status))).toBe(false);
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ ...payload, error: { code: -32600, message: "Missing session ID" } },
|
||||
{ ...payload, error: { code: -32603, message } },
|
||||
{ ...payload, error: message },
|
||||
{ ...payload, jsonrpc: "1.0" },
|
||||
null,
|
||||
])("keeps unrecognized errors reportable: %j", async (body) => {
|
||||
expect(await isSlackMcpAccessDisabledResponse(endpoint, response(body))).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects non-JSON content and malformed JSON", async () => {
|
||||
expect(await isSlackMcpAccessDisabledResponse(endpoint, response(payload, 400, "text/html"))).toBe(false);
|
||||
expect(await isSlackMcpAccessDisabledResponse(endpoint,
|
||||
new Response("{", { status: 400, headers: { "content-type": "application/json" } }))).toBe(false);
|
||||
});
|
||||
|
||||
it("bounds and cancels oversized error responses", async () => {
|
||||
const cancel = vi.fn();
|
||||
const body = new ReadableStream({
|
||||
start(controller) { controller.enqueue(new TextEncoder().encode(JSON.stringify({ ...payload, extra: "x".repeat(8192) }))); },
|
||||
cancel,
|
||||
});
|
||||
expect(await isSlackMcpAccessDisabledResponse(endpoint,
|
||||
new Response(body, { status: 400, headers: { "content-type": "application/json" } }))).toBe(false);
|
||||
expect(cancel).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("bounds and cancels a stalled error body", async () => {
|
||||
vi.useFakeTimers();
|
||||
const cancel = vi.fn();
|
||||
const result = isSlackMcpAccessDisabledResponse(endpoint,
|
||||
new Response(new ReadableStream({ cancel }), { status: 400, headers: { "content-type": "application/json" } }));
|
||||
await vi.advanceTimersByTimeAsync(2_000);
|
||||
expect(await result).toBe(false);
|
||||
expect(cancel).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,46 @@
|
||||
const ACCESS_DISABLED_MESSAGE = "App is not enabled for Slack MCP server access.";
|
||||
const MAX_ERROR_BYTES = 8 * 1024;
|
||||
|
||||
/** Recognize only Slack's documented-endpoint response for missing app setup.
|
||||
* Never return provider text or its embedded account/settings links to callers. */
|
||||
export async function isSlackMcpAccessDisabledResponse(
|
||||
endpoint: string,
|
||||
response: Response,
|
||||
): Promise<boolean> {
|
||||
const url = new URL(endpoint);
|
||||
if (url.origin !== "https://mcp.slack.com" || url.pathname !== "/mcp"
|
||||
|| response.status !== 400
|
||||
|| response.headers.get("content-type")?.split(";")[0]?.trim().toLowerCase() !== "application/json") return false;
|
||||
|
||||
const reader = response.body?.getReader();
|
||||
if (!reader) return false;
|
||||
const chunks: Uint8Array[] = [];
|
||||
let bytes = 0;
|
||||
let timedOut = false;
|
||||
const timeout = setTimeout(() => {
|
||||
timedOut = true;
|
||||
void reader.cancel().catch(() => undefined);
|
||||
}, 2_000);
|
||||
timeout.unref();
|
||||
try {
|
||||
while (true) {
|
||||
const { done, value } = await reader.read();
|
||||
if (timedOut) return false;
|
||||
if (done) break;
|
||||
bytes += value.byteLength;
|
||||
if (bytes > MAX_ERROR_BYTES) return false;
|
||||
chunks.push(value);
|
||||
}
|
||||
const payload = JSON.parse(Buffer.concat(chunks).toString("utf8"));
|
||||
const error = payload?.error;
|
||||
return payload?.jsonrpc === "2.0" && error?.code === -32600
|
||||
&& typeof error.message === "string"
|
||||
&& (error.message === ACCESS_DISABLED_MESSAGE || error.message.startsWith(`${ACCESS_DISABLED_MESSAGE} `));
|
||||
} catch {
|
||||
return false;
|
||||
} finally {
|
||||
clearTimeout(timeout);
|
||||
await reader.cancel().catch(() => undefined);
|
||||
reader.releaseLock();
|
||||
}
|
||||
}
|
||||
@@ -186,6 +186,7 @@ import {
|
||||
import { isUniqueViolation } from "../db-errors.js";
|
||||
import { logger } from "../middleware/logger.js";
|
||||
import { logActivity } from "./activity-log.js";
|
||||
import { isSlackMcpAccessDisabledResponse } from "./slack-mcp-error.js";
|
||||
import {
|
||||
initializeMcpHttpSession,
|
||||
McpHttpInitializationError,
|
||||
@@ -2804,6 +2805,7 @@ function healthFailureHttpStatus(failure: {
|
||||
}): number {
|
||||
if (failure.status === "missing_secret") return 422;
|
||||
if (failure.code === "oauth_challenge") return 422;
|
||||
if (failure.code === "slack_mcp_access_disabled") return 422;
|
||||
if (failure.code === "user_authorization_required") return 422;
|
||||
if (failure.code === "composio_broker_retired") return 422;
|
||||
if (failure.code === "tool_connection_transport_unsupported") return 422;
|
||||
@@ -2825,6 +2827,9 @@ function sanitizeHttpFailure(error: unknown): {
|
||||
} {
|
||||
if (error instanceof HttpError) {
|
||||
const code = asRecord(error.details).code;
|
||||
if (code === "slack_mcp_access_disabled") {
|
||||
return { status: "error", message: error.message, code };
|
||||
}
|
||||
if (code === "user_authorization_required") {
|
||||
return { status: "error", message: error.message, code };
|
||||
}
|
||||
@@ -6859,6 +6864,12 @@ export function toolAccessService(
|
||||
}
|
||||
}
|
||||
if (!response.ok) {
|
||||
if (await isSlackMcpAccessDisabledResponse(endpoint, response)) {
|
||||
throw unprocessable(
|
||||
"Slack MCP access is disabled for this app. Ask the Slack app owner to enable MCP access, then refresh this connection.",
|
||||
{ code: "slack_mcp_access_disabled", setupUrl: connectionSetupUrl(connection) },
|
||||
);
|
||||
}
|
||||
const authenticate = response.headers.get("www-authenticate") ?? "";
|
||||
if (
|
||||
response.status === 401 &&
|
||||
|
||||
Reference in new issue
Block a user