diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 94fe9ae..c741330 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -20,11 +20,11 @@ on: tags: - 'v*' +# Nothing at the top level, so a job added later starts with no write access and has to ask for +# what it needs. Scorecard rates a workflow-wide `contents: write` a high finding for exactly that +# reason: it is inherited by every job, including ones nobody thought about when it was granted. permissions: - contents: write - # Lets this workflow ask GitHub for the signed OIDC token that NuGet.org trades for a - # short-lived publishing key. Without it the login step below cannot get a token at all. - id-token: write + contents: read env: CONFIGURATION: Release @@ -35,6 +35,13 @@ jobs: release: runs-on: ubuntu-latest + permissions: + # Creating the GitHub release at the end of this job. + contents: write + # Lets this job ask GitHub for the signed OIDC token that NuGet.org trades for a short-lived + # publishing key. Without it the login step below cannot get a token at all. + id-token: write + steps: - name: Checkout uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 diff --git a/CHANGELOG.md b/CHANGELOG.md index 65d0987..124369f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,34 @@ Eleven new packages, and the schedule model that several of them needed. Everyth additive: no public API was removed or changed, and an application that upgrades without touching its code behaves exactly as it did. +### Fixed + +- **The dashboard's own page did not encode its title.** `MapHealthieUI` builds that one page as a + string rather than through Razor, which encodes every interpolation for you, so a + `HealthieUIOptions.DashboardTitle` built from anything the host did not write itself could close + the title element and open a script one. +- **A dashboard component left its handler behind when it was disposed.** The service is scoped to a + circuit, so it was assumed the circuit ending released everything; that holds only while the + dashboard is mounted once per circuit, and a host that routes to it inside its own layout builds a + new one every time the user navigates back. `IHealthieDashboardService.UnsubscribeFromStateChangesAsync` + is the missing half, and the component calls it. +- **Two callers scheduling one checker at once could leave a timer nobody could stop.** Installing a + schedule was "cancel the old one, then start the new one", and only the last one stored was + reachable; the other kept triggering, could not be unscheduled, and held a linked + `CancellationTokenSource` that was never disposed. +- **`HealthieMcpOptions.MaxHistoryPageSize` was documented and never read.** `get_check_history` + clamped to a hard-coded 200 instead, so a host that lowered the option got no such thing. +- **The relational table-name guard admitted a name with a trailing newline.** In .NET `$` matches + immediately before one, so a table name ending in one passed a check whose own error message says + it does not. Nothing could be smuggled through it -- a lone trailing newline is whitespace to + every engine here -- but the guard now ends at `\z` and means what it says. +- **The release workflow granted `contents: write` to every job in it.** Only the one job that + creates the GitHub release needs it, and the top level now grants nothing but read, so a job added + later starts with no write access rather than inheriting it. +- **A checker name from a REST route reached the log with its control characters intact.** The + not-found branch logs a name precisely when it matches nothing, percent-encoded CR and LF arrive + decoded, and a log sink writing plain text writes them as line breaks. + ### Added - **Optimistic concurrency on `IStateProvider`.** A state write can now be made conditional on the diff --git a/src/Healthie.Api/Controllers/HealthCheckersController.cs b/src/Healthie.Api/Controllers/HealthCheckersController.cs index ac064df..d4d407c 100644 --- a/src/Healthie.Api/Controllers/HealthCheckersController.cs +++ b/src/Healthie.Api/Controllers/HealthCheckersController.cs @@ -99,7 +99,7 @@ public async Task SetCheckerInterval(string checkerName, [FromQue var checkers = await pulsesScheduler.GetPulseCheckersAsync(cancellationToken).ConfigureAwait(false); if (!checkers.ContainsKey(checkerName)) { - logger?.LogWarning("Checker '{CheckerName}' not found for setting interval.", checkerName); + logger?.LogWarning("Checker '{CheckerName}' not found for setting interval.", ForLog(checkerName)); return NotFound($"Checker '{checkerName}' not found."); } @@ -108,7 +108,7 @@ public async Task SetCheckerInterval(string checkerName, [FromQue } catch (Exception ex) { - logger?.LogError(ex, "Error setting interval for checker '{CheckerName}'.", checkerName); + logger?.LogError(ex, "Error setting interval for checker '{CheckerName}'.", ForLog(checkerName)); return StatusCode(StatusCodes.Status500InternalServerError, "An unexpected error occurred."); } } @@ -136,7 +136,7 @@ public async Task SetCheckerThreshold(string checkerName, [FromQu var checkers = await pulsesScheduler.GetPulseCheckersAsync(cancellationToken).ConfigureAwait(false); if (!checkers.ContainsKey(checkerName)) { - logger?.LogWarning("Checker '{CheckerName}' not found for setting threshold.", checkerName); + logger?.LogWarning("Checker '{CheckerName}' not found for setting threshold.", ForLog(checkerName)); return NotFound($"Checker '{checkerName}' not found."); } @@ -145,7 +145,7 @@ public async Task SetCheckerThreshold(string checkerName, [FromQu } catch (Exception ex) { - logger?.LogError(ex, "Error setting threshold for checker '{CheckerName}'.", checkerName); + logger?.LogError(ex, "Error setting threshold for checker '{CheckerName}'.", ForLog(checkerName)); return StatusCode(StatusCodes.Status500InternalServerError, "An unexpected error occurred."); } } @@ -172,7 +172,7 @@ public async Task StartChecker(string checkerName, CancellationTo var checkers = await pulsesScheduler.GetPulseCheckersAsync(cancellationToken).ConfigureAwait(false); if (!checkers.ContainsKey(checkerName)) { - logger?.LogWarning("Checker '{CheckerName}' not found for starting.", checkerName); + logger?.LogWarning("Checker '{CheckerName}' not found for starting.", ForLog(checkerName)); return NotFound($"Checker '{checkerName}' not found."); } @@ -181,7 +181,7 @@ public async Task StartChecker(string checkerName, CancellationTo } catch (Exception ex) { - logger?.LogError(ex, "Error starting checker '{CheckerName}'.", checkerName); + logger?.LogError(ex, "Error starting checker '{CheckerName}'.", ForLog(checkerName)); return StatusCode(StatusCodes.Status500InternalServerError, "An unexpected error occurred."); } } @@ -208,7 +208,7 @@ public async Task StopChecker(string checkerName, CancellationTok var checkers = await pulsesScheduler.GetPulseCheckersAsync(cancellationToken).ConfigureAwait(false); if (!checkers.ContainsKey(checkerName)) { - logger?.LogWarning("Checker '{CheckerName}' not found for stopping.", checkerName); + logger?.LogWarning("Checker '{CheckerName}' not found for stopping.", ForLog(checkerName)); return NotFound($"Checker '{checkerName}' not found."); } @@ -217,7 +217,7 @@ public async Task StopChecker(string checkerName, CancellationTok } catch (Exception ex) { - logger?.LogError(ex, "Error stopping checker '{CheckerName}'.", checkerName); + logger?.LogError(ex, "Error stopping checker '{CheckerName}'.", ForLog(checkerName)); return StatusCode(StatusCodes.Status500InternalServerError, "An unexpected error occurred."); } } @@ -244,7 +244,7 @@ public async Task TriggerChecker(string checkerName, Cancellation var checkers = await pulsesScheduler.GetPulseCheckersAsync(cancellationToken).ConfigureAwait(false); if (!checkers.TryGetValue(checkerName, out var checker)) { - logger?.LogWarning("Checker '{CheckerName}' not found for triggering.", checkerName); + logger?.LogWarning("Checker '{CheckerName}' not found for triggering.", ForLog(checkerName)); return NotFound($"Checker '{checkerName}' not found."); } @@ -253,7 +253,7 @@ public async Task TriggerChecker(string checkerName, Cancellation } catch (Exception ex) { - logger?.LogError(ex, "Error triggering checker '{CheckerName}'.", checkerName); + logger?.LogError(ex, "Error triggering checker '{CheckerName}'.", ForLog(checkerName)); return StatusCode(StatusCodes.Status500InternalServerError, "An unexpected error occurred."); } } @@ -280,7 +280,7 @@ public async Task ResetChecker(string checkerName, CancellationTo var checkers = await pulsesScheduler.GetPulseCheckersAsync(cancellationToken).ConfigureAwait(false); if (!checkers.ContainsKey(checkerName)) { - logger?.LogWarning("Checker '{CheckerName}' not found for resetting.", checkerName); + logger?.LogWarning("Checker '{CheckerName}' not found for resetting.", ForLog(checkerName)); return NotFound($"Checker '{checkerName}' not found."); } @@ -290,8 +290,31 @@ public async Task ResetChecker(string checkerName, CancellationTo } catch (Exception ex) { - logger?.LogError(ex, "Error resetting checker '{CheckerName}'.", checkerName); + logger?.LogError(ex, "Error resetting checker '{CheckerName}'.", ForLog(checkerName)); return StatusCode(StatusCodes.Status500InternalServerError, "An unexpected error occurred."); } } + + /// + /// Strips control characters from a name before it is written to a log. + /// + /// The name as the route supplied it. + /// The name, with any control character replaced. + /// + /// The name comes from the route, so a caller chooses it, and it does not have to match a real + /// checker -- the not-found branch logs it precisely when it does not. Percent-encoded CR and LF + /// arrive here decoded, and a log sink that writes plain text writes them as line breaks, which + /// lets a caller forge whole log entries. Structured sinks that encode their values are already + /// safe; this makes the others safe too. + /// + internal static string ForLog(string name) + { + if (!name.Any(char.IsControl)) + { + return name; + } + + return new string([.. name.Select(c => char.IsControl(c) ? '\uFFFD' : c)]); + } + } diff --git a/src/Healthie.Api/Healthie.Api.csproj b/src/Healthie.Api/Healthie.Api.csproj index 414e0c3..583ee94 100644 --- a/src/Healthie.Api/Healthie.Api.csproj +++ b/src/Healthie.Api/Healthie.Api.csproj @@ -6,6 +6,10 @@ $(HealthieCommonTags);api;aspnetcore;rest + + + + diff --git a/src/Healthie.Dashboard/Components/HealthieDashboard.razor.cs b/src/Healthie.Dashboard/Components/HealthieDashboard.razor.cs index 4317b84..8f9cec4 100644 Binary files a/src/Healthie.Dashboard/Components/HealthieDashboard.razor.cs and b/src/Healthie.Dashboard/Components/HealthieDashboard.razor.cs differ diff --git a/src/Healthie.Dashboard/Services/HealthieDashboardService.cs b/src/Healthie.Dashboard/Services/HealthieDashboardService.cs index dcc9bb2..d4273a2 100644 --- a/src/Healthie.Dashboard/Services/HealthieDashboardService.cs +++ b/src/Healthie.Dashboard/Services/HealthieDashboardService.cs @@ -321,6 +321,24 @@ public async Task SubscribeToStateChangesAsync( } } + /// + public async Task UnsubscribeFromStateChangesAsync( + Func onStateChanged, + CancellationToken cancellationToken = default) + { + ArgumentNullException.ThrowIfNull(onStateChanged); + + await _handlersLock.WaitAsync(cancellationToken).ConfigureAwait(false); + try + { + _handlers.Remove(onStateChanged); + } + finally + { + _handlersLock.Release(); + } + } + /// /// Hands a state change to every subscriber. /// @@ -330,7 +348,22 @@ public async Task SubscribeToStateChangesAsync( /// private async Task NotifyAsync(string name, PulseCheckerState state) { - foreach (var handler in _handlers.ToArray()) + // Copied under the lock: this runs on whichever thread ran the check, while a component + // mounting or being disposed may be writing the same list. + await _handlersLock.WaitAsync().ConfigureAwait(false); + + Func[] handlers; + + try + { + handlers = [.. _handlers]; + } + finally + { + _handlersLock.Release(); + } + + foreach (var handler in handlers) { try { diff --git a/src/Healthie.Dashboard/Services/IHealthieDashboardService.cs b/src/Healthie.Dashboard/Services/IHealthieDashboardService.cs index cd7124d..5b97ebf 100644 --- a/src/Healthie.Dashboard/Services/IHealthieDashboardService.cs +++ b/src/Healthie.Dashboard/Services/IHealthieDashboardService.cs @@ -17,13 +17,35 @@ internal interface IHealthieDashboardService : IAsyncDisposable /// A token to monitor for cancellation requests. /// /// Subscribing is what starts the flow of updates, so this both registers the handler and - /// attaches to the checkers. Handlers are released when the service is disposed, which happens - /// when the circuit ends. + /// attaches to the checkers. Every subscription should be matched by + /// when the subscriber goes away. /// Task SubscribeToStateChangesAsync( Func onStateChanged, CancellationToken cancellationToken = default); + /// + /// Stops handing state changes to a handler that was subscribed earlier. + /// + /// The handler passed to . + /// A token to monitor for cancellation requests. + /// + /// + /// This service is scoped to a circuit, so it was assumed that letting the circuit end released + /// everything. That holds only while the dashboard is mounted once per circuit, and it is not: + /// a host that routes to HealthieDashboard inside its own layout builds a new component + /// every time the user navigates back to it, on the same circuit. Without this, each of those + /// left its handler behind, and every later state change was handed to a component that had + /// been torn down. + /// + /// + /// Passing a handler that is not subscribed does nothing. + /// + /// + Task UnsubscribeFromStateChangesAsync( + Func onStateChanged, + CancellationToken cancellationToken = default); + /// /// Triggers every registered pulse checker. /// diff --git a/src/Healthie.Dashboard/StartupExtensions.cs b/src/Healthie.Dashboard/StartupExtensions.cs index c48d2a8..e3d7b25 100644 --- a/src/Healthie.Dashboard/StartupExtensions.cs +++ b/src/Healthie.Dashboard/StartupExtensions.cs @@ -3,6 +3,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.DependencyInjection; +using System.Net; namespace Healthie.Dashboard; @@ -63,7 +64,27 @@ public static IEndpointConventionBuilder MapHealthieUI( return endpoints.MapGet(DashboardPath, (HttpContext context) => { var opts = context.RequestServices.GetService(); - var title = opts?.DashboardTitle ?? "System Health"; + + context.Response.ContentType = "text/html"; + return context.Response.WriteAsync(BuildPage(opts?.DashboardTitle)); + }); + } + + /// + /// The host page that renders the dashboard component. + /// + /// + /// Separate from the endpoint so it can be asserted on without a host. This is the one place in + /// the library that builds HTML as a string instead of letting Razor build it, which is why the + /// title is encoded here by hand -- Razor would have done it everywhere else. + /// + internal static string BuildPage(string? dashboardTitle) + { + // The title is a host's setting, not a visitor's, so this matters only when a host + // builds it from something it did not write -- a per-tenant label, a value out of a + // database -- but that is a normal thing to do, and nothing here would have stopped it + // closing the title element and opening a script one. + var title = WebUtility.HtmlEncode(dashboardTitle ?? "System Health"); var html = $$""" @@ -89,8 +110,6 @@ embed the component in a page of their own and own their own body. """; - context.Response.ContentType = "text/html"; - return context.Response.WriteAsync(html); - }); + return html; } } diff --git a/src/Healthie.DependencyInjection/TimerPulseScheduler.cs b/src/Healthie.DependencyInjection/TimerPulseScheduler.cs index 14a13b8..f1e29a7 100644 --- a/src/Healthie.DependencyInjection/TimerPulseScheduler.cs +++ b/src/Healthie.DependencyInjection/TimerPulseScheduler.cs @@ -24,6 +24,20 @@ public sealed class TimerPulseScheduler : IPulseScheduler, IAsyncDisposable, IDi private static readonly TimeSpan MaxDelay = TimeSpan.FromHours(1); private readonly ConcurrentDictionary _timers = new(); + + /// + /// Serialises scheduling, so a checker is never left running under a timer nobody holds. + /// + /// + /// The dictionary is concurrent, but installing a schedule is "stop the old one, then start the + /// new one" -- two operations, which a second caller can interleave with. A lock is the whole + /// fix: scheduling happens at startup and when somebody changes an interval, so it is never on + /// a hot path. + /// + private readonly SemaphoreSlim _scheduling = new(1, 1); + + /// Set once the host has stopped this scheduler. Read and written under the lock. + private bool _disposed; private readonly ILogger? _logger; /// @@ -61,32 +75,75 @@ public async Task ScheduleAsync( // schedule should not be stopped by a request carrying a bad one. var cron = schedule.IsCron ? ParseCron(schedule.CronExpression!, checker.Name) : null; - await UnscheduleAsync(checker, cancellationToken).ConfigureAwait(false); + // Stopping the old schedule and installing the new one is two steps, and two callers + // scheduling one checker at once -- an interval changed from the dashboard while the + // scheduler starts it, say -- could each install a timer. Only the last would be in the + // dictionary; the other kept running with nothing able to reach it, and its linked + // CancellationTokenSource was never disposed, so its registration on the parent token + // outlived it too. + await _scheduling.WaitAsync(cancellationToken).ConfigureAwait(false); - var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - _timers[checker.Name] = cts; + try + { + // A request can outlive the decision to shut down -- an interval changed from the API + // while the host stops. Installing a timer now would leave one running that Dispose has + // already been past, so there would be nothing left to stop it. Quietly rather than by + // throwing: the host is going away, and that is not the caller's mistake. + if (_disposed) + { + return; + } - _ = Task.Run( - () => cron is null - ? RunPeriodicallyAsync(checker, schedule.Period!.Value, cts.Token) - : RunOnCronAsync(checker, cron, cts.Token), - cts.Token); + StopExisting(checker.Name); + + var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + _timers[checker.Name] = cts; + + _ = Task.Run( + () => cron is null + ? RunPeriodicallyAsync(checker, schedule.Period!.Value, cts.Token) + : RunOnCronAsync(checker, cron, cts.Token), + cts.Token); + } + finally + { + _scheduling.Release(); + } + } + + /// + /// Cancels and disposes the timer for a checker, if it has one. + /// + /// + /// Takes no lock, so it can be called from inside one. is the + /// same thing with the lock held. + /// + private void StopExisting(string name) + { + if (_timers.TryRemove(name, out var existing)) + { + existing.Cancel(); + existing.Dispose(); + } } /// - public Task UnscheduleAsync( + public async Task UnscheduleAsync( IPulseChecker checker, CancellationToken cancellationToken = default) { ArgumentNullException.ThrowIfNull(checker); - if (_timers.TryRemove(checker.Name, out var cts)) + await _scheduling.WaitAsync(cancellationToken).ConfigureAwait(false); + + try { - cts.Cancel(); - cts.Dispose(); + StopExisting(checker.Name); + } + finally + { + _scheduling.Release(); } - - return Task.CompletedTask; } /// @@ -228,13 +285,35 @@ private static CronExpression ParseCron(string expression, string checkerName) /// public void Dispose() { - foreach (var kvp in _timers) + // Shutdown races the last requests: an interval can be changed from the API while the host + // is stopping. Without the lock this is a third writer of _timers, and clearing the + // dictionary between an in-flight schedule's stop and its install drops a live timer without + // cancelling it -- the leak the lock exists to prevent, arriving by another door. + var acquired = _scheduling.Wait(TimeSpan.FromSeconds(5)); + + try { - kvp.Value.Cancel(); - kvp.Value.Dispose(); + foreach (var kvp in _timers) + { + kvp.Value.Cancel(); + kvp.Value.Dispose(); + } + + _timers.Clear(); + _disposed = true; + } + finally + { + if (acquired) + { + _scheduling.Release(); + } } - _timers.Clear(); + // The semaphore is deliberately not disposed. SemaphoreSlim only needs it once its + // AvailableWaitHandle has been asked for, which nothing here does, and disposing it while + // another thread sits in WaitAsync leaves that thread waiting for ever rather than throwing + // -- a hung shutdown instead of a noisy one. } /// diff --git a/src/Healthie.Mcp/HealthieTools.cs b/src/Healthie.Mcp/HealthieTools.cs index bc238e3..0308489 100644 --- a/src/Healthie.Mcp/HealthieTools.cs +++ b/src/Healthie.Mcp/HealthieTools.cs @@ -16,8 +16,18 @@ namespace Healthie.Mcp; /// and are only exposed when they are turned on. /// [McpServerToolType] -public sealed class HealthieTools(IPulsesScheduler pulsesScheduler) +public sealed class HealthieTools(IPulsesScheduler pulsesScheduler, HealthieMcpOptions? options = null) { + /// + /// The host's options, or the defaults. + /// + /// + /// Optional so that adding it does not change how this type is constructed. DI always supplies + /// it -- AddHealthieMcp registers it -- so the fallback is for a caller that builds this + /// by hand, which used to be the only way it worked. + /// + private readonly HealthieMcpOptions _options = options ?? new HealthieMcpOptions(); + /// Returns the current health of every pulse checker. [McpServerTool(Name = "get_health_states")] [Description("Returns the current health of every monitored component, including the last result and when it last ran. Start here to see the overall health of the system.")] @@ -74,7 +84,10 @@ public async Task GetCheckHistoryAsync( // History is recorded oldest-first; a reader almost always wants the newest first. var newestFirst = history.AsEnumerable().Reverse().ToList(); - var page = newestFirst.Skip(Math.Max(offset, 0)).Take(Math.Clamp(limit, 1, 200)).ToList(); + // The host's cap, not a number hard-coded here. It was configurable and ignored, so a host + // that lowered it to keep responses small got no such thing. + var cap = Math.Max(_options.MaxHistoryPageSize, 1); + var page = newestFirst.Skip(Math.Max(offset, 0)).Take(Math.Clamp(limit, 1, cap)).ToList(); return new HistoryPage( name, diff --git a/src/Healthie.StateProviding.Relational/RelationalDialect.cs b/src/Healthie.StateProviding.Relational/RelationalDialect.cs index fc51118..0f5da4e 100644 --- a/src/Healthie.StateProviding.Relational/RelationalDialect.cs +++ b/src/Healthie.StateProviding.Relational/RelationalDialect.cs @@ -90,8 +90,15 @@ public sealed record RelationalDialect( /// /// A table name, optionally schema-qualified. Anything else is refused rather than interpolated. /// + /// + /// Ends at \z rather than $, which in .NET also matches immediately before a + /// single trailing newline -- so "state\n" passed a guard whose own error message says + /// it does not. A lone trailing newline is only whitespace to every engine here, so nothing + /// could be smuggled through it, but a guard that admits what it documents as impossible is + /// worth closing before something later depends on it. + /// private static readonly Regex SafeTableName = new( - @"^[A-Za-z_][A-Za-z0-9_]*(\.[A-Za-z_][A-Za-z0-9_]*)?$", + @"^[A-Za-z_][A-Za-z0-9_]*(\.[A-Za-z_][A-Za-z0-9_]*)?\z", RegexOptions.CultureInvariant, TimeSpan.FromSeconds(1)); diff --git a/tests/Healthie.Tests.Unit/DashboardServiceTests.cs b/tests/Healthie.Tests.Unit/DashboardServiceTests.cs index bad0e79..953f4b6 100644 --- a/tests/Healthie.Tests.Unit/DashboardServiceTests.cs +++ b/tests/Healthie.Tests.Unit/DashboardServiceTests.cs @@ -29,6 +29,50 @@ private static (IHealthieDashboardService Service, ControllablePulseChecker Chec return (CreateService([checker, .. extra]), checker); } + /// + /// A handler stayed registered for the life of the circuit, and there was no way to drop one. + /// + /// + /// The service is scoped to a circuit, so it was assumed the circuit ending released everything. + /// That holds only while the dashboard is mounted once per circuit, and a host that routes to it + /// inside its own layout builds a new component every time the user navigates back -- each one + /// leaving its handler behind, still being called after it was torn down. + /// + [Fact] + public async Task UnsubscribeFromStateChangesAsync_StopsTheHandlerBeingCalled() + { + var (service, checker) = CreateServiceWithRealChecker(); + await using var _ = service; + + var calls = 0; + + Task Handler(string name, PulseCheckerState state) + { + Interlocked.Increment(ref calls); + return Task.CompletedTask; + } + + await service.SubscribeToStateChangesAsync(Handler, Ct); + await checker.TriggerAsync(Ct); + + var whileSubscribed = Volatile.Read(ref calls); + Assert.True(whileSubscribed > 0, "the handler should be called while it is subscribed"); + + await service.UnsubscribeFromStateChangesAsync(Handler, Ct); + await checker.TriggerAsync(Ct); + + Assert.Equal(whileSubscribed, Volatile.Read(ref calls)); + } + + [Fact] + public async Task UnsubscribeFromStateChangesAsync_ForAHandlerThatIsNotSubscribed_DoesNothing() + { + var (service, _) = CreateServiceWithRealChecker(); + await using var __ = service; + + await service.UnsubscribeFromStateChangesAsync((_, _) => Task.CompletedTask, Ct); + } + [Fact] public async Task SubscribeToStateChangesAsync_WhenACheckerChangesState_NotifiesTheSubscriber() { diff --git a/tests/Healthie.Tests.Unit/HardeningTests.cs b/tests/Healthie.Tests.Unit/HardeningTests.cs new file mode 100644 index 0000000..5c6aa1a --- /dev/null +++ b/tests/Healthie.Tests.Unit/HardeningTests.cs @@ -0,0 +1,98 @@ +using Healthie.Api.Controllers; +using Healthie.StateProviding.Relational; +using System.Text.RegularExpressions; + +namespace Healthie.Tests.Unit; + +/// +/// Guards that did not guard quite what they said they did. +/// +public class HardeningTests +{ + /// + /// The dashboard page is the one place in the library that builds HTML as a string instead of + /// letting Razor build it, and the title went in unencoded. + /// + [Fact] + public void ADashboardTitle_CannotCloseTheTitleElement() + { + var page = Healthie.Dashboard.StartupExtensions.BuildPage(""); + + // One closing title tag: the one the page itself writes. + Assert.Equal(1, Regex.Matches(page, "", RegexOptions.IgnoreCase).Count); + Assert.DoesNotContain("