fix(mcp): 修复配置导入、凭据管理与协议边界

修复生命周期锁阻塞事件循环、旧连接回调误停新连接及超时契约不一致。

补齐 MCP JSON 兼容导入、密钥拆分与失败重试,修复 Header 大小写草稿丢失,迁移大小写敏感的环境变量凭据。

在 SSE 行拼接前限制缓冲大小,增加并发、迁移和流式输入回归测试;忽略本机 MCP 数据及 server.json/servers.json。

验证:后端 185 项、前端 54 项测试通过,前端生产构建、相关文件 Ruff 与暂存差异检查通过。
This commit is contained in:
2026-09-03 22:38:35 +08:00
parent 2f7066aa92
commit 9b8b10cdb1
13 changed files with 1097 additions and 80 deletions
+180 -6
View File
@@ -2,6 +2,8 @@ import asyncio
import threading
from types import SimpleNamespace
import pytest
from app.contracts import (
McpServerSecretStatus,
McpServerSecretWriteRequest,
@@ -61,9 +63,7 @@ def test_mcp_secret_routes_offload_blocking_lifecycle_work(monkeypatch) -> None:
)
)
deleted = asyncio.run(
routes.delete_mcp_server_secret(
"server-1", "TOKEN", kind="environment"
)
routes.delete_mcp_server_secret("server-1", "TOKEN", kind="environment")
)
assert written.configured is True
@@ -77,6 +77,172 @@ def test_health() -> None:
assert response.model_dump() == {"status": "ok"}
def test_mcp_create_and_trust_are_not_executed_on_event_loop(monkeypatch) -> None:
from app import routes
from app.contracts import McpServerCreateRequest, McpServerTrustRequest
caller = threading.get_ident()
workers = []
class Registry:
def create(self, request):
workers.append(threading.get_ident())
return "created"
def trust(self, server_id, digest):
workers.append(threading.get_ident())
return "trusted"
monkeypatch.setattr(routes, "container", SimpleNamespace(mcp_servers=Registry()))
assert (
asyncio.run(
routes.create_mcp_server(McpServerCreateRequest(name="test", command="uvx"))
)
== "created"
)
assert (
asyncio.run(
routes.trust_mcp_server(
"test", McpServerTrustRequest(command_digest="a" * 64)
)
)
== "trusted"
)
assert len(workers) == 2
assert all(worker != caller for worker in workers)
def test_mcp_split_config_and_secret_requests_persist_without_plaintext(
monkeypatch,
) -> None:
from fastapi.testclient import TestClient
from app import routes
from app.agent.tools import ToolRegistry
from app.config import get_settings
from app.extensions.mcp_registry import McpServerRegistry
from app.main import app
from app.providers.credentials import EncryptedCredentialStore
service = McpServerRegistry(
ToolRegistry(),
EncryptedCredentialStore(),
get_settings().data_dir,
allow_process_launch=True,
)
monkeypatch.setattr(routes, "container", SimpleNamespace(mcp_servers=service))
client = TestClient(app)
config = {
"name": "MiniMax configuration test",
"command": "uvx",
"environment": {"MINIMAX_API_HOST": "https://api.minimaxi.com"},
"secret_environment_keys": ["MINIMAX_API_KEY"],
"startup_timeout_seconds": 120,
"tool_timeout_seconds": 300,
}
# Reproduce the old frontend payload. The backend still enforces separation.
invalid = client.post(
"/api/mcp/servers",
json={
**config,
"environment": {
**config["environment"],
"MINIMAX_API_KEY": "synthetic-only",
},
},
)
assert invalid.status_code == 422
assert invalid.json()["error"]["code"] == "MCP_ENVIRONMENT_INVALID"
created = client.post("/api/mcp/servers", json=config)
assert created.status_code == 201
server_id = created.json()["server_id"]
saved = client.put(
f"/api/mcp/servers/{server_id}/secrets/MINIMAX_API_KEY",
json={"secret": "synthetic-only"},
)
assert saved.status_code == 200
current = client.get(f"/api/mcp/servers/{server_id}")
assert current.json()["secret_environment"] == {"MINIMAX_API_KEY": True}
assert "synthetic-only" not in current.text
assert "synthetic-only" not in service._path.read_text(encoding="utf-8")
_, credentials_path = service.credentials._paths()
assert "synthetic-only" not in credentials_path.read_text(encoding="utf-8")
assert not current.json()["enabled"] # Saving never starts a third-party process.
client.close()
@pytest.mark.parametrize("operation", ["create", "trust"])
def test_mcp_lifecycle_lock_contention_keeps_event_loop_responsive(
monkeypatch,
operation,
) -> None:
from app import routes
from app.agent.tools import ToolRegistry
from app.config import get_settings
from app.contracts import McpServerCreateRequest, McpServerTrustRequest
from app.extensions.mcp_registry import McpServerRegistry
from app.providers.credentials import EncryptedCredentialStore
service = McpServerRegistry(
ToolRegistry(),
EncryptedCredentialStore(),
get_settings().data_dir,
allow_process_launch=True,
)
request = McpServerCreateRequest(
name="Lock contention fixture", command="not-executed"
)
server = service.create(request)
monkeypatch.setattr(routes, "container", SimpleNamespace(mcp_servers=service))
entered = threading.Event()
locked = threading.Event()
release = threading.Event()
original = getattr(service, operation)
def observed(*args):
entered.set()
return original(*args)
def hold_lifecycle_lock():
with service._lifecycle_lock:
locked.set()
release.wait(timeout=5)
monkeypatch.setattr(service, operation, observed)
holder = threading.Thread(target=hold_lifecycle_lock, daemon=True)
holder.start()
# An independent watchdog lets the test fail rather than hang if a regression
# blocks the event loop itself (an asyncio timeout alone cannot catch that).
watchdog = threading.Timer(5, release.set)
watchdog.start()
async def exercise():
pending = asyncio.create_task(
routes.create_mcp_server(request)
if operation == "create"
else routes.trust_mcp_server(
server.server_id,
McpServerTrustRequest(command_digest=server.command_digest),
)
)
try:
assert await asyncio.to_thread(entered.wait, 2)
assert not pending.done()
assert not release.is_set()
assert (await health()).status == "ok"
finally:
release.set()
await pending
try:
assert locked.wait(timeout=2)
asyncio.run(exercise())
finally:
release.set()
watchdog.cancel()
holder.join(timeout=2)
def test_service_status() -> None:
response = asyncio.run(service_status())
@@ -93,7 +259,9 @@ def test_core_collections_are_typed() -> None:
assert notes.items == []
assert notes.page.limit == 20
assert [skill.manifest.skill_id for skill in skills.items] == ["knowledge-assistant"]
assert [skill.manifest.skill_id for skill in skills.items] == [
"knowledge-assistant"
]
assert skills.items[0].status == "ready"
assert [plugin.manifest.plugin_id for plugin in plugins.items] == ["text-tools"]
assert plugins.items[0].status == "ready"
@@ -114,9 +282,15 @@ def test_provider_presets_include_openai_and_deepseek() -> None:
def test_provider_presets_static_route_precedes_provider_id_route() -> None:
from app.routes import router
get_paths = [route.path for route in router.routes if "GET" in getattr(route, "methods", set())]
get_paths = [
route.path
for route in router.routes
if "GET" in getattr(route, "methods", set())
]
assert get_paths.index("/api/providers/presets") < get_paths.index("/api/providers/{provider_id}")
assert get_paths.index("/api/providers/presets") < get_paths.index(
"/api/providers/{provider_id}"
)
def test_openapi_contains_documented_frontend_interfaces() -> None:
+212 -7
View File
@@ -1,4 +1,5 @@
import asyncio
import hashlib
import json
import sys
import threading
@@ -111,7 +112,9 @@ def test_update_disables_server_and_revokes_command_trust() -> None:
service.shutdown()
def test_update_remains_retryable_when_removed_secret_cleanup_fails(monkeypatch) -> None:
def test_update_remains_retryable_when_removed_secret_cleanup_fails(
monkeypatch,
) -> None:
service = registry()
created = service.create(request())
service.put_secret(created.server_id, "TEST_MCP_SECRET", "keep-until-retry")
@@ -166,7 +169,9 @@ def test_unavailable_server_removes_bridge_host(monkeypatch) -> None:
removed: list[str] = []
monkeypatch.setattr(service.bridge, "remove", removed.append)
service._unavailable(created.server_id, "connection lost")
generation = object()
service._generations[created.server_id] = generation
service._unavailable(created.server_id, generation, "connection lost")
current = service.get(created.server_id)
assert removed == [f"mcp.{created.server_id}"]
@@ -175,6 +180,180 @@ def test_unavailable_server_removes_bridge_host(monkeypatch) -> None:
service.shutdown()
def test_old_failure_callback_cannot_stop_replacement_host(monkeypatch) -> None:
service = registry()
callbacks = []
original_start = service.bridge.start
def capture_callback(*args, **kwargs):
callbacks.append(args[4])
return original_start(*args, **kwargs)
monkeypatch.setattr(service.bridge, "start", capture_callback)
created = service.create(request(secret_environment_keys=[]))
service.trust(created.server_id, created.command_digest)
callback_thread = None
try:
service.test(created.server_id)
service.enable(created.server_id)
old_callback = callbacks[-1]
callback_started = threading.Event()
callback_finished = threading.Event()
def delayed_failure():
callback_started.set()
old_callback(f"mcp.{created.server_id}", "delayed old failure")
callback_finished.set()
# Queue the old callback while a replacement owns the lifecycle lock.
with service._lifecycle_lock:
callback_thread = threading.Thread(target=delayed_failure, daemon=True)
callback_thread.start()
assert callback_started.wait(timeout=2)
service.disable(created.server_id)
service.enable(created.server_id)
assert callback_finished.wait(timeout=2)
assert service.get(created.server_id).enabled is True
assert service.get(created.server_id).status == "ready"
assert service.tools.definitions()
callbacks[-1](f"mcp.{created.server_id}", "current failure")
assert service.get(created.server_id).enabled is False
assert service.get(created.server_id).status == "unhealthy"
finally:
service.shutdown()
if callback_thread is not None:
callback_thread.join(timeout=2)
def test_header_case_only_rename_preserves_secret() -> None:
service = registry()
config = {
"name": "HTTP",
"transport": "streamable_http",
"url": "https://example.test/mcp",
"secret_header_keys": ["Authorization"],
}
created = service.create(McpServerCreateRequest(**config))
service.put_secret(created.server_id, "Authorization", "synthetic", kind="header")
config["secret_header_keys"] = ["authorization"]
updated = service.update(
created.server_id, McpServerUpdateRequest(**config, version=created.version)
)
assert updated.secret_headers == {"authorization": True}
assert (
service.credentials.resolve(
service._secret_id(created.server_id, "authorization", "header")
)
== "synthetic"
)
def test_environment_secrets_are_case_sensitive_and_delete_independently() -> None:
service = registry()
created = service.create(request(secret_environment_keys=["TOKEN", "token"]))
service.put_secret(created.server_id, "TOKEN", "upper")
service.put_secret(created.server_id, "token", "lower")
assert (
service.credentials.resolve(service._secret_id(created.server_id, "TOKEN"))
== "upper"
)
assert (
service.credentials.resolve(service._secret_id(created.server_id, "token"))
== "lower"
)
service.delete_secret(created.server_id, "TOKEN")
assert service.get(created.server_id).secret_environment == {
"TOKEN": False,
"token": True,
}
def test_legacy_environment_credential_migration_is_idempotent() -> None:
service = registry()
created = service.create(request(secret_environment_keys=["TOKEN"]))
suffix = hashlib.sha256(b"environment\0token").hexdigest()[:20]
legacy_id = f"mcp.{created.server_id}.{suffix}"
service.credentials.put(legacy_id, "legacy-value")
service._records[created.server_id]["secret_environment_version"] = 1
service._write()
migrated = registry()
assert migrated.get(created.server_id).secret_environment == {"TOKEN": True}
assert (
migrated.credentials.resolve(migrated._secret_id(created.server_id, "TOKEN"))
== "legacy-value"
)
assert not migrated.credentials.has(legacy_id)
migrated.put_secret(created.server_id, "TOKEN", "new-value")
assert (
registry().credentials.resolve(migrated._secret_id(created.server_id, "TOKEN"))
== "new-value"
)
def test_ambiguous_legacy_credentials_are_not_assigned_to_two_variables() -> None:
service = registry()
created = service.create(request(secret_environment_keys=["TOKEN", "token"]))
suffix = hashlib.sha256(b"environment\0token").hexdigest()[:20]
legacy_id = f"mcp.{created.server_id}.{suffix}"
service.credentials.put(legacy_id, "cannot-reconstruct-originals")
service._records[created.server_id]["secret_environment_version"] = 1
service._write()
migrated = registry()
current = migrated.get(created.server_id)
assert current.secret_environment == {"TOKEN": False, "token": False}
assert current.enabled is False
assert current.last_test_succeeded is None
assert migrated.credentials.has(
legacy_id
) # Keep the original ciphertext recoverable.
migrated.put_secret(created.server_id, "TOKEN", "upper")
migrated.put_secret(created.server_id, "token", "lower")
assert registry().get(created.server_id).secret_environment == {
"TOKEN": True,
"token": True,
}
migrated.delete(created.server_id)
assert not migrated.credentials.has(legacy_id)
def test_credential_id_migration_keeps_new_values_and_is_atomic(monkeypatch) -> None:
credentials = EncryptedCredentialStore()
credentials.put("mcp.old", "old-value")
credentials.put("mcp.new", "new-value")
original_write = credentials._write_tokens
def fail_write(_tokens):
raise CredentialStoreError("synthetic failure")
monkeypatch.setattr(credentials, "_write_tokens", fail_write)
with pytest.raises(CredentialStoreError):
credentials.move_many({"mcp.old": "mcp.new"})
assert credentials.resolve("mcp.old") == "old-value"
assert credentials.resolve("mcp.new") == "new-value"
monkeypatch.setattr(credentials, "_write_tokens", original_write)
credentials.move_many({"mcp.old": "mcp.new"})
assert credentials.resolve("mcp.old") is None
assert credentials.resolve("mcp.new") == "new-value"
def test_ambiguous_legacy_secret_is_not_resurrected_after_removing_a_key() -> None:
service = registry()
created = service.create(request(secret_environment_keys=["TOKEN", "token"]))
legacy_id = service._legacy_environment_secret_id(created.server_id, "TOKEN")
service.credentials.put(legacy_id, "ambiguous-old-value")
service._records[created.server_id]["secret_environment_version"] = 1
service._write()
migrated = registry()
migrated.update(
created.server_id,
McpServerUpdateRequest(
**request(secret_environment_keys=["token"]).model_dump(),
version=created.version,
),
)
assert registry().get(created.server_id).secret_environment == {"token": False}
def test_production_rejects_process_launch_even_after_approval() -> None:
service = registry(launch=False)
created = service.create(request(secret_environment_keys=[]))
@@ -184,6 +363,29 @@ def test_production_rejects_process_launch_even_after_approval() -> None:
assert error.value.code == "MCP_SANDBOX_REQUIRED"
@pytest.mark.parametrize("startup,tool", [(120, 300), (1.5, 2.5)])
def test_server_timeouts_survive_bridge_adaptation_and_reload(startup, tool) -> None:
service = registry()
created = service.create(
request(
secret_environment_keys=[],
startup_timeout_seconds=startup,
tool_timeout_seconds=tool,
)
)
service.trust(created.server_id, created.command_digest)
try:
tested = service.test(created.server_id)
assert tested.last_test_succeeded is True
assert tested.startup_timeout_seconds == startup
assert tested.tool_timeout_seconds == tool
restored = registry().get(created.server_id)
assert restored.startup_timeout_seconds == startup
assert restored.tool_timeout_seconds == tool
finally:
service.shutdown()
def test_enable_requires_successful_test_and_update_checks_version() -> None:
service = registry()
created = service.create(request(secret_environment_keys=[]))
@@ -307,15 +509,18 @@ def test_lifecycle_operations_are_serialized_and_tool_names_are_isolated() -> No
service.test(server.server_id)
with ThreadPoolExecutor(max_workers=4) as pool:
enabled = list(pool.map(lambda item: service.enable(item.server_id), servers * 2))
enabled = list(
pool.map(lambda item: service.enable(item.server_id), servers * 2)
)
assert all(item.enabled for item in enabled)
names = [
item.name
for item in service.tools.definitions()
if item.source == "mcp_server"
item.name for item in service.tools.definitions() if item.source == "mcp_server"
]
assert len(names) == len(set(names))
assert all(any(name.startswith(f"mcp.{item.server_id}.") for name in names) for item in servers)
assert all(
any(name.startswith(f"mcp.{item.server_id}.") for name in names)
for item in servers
)
with ThreadPoolExecutor(max_workers=4) as pool:
list(pool.map(lambda item: service.disable(item.server_id), servers * 2))
+83
View File
@@ -0,0 +1,83 @@
import json
from contextlib import closing
import httpx
import pytest
from app.extensions import mcp
class ChunkStream(httpx.SyncByteStream):
def __init__(self, chunks):
self.chunks = chunks
self.bytes_read = 0
def __iter__(self):
for chunk in self.chunks:
self.bytes_read += len(chunk)
yield chunk
def test_sse_rejects_unterminated_line_before_reading_entire_stream(monkeypatch):
monkeypatch.setattr(mcp, "MAX_MCP_MESSAGE_BYTES", 1024)
stream = ChunkStream([b"x" * 256] * 256)
with (
closing(httpx.Response(200, stream=stream)) as response,
pytest.raises(mcp.McpBridgeError, match="too large"),
):
list(mcp._iter_sse(response))
assert stream.bytes_read == 1280
def test_sse_limits_combined_event_before_partial_line_is_complete(monkeypatch):
monkeypatch.setattr(mcp, "MAX_MCP_MESSAGE_BYTES", 32)
stream = ChunkStream(
[b"data: 123456789\n", b"data: 123456789\n", b"x", b"not-read"]
)
with (
closing(httpx.Response(200, stream=stream)) as response,
pytest.raises(mcp.McpBridgeError, match="too large"),
):
list(mcp._iter_sse(response))
assert stream.bytes_read == 33
@pytest.mark.parametrize("separator", [b"\n", b"\r", b"\r\n"])
@pytest.mark.parametrize("chunk_size", [1, 2, 7, 1024])
def test_sse_preserves_utf8_and_line_endings_across_chunks(separator, chunk_size):
payload = json.dumps(
{"jsonrpc": "2.0", "id": 1, "result": {"text": "中文"}}, ensure_ascii=False
)
wire = b"\xef\xbb\xbf" + separator.join(
[
b": heartbeat",
b"event: message",
b"id: replay-1",
("data: " + payload).encode(),
b"",
b"",
]
)
stream = ChunkStream(
[wire[index : index + chunk_size] for index in range(0, len(wire), chunk_size)]
)
with closing(httpx.Response(200, stream=stream)) as response:
assert list(mcp._iter_sse(response)) == [("message", "replay-1", payload)]
def test_sse_event_limit_resets_between_events(monkeypatch):
monkeypatch.setattr(mcp, "MAX_MCP_MESSAGE_BYTES", 16)
with closing(
httpx.Response(200, stream=ChunkStream([b"data: one\n\ndata: two\r\r"]))
) as response:
assert list(mcp._iter_sse(response)) == [
("message", None, "one"),
("message", None, "two"),
]
def test_sse_preserves_multiline_data_and_final_unterminated_line():
with closing(
httpx.Response(200, stream=ChunkStream([b"data: first\ndata: last"]))
) as response:
assert list(mcp._iter_sse(response)) == [("message", None, "first\nlast")]