Say so at startup when anyone can stop a checker without logging in

CHANGELOG.md

@@ -80,6 +80,18 @@ its code behaves exactly as it did.
   to completion without interleaving anything else, which is the guarantee a read-then-write cannot
   give. Durability is whatever the Redis is configured for, and the package README says so rather
   than implying more.
+- **A startup warning when a surface that can change a checker is reachable without
+  authenticating.** `AddHealthieController` still does not require authorization unless asked, and
+  the dashboard's `AllowMutations` still defaults to `true` -- both are deliberate and changing
+  either would break every application that maps them. What was missing was anyone being told: an
+  application that maps one and stops there lets whoever can reach it stop a checker or clear a
+  failing streak, which hides an incident rather than reporting one. Logged once, at `Warning`, on
+  application start.
+
+  Asked of the endpoints rather than of the flags that built them, so an application that applied
+  authorization its own way -- `RequireAuthorization()`, an endpoint group, its own attribute -- is
+  not warned at. A warning that fires on correctly secured applications gets filtered out, and then
+  it is not there for the one that needs it.
 - **Schedules.** `PulseSchedule` says either "every this long" or "on this cron expression", and
   sits alongside `PulseInterval` rather than replacing it. The enum stopped at five minutes, which
   is short of what a certificate-expiry or disk-space check wants. Cron is standard Unix syntax and

README.md

@@ -834,7 +834,9 @@ and both are deliberate as they stand:
 - **`Healthie.Api` requires no authorization unless the host asks for it**, and the dashboard's
   `HealthieUIOptions.AllowMutations` defaults to `true`. A host that maps either and does nothing
   else exposes read *and* write control of its checkers. Both are documented, and changing either
-  default is a behaviour break for every existing consumer.
+  default is a behaviour break for every existing consumer -- so instead of changing them, both now
+  log a `Warning` at startup naming exactly what is exposed. The warning reads the endpoints, not
+  the flags, so securing them any way at all silences it.
 - **Restore is not pinned by hash.** The fix is NuGet lock files, which means every package change
   needs the lock updated and CI running in locked mode.
 

src/Healthie.Api/Diagnostics/Log.cs

@@ -0,0 +1,28 @@
+using Microsoft.Extensions.Logging;
+
+namespace Healthie.Api.Diagnostics;
+
+/// <summary>
+/// The log messages <see cref="Healthie.Api"/> writes.
+/// </summary>
+/// <remarks>
+/// Source-generated, as in the other packages, and with event ids in their own 4000 range so a
+/// filter written against them keeps meaning what it meant.
+/// </remarks>
+internal static partial class Log
+{
+    /// <remarks>
+    /// Warning, and only once at startup. It describes a configuration an operator chose and can
+    /// change, not something going wrong at runtime, so repeating it per request would bury the
+    /// logs it is trying to be noticed in.
+    /// </remarks>
+    [LoggerMessage(
+        EventId = 4001,
+        Level = LogLevel.Warning,
+        Message = "Healthie: {Count} endpoint(s) that can change a pulse checker are reachable " +
+            "without authenticating -- {Routes}. Anyone who can reach this application can stop a " +
+            "checker or clear a failing streak, which hides an incident rather than reporting one. " +
+            "Pass requireAuthorization: true to AddHealthieController, or apply your own " +
+            "authorization to these endpoints.")]
+    public static partial void MutatingEndpointsAreUnauthenticated(ILogger logger, int count, string routes);
+}

src/Healthie.Api/Diagnostics/UnauthenticatedSurfaceWarning.cs

@@ -0,0 +1,92 @@
+using Healthie.Api.Routes;
+using Microsoft.AspNetCore.Authorization;
+using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Mvc.Authorization;
+using Microsoft.AspNetCore.Routing;
+using Microsoft.Extensions.Hosting;
+using Microsoft.Extensions.Logging;
+
+namespace Healthie.Api.Diagnostics;
+
+/// <summary>
+/// Says so, once and loudly, when the endpoints that can change a checker are reachable without
+/// authenticating.
+/// </summary>
+/// <remarks>
+/// <para>
+/// <c>AddHealthieController</c> does not require authorization unless it is asked to, which is a
+/// deliberate default -- this controller ships into someone else's MVC pipeline, and demanding a
+/// policy it knows nothing about would break every application that maps it. The consequence is
+/// that an application which maps it and does nothing else lets anyone stop a checker or clear a
+/// failing streak. That is not a wrong default so much as one worth being told about.
+/// </para>
+/// <para>
+/// Asked of the endpoints rather than of the flag that built them: the host may have applied
+/// authorization some other way -- a group, middleware, an
+/// <see cref="AuthorizeAttribute"/> of its own -- and a warning that cried wolf at a correctly
+/// secured application would be scrolled past within a week, which costs more than saying nothing.
+/// </para>
+/// </remarks>
+internal sealed class UnauthenticatedSurfaceWarning(
+    EndpointDataSource endpoints,
+    IHostApplicationLifetime lifetime,
+    ILogger<UnauthenticatedSurfaceWarning> logger) : IHostedService
+{
+    /// <summary>The methods that change something, as opposed to reporting it.</summary>
+    private static readonly string[] MutatingMethods = ["POST", "PUT", "PATCH", "DELETE"];
+
+    public Task StartAsync(CancellationToken cancellationToken)
+    {
+        // After the application has started, because that is when the endpoints exist. Reading them
+        // during StartAsync races the routing system that builds them.
+        lifetime.ApplicationStarted.Register(Warn);
+
+        return Task.CompletedTask;
+    }
+
+    public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask;
+
+    private void Warn()
+    {
+        var unprotected = endpoints.Endpoints
+            .OfType<RouteEndpoint>()
+            .Where(IsHealthieRoute)
+            .Where(Mutates)
+            .Where(endpoint => !IsProtected(endpoint))
+            .Select(endpoint => endpoint.RoutePattern.RawText)
+            .Where(route => route is not null)
+            .Distinct(StringComparer.OrdinalIgnoreCase)
+            .Order(StringComparer.OrdinalIgnoreCase)
+            .ToArray();
+
+        if (unprotected.Length == 0)
+        {
+            return;
+        }
+
+        Log.MutatingEndpointsAreUnauthenticated(logger, unprotected.Length, string.Join(", ", unprotected!));
+    }
+
+    /// <summary>
+    /// Whether anything on this endpoint requires an authenticated caller.
+    /// </summary>
+    /// <remarks>
+    /// Two shapes, because authorization arrives two ways and only one of them is metadata.
+    /// <c>RequireAuthorization()</c> and <c>[Authorize]</c> put an <see cref="IAuthorizeData"/> on
+    /// the endpoint. <c>AddHealthieController(requireAuthorization: true)</c> adds an
+    /// <see cref="AuthorizeFilter"/> through an MVC convention, and that is an
+    /// <c>IFilterMetadata</c> rather than an <see cref="IAuthorizeData"/> -- so looking only for the
+    /// latter warned about the one configuration that had asked for authorization by name.
+    /// </remarks>
+    private static bool IsProtected(Endpoint endpoint) =>
+        endpoint.Metadata.GetMetadata<IAuthorizeData>() is not null
+        || endpoint.Metadata.GetMetadata<AuthorizeFilter>() is not null;
+
+    private static bool IsHealthieRoute(RouteEndpoint endpoint) =>
+        endpoint.RoutePattern.RawText?.StartsWith(RoutesConstants.HealthieApiRoute, StringComparison.OrdinalIgnoreCase)
+            ?? false;
+
+    private static bool Mutates(RouteEndpoint endpoint) =>
+        endpoint.Metadata.GetMetadata<HttpMethodMetadata>() is { } methods
+        && methods.HttpMethods.Any(method => MutatingMethods.Contains(method, StringComparer.OrdinalIgnoreCase));
+}

src/Healthie.Api/StartupExtensions.cs

@@ -1,6 +1,9 @@
 using Healthie.Api.Controllers;
 using Healthie.Api.Conventions;
+using Healthie.Api.Diagnostics;
 using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.DependencyInjection.Extensions;
+using Microsoft.Extensions.Hosting;
 
 namespace Healthie.Api;
 
@@ -37,6 +40,11 @@ public static IMvcBuilder AddHealthieController(
         // Ensure HealthCheckersController from Healthie.Api assembly is discovered.
         mvcBuilder.AddApplicationPart(typeof(HealthCheckersController).Assembly);
 
+        // Says so at startup if the endpoints that can change a checker end up reachable without
+        // authenticating. TryAdd because calling this twice should not warn twice.
+        services.TryAddEnumerable(
+            ServiceDescriptor.Singleton<IHostedService, UnauthenticatedSurfaceWarning>());
+
         return mvcBuilder;
     }
 }

src/Healthie.Dashboard/Diagnostics/Log.cs

@@ -0,0 +1,26 @@
+using Microsoft.Extensions.Logging;
+
+namespace Healthie.Dashboard.Diagnostics;
+
+/// <summary>
+/// The log messages the dashboard writes.
+/// </summary>
+/// <remarks>
+/// Source-generated, as in the other packages, and with event ids in their own 5000 range so a
+/// filter written against them keeps meaning what it meant.
+/// </remarks>
+internal static partial class Log
+{
+    /// <remarks>
+    /// Warning, and only once at startup. It describes a configuration an operator chose and can
+    /// change, not something going wrong at runtime.
+    /// </remarks>
+    [LoggerMessage(
+        EventId = 5001,
+        Level = LogLevel.Warning,
+        Message = "Healthie: the dashboard at {Path} is reachable without authenticating and its " +
+            "controls are on, so anyone who can reach this application can pause a checker or reset " +
+            "a failing streak. Chain RequireAuthorization() onto MapHealthieUI(), or set " +
+            "HealthieUIOptions.AllowMutations to false to serve it read-only.")]
+    public static partial void DashboardIsUnauthenticatedAndWritable(ILogger logger, string path);
+}

src/Healthie.Dashboard/Diagnostics/UnauthenticatedDashboardWarning.cs

@@ -0,0 +1,68 @@
+using Microsoft.AspNetCore.Authorization;
+using Microsoft.AspNetCore.Routing;
+using Microsoft.Extensions.Hosting;
+using Microsoft.Extensions.Logging;
+
+namespace Healthie.Dashboard.Diagnostics;
+
+/// <summary>
+/// Says so, once and loudly, when the dashboard is reachable without authenticating and its
+/// controls are on.
+/// </summary>
+/// <remarks>
+/// <para>
+/// <see cref="HealthieUIOptions.AllowMutations"/> defaults to <c>true</c> because the dashboard
+/// exists to manage checkers and a read-only default would make every first run look broken. It is
+/// not authorization and does not pretend to be: it decides which controls are rendered, for
+/// everyone, and <c>RequireAuthorization</c> on the mapped endpoint is the other half. An
+/// application that maps the dashboard and stops there has given anyone who can reach it the
+/// ability to pause a checker or reset a failing streak.
+/// </para>
+/// <para>
+/// Asked of the endpoint rather than assumed from the option, so an application that secured it --
+/// by chaining <c>RequireAuthorization</c>, by an endpoint group, by its own attribute -- is not
+/// warned at. A warning that fires on correctly secured applications gets filtered out, and then it
+/// is not there for the one that needs it.
+/// </para>
+/// </remarks>
+internal sealed class UnauthenticatedDashboardWarning(
+    EndpointDataSource endpoints,
+    HealthieUIOptions options,
+    IHostApplicationLifetime lifetime,
+    ILogger<UnauthenticatedDashboardWarning> logger) : IHostedService
+{
+    public Task StartAsync(CancellationToken cancellationToken)
+    {
+        // Nothing to say when the controls are not rendered: reaching the board then shows health
+        // and changes nothing.
+        if (!options.AllowMutations)
+        {
+            return Task.CompletedTask;
+        }
+
+        // After the application has started, because that is when the endpoints exist.
+        lifetime.ApplicationStarted.Register(Warn);
+
+        return Task.CompletedTask;
+    }
+
+    public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask;
+
+    private void Warn()
+    {
+        var dashboard = endpoints.Endpoints
+            .OfType<RouteEndpoint>()
+            .FirstOrDefault(endpoint => string.Equals(
+                "/" + endpoint.RoutePattern.RawText?.TrimStart('/'),
+                StartupExtensions.DashboardPath,
+                StringComparison.OrdinalIgnoreCase));
+
+        // Not mapped at all, or already behind an authorization policy.
+        if (dashboard is null || dashboard.Metadata.GetMetadata<IAuthorizeData>() is not null)
+        {
+            return;
+        }
+
+        Log.DashboardIsUnauthenticatedAndWritable(logger, StartupExtensions.DashboardPath);
+    }
+}

src/Healthie.Dashboard/StartupExtensions.cs

@@ -1,8 +1,11 @@
+using Healthie.Dashboard.Diagnostics;
 using Healthie.Dashboard.Services;
 using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Routing;
 using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.DependencyInjection.Extensions;
+using Microsoft.Extensions.Hosting;
 using System.Net;
 
 namespace Healthie.Dashboard;
@@ -42,6 +45,11 @@ public static IServiceCollection AddHealthieUI(
         services.AddScoped<IHealthieDashboardService, HealthieDashboardService>();
         services.AddScoped<HealthieThemeState>();
 
+        // Says so at startup if the board ends up reachable without authenticating while its
+        // controls are on. TryAdd because calling this twice should not warn twice.
+        services.TryAddEnumerable(
+            ServiceDescriptor.Singleton<IHostedService, UnauthenticatedDashboardWarning>());
+
         return services;
     }