diff --git a/CHANGELOG.md b/CHANGELOG.md index f058d01..68fe849 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,13 @@ product overview. ## Unreleased +- Clarify Composio file-staging identity arguments in every engine's per-run instructions: + use `"user"` for `composio-user` and `"agent"` for `composio-agent`. - Update the pinned Claude Code and Codex CLIs to 2.1.300 and 0.162.1. +- Update the gateway dependencies to Composio SDK 0.22.0, Express 5.3.0 and Acorn 8.19.0. + Download filenames retain their names and required escaping; Express now omits optional quotes + around simple attachment filenames. Regression checks cover the installed SDK's session + creation/resume contract and public downloads with spaces, quotes and Unicode names. - Google Chat card buttons now work: a Chat app connected through a Pub/Sub topic never receives button clicks (Google delivers them only to an HTTP endpoint connection), so Files / Variables / @@ -123,6 +129,11 @@ product overview. static IPv4) and four recommended EU hosts with estimated monthly prices — Hetzner CCX23 (≈ €25), netcup VPS 2000 G12 (≈ €16), RoLeaf in Romania (≈ €32) and Google Cloud `n4-standard-4` (≈ €139). +- **Custom MCP servers from Slack.** ⚙️ Settings → MCPs now lists the conversation's custom MCP + servers and your own, and lets you add, update or remove them in place. A conversation's servers + are editable by its managers and admins; your own are yours to manage. Tokens stay write-only. + Repeated edits reuse the Settings window; slow address/access checks acknowledge promptly, + time out safely and recheck current authority before saving. - Teams channel and group-chat answers now end with a native mention of the requester, matching Slack. Long answers tag them on the final chunk before the statistics footer; DMs skip the tag. diff --git a/FEATURES.md b/FEATURES.md index 1b8e524..167fbc5 100644 --- a/FEATURES.md +++ b/FEATURES.md @@ -210,6 +210,21 @@ control-plane status. (admin UI → conversation → Connections → *Custom MCP servers*) or to one person (Users → the person → *Personal MCP servers*). Each scope holds at most five. OAuth-only servers are out of scope and go through Composio. +- **In Slack too:** the ⚙️ Settings → **MCPs** page has a *Custom MCP servers* section with two + lists, masked like the admin UI (server name, URL and last four token characters): + - *This conversation* — rows can be added, updated and removed only by the channel's managers + and admins (`canManage`, re-checked on every click and submission). + - *Yours* — the viewer's own `my-` servers, which any authorized user manages for + themselves. + - *Add or update…* pushes a form (name, URL, optional token). It is never prefilled; a blank + token keeps the stored one for an existing name. Local field errors appear inline. Remote + address/membership checks acknowledge promptly and show progress in the existing Settings + page; a refusal offers a fresh form with the token draft cleared. Checks time out after + 15 seconds and cannot save later; current authority is rechecked before the write. + Successful saves reuse that page, so repeated edits do not consume Slack's modal stack. + - *Remove* asks for confirmation and takes the server out of live relay grants. Saves and + removals go through the same `custom-mcp-store.js` path as the admin UI. Audit rows name the + Slack user. The Teams settings dialog does not offer this yet. - Runs receive a conversation's servers as `custom-` and the AUTHOR's own as `my-`, in Claude and Codex turns (fresh, resumed, cross-engine fallback) and in SSH sessions. The prefixes keep them apart from each other, the built-in servers and Cloud MCP selections. Lean injects @@ -2609,7 +2624,8 @@ A categorized catalog of what's shipped. Cross-linked to `TEST-PLAN.md` checks. (`user` → `composio-user`, `agent` → `composio-agent`) and the key resolved for THAT identity is the one spent, through the same precedence the MCP config uses; a named identity with no key is reported rather than silently replaced by the other one. Every run that has a Composio identity - is also told this handoff rule in its per-run identity line (not only in the gateway-usage guide), + is also told this handoff rule and the exact `"user"`/`"agent"` argument mapping in its per-run + identity line (not only in the gateway-usage guide), so an engine that never opens the guide's sharing page still stages instead of relaying base64. The key never enters the container, never reaches the model, and never appears in an error message. Nothing is published. diff --git a/TEST-PLAN.md b/TEST-PLAN.md index cbc9304..ed9ba73 100644 --- a/TEST-PLAN.md +++ b/TEST-PLAN.md @@ -1,5 +1,41 @@ # ChannelGate — Test Plan +## Composio staging identity instructions (2026-10-14) + +The per-run identity preamble explicitly maps `composio-user` to the staging argument `"user"` +and `composio-agent` to `"agent"`, matching the existing tool schema and guide. The preamble +regression checks both mappings in every identity variant and retains the account authorization, +URL-only handoff and no-base64 rules; existing staging tests verify the selected key. +Live Claude/Codex Composio-script and file-staging cases still apply. An explanation-only live +failure is evidence of an incorrect proposed argument, not an executed staging failure. This +instruction clarification requires a fresh deployed retest before any live pass is claimed. + +## Compatible gateway dependency updates (2026-10-14) + +Composio core 0.22.0, Express 5.3.0 and Acorn 8.19.0 are ported to beta without release or +container image changes. The former dependency PR failed because its download assertion required +quotes around a valid HTTP token filename; Express's content-disposition 2 now omits those quotes. +`test/public-file-links.test.js` accepts either valid token representation and additionally checks +real downloads with spaces, escaped quotes and Unicode filenames. `test/composio-sdk.test.js` +uses the installed SDK with a mocked HTTP boundary to verify create/resume requests, identity, +connection-management restrictions and non-exposure of the daemon key to callers. + +Validation: clean `npm ci --ignore-scripts --no-fund` installs 189 packages with zero reported +vulnerabilities; `npm audit --omit=dev --audit-level=high`, static checking (874 JS files), secret +scan and `git diff --check` pass. On Node 22.23.2, the focused public-file-links, file-download, +file-editor, file-upload, web-security, admin-auth-bind, mcp-socket-server, composio-sdk, +composio-sdk-bridge, composio-sdk-run, composio-sdk-settings, composio-sessions and +composio-entitlement suites pass **93/93**, no skips. The two changed suites also pass **20/20**, +no skips, on Node **22.13.0** and **24.21.0**. + +Compatibility limitation: published npm metadata for both the previous Composio core 0.19.0 and +0.22.0 declares Node >=22.22.3, above the gateway's existing >=22.13 floor. The lower-floor result +above demonstrates only the exercised installed-SDK paths; it does not claim upstream support +for every SDK API or resolve that pre-existing metadata mismatch. The gateway floor is unchanged. +Full combined regression/coverage and CI remain integration checks. Existing live Composio +Claude/Codex session and public-link acceptance gates still apply; mocked HTTP is not live +provider acceptance. + ## Google Chat HTTP endpoint — working card buttons (2026-10-14) Root cause: a Chat app has one connection setting and Google delivers card/button clicks only to @@ -289,6 +325,78 @@ must run applicable gates before stable promotion; no live pass is implied by au - [x] Full combined regression/security coverage passed on a clean install of the patched lockfile, including remote MCP HTTP/stdio transports, OAuth metadata, and public-address guards. +## Custom MCP servers in Slack Settings (2026-10-14) + +### Regression +- [x] Automated: `node --test test/slack-custom-mcp-settings.test.js`. + - The MCPs page lists both scopes masked (last four token characters only). + - Conversation add/remove controls appear only with `canManageCustomMcp`; a member sees only + their own. + - Remove buttons confirm first. + - The form is never prefilled, its metadata carries the scope, and it parses only typed values. + - Refusals map to the right field. + - `channelSettingsEditOptions` follows `manageAccess`; admins always qualify. + - The personal save writes the viewer's own record without echoing the token. + - Refused: a loopback URL (safe refusal notice), a missing token (token field), a non-manager's + conversation save (nothing written) and a form owned by someone else. + - An admin's conversation save records `<@U…>` as `setBy`. Remove by a non-manager is refused; + removal by an admin, and of one's own server, succeeds. + - Repeated pushed-form saves reuse the original Settings parent. Four consecutive updates keep + a blank token intact; a failed parent refresh never acknowledges the same submission twice. + The regression failed against the initial implementation (it returned `response_action: + update` on the pushed form), then passed after using parent navigation. + - The real submission context refuses departed members, revoked admission, mismatched channel + slugs and revoked manager rights before a mutation. + - `channelSettingsEditOptions` includes custom-MCP authority in the settings modal regressions. + - Remote DNS/membership checks acknowledge before waiting; role loss during either check is + refused before the write. Both timeout paths clear the token draft, offer a new form and + remain unable to write after the delayed work eventually finishes. + - Native Add consumes its trigger with a safe placeholder before fetching membership, then + hydrates the same view. Add and Remove recheck local authority after delayed roster reads; + a manager revoked during the wait cannot open a form or remove a server. + - Focused validation: 79 tests passed across `slack-custom-mcp-settings`, + `channel-settings-modal`, `custom-mcps`, `file-form-stack` and `slack-secret-actions`. + These are isolated automated checks, not deployed Slack evidence. + +### Live gates — UNEXECUTED + +Fixture: one disposable Slack QA channel with a manager and an approved non-manager, a second +channel for isolation, and a disposable HTTPS Bearer MCP endpoint exposing a harmless `ping` +tool that returns a recognizable marker. Provision endpoint and tokens outside chat. Configure +channel management for admins/managers, give both people access, and use separate Claude and +Codex threads with Lean off. Record deployed commit/image, channel/thread identifiers privately, +action timestamps, redacted native UI captures and tool/result evidence; never record tokens. + +- [ ] `SLACK-CMCP-01` (engine-independent native UI): manager opens a reply's ⚙️ Settings → MCPs, + adds `qa` under *This conversation*, then updates the same name's public URL four consecutive + times with a blank token. Pass: every save returns to the same MCPs page without a stack-limit + error; only `custom-qa` exists, its token suffix stays unchanged, and the admin UI matches. + Rotate the token once; pass: only the new suffix is shown and no full token is returned. +- [ ] `SLACK-CMCP-02` (engine-independent native UI): approved non-manager opens Settings → MCPs. + Pass: conversation Add/Remove controls are absent; the user can add and update `qa` under + *Yours* as `my-qa`. Another member's Settings must not list this personal server. Revoke the + first user's membership with their form open and submit; pass: refusal, no mutation. +- [ ] `SLACK-CMCP-03` (engine-independent native UI): try `https://127.0.0.1/`, an HTTP URL, + invalid name and a new name without a token. Pass: local validation gives an appropriate field + error; the asynchronous private-address refusal gives a safe notice and a fresh-form retry, + without returning the token draft. No new row or successful-save audit appears. Open a manager's form, revoke management rights, then submit; pass: + refusal and unchanged channel list. Delay the fixture address or roster response past 15 seconds; + pass: prompt acknowledgement, timeout notice and no late write after the response completes. + Cancel a Remove confirmation; the server remains. +- [ ] `SLACK-CMCP-04-Claude` and `SLACK-CMCP-04-Codex`: for each engine, as the personal owner, + prompt "Call ping once on custom-qa and once on my-qa; report their markers without credentials." + Pass: both tools return the fixture marker with scope-prefixed server names. As the other + approved member, prompt "Call custom-qa ping; check whether my-qa is available without calling + another server." Pass: channel tool succeeds and owner's personal server is absent. In the + second channel without `custom-qa`, the owner's `my-qa` remains available and `custom-qa` is absent. +- [ ] `SLACK-CMCP-05-Claude` and `SLACK-CMCP-05-Codex`: pause a fixture turn before another ping, + remove the matching conversation and personal servers through confirmed native controls, + then continue. Pass: the live relay refuses the removed server on its next request; fresh turns + no longer list either tool. Collect native row disappearance plus relay refusal/tool inventory. + +These native UI actions and real engine turns are release gates. Mocked Slack transports, +Chromium rendering of a local fixture and successful code tests do not satisfy them. + ## Custom MCP connections (2026-10-01) ### Regression diff --git a/documentation/src/content/docs/controls/mcp-connections.md b/documentation/src/content/docs/controls/mcp-connections.md index 4ba2168..7518e5f 100644 --- a/documentation/src/content/docs/controls/mcp-connections.md +++ b/documentation/src/content/docs/controls/mcp-connections.md @@ -5,6 +5,31 @@ description: Discover and manage engine-specific optional tool servers in a conv These controls select servers already available through the active engine inventory. Configure server definitions and authentication using the supported native engine configuration and Cloud MCP assignment paths; there is no separate general-purpose MCP catalog editor in the admin UI. +## Custom servers in Slack Settings + +Open **⚙️ Settings** on a reply, choose **MCPs**, then find **Custom MCP servers**. + +- **This conversation** contains `custom-` servers shared by runs here. Channel managers + and administrators can add, update and remove them; authority is checked again when saving. +- **Yours** contains your personal `my-` servers. Only runs you start receive these, + including your runs in other conversations. Other people do not see your personal list. + +Choose **Add or update**, enter a name and public HTTPS server URL, then enter the Bearer token +in the private modal. For an existing name, leave the token blank to keep it or enter a new token +to replace it. Stored tokens are never prefilled or revealed; the list shows only their last four +characters. Each scope holds up to five servers. OAuth-only servers use Composio instead. + +Saving shows progress while the gateway checks the public address and your current access, then +returns to the existing MCPs page, including after repeated updates. Local field errors remain +inline. If a remote check fails or times out, **Try again** opens a fresh form; re-enter the token +because its draft was cleared. Timed-out checks do not save later. **Remove** asks for +confirmation and revokes that server's live relay access. New runs receive the saved list; Lean +runs receive no custom servers. Claude and Codex use the same scoped connections. + +Admins can also manage these servers in the admin UI under the conversation's **Connections** or +the person's **Personal MCP servers**. Custom servers are separate from the engine inventory and +Cloud MCP selection controls below. Never paste a token into chat or a project MCP settings file. + ## list_available_mcps Discover the current engine’s host-configured MCP inventory. diff --git a/package-lock.json b/package-lock.json index 04ab553..e2542c1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,15 +9,15 @@ "version": "0.6.3", "license": "SEE LICENSE IN LICENSE.md", "dependencies": { - "@composio/core": "0.19.0", + "@composio/core": "0.22.0", "@modelcontextprotocol/sdk": "^1.0.0", "@slack/bolt": "^5.1.0", - "express": "^5.1.0", + "express": "^5.3.0", "mcp-remote": "0.14.3", "zod": "^4.6.5" }, "devDependencies": { - "acorn": "8.18.0", + "acorn": "8.19.0", "acorn-globals": "7.0.1" }, "engines": { @@ -31,22 +31,25 @@ "license": "MIT" }, "node_modules/@composio/client": { - "version": "0.1.0-alpha.76", - "resolved": "https://registry.npmjs.org/@composio/client/-/client-0.1.0-alpha.76.tgz", - "integrity": "sha512-MXC5JGRVdiQ4EgLricy9o/mqBa1+1T7wHFZ6Q4ZJkrjzZqOMvxTgy21Zlb5J/1oGkB2bg9UDzpH8PkXCp/D4zA==", - "license": "Apache-2.0" + "version": "2.0.0-rc.8", + "resolved": "https://registry.npmjs.org/@composio/client/-/client-2.0.0-rc.8.tgz", + "integrity": "sha512-0C0Flmblx0Y3i6iGUY/MvST4aNXn3xEmPEA4wqZd3acm9x6u61/0i1LXCgBzFTTZEveK8YBFRu8Lpo/SMZwAFQ==", + "license": "Apache-2.0", + "engines": { + "node": ">=22" + } }, "node_modules/@composio/core": { - "version": "0.19.0", - "resolved": "https://registry.npmjs.org/@composio/core/-/core-0.19.0.tgz", - "integrity": "sha512-VmK4e8Kt+wW+xJUOkdytgjJdbLTmCXJZ303EJCp7ymwfojKYGfe9Kzz9z/HbhWTc5VKWUtHXzxTpWTbhHAnweA==", + "version": "0.22.0", + "resolved": "https://registry.npmjs.org/@composio/core/-/core-0.22.0.tgz", + "integrity": "sha512-Inc7/yqhRQTfvU3km/VwSpewwJ9O/5mnC2FjSPDp56MfEdUfx61rwoi6+pancTUhHsHic5qe0lB4C2rHd7iioA==", "license": "ISC", "dependencies": { - "@composio/client": "0.1.0-alpha.76", + "@composio/client": "2.0.0-rc.8", "@composio/json-schema-to-zod": "0.3.3", "@types/json-schema": "^7.0.15", "is-fs-case-sensitive": "^2.0.0", - "openai": "^7.15.0", + "openai": "^7.21.0", "picocolors": "^1.1.1", "pusher-js": "^8.6.0", "semver": "^7.8.5", @@ -376,9 +379,9 @@ } }, "node_modules/acorn": { - "version": "8.18.0", - "resolved": "https://registry.npmjs.org/acorn/-/acorn-8.18.0.tgz", - "integrity": "sha512-lGq+9yr1/GuAWaVYIHRjvvySG5/4VfKIvC8EWxStPdcDh/Ka7FG3twP6v4d5BkravUilhIAsG4Qj83t02LWUPQ==", + "version": "8.19.0", + "resolved": "https://registry.npmjs.org/acorn/-/acorn-8.19.0.tgz", + "integrity": "sha512-oJlA3XiRm7Cyk6qFD2Jn8ak9B7jSy0qy00ADO3+8dpT0LSjFihQYv4C02LFCSYJV3Q37xYwwRy5m+IpIiUzqWw==", "dev": true, "license": "MIT", "bin": { @@ -548,9 +551,9 @@ } }, "node_modules/content-disposition": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/content-disposition/-/content-disposition-1.1.0.tgz", - "integrity": "sha512-5jRCH9Z/+DRP7rkvY83B+yGIGX96OYdJmzngqnw2SBSxqCFPd0w2km3s5iawpGX8krnwSGmF0FW5Nhr0Hfai3g==", + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/content-disposition/-/content-disposition-2.0.1.tgz", + "integrity": "sha512-e+H0ZXHSWYrENhQzw1LPuP4oF5MzVKmDU6d3hxlvaPEYLLg62MxtQNPRx4SYSuYJSBUgnQIG4HIN2tEtNv7Dog==", "license": "MIT", "engines": { "node": ">=18" @@ -814,15 +817,15 @@ } }, "node_modules/express": { - "version": "5.2.1", - "resolved": "https://registry.npmjs.org/express/-/express-5.2.1.tgz", - "integrity": "sha512-hIS4idWWai69NezIdRt2xFVofaF4j+6INOpJlVOLDO8zXGpUVEVzIYk12UUi2JzjEzWL3IOAxcTubgz9Po0yXw==", + "version": "5.3.0", + "resolved": "https://registry.npmjs.org/express/-/express-5.3.0.tgz", + "integrity": "sha512-qCqy1xSoaugxOD4IVHb1ZWdkvCF2W3aE2okKouWj8VwJ+ztskSiXoBnCSJqfKT+cVPJhuJqe9YLbY425+Zh00g==", "license": "MIT", "dependencies": { "accepts": "^2.0.0", - "body-parser": "^2.2.1", - "content-disposition": "^1.0.0", - "content-type": "^1.0.5", + "body-parser": "^2.3.0", + "content-disposition": "^2.0.1", + "content-type": "^2.0.0", "cookie": "^0.7.1", "cookie-signature": "^1.2.1", "debug": "^4.4.0", @@ -838,14 +841,14 @@ "on-finished": "^2.4.1", "once": "^1.4.0", "parseurl": "^1.3.3", - "proxy-addr": "^2.0.7", - "qs": "^6.14.0", + "proxy-addr": "^2.0.8", + "qs": "^6.16.0", "range-parser": "^1.2.1", "router": "^2.2.0", "send": "^1.1.0", "serve-static": "^2.2.0", "statuses": "^2.0.1", - "type-is": "^2.0.1", + "type-is": "^2.1.0", "vary": "^1.1.2" }, "engines": { @@ -874,6 +877,19 @@ "express": ">= 4.11" } }, + "node_modules/express/node_modules/content-type": { + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/content-type/-/content-type-2.1.0.tgz", + "integrity": "sha512-mj7UPXE0jaqaOsukNZRUEfEi2AcL7C/vwmwcHV0O97eO1E1pxBZuyjlZrx5seTaNBg1U6+o35wpa35Qfcc+7ag==", + "license": "MIT", + "engines": { + "node": ">=18" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/express" + } + }, "node_modules/fast-deep-equal": { "version": "3.1.3", "resolved": "https://registry.npmjs.org/fast-deep-equal/-/fast-deep-equal-3.1.3.tgz", diff --git a/package.json b/package.json index 045a226..eee8c6f 100644 --- a/package.json +++ b/package.json @@ -53,15 +53,15 @@ "node": ">=22.13" }, "dependencies": { - "@composio/core": "0.19.0", + "@composio/core": "0.22.0", "@modelcontextprotocol/sdk": "^1.0.0", "@slack/bolt": "^5.1.0", - "express": "^5.1.0", + "express": "^5.3.0", "mcp-remote": "0.14.3", "zod": "^4.6.5" }, "devDependencies": { - "acorn": "8.18.0", + "acorn": "8.19.0", "acorn-globals": "7.0.1" }, "overrides": { diff --git a/src/gateway/custom-mcp-store.js b/src/gateway/custom-mcp-store.js index b5c99d6..cf86950 100644 --- a/src/gateway/custom-mcp-store.js +++ b/src/gateway/custom-mcp-store.js @@ -20,8 +20,10 @@ function hostOf(url) { } /** Add or update one of a channel's custom MCP servers. Returns the masked list. */ -export async function saveChannelCustomMcp(slug, { name, url, token } = {}, { actor = "", lookup } = {}) { +export async function saveChannelCustomMcp(slug, { name, url, token } = {}, { actor = "", lookup, authorize } = {}) { const checkedUrl = await assertPublicCustomMcpUrl(url, lookup ? { lookup } : {}); + // Interactive surfaces may need to re-check access after the asynchronous DNS check. + if (authorize) await authorize(); const saved = await patchChannelMeta(slug, (existing) => ({ customMcps: patchCustomMcps(existing?.customMcps, { set: { name, url: checkedUrl, token }, actor }), })); @@ -48,9 +50,11 @@ export async function listUserCustomMcps(userId) { } /** Add or update one of a person's own custom MCP servers. Returns the masked list. */ -export async function saveUserCustomMcp(userId, { name, url, token } = {}, { actor = "", lookup } = {}) { +export async function saveUserCustomMcp(userId, { name, url, token } = {}, { actor = "", lookup, authorize } = {}) { if (!userId) throw new Error("No user to change."); const checkedUrl = await assertPublicCustomMcpUrl(url, lookup ? { lookup } : {}); + // Interactive surfaces may need to re-check access after the asynchronous DNS check. + if (authorize) await authorize(); const next = patchCustomMcps((await getUser(userId))?.customMcps, { set: { name, url: checkedUrl, token }, actor: actor || userId }); const saved = await setUser(userId, { customMcps: next }); const stored = normalizeCustomMcpName(name); diff --git a/src/gateway/gateway-usage/references/administration.md b/src/gateway/gateway-usage/references/administration.md index e674e77..b7f156b 100644 --- a/src/gateway/gateway-usage/references/administration.md +++ b/src/gateway/gateway-usage/references/administration.md @@ -91,9 +91,10 @@ gateway applies them; explicitly exempt tools do not receive a human-approval re - `list_channel_mcps` — what's allowed here now. - `add_channel_mcps` / `remove_channel_mcps` — allow/stop MCP servers here (by name). - **Custom MCP servers** (a remote server by URL + Bearer token) are added by a person, never by - you: an admin opens the admin UI → the conversation → Connections → *Custom MCP servers* (they - reach every run here as `custom-`), or the user drawer → *Personal MCP servers* (only - that person's runs, as `my-`). HTTPS and public addresses only; OAuth-only servers go + you. In Slack: a reply's ⚙️ Settings → **MCPs** → *Custom MCP servers*. Channel managers and + admins edit *This conversation* (every run here gets them as `custom-`); anyone edits + *Yours* (only their own runs get them, as `my-`). Admins can also use the admin UI → the + conversation → Connections, or Users → the person. HTTPS and public addresses only; OAuth-only servers go through Composio instead. Never ask for the token in chat, and never write an MCP definition into a settings or `.mcp.json` file — the engine ignores them. diff --git a/src/gateway/gateway-usage/references/sharing-files.md b/src/gateway/gateway-usage/references/sharing-files.md index eabd77a..04e4daf 100644 --- a/src/gateway/gateway-usage/references/sharing-files.md +++ b/src/gateway/gateway-usage/references/sharing-files.md @@ -41,7 +41,9 @@ Then run the Composio tool and pass that object through **unchanged** as its fil Two things that will bite you if you skip them: -- **`identity` must match the identity you then execute with.** A file staged with +- **`identity` accepts only `"user"` or `"agent"`.** Use `"user"` for `composio-user` and + `"agent"` for `composio-agent`; the server names are not accepted argument values. + The value must match the identity you then execute with. A file staged with `composio-user`'s key is invisible to `composio-agent`, and vice versa. Decide whose account is doing the upload first (the rules are in the skill's "Tool identities" section — an upload is a write, so the account must be settled, not guessed), then stage on that one. diff --git a/src/gateway/mcp.js b/src/gateway/mcp.js index 890126a..850637e 100644 --- a/src/gateway/mcp.js +++ b/src/gateway/mcp.js @@ -122,7 +122,7 @@ export function composioIdentityPreamble({ user = false, agent = false } = {}) { // QA-0925 (FSHARE-02): the routing lived only in the gateway-usage guide, and a Codex turn that // skipped that page pushed a PDF through the workbench as base64 instead. Every run that has a // Composio identity needs the one handoff rule, whichever engine and whichever pages it reads. - const files = " To hand a file from this folder to a Composio tool that takes a file object (Drive upload, email attachment, …), call `gateway` → `stage_file_for_composio` with `identity` set to the identity that will run that tool and pass its result on unchanged; a tool that only ingests by URL takes a `create_public_file_link` upload link instead. Never push a file's bytes as base64 or chunks through the workbench or another Composio tool to get around staging, including when staging fails or the file is too large."; + const files = " To hand a file from this folder to a Composio tool that takes a file object (Drive upload, email attachment, …), call `gateway` → `stage_file_for_composio` with `identity: \"user\"` for tools run through `composio-user`, or `identity: \"agent\"` for `composio-agent`; these are the only accepted identity values. Pass its result on unchanged; a tool that only ingests by URL takes a `create_public_file_link` upload link instead. Never push a file's bytes as base64 or chunks through the workbench or another Composio tool to get around staging, including when staging fails or the file is too large."; if (user && agent) { return "[Composio identities in THIS run: `composio-user` (the requester's own accounts) and `composio-agent` (the shared agent's own). " + "Reads and searches may use either or both identities without asking which account unless the user restricts the account or scope. Writes, sends and other state changes require the intended identity and connected account: reuse an established choice, or ask \"which account?\" if unresolved before mutating; continue independent authorized reads." + ownership + files + "]\n\n"; diff --git a/src/slack/app.js b/src/slack/app.js index 21e201b..29e30bd 100644 --- a/src/slack/app.js +++ b/src/slack/app.js @@ -9,7 +9,7 @@ import { createFileFormNavigation } from "./file-form-navigation.js"; import pkg from "@slack/bolt"; const { App, LogLevel } = pkg; -import { upsertChannelEntry, getChannelEntry, getChannelMeta, saveChannelMeta, patchChannelMeta, defaultChannelMeta, getUsers, isAdmin, isApproved, listChannels, getComposioToken, getToolboxToken } from "../config/store.js"; +import { upsertChannelEntry, getChannelEntry, getChannelMeta, saveChannelMeta, patchChannelMeta, defaultChannelMeta, getUsers, getUser, isAdmin, isApproved, listChannels, getComposioToken, getToolboxToken } from "../config/store.js"; import { ensureChannelFolder, effectiveWorkDir } from "../gateway/folders.js"; import { effectiveMeta } from "../gateway/run.js"; import { modeLabel, isAuthorized, canManage, modeSettingsPatch } from "../gateway/modes.js"; @@ -72,6 +72,9 @@ import { CHANNEL_SETTINGS_AUTOMATION_TOGGLE_ACTION_ID, CHANNEL_SETTINGS_AUTOMATION_DELETE_ACTION_ID, escapeMrkdwn, CHANNEL_SETTINGS_CLEAR_MAKE_ACTION_ID, CHANNEL_SETTINGS_CLEAR_TOOLBOX_ACTION_ID, CHANNEL_SETTINGS_CLOUD_ENGINE_PREFIX, CHANNEL_SETTINGS_CLOUD_MANAGE_ACTION_ID, + CHANNEL_SETTINGS_CUSTOM_MCP_ADD_ACTION_ID, CHANNEL_SETTINGS_CUSTOM_MCP_REMOVE_ACTION_ID, CHANNEL_SETTINGS_CUSTOM_MCP_CALLBACK_ID, + CUSTOM_MCP_NAME_BLOCK_ID, CUSTOM_MCP_URL_BLOCK_ID, CUSTOM_MCP_TOKEN_BLOCK_ID, + buildCustomMcpEditorView, buildCustomMcpStatusView, readCustomMcpForm, customMcpScope, CHANNEL_SETTINGS_CLOUD_PAGE_PREFIX, CHANNEL_SETTINGS_CLOUD_TOGGLE_PREFIX, CHANNEL_SETTINGS_CONNECTIONS_CALLBACK_ID, CHANNEL_SETTINGS_CONNECTIONS_EDIT_ACTION_ID, CHANNEL_SETTINGS_FALLBACK_ACTION_ID, CHANNEL_SETTINGS_THREAD_RESET_ACTION_ID, @@ -83,6 +86,8 @@ import { CONNECTION_COMPOSIO_BLOCK_ID, CONNECTION_MAKE_KEY_BLOCK_ID, CONNECTION_MAKE_URL_BLOCK_ID, CONNECTION_TOOLBOX_BLOCK_ID, TEMPLATE_BLOCK_ID, } from "./channel-settings.js"; +import { listCustomMcps, patchCustomMcps } from "../gateway/custom-mcps.js"; +import { listUserCustomMcps, removeChannelCustomMcp, removeUserCustomMcp, saveChannelCustomMcp, saveUserCustomMcp } from "../gateway/custom-mcp-store.js"; import { ACCESS_EDIT_ACTION_ID, ACCESS_CALLBACK_ID, accessFieldTarget, accessSettingsPatch, accessSettingsSnapshot, assertAccessManager, readAccessFieldValue } from "./access-settings.js"; import { assertValidEnvName, assertValidEnvValue, listChannelEnv, patchChannelEnv } from "../config/channel-env.js"; import { assertValidSwapRuleFields } from "../gateway/egress/catalog-rules.js"; @@ -591,6 +596,154 @@ export async function channelSettingsContext(client, { channelId, userId, expect return ctx; } +// A conversation's custom MCP servers reach every run in it: only its managers and admins change +// them (the same authority as its access settings). A person's own need no check beyond owning the +// modal, which channelSettingsContext and the owner match already did. +function assertCustomMcpManager(meta, who) { + if (!canManage(meta, who)) throw new Error("Only this channel's managers or an admin can change its custom MCP servers."); +} + +// Which form field a custom MCP refusal belongs under (src/gateway/custom-mcps.js messages). +export function customMcpErrorBlock(message = "") { + const text = String(message); + if (/^Token|Bearer token/.test(text)) return CUSTOM_MCP_TOKEN_BLOCK_ID; + if (/^URL/.test(text)) return CUSTOM_MCP_URL_BLOCK_ID; + return CUSTOM_MCP_NAME_BLOCK_ID; +} + +function inlineMcpName(name) { + return `\`${String(name || "").replaceAll("`", "'")}\``; +} + +// One MCPs-page custom MCP button, after channelSettingsContext admitted the clicker: "Add or +// update" answers the form to push; "Remove" removes and answers the page notice plus fresh meta. +export async function applyCustomMcpAction({ actionId, command = {}, entry, meta, state, clicker, userIsAdmin = false, userIsApproved = false }) { + const scope = customMcpScope(command.s); + if (!scope || ![CHANNEL_SETTINGS_CUSTOM_MCP_ADD_ACTION_ID, CHANNEL_SETTINGS_CUSTOM_MCP_REMOVE_ACTION_ID].includes(actionId)) throw new Error("This custom MCP control expired. Open Settings again."); + if (scope === "channel") assertCustomMcpManager(meta, { authorId: clicker, isAdminUser: userIsAdmin, isApprovedUser: userIsApproved }); + if (actionId === CHANNEL_SETTINGS_CUSTOM_MCP_ADD_ACTION_ID) { + return { + view: buildCustomMcpEditorView(state, { + scope, + channelName: entry.name, + existing: scope === "channel" ? listCustomMcps(meta.customMcps, "channel") : await listUserCustomMcps(clicker), + }), + }; + } + const name = String(command.n || ""); + if (scope === "channel") await removeChannelCustomMcp(entry.slug, name, { actor: `<@${clicker}>` }); + else await removeUserCustomMcp(clicker, name, { actor: clicker }); + return { + meta: (await getChannelMeta(entry.slug)) || meta, + notice: `🗑️ Removed ${inlineMcpName(`${scope === "channel" ? "custom" : "my"}-${name}`)}. Runs stop receiving it, including any in progress.`, + }; +} + +// Consume Add's short-lived trigger before calling Slack for current membership. Only a safe, +// owner-bound placeholder is pushed until both membership and fresh local authority admit it. +export async function handleCustomMcpSettingsAction({ ack, body, action, client }, { + context = channelSettingsContext, rootView = settingsRootView, +} = {}) { + await ack(); + const clicker = body?.user?.id; + const actionId = action?.action_id; + const command = settingsCommand(action); + let target = body?.view; + try { + const state = parseSettingsMetadata(body?.view?.private_metadata); + if (!clicker || state.ownerId !== clicker) throw new Error("This channel settings view isn't yours. Open your own from a recent reply."); + if (![CHANNEL_SETTINGS_CUSTOM_MCP_ADD_ACTION_ID, CHANNEL_SETTINGS_CUSTOM_MCP_REMOVE_ACTION_ID].includes(actionId)) throw new Error("This custom MCP control expired. Open Settings again."); + const scope = customMcpScope(command.s); + if (!scope) throw new Error("This custom MCP control expired. Open Settings again."); + const request = { channelId: state.channelId, userId: clicker, expectedSlug: state.slug }; + if (actionId === CHANNEL_SETTINGS_CUSTOM_MCP_ADD_ACTION_ID) { + const local = await context(client, request); + if (scope === "channel") assertCustomMcpManager(local.meta, { authorId: clicker, isAdminUser: local.userIsAdmin, isApprovedUser: local.userIsApproved }); + if (!body?.trigger_id) throw new Error("Slack didn't provide a trigger for this editor. Reopen Settings and try again."); + const opened = await client.views.push({ trigger_id: body.trigger_id, view: buildCustomMcpStatusView(state, { scope }) }); + if (!opened?.view?.id) throw new Error("Slack did not return the editor identity. Reopen Settings and try again."); + target = opened.view; + } + await context(client, { ...request, verifyMembership: true }); + const fresh = await context(client, { ...request, verifyMembership: false }); + const result = await applyCustomMcpAction({ actionId, command, ...fresh, state, clicker }); + await client.views.update({ + view_id: target.id, + ...(result.view && target.hash ? { hash: target.hash } : {}), + view: result.view || await rootView(fresh.entry, result.meta, { ...state, tab: "mcp" }, fresh.userIsAdmin, { tab: "mcp", notice: result.notice }), + }); + } catch (error) { + if (target?.id) await client.views.update({ view_id: target.id, view: buildChannelSettingsErrorView(error.message || "Couldn't update this server. Reopen Settings to check it.") }); + } +} + +// Validate local fields inline, then pop the secret-bearing form before remote membership/DNS +// work. The existing parent receives progress/result/error, never another retained stack layer. +export async function handleCustomMcpFormSubmission({ ack, body, view, client }, { + context = channelSettingsContext, rootView = settingsRootView, lookup, timeoutMs = 15_000, +} = {}) { + const navigation = createFileFormNavigation({ ack, client, view }); + const clicker = body?.user?.id; + let state, scope, saved = false, timer; + try { + state = parseEditorMetadata(view?.private_metadata); + scope = state.view === "custom_mcp_c" ? "channel" : state.view === "custom_mcp_p" ? "user" : ""; + if (!clicker || state.ownerId !== clicker || !scope) throw new Error("This form expired. Open Settings again."); + const request = { channelId: state.channelId, userId: clicker, expectedSlug: state.slug }; + const admit = ({ meta, userIsAdmin, userIsApproved }) => { + if (scope === "channel") assertCustomMcpManager(meta, { authorId: clicker, isAdminUser: userIsAdmin, isApprovedUser: userIsApproved }); + }; + // No remote roster lookup yet: this checks current local admission, scope and field syntax. + let current = await context(client, { ...request, verifyMembership: false }); + admit(current); + const form = readCustomMcpForm(view); + patchCustomMcps(scope === "channel" ? current.meta.customMcps : (await getUser(clicker))?.customMcps, { set: form }); + + await navigation.show(buildCustomMcpStatusView(state, { scope })); + let expired = false; + const timeoutError = () => new Error("The access or address check timed out. No server was saved; try again."); + const assertTimely = () => { if (expired) throw timeoutError(); }; + const authorize = async () => { + assertTimely(); + // DNS and roster calls can be slow. Re-read local authority after BOTH have finished so a + // removed manager or revoked user cannot use an already-open form to commit a late write. + await context(client, { ...request, verifyMembership: true }); + assertTimely(); + current = await context(client, { ...request, verifyMembership: false }); + admit(current); + assertTimely(); + }; + const options = { actor: scope === "channel" ? `<@${clicker}>` : clicker, lookup, authorize }; + const operation = scope === "channel" + ? saveChannelCustomMcp(current.entry.slug, form, options) + : saveUserCustomMcp(clicker, form, options); + const servers = await Promise.race([operation, new Promise((_, reject) => { + timer = setTimeout(() => { expired = true; reject(timeoutError()); }, timeoutMs); + })]); + clearTimeout(timer); + saved = true; + const server = servers.find((entry) => entry.name === String(form.name).trim().toLowerCase()); + const fresh = (await getChannelMeta(current.entry.slug)) || current.meta; + await navigation.show(await rootView(current.entry, fresh, { ...state, tab: "mcp" }, current.userIsAdmin, { + tab: "mcp", + notice: `✅ Saved ${inlineMcpName(server?.serverName || form.name)}. Its tools reach the next run.`, + })); + } catch (error) { + const message = String(error.message || "Couldn't save that server.").slice(0, 300); + if (navigation.acknowledged) { + await navigation.show(buildCustomMcpStatusView(state, { + scope, retry: !saved, + message: saved ? "The server was saved, but Settings could not refresh. Reopen Settings to check it." + : `${message} Reopen the form to retry; the token draft has been cleared.`, + })); + return; + } + await ack({ response_action: "errors", errors: { [customMcpErrorBlock(message)]: message.slice(0, 150) } }); + } finally { + clearTimeout(timer); + } +} + function channelSettingsSnapshot(meta = {}) { const effective = effectiveMeta(meta); const channelLogin = effective.codexAuthSource === "channel"; @@ -657,6 +810,9 @@ export function channelSettingsEditOptions(meta, userIsAdmin, { authorId = "", i canEditOrgSecrets: Boolean(userIsAdmin), canManageCloudMcp: Boolean(userIsAdmin), canManageVpn: canManage(meta, { authorId, isAdminUser: userIsAdmin, isApprovedUser }), + // The conversation's custom MCP servers reach every run here, so they are a manager's call; + // a person's own (`my-`) are always theirs to edit. + canManageCustomMcp: canManage(meta, { authorId, isAdminUser: userIsAdmin, isApprovedUser }), canEditAccess: !meta.isDM && canManage(meta, { authorId, isAdminUser: userIsAdmin, isApprovedUser }), }; } @@ -969,6 +1125,11 @@ async function settingsRootView(entry, meta, state, userIsAdmin, { tab = state.t snapshot.automations = listForChannel(state.channelId); snapshot.orgSecrets = secretScopes.organization; snapshot.personalSecrets = secretScopes.personal; + // Masked only (name, server name, URL, last4). `personal` is the VIEWER's own, like the secrets. + snapshot.customMcps = { + channel: listCustomMcps(meta.customMcps, "channel"), + personal: await listUserCustomMcps(state.ownerId).catch(() => []), + }; return buildChannelSettingsView({ ...snapshot, vpn: vpn ?? unconfiguredChannelVpnStatus(meta) }, { ...state, tab }, { channelName: entry.name, tab, @@ -1436,6 +1597,10 @@ async function connectAndWire(app) { await handleChannelVpnSettingsAction({ ack, body, action, client }); return; } + if ([CHANNEL_SETTINGS_CUSTOM_MCP_ADD_ACTION_ID, CHANNEL_SETTINGS_CUSTOM_MCP_REMOVE_ACTION_ID].includes(action?.action_id)) { + await handleCustomMcpSettingsAction({ ack, body, action, client }); + return; + } await ack(); const clicker = body?.user?.id; const command = settingsCommand(action); @@ -1778,6 +1943,8 @@ async function connectAndWire(app) { } }); + app.view(CHANNEL_SETTINGS_CUSTOM_MCP_CALLBACK_ID, (args) => handleCustomMcpFormSubmission(args)); + app.view(CHANNEL_SETTINGS_TEMPLATE_CALLBACK_ID, async ({ ack, body, view, client }) => { const clicker = body?.user?.id; let state; diff --git a/src/slack/channel-settings.js b/src/slack/channel-settings.js index 8f2c3f0..507342b 100644 --- a/src/slack/channel-settings.js +++ b/src/slack/channel-settings.js @@ -34,6 +34,17 @@ export const CHANNEL_SETTINGS_CLEAR_COMPOSIO_ACTION_ID = "cg_channel_settings_cl export const CHANNEL_SETTINGS_CLEAR_TOOLBOX_ACTION_ID = "cg_channel_settings_clear_toolbox"; export const CHANNEL_SETTINGS_CLEAR_MAKE_ACTION_ID = "cg_channel_settings_clear_make"; export const CHANNEL_SETTINGS_CLOUD_MANAGE_ACTION_ID = "cg_channel_settings_cloud_manage"; +// Custom MCP servers (src/gateway/custom-mcps.js) on the MCPs page: one pushed form adds or updates +// a server in either scope — the conversation's (`custom-`, managers and admins) or the +// clicker's own (`my-`) — and a Remove button per row. The scope rides in the button value +// and the form's metadata; the handler re-checks who may write it. +export const CHANNEL_SETTINGS_CUSTOM_MCP_ADD_ACTION_ID = "cg_channel_settings_custom_mcp_add"; +export const CHANNEL_SETTINGS_CUSTOM_MCP_REMOVE_ACTION_ID = "cg_channel_settings_custom_mcp_remove"; +export const CHANNEL_SETTINGS_CUSTOM_MCP_CALLBACK_ID = "cg_channel_settings_custom_mcp_form"; +export const CUSTOM_MCP_NAME_BLOCK_ID = "cg_custom_mcp_name"; +export const CUSTOM_MCP_URL_BLOCK_ID = "cg_custom_mcp_url"; +export const CUSTOM_MCP_TOKEN_BLOCK_ID = "cg_custom_mcp_token"; +const CUSTOM_MCP_INPUT_ACTION_ID = "value"; export const CHANNEL_SETTINGS_CLOUD_ENGINE_PREFIX = "cg_channel_settings_cloud_engine_"; export const CHANNEL_SETTINGS_CLOUD_TOGGLE_PREFIX = "cg_channel_settings_cloud_toggle_"; export const CHANNEL_SETTINGS_CLOUD_PAGE_PREFIX = "cg_channel_settings_cloud_page_"; @@ -305,7 +316,48 @@ function runtimeBlocks(snapshot = {}, state = {}, { canEditRuntime = true, canEn return blocks; } -function mcpBlocks(snapshot = {}, state = {}, { canManageCloudMcp = false } = {}) { +// A custom MCP scope as the page shows it: `c` the conversation's, `p` the viewer's own. +export function customMcpScope(letter) { + return letter === "c" ? "channel" : letter === "p" ? "user" : ""; +} + +function customMcpRows(list, state, letter, canEdit) { + const rows = (Array.isArray(list) ? list : []).map((entry) => { + const section = { + type: "section", + text: mrkdwn(`${inlineCode(entry.serverName)} ${escapeMrkdwn(String(entry.url || "").slice(0, 200))}\nBearer ${inlineCode(`••••${entry.tokenLast4 || ""}`)}`), + }; + if (canEdit) { + section.accessory = button(CHANNEL_SETTINGS_CUSTOM_MCP_REMOVE_ACTION_ID, "Remove", state, "custom_mcp_remove", { s: letter, n: entry.name }, { + style: "danger", + confirm: destructiveConfirm(`Remove ${entry.serverName}?`, letter === "c" + ? "Runs in this conversation stop receiving this server, including any run in progress. The token can't be recovered." + : "Your runs stop receiving this server everywhere, including any run in progress. The token can't be recovered."), + }); + } + return section; + }); + return rows.length ? rows : [{ type: "context", elements: [mrkdwn("_none_")] }]; +} + +function customMcpBlocks(snapshot = {}, state = {}, { canManageCustomMcp = false } = {}) { + const custom = snapshot.customMcps || {}; + return [ + { type: "divider" }, + { type: "header", text: plain("Custom MCP servers") }, + { type: "context", elements: [mrkdwn("A remote MCP server added by HTTPS URL + Bearer token (servers that need an OAuth sign-in go through Composio). The gateway connects on the agent's behalf, so the token never enters the container. Tokens are write-only.")] }, + fieldBlock("This conversation", "Every run here receives these as `custom-`."), + ...customMcpRows(custom.channel, state, "c", canManageCustomMcp), + canManageCustomMcp + ? { type: "actions", elements: [button(CHANNEL_SETTINGS_CUSTOM_MCP_ADD_ACTION_ID, "Add or update a conversation server", state, "custom_mcp_add", { s: "c" })] } + : { type: "context", elements: [mrkdwn("_Only this channel's managers and admins can change the conversation's servers._")] }, + fieldBlock("Yours", "Only runs *you* start receive these, as `my-`, in every conversation you talk in."), + ...customMcpRows(custom.personal, state, "p", true), + { type: "actions", elements: [button(CHANNEL_SETTINGS_CUSTOM_MCP_ADD_ACTION_ID, "Add or update my server", state, "custom_mcp_add", { s: "p" })] }, + ]; +} + +function mcpBlocks(snapshot = {}, state = {}, { canManageCloudMcp = false, canManageCustomMcp = false } = {}) { const c = snapshot.connections || {}; const cloud = snapshot.cloudMcp || {}; const composio = c.composioMode === "sdk" @@ -360,6 +412,7 @@ function mcpBlocks(snapshot = {}, state = {}, { canManageCloudMcp = false } = {} ], }, ...(remove.length ? [{ type: "actions", elements: remove }] : []), + ...customMcpBlocks(snapshot, state, { canManageCustomMcp }), ...(canManageCloudMcp ? [ { type: "divider" }, { type: "header", text: plain("Cloud MCP") }, @@ -371,7 +424,7 @@ function mcpBlocks(snapshot = {}, state = {}, { canManageCloudMcp = false } = {} type: "actions", elements: [button(CHANNEL_SETTINGS_CLOUD_MANAGE_ACTION_ID, "Manage Cloud MCP", state, "cloud_manage", {}, { style: "primary" })], }, - { type: "context", elements: [mrkdwn("Gateway, Composio, Toolbox, and Make toolbox are injected separately from the Cloud MCP picker.")] }, + { type: "context", elements: [mrkdwn("Gateway, Composio, Toolbox, Make toolbox and custom MCP servers are injected separately from the Cloud MCP picker.")] }, ] : []), ]; } @@ -710,6 +763,7 @@ export function buildChannelSettingsView(snapshot = {}, state = {}, { // may edit this channel's own secrets. canEditOrgSecrets = false, canManageCloudMcp = false, + canManageCustomMcp = false, canEditAccess = false, canManageVpn = false, notice = "", @@ -720,7 +774,7 @@ export function buildChannelSettingsView(snapshot = {}, state = {}, { : active === "automations" ? automationsBlocks(snapshot, state) : active === "mcp" - ? mcpBlocks(snapshot, state, { canManageCloudMcp }) + ? mcpBlocks(snapshot, state, { canManageCloudMcp, canManageCustomMcp }) : active === "skills" ? skillsBlocks(snapshot, state) : active === "secrets" @@ -733,7 +787,7 @@ export function buildChannelSettingsView(snapshot = {}, state = {}, { title: plain("Channel settings"), close: plain("Done"), blocks: [ - { type: "context", elements: [mrkdwn(`Settings for *#${escapeMrkdwn(channelName || "this channel")}*. Anyone authorized to use the agent here can edit these settings. Access settings and VPN controls require a channel manager or admin. Cloud MCP is admin-only.`)] }, + { type: "context", elements: [mrkdwn(`Settings for *#${escapeMrkdwn(channelName || "this channel")}*. Anyone authorized to use the agent here can edit these settings. Access settings, VPN controls and this conversation's custom MCP servers require a channel manager or admin. Cloud MCP is admin-only.`)] }, ...(notice ? [{ type: "section", text: mrkdwn(notice) }] : []), tabRow(state, active), { type: "divider" }, @@ -849,6 +903,72 @@ export function buildConnectionsEditorView(connections = {}, state = {}, { chann }; } +// The pushed form behind "Add or update …" on the MCPs page. Saving an existing name with the token +// left blank keeps the stored token (a URL change); a new name needs one. +export function buildCustomMcpEditorView(state = {}, { scope = "user", channelName = "", existing = [] } = {}) { + const channel = scope === "channel"; + const names = (Array.isArray(existing) ? existing : []).map((entry) => entry.name).filter(Boolean); + return { + type: "modal", + callback_id: CHANNEL_SETTINGS_CUSTOM_MCP_CALLBACK_ID, + private_metadata: editorMetadata(state, { view: channel ? "custom_mcp_c" : "custom_mcp_p" }), + title: plain(channel ? "Conversation MCP server" : "My MCP server"), + submit: plain("Save"), + close: plain("Cancel"), + blocks: [ + { type: "context", elements: [mrkdwn(channel + ? `Every run in *#${escapeMrkdwn(channelName || "this channel")}* receives this server as \`custom-\`.` + : "Only runs you start receive this server, as `my-`, in every conversation you talk in.")] }, + { + type: "input", + block_id: CUSTOM_MCP_NAME_BLOCK_ID, + label: plain("Name"), + element: { type: "plain_text_input", action_id: CUSTOM_MCP_INPUT_ACTION_ID, placeholder: plain("e.g. linear"), max_length: 32 }, + hint: plain(names.length + ? `Lowercase letters, digits and dashes. Saved here: ${names.join(", ").slice(0, 120)} — reuse a name to update it.` + : "Lowercase letters, digits and dashes."), + }, + { + type: "input", + block_id: CUSTOM_MCP_URL_BLOCK_ID, + label: plain("Server URL"), + element: { type: "plain_text_input", action_id: CUSTOM_MCP_INPUT_ACTION_ID, placeholder: plain("https://mcp.example.com/mcp"), max_length: 2048 }, + hint: plain("HTTPS, on a public address."), + }, + optionalTextInput({ + blockId: CUSTOM_MCP_TOKEN_BLOCK_ID, + actionId: CUSTOM_MCP_INPUT_ACTION_ID, + label: "Bearer token", + placeholder: "Required for a new server", + maxLength: 3000, + hint: "Stored write-only and never shown again. Leave blank when updating an existing server to keep its token.", + }), + ], + }; +} + +// A submitted secret-bearing form is popped before remote validation. This safe status view +// occupies its existing parent; retry opens a fresh blank form rather than replaying a token. +export function buildCustomMcpStatusView(state, { scope, message, retry = false } = {}) { + return { + type: "modal", + callback_id: "cg_channel_settings_modal", + private_metadata: settingsMetadata({ ...state, tab: "mcp" }), + title: plain("Channel settings"), + close: plain("Done"), + blocks: [ + { type: "section", text: mrkdwn(escapeMrkdwn(message || "Checking access and the server address…")) }, + ...(retry ? [{ type: "actions", elements: [button(CHANNEL_SETTINGS_CUSTOM_MCP_ADD_ACTION_ID, "Try again", state, "custom_mcp_add", { s: scope === "channel" ? "c" : "p" })] }] : []), + ], + }; +} + +export function readCustomMcpForm(view = {}) { + const values = view?.state?.values || {}; + const text = (blockId) => String(values[blockId]?.[CUSTOM_MCP_INPUT_ACTION_ID]?.value || "").trim(); + return { name: text(CUSTOM_MCP_NAME_BLOCK_ID), url: text(CUSTOM_MCP_URL_BLOCK_ID), token: text(CUSTOM_MCP_TOKEN_BLOCK_ID) }; +} + export function readConnectionsForm(view = {}) { const values = view?.state?.values || {}; const textValue = (blockId, actionId) => String(values[blockId]?.[actionId]?.value || "").trim(); diff --git a/test/channel-settings-modal.test.js b/test/channel-settings-modal.test.js index 5e7f364..822fb0a 100644 --- a/test/channel-settings-modal.test.js +++ b/test/channel-settings-modal.test.js @@ -560,7 +560,8 @@ test("Settings and secrets admit authorized members and guests, but Cloud MCP re // A non-admin may edit this channel's own secrets in every mode, but never the organization's: // that scope reaches every conversation in the deployment. assert.deepEqual(channelSettingsEditOptions({ ...base, ...flags }, false), { - canEnableAdmin: false, canEditRuntime: true, canEditSecrets: true, canEditOrgSecrets: false, canManageCloudMcp: false, canManageVpn: false, canEditAccess: false, + canEnableAdmin: false, canEditRuntime: true, canEditSecrets: true, canEditOrgSecrets: false, canManageCloudMcp: false, canManageVpn: false, + canManageCustomMcp: false, canEditAccess: false, }); assert.equal(channelSettingsEditOptions({ ...base, ...flags }, true).canEditOrgSecrets, true); } diff --git a/test/composio-identity-preamble.test.js b/test/composio-identity-preamble.test.js index 6e47f8a..0a11b85 100644 --- a/test/composio-identity-preamble.test.js +++ b/test/composio-identity-preamble.test.js @@ -63,7 +63,9 @@ test("every identity variant routes a file handoff through the gateway's staging // through the Composio workbench. The rule has to ride the per-run line every engine reads. for (const ids of [{ user: true, agent: true }, { user: true }, { agent: true }]) { const line = composioIdentityPreamble(ids); - assert.match(line, /`gateway` → `stage_file_for_composio` with `identity` set to the identity that will run that tool/); + assert.match(line, /`gateway` → `stage_file_for_composio` with `identity: "user"` for tools run through `composio-user`/); + assert.match(line, /`identity: "agent"` for `composio-agent`; these are the only accepted identity values/); + assert.doesNotMatch(line, /identity: "composio-(?:user|agent)"/); assert.match(line, /only ingests by URL takes a `create_public_file_link` upload link instead/); assert.match(line, /Never push a file's bytes as base64 or chunks through the workbench or another Composio tool to get around staging/); assert.equal(line.trim().split("\n").length, 1, "still one line"); diff --git a/test/composio-sdk.test.js b/test/composio-sdk.test.js index 25892a2..d1f1c1c 100644 --- a/test/composio-sdk.test.js +++ b/test/composio-sdk.test.js @@ -1,5 +1,6 @@ import test from "node:test"; import assert from "node:assert/strict"; +import { Composio } from "@composio/core"; import { ensureTestEnv, clearTestLicense, testLicenseEnv } from "./helpers.js"; ensureTestEnv(); @@ -31,6 +32,44 @@ function fakeClient({ create, use } = {}) { }; } +test("installed SDK creates and resumes gateway sessions with the correct identity and no key exposure", async (t) => { + const calls = []; + t.mock.method(globalThis, "fetch", async (input, init) => { + const request = new Request(input, init); + if (request.url === "https://registry.npmjs.org/@composio/core/latest") { + assert.equal(request.headers.get("x-api-key"), null); + return Response.json({ version: "0.22.0" }); + } + assert.equal(new URL(request.url).origin, "https://backend.composio.dev"); + const body = request.method === "POST" ? await request.json() : null; + calls.push({ url: request.url, method: request.method, body, key: request.headers.get("x-api-key") }); + return Response.json({ + session_id: "trs_sdk_contract", + mcp: { type: "http", url: "https://backend.composio.dev/tool_router/v3/trs_sdk_contract/mcp" }, + config: { user_id: body?.user_id || "test-user", manage_connections: { enable: false } }, + }); + }); + const apiKey = "ak_sdk_contract_fixture"; + const client = new Composio({ apiKey, userApiKey: null, orgApiKey: null, allowTracking: false }); + const options = { + workspaceId: "TSDKCONTRACT", kind: "user", id: "UCONTRACT", threadKey: "installed-sdk", + accessKind: "owner", manageConnections: false, client, + }; + const created = await resolveSdkSession(options); + const resumed = await resolveSdkSession(options); + assert.deepEqual(resumed, created); + assert.equal(created.url, "https://backend.composio.dev/tool_router/v3/trs_sdk_contract/mcp"); + assert.equal(created.source, "sdk-user"); + assert.equal(JSON.stringify(created).includes(apiKey), false); + assert.equal(calls.length, 2); + assert.equal(calls[0].method, "POST"); + assert.equal(calls[0].body.user_id, composioIdentity(options)); + assert.equal(calls[0].body.manage_connections.enable, false); + assert.equal(calls[1].method, "GET"); + assert.ok(calls[1].url.endsWith("/tool_router/session/trs_sdk_contract")); + assert.ok(calls.every((call) => call.key === apiKey)); +}); + test("personal and shared sessions use stable identities with explicit management rights", async () => { const calls = []; const client = fakeClient({ diff --git a/test/public-file-links.test.js b/test/public-file-links.test.js index cd6f799..d6707c5 100644 --- a/test/public-file-links.test.js +++ b/test/public-file-links.test.js @@ -186,7 +186,8 @@ test("a live token streams the file once, audits the fetch and spends one downlo assert.equal(res.headers.get("cache-control"), "no-store, max-age=0"); assert.equal(res.headers.get("x-content-type-options"), "nosniff"); assert.equal(res.headers.get("x-robots-tag"), "noindex, nofollow"); - assert.match(res.headers.get("content-disposition"), /attachment; filename="PROPOSAL\.pdf"/); + // HTTP token filenames may be quoted or bare (content-disposition 2 uses the latter). + assert.match(res.headers.get("content-disposition"), /^attachment; filename=(?:"PROPOSAL\.pdf"|PROPOSAL\.pdf)$/); assert.equal(await res.text(), "%PDF-1.7 proposal\n"); assert.equal(audits.length, 1); @@ -200,6 +201,23 @@ test("a live token streams the file once, audits the fetch and spends one downlo assert.equal(getDb().prepare("SELECT downloads FROM public_file_links WHERE id = ?").get(link.id).downloads, 1); }); +test("download filenames retain required quoting and Unicode encoding", async (t) => { + const root = await scratchRoot(t); + const { base } = await serve(t, root); + for (const [filename, expected] of [ + ["Quarterly report.pdf", 'attachment; filename="Quarterly report.pdf"'], + ["report \"final\".pdf", 'attachment; filename="report \\"final\\".pdf"'], + ["report-\u20ac.pdf", "attachment; filename=\"report-?.pdf\"; filename*=UTF-8''report-%E2%82%AC.pdf"], + ]) { + await writeFile(path.join(root, "work", filename), "%PDF-1.7 proposal\n"); + const link = mint({ relative: `work/${filename}`, filename }); + const res = await fetch(`${base}/f/${link.token}`); + assert.equal(res.status, 200); + assert.equal(res.headers.get("content-disposition"), expected); + assert.equal(await res.text(), "%PDF-1.7 proposal\n"); + } +}); + test("a HEAD probe verifies the file without spending a download", async (t) => { const root = await scratchRoot(t); const { base, audits } = await serve(t, root); diff --git a/test/slack-custom-mcp-settings.test.js b/test/slack-custom-mcp-settings.test.js new file mode 100644 index 0000000..a89b9bf --- /dev/null +++ b/test/slack-custom-mcp-settings.test.js @@ -0,0 +1,382 @@ +// Custom MCP servers on the Slack Settings → MCPs page (src/slack/channel-settings.js + app.js): +// the conversation's servers (`custom-`, managers and admins) and the viewer's own +// (`my-`), masked rows, the pushed add/update form, and the handlers' authority and refusals. +import test from "node:test"; +import assert from "node:assert/strict"; +import { ensureTestEnv } from "./helpers.js"; + +ensureTestEnv(); + +const { + buildChannelSettingsView, + buildCustomMcpEditorView, + readCustomMcpForm, + parseEditorMetadata, + CHANNEL_SETTINGS_CUSTOM_MCP_ADD_ACTION_ID, + CHANNEL_SETTINGS_CUSTOM_MCP_REMOVE_ACTION_ID, + CHANNEL_SETTINGS_CUSTOM_MCP_CALLBACK_ID, + CUSTOM_MCP_NAME_BLOCK_ID, + CUSTOM_MCP_URL_BLOCK_ID, + CUSTOM_MCP_TOKEN_BLOCK_ID, + settingsCommand, +} = await import("../src/slack/channel-settings.js"); +const { applyCustomMcpAction, channelSettingsEditOptions, customMcpErrorBlock, handleCustomMcpFormSubmission, handleCustomMcpSettingsAction } = await import("../src/slack/app.js"); +const store = await import("../src/config/store.js"); + +const state = { channelId: "C_SLACK_CMCP", slug: "slack-cmcp", threadTs: "", ownerId: "U_OWNER" }; +const PUBLIC_URL = "https://93.184.215.14/mcp"; // a literal public address: no DNS needed offline +const entry = await store.upsertChannelEntry(state.channelId, { name: "slack-cmcp", type: "channel", isDM: false }); +state.slug = entry.slug; +await store.saveChannelMeta(entry.slug, store.defaultChannelMeta({ channelId: state.channelId, name: "slack-cmcp", type: "channel", isDM: false })); +await store.setUser("U_OWNER", { name: "Owner", approved: true }); + +const masked = { + channel: [{ name: "linear", serverName: "custom-linear", url: "https://mcp.linear.app/mcp", hasToken: true, tokenLast4: "ABCD" }], + personal: [{ name: "notes", serverName: "my-notes", url: "https://notes.example.com/mcp", hasToken: true, tokenLast4: "WXYZ" }], +}; +const flat = (view) => JSON.stringify(view); +const buttons = (view, actionId) => view.blocks.flatMap((block) => [...(block.elements || []), ...(block.accessory ? [block.accessory] : [])]) + .filter((element) => element?.action_id === actionId); + +test("the MCPs page lists both scopes masked, with conversation controls only for a manager", () => { + const manager = buildChannelSettingsView({ customMcps: masked }, { ...state, tab: "mcp" }, { tab: "mcp", canManageCustomMcp: true }); + assert.match(flat(manager), /Custom MCP servers/); + assert.match(flat(manager), /custom-linear/); + assert.match(flat(manager), /my-notes/); + assert.match(flat(manager), /••••ABCD/); + const managerAdds = buttons(manager, CHANNEL_SETTINGS_CUSTOM_MCP_ADD_ACTION_ID).map((b) => settingsCommand(b).s); + assert.deepEqual(managerAdds, ["c", "p"]); + const removes = buttons(manager, CHANNEL_SETTINGS_CUSTOM_MCP_REMOVE_ACTION_ID).map((b) => settingsCommand(b)); + assert.deepEqual(removes.map((c) => [c.s, c.n]), [["c", "linear"], ["p", "notes"]]); + assert.ok(removes.length && buttons(manager, CHANNEL_SETTINGS_CUSTOM_MCP_REMOVE_ACTION_ID).every((b) => b.confirm), "removal asks first"); + + const member = buildChannelSettingsView({ customMcps: masked }, { ...state, tab: "mcp" }, { tab: "mcp", canManageCustomMcp: false }); + assert.deepEqual(buttons(member, CHANNEL_SETTINGS_CUSTOM_MCP_ADD_ACTION_ID).map((b) => settingsCommand(b).s), ["p"], "a member edits only their own"); + assert.deepEqual(buttons(member, CHANNEL_SETTINGS_CUSTOM_MCP_REMOVE_ACTION_ID).map((b) => settingsCommand(b).s), ["p"]); + assert.match(flat(member), /Only this channel's managers and admins/); + + const empty = buildChannelSettingsView({}, { ...state, tab: "mcp" }, { tab: "mcp" }); + assert.match(flat(empty), /_none_/); +}); + +test("the form is never prefilled, names the scope, and parses only what was typed", () => { + const view = buildCustomMcpEditorView(state, { scope: "channel", channelName: "slack-cmcp", existing: masked.channel }); + assert.equal(view.callback_id, CHANNEL_SETTINGS_CUSTOM_MCP_CALLBACK_ID); + assert.equal(parseEditorMetadata(view.private_metadata).view, "custom_mcp_c"); + assert.equal(parseEditorMetadata(buildCustomMcpEditorView(state, { scope: "user" }).private_metadata).view, "custom_mcp_p"); + assert.deepEqual(view.blocks.filter((b) => b.type === "input").map((b) => [b.block_id, Boolean(b.optional)]), [ + [CUSTOM_MCP_NAME_BLOCK_ID, false], [CUSTOM_MCP_URL_BLOCK_ID, false], [CUSTOM_MCP_TOKEN_BLOCK_ID, true], + ]); + assert.ok(view.blocks.every((b) => !b.element?.initial_value), "nothing is prefilled"); + assert.match(flat(view), /linear/, "the hint names the existing servers so one can be updated"); + const values = { + [CUSTOM_MCP_NAME_BLOCK_ID]: { value: { value: " Linear " } }, + [CUSTOM_MCP_URL_BLOCK_ID]: { value: { value: " https://x.example.com/mcp " } }, + [CUSTOM_MCP_TOKEN_BLOCK_ID]: { value: { value: null } }, + }; + assert.deepEqual(readCustomMcpForm({ state: { values } }), { name: "Linear", url: "https://x.example.com/mcp", token: "" }); +}); + +test("refusals land under the field they are about", () => { + assert.equal(customMcpErrorBlock("URL refused: refuses to reach a loopback address"), CUSTOM_MCP_URL_BLOCK_ID); + assert.equal(customMcpErrorBlock("URL must use https://"), CUSTOM_MCP_URL_BLOCK_ID); + assert.equal(customMcpErrorBlock("A Bearer token is required for a new custom MCP"), CUSTOM_MCP_TOKEN_BLOCK_ID); + assert.equal(customMcpErrorBlock("Token must be at least 8 characters"), CUSTOM_MCP_TOKEN_BLOCK_ID); + assert.equal(customMcpErrorBlock("Name must be 1-32 characters"), CUSTOM_MCP_NAME_BLOCK_ID); + assert.equal(customMcpErrorBlock("At most 5 custom MCP servers here — remove one first"), CUSTOM_MCP_NAME_BLOCK_ID); +}); + +test("conversation servers follow the channel's manage policy; admins always qualify", () => { + const meta = { manageAccess: "admins" }; + assert.equal(channelSettingsEditOptions(meta, true, { authorId: "U1" }).canManageCustomMcp, true); + assert.equal(channelSettingsEditOptions(meta, false, { authorId: "U1", isApprovedUser: true }).canManageCustomMcp, false); + assert.equal(channelSettingsEditOptions({ manageAccess: "members" }, false, { authorId: "U1", isApprovedUser: true }).canManageCustomMcp, true); +}); + +function submission(view, values, clicker = "U_OWNER", updates = []) { + return { client: { views: { update: async (payload) => updates.push(payload) } }, body: { user: { id: clicker } }, view: { private_metadata: view.private_metadata, state: { values } } }; +} +const formValues = ({ name, url, token = "" }) => ({ + [CUSTOM_MCP_NAME_BLOCK_ID]: { value: { value: name } }, + [CUSTOM_MCP_URL_BLOCK_ID]: { value: { value: url } }, + [CUSTOM_MCP_TOKEN_BLOCK_ID]: { value: { value: token } }, +}); +const ctxFor = (overrides = {}) => async () => ({ entry, meta: (await store.getChannelMeta(entry.slug)) || {}, userIsAdmin: false, userIsApproved: true, ...overrides }); +const rootView = async (_entry, _meta, _state, _admin, { notice }) => ({ type: "modal", notice }); + +test("the form saves the viewer's own server, writes it to THEIR record, and returns to the page", async () => { + const acks = []; + const view = buildCustomMcpEditorView(state, { scope: "user" }); + await handleCustomMcpFormSubmission({ ack: async (a) => acks.push(a), ...submission(view, formValues({ name: "Notes", url: PUBLIC_URL, token: "slack-user-token-4242" })) }, { context: ctxFor(), rootView }); + assert.equal(acks[0].response_action, "update"); + assert.match(JSON.stringify(acks[0].view), /Checking access/); + assert.equal((await store.getUser("U_OWNER")).customMcps[0].token, "slack-user-token-4242"); + assert.ok(!JSON.stringify(acks).includes("slack-user-token"), "the reply never echoes the token"); +}); + +test("the form refuses internal URLs, a missing token, and a non-manager's conversation write", async () => { + const userView = buildCustomMcpEditorView(state, { scope: "user" }); + let acks = []; + const updates = []; + await handleCustomMcpFormSubmission({ ack: async (a) => acks.push(a), ...submission(userView, formValues({ name: "evil", url: "https://127.0.0.1:4747/api", token: "abcdefgh1234" }), "U_OWNER", updates) }, { context: ctxFor(), rootView }); + assert.equal(acks[0].response_action, "update", "acknowledge before asynchronous address validation"); + assert.match(JSON.stringify(updates.at(-1).view), /URL refused/); + + acks = []; + await handleCustomMcpFormSubmission({ ack: async (a) => acks.push(a), ...submission(userView, formValues({ name: "fresh", url: PUBLIC_URL })) }, { context: ctxFor(), rootView }); + assert.match(acks[0].errors[CUSTOM_MCP_TOKEN_BLOCK_ID], /token is required/); + + const channelView = buildCustomMcpEditorView(state, { scope: "channel" }); + acks = []; + await handleCustomMcpFormSubmission({ ack: async (a) => acks.push(a), ...submission(channelView, formValues({ name: "linear", url: PUBLIC_URL, token: "chan-token-9999" })) }, { context: ctxFor({ meta: { manageAccess: "admins" } }), rootView }); + assert.equal(acks[0].response_action, "errors"); + assert.match(acks[0].errors[CUSTOM_MCP_NAME_BLOCK_ID], /managers or an admin/); + assert.equal((await store.getChannelMeta(entry.slug)).customMcps, undefined, "nothing was written"); + + acks = []; + await handleCustomMcpFormSubmission({ ack: async (a) => acks.push(a), ...submission(channelView, formValues({ name: "linear", url: PUBLIC_URL, token: "chan-token-9999" }), "U_SOMEONE_ELSE") }, { context: ctxFor({ userIsAdmin: true }), rootView }); + assert.match(acks[0].errors[CUSTOM_MCP_NAME_BLOCK_ID], /expired/, "a form owned by someone else is refused"); +}); + +test("an admin saves a conversation server; Remove takes it out again; members cannot touch it", async () => { + const acks = []; + const channelView = buildCustomMcpEditorView(state, { scope: "channel" }); + await handleCustomMcpFormSubmission({ ack: async (a) => acks.push(a), ...submission(channelView, formValues({ name: "linear", url: PUBLIC_URL, token: "chan-token-9999" })) }, { context: ctxFor({ userIsAdmin: true }), rootView }); + assert.equal(acks[0].response_action, "update"); + const meta = await store.getChannelMeta(entry.slug); + assert.equal(meta.customMcps[0].setBy, "<@U_OWNER>"); + + const added = await applyCustomMcpAction({ actionId: CHANNEL_SETTINGS_CUSTOM_MCP_ADD_ACTION_ID, command: { s: "c" }, entry, meta, state, clicker: "U_OWNER", userIsAdmin: true }); + assert.equal(added.view.callback_id, CHANNEL_SETTINGS_CUSTOM_MCP_CALLBACK_ID); + + await assert.rejects( + applyCustomMcpAction({ actionId: CHANNEL_SETTINGS_CUSTOM_MCP_REMOVE_ACTION_ID, command: { s: "c", n: "linear" }, entry, meta: { ...meta, manageAccess: "admins" }, state, clicker: "U_MEMBER", userIsApproved: true }), + /managers or an admin/, + ); + const removed = await applyCustomMcpAction({ actionId: CHANNEL_SETTINGS_CUSTOM_MCP_REMOVE_ACTION_ID, command: { s: "c", n: "linear" }, entry, meta, state, clicker: "U_OWNER", userIsAdmin: true }); + assert.match(removed.notice, /custom-linear/); + assert.deepEqual(removed.meta.customMcps, []); + + const mine = await applyCustomMcpAction({ actionId: CHANNEL_SETTINGS_CUSTOM_MCP_REMOVE_ACTION_ID, command: { s: "p", n: "notes" }, entry, meta, state, clicker: "U_OWNER", userIsApproved: true }); + assert.match(mine.notice, /my-notes/); + assert.deepEqual((await store.getUser("U_OWNER")).customMcps, []); + await assert.rejects(applyCustomMcpAction({ actionId: CHANNEL_SETTINGS_CUSTOM_MCP_ADD_ACTION_ID, command: { s: "x" }, entry, meta, state, clicker: "U_OWNER" }), /expired/); +}); + + +test("repeated custom MCP saves pop the temporary form and reuse its Settings parent", async () => { + const updates = [], acks = []; + const client = { views: { update: async (payload) => updates.push(payload) } }; + const form = buildCustomMcpEditorView(state, { scope: "user" }); + for (let i = 0; i < 4; i++) { + await handleCustomMcpFormSubmission({ + client, ack: async (payload) => acks.push(payload), body: { user: { id: state.ownerId } }, + view: { ...form, id: `V_FORM_${i}`, previous_view_id: "V_SETTINGS", state: { values: formValues({ + name: "repeat", url: `${PUBLIC_URL}/${i}`, token: i === 0 ? "repeat-token-1234" : "", + }) } }, + }, { context: ctxFor(), rootView }); + assert.equal(acks.at(-1), undefined, "a successful save pops the form instead of retaining a Settings duplicate"); + assert.equal(updates.at(-1).view_id, "V_SETTINGS"); + assert.match(updates.at(-1).view.notice, /my-repeat/); + const stored = (await store.getUser(state.ownerId)).customMcps.find((server) => server.name === "repeat"); + assert.equal(stored.url, `${PUBLIC_URL}/${i}`); + assert.equal(stored.token, "repeat-token-1234", "blank-token edits preserve the original credential"); + assert.ok(!JSON.stringify(updates).includes("repeat-token")); + } + assert.equal(acks.length, 4); + assert.equal(updates.length, 8); +}); + +test("submission checks current membership, admission, slug and manager authority before writing", async () => { + const original = await store.getChannelMeta(entry.slug); + const form = buildCustomMcpEditorView(state, { scope: "user" }); + const values = formValues({ name: "denied", url: PUBLIC_URL, token: "never-save-this-token" }); + for (const variant of ["membership", "admission", "slug", "manager"]) { + await store.setUser(state.ownerId, { approved: variant !== "admission", isAdmin: false }); + await store.saveChannelMeta(entry.slug, { ...original, manageAccess: "admins" }); + const deniedForm = variant === "manager" ? buildCustomMcpEditorView(state, { scope: "channel" }) + : variant === "slug" ? buildCustomMcpEditorView({ ...state, slug: "wrong-channel" }, { scope: "user" }) : form; + const acks = []; + const beforeUser = await store.getUser(state.ownerId); + const beforeChannel = await store.getChannelMeta(entry.slug); + const updates = []; + const client = { + conversations: { members: async () => ({ members: variant === "membership" ? [] : [state.ownerId], response_metadata: {} }) }, + views: { update: async (payload) => updates.push(payload) }, + }; + await handleCustomMcpFormSubmission({ ...submission(deniedForm, values), client, ack: async (payload) => acks.push(payload) }, { rootView }); + if (variant === "membership") { + assert.equal(acks[0].response_action, "update", variant); + assert.match(JSON.stringify(updates.at(-1).view), /member/, variant); + } else { + assert.equal(acks[0].response_action, "errors", variant); + assert.match(acks[0].errors[CUSTOM_MCP_NAME_BLOCK_ID], /authorized|expired|managers/, variant); + } + assert.deepEqual((await store.getUser(state.ownerId)).customMcps, beforeUser.customMcps, variant); + assert.deepEqual((await store.getChannelMeta(entry.slug)).customMcps, beforeChannel.customMcps, variant); + } + await store.setUser(state.ownerId, { approved: true }); + await store.saveChannelMeta(entry.slug, original); +}); + +test("a parent refresh failure never acknowledges an already-saved submission twice", async () => { + const acks = [], updates = []; + const client = { views: { update: async (payload) => { + updates.push(payload); + if (updates.length === 2) throw new Error("temporary Slack update failure"); + } } }; + const form = buildCustomMcpEditorView(state, { scope: "user" }); + await handleCustomMcpFormSubmission({ + client, ack: async (payload) => acks.push(payload), body: { user: { id: state.ownerId } }, + view: { ...form, id: "V_FORM", previous_view_id: "V_SETTINGS", state: { values: formValues({ + name: "repeat", url: PUBLIC_URL, token: "rotated-test-token-7777", + }) } }, + }, { context: ctxFor(), rootView }); + assert.deepEqual(acks, [undefined]); + assert.equal(updates.length, 3); + assert.equal(updates[2].view_id, "V_SETTINGS"); + assert.match(JSON.stringify(updates[2].view), /server was saved/); + assert.ok(!JSON.stringify(updates).includes("rotated-test-token")); + assert.equal((await store.getUser(state.ownerId)).customMcps.find((server) => server.name === "repeat").token, "rotated-test-token-7777"); +}); + +function deferred() { + let resolve; + const promise = new Promise((done) => { resolve = done; }); + return { promise, resolve }; +} + +for (const delayedStage of ["dns", "membership"]) { + test(`slow ${delayedStage} is acknowledged first; revocation before completion refuses the write`, async () => { + const original = await store.getChannelMeta(entry.slug); + await store.saveChannelMeta(entry.slug, { ...original, manageAccess: "admins" }); + await store.setUser(state.ownerId, { approved: true, isAdmin: true }); + const entered = deferred(), release = deferred(); + const acks = [], updates = []; + const client = { + conversations: { members: async () => { + if (delayedStage === "membership") { entered.resolve(); await release.promise; } + return { members: [state.ownerId], response_metadata: {} }; + } }, + views: { update: async (payload) => updates.push(payload) }, + }; + const lookup = async () => { + if (delayedStage === "dns") { entered.resolve(); await release.promise; } + return [{ address: "93.184.215.14", family: 4 }]; + }; + const form = buildCustomMcpEditorView(state, { scope: "channel" }); + const pending = handleCustomMcpFormSubmission({ + ...submission(form, formValues({ name: "revoked", url: "https://fixture.example.com/mcp", token: "revoked-test-token" })), + client, ack: async (payload) => acks.push(payload), + }, { rootView, lookup }); + await entered.promise; + assert.equal(acks.length, 1, "Slack receives its acknowledgement before remote work completes"); + assert.match(JSON.stringify(acks[0].view), /Checking access/); + await store.setUser(state.ownerId, { isAdmin: false }); + release.resolve(); + await pending; + assert.equal(acks.length, 1); + assert.match(JSON.stringify(updates.at(-1).view), /managers or an admin/); + assert.deepEqual((await store.getChannelMeta(entry.slug)).customMcps, original.customMcps); + assert.ok(!JSON.stringify([...acks, ...updates]).includes("revoked-test-token")); + await store.saveChannelMeta(entry.slug, original); + }); +} + +for (const delayedStage of ["dns", "membership"]) { + test(`a timed-out ${delayedStage} check clears the draft and cannot write when it resolves later`, async () => { + const release = deferred(), finished = deferred(); + const acks = [], updates = []; + const before = (await store.getUser(state.ownerId)).customMcps; + const client = { views: { update: async (payload) => updates.push(payload) } }; + const lookup = async () => { + if (delayedStage === "dns") await release.promise; + return [{ address: "93.184.215.14", family: 4 }]; + }; + const context = async (_client, request) => { + if (delayedStage === "membership" && request.verifyMembership) await release.promise; + return ctxFor()(); + }; + const form = buildCustomMcpEditorView(state, { scope: "user" }); + await handleCustomMcpFormSubmission({ + ...submission(form, formValues({ name: "timeout", url: "https://fixture.example.com/mcp", token: "timeout-test-token" })), + client, ack: async (payload) => acks.push(payload), + }, { context, rootView, lookup, timeoutMs: 15 }); + assert.equal(acks.length, 1); + const errorView = updates.at(-1).view; + assert.match(JSON.stringify(errorView), /timed out/); + assert.equal(buttons(errorView, CHANNEL_SETTINGS_CUSTOM_MCP_ADD_ACTION_ID).length, 1, "retry opens a fresh form"); + assert.ok(!JSON.stringify([...acks, ...updates]).includes("timeout-test-token")); + release.resolve(); + // Drain the settled DNS/authorization continuation, including its store promise. + setImmediate(finished.resolve); + await finished.promise; + assert.deepEqual((await store.getUser(state.ownerId)).customMcps, before, "no late write after timeout"); + assert.equal(acks.length, 1); + }); +} + +for (const operation of ["add", "remove"]) { + test(`native ${operation} rechecks local authority after a delayed roster; Add consumes its trigger first`, async () => { + const original = await store.getChannelMeta(entry.slug); + const customMcps = [{ name: "retained", url: PUBLIC_URL, token: "retained-test-token-8888" }]; + await store.saveChannelMeta(entry.slug, { ...original, manageAccess: "admins", customMcps }); + await store.setUser(state.ownerId, { approved: true, isAdmin: true }); + const parent = buildChannelSettingsView({ customMcps: { channel: [{ name: "retained", serverName: "custom-retained", url: PUBLIC_URL, tokenLast4: "8888" }] } }, state, { tab: "mcp", canManageCustomMcp: true }); + const actionId = operation === "add" ? CHANNEL_SETTINGS_CUSTOM_MCP_ADD_ACTION_ID : CHANNEL_SETTINGS_CUSTOM_MCP_REMOVE_ACTION_ID; + const action = buttons(parent, actionId)[0]; + const entered = deferred(), release = deferred(); + const acks = [], pushes = [], updates = []; + const client = { + conversations: { members: async () => { + entered.resolve(); await release.promise; + return { members: [state.ownerId], response_metadata: {} }; + } }, + views: { + push: async (payload) => { pushes.push(payload); return { view: { id: "V_LOADING", hash: "loading-hash" } }; }, + update: async (payload) => updates.push(payload), + }, + }; + const pending = handleCustomMcpSettingsAction({ + client, action, ack: async (payload) => acks.push(payload), + body: { user: { id: state.ownerId }, trigger_id: "short-lived-trigger", view: { ...parent, id: "V_SETTINGS" } }, + }, { rootView }); + await entered.promise; + assert.deepEqual(acks, [undefined]); + if (operation === "add") { + assert.equal(pushes.length, 1, "consume trigger before the remote roster wait"); + assert.equal(pushes[0].trigger_id, "short-lived-trigger"); + assert.equal(pushes[0].view.blocks.some((b) => b.type === "input"), false, "no secret form until admitted"); + } + await store.setUser(state.ownerId, { isAdmin: false }); + release.resolve(); + await pending; + assert.match(JSON.stringify(updates.at(-1).view), /managers or an admin/); + assert.equal(updates.at(-1).view_id, operation === "add" ? "V_LOADING" : "V_SETTINGS"); + assert.deepEqual((await store.getChannelMeta(entry.slug)).customMcps, customMcps, "revoked manager cannot remove"); + assert.ok(!JSON.stringify([...pushes, ...updates]).includes("retained-test-token")); + await store.saveChannelMeta(entry.slug, original); + }); +} + +test("native Add hydrates the pushed placeholder and refuses another person's view", async () => { + const parent = buildChannelSettingsView({}, state, { tab: "mcp" }); + const action = buttons(parent, CHANNEL_SETTINGS_CUSTOM_MCP_ADD_ACTION_ID)[0]; + const acks = [], pushes = [], updates = []; + const client = { + conversations: { members: async () => ({ members: [state.ownerId], response_metadata: {} }) }, + views: { + push: async (payload) => { pushes.push(payload); return { view: { id: "V_LOADING", hash: "loading-hash" } }; }, + update: async (payload) => updates.push(payload), + }, + }; + const args = { client, action, ack: async (payload) => acks.push(payload), body: { user: { id: state.ownerId }, trigger_id: "trigger", view: { ...parent, id: "V_SETTINGS" } } }; + await handleCustomMcpSettingsAction(args, { rootView }); + assert.equal(updates.at(-1).view_id, "V_LOADING"); + assert.equal(updates.at(-1).hash, "loading-hash"); + assert.equal(updates.at(-1).view.callback_id, CHANNEL_SETTINGS_CUSTOM_MCP_CALLBACK_ID); + assert.ok(updates.at(-1).view.blocks.every((b) => !b.element?.initial_value)); + await handleCustomMcpSettingsAction({ ...args, body: { ...args.body, user: { id: "U_OTHER" } } }, { rootView }); + assert.equal(pushes.length, 1, "an owner mismatch opens nothing"); + assert.match(JSON.stringify(updates.at(-1).view), /isn't yours/); +});