From 73ff7d6998dc4827a0f6ba002386275a7c1ec804 Mon Sep 17 00:00:00 2001 From: Paul Lizer Date: Sat, 19 Sep 2026 08:16:12 -0400 Subject: [PATCH 1/3] Remove MCP stdio support and harden action authorization Retire stdio for every role and scope while retaining existing actions for explicit remote reconfiguration or deletion. Bind remote MCP authorization to trusted origins, current callers, and current policy. Harden legacy imports and migration, update remote-only configuration and UI, add regression coverage, and advance the application version to 0.261.029. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- application/single_app/config.py | 2 +- .../functions_action_connection_tests.py | 18 +- .../single_app/functions_action_manifest.py | 153 ++ .../single_app/functions_global_actions.py | 145 +- .../single_app/functions_governance.py | 7 +- .../single_app/functions_group_actions.py | 121 +- .../functions_legacy_action_management.py | 281 +++ .../single_app/functions_mcp_destinations.py | 228 ++- .../single_app/functions_mcp_operations.py | 53 +- .../functions_mcp_preconfigurations.py | 115 +- .../single_app/functions_mcp_presets.py | 57 +- .../single_app/functions_personal_actions.py | 883 +++++++-- .../single_app/json_schema_validation.py | 36 +- application/single_app/mcp_presets/README.md | 18 +- .../mcp_presets/definitions/generic.json | 5 +- .../mcp_presets/definitions/splunk.json | 3 +- .../mcp_presets/mcp_server_preset.schema.json | 10 +- .../single_app/plugin_validation_endpoint.py | 76 +- .../single_app/route_backend_plugins.py | 374 ++-- application/single_app/route_backend_users.py | 96 +- application/single_app/route_migration.py | 118 +- .../single_app/semantic_kernel_loader.py | 180 +- .../logged_plugin_loader.py | 39 +- .../semantic_kernel_plugins/mcp_plugin.py | 69 +- .../mcp_plugin_factory.py | 156 +- .../plugin_health_checker.py | 46 +- .../single_app/static/js/plugin_common.js | 37 +- .../static/js/plugin_modal_stepper.js | 254 +-- .../static/js/workspace/group_plugins.js | 18 +- .../static/js/workspace/view-utils.js | 62 + .../js/workspace/workspace-migration.js | 78 +- .../static/js/workspace/workspace_plugins.js | 56 +- ...mcp_plugin.additional_settings.schema.json | 18 +- .../single_app/templates/_plugin_modal.html | 20 +- .../single_app/templates/workspace.html | 4 +- docs/_data/app_surface.yml | 2 +- docs/admin/governance.md | 31 +- .../features/ACTION_TEST_CONNECTION.md | 32 +- .../features/MCP_PLUGIN_ROBUSTNESS_PLAN.md | 30 +- .../features/MCP_SERVER_PRECONFIGURATIONS.md | 25 +- .../features/MCP_SERVER_PRESETS.md | 26 +- .../v0.241.103/MCP_ACTION_CONFIGURATION.md | 2 + ...MCP_STDIO_REMOVAL_AND_AUTHORIZATION_FIX.md | 130 ++ .../release-250-agents-actions.md | 2 + docs/reference/actions/mcp.md | 28 +- ...action_connection_test_secret_redaction.py | 25 +- .../test_action_test_connection_endpoints.py | 38 +- .../test_mcp_action_manifest_workflow.py | 18 +- .../test_mcp_action_route_security.py | 364 ++++ .../test_mcp_authorization_context.py | 904 ++++++++++ ...nation_governance_and_preconfigurations.py | 33 +- .../test_mcp_legacy_stdio_management.py | 1593 +++++++++++++++++ functional_tests/test_mcp_server_presets.py | 368 ++-- functional_tests/test_mcp_stdio_removal.py | 253 +++ functional_tests/test_mcp_stdio_removal_ui.js | 717 ++++++++ .../test_mcp_tool_argument_normalization.py | 85 +- .../test_mcp_user_settings_ingestion.py | 304 ++++ ...t_user_plugin_bulk_save_id_preservation.py | 546 +++--- .../test_user_profile_idor_authorization.py | 25 +- ui_tests/test_workspace_mcp_action_modal.py | 388 +++- 60 files changed, 8187 insertions(+), 1618 deletions(-) create mode 100644 application/single_app/functions_action_manifest.py create mode 100644 application/single_app/functions_legacy_action_management.py create mode 100644 docs/explanation/fixes/MCP_STDIO_REMOVAL_AND_AUTHORIZATION_FIX.md create mode 100644 functional_tests/test_mcp_action_route_security.py create mode 100644 functional_tests/test_mcp_authorization_context.py create mode 100644 functional_tests/test_mcp_legacy_stdio_management.py create mode 100644 functional_tests/test_mcp_stdio_removal.py create mode 100644 functional_tests/test_mcp_stdio_removal_ui.js create mode 100644 functional_tests/test_mcp_user_settings_ingestion.py diff --git a/application/single_app/config.py b/application/single_app/config.py index 140c40b08..b56b998d7 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.028" +VERSION = "0.261.029" 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..711dd5dd5 --- /dev/null +++ b/application/single_app/functions_action_manifest.py @@ -0,0 +1,153 @@ +# 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): + """A mapping whose server-only origin is not part of its JSON payload.""" + + def __init__(self, manifest, origin): + if not isinstance(origin, McpActionOrigin): + raise ValueError("Action origin is required.") + super().__init__(manifest) + self._action_origin = origin + + @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..90b41bce3 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,14 +130,18 @@ 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() @@ -123,7 +153,7 @@ def save_global_action(action_data, user_id=None): item=action_data['id'], partition_key=action_data['id'] ) - except Exception: + except exceptions.CosmosResourceNotFoundError: pass if existing_action: @@ -141,12 +171,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 +188,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 +211,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 +243,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 +251,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 +261,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 e394cffe8..7893c8528 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) @@ -1259,7 +1260,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 @@ -1301,7 +1302,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..3a0fdef18 100644 --- a/application/single_app/functions_personal_actions.py +++ b/application/single_app/functions_personal_actions.py @@ -7,20 +7,53 @@ 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_settings import get_settings, get_user_settings +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 +69,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, 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 +146,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 +160,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 +180,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 +202,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 +242,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 +265,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, 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 +286,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 +323,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 +343,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. + 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, 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 +888,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 +908,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 +924,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" %} -