From 3c02ff707cd4bfd837a1ba0ceb52d459dfaeabcc Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Sat, 2 May 2026 13:26:54 -0400 Subject: [PATCH] Guard run_user_tool against deactivated user-defined tools deactivate_unprivileged_tool deliberately only flips the per-user UserDynamicToolAssociation.active flag, leaving DynamicTool.active intact so other users with associations to the same DynamicTool aren't affected (the model schema permits many-to-many, even though the current create path is 1:1). That means a user who deactivates "their" UDT can still resolve it by UUID through the toolbox -- and run it via tools_service._create -- because get_unprivileged_tool_by_uuid doesn't filter by association.active either. Add a runtime preflight in run_user_tool that fails the call when either the underlying tool or the calling user's association is inactive. Also surfaces unauthenticated and unowned errors as clean ValueErrors before reaching the deeper service layer. Tightening the chokepoint (DynamicToolManager.get_unprivileged_tool_by_uuid) to filter by association.active would close this across all entry points but is a meaningful behavior change for the existing UnprivilegedToolsApi endpoints; leaving that for a separate review. --- lib/galaxy/agents/operations.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/lib/galaxy/agents/operations.py b/lib/galaxy/agents/operations.py index 88eca2907d2..c7adb2b22e8 100644 --- a/lib/galaxy/agents/operations.py +++ b/lib/galaxy/agents/operations.py @@ -920,6 +920,31 @@ class AgentOperationsManager: return {"uuid": uuid, "deactivated": True} def run_user_tool(self, history_id: str, tool_uuid: str, inputs: dict[str, Any]) -> dict[str, Any]: + from sqlalchemy import select + + from galaxy.model import UserDynamicToolAssociation + + user = self.trans.user + if not user: + raise ValueError("User must be authenticated") + + dynamic_tool = self.dynamic_tools_manager.get_unprivileged_tool_by_uuid(user, tool_uuid) + if dynamic_tool is None: + raise ValueError(f"User-defined tool {tool_uuid!r} not found") + # UDT deactivation is per-user by design: deactivate_unprivileged_tool only + # flips the user-association, leaving DynamicTool.active intact so other + # users sharing the underlying tool aren't affected. The runtime check has + # to look at the association, not just dynamic_tool.active. + session = self.dynamic_tools_manager.session() + assoc_active = session.scalar( + select(UserDynamicToolAssociation.active).where( + UserDynamicToolAssociation.user_id == user.id, + UserDynamicToolAssociation.dynamic_tool_id == dynamic_tool.id, + ) + ) + if not dynamic_tool.active or not assoc_active: + raise ValueError(f"User-defined tool {tool_uuid!r} is deactivated") + payload = { "history_id": history_id, "tool_uuid": tool_uuid,