diff --git a/docsgpt/api/user/tools/routes.py b/docsgpt/api/user/tools/routes.py index d7677e18..ea09e8fe 100644 --- a/docsgpt/api/user/tools/routes.py +++ b/docsgpt/api/user/tools/routes.py @@ -1,5 +1,7 @@ """Tool management routes.""" +import copy + from flask import current_app, jsonify, make_response, request from flask_restx import fields, Namespace, Resource @@ -13,6 +15,7 @@ from docsgpt.agents.default_tools import ( is_synthesized_tool_id, WORKFLOW_ONLY_BUILTINS, ) +from docsgpt.agents.tool_pins import merge_submitted_actions, PinChangeRefused from docsgpt.agents.tools.spec_parser import parse_spec from docsgpt.agents.tools.tool_manager import ToolManager from docsgpt.api import api @@ -185,6 +188,24 @@ def transform_actions(actions_metadata): return transformed +def _stored_actions(tool_doc: dict) -> list: + """A tool's stored actions, or its class's own for a row stored without any. + + Args: + tool_doc: The ``user_tools`` row. + + Returns: + The action list submitted actions are validated against. + """ + actions = tool_doc.get("actions") or [] + if actions or tool_doc.get("name") in ("mcp_tool", "api_tool"): + return actions + tool_instance = tool_manager.tools.get(tool_doc.get("name")) + if tool_instance is None: + return actions + return transform_actions(copy.deepcopy(tool_instance.get_actions_metadata())) + + tools_ns = Namespace("tools", description="Tool management operations", path="/api") # Tools the Connectors page adds through "Add custom connector" rather than @@ -814,18 +835,29 @@ class UpdateToolActions(Resource): with db_session() as conn: repo = UserToolsRepository(conn) tool_doc = repo.get_any(data["id"], user) - if tool_doc: - repo.update(str(tool_doc["id"]), user, {"actions": data["actions"]}) - else: + owner = user + if tool_doc is None: # Team editor write path (secrets stay owner-only — actions # carry no credentials, so editing them is safe). owner = effective_write_owner(conn, "tool", data["id"], user) - if not owner: + tool_doc = repo.get_by_id(data["id"]) if owner else None + if not tool_doc or tool_doc.get("user_id") != owner: return make_response( jsonify({"success": False, "message": "Tool not found"}), 404, ) - repo.update(data["id"], owner, {"actions": data["actions"]}) + # The stored actions are the schema: nothing can be added, + # and a fixed value is the owner's to set (an editor pinning + # a Telegram chat would redirect the owner's bot). + try: + actions = merge_submitted_actions( + _stored_actions(tool_doc), data["actions"], may_change_pins=owner == user, + ) + except PinChangeRefused as err: + return make_response(jsonify({"success": False, "message": str(err)}), 403) + except ValueError as err: + return make_response(jsonify({"success": False, "message": str(err)}), 400) + repo.update(str(tool_doc["id"]), owner, {"actions": actions}) except Exception as err: current_app.logger.error( f"Error updating tool actions: {err}", exc_info=True diff --git a/tests/api/user/test_tools_routes.py b/tests/api/user/test_tools_routes.py index 737c0cd6..489166b7 100644 --- a/tests/api/user/test_tools_routes.py +++ b/tests/api/user/test_tools_routes.py @@ -1306,26 +1306,103 @@ class TestUpdateToolActionsHappy: response = UpdateToolActions().post() assert response.status_code == 404 - def test_updates_actions(self, app, pg_conn): - from docsgpt.api.user.tools.routes import UpdateToolActions + @staticmethod + def _seed_actions_tool(pg_conn, user): + from docsgpt.storage.db.repositories.user_tools import UserToolsRepository - user = "u-actions" - tool = _seed_tool(pg_conn, user=user) + return UserToolsRepository(pg_conn).create( + user, + "telegram", + config={}, + display_name="Telegram", + description="", + actions=[ + { + "name": "telegram_send_message", + "description": "Send a message", + "active": True, + "parameters": { + "type": "object", + "properties": { + "text": {"type": "string", "filled_by_llm": True, "value": ""}, + "chat_id": {"type": "string", "filled_by_llm": True, "value": ""}, + }, + }, + } + ], + status=True, + ) + + def _post(self, app, pg_conn, user, tool_id, actions): + from docsgpt.api.user.tools.routes import UpdateToolActions with _patch_tools_db(pg_conn), app.test_request_context( "/api/update_tool_actions", method="POST", - json={ - "id": str(tool["id"]), - "actions": [ - {"name": "action_1", "active": True, "parameters": {}} - ], - }, + json={"id": tool_id, "actions": actions}, ): from flask import request request.decoded_token = {"sub": user} - response = UpdateToolActions().post() + return UpdateToolActions().post() + + @staticmethod + def _stored(pg_conn, tool): + from docsgpt.storage.db.repositories.user_tools import UserToolsRepository + + return UserToolsRepository(pg_conn).get_by_id(str(tool["id"]))["actions"][0] + + def test_updates_actions(self, app, pg_conn): + user = "u-actions" + tool = self._seed_actions_tool(pg_conn, user) + response = self._post( + app, pg_conn, user, str(tool["id"]), [{"name": "telegram_send_message", "active": False}], + ) assert response.status_code == 200 + assert self._stored(pg_conn, tool)["active"] is False + + def test_owner_pins_a_parameter(self, app, pg_conn): + user = "u-actions-pin" + tool = self._seed_actions_tool(pg_conn, user) + response = self._post(app, pg_conn, user, str(tool["id"]), [{ + "name": "telegram_send_message", + "parameters": {"properties": {"chat_id": {"filled_by_llm": False, "value": "123"}}}, + }]) + assert response.status_code == 200 + chat_id = self._stored(pg_conn, tool)["parameters"]["properties"]["chat_id"] + assert chat_id["filled_by_llm"] is False and chat_id["value"] == "123" + + @pytest.mark.parametrize( + "actions", + [ + [{"name": "made_up_action", "active": True}], + [{"name": "telegram_send_message", "parameters": {"properties": {"token": {"value": "x"}}}}], + [{"active": True}], + "not a list", + ], + ) + def test_rejects_actions_that_do_not_match_the_schema(self, app, pg_conn, actions): + user = "u-actions-bad" + tool = self._seed_actions_tool(pg_conn, user) + response = self._post(app, pg_conn, user, str(tool["id"]), actions) + assert response.status_code == 400 + assert self._stored(pg_conn, tool)["active"] is True + + def test_team_editor_can_change_actions_but_not_pins(self, app, pg_conn): + owner, editor = "u-owner", "u-editor" + tool = self._seed_actions_tool(pg_conn, owner) + with patch("docsgpt.api.user.tools.routes.effective_write_owner", return_value=owner): + toggled = self._post( + app, pg_conn, editor, str(tool["id"]), [{"name": "telegram_send_message", "active": False}], + ) + pinned = self._post(app, pg_conn, editor, str(tool["id"]), [{ + "name": "telegram_send_message", + "parameters": {"properties": {"chat_id": {"filled_by_llm": False, "value": "666"}}}, + }]) + assert toggled.status_code == 200 + assert pinned.status_code == 403 + stored = self._stored(pg_conn, tool) + assert stored["active"] is False + assert stored["parameters"]["properties"]["chat_id"]["filled_by_llm"] is True class TestUpdateToolStatusHappy: