diff --git a/CHANGELOG.md b/CHANGELOG.md index 24e8939..1c17088 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,22 @@ All notable changes to DotNetDevMCP are documented here. The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); versions follow [SemVer](https://semver.org/). +## [Unreleased] + +### Security +- **`--http` DNS rebinding / cross-origin protection.** Per the MCP Streamable HTTP transport's security guidance, the server + now validates the `Origin` header on every request that carries one: only `http://localhost:`, + `http://127.0.0.1:` and `http://[::1]:` (the server's own port) are accepted, plus any exact value passed via + the new repeatable `--allowed-origin ` option (validated at startup: must be a bare `http`/`https` origin, no + path/query/fragment/userinfo/wildcard). Everything else, including other localhost ports and `https://` origins not + explicitly allow-listed, gets a 403. A request whose `Host` header doesn't name this machine's loopback interface + (`localhost`, `127.0.0.1`, `[::1]`) also gets a 403 (DNS rebinding defense). Requests without an `Origin` header - every + non-browser MCP client, and a browser's simple GET/HEAD - are not rejected by this check; they still only get whatever + the MCP endpoint itself returns for that request (typically 404/405 outside a POST). The server sends no CORS headers, + so `--allowed-origin` does not let a browser page call it directly from that origin - it's for a local dev-server proxy + that forwards the original `Origin`, or a non-browser client that happens to set one. `--http` still has no + authentication or TLS and still shouldn't be exposed beyond localhost. + ## [0.3.3] - 2026-09-24 Prompted by an external evaluation; each claim was checked against the code first. diff --git a/README.md b/README.md index dca924a..624828c 100644 --- a/README.md +++ b/README.md @@ -55,7 +55,7 @@ claude mcp add dotnetdevmcp -- dnx DotNetDevMCP --yes `dnx` downloads the package from NuGet.org on first run. Prefer a permanent install? `dotnet tool install -g DotNetDevMCP`, then use `dotnetdevmcp` as the command. -Pass `--load-solution ` to have Roslyn load your solution at startup, or let the agent call `SharpTool_LoadSolution` when it needs to. `--http --port 3001` serves Streamable HTTP instead of stdio (localhost only, no authentication: see [Security](#security)). `--clean-env` starts `dotnet` and `git` with a minimal environment so tokens and cloud credentials in environment variables aren't passed on. `dotnetdevmcp --help` lists everything. +Pass `--load-solution ` to have Roslyn load your solution at startup, or let the agent call `SharpTool_LoadSolution` when it needs to. `--http --port 3001` serves Streamable HTTP instead of stdio (localhost only, no authentication: see [Security](#security)); it rejects requests carrying a foreign `Origin` header or a non-localhost `Host` header. `--allowed-origin ` (repeatable) adds an extra origin to that allow-list, e.g. `--allowed-origin http://localhost:5173` for a local dev-server proxy that forwards its `Origin` - the server sends no CORS headers, so this doesn't let a browser page call it directly. `--clean-env` starts `dotnet` and `git` with a minimal environment so tokens and cloud credentials in environment variables aren't passed on. `dotnetdevmcp --help` lists everything. Git and Monitoring tools (see the table below) are off by default - a shell an agent already has covers them, and every registered tool costs context tokens in every session. Pass `--enable git,monitoring` (comma-separated and/or repeated, e.g. `--enable git --enable monitoring`) to turn either or both on. diff --git a/SECURITY.md b/SECURITY.md index 2c69deb..df8f5e2 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -73,6 +73,7 @@ can't add options such as `-p:CustomBeforeMicrosoftCommonTargets=...` or `--outp | Default (git and monitoring tools off) | Fewer tools for the agent to misuse | - | | `--clean-env` | Child processes get a minimal environment: tokens, API keys and cloud credentials in environment variables are not passed on | Not a sandbox: files such as `~/.aws/credentials` and the network are still reachable | | Edits stay in the solution directory | Roslyn edit tools refuse paths outside it | Doesn't restrict what a build does | +| `--http`'s built-in Origin/Host checks | Rejects requests carrying a foreign `Origin` header and requests whose `Host` doesn't name this machine's loopback interface (DNS rebinding) | Not authentication, and sends no CORS headers: `--allowed-origin` doesn't let a browser page call it directly, only a proxy or non-browser client that sets that Origin; any request without an `Origin` header still reaches the port | ### Untrusted code @@ -82,10 +83,18 @@ isolation itself. ### `--http` mode -HTTP mode listens on `localhost` only and has **no authentication, TLS or origin checks**. Anyone who can reach the port can -build, test and edit with your privileges. Don't forward the port, put it behind a proxy, or run it on a shared machine. -A multi-user or hosted deployment would need authentication, a sandbox per session and audit logging; DotNetDevMCP doesn't -provide those today. +HTTP mode listens on `localhost` only. A request that carries an `Origin` header is checked against an allow-list: only +`http://localhost:`, `http://127.0.0.1:` and `http://[::1]:` (the server's own port), plus any exact +value passed via `--allowed-origin` (validated at startup - no wildcards, no path/query/fragment/userinfo), are accepted. +Everything else - including other localhost ports and `https://` origins not explicitly allow-listed - gets a 403. A +request whose `Host` header doesn't name this machine's loopback interface also gets a 403 (DNS rebinding defense). +Requests without an `Origin` header (every non-browser MCP client, and a browser's simple GET/HEAD) are not rejected by +this check; they still only get whatever the MCP endpoint itself returns for that request. The server sends no CORS +headers, so `--allowed-origin` does not let a browser page call it directly from that origin - it's for a local +dev-server proxy that forwards the original `Origin`, or a non-browser client that happens to set one. It still has +**no authentication or TLS**: anyone on the loopback interface who can reach the port can build, test and edit with your +privileges. Don't forward the port, put it behind a proxy, or run it on a shared machine. A multi-user or hosted +deployment would need authentication, a sandbox per session and audit logging; DotNetDevMCP doesn't provide those today. ## Security Updates diff --git a/src/DotNetDevMCP.Server/AllowedOriginValidation.cs b/src/DotNetDevMCP.Server/AllowedOriginValidation.cs new file mode 100644 index 0000000..30cfb34 --- /dev/null +++ b/src/DotNetDevMCP.Server/AllowedOriginValidation.cs @@ -0,0 +1,76 @@ +// Copyright (c) 2025 Ahmed Mustafa +// Validates and normalizes --allowed-origin values before they're handed to LocalOriginGuard, so a +// typo'd or nonsensical value (a path, a wildcard, "null") fails fast at startup instead of silently +// never matching (or, worse, matching more than intended). + +namespace DotNetDevMCP.Server; + +/// Validates a single --allowed-origin value. +public static class AllowedOriginValidation +{ + /// + /// Checks that is an absolute http/https URI with nothing + /// but scheme, host and optional port - no userinfo, path (other than an implicit trailing "/"), + /// query or fragment - and isn't the literal string "null" or a wildcard. + /// + /// The raw value from --allowed-origin. + /// + /// On success, the value normalized to a bare origin with no trailing slash (e.g. + /// http://localhost:5173/ -> http://localhost:5173). On failure, echoes + /// back unchanged. + /// + /// A short reason the value is invalid, or null if it's valid. + public static string? Validate(string value, out string normalized) + { + normalized = value; + + if (string.IsNullOrWhiteSpace(value)) + { + return "must not be empty."; + } + + if (string.Equals(value, "null", StringComparison.OrdinalIgnoreCase)) + { + return "'null' is the serialized Origin of an opaque/sandboxed page, not a real origin - it can't be used here."; + } + + if (value.Contains('*')) + { + return "wildcards are not allowed; give the exact origin."; + } + + if (!Uri.TryCreate(value, UriKind.Absolute, out var uri)) + { + return "must be an absolute URI, e.g. http://localhost:5173."; + } + + if (uri.Scheme is not ("http" or "https")) + { + return $"scheme must be http or https, not '{uri.Scheme}'."; + } + + if (!string.IsNullOrEmpty(uri.UserInfo)) + { + return "must not include a userinfo (user:pass@) component."; + } + + if (uri.AbsolutePath != "/") + { + return "must not include a path."; + } + + if (!string.IsNullOrEmpty(uri.Query)) + { + return "must not include a query string."; + } + + if (!string.IsNullOrEmpty(uri.Fragment)) + { + return "must not include a fragment."; + } + + // Scheme + host [+ port], no trailing slash - matches how a browser formats the Origin header. + normalized = uri.GetLeftPart(UriPartial.Authority); + return null; + } +} diff --git a/src/DotNetDevMCP.Server/LocalOriginGuard.cs b/src/DotNetDevMCP.Server/LocalOriginGuard.cs new file mode 100644 index 0000000..486b87a --- /dev/null +++ b/src/DotNetDevMCP.Server/LocalOriginGuard.cs @@ -0,0 +1,115 @@ +// Copyright (c) 2025 Ahmed Mustafa +// DNS-rebinding / cross-origin defense for --http mode, per the MCP Streamable HTTP transport +// security guidance: servers MUST validate the Origin header on incoming connections and SHOULD +// bind only to localhost. We already bind to localhost only; this adds the Origin/Host checks. + +namespace DotNetDevMCP.Server; + +/// +/// Rejects any request carrying a foreign Origin header, and any request whose Host header +/// doesn't name this machine's loopback interface, so a malicious page (via DNS rebinding or a plain +/// fetch()) can't reach the MCP server through a victim's browser. Requests without an Origin +/// header - every non-browser MCP client, and a browser's simple GET/HEAD - aren't rejected by this +/// check; they just get whatever the MCP endpoint itself returns for that request. +/// +public static class LocalOriginGuard +{ + private static readonly string[] LoopbackHosts = ["localhost", "127.0.0.1", "[::1]"]; + + /// + /// Pure decision logic, kept free of any ASP.NET Core types so it can be unit tested directly. + /// Returns a short reason to reject the request with, or null to allow it. + /// + /// The raw Origin header value, or null/empty if absent. + /// The raw Host header value (may include a port), or null/empty if absent. + /// The port this server is listening on. + /// Additional allowed origins from --allowed-origin, exact strings. + public static string? Reject(string? origin, string? host, int port, IReadOnlyCollection extraOrigins) + { + if (!string.IsNullOrEmpty(origin)) + { + string[] builtIn = + [ + $"http://localhost:{port}", + $"http://127.0.0.1:{port}", + $"http://[::1]:{port}", + ]; + + bool allowed = false; + foreach (var candidate in builtIn) + { + if (string.Equals(origin, candidate, StringComparison.OrdinalIgnoreCase)) { allowed = true; break; } + } + if (!allowed) + { + foreach (var candidate in extraOrigins) + { + if (string.Equals(origin, candidate, StringComparison.OrdinalIgnoreCase)) { allowed = true; break; } + } + } + + if (!allowed) + { + return $"Origin '{origin}' is not allowed. This server only accepts requests from localhost origins " + + "(plus any configured --allowed-origin)."; + } + } + + // A missing or empty Host header does reach this code - Kestrel does not reject it for us + // (an HTTP/1.0 request with no Host header, or one with an empty Host value, is passed through). + // We allow it deliberately: every real browser always sends Host, so a request without one is + // necessarily a non-browser client, which isn't the DNS-rebinding threat this check defends against. + if (string.IsNullOrEmpty(host)) + { + return null; + } + + var hostOnly = StripPort(host); + foreach (var loopback in LoopbackHosts) + { + if (string.Equals(loopback, hostOnly, StringComparison.OrdinalIgnoreCase)) + { + return null; + } + } + + return $"Host '{host}' is not allowed. This server only accepts requests addressed to localhost, " + + "127.0.0.1 or [::1] (DNS rebinding protection)."; + } + + /// Strips the trailing ":port" from a Host header value. IPv6 literals keep their brackets + /// (e.g. "[::1]:3001" -> "[::1]") so they can be compared against as-is. + private static string StripPort(string hostHeader) + { + if (hostHeader.StartsWith('[')) + { + var end = hostHeader.IndexOf(']'); + return end < 0 ? hostHeader : hostHeader[..(end + 1)]; + } + + var colon = hostHeader.IndexOf(':'); + return colon < 0 ? hostHeader : hostHeader[..colon]; + } + + /// Registers the guard as middleware. Must run before MapMcp() so a rejected request + /// never reaches the MCP transport. + public static IApplicationBuilder UseLocalOriginGuard(this IApplicationBuilder app, int port, IReadOnlyCollection extraOrigins) + { + return app.Use(async (context, next) => + { + var origin = context.Request.Headers.Origin.Count > 0 ? context.Request.Headers.Origin.ToString() : null; + var host = context.Request.Headers.Host.Count > 0 ? context.Request.Headers.Host.ToString() : null; + + var reason = Reject(origin, host, port, extraOrigins); + if (reason is not null) + { + context.Response.StatusCode = StatusCodes.Status403Forbidden; + context.Response.ContentType = "text/plain"; + await context.Response.WriteAsync(reason); + return; + } + + await next(context); + }); + } +} diff --git a/src/DotNetDevMCP.Server/Program.cs b/src/DotNetDevMCP.Server/Program.cs index 0808c5e..7237c41 100644 --- a/src/DotNetDevMCP.Server/Program.cs +++ b/src/DotNetDevMCP.Server/Program.cs @@ -42,6 +42,7 @@ public static async Task Main(string[] args) var gitCommitEditsOption = new Option("--git-commit-edits") { Description = "Let edit tools (RenameSymbol, OverwriteMember, AddMember, MoveMember, FindAndReplace, CreateRoslynDocument, OverwriteRoslynDocument, ManageUsings, ManageAttributes) create a git branch and commit after each change, and enable SharpTool_Undo. Off by default: edits are still applied to disk and compile-checked, they just don't touch git or your current branch." }; var disableGitOption = new Option("--disable-git") { Description = "Deprecated, no-op. Git integration in code-intelligence tools is off by default; use --git-commit-edits to opt in." }; var cleanEnvOption = new Option("--clean-env") { Description = "Give every dotnet/git child process a minimal, allow-listed environment instead of inheriting this server's full one. Scrubs environment variables only; it is not a sandbox: child processes still run with your user's file-system and network access. Off by default." }; + var allowedOriginOption = new Option("--allowed-origin") { Description = "Extra Origin allowed for --http (repeatable), e.g. http://localhost:5173", DefaultValueFactory = _ => [] }; var enableOption = new Option("--enable") { Description = "Enable optional tool groups, off by default: 'git' (repo status/branch/stage/commit/push/pull/log/diff) and 'monitoring' (process performance/GC/health/profiling). Comma-separated and/or repeated, e.g. \"--enable git,monitoring\" or \"--enable git --enable monitoring\".", @@ -52,7 +53,7 @@ public static async Task Main(string[] args) var root = new RootCommand("DotNetDevMCP - MCP server for .NET development: Roslyn code intelligence, build, affected-test selection, git, orchestration.") { - httpOption, portOption, logDirOption, logLevelOption, loadSolutionOption, buildConfigurationOption, gitCommitEditsOption, disableGitOption, enableOption, cleanEnvOption + httpOption, portOption, logDirOption, logLevelOption, loadSolutionOption, buildConfigurationOption, gitCommitEditsOption, disableGitOption, enableOption, cleanEnvOption, allowedOriginOption }; var parsed = root.Parse(args); @@ -75,6 +76,24 @@ public static async Task Main(string[] args) bool enableGit = enabledGroups.Contains("git"); bool enableMonitoring = enabledGroups.Contains("monitoring"); bool cleanEnv = parsed.GetValue(cleanEnvOption); + string[] allowedOrigins = parsed.GetValue(allowedOriginOption) ?? []; + + // Ignored in stdio mode (--allowed-origin only affects --http), so only validate when it matters: + // a typo'd or nonsensical value should fail fast at startup rather than silently never matching. + if (http && allowedOrigins.Length > 0) + { + var normalized = new string[allowedOrigins.Length]; + for (var i = 0; i < allowedOrigins.Length; i++) + { + var error = AllowedOriginValidation.Validate(allowedOrigins[i], out normalized[i]); + if (error is not null) + { + Console.Error.WriteLine($"--allowed-origin '{allowedOrigins[i]}' is invalid: {error}"); + return 2; + } + } + allowedOrigins = normalized; + } Log.Logger = BuildLogger(logLevel, logDir); @@ -99,7 +118,7 @@ public static async Task Main(string[] args) Log.Information("Starting {App} v{Version} ({Transport})", ApplicationName, ApplicationVersion, http ? $"http://localhost:{port}" : "stdio"); IHost host = http - ? BuildHttpHost(args, port, gitCommitEdits, buildConfiguration, enableGit, enableMonitoring) + ? BuildHttpHost(args, port, gitCommitEdits, buildConfiguration, enableGit, enableMonitoring, allowedOrigins) : BuildStdioHost(args, gitCommitEdits, buildConfiguration, enableGit, enableMonitoring); if (!string.IsNullOrEmpty(solutionPath)) @@ -130,13 +149,16 @@ private static IHost BuildStdioHost(string[] args, bool gitCommitEdits, string? return builder.Build(); } - private static IHost BuildHttpHost(string[] args, int port, bool gitCommitEdits, string? buildConfiguration, bool enableGit, bool enableMonitoring) + private static IHost BuildHttpHost(string[] args, int port, bool gitCommitEdits, string? buildConfiguration, bool enableGit, bool enableMonitoring, IReadOnlyCollection allowedOrigins) { var builder = WebApplication.CreateBuilder(new WebApplicationOptions { Args = args }); builder.Host.UseSerilog(); builder.WebHost.UseUrls($"http://localhost:{port}"); AddServices(builder.Services, gitCommitEdits, buildConfiguration, enableGit, enableMonitoring).WithHttpTransport(); var app = builder.Build(); + // MCP Streamable HTTP transport security requirement: validate Origin (DNS rebinding defense) + // and localhost-only Host before any request reaches the MCP endpoint. + app.UseLocalOriginGuard(port, allowedOrigins); app.MapMcp(); return app; } diff --git a/tests/DotNetDevMCP.Integration.Tests/AllowedOriginValidationTests.cs b/tests/DotNetDevMCP.Integration.Tests/AllowedOriginValidationTests.cs new file mode 100644 index 0000000..1a77a0b --- /dev/null +++ b/tests/DotNetDevMCP.Integration.Tests/AllowedOriginValidationTests.cs @@ -0,0 +1,73 @@ +using DotNetDevMCP.Server; + +namespace DotNetDevMCP.Integration.Tests; + +/// Unit tests for the --allowed-origin value validator/normalizer. +public class AllowedOriginValidationTests +{ + [Fact] + public void Null_literal_is_rejected() + { + var error = AllowedOriginValidation.Validate("null", out _); + Assert.NotNull(error); + } + + [Fact] + public void Wildcard_is_rejected() + { + var error = AllowedOriginValidation.Validate("http://*.example.com", out _); + Assert.NotNull(error); + } + + [Fact] + public void Trailing_slash_is_normalized_away() + { + var error = AllowedOriginValidation.Validate("http://localhost:5173/", out var normalized); + Assert.Null(error); + Assert.Equal("http://localhost:5173", normalized); + } + + [Fact] + public void Path_is_rejected() + { + var error = AllowedOriginValidation.Validate("http://localhost:5173/app", out _); + Assert.NotNull(error); + } + + [Fact] + public void Https_is_accepted() + { + var error = AllowedOriginValidation.Validate("https://localhost:5173", out var normalized); + Assert.Null(error); + Assert.Equal("https://localhost:5173", normalized); + } + + [Fact] + public void Ftp_scheme_is_rejected() + { + var error = AllowedOriginValidation.Validate("ftp://localhost", out _); + Assert.NotNull(error); + } + + [Fact] + public void Query_string_is_rejected() + { + var error = AllowedOriginValidation.Validate("http://localhost:5173?x=1", out _); + Assert.NotNull(error); + } + + [Fact] + public void Userinfo_is_rejected() + { + var error = AllowedOriginValidation.Validate("http://user:pass@localhost:5173", out _); + Assert.NotNull(error); + } + + [Fact] + public void Bare_origin_without_trailing_slash_round_trips_unchanged() + { + var error = AllowedOriginValidation.Validate("http://a", out var normalized); + Assert.Null(error); + Assert.Equal("http://a", normalized); + } +} diff --git a/tests/DotNetDevMCP.Integration.Tests/DotNetDevMCP.Integration.Tests.csproj b/tests/DotNetDevMCP.Integration.Tests/DotNetDevMCP.Integration.Tests.csproj index 1d4cb3c..89baebb 100644 --- a/tests/DotNetDevMCP.Integration.Tests/DotNetDevMCP.Integration.Tests.csproj +++ b/tests/DotNetDevMCP.Integration.Tests/DotNetDevMCP.Integration.Tests.csproj @@ -18,4 +18,8 @@ + + + + diff --git a/tests/DotNetDevMCP.Integration.Tests/LocalOriginGuardAllowedOriginIntegrationTests.cs b/tests/DotNetDevMCP.Integration.Tests/LocalOriginGuardAllowedOriginIntegrationTests.cs new file mode 100644 index 0000000..9112188 --- /dev/null +++ b/tests/DotNetDevMCP.Integration.Tests/LocalOriginGuardAllowedOriginIntegrationTests.cs @@ -0,0 +1,21 @@ +using System.Net; + +namespace DotNetDevMCP.Integration.Tests; + +/// Real-server test for a server started with an extra --allowed-origin. A separate +/// server process/config from , so it gets its own +/// test class (and so its own InitializeAsync/DisposeAsync lifecycle). +public class LocalOriginGuardAllowedOriginIntegrationTests : LocalOriginGuardIntegrationTestsBase +{ + protected override string ExtraArgs => "--allowed-origin http://a"; + + [Fact] + public async Task Configured_allowed_origin_initialize_succeeds() + { + using var request = CreateInitializeRequest(Port, origin: "http://a"); + + using var response = await Http.SendAsync(request); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } +} diff --git a/tests/DotNetDevMCP.Integration.Tests/LocalOriginGuardIntegrationTests.cs b/tests/DotNetDevMCP.Integration.Tests/LocalOriginGuardIntegrationTests.cs new file mode 100644 index 0000000..dd2f076 --- /dev/null +++ b/tests/DotNetDevMCP.Integration.Tests/LocalOriginGuardIntegrationTests.cs @@ -0,0 +1,38 @@ +using System.Net; + +namespace DotNetDevMCP.Integration.Tests; + +/// Real-server tests for the default (no extra --allowed-origin) configuration. +public class LocalOriginGuardIntegrationTests : LocalOriginGuardIntegrationTestsBase +{ + [Fact] + public async Task Foreign_origin_post_is_rejected_with_403() + { + using var request = CreateInitializeRequest(Port, origin: "http://evil.example"); + + using var response = await Http.SendAsync(request); + + Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode); + } + + [Fact] + public async Task Rebinding_host_header_is_rejected_with_403() + { + using var request = CreateInitializeRequest(Port, hostOverride: "evil.example"); + + using var response = await Http.SendAsync(request); + + Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode); + } + + [Fact] + public async Task No_origin_initialize_reaches_mcp_and_succeeds() + { + // No Origin header at all - simulates a non-browser MCP client performing a real handshake. + using var request = CreateInitializeRequest(Port); + + using var response = await Http.SendAsync(request); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } +} diff --git a/tests/DotNetDevMCP.Integration.Tests/LocalOriginGuardIntegrationTestsBase.cs b/tests/DotNetDevMCP.Integration.Tests/LocalOriginGuardIntegrationTestsBase.cs new file mode 100644 index 0000000..98450c0 --- /dev/null +++ b/tests/DotNetDevMCP.Integration.Tests/LocalOriginGuardIntegrationTestsBase.cs @@ -0,0 +1,200 @@ +using System.Diagnostics; +using System.Net.Http.Headers; +using System.Net; +using System.Net.Sockets; +using System.Text; + +namespace DotNetDevMCP.Integration.Tests; + +/// +/// Starts the real dotnetdevmcp --http host (via the project reference to DotNetDevMCP.Server) as a +/// child process on a free port, and hits it over real HTTP - no fakes, so this exercises the actual +/// Kestrel pipeline including our origin/host guard. One process per test method (xUnit creates a new +/// instance of the test class, and so calls InitializeAsync/DisposeAsync, for every [Fact]). +/// +public abstract class LocalOriginGuardIntegrationTestsBase : IAsyncLifetime +{ + /// Extra CLI args appended after "--http --port <port>". Override to test e.g. --allowed-origin. + protected virtual string ExtraArgs => ""; + + private Process? _process; + private readonly StringBuilder _stderr = new(); + private readonly object _stderrLock = new(); + + protected int Port { get; private set; } + protected HttpClient Http { get; } = new(); + + public async Task InitializeAsync() + { + const int maxAttempts = 2; + Exception? lastError = null; + + for (var attempt = 1; attempt <= maxAttempts; attempt++) + { + try + { + await StartOnceAsync(); + return; + } + catch (Exception ex) + { + lastError = ex; + // If InitializeAsync throws, xUnit 2.9.3 never calls DisposeAsync for this instance, + // so the child process (if one got started) would otherwise leak. Kill it here. + KillQuietly(); + if (attempt == maxAttempts) + { + throw new InvalidOperationException($"Failed to start the server after {maxAttempts} attempt(s).", lastError); + } + // Retry once more on a fresh port - covers a losing race for the port we picked + // (another process binds it between us releasing the listener and the server binding it). + } + } + + throw lastError ?? new InvalidOperationException("unreachable"); + } + + private async Task StartOnceAsync() + { + Port = GetFreeTcpPort(); + + var dllPath = Path.Combine(AppContext.BaseDirectory, "dotnetdevmcp.dll"); + if (!File.Exists(dllPath)) + { + throw new FileNotFoundException($"Expected the referenced server build at {dllPath}"); + } + + _process = new Process + { + StartInfo = new ProcessStartInfo + { + FileName = "dotnet", + Arguments = $"\"{dllPath}\" --http --port {Port} {ExtraArgs}".TrimEnd(), + WorkingDirectory = AppContext.BaseDirectory, + RedirectStandardOutput = true, + RedirectStandardError = true, + UseShellExecute = false, + }, + }; + _process.OutputDataReceived += (_, _) => { }; + _process.ErrorDataReceived += (_, e) => + { + if (e.Data is null) return; + lock (_stderrLock) _stderr.AppendLine(e.Data); + }; + + _process.Start(); + _process.BeginOutputReadLine(); + _process.BeginErrorReadLine(); + + await WaitForPortAsync(Port, TimeSpan.FromSeconds(30)); + } + + public async Task DisposeAsync() + { + Http.Dispose(); + if (_process is { HasExited: false }) + { + try + { + _process.Kill(entireProcessTree: true); + await _process.WaitForExitAsync(); + } + catch + { + // Best-effort cleanup. + } + } + _process?.Dispose(); + } + + private void KillQuietly() + { + try + { + if (_process is { HasExited: false }) + { + _process.Kill(entireProcessTree: true); + } + } + catch + { + // We're already failing; don't let cleanup mask the original error. + } + finally + { + _process?.Dispose(); + _process = null; + } + } + + private async Task WaitForPortAsync(int port, TimeSpan timeout) + { + var deadline = DateTime.UtcNow + timeout; + while (DateTime.UtcNow < deadline) + { + if (_process is { HasExited: true }) + { + throw new InvalidOperationException( + $"Server process exited early (exit code {_process.ExitCode}) before listening on port {port}. Stderr:\n{ReadStderr()}"); + } + + try + { + using var client = new TcpClient(); + await client.ConnectAsync(IPAddress.Loopback, port); + return; + } + catch (SocketException) + { + await Task.Delay(250); + } + } + + throw new TimeoutException( + $"Server did not start listening on port {port} within {timeout}. Stderr:\n{ReadStderr()}"); + } + + private string ReadStderr() + { + lock (_stderrLock) return _stderr.ToString(); + } + + private static int GetFreeTcpPort() + { + var listener = new TcpListener(IPAddress.Loopback, 0); + listener.Start(); + var port = ((IPEndPoint)listener.LocalEndpoint).Port; + listener.Stop(); + return port; + } + + /// Builds a real MCP initialize POST, exactly what a compliant Streamable HTTP + /// client sends to establish a session, so a 200 here proves the request actually reached the MCP + /// endpoint rather than just failing to be a 403. + protected static HttpRequestMessage CreateInitializeRequest(int port, string? origin = null, string? hostOverride = null) + { + const string body = """ + {"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-06-18","capabilities":{},"clientInfo":{"name":"t","version":"0"}}} + """; + + var request = new HttpRequestMessage(HttpMethod.Post, $"http://localhost:{port}/") + { + Content = new StringContent(body, Encoding.UTF8, "application/json"), + }; + request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json")); + request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("text/event-stream")); + if (origin is not null) + { + request.Headers.Add("Origin", origin); + } + if (hostOverride is not null) + { + // Overrides the Host header actually sent on the wire; the TCP connection still goes to + // localhost:port via the request URI, simulating DNS rebinding (attacker DNS resolves + // evil.example to 127.0.0.1, but the browser still sends "Host: evil.example"). + request.Headers.Host = hostOverride; + } + return request; + } +} diff --git a/tests/DotNetDevMCP.Integration.Tests/LocalOriginGuardTests.cs b/tests/DotNetDevMCP.Integration.Tests/LocalOriginGuardTests.cs new file mode 100644 index 0000000..ea623ad --- /dev/null +++ b/tests/DotNetDevMCP.Integration.Tests/LocalOriginGuardTests.cs @@ -0,0 +1,72 @@ +using DotNetDevMCP.Server; + +namespace DotNetDevMCP.Integration.Tests; + +/// Unit tests for the pure decision logic behind --http's Origin/Host validation +/// (DNS rebinding and cross-origin browser request defense). No server involved here - +/// see for the real-host version. +public class LocalOriginGuardTests +{ + private const int Port = 3001; + private static readonly string[] NoExtraOrigins = []; + + [Fact] + public void Allowed_localhost_origin_is_accepted() + { + Assert.Null(LocalOriginGuard.Reject("http://localhost:3001", "localhost:3001", Port, NoExtraOrigins)); + } + + [Fact] + public void Foreign_origin_is_rejected() + { + Assert.NotNull(LocalOriginGuard.Reject("http://evil.example", "localhost:3001", Port, NoExtraOrigins)); + } + + [Fact] + public void Missing_origin_is_accepted_for_a_loopback_host() + { + // Non-browser MCP clients don't send an Origin header at all. + Assert.Null(LocalOriginGuard.Reject(null, "localhost:3001", Port, NoExtraOrigins)); + } + + [Fact] + public void Rebinding_host_with_no_origin_is_rejected() + { + Assert.NotNull(LocalOriginGuard.Reject(null, "evil.example:3001", Port, NoExtraOrigins)); + } + + [Fact] + public void Loopback_ipv4_host_is_accepted() + { + Assert.Null(LocalOriginGuard.Reject(null, "127.0.0.1:3001", Port, NoExtraOrigins)); + } + + [Fact] + public void Loopback_ipv6_host_is_accepted() + { + Assert.Null(LocalOriginGuard.Reject(null, "[::1]:3001", Port, NoExtraOrigins)); + } + + [Fact] + public void Configured_extra_origin_is_accepted() + { + Assert.Null(LocalOriginGuard.Reject("http://localhost:5173", "localhost:3001", Port, ["http://localhost:5173"])); + } + + [Fact] + public void Similar_looking_lookalike_origin_is_rejected() + { + // "localhost.evil.example" contains "localhost" as a prefix but is a different host entirely. + Assert.NotNull(LocalOriginGuard.Reject("http://localhost.evil.example:3001", "localhost:3001", Port, NoExtraOrigins)); + } + + [Fact] + public void Missing_or_empty_host_header_is_allowed_as_a_non_browser_client() + { + // Kestrel does not reject a missing/empty Host header for us, so this does reach the guard. + // We allow it: every real browser sends Host, so its absence means a non-browser client, + // which isn't the DNS-rebinding threat this check defends against. + Assert.Null(LocalOriginGuard.Reject(null, null, Port, NoExtraOrigins)); + Assert.Null(LocalOriginGuard.Reject(null, "", Port, NoExtraOrigins)); + } +}