Skip to content

Removing a connector leaves its grants, so adding the same app back puts every action on the same Bots again #571

Description

@zopeVaibhav

A grant is a row in plugin_grants whose ref is plain text, and nothing removes those rows when
the connector they name is removed. An administrator who takes an app away and adds it back gives
every Bot that held its actions all of them again, without granting anything.

What it looks like

removeServer is thorough about everything except the grants (server/src/plugins/store.ts:3282).
It revokes the server's configured credential, revokes every person's stored token, revokes each
brokered account at Composio and deletes the composio_connections rows, deletes the mcp_servers
row — which cascades mcp_tools — and writes configuration.changed. plugin_grants is never
touched, so every mcp grant for that server survives its server.

They are inert while the app is gone, because listForAgent matches a grant against a tool row that
no longer exists (server/src/plugins/store.ts:4488). They are also invisible. The one surface that
reports a grant on a tool nothing advertises builds that list per server row
(server/src/plugins/store.ts:3906, read back by listServers at server/src/plugins/store.ts:3961),
and after a removal there is no row to report under. Nothing else lists a grant by ref, so between
the two acts the rows exist only in the table.

A brokered app is added back at the id it had: addBrokeredApp mints composio-<slug>
(server/src/plugins/store.ts:3004), and the actions come back under their own names, so the refs
the surviving grants name resolve again. The connector's own documentation states the opposite
outcome — "Re-adding the app afterwards starts empty rather than silently restoring everybody who
had connected before" (docs/plugins/composio.md:467) — which is true of the consent rows it is
written about and not true of the grants.

Why it happens

The schema says why the column has no foreign key: a refresh deletes and rewrites every tool row a
server has, so a cascade would empty a Bot's grants on an ordinary refresh
(server/src/db/schema/plugins.ts:499). That argument is about a tool list being replaced. Nothing
covers the server row being deleted for good, and the deployment already draws that distinction
elsewhere — uninstallSkill removes a skill's grants in the same transaction that removes the skill
(server/src/plugins/store.ts:4337).

Reproduction

Against a migrated test database at 84e5b67, driving the plugin store directly:

  1. Enable a brokered app, and grant one of its actions to a Bot. listForAgent offers it.
  2. Remove the app with removeServer.
  3. The grant row is still in plugin_grants, and listServers reports no server it belongs to.
  4. Add the same app again. Nobody grants anything.
  5. decide("mcp", ref, botId) answers allowed, and listForAgent offers the action again.

For an app that needs no authentication — 34 of the catalogue — step 5 is a working call: no account
is required, so callTool succeeds immediately after the re-add.

Why it matters

Removing and re-adding is ordinary administration, and with a broker it is a two-click act on a
picker. The unit is an app rather than a tool: Slack publishes 167 actions, and a grant screen that
was carefully ticked through once is restored wholesale by an act that says nothing about grants. For
an app people connect accounts to, the restored grant waits for the first person to reconnect and
then acts; for an app needing no account, it acts at once. Either way the trail shows no grant,
because none was made, and the audit row for the removal does not say what it released.

What a fix probably has to do

Delete the server's mcp grants in the same step that deletes the server row, and say on the removal
row which refs went and from which Bots. Deployments that have already removed a granted connector
are carrying the rows now, so the fix wants a migration dropping grants whose server half names no
mcp_servers row. Only mcp grants: skill and bot grants share the table.

Filtering at read time instead would leave the rows, and the rows are what an administrator reads to
answer what a Bot may do.

Severity

Medium. It needs an app to be removed while granted and then added back under the same name, which is
the ordinary way to reinstate one. It grants no Bot anything an administrator had not granted at some
point, so this is a grant returning without being asked for rather than a new capability appearing —
and every call it enables is still decided, policy-evaluated and audited.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions