Stop a removed MCP connection from saving its server as a custom tool

After removing a Linear connection, connecting again could skip signing in
and save Linear as an unconnected custom tool: a client cached before the
removal still held the old tokens, and a late token write re-created a
connection for them. A token write for a named connection no longer creates
one, removing or disconnecting a connection drops its cached clients, and a
sign-in server is never saved without its connection.
This commit is contained in:
arc53-machine committed 2026-09-29 13:13:27 +01:00
1 parent 159ac03904
commit 56ababae71
5 files changed
+124 -6

No files matched your search

+36 -1
View File
@@ -355,11 +355,19 @@ class TestMCPServerSave:
"""Signed in before: the server answers with the saved tokens, no new handshake."""
from docsgpt.api.user.tools.mcp import MCPServerSave
from docsgpt.storage.db.repositories.connector_sessions import ConnectorSessionsRepository
connection = ConnectorSessionsRepository(pg_conn).create(
"u-signed-in", "mcp:https://mcp.linear.app", connector_key="mcp:linear", auth_kind="mcp_oauth",
display_name="Linear", account_label="Linear", server_url="https://mcp.linear.app",
)
signed_in = MagicMock()
signed_in.get_actions_metadata.return_value = [{"name": "search"}]
with _patch_db(pg_conn), patch(
"docsgpt.api.user.tools.mcp.MCPTool", return_value=signed_in,
), patch("docsgpt.api.user.tools.mcp._mcp_connection", return_value=None), app.test_request_context(
), patch(
"docsgpt.api.user.tools.mcp._mcp_connection", return_value=str(connection["id"]),
), app.test_request_context(
"/api/mcp_server/save", method="POST",
json={
"displayName": "Linear",
@@ -446,6 +454,33 @@ class TestMCPServerSave:
assert team["filled_by_llm"] is False and team["value"] == "ENG"
class TestSignInServerWithoutAConnection:
def test_an_oauth_server_is_not_saved_without_its_connection(self, app, pg_conn):
"""A client still cached from a removed connection can answer the
discovery; the tool must not then be saved as an unconnected server."""
from docsgpt.api.user.tools.mcp import MCPServerSave
from docsgpt.storage.db.repositories.user_tools import UserToolsRepository
user = "u-mcp-no-connection"
fake_tool = MagicMock()
fake_tool.get_actions_metadata.return_value = [{"name": "list_issues", "parameters": {"properties": {}}}]
with _patch_db(pg_conn), patch(
"docsgpt.api.user.tools.mcp.MCPTool", return_value=fake_tool,
), app.test_request_context(
"/api/mcp_server/save", method="POST",
json={
"displayName": "Linear",
"config": {"transport_type": "http", "server_url": "https://mcp.linear.app/mcp", "auth_type": "oauth"},
"status": True,
},
):
from flask import request
request.decoded_token = {"sub": user}
response = MCPServerSave().post()
assert response.status_code == 400
assert UserToolsRepository(pg_conn).list_for_user(user) == []
class TestMCPOAuthCallback:
def test_error_param_redirects_error(self, app):
from docsgpt.api.user.tools.mcp import MCPOAuthCallback
+35 -4
View File
@@ -521,13 +521,44 @@ class TestMcpConnectionScope:
def test_writes_never_land_on_another_servers_connection(self, pg_conn):
cid = self._mcp(pg_conn)
with _patch_service_db(pg_conn):
row = service.update_mcp_secrets(
"alice", "https://attacker.example", {"tokens": {"access_token": "planted"}}, connection_id=cid,
)
assert str(row["id"]) != cid
with pytest.raises(service.ConnectionUnavailable):
service.update_mcp_secrets(
"alice", "https://attacker.example", {"tokens": {"access_token": "planted"}}, connection_id=cid,
)
assert service.read_mcp_secrets("alice", "https://mcp.notion.com", cid)["tokens"]["access_token"] == (
"alice-mcp-token"
)
assert service.read_mcp_secrets("alice", "https://attacker.example") == {}
def test_a_removed_connection_is_not_brought_back_by_a_late_token_write(self, pg_conn):
"""A client or sync still running when its connection is removed may
renew the tokens afterwards; that must not re-create the connection."""
cid = self._mcp(pg_conn)
with _patch_service_db(pg_conn):
service.remove_connection(pg_conn, service.ConnectorSessionsRepository(pg_conn).get(cid))
with pytest.raises(service.ConnectionUnavailable):
service.update_mcp_secrets(
"alice", "https://mcp.notion.com", {"tokens": {"access_token": "renewed"}}, connection_id=cid,
)
assert service.read_mcp_secrets("alice", "https://mcp.notion.com") == {}
def test_removing_a_connection_forgets_its_cached_mcp_clients(self, pg_conn):
import docsgpt.api.user # noqa: F401 (loads mcp_tool without the circular import)
from docsgpt.agents.tools import mcp_tool
cid = self._mcp(pg_conn)
mcp_tool._mcp_clients_cache.update({
f"https://mcp.notion.com/mcp#http#oauth:{cid}:DocsGPT:none:cb": {"client": object(), "created_at": 0},
"https://mcp.notion.com/mcp#http#oauth:alice:DocsGPT:none:cb": {"client": object(), "created_at": 0},
"https://mcp.notion.com/mcp#http#oauth:bob:DocsGPT:none:cb": {"client": object(), "created_at": 0},
})
try:
with _patch_service_db(pg_conn):
service.remove_connection(pg_conn, service.ConnectorSessionsRepository(pg_conn).get(cid))
keys = [k for k in mcp_tool._mcp_clients_cache if "mcp.notion.com" in k]
assert keys == ["https://mcp.notion.com/mcp#http#oauth:bob:DocsGPT:none:cb"]
finally:
mcp_tool._mcp_clients_cache.clear()
def test_mcp_routes_drop_client_supplied_connection_id(self):
"""Only the tool executor may pick the connection whose tokens a tool uses."""