Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
# Shell scripts must keep LF: tools/compile-check.sh runs inside a Linux container,
# where CRLF would fail with "bad interpreter".
*.sh text eol=lf

# compile-check.sh reads these line by line; a CR from a core.autocrlf checkout would
# become part of every -define: symbol and reference name.
tools/compile-defines.txt text eol=lf
tools/compile-refs/*.txt text eol=lf
4 changes: 3 additions & 1 deletion .github/workflows/e2e-bridge.yml
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,9 @@ jobs:
-v "$RUNNER_TEMP/unity-config:/root/.config/unity3d" \
-v "$RUNNER_TEMP/unity-local:/root/.local/share/unity3d" \
"$UNITY_IMAGE" bash -lc '
set -euxo pipefail
# No -x here: xtrace would echo the expanded -password/-serial arguments into
# the job log, and GitHub secret masking is a last line of defence, not a design.
set -euo pipefail
/opt/unity/Editor/Unity -batchmode -nographics -logFile - \
-username "$UNITY_EMAIL" -password "$UNITY_PASSWORD" -serial "$UNITY_SERIAL" -quit || true
'
Expand Down
7 changes: 5 additions & 2 deletions .github/workflows/python-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,11 @@ jobs:
- name: Install dependencies
run: |
cd Server
uv sync
uv pip install -e ".[dev]"
# --locked fails if uv.lock disagrees with pyproject.toml instead of silently
# re-resolving, so a dependency or version bump that forgot `uv lock` shows up here.
# The dev extra is resolved in the lock too, so pytest runs against pinned versions
# instead of whatever `uv pip install` would fetch today.
uv sync --locked --extra dev

- name: Run tests with coverage
run: |
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ jobs:
git config user.name "GitHub Actions"
git config user.email "actions@github.com"
git checkout -b "$BRANCH"
git add MCPForUnity/package.json manifest.json "Server/pyproject.toml" Server/README.md
git add MCPForUnity/package.json manifest.json "Server/pyproject.toml" Server/uv.lock Server/README.md
if git diff --cached --quiet; then
echo "No version changes to commit."
else
Expand Down
6 changes: 6 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,12 @@ cd Server && uv run pytest tests/ -k "test_create_material" -v
# Local multi-version compile check (parity with CI matrix, see tools/unity-versions.json)
tools/check-unity-versions.sh # compile-only across installed Unity Hub editors
tools/check-unity-versions.sh --full # full EditMode test run

# License-free Roslyn compile of MCPForUnity, the same gate compile-check.yml runs on every PR.
# No Editor launch, ~1 min per version. EXTRA_REFS must hold Newtonsoft.Json.dll and
# nunit.framework.dll (copy them from TestProjects/UnityMCPTests/Library/PackageCache);
# the script header has the full recipe, including the Windows/Git Bash form.
UNITY_DATA=/path/to/Editor/Data UNITY_VERSION=2021.3.45f2 EXTRA_REFS=/path/to/refs tools/compile-check.sh
```

#### Local headless test harness
Expand Down
13 changes: 7 additions & 6 deletions MCPForUnity/Editor/McpCiBoot.cs
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
using System;
using MCPForUnity.Editor.Constants;
using MCPForUnity.Editor.Services;
using MCPForUnity.Editor.Services.Transport.Transports;
using UnityEditor;

namespace MCPForUnity.Editor
{
public static class McpCiBoot
{
public static void StartStdioForCi()
{
try
{
EditorPrefs.SetBool(EditorPrefKeys.UseHttpTransport, false);
// Session-scoped, not EditorPrefs: this must not rewrite the developer's real
// transport preference, and it has to beat the value EditorConfigurationCache
// already read at domain load, which HttpAutoStartHandler consults on its first tick.
try
{
EditorConfigurationCache.Instance.PinStdioForSession();
}
catch { /* ignore */ }

Expand Down
37 changes: 35 additions & 2 deletions MCPForUnity/Editor/Services/EditorConfigurationCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,14 @@ public static EditorConfigurationCache Instance
/// </summary>
public event Action<string> OnConfigurationChanged;

// A headless CI/harness editor (McpCiBoot) must run stdio no matter what the machine-wide
// EditorPrefs say: on Windows those prefs are per-user, so a developer who uses HTTP would
// otherwise have HttpAutoStartHandler stop the stdio bridge the harness is talking to.
// SessionState lives exactly as long as that editor process and survives its domain reloads.
internal const string SessionKeyForceStdio = "MCPForUnity.ForceStdioForSession";

// Cached values - most frequently read
private bool _forceStdioForSession;
private bool _useHttpTransport;
private bool _debugLogs;
private bool _devModeForceServerRefresh;
Expand All @@ -59,9 +66,9 @@ public static EditorConfigurationCache Instance

/// <summary>
/// Whether to use HTTP transport (true) or Stdio transport (false).
/// Default: true
/// Default: true. Always false while <see cref="PinStdioForSession"/> is in effect.
/// </summary>
public bool UseHttpTransport => _useHttpTransport;
public bool UseHttpTransport => !_forceStdioForSession && _useHttpTransport;

/// <summary>
/// Whether debug logging is enabled.
Expand Down Expand Up @@ -128,6 +135,7 @@ private EditorConfigurationCache()
/// </summary>
public void Refresh()
{
_forceStdioForSession = SessionState.GetBool(SessionKeyForceStdio, false);
_useHttpTransport = EditorPrefs.GetBool(EditorPrefKeys.UseHttpTransport, true);
_debugLogs = EditorPrefs.GetBool(EditorPrefKeys.DebugLogs, false);
_devModeForceServerRefresh = EditorPrefs.GetBool(EditorPrefKeys.DevModeForceServerRefresh, false);
Expand All @@ -140,6 +148,31 @@ public void Refresh()
_unitySocketPort = EditorPrefs.GetInt(EditorPrefKeys.UnitySocketPort, 0);
}

/// <summary>
/// Force stdio transport for the rest of this editor session without touching EditorPrefs.
/// Every UseHttpTransport consumer (auto-start, reload handlers, BridgeControlService,
/// client configurators) sees stdio until <see cref="UnpinStdioForSession"/> or editor exit.
/// </summary>
public void PinStdioForSession()
{
SessionState.SetBool(SessionKeyForceStdio, true);
if (!_forceStdioForSession)
{
_forceStdioForSession = true;
OnConfigurationChanged?.Invoke(nameof(UseHttpTransport));
}
}

public void UnpinStdioForSession()
{
SessionState.EraseBool(SessionKeyForceStdio);
if (_forceStdioForSession)
{
_forceStdioForSession = false;
OnConfigurationChanged?.Invoke(nameof(UseHttpTransport));
}
}

/// <summary>
/// Set UseHttpTransport and update cache + EditorPrefs atomically.
/// </summary>
Expand Down
33 changes: 28 additions & 5 deletions Server/src/services/api_key_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from __future__ import annotations

import asyncio
import hashlib
import logging
import time
from dataclasses import dataclass
Expand Down Expand Up @@ -35,6 +36,11 @@ class ApiKeyService:
REQUEST_TIMEOUT: float = 5.0
MAX_RETRIES: int = 1

# Confirmed-invalid keys are cached too, so a bad key does not re-hit the auth service on
# every call. That also means an unauthenticated caller grows the cache by one entry per
# random key it tries; the cap keeps that bounded and negatives are the first to go.
MAX_CACHE_ENTRIES: int = 1024

def __init__(
self,
validation_url: str,
Expand Down Expand Up @@ -108,24 +114,41 @@ async def validate(self, api_key: str) -> ValidationResult:
# not be cached to avoid locking out users during service outages.
if result.cacheable:
async with self._cache_lock:
expires_at = time.time() + self._cache_ttl
now = time.time()
if len(self._cache) >= self.MAX_CACHE_ENTRIES:
for stale in [k for k, v in self._cache.items() if v[3] <= now]:
del self._cache[stale]
if len(self._cache) >= self.MAX_CACHE_ENTRIES:
if not result.valid:
# Full of live entries: a negative verdict is not worth evicting
# anything for. The caller still gets the answer.
return result
# Make room for a validated key: drop a negative entry if there is
# one, otherwise the validated key that expires soonest.
negatives = [k for k, v in self._cache.items() if not v[0]]
pool = negatives or list(self._cache)
del self._cache[min(pool, key=lambda k: self._cache[k][3])]
self._cache[api_key] = (
result.valid,
result.user_id,
result.metadata,
expires_at,
now + self._cache_ttl,
)

return result

@staticmethod
def _fingerprint(api_key: str) -> str:
"""One-way handle for log lines. Eight literal characters of a key were enough to
correlate a leaked log with a key; a hash prefix correlates without exposing any."""
return "sha256:" + hashlib.sha256(api_key.encode("utf-8")).hexdigest()[:12]

async def _validate_external(self, api_key: str) -> ValidationResult:
"""Call external validation endpoint.

Failure mode: fail closed (treat as invalid on errors).
"""
# Redact API key from logs
redacted_key = f"{api_key[:4]}...{api_key[-4:]}" if len(
api_key) > 8 else "***"
redacted_key = self._fingerprint(api_key)

for attempt in range(self.MAX_RETRIES + 1):
try:
Expand Down
7 changes: 7 additions & 0 deletions Server/src/services/custom_tool_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,13 @@ def get_instance(cls) -> "CustomToolService":

# --- HTTP Routes -----------------------------------------------------
def _register_http_routes(self) -> None:
# The plugin registers custom tools over the hub WebSocket (register_tools message),
# so this REST route only serves local tooling. A remote-hosted server must not expose
# it: it carries no API-key check, so any caller could replace tool definitions for
# every tenant. Mirrors the /api/command gate in main.py.
if config.http_remote_hosted:
return

@self._mcp.custom_route("/register-tools", methods=["POST"])
async def register_tools(request: Request) -> JSONResponse:
try:
Expand Down
27 changes: 26 additions & 1 deletion Server/src/services/tools/debug_request_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,31 @@
from transport.unity_instance_middleware import get_unity_instance_middleware
from transport.plugin_hub import PluginHub

_SECRET_FLAG_MARKERS = ("token", "secret", "password", "api-key", "api_key", "apikey")


def _redact_argv(argv: list[str]) -> list[str]:
"""Keep flag names for diagnosis, hide the values of secret-bearing ones.

A remote-hosted server is started with --api-key-service-token on its command line and
this tool is callable by every authenticated tenant, so the raw argv handed out the
service credential. The flag shape is what helps debug a deployment; the value never is.
"""
# Every secret-bearing flag the server accepts takes a value, so the token after a bare
# flag is always that value, even when it happens to start with "-".
out: list[str] = []
hide_next = False
for arg in argv:
if hide_next:
out.append("***")
hide_next = False
continue
name, sep, _value = arg.partition("=")
secret = name.startswith("-") and any(m in name.lower() for m in _SECRET_FLAG_MARKERS)
out.append(f"{name}=***" if secret and sep else arg)
hide_next = secret and not sep
return out


@mcp_for_unity_tool(
unity_target=None,
Expand Down Expand Up @@ -65,7 +90,7 @@ async def debug_request_context(ctx: Context) -> dict[str, Any]:
"server": {
"version": get_package_version(),
"cwd": os.getcwd(),
"argv": list(sys.argv),
"argv": _redact_argv(sys.argv),
},
"request_context": {
"client_id": rc_client_id,
Expand Down
124 changes: 124 additions & 0 deletions Server/tests/integration/test_api_key_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -454,3 +454,127 @@ async def capture_post(url, *, json=None, headers=None):

assert captured_headers.get("X-Service-Token") == "test-svc-token-123"
assert captured_headers.get("Content-Type") == "application/json"


# ---------------------------------------------------------------------------
# Cache bound + log redaction
# ---------------------------------------------------------------------------

def _patched_client(mock_resp):
ctx = patch("httpx.AsyncClient")
MockClient = ctx.start()
instance = AsyncMock()
instance.__aenter__ = AsyncMock(return_value=instance)
instance.__aexit__ = AsyncMock(return_value=False)
instance.post = AsyncMock(return_value=mock_resp)
MockClient.return_value = instance
return ctx, instance


class TestCacheBound:
@pytest.mark.asyncio
async def test_negative_results_cannot_grow_cache_past_cap(self, monkeypatch):
"""Unauthenticated callers choose the key, so every failed guess used to add an
entry. The cap must hold no matter how many distinct bad keys arrive."""
monkeypatch.setattr(ApiKeyService, "MAX_CACHE_ENTRIES", 5)
svc = _make_service()
ctx, _ = _patched_client(_mock_response(401))
try:
for i in range(50):
result = await svc.validate(f"bad-key-{i:04d}-padding-to-length")
assert result.valid is False
finally:
ctx.stop()
assert len(svc._cache) <= 5

@pytest.mark.asyncio
async def test_valid_key_still_cached_when_cap_is_full_of_negatives(self, monkeypatch):
monkeypatch.setattr(ApiKeyService, "MAX_CACHE_ENTRIES", 3)
svc = _make_service()
ctx, instance = _patched_client(_mock_response(401))
try:
for i in range(3):
await svc.validate(f"bad-key-{i:04d}-padding-to-length")
instance.post = AsyncMock(return_value=_mock_response(
200, {"valid": True, "user_id": "user-1"}))
r1 = await svc.validate("good-key-000-padding-to-length")
calls_after_first = instance.post.await_count
r2 = await svc.validate("good-key-000-padding-to-length")
finally:
ctx.stop()
assert r1.valid and r2.valid
# Second call was served from cache: a validated key evicts a negative entry.
assert instance.post.await_count == calls_after_first
assert len(svc._cache) <= 3
assert "good-key-000-padding-to-length" in svc._cache

@pytest.mark.asyncio
async def test_expired_entries_are_purged_before_evicting_live_ones(self, monkeypatch):
monkeypatch.setattr(ApiKeyService, "MAX_CACHE_ENTRIES", 2)
svc = _make_service()
ctx, _ = _patched_client(_mock_response(200, {"valid": True, "user_id": "u"}))
try:
await svc.validate("live-key-aaaa-padding-to-length")
await svc.validate("stale-key-bbbb-padding-to-length")
async with svc._cache_lock:
v = svc._cache["stale-key-bbbb-padding-to-length"]
svc._cache["stale-key-bbbb-padding-to-length"] = (v[0], v[1], v[2], time.time() - 1)
await svc.validate("new-key-cccc-padding-to-length")
finally:
ctx.stop()
assert "live-key-aaaa-padding-to-length" in svc._cache
assert "stale-key-bbbb-padding-to-length" not in svc._cache
assert "new-key-cccc-padding-to-length" in svc._cache


class TestLogRedaction:
KEY = "sk-live-ABCDEFGHIJKLMNOPQRSTUVWXYZ"

def test_fingerprint_contains_no_key_characters_and_is_stable(self):
fp = ApiKeyService._fingerprint(self.KEY)
assert fp.startswith("sha256:")
assert self.KEY[:4] not in fp and self.KEY[-4:] not in fp
assert fp == ApiKeyService._fingerprint(self.KEY)
assert fp != ApiKeyService._fingerprint(self.KEY + "x")

@pytest.mark.asyncio
async def test_warning_on_auth_service_error_does_not_log_key_fragments(self):
# Assert on the logger call itself rather than captured text: other test modules
# reconfigure the "mcp-for-unity-server" logger, which makes caplog order-dependent.
svc = _make_service()
ctx, _ = _patched_client(_mock_response(500))
with patch("services.api_key_service.logger") as mock_logger:
try:
result = await svc.validate(self.KEY)
finally:
ctx.stop()
assert result.valid is False
assert mock_logger.warning.called
rendered = [
(call.args[0] % tuple(call.args[1:])) if len(call.args) > 1 else str(call.args[0])
for call in mock_logger.warning.call_args_list
]
assert any("API key validation returned status 500" in line for line in rendered)
for line in rendered:
assert self.KEY not in line
assert self.KEY[:4] not in line
assert self.KEY[-4:] not in line

@pytest.mark.asyncio
async def test_new_valid_key_evicts_a_negative_before_any_valid_entry(self, monkeypatch):
monkeypatch.setattr(ApiKeyService, "MAX_CACHE_ENTRIES", 3)
svc = _make_service()
ctx, instance = _patched_client(_mock_response(200, {"valid": True, "user_id": "u"}))
try:
await svc.validate("valid-key-aaaa-padding-to-length")
await svc.validate("valid-key-bbbb-padding-to-length")
instance.post = AsyncMock(return_value=_mock_response(401))
await svc.validate("bad-key-cccc-padding-to-length")
instance.post = AsyncMock(return_value=_mock_response(200, {"valid": True, "user_id": "u"}))
await svc.validate("valid-key-dddd-padding-to-length")
finally:
ctx.stop()
assert "bad-key-cccc-padding-to-length" not in svc._cache
assert "valid-key-aaaa-padding-to-length" in svc._cache
assert "valid-key-bbbb-padding-to-length" in svc._cache
assert "valid-key-dddd-padding-to-length" in svc._cache
Loading
Loading