diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index cd84ccf..2c73633 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -12,7 +12,7 @@ "plugins": [ { "name": "unraid", - "source": "./", + "source": "skills/unraid", "description": "Query and monitor Unraid servers via GraphQL API - array status, disk health, containers, VMs, system monitoring", "version": "0.2.0", "tags": ["unraid", "monitoring", "homelab", "graphql", "docker", "virtualization"], diff --git a/docs/research/unraid-api-crawl.md b/docs/research/unraid-api-crawl.md index c009554..9117a40 100644 --- a/docs/research/unraid-api-crawl.md +++ b/docs/research/unraid-api-crawl.md @@ -81,7 +81,7 @@ The Unraid API provides a GraphQL interface that allows you to interact with you #### Enabling the GraphQL Sandbox **Live Documentation:** -- View the complete API schema and documentation at [Apollo GraphQL Studio](https://studio.apollographql.com/graph/Unraid-API/variant/current/home) +- View the complete API schema and documentation at [Apollo GraphQL Studio](https://studio.apollographql.com/graph/Unraid-API/variant/current/home) *(requires authentication; may not be publicly accessible)* **WebGUI method (recommended):** 1. Navigate to **Settings > Management Access > Developer Options** @@ -468,7 +468,7 @@ unraid-api apikey --delete --name "workflow key" --json #!/bin/bash set -e -# Set up cleanup trap early so it fires even if key creation fails +# Define cleanup function (deletes the temporary key) cleanup() { echo "Cleaning up..."; unraid-api apikey --delete --name "temp deployment key" 2>/dev/null || true; } # 1. Create temporary API key @@ -479,7 +479,7 @@ KEY_DATA=$(unraid-api apikey --create \ --description "Temporary key for deployment $(date)" \ --json) -# Register trap after key creation succeeds +# Register trap only after key creation succeeds (nothing to clean up if creation failed) trap cleanup EXIT # 2. Extract the API key diff --git a/scripts/validate-marketplace.sh b/scripts/validate-marketplace.sh index d2a6692..e668c3c 100755 --- a/scripts/validate-marketplace.sh +++ b/scripts/validate-marketplace.sh @@ -1,7 +1,7 @@ #!/usr/bin/env bash # Validate Claude Code marketplace and plugin structure -set -euo pipefail +set -uo pipefail # Colors for output RED='\033[0;31m' diff --git a/skills/unraid/examples/read-logs.sh b/skills/unraid/examples/read-logs.sh index b7a58be..8193839 100755 --- a/skills/unraid/examples/read-logs.sh +++ b/skills/unraid/examples/read-logs.sh @@ -8,6 +8,16 @@ QUERY_SCRIPT="$SCRIPT_DIR/../scripts/unraid-query.sh" LOG_NAME="${1:-syslog}" LINES="${2:-20}" +# Validate inputs to prevent GraphQL injection +if ! [[ "$LOG_NAME" =~ ^[a-zA-Z0-9_./-]+$ ]]; then + echo "Error: Invalid log name. Only alphanumeric characters, dots, slashes, hyphens, and underscores are allowed." >&2 + exit 1 +fi +if ! [[ "$LINES" =~ ^[0-9]+$ ]]; then + echo "Error: Lines must be a positive integer." >&2 + exit 1 +fi + echo "=== Reading $LOG_NAME (last $LINES lines) ===" echo "" diff --git a/unraid_mcp/main.py b/unraid_mcp/main.py index 36c772b..a2957d5 100644 --- a/unraid_mcp/main.py +++ b/unraid_mcp/main.py @@ -19,6 +19,18 @@ async def shutdown_cleanup() -> None: print(f"Error during cleanup: {e}") +def _run_shutdown_cleanup() -> None: + """Run shutdown cleanup, suppressing expected event loop errors.""" + try: + asyncio.run(shutdown_cleanup()) + except RuntimeError as e: + msg = str(e).lower() + if "event loop is closed" in msg or "no running event loop" in msg: + pass # Expected during shutdown + else: + print(f"WARNING: Unexpected error during cleanup: {e}", file=sys.stderr) + + def main() -> None: """Main entry point for the Unraid MCP Server.""" try: @@ -27,28 +39,10 @@ def main() -> None: run_server() except KeyboardInterrupt: print("\nServer stopped by user") - try: - asyncio.run(shutdown_cleanup()) - except RuntimeError as e: - if ( - "event loop is closed" in str(e).lower() - or "no running event loop" in str(e).lower() - ): - pass # Expected during shutdown - else: - print(f"WARNING: Unexpected error during cleanup: {e}", file=sys.stderr) + _run_shutdown_cleanup() except Exception as e: print(f"Server failed to start: {e}") - try: - asyncio.run(shutdown_cleanup()) - except RuntimeError as e: - if ( - "event loop is closed" in str(e).lower() - or "no running event loop" in str(e).lower() - ): - pass # Expected during shutdown - else: - print(f"WARNING: Unexpected error during cleanup: {e}", file=sys.stderr) + _run_shutdown_cleanup() raise diff --git a/unraid_mcp/server.py b/unraid_mcp/server.py index 18c652f..be711da 100644 --- a/unraid_mcp/server.py +++ b/unraid_mcp/server.py @@ -48,7 +48,7 @@ def register_all_modules() -> None: register_subscription_resources(mcp) logger.info("Subscription resources registered") - # Register all 10 consolidated tools + # Register all consolidated tools registrars = [ register_info_tool, register_array_tool, @@ -64,7 +64,7 @@ def register_all_modules() -> None: for registrar in registrars: registrar(mcp) - logger.info("All 10 tools registered successfully - Server ready!") + logger.info(f"All {len(registrars)} tools registered successfully - Server ready!") except Exception as e: logger.error(f"Failed to register modules: {e}", exc_info=True) diff --git a/unraid_mcp/tools/docker.py b/unraid_mcp/tools/docker.py index bd9d4ad..b665e47 100644 --- a/unraid_mcp/tools/docker.py +++ b/unraid_mcp/tools/docker.py @@ -134,13 +134,34 @@ DOCKER_ACTIONS = Literal[ _DOCKER_ID_PATTERN = re.compile(r"^[a-f0-9]{64}(:[a-z0-9]+)?$", re.IGNORECASE) +def _safe_get(data: dict[str, Any], *keys: str, default: Any = None) -> Any: + """Safely traverse nested dict keys, handling None intermediates.""" + current = data + for key in keys: + if not isinstance(current, dict): + return default + current = current.get(key) + return current if current is not None else default + + def find_container_by_identifier( identifier: str, containers: list[dict[str, Any]] ) -> dict[str, Any] | None: - """Find a container by ID or name with fuzzy matching.""" + """Find a container by ID or name with fuzzy matching. + + Match priority: + 1. Exact ID match + 2. Exact name match (case-sensitive) + 3. Name starts with identifier (case-insensitive) + 4. Name contains identifier as substring (case-insensitive) + + Note: Short identifiers (e.g. "db") may match unintended containers + via substring. Use more specific names or IDs for precision. + """ if not containers: return None + # Priority 1 & 2: exact matches for c in containers: if c.get("id") == identifier: return c @@ -148,10 +169,19 @@ def find_container_by_identifier( return c id_lower = identifier.lower() + + # Priority 3: prefix match (more precise than substring) + for c in containers: + for name in c.get("names", []): + if name.lower().startswith(id_lower): + logger.info(f"Prefix match: '{identifier}' -> '{name}'") + return c + + # Priority 4: substring match (least precise) for c in containers: for name in c.get("names", []): if id_lower in name.lower(): - logger.info(f"Fuzzy match: '{identifier}' -> '{name}'") + logger.info(f"Substring match: '{identifier}' -> '{name}'") return c return None @@ -177,7 +207,7 @@ async def _resolve_container_id(container_id: str) -> str: } """ data = await make_graphql_request(list_query) - containers = data.get("docker", {}).get("containers", []) + containers = _safe_get(data, "docker", "containers", default=[]) resolved = find_container_by_identifier(container_id, containers) if resolved: actual_id = str(resolved.get("id", "")) @@ -240,12 +270,12 @@ def register_docker_tool(mcp: FastMCP) -> None: # --- Read-only queries --- if action == "list": data = await make_graphql_request(QUERIES["list"]) - containers = data.get("docker", {}).get("containers", []) + containers = _safe_get(data, "docker", "containers", default=[]) return {"containers": list(containers) if isinstance(containers, list) else []} if action == "details": data = await make_graphql_request(QUERIES["details"]) - containers = data.get("docker", {}).get("containers", []) + containers = _safe_get(data, "docker", "containers", default=[]) container = find_container_by_identifier(container_id or "", containers) if container: return container @@ -260,7 +290,7 @@ def register_docker_tool(mcp: FastMCP) -> None: data = await make_graphql_request( QUERIES["logs"], {"id": actual_id, "tail": tail_lines} ) - return {"logs": data.get("docker", {}).get("logs")} + return {"logs": _safe_get(data, "docker", "logs")} if action == "networks": data = await make_graphql_request(QUERIES["networks"]) @@ -269,16 +299,16 @@ def register_docker_tool(mcp: FastMCP) -> None: if action == "network_details": data = await make_graphql_request(QUERIES["network_details"], {"id": network_id}) - return dict(data.get("dockerNetwork", {})) + return dict(data.get("dockerNetwork") or {}) if action == "port_conflicts": data = await make_graphql_request(QUERIES["port_conflicts"]) - conflicts = data.get("docker", {}).get("portConflicts", []) + conflicts = _safe_get(data, "docker", "portConflicts", default=[]) return {"port_conflicts": list(conflicts) if isinstance(conflicts, list) else []} if action == "check_updates": data = await make_graphql_request(QUERIES["check_updates"]) - statuses = data.get("docker", {}).get("containerUpdateStatuses", []) + statuses = _safe_get(data, "docker", "containerUpdateStatuses", default=[]) return {"update_statuses": list(statuses) if isinstance(statuses, list) else []} # --- Mutations --- @@ -300,7 +330,7 @@ def register_docker_tool(mcp: FastMCP) -> None: if start_data.get("idempotent_success"): result = {} else: - result = start_data.get("docker", {}).get("start", {}) + result = _safe_get(start_data, "docker", "start", default={}) response: dict[str, Any] = { "success": True, "action": "restart", @@ -312,7 +342,7 @@ def register_docker_tool(mcp: FastMCP) -> None: if action == "update_all": data = await make_graphql_request(MUTATIONS["update_all"]) - results = data.get("docker", {}).get("updateAllContainers", []) + results = _safe_get(data, "docker", "updateAllContainers", default=[]) return {"success": True, "action": "update_all", "containers": results} # Single-container mutations @@ -336,7 +366,7 @@ def register_docker_tool(mcp: FastMCP) -> None: "message": f"Container already in desired state for '{action}'", } - docker_data = data.get("docker", {}) + docker_data = data.get("docker") or {} # Map action names to GraphQL response field names where they differ response_field_map = { "update": "updateContainer", diff --git a/unraid_mcp/tools/health.py b/unraid_mcp/tools/health.py index 32f5629..eae568a 100644 --- a/unraid_mcp/tools/health.py +++ b/unraid_mcp/tools/health.py @@ -204,6 +204,8 @@ async def _comprehensive_check() -> dict[str, Any]: return health_info except Exception as e: + # Intentionally broad: health checks must always return a result, + # even on unexpected failures, so callers never get an unhandled exception. logger.error(f"Health check failed: {e}") return { "status": "unhealthy", @@ -222,6 +224,9 @@ async def _diagnose_subscriptions() -> dict[str, Any]: await ensure_subscriptions_started() status = subscription_manager.get_subscription_status() + # This list is intentionally placed into the summary dict below and then + # appended to in the loop — the mutable alias ensures both references + # reflect the same data without a second pass. connection_issues: list[dict[str, Any]] = [] diagnostic_info: dict[str, Any] = { diff --git a/unraid_mcp/tools/keys.py b/unraid_mcp/tools/keys.py index b444ab0..f556a85 100644 --- a/unraid_mcp/tools/keys.py +++ b/unraid_mcp/tools/keys.py @@ -101,9 +101,9 @@ def register_keys_tool(mcp: FastMCP) -> None: if not name: raise ToolError("name is required for 'create' action") input_data: dict[str, Any] = {"name": name} - if roles: + if roles is not None: input_data["roles"] = roles - if permissions: + if permissions is not None: input_data["permissions"] = permissions data = await make_graphql_request(MUTATIONS["create"], {"input": input_data}) return { @@ -117,7 +117,7 @@ def register_keys_tool(mcp: FastMCP) -> None: input_data: dict[str, Any] = {"id": key_id} if name: input_data["name"] = name - if roles: + if roles is not None: input_data["roles"] = roles data = await make_graphql_request(MUTATIONS["update"], {"input": input_data}) return { diff --git a/unraid_mcp/tools/storage.py b/unraid_mcp/tools/storage.py index e5d938f..a12f6be 100644 --- a/unraid_mcp/tools/storage.py +++ b/unraid_mcp/tools/storage.py @@ -70,7 +70,10 @@ def format_bytes(bytes_value: int | None) -> str: """Format byte values into human-readable sizes.""" if bytes_value is None: return "N/A" - value = float(int(bytes_value)) + try: + value = float(int(bytes_value)) + except (ValueError, TypeError): + return "N/A" for unit in ["B", "KB", "MB", "GB", "TB", "PB"]: if value < 1024.0: return f"{value:.2f} {unit}" @@ -118,7 +121,7 @@ def register_storage_tool(mcp: FastMCP) -> None: query = QUERIES[action] variables: dict[str, Any] | None = None - custom_timeout = DISK_TIMEOUT if action == "disks" else None + custom_timeout = DISK_TIMEOUT if action in ("disks", "disk_details") else None if action == "disk_details": variables = {"id": disk_id} diff --git a/unraid_mcp/tools/virtualization.py b/unraid_mcp/tools/virtualization.py index d6ccf9a..562c550 100644 --- a/unraid_mcp/tools/virtualization.py +++ b/unraid_mcp/tools/virtualization.py @@ -64,6 +64,8 @@ VM_ACTIONS = Literal[ "reset", ] +ALL_ACTIONS = set(QUERIES) | set(MUTATIONS) | {"details"} + def register_vm_tool(mcp: FastMCP) -> None: """Register the unraid_vm tool with the FastMCP instance.""" @@ -87,9 +89,8 @@ def register_vm_tool(mcp: FastMCP) -> None: reboot - Reboot a VM (requires vm_id) reset - Reset a VM (requires vm_id, confirm=True) """ - all_actions = set(QUERIES) | set(MUTATIONS) | {"details"} - if action not in all_actions: - raise ToolError(f"Invalid action '{action}'. Must be one of: {sorted(all_actions)}") + if action not in ALL_ACTIONS: + raise ToolError(f"Invalid action '{action}'. Must be one of: {sorted(ALL_ACTIONS)}") if action != "list" and not vm_id: raise ToolError(f"vm_id is required for '{action}' action")