Addresses review of the initial MCP HTTP server cut. Loopback bind alone
does not protect against browser-driven attacks — any page in the user's
browser is also on loopback. Adds a three-layer security gate, a body
size cap, a handler concurrency limit, and full HTTP-transport coverage.
Security
- McpHttpServer rejects requests with an Origin header (browsers always
set it; real MCP clients never do). Validates Host against loopback
names (DNS-rebinding pivot defense). Requires application/json on
POST so a cross-origin browser fetch must trigger a CORS preflight,
which we never satisfy.
- 4 MiB body cap with bounded streaming read (413 on overrun).
- 8-handler concurrency cap so a misbehaving local client cannot pin
every threadpool thread on long-running screen/camera calls.
- Settings migration: legacy McpOnlyMode=false no longer silently
inherits EnableNodeMode (would have flipped MCP on without consent).
Bridge
- _capabilities snapshotted to array per call (was racing concurrent
enumeration vs UI-thread mutation).
- Non-integer / out-of-range / string ids round-trip via GetRawText
instead of GetInt64 (used to strip the id from error responses on
fractional or big-int ids, breaking client correlation).
- tools/call validates 'arguments' is an object if present and rejects
empty 'name'.
- Generic 'internal error' on the wire for unhandled exceptions; full
exception with stack goes to the log via Error(string, Exception).
- Empty resources/list and prompts/list for Cursor compat (was
MethodNotFound).
- Non-object root → InvalidRequest.
Lifecycle
- Start() failure now disposes the half-constructed listener/CTS
instead of leaking the port reservation.
- _disposed guarded with Interlocked.Exchange (idempotent across
threads).
- App.xaml.cs warns when both modes are enabled but gateway
prerequisites are missing — silent fall-back to MCP-only was
confusing.
- NodeService.McpStartupError surfaces the actual failure to Settings
UI; status text shows 'Failed to start: <reason>' instead of the
misleading 'Stopped — save and restart to start'.
Refactor
- McpHttpServer moved from OpenClaw.Tray.WinUI/Services to
OpenClaw.Shared/Mcp (no WinUI deps; lets it be unit-tested).
Tests
- 10 new McpToolBridgeTests: non-object root, missing/non-string
params, non-object arguments, fractional/big-int/string ids,
empty resources/prompts, generic-message guarantee on internal
errors.
- New McpHttpServerTests (13): GET probe, valid POST, Origin reject,
rebind-Host reject, localhost-Host accept, text/plain reject,
json+charset accept, PUT reject, oversized body reject, notification
204, idempotent dispose, ctor null guards.
Docs
- MCP_MODE.md: rewritten Security Model section with the three-layer
model and curl-based 'verify the gate' examples; tool list updated
to reflect master (screen.snapshot, camera.clip, location.get).
All 715 tests pass (32 MCP, 20 integration, 663 other).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
86dad64f11
