Let a GitHub connection opt into write tools

A GitHub connection's MCP tool can now point at GitHub's full endpoint
(/mcp/) instead of the read-only one when its owner opts in, at setup
(allow_writes) or later (PUT /api/connections/<id>/writes), which re-reads
the actions and keeps the choices for those on both endpoints. Actions from
the write endpoint are writes unless GitHub marks them read-only, so they
default to asking first.

Admins can forbid it per connector (allow_writes in Admin > Connectors,
kept in app_metadata). Then the option is refused, a refresh goes back to
read-only, and at run time the tool only ever calls the read-only endpoint
and write calls are denied with a reason.
This commit is contained in:
arc53-machine committed 2026-09-29 11:22:01 +01:00
1 parent dba2de590b
commit 12ad4f5e85
9 files changed
+706 -38

No files matched your search

+29
View File
@@ -1033,6 +1033,29 @@ class ToolExecutor:
or require_approval
)
# An admin forbade changes through this connector (GitHub): its tool
# already calls the read-only endpoint, so say why instead of failing.
if resolved is not None and not resolved.writes_allowed:
from docsgpt.connectors.permissions import ACCESS_WRITE, action_access
if action_access(tool_data.get("name"), action_data) == ACCESS_WRITE:
return {
"call_id": call_id,
"name": llm_name,
"tool_name": tool_data.get("name", "unknown"),
"tool_id": tool_id,
"action_name": action_name,
"llm_name": llm_name,
"arguments": arguments,
"pause_type": "headless_denied",
"deny_reason": (
f"An admin turned off changes through {resolved.connector_name or 'this service'}. "
"It can only look things up."
),
"error_type": "tool_not_allowed",
"thought_signature": getattr(call, "thought_signature", None),
}
# A member running someone else's account (a shared tool in owner
# mode) always confirms write actions, whatever the owner chose for
# themselves.
@@ -1793,6 +1816,12 @@ class ToolExecutor:
f"{resolved.connector_name or 'This service'} needs to be connected",
connection_id=resolved.connection_id,
)
if service.builtin_mcp_config(definition) is not None:
# Only the connector's own endpoints: GitHub's write one while
# the tool opted in and an admin allows it, else read-only.
tool_config["server_url"] = service.builtin_mcp_url(
definition, tool_config.get("server_url"), resolved.writes_allowed,
)
audit_delegation(
resolved,
invoker=self.user,
+22 -1
View File
@@ -23,6 +23,7 @@ from docsgpt.storage.db.repositories.connector_policies import (
ALLOW_CUSTOM_MCP_KEY,
CREDENTIAL_POLICIES,
ConnectorPoliciesRepository,
allow_writes_key,
)
from docsgpt.storage.db.session import db_readonly, db_session
@@ -57,6 +58,7 @@ class AdminConnectorsResource(Resource):
"""Every connector with its policy, setup state and connection count."""
with db_readonly() as conn:
policies = ConnectorPoliciesRepository(conn).all()
loaded = service.load_policies(conn)
allow_custom = service.custom_mcp_allowed(conn)
counts = _connection_counts(conn)
connectors = []
@@ -88,6 +90,11 @@ class AdminConnectorsResource(Resource):
"connection_count": counts.get(definition.key, 0),
"docs_url": definition.docs_url,
"mcp_url": definition.mcp_url,
# Whether members may let agents make changes (GitHub);
# None where the connector offers no such choice.
"allow_writes": (
service.writes_allowed(loaded, definition.key) if definition.mcp_write_url else None
),
}
)
return make_response(
@@ -106,7 +113,7 @@ class AdminConnectorsResource(Resource):
@admin_required
def put(self):
"""Update policies: {policies: {key: {enabled?, credential_mode?}}, allow_custom_mcp?}."""
"""Update policies: {policies: {key: {enabled?, credential_mode?, allow_writes?}}, allow_custom_mcp?}."""
body = request.get_json(silent=True) or {}
updates = body.get("policies") or {}
if not isinstance(updates, dict):
@@ -119,6 +126,15 @@ class AdminConnectorsResource(Resource):
return make_response(jsonify({"success": False, "message": "Unknown credential mode"}), 400)
if "enabled" in change and not isinstance(change["enabled"], bool):
return make_response(jsonify({"success": False, "message": "enabled must be true or false"}), 400)
if "allow_writes" in change:
if not catalog.get_definition(key).mcp_write_url:
return make_response(
jsonify({"success": False, "message": f"{key} has no write access to allow"}), 400,
)
if not isinstance(change["allow_writes"], bool):
return make_response(
jsonify({"success": False, "message": "allow_writes must be true or false"}), 400,
)
allow_custom = body.get("allow_custom_mcp")
if allow_custom is not None and not isinstance(allow_custom, bool):
return make_response(jsonify({"success": False, "message": "allow_custom_mcp must be a boolean"}), 400)
@@ -126,6 +142,11 @@ class AdminConnectorsResource(Resource):
with db_session() as conn:
repo = ConnectorPoliciesRepository(conn)
for key, change in updates.items():
if "allow_writes" in change:
value = "true" if change["allow_writes"] else "false"
AppMetadataRepository(conn).set(allow_writes_key(key), value)
if set(change) == {"allow_writes"}:
continue
repo.upsert(
key,
enabled=change.get("enabled"),
+51 -4
View File
@@ -229,8 +229,9 @@ class ConnectionDisconnect(Resource):
class ConnectionSetup(Resource):
@api.doc(
description=(
"Apply the connect wizard's choices: {create_tools, tool_permissions?, "
"sync?: {items, frequency, name?}}. Honours an Idempotency-Key header for the sync."
"Apply the connect wizard's choices: {create_tools, allow_writes?, tool_permissions?, "
"sync?: {items, frequency, name?}}. allow_writes points GitHub's tool at its write endpoint. "
"Honours an Idempotency-Key header for the sync."
)
)
def post(self, connection_id: str):
@@ -239,22 +240,31 @@ class ConnectionSetup(Resource):
return _unauthorized()
body = _json_body()
create_tools = body.get("create_tools", True)
allow_writes = body.get("allow_writes", False)
if not isinstance(allow_writes, bool):
return _error("allow_writes must be true or false", 400)
try:
mcp_actions = None
with db_readonly() as conn:
row = _owned(conn, connection_id, user_id)
discover = bool(row) and create_tools and service.needs_mcp_discovery(conn, row)
forbidden = bool(row) and allow_writes and not service.writes_allowed(
service.load_policies(conn), catalog.connector_key_for_row(row),
)
if row is None:
return _not_found()
if service.normalize_status(row) != service.STATUS_CONNECTED:
return _error("Reconnect before setting up", 409, code="reconnect")
if forbidden:
return _error("Changes through this connector are turned off by an admin", 403,
code="writes_forbidden")
if discover:
# GitHub's tool is its MCP server: read its actions before
# the write transaction, not while holding it open.
from docsgpt.connectors.mcp import discover_builtin_actions
try:
mcp_actions = discover_builtin_actions(user_id, row)
mcp_actions = discover_builtin_actions(user_id, row, writes=allow_writes)
except service.ConnectionUnavailable:
return _error("Reconnect before setting up", 409, code="reconnect")
except Exception as err:
@@ -269,7 +279,7 @@ class ConnectionSetup(Resource):
if create_tools:
tools = service.ensure_connection_tools(
conn, user_id, row, permissions=body.get("tool_permissions") or None,
mcp_actions=mcp_actions,
mcp_actions=mcp_actions, mcp_writes=allow_writes,
)
account_parameters = service.connection_parameters(row)
tool_payload = [service.serialize_tool(tool, account_parameters) for tool in tools]
@@ -587,6 +597,43 @@ class ConnectionRefreshTools(Resource):
return make_response(jsonify({"success": True, **diff}), 200)
@connections_ns.route("/connections/<string:connection_id>/writes")
class ConnectionWrites(Resource):
@api.doc(
description=(
"GitHub: let agents make changes through this connection, or only read: {allow}. "
"Re-reads the tool's actions from the matching endpoint. Owner only."
)
)
def put(self, connection_id: str):
from docsgpt.connectors.mcp import NoMcpTool, set_builtin_writes
user_id = _user_id()
if not user_id:
return _unauthorized()
allow = _json_body().get("allow")
if not isinstance(allow, bool):
return _error("allow must be true or false", 400)
with db_readonly() as conn:
row = _owned(conn, connection_id, user_id)
if row is None:
return _not_found()
try:
result = set_builtin_writes(user_id, row, allow)
except service.WritesForbidden as err:
return _error(str(err), 403, code="writes_forbidden")
except NoMcpTool as err:
return _error(str(err), 409, code="no_tools")
except ValueError as err:
return _error(str(err), 400)
except service.ConnectionUnavailable:
return _error("Reconnect to continue", 409, code="reconnect")
except Exception as err:
current_app.logger.error(f"Error switching write access: {err}", exc_info=True)
return _error("The service's tools could not be reached. Try again.", 502, code="tools_unavailable")
return make_response(jsonify({"success": True, **result}), 200)
@connections_ns.route("/connections/tools/<string:tool_id>/credential-mode")
class ToolCredentialMode(Resource):
@api.doc(
+10 -1
View File
@@ -89,6 +89,10 @@ class ConnectorDefinition:
(created and enabled), ``ask`` or ``off``; ``sync`` likewise.
mcp_url: MCP endpoint for presets, and for a built-in connector whose
tool is its service's own MCP server (GitHub).
mcp_write_url: The same server's endpoint that also offers write
actions, which a connection opts into and an admin can forbid
(GitHub's full server beside its read-only one). ``mcp_url``
stays the default.
publisher: ``built_in``, ``preset`` or ``custom``.
docs_url: Setup guide for admins.
oauth_scopes: Scopes an MCP preset requests.
@@ -115,6 +119,7 @@ class ConnectorDefinition:
tool_templates: tuple[str, ...] = ()
setup: dict = field(default_factory=lambda: {"tools": "auto", "sync": "ask"})
mcp_url: Optional[str] = None
mcp_write_url: Optional[str] = None
publisher: str = "built_in"
docs_url: Optional[str] = None
oauth_scopes: tuple[str, ...] = ()
@@ -163,6 +168,7 @@ class ConnectorDefinition:
"tool_templates": list(self.tool_templates),
"setup": dict(self.setup),
"mcp_url": self.mcp_url,
"writes_opt_in": bool(self.mcp_write_url),
"publisher": self.publisher,
"docs_url": self.docs_url,
"oauth_scopes": list(self.oauth_scopes),
@@ -181,6 +187,7 @@ def base_url(url: Optional[str]) -> str:
_DOCS = "https://docs.docsgpt.cloud/Guides/Connectors"
GITHUB_MCP_URL = "https://api.githubcopilot.com/mcp/readonly"
GITHUB_MCP_WRITE_URL = "https://api.githubcopilot.com/mcp/"
# Tool templates any connector's server can use (an MCP server, an OpenAPI
# spec): a tool made from one belongs to its connection, not to a connector.
@@ -239,9 +246,11 @@ _BUILT_IN: tuple[ConnectorDefinition, ...] = (
oauth_settings=("GITHUB_CLIENT_ID", "GITHUB_CLIENT_SECRET", "GITHUB_APP_SLUG"),
sync_ingestor="github",
setup_fields=(CredentialField("repo_url", "Repository", secret=False),),
# GitHub's own MCP server, read-only: agents look things up, never change them.
# GitHub's own MCP server, read-only unless the connection opts into
# changes (issues, comments, pull requests) and an admin allows them.
tool_templates=("mcp_tool",),
mcp_url=GITHUB_MCP_URL,
mcp_write_url=GITHUB_MCP_WRITE_URL,
setup={"tools": "ask", "sync": "ask"},
docs_url=f"{_DOCS}#github",
),
+132 -27
View File
@@ -1,15 +1,38 @@
"""MCP helpers for connections: re-scanning a server's actions."""
"""MCP helpers for connections: re-scanning a server's actions, switching GitHub's writes."""
from __future__ import annotations
from typing import Optional
from docsgpt.agents.tool_pins import carry_pins
from docsgpt.connectors import catalog, service
from docsgpt.connectors.permissions import apply_default_permissions
from docsgpt.connectors.catalog import ConnectorDefinition
from docsgpt.connectors.permissions import ACCESS_READ, ACCESS_WRITE, apply_default_permissions
from docsgpt.storage.db.repositories.connector_sessions import ConnectorSessionsRepository
from docsgpt.storage.db.repositories.user_tools import UserToolsRepository
from docsgpt.storage.db.session import db_readonly, db_session
class NoMcpTool(LookupError):
"""The connection has no MCP tool yet (it was set up without tools)."""
def _builtin_definition(connection: dict) -> Optional[ConnectorDefinition]:
"""The connection's built-in connector when its tool is that service's MCP server."""
definition = catalog.get_definition(catalog.connector_key_for_row(connection))
return definition if service.builtin_mcp_config(definition) is not None else None
def _write_endpoint_access(action: dict) -> dict:
"""Stamp an action of a write endpoint: a read only when the server marks it read-only.
GitHub marks its read tools, so a name is no evidence there: from the name
alone ``mark_all_notifications_read`` would pass for a read.
"""
annotations = action.get("annotations") if isinstance(action.get("annotations"), dict) else {}
return {**action, "access": ACCESS_READ if annotations.get("readOnlyHint") is True else ACCESS_WRITE}
def _discover(user_id: str, connection: dict, tool: dict) -> list[dict]:
from docsgpt.agents.tools.mcp_tool import MCPTool
@@ -26,56 +49,138 @@ def _discover(user_id: str, connection: dict, tool: dict) -> list[dict]:
return mcp_tool.get_actions_metadata()
def discover_builtin_actions(user_id: str, connection: dict) -> list[dict]:
def _classified(connection: dict, config: dict, actions: list[dict]) -> list[dict]:
"""``actions`` as read from ``config``'s server, stamped strictly when it is a write endpoint."""
definition = _builtin_definition(connection)
if definition is None or not definition.mcp_write_url or config.get("server_url") != definition.mcp_write_url:
return actions
return [_write_endpoint_access(action) for action in actions]
def discover_builtin_actions(user_id: str, connection: dict, writes: bool = False) -> list[dict]:
"""The actions of a built-in connector's MCP server (GitHub's), read with the connection.
Args:
user_id: The connection's owner.
connection: The connection row.
writes: Read the endpoint that also offers write actions; the caller
checked that an admin allows them.
Raises:
ValueError: The connector has no MCP server of its own.
service.ConnectionUnavailable: The connection needs reconnecting.
Exception: The server could not be reached or listed.
"""
definition = catalog.get_definition(catalog.connector_key_for_row(connection))
config = service.builtin_mcp_config(definition)
config = service.builtin_mcp_config(_builtin_definition(connection), writes=writes)
if config is None:
raise ValueError("This connector has no MCP server")
return _discover(user_id, connection, {"config": config})
return _classified(connection, config, _discover(user_id, connection, {"config": config}))
def refresh_mcp_tools(user_id: str, connection: dict) -> dict:
"""Re-read the actions of every MCP tool on a connection.
def _rediscover(user_id: str, connection: dict, tool: dict, config: dict) -> tuple[set, set, dict]:
"""Re-read one MCP tool's actions from ``config``'s server and store both.
Actions that still exist keep the permissions the user chose and the
fixed values of parameters they still have; new ones get the defaults
from their annotations (reads always allowed, writes need approval);
removed ones disappear.
Returns:
``(added, removed, tool)``: action names, and the serialized tool.
"""
discovered = _classified(connection, config, _discover(user_id, connection, {**tool, "config": config}))
fresh = apply_default_permissions("mcp_tool", service._transform_actions(discovered))
previous = {a.get("name"): a for a in (tool.get("actions") or []) if isinstance(a, dict)}
merged = []
for action in fresh:
old = previous.get(action.get("name"))
if old is not None:
action = {
**carry_pins(old, action),
"active": old.get("active", True),
"require_approval": bool(old.get("require_approval")),
}
merged.append(action)
names = {a.get("name") for a in merged}
fields = {"actions": merged}
if config != (tool.get("config") or {}):
fields["config"] = config
with db_session() as conn:
repo = UserToolsRepository(conn)
repo.update(str(tool["id"]), user_id, fields)
serialized = service.serialize_tool(repo.get_any(str(tool["id"]), user_id))
return names - set(previous), set(previous) - names, serialized
def refresh_mcp_tools(user_id: str, connection: dict) -> dict:
"""Re-read the actions of every MCP tool on a connection.
A built-in connector's tool is re-read from its own endpoint: the write
one while it is set up for writes and an admin allows them, the read-only
one otherwise, which it then keeps.
Returns:
``{"added": [...], "removed": [...], "tools": [...]}``.
"""
with db_readonly() as conn:
tools = [t for t in ConnectorSessionsRepository(conn).list_tools(str(connection["id"]))
if t.get("name") == "mcp_tool"]
policies = service.load_policies(conn) if tools else {}
definition = _builtin_definition(connection)
added: set[str] = set()
removed: set[str] = set()
refreshed = []
for tool in tools:
fresh = apply_default_permissions("mcp_tool", service._transform_actions(_discover(user_id, connection, tool)))
previous = {a.get("name"): a for a in (tool.get("actions") or []) if isinstance(a, dict)}
merged = []
for action in fresh:
old = previous.get(action.get("name"))
if old is not None:
action = {
**carry_pins(old, action),
"active": old.get("active", True),
"require_approval": bool(old.get("require_approval")),
}
merged.append(action)
names = {a.get("name") for a in merged}
added |= names - set(previous)
removed |= set(previous) - names
with db_session() as conn:
repo = UserToolsRepository(conn)
repo.update(str(tool["id"]), user_id, {"actions": merged})
refreshed.append(service.serialize_tool(repo.get_any(str(tool["id"]), user_id)))
config = dict(tool.get("config") or {})
if definition is not None:
config["server_url"] = service.builtin_mcp_url(
definition, config.get("server_url"), service.writes_allowed(policies, definition.key),
)
tool_added, tool_removed, serialized = _rediscover(user_id, connection, tool, config)
added |= tool_added
removed |= tool_removed
refreshed.append(serialized)
return {"added": sorted(added), "removed": sorted(removed), "tools": refreshed}
def set_builtin_writes(user_id: str, connection: dict, allow: bool) -> dict:
"""Point a connection's built-in MCP tool (GitHub's) at its write or read-only endpoint.
The tool's actions are re-read from the new endpoint, keeping the
choices made for actions that exist on both.
Args:
user_id: The connection's owner.
connection: The connection row.
allow: Let agents make changes (the write endpoint) or only read.
Returns:
``{"added", "removed", "tools", "writes"}``.
Raises:
ValueError: The connector offers no write access.
NoMcpTool: The connection has no such tool yet.
service.WritesForbidden: ``allow`` while an admin forbids writes.
service.ConnectionUnavailable: The connection needs reconnecting.
"""
definition = _builtin_definition(connection)
if definition is None or not definition.mcp_write_url:
raise ValueError("This connector has no write access to turn on")
with db_readonly() as conn:
tools = [t for t in ConnectorSessionsRepository(conn).list_tools(str(connection["id"]))
if t.get("name") == "mcp_tool"]
allowed = service.writes_allowed(service.load_policies(conn), definition.key)
if not tools:
raise NoMcpTool("Add this connection's tools first")
if allow and not allowed:
raise service.WritesForbidden(f"An admin turned off changes through {definition.name}")
added: set[str] = set()
removed: set[str] = set()
refreshed = []
for tool in tools:
config = {**(tool.get("config") or {}), **service.builtin_mcp_config(definition, writes=allow)}
tool_added, tool_removed, serialized = _rediscover(user_id, connection, tool, config)
added |= tool_added
removed |= tool_removed
refreshed.append(serialized)
return {"added": sorted(added), "removed": sorted(removed), "tools": refreshed, "writes": allow}
+11
View File
@@ -34,6 +34,8 @@ class ResolvedConnection:
connector_key: Catalog key of the service.
connector_name: Name shown to the user ("Notion", or a custom label).
delegated: The row belongs to someone other than the invoker.
writes_allowed: Whether an admin lets agents make changes through a
connector that offers them as an opt-in (GitHub); True elsewhere.
"""
row: Optional[dict]
@@ -41,6 +43,7 @@ class ResolvedConnection:
connector_key: Optional[str]
connector_name: Optional[str]
delegated: bool = False
writes_allowed: bool = True
@property
def connection_id(self) -> Optional[str]:
@@ -104,9 +107,17 @@ def resolve_connection(resource: dict, invoker_user_id: Optional[str]) -> Option
connector_key=key,
connector_name=_name_for(row or owned, key),
delegated=bool(row and invoker_user_id and row.get("user_id") != invoker_user_id),
writes_allowed=_writes_allowed(policies, key),
)
def _writes_allowed(policies: dict, key: Optional[str]) -> bool:
definition = catalog.get_definition(key) if key else None
if definition is None or not definition.mcp_write_url:
return True
return service.writes_allowed(policies, key)
def _member_connection(repo: ConnectorSessionsRepository, owned: Optional[dict], invoker: str) -> Optional[dict]:
"""The invoker's own connection to the service the owner's connection is for.
+78 -5
View File
@@ -304,6 +304,7 @@ def connection_detail(conn, row: dict) -> dict:
detail = serialize_connection(row, {"sources": len(sources), "tools": len(tools)})
detail["sources"] = sources
detail["tools"] = tools
detail["writes"] = builtin_writes(catalog.get_definition(detail["connector_key"]), tool_rows)
return detail
@@ -361,6 +362,7 @@ def catalog_for_user(conn, user_id: str, *, is_admin: bool, policies: Optional[d
"status": status,
"state": _card_state(definition, available, disabled, status),
"credential_policy": policy.get("credential_mode") or "choose",
"writes_allowed": writes_allowed(policies, definition.key),
}
)
return entries
@@ -1269,12 +1271,17 @@ def split_secrets(config: dict, config_requirements: dict) -> tuple[dict, dict]:
return public, secrets
def builtin_mcp_config(definition: Optional[ConnectorDefinition]) -> Optional[dict]:
def builtin_mcp_config(definition: Optional[ConnectorDefinition], writes: bool = False) -> Optional[dict]:
"""Tool config of a built-in connector whose tool is its service's MCP server.
The token is not part of it: the executor reads it from the connection
at run time and sends it as a bearer token.
Args:
definition: The connector.
writes: Use the endpoint that also offers write actions, when the
connector has one (GitHub's full server).
Returns:
The config, or None when the connector has no such tool.
"""
@@ -1285,7 +1292,40 @@ def builtin_mcp_config(definition: Optional[ConnectorDefinition]) -> Optional[di
or "mcp_tool" not in definition.tool_templates
):
return None
return {"server_url": definition.mcp_url, "auth_type": "bearer", "transport_type": "http", "timeout": 30}
url = definition.mcp_write_url if writes and definition.mcp_write_url else definition.mcp_url
return {"server_url": url, "auth_type": "bearer", "transport_type": "http", "timeout": 30}
def builtin_mcp_url(definition: Optional[ConnectorDefinition], stored_url: Optional[str], allowed: bool) -> str:
"""The endpoint a built-in connector's MCP tool may call, whatever its config says.
Only the connector's own endpoints are ever used: the write endpoint when
the tool was set up for writes and an admin allows them, the read-only
one otherwise (another path on the same host included).
Args:
definition: The connector, with ``mcp_url`` set.
stored_url: The ``server_url`` in the tool's config.
allowed: Whether an admin allows writes through this connector.
"""
if allowed and definition.mcp_write_url and stored_url == definition.mcp_write_url:
return definition.mcp_write_url
return definition.mcp_url or ""
def builtin_writes(definition: Optional[ConnectorDefinition], tool_rows: Iterable[dict]) -> Optional[bool]:
"""Whether a connection's built-in MCP tool is set up for writes.
Returns:
None when the connector offers no writes or the connection has no such
tool yet; otherwise whether the tool uses the write endpoint.
"""
if definition is None or not definition.mcp_write_url or builtin_mcp_config(definition) is None:
return None
for tool in tool_rows:
if tool.get("name") == "mcp_tool":
return (_json(tool.get("config")) or {}).get("server_url") == definition.mcp_write_url
return None
def needs_mcp_discovery(conn, connection: dict) -> bool:
@@ -1303,6 +1343,7 @@ def ensure_connection_tools(
connection: dict,
permissions: Optional[dict] = None,
mcp_actions: Optional[list] = None,
mcp_writes: bool = False,
) -> list[dict]:
"""Create the connector's tools once; later calls return the existing ones.
@@ -1317,6 +1358,8 @@ def ensure_connection_tools(
mcp_actions: The actions of a built-in connector's MCP server
(GitHub's), discovered by the caller outside this transaction.
Without them that tool is not created.
mcp_writes: Point that tool at the endpoint that also offers writes;
the caller checked that an admin allows them.
"""
repo = ConnectorSessionsRepository(conn)
existing = repo.list_tools(str(connection["id"]))
@@ -1325,7 +1368,7 @@ def ensure_connection_tools(
if not definition or not definition.tool_templates:
return existing
have = {tool.get("name") for tool in existing}
mcp_config = builtin_mcp_config(definition)
mcp_config = builtin_mcp_config(definition, writes=mcp_writes)
created = []
for template in definition.tool_templates:
if template in have:
@@ -1522,6 +1565,10 @@ class ConnectorDisabled(Exception):
"""An admin turned this connector (or custom MCP servers) off."""
class WritesForbidden(Exception):
"""An admin forbade write actions through this connector (GitHub's changes)."""
def custom_mcp_allowed(conn) -> bool:
"""The instance-wide "Allow custom MCP servers" switch (on unless turned off)."""
from docsgpt.storage.db.repositories.app_metadata import AppMetadataRepository
@@ -1549,15 +1596,41 @@ def connector_is_enabled(policies: dict, connector_key: Optional[str]) -> bool:
def load_policies(conn) -> dict[str, dict]:
"""Every connector's policy, with custom MCP folded in from its switch."""
from docsgpt.storage.db.repositories.connector_policies import ConnectorPoliciesRepository
"""Every connector's policy, with custom MCP and write switches folded in.
A connector that offers writes (GitHub) gets ``allow_writes``: False when
an admin forbade them.
"""
from docsgpt.storage.db.repositories.app_metadata import AppMetadataRepository
from docsgpt.storage.db.repositories.connector_policies import ConnectorPoliciesRepository, allow_writes_key
policies = dict(ConnectorPoliciesRepository(conn).all())
if not custom_mcp_allowed(conn):
policies["custom_mcp"] = {**policies.get("custom_mcp", {}), "enabled": False}
metadata = AppMetadataRepository(conn)
for definition in catalog.all_definitions():
if definition.mcp_write_url and metadata.get(allow_writes_key(definition.key)) == "false":
policies[definition.key] = {**policies.get(definition.key, {}), "allow_writes": False}
return policies
def writes_allowed(policies: dict, connector_key: Optional[str]) -> bool:
"""Whether a connection to this connector may be set up for write actions.
Args:
policies: ``connector_key`` to policy row, from :func:`load_policies`.
connector_key: The catalog key.
Returns:
False for a connector that offers no writes, or whose writes an admin
forbade.
"""
definition = catalog.get_definition(connector_key) if connector_key else None
if definition is None or not definition.mcp_write_url:
return False
return (policies.get(connector_key) or {}).get("allow_writes") is not False
def connector_enabled(conn, row: dict) -> bool:
"""Whether an admin left on the connector a connection row belongs to.
@@ -12,6 +12,16 @@ CREDENTIAL_POLICIES = ("choose", "owner", "member")
ALLOW_CUSTOM_MCP_KEY = "connectors.allow_custom_mcp"
def allow_writes_key(connector_key: str) -> str:
"""``app_metadata`` key of the admin's "let agents make changes" switch for a connector.
Only connectors whose tools opt into writes (GitHub) have one. It lives in
``app_metadata`` like the custom MCP switch: ``"false"`` forbids writes,
anything else (or nothing) allows them.
"""
return f"connectors.{connector_key}.allow_writes"
class ConnectorPoliciesRepository:
"""Whether a connector is enabled and which credential mode it forces."""
+363
View File
@@ -0,0 +1,363 @@
"""GitHub write access: opted into per connection, forbidden by an admin, enforced at run time."""
from __future__ import annotations
import json
from contextlib import contextmanager
from types import SimpleNamespace
from unittest.mock import MagicMock, patch
import pytest
from flask import Flask
from sqlalchemy import text
import docsgpt.api.user # noqa: F401 (loads mcp_tool without the circular import)
import docsgpt.agents.tools.mcp_tool # noqa: F401,E402 (patched below)
from docsgpt.connectors import catalog, mcp, service
from docsgpt.security.encryption import encrypt_json
READONLY_MCP = "https://api.githubcopilot.com/mcp/readonly"
WRITE_MCP = "https://api.githubcopilot.com/mcp/"
@pytest.fixture
def app():
return Flask(__name__)
@contextmanager
def _db(conn):
@contextmanager
def _yield():
yield conn
with patch.multiple("docsgpt.api.connector.connections", db_session=_yield, db_readonly=_yield), \
patch.multiple("docsgpt.api.admin.connectors", db_session=_yield, db_readonly=_yield), \
patch.multiple("docsgpt.connectors.service", db_session=_yield, db_readonly=_yield), \
patch.multiple("docsgpt.connectors.mcp", db_session=_yield, db_readonly=_yield), \
patch.multiple("docsgpt.connectors.resolve", db_readonly=_yield):
yield
def _call(app, resource, method, path, user="alice", body=None, args=(), roles=None):
with app.test_request_context(path, method=method.upper(), json=body):
from flask import request
request.decoded_token = {"sub": user, "roles": roles or ["user"]} if user else None
return getattr(resource(), method)(*args)
def _connection(conn, user="alice") -> str:
return str(conn.execute(
text(
"INSERT INTO connector_sessions (user_id, provider, connector_key, auth_kind, status, account_label, "
"encrypted_credentials) VALUES (:u, 'github', 'github', 'api_key', 'connected', 'octocat', :e) "
"RETURNING id"
),
{"u": user, "e": encrypt_json({"credentials": {"access_token": "github_pat_alice"}}, user)},
).scalar())
def _tool(conn, cid, server_url, actions) -> str:
return str(conn.execute(
text(
"INSERT INTO user_tools (user_id, name, config, actions, connection_id) VALUES ('alice', 'mcp_tool', "
"CAST(:c AS jsonb), CAST(:a AS jsonb), CAST(:cid AS uuid)) RETURNING id"
),
{"c": json.dumps({"server_url": server_url, "auth_type": "bearer", "transport_type": "http", "timeout": 30}),
"a": json.dumps(actions), "cid": cid},
).scalar())
def _row(conn, cid) -> dict:
return dict(conn.execute(
text("SELECT * FROM connector_sessions WHERE id = CAST(:c AS uuid)"), {"c": cid},
).one()._mapping)
def _stored(conn, cid) -> tuple[dict, dict]:
config, actions = conn.execute(
text("SELECT config, actions FROM user_tools WHERE connection_id = CAST(:c AS uuid)"), {"c": cid},
).one()
return config, {a["name"]: a for a in actions}
def _forbid(conn):
from docsgpt.storage.db.repositories.app_metadata import AppMetadataRepository
from docsgpt.storage.db.repositories.connector_policies import allow_writes_key
AppMetadataRepository(conn).set(allow_writes_key("github"), "false")
def _action(name, read_only=None):
action = {"name": name, "description": name,
"parameters": {"type": "object", "properties": {"owner": {"type": "string"}}}}
if read_only is not None:
action["annotations"] = {"readOnlyHint": read_only}
return action
# What GitHub's servers list: the read-only endpoint only reads; the full one
# adds writes, annotated ``readOnlyHint: false`` (or not annotated at all).
READ_ACTIONS = [_action("get_issue", True), _action("search_code", True)]
WRITE_ACTIONS = READ_ACTIONS + [
_action("create_issue", False),
_action("add_issue_comment", False),
_action("mark_all_notifications_read"),
]
def _fake_discovery(calls):
"""Stands in for GitHub's MCP server: lists the actions of the endpoint asked for."""
def discover(user_id, connection, tool):
url = tool["config"]["server_url"]
calls.append(url)
return [dict(a) for a in (WRITE_ACTIONS if url == WRITE_MCP else READ_ACTIONS)]
return discover
class TestCatalog:
def test_github_offers_writes_on_its_full_endpoint(self):
definition = catalog.get_definition("github")
assert definition.mcp_url == READONLY_MCP
assert definition.mcp_write_url == WRITE_MCP
assert definition.to_dict()["writes_opt_in"] is True
assert catalog.get_definition("telegram").to_dict()["writes_opt_in"] is False
def test_members_see_whether_writes_are_allowed(self, pg_conn):
entries = {e["key"]: e for e in service.catalog_for_user(pg_conn, "alice", is_admin=False)}
assert entries["github"]["writes_allowed"] is True
assert entries["telegram"]["writes_allowed"] is False
_forbid(pg_conn)
entries = {e["key"]: e for e in service.catalog_for_user(pg_conn, "alice", is_admin=False)}
assert entries["github"]["writes_allowed"] is False
def test_builtin_config_picks_the_endpoint(self):
definition = catalog.get_definition("github")
assert service.builtin_mcp_config(definition)["server_url"] == READONLY_MCP
assert service.builtin_mcp_config(definition, writes=True)["server_url"] == WRITE_MCP
class TestClassification:
def test_full_endpoint_actions_are_writes_unless_marked_read_only(self):
"""A GitHub action is a read only when GitHub says so: a name like
``mark_all_notifications_read`` must not pass for one."""
connection = {"id": "c1", "auth_kind": "api_key", "status": "connected", "connector_key": "github"}
fake = MagicMock()
fake.get_actions_metadata.return_value = [dict(a) for a in WRITE_ACTIONS]
with patch.object(service, "access_credentials", return_value={"access_token": "t"}), \
patch("docsgpt.agents.tools.mcp_tool.MCPTool", return_value=fake):
actions = mcp.discover_builtin_actions("alice", connection, writes=True)
access = {a["name"]: a["access"] for a in actions}
assert access == {
"get_issue": "read", "search_code": "read",
"create_issue": "write", "add_issue_comment": "write", "mark_all_notifications_read": "write",
}
class TestSetup:
def test_opting_in_uses_the_full_endpoint_and_writes_ask_first(self, app, pg_conn):
from docsgpt.api.connector.connections import ConnectionDetail, ConnectionSetup
cid = _connection(pg_conn)
calls = []
with _db(pg_conn), patch.object(mcp, "_discover", side_effect=_fake_discovery(calls)):
resp = _call(app, ConnectionSetup, "post", f"/api/connections/{cid}/setup",
body={"create_tools": True, "allow_writes": True}, args=[cid])
detail = _call(app, ConnectionDetail, "get", f"/api/connections/{cid}", args=[cid]).get_json()
assert resp.status_code == 200
assert calls == [WRITE_MCP]
config, actions = _stored(pg_conn, cid)
assert config["server_url"] == WRITE_MCP
assert actions["create_issue"]["access"] == "write" and actions["create_issue"]["require_approval"] is True
assert actions["mark_all_notifications_read"]["access"] == "write"
assert actions["get_issue"]["access"] == "read" and not actions["get_issue"].get("require_approval")
assert detail["connection"]["writes"] is True
permissions = {a["name"]: a["permission"] for a in detail["connection"]["tools"][0]["actions"]}
assert permissions["create_issue"] == "ask" and permissions["get_issue"] == "always"
def test_read_only_is_the_default(self, app, pg_conn):
from docsgpt.api.connector.connections import ConnectionDetail, ConnectionSetup
cid = _connection(pg_conn)
calls = []
with _db(pg_conn), patch.object(mcp, "_discover", side_effect=_fake_discovery(calls)):
_call(app, ConnectionSetup, "post", f"/api/connections/{cid}/setup",
body={"create_tools": True}, args=[cid])
detail = _call(app, ConnectionDetail, "get", f"/api/connections/{cid}", args=[cid]).get_json()
assert calls == [READONLY_MCP]
assert _stored(pg_conn, cid)[0]["server_url"] == READONLY_MCP
assert detail["connection"]["writes"] is False
def test_forbidden_writes_are_refused(self, app, pg_conn):
from docsgpt.api.connector.connections import ConnectionSetup
cid = _connection(pg_conn)
_forbid(pg_conn)
with _db(pg_conn), patch.object(mcp, "_discover") as discover:
resp = _call(app, ConnectionSetup, "post", f"/api/connections/{cid}/setup",
body={"create_tools": True, "allow_writes": True}, args=[cid])
assert resp.status_code == 403
assert resp.get_json()["code"] == "writes_forbidden"
discover.assert_not_called()
assert pg_conn.execute(text("SELECT count(*) FROM user_tools")).scalar() == 0
class TestSwitchWrites:
def _read_only_tool(self, conn, cid):
actions = mcp.apply_default_permissions("mcp_tool", service._transform_actions(
[dict(a) for a in READ_ACTIONS]))
# The user turned one read off; that choice survives switching.
actions = [{**a, "active": a["name"] != "search_code"} for a in actions]
return _tool(conn, cid, READONLY_MCP, actions)
def test_turning_writes_on_and_off_keeps_choices(self, app, pg_conn):
from docsgpt.api.connector.connections import ConnectionWrites
cid = _connection(pg_conn)
self._read_only_tool(pg_conn, cid)
calls = []
with _db(pg_conn), patch.object(mcp, "_discover", side_effect=_fake_discovery(calls)):
on = _call(app, ConnectionWrites, "put", f"/api/connections/{cid}/writes",
body={"allow": True}, args=[cid])
config, actions = _stored(pg_conn, cid)
assert on.status_code == 200
assert on.get_json()["writes"] is True
assert sorted(on.get_json()["added"]) == ["add_issue_comment", "create_issue",
"mark_all_notifications_read"]
assert config["server_url"] == WRITE_MCP
assert actions["search_code"]["active"] is False
assert actions["create_issue"]["require_approval"] is True
off = _call(app, ConnectionWrites, "put", f"/api/connections/{cid}/writes",
body={"allow": False}, args=[cid])
assert calls == [WRITE_MCP, READONLY_MCP]
config, actions = _stored(pg_conn, cid)
assert off.get_json()["writes"] is False
assert config["server_url"] == READONLY_MCP
assert set(actions) == {"get_issue", "search_code"}
assert actions["search_code"]["active"] is False
def test_forbidden_writes_cannot_be_turned_on_but_can_be_turned_off(self, app, pg_conn):
from docsgpt.api.connector.connections import ConnectionWrites
cid = _connection(pg_conn)
_tool(pg_conn, cid, WRITE_MCP, [_action("create_issue", False)])
_forbid(pg_conn)
calls = []
with _db(pg_conn), patch.object(mcp, "_discover", side_effect=_fake_discovery(calls)):
on = _call(app, ConnectionWrites, "put", "/x", body={"allow": True}, args=[cid])
off = _call(app, ConnectionWrites, "put", "/x", body={"allow": False}, args=[cid])
assert on.status_code == 403 and on.get_json()["code"] == "writes_forbidden"
assert off.status_code == 200
assert _stored(pg_conn, cid)[0]["server_url"] == READONLY_MCP
def test_owner_only_github_only_and_needs_the_tool(self, app, pg_conn):
from docsgpt.api.connector.connections import ConnectionWrites
cid = _connection(pg_conn)
with _db(pg_conn):
bob = _call(app, ConnectionWrites, "put", "/x", user="bob", body={"allow": True}, args=[cid])
no_tool = _call(app, ConnectionWrites, "put", "/x", body={"allow": True}, args=[cid])
bad = _call(app, ConnectionWrites, "put", "/x", body={"allow": "yes"}, args=[cid])
assert bob.status_code == 404
assert no_tool.status_code == 409 and no_tool.get_json()["code"] == "no_tools"
assert bad.status_code == 400
telegram = str(pg_conn.execute(text(
"INSERT INTO connector_sessions (user_id, provider, connector_key, auth_kind, status, account_label) "
"VALUES ('alice', 'telegram', 'telegram', 'api_key', 'connected', 'bot') RETURNING id"
)).scalar())
with _db(pg_conn):
other = _call(app, ConnectionWrites, "put", "/x", body={"allow": True}, args=[telegram])
assert other.status_code == 400
def test_refresh_after_an_admin_forbids_writes_goes_read_only(self, pg_conn):
cid = _connection(pg_conn)
_tool(pg_conn, cid, WRITE_MCP, [_action("create_issue", False)])
_forbid(pg_conn)
calls = []
with _db(pg_conn), patch.object(mcp, "_discover", side_effect=_fake_discovery(calls)):
result = mcp.refresh_mcp_tools("alice", _row(pg_conn, cid))
assert calls == [READONLY_MCP]
assert result["removed"] == ["create_issue"]
assert _stored(pg_conn, cid)[0]["server_url"] == READONLY_MCP
class TestAdmin:
def test_admin_can_forbid_github_writes(self, app, pg_conn):
from docsgpt.api.admin.connectors import AdminConnectorsResource
with _db(pg_conn):
before = _call(app, AdminConnectorsResource, "get", "/x", roles=["admin"]).get_json()
resp = _call(app, AdminConnectorsResource, "put", "/x", roles=["admin"],
body={"policies": {"github": {"allow_writes": False}}})
rows = {c["key"]: c for c in before["connectors"]}
assert rows["github"]["allow_writes"] is True
assert rows["telegram"]["allow_writes"] is None
assert resp.status_code == 200
assert {c["key"]: c for c in resp.get_json()["connectors"]}["github"]["allow_writes"] is False
assert service.writes_allowed(service.load_policies(pg_conn), "github") is False
def test_rejects_writes_policy_where_it_means_nothing(self, app, pg_conn):
from docsgpt.api.admin.connectors import AdminConnectorsResource
with _db(pg_conn):
other = _call(app, AdminConnectorsResource, "put", "/x", roles=["admin"],
body={"policies": {"telegram": {"allow_writes": False}}})
not_bool = _call(app, AdminConnectorsResource, "put", "/x", roles=["admin"],
body={"policies": {"github": {"allow_writes": "no"}}})
assert other.status_code == 400
assert not_bool.status_code == 400
def _tool_data(cid, server_url=WRITE_MCP):
return {
"id": "tool-gh", "user_id": "alice", "name": "mcp_tool", "connection_id": cid,
"config": {"server_url": server_url, "auth_type": "bearer"}, "credential_mode": "owner",
"actions": [
{"name": "create_issue", "access": "write", "active": True, "require_approval": False},
{"name": "get_issue", "access": "read", "active": True},
],
}
class TestRuntime:
def _loaded_url(self, pg_conn, tool):
from docsgpt.agents.tool_executor import ToolExecutor
with _db(pg_conn), patch("docsgpt.agents.tool_executor.ToolManager") as manager:
ToolExecutor(user="alice")._get_or_load_tool(tool, "t1", "create_issue")
return manager.return_value.load_tool.call_args.kwargs["tool_config"]["server_url"]
def test_write_endpoint_is_used_while_allowed(self, pg_conn):
assert self._loaded_url(pg_conn, _tool_data(_connection(pg_conn))) == WRITE_MCP
def test_forbidden_writes_fall_back_to_the_read_only_endpoint(self, pg_conn):
cid = _connection(pg_conn)
_forbid(pg_conn)
assert self._loaded_url(pg_conn, _tool_data(cid)) == READONLY_MCP
def test_any_other_path_on_githubs_host_is_read_only(self, pg_conn):
cid = _connection(pg_conn)
assert self._loaded_url(pg_conn, _tool_data(cid, "https://api.githubcopilot.com/mcp/x/all")) == READONLY_MCP
def _pause(self, pg_conn, action_name):
from docsgpt.agents.tool_executor import ToolExecutor
cid = _connection(pg_conn)
_forbid(pg_conn)
with _db(pg_conn), patch("docsgpt.agents.tool_executor.ToolActionParser") as parser:
parser.return_value.parse_args.return_value = ("t1", action_name, {})
return ToolExecutor(user="alice").check_pause(
{"t1": _tool_data(cid)}, SimpleNamespace(id="c1", name=action_name, thought_signature=None),
"OpenAILLM",
)
def test_forbidden_write_is_denied_with_a_reason(self, pg_conn):
pause = self._pause(pg_conn, "create_issue")
assert pause["pause_type"] == "headless_denied"
assert "admin" in pause["deny_reason"]
def test_reads_still_run_when_writes_are_forbidden(self, pg_conn):
assert self._pause(pg_conn, "get_issue") is None