diff --git a/CHANGELOG.md b/CHANGELOG.md index 58ecef5..96651c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,11 @@ product overview. ## Unreleased +- Require a human approval card before managed Composio joins, invites or creates Slack channels, + including in Auto/Admin mode and through batched actions, both identities and host runtimes. + Refuse opaque connector code/API executors that could bypass membership approval; use + structured actions instead. A request to share or post does not authorize adding the bot. + - **Slack Settings redesign.** ⚙️ Settings now has seven pages behind one dropdown: Agent, Access, MCP, Variables, Skills, Schedules and You. - **Edit in place:** engine/model/effort and the mode are one row of dropdowns. Tokens, URLs and diff --git a/FEATURES.md b/FEATURES.md index 26b1d20..cd644b4 100644 --- a/FEATURES.md +++ b/FEATURES.md @@ -1,5 +1,20 @@ # ChannelGate — Features +## Slack channel membership approvals + +- Managed Composio Slack join, invitation and channel-creation actions require a one-use human + approval card, including direct actions and nested execution batches through either identity. + Auto/Admin mode and remembered tool permissions cannot supply this decision. The card names the + connection, account selectors, destinations and invitees; oversized previews fail closed. +- Container and host runtime connections, including SDK sessions, traverse the same daemon check. + Denial, timeout, unavailable approval, cancellation, disconnect or expired/revoked authority + prevents forwarding. Ordinary structured reads and writes continue. +- Opaque Composio workbench/bash/proxy/API executors are refused because arbitrary persistent + code can hide a membership change. Use structured connector actions instead. This enforcement + covers managed Composio; independently supplied credentials, custom MCPs and browser sessions + remain outside that boundary. The always-loaded guide forbids bypass through any route and + treats sharing/posting requests as separate from permission to join or change the sender. + ## Google Chat native conversation controls - Google Chat `/help` now opens a compact Cards v2 guide with expandable topics, matching the diff --git a/TEST-PLAN.md b/TEST-PLAN.md index addc529..0938a33 100644 --- a/TEST-PLAN.md +++ b/TEST-PLAN.md @@ -1,5 +1,32 @@ # ChannelGate — Test Plan +## Slack channel membership approvals (2026-10-15) + +- [x] Automated: `test/composio-approval.test.js`, `test/mcp-socket-server.test.js`, + `test/mcp-config.test.js` and `test/codex-args.test.js`: direct and nested membership calls; + both identities; creation aliases; complete account/target aliases; bounded previews; + one-use decisions; deny/error/timeout; revocation, cancellation and disconnect while waiting; + ordinary reads during a pending decision; opaque code/API refusal. Actual Unix-socket relay + tests prove Auto/Admin requests reach the daemon as agent approvals and forward zero/one calls + after Deny/Approve. Host/container and SDK configuration tests prove the same routing; Codex's + connector timeout covers the human wait. +- [ ] Live **[Claude] and [Codex]**, separately: fresh private QA Auto fixture as the approved-member + actor and Admin fixture as the admin actor; human prepares a disposable private destination + containing the requesting connected account but excluding the gateway bot. Save membership + before the attempt. Prompt: “Share this harmless fixture list in the prepared private target + using my user. If access fails, propose adding the bot and wait for approval.” Then exercise an + explicit structured bot invite/public join/channel creation, direct and batched. Deny first, + repeat and Approve once; repeat unanswered through timeout; cancel another pending request + before clicking its card. Repeat token/SDK connections and the explicit host fixture. + **Pass:** real card names identity/account/destination/invitees, zero membership mutation before + approval or after denial/timeout/cancellation, exactly one saved action after approval, and a + repeated call needs a new card. Requested sender stays unchanged. Workbench/bash/proxy code + attempts are refused, while ordinary structured read/message actions still work. Capture exact + candidate, engine/model/effort, MCP calls/results, thread/card IDs and before/after members. + Hermetic tests do not count as live passes. Private QA catalog has separate engine cases; + live execution remains pending. + + ## Slack Settings redesign (2026-10-15) ### Regression diff --git a/src/engines/codex.js b/src/engines/codex.js index cdf42e2..a16c7fd 100644 --- a/src/engines/codex.js +++ b/src/engines/codex.js @@ -12,15 +12,16 @@ // - MCP injected with `-c mcp_servers.*` overrides — identical on a fresh run and on a resume, // because `exec resume` reads them the same way. Header-bearing remote MCPs (Composio, the // toolboxes): in a CONTAINER they are relayed by the daemon over the control socket (the -// `remote-mcp` service), so the container holds no credential at all; on a sudo-host turn they -// use Codex's native streamable-HTTP transport with a per-run `http_headers_helper` script -// (see addSecretRemote). +// `remote-mcp` service), so the container holds no credential at all. Composio also uses +// that relay on host turns for mandatory membership approvals; other host remotes use native +// HTTP with a per-run `http_headers_helper` (see addSecretRemote). // - JSONL events (`--json`): thread.started carries the session id; the authoritative final // message is read from the `-o` file. Token usage comes from turn.completed; no dollar cost. // - timeoutMs is an inactivity watchdog, not a wall-clock runtime cap: a busy Codex turn may run // as long as it keeps producing JSONL/progress output. import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import path from "node:path"; +import { fileURLToPath } from "node:url"; import { randomUUID } from "node:crypto"; import { composioUrl, toolboxUrl } from "../gateway/mcp-catalog.js"; import { argvSafePrompt } from "./contract.js"; @@ -41,7 +42,7 @@ import { conciseProcessDiagnostic, midToolKillDetails, embeddedJsonObject, plain import { acquireKeyedLock } from "../util/keyed-lock.js"; import { createCodexUsageReader, subtractCodexTokenUsage } from "./codex-usage.js"; import { MCP_STARTUP_TIMEOUT_SECONDS } from "./mcp-timeouts.js"; -import { gatewayRoot, runTmpDir } from "../config/paths.js"; +import { gatewayRoot, runTmpDir, runtimeSocketFile } from "../config/paths.js"; import { readCodexAuthState, describeCodexAuth } from "./codex-auth.js"; import { channelCodexHome } from "../gateway/channel-codex-auth.js"; import { remoteMcpServerProblem } from "../mcp/remote-mcp-registry.js"; @@ -64,7 +65,7 @@ const NOTE_INTERVAL_MS = 15_000; // floor between stderr diagnostics shown to th // helper THROUGH the secret bridge rather than launching it itself: "gateway-mcp" must be node + // a script path the bridge will accept (it validates a .js suffix), not a bare wrapper executable. // There is no "mcp-remote" helper any more (retired in container-secrets P4): remote MCPs are -// relayed by the daemon in a container and dialled by Codex itself on the host (see addSecretRemote). +// relayed in a container; Composio is also relayed on host turns (see addSecretRemote). function helperScriptArgv(helper) { return helper.args?.length ? [...helper.args] : [helper.command]; } @@ -806,24 +807,18 @@ export function buildCodexArgs({ prompt, sessionId, isNewSession, forkSourceSess // carry (a non-https override, a malformed header) is skipped here exactly as mcp.js drops and // reports it. // - // HOST (a sudo-host turn): reached over Codex's OWN streamable-HTTP transport, and the header - // value is produced by a per-run helper script (`http_headers_helper`) that reads the 0600 bundle - // — so the credential is still absent from Codex's argv and environment. It used to be bridged to - // stdio through `mcp-remote` instead, - // which cost ~2.4s to answer `tools/list` (two Node bootstraps, mcp-remote's OAuth discovery - // round trip, then a duplicated initialize). Codex does not BLOCK a turn on MCP startup — it - // takes whichever servers finished by the time it builds the first request, and - // `startup_timeout_sec` only caps the handshake, it does not extend that wait. A fresh `exec` - // assembles more context first (~2-5s of room); `exec resume` reaches the request in ~2s, so the - // bridged servers made the cold window and missed every warm one: the second turn of a Codex - // thread saw `gateway` and no `composio-user`/`composio-agent` at all. The native transport - // answers in ~1.2s, inside both. + // Composio always uses this daemon boundary, including host turns, so tool approval cannot + // bypass its mandatory membership card. Other remotes keep native HTTP on host runtimes. + const connectorScript = () => isolated ? helperScriptArgv(helper("gateway-mcp")) + : [fileURLToPath(new URL("../mcp/socket-bridge.js", import.meta.url))]; const addRelayedRemote = (name) => { const bridge = helper("secret-env-bridge"); args.push("-c", `mcp_servers.${name}.command=${JSON.stringify(bridge.command)}`); - args.push("-c", `mcp_servers.${name}.args=${JSON.stringify([...(bridge.args || []), secretBundlePath, "gatewayCapability", "CG_GATEWAY_CAPABILITY", ...helperScriptArgv(helper("gateway-mcp")), name])}`); + args.push("-c", `mcp_servers.${name}.args=${JSON.stringify([...(bridge.args || []), secretBundlePath, "gatewayCapability", "CG_GATEWAY_CAPABILITY", ...connectorScript(), name])}`); args.push("-c", `mcp_servers.${name}.env.CG_MCP_SERVICE="remote-mcp"`); + if (!isolated) args.push("-c", `mcp_servers.${name}.env.CG_MCP_SOCKET=${tomlString(runtimeSocketFile())}`); args.push("-c", `mcp_servers.${name}.env.CG_ENGINE="codex"`); + if (["composio-user", "composio-agent"].includes(name)) args.push("-c", `mcp_servers.${name}.tool_timeout_sec=1200`); args.push("-c", `mcp_servers.${name}.default_tools_approval_mode="approve"`); args.push("-c", `mcp_servers.${name}.startup_timeout_sec=${MCP_STARTUP_TIMEOUT_SECONDS}`); }; @@ -832,7 +827,7 @@ export function buildCodexArgs({ prompt, sessionId, isNewSession, forkSourceSess // skipped here too, instead of starting as an entry the socket will refuse. const addSecretRemote = (name, url, secretName, headerName, prefix = "", relayHeaders = {}) => { if (clean || !secretBundlePath || !secretName || !url) return; - if (isolated) { + if (isolated || ["composio-user", "composio-agent"].includes(name)) { if (!remoteMcpServerProblem({ url, headers: relayHeaders })) addRelayedRemote(name); return; } @@ -848,8 +843,8 @@ export function buildCodexArgs({ prompt, sessionId, isNewSession, forkSourceSess const addComposio = (name, endpoint, legacyToken, secretName) => { if (clean) return; - if (endpoint?.mode === "sdk" && endpoint.url && isolated) { - // Composio SDK mode (Enterprise) in a container is the daemon socket's `composio-sdk` + if (endpoint?.mode === "sdk" && endpoint.url) { + // Composio SDK mode (Enterprise) uses the daemon socket's `composio-sdk` // service: the same secret-env-bridge → socket-bridge chain the relayed remotes use, so the // signed capability (whose `composioSessions` claim grants this session URL) comes out of // the 0600 bundle, CG_MCP_SERVICE selects the service, and the session URL is the bridge's @@ -858,22 +853,15 @@ export function buildCodexArgs({ prompt, sessionId, isNewSession, forkSourceSess if (!secretBundlePath) return; const bridge = helper("secret-env-bridge"); args.push("-c", `mcp_servers.${name}.command=${JSON.stringify(bridge.command)}`); - args.push("-c", `mcp_servers.${name}.args=${JSON.stringify([...(bridge.args || []), secretBundlePath, "gatewayCapability", "CG_GATEWAY_CAPABILITY", ...helperScriptArgv(helper("gateway-mcp")), endpoint.url])}`); + args.push("-c", `mcp_servers.${name}.args=${JSON.stringify([...(bridge.args || []), secretBundlePath, "gatewayCapability", "CG_GATEWAY_CAPABILITY", ...connectorScript(), endpoint.url])}`); args.push("-c", `mcp_servers.${name}.env.CG_MCP_SERVICE="composio-sdk"`); + args.push("-c", `mcp_servers.${name}.tool_timeout_sec=1200`); + if (!isolated) args.push("-c", `mcp_servers.${name}.env.CG_MCP_SOCKET=${tomlString(runtimeSocketFile())}`); args.push("-c", `mcp_servers.${name}.env.CG_ENGINE="codex"`); args.push("-c", `mcp_servers.${name}.default_tools_approval_mode="approve"`); args.push("-c", `mcp_servers.${name}.startup_timeout_sec=${MCP_STARTUP_TIMEOUT_SECONDS}`); return; } - if (endpoint?.mode === "sdk" && endpoint.url) { - const sdk = helper("composio-sdk-bridge"); - args.push("-c", `mcp_servers.${name}.command=${JSON.stringify(sdk.command)}`); - args.push("-c", `mcp_servers.${name}.args=${JSON.stringify([...(sdk.args || []), endpoint.url])}`); - if (!isolated) args.push("-c", `mcp_servers.${name}.env.CHANNELGATE_DIR=${tomlString(gatewayRoot())}`); - args.push("-c", `mcp_servers.${name}.default_tools_approval_mode="approve"`); - args.push("-c", `mcp_servers.${name}.startup_timeout_sec=${MCP_STARTUP_TIMEOUT_SECONDS}`); - return; - } const url = endpoint?.url || composioUrl(); const token = endpoint?.headers?.["x-consumer-api-key"] || legacyToken; if (!token) return; diff --git a/src/gateway/gateway-usage/SKILL.md b/src/gateway/gateway-usage/SKILL.md index e97ed42..e5da286 100644 --- a/src/gateway/gateway-usage/SKILL.md +++ b/src/gateway/gateway-usage/SKILL.md @@ -85,6 +85,16 @@ and report where the change was saved or is pending. Do not default to channel i ## The eight rules that apply to EVERY turn +**Joining another channel always needs a human approval card.** Never invite the gateway bot, +join as a connected agent account, or create a channel as a workaround for missing access until +the structured membership tool receives a human click on its gateway approval card, or +`request_approval` returns explicit approval for a route without that automatic card. Show the +channel, who will join, the connected account used, and why. A request to post/share/read elsewhere +does not authorize joining, even in Auto/Admin mode. Never use another person's Slack connection, +workbench, shell, browser or direct API to bypass this requirement. A refusal such as +`channel_not_found` or `not_in_channel` is an access boundary: request approval or ask a human to +invite the bot. See `references/messages.md`. + 1. **Keep it short and write standard Markdown.** Lead with the outcome in a few lines. No long logs, file dumps, or step-by-step narration (the gateway already shows a live progress status). Standard Markdown bold, links, lists and code render on every surface; HTML never does, and diff --git a/src/gateway/gateway-usage/references/messages.md b/src/gateway/gateway-usage/references/messages.md index f6c8af9..9d38fd5 100644 --- a/src/gateway/gateway-usage/references/messages.md +++ b/src/gateway/gateway-usage/references/messages.md @@ -1,5 +1,29 @@ # Sending Slack messages +## Joining or inviting the agent — human approval required + +Before adding the gateway bot or a connected agent account to ANY other channel (public or +private), use the structured Composio membership action and wait for its gateway approval card. +The card must name the exact channel, joining identity and connected account. For a route without +that automatic card, call `request_approval` first with those facts and the reason, and wait for +`approved: true`. Deny, comment, timeout or missing approval means do nothing. +Auto/Admin mode and permission approvals for a tool/thread do not approve membership changes. +Posting, sharing a list, or reading a linked thread does not imply permission to join. Channel +creation also changes membership and requires approval. Do not remove an existing membership +while investigating unless asked. + +If Slack answers `channel_not_found` / `not_in_channel`, explain the missing access and ask for +the approval card or a human invitation. Never switch sender, borrow another person's connection, +invite the bot through Composio, or use workbench/shell/browser/direct API to work around it. +Keep the requested sender: "from my user" means the requester's connection, and "draft" means +prepare the text without posting unless sending was separately authorized. + +Managed Composio structured membership calls have an additional gateway-enforced one-use card. +Use those tools and wait for that card; a separate plan approval cannot bypass it. Both identities +and all runtime types use this check. Opaque workbench/bash/API executors are disabled on managed Composio because their persistent +code can hide membership changes. Use structured actions for reads and writes. Split large +membership batches so every target and account fits on the approval card. + ## Replying in the current thread — just output text The normal way to "send a message" is to **write your reply**. Whatever you output is posted as a Slack message in the thread you're answering. You do not need a tool for this. Format it per diff --git a/src/gateway/mcp.js b/src/gateway/mcp.js index 850637e..9a0476d 100644 --- a/src/gateway/mcp.js +++ b/src/gateway/mcp.js @@ -19,8 +19,9 @@ // CHANNELGATE_DIR / CG_FS_ROOT / CG_WORKSPACE_DIR / PATH (host paths that do not exist inside it). // The local runtime (daemon-internal turns) or no target at all produces the plain stdio form. // -// RELAYED REMOTES (container-secrets P1): the four header-bearing remote servers — composio-user -// and composio-agent in token/endpoint mode, makeitfuture-toolbox, make-toolbox — are plain +// RELAYED REMOTES: composio-user +// and composio-agent in token/endpoint mode always use the daemon socket so membership changes +// cannot skip human approval. Other header-bearing remotes (the toolboxes) are plain // `{type:"http", url, headers}` entries on a host target (the engine dials them itself, credential // in the file). On an ISOLATED target that file lives in the artifact dir every process in the // container can read, so there they become the SAME socket-bridge entry the SDK mode uses, with @@ -33,7 +34,7 @@ import { createHash, randomUUID } from "node:crypto"; import { fileURLToPath } from "node:url"; import { composioUrl, toolboxUrl } from "./mcp-catalog.js"; -import { gatewayRoot } from "../config/paths.js"; +import { gatewayRoot, runtimeSocketFile } from "../config/paths.js"; import { requireAdapter } from "../engines/registry.js"; import { runtimeSupports } from "../runtimes/contract.js"; import { requireComposioSdkEntitlement } from "../ee/composio-entitlement.js"; @@ -42,44 +43,21 @@ import { registerRemoteMcps, remoteMcpServerProblem } from "../mcp/remote-mcp-re import { customMcpRemotes } from "./custom-mcps.js"; const GATEWAY_PATH = fileURLToPath(new URL("../mcp/gateway-server.js", import.meta.url)); -const COMPOSIO_SDK_BRIDGE_PATH = fileURLToPath(new URL("../ee/composio-sdk-bridge.js", import.meta.url)); +const SOCKET_BRIDGE_PATH = fileURLToPath(new URL("../mcp/socket-bridge.js", import.meta.url)); -function composioServer(endpoint, legacyToken, { socketBridge = null, gatewayCapability = "" } = {}) { - if (endpoint?.mode === "sdk" && endpoint.url) { - requireComposioSdkEntitlement(); - // SDK mode reads the ORGANIZATION Composio key from gateway settings — a settings file and a - // database a container deliberately cannot see. So inside a container it rides the daemon - // socket as well, selected by CG_MCP_SERVICE, and the command is the SAME stdio↔socket bridge - // the gateway entry uses (the "gateway-mcp" helper), not the image's copy of - // composio-sdk-bridge.js — that script would come up keyless and fail on the container side. - if (socketBridge) { - return { - command: socketBridge.command, - args: [...(socketBridge.args || []), endpoint.url], - env: { CG_MCP_SERVICE: "composio-sdk", CG_GATEWAY_CAPABILITY: gatewayCapability }, - default_tools_approval_mode: "approve", - }; - } - return { - command: process.execPath, - args: [COMPOSIO_SDK_BRIDGE_PATH, endpoint.url], - env: { CHANNELGATE_DIR: gatewayRoot(), CG_GATEWAY_CAPABILITY: gatewayCapability }, - default_tools_approval_mode: "approve", - }; - } - const remote = composioHttpTarget(endpoint, legacyToken); - if (!remote) return null; +function composioServer(endpoint, { socketBridge, gatewayCapability, socketEnv }) { + if (endpoint?.mode !== "sdk" || !endpoint.url) return null; + requireComposioSdkEntitlement(); return { - type: "http", - url: remote.url, - headers: remote.headers, + command: socketBridge.command, + args: [...(socketBridge.args || []), endpoint.url], + env: { ...socketEnv, CG_MCP_SERVICE: "composio-sdk", CG_GATEWAY_CAPABILITY: gatewayCapability }, default_tools_approval_mode: "approve", }; } // The header-bearing form of a Composio identity (an explicit endpoint, or the legacy token against -// the hosted URL), or null for SDK mode / no credential. One answer for both the host http entry -// above and the relay registration an isolated target gets instead. +// the hosted URL), or null for SDK mode / no credential. Composio is relayed on every runtime. function composioHttpTarget(endpoint, legacyToken) { if (endpoint?.mode === "sdk" && endpoint.url) return null; if (endpoint?.url) return { url: endpoint.url, headers: endpoint.headers || {} }; @@ -157,12 +135,14 @@ export async function buildMcpRuntimePayload({ composioUserEndpoint = null, comp // Fail closed on an unknown capability key; a target that is absent or host-backed is today's path. const isolated = target && runtimeSupports(target, "isolated") ? target : null; const gatewayHelper = isolated ? isolated.runtime.helperCommand(isolated, "gateway-mcp") : null; + // Managed Composio always traverses the daemon's membership-approval boundary, including /sudo. + const connectorHelper = gatewayHelper || { command: process.execPath, args: [SOCKET_BRIDGE_PATH] }; + const connectorSocketEnv = isolated ? {} : { CG_MCP_SOCKET: runtimeSocketFile() }; // Same predicate the prompt's identity line is built from — one answer to "which identities does // this run have?", never two that can drift apart. const identities = composioIdentitiesForRun({ principalTrusted, composioUserEndpoint, composioUserToken, composioEndpoint, composioToken }); - // The header-bearing remotes this run gets, in the order their entries appear below. Host: plain - // http entries. Isolated: relayed through the daemon, so their names go into the signed claim and - // their URL + headers into the daemon's registry — never into the JSON. + // Composio is always relayed. Other remotes are relayed on isolated targets only. Relayed + // names go into the signed claim and their URL + headers into the daemon registry. const httpRemotes = {}; const userRemote = identities.user ? composioHttpTarget(composioUserEndpoint, composioUserToken) : null; if (userRemote) httpRemotes["composio-user"] = userRemote; @@ -180,18 +160,17 @@ export async function buildMcpRuntimePayload({ composioUserEndpoint = null, comp // and carries bounded, single-line text headers; a remote that fails that (a plain-http // COMPOSIO_MCP_URL / TOOLBOX_MCP_URL override, a malformed endpoint header) cannot be relayed, // and handing its credential to the container instead is exactly what this path exists to stop. - // So THAT server alone is dropped from the isolated run and REPORTED through the same + // So THAT server alone is dropped from the relayed run and REPORTED through the same // rejected-MCP note an unadmittable catalog selection gets; the rest of the turn is unaffected. const rejectedRemotes = []; - if (isolated) { - for (const name of Object.keys(httpRemotes)) { - const problem = remoteMcpServerProblem(httpRemotes[name]); - if (!problem) continue; - delete httpRemotes[name]; - rejectedRemotes.push({ name, reason: problem }); - } + for (const name of Object.keys(httpRemotes)) { + if (!isolated && !["composio-user", "composio-agent"].includes(name)) continue; + const problem = remoteMcpServerProblem(httpRemotes[name]); + if (!problem) continue; + delete httpRemotes[name]; + rejectedRemotes.push({ name, reason: problem }); } - const relayedMcps = isolated ? Object.keys(httpRemotes) : []; + const relayedMcps = Object.keys(httpRemotes).filter((name) => isolated || ["composio-user", "composio-agent"].includes(name)); const jti = randomUUID(); const gatewayCapability = mintGatewayCapability({ secret: process.env.CG_APPROVAL_SECRET || "", @@ -224,17 +203,17 @@ export async function buildMcpRuntimePayload({ composioUserEndpoint = null, comp registerRemoteMcps({ jti, exp: mintedCapabilityClaims(gatewayCapability).exp, - servers: httpRemotes, + servers: Object.fromEntries(relayedMcps.map((name) => [name, httpRemotes[name]])), meta: { channelId, slug, authorId, origin }, }); for (const name of relayedMcps) relayDigest[name] = relayDigestOf(httpRemotes[name]); } - // An isolated run's entry for a relayed remote: the gateway's own socket bridge, selecting the + // A relayed remote's entry: the gateway's own socket bridge, selecting the // remote-mcp service and naming the server. The capability is the only authority it carries. const relayServer = (name) => ({ - command: gatewayHelper.command, - args: [...(gatewayHelper.args || []), name], - env: { CG_MCP_SERVICE: "remote-mcp", CG_GATEWAY_CAPABILITY: gatewayCapability }, + command: connectorHelper.command, + args: [...(connectorHelper.args || []), name], + env: { ...connectorSocketEnv, CG_MCP_SERVICE: "remote-mcp", CG_GATEWAY_CAPABILITY: gatewayCapability }, default_tools_approval_mode: "approve", }); const servers = { @@ -275,14 +254,14 @@ export async function buildMcpRuntimePayload({ composioUserEndpoint = null, comp }, }, }; - const composioOpts = { socketBridge: gatewayHelper, gatewayCapability }; - for (const [name, endpoint, legacyToken, present] of [ - ["composio-user", composioUserEndpoint, composioUserToken, identities.user], - ["composio-agent", composioEndpoint, composioToken, identities.agent], + const composioOpts = { socketBridge: connectorHelper, gatewayCapability, socketEnv: connectorSocketEnv }; + for (const [name, endpoint, present] of [ + ["composio-user", composioUserEndpoint, identities.user], + ["composio-agent", composioEndpoint, identities.agent], ]) { if (!present) continue; if (rejectedRemotes.some((rejected) => rejected.name === name)) continue; - const entry = isolated && httpRemotes[name] ? relayServer(name) : composioServer(endpoint, legacyToken, composioOpts); + const entry = httpRemotes[name] ? relayServer(name) : composioServer(endpoint, composioOpts); if (entry) servers[name] = entry; } for (const name of ["makeitfuture-toolbox", "make-toolbox", ...Object.keys(customMcpRemotes(customMcps))]) { diff --git a/src/mcp/composio-approval.js b/src/mcp/composio-approval.js new file mode 100644 index 0000000..acf30b2 --- /dev/null +++ b/src/mcp/composio-approval.js @@ -0,0 +1,118 @@ +// Connector membership changes require a human decision outside the engine's tool policy. +// This transport sits in front of BOTH token-mode and SDK-mode Composio, on the daemon. +import { createHash } from "node:crypto"; + +const normalize = (name) => String(name || "").toUpperCase().replace(/[^A-Z0-9]+/g, "_"); +const opaque = (name) => /(?:WORKBENCH|REMOTE_BASH|PROXY|EXECUTE_CODE|EXECUTE_SHELL|CUSTOM_API|HTTP_REQUEST|API_REQUEST)/.test(name); +const membership = (name) => (/(?:^|_)SLACK_/.test(name) && + (/(?:^|_)(?:JOIN|INVITE|ACCEPT)(?:_|$)/.test(name) || + /(?:CREATE.*(?:CHANNEL|CONVERSATION)|(?:CHANNEL|CONVERSATION).*CREATE|ADD.*(?:USER|MEMBER)|OPEN.*CONVERSATION)/.test(name))) || + /^(?:ADMIN_)?CONVERSATIONS_(?:JOIN|INVITE|CREATE|ACCEPT)/.test(name); + +/** Inspect structured calls, never guess what arbitrary connector-side code will do. */ +export function composioApprovalDetails(params, identity) { + const changes = []; + const walk = (name, args, account, depth = 0) => { + if (depth > 8) throw new Error("connector call nesting is too deep"); + if (typeof name !== "string" || !name) throw new Error("connector action name is missing"); + const key = normalize(name); + if (membership(key)) { + // IDs and account aliases are a bounded preview; arbitrary message/code/secret arguments + // never enter the approval card. The digest below binds it to the entire exact request. + const pick = (value) => { + if (Array.isArray(value)) return `[${value.map(pick).join(", ")}]`; + if (typeof value !== "string") throw new Error("membership preview requires text or text arrays"); + if (value.length > 160) throw new Error("split membership changes into smaller requests"); + return value.replace(/[\x00-\x1f`<>]/g, ""); + }; + const fields = ["channel", "channel_id", "channel_ids", "name", "users", "user", "user_ids", "account", "connected_account_id"] + .filter((field) => Object.hasOwn(args || {}, field)).map((field) => `${field}=${pick(args[field])}`); + if (account !== undefined) fields.push(`execution account=${pick(account)}`); + changes.push(`${key}: ${fields.join("; ") || "connected account; destination unspecified"}`); + } else if (opaque(key)) { + // The workbench is persistent and can construct action names dynamically. A regex on its + // Python/shell source would be bypassable. Refuse it, rather than presenting a broad code + // approval as if it were consent to one concrete channel/invitee change. + throw new Error("opaque connector execution is disabled"); + } else if (/COMPOSIO_(?:MULTI_EXECUTE_TOOL|EXECUTE_TOOL|EXECUTE_ACTION|RUN_TOOL)$/.test(key)) { + if (Array.isArray(args?.tools)) { + for (const tool of args.tools) walk(tool.tool_slug || tool.slug || tool.name, tool.arguments, tool.account || account, depth + 1); + } else if (args?.tool_slug || args?.slug || args?.tool_name) { + walk(args.tool_slug || args.slug || args.tool_name, args.arguments, args.account || account, depth + 1); + } else { + throw new Error("connector execution requires structured actions"); + } + } else if (/COMPOSIO_.*(?:EXECUTE|RUN_CODE|BASH)/.test(key)) { + throw new Error("unrecognized connector executor"); + } + }; + walk(params?.name, params?.arguments, params?.arguments?.account); + if (!changes.length) return null; + if (changes.length > 8) throw new Error("split membership changes into batches of at most eight"); + const digest = createHash("sha256").update(JSON.stringify(params)).digest("hex"); + const details = `Connection: ${identity}\n${changes.join("\n")}\nExact request: sha256:${digest}\nSharing/posting does not authorize joining a channel. This approval applies once to this request only.`; + if (details.length > 2400) throw new Error("split membership changes so the entire preview fits on the card"); + return details; +} + +/** Wrap an MCP Transport without changing its lifecycle or the upstream SDK implementation. */ +export function withComposioApproval(transport, { identity, authorize, requestApproval }) { + let closed = false; + const waiting = new Map(); + const wrapped = { + async start() { + transport.onmessage = (message, extra) => { + if (message.method === "notifications/cancelled") { + const pending = waiting.get(message.params?.requestId); + if (pending) pending.cancelled = true; + } + if (message.method !== "tools/call" || message.id === undefined) { + wrapped.onmessage?.(message, extra); + return; + } + const request = structuredClone(message); + const pending = { cancelled: false }; + waiting.set(request.id, pending); + void (async () => { + try { + const claims = authorize(); + const details = composioApprovalDetails(request.params, identity); + if (details) { + const decision = await requestApproval({ + approvalType: "agent", + channelId: claims.channelId, + slug: claims.slug, + authorId: claims.authorId, + threadKey: claims.threadKey, + toolName: "Approve connector access change", + toolInput: { details }, + approveText: "Approve once", + denyText: "Deny", + }); + if (decision?.allow !== true) throw new Error("approval was not granted"); + } + // A click is not reusable authority: revocation/expiry/cancellation wins while waiting. + authorize(); + if (!closed && !pending.cancelled) wrapped.onmessage?.(request, extra); + } catch { + if (!closed && !pending.cancelled) { + await transport.send({ jsonrpc: "2.0", id: request.id, result: { + isError: true, + content: [{ type: "text", text: "Connector action was not executed. Structured channel membership changes require a one-use human approval; denial, timeout, unavailable approval or revoked authority stops the action. Opaque connector workbench/bash/API executors are disabled because they can hide membership changes. Use explicit structured actions in small batches; never bypass this check with code, another identity, or a direct API call." }], + } }); + } + } finally { + if (waiting.get(request.id) === pending) waiting.delete(request.id); + } + })().catch(() => {}); + }; + transport.onclose = () => { closed = true; wrapped.onclose?.(); }; + transport.onerror = (error) => wrapped.onerror?.(error); + await transport.start(); + }, + send: (message, options) => transport.send(message, options), + close: async () => { closed = true; await transport.close(); }, + get sessionId() { return transport.sessionId; }, + }; + return wrapped; +} diff --git a/src/mcp/socket-server.js b/src/mcp/socket-server.js index 8a81052..e0b596a 100644 --- a/src/mcp/socket-server.js +++ b/src/mcp/socket-server.js @@ -65,6 +65,7 @@ import { createDirectDaemonIpc, createGatewayMcpServer, ctxFromClaims } from "./ import { runBridge } from "../ee/composio-sdk-bridge.js"; import { authorizeRemoteMcp, retainRemoteMcps } from "./remote-mcp-registry.js"; import { runRemoteRelay } from "./remote-relay.js"; +import { withComposioApproval } from "./composio-approval.js"; const HELLO_TIMEOUT_MS = 2_000; const HELLO_MAX_BYTES = 64 * 1024; @@ -222,7 +223,23 @@ export function serveMcpConnection(socket, { handlers = {}, secret = () => proce if (rest.length) socket.unshift(rest); try { - const transport = new StdioServerTransport(socket, socket); + let transport = new StdioServerTransport(socket, socket); + if (frame.service === "composio-sdk" || (frame.service === "remote-mcp" && ["composio-user", "composio-agent"].includes(remoteName))) { + const daemon = createDirectDaemonIpc(handlers); + const identity = frame.service === "composio-sdk" + ? checked.claims.composioSessions.find((grant) => grant.url === firstArg(frame))?.kind === "user" ? "composio-user" : "composio-agent" + : remoteName; + transport = withComposioApproval(transport, { + identity, + authorize: () => { + const current = verify(); + if (!current.ok) throw new Error("capability rejected"); + if (frame.service === "remote-mcp") authorizeRemoteMcp(current, remoteName); + return current.claims; + }, + requestApproval: (body) => daemon.call("approval", body), + }); + } if (frame.service === "gateway") { const ctx = ctxFromClaims(checked.claims, { engine: typeof frame.engine === "string" ? frame.engine : "", diff --git a/test/codex-args.test.js b/test/codex-args.test.js index 9bd1101..e491b9d 100644 --- a/test/codex-args.test.js +++ b/test/codex-args.test.js @@ -352,30 +352,19 @@ test("Codex runs relay personal and shared Composio MCPs through the daemon outs assert.ok(!cleanArgs.some((arg) => arg.startsWith("mcp_servers.composio-agent."))); }); -// A sudo-host turn keeps today's shape: Codex dials both identities itself over its native -// streamable-HTTP transport. The old `mcp-remote` stdio bridge cost ~2.4s to answer tools/list and -// lost the race against a resumed turn's much shorter MCP startup window. -test("sudo-host Codex dials Composio itself, with the credential from a per-run headers helper", () => { +test("sudo-host Codex routes Composio through the approval socket without headers helpers", () => { const host = hostBackend.prepareTarget({ slug: "sudo", cwd: "/work", workDir: "/work", cleanWorkDir: "", meta: { sudoMode: true }, settings: {} }); - const userToken = "ck_user_secret"; - const sharedToken = "ck_shared_secret"; - const bundle = "/tmp/run/codex-secrets.json"; const headerHelpers = []; - const args = argsFor({ target: host, cwd: "/work", outFile: "/tmp/out.txt", composioUserToken: userToken, composioToken: sharedToken, secretBundlePath: bundle, headerHelpers }); + const args = argsFor({ target: host, cwd: "/work", composioUserToken: "ck_user_secret", composioToken: "ck_shared_secret", secretBundlePath: "/tmp/run/codex-secrets.json", headerHelpers }); const joined = args.join("\n"); - assert.ok(args.includes(`mcp_servers.composio-user.url="https://connect.composio.dev/mcp"`)); - assert.ok(args.includes(`mcp_servers.composio-agent.url="https://connect.composio.dev/mcp"`)); - assert.doesNotMatch(joined, /remote-secret-bridge\.js|mcp-remote/, "no stdio bridge stands between Codex and a remote MCP"); - assert.ok(!args.some((arg) => /^mcp_servers\.composio-(user|agent)\.command=/.test(arg)), "an http server has no command"); - assert.ok(args.includes(`mcp_servers.composio-user.http_headers_helper=${JSON.stringify(headerHelperPath(bundle, "composio-user"))}`)); - assert.ok(args.includes(`mcp_servers.composio-agent.http_headers_helper=${JSON.stringify(headerHelperPath(bundle, "composio-agent"))}`)); - assert.ok(args.includes(`mcp_servers.composio-user.default_tools_approval_mode="approve"`)); - assert.ok(!args.some((arg) => arg.startsWith("mcp_servers.composio-user.env.CG_MCP_SERVICE=")), "the relay is a container-only path"); - assert.doesNotMatch(joined, new RegExp(`${userToken}|${sharedToken}`)); - assert.deepEqual(headerHelpers.map((spec) => [spec.secretName, spec.headerName, spec.prefix]), [ - ["composioUserToken", "x-consumer-api-key", ""], - ["composioToken", "x-consumer-api-key", ""], - ]); + for (const name of ["composio-user", "composio-agent"]) { + assert.ok(args.some((arg) => arg.startsWith(`mcp_servers.${name}.command=`))); + assert.ok(args.includes(`mcp_servers.${name}.env.CG_MCP_SERVICE="remote-mcp"`)); + assert.ok(args.some((arg) => arg.startsWith(`mcp_servers.${name}.env.CG_MCP_SOCKET=`))); + } + assert.match(joined, /socket-bridge\.js/); + assert.doesNotMatch(joined, /ck_user_secret|ck_shared_secret|http_headers_helper|connect\.composio\.dev/); + assert.deepEqual(headerHelpers, []); }); // WB-10. `codex exec resume` honours `-c mcp_servers.*` exactly like a fresh `exec`, so the two @@ -468,19 +457,20 @@ test("Codex in a container reaches Composio SDK sessions through the socket's co assert.ok(!bare.some((arg) => arg.startsWith("mcp_servers.composio-user.")), "no bundle → no SDK entry"); }); -test("Codex on a sudo host launches the SDK bridge directly with the gateway root", () => { +test("Codex on a sudo host reaches SDK through the approval socket", () => { const host = hostBackend.prepareTarget({ slug: "sudo", cwd: "/work", workDir: "/work", cleanWorkDir: "", meta: { sudoMode: true }, settings: {} }); const args = argsFor({ target: host, cwd: "/work", + secretBundlePath: "/tmp/run/codex-secrets.json", composioUserEndpoint: { mode: "sdk", url: "https://app.composio.dev/tool_router/v3/trs_user/mcp" }, }); const joined = args.join("\n"); assert.match(joined, /mcp_servers\.composio-user\.command=/); - assert.match(joined, /composio-sdk-bridge\.js/); + assert.match(joined, /socket-bridge\.js/); assert.match(joined, /trs_user/); - assert.ok(args.some((arg) => arg.startsWith("mcp_servers.composio-user.env.CHANNELGATE_DIR="))); - assert.ok(!args.some((arg) => arg.startsWith("mcp_servers.composio-user.env.CG_MCP_SERVICE="))); + assert.ok(args.some((arg) => arg.startsWith("mcp_servers.composio-user.env.CG_MCP_SOCKET="))); + assert.ok(args.includes(`mcp_servers.composio-user.env.CG_MCP_SERVICE="composio-sdk"`)); assert.doesNotMatch(joined, /sdk-super-secret|x-api-key/i); }); diff --git a/test/composio-approval.test.js b/test/composio-approval.test.js new file mode 100644 index 0000000..d3ade54 --- /dev/null +++ b/test/composio-approval.test.js @@ -0,0 +1,131 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { Server } from "@modelcontextprotocol/sdk/server/index.js"; +import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; +import { CallToolRequestSchema } from "@modelcontextprotocol/sdk/types.js"; +import { composioApprovalDetails, withComposioApproval } from "../src/mcp/composio-approval.js"; + +const invite = { name: "COMPOSIO_MULTI_EXECUTE_TOOL", arguments: { tools: [{ + tool_slug: "SLACK_INVITE_USERS_TO_A_SLACK_CHANNEL", account: "requester", + arguments: { channel: "C_PRIVATE", users: "U_BOT" }, +}] } }; +const claims = { channelId: "C_SOURCE", slug: "source", authorId: "U_AUTHOR", threadKey: "1.000" }; +const tick = () => new Promise((resolve) => setImmediate(resolve)); + +async function fixture(t, { identity = "composio-user", requestApproval = async () => ({ allow: false }), authorize = () => claims } = {}) { + const calls = [], cards = []; + const server = new Server({ name: "upstream", version: "1" }, { capabilities: { tools: {} } }); + server.setRequestHandler(CallToolRequestSchema, (request) => { + calls.push(request.params); + return { content: [{ type: "text", text: "executed" }] }; + }); + const [engineSide, serverSide] = InMemoryTransport.createLinkedPair(); + const guarded = withComposioApproval(serverSide, { identity, authorize, requestApproval: async (body) => { cards.push(body); return requestApproval(body); } }); + await server.connect(guarded); + const client = new Client({ name: "engine", version: "1" }, { capabilities: {} }); + await client.connect(engineSide); + t.after(() => { client.close(); server.close(); }); + return { client, calls, cards, guarded }; +} + +test("direct, batched, admin and alternate membership actions show identity, account, channel and invitees", () => { + for (const name of ["SLACK_JOIN_AN_EXISTING_CONVERSATION", "SLACK_INVITE_USER_TO_CHANNEL", "SLACK_ADMIN_CONVERSATIONS_INVITE", "SLACK_CREATE_CHANNEL", "SLACK_CONVERSATIONS_CREATE", "SLACK_ADMIN_CONVERSATIONS_CREATE", "SLACK_ACCEPT_SHARED_INVITE", "conversations.join"]) { + assert.match(composioApprovalDetails({ name, arguments: { channel: "C_PRIVATE", users: "U_BOT" } }, "composio-agent"), /C_PRIVATE/); + } + const details = composioApprovalDetails(invite, "composio-user"); + assert.match(details, /composio-user/); + assert.match(details, /U_BOT/); + assert.match(details, /requester/); + assert.match(details, /sha256:[a-f0-9]{64}/); + assert.equal(composioApprovalDetails({ name: "SLACK_FETCH_MESSAGE_THREAD_FROM_A_CONVERSATION", arguments: {} }, "composio-user"), null); + assert.equal(composioApprovalDetails({ name: "COMPOSIO_SEARCH_TOOLS", arguments: { queries: [{ use_case: "invite bot" }] } }, "composio-user"), null); + assert.equal(composioApprovalDetails({ name: "SLACK_SENDS_A_MESSAGE_TO_A_SLACK_CHANNEL", arguments: { text: "SLACK_INVITE_USER_TO_CHANNEL" } }, "composio-user"), null); +}); + +for (const identity of ["composio-user", "composio-agent"]) { + test(`${identity}: no upstream mutation before a click; deny/error/timeout fail closed`, async (t) => { + let decide; + const f = await fixture(t, { identity, requestApproval: () => new Promise((resolve) => { decide = resolve; }) }); + const pending = f.client.callTool(invite); + await tick(); + assert.equal(f.calls.length, 0); + assert.equal(f.cards.length, 1); + assert.equal(f.cards[0].approvalType, "agent", "Auto/Admin/remembered permission grants cannot approve agent requests"); + assert.equal(f.cards[0].channelId, "C_SOURCE"); + decide({ allow: false }); + assert.equal((await pending).isError, true); + assert.equal(f.calls.length, 0); + for (const decision of [undefined, { pending: true }, { allow: false, reason: "approval timed out" }]) { + const denied = await fixture(t, { identity, requestApproval: async () => decision }); + assert.equal((await denied.client.callTool(invite)).isError, true); + assert.equal(denied.calls.length, 0); + } + const error = await fixture(t, { identity, requestApproval: async () => { throw new Error("unavailable with secret"); } }); + const result = await error.client.callTool(invite); + assert.equal(result.isError, true); + assert.doesNotMatch(result.content[0].text, /with secret/); + }); +} + +test("approval executes the exact snapshot once; a repeated action requires another card", async (t) => { + const f = await fixture(t, { requestApproval: async () => ({ allow: true }) }); + for (let n = 0; n < 2; n++) assert.equal((await f.client.callTool(invite)).content[0].text, "executed"); + assert.deepEqual(f.calls, [invite, invite]); + assert.equal(f.cards.length, 2); +}); + +test("opaque executors fail closed without an unverifiable approval card", async (t) => { + const f = await fixture(t); + for (const name of ["COMPOSIO_REMOTE_WORKBENCH", "COMPOSIO_REMOTE_BASH_TOOL", "COMPOSIO_PROXY_EXECUTE", "SLACK_CUSTOM_API_CALL", "COMPOSIO_EXECUTE_UNKNOWN"]) { + assert.equal((await f.client.callTool({ name, arguments: { code: "saved_helper()" } })).isError, true); + } + assert.equal(f.cards.length, 0); + assert.equal(f.calls.length, 0); +}); + +test("reads continue while membership approval waits; revocation wins over a later click", async (t) => { + let decide, valid = true; + const f = await fixture(t, { + authorize: () => { if (!valid) throw new Error("revoked"); return claims; }, + requestApproval: () => new Promise((resolve) => { decide = resolve; }), + }); + const pending = f.client.callTool(invite); + await tick(); + assert.equal((await f.client.callTool({ name: "SLACK_FIND_CHANNELS", arguments: { query: "general" } })).content[0].text, "executed"); + valid = false; + decide({ allow: true }); + assert.equal((await pending).isError, true); + assert.equal(f.calls.length, 1); +}); + +test("cancelled and disconnected calls cannot execute after a late approval", async (t) => { + for (const disconnect of [false, true]) { + let decide; + const f = await fixture(t, { requestApproval: () => new Promise((resolve) => { decide = resolve; }) }); + const controller = new AbortController(); + const pending = f.client.callTool(invite, undefined, { signal: controller.signal }).catch(() => null); + await tick(); + if (disconnect) await f.client.close(); + else controller.abort(); + await pending; + await tick(); + decide({ allow: true }); + await tick(); + assert.equal(f.calls.length, 0); + } +}); + +test("malformed batch execution fails closed rather than forwarding opaque actions", async (t) => { + const f = await fixture(t); + for (const args of [{}, { tools: [{ arguments: {} }] }]) { + assert.equal((await f.client.callTool({ name: "COMPOSIO_MULTI_EXECUTE_TOOL", arguments: args })).isError, true); + } + assert.equal(f.calls.length, 0); +}); + +test("preview shows every channel/account alias and arrays; oversized previews are refused", () => { + const d = composioApprovalDetails({ name: "SLACK_INVITE_USER_TO_CHANNEL", arguments: { channel: "C_SAFE", channel_id: "C_OTHER", user_ids: ["U_A", "U_B"], account: "one", connected_account_id: "two" } }, "composio-user"); + for (const v of ["channel=C_SAFE", "channel_id=C_OTHER", "user_ids=[U_A, U_B]", "account=one", "connected_account_id=two"]) assert.ok(d.includes(v), v); + assert.throws(() => composioApprovalDetails({ name: "SLACK_INVITE_USER_TO_CHANNEL", arguments: { users: "U".repeat(200) } }, "composio-user")); +}); diff --git a/test/mcp-config.test.js b/test/mcp-config.test.js index dc03bf7..82564c8 100644 --- a/test/mcp-config.test.js +++ b/test/mcp-config.test.js @@ -7,7 +7,8 @@ ensureTestEnv(); process.env.CG_APPROVAL_SECRET = "mcp-config-signing-secret"; const { buildMcpConfig: buildRawMcpConfig } = await import("../src/gateway/mcp.js"); const { verifyGatewayCapability } = await import("../src/gateway/mcp-capability.js"); -const { workspaceRoot } = await import("../src/config/paths.js"); +const { lookupRemoteMcp, clearRemoteMcps } = await import("../src/mcp/remote-mcp-registry.js"); +const { workspaceRoot, runtimeSocketFile } = await import("../src/config/paths.js"); const { allowedFsRoot } = await import("../src/web/security.js"); const { createFakeRuntime } = await import("./fixtures/fake-runtime-backend.js"); const { localRuntimeTarget } = await import("../src/engines/runtime-target.js"); @@ -29,12 +30,20 @@ test("Claude MCP config exposes personal and agent Composio with separate creden composioToken: "ck_shared_secret", })); - assert.equal(config.mcpServers["composio-user"].type, "http"); - assert.equal(config.mcpServers["composio-user"].headers["x-consumer-api-key"], "ck_user_secret"); + const user = config.mcpServers["composio-user"]; + const shared = config.mcpServers["composio-agent"]; + const claims = verifyGatewayCapability(user.env.CG_GATEWAY_CAPABILITY, { secret: process.env.CG_APPROVAL_SECRET }).claims; + assert.match(user.args[0], /socket-bridge\.js$/); + assert.equal(user.args[1], "composio-user"); + assert.equal(user.env.CG_MCP_SOCKET, runtimeSocketFile()); + assert.equal(user.env.CG_MCP_SERVICE, "remote-mcp"); + assert.equal(lookupRemoteMcp(claims.jti, "composio-user").headers["x-consumer-api-key"], "ck_user_secret"); + assert.equal(lookupRemoteMcp(claims.jti, "composio-agent").headers["x-consumer-api-key"], "ck_shared_secret"); + assert.doesNotMatch(JSON.stringify(config), /ck_user_secret|ck_shared_secret/); + clearRemoteMcps(claims.jti); assert.equal(config.mcpServers["composio-user"].default_tools_approval_mode, "approve"); - assert.equal(config.mcpServers["composio-agent"].type, "http"); - assert.equal(config.mcpServers["composio-agent"].headers["x-consumer-api-key"], "ck_shared_secret"); + assert.equal(shared.args[1], "composio-agent"); assert.equal(config.mcpServers["composio-agent"].default_tools_approval_mode, "approve"); // The bare legacy name is gone: nothing may still be reachable as `mcp__composio__*`. assert.equal(config.mcpServers.composio, undefined); @@ -68,9 +77,10 @@ test("Claude MCP config bridges SDK sessions without exposing the organization k for (const name of ["composio-user", "composio-agent"]) { const server = config.mcpServers[name]; assert.equal(server.command, process.execPath); - assert.match(server.args[0], /composio-sdk-bridge\.js$/); + assert.match(server.args[0], /socket-bridge\.js$/); assert.match(server.args[1], /^https:\/\/app\.composio\.dev\//); - assert.equal(server.env.CHANNELGATE_DIR, process.env.CHANNELGATE_DIR); + assert.equal(server.env.CG_MCP_SERVICE, "composio-sdk"); + assert.equal(server.env.CG_MCP_SOCKET, runtimeSocketFile()); assert.equal(server.default_tools_approval_mode, "approve"); } assert.doesNotMatch(JSON.stringify(config), /sdk-super-secret|x-api-key/i); @@ -179,12 +189,12 @@ test("the daemon's own local target (or none) produces the checkout-script confi assert.doesNotMatch(none, /cg-mcp-bridge/, "the socket bridge is the image's, never a local child's"); }); -test("SDK-mode Composio rides the same socket bridge in a container, and the plain script when there is no target", async () => { +test("SDK-mode Composio rides the approval socket bridge on container and host runtimes", async () => { const endpoint = { mode: "sdk", url: "https://backend.composio.dev/api/v3/tool_router/session-1/mcp" }; const host = JSON.parse(await buildMcpConfig({ composioUserEndpoint: endpoint })); assert.equal(host.mcpServers["composio-user"].command, process.execPath); - assert.match(host.mcpServers["composio-user"].args[0], /composio-sdk-bridge\.js$/); - assert.ok(host.mcpServers["composio-user"].env.CHANNELGATE_DIR); + assert.match(host.mcpServers["composio-user"].args[0], /socket-bridge\.js$/); + assert.equal(host.mcpServers["composio-user"].env.CG_MCP_SOCKET, runtimeSocketFile()); const fake = createFakeRuntime(); const isolated = JSON.parse(await buildMcpConfig({ composioUserEndpoint: endpoint, target: fake.target() })); @@ -308,22 +318,24 @@ test("one malformed header drops only that server (announced), never the whole c } }); -test("a host target keeps today's direct http entries, byte for byte, and registers nothing", async () => { +test("a host target relays Composio for approvals and preserves direct toolbox entries", async () => { const { remoteMcpRegistryStats } = await import("../src/mcp/remote-mcp-registry.js"); const { toolboxUrl } = await import("../src/gateway/mcp-catalog.js"); const opts = { composioUserToken: "ck_user", toolboxToken: "tb-1", composioToken: "ck_shared", makeToolboxUrl: "https://eu1.make.com/mcp/server/x", makeToolboxKey: "mk" }; const before = remoteMcpRegistryStats().registrations; const host = JSON.parse(await buildMcpConfig(opts)); const local = JSON.parse(await buildMcpConfig({ ...opts, target: localRuntimeTarget("/work") })); - assert.equal(remoteMcpRegistryStats().registrations, before, "a host run never touches the relay registry"); + assert.equal(remoteMcpRegistryStats().registrations, before + 2, "each host run registers its Composio connections"); for (const config of [host, local]) { - assert.equal(JSON.stringify(config.mcpServers["composio-user"]), JSON.stringify({ type: "http", url: "https://connect.composio.dev/mcp", headers: { "x-consumer-api-key": "ck_user" }, default_tools_approval_mode: "approve" })); - assert.equal(JSON.stringify(config.mcpServers["composio-agent"]), JSON.stringify({ type: "http", url: "https://connect.composio.dev/mcp", headers: { "x-consumer-api-key": "ck_shared" }, default_tools_approval_mode: "approve" })); + assert.equal(config.mcpServers["composio-user"].env.CG_MCP_SERVICE, "remote-mcp"); + assert.equal(config.mcpServers["composio-agent"].env.CG_MCP_SERVICE, "remote-mcp"); assert.equal(JSON.stringify(config.mcpServers["makeitfuture-toolbox"]), JSON.stringify({ type: "http", url: toolboxUrl(), headers: { Authorization: "Bearer tb-1" } })); assert.equal(JSON.stringify(config.mcpServers["make-toolbox"]), JSON.stringify({ type: "http", url: "https://eu1.make.com/mcp/server/x", headers: { Authorization: "Bearer mk" } })); assert.deepEqual(Object.keys(config.mcpServers), ["gateway", "composio-user", "composio-agent", "makeitfuture-toolbox", "make-toolbox"]); const claims = verifyGatewayCapability(config.mcpServers.gateway.env.CG_GATEWAY_CAPABILITY, { secret: process.env.CG_APPROVAL_SECRET }).claims; - assert.equal(claims.remoteMcps, undefined, "a host capability carries no relay claim"); + assert.deepEqual(claims.remoteMcps, ["composio-user", "composio-agent"]); + assert.equal(lookupRemoteMcp(claims.jti, "make-toolbox"), null); + clearRemoteMcps(claims.jti); } }); diff --git a/test/mcp-socket-server.test.js b/test/mcp-socket-server.test.js index b001326..a8fb341 100644 --- a/test/mcp-socket-server.test.js +++ b/test/mcp-socket-server.test.js @@ -563,3 +563,32 @@ test("clear_my_composio_token / clear_my_toolbox_token drop every relay grant mi clearRemoteMcps(`theirs-${tool}`); } }); + +test("Composio membership calls on the real socket require an exact agent approval even in Auto/Admin", async (t) => { + const remote = fakeRemote(); + const cards = []; + let allow = false; + const socketPath = await serverOn(t, { approval: async (body) => { cards.push(body); return { allow }; } }, { connectRemote: remote.connectRemote }); + const jti = "relay-jti-membership-gate"; + const cap = capability({ remoteMcps: ["composio-user"], jti }); + registerRemoteMcps({ jti, exp: Date.now() + 60_000, servers: { "composio-user": RELAYED["composio-user"] } }); + t.after(() => clearRemoteMcps(jti)); + await saveChannelMeta(SLUG, { channelId: CHANNEL, platform: "slack", autoMode: true, adminMode: true }); + t.after(() => saveChannelMeta(SLUG, { channelId: CHANNEL, platform: "slack", allowBash: false, adminMode: false })); + await withBridgeClient(socketPath, { CG_GATEWAY_CAPABILITY: cap, CG_MCP_SERVICE: "remote-mcp" }, async (client) => { + const action = { name: "COMPOSIO_MULTI_EXECUTE_TOOL", arguments: { tools: [{ tool_slug: "SLACK_INVITE_USERS_TO_A_SLACK_CHANNEL", account: "requester", arguments: { channel: "C_DESTINATION", users: "U_BOT" } }] } }; + assert.equal((await client.callTool(action)).isError, true); + assert.equal(remote.calls.length, 0, "denial never reaches Slack"); + allow = true; + assert.equal((await client.callTool(action)).isError, undefined); + assert.deepEqual(remote.calls, [action], "approved exact call forwards once"); + assert.equal(cards.length, 2, "no remembered grant from a previous request"); + for (const card of cards) { + assert.equal(card.approvalType, "agent"); + assert.equal(card.authorId, AUTHOR); + assert.equal(card.channelId, CHANNEL); + assert.match(card.toolInput.details, /C_DESTINATION/); + assert.match(card.toolInput.details, /U_BOT/); + } + }, ["composio-user"]); +});