Harden HTTP defaults and improve stop-server fallback#751
Merged
dsarno merged 4 commits intoCoplayDev:betafrom Feb 14, 2026
Merged
Harden HTTP defaults and improve stop-server fallback#751dsarno merged 4 commits intoCoplayDev:betafrom
dsarno merged 4 commits intoCoplayDev:betafrom
Conversation
Contributor
Reviewer's GuideCentralizes HTTP URL security policy, introduces explicit editor preferences and UI gating for risky HTTP configurations (LAN bind and insecure remote HTTP), hardens local/remote launch logic, improves stale pidfile handling when stopping the local HTTP server, and updates characterization tests and README security documentation. Sequence diagram for HTTP Local server start with security policy gatingsequenceDiagram
actor UnityUser
participant McpConnectionSection
participant HttpEndpointUtility
participant ServerManagementService as MCPServiceLocator_Server
UnityUser->>McpConnectionSection: Click HTTP server toggle (start)
McpConnectionSection->>HttpEndpointUtility: GetLocalBaseUrl()
McpConnectionSection->>HttpEndpointUtility: IsHttpLocalUrlAllowedForLaunch(localBaseUrl, out error)
alt URL blocked by policy
HttpEndpointUtility-->>McpConnectionSection: false, error
McpConnectionSection->>UnityUser: Show dialog "Cannot Start HTTP Server"
McpConnectionSection->>McpConnectionSection: Log warning and return
else URL allowed
HttpEndpointUtility-->>McpConnectionSection: true
McpConnectionSection->>MCPServiceLocator_Server: StartLocalHttpServer()
alt server started
MCPServiceLocator_Server-->>McpConnectionSection: true
McpConnectionSection->>McpConnectionSection: Auto-start session
else server failed
MCPServiceLocator_Server-->>McpConnectionSection: false
McpConnectionSection->>UnityUser: Show error status
end
end
Sequence diagram for HTTP Remote connection start with URL policy gatingsequenceDiagram
actor UnityUser
participant McpConnectionSection
participant HttpEndpointUtility
participant WebSocketTransportClient
UnityUser->>McpConnectionSection: Click Start Session
McpConnectionSection->>McpConnectionSection: Determine selected transport
alt HTTP Remote selected
McpConnectionSection->>HttpEndpointUtility: IsCurrentRemoteUrlAllowed(out error)
alt remote URL blocked
HttpEndpointUtility-->>McpConnectionSection: false, error
McpConnectionSection->>UnityUser: Show dialog "Connection Blocked"
McpConnectionSection->>McpConnectionSection: Log warning and return
else remote URL allowed
HttpEndpointUtility-->>McpConnectionSection: true
McpConnectionSection->>WebSocketTransportClient: StartAsync()
end
else other transport
McpConnectionSection->>WebSocketTransportClient: StartAsync()
end
alt remote scope inside StartAsync
WebSocketTransportClient->>HttpEndpointUtility: IsRemoteScope()
WebSocketTransportClient->>HttpEndpointUtility: IsCurrentRemoteUrlAllowed(out error)
alt blocked by policy
HttpEndpointUtility-->>WebSocketTransportClient: false, error
WebSocketTransportClient->>WebSocketTransportClient: Set state Disconnected
WebSocketTransportClient->>McpConnectionSection: return false
else allowed
HttpEndpointUtility-->>WebSocketTransportClient: true
WebSocketTransportClient->>WebSocketTransportClient: Open WebSocket
WebSocketTransportClient-->>McpConnectionSection: return true
end
end
Updated class diagram for HTTP endpoint security utilities and preferencesclassDiagram
class HttpEndpointUtility {
+string GetLocalBaseUrl()
+void SaveLocalBaseUrl(string userValue)
+string GetRemoteBaseUrl()
+void SaveRemoteBaseUrl(string userValue)
+ConfiguredTransport GetCurrentServerTransport()
+bool AllowLanHttpBind()
+bool AllowInsecureRemoteHttp()
+bool IsLoopbackHost(string host)
+bool IsBindAllInterfacesHost(string host)
+bool IsHttpLocalUrlAllowedForLaunch(string url, string error)
+bool IsRemoteUrlAllowed(string remoteBaseUrl, string error)
+bool IsCurrentRemoteUrlAllowed(string error)
+string GetHttpLocalHostRequirementText()
-string NormalizeBaseUrl(string value, string defaultUrl, bool remoteScope)
}
class EditorPrefKeys {
<<static>>
+string ProjectScopedToolsLocalHttp
+string AllowLanHttpBind
+string AllowInsecureRemoteHttp
}
HttpEndpointUtility --> EditorPrefKeys : uses
Updated class diagram for connection and advanced editor sectionsclassDiagram
class MCPForUnityEditorWindow {
+void CreateGUI()
}
class McpAdvancedSection {
-Toggle allowLanHttpBindToggle
-Toggle allowInsecureRemoteHttpToggle
+event Action OnGitUrlChanged
+event Action OnHttpServerCommandUpdateRequested
+event Action OnTestConnectionRequested
+void CacheUIElements()
+void InitializeUI()
+void RegisterCallbacks()
+void UpdatePathOverrides()
+void UpdateHealthStatus(bool isHealthy, string statusText)
}
class McpConnectionSection {
+void UpdateConnectionStatus()
+void UpdateHttpServerCommandDisplay()
+void UpdateStartHttpButtonState()
+void SetHealthStatusUpdateCallback(Action callback)
+bool IsHttpLocalSelected()
-async void OnHttpServerToggleClicked()
-async void OnConnectionToggleClicked()
}
class HttpEndpointUtility {
+bool IsHttpLocalUrlAllowedForLaunch(string url, string error)
+bool IsCurrentRemoteUrlAllowed(string error)
+string GetLocalBaseUrl()
+string GetHttpLocalHostRequirementText()
}
MCPForUnityEditorWindow --> McpAdvancedSection : creates
MCPForUnityEditorWindow --> McpConnectionSection : creates
McpAdvancedSection --> McpConnectionSection : OnHttpServerCommandUpdateRequested
McpConnectionSection --> HttpEndpointUtility : URL policy checks
Updated class diagram for server management and transport policy integrationclassDiagram
class ServerManagementService {
+bool StopLocalHttpServerInternal(bool quiet, int? portOverride, bool force)
+bool IsLocalUrl()
+bool CanStartLocalServer()
-static bool IsLocalUrl(string url)
-void ClearLocalServerPidTracking()
-void DeletePidFile(string pidFilePath)
}
class IServerManagementService {
<<interface>>
+bool CanStartLocalServer()
}
class ServerCommandBuilder {
+bool TryBuildCommand(string fileName, string arguments, string error)
+string QuoteIfNeeded(string input)
}
class WebSocketTransportClient {
+Task~bool~ StartAsync()
}
class HttpEndpointUtility {
+bool IsLoopbackHost(string host)
+bool IsBindAllInterfacesHost(string host)
+bool IsHttpLocalUrlAllowedForLaunch(string url, string error)
+bool IsCurrentRemoteUrlAllowed(string error)
+string GetLocalBaseUrl()
}
class EditorConfigurationCache {
<<singleton>>
+EditorConfigurationCache Instance
+bool UseHttpTransport
}
ServerManagementService ..|> IServerManagementService
ServerManagementService --> HttpEndpointUtility : launch policy
ServerCommandBuilder --> HttpEndpointUtility : validate HTTP Local URL
WebSocketTransportClient --> HttpEndpointUtility : validate HTTP Remote URL
ServerManagementService --> EditorConfigurationCache : checks UseHttpTransport
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
📝 WalkthroughWalkthroughAdds editor security controls and enforcement for HTTP endpoints: two new EditorPrefs keys and UI toggles, extended HttpEndpointUtility host/URL policy checks, updated server management and transport startup flows to enforce local/remote HTTP policies, and corresponding unit tests and docs updates. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Editor UI
participant Service as ServerManagementService
participant Transport as WebSocketTransportClient
participant Util as HttpEndpointUtility
UI->>Service: request start local server (reads prefs & URL)
Service->>Util: IsHttpLocalUrlAllowedForLaunch(localUrl)
Util-->>Service: allowed / error
alt allowed
Service-->>UI: start server (proceed)
else blocked
Service-->>UI: show error dialog / tooltip
end
UI->>Transport: start connection
Transport->>Util: IsCurrentRemoteUrlAllowed()
Util-->>Transport: allowed / error
alt allowed
Transport-->>Service: proceed startup
else blocked
Transport-->>UI: set Disconnected + log error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The loopback/bind-all checks in
HttpEndpointUtility.IsLoopbackHost/IsBindAllInterfacesHostrely on a small set of string literals; consider usingIPAddress.TryParseplusIPAddress.IsLoopback/ checkingIPAddress.Any/IPv6Anyto robustly handle equivalent representations and avoid edge cases around IPv6 formatting. - There are several call sites that recompute
HttpEndpointUtility.GetLocalBaseUrl()and then immediately callIsHttpLocalUrlAllowedForLaunch(e.g., inMcpConnectionSection.UpdateHttpServerCommandDisplay,UpdateStartHttpButtonState,OnHttpServerToggleClicked); you could factor this into a small helper that returns(bool allowed, string error, string url)to reduce repetition and ensure consistent behavior/tooltips.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The loopback/bind-all checks in `HttpEndpointUtility.IsLoopbackHost` / `IsBindAllInterfacesHost` rely on a small set of string literals; consider using `IPAddress.TryParse` plus `IPAddress.IsLoopback` / checking `IPAddress.Any` / `IPv6Any` to robustly handle equivalent representations and avoid edge cases around IPv6 formatting.
- There are several call sites that recompute `HttpEndpointUtility.GetLocalBaseUrl()` and then immediately call `IsHttpLocalUrlAllowedForLaunch` (e.g., in `McpConnectionSection.UpdateHttpServerCommandDisplay`, `UpdateStartHttpButtonState`, `OnHttpServerToggleClicked`); you could factor this into a small helper that returns `(bool allowed, string error, string url)` to reduce repetition and ensure consistent behavior/tooltips.
## Individual Comments
### Comment 1
<location> `README.md:210` </location>
<code_context>
+
+Network defaults are intentionally fail-closed:
+* **HTTP Local** allows loopback-only hosts by default (`127.0.0.1`, `localhost`, `::1`).
+* Bind-all interfaces (`0.0.0.0`, `::`) requires explicit opt-in in **Advanced Settings** via **Allow LAN Bind (HTTP Local)**.
+* **HTTP Remote** requires `https://` by default.
+* Plaintext `http://` for remote endpoints requires explicit opt-in via **Allow Insecure Remote HTTP**.
</code_context>
<issue_to_address>
**suggestion (typo):** Consider changing "requires" to "require" to match the plural subject "interfaces".
Because "interfaces" is plural, the verb should also be plural: "Bind-all interfaces (`0.0.0.0`, `::`) require explicit opt-in in **Advanced Settings** via **Allow LAN Bind (HTTP Local)**."
```suggestion
* Bind-all interfaces (`0.0.0.0`, `::`) require explicit opt-in in **Advanced Settings** via **Allow LAN Bind (HTTP Local)**.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Issue Coverage
Validation
Summary by Sourcery
Harden HTTP endpoint security defaults and enforce URL policy checks for local and remote connections, while improving local server stop behavior when pidfiles are stale.
New Features:
Enhancements:
Documentation:
Tests:
Summary by CodeRabbit
New Features
UI Changes
Bug Fixes
Tests
Documentation