mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-25 09:41:04 +01:00
fix: qualify Slack transport and task tool initialization
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
985e0ea223
commit
4e00210da0
11 files changed
+119
-25
No files matched your search
@@ -1,6 +1,13 @@
|
||||
import { z } from "zod";
|
||||
|
||||
const channel = z.string().regex(/^[CGD][A-Z0-9]+$/);
|
||||
export const slackSearchConfigSchema = z
|
||||
.object({
|
||||
clientId: z.string().regex(/^\d+\.\d+$/),
|
||||
clientSecret: z.string().min(10).max(512),
|
||||
})
|
||||
.strict();
|
||||
|
||||
const channel = z.string().regex(/^[CGD][A-Z0-9]+$/).describe("Slack channel ID (for example C012AB3CD), not a channel name or URL. Use the assigned source channel or slack_channels results.");
|
||||
const user = z.string().regex(/^[UW][A-Z0-9]+$/);
|
||||
const timestamp = z.string().regex(/^\d+\.\d+$/);
|
||||
const text = z.string().min(1).max(12000);
|
||||
|
||||
@@ -3937,7 +3937,7 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
|
||||
});
|
||||
|
||||
it("binds Slack tools to the admitted linked request, recovers its provenance and revokes retained access", async () => {
|
||||
const { resolveConnectorAssignments, executeConnectorTool } = await import("../services/connector-runtime.js");
|
||||
const { resolveConnectorAssignments, executeConnectorTool, applyConnectorSkills, prepareConnectorSkillDelivery } = await import("../services/connector-runtime.js");
|
||||
const { executeSlackTool } = await import("../services/connectors/slack.js");
|
||||
const { resolveSlackTaskAuthority } = await import("../services/connectors/slack-authority.js");
|
||||
await instanceSettingsService(db).updateExperimental({ enableChatConnectors: true });
|
||||
@@ -3955,16 +3955,44 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
|
||||
await db.insert(heartbeatRuns).values({ id: runId, companyId: fixture.companyId, agentId: fixture.assignedAgentId, status: "running", invocationSource: "assignment", responsibleUserId: "owner-user", contextSnapshot: { issueId: conversation.issueId } });
|
||||
const origin = await initializeRunIdentity(db, { companyId: fixture.companyId, runId, issueId: conversation.issueId, responsibleUserId: "owner-user", messageIds: [String(action.payload.commentId)], cause: "instruction" });
|
||||
const binding = { companyId: fixture.companyId, agentId: fixture.assignedAgentId, runId, issueId: conversation.issueId };
|
||||
// An ingress transaction may briefly own the endpoint while a retained
|
||||
// run resolves its tools. Resolution waits, then rechecks authorization.
|
||||
let unlockEndpoint!: () => void;
|
||||
let lockedEndpoint!: () => void;
|
||||
const endpointLocked = new Promise<void>(resolve => { lockedEndpoint = resolve; });
|
||||
const endpointRelease = new Promise<void>(resolve => { unlockEndpoint = resolve; });
|
||||
const ingressLock = db.transaction(async tx => {
|
||||
await tx.select().from(chatEndpoints).where(eq(chatEndpoints.id, endpoint.id)).for("no key update");
|
||||
lockedEndpoint();
|
||||
await endpointRelease;
|
||||
});
|
||||
await endpointLocked;
|
||||
const resolvingAuthority = resolveSlackTaskAuthority(db, binding);
|
||||
// Attach the rejection handler before waiting so NOWAIT regressions fail
|
||||
// as an assertion, not an unhandled rejection.
|
||||
const resolvedAuthority = resolvingAuthority.then(value => ({ value }), error => ({ error }));
|
||||
try {
|
||||
await new Promise(resolve => setTimeout(resolve, 100));
|
||||
} finally {
|
||||
unlockEndpoint();
|
||||
await ingressLock;
|
||||
}
|
||||
expect(await resolvedAuthority).toMatchObject({ value: { slackUserId: "UTOOLS" } });
|
||||
const assignments = await resolveConnectorAssignments(db, binding);
|
||||
expect(assignments.map(a => a.key)).toEqual(["slack"]);
|
||||
expect(assignments[0].resources[0].metadata?.channelId).toBe("CTOOLS");
|
||||
const skillConfig = await applyConnectorSkills({}, [], assignments);
|
||||
const runnerDelivery = await prepareConnectorSkillDelivery(skillConfig, "paperclip_runner");
|
||||
expect(runnerDelivery.instructions).toContain('"channelId": "CTOOLS"');
|
||||
expect(runnerDelivery.instructions).toContain("untrusted source material");
|
||||
expect(runnerDelivery.instructions).not.toContain("xoxb-test-token");
|
||||
expect(JSON.stringify(assignments)).not.toContain("xoxb-test-token");
|
||||
await expect(resolveConnectorAssignments(db, { ...binding, agentId: fixture.replacementAgentId })).resolves.toEqual([]);
|
||||
await expect(resolveConnectorAssignments(db, { ...binding, issueId: randomUUID() })).resolves.toEqual([]);
|
||||
await expect(resolveConnectorAssignments(db, { ...binding, companyId: randomUUID() })).resolves.toEqual([]);
|
||||
const fetched = vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => {
|
||||
const method = String(input).split("/").at(-1);
|
||||
const args = JSON.parse(String(init?.body ?? "{}"));
|
||||
const args = Object.fromEntries(new URLSearchParams(String(init?.body ?? "")));
|
||||
if (method === "users.info") return Response.json({ ok: true, user: { id: "UTOOLS", team_id: "T-PAPERCLIP" } });
|
||||
if (method === "conversations.info") return Response.json({ ok: true, channel: { id: args.channel, is_member: true, is_private: args.channel === "GPRIVATE" } });
|
||||
if (method === "conversations.members") return Response.json({ ok: true, members: ["UTOOLS"] });
|
||||
|
||||
@@ -29,6 +29,7 @@ const apiPrefixes: Record<string, string> = {
|
||||
"board-chat.ts": "/api",
|
||||
"built-in-agents.ts": "/api",
|
||||
"chat-channels.ts": "/api",
|
||||
"slack-tools.ts": "/api",
|
||||
"email.ts": "/api",
|
||||
"cloud.ts": "/api/cloud",
|
||||
"companies.ts": "/api/companies",
|
||||
|
||||
@@ -13,6 +13,8 @@ import {
|
||||
emailEndpointSetupSchema,
|
||||
emailConnectionSchema,
|
||||
emailSendSchema,
|
||||
slackToolCallSchema,
|
||||
slackSearchConfigSchema,
|
||||
// Agent
|
||||
AGENT_PALETTE_IDS,
|
||||
AGENT_AVATAR_SIZES,
|
||||
@@ -1483,6 +1485,12 @@ const BOARD_ONLY_OPERATIONS = new Set([
|
||||
// Chat endpoints expose provider credentials, identity mappings, access
|
||||
// policy, and replay controls. Every mounted handler asserts a board actor;
|
||||
// keep the generated security contract equally restrictive.
|
||||
"GET /api/slack/search/callback",
|
||||
"GET /api/companies/{companyId}/slack/endpoints/{endpointId}/capabilities",
|
||||
"GET /api/companies/{companyId}/slack/endpoints/{endpointId}/search",
|
||||
"PUT /api/companies/{companyId}/slack/endpoints/{endpointId}/search",
|
||||
"POST /api/companies/{companyId}/slack/endpoints/{endpointId}/search/connect",
|
||||
"DELETE /api/companies/{companyId}/slack/endpoints/{endpointId}/search",
|
||||
"GET /api/companies/{companyId}/chat-endpoints",
|
||||
"POST /api/companies/{companyId}/chat-endpoints",
|
||||
"GET /api/chat-endpoints/{endpointId}",
|
||||
@@ -1622,7 +1630,7 @@ function resolveOperationAuthLevel(
|
||||
): OpenApiAuthLevel {
|
||||
const key = operationKey(method, path);
|
||||
if (PUBLIC_OPERATIONS.has(key)) return "public";
|
||||
if (key === "POST /api/mcp/project-tools") return "agent_run";
|
||||
if (key === "POST /api/mcp/project-tools" || key === "POST /api/companies/{companyId}/slack/tasks/{issueId}/tools") return "agent_run";
|
||||
if (RUNTIME_TOOLS_OPERATIONS.has(key)) return "runtime_tools";
|
||||
if (INSTANCE_ADMIN_OPERATIONS.has(key)) return "instance_admin";
|
||||
if (
|
||||
@@ -2101,6 +2109,28 @@ for (const [method, path, summary, body, success] of [
|
||||
});
|
||||
}
|
||||
|
||||
// Slack bot tools use the verified task/run identity; setup and search grants
|
||||
// remain board-only and company/endpoint scoped.
|
||||
for (const [method, path, summary, body] of [
|
||||
["get", "/api/companies/{companyId}/slack/endpoints/{endpointId}/capabilities", "Inspect Slack bot capabilities and missing scopes", undefined],
|
||||
["get", "/api/companies/{companyId}/slack/endpoints/{endpointId}/search", "Read personal Slack search authorization status", undefined],
|
||||
["put", "/api/companies/{companyId}/slack/endpoints/{endpointId}/search", "Configure optional Slack search OAuth credentials", slackSearchConfigSchema],
|
||||
["post", "/api/companies/{companyId}/slack/endpoints/{endpointId}/search/connect", "Begin personal Slack search authorization", undefined],
|
||||
["delete", "/api/companies/{companyId}/slack/endpoints/{endpointId}/search", "Disconnect personal Slack search and invalidate pending authorization", undefined],
|
||||
["post", "/api/companies/{companyId}/slack/tasks/{issueId}/tools", "Execute a task-bound Slack bot tool", slackToolCallSchema],
|
||||
] as const) {
|
||||
registry.registerPath({ method, path, tags: ["chat-channels"], summary,
|
||||
description: "Experimental Slack task tools. Current company, endpoint, linked requester, task/run authority and action permissions are revalidated. Bot credentials remain server-side. Search grants never authorize writes or expand bot membership. Native search is unavailable until the runtime qualifies transient result handling.",
|
||||
request: { params: z.object(Object.fromEntries([...path.matchAll(/\{([^}]+)\}/g)].map(match => [match[1], z.string().uuid()]))), ...(body ? { body: jsonBody(body) } : {}) },
|
||||
responses: { 200: r.ok(), 400: r.badRequest, 401: r.unauthorized, 403: r.forbidden, 404: r.notFound, 409: r.conflict },
|
||||
});
|
||||
}
|
||||
registry.registerPath({ method: "get", path: "/api/slack/search/callback", tags: ["chat-channels"], summary: "Complete personal Slack search OAuth",
|
||||
description: "Requires the same signed-in user, single-use state, linked Slack identity and workspace; redirects to connector Access. Never accepts model-supplied identity.",
|
||||
request: { query: z.object({ state: z.string(), code: z.string() }) },
|
||||
responses: { 302: { description: "Redirect to connector Access" }, 400: r.badRequest, 401: r.unauthorized, 403: r.forbidden },
|
||||
});
|
||||
|
||||
// ─── Chat Channels ─────────────────────────────────────────────────────────
|
||||
|
||||
registry.registerPath({
|
||||
|
||||
@@ -85,8 +85,7 @@ export function slackToolRoutes(db: Db, publicBaseUrl?: string) {
|
||||
))
|
||||
);
|
||||
}
|
||||
const searchPath = "/companies/:companyId/slack/endpoints/:endpointId/search";
|
||||
router.get(searchPath, async (req, res) => {
|
||||
router.get("/companies/:companyId/slack/endpoints/:endpointId/search", async (req, res) => {
|
||||
const userId = user(req);
|
||||
const companyId = String(req.params.companyId);
|
||||
assertCompanyAccess(req, companyId);
|
||||
@@ -95,7 +94,7 @@ export function slackToolRoutes(db: Db, publicBaseUrl?: string) {
|
||||
canConfigure: await canConfigure(req, companyId, userId),
|
||||
});
|
||||
});
|
||||
router.put(searchPath, async (req, res) => {
|
||||
router.put("/companies/:companyId/slack/endpoints/:endpointId/search", async (req, res) => {
|
||||
const userId = user(req);
|
||||
const companyId = String(req.params.companyId);
|
||||
assertCompanyAccess(req, companyId);
|
||||
@@ -119,7 +118,7 @@ export function slackToolRoutes(db: Db, publicBaseUrl?: string) {
|
||||
),
|
||||
);
|
||||
});
|
||||
router.post(`${searchPath}/connect`, async (req, res) => {
|
||||
router.post("/companies/:companyId/slack/endpoints/:endpointId/search/connect", async (req, res) => {
|
||||
const userId = user(req);
|
||||
const companyId = String(req.params.companyId);
|
||||
assertCompanyAccess(req, companyId);
|
||||
@@ -129,7 +128,7 @@ export function slackToolRoutes(db: Db, publicBaseUrl?: string) {
|
||||
await oauth.start(companyId, String(req.params.endpointId), userId),
|
||||
);
|
||||
});
|
||||
router.delete(searchPath, async (req, res) => {
|
||||
router.delete("/companies/:companyId/slack/endpoints/:endpointId/search", async (req, res) => {
|
||||
const userId = user(req);
|
||||
const companyId = String(req.params.companyId);
|
||||
assertCompanyAccess(req, companyId);
|
||||
|
||||
@@ -38168,6 +38168,14 @@ export function chatChannelService(db: Db, options: ChatChannelServiceOptions) {
|
||||
)).limit(1);
|
||||
if (!action || action.payload.requestedByActorType !== "user" || action.payload.requestedByActorId !== run.responsibleUserId)
|
||||
throw forbidden("Slack tools require a verified linked Slack request");
|
||||
// This standalone resolver does not own the scheduler's issue lock. Use
|
||||
// the ingress lock order (endpoint, then issue) so a normal webhook or
|
||||
// endpoint update cannot turn tool setup into a NOWAIT failure. The
|
||||
// scheduler retains its nonblocking check when it owns the issue first.
|
||||
await tx.select({ id: chatEndpoints.id }).from(chatEndpoints).where(and(
|
||||
eq(chatEndpoints.companyId, binding.companyId),
|
||||
eq(chatEndpoints.id, action.endpointId),
|
||||
)).for("no key update");
|
||||
const source = await authorizeInboundWakeup(tx, action);
|
||||
if (source.endpoint.provider !== "slack") throw forbidden("This is not a Slack task");
|
||||
const [principal] = await tx.select().from(chatExternalPrincipals).where(and(
|
||||
|
||||
@@ -216,7 +216,17 @@ export async function prepareConnectorSkillDelivery(
|
||||
adapterType === "paperclip_runner" ||
|
||||
(config.engine === "cli" &&
|
||||
["codex_local", "claude_local", "kimi_local"].includes(adapterType));
|
||||
if (scopedFiles) return { config, instructions: "" };
|
||||
if (scopedFiles) {
|
||||
// Runner models with semantic tools cannot necessarily read staged skill
|
||||
// files. Supply the Slack contract and verified source IDs in their input;
|
||||
// keep the staged bundle for CLI-capable engines and compatibility hashing.
|
||||
const slack = adapterType === "paperclip_runner"
|
||||
? config.paperclipRuntimeSkills.filter(entry => entry.key === "paperclipai/paperclip/slack")
|
||||
: [];
|
||||
const instructions = (await Promise.all(slack.map(entry =>
|
||||
fs.readFile(path.join(entry.source, "SKILL.md"), "utf8")))).join("\n\n");
|
||||
return { config, instructions };
|
||||
}
|
||||
const assigned = config.paperclipRuntimeSkills.filter((entry) =>
|
||||
isConnectorSkill(entry.key),
|
||||
);
|
||||
|
||||
@@ -171,6 +171,18 @@ describe("Slack contracts and transport", () => {
|
||||
).rejects.toThrow("Unsupported");
|
||||
expect(fetcher).not.toHaveBeenCalled();
|
||||
});
|
||||
it("encodes Slack arguments like the official SDK, with nested values as JSON fields", async () => {
|
||||
const fetcher = vi.fn().mockImplementation(async () => Response.json({ ok: true }));
|
||||
await slackClient("bot-secret", fetcher)("conversations.list", {
|
||||
types: "public_channel,private_channel,mpim,im", limit: 50, exclude_archived: true, cursor: undefined,
|
||||
});
|
||||
expect(fetcher.mock.calls[0][1].headers["content-type"]).toContain("application/x-www-form-urlencoded");
|
||||
expect(Object.fromEntries(new URLSearchParams(fetcher.mock.calls[0][1].body))).toEqual({
|
||||
types: "public_channel,private_channel,mpim,im", limit: "50", exclude_archived: "true",
|
||||
});
|
||||
await slackClient("bot-secret", fetcher)("files.completeUploadExternal", { files: [{ id: "F123", title: "Test & review" }] });
|
||||
expect(JSON.parse(new URLSearchParams(fetcher.mock.calls[1][1].body).get("files")!)).toEqual([{ id: "F123", title: "Test & review" }]);
|
||||
});
|
||||
it("reports missing scopes without exposing raw provider errors or credentials", async () => {
|
||||
const fetcher = vi.fn().mockResolvedValue(
|
||||
new Response(
|
||||
|
||||
@@ -39,9 +39,14 @@ export function slackClient(token: string, fetchImpl = fetch) {
|
||||
signal: AbortSignal.timeout(20_000),
|
||||
headers: {
|
||||
authorization: `Bearer ${token}`,
|
||||
"content-type": "application/json; charset=utf-8",
|
||||
"content-type": "application/x-www-form-urlencoded; charset=utf-8",
|
||||
},
|
||||
body: JSON.stringify(args),
|
||||
// Match Slack's official WebClient encoding, including nested JSON
|
||||
// arguments. Some Web API methods reject a JSON request body.
|
||||
body: new URLSearchParams(Object.entries(args)
|
||||
.filter(([, value]) => value !== undefined && value !== null)
|
||||
.map(([key, value]) => [key, typeof value === "object"
|
||||
? JSON.stringify(value) : String(value)])).toString(),
|
||||
});
|
||||
} catch {
|
||||
throw unprocessable(
|
||||
@@ -104,7 +109,7 @@ export function slackClient(token: string, fetchImpl = fetch) {
|
||||
? result.needed
|
||||
: undefined;
|
||||
throw unprocessable(
|
||||
`Slack could not perform this operation: ${code}${scopes ? `. Reinstall the app with ${scopes}.` : ". Check app permissions and Slack feature availability."}`,
|
||||
`Slack could not perform this operation: ${code}${scopes ? `. Reinstall the app with ${scopes}.` : code === "invalid_arguments" ? ". Check the tool arguments; installing another connector will not fix this request." : ". Check app permissions and Slack feature availability."}`,
|
||||
{ code: `slack_${code}`, missingScopes: scopes },
|
||||
);
|
||||
}
|
||||
|
||||
@@ -62,11 +62,10 @@ describe("Slack transient search provider (not exposed to retaining runtimes)",
|
||||
const init = (
|
||||
fetched.mock.calls as unknown as [unknown, RequestInit][]
|
||||
)[0][1];
|
||||
expect(JSON.parse(String(init.body))).toMatchObject({
|
||||
action_token: "verified-event-token",
|
||||
term_clauses: ["in:<#C1>", "from:<@U1>"],
|
||||
include_context_messages: false,
|
||||
});
|
||||
const sent = new URLSearchParams(String(init.body));
|
||||
expect(sent.get("action_token")).toBe("verified-event-token");
|
||||
expect(JSON.parse(sent.get("term_clauses")!)).toEqual(["in:<#C1>", "from:<@U1>"]);
|
||||
expect(sent.get("include_context_messages")).toBe("false");
|
||||
expect(JSON.stringify(result)).not.toContain("verified-event-token");
|
||||
});
|
||||
it("uses a search-only personal grant for private search and rejects revoked access before delivery", async () => {
|
||||
@@ -97,7 +96,7 @@ describe("Slack transient search provider (not exposed to retaining runtimes)",
|
||||
expect(init.headers).toMatchObject({
|
||||
authorization: "Bearer personal-token",
|
||||
});
|
||||
expect(JSON.parse(String(init.body))).not.toHaveProperty("action_token");
|
||||
expect(new URLSearchParams(String(init.body)).has("action_token")).toBe(false);
|
||||
});
|
||||
it("does not expose file hits outside bot-verified channel shares", async () => {
|
||||
const fetched = vi.fn(async (url: RequestInfo | URL) =>
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { slackSearchConfigSchema } from "@paperclipai/shared";
|
||||
import { SLACK_NATIVE_SEARCH_LIMITATION } from "./slack-native-search.js";
|
||||
import { syncConnectionCredentialBindings } from "../connection-credential-bindings.js";
|
||||
import { randomBytes } from "node:crypto";
|
||||
@@ -29,12 +30,6 @@ export const SLACK_SEARCH_USER_SCOPES = [
|
||||
"search:read.private",
|
||||
"search:read.files",
|
||||
];
|
||||
export const slackSearchConfigSchema = z
|
||||
.object({
|
||||
clientId: z.string().regex(/^\d+\.\d+$/),
|
||||
clientSecret: z.string().min(10).max(512),
|
||||
})
|
||||
.strict();
|
||||
const configSchema = z.object({
|
||||
clientId: z.string(),
|
||||
clientSecretId: z.string().uuid(),
|
||||
|
||||
Reference in new issue
Block a user