fix(visibility): prevent auto-refresh from overriding tool/prompt/resource visibility (#3574)

* restrict visibility change with auto refresh

Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com>

* linting

Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com>

* fix: formatting cleanup in test_gateway_service_extended.py

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(visibility): new tools inherit gateway visibility instead of hardcoded 'public'

_create_db_tool() hardcoded visibility="public" for all newly discovered
tools, while resources and prompts correctly used gateway.visibility.
This caused new tools on team-scoped or private gateways to be created
with public visibility, breaking team isolation.

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(visibility): use explicit update_visibility flag instead of created_via check

The previous check `should_update_visibility = created_via == "update"` was
too broad: any gateway edit (e.g. description change) would trigger
visibility propagation to all linked tools/resources/prompts, overwriting
per-item customizations.

Replace with an explicit `update_visibility: bool` parameter on the three
sync helpers, set to True only when the user actually changed the gateway's
visibility in the request (`gateway_update.visibility is not None`).  This
aligns the helper-level gate with the direct propagation logic at lines
1902-1912 which already checks the same condition.

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

---------

Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com>
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
Co-authored-by: Mihai Criveti <crivetimihai@gmail.com>

Author: kevalmahajan

mcpgateway/services/gateway_service.py

@@ -2078,14 +2078,16 @@ async def update_gateway(
                         gateway.auth_value = None
                         gateway.oauth_config = None
 
-                    # Update tools using helper method
-                    tools_to_add = self._update_or_create_tools(db, tools, gateway, "update")
+                    # Update tools using helper method — only propagate visibility
+                    # when the user explicitly changed it in this request
+                    _vis_changed = gateway_update.visibility is not None
+                    tools_to_add = self._update_or_create_tools(db, tools, gateway, "update", update_visibility=_vis_changed)
 
                     # Update resources using helper method
-                    resources_to_add = self._update_or_create_resources(db, resources, gateway, "update")
+                    resources_to_add = self._update_or_create_resources(db, resources, gateway, "update", update_visibility=_vis_changed)
 
                     # Update prompts using helper method
-                    prompts_to_add = self._update_or_create_prompts(db, prompts, gateway, "update")
+                    prompts_to_add = self._update_or_create_prompts(db, prompts, gateway, "update", update_visibility=_vis_changed)
 
                     # Log newly added items
                     items_added = len(tools_to_add) + len(resources_to_add) + len(prompts_to_add)
@@ -4051,17 +4053,18 @@ def _create_db_tool(
             # Inherit team assignment and visibility from gateway
             team_id=gateway.team_id,
             owner_email=gateway.owner_email,
-            visibility="public",  # Federated tools should be public for discovery
+            visibility=gateway.visibility,
         )
 
-    def _update_or_create_tools(self, db: Session, tools: List[Any], gateway: DbGateway, created_via: str) -> List[DbTool]:
+    def _update_or_create_tools(self, db: Session, tools: List[Any], gateway: DbGateway, created_via: str, update_visibility: bool = False) -> List[DbTool]:
         """Helper to handle update-or-create logic for tools from MCP server.
 
         Args:
             db: Database session
             tools: List of tools from MCP server
             gateway: Gateway object
             created_via: String indicating creation source ("oauth", "update", etc.)
+            update_visibility: Whether to propagate gateway visibility to existing tools
 
         Returns:
             List of new tools to be added to the database
@@ -4123,7 +4126,8 @@ def _update_or_create_tools(self, db: Session, tools: List[Any], gateway: DbGate
                     except Exception:
                         gateway_tool_auth_value = encode_auth(gateway.auth_value) if isinstance(gateway.auth_value, dict) else gateway.auth_value
                         auth_value_changed = existing_tool.auth_value != gateway_tool_auth_value
-                    auth_fields_changed = existing_tool.auth_type != gateway.auth_type or auth_value_changed or existing_tool.visibility != gateway.visibility
+
+                    auth_fields_changed = existing_tool.auth_type != gateway.auth_type or auth_value_changed or (update_visibility and existing_tool.visibility != gateway.visibility)
 
                     if basic_fields_changed or schema_fields_changed or auth_fields_changed:
                         fields_to_update = True
@@ -4142,7 +4146,8 @@ def _update_or_create_tools(self, db: Session, tools: List[Any], gateway: DbGate
                         existing_tool.jsonpath_filter = tool.jsonpath_filter
                         existing_tool.auth_type = gateway.auth_type
                         existing_tool.auth_value = encode_auth(gateway.auth_value) if isinstance(gateway.auth_value, dict) else gateway.auth_value
-                        existing_tool.visibility = gateway.visibility
+                        if update_visibility:
+                            existing_tool.visibility = gateway.visibility
                         logger.debug(f"Updated existing tool: {tool.name}")
                 else:
                     # Create new tool if it doesn't exist
@@ -4162,14 +4167,15 @@ def _update_or_create_tools(self, db: Session, tools: List[Any], gateway: DbGate
 
         return tools_to_add
 
-    def _update_or_create_resources(self, db: Session, resources: List[Any], gateway: DbGateway, created_via: str) -> List[DbResource]:
+    def _update_or_create_resources(self, db: Session, resources: List[Any], gateway: DbGateway, created_via: str, update_visibility: bool = False) -> List[DbResource]:
         """Helper to handle update-or-create logic for resources from MCP server.
 
         Args:
             db: Database session
             resources: List of resources from MCP server
             gateway: Gateway object
             created_via: String indicating creation source ("oauth", "update", etc.)
+            update_visibility: Whether to propagate gateway visibility to existing resources
 
         Returns:
             List of new resources to be added to the database
@@ -4206,7 +4212,7 @@ def _update_or_create_resources(self, db: Session, resources: List[Any], gateway
                         or existing_resource.description != resource.description
                         or existing_resource.mime_type != resource.mime_type
                         or existing_resource.uri_template != resource.uri_template
-                        or existing_resource.visibility != gateway.visibility
+                        or (update_visibility and existing_resource.visibility != gateway.visibility)
                     ):
                         fields_to_update = True
 
@@ -4215,7 +4221,8 @@ def _update_or_create_resources(self, db: Session, resources: List[Any], gateway
                         existing_resource.description = resource.description
                         existing_resource.mime_type = resource.mime_type
                         existing_resource.uri_template = resource.uri_template
-                        existing_resource.visibility = gateway.visibility
+                        if update_visibility:
+                            existing_resource.visibility = gateway.visibility
                         logger.debug(f"Updated existing resource: {resource.uri}")
                 else:
                     # Create new resource if it doesn't exist
@@ -4238,14 +4245,15 @@ def _update_or_create_resources(self, db: Session, resources: List[Any], gateway
 
         return resources_to_add
 
-    def _update_or_create_prompts(self, db: Session, prompts: List[Any], gateway: DbGateway, created_via: str) -> List[DbPrompt]:
+    def _update_or_create_prompts(self, db: Session, prompts: List[Any], gateway: DbGateway, created_via: str, update_visibility: bool = False) -> List[DbPrompt]:
         """Helper to handle update-or-create logic for prompts from MCP server.
 
         Args:
             db: Database session
             prompts: List of prompts from MCP server
             gateway: Gateway object
             created_via: String indicating creation source ("oauth", "update", etc.)
+            update_visibility: Whether to propagate gateway visibility to existing prompts
 
         Returns:
             List of new prompts to be added to the database
@@ -4280,14 +4288,15 @@ def _update_or_create_prompts(self, db: Session, prompts: List[Any], gateway: Db
                     if (
                         existing_prompt.description != prompt.description
                         or existing_prompt.template != (prompt.template if hasattr(prompt, "template") else "")
-                        or existing_prompt.visibility != gateway.visibility
+                        or (update_visibility and existing_prompt.visibility != gateway.visibility)
                     ):
                         fields_to_update = True
 
                     if fields_to_update:
                         existing_prompt.description = prompt.description
                         existing_prompt.template = prompt.template if hasattr(prompt, "template") else ""
-                        existing_prompt.visibility = gateway.visibility
+                        if update_visibility:
+                            existing_prompt.visibility = gateway.visibility
                         logger.debug(f"Updated existing prompt: {prompt.name}")
                 else:
                     # Create new prompt if it doesn't exist

tests/unit/mcpgateway/services/test_gateway_service_extended.py

@@ -700,8 +700,8 @@ async def test_update_or_create_tools_existing_tools(self):
         tools = [mock_tool]
         context = "update"
 
-        # Call the helper method
-        result = service._update_or_create_tools(mock_db, tools, mock_gateway, context)
+        # Call the helper method (with explicit visibility change)
+        result = service._update_or_create_tools(mock_db, tools, mock_gateway, context, update_visibility=True)
 
         # Should return empty list (no new tools, existing one updated)
         assert len(result) == 0
@@ -851,8 +851,8 @@ async def test_update_or_create_resources_existing_resources(self):
         resources = [mock_resource]
         context = "update"
 
-        # Call method
-        result = service._update_or_create_resources(mock_db, resources, mock_gateway, context)
+        # Call method (with explicit visibility change)
+        result = service._update_or_create_resources(mock_db, resources, mock_gateway, context, update_visibility=True)
 
         # Should return empty list (no new resources)
         assert len(result) == 0
@@ -943,8 +943,8 @@ async def test_update_or_create_prompts_existing_prompts(self):
         prompts = [mock_prompt]
         context = "update"
 
-        # Call the helper method
-        result = service._update_or_create_prompts(mock_db, prompts, mock_gateway, context)
+        # Call the helper method (with explicit visibility change)
+        result = service._update_or_create_prompts(mock_db, prompts, mock_gateway, context, update_visibility=True)
 
         # Should return empty list (no new prompts, existing one updated)
         assert len(result) == 0
@@ -1048,7 +1048,7 @@ async def test_helper_methods_mixed_operations(self):
         assert existing_tool2.original_description == "Updated description"
         assert existing_tool2.url == "http://new.com"  # Updated from gateway
         assert existing_tool2.auth_type == "bearer"  # Updated from gateway
-        assert existing_tool2.visibility == "public"  # Updated from gateway
+        assert existing_tool2.visibility == "private"  # Visibility NOT updated from gateway because context is NOT update
 
     @pytest.mark.asyncio
     async def test_helper_methods_empty_input_lists(self):
@@ -1277,7 +1277,7 @@ async def test_helper_methods_tool_removal_scenario(self):
         # existing_tool1 should be updated with gateway values (even if description stays the same)
         assert existing_tool1.url == "http://new.com"  # Updated from gateway
         assert existing_tool1.auth_type == "bearer"  # Updated from gateway
-        assert existing_tool1.visibility == "public"  # Updated from gateway
+        assert existing_tool1.visibility == "private"  # Visibility NOT updated from gateway because context is NOT update
 
         # existing_tool3 should be updated (description not customized, so upstream value applies)
         assert existing_tool3.description == "Updated description"
@@ -1350,13 +1350,13 @@ async def test_helper_methods_resource_removal_scenario(self):
 
         # existing_resource1 should be updated with gateway values
         assert existing_resource1.description == "Keep this resource"
-        assert existing_resource1.visibility == "public"  # Updated from gateway
+        assert existing_resource1.visibility == "private"  # Visibility NOT updated from gateway because context is NOT update
 
         # existing_resource3 should be updated
         assert existing_resource3.description == "Updated description"
         assert existing_resource3.mime_type == "application/json"
         assert existing_resource3.uri_template == "new template"
-        assert existing_resource3.visibility == "public"  # Updated from gateway
+        assert existing_resource3.visibility == "private"  # Visibility NOT updated from gateway because context is NOT update
 
     @pytest.mark.asyncio
     async def test_helper_methods_prompt_removal_scenario(self):
@@ -1416,12 +1416,12 @@ async def test_helper_methods_prompt_removal_scenario(self):
         # existing_prompt1 should be updated with gateway values
         assert existing_prompt1.description == "Keep this prompt"
         assert existing_prompt1.template == "Keep template"
-        assert existing_prompt1.visibility == "public"  # Updated from gateway
+        assert existing_prompt1.visibility == "private"  # Visibility NOT updated from gateway because context is NOT update
 
         # existing_prompt3 should be updated
         assert existing_prompt3.description == "Updated description"
         assert existing_prompt3.template == "Updated template"
-        assert existing_prompt3.visibility == "public"  # Updated from gateway
+        assert existing_prompt3.visibility == "private"  # Visibility NOT updated from gateway because context is NOT update
 
     @pytest.mark.asyncio
     async def test_fetch_tools_after_oauth_prompt_stale_removal_uses_original_name(self):
@@ -1536,3 +1536,139 @@ async def test_helper_methods_complete_removal_scenario(self):
         assert tools_to_remove[0].original_name == "old_tool"
         assert resources_to_remove[0].uri == "file:///old.txt"
         assert prompts_to_remove[0].name == "old_prompt"
+
+    @pytest.mark.asyncio
+    async def test_update_visibility_logic(self):
+        """Test that existing items retain visibility on auto-refresh, but inherit on manual update."""
+        service = GatewayService()
+        mock_db = MagicMock()
+        mock_gateway = MagicMock()
+        mock_gateway.id = "gw"
+        mock_gateway.url = "http://gw.com"
+        mock_gateway.auth_type = "none"
+        mock_gateway.visibility = "public"
+
+        # Mock tools
+        existing_tool = MagicMock()
+        existing_tool.original_name = "test_tool"
+        existing_tool.description = "Test Tool"
+        existing_tool.original_description = "Test Tool"
+        existing_tool.visibility = "private"
+
+        tool_from_server = MagicMock()
+        tool_from_server.name = "test_tool"
+        tool_from_server.description = "Test Tool"
+
+        # Mock resources
+        existing_res = MagicMock()
+        existing_res.uri = "file:///test"
+        existing_res.name = "test"
+        existing_res.description = "Test Res"
+        existing_res.visibility = "team"
+
+        res_from_server = MagicMock()
+        res_from_server.uri = "file:///test"
+        res_from_server.name = "test"
+        res_from_server.description = "Test Res"
+
+        # Mock prompts
+        existing_prompt = MagicMock()
+        existing_prompt.original_name = "test_prompt"
+        existing_prompt.name = "test_prompt"
+        existing_prompt.description = "Test Prompt"
+        existing_prompt.visibility = "private"
+
+        prompt_from_server = MagicMock()
+        prompt_from_server.name = "test_prompt"
+        prompt_from_server.description = "Test Prompt"
+
+        # --- Test 1: AUTO REFRESH Context ---
+        def create_mock_result(item):
+            mock_result = MagicMock()
+            mock_result.scalars.return_value.all.return_value = [item]
+            return mock_result
+
+        # Reset visibilities
+        existing_tool.visibility = "private"
+        existing_res.visibility = "team"
+        existing_prompt.visibility = "private"
+
+        mock_db.execute.side_effect = [
+            create_mock_result(existing_tool),
+        ]
+        service._update_or_create_tools(mock_db, [tool_from_server], mock_gateway, "auto_refresh")
+        assert existing_tool.visibility == "private"
+
+        mock_db.execute.side_effect = [
+            create_mock_result(existing_res),
+        ]
+        service._update_or_create_resources(mock_db, [res_from_server], mock_gateway, "health_check")
+        assert existing_res.visibility == "team"
+
+        mock_db.execute.side_effect = [
+            create_mock_result(existing_prompt),
+        ]
+        service._update_or_create_prompts(mock_db, [prompt_from_server], mock_gateway, "rediscovery")
+        assert existing_prompt.visibility == "private"
+
+        # --- Test 2: MANUAL UPDATE with explicit visibility change ---
+        mock_db.execute.side_effect = [
+            create_mock_result(existing_tool),
+        ]
+        service._update_or_create_tools(mock_db, [tool_from_server], mock_gateway, "update", update_visibility=True)
+        assert existing_tool.visibility == "public"
+
+        mock_db.execute.side_effect = [
+            create_mock_result(existing_res),
+        ]
+        service._update_or_create_resources(mock_db, [res_from_server], mock_gateway, "update", update_visibility=True)
+        assert existing_res.visibility == "public"
+
+        mock_db.execute.side_effect = [
+            create_mock_result(existing_prompt),
+        ]
+        service._update_or_create_prompts(mock_db, [prompt_from_server], mock_gateway, "update", update_visibility=True)
+        assert existing_prompt.visibility == "public"
+
+        # --- Test 3: UPDATE without visibility change (e.g. description-only edit) ---
+        # Visibility must NOT be overwritten even though created_via is "update"
+        existing_tool.visibility = "private"
+        existing_res.visibility = "team"
+        existing_prompt.visibility = "private"
+
+        mock_db.execute.side_effect = [create_mock_result(existing_tool)]
+        service._update_or_create_tools(mock_db, [tool_from_server], mock_gateway, "update", update_visibility=False)
+        assert existing_tool.visibility == "private"
+
+        mock_db.execute.side_effect = [create_mock_result(existing_res)]
+        service._update_or_create_resources(mock_db, [res_from_server], mock_gateway, "update", update_visibility=False)
+        assert existing_res.visibility == "team"
+
+        mock_db.execute.side_effect = [create_mock_result(existing_prompt)]
+        service._update_or_create_prompts(mock_db, [prompt_from_server], mock_gateway, "update", update_visibility=False)
+        assert existing_prompt.visibility == "private"
+
+    def test_create_db_tool_inherits_gateway_visibility(self):
+        """New tools created via _create_db_tool inherit visibility from the gateway, not hardcoded 'public'."""
+        service = GatewayService()
+        tool = MagicMock()
+        tool.name = "new_tool"
+        tool.description = "A tool"
+        tool.request_type = "POST"
+        tool.headers = {}
+        tool.input_schema = {}
+        tool.annotations = {}
+        tool.jsonpath_filter = None
+
+        for vis in ("private", "team", "public"):
+            gateway = MagicMock()
+            gateway.url = "http://gw.com"
+            gateway.name = "gw"
+            gateway.auth_type = "none"
+            gateway.auth_value = None
+            gateway.team_id = "t1"
+            gateway.owner_email = "owner@example.com"
+            gateway.visibility = vis
+
+            db_tool = service._create_db_tool(tool=tool, gateway=gateway)
+            assert db_tool.visibility == vis, f"Expected {vis}, got {db_tool.visibility}"