diff --git a/application/single_app/config.py b/application/single_app/config.py index cc55f92ec..60e28134a 100644 --- a/application/single_app/config.py +++ b/application/single_app/config.py @@ -98,7 +98,7 @@ EXECUTOR_TYPE = 'thread' EXECUTOR_MAX_WORKERS = 30 SESSION_TYPE = 'filesystem' -VERSION = "0.261.030" +VERSION = "0.261.031" IS_DEVELOPMENT = is_development_env_enabled() # Opt-out for deployments where App Service Easy Auth is active but the platform diff --git a/application/single_app/functions_action_connection_tests.py b/application/single_app/functions_action_connection_tests.py index dce1dc27d..04ac56106 100644 --- a/application/single_app/functions_action_connection_tests.py +++ b/application/single_app/functions_action_connection_tests.py @@ -526,16 +526,26 @@ def test_log_analytics_connection(manifest: Dict[str, Any]) -> Dict[str, Any]: return result -def test_mcp_connection(manifest: Dict[str, Any]) -> Dict[str, Any]: +def test_mcp_connection(manifest: Dict[str, Any], *, origin=None) -> Dict[str, Any]: """Validate an MCP action by initializing a session and listing the server's tools.""" + # MCP runtime dependencies are needed only when this connection tester is selected. + from functions_mcp_operations import ( + McpRuntimeError, + classify_mcp_exception, + get_mcp_error_http_status, + ) from semantic_kernel_plugins.mcp_plugin_factory import McpPluginFactory try: - probe_result = asyncio.run(McpPluginFactory.probe_server_from_config(manifest)) + probe_result = asyncio.run(McpPluginFactory.probe_server_from_config(manifest, origin=origin)) except Exception as exc: + error_info = classify_mcp_exception(exc, "capability_probe") + category = exc.category if isinstance(exc, McpRuntimeError) else error_info["category"] result = build_failure_result( - f"The MCP server connection failed: {sanitize_connection_error(exc, manifest)}", - status=502, + error_info["message"], + status=get_mcp_error_http_status(category), + error_type=category, + retryable=False, ) _log_connection_test("mcp", result) return result diff --git a/application/single_app/functions_action_manifest.py b/application/single_app/functions_action_manifest.py new file mode 100644 index 000000000..bdf031bf9 --- /dev/null +++ b/application/single_app/functions_action_manifest.py @@ -0,0 +1,159 @@ +# functions_action_manifest.py +"""Dependency-free action type, origin, and retired-transport contracts.""" + +from copy import deepcopy +from dataclasses import dataclass +import re + + +MCP_STDIO_REMOVED_CODE = "mcp_stdio_removed" +MCP_STDIO_REMOVED_MESSAGE = ( + "This action uses stdio, which is no longer supported. " + "Reconfigure it to use a supported remote MCP server, or delete it." +) +MCP_TYPE_ALIASES = frozenset({ + "mcp", "mcpplugin", "modelcontextprotocol", "modelcontextprotocolplugin", +}) + + +class McpConfigurationError(ValueError): + """An invalid MCP configuration with a stable, safe public message.""" + + def __init__(self, message="MCP configuration is invalid.", code="validation"): + super().__init__(message) + self.public_message = message + self.code = code + + +class McpStdioRemovedError(McpConfigurationError): + """A retired configuration must never reach an executable connector.""" + + def __init__(self): + super().__init__(MCP_STDIO_REMOVED_MESSAGE, MCP_STDIO_REMOVED_CODE) + + +def resolve_action_type(manifest): + """Resolve supported MCP aliases without collapsing other runtime types.""" + if not isinstance(manifest, dict): + raise ValueError("Action configuration must be an object.") + declared_type = manifest.get("type") + if declared_type is None or (isinstance(declared_type, str) and not declared_type.strip()): + metadata = manifest.get("metadata") + declared_type = metadata.get("type", "") if isinstance(metadata, dict) else "" + if not isinstance(declared_type, str): + raise ValueError("Action type must be a string.") + declared_type = declared_type.strip() + compact_type = re.sub(r"[\s_-]", "", declared_type).lower() + return "mcp" if compact_type in MCP_TYPE_ALIASES else declared_type + + +def is_mcp_action(manifest): + """Return whether the effective runtime action is MCP.""" + return resolve_action_type(manifest) == "mcp" + + +def is_retired_mcp_stdio(manifest): + """Inspect historical records without running active-config normalization.""" + if not isinstance(manifest, dict): + return False + try: + if not is_mcp_action(manifest): + return False + except ValueError: + return False + fields = manifest.get("additionalFields") + transport = fields.get("transport", "") if isinstance(fields, dict) else "" + return ( + str(transport or "").strip().lower() == "stdio" + or str(manifest.get("endpoint") or "").strip().lower().startswith("stdio:") + ) + + +def get_action_execution_status(manifest): + """Derive management information; never use it as execution authority.""" + if is_retired_mcp_stdio(manifest): + return { + "state": "unsupported", + "code": MCP_STDIO_REMOVED_CODE, + "message": MCP_STDIO_REMOVED_MESSAGE, + } + return None + + +@dataclass(frozen=True) +class McpActionOrigin: + """Authoritative origin established by a server-side authorized lookup.""" + + scope_type: str + scope_id: str + action_id: str = "" + + def __post_init__(self): + if self.scope_type not in {"personal", "group", "global"}: + raise ValueError("Action origin has an unsupported scope.") + if not isinstance(self.scope_id, str) or not self.scope_id.strip(): + raise ValueError("Action origin requires a scope identifier.") + if self.scope_type == "global" and self.scope_id != "global": + raise ValueError("Global action origin requires the global scope.") + if not isinstance(self.action_id, str): + raise ValueError("Action origin has an invalid action identifier.") + + +class ScopedActionManifest(dict): + """Compare JSON payloads, keeping server-only authorization origin separate.""" + + def __init__(self, manifest, origin): + if not isinstance(origin, McpActionOrigin): + raise ValueError("Action origin is required.") + super().__init__(manifest) + self._action_origin = origin + + def __eq__(self, other): + return dict.__eq__(self, other) + + def __ne__(self, other): + return dict.__ne__(self, other) + + @property + def action_origin(self): + return self._action_origin + + def copy(self): + return ScopedActionManifest(self, self._action_origin) + + +def get_action_origin(manifest): + """Never interpret dictionary keys as proof of a trusted origin.""" + return manifest.action_origin if isinstance(manifest, ScopedActionManifest) else None + + +def copy_action_manifest(manifest): + """Copy an internal manifest without discarding its lookup provenance.""" + return deepcopy(manifest) + + +def bind_action_origin(manifest, scope_type, scope_id): + """Bind a record to the collection/partition used by its authorized caller.""" + origin = McpActionOrigin( + scope_type=scope_type, + scope_id=str(scope_id or ""), + action_id=str(manifest.get("id") or ""), + ) + bound = ScopedActionManifest(deepcopy(dict(manifest)), origin) + bound["type"] = resolve_action_type(bound) + for field in ("runtime_user_id", "action_origin", "_action_origin", "execution_status"): + bound.pop(field, None) + bound["scope"] = "user" if scope_type == "personal" else scope_type + bound["scope_id"] = origin.scope_id + bound["is_global"] = scope_type == "global" + bound["is_group"] = scope_type == "group" + if scope_type == "personal": + bound["user_id"] = origin.scope_id + bound.pop("group_id", None) + elif scope_type == "group": + bound["group_id"] = origin.scope_id + bound.pop("user_id", None) + else: + bound.pop("user_id", None) + bound.pop("group_id", None) + return bound diff --git a/application/single_app/functions_global_actions.py b/application/single_app/functions_global_actions.py index 94a676ae5..a77f2f853 100644 --- a/application/single_app/functions_global_actions.py +++ b/application/single_app/functions_global_actions.py @@ -6,11 +6,13 @@ global_actions container with id partitioning. """ +import logging import uuid -import json -import traceback from datetime import datetime +from azure.cosmos import exceptions from config import cosmos_global_actions_container +from functions_action_manifest import McpConfigurationError, bind_action_origin +from functions_appinsights import log_event from functions_authentication import get_current_user_id from functions_keyvault import keyvault_plugin_save_helper, keyvault_plugin_get_helper, keyvault_plugin_delete_helper, SecretReturnType from functions_workspace_identities import ( @@ -19,6 +21,39 @@ validate_action_identity_reference, ) from functions_chat_bootstrap_cache import bump_chat_bootstrap_global_cache_version +from functions_legacy_action_management import ( + authorize_scoped_mcp_secret_read, + prepare_scoped_action, + retired_action_management_view, + validate_scoped_mcp_action, +) +from functions_settings import get_settings + + +def _clean_action(action, return_type, action_id=None): + retired_view = retired_action_management_view(action, "global", "global") + if retired_view is not None: + retired_view.setdefault("is_enabled", True) + return retired_view + cleaned = {key: value for key, value in action.items() if not key.startswith("_")} + cleaned = bind_action_origin(cleaned, "global", "global") + if return_type == SecretReturnType.NAME and cleaned["type"] == "mcp": + # Workspace identity hydration treats NAME like VALUE, so defer it until authorization. + cleaned.setdefault("is_enabled", True) + return cleaned + if return_type == SecretReturnType.VALUE and cleaned["type"] == "mcp": + authorize_scoped_mcp_secret_read(cleaned, get_settings()) + cleaned = keyvault_plugin_get_helper( + cleaned, scope_value=action_id or action.get("id"), scope="global", return_type=return_type + ) + cleaned = hydrate_action_identity_reference( + cleaned, + WORKSPACE_IDENTITY_SCOPE_GLOBAL, + WORKSPACE_IDENTITY_SCOPE_GLOBAL, + return_type=return_type, + ) + cleaned.setdefault("is_enabled", True) + return bind_action_origin(cleaned, "global", "global") def get_global_actions(return_type=SecretReturnType.TRIGGER, include_disabled=False): """ @@ -40,25 +75,18 @@ def get_global_actions(return_type=SecretReturnType.TRIGGER, include_disabled=Fa query=query, enable_cross_partition_query=True )) - # Resolve Key Vault references for each action - actions = [keyvault_plugin_get_helper(a, scope_value=a.get('id'), scope="global", return_type=return_type) for a in actions] - actions = [ - hydrate_action_identity_reference( - action, - WORKSPACE_IDENTITY_SCOPE_GLOBAL, - WORKSPACE_IDENTITY_SCOPE_GLOBAL, - return_type=return_type, - ) - for action in actions - ] - for action in actions: - action.setdefault('is_enabled', True) - return actions - - except Exception as e: - print(f"โŒ Error getting global actions: {str(e)}") - traceback.print_exc() + except exceptions.CosmosResourceNotFoundError: return [] + except Exception as exc: + log_event("[PLUGINS] Global action listing failed", level=logging.ERROR, + extra={"error_type": type(exc).__name__}) + raise + try: + return [_clean_action(action, return_type) for action in actions] + except Exception as exc: + log_event("[PLUGINS] Global action normalization failed", level=logging.WARNING, + extra={"error_type": type(exc).__name__}) + raise def get_global_action(action_id, return_type=SecretReturnType.TRIGGER): @@ -76,20 +104,18 @@ def get_global_action(action_id, return_type=SecretReturnType.TRIGGER): item=action_id, partition_key=action_id ) - # Resolve Key Vault references - action = keyvault_plugin_get_helper(action, scope_value=action_id, scope="global", return_type=return_type) - action = hydrate_action_identity_reference( - action, - WORKSPACE_IDENTITY_SCOPE_GLOBAL, - WORKSPACE_IDENTITY_SCOPE_GLOBAL, - return_type=return_type, - ) - print(f"โœ… Found global action: {action_id}") - return action - - except Exception as e: - print(f"โŒ Error getting global action {action_id}: {str(e)}") + except exceptions.CosmosResourceNotFoundError: return None + except Exception as exc: + log_event("[PLUGINS] Global action lookup failed", level=logging.ERROR, + extra={"action_id": action_id, "error_type": type(exc).__name__}) + raise + try: + return _clean_action(action, return_type, action_id=action_id) + except Exception as exc: + log_event("[PLUGINS] Global action normalization failed", level=logging.WARNING, + extra={"action_id": action_id, "error_type": type(exc).__name__}) + raise def save_global_action(action_data, user_id=None): @@ -104,27 +130,30 @@ def save_global_action(action_data, user_id=None): dict: Saved action data or None if failed """ try: + action_data = prepare_scoped_action(action_data, "global", "global") if user_id is None: user_id = get_current_user_id() + actor_user_id = user_id if not user_id: user_id = "system" # Ensure required fields - if 'id' not in action_data: + if not action_data.get('id'): action_data['id'] = str(uuid.uuid4()) + if not isinstance(action_data['id'], str): + raise ValueError("Action ID must be a string.") # Add metadata action_data['is_global'] = True now = datetime.utcnow().isoformat() # Check if this is a new action or an update to preserve created_by/created_at - existing_action = None try: existing_action = cosmos_global_actions_container.read_item( item=action_data['id'], partition_key=action_data['id'] ) - except Exception: - pass + except exceptions.CosmosResourceNotFoundError: + existing_action = None if existing_action: action_data['created_by'] = existing_action.get('created_by') or user_id @@ -141,12 +170,14 @@ def save_global_action(action_data, user_id=None): action_data['modified_by'] = user_id action_data['modified_at'] = now action_data['updated_at'] = now + action_data = bind_action_origin(action_data, "global", "global") + if action_data["type"] == "mcp": + validate_scoped_mcp_action(action_data, actor_user_id, get_settings()) validate_action_identity_reference( action_data, WORKSPACE_IDENTITY_SCOPE_GLOBAL, WORKSPACE_IDENTITY_SCOPE_GLOBAL, ) - print(f"๐Ÿ’พ Saving global action: {action_data.get('name', 'Unknown')}") # Store secrets in Key Vault before upsert action_data = keyvault_plugin_save_helper( action_data, @@ -156,12 +187,15 @@ def save_global_action(action_data, user_id=None): ) result = cosmos_global_actions_container.upsert_item(body=action_data) bump_chat_bootstrap_global_cache_version(reason="global_action_saved") - print(f"โœ… Global action saved successfully: {result['id']}") - return result + return bind_action_origin( + {key: value for key, value in result.items() if not key.startswith("_")}, + "global", + "global", + ) - except Exception as e: - print(f"โŒ Error saving global action: {str(e)}") - traceback.print_exc() + except Exception as exc: + log_event("[PLUGINS] Global action save failed", level=logging.ERROR, + extra={"error_type": type(exc).__name__}) raise @@ -176,22 +210,20 @@ def delete_global_action(action_id): bool: True if successful, False otherwise """ try: - print(f"๐Ÿ—‘๏ธ Deleting global action: {action_id}") # Delete secrets from Key Vault before deleting the action - action = get_global_action(action_id, return_type=SecretReturnType.NAME) - if action: - keyvault_plugin_delete_helper(action, scope_value=action_id, scope="global") + action = cosmos_global_actions_container.read_item(item=action_id, partition_key=action_id) + action = bind_action_origin(action, "global", "global") + keyvault_plugin_delete_helper(action, scope_value=action_id, scope="global") cosmos_global_actions_container.delete_item( item=action_id, partition_key=action_id ) bump_chat_bootstrap_global_cache_version(reason="global_action_deleted") - print(f"โœ… Global action deleted successfully: {action_id}") return True - except Exception as e: - print(f"โŒ Error deleting global action {action_id}: {str(e)}") - traceback.print_exc() + except Exception as exc: + log_event("[PLUGINS] Global action deletion failed", level=logging.ERROR, + extra={"action_id": action_id, "error_type": type(exc).__name__}) return False @@ -210,6 +242,7 @@ def update_global_action_enabled(action_id, is_enabled, user_id=None): try: if user_id is None: user_id = get_current_user_id() + actor_user_id = user_id if not user_id: user_id = "system" @@ -217,6 +250,9 @@ def update_global_action_enabled(action_id, is_enabled, user_id=None): item=action_id, partition_key=action_id ) + action = prepare_scoped_action(action, "global", "global") + if action["type"] == "mcp": + validate_scoped_mcp_action(action, actor_user_id, get_settings()) now = datetime.utcnow().isoformat() action['is_enabled'] = bool(is_enabled) action['modified_by'] = user_id @@ -224,8 +260,10 @@ def update_global_action_enabled(action_id, is_enabled, user_id=None): action['updated_at'] = now result = cosmos_global_actions_container.upsert_item(body=action) bump_chat_bootstrap_global_cache_version(reason="global_action_enabled_updated") - return result - except Exception as e: - print(f"โŒ Error updating enabled state for global action {action_id}: {str(e)}") - traceback.print_exc() + return bind_action_origin(result, "global", "global") + except (McpConfigurationError, PermissionError): + raise + except Exception as exc: + log_event("[PLUGINS] Global action enabled-state update failed", level=logging.ERROR, + extra={"action_id": action_id, "error_type": type(exc).__name__}) return None diff --git a/application/single_app/functions_governance.py b/application/single_app/functions_governance.py index 029885f9c..77b43bf3f 100644 --- a/application/single_app/functions_governance.py +++ b/application/single_app/functions_governance.py @@ -16,6 +16,7 @@ import app_settings_cache from config import cosmos_governance_item_policies_container, cosmos_governance_policies_container from functions_activity_logging import log_governance_change +from functions_action_manifest import resolve_action_type from functions_group import get_user_groups from functions_public_workspaces import get_user_public_workspaces from functions_settings import get_settings @@ -1156,7 +1157,7 @@ def _normalize_action_scope(scope: str) -> str: def normalize_governed_action_type(action_type: Any) -> str: - normalized_type = str(action_type or "").strip().lower().replace("-", "_").replace(" ", "_") + normalized_type = resolve_action_type({"type": action_type or ""}).lower().replace("-", "_").replace(" ", "_") return ACTION_TYPE_ALIASES.get(normalized_type, normalized_type) @@ -1260,7 +1261,7 @@ def filter_actions_by_action_type_access( for action in actions or []: if not isinstance(action, dict): continue - if is_action_type_access_allowed(feature_key, user_id, action.get("type"), scope): + if is_action_type_access_allowed(feature_key, user_id, resolve_action_type(action), scope): governed_actions.append(action) return governed_actions @@ -1302,7 +1303,7 @@ def ensure_global_action_access(user_id: str, action: Dict[str, Any]) -> None: ensure_action_type_access( "governance_global_actions_usage", normalized_user_id, - action.get("type"), + resolve_action_type(action), "global", ) diff --git a/application/single_app/functions_group_actions.py b/application/single_app/functions_group_actions.py index 19d58d7b5..4dfd73c09 100644 --- a/application/single_app/functions_group_actions.py +++ b/application/single_app/functions_group_actions.py @@ -2,15 +2,17 @@ """Group-level plugin/action management helpers.""" +import logging import re import uuid from datetime import datetime from typing import Any, Dict, List, Optional -from functions_debug import debug_print from azure.cosmos import exceptions -from flask import current_app from config import cosmos_group_actions_container +from functions_action_manifest import bind_action_origin, is_retired_mcp_stdio +from functions_appinsights import log_event +from functions_authentication import get_current_user_id from functions_keyvault import ( SecretReturnType, keyvault_plugin_delete_helper, @@ -24,6 +26,13 @@ ) from functions_governance import ensure_action_type_access, filter_actions_by_action_type_access from functions_chat_bootstrap_cache import bump_chat_bootstrap_global_cache_version +from functions_legacy_action_management import ( + authorize_scoped_mcp_secret_read, + prepare_scoped_action, + retired_action_management_view, + validate_scoped_mcp_action, +) +from functions_settings import get_settings _NAME_PATTERN = re.compile(r"^[A-Za-z0-9_-]+$") @@ -45,14 +54,18 @@ def get_group_actions( partition_key=group_id, ) ) - return [_clean_action(action, group_id, return_type) for action in results] except exceptions.CosmosResourceNotFoundError: return [] except Exception as exc: - debug_print( - "Error fetching group actions for %s: %s", group_id, exc - ) - return [] + log_event("[PLUGINS] Group action listing failed", level=logging.ERROR, + extra={"group_id": group_id, "error_type": type(exc).__name__}) + raise + try: + return [_clean_action(action, group_id, return_type) for action in results] + except Exception as exc: + log_event("[PLUGINS] Group action normalization failed", level=logging.WARNING, + extra={"group_id": group_id, "error_type": type(exc).__name__}) + raise def get_governed_group_actions( @@ -62,7 +75,11 @@ def get_governed_group_actions( ) -> List[Dict[str, Any]]: """Return group actions that the user can access by action type governance.""" actions = get_group_actions(group_id, return_type=return_type) - return filter_actions_by_action_type_access(user_id, actions, 'governance_group_actions', 'group') + active_actions = [action for action in actions if not is_retired_mcp_stdio(action)] + allowed_actions = filter_actions_by_action_type_access( + user_id, active_actions, 'governance_group_actions', 'group' + ) + return [action for action in actions if is_retired_mcp_stdio(action) or action in allowed_actions] def get_group_action( @@ -70,39 +87,49 @@ def get_group_action( ) -> Optional[Dict[str, Any]]: """Fetch a single group action by id or name.""" try: - action = cosmos_group_actions_container.read_item( - item=action_id, - partition_key=group_id, - ) - except exceptions.CosmosResourceNotFoundError: - query = "SELECT * FROM c WHERE c.group_id = @group_id AND c.name = @name" - parameters = [ - {"name": "@group_id", "value": group_id}, - {"name": "@name", "value": action_id}, - ] - actions = list( - cosmos_group_actions_container.query_items( - query=query, - parameters=parameters, + try: + action = cosmos_group_actions_container.read_item( + item=action_id, partition_key=group_id, ) - ) - if not actions: - return None - action = actions[0] - except Exception as exc: - debug_print( - "Error fetching group action %s for %s: %s", action_id, group_id, exc - ) + except exceptions.CosmosResourceNotFoundError: + query = "SELECT * FROM c WHERE c.group_id = @group_id AND c.name = @name" + parameters = [ + {"name": "@group_id", "value": group_id}, + {"name": "@name", "value": action_id}, + ] + actions = list( + cosmos_group_actions_container.query_items( + query=query, + parameters=parameters, + partition_key=group_id, + ) + ) + if not actions: + return None + action = actions[0] + except exceptions.CosmosResourceNotFoundError: return None + except Exception as exc: + log_event("[PLUGINS] Group action lookup failed", level=logging.ERROR, + extra={"group_id": group_id, "action_id": action_id, "error_type": type(exc).__name__}) + raise - return _clean_action(action, group_id, return_type) + try: + return _clean_action(action, group_id, return_type) + except Exception as exc: + log_event("[PLUGINS] Group action normalization failed", level=logging.WARNING, + extra={"group_id": group_id, "action_id": action_id, "error_type": type(exc).__name__}) + raise def save_group_action(group_id: str, action_data: Dict[str, Any], user_id: Optional[str] = None) -> Dict[str, Any]: """Create or update a group action entry.""" - payload = dict(action_data) + payload = prepare_scoped_action(action_data, "group", group_id) + user_id = user_id or get_current_user_id() action_id = payload.get("id") or str(uuid.uuid4()) + if not isinstance(action_id, str): + raise ValueError("Action ID must be a string.") payload["id"] = action_id payload["group_id"] = group_id @@ -118,8 +145,6 @@ def save_group_action(group_id: str, action_data: Dict[str, Any], user_id: Optio ) except exceptions.CosmosResourceNotFoundError: pass - except Exception: - pass if existing_action: payload["created_by"] = existing_action.get("created_by", user_id) @@ -148,6 +173,9 @@ def save_group_action(group_id: str, action_data: Dict[str, Any], user_id: Optio ensure_action_type_access('governance_group_actions', user_id, payload.get('type'), 'group') payload.pop("user_id", None) + payload = bind_action_origin(payload, "group", group_id) + if payload["type"] == "mcp": + validate_scoped_mcp_action(payload, user_id, get_settings()) validate_action_identity_reference( payload, @@ -167,9 +195,8 @@ def save_group_action(group_id: str, action_data: Dict[str, Any], user_id: Optio bump_chat_bootstrap_global_cache_version(reason="group_action_saved") return _clean_action(stored, group_id, SecretReturnType.TRIGGER) except Exception as exc: - debug_print( - "Error saving group action %s for %s: %s", action_id, group_id, exc - ) + log_event("[PLUGINS] Group action save failed", level=logging.ERROR, + extra={"group_id": group_id, "action_id": action_id, "error_type": type(exc).__name__}) raise @@ -192,9 +219,8 @@ def delete_group_action(group_id: str, action_id: str) -> bool: bump_chat_bootstrap_global_cache_version(reason="group_action_deleted") return True except Exception as exc: - debug_print( - "Error deleting group action %s for %s: %s", action_id, group_id, exc - ) + log_event("[PLUGINS] Group action deletion failed", level=logging.ERROR, + extra={"group_id": group_id, "action_id": action_id, "error_type": type(exc).__name__}) raise @@ -251,7 +277,17 @@ def _clean_action( group_id: str, return_type: SecretReturnType, ) -> Dict[str, Any]: + retired_view = retired_action_management_view(action, "group", group_id) + if retired_view is not None: + return retired_view cleaned = {k: v for k, v in action.items() if not k.startswith("_")} + cleaned = bind_action_origin(cleaned, "group", group_id) + if return_type == SecretReturnType.NAME and cleaned["type"] == "mcp": + # Workspace identity hydration treats NAME like VALUE, so defer it until authorization. + return cleaned + if return_type == SecretReturnType.VALUE and cleaned["type"] == "mcp": + ensure_action_type_access("governance_group_actions", get_current_user_id(), "mcp", "group") + authorize_scoped_mcp_secret_read(cleaned, get_settings()) cleaned = keyvault_plugin_get_helper( cleaned, scope_value=group_id, @@ -264,7 +300,4 @@ def _clean_action( group_id, return_type=return_type, ) - cleaned.setdefault("is_global", False) - cleaned.setdefault("is_group", True) - cleaned.setdefault("scope", "group") - return cleaned + return bind_action_origin(cleaned, "group", group_id) diff --git a/application/single_app/functions_legacy_action_management.py b/application/single_app/functions_legacy_action_management.py new file mode 100644 index 000000000..de034fce9 --- /dev/null +++ b/application/single_app/functions_legacy_action_management.py @@ -0,0 +1,281 @@ +# functions_legacy_action_management.py +"""Safe historical-action views, source identities, and scoped save validation.""" + +from collections import Counter +from copy import deepcopy +from dataclasses import dataclass +import hashlib +import json + +from functions_action_manifest import ( + McpConfigurationError, + McpStdioRemovedError, + bind_action_origin, + get_action_execution_status, + get_action_origin, + is_retired_mcp_stdio, + resolve_action_type, +) + + +LEGACY_ACTION_PREFIX = "legacy-action-" +LEGACY_ACTION_SOURCE = "settings.plugins" +_MANAGEMENT_FIELDS = { + "execution_status", "is_legacy", "legacy_source", "legacy_locator", +} +_DISPLAY_FIELDS = ( + "id", "name", "displayName", "description", "created_by", "created_at", + "modified_by", "modified_at", "last_updated", "updated_at", +) + + +class LegacyActionConflictError(ValueError): + """The selected source or destination no longer matches the verified record.""" + + code = "legacy_action_conflict" + public_message = "This legacy action has changed. Refresh the action list and try again." + + def __init__(self): + super().__init__(self.public_message) + + +class LegacyActionSourceUpdateError(RuntimeError): + """A stored destination was not followed by confirmed source cleanup.""" + + code = "legacy_source_update_failed" + public_message = ( + "The legacy settings could not be updated. The original action was retained; " + "refresh and retry." + ) + + def __init__(self): + super().__init__(self.public_message) + + +class LegacyActionSecretConflictError(LegacyActionConflictError): + """Distinct action IDs must not overwrite the same name-based secret storage.""" + + code = "legacy_secret_name_conflict" + public_message = ( + "Another action uses the same credential storage name. " + "Choose a different action name before saving or reconfiguring it." + ) + + +@dataclass(frozen=True) +class LegacyActionSnapshot: + """An internal snapshot from an authorized owner's settings, never request data.""" + + owner_id: str + locator: str + record: object + index: int + duplicate_count: int + + +def action_snapshot_digest(value): + """Fingerprint JSON without revealing its credential-bearing contents.""" + serialized = json.dumps(value, sort_keys=True, separators=(",", ":"), ensure_ascii=True) + return hashlib.sha256(serialized.encode("utf-8")).hexdigest() + + +def legacy_action_snapshots(user_id, plugins): + """Identify records by owner and exact content, not name or aggregate counts. + + Including identical-record multiplicity prevents replaying a deleted duplicate's + locator against a different, indistinguishable remaining copy. + """ + if not isinstance(plugins, list): + raise LegacyActionConflictError() + fingerprints = [ + action_snapshot_digest([LEGACY_ACTION_SOURCE, str(user_id), plugin]) + for plugin in plugins + ] + totals = Counter(fingerprints) + occurrences = Counter() + snapshots = [] + for index, (plugin, fingerprint) in enumerate(zip(plugins, fingerprints)): + occurrence = occurrences[fingerprint] + occurrences[fingerprint] += 1 + locator = f"{LEGACY_ACTION_PREFIX}{fingerprint}-{totals[fingerprint]}-{occurrence}" + snapshots.append(LegacyActionSnapshot( + owner_id=str(user_id), + locator=locator, + record=deepcopy(plugin), + index=index, + duplicate_count=totals[fingerprint], + )) + return snapshots + + +def find_legacy_action_snapshot(user_id, plugins, locator): + """Resolve only a freshly derived, owner-bound source locator.""" + if not isinstance(locator, str) or not locator.startswith(LEGACY_ACTION_PREFIX): + raise LegacyActionConflictError() + for snapshot in legacy_action_snapshots(user_id, plugins): + if snapshot.locator == locator: + return snapshot + raise LegacyActionConflictError() + + +def _safe_management_view(manifest, scope_type, scope_id): + original = manifest if isinstance(manifest, dict) else {} + view = { + field: original[field] + for field in _DISPLAY_FIELDS + if isinstance(original.get(field), str) + } + try: + action_type = resolve_action_type(original) + except ValueError: + action_type = "" + view.setdefault("name", "") + view.setdefault("displayName", view["name"]) + view.setdefault("description", "") + view.update({ + "type": action_type, + "endpoint": "", + "auth": {"type": "NoAuth"}, + "metadata": {"type": action_type}, + "additionalFields": {}, + }) + if "is_enabled" in original: + view["is_enabled"] = original["is_enabled"] is True + elif scope_type == "global": + view["is_enabled"] = True + status = get_action_execution_status(original) + if status: + view["additionalFields"]["transport"] = "stdio" + view = bind_action_origin(view, scope_type, scope_id) + if status: + view["execution_status"] = status + return view + + +def retired_action_management_view(manifest, scope_type, scope_id): + """Return a credential-free retired view, or None for an unrelated record.""" + if not is_retired_mcp_stdio(manifest): + return None + return _safe_management_view(manifest, scope_type, scope_id) + + +def is_unchanged_retired_action(submitted, original, scope_type, scope_id): + """Compare the complete server-produced view; status is never authority.""" + expected = retired_action_management_view(original, scope_type, scope_id) + return expected is not None and isinstance(submitted, dict) and submitted == expected + + +def legacy_action_management_view(snapshot): + """Expose a legacy source without hydrating credentials or process settings.""" + view = _safe_management_view(snapshot.record, "personal", snapshot.owner_id) + view["id"] = snapshot.locator + view["is_legacy"] = True + view["legacy_source"] = LEGACY_ACTION_SOURCE + view["legacy_locator"] = snapshot.locator + status = view.get("execution_status") or { + "state": "unavailable", + "code": "legacy_action_requires_migration", + "message": "This action remains in legacy settings. Reconfigure it or delete it.", + } + view = bind_action_origin(view, "personal", snapshot.owner_id) + view["execution_status"] = status + return view + + +def is_unchanged_legacy_action(submitted, snapshot): + """Require an exact safe view from the currently authorized source snapshot.""" + return isinstance(submitted, dict) and submitted == legacy_action_management_view(snapshot) + + +def prepare_scoped_action(action, scope_type, scope_id): + """Clone and bind an incoming action, rejecting retired transports immediately.""" + if not isinstance(action, dict): + raise ValueError("Action configuration must be an object.") + payload = { + key: deepcopy(value) + for key, value in action.items() + if not key.startswith("_") and key not in _MANAGEMENT_FIELDS + } + payload = bind_action_origin(payload, scope_type, scope_id) + if is_retired_mcp_stdio(payload): + raise McpStdioRemovedError() + if payload["type"] == "mcp": + # Save-time normalization is deferred so management-only inspection never + # loads preset catalogs or executable action dependencies during bootstrap. + from functions_mcp_operations import ( + normalize_mcp_additional_fields, + validate_mcp_endpoint_for_transport, + ) + + fields = payload.get("additionalFields", {}) + if not isinstance(fields, dict): + raise McpConfigurationError("MCP additional settings must be an object.") + payload["additionalFields"] = normalize_mcp_additional_fields(fields) + errors = validate_mcp_endpoint_for_transport( + payload.get("endpoint"), payload["additionalFields"]["transport"] + ) + if errors: + raise McpConfigurationError("A valid remote MCP endpoint is required.") + return payload + + +def validate_action_configuration(action): + """Use the normal schema and health validators before an import can write secrets.""" + # Health validation imports plugin classes and configuration-dependent modules. + # Load it only at the save boundary, after settings/bootstrap initialization. + from json_schema_validation import validate_plugin + from semantic_kernel_plugins.plugin_health_checker import PluginHealthChecker + + schema_error = validate_plugin(action) + if schema_error: + raise McpConfigurationError("Action configuration is invalid.") + valid, _errors = PluginHealthChecker.validate_plugin_manifest(action, resolve_action_type(action)) + if not valid: + raise McpConfigurationError("Action configuration is invalid.") + + +def authorize_scoped_mcp_secret_read(action, settings): + """Authorize value hydration using the current caller, never a manifest principal.""" + # Runtime policy imports are deferred until credential use; management-only + # inspection must remain independent of settings and connector initialization. + from functions_mcp_preconfigurations import authorize_mcp_action + + return authorize_mcp_action(action, settings=settings, operation="action_secret_resolution") + + +def validate_scoped_mcp_action(action, user_id, settings): + """Validate MCP configuration and current policy using an already bound origin.""" + if resolve_action_type(action) != "mcp": + return + if is_retired_mcp_stdio(action): + raise McpStdioRemovedError() + origin = get_action_origin(action) + if origin is None or not user_id: + raise PermissionError("An authorized action scope and current user are required.") + validate_action_configuration(action) + + # Policy modules are needed only for executable saves; management projections + # stay independent of governance/settings initialization. + from functions_mcp_destinations import ( + assert_mcp_destination_allowed, + get_mcp_destination_policy_config, + ) + from functions_mcp_preconfigurations import assert_mcp_preconfiguration_manifest_allowed + + policy = get_mcp_destination_policy_config(settings, user_id=user_id) + assert_mcp_destination_allowed( + action, + scope_type=origin.scope_type, + scope_id=origin.scope_id, + user_id=user_id, + policy_config=policy, + operation="action_save", + ) + assert_mcp_preconfiguration_manifest_allowed( + action, + scope_type=origin.scope_type, + scope_id=origin.scope_id, + user_id=user_id, + settings=settings, + operation="action_save", + ) diff --git a/application/single_app/functions_mcp_destinations.py b/application/single_app/functions_mcp_destinations.py index abc556537..751a9c08e 100644 --- a/application/single_app/functions_mcp_destinations.py +++ b/application/single_app/functions_mcp_destinations.py @@ -18,8 +18,15 @@ session = None from functions_appinsights import log_event +from functions_action_manifest import ( + McpActionOrigin, + McpConfigurationError, + McpStdioRemovedError, + get_action_origin, + is_mcp_action, + is_retired_mcp_stdio, +) from functions_mcp_operations import ( - MCP_PLUGIN_TYPE, MCP_REMOTE_TRANSPORTS, normalize_mcp_additional_fields, validate_mcp_endpoint_for_transport, @@ -39,7 +46,6 @@ MCP_DESTINATION_SCOPE_ALL = "all" MCP_DESTINATION_SCOPE_ALIASES = { - "": MCP_DESTINATION_SCOPE_PERSONAL, "user": MCP_DESTINATION_SCOPE_PERSONAL, "personal": MCP_DESTINATION_SCOPE_PERSONAL, "workspace": MCP_DESTINATION_SCOPE_PERSONAL, @@ -72,8 +78,34 @@ class McpDestinationPolicyError(PermissionError): def normalize_mcp_destination_scope(scope_type): """Normalize an outbound MCP destination policy scope.""" - normalized_scope = str(scope_type or "").strip().lower() - return MCP_DESTINATION_SCOPE_ALIASES.get(normalized_scope, MCP_DESTINATION_SCOPE_PERSONAL) + normalized_scope = scope_type.strip().lower() if isinstance(scope_type, str) else "" + if normalized_scope not in MCP_DESTINATION_SCOPE_ALIASES: + raise McpDestinationPolicyError("MCP action requires a valid authorized scope.") + return MCP_DESTINATION_SCOPE_ALIASES[normalized_scope] + + +def resolve_mcp_destination_scope(manifest, scope_type=None, scope_id="", *, origin=None): + """Resolve only server-bound provenance or an explicitly authorized scope.""" + stored_origin = get_action_origin(manifest) + if origin is not None and stored_origin is not None and origin != stored_origin: + raise McpDestinationPolicyError("MCP action scope does not match its authorized origin.") + action_origin = origin if origin is not None else stored_origin + if action_origin is not None: + if not isinstance(action_origin, McpActionOrigin): + raise McpDestinationPolicyError("MCP action requires a trusted origin.") + if scope_type is not None and normalize_mcp_destination_scope(scope_type) != action_origin.scope_type: + raise McpDestinationPolicyError("MCP action scope does not match its authorized origin.") + if scope_id not in (None, "") and scope_id != action_origin.scope_id: + raise McpDestinationPolicyError("MCP action scope does not match its authorized origin.") + return action_origin.scope_type, action_origin.scope_id + + normalized_scope = normalize_mcp_destination_scope(scope_type) + if not isinstance(scope_id, str) or not scope_id.strip(): + raise McpDestinationPolicyError("MCP action requires an authorized scope identifier.") + normalized_scope_id = scope_id.strip() + if normalized_scope == MCP_DESTINATION_SCOPE_GLOBAL and normalized_scope_id != MCP_DESTINATION_SCOPE_GLOBAL: + raise McpDestinationPolicyError("Global MCP actions require the global destination scope.") + return normalized_scope, normalized_scope_id def normalize_mcp_policy_id(value): @@ -152,34 +184,34 @@ def _get_request_user_id(): return "" user = session.get("user") if isinstance(user, dict): - return str(user.get("oid") or "").strip() + user_id = user.get("oid") + return user_id.strip() if isinstance(user_id, str) else "" except RuntimeError: return "" return "" +def get_mcp_execution_user_id(): + """Use the current authenticated request, including workflow request contexts.""" + user_id = _get_request_user_id() + if not user_id: + raise McpDestinationPolicyError("MCP execution requires an authenticated user.") + return user_id + + def _list_governance_item_policies(entity_type): + # Governance owns initialized storage; load it only when evaluating active policy. try: from functions_governance import list_item_policies - except Exception as exc: - log_event( - "[MCP_DESTINATION_POLICY] Unable to import governance item policies", - extra={"entity_type": entity_type, "error": str(exc)}, - level=logging.WARNING, - debug_only=True, - ) - return [] - - try: return list_item_policies(entity_type=entity_type) except Exception as exc: log_event( "[MCP_DESTINATION_POLICY] Unable to load governance item policies", - extra={"entity_type": entity_type, "error": str(exc)}, + extra={"entity_type": entity_type, "exception_type": type(exc).__name__}, level=logging.WARNING, debug_only=True, ) - return [] + raise McpDestinationPolicyError("MCP destination policy is currently unavailable.") from exc def _get_governance_group_ids_for_user(user_id): @@ -187,27 +219,18 @@ def _get_governance_group_ids_for_user(user_id): if not normalized_user_id: return set() + # Group grants depend on governance storage initialized by the application owner. try: from functions_governance import get_user_governance_group_ids - except Exception as exc: - log_event( - "[MCP_DESTINATION_POLICY] Unable to import governance group lookup", - extra={"error": str(exc)}, - level=logging.WARNING, - debug_only=True, - ) - return set() - - try: return set(get_user_governance_group_ids(normalized_user_id)) except Exception as exc: log_event( "[MCP_DESTINATION_POLICY] Unable to load governance group ids", - extra={"user_id_present": bool(normalized_user_id), "error": str(exc)}, + extra={"user_id_present": bool(normalized_user_id), "exception_type": type(exc).__name__}, level=logging.WARNING, debug_only=True, ) - return set() + raise McpDestinationPolicyError("MCP destination policy is currently unavailable.") from exc def _governance_item_policy_applies_to_user(policy, user_id, user_group_ids): @@ -344,40 +367,50 @@ def ensure_user_group_ids(): return policy_config +def _get_environment_destination_policy_config(): + """Keep deployment restrictions separate so settings cannot widen them.""" + def environment_value(key, default=None): + value = os.getenv(key) + return value if value is not None else _get_current_app_config_value(key, default) + + return { + "enabled": _coerce_bool(environment_value(ENABLE_MCP_DESTINATION_GOVERNANCE_ENV)), + "block_unsafe_destinations": _coerce_bool(environment_value(MCP_BLOCK_UNSAFE_DESTINATIONS_ENV)), + "common_patterns": _coerce_pattern_list(environment_value(MCP_ALLOWED_DESTINATIONS_ENV)), + "scope_patterns": { + MCP_DESTINATION_SCOPE_PERSONAL: _coerce_pattern_list( + environment_value(MCP_ALLOWED_PERSONAL_DESTINATIONS_ENV) + ), + MCP_DESTINATION_SCOPE_GROUP: _coerce_pattern_list( + environment_value(MCP_ALLOWED_GROUP_DESTINATIONS_ENV) + ), + MCP_DESTINATION_SCOPE_GLOBAL: _coerce_pattern_list( + environment_value(MCP_ALLOWED_GLOBAL_DESTINATIONS_ENV) + ), + }, + "group_patterns": {}, + "denied_scope_patterns": {}, + "denied_group_patterns": {}, + } + + def get_mcp_destination_policy_config(settings=None, user_id=""): - """Return outbound MCP destination policy config from settings and environment.""" + """Combine supplied current settings with a non-overridable deployment floor.""" + environment_policy = _get_environment_destination_policy_config() scoped_group_patterns = _get_nested_setting(settings, "mcp_allowed_group_destination_overrides", {}) if not isinstance(scoped_group_patterns, dict): scoped_group_patterns = {} policy_config = { "enabled": _coerce_bool( - _get_nested_setting( - settings, - "enable_mcp_destination_governance", - _get_current_app_config_value( - "ENABLE_MCP_DESTINATION_GOVERNANCE", - os.getenv(ENABLE_MCP_DESTINATION_GOVERNANCE_ENV, "false"), - ), - ), - default=False, - ), + _get_nested_setting(settings, "enable_mcp_destination_governance") + ) or environment_policy["enabled"], "block_unsafe_destinations": _coerce_bool( - _get_nested_setting( - settings, - "mcp_block_unsafe_destinations", - _get_current_app_config_value( - "MCP_BLOCK_UNSAFE_DESTINATIONS", - os.getenv(MCP_BLOCK_UNSAFE_DESTINATIONS_ENV, "false"), - ), - ), - default=False, - ), + _get_nested_setting(settings, "mcp_block_unsafe_destinations") + ) or environment_policy["block_unsafe_destinations"], "common_patterns": _coerce_pattern_list( _get_nested_setting( - settings, - "mcp_allowed_destinations", - _get_current_app_config_value("MCP_ALLOWED_DESTINATIONS", os.getenv(MCP_ALLOWED_DESTINATIONS_ENV, "")), + settings, "mcp_allowed_destinations", environment_policy["common_patterns"] ) ), "scope_patterns": { @@ -385,30 +418,21 @@ def get_mcp_destination_policy_config(settings=None, user_id=""): _get_nested_setting( settings, "mcp_allowed_personal_destinations", - _get_current_app_config_value( - "MCP_ALLOWED_PERSONAL_DESTINATIONS", - os.getenv(MCP_ALLOWED_PERSONAL_DESTINATIONS_ENV, ""), - ), + environment_policy["scope_patterns"][MCP_DESTINATION_SCOPE_PERSONAL], ) ), MCP_DESTINATION_SCOPE_GROUP: _coerce_pattern_list( _get_nested_setting( settings, "mcp_allowed_group_destinations", - _get_current_app_config_value( - "MCP_ALLOWED_GROUP_DESTINATIONS", - os.getenv(MCP_ALLOWED_GROUP_DESTINATIONS_ENV, ""), - ), + environment_policy["scope_patterns"][MCP_DESTINATION_SCOPE_GROUP], ) ), MCP_DESTINATION_SCOPE_GLOBAL: _coerce_pattern_list( _get_nested_setting( settings, "mcp_allowed_global_destinations", - _get_current_app_config_value( - "MCP_ALLOWED_GLOBAL_DESTINATIONS", - os.getenv(MCP_ALLOWED_GLOBAL_DESTINATIONS_ENV, ""), - ), + environment_policy["scope_patterns"][MCP_DESTINATION_SCOPE_GLOBAL], ) ), }, @@ -488,7 +512,7 @@ def _unsafe_host_reason(hostname): def describe_mcp_destination(manifest): """Return normalized MCP destination metadata for a manifest.""" - if not isinstance(manifest, dict) or manifest.get("type") != MCP_PLUGIN_TYPE: + if not isinstance(manifest, dict) or not is_mcp_action(manifest): return { "is_mcp": False, "is_remote": False, @@ -497,6 +521,8 @@ def describe_mcp_destination(manifest): "normalized_endpoint": "", } + if is_retired_mcp_stdio(manifest): + raise McpStdioRemovedError() additional_fields = normalize_mcp_additional_fields(manifest.get("additionalFields", {})) transport = additional_fields.get("transport") endpoint = str(manifest.get("endpoint") or "").strip() @@ -535,9 +561,14 @@ def build_mcp_destination_log_context(manifest): try: descriptor = describe_mcp_destination(manifest) except ValueError as exc: + try: + mcp_action = is_mcp_action(manifest) + except ValueError: + mcp_action = False return { - "is_mcp": isinstance(manifest, dict) and manifest.get("type") == MCP_PLUGIN_TYPE, - "destination_error": str(exc), + "is_mcp": mcp_action, + "destination_error": "MCP destination configuration is invalid.", + "error_type": getattr(exc, "code", "validation"), } context = { @@ -621,28 +652,29 @@ def _denied_patterns_for_scope(policy_config, scope_type, scope_id): return patterns -def infer_mcp_destination_scope(manifest, fallback_scope=MCP_DESTINATION_SCOPE_PERSONAL): - """Infer an action scope from a stored MCP manifest when a caller does not pass one.""" - if not isinstance(manifest, dict): - return normalize_mcp_destination_scope(fallback_scope), "" - manifest_scope = str(manifest.get("scope") or "").strip().lower() - if manifest_scope: - scope_type = normalize_mcp_destination_scope(manifest_scope) - elif manifest.get("is_group"): - scope_type = MCP_DESTINATION_SCOPE_GROUP - elif manifest.get("is_global"): - scope_type = MCP_DESTINATION_SCOPE_GLOBAL - else: - scope_type = normalize_mcp_destination_scope(fallback_scope) - - if scope_type == MCP_DESTINATION_SCOPE_GROUP: - return scope_type, manifest.get("group_id") or manifest.get("scope_id") or "" - if scope_type == MCP_DESTINATION_SCOPE_GLOBAL: - return scope_type, MCP_DESTINATION_SCOPE_GLOBAL - return scope_type, manifest.get("user_id") or manifest.get("scope_id") or "" +def infer_mcp_destination_scope(manifest): + """Read server-bound origin; legacy JSON scope flags are not authority.""" + return resolve_mcp_destination_scope(manifest) -def evaluate_mcp_destination_policy(manifest, scope_type=None, scope_id="", policy_config=None, user_id=""): +def resolve_mcp_execution_context(manifest, *, origin=None): + """Require immutable provenance and the caller currently executing the action.""" + descriptor = describe_mcp_destination(manifest) + if not descriptor["is_mcp"]: + raise McpConfigurationError("Only an MCP action can use an MCP connector.") + action_origin = origin if origin is not None else get_action_origin(manifest) + if not isinstance(action_origin, McpActionOrigin): + raise McpDestinationPolicyError("MCP execution requires a trusted action origin.") + scope_type, scope_id = resolve_mcp_destination_scope(manifest, origin=action_origin) + user_id = get_mcp_execution_user_id() + if scope_type == MCP_DESTINATION_SCOPE_PERSONAL and user_id != scope_id: + raise McpDestinationPolicyError("MCP action is not authorized for this user.") + return action_origin, user_id + + +def evaluate_mcp_destination_policy( + manifest, scope_type=None, scope_id="", policy_config=None, user_id="", *, origin=None +): """Evaluate whether an MCP manifest may connect to its configured destination.""" descriptor = describe_mcp_destination(manifest) if not descriptor["is_mcp"] or not descriptor["is_remote"]: @@ -653,11 +685,20 @@ def evaluate_mcp_destination_policy(manifest, scope_type=None, scope_id="", poli "matched_pattern": "", } - policy = policy_config or get_mcp_destination_policy_config(user_id=user_id) - inferred_scope_type, inferred_scope_id = infer_mcp_destination_scope(manifest) - normalized_scope = normalize_mcp_destination_scope(scope_type or inferred_scope_type) - normalized_scope_id = scope_id if scope_id not in (None, "") else inferred_scope_id + normalized_scope, normalized_scope_id = resolve_mcp_destination_scope( + manifest, scope_type, scope_id, origin=origin + ) + policy = policy_config if policy_config is not None else get_mcp_destination_policy_config(user_id=user_id) + environment_decision = _evaluate_destination_descriptor_policy( + descriptor, normalized_scope, normalized_scope_id, _get_environment_destination_policy_config() + ) + if not environment_decision["allowed"]: + environment_decision["reason"] = "MCP destination is blocked by deployment policy." + return environment_decision + return _evaluate_destination_descriptor_policy(descriptor, normalized_scope, normalized_scope_id, policy) + +def _evaluate_destination_descriptor_policy(descriptor, normalized_scope, normalized_scope_id, policy): unsafe_reason = _unsafe_host_reason(descriptor.get("host")) if policy.get("block_unsafe_destinations") and unsafe_reason: return { @@ -721,6 +762,8 @@ def assert_mcp_destination_allowed( operation="mcp", user_id="", mcp_operation_id="", + *, + origin=None, ): """Raise when an outbound MCP destination is denied by policy.""" decision = evaluate_mcp_destination_policy( @@ -729,6 +772,7 @@ def assert_mcp_destination_allowed( scope_id=scope_id, policy_config=policy_config, user_id=user_id, + origin=origin, ) descriptor = decision.get("descriptor") or {} destination_context = build_mcp_destination_log_context(manifest) diff --git a/application/single_app/functions_mcp_operations.py b/application/single_app/functions_mcp_operations.py index 60d5e9f2f..6c00cee87 100644 --- a/application/single_app/functions_mcp_operations.py +++ b/application/single_app/functions_mcp_operations.py @@ -7,6 +7,8 @@ from jsonschema import Draft7Validator from jsonschema.exceptions import SchemaError +from functions_action_manifest import McpConfigurationError, McpStdioRemovedError + MCP_PLUGIN_TYPE = "mcp" MCP_DEFAULT_SERVER_PROFILE = "generic" @@ -16,12 +18,10 @@ MCP_DEFAULT_SSE_READ_TIMEOUT_SECONDS = 300 MCP_DEFAULT_RETRY_COUNT = 0 MCP_DEFAULT_RETRY_BACKOFF_SECONDS = 1 -MCP_STDIO_ENDPOINT = "stdio://local" MCP_SUPPORTED_TRANSPORTS = { "streamable_http", "sse", "websocket", - "stdio", } MCP_REMOTE_TRANSPORTS = { "streamable_http", @@ -87,19 +87,29 @@ def __init__(self, message, category="unknown", operation="mcp", detail=None, re def normalize_mcp_transport(value): """Normalize supported MCP transport aliases.""" + if value is not None and not isinstance(value, str): + raise McpConfigurationError("MCP transport must be a supported remote transport.") normalized_value = str(value or "").strip().lower().replace("-", "_") + if not normalized_value: + return MCP_DEFAULT_TRANSPORT + if normalized_value == "stdio": + raise McpStdioRemovedError() aliases = { "http": "streamable_http", "streamablehttp": "streamable_http", "streamable_http": "streamable_http", + "sse": "sse", "server_sent_events": "sse", "eventsource": "sse", "ws": "websocket", "wss": "websocket", "websocket": "websocket", - "stdio": "stdio", } - return aliases.get(normalized_value, MCP_DEFAULT_TRANSPORT) + if normalized_value not in aliases: + raise McpConfigurationError( + "Unsupported MCP transport. Choose streamable_http, sse, or websocket." + ) + return aliases[normalized_value] def normalize_mcp_server_profile(value): @@ -292,11 +302,11 @@ def get_mcp_custom_header_validation_errors(headers): def validate_mcp_endpoint_for_transport(endpoint, transport): """Return validation errors for an MCP endpoint and transport combination.""" + endpoint_text = str(endpoint or "").strip() + if endpoint_text.lower().startswith("stdio:"): + raise McpStdioRemovedError() normalized_transport = normalize_mcp_transport(transport) - if normalized_transport not in MCP_REMOTE_TRANSPORTS: - return [] - endpoint_text = str(endpoint or "").strip() if not endpoint_text: return ["MCP plugin requires an endpoint for remote transports"] if "\r" in endpoint_text or "\n" in endpoint_text: @@ -331,7 +341,22 @@ def classify_mcp_exception(exc, operation="mcp"): message = "MCP operation failed. Check the server endpoint, transport, authentication, and server logs." retryable = True - if isinstance(exc, TimeoutError) or "timeout" in detail_lower or "timed out" in detail_lower: + if isinstance(exc, McpConfigurationError): + category = exc.code + message = exc.public_message + detail = message + retryable = False + elif isinstance(exc, PermissionError): + category = "authorization" + message = "MCP action is not authorized for this operation." + detail = message + retryable = False + elif isinstance(exc, ValueError): + category = "validation" + message = "MCP configuration is invalid. Check the action settings." + detail = message + retryable = False + elif isinstance(exc, TimeoutError) or "timeout" in detail_lower or "timed out" in detail_lower: category = "timeout" message = "MCP operation timed out. Check timeout settings and server responsiveness." elif any(term in detail_lower for term in ("certificate", "ssl", "tls", "handshake failure")): @@ -369,6 +394,10 @@ def classify_mcp_exception(exc, operation="mcp"): def get_mcp_error_http_status(category): """Map an MCP error category to an HTTP status suitable for discovery responses.""" + if category in {"validation", "mcp_stdio_removed"}: + return 400 + if category == "authorization": + return 403 if category == "authentication": return 401 if category == "timeout": @@ -532,11 +561,11 @@ def validate_mcp_tool_arguments(tool, arguments): def normalize_mcp_additional_fields(additional_fields): """Normalize MCP additionalFields while preserving unknown future fields.""" normalized_fields = dict(additional_fields) if isinstance(additional_fields, dict) else {} + normalized_fields["transport"] = normalize_mcp_transport(normalized_fields.get("transport")) normalized_fields["server_profile"] = normalize_mcp_server_profile(normalized_fields.get("server_profile")) normalized_fields["preconfiguration_id"] = normalize_mcp_preconfiguration_id( normalized_fields.get("preconfiguration_id") ) - normalized_fields["transport"] = normalize_mcp_transport(normalized_fields.get("transport")) normalized_fields["auth_method"] = normalize_mcp_auth_method(normalized_fields.get("auth_method")) normalized_fields["api_key_header_name"] = str(normalized_fields.get("api_key_header_name") or "X-API-Key").strip() or "X-API-Key" normalized_fields["load_tools"] = bool(normalized_fields.get("load_tools", True)) @@ -573,9 +602,7 @@ def normalize_mcp_additional_fields(additional_fields): ) normalized_fields["mcp_tools"] = normalize_mcp_tool_metadata(normalized_fields.get("mcp_tools")) - if not isinstance(normalized_fields.get("args"), list): - normalized_fields["args"] = normalize_mcp_string_list(normalized_fields.get("args"), max_items=50) - if not isinstance(normalized_fields.get("env"), dict): - normalized_fields["env"] = {} + for retired_field in ("command", "args", "env"): + normalized_fields.pop(retired_field, None) return normalized_fields \ No newline at end of file diff --git a/application/single_app/functions_mcp_preconfigurations.py b/application/single_app/functions_mcp_preconfigurations.py index dc3ea05bd..c0f95ab7a 100644 --- a/application/single_app/functions_mcp_preconfigurations.py +++ b/application/single_app/functions_mcp_preconfigurations.py @@ -16,6 +16,7 @@ from jsonschema.exceptions import ValidationError from functions_appinsights import log_event +from functions_action_manifest import is_mcp_action from functions_mcp_catalog_implementations import ( McpImplementationValidationError, clear_mcp_implementation_schema_cache, @@ -27,9 +28,13 @@ MCP_DESTINATION_SCOPE_GROUP, MCP_DESTINATION_SCOPE_PERSONAL, McpDestinationPolicyError, + assert_mcp_destination_allowed, + describe_mcp_destination, evaluate_mcp_destination_policy, get_mcp_destination_policy_config, normalize_mcp_destination_scope, + resolve_mcp_destination_scope, + resolve_mcp_execution_context, ) from functions_mcp_operations import ( MCP_PLUGIN_TYPE, @@ -420,12 +425,47 @@ def _build_manifest_without_preconfiguration_match(manifest): return destination_manifest +def _filter_allowed_policy_patterns(policy_config, predicate): + """Preserve deny rules while testing a required, specific kind of grant.""" + filtered_policy = copy.deepcopy(policy_config) + filtered_policy["common_patterns"] = [ + pattern for pattern in policy_config.get("common_patterns", []) if predicate(pattern) + ] + for key in ("scope_patterns", "group_patterns"): + filtered_policy[key] = { + scope: [pattern for pattern in patterns if predicate(pattern)] + for scope, patterns in policy_config.get(key, {}).items() + } + return filtered_policy + + +def _evaluate_explicit_preconfiguration_policy( + preconfiguration, manifest, scope_type, scope_id, user_id, policy_config +): + required_pattern = f"{MCP_PRECONFIGURATION_POLICY_PREFIX}{preconfiguration['id']}" + explicit_policy = _filter_allowed_policy_patterns( + policy_config, lambda pattern: str(pattern).strip().lower() == required_pattern + ) + return evaluate_mcp_destination_policy( + manifest, + scope_type=scope_type, + scope_id=scope_id, + policy_config=explicit_policy, + user_id=user_id, + ) + + def _evaluate_specific_destination_policy(manifest, scope_type, scope_id="", user_id="", policy_config=None): + if policy_config is None: + policy_config = get_mcp_destination_policy_config(user_id=user_id) + specific_policy = _filter_allowed_policy_patterns( + policy_config, lambda pattern: _is_specific_destination_policy_match({"matched_pattern": pattern}) + ) return evaluate_mcp_destination_policy( _build_manifest_without_preconfiguration_match(manifest), scope_type=scope_type, scope_id=scope_id, - policy_config=policy_config, + policy_config=specific_policy, user_id=user_id, ) @@ -453,6 +493,8 @@ def _is_preconfiguration_available_for_scope( if not _is_scope_eligible(preconfiguration, normalized_scope): return False + if policy_config is None: + policy_config = get_mcp_destination_policy_config(user_id=user_id) manifest = _build_preconfiguration_manifest(preconfiguration) decision = evaluate_mcp_destination_policy( manifest, @@ -465,8 +507,12 @@ def _is_preconfiguration_available_for_scope( return False if _requires_explicit_preconfiguration_policy(preconfiguration): + explicit_decision = _evaluate_explicit_preconfiguration_policy( + preconfiguration, manifest, normalized_scope, scope_id, user_id, policy_config + ) return ( - _is_explicit_preconfiguration_policy_match(preconfiguration, decision) + explicit_decision.get("allowed") + and _is_explicit_preconfiguration_policy_match(preconfiguration, explicit_decision) and _enterprise_destination_policy_is_allowed( preconfiguration, manifest, @@ -509,19 +555,26 @@ def build_mcp_server_preconfigurations_response( def evaluate_mcp_preconfiguration_manifest_policy( manifest, - scope_type=MCP_DESTINATION_SCOPE_PERSONAL, + scope_type=None, scope_id="", user_id="", settings=None, + *, + origin=None, + policy_config=None, ): """Evaluate catalog-specific MCP preconfiguration policy for a submitted manifest.""" - if not isinstance(manifest, dict) or manifest.get("type") != MCP_PLUGIN_TYPE: + if not isinstance(manifest, dict) or not is_mcp_action(manifest): return { "allowed": True, "reason": "not_mcp_preconfiguration", "matched_pattern": "", } + describe_mcp_destination(manifest) + scope_type, scope_id = resolve_mcp_destination_scope( + manifest, scope_type, scope_id, origin=origin + ) additional_fields = manifest.get("additionalFields") if isinstance(manifest.get("additionalFields"), dict) else {} preconfiguration_id = normalize_mcp_preconfiguration_id(additional_fields.get("preconfiguration_id")) if not preconfiguration_id: @@ -540,6 +593,14 @@ def evaluate_mcp_preconfiguration_manifest_policy( "preconfiguration_id": preconfiguration_id, } + if not _is_scope_eligible(preconfiguration, scope_type): + return { + "allowed": False, + "reason": "MCP preconfiguration is not available for this action scope.", + "matched_pattern": "", + "preconfiguration_id": preconfiguration_id, + } + if not _requires_explicit_preconfiguration_policy(preconfiguration): return { "allowed": True, @@ -548,13 +609,10 @@ def evaluate_mcp_preconfiguration_manifest_policy( "preconfiguration_id": preconfiguration_id, } - policy_config = get_mcp_destination_policy_config(settings, user_id=user_id) - decision = evaluate_mcp_destination_policy( - manifest, - scope_type=scope_type, - scope_id=scope_id, - policy_config=policy_config, - user_id=user_id, + if policy_config is None: + policy_config = get_mcp_destination_policy_config(settings, user_id=user_id) + decision = _evaluate_explicit_preconfiguration_policy( + preconfiguration, manifest, scope_type, scope_id, user_id, policy_config ) if decision.get("allowed") and _is_explicit_preconfiguration_policy_match(preconfiguration, decision): if not _enterprise_destination_policy_is_allowed( @@ -588,11 +646,14 @@ def evaluate_mcp_preconfiguration_manifest_policy( def assert_mcp_preconfiguration_manifest_allowed( manifest, - scope_type=MCP_DESTINATION_SCOPE_PERSONAL, + scope_type=None, scope_id="", user_id="", settings=None, operation="mcp", + *, + origin=None, + policy_config=None, ): """Raise when a submitted MCP manifest uses a gated preconfiguration without explicit policy.""" decision = evaluate_mcp_preconfiguration_manifest_policy( @@ -601,12 +662,38 @@ def assert_mcp_preconfiguration_manifest_allowed( scope_id=scope_id, user_id=user_id, settings=settings, + origin=origin, + policy_config=policy_config, ) if decision.get("allowed"): return decision - raise McpDestinationPolicyError( - f"{decision.get('reason')} Operation '{operation}' is not allowed for this MCP preconfiguration." + raise McpDestinationPolicyError(decision.get("reason") or "MCP preconfiguration is not authorized.") + + +def authorize_mcp_action(manifest, *, origin=None, settings, operation="mcp"): + """Authorize credential or connector use with current caller and owner-supplied settings.""" + action_origin, user_id = resolve_mcp_execution_context(manifest, origin=origin) + if not isinstance(settings, dict): + raise McpDestinationPolicyError("MCP destination policy is currently unavailable.") + + policy_config = get_mcp_destination_policy_config(settings, user_id=user_id) + assert_mcp_destination_allowed( + manifest, + policy_config=policy_config, + operation=operation, + user_id=user_id, + mcp_operation_id=str(manifest.get("mcp_operation_id") or ""), + origin=action_origin, + ) + assert_mcp_preconfiguration_manifest_allowed( + manifest, + user_id=user_id, + settings=settings, + operation=operation, + origin=action_origin, + policy_config=policy_config, ) + return action_origin def clear_mcp_server_preconfiguration_cache(): diff --git a/application/single_app/functions_mcp_presets.py b/application/single_app/functions_mcp_presets.py index 61e200474..0d04d552a 100644 --- a/application/single_app/functions_mcp_presets.py +++ b/application/single_app/functions_mcp_presets.py @@ -27,6 +27,7 @@ MCP_PRESET_PATHS_ENV = "SIMPLECHAT_MCP_PRESET_PATHS" MCP_PRESET_BUNDLED_SOURCE = "bundled" MCP_PRESET_CUSTOM_SOURCE = "custom" +MCP_PRESET_REMOTE_TRANSPORTS = {"streamable_http", "sse", "websocket"} MCP_PRESETS_ROOT = os.path.join(os.path.dirname(__file__), "mcp_presets") MCP_PRESET_SCHEMA_PATH = os.path.join(MCP_PRESETS_ROOT, MCP_PRESET_SCHEMA_FILE) @@ -59,10 +60,9 @@ "websocketEndpointPlaceholder": "wss://example.com/mcp", }, "constraints": { - "allowedTransports": ["streamable_http", "sse", "websocket", "stdio"], + "allowedTransports": ["streamable_http", "sse", "websocket"], "allowedAuthMethods": ["none", "bearer", "api_key", "basic", "identity"], "customHeadersAllowed": True, - "stdioAllowed": True, }, "implementation": { "id": MCP_DEFAULT_SERVER_PRESET_ID, @@ -80,6 +80,10 @@ class McpPresetValidationError(ValueError): """Raised when an MCP preset definition fails validation.""" +class McpPresetUnavailableError(McpPresetValidationError): + """Raised when a preset requires a retired MCP transport.""" + + def normalize_mcp_preset_id(value): """Normalize a preset identifier without accepting unsafe file path syntax.""" normalized_value = str(value or "").strip().lower() @@ -143,6 +147,40 @@ def _iter_preset_definition_paths(): yield os.path.join(directory, file_name), source +def _normalize_remote_mcp_preset(definition): + """Sanitize a copy of legacy remote presets without changing their default.""" + normalized = copy.deepcopy(definition) + if not isinstance(normalized, dict): + return normalized + + defaults = normalized.get("defaults") + default_transport = defaults.get("transport") if isinstance(defaults, dict) else None + if isinstance(default_transport, str) and default_transport.strip().lower() == "stdio": + raise McpPresetUnavailableError( + "MCP preset is unavailable because its default transport is stdio, which is no longer supported." + ) + + constraints = normalized.get("constraints") + if isinstance(constraints, dict): + constraints.pop("stdioAllowed", None) + transports = constraints.get("allowedTransports") + if isinstance(transports, list): + remote_transports = [ + transport for transport in transports + if not (isinstance(transport, str) and transport.strip().lower() == "stdio") + ] + if not any( + isinstance(transport, str) and transport in MCP_PRESET_REMOTE_TRANSPORTS + for transport in remote_transports + ): + raise McpPresetUnavailableError( + "MCP preset is unavailable because it has no supported remote transport." + ) + constraints["allowedTransports"] = remote_transports + + return normalized + + def _validate_mcp_preset(definition, file_path): schema = _load_mcp_preset_schema() validator = Draft7Validator(schema) @@ -151,6 +189,9 @@ def _validate_mcp_preset(definition, file_path): messages = "; ".join(error.message for error in errors) raise McpPresetValidationError(f"{os.path.basename(file_path)} failed schema validation: {messages}") + if definition["defaults"]["transport"] not in definition["constraints"]["allowedTransports"]: + raise McpPresetValidationError("The default MCP transport must be allowed by the preset.") + preset_id = normalize_mcp_preset_id(definition.get("id")) if definition.get("id") != preset_id: raise McpPresetValidationError(f"{os.path.basename(file_path)} has an invalid preset id.") @@ -191,7 +232,17 @@ def load_mcp_server_presets(): for file_path, source in _iter_preset_definition_paths(): try: definition = _load_json_file(file_path) + definition = _normalize_remote_mcp_preset(definition) _validate_mcp_preset(definition, file_path) + except McpPresetUnavailableError as exc: + preset_id = definition.get("id") if isinstance(definition, dict) else None + safe_preset_id = preset_id if isinstance(preset_id, str) and MCP_PRESET_ID_PATTERN.fullmatch(preset_id) else "unknown" + log_event( + f"[MCP_PRESETS] Preset '{safe_preset_id}' skipped. {exc}", + level=logging.WARNING, + debug_only=True, + ) + continue except ( OSError, json.JSONDecodeError, @@ -200,7 +251,7 @@ def load_mcp_server_presets(): McpPresetValidationError, ) as exc: log_event( - f"[MCP_PRESETS] Failed to load MCP preset definition: {exc}", + f"[MCP_PRESETS] Invalid MCP preset definition skipped ({type(exc).__name__}).", level=logging.WARNING, debug_only=True, ) diff --git a/application/single_app/functions_personal_actions.py b/application/single_app/functions_personal_actions.py index d51aaf4a6..b83d444f7 100644 --- a/application/single_app/functions_personal_actions.py +++ b/application/single_app/functions_personal_actions.py @@ -7,20 +7,52 @@ personal_actions container with user_id partitioning. """ +import logging import uuid -from datetime import datetime +from copy import deepcopy +from collections import Counter +from datetime import datetime, timezone +from azure.core import MatchConditions from azure.cosmos import exceptions -from flask import current_app -from functions_keyvault import keyvault_plugin_save_helper, keyvault_plugin_get_helper, keyvault_plugin_delete_helper, SecretReturnType -from functions_settings import get_user_settings, update_user_settings +import functions_settings as user_settings_service +from functions_action_manifest import ( + McpConfigurationError, + McpStdioRemovedError, + bind_action_origin, + is_retired_mcp_stdio, + resolve_action_type, +) +from functions_appinsights import log_event +from functions_keyvault import ( + SecretReturnType, + clean_name_for_keyvault, + keyvault_plugin_delete_helper, + keyvault_plugin_get_helper, + keyvault_plugin_save_helper, + redact_plugin_secret_values, +) +from functions_legacy_action_management import ( + LEGACY_ACTION_PREFIX, + LegacyActionConflictError, + LegacyActionSecretConflictError, + LegacyActionSourceUpdateError, + action_snapshot_digest, + authorize_scoped_mcp_secret_read, + find_legacy_action_snapshot, + is_unchanged_legacy_action, + legacy_action_management_view, + legacy_action_snapshots, + prepare_scoped_action, + retired_action_management_view, + validate_action_configuration, + validate_scoped_mcp_action, +) from functions_workspace_identities import ( WORKSPACE_IDENTITY_SCOPE_PERSONAL, hydrate_action_identity_reference, validate_action_identity_reference, ) -from functions_debug import debug_print -from config import cosmos_personal_actions_container -import logging +from config import cosmos_personal_actions_container, cosmos_user_settings_container from functions_governance import ensure_action_type_access, filter_actions_by_action_type_access from functions_chat_bootstrap_cache import bump_chat_bootstrap_user_cache_version @@ -36,7 +68,73 @@ def get_governed_personal_actions(user_id, return_type=SecretReturnType.TRIGGER) list: List of action/plugin dictionaries """ actions = get_personal_actions(user_id, return_type=return_type) - return filter_actions_by_action_type_access(user_id, actions, 'governance_user_actions', 'personal') + active_actions = [action for action in actions if not is_retired_mcp_stdio(action)] + allowed_actions = filter_actions_by_action_type_access( + user_id, active_actions, 'governance_user_actions', 'personal' + ) + return [action for action in actions if is_retired_mcp_stdio(action) or action in allowed_actions] + + +def _clean_action(action, user_id, return_type): + if return_type == SecretReturnType.NAME: + return bind_action_origin( + {key: value for key, value in action.items() if not key.startswith("_")}, + "personal", + user_id, + ) + retired_view = retired_action_management_view(action, "personal", user_id) + if retired_view is not None: + return retired_view + cleaned = {key: value for key, value in action.items() if not key.startswith("_")} + cleaned = bind_action_origin(cleaned, "personal", user_id) + if return_type == SecretReturnType.VALUE and cleaned["type"] == "mcp": + ensure_action_type_access("governance_user_actions", user_id, "mcp", "personal") + authorize_scoped_mcp_secret_read(cleaned, user_settings_service.get_settings()) + cleaned = keyvault_plugin_get_helper( + cleaned, scope_value=user_id, scope="user", return_type=return_type + ) + cleaned = hydrate_action_identity_reference( + cleaned, + WORKSPACE_IDENTITY_SCOPE_PERSONAL, + user_id, + return_type=return_type, + ) + return bind_action_origin(cleaned, "personal", user_id) + + +def _clean_actions(actions, user_id, return_type): + try: + return [_clean_action(action, user_id, return_type) for action in actions] + except Exception as exc: + log_event("[PLUGINS] Personal action normalization failed", + level=logging.WARNING, extra={"user_id": user_id, "error_type": type(exc).__name__}) + raise + + +def get_personal_action_record(user_id, action_id): + """Read an exact personal ID without secret hydration; never send this to a browser.""" + try: + action = cosmos_personal_actions_container.read_item(item=action_id, partition_key=user_id) + except exceptions.CosmosResourceNotFoundError: + return None + return bind_action_origin(action, "personal", user_id) + + +def _find_personal_action_record(user_id, action_id): + action = get_personal_action_record(user_id, action_id) + if action is not None: + return action + actions = list(cosmos_personal_actions_container.query_items( + query="SELECT * FROM c WHERE c.user_id = @user_id AND c.name = @name", + parameters=[ + {"name": "@user_id", "value": user_id}, + {"name": "@name", "value": action_id}, + ], + partition_key=user_id, + )) + if len(actions) > 1: + raise LegacyActionConflictError() + return bind_action_origin(actions[0], "personal", user_id) if actions else None def get_personal_actions(user_id, return_type=SecretReturnType.TRIGGER): """ @@ -47,6 +145,9 @@ def get_personal_actions(user_id, return_type=SecretReturnType.TRIGGER): Returns: list: List of action/plugin dictionaries + + NAME returns internal originals without secret or identity hydration. + Browser callers must use TRIGGER or an explicit retired management projection. """ try: query = "SELECT * FROM c WHERE c.user_id = @user_id" @@ -58,25 +159,13 @@ def get_personal_actions(user_id, return_type=SecretReturnType.TRIGGER): partition_key=user_id )) - # Remove Cosmos metadata for cleaner response and resolve Key Vault references - cleaned_actions = [] - for action in actions: - cleaned_action = {k: v for k, v in action.items() if not k.startswith('_')} - cleaned_action = keyvault_plugin_get_helper(cleaned_action, scope_value=user_id, scope="user", return_type=return_type) - cleaned_action = hydrate_action_identity_reference( - cleaned_action, - WORKSPACE_IDENTITY_SCOPE_PERSONAL, - user_id, - return_type=return_type, - ) - cleaned_actions.append(cleaned_action) - return cleaned_actions - except exceptions.CosmosResourceNotFoundError: return [] - except Exception as e: - debug_print(f"Error fetching personal actions for user {user_id}: {e}") - return [] + except Exception as exc: + log_event("[PLUGINS] Personal action listing failed", level=logging.ERROR, + extra={"user_id": user_id, "error_type": type(exc).__name__}) + raise + return _clean_actions(actions, user_id, return_type) def get_personal_action(user_id, action_id, return_type=SecretReturnType.TRIGGER): """ @@ -90,43 +179,16 @@ def get_personal_action(user_id, action_id, return_type=SecretReturnType.TRIGGER dict: Action dictionary or None if not found """ try: - try: - action = cosmos_personal_actions_container.read_item( - item=action_id, - partition_key=user_id - ) - except exceptions.CosmosResourceNotFoundError: - # If not found by ID, try to find by name - query = "SELECT * FROM c WHERE c.user_id = @user_id AND c.name = @name" - parameters = [ - {"name": "@user_id", "value": user_id}, - {"name": "@name", "value": action_id} - ] - - actions = list(cosmos_personal_actions_container.query_items( - query=query, - parameters=parameters, - partition_key=user_id - )) - - if not actions: - return None - action = actions[0] - - # Remove Cosmos metadata and resolve Key Vault references - cleaned_action = {k: v for k, v in action.items() if not k.startswith('_')} - cleaned_action = keyvault_plugin_get_helper(cleaned_action, scope_value=user_id, scope="user", return_type=return_type) - cleaned_action = hydrate_action_identity_reference( - cleaned_action, - WORKSPACE_IDENTITY_SCOPE_PERSONAL, - user_id, - return_type=return_type, - ) - return cleaned_action - - except Exception as e: - debug_print(f"Error fetching action {action_id} for user {user_id}: {e}") + action = _find_personal_action_record(user_id, action_id) + except exceptions.CosmosResourceNotFoundError: + return None + except Exception as exc: + log_event("[PLUGINS] Personal action lookup failed", level=logging.ERROR, + extra={"user_id": user_id, "action_id": action_id, "error_type": type(exc).__name__}) + raise + if action is None: return None + return _clean_actions([action], user_id, return_type)[0] def save_personal_action(user_id, action_data, enforce_governance=True): """ @@ -139,21 +201,23 @@ def save_personal_action(user_id, action_data, enforce_governance=True): Returns: dict: Saved action data with ID """ + return _save_personal_action(user_id, action_data, enforce_governance=enforce_governance) + + +def _save_personal_action(user_id, action_data, enforce_governance=True, migration_snapshot=None): try: - # Check if an action with this name already exists + action_data = prepare_scoped_action(action_data, "personal", user_id) + if action_data.get("id") and ( + not isinstance(action_data["id"], str) or action_data["id"].startswith(LEGACY_ACTION_PREFIX) + ): + raise ValueError("Action ID is invalid.") existing_action = None if action_data.get('id'): - existing_action = get_personal_action( - user_id, - action_data['id'], - return_type=SecretReturnType.NAME, - ) - if 'name' in action_data and action_data['name']: - existing_action = existing_action or get_personal_action( - user_id, - action_data['name'], - return_type=SecretReturnType.NAME, - ) + existing_action = get_personal_action_record(user_id, action_data['id']) + elif action_data.get('name'): + existing_action = _find_personal_action_record(user_id, action_data['name']) + if migration_snapshot is not None and existing_action is not None: + raise LegacyActionConflictError() # Preserve existing ID if updating, or generate new ID if creating now = datetime.utcnow().isoformat() @@ -177,12 +241,6 @@ def save_personal_action(user_id, action_data, enforce_governance=True): action_data['user_id'] = user_id action_data['last_updated'] = now - validate_action_identity_reference( - action_data, - WORKSPACE_IDENTITY_SCOPE_PERSONAL, - user_id, - ) - # Validate required fields required_fields = ['name', 'displayName', 'type', 'description'] for field in required_fields: @@ -206,6 +264,19 @@ def save_personal_action(user_id, action_data, enforce_governance=True): if enforce_governance: ensure_action_type_access('governance_user_actions', user_id, action_data.get('type'), 'personal') + + action_data = bind_action_origin(action_data, "personal", user_id) + if action_data["type"] == "mcp": + validate_scoped_mcp_action(action_data, user_id, user_settings_service.get_settings()) + if migration_snapshot is not None: + validate_action_configuration(action_data) + validate_action_identity_reference( + action_data, + WORKSPACE_IDENTITY_SCOPE_PERSONAL, + user_id, + ) + _ensure_personal_secret_name_available(user_id, action_data, migration_snapshot) + configuration_digest = _action_configuration_digest(action_data) # Store secrets in Key Vault before upsert action_data = keyvault_plugin_save_helper( @@ -214,14 +285,28 @@ def save_personal_action(user_id, action_data, enforce_governance=True): scope="user", existing_plugin=existing_action, ) - result = cosmos_personal_actions_container.upsert_item(body=action_data) + if migration_snapshot is not None: + action_data["_legacy_migration"] = { + "source_locator": migration_snapshot.locator, + "destination_digest": _stored_action_digest(action_data), + "configuration_digest": configuration_digest, + } + try: + result = cosmos_personal_actions_container.create_item(body=action_data) + except exceptions.CosmosHttpResponseError as exc: + if exc.status_code == 409: + raise LegacyActionConflictError() from exc + raise + else: + result = cosmos_personal_actions_container.upsert_item(body=action_data) # Remove Cosmos metadata from response cleaned_result = {k: v for k, v in result.items() if not k.startswith('_')} bump_chat_bootstrap_user_cache_version(user_id, reason="personal_action_saved") - return cleaned_result + return bind_action_origin(cleaned_result, "personal", user_id) - except Exception as e: - debug_print(f"Error saving action for user {user_id}: {e}") + except Exception as exc: + log_event("[PLUGINS] Personal action save failed", level=logging.ERROR, + extra={"user_id": user_id, "error_type": type(exc).__name__}) raise def delete_personal_action(user_id, action_id): @@ -237,11 +322,14 @@ def delete_personal_action(user_id, action_id): """ try: # Try to find the action first to get the correct ID - action = get_personal_action(user_id, action_id, return_type=SecretReturnType.NAME) + if isinstance(action_id, str) and action_id.startswith(LEGACY_ACTION_PREFIX): + return delete_legacy_personal_action(user_id, action_id) + action = _find_personal_action_record(user_id, action_id) if not action: return False - ensure_action_type_access('governance_user_actions', user_id, action.get('type'), 'personal') + if not is_retired_mcp_stdio(action): + ensure_action_type_access('governance_user_actions', user_id, resolve_action_type(action), 'personal') # Delete secrets from Key Vault before deleting the action keyvault_plugin_delete_helper(action, scope_value=user_id, scope="user") @@ -254,93 +342,521 @@ def delete_personal_action(user_id, action_id): except exceptions.CosmosResourceNotFoundError: return False - except Exception as e: - debug_print(f"Error deleting action {action_id} for user {user_id}: {e}") + except Exception as exc: + log_event("[PLUGINS] Personal action deletion failed", level=logging.ERROR, + extra={"user_id": user_id, "action_id": action_id, "error_type": type(exc).__name__}) raise -def ensure_migration_complete(user_id): +def _read_legacy_settings_document(user_id): + # The settings accessor remains the object-level authorization boundary. + # Its request cache cannot prove that a source is unchanged before deletion. + user_settings_service.get_user_settings(user_id) + try: + document = cosmos_user_settings_container.read_item(item=user_id, partition_key=user_id) + except exceptions.CosmosResourceNotFoundError: + document = {"id": user_id, "settings": {"plugins": []}} + return deepcopy(document) + + +def _read_legacy_settings_for_preflight(user_id): + # Reuse the settings accessor's authorization without its profile repair or + # default-document writes; rejecting an import must leave all stored data alone. + user_settings_service._authorize_user_settings_access(user_id, "update") + try: + document = cosmos_user_settings_container.read_item(item=user_id, partition_key=user_id) + except exceptions.CosmosResourceNotFoundError: + document = {"id": user_id, "settings": {"plugins": []}} + return deepcopy(document) + + +def _document_plugins(document): + settings = document.get("settings") or {} + plugins = settings.get("plugins", []) + return [] if plugins is None else plugins + + +def _get_legacy_snapshot(user_id, locator): + document = _read_legacy_settings_document(user_id) + return find_legacy_action_snapshot(user_id, _document_plugins(document), locator) + + +def _ensure_legacy_management_access(user_id, snapshot): + if not is_retired_mcp_stdio(snapshot.record): + ensure_action_type_access( + "governance_user_actions", user_id, resolve_action_type(snapshot.record), "personal" + ) + + +def list_legacy_personal_actions(user_id): + """List safe, management-only views without importing legacy actions.""" + try: + document = _read_legacy_settings_document(user_id) + views = [] + for snapshot in legacy_action_snapshots(user_id, _document_plugins(document)): + try: + _ensure_legacy_management_access(user_id, snapshot) + except PermissionError: + continue + views.append(legacy_action_management_view(snapshot)) + return views + except Exception as exc: + log_event("[PLUGINS] Legacy action listing failed", level=logging.ERROR, + extra={"user_id": user_id, "error_type": type(exc).__name__}) + raise + + +def get_legacy_personal_action(user_id, locator): + """Resolve an authorized legacy locator to its credential-free management view.""" + snapshot = _get_legacy_snapshot(user_id, locator) + _ensure_legacy_management_access(user_id, snapshot) + return legacy_action_management_view(snapshot) + + +def get_legacy_personal_action_record(user_id, locator): + """Return an internal source copy for validation, never for browser serialization.""" + snapshot = _get_legacy_snapshot(user_id, locator) + _ensure_legacy_management_access(user_id, snapshot) + return deepcopy(snapshot.record) + + +def is_unchanged_legacy_personal_action(user_id, submitted): + """Qualify an authorized unchanged management view without importing its source.""" + if not isinstance(submitted, dict): + return False + try: + snapshot = _get_legacy_snapshot(user_id, submitted.get("id")) + except LegacyActionConflictError: + return False + _ensure_legacy_management_access(user_id, snapshot) + return is_unchanged_legacy_action(submitted, snapshot) + + +def prepare_legacy_personal_actions_update(user_id, submitted_plugins): + """Preflight a settings.plugins update without action, secret, or settings writes. + + The returned ``plugins`` list contains authoritative raw source records and is + backend-only. Callers gate ``has_imports`` with allow_user_plugins before any + other mutations, and can omit the plugins update entirely when ``changed`` is + false. Existing legacy actions are reconfigured through the dedicated Actions + operation, which stores a destination before removing its exact source. """ - Ensure that migration is complete by checking for and cleaning up any remaining legacy data. - This is more thorough than just checking if personal container is empty. - - Args: - user_id (str): The user's unique identifier - - Returns: - int: Number of actions migrated (0 if already migrated) + if not isinstance(submitted_plugins, list): + raise McpConfigurationError("Actions must be provided as a list.") + document = _read_legacy_settings_for_preflight(user_id) + originals = _document_plugins(document) + snapshots = legacy_action_snapshots(user_id, originals) + by_locator = {snapshot.locator: snapshot for snapshot in snapshots} + by_id = {} + for snapshot in snapshots: + if isinstance(snapshot.record, dict) and isinstance(snapshot.record.get("id"), str): + by_id.setdefault(snapshot.record["id"], []).append(snapshot) + + replacements = {} + additions = [] + imported = [] + seen_ids = set() + for submitted in submitted_plugins: + if not isinstance(submitted, dict): + raise McpConfigurationError("Each action must be an object.") + submitted_id = submitted.get("id") + if submitted_id is not None and not isinstance(submitted_id, str): + raise McpConfigurationError("Action identifiers must be strings.") + if submitted_id: + if submitted_id in seen_ids: + raise McpConfigurationError("Each action identifier must occur only once.") + seen_ids.add(submitted_id) + + if submitted_id and submitted_id.startswith(LEGACY_ACTION_PREFIX): + snapshot = by_locator.get(submitted_id) + if snapshot is None: + raise LegacyActionConflictError() + if is_unchanged_legacy_action(submitted, snapshot): + _ensure_legacy_management_access(user_id, snapshot) + if snapshot.index in replacements: + raise LegacyActionConflictError() + replacements[snapshot.index] = deepcopy(snapshot.record) + continue + if is_retired_mcp_stdio(submitted): + raise McpStdioRemovedError() + raise McpConfigurationError("Use Actions to reconfigure an existing legacy action.") + + if is_retired_mcp_stdio(submitted): + raise McpStdioRemovedError() + if any(field in submitted for field in ( + "is_legacy", "legacy_source", "legacy_locator", "execution_status", + )): + raise McpConfigurationError("Legacy management fields require a current action locator.") + + matches = by_id.get(submitted_id, []) if submitted_id else [ + snapshot for snapshot in snapshots if snapshot.record == submitted + ] + if len(matches) > 1: + raise LegacyActionConflictError() + snapshot = matches[0] if matches else None + if snapshot is not None: + if snapshot.index in replacements: + raise LegacyActionConflictError() + if is_retired_mcp_stdio(snapshot.record): + raise McpConfigurationError("Use Actions to reconfigure an existing legacy action.") + + payload = _prepare_personal_action_configuration(user_id, submitted) + _ensure_personal_secret_name_available( + user_id, payload, snapshot, legacy_sources=snapshots + ) + if snapshot is not None and submitted == snapshot.record: + replacements[snapshot.index] = deepcopy(snapshot.record) + else: + imported.append(payload) + if snapshot is None: + additions.append(payload) + else: + replacements[snapshot.index] = payload + + prepared = [] + for snapshot in snapshots: + if snapshot.index in replacements: + prepared.append(replacements[snapshot.index]) + elif is_retired_mcp_stdio(snapshot.record): + prepared.append(deepcopy(snapshot.record)) + else: + _ensure_legacy_management_access(user_id, snapshot) + prepared.extend(additions) + for payload in imported: + sentinel = object() + if redact_plugin_secret_values(payload, redaction_value=sentinel) == payload: + continue + secret_name = clean_name_for_keyvault(payload.get("name", "")).lower() + for other in prepared: + if other is payload or not isinstance(other, dict): + continue + name = other.get("name") + if isinstance(name, str) and clean_name_for_keyvault(name).lower() == secret_name: + raise LegacyActionSecretConflictError() + return { + "plugins": deepcopy(prepared), + "has_imports": bool(imported), + "changed": prepared != originals, + "source_etag": document.get("_etag"), + } + + +def prepare_legacy_personal_action_reconfiguration(user_id, locator, replacement): + """Preflight conversion without writes; commit through the reconfiguration API. + + The result is an internal validated manifest with the destination's real ID. + Keep the management locator separately for the later commit operation. """ + document = _read_legacy_settings_for_preflight(user_id) + plugins = _document_plugins(document) + snapshots = legacy_action_snapshots(user_id, plugins) + snapshot = find_legacy_action_snapshot(user_id, plugins, locator) + payload = _legacy_destination_payload( + user_id, snapshot, replacement=replacement, legacy_sources=snapshots + ) + _verified_legacy_destination(user_id, snapshot, payload) + return payload + + +def prepare_legacy_action_settings_update(user_id, incoming_plugins): + """Return backend-only plugins, has_imports, changed, and source_etag without writes.""" + return prepare_legacy_personal_actions_update(user_id, incoming_plugins) + + +def validate_legacy_personal_action_reconfiguration(user_id, locator, replacement): + """Return the validated destination manifest without committing the conversion.""" + return prepare_legacy_personal_action_reconfiguration(user_id, locator, replacement) + + +def _remove_legacy_snapshot(user_id, snapshot): + document = _read_legacy_settings_document(user_id) + plugins = _document_plugins(document) + current = find_legacy_action_snapshot(user_id, plugins, snapshot.locator) + if current.owner_id != snapshot.owner_id or current.record != snapshot.record: + raise LegacyActionConflictError() + if not document.get("_etag"): + raise LegacyActionSourceUpdateError() + + updated = deepcopy(document) + updated["settings"]["plugins"] = plugins[:current.index] + plugins[current.index + 1:] + updated["lastUpdated"] = datetime.now(timezone.utc).isoformat() try: - user_settings = get_user_settings(user_id) - plugins = user_settings.get('settings', {}).get('plugins', []) - - # If there are still legacy plugins, migrate them - if plugins: - # Check if we already have personal actions to avoid duplicate migration - existing_personal_actions = get_personal_actions(user_id) - - # Only migrate if we don't already have personal actions or if legacy count is higher - if not existing_personal_actions or len(plugins) > len(existing_personal_actions): - return migrate_actions_from_user_settings(user_id) + stored = cosmos_user_settings_container.replace_item( + item=user_id, + body=updated, + etag=document["_etag"], + match_condition=MatchConditions.IfNotModified, + ) + except exceptions.CosmosHttpResponseError as exc: + if exc.status_code == 412: + raise LegacyActionConflictError() from exc + raise LegacyActionSourceUpdateError() from exc + except Exception as exc: + raise LegacyActionSourceUpdateError() from exc + if not isinstance(stored, dict) or _document_plugins(stored) != updated["settings"]["plugins"]: + raise LegacyActionSourceUpdateError() + + try: + user_settings_service._set_request_cached_user_settings(user_id, stored) + user_settings_service._delete_user_ui_settings_cache(user_id) + bump_chat_bootstrap_user_cache_version(user_id, reason="legacy_action_removed") + except Exception as exc: + log_event("[PLUGINS] Legacy action cache refresh failed after verified source update", + level=logging.WARNING, extra={"user_id": user_id, "error_type": type(exc).__name__}) + + +def delete_legacy_personal_action(user_id, locator): + """Explicitly remove one exact source; retired cleanup is not MCP usage.""" + snapshot = _get_legacy_snapshot(user_id, locator) + _ensure_legacy_management_access(user_id, snapshot) + _remove_legacy_snapshot(user_id, snapshot) + return True + + +def _stored_action_digest(action): + return action_snapshot_digest({ + key: value for key, value in action.items() if not key.startswith("_") + }) + + +def _action_configuration_digest(action): + audit_fields = { + "created_by", "created_at", "modified_by", "modified_at", "last_updated", "updated_at", + } + return action_snapshot_digest({ + key: value for key, value in action.items() + if not key.startswith("_") and key not in audit_fields + }) + + +def _ensure_personal_secret_name_available(user_id, payload, snapshot=None, *, legacy_sources=None): + """Prevent distinct IDs from sharing Key Vault's scope-and-name secret keys.""" + sentinel = object() + redacted = redact_plugin_secret_values(payload, redaction_value=sentinel) + if redacted == payload: + return + secret_name = clean_name_for_keyvault(payload.get("name", "")).lower() + existing = cosmos_personal_actions_container.query_items( + query="SELECT * FROM c WHERE c.user_id = @user_id", + parameters=[{"name": "@user_id", "value": user_id}], + partition_key=user_id, + ) + for action in existing: + name = action.get("name") + if ( + action.get("id") != payload.get("id") + and isinstance(name, str) + and clean_name_for_keyvault(name).lower() == secret_name + ): + raise LegacyActionSecretConflictError() + if legacy_sources is None: + document = _read_legacy_settings_document(user_id) + legacy_sources = legacy_action_snapshots(user_id, _document_plugins(document)) + for other in legacy_sources: + if snapshot is not None and other.locator == snapshot.locator: + continue + if ( + isinstance(other.record, dict) + and isinstance(other.record.get("name"), str) + and clean_name_for_keyvault(other.record["name"]).lower() == secret_name + ): + raise LegacyActionSecretConflictError() + + +def _prepare_personal_action_configuration(user_id, incoming): + payload = prepare_scoped_action(incoming, "personal", user_id) + payload.setdefault("displayName", payload.get("name", "")) + payload.setdefault("description", "") + payload.setdefault("endpoint", "") + payload.setdefault("auth", {"type": "NoAuth"}) + payload.setdefault("metadata", {}) + payload.setdefault("additionalFields", {}) + ensure_action_type_access("governance_user_actions", user_id, payload["type"], "personal") + validate_action_configuration(payload) + if payload["type"] == "mcp": + validate_scoped_mcp_action(payload, user_id, user_settings_service.get_settings()) + validate_action_identity_reference(payload, WORKSPACE_IDENTITY_SCOPE_PERSONAL, user_id) + return payload + + +def _legacy_destination_payload(user_id, snapshot, replacement=None, *, legacy_sources=None): + original = snapshot.record + if not isinstance(original, dict): + raise ValueError("Legacy action configuration is invalid.") + incoming = deepcopy(original if replacement is None else replacement) + if not isinstance(incoming, dict): + raise ValueError("Action configuration must be an object.") + source_id = original.get("id") + if source_id and (not isinstance(source_id, str) or source_id.startswith(LEGACY_ACTION_PREFIX)): + raise LegacyActionConflictError() + incoming["id"] = source_id or str(uuid.uuid5(uuid.NAMESPACE_URL, f"{user_id}:{snapshot.locator}")) + if is_retired_mcp_stdio(original) and resolve_action_type(incoming) != "mcp": + raise ValueError("Reconfigure this action with a supported remote MCP server.") + payload = _prepare_personal_action_configuration(user_id, incoming) + _ensure_personal_secret_name_available( + user_id, payload, snapshot, legacy_sources=legacy_sources + ) + return payload + + +def _verified_legacy_destination(user_id, snapshot, payload): + existing = get_personal_action_record(user_id, payload["id"]) + if existing is None: + return None + receipt = existing.get("_legacy_migration") + if not isinstance(receipt, dict) or ( + receipt.get("source_locator") != snapshot.locator + or receipt.get("configuration_digest") != _action_configuration_digest(payload) + or receipt.get("destination_digest") != _stored_action_digest(existing) + ): + raise LegacyActionConflictError() + return existing + + +def _store_legacy_replacement(user_id, snapshot, payload): + current = _get_legacy_snapshot(user_id, snapshot.locator) + if current.record != snapshot.record: + raise LegacyActionConflictError() + existing = _verified_legacy_destination(user_id, snapshot, payload) + if existing is None: + _save_personal_action(user_id, payload, migration_snapshot=snapshot) + existing = _verified_legacy_destination(user_id, snapshot, payload) + if existing is None: + raise LegacyActionSourceUpdateError() + _remove_legacy_snapshot(user_id, snapshot) + return existing + + +def reconfigure_legacy_personal_action(user_id, locator, replacement): + """Store a validated replacement before conditionally removing its exact source.""" + snapshot = _get_legacy_snapshot(user_id, locator) + payload = _legacy_destination_payload(user_id, snapshot, replacement=replacement) + stored = _store_legacy_replacement(user_id, snapshot, payload) + return _clean_action(stored, user_id, SecretReturnType.TRIGGER) + + +def _migration_outcome(snapshot, code, retryable=False, action_id=None): + record = snapshot.record if isinstance(snapshot.record, dict) else {} + outcome = { + "id": snapshot.locator, + "name": record.get("name") if isinstance(record.get("name"), str) else "", + "code": code, + "retryable": retryable, + } + if action_id: + outcome["action_id"] = action_id + return outcome + + +def _legacy_identity_counts(snapshots): + return Counter( + snapshot.record["id"] for snapshot in snapshots + if isinstance(snapshot.record, dict) and isinstance(snapshot.record.get("id"), str) + and snapshot.record["id"] + ) + + +def _prepare_legacy_migration(user_id, snapshot, identity_counts): + if is_retired_mcp_stdio(snapshot.record): + return None, "mcp_stdio_removed" + source_id = snapshot.record.get("id") if isinstance(snapshot.record, dict) else None + if snapshot.duplicate_count > 1 or (isinstance(source_id, str) and identity_counts[source_id] > 1): + return None, "legacy_identity_conflict" + try: + payload = _legacy_destination_payload(user_id, snapshot) + _verified_legacy_destination(user_id, snapshot, payload) + return payload, None + except LegacyActionConflictError as exc: + return None, exc.code + except PermissionError: + return None, "action_governance_denied" + except ValueError: + return None, "invalid_action_configuration" + + +def get_action_migration_status(user_id): + """Describe actionable work separately from records requiring manual changes.""" + document = _read_legacy_settings_document(user_id) + snapshots = legacy_action_snapshots(user_id, _document_plugins(document)) + identity_counts = _legacy_identity_counts(snapshots) + status = { + "pending_count": 0, "retained_count": 0, "retired_count": 0, + "failed_count": 0, "retained": [], "failed": [], "actions": [], + } + for snapshot in snapshots: + try: + _payload, reason = _prepare_legacy_migration(user_id, snapshot, identity_counts) + if reason: + status["retained"].append(_migration_outcome(snapshot, reason)) + if reason == "mcp_stdio_removed": + status["retired_count"] += 1 else: - # Clean up legacy data without migration (already migrated) - settings_to_update = user_settings.get('settings', {}) - settings_to_update['plugins'] = [] # Set to empty array instead of removing - update_user_settings(user_id, settings_to_update) - debug_print(f"Cleaned up legacy plugin data for user {user_id} (already migrated)") - return 0 - - return 0 - - except Exception as e: - debug_print(f"Error ensuring action migration complete for user {user_id}: {e}") - return 0 + status["pending_count"] += 1 + try: + _ensure_legacy_management_access(user_id, snapshot) + except (PermissionError, ValueError): + continue + status["actions"].append(legacy_action_management_view(snapshot)) + except Exception as exc: + status["failed"].append(_migration_outcome(snapshot, "migration_check_failed", retryable=True)) + log_event("[PLUGINS] Legacy action migration inspection failed", level=logging.WARNING, + extra={"user_id": user_id, "error_type": type(exc).__name__}) + status["retained_count"] = len(status["retained"]) + status["failed_count"] = len(status["failed"]) + status["complete"] = status["pending_count"] == 0 and status["failed_count"] == 0 + status["total_count"] = len(snapshots) + return status + + +def ensure_migration_complete(user_id): + """Attempt only verified record-level migration; never clean up by counts.""" + return migrate_actions_from_user_settings(user_id) + def migrate_actions_from_user_settings(user_id): - """ - Migrate actions/plugins from user settings to personal_actions container. - - Args: - user_id (str): The user's unique identifier - - Returns: - int: Number of actions migrated - """ + """Migrate valid records independently and report retained/failed sources safely.""" + result = { + "migrated_count": 0, "retained_count": 0, "failed_count": 0, + "migrated": [], "retained": [], "failed": [], "complete": False, + } try: - user_settings = get_user_settings(user_id) - plugins = user_settings.get('settings', {}).get('plugins', []) - - # Get existing personal actions to avoid duplicates - existing_personal_actions = get_personal_actions(user_id) - existing_action_names = {action['name'] for action in existing_personal_actions} - - migrated_count = 0 - for plugin in plugins: + document = _read_legacy_settings_document(user_id) + snapshots = legacy_action_snapshots(user_id, _document_plugins(document)) + identity_counts = _legacy_identity_counts(snapshots) + for snapshot in snapshots: try: - # Skip if plugin already exists in personal container - if plugin.get('name') in existing_action_names: - debug_print(f"Skipping migration of plugin '{plugin.get('name')}' - already exists") + payload, reason = _prepare_legacy_migration(user_id, snapshot, identity_counts) + if reason: + result["retained"].append(_migration_outcome(snapshot, reason)) continue - # Ensure plugin has an ID (generate GUID if missing) - if 'id' not in plugin or not plugin['id']: - plugin['id'] = str(uuid.uuid4()) - # Store secrets in Key Vault before migration - plugin = keyvault_plugin_save_helper(plugin, scope_value=user_id, scope="user") - save_personal_action(user_id, plugin, enforce_governance=False) - migrated_count += 1 - except Exception as e: - debug_print(f"Error migrating plugin {plugin.get('name', 'unknown')} for user {user_id}: {e}") - - # Always remove plugins from user settings after processing (even if no new ones migrated) - settings_to_update = user_settings.get('settings', {}) - settings_to_update['plugins'] = [] # Set to empty array instead of removing - update_user_settings(user_id, settings_to_update) - - debug_print(f"Migrated {migrated_count} new actions for user {user_id}, cleaned up legacy data") - return migrated_count - - except Exception as e: - debug_print(f"Error during action migration for user {user_id}: {e}") - return 0 + stored = _store_legacy_replacement(user_id, snapshot, payload) + result["migrated"].append(_migration_outcome(snapshot, "migrated", action_id=stored["id"])) + except LegacyActionConflictError as exc: + result["retained"].append(_migration_outcome(snapshot, exc.code)) + except (PermissionError, ValueError): + result["retained"].append(_migration_outcome(snapshot, "action_validation_failed")) + except Exception as exc: + code = ( + "legacy_source_update_failed" + if isinstance(exc, LegacyActionSourceUpdateError) + else "action_migration_failed" + ) + result["failed"].append(_migration_outcome(snapshot, code, retryable=True)) + log_event("[PLUGINS] Legacy action migration failed", level=logging.WARNING, + extra={"user_id": user_id, "error_type": type(exc).__name__}) + remaining = get_action_migration_status(user_id) + result["complete"] = remaining["complete"] and not result["failed"] + result["pending_count"] = remaining["pending_count"] + except PermissionError: + raise + except Exception as exc: + result["failed"].append({ + "code": "legacy_source_read_failed", "retryable": True, + }) + log_event("[PLUGINS] Legacy action source inspection failed", level=logging.ERROR, + extra={"user_id": user_id, "error_type": type(exc).__name__}) + for category in ("migrated", "retained", "failed"): + result[f"{category}_count"] = len(result[category]) + return result def get_actions_by_names(user_id, action_names, return_type=SecretReturnType.TRIGGER): """ @@ -371,18 +887,13 @@ def get_actions_by_names(user_id, action_names, return_type=SecretReturnType.TRI partition_key=user_id )) - # Remove Cosmos metadata - cleaned_actions = [] - for action in actions: - cleaned_action = {k: v for k, v in action.items() if not k.startswith('_')} - cleaned_action = keyvault_plugin_get_helper(cleaned_action, scope_value=user_id, scope="user", return_type=return_type) - cleaned_actions.append(cleaned_action) - - return cleaned_actions - - except Exception as e: - debug_print(f"Error fetching actions by names for user {user_id}: {e}") + except exceptions.CosmosResourceNotFoundError: return [] + except Exception as exc: + log_event("[PLUGINS] Personal action name lookup failed", level=logging.ERROR, + extra={"user_id": user_id, "error_type": type(exc).__name__}) + raise + return _clean_actions(actions, user_id, return_type) def get_actions_by_type(user_id, action_type, return_type=SecretReturnType.TRIGGER): """ @@ -396,11 +907,15 @@ def get_actions_by_type(user_id, action_type, return_type=SecretReturnType.TRIGG list: List of action dictionaries """ try: + effective_type = resolve_action_type({"type": action_type}) query = "SELECT * FROM c WHERE c.user_id = @user_id AND c.type = @type" parameters = [ {"name": "@user_id", "value": user_id}, - {"name": "@type", "value": action_type} + {"name": "@type", "value": effective_type} ] + if effective_type == "mcp": + query = "SELECT * FROM c WHERE c.user_id = @user_id" + parameters = [{"name": "@user_id", "value": user_id}] actions = list(cosmos_personal_actions_container.query_items( query=query, @@ -408,15 +923,14 @@ def get_actions_by_type(user_id, action_type, return_type=SecretReturnType.TRIGG partition_key=user_id )) - # Remove Cosmos metadata - cleaned_actions = [] - for action in actions: - cleaned_action = {k: v for k, v in action.items() if not k.startswith('_')} - cleaned_action = keyvault_plugin_get_helper(cleaned_action, scope_value=user_id, scope="user", return_type=return_type) - cleaned_actions.append(cleaned_action) - - return cleaned_actions - - except Exception as e: - debug_print(f"Error fetching actions by type {action_type} for user {user_id}: {e}") + except exceptions.CosmosResourceNotFoundError: return [] + except Exception as exc: + log_event("[PLUGINS] Personal action type lookup failed", level=logging.ERROR, + extra={"user_id": user_id, "error_type": type(exc).__name__}) + raise + matching_actions = ( + action for action in actions + if effective_type != "mcp" or resolve_action_type(action) == effective_type + ) + return _clean_actions(matching_actions, user_id, return_type) diff --git a/application/single_app/json_schema_validation.py b/application/single_app/json_schema_validation.py index 9bc76e24c..b3e975ed0 100644 --- a/application/single_app/json_schema_validation.py +++ b/application/single_app/json_schema_validation.py @@ -6,6 +6,13 @@ from functools import lru_cache from jsonschema import validate, ValidationError, Draft7Validator, Draft6Validator, RefResolver +from functions_action_manifest import ( + MCP_STDIO_REMOVED_MESSAGE, + McpConfigurationError, + is_retired_mcp_stdio, + resolve_action_type, +) +from functions_mcp_operations import normalize_mcp_transport, validate_mcp_endpoint_for_transport from functions_blob_storage_operations import BLOB_STORAGE_PLUGIN_TYPE, derive_blob_endpoint_from_connection_string from functions_chart_operations import CHART_DEFAULT_ENDPOINT from functions_databricks_operations import DATABRICKS_LEGACY_TABLE_PLUGIN_TYPE, DATABRICKS_PLUGIN_TYPE @@ -39,6 +46,8 @@ 'modified_at', 'modified_by', 'scope', + 'scope_id', + 'execution_status', 'updated_at', 'user_id', } @@ -115,7 +124,7 @@ def validate_plugin_auth_type_allowed(plugin): if not declared_auth_type: return None - plugin_type = str(plugin.get('type') or '').strip().lower() + plugin_type = resolve_action_type(plugin).lower() if plugin_type == DATABRICKS_LEGACY_TABLE_PLUGIN_TYPE: plugin_type = DATABRICKS_PLUGIN_TYPE @@ -130,7 +139,9 @@ def validate_plugin_auth_type_allowed(plugin): def apply_plugin_validation_defaults(plugin): plugin_copy = plugin.copy() if isinstance(plugin, dict) else {} - plugin_type = str(plugin_copy.get('type', '') or '').strip().lower() + plugin_type = resolve_action_type(plugin_copy).lower() + if plugin_type: + plugin_copy['type'] = plugin_type if plugin_type == DATABRICKS_LEGACY_TABLE_PLUGIN_TYPE: plugin_type = DATABRICKS_PLUGIN_TYPE plugin_copy['type'] = DATABRICKS_PLUGIN_TYPE @@ -154,8 +165,23 @@ def apply_plugin_validation_defaults(plugin): def validate_plugin(plugin): schema = load_schema('plugin.schema.json') - plugin_copy = apply_plugin_validation_defaults(plugin) + if is_retired_mcp_stdio(plugin): + return MCP_STDIO_REMOVED_MESSAGE + try: + plugin_copy = apply_plugin_validation_defaults(plugin) + except ValueError: + return 'Invalid action type.' plugin_type = str(plugin_copy.get('type', '') or '').strip().lower() + if plugin_type == 'mcp': + fields = plugin_copy.get('additionalFields') + fields = fields if isinstance(fields, dict) else {} + try: + transport = normalize_mcp_transport(fields.get('transport')) + endpoint_errors = validate_mcp_endpoint_for_transport(plugin_copy.get('endpoint'), transport) + except McpConfigurationError as exc: + return exc.public_message + if endpoint_errors: + return '; '.join(endpoint_errors) # First run schema validation if schema.get("$ref") and schema.get("definitions"): @@ -165,6 +191,10 @@ def validate_plugin(plugin): errors = sorted(validator.iter_errors(plugin_copy), key=lambda e: e.path) if errors: return '; '.join([f"{plugin.get('name', '')}: {e.message}" for e in errors]) + + auth_error = validate_plugin_auth_type_allowed(plugin_copy) + if auth_error: + return auth_error # Additional business logic validation # For non-SQL plugins, endpoint must not be empty diff --git a/application/single_app/mcp_presets/README.md b/application/single_app/mcp_presets/README.md index aa5853b45..fc21609c5 100644 --- a/application/single_app/mcp_presets/README.md +++ b/application/single_app/mcp_presets/README.md @@ -1,6 +1,10 @@ # MCP Server Presets -Last updated for SimpleChat version: **0.250.067** +Last updated for SimpleChat version: **0.261.029** + +Remote-only preset compatibility implemented in version: **0.261.029**. + +Related configuration update: `application\single_app\config.py` advances `VERSION` from `0.261.028` to `0.261.029`. ## Purpose @@ -26,7 +30,7 @@ The user or preconfiguration still supplies the actual endpoint. Preset definitions may include: -* Default transport: `streamable_http`, `sse`, `websocket`, or `stdio`. +* Default transport: `streamable_http`, `sse`, or `websocket`. * Default auth method: `none`, `bearer`, `api_key`, `basic`, or `identity`. * Timeout and retry defaults. * Prompt/tool loading defaults. @@ -36,6 +40,8 @@ Preset definitions may include: * Suggested header names and descriptions. * Compatibility warnings. +Stdio and local process command, argument, and environment configuration are unsupported for every role and scope, including Admin/global. Presets cannot opt into local process execution or override remote destination governance. + ## What Does Not Belong Here Do not put these values in presets: @@ -58,6 +64,14 @@ Those belong in MCP preconfigurations or in implementation-specific preconfigura 5. Never include credentials or secret-bearing fields. 6. Use warnings for compatibility caveats, not security policy decisions. +## Upgrading Older Presets + +The loader keeps otherwise valid remote custom presets compatible by normalizing a copy before validation. It removes inert `stdioAllowed` and retired process-only fields, filters stdio out of `allowedTransports`, and preserves a valid remote default. Authentication, header guidance, timeouts, and provider-specific remote settings remain intact; similarly named tool arguments or provider fields are not retired MCP process settings. + +A stdio-default or stdio-only preset is unavailable with a diagnostic, not converted to HTTP. Other valid presets still load. Revise the source definition explicitly for an approved remote endpoint's transport, or remove the obsolete definition. + +New preset definitions should omit retired fields. Loading a preset does not migrate existing stdio actions: those remain visible but non-executable until explicitly reconfigured with a supported remote transport and valid endpoint, or explicitly deleted. + ## Implementation-Specific Settings The base preset schema stays provider-neutral. If a preset needs compatibility metadata that is specific to a server family, add: diff --git a/application/single_app/mcp_presets/definitions/generic.json b/application/single_app/mcp_presets/definitions/generic.json index 585563128..52dfe2180 100644 --- a/application/single_app/mcp_presets/definitions/generic.json +++ b/application/single_app/mcp_presets/definitions/generic.json @@ -25,10 +25,9 @@ "websocketEndpointPlaceholder": "wss://example.com/mcp" }, "constraints": { - "allowedTransports": ["streamable_http", "sse", "websocket", "stdio"], + "allowedTransports": ["streamable_http", "sse", "websocket"], "allowedAuthMethods": ["none", "bearer", "api_key", "basic", "identity"], - "customHeadersAllowed": true, - "stdioAllowed": true + "customHeadersAllowed": true }, "implementation": { "id": "generic", diff --git a/application/single_app/mcp_presets/definitions/splunk.json b/application/single_app/mcp_presets/definitions/splunk.json index 29c26a35b..8c83a2f56 100644 --- a/application/single_app/mcp_presets/definitions/splunk.json +++ b/application/single_app/mcp_presets/definitions/splunk.json @@ -27,8 +27,7 @@ "constraints": { "allowedTransports": ["streamable_http"], "allowedAuthMethods": ["bearer"], - "customHeadersAllowed": true, - "stdioAllowed": false + "customHeadersAllowed": true }, "implementation": { "id": "splunk", diff --git a/application/single_app/mcp_presets/mcp_server_preset.schema.json b/application/single_app/mcp_presets/mcp_server_preset.schema.json index f14e62ad3..debf97e04 100644 --- a/application/single_app/mcp_presets/mcp_server_preset.schema.json +++ b/application/single_app/mcp_presets/mcp_server_preset.schema.json @@ -66,7 +66,7 @@ "properties": { "transport": { "type": "string", - "enum": ["streamable_http", "sse", "websocket", "stdio"] + "enum": ["streamable_http", "sse", "websocket"] }, "auth_method": { "type": "string", @@ -159,8 +159,7 @@ "required": [ "allowedTransports", "allowedAuthMethods", - "customHeadersAllowed", - "stdioAllowed" + "customHeadersAllowed" ], "properties": { "allowedTransports": { @@ -169,7 +168,7 @@ "uniqueItems": true, "items": { "type": "string", - "enum": ["streamable_http", "sse", "websocket", "stdio"] + "enum": ["streamable_http", "sse", "websocket"] } }, "allowedAuthMethods": { @@ -183,9 +182,6 @@ }, "customHeadersAllowed": { "type": "boolean" - }, - "stdioAllowed": { - "type": "boolean" } } }, diff --git a/application/single_app/plugin_validation_endpoint.py b/application/single_app/plugin_validation_endpoint.py index 6cd868d60..1a039c0b0 100644 --- a/application/single_app/plugin_validation_endpoint.py +++ b/application/single_app/plugin_validation_endpoint.py @@ -9,6 +9,7 @@ from flask import Blueprint, current_app, jsonify, request from functions_appinsights import log_event +from functions_action_manifest import resolve_action_type from functions_authentication import admin_required, admin_required_blueprint, login_required, user_required, user_required_blueprint from functions_global_actions import save_global_action from functions_settings import get_settings, update_settings @@ -24,6 +25,25 @@ plugin_validation_admin_bp.before_request(admin_required_blueprint()) +def _find_plugin_class(plugin_type, discovered_plugins): + """Keep MCP class selection consistent with manifest validation.""" + def normalize(value): + return value.replace('_', '').replace('-', '').replace('plugin', '').lower() + + if not plugin_type: + return None + normalized_type = normalize(plugin_type) + for class_name, plugin_class in discovered_plugins.items(): + if resolve_action_type({'type': class_name}) == 'mcp': + if plugin_type == 'mcp': + return plugin_class + continue + normalized_class = normalize(class_name) + if normalized_type == normalized_class or normalized_type in normalized_class: + return plugin_class + return None + + def _validate_plugin_manifest_request(): manifest = apply_plugin_validation_defaults(request.get_json(silent=True) or {}) if not manifest: @@ -71,9 +91,11 @@ def validate_plugin_manifest(): """ try: return _validate_plugin_manifest_request() + except ValueError: + return jsonify({'error': 'Invalid action configuration.'}), 400 except Exception as e: log_event(f"[PLUGIN_VALIDATION] Error validating manifest: {str(e)}", level=logging.ERROR) - return jsonify({'error': f'Validation failed: {str(e)}'}), 500 + return jsonify({'error': 'Unable to validate the action.'}), 500 @plugin_validation_admin_bp.route('/api/admin/plugins/validate', methods=['POST']) @@ -88,9 +110,11 @@ def validate_plugin_manifest_admin(): """ try: return _validate_plugin_manifest_request() + except ValueError: + return jsonify({'error': 'Invalid action configuration.'}), 400 except Exception as e: log_event(f"[PLUGIN_VALIDATION] Error validating manifest: {str(e)}", level=logging.ERROR) - return jsonify({'error': f'Validation failed: {str(e)}'}), 500 + return jsonify({'error': 'Unable to validate the action.'}), 500 @plugin_validation_admin_bp.route('/api/admin/plugins/test-instantiation', methods=['POST']) @@ -109,24 +133,13 @@ def test_plugin_instantiation(): if not manifest: return jsonify({'error': 'No manifest provided'}), 400 - plugin_type = manifest.get('type', '') + plugin_type = resolve_action_type(manifest) plugin_name = manifest.get('name', 'unnamed') # Discover available plugins discovered_plugins = discover_plugins() - # Find matching plugin class - def normalize(s): - return s.replace('_', '').replace('-', '').replace('plugin', '').lower() if s else '' - - normalized_type = normalize(plugin_type) - matched_class = None - - for class_name, cls in discovered_plugins.items(): - normalized_class = normalize(class_name) - if normalized_type == normalized_class or normalized_type in normalized_class: - matched_class = cls - break + matched_class = _find_plugin_class(plugin_type, discovered_plugins) if not matched_class: return jsonify({ @@ -193,20 +206,10 @@ def check_plugin_health(plugin_name): return jsonify({'error': f'Plugin {plugin_name} not found'}), 404 # Try to instantiate and check health - plugin_type = plugin_manifest.get('type', '') + plugin_type = resolve_action_type(plugin_manifest) discovered_plugins = discover_plugins() - def normalize(s): - return s.replace('_', '').replace('-', '').replace('plugin', '').lower() if s else '' - - normalized_type = normalize(plugin_type) - matched_class = None - - for class_name, cls in discovered_plugins.items(): - normalized_class = normalize(class_name) - if normalized_type == normalized_class or normalized_type in normalized_class: - matched_class = cls - break + matched_class = _find_plugin_class(plugin_type, discovered_plugins) if not matched_class: return jsonify({ @@ -271,20 +274,13 @@ def repair_plugin(plugin_name): return jsonify({'error': f'Plugin {plugin_name} not found'}), 404 # Try to instantiate the plugin - plugin_type = plugin_manifest.get('type', '') + plugin_type = resolve_action_type(plugin_manifest) discovered_plugins = discover_plugins() - - def normalize(s): - return s.replace('_', '').replace('-', '').replace('plugin', '').lower() if s else '' - - normalized_type = normalize(plugin_type) - matched_class = None - - for class_name, cls in discovered_plugins.items(): - normalized_class = normalize(class_name) - if normalized_type == normalized_class or normalized_type in normalized_class: - matched_class = cls - break + if plugin_type == 'mcp': + valid, errors = PluginHealthChecker.validate_plugin_manifest(plugin_manifest, plugin_type) + if not valid: + return jsonify({'success': False, 'error': 'MCP configuration requires reconfiguration.', 'errors': errors}), 400 + matched_class = _find_plugin_class(plugin_type, discovered_plugins) if not matched_class: return jsonify({ diff --git a/application/single_app/route_backend_plugins.py b/application/single_app/route_backend_plugins.py index d53f5e2f7..66b6a8f37 100644 --- a/application/single_app/route_backend_plugins.py +++ b/application/single_app/route_backend_plugins.py @@ -28,6 +28,16 @@ from functions_settings import get_settings, is_tabular_processing_enabled, update_settings from functions_authentication import * from functions_appinsights import log_event +from functions_action_manifest import ( + McpActionOrigin, + McpConfigurationError, + McpStdioRemovedError, + ScopedActionManifest, + bind_action_origin, + get_action_origin, + is_retired_mcp_stdio, + resolve_action_type, +) from swagger_wrapper import swagger_route, get_auth_security import logging import os @@ -124,7 +134,6 @@ from functions_mcp_operations import ( MCP_CUSTOM_HEADERS_FIELD, MCP_PLUGIN_TYPE, - MCP_STDIO_ENDPOINT, McpRuntimeError, get_mcp_error_http_status, normalize_mcp_additional_fields, @@ -161,6 +170,12 @@ is_action_type_access_allowed, upsert_item_policy, ) +from functions_legacy_action_management import ( + LEGACY_ACTION_PREFIX, + LegacyActionConflictError, + LegacyActionSourceUpdateError, + is_unchanged_retired_action, +) ACTION_VALIDATION_ERROR_MESSAGE = "Invalid action configuration." @@ -175,9 +190,15 @@ def _apply_plugin_runtime_defaults(plugin_payload): if not isinstance(plugin_payload, dict): - return plugin_payload + raise McpConfigurationError("Action configuration must be an object.") - plugin_type = plugin_payload.get('type', '') + try: + plugin_type = resolve_action_type(plugin_payload) + except ValueError as exc: + raise McpConfigurationError("Invalid action type.") from exc + plugin_payload['type'] = plugin_type + if is_retired_mcp_stdio(plugin_payload): + raise McpStdioRemovedError() if plugin_type in ['sql_schema', 'sql_query']: if not str(plugin_payload.get('endpoint') or '').strip(): plugin_payload['endpoint'] = f'sql://{plugin_type}' @@ -202,9 +223,6 @@ def _apply_plugin_runtime_defaults(plugin_payload): additional_fields = normalize_mcp_additional_fields(additional_fields) plugin_payload['additionalFields'] = additional_fields - if additional_fields.get('transport') == 'stdio' and not str(plugin_payload.get('endpoint') or '').strip(): - plugin_payload['endpoint'] = MCP_STDIO_ENDPOINT - auth = plugin_payload.get('auth') if isinstance(plugin_payload.get('auth'), dict) else {} auth_method = additional_fields.get('auth_method') or 'none' if auth_method == 'none': @@ -622,6 +640,28 @@ def get_plugin_types(allowed_type_filter=None): bpap.before_request(login_required_blueprint()) +@bpap.errorhandler(McpConfigurationError) +def _handle_mcp_configuration_error(exc): + log_event( + "[PLUGIN_VALIDATION] MCP configuration rejected", + extra={"error_type": exc.code}, + level=logging.WARNING, + ) + return jsonify({'success': False, 'error': exc.public_message, 'error_type': exc.code}), 400 + + +@bpap.errorhandler(LegacyActionConflictError) +@bpap.errorhandler(LegacyActionSourceUpdateError) +def _handle_legacy_action_error(exc): + log_event( + "[PLUGIN_VALIDATION] Legacy action operation could not be completed", + extra={"error_type": exc.code}, + level=logging.WARNING, + ) + status = 409 if isinstance(exc, LegacyActionConflictError) else 500 + return jsonify({'success': False, 'error': exc.public_message, 'error_type': exc.code}), status + + def _redact_plugin_for_logging(plugin): """Return a plugin manifest with secret-bearing values redacted for logging.""" if not isinstance(plugin, dict): @@ -634,36 +674,50 @@ def _resolve_plugin_secret_context(plugin_manifest, fallback_scope_value, fallba if not isinstance(plugin_manifest, dict): return fallback_scope_value, fallback_scope - plugin_scope = str(plugin_manifest.get("scope") or "").strip().lower() - if plugin_scope == "group" or plugin_manifest.get("is_group"): - return plugin_manifest.get("group_id"), "group" - if plugin_scope == "global" or plugin_manifest.get("is_global"): - return plugin_manifest.get("id") or fallback_scope_value, "global" - if plugin_scope == "user" or plugin_manifest.get("user_id"): - return plugin_manifest.get("user_id") or fallback_scope_value, "user" - return fallback_scope_value, fallback_scope + origin = get_action_origin(plugin_manifest) + if origin is None: + raise PermissionError("Action lookup provenance is required.") + if origin.scope_type == "global": + return origin.action_id, "global" + return origin.scope_id, "user" if origin.scope_type == "personal" else "group" def _resolve_action_identity_context(data, existing_plugin, user_id): """Resolve the authoritative identity scope for an action test or save request.""" - plugin_scope = "" + aliases = { + "user": "personal", "personal": "personal", "workspace": "personal", + "group": "group", "group_action": "group", + "global": "global", "admin": "global", "global_action": "global", + } + requested_value = str((data or {}).get("action_scope") or "personal").strip().lower() + if requested_value not in aliases: + raise ValueError("Action scope is invalid.") + requested_scope = aliases[requested_value] if isinstance(existing_plugin, dict): - plugin_scope = str(existing_plugin.get("scope") or "").strip().lower() - if plugin_scope == "group" or existing_plugin.get("is_group"): + origin = get_action_origin(existing_plugin) + if origin is None: + raise PermissionError("Action lookup provenance is required.") + if (data or {}).get("action_scope") and requested_scope != origin.scope_type: + raise PermissionError("Action scope does not match the existing action.") + if origin.scope_type == "group": active_group = require_active_group(user_id) + if active_group != origin.scope_id: + raise PermissionError("Action does not belong to the selected group.") assert_group_role( user_id, active_group, allowed_roles=("Owner", "Admin", "DocumentManager", "User"), ) return WORKSPACE_IDENTITY_SCOPE_GROUP, active_group - if plugin_scope == "global" or existing_plugin.get("is_global"): + if origin.scope_type == "global": if "Admin" not in session.get("user", {}).get("roles", []): raise PermissionError("Admin role required for global action identities") return WORKSPACE_IDENTITY_SCOPE_GLOBAL, WORKSPACE_IDENTITY_SCOPE_GLOBAL + if origin.scope_id != user_id: + raise PermissionError("Action does not belong to the current user.") + return WORKSPACE_IDENTITY_SCOPE_PERSONAL, user_id - requested_scope = str((data or {}).get("action_scope") or "personal").strip().lower() - if requested_scope in {"group", "group_action"}: + if requested_scope == "group": active_group = require_active_group(user_id) assert_group_role( user_id, @@ -671,7 +725,7 @@ def _resolve_action_identity_context(data, existing_plugin, user_id): allowed_roles=("Owner", "Admin", "DocumentManager", "User"), ) return WORKSPACE_IDENTITY_SCOPE_GROUP, active_group - if requested_scope in {"global", "admin"}: + if requested_scope == "global": if "Admin" not in session.get("user", {}).get("roles", []): raise PermissionError("Admin role required for global action identities") return WORKSPACE_IDENTITY_SCOPE_GLOBAL, WORKSPACE_IDENTITY_SCOPE_GLOBAL @@ -683,22 +737,9 @@ def _validate_action_identity_for_scope(plugin_manifest, scope_type, scope_id): validate_action_identity_reference(plugin_manifest, scope_type, scope_id) -def _reject_non_admin_mcp_stdio(plugin_manifest, scope_label="personal"): - """Block stdio MCP actions outside admin/global action management.""" - if not isinstance(plugin_manifest, dict): - return None - if plugin_manifest.get('type') != MCP_PLUGIN_TYPE: - return None - - additional_fields = normalize_mcp_additional_fields(plugin_manifest.get('additionalFields', {})) - if additional_fields.get('transport') == 'stdio': - return f"MCP stdio transport is only available for admin-managed global actions, not {scope_label} actions." - return None - - def _enforce_mcp_destination_policy(plugin_manifest, scope_type, scope_id, operation, user_id=None, mcp_operation_id=""): """Enforce outbound MCP destination policy for save, discovery, and runtime-adjacent routes.""" - if not isinstance(plugin_manifest, dict) or plugin_manifest.get('type') != MCP_PLUGIN_TYPE: + if not isinstance(plugin_manifest, dict) or resolve_action_type(plugin_manifest) != MCP_PLUGIN_TYPE: return None normalized_user_id = str(user_id or get_current_user_id() or '').strip() @@ -884,12 +925,22 @@ def _resolve_secret_value_for_sql_test(value, field_name, scope_value=None, scop def _load_existing_plugin_for_test(plugin_context, user_id): """Load an existing plugin manifest with Key Vault reference names for edit-time plugin tests.""" - if not isinstance(plugin_context, dict): + if plugin_context is None: return None - - plugin_scope = (plugin_context.get('scope') or 'user').lower() + if not isinstance(plugin_context, dict): + raise ValueError("Action lookup context must be an object.") + + plugin_scope = str(plugin_context.get('scope') or 'user').strip().lower() + if plugin_scope not in {'user', 'personal', 'group', 'global'}: + raise ValueError("Action lookup scope is invalid.") + for field in ('id', 'name'): + value = plugin_context.get(field) + if value is not None and not isinstance(value, str): + raise ValueError("Action lookup identifier must be a string.") plugin_identifier = plugin_context.get('id') or plugin_context.get('name') if not plugin_identifier: + if 'id' in plugin_context or 'name' in plugin_context: + raise ValueError("The requested action must have an identifier.") return None if plugin_scope == 'group': @@ -899,16 +950,40 @@ def _load_existing_plugin_for_test(plugin_context, user_id): active_group, allowed_roles=("Owner", "Admin", "DocumentManager", "User"), ) - return get_group_action(active_group, plugin_identifier, return_type=SecretReturnType.NAME) + plugin = get_group_action(active_group, plugin_identifier, return_type=SecretReturnType.NAME) + scope_type, scope_id = "group", active_group - if plugin_scope == 'global': + elif plugin_scope == 'global': # Global actions are admin-managed. Gate here so every test route inherits the check, # including routes that do not otherwise resolve an action identity scope. if "Admin" not in session.get("user", {}).get("roles", []): raise PermissionError("Admin role required to load a global action for testing.") - return get_global_action(plugin_identifier, return_type=SecretReturnType.NAME) + plugin = get_global_action(plugin_identifier, return_type=SecretReturnType.NAME) + scope_type, scope_id = "global", "global" + else: + if plugin_identifier.startswith(LEGACY_ACTION_PREFIX): + plugin = get_legacy_personal_action_record(user_id, plugin_identifier) + else: + plugin = get_personal_action(user_id, plugin_identifier, return_type=SecretReturnType.NAME) + scope_type, scope_id = "personal", user_id + if plugin is None: + raise LookupError("The requested action was not found.") + return bind_action_origin(plugin, scope_type, scope_id) + - return get_personal_action(user_id, plugin_identifier, return_type=SecretReturnType.NAME) +def _action_test_origin(existing_plugin, scope_type, scope_id): + if existing_plugin is not None: + origin = get_action_origin(existing_plugin) + if origin is None or (origin.scope_type, origin.scope_id) != (scope_type, scope_id): + raise PermissionError("Action lookup provenance does not match the authorized scope.") + return origin + return McpActionOrigin(scope_type, scope_id) + + +def _action_origin_secret_context(origin): + if origin.scope_type == "global": + return origin.action_id, "global" + return origin.scope_id, "user" if origin.scope_type == "personal" else "group" def _load_existing_plugin_for_sql_test(plugin_context, user_id): @@ -927,6 +1002,7 @@ def get_user_plugins(): # Get plugins from the new personal_actions container plugins = get_governed_personal_actions(user_id) + plugins.extend(list_legacy_personal_actions(user_id)) # Always mark user plugins as is_global: False for plugin in plugins: @@ -968,7 +1044,9 @@ def get_user_plugins(): @enabled_required("allow_user_plugins") def set_user_plugins(): user_id = get_current_user_id() - plugins = request.json if isinstance(request.json, list) else [] + plugins = request.get_json(silent=True) + if not isinstance(plugins, list): + raise McpConfigurationError("Actions must be provided as a list.") # Get global plugin names (case-insensitive) global_plugins = get_global_actions() @@ -978,14 +1056,49 @@ def set_user_plugins(): current_actions = get_personal_actions(user_id, return_type=SecretReturnType.NAME) current_action_names = set(action['name'] for action in current_actions) current_action_ids = {action.get('id') for action in current_actions if action.get('id')} + current_actions_by_id = {action['id']: action for action in current_actions if action.get('id')} + legacy_actions = {action['id']: action for action in list_legacy_personal_actions(user_id)} + global_actions_by_id = {action['id']: action for action in global_plugins if action.get('id')} # Filter out plugins whose name matches a global plugin name filtered_plugins = [] + legacy_replacements = {} new_plugin_names = set() new_plugin_ids = set() + submitted_ids = set() for plugin in plugins: - if plugin.get('name', '').lower() in global_plugin_names: + if not isinstance(plugin, dict): + raise McpConfigurationError("Each action must be an object.") + submitted_id = plugin.get('id') + if submitted_id is not None: + if not isinstance(submitted_id, str): + raise McpConfigurationError("Action identifiers must be strings.") + if submitted_id and submitted_id in submitted_ids: + raise McpConfigurationError("Each action identifier must occur only once.") + submitted_ids.add(submitted_id) + legacy_action = None + if isinstance(submitted_id, str) and submitted_id.startswith(LEGACY_ACTION_PREFIX): + legacy_action = legacy_actions.get(submitted_id) + if legacy_action is None: + raise LegacyActionConflictError() + if plugin == legacy_action: + continue + if is_retired_mcp_stdio(plugin): + existing_action = current_actions_by_id.get(submitted_id) + if existing_action is not None: + original = get_personal_action_record(user_id, submitted_id) + if is_unchanged_retired_action(plugin, original, 'personal', user_id): + continue + global_action = global_actions_by_id.get(submitted_id) + if global_action is not None and plugin == global_action: + continue + raise McpStdioRemovedError() + if ( + plugin.get('name', '').lower() in global_plugin_names + and submitted_id not in current_actions_by_id + and legacy_action is None + ): continue # Skip global plugins plugin_to_save = dict(plugin) # Remove is_global if present @@ -1005,14 +1118,13 @@ def set_user_plugins(): if field == 'id': continue plugin_to_save.pop(field, None) + for field in ('execution_status', 'is_legacy', 'legacy_source', 'legacy_locator', 'runtime_user_id'): + plugin_to_save.pop(field, None) # Handle endpoint based on plugin type - plugin_type = plugin_to_save.get('type', '') plugin_to_save.setdefault('endpoint', '') _apply_plugin_runtime_defaults(plugin_to_save) - mcp_stdio_error = _reject_non_admin_mcp_stdio(plugin_to_save, scope_label='personal') - if mcp_stdio_error: - return jsonify({'error': mcp_stdio_error}), 400 + plugin_type = plugin_to_save['type'] try: _validate_action_identity_for_scope( plugin_to_save, @@ -1030,13 +1142,6 @@ def set_user_plugins(): elif 'type' not in plugin_to_save['auth']: plugin_to_save['auth']['type'] = 'identity' - # Auto-fill type from metadata if missing or empty - if not plugin_to_save.get('type'): - if plugin_to_save.get('metadata', {}).get('type'): - plugin_to_save['type'] = plugin_to_save['metadata']['type'] - else: - plugin_to_save['type'] = 'unknown' # Default type - debug_print(f"Plugin build: {_redact_plugin_for_logging(plugin_to_save)}") validation_error = validate_plugin(plugin_to_save) if validation_error: @@ -1046,6 +1151,7 @@ def set_user_plugins(): return jsonify({'error': f'Plugin validation failed: {"; ".join(validation_errors)}'}), 400 try: + ensure_action_type_access('governance_user_actions', user_id, plugin_type, 'personal') _enforce_mcp_destination_policy( plugin_to_save, WORKSPACE_IDENTITY_SCOPE_PERSONAL, @@ -1053,12 +1159,16 @@ def set_user_plugins(): operation='personal_action_save', user_id=user_id, ) - except McpDestinationPolicyError: + except PermissionError: return jsonify({'error': 'MCP destination is not allowed by governance policy.'}), 403 except ValueError: return jsonify({'error': 'MCP destination configuration is invalid.'}), 400 - + + if legacy_action is not None: + prepare_legacy_personal_action_reconfiguration(user_id, submitted_id, plugin_to_save) filtered_plugins.append(plugin_to_save) + if legacy_action is not None: + legacy_replacements[submitted_id] = plugin_to_save new_plugin_names.add(plugin_to_save['name']) if plugin_to_save.get('id'): new_plugin_ids.add(plugin_to_save['id']) @@ -1067,10 +1177,15 @@ def set_user_plugins(): plugins_to_delete = [] try: for plugin in filtered_plugins: - save_personal_action(user_id, plugin) + if plugin.get('id') in legacy_replacements: + reconfigure_legacy_personal_action(user_id, plugin['id'], plugin) + else: + save_personal_action(user_id, plugin) # Delete any plugins that are no longer in the list for action in current_actions: + if is_retired_mcp_stdio(action): + continue action_id = action.get('id') action_name = action.get('name') if action_id and action_id in new_plugin_ids: @@ -1082,6 +1197,8 @@ def set_user_plugins(): for action in plugins_to_delete: delete_personal_action(user_id, action.get('id') or action.get('name')) + except (LegacyActionConflictError, LegacyActionSourceUpdateError) as exc: + return _handle_legacy_action_error(exc) except ValueError as e: debug_print(f"Validation error saving personal actions for user {user_id}: {e}") return jsonify({'error': ACTION_VALIDATION_ERROR_MESSAGE}), 400 @@ -1121,7 +1238,10 @@ def delete_user_plugin(plugin_name): # Try to delete from personal_actions container try: - deleted = delete_personal_action(user_id, plugin_name) + if plugin_name.startswith(LEGACY_ACTION_PREFIX): + deleted = delete_legacy_personal_action(user_id, plugin_name) + else: + deleted = delete_personal_action(user_id, plugin_name) except PermissionError: return jsonify({'error': 'You are not authorized to delete this action.'}), 403 @@ -1199,7 +1319,8 @@ def get_group_action_route(action_id): if not action: return jsonify({'error': 'Action not found'}), 404 try: - ensure_action_type_access('governance_group_actions', user_id, action.get('type'), 'group') + if not is_retired_mcp_stdio(action): + ensure_action_type_access('governance_group_actions', user_id, resolve_action_type(action), 'group') except PermissionError as exc: return jsonify({'error': str(exc)}), 403 return jsonify(action), 200 @@ -1225,6 +1346,7 @@ def create_group_action_route(): return jsonify({'error': 'You are not authorized to create this group action.'}), 403 payload = request.get_json(silent=True) or {} + _apply_plugin_runtime_defaults(payload) try: validate_group_action_payload(payload, partial=False) except ValueError as exc: @@ -1236,11 +1358,6 @@ def create_group_action_route(): for key in ('group_id', 'last_updated', 'user_id', 'is_global', 'is_group', 'scope'): payload.pop(key, None) - _apply_plugin_runtime_defaults(payload) - mcp_stdio_error = _reject_non_admin_mcp_stdio(payload, scope_label='group') - if mcp_stdio_error: - return jsonify({'error': mcp_stdio_error}), 400 - # Merge with schema to ensure all required fields are present (same as global actions) schema_dir = os.path.join(current_app.root_path, 'static', 'json', 'schemas') merged = get_merged_plugin_settings(payload.get('type'), payload, schema_dir) @@ -1316,17 +1433,14 @@ def update_group_action_route(action_id): return jsonify({'error': 'Action not found'}), 404 updates = request.get_json(silent=True) or {} + if not isinstance(updates, dict): + raise McpConfigurationError("Action configuration must be an object.") if updates.get('is_global'): return jsonify({'error': 'Global actions cannot be modified within a group.'}), 400 for key in ('id', 'group_id', 'last_updated', 'user_id', 'is_global', 'is_group', 'scope'): updates.pop(key, None) - try: - validate_group_action_payload(updates, partial=True) - except ValueError as exc: - return jsonify({'error': str(exc)}), 400 - merged = dict(existing) merged.update(updates) merged['is_global'] = False @@ -1334,9 +1448,6 @@ def update_group_action_route(action_id): merged['id'] = existing.get('id', action_id) _apply_plugin_runtime_defaults(merged) - mcp_stdio_error = _reject_non_admin_mcp_stdio(merged, scope_label='group') - if mcp_stdio_error: - return jsonify({'error': mcp_stdio_error}), 400 try: validate_group_action_payload(merged, partial=False) @@ -1415,8 +1526,8 @@ def delete_group_action_route(action_id): try: existing = get_group_action(active_group, action_id, return_type=SecretReturnType.NAME) - if existing: - ensure_action_type_access('governance_group_actions', user_id, existing.get('type'), 'group') + if existing and not is_retired_mcp_stdio(existing): + ensure_action_type_access('governance_group_actions', user_id, resolve_action_type(existing), 'group') removed = delete_group_action(active_group, action_id) except PermissionError as exc: return jsonify({'error': str(exc)}), 403 @@ -1577,6 +1688,8 @@ def set_plugin_enabled(plugin_name): if plugin_to_update is None: log_event("Toggle plugin enabled failed: not found", level=logging.WARNING, extra={"action": "toggle-enabled", "plugin_name": plugin_name}) return jsonify({'error': 'Plugin not found.'}), 404 + if is_retired_mcp_stdio(plugin_to_update): + return _handle_mcp_configuration_error(McpStdioRemovedError()) result = update_global_action_enabled( plugin_to_update.get('id'), @@ -1617,6 +1730,8 @@ def add_plugin(): try: plugins = get_global_actions(include_disabled=True) new_plugin = request.get_json(silent=True) or {} + if not isinstance(new_plugin, dict): + raise McpConfigurationError("Action configuration must be an object.") governance_policy_payload = new_plugin.pop('governance_policy', None) if isinstance(new_plugin, dict) else None _apply_plugin_runtime_defaults(new_plugin) new_plugin = apply_plugin_validation_defaults(new_plugin) @@ -1694,6 +1809,8 @@ def add_plugin(): # --- HOT RELOAD TRIGGER --- setattr(builtins, "kernel_reload_needed", True) return jsonify({'success': True}) + except McpConfigurationError as exc: + return _handle_mcp_configuration_error(exc) except ValueError as e: log_event(f"Validation error adding plugin: {e}", level=logging.WARNING) return jsonify({'error': PLUGIN_VALIDATION_ERROR_MESSAGE}), 400 @@ -1712,6 +1829,8 @@ def edit_plugin(plugin_name): try: plugins = get_global_actions(include_disabled=True) updated_plugin = request.get_json(silent=True) or {} + if not isinstance(updated_plugin, dict): + raise McpConfigurationError("Action configuration must be an object.") governance_policy_payload = updated_plugin.pop('governance_policy', None) if isinstance(updated_plugin, dict) else None _apply_plugin_runtime_defaults(updated_plugin) updated_plugin = apply_plugin_validation_defaults(updated_plugin) @@ -1803,6 +1922,8 @@ def edit_plugin(plugin_name): log_event("Edit plugin failed: not found", level=logging.WARNING, extra={"action": "edit", "plugin_name": plugin_name}) return jsonify({'error': 'Plugin not found.'}), 404 + except McpConfigurationError as exc: + return _handle_mcp_configuration_error(exc) except ValueError as e: log_event(f"Validation error editing plugin: {e}", level=logging.WARNING) return jsonify({'error': PLUGIN_VALIDATION_ERROR_MESSAGE}), 400 @@ -1960,11 +2081,15 @@ def discover_mcp_tools(): try: existing_plugin = _load_existing_plugin_for_test(payload.get('plugin_context'), user_id) scope_type, scope_id = _resolve_action_identity_context(payload, existing_plugin, user_id) - plugin_scope_value, plugin_scope = _resolve_plugin_secret_context(existing_plugin, user_id) + origin = _action_test_origin(existing_plugin, scope_type, scope_id) + plugin_scope_value, plugin_scope = _action_origin_secret_context(origin) discovery_manifest = dict(payload) discovery_manifest['mcp_operation_id'] = mcp_operation_id - discovery_manifest['runtime_user_id'] = user_id + discovery_manifest.pop('runtime_user_id', None) + discovery_manifest.pop('id', None) + if origin.action_id: + discovery_manifest['id'] = origin.action_id discovery_manifest['type'] = MCP_PLUGIN_TYPE discovery_manifest.setdefault('name', 'mcp_discovery') discovery_manifest.setdefault('displayName', 'MCP Discovery') @@ -1972,28 +2097,15 @@ def discover_mcp_tools(): discovery_manifest.setdefault('metadata', {}) discovery_manifest.setdefault('additionalFields', {}) _apply_plugin_runtime_defaults(discovery_manifest) - if discovery_manifest.get('additionalFields', {}).get('transport') == 'stdio' and scope_type != WORKSPACE_IDENTITY_SCOPE_GLOBAL: - log_event( - "[MCP_DISCOVERY] Failed", - extra=_build_mcp_discovery_log_context( - mcp_operation_id, - user_id, - discovery_manifest, - scope_type, - scope_id, - started_at, - { - "category": "authorization", - "http_status": 403, - "failure_stage": "transport_scope_validation", - }, - ), - level=logging.WARNING, - ) - return jsonify({ - 'error': 'MCP stdio discovery is only available for admin-managed global actions.', - 'mcp_operation_id': mcp_operation_id, - }), 403 + discovery_manifest = ScopedActionManifest(discovery_manifest, origin) + _enforce_mcp_destination_policy( + discovery_manifest, + scope_type, + scope_id, + operation='mcp_discovery_prepare', + user_id=user_id, + mcp_operation_id=mcp_operation_id, + ) auth = discovery_manifest.get('auth') if isinstance(discovery_manifest.get('auth'), dict) else {} existing_auth = existing_plugin.get('auth') if isinstance(existing_plugin, dict) and isinstance(existing_plugin.get('auth'), dict) else {} @@ -2010,6 +2122,7 @@ def discover_mcp_tools(): scope_id, return_type=SecretReturnType.VALUE, ) + discovery_manifest = ScopedActionManifest(discovery_manifest, origin) else: auth = discovery_manifest.get('auth') if isinstance(discovery_manifest.get('auth'), dict) else {} if auth.get('key'): @@ -2061,7 +2174,7 @@ def discover_mcp_tools(): mcp_operation_id=mcp_operation_id, ) - probe_result = asyncio.run(McpPluginFactory.probe_server_from_config(discovery_manifest)) + probe_result = asyncio.run(McpPluginFactory.probe_server_from_config(discovery_manifest, origin=origin)) tools = probe_result.get('tools', []) if isinstance(probe_result, dict) else [] log_event( "[MCP_DISCOVERY] Completed", @@ -2114,6 +2227,18 @@ def discover_mcp_tools(): 'error': 'MCP destination is not allowed by governance policy.', 'mcp_operation_id': mcp_operation_id, }), 403 + except McpConfigurationError as exc: + log_event( + "[MCP_DISCOVERY] Configuration rejected", + extra={"error_type": exc.code, "mcp_operation_id": mcp_operation_id}, + level=logging.WARNING, + ) + return jsonify({ + 'success': False, + 'error': exc.public_message, + 'error_type': exc.code, + 'mcp_operation_id': mcp_operation_id, + }), 400 except (LookupError, ValueError) as exc: log_event( "[MCP_DISCOVERY] Failed", @@ -3022,7 +3147,8 @@ def _prepare_action_test_manifest(data, plugin_type, plugin_label): scope_type, scope_id = _resolve_action_identity_context(data, existing_plugin, user_id) # Secret references are resolved against the loaded action's own Key Vault scope, never # against a scope derived from the request body. - plugin_scope_value, plugin_scope = _resolve_plugin_secret_context(existing_plugin, user_id) + origin = _action_test_origin(existing_plugin, scope_type, scope_id) + plugin_scope_value, plugin_scope = _action_origin_secret_context(origin) default_name = f'{plugin_type}_connection_test' manifest = { @@ -3041,6 +3167,15 @@ def _prepare_action_test_manifest(data, plugin_type, plugin_label): manifest['identity_id'] = identity_id _apply_plugin_runtime_defaults(manifest) + manifest = ScopedActionManifest(manifest, origin) + if plugin_type == MCP_PLUGIN_TYPE: + _enforce_mcp_destination_policy( + manifest, + scope_type, + scope_id, + operation='mcp_connection_test_prepare', + user_id=user_id, + ) existing_auth = {} existing_additional_fields = {} @@ -3099,6 +3234,7 @@ def _prepare_action_test_manifest(data, plugin_type, plugin_label): scope_id, return_type=SecretReturnType.VALUE, ) + manifest = ScopedActionManifest(manifest, origin) else: auth = manifest.get('auth') if isinstance(manifest.get('auth'), dict) else {} if auth.get('key'): @@ -3138,17 +3274,22 @@ def _run_action_connection_test(plugin_type, plugin_label, tester, before_test=N manifest, scope_type, scope_id = _prepare_action_test_manifest(data, plugin_type, plugin_label) if callable(before_test): before_test(manifest, scope_type, scope_id) - except PermissionError as exc: - return jsonify({'success': False, 'error': str(exc)}), 403 - except LookupError as exc: - return jsonify({'success': False, 'error': str(exc)}), 404 - except ValueError as exc: - return jsonify({'success': False, 'error': str(exc)}), 400 + except McpConfigurationError as exc: + return _handle_mcp_configuration_error(exc) + except PermissionError: + return jsonify({'success': False, 'error': 'This action connection test is not authorized.'}), 403 + except LookupError: + return jsonify({'success': False, 'error': 'The requested action was not found.'}), 404 + except ValueError: + return jsonify({'success': False, 'error': 'Invalid action connection test configuration.'}), 400 except McpRuntimeError as exc: - return jsonify({'success': False, 'error': str(exc)}), get_mcp_error_http_status(exc) + return jsonify({'success': False, 'error': 'MCP operation failed.', 'error_type': exc.category}), get_mcp_error_http_status(exc.category) try: - result = tester(manifest) + if plugin_type == MCP_PLUGIN_TYPE: + result = tester(manifest, origin=get_action_origin(manifest)) + else: + result = tester(manifest) except Exception as exc: log_event( f'[ACTION_TEST] {plugin_label} connection test raised an unexpected error: {exc}', @@ -3226,25 +3367,10 @@ def test_log_analytics_action_connection(): @user_required def test_mcp_action_connection(): """Test an MCP action by initializing a session and listing the server's tools.""" - - def enforce_mcp_test_policy(manifest, scope_type, scope_id): - if scope_type != WORKSPACE_IDENTITY_SCOPE_GLOBAL: - stdio_error = _reject_non_admin_mcp_stdio(manifest, scope_label='non-global') - if stdio_error: - raise PermissionError(stdio_error) - _enforce_mcp_destination_policy( - manifest, - scope_type, - scope_id, - operation='mcp_connection_test', - user_id=get_current_user_id(), - ) - return _run_action_connection_test( MCP_PLUGIN_TYPE, 'MCP', test_mcp_connection, - before_test=enforce_mcp_test_policy, ) diff --git a/application/single_app/route_backend_users.py b/application/single_app/route_backend_users.py index 25f414d62..5406bf0e0 100644 --- a/application/single_app/route_backend_users.py +++ b/application/single_app/route_backend_users.py @@ -11,6 +11,7 @@ normalize_collaboration_user, ) from functions_appinsights import log_event +from functions_action_manifest import McpConfigurationError from functions_ai_notice import ( AI_NOTICE_USER_SETTINGS_KEY, build_ai_notice_dismissal_record, @@ -26,6 +27,11 @@ LATEST_FEATURES_HIDDEN_VERSION_SETTING, normalize_latest_features_hidden_version, ) +from functions_legacy_action_management import ( + LegacyActionConflictError, + legacy_action_management_view, + legacy_action_snapshots, +) from functions_public_workspaces import update_active_public_workspace_for_user from functions_settings import * from swagger_wrapper import swagger_route, get_auth_security @@ -463,26 +469,30 @@ def add_suggestion(raw_value, source_label): def user_settings(): try: user_id = get_current_user_id() - if not user_id: # Redundant if get_current_user_id raises error, but safe - return jsonify({"error": "Unable to identify user"}), 401 - except ValueError as e: - # Handle case where get_current_user_id fails (e.g., session issue) - print(f"Error getting user ID: {e}") - return jsonify({"error": str(e)}), 401 + if not user_id: + return jsonify({"error": "Unable to identify user"}), 401 + except ValueError: + log_event("Unable to identify settings user", level=logging.WARNING) + return jsonify({"error": "Unable to identify user"}), 401 except Exception as e: - # Catch other potential errors during user ID retrieval - print(f"Unexpected error getting user ID: {e}") - return jsonify({"error": "Internal server error identifying user"}), 500 + log_event( + "Unable to identify settings user", + extra={"error_type": type(e).__name__}, + level=logging.ERROR, + ) + return jsonify({"error": "Internal server error identifying user"}), 500 # --- Handle POST Request (Update Settings) --- if request.method == 'POST': try: # Expect JSON data, as sent by the fetch API in chat-layout.js - data = request.get_json() + data = request.get_json(silent=True) - if not data: + if data is None: return jsonify({"error": "Missing JSON body"}), 400 + if not isinstance(data, dict): + return jsonify({"error": "Request body must be an object"}), 400 # The JS sends { settings: { key: value, ... } } # Extract the inner 'settings' dictionary @@ -535,7 +545,11 @@ def user_settings(): } # Add others as needed invalid_keys = set(settings_to_update.keys()) - allowed_keys if invalid_keys: - print(f"Warning: Received invalid settings keys: {invalid_keys}") + log_event( + "Ignored unsupported user settings fields", + extra={"field_count": len(invalid_keys)}, + level=logging.WARNING, + ) settings_to_update = { key: value for key, value in settings_to_update.items() @@ -637,6 +651,20 @@ def user_settings(): return jsonify({"error": "Invalid Latest Features hidden version"}), 400 settings_to_update[LATEST_FEATURES_HIDDEN_VERSION_SETTING] = hidden_version + if "plugins" in settings_to_update: + # Action storage is only needed for imports, not settings/profile bootstrap. + from functions_personal_actions import prepare_legacy_personal_actions_update + + prepared_actions = prepare_legacy_personal_actions_update( + user_id, settings_to_update["plugins"] + ) + if prepared_actions["has_imports"] and not get_settings().get("allow_user_plugins", False): + return jsonify({"error": "Personal action imports are disabled."}), 403 + if prepared_actions["changed"]: + settings_to_update["plugins"] = prepared_actions["plugins"] + else: + settings_to_update.pop("plugins") + active_group_updated = False active_public_workspace_updated = False @@ -682,20 +710,48 @@ def user_settings(): # update_user_settings should ideally log the specific error return jsonify({"error": "Failed to update settings"}), 500 + except (McpConfigurationError, LegacyActionConflictError) as exc: + log_event( + "Rejected legacy action settings update", + extra={"user_id": user_id, "error_type": exc.code}, + level=logging.WARNING, + ) + status = 409 if isinstance(exc, LegacyActionConflictError) else 400 + return jsonify({"error": exc.public_message, "error_type": exc.code}), status + except PermissionError: + log_event("User settings update denied", extra={"user_id": user_id}, level=logging.WARNING) + return jsonify({"error": "You are not authorized to import or change these actions."}), 403 + except ValueError: + log_event("Invalid user settings update", extra={"user_id": user_id}, level=logging.WARNING) + return jsonify({"error": "Invalid action configuration."}), 400 except Exception as e: - # Catch potential JSON parsing errors or other unexpected issues - print(f"Error processing POST /api/user/settings: {e}") + log_event( + "Failed to update user settings", + extra={"user_id": user_id, "error_type": type(e).__name__}, + level=logging.ERROR, + ) return jsonify({"error": "Internal server error processing request"}), 500 - # --- Handle GET Request (Retrieve Settings) --- - # This part remains largely the same as your original try: - user_settings_data = get_user_settings(user_id) # This fetches the whole document - # The frontend JS expects the document structure, including the 'settings' key inside it. - return jsonify(user_settings_data), 200 # Return the full document or {} if not found + user_settings_data = get_user_settings(user_id) + public_user_settings = sanitize_settings_for_user(user_settings_data) + stored_settings = user_settings_data.get("settings", {}) + if "plugins" in stored_settings: + stored_plugins = stored_settings["plugins"] + if stored_plugins is None: + stored_plugins = [] + public_user_settings["settings"]["plugins"] = [ + legacy_action_management_view(snapshot) + for snapshot in legacy_action_snapshots(user_id, stored_plugins) + ] + return jsonify(public_user_settings), 200 except Exception as e: - print(f"Error retrieving settings for user {user_id}: {e}") + log_event( + "Failed to retrieve user settings", + extra={"user_id": user_id, "error_type": type(e).__name__}, + level=logging.ERROR, + ) return jsonify({"error": "Failed to retrieve user settings"}), 500 @bp.route('/api/user/profile-image/', methods=['GET']) diff --git a/application/single_app/route_migration.py b/application/single_app/route_migration.py index 9c4f572e2..dbb7f89f2 100644 --- a/application/single_app/route_migration.py +++ b/application/single_app/route_migration.py @@ -4,13 +4,21 @@ Migration endpoints for moving data from user settings to personal containers. """ -from flask import Blueprint, jsonify, request +import logging + +from flask import Blueprint, jsonify from functions_authentication import get_current_user_id, login_required, user_required, user_required_blueprint from functions_personal_agents import migrate_agents_from_user_settings, get_personal_agents -from functions_personal_actions import migrate_actions_from_user_settings, get_personal_actions +from functions_personal_actions import ( + get_action_migration_status, + get_personal_actions, + list_legacy_personal_actions, + migrate_actions_from_user_settings, +) +from functions_settings import get_user_settings, update_user_settings +from functions_keyvault import redact_plugin_secret_values from functions_appinsights import log_event from swagger_wrapper import swagger_route, get_auth_security -import logging bp_migration = Blueprint('migration', __name__) bp_migration.before_request(user_required_blueprint()) @@ -29,7 +37,7 @@ def migrate_user_agents(): migrated_count = migrate_agents_from_user_settings(user_id) agents = get_personal_agents(user_id) - log_event("User agents migrated", extra={ + log_event("[USER_SETTINGS] User agents migrated", extra={ "user_id": user_id, "migrated_count": migrated_count, "total_agents": len(agents) @@ -42,8 +50,9 @@ def migrate_user_agents(): 'agents': agents }) - except Exception as e: - log_event(f"Error migrating user agents: {e}", level=logging.ERROR, exceptionTraceback=True) + except Exception as exc: + log_event("[USER_SETTINGS] Agent migration failed", level=logging.ERROR, + extra={"user_id": user_id, "error_type": type(exc).__name__}) return jsonify({'error': 'Failed to migrate agents'}), 500 @bp_migration.route('/api/migrate/actions', methods=['POST']) @@ -57,24 +66,31 @@ def migrate_user_actions(): user_id = get_current_user_id() try: - migrated_count = migrate_actions_from_user_settings(user_id) - actions = get_personal_actions(user_id) + outcome = migrate_actions_from_user_settings(user_id) + actions = [redact_plugin_secret_values(action) for action in get_personal_actions(user_id)] + legacy_actions = list_legacy_personal_actions(user_id) - log_event("User actions migrated", extra={ + log_event("[PLUGINS] User action migration processed", extra={ "user_id": user_id, - "migrated_count": migrated_count, + "migrated_count": outcome["migrated_count"], + "retained_count": outcome["retained_count"], + "failed_count": outcome["failed_count"], "total_actions": len(actions) }) return jsonify({ - 'success': True, - 'migrated_count': migrated_count, + **outcome, + 'success': outcome["complete"] and outcome["failed_count"] == 0, 'total_actions': len(actions), - 'actions': actions + 'actions': actions + legacy_actions, + 'action_migration': outcome, }) - except Exception as e: - log_event(f"Error migrating user actions: {e}", level=logging.ERROR, exceptionTraceback=True) + except PermissionError: + return jsonify({'error': 'You are not authorized to migrate these actions.'}), 403 + except Exception as exc: + log_event("[PLUGINS] User action migration failed", level=logging.ERROR, + extra={"user_id": user_id, "error_type": type(exc).__name__}) return jsonify({'error': 'Failed to migrate actions'}), 500 @bp_migration.route('/api/migrate/all', methods=['POST']) @@ -89,49 +105,47 @@ def migrate_all_user_data(): try: agents_migrated = migrate_agents_from_user_settings(user_id) - actions_migrated = migrate_actions_from_user_settings(user_id) + action_outcome = migrate_actions_from_user_settings(user_id) - # Force clear any remaining legacy data in user settings - from functions_settings import get_user_settings, update_user_settings user_settings = get_user_settings(user_id) - settings_to_update = user_settings.get('settings', {}) - - # Set legacy data to empty arrays instead of removing keys - legacy_cleared = False - if settings_to_update.get('agents'): - settings_to_update['agents'] = [] - legacy_cleared = True - if settings_to_update.get('plugins'): - settings_to_update['plugins'] = [] - legacy_cleared = True - - if legacy_cleared: - update_user_settings(user_id, settings_to_update) - log_event(f"Forced clearing of legacy data for user {user_id}") + if user_settings.get('settings', {}).get('agents'): + agents_cleared = update_user_settings(user_id, {'agents': []}) + if not agents_cleared: + return jsonify({'error': 'Failed to finish agent migration.'}), 500 agents = get_personal_agents(user_id) - actions = get_personal_actions(user_id) + actions = [redact_plugin_secret_values(action) for action in get_personal_actions(user_id)] + legacy_actions = list_legacy_personal_actions(user_id) - log_event("All user data migrated", extra={ + log_event("[USER_SETTINGS] User migration processed", extra={ "user_id": user_id, "agents_migrated": agents_migrated, - "actions_migrated": actions_migrated, + "actions_migrated": action_outcome["migrated_count"], + "actions_retained": action_outcome["retained_count"], + "actions_failed": action_outcome["failed_count"], "total_agents": len(agents), "total_actions": len(actions) }) return jsonify({ - 'success': True, + 'success': action_outcome["complete"] and action_outcome["failed_count"] == 0, 'agents_migrated': agents_migrated, - 'actions_migrated': actions_migrated, + 'actions_migrated': action_outcome["migrated_count"], + 'actions_retained': action_outcome["retained_count"], + 'actions_failed': action_outcome["failed_count"], + 'action_migration': action_outcome, + 'migration_complete': action_outcome["complete"], 'total_agents': len(agents), 'total_actions': len(actions), 'agents': agents, - 'actions': actions + 'actions': actions + legacy_actions, }) - except Exception as e: - log_event(f"Error migrating all user data: {e}", level=logging.ERROR, exceptionTraceback=True) + except PermissionError: + return jsonify({'error': 'You are not authorized to migrate these actions.'}), 403 + except Exception as exc: + log_event("[USER_SETTINGS] User migration failed", level=logging.ERROR, + extra={"user_id": user_id, "error_type": type(exc).__name__}) return jsonify({'error': 'Failed to migrate user data'}), 500 @bp_migration.route('/api/migrate/status', methods=['GET']) @@ -145,23 +159,25 @@ def get_migration_status(): user_id = get_current_user_id() try: - from functions_settings import get_user_settings - # Check current user settings user_settings = get_user_settings(user_id).get('settings', {}) legacy_agents = user_settings.get('agents', []) - legacy_actions = user_settings.get('plugins', []) + action_status = get_action_migration_status(user_id) # Check personal containers personal_agents = get_personal_agents(user_id) - personal_actions = get_personal_actions(user_id) + personal_actions = [redact_plugin_secret_values(action) for action in get_personal_actions(user_id)] return jsonify({ 'legacy_data': { 'agents_count': len(legacy_agents), - 'actions_count': len(legacy_actions), + 'actions_count': action_status["total_count"], + 'actions_pending_count': action_status["pending_count"], + 'actions_retained_count': action_status["retained_count"], + 'actions_unsupported_count': action_status["retired_count"], + 'actions_failed_count': action_status["failed_count"], 'agents': legacy_agents, - 'actions': legacy_actions + 'actions': action_status["actions"], }, 'personal_containers': { 'agents_count': len(personal_agents), @@ -169,9 +185,13 @@ def get_migration_status(): 'agents': personal_agents, 'actions': personal_actions }, - 'migration_needed': len(legacy_agents) > 0 or len(legacy_actions) > 0 + 'action_migration': action_status, + 'migration_needed': len(legacy_agents) > 0 or not action_status["complete"], }) - except Exception as e: - log_event(f"Error checking migration status: {e}", level=logging.ERROR, exceptionTraceback=True) + except PermissionError: + return jsonify({'error': 'You are not authorized to view this migration status.'}), 403 + except Exception as exc: + log_event("[USER_SETTINGS] Migration status lookup failed", level=logging.ERROR, + extra={"user_id": user_id, "error_type": type(exc).__name__}) return jsonify({'error': 'Failed to check migration status'}), 500 diff --git a/application/single_app/semantic_kernel_loader.py b/application/single_app/semantic_kernel_loader.py index 2c38cad7c..d48f1a1c2 100644 --- a/application/single_app/semantic_kernel_loader.py +++ b/application/single_app/semantic_kernel_loader.py @@ -46,6 +46,17 @@ resolve_openai_style_request_api_version, ) from functions_appinsights import log_event, get_appinsights_logger +from functions_action_manifest import ( + McpConfigurationError, + McpStdioRemovedError, + ScopedActionManifest, + copy_action_manifest, + get_action_execution_status, + get_action_origin, + is_mcp_action, + is_retired_mcp_stdio, + resolve_action_type, +) from functions_authentication import get_current_user_id_or_none from semantic_kernel_plugins.plugin_health_checker import PluginHealthChecker, PluginErrorRecovery from semantic_kernel_plugins.logged_plugin_loader import create_logged_plugin_loader @@ -111,6 +122,8 @@ ) from semantic_kernel_plugins.plugin_loader import discover_plugins from functions_mcp_operations import MCP_PLUGIN_TYPE +from functions_mcp_destinations import McpDestinationPolicyError, resolve_mcp_execution_context +from functions_mcp_preconfigurations import authorize_mcp_action from semantic_kernel_plugins.databricks_plugin_factory import DatabricksPluginFactory from semantic_kernel_plugins.mcp_plugin_factory import McpPluginFactory from semantic_kernel_plugins.openapi_plugin_factory import OpenApiPluginFactory @@ -1301,18 +1314,9 @@ def load_agent_specific_plugins(kernel, plugin_names, settings, mode_label="glob ) debug_print(f"[SK_LOADER] Filtered to {len(plugin_manifests)} plugin manifests after matching names/IDs") - debug_print(f"[SK_LOADER] Plugin manifests to load: {plugin_manifests}") + debug_print(f"[SK_LOADER] Plugin names to load: {[manifest.get('name') for manifest in plugin_manifests]}") - if settings.get("enable_key_vault_secret_storage", False) and settings.get("key_vault_name"): - debug_print(f"[SK_LOADER] Resolving Key Vault secrets in plugin manifests if needed") - try: - plugin_manifests = [resolve_key_vault_secrets_in_plugins(p, settings) for p in plugin_manifests] - debug_print(f"[SK_LOADER] Resolved Key Vault secrets in plugin manifests {plugin_manifests}") - except Exception as e: - log_event(f"[SK_LOADER] Failed to resolve Key Vault secrets in plugin manifests: {e}", level=logging.ERROR, exceptionTraceback=True) - print(f"[SK_LOADER] Failed to resolve Key Vault secrets in plugin manifests: {e}") - - plugin_manifests = [hydrate_workspace_identity_in_plugin(p) for p in plugin_manifests] + plugin_manifests = _prepare_plugin_manifests_for_runtime(plugin_manifests, settings) if not plugin_manifests: print(f"[SK_LOADER] Warning: No plugin manifests found for names/IDs: {plugin_names}") @@ -1393,6 +1397,7 @@ def load_agent_specific_plugins(kernel, plugin_names, settings, mode_label="glob agent_other_settings=agent_other_settings, group_id=group_id, ) + plugin_manifests = _prepare_plugin_manifests_for_runtime(plugin_manifests, settings) _load_agent_plugins_original_method(kernel, plugin_manifests, mode_label) except Exception as fallback_error: log_event( @@ -1413,7 +1418,7 @@ def _apply_agent_plugin_runtime_overlays(plugin_manifests, agent_other_settings= overlaid_manifests = [] for manifest in plugin_manifests or []: - manifest_copy = dict(manifest) + manifest_copy = copy_action_manifest(manifest) if group_id and not manifest_copy.get('group_id'): manifest_copy['default_group_id'] = group_id @@ -1495,7 +1500,14 @@ def _load_agent_plugins_original_method(kernel, plugin_manifests, mode_label="gl discovered_plugins = discover_plugins() for manifest in plugin_manifests: - plugin_type = manifest.get('type') + try: + plugin_type = resolve_action_type(manifest) + except ValueError: + log_event("[SK_LOADER] Skipping action with an invalid type.", level=logging.WARNING) + continue + if get_action_execution_status(manifest): + _log_unavailable_mcp_action(manifest) + continue name = manifest.get('name') description = manifest.get('description', '') @@ -1504,8 +1516,10 @@ def normalize(s): return s.replace('_', '').replace('-', '').replace('plugin', '').lower() if s else '' normalized_type = normalize(plugin_type) - matched_class = None + matched_class = McpPluginFactory if plugin_type == MCP_PLUGIN_TYPE else None for class_name, cls in discovered_plugins.items(): + if resolve_action_type({"type": cls.__name__}) == MCP_PLUGIN_TYPE and plugin_type != MCP_PLUGIN_TYPE: + continue normalized_class = normalize(class_name) if normalized_type == normalized_class or normalized_type in normalized_class: matched_class = cls @@ -1529,8 +1543,11 @@ def normalize(s): elif plugin_type == YAMCS_PLUGIN_TYPE: plugin = YamcsPluginFactory.create_from_config(manifest) print(f"[SK_LOADER] Created Yamcs plugin: {name}") - elif plugin_type == MCP_PLUGIN_TYPE or normalized_type == normalize(MCP_PLUGIN_TYPE): - plugin = McpPluginFactory.create_from_config(manifest) + elif plugin_type == MCP_PLUGIN_TYPE: + origin = get_action_origin(manifest) + if origin is None: + raise McpDestinationPolicyError("MCP execution requires a trusted action origin.") + plugin = McpPluginFactory.create_from_config(manifest, origin=origin) print(f"[SK_LOADER] Created MCP plugin: {name}") else: # Standard plugin instantiation @@ -2090,6 +2107,14 @@ def _get_plugin_secret_context(plugin_manifest): if not isinstance(plugin_manifest, dict): return None, None + if is_mcp_action(plugin_manifest): + origin = get_action_origin(plugin_manifest) + if origin is None: + raise McpDestinationPolicyError("MCP execution requires a trusted action origin.") + if origin.scope_type == "global": + return origin.action_id, "global" + return origin.scope_id, "user" if origin.scope_type == "personal" else "group" + plugin_scope = str(plugin_manifest.get("scope") or "").strip().lower() if plugin_scope == "group" or plugin_manifest.get("is_group"): return plugin_manifest.get("group_id"), "group" @@ -2105,6 +2130,12 @@ def _get_plugin_identity_context(plugin_manifest): if not isinstance(plugin_manifest, dict): return None, None + if is_mcp_action(plugin_manifest): + origin = get_action_origin(plugin_manifest) + if origin is None: + raise McpDestinationPolicyError("MCP execution requires a trusted action origin.") + return origin.scope_type, origin.scope_id + plugin_scope = str(plugin_manifest.get("scope") or "").strip().lower() if plugin_scope == "group" or plugin_manifest.get("is_group"): return WORKSPACE_IDENTITY_SCOPE_GROUP, plugin_manifest.get("group_id") @@ -2119,31 +2150,46 @@ def hydrate_workspace_identity_in_plugin(plugin_manifest): """Resolve a reusable workspace identity reference before runtime plugin loading.""" if not isinstance(plugin_manifest, dict): return plugin_manifest + if is_retired_mcp_stdio(plugin_manifest): + raise McpStdioRemovedError() if not get_action_identity_reference_id(plugin_manifest): return plugin_manifest + mcp_action = is_mcp_action(plugin_manifest) + if mcp_action: + origin, _ = resolve_mcp_execution_context(plugin_manifest) + authorize_mcp_action( + plugin_manifest, + origin=origin, + settings=get_settings(), + operation="mcp_identity_hydration", + ) scope_type, scope_id = _get_plugin_identity_context(plugin_manifest) if not scope_type or not scope_id: return plugin_manifest try: - return hydrate_action_identity_reference( - plugin_manifest, + hydrated_manifest = hydrate_action_identity_reference( + copy_action_manifest(plugin_manifest), scope_type, scope_id, return_type=SecretReturnType.VALUE, ) + origin = get_action_origin(plugin_manifest) + return ScopedActionManifest(hydrated_manifest, origin) if origin is not None else hydrated_manifest except Exception as exc: log_event( - f"[SK_LOADER] Failed to hydrate workspace identity for plugin '{plugin_manifest.get('name')}': {exc}", + "[SK_LOADER] Failed to hydrate workspace identity for plugin", extra={ "plugin_name": plugin_manifest.get("name"), "plugin_id": plugin_manifest.get("id"), "scope_type": scope_type, + "exception_type": type(exc).__name__, }, level=logging.ERROR, - exceptionTraceback=True, ) + if mcp_action: + raise McpConfigurationError("MCP action credentials could not be resolved.") from exc return plugin_manifest @@ -2169,13 +2215,23 @@ def resolve_key_vault_secrets_in_plugins(plugin_manifest, settings): """ if not isinstance(plugin_manifest, dict): raise ValueError("Plugin manifest must be a dictionary") - + mcp_action = is_mcp_action(plugin_manifest) + if mcp_action: + origin, _ = resolve_mcp_execution_context(plugin_manifest) + settings = get_settings() + authorize_mcp_action( + plugin_manifest, + origin=origin, + settings=settings, + operation="mcp_secret_hydration", + ) + kv_name = settings.get("key_vault_name") if not kv_name: raise ValueError("Key Vault name not configured in settings") scope_value, scope = _get_plugin_secret_context(plugin_manifest) - resolved_manifest = dict(plugin_manifest) + resolved_manifest = copy_action_manifest(plugin_manifest) auth = plugin_manifest.get("auth", {}) if isinstance(auth, dict): @@ -2184,6 +2240,8 @@ def resolve_key_vault_secrets_in_plugins(plugin_manifest, settings): value = auth.get(auth_field) if not isinstance(value, str) or not validate_secret_name_dynamic(value): continue + if mcp_action and not scope_value: + raise McpDestinationPolicyError("MCP credential resolution requires an authorized action identifier.") try: resolved_auth[auth_field] = resolve_secret_reference_for_context( value, @@ -2193,6 +2251,8 @@ def resolve_key_vault_secrets_in_plugins(plugin_manifest, settings): context_label=f"plugin auth field '{auth_field}'", ) except ValueError as exc: + if mcp_action: + raise McpConfigurationError("MCP action credentials could not be resolved.") from exc log_event( f"[SK_LOADER] Blocked plugin auth secret resolution for field '{auth_field}': {exc}", extra={ @@ -2213,6 +2273,8 @@ def resolve_key_vault_secrets_in_plugins(plugin_manifest, settings): continue if not (field_name.endswith("__Secret") or _is_sensitive_plugin_additional_field(plugin_manifest, field_name)): continue + if mcp_action and not scope_value: + raise McpDestinationPolicyError("MCP credential resolution requires an authorized action identifier.") try: resolved_additional_fields[field_name] = resolve_secret_reference_for_context( value, @@ -2222,6 +2284,8 @@ def resolve_key_vault_secrets_in_plugins(plugin_manifest, settings): context_label=f"plugin additional field '{field_name}'", ) except ValueError as exc: + if mcp_action: + raise McpConfigurationError("MCP action credentials could not be resolved.") from exc log_event( f"[SK_LOADER] Blocked plugin additionalField secret resolution for '{field_name}': {exc}", extra={ @@ -2236,16 +2300,56 @@ def resolve_key_vault_secrets_in_plugins(plugin_manifest, settings): return resolved_manifest + +def _log_unavailable_mcp_action(manifest, error=None): + status = get_action_execution_status(manifest) or { + "code": getattr(error, "code", "authorization" if isinstance(error, PermissionError) else "validation"), + "message": "MCP action could not be loaded. Check its configuration and access policy.", + } + log_event( + "[SK_LOADER] MCP action unavailable", + extra={"plugin_name": manifest.get("name"), "plugin_id": manifest.get("id"), **status}, + level=logging.WARNING, + ) + + +def _prepare_plugin_manifests_for_runtime(plugin_manifests, settings): + """Hydrate independently so unavailable MCP actions do not disable other actions.""" + prepared_manifests = [] + for manifest in plugin_manifests or []: + if is_retired_mcp_stdio(manifest): + _log_unavailable_mcp_action(manifest) + prepared_manifests.append(manifest) + continue + mcp_action = False + try: + prepared = copy_action_manifest(manifest) + prepared["type"] = resolve_action_type(prepared) + mcp_action = is_mcp_action(prepared) + if mcp_action and get_action_origin(prepared) is None: + raise McpDestinationPolicyError("MCP execution requires a trusted action origin.") + if settings.get("enable_key_vault_secret_storage", False) and settings.get("key_vault_name"): + prepared = resolve_key_vault_secrets_in_plugins(prepared, settings) + prepared = hydrate_workspace_identity_in_plugin(prepared) + prepared_manifests.append(prepared) + except Exception as exc: + if mcp_action: + _log_unavailable_mcp_action(manifest, exc) + continue + log_event( + "[SK_LOADER] Could not prepare action credentials", + extra={"plugin_name": manifest.get("name"), "exception_type": type(exc).__name__}, + level=logging.WARNING, + ) + prepared_manifests.append(manifest) + return prepared_manifests + + def load_plugins_for_kernel(kernel, plugin_manifests, settings, mode_label="global"): """ DRY helper to load plugins from a manifest list (user or global). """ - if settings.get("enable_key_vault_secret_storage", False) and settings.get("key_vault_name"): - try: - plugin_manifests = [resolve_key_vault_secrets_in_plugins(p, settings) for p in plugin_manifests] - except Exception as e: - log_event(f"[SK_LOADER] Failed to resolve Key Vault secrets in plugin manifests: {e}", level=logging.ERROR, exceptionTraceback=True) - plugin_manifests = [hydrate_workspace_identity_in_plugin(p) for p in plugin_manifests] + plugin_manifests = _prepare_plugin_manifests_for_runtime(plugin_manifests, settings) # Create logged plugin loader for enhanced logging logged_loader = create_logged_plugin_loader(kernel) @@ -2377,15 +2481,24 @@ def _load_plugins_original_method(kernel, plugin_manifests, settings, mode_label try: discovered_plugins = discover_plugins() for manifest in plugin_manifests: - plugin_type = manifest.get('type') + try: + plugin_type = resolve_action_type(manifest) + except ValueError: + log_event("[SK_LOADER] Skipping action with an invalid type.", level=logging.WARNING) + continue + if get_action_execution_status(manifest): + _log_unavailable_mcp_action(manifest) + continue name = manifest.get('name') description = manifest.get('description', '') # Normalize for matching def normalize(s): return s.replace('_', '').replace('-', '').replace('plugin', '').lower() if s else '' normalized_type = normalize(plugin_type) - matched_class = None + matched_class = McpPluginFactory if plugin_type == MCP_PLUGIN_TYPE else None for class_name, cls in discovered_plugins.items(): + if resolve_action_type({"type": cls.__name__}) == MCP_PLUGIN_TYPE and plugin_type != MCP_PLUGIN_TYPE: + continue normalized_class = normalize(class_name) if normalized_type == normalized_class or normalized_type in normalized_class: matched_class = cls @@ -2404,8 +2517,11 @@ def normalize(s): plugin = TableauPluginFactory.create_from_config(manifest) elif plugin_type == YAMCS_PLUGIN_TYPE: plugin = YamcsPluginFactory.create_from_config(manifest) - elif plugin_type == MCP_PLUGIN_TYPE or normalized_type == normalize(MCP_PLUGIN_TYPE): - plugin = McpPluginFactory.create_from_config(manifest) + elif plugin_type == MCP_PLUGIN_TYPE: + origin = get_action_origin(manifest) + if origin is None: + raise McpDestinationPolicyError("MCP execution requires a trusted action origin.") + plugin = McpPluginFactory.create_from_config(manifest, origin=origin) else: # Standard plugin instantiation with health checking and robust error handling plugin_instance, instantiation_errors = PluginHealthChecker.create_plugin_safely( diff --git a/application/single_app/semantic_kernel_plugins/logged_plugin_loader.py b/application/single_app/semantic_kernel_plugins/logged_plugin_loader.py index 114ebf272..0c7d0186c 100644 --- a/application/single_app/semantic_kernel_plugins/logged_plugin_loader.py +++ b/application/single_app/semantic_kernel_plugins/logged_plugin_loader.py @@ -16,6 +16,13 @@ from semantic_kernel_plugins.plugin_invocation_logger import get_plugin_logger, plugin_function_logger, auto_wrap_plugin_functions from semantic_kernel_plugins.plugin_loader import discover_plugins from functions_appinsights import log_event +from functions_action_manifest import ( + McpConfigurationError, + copy_action_manifest, + get_action_execution_status, + get_action_origin, + resolve_action_type, +) from functions_debug import debug_print from functions_databricks_operations import DATABRICKS_LEGACY_TABLE_PLUGIN_TYPE, DATABRICKS_PLUGIN_TYPE from functions_mcp_operations import MCP_PLUGIN_TYPE @@ -53,7 +60,15 @@ def load_plugin_from_manifest(self, manifest: Dict[str, Any], bool: True if plugin loaded successfully, False otherwise """ plugin_name = manifest.get('name') - plugin_type = manifest.get('type') + plugin_type = resolve_action_type(manifest) + execution_status = get_action_execution_status(manifest) + if execution_status: + log_event( + "[LOGGED_PLUGIN_LOADER] Skipping unavailable MCP action", + extra={"plugin_name": plugin_name, **execution_status}, + level=logging.WARNING, + ) + return False # Debug logging log_event(f"[LOGGED_PLUGIN_LOADER] Starting to load plugin: {plugin_name} (type: {plugin_type})") @@ -115,6 +130,8 @@ def load_plugin_from_manifest(self, manifest: Dict[str, Any], def _create_plugin_instance(self, manifest: Dict[str, Any]): """Create a plugin instance from manifest.""" + manifest = copy_action_manifest(manifest) + manifest["type"] = resolve_action_type(manifest) plugin_name = manifest.get('name') plugin_type = manifest.get('type') @@ -150,8 +167,10 @@ def normalize(s): debug_print(f"[LOGGED_PLUGIN_LOADER] Normalized plugin type: {normalized_type}") matched_class = None for class_name, cls in discovered_plugins.items(): + if resolve_action_type({"type": cls.__name__}) == MCP_PLUGIN_TYPE: + continue normalized_class = normalize(class_name) - print("[LOGGED_PLUGIN_LOADER] Checking plugin class:", class_name, "normalized:", normalized_class) + debug_print(f"[LOGGED_PLUGIN_LOADER] Checking plugin class {class_name}") if normalized_type == normalized_class or normalized_type in normalized_class: matched_class = cls debug_print(f"[LOGGED_PLUGIN_LOADER] Matched class for plugin '{name}' of type '{plugin_type}': {matched_class}") @@ -292,7 +311,10 @@ def _create_mcp_plugin(self, manifest: Dict[str, Any]): """Create an MCP plugin instance.""" plugin_name = manifest.get('name') try: - plugin_instance = McpPluginFactory.create_from_config(manifest) + origin = get_action_origin(manifest) + if origin is None: + raise PermissionError("MCP execution requires a trusted action origin.") + plugin_instance = McpPluginFactory.create_from_config(manifest, origin=origin) log_event( "[LOGGED_PLUGIN_LOADER] Successfully created MCP plugin instance using factory", extra={"plugin_name": plugin_name}, @@ -301,12 +323,13 @@ def _create_mcp_plugin(self, manifest: Dict[str, Any]): return plugin_instance except Exception as e: log_event( - "[LOGGED_PLUGIN_LOADER] General error creating MCP plugin", - extra={"plugin_name": plugin_name, "error": str(e)}, + "[LOGGED_PLUGIN_LOADER] MCP action could not be loaded", + extra={ + "plugin_name": plugin_name, + "error_type": getattr(e, "code", "authorization" if isinstance(e, PermissionError) else "validation"), + }, level=logging.ERROR, - exceptionTraceback=True, ) - self.logger.error(f"Failed to create MCP plugin: {e}") return None def _create_python_plugin(self, manifest: Dict[str, Any]): @@ -321,6 +344,8 @@ def _create_python_plugin(self, manifest: Dict[str, Any]): try: module = importlib.import_module(f"semantic_kernel_plugins.{module_name}") plugin_class = getattr(module, class_name) + if resolve_action_type({"type": plugin_class.__name__}) == MCP_PLUGIN_TYPE: + raise McpConfigurationError("MCP actions must use the MCP action type.") return plugin_class(manifest) except (ImportError, AttributeError) as e: self.logger.error(f"Failed to create Python plugin {class_name} from {module_name}: {e}") diff --git a/application/single_app/semantic_kernel_plugins/mcp_plugin.py b/application/single_app/semantic_kernel_plugins/mcp_plugin.py index 52f64efaf..be6143e8c 100644 --- a/application/single_app/semantic_kernel_plugins/mcp_plugin.py +++ b/application/single_app/semantic_kernel_plugins/mcp_plugin.py @@ -6,6 +6,15 @@ from semantic_kernel.functions import kernel_function from semantic_kernel.functions.kernel_plugin import KernelPlugin +from functions_action_manifest import ( + McpActionOrigin, + McpConfigurationError, + McpStdioRemovedError, + copy_action_manifest, + get_action_origin, + is_retired_mcp_stdio, + resolve_action_type, +) from functions_debug import debug_print from functions_mcp_operations import ( MCP_PLUGIN_TYPE, @@ -23,16 +32,30 @@ class McpPlugin(BasePlugin): """Model Context Protocol action descriptor.""" - def __init__(self, manifest: Optional[Dict[str, Any]] = None): + def __init__( + self, manifest: Optional[Dict[str, Any]] = None, *, origin: Optional[McpActionOrigin] = None + ): super().__init__(manifest) - self.manifest = manifest or {} + self.manifest = copy_action_manifest(manifest) if manifest is not None else {"type": MCP_PLUGIN_TYPE} + self._origin = origin if origin is not None else get_action_origin(self.manifest) + self._additional_fields = self._validate_manifest() self._metadata = self.manifest.get("metadata", {}) if isinstance(self.manifest.get("metadata"), dict) else {} - self._additional_fields = normalize_mcp_additional_fields(self.manifest.get("additionalFields", {})) + self.manifest["type"] = MCP_PLUGIN_TYPE + self.manifest["additionalFields"] = self._additional_fields self._allowed_tool_names = set(self._additional_fields.get("allowed_tool_names") or []) self._tools = self._filter_tools( normalize_mcp_tool_metadata(self._additional_fields.get("mcp_tools", [])) ) + def _validate_manifest(self): + if not isinstance(self.manifest, dict) or resolve_action_type(self.manifest) != MCP_PLUGIN_TYPE: + raise McpConfigurationError("Only an MCP action can use an MCP connector.") + if is_retired_mcp_stdio(self.manifest): + raise McpStdioRemovedError() + if self._origin is not None and not isinstance(self._origin, McpActionOrigin): + raise PermissionError("MCP execution requires a trusted action origin.") + return normalize_mcp_additional_fields(self.manifest.get("additionalFields", {})) + def _filter_tools(self, tools: List[Dict[str, Any]]) -> List[Dict[str, Any]]: if not self._allowed_tool_names: return tools @@ -135,6 +158,16 @@ def list_configured_tools(self) -> dict: @kernel_function(description="Call an MCP tool by its original MCP tool name with a JSON object of arguments.") async def call_tool(self, tool_name: str, arguments: Optional[dict] = None) -> dict: """Call a configured MCP tool by original name.""" + try: + self._validate_manifest() + except (ValueError, PermissionError) as exc: + error_info = classify_mcp_exception(exc, "tool_call") + return { + "success": False, + "error": error_info["message"], + "error_type": error_info["category"], + "retryable": False, + } normalized_tool_name = str(tool_name or "").strip() if not normalized_tool_name: return { @@ -187,46 +220,32 @@ async def invoke_tool(self, tool_name: str, arguments: Optional[dict] = None) -> f"endpoint_present={bool(str(self.manifest.get('endpoint') or '').strip())} " f"argument_keys={sorted(arguments.keys()) if isinstance(arguments, dict) else []}" ) + # The factory constructs this descriptor; defer the reverse runtime dependency. from semantic_kernel_plugins.mcp_plugin_factory import McpPluginFactory result = await McpPluginFactory.call_tool_from_config( self.manifest, tool_name, arguments or {}, + origin=self._origin, ) debug_print( f"[MCP_PLUGIN] MCP tool completed tool_name={tool_name} " f"success={result.get('success') if isinstance(result, dict) else ''}" ) return result - except ValueError as exc: - debug_print(f"[MCP_PLUGIN] MCP tool validation failed tool_name={tool_name} message={exc}") - return { - "success": False, - "error": str(exc), - "error_type": "validation", - } - except McpRuntimeError as exc: - debug_print( - f"[MCP_PLUGIN] MCP tool call failed tool_name={tool_name} " - f"category={exc.category} operation={exc.operation}" - ) - return { - "success": False, - "error": str(exc), - "error_type": exc.category, - "operation": exc.operation, - "details": exc.detail, - } except Exception as exc: error_info = classify_mcp_exception(exc, "tool_call") + category = exc.category if isinstance(exc, McpRuntimeError) else error_info["category"] debug_print( f"[MCP_PLUGIN] MCP tool call failed tool_name={tool_name} " f"exception_type={type(exc).__name__} category={error_info['category']}" ) return { "success": False, - "error": f"Failed to call MCP tool '{tool_name}'. {error_info['message']}", - "error_type": error_info["category"], - "details": error_info["detail"], + "error": error_info["message"], + "error_type": category, + "operation": "tool_call", + "details": error_info["message"], + "retryable": False, } \ No newline at end of file diff --git a/application/single_app/semantic_kernel_plugins/mcp_plugin_factory.py b/application/single_app/semantic_kernel_plugins/mcp_plugin_factory.py index 00a74db0b..eb826a114 100644 --- a/application/single_app/semantic_kernel_plugins/mcp_plugin_factory.py +++ b/application/single_app/semantic_kernel_plugins/mcp_plugin_factory.py @@ -9,12 +9,19 @@ from semantic_kernel.connectors.mcp import ( MCPSsePlugin, - MCPStdioPlugin, MCPStreamableHttpPlugin, MCPWebsocketPlugin, ) from functions_appinsights import log_event +from functions_action_manifest import ( + McpActionOrigin, + McpConfigurationError, + McpStdioRemovedError, + copy_action_manifest, + is_retired_mcp_stdio, + resolve_action_type, +) from functions_debug import debug_print from functions_mcp_operations import ( MCP_CUSTOM_HEADERS_FIELD, @@ -31,17 +38,42 @@ validate_mcp_endpoint_for_transport, ) from functions_mcp_destinations import ( - assert_mcp_destination_allowed, + McpDestinationPolicyError, build_mcp_destination_log_context, - infer_mcp_destination_scope, + resolve_mcp_execution_context, ) -from functions_mcp_preconfigurations import assert_mcp_preconfiguration_manifest_allowed +from functions_mcp_preconfigurations import authorize_mcp_action from semantic_kernel_plugins.mcp_plugin import McpPlugin class McpPluginFactory: """Factory for MCP plugin instances from stored action manifests.""" + @classmethod + def _normalize_manifest(cls, config: Dict[str, Any]) -> Dict[str, Any]: + """Validate the MCP boundary before policy, credentials, or connectors.""" + if not isinstance(config, dict) or resolve_action_type(config) != MCP_PLUGIN_TYPE: + raise McpConfigurationError("Only an MCP action can use an MCP connector.") + if is_retired_mcp_stdio(config): + raise McpStdioRemovedError() + manifest = copy_action_manifest(config) + manifest["type"] = MCP_PLUGIN_TYPE + manifest["additionalFields"] = normalize_mcp_additional_fields(manifest.get("additionalFields", {})) + return manifest + + @staticmethod + def _get_current_settings(): + # Descriptors are imported during discovery; only execution may access the settings owner. + from functions_settings import get_settings + + try: + settings = get_settings() + except Exception as exc: + raise McpDestinationPolicyError("MCP destination policy is currently unavailable.") from exc + if not isinstance(settings, dict): + raise McpDestinationPolicyError("MCP destination policy is currently unavailable.") + return settings + @classmethod def _build_operation_log_context( cls, @@ -51,7 +83,7 @@ def _build_operation_log_context( tool_name: str = "", ) -> Dict[str, Any]: """Build low-sensitivity structured telemetry for an outbound MCP operation.""" - manifest = dict(config or {}) + manifest = copy_action_manifest(config or {}) additional_fields = normalize_mcp_additional_fields(manifest.get("additionalFields", {})) auth = manifest.get("auth") if isinstance(manifest.get("auth"), dict) else {} context = { @@ -91,36 +123,41 @@ def _log_connector_event( log_event(message, extra=context, level=level, debug_only=debug_only) @classmethod - def create_from_config(cls, config: Dict[str, Any]) -> McpPlugin: + def create_from_config(cls, config: Dict[str, Any], *, origin: Optional[McpActionOrigin] = None) -> McpPlugin: """Create an MCP plugin from an action manifest.""" - manifest = dict(config or {}) - manifest["additionalFields"] = normalize_mcp_additional_fields(manifest.get("additionalFields", {})) - return McpPlugin(manifest) + manifest = cls._normalize_manifest(config) + return McpPlugin(manifest, origin=origin) @classmethod - async def discover_tools_from_config(cls, config: Dict[str, Any]) -> List[Dict[str, Any]]: + async def discover_tools_from_config( + cls, config: Dict[str, Any], *, origin: Optional[McpActionOrigin] = None + ) -> List[Dict[str, Any]]: """Connect to an MCP server and return normalized tool metadata.""" + manifest = cls._normalize_manifest(config) return await cls._run_with_retries( - config, + manifest, "tool_discovery", - lambda: cls._discover_tools_once(config), + lambda: cls._discover_tools_once(manifest, origin=origin), ) @classmethod - async def probe_server_from_config(cls, config: Dict[str, Any]) -> Dict[str, Any]: + async def probe_server_from_config( + cls, config: Dict[str, Any], *, origin: Optional[McpActionOrigin] = None + ) -> Dict[str, Any]: """Connect to an MCP server and return tool metadata plus compatibility hints.""" + manifest = cls._normalize_manifest(config) return await cls._run_with_retries( - config, + manifest, "capability_probe", - lambda: cls._probe_server_once(config), + lambda: cls._probe_server_once(manifest, origin=origin), ) @classmethod - async def _probe_server_once(cls, config: Dict[str, Any]) -> Dict[str, Any]: + async def _probe_server_once(cls, config: Dict[str, Any], *, origin=None) -> Dict[str, Any]: """Perform one MCP compatibility probe attempt.""" - manifest = dict(config or {}) + manifest = cls._normalize_manifest(config) additional_fields = normalize_mcp_additional_fields(manifest.get("additionalFields", {})) - connector = cls.create_connector(manifest) + connector = cls.create_connector(manifest, origin=origin) started_at = time.perf_counter() try: cls._log_connector_event( @@ -176,9 +213,9 @@ async def _probe_server_once(cls, config: Dict[str, Any]) -> Dict[str, Any]: await connector.close() @classmethod - async def _discover_tools_once(cls, config: Dict[str, Any]) -> List[Dict[str, Any]]: + async def _discover_tools_once(cls, config: Dict[str, Any], *, origin=None) -> List[Dict[str, Any]]: """Perform one MCP tool discovery attempt.""" - connector = cls.create_connector(config) + connector = cls.create_connector(config, origin=origin) started_at = time.perf_counter() try: cls._log_connector_event( @@ -248,18 +285,21 @@ async def call_tool_from_config( config: Dict[str, Any], tool_name: str, arguments: Optional[Dict[str, Any]] = None, + *, + origin: Optional[McpActionOrigin] = None, ) -> Dict[str, Any]: """Connect to an MCP server, invoke one tool, and normalize the result.""" - configured_tool = cls._find_configured_tool(config, tool_name) + manifest = cls._normalize_manifest(config) + configured_tool = cls._find_configured_tool(manifest, tool_name) normalized_arguments = ( normalize_mcp_tool_call_arguments(configured_tool, arguments) if configured_tool else arguments ) return await cls._run_with_retries( - config, + manifest, "tool_call", - lambda: cls._call_tool_once(config, tool_name, normalized_arguments), + lambda: cls._call_tool_once(manifest, tool_name, normalized_arguments, origin=origin), ) @classmethod @@ -287,14 +327,17 @@ async def _call_tool_once( config: Dict[str, Any], tool_name: str, arguments: Optional[Dict[str, Any]] = None, + *, + origin=None, ) -> Dict[str, Any]: """Perform one MCP tool call attempt.""" - connector = cls.create_connector(config) + connector = cls.create_connector(config, origin=origin) try: debug_print(f"[MCP_PLUGIN_FACTORY] Connecting to MCP server for tool call tool_name={tool_name}.") await connector.connect() raw_result = await connector.call_tool(tool_name, **(arguments or {})) - additional_fields = normalize_mcp_additional_fields((config or {}).get("additionalFields", {})) + manifest = cls._normalize_manifest(config) + additional_fields = manifest["additionalFields"] result = cls._serialize_tool_result( tool_name, raw_result, @@ -312,7 +355,8 @@ async def _call_tool_once( @classmethod async def _run_with_retries(cls, config: Dict[str, Any], operation: str, operation_factory): """Run an MCP operation with bounded retries and classified failures.""" - additional_fields = normalize_mcp_additional_fields((config or {}).get("additionalFields", {})) + manifest = cls._normalize_manifest(config) + additional_fields = manifest["additionalFields"] retry_count = int(additional_fields.get("retry_count") or 0) retry_backoff_seconds = int(additional_fields.get("retry_backoff_seconds") or 1) @@ -322,14 +366,10 @@ async def _run_with_retries(cls, config: Dict[str, Any], operation: str, operati return await operation_factory() except McpRuntimeError: raise + except (McpConfigurationError, PermissionError): + raise except Exception as exc: error_info = classify_mcp_exception(exc, operation) - if isinstance(exc, ValueError) and error_info["category"] == "unknown": - error_info.update({ - "category": "validation", - "message": error_info["detail"] or "MCP configuration is invalid.", - "retryable": False, - }) if error_info["retryable"] and attempt < retry_count: delay = retry_backoff_seconds * (2 ** attempt) @@ -360,16 +400,15 @@ async def _run_with_retries(cls, config: Dict[str, Any], operation: str, operati error_info["message"], category=error_info["category"], operation=operation, - detail=error_info["detail"], + detail=error_info["message"], retryable=error_info["retryable"], ) from exc @classmethod - def create_connector(cls, config: Dict[str, Any]): + def create_connector(cls, config: Dict[str, Any], *, origin: Optional[McpActionOrigin] = None): """Create the native Semantic Kernel MCP connector for a manifest.""" - manifest = dict(config or {}) - additional_fields = normalize_mcp_additional_fields(manifest.get("additionalFields", {})) - manifest["additionalFields"] = additional_fields + manifest = cls._normalize_manifest(config) + additional_fields = manifest["additionalFields"] transport = additional_fields.get("transport") name = str(manifest.get("name") or MCP_PLUGIN_TYPE).strip() or MCP_PLUGIN_TYPE description = str(manifest.get("description") or "Model Context Protocol action").strip() @@ -377,48 +416,13 @@ def create_connector(cls, config: Dict[str, Any]): load_tools = bool(additional_fields.get("load_tools", True)) load_prompts = bool(additional_fields.get("load_prompts", False)) - inferred_scope_type, inferred_scope_id = infer_mcp_destination_scope(manifest) - mcp_operation_id = str(manifest.get("mcp_operation_id") or "").strip() - assert_mcp_destination_allowed( + action_origin, _ = resolve_mcp_execution_context(manifest, origin=origin) + authorize_mcp_action( manifest, - scope_type=inferred_scope_type, - scope_id=inferred_scope_id, + origin=action_origin, + settings=cls._get_current_settings(), operation="mcp_runtime_connector", - user_id=manifest.get("runtime_user_id") or manifest.get("user_id") or "", - mcp_operation_id=mcp_operation_id, ) - assert_mcp_preconfiguration_manifest_allowed( - manifest, - scope_type=inferred_scope_type, - scope_id=inferred_scope_id, - operation="mcp_runtime_connector", - user_id=manifest.get("runtime_user_id") or manifest.get("user_id") or "", - ) - - if transport == "stdio": - command = str(additional_fields.get("command") or "").strip() - if not command: - raise ValueError("MCP stdio transport requires a command.") - log_event( - "[MCP_OUTBOUND] Creating MCP stdio connector", - extra={ - **cls._build_operation_log_context(manifest, "create_connector"), - "command_present": bool(command), - "args_count": len(list(additional_fields.get("args") or [])), - }, - level=logging.INFO, - debug_only=True, - ) - return MCPStdioPlugin( - name=name, - command=command, - args=list(additional_fields.get("args") or []), - env=dict(additional_fields.get("env") or {}), - load_tools=load_tools, - load_prompts=load_prompts, - request_timeout=request_timeout, - description=description, - ) endpoint = str(manifest.get("endpoint") or "").strip() endpoint_errors = validate_mcp_endpoint_for_transport(endpoint, transport) diff --git a/application/single_app/semantic_kernel_plugins/plugin_health_checker.py b/application/single_app/semantic_kernel_plugins/plugin_health_checker.py index 80f20a928..15ed1d0b5 100644 --- a/application/single_app/semantic_kernel_plugins/plugin_health_checker.py +++ b/application/single_app/semantic_kernel_plugins/plugin_health_checker.py @@ -10,6 +10,12 @@ from urllib.parse import urlparse from semantic_kernel_plugins.base_plugin import BasePlugin from functions_appinsights import log_event +from functions_action_manifest import ( + MCP_STDIO_REMOVED_MESSAGE, + McpConfigurationError, + is_retired_mcp_stdio, + resolve_action_type, +) from functions_azure_endpoint_validation import ( validate_azure_blob_endpoint, validate_azure_cosmos_endpoint, @@ -137,6 +143,15 @@ def validate_plugin_manifest(manifest: Dict[str, Any], plugin_type: str) -> Tupl if not isinstance(manifest, dict): errors.append("Manifest must be a dictionary") return False, errors + try: + plugin_type = resolve_action_type(manifest) + except ValueError: + return False, ["Action type must be a string."] + manifest = manifest.copy() + if plugin_type: + manifest['type'] = plugin_type + if is_retired_mcp_stdio(manifest): + return False, [MCP_STDIO_REMOVED_MESSAGE] # Required fields required_fields = ['name', 'type'] @@ -468,7 +483,10 @@ def validate_plugin_manifest(manifest: Dict[str, Any], plugin_type: str) -> Tupl errors.append("SimpleChat plugin requires auth.type='user'") elif plugin_type == MCP_PLUGIN_TYPE: - additional_fields = normalize_mcp_additional_fields(manifest.get('additionalFields', {})) + try: + additional_fields = normalize_mcp_additional_fields(manifest.get('additionalFields', {})) + except McpConfigurationError as exc: + return False, [exc.public_message] transport = additional_fields.get('transport') endpoint = str(manifest.get('endpoint') or '').strip() auth = manifest.get('auth', {}) if isinstance(manifest.get('auth'), dict) else {} @@ -476,14 +494,10 @@ def validate_plugin_manifest(manifest: Dict[str, Any], plugin_type: str) -> Tupl auth_method = normalize_mcp_auth_method(additional_fields.get('auth_method')) if transport not in MCP_SUPPORTED_TRANSPORTS: - errors.append("MCP plugin requires additionalFields.transport to be streamable_http, sse, websocket, or stdio") + errors.append("MCP plugin requires additionalFields.transport to be streamable_http, sse, or websocket") if transport in MCP_REMOTE_TRANSPORTS: errors.extend(validate_mcp_endpoint_for_transport(endpoint, transport)) - elif transport == 'stdio': - command = str(additional_fields.get('command') or '').strip() - if not command: - errors.append("MCP stdio transport requires additionalFields.command") if auth_type not in {'NoAuth', 'key', 'identity'}: errors.append("MCP plugin supports auth.type values 'NoAuth', 'key', or 'identity'") @@ -720,7 +734,25 @@ def create_plugin_safely(plugin_class, manifest: Dict[str, Any], plugin_name: st """ errors = [] plugin_instance = None - + if resolve_action_type({'type': plugin_class.__name__}) == MCP_PLUGIN_TYPE: + if resolve_action_type(manifest) != MCP_PLUGIN_TYPE: + return None, ["MCP action type is invalid."] + valid, errors = PluginHealthChecker.validate_plugin_manifest(manifest, MCP_PLUGIN_TYPE) + if not valid: + return None, errors + try: + plugin_instance = plugin_class(manifest) + except (ValueError, TypeError, KeyError, PermissionError) as exc: + log_event( + "[PLUGIN_CREATION] MCP descriptor creation rejected", + extra={"error_type": type(exc).__name__}, + level=logging.WARNING, + ) + return None, ["MCP action configuration is invalid."] + health_report = PluginHealthChecker.check_plugin_health(plugin_instance, plugin_name) + PluginHealthChecker.log_plugin_health(health_report) + return plugin_instance, health_report.get('errors', []) + try: # Try manifest-based instantiation first try: diff --git a/application/single_app/static/js/plugin_common.js b/application/single_app/static/js/plugin_common.js index 4d17dd7b6..76b37db3e 100644 --- a/application/single_app/static/js/plugin_common.js +++ b/application/single_app/static/js/plugin_common.js @@ -4,7 +4,7 @@ import { showToast } from "./chat/chat-toast.js" import { humanizeName, truncateDescription, - openViewModal, createActionCard + openViewModal, createActionCard, getMcpRetirementStatus } from './workspace/view-utils.js'; // Fetch merged plugin settings from backend given type and current settings @@ -64,7 +64,7 @@ export function escapeHtml(str) { } // Render plugins table (parameterized for tbody selector and button handlers) -export function renderPluginsTable({plugins, tbodySelector, onEdit, onDelete, onView, onToggleEnabled, onGovern, onDuplicate, ensureTable = true, isAdmin = false}) { +export function renderPluginsTable({plugins, tbodySelector, onEdit, onDelete, onView, onToggleEnabled, onGovern, onDuplicate, ensureTable = true, isAdmin = false, getPluginKey = plugin => plugin.name || ''}) { // Optionally ensure the table is present before rendering if (ensureTable) { ensurePluginsTableInRoot(); @@ -95,10 +95,11 @@ export function renderPluginsTable({plugins, tbodySelector, onEdit, onDelete, on plugins.forEach(plugin => { const tr = document.createElement('tr'); - const pluginName = plugin.name || ''; + const pluginName = getPluginKey(plugin); const displayName = humanizeName(plugin.display_name || plugin.name); const description = plugin.description || 'No description available'; const isEnabled = plugin.is_enabled !== false; + const retirementStatus = getMcpRetirementStatus(plugin); const truncatedDesc = truncateDescription(description, 90); const nameCell = document.createElement('td'); @@ -115,16 +116,22 @@ export function renderPluginsTable({plugins, tbodySelector, onEdit, onDelete, on } nameCell.appendChild(document.createTextNode(' ')); const statusBadge = document.createElement('span'); - statusBadge.className = isEnabled - ? 'badge bg-success-subtle text-success-emphasis border border-success-subtle' - : 'badge bg-secondary'; - statusBadge.textContent = isEnabled ? 'Enabled' : 'Disabled'; + statusBadge.className = retirementStatus + ? 'badge bg-warning text-dark' + : (isEnabled ? 'badge bg-success-subtle text-success-emphasis border border-success-subtle' : 'badge bg-secondary'); + statusBadge.textContent = retirementStatus ? 'Unsupported' : (isEnabled ? 'Enabled' : 'Disabled'); nameCell.appendChild(statusBadge); const descriptionCell = document.createElement('td'); descriptionCell.className = 'text-muted small'; descriptionCell.title = description; descriptionCell.textContent = truncatedDesc; + if (retirementStatus) { + const notice = document.createElement('div'); + notice.className = 'mt-1 text-warning-emphasis mcp-retirement-notice'; + notice.textContent = retirementStatus.message; + descriptionCell.appendChild(notice); + } const actionsCell = document.createElement('td'); const actionButtons = document.createElement('div'); @@ -133,11 +140,13 @@ export function renderPluginsTable({plugins, tbodySelector, onEdit, onDelete, on let editDeleteButtons = ''; if (isAdmin || !plugin.is_global) { - actionButtons.appendChild(createActionButton('btn btn-sm btn-outline-secondary edit-plugin-btn', 'Edit action', 'bi bi-pencil', onEdit, pluginName)); + actionButtons.appendChild(createActionButton('btn btn-sm btn-outline-secondary edit-plugin-btn', retirementStatus ? 'Reconfigure action' : 'Edit action', 'bi bi-pencil', onEdit, pluginName)); if (isAdmin) { actionButtons.appendChild(createActionButton('btn btn-sm btn-outline-info govern-plugin-btn', 'Govern action', 'bi bi-shield-check', onGovern, pluginName)); - actionButtons.appendChild(createActionButton('btn btn-sm btn-outline-secondary duplicate-plugin-btn', 'Duplicate action', 'bi bi-files', onDuplicate, pluginName)); - if (onToggleEnabled) { + if (!retirementStatus) { + actionButtons.appendChild(createActionButton('btn btn-sm btn-outline-secondary duplicate-plugin-btn', 'Duplicate action', 'bi bi-files', onDuplicate, pluginName)); + } + if (onToggleEnabled && !retirementStatus) { actionButtons.appendChild(createActionButton( `btn btn-sm ${isEnabled ? 'btn-outline-warning' : 'btn-outline-success'} toggle-plugin-btn`, isEnabled ? 'Disable action' : 'Enable action', @@ -159,7 +168,7 @@ export function renderPluginsTable({plugins, tbodySelector, onEdit, onDelete, on } // Render plugins grid (card-based view) -export function renderPluginsGrid({plugins, containerSelector, onEdit, onDelete, onView, isAdmin = false}) { +export function renderPluginsGrid({plugins, containerSelector, onEdit, onDelete, onView, isAdmin = false, getPluginKey = plugin => plugin.name || ''}) { const container = document.querySelector(containerSelector); if (!container) return; container.innerHTML = ''; @@ -169,9 +178,9 @@ export function renderPluginsGrid({plugins, containerSelector, onEdit, onDelete, } plugins.forEach(plugin => { const card = createActionCard(plugin, { - onView: (p) => { if (onView) onView(p.name); }, - onEdit: (p) => onEdit(p.name), - onDelete: (p) => onDelete(p.name), + onView: (p) => { if (onView) onView(getPluginKey(p)); }, + onEdit: (p) => onEdit(getPluginKey(p)), + onDelete: (p) => onDelete(getPluginKey(p)), canManage: isAdmin || !plugin.is_global, isAdmin }); diff --git a/application/single_app/static/js/plugin_modal_stepper.js b/application/single_app/static/js/plugin_modal_stepper.js index 0743a79b5..7d1ebd0e9 100644 --- a/application/single_app/static/js/plugin_modal_stepper.js +++ b/application/single_app/static/js/plugin_modal_stepper.js @@ -1,7 +1,7 @@ // plugin_modal_stepper.js // Multi-step modal functionality for action/plugin creation import { showToast } from "./chat/chat-toast.js"; -import { getTypeIcon } from "./workspace/view-utils.js"; +import { getTypeIcon, getMcpRetirementStatus, isMcpActionType } from "./workspace/view-utils.js"; // Action types hidden from the creation UI (backend plugins remain intact) const HIDDEN_ACTION_TYPES = ['sql_schema', 'ui_test', 'queue_storage', 'embedding_model', 'databricks_table']; @@ -111,7 +111,7 @@ const MSGRAPH_DEFAULT_CALENDAR_DELAY_SECONDS = 60; const MSGRAPH_MIN_CALENDAR_DELAY_SECONDS = 5; const MSGRAPH_MAX_CALENDAR_DELAY_SECONDS = 600; const MCP_DEFAULT_SERVER_PROFILE = 'generic'; -const MCP_STDIO_ENDPOINT = 'stdio://local'; +const MCP_REMOTE_TRANSPORTS = new Set(['streamable_http', 'sse', 'websocket']); const MCP_FALLBACK_SERVER_PRESETS = [ { id: MCP_DEFAULT_SERVER_PROFILE, @@ -138,10 +138,9 @@ const MCP_FALLBACK_SERVER_PRESETS = [ websocketEndpointPlaceholder: 'wss://example.com/mcp' }, constraints: { - allowedTransports: ['streamable_http', 'sse', 'websocket', 'stdio'], + allowedTransports: ['streamable_http', 'sse', 'websocket'], allowedAuthMethods: ['none', 'bearer', 'api_key', 'basic', 'identity'], - customHeadersAllowed: true, - stdioAllowed: true + customHeadersAllowed: true }, warnings: [] } @@ -387,6 +386,7 @@ export class PluginModalStepper { this.mcpServerPreconfigurations = MCP_FALLBACK_SERVER_PRECONFIGURATIONS; this.mcpServerPreconfigurationMap = {}; this.mcpServerPreconfigurationsLoaded = false; + this.mcpConfigurationInitialized = false; this._loadPluginSchema().then(() => { // Load schema on initialization this._populateGenericAuthTypeDropdown(); // Dynamically populate generic auth type dropdown after schema loads (will be called again after schema loads) @@ -985,10 +985,24 @@ export class PluginModalStepper { async showModal(plugin = null) { this.isEditMode = !!plugin; - this.selectedType = plugin?.type || null; + const declaredType = plugin?.type; + const effectiveType = declaredType == null || (typeof declaredType === 'string' && !declaredType.trim()) + ? plugin?.metadata?.type + : declaredType; + this.selectedType = isMcpActionType(effectiveType) ? MCP_PLUGIN_TYPE : (effectiveType || null); + this.mcpConfigurationInitialized = false; // Store original plugin state for change tracking this.originalPlugin = plugin ? JSON.parse(JSON.stringify(plugin)) : null; + const retirementStatus = getMcpRetirementStatus(plugin); + const retirementNotice = document.getElementById('mcp-retirement-notice'); + if (retirementNotice) { + retirementNotice.textContent = retirementStatus?.message || ''; + retirementNotice.classList.toggle('d-none', !retirementStatus); + } + this.setMcpDiscoveryStatus(''); + this.setMcpDiscoveryWarnings([]); + document.getElementById('mcp-test-connection-result')?.classList.add('d-none'); // Reset modal state this.currentStep = 1; @@ -997,7 +1011,7 @@ export class PluginModalStepper { this.updateNavigationButtons(); // Set modal title - const title = this.isEditMode ? 'Edit Action' : 'Add Action'; + const title = retirementStatus ? 'Reconfigure Action' : (this.isEditMode ? 'Edit Action' : 'Add Action'); document.getElementById('plugin-modal-title').textContent = title; // Clear error messages @@ -1013,6 +1027,9 @@ export class PluginModalStepper { if (this.isEditMode) { this.populateFormFromPlugin(plugin); + if (this.isMcpType()) { + this.showConfigSectionForType(); + } // Skip to step 2 for editing this.goToStep(2); } else { @@ -1381,7 +1398,7 @@ export class PluginModalStepper { } isMcpType(type = this.selectedType) { - return !!(type && type.toLowerCase() === MCP_PLUGIN_TYPE); + return isMcpActionType(type); } isSimpleChatType(type = this.selectedType) { @@ -2868,10 +2885,31 @@ export class PluginModalStepper { const labels = { streamable_http: 'Streamable HTTP', sse: 'Server-Sent Events', - websocket: 'WebSocket', - stdio: 'Stdio' + websocket: 'WebSocket' + }; + return labels[transport] || 'Select a remote transport'; + } + + normalizeMcpTransportForForm(value) { + if (value == null || (typeof value === 'string' && !value.trim())) { + return 'streamable_http'; + } + if (typeof value !== 'string') { + return ''; + } + const normalized = value.trim().toLowerCase().replace(/-/g, '_'); + const aliases = { + http: 'streamable_http', + streamablehttp: 'streamable_http', + streamable_http: 'streamable_http', + sse: 'sse', + server_sent_events: 'sse', + eventsource: 'sse', + ws: 'websocket', + wss: 'websocket', + websocket: 'websocket' }; - return labels[transport] || transport || '-'; + return Object.prototype.hasOwnProperty.call(aliases, normalized) ? aliases[normalized] : ''; } async loadMcpServerPresets() { @@ -3096,7 +3134,9 @@ export class PluginModalStepper { setIfBlank('plugin-description', preconfiguration.description); setFieldValue('mcp-server-profile', preconfiguration.presetId || MCP_DEFAULT_SERVER_PROFILE); this.applyMcpServerProfile({ applyDefaults: true }); - setFieldValue('mcp-transport', preconfiguration.transport); + if (!getMcpRetirementStatus(this.originalPlugin)) { + setFieldValue('mcp-transport', preconfiguration.transport); + } setFieldValue('mcp-endpoint', preconfiguration.endpoint); setFieldValue('mcp-auth-method', defaults.auth_method); setFieldValue('mcp-api-key-header-name', defaults.api_key_header_name); @@ -3138,7 +3178,9 @@ export class PluginModalStepper { } }; - setValue('mcp-transport', 'transport', 'streamable_http'); + if (!getMcpRetirementStatus(this.originalPlugin)) { + setValue('mcp-transport', 'transport', 'streamable_http'); + } if (!this.getSelectedActionIdentity('mcp')) { setValue('mcp-auth-method', 'auth_method', 'none'); } @@ -3261,10 +3303,15 @@ export class PluginModalStepper { } initializeMcpConfiguration() { + if (this.mcpConfigurationInitialized) { + this.toggleMcpTransportFields(); + this.toggleMcpAuthFields(); + return; + } const defaults = { 'mcp-preconfiguration': '', 'mcp-server-profile': this.mcpDefaultServerPreset || MCP_DEFAULT_SERVER_PROFILE, - 'mcp-transport': 'streamable_http', + 'mcp-transport': this.isEditMode ? '' : 'streamable_http', 'mcp-auth-method': 'none', 'mcp-api-key-header-name': 'X-API-Key', 'mcp-custom-headers': '{}', @@ -3274,8 +3321,7 @@ export class PluginModalStepper { 'mcp-retry-count': '0', 'mcp-retry-backoff-seconds': '1', 'mcp-tool-result-policy': 'truncate', - 'mcp-tool-metadata': '[]', - 'mcp-env': '{}' + 'mcp-tool-metadata': '[]' }; Object.entries(defaults).forEach(([id, value]) => { @@ -3285,15 +3331,12 @@ export class PluginModalStepper { } }); + this.mcpConfigurationInitialized = true; this.updateMcpTransportOptions(); - this.applyMcpServerProfile(); + this.applyMcpServerProfile({ applyDefaults: !this.isEditMode }); this.toggleMcpAuthFields(); } - isAdminActionScope() { - return window.location.pathname.includes('/admin') || this.actionIdentityScope?.scope === 'global'; - } - updateMcpTransportOptions() { const transportSelect = document.getElementById('mcp-transport'); if (!transportSelect) { @@ -3303,49 +3346,29 @@ export class PluginModalStepper { const preset = this.getSelectedMcpServerPreset(); const constraints = preset?.constraints || {}; const allowedTransports = new Set(constraints.allowedTransports || []); - const allowStdio = this.isAdminActionScope(); Array.from(transportSelect.options).forEach(option => { - let allowed = allowedTransports.size === 0 || allowedTransports.has(option.value); - if (option.value === 'stdio') { - allowed = allowed && allowStdio && constraints.stdioAllowed !== false; - } + const allowed = MCP_REMOTE_TRANSPORTS.has(option.value) + && (allowedTransports.size === 0 || allowedTransports.has(option.value)); option.disabled = !allowed; }); - if (transportSelect.selectedOptions.length && transportSelect.selectedOptions[0].disabled) { - const fallbackTransport = this.getMcpPresetDefault(preset, 'transport', 'streamable_http'); - const fallbackOption = Array.from(transportSelect.options).find(option => option.value === fallbackTransport && !option.disabled) - || Array.from(transportSelect.options).find(option => !option.disabled); - if (fallbackOption) { - transportSelect.value = fallbackOption.value; - } - } - - if (!allowStdio && transportSelect.value === 'stdio') { - const firstRemoteOption = Array.from(transportSelect.options).find(option => option.value !== 'stdio' && !option.disabled); - if (firstRemoteOption) { - transportSelect.value = firstRemoteOption.value; - } - this.setMcpDiscoveryStatus('Stdio transport is only available for admin-managed global actions.', 'warning'); + if (!MCP_REMOTE_TRANSPORTS.has(transportSelect.value) + || transportSelect.selectedOptions[0]?.disabled) { + transportSelect.value = ''; } } toggleMcpTransportFields() { this.updateMcpTransportOptions(); - const transport = document.getElementById('mcp-transport')?.value || 'streamable_http'; + const transport = document.getElementById('mcp-transport')?.value || ''; const endpointGroup = document.getElementById('mcp-endpoint-group'); - const stdioGroup = document.getElementById('mcp-stdio-group'); const endpointInput = document.getElementById('mcp-endpoint'); - const isStdio = transport === 'stdio'; if (endpointGroup) { - endpointGroup.classList.toggle('d-none', isStdio); - } - if (stdioGroup) { - stdioGroup.classList.toggle('d-none', !isStdio); + endpointGroup.classList.remove('d-none'); } - if (!isStdio && endpointInput && !endpointInput.value.trim()) { + if (endpointInput && !endpointInput.value.trim()) { const profile = document.getElementById('mcp-server-profile')?.value || this.mcpDefaultServerPreset || MCP_DEFAULT_SERVER_PROFILE; endpointInput.placeholder = this.getMcpEndpointPlaceholder(transport, profile); } @@ -3382,7 +3405,9 @@ export class PluginModalStepper { populateMcpForm(plugin) { const additionalFields = plugin.additionalFields || plugin.additional_fields || {}; const auth = plugin.auth || {}; - const transport = additionalFields.transport || 'streamable_http'; + const retirementStatus = getMcpRetirementStatus(plugin); + const transport = retirementStatus ? '' : this.normalizeMcpTransportForForm(additionalFields.transport); + this.mcpConfigurationInitialized = true; const storedProfile = additionalFields.server_profile || this.mcpDefaultServerPreset || MCP_DEFAULT_SERVER_PROFILE; const storedPreconfiguration = additionalFields.preconfiguration_id || ''; @@ -3395,10 +3420,7 @@ export class PluginModalStepper { ? storedProfile : (this.mcpDefaultServerPreset || MCP_DEFAULT_SERVER_PROFILE); document.getElementById('mcp-transport').value = transport; - document.getElementById('mcp-endpoint').value = transport === 'stdio' ? '' : (plugin.endpoint || ''); - document.getElementById('mcp-command').value = additionalFields.command || ''; - document.getElementById('mcp-args').value = Array.isArray(additionalFields.args) ? additionalFields.args.join('\n') : ''; - document.getElementById('mcp-env').value = JSON.stringify(additionalFields.env || {}, null, 2); + document.getElementById('mcp-endpoint').value = retirementStatus ? '' : (plugin.endpoint || ''); document.getElementById('mcp-load-tools').checked = additionalFields.load_tools !== false; document.getElementById('mcp-load-prompts').checked = Boolean(additionalFields.load_prompts); document.getElementById('mcp-validate-tool-arguments').checked = Boolean(additionalFields.validate_tool_arguments); @@ -3419,11 +3441,14 @@ export class PluginModalStepper { authMethod = 'bearer'; } document.getElementById('mcp-auth-method').value = authMethod; + ['mcp-bearer-token', 'mcp-api-key-value', 'mcp-basic-username', 'mcp-basic-password'].forEach(id => { + document.getElementById(id).value = ''; + }); + document.getElementById('mcp-api-key-header-name').value = additionalFields.api_key_header_name || 'X-API-Key'; if (authMethod === 'bearer') { document.getElementById('mcp-bearer-token').value = auth.key || ''; } else if (authMethod === 'api_key') { - document.getElementById('mcp-api-key-header-name').value = additionalFields.api_key_header_name || 'X-API-Key'; document.getElementById('mcp-api-key-value').value = auth.key || ''; } else if (authMethod === 'basic') { document.getElementById('mcp-basic-username').value = auth.identity || ''; @@ -3437,8 +3462,36 @@ export class PluginModalStepper { this.toggleMcpAuthFields(); } + validateMcpRemoteConfiguration() { + const transport = document.getElementById('mcp-transport')?.value || ''; + const allowedTransports = this.getSelectedMcpServerPreset()?.constraints?.allowedTransports || []; + if (!MCP_REMOTE_TRANSPORTS.has(transport) + || (allowedTransports.length && !allowedTransports.includes(transport))) { + throw new Error('Select a supported remote MCP transport before saving, testing, or discovering tools.'); + } + const endpoint = document.getElementById('mcp-endpoint')?.value.trim() || ''; + if (!endpoint) { + throw new Error('Endpoint is required for MCP remote transports.'); + } + let parsedEndpoint; + try { + parsedEndpoint = new URL(endpoint); + } catch (error) { + throw new Error('MCP endpoint must be a valid absolute URL.'); + } + const allowedSchemes = transport === 'websocket' ? ['ws:', 'wss:'] : ['http:', 'https:']; + if (!allowedSchemes.includes(parsedEndpoint.protocol) || !parsedEndpoint.host) { + const schemeLabel = allowedSchemes.map(scheme => scheme.replace(':', '')).join('/'); + throw new Error(`MCP ${transport} transport requires a valid ${schemeLabel} endpoint.`); + } + if (parsedEndpoint.username || parsedEndpoint.password) { + throw new Error('MCP endpoint must not include embedded credentials.'); + } + return { transport, endpoint }; + } + getMcpConfiguration() { - const transport = document.getElementById('mcp-transport')?.value || 'streamable_http'; + const { transport, endpoint } = this.validateMcpRemoteConfiguration(); const selectedPreconfigurationId = document.getElementById('mcp-preconfiguration')?.value || ''; const selectedPreconfiguration = this.getMcpServerPreconfiguration(selectedPreconfigurationId); const selectedIdentity = this.getSelectedActionIdentity('mcp'); @@ -3468,14 +3521,6 @@ export class PluginModalStepper { additionalFields.additionalSettings = JSON.parse(JSON.stringify(selectedPreconfiguration.additionalSettings)); } - let endpoint = document.getElementById('mcp-endpoint')?.value.trim() || ''; - if (transport === 'stdio') { - endpoint = MCP_STDIO_ENDPOINT; - additionalFields.command = document.getElementById('mcp-command')?.value.trim() || ''; - additionalFields.args = this.parseTextareaLines('mcp-args'); - additionalFields.env = this.parseJsonObjectField('mcp-env', 'Environment', {}); - } - const auth = {}; let identityId = ''; if (selectedIdentity) { @@ -4350,8 +4395,6 @@ export class PluginModalStepper { } } else if (isMcpVisible) { const transport = document.getElementById('mcp-transport').value; - const endpoint = document.getElementById('mcp-endpoint').value.trim(); - const command = document.getElementById('mcp-command').value.trim(); const authMethod = document.getElementById('mcp-auth-method').value; const selectedIdentity = this.getSelectedActionIdentity('mcp'); const requestTimeout = parseInt(document.getElementById('mcp-request-timeout').value, 10); @@ -4361,42 +4404,11 @@ export class PluginModalStepper { const retryBackoffSeconds = parseInt(document.getElementById('mcp-retry-backoff-seconds').value, 10); let customHeaders = {}; - if (!['streamable_http', 'sse', 'websocket', 'stdio'].includes(transport)) { - this.showError('Select a supported MCP transport.'); - return false; - } - if (transport === 'stdio') { - if (!command) { - this.showError('Command is required for MCP stdio transport.'); - return false; - } - try { - this.parseJsonObjectField('mcp-env', 'Environment', {}); - } catch (error) { + try { + this.validateMcpRemoteConfiguration(); + } catch (error) { this.showError(error.message); return false; - } - } else { - if (!endpoint) { - this.showError('Endpoint is required for MCP remote transports.'); - return false; - } - try { - const parsedEndpoint = new URL(endpoint); - const allowedSchemes = transport === 'websocket' ? ['ws:', 'wss:'] : ['http:', 'https:']; - if (!allowedSchemes.includes(parsedEndpoint.protocol) || !parsedEndpoint.host) { - const schemeLabel = allowedSchemes.map(scheme => scheme.replace(':', '')).join('/'); - this.showError(`MCP ${transport} transport requires a valid ${schemeLabel} endpoint.`); - return false; - } - if (parsedEndpoint.username || parsedEndpoint.password) { - this.showError('MCP endpoint must not include embedded credentials.'); - return false; - } - } catch (error) { - this.showError('MCP endpoint must be a valid absolute URL.'); - return false; - } } if (!document.getElementById('mcp-load-tools').checked && !document.getElementById('mcp-load-prompts').checked) { @@ -5780,12 +5792,7 @@ export class PluginModalStepper { } if (testKey === 'mcp') { - const mcpConfig = this.getMcpConfiguration(); - const transport = mcpConfig.additionalFields?.transport; - if (transport !== 'stdio' && !mcpConfig.endpoint) { - throw new Error('Enter the MCP server endpoint before testing the connection.'); - } - return mcpConfig; + return this.getMcpConfiguration(); } if (testKey === 'snowflake') { @@ -6279,9 +6286,9 @@ export class PluginModalStepper { populateFormFromPlugin(plugin) { // Step 2 fields document.getElementById('plugin-name').value = plugin.name || ''; - document.getElementById('plugin-display-name').value = plugin.displayName || ''; + document.getElementById('plugin-display-name').value = plugin.displayName || plugin.display_name || ''; document.getElementById('plugin-description').value = plugin.description || ''; - document.getElementById('plugin-type').value = plugin.type || ''; + document.getElementById('plugin-type').value = this.selectedType || ''; // Step 3 fields - populate based on plugin type const isOpenApiType = plugin.type && plugin.type.toLowerCase().includes('openapi'); @@ -6499,7 +6506,7 @@ export class PluginModalStepper { this.populateTableauForm(plugin); } else if (this.isYamcsType(plugin.type)) { this.populateYamcsForm(plugin); - } else if (this.isMcpType(plugin.type)) { + } else if (this.isMcpType()) { this.populateMcpForm(plugin); } else if (this.isSimpleChatType(plugin.type)) { const additionalFields = plugin.additionalFields || plugin.additional_fields || {}; @@ -6536,7 +6543,7 @@ export class PluginModalStepper { // Step 4 fields const metadata = plugin.metadata && Object.keys(plugin.metadata).length > 0 ? JSON.stringify(plugin.metadata, null, 2) : '{}'; - const additionalFields = plugin.additionalFields && Object.keys(plugin.additionalFields).length > 0 ? + const additionalFields = !this.isMcpType() && plugin.additionalFields && Object.keys(plugin.additionalFields).length > 0 ? JSON.stringify(plugin.additionalFields, null, 2) : '{}'; document.getElementById('plugin-metadata').value = metadata; @@ -6549,6 +6556,9 @@ export class PluginModalStepper { } getFormData() { + if (this.isMcpType()) { + this.validateMcpRemoteConfiguration(); + } // Determine which configuration section is active const openApiSection = document.getElementById('openapi-config-section'); const sqlSection = document.getElementById('sql-config-section'); @@ -7109,8 +7119,7 @@ export class PluginModalStepper { } else if (this.isYamcsType()) { return this.normalizeYamcsServerUrl(document.getElementById('yamcs-server-url')?.value || ''); } else if (isMcpType) { - const transport = document.getElementById('mcp-transport')?.value || 'streamable_http'; - return transport === 'stdio' ? MCP_STDIO_ENDPOINT : document.getElementById('mcp-endpoint').value.trim(); + return document.getElementById('mcp-endpoint').value.trim(); } else if (isAzureMapsType) { return AZURE_MAPS_DEFAULT_ENDPOINT; } else if (isLogAnalyticsType) { @@ -7529,7 +7538,7 @@ export class PluginModalStepper { return; } - const transport = document.getElementById('mcp-transport')?.value || 'streamable_http'; + const transport = document.getElementById('mcp-transport')?.value || ''; const serverProfile = document.getElementById('mcp-server-profile')?.value || this.mcpDefaultServerPreset || MCP_DEFAULT_SERVER_PROFILE; const preconfigurationId = document.getElementById('mcp-preconfiguration')?.value || ''; const loadTools = Boolean(document.getElementById('mcp-load-tools')?.checked); @@ -7899,7 +7908,11 @@ export class PluginModalStepper { } else if (isYamcsType) { currentAdditionalFields = JSON.stringify(this.getYamcsConfiguration().additionalFields, null, 2); } else if (isMcpType) { - currentAdditionalFields = JSON.stringify(this.getMcpConfiguration().additionalFields, null, 2); + try { + currentAdditionalFields = JSON.stringify(this.getMcpConfiguration().additionalFields, null, 2); + } catch (error) { + return null; + } } else if (isSimpleChatType) { currentAdditionalFields = JSON.stringify({ simplechat_capabilities: this.getSelectedSimpleChatCapabilities() @@ -7982,8 +7995,13 @@ export class PluginModalStepper { // Compare additional fields try { - const originalAdditionalFieldsStr = this.originalPlugin.additionalFields && Object.keys(this.originalPlugin.additionalFields).length > 0 ? - JSON.stringify(this.originalPlugin.additionalFields, null, 2) : '{}'; + const originalFields = this.originalPlugin.additionalFields || this.originalPlugin.additional_fields || {}; + const displayFields = isMcpType + ? Object.fromEntries(Object.entries(originalFields).filter(([key]) => + !['command', 'args', 'env', 'cwd', 'env_file', 'encoding', 'encoding_error_handler'].includes(key))) + : originalFields; + const originalAdditionalFieldsStr = Object.keys(displayFields).length > 0 + ? JSON.stringify(displayFields, null, 2) : '{}'; if (currentAdditionalFields !== originalAdditionalFieldsStr) { changes.additionalFields = { before: originalAdditionalFieldsStr, @@ -8134,6 +8152,7 @@ export class PluginModalStepper { } clearForm() { + this.mcpConfigurationInitialized = false; // Clear all form fields for new action creation // Use safe setting to avoid errors with missing elements @@ -8213,9 +8232,6 @@ export class PluginModalStepper { safeSetValue('mcp-server-profile', this.mcpDefaultServerPreset || MCP_DEFAULT_SERVER_PROFILE); safeSetValue('mcp-transport', 'streamable_http'); safeSetValue('mcp-endpoint'); - safeSetValue('mcp-command'); - safeSetValue('mcp-args'); - safeSetValue('mcp-env', '{}'); safeSetValue('mcp-auth-method', 'none'); safeSetValue('mcp-identity-select'); safeSetValue('mcp-bearer-token'); diff --git a/application/single_app/static/js/workspace/group_plugins.js b/application/single_app/static/js/workspace/group_plugins.js index da6806805..478f564a1 100644 --- a/application/single_app/static/js/workspace/group_plugins.js +++ b/application/single_app/static/js/workspace/group_plugins.js @@ -5,7 +5,7 @@ import { ensurePluginsTableInRoot, validatePluginManifest } from "../plugin_comm import { showToast } from "../chat/chat-toast.js"; import { humanizeName, truncateDescription, escapeHtml as escapeHtmlUtil, - setupViewToggle, switchViewContainers, openViewModal, createActionCard + setupViewToggle, switchViewContainers, openViewModal, createActionCard, getMcpRetirementStatus } from './view-utils.js'; const root = document.getElementById("group-plugins-root"); @@ -187,6 +187,22 @@ function renderPluginsTable(list) { ${escapeHtml(shortDesc)} ${actionsHtml}`; + const retirementStatus = getMcpRetirementStatus(plugin); + if (retirementStatus) { + const badge = document.createElement('span'); + badge.className = 'badge bg-warning text-dark ms-1'; + badge.textContent = 'Unsupported'; + tr.children[0].appendChild(badge); + const notice = document.createElement('div'); + notice.className = 'mt-1 text-warning-emphasis mcp-retirement-notice'; + notice.textContent = retirementStatus.message; + tr.children[1].appendChild(notice); + const editButton = tr.querySelector('.edit-group-plugin-btn'); + if (editButton) { + editButton.title = 'Reconfigure action'; + } + } + tbody.appendChild(tr); }); } diff --git a/application/single_app/static/js/workspace/view-utils.js b/application/single_app/static/js/workspace/view-utils.js index aa62d9224..6f64df131 100644 --- a/application/single_app/static/js/workspace/view-utils.js +++ b/application/single_app/static/js/workspace/view-utils.js @@ -2,6 +2,49 @@ // Shared utilities for list/grid view toggle, name humanization, and view modal // Used by personal and group agents/actions workspace modules +export const MCP_STDIO_REMOVED_MESSAGE = 'This action uses stdio, which is no longer supported. Reconfigure it to use a supported remote MCP server, or delete it.'; + +export function isMcpActionType(type) { + return typeof type === 'string' + && ['mcp', 'mcpplugin', 'modelcontextprotocol', 'modelcontextprotocolplugin'] + .includes(type.replace(/[\s_-]/g, '').toLowerCase()); +} + +export function getMcpRetirementStatus(action) { + if (!action || typeof action !== 'object') { + return null; + } + const serverStatus = action.execution_status; + const fields = action.additionalFields || action.additional_fields || {}; + const declaredType = action.type; + const effectiveType = declaredType == null || (typeof declaredType === 'string' && !declaredType.trim()) + ? action.metadata?.type + : declaredType; + const retiredTransport = typeof fields.transport === 'string' && fields.transport.trim().toLowerCase() === 'stdio'; + const retiredEndpoint = typeof action.endpoint === 'string' && action.endpoint.trim().toLowerCase().startsWith('stdio:'); + if ((serverStatus?.state === 'unsupported' && serverStatus.code === 'mcp_stdio_removed') + || (isMcpActionType(effectiveType) && (retiredTransport || retiredEndpoint))) { + return { + state: 'unsupported', + code: 'mcp_stdio_removed', + message: MCP_STDIO_REMOVED_MESSAGE + }; + } + return null; +} + +export function createMcpRetirementNotice(action) { + const status = getMcpRetirementStatus(action); + if (!status) { + return null; + } + const notice = document.createElement('div'); + notice.className = 'alert alert-warning small mcp-retirement-notice'; + notice.setAttribute('role', 'status'); + notice.textContent = status.message; + return notice; +} + /** * Convert a technical name to a human-readable display name. * Handles underscores, camelCase, PascalCase, and consecutive uppercase. @@ -220,6 +263,10 @@ export function openViewModal(item, type, callbacks = {}) { } else { titleEl.textContent = "Action Details"; bodyEl.innerHTML = buildActionViewHtml(item); + const retirementNotice = createMcpRetirementNotice(item); + if (retirementNotice) { + bodyEl.prepend(retirementNotice); + } } // Build footer buttons dynamically @@ -243,6 +290,9 @@ export function openViewModal(item, type, callbacks = {}) { editBtn.type = 'button'; editBtn.className = 'btn btn-outline-secondary'; editBtn.innerHTML = 'Edit'; + if (type === 'action' && getMcpRetirementStatus(item)) { + editBtn.textContent = 'Reconfigure'; + } editBtn.addEventListener('click', () => { bootstrap.Modal.getInstance(modalEl)?.hide(); onEdit(item); @@ -780,6 +830,18 @@ export function createActionCard(plugin, options = {}) { const editBtn = col.querySelector(".item-card-edit-btn"); const deleteBtn = col.querySelector(".item-card-delete-btn"); + const retirementNotice = createMcpRetirementNotice(plugin); + if (retirementNotice) { + const badge = document.createElement('span'); + badge.className = 'badge bg-warning text-dark ms-1'; + badge.textContent = 'Unsupported'; + col.querySelector('.card-title').appendChild(badge); + col.querySelector('.item-card-buttons').before(retirementNotice); + if (editBtn) { + editBtn.title = 'Reconfigure action'; + } + } + if (viewBtn && onView) viewBtn.addEventListener("click", (e) => { e.stopPropagation(); onView(plugin); }); if (editBtn && onEdit) editBtn.addEventListener("click", (e) => { e.stopPropagation(); onEdit(plugin); }); if (deleteBtn && onDelete) deleteBtn.addEventListener("click", (e) => { e.stopPropagation(); onDelete(plugin); }); diff --git a/application/single_app/static/js/workspace/workspace-migration.js b/application/single_app/static/js/workspace/workspace-migration.js index 37fc44f26..27d8cec3c 100644 --- a/application/single_app/static/js/workspace/workspace-migration.js +++ b/application/single_app/static/js/workspace/workspace-migration.js @@ -22,29 +22,35 @@ export async function checkMigrationStatus() { } const data = await response.json(); - console.log('Migration status:', data); - if (data.migration_needed) { - const { legacy_data } = data; - const hasAgents = legacy_data.agents_count > 0; - const hasActions = legacy_data.actions_count > 0; + const legacyData = data.legacy_data; + const actionCount = legacyData.actions_pending_count + legacyData.actions_failed_count; + const hasAgents = legacyData.agents_count > 0; + const hasActions = actionCount > 0; // Update banner text based on what needs migration let itemText = ''; if (hasAgents && hasActions) { - itemText = `${legacy_data.agents_count} agents and ${legacy_data.actions_count} actions`; + itemText = `${legacyData.agents_count} agents and ${actionCount} actions`; } else if (hasAgents) { - itemText = `${legacy_data.agents_count} agent${legacy_data.agents_count > 1 ? 's' : ''}`; + itemText = `${legacyData.agents_count} agent${legacyData.agents_count > 1 ? 's' : ''}`; } else if (hasActions) { - itemText = `${legacy_data.actions_count} action${legacy_data.actions_count > 1 ? 's' : ''}`; + itemText = `${actionCount} action${actionCount > 1 ? 's' : ''}`; } - const bannerText = migrationBanner.querySelector('small'); + const bannerText = migrationBanner?.querySelector('small'); if (bannerText) { - bannerText.textContent = `We've found ${itemText} in your old settings. Click to migrate them to the new improved storage system for better performance and reliability.`; + let message = `We've found ${itemText} ready to migrate or retry. Their original settings are kept until migration is verified.`; + if (legacyData.actions_retained_count > 0) { + message += ` ${legacyData.actions_retained_count} other legacy actions need manual reconfiguration or deletion in Actions.`; + } + bannerText.textContent = message; } showMigrationBanner(); + } else { + // Retired records stay visible in Actions, not in a recurring migration prompt. + hideMigrationBanner(); } } catch (error) { console.error('Error checking migration status:', error); @@ -56,7 +62,8 @@ export async function checkMigrationStatus() { */ function showMigrationBanner() { if (migrationBanner) { - migrationBanner.style.display = 'block'; + migrationBanner.style.removeProperty('display'); + migrationBanner.classList.remove('d-none'); } } @@ -65,7 +72,8 @@ function showMigrationBanner() { */ function hideMigrationBanner() { if (migrationBanner) { - migrationBanner.style.display = 'none'; + migrationBanner.style.removeProperty('display'); + migrationBanner.classList.add('d-none'); } } @@ -74,11 +82,16 @@ function hideMigrationBanner() { */ function showMigrationProgress() { if (migrationProgress) { - migrationProgress.style.display = 'block'; + migrationProgress.style.removeProperty('display'); + migrationProgress.classList.remove('d-none'); } if (migrateAllBtn) { migrateAllBtn.disabled = true; - migrateAllBtn.innerHTML = 'Migrating...'; + const spinner = document.createElement('span'); + spinner.className = 'spinner-border spinner-border-sm me-2'; + spinner.setAttribute('role', 'status'); + spinner.setAttribute('aria-hidden', 'true'); + migrateAllBtn.replaceChildren(spinner, document.createTextNode('Migrating...')); } } @@ -87,14 +100,18 @@ function showMigrationProgress() { */ function hideMigrationProgress() { if (migrationProgress) { - migrationProgress.style.display = 'none'; + migrationProgress.style.removeProperty('display'); + migrationProgress.classList.add('d-none'); } if (progressBar) { progressBar.style.width = '0%'; } if (migrateAllBtn) { migrateAllBtn.disabled = false; - migrateAllBtn.innerHTML = ' Migrate Now'; + const icon = document.createElement('i'); + icon.className = 'bi bi-arrow-up'; + icon.setAttribute('aria-hidden', 'true'); + migrateAllBtn.replaceChildren(icon, document.createTextNode(' Migrate Now')); } } @@ -134,29 +151,24 @@ async function performMigration() { const result = await response.json(); updateMigrationProgress(90, 'Finalizing...'); - // Small delay to show completion - setTimeout(() => { - updateMigrationProgress(100, 'Migration completed successfully!'); - - // Hide progress and banner after a brief success display - setTimeout(() => { - hideMigrationProgress(); - hideMigrationBanner(); - - // Show success toast - showToast('Migration completed successfully! Your agents and actions are now using the improved storage system.', 'success'); - - // Refresh the current tab's data - refreshCurrentTabData(); - }, 1500); - }, 500); + const actionOutcome = result.action_migration; + hideMigrationProgress(); + if (actionOutcome.failed_count > 0 || !actionOutcome.complete) { + showToast('Migration is incomplete. Unfinished actions remain in your settings; retry migration to finish them.', 'warning'); + } else if (actionOutcome.retained_count > 0) { + showToast(`${actionOutcome.migrated_count} actions migrated. ${actionOutcome.retained_count} legacy actions were kept for manual reconfiguration or deletion in Actions. Stdio actions cannot run.`, 'warning'); + } else { + showToast('Migration completed successfully. Your agents and actions now use the improved storage system.', 'success'); + } + refreshCurrentTabData(); + await checkMigrationStatus(); } catch (error) { console.error('Migration error:', error); hideMigrationProgress(); // Show error toast - showToast('Migration failed. Please try again or contact support if the issue persists.', 'error'); + showToast('Migration could not finish. Original actions are kept until their migration is verified. Please retry.', 'error'); } } diff --git a/application/single_app/static/js/workspace/workspace_plugins.js b/application/single_app/static/js/workspace/workspace_plugins.js index b8b4ab964..26172b6d3 100644 --- a/application/single_app/static/js/workspace/workspace_plugins.js +++ b/application/single_app/static/js/workspace/workspace_plugins.js @@ -2,7 +2,7 @@ import { renderPluginsTable, renderPluginsGrid, ensurePluginsTableInRoot, validatePluginManifest, getErrorMessageFromResponse } from '../plugin_common.js'; import { showToast } from "../chat/chat-toast.js" import { - setupViewToggle, switchViewContainers, openViewModal + setupViewToggle, switchViewContainers, openViewModal, getMcpRetirementStatus } from './view-utils.js'; const root = document.getElementById('workspace-plugins-root'); @@ -19,15 +19,17 @@ function renderError(msg) { } function getViewHandlers() { + const findPlugin = key => plugins.find(plugin => plugin.id === key) || plugins.find(plugin => plugin.name === key); return { - onEdit: name => openPluginModal(plugins.find(p => p.name === name)), - onDelete: name => deletePlugin(name), - onView: name => { - const plugin = plugins.find(p => p.name === name); + getPluginKey: plugin => plugin.id || plugin.name || '', + onEdit: key => openPluginModal(findPlugin(key)), + onDelete: key => deletePlugin(key), + onView: key => { + const plugin = findPlugin(key); if (plugin) { - openViewModal(plugin, 'action', { + openViewModal(plugin, 'action', plugin.is_global ? {} : { onEdit: (item) => openPluginModal(item), - onDelete: (item) => deletePlugin(item.name) + onDelete: (item) => deletePlugin(item.id || item.name) }); } } @@ -106,14 +108,14 @@ async function fetchPlugins() { } } -function openPluginModal(plugin = null) { +async function openPluginModal(plugin = null) { // Use the new multi-step modal if (window.pluginModalStepper) { window.pluginModalStepper.setActionScope({ scope: 'personal', apiBase: '/api/workspace-identities/personal' }); - const modal = window.pluginModalStepper.showModal(plugin); + const modal = await window.pluginModalStepper.showModal(plugin); // Set up save handler setupSaveHandler(plugin, modal); @@ -191,15 +193,23 @@ async function savePlugin(pluginData, existingPlugin = null) { let plugins = await res.json(); // Update or add the plugin - const existingIndex = plugins.findIndex(p => { - if (payload.id && p.id === payload.id) { - return true; - } - if (existingPlugin?.name && p.name === existingPlugin.name) { - return true; - } - return p.name === payload.name; - }); + let existingIndex; + if (existingPlugin?.id) { + existingIndex = plugins.findIndex(item => item.id === existingPlugin.id); + if (existingIndex < 0) { + throw new Error('This action is no longer available. Refresh the action list before saving.'); + } + } else { + const matchingName = existingPlugin?.name || payload.name; + const matches = plugins.map((item, index) => item.name === matchingName ? index : -1).filter(index => index >= 0); + if (matches.length > 1) { + throw new Error('Multiple actions have this name. Refresh the list and edit the specific action.'); + } + existingIndex = matches.length ? matches[0] : -1; + if (!existingPlugin && existingIndex >= 0 && getMcpRetirementStatus(plugins[existingIndex])) { + throw new Error('An unsupported action already has this name. Reconfigure or delete that action explicitly.'); + } + } if (existingIndex >= 0) { plugins[existingIndex] = payload; } else { @@ -219,11 +229,17 @@ async function savePlugin(pluginData, existingPlugin = null) { } } -async function deletePlugin(name) { +async function deletePlugin(key) { + const plugin = plugins.find(item => item.id === key) || plugins.find(item => item.name === key); + if (!plugin || plugin.is_global) { + return; + } + const name = plugin.name || ''; + const locator = plugin.id || name; if (!confirm(`Are you sure you want to delete action "${name}"?`)) return; try { - const res = await fetch(`/api/user/plugins/${encodeURIComponent(name)}`, { + const res = await fetch(`/api/user/plugins/${encodeURIComponent(locator)}`, { method: 'DELETE', headers: { 'Content-Type': 'application/json' } }); diff --git a/application/single_app/static/json/schemas/mcp_plugin.additional_settings.schema.json b/application/single_app/static/json/schemas/mcp_plugin.additional_settings.schema.json index ae5e97a65..f6b244d39 100644 --- a/application/single_app/static/json/schemas/mcp_plugin.additional_settings.schema.json +++ b/application/single_app/static/json/schemas/mcp_plugin.additional_settings.schema.json @@ -39,7 +39,7 @@ }, "transport": { "type": "string", - "enum": ["streamable_http", "sse", "websocket", "stdio"], + "enum": ["streamable_http", "sse", "websocket"], "default": "streamable_http", "description": "MCP transport used to connect to the server." }, @@ -116,22 +116,6 @@ "default": "truncate", "description": "How SimpleChat handles MCP tool results that exceed the configured result size limit." }, - "command": { - "type": "string", - "description": "Command used when transport is stdio." - }, - "args": { - "type": "array", - "items": {"type": "string"}, - "default": [], - "description": "Command arguments used when transport is stdio." - }, - "env": { - "type": "object", - "default": {}, - "additionalProperties": {"type": "string"}, - "description": "Environment variables used when transport is stdio." - }, "allowed_tool_names": { "type": "array", "items": {"type": "string"}, diff --git a/application/single_app/templates/_plugin_modal.html b/application/single_app/templates/_plugin_modal.html index e434bbd3a..85c870f4e 100644 --- a/application/single_app/templates/_plugin_modal.html +++ b/application/single_app/templates/_plugin_modal.html @@ -35,6 +35,7 @@
+
Select Action Type
@@ -737,11 +738,11 @@
API Information
- + -
@@ -750,21 +751,6 @@
API Information
-
-
- - -
-
- - -
One argument per line.
-
-
- - -
-
diff --git a/application/single_app/templates/workspace.html b/application/single_app/templates/workspace.html index 1be8f7267..f7f8f5723 100644 --- a/application/single_app/templates/workspace.html +++ b/application/single_app/templates/workspace.html @@ -576,7 +576,7 @@

Personal Workspace

{% include "_semantic_search_health_warning.html" %} -