chore: apply CodeRabbit suggestions

- README path separators (todo in separate doc commit)
- manage_gameobject: pop prefabFolder not prefab_folder
- execute_menu_item: make sync to avoid blocking event loop
- telemetry: validate endpoint scheme (allow http/https only) and re-validate at send time
main
David Sarno 2025-09-08 20:25:07 -07:00
parent 979757e38a
commit 7f0527f708
3 changed files with 33 additions and 8 deletions

View File

@ -13,6 +13,7 @@ import sys
import platform import platform
import logging import logging
from enum import Enum from enum import Enum
from urllib.parse import urlparse
from dataclasses import dataclass, asdict from dataclasses import dataclass, asdict
from typing import Optional, Dict, Any, List from typing import Optional, Dict, Any, List
from pathlib import Path from pathlib import Path
@ -64,9 +65,11 @@ class TelemetryConfig:
self.enabled = not self._is_disabled() self.enabled = not self._is_disabled()
# Telemetry endpoint (Cloud Run default; override via env) # Telemetry endpoint (Cloud Run default; override via env)
self.endpoint = os.environ.get( default_ep = "https://unity-mcp-telemetry-375728817078.us-central1.run.app/telemetry/events"
"UNITY_MCP_TELEMETRY_ENDPOINT", self.default_endpoint = default_ep
"https://unity-mcp-telemetry-375728817078.us-central1.run.app/telemetry/events" self.endpoint = self._validated_endpoint(
os.environ.get("UNITY_MCP_TELEMETRY_ENDPOINT", default_ep),
default_ep,
) )
# Local storage for UUID and milestones # Local storage for UUID and milestones
@ -109,6 +112,25 @@ class TelemetryConfig:
data_dir.mkdir(parents=True, exist_ok=True) data_dir.mkdir(parents=True, exist_ok=True)
return data_dir return data_dir
def _validated_endpoint(self, candidate: str, fallback: str) -> str:
"""Validate telemetry endpoint URL scheme; allow only http/https.
Falls back to the provided default on error.
"""
try:
parsed = urlparse(candidate)
if parsed.scheme not in ("https", "http"):
raise ValueError(f"Unsupported scheme: {parsed.scheme}")
# Basic sanity: require network location and path
if not parsed.netloc:
raise ValueError("Missing netloc in endpoint")
return candidate
except Exception as e:
logger.debug(
f"Invalid telemetry endpoint '{candidate}', using default. Error: {e}",
exc_info=True,
)
return fallback
class TelemetryCollector: class TelemetryCollector:
"""Main telemetry collection class""" """Main telemetry collection class"""
@ -227,7 +249,9 @@ class TelemetryCollector:
# Prefer httpx when available; otherwise fall back to urllib # Prefer httpx when available; otherwise fall back to urllib
if httpx: if httpx:
with httpx.Client(timeout=self.config.timeout) as client: with httpx.Client(timeout=self.config.timeout) as client:
response = client.post(self.config.endpoint, json=payload) # Re-validate endpoint at send time to handle dynamic changes
endpoint = self.config._validated_endpoint(self.config.endpoint, self.config.default_endpoint)
response = client.post(endpoint, json=payload)
if response.status_code == 200: if response.status_code == 200:
logger.debug(f"Telemetry sent: {record.record_type}") logger.debug(f"Telemetry sent: {record.record_type}")
else: else:
@ -236,8 +260,9 @@ class TelemetryCollector:
import urllib.request import urllib.request
import urllib.error import urllib.error
data_bytes = json.dumps(payload).encode("utf-8") data_bytes = json.dumps(payload).encode("utf-8")
endpoint = self.config._validated_endpoint(self.config.endpoint, self.config.default_endpoint)
req = urllib.request.Request( req = urllib.request.Request(
self.config.endpoint, endpoint,
data=data_bytes, data=data_bytes,
headers={"Content-Type": "application/json"}, headers={"Content-Type": "application/json"},
method="POST", method="POST",

View File

@ -14,7 +14,7 @@ def register_execute_menu_item_tools(mcp: FastMCP):
@mcp.tool() @mcp.tool()
@telemetry_tool("execute_menu_item") @telemetry_tool("execute_menu_item")
async def execute_menu_item( def execute_menu_item(
ctx: Any, ctx: Any,
menu_path: str, menu_path: str,
action: str = 'execute', action: str = 'execute',

View File

@ -122,9 +122,9 @@ def register_manage_gameobject_tools(mcp: FastMCP):
params["prefabPath"] = constructed_path.replace("\\", "/") params["prefabPath"] = constructed_path.replace("\\", "/")
elif not params["prefabPath"].lower().endswith(".prefab"): elif not params["prefabPath"].lower().endswith(".prefab"):
return {"success": False, "message": f"Invalid prefab_path: '{params['prefabPath']}' must end with .prefab"} return {"success": False, "message": f"Invalid prefab_path: '{params['prefabPath']}' must end with .prefab"}
# Ensure prefab_folder itself isn't sent if prefabPath was constructed or provided # Ensure prefabFolder itself isn't sent if prefabPath was constructed or provided
# The C# side only needs the final prefabPath # The C# side only needs the final prefabPath
params.pop("prefab_folder", None) params.pop("prefabFolder", None)
# -------------------------------- # --------------------------------
# Use centralized retry helper # Use centralized retry helper