Skip to content

Commit 7f0527f

Browse files
committed
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
1 parent 979757e commit 7f0527f

File tree

3 files changed

+33
-8
lines changed

3 files changed

+33
-8
lines changed

UnityMcpBridge/UnityMcpServer~/src/telemetry.py

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import platform
1414
import logging
1515
from enum import Enum
16+
from urllib.parse import urlparse
1617
from dataclasses import dataclass, asdict
1718
from typing import Optional, Dict, Any, List
1819
from pathlib import Path
@@ -64,9 +65,11 @@ def __init__(self):
6465
self.enabled = not self._is_disabled()
6566

6667
# Telemetry endpoint (Cloud Run default; override via env)
67-
self.endpoint = os.environ.get(
68-
"UNITY_MCP_TELEMETRY_ENDPOINT",
69-
"https://unity-mcp-telemetry-375728817078.us-central1.run.app/telemetry/events"
68+
default_ep = "https://unity-mcp-telemetry-375728817078.us-central1.run.app/telemetry/events"
69+
self.default_endpoint = default_ep
70+
self.endpoint = self._validated_endpoint(
71+
os.environ.get("UNITY_MCP_TELEMETRY_ENDPOINT", default_ep),
72+
default_ep,
7073
)
7174

7275
# Local storage for UUID and milestones
@@ -109,6 +112,25 @@ def _get_data_directory(self) -> Path:
109112
data_dir.mkdir(parents=True, exist_ok=True)
110113
return data_dir
111114

115+
def _validated_endpoint(self, candidate: str, fallback: str) -> str:
116+
"""Validate telemetry endpoint URL scheme; allow only http/https.
117+
Falls back to the provided default on error.
118+
"""
119+
try:
120+
parsed = urlparse(candidate)
121+
if parsed.scheme not in ("https", "http"):
122+
raise ValueError(f"Unsupported scheme: {parsed.scheme}")
123+
# Basic sanity: require network location and path
124+
if not parsed.netloc:
125+
raise ValueError("Missing netloc in endpoint")
126+
return candidate
127+
except Exception as e:
128+
logger.debug(
129+
f"Invalid telemetry endpoint '{candidate}', using default. Error: {e}",
130+
exc_info=True,
131+
)
132+
return fallback
133+
112134
class TelemetryCollector:
113135
"""Main telemetry collection class"""
114136

@@ -227,7 +249,9 @@ def _send_telemetry(self, record: TelemetryRecord):
227249
# Prefer httpx when available; otherwise fall back to urllib
228250
if httpx:
229251
with httpx.Client(timeout=self.config.timeout) as client:
230-
response = client.post(self.config.endpoint, json=payload)
252+
# Re-validate endpoint at send time to handle dynamic changes
253+
endpoint = self.config._validated_endpoint(self.config.endpoint, self.config.default_endpoint)
254+
response = client.post(endpoint, json=payload)
231255
if response.status_code == 200:
232256
logger.debug(f"Telemetry sent: {record.record_type}")
233257
else:
@@ -236,8 +260,9 @@ def _send_telemetry(self, record: TelemetryRecord):
236260
import urllib.request
237261
import urllib.error
238262
data_bytes = json.dumps(payload).encode("utf-8")
263+
endpoint = self.config._validated_endpoint(self.config.endpoint, self.config.default_endpoint)
239264
req = urllib.request.Request(
240-
self.config.endpoint,
265+
endpoint,
241266
data=data_bytes,
242267
headers={"Content-Type": "application/json"},
243268
method="POST",

UnityMcpBridge/UnityMcpServer~/src/tools/execute_menu_item.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ def register_execute_menu_item_tools(mcp: FastMCP):
1414

1515
@mcp.tool()
1616
@telemetry_tool("execute_menu_item")
17-
async def execute_menu_item(
17+
def execute_menu_item(
1818
ctx: Any,
1919
menu_path: str,
2020
action: str = 'execute',

UnityMcpBridge/UnityMcpServer~/src/tools/manage_gameobject.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,9 @@ def manage_gameobject(
122122
params["prefabPath"] = constructed_path.replace("\\", "/")
123123
elif not params["prefabPath"].lower().endswith(".prefab"):
124124
return {"success": False, "message": f"Invalid prefab_path: '{params['prefabPath']}' must end with .prefab"}
125-
# Ensure prefab_folder itself isn't sent if prefabPath was constructed or provided
125+
# Ensure prefabFolder itself isn't sent if prefabPath was constructed or provided
126126
# The C# side only needs the final prefabPath
127-
params.pop("prefab_folder", None)
127+
params.pop("prefabFolder", None)
128128
# --------------------------------
129129

130130
# Use centralized retry helper

0 commit comments

Comments
 (0)