Skip to content

Commit e9dac01

Browse files
authored
Harden HTTP defaults and improve stop-server fallback (#751)
* Harden HTTP defaults and improve stop-server fallback * Tighten toggle update behavior and IPv6 loopback coverage * Remove external scan reference from README security section * Improve host parsing and local URL policy reuse
1 parent 14bc7b8 commit e9dac01

13 files changed

Lines changed: 554 additions & 63 deletions

File tree

MCPForUnity/Editor/Constants/EditorPrefKeys.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ internal static class EditorPrefKeys
3030
internal const string GitUrlOverride = "MCPForUnity.GitUrlOverride";
3131
internal const string DevModeForceServerRefresh = "MCPForUnity.DevModeForceServerRefresh";
3232
internal const string ProjectScopedToolsLocalHttp = "MCPForUnity.ProjectScopedTools.LocalHttp";
33+
internal const string AllowLanHttpBind = "MCPForUnity.Security.AllowLanHttpBind";
34+
internal const string AllowInsecureRemoteHttp = "MCPForUnity.Security.AllowInsecureRemoteHttp";
3335

3436
internal const string PackageDeploySourcePath = "MCPForUnity.PackageDeploy.SourcePath";
3537
internal const string PackageDeployLastBackupPath = "MCPForUnity.PackageDeploy.LastBackupPath";

MCPForUnity/Editor/Helpers/HttpEndpointUtility.cs

Lines changed: 170 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System;
2+
using System.Net;
23
using MCPForUnity.Editor.Constants;
34
using MCPForUnity.Editor.Models;
45
using MCPForUnity.Editor.Services;
@@ -51,15 +52,15 @@ public static void SaveBaseUrl(string userValue)
5152
public static string GetLocalBaseUrl()
5253
{
5354
string stored = EditorPrefs.GetString(LocalPrefKey, DefaultLocalBaseUrl);
54-
return NormalizeBaseUrl(stored, DefaultLocalBaseUrl);
55+
return NormalizeBaseUrl(stored, DefaultLocalBaseUrl, remoteScope: false);
5556
}
5657

5758
/// <summary>
5859
/// Saves a user-provided URL to the local HTTP pref.
5960
/// </summary>
6061
public static void SaveLocalBaseUrl(string userValue)
6162
{
62-
string normalized = NormalizeBaseUrl(userValue, DefaultLocalBaseUrl);
63+
string normalized = NormalizeBaseUrl(userValue, DefaultLocalBaseUrl, remoteScope: false);
6364
EditorPrefs.SetString(LocalPrefKey, normalized);
6465
}
6566

@@ -74,7 +75,7 @@ public static string GetRemoteBaseUrl()
7475
{
7576
return DefaultRemoteBaseUrl;
7677
}
77-
return NormalizeBaseUrl(stored, DefaultRemoteBaseUrl);
78+
return NormalizeBaseUrl(stored, DefaultRemoteBaseUrl, remoteScope: true);
7879
}
7980

8081
/// <summary>
@@ -87,7 +88,7 @@ public static void SaveRemoteBaseUrl(string userValue)
8788
EditorPrefs.SetString(RemotePrefKey, DefaultRemoteBaseUrl);
8889
return;
8990
}
90-
string normalized = NormalizeBaseUrl(userValue, DefaultRemoteBaseUrl);
91+
string normalized = NormalizeBaseUrl(userValue, DefaultRemoteBaseUrl, remoteScope: true);
9192
EditorPrefs.SetString(RemotePrefKey, normalized);
9293
}
9394

@@ -146,10 +147,169 @@ public static ConfiguredTransport GetCurrentServerTransport()
146147
return IsRemoteScope() ? ConfiguredTransport.HttpRemote : ConfiguredTransport.Http;
147148
}
148149

150+
/// <summary>
151+
/// Returns true when advanced settings allow binding HTTP Local to all interfaces
152+
/// (e.g. 0.0.0.0 / ::). Disabled by default.
153+
/// </summary>
154+
public static bool AllowLanHttpBind()
155+
{
156+
return EditorPrefs.GetBool(EditorPrefKeys.AllowLanHttpBind, false);
157+
}
158+
159+
/// <summary>
160+
/// Returns true when advanced settings allow insecure HTTP/WS for remote endpoints.
161+
/// Disabled by default.
162+
/// </summary>
163+
public static bool AllowInsecureRemoteHttp()
164+
{
165+
return EditorPrefs.GetBool(EditorPrefKeys.AllowInsecureRemoteHttp, false);
166+
}
167+
168+
/// <summary>
169+
/// Returns true if the host is loopback-only.
170+
/// </summary>
171+
public static bool IsLoopbackHost(string host)
172+
{
173+
if (string.IsNullOrWhiteSpace(host))
174+
{
175+
return false;
176+
}
177+
178+
string normalized = host.Trim().Trim('[', ']').ToLowerInvariant();
179+
if (normalized == "localhost")
180+
{
181+
return true;
182+
}
183+
184+
if (IPAddress.TryParse(normalized, out IPAddress parsedIp))
185+
{
186+
return IPAddress.IsLoopback(parsedIp);
187+
}
188+
189+
return false;
190+
}
191+
192+
/// <summary>
193+
/// Returns true if the host is a bind-all-interfaces address.
194+
/// </summary>
195+
public static bool IsBindAllInterfacesHost(string host)
196+
{
197+
if (string.IsNullOrWhiteSpace(host))
198+
{
199+
return false;
200+
}
201+
202+
string normalized = host.Trim().Trim('[', ']').ToLowerInvariant();
203+
if (IPAddress.TryParse(normalized, out IPAddress parsedIp))
204+
{
205+
return parsedIp.Equals(IPAddress.Any) || parsedIp.Equals(IPAddress.IPv6Any);
206+
}
207+
208+
return false;
209+
}
210+
211+
/// <summary>
212+
/// Returns true when the URL host is acceptable for HTTP Local launch.
213+
/// Loopback is always allowed. Bind-all interfaces requires explicit opt-in.
214+
/// </summary>
215+
public static bool IsHttpLocalUrlAllowedForLaunch(string url, out string error)
216+
{
217+
error = null;
218+
if (string.IsNullOrWhiteSpace(url))
219+
{
220+
error = "HTTP Local requires a loopback URL (localhost/127.0.0.1/::1).";
221+
return false;
222+
}
223+
224+
if (!Uri.TryCreate(url, UriKind.Absolute, out var uri))
225+
{
226+
error = $"Invalid URL: {url}";
227+
return false;
228+
}
229+
230+
string host = uri.Host;
231+
if (IsLoopbackHost(host))
232+
{
233+
return true;
234+
}
235+
236+
if (IsBindAllInterfacesHost(host))
237+
{
238+
if (AllowLanHttpBind())
239+
{
240+
return true;
241+
}
242+
243+
error = "Binding to all interfaces (0.0.0.0/::) is disabled by default. " +
244+
"Enable \"Allow LAN bind for HTTP Local\" in Advanced Settings to opt in.";
245+
return false;
246+
}
247+
248+
error = "HTTP Local requires a loopback URL (localhost/127.0.0.1/::1).";
249+
return false;
250+
}
251+
252+
/// <summary>
253+
/// Returns true when remote URL is allowed by current security policy.
254+
/// HTTPS is required by default; HTTP needs explicit opt-in.
255+
/// </summary>
256+
public static bool IsRemoteUrlAllowed(string remoteBaseUrl, out string error)
257+
{
258+
error = null;
259+
if (string.IsNullOrWhiteSpace(remoteBaseUrl))
260+
{
261+
error = "HTTP Remote requires a configured URL.";
262+
return false;
263+
}
264+
265+
if (!Uri.TryCreate(remoteBaseUrl, UriKind.Absolute, out var uri))
266+
{
267+
error = $"Invalid HTTP Remote URL: {remoteBaseUrl}";
268+
return false;
269+
}
270+
271+
if (uri.Scheme.Equals("https", StringComparison.OrdinalIgnoreCase))
272+
{
273+
return true;
274+
}
275+
276+
if (uri.Scheme.Equals("http", StringComparison.OrdinalIgnoreCase))
277+
{
278+
if (AllowInsecureRemoteHttp())
279+
{
280+
return true;
281+
}
282+
283+
error = "HTTP Remote requires HTTPS by default. Enable \"Allow insecure HTTP for HTTP Remote\" in Advanced Settings to opt in.";
284+
return false;
285+
}
286+
287+
error = $"Unsupported URL scheme '{uri.Scheme}'. Use https:// (or http:// only with explicit insecure opt-in).";
288+
return false;
289+
}
290+
291+
/// <summary>
292+
/// Returns true when the currently configured remote URL satisfies security policy.
293+
/// </summary>
294+
public static bool IsCurrentRemoteUrlAllowed(out string error)
295+
{
296+
return IsRemoteUrlAllowed(GetRemoteBaseUrl(), out error);
297+
}
298+
299+
/// <summary>
300+
/// Human-readable host requirement for HTTP Local based on current security settings.
301+
/// </summary>
302+
public static string GetHttpLocalHostRequirementText()
303+
{
304+
return AllowLanHttpBind()
305+
? "localhost/127.0.0.1/::1/0.0.0.0/::"
306+
: "localhost/127.0.0.1/::1";
307+
}
308+
149309
/// <summary>
150310
/// Normalizes a URL so that we consistently store just the base (no trailing slash/path).
151311
/// </summary>
152-
private static string NormalizeBaseUrl(string value, string defaultUrl)
312+
private static string NormalizeBaseUrl(string value, string defaultUrl, bool remoteScope)
153313
{
154314
if (string.IsNullOrWhiteSpace(value))
155315
{
@@ -158,10 +318,13 @@ private static string NormalizeBaseUrl(string value, string defaultUrl)
158318

159319
string trimmed = value.Trim();
160320

161-
// Ensure scheme exists; default to http:// if user omitted it.
321+
// Ensure scheme exists.
322+
// For HTTP Remote, default to https:// to avoid accidental plaintext transport.
323+
// For HTTP Local, default to http:// for zero-friction local setup.
162324
if (!trimmed.Contains("://"))
163325
{
164-
trimmed = $"http://{trimmed}";
326+
string defaultScheme = remoteScope ? "https" : "http";
327+
trimmed = $"{defaultScheme}://{trimmed}";
165328
}
166329

167330
// Remove trailing slash segments.

MCPForUnity/Editor/Services/IServerManagementService.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public interface IServerManagementService
5858
/// <summary>
5959
/// Check if the local HTTP server can be started
6060
/// </summary>
61-
/// <returns>True if HTTP transport is enabled and URL is local</returns>
61+
/// <returns>True if HTTP transport is enabled and URL satisfies local launch security policy</returns>
6262
bool CanStartLocalServer();
6363
}
6464
}

MCPForUnity/Editor/Services/Server/ServerCommandBuilder.cs

Lines changed: 4 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,11 @@ public bool TryBuildCommand(out string fileName, out string arguments, out strin
3131
}
3232

3333
string httpUrl = HttpEndpointUtility.GetLocalBaseUrl();
34-
if (!IsLocalUrl(httpUrl))
34+
if (!HttpEndpointUtility.IsHttpLocalUrlAllowedForLaunch(httpUrl, out string localUrlError))
3535
{
36-
error = $"The configured URL ({httpUrl}) is not a local address. Local server launch only works for localhost.";
36+
error = string.IsNullOrEmpty(localUrlError)
37+
? $"The configured URL ({httpUrl}) is not allowed for HTTP Local launch."
38+
: $"{localUrlError} (configured URL: {httpUrl})";
3739
return false;
3840
}
3941

@@ -129,23 +131,5 @@ public string QuoteIfNeeded(string input)
129131
return input.IndexOf(' ') >= 0 ? $"\"{input}\"" : input;
130132
}
131133

132-
/// <summary>
133-
/// Check if a URL is local (localhost, 127.0.0.1, 0.0.0.0, ::1)
134-
/// </summary>
135-
private static bool IsLocalUrl(string url)
136-
{
137-
if (string.IsNullOrEmpty(url)) return false;
138-
139-
try
140-
{
141-
var uri = new Uri(url);
142-
string host = uri.Host.ToLower();
143-
return host == "localhost" || host == "127.0.0.1" || host == "0.0.0.0" || host == "::1";
144-
}
145-
catch
146-
{
147-
return false;
148-
}
149-
}
150134
}
151135
}

MCPForUnity/Editor/Services/ServerManagementService.cs

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -654,13 +654,32 @@ private bool StopLocalHttpServerInternal(bool quiet, int? portOverride = null, b
654654
}
655655
return false;
656656
}
657-
if (!quiet)
657+
658+
// If the pidfile PID is no longer the active listener, treat handshake state as stale
659+
// and continue with guarded port-based heuristics below.
660+
if (!pidIsListener)
658661
{
659-
McpLog.Warn(
660-
$"Refusing to stop port {port}: pidfile PID {pidFromFile} failed validation " +
661-
$"(listener={pidIsListener}, tokenMatch={tokenMatches}, tokenQueryOk={tokenQueryOk}).");
662+
if (!quiet)
663+
{
664+
McpLog.Warn(
665+
$"Stale pidfile for port {port}: pidfile PID {pidFromFile} is not the current listener " +
666+
$"(tokenMatch={tokenMatches}, tokenQueryOk={tokenQueryOk}). Falling back to guarded port heuristics.");
667+
}
668+
try { DeletePidFile(pidFilePath); } catch { }
669+
ClearLocalServerPidTracking();
670+
}
671+
else
672+
{
673+
// PID still owns the listener, but identity validation failed.
674+
// Fail closed to avoid terminating unrelated processes.
675+
if (!quiet)
676+
{
677+
McpLog.Warn(
678+
$"Refusing to stop port {port}: pidfile PID {pidFromFile} failed validation " +
679+
$"(listener={pidIsListener}, tokenMatch={tokenMatches}, tokenQueryOk={tokenQueryOk}).");
680+
}
681+
return false;
662682
}
663-
return false;
664683
}
665684
}
666685
}
@@ -875,7 +894,8 @@ public bool IsLocalUrl()
875894
}
876895

877896
/// <summary>
878-
/// Check if a URL is local (localhost, 127.0.0.1, 0.0.0.0)
897+
/// Check if a URL is local or bind-all (localhost/loopback and 0.0.0.0/::).
898+
/// This helper is intentionally broader than local-launch policy checks.
879899
/// </summary>
880900
private static bool IsLocalUrl(string url)
881901
{
@@ -884,8 +904,8 @@ private static bool IsLocalUrl(string url)
884904
try
885905
{
886906
var uri = new Uri(url);
887-
string host = uri.Host.ToLower();
888-
return host == "localhost" || host == "127.0.0.1" || host == "0.0.0.0" || host == "::1";
907+
string host = uri.Host;
908+
return HttpEndpointUtility.IsLoopbackHost(host) || HttpEndpointUtility.IsBindAllInterfacesHost(host);
889909
}
890910
catch
891911
{
@@ -899,7 +919,13 @@ private static bool IsLocalUrl(string url)
899919
public bool CanStartLocalServer()
900920
{
901921
bool useHttpTransport = EditorConfigurationCache.Instance.UseHttpTransport;
902-
return useHttpTransport && IsLocalUrl();
922+
if (!useHttpTransport)
923+
{
924+
return false;
925+
}
926+
927+
string httpUrl = HttpEndpointUtility.GetLocalBaseUrl();
928+
return HttpEndpointUtility.IsHttpLocalUrlAllowedForLaunch(httpUrl, out _);
903929
}
904930

905931
private System.Diagnostics.ProcessStartInfo CreateTerminalProcessStartInfo(string command)

MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,15 @@ public async Task<bool> StartAsync()
8686
? EditorPrefs.GetString(EditorPrefKeys.ApiKey, string.Empty)
8787
: string.Empty;
8888

89+
if (HttpEndpointUtility.IsRemoteScope()
90+
&& !HttpEndpointUtility.IsCurrentRemoteUrlAllowed(out string remoteUrlError))
91+
{
92+
string message = remoteUrlError ?? "HTTP Remote URL is not allowed by current security settings.";
93+
_state = TransportState.Disconnected(TransportDisplayName, message);
94+
McpLog.Error($"[WebSocket] {message}");
95+
return false;
96+
}
97+
8998
// Get project root path (strip /Assets from dataPath) for focus nudging
9099
string dataPath = Application.dataPath;
91100
if (!string.IsNullOrEmpty(dataPath))

0 commit comments

Comments
 (0)