From 36df8933f0cbb6a83a4d7bd44c7aa520253b2a9f Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Mon, 2 Nov 2026 19:55:29 -0600 Subject: [PATCH] fix(slack): resolve manager grants during bot setup (#18707) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Slack chat connections let people start and continue work with an agent. > - Managed setup reuses a personal Slack workspace authorization to create and install the bot. > - The provisioning route passed that Slack grant ID to the generic tool-grant lookup. > - A valid workspace therefore failed with a 404 before provisioning started. > - This pull request selects the existing Slack grant resolver for provisioning and validates the request-body ID. > - The benefit is that saved Slack setup can continue while company and personal ownership checks remain enforced. ## Linked Issues or Issue Description Refs: #15679, which introduced managed Slack setup. **What happened?** Continuing a saved managed Slack draft returned `Connection management resource not found`. The workspace list and endpoint read both succeeded. The provisioning POST returned 404. **Expected behavior** Continue should use the selected personal Slack manager grant and create or resume the bot once. **Steps to reproduce** 1. Authorize a Slack workspace for managed bot setup. 2. Create or resume a managed Slack chat connection. 3. Select the saved workspace and choose Continue. ## What Changed - Select the Slack manager-grant resolver for the managed provisioning route. - Accept a validated grant ID from the route parameter or request body. Enforce the current user's ownership for both board and agent requests. - Run the existing duplicate-install regression through the HTTP route. Add HTTP rejection cases for missing, malformed, foreign-company and other-user grants. - Document the grant boundary and update the matching feature and agent-access maps. The shared authorization guard owns resource resolution and company checks. Its existing Slack resolver previously handled only the revocation route parameter. This change extends that resolver to the provisioning body's grant ID. Generic tool-grant resolution and deployments without managed Slack keep their existing path. ## Verification - Before the fix, the HTTP installation regression failed with the same 404 as the browser walkthrough. - `pnpm exec vitest run server/src/__tests__/chat-channels.integration.test.ts -t 'managed Slack'` — 31 passed. Uses real PostgreSQL, production routes and provider fixtures. The board actor is a fixture; this does not qualify live Slack or agent authentication. - `node scripts/check-maps.mjs` and `git diff --check` — passed. - `pnpm -r typecheck` and `pnpm build` — passed. Full local Vitest is running. [All CI gates passed](https://github.com/paperclipai/paperclip/actions/runs/41561466277) for commit c04b426e60afad6b5127954fcd939af5514a8d8c. Greptile and Paperclip reviews are 5/5 with no required changes. Deployed Slack acceptance remains pending. ## Risks - Low risk. The guard must select the correct grant registry. Tests cover valid ownership, duplicate requests and rejected IDs. - Board requests now receive the ownership denial in the guard. The Slack service already required the same owner. - No schema, credential format, provider scope or runtime transport changes. ## Model Used OpenAI Codex, GPT-6 family, with reasoning, repository editing, browser use and command execution. The runtime does not expose a more precise model ID or context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip --- doc/connections/slack-setup.md | 3 +++ maps/agent-access-map/chat-channels.md | 2 +- maps/feature-map/chat-channels.md | 7 +++++++ .../chat-channels.integration.test.ts | 19 ++++++++++++++++++- server/src/routes/chat-channels.ts | 2 +- .../src/routes/connection-management-authz.ts | 4 ++-- 6 files changed, 32 insertions(+), 5 deletions(-) diff --git a/doc/connections/slack-setup.md b/doc/connections/slack-setup.md index a047b3e28cc..51be34335b7 100644 --- a/doc/connections/slack-setup.md +++ b/doc/connections/slack-setup.md @@ -52,6 +52,9 @@ Slack user, workspace, and manager app. Another company member must authorize th own account, even for the same workspace. Access and rotating refresh tokens are user-scoped vault entries. Refreshes serialize across connections, persist the new refresh token, and require fresh authorization after an ambiguous exchange. +Managed provisioning resolves the submitted grant from the Slack manager-grant +registry. Both provisioning and revocation check its company and current user; +these IDs are separate from tool-connection grants. Revoking a grant requests provider revocation, removes its local credentials, invalidates pending manager authorization, and prevents future provisioning with it. The local activity records whether provider revocation succeeded. Provider errors never restore local access. It does not uninstall diff --git a/maps/agent-access-map/chat-channels.md b/maps/agent-access-map/chat-channels.md index da6e5478a37..745d4de01b3 100644 --- a/maps/agent-access-map/chat-channels.md +++ b/maps/agent-access-map/chat-channels.md @@ -70,7 +70,7 @@ Gaps: This individual operation has source review but no recorded HTTP verificat API: `present` — `POST /api/chat-endpoints/{endpointId}/slack/managed/provision`; JSON REST. Agent access: `available` — agent REST and default native `search_api` / `call_api`. -Policy: `tools:manage_connections`; [shared management policy](../../doc/plans/2026-10-13-connection-api-parity.md#authorization-contract). Resolved resources and every affected target are checked; personal ownership remains the responsible user. +Policy: `tools:manage_connections`; [shared management policy](../../doc/plans/2026-10-13-connection-api-parity.md#authorization-contract). Resolved resources and every affected target are checked. The request body's grantId resolves from the Slack manager-grant registry; its company and owner must match the endpoint and current responsible user. A tool-connection grant does not authorize Slack provisioning. Evidence: `source-only` — [route](../../server/src/routes/chat-channels.ts); reviewed in the working tree based on the source commit above. Verification: `not-run` — individual operation not executed in this audit; shared authorization and representative family flows are exercised by the authenticated integration suite. Follow the route schema, inspect persistence/audit, and assert owner, scope and revoked-authority denial. Gaps: This individual operation has source review but no recorded HTTP verification. Provider availability and live setup remain unqualified. diff --git a/maps/feature-map/chat-channels.md b/maps/feature-map/chat-channels.md index 7eb26bbc117..7db0748a607 100644 --- a/maps/feature-map/chat-channels.md +++ b/maps/feature-map/chat-channels.md @@ -49,6 +49,13 @@ AgentMail bypasses the chat-connectors UI gate, GitHub uses `enableGitHubReviewBots`, and other chat providers use `enableChatConnectors`. Setup is covered in [connection setup](./connection-setup.md). +For managed Slack setup, resume a saved draft with an authorized workspace and +choose Continue. Verify it creates and installs the bot once. The HTTP route +must resolve the selected Slack manager grant, reject malformed or missing IDs, +and deny another company's or person's grant before provider calls. The channel +integration suite exercises this route with a board fixture and provider fixtures; +live Slack setup remains a separate acceptance check. + ### `external-thread` Automated: [channel integration](../../server/src/__tests__/chat-channels.integration.test.ts) uses provider fixtures; it does not prove a live Slack, Discord, Teams, Telegram, GitHub, email, or Photon delivery. diff --git a/server/src/__tests__/chat-channels.integration.test.ts b/server/src/__tests__/chat-channels.integration.test.ts index cabb3d84093..eda5fab7109 100644 --- a/server/src/__tests__/chat-channels.integration.test.ts +++ b/server/src/__tests__/chat-channels.integration.test.ts @@ -9777,7 +9777,9 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { }); it("creates and installs once, uploads the avatar and links only the manager grant owner", async () => { const f = await managedFixture(); - await Promise.all([f.provision(), f.provision()]); + await Promise.all([1, 2].map(() => request(f.app) + .post(`/api/chat-endpoints/${f.endpoint.id}/slack/managed/provision`) + .send(f.input).expect(200))); const saved = await f.service.get(f.endpoint.id); expect(saved.setup).toMatchObject({ slackSetupMethod: "managed", slackRegistration: { status: "configured", appId: f.appId }, slackAvatar: { status: "uploaded" }, slackAccount: { status: "linked", paperclipUserId: actor.userId, externalUserId: f.installerId } }); expect(f.provider.mock.calls.filter(([url]) => String(url).endsWith("apps.manifest.create"))).toHaveLength(1); @@ -9786,6 +9788,21 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { for (const secret of [f.credentials.accessToken, f.credentials.refreshToken!, f.token, clientSecret, signingSecret, "claim-canary"]) expect(JSON.stringify({ saved, activities, choices: await f.grants.choices(f.companyId, actor) })).not.toContain(secret); await expect(f.service.update(f.endpoint.id, { slackSetupMethod: "automatic" }, actor.userId)).rejects.toThrow(); }); + it("rejects missing, malformed, foreign and other-user manager grants before provisioning", async () => { + const f = await managedFixture(); + const foreign = await seedCompany(); + const grants = await db.insert(chatSlackManagerGrants).values([ + { companyId: foreign.companyId, userId: actor.userId, slackUserId: "UFOREIGN", workspaceId: "TFOREIGN", workspaceName: "Foreign", managerAppId: "AMANAGER" }, + { companyId: f.companyId, userId: "another-user", slackUserId: "UOTHER", workspaceId: "TOTHER", workspaceName: "Other", managerAppId: "AMANAGER" }, + ]).returning(); + f.provider.mockClear(); + for (const [grantId, status] of [[randomUUID(), 404], ["invalid", 400], [grants[0]!.id, 404], [grants[1]!.id, 403]] as const) { + await request(f.app).post(`/api/chat-endpoints/${f.endpoint.id}/slack/managed/provision`) + .send({ ...f.input, grantId }).expect(status); + } + expect(f.provider).not.toHaveBeenCalled(); + expect(await db.select().from(chatSlackRegistrations).where(eq(chatSlackRegistrations.endpointId, f.endpoint.id))).toEqual([]); + }); it("reuses the same personal workspace grant for another draft without another authorization", async () => { const f = await managedFixture(); const draft = await f.service.create(f.companyId, { provider: "slack", assignedAgentId: f.assignedAgentId, slackSetupMethod: "managed" }, actor.userId); diff --git a/server/src/routes/chat-channels.ts b/server/src/routes/chat-channels.ts index 947e1a7ada1..162c2c3829b 100644 --- a/server/src/routes/chat-channels.ts +++ b/server/src/routes/chat-channels.ts @@ -230,7 +230,7 @@ export function chatChannelRoutes(db: Db, options: ChatChannelRouteOptions) { if (req.actor.type === "agent") { res.json(await setupHandoff(req, "The responsible user must open the saved setup to bind their browser session and consent to Slack access.")); return; } res.json(await service.slackRegistration.managerGrants.authorize(endpointId(req), slackActor(req))); }); - router.post("/chat-endpoints/:endpointId/slack/managed/provision", managementGuard("connections:configure", "POST /chat-endpoints/:endpointId/slack/managed/provision"), validate(slackManagedProvisionSchema), async (req, res) => { + router.post("/chat-endpoints/:endpointId/slack/managed/provision", managementGuard("connections:configure", "POST /chat-endpoints/:endpointId/slack/managed/provision", { slackManagerGrant: true }), validate(slackManagedProvisionSchema), async (req, res) => { if (!(await assertEndpointManagementAccess(req, res))) return; res.set("Cache-Control", "no-store"); res.json(await service.slackRegistration.managed.provision(endpointId(req), slackActor(req), req.body)); diff --git a/server/src/routes/connection-management-authz.ts b/server/src/routes/connection-management-authz.ts index d747ebfb271..6c252e1a3cf 100644 --- a/server/src/routes/connection-management-authz.ts +++ b/server/src/routes/connection-management-authz.ts @@ -173,9 +173,9 @@ export function connectionManagementGuard(db: Db, purpose: "tool" | "chat" | "ai if (secret.deletedAt || secret.status !== "active" || (secret.scope === "user" && secret.ownerUserId !== userId)) throw forbidden("Credential binding is unavailable to the responsible user"); } if (options.slackManagerGrant) { - const [grant] = await db.select().from(chatSlackManagerGrants).where(eq(chatSlackManagerGrants.id, String(req.params.grantId))); + const [grant] = await db.select().from(chatSlackManagerGrants).where(eq(chatSlackManagerGrants.id, uuid(req.params.grantId ?? body.grantId))); bindCompany(grant); resource.provider = "slack"; - if (req.actor.type === "agent" && grant.userId !== userId) throw forbidden("Manager consent belongs to another user"); + if (grant.userId !== userId) throw forbidden("Manager consent belongs to another user"); } if (!options.slackManagerGrant && (req.params.grantId || body.grantId)) { const [grant] = await db.select().from(connectionGrants).where(eq(connectionGrants.id, uuid(req.params.grantId ?? body.grantId)));