From 33b194543359e1b0a4e6ff8032a79b5b9bbdf9c3 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Thu, 24 Sep 2026 00:05:32 -0400 Subject: [PATCH 1/2] test(darling): live HTTP proof of the web host's gates (#4128a) Extracts everything after builder.Build() through UseStaticFiles into an internal ConfigurePipeline(WebApplication app, ...) method that TryStartServerAsync calls with the same values in the same order, then adds DarlingWebHostGateLiveTests: six TestServer-driven cases covering network-mode no-token (login page), right-token+in-CIDR (passes), right-token+out-of-CIDR (403), foreign Host in both modes (400), and loopback no-token (reaches past the gates). HostHeaderGuardTests' EachHost_RunsTheGuardInBothModes_NotOnlyWhenNetworkExposed is updated to search for the auth-gating 'if (networkMode)' starting from the guard's own position, since the extraction moved TryStartServerAsync's unrelated post-start 'if (networkMode)' branch ahead of ConfigurePipeline in the file. --- Darling/Darling.Tests/Darling.Tests.csproj | 3 + .../DarlingWebHostGateLiveTests.cs | 171 ++++++++ Darling/Darling.Tests/HostHeaderGuardTests.cs | 23 +- Darling/Darling.Tests/packages.lock.json | 6 + .../Mcp/DarlingWebHostService.cs | 367 +++++++++--------- Directory.Packages.props | 1 + 6 files changed, 391 insertions(+), 180 deletions(-) create mode 100644 Darling/Darling.Tests/DarlingWebHostGateLiveTests.cs diff --git a/Darling/Darling.Tests/Darling.Tests.csproj b/Darling/Darling.Tests/Darling.Tests.csproj index eb9ef27756..97fd72db41 100644 --- a/Darling/Darling.Tests/Darling.Tests.csproj +++ b/Darling/Darling.Tests/Darling.Tests.csproj @@ -16,6 +16,9 @@ + + diff --git a/Darling/Darling.Tests/DarlingWebHostGateLiveTests.cs b/Darling/Darling.Tests/DarlingWebHostGateLiveTests.cs new file mode 100644 index 0000000000..fa10f5fb76 --- /dev/null +++ b/Darling/Darling.Tests/DarlingWebHostGateLiveTests.cs @@ -0,0 +1,171 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; +using System.Net; +using System.Net.Http; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using Npgsql; +using PerformanceMonitor.Darling.Analysis; +using PerformanceMonitor.Darling.Service.Mcp; +using Xunit; + +namespace Darling.Tests; + +/// +/// #4128 part (a): a live HTTP proof of the web host's gates, through the SAME ConfigurePipeline +/// method production calls (extracted from TryStartServerAsync for exactly this purpose), driven by a +/// real Kestrel pipeline over rather than by reading source text or calling the pure +/// decision functions directly. already proves the WIRING ORDER by +/// parsing the source; this class proves the wired pipeline actually REFUSES and ADMITS requests the way the +/// source claims it does end to end (Host header spoof, missing/valid/wrong token, in/out-of-CIDR remote). +/// +/// The gates read context.Connection.RemoteIpAddress, so every request goes through +/// (which allows setting it) rather than through +/// (which does not expose it). +/// +public sealed class DarlingWebHostGateLiveTests +{ + private const string ListenIp = "192.168.1.205"; + private const string AllowedCidr = "192.168.1.0/24"; + private const string Token = "correct-token-value"; + private static readonly IPAddress InCidrRemote = IPAddress.Parse("192.168.1.50"); + private static readonly IPAddress OutOfCidrRemote = IPAddress.Parse("10.0.0.9"); + + /// + /// Builds a running the REAL DarlingWebHostService.ConfigurePipeline — + /// the exact method the production TryStartServerAsync calls right after builder.Build(), + /// in the same order, with the same values (see that call site). The only difference from production is + /// the transport (TestServer instead of Kestrel sockets) and the store pool (a data source that is never + /// opened, because none of these gates touch Postgres). + /// + private static async Task BuildServer(bool networkMode) + { + var builder = WebApplication.CreateBuilder(); + builder.WebHost.UseTestServer(); + builder.Logging.ClearProviders(); + + // A pool the gates never open (they run entirely ahead of DarlingWebEndpoints.MapAll's routes). + var postgres = NpgsqlDataSource.Create("Host=localhost;Database=postgres;Username=darling"); + builder.Services.AddSingleton(postgres); + + var app = builder.Build(); + + var host = new DarlingWebHostService( + NullLogger.Instance, + new WebRuntimeState(), + new CollectorRuntimeState(), + new WebTlsCertificateState(), + new BaselineCache()); + + host.ConfigurePipeline( + app, + postgres, + networkMode: networkMode, + networkListenIp: networkMode ? IPAddress.Parse(ListenIp) : null, + allowedCidr: IPNetwork.Parse(AllowedCidr), + accessToken: Token, + oidcClient: null); + + await app.StartAsync(); + return app.GetTestServer(); + } + + private static Task Send(TestServer server, string path, string host, IPAddress? remote, string? token = null) + { + var target = token is null ? path : $"{path}?token={Uri.EscapeDataString(token)}"; + return server.SendAsync(ctx => + { + ctx.Request.Method = "GET"; + ctx.Request.Path = target.Contains('?') ? target[..target.IndexOf('?')] : target; + ctx.Request.QueryString = target.Contains('?') ? new QueryString(target[target.IndexOf('?')..]) : QueryString.Empty; + ctx.Request.Headers.Host = host; + ctx.Connection.RemoteIpAddress = remote; + }); + } + + /// Network mode, no credential at all: the Host header is legitimate but no token/cookie is + /// presented, so the auth gate serves the login page (200 — see WebRequestAction.ShowLogin, + /// deliberately not a 401; HostHeaderGuardTests and DarlingWebAuthTests pin why). + [Fact] + public async Task NetworkMode_NoToken_ShowsLoginRatherThanServingTheApp() + { + using var server = await BuildServer(networkMode: true); + var ctx = await Send(server, "/", ListenIp, InCidrRemote); + + Assert.Equal(StatusCodes.Status200OK, ctx.Response.StatusCode); + // The login page, not a static-file/app response: its body is generated inline by WriteLoginPageAsync. + Assert.Equal("text/html; charset=utf-8", ctx.Response.ContentType); + } + + /// Network mode, the right token, an in-CIDR remote: the gate exchanges the token for a + /// session cookie and 302-redirects (WebRequestAction.SetCookieAndRedirect) — proof the request + /// got PAST every gate, not a login page and not a refusal. + [Fact] + public async Task NetworkMode_RightTokenInCidr_PassesTheGates() + { + using var server = await BuildServer(networkMode: true); + var ctx = await Send(server, "/", ListenIp, InCidrRemote, token: Token); + + Assert.Equal(StatusCodes.Status302Found, ctx.Response.StatusCode); + Assert.True(ctx.Response.Headers.SetCookie.Count > 0, "a valid token must be exchanged for a session cookie"); + } + + /// Network mode, the right token, but the remote is OUTSIDE allowFrom: the CIDR gate is + /// outermost (#2550) and forbids before the token is even considered. + [Fact] + public async Task NetworkMode_RightTokenOutsideCidr_IsForbidden() + { + using var server = await BuildServer(networkMode: true); + var ctx = await Send(server, "/", ListenIp, OutOfCidrRemote, token: Token); + + Assert.Equal(StatusCodes.Status403Forbidden, ctx.Response.StatusCode); + } + + /// A foreign Host header is refused by the DNS-rebinding guard in NETWORK mode — before the + /// CIDR/token gate ever runs (#1576/#1648). + [Fact] + public async Task NetworkMode_ForeignHost_IsRefused() + { + using var server = await BuildServer(networkMode: true); + var ctx = await Send(server, "/", "evil.com", InCidrRemote, token: Token); + + Assert.Equal(StatusCodes.Status400BadRequest, ctx.Response.StatusCode); + } + + /// The same rebinding guard runs in LOOPBACK mode too (#1576 fixed a loopback-only gap): a + /// foreign Host is refused even though loopback registers no CIDR/token middleware at all. + [Fact] + public async Task LoopbackMode_ForeignHost_IsRefused() + { + using var server = await BuildServer(networkMode: false); + var ctx = await Send(server, "/", "evil.com", IPAddress.Loopback); + + Assert.Equal(StatusCodes.Status400BadRequest, ctx.Response.StatusCode); + } + + /// Loopback mode's own posture: no token middleware is registered at all, so a loopback Host + /// with no token reaches past the gates (a 404 from the endpoint mapping / static files proves it got + /// through — no gate short-circuited with a 400/403, and there is no login page in this mode). + [Fact] + public async Task LoopbackMode_NoToken_ReachesPastTheGates() + { + using var server = await BuildServer(networkMode: false); + var ctx = await Send(server, "/some/unmapped/path", "localhost", IPAddress.Loopback); + + Assert.NotEqual(StatusCodes.Status400BadRequest, ctx.Response.StatusCode); + Assert.NotEqual(StatusCodes.Status403Forbidden, ctx.Response.StatusCode); + } +} diff --git a/Darling/Darling.Tests/HostHeaderGuardTests.cs b/Darling/Darling.Tests/HostHeaderGuardTests.cs index 3d2b972a34..bd7ffe328e 100644 --- a/Darling/Darling.Tests/HostHeaderGuardTests.cs +++ b/Darling/Darling.Tests/HostHeaderGuardTests.cs @@ -143,9 +143,12 @@ private static string ReadHostSource(string fileName) var source = File.ReadAllText(path); /* Guard the guard: if the host were renamed or restructured past recognition, the assertions below - could pass vacuously on an empty/mismatched parse. Pin the anchors they all key off. */ + could pass vacuously on an empty/mismatched parse. Pin the anchors they all key off. "app.Use(" + (not "_app.Use(") matches both hosts: the MCP host still calls the field directly (_app.Use), while + the web host's gates were extracted (#4128) into ConfigurePipeline(WebApplication app, ...), which + calls app.Use — "app.Use(" is a substring of "_app.Use(" too, so this anchor covers both shapes. */ Assert.Contains("_app = builder.Build();", source, StringComparison.Ordinal); - Assert.Contains("_app.Use(", source, StringComparison.Ordinal); + Assert.Contains("app.Use(", source, StringComparison.Ordinal); return source; } @@ -153,13 +156,13 @@ could pass vacuously on an empty/mismatched parse. Pin the anchors they all key private static string FirstMiddlewareAfterBuild(string source) { var afterBuild = source[(source.IndexOf("_app = builder.Build();", StringComparison.Ordinal) + 1)..]; - var firstUse = afterBuild.IndexOf("_app.Use(", StringComparison.Ordinal); + var firstUse = afterBuild.IndexOf("app.Use(", StringComparison.Ordinal); Assert.True(firstUse >= 0, "no middleware is registered after builder.Build()"); /* The first Use's lambda body — long enough to cover the guard's if/return, short enough that a SECOND Use's body cannot bleed in and satisfy the assertion by accident. */ var body = afterBuild[firstUse..]; - var nextUse = body.IndexOf("_app.Use(", 1, StringComparison.Ordinal); + var nextUse = body.IndexOf("app.Use(", 1, StringComparison.Ordinal); return nextUse > 0 ? body[..nextUse] : body; } @@ -188,10 +191,16 @@ public void EachHost_RunsTheGuardInBothModes_NotOnlyWhenNetworkExposed(string fi var afterBuild = source[(source.IndexOf("_app = builder.Build();", StringComparison.Ordinal) + 1)..]; var guard = afterBuild.IndexOf("IsAllowedHost(context.Request.Host.Host", StringComparison.Ordinal); - var networkOnly = afterBuild.IndexOf("if (networkMode)", StringComparison.Ordinal); - Assert.True(guard >= 0, "the Host-header guard middleware is missing"); - Assert.True(networkOnly >= 0, "expected a network-mode-only middleware block to exist after Build()"); + + /* #4128: the web host's gates were extracted into ConfigurePipeline, a method the file defines + AFTER TryStartServerAsync's own unrelated "if (networkMode)" (its post-start logging branch) — + so the FIRST "if (networkMode)" after Build() is no longer necessarily the one that gates the + auth middleware. Searching from the guard's own position finds the auth-gating conditional that + actually follows it in the same method, which is the invariant this test claims either way. */ + var networkOnly = afterBuild.IndexOf("if (networkMode)", guard, StringComparison.Ordinal); + + Assert.True(networkOnly >= 0, "expected a network-mode-only middleware block to exist after the guard"); Assert.True( guard < networkOnly, "the Host-header guard must be registered BEFORE the network-mode-only block — inside it, the " + diff --git a/Darling/Darling.Tests/packages.lock.json b/Darling/Darling.Tests/packages.lock.json index 212f8cd112..68efe92178 100644 --- a/Darling/Darling.Tests/packages.lock.json +++ b/Darling/Darling.Tests/packages.lock.json @@ -2,6 +2,12 @@ "version": 2, "dependencies": { "net10.0-windows7.0": { + "Microsoft.AspNetCore.TestHost": { + "type": "Direct", + "requested": "[10.0.12, )", + "resolved": "10.0.12", + "contentHash": "bTHgbcjSrLxlqSasYRDU9BcJ3hDGoqIjtDY0soLNrndOWi3kxtOyddwUMaNZgtoI2Uf6y7ZQs2EuVj0c20B7mA==" + }, "xunit.v3": { "type": "Direct", "requested": "[4.0.1, )", diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingWebHostService.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingWebHostService.cs index ed863813e1..634cefb6da 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingWebHostService.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingWebHostService.cs @@ -771,179 +771,7 @@ tool parameters through DI. */ builder.Services.AddSingleton(postgres); _app = builder.Build(); - - /* #2479 item 5: the gates below used to refuse silently. Rate-limited per (gate, source), - because this port is LAN-exposed on purpose - see DarlingHttpRefusalLog. Created per - started server so a rebind starts with a clean budget. */ - var refusals = new DarlingHttpRefusalLog(); - - /* Pipeline order: the Host-allowlist middleware runs FIRST on EVERY request (both modes) as the - DNS-rebinding guard, then (network mode only) the auth middleware, then DarlingWebEndpoints.MapAll - -> UseDefaultFiles -> UseStaticFiles. WebApplication auto-inserts UseRouting at the head and - UseEndpoints at the tail, so the static-file middleware sits behind these gates and serves the SPA - for non-API paths. */ - - /* DNS-rebinding guard — runs in BOTH modes (the #1576 fix: it previously guarded network mode only, - leaving the tokenless loopback write path reachable cross-origin via a DNS rebind). The loopback - surface is tokenless, so a browser ON the host that loads attacker content could be rebound to - 127.0.0.1:5153 and read/write the whole surface same-origin. Require the Host header to name an - address we actually bind — a loopback name/IP (localhost / 127.0.0.1 / [::1]) or, in network mode, - the configured listen IP. networkListenIp is null in loopback mode, so ONLY loopback Hosts pass - there; a rebound foreign hostname pointed at 127.0.0.1:5153 is rejected 400 before any auth - decision, route handler, or static file. */ - _app.Use(async (context, next) => - { - if (!IsAllowedHost(context.Request.Host.Host, networkListenIp)) - { - refusals.Report( - _logger, "Web dashboard", DarlingRefusalGate.HostAllowlist, StatusCodes.Status400BadRequest, - context.Connection.RemoteIpAddress, - $"the Host header '{DarlingHttpRefusalLog.Sanitize(context.Request.Host.Host)}' is not an address this endpoint binds" - + " (a loopback name/IP, or web.network.listen when LAN-exposed)", - DateTime.UtcNow); - context.Response.StatusCode = StatusCodes.Status400BadRequest; - return; - } - - await next(context); - }); - - if (networkMode) - { - var cidr = allowedCidr; - var token = accessToken; - /* Per-process signing key: a restart regenerates it and thereby invalidates every session - cookie (acceptable — the operator re-presents the token once). The OIDC transaction key is - DERIVED from it rather than shared — see DeriveTransactionKey for why sharing the raw key - across the two cookie shapes would let a pre-auth transaction cookie impersonate a session. */ - var signingKey = RandomNumberGenerator.GetBytes(SigningKeyBytes); - var transactionKey = DarlingWebOidc.DeriveTransactionKey(signingKey); - - _app.Use(async (context, next) => - { - /* The Host-allowlist / DNS-rebinding guard already ran above (both modes); this gate owns - the network auth decision (session cookie / ?token= / in-CIDR / the sign-in flow). */ - var remote = context.Connection.RemoteIpAddress; - var hasValidCookie = TryValidateSessionCookie( - context.Request.Cookies[SessionCookieName], signingKey, DateTimeOffset.UtcNow, out var cookieSubject); - - /* Resolve WHO holds the cookie (#2550). A cryptographically valid cookie whose subject - slot encodes a seat we do not recognize is treated as NO cookie: refusing is - recoverable (re-login), serving an unresolvable identity is not. */ - var seat = DarlingWebSeat.SharedToken; - if (hasValidCookie && !DarlingWebSeat.TryResolveSeat(cookieSubject, out seat)) - { - hasValidCookie = false; - seat = DarlingWebSeat.SharedToken; - } - - var presentedToken = context.Request.Query["token"].ToString(); - var hasValidToken = DarlingHostBinding.FixedTimeTokenEquals(presentedToken, token); - var isAuthFlowRoute = IsAuthFlowPath(context.Request.Path.Value ?? "/", oidcClient is not null); - - switch (DecideWebRequest(remote, cidr, isAuthFlowRoute, hasValidCookie, hasValidToken)) - { - case WebRequestAction.Allow: - /* The seat rides HttpContext.Items so EVERY downstream consumer — /api/session, - the updated_by stamps, endpoints added on any parallel branch — resolves the - same identity without per-endpoint wiring. */ - context.Items[DarlingWebSeat.HttpContextItemKey] = seat; - - /* The group-level write gate (#2550): a read-only seat is refused every - mutating request HERE, before routing, so a write endpoint added tomorrow is - born gated instead of born exposed. */ - if (!DarlingWebSeat.IsRequestAllowed(seat, context.Request.Method, context.Request.Path.Value ?? "/")) - { - refusals.Report( - _logger, "Web dashboard", DarlingRefusalGate.ReadOnlySeat, StatusCodes.Status403Forbidden, - remote, - $"the signed-in seat is read-only (viewer role) and {context.Request.Method} is a mutation", - DateTime.UtcNow); - context.Response.StatusCode = StatusCodes.Status403Forbidden; - context.Response.ContentType = "application/json; charset=utf-8"; - await context.Response.WriteAsync("{\"error\": \"This account has read-only access.\"}"); - return; - } - - await next(context); - return; - - case WebRequestAction.HandleAuthFlow: - /* The flow endpoints are reachable WITHOUT a credential by design (a sign-in has - to start somewhere), and this pipeline registers no exception-handling - middleware — so anything that throws in here would answer an anonymous caller - with an unhandled 500 and a stack-shaped failure instead of a refusal. The - #2744 review found one such throw for real (an out-of-range exp reaching - DateTimeOffset); that root cause is fixed in DarlingWebOidc, and this boundary - exists so the NEXT one is also a clean refusal rather than a 500. A client - abort still propagates — that is not a failure to report. */ - try - { - await HandleAuthFlowAsync(context, oidcClient, signingKey, transactionKey, seat, refusals); - } - catch (OperationCanceledException) when (context.RequestAborted.IsCancellationRequested) - { - throw; - } - catch (Exception ex) - { - ReportSignInRefusal(refusals, remote, StatusCodes.Status500InternalServerError, - $"the sign-in flow failed unexpectedly: {DarlingHttpRefusalLog.Sanitize(ex.Message, 128)}"); - if (!context.Response.HasStarted) - { - await WriteSignInErrorAsync(context, StatusCodes.Status500InternalServerError, - "The sign-in could not be completed. The service log names what failed."); - } - } - - return; - - case WebRequestAction.SetCookieAndRedirect: - AppendSessionCookie(context, signingKey); - /* 302 (default) to the same path with ?token= stripped, so the token never lingers - in browser history, bookmarks, or a Referer header. */ - context.Response.Redirect(BuildPathWithoutToken(context.Request)); - return; - - case WebRequestAction.Forbid: - /* An out-of-CIDR remote, or one whose address ASP.NET Core could not report - (which fails closed). A wrong credential from inside the CIDR is ShowLogin, - not this - see below. The CIDR stays OUTERMOST: it wins over a valid cookie, - a valid token, AND the OIDC endpoints (#2550). */ - refusals.Report( - _logger, "Web dashboard", DarlingRefusalGate.SourceCidr, StatusCodes.Status403Forbidden, - remote, - remote is null - ? "its source address could not be determined, which fails closed" - : $"its address is outside web.network.allowFrom ({cidr})", - DateTime.UtcNow); - context.Response.StatusCode = StatusCodes.Status403Forbidden; - return; - - default: /* ShowLogin */ - /* A 200 rather than a 401, so this is not a "rejected request" by status - and - it is exactly the state an operator asks about when a ?token= they pasted did - not work. Logged ONLY when a token was actually presented and did not match: - a first visit with no token is the normal path to the login page and logging - it would make every bookmark a warning. */ - if (!string.IsNullOrEmpty(presentedToken)) - { - refusals.Report( - _logger, "Web dashboard", DarlingRefusalGate.Token, StatusCodes.Status200OK, - remote, - "the presented ?token= does not match web.network.encryptedToken, so the login page was served instead", - DateTime.UtcNow); - } - - await WriteLoginPageAsync(context, oidcClient is not null); - return; - } - }); - } - - DarlingWebEndpoints.MapAll(_app, postgres, _collectorState, _logger, _baselineCache); - _app.UseDefaultFiles(); - _app.UseStaticFiles(); + ConfigurePipeline(_app, postgres, networkMode, networkListenIp, allowedCidr, accessToken, oidcClient); /* #2389: name the authority for each half of what is being started — enabled/port from whichever plane the supervisor resolved, listen/allowFrom/token always from darling.json. */ @@ -1073,6 +901,199 @@ still has to authenticate below. */ construction. --------------------------------------------------------------------------------------------------- */ + /// + /// Everything AFTER builder.Build(): the Host-allowlist/DNS-rebinding guard (both modes), the + /// network-mode auth middleware, then -> UseDefaultFiles + /// -> UseStaticFiles. Extracted (#4128) so a live-HTTP test can build the SAME pipeline against a + /// TestServer instead of a second, hand-copied one that could silently drift from production. The + /// production call site passes exactly these values, in exactly this order — see TryStartServerAsync. + /// Instance method, not static: the gates call back into and + /// , and read _logger/_collectorState/_baselineCache, + /// exactly as before the extraction — only the receiver (this vs. a test-constructed instance) changes. + /// + internal void ConfigurePipeline( + WebApplication app, + NpgsqlDataSource postgres, + bool networkMode, + IPAddress? networkListenIp, + IPNetwork allowedCidr, + string accessToken, + DarlingWebOidcClient? oidcClient) + { + /* #2479 item 5: the gates below used to refuse silently. Rate-limited per (gate, source), + because this port is LAN-exposed on purpose - see DarlingHttpRefusalLog. Created per + started server so a rebind starts with a clean budget. */ + var refusals = new DarlingHttpRefusalLog(); + + /* Pipeline order: the Host-allowlist middleware runs FIRST on EVERY request (both modes) as the + DNS-rebinding guard, then (network mode only) the auth middleware, then DarlingWebEndpoints.MapAll + -> UseDefaultFiles -> UseStaticFiles. WebApplication auto-inserts UseRouting at the head and + UseEndpoints at the tail, so the static-file middleware sits behind these gates and serves the SPA + for non-API paths. */ + + /* DNS-rebinding guard — runs in BOTH modes (the #1576 fix: it previously guarded network mode only, + leaving the tokenless loopback write path reachable cross-origin via a DNS rebind). The loopback + surface is tokenless, so a browser ON the host that loads attacker content could be rebound to + 127.0.0.1:5153 and read/write the whole surface same-origin. Require the Host header to name an + address we actually bind — a loopback name/IP (localhost / 127.0.0.1 / [::1]) or, in network mode, + the configured listen IP. networkListenIp is null in loopback mode, so ONLY loopback Hosts pass + there; a rebound foreign hostname pointed at 127.0.0.1:5153 is rejected 400 before any auth + decision, route handler, or static file. */ + app.Use(async (context, next) => + { + if (!IsAllowedHost(context.Request.Host.Host, networkListenIp)) + { + refusals.Report( + _logger, "Web dashboard", DarlingRefusalGate.HostAllowlist, StatusCodes.Status400BadRequest, + context.Connection.RemoteIpAddress, + $"the Host header '{DarlingHttpRefusalLog.Sanitize(context.Request.Host.Host)}' is not an address this endpoint binds" + + " (a loopback name/IP, or web.network.listen when LAN-exposed)", + DateTime.UtcNow); + context.Response.StatusCode = StatusCodes.Status400BadRequest; + return; + } + + await next(context); + }); + + if (networkMode) + { + var cidr = allowedCidr; + var token = accessToken; + /* Per-process signing key: a restart regenerates it and thereby invalidates every session + cookie (acceptable — the operator re-presents the token once). The OIDC transaction key is + DERIVED from it rather than shared — see DeriveTransactionKey for why sharing the raw key + across the two cookie shapes would let a pre-auth transaction cookie impersonate a session. */ + var signingKey = RandomNumberGenerator.GetBytes(SigningKeyBytes); + var transactionKey = DarlingWebOidc.DeriveTransactionKey(signingKey); + + app.Use(async (context, next) => + { + /* The Host-allowlist / DNS-rebinding guard already ran above (both modes); this gate owns + the network auth decision (session cookie / ?token= / in-CIDR / the sign-in flow). */ + var remote = context.Connection.RemoteIpAddress; + var hasValidCookie = TryValidateSessionCookie( + context.Request.Cookies[SessionCookieName], signingKey, DateTimeOffset.UtcNow, out var cookieSubject); + + /* Resolve WHO holds the cookie (#2550). A cryptographically valid cookie whose subject + slot encodes a seat we do not recognize is treated as NO cookie: refusing is + recoverable (re-login), serving an unresolvable identity is not. */ + var seat = DarlingWebSeat.SharedToken; + if (hasValidCookie && !DarlingWebSeat.TryResolveSeat(cookieSubject, out seat)) + { + hasValidCookie = false; + seat = DarlingWebSeat.SharedToken; + } + + var presentedToken = context.Request.Query["token"].ToString(); + var hasValidToken = DarlingHostBinding.FixedTimeTokenEquals(presentedToken, token); + var isAuthFlowRoute = IsAuthFlowPath(context.Request.Path.Value ?? "/", oidcClient is not null); + + switch (DecideWebRequest(remote, cidr, isAuthFlowRoute, hasValidCookie, hasValidToken)) + { + case WebRequestAction.Allow: + /* The seat rides HttpContext.Items so EVERY downstream consumer — /api/session, + the updated_by stamps, endpoints added on any parallel branch — resolves the + same identity without per-endpoint wiring. */ + context.Items[DarlingWebSeat.HttpContextItemKey] = seat; + + /* The group-level write gate (#2550): a read-only seat is refused every + mutating request HERE, before routing, so a write endpoint added tomorrow is + born gated instead of born exposed. */ + if (!DarlingWebSeat.IsRequestAllowed(seat, context.Request.Method, context.Request.Path.Value ?? "/")) + { + refusals.Report( + _logger, "Web dashboard", DarlingRefusalGate.ReadOnlySeat, StatusCodes.Status403Forbidden, + remote, + $"the signed-in seat is read-only (viewer role) and {context.Request.Method} is a mutation", + DateTime.UtcNow); + context.Response.StatusCode = StatusCodes.Status403Forbidden; + context.Response.ContentType = "application/json; charset=utf-8"; + await context.Response.WriteAsync("{\"error\": \"This account has read-only access.\"}"); + return; + } + + await next(context); + return; + + case WebRequestAction.HandleAuthFlow: + /* The flow endpoints are reachable WITHOUT a credential by design (a sign-in has + to start somewhere), and this pipeline registers no exception-handling + middleware — so anything that throws in here would answer an anonymous caller + with an unhandled 500 and a stack-shaped failure instead of a refusal. The + #2744 review found one such throw for real (an out-of-range exp reaching + DateTimeOffset); that root cause is fixed in DarlingWebOidc, and this boundary + exists so the NEXT one is also a clean refusal rather than a 500. A client + abort still propagates — that is not a failure to report. */ + try + { + await HandleAuthFlowAsync(context, oidcClient, signingKey, transactionKey, seat, refusals); + } + catch (OperationCanceledException) when (context.RequestAborted.IsCancellationRequested) + { + throw; + } + catch (Exception ex) + { + ReportSignInRefusal(refusals, remote, StatusCodes.Status500InternalServerError, + $"the sign-in flow failed unexpectedly: {DarlingHttpRefusalLog.Sanitize(ex.Message, 128)}"); + if (!context.Response.HasStarted) + { + await WriteSignInErrorAsync(context, StatusCodes.Status500InternalServerError, + "The sign-in could not be completed. The service log names what failed."); + } + } + + return; + + case WebRequestAction.SetCookieAndRedirect: + AppendSessionCookie(context, signingKey); + /* 302 (default) to the same path with ?token= stripped, so the token never lingers + in browser history, bookmarks, or a Referer header. */ + context.Response.Redirect(BuildPathWithoutToken(context.Request)); + return; + + case WebRequestAction.Forbid: + /* An out-of-CIDR remote, or one whose address ASP.NET Core could not report + (which fails closed). A wrong credential from inside the CIDR is ShowLogin, + not this - see below. The CIDR stays OUTERMOST: it wins over a valid cookie, + a valid token, AND the OIDC endpoints (#2550). */ + refusals.Report( + _logger, "Web dashboard", DarlingRefusalGate.SourceCidr, StatusCodes.Status403Forbidden, + remote, + remote is null + ? "its source address could not be determined, which fails closed" + : $"its address is outside web.network.allowFrom ({cidr})", + DateTime.UtcNow); + context.Response.StatusCode = StatusCodes.Status403Forbidden; + return; + + default: /* ShowLogin */ + /* A 200 rather than a 401, so this is not a "rejected request" by status - and + it is exactly the state an operator asks about when a ?token= they pasted did + not work. Logged ONLY when a token was actually presented and did not match: + a first visit with no token is the normal path to the login page and logging + it would make every bookmark a warning. */ + if (!string.IsNullOrEmpty(presentedToken)) + { + refusals.Report( + _logger, "Web dashboard", DarlingRefusalGate.Token, StatusCodes.Status200OK, + remote, + "the presented ?token= does not match web.network.encryptedToken, so the login page was served instead", + DateTime.UtcNow); + } + + await WriteLoginPageAsync(context, oidcClient is not null); + return; + } + }); + } + + DarlingWebEndpoints.MapAll(app, postgres, _collectorState, _logger, _baselineCache); + app.UseDefaultFiles(); + app.UseStaticFiles(); + } + /// The verdict for one network-mode request, with the sign-in flow added (#2550). The first four /// members are 's, mapped one-to-one. internal enum WebRequestAction { Allow, Forbid, SetCookieAndRedirect, ShowLogin, HandleAuthFlow } diff --git a/Directory.Packages.props b/Directory.Packages.props index 5a8a53292e..fb400f095d 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -22,6 +22,7 @@ + From e6a88a8a53a8397b821854bb86ea7a1827dc361f Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Thu, 24 Sep 2026 00:09:36 -0400 Subject: [PATCH 2/2] 4128: follow the extracted pipeline's MapAll(app, ...) in two source pins, and make TestServer.CreateClient prose, not a cref --- Darling/Darling.Tests/DarlingWebHostGateLiveTests.cs | 2 +- Darling/Darling.Tests/FleetSweepWebFeedTests.cs | 2 +- Darling/Darling.Tests/SharedBaselineCacheTests.cs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Darling/Darling.Tests/DarlingWebHostGateLiveTests.cs b/Darling/Darling.Tests/DarlingWebHostGateLiveTests.cs index fa10f5fb76..c422868c95 100644 --- a/Darling/Darling.Tests/DarlingWebHostGateLiveTests.cs +++ b/Darling/Darling.Tests/DarlingWebHostGateLiveTests.cs @@ -34,7 +34,7 @@ namespace Darling.Tests; /// /// The gates read context.Connection.RemoteIpAddress, so every request goes through /// (which allows setting it) rather than through -/// (which does not expose it). +/// TestServer.CreateClient (which does not expose it). /// public sealed class DarlingWebHostGateLiveTests { diff --git a/Darling/Darling.Tests/FleetSweepWebFeedTests.cs b/Darling/Darling.Tests/FleetSweepWebFeedTests.cs index 2221973da0..93b184ba62 100644 --- a/Darling/Darling.Tests/FleetSweepWebFeedTests.cs +++ b/Darling/Darling.Tests/FleetSweepWebFeedTests.cs @@ -606,7 +606,7 @@ public void TheSweepRoutes_LogThroughTheServiceLogger_NotTheProviderlessAppFacto "Darling", "PerformanceMonitor.Darling.Service", "Mcp", "DarlingWebHostService.cs"); Assert.Contains("builder.Logging.ClearProviders();", host, StringComparison.Ordinal); Assert.Contains( - "DarlingWebEndpoints.MapAll(_app, postgres, _collectorState, _logger, _baselineCache);", + "DarlingWebEndpoints.MapAll(app, postgres, _collectorState, _logger, _baselineCache);", host, StringComparison.Ordinal); } diff --git a/Darling/Darling.Tests/SharedBaselineCacheTests.cs b/Darling/Darling.Tests/SharedBaselineCacheTests.cs index e9047a4a4e..5e1c0bb2a2 100644 --- a/Darling/Darling.Tests/SharedBaselineCacheTests.cs +++ b/Darling/Darling.Tests/SharedBaselineCacheTests.cs @@ -142,7 +142,7 @@ public void TheService_HandsOneBaselineCacheToEveryHostsAnalysis() Assert.Contains("new DarlingAnalysisService(postgres, planFetcher, _logger, _baselineCache)", mcp, StringComparison.Ordinal); var web = Code("Darling", "PerformanceMonitor.Darling.Service", "Mcp", "DarlingWebHostService.cs"); - Assert.Contains("DarlingWebEndpoints.MapAll(_app, postgres, _collectorState, _logger, _baselineCache);", web, StringComparison.Ordinal); + Assert.Contains("DarlingWebEndpoints.MapAll(app, postgres, _collectorState, _logger, _baselineCache);", web, StringComparison.Ordinal); var endpoints = Code("Darling", "PerformanceMonitor.Darling.Service", "DarlingWebEndpoints.cs"); Assert.Contains("new DarlingAnalysisService(postgres, baselineCache: baselineCache)", endpoints, StringComparison.Ordinal);